Re: [gentoo-dev] [PMS] [PATCH] Correct the definition of ESYSROOT as EPREFIX isn't always applicable

2019-07-30 Thread James Le Cuirot
On Mon, 29 Jul 2019 14:05:10 +0200
Alexis Ballier  wrote:

> There seems to be unneeded whitespace only changes here that make the
> diff less readable

Sorry about that. I've only changed one cell in that table.

> Admittedly without a full understanding of the problem, but this looks
> wrong to me: SYSROOT, EPREFIX and BROOT are only relevant in build
> phases (src_*); (EPREFIX is a little special here but mostly for
> convenience). ROOT is only relevant in pkg_* phases. I don't see how
> this can work. Say I build a binpkg with ROOT=/ then use that binpkg
> with ROOT=/somewhere, you can't go back and change SYSROOT.

ROOT is used to determine ESYSROOT, not the other way around. As you
say, (E)SYSROOT is only relevant in src phases so it doesn't matter if
ROOT has changed when installing a binpkg.

I take your point that setting a src phase variable based on a pkg
phase variable seems odd but we're only using ROOT to determine the
applicable prefix. We're not taking the actual value of ROOT. When
Portage works all this out, it has access to all the necessary
variables. It only filters the variables based on the phase function
later on.

> Also, I think the wording could be more "programmatic" (if ... then
> ESYSROOT is ... else ... ) to avoid confusion.

Given that there are three clauses, I thought that using "respectively"
would be shorter and clearer but I'll try some other wording to see how
it looks.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer


pgpHV_CFv44RS.pgp
Description: OpenPGP digital signature


gentoo-dev@lists.gentoo.org

2019-07-30 Thread Benda Xu
Michał Górny  writes:

> Add a new @INCLUDES_EPREFIX tag that indicates that a particular
> function (= getter) or variable is a path that includes ${EPREFIX}.
> This will be used in pkgcheck to detect accidental variable and getter
> combinations that result in double prefix.
>
> Signed-off-by: Michał Górny 
> ---
>  eclass-to-manpage.awk | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/eclass-to-manpage.awk b/eclass-to-manpage.awk
> index 44708d4..f8620c8 100755
> --- a/eclass-to-manpage.awk
> +++ b/eclass-to-manpage.awk
> @@ -33,6 +33,7 @@
>  # @MAINTAINER:
>  # 
>  # [@INTERNAL]
> +# [@INCLUDES_EPREFIX] (the function outputs path that includes ${EPREFIX})
>  # @DESCRIPTION:
>  # 
>  
> @@ -42,6 +43,7 @@
>  # [@INTERNAL] (internal eclass use variable)
>  # [@DEFAULT_UNSET]
>  # [@REQUIRED]
> +# [@INCLUDES_EPREFIX] (the variable is a path that includes ${EPREFIX})
>  # @DESCRIPTION:
>  # 
>  # foo=""
> @@ -54,6 +56,7 @@
>  # [@INTERNAL] (internal eclass use variable)
>  # [@DEFAULT_UNSET]
>  # [@REQUIRED]
> +# [@INCLUDES_EPREFIX] (the variable is a path that includes ${EPREFIX})
>  # @DESCRIPTION:
>  # 
>  # foo=""
> @@ -252,6 +255,10 @@ function handle_function() {
>   internal = 1
>   getline
>   }
> + if ($2 == "@INCLUDES_EPREFIX") {
> + includes_eprefix = 1
> + getline
> + }
>   if ($2 == "@DESCRIPTION:")
>   desc = eat_paragraph()
>  
> @@ -314,6 +321,8 @@ function _handle_variable() {
>   user_variable = 1
>   else if ($2 == "@OUTPUT_VARIABLE")
>   output_variable = 1
> + else if ($2 == "@INCLUDES_EPREFIX")
> + includes_eprefix = 1
>   else
>   opts = 0
>   }

That looks like a good idea.

Yours,
Benda


[gentoo-dev] Re: [PMS] [PATCH] Correct the definition of ESYSROOT as EPREFIX isn't always applicable

2019-07-30 Thread Benda Xu
Hi James,

Sorry that I have re-ordered your text.

> A check was added to Portage to ensure this held. Myself, the
> ChromiumOS team, and others have since been caught out by this check
> when trying to bootstrap brand new systems from scratch. You cannot
> bootstrap with no headers at all! The check will therefore be adjusted
> to merely ensure that SYSROOT is / when ROOT is /.

It is very encouraging to see contributions from the ChromiumOS team to
Gentoo!

> What if we want to bootstrap a brand new prefixed system using the
> crossdev system as SYSROOT? This is the distinct SYSROOT case. The
> problem is that there is no distinct variable for SYSROOT's prefix
> and, as already stated, ESYSROOT is always ${SYSROOT}${EPREFIX}. We
> therefore cannot do it! If the crossdev prefix is blank then ROOT's
> must be blank too.

My question: is there a case when both SYSROOT and ESYSROOT are needed?
As far as I can see, SYSROOT is strictly build-time. Therefore setting
SYSROOT= would be enough for all.

Why did ESYSROOT exist in the first place?  Was it only for the sake of
symmetry, or did I miss something?

> It was originally envisaged (but not stated in PMS) that SYSROOT would
> only ever need to equal / or ROOT as a distinct SYSROOT would have no
> benefit.
...
> There were differing assumptions about how prefixes applied to the
> above. EPREFIX is traditionally something the user sets so some
> thought that it would be applied to SYSROOT, regardless of the
> latter's value. In order to honor the rule about there being no
> distinct SYSROOT, this would mean that if SYSROOT is / then EPREFIX
> would have to match BROOT.
...
> I also never intended to have the aforementioned limitation where
> EPREFIX must match BROOT when SYSROOT is /. These are both entirely
> artificial restrictions.

I agree. This is an artificial restrictions.

> What about the cross-prefix case? Here, SYSROOT matches both / and
> ROOT so which prefix do we choose? The bootstrap-prefix.sh script sets
> flags to build against the target prefix so EPREFIX is used in this
> case. This happens to fit the current definition of ESYSROOT anyway.

In this bootstrap case, SYSROOT=EPREFIX would do.  Again, no ESYSROOT is
needed.


In conclusion, IMHO, if SYSROOT can be overridden without restriction
("distinct" in your email), we could cover all the use cases.

Yours,
Benda



Re: [gentoo-dev] [PATCH] user.eclass: Use egetent in eget{user,group}name

2019-07-30 Thread Mike Gilbert
On Tue, Jul 30, 2019 at 10:45 AM Michał Górny  wrote:
>
> Use egetent+cut to obtain user/group names rather than id(1).
> The latter has no real advantage (besides being shorter to type),
> and does not work correctly for getting groups.
>
> Closes: https://bugs.gentoo.org/691056
> Signed-off-by: Michał Górny 

Looks good to me.



[gentoo-dev] [PATCH] user.eclass: Use egetent in eget{user,group}name

2019-07-30 Thread Michał Górny
Use egetent+cut to obtain user/group names rather than id(1).
The latter has no real advantage (besides being shorter to type),
and does not work correctly for getting groups.

Closes: https://bugs.gentoo.org/691056
Signed-off-by: Michał Górny 
---
 eclass/user.eclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/eclass/user.eclass b/eclass/user.eclass
index a3cacb6d5f10..f433d32bf7ed 100644
--- a/eclass/user.eclass
+++ b/eclass/user.eclass
@@ -358,7 +358,7 @@ enewgroup() {
 egetusername() {
[[ $# -eq 1 ]] || die "usage: egetusername "
 
-   id -u -n "$1"
+   egetent passwd "$1" | cut -d: -f1
 }
 
 # @FUNCTION: egetgroupname
@@ -368,7 +368,7 @@ egetusername() {
 egetgroupname() {
[[ $# -eq 1 ]] || die "usage: egetgroupname "
 
-   id -g -n "$1"
+   egetent group "$1" | cut -d: -f1
 }
 
 # @FUNCTION: egethome
-- 
2.22.0




gentoo-dev@lists.gentoo.org

2019-07-30 Thread Michał Górny
Add a new @INCLUDES_EPREFIX tag that indicates that a particular
function (= getter) or variable is a path that includes ${EPREFIX}.
This will be used in pkgcheck to detect accidental variable and getter
combinations that result in double prefix.

Signed-off-by: Michał Górny 
---
 eclass-to-manpage.awk | 9 +
 1 file changed, 9 insertions(+)

diff --git a/eclass-to-manpage.awk b/eclass-to-manpage.awk
index 44708d4..f8620c8 100755
--- a/eclass-to-manpage.awk
+++ b/eclass-to-manpage.awk
@@ -33,6 +33,7 @@
 # @MAINTAINER:
 # 
 # [@INTERNAL]
+# [@INCLUDES_EPREFIX] (the function outputs path that includes ${EPREFIX})
 # @DESCRIPTION:
 # 
 
@@ -42,6 +43,7 @@
 # [@INTERNAL] (internal eclass use variable)
 # [@DEFAULT_UNSET]
 # [@REQUIRED]
+# [@INCLUDES_EPREFIX] (the variable is a path that includes ${EPREFIX})
 # @DESCRIPTION:
 # 
 # foo=""
@@ -54,6 +56,7 @@
 # [@INTERNAL] (internal eclass use variable)
 # [@DEFAULT_UNSET]
 # [@REQUIRED]
+# [@INCLUDES_EPREFIX] (the variable is a path that includes ${EPREFIX})
 # @DESCRIPTION:
 # 
 # foo=""
@@ -252,6 +255,10 @@ function handle_function() {
internal = 1
getline
}
+   if ($2 == "@INCLUDES_EPREFIX") {
+   includes_eprefix = 1
+   getline
+   }
if ($2 == "@DESCRIPTION:")
desc = eat_paragraph()
 
@@ -314,6 +321,8 @@ function _handle_variable() {
user_variable = 1
else if ($2 == "@OUTPUT_VARIABLE")
output_variable = 1
+   else if ($2 == "@INCLUDES_EPREFIX")
+   includes_eprefix = 1
else
opts = 0
}
-- 
2.22.0