Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: https://www.five-ten-sg.com/util/passwordsafe.spec SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24-1.fc44.src.rpm Description: Password Safe is a password manager. It stores your passwords in an encrypted file, allowing you to remember only one password (the "master password"), instead of all the username/password combinations that you use. For extra security, you can use a Yubikey device to provide two-factor authentication. Fedora Account System Username: carllibpst
Updated spec file with the same url. SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24.0-1.fc44.src.rpm
So there's a bunch of things to comment here, let me go through the order they appear in the spec file: - Whenever possible, please use `%autorelease`/`%autochangelog` [1], it helps other contributors, particularly with rebasing PRs - The `Source0` should be an url, in this case `https://github.com/pwsafe/pwsafe/archive/refs/tags/%{version}.tar.gz` or equivalent - License must be in SPDX format: `Artistic-2.0` - We would need to do a proper license check for this one since often there are undocumented embedded files with different license/copyright. At the very least the Yubico license files stand out - We do not use Group/Packager fields, and especially not Vendor - Please narrow down the `BuildRequires`, `make` for example is obviously not used here - `Requires` and handled automagically, particularly for libraries. You MUST NOT add them manually as these can change out of your control - For the `Obsoletes` see the packaging guideline on this [2] - It is best to use `%autosetup` or `%autopatch` to make adding/removing patches easier - Unless you need to port this to an old RHEL, include a `%conf` section which would have the `%cmake` (configure) step - In the `%build`, please follow the cmake guidelines [3], specifically, use the `%cmake*` macros, do not hard-code the generator, do not manually handle the builddir, etc. - A review of the available build flags is needed, the linkage, vendoring of source, etc. Did you notice anything that I should keep in mind before diving into it? - I do not see value for converting the markdown and other such text files into pdf - What's the reason for deleting some of the source files in the %build phase? It would make debugging in `mock` more difficult at the very least - `%clean` is deprecated - Please do not use manual `install` command. Everything should be handled in either `%cmake_install` (if not available, please raise an issue to upstream), or `%files` (for `LICENSE`, `%doc`, etc.) - There is something missing that should be done with the icons, but I need to cross-reference some other packages - Same for the translation files - `%check` section is missing, and I can tell that they have ctest tests - The %defattr is unnecessary afaict [1]: https://fedora-infra.github.io/rpmautospec-docs/index.html [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#renaming-or-replacing-existing-packages [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/CMake/
- Whenever possible, please use `%autorelease`/`%autochangelog` [1], it helps other contributors, particularly with rebasing PRs I don't think the upstream git repo is compatible with that. - The `Source0` should be an url, in this case `https://github.com/pwsafe/pwsafe/archive/refs/tags/%{version}.tar.gz` or equivalent done. - License must be in SPDX format: `Artistic-2.0` done. - We would need to do a proper license check for this one since often there are undocumented embedded files with different license/copyright. At the very least the Yubico license files stand out I have removed the Yubico license files from the package. There is no code in this project from Yubico - although it links to both libyubikey and ykpers. The license files here are copies of the license files in libyubikey and ykpers. But we surely don't need to include license files from every package that we link to? - We do not use Group/Packager fields, and especially not Vendor done. - Please narrow down the `BuildRequires`, `make` for example is obviously not used here I need access to the packager group to run scratch builds to see which of those are already included in the standard build environment and can be removed. - `Requires` and handled automagically, particularly for libraries. You MUST NOT add them manually as these can change out of your control done. - For the `Obsoletes` see the packaging guideline on this [2] There is an existing Fedora pwsafe package that is an ancient cli only program. It provides /usr/bin/pwsafe which conflicts with this package. The maintainer is going to retire that package. This package Obsoletes every version of that old package. - It is best to use `%autosetup` or `%autopatch` to make adding/removing patches easier done. - Unless you need to port this to an old RHEL, include a `%conf` section which would have the `%cmake` (configure) step done. - In the `%build`, please follow the cmake guidelines [3], specifically, use the `%cmake*` macros, do not hard-code the generator, do not manually handle the builddir, etc. done. - A review of the available build flags is needed, the linkage, vendoring of source, etc. Did you notice anything that I should keep in mind before diving into it? No. - I do not see value for converting the markdown and other such text files into pdf Consider an end-user who wants to read the doc files. Hardly any end-users are familiar with either rtf or markdown. On my F44 system, file manager right click on .rtf suggests wordpad, which does not exist in Fedora. Right click on .md suggests emacs which won't display it nicely. End-users are familiar with .pdf, I think evince is installed by default, and file manager right click opens it. - What's the reason for deleting some of the source files in the %build phase? It would make debugging in `mock` more difficult at the very least done. Upstream docs directory is cluttered with files, so we only package the ones that are relevant. - `%clean` is deprecated done. - Please do not use manual `install` command. Everything should be handled in either `%cmake_install` (if not available, please raise an issue to upstream), or `%files` (for `LICENSE`, `%doc`, etc.) %cmake_install does almost everything, but it does not install any of the docs files. I install those manually via `install`. - There is something missing that should be done with the icons, but I need to cross-reference some other packages - Same for the translation files - `%check` section is missing, and I can tell that they have ctest tests If we use `%cmake` rather than `%cmake -DNO_GTEST=ON`, it does a git clone to download googletest source code, which is not allowed in Fedora. But in that case it does find two tests which pass. - The %defattr is unnecessary afaict done. Updated spec and srpm Spec URL: https://www.five-ten-sg.com/util/passwordsafe.spec SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24.0-1.fc44.src.rpm
> > - Whenever possible, please use `%autorelease`/`%autochangelog` [1], it helps other contributors, particularly with rebasing PRs > I don't think the upstream git repo is compatible with that. The upstream git repo does not play a role into this. The only fields that are affected are `Release` and `%changelog` which are downstream only. The git connection is with the dist-git, and even that one is not necessary. > But we surely don't need to include license files from every package that we link to? Yes, that is not done in other packages either. Technically you need to add if any of the other symbols are re-compiled from the header file, e.g. C++ templates, but even that case is hard to track and is often overlooked. > I need access to the packager group to run scratch builds to see which of those are already included in the standard build environment and can be removed. Firstly you can use copr [1] to do the builds which are 90% the same as koji builds. `mock` would also do that. Secondly that is not what I was referring to. If it is a dependency defined from cmake requirement then yes explicitly define it. The one that made me raise my eyebrow was having both `make` and `ninja-build`. In the current list, `ninja-build` and `rpmdevtools` shold be dropped. > This package Obsoletes every version of that old package. You still MUST specify the specific version to obsolete even if that is the case. You can use a version higher to not have to sync with the release. > Consider an end-user who wants to read the doc files. Hardly any end-users are familiar with either rtf or markdown. I beg a differ. Using the original markdown and rtf is common standard [2] and markdown is made to be human readable. In contrast ooffice conversion is not used in other packages [3]. The man page should be the recommended user-facing documentation. This is not a blocking issue for me though, so do as you wish. > If we use `%cmake` rather than `%cmake -DNO_GTEST=ON`, it does a git clone to download googletest source code Please highlight this because there are standard workarounds. Please request upstream to use `FetchContent` instead of the manual build snippet in [4]. This is the modern CMake approach that is packager friendly and will work with `gtest-devel` out-of-the-box. You may ping me on the upstream issue (@LecrisUT) or point them to a reference like this one [5] [1]: https://copr.fedorainfracloud.org/coprs/ [2]: https://sourcegraph.com/search?q=context:global+repo:src.fedoraproject.org+README.md&patternType=keyword&sm=0 [3]: https://sourcegraph.com/search?q=context:global+repo:src.fedoraproject.org+ooffice&patternType=keyword&sm=0 [4]: https://github.com/pwsafe/pwsafe/blob/a2f591079e7d889a487c59d00e497549401ea88d/CMakeLists.txt#L225-L251 [5]: https://github.com/spglib/spglib/blob/12355c77fb7c505a55f52cae36341d73b781a065/test/CMakeLists.txt#L87-L102
> I don't think the upstream git repo is compatible with that. The upstream git repo does not play a role into this. The only fields that are affected are `Release` and `%changelog` which are downstream only. The git connection is with the dist-git, and even that one is not necessary. I tried that, and got a changelog entry of * Fri May 15 2026 John Doe <packager> - 1.24.0-1.fc44 - local build The upstream git repo is the only source of commit messages that would make sense to put into a changelog. It looks like I need a changelog file in the dist-git when this package gets into Fedora. Is there a way for %autochangelog to pick up a changelog file for local builds? In the current list, `ninja-build` and `rpmdevtools` shold be dropped. done. You still MUST specify the specific version to obsolete even if that is the case. You can use a version higher to not have to sync with the release. done. > If we use `%cmake` rather than `%cmake -DNO_GTEST=ON`, it does a git clone to download googletest source code Please highlight this because there are standard workarounds. Ah, they have a switch to turn off that download, and use the installed gtest. That works nicely. Updated spec and srpm Spec URL: https://www.five-ten-sg.com/util/passwordsafe.spec SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24.0-1.fc44.src.rpm
Ah, found a mechanism for a local build to pick up the changelog file with %autochangelog. Updated spec and srpm Spec URL: https://www.five-ten-sg.com/util/passwordsafe.spec SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24.0-1.fc44.src.rpm
fedpkg mockbuild worked nicely to update the buildrequires list. There are some license issues. I scanned the project for any copyrights other than Rony Shapiro, and found two. src/core/pugixml/pugixml.cpp - includes what seems to be MIT license at the end of the cpp file. In %build I could copy that section of the cpp file into something like pugixml.LICENSE and install that from %files. src/core/crypto/external/Chromium/LICENSE - seems to be BSD-3-Clause. Upstream packaging does not install this. Since the filename is just "LICENSE", I assume we should install it as Chromium/LICENSE or Chromium.LICENSE
> I tried that, and got a changelog entry of > * Fri May 15 2026 John Doe <packager> - 1.24.0-1.fc44 > - local build To give more context, that is fine and working as it's supposed to. The changelog after import should be sanitized anyway, and the changelog here doesn't make much difference, the fedora-review bot would help instead (seems to be offline, will check up with the folks). > There are some license issues. I scanned the project for any copyrights other than Rony Shapiro, and found two. Main thing to do is notify upstream properly and ask them to track these. REUSE is an interesting project they can consider for this, otherwise just a simple note in the readme, COPYING, or similar is fine. Both upstream and downstream though should check if the licensed files are pulled in always or conditionally. > src/core/pugixml/pugixml.cpp - includes what seems to be MIT license at the end of the cpp file. In %build I could copy that section of the cpp file into something like pugixml.LICENSE and install that from %files. > > src/core/crypto/external/Chromium/LICENSE - seems to be BSD-3-Clause. Upstream packaging does not install this. Since the filename is just "LICENSE", I assume we should install it as Chromium/LICENSE or Chromium.LICENSE Yes, install the appropriate files, preference with the `pugixml.LICENSE` or equivalent single-file format, but also include them in the `License` glued by `AND`, i.e. `Artistic-2.0 AND MIT AND BSD-3-Clause` with a breakdown of the sources (format is free-form, but check some examples [1,2]) of the external files and `Provides: bundled` [3]. Extracting the license header from pugixml.cpp seems hard, I would not recommend on that. See instead about getting it from upstream [4], or preferably, please reach out to upstream and ask them to use `FetchContent` approach to bundling that way we can get the whole repo files for the bundled content (or de-bundle) and it is easier for them to update, win-win. And more notes on the current state - Doing a `touch --no-create %{_datadir}/icons/hicolor` is incorrect. Instead you have to `Requires: hicolor-icon-theme`, see chatterino example. The other `%post*` are also unnecessary and if not, they should be done at build-level - Afaict `appstream-util validate-relax` is also a step that should be done at `%check` - See the locale guideline [5] for how to handle these. It is the first time for me to review these, so I am using that and other reference usages [6]. This is a review blocker because ``` passwordsafe.x86_64: W: file-not-in-%lang /usr/share/locale/da/LC_MESSAGES/pwsafe.mo ``` - Please de-glob the `%{_bindir}/pwsafe*`, and to that extent, what is the difference and relation between `pwsafe` and `pwsafe-cli`? - Some rpmlint output (should have been given by the fedora-review bot) ``` passwordsafe.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/passwordsafe-1.24.0/config.txt passwordsafe.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/passwordsafe-1.24.0/help.txt passwordsafe.x86_64: W: unused-direct-shlib-dependency /usr/bin/pwsafe-cli /lib64/libmagic.so.1 passwordsafe.x86_64: W: summary-ended-with-dot Password Safe is a password management utility. passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli passwordsafe.x86_64: W: file-not-in-%lang /usr/share/locale/da/LC_MESSAGES/pwsafe.mo ``` `obsolete-not-provided` and `no-manual-page-for-binary` issues are fine to ignore - Can you remove `src/ui/Windows` and `src/os/windows` in `%prep`. Those have more differently licensed files and should make sure these are not pulled in without updating the metadata. Otherwise, I think all would be lgtm. I will post the fedora-review checklist after these [1]: https://src.fedoraproject.org/rpms/chatterino2/blob/rawhide/f/chatterino2.spec#_61 [2]: https://src.fedoraproject.org/rpms/openmw/blob/rawhide/f/openmw.spec#_34 [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling [4]: https://github.com/zeux/pugixml [5]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#handling_locale_files [6]: https://sourcegraph.com/search?q=context:global+repo:src.fedoraproject.org+%25find_lang&patternType=keyword&sm=0
Icons - done. pugixml - they have a bundled 1.7 version. Fedora has 1.15 but it is not fully compatible with 1.7 - I made a stab at converting but it is not trivial. appstream-util - done. - Can you remove `src/ui/Windows` and `src/os/windows` in `%prep`. Those have more differently licensed files and should make sure these are not pulled in without updating the metadata. Done, but those are still in the tarball in the srpm. Do we need to rebuild the tarball to ensure we are not distributing that source?
> Done, but those are still in the tarball in the srpm. Do we need to rebuild the tarball to ensure we are not distributing that source? It is fine, they are still allowed license. The main reason there was to not pick up additional licenses during an update without a review. > they have a bundled 1.7 version. Fedora has 1.15 but it is not fully compatible with 1.7 `FetchContent` can work both ways, either by taking it from a cmake config file (`pugixml-devel`) or pointing it to a git submodule (or equivalent). And it is upstream friendly in that it can be locked to the current version that they support.
appstream-util validate-relax needs network access to validate the org.pwsafe.pwsafe.metainfo.xml file which contains external links to screen shots. With fedpkg mockbuild --enable-network, it passes, but I assume that a real fedora build won't have network access. Does a real fedora build somehow allow network access for that part of %check, or does Fedora disallow external links in metainfo files? If the links are disallowed, I can patch the metainfo file to reference local images (presumably installed with the other doc files). hm, https://freedesktop.org/software/appstream/docs/chap-Metadata.html says the url for screenshots must be an http/https reference to a public web site. pugixml - still working on unbundling this.
> appstream-util validate-relax needs network access See references [1]. Add `--nonet` should do the trick. [1]: https://sourcegraph.com/search?q=context:global+repo:src.fedoraproject.org+appstream-util&patternType=keyword&sm=0
appstream-util validate-relax --nonet works nicely. pugixml - the only incompatible change 1.7 to 1.15 is in a piece of code that has a bug, and the fix is to replace the call to pugixml. Upstream is working on that. When that is done, I can patch the code to unbundle pugixml and add buildrequires pugixml-devel. I will update here when that happens.
Debundling pugixml is not a blocker for me, feel free to address it later. Main point about it is to let upstream know about our downstream packaging needs, and about the technology that best facilitates both upstream and downstream's needs
[fedora-review-service-build]
Copr build: https://copr.fedorainfracloud.org/coprs/build/10492912 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10492912-passwordsafe/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
Created attachment 2142609 [details] The .spec file difference from Copr build 10492912 to 10506337
Copr build: https://copr.fedorainfracloud.org/coprs/build/10506337 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10506337-passwordsafe/fedora-review/review.txt Found issues: - Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/passwordsafe/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ Please know that there can be false-positives. --- 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.
Created attachment 2142767 [details] The .spec file difference from Copr build 10506337 to 10508546
Copr build: https://copr.fedorainfracloud.org/coprs/build/10508546 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10508546-passwordsafe/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.
passwordsafe.src: E: spelling-error ('macOS', '%description -l en_US macOS -> ma Cos, mac OS, mac-OS') passwordsafe.x86_64: E: spelling-error ('macOS', '%description -l en_US macOS -> ma Cos, mac OS, mac-OS') That is strange. The official name of the Apple Mac operating system is macOS. See https://support.apple.com/en-us/109033 for a sample. Large data in /usr/share - I presume this means we should have a noarch subpackage for those. I will investigate this. Reviewer should test that the package builds in mock - it does but that was marked as fail?
Do not worry about the fedora review output, this is to be used by the reviewer primarily, and I went over it and selected the parts that are relevant previously. It has quite a few known false-positives (or negatives?). Do run it though with that command whenever you make changes so that it can produce a diff and notify me that there are changes. > Large data in /usr/share - I presume this means we should have a noarch subpackage for those. I will investigate this. This is because of the pdf generated docs, I guess another reason why they are not encouraged. You can either drop the pdf formats, or move them into a `doc` subpackage. Please ping me when you are ready for me to look at this again or if you have questions
wc -c $(rpm -ql passwordsafe | grep usr/share/doc ) 761181 total wc -c $(rpm -ql passwordsafe | grep usr/share/locale) 1129944 total wc -c $(rpm -ql passwordsafe | grep usr/share/passwordsafe/help) 13668100 total The big one is help files. I have moved those into -help noarch subpackage. [fedora-review-service-build]
password safe help/about contains a link to check for the latest version. But that checks with the upstream, presumably for their rpm package that is not fedora compliant. There was some upstream discussion of distributions patching that out, to avoid encouraging end-users to install packages outside the distribution package repositories. What are your feelings on that?
> The big one is help files. I have moved those into -help noarch subpackage. Please no, that is not according to guidelines. The guidelines is to have `doc` subpackage [1] if it is necessary. Looking at the files in help, they are all just zip archives of the site https://pwsafe.org/, but in an un-navigable format as these are just snippets without a main `index.html` to glue them into a single page. My suggestion would be to drop these. The alternative of `doc` subpackage is also acceptable. > password safe help/about contains a link to check for the latest version Thanks for pointing it out. The text is not ideal, but it is not a blocker for the review either (it does not actively bypass the packaging), it looks like a good check to have overall. Open the discussion to make this editable, it does not look hard to do so [2]. My recommendation is something along the lines "You can try to update with [insert update command], or suggest an update at [insert src.fp.o link here]" [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_documentation [2]: https://github.com/pwsafe/pwsafe/blob/a8825d431e38817ecf2a9c9912f623be9f2c4818/src/ui/wxWidgets/AboutDlg.cpp#L538-L549
Created attachment 2142920 [details] The .spec file difference from Copr build 10508546 to 10512416
Copr build: https://copr.fedorainfracloud.org/coprs/build/10512416 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10512416-passwordsafe/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.
changed -help to -doc subpackage, which now contains the doc files, language translations and help .zip files. Those .zip files are used by the pwsafe binary to display html formatted help. [fedora-review-service-build]
> language translations Afaik, translations files *MUST NOT* be in a different package I did not find any instance where that is the case, let me know if you see one otherwise.
Created attachment 2142969 [details] The .spec file difference from Copr build 10512416 to 10513632
Copr build: https://copr.fedorainfracloud.org/coprs/build/10513632 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10513632-passwordsafe/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.
Translations and the man page moved back to the main package. Take directory ownership of /usr/share/passwordsafe [fedora-review-service-build]
Created attachment 2143037 [details] The .spec file difference from Copr build 10513632 to 10514284
Copr build: https://copr.fedorainfracloud.org/coprs/build/10514284 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10514284-passwordsafe/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.
upstream fix for zh -> zh_CN can be removed from the spec [fedora-review-service-build]
Created attachment 2143510 [details] The .spec file difference from Copr build 10514284 to 10527355
Copr build: https://copr.fedorainfracloud.org/coprs/build/10527355 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10527355-passwordsafe/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.
Various comments on the current state - The bundling approach of pugixml is dubious. Please add `Provides: bundled()` accordingly and provide an issue/PR about debundling in upstream. You said that you would be contacting upstream about several issues, but I do not see any such activities and especially none that are tracked in the spec file - The bundling of the svgs is quite questionable, and it will slow down the review to figure out its providence, licensing, etc. - All patches do not have providence or explanation to what they are trying to do - Please separate the BuildRequires to be individual lines and comment where it is relevant, e.g. test dependencies, doc building dependencies etc. - The current `pwsafe` version in Fedora is `0.20.0`, please use that for the Obsoletes - Consider de-globing the mandir to make sure it would not be lost - What is the deal with `snapshot.patch`, what were you trying to do? - Please avoid blindly `sed`. Instead create patches for those and discuss with upstream to include them in some form The whole handling of oofice and pandoc is creating more complications than good, and this is causing the only major blocker right now. Consider removing it, markdown is perfectly readable documentation that does not need a separate application to render. Try to focus on resolving these issues without making too many other changes, because it is very close to being ready to be approved.
Created attachment 2144019 [details] The .spec file difference from Copr build 10527355 to 10556357
Copr build: https://copr.fedorainfracloud.org/coprs/build/10556357 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10556357-passwordsafe/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.
Aha, now I understand what that snapshot.patch was. Please do not use a patch, instead you can use `%forgemeta` macros [1,2] or a manual equivalent (i.e. use a commit hash instead of a version). Anything that follows the snapshot guidelines [3]. Here is a snippet implementation using forgemeta ``` %global forgeurl0 https://github.com/pwsafe/pwsafe %global version0 1.24.0 # Using a more recent snapshot of `master` to pull in various necessary patches until 1.25 is out %global date 20260604 %global commit 1d41f7b %forgemeta Name: passwordsafe Version: %forgeversion Source0: %forgesource0 ... %prep %forgeautosetup -p1 ``` Advantage of `%forgemeta` approach is that you can more easily go in and out of a snapshot and it complies with the snapshot versioning guidelines --- If a patch is not designed to be downstream-only, include a link to their sources. In particular, Patch4 should be written as ``` # fix the CMakeLists.txt compiler options for pugixml 1.15 Patch4: https://github.com/pwsafe/pwsafe/pull/1807.patch ``` (Use `spectool -g -P` to download the patch for srpm building. You will need to delete the local copy of the patch to refresh it. ) --- Change the osboletes line to have a `<` sign [4]. Just use the closest higher bump that is in Fedora pwsafe = 0.20.0 -> pwsafe < 0.20.1 -- Please include links to the discussions of Patch2, Patch3 and pugixml's char vs wchar --- The build log reports ``` *** WARNING: ./usr/src/debug/passwordsafe-1.24.0-1.fc45.x86_64/src/core/Util.cpp is executable but has no shebang, removing executable bit ``` Patch is easy, just a `chmod -x` for those files. --- Package review follows, not everything is actionable, see comments first, will mark them with a `(comment: ...)` Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [-]: Package does not contain kernel modules. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. 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. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* MIT License", "MIT License", "*No copyright* Artistic License 2.0", "BSD 2-Clause License", "Artistic License", "GNU General Public License v2.0 or later", "BSD 3-Clause License". 2852 files have unknown license. Detailed output of licensecheck in /home/cle/FedoraRPMs/2468312-passwordsafe/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/passwordsafe (comment: it fails for the `doc` sub-package. But that subpackage should `Requires: pwsafe = %version-%release`) [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries or specifies bundled libraries with Provides: bundled(<libname>) if unbundling is not possible. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [?]: Package uses nothing in %doc for runtime. [x]: The spec file handles locales properly. [x]: Package consistently uses macros (instead of hard-coded directory names). [?]: Package is named according to the Package Naming Guidelines. (comment: it can be renamed to `pwsafe` directly as that is orphaned and maybe retired soon, leaving that for the sponsor or others to chime in) [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [x]: If the package is a rename of another package, proper Obsoletes and Provides are present. (comment: Provides could also be included, but no strong feelings given how old it is and the comment 3 items above) [!]: Requires correct, justified where necessary. (comment: Missing note about `hicoloder-icon-theme`, and the Requires relation between `doc` subpackage and main should be reversed. You can use `Suggests: %name-doc` instead) [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [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]: The License field must be a valid SPDX expression. [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]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [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 ===== 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). [?]: Package functions as described. [x]: Latest version is packaged. (comment: snapshot is used for now, should be reverted to version at next opportunity) [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. (comment: more detailed breakdown made previously) [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: 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]: Fully versioned dependency in subpackages if applicable. [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). See: (this test has no URL) (comment: known false-positive, only diff is from %autochangelog) [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 1310720 bytes in /usr/share [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Rpmlint ------- Checking: passwordsafe-1.24.0-1.fc45.x86_64.rpm passwordsafe-doc-1.24.0-1.fc45.noarch.rpm passwordsafe-1.24.0-1.fc45.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpg0majchw')] checks: 32, packages: 3 passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli 3 packages and 0 specfiles checked; 0 errors, 2 warnings, 12 filtered, 0 badness; has taken 1.2 s Rpmlint (debuginfo) ------------------- Checking: passwordsafe-debuginfo-1.24.0-1.fc45.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmprcc02xc6')] checks: 32, packages: 1 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 12 filtered, 0 badness; has taken 1.7 s Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.9.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 3 passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli 3 packages and 0 specfiles checked; 0 errors, 2 warnings, 22 filtered, 0 badness; has taken 0.9 s Source checksums ---------------- https://github.com/pwsafe/pwsafe/archive/refs/tags/1.24.0.tar.gz : CHECKSUM(SHA256) this package : 234f88f7c224c9dcaea5650afca5957fcf3f80ea411d92a34f26468e7ecd0485 CHECKSUM(SHA256) upstream package : 234f88f7c224c9dcaea5650afca5957fcf3f80ea411d92a34f26468e7ecd0485 https://github.com/zeux/pugixml/archive/v1.15/pugixml-1.15.tar.gz : CHECKSUM(SHA256) this package : b39647064d9e28297a34278bfb897092bf33b7c487906ddfc094c9e8868bddcb CHECKSUM(SHA256) upstream package : b39647064d9e28297a34278bfb897092bf33b7c487906ddfc094c9e8868bddcb Requires -------- passwordsafe (rpmlib, GLIBC filtered): hicolor-icon-theme libX11.so.6()(64bit) libXtst.so.6()(64bit) libc.so.6()(64bit) libcurl.so.4()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libm.so.6()(64bit) libmagic.so.1()(64bit) libqrencode.so.4()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.15)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libuuid.so.1()(64bit) libuuid.so.1(UUID_1.0)(64bit) libwx_baseu-3.2.so.0()(64bit) libwx_baseu-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_aui-3.2.so.0()(64bit) libwx_gtk3u_aui-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_core-3.2.so.0()(64bit) libwx_gtk3u_core-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_html-3.2.so.0()(64bit) libwx_gtk3u_html-3.2.so.0(WXU_3.2)(64bit) libxerces-c-3.3.so()(64bit) libykpers-1.so.1()(64bit) libykpers-1.so.1(LIBYKPERS_1.0)(64bit) libykpers-1.so.1(LIBYKPERS_1.15)(64bit) libykpers-1.so.1(LIBYKPERS_1.4)(64bit) libykpers-1.so.1(LIBYKPERS_1.5)(64bit) libykpers-1.so.1(LIBYKPERS_1.7)(64bit) passwordsafe-doc rtld(GNU_HASH) passwordsafe-doc (rpmlib, GLIBC filtered): Provides -------- passwordsafe: application() application(pwsafe.desktop) bundled(pugixml) metainfo() metainfo(org.pwsafe.pwsafe.metainfo.xml) passwordsafe passwordsafe(x86-64) passwordsafe-doc: passwordsafe-doc Diff spec file in url and in SRPM --------------------------------- --- /home/cle/FedoraRPMs/2468312-passwordsafe/srpm/passwordsafe.spec 2026-06-04 09:55:47.885338599 +0200 +++ /home/cle/FedoraRPMs/2468312-passwordsafe/srpm-unpacked/passwordsafe.spec 2026-06-03 02:00:00.000000000 +0200 @@ -1,2 +1,12 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.8.4) +## 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 + %global pugixml_version 1.15 @@ -146,3 +156,34 @@ %changelog -%autochangelog +## START: Generated by rpmautospec +* Wed Jun 03 2026 Carl Byington <carl> - 1.24.0-1 +- Include fixes after the official 1.24.0 release. +- Add context menu to copy the custom field values. +- The report files are now always bom-less utf-8 encoded. +- Text input for custom fields value is now resizeable. +- Add browse + autotype feature. +- Add 1.24.0 changelog entries from upstream. +- Custom fields can now be added to each entry. That is, fields with + user-defined names and values. +- Changing the height of the Add/Edit dialog will cause a vertical + scrollbar to be added. +- In the "Flattened List" view, sorting by time columns (e.g., entry + creation time) now works correctly. +* Tue May 05 2026 Carl Byington <carl> - 1.23-2 +- Convert docs to pdf format. +- Properly tag doc and license files. +- Enable debug packages. +* Tue Sep 29 2015 Rony Shapiro <ronys> - 0.97BETA-4 +- Rename package to passwordsafe to avoid conflict with command-line pwsafe +* Thu Oct 24 2013 Johan Vromans <jvromans> - 0.92BETA-3 +- Additional buildrequires for some platforms. +* Sun Sep 22 2013 Rony Shapiro <ronys> - 0.92BETA-2 +- Changes for YubiKey build +* Sun Dec 02 2012 Rony Shapiro <ronys> - 0.8BETA-2 +- Streamline rpm build process under PasswordSafe source tree +- Remove unwanted doc files from RPM +* Thu Nov 08 2012 David Dreggors <dadreggors> - 0.8BETA-1 +- Spec file created for Fedora rpm builds +- First rpm build for Fedora 17 + +## END: Generated by rpmautospec Generated by fedora-review 0.11.0 (05c5b26) last change: 2025-11-29 Command line :/usr/bin/fedora-review -b 2468312 Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Shell-api, Generic Disabled plugins: R, fonts, PHP, Ocaml, Haskell, Perl, SugarActivity, Python, Java Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH [1]: https://forge.fedoraproject.org/packaging/forge-srpm-macros/ [2]: https://src.fedoraproject.org/rpms/span/blob/rawhide/f/span.spec [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots [4]: https://docs.fedoraproject.org/en-US/packaging-guidelines/#renaming-or-replacing-existing-packages
Going back to Obsoletes: pwsafe < 2.0.0-1 ; the latest in fedora is 0.2.0, but there are other pwsafe rpm packages in third part repos with version numbers of 1.x.x. We need to obsolete all of those since they also try to take /usr/bin/pwsafe. There is no upstream discussion of pugixml char vs wchar. This package started in the Windows world at a time when some folks thought that 16 bit char was the way to go, before utf-8 won. It has deep dependencies on wchar, and it uses pugixml in wchar mode. Package name should stay with 'passwordsafe' since that is what upstream is using. Spec URL: https://www.five-ten-sg.com/util/passwordsafe.spec SRPM URL: https://www.five-ten-sg.com/util/passwordsafe-1.24.0^20260604git1d41f7b-1.fc44.src.rpm [fedora-review-service-build]
Created attachment 2144288 [details] The .spec file difference from Copr build 10556357 to 10569917
Copr build: https://copr.fedorainfracloud.org/coprs/build/10569917 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2468312-passwordsafe/fedora-rawhide-x86_64/10569917-passwordsafe/fedora-review/review.txt Found issues: - Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/passwordsafe/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ Please know that there can be false-positives. --- 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.
Thanks, all LGTM now. A request for sponshorship ticket should be automatically created after this, but feel free to drop a comment on the mailing list or #devel matrix room. And finally thank you for sticking up with the review to the very end. For the sponsor (or other reviewers) 2 topics that I am not sure about: - Is it ok for the Obsoletes to be catching any 1.x.x that are not packaged in Fedora? - Otherwise the replacement of `pwsafe` looks sane to me, but more eyes are appreciated --- Response to the previous reply > but there are other pwsafe rpm packages in third part repos with version numbers of 1.x.x That is a bit of information that would have been useful earlier. I do not think it is our place to obsolete the third-party packaging as we cannot carry their context, e.g. maybe it is built with different cmake flags on purpose. My gut feeling is that we should not obsolete those, but I will leave it to the sponsor to handle. > Package name should stay with 'passwordsafe' since that is what upstream is using. Sure, but there is also an argument for pwsafe as that is the upstream git namespace. You can totally have the srpm name be `pwsafe` and the binary rpm to be `passwordsafe` (very common in rust crates with binaries). But I don't have enough context to make an informed decision there. > # kill off executable bit on all files > chmod -R -x+X . It is an overkill, but ultimately not a blocker for me. Just that this should be submitted to upstream as a PR. --- Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/cle/FedoraRPMs/2468312-passwordsafe/diff.txt See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ (comment: Checked this manually, looks like false-positive) ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Package does not contain any libtool archives (.la) [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. 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. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* MIT License", "MIT License", "*No copyright* Artistic License 2.0", "BSD 2-Clause License", "Artistic License", "GNU General Public License v2.0 or later", "BSD 3-Clause License". 2855 files have unknown license. Detailed output of licensecheck in /home/cle/FedoraRPMs/2468312-passwordsafe/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries or specifies bundled libraries with Provides: bundled(<libname>) if unbundling is not possible. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Development files must be in a -devel package [?]: Package uses nothing in %doc for runtime. [x]: The spec file handles locales properly. [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. [x]: 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]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [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]: The License field must be a valid SPDX expression. [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]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-file-validate if there is such a file. [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]: 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 ===== SHOULD items ===== Generic: [x]: 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. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: 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]: Fully versioned dependency in subpackages if applicable. [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: Bad spec filename: /home/cle/FedoraRPMs/2468312-passwordsafe/srpm- unpacked/passwordsafe.spec See: (this test has no URL) (comment: see false-positive above) [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Note: Arch-ed rpms have a total of 1320960 bytes in /usr/share [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). Rpmlint ------- Checking: passwordsafe-1.24.0^20260604git1d41f7b-1.fc45.x86_64.rpm passwordsafe-doc-1.24.0^20260604git1d41f7b-1.fc45.noarch.rpm passwordsafe-1.24.0^20260604git1d41f7b-1.fc45.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmp0ljyr182')] checks: 32, packages: 3 passwordsafe.spec: W: patch-not-applied Patch2: remove-unreferenced-libmagic.patch passwordsafe.spec: W: patch-not-applied Patch3: bomless-utf8-output.patch passwordsafe.spec: W: patch-not-applied Patch4: https://github.com/pwsafe/pwsafe/pull/1807.patch passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli passwordsafe.x86_64: W: incoherent-version-in-changelog 1.24.0-1 ['1.24.0^20260604git1d41f7b-1.fc45', '1.24.0^20260604git1d41f7b-1'] 3 packages and 0 specfiles checked; 0 errors, 6 warnings, 12 filtered, 0 badness; has taken 1.3 s Rpmlint (debuginfo) ------------------- Checking: passwordsafe-debuginfo-1.24.0^20260604git1d41f7b-1.fc45.x86_64.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmp9d1lh2do')] checks: 32, packages: 1 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 12 filtered, 0 badness; has taken 1.8 s Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.9.0 configuration: /usr/lib/python3.14/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 3 passwordsafe.x86_64: W: obsolete-not-provided pwsafe passwordsafe.x86_64: W: no-manual-page-for-binary pwsafe-cli passwordsafe.x86_64: W: incoherent-version-in-changelog 1.24.0-1 ['1.24.0^20260604git1d41f7b-1.fc45', '1.24.0^20260604git1d41f7b-1'] 3 packages and 0 specfiles checked; 0 errors, 3 warnings, 22 filtered, 0 badness; has taken 0.7 s Source checksums ---------------- https://github.com/pwsafe/pwsafe/archive/1d41f7b/pwsafe-1d41f7b.tar.gz : CHECKSUM(SHA256) this package : 1b639966889eba3c79588c608dfec5d9bcd892c5988d62a04fc31d3a3ea7ed08 CHECKSUM(SHA256) upstream package : cb6d82b33f7088e812d6ab556978daf3afb97d806fdd86db03ed19ff7d17ee03 diff -r also reports differences (comment: confirmed they are different checksums. Ran pkgdiff and other stuff on it, but did not see bit-wise differences, the diff must me in the metadata) Requires -------- passwordsafe (rpmlib, GLIBC filtered): hicolor-icon-theme libX11.so.6()(64bit) libXtst.so.6()(64bit) libc.so.6()(64bit) libcurl.so.4()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libm.so.6()(64bit) libmagic.so.1()(64bit) libqrencode.so.4()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.15)(64bit) libstdc++.so.6(CXXABI_1.3.2)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libuuid.so.1()(64bit) libuuid.so.1(UUID_1.0)(64bit) libwx_baseu-3.2.so.0()(64bit) libwx_baseu-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_aui-3.2.so.0()(64bit) libwx_gtk3u_aui-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_core-3.2.so.0()(64bit) libwx_gtk3u_core-3.2.so.0(WXU_3.2)(64bit) libwx_gtk3u_html-3.2.so.0()(64bit) libwx_gtk3u_html-3.2.so.0(WXU_3.2)(64bit) libxerces-c-3.3.so()(64bit) libykpers-1.so.1()(64bit) libykpers-1.so.1(LIBYKPERS_1.0)(64bit) libykpers-1.so.1(LIBYKPERS_1.15)(64bit) libykpers-1.so.1(LIBYKPERS_1.4)(64bit) libykpers-1.so.1(LIBYKPERS_1.5)(64bit) libykpers-1.so.1(LIBYKPERS_1.7)(64bit) rtld(GNU_HASH) passwordsafe-doc (rpmlib, GLIBC filtered): passwordsafe Provides -------- passwordsafe: application() application(pwsafe.desktop) bundled(pugixml) metainfo() metainfo(org.pwsafe.pwsafe.metainfo.xml) passwordsafe passwordsafe(x86-64) passwordsafe-doc: passwordsafe-doc Generated by fedora-review 0.11.0 (05c5b26) last change: 2025-11-29 Command line :/usr/bin/fedora-review -b 2468312 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: fonts, Ocaml, Python, Perl, SugarActivity, Haskell, Java, R, PHP Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
I have some additional comments (not blockers, but it'd be good to address): Please split BuildRequires: into separate lines and sort alphabetically. It makes future diffs cleaner. rm -rf src/ui/Windows rm -rf src/os/windows rm -rf src/os/mac Use rm -rv. -f makes it not fail if the files are missing in a future tarball. dos2unix config.txt dos2unix help.txt Please use dos2unix -k to keep the timestaps of the original files (https://docs.fedoraproject.org/en-US/packaging-guidelines/#_timestamps). Would it make sense to split pwsafe or pwsafe-cli to separate (sub-)package? Some admins like to avoid installing GUI libraries on headless servers if they can.
FYI I've just sponsored you, so you can go ahead with importing the package.
The Pagure repository was created at https://src.fedoraproject.org/rpms/passwordsafe Monitoring: Wasn't able to create project in Anitya. You can create it manually on: `https://release-monitoring.org`