1 |
On 08/06/2012 01:20 PM, Michał Górny wrote: |
2 |
> Ok, a few comments from me. |
3 |
> |
4 |
> On Mon, 06 Aug 2012 13:00:08 +0300 |
5 |
> Samuli Suominen <ssuominen@g.o> wrote: |
6 |
> |
7 |
>> --- systemd.eclass 2012-08-06 12:49:20.532528219 +0300 |
8 |
>> +++ /tmp/systemd.eclass 2012-08-06 12:57:28.333542735 +0300 |
9 |
> |
10 |
> First of all, I'm not really convinced systemd.eclass is a good place |
11 |
> for this. |
12 |
> |
13 |
> The main reason for introducing a separate systemd.eclass was to have |
14 |
> a single place to control installing systemd unit files. The rule is |
15 |
> quite simple here: you install systemd unit files, you inherit |
16 |
> the eclass. |
17 |
> |
18 |
> Most importantly, this allows us to easily find out which packages |
19 |
> install such files and perform global operations on them. For example, |
20 |
> if a particular user had systemd locations in INSTALL_MASK and changed |
21 |
> his mind, he can easily update his system by rebuilding all packages |
22 |
> inheriting systemd.eclass. |
23 |
> |
24 |
> If all packages installing udev rules start inheriting it, the above |
25 |
> will no longer be correct. Also, the opposite way -- rebuilding |
26 |
> packages installing udev rules -- won't be that easy. |
27 |
> |
28 |
> That's not my call but I believe that putting those functions in |
29 |
> separate udev.eclass could be the best course of action, for the reason |
30 |
> stated above -- we can easily find out what installs them, and rebuild |
31 |
> it all at once. |
32 |
|
33 |
|
34 |
OK, we can make it udev.eclass and then we can add the virtual/pkgconfig |
35 |
and sys-fs/udev dependency of it. |
36 |
|
37 |
|
38 |
> |
39 |
>> @@ -25,6 +25,8 @@ |
40 |
>> # } |
41 |
>> # @CODE |
42 |
>> |
43 |
>> +inherit toolchain-funcs |
44 |
>> + |
45 |
>> case ${EAPI:-0} in |
46 |
>> 0|1|2|3|4) ;; |
47 |
>> *) die "${ECLASS}.eclass API in EAPI ${EAPI} not yet |
48 |
>> established." @@ -34,6 +36,31 @@ |
49 |
>> DEPEND="!<sys-apps/systemd-29-r4 |
50 |
>> !=sys-apps/systemd-37-r1" |
51 |
>> |
52 |
>> +# @FUNCTION: _systemd_get_udevdir |
53 |
>> +# @INTERNAL |
54 |
>> +# @DESCRIPTION: |
55 |
>> +# Get unprefixed udevdir. |
56 |
>> +_systemd_get_udevdir() { |
57 |
>> + if $($(tc-getPKG_CONFIG) --exists udev); then |
58 |
>> + echo -n "$($(tc-getPKG_CONFIG) --variable=udevdir |
59 |
>> udev)" |
60 |
> |
61 |
> Secondly, I believe you shouldn't do such a thing *without* depending |
62 |
> on udev. Even though there is fallback here, there is another problem: |
63 |
> you are introducing a *potential inconsistency* between built packages, |
64 |
> depending on contents of udev.pc. Thus, I believe the package should |
65 |
> depend on the package providing it so that the dependency tree is |
66 |
> complete. |
67 |
|
68 |
OK, I can agree with udev.eclass as I stated before already. |
69 |
|
70 |
> |
71 |
> Also, I'm not so sure if this will work correctly for Prefix. |
72 |
|
73 |
I'm sure that is easily checked and we will get feedback quickly. |
74 |
|
75 |
> |
76 |
>> + else |
77 |
>> + echo -n /lib/udev |
78 |
>> + fi |
79 |
>> +} |
80 |
> |
81 |
> And finally, I believe we shouldn't even be making the install location |
82 |
> variable. |
83 |
|
84 |
It's the upstream way to determine the path and is used treewide by |
85 |
upstream configure scripts etc. for various packages already. |
86 |
|
87 |
> Right now, the Gentoo location for udev rules is /lib/udev, |
88 |
> and I believe William will agree with me on this. We hadn't decided on |
89 |
> moving them, and thus all udev and systemd versions in the tree *will* |
90 |
> support /lib/udev (I still need to commit patched systemd). |
91 |
> |
92 |
> If we decide to move rules to /usr/lib/udev, I believe we will move |
93 |
> them all at once. And then all users will have to use a newer |
94 |
> (or patched) udev supporting the new location (or both). In order to |
95 |
> enforce that, the eclass would have to block old udev versions (like |
96 |
> the systemd.eclass blocks old systemd versions). |
97 |
|
98 |
The udevdir is whatever udev.pc determines it to be, and currently in |
99 |
~arch it's set to /usr/lib/udev |
100 |
Therefore the Gentoo udevdir is also /usr/lib/udev for ~arch |
101 |
|
102 |
As in, packages in tree are using it already, independent of getting |
103 |
this helper function to tree or not. |
104 |
|
105 |
> |
106 |
> Making the install location dynamic is just asking for trouble. |
107 |
> Consider the following: user installs new udev, rebuilds package, then |
108 |
> downgrades udev. Package rules no longer work. That's what we would |
109 |
> like to avoid. |
110 |
|
111 |
Downgrading is never a good idea when you don't know what you are doing. |
112 |
|
113 |
> |
114 |
>> + |
115 |
>> +# @FUNCTION: systemd_get_udevdir |
116 |
>> +# @DESCRIPTION: |
117 |
>> +# Output the path for the udev directory (not including ${D}). |
118 |
>> +# This function always succeeds, even if udev is not installed. |
119 |
>> +# Dependencies need to include sys-fs/udev or otherwise |
120 |
>> +# the fallback return value is /lib/udev. |
121 |
>> +systemd_get_udevdir() { |
122 |
>> + has "${EAPI:-0}" 0 1 2 && ! use prefix && EPREFIX= |
123 |
>> + debug-print-function ${FUNCNAME} "${@}" |
124 |
>> + |
125 |
>> + echo -n "${EPREFIX}$(_systemd_get_udevdir)" |
126 |
>> +} |
127 |
>> + |
128 |
>> # @FUNCTION: _systemd_get_unitdir |
129 |
>> # @INTERNAL |
130 |
>> # @DESCRIPTION: |
131 |
> |
132 |
> As a final note, please note that this is mostly my opinion as systemd |
133 |
> maintainer. I believe William has a final word on udev itself. |
134 |
|
135 |
Summarizing: |
136 |
|
137 |
- Will call this udev.eclass and add the dependencies for udev and |
138 |
pkg-config |
139 |
|
140 |
- Gentoo's udevdir is already set to /usr/lib/udev by |
141 |
>=sys-fs/udev-187-r1 in Portage, and is widely queried by upstream |
142 |
configure scripts |
143 |
We should leave it like it is now, and get this helper function ASAP in |
144 |
tree to get things consistent again |
145 |
|
146 |
- Samuli |