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

Add (initial) commit-signoff and gpg-signing support #405

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

OdinVex
Copy link
Contributor

@OdinVex OdinVex commented Dec 22, 2022

Adds (initial) support for automatic commit-signoff (ex: Signed-off-by: Odin Vex <[email protected]>) and gpg-signing support on the Options->General tab. Do note that not all git servers have gpg-signing enabled, at which point it will simply throw an error (from git, safely), and you'll need to disable gpg-signing.

Some development environments require signoff or even gpg-signing to be enabled.

Note: This commit lacks localization for user-viewed text in the Settings Dialog, specifically lines 113-122 and 140-143. I wasn't sure if the texts could be worded with more clarity or not. Submitting as-is, for now.

Note: I've been using these options without the UI additions for quite a while, stable with no issues, but I use Gitea almost exclusively. This is just adding easy UI options to enable them for people who hate tampering with the git config directly.

Fixes #121 and #221.

@OdinVex OdinVex changed the title Add initial signoff and gpg-signing support Add (initial) commit-signoff and gpg-signing support Dec 22, 2022
@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 22, 2022

...I forgot to use cl-fmt.sh again, my bad. Executed, amended commit and push-forced (PR).

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 22, 2022

Maybe we could update the UI to reflect gpg-signed commits/pushes/tags. A lock icon similar to GitHub with a status tooltip of 'Verified' might be the nicest way to go. I think the following areas could use an icon:

Left of the branch name in the tracking tree, same situation on the branch details area on the right.

@exactly-one-kas
Copy link
Collaborator

I'm not sure how this works
libgit2 doesn't support automatic format.signOff

and pgp signing support is currently not implemented in libgit2 and there are no plans to support it there
The implementation for Gittyup is currently being done by @Akselmo

Do you want these options to control behavior outside of Gittyup?

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 22, 2022

Gittyup signs off on my commits with these options in my git config (inside Gittyup). I have other stuff in the config for which specific gpg key to use. I don't know anything about your backend as far as how it uses git. (It does currently work, I've been using Gittyup to do this without the UI for about a year now. I've simply added the option to toggle this behavior into the UI instead of relying on the config.) I do not know how this will affect Flatpak versions, only installed versions.

I don't have much time today to delve deep, is there a specification for libgit2 that mentions what it does support or doesn't? A very quick glance at their docs hasn't helped me, but I've also just woken up.

Edit: I forgot that last year I modified Gittyup to remove code that worked with commits to use my installation's git instead, to specifically support signoff and GPG because libgit2 does support git_commit_create_with_signature but that is required to be implemented via the software. I implemented it using host git. It was a hackish work-around I forgot about. It works, but I don't think Gittyup wants to use local git installations...even though that would bring 'your installed version of git' support. You can close this Pull Request, I'll update the Issues appropriately. Sorry I got everyone's hopes up on that.

@Alex-Kent
Copy link

Alex-Kent commented Dec 23, 2022

Edit: I forgot that last year I modified Gittyup to remove code that worked with commits to use my installation's git instead, to specifically support signoff and GPG because libgit2 does support git_commit_create_with_signature but that is required to be implemented via the software. I implemented it using host git. It was a hackish work-around I forgot about. It works, but I don't think Gittyup wants to use local git installations...even though that would bring 'your installed version of git' support.

Could you add your modifications to the patch you submitted? A configuration option could be added along the lines of "Enable signing and signoff (requires git to be installed)" Turning that option on could switch to your code instead of going through the library. If and when libgit2 adds native support the option and your work-around code could be removed.

Irrespective of whether the git side of things is handled through libgit2, one would want to make use of the user's existing gpg installation for doing the signing (for security reasons). IMO that's really not the kind of thing you want to handle in-application.

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

Edit: I forgot that last year I modified Gittyup to remove code that worked with commits to use my installation's git instead, to specifically support signoff and GPG because libgit2 does support git_commit_create_with_signature but that is required to be implemented via the software. I implemented it using host git. It was a hackish work-around I forgot about. It works, but I don't think Gittyup wants to use local git installations...even though that would bring 'your installed version of git' support.

Could you add your modifications to the patch you submitted? A configuration option could be added along the lines of "Enable signing and signoff (requires git to be installed)" Turning that option on could switch to your code instead of going through the library. If and when libgit2 adds native support the option and your work-around code could be removed.

I thought about that at first but realized that my changes are also specific to my system, I even used hard-coded paths and a lot of hackish code. I think it currently unsuitable for public use. Not only that, but I'm not that big on Qt, so it's a bit labored.

As far as libgit2 is concerned, it has support for allowing the client to specify signed objects and stuff, it requires the client to handle the means of handing data off to be signed by the host (or whatever non-libgit2 means) and libgit2 can use it with git_commit_create_with_signature. For signoff, I believe Gittyup has something like that 'kind of' happening in the code, but I'm not sure. It's more like an invented work-around 'featurette' of Gittyup. Could be wrong on that bit, though, I'm exhausted lately.

Edit: I'm not sure why Gittyup was developed to use libgit2 in the first place. I would've simply required git to be installed to work alongside it. That way people could get newer features with less effort from Gittyup (or roll-back if ever desired), plus customization of interactions with git.

@Alex-Kent
Copy link

Alex-Kent commented Dec 23, 2022

See my edit regarding GPG.

Edit: I'm not sure why Gittyup was developed to use libgit2 in the first place. I would've simply required git to be installed to work alongside it. That way people could get newer features with less effort from Gittyup (or roll-back if ever desired), plus customization of interactions with git.

+1

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

See my edit regarding GPG

Just seen, and I agree, I'd rather let a git binary installed do all of the work, it'd be known standard and compatible without any possible hiccups, and that's why I went that way with my local src (several versions old but also contains all of the fixes, some stuff is just half-hazardly pulled in). I merely meant that libgit2 does include the ability to commit with a signature, but requires the client to handle the actual signing and such.

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

A hackish work-around for now might be to have Gittyup automatically append to commit messages Signed-off-by: Herp Derp <[email protected]> if it doesn't end with it already, but it's hackish..eh. Before I implemented them in my binary, I simply swapped to terminal and did a commit signed with signoff as well. git commit -S -s -m "Herp Derp added" -m "Description of changes." *shrug*

@Alex-Kent
Copy link

Kernighan and Pike's words come to mind: Program design in the UNIX environment

@Alex-Kent
Copy link

To elaborate on my earlier comment regarding GPG:

Calling an external gpg binary to handle signing is pretty much a requirement. Even if signing could be done in-application many organizations exert tight control over private keys and flat-out disallow access to such material except through a small number of trusted applications (e.g. gpg). Doing the crypto in-application would (at the very least) make the signing feature unusable in such environments.

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

To elaborate on my earlier comment regarding GPG:

Calling an external gpg binary to handle signing is pretty much a requirement. Even if signing could be done in-application many organizations exert tight control over private keys and flat-out disallow access to such material except through a small number of trusted applications (e.g. gpg). Doing the crypto in-application would (at the very least) make the signing feature unusable in such environments.

I agree, but they seem intent on using Flatpak, -_-.

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

I've managed to refactor the latest version of Gittyup to the point that the decision for a wrapper around calling gpg-signing can be implemented by adjusting src/git/Repository.cpp, specifically Commit Repository::commit(const Signature &author, const Signature &committer, const QString &message, const AnnotatedCommit &mergeHead).

I've edited the function to use a buffer instead, so that the path the routine takes can transparently implement signing or optionally not. It's a preparatory refactor for adding in the simple “if gpg, sign” one-liner from a later-implemented wrapper. I haven't pushed that section to my fork yet, I'm wondering what the lead developer wishes to do as far as methods of implementation. Can Flatpak integrate gpg-related stuff? Can Flatpak 'escape' the sandbox (or can holes be poked in with Flatseal) to allow access to host gpg?

Added UI checkboxes to graphically manage global config options regarding automatically signing-off on commits and gpg-enabled signing

Signed-off-by: Odin Vex <[email protected]>
@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 23, 2022

Woops, ignore that push, only the Repository.cpp stuff is relevant right now. Will wait on pushing to my fork until done.

@Murmele
Copy link
Owner

Murmele commented Dec 29, 2022

@OdinVex can you mark the mr as draft as long as it is not ready?

@OdinVex
Copy link
Contributor Author

OdinVex commented Dec 29, 2022

@OdinVex can you mark the mr as draft as long as it is not ready?

I've just now learned of marking things as Draft. Neat. :) It won't be ready until a decision can be made about how best to GPG sign things. Either a migration to shell-executed git binaries or a different library than libgit2 or include an additional library for GPG interactions, such as gpgme. Marking as Draft in the meanwhile. (My local copy which relies on a much older base of Gittyup with everything pulled in has all of the libgit2 stuff removed and uses the executing environment.) Most reasons in above posts.

Edit: The Convert to Draft doesn't actually do anything for me, GitHub's JavaCrap isn't working correctly for me. Whatever happened to the good ol' days without JavaCrap... *sigh* Will mark as Draft when able. Will find a work-around.

Edit: Marked as Draft, finally. Had to edit page HTML to a simple HTML5 form and disconnect all JavaCrap routines on the fly dynamically. *grumbles about JavaScript should be made illegal*

@OdinVex OdinVex marked this pull request as draft December 29, 2022 17:40
@Murmele Murmele added the enhancement New feature or request label Jan 4, 2023
@mwerle
Copy link
Contributor

mwerle commented Nov 20, 2023

Would it be feasibly to split this PR into two separate items? One for the "Signed-off-by" and one for the GPG work? It seems to me that there is no holdup for the former, while there are decisions to be made for the GPG work to be completed.

@OdinVex
Copy link
Contributor Author

OdinVex commented Nov 20, 2023

Would it be feasibly to split this PR into two separate items? One for the "Signed-off-by" and one for the GPG work? It seems to me that there is no holdup for the former, while there are decisions to be made for the GPG work to be completed.

Sure. I ended up writing scripts to do what I want anyway. This more or less got it prepared for it, only needed the GPG sign piece and there is a one-liner in there with a TODO comment showing where. Maybe edit and comment that out, but still import entire pull request and remind that it is initial support for the concept of GPG signing but complete support for commit-signoff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] GPG Signing Support
5 participants