Bug 74519 - [Qt] Automatic clean build feature always do clean build with --no-webkit2
Summary: [Qt] Automatic clean build feature always do clean build with --no-webkit2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Critical
Assignee: Nandor Huszka
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2011-12-14 10:44 PST by Csaba Osztrogonác
Modified: 2012-02-01 02:55 PST (History)
5 users (show)

See Also:


Attachments
Modifications in build-webkit, build-jsc, webkitdirs.pm (4.53 KB, patch)
2012-01-27 01:22 PST, Nandor Huszka
no flags Details | Formatted Diff | Diff
Modifications in build-webkit, build-jsc, webkitdirs.pm (4.37 KB, patch)
2012-01-27 01:24 PST, Nandor Huszka
no flags Details | Formatted Diff | Diff
Modifications in the build-system (1.56 KB, patch)
2012-01-30 05:28 PST, Nandor Huszka
no flags Details | Formatted Diff | Diff
Modifications in the build-system (2.22 KB, patch)
2012-01-30 05:33 PST, Nandor Huszka
vestbo: review-
vestbo: commit-queue-
Details | Formatted Diff | Diff
Patch: features.prf (1.44 KB, patch)
2012-01-31 01:02 PST, Nandor Huszka
no flags Details | Formatted Diff | Diff
Patch: features.prf (1.43 KB, patch)
2012-01-31 01:23 PST, Nandor Huszka
vestbo: review-
vestbo: commit-queue-
Details | Formatted Diff | Diff
Patch (1.89 KB, patch)
2012-02-01 01:40 PST, Nandor Huszka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2011-12-14 10:44:12 PST
I have no idea why, but every build is clean build with --no-webkit2 option:
Feature PLUGIN_ARCHITECTURE_UNSUPPORTED was removed, clean build needed!
Comment 1 Csaba Osztrogonác 2011-12-14 11:01:58 PST
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.
Comment 2 Csaba Osztrogonác 2011-12-14 11:27:28 PST
I removed --no-webkit2 from our Qt5-WK1 bot config temporarily. I'll check this bug tomorrow.
Comment 3 Tor Arne Vestbø 2011-12-14 12:12:11 PST
Cool, let's look at it tomorrow.
Comment 4 Csaba Osztrogonác 2011-12-16 05:57:12 PST
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.
Comment 5 Csaba Osztrogonác 2011-12-17 03:00:59 PST
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.
Comment 6 Csaba Osztrogonác 2012-01-12 07:52:54 PST
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.
Comment 7 Nandor Huszka 2012-01-27 01:22:02 PST
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.
Comment 8 Nandor Huszka 2012-01-27 01:24:02 PST
Created attachment 124274 [details]
Modifications in build-webkit, build-jsc, webkitdirs.pm
Comment 9 Tor Arne Vestbø 2012-01-27 03:08:01 PST
(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.
Comment 10 Nandor Huszka 2012-01-29 23:30:51 PST
(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.
Comment 11 Nandor Huszka 2012-01-30 05:28:09 PST
Created attachment 124531 [details]
Modifications in the build-system

I took Tor Arne Vestbø's advice, the patch always set PLUGIN_ARCHITECTURE_UNSUPPORTED define.
Comment 12 Nandor Huszka 2012-01-30 05:33:18 PST
Created attachment 124533 [details]
Modifications in the build-system 

This patch contains ChangeLog too.
Comment 13 Tor Arne Vestbø 2012-01-30 05:46:54 PST
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.
Comment 14 Nandor Huszka 2012-01-31 01:02:29 PST
Created attachment 124679 [details]
Patch: features.prf

(In reply to comment #13)
Thank you for the help, I wrote a comment too.
Comment 15 WebKit Review Bot 2012-01-31 01:05:17 PST
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.
Comment 16 Nandor Huszka 2012-01-31 01:23:01 PST
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.
Comment 17 WebKit Review Bot 2012-01-31 01:24:51 PST
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.
Comment 18 Tor Arne Vestbø 2012-01-31 01:42:05 PST
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
    }
}
Comment 19 Nandor Huszka 2012-02-01 01:40:09 PST
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.
Comment 20 Nandor Huszka 2012-02-01 02:00:18 PST
(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
Comment 21 Tor Arne Vestbø 2012-02-01 02:40:54 PST
Comment on attachment 124905 [details]
Patch

Thanks!
Comment 22 WebKit Review Bot 2012-02-01 02:55:41 PST
Comment on attachment 124905 [details]
Patch

Clearing flags on attachment: 124905

Committed r106461: <http://trac.webkit.org/changeset/106461>
Comment 23 WebKit Review Bot 2012-02-01 02:55:47 PST
All reviewed patches have been landed.  Closing bug.