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

tools: fix release script on macOS 10.12 #8824

Closed
wants to merge 1 commit into from

Conversation

evanlucas
Copy link
Contributor

Checklist
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

Description of change

Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822

/cc @nodejs/release

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Sep 28, 2016
@MylesBorins
Copy link
Contributor

I'd like to verify that this doesn't break support for subkeys... as this code has been fragile to that before

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM but I agree with @thealphanerd that we'll want to test this a bit more before landing

@evanlucas
Copy link
Contributor Author

/cc @Fishrock123 and @rvagg any opinion on this?

@rvagg
Copy link
Member

rvagg commented Oct 12, 2016

I think this might be from a new version of git, or perhaps gnupg, I'm noticing now on 10.10 that it comes out saying "using RSA key ..." with "key ID" nowhere to be found!

I'd still be more comfortable limiting the text a tiny bit more, perhaps just inserting a grep key there, without the awk?

@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@evanlucas
Copy link
Contributor Author

@rvagg the reason I switched it around was because more of the key is now shown in the newer versions. Here is an example to show what I mean:

Before:

gpg: Signature made Tue Sep 27 19:06:18 2016 CDT using RSA key ID 4C206CA9

After:

gpg:                using RSA key B63B535A4C206CA9

So the if [ "${gpgtagkey}" != "${gpgkey}" ]; then would end up failing

@rvagg
Copy link
Member

rvagg commented Oct 19, 2016

@evanlucas what I meant was, inserting a grep for the word "key" as well, probably not really a material change though.

@thealphanerd just got caught by this for v6.9.1 so we should get it merged and backported to v7, v6 and v4.

Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: nodejs#8822
@evanlucas
Copy link
Contributor Author

@rvagg ah sorry. Updated. PTAL

@rvagg
Copy link
Member

rvagg commented Oct 20, 2016

lgtm

jasnell pushed a commit that referenced this pull request Oct 20, 2016
Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822
PR-URL: #8824
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@jasnell
Copy link
Member

jasnell commented Oct 20, 2016

Landed in 6845d6e

@jasnell jasnell closed this Oct 20, 2016
jasnell pushed a commit that referenced this pull request Oct 20, 2016
Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822
PR-URL: #8824
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822
PR-URL: #8824
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 11, 2016
Previously, we were relying on the output of gpg from git tag -v to
verify that the key selected by the releaser is the key that was used
to sign the tag. This output can change depending on the version of git
being used. Now, we just check that the output of git tag -v contains
the key selected.

Fixes: #8822
PR-URL: #8824
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
This was referenced Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants