https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Bug ID: 2136235 Summary: Review Request: mingw-python-build - MinGW Python build library Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: manisandro@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://smani.fedorapeople.org/review/mingw-python-build.spec SRPM URL: https://smani.fedorapeople.org/review/mingw-python-build-0.8.0-1.fc38.src.rp... Description: MinGW Python build library Fedora Account System Username: smani
Test builds: https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11-build/build...
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Maxwell G gotmax@e.email changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |gotmax@e.email Doc Type|--- |If docs needed, set a value
--- Comment #1 from Maxwell G gotmax@e.email --- I would use `%py3_build` and `%py3_install` (build uses setuptools) and get rid of the bootstrap logic.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #2 from Sandro Mani manisandro@gmail.com --- Spec URL: https://smani.fedorapeople.org/review/mingw-python-build.spec SRPM URL: https://smani.fedorapeople.org/review/mingw-python-build-0.8.0-2.fc38.src.rp...
%changelog * Wed Oct 19 2022 Sandro Mani manisandro@gmail.com - 0.8.0-2 - Switch to setuptools based build and drop bootstrap logic
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2136237
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2136237 [Bug 2136237] Review Request: mingw-python-installer - MinGW Python installer library
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2136238
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2136238 [Bug 2136238] Review Request: mingw-python-pep517 - MinGW Python pep517 library
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? CC| |gui1ty@penguinpee.nl Blocks|2136237, 2136238 | Assignee|nobody@fedoraproject.org |gui1ty@penguinpee.nl Depends On| |2136237
--- Comment #3 from Sandro gui1ty@penguinpee.nl --- I take this one.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=2136237 [Bug 2136237] Review Request: mingw-python-installer - MinGW Python installer library https://bugzilla.redhat.com/show_bug.cgi?id=2136238 [Bug 2136238] Review Request: mingw-python-pep517 - MinGW Python pep517 library
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #4 from Sandro gui1ty@penguinpee.nl --- With the bootstrapped mingw-python-installer present in Copr, this package builds, but fails to install.
Copr build: https://copr.fedorainfracloud.org/coprs/gui1ty/reviews/build/4965807/
I also tried building de non-bootstrapped installer and pep517. They fail because nothing provides mingw{32,64}-python3-build. I want to test the non-bootstrapped version before approving those.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(manisandro@gmail. | |com)
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro Mani manisandro@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(manisandro@gmail. | |com) |
--- Comment #5 from Sandro Mani manisandro@gmail.com --- The builds are failing due to mingw-python-wheel not yet being in the repos (I want to push the entire batch at once, once all pieces are in place). You can find the SRPM in my test repo [1]. Perhaps that repo is also already a sufficient demonstration that the pieces work together.
[1] https://copr.fedorainfracloud.org/coprs/smani/mingw-python3-3.11-build/build...
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #6 from Sandro gui1ty@penguinpee.nl --- (In reply to Sandro Mani from comment #5)
Perhaps that repo is also already a sufficient demonstration that the pieces work together.
True to some extent. It would be even more helpful, had you enabled 'Run fedora-review tool for packages in this project'. ;)
How many of the packages still need to be reviewed? I understand it's not straight forward having packages depend on each other, but one has to start somewhere and build it up. Now it feels like a hit and miss. Is there a dependency tree of some kind for all the packages?
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #7 from Sandro Mani manisandro@gmail.com ---
True to some extent. It would be even more helpful, had you enabled 'Run fedora-review tool for packages in this project'. ;)
I actually wasn't aware of that option, I'll keep it in mind!
How many of the packages still need to be reviewed? I understand it's not straight forward having packages depend on each other, but one has to start somewhere and build it up. Now it feels like a hit and miss. Is there a dependency tree of some kind for all the packages?
The dependency tree resp build sequence is:
mingw-python3
mingw-python-setuptools mingw-python-wheel
mingw-python-flit-core mingw-python-pep517 (bootstrap) mingw-python-pyparsing (bootstrap) mingw-python-packaging (bootstrap) mingw-python-installer (boostrap) mingw-python-build
mingw-python-pep517 mingw-python-pyparsing mingw-python-packaging mingw-python-installer
Of these, mingw-python-flit-core, mingw-python-pep517, mingw-python-installer and mingw-python-build are pending review.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #8 from Sandro gui1ty@penguinpee.nl --- So, mingw-python-wheel has been approved. Could you push that to rawhide? Looks like it doesn't depend on any of the packages still in review.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #9 from Sandro Mani manisandro@gmail.com --- Yes I can indeed to that, proceeding.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #10 from Sandro Mani manisandro@gmail.com --- I've imported the package, but it fails to build with python3.10, not really sure why [1]. I'd prefer not spending time to debug the python3.10 build as it builds fine with python3.11 and I'm not planning to ship with python3.10 anyway.
[1] https://kojipkgs.fedoraproject.org//work/tasks/3071/93463071/build.log
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #11 from Sandro gui1ty@penguinpee.nl --- I only asked for a rawhide build. Rawhide is python3.11, so why are you running up against python3.10? It's fine not releasing the package for older python versions or leaving that for later.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #12 from Sandro Mani manisandro@gmail.com --- No actually mingw-python3 on rawhide is still 3.10, the 3.11 push is pending all these reviews.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #13 from Sandro gui1ty@penguinpee.nl --- Alright. I misunderstood. So, basically everything depends on each other in a rather fragile way. I know you have a proof of concept in Copr, I'm just wondering how this is going to pan out once you start updating packages.
Let me reset my bearings and zoom out a bit...
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #14 from Sandro Mani manisandro@gmail.com --- Well the typical strategy in these cases is to do all the builds in a side-tag, and then merge when the entire package set is built.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
--- Comment #15 from Sandro gui1ty@penguinpee.nl --- Issues: ======= - Package contains BR: python2-devel or python3-devel => Not sure if there should be a BR: python3-devel since we are talking mingw here and the package builds fine without it
- Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/rpm, /usr/lib/rpm/macros.d => Looks like a missing requires on rpm (or whatever else is able to provide the directories you are installing to)
- README.md is missing in %files => tarball provides README.md. Please include it as %doc
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== MUST items =====
Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Using prebuilt packages [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/rpm, /usr/lib/rpm/macros.d [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [!]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
Python: [x]: Python eggs must not download any dependencies during the build process. [x]: Package meets the Packaging Guidelines::Python [x]: Packages MUST NOT have dependencies (either build-time or runtime) on packages named with the unversioned python- prefix unless no properly versioned package exists. Dependencies on Python packages instead MUST use names beginning with python2- or python3- as appropriate. [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files [x]: Binary eggs must be removed in %prep
===== SHOULD items =====
Generic: [x]: Reviewer should test that the package builds in mock. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [?]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [!]: Rpmlint is run on all installed packages. Note: Mock build failed See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_use_rpmlint
Rpmlint ------- Some warnings regarding duplicate files and no documentation => README.md not included
Source checksums ---------------- https://files.pythonhosted.org/packages/source/b/build/build-0.8.0.tar.gz : CHECKSUM(SHA256) this package : 887a6d471c901b1a6e6574ebaeeebb45e5269a79d095fe9a8f88d6614ed2e5f0 CHECKSUM(SHA256) upstream package : 887a6d471c901b1a6e6574ebaeeebb45e5269a79d095fe9a8f88d6614ed2e5f0
Requires -------- mingw32-python3-build (rpmlib, GLIBC filtered): /usr/bin/python3.11 mingw32(python(abi)) mingw32(python3.11dist(packaging)) mingw32(python3.11dist(pep517)) mingw32-python3-installer mingw32-python3-setuptools mingw32-python3-wheel
mingw64-python3-build (rpmlib, GLIBC filtered): /usr/bin/python3.11 mingw64(python(abi)) mingw64(python3.11dist(packaging)) mingw64(python3.11dist(pep517)) mingw64-python3-installer mingw64-python3-setuptools mingw64-python3-wheel
Provides -------- mingw32-python3-build: mingw32(python3.11dist(build)) mingw32(python3dist(build)) mingw32-python3-build rpm_macro(mingw32_py3_build_host_wheel) rpm_macro(mingw32_py3_build_wheel) rpm_macro(mingw32_py3_install_host_wheel) rpm_macro(mingw32_py3_install_wheel)
mingw64-python3-build: mingw64(python3.11dist(build)) mingw64(python3dist(build)) mingw64-python3-build rpm_macro(mingw64_py3_build_host_wheel) rpm_macro(mingw64_py3_build_wheel) rpm_macro(mingw64_py3_install_host_wheel) rpm_macro(mingw64_py3_install_wheel)
Generated by fedora-review 0.8.0 (e988316) last change: 2022-04-07 Command line :/usr/bin/fedora-review --no-colors --prebuilt --rpm-spec --name mingw-python-build --mock-config /var/lib/copr-rpmbuild/results/configs/child.cfg Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Python, Generic Disabled plugins: SugarActivity, C/C++, Haskell, Ocaml, R, Java, Perl, fonts, PHP Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #16 from Sandro Mani manisandro@gmail.com --- Spec URL: https://smani.fedorapeople.org/review/mingw-python-build.spec SRPM URL: https://smani.fedorapeople.org/review/mingw-python-build-0.8.0-3.fc38.src.rp...
%changelog * Sun Oct 30 2022 Sandro Mani manisandro@gmail.com - 0.8.0-3 - Require rpm for %%{_rpmconfigdir}/macros.d/
- Package contains BR: python2-devel or python3-devel
=> Not sure if there should be a BR: python3-devel since we are talking mingw here and the package builds fine without it
No this does not apply to mingw-python packages
- Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/rpm, /usr/lib/rpm/macros.d
=> Looks like a missing requires on rpm (or whatever else is able to provide the directories you are installing to)
Fixed
- README.md is missing in %files
=> tarball provides README.md. Please include it as %doc
Docs omitted per [1].
[1] https://docs.fedoraproject.org/en-US/packaging-guidelines/MinGW/#_files_whic...
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Sandro gui1ty@penguinpee.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #17 from Sandro gui1ty@penguinpee.nl --- I should have taken a closer look at the MinGW guidelines when taking on these reviews, instead of approaching this from the Python angle. ;)
At least I learned something should I come across MinGW again in a future review.
Package is approved.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #18 from Sandro Mani manisandro@gmail.com --- Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
--- Comment #19 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/mingw-python-build
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- FEDORA-2022-f7a5568776 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-f7a5568776
https://bugzilla.redhat.com/show_bug.cgi?id=2136235
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|MODIFIED |CLOSED Last Closed| |2022-11-01 11:05:18
--- Comment #21 from Fedora Update System updates@fedoraproject.org --- FEDORA-2022-f7a5568776 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2136235 Bug 2136235 depends on bug 2136237, which changed state.
Bug 2136237 Summary: Review Request: mingw-python-installer - MinGW Python installer library https://bugzilla.redhat.com/show_bug.cgi?id=2136237
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com