errose28 commented on PR #6501:
URL: https://github.com/apache/ozone/pull/6501#issuecomment-2048906771
In the time since #5538 I've had to learn how pnpm works more or less for
the new Ozone website, so I'm pretty sure I can explain what happened here.
This is still not an area I have much experience in so please double check for
yourself.
Prior to #5538, we would see dependabot PRs failing in the build step with
this error:
```
[INFO] --- frontend-maven-plugin:1.12.0:npx (install frontend
dependencies) @ ozone-recon ---
[INFO] npm not inheriting proxy config from Maven
[INFO] Running 'npx [email protected] install --frozen-lockfile' in
/home/runner/work/ozone/ozone/hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web
[INFO]  ERR_PNPM_LOCKFILE_CONFIG_MISMATCH  Cannot proceed with the
frozen installation. The current "settings.autoInstallPeers" configuration
doesn't match the value found in the lockfile
[INFO]
[INFO] Update your lockfile using "pnpm install --no-frozen-lockfile"
```
In the version of the code where we would see this error, [we did not
explicitly define a pnpm version in
package.json](https://github.com/apache/ozone/blob/8e52a3a6f2c9819163e602765fd4dbe9501a0f0f/hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/package.json).
Our pnpm version was still pinned for stability, but it is pinned in
[pom.xml](https://github.com/apache/ozone/blob/10a1dbfecefc87821bb497dd7f6595b7556253e7/hadoop-ozone/recon/pom.xml#L27)
since #2115 because we have a somewhat atypical build where Maven, our primary
build system, then invokes pnpm as a secondary build sytem.
Dependabot supports both pnpm v7 and v8. If the project does explicitly
define a version in package.json, it will default to v8, the latest. Our
atypical build structure did not pin the pnpm version in the typical place in
package.json, so dependabot generated the lockfile with pnpm v8. Then our build
job would come in after and run on this lockfile with pnpm v7 pinned in pom.xml.
The error we were seeing is due to a difference in pnpm v7 and v8. In v8,
`autoInstallPeers` defaults to `true`. This would cause dependabot to [add
these lines to the top of the pnpm-lock.yaml file it
generated](https://patch-diff.githubusercontent.com/raw/apache/ozone/pull/5143.diff):
```
settings:
autoInstallPeers: true
excludeLinksFromLockfile: false
```
However, in v7, `autoInstallPeers` defaults to `false`. The build runs with
`--frozen-lockfile` as shown above, so when this config in the lockfile
generated by dependabot (`true`) does not match the one pnpm v7 is running with
(`false`), it fails.
The solution is just to pin pnpm to a v7 version in package.json where
dependabot will see it. This way dependabot generates the lockfile with the
same version we will use to build it in the job. So #5538 contained the fix,
but it was not the extra dependabot workflow, it was [this single
line](https://github.com/apache/ozone/pull/5538/files#diff-2af6209cdc38f5254d57c8f7905679dc885aa17ee8f32b59bea72c3d17e5378fR2).
I was kind of on to this with [this
comment](https://github.com/apache/ozone/pull/5538#pullrequestreview-1711244238),
but at the time I didn't have enough pnpm knowledge to make the case that the
extra workflow was unnecessary, other than that it seemed lots of other people
were using pnpm and dependabot without needing it so there must be a simpler
fix. For posterity I will answer my own questions now:
> * Is there a way to configure dependabot to work correctly without
needing a custom action? I don't totally understand the current error before
this PR or if there is a config to fix this:
I explained the error above, and yes there is a one line config to fix this
problem.
> Maybe something like these could fix it?
> https://github.com/pnpm/pnpm/issues/6649
> <https://github.com/orgs/pnpm/discussions/6633>
Yes both of those threads mention pinning your pnpm version so that
dependabot uses the same version as the repo's build, which will fix the
problem.
All of that to say that code wise this change LGTM since we are now pinning
the pnpm version in package.json and the dependabot workflow isn't the actual
fix. Based on the PR description I think the reason this PR works is different
than we initially thought. There is also the caveat that we have pnpm version
pinned in package.json and pom.xml and have to manually keep them in sync.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
