Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial Windows support #320

Merged
merged 1 commit into from Apr 16, 2016
Merged

Initial Windows support #320

merged 1 commit into from Apr 16, 2016

Conversation

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Apr 16, 2016

r? @edunham @aneeshusa

I don't expect it to succeed yet (windows was broken by a recent servo commit), so homu is not gated on it. But, like arm64, I'd like to get it building regularly so that I can see the output.


This change is Reviewable

@@ -231,6 +232,16 @@ arm64_factory = create_servo_factory([
steps.Compile(command=["./mach", "build", "--rel", "--target=aarch64-unknown-linux-gnu"], env=arm64_compile_env),
])

windows_compile_env = dict({'PATH': 'C:\\msys64\\mingw64\\bin;C:\\msys64\\usr\\bin\\C:\\Windows\\system32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program Files\\Amazon\\cfn-bootstrap\\',

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Apr 16, 2016

Member

Let's use a raw string to make this nicer.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

Just curious: did you make any progress with #302?

@@ -232,7 +232,7 @@ arm64_factory = create_servo_factory([
steps.Compile(command=["./mach", "build", "--rel", "--target=aarch64-unknown-linux-gnu"], env=arm64_compile_env),
])

windows_compile_env = dict({'PATH': 'C:\\msys64\\mingw64\\bin;C:\\msys64\\usr\\bin\\C:\\Windows\\system32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program Files\\Amazon\\cfn-bootstrap\\',
windows_compile_env = dict({'PATH': r'C:\msys64\mingw64\bin;C:\msys64\usr\bin\C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Amazon\cfn-bootstrap',

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Apr 16, 2016

Author Contributor

@aneeshusa like this?

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Apr 16, 2016

Member

Yes, much easier to work with. (For some reason GitHub highlights these weird, though.)

Did you remove the slash at the end on purpose?

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

And re: #302, it's going to be WAY harder than expected. buildbot requires an incredibly manual setup process (involving a bunch of manual registry key edits, a few aborted service start/restarts to create initial keys with proper permissions, etc.), so I just muscled my way through the setup manually.

When we go to stand up a second machine and we're not in urgent-Windows-mode, I'd like to look at it again, because nobody should have to go through that manually.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

I figured as much, that sounds painful. Which version of Windows is this machine running? I think that would be valuable to document (on the wiki).

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

Will do! I went with Windows Server 2012. I like that it's got a longer support cycle, but the lack of a start button is incredibly disturbing :-)

windows_compile_env = dict({'PATH': r'C:\msys64\mingw64\bin;C:\msys64\usr\bin\C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Amazon\cfn-bootstrap',
'MSYSTEM': 'MINGW64',
'MSYS': 'winsymlinks=lnk'},
**common_test_env)

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Apr 16, 2016

Member

What about the CARGO_HOME and SERVO_CACHE_DIR environment variables we set for Linux and OS X?

(I think there's a port of ccache to windows, but we can worry about that later.)

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Apr 16, 2016

Author Contributor

I'm skipping them because we only have one build type, at least for now, and I'm trying to get something that works similarly to the current AppVeyor config (https://github.com/servo/servo/blob/master/appveyor.yml).

I really wish that we could use ccache, but it doesn't work on Windows due to known build problems with the components we pick up from Firefox. I already spent a bunch of time trying to get it working (and failing) on AppVeyor and locally.

This comment has been minimized.

Copy link
@aneeshusa

aneeshusa Apr 16, 2016

Member

Do we need something similar to servo/servo#10528?

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Apr 16, 2016

Author Contributor

Luckily, I have 100GB of disk on this machine :-) AppVeyor's cache is only 500MB.

That said, we have no cache reduction on any of the machines, so they will all slowly accrete more cargo packages & rust/cargo binaries over time until they fill all available disk. Opening #321

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

This LGTM after a squash.

Side note: As far as I can tell we haven't actually deployed any of the last 3-4 PRs to our buildbot machines. I'm sure you and @edunham both have enough on your plates already; the Homu queue only has one build right now, so can you show me exactly how to restart Buildbot cleanly to deploy this when it finishes?

@larsbergstrom larsbergstrom force-pushed the larsbergstrom:windows branch from 358de78 to 9916006 Apr 16, 2016
@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

@bors-servo r=aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2016

📌 Commit 9916006 has been approved by aneeshusa

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2016

Testing commit 9916006 with merge d268ef1...

bors-servo added a commit that referenced this pull request Apr 16, 2016
Initial Windows support

r? @edunham @aneeshusa

I don't expect it to succeed yet (windows was broken by a recent servo commit), so homu is not gated on it. But, like arm64, I'd like to get it building regularly so that I can see the output.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/320)
<!-- Reviewable:end -->
@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

@aneeshusa I typically just wait for the queue to empty, log in, and then highstate. I've found that the "careful" buildbot restarts that we described have a downside that we end up with a buildbot master process that is running as an orphaned child process instead of as a child of the service framework, so I don't use it very often.

I think that if we change the salt rules to push out the files but only send a SIGHUP to a running buildbot process, that should have the best effect, and could be integrated with normal salt rules.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

OK, can I try deploying these changes? I'm logged into a tmux instance on servo-master1 if you want to watch.

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

@aneeshusa Sure! Please go right ahead, and thanks for all of the late-Friday-night reviews & chatter :-)

@bors-servo
Copy link
Contributor

bors-servo commented Apr 16, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 9916006 into servo:master Apr 16, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

Just deployed. BTW, I turned off the automatic buildbot-master restart in #307, so I also restarted it manually with service buildbot-master restart.

I'm running tail +F twistd.log in tmux and I'm seeing a lot of this:

invalid login from unknown user 'servo-windows1'

As well as the same message for servo-macpro1. I think we're missing some authentication?

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

I know that macpro1 should not be working, but servo-windows1 should be working because of this:
https://github.com/servo/saltfs/blob/master/buildbot/master/files/config/master.cfg#L27-L28

I'm not sure what's going wrong here...

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

Hrm, I re-ran the steps here:
https://github.com/servo/servo/wiki/Buildbot-administration#dealing-with-troubles

And now it's seeing servo-windows1. However, the buildbot process seems to be running as root instead of as the servo user.

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

Which steps in particular did you run? It's weird that a single restart didn't work properly.

@larsbergstrom
Copy link
Contributor Author

larsbergstrom commented Apr 16, 2016

I attempted to run:

# su - servo
# buildbot restart --clean --nodaemon /home/servo/buildbot/master &

But it told me that the .pid file was not owned by the servo user. So I just ran it as root and the service turned over nicely :-)

@aneeshusa
Copy link
Member

aneeshusa commented Apr 16, 2016

Also, I think something may be wrong with our buildbot-master.conf file. service buildbot-master status says that it's not running, and checking in syslog says it was terminated with status 1, so maybe it's being invoked incorrectly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.