Subject: Re: [bug#29271] [PATCH 1/1] doc: Split the "Substitutes" section into subsections.

From: Chris Marusich <[email-masked]>
Date: Tue, 14 Nov 2017 21:21:05 -0800

Hi everyone,

Thank you for taking the time to review my patch!

myglc2 <[email-masked]> writes:

> On 11/12/2017 at 20:25 Leo Famulari writes:
>
> ISTM that "disabled by default when using Guix on a foreign
> distribution" is too strong. How about ...
>
> Substitutes from Hydra are enabled by default in the Guix System
> Distribution (@pxref{GNU Distribution}) and by one of the recommended
> installation steps for Guix on a foreign distribution
> (@pxref{Installation}).

What if we say something like "disabled by default when using Guix on a
foreign distro, unless you've explicitly enabled them via one of the
recommended installation steps (@pxref{Installation})"?  Let me know if
you think that would be OK.

I want to clarify the fact that the substitutes are disabled by default
when installing Guix on a foreign distro.  That's a bit surprising, and
it needs to be clear.  At the same time, I think it's good to remind
users that they are in full control and can enable the substitutes
easily.

Leo Famulari <[email-masked]> writes:

> Personally, I find it impossible to read diffs of natural language text;
> it just doesn't work for me like it does with code.
>
> So, I will just ask, does this duplicate instructions regarding
> substitutes in other sections? We should be careful to avoid that.

No, there is no duplication (as far as I can tell!).  90% of this commit
is about moving existing text into subsections.  9% is the new
sub-section "Substitution Failure", and 1% is minor adjustments
elsewhere (e.g., removing an incorrect statement from the "Invoking
guix-daemon" section).

If this were code, I might have broken it out more into separate
commits, but since the GNU standards explicitly say that we don't even
need ChangeLog entries for documentation, I think it's OK to
conveniently squish these changes into one commit.

>> +@anchor{fallback-option}
>>  @item --fallback
>> -When substituting a pre-built binary fails, fall back to building
>> -packages locally.
>> +Attempt to build locally instead of issuing an error when substitutes
>> +are enabled and a substitute is available, but the substitution attempt
>> +fails (@pxref{Substitution Failure}).
>
> I was able to read this smaller change. I think the new sentence is too
> byzantine. I know what --fallback does but I found this sentence hard to
> understand.
>
> The condition that determines whether we "attempt to build locally"
> should be at the beginning of the sentence, like this:
>
> "When substitutes are enabled and a substitute is available, but the
> substitution fails, fall back to building the derivation locally."
>
> This is basically the same as before, but with some more detail about
> the condition.

I agree that is better, as long as we include a cross-reference to the
more detailed "Substitution Failure" section (which is what Eric
suggested).  I'll update the patch along those lines.

myglc2 <[email-masked]> writes:

> Chris,
>
> I installed the patch and read the sections. This is great stuff.
>
> In addition to the minor suggestion in a sub-thread, how about
> replacing...
>
>> +When substitutes are enabled and a substitute for a derivation is
>> +available, but the substitution attempt fails, Guix will attempt to
>> +build the derivation locally depending on whether or not
>> +@code{--fallback} was given (@pxref{fallback-option}).  Specifically, if
>> +@code{--fallback} was omitted, then no local build will be performed,
>> +and the derivation is considered to have failed.  However, if
>> +@code{--fallback} was given, then Guix will attempt to build the
>> +derivation locally, and the success or failure of the derivation depends
>> +on the success or failure of the local build.  Note that when
>
> ... with...
>
> When substitutes are enabled and a substitute for a derivation is
> available, but the substitution attempt fails, Guix will report an error
> unless the @code{--fallback} was given (@pxref{fallback-option}), in
> which case Guix will attempt to build the derivation locally Note that
> when

Personally, I prefer the more detailed explanation because it's less
ambiguous.  What does "report an error" mean?  To those who already
know, it might be clear, but to someone who is learning for the first
time, the ambiguity is unhelpful.

However, if others prefer the less verbose wording, I will use it.
What do you think, Leo and Eric?

-- 
Chris