[bug#50072] [PATCH WIP 0/4] Add upstream updater for git-fetch origins.

[zimoun]

Hi Maxime,

On Wed, 05 Jan 2022 at 12:27, Maxime Devos <maximedevos@telenet.be> wrote:
> zimoun schreef op wo 05-01-2022 om 12:48 [+0100]:
>> Well, I think ’#:recursive?’ is confusing, and ’auto’ too because it is
>> not POLA for a plumbing function, IMHO.  [...]
>
> Principle of least authority, or principle of least astonishment?
> I presume the latter.

Latter. :-)

>> Anyway. It is v4 and it is ready to merge. :-)
>
> I vote for a purple bikeshed! But your orange bikeshed would also keep
> the bikes out of the rain.

:-)

>> --8<---------------cut here---------------start------------->8---
>>   "Compute the hash of FILE with ALGORITHM.  If NAR-SERIALIZER? is
>>   #true, compute the combined hash (NAR hash) of FILE for which (SELECT?
>>   FILE STAT) returns true.
>> 
>>   If NAR-SERIALIZER? is #false, compute the regular hash using the
>>   default serializer.  It is meant to be used for a regular file.
>> 
>>   If NAR-SERIALIZER? is 'auto', when FILE is a directory, compute the
>>   combined hash (NAR hash).  When FILE is a regular file, compute the
>>   regular hash using the default serializer.  The option ’auto’ is meant
>>   to apply by default the expected hash computation.
>> 
>>   Symbolic links are not dereferenced unless NAR-SERIALIZER? is false.
>> 
>>   This procedure must only be used under controlled circumstances; the
>>   detection of symbolic links in FILE is racy.
>> --8<---------------cut here---------------end--------------->8---

> The nar hash / regular hash difference seems a very low-level detail to
> me, that most (all?) users don't need to be bothered about. Except
> maybe if FILE denotes an executable regular file, but file-hash* is
> currently only used on tarballs/zip files/git checkouts, which aren't
> executable files unless weirdness or some kind of attack is happening.
>
> I think that, the ‘least astonishing’ thing to do here, is computing
> the hash that would go into the 'hash' / 'sha256' field of 'origin'
> objects by default, and not the nar hash for regular files that's
> almost never used.

I do not understand what you mean here.  ’file-hash*’ is a low-level
detail, no?  Whatever. :-)

Well, I am sorry if my 3 naive comments are not convenient.  Just, to be
sure, I am proposing:

 1) It is v4 and ready, I guess.  About ’auto’, I could have waken up
 earlier. :-) And it can be still improved later as you are saying in
 the other answer.  So, we are done, right?

 2) From my point of view, ’#:recursive?’ needs to be adapted in
 agreement with the discussion [1], quoting Ludo:

        Thinking more about it, I think confusion stems from the term
        “recursive” (inherited from Nix) because, as you write, it
        doesn’t necessarily have to do with recursion and directory
        traversal.

        Instead, it has to do with the serialization method.

        1: <http://issues.guix.gnu.org/issue/51307>

   And I do not have a strong opinion.  Just a naive remark.

 3) Whatever the keyword for the current v4 ’#:recursive?’ is picked, I
  still find the current docstring wording unclear.  In fact, reading
  the code is more helpful. :-) I am just proposing a reword which
  appears to me clearer than the current v4 one.  Maybe, I am missing
  the obvious.  Or maybe this proposed rewording is not clearer. :-)

WDYT?

Cheers,
simon