Subject: Re: patches to bulk build
To: Hubert Feyrer <firstname.lastname@example.org>
From: Dan McMahill <dmcmahill@NetBSD.org>
Date: 03/10/2005 20:10:50
On Fri, Mar 11, 2005 at 12:34:46AM +0100, Hubert Feyrer wrote:
> On Thu, 10 Mar 2005, Dan McMahill wrote:
> >Can someone do a sanity check over this patch? It works around
> >the problem where grep on solaris (and maybe others) drops long
> >lines in input files. It seems to be working for me on solaris-2.9,
> >but I'd appreciate comments on a cleaner way to do this.
> I don't understand what the code does, but looking at the diff, here are a
> few comments:
avoids feeding long line to grep which has the rude behaviour of dropping
> * maybe rename _ESCPATH so that it's obvious it's a version of PKGPATH,
> e.g. PKGPATH_ESC
> * in the second hunk (@@ -298,5 +300,5 @@), you replace a "grep
> | sed" call with just a (more complex) sed call. Maybe make that
> "sed | sed" (with the second one being the old sed call, unchanged),
> so it's more obvious (and more readable) on what's going on.
> * @@ -316,5 +318,7 @@: the original patch looks for "( |$$)" at the end,
> while the new version only seems to look for a space (after $$pkgdir2).
> Is that correct? (No idea what the substitution to 'yes' does.
yes. There is always a space at the end.
> * @@ -346,5 +350,5 @@: Substitution patterns "^.*:" and "^[^:]*" aren't
> exactly the same IIRC (greedy match on the former). Is that intended?
> (Maybe only change one thing at a time :) Occurs in other places!
right, the former can match more than what's wanted
> * @@ -397,5 +401,7 @@: same as above, with "^[^:]*:[ \t]*" replacing
same here. the regexp wasn't quite right.
> Can you test this on NetBSD before committing? (Just to be sure :)
> Maybe doing so on a restricted bulk build with only 1-2 pkgs would be
> enough (I forgot what variable that can be set with).
it's on tomorrow's todo list.