Support for Yarn Berry #194

thitch97 · 2022-05-03T02:56:39Z

Readable

Checklist

I have viewed, signed, and submitted the Contributor License Agreement.
I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
I have added an integration test, if necessary.
I have reviewed the styleguide for guidance on my code quality.
I'm happy with the commit history on this PR (I have rebased/squashed as needed).

text/nodejs/0014-yarn-berry.md

… Add unresolved questions

thitch97 · 2022-05-11T15:42:45Z

text/nodejs/0014-yarn-berry.md

+## Unresolved Questions and Bikeshedding
+
+
+- Should this buildpack require `yarn`?


Would love to hear any thoughts on this item (and the related one below). Configuring corepack to correctly enable yarn in the offline case would require the developer to corepack prepare their yarn release so that the buildpack can corepack hydrate it. Does that seem reasonable? Foregoing the yarn requirement is starting to feel more idealistic than it is practical.

An argument against introducing another buildpack here is that it would increase complexity in other buildpacks that use yarn like the Web Servers buildpack.

ryanmoran · 2022-05-11T18:58:01Z

text/nodejs/0014-yarn-berry.md

+
+- If there is no `.pnp.cjs` in the working directory, even if a local cache exists.
+
+- If the `yarn.lock` has changed.


From what? Is this from a checksum recorded in a previous build?

Yes, the yarn-install buildpack generates a checksum of yarn.lock and the output of yarn config --list. The plan is to do the same, replacing yarn config with yarn info.

Can you clarify what happens if there is a .pnp.cjs file and a local cache and the yarn.lock file has changed?

I've reworked this section a bit, let me know if that makes sense.

ryanmoran · 2022-05-31T17:21:10Z

text/nodejs/0014-yarn-berry.md

+
+`yarn-berry-install`
+
+Provides: `yarn_packages` OR `node_modules` Requires: `node`, during `build` and `launch`


Why does this require anything at launch? At launch, this buildpack isn't really doing anything other than making sure that there are some JavaScript files available on the filesystem. I'd expect that if someone wanted to run these files, they'd need to require node some other way, no?

You are correct, I've updated accordingly.

ryanmoran · 2022-05-31T17:24:52Z

text/nodejs/0014-yarn-berry.md

+
+- If there is no `.pnp.cjs` in the working directory, even if a local cache exists.
+
+- If the `yarn.lock` has changed.


Can you clarify what happens if there is a .pnp.cjs file and a local cache and the yarn.lock file has changed?

aurelienbottazini · 2022-06-09T16:57:48Z

text/nodejs/0014-yarn-berry.md

+fail if the `yarn.lock` file needs to be updated. As such, developers must run
+`yarn install` locally in order to update their app's `yarn.lock` file.
+`--frozen-lockfile` has been deprecated in favour of `--immutable`, which this
+buildpack will utilize.


I am bit concerned by this.
I agree --immutable is a good idea in theory.
But I had strange cases where for an identical node and yarn version, yarn install --immutable would fail on some env and succeed on others. Very frustrating and hard to debug.

Should we instead not specify the option by default and let users either

add enableImmutableInstalls: true to .yarnrc.yml

or set with yarn env vars mechanism: YARN_ENABLE_IMMUTABLE_INSTALLS

This is helpful feedback, thank you. I'm okay with making this configurable.

ryanmoran · 2022-06-10T15:05:32Z

text/nodejs/0014-yarn-berry.md

+- Include logic for Yarn Berry workflows in the existing `yarn-install`
+  buildpack.
+
+  Berry includes breaking changes to the `yarn` CLI used by the buildpack to
+  install dependencies. The changes to CLI options are such that at the very
+  least, the buildpack would need to decide which ones to use based on the type
+  of application being built. Trying to incorporate both workflows will
+  increase the scope of responsibility of the buildpack, likely increasing its
+  complexity at the expense of maintainability. Classic Yarn is now in
+  maintenance mode and, though it is still heavily used now (due to the
+  relatively slow adoption of Berry), its unclear how long it will be
+  supported. For this reason, it is even more prudent to keep the logic
+  separate to facilitate the eventual transition.


I think it might be reasonable to still get Berry support in the existing buildpack if we just take some effort to keep the "Classic" and Berry parts separate. If we did something like move all the existing code in the buildpack into a classic package and then created a new berry package we can keep them logically separated. We could then just have a small amount of code at the top-level that decides with codepath to invoke.

I hadn't considered separate packages, that's an interesting suggestion. It would remove a lot of overhead in the event that we drop support for Classic altogether.

ekcasey · 2022-06-14T17:56:47Z

text/nodejs/0014-yarn-berry.md

+    id = "paketo-buildpacks/yarn-berry-install"
+
+  [[order.group]]
+    id = "paketo-buildpacks/node-module-bom"


Are any updates to this buildpack required to generate the SBOM in cases where there is a yarn cache instead of node_modules?

Good question. I think the inclusion of node-module-bom here is a mistake on my part. The buildpack shouldn't need to leverage that buildpack at all for generating the SBoM and would instead include logic for SBoM generation akin to what exists in yarn-install currently.

ekcasey · 2022-06-14T18:02:26Z

text/nodejs/0014-yarn-berry.md

+All `node` commands run via the shell should be run with `yarn node` instead. The default
+start command in the Yarn Start buildpack may need to be modified.


Can we elaborate on any changes needed to the yarn-start or node-start (included in the berry group above) buildpacks? Seems like there could be some gotchas here with detection criteria and cache vs modules configurations.

ekcasey · 2022-06-14T18:04:34Z

text/nodejs/0014-yarn-berry.md

+  [[order.group]]
+    id = "paketo-buildpacks/yarn-berry-install"


Should this be optional and opt out if users are going with the zero-installs method? This might mess up detection for the group without some changes in (yarn start), but it could be confusing to see yarn install opt-in when you are explicitly striving for zero-installs?

This order group may now be obsolete as I was able to prove out a sane way to keep the build logic for both Yarn classic and Berry in a single buildpack (per @ryanmoran's suggestion). As for the UX, I figured we could indicate through the logs whenever the buildpack has chosen to forgo running yarn install based on adherence to the zero-installs setup.

divoxx · 2022-07-08T19:16:10Z

Is this RFC still actively being worked on? We have interest in helping push this forward as well, if help is needed.

ekcasey · 2022-07-12T17:08:53Z

@thitch97 IIRC you mentioned during WG you were making some largish changes to the RFC. Do you mind posting a comment describing where it is at and what your intentions are? If it needs more work before it is ready for further discussion/reviews maybe we should put it back into draft temporarily?

thitch97 · 2022-07-12T18:23:04Z

@divoxx @ekcasey My intention at the moment is to shift to the model initially enumerated in the "Rationale and Alternatives" section. It turns out that hosting both Classic and Berry codepaths in one buildpack isn't as heavy an engineering/maintenance burden as it seemed (and in fact removes some overhead). As such, many of the technical decisions here, especially the order groupings, are now obsolete.

divoxx · 2022-07-12T21:23:44Z

Sounds good and thanks for the update @thitch97 & @ekcasey

We at @doximity are very interested in Yarn Berry support as well, so feel free to reach out to me in case any help is needed.

fg-j · 2022-07-19T18:18:13Z

From WG 7/19/22:

In latest update of this RFC, the Build Plan dependency node_modules has been renamed node_packages. This simplifies plan resolution.
In latest update, Yarn Berry builds still require yarn at build, instead of using corepack. This presents some issues that are hashed out in the Alternatives and Unresolved Questions sections.
Concern: Yarn Berry doesn't seem to play well with offline builds.
- Comment: "offline" builds are rare; builds that only have access to a private network are more common
- Comment: there still is a fully offline workflow for yarn 1
- Q: Could we enable users to volume-mount a package cache for offline builds? The maven buildpack currently supports this?
  - This sounds tricky but possible. In yarn context, your global cache on your workstation might contain dependencies that don't have anything to do with the app in question. Also, the location of the package cache isn't configurable for yarn.
Requested change: Add a list of buildpacks that provide or require the node_modules plan entry so it's easy to tell which buildpacks would need to change

Seeking @paketo-buildpacks/nodejs-maintainers feedback on these changes.

ryanmoran · 2022-07-21T14:53:36Z

text/nodejs/0014-yarn-berry.md

+Provides: `node_pkgs`
+Requires: `node`, `yarn` during `build`
+
+The Node.js buildpacks use `node_modules` as a keyword for providing/requiring
+packages in the build plan across the board. Since Berry explicitly excludes
+`node_modules`, however, it seems reasonable to instead use a generic term for
+node packages such as `node_pkgs`.
+
+The buildpacks affected by this change to the `node_modules` build plan dependency are:
+
+- `npm-install`
+- `yarn-install`
+- `npm-start`
+- `yarn-start`
+- `node-start`
+- `node-run-script`
+- `node-module-bom`
+- `rails-assets`


Can you provide some more of the rational behind this choice? I'm not seeing how we don't end up in a state where its either confusing to Berry users or confusing to non-Berry users about what is being provided here.

@ryanmoran The rationale was that node_modules is just not an artifact that is produced in the Berry PnP case. I thought that providing node_modules would be confusing to users who've opted into PnP, and that referring to them both as a set of Node.js "packages" was a reasonable compromise. It also seemed neater than using the Or field to provide either node_modules or something like yarn_cache.

To me, it'd be more intuitive if the yarn classic and yarn berry paths resulted in two slightly different build plans, since they do involve dealing with different sets of artifacts.

I'd be in favor of the yarn-install buildpack providing node_modules if it plans on running yarn install and yarn_cache if it doesn't. Then the start command buildpacks could require one Or the other.

joshuatcasey · 2022-07-26T18:06:59Z

2022-07-26 Working Group notes:

Use different build plans for yarn classic and yarn berry (#194 (comment))

text/nodejs/0014-yarn-berry.md

dmikusa · 2022-08-02T18:41:08Z

2022-08-02 WG Notes:

Approved by Node maintainers. Final call for any comments. Scheduled to be merged soon.

thitch97 added 3 commits May 2, 2022 22:53

first draft

c3ff877

Fix errors

393f1b6

Answer unresolved questions

eb6d921

thitch97 marked this pull request as ready for review May 10, 2022 18:01

thitch97 requested a review from a team as a code owner May 10, 2022 18:01

thitch97 requested review from ryanmoran and paketo-bot-reviewer May 10, 2022 18:01

thitch97 mentioned this pull request May 10, 2022

Support for Yarn 2 apps paketo-buildpacks/yarn#19

Closed

robdimsdale reviewed May 11, 2022

View reviewed changes

text/nodejs/0014-yarn-berry.md Outdated Show resolved Hide resolved

robdimsdale reviewed May 11, 2022

View reviewed changes

text/nodejs/0014-yarn-berry.md Show resolved Hide resolved

robdimsdale reviewed May 11, 2022

View reviewed changes

text/nodejs/0014-yarn-berry.md Outdated Show resolved Hide resolved

thitch97 added 2 commits May 11, 2022 00:13

Update provides/requires; Add more specific cases for 'yarn install';…

2c46abf

… Add unresolved questions

Add note about SBoM generation

5f2611f

thitch97 commented May 11, 2022

View reviewed changes

ryanmoran reviewed May 11, 2022

View reviewed changes

ryanmoran suggested changes May 31, 2022

View reviewed changes

Clarify zero-install strategy

f229d75

ryanmoran mentioned this pull request Jun 8, 2022

Paketo Yarn Install Buildpack 0.9.0 fails to build paketo-buildpacks/yarn-install#345

Closed

3 tasks

aurelienbottazini reviewed Jun 9, 2022

View reviewed changes

ryanmoran reviewed Jun 10, 2022

View reviewed changes

ekcasey reviewed Jun 14, 2022

View reviewed changes

thitch97 marked this pull request as draft July 12, 2022 18:11

Add Berry workflows to existing yarn-install buildpack

240950e

thitch97 marked this pull request as ready for review July 19, 2022 15:42

Add list of buildpacks which use node_modules dependency

73cffc3

ryanmoran reviewed Jul 21, 2022

View reviewed changes

Provide node_modules OR yarn_pkgs

2a8ee51

ryanmoran reviewed Aug 1, 2022

View reviewed changes

text/nodejs/0014-yarn-berry.md Outdated Show resolved Hide resolved

Detection criteria correction

a93dd0d

ryanmoran approved these changes Aug 2, 2022

View reviewed changes

fg-j self-requested a review August 2, 2022 18:26

ryanmoran merged commit a6d500e into main Aug 3, 2022

ryanmoran deleted the yarn2 branch August 3, 2022 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Yarn Berry #194

Support for Yarn Berry #194

thitch97 commented May 3, 2022 •

edited

Loading

thitch97 May 11, 2022 •

edited by ryanmoran

Loading

ryanmoran May 11, 2022

thitch97 May 11, 2022

ryanmoran May 31, 2022

thitch97 Jun 8, 2022

ryanmoran May 31, 2022

thitch97 Jun 8, 2022

ryanmoran May 31, 2022

aurelienbottazini Jun 9, 2022 •

edited

Loading

thitch97 Jun 10, 2022

ryanmoran Jun 10, 2022

thitch97 Jun 10, 2022

ekcasey Jun 14, 2022

thitch97 Jun 21, 2022

ekcasey Jun 14, 2022

ekcasey Jun 14, 2022

thitch97 Jul 5, 2022

divoxx commented Jul 8, 2022

ekcasey commented Jul 12, 2022

thitch97 commented Jul 12, 2022 •

edited

Loading

divoxx commented Jul 12, 2022

fg-j commented Jul 19, 2022

ryanmoran Jul 21, 2022

thitch97 Jul 21, 2022 •

edited

Loading

fg-j Jul 22, 2022

joshuatcasey commented Jul 26, 2022 •

edited

Loading

dmikusa commented Aug 2, 2022

		## Unresolved Questions and Bikeshedding


		- Should this buildpack require `yarn`?


		- If there is no `.pnp.cjs` in the working directory, even if a local cache exists.

		- If the `yarn.lock` has changed.


		`yarn-berry-install`

		Provides: `yarn_packages` OR `node_modules` Requires: `node`, during `build` and `launch`

		All `node` commands run via the shell should be run with `yarn node` instead. The default
		start command in the Yarn Start buildpack may need to be modified.

Support for Yarn Berry #194

Support for Yarn Berry #194

Conversation

thitch97 commented May 3, 2022 • edited Loading

Checklist

thitch97 May 11, 2022 • edited by ryanmoran Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aurelienbottazini Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divoxx commented Jul 8, 2022

ekcasey commented Jul 12, 2022

thitch97 commented Jul 12, 2022 • edited Loading

divoxx commented Jul 12, 2022

fg-j commented Jul 19, 2022

Choose a reason for hiding this comment

thitch97 Jul 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuatcasey commented Jul 26, 2022 • edited Loading

dmikusa commented Aug 2, 2022

thitch97 commented May 3, 2022 •

edited

Loading

thitch97 May 11, 2022 •

edited by ryanmoran

Loading

aurelienbottazini Jun 9, 2022 •

edited

Loading

thitch97 commented Jul 12, 2022 •

edited

Loading

thitch97 Jul 21, 2022 •

edited

Loading

joshuatcasey commented Jul 26, 2022 •

edited

Loading