public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
@ 2021-01-03  1:09 Mike Gilbert
  2021-01-03 12:52 ` James Le Cuirot
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Gilbert @ 2021-01-03  1:09 UTC (permalink / raw
  To: gentoo-dev; +Cc: Mike Gilbert

When cross-compiling, users will typically have
PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper.

When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config
output get prefixed with this value.

Signed-off-by: Mike Gilbert <floppym@gentoo.org>
---

This patch has already been pushed, but I figured I would send it for
review in case someone else can think of a failure case, or has a better
solution.

 eclass/systemd.eclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
index 81065a0af79a..f6d1fa2d92d6 100644
--- a/eclass/systemd.eclass
+++ b/eclass/systemd.eclass
@@ -50,6 +50,7 @@ _systemd_get_dir() {
 
 	if $(tc-getPKG_CONFIG) --exists systemd; then
 		d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die
+		d=${d#${SYSROOT}}
 		d=${d#${EPREFIX}}
 	else
 		d=${fallback}
-- 
2.30.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-03  1:09 [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output Mike Gilbert
@ 2021-01-03 12:52 ` James Le Cuirot
  2021-01-03 13:02   ` James Le Cuirot
  2021-01-03 15:16   ` Mike Gilbert
  0 siblings, 2 replies; 9+ messages in thread
From: James Le Cuirot @ 2021-01-03 12:52 UTC (permalink / raw
  To: gentoo-dev

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

On Sat,  2 Jan 2021 20:09:04 -0500
Mike Gilbert <floppym@gentoo.org> wrote:

> When cross-compiling, users will typically have
> PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper.
> 
> When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config
> output get prefixed with this value.
> 
> Signed-off-by: Mike Gilbert <floppym@gentoo.org>
> ---
> 
> This patch has already been pushed, but I figured I would send it for
> review in case someone else can think of a failure case, or has a better
> solution.
> 
>  eclass/systemd.eclass | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
> index 81065a0af79a..f6d1fa2d92d6 100644
> --- a/eclass/systemd.eclass
> +++ b/eclass/systemd.eclass
> @@ -50,6 +50,7 @@ _systemd_get_dir() {
>  
>  	if $(tc-getPKG_CONFIG) --exists systemd; then
>  		d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die
> +		d=${d#${SYSROOT}}
>  		d=${d#${EPREFIX}}
>  	else
>  		d=${fallback}

I was going to say this is not the best approach as it would be better
to tell pkg-config to not add the SYSROOT in the first place. I now
realise we have shot ourselves in the foot with cross-pkg-config as it
uses SYSROOT to set both PKG_CONFIG_SYSROOT_DIR to control the output
and PKG_CONFIG_LIBDIR to find the .pc files in the first place. I have
had prefix-related fixes for cross-pkg-config lined up for over a year
but unfortunately they do not address this. Trying to accommodate this
use case would probably just make it more confusing though so maybe
your approach is best after all.

The EPREFIX line is (sometimes) wrong in EAPI 7 though and the same goes
for udev.eclass. It took a while to get this agreed and corrected in
PMS but if SYSROOT points to / then the effective prefix is BROOT, not
EPREFIX; they may not be the same. If you just strip ESYSROOT then
it will always do the right thing but you'll need this fall back for
older EAPIs. I'm not sure why you didn't do it in one line? I forget if
EPREFIX is normalised to be / rather thank blank.

d=${d#${SYSROOT%/}/${EPREFIX#/}}

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-03 12:52 ` James Le Cuirot
@ 2021-01-03 13:02   ` James Le Cuirot
  2021-01-03 15:16   ` Mike Gilbert
  1 sibling, 0 replies; 9+ messages in thread
From: James Le Cuirot @ 2021-01-03 13:02 UTC (permalink / raw
  To: gentoo-dev

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

On Sun, 3 Jan 2021 12:52:08 +0000
James Le Cuirot <chewi@gentoo.org> wrote:

> On Sat,  2 Jan 2021 20:09:04 -0500
> Mike Gilbert <floppym@gentoo.org> wrote:
> 
> > When cross-compiling, users will typically have
> > PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper.
> > 
> > When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config
> > output get prefixed with this value.
> > 
> > Signed-off-by: Mike Gilbert <floppym@gentoo.org>
> > ---
> > 
> > This patch has already been pushed, but I figured I would send it for
> > review in case someone else can think of a failure case, or has a better
> > solution.
> > 
> >  eclass/systemd.eclass | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
> > index 81065a0af79a..f6d1fa2d92d6 100644
> > --- a/eclass/systemd.eclass
> > +++ b/eclass/systemd.eclass
> > @@ -50,6 +50,7 @@ _systemd_get_dir() {
> >  
> >  	if $(tc-getPKG_CONFIG) --exists systemd; then
> >  		d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die
> > +		d=${d#${SYSROOT}}
> >  		d=${d#${EPREFIX}}
> >  	else
> >  		d=${fallback}  
> 
> I was going to say this is not the best approach as it would be better
> to tell pkg-config to not add the SYSROOT in the first place. I now
> realise we have shot ourselves in the foot with cross-pkg-config as it
> uses SYSROOT to set both PKG_CONFIG_SYSROOT_DIR to control the output
> and PKG_CONFIG_LIBDIR to find the .pc files in the first place. I have
> had prefix-related fixes for cross-pkg-config lined up for over a year
> but unfortunately they do not address this. Trying to accommodate this
> use case would probably just make it more confusing though so maybe
> your approach is best after all.
> 
> The EPREFIX line is (sometimes) wrong in EAPI 7 though and the same goes
> for udev.eclass. It took a while to get this agreed and corrected in
> PMS but if SYSROOT points to / then the effective prefix is BROOT, not
> EPREFIX; they may not be the same. If you just strip ESYSROOT then
> it will always do the right thing but you'll need this fall back for
> older EAPIs. I'm not sure why you didn't do it in one line? I forget if
> EPREFIX is normalised to be / rather thank blank.
> 
> d=${d#${SYSROOT%/}/${EPREFIX#/}}

Had another minute to think. EPREFIX always strips the trailing slash
so it would be blank.

d=${d#${SYSROOT%/}${EPREFIX}}

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-03 12:52 ` James Le Cuirot
  2021-01-03 13:02   ` James Le Cuirot
@ 2021-01-03 15:16   ` Mike Gilbert
  2021-01-04 23:18     ` James Le Cuirot
  1 sibling, 1 reply; 9+ messages in thread
From: Mike Gilbert @ 2021-01-03 15:16 UTC (permalink / raw
  To: Gentoo Dev

On Sun, Jan 3, 2021 at 7:52 AM James Le Cuirot <chewi@gentoo.org> wrote:
>
> On Sat,  2 Jan 2021 20:09:04 -0500
> Mike Gilbert <floppym@gentoo.org> wrote:
>
> > When cross-compiling, users will typically have
> > PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper.
> >
> > When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config
> > output get prefixed with this value.
> >
> > Signed-off-by: Mike Gilbert <floppym@gentoo.org>
> > ---
> >
> > This patch has already been pushed, but I figured I would send it for
> > review in case someone else can think of a failure case, or has a better
> > solution.
> >
> >  eclass/systemd.eclass | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
> > index 81065a0af79a..f6d1fa2d92d6 100644
> > --- a/eclass/systemd.eclass
> > +++ b/eclass/systemd.eclass
> > @@ -50,6 +50,7 @@ _systemd_get_dir() {
> >
> >       if $(tc-getPKG_CONFIG) --exists systemd; then
> >               d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die
> > +             d=${d#${SYSROOT}}
> >               d=${d#${EPREFIX}}
> >       else
> >               d=${fallback}
>
> I was going to say this is not the best approach as it would be better
> to tell pkg-config to not add the SYSROOT in the first place. I now
> realise we have shot ourselves in the foot with cross-pkg-config as it
> uses SYSROOT to set both PKG_CONFIG_SYSROOT_DIR to control the output
> and PKG_CONFIG_LIBDIR to find the .pc files in the first place. I have
> had prefix-related fixes for cross-pkg-config lined up for over a year
> but unfortunately they do not address this. Trying to accommodate this
> use case would probably just make it more confusing though so maybe
> your approach is best after all.

It would be cleaner overall if we could prevent SYSROOT from being
added to the paths in the first place.

> The EPREFIX line is (sometimes) wrong in EAPI 7 though and the same goes
> for udev.eclass. It took a while to get this agreed and corrected in
> PMS but if SYSROOT points to / then the effective prefix is BROOT, not
> EPREFIX; they may not be the same.

Ugh, that "corrected" logic in PMS head is nasty.

> If you just strip ESYSROOT then
> it will always do the right thing but you'll need this fall back for
> older EAPIs. I'm not sure why you didn't do it in one line?

I was trying to accommodate older EAPIs that do not define ESYSROOT.
This seemed like the simplest approach. It also allows for the case
where someone is not using cross-pkg-config, and
PKG_CONFIG_SYSROOT_DIR is unset. Since SYSROOT and EPREFIX are removed
separately, if one of them is already missing, it will just be
skipped.

> I forget if EPREFIX is normalised to be / rather thank blank.
>
> d=${d#${SYSROOT%/}/${EPREFIX#/}}

SYSROOT never ends with a / in modern versions of portage (regardless
of EAPI), so there's currently no need to remove a trailing slash.

https://gitweb.gentoo.org/proj/portage.git/commit/?id=1b5110557d1dd725f7c12bbed4b7ceaaec29f2a3

I have never seen EPREFIX equal to "/", or end with a trailing slash.
Given it is most commonly used as "${EPREFIX}/usr", this would result
in double-slashes everywhere.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-03 15:16   ` Mike Gilbert
@ 2021-01-04 23:18     ` James Le Cuirot
  2021-01-04 23:45       ` Mike Gilbert
  0 siblings, 1 reply; 9+ messages in thread
From: James Le Cuirot @ 2021-01-04 23:18 UTC (permalink / raw
  To: gentoo-dev

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

On Sun, 3 Jan 2021 10:16:49 -0500
Mike Gilbert <floppym@gentoo.org> wrote:

> On Sun, Jan 3, 2021 at 7:52 AM James Le Cuirot <chewi@gentoo.org> wrote:
> >
> > On Sat,  2 Jan 2021 20:09:04 -0500
> > Mike Gilbert <floppym@gentoo.org> wrote:
> >  
> > > When cross-compiling, users will typically have
> > > PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper.
> > >
> > > When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config
> > > output get prefixed with this value.
> > >
> > > Signed-off-by: Mike Gilbert <floppym@gentoo.org>
> > > ---
> > >
> > > This patch has already been pushed, but I figured I would send it for
> > > review in case someone else can think of a failure case, or has a better
> > > solution.
> > >
> > >  eclass/systemd.eclass | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass
> > > index 81065a0af79a..f6d1fa2d92d6 100644
> > > --- a/eclass/systemd.eclass
> > > +++ b/eclass/systemd.eclass
> > > @@ -50,6 +50,7 @@ _systemd_get_dir() {
> > >
> > >       if $(tc-getPKG_CONFIG) --exists systemd; then
> > >               d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die
> > > +             d=${d#${SYSROOT}}
> > >               d=${d#${EPREFIX}}
> > >       else
> > >               d=${fallback}  
> 
> > The EPREFIX line is (sometimes) wrong in EAPI 7 though and the same goes
> > for udev.eclass. It took a while to get this agreed and corrected in
> > PMS but if SYSROOT points to / then the effective prefix is BROOT, not
> > EPREFIX; they may not be the same.  
> 
> Ugh, that "corrected" logic in PMS head is nasty.

I wish it could be simpler but that's just the nature of the beast. I
could give an example of why it matters, even in this context, but I
think you already know. There's some good news though...

> > If you just strip ESYSROOT then
> > it will always do the right thing but you'll need this fall back for
> > older EAPIs. I'm not sure why you didn't do it in one line?  
> 
> I was trying to accommodate older EAPIs that do not define ESYSROOT.
> This seemed like the simplest approach. It also allows for the case
> where someone is not using cross-pkg-config, and
> PKG_CONFIG_SYSROOT_DIR is unset. Since SYSROOT and EPREFIX are removed
> separately, if one of them is already missing, it will just be
> skipped.

I saw your new patch. I noticed that it sets the "d" variable but
doesn't make use of it. That aside, I thought perhaps I could come up
with something cleaner. That's when I made an amusing discovery.

$ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev      
/lib/udev

The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all.
And why would it be? The man page says that this variable is only
applied to -I and -L flags. I don't know for sure but I suspect that
pkg-config just sees this as some arbitrary variable with no special
path handling at all. I wonder what led you to think that this fix was
necessary?

That said, you'll still need special handling for EAPI 7. Unfortunately
there's no separate variable for the prefix alone but at least it's
simpler now. Something like this?

diff --git a/eclass/udev.eclass b/eclass/udev.eclass
index 2873ae9a92c3..f10ea0de01a2 100644
--- a/eclass/udev.eclass
+++ b/eclass/udev.eclass
@@ -50,12 +50,19 @@ fi
 # @DESCRIPTION:
 # Get unprefixed udevdir.
 _udev_get_udevdir() {
-       if $($(tc-getPKG_CONFIG) --exists udev); then
-               local udevdir="$($(tc-getPKG_CONFIG) --variable=udevdir udev)"
-               echo "${udevdir#${EPREFIX%/}}"
-       else
-               echo /lib/udev
+       local udevdir="/lib/udev"
+
+       if $(tc-getPKG_CONFIG) --exists udev; then
+               udevdir="$($(tc-getPKG_CONFIG) --variable=udevdir udev)" || die
+
+               if [[ ${EAPI:-0} == [0123456] ]]; then
+                       udevdir=${udevdir#${EPREFIX}}
+               else
+                       udevdir=${udevdir#${ESYSROOT#${SYSROOT}}}
+               fi
        fi
+
+       echo "${udevdir}"
 }

One last question. Why is this dynamic at all? Shouldn't it just be
hardcoded to /lib/udev? Sure, a user could patch udev to make it
something different if they really wanted but there are plenty of other
paths we just assume. What makes this one special?

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-04 23:18     ` James Le Cuirot
@ 2021-01-04 23:45       ` Mike Gilbert
  2021-01-05  0:28         ` Mike Gilbert
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Gilbert @ 2021-01-04 23:45 UTC (permalink / raw
  To: Gentoo Dev

On Mon, Jan 4, 2021 at 6:18 PM James Le Cuirot <chewi@gentoo.org> wrote:
> $ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> /lib/udev
>
> The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all.
> And why would it be? The man page says that this variable is only
> applied to -I and -L flags. I don't know for sure but I suspect that
> pkg-config just sees this as some arbitrary variable with no special
> path handling at all. I wonder what led you to think that this fix was
> necessary?

Interesting!

pkg-config behaves differently on my system:

% PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
/foo/lib/udev

This appears to be a difference in behavior between dev-util/pkgconfig
and dev-util/pkgconf. I am using pkgconf, and I would guess you are
using pkgconfig.

I guess I will ask pkgconf upstream for help on this; it seems like
this is probably an unintended behavior.

> One last question. Why is this dynamic at all? Shouldn't it just be
> hardcoded to /lib/udev? Sure, a user could patch udev to make it
> something different if they really wanted but there are plenty of other
> paths we just assume. What makes this one special?

sys-apps/systemd has a USE flag called "split-usr". This is meant to
allow users to perform a /usr merge if desired. When split-usr is
disabled, udevdir becomes /usr/lib/udev instead of /lib/udev.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-04 23:45       ` Mike Gilbert
@ 2021-01-05  0:28         ` Mike Gilbert
  2021-01-06 22:47           ` James Le Cuirot
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Gilbert @ 2021-01-05  0:28 UTC (permalink / raw
  To: Gentoo Dev

On Mon, Jan 4, 2021 at 6:45 PM Mike Gilbert <floppym@gentoo.org> wrote:
>
> On Mon, Jan 4, 2021 at 6:18 PM James Le Cuirot <chewi@gentoo.org> wrote:
> > $ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> > /lib/udev
> >
> > The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all.
> > And why would it be? The man page says that this variable is only
> > applied to -I and -L flags. I don't know for sure but I suspect that
> > pkg-config just sees this as some arbitrary variable with no special
> > path handling at all. I wonder what led you to think that this fix was
> > necessary?
>
> Interesting!
>
> pkg-config behaves differently on my system:
>
> % PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> /foo/lib/udev
>
> This appears to be a difference in behavior between dev-util/pkgconfig
> and dev-util/pkgconf. I am using pkgconf, and I would guess you are
> using pkgconfig.
>
> I guess I will ask pkgconf upstream for help on this; it seems like
> this is probably an unintended behavior.

It seems that the pkgconf behavior is intentional.

https://github.com/pkgconf/pkgconf/issues/69

I opened an issue to see if we can get some kind of opt-out.

https://github.com/pkgconf/pkgconf/issues/205


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-05  0:28         ` Mike Gilbert
@ 2021-01-06 22:47           ` James Le Cuirot
  2021-01-06 23:18             ` Mike Gilbert
  0 siblings, 1 reply; 9+ messages in thread
From: James Le Cuirot @ 2021-01-06 22:47 UTC (permalink / raw
  To: gentoo-dev

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

On Mon, 4 Jan 2021 19:28:37 -0500
Mike Gilbert <floppym@gentoo.org> wrote:

> On Mon, Jan 4, 2021 at 6:45 PM Mike Gilbert <floppym@gentoo.org> wrote:
> >
> > On Mon, Jan 4, 2021 at 6:18 PM James Le Cuirot <chewi@gentoo.org> wrote:  
> > > $ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> > > /lib/udev
> > >
> > > The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all.
> > > And why would it be? The man page says that this variable is only
> > > applied to -I and -L flags. I don't know for sure but I suspect that
> > > pkg-config just sees this as some arbitrary variable with no special
> > > path handling at all. I wonder what led you to think that this fix was
> > > necessary?  
> >
> > Interesting!
> >
> > pkg-config behaves differently on my system:
> >
> > % PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> > /foo/lib/udev
> >
> > This appears to be a difference in behavior between dev-util/pkgconfig
> > and dev-util/pkgconf. I am using pkgconf, and I would guess you are
> > using pkgconfig.
> >
> > I guess I will ask pkgconf upstream for help on this; it seems like
> > this is probably an unintended behavior.  
> 
> It seems that the pkgconf behavior is intentional.
> 
> https://github.com/pkgconf/pkgconf/issues/69
> 
> I opened an issue to see if we can get some kind of opt-out.
> 
> https://github.com/pkgconf/pkgconf/issues/205

Hmmm. At this point, I'm thinking maybe we should just address this in
cross-pkg-config. It seems unfair to ask upstream to accommodate this
when the tool is just doing what we asked it to. Perhaps it could
respect PKG_CONFIG_SYSROOT_DIR if it is already set, even when empty.
It wouldn't allow to you set this differently for the build host but
you shouldn't ever have to. I think I'd prefer this over adding yet
another confusing variable. The same could be applied to
PKG_CONFIG_LIBDIR and PKG_CONFIG_SYSTEM_LIBRARY_PATH for consistency.
What do you think?

On a side note, I think that what cross-pkg-config does should be part
of Portage or something rather than part of crossdev because there's
nothing really cross-compile-specific about it. You can set SYSROOT
when building natively, although that tends to run into even more
issues than you get with cross.

-- 
James Le Cuirot (chewi)
Gentoo Linux Developer

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output
  2021-01-06 22:47           ` James Le Cuirot
@ 2021-01-06 23:18             ` Mike Gilbert
  0 siblings, 0 replies; 9+ messages in thread
From: Mike Gilbert @ 2021-01-06 23:18 UTC (permalink / raw
  To: Gentoo Dev

On Wed, Jan 6, 2021 at 5:47 PM James Le Cuirot <chewi@gentoo.org> wrote:
>
> On Mon, 4 Jan 2021 19:28:37 -0500
> Mike Gilbert <floppym@gentoo.org> wrote:
>
> > On Mon, Jan 4, 2021 at 6:45 PM Mike Gilbert <floppym@gentoo.org> wrote:
> > >
> > > On Mon, Jan 4, 2021 at 6:18 PM James Le Cuirot <chewi@gentoo.org> wrote:
> > > > $ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> > > > /lib/udev
> > > >
> > > > The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all.
> > > > And why would it be? The man page says that this variable is only
> > > > applied to -I and -L flags. I don't know for sure but I suspect that
> > > > pkg-config just sees this as some arbitrary variable with no special
> > > > path handling at all. I wonder what led you to think that this fix was
> > > > necessary?
> > >
> > > Interesting!
> > >
> > > pkg-config behaves differently on my system:
> > >
> > > % PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev
> > > /foo/lib/udev
> > >
> > > This appears to be a difference in behavior between dev-util/pkgconfig
> > > and dev-util/pkgconf. I am using pkgconf, and I would guess you are
> > > using pkgconfig.
> > >
> > > I guess I will ask pkgconf upstream for help on this; it seems like
> > > this is probably an unintended behavior.
> >
> > It seems that the pkgconf behavior is intentional.
> >
> > https://github.com/pkgconf/pkgconf/issues/69
> >
> > I opened an issue to see if we can get some kind of opt-out.
> >
> > https://github.com/pkgconf/pkgconf/issues/205
>
> Hmmm. At this point, I'm thinking maybe we should just address this in
> cross-pkg-config. It seems unfair to ask upstream to accommodate this
> when the tool is just doing what we asked it to. Perhaps it could
> respect PKG_CONFIG_SYSROOT_DIR if it is already set, even when empty.
> It wouldn't allow to you set this differently for the build host but
> you shouldn't ever have to. I think I'd prefer this over adding yet
> another confusing variable. The same could be applied to
> PKG_CONFIG_LIBDIR and PKG_CONFIG_SYSTEM_LIBRARY_PATH for consistency.
> What do you think?

I like the idea of having cross-pkg-config respect
PKG_CONFIG_SYSROOT_DIR from the calling environment (even if it is
empty). That's just a more flexible design overall.

However, I think there's a fundamental design conflict here. Prefixing
all pkgconfig variables that happen to start with a slash is
problematic, and the desired result depends on the context in which it
will be used.

If the result is going to be used to find some existing file in
SYSROOT (like the SDK example included in issue 69 from above), then
we want it to be prefixed with SYSROOT.

If the result is going to be used to install new files, we don't want
SYSROOT in the result. The package manager is responsible for
prefixing the paths with ROOT when merging the files.

We could apply workarounds in ebuilds/eclasses to make this
distinction in Gentoo by setting PKG_CONFIG_SYSROOT_DIR selectively.
However, I wonder if there is a workable solution to this that could
be applied in upstream projects.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-06 23:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-03  1:09 [gentoo-dev] [PATCH] systemd.eclass: remove SYSROOT from pkg-config output Mike Gilbert
2021-01-03 12:52 ` James Le Cuirot
2021-01-03 13:02   ` James Le Cuirot
2021-01-03 15:16   ` Mike Gilbert
2021-01-04 23:18     ` James Le Cuirot
2021-01-04 23:45       ` Mike Gilbert
2021-01-05  0:28         ` Mike Gilbert
2021-01-06 22:47           ` James Le Cuirot
2021-01-06 23:18             ` Mike Gilbert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox