Bug 74519 – [Qt] Automatic clean build feature always do clean build with --no-webkit2

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

I have no idea why, but every build is clean build with --no-webkit2 option:
Feature PLUGIN_ARCHITECTURE_UNSUPPORTED was removed, clean build needed!

[Csaba Osztrogonác]

Additionally with --no-webkit2 WEBKIT_TESTFONTS feauture
landed in http://trac.webkit.org/changeset/102776 doesn't work,
but it work's with Qt5-WK1 if we don't use --no-webkit2.

[Csaba Osztrogonác]

I removed --no-webkit2 from our Qt5-WK1 bot config temporarily. I'll check this bug tomorrow.

[Tor Arne Vestbø]

Cool, let's look at it tomorrow.

[Csaba Osztrogonác]

Maybe it is related to http://trac.webkit.org/changeset/99872
I hope http://trac.webkit.org/changeset/103058 fixed it.

But we I'd like to leave this bug open for several days to make sure if it is fixed.

[Csaba Osztrogonác]

Now the problem is worse. run-javascriptcore-tests call build-jsc which always triggers a clean build, but build-jsc builds only jsc and it broke layout and API tests.

Running: build-jsc --qt --release --32-bit --qt
Feature PLUGIN_ARCHITECTURE_UNSUPPORTED added, clean build needed!

I disabled --no-webkit on the Qt5-WK1 bot again.

[Csaba Osztrogonác]

This bug is still valid, you can reproduce it easily:
- Tools/Scripts/build-webkit --no-webkit2
- Tools/Scripts/run-javascriptcore-tests

The root of the problem is that build-webkit pass the BUILD_WEBKIT_ARGS environment: --no-webkit2 to qmake as --qmakearg=CONFIG+=no_webkit2,
but build-jsc doesn't. And the problem is that webkit2 build contains PLUGIN_ARCHITECTURE_UNSUPPORTED define, but --no-webkit2 build doesn't.

[Nandor Huszka]

Created attachment 124273 [details]
Modifications in build-webkit, build-jsc, webkitdirs.pm

I ran Tools/Scripts/build-webkit --no-webkit2 and then Tools/Scripts/build-jsc with this patch.
Probably it fixes the problem, but it is not final yet.

[Nandor Huszka]

Created attachment 124274 [details]
Modifications in build-webkit, build-jsc, webkitdirs.pm

[Tor Arne Vestbø]

(In reply to comment #8)
> Created an attachment (id=124274) [details]
> Modifications in build-webkit, build-jsc, webkitdirs.pm

I think this patch is way to intrusive for what it does. 

I see a few options: 

  - Set BUILD_WEBKIT_ARGS on the bots for all processes, not just when running build-webkit. Since build-jsc or run-javascriptcore-tests will potentially build webkit, BUILD_WEBKIT_ARGS should be set for those as well.

  - Make build-webkit do a pure build and not a reconfigure as well when called without any arguments (or from another script), so that if you do build-webkit --no-video, and then call build-webkit, it would just build and not complain about video now being enabled. 

  - Set the PLUGIN_ARCHITECTURE_UNSUPPORTED define to something, even when not building webkit2.

or some combination of these.

[Nandor Huszka]

(In reply to comment #9)
Thank you for the comments, I will try to find a less complicated solution with the help of options you have mentioned.

[Nandor Huszka]

Created attachment 124531 [details]
Modifications in the build-system

I took Tor Arne Vestbø's advice, the patch always set PLUGIN_ARCHITECTURE_UNSUPPORTED define.

[Nandor Huszka]

Created attachment 124533 [details]
Modifications in the build-system 

This patch contains ChangeLog too.

[Tor Arne Vestbø]

Comment on attachment 124533 [details]
Modifications in the build-system 

Adding it to build-webkit makes no sense at all. This should be a features.prf change only. And you put a no_webkit2: PLUGIN_ARCHITECTURE_UNSUPPORTED=1 block at the start and add a comment that the reason we're doing that is because of this bug.

[Nandor Huszka]

Created attachment 124679 [details]
Patch: features.prf

(In reply to comment #13)
Thank you for the help, I wrote a comment too.

[WebKit Review Bot]

Attachment 124679 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nandor Huszka]

Created attachment 124683 [details]
Patch: features.prf 

I do not understand why merging conflict caused in skiplists, the patch did not deal with them. Whatever I have cleaned my repo and redo all the modifications.

[WebKit Review Bot]

Attachment 124683 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   c57450d..c1d3a24  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 106343 = c57450da079d27d2b4fd1ee5daecdf7a12dce827
r106344 = c1d3a24ee41e907f3a05d56935348677c33112a7
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tor Arne Vestbø]

Comment on attachment 124683 [details]
Patch: features.prf 

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

> Tools/qmake/mkspecs/features/features.prf:113
> +# --no-webkit2 build did not contain this define, but it needs to avoid unwanted clean builds by other scripts

"it's needed"

> Tools/qmake/mkspecs/features/features.prf:115
> +no_webkit2:!contains(DEFINES, PLUGIN_ARCHITECTURE_UNSUPPORTED): {

I'd write this (along with the lines below) as

!contains(DEFINES, PLUGIN_ARCHITECTURE_UNSUPPORTED) {
    no_webkit2 {
        # --no-webkit2 build did not contain this define, but it's needed to avoid unwanted clean builds by other scripts
        DEFINES += PLUGIN_ARCHITECTURE_UNSUPPORTED=1
    } else {
        # The old block
    }
}

[Nandor Huszka]

Created attachment 124905 [details]
Patch

Thank you for the advice, under a new bug there will be a more general solution for plugins that missing from no-webkit2 build.

[Nandor Huszka]

(In reply to comment #19)
> under a new bug there will be a more general solution for plugins that missing from no-webkit2 build.

It can be found there:
https://bugs.webkit.org/show_bug.cgi?id=77533

[Tor Arne Vestbø]

Comment on attachment 124905 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 124905 [details]
Patch

Clearing flags on attachment: 124905

Committed r106461: <http://trac.webkit.org/changeset/106461>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.