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 683374 - if changes to configure arguments, force run autogen.sh
if changes to configure arguments, force run autogen.sh
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other All
: Normal enhancement
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
Depends on:
Blocks:
 
 
Reported: 2012-09-04 23:21 UTC by Craig Keogh
Modified: 2012-09-13 12:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Re-run autogen.sh if autogen arguments change (10.97 KB, patch)
2012-09-05 12:38 UTC, Craig Keogh
reviewed Details | Review
Re-run autogen.sh if autogen arguments change (11.08 KB, patch)
2012-09-06 13:14 UTC, Craig Keogh
committed Details | Review

Description Craig Keogh 2012-09-04 23:21:00 UTC
From bug 660844 comment 27: (thanks Colin Walters)
when the jhbuild moduleset changes configure arguments, but the module hasn't updated their configure.ac, we won't rebuild.

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.

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 1 Craig Keogh 2012-09-05 12:38:58 UTC
Created attachment 223526 [details] [review]
Re-run autogen.sh if autogen arguments change
Comment 2 Colin Walters 2012-09-05 17:57:56 UTC
Review of attachment 223526 [details] [review]:

Just one minor comment...looks fine to me in general.

::: jhbuild/modtypes/autotools.py
@@ +94,3 @@
         return potential_stbuf.st_mtime > other_stbuf.st_mtime
 
+    def _get_configure_cmd(self, buildscript):

This function is scary...but as far as I can see you've correctly transplanted the code.

@@ +173,3 @@
+        if db_entry:
+            configure_hash = db_entry.metadata.get('configure-hash')
+            if configure_hash:

Note if done this way, we won't detect configure changes until the module is rebuilt.  That's probably fine, but worth noting.

The other alternative would be to:

if configure_hash is None:
   return False

Thus forcing a one-time reautogen of modules, and then they'd all have configure-hash.
Comment 3 Craig Keogh 2012-09-06 06:06:15 UTC
Review of attachment 223526 [details] [review]:

Thank you for the review.

::: jhbuild/modtypes/autotools.py
@@ +94,3 @@
         return potential_stbuf.st_mtime > other_stbuf.st_mtime
 
+    def _get_configure_cmd(self, buildscript):

I don't like scary. What is scary? Any suggestions?

@@ +173,3 @@
+        if db_entry:
+            configure_hash = db_entry.metadata.get('configure-hash')
+            if configure_hash:

I couldn't decide which is better:
- force a one-time reautogen
- reautogen in slow time, but may miss some autogenargs changes.

I think I like force a one-time reautogen.
Comment 4 Craig Keogh 2012-09-06 13:14:11 UTC
Created attachment 223646 [details] [review]
Re-run autogen.sh if autogen arguments change


Incorporated 'force one-time reconfigure if no configure-hash'.
Comment 5 Colin Walters 2012-09-06 13:27:31 UTC
(In reply to comment #3)
> Review of attachment 223526 [details] [review]:
> 
> Thank you for the review.
> 
> ::: jhbuild/modtypes/autotools.py
> @@ +94,3 @@
>          return potential_stbuf.st_mtime > other_stbuf.st_mtime
> 
> +    def _get_configure_cmd(self, buildscript):
> 
> I don't like scary. What is scary? Any suggestions?

Eh, it's not your fault - it's just a lot of ugly code accumulated over time.  I don't have any good ideas for how to fix it.  Let's just keep it as is.

> I think I like force a one-time reautogen.

Yeah, me too.
Comment 6 Colin Walters 2012-09-06 13:28:00 UTC
Review of attachment 223646 [details] [review]:

Looks good.
Comment 7 Craig Keogh 2012-09-07 07:51:24 UTC
Comment on attachment 223646 [details] [review]
Re-run autogen.sh if autogen arguments change

Committed.
http://git.gnome.org/browse/jhbuild/commit/?id=cc2bcbe0c0cb85538866fa1c87af7ddb94fd9c48
Comment 8 Marcin Wojdyr 2012-09-12 15:25:53 UTC
the construction of the configure command is quite complicated now,
I was puzzled for a while why the first time I compile a module 'configure' is run, and the second time jhbuild tries to run 'autogen.sh', which is not there.

The reason is that if _get_configure_cmd() is called from skip_configure(), then configure_cmd is cached and these lines in do_configure() have no effect:

        if self.autogen_sh == 'autogen.sh' and \
        not os.path.exists(os.path.join(srcdir, self.autogen_sh)):
            # if there is no autogen.sh, automatically fallback to configure
            if os.path.exists(os.path.join(srcdir, 'configure')):
                self.autogen_sh = 'configure'

I think it would make sense to move this snippet to _get_configure_cmd()
Comment 9 Craig Keogh 2012-09-13 12:45:43 UTC
(In reply to comment #8)
> the construction of the configure command is quite complicated now,
> I was puzzled for a while why the first time I compile a module 'configure' is
> run, and the second time jhbuild tries to run 'autogen.sh', which is not there.
> 
> The reason is that if _get_configure_cmd() is called from skip_configure(),
> then configure_cmd is cached and these lines in do_configure() have no effect:
> 
>         if self.autogen_sh == 'autogen.sh' and \
>         not os.path.exists(os.path.join(srcdir, self.autogen_sh)):
>             # if there is no autogen.sh, automatically fallback to configure
>             if os.path.exists(os.path.join(srcdir, 'configure')):
>                 self.autogen_sh = 'configure'
> 
> I think it would make sense to move this snippet to _get_configure_cmd()

Thank you for the clear explanation. Fixed.

http://git.gnome.org/browse/jhbuild/commit/?id=5a7ec65a42bd16cdc716ea6df5c3ce9430b54341