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-1.fc25.src.rpm Description: A CLI front end to ecryptfs that works with normal user account Fedora Account System Username: raphgro Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=16790894 ecryptfs-simple is needed for a feature in zulucrypt 5.0.2: "" Upstream is very much alive,i created this fork just to put the code on github because upstream isnt here. Its easier to keep track of changes,accept bug reports,pull requests and have discussions when projects are hosted "properly". Functionality wise,this forks adds support for unlocking volumes in read only mode but i think upstream will eventually add it. "" https://github.com/mhogomchungu/ecryptfs-simple/issues/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? 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.