[email protected] wrote:

> >You don't really describe in the bug how PUBLISHALL is broken and
> >ALL_MANIFESTS is incomplete, so I don't know how to evaluate your fix.  As
> >far as I can tell, the only substantive change is the exit 0, but that's
> >got to be a teeny corner case.
> 
> Sorry, I realize it was a bit terse. The issue was that the shell
> construct was called ALL_MANIFESTS:sh rather than the expected
> ALL_MANIFESTS.<something> and then on the next line, we don't actually
> end up evaluating the shell fragment at all.

I see.  Does it work if you just remove the :sh from the definition of
MANIFESTS?  The definition of ALL_MANIFESTS is supposed to work as it is,
but I can see how trying to execute its value on the next line would give
you crud.

> So the fix is to rename
> the construct along the lines of the other cases in the Makefile and
> then properly evaluate it using $(ALL_MANIFESTS.cmd:sh).
> 
>       ALL_MANIFESTS.cmd = \
>               .
>               .
>               .
>       ALL_MANIFESTS     = $(ALL_MANIFESTS.cmd:sh)
> 
> To be consistent with the other examples, I also broke this up into
> separate lines.

Okay.

And while you're changing i386_DEFINES and sparc_DEFINES, you might as well
update line 92 (ARCH_DEFINES) to use the same whitespace pattern as
everything else.

> >Also note that there's no need to put diffs into the suggested fix.  That's
> >why we put changeset ids into the bug.
> 
> Aye, just habit out of using Bugster.

Understood.  We should stop doing this in ON, too.

Anyway, +1 on what you've got.

Thanks,
Danek
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to