* bug#63261: Recent changes to git config cause errors for non-committers @ 2023-05-04 10:47 Brian Cully via Bug reports for GNU Guix 2023-05-04 13:38 ` Maxim Cournoyer ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Brian Cully via Bug reports for GNU Guix @ 2023-05-04 10:47 UTC (permalink / raw) To: 63261; +Cc: Maxim Cournoyer I've run into two issues with the recent changes to git config integration: 1) All commits must now be signed, even if you're not a committer. This breaks just tons of things, including rebasing. I'm not sure how to fix this without just disabling that configuration line altogether. 2) Some ‘make’ rules now require git to be installed so that ‘git config’ can add ‘etc/git/gitconfig’ to the local configuration. So, for instance, ‘guix shell --pure -D guix -- make’ will now fail. Calls to git should be prefixed with a test to see if there is a git executable in the path. -bjc ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#63261: Recent changes to git config cause errors for non-committers 2023-05-04 10:47 bug#63261: Recent changes to git config cause errors for non-committers Brian Cully via Bug reports for GNU Guix @ 2023-05-04 13:38 ` Maxim Cournoyer 2023-05-04 14:06 ` bug#63261: [PATCH] Makefile.am: Only auto-configure Git when available Maxim Cournoyer 2023-05-07 3:34 ` bug#63261: Recent changes to git config cause errors for non-committers Maxim Cournoyer 2 siblings, 0 replies; 14+ messages in thread From: Maxim Cournoyer @ 2023-05-04 13:38 UTC (permalink / raw) To: Brian Cully; +Cc: 63261 Hi! Thanks for the feedback. Brian Cully <bjc@spork.org> writes: > I've run into two issues with the recent changes to git config > integration: > > 1) All commits must now be signed, even if you're not a > committer. This breaks just tons of things, including rebasing. I'm > not sure how to fix this without just disabling that configuration > line altogether. Could you elaborate on 'tons of things' ? :-) And why does it break rebasing? I rebase my branches often while signing the commits, so I'll need more details to understand the issue. The idea was to distribute the basic configuration that makes collaborating on Guix easier, by auto-configuring things that previously were left to do manually. By having non-committers also follow the committers' flow, it also prepares them to become committers, if they wish :-). > 2) Some ‘make’ rules now require git to be installed so that ‘git > config’ can add ‘etc/git/gitconfig’ to the local configuration. So, > for instance, ‘guix shell --pure -D guix -- make’ will now fail. Calls > to git should be prefixed with a test to see if there is a git > executable in the path. This one is a clear problem, for example causing issues to build a release tarball of Guix where git shouldn't be a requirement. It should be easy to fix with a test as you suggest. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#63261: [PATCH] Makefile.am: Only auto-configure Git when available. 2023-05-04 10:47 bug#63261: Recent changes to git config cause errors for non-committers Brian Cully via Bug reports for GNU Guix 2023-05-04 13:38 ` Maxim Cournoyer @ 2023-05-04 14:06 ` Maxim Cournoyer 2023-05-07 3:34 ` bug#63261: Recent changes to git config cause errors for non-committers Maxim Cournoyer 2 siblings, 0 replies; 14+ messages in thread From: Maxim Cournoyer @ 2023-05-04 14:06 UTC (permalink / raw) To: 63261; +Cc: Maxim Cournoyer, bjc * Makefile.am (.git/hooks/pre-push): Only run recipe if the '.git' directory exists. Make it silent. (.git/config): Likewise, and also check if the 'git' command is available. Reported-by: Brian Cully <bjc@spork.org> --- Makefile.am | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 4a9124e0c2..8af69f88ac 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1115,10 +1115,14 @@ cuirass-jobs: $(GOBJECTS) # Git auto-configuration. .git/hooks/pre-push: etc/git/pre-push - cp etc/git/pre-push .git/hooks/pre-push + $(AM_V_at)if test -d .git; then \ + cp etc/git/pre-push .git/hooks/pre-push; \ + fi .git/config: etc/git/gitconfig - git config include.path ../etc/git/gitconfig + $(AM_V_at)if command -v git >/dev/null && test -d .git; then \ + git config include.path ../etc/git/gitconfig; \ + fi nodist_noinst_DATA = .git/hooks/pre-push .git/config base-commit: ae6643326a983d141fd8320a755181d21e5a1f2f -- 2.39.2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#63261: Recent changes to git config cause errors for non-committers 2023-05-04 10:47 bug#63261: Recent changes to git config cause errors for non-committers Brian Cully via Bug reports for GNU Guix 2023-05-04 13:38 ` Maxim Cournoyer 2023-05-04 14:06 ` bug#63261: [PATCH] Makefile.am: Only auto-configure Git when available Maxim Cournoyer @ 2023-05-07 3:34 ` Maxim Cournoyer 2023-05-13 19:03 ` Leo Famulari 2 siblings, 1 reply; 14+ messages in thread From: Maxim Cournoyer @ 2023-05-07 3:34 UTC (permalink / raw) To: Brian Cully via Bug reports for GNU Guix; +Cc: 63261-done, Brian Cully Hi, Brian Cully via Bug reports for GNU Guix <bug-guix@gnu.org> writes: > I've run into two issues with the recent changes to git config > integration: > > 1) All commits must now be signed, even if you're not a > committer. This breaks just tons of things, including rebasing. I'm > not sure how to fix this without just disabling that configuration > line altogether. > > 2) Some ‘make’ rules now require git to be installed so that ‘git > config’ can add ‘etc/git/gitconfig’ to the local configuration. So, > for instance, ‘guix shell --pure -D guix -- make’ will now fail. Calls > to git should be prefixed with a test to see if there is a git > executable in the path. 2. has been fixed (also by Ludovic, ah!). I'm not sure a good solution can be found for 1. but it seems reasonable to me that contributors working on Guix learn to sign their commits with GnuPG; that way they are on the right path to become a Guix committer, already getting up to speed with the required practices. Closing, but we can continue discussing it. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#63261: Recent changes to git config cause errors for non-committers 2023-05-07 3:34 ` bug#63261: Recent changes to git config cause errors for non-committers Maxim Cournoyer @ 2023-05-13 19:03 ` Leo Famulari 2023-05-15 13:38 ` Maxim Cournoyer 0 siblings, 1 reply; 14+ messages in thread From: Leo Famulari @ 2023-05-13 19:03 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: 63261, 63261-done, Brian Cully Can someone help me by pointing to the original discussion of this change? ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#63261: Recent changes to git config cause errors for non-committers 2023-05-13 19:03 ` Leo Famulari @ 2023-05-15 13:38 ` Maxim Cournoyer 2023-05-18 16:59 ` Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] Leo Famulari 0 siblings, 1 reply; 14+ messages in thread From: Maxim Cournoyer @ 2023-05-15 13:38 UTC (permalink / raw) To: Leo Famulari; +Cc: 63261, 63261-done, Brian Cully Hi Leo, Leo Famulari <leo@famulari.name> writes: > Can someone help me by pointing to the original discussion of this > change? It was the team.scm issue in https://issues.guix.gnu.org/58813 that led to this change. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 14+ messages in thread
* Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-15 13:38 ` Maxim Cournoyer @ 2023-05-18 16:59 ` Leo Famulari 2023-05-19 3:24 ` Maxim Cournoyer 0 siblings, 1 reply; 14+ messages in thread From: Leo Famulari @ 2023-05-18 16:59 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel On Mon, May 15, 2023 at 09:38:33AM -0400, Maxim Cournoyer wrote: > Hi Leo, > > Leo Famulari <leo@famulari.name> writes: > > > Can someone help me by pointing to the original discussion of this > > change? > > It was the team.scm issue in https://issues.guix.gnu.org/58813 that led > to this change. Thanks for pointing it out. In my opinion, this change should be discussed more publicly so that a consensus can be reached about it. This change affects the workflow of everyone that hacks on Guix, including people who are trying it for the very first time. It's a big change. It's not okay to only discuss such a big change in a ticket with the title "can't substitute etc/teams.scm command as doc suggests". I am asking for this to be reverted until a consensus is reached. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-18 16:59 ` Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] Leo Famulari @ 2023-05-19 3:24 ` Maxim Cournoyer 2023-05-19 9:34 ` Josselin Poiret 0 siblings, 1 reply; 14+ messages in thread From: Maxim Cournoyer @ 2023-05-19 3:24 UTC (permalink / raw) To: Leo Famulari; +Cc: guix-devel Hi Leo, Leo Famulari <leo@famulari.name> writes: > On Mon, May 15, 2023 at 09:38:33AM -0400, Maxim Cournoyer wrote: >> Hi Leo, >> >> Leo Famulari <leo@famulari.name> writes: >> >> > Can someone help me by pointing to the original discussion of this >> > change? >> >> It was the team.scm issue in https://issues.guix.gnu.org/58813 that led >> to this change. > > Thanks for pointing it out. > > In my opinion, this change should be discussed more publicly so that a > consensus can be reached about it. > > This change affects the workflow of everyone that hacks on Guix, > including people who are trying it for the very first time. > > It's a big change. > > It's not okay to only discuss such a big change in a ticket with the > title "can't substitute etc/teams.scm command as doc suggests". > > I am asking for this to be reverted until a consensus is reached. Thanks for voicing your thoughts on this. I originally thought the included fragments (via the 'include.path' git config option) could we overridden by a user but it seems they can't, making the change more intrusive than it should have been. I've reverted just the offending bits: --8<---------------cut here---------------start------------->8--- diff --git a/etc/git/gitconfig b/etc/git/gitconfig index 831fbce6e7..907ad01804 100644 --- a/etc/git/gitconfig +++ b/etc/git/gitconfig @@ -1,6 +1,3 @@ -[commit] - gpgsign = true - [diff "scheme"] xfuncname = "^(\\(define.*)$" --8<---------------cut here---------------end--------------->8--- Let me know if something is amiss. -- Thanks, Maxim ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-19 3:24 ` Maxim Cournoyer @ 2023-05-19 9:34 ` Josselin Poiret 2023-05-19 16:22 ` Simon Tournier 2023-05-26 17:54 ` Leo Famulari 0 siblings, 2 replies; 14+ messages in thread From: Josselin Poiret @ 2023-05-19 9:34 UTC (permalink / raw) To: Maxim Cournoyer, Leo Famulari; +Cc: guix-devel [-- Attachment #1: Type: text/plain, Size: 872 bytes --] Hi Maxim and Leo, Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Thanks for voicing your thoughts on this. I originally thought the > included fragments (via the 'include.path' git config option) could we > overridden by a user but it seems they can't, making the change more > intrusive than it should have been. Just FYI, it seems that it was possible to override that behavior by making sure the include was before the overriding gpgsign = false in the repo's .git/config. Still, it's probably best to let users judge what their workflow should be. I'm curious Leo, in general (not Guix because we have a pre-push hook), how do you make sure you always publish signed commits? I don't want to put unsigned commits anywhere except locally, but it feels like I might just forget to sign them before pushing. Best, -- Josselin Poiret [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 682 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-19 9:34 ` Josselin Poiret @ 2023-05-19 16:22 ` Simon Tournier 2023-05-24 21:00 ` Vagrant Cascadian 2023-05-26 17:54 ` Leo Famulari 1 sibling, 1 reply; 14+ messages in thread From: Simon Tournier @ 2023-05-19 16:22 UTC (permalink / raw) To: Josselin Poiret, Maxim Cournoyer, Leo Famulari; +Cc: guix-devel Hi Josselin, On ven., 19 mai 2023 at 11:34, Josselin Poiret <dev@jpoiret.xyz> wrote: > I'm curious Leo, in general (not Guix because we have a pre-push hook), > how do you make sure you always publish signed commits? I don't want to > put unsigned commits anywhere except locally, but it feels like I might > just forget to sign them before pushing. Well, I am not Leo. :-) Maybe I misunderstand your question but usually my file ~/.gitconfig contains my default; say always sign. Then locally, for some project [1], I set other options with the local file .git/config of the repository. And for the ones I do not want to sign locally but I will push signed, I have pre-push hooks. Note, in practise, I do not have such configuration. :-) 1: https://gitlab.inria.fr/guix-hpc/website Cheers, simon ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-19 16:22 ` Simon Tournier @ 2023-05-24 21:00 ` Vagrant Cascadian 2023-05-24 21:17 ` Felix Lechner via Development of GNU Guix and the GNU System distribution. 0 siblings, 1 reply; 14+ messages in thread From: Vagrant Cascadian @ 2023-05-24 21:00 UTC (permalink / raw) To: Simon Tournier, Josselin Poiret, Maxim Cournoyer, Leo Famulari; +Cc: guix-devel [-- Attachment #1: Type: text/plain, Size: 1474 bytes --] On 2023-05-19, Simon Tournier wrote: > On ven., 19 mai 2023 at 11:34, Josselin Poiret <dev@jpoiret.xyz> wrote: >> I'm curious Leo, in general (not Guix because we have a pre-push hook), >> how do you make sure you always publish signed commits? I don't want to >> put unsigned commits anywhere except locally, but it feels like I might >> just forget to sign them before pushing. > > Well, I am not Leo. :-) Maybe I misunderstand your question but usually > my file ~/.gitconfig contains my default; say always sign. Then > locally, for some project [1], I set other options with the local file > .git/config of the repository. > > And for the ones I do not want to sign locally but I will push signed, I > have pre-push hooks. Note, in practise, I do not have such > configuration. :-) This is basically a show-stopper for me working on guix right now. I intentionally do not have access to my openpgp key on Guix System machines. This completely breaks my workflow. Neither changing ~/.gitconfig not .git/config in the working repository seems to work around this. I think the case can be made that not requiring signatures will actually prevent unintentional changes from getting pushed to the archive, as the server-side hooks will prevent unsigned changes from landing in the repository... this is why I prefer to leave my local work-in-progress changes unsigned. I only sign things I am confident I might want to push. Please revert ASAP. live well, vagrant [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-24 21:00 ` Vagrant Cascadian @ 2023-05-24 21:17 ` Felix Lechner via Development of GNU Guix and the GNU System distribution. 2023-05-24 22:02 ` Vagrant Cascadian 0 siblings, 1 reply; 14+ messages in thread From: Felix Lechner via Development of GNU Guix and the GNU System distribution. @ 2023-05-24 21:17 UTC (permalink / raw) To: Vagrant Cascadian Cc: Simon Tournier, Josselin Poiret, Maxim Cournoyer, Leo Famulari, guix-devel Hi Vagrant, On Wed, May 24, 2023 at 2:01 PM Vagrant Cascadian <vagrant@debian.org> wrote: > > Please revert ASAP. I am not affected, but if I understood Maxim's message from 5/18 correctly, the signature requirement was already reverted: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=03b453cfe54756bcec6b7c7dfaf71566d84c7a75 Peace, Felix ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-24 21:17 ` Felix Lechner via Development of GNU Guix and the GNU System distribution. @ 2023-05-24 22:02 ` Vagrant Cascadian 0 siblings, 0 replies; 14+ messages in thread From: Vagrant Cascadian @ 2023-05-24 22:02 UTC (permalink / raw) To: Felix Lechner Cc: Simon Tournier, Josselin Poiret, Maxim Cournoyer, Leo Famulari, guix-devel [-- Attachment #1: Type: text/plain, Size: 1832 bytes --] On 2023-05-24, Felix Lechner via "Development of GNU Guix and the GNU System distribution." wrote: > On Wed, May 24, 2023 at 2:01 PM Vagrant Cascadian <vagrant@debian.org> wrote: >> >> Please revert ASAP. > > I am not affected, but if I understood Maxim's message from 5/18 > correctly, the signature requirement was already reverted: > > https://git.savannah.gnu.org/cgit/guix.git/commit/?id=03b453cfe54756bcec6b7c7dfaf71566d84c7a75 True! Figured this out, but wow it was circuitous! So, the very non-obvious behavior was triggered by using git worktrees with a mix of updated and not very updated checkouts... So I have ~/src/guix and a ~/src/guix-workspace which is a worktree from ~/src/guix... When I ran "./configure && ./bootstrap ... && make" from ~/src/guix-workspace it installed a rule in ~/src/guix/.git/config: [include] path = ../etc/git/gitconfig So there was no obvious entry for [commit] in the configuration file... so on a glance I missed that. Manually adding: [commit] gpgsign=false Did work around the problem. My initial check for ~/src/guix-workspace/.git/config was wrong, as it was a worktree and .git is a file... so it obviously did not show any configuration at a non-resolvable path. Also updating the git checkout at ~/src/guix fixed the issue, even though I was *working* from ~/src/guix-workspace and my working directory etc/git/gitconfig had the fix... Having an [include] entry pointing to files that were not in my working directory from some arbitrary checkout (I will reiterate my working directory actually *did* have the fix!) kind of broke my mind a bit. I get it now. I think. :) This is not the first time I have been burned by using worktrees, but it was certainly the most convoluted! live well, vagrant [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 227 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] 2023-05-19 9:34 ` Josselin Poiret 2023-05-19 16:22 ` Simon Tournier @ 2023-05-26 17:54 ` Leo Famulari 1 sibling, 0 replies; 14+ messages in thread From: Leo Famulari @ 2023-05-26 17:54 UTC (permalink / raw) To: Josselin Poiret; +Cc: Maxim Cournoyer, guix-devel [-- Attachment #1: Type: text/plain, Size: 1255 bytes --] On Fri, May 19, 2023 at 11:34:35AM +0200, Josselin Poiret wrote: > I'm curious Leo, in general (not Guix because we have a pre-push hook), > how do you make sure you always publish signed commits? I don't want to > put unsigned commits anywhere except locally, but it feels like I might > just forget to sign them before pushing. In general, I don't rigorously sign Git commits for projects that aren't Guix. You could set "gpgsign = true" in '~/.gitconfig'. I do sign commits sometimes for non-Guix projects, but without a code-authentication system like Guix's, I don't perceive a strong reason to always sign commits. There is *some* reason to always sign commits, which is to provide an unambiguous statement of authorship / provenance. But, it doesn't seem like most projects have a mechanism with which to derive value from the signatures. Also, it doesn't seem like there is much demand for this, in general. Git itself offers nothing, so each project has to design their own solution. I doubt many projects would consider that effort to be worthwhile. Instead they rely on the access controls of their centralized repo, typically Github, and Github's security seems fine in practice. I think that Guix is pushing the state of the art here. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-05-26 17:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-04 10:47 bug#63261: Recent changes to git config cause errors for non-committers Brian Cully via Bug reports for GNU Guix 2023-05-04 13:38 ` Maxim Cournoyer 2023-05-04 14:06 ` bug#63261: [PATCH] Makefile.am: Only auto-configure Git when available Maxim Cournoyer 2023-05-07 3:34 ` bug#63261: Recent changes to git config cause errors for non-committers Maxim Cournoyer 2023-05-13 19:03 ` Leo Famulari 2023-05-15 13:38 ` Maxim Cournoyer 2023-05-18 16:59 ` Should commit signing always be required for local work? [was Re: bug#63261: Recent changes to git config cause errors for non-committers] Leo Famulari 2023-05-19 3:24 ` Maxim Cournoyer 2023-05-19 9:34 ` Josselin Poiret 2023-05-19 16:22 ` Simon Tournier 2023-05-24 21:00 ` Vagrant Cascadian 2023-05-24 21:17 ` Felix Lechner via Development of GNU Guix and the GNU System distribution. 2023-05-24 22:02 ` Vagrant Cascadian 2023-05-26 17:54 ` Leo Famulari
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.