[PATCH] jhbuildrc: Make configuration and usage easier

[Dirk Wallenstein]

On Mon, Apr 26, 2010 at 06:33:01AM -0700, Dan Nicholson wrote:
> On Mon, Apr 26, 2010 at 12:38 AM, Dirk Wallenstein <halsmit at t-online.de> wrote:
> > Simply passing jhbuildrc to jhbuild will start a complete build with the
> > same locations used previously, namely ~/xorg and ~/xorg-build. These
> > locations can be easily changed by specifying paths in the configuration
> > section.
> >
> > Signed-off-by: Dirk Wallenstein <halsmit at t-online.de>
> > ---
> >  jhbuildrc |   69 +++++++++++++++++++++++++++++++++++++++++++++++++-----------
> >  1 files changed, 56 insertions(+), 13 deletions(-)
> >
> > diff --git a/jhbuildrc b/jhbuildrc
> > index de6a037..6e795f0 100644
> > --- a/jhbuildrc
> > +++ b/jhbuildrc
> > @@ -1,22 +1,65 @@
> >  # how to use this file?
> >  #
> > -# mkdir -p $HOME/xorg/util
> > -# git clone git://anongit.freedesktop.org/git/xorg/util/modular/ $HOME/xorg/util/modular
> > -# jhbuild -f $HOME/xorg/util/modular/jhbuildrc
> > -#
> > -# Source tree will be in $HOME/xorg
> > -# Binaries will be in $HOME/xorg-build
> > -#
> > +# Make a copy of this file and fill in the two configuration variables, or
> > +# leave them empty to use the defaults. Then start the build process with:
> > +#        $> jhbuild -f <your-edited-copy-of-this-file>
> > +
> > +# Passing this file to jhbuild will automatically create the source path and
> > +# clone the modular repository to obtain the module list.
> > +
> > +#-------- Configuration ------------------------------------------
> > +
> > +# Specify the destination path for the sources.
> > +# An empty string will select '~/xorg'.
> > +# You may use a '~' or '~user' preamble.
> > +checkoutroot = ""
> > +
> > +# Specify the destination path for the installation.
> > +# An empty string will select '~/xorg-build'.
> > +# You may use a '~' or '~user' preamble.
> > +prefix = ""
> > +
> > +
> > +#-------- Internal -----------------------------------------------
> > +
> > +# Notes:
> > +# 'prefix' will be created by jhbuild itself.
> > +
> > +
> > +# Fallback to the default for prefix, or expand possible '~user' spec
> > +if len(prefix):
> > +    prefix = os.path.expanduser( prefix )
> > +else:
> > +    prefix = os.path.join(os.environ['HOME'], 'xorg-build')
> > +
> > +# Fallback to the default for checkoutroot, or expand possible '~user' spec
> > +if len(checkoutroot):
> > +    checkoutroot = os.path.expanduser( checkoutroot )
> > +else:
> > +    checkoutroot = os.environ['HOME']
> 
> This doesn't set checkoutroot to '~/xorg' like the comment above says.
> I guess you want os.path.join(os.environ['HOME'], 'xorg').

Ups, I was misguided. I thought the 'xorg' subdirectory created by
jhbuild was due to the specification in the variable 'modules', but it
is just a result of the checkoutdir attributes in xorg.modules. In
consequence, I think it is best to deviate from the previous default for
checkoutroot, which was '~', and use something more meaningful like
os.path.join(os.environ['HOME'], 'xorg-source') as default. 

I've put the default values directly into the configuration section and
just kept the expanduser call.

I just realized that the checkout from build.sh deviates from the one
created by jhbuild with the current xorg.modules. JHBuild keeps the real
structure and therefore has xorg and xcb top-level directories - which I
think is good, if you take augmenting the moduleset with additional
repos into consideration. But it might make mixing jhbuild and build.sh
problematic. Maybe a simple
$> ln -s xorg .
in the repo root is enough, but I haven't tried that, yet.

> > +
> > +
> > +
> 
> Can you remove some of the gratuitous newlines? A single newline is
> probably all that's needed. Two newlines when you really need to break
> one section from another. Three is just wasting space.
Fixed.

> Also, personal style, but I'd rather not have the padding around the
> arguments in parentheses. Either way, have the style be consistent in
> the file.
Fixed. Don't know why I skipped a final formatting pass.

> > +# Checkout util/modular if nonexistent
> > +modular_destination = os.path.join( checkoutroot , 'xorg' , 'util' , 'modular')
> > +
> > +if not os.path.exists( modular_destination ):
> > +    try:
> > +        os.makedirs( modular_destination )
> > +    except OSError, error:
> > +        sys.exit( "Could not create the path to the modular repo (%s). "
> > +                    "The error is : %s" % ( modular_destination , error ) )
> > +    if os.system( "git clone git://anongit.freedesktop.org/git/xorg/util/modular/"
> > +                    " '%s'" % modular_destination ) != 0:
> > +        sys.exit( "Could not clone util/modular" )
> > +
> > +
> >
> > -#moduleset = 'http://cgit.freedesktop.org/xorg/util/modular/blob/xorg.modules'
> > -# Requires the module set be in $HOME/xorg/util/modular/
> > -moduleset = os.path.join(os.environ['HOME'], 'xorg', 'util', 'modular', 'xorg.modules')
> > +#moduleset = 'http://cgit.freedesktop.org/xorg/util/modular/xorg.modules'
> > +moduleset = os.path.join( modular_destination , 'xorg.modules')
> 
> If you're already editing jhbuildrc, don't you already have modular? I
> guess there would be two major use cases.
> 
> 1. Already have modular and have tweaked jhbuildrc to point to the
> module file I want. In this case, cloning modular and/or setting the
> moduleset to that cloned file is unnecessary.
>
> 2. Got jhbuildrc from somewhere (not modular?) and just want to start
> building. That seems to be what we have here, and I'm glad to see you
> addressing it.
>
> Maybe something like this could help.
> 
> # Set this to False if you already have a module file
> checkout_moduleset = True
> modular_destination = os.path.join(checkoutroot , 'xorg' , 'util' , 'modular')
> moduleset = os.path.join(modular_destination , 'xorg.modules')
> 
> if checkout_moduleset:
>     clone modular
> 
> if not os.path.isfile(moduleset):
>     sys.exit("Module file \"%s\" does not exist" % moduleset)

I've put the modular clone operation into a function, which will also
inhibit the warning concerning the unknown configuration variable. I
have paired the call with the moduleset assignment and added a comment
how to omit that checkout. 

The second use case was the intention. Just add the direct download link
and the new inital comment from this patch to a wiki page, and a user
might be able to quickly build a complete Xorg with jhbuild, while
avoiding the need to know the more difficult parts of the configuration.

> 
> >  modules = [ 'xorg' ]
> 
> This is orthogonal, but can we add a comment above this setting
> explaining what some of the different module sets are?
Done. 

> >
> > -# All modules will be in $HOME/xorg/ after the checkout
> > -checkoutroot = os.environ['HOME']
> > -prefix = os.path.join(os.environ['HOME'], 'xorg-build')
> >  os.environ['ACLOCAL'] = 'aclocal -I ' + os.path.join(prefix, 'share', 'aclocal')
> >  os.environ['PKG_CONFIG_PATH'] = os.path.join(prefix, 'lib', 'pkgconfig') \
> >     + ':' + os.path.join(prefix, 'share', 'pkgconfig')
> 
> Seems look a nice improvement for the most part, but I'd like to
> discuss some more before committing to master.

Wonderful. I'm trying to figure out jhbuild more closely, and any hints
are welcome. I'm currently trying to add some more commented examples
for additional jhbuild configuration variables like module_autogenargs.
I'll test that and will send a completely reworked v2 soon.

> 
> --
> Dan

Greetings,
Dirk