[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#880920: Document Rules-Requires-Root field



Hi Sean,

On 15-06-18 14:43, Sean Whitton wrote:
> On Thu, Jun 14 2018, Paul Gevers wrote:
> 
>>> + - A space separated list of keywords described below.  These must

                                         insert "keywords" here?  ^

>>> +   always contain a forward slash, which sets them apart from the
>>> +   other values.  When this list is provided, the builder must provide
>>
>>             ^^^^^^ plural here, makes sense.

Ehm, reading again, it sets it apart from which other values? It is the
forward slash that marks them as being different from ``no`` and
``binary-targets``? Or do you mean other field values? Than maybe remove
the "the" in "apart from THE other values". And why would that matter?
I.e. what does ", which sets them apart from the other values" add?

>>> +   a `gain root command` (as defined in :ref:`debian/rules and
>>> +   Rules-Requires-Root <s-debianrules-gainrootapi>`) *or* pretend that
>>> +   the value was set to ``binary-targets``, and both the builder and
>>> +   the package's ``debian/rules`` script must downgrade accordingly
>>> +   (see below).
>>> +
>>> +If the package builder supports the ``Rules-Requires-Root`` field and
>>> +want to enable the feature, then it must set the environment variable
>>> +``DEB_RULES_REQUIRES_ROOT`` when invoking the package building script
>>> +``debian/rules``.  The value of ``DEB_RULES_REQUIRES_ROOT`` should be
>>> +one of:
>>> +
>>> + * The value of ``Rules-Requires-Root`` if the builder can support
>>
>>           ^^^^^ singular here. I find this ambiguous. I think you mean
>> to treat the list of values above as one variable by calling it singular
>> here, a suggested by the remark about space below.
> 
> The latter use of 'value' refers to the value of a field, but a field
> cannot have more than one value, so it has to be singular.

I understand that and still find it confusing. Maybe ambiguous isn't the
right word, it just meant I had to read the line twice before it struck.
And as the value of the field can contain more keywords, how does this
work for the builder? I think it means that if it can support one
keyword, it can support all. Maybe I was getting confused by this being
under the text "If the package builder supports the
``Rules-Requires-Root`` field " then how could it not support the value.
Thinking about it again and again, I understand it could ONLY support
"no" but not the keywords. So maybe it is OK as it is, it just means
more brain power than I expected (I am not very used to reading
specifications like that).

>>> +This field intentionally does not enable a package to request a true
>>> +root over fakeroot.
>>
>> Apparently some details in the implementation are unclear to me, as I
>> don't get this statement if the example at the end includes a sudo
>> example. Isn't that a true root or is that not what you mean? Is only
>> $(su root) a real root (and why wouldn't that work)?
> 
> Using R-R-R, a package can declare "I need a command that will give me
> root", but under this specification, either fakeroot or real root is
> considered an acceptable answer to that declaration.

Sure, that is exactly how I read that sentence as well.

> An alternative would be `Rules-Requires-Real-Root: yes` which would
> allow a package to declare "I need a command giving me actual root on
> the system building the package, not just fakeroot".  Then fakeroot
> would not be an acceptable answer.  But that is not the spec.
> 
> In short, the spec does not distinguish between real root and fakeroot,
> such that a package could request the former over the latter.

However, IIUC, the `gain root command` can be set to $(su root) and I
would get real root (or not?). Am I missing something?

>>> +A tool may use the `gain root command` to do something under
>>
>>                   ^^^ a?
>>                        ^^^^^^^^^^^^^^^^^ should this be linked to the
>> *section describing it? It drops out of thin air here.
> 
> There is already a cross reference just above, near "A space separated
> list of keywords described below."  Adding another cross reference seems
> like it would make things a bit cluttered?  I don't mind adding it if
> you are sure it's needed.

Sorry, I missed that. Never mind.

>>> -    The ``binary`` targets must be invoked as root.  [#]_
>>> +    The ``binary`` targets may be invoked as root depending on the
>>> +    value of the :ref:`Rules-Requires-Root <s-f-Rules-Requires-Root>`
>>> +    field.  [#]_
>>
>> I have difficulty parsing this sentence. I think I know what is meant.
>> The ``binary`` may always be invoked as root, but depending on the value
>> of R³, it may also not.
> 
> I read it in the same way as you do, but I agree that it is not a good
> sentence.
> 
> How about:
> 
>     The ``binary`` targets may need to be invoked as root depending on
>     the value of the Rules-Requires-Root field.

Much better in my opinion.

>>> +Depending on the value of the :ref:`Rules-Requires-Root
>>> +<s-f-Rules-Requires-Root>` field, the package builder
>>> +(e.g. dpkg-buildpackage) may run the ``debian/rules`` target as an
>>> +unprivileged user and provide a `gain root command`.  This command
>>> +allows the ``debian/rules`` target to run particular subcommands under
>>
>>    ^^^^^^ lintian will tell you this should be "enables"
> 
> As a native speaker I find 'allows' more natural than 'enables'.
> 'Allows' is certainly not grammatically incorrect.  Can you point me to
> the Lintian tag?

Hmm, maybe Lintian is only complaining about "allows to", but I read the
line having the same problem. It's not the command that allows the
debian/rules to do something, but the builder that allows the
debian/rules to use that command. The command enables debian/rules to
run with root permissions. I don't mind it staying, however, it is just
that due to Lintian, every time I see the word "allows" I have to think
about it.

https://lintian.debian.org/tags/spelling-error-in-binary.html
https://lintian.debian.org/tags/spelling-error-in-patch-description.html
maybe more

>>> +(fake)root.
>>> +
>>> +The `gain root command` is passed to the build script via the
>>> +``DEB_GAIN_ROOT_CMD`` environment variable.  The contents of this
>>> +variable is a space separated list, the first entry of which is the
>>> +command, and the proceeding entries of which are arguments to the
>>> +command.  The `gain root command` must be available via PATH.  The
>>> +`gain root command` must not rely on shell features because it need
>>> +not be used via a shell.
>>
>> I am not a native speaker, but isn't "doesn't need to" more natural?
>> Otherwise it should be "needs" I guess.
> 
> s/need/needs/ would not be grammatically correct.

Interesting, "It need"... whatever.

> I am a native speaker and I find the "need not" more idiomatic than
> "doesn't need to", but I have to admit that I do not know why :)
> 
> Here is a possible explanation, but please do not rely on this in the
> future unless some other native speakers are able to confirm it:
> 
> "The gain root command ... because it need not be used via a shell"
> <-- it /can/ be used without a shell and /might/ be used without a shell,
> /therefore/ it should not rely on shell features.
> 
> "The gain root command ... because it doesn't need to be used via a
> shell" <-- it /can/ be used without a shell,
> /therefore/ it should not rely on shell features.
> 
> i.e. the 'need not' carries the additional connotation that it might
> /actually/ be used without a shell, which is the reason for not relying
> on shell features.

Fine. Then just let me point out that as a non-native speaker this may
be too subtle and the sentence strikes as unusual. Albeit it may be
better, the text may be easier to read for non-native speakers if you
refrain from this construction. I won't object if you keep it though.

>>> +The following are examples of valid gain root commands (in syntax of
>>                                       `gain root command`s ?
> 
> Unfortunately that change breaks Sphinx, causing it to print literal
> backticks.

Ack, then leave it.

Paul

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: