fix(version): create correct independent tags when using --sign-git-tag#3917
fix(version): create correct independent tags when using --sign-git-tag#3917JamesHenry merged 5 commits intolerna:mainfrom
Conversation
JamesHenry
left a comment
There was a problem hiding this comment.
Thanks a lot for this @ivml!
Out of curiosity was this GPG key gen approach copied from/inspired by another open source repo?
| NX_AGENT_NAME: ${{ matrix.agent }} | ||
|
|
||
| - name: Revoke and delete GPG key | ||
| run: | |
There was a problem hiding this comment.
We would want this to always run, even in error cases, right? So we need to add the if always syntax to this step
| - name: Revoke and delete GPG key | ||
| run: | | ||
| # As instructed in the revocation file, the colon needs to first be removed | ||
| sed -i "s/:-----BEGIN PGP PUBLIC KEY BLOCK-----/-----BEGIN PGP PUBLIC KEY BLOCK-----/" $REVOCATION_FILE |
There was a problem hiding this comment.
| sed -i "s/:-----BEGIN PGP PUBLIC KEY BLOCK-----/-----BEGIN PGP PUBLIC KEY BLOCK-----/" $REVOCATION_FILE | |
| sed -i "s/:-----BEGIN GPG PUBLIC KEY BLOCK-----/-----BEGIN GPG PUBLIC KEY BLOCK-----/" $REVOCATION_FILE |
There was a problem hiding this comment.
This one is not a typo. If you were to open a revocation file in a text editor, it would look something like this:
This is a revocation certificate for the OpenPGP key:
pub rsa2048 2023-12-11 [SC]
81CFAE4BD0034D6D1AD443408F11F369BDA6B541
uid Tester McPerson <test@example.com>
A revocation certificate is a kind of "kill switch" to publicly
declare that a key shall not anymore be used. It is not possible
to retract such a revocation certificate once it has been published.
Use it to revoke this key in case of a compromise or loss of
the secret key. However, if the secret key is still accessible,
it is better to generate a new revocation certificate and give
a reason for the revocation. For details see the description of
of the gpg command "--generate-revocation" in the GnuPG manual.
To avoid an accidental use of this file, a colon has been inserted
before the 5 dashes below. Remove this colon with a text editor
before importing and publishing this revocation certificate.
:-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: This is a revocation certificate
iQE2BCABCAAgFiEEgd+uS9ADTW0a1ENAjxHzab2msUEFAmV2yBoCHQAACgkQjxHz
...
-----END PGP PUBLIC KEY BLOCK-----
There was a problem hiding this comment.
Ah sorry that was me just not being familiar enough with these specs/standards
There was a problem hiding this comment.
To be honest it's an easy thing to miss - your other suggestions were indeed typos, so it made sense for this to also be a typo :) It's good to double-check in these situations, so your feedback was spot on!
|
Sorry just read your note about the potential difficulty for local test runs, I agree. Maybe we could exclude this spec if |
Thanks, that is a good suggestion. I attempted to do that in ab6c403 but I am unsure if there are no better alternatives for the syntax.
There is not a particular example that I am aware of. Signing a commit in a workflow is something that can be done relatively easy if you have generated a key, uploaded it to your account and made some secrets. I have stumbled on and used this gist as an example in the past - https://gist.github.com/vansergen/88eb7e71fea2e3bdaf6aa3e752371eb7 Generating the key inside the workflow, storing it, and revoking it was way less intuitive. I had a bunch of failed attempts in my fork that took small incremental changes to get to here (that I have cleaned up). I mostly had an idea, and took it to Copilot for validation and error deciphering. For example, it was that back-and-forth that told me that the revocation file path is written to |
2cc1ddc to
5956ed9
Compare
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 5956ed9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 8 targets
Sent with 💌 from NxCloud. |
|
Thanks a lot @ivml! |
Since
lerna@7.3.0, runninglerna versionwith independent versioning and the--sign-git-tagflag would always create0,1, ...Nas tags instead of the actual version.Description
A recent fix done in #3832 introduced a new issue. In TypeScript,
for (const tag in tags)would iterate through the array of tags. However,for...inreturns a list of keys on thetagsobject, meaning that it iterates through the indices of the array, rather than the values. This means that regardless of what version number we are creating a tag for, that value will never be used - its index in thetagsarray will be used instead.The issue is a bit niche, as it only happens when the repo is configured to use
independentversioning and the--sign-git-tagflag is used.Motivation and Context
This is an issue because your project might have a specific version (e.g.
1.0.0), butlerna versionwill create the tag0for it. This is especially crucial when using--conventional-commits, as that flag uses the latest tag in order to evaluate the next version bump.The issue is described in more detail in #3916.
How Has This Been Tested?
For this fix, I wrote some E2E tests in
e2e/version/src/sign-git-tag.spec.tsthat bump the version and use the--sign-git-tagflag. You can see the tests failing when usingfor...in, but pass when usingfor...of.Things get a bit interesting - since the E2E tests run actual
gitcommands, the tests need a GPG key, otherwise they will fail. For this reason, I have changed theci.ymlworkflow to generate a GPG key, store its ID and revocation file, configure git to use the key, and finally, revoke and delete the key after the tests pass.I have a feeling this might be overkill, and it probably makes testing locally harder, as it implies the user has setup a key themselves.
Therefore, I would love some feedback on the tests - whether it was a good idea, whether the implementation was overkill (especially for a one-word change), and whether there are alternatives (e.g. mocking git signing somehow).
Types of changes
Checklist: