Bug#880920: Document Rules-Requires-Root field
[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 <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).
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 root). That's all the sentence says.
>>>> - 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.
Committed.
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 <niels@thykier.net>
> + Wording: Guillem Jover <guillem@debian.org>
> + Wording: Sean Whitton <spwhitton@spwhitton.name>
> + 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 <s-f-Testsuite>`
>
> +- :ref:`Rules-Requires-Root <s-f-Rules-Requires-Root>`
> +
> The fields in the binary package paragraphs are:
>
> - :ref:`Package <s-f-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 <s-debianrules>`.
> +
> +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
> + <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
> +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 ``<namespace>/<case>``, where:
> +
> + * ``<namespace>`` must consist entirely of printable ASCII characters
> + except for any whitespace and the forward slash (``/``). It must
> + consist of at least 2 characters.
> +
> + * ``/`` (between ``<namespace>`` and ``<case>``) is a single ASCII
> + forward slash.
> +
> + * ``<case>`` 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 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 an error, if it finds
> +unknown keywords in namespaces it owns, but it is not required to do
> +this for all keywords in the namespace.
> +
> +Provided keywords
> +^^^^^^^^^^^^^^^^^
> +
> +The following keywords are defined:
> +
> + * ``dpkg/target-subcommand``: declares that there exists a command
> + that the ``debian/rules`` file must run under (fake)root
> +
> + * ``dpkg/target/foo``: declares that the additional, package-specific
> + target ``foo`` (that is, not one of the targets specified in
> + :ref:`Main building script: debian/rules <s-debianrules>`) must be
> + run under (fake)root
> +
> +This list is intentionally incomplete. You should consult the
> +documentation of the tool or package in question for which keywords it
> +defines and when they are needed.
> +
> .. _s5.7:
>
> User-defined fields
> diff --git a/policy/ch-source.rst b/policy/ch-source.rst
> index e3b1981..418dc84 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -346,7 +346,9 @@ The targets are as follows:
> architecture-dependent or not), it must still exist and must always
> succeed.
>
> - The ``binary`` targets must be invoked as root. [#]_
> + The ``binary`` targets may need to be invoked as root depending on
> + the value of the :ref:`Rules-Requires-Root
> + <s-f-Rules-Requires-Root>` field. [#]_
>
> ``clean`` (required)
> This must undo any effects that the ``build`` and ``binary``
> @@ -435,6 +437,12 @@ should not be used to get the CPU or system information; the
> that. GNU style variables should generally only be used with upstream
> build systems.
>
> +The builder may set ``DEB_RULES_REQUIRES_ROOT`` environment variable
> +when calling any of the mandatory targets as defined in
> +:ref:`Rules-Requires-Root <s-f-Rules-Requires-Root>`. If the variable
> +is not set, the package must behave as if it was set to
> +``binary-targets``.
> +
> .. _s-debianrules-options:
>
> ``debian/rules`` and ``DEB_BUILD_OPTIONS``
> @@ -525,6 +533,53 @@ order to make it work for your package.
> # Code to run the package test suite.
> endif
>
> +
> +.. _s-debianrules-gainrootapi:
> +
> +``debian/rules`` and ``Rules-Requires-Root``
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +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
> +(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 will
> +not necessarily be invoked via a shell.
> +
> +The `gain root command` must not run interactively, including
> +prompting for any user input. It must be possible to prepend the
> +`gain root command` to an existing command and its arguments, without
> +needing to alter or quote the existing command and its arguments.
> +Furthermore, the `gain root command` must preserve all environment
> +variables without the caller having to explicitly request any
> +preservation.
> +
> +The following are examples of valid gain root commands (in syntax of
> +sh), assuming the tools used are available and properly configured::
> +
> + # Command "sudo", with arguments "-nE" and "--"
> + export DEB_GAIN_ROOT_CMD='sudo -nE --'
> + # Command "fakeroot" with the single argument "--"
> + export DEB_GAIN_ROOT_CMD='fakeroot --'
> +
> +Examples of valid use of the `gain root command`::
> +
> + # sh-syntax (assumes set -e semantics for error handling)
> + $DEB_GAIN_ROOT_CMD some-cmd --which-requires-root
> +
> + # perl
> + my @cmd = ('some-cmd', '--which-requires-root');
> + unshift(@cmd, split(' ', $ENV{DEB_GAIN_ROOT_CMD})) if $ENV{DEB_GAIN_ROOT_CMD};
> + system(@cmd) == or die("@cmd failed");
> +
> .. _s-substvars:
>
> Variable substitutions: ``debian/substvars``
> diff --git a/policy/upgrading-checklist.rst b/policy/upgrading-checklist.rst
> index ec61f95..ad24eeb 100644
> --- a/policy/upgrading-checklist.rst
> +++ b/policy/upgrading-checklist.rst
> @@ -39,6 +39,18 @@ The sections in this checklist match the values for the
> except in the two anomalous historical cases where normative
> requirements were changed in a minor patch release.
>
> +Version 4.1.5
> +-------------
> +
> +Unreleased.
> +
> +4.9.2
> + Document how ``debian/rules`` and the ``Rules-Requires-Root``
> + field interact.
> +
> +5.6.31
> + Document the ``Rules-Requires-Root`` field.
> +
> Version 4.1.4
> -------------
Interdiff:
> diff --git a/policy/ch-controlfields.rst b/policy/ch-controlfields.rst
> index f0a0f92..3afba4c 100644
> --- a/policy/ch-controlfields.rst
> +++ b/policy/ch-controlfields.rst
> @@ -1042,14 +1042,14 @@ The field can consist of exactly one of the following three items:
> ``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 <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).
> + - 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
> + <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
> wants to enable the feature, then it must set the environment variable
> diff --git a/policy/ch-source.rst b/policy/ch-source.rst
> index 214d716..418dc84 100644
> --- a/policy/ch-source.rst
> +++ b/policy/ch-source.rst
> @@ -346,9 +346,9 @@ The targets are as follows:
> architecture-dependent or not), it must still exist and must always
> succeed.
>
> - The ``binary`` targets may be invoked as root depending on the
> - value of the :ref:`Rules-Requires-Root <s-f-Rules-Requires-Root>`
> - field. [#]_
> + The ``binary`` targets may need to be invoked as root depending on
> + the value of the :ref:`Rules-Requires-Root
> + <s-f-Rules-Requires-Root>` field. [#]_
>
> ``clean`` (required)
> This must undo any effects that the ``build`` and ``binary``
> @@ -551,8 +551,8 @@ The `gain root command` is passed to the build script via the
> 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.
> +`gain root command` must not rely on shell features because it will
> +not necessarily be invoked via a shell.
>
> The `gain root command` must not run interactively, including
> prompting for any user input. It must be possible to prepend the
--
Sean Whitton
Reply to: