Spec URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp.spec SRPM URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp-0.7.4-1.fc22.src.rpm Description: Minimalist password safe, a wrapper around GnuPG and a text editor Fedora Account System Username: raphgro Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=10479656
Reviewing
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= -> No BuildRequires: gcc It's needed after last review of guidelines. We should have ALL BRs. -> mdp-0.7.4/src/strdelim.c has BSD (2 clause) license -> %[name] is not valid macros (%description section) -> preserve timestamps. please use '-p' for install sed -i -e "s|install |install -p |" Makefile.src -> SPEC and SRPM is inconsistent -> mdp.src:30: W: rpm-buildroot-usage %build %configure --debug --prefix=%{buildroot}%{_prefix} I'd prefer to patch to use DESTDIR instead of --prefix=%{buildroot} -> I'm not sure about some sources. For example arc4.c. It looks like bundle, but I'm not sure. Don't have time to check myself. Notes: ====== -> make %{?_smp_mflags} could be simplier %make_build -> %setup -q I'd prefer to use %autosetup -> %check does pushd but not popd -> Ignore install fails (it's because my system a bit broken, because rawhide broken deps) ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [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. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "ISC", "BSD (2 clause)", "Unknown or generated". 53 files have unknown license. Detailed output of licensecheck in /home/brain/1246790-mdp/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [!]: Macros in Summary, %description expandable at SRPM build time. Note: Macros in: mdp (description) [-]: 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. [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. [ ]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 133120 bytes in 75 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [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 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]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [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]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [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]: 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). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in mdp- debuginfo [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [!]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [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]: Uses parallel make %{?_smp_mflags} macro. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [!]: Rpmlint is run on all installed packages. Note: Mock build failed See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint [!]: 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) [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Installation errors ------------------- INFO: mock.py version 1.2.12 starting (python version = 3.4.3)... Start: init plugins INFO: selinux enabled Finish: init plugins Start: run Start: chroot init INFO: calling preinit hooks INFO: enabled root cache INFO: enabled dnf cache Start: cleaning dnf metadata Finish: cleaning dnf metadata INFO: enabled ccache Mock Version: 1.2.12 INFO: Mock Version: 1.2.12 Finish: chroot init INFO: installing package(s): /home/brain/1246790-mdp/results/mdp-0.7.4-1.fc24.x86_64.rpm /home/brain/1246790-mdp/results/mdp-debuginfo-0.7.4-1.fc24.x86_64.rpm /home/brain/1246790-mdp/results/mdp-debuginfo-0.7.4-1.fc24.x86_64.rpm ERROR: Command failed. See logs for output. # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 24 install /home/brain/1246790-mdp/results/mdp-0.7.4-1.fc24.x86_64.rpm /home/brain/1246790-mdp/results/mdp-debuginfo-0.7.4-1.fc24.x86_64.rpm /home/brain/1246790-mdp/results/mdp-debuginfo-0.7.4-1.fc24.x86_64.rpm --setopt=tsflags=nocontexts Rpmlint ------- Checking: mdp-0.7.4-1.fc24.x86_64.rpm mdp-debuginfo-0.7.4-1.fc24.x86_64.rpm mdp-0.7.4-1.fc24.src.rpm mdp.x86_64: W: manual-page-warning /usr/share/man/man1/mdp.1.gz 136: warning: macro `/'' not defined mdp.src:30: W: rpm-buildroot-usage %build %configure --debug --prefix=%{buildroot}%{_prefix} 3 packages and 0 specfiles checked; 0 errors, 2 warnings. Diff spec file in url and in SRPM --------------------------------- --- /home/brain/1246790-mdp/srpm/mdp.spec 2015-08-02 22:38:34.991601722 +0300 +++ /home/brain/1246790-mdp/srpm-unpacked/mdp.spec 2015-07-25 23:01:53.000000000 +0300 @@ -24,5 +24,4 @@ %prep %setup -q -# W: spurious-executable-perm find tests -name \*.sh |xargs chmod 644 @@ -45,5 +44,4 @@ rm functional/test_pager_timeout.sh make %{?_smp_mflags} test-all -rm -f regress/lock/fake_lock # exclude binaries from doc make clean Requires -------- mdp (rpmlib, GLIBC filtered): gnupg libc.so.6()(64bit) libncursesw.so.5()(64bit) libtinfo.so.5()(64bit) rtld(GNU_HASH) mdp-debuginfo (rpmlib, GLIBC filtered): Provides -------- mdp: mdp mdp(x86-64) mdp-debuginfo: mdp-debuginfo mdp-debuginfo(x86-64) Source checksums ---------------- https://github.com/tamentis/mdp/archive/v0.7.4.tar.gz#/mdp-0.7.4.tar.gz : CHECKSUM(SHA256) this package : 8f8c3cb4fecea9b4c4fe79aeb4eb7f92a89ae8dd36795f454dae5fc648b7ad12 CHECKSUM(SHA256) upstream package : 8f8c3cb4fecea9b4c4fe79aeb4eb7f92a89ae8dd36795f454dae5fc648b7ad12 Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 Command line :/usr/bin/fedora-review -b 1246790 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, C/C++ Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Spec URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp.spec SRPM URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp-0.7.4-2.fc22.src.rpm * Fri Aug 14 2015 Raphael Groner <> - 0.7.4-2 - mention BSD license - preserve timestamps - install into DESTDIR - use correct name macro in description All of the other points that you mention are good suggestions but not for any point of discussion in here and I assume there meant as SHOULD/CAN only. If you want, I can give you some other thoughts of my point of view, but it's out of scope here to discuss about personal preferences in alternatives for the same effect, sorry. Thanks for the review!
Task info (0.7.4-2): http://koji.fedoraproject.org/koji/taskinfo?taskID=10703194
I don't have chance to package this now: https://github.com/visit1985/mdp
(In reply to Christopher Meng from comment #5) > I don't have chance to package this now: > > https://github.com/visit1985/mdp Well, it seems that you've to be creative in finding another alternative package name. Some other example that could lead to confusion now: http://mdp-toolkit.sourceforge.net/
<rant> http://www.abbreviations.com/MDP </rant>
(In reply to Raphael Groner from comment #6) > Well, it seems that you've to be creative in finding another alternative > package name. Some other example that could lead to confusion now: > http://mdp-toolkit.sourceforge.net/ That's python-mdp. Are you serious? My mdp has 2000+ github stars, while your mdp doesn't have that. Whatever.
Christopher, could you please be more specific about your issue? Sorry but I fail to understand.
My issue is that could this package be renamed after consulting with upstream? My mdp package means (M)ark(D)own (P)resentation, and it's pretty useful and awesome with 2000+ stars on github. However I can't parse the abbreviation of this mdp package. I'm preventing name collision due to random name generated by upstream.
(In reply to Christopher Meng from comment #10) > My issue is that could this package be renamed after consulting with > upstream? Simple answer with two letters only and for the last time: No.
Suggestion: Use markdown-presentation-mdp You can add to your package a Conflicts: mdp to avoid installation of both packages and get rid of a potential binary conflict.
Chiming in because Raphael brought up the naming conflict on #fedora-devel, and neither he nor I found guidelines that give a clear-cut solution. This is the closest I found that touches the issue: https://fedoraproject.org/wiki/Packaging:Conflicts#Incompatible_Binary_Files_with_Conflicting_Naming_.28and_stubborn_upstreams.29 NB: "stubborn upstreams" and "[p]lease consider this as a last resort" -- besides the naming issue there is no reason why both packages shouldn't be able to coexist on the same system. In my very personal opinion, neither project is so well established that it would automatically give one of them precedence over the other. Anecdata: I didn't know of either before this came up. One of the projects is 2 years older, the other is apparently more popular by being "starred" more often on github. I think that neither fact gives one of them the edge. Therefore, I'd ask you two to approach FESCo to get this issue resolved: "Enter new FESCo ticket" -- https://fedorahosted.org/fesco/newticket I can't predict what they'll decide but I guess it will be something along the lines of that one or both of the projects need to have a more distinctive name for the package as well as the contained files (binaries, man pages, etc.) in Fedora. E.g. "mdp-pwsafe" and "mdp-presenter". Whether or not the upstream projects will follow suit I can't say of course.
Conflicts are within the FPC's domain, not FESCo's: https://fedoraproject.org/wiki/Packaging:Conflicts#Other_Uses_of_Conflicts:
What I do not understand: Why is it tried to use this package review to clear issues about another package (with the intention to name it equally) when it's not obviously packaged yet and there's not even a separate package request (or then a review) about that package? First come first serve, that's life. My review request is about "mdp - simplified password manager" [1], and nothing else more as well as that I ensured there's no other bug or existent package or the like with keyword "mdp" as the guidelines say. Otherwise we could misuse this review to small talk about the sense of Fedora guidelines and upstream names in an overall general. Sorry but I feel to emphasize that now so strongly, I understand the comments of Nils (comment #13) and Michael (comment #14) in that sense. This all makes me sad. [1] https://tamentis.com/projects/mdp/ Igor, please continue with the official review and we're all fine with this issue.
> However I can't parse the abbreviation of this mdp package. "Mot de Passe" https://fr.wikipedia.org/wiki/MDP -> https://fr.wikipedia.org/wiki/Mot_de_passe > This all makes me sad. Rest assured, conflicts don't have much potential to make many people *happy*. Conflicts are highly dangerous. Imagine the infamous scenario where a conflicting package is pulled in via a dependency-chain. Hell breaks lose. Especially embarrassing if this is discovered during the attempt of installing the distribution. If during review somebody points out a potential conflict of packages or files, at least there ought to be an attempt at making the fundamental problem known to the upstream author(s). Some upstreams do understand the problem and offer a better name. A less generic name. Other upstreams consider it to be entirely the packager's responsibility to deal with conflicts at the distribution level. That may even involve renaming of files everywhere within a package -- which can result in a herculean task for large/huge packages. Fedora's guidelines cannot offer much. The problem is not simple. Especially not with CLI programs in a shared $PATH and with potentially conflicting file names. No further comments from me in this ticket.
mdp is obviously an alternative to yapet [1] whose package name is also somehow misleading and what a big surprise about alternatives, and wow - it is maintained by Mr. Meng, ... [2] http://www.guengel.ch/myapps/yapet/
(In reply to Raphael Groner from comment #17) > mdp is obviously an alternative to yapet [1] whose package name is also > somehow misleading and what a big surprise about alternatives, and wow - it > is maintained by Mr. Meng, ... > > [2] http://www.guengel.ch/myapps/yapet/ This doesn't support you in this case I think, yapet was adopted by myself, I wasn't the initial submitter. And there is no conflicts among various distros[1]. Original submitter is Simon Wesp, which caused many trouble to me, for example is the surf package in Fedora. [1]---http://pkgs.org/search/?keyword=yapet
Christopher, for the very last time: Please stop to grumble just around, this gets us sad. Consult FPC with your issue, as we all suggest you. And please, better give us some suggestions from your side how to solve the issue. Thank you very much!
(In reply to Raphael Groner from comment #19) > Christopher, for the very last time: Please stop to grumble just around, > this gets us sad. Consult FPC with your issue, as we all suggest you. And > please, better give us some suggestions from your side how to solve the > issue. Thank you very much! You misunderstood that. I didn't grumble here, I didn't stop you packaging your mdp into the repo since package 'prwd', which is from the same upstream, has already been packaged and used by myself for a long time. I'd decided to quit my mdp packaging from comment 11, but you insisted on arguing with me still at comment 17 (while I was silent from comment 11 to comment 19) with try to point out yapet has the same issue. Do you really need some comments like "Ok, please go ahead", "Sorry for the trouble" or some others to build confidence to continue? I guess not, right?
Hmm while still thinking about our package name conflict, I could offer alternative package names 'mdpass' or 'mdpassword', but we should ensure the package still gets found via the upstream name 'mdp'. Renaming of binary, documentation folder and manpage with its content should be easy to do. Any wishes to apply this suggestion?
Removing Assignee cause no response since over a month. :(
Taking this one up!
Removing Assignee cause no response since over nearly two monthes. :(
Hrm, I thought we'd spoken over IRC and I'd informed you that I was going to be on holiday all through December - and I'd asked which review you wanted me to do first etc. since I'd taken up two of them?
Anyway, I don't expect any free cycles anytime soon, but I'll keep an eye on this review and take it up again when I have the time if no one else has until then. Cheers, Ankur
I'll echo other contributors to this review, and strongly suggest making an effort to avoid any possible future conflicts by: * avoid using just 'mdp' as package name * avoid using 'mdp' as binary name One good way to start that, would be to ask your upstream for alternatives.
1. SHOULD: Naming Not OK, see comment #27 (and prior discussion) 2. MUST: %%build NOT ok, builds with -O0 and generates build-time warning: /usr/include/features.h:328:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] # warning _FORTIFY_SOURCE requires compiling with optimization (-O) this is to due to configure flag: -debug, I'd recommend you remove that. While we're at it, '--prefix=%{_prefix}' is superfluous too. 3. SHOULD fix %%check, you do a 'pushd ...', but never do any 'popd' (please add one) license: ok sources: ok 18a26b7c9ce4aef6e0e2658e4ed19977 mdp-0.7.4.tar.gz scriptlets: n/a
Okay, no chance to go with the too general upstream name, obviously.
Okay, yet another try. Please continue with the review. Spec URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp.spec SRPM URL: https://raphgro.fedorapeople.org/review/gpg/mdp/mdp-0.7.4-3.fc23.src.rpm Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=13468909 %changelog * Sat Mar 26 2016 Raphael Groner <projects.rg> - 0.7.4-3 - rename binary, use namespace:project in package name - remove debug flag
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=13468940
Rex, please continue with the review.
Interesting, you kept the main pkg the same and only produce a renamed sub-package? that approach, complicates things more than a simple rename, but I won't consider that a blocker. otherwise, the important item of the compilation flags seems fixed APPROVED
Currently, we do not have any package directly named mdp, besides indirect python-mdp. At least, we have no conflict with binary name and (sub-)package installation.
Thanks for the review!
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/mdp
mdp-0.7.4-4.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-ce678bfad2
mdp-0.7.4-4.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-c907af7577
mdp-0.7.4-4.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-2016-ce678bfad2
mdp-0.7.4-4.fc23 has been pushed to the Fedora 23 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-2016-c907af7577
mdp-0.7.4-4.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.
mdp-0.7.4-4.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
Christopher, could you please be more specific about your issue? Sorry but I fail to understand. https://newsflashuk.com/
I wish i could understand you better, you can send more information about it to me, my name is GUC, via https://www.gospellife.com.ng/guc-obinigwe-lyricsvideo-mp3-download/
Hey There. I found your blog using msn. This is a very well written article. I will be sure to bookmark it and return to read more of your useful information. Thanks for the post. I will certainly return. https://www.moondev.com.ng