GNOME Bugzilla – Bug 741891
gnome-autogen variables checks
Last modified: 2014-12-23 15:34:10 UTC
Some additional checks for deprecated properties and misc improvements (btw git-bz fails to file bug with "A legal Version was not set." error)
Created attachment 293213 [details] [review] README: remove check for automake in example It's usually not required, so let's not show a bad example
Created attachment 293214 [details] [review] gnome-autogen: infer PKG_NAME from configure.ac Let's not repeat ourself in autogen.sh, there are already many places with the package name.
Created attachment 293215 [details] [review] gnome-autogen: warn for deprecated variables Well, many people still use deprecated variables for 10y, maybe it's time to let them know.
Created attachment 293216 [details] [review] gnome-autogen: warn if $srcdir is undefined Not setting $srcdir will lead to broken out-of-tree autogen call, so let's ask to set it
These patches look good to me, but I do wonder if we still actually need gnome-autogen.sh. Loads of projects do without it fine, using a hard-coded autogen.sh which just calls the few necessary tools these days (autoreconf and ./configure; maybe gtkdocize too).
Review of attachment 293213 [details] [review]: Fine.
Review of attachment 293214 [details] [review]: ::: macros2/gnome-autogen.sh @@ +41,3 @@ echo "$@" echo $ECHO_N "$normal" $ECHO_C +} Unintended whitespace change. @@ +46,3 @@ } +PKG_NAME=`autoconf --trace "AC_INIT:$1" "$srcdir/configure.ac"` Shouldn't this be after the autoreconf version check?
Review of attachment 293215 [details] [review]: Fine.
Review of attachment 293216 [details] [review]: ::: macros2/gnome-autogen.sh @@ +62,3 @@ +if [ -z "$srcdir" ]; then + printerr "***Warning*** \$srcdir is not defined in autogen, out of dir build is broken!" Misleading warning. An out-of-tree build may work fine, it is just the autogen.sh step that would be broken by not having srcdir defined.
(In reply to comment #9) > > Misleading warning. An out-of-tree build may work fine, it is just the > autogen.sh step that would be broken by not having srcdir defined. Hmm should I rephrase? It's a broken "autogen" if you can't call path/to/autogen.sh && make ?
(In reply to comment #7) > Shouldn't this be after the autoreconf version check? Well, version_check uses PKG_NAME, so in case of error, it wouldn't print the name of the project, but this is really minor since the sentence woulld still be correct. If autoconf is missing, PKG_NAME will be empty, and it would print the same error. So one way or the other, it doesn't change much.
(In reply to comment #5) > These patches look good to me, but I do wonder if we still actually need > gnome-autogen.sh. Loads of projects do without it fine, using a hard-coded > autogen.sh which just calls the few necessary tools these days (autoreconf and > ./configure; maybe gtkdocize too). Well, I am also used to call autoreconf. But recently, I have been annoyed by so many broken autogen.sh, in particular very few handle out-of-tree build needed by jhbuild "buildroot" option. Or call intltoolize && gettextize in wrong way, or do not have NOCONFIGURE check etc.. Also, many autogen end up longer than just autoreconf, but in many case, replacing autoreconf with gnome-autogen will solve almost all issues and reduce autogen.sh. So, I think it's a good idea to keep and use gnome-autogen at this point.
Review of attachment 293216 [details] [review]: (In reply to comment #10) > (In reply to comment #9) > > > > Misleading warning. An out-of-tree build may work fine, it is just the > > autogen.sh step that would be broken by not having srcdir defined. > > Hmm should I rephrase? It's a broken "autogen" if you can't call > path/to/autogen.sh && make ? "autogen" seems fine.
Created attachment 293252 [details] [review] gnome-autogen: warn if $srcdir is undefined Not setting $srcdir will lead to broken out-of-tree autogen call, so let's ask to set it
Review of attachment 293252 [details] [review]: ::: macros2/gnome-autogen.sh @@ +39,3 @@ echo "$@" echo $ECHO_N "$normal" $ECHO_C +} Undesired whitespace change should be removed. @@ +353,2 @@ topdir=`pwd` +for configure_ac in $configure_files; do Undesired whitespace change.
Attachment 293214 [details] pushed as 89f74f5 - gnome-autogen: infer PKG_NAME from configure.ac Attachment 293215 [details] pushed as 4c8d8ad - gnome-autogen: warn for deprecated variables Attachment 293252 [details] pushed as 8dd7146 - gnome-autogen: warn if $srcdir is undefined
(In reply to comment #12) > (In reply to comment #5) > > These patches look good to me, but I do wonder if we still actually need > > gnome-autogen.sh. Loads of projects do without it fine, using a hard-coded > > autogen.sh which just calls the few necessary tools these days (autoreconf and > > ./configure; maybe gtkdocize too). > > Well, I am also used to call autoreconf. But recently, I have been annoyed by > so many broken autogen.sh, in particular very few handle out-of-tree build > needed by jhbuild "buildroot" option. Or call intltoolize && gettextize in > wrong way, or do not have NOCONFIGURE check etc.. > > Also, many autogen end up longer than just autoreconf, but in many case, > replacing autoreconf with gnome-autogen will solve almost all issues and reduce > autogen.sh. So, I think it's a good idea to keep and use gnome-autogen at this > point. Hmm, yes. I just think that it seems a bit pointless to have a dependency on gnome-common for (essentially) a bit of shell script. Perhaps NOCONFIGURE is something which should be submitted to upstream autoconf (so configure is generated with an "if NOCONFIGURE; then exit 0; else pass; fi" bit at the beginning.