Hi Hartmut, > Please but this int a new line - makeing it easier to copy & paste. > (Leading emptry lines doe nor effect "from __future__ import"). adopted Ricardo’s suggestion here. > This looks very uncommon (and make my mind hooking on it). Please use > this, which is more common and less mindbogling ;-) Done. > Any reason for converting the req into a string first? IC, > "`requirements` must be a string". So I'd move the "str()" to the > function call: Yes, the string is used in the error message. > if group not in {'console_scripts', 'gui_scripts'}: > Why not gui-scripts? > If not adding gui-scripts, just use "if group != 'concolse_scrips':" I wasn’t aware this exists. Added, thanks! > Does it make sence to try loading the top-level modules *after* the the > entry-point? Chances are very high that the entry-points implicitly test > the top-level modules already. > IMHO it would make more sense to first try the top-level modules, and > even stop processing if this fails. True, I reversed the order. Still, I think continuing with the entry points might be worth, if it points out more (different) errors. For small packages this might not be the case, but larger ones often only re-export specific names in the top-level module, thus testing might reveal more issues. > Please add a short explanation why there can be unavailable top-level > modules. Done. > While not having antoher idea, I'm not happy with this name. "loadable" > is not easy to get. (See also below, where this term is used in commit message.) > top-level-modules-are-importable? I’m also not happy with the name, but can’t think of a better one right now. Anyone? > AFAIKS the packages only differ by some requirements. So I suggest > using a function to return the packages. This makes it easier to spot > the actull differences. > […] > (mkdir-p "src/dummy") > (chdir "src") Done. > I would strip this down (version is not required AFAIK) and put it one > line (her assuming you use (format) for creating the code within a function: Not sure whether it’s optional or not, [1] does not say it is. No harm in keeping it? [1] https://setuptools.readthedocs.io/en/latest/references/keywords.html?highlight=version#keywords > Arguments are not used? Fixed. > Any reason for relaxing this? Why not use python-pytest-6 as input? Yes, our pytest@6 package reports its version as 0.0.0, so this patch is required with either package. Or we figure out how to fix pytest@6 (works fine with PEP 517 compliant build system). > Please rephrase into something like > Do not validate loadability Done. > Dosn't this change fix the checks? So this comment would be obsoltes and > "#:tests #t" can be removed. Should’ve been #f, sorry, fixed. > I suggest keeping the old way, as this is will automatically update to > upstream changes. Okay, I’ve added another phase that disables the test manually, because it fails for me every time. See attached patchset (v2) or git repository for updated patches. I’ve also rebuilt all 2284 packages depending on python-build-system. Currently 426 of them fail to build for any reason (not necessarily caused by this patchset; I’ve seen some unrelated issues). Due to the large number of packages failing I suggest moving forward with applying this first batch and then fixing everything else incrementally, unless some major package is broken (see attached list). WDYT? Cheers, Lars