-
Notifications
You must be signed in to change notification settings - Fork 178
New option to be able to disable sign for commit/tag to be able use this with jgit when you have gpgsign configured in ~/.gitconfig #1266
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
Conversation
…his with jgit when you have gpgsign forced in ~/.gitconfig Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
.setMessage(message) | ||
.setAuthor(author.name, author.email) | ||
.setCommitter(committer.name, committer.email) | ||
.setSign(parameters.getBoolean(CommandParameter.SCM_COMMIT_SIGN, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@olamy This is actually a tri-state where true
means force signing (which often fails with Signing service not available
). I think the default should be null
(https://javadoc.io/static/org.eclipse.jgit/org.eclipse.jgit/5.13.0.202109080827-r/org/eclipse/jgit/api/CommitCommand.html#setSign-java.lang.Boolean-) to consider signing settings from the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indicates that a JGit test case is somehow missing here as well...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great.
Iintiallly, I added this option because I have this in ~/.gitconfig
and so couldn't run unit test without failures.
[commit]
gpgSign = true
[tag]
gpgSign = true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO signing with JGit requires a dependency on Bouncycastle probably some more glue code: https://bugs.eclipse.org/bugs/show_bug.cgi?id=382212.
The BouncyCastle-based signer lives in bundle org.eclipse.jgit.gpg.bc. GpgSigner in org.eclipse.jgit uses the ServiceLocator mechanism to find the concrete implementation. org.eclipse.jgit.gpg.bc is an OSGi fragment for host bundle org.eclipse.jgit, so in an OSGi environment it should find the implementation right away. In other environments you may have to ensure the class loader has the fragment on its classpath, or use GpgSigner.setDefault() explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes correct not really sure we can support it .
the idea was only to be able to disable it in some cases for jgit and gitexe
* @since 2.1.1 | ||
*/ | ||
@Parameter(property = "forceNoSign", defaultValue = "false") | ||
private boolean forceNoSign; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there is both sign
and forceNoSign
which can be set independently of each other? Which ones is gonna win @olamy ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing really prevent you using git tag --sign --no-sign
But they do have a different behaviour (but I agree no sense used together maybe a warning in this case?)
forcing --sign
in a the mojo means you forcing the tag to be signed (whatever environment people have).
The other one is really a sort of parameter to avoid failure for scm developer having configured signing in ~/.gitconfig
another option is possibly for testing only to change local configuration of the git test repos.
such
git config --local commit.gpgsign false
git config --local tag.gpgsign false
I vaguely remember issue with jgit and modifying my ~/.gitconfig
(so if testing that make a backup first :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look at my proposal in #1293.
Signed-off-by: Olivier Lamy olamy@apache.org
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verify
to make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
mvn -Prun-its verify
).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.