Re: [PATCH 0/1] Go importer

[Leo Famulari]

On Mon, Jun 04, 2018 at 10:18:53AM +0200, Pierre-Antoine Rouby wrote:
> Attached an update of importer.

Thanks! I'm sorry my response is so late.

> From 3e9b4fa12811432fdd7a4d6330f9093dcc72d25a Mon Sep 17 00:00:00 2001
> From: Rouby Pierre-Antoine <address@hidden>
> Date: Thu, 26 Apr 2018 15:05:23 +0200
> Subject: [PATCH] import: Add gopkg importer.
> 
> * guix/import/gopkg.scm: New file.
> * guix/scripts/import/gopkg.scm: New file.
> * guix/scripts/import.scm: Add 'gopkg'.
> * Makefile.am: Add 'gopkg' importer in modules list.

I wonder which of the new files needs to be added to Makefile.am? My
Autotools knowledge is not very strong...

> +(define (get-new-tmp-dir)
> +  "Return new temp directory."
> +  (let ((tmp "/tmp/guix-import-gopkg"))
> +    (define (new num)
> +      (let ((new-dir (string-append tmp "-" (number->string num))))
> +        (if (file-exists? new-dir)
> +            (new (+ num 1))
> +            new-dir)))
> +    (if (file-exists? tmp)
> +        (new 0)
> +        tmp)))
> +
> +(define tmp-dir (get-new-tmp-dir))

I noticed a couple issues with this code. First, the names of the
temporary directories are predictable (they use an incrementing
integer). Second, the temporary files are not deleted after the importer
runs. I've attached a modified patch that addresses this by using ((guix
utils) call-with-temporary-directory), which should address these
problems. [0]

> +(define* (git-fetch url commit directory
> +                    #:key (git-command "git") recursive?)
> +  "Fetch COMMIT from URL into DIRECTORY.  COMMIT must be a valid Git commit
> +identifier.  When RECURSIVE? is true, all the sub-modules of URL are fetched,
> +recursively.  Return #t on success, #f otherwise."
> +  ;; Disable TLS certificate verification.  The hash of the checkout is known
> +  ;; in advance anyway.
> +  (setenv "GIT_SSL_NO_VERIFY" "true")

My patch turns certificate verification back on. When importing, the
hash of the checkout is not known in advance.

And finally I added some brief documentation to the manual. Maybe there
could be further clean-up and code deduplication with other parts of the
Guix codebase, but I think it's better to have this importer in Guix
now.

What do you think of my patch? Does it still work for you?

[0] Actually, the temp directories will not be cleaned up due to
<https://bugs.gnu.org/32126>, but I think this will eventually be fixed.

0001-import-Add-gopkg-importer.patch
Description: Text document

signature.asc
Description: PGP signature