[Bug 1804529] Review Request: pam-cryptsetup - PAM module for updating LUKS-encrypted volumes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1804529

Michel Alexandre Salim <michel@xxxxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(loganjerry@gmail.
                   |                            |com)



--- Comment #6 from Michel Alexandre Salim <michel@xxxxxxxxxxxxxxx> ---
(In reply to Jerry James from comment #4)
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> 
> Issues:
> =======
> - If your application is a C or C++ application you must list a
>   BuildRequires against gcc, gcc-c++ or clang.
>   Note: No gcc, gcc-c++ or clang found in BuildRequires
>   See: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/
> 
Added

> - Remove "rm -rf %{buildroot} $RPM_BUILD_ROOT" at the start of %install.  See
>   the third bullet point here:
>  
Removed, thanks. Not sure why rpmdev-newspec's template still has this line!
I'll file a patch for that too

> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections
> 
> - There is an unused direct library dependency:
>   $ ldd -u /usr/lib64/security/pam_cryptsetup.so
>   Unused direct dependencies:
>           /lib64/libcryptsetup.so.12
> 
>   This is due to libtool reordering -Wl,--as-needed *after* the libraries to
> be
>   linked.  I usually insert this snippet into my spec files, between
> %configure
>   and %make_build, to address that issue.  I'm offering this as a suggestion;
>   take it or leave it as you wish.
> 
> # Workaround libtool reordering -Wl,--as-needed after all the libraries.
> sed -i 's|CC="\(.*g..\)"|CC="\1 -Wl,--as-needed"|' libtool
> 
Thanks!

> - Another suggestion: it is possible to give a valid URL for Source0.  See
>   https://fedoraproject.org/wiki/Packaging:SourceURL#Commit_Revision.  In
> this
>   case, the URL would be something like this (you would have to add a
>   definition for %commit):
> 
>   Source0: %{url}/archive/%{commit}/%{name}-%{snapshot_rev}.tar.gz
> 
>   The benefit is that you can use spectool to download future versions of the
>   tarball.
> 
Believe it or not, coming back to packaging after a while, I tried to find that
page and couldn't. Thanks!

Spec URL: https://salimma.fedorapeople.org/specs/admin/pam-cryptsetup.spec
SRPM URL:
https://salimma.fedorapeople.org/specs/admin/pam-cryptsetup-0.1-0.3.20190823.7b42892.fc32.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux