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
those lines

>  * maybe rename _ESCPATH so that it's obvious it's a version of PKGPATH,
>    e.g. PKGPATH_ESC

will do.

>  * 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.

-Dan

--