#782 Forbid %{pythonX_site(lib|arch)}/* in %files
Closed: accepted 5 years ago Opened 5 years ago by churchyard.

I'll prep the draft if we agree on this.

The idea:

One must not use any of the following in the %files section:

%{python3_sitelib}/*
%{python3_sitearch}/*
%{python2_sitelib}/*
%{python2_sitearch}/*
%{python_sitelib}/*
%{python_sitearch}/*

Note that %{python_sitelib}/%{python_sitearch} is already forbidden anyway.

Explanation

Owning all files in the site-packages directory can lead to several unexpected outcomes:

  • On Python 3, when there is a Python file, such as %{python3_sitelib}/example.py, a corresponding pycache file is created at %{python3_sitelib}/__pycache__/example.cpython-37.opt-1.pycexample.py. Package listing %{python3_sitelib}/* will own %{python3_sitelib}/__pycache__ and we don't want that. Note that even if such file is not there, any future update may bring it in and this goes unnoticed until errors are reported: #1529071
  • With %{python_sitelib}/* it's even worse, because that evaluates to /* if /usr/bin/python/ is not present.

I realize that I haven't listed a problematic use case of %{python2_sitexxx}/*, yet I rather forbid them all than only forbidding some.


I definitely agree and pointed out this on some reviews.

I don't have any problem with this but note that there's no real prohibition against multiple directory ownership in RPM or our guidelines. I'm actually surprised there's a problem with multiple packages owning __pycache__.

I assume this would just take the form of a sentence at the end of https://fedoraproject.org/wiki/Packaging:Python#Byte_compiling forbidding the overly wide pattern match.

Note that the linked bugzilla is something a bit different (my mystake), there is no real problem with multiple packages owning __pycache__. Except that it is weird.

Finally, the guidelines already said this:

(You should not include the directories %{python3_sitearch}/pycache or %{python3_sitelib}/pycache because they are already owned by the python3-libs package.)

I have already removed the parentheses and capitalized the "should not" in order to make this seem more like a rule and less like an offhand comment. But that means this ticket basically comes down to changing the "SHOULD NOT" to "MUST NOT" (which we just voted on and approved).

It's possible that I'm misunderstanding what's being requested here. I could certainly go for a bit of reorganization to lift this out of the "Byte compiling" section, since it's kind of weird to have it there anyway.

It would be nice to add a check in fedora-review for this. I know I let it slipt many in my reviews:

$ rg -i "%\{python3_sitearch\}/\*|%\{python2_sitearch\}/\*|%\{python3_sitelib\}/\*|%\{python2_sitelib\}/\*" -g "*.spec" --maxdepth 2

clustersos/clustersos.spec
39:%{python2_sitelib}/*

python-yubikey-manager/python-yubikey-manager.spec
53:%{python3_sitelib}/*

python-easyargs/python-easyargs.spec
92:%{python2_sitelib}/*
98:%{python3_sitelib}/*

python-pytest-virtualenv/python-pytest-virtualenv.spec
66:%{python2_sitelib}/*
70:%{python3_sitelib}/*

flatcam/flatcam.spec
85:%{python2_sitelib}/*

python-jaydebeapi/python-jaydebeapi.spec
79:%{python2_sitelib}/*
85:%{python3_sitelib}/*

python-pytelegrambotapi/python-pytelegrambotapi.spec
57:%{python2_sitelib}/*
62:%{python3_sitelib}/*

python-azure-sdk/python-azure-sdk.spec
185:%{python2_sitelib}/*
192:%{python3_sitelib}/*

python-azure-storage/python-azure-storage.spec
138:%{python2_sitelib}/*
145:%{python3_sitelib}/*

python-msrestazure/python-msrestazure.spec
106:%{python2_sitelib}/*
113:%{python3_sitelib}/*

python3-cryptography/python3-cryptography.spec
109:%{python3_sitearch}/*

python3-pytest-cov/python3-pytest-cov.spec
61:%{python3_sitelib}/*

dxf2gcode/dxf2gcode.spec
113:%{python3_sitelib}/*

python-graphitesend/python-graphitesend.spec
82:%{python2_sitelib}/*
89:%{python3_sitelib}/*

astrometry/astrometry.spec
208:%{python2_sitearch}/*

notmuch-abook/notmuch-abook.spec
43:%{python3_sitelib}/*

linuxcnc/linuxcnc.spec
135:%{python2_sitearch}/*

python-pydap/python-pydap.spec
87:%{python2_sitelib}/*
92:%{python3_sitelib}/*

vint/vint.spec
54:%{python3_sitelib}/*

python-vsc-install/python-vsc-install.spec
85:%{python2_sitelib}/*
91:%{python3_sitelib}/*

python-pymssql/python-pymssql.spec
92:%{python2_sitearch}/*
99:%{python3_sitearch}/*

python2-lxc/python2-lxc.spec
37:%{python2_sitearch}/*

python-twilio/python-twilio.spec
109:%{python2_sitelib}/*
115:%{python3_sitelib}/*

python-enlighten/python-enlighten.spec
118:%{python2_sitelib}/*
124:%{python3_sitelib}/*

classifier/classifier.spec
38:%{python3_sitelib}/*

python-oletools/python-oletools.spec
226:%{python2_sitelib}/*
259:%{python3_sitelib}/*

python-asttokens/python-asttokens.spec
62:%{python2_sitelib}/*
67:%{python3_sitelib}/*

python-logzero/python-logzero.spec
49:%{python2_sitelib}/*
54:%{python3_sitelib}/*

python-j1m.sphinxautointerface/python-j1m.sphinxautointerface.spec
108:%{python2_sitelib}/*
113:%{python3_sitelib}/*

python-hexdump/python-hexdump.spec
77:%{python2_sitelib}/*
82:%{python3_sitelib}/*

python-rmtest/python-rmtest.spec
64:%{python2_sitelib}/*
69:%{python3_sitelib}/*

urlscan/urlscan.spec
42:%{python3_sitelib}/*

python-f5-icontrol-rest/python-f5-icontrol-rest.spec
122:%{python2_sitelib}/*
129:%{python3_sitelib}/*

shyaml/shyaml.spec
51:%{python3_sitelib}/*

python3-docker/python3-docker.spec
66:%{python3_sitelib}/*

python-cattrs/python-cattrs.spec
76:%{python2_sitelib}/*
81:%{python3_sitelib}/*

python3-saml/python3-saml.spec
41:%{python3_sitelib}/*

yubikey-piv-manager/yubikey-piv-manager.spec
51:%{python3_sitelib}/*

ldoce5viewer/ldoce5viewer.spec
128:%{python2_sitelib}/*

python-tree-format/python-tree-format.spec
74:%{python2_sitelib}/*
79:%{python3_sitelib}/*

python-pycryptodomex/python-pycryptodomex.spec
118:%{python2_sitearch}/*
125:%{python3_sitearch}/*

python-poyo/python-poyo.spec
97:%{python2_sitelib}/*
104:%{python3_sitelib}/*

python-jinja2-time/python-jinja2-time.spec
78:%{python2_sitelib}/*
84:%{python3_sitelib}/*

python-cookiecutter/python-cookiecutter.spec
129:%{python2_sitelib}/*
135:%{python3_sitelib}/*

python-pytest-remotedata/python-pytest-remotedata.spec
64:%{python3_sitelib}/*

python-pytest-openfiles/python-pytest-openfiles.spec
56:%{python3_sitelib}/*

python-pytest-doctestplus/python-pytest-doctestplus.spec
85:%{python3_sitelib}/*

python-pytest-arraydiff/python-pytest-arraydiff.spec
73:%{python3_sitelib}/*

python-pytest-astropy/python-pytest-astropy.spec
55:%{python3_sitelib}/*

python2-astropy/python2-astropy.spec
145:%{python2_sitearch}/*

python-libusb1/python-libusb1.spec
63:%{python2_sitelib}/*
68:%{python3_sitelib}/*

python-social-auth-core/python-social-auth-core.spec
80:%{python3_sitelib}/*

python-neo4j-driver/python-neo4j-driver.spec
47:%{python2_sitearch}/*
51:%{python3_sitearch}/*

libemu/libemu.spec
334:%{python2_sitearch}/*
338:%{python3_sitearch}/*

python-neomodel/python-neomodel.spec
89:%{python3_sitelib}/*
97:%{python2_sitelib}/*

python-click-completion/python-click-completion.spec
103:%{python2_sitelib}/*
109:%{python3_sitelib}/*

borgmatic/borgmatic.spec
52:%{python3_sitelib}/*

python-flask-paranoid/python-flask-paranoid.spec
50:%{python2_sitelib}/*
54:%{python3_sitelib}/*

python-sphinxcontrib-trio/python-sphinxcontrib-trio.spec
58:%{python3_sitelib}/*

sos-collector/sos-collector.spec
61:%{python2_sitelib}/*
63:%{python3_sitelib}/*

battray/battray.spec
38:%{python3_sitelib}/*

python-pyte/python-pyte.spec
67:%{python2_sitelib}/*
72:%{python3_sitelib}/*

fedscm-admin/fedscm-admin.spec
76:%{python3_sitelib}/*

python-intervaltree/python-intervaltree.spec
50:%{python2_sitelib}/*
53:%{python3_sitelib}/*

python-ROPGadget/python-ROPGadget.spec
58:%{python2_sitelib}/*
62:%{python3_sitelib}/*

python-f5-sdk/python-f5-sdk.spec
129:%{python2_sitelib}/*
136:%{python3_sitelib}/*

python-mox/python-mox.spec
82:%{python2_sitelib}/*
88:%{python3_sitelib}/*

python-django-crispy-forms/python-django-crispy-forms.spec
47:%{python3_sitelib}/*

gr-hpsdr/gr-hpsdr.spec
58:%{python2_sitearch}/*

python-bigsuds/python-bigsuds.spec
95:%{python2_sitelib}/*
101:%{python3_sitelib}/*

Do you want me to propose Pull Requests for each one of these or should a Proven Packager do some magic?

@tibbs I'd still like to explicitly forbid the pattern of %{pythonX_site???}/*. Or at least mention it next to the rule that's already there.

@eclipseo If you can script the change, I can push it.

@eclipseo If you can script the change, I can push it.

Sorry I have no idea how to do that.

From: https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2018-07-19/fpc.2018-07-19-16.00.txt

  • 782 Forbid %{pythonX_site(lib|arch)}/* in %files (geppetto,
    17:03:18)
  • ACTION: Forbid %{pythonX_site(lib|arch)}/* in %files (+7, 0:0, -1:0)
    (geppetto, 17:19:18)

Metadata Update from @james:
- Issue untagged with: draftneeded, meeting
- Issue tagged with: writeup

5 years ago

I have written a check for %{pythonX_site(lib|arch)}/* in Fedora-Review:

https://pagure.io/FedoraReview/pull-request/303#request_diff

I did write up the SHOULD->MUST change we approved in the meeting, but I assume there is still a draft to come so I'll bump this back to the meeting state.

On the other hand, if we're done then I guess I can just close this. But for now I'll just announce the one change that was made (and then I guess we can set to fixing the packages which own the toplevel pycache directory).

Announcement text:

The Python guidelines were modified to indicate that packages must not own the top-level pycache directory.

Metadata Update from @tibbs:
- Issue untagged with: writeup
- Issue tagged with: meeting

5 years ago

BTW:

$ dnf repoquery --disablerepo='*' --enablerepo='koji' -f '/usr/lib*/python3.*/site-packages/__pycache__' --source | pkgname
audit
brotli
btest
cairo-dock-plug-ins
concordance
dxf2gcode
graphviz
ldns
libcap-ng
libftdi
liblinear
mathgl
mraa
netstat-monitor
openscap
policycoreutils
pydot
pyicu
pypolicyd-spf
pysubnettree
pythia8
python-apipkg
python-args
python-bigsuds
python-cachez
python-click-completion
python-cmdln
python-configparser
python-cookies
python-cram
python-cycler
python-debian
python-demjson
python-dialog
python-empy
python-enlighten
python-feedparser
python-flask-assets
python-flask-principal
python-flask-whooshee
python-flexmock
python-fuse
python-gzipstream
python-hwdata
python-imagesize
python-ipgetter
python-libusb1
python-mimeparse
python-mysql
python-novaclient-os-networks
python-novaclient-os-virtual-interfaces
python-optcomplete
python-pbkdf2
python-pefile
python-pika-pool
python-plaintable
python-polib
python-py2neo
python-pylcdsysinfo
python-pyphen
python-pysctp
python-pyspf
python-pytest-fixture-config
python-pytest-virtualenv
python-random2
python-regex
python-responses
python-sane
python-setuptools_hg
python-simplemediawiki
python-utmp
python-xmltramp
python3
python34
python35
python36
qpid-proton
root
sendKindle
shogun
unbound

Of course python3, python34, python35, python36 are ok.

Discussed this week: https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2018-07-26/fpc.2018-07-26-16.00.txt

  • 782 Forbid %{pythonX_site(lib|arch)}/* in %files (geppetto,
    16:34:41)
  • LINK: https://pagure.io/packaging-committee/issue/782 (mhroncok,
    16:35:14)
  • ACTION: Recommend not listing %{pythonX_sitelib}/* (and sitearch)
    in files (+1:7, 0:0, -1:0) (geppetto, 16:48:09)

I don't have any problem with this but note that there's no real prohibition against multiple directory ownership in RPM or our guidelines. I'm actually surprised there's a problem with multiple packages owning pycache.
I assume this would just take the form of a sentence at the end of https://fedoraproject.org/wiki/Packaging:Python#Byte_compiling forbidding the overly wide pattern match.

Actually, the example SPEC file would also have to be changed:

https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file

As it currently features such patterns in the files section:

%{python2_sitelib}/*
%{python3_sitelib}/*

There is one example of %python_sitelib that I think is valid, and that's if you're trying to support an application as Python 2 or Python 3 only depending on the distro target (ex. Python 2 for EPEL7, Python 3 for Fedora). This works because overriding %__python with %__python2 / %__python3 effectively turns it Python 2 or Python 3.

Yes. This is about %{python_sitelib}/* glob in files.

@churchyard, sorry, that was in response to:

Note that %{python_sitelib}/%{python_sitearch} is already forbidden anyway.

Oh. Yes, it's not forbidden totally, it is ok if you redefine %__python. This was a simplification on my side, sorry about that.

Metadata Update from @james:
- Issue untagged with: meeting
- Issue assigned to tibbs
- Issue tagged with: writeup

5 years ago

I think we are good to writeup from the last discussion ... churchyard was there anything else to discuss?

So looking at this again to make sure everything that needs writing up is written up. Just noticed the "Files to include" section says/said:

  • .pyc and .pyo byte compiled files (and, if present, the enclosing __pycache__ directory in most cases).

So, fun.

I've added a bit to the "Files to include" section about this, and cleaned up the above and the things that @gsauthof mentioned previously (though to be fair I'm not 100% sure how to best handle egg-info without the single big glob).

And of course this will all be churning again with #793 . Maybe we'll get this all sorted about the time the python folks decide to completely break the language again.

Announcement text:

The python guidelines were modified to indicate that packagers should not simply glob all files and directories under the sitelib and sitearch directories. Other small changes were made to avoid conflicting wit this requirement.

Metadata Update from @tibbs:
- Issue untagged with: writeup
- Issue tagged with: announce

5 years ago

Metadata Update from @tibbs:
- Issue untagged with: announce
- Issue close_status updated to: accepted
- Issue status updated to: Closed (was: Open)

5 years ago

Login to comment on this ticket.

Metadata