From debbugs-submit-bounces@debbugs.gnu.org Tue Dec 10 09:28:20 2019 Received: (at 38320) by debbugs.gnu.org; 10 Dec 2019 14:28:20 +0000 Received: from localhost ([127.0.0.1]:55116 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iegUu-0004VF-Ed for submit@debbugs.gnu.org; Tue, 10 Dec 2019 09:28:20 -0500 Received: from eggs.gnu.org ([209.51.188.92]:45534) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iegUt-0004V3-5U for 38320@debbugs.gnu.org; Tue, 10 Dec 2019 09:28:19 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]:52614) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1iegUn-0005oM-Rp; Tue, 10 Dec 2019 09:28:13 -0500 Received: from [2001:660:6102:320:e120:2c8f:8909:cdfe] (port=48382 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1iegUm-0006wQ-MI; Tue, 10 Dec 2019 09:28:13 -0500 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Mathieu Othacehe Subject: Re: bug#38320: Cuirass: Allow to use authenticated Git repositories as inputs References: <875zjc8ciz.fsf@lassieur.org> <878so4t6mk.fsf@gmail.com> <87r21v9cmi.fsf@gnu.org> <87h829sb73.fsf@gmail.com> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 20 Frimaire an 228 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Tue, 10 Dec 2019 15:28:09 +0100 In-Reply-To: <87h829sb73.fsf@gmail.com> (Mathieu Othacehe's message of "Mon, 09 Dec 2019 17:41:52 +0100") Message-ID: <877e34z24m.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 38320 Cc: 38320@debbugs.gnu.org, Erik Edrosa , =?utf-8?Q?Cl=C3=A9ment?= Lassieur X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hi! Mathieu Othacehe skribis: > Here's a patch that add support for ssh authenticated repositories in > "clone" and "remote-fetch" methods of Guile-Git. Woow, awesome! > At first, I used Guile-SSH in the tests to start an SSH server, but as > "make-server" call of Guile-SSH is really low level, this is not very > realistic. I just ended up with a half-broken ssh server, poorly > implemented, after (too many hours) spent reading ssh dumps. Oh, I thought it=E2=80=99d be easier to scrap bits from the example SSH ser= ver that=E2=80=99s in Guile-SSH, perhaps a wishlist item for them. > So the strategy is to spawn an openssh server for the tests. It seems to > work alright, using key based or ssh-agent authentication. Anyway, if it works with sshd, that=E2=80=99s great. > From ae3c5a9851b02e78096963616d4e2f999119fc4d Mon Sep 17 00:00:00 2001 > From: Mathieu Othacehe > Date: Mon, 9 Dec 2019 16:16:45 +0100 > Subject: [PATCH] Add ssh authentication support. > > * Makefile.am (SOURCES): Add git/auth.scm, > (TESTS): add tests/clone.scm. > * configure.ac: Check for git and ssh binaries. > * git.scm (%public-modules): Add (git auth) and (git bindings). > * git/auth.scm: New file. > * git/clone.scm (clone): Add an auth-method argument. Pass it to > new init-fetch-options call, before proceeding to clone. > * git/remote.scm (remote-fetch): Add an auth-method. Pass it to > init-fetch-options before proceeding to fetch. > * git/structs.scm (clone-options-fetch-options): Do not return a copy of > fetch-options nested inside clone-options. Instead, find the offset of > fetch-options and use it to create a pointer to fetch-options. > * git/fetch.scm (init-fetch-options): New exported procedure, > (make-fetch-options): call the procedure above to initialize fetch-option= s, > (set-fetch-auth-with-ssh-agent!): handle the case where username is not s= et > and libgit2 asks for one. > (set-fetch-auth-with-default-ssh-key!): remove this procedure, > (set-fetch-auth-with-ssh-key): new procedure. > * tests/.ssh/id_rsa_client: New file. > * tests/.ssh/id_rsa_client.pub: New file. > * tests/.ssh/id_rsa_server: New file. > * tests/clone.scm: New file. > * tests/ssh.scm.in: New file. [...] > (define-module (git fetch) > #:use-module (system foreign) > + #:use-module (git auth) > #:use-module (git bindings) > #:use-module (git cred) > #:use-module (git structs) > #:use-module (git types) > #:use-module (srfi srfi-26) >=20=20 > - #:export (make-fetch-options > + #:export (init-fetch-options > + make-fetch-options I think we should keep =E2=80=98init-fetch-options=E2=80=99 private. > fetch-init-options ;deprecated! =E2=80=98init-fetch-options=E2=80=99, =E2=80=98fetch-init-options=E2=80=99,= hmm=E2=80=A6 o_O > new file mode 100644 > index 0000000..7e16000 > --- /dev/null > +++ b/tests/.ssh/id_rsa_client I wonder if we should generate those upon =E2=80=98make check=E2=80=99. Th= oughts? (It shouldn=E2=80=99t be a blocker though.) > +(with-sshd-server ssh-server-port > + (with-repository "simple-bare" directory > + (test-equal "clone-auth-ssh-credentials" > + "3f848a1a52416ac99a5c5bf2e6bd55eb7b99d55b" > + (clone-test directory (make-client-ssh-auth)))) > + > + (with-repository "simple-bare" directory > + (test-equal "clone-auth-ssh-agent" > + "3f848a1a52416ac99a5c5bf2e6bd55eb7b99d55b" > + (with-ssh-agent > + (clone-test directory (%make-auth-ssh-agent))))) > + > + (with-repository "simple-bare" directory > + (test-assert "clone-and-fetch-auth-ssh-credentials" > + (let* ((auth (make-client-ssh-auth)) > + (do-clone (clone-test directory auth)) > + (clone-dir (in-vicinity directory "out")) > + (repository (repository-open clone-dir)) > + (remote (remote-lookup repository "origin"))) > + (remote-fetch remote #:auth-method auth) > + #t)))) I think we should add something like: (define (sshd-available?) ;; Return #t if sshd is available (it does not support ;; =E2=80=98--version=E2=80=99 or anything similar though). (not (=3D 127 (system* sshd "--something-not-supported")))) (unless (sshd-available?) (test-skip 1)) ;; =E2=80=A6 Apart from this detail, it looks great to me! You have push access, right? Speaking of which, we really need to push a release at some point. Erik, would you be available to do that, or would you like to delegate? Thanks, Ludo=E2=80=99.