Add an MSVC builder and rename the old one as GNU, but do not gate. #491

larsbergstrom · 2016-09-27T20:19:28Z

This does a few things:

Adds a servo-windows2 buildbot
Adds a -msvc version of our build rules
Renames the default windows build to -gnu

This change is

aneeshusa · 2016-09-27T21:26:51Z

buildbot/master/files/config/master.cfg

-    DynamicServoBuilder("windows-dev", WINDOWS_SLAVES, envs.build_windows),
-    DynamicServoBuilder("windows-nightly", WINDOWS_SLAVES, envs.build_windows),
+    DynamicServoBuilder("windows-dev-gnu", WINDOWS_SLAVES, envs.build_windows_gnu),
+    DynamicServoBuilder("windows-dev-msvc", WINDOWS_SLAVES, envs.build_windows_msvc),


Our factories have extra logic for "windows-*" builders, and I don't think it would all apply to MSVC. You'll likely need to either change that logic or use a raw util.BuilderConfig with a different factory (similar to the doc builder).

bors-servo · 2016-09-30T16:46:24Z

☔ The latest upstream changes (presumably #497) made this pull request unmergeable. Please resolve the merge conflicts.

edunham · 2016-10-10T16:50:12Z

Reviewable has not handled the rebasing and force pushing here gracefully at all. I've inspected the GitHub diffs, since they show latest commit vs master, and it looks like you've successfully addressed Aneesh's concerns about the windows.* logic in the factories. Assuming the path wizardry is correct, this looks good to me.

Reviewed 2 of 4 files at r2, 1 of 3 files at r3, 1 of 3 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

buildbot/master/files/config/environments.py, line 74 at r2 (raw file):

        r'C:\Program Files\Amazon\cfn-bootstrap',
        r'C:\Program Files\Git\cmd',
        r'C:\Program Files (x86)\WiX Toolset v3.10\bin',

I'll take your word for it that these paths are correct; I haven't dug around in the Windows builders enough to recognize if they were wrong

Comments from Reviewable

aneeshusa · 2016-10-10T16:54:46Z

In some places you've updated one implementation but not the other, please make sure they both match so we don't have strange regressions after #504.

bors-servo · 2016-10-19T16:59:41Z

☔ The latest upstream changes (presumably #504) made this pull request unmergeable. Please resolve the merge conflicts.

aneeshusa · 2016-10-19T17:03:20Z

Didn't see you had updated this, will review after my exam.

larsbergstrom · 2016-10-19T17:06:07Z

No worries! I just did it and need to redo it (splitting it into servo/servo, too), as I just bitrotted it by landing your other fix, which I'd unfortunately not reviewed while I was out sick for most of the last week.

Possibly I can pre-land the "extra" builders into servo/servo first and if that doesn't break anything then I can land the salt/buildbot-side changes second without breaking things. And then a follow-up in servo/servo to remove the now-deprecated builders?

aneeshusa · 2016-10-19T17:21:07Z

I'd like to get #504 deployed and working before we change the builder/steps list again, in case we need to do a rollback/revert/quickfix. But yes, that 3-step process sounds good.

aneeshusa · 2016-10-19T17:25:54Z

Filed servo/servo#13838 to make it easier to change buildbot_steps.yml in the future without causing breakage.

bors-servo · 2016-10-20T11:03:53Z

☔ The latest upstream changes (presumably #516) made this pull request unmergeable. Please resolve the merge conflicts.

larsbergstrom · 2016-12-20T20:49:24Z

r? @edunham @aneeshusa

I think this'll do it for adding the new normal and nightly builders. Note that it can't be deployed until I land the corresponding servo-side changes to the steps.yml file (which I'm currently testing and should have up shortly).

aneeshusa · 2016-12-20T21:04:09Z

buildbot/master/files/config/factories.py

@@ -90,6 +90,8 @@ def setDefaultWorkdir(self, workdir):
    @defer.inlineCallbacks
    def run(self):
        self.is_windows = re.match('windows.*', self.builder_name) is not None
+        self.is_win_gnu = re.match('windows.*gnu', builder_name) is not None
+        self.is_win_msvc = re.match('windows.*msvc', builder_name) is not None


self.is_win_msvc isn't used anywhere, so no need to add it.

larsbergstrom · 2016-12-22T16:52:24Z

@bors-servo r=aneeshusa

bors-servo · 2016-12-22T16:52:24Z

📌 Commit aeed165 has been approved by aneeshusa

bors-servo · 2016-12-22T16:52:27Z

⌛ Testing commit aeed165 with merge 20f2fae...

@aneeshusa

Add an MSVC builder and rename the old one as GNU, but do not gate. This does a few things: 1) Adds a `servo-windows2` buildbot 2) Adds a `-msvc` version of our build rules 3) Renames the default windows build to `-gnu` r? @aneeshusa or @edunham  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/491)

bors-servo · 2016-12-22T17:29:02Z

💔 Test failed - status-travis

…mu on MSVC yet.

larsbergstrom · 2016-12-22T17:31:22Z

@bors-servo r=aneeshusa

bors-servo · 2016-12-22T17:31:23Z

📌 Commit 925a2c8 has been approved by aneeshusa

bors-servo · 2016-12-22T17:31:27Z

⌛ Testing commit 925a2c8 with merge 61cea80...

@aneeshusa

Add an MSVC builder and rename the old one as GNU, but do not gate. This does a few things: 1) Adds a `servo-windows2` buildbot 2) Adds a `-msvc` version of our build rules 3) Renames the default windows build to `-gnu` r? @aneeshusa or @edunham  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/491)

bors-servo · 2016-12-22T17:59:49Z

☀️ Test successful - status-travis

@larsbergstrom

Use correct Windows GNU builder name for Homu Fixup for #491. r? @larsbergstrom  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/562)

@metajack

…m larsbergstrom:add_msvc); r=aneeshusa r? @metajack @edunham The idea here is that I'm adding `-gnu` and `-msvc` variants in advance of the buildbot changes (in servo/saltfs#491). Once those land and are pushed out to the buildbot master, it'll pick up these changed files and then we can remove the unsuffixed `windows-dev` and `windows-nightly`. Source-Repo: https://github.com/servo/servo Source-Revision: 25f32c4513a9d7c3514c4079eef35b1cc5c6b286

…m larsbergstrom:add_msvc); r=aneeshusa r? metajack edunham The idea here is that I'm adding `-gnu` and `-msvc` variants in advance of the buildbot changes (in servo/saltfs#491). Once those land and are pushed out to the buildbot master, it'll pick up these changed files and then we can remove the unsuffixed `windows-dev` and `windows-nightly`. Source-Repo: https://github.com/servo/servo Source-Revision: 25f32c4513a9d7c3514c4079eef35b1cc5c6b286 UltraBlame original commit: 2472b1347fe70eb7bbefe585032cb3d1be50aa67

aneeshusa reviewed Sep 27, 2016

View reviewed changes

aneeshusa added S-needs-code-changes S-needs-rebase labels Oct 3, 2016

aneeshusa self-assigned this Oct 3, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 8, 2016

larsbergstrom force-pushed the add_windows_msvc branch from d9de5e0 to 6cbd76a Compare October 9, 2016 10:59

aneeshusa added S-needs-code-changes and removed S-awaiting-review S-needs-rebase labels Oct 10, 2016

larsbergstrom force-pushed the add_windows_msvc branch from 5af4ca6 to 552e0e1 Compare October 19, 2016 16:11

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 19, 2016

highfive added the S-needs-rebase label Oct 19, 2016

larsbergstrom mentioned this pull request Nov 15, 2016

Why are Servo builds 31MB MacOS, 84MB Linux, 102MB Windows? servo/servo#14176

Open

larsbergstrom force-pushed the add_windows_msvc branch 2 times, most recently from f5c1e83 to 88cdc6b Compare December 20, 2016 20:32

highfive assigned edunham and unassigned aneeshusa Dec 20, 2016

aneeshusa reviewed Dec 20, 2016

View reviewed changes

highfive added the S-awaiting-review label Dec 22, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Dec 22, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Dec 22, 2016

Add an MSVC builder and rename the old one as GNU, but do not gate ho…

925a2c8

…mu on MSVC yet.

larsbergstrom force-pushed the add_windows_msvc branch from aeed165 to 925a2c8 Compare December 22, 2016 17:31

highfive added S-awaiting-review and removed S-tests-failed labels Dec 22, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Dec 22, 2016

bors-servo merged commit 925a2c8 into servo:master Dec 22, 2016

highfive removed the S-awaiting-merge label Dec 22, 2016

bors-servo mentioned this pull request Dec 22, 2016

Export Windows Python on Windows builders #537

Closed

aneeshusa mentioned this pull request Dec 22, 2016

Use correct Windows GNU builder name for Homu #562

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an MSVC builder and rename the old one as GNU, but do not gate. #491

Add an MSVC builder and rename the old one as GNU, but do not gate. #491

larsbergstrom commented Sep 27, 2016 •

edited

aneeshusa Sep 27, 2016

bors-servo commented Sep 30, 2016

edunham commented Oct 10, 2016

aneeshusa commented Oct 10, 2016

bors-servo commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

larsbergstrom commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

bors-servo commented Oct 20, 2016

larsbergstrom commented Dec 20, 2016

aneeshusa Dec 20, 2016

larsbergstrom Dec 20, 2016

larsbergstrom commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

larsbergstrom commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

Add an MSVC builder and rename the old one as GNU, but do not gate. #491

Add an MSVC builder and rename the old one as GNU, but do not gate. #491

Conversation

larsbergstrom commented Sep 27, 2016 • edited

aneeshusa Sep 27, 2016

Choose a reason for hiding this comment

bors-servo commented Sep 30, 2016

edunham commented Oct 10, 2016

aneeshusa commented Oct 10, 2016

bors-servo commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

larsbergstrom commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

aneeshusa commented Oct 19, 2016

bors-servo commented Oct 20, 2016

larsbergstrom commented Dec 20, 2016

aneeshusa Dec 20, 2016

Choose a reason for hiding this comment

larsbergstrom Dec 20, 2016

Choose a reason for hiding this comment

larsbergstrom commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

larsbergstrom commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

bors-servo commented Dec 22, 2016

larsbergstrom commented Sep 27, 2016 •

edited