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

Release candidate version logic is fragile #350

Closed
shawnbot opened this issue Sep 21, 2017 · 6 comments · Fixed by #353
Closed

Release candidate version logic is fragile #350

shawnbot opened this issue Sep 21, 2017 · 6 comments · Fixed by #353
Assignees

Comments

@shawnbot
Copy link
Contributor

The v9.4.0 PR (#329) push build failed because since the previous 9.4.0 release candidate was published, Travis also built the v9.3.0 tag that I pushed when I was back-filling the releases. The logic in script/release-candidate basically goes: "grab the latest version published to the rc dist-tag, and if its major.minor.patch matches the one we're publishing, increment it; otherwise, start at .rc-0." So in this edge case, it was comparing 9.4.0 against 9.3.0 and trying to publish 9.4.0-rc.0, which was already published.

I added some guards against building tags in db7637e, but we should test to make sure that they work. For instance, we could manually tag b009972 as v9.4.0 and make sure that Travis doesn't attempt to build that release again before we publish the release on GitHub.

@shawnbot
Copy link
Contributor Author

shawnbot commented Sep 22, 2017

Okay. After I tagged v9.4.0, Travis ran this build, which didn't deploy/publish. I think this is fixed, but would love @jonrohan's eyes on it just to be sure.

This was referenced Sep 22, 2017
@shawnbot
Copy link
Contributor Author

shawnbot commented Oct 4, 2017

Yeah, so this is still not right. The problem now is that we're bumping all of the package.json version fields, but not the corresponding dependency versions. For instance, when #353 published primer-css@9.5.0-rc.5, it published a reference to the versions that will be released (without the -rc.x prerelease suffix), such as primer-forms@1.4.0:

$ npm i primer-css@9.5.0-rc.5
npm ERR! code ETARGET
npm ERR! notarget No compatible version found: primer-forms@1.4.0
npm ERR! notarget Valid install targets:
npm ERR! notarget 0.2.0, 0.2.1, 0.2.2, 0.3.0, 0.4.0, 0.5.0, 0.6.0, 0.7.0, 0.7.1, 0.7.2, 0.7.3, 1.0.3, 1.0.4, 1.0.5, 1.0.6, 1.0.7-mono.0, 1.0.7-monotest.0, 1.0.7-monobeta.0, 1.0.7-themyscira.0, 1.0.8-0, 1.0.8-1, 1.0.8, 1.0.9, 1.0.10, 1.0.11, 1.0.12, 1.0.13-0, 1.0.13, 1.1.0-alpha.92656aa4, 1.1.0-alpha.c1ad9722, 1.1.0-alpha.9991349f, 1.1.0-alpha.a7f1f710, 1.1.0-alpha.0ec98f7c, 1.1.0-alpha.9c2cb073, 1.1.0-alpha.2da4646b, 1.1.0-alpha.d7875d5c, 1.1.0-alpha.c5efc988, 1.1.0-alpha.164ab439, 1.1.0-alpha.aca4e37a, 1.1.0-alpha.9d047e94, 1.1.0-alpha.35cfeeda, 1.1.0-alpha.46d97219, 1.1.0-alpha.36799d9b, 1.1.0-alpha.656d1ead, 1.1.0-alpha.a246e616, 1.1.0-alpha.6401d7cf, 1.1.0-alpha.7974d22b, 1.1.0-alpha.6dae25c2, 1.2.0-alpha.47779470, 1.0.13-1, 1.0.13-2, 1.0.13-3, 1.0.13-4, 1.2.0-alpha.39d50db9, 1.2.0-alpha.7c1f4bf1, 1.2.0-alpha.a1bd7859, 1.2.0-alpha.17c4cf97, 1.2.0-alpha.71aa3894, 1.2.0-alpha.d0281a6b, 1.2.0-alpha.72cfd878, 1.2.0-alpha.3d54e445, 1.1.0-rc.4, 1.1.0-rc.5, 1.1.0-rc.6, 1.1.0-rc.7, 1.1.0, 1.2.0-alpha.e97d9417, 1.2.0-alpha.376de7cb, 1.2.0-alpha.70168740, 1.2.0-alpha.410ddacb, 1.1.1-rc.8, 1.1.1-rc.9, 1.1.1-rc.10, 1.2.0-alpha.eb8a8768, 1.2.0-alpha.dc93c0dc, 1.2.0-alpha.18393b95, 1.2.0-alpha.dc1c6de7, 1.2.0-alpha.e353ed52, 1.1.1-rc.11, 1.1.1-rc.12, 1.2.0-alpha.4f0a1ee6, 1.1.1-rc.13, 1.1.1, 1.2.0-alpha.bcb1bad6, 1.2.0-alpha.e9cfc652, 1.2.0-alpha.95570ed3, 1.2.0-alpha.d6a171d3, 1.2.0-alpha.df13c7be, 1.2.0-alpha.d66d9e50, 1.2.0-alpha.5393754b, 1.2.0-alpha.5966046b, 1.2.0-alpha.3715510e, 1.2.0-alpha.0522c357, 1.2.0-alpha.e7831590, 1.2.0-alpha.0eb9fa6a, 1.1.1-rc.14, 1.1.1-rc.15, 1.2.0-rc.0, 1.2.0-rc.1, 1.2.0, 1.3.0-alpha.fe5e3a3d, 1.2.0-alpha.835d6aca, 1.2.0-alpha.1e61e099, 1.2.0-alpha.872c7b8c, 1.2.0-alpha.a90a48ad, 1.2.0-alpha.d4d52eda, 1.2.0-alpha.b11ddab3, 1.3.0-alpha.6cf70ab8, 1.3.0-alpha.cd9a484a, 1.3.0-alpha.5125486c, 1.3.0-alpha.34671876, 1.3.0-alpha.4607fa12, 1.3.0-alpha.0dd13c10, 1.3.0-alpha.a2faeb3f, 1.3.0-alpha.c350fb30, 1.3.0-alpha.ed908c17, 1.3.0-rc.0, 1.3.0-alpha.2c694a6f, 1.3.0-alpha.c1ca5b73, 1.3.0-alpha.1ea1c542, 1.3.0-alpha.63868222, 1.3.0-alpha.6767e15b, 1.3.0-alpha.bc699360, 1.3.0-alpha.86dc9d85, 1.3.0-alpha.0794a612, 1.3.0-alpha.07ea7d95, 1.3.0-alpha.2d0dc0a9, 1.3.0-rc.1, 1.4.0-alpha.725699da, 1.4.0-alpha.49431621, 1.3.0-rc.2, 1.3.0, 1.4.0-alpha.edbbf3c9, 1.4.0-alpha.b040c70a, 1.4.0-alpha.9e852ee7, 1.4.0-alpha.0201c56b, 1.4.0-alpha.6a78e694, 1.4.0-alpha.1cc69b58, 1.4.0-alpha.e857377f, 1.4.0-alpha.5e3fb7b7, 1.4.0-alpha.71e36148, 1.4.0-alpha.cba147d1, 1.4.0-alpha.24f38241, 1.4.0-alpha.7064500, 1.4.0-alpha.7f57fd19, 1.3.0-rc.3, 1.4.0-alpha.0444a988, 1.4.0-alpha.1dd7c0fc, 1.4.0-alpha.c3b708a8, 1.4.0-alpha.7acfa286, 1.4.0-alpha.deecf9fc, 1.4.0-alpha.8f695d94, 1.4.0-alpha.2e955279, 1.4.0-alpha.670dd75f, 1.4.0-alpha.3f6fbeb8, 1.4.0-alpha.d7669ef0, 1.3.0-rc.4, 1.4.0-alpha.662e34a7, 1.4.0-alpha.58e5748b, 1.4.0-alpha.196d0ee8, 1.4.0-alpha.db62cc5a, 1.4.0-alpha.7e6208ed, 1.4.0-alpha.620b6ab4, 1.4.0-alpha.10ecf0bd, 1.4.0-alpha.2b4fde00, 1.4.0-alpha.1eb7bceb, 1.4.0-alpha.68cefb21, 1.4.0-alpha.68a47d07, 1.4.0-alpha.00ff9fdf, 1.4.0-alpha.3805ebb6, 1.4.0-alpha.495b352f, 1.4.0-alpha.a0416d0c, 1.4.0-alpha.4e20437f, 1.4.0-alpha.6cff0edc, 1.4.0-alpha.74e40479, 1.4.0-alpha.cef502ae, 1.4.0-alpha.1f8eed47, 1.4.0-alpha.ff49bb35, 1.4.0-alpha.3f685417, 1.4.0-alpha.7cc2985b, 1.4.0-alpha.c4a2e1f5, 1.4.0-alpha.1c0fe35c, 1.4.0-alpha.8c9d96b7, 1.4.0-alpha.5cf0e083, 1.4.0-alpha.a79aaac4, 1.4.0-alpha.004083cf, 1.4.0-alpha.a92491ae, 1.4.0-alpha.6ae842b4, 1.4.0-alpha.b94a0894, 1.4.0-alpha.91b12b7f, 1.4.0-alpha.99e791d1, 1.4.0-alpha.1e209b68, 1.4.0-alpha.d7ecafb7, 1.4.0-alpha.b1b0f36a, 1.4.0-alpha.18f330dd, 1.4.0-alpha.b5261c5a, 1.4.0-alpha.65fc2d56, 1.4.0-alpha.c202630b, 1.4.0-alpha.b0c397aa, 1.4.0-alpha.e92be9a9, 1.4.0-alpha.05244e6c, 1.4.0-alpha.e4440c27, 1.4.0-alpha.98dd1f8b, 1.3.0-rc.5, 1.3.0-rc.6, 1.3.0-rc.7, 1.3.1, 1.4.0-alpha.65deba26, 1.4.0-alpha.82d366f4, 1.4.0-alpha.971dd13f, 1.4.0-alpha.eb118ad7, 1.4.0-alpha.9009c792, 1.4.0-alpha.ac917ffe, 1.4.0-rc.0, 1.4.0-alpha.ff961b66, 1.4.0-alpha.624e39eb, 1.4.0-alpha.0b69014a, 1.4.0-rc.1, 1.4.0-alpha.c4adcad2, 1.3.2-rc.0, 1.4.0-alpha.02d19bc1, 1.4.0-rc.2, 1.4.0-rc.3, 1.3.1-rc.1, 1.4.0-rc.4, 1.3.1-rc.2, 1.4.0-alpha.467f3674, 1.4.0-rc.5
npm ERR! notarget 
npm ERR! notarget This is most likely not a problem with npm itself.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.
npm ERR! notarget 
npm ERR! notarget It was specified as a dependency of 'primer-css'
npm ERR! notarget 

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/shawnbot/.npm/_logs/2017-10-04T17_27_02_388Z-debug.log

This should be possible with Lerna, but we still have to figure out how to ensure that prerelease versions don't conflict with previously (possibly erroneously) published release candidates. In other words, if we file a PR that will publish, say, 9.6.0, which publishes release candidate version 9.6.0-rc.0, then we close that PR and open a new one, our bump logic should be smart enough to notice that the next release candidate will be 9.6.0-rc.1.

The more complicated issue is that multiple primer-css release candidates could potentially bump submodules to the same version (which has already been the case with our 9.5.0 and 10.0.0 PRs, #353 and #354). To guard against this, we might even consider using the target primer-css version as the release candidate prerelease identifier, so you'd end up with guaranteed unique versions like:

package 9.5.0 10.0.0
primer-css 9.5.0-rc.0 10.0.0-rc.0
primer-alerts 1.5.0-9.5.0-rc.0 1.5.0-10.0.0-rc.0
primer-forms 1.4.0-9.5.0-rc.0 1.5.0-10.0.0-rc.0

So the logic would look like:

  1. Figure out the most recently published rc prerelease for the primer-css@major.minor.patch in git.

  2. Increment either the most recently published RC version or the local one with:

    version=$(semver -i prerelease --preid rc $rc_version)
  3. Use Lerna to bump all of the packages using the same prerelease identifier, which is scoped to the target primer-css version:

    lerna publish --no-git --cd-version prerelease --preid $version

    Note: this would require us to upgrade Lerna v2.1.0 or higher, which is where I added support for prereleases.

Assuming that works, we'll also have automatically published, for instance, primer-css@9.5.0-9.5.0-rc.0, in which case we may wish to manually modify the version and only re-publish primer-css@9.5.0-rc.0.

@broccolini @jonrohan lmk if this sounds right, wrong, crazy, etc.

@shawnbot
Copy link
Contributor Author

shawnbot commented Oct 4, 2017

This is not fixed by #353, unfortunately.

@shawnbot shawnbot reopened this Oct 4, 2017
@shawnbot
Copy link
Contributor Author

shawnbot commented Oct 9, 2017

@broccolini made the brilliant suggestion that we could do this locally and manually, e.g. with:

lerna publish --cd-version prerelease --preid rc

And just, like, publish release candidates more explicitly rather than automatically (on any push to a release-* branch). Thoughts?

@shawnbot
Copy link
Contributor Author

One minor (:rimshot:) snag to this is that semver (the node module) actually increments the patch version when you increment with a prerelease identifier. So that would be kind of funky because every release candidate would be a patch version higher than the eventual release.

I've done a rewrite of our release-candidate script in #370 that does, if not the completely right thing, the more correct thing than what we're doing now. @jonrohan, I'd love it if you could take a look and lmk what you think.

@jonrohan
Copy link
Member

I've merged in #370 and haven't hit any trouble yet

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

Successfully merging a pull request may close this issue.

2 participants