Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/blob/rawhide/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4211337167/artifacts/file/python-setuptools-gettext-0.1.3-1.fc39.src.rpm Description: Setuptools helpers for gettext Fedora Account System Username: bkhl Since I'm not in the packager group (yet?), that GitLab project has a quick-and-dirty CI config to build the package. I want to add this package and will try to also add it to epel8 and epel9, with the aim to eventually be able to include an up-to-date version of Breezy (https://src.fedoraproject.org/rpms/breezy) into epel8 and epel9. Once I'll get there I intend to volunteer to co-maintain Breezy in order to maintain EPEL branches of it there, eventually resolving https://bugzilla.redhat.com/show_bug.cgi?id=2107044 I'll need a sponsor for this request. I already had someone in the EPEL Matrix chat volunteer to do that, so I'll ping him as soon as I've posted this.
Hi Björn, I've got this assigned to me now and I'll do the review as stated in chat. There is an automated tool called fedora-review that we normally run on new packages to catch common mistakes. It requires the Spec URL to be the spec file in plaintext, and the SRPM URL to be a direct link to the SRPM file. Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/raw/rawhide/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4211337167/artifacts/raw/python-setuptools-gettext-0.1.3-1.fc39.src.rpm This comment is enough for me to run the tool now, but as we iterate on the spec file make sure to use URLs like that so I can run the tool again.
The License tag is using the old GPLv2+ identifier. It's required to use the new SPDX identifier, which in this case is GPL-2.0-or-later. https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_valid_license_short_names ================================================================================ Upstream has two different licenses in their setup.cfg file. license = GPLv2 or later https://github.com/breezy-team/setuptools-gettext/blob/v0.1.3/setup.cfg#L6 License :: OSI Approved :: Apache Software License https://github.com/breezy-team/setuptools-gettext/blob/v0.1.3/setup.cfg#L14 Can you open an issue with them to get clarification on what the intended license is, so we can make sure the package is marked correctly? ================================================================================ What is the purpose of the ExclusiveArch tag? The dependencies appear to be available on all architectures, so I can't see why this would be necessary. ================================================================================ Speaking of dependencies, I don't see any mention in the the source code of semantic-version, typing-extensions, or wheel. The comment about setuptools_scm and wheel doesn't seem to be relevant (or accurate, as RHEL 8 and 9 have both of those). ================================================================================ You mentioned building this for EPEL 8. RHEL 8 only has setuptools 39.2.0, but this software and the spec file set a minimum version of 46.1. That minimum has been upstream since the initial commit. I don't actually think it's accurate. After patching it out I was able to build this spec file for EPEL 8. I recommend adding this line to %prep. sed -e 's/setuptools>=46.1/setuptools/' -i setup.cfg ================================================================================ There is a macro to do an import test in %check, it would be better to switch to that instead of the manual import. %py3_check_import setuptools_gettext
I created a pull request here to clarify the license in the Python package metadata: https://github.com/breezy-team/setuptools-gettext/pull/11 I don't think we need to wait for a release with that though, since there's no ambiguity in the source. The repo only contains the GPL license file, the only source file has a GPL 2.0+ preamble, and they also have a license note in the pyproject.toml that specifies GPL 2.0+. It's pretty clear the Apache thing is just a copy paste error in the Python package metadata "troves" list. Regarding the other comments: * The ExclusiveArch/semantic-version/typing-extension/wheel are all things I saw in another package I took inspiration from, but that contained Rust code. All the remnants of that are removed now. * Added the suggested sed command to remove restrictions on setuptools version and I also don't see there's any reason to have it. * Added the py3_check_import macro as suggested and that also works fine. Updated links: Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4217201264/artifacts/file/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4217201264/artifacts/file/python-setuptools-gettext-0.1.3-1.fc39.src.rpm
Noticed now this still doesn't build in EPEL 8. One issue was a typo in the 'rm -rf setuptools_gettext.egg-info' which I fixed now, but still getting errors in the EPEL 8 build where it tries to look for setuptools>=46.1. I suspect it's some issue with the %autosetup step, will keep digging. In the meanwhile here is the latest version now which at least has that typo fixed: Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4218305542/artifacts/file/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4218305542/artifacts/file/python-setuptools-gettext-0.1.3-1.fc39.src.rpm
So turns out I was just blind to the BuildRequires line that still had the 46.1 requirement. Finally now have a spec that addresses your comments and builds on epel8 as well: Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4219668903/artifacts/file/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4219668903/artifacts/file/python-setuptools-gettext-0.1.3-1.fc39.src.rpm
…. and here you go with proper "raw" links: Spec URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4219668903/artifacts/raw/python-setuptools-gettext.spec SRPM URL: https://gitlab.com/bkhl/python-setuptools-gettext/-/jobs/4219668903/artifacts/raw/python-setuptools-gettext-0.1.3-1.fc39.src.rpm Sorry about the comment spam. This should be the final one now.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5870222 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2192597-python-setuptools-gettext/fedora-rawhide-x86_64/05870222-python-setuptools-gettext/fedora-review/review.txt 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.
> I created a pull request here to clarify the license in the Python package metadata: https://github.com/breezy-team/setuptools-gettext/pull/11 Awesome! > I don't think we need to wait for a release with that though, since there's no ambiguity in the source. The repo only contains the GPL license file, the only source file has a GPL 2.0+ preamble, and they also have a license note in the pyproject.toml that specifies GPL 2.0+. Agreed, but I would consider including your pull request as a patch in the package. We don't need to hold up the review on it though. This package is APPROVED and I've sponsored you into the packager group. Note that in the review output below the failure about the spec file mismatch with the SRPM is due to the %autorelease and %autochangelog macros. Just make sure that when you import it into dist-git that it has the actual macros. One last thing I'll point out is that in your GitLab repo you have a changelog file. That is used when a package has an existing changelog and is converting to rpmautospec. Since this is a new package, the changelog should be automatically generated from the initial import commit. I'm not sure that rpmautospec will do the right thing if the initial import commit has a changelog file. You might end up with a duplicate changelog entry, so it would be best to just delete it to be sure. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== MUST items ===== Generic: [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]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [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. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [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 must own all directories that it creates. [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]: 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]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [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: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [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: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Diff spec file in url and in SRPM --------------------------------- --- /home/carl/packaging/reviews/python-setuptools-gettext/2192597-python-setuptools-gettext/srpm/python-setuptools-gettext.spec 2023-05-03 10:52:59.433071869 -0500 +++ /home/carl/packaging/reviews/python-setuptools-gettext/2192597-python-setuptools-gettext/srpm-unpacked/python-setuptools-gettext.spec 2023-04-28 19:00:00.000000000 -0500 @@ -1,2 +1,12 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.3.5) +## RPMAUTOSPEC: autorelease, autochangelog +%define autorelease(e:s:pb:n) %{?-p:0.}%{lua: + release_number = 1; + base_release_number = tonumber(rpm.expand("%{?-b*}%{!?-b:1}")); + print(release_number + base_release_number - 1); +}%{?-e:.%{-e*}}%{?-s:.%{-s*}}%{!?-n:%{?dist}} +## END: Set by rpmautospec + Name: python-setuptools-gettext Version: 0.1.3 @@ -42,3 +52,5 @@ %changelog -%autochangelog +* Sat Apr 29 2023 Björn Lindström <bkhl> - 0.1.3-1 +- Initial package. +
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-setuptools-gettext
I see I'm in the packager group now and managed to get this as far as a test in Bodhi now. I interpreted your last comment to mean I need to explicitly set release number and update changelog in the spec file as fedpkg won't work with %autochangelog/%autorelease, so did that. In doing that I failed to add the release tag, so got this. https://artifacts.dev.testing-farm.io/18506ae7-b9d8-4295-9ad5-0cd1737c44cd/ To be on the safe side bumped the release version to 2 along with adding the dist tag, so as part of that added to the changelog too. So should I understand it correctly that this is the correct way to add changelogs and bump versions, or is there some way to switch to having it be automated now that I've made an initial build?
FEDORA-2023-3683d7ce8d has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-3683d7ce8d
FEDORA-2023-9f666f38fa has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-9f666f38fa
FEDORA-EPEL-2023-a3b475e686 has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-a3b475e686
FEDORA-EPEL-2023-b22fe53883 has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-b22fe53883
FEDORA-EPEL-2023-b22fe53883 has been pushed to the Fedora EPEL 8 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-b22fe53883 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-3683d7ce8d has been pushed to the Fedora 38 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-3683d7ce8d \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-3683d7ce8d See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-9f666f38fa has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-9f666f38fa \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-9f666f38fa See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2023-a3b475e686 has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-a3b475e686 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
> I interpreted your last comment to mean I need to explicitly set release number and update changelog in the spec file as fedpkg won't work with %autochangelog/%autorelease, so did that. No that's not what I meant, sorry for the confusion. You could have had the initial import commit use %autorelease/%autochangelog with no changelog file. fedpkg works fine with those macros, but when using `fedpkg mockbuild` the resulting spec file inside the SRPM will have the macros expanded (for compatibility reasons, I think). Looking a bit closer, I think `fedpkg import` should have refused to import that SRPM based on this code in rpkg, the underlying library it uses. https://pagure.io/rpkg/c/3087dd72e933568896b2f167bab75194482a6cd1 But your initial import has the macros, so I'm not sure what happened. Did you import the SRPM from `fedpkg mockbuild`, or something else? I'm going to have to dig into this one a bit more and get back to you about the correct steps to take. Sorry again you hit this rough edge. The spec file is totally fine as it currently is in distgit. > So should I understand it correctly that this is the correct way to add changelogs and bump versions, or is there some way to switch to having it be automated now that I've made an initial build? From here you could do a few things. You could just keep writing changelog entries (rpmdev-bumpspec from rpmdevtools helps with this). Or you could switch to the rpmautospec macros by running `rpmdevautospec convert`. This will create a commit that adds the macros and moves the existing changelog entries to a changelog file. Ideally you would run this right before bumping the version in the spec file. https://docs.pagure.org/fedora-infra.rpmautospec/opting-in.html#for-existing-packages
I think thewas probably I got ahead of myself and did a 'git push' before the first 'fedpkg import'. When then 'fedpkg import' didn't work because of an error about autochangelog/autorelease, I manually pushed the other changes as well until I got it into a state where I could 'fedpkg import', which only worked after I removed those macros entirely. I understand it corrently now that typically if you make an update to include a new build, you build the SRPM with 'fedpkg mockbuild' first, then do 'fedpkg import' which will also push the change to the repository?
FEDORA-EPEL-2023-a3b475e686 has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2023-b22fe53883 has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
`fedpkg import` is only used for the initial import from a reviewed SRPM. Going forward, you can use `fedpkg mockbuild` to do a local test, but it's not strictly required. The routine package update workflow would look like this: 1. Edit the spec file. 2. Download the new source tarball, usually with `spectool -g python-setuptools-gettext.spec`. 3. Verify it builds with `fedpkg mockbuild` (optional). 4. `git add python-setuptools-gettext.spec` 5. Upload the new sources to the lookaside cache with `fedpkg new-sources setuptools-gettext-*.tar.gz`. This will also modify the sources and .gitignore files and add them to the git stage. 6. `git commit` 7. `git push` 8. `fedpkg build` 9. `fedpkg update` (if not on the rawhide branch) These commands are described in more detail in the package maintenance guide. https://docs.fedoraproject.org/en-US/package-maintainers/Package_Maintenance_Guide/
FEDORA-2023-9f666f38fa has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2023-3683d7ce8d has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.