On Tue, Nov 04, 2008 at 08:33:32AM +0000, Paul Cunningham wrote:
> Dean Roehrich wrote:
> >I've submitted a webrev to http://cr.opensolaris.org/~roehrich/quilt/
> >to integrate quilt into SFW.
> >
> >Quilt is a tool to manage a stack of patches; it's the predecessor to
> >Mercurial Queues (mq).
>
> === Start of Comments ====
>
> 1. usr/src/cmd/Makefile
> & usr/src/pkgdefs/Makefile
> Why is the fping stuff commented out??
Oops.
> 2. usr/src/cmd/quilt/install-sfw
> cosmetic: add line space after line 1, ie.
> 1 #! /usr/bin/ksh93
> #
> 2 # CDDL HEADER START
> 3 #
Done.
> 3. usr/src/pkgdefs/SUNWquiltr/depend
> You can remove this as you have told the build to use the
> default 'depend' in SUNWquiltr/Makefile
Done.
> 4. usr/src/pkgdefs/SUNWquiltr/pkginfo.tmpl
> & usr/src/pkgdefs/SUNWquiltu/pkginfo.tmpl
> Remove the version from the NAME= line and put it on
> the DESC= line, eg. ...
> NAME="Quilt"
> DESC="Quilt - tool to manage series of patches (0.47)"
Done.
For SUNWquiltr, should I keep the "root" in it?
DESC="Quilt - tool to manage series of patches (0.47) (root)"
> 5. usr/src/pkgdefs/SUNWquiltu/Makefile
> Remove the 'DATAFILES = depend' line unless you really
> have no other dependencies, if not remove SUNWquiltu/depend
> instead (see (6)).
I suppose that's why my dependency file kept being overwritten. Thanks. And
yes, I was trying to list dependencies for all the GNU tools that quilt uses.
> 6. usr/src/pkgdefs/SUNWquiltu/depend
> Shouldn't this depend on SUNWquiltr?
Yes.
> Have you checked it has no other dependencies with the
> dependency checker script?
I will do that.
> Move the 'Copyright' lines to after the 'CDDL HEADER END'
> header. And change year.
Done.
> 7. usr/src/cmd/quilt/Makefile.sfw
> Do you really need line ? ...
> 59 @find . -name core -exec rm -f {} \;
I'll remove that.
> I think you can also extract the version from the METADATA
> file ...
> 27 APP = $(COMPONENT_NAME:sh)-0.47
Thanks.
> Delete extra line space ..
> 88
Done.
Paul, thanks for taking the time. I'll generate another webrev after I've
tested the build with these changes.
Dean
