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

["Ludovic Courtès" <ludo@gnu.org>]

Hi!

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:


[...]

>>>  (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:

[...]

> +  (define (absolute? file)
> +    (string-prefix? "/" file))
> +
> +  (if (or (every absolute? (list file1 file2))
> +          (every (negate absolute?) (list file1 file2)))

Yes, that could work.

>> 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 :-).

Sure, but it’s always easier to reason about code that is stricter.

>>> +(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.

Oh right!

>> 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.

Actually I don’t have a good suggestion.  :-)
‘strip-child-directories’ maybe?

Thanks,
Ludo’.