Bug 1402590
| Summary: | Review Request: ecryptfs-simple - A CLI front end to ecryptfs that works with normal user account | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Raphael Groner <projects.rg> |
| Component: | Package Review | Assignee: | Rex Dieter <rdieter> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | package-review, projects.rg, rdieter, yunying.sun |
| Target Milestone: | --- | Flags: | rdieter:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-03-11 11:49:53 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: | |||
|
Description
Raphael Groner
2016-12-07 22:22:27 UTC
(This is an un-official review from a new packager) 1. > %global _hardened_build Seems _hardened_build is set as default starting from Fedora 23, according to: https://fedoraproject.org/wiki/Changes/Harden_All_Packages Suppose this line can be removed? 2. > Source: %{url}/releases/download/2016.11.16.1/%{name}.%{version}.tar.xz Use (%version) instead of 2016.11.16.1. 3. > #BuildRequires: gcc > #BuildRequires: gcc-c++ BR for gcc & gcc-c++ would be required according to: http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B 4. > %setup -qn%{name}.%{version} missing a space before %{name}? (In reply to Yunying Sun from comment #1) > (This is an un-official review from a new packager) > > 1. > > %global _hardened_build > Seems _hardened_build is set as default starting from Fedora 23, according > to: https://fedoraproject.org/wiki/Changes/Harden_All_Packages > Suppose this line can be removed? fedora-review said to need this line. > 2. > > Source: %{url}/releases/download/2016.11.16.1/%{name}.%{version}.tar.xz > Use (%version) instead of 2016.11.16.1. OK, but this is no blocker. > 3. > > #BuildRequires: gcc > > #BuildRequires: gcc-c++ > BR for gcc & gcc-c++ would be required according to: > http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B fedora-review said to remove these lines. cmake (and rpmbuild) depends on gcc anyways. No blocker here, too. https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler > 4. > > %setup -qn%{name}.%{version} > missing a space before %{name}? OK, but this is no blocker. Fedora-review could use some bug reports for these. I'm not sure who is working on it these days and sadly I don't have the time to fix it up. %_hardened_build is set by default on all active Fedora releases. The dependencies of rpm-build can and do change; perl-generators was a dependency in F24 but isn't in F25, for example. You can assume only that you have the basic shell utilities and what's necessary for you to unpack the common archive formats (so %setup works) and for RPM to actually build packages. That's all. I can review this. naming: ok
sources: ok
09d747c5bf7e071f10c3b0833dd3400b ecryptfs-simple.2016.11.16.1.tar.xz
1. hardended_build, agree with sentiments so far, including the macro here serves no useful purpose, SHOULD remove it
2. MUST add
BuildRequires: gcc
(gcc-c++ appears unused). all dependencies must be explicitly included (should not implicitly rely on other packages to pull this in for you).
Given this addition, then you SHOULD remove:
BuildRequires: glibc-devel
(it is a direct dependency of gcc)
3. SHOULD use
make install/fast ...
instead of
%{make_install}
macro (which is tailored for autoconf/automake projects)
4. MUST remove scriptlet:
%post
chmod 4755 %{_bindir}/%{name}
if the binary needs special permissions, either fix in %install or do it via %attr, not in a scriptlet
5. licensing not ok, MUST change to
License: GPLv2
included ecryptfs-simple.c is clearly v2 only (no ... or later clause).
for item 4, in case you're unfamiliar with %attr usage, see: http://rpm.org/max-rpm-snapshot/s1-rpm-inside-files-list-directives.html ping, any update? Ad item 4: I am not sure if we intentionally need the setuid bit. Let's try clearly without. Spec URL: https://raphgro.fedorapeople.org/review/util/ecryptfs-simple.spec SRPM URL: https://raphgro.fedorapeople.org/review/util/ecryptfs-simple-2016.11.16.1-2.fc25.src.rpm %changelog * Thu Feb 26 2017 Raphael Groner <> - 2016.11.16.1-2 - remove useless _hardened_build macro - fix needed BR for gcc - use make install/fast - remove chmod scriptlet - use GPLv2 Thanks, looks good, APPROVED Thanks for the review! Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/ecryptfs-simple ecryptfs-simple-2016.11.16.1-2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-948b76f5ce ecryptfs-simple-2016.11.16.1-2.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-077a3a307d ecryptfs-simple-2016.11.16.1-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-14e1c5870c zulucrypt-5.1.0-3.fc25 ecryptfs-simple-2016.11.16.1-3.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-d3ed084484 ecryptfs-simple-2016.11.16.1-2.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-077a3a307d ecryptfs-simple-2016.11.16.1-2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-948b76f5ce ecryptfs-simple-2016.11.16.1-3.fc25, zulucrypt-5.1.0-3.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-d3ed084484 ecryptfs-simple-2016.11.16.1-3.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-14e1c5870c ecryptfs-simple-2016.11.16.1-3.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. ecryptfs-simple-2016.11.16.1-3.fc25, zulucrypt-5.1.0-3.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. |