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

RFC: Improving lerna publish #1767

Open
ThisIsMissEm opened this issue Nov 7, 2018 · 10 comments
Open

RFC: Improving lerna publish #1767

ThisIsMissEm opened this issue Nov 7, 2018 · 10 comments

Comments

@ThisIsMissEm
Copy link
Contributor

Proposal for the future of lerna publish:

  • deprecate automatic versioning, use lerna version first instead
    • this enables a more reliable & retryable lerna publish and removes functionality that’s duplicated between lerna version and lerna publish
  • switch to libnpmpublish instead of shelling out to npm/yarn
    • This allows us to avoid a lot of the weird issues that can happen due to things like $PATH, different versions of npm/yarn, and the different capabilities of each.
  • separate tagging as “latest” (dist-tag) from publishing the tarball, as to make “lerna publish” not give partial “latest” releases
    • Essentially whilst a publish is ongoing, installs should not fail
    • This also moves us towards a more “release” based model, which seems to be what people intend (looking at apollo/babel/etc)
  • implement retries via checking the registry for each package at a given version, giving the option to abort the publish or continue
    • this allows for partial publishes to succeed, related to splitting versioning and publishing
  • switch to serial/sequential/queue-based publishing of packages
    • This gives better error handling and OTP support
    • Concurrent publishes lead to a lot of error conditions resulting in complex code & hard to debug errors
  • remove execScripts
    • this functionality is only used by a small subset of lerna users, and is probably better done via a custom “publish” command in your package.json at root level. Or via a custom wrapler around lerna

These are my initial thoughts, and I’m definitely interested to keep discussing these to inform development decisions.

The above list is based on some private conversation with various people, and opens up doors for future developments or improvements whilst greatly simplifying the codebase.

@mjhenkes
Copy link

mjhenkes commented Nov 9, 2018

+1 to separating versioning from publishing.

In our lerna workflow many developers work on the various packages but very few have privileges to publish. This creates a situation where the publisher needs to know at publish time how to version a component. The developer pushing changes should know how their changes affect the version and ideally would be able to indicate that prior to publish time.

Secondly separating versioning from publishing would allow us to create an automated job to publish the packages. No versions choices would need to be made at publish time.

@evocateur
Copy link
Member

@mjhenkes It's important to note that versioning (lerna version) is separate from publishing (lerna publish) today, it's just that lerna publish isn't quite quick enough on the uptake to realize this without a special from-git positional argument. You can pretty much call lerna publish from-git unconditionally in CI, as it will just no-op if the current revision lacks an annotated tag (or tags) created by lerna version.

@evocateur
Copy link
Member

deprecate automatic versioning, use lerna version first instead

lerna version is already implicitly used by lerna publish, though the context and decisions around that are certainly up for debate. There shouldn't be a lot of duplication, though there is a lot of overlap (and doubtless things I missed when teasing the commands apart).

separate tagging as “latest” (dist-tag) from publishing the tarball

This was actually the default in early lerna 2.x, it would first publish to a lerna-temp dist-tag and then after all packages were published would loop through and move lerna-temp to latest. This pattern can be enabled in lerna 3 with the --temp-tag option.

Should it be restored as the default? I'm not against it, I suppose. We still need to do better "disaster recovery," in any case. Temporary dist-tags aren't going to solve the entire problem (not that you were implying that).

remove execScripts

Big 👍

@ThisIsMissEm
Copy link
Contributor Author

ThisIsMissEm commented Nov 10, 2018

On lerna version vs lerna publish: I think the code just confused me there, on my first read through I thought it was actually versioning packages.

it's just that lerna publish isn't quite quick enough on the uptake to realize this without a special from-git positional argument.

I'd say from-git should probably be the only path through lerna publish, as this makes the command not mutate or change versions of packages (which seems to be what happens in the other code paths [see makeVersion]).

I'd also say that there should not be an implicit (or any other) usage of lerna version from lerna publish. Currently, it seems people just run lerna publish unaware that it's actually doing two commands in one. By having this behaviour, we make it much harder to do things like publish retries or recoveries.

separate tagging as “latest” (dist-tag) from publishing the tarball

This was actually the default in early lerna 2.x,

Is there a reason it was moved to be non-default for lerna 3.x?

@gziolo
Copy link

gziolo commented Nov 13, 2018

For the record, we dicussed publishing flow improvments today (November 13th) during weekly WordPress core JavaScript chat (you can also check chat transcript for more details). We also track the same issue in the Gutenberg repository: WordPress/gutenberg#11780. In this repository we maintain source code of all npm packages published under WordPress organization.

In our lerna workflow many developers work on the various packages but very few have privileges to publish. This creates a situation where the publisher needs to know at publish time how to version a component. The developer pushing changes should know how their changes affect the version and ideally would be able to indicate that prior to publish time.

Secondly separating versioning from publishing would allow us to create an automated job to publish the packages. No versions choices would need to be made at publish time.

At the moment we defer to Lerna performing version increments for all packages. We only express our intent for the change inside of the corresponding changelog files when working on PRs. The biggest downside of this approach is that we need to scan all existing changelog files during publishing and re-apply manually the proper version bump for each package. It would be a nice improvement if we could update both changelog and package.json with a new version at the same time when PR gets merged. Lerna could use that and skip the step to ask what version should be applied.

There is also this issue that some packages get updated only because they depend on other updated packages. This is where Lerna is the most helpful because it's almost impossible to detect it manually when you have to maintain 50 packages with independent versioning.

It might what from-git is offering, but to be honest, I don't quite understand how it works at the moment based on this issue and existing documentation: https://github.com/lerna/lerna/tree/master/commands/publish#bump-from-git.

One thing which didn't work well in the past is pushing publish commit. It takes some time to provide all version increments and publish all packages. We had the case where new commit was pushed to master which broke the flow two-fold. We had to rebase the branch before pushing changes which caused that git hash references weren't working properly anymore. Next time we asked Lerna to list which packages have changed, it had to use the previous hash which wasn't ideal.

@gziolo
Copy link

gziolo commented Nov 13, 2018

  • implement retries via checking the registry for each package at a given version, giving the option to abort the publish or continue
    • this allows for partial publishes to succeed, related to splitting versioning and publishing
  • switch to serial/sequential/queue-based publishing of packages
    • This gives better error handling and OTP support
    • Concurrent publishes lead to a lot of error conditions resulting in complex code & hard to debug errors

In the past, we had publish failures caused by 2FA timeout where we had more packages to publish than it could be handled in 30 seconds. The following steps don't quite work well in the majority of cases to recover from such failure:

  1. Check if tags were created locally, if yes use git push --tags to send them to upstream (ensure you didn’t create your own tags).
  2. Check if publish commit was created, if yes use git push to send it to upstream (ensure your history is in the proper state).
  3. Identify all packages that were published and verify there are no local paths for local packages inside package.json files (e.x. "@wordpress/compose": "^1.0.0").
  4. cd to each package that needs to be published and run npm publish --otp=******.

It's tons of manual work and we had 2 releases where we published local paths inside package.json files instead of the latest version of dependencies. Example package which was published by accident with the local path instead of npm version:
https://unpkg.com/@wordpress/block-library@2.2.0/package.json

"@wordpress/autop": "file:../autop",
"@wordpress/blob": "file:../blob",
"@wordpress/blocks": "file:../blocks",
"@wordpress/components": "file:../components",
"@wordpress/compose": "file:../compose",
...

I don't have enough technical knowledge to propose a proper solution inside Lerna. However, I'm happy to discuss all possible improvements in the flow on a higher level. If we agree on the future steps and will have a list of tasks to work on, we will try to help find volunteers to implement them. We plan to discuss publishing packages again next week. For us, the most pressing part is having a reliable way to publish to npm using 2FA enabled for publishing.

@ntwb
Copy link

ntwb commented Nov 13, 2018

What I'm wondering is, and will test later, is if the following workflow is a valid way of publishing packages:

  1. Run lerna updated to check what packages require publishing.
  2. Update each of the above packages changelog.md with the version to be published.
  3. Run lerna version to update each packages version for publishing.
  4. Run lerna publish from-git

The above workflow would remove all the manual tasks out of the publish workflow, it would be no guarantee that lerna publish from-git would complete within 30 seconds for npm's OTP, but I'm also thinking hoping that even if publishing failed part way through at this point it would be easier to recover from.

@milesj
Copy link
Contributor

milesj commented Dec 4, 2018

I'm really in favor of the version/publish split, as it would help automate releases a bit. In some of the current projects at my employer, we're utilizing Yarn workspaces + semantic-release + semantic-release-monorepo (which is very hacky) to automate releases based on commit messages... which works, but is also problematic.

If this could be automated via Lerna, we could forego the semantic-release process and embrace Lerna 100%. Even more so if there were some hooks to update changelog, generate GH releases, etc.

@ThisIsMissEm
Copy link
Contributor Author

ThisIsMissEm commented Dec 4, 2018 via email

evocateur added a commit that referenced this issue Dec 7, 2018
Incremental progress toward #1767

* libnpmaccess -> libnpm/access
* npm-lifecycle -> libnpm/run-script
* npm-package-arg -> libnpm/parse-arg
* npmlog -> libnpm/log
* pacote.manifest -> libnpm/manifest
* pacote.packument -> libnpm/packument
* manually hoist find-up@3
nicolo-ribaudo pushed a commit to babel/lerna that referenced this issue Dec 18, 2018
Incremental progress toward lerna#1767

* libnpmaccess -> libnpm/access
* npm-lifecycle -> libnpm/run-script
* npm-package-arg -> libnpm/parse-arg
* npmlog -> libnpm/log
* pacote.manifest -> libnpm/manifest
* pacote.packument -> libnpm/packument
* manually hoist find-up@3
@evocateur
Copy link
Member

switch to libnpmpublish instead of shelling out to npm/yarn

This is done as of v3.7.0 (v3.7.1 improved the logging a bit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants