Dean Roehrich wrote:
> On Wed, Nov 05, 2008 at 01:40:39AM -0600, Norm Jacobs wrote:
>
>> You made a statement about your tarball in your mail that is
>> disturbing. Your tarball should be the exact tarball that you pulled
>> down from the component's ftp or web site. If you need to apply patches
>> to the unpacked source, that's "ok", though we want you to file bugs for
>> things you are fixing with patches and submit them upstream. Hopefully,
>> as the component evolves, it will incorporate your changes and require
>> less (preferably no) patching in a future version. Specific comments
>> are below.
>>
>
> I did a cvs checkout and tar'd it up. The quilt cvs repo represents version
> 0.47 plus a few fixes that have gone in over the past few weeks; some of those
> fixes are a result of this porting effort. Two of the patches from my first
> webrev were accepted over the weekend, so for the second webrev I did another
> checkout and made another tarball.
>
In general, we have been taking the community tarballs and applying
patches to them, even when the patches are things that are in the
community SCM repository, but newer than the tarball. This makes it
clear what we have applied on top of the base foo-### component.
> For those CRs that I open for the patches, when should those CRs be closed?
>
> Do open CRs prevent me from doing a putback into SFW?
>
If you have opened up the CRs in the component's bug tracking system and
supplied the patches upstream, that's all we can ask.
>> Are the various --with-{util} options build or runtime dependencies?
>>
>
> Would you elaborate?
>
My question was do you need /usr/gnu/bin/XXX during the build, on the
installed system for quilt to use at runtime, or both? Based on one of
the packages, it would appear that they are at least runtime dependencies.
>> usr/src/cmd/quilt/install-sfw
>>
>> Lines 34-35 define an unused variable.
>>
>
> The comments should mention only those variables that I actually use?
>
If you aren't using them, delete them.
>> You might consider using the component's "install" target to
>> populate the proto area.
>>
>
> I see, $(ROOT) is the proto area.
>
> I'm running into a bit of a mess because quilt's configure script is getting
> '/opt/onbld/bin/i386/install -c' for the AC_PROG_INSTALL macro, and -c is an
> invalid option. It seems to me that autoconf in SFW should have gotten this
> right?
>
> Given this complication, I'd rather not abandon the install-sfw approach.
>
You are welcome to use the install-sfw approach, I was just pointing out
another option that is in use in the gate. The selection of the wrong
'install' is often resolved by setting "INSTALL" in the calling
environment. Eg.
(cd $(VER) ; env - INSTALL=$(INSTALL_PROTO) ... make ... install)
>> usr/src/cmd/quilt/quilt-0.47.tar.gz
>>
>> This is missing from your webrev and as a result, I am assuming that
>> it's not under teamware control. You will need to use "wx create"
>> to add it to your workspace or you will end up breaking the gate if
>> your RTI advocate doesn't catch it.
>>
>
> I wasn't sure about having binary files in webrev.
>
> I'll add it.
>
Webrev spits out some errors that can be safely ignored, but it does
indicate that it's checked into your workspace.
>> usr/src/cmd/quilt/patch.guards-man
>>
>> You should use pod2man to generate the man page from the perl file
>> during the build instead of patching this file in and having to
>> updated the patch with each update. If you were to use the
>> "install" target from the component to populate the proto area, this
>> would happen automatically, but it appears that the component
>> Makefiles may need work to honor DESTDIR.
>>
>
> The regular quilt 'make install' step does not attempt to run pod2man, or
> to install the guards manpage.
>
> Today this patch creates quilt-0.47/doc/guards.1. Instead of that, I could
> modify the 'install' target in Makefile.sfw to run:
>
> (cd $(APP); \
> /usr/perl5/bin/pod2man bin/guards.in > doc/guards.1)
>
I would prefer to see it generated from the perl file so that it matches
what they supply without having to re-patch at the next upgrade.
>> usr/src/cmd/quilt/patch.import-zcat
>>
>> This patch should be submitted back upstream. They are clearly
>> unpacking a gzipped file using zcat(1), where gzcat(1) is the
>> correct command to use.
>>
>
> No, on linux machines 'zcat' is GNU zcat. And they don't have or need a thing
> called gzcat.
>
sigh. gzip used to be installed under several different names (gzcat
being one of them) on Linux. Apparently I missed the memo. You might
use "gzip -dc" and try sending that upstream, which should be more
universal than assuming that zcat(1) on any system supports lz77 (gzip)
compression.
-Norm
