Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Niels Thykier
Sean Whitton:
> [...]

Hi,

Thanks for the updated version.  :)

My second also applies to the re-worded variant quoted below

> Here is the complete new diff for seconding.  Below that, I've included
> the interdiff between the patch Niels seconded and the new one.
> 
>> diff --git a/debian/changelog b/debian/changelog
>> index 2dea331..b89816e 100644
>> --- a/debian/changelog
>> +++ b/debian/changelog
>> @@ -1,5 +1,11 @@
>> -debian-policy (4.1.4.2) UNRELEASED; urgency=medium
>> +debian-policy (4.1.5.0) UNRELEASED; urgency=medium
>>
>> +  * Policy: Document Rules-Requires-Root
>> +Wording: Niels Thykier 
>> +Wording: Guillem Jover 
>> +Wording: Sean Whitton 
>> +Seconded: ...
>> +Closes: #880920
>>* Fix URL to the alioth lists service in footnote (Closes: #896749).
>>  Thanks Martin Zobel-Helas for the report.
>>
>> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
>> index 0771346..3afba4c 100644
>> --- a/policy/ch-controlfields.rst
>> +++ b/policy/ch-controlfields.rst
>> @@ -129,6 +129,8 @@ package) are:
>>
>>  -  :ref:`Testsuite `
>>
>> +-  :ref:`Rules-Requires-Root `
>> +
>>  The fields in the binary package paragraphs are:
>>
>>  -  :ref:`Package ` (mandatory)
>> @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
>> control files
>>  field may also be used in source package control files
>>  (``debian/control``) if needed in other situations.
>>
>> +.. _s-f-Rules-Requires-Root:
>> +
>> +``Rules-Requires-Root``
>> +~~~
>> +
>> +Simple field that defines if the source package requires access to
>> +root (or fakeroot) during selected targets in the :ref:`Main building
>> +script: debian/rules `.
>> +
>> +The field can consist of exactly one of the following three items:
>> +
>> + - ``no``: Declares that neither root nor fakeroot is required.
>> +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
>> +   target in ``debian/rules`` with an unprivileged user.
>> +
>> + - ``binary-targets`` (default): Declares that the package will need
>> +   the root (or fakeroot) when either of the ``binary``,
>> +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
>> +   how every tool behaved before this field was defined.
>> +
>> + - A space separated list of keywords described below.  These keywords
>> +   must always contain a forward slash, which sets them apart from the
>> +   other possible values of ``Rules-Requires-Root``.  When this list
>> +   is provided, the builder must provide a `gain root command` (as
>> +   defined in :ref:`debian/rules and Rules-Requires-Root
>> +   `) *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
>> +wants 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
>> +   that value.  The builder may trim unnecessary whitespace used to
>> +   format the field for readability.
>> +
>> + * The value ``binary-targets`` if it cannot support the value of
>> +   ``Rules-Requires-Root``.
>> +
>> +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
>> +or set it to ``binary-targets`` if it has been requested to test
>> +whether the package it builds correctly implements the fall-back for
>> +legacy builders.
>> +
>> +Remarks
>> +^^^
>> +
>> +All packages and builders must support ``binary-targets`` as this was
>> +the historical behaviour prior to the introduction of this field.
>> +
>> +Any tool (particularly older versions of them) may be unaware of this
>> +field and behave like the field was set to ``binary-targets``.  The
>> +package build must gracefully cope with this and produce a
>> +semantically equivalent result.
>> +
>> +This field intentionally does not enable a package to request a true
>> +root over fakeroot.
>> +
>> +Definition of the keywords
>> +^^
>> +
>> +The keywords have the format ``/``, where:
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace and the forward slash (``/``).  It must
>> +   consist of at least 2 characters.
>> +
>> + * ``/`` (between  and ) is a single ASCII
>> +   forward slash.
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace.  It must consist of at least 2
>> +   characters.
>> +
>> +These keywords define where the package build script ``debian/rules``,
>> +or the tools called by that script, will need access to root or
>> +fakeroot.
>> +
>> +In addition to the keywords defined in the next section, each tool or
>> +package may 

Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Paul Gevers
Seconded.

On 15-06-18 19:02, Sean Whitton wrote:
> On Fri, Jun 15 2018, Simon McVittie wrote:
> 
>> This all seems valid to me, but it's relatively subtle. Perhaps you could
>> clarify this sentence by saying "The gain root command must (blah blah)
>> because it will not necessarily be used by a shell"? I think that's
>> maybe easier to understand?
> 
> It would be useful if Paul could confirm, but this does seem easier to
> understand, and it doesn't lose any meaning.  So I've gone ahead and
> committed something like this to my branch -- thanks!

Thanks. I think this is easier to read.

> On Fri, Jun 15 2018, Paul Gevers wrote:
> 
>> 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".
> 
> It sets them apart from 'binary-targets' and 'no', indeed.
> 
> What I have now is this:
> 
> These keywords must always contain a forward slash, which sets them
> apart from the other possible values of ``Rules-Requires-Root``.
> 
>> And why would that matter?
>> I.e. what does ", which sets them apart from the other values" add?
> 
> I don't know exactly what Niels had in mind when he wrote it, but as a
> reader I for one find it useful in understanding the text.

I don't. But I don't mind them in.

>>> 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?
> 
> The *builder* may set the gain root command to $(su root), but the
> *package* cannot request $(su root).  That's all the sentence says.

Sorry, I now better realize it is the builder that determines what the
content of the command is. My bad for mixing this up.

>> diff --git a/debian/changelog b/debian/changelog
>> index 2dea331..b89816e 100644
>> --- a/debian/changelog
>> +++ b/debian/changelog
>> @@ -1,5 +1,11 @@
>> -debian-policy (4.1.4.2) UNRELEASED; urgency=medium
>> +debian-policy (4.1.5.0) UNRELEASED; urgency=medium
>>
>> +  * Policy: Document Rules-Requires-Root
>> +Wording: Niels Thykier 
>> +Wording: Guillem Jover 
>> +Wording: Sean Whitton 
>> +Seconded: ...
>> +Closes: #880920
>>* Fix URL to the alioth lists service in footnote (Closes: #896749).
>>  Thanks Martin Zobel-Helas for the report.
>>
>> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
>> index 0771346..3afba4c 100644
>> --- a/policy/ch-controlfields.rst
>> +++ b/policy/ch-controlfields.rst
>> @@ -129,6 +129,8 @@ package) are:
>>
>>  -  :ref:`Testsuite `
>>
>> +-  :ref:`Rules-Requires-Root `
>> +
>>  The fields in the binary package paragraphs are:
>>
>>  -  :ref:`Package ` (mandatory)
>> @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
>> control files
>>  field may also be used in source package control files
>>  (``debian/control``) if needed in other situations.
>>
>> +.. _s-f-Rules-Requires-Root:
>> +
>> +``Rules-Requires-Root``
>> +~~~
>> +
>> +Simple field that defines if the source package requires access to
>> +root (or fakeroot) during selected targets in the :ref:`Main building
>> +script: debian/rules `.
>> +
>> +The field can consist of exactly one of the following three items:
>> +
>> + - ``no``: Declares that neither root nor fakeroot is required.
>> +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
>> +   target in ``debian/rules`` with an unprivileged user.
>> +
>> + - ``binary-targets`` (default): Declares that the package will need
>> +   the root (or fakeroot) when either of the ``binary``,
>> +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
>> +   how every tool behaved before this field was 

Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Simon McVittie
On Fri, 15 Jun 2018 at 18:02:39 +0100, Sean Whitton wrote:
> Here is the complete new diff for seconding

Seconded (as below).
smcv

> > diff --git a/debian/changelog b/debian/changelog
> > index 2dea331..b89816e 100644
> > --- a/debian/changelog
> > +++ b/debian/changelog
> > @@ -1,5 +1,11 @@
> > -debian-policy (4.1.4.2) UNRELEASED; urgency=medium
> > +debian-policy (4.1.5.0) UNRELEASED; urgency=medium
> >
> > +  * Policy: Document Rules-Requires-Root
> > +Wording: Niels Thykier 
> > +Wording: Guillem Jover 
> > +Wording: Sean Whitton 
> > +Seconded: ...
> > +Closes: #880920
> >* Fix URL to the alioth lists service in footnote (Closes: #896749).
> >  Thanks Martin Zobel-Helas for the report.
> >
> > diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
> > index 0771346..3afba4c 100644
> > --- a/policy/ch-controlfields.rst
> > +++ b/policy/ch-controlfields.rst
> > @@ -129,6 +129,8 @@ package) are:
> >
> >  -  :ref:`Testsuite `
> >
> > +-  :ref:`Rules-Requires-Root `
> > +
> >  The fields in the binary package paragraphs are:
> >
> >  -  :ref:`Package ` (mandatory)
> > @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
> > control files
> >  field may also be used in source package control files
> >  (``debian/control``) if needed in other situations.
> >
> > +.. _s-f-Rules-Requires-Root:
> > +
> > +``Rules-Requires-Root``
> > +~~~
> > +
> > +Simple field that defines if the source package requires access to
> > +root (or fakeroot) during selected targets in the :ref:`Main building
> > +script: debian/rules `.
> > +
> > +The field can consist of exactly one of the following three items:
> > +
> > + - ``no``: Declares that neither root nor fakeroot is required.
> > +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
> > +   target in ``debian/rules`` with an unprivileged user.
> > +
> > + - ``binary-targets`` (default): Declares that the package will need
> > +   the root (or fakeroot) when either of the ``binary``,
> > +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
> > +   how every tool behaved before this field was defined.
> > +
> > + - A space separated list of keywords described below.  These keywords
> > +   must always contain a forward slash, which sets them apart from the
> > +   other possible values of ``Rules-Requires-Root``.  When this list
> > +   is provided, the builder must provide a `gain root command` (as
> > +   defined in :ref:`debian/rules and Rules-Requires-Root
> > +   `) *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
> > +wants 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
> > +   that value.  The builder may trim unnecessary whitespace used to
> > +   format the field for readability.
> > +
> > + * The value ``binary-targets`` if it cannot support the value of
> > +   ``Rules-Requires-Root``.
> > +
> > +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
> > +or set it to ``binary-targets`` if it has been requested to test
> > +whether the package it builds correctly implements the fall-back for
> > +legacy builders.
> > +
> > +Remarks
> > +^^^
> > +
> > +All packages and builders must support ``binary-targets`` as this was
> > +the historical behaviour prior to the introduction of this field.
> > +
> > +Any tool (particularly older versions of them) may be unaware of this
> > +field and behave like the field was set to ``binary-targets``.  The
> > +package build must gracefully cope with this and produce a
> > +semantically equivalent result.
> > +
> > +This field intentionally does not enable a package to request a true
> > +root over fakeroot.
> > +
> > +Definition of the keywords
> > +^^
> > +
> > +The keywords have the format ``/``, where:
> > +
> > + *  must consist entirely of printable ASCII characters
> > +   except for any whitespace and the forward slash (``/``).  It must
> > +   consist of at least 2 characters.
> > +
> > + * ``/`` (between  and ) is a single ASCII
> > +   forward slash.
> > +
> > + *  must consist entirely of printable ASCII characters
> > +   except for any whitespace.  It must consist of at least 2
> > +   characters.
> > +
> > +These keywords define where the package build script ``debian/rules``,
> > +or the tools called by that script, will need access to root or
> > +fakeroot.
> > +
> > +In addition to the keywords defined in the next section, each tool or
> > +package may define keywords 

Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Sean Whitton
[CCing Niels whose second has become invalid -- see interdiff at bottom
of mail]

Hello,

On Fri, Jun 15 2018, Simon McVittie wrote:

> This all seems valid to me, but it's relatively subtle. Perhaps you could
> clarify this sentence by saying "The gain root command must (blah blah)
> because it will not necessarily be used by a shell"? I think that's
> maybe easier to understand?

It would be useful if Paul could confirm, but this does seem easier to
understand, and it doesn't lose any meaning.  So I've gone ahead and
committed something like this to my branch -- thanks!

On Fri, Jun 15 2018, Paul Gevers wrote:

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

It sets them apart from 'binary-targets' and 'no', indeed.

What I have now is this:

These keywords must always contain a forward slash, which sets them
apart from the other possible values of ``Rules-Requires-Root``.

> And why would that matter?
> I.e. what does ", which sets them apart from the other values" add?

I don't know exactly what Niels had in mind when he wrote it, but as a
reader I for one find it useful in understanding the text.

 +   a `gain root command` (as defined in :ref:`debian/rules and
 +   Rules-Requires-Root `) *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).

Yes, there is a distinction between a builder supporting the field, and
supporting any particular values of the field.

 +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?

The *builder* may set the gain root command to $(su root), but the
*package* cannot request $(su 

Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Bill Allombert
On Fri, Jun 15, 2018 at 03:35:09PM +0100, Simon McVittie wrote:
> On Fri, 15 Jun 2018 at 13:43:36 +0100, Sean Whitton wrote:
> > On Thu, Jun 14 2018, Paul Gevers wrote:
> > >> +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?
> 
> (I am a native en_GB speaker)
> 
> I agree that the quoted diff was correct English.
> 
> Lintian's rather simplistic spelling and grammar checker complains
> about the formulation "this utility *allows to* reticulate splines"
> (my emphasis), which is a fairly common bit of en_DE. I assume it's
> a literal translation of something that's correct in German?

This is also common in en_FR, so it is probably valid in en_EU.
The English passive forms rules are awkward for latin speakers.
It is common to use the latin passive forms rules in en_EU.

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 



Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Simon McVittie
On Fri, 15 Jun 2018 at 13:43:36 +0100, Sean Whitton wrote:
> On Thu, Jun 14 2018, Paul Gevers wrote:
> >> +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?

(I am a native en_GB speaker)

I agree that the quoted diff was correct English.

Lintian's rather simplistic spelling and grammar checker complains
about the formulation "this utility *allows to* reticulate splines"
(my emphasis), which is a fairly common bit of en_DE. I assume it's
a literal translation of something that's correct in German?

That isn't correct English: it doesn't say who or what is given
the ability to reticulate splines. The Lintian spellchecker suggests
"this utility *allows one to* reticulate splines", which is correct and
general but maybe overly formal. "This utility allows a user to reticulate
splines", "this utility allows game developers to reticulate splines" or
"this utility allows simulation game engines to reticulate splines"[1]
would also be correct, but Lintian's spellchecker doesn't have enough
context to know who/what.

In this case, the sentence correctly says who/what is given the ability
to run subcommands: the ``debian/rules`` target.

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

This all seems valid to me, but it's relatively subtle. Perhaps you could
clarify this sentence by saying "The gain root command must (blah blah)
because it will not necessarily be used by a shell"? I think that's
maybe easier to understand?

smcv

[1] http://sims.wikia.com/wiki/Reticulating_splines



Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Paul Gevers
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 `) *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 `
>>> +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
>>> +` field, the package builder
>>> +(e.g. dpkg-buildpackage) may run the 

Bug#880920: Document Rules-Requires-Root field

2018-06-15 Thread Sean Whitton
Hello Paul,

On Thu, Jun 14 2018, Paul Gevers wrote:

>> + - A space separated list of keywords described below.  These must
>> +   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.
>
>> +   a `gain root command` (as defined in :ref:`debian/rules and
>> +   Rules-Requires-Root `) *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'm sorry, but I do not see how it could be ambiguous.  Maybe you could
state the multiple senses between which you find it to be ambiguous.

>> +   that value.  The builder may trim unnecessary whitespace used to
>> +   format the field for readability.
>> +
>> + * The value ``binary-targets`` if it cannot support the value of
>> +   ``Rules-Requires-Root``.
>> +
>> +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
>> +or set it to ``binary-targets`` if it has been requested to test
>> +whether the package it builds correctly implements the fall-back for
>> +legacy builders.
>> +
>> +Remarks
>> +^^^
>> +
>> +All packages and builders must support ``binary-targets`` as this was
>> +the historical behaviour prior to the introduction of this field.
>> +
>> +Any tool (partiularly older versions of them) may be unaware of this
>> +field and behave like the field was set to ``binary-targets``.  The
>> +package build must gracefully cope with this and produce a
>> +semantically equivalent result.
>> +
>> +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.

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.

>> +
>> +Definition of the keywords
>> +^^
>> +
>> +The keywords have the format ``/``, where:
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace and the forward slash (``/``).  It must
>> +   consist of at least 2 characters.
>> +
>> + * ``/`` (between  and ) is a single ASCII
>> +   forward slash.
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace.  It must consist of at least 2
>> +   characters.
>> +
>> +These keywords define where the package build script ``debian/rules``,
>> +or the tools called by that script, will need access to root or
>> +fakeroot.
>> +
>> +In addition to the keywords defined in the next section, each tool or
>> +package may define keywords within a namespace named after that tool
>> +or package.  The package or tool is considered to own that namespace.
>> +
>> +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.

>> +(fake)root if and only if the tool defines an appropriate keyword in
>> +its namespace, and the package lists that keyword in
>> +``Rules-Requires-Root``.
>> +
>> +All tools must ignore keywords under namespaces they do not know or
>> +own.  A tool may emit a warning, or 

Bug#880920: Document Rules-Requires-Root field

2018-06-14 Thread Paul Gevers
I want to second this text, but have some questions.

> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
> index 0771346..3519d99 100644
> --- a/policy/ch-controlfields.rst
> +++ b/policy/ch-controlfields.rst

> @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
> control files
>  field may also be used in source package control files
>  (``debian/control``) if needed in other situations.
>
> +.. _s-f-Rules-Requires-Root:
> +
> +``Rules-Requires-Root``
> +~~~
> +
> +Simple field that defines if the source package requires access to
> +root (or fakeroot) during selected targets in the :ref:`Main building
> +script: debian/rules `.
> +
> +The field can consist of exactly one of the following three items:
> +
> + - ``no``: Declares that neither root nor fakeroot is required.
> +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
> +   target in ``debian/rules`` with an unprivileged user.
> +
> + - ``binary-targets`` (default): Declares that the package will need
> +   the root (or fakeroot) when either of the ``binary``,
> +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
> +   how every tool behaved before this field was defined.
> +
> + - A space separated list of keywords described below.  These must
> +   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.

> +   a `gain root command` (as defined in :ref:`debian/rules and
> +   Rules-Requires-Root `) *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.

> +   that value.  The builder may trim unnecessary whitespace used to
> +   format the field for readability.
> +
> + * The value ``binary-targets`` if it cannot support the value of
> +   ``Rules-Requires-Root``.
> +
> +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
> +or set it to ``binary-targets`` if it has been requested to test
> +whether the package it builds correctly implements the fall-back for
> +legacy builders.
> +
> +Remarks
> +^^^
> +
> +All packages and builders must support ``binary-targets`` as this was
> +the historical behaviour prior to the introduction of this field.
> +
> +Any tool (partiularly older versions of them) may be unaware of this
> +field and behave like the field was set to ``binary-targets``.  The
> +package build must gracefully cope with this and produce a
> +semantically equivalent result.
> +
> +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)?

> +
> +Definition of the keywords
> +^^
> +
> +The keywords have the format ``/``, where:
> +
> + *  must consist entirely of printable ASCII characters
> +   except for any whitespace and the forward slash (``/``).  It must
> +   consist of at least 2 characters.
> +
> + * ``/`` (between  and ) is a single ASCII
> +   forward slash.
> +
> + *  must consist entirely of printable ASCII characters
> +   except for any whitespace.  It must consist of at least 2
> +   characters.
> +
> +These keywords define where the package build script ``debian/rules``,
> +or the tools called by that script, will need access to root or
> +fakeroot.
> +
> +In addition to the keywords defined in the next section, each tool or
> +package may define keywords within a namespace named after that tool
> +or package.  The package or tool is considered to own that namespace.
> +
> +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.

> +(fake)root if and only if the tool defines an appropriate keyword in
> +its namespace, and the package lists that keyword in
> +``Rules-Requires-Root``.
> +
> +All tools must ignore keywords under namespaces they do not know or
> +own.  A tool may emit a warning, or abort with 

Bug#880920: Minor typo Re: Bug#880920: Document Rules-Requires-Root field

2018-05-19 Thread Jens Reyer
Hi,

just a minor typo I guess, absolutely no review, sorry:

"If the package builder supports the Rules-Requires-Root field and want
to enable the feature"

s/want/wants

Greets
jre



Bug#880920: Document Rules-Requires-Root field

2018-05-19 Thread Niels Thykier
Sean Whitton:
> control: tag -1 +patch
> 
> Hello,
> 
> Seeking one further second (on top of those from Niels and I) to the
> following diff:
> 

As mentioned, I already seconded it.  However, my mail was not signed
and for avoidance of doubt, I am hereby seconding it with a signed message.

@Sean: Also, I spotted a typo of "particularly" highlighted below that I
missed in the previous review.

>> diff --git a/debian/changelog b/debian/changelog
>> index 2dea331..b89816e 100644
>> --- a/debian/changelog
>> +++ b/debian/changelog
>> @@ -1,5 +1,11 @@
>> -debian-policy (4.1.4.2) UNRELEASED; urgency=medium
>> +debian-policy (4.1.5.0) UNRELEASED; urgency=medium
>>
>> +  * Policy: Document Rules-Requires-Root
>> +Wording: Niels Thykier 
>> +Wording: Guillem Jover 
>> +Wording: Sean Whitton 
>> +Seconded: ...
>> +Closes: #880920
>>* Fix URL to the alioth lists service in footnote (Closes: #896749).
>>  Thanks Martin Zobel-Helas for the report.
>>
>> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
>> index 0771346..3519d99 100644
>> --- a/policy/ch-controlfields.rst
>> +++ b/policy/ch-controlfields.rst
>> @@ -129,6 +129,8 @@ package) are:
>>
>>  -  :ref:`Testsuite `
>>
>> +-  :ref:`Rules-Requires-Root `
>> +
>>  The fields in the binary package paragraphs are:
>>
>>  -  :ref:`Package ` (mandatory)
>> @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
>> control files
>>  field may also be used in source package control files
>>  (``debian/control``) if needed in other situations.
>>
>> +.. _s-f-Rules-Requires-Root:
>> +
>> +``Rules-Requires-Root``
>> +~~~
>> +
>> +Simple field that defines if the source package requires access to
>> +root (or fakeroot) during selected targets in the :ref:`Main building
>> +script: debian/rules `.
>> +
>> +The field can consist of exactly one of the following three items:
>> +
>> + - ``no``: Declares that neither root nor fakeroot is required.
>> +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
>> +   target in ``debian/rules`` with an unprivileged user.
>> +
>> + - ``binary-targets`` (default): Declares that the package will need
>> +   the root (or fakeroot) when either of the ``binary``,
>> +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
>> +   how every tool behaved before this field was defined.
>> +
>> + - A space separated list of keywords described below.  These must
>> +   always contain a forward slash, which sets them apart from the
>> +   other values.  When this list is provided, the builder must provide
>> +   a `gain root command` (as defined in :ref:`debian/rules and
>> +   Rules-Requires-Root `) *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
>> +   that value.  The builder may trim unnecessary whitespace used to
>> +   format the field for readability.
>> +
>> + * The value ``binary-targets`` if it cannot support the value of
>> +   ``Rules-Requires-Root``.
>> +
>> +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
>> +or set it to ``binary-targets`` if it has been requested to test
>> +whether the package it builds correctly implements the fall-back for
>> +legacy builders.
>> +
>> +Remarks
>> +^^^
>> +
>> +All packages and builders must support ``binary-targets`` as this was
>> +the historical behaviour prior to the introduction of this field.
>> +
>> +Any tool (partiularly older versions of them) may be unaware of this
  ^^^

Typo of particularly

>> +field and behave like the field was set to ``binary-targets``.  The
>> +package build must gracefully cope with this and produce a
>> +semantically equivalent result.
>> +
>> +This field intentionally does not enable a package to request a true
>> +root over fakeroot.
>> +
>> +Definition of the keywords
>> +^^
>> +
>> +The keywords have the format ``/``, where:
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace and the forward slash (``/``).  It must
>> +   consist of at least 2 characters.
>> +
>> + * ``/`` (between  and ) is a single ASCII
>> +   forward slash.
>> +
>> + *  must consist entirely of printable ASCII characters
>> +   except for any whitespace.  It must consist of at least 2
>> +   characters.
>> +
>> +These keywords define where the package build script 

Bug#880920: Document Rules-Requires-Root field

2018-05-13 Thread Sean Whitton
control: tag -1 +patch

Hello,

Seeking one further second (on top of those from Niels and I) to the
following diff:

> diff --git a/debian/changelog b/debian/changelog
> index 2dea331..b89816e 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,5 +1,11 @@
> -debian-policy (4.1.4.2) UNRELEASED; urgency=medium
> +debian-policy (4.1.5.0) UNRELEASED; urgency=medium
>
> +  * Policy: Document Rules-Requires-Root
> +Wording: Niels Thykier 
> +Wording: Guillem Jover 
> +Wording: Sean Whitton 
> +Seconded: ...
> +Closes: #880920
>* Fix URL to the alioth lists service in footnote (Closes: #896749).
>  Thanks Martin Zobel-Helas for the report.
>
> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
> index 0771346..3519d99 100644
> --- a/policy/ch-controlfields.rst
> +++ b/policy/ch-controlfields.rst
> @@ -129,6 +129,8 @@ package) are:
>
>  -  :ref:`Testsuite `
>
> +-  :ref:`Rules-Requires-Root `
> +
>  The fields in the binary package paragraphs are:
>
>  -  :ref:`Package ` (mandatory)
> @@ -1020,6 +1022,118 @@ This field is automatically added to Debian source 
> control files
>  field may also be used in source package control files
>  (``debian/control``) if needed in other situations.
>
> +.. _s-f-Rules-Requires-Root:
> +
> +``Rules-Requires-Root``
> +~~~
> +
> +Simple field that defines if the source package requires access to
> +root (or fakeroot) during selected targets in the :ref:`Main building
> +script: debian/rules `.
> +
> +The field can consist of exactly one of the following three items:
> +
> + - ``no``: Declares that neither root nor fakeroot is required.
> +   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
> +   target in ``debian/rules`` with an unprivileged user.
> +
> + - ``binary-targets`` (default): Declares that the package will need
> +   the root (or fakeroot) when either of the ``binary``,
> +   ``binary-arch`` or ``binary-indep`` targets are called.  This is
> +   how every tool behaved before this field was defined.
> +
> + - A space separated list of keywords described below.  These must
> +   always contain a forward slash, which sets them apart from the
> +   other values.  When this list is provided, the builder must provide
> +   a `gain root command` (as defined in :ref:`debian/rules and
> +   Rules-Requires-Root `) *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
> +   that value.  The builder may trim unnecessary whitespace used to
> +   format the field for readability.
> +
> + * The value ``binary-targets`` if it cannot support the value of
> +   ``Rules-Requires-Root``.
> +
> +A compliant builder may also leave ``DEB_RULES_REQUIRES_ROOT`` unset
> +or set it to ``binary-targets`` if it has been requested to test
> +whether the package it builds correctly implements the fall-back for
> +legacy builders.
> +
> +Remarks
> +^^^
> +
> +All packages and builders must support ``binary-targets`` as this was
> +the historical behaviour prior to the introduction of this field.
> +
> +Any tool (partiularly older versions of them) may be unaware of this
> +field and behave like the field was set to ``binary-targets``.  The
> +package build must gracefully cope with this and produce a
> +semantically equivalent result.
> +
> +This field intentionally does not enable a package to request a true
> +root over fakeroot.
> +
> +Definition of the keywords
> +^^
> +
> +The keywords have the format ``/``, where:
> +
> + *  must consist entirely of printable ASCII characters
> +   except for any whitespace and the forward slash (``/``).  It must
> +   consist of at least 2 characters.
> +
> + * ``/`` (between  and ) is a single ASCII
> +   forward slash.
> +
> + *  must consist entirely of printable ASCII characters
> +   except for any whitespace.  It must consist of at least 2
> +   characters.
> +
> +These keywords define where the package build script ``debian/rules``,
> +or the tools called by that script, will need access to root or
> +fakeroot.
> +
> +In addition to the keywords defined in the next section, each tool or
> +package may define keywords within a namespace named after that tool
> +or package.  The package or tool is considered to own that namespace.
> +
> +A tool may use the `gain root command` to do something under
> +(fake)root if and only if the tool defines an 

Processed: Re: Bug#880920: Document Rules-Requires-Root field

2018-05-13 Thread Debian Bug Tracking System
Processing control commands:

> tag -1 +patch
Bug #880920 [debian-policy] Document Rules-Requires-Root field
Added tag(s) patch.

-- 
880920: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=880920
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems



Bug#880920: Document Rules-Requires-Root field

2018-05-13 Thread Niels Thykier
Sean Whitton:
> Hello Niels,
> 
> On Sun, Feb 25 2018, Niels Thykier wrote:
> 
>> Attached is my updated draft along with a changes since the previous
>> draft.
> 
> Thank you for this!  Let's get this into a release of the Policy Manual.
> 

Hi,

Thanks for cleaning up the text. :)

> I've pushed a bug880920-spwhitton branch to the Policy repo for you to
> review.  Aside from a single substantive change explained below, all my
> commits are rewordings for clarity and readability.  My commit messages
> should explain them.
> 
> It would be most efficient if you could base new patches on my
> bug880920-spwhitton branch.  Once we are happy with that branch, we can
> prepare a diff of the whole branch and post that to this bug to seek
> seconds, and then just merge the branch.
> 
Hi,

I have reviewed the commits on the branch (with HEAD at
efa61ef2c2580ac9a3c4ba2f0756249b4c862989) and I am happy with the
individual changes you have done on top of my initial proposal.

>From my PoV, I think it is ready for seconding/wider review and I am
happy to support/second it.

>> +The builder should set ``DEB_RULES_REQUIRES_ROOT`` environment
>> +variable when calling any of the mandatory targets as defined in
>> +:ref:`Rules-Requires-Root `.  If the
>> variable +is not set, the package must behave as if it was set to
>> +``binary-targets``.
>> +
> 
> I think s/should/may/ in the first line -- can you explain why you think
> it is worth enforcing this upon every build tool that might ever be
> uploaded to Debian, given that there exists a solid fallback?
> 

I am fine with it being relaxed to a "may", as I think documenting R³ is
more important than whether supporting is subject to a "should" or a "may".

Thanks,
~Niels



Bug#880920: Document Rules-Requires-Root field

2018-05-13 Thread Sean Whitton
Hello Niels,

On Sun, Feb 25 2018, Niels Thykier wrote:

> Attached is my updated draft along with a changes since the previous
> draft.

Thank you for this!  Let's get this into a release of the Policy Manual.

I've pushed a bug880920-spwhitton branch to the Policy repo for you to
review.  Aside from a single substantive change explained below, all my
commits are rewordings for clarity and readability.  My commit messages
should explain them.

It would be most efficient if you could base new patches on my
bug880920-spwhitton branch.  Once we are happy with that branch, we can
prepare a diff of the whole branch and post that to this bug to seek
seconds, and then just merge the branch.

> +The builder should set ``DEB_RULES_REQUIRES_ROOT`` environment
> +variable when calling any of the mandatory targets as defined in
> +:ref:`Rules-Requires-Root `.  If the
> variable +is not set, the package must behave as if it was set to
> +``binary-targets``.
> +

I think s/should/may/ in the first line -- can you explain why you think
it is worth enforcing this upon every build tool that might ever be
uploaded to Debian, given that there exists a solid fallback?

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#880920: Document Rules-Requires-Root field

2018-01-16 Thread Ian Jackson
Josh Triplett writes ("Bug#880920: Document Rules-Requires-Root field"):
> On Fri, 29 Dec 2017 11:29:00 + Niels Thykier <ni...@thykier.net> wrote:
> > +  # Command "sudo", with arguments "-nE" and "--"
> > +  export DPKG_GAIN_ROOT_CMD='sudo -nE --'
> > +  # Command "fakeroot" with the single argument "--"
> > +  export DPKG_GAIN_ROOT_CMD='fakeroot --'
> 
> You use sudo with -E here to preserve the environment. If that's a
> requirement, then the preceding paragraph should explicitly say
> something like "the command must preserve all environment variables,
> unmodified".

I think the environment does need to be preserved, indeed.

Ian.

-- 
Ian Jackson <ijack...@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.



Bug#880920: Document Rules-Requires-Root field

2018-01-01 Thread Josh Triplett
On Fri, 29 Dec 2017 11:29:00 + Niels Thykier  wrote:
> Here is an initial draft for how I think it could look.
> 
> Review welcome.
[...]

> +The `gain root command` is passed to the build script via the
> +``DPKG_GAIN_ROOT_CMD`` environment variable.  It is space separated
> +list with the first word being the command (which should available be
> +in PATH) and additional words being arguments.  The `gain root
> +command` may not be used via a shell and accordingly it must not rely
> +on shell features.
> +
> +The `gain root command` must not prompt or require human interaction
> +(as the build script itself must not require interaction).
> +Furthermore, it must be possible to preprend the command to an
> +existing command without having to alter or quote the command being
> +invoked.
> +
> +The following are examples of valid defitions root commands (in the sh
> +syntax) provided the tools are available and properly configured::
> +
> +  # Command "sudo", with arguments "-nE" and "--"
> +  export DPKG_GAIN_ROOT_CMD='sudo -nE --'
> +  # Command "fakeroot" with the single argument "--"
> +  export DPKG_GAIN_ROOT_CMD='fakeroot --'

You use sudo with -E here to preserve the environment. If that's a
requirement, then the preceding paragraph should explicitly say
something like "the command must preserve all environment variables,
unmodified".



Bug#880920: Document Rules-Requires-Root field

2017-12-29 Thread Niels Thykier
On Wed, 08 Nov 2017 18:20:59 -0700 Sean Whitton
<spwhit...@spwhitton.name> wrote:
> Hello Ian,
> 
> On Wed, Nov 08 2017, Ian Jackson wrote:
> 
> > Sean Whitton writes ("Bug#880920: Document Rules-Requires-Root
> >field"):
> >> I wonder if we should just make Policy the new home of the spec that
> >> Niels and Guillem have already written?
> >
> > I certainly would rather not block incorporation of this new spec into
> > Debian's official document set, on the task of reformatting it into
> > docbook.
> >
> > So yes it should probably go into the policy package (since there is
> > no better home for it).
> 
> Given that we are now rST I think we should not just dump the spec into
> the policy package, but hold out on a patch to the manual itself, since
> writing such a thing is not very hard at all.
> 
> -- 
> Sean Whitton

Hi,

Here is an initial draft for how I think it could look.

Review welcome.

Thanks,
~Niels
>From 337d56b25eadd4dd0d80f30019e9b837dbf01210 Mon Sep 17 00:00:00 2001
From: Niels Thykier <ni...@thykier.net>
Date: Fri, 29 Dec 2017 11:28:08 +
Subject: [PATCH] Initial draft for Rules-Requires-Root

Signed-off-by: Niels Thykier <ni...@thykier.net>
---
 policy/ch-controlfields.rst | 90 +
 policy/ch-source.rst| 46 ++-
 2 files changed, 135 insertions(+), 1 deletion(-)

diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
index 0771346..5310813 100644
--- a/policy/ch-controlfields.rst
+++ b/policy/ch-controlfields.rst
@@ -129,6 +129,8 @@ package) are:
 
 -  :ref:`Testsuite `
 
+-  :ref:`Rules-Requires-Root `
+
 The fields in the binary package paragraphs are:
 
 -  :ref:`Package ` (mandatory)
@@ -1020,6 +1022,94 @@ This field is automatically added to Debian source control files
 field may also be used in source package control files
 (``debian/control``) if needed in other situations.
 
+.. _s-f-Rules-Requires-Root:
+
+``Rules-Requires-Root``
+~~~
+
+Simple field that defines if the source package requires access to
+root (or fakeroot) during selected targets in the :ref:`Main building
+script: debian/rules `.
+
+The field can consist of exactly one of either of the following:
+
+ - ``no``: Declares that neither root nor fakeroot is required.
+   Package builders (e.g. dpkg-buildpackage) may choose to invoke any
+   target in ``debian/rules`` with an unprivileged user.
+
+ - ``binary-targets`` (default): Declares that the package will need
+   the root (or fakeroot) when either of the ``binary``,
+   ``binary-arch`` or ``binary-indep`` targets are called.  This is
+   how every tool behaved before this field was defined.
+
+ - A space separated list of keywords described below.  These must
+   always contain a forward slash, which sets them apart from the
+   other values.  When this list is provided, the builder must provide
+   a `gain root command` (as defined in :ref:`debian/rules - Gain root
+   api for Rules-Requires-Root `) *or*
+   pretend that the value was set to ``binary-targets`` and both
+   parties must degrade accordingly (see below).
+
+Please note that any tool (partiularly older versions of them) may be
+unaware of this field and behave like the field was set to
+``binary-targets``.  The package build must gracefully cope with this
+and produce the same semantical result regardless.  The value of this
+field must not degrade if a builder tool invokes the package build
+
+This field intentionally does not enable a package to request a true
+root over fakeroot.
+
+**Definition of the keywords**:
+
+The keywords have the format ``/``, where:
+
+ *  must consist entirely of printable ASCII characters
+   except for any whitespace and the forward slash (``/``).  It must
+   consist of at least 2 characters.
+
+ * ``/`` (between  and ) is a single ASCII
+   forward slash.
+
+ *  must consist entirely of printable ASCII characters
+   except for any whitespace.  It must consist of at least 2
+   characters.
+
+These keywords define where the package build script (or the tools
+called from it) will need access to root or fakeroot.  If ``debian/rules``
+directly needs to invoke a tool as root or under fakeroot, then it must
+use the keyword ``dpkg/target-subcommand``.
+
+Furthermore, each tool or package may claim its own namespace named
+after it and create keywords based on that namespace.  The tool may
+use the `gain root command` to perform a given action as root or under
+fakeroot if (but only if) a package lists of said keyword in the
+``Rules-Requires-Root`` field.
+
+All tools must ignore keywords with namespaces they do not know or
+own.  A tool may choose warn or abort with an error if it finds
+unknown keywords in namespaces it provides or owns (but it is not
+required to this for all keywords in the namespace).

Bug#880920: Document Rules-Requires-Root field

2017-11-12 Thread Guillem Jover
On Sun, 2017-11-05 at 10:20:35 -0700, Sean Whitton wrote:
> Package: debian-policy
> Version 4.1.1.1
> Severity: normal
> User: debian-pol...@packages.debian.org
> Usertags: proposal
> 
> On Sat, Nov 04 2017, Niels Thykier wrote:
> > While there has not been any comments / feedback on devel-devel, we
> > have seen about 35-40 packages adopting R³ since the proposal was
> > posted to debian-devel. :)
> >   This puts us on about ~50 packages that can now build without
> > (fake)root today (per [codesearch query]).
> 
> Given this, Rules-Requires-Root should go into Policy.

Yes, now, that the feedback deadline has passed.

> I wonder if we should just make Policy the new home of the spec that
> Niels and Guillem have already written?

While I think this should be obviously documented in policy. I'm not
planning on removing the "spec" from dpkg itself, because it's been my
growing opinion that dpkg should be self-contained when it comes to the
documentation about every of the interfaces it uses and provides. More
so because dpkg is used beyond Debian, and in many places it has no
saying on policies (vs. mechanisms), and in others it allows things
that Debian policy does not.

I could perhaps consider removing the debhelper keyword mention from
there because it feels like a layer violation, but I'm not overly
bothered.

Thanks,
Guillem



Bug#880920: Document Rules-Requires-Root field

2017-11-08 Thread Sean Whitton
Hello Ian,

On Wed, Nov 08 2017, Ian Jackson wrote:

> Sean Whitton writes ("Bug#880920: Document Rules-Requires-Root
>field"):
>> I wonder if we should just make Policy the new home of the spec that
>> Niels and Guillem have already written?
>
> I certainly would rather not block incorporation of this new spec into
> Debian's official document set, on the task of reformatting it into
> docbook.
>
> So yes it should probably go into the policy package (since there is
> no better home for it).

Given that we are now rST I think we should not just dump the spec into
the policy package, but hold out on a patch to the manual itself, since
writing such a thing is not very hard at all.

-- 
Sean Whitton


signature.asc
Description: PGP signature


Bug#880920: Document Rules-Requires-Root field

2017-11-08 Thread Russ Allbery
Ian Jackson <ijack...@chiark.greenend.org.uk> writes:
> Sean Whitton writes ("Bug#880920: Document Rules-Requires-Root field"):

>> I wonder if we should just make Policy the new home of the spec that
>> Niels and Guillem have already written?

> I certainly would rather not block incorporation of this new spec into
> Debian's official document set, on the task of reformatting it into
> docbook.

Policy is now reStructuredText, so hopefully the reformatting for any
future specs will be much, much easier.

-- 
Russ Allbery (r...@debian.org)   <http://www.eyrie.org/~eagle/>



Bug#880920: Document Rules-Requires-Root field

2017-11-08 Thread Ian Jackson
Sean Whitton writes ("Bug#880920: Document Rules-Requires-Root field"):
> I wonder if we should just make Policy the new home of the spec that
> Niels and Guillem have already written?

I certainly would rather not block incorporation of this new spec into
Debian's official document set, on the task of reformatting it into
docbook.

So yes it should probably go into the policy package (since there is
no better home for it).

Ian.

-- 
Ian Jackson <ijack...@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.



Bug#880920: Document Rules-Requires-Root field

2017-11-05 Thread Sean Whitton
Package: debian-policy
Version 4.1.1.1
Severity: normal
User: debian-pol...@packages.debian.org
Usertags: proposal

On Sat, Nov 04 2017, Niels Thykier wrote:

> While there has not been any comments / feedback on devel-devel, we
> have seen about 35-40 packages adopting R³ since the proposal was
> posted to debian-devel. :)
>   This puts us on about ~50 packages that can now build without
> (fake)root today (per [codesearch query]).

Given this, Rules-Requires-Root should go into Policy.

I wonder if we should just make Policy the new home of the spec that
Niels and Guillem have already written?

-- 
Sean Whitton


signature.asc
Description: PGP signature