Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPECPARTS dir in %_builddir/%buildsubdir is leaking to setuptools package discovery #2532

Closed
hroncok opened this issue Jun 5, 2023 · 13 comments
Milestone

Comments

@hroncok
Copy link
Contributor

hroncok commented Jun 5, 2023

The SPECPARTS directory is leaking into setuptools package discovery. When upstream Python projects choose to use automatic Python package discovery by setuptools, SPECPARTS is considered a Python package (because empty directories actually are Python packages) and when not explicitly excluded, it makes setuptools die with:

...
discovered packages -- ['pgactivity', 'SPECPARTS', 'pgactivity.queries']
Traceback (most recent call last):
...
  File "/usr/lib/python3.11/site-packages/setuptools/discovery.py", line 441, in _analyse_flat_layout
    return self._analyse_flat_packages() or self._analyse_flat_modules()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/setuptools/discovery.py", line 447, in _analyse_flat_packages
    self._ensure_no_accidental_inclusion(top_level, "packages")
  File "/usr/lib/python3.11/site-packages/setuptools/discovery.py", line 477, in _ensure_no_accidental_inclusion
    raise PackageDiscoveryError(cleandoc(msg))
setuptools.errors.PackageDiscoveryError: Multiple top-level packages discovered in a flat-layout: ['SPECPARTS', 'pgactivity'].

(Full traceback at dalibo/pg_activity#378 (comment))

I suppose other upstreams might consider a new directory in $PWD something to automatically consider important.


Could this directory either be moved outside of %_builddir/%buildsubdir or at least be hidden (e.g. .SPECPARTS)? Thanks

@ffesti
Copy link
Contributor

ffesti commented Jun 6, 2023

Yeah, I see this is a problem. The fact that we don't really have a proper directory for the build is really not great. We basically rely on the source tarball providing a directory (*). Having the directory beside the buildsubdir is probably the best solution for now until we move the whole thing one dir down (which is unlikely to happen in 4.19).

*) details apply

@pmatilai
Copy link
Member

pmatilai commented Jun 6, 2023

(see #2078 for further background)

@hroncok
Copy link
Contributor Author

hroncok commented Jun 6, 2023

Another Fedora package is known to be impacted. python-quantities: https://bugzilla.redhat.com/show_bug.cgi?id=2213013

@pmatilai
Copy link
Member

pmatilai commented Jun 7, 2023

Repeating this here to attach it to the issue rather than a specific PR:

An easy solution would be letting a package override the path. That allows the handful of special cases to handle it on spec level while leaving it loud and clear for the others. Those overriding it are obviously aware of the magic it entails so that's not a problem.

Currently the macro thing is kinda backwards in that the directory is hardcoded in C and then exposed as a macro, when it should be the other way around.

@pmatilai pmatilai moved this from Backlog to Todo in RPM Jun 7, 2023
@hroncok
Copy link
Contributor Author

hroncok commented Jun 7, 2023

An easy solution would be letting a package override the path. That allows the handful of special cases to handle it on spec level while leaving it loud and clear for the others...

What would be the advice for packages affected by the setuptools package discovery issue? Putting %global specpartsdir .SPECPARTS in the spec? Do we want dozens of specs with that?

@dmnks
Copy link
Contributor

dmnks commented Jun 7, 2023

I wonder if we could make it the default value for now (i.e. in the spirit of #2533 basically)?

@hroncok
Copy link
Contributor Author

hroncok commented Jun 7, 2023

In our testing Copr for Python 3.12, the r"Multiple top-level packages discovered in a flat-layout: \[.*'SPECPARTS'" regex was found in:

  • python-uc-micro-py
  • python-linkify-it-py
  • python-nose2

@pmatilai
Copy link
Member

pmatilai commented Jun 7, 2023

I don't know. Three affected packages is a handful where a workaround seems acceptable, if it's dozens of packages then less so. Unless it could be hidden in the surrounding helper macros.

@ffesti
Copy link
Contributor

ffesti commented Jun 7, 2023

#2534 would solve that issue by moving the SPECPARTS dir one dir up. It's only one char different for moving it back in. But so far I don't quite see a drawback for doing it this way except there is now one more way how builds of the same package can step on each others toe.

@dlax
Copy link

dlax commented Jun 7, 2023

Three affected packages is a handful where a workaround seems acceptable, if it's dozens of packages then less so.

"Automatic discovery" in setuptools is quite new (v61.0.0, 24 Mar 2022) and still in beta; that's probably why so few packages use it for now.

@pmatilai
Copy link
Member

pmatilai commented Jun 8, 2023

@ffesti's proposal is probably the least painful way out for now: it avoids polluting specs with crap that would become obsolete with #2078.

ffesti added a commit to ffesti/rpm that referenced this issue Jun 13, 2023
so it can be configured from the spec file

Also move the dir beside the %buildsubdir as %{buildsubdir}-SPECPARTS so
it doesn't pollute the %buildsubdir.

Resolves: rpm-software-management#2532
ffesti added a commit to ffesti/rpm that referenced this issue Jun 13, 2023
so it can be configured from the spec file

Also move the dir beside the %buildsubdir as %{buildsubdir}-SPECPARTS so
it doesn't pollute the %buildsubdir.

Resolves: rpm-software-management#2532
@github-project-automation github-project-automation bot moved this from Todo to Done in RPM Jun 13, 2023
dmnks pushed a commit to dmnks/rpm that referenced this issue Aug 1, 2023
so it can be configured from the spec file

Also move the dir beside the %buildsubdir as %{buildsubdir}-SPECPARTS so
it doesn't pollute the %buildsubdir.

Resolves: rpm-software-management#2532
(cherry picked from commit df8d1fb)
dmnks pushed a commit that referenced this issue Aug 1, 2023
so it can be configured from the spec file

Also move the dir beside the %buildsubdir as %{buildsubdir}-SPECPARTS so
it doesn't pollute the %buildsubdir.

Resolves: #2532
(cherry picked from commit df8d1fb)
@jwakely
Copy link
Contributor

jwakely commented Sep 4, 2023

Could the default be SPECPARTS-%{name} instead of %{name}-SPECPARTS, so that %{_builddir}/%{name}* doesn't change meaning?

@pmatilai
Copy link
Member

pmatilai commented Sep 5, 2023

That assumes %{_builddir}/%{name}* has a meaning to begin with. Any package making assumptions about has already stepped outside its boundaries.

dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
This reverts commit 8a5fa22, which was
introduced as a workaround for an issue RPM build system:

  rpm-software-management/rpm#2532

but now poses problems with recent setuptools versions that warn about
missing packages:

  #411

Now that the RPM issue got resolved, it seems safe to get back to
automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.
dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
This reverts commit 8a5fa22, which was
introduced as a workaround for an issue RPM build system:
rpm-software-management/rpm#2532

but now poses problems with recent setuptools versions that warn about
missing packages: #411

Now that the RPM issue got resolved, it seems safe to get back to
automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.
dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
This reverts commit 8a5fa22, which was
introduced as a workaround for an issue RPM build system
(rpm-software-management/rpm#2532) but now
poses problems with recent setuptools versions that warn about missing
packages: #411

Now that the RPM issue got resolved, it seems safe to get back to
automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.
dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
This reverts commit 8a5fa22, which was
introduced as a workaround for an issue RPM build system
(rpm-software-management/rpm#2532) but now
poses problems with recent setuptools versions that warn about missing
packages: #411

Now that the RPM issue got resolved, it seems safe to get back to
automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.
dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
Automatic package discovery by setuptools was disabled in commit
8a5fa22, somehow as a workaround for an issue RPM build system
(rpm-software-management/rpm#2532) or maybe
because it was misconfigured then.

However, having to declare packages explicitly (options 'packages' in
'[tool.setuptools]' section of pyproject.toml) is tedious and
error-prone as shown in #411
which indicates that we were missing some sub-packages.

In the meantime, the RPM issue got resolved, so it seems safe to get
back to automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.

So we get back to automatic discovery here, although not using
[tool.setuptools.packages.find], but simply letting setuptools discover
the "flat-layout" we're using, as can be seen by the following messages
from 'python -m build' un:

    * Building sdist...
    No `packages` or `py_modules` configuration, performing automatic discovery.
    `flat-layout` detected -- analysing .
    discovered packages -- ['pgactivity', 'pgactivity.profiles', 'pgactivity.queries']

This requires no configuration as we use common excluded names (like
'tests', 'docs').
dlax added a commit to dalibo/pg_activity that referenced this issue Apr 3, 2024
Automatic package discovery by setuptools was disabled in commit
8a5fa22, somehow as a workaround for an issue RPM build system
(rpm-software-management/rpm#2532) or maybe
because it was misconfigured then.

However, having to declare packages explicitly (options 'packages' in
'[tool.setuptools]' section of pyproject.toml) is tedious and
error-prone as shown in #411
which indicates that we were missing some sub-packages.

In the meantime, the RPM issue got resolved, so it seems safe to get
back to automatic discovery as this resolves the later issue in a more
future-proof way than explicitly listing packages.

So we get back to automatic discovery here, although not using
[tool.setuptools.packages.find], but simply letting setuptools discover
the "flat-layout" we're using, as can be seen by the following messages
from 'python -m build' un:

    * Building sdist...
    No `packages` or `py_modules` configuration, performing automatic discovery.
    `flat-layout` detected -- analysing .
    discovered packages -- ['pgactivity', 'pgactivity.profiles', 'pgactivity.queries']

This requires no configuration as we use common excluded names (like
'tests', 'docs').
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

6 participants