[bug#28281] [PATCH] gnu: Add os-prober.

[bug#28281] [PATCH] gnu: Add os-prober.

[Arun Isaac]

Here's an updated patch.

> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (define (find-files-non-recursive directory)
> +               (find-files directory
> +                           (lambda (file stat)
> +                             (string-match (string-append "^" directory "/[^/]*$")
> +                                           file))
> +                           #:directories? #t))

Do you think it would be a good idea to add a #:recursive? keyword
argument to `find-files' in (guix build utils), instead of creating this
`find-files-non-recursive-function' here?

> +             (let* ((out (assoc-ref outputs "out"))
> +                    (bin (string-append out "/bin"))
> +                    (lib (string-append out "/lib"))
> +                    (share (string-append out "/share")))
> +               (for-each (cut install-file <> bin)
> +                         (list "linux-boot-prober" "os-prober"))
> +               (install-file "newns" (string-append lib "/os-prober"))
> +               (install-file "common.sh" (string-append share "/os-prober"))
> +               (install-file "os-probes/mounted/powerpc/20macosx"
> +                             (string-append lib "/os-probes/mounted"))
> +               (for-each
> +                (lambda (directory)
> +                  (for-each
> +                   (lambda (file)
> +                     (let ((destination (string-append lib "/" directory
> +                                                       "/" (basename file))))
> +                       (mkdir-p (dirname destination))
> +                       (copy-recursively file destination)))
> +                   (append (find-files-non-recursive (string-append directory "/common"))
> +                           (find-files-non-recursive (string-append directory "/x86")))))
> +                (list "os-probes" "os-probes/mounted" "os-probes/init"
> +                      "linux-boot-probes" "linux-boot-probes/mounted"))
> +               #t))))))

I have used `find-files' instead of `ftw', as you suggested. The install
procedure is clearer now. but still remains quite messy. It is not
simple enough to separate the "candidate-files" and "install-file"
logic.

WDYT? Will the above code do?

[bug#28281] [PATCH] gnu: Add os-prober.

[Ludovic Courtès]

Heya,

Arun Isaac <arunisaac@systemreboot.net> skribis:

> Here's an updated patch.
>
>> +         (replace 'install
>> +           (lambda* (#:key outputs #:allow-other-keys)
>> +             (define (find-files-non-recursive directory)
>> +               (find-files directory
>> +                           (lambda (file stat)
>> +                             (string-match (string-append "^" directory "/[^/]*$")
>> +                                           file))
>> +                           #:directories? #t))
>
> Do you think it would be a good idea to add a #:recursive? keyword
> argument to `find-files' in (guix build utils), instead of creating this
> `find-files-non-recursive-function' here?

Hmm I didn’t understand that it *had* to be non-recursive.  Does it
really make a difference?

If it does, then ‘scandir’ from (ice-9 ftw) would be the thing.  Sorry
if I led you in the wrong direction.  :-/

>> +             (let* ((out (assoc-ref outputs "out"))
>> +                    (bin (string-append out "/bin"))
>> +                    (lib (string-append out "/lib"))
>> +                    (share (string-append out "/share")))
>> +               (for-each (cut install-file <> bin)
>> +                         (list "linux-boot-prober" "os-prober"))
>> +               (install-file "newns" (string-append lib "/os-prober"))
>> +               (install-file "common.sh" (string-append share "/os-prober"))
>> +               (install-file "os-probes/mounted/powerpc/20macosx"
>> +                             (string-append lib "/os-probes/mounted"))
>> +               (for-each
>> +                (lambda (directory)
>> +                  (for-each
>> +                   (lambda (file)
>> +                     (let ((destination (string-append lib "/" directory
>> +                                                       "/" (basename file))))
>> +                       (mkdir-p (dirname destination))
>> +                       (copy-recursively file destination)))
>> +                   (append (find-files-non-recursive (string-append directory "/common"))
>> +                           (find-files-non-recursive (string-append directory "/x86")))))
>> +                (list "os-probes" "os-probes/mounted" "os-probes/init"
>> +                      "linux-boot-probes" "linux-boot-probes/mounted"))
>> +               #t))))))
>
> I have used `find-files' instead of `ftw', as you suggested. The install
> procedure is clearer now. but still remains quite messy. It is not
> simple enough to separate the "candidate-files" and "install-file"
> logic.
>
> WDYT? Will the above code do?

Sure.  I mean, we should try our best to make things clearer, while not
losing our hairs on it.  Sometimes it just has to be messy.

Thanks,
Ludo’.

[bug#28281] [PATCH] gnu: Add os-prober.

[Arun Isaac]

>>> +         (replace 'install
>>> +           (lambda* (#:key outputs #:allow-other-keys)
>>> +             (define (find-files-non-recursive directory)
>>> +               (find-files directory
>>> +                           (lambda (file stat)
>>> +                             (string-match (string-append "^" directory "/[^/]*$")
>>> +                                           file))
>>> +                           #:directories? #t))
>>
>> Do you think it would be a good idea to add a #:recursive? keyword
>> argument to `find-files' in (guix build utils), instead of creating this
>> `find-files-non-recursive-function' here?
>
> Hmm I didn’t understand that it *had* to be non-recursive.  Does it
> really make a difference?

I am trying to do

cp -r /some/directory/* destination

To do this, I used `find-files-non-recursive' to get all files in
/some/directory/ and applied `copy-recursively' on each one of them.

Do you have a better way of doing this in mind?

> If it does, then ‘scandir’ from (ice-9 ftw) would be the thing.  Sorry
> if I led you in the wrong direction.  :-/

Yes, scandir could be used. But, it also returns "." and "..", and I'll
have to filter them out. So code length, or clarity-wise, it won't be
much of an improvement. That is why I thought adding a #:recursive?
argument to `find-files' would be nice.

>>> +             (let* ((out (assoc-ref outputs "out"))
>>> +                    (bin (string-append out "/bin"))
>>> +                    (lib (string-append out "/lib"))
>>> +                    (share (string-append out "/share")))
>>> +               (for-each (cut install-file <> bin)
>>> +                         (list "linux-boot-prober" "os-prober"))
>>> +               (install-file "newns" (string-append lib "/os-prober"))
>>> +               (install-file "common.sh" (string-append share "/os-prober"))
>>> +               (install-file "os-probes/mounted/powerpc/20macosx"
>>> +                             (string-append lib "/os-probes/mounted"))
>>> +               (for-each
>>> +                (lambda (directory)
>>> +                  (for-each
>>> +                   (lambda (file)
>>> +                     (let ((destination (string-append lib "/" directory
>>> +                                                       "/" (basename file))))
>>> +                       (mkdir-p (dirname destination))
>>> +                       (copy-recursively file destination)))
>>> +                   (append (find-files-non-recursive (string-append directory "/common"))
>>> +                           (find-files-non-recursive (string-append directory "/x86")))))
>>> +                (list "os-probes" "os-probes/mounted" "os-probes/init"
>>> +                      "linux-boot-probes" "linux-boot-probes/mounted"))
>>> +               #t))))))

[bug#28281] [PATCH] gnu: Add os-prober.

[Ludovic Courtès]

Arun Isaac <arunisaac@systemreboot.net> skribis:

>>>> +         (replace 'install
>>>> +           (lambda* (#:key outputs #:allow-other-keys)
>>>> +             (define (find-files-non-recursive directory)
>>>> +               (find-files directory
>>>> +                           (lambda (file stat)
>>>> +                             (string-match (string-append "^" directory "/[^/]*$")
>>>> +                                           file))
>>>> +                           #:directories? #t))
>>>
>>> Do you think it would be a good idea to add a #:recursive? keyword
>>> argument to `find-files' in (guix build utils), instead of creating this
>>> `find-files-non-recursive-function' here?
>>
>> Hmm I didn’t understand that it *had* to be non-recursive.  Does it
>> really make a difference?
>
> I am trying to do
>
> cp -r /some/directory/* destination
>
> To do this, I used `find-files-non-recursive' to get all files in
> /some/directory/ and applied `copy-recursively' on each one of them.
>
> Do you have a better way of doing this in mind?

Would (copy-recursively "/some/directory" destination) work for you?

>> If it does, then ‘scandir’ from (ice-9 ftw) would be the thing.  Sorry
>> if I led you in the wrong direction.  :-/
>
> Yes, scandir could be used. But, it also returns "." and "..", and I'll
> have to filter them out. So code length, or clarity-wise, it won't be
> much of an improvement. That is why I thought adding a #:recursive?
> argument to `find-files' would be nice.

Yeah, looks like none of the options is a perfect match.

HTH,
LUdo’.

[bug#28281] [PATCH] gnu: Add os-prober.

[Arun Isaac]

>> I am trying to do
>>
>> cp -r /some/directory/* destination
>>
>> To do this, I used `find-files-non-recursive' to get all files in
>> /some/directory/ and applied `copy-recursively' on each one of them.
>>
>> Do you have a better way of doing this in mind?
>
> Would (copy-recursively "/some/directory" destination) work for you?

No, it wouldn't. That would recreate /some/directory at the
destination. I only want the files inside /some/directory to be copied,
not /some/directory itself.

[bug#28281] [PATCH] gnu: Add os-prober.

[Ludovic Courtès]

Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> skribis:

>>> I am trying to do
>>>
>>> cp -r /some/directory/* destination
>>>
>>> To do this, I used `find-files-non-recursive' to get all files in
>>> /some/directory/ and applied `copy-recursively' on each one of them.
>>>
>>> Do you have a better way of doing this in mind?
>>
>> Would (copy-recursively "/some/directory" destination) work for you?
>
> No, it wouldn't. That would recreate /some/directory at the
> destination. I only want the files inside /some/directory to be copied,
> not /some/directory itself.

Ah sorry.  Then yeah, either ‘scandir’ or ‘find-files’, whichever you
find is the least cumbersome.

Feel free to push something along these lines!

Ludo’.

[bug#28281] [PATCH] gnu: Add os-prober.

[Arun Isaac]

>>> Would (copy-recursively "/some/directory" destination) work for you?
>>
>> No, it wouldn't. That would recreate /some/directory at the
>> destination. I only want the files inside /some/directory to be copied,
>> not /some/directory itself.
>
> Ah sorry.  Then yeah, either ‘scandir’ or ‘find-files’, whichever you
> find is the least cumbersome.
>
> Feel free to push something along these lines!

Not sure I follow you. Should I

1. push the patch I sent most recently

OR

2. or add a #:recursive? argument to `find-files', and push a patch
which uses this new `find-files'?

IMO, approach 2 is a better idea, though it could be that we are adding
too many keyword arguments to `find-files'.

WDYT?

[bug#28281] [PATCH] gnu: Add os-prober.

[Ludovic Courtès]

Arun Isaac <arunisaac@systemreboot.net> skribis:

>>>> Would (copy-recursively "/some/directory" destination) work for you?
>>>
>>> No, it wouldn't. That would recreate /some/directory at the
>>> destination. I only want the files inside /some/directory to be copied,
>>> not /some/directory itself.
>>
>> Ah sorry.  Then yeah, either ‘scandir’ or ‘find-files’, whichever you
>> find is the least cumbersome.
>>
>> Feel free to push something along these lines!
>
> Not sure I follow you. Should I
>
> 1. push the patch I sent most recently
>
> OR
>
> 2. or add a #:recursive? argument to `find-files', and push a patch
> which uses this new `find-files'?
>
> IMO, approach 2 is a better idea, though it could be that we are adding
> too many keyword arguments to `find-files'.
>
> WDYT?

I’m for approach #1, go for it!  :-)

Approach #2 would take a full rebuild, and it would make ‘find-files’
equivalent to ‘scandir’, so not worth it IMO.

Sorry for being unclear!

Ludo’.

bug#28281: [PATCH] gnu: Add os-prober.

[Arun Isaac]

>> Should I
>>
>> 1. push the patch I sent most recently
>>
>> OR
>>
>> 2. or add a #:recursive? argument to `find-files', and push a patch
>> which uses this new `find-files'?
>>
>> IMO, approach 2 is a better idea, though it could be that we are adding
>> too many keyword arguments to `find-files'.
>>
>> WDYT?
>
> I’m for approach #1, go for it!  :-)

Ok, pushed!