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@xxxxxxxxxx> 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. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. -- Fedora Extras Perl SIG http://www.fedoraproject.org/wiki/Extras/SIGs/Perl Fedora-perl-devel-list mailing list Fedora-perl-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-perl-devel-list
