[PATCH] jhbuildrc: Make configuration and usage easier

[Dan Nicholson]

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').

> +
> +
> +

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.

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.

> +# 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)


>  modules = [ 'xorg' ]

This is orthogonal, but can we add a comment above this setting
explaining what some of the different module sets are?

>
> -# 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.

--
Dan