Bug 2050682
| Summary: | Review Request: lemonldap-ng - Web Single Sign On (SSO) and Access Management | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Xavier Bachelot <xavier> |
| Component: | Package Review | Assignee: | Emmanuel Seyman <emmanuel> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | clem.oudot, emmanuel, package-review |
| Target Milestone: | --- | Flags: | emmanuel:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://lemonldap-ng.org | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2023-11-12 07:33:13 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Attachments: | |||
|
Description
Xavier Bachelot
2022-02-04 12:56:31 UTC
This is based on upstream specfile, which has been cleaned up along the way, with all said changes included upstream. It should be in a good enough shape for a review. Builds and run on Fedora and EL7 and EL8. Update to 2.0.14. Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.0.14-1.fc37.src.rpm COPR build: https://copr.fedorainfracloud.org/coprs/xavierb/lemonldap-ng/build/3541213/ Sorry, I clean forgot about this review. Everything in the last spec looks ok, with three exceptions: a) I really don't think "LemonLDAP-NG WebSSO" makes a good summary for Fedora users. Something like "Web Single Sign On (SSO) and Access Management" would work much better. b) At one point, you make vim the default editor. Why do you do this? c) Building the package in mock and running rpmlint on the resulting binairies gives quite a lot of errors (mostly "files-duplicate" warnings in lemonldap-ng-doc.noarch). Please consider fixing this. Hi Emmanuel, Here's a start at polishing the packaging. It addresses the points you raised (not completely for duplicate files) and more, but there's still more to do. https://gitlab.ow2.org/xavierb/lemonldap-ng/-/commits/fedora_review/ I'll file a bug upstream for the duplicated doc files. And also a PR once we're both happy with the packaging. Regards, Xavier Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.0.14-3.fc37.src.rpm * Fri Apr 15 2022 Xavier Bachelot <xavier> - 2.0.14-3 - Improve Summary: - Drop updates-alternatives from %%post - Split long line - Add upstream patch to avoid duplicated and hidden files in -doc - Drop useless comments - Add provides for bundled javascript libraries and adapt License tag accordingly - Add provides for bundled font and adapt License tag accordingly - Make use of %%lm_sharedir instead of plain path - Add BR: for uglify-js - Move examples to %%_docdir - Fix perms on %%lm_vardir/captcha and %%_rundir/llng-fastcgi-server - Drop redundant dirs creation and perms change * Thu Feb 24 2022 Xavier Bachelot <xavier> - 2.0.14-2 - Prepare for EL9 support Okay, I tried to come back to this review (after putting it off for far too long). While everything I noted previously seems to have been fixed, the src.rpm no longer builds. I tried to upgrade to 2.0.15.1 but that too fails. Xavier, can you fix the build problem? Once that's done, we can (finally) finish this review once and for all. Hi Emmanuel, Here's an update to latest release. Lasso support has been disabled for F38+ but I guess it could be enabled back after the following PR to lasso has been merged: https://src.fedoraproject.org/rpms/lasso/pull-request/8 has been merged. Upstream draft PR for specfile changes for the Fedora review is here : https://gitlab.ow2.org/lemonldap-ng/lemonldap-ng/-/merge_requests/332 Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.0.16-2.fc39.src.rpm Regards, Xavier Copr build: https://copr.fedorainfracloud.org/coprs/build/5617891 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2050682-lemonldap-ng/fedora-rawhide-x86_64/05617891-lemonldap-ng/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. Rebase to latest release: Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.16.1-2.fc39.src.rpm Created attachment 1954616 [details]
The .spec file difference from Copr build 5617891 to 5728981
Copr build: https://copr.fedorainfracloud.org/coprs/build/5728981 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2050682-lemonldap-ng/fedora-rawhide-x86_64/05728981-lemonldap-ng/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. Rebase to latest release: Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.16.2-2.fc39.src.rpm Created attachment 1965280 [details]
The .spec file difference from Copr build 5728981 to 5929465
Copr build: https://copr.fedorainfracloud.org/coprs/build/5929465 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2050682-lemonldap-ng/fedora-rawhide-x86_64/05929465-lemonldap-ng/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. FTR, I've restarted the review but it's going to take a few days (this is a complex spec file). Spec URL: https://www.bachelot.org/fedora/SPECS/lemonldap-ng.spec SRPM URL: https://www.bachelot.org/fedora/SRPMS/lemonldap-ng-2.17.0-1.fc40.src.rpm Created attachment 1988071 [details]
The .spec file difference from Copr build 5929465 to 6393228
Copr build: https://copr.fedorainfracloud.org/coprs/build/6393228 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2050682-lemonldap-ng/fedora-rawhide-x86_64/06393228-lemonldap-ng/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. [fedora-review-service-build]
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.
[x]: If the package is under multiple licenses, the licensing breakdown
must be documented in the spec.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Each %files section contains %defattr if rpm < 4.4
Note: %defattr present but not needed
[-]: 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 235520 bytes in 17 files.
[x]: Package complies to the Packaging Guidelines
[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 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]: %config files are marked noreplace or the reason is justified.
[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]: No %config files under /usr.
[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]: systemd_post is invoked in %post, systemd_preun in %preun, and
systemd_postun in %postun for Systemd service files.
Note: Systemd service file(s) in lemonldap-ng-fastcgi-server
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
Perl:
[x]: Package contains the mandatory BuildRequires and Requires:.
===== SHOULD items =====
Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: Uses parallel make %{?_smp_mflags} macro.
[!]: Avoid bundling fonts in non-fonts packages.
Note: Package contains font files
[-]: 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]: Fully versioned dependency in subpackages if applicable.
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Scriptlets must be sane, if used.
[-]: 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]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate
[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:
[x]: Rpmlint is run on all installed packages.
Rpmlint
-------
[manu@orient lemonldap-ng]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm lemonldap-ng.spec
=============================================== rpmlint session starts ===============================================
rpmlint: 2.4.0
configuration:
/usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
/etc/xdg/rpmlint/fedora-legacy-licenses.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: 31, packages: 18
perl-Lemonldap-NG-Handler.noarch: E: useless-provides perl(Lemonldap::NG::Handler::Main)
perl-Lemonldap-NG-Manager.noarch: E: useless-provides perl(Lemonldap::NG::Manager::Api)
perl-Lemonldap-NG-Manager.noarch: E: useless-provides perl(Lemonldap::NG::Manager::Attributes)
perl-Lemonldap-NG-Portal.noarch: E: useless-provides perl(Lemonldap::NG::Portal::Main)
lemonldap-ng.spec:270: W: unversioned-explicit-provides bundled(js-es5-shim)
lemonldap-ng.spec:271: W: unversioned-explicit-provides bundled(js-filesaver)
lemonldap-ng.spec:289: W: unversioned-explicit-provides bundled(js-qrious)
lemonldap-ng.spec:270: W: unversioned-explicit-provides bundled(js-es5-shim)
lemonldap-ng.spec:271: W: unversioned-explicit-provides bundled(js-filesaver)
lemonldap-ng.spec:289: W: unversioned-explicit-provides bundled(js-qrious)
lemonldap-ng-fastcgi-server.noarch: W: post-without-tmpfile-creation /usr/lib/tmpfiles.d/llng-fastcgi-server.conf
lemonldap-ng-fastcgi-server.noarch: W: only-non-binary-in-usr-lib
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/cache/lemonldap-ng 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/conf 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/notifications 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/psessions 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/psessions/lock 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/sessions 750
lemonldap-ng-conf.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/sessions/lock 750
lemonldap-ng-portal.noarch: E: non-standard-dir-perm /var/lib/lemonldap-ng/captcha 750
lemonldap-ng-conf.noarch: E: non-readable /var/lib/lemonldap-ng/conf/lmConf-1.json 640
lemonldap-ng-selinux.noarch: E: non-readable /var/lib/selinux/targeted/active/modules/200/lemonldap-ng 0
lemonldap-ng.noarch: W: no-documentation
lemonldap-ng-nginx.noarch: W: no-documentation
lemonldap-ng-selinux.noarch: W: no-documentation
lemonldap-ng-test.noarch: W: no-documentation
lemonldap-ng-uwsgi-app.noarch: W: no-documentation
lemonldap-ng-conf.noarch: W: install-file-in-docs /usr/share/doc/lemonldap-ng-conf/INSTALL
lemonldap-ng-doc.noarch: W: install-file-in-docs /usr/share/doc/lemonldap-ng-doc/INSTALL
lemonldap-ng-doc.noarch: E: files-duplicated-waste 237297
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-2.3.2/img/glyphicons-halflings-white.png /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-2.3.2/img/glyphicons-halflings-white.png
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-2.3.2/img/glyphicons-halflings.png /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-2.3.2/img/glyphicons-halflings.png
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-3.4.1/fonts/glyphicons-halflings-regular.eot /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-3.4.1/fonts/glyphicons-halflings-regular.eot
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-3.4.1/fonts/glyphicons-halflings-regular.svg /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-3.4.1/fonts/glyphicons-halflings-regular.svg
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-3.4.1/fonts/glyphicons-halflings-regular.ttf /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-3.4.1/fonts/glyphicons-halflings-regular.ttf
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-3.4.1/fonts/glyphicons-halflings-regular.woff /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-3.4.1/fonts/glyphicons-halflings-regular.woff
lemonldap-ng-doc.noarch: W: files-duplicate /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootswatch-3.4.1/fonts/glyphicons-halflings-regular.woff2 /usr/share/lemonldap-ng/doc/pages/documentation/current/_static/bootstrap-3.4.1/fonts/glyphicons-halflings-regular.woff2
lemonldap-ng-handler.noarch: W: files-duplicate /usr/share/lemonldap-ng/llng-server/llng-server.psgi /usr/share/doc/lemonldap-ng/examples/handler/llng-server.psgi
lemonldap-ng-manager.noarch: W: files-duplicate /usr/share/lemonldap-ng/manager/htdocs/manager.fcgi /usr/share/doc/lemonldap-ng/examples/manager/manager.fcgi
lemonldap-ng-manager.noarch: W: files-duplicate /usr/share/lemonldap-ng/manager/htdocs/manager.psgi /usr/share/doc/lemonldap-ng/examples/manager/manager.psgi
lemonldap-ng-manager.noarch: W: files-duplicate /usr/share/lemonldap-ng/manager/htdocs/static/forms/text.html /usr/share/lemonldap-ng/manager/htdocs/static/forms/intOrNull.html
lemonldap-ng-portal.noarch: W: files-duplicate /usr/share/lemonldap-ng/portal/htdocs/index.fcgi /usr/share/doc/lemonldap-ng/examples/portal/index.fcgi
lemonldap-ng-portal.noarch: W: files-duplicate /usr/share/lemonldap-ng/portal/htdocs/index.psgi /usr/share/doc/lemonldap-ng/examples/portal/index.psgi
lemonldap-ng-portal.noarch: W: files-duplicate /usr/share/lemonldap-ng/portal/templates/common/key.png /usr/share/lemonldap-ng/portal/htdocs/static/common/icons/key.png
lemonldap-ng-selinux.noarch: W: dangerous-command-in-%pre cp
lemonldap-ng-selinux.noarch: W: dangerous-command-in-%posttrans rm
Source checksums
----------------
https://release.ow2.org/lemonldap/lemonldap-ng-2.17.0.tar.gz :
CHECKSUM(SHA256) this package : 69575a7c51edc4911d8782c621ecb2183e07150a23987218e1e66cb14f457453
CHECKSUM(SHA256) upstream package : 69575a7c51edc4911d8782c621ecb2183e07150a23987218e1e66cb14f457453
I would remove INSTALL from the %doc tag since it doesn't make sense to include it in an rpm and I guess you've won the right to update to 2.17.1 which was released a few days ago.
Your package is APPROVED.
Thanks Emmanuel :-) I knew about 2.17.1, and I did build it in my LLNG COPR, but there's no packaging changes since 2.17.0. The Pagure repository was created at https://src.fedoraproject.org/rpms/lemonldap-ng It looks like this has been released on all branches. |