Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renovate PRs may include yarn.lock with mismatched lockfile #30051

Closed
MikeMcC399 opened this issue Aug 18, 2024 · 3 comments · Fixed by #30060
Closed

Renovate PRs may include yarn.lock with mismatched lockfile #30051

MikeMcC399 opened this issue Aug 18, 2024 · 3 comments · Fixed by #30060
Assignees
Labels
type: chore Work is required w/ no deliverable to end user

Comments

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Aug 18, 2024

Current behavior

When yarn (install) is run against the repo after a Renovate-generated PR has been merged, a git diff may occur.

It appears that Renovate runs yarn without using the postinstall step yarn-deduplicate --strategy highest.

const postInstallCommands = {
local: 'patch-package && yarn-deduplicate --strategy=highest && lerna run rebuild-better-sqlite3 --scope @packages/server && yarn build && yarn build-v8-snapshot-dev',
ci: 'patch-package && lerna run rebuild-better-sqlite3 --scope @packages/server',
}

Desired behavior

If Renovate PRs include updated yarn.lock files, then they should agree with the result of independently running yarn.

Test code to reproduce

On Ubuntu 22.04.4 LTS, Node.js 18.17.1, Yarn 1.22.22

npm install [email protected] -g

git clone https://github.com/cypress-io/cypress
git clean -xfd # in case of a repeat run
git checkout 3f4ab2959996ffd01117fa9c103d85f5209cf1af # after merge of #29610
# HEAD is now at 3f4ab29599 chore(deps): update dependency cypress-example-kitchensink to v3.0.0 (#29610)
yarn
git diff

No differences reported.
Now go to one commit later in the develop branch history:

git clean -xfd
git checkout ceb11a355a4e60f9833412cb03d56756b0dc4800 # after merge of #29571
# Previous HEAD position was 3f4ab29599 chore(deps): update dependency cypress-example-kitchensink to v3.0.0 (#29610)
# HEAD is now at ceb11a355a chore(deps): update dependency @antfu/utils to ^0.7.8 (#29571)
yarn
git diff

Now yarn.lock is showing differences - see Debug Logs below.

For reference, here is the history snippet:

$ git log --oneline -2
ceb11a355a (HEAD) chore(deps): update dependency @antfu/utils to ^0.7.8 (#29571)
3f4ab29599 chore(deps): update dependency cypress-example-kitchensink to v3.0.0 (#29610)

Cypress Version

13.13.3

Node version

v18.17.1

Operating System

Ubuntu 22.04.4 LTS

Debug Logs

$ git diff
diff --git a/yarn.lock b/yarn.lock
index 9ca860fede..b61d3410ae 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -149,12 +149,7 @@
   resolved "https://registry.yarnpkg.com/@antfu/utils/-/utils-0.5.2.tgz#8c2d931ff927be0ebe740169874a3d4004ab414b"
   integrity sha512-CQkeV+oJxUazwjlHD0/3ZD08QWKuGQkhnrKo3e6ly5pd48VUpXbb77q0xMU4+vc2CkJnDS02Eq/M9ugyX20XZA==

-"@antfu/utils@^0.7.2", "@antfu/utils@^0.7.7":
-  version "0.7.7"
-  resolved "https://registry.yarnpkg.com/@antfu/utils/-/utils-0.7.7.tgz#26ea493a831b4f3a85475e7157be02fb4eab51fb"
-  integrity sha512-gFPqTG7otEJ8uP6wrhDv6mqwGWYZKNvAcCq6u9hOj0c+IKCEsY4L1oC9trPq2SaWIzAfHvqfBDxF591JkMf+kg==
-
-"@antfu/utils@^0.7.8":
+"@antfu/utils@^0.7.2", "@antfu/utils@^0.7.7", "@antfu/utils@^0.7.8":
   version "0.7.8"
   resolved "https://registry.yarnpkg.com/@antfu/utils/-/utils-0.7.8.tgz#86cb0974bcab7e64e29b57d6d9021102307257de"
   integrity sha512-rWQkqXRESdjXtc+7NRfK9lASQjpXJu1ayp7qi1d23zZorY+wBHVLHHoVcMsEnkqEBWTFqbztO7/QdJFzyEcLTg==

Other

Running yarn outside of Renovate includes the postinstall command yarn-deduplicate --strategy highest.

In the above example, this removes the unneeded @antfu/[email protected] since semver needs are all satisfied by @antfu/[email protected].

This can be simulated by separately executing the following:

yarn run yarn-deduplicate --strategy highest

Adding this command to the original steps to reproduce:

npm install [email protected] -g

git clone https://github.com/cypress-io/cypress
git clean -xfd # in case of a repeat run
git checkout 3f4ab2959996ffd01117fa9c103d85f5209cf1af # after merge of #29610
# HEAD is now at 3f4ab29599 chore(deps): update dependency cypress-example-kitchensink to v3.0.0 (#29610)
yarn
git checkout ceb11a355a4e60f9833412cb03d56756b0dc4800 # after merge of #29571
# Previous HEAD position was 3f4ab29599 chore(deps): update dependency cypress-example-kitchensink to v3.0.0 (#29610)
# HEAD is now at ceb11a355a chore(deps): update dependency @antfu/utils to ^0.7.8 (#29571)
yarn run yarn-deduplicate --strategy highest
git diff

shows the same git diff as in the original steps to reproduce.

@MikeMcC399
Copy link
Contributor Author

It may be possible to resolve this issue by extending .husky/pre-commit actions to run yarn-deduplicate. This method is also successfully used in https://github.com/cypress-io/github-action/blob/master/.husky/pre-commit for a similar problem.

yarn run yarn-deduplicate --strategy highest
if [ "$(git diff --ignore-space-at-eol | wc -l)" -gt "0" ]; then
    echo yarn.lock deduplication changes detected
    echo yarn.lock added to this commit
    git add yarn.lock
fi

If this sounds like a reasonable solution then it would be better for a PR to come from the core Cypress.io team as I have no write access to the repo and no access to Renovate logs etc., so I would be unable to trigger any test runs using Renovate.

@jennifer-shehane jennifer-shehane added the type: chore Work is required w/ no deliverable to end user label Aug 19, 2024
@jennifer-shehane
Copy link
Member

@MikeMcC399 It looks like Renovate has an option to turn this on: https://docs.renovatebot.com/configuration-options/#postupdateoptions - yarnDedupeHighest

@MikeMcC399
Copy link
Contributor Author

@jennifer-shehane

It looks like Renovate has an option to turn this on: https://docs.renovatebot.com/configuration-options/#postupdateoptions - yarnDedupeHighest

Thanks for uncovering that Renovate option! In that case the option could be added to the config in renovate.json, which would put it in the place where it really belongs. I haven't noticed any other process being responsible for leaving out deduplication, so the Husky hook suggestion would not need to be implemented.

The same comment about submitting a PR and testing still applies though, so I think that it would be better for the Cypress.io to do it as I don't have the right access to be effective in testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore Work is required w/ no deliverable to end user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants