Subject: Re: [bug#32740] [PATCH] git-download: Don't assume the git checkout is the parent of ".git".

From: Marius Bakke <[email-masked]>
Date: Sun, 23 Sep 2018 15:13:09 +0200

[email-masked] (Ludovic Courtès) writes:

> Hello Marius!
>
> Marius Bakke <[email-masked]> skribis:
>
>> This makes it play nicely with worktrees.
>>
>> * guix/git-download.scm (git-file-list): Use REPOSITORY-WORKING-DIRECTORY to
>> locate checkout.  Rename from "top" to "workdir".
>> ---
>>
>> Notes:
>>     Guix,
>>     
>>     This fixes (current-guix) for me in a worktree.  Testing needed on other git
>>     setups!
>>
>>  guix/git-download.scm | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/guix/git-download.scm b/guix/git-download.scm
>> index 24cf11be5..a7c8173f4 100644
>> --- a/guix/git-download.scm
>> +++ b/guix/git-download.scm
>> @@ -158,19 +158,22 @@ also includes directories, not just regular files.  The returned file names
>>  are relative to DIRECTORY, which is not necessarily the root of the checkout."
>>    (let* ((directory  (canonicalize-path directory))
>>           (dot-git    (repository-discover directory))
>> -         (top        (dirname dot-git))
>>           (repository (repository-open dot-git))
>> +         (workdir    (canonicalize-path
>> +                      ;; XXX: This variable is mistakenly private in Guile-Git 0.1.0.
>> +                      ((@@ (git repository) repository-working-directory)
>> +                       repository)))
>
> I think we can avoid the call to ‘canonicalize-path’ here, can’t we?
> It’s costly, and since we did it just above, it shouldn’t be needed
> here.

You're right.  I mostly needed it because (repository-working-directory)
includes a trailing slash.  This behaviour seems to be consistent, so I
managed to simplify the code further by assuming that is the case:

From e8b443e1de0a5b1e3dfeee024cd0625790f4f834 Mon Sep 17 00:00:00 2001
From: Marius Bakke <[email-masked]>
Date: Sat, 15 Sep 2018 11:53:40 +0200
Subject: [PATCH] git-download: Don't assume the working directory is the
 parent of ".git".

* guix/git-download.scm (git-file-list): Use REPOSITORY-WORKING-DIRECTORY to
locate checkout.  Rename from "top" to "workdir".
---
 guix/git-download.scm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/guix/git-download.scm b/guix/git-download.scm
index 24cf11be5..eb20927c7 100644
--- a/guix/git-download.scm
+++ b/guix/git-download.scm
@@ -156,22 +156,21 @@ HASH-ALGO (a symbol).  Use NAME as the file name, or a generic name if #f."
 The result is similar to that of the 'git ls-files' command, except that it
 also includes directories, not just regular files.  The returned file names
 are relative to DIRECTORY, which is not necessarily the root of the checkout."
-  (let* ((directory  (canonicalize-path directory))
+  (let* ((directory  (string-append (canonicalize-path directory) "/"))
          (dot-git    (repository-discover directory))
-         (top        (dirname dot-git))
          (repository (repository-open dot-git))
+         ;; XXX: This procedure is mistakenly private in Guile-Git 0.1.0.
+         (workdir    ((@@ (git repository) repository-working-directory)
+                      repository))
          (head       (repository-head repository))
          (oid        (reference-target head))
          (commit     (commit-lookup repository oid))
          (tree       (commit-tree commit))
          (files      (tree-list tree)))
     (repository-close! repository)
-    (if (string=? top directory)
+    (if (string=? workdir directory)
         files
-        (let ((relative (string-append
-                         (string-drop directory
-                                      (+ 1 (string-length top)))
-                         "/")))
+        (let ((relative (string-drop directory (string-length workdir))))
           (filter-map (lambda (file)
                         (and (string-prefix? relative file)
                              (string-drop file (string-length relative))))
-- 
2.19.0

WDYT?