<comments in-line>
James Carlson wrote:
> [http://cr.opensolaris.org/~mwaterl/6739234.changes_only/webrev/]
>
> Moriah Waterland writes:
>>> 75: nit: format string should be const, and should have /*
>>> PRINTFLIKE1 */ here.
>> Done, only sort of. I added the /* PRINTFLIKE1 */ but had problems with
>> changing the format string. Please see Note 7.
>
> The cascade errors are due to the definitions of functions like echo()
> and echoDebug() in libinst.
>
>>> usr/src/lib/libinstzones/common/instzones_lib.h
>>> 305,307,309,344,363,366: fmt should be const.
>> Left alone. Tried this change, but ran into build problems in
>> usr/src/cmd/svr4pkg. I had made the changes that you suggested as well
>> as adding const to the function definitions for:
>> o _z_add_arg()
>> o _z_program_error
>> o _z_strPrintf_r
>> o _z_strPrintf
>> o _z_echo
>> o _z_echoDebug
>> o _z_program_error()
>> Please see Note 7. for more information about build issues. I would be
>> happy to file a bug on this if you like. Please just let me know.
>
> Please do file a bug.
>
Done. Bug is:
6843636 libinstzones has fmt strings that should be const
>>> usr/src/lib/libpkg/common/p12lib.c
>>> usr/src/lib/libpkg/common/p12lib.h
>>> Please seek out legal help with this part. I don't think that
>>> putting this code (which clearly belongs to someone else) under CDDL
>>> is the right thing to do. The original Sun copyright notice
>>> (without CDDL) was probably correct.
>>>
>>> Plus, you'll need to update the third party license files.
>> Removed CDDL from both p12lib.c and p12lib.h. In usr/src/lib/libpkg, I
>> created a THIRDPARTYLICENSE file and included the OpenSSL copyright and
>> license as well as the AT&T copyright. I also created the
>> THIRDPARTYLICENSE.descrip file. I modified the package definition for
>> SUNWpkgcmdsu to include these licenses.
>
> OK. As long as you've had the necessary legal reviews and you've
> followed the instructions in $SRC/README.license-files, I'm happy.
Done. I also spoke with Bonnie about this and she agreed with your
assessment that the CDDL should not be in those files.
>>> usr/src/lib/libpkg/common/pkgweb.c
>>>
>>> 1831-1832: what is this?
>> Removed comments. But left the code changes there. The changes were
>> required to address the problems from tmpfile[] array being declared as
>> static, but was then redefined multiple times elsewhere in the file.
>>
>> + /ws/onnv-tools/SUNWspro/SS12/bin/cc
>> .
>> .
>> "../common/pkgweb.c", line 1832: warning: name redefined by pragma
>> redefine_extname declared static: tmpfile (E_PRAGMA_REDEFINE_STATIC)
>> cc: acomp failed for ../common/pkgweb.c
>> *** Error code 2
>
> The reason the compiler was complaining is that "tmpfile" is the name
> of a standard library function. The name of the variable itself is
> bad, not the storage specifier. Removing "static" doesn't fix this
> problem properly.
>
> In fact, removing "static" creates a new problem. The code is now
> broken because of line 1886: this array is returned to the caller. It
> *cannot* be automatic storage. It must remain static.
>
Fixed. Replaced "static" and then went through pkgweb.c changing changed
all definitions and calls to "tmpfile" into "tmp_file".
>>> usr/src/pkgdefs/SUNWpkgcmdsr/prototype_com
>>>
>>> 34-36: assignments shouldn't be needed here.
>> Left alone. I think that they are necessary. I tried removing these
>> assignments, but then I saw the following errors:
>> opensores# make pkg
>> pkgmk -f prototype_i386 -d
>> /export/ws/mwaterl/stepchild_of_master/packages/i386/nightly-nd \
>> -r "/export/ws/mwaterl/stepchild_of_master/proto/root_i386 " -o
>> SUNWpkgcmdsr
>> ## Building pkgmap from package prototype file.
>> pkgmk: ERROR: unable to open pkginfo file <0&>: No such file or
>> directory
>> pkgmk: ERROR: Invalid combinations of zone parameters in pkginfo file
>> ## Packaging was not successful.
>> *** Error code 1
>> make: Fatal error: Command failed for target `pkg'
>
> I'm not willing to sign off on that one just yet. This requires some
> deeper investigation. Those assignments make no sense, and I'm not
> sure that the package is being produced properly even when it shows no
> errors.
>
> (It concerns me because we have IPS conversion down the road
> somewhere, and "weird" packages will likely make that job harder.)
>
Done. Umm, well I think that this resulted from a misunderstanding on
my part. I removed those assignments and the package builds fine.
>>> usr/src/pkgdefs/SUNWpkgcmdsu/prototype_com
> [...]
>>> 80: we typically don't ship compilation symlinks for internal
>>> libraries. Will anything outside of ON compile against this?
>> Eventually no, nothing outside of ON should compile against this. But,
>> it is currently being used by projects in the new OpenSolaris installer:
>> o http://src.opensolaris.org/source/xref/caiman/slim_source/
>
> Oh. You might consider using a SUNWpkgcmdsint (internal) package to
> deliver consolidation private objects.
Done. I ended up calling it SUNWinstallint.
We plan to use this package during the process of integrating chunks of
the new OpenSolaris installer.
