[gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

The current description of ROOT makes no sense and just confuses people.
The new description is paraphrased from PMS.
---
 ebuild-writing/variables/text.xml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ebuild-writing/variables/text.xml b/ebuild-writing/variables/text.xml
index ef15347..6ba6379 100644
--- a/ebuild-writing/variables/text.xml
+++ b/ebuild-writing/variables/text.xml
@@ -100,8 +100,9 @@ them.
   <tr>
     <ti><c>ROOT</c></ti>
     <ti>
-      Path to the root directory. When not using <c>${D}</c>,
-      always prepend <c>${ROOT}</c> to the path.
+      The absolute path to the root directory into which the package is to be merged.
+      Use this when refering to installed files in <c>pkg_*</c> functions.
+      Never use this in <c>src_*</c> functions.
     </ti>
   </tr>
   <tr>
-- 
2.8.2

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Michael Orlitzky]

On 05/08/2016 01:42 PM, Mike Gilbert wrote:
> The current description of ROOT makes no sense and just confuses people.
> The new description is paraphrased from PMS.

The current version is bad, but the PMS version isn't great either.

We really need examples for D, ROOT, ED, EROOT, and EPREFIX.
When/where/why should they (not) be used? How do they compare/contrast?
A lot of people just guess. Our invocation of emake DESTDIR="${D}"
pretty well explains $D, but how the rest of them interact is left up to
your imagination.

We have maybe 150 ebuilds in the tree using $ROOT in src_* functions.
Some are bugs, but many look OK to me. Do we really want to say "never"
do that?

Here's the world's worst bash parser that I used to find them. It
doesn't work, but it gives you a list of likely candidates if you run it
on every ebuild in gentoo.git.


#!/usr/bin/python3

from sys import argv

in_src_func = False

with open(argv[1]) as f:
    for line in f:
        if not in_src_func:
            if line[0:4] == "src_" and line.rstrip()[-1] == "{":
                in_src_func = True
        else:
            if line.rstrip() == "}":
                in_src_func = False
            else:
                if "$ROOT" in line or "${ROOT}" in line:
                    print(argv[1], ":", line.strip())

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[M. J. Everitt]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

On 10/05/16 16:08, Michael Orlitzky wrote:
> On 05/08/2016 01:42 PM, Mike Gilbert wrote:
>> The current description of ROOT makes no sense and just confuses people.
>> The new description is paraphrased from PMS.
> The current version is bad, but the PMS version isn't great either.
>
> We really need examples for D, ROOT, ED, EROOT, and EPREFIX.
> When/where/why should they (not) be used? How do they compare/contrast?
> A lot of people just guess. Our invocation of emake DESTDIR="${D}"
> pretty well explains $D, but how the rest of them interact is left up to
> your imagination.
Agreed .. the devmanual doesn't expand a lot on this, and many of the
other ebuild variables A, S, etc and how to properly use them.

The 'emake' statement is part of default phases in EAPI5+ so this
may/should(?) not appear in newer ebuilds I would have thought, but
perhaps someone could clarify on this.

Michael.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Tue, May 10, 2016 at 11:08 AM, Michael Orlitzky <mjo@gentoo.org> wrote:
> We have maybe 150 ebuilds in the tree using $ROOT in src_* functions.
> Some are bugs, but many look OK to me. Do we really want to say "never"
> do that?

According to PMS, it is only legal in pkg functions.

Can you point to an example where using ROOT in a src function is appropriate?

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[James Le Cuirot]

[-- Attachment #1: Type: text/plain, Size: 1712 bytes --]

On Tue, 10 May 2016 14:28:46 -0400
Mike Gilbert <floppym@gentoo.org> wrote:

> On Tue, May 10, 2016 at 11:08 AM, Michael Orlitzky <mjo@gentoo.org>
> wrote:
> > We have maybe 150 ebuilds in the tree using $ROOT in src_*
> > functions. Some are bugs, but many look OK to me. Do we really want
> > to say "never" do that?  
> 
> According to PMS, it is only legal in pkg functions.
> 
> Can you point to an example where using ROOT in a src function is
> appropriate?

I thought that it was necessary for cross-compiling in some cases but I
was told that this should be SYSROOT instead. There is a bug open to
get that officially described in the PMS.

https://bugs.gentoo.org/show_bug.cgi?id=573306

Apart from that, I have grepped the tree for ROOT and I think many are
confusing it with EPREFIX. The example that stands out most for me is:

app-crypt/keybase/keybase-0.8.25.ebuild:
dosym "${D}/opt/${PN}/bin/main.js" "${ROOT}/usr/bin/keybase"

This is so wrong. If ROOT were /mnt/foo then the final result would be
a symlink at /mnt/foo/mnt/foo/usr/bin/keybase and it would point to
/var/tmp/portage/app-crypt/keybase-0.8.25/image//opt/keybase/bin/main.js.
Fortunately this madness is gone in later versions.

If you want to know what ROOT does then try it yourself.

ROOT=/mnt/foo emerge app-shells/dash

This isn't a system you can chroot into but it could be if you emerge
the whole of @system. Then trying throwing EPREFIX into the mix.

ROOT=/mnt/foo EPREFIX=/bar emerge app-shells/dash

I'm less familiar with prefixed systems and I think very few people mix
EPREFIX with ROOT but I try to do right by both.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 951 bytes --]

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Michael Orlitzky]

On 05/10/2016 02:28 PM, Mike Gilbert wrote:
> On Tue, May 10, 2016 at 11:08 AM, Michael Orlitzky <mjo@gentoo.org> wrote:
>> We have maybe 150 ebuilds in the tree using $ROOT in src_* functions.
>> Some are bugs, but many look OK to me. Do we really want to say "never"
>> do that?
> 
> According to PMS, it is only legal in pkg functions.
> 
> Can you point to an example where using ROOT in a src function is appropriate?
> 

Modulo the question "when should you use $ROOT over $EPREFIX"...

  * the chrome-binary-plugins ebuilds use it in src_install.

  * dmraid does

      einfo "Appending pkg.m4 from system to aclocal.m4"
      cat "${ROOT}"/usr/share/aclocal/pkg.m4 >>"${S}"/aclocal.m4 || \
        die "Could not append pkg.m4

Both of those look like EPREFIX candidates to me, but they're not
obviously wrong, either. Maybe it made sense in EAPI <= 3.

Anywhere that you need addpredict() it also seems reasonable. The
v4l-dvb-saa716x ebuilds use "${ROOT}/usr/src/linux/" where EPREFIX would
not be a good replacement.

Something needs to be fixed, though: you're right that the PMS limits
ROOT to pkg_*. Who knew? If we want to be serious about banning ROOT in
src_*, we should add a repoman error.

NB: the patch uses the HTTP spelling of "refering".

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Ian Stakenvicius]

[-- Attachment #1.1: Type: text/plain, Size: 412 bytes --]

On 10/05/16 05:25 PM, Michael Orlitzky wrote:
>   * dmraid does
> 
>       einfo "Appending pkg.m4 from system to aclocal.m4"
>       cat "${ROOT}"/usr/share/aclocal/pkg.m4 >>"${S}"/aclocal.m4 || \
>         die "Could not append pkg.m4

I'm pretty sure dmraid was me, and that code likely predates EPREFIX
(despite whatever the EAPI= says in dmraid's ebuild).  I'll fix /
revisit that one soon.




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Tue, May 10, 2016 at 5:25 PM, Michael Orlitzky <mjo@gentoo.org> wrote:
> On 05/10/2016 02:28 PM, Mike Gilbert wrote:
>> On Tue, May 10, 2016 at 11:08 AM, Michael Orlitzky <mjo@gentoo.org> wrote:
>>> We have maybe 150 ebuilds in the tree using $ROOT in src_* functions.
>>> Some are bugs, but many look OK to me. Do we really want to say "never"
>>> do that?
>>
>> According to PMS, it is only legal in pkg functions.
>>
>> Can you point to an example where using ROOT in a src function is appropriate?
>>
>
> Modulo the question "when should you use $ROOT over $EPREFIX"...
>
>   * the chrome-binary-plugins ebuilds use it in src_install.

This is quite obviously wrong. I happen to maintain that ebuild, so I
just fixed.

>   * dmraid does
>
>       einfo "Appending pkg.m4 from system to aclocal.m4"
>       cat "${ROOT}"/usr/share/aclocal/pkg.m4 >>"${S}"/aclocal.m4 || \
>         die "Could not append pkg.m4

This should be one of two things:

cat /usr/share/aclocal/pkg.m4

Or, if prefix support is desired:

cat "${EPREFIX}"/usr/share/aclocal/pkg.m4

> Both of those look like EPREFIX candidates to me, but they're not
> obviously wrong, either. Maybe it made sense in EAPI <= 3.
>
> Anywhere that you need addpredict() it also seems reasonable. The
> v4l-dvb-saa716x ebuilds use "${ROOT}/usr/src/linux/" where EPREFIX would
> not be a good replacement.

Nah, usually addpredict is dealing with stuff in the system that is
performing the build. Sandboxed phases should never be looking at
ROOT.

Those ebuilds should probably do addpredict /usr/src/linux instead.

>
> Something needs to be fixed, though: you're right that the PMS limits
> ROOT to pkg_*. Who knew? If we want to be serious about banning ROOT in
> src_*, we should add a repoman error.

Based on my own understanding of the ROOT variable, its use in src
functions is always nonsensical, especially when you consider binpkgs.
A binpkg can be installed with a ROOT value that is completely
different from its value when the package is being compiled.

To put it another way, in src functions, ROOT could/should always be
"/". The real value of ROOT isn't determined until merge time.

I agree, repoman should be catching this stuff.

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Brian Dolbec]

On Tue, 10 May 2016 21:42:19 -0400
Mike Gilbert <floppym@gentoo.org> wrote:

> On Tue, May 10, 2016 at 5:25 PM, Michael Orlitzky <mjo@gentoo.org>
> wrote:
> > On 05/10/2016 02:28 PM, Mike Gilbert wrote:  
> >> On Tue, May 10, 2016 at 11:08 AM, Michael Orlitzky
> >> <mjo@gentoo.org> wrote:  
> >>> We have maybe 150 ebuilds in the tree using $ROOT in src_*
> >>> functions. Some are bugs, but many look OK to me. Do we really
> >>> want to say "never" do that?  
> >>
> >> According to PMS, it is only legal in pkg functions.
> >>
> >> Can you point to an example where using ROOT in a src function is
> >> appropriate? 
> >
> > Modulo the question "when should you use $ROOT over $EPREFIX"...
> >
> >   * the chrome-binary-plugins ebuilds use it in src_install.  
> 
> This is quite obviously wrong. I happen to maintain that ebuild, so I
> just fixed.
> 
> >   * dmraid does
> >
> >       einfo "Appending pkg.m4 from system to aclocal.m4"
> >       cat "${ROOT}"/usr/share/aclocal/pkg.m4 >>"${S}"/aclocal.m4 ||
> > \ die "Could not append pkg.m4  
> 
> This should be one of two things:
> 
> cat /usr/share/aclocal/pkg.m4
> 
> Or, if prefix support is desired:
> 
> cat "${EPREFIX}"/usr/share/aclocal/pkg.m4
> 
> > Both of those look like EPREFIX candidates to me, but they're not
> > obviously wrong, either. Maybe it made sense in EAPI <= 3.
> >
> > Anywhere that you need addpredict() it also seems reasonable. The
> > v4l-dvb-saa716x ebuilds use "${ROOT}/usr/src/linux/" where EPREFIX
> > would not be a good replacement.  
> 
> Nah, usually addpredict is dealing with stuff in the system that is
> performing the build. Sandboxed phases should never be looking at
> ROOT.
> 
> Those ebuilds should probably do addpredict /usr/src/linux instead.
> 
> >
> > Something needs to be fixed, though: you're right that the PMS
> > limits ROOT to pkg_*. Who knew? If we want to be serious about
> > banning ROOT in src_*, we should add a repoman error.  
> 
> Based on my own understanding of the ROOT variable, its use in src
> functions is always nonsensical, especially when you consider binpkgs.
> A binpkg can be installed with a ROOT value that is completely
> different from its value when the package is being compiled.
> 
> To put it another way, in src functions, ROOT could/should always be
> "/". The real value of ROOT isn't determined until merge time.
> 
> I agree, repoman should be catching this stuff.
> 

Then Please contribute some test ebuilds for the gen-b0rk repo to test
repoman or any other Q/A apps with.  If they fail to detect the test
ebuilds, it will give us something to use to help trace the code errors.
It'll also make for a much better testing system overall for any app
performing any type of Q/A on ebuilds.


-- 
Brian Dolbec <dolsen>

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Tue, May 10, 2016 at 9:42 PM, Mike Gilbert <floppym@gentoo.org> wrote:
>> Anywhere that you need addpredict() it also seems reasonable. The
>> v4l-dvb-saa716x ebuilds use "${ROOT}/usr/src/linux/" where EPREFIX would
>> not be a good replacement.
>
> Nah, usually addpredict is dealing with stuff in the system that is
> performing the build. Sandboxed phases should never be looking at
> ROOT.
>
> Those ebuilds should probably do addpredict /usr/src/linux instead.

Oh, I see. It's compiling kernel modules based on linux sources
installed in ROOT. Given the definition in PMS, I would say that's
undefined behavior at best.

I think SYSROOT might be a better fit here.

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Tue, May 10, 2016 at 10:19 PM, Mike Gilbert <floppym@gentoo.org> wrote:
> On Tue, May 10, 2016 at 9:42 PM, Mike Gilbert <floppym@gentoo.org> wrote:
>>> Anywhere that you need addpredict() it also seems reasonable. The
>>> v4l-dvb-saa716x ebuilds use "${ROOT}/usr/src/linux/" where EPREFIX would
>>> not be a good replacement.
>>
>> Nah, usually addpredict is dealing with stuff in the system that is
>> performing the build. Sandboxed phases should never be looking at
>> ROOT.
>>
>> Those ebuilds should probably do addpredict /usr/src/linux instead.
>
> Oh, I see. It's compiling kernel modules based on linux sources
> installed in ROOT. Given the definition in PMS, I would say that's
> undefined behavior at best.
>
> I think SYSROOT might be a better fit here.

Actually, given the linux-mod eclass usage, it seems like
${KERNEL_DIR} would be a better choice than ${ROOT}/usr/src/linux.

It's also problematic that linux-info.eclass refers to ROOT in global
scope, but that's a separate bug/discussion.

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Tue, May 10, 2016 at 9:54 PM, Brian Dolbec <dolsen@gentoo.org> wrote:
>> I agree, repoman should be catching this stuff.
>>
>
> Then Please contribute some test ebuilds for the gen-b0rk repo to test
> repoman or any other Q/A apps with.  If they fail to detect the test
> ebuilds, it will give us something to use to help trace the code errors.
> It'll also make for a much better testing system overall for any app
> performing any type of Q/A on ebuilds.

I don't think there is a repoman check for ROOT in src functions
(yet). I was just saying that I think it would make sense to add that.

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Brian Dolbec]

On Tue, 10 May 2016 22:40:39 -0400
Mike Gilbert <floppym@gentoo.org> wrote:

> On Tue, May 10, 2016 at 9:54 PM, Brian Dolbec <dolsen@gentoo.org>
> wrote:
> >> I agree, repoman should be catching this stuff.
> >>  
> >
> > Then Please contribute some test ebuilds for the gen-b0rk repo to
> > test repoman or any other Q/A apps with.  If they fail to detect
> > the test ebuilds, it will give us something to use to help trace
> > the code errors. It'll also make for a much better testing system
> > overall for any app performing any type of Q/A on ebuilds.  
> 
> I don't think there is a repoman check for ROOT in src functions
> (yet). I was just saying that I think it would make sense to add that.
> 

As you know, with the modular system now in place, It should be quite
easy to add that check now.  There is no need to figure out where in
over 3000 lines of spaghetti code to insert it without messing up
something else.

Make the test ebuilds, at least one good one, plus however many to test
the broken parts.  It should only take an hour to add that check.

-- 
Brian Dolbec <dolsen>

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Göktürk Yüksek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Mike Gilbert:
> The current description of ROOT makes no sense and just confuses
> people. The new description is paraphrased from PMS. --- 
> ebuild-writing/variables/text.xml | 5 +++-- 1 file changed, 3
> insertions(+), 2 deletions(-)
> 
> diff --git a/ebuild-writing/variables/text.xml
> b/ebuild-writing/variables/text.xml index ef15347..6ba6379 100644 
> --- a/ebuild-writing/variables/text.xml +++
> b/ebuild-writing/variables/text.xml @@ -100,8 +100,9 @@ them. <tr> 
> <ti><c>ROOT</c></ti> <ti> -      Path to the root directory. When
> not using <c>${D}</c>, -      always prepend <c>${ROOT}</c> to the
> path. +      The absolute path to the root directory into which the
> package is to be merged. +      Use this when refering to installed
> files in <c>pkg_*</c> functions. +      Never use this in
> <c>src_*</c> functions. </ti> </tr> <tr>
> 

Sorry for the late reply. There is actually a bug for this issue [0].
I'll rebase the patch attached to the bug on top of yours if you have
no technical objections to the content.

[0] https://bugs.gentoo.org/show_bug.cgi?id=144332

-----BEGIN PGP SIGNATURE-----

iQEcBAEBCgAGBQJXN5oEAAoJEIT4AuXAiM4zCBoIAJb45hK2TtB4CpPMDC474XBc
sLk7H+2dDf4mFIkpqr6sTQlW4p/B+qNf/ov/RPC6/Rq3paDNsM9vZYa793pnVk0N
VjJo7iUA5Nmyv7mvgurMNFuiM5VjE3+64QGyTdw5Z9bBDYkWdK4bklZT1lCfIgcQ
OPpuLmuv80QpqUPyEqViqrmLaX9ajogGzeRlUxqUZJtNypNNJxSJ6xtUoLv/aEu5
htixSAMgUl25f+Xz6VY8GY/bhChx6gnEnzMolWQTJqiwfNgYCiRKLtZhAi0AkUwt
37xkVmjM48j9c0x5Dd5Wdk/nNKECatP0FthXcbzzHcNv46XXa9cNre6+8OcVzKc=
=fyKs
-----END PGP SIGNATURE-----

Re: [gentoo-dev] [PATCH] ebuild-writing/variables: better describe ROOT

[Mike Gilbert]

On Sat, May 14, 2016 at 5:35 PM, Göktürk Yüksek <gokturk@binghamton.edu> wrote:
> Mike Gilbert:
>> The current description of ROOT makes no sense and just confuses
>> people. The new description is paraphrased from PMS. ---
>> ebuild-writing/variables/text.xml | 5 +++-- 1 file changed, 3
>> insertions(+), 2 deletions(-)
>>
>> diff --git a/ebuild-writing/variables/text.xml
>> b/ebuild-writing/variables/text.xml index ef15347..6ba6379 100644
>> --- a/ebuild-writing/variables/text.xml +++
>> b/ebuild-writing/variables/text.xml @@ -100,8 +100,9 @@ them. <tr>
>> <ti><c>ROOT</c></ti> <ti> -      Path to the root directory. When
>> not using <c>${D}</c>, -      always prepend <c>${ROOT}</c> to the
>> path. +      The absolute path to the root directory into which the
>> package is to be merged. +      Use this when refering to installed
>> files in <c>pkg_*</c> functions. +      Never use this in
>> <c>src_*</c> functions. </ti> </tr> <tr>
>>
>
> Sorry for the late reply. There is actually a bug for this issue [0].
> I'll rebase the patch attached to the bug on top of yours if you have
> no technical objections to the content.
>
> [0] https://bugs.gentoo.org/show_bug.cgi?id=144332

The expanded explanation seems useful indeed.