Re: Updating the “pre-push” Git hook
On Mon, May 25, 2020 at 01:13:16PM -0700, Vagrant Cascadian wrote: > Wait a minute... you're saying this is something that needs to be > configured on each committer's machine(s)? Yes, it was recommended in HACKING and then, after we removed that file, in the Commit Access section of the manual. We try to remind new committers to read these files but maybe we forgot for you. However, the security model does not depend on either it or on a post-receive server hook. It's simply to prevent mistakes. There are commits in the repo that are unsigned when they should have been signed, and even one commit that is signed but that fails the signature check :/ A simple pre-push hook would have caught that. signature.asc Description: PGP signature
Re: Updating the “pre-push” Git hook
Hi, Ricardo Wurmus skribis: > Ludovic Courtès writes: > >> Could you try: >> >> mv ~/.cache/guix/authentication/channels/guix{,.bak} >> time make authenticate >> mv ~/.cache/guix/authentication/channels/guix{.bak,} >> >> ? > > real 0m49.496s > user 0m43.733s > sys 0m1.658s Same timing if you run: make guix/{git,openpgp}.go beforehand? I get 29s for 16865 commits. > And then running it again: > > $ [env] time make authenticate > Authenticating Git checkout... > Authenticating d68de95 to fb1675e (0 commits)... > > real 0m2.692s > user 0m2.877s > sys 0m0.128s Oh I see that too. Roughly half of the time seems to be spent loading the keyring from the ‘keyring’ branch, and the other half is traversing the commit graph: --8<---cut here---start->8--- $ ./pre-inst-env guile --no-auto-compile -e git-authenticate ./build-aux/git-authenticate.scm d68de958b60426798ed62797ff7c96c327a672ac $(git rev-parse HEAD) Authenticating d68de95 to a1a3bd5 (0 commits)... % cumulative self time seconds seconds procedure 37.04 0.73 0.73 anon #x4a44d0 12.96 0.25 0.25 anon #x49e788 11.11 0.22 0.22 guix/openpgp.scm:1030:0:crc24 6.48 0.16 0.13 anon #x4a2810 4.63 0.09 0.09 ice-9/popen.scm:145:0:reap-pipes 3.70 0.07 0.07 ice-9/vlist.scm:539:0:vhash-assq 3.70 0.07 0.07 anon #x497578 2.78 0.05 0.05 anon #x4a52c0 2.78 0.05 0.05 anon #x4a27e0 2.78 0.05 0.05 anon #x49d9e0 1.85 0.05 0.04 anon #x4a5494 1.85 0.05 0.04 anon #x497190 0.93 0.62 0.02 guix/openpgp.scm:1056:0:read-radix-64 0.93 0.33 0.02 gcrypt/base64.scm:154:2:base64-decode 0.93 0.04 0.02 anon #x4a2878 0.93 0.02 0.02 anon #x4a2840 0.93 0.02 0.02 anon #x49f928 0.93 0.02 0.02 git/commit.scm:213:0:commit-parents 0.93 0.02 0.02 anon #x4a1ab8 0.93 0.02 0.02 anon #x4975a8 0.93 0.02 0.02 anon #x5f6210 0.00 1.96 0.00 /home/ludo/src/guix/build-aux/git-authenticate.scm:430:3 0.00 1.15 0.00 guix/git.scm:387:0:commit-closure 0.00 1.15 0.00 guix/git.scm:401:0:commit-difference 0.00 1.05 0.00 srfi/srfi-1.scm:530:0:unfold 0.00 0.96 0.00 git/commit.scm:198:4 0.00 0.82 0.00 srfi/srfi-1.scm:452:2:fold 0.00 0.82 0.00 /home/ludo/src/guix/build-aux/git-authenticate.scm:356:0:authenticate-commits 0.00 0.82 0.00 guix/progress.scm:65:0:call-with-progress-reporter 0.00 0.73 0.00 git/commit.scm:197:14 0.00 0.62 0.00 /home/ludo/src/guix/build-aux/git-authenticate.scm:346:10 0.00 0.20 0.00 guix/openpgp.scm:987:0:get-openpgp-keyring 0.00 0.15 0.00 guix/openpgp.scm:610:0:get-signature 0.00 0.11 0.00 ice-9/format.scm:39:0:format 0.00 0.09 0.00 anon #x495690 0.00 0.09 0.00 anon #x497d54 0.00 0.09 0.00 git/types.scm:83:0 0.00 0.07 0.00 guix/openpgp.scm:614:2:get-sig 0.00 0.07 0.00 gcrypt/pk-crypto.scm:103:4 0.00 0.05 0.00 ice-9/rdelim.scm:193:0:read-line 0.00 0.05 0.00 guix/openpgp.scm:845:0:get-public-key 0.00 0.05 0.00 ice-9/format.scm:759:2:format:out-obj-padded 0.00 0.05 0.00 ice-9/format.scm:113:2:format:format-work 0.00 0.04 0.00 guix/sets.scm:84:0:set-insert 0.00 0.02 0.00 git/types.scm:106:0:make-double-pointer 0.00 0.02 0.00 ice-9/ports.scm:545:0:call-with-output-string --- Sample count: 108 Total time: 1.963880152 seconds (0.382282758 seconds in GC) --8<---cut here---end--->8--- We can make that a bit faster by not loading the keyring when there’s nothing to do, and by storing keys in binary format instead of ASCII-armored, if needed. Thanks, Ludo’.
Re: Updating the “pre-push” Git hook
Hi! Vagrant Cascadian skribis: > On 2020-05-24, Ludovic Courtès wrote: >> Efraim Flashner skribis: >>> On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: Hello Guix! I think we should change our pre-push hook as shown below. Thoughts? > ... >>> (ins)efraim@E5400 ~$ type -P make >>> (ins)efraim@E5400 ~$ command -v make >>> >>> I'd need to run 'guix environment --ad-hoc make -- git push' >> >> You’d need to run ‘git push’ from a full Guix development environment. >> Do you think it could be a problem? > > Wait a minute... you're saying this is something that needs to be > configured on each committer's machine(s)? > > Shouldn't it be on the server-side recieve hooks instead, otherwise > someone might accidentally (or intentially) push commits not > appropriately signed to the repository or validated by this check... > > Or is this an optional check for recommended for committers? Have I been > missing something all along that I was supposed to be doing? It should be a server-side check so we don’t shoot ourselves in the foot. However, it’s not done yet (but hey, the code is not even a month old :-)), so in the meantime, this hook will be very strongly recommended. Making this a server-side hook on Savannah will be challenging since “we” don’t have direct access to Savannah. That makes me wonder if we should have a push server say on berlin, and make Savannah mirror it or something. Help welcome! > For my own workflow, I usually do not (yet) sign or push commits from a > machine with guix installed... it's a bit awkward, admittedly, but I > don't yet have any SSH or OpenPGP keys I trust guix with directly > (ironically, "make authenticate" is working towards addressing exactly > that trust issue). Heh. :-) Thanks, Ludo’.
Re: Updating the “pre-push” Git hook
Efraim Flashner skribis: > I'd probably run 'guix environment guix -- git push origin master' and > view it as an additional safe guard to not push to the wrong branch or > something, similar to how I view the password on the key. I bet there's > an option to create a repo-specific alias in .git/config so that 'git > push' will run inside 'guix environment guix'. OK. > I'm not convinced that my case is unique or that it should hold back the > change. How does it work if 'make authenticate' fails? ‘git push’ fails, just like with the current pre-push hook. Ludo’.
Re: Updating the “pre-push” Git hook
On 2020-05-24, Ludovic Courtès wrote: > Efraim Flashner skribis: >> On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: >>> Hello Guix! >>> >>> I think we should change our pre-push hook as shown below. >>> >>> Thoughts? ... >> (ins)efraim@E5400 ~$ type -P make >> (ins)efraim@E5400 ~$ command -v make >> >> I'd need to run 'guix environment --ad-hoc make -- git push' > > You’d need to run ‘git push’ from a full Guix development environment. > Do you think it could be a problem? Wait a minute... you're saying this is something that needs to be configured on each committer's machine(s)? Shouldn't it be on the server-side recieve hooks instead, otherwise someone might accidentally (or intentially) push commits not appropriately signed to the repository or validated by this check... Or is this an optional check for recommended for committers? Have I been missing something all along that I was supposed to be doing? For my own workflow, I usually do not (yet) sign or push commits from a machine with guix installed... it's a bit awkward, admittedly, but I don't yet have any SSH or OpenPGP keys I trust guix with directly (ironically, "make authenticate" is working towards addressing exactly that trust issue). live well, vagrant signature.asc Description: PGP signature
Re: Updating the “pre-push” Git hook
Ludovic Courtès writes: > Could you try: > > mv ~/.cache/guix/authentication/channels/guix{,.bak} > time make authenticate > mv ~/.cache/guix/authentication/channels/guix{.bak,} > > ? real0m49.496s user0m43.733s sys 0m1.658s And then running it again: --8<---cut here---start->8--- $ [env] time make authenticate Authenticating Git checkout... Authenticating d68de95 to fb1675e (0 commits)... real0m2.692s user0m2.877s sys 0m0.128s $ [env] time make authenticate Authenticating Git checkout... Authenticating d68de95 to fb1675e (0 commits)... real0m2.754s user0m2.939s sys 0m0.111s --8<---cut here---end--->8--- -- Ricardo
Re: Updating the “pre-push” Git hook
On Sun, May 24, 2020 at 11:45:34PM +0200, Ludovic Courtès wrote: > Hi, > > Efraim Flashner skribis: > > > On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: > >> Hello Guix! > >> > >> I think we should change our pre-push hook as shown below. > >> > >> Thoughts? > >> > >> Thanks, > >> Ludo’. > >> > > > > (ins)efraim@E5400 ~$ type -P make > > (ins)efraim@E5400 ~$ command -v make > > > > I'd need to run 'guix environment --ad-hoc make -- git push' > > You’d need to run ‘git push’ from a full Guix development environment. > Do you think it could be a problem? I'd probably run 'guix environment guix -- git push origin master' and view it as an additional safe guard to not push to the wrong branch or something, similar to how I view the password on the key. I bet there's an option to create a repo-specific alias in .git/config so that 'git push' will run inside 'guix environment guix'. I'm not convinced that my case is unique or that it should hold back the change. How does it work if 'make authenticate' fails? -- Efraim Flashner אפרים פלשנר GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted signature.asc Description: PGP signature
Re: Updating the “pre-push” Git hook
Hi, Efraim Flashner skribis: > On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: >> Hello Guix! >> >> I think we should change our pre-push hook as shown below. >> >> Thoughts? >> >> Thanks, >> Ludo’. >> > > (ins)efraim@E5400 ~$ type -P make > (ins)efraim@E5400 ~$ command -v make > > I'd need to run 'guix environment --ad-hoc make -- git push' You’d need to run ‘git push’ from a full Guix development environment. Do you think it could be a problem? Thanks, Ludo’.
Re: Updating the “pre-push” Git hook
Hi, Ricardo Wurmus skribis: > Leo Famulari writes: > >> On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: >>> Hello Guix! >>> >>> I think we should change our pre-push hook as shown below. >>> >>> Thoughts? >> >> Is it fast? :) > > This depends on how many commits you have previously authenticated. But > even the slow case is pretty fast. I just authenticated 16,290 commits > in less than a minute. Regular contributors won’t have to authenticate > nearly as many commits. > > For 0 commits it takes 4 seconds. Hmm for me it’s more like 0 to 1 second, and it’s ~20s to authenticate 14K commits: https://issues.guix.gnu.org/issue/22883#61 Could you try: mv ~/.cache/guix/authentication/channels/guix{,.bak} time make authenticate mv ~/.cache/guix/authentication/channels/guix{.bak,} ? Anyway, for normal usage, it should be faster than the shell script. Also, it performs a different job: the shell script would only check whether a commit is signed at all. Well, give it a try and lemme know! Ludo’.
Re: Updating the “pre-push” Git hook
Leo Famulari writes: > On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: >> Hello Guix! >> >> I think we should change our pre-push hook as shown below. >> >> Thoughts? > > Is it fast? :) This depends on how many commits you have previously authenticated. But even the slow case is pretty fast. I just authenticated 16,290 commits in less than a minute. Regular contributors won’t have to authenticate nearly as many commits. For 0 commits it takes 4 seconds. -- Ricardo
Re: Updating the “pre-push” Git hook
On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: > Hello Guix! > > I think we should change our pre-push hook as shown below. > > Thoughts? > > Thanks, > Ludo’. > (ins)efraim@E5400 ~$ type -P make (ins)efraim@E5400 ~$ command -v make I'd need to run 'guix environment --ad-hoc make -- git push' -- Efraim Flashner אפרים פלשנר GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted signature.asc Description: PGP signature
Re: Updating the “pre-push” Git hook
On Fri, May 22, 2020 at 10:44:48PM +0200, Ludovic Courtès wrote: > Hello Guix! > > I think we should change our pre-push hook as shown below. > > Thoughts? Is it fast? :)