Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=519395

--- Comment #1 from Stepan Kasal skasal@redhat.com 2009-08-26 09:21:26 EDT --- Created an attachment (id=358711) --> (https://bugzilla.redhat.com/attachment.cgi?id=358711) proposed patch

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=519395

--- Comment #3 from Stepan Kasal skasal@redhat.com 2009-08-26 11:48:44 EDT --- (In reply to comment #2)

May-be you should explain which issues you are trying to solve. I don't see them.

Well, I have to admit that I'm not trying to solve any real problem, I was just replacing things I did not like with "nicer" patterns. I wanted to make the cpanspec-generated code as educative as possible, because it gets replicated many times.

Thank you, Ralf, for your comment, you made me to think more about the changes. Rationale follows.

I agree with your proposal on adopting the new buildroot,

Great. Even FPG mention that this is the best of the allowed alternatives.

• Your proposal adds further external commands (pipe, xargs), where using

find's internal -exec is more efficient.

Generally speaking, xargs is often more efficient than -exec, as it calls the command only once for many arguments. That's why I prefer it. But in this case, you are right: the rm command is called ony a few times, if ever, so it is more appropriate here.

Anyway, there is a way to combine both advantages: we can use find ... -exec rm -f {} + The plus terminator in find is defined by POSIX.

• -size 0 complies to POSIX, -empty doesn't.

Right.

• Your proposal is sensitive to white space quoting, due to using xargs

Right. To fix it, I'd have to use "-print0|xargs -0", another non-POSIX feature.

(The original construct also is, due to missing quotes)

Right, should be fixed. I think that the values in @MACROS should contain quotes and then would be used unquoted.

To sum up, you convinced me to back out my changes to first two instances of "find", except for perhaps changing the -exec delimiter to +.

But I think the situation is different with the last instance of find:

find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ;

Note that the stderr redirection does not apply to rmdir, but to the whole find command. Consequently, any error output from find is discarded.

At the very least, we should make this clear by reordering the arguments:

find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} ; 2>/dev/null

But I should it would be better to overcome this limitation by using the non-POSIX predicate -empty (-size 0 does not work for directories). That means I still support my original proposal.

I'll post an updated patch tomorrow.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=519395

--- Comment #5 from Stepan Kasal skasal@redhat.com 2009-08-27 10:43:13 EDT --- (In reply to comment #4)

...

  find ... -exec rm -f {} +

...

will be incompatible with EL-5, let alone EL-4.

Right, so I will not use it. Thank you for warning me.

OTOH, I have just checked that -empty (to detect an empty directory) is present on RHEL 4.5, so we can safely use it.

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=519395

--- Comment #7 from Stepan Kasal skasal@redhat.com 2009-08-28 14:27:32 EDT --- (In reply to comment #6)

• You are fixing anything - Don't "try to fix" what is not broken.
• You are adding new regressions,

That is a good angle of view, and you convinced me to back out some of the originally proposed changes.

But the command find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ; is very confusing and its current state. The first impression is, naturally, that it means "run rmdir with stderr redirected to /dev/null". Only after some concentration, the reader realizes that the redirection applies to the whole find command. Changing the position of the redirection is the minimum that should be done (see the end of comment #3).

But, still, running "find 2>/dev/null" is not nice. What can be done about it?

It would be nice if we were able to instruct find to execute rmdir redirecting its stderr; but all ways I can imagine are too complicated.

So I want to do a trade: sacrifice a tiny bit of portability in order to see the error messages from find. Does that sound fair?

Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=519395

--- Comment #10 from Bug Zapper fedora-triage-list@redhat.com 2010-11-04 06:22:05 EDT ---

This message is a reminder that Fedora 12 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 12. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 12 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug to the applicable version. If you are unable to change the version, please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete.

The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping