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

doc: suggest disabling GPG signed commits to allow build to pass #225

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

dawngerpony
Copy link
Contributor

@dawngerpony dawngerpony commented Aug 30, 2023

Add instructions to disable signed commits in the rewrite repository if necessary because JGit does not currently have support for SSH format GPG signatures.

What's changed?

Added a suggestion to the "building from source" page to disable GPG signed commits in the repo, in case you are using SSH format GPG signed commits globally, which JGit does not currently support.

What's your motivation?

Make it easier for people to build OpenRewrite from source without running into mysterious test errors related to their git configuration.

Errors during ./gradlew build look like this when SSH GPG commit signing is configured:

GitProvenanceTest > detachedHeadJenkinsNoLocalBranch(Path) FAILED
    java.lang.IllegalArgumentException: Invalid value: gpg.format=ssh
        at org.eclipse.jgit.lib.DefaultTypedConfigGetter.getEnum(DefaultTypedConfigGetter.java:105)
        at org.eclipse.jgit.lib.Config.getEnum(Config.java:403)
        at org.eclipse.jgit.lib.GpgConfig.<init>(GpgConfig.java:86)
        at org.eclipse.jgit.api.CommitCommand.processOptions(CommitCommand.java:620)
        at org.eclipse.jgit.api.CommitCommand.call(CommitCommand.java:180)
        at org.openrewrite.marker.GitProvenanceTest.initGitWithOneCommit(GitProvenanceTest.java:143)
        at org.openrewrite.marker.GitProvenanceTest.detachedHeadJenkinsNoLocalBranch(GitProvenanceTest.java:127)

GitProvenanceTest > detachedHeadBehindBranchHead(Path) FAILED
    java.lang.IllegalArgumentException: Invalid value: gpg.format=ssh
        at org.eclipse.jgit.lib.DefaultTypedConfigGetter.getEnum(DefaultTypedConfigGetter.java:105)
        at org.eclipse.jgit.lib.Config.getEnum(Config.java:403)
        at org.eclipse.jgit.lib.GpgConfig.<init>(GpgConfig.java:86)
        at org.eclipse.jgit.api.CommitCommand.processOptions(CommitCommand.java:620)
        at org.eclipse.jgit.api.CommitCommand.call(CommitCommand.java:180)
        at org.openrewrite.marker.GitProvenanceTest.detachedHeadBehindBranchHead(GitProvenanceTest.java:160)

Anything in particular you'd like reviewers to focus on?

Not really, this is a single-line documentation update.

Anyone you would like to review specifically?

No.

Have you considered any alternatives or workarounds?

Yes, disable GPG signed commits globally, but this may not be an option or the most convenient option for most people because they would like to sign their commits elsewhere.

I looked into upgrading JGit but not even master supports the SSH format. See GpgConfig.java for more information, or this bug.

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases N/A
  • I've added the license header to any new files through ./gradlew licenseFormat N/A, no new files
  • I've used the IntelliJ IDEA auto-formatter on affected files N/A
  • I've updated the documentation (if applicable)

Add instructions to disable signed commits in the rewrite repository if necessary because JGit does not currently have support for SSH format GPG signatures.
@dawngerpony dawngerpony changed the title Suggest disabling GPG signed commits to allow build to pass Doc: suggest disabling GPG signed commits to allow build to pass Aug 30, 2023
@dawngerpony dawngerpony changed the title Doc: suggest disabling GPG signed commits to allow build to pass doc: suggest disabling GPG signed commits to allow build to pass Aug 30, 2023
@timtebeek timtebeek requested a review from mike-solomon August 30, 2023 10:39
@timtebeek timtebeek added the documentation Improvements or additions to documentation label Aug 30, 2023
@timtebeek
Copy link
Contributor

Sorry to hear you had issues getting the tests to work, and thanks for the detailed analysis and update to the docs here! I've tagged Mike for a final review before we merge; this is very helpful to get sorted thanks!

Copy link
Contributor

@mike-solomon mike-solomon left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Thanks so much for documenting this! Appreciate you taking the time to help others.

@mike-solomon mike-solomon merged commit a4846d0 into openrewrite:master Aug 30, 2023
3 checks passed
@dawngerpony
Copy link
Contributor Author

You're both very welcome! I'd like to start contributing more to the OpenRewrite codebase itself and the docs felt like a good place to start. Great to have my PR merged so quickly 🚀

@dawngerpony dawngerpony deleted the patch-1 branch August 31, 2023 06:36
@timtebeek
Copy link
Contributor

Glad to hear you'd like to start contributing more! Feel free to tag me in any issue you'd like to take on, and I can assign the issue to you and provide guidance as needed. We have a few items tagged as good-first-issue, but you're welcome to work on anything. We also have a public Slack if you have any questions. Hope that helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants