Mark,
Here is the second half of my response.
thanks,
Moriah
Mark J. Nelson wrote:
> This is my last feedback.
>
> I explicitly did NOT review the source code, but concentrated on
> makefiles and packaging.
>
>
>
> usr/src/cmd/svr4pkg/pkgscripts:
> - What is "special_contents" used for? (Or is it unreferenced?)
> - None of the scripts specify an interpreter--I think they should.
> (#!/bin/sh)
It appears to just be unreferenced junk that is still laying around. I
removed the file (hg remove) and it didn't appear to cause any issues.
> - Usually, if you want a script to be executable, you name it "blah.sh,"
> and use the ".sh:" suffix rule to create "blah." You wouldn't even need
> to update the Makefile to accomplish this--simply rename the scripts,
> and it should work correctly.
Done, these files are actually just class action scripts. I was setting
the permissions because that was how it was handled in the Legacy
Install gate. After looking at other examples, I concluded that I do
not need to explicitly set the permissions for the scripts.
>
> usr/src/lib/libinstzones/Makefile:
> usr/src/lib/libpkg/Makefile:
> _msg: Since you're already enumerating the object files in Makefile.com,
> and you're preprocessing for message extraction, it makes much more
> sense to handle _msg in Makefile.com, instead of here. And if you're
> really being clever, for libinstzones, you only need to extract from
> zones_strings.h, using the -a flag for xgettext, and not calling the
> preprocessor.
I ended up leaving this alone. I would have liked to have cleaned this
up, but I have already had way too much fun with these Makefiles and
have limited resources. Thank you for the advice and I will file a bug
on this next week.
> install_h: both of these makefiles have an install_h target, but they're
> not listed in HDRSUBDIRS in usr/src/lib/Makefile. If you don't plan to
> put the headers into the proto area, you can skip including Makefile.lib
> and Makefile.targ, and get rid of the HDRS and HDRDIR definitions.
Removed.
>
> usr/src/lib/libinstzones/common/mapfile-vers:
> - please alphabetize
Done
>
> It looks like you're adding lint libraries to the proto area, but I
> don't see where you're delivering them. If you plan for anything
> outside of ON to use them, they belong in SUNWarc. If not, they need to
> be in a packaging exception list, don't they?
>
I do not intend for anything outside of ON to use these lint libraries.
Is there a common exception list? I found:
o usr/src/pkgdefs/etc/exception_list_i386
as well as
o usr/src/pkgdefs/etc/exception_list_sparc
I can add these libraries to both lists, but it seemed strange not to
see a common list.
