-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Report error when vcvarsall fails #26198
Conversation
Heads up! This PR modifies the following files:
|
python/servo/build_commands.py
Outdated
@@ -315,11 +315,15 @@ def build(self, release=False, dev=False, jobs=None, params=None, media_stack=No | |||
process = subprocess.Popen('("%s" %s > nul) && "python" -c "import os; print(repr(os.environ))"' % | |||
(os.path.join(vs_dirs['vcdir'], "Auxiliary", "Build", "vcvarsall.bat"), "x64"), | |||
stdout=subprocess.PIPE, shell=True) | |||
stdout, _ = process.communicate() | |||
stdout, err = process.communicate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be stderr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, i'd manually copied over the patch
679c28d
to
371ba67
Compare
@bors-servo r+ |
📌 Commit 371ba67 has been approved by |
Report error when vcvarsall fails In trying to get my Windows cross build working on my desktop, I ended up spending a lot of time trying to replicate a failure within servo's build system that ultimately turned out to be vcvarsall silently failing (I was missing a trailing slash in my `VSINSTALLDIR` env var, which vcvarsall does not handle well at all) We should report an error when this happens. r? @jdm
💔 Test failed - status-taskcluster |
Report error when vcvarsall fails In trying to get my Windows cross build working on my desktop, I ended up spending a lot of time trying to replicate a failure within servo's build system that ultimately turned out to be vcvarsall silently failing (I was missing a trailing slash in my `VSINSTALLDIR` env var, which vcvarsall does not handle well at all) We should report an error when this happens. r? @jdm
💔 Test failed - status-taskcluster |
@bors-servo retry |
☀️ Test successful - status-taskcluster |
In trying to get my Windows cross build working on my desktop, I ended up spending a lot of time trying to replicate a failure within servo's build system that ultimately turned out to be vcvarsall silently failing (I was missing a trailing slash in my
VSINSTALLDIR
env var, which vcvarsall does not handle well at all)We should report an error when this happens.
r? @jdm