On 03/30/2012 10:14 AM, Edwin Beasant wrote:
On 30/03/2012 16:57, Rich Burridge wrote:
...
* lines 93-94 where is CLEAN_PATHS used?
It's used in the prep.mk, line 119 part of the clean rule.
Ah, thanks.
* line 32: Have you passed the proposed package name by David (Comay)?
Personally, I feel the name needs at least one more hierarchical level,
but David should be able to advise you here.
The package names are as they are in ON currently, were originally
planned to remain that way to ease the migration (based on a package
upgrade methodology).
Then that's fair enough. Somebody (probably with David's input) made the
decision to
name that package that name at some point in time, so who am I to argue.
I also looked in the PSARC 2012/002 case, and it's not listed there.
Whatever the package name ends up being, it should be an exported
interface for that PSARC case.
The intention for the PSARC case was to record the deltas from ON, and
that this migration be treated as such, rather than to introduce and
redocument the existing ksh93 interfaces.
These package names did originate from the great renaming, but as per
above, they will remain the same as ON. The original ksh93 case
2006/550
(http://psarc.us.oracle.com/PSARC/2006/550/materials/proposal) doesnt
mention a package name either (it would have been SUNwcs)
There was never a PSARC case specifically to document all the old -> new
names in
the Great Renaming. I believe we've been relying on new cases to include
these
new package names as they come to ARC. It's really irrelevant as related
to this
code review, but it would have been nice if it had been included in the
PSARC
case materials.
* Why is this in a separate package? It seems that it would be useful
(and
not too expensive size-wise) to just include it in the standard
ksh93.p5m
package manifest.
The idea was to preserve the parity with ON to enable the migration as
per the planned methodology with minimum fuss. This doesn't preclude
the integration of this into the main ksh93 package in the future.
Yup. Fair comment. Why it was ever a separate package there is beyond the
scope of this review.
.../make-rules/attpackagemake.mk:
* line 71: do you want to use $(GMAKE) here instead of just "make"?
We've
seen a bug in another Userland component recently where it was picking
up the wrong make.
In this case, no - 'make' is an instruction to the bin/package system
to build the package, not a binary name. (eugh !)
Ack! Sorry, didn't pick up on that.
.../make-rules/prep.mk:
* lines 24-25 I agree that we need to patch in sequence, but I
thought that
that was what the Userland build infrastructure did. Am I wrong?
Otherwise
how are the numbered patches (seen with some components) going to be
applied in numerical order? I'm guessing (from looking at lines 63-78)
that you've changed things here. Does your new method still work with
all
other Userland components (and that numerical patches continue to be
applied
in the correct order for those components that use them)?
I've made the behaviour backwards compatible with the previous
incarnation:
The patches were applied in the filename collation order as provided
by sort. The problem is that the original system could download
multiple tarballs, but could not:
1. Unpack and/or relocate tarballs in order - every package that uses
multiple tarballs currently manually unpacks them after downloading.
2. Patch patches after each tarball unpacking - in the AT&T case,
several tarballs and patches have to be overlaid, to form the source
tree.
When PATCH_EACH_ARCHIVE is non-null, the behaviour of specifying
patches for each archive/tarball, in order is available.
Each archive will be downloaded, unpacked (optionally relocated), post
unpack rules executed and patched in turn.
Okay. Thanks for the explanation. If the existing components just
continue to
be patched correctly, that's fine by me.
Cheers for taking the time:
I have re-rolled the webrev with the changes mentioned above,
These look good to me (as related to my comments).
I'd like somebody else to review this (prep.mk in particular).
Thanks.
_______________________________________________
userland-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/userland-discuss
