[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 3 Sep 2014 10:56:10 +0400
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: recent updates
Galaxy,
On Sat, Jul 12, 2014 at 09:18:34PM +0400, (GalaxyMaster) wrote:
> On Sat, Jul 12, 2014 at 06:57:13PM +0400, Solar Designer wrote:
> > Thanks! I saw some weird stuff in your commits, on which I'll likely
> > comment later.
>
> I'll gladly accept your comments.
I feel that along with the changes that should have been made you've
also been making totally unnecessary and inconsistent changes, like:
%install
-rm -rf %buildroot
+rm -rf -- '%buildroot'
and even:
%install
-rm -rf %buildroot
+[ '%buildroot' != '/' -a -d '%buildroot' ] && rm -rf -- '%buildroot'
These would make some sense if we were to move all packages to the same
new conventions, but historically we had decided not to bother checking
for '/' in that "rm" at least in spec files that are only part of Owl
(and are not packaged into tarballs in ~/archives/ for distribution
separately from Owl). Anyway, if you feel we want to introduce this
ancient paranoia in here, I suppose we could... but it'd only make sense
if we do it consistently. BTW, coreutils' rm now has this option, which
we could use:
--preserve-root
fail to operate recursively on `/'
As to putting macros in single quotes, which you similarly don't do
consistently, this also makes little sense to me. A lot of things
will fail in many places in the build scripts if e.g. %buildroot
contains a bad character... and putting it in single quotes does not
prevent a problem if %buildroot expands into a string containing the
single quote character itself.
Then, in a few spec files you switched patch file names from explicit to
using %name. I think this is not helping (I prefer being able to
copy-paste full patch file names), especially when inconsistent.
There are some added instances of "#exit 123" in spec files, I guess you
just forgot to remove them before committing.
Your use of "--" is sometimes weird:
rm -rf -- tmp-check
("tmp-check" is a hard-coded directory name here, so you know it does
not start with a dash.) I don't even see much reason to use "--" with
%buildroot and other macros, since we're not supporting arbitrarily
weird values for them anyway. Why pretend that we do?
If we actually want to make our buildworld safer, maybe buildworld.sh
should sanity-check its $HOME setting to ensure it doesn't contain "bad"
chars? I doubt we'll ever bring Owl to a state, let alone keep it in
that state, where buildworld would succeed with any weird characters in
%buildroot. There are too many third-party Makefiles, etc. involved,
and it'd be a waste of time to be patching them for a non-issue.
> Truth to be told, our previous
> extraction of RPM was broken even worse. At least my current
> implementation relies on bash, dd, and xz on the host system and
> extracts all dependencies, where the original one was using host's
> rpm2cpio and was extracting rpm binaries only (forgetting about
> dependencies like libpopt, etc.).
Hmm. IIRC, our RPM binary extraction had been tested at some point,
when upgrading from older versions of Owl with older RPM. Maybe some
breakage got in at a later time.
Thanks,
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
