1 |
On Mon, 2019-11-25 at 20:39 +0200, Joonas Niilola wrote: |
2 |
> Hey, |
3 |
> |
4 |
> |
5 |
> On 11/25/19 7:38 PM, Michał Górny wrote: |
6 |
> > Hi, |
7 |
> > |
8 |
> > TL;DR: should we depend on setuptools by default? Alternatively, should |
9 |
> > we add distutils_enable_setuptools API to provide at least partial |
10 |
> > validity checks. |
11 |
> > |
12 |
> > |
13 |
> > The problem |
14 |
> > =========== |
15 |
> > The vast majority of Python packages nowadays uses setuptools as their |
16 |
> > build system. According to a cheap grep, 1633 out of 2415 packages |
17 |
> > using distutils-r1 depends on it -- and I suspect the vast majority of |
18 |
> > those that do not are simply missing the dependency. |
19 |
> |
20 |
> Truthfully I thought distutils already (B)DEPENDed on setuptools, so at |
21 |
> least I've left it out "on purpose". I was recently made aware it |
22 |
> doesn't. No bug reports are made about it, so I think it's safe to |
23 |
> assume setuptools is already present in every system? |
24 |
|
25 |
Yes, we do get bug reports about it occassionally. |
26 |
|
27 |
> |
28 |
> |
29 |
> > Are there valid cases for not using setuptools? Notably, packages |
30 |
> > needed to bootstrap setuptools aren't using it. Plus some simple |
31 |
> > packages that really don't need its features. |
32 |
> > |
33 |
> > There are also packages that use setuptools but have distutils fallback. |
34 |
> > We don't like them because switching between build systems involves file |
35 |
> > <-> directory replacement that isn't handled cleanly by our package |
36 |
> > managers. Therefore, we want to always force one build system in them. |
37 |
> > |
38 |
> > The setuptools dependency is so common it's easy to miss. Notably, it's |
39 |
> > a prerequisite of specifying package dependencies, so it's normally not |
40 |
> > listed in dependencies. |
41 |
> > |
42 |
> > Finally, while most of the time setuptools is just BDEPEND, there are |
43 |
> > cases when it's RDEPEND as well. The most common is the use |
44 |
> > of entry_points -- and again, upstreams don't mention it explicitly. |
45 |
> > |
46 |
> > All that considered, I think we should work on providing a better API |
47 |
> > for depending on setuptools, to reduce the number of missing |
48 |
> > dependencies and make it possible to automatically test for them |
49 |
> > (reminder: PMS doesn't permit inspecting *DEPEND). |
50 |
> > |
51 |
> > |
52 |
> > Variant 1: automatic dependency on setuptools |
53 |
> > ============================================= |
54 |
> > Basically, we add a new trinary pre-inherit variable: |
55 |
> > |
56 |
> > DISTUTILS_USE_SETUPTOOLS=no |
57 |
> > -> no deps |
58 |
> > DISTUTILS_USE_SETUPTOOLS=bdepend |
59 |
> > -> add to BDEPEND (the default) |
60 |
> > DISTUTILS_USE_SETUPTOOLS=rdepend |
61 |
> > -> add to BDEPEND+RDEPEND |
62 |
> > |
63 |
> > This is roughly 'erring on the safe side'. The default will work for |
64 |
> > the majority of packages. We will have to disable it for setuptools |
65 |
> > bootstrap deps, and devs will be able to adjust it to correct values |
66 |
> > as they update ebuilds. For the time being, existing *DEPEND |
67 |
> > on setuptools will avoid breaking stuff. |
68 |
> > |
69 |
> > This will also enable me to add extra QA checks to esetup.py. It should |
70 |
> > be able to reasonably detect incorrect value and report it. This will |
71 |
> > imply some 'false positives' for packages that use the old method of |
72 |
> > specifying setuptools in RDEPEND but that's a minor hassle. |
73 |
> > |
74 |
> > Pros: |
75 |
> > - works out of the box for the majority of packages |
76 |
> > - enables full-range QA checking |
77 |
> > |
78 |
> > Cons: |
79 |
> > - pre-inherit variable |
80 |
> > - some (harmless) false positives on existing packages |
81 |
> |
82 |
> I'm a fan of this solution, unless it causes a lot of new CI noise. I |
83 |
> think people are getting fed up with recent changes already, starting to |
84 |
> ignore whatever CI bot says, unless it's fatal. |
85 |
> |
86 |
> No harm in double-defining setuptools in BDEPEND right...? (via eclass |
87 |
> and ebuild) |
88 |
|
89 |
Yes, double defining is fine. |
90 |
|
91 |
> |
92 |
> |
93 |
> > |
94 |
> > Variant 2: distutils_enable_setuptools |
95 |
> > ====================================== |
96 |
> > The alternative method is to add another function |
97 |
> > to the distutils_enable* series: |
98 |
> > |
99 |
> > distutils_enable_setuptools [-r] |
100 |
> > |
101 |
> > The basic form adds it to BDEPEND, -r B+RDEPEND. Of course, no dep is |
102 |
> > present by default. The main difference from setting deps explicitly is |
103 |
> > that it permits us to do a minimal QA check between pure BDEPEND |
104 |
> > and B+RDEPEND for now. |
105 |
> > |
106 |
> > When all ebuilds are migrated from explicit dependencies to this method, |
107 |
> > we can also start detecting missing deps completely. However, that |
108 |
> > presumes we require using this function rather than explicit deps. |
109 |
> > |
110 |
> > Pros: |
111 |
> > - no pre-inherit variables |
112 |
> > |
113 |
> > Cons: |
114 |
> > - only partial QA check possible for the time being |
115 |
> > - requires migrating all ebuilds long-term |
116 |
> |
117 |
> I don't like this at this point. This would just bring unwanted noise |
118 |
> and lots of editing in ebuilds. V1 and V3 are better IMHO. This could be |
119 |
> implemented in distutils-r2.eclass, though. |
120 |
> |
121 |
> |
122 |
> > |
123 |
> > Variant 3: leave as-is, add minimal install-qa-check.d |
124 |
> > ====================================================== |
125 |
> > We can just continue adding deps manually, and add a minimal install-qa- |
126 |
> > check.d that greps installed scripts for entry_points usage. This will |
127 |
> > let us detect missing RDEPEND on setuptools but not BDEPEND. |
128 |
> > |
129 |
> > Pros: |
130 |
> > - no changes to ebuilds |
131 |
> > |
132 |
> > Cons: |
133 |
> > - only partial QA check possible |
134 |
> |
135 |
> The old way, is also good. |
136 |
> |
137 |
> |
138 |
> > |
139 |
> > WDYT? |
140 |
> > ===== |
141 |
> > Both options have their pros and cons. I think V1 is the best since it |
142 |
> > avoids a common mistake, and gives full range QA check. It also doesn't |
143 |
> > interfere with existing deps. V2 neatly fits into the recent series but |
144 |
> > still requires users to remember to call it, and we can't report missing |
145 |
> > calls until we clean up all ebuilds. V3 provides only minimal QA |
146 |
> > improvement without changing the eclass. |
147 |
> > |
148 |
> > WDYT? Do you have any other ideas? |
149 |
> |
150 |
> As I said, I don't even know how one ends up with a system without |
151 |
> setuptools, so I don't view this "superimportant". Still I like your |
152 |
> suggestions 1 & 3 in handling it in future. However if you're going to |
153 |
> implement CI checks with this, I don't believe the timing is right |
154 |
> currently. |
155 |
> |
156 |
|
157 |
1. If you have no packages with RDEPEND on setuptools and some --with- |
158 |
bdeps setting, --depclean can kill it. |
159 |
|
160 |
2. Early after install. |
161 |
|
162 |
3. On tinderbox, if no dep pulled it in. |
163 |
|
164 |
-- |
165 |
Best regards, |
166 |
Michał Górny |