https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Bug ID: 2242971 Summary: Review Request: python-torch - PyTorch Product: Fedora Version: rawhide OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: trix@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-2.fc40.src.rpm
This is a minimal, cpu only, no frills or features PyTorch.
Reproducible: Always
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Davide Cavalca davide@cavalca.name changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |davide@cavalca.name Status|NEW |ASSIGNED Flags| |fedora-review?
--- Comment #1 from Davide Cavalca davide@cavalca.name --- Taking this review
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Davide Cavalca davide@cavalca.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |davide@cavalca.name
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/pytorch/ | |pytorch
--- Comment #2 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6511531 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Tom Rix trix@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2243052
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2243052 [Bug 2243052] Review Request: python-torchvision - A PyTorch vision package
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #3 from Davide Cavalca davide@cavalca.name --- Notes from a first pass: - URL should be https://pytorch.org/, define %forgeurl or something like that for the github repo - please add a comment for each patch documenting what it does, and ideally a link to an upstream submission for it, or a note that it's internal only to Fedora and why - why are we statically linking blas? - you can drop the %{?python_provide:%python_provide python3-%{pypi_name}} it's not needed anymore - please use the pyproject macros instead of the deprecated python build macros; this should also allow you to drop all the Requires for the subpackages as they'll be autogenerated - use %{_includedir} instead of hardcoding /usr/include in %prep; also consider symlinking instead of coyping to make it more explicit where these come from - the thirdy_party stuff we're not removing needs to be declared as Provides: bundled() and taken into account in the License field - you shouldn't need to export CC and export CXX, %set_build_flags is called by default these days and does that for you
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ngompa13@gmail.com
--- Comment #4 from Neal Gompa ngompa13@gmail.com --- Additional notes:
%license LICENSE %license third_party/miniz-2.1.0/LICENSE
I'm not sure if this means that you have two license files or not. Double check you're not just overwriting one with another.
%{python3_sitearch}/
This is not allowed, as it's too greedy. Please list out more so that it's more obvious what's in it.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #5 from Neal Gompa ngompa13@gmail.com --- Note that if you switch to pyproject macros to build this, then the %python3_sitelib line disappears entirely from the file list, as those macros generate file list files for you.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #6 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-3.fc40.src.rpm
Url changes patches comments, 1 upstreamed static linking of blas is a bug in lapack see BZ 2243823 pyproject-ing fails because this is a cmake project _includedir change made bunding done cc,cxx removed python3_sitearch impoved a bit, this is a very deep path.
fixed so versioning
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #7 from Ben Beasley code@musicinmybrain.net --- (In reply to Tom Rix from comment #6)
pyproject-ing fails because this is a cmake project
It’s not quite that simple. If you can build with %py3_build it’s almost guaranteed that %pyproject_wheel can work, possibly with some minor adjustments. (I maintain several packages that use pyproject-rpm-macros and include a Python extension built with cmake or meson.) I’d like to offer a proof of concept for pytorch, but it may be a few days before I can try it out.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 1993806 --> https://bugzilla.redhat.com/attachment.cgi?id=1993806&action=edit The .spec file difference from Copr build 6511531 to 6524145
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #9 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6524145 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #10 from Tom Rix trix@redhat.com --- (In reply to Ben Beasley from comment #7)
(In reply to Tom Rix from comment #6)
pyproject-ing fails because this is a cmake project
It’s not quite that simple. If you can build with %py3_build it’s almost guaranteed that %pyproject_wheel can work, possibly with some minor adjustments. (I maintain several packages that use pyproject-rpm-macros and include a Python extension built with cmake or meson.) I’d like to offer a proof of concept for pytorch, but it may be a few days before I can try it out.
Please go ahead and try, the upstream location of this spec is https://github.com/trixirt/pytorch-fedora/blob/main/python-torch.spec When your proof of concept plans out, please send me a PR.
The autogenerated build requires says we are missing python-cmake and python-ninja. Are these packages in the works ?
These missing packages, the docs saying cmake is a WIP and %project_wheel failing lead me to go back to py3_build.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #11 from Ben Beasley code@musicinmybrain.net --- (In reply to Tom Rix from comment #10)
The autogenerated build requires says we are missing python-cmake and python-ninja. Are these packages in the works ?
Both https://pypi.org/project/cmake/ and https://pypi.org/project/ninja/ are giant hacks to shove the corresponding executable inside binary wheels so that they can be installed in virtualenvs using pip rather than relying on OS-level installations. They don’t include importable Python modules. Since OS-level installations are what we do best, the reasonable thing to do is to patch those dependencies out (noting in a comment that the patch is downstream-only because the dependencies do make sense for the way upstream builds wheels), and just have:
BuildRequires: cmake BuildRequires: ninja
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #12 from Tom Rix trix@redhat.com --- (In reply to Ben Beasley from comment #11)
(In reply to Tom Rix from comment #10)
The autogenerated build requires says we are missing python-cmake and python-ninja. Are these packages in the works ?
Both https://pypi.org/project/cmake/ and https://pypi.org/project/ninja/ are giant hacks to shove the corresponding executable inside binary wheels so that they can be installed in virtualenvs using pip rather than relying on OS-level installations. They don’t include importable Python modules. Since OS-level installations are what we do best, the reasonable thing to do is to patch those dependencies out (noting in a comment that the patch is downstream-only because the dependencies do make sense for the way upstream builds wheels), and just have:
BuildRequires: cmake BuildRequires: ninja
:) ok we have the same opinion of the python-cmake,ninja I gave it a look when i was trying the pyproject out the first time and went down a rabbit hole
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #13 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-4.fc40.src.rpm
This brings in the the gloo and xnnpack packages that were recently packaged A --with pyproject option added to make testing the pyproject_ macros easier
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #14 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 1994231 --> https://bugzilla.redhat.com/attachment.cgi?id=1994231&action=edit The .spec file difference from Copr build 6524145 to 6532598
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #15 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6532598 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Tom Rix trix@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(davide@cavalca.na | |me)
--- Comment #16 from Tom Rix trix@redhat.com --- I believe all of the outstanding comments other than broken pyproject have been addressed. I will be out next week and would like to address any new comments before then. Are there any ?
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #17 from Neal Gompa ngompa13@gmail.com --- Just drop all the pyproject stuff since it doesn't work. It's perfectly fine to use the regular python macros instead.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Davide Cavalca davide@cavalca.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(davide@cavalca.na | |me) |
--- Comment #18 from Davide Cavalca davide@cavalca.name --- Can you file a ticket against https://src.fedoraproject.org/rpms/pyproject-rpm-macros for the pyproject issue?
License: BSD-3-Clause and MIT
This isn't a valid SPDX expression, you need to capitalize the AND
# Broken, looking for cmake,ninja # %generate_buildrequires # %pyproject_buildrequires
You can't comment out macros, they'll still get expanded and potentially break stuff. Use something like # %%generate_build_requires to escape them or drop these entirely
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #19 from Neal Gompa ngompa13@gmail.com --- (In reply to Davide Cavalca from comment #18)
Can you file a ticket against https://src.fedoraproject.org/rpms/pyproject-rpm-macros for the pyproject issue?
License: BSD-3-Clause and MIT
This isn't a valid SPDX expression, you need to capitalize the AND
Actually, it's not required. I do need to bug the SPDX people about this, actually. :/
# Broken, looking for cmake,ninja # %generate_buildrequires # %pyproject_buildrequires
You can't comment out macros, they'll still get expanded and potentially break stuff. Use something like # %%generate_build_requires to escape them or drop these entirely
Right, that's why I asked for it to be dropped.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #20 from Tom Rix trix@redhat.com --- SPEC URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-5.fc40.src.rpm
dropped the %generate_* capitalized AND Filed a bz 2244862 on pyproject-rpm-macros Left the --with pyproject logic as-is so it could be used as a reproducer in the pyproject-rpm-macros bz
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Miro Hrončok mhroncok@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok@redhat.com
--- Comment #21 from Miro Hrončok mhroncok@redhat.com --- What does "Broken, looking for cmake,ninja" mean?
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #22 from Miro Hrončok mhroncok@redhat.com --- Upstream says they need the following Python packages to build:
https://github.com/pytorch/pytorch/blob/v2.1.0/pyproject.toml [build-system] requires = [ "setuptools", "wheel", "astunparse", "numpy", "ninja", "pyyaml", "cmake", "typing-extensions", "requests", ]
Apparently, they are using https://pypi.org/project/cmake/ and https://pypi.org/project/ninja/ -- if you want to use "normal" cmake and ninja instead, unfortunately, you need to patch/sed those out.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #23 from Davide Cavalca davide@cavalca.name --- I tried running fedora-review against it but there are too many warnings/errors to paste it here, so I'll attach it instead. Things of note:
Header files in -devel subpackage, if present. Note: python3-torch : /usr/lib64/python3.12/site- packages/torch/_inductor/codegen/cpp_prefix.h python3-torch :
and so on (there's a lot of these) -- are these useful/appropriate to ship it in the main package, or should they be moved to a -devel package?
python3-torch.aarch64: E: zero-length /usr/lib64/python3.12/site-packages/torch/ao/quantization/backend_config/observation_type.py python3-torch.aarch64: E: zero-length /usr/lib64/python3.12/site-packages/torch/cuda/error.py python3-torch.aarch64: E: zero-length /usr/lib64/python3.12/site-packages/torch/include/ATen/cudnn/Exceptions.h
These need double checking if they're actually supposed to be empty
python3-torch.aarch64: E: unused-direct-shlib-dependency /usr/lib64/python3.12/site-packages/torch/lib/libshm.so /usr/lib64/python3.12/site-packages/torch/lib/libtorch.so.2.1
and so on (there's a lot of these too), see https://fedoraproject.org/wiki/Common_Rpmlint_issues#unused-direct-shlib-dep...
python3-torch.aarch64: E: undefined-non-weak-symbol /usr/lib64/python3.12/site-packages/torch/lib/libshm.so _gfortran_stop_string (/usr/lib64/python3.12/site-packages/torch/lib/libtorch_cpu.so.2.1)
and so on (there's a lot of these too), I'm actually not sure what this one means
python3-torch.aarch64: W: position-independent-executable-suggested /usr/lib64/python3.12/site-packages/torch/bin/torch_shm_manager
Generally our build flags should be doing -fPIE/-fPIC by default, are they not getting passed properly?
python3-torch.aarch64: E: non-executable-script /usr/lib64/python3.12/site-packages/torch/_appdirs.py 644 /usr/bin/env python3
and so on (there's a lot of these too), these usually mean you need to strip the shebang from the files as they're not meant to be run directly.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #24 from Davide Cavalca davide@cavalca.name --- Created attachment 1994491 --> https://bugzilla.redhat.com/attachment.cgi?id=1994491&action=edit fedora-review template output
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #25 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 1994495 --> https://bugzilla.redhat.com/attachment.cgi?id=1994495&action=edit The .spec file difference from Copr build 6532598 to 6544163
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #26 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6544163 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #27 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-6.fc40.src.rpm
Clears up the errors. Still several warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #28 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 1995205 --> https://bugzilla.redhat.com/attachment.cgi?id=1995205&action=edit The .spec file difference from Copr build 6544163 to 6557867
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #29 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6557867 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #30 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-7.fc40.src.rpm
The biggest change was splitting the large lists of files and dirs out to files that are included. Most other errors / warnings cleaned up. The last (current) big error set is
Complain about unused lib
python3-torch.x86_64: E: unused-direct-shlib-dependency /usr/lib64/python3.12/site-packages/torch/lib/libtorch_python.so.2.1.0 /lib64/libgfortran.so.5
Complain later about undefined symbol that the earlier complained about lib contains
python3-torch.x86_64: E: undefined-non-weak-symbol /usr/lib64/python3.12/site-packages/torch/lib/libshm.so.2.1.0 _gfortran_stop_string (/usr/lib64/python3.12/site-\ packages/torch/lib/libtorch_cpu.so.2.1)
And when used to both build another package, python-torchvision, and run some samples there is no runtime 'undefined symbol' or any error.
This looks like a false positive
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #31 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 1998101 --> https://bugzilla.redhat.com/attachment.cgi?id=1998101&action=edit The .spec file difference from Copr build 6557867 to 6615837
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #32 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6615837 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #33 from Sandro gui1ty@penguinpee.nl --- A minor thing that caught my eye: the title of the review request should be of the form 'Review Request: <main package name here> - <short summary here>'. The '<short summary here>' is usually the %{summary} from the spec file. Looking at the spec file, that would be "An AI/ML python package". However, on PyPI I see "Tensors and Dynamic neural networks in Python with strong GPU acceleration". That's probably too long for the spec file summary, yet more to the point, I guess (except for the GPU acceleration part).
Just a shout from the side line. Feel free to ignore.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #34 from Tom Rix trix@redhat.com --- (In reply to Sandro from comment #33)
A minor thing that caught my eye: the title of the review request should be of the form 'Review Request: <main package name here> - <short summary here>'. The '<short summary here>' is usually the %{summary} from the spec file. Looking at the spec file, that would be "An AI/ML python package". However, on PyPI I see "Tensors and Dynamic neural networks in Python with strong GPU acceleration". That's probably too long for the spec file summary, yet more to the point, I guess (except for the GPU acceleration part).
Just a shout from the side line. Feel free to ignore.
PyTorch is such a big and well know project, I think of more as a brand name like 'Coke' , that's why i added the '- PyTorch' to this review instead of the short summary in the spec, which as you say isn't the long thing in the git project because it is long and at this point misleading wrt GPU acceleration.
CUDA is impossible. ROCm is planned. Sycl/OneAPI would be nice, but at this time unplanned. Vulkan and OpenCL I do not think have full support in PyTorch, missing some/most of the necessary math libs.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #35 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-8.fc40.src.rpm
Changes - Expand the gfortran patch to cover more libs needing to link to libgfortran # A Fedora libblas.a problem of undefined symbol # libtorch_cpu.so: undefined symbol: _gfortran_stop_string Patch5: 0001-torch-unresolved-syms-need-gfortran.patch
- Most of the -7 fedora-review Errors
# libtorch_python.so: undefined symbols: Py* Patch7: 0001-python-torch-link-with-python.patch # E: unused-direct-shlib-dependency libshm.so.2.1.0 libtorch.so.2.1 Patch8: 0001-python-torch-remove-ubuntu-specific-linking.patch
The only remaining is
python3-torch.x86_64: E: shared-library-without-dependency-information /usr/lib64/python3.12/site-packages/torch/lib/libtorch.so.2.1.0
python3-torch.x86_64: E: files-duplicated-waste 183285
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #36 from Davide Cavalca davide@cavalca.name --- Looks like this breaks fedora-review because of the %include usage.
Source1: pt_dirs.txt Source2: pt_devel_headers.txt Source3: pt_python.txt
Please add comments to document how these were generated (or even better, generate them with a script and run that in %install instead like the systemd package does).
Summary: An AI/ML python package
As the comment above mentioned, this needs to match the summary in the review title; if you want it to include PyTorch for visibility, I'd suggest something like "PyTorch AI/ML framework".
# auto reviewer not happy with : BSD-0-Clause AND Khronos
Neither of these are valid SPDX names. BSD-0-Clause is most likely 0BSD (https://spdx.org/licenses/0BSD.html). As for Khronos, you should submit that one for review at https://gitlab.com/fedora/legal/fedora-license-data as I don't see it in the list of allowed licenses (https://docs.fedoraproject.org/en-US/legal/allowed-licenses/).
# Limit to these because they are well behaved with clang ExclusiveArch: x86_64 aarch64 %global toolchain clang
Is this an upstream limitation? We definitely have other packages using clang that build find on all arches.
# And they need to be stripped
Why are we manually stripping stuff in %install?
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #37 from Davide Cavalca davide@cavalca.name --- I'm also unable to rebuild this locally from the last src.rpm:
ERROR: Exception(python-torch-2.1.0-8.fc40.src.rpm) Config(fedora-rawhide-aarch64) 0 minutes 8 seconds INFO: Results and/or logs in: /var/lib/mock/fedora-rawhide-aarch64/result ERROR: Source RPM is not installable: Updating / installing... python-torch-2.1.0-8.fc40 warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root warning: user trix does not exist - using root warning: group trix does not exist - using root ######################################## error: unpacking of archive failed on file /builddir/build/SOURCES/pytorch-v2.1.0.tar.gz;655d19b2: cpio: read failed - Inappropriate ioctl for device error: /builddir/build/originals/python-torch-2.1.0-8.fc40.src.rpm cannot be installed
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #38 from Tom Rix trix@redhat.com --- (In reply to Davide Cavalca from comment #36)
Looks like this breaks fedora-review because of the %include usage.
Source1: pt_dirs.txt Source2: pt_devel_headers.txt Source3: pt_python.txt
Please add comments to document how these were generated (or even better, generate them with a script and run that in %install instead like the systemd package does).
These are only somewhat automated, some judgement was used to include things. This package has a wide and deep directory / file structure. This approach is a less bad option to comment #4 from Neal. I would prefer to use aggressive globbling. How aggressive can I be ?
Summary: An AI/ML python package
As the comment above mentioned, this needs to match the summary in the review title; if you want it to include PyTorch for visibility, I'd suggest something like "PyTorch AI/ML framework".
Ok.
# auto reviewer not happy with : BSD-0-Clause AND Khronos
Neither of these are valid SPDX names. BSD-0-Clause is most likely 0BSD (https://spdx.org/licenses/0BSD.html). As for Khronos, you should submit that one for review at https://gitlab.com/fedora/legal/fedora-license-data as I don't see it in the list of allowed licenses (https://docs.fedoraproject.org/en-US/legal/allowed-licenses/).
Thanks for the 0BSD pointer. From the breakdown of the licenses, the Khronos ones are OpenCL headers They are not used so I am rm-ing them in prep.
# Limit to these because they are well behaved with clang ExclusiveArch: x86_64 aarch64 %global toolchain clang
Is this an upstream limitation? We definitely have other packages using clang that build find on all arches.
To cover the other arches in an early attempt here https://github.com/trixirt/pytorch-fedora/commit/e8602044dda5fe806787b060650... Ment using gcc. But there are many, many warnings when gcc is used, because I suspect, pytorch is really only built with clang. So instead of chasing problems with gcc, I reverted back to clang. And rawhide's clang changes faster than pytorch's, this choice of arches are imo the most stable for both pythorch and clang. Maybe this is only x86_64 ? I'd like to get aarch64 in as well.
# And they need to be stripped
Why are we manually stripping stuff in %install?
Fixed with fiddling with the cmake build type.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #39 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-9.fc40.src.rpm
Above changes made from comment 38 Additions : Replaced the %include lists with -f <generated_list> Add a -test subpackage
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #40 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2001596 --> https://bugzilla.redhat.com/attachment.cgi?id=2001596&action=edit The .spec file difference from Copr build 6615837 to 6695821
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #41 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6695821 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #42 from Davide Cavalca davide@cavalca.name --- Thank you, this is looking much better now. The only remaining rpmlint issues of note are
python3-torch.x86_64: E: shared-library-without-dependency-information /usr/lib64/python3.12/site-packages/torch/lib/libtorch.so.2.1.0 python3-torch.x86_64: W: position-independent-executable-suggested /usr/lib64/python3.12/site-packages/torch/bin/torch_shm_manager
which seem to imply our cflags aren't being passed properly somewhere in the build. I'd recommend forcing cmake to be more verbose during the build to verify this. There's also a lot of linker warnings such as
/usr/bin/ld: warning: LLVM gold plugin: aten/src/ATen/native/cpu/GridSamplerKernel.cpp:663:5: loop not unrolled: the optimizer was unable to perform the requested transformation; the transformation might be disabled or specified as part of an unsupported transformation ordering
but they don't seem to be breaking the build.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #43 from Neal Gompa ngompa13@gmail.com --- There is one significant thing you should do:
Requires: python3dist(filelock) Requires: python3dist(fsspec) Requires: python3dist(jinja2) Requires: python3dist(networkx) Requires: python3dist(setuptools) Requires: python3dist(sympy) Requires: python3dist(typing-extensions)
This should all be dropped, since the Python dependency generator already takes care of this.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #44 from Tom Rix trix@redhat.com --- Spec URL: https://trix.fedorapeople.org/python-torch.spec SRPM URL: https://trix.fedorapeople.org/python-torch-2.1.0-10.fc40.src.rpm
pie for torch_shm_manager has been addressed.
libtorch error is caused by the global change to use as-needed to fix unused-direct-shlib errors. Since libtorch is not a proper library but rather a wrapper/accumlator of other pythorch libraries the as-needed linking is not approriate, so the new change is to turn off as-needed just for libtorch. This resolves the issue but causes unused-direct-shlib error to be reported for libtorch. These new issues i believe are to be expected for a wrapper library.
Requires for python removed
Rework to use openblas over blas so the -lgfortran and the static blas,lapack libraries could be removed.
gold turned off
some verbose output flags added.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #45 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2002392 --> https://bugzilla.redhat.com/attachment.cgi?id=2002392&action=edit The .spec file difference from Copr build 6695821 to 6714617
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #46 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/6714617 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Please take a look if any issues were found.
--- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #47 from Neal Gompa ngompa13@gmail.com --- From my perspective, this is probably as good as it gets. Unless Davide sees something I'm missing, I think we're good to go here.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Davide Cavalca davide@cavalca.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #48 from Davide Cavalca davide@cavalca.name --- Thanks again for working on this! I agree with Neal, this is good to go now. Package APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #49 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/python-torch
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #50 from Tom Rix trix@redhat.com --- Davide and Neal Thanks for all the hard work on the review! Now Fedora can start building out the other PyTorch AI packages
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
--- Comment #51 from Ben Beasley code@musicinmybrain.net --- A follow-up discussing the consequences of versioning the .so files in /usr/lib64/python3.12/site-packages/torch/lib/: bug 2253018
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Tom Rix trix@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|2243052 |
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2243052 [Bug 2243052] Review Request: python-torchvision - A PyTorch vision package
https://bugzilla.redhat.com/show_bug.cgi?id=2242971
Tom Rix trix@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |CURRENTRELEASE Status|POST |CLOSED Last Closed| |2024-01-14 16:33:59
package-review@lists.fedoraproject.org