After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 660844 - Default to running autogen.sh on build
Default to running autogen.sh on build
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
: 664130 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-10-03 23:42 UTC by Colin Walters
Modified: 2012-09-04 23:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add 'autogen_after_update' config option, default it to True (3.05 KB, patch)
2011-10-03 23:43 UTC, Colin Walters
rejected Details | Review
build/buildone: Always run autogen, remove alwaysautogen config option (12.11 KB, patch)
2011-10-04 14:47 UTC, Colin Walters
committed Details | Review
autotools: Do autogen if there's no Makefile (1001 bytes, patch)
2011-10-13 01:50 UTC, Colin Walters
reviewed Details | Review
build, buildone: Add --no-autogen options (2.19 KB, patch)
2011-10-13 01:50 UTC, Colin Walters
needs-work Details | Review
autotools: Don't run autogen.sh/configure if newer than configure.ac (2.21 KB, patch)
2011-11-21 16:05 UTC, Colin Walters
needs-work Details | Review
autotools: Don't run autogen.sh/configure if newer than configure.ac (4.54 KB, patch)
2012-08-27 12:29 UTC, Craig Keogh
none Details | Review
autotools: Don't run autogen.sh/configure if newer than configure.ac (4.54 KB, patch)
2012-08-27 12:48 UTC, Craig Keogh
committed Details | Review

Description Colin Walters 2011-10-03 23:42:59 UTC
This is just a request for comment - I'm not totally sure we want
to do it this way.

The build reliability problem is real though.
Comment 1 Colin Walters 2011-10-03 23:43:01 UTC
Created attachment 198168 [details] [review]
Add 'autogen_after_update' config option, default it to True

To improve build reliability, we really want to run autogen.sh after
the tree has changed.  There are several things that Automake's
maintainer-mode gets wrong, notably it won't know to rerun gtkdocize,
etc.

Honestly we should probably default to running autogen.sh, and
consider a separate command (e.g. jhbuild make) to fast-build the
current tree.
Comment 2 Colin Walters 2011-10-04 14:47:37 UTC
Created attachment 198221 [details] [review]
build/buildone: Always run autogen, remove alwaysautogen config option

For what I'm fairly sure was originally a "speed" rationale, jhbuild
has up until now required a "-a" option to enable running autogen.sh.

However, one thing I am trying hard to eradicate from jhbuild is hard
to debug build failures.  And not running autogen.sh after we've done
a git pull, and then maybe relying on the module's AM_MAINTAINER_MODE
to rerun the autotools is a big source of very very hard to debug
failures.  There are actually some situations that 'make' cannot
reasonably detect (such as switching from recursive to nonrecursive
make).

Recently we added 'jhbuild make' which is more of the developer 'make
go fast' button one can use when actively hacking on a module.  It is
now explicitly defined to skip autogen.sh.

As far as the concern "but my builds are going to be slow!!!", I have
multiple answers to that:

0) We've already landed the partial build work, so you already have
   less to build than you did historically.
1) We are going to make up some of the speed by defaulting to parallel
   make, like we should have from day 0.  A patch already exists.
2) We can be more intelligent about detecting whether we need to run
   autogen.sh (this is a bit tricky but not impossible)
3) Longer term, jhbuild will be shipping binaries from gnome.org, so
   you will really only have to build what you modify.
Comment 3 Colin Walters 2011-10-04 14:48:51 UTC
Two different approaches here - honestly I am leaning pretty strongly to the patch in comment 2.  Let's make a hard cut here for reliability, and make up in speed elsewhere.
Comment 4 Frederic Peters 2011-10-08 20:54:59 UTC
Review of attachment 198221 [details] [review]:

::: jhbuild/commands/base.py
@@ -212,3 @@
-            make_option('-a', '--autogen',
-                        action='store_true', dest='autogen', default=False,
-                        help=_('always run autogen.sh')),

Let's keep -a, --autogen, for compatibility with existing user command aliases. (same idea as the handling of alwaysautogen you kept in config.py).

@@ -306,3 @@
-            make_option('-a', '--autogen',
-                        action='store_true', dest='autogen', default=False,
-                        help=_('always run autogen.sh')),

ditto.

::: jhbuild/defaults.jhbuildrc
@@ +191,3 @@
+
+# No longer used variables
+alwaysautogen = None

Could this one be removed?
Comment 5 Frederic Peters 2011-10-08 20:56:48 UTC
Review of attachment 198168 [details] [review]:

As it can be guessed by the review of this other one, we'll do it in the second way.
Comment 6 Colin Walters 2011-10-09 14:45:43 UTC
(In reply to comment #4)
> Review of attachment 198221 [details] [review]:
> 
> ::: jhbuild/commands/base.py
> @@ -212,3 @@
> -            make_option('-a', '--autogen',
> -                        action='store_true', dest='autogen', default=False,
> -                        help=_('always run autogen.sh')),
> 
> Let's keep -a, --autogen, for compatibility with existing user command aliases.
> (same idea as the handling of alwaysautogen you kept in config.py).

Ok.
 
> Could this one be removed?

Yes, the only cost is a warning if the user has set it in ~/.jhbuildrc.  But eh, it's just a warning, and they can remove it easily.
Comment 7 Colin Walters 2011-10-09 14:46:33 UTC
Attachment 198221 [details] pushed as 96182ea - build/buildone: Always run autogen, remove alwaysautogen config option
Comment 8 Craig Keogh 2011-10-09 22:39:27 UTC
How do I not always-autogen-when-module-updates?
Comment 9 Colin Walters 2011-10-09 22:58:00 UTC
(In reply to comment #8)
> How do I not always-autogen-when-module-updates?

You can't basically.  If you're just rebuilding one thing, you can use 'jhbuild make' (though it should autogen if there's no makefile, I just hit that).

Why do you want that?  Try to compile a bunch of modules faster?
Comment 10 Craig Keogh 2011-10-12 11:02:05 UTC
(In reply to comment #9)

> Why do you want that?  Try to compile a bunch of modules faster?

Yes, faster. always-autogen-when-module-updates builds are 5 times slower. gtk+ is now 5 minutes rather than 50 seconds.

# cd gtk+
# time jhbuild make
real	0m50.250s
user	0m21.211s
sys	0m12.876s

# time jhbuild buildone -nf gtk+
real	4m59.998s
user	3m32.110s
sys	0m52.095s
Comment 11 Colin Walters 2011-10-13 01:50:24 UTC
Created attachment 198898 [details] [review]
autotools: Do autogen if there's no Makefile

It's nice if "git clean -dfx; jhbuild make" does the right thing.
Comment 12 Colin Walters 2011-10-13 01:50:30 UTC
Created attachment 198899 [details] [review]
build, buildone: Add --no-autogen options

This is a knob for people who want to try building faster, at the
possible cost of some hard to debug errors.
Comment 13 Colin Walters 2011-10-13 01:52:36 UTC
(In reply to comment #10)
> (In reply to comment #9)
> 
> > Why do you want that?  Try to compile a bunch of modules faster?
> 
> Yes, faster. always-autogen-when-module-updates builds are 5 times slower. gtk+
> is now 5 minutes rather than 50 seconds.

Note if you care about speed and have multiple cores you really want:
https://bugzilla.gnome.org/show_bug.cgi?id=654686

Anyways let me know about the attached patches here.
Comment 14 Colin Walters 2011-11-15 17:47:23 UTC
*** Bug 664130 has been marked as a duplicate of this bug. ***
Comment 15 Craig Keogh 2011-11-20 01:25:59 UTC
Review of attachment 198899 [details] [review]:

I would also like to adjust the setting in ~/.jhbuildrc too. And the manual needs update. But I can do this if you're busy :)

::: jhbuild/commands/base.py
@@ +266,3 @@
+            make_option('--no-autogen', dest='no_autogen',
+                        action='store_true', default=False,
+                        help=_('Do not run autogen.sh')),

'Do not run autogen.sh' isn't entirely true. It will still run autogen.sh if required. Subsequently I think it should be called something other than '--no-autogen'
Comment 16 Colin Walters 2011-11-20 21:43:54 UTC
One thing that occurs to me is we could probably store the timestamp for configure.{in,ac} in the packagedb, and default to rerunning autogen.sh only if it's newer.

That would give back pretty much all of the speed, while still keeping a lot of reliability.
Comment 17 Craig Keogh 2011-11-21 00:55:00 UTC
(In reply to comment #16)
> One thing that occurs to me is we could probably store the timestamp for
> configure.{in,ac} in the packagedb, and default to rerunning autogen.sh only if
> it's newer.

I like it.
Comment 18 Colin Walters 2011-11-21 16:05:13 UTC
Created attachment 201826 [details] [review]
autotools: Don't run autogen.sh/configure if newer than configure.ac

This should avoid the main problems of autoreconf != autogen.sh while
still maintaining speed in the unmodified configure.ac case.
Comment 19 Craig Keogh 2011-12-10 12:22:22 UTC
Review of attachment 201826 [details] [review]:

Thank you for the patch.

_internal_noautogen should go from commands/make.py, modtypes/waf.py and config.py.
Don't worry about adding _file_exists_and_is_newer_than to waf, as waf is rarely used.

::: jhbuild/modtypes/autotools.py
@@ +92,3 @@
 
+    def _file_exists_and_is_newer_than(self, potential, other):
+        other_stbuf = os.stat(other)

Add other_stbuf ... inside the try block. Unlikely to occur I know, but you've coded up the try block, may as well use it.
Comment 20 Craig Keogh 2011-12-15 12:09:20 UTC
Review of attachment 201826 [details] [review]:

With the patch in my tree the following occurred:

1. folks-0.6.5 (from tarball) built successfully
2. moduleset was updated to folks-0.6.6
3. run 'jhbuild tinderbox'
4. folks build fails because it didn't configure:

Building folks 2011-12-15 20:15:28.238
make V=1 -j 4
make: *** No targets specified and no makefile found.  Stop.

Inside the folks tarball timestamp is:
Dec 14 10:08 configure
Dec 14 10:06 configure.ac

The only way I can get folks building is by:
touch folks-0.6.6/configure.ac
Comment 21 Craig Keogh 2012-08-22 01:47:58 UTC
When fixing this bug, need to ensure trycheckout still works.
Comment 22 Craig Keogh 2012-08-27 12:29:48 UTC
Created attachment 222540 [details] [review]
autotools: Don't run autogen.sh/configure if newer than  configure.ac


Updated patch to address review comments. Colin, could you please review as I've listed you as author. You did 97% of the work.
Comment 23 Craig Keogh 2012-08-27 12:48:04 UTC
Created attachment 222542 [details] [review]
autotools: Don't run autogen.sh/configure if newer than  configure.ac


Rebase patch onto master.
Comment 24 Colin Walters 2012-08-27 13:04:26 UTC
Comment on attachment 198898 [details] [review]
autotools: Do autogen if there's no Makefile

Obsoleted by last attachment.
Comment 25 Colin Walters 2012-08-27 13:07:24 UTC
Review of attachment 222542 [details] [review]:

Tested this a bit, works for me.  I'm happy "git clean -dfx; jhbuild make" works now. 

Code looks correct.
Comment 26 Craig Keogh 2012-08-29 12:06:15 UTC
Comment on attachment 222542 [details] [review]
autotools: Don't run autogen.sh/configure if newer than  configure.ac


Thank you for the review Colin. Committed.
http://git.gnome.org/browse/jhbuild/commit/?id=81abb5e8de16aefd42788719ec3e880ec63dfde2
Comment 27 Colin Walters 2012-09-04 23:10:46 UTC
There is a regression from this; when the jhbuild moduleset changes configure arguments, but the module hasn't updated their configure.ac, we won't rebuild, whereas before, we did.

Concretely I had a build of tracker around for a while, I'd updated my jhbuild checkout to get http://git.gnome.org/browse/jhbuild/commit/?id=3021c46d6d6c807f8bfdec86c16c239f4cbcaf48 but jhbuild didn't rerun configure, so the new argument wasn't used.
Comment 28 Colin Walters 2012-09-04 23:12:18 UTC
If we wanted to handle this, I think we'd need to store the arguments used to build (or a checksum) in the install data, and compare it to decide whether or not to skip autogen.
Comment 29 Craig Keogh 2012-09-04 23:24:20 UTC
Thank you Colin. That bug was present before this bug was implemented (the 1st implementation), so I've opened a new bug, see bug 683374.