1 |
On Sun, 3 Jan 2021 10:16:49 -0500 |
2 |
Mike Gilbert <floppym@g.o> wrote: |
3 |
|
4 |
> On Sun, Jan 3, 2021 at 7:52 AM James Le Cuirot <chewi@g.o> wrote: |
5 |
> > |
6 |
> > On Sat, 2 Jan 2021 20:09:04 -0500 |
7 |
> > Mike Gilbert <floppym@g.o> wrote: |
8 |
> > |
9 |
> > > When cross-compiling, users will typically have |
10 |
> > > PKG_CONFIG_SYSROOT=${SYSROOT} defined via pkg-config wrapper. |
11 |
> > > |
12 |
> > > When PKG_CONFIG_SYSROOT is set, all paths included in pkg-config |
13 |
> > > output get prefixed with this value. |
14 |
> > > |
15 |
> > > Signed-off-by: Mike Gilbert <floppym@g.o> |
16 |
> > > --- |
17 |
> > > |
18 |
> > > This patch has already been pushed, but I figured I would send it for |
19 |
> > > review in case someone else can think of a failure case, or has a better |
20 |
> > > solution. |
21 |
> > > |
22 |
> > > eclass/systemd.eclass | 1 + |
23 |
> > > 1 file changed, 1 insertion(+) |
24 |
> > > |
25 |
> > > diff --git a/eclass/systemd.eclass b/eclass/systemd.eclass |
26 |
> > > index 81065a0af79a..f6d1fa2d92d6 100644 |
27 |
> > > --- a/eclass/systemd.eclass |
28 |
> > > +++ b/eclass/systemd.eclass |
29 |
> > > @@ -50,6 +50,7 @@ _systemd_get_dir() { |
30 |
> > > |
31 |
> > > if $(tc-getPKG_CONFIG) --exists systemd; then |
32 |
> > > d=$($(tc-getPKG_CONFIG) --variable="${variable}" systemd) || die |
33 |
> > > + d=${d#${SYSROOT}} |
34 |
> > > d=${d#${EPREFIX}} |
35 |
> > > else |
36 |
> > > d=${fallback} |
37 |
> |
38 |
> > The EPREFIX line is (sometimes) wrong in EAPI 7 though and the same goes |
39 |
> > for udev.eclass. It took a while to get this agreed and corrected in |
40 |
> > PMS but if SYSROOT points to / then the effective prefix is BROOT, not |
41 |
> > EPREFIX; they may not be the same. |
42 |
> |
43 |
> Ugh, that "corrected" logic in PMS head is nasty. |
44 |
|
45 |
I wish it could be simpler but that's just the nature of the beast. I |
46 |
could give an example of why it matters, even in this context, but I |
47 |
think you already know. There's some good news though... |
48 |
|
49 |
> > If you just strip ESYSROOT then |
50 |
> > it will always do the right thing but you'll need this fall back for |
51 |
> > older EAPIs. I'm not sure why you didn't do it in one line? |
52 |
> |
53 |
> I was trying to accommodate older EAPIs that do not define ESYSROOT. |
54 |
> This seemed like the simplest approach. It also allows for the case |
55 |
> where someone is not using cross-pkg-config, and |
56 |
> PKG_CONFIG_SYSROOT_DIR is unset. Since SYSROOT and EPREFIX are removed |
57 |
> separately, if one of them is already missing, it will just be |
58 |
> skipped. |
59 |
|
60 |
I saw your new patch. I noticed that it sets the "d" variable but |
61 |
doesn't make use of it. That aside, I thought perhaps I could come up |
62 |
with something cleaner. That's when I made an amusing discovery. |
63 |
|
64 |
$ PKG_CONFIG_SYSROOT_DIR=/foo pkg-config --variable=udevdir udev |
65 |
/lib/udev |
66 |
|
67 |
The udevdir variable is not affected by PKG_CONFIG_SYSROOT_DIR at all. |
68 |
And why would it be? The man page says that this variable is only |
69 |
applied to -I and -L flags. I don't know for sure but I suspect that |
70 |
pkg-config just sees this as some arbitrary variable with no special |
71 |
path handling at all. I wonder what led you to think that this fix was |
72 |
necessary? |
73 |
|
74 |
That said, you'll still need special handling for EAPI 7. Unfortunately |
75 |
there's no separate variable for the prefix alone but at least it's |
76 |
simpler now. Something like this? |
77 |
|
78 |
diff --git a/eclass/udev.eclass b/eclass/udev.eclass |
79 |
index 2873ae9a92c3..f10ea0de01a2 100644 |
80 |
--- a/eclass/udev.eclass |
81 |
+++ b/eclass/udev.eclass |
82 |
@@ -50,12 +50,19 @@ fi |
83 |
# @DESCRIPTION: |
84 |
# Get unprefixed udevdir. |
85 |
_udev_get_udevdir() { |
86 |
- if $($(tc-getPKG_CONFIG) --exists udev); then |
87 |
- local udevdir="$($(tc-getPKG_CONFIG) --variable=udevdir udev)" |
88 |
- echo "${udevdir#${EPREFIX%/}}" |
89 |
- else |
90 |
- echo /lib/udev |
91 |
+ local udevdir="/lib/udev" |
92 |
+ |
93 |
+ if $(tc-getPKG_CONFIG) --exists udev; then |
94 |
+ udevdir="$($(tc-getPKG_CONFIG) --variable=udevdir udev)" || die |
95 |
+ |
96 |
+ if [[ ${EAPI:-0} == [0123456] ]]; then |
97 |
+ udevdir=${udevdir#${EPREFIX}} |
98 |
+ else |
99 |
+ udevdir=${udevdir#${ESYSROOT#${SYSROOT}}} |
100 |
+ fi |
101 |
fi |
102 |
+ |
103 |
+ echo "${udevdir}" |
104 |
} |
105 |
|
106 |
One last question. Why is this dynamic at all? Shouldn't it just be |
107 |
hardcoded to /lib/udev? Sure, a user could patch udev to make it |
108 |
something different if they really wanted but there are plenty of other |
109 |
paths we just assume. What makes this one special? |
110 |
|
111 |
-- |
112 |
James Le Cuirot (chewi) |
113 |
Gentoo Linux Developer |