Bug 81769 – [GTK] Add make to the jhbuild moduleset

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Garcia Campos]

To add make and apply custom patches to fix several issues:

 - The argument list is too long error that happens when running make distcheck
 - Parallel build issues of make 3.82, see bug #79498

[Carlos Garcia Campos]

Created attachment 133030 [details]
Patch

[Philippe Normand]

Comment on attachment 133030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133030&action=review

Looks good but I'd like the EFL guys to test this before landing

> Tools/Scripts/update-webkit-libs-jhbuild:-53
> -chdir(relativeScriptsDir() . "/../".$platform) or die $!;

Are you sure this needs to be removed?

[Philippe Normand]

Can the EFL folks have a look at this patch and test it please?

[Carlos Garcia Campos]

(In reply to comment #2)
> (From update of attachment 133030 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133030&action=review
> 
> Looks good but I'd like the EFL guys to test this before landing
> 
> > Tools/Scripts/update-webkit-libs-jhbuild:-53
> > -chdir(relativeScriptsDir() . "/../".$platform) or die $!;
> 
> Are you sure this needs to be removed?

That was used to run jhbuild directly, we are now using the wrapper script.

[Philippe Normand]

Comment on attachment 133030 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133030&action=review

> Tools/ChangeLog:8
> +        Add bootstrap.modules with GNU make and custom patches top fix

s/top/to

Spotted by drott

> Tools/Scripts/update-webkit-libs-jhbuild:60
> -my @jhbuildArgs = ("../../WebKitBuild/Dependencies/Root/bin/jhbuild", "--no-interact", "-f", "jhbuildrc");
> -push(@jhbuildArgs, @ARGV[2..-1]);
> -if (system(@jhbuildArgs) != 0) {
> -    die "Running jhbuild failed.\n"
> -}
> +runJhbuild("build");

Hum you no longer pass "--no-interact", "-f", "jhbuildrc" to jhbuild?

[Dominik Röttsches (drott)]

When cleaning up WebKitBuild and running from scratch:

Updating EFL port required support tools using jhbuild...
jhbuild bootstrap: failed to parse bootstrap: [Errno 2] No such file or directory: 'bootstrap'
Running jhbuild-wrapper bootstrap failed.
Traceback (most recent call last):
  File "/fast/dominik/dev/WebKitGit_EFL/Tools/efl/../Scripts/../../Tools/jhbuild/jhbuild-wrapper", line 136, in <module>
    ensure_jhbuild()
  File "/fast/dominik/dev/WebKitGit_EFL/Tools/efl/../Scripts/../../Tools/jhbuild/jhbuild-wrapper", line 126, in ensure_jhbuild
    update_webkit_libs_jhbuild()
  File "/fast/dominik/dev/WebKitGit_EFL/Tools/efl/../Scripts/../../Tools/jhbuild/jhbuild-wrapper", line 110, in update_webkit_libs_jhbuild
    raise Exception('jhbuild configure failed with return code: %i' % process.returncode)
Exception: jhbuild configure failed with return code: 1

[Carlos Garcia Campos]

(In reply to comment #5)
> (From update of attachment 133030 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133030&action=review
> 
> > Tools/ChangeLog:8
> > +        Add bootstrap.modules with GNU make and custom patches top fix
> 
> s/top/to
> 
> Spotted by drott
> 
> > Tools/Scripts/update-webkit-libs-jhbuild:60
> > -my @jhbuildArgs = ("../../WebKitBuild/Dependencies/Root/bin/jhbuild", "--no-interact", "-f", "jhbuildrc");
> > -push(@jhbuildArgs, @ARGV[2..-1]);
> > -if (system(@jhbuildArgs) != 0) {
> > -    die "Running jhbuild failed.\n"
> > -}
> > +runJhbuild("build");
> 
> Hum you no longer pass "--no-interact", "-f", "jhbuildrc" to jhbuild?

yes, jhbuild-wrapper does it

[Martin Robinson]

It isn't too important to build JHbuild itself with a patched make, so why not just add make and the patches to the main moduleset? Wouldn't that make this patch a lot smaller?

[Carlos Garcia Campos]

Created attachment 133044 [details]
Updated patch to fix efl build

Check whether bootstrap moduleset file exists before trying to run jhbuild bootstrap

[Carlos Garcia Campos]

(In reply to comment #8)
> It isn't too important to build JHbuild itself with a patched make, so why not just add make and the patches to the main moduleset? Wouldn't that make this patch a lot smaller?

Because it's not a dependency.

[Dominik Röttsches (drott)]

(In reply to comment #9)
> Created an attachment (id=133044) [details]
> Updated patch to fix efl build

Works for me. Thanks.

[Carlos Garcia Campos]

I've split this patch, see bug #81888

[Carlos Garcia Campos]

Created attachment 133224 [details]
New patch

Instead of running bootstrap in update-webkit-libs-jhbuild, it simply adds a new step for the bots to run bootstrap before installing the dependencies. Developers who use build-webkit and want to install patched make only have to run Tools/jhbuild/jhbuild-wrapper --gtk bootstrap
This way we don't need to change anything in efl

[Philippe Normand]

Comment on attachment 133224 [details]
New patch

LGTM, Martin, WDYT?

[Gustavo Noronha (kov)]

Like I said on IRC, I would prefer to have this on the existing jhbuild.modules. I understand this would be incorrect from a conceptual point of view, since make is not a dependency, but I think practicality beats purity (Python Zen™) in this case. Adding another step will certainly add more bugs we'll need to fix. In any case, if we do add the bootstrap stuff, we need to make sure it's run in Tools/Scripts/webkitdirs.pm, here (~ line 1900) :

    # We might need to update jhbuild dependencies.
    if (checkForArgumentAndRemoveFromArrayRef("--update-gtk", \@buildParams)) {
        system("perl", "$sourceDir/Tools/Scripts/update-webkitgtk-libs") == 0 or die $!;
    }

[Carlos Garcia Campos]

(In reply to comment #15)
> Like I said on IRC, I would prefer to have this on the existing jhbuild.modules. I understand this would be incorrect from a conceptual point of view, since make is not a dependency, but I think practicality beats purity (Python Zen™) in this case. Adding another step will certainly add more bugs we'll need to fix.

fair enough

[Gustavo Noronha (kov)]

Created attachment 133302 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 133302 [details]
Patch

Clearing flags on attachment: 133302

Committed r111749: <http://trac.webkit.org/changeset/111749>

[Gustavo Noronha (kov)]

All reviewed patches have been landed.  Closing bug.

[Zan Dobersek]

*** Bug 111156 has been marked as a duplicate of this bug. ***