feat: NPM registry package for `garden` #5087

worldofgeese · 2023-09-13T14:19:54Z

What this PR does / why we need it:

This adds an NPM registry package to be published by our GitHub Actions workflow for garden.

Which issue(s) this PR fixes:

Unblocks #4935.

Special notes for your reviewer:

There's a chicken and egg problem here you may run into when testing the GitHub Actions publish workflow, which is that the top-level yarn && yarn build expects these packages to exist but only core is published. NPM can also derive these packages locally so it may be a non-issue but it's something to watch out for.

I have tested a version of this (and you can too) by publishing the CLI package in my own NPM registry namespace. Run npx @worldofgeese/cli. It has been my daily driver for a week now. The difference between this and the @garden-io/cli is the package.json file. You'll notice in my test package I've stripped out a lot of @garden-io/cli's dependencies but in this PR I've modified @garden-io/cli directly to offer a more elegant solution. We need to test that these modifications work as well as they did in my own test package.

Bumps google/cloud-sdk from `183f189` to `8663c06`. --- updated-dependencies: - dependency-name: google/cloud-sdk dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Consumers of our NPM packages won't need TS files, which are only required in the build step.

`npm publish` has no facility for publishing multiple packages at once. `lerna` gives us this capability and enables uniform version bumps across packages.

The `garden` CLI package depends on the majority of our other packages. For NPM publishing, we need to set these to non-private.

The `garden` CLI requires a `static` dir to be in proximate location to its run path. It also requires some Node options set for performance reasons. We can do both of these within the `garden` CLI shell script. By hiding these options behind an environment flag, the Pkg `garden` we currently vend is unaffected.

cli/bin/garden

Co-authored-by: Steffen Neubauer <[email protected]>

This should solve for our `Error: Patch file found for package pkg which is not present at node_modules/pkg` build error.

TimBeyer · 2023-09-14T13:54:49Z

.github/workflows/publish-release.yml

      - name: Publish stable version if normal release
        if: github.event.release.prerelease != true
        working-directory: ./core
        run: |
-          npm publish
+          lerna publish from-package --yes


Could we add some documentation how exactly the process works that increments the package version to the correct version number?

@TimBeyer sure! In a code comment?

I think you could leave a few short comments in here and then more detailed in https://github.com/garden-io/garden/blob/main/RELEASE_PROCESS.md

TimBeyer · 2023-09-14T13:56:00Z

lerna.json

@@ -0,0 +1,16 @@
+{
+    "$schema": "node_modules/lerna/schemas/lerna-schema.json",
+    "version": "0.13.13",


Will this version get updated correctly with every release publishing step?

Yes! lerna can automatically bump versions according to one of a number of strategies.

What I mean is, does this number here get updated and committed back or is this just a placeholder of sorts?

worldofgeese · 2023-09-14T13:57:12Z

In the build logs, we're getting build failures due to patch-package acting on a missing pkg in node_modules. This wasn't an issue in my test package because I created a slim version of the CLI that didn't include pkg. I don't know how to resolve this. I've tried to remove nohoist but that didn't solve it.

error /home/circleci/project/node_modules/@garden-io/cli: Command failed.
Exit code: 1
Command: patch-package
Arguments: 
Directory: /home/circleci/project/node_modules/@garden-io/cli
Output:
patch-package 6.5.1
Applying patches...
Error: Patch file found for package pkg which is not present at node_modules/pkg
---
patch-package finished with 1 error(s).
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Exited with code exit status 1

Any suggestions @TimBeyer or @stefreak?

stefreak · 2023-09-14T13:29:13Z

cli/bin/garden

+const gitDir = path.join(process.env.STATIC_DIR, '.git');
+
+if (!fs.existsSync(gitDir)) {
+  exec(`git init ${process.env.STATIC_DIR}`, (error, stdout, stderr) => {


I think this is a little early in the code to call git init; Also if we generally call git init on the static directory if it isn't a git repository yet (until we fixed #4477) we also fix a windows issue (#5030 and #2614)

@stefreak, where would you move this call?

I think we should finalize and merge #5120 before merging this, so we don't have to call it at all.

TimBeyer · 2023-09-14T14:13:04Z

In the build logs, we're getting build failures due to patch-package acting on a missing pkg in node_modules. This wasn't an issue in my test package because I created a slim version of the CLI that didn't include pkg. I don't know how to resolve this. I've tried to remove nohoist but that didn't solve it.
error /home/circleci/project/node_modules/@garden-io/cli: Command failed.
Exit code: 1
Command: patch-package
Arguments: 
Directory: /home/circleci/project/node_modules/@garden-io/cli
Output:
patch-package 6.5.1
Applying patches...
Error: Patch file found for package pkg which is not present at node_modules/pkg
---
patch-package finished with 1 error(s).
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Exited with code exit status 1
Any suggestions @TimBeyer or @stefreak?

I'm wondering if we need to restructure things a tiny bit.
Currently cli both does the bundling and docs generation, and is the actual CLI code that runs.
Probably we should move the release parts out into a private package and only call that in CI so that we don't really need to install a big tool like pkg just to run garden via npm / npx.

Alternatively we could maybe move pkg and patch-package into dev dependencies since they are really only used in CI for creating the final release. We also have the postinstall script which needs to be changed. On npm I would use prepare instead, which doesn't run when you install the package from npm but runs when you install the dependencies locally, publish the package, or when installed from a git source. However I believe that yarn treats it slightly differently so we'd have to look into that.

stefreak · 2023-09-14T16:08:45Z

A big problem with this approach is that dependencies will not be pinned (npm install @garden/cli will ignore the yarn.lock file we have in place).

If we'd be using NPM, the solution would be to switch from using package-lock.json to using the npm-shrinkwrap.json.

But Yarn does not support npm-shrinkwrap.json – and we can't switch to NPM, because the PKG-built binary does not work anymore correctly with the node_modules installed by NPM.

Using both yarn.lock and npm-shrinkwrap.json we'd need to solve the problem of keeping both files in sync, or otherwise our unit, integration and e2e tests might not be covering exactly the dependencies that end up in Garden that ships through NPM / NPX.

worldofgeese · 2023-09-26T15:27:33Z

Hey @stefreak and @TimBeyer thank you both for your feedback and great work in making this PR possible! I will continue working on this. I just synced with Tim and collected the steps still left before this is merge-ready. Can you both look this over and let me know if there's anything missing? Thank you!

separate out the CLI package from the Pkg package with a new builder folder and recipe
add a prepublishOnly life cycle script to npm shrinkwrap the CLI package
wait on Vova's PR to land that removes static
remove static dir git init step
test how lerna bumps package versions on my own branch: does it commit back?
rebase on tip of main

dependabot bot and others added 7 commits September 8, 2023 10:54

chore: ignore .ts and static dir

dc3ba15

Consumers of our NPM packages won't need TS files, which are only required in the build step.

feat: use lerna to publish packages to NPM

1a5267e

`npm publish` has no facility for publishing multiple packages at once. `lerna` gives us this capability and enables uniform version bumps across packages.

chore: don't use private packages

87999b0

The `garden` CLI package depends on the majority of our other packages. For NPM publishing, we need to set these to non-private.

fix: remove traling semicolon

d70e5b2

Merge branch 'main' into garden-npm-publish

a038ed0

worldofgeese requested review from stefreak, twelvemo and shumailxyz September 14, 2023 09:01

worldofgeese added 2 commits September 14, 2023 11:22

Merge branch 'main' into garden-npm-publish

b6a2196

fix: ensure static dir is collected

e111e12

stefreak reviewed Sep 14, 2023

View reviewed changes

cli/bin/garden Outdated Show resolved Hide resolved

worldofgeese and others added 2 commits September 14, 2023 15:24

NODE_OPTIONS to later execution

2e899ba

Co-authored-by: Steffen Neubauer <[email protected]>

Allow hoisting pkg

c53806a

This should solve for our `Error: Patch file found for package pkg which is not present at node_modules/pkg` build error.

TimBeyer reviewed Sep 14, 2023

View reviewed changes

stefreak reviewed Sep 14, 2023

View reviewed changes

kevinmichaelchen mentioned this pull request Nov 6, 2023

+garden -- Develop and test complex systems faster. pkgxdev/pantry#3951

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: NPM registry package for `garden` #5087

feat: NPM registry package for `garden` #5087

worldofgeese commented Sep 13, 2023 •

edited

Loading

TimBeyer Sep 14, 2023

worldofgeese Sep 14, 2023

TimBeyer Sep 14, 2023

TimBeyer Sep 14, 2023

worldofgeese Sep 14, 2023

TimBeyer Sep 14, 2023

worldofgeese commented Sep 14, 2023 •

edited

Loading

stefreak Sep 14, 2023

worldofgeese Sep 25, 2023

stefreak Sep 25, 2023

TimBeyer commented Sep 14, 2023 •

edited

Loading

stefreak commented Sep 14, 2023

worldofgeese commented Sep 26, 2023 •

edited by TimBeyer

Loading

feat: NPM registry package for garden #5087

Are you sure you want to change the base?

feat: NPM registry package for garden #5087

Conversation

worldofgeese commented Sep 13, 2023 • 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

worldofgeese commented Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimBeyer commented Sep 14, 2023 • edited Loading

stefreak commented Sep 14, 2023

worldofgeese commented Sep 26, 2023 • edited by TimBeyer Loading

feat: NPM registry package for `garden` #5087

feat: NPM registry package for `garden` #5087

worldofgeese commented Sep 13, 2023 •

edited

Loading

worldofgeese commented Sep 14, 2023 •

edited

Loading

TimBeyer commented Sep 14, 2023 •

edited

Loading

worldofgeese commented Sep 26, 2023 •

edited by TimBeyer

Loading