Re: [PATCH] newvers.sh

In message: <20100316002939.GA36963_at_dragon.NUXI.org>
            "David O'Brien" <obrien_at_freebsd.org> writes:
: On Mon, Mar 15, 2010 at 08:44:26AM -0600, M. Warner Losh wrote:
: > In message: <20100315142806.GA5786_at_dragon.NUXI.org>
: >             "David O'Brien" <obrien_at_FreeBSD.org> writes:
: > : On Sat, Mar 13, 2010 at 09:13:03PM -0700, M. Warner Losh wrote:
: > : > In message: <20100312171206.GA31761_at_dragon.NUXI.org>
: > : >             "David O'Brien" <obrien_at_freebsd.org> writes:
: > : > : * Simplify SRCDIR calculation by directly finding the kernel sources
: > : > :   based directly on one of them.
: > : > : Reviewed by: dhw
: > : > :   This change does not increase the kernel build time.  It also continues
: > : > :   to restrict the revision to just the kernel sources, and not the whole
: > : > :   tree.
: > : > : Timing tests by: dhw
: > : > 
: > : > <patch omitted>
: > : > 
: > : > I have a better simplification, I think, that works for me for each of
: > : > the tests that I've done (both traditional and buildkernel builds).
: > : 
: > : I also have other simplifications.  But want to do this in stages,
: > : least there be some corner case or misunderstanding by someone.
: > 
: > Normally this is a sensible process.  However, my patches change how
: > we guess where sys is, and as such, I think we should go directly
: > there and work out the corner cases and such there.
: 
: Well that's what I was trying to do with the patch I sent you in
: Message-ID: <20100308010125.GA6387_at_dragon.NUXI.org> on 7-March-2010.
: There you didn't care for that approach and would not give details
: as to the claims you made.

I saw your patch and thought this one would be better and simplify
newvers.sh.  I saw nothing wrong, per se, with the patch.  Instead, I
saw an opportunity to refine it and make the resulting newvers.sh
simpler by using values already computed by the build system.

: I will reply in another email the issues with your patch.

I welcome constructive criticism of the patch.

: > : I would prefer to do this in stages so things aren't misunderstood.
: > : This patch is one of the key parts that shows I am not making the
: > : 'svnversion' run take longer, and that it does not cover more of
: > : the tree than before.
: > 
: > That's exactly what my patch does by passing in SYSDIR directly.  Why
: > guess and then replace it with a different guess.
: 
: Guess???  'newvers.sh' is not going to be within the kernel sources
: tree??  Are you allowing for the person that did a 'mv sys/conf/newvers.sh'
: to elsewhere?  Considering you have "sh $S/conf/newvers.sh", I don't see
: where you are.  Is "$0" confusing you?

The kernel source tree is inferred from $0, yes.  My method passes it
in directly, and makes newvers.sh shell variable usage more uniform
with the rest of the kernel.  It also eliminated a number of lines
from newvers.sh by using the value the kernel build process already
had cached.  The elimination of duplicated computation is why I think
this is a better approach.

Warner