Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 220888
Summary: | Review Request: fakeroot - Gives a fake root environment | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Axel Thimm <axel.thimm> |
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | lkundrak, peter, rdieter |
Target Milestone: | --- | Flags: | gwync:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-09 11:49:04 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: | |||
Bug Depends On: | 220943 | ||
Bug Blocks: | 163779 |
Description
Axel Thimm
2006-12-28 13:00:10 UTC
When i look at SPEC only i found %{_libdir}/libfakeroot/libfakeroot.a %{_libdir}/libfakeroot/libfakeroot.la above things should not be added and they must be removed from installation. Following file should be installed from -devel package %{_libdir}/libfakeroot/libfakeroot-0.so you need to add -devel package. Kindly check your other package's SPEC which looks good. http://dl.atrpms.net/all/libcdaudio.spec What does libcdaudio have to do with this submission? :) I'm removing static and libtool (although the latter has been in investigation by the FPC whether it is really neccessary), but it makes no sense to add a devel subpackage with a single symlink in it. Other than that libfakeroot-0.so isn't a symlink, but libfakeroot.so is, so it would had been the latter that would need to be cast out. But the latter is the one that is being used by fakeroot, so it isn't even possible to split these two. Note that the so files found here are not conventional shared libs that are intended to be linked against, but are dsos for preloading ld.so, e.g. part of runtime proper. (In reply to comment #3) > What does libcdaudio have to do with this submission? :) You are right. My comment was not complete. I am really sorry for that. I should have mentioned that check what other SPEC did with .so* files installation. > > I'm removing static and libtool (although the latter has been in investigation > by the FPC whether it is really neccessary), but it makes no sense to add a > devel subpackage with a single symlink in it. > > Other than that libfakeroot-0.so isn't a symlink, but libfakeroot.so is, so it > would had been the latter that would need to be cast out. But the latter is the > one that is being used by fakeroot, so it isn't even possible to split these two. > > Note that the so files found here are not conventional shared libs that are > intended to be linked against, but are dsos for preloading ld.so, e.g. part of > runtime proper. OK. P.S. This package is missing a BR in Fedora (po4a), so it's not 100% ready for review, yet. I'm commiting the missing BR shortly and will block this submission with it then. OK, the missing packages are now submitted as well - note that the dependency depth is three more packages found in bug #220943, bug #220944 and bug #220945. Spec URL: http://dl.atrpms.net/all/fakeroot.spec SRPM URL: http://dl.atrpms.net/all/fakeroot-1.5.10-10.at.src.rpm * Thu Dec 28 2006 Axel Thimm <Axel.Thimm> - 1.5.10-10 - Don't build static lib. - Exclude libtool lib. - %%makeinstall to make install DESTDIR=%%buildroot. Is it possible to expand upon the %description a bit? I can't discern the function of this package from reading what's currently there. Even the summary from the web page: fakeroot provides a fake "root environment" by means of LD_PRELOAD and SYSV IPC or TCP trickery. is better, although it's still not all that explanatory. Indeed, maybe something along the lines of the following (assembled from the manpage): fakeroot runs a command in an environment wherein it appears to have root privileges for file manipulation. fakeroot works by replacing the file manipulation library functions (chmod(2), stat(2) etc.) by ones that simulate the effect the real library functions would have had, had the user really been root. But I won't yet respin a package with only this change, e.g. whoover reviews this, please assume the description will be the one above. Spec URL: http://dl.atrpms.net/all/fakeroot.spec SRPM URL: http://dl.atrpms.net/all/fakeroot-1.5.10-12.at.src.rpm * Sun Dec 31 2006 Axel Thimm <Axel.Thimm> - 1.5.10-12 - Add %%{_libdir}/libfakeroot to %%files. - Add %%check. * Fri Dec 29 2006 Axel Thimm <Axel.Thimm> - 1.5.10-11 - Extend the %%description a bit. A couple of comments (with full review coming later tonight when I get home from work): The gcc-c++ and /usr/bin/getopt (part of the util-linux package) BuildRequires are not nessary, as they are part of the standard build root (see http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions for more information). Your BuildRoot tag is incorrect. It should be "%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)" as specified in the guidelines. (See http://fedoraproject.org/wiki/Packaging/Guidelines#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1 for more information.) Thanks for reviewing this! Please note that to build this package you need to build three other packages submitted (see the blocking bugs). If you would review them as well, that would be great! o buildroot: The wording in the guidelines is the `preferred' one, and it has been discussed that we should skip or change that. In any case that should not be a blocker. http://www.redhat.com/archives/fedora-packaging/2006-July/msg00167.html o `redundant' BRs: They do not hurt and are also tagged as `not needed', not `forbidden'. Especially gcc-c++/libstdc++ could be removed from future minimal roots for an easy gain of space. It's already been removed in other buildsystems. (In reply to comment #11) > Thanks for reviewing this! Please note that to build this package you need to > build three other packages submitted (see the blocking bugs). If you would > review them as well, that would be great! I'll take care of those ones as well. :] > o buildroot: The wording in the guidelines is the `preferred' one, and it has > been discussed that we should skip or change that. In any case that should not > be a blocker. > > http://www.redhat.com/archives/fedora-packaging/2006-July/msg00167.html It was "discussed"; but no conclusion in that thread (or any similar that I can find via a brief Google search) has arisen. The consensus (with which I agree) is that using a terser BuildRoot breaks multi-user build environments and causes more trouble than it's worth. Until rpm-build (or whatever ends up carrying it) defaults to a sane buildroot (at which point we can stop putting them in our spec files entirely), we should keep what is in the Guidelines; and doing otherwise is therefore a blocker. Please fix that (in the other ones you're packaging too). Thank you. > o `redundant' BRs: They do not hurt and are also tagged as `not needed', not > `forbidden'. Especially gcc-c++/libstdc++ could be removed from future minimal > roots for an easy gain of space. It's already been removed in other > buildsystems. Understood. Thanks for the explanation. I disagree that there was a consensus in the way you interprete it. :)
There was only one member that insisted on turning the current buildroot
recommendation to a mandatory item, while the rest was convinced that the
"preferred buildroot" isn't really a good one as it breaks multi-arch builds,
which is a far more common case than multi-user builds (Note: We are always
talking of the same package with the same nevr!!!). To quote:
The reviewers need to be whacked with a clue-stick. A working (non-broken)
buildroot is *not* a blocker."
> we should keep what is in the Guidelines; and doing otherwise is therefore a
> blocker
This is not a MUST item, so this can't block the package. And if you still think
strongly about it you can and should revive the discussion on fedora-packaging.
Peter, BuildRoot is a should/recommended/preferred item (not MUST) in the Guidelines. If you're going to be inflexible here, please withdraw yourself as reviewer, and let someone else do it. (In reply to comment #13) > > we should keep what is in the Guidelines; and doing otherwise is therefore a > > blocker > > This is not a MUST item, so this can't block the package. And if you still think > strongly about it you can and should revive the discussion on fedora-packaging. (In reply to comment #14) > Peter, BuildRoot is a should/recommended/preferred item (not MUST) in the > Guidelines. If you're going to be inflexible here, please withdraw yourself > as reviewer, and let someone else do it. Actually, it *is* a MUST item. The ReviewGuidelines explicitly state: "MUST: The package must meet the Packaging Guidelines." Part of the packaging guidelines is this BuildRoot determination. Though it is only a highly recommended value when the usage of "preferred" is literally taken; it is the spirit of the guidelines that makes this a requirement IMHO. The guidelines are there to ensure that packages going into Fedora all meet the same level of high quality assurance standards. While it is understandable that there are those packages which cannot meet specific guidelines for valid reasons, making the tag more terse than it already is is not one of them to me. (No disrespect intended, but how hard is it, really, to copy and paste the "BuildRoot: ..." line from the Guidelines wiki page? ^_^) If the Guidelines mean otherwise, why does it not mention any proper alternative? We seem to be in much disagreement about this, having reached somewhat of an impasse on these terms. I'm afraid I am therefore unable, in good conscience, review your submitted packages. Unfortunately, from a cursory glance at the .spec and rpmlint, this appears to be the only major issue that I can spot. Take care. I'm sorry that you feel so strongly about that that you had to drop three (!!!)
reviews. What you consider a blocker is far from being one, both technically
from a specification POV (because a mandatory following of the guidelines that
designate a "preferred" buildroot explicitely does not make that unique and
leaves room for many other buildroots) as well from a semantically POV since the
implied brokenness wrt to multi-users is smaller than the one wrt multi-arch as
present in the "preferred" buildroot (while both are almost neglidgible
corner-cases anyway).
> No disrespect intended, but how hard is it, really, to copy and paste the
> "BuildRoot: ..." line from the Guidelines wiki page? ^_^)
Rephrasing the above: How hard is it, really, to simply let the current
BuildRoot pass?
Anyway I think fakeroot is a useful package and it will probably find another
reviewer. Still thanks for trying to do the review in the first place.
(In reply to comment #16) > What you consider a blocker is far from being one, both technically > from a specification POV (because a mandatory following of the guidelines that > designate a "preferred" buildroot explicitely does not make that unique and > leaves room for many other buildroots) [...] The whole purpose of guidelines and QA rules in this manner is to ensure that all potential packages conform to them for the benefit of all (or don't if there is good and valid reason not to do so). > [...] as well from a semantically POV since the > implied brokenness wrt to multi-users is smaller than the one wrt multi-arch as > present in the "preferred" buildroot (while both are almost neglidgible > corner-cases anyway). The way you have your BuildRoot setup, though, adds this multi-users brokenness to the already existent multi-arch breakage that could occur. If it in fact fixed it but still differed from what the Guidelines show, then I would definitely not consider it a blocker; and post an RFC for this to the relevant mailing list(s). > > No disrespect intended, but how hard is it, really, to copy and paste the > > "BuildRoot: ..." line from the Guidelines wiki page? ^_^) > > Rephrasing the above: How hard is it, really, to simply let the current > BuildRoot pass? It is very difficult for me, quite frankly. I am one of the main personnel who closely handle inventory control for my employer (a microelectronics R&D firm); and me approving of that incorrect BuildRoot would be akin in many ways to my signing a bill of materials, work order traveler, or other project document which contains information I know to be incorrect, incomplete, or otherwise improper. It's simply not the right thing to do. (As I said, though: if I felt that this reasoning for the deviation was good and valid, I'd let it through for certain.) > Anyway I think fakeroot is a useful package and it will probably find another > reviewer. Still thanks for trying to do the review in the first place. As do I. I hope your submission succeeds and wish you well. Hopefully this disagreement between us does not become unnecessary animosity. :] Please, let me restate that this no conflict with the packaging guidelines. The "preferred" can be at best interpreted as a SHOULD requirement, so by definition it is nothing to block on, and it is also nothing that "slips" QA in any way. I would even consider the demythifying of the needed `id -un` urban legend as an improvent in package quality even if it jeopardizes the use-case where several users build exactly the same package at the same time, which probably happened already twice since the passing of the millenium (and I mean the first one ;). > > > how hard is it, really, to copy and paste the > > > "BuildRoot: ..." line from the Guidelines wiki page? ^_^) > > Rephrasing the above: How hard is it, really, to simply let the current > > BuildRoot pass? > It is very difficult for me Well, that makes two stubborn packagers out of us, doesn't it? Anyway I need to stay hard on this as a long-time educational measure. id -un only landed in there due to early misunderstanding that "root" didn't mean a user but a build-"root" and the id -un obfusaction serves little to nothing. > > Anyway I think fakeroot is a useful package and it will probably find > > another reviewer. Still thanks for trying to do the review in the first > > place. > As do I. I hope your submission succeeds and wish you well. Hopefully > this disagreement between us does not become unnecessary animosity. :] Well, my Peter Voodo doll is rather pinned up right now, but no, there are no hard feelings for someone expressing his sincere opinion, even if it's diametrical opposite to mine. Now let me fire up that doll. ;) P.S. if you feel this discussion needs to be carried on, please let's leave bugzilla for the review and discuss this preferably on fedora-packaging or any other list, we abused bugzilla for discussing the packaging guidelines. One comment: I prefer using %(%{__id_u} -n) because it "may be" safer. However also I know that not a few Fedora contributors, including who frequently reviews other person's review requests or even who is a sponsor, disagree with "mandatory" use of %(%{__id_u} -n). Currently this is still under discussion, in my opinion. I suggest for rather new contibutors to use %(%{__id_u} -n), however I don't force people to use this currently who seems to have some reasons why they dislike this. So.. as Axel says, this is a issue of fedora-packaging mailing list (if someone thinks that discussion is needed... however it seems that this has been discussed already), not on _this_ bugzilla report, and should not be a blocker. Some question(s) and issue(s) * Is "po4a" really needed for rebuilding this? I can rebuild this without "po4a" by mockbuild on FC-devel i386. It seems that po4a is needed when you want to re-create some documents? * -------------------------------------- BuildRequires: gcc-c++ BuildRequires: /usr/bin/getopt (util-linux) -------------------------------------- These are not needed because these are included in minimal build environment. * I think adding debian/changelog is preferable because the upsteam of this package is debian people (correct?). * Does this package fail on parallel make? * rpmlint complains: ----------------------------------------------- E: fakeroot zero-length /usr/share/doc/fakeroot-1.5.10/README ----------------------------------------------- > * Is "po4a" really needed for rebuilding this? Indeed, you are right, the latest release says: "Drop po4a from build-deps." I'll comment BR: po4a away (but keep it in case the next release forgets again to process the language files). > zero-length [...]/README [...] debian/changelog Hm, a bit sloppy in upstream. I'll add debian/changelog, but I'd like to keep the zero-sized README in case it (hopefully) gets filled with content. I.e. better to carry an empty file, than to forget to check upon the next package update. What do you say? I wouldn't really object, if you insist on removing it. | o `redundant' BRs: They do not hurt and are also tagged as `not needed', not | `forbidden'. Especially gcc-c++/libstdc++ could be removed from future | minimal roots for an easy gain of space. It's already been removed in other | buildsystems. Well, then please check if %{_smp_mflags} is okay? If adding this bears no problem, please add this. (In reply to comment #21) > > zero-length [...]/README [...] debian/changelog > > What do you say? I wouldn't really object, if you insist on removing it. Well, then I don't force to remove it. I had already fixed the above topics and I thought you'd probably prefer to remove the empty README, so it's gone now. Spec URL: http://dl.atrpms.net/all/fakeroot.spec SRPM URL: http://dl.atrpms.net/all/fakeroot-1.5.13-9.at.src.rpm * Sun Jan 7 2007 Axel Thimm <Axel.Thimm> - 1.5.10-13 - po4a currently not need as a BR. - remove empty README, add debian/changelog. The problem with parallel makes is that they are not really testable. Every invocation can have different ordering and results (it depends on the runtime needed per parallel target). In principle ony would haveto review the Makefile structure to be sure and this has to be done each time. So I rather play it safe than be sorry. Last comment had a typo in the src.rpm's URL, it's SRPM URL: http://dl.atrpms.net/all/fakeroot-1.5.10-13.at.src.rpm Okay. Now: ------------------------------------------------ This package (fakeroot) is APPROVED by me. ------------------------------------------------ INFO: I checked http://ftp.debian.org/debian/pool/main/f/fakeroot/ and it seems that 1.5.12 is released YESTERDAY. Thanks for the review of fakeroot and all three dependent packages! Since 1.5.10 was approved, I started by importing this version before testing the new one. I'll test 1.5.12 and issue an update before the freeze. Package Change Request ====================== Package Name: Pound New Branches: epel7 Owners: lkundrak The Fedora maintainer (athimm) does not maintain EPEL packages [1], has no EL-6 branch. The EL-5 maintainer (rwmjones) does not want to maintain it. Git done (by process-git-requests). Package Change Request ====================== Package Name: fakeroot New Branches: epel7 Owners: lkundrak Whoops wrong package name. Sorry. Git done (by process-git-requests). |