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 - Review Request: fakeroot - Gives a fake root environment
Summary: Review Request: fakeroot - Gives a fake root environment
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 220943
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-12-28 13:00 UTC by Axel Thimm
Modified: 2014-04-08 15:53 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-09 11:49:04 UTC
Type: ---
Embargoed:
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Axel Thimm 2006-12-28 13:00:10 UTC
Spec URL: http://dl.atrpms.net/all/fakeroot.spec
SRPM URL: http://dl.atrpms.net/all/fakeroot-1.5.10-9.at.src.rpm
Description:
Gives a fake root environment.

Comment 1 Parag AN(पराग) 2006-12-28 14:34:42 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.

Comment 2 Parag AN(पराग) 2006-12-28 14:47:02 UTC
Kindly check your other package's SPEC which looks good.
http://dl.atrpms.net/all/libcdaudio.spec

Comment 3 Axel Thimm 2006-12-28 15:06:46 UTC
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.

Comment 4 Parag AN(पराग) 2006-12-28 15:15:07 UTC
(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.

Comment 5 Axel Thimm 2006-12-28 15:19:41 UTC
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.

Comment 6 Axel Thimm 2006-12-29 12:25:20 UTC
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.


Comment 7 Jason Tibbitts 2006-12-29 17:46:24 UTC
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.

Comment 8 Axel Thimm 2006-12-29 18:05:11 UTC
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.


Comment 9 Axel Thimm 2007-01-01 14:30:15 UTC
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.


Comment 10 Peter Gordon 2007-01-04 19:20:18 UTC
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.)

Comment 11 Axel Thimm 2007-01-04 20:06:14 UTC
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.

Comment 12 Peter Gordon 2007-01-05 02:13:10 UTC
(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.

Comment 13 Axel Thimm 2007-01-05 03:12:29 UTC
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.


Comment 14 Rex Dieter 2007-01-05 03:31:59 UTC
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.

Comment 15 Peter Gordon 2007-01-05 03:46:37 UTC
(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.

Comment 16 Axel Thimm 2007-01-05 04:20:20 UTC
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.


Comment 17 Peter Gordon 2007-01-05 04:50:29 UTC
(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. :]

Comment 18 Axel Thimm 2007-01-05 05:46:28 UTC
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.

Comment 19 Mamoru TASAKA 2007-01-05 06:55:09 UTC
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.

Comment 20 Mamoru TASAKA 2007-01-07 17:10:54 UTC
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
-----------------------------------------------

Comment 21 Axel Thimm 2007-01-07 19:11:08 UTC
> * 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.


Comment 22 Mamoru TASAKA 2007-01-08 15:16:01 UTC
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.

Comment 23 Axel Thimm 2007-01-08 17:46:35 UTC
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.

Comment 24 Axel Thimm 2007-01-08 18:06:02 UTC
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


Comment 25 Mamoru TASAKA 2007-01-09 06:31:15 UTC
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.

Comment 26 Axel Thimm 2007-01-09 11:49:04 UTC
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.


Comment 27 Lubomir Rintel 2014-04-07 11:56:57 UTC
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.

Comment 28 Gwyn Ciesla 2014-04-07 12:19:23 UTC
Git done (by process-git-requests).

Comment 29 Lubomir Rintel 2014-04-08 14:34:42 UTC
Package Change Request
======================
Package Name: fakeroot
New Branches: epel7
Owners: lkundrak

Whoops wrong package name. Sorry.

Comment 30 Gwyn Ciesla 2014-04-08 15:53:32 UTC
Git done (by process-git-requests).


Note You need to log in before you can comment on or make changes to this bug.