[bug#49149] [PATCH 0/7] Add deb format for guix pack.

[Maxim Cournoyer]

Hey,

Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>  (define (file-prefix? file1 file2)
>> -  "Return #t if FILE1 denotes the name of a file that is a parent of FILE2,
>> -where both FILE1 and FILE2 are absolute file name.  For example:
>> +  "Return #t if FILE1 denotes the name of a file that is a parent of FILE2.
>> +For example:
>>  
>>    (file-prefix? \"/gnu\" \"/gnu/store\")
>>    => #t
>> @@ -240,19 +241,41 @@ where both FILE1 and FILE2 are absolute file name.  
>> For example:
>>    (file-prefix? \"/gn\" \"/gnu/store\")
>>    => #f
>>  "
>> -  (and (string-prefix? "/" file1)
>> -       (string-prefix? "/" file2)
>
> Doesn’t it have the effect that now:
>
>   (file-prefix? "gnu" "/gnu/store") => #t
>
> ?

Good catch.  That seems sub-optimal.  How about:

--8<---------------cut here---------------start------------->8---
modified   gnu/system/file-systems.scm
@@ -233,6 +233,8 @@
 
 (define (file-prefix? file1 file2)
   "Return #t if FILE1 denotes the name of a file that is a parent of FILE2.
+FILE1 and FILE2 must both be either absolute or relative, else #f is returned.
+
 For example:
 
   (file-prefix? \"/gnu\" \"/gnu/store\")
@@ -241,17 +243,24 @@ For example:
   (file-prefix? \"/gn\" \"/gnu/store\")
   => #f
 "
-  (let loop ((file1 (string-tokenize file1 %not-slash))
-             (file2 (string-tokenize file2 %not-slash)))
-    (match file1
-      (()
-       #t)
-      ((head1 tail1 ...)
-       (match file2
-         ((head2 tail2 ...)
-          (and (string=? head1 head2) (loop tail1 tail2)))
-         (()
-          #f))))))
+  (define (absolute? file)
+    (string-prefix? "/" file))
+
+  (if (or (every absolute? (list file1 file2))
+          (every (negate absolute?) (list file1 file2)))
+      (let loop ((file1 (string-tokenize file1 %not-slash))
+                 (file2 (string-tokenize file2 %not-slash)))
+        (match file1
+          (()
+           #t)
+          ((head1 tail1 ...)
+           (match file2
+             ((head2 tail2 ...)
+              (and (string=? head1 head2) (loop tail1 tail2)))
+             (()
+              #f)))))
+      ;; FILE1 and FILE2 are a mix of absolute and relative paths.
+      #f))
--8<---------------cut here---------------end--------------->8---
 
 (define (file-name-depth file-name)
   (length (string-tokenize file-name %not-slash)))

> I’d rather insist on absolute file names and preserve the initial
> semantics, to avoid bad surprises.

I agree that not changing the original semantics would be safest;
nevertheless, we're talking about an internal helper that isn't widely
use; its couple usages are easy to review (and deals with mount points
which seems safe to assume are exclusively using absolute paths).
Especially after the above fix :-).

>> +(define (reduce-directories file-names)
>> +  "Eliminate entries in FILE-NAMES that are children of other entries in
>> +FILE-NAMES.  This is for example useful when passing a list of files to GNU
>> +tar, which would otherwise descend into each directory passed and archive 
>> the
>> +duplicate files as hard links, which can be undesirable."
>> +  (let* ((file-names/sorted
>> +          ;; Ascending sort by file hierarchy depth, then by file name 
>> length.
>> +          (stable-sort (delete-duplicates file-names)
>> +                       (lambda (f1 f2)
>> +                         (let ((depth1 (file-name-depth f1))
>> +                               (depth2 (file-name-depth f2)))
>> +                           (if (= depth1 depth2)
>> +                               (string< f1 f2)
>> +                               (< depth1 depth2)))))))
>> +    (reverse (fold (lambda (file-name results)
>> +                     (if (find (cut file-prefix? <> file-name) results)
>> +                         results        ;parent found -- skipping
>> +                         (cons file-name results)))
>> +                   '()
>> +                   file-names/sorted))))
>
> Likewise, I suspect it doesn’t work as intended if there are relative
> file names in the list, no?

You can see it at work in the tests/file-systems test module; it reduces

(reduce-directories '("./opt/gnu/etc" "./opt/gnu/" "./opt/gnu/bin"
                        "./opt/gnu/lib/debug" "./opt/gnuism" "a/b/c"
                        "a/b/c"))

into '("./opt/gnu/" "./opt/gnuism" "a/b/c"), none of which are absolute
file names.

> Perhaps we could add an example to the docstring.  Also, the word
> “reduce” doesn’t appear in the docstring, which to me suggests
> suboptimal naming.  ;-)

That the word 'reduce' doesn't appear in the docstring was a conscious
effort of mine to not bore the reader with repeating the same terms, ah!
But naming is hard; I'm open to suggestions.

Maxim