GNOME Bugzilla – Bug 654555
Don't have two ways to be installed
Last modified: 2011-10-09 00:11:45 UTC
There are two ways to build/install jhbuild: * Normal ./configure --prefix=..., make; make install * make -f Makefile.plain install For the latter case, PKGDATADIR is None, and all code referencing data files have to handle both PKGDATADIR and SRCDIR. This is a maintenance and code pain. We could fix this by instead installing jhbuild in ~/.local, i.e. ~/.local/bin/jhbuild, ~/.local/share/jhbuild. Downside: Some people have ~/bin in their path, ~/.local/bin is not by default.
I like the idea. What I would like is one way: ./autogen.sh --prefix=...; make; make install That just works if you have autotools installed or not, gnome-common installed or not, gnome-doc-utils installed or not. (Use autogen.sh rather than configure so configure isn't in git, and most people use JHBuild from git)
Created attachment 192940 [details] [review] Enhance autogen.sh so only 1 way to install JHBuild If gnome-common and gnome-doc-utils are available, autogen.sh will configure JHBuild to install via autotools. If gnome-common and gnome-doc-utils are not available, autogen.sh will configure JHBuild to install via a plain Makefile. autogen.sh is used to configure JHBuild because the most common way to obtain JHBuild is via git and a 'configure' should not be checked into git.
Created attachment 192941 [details] [review] [doc] Update manual for 1 way to install JHBuild
Created attachment 192942 [details] [review] Add /Makefile.inc to .gitignore
Review of attachment 192940 [details] [review]: Very clever approach overall. Did you test it in both setups? ::: autogen.sh @@ +46,3 @@ +# characters. Invalid variable names are ignored. +# parse_commandline is not using getopt as getopt doesn't support the long form +# on Solaris, BSD and MacOS. Kind of gross code; you could consider writing autogen.sh in Python you know. It's not like jhbuild will run without Python anyways. (But not a problem if you're happy with the shell script) @@ +113,3 @@ +configure_with_autotools() +{ + touch $srcdir/ChangeLog # required for automake I just pushed a commit which should fix this: http://git.gnome.org/browse/jhbuild/commit/?id=b69c27b30785874b5e8f312c9b36630a742deef2
(In reply to comment #5) > Review of attachment 192940 [details] [review]: > > Very clever approach overall. Did you test it in both setups? Yes, I did test both. Here is a use case I didn't cater for: What is someone is following some old instructions and tries the Makefile.plain method without running autogen.sh. There should be a message to guide the user in this case. I'll fix this. > Kind of gross code; you could consider writing autogen.sh in Python you know. > It's not like jhbuild will run without Python anyways. Yes, agree the code is ugly. I never thought of writing it in Python. Not a bad idea. There may be some bash-only code in my autogen.sh and may not work on all platforms - using Python would alleviate that problem. Thank you for the review.
(In reply to comment #6) > > Yes, agree the code is ugly. I never thought of writing it in Python. Not a bad > idea. There may be some bash-only code in my autogen.sh and may not work on all > platforms - using Python would alleviate that problem. It's up to you, as far as I'm concerned it's good to commit (after removing the ChangeLog bit).
What about i18n? Something like this [1]. (Side node: i18n is easier in Python so that adds strength to the 'rewrite in Python' argument) After this bug is fixed, need to roll new JHBuild tarball and update the online manual as soon as possible. Maybe consider landing this bug same time as bug 646510. Then only one tarball required. [1] http://www.gnu.org/s/hello/manual/gettext/sh.html
Created attachment 194902 [details] [review] Enhance autogen.sh so only 1 way to install JHBuild I've incorporated: - Colin's comment - added i18n support - Display an error message if a user tries the old Makefile.plain method (i.e. following outdated instructions)
Created attachment 194903 [details] [review] Enhance autogen.sh so only 1 way to install JHBuild I've incorporated: - Colin's comment - added i18n support - Display an error message if a user tries the old Makefile.plain method (i.e. following outdated instructions) Attachment 194902 [details] had the wrong file mode. Now fixed.
This still looks good to me. Go ahead and commit, Craig?
Frédéric, If you agree, could you push the 3 patches and roll a JHBuild tarball? The changes affect the way JHBuild is installed - I want the JHBuild install instructions [1] to be accurate for new users. I'll update the wiki. [1] http://developer.gnome.org/jhbuild/stable/
It failed distcheck (autogen.sh was not distributed), fixed, pushed, and released a 3.2.1 tarball.
Guys, I guess bug 648300 is now obsolete because ofthe fix for this? Can you confirm? Thanks
The changes to autogen.sh introduced in the commit fixing this bug are not compatible with all sh implementations, e.g. dash used in debian and ubuntu. Either the script needs to be fixed to work with those shells or you'll need to replace #!/bin/sh by #!/bin/bash.
Thanks. commit 87546661a7bc4bf562675a354eb2125597831288 Author: Frédéric Péters <fpeters@0d.be> Date: Sat Oct 8 17:02:57 2011 -0400 Switch autogen.sh to /bin/bash as it uses bashisms (GNOME bug 654555)
Created attachment 198633 [details] [review] Proposed patch I'm not used to write shell scripts, but I think I found a workaround for the bashisms using expr command from coreutils. I'm attaching a patch that works for me (/bin/sh linked to dash), but maybe needs a deeper review.
Thanks Frédéric! I updated the wiki [1]. [1] http://live.gnome.org/Jhbuild