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

Bug#880920: Document Rules-Requires-Root field



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

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

                  ^^^ 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 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..214d716 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 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.

>  ``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

   ^^^^^^ lintian will tell you this should be "enables"

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

> +
> +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
                                      `gain root command`s ?

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

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: