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

Create `mach bootstrap` based on Mozilla's mozboot bootstrapper #12916

Merged
merged 2 commits into from Sep 8, 2016

Conversation

@UK992
Copy link
Contributor

UK992 commented Aug 17, 2016

Fixes #12914
I've made this few weeks ago, its an example how could everything looks like.
It downloads and setup all needed dependencies for MSVC.
It's has version in case if some dependencies need to be updated.
Zip files and folder in zip need to be named <dep>-<version>.
Also if cmake already exist in PATH, it won't download it again.

I want opinion on that, if this is right approaches and how to improve it.

cc @vvuk


This change is Reviewable

@highfive
Copy link

highfive commented Aug 17, 2016

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py, python/servo/bootstrap_commands.py
@saschanaz
Copy link
Contributor

saschanaz commented Aug 18, 2016

Can the command window be kept when error occurred? Users won't see the error messages unless they manually run cmd.

@UK992
Copy link
Contributor Author

UK992 commented Aug 18, 2016

@saschanaz I don't understand, can you please show which part of code did you mean?

@saschanaz
Copy link
Contributor

saschanaz commented Aug 18, 2016

@UK992 I was talking about mach.bat ECHO lines but never mind, the error message was thrown from mach rather than mach.bat. Is there a way to keep cmd window when mach throws any error?

(The error message was about the nonexistence of python virtualenv)

@UK992 UK992 force-pushed the UK992:msvc-dependencies branch from 9021bcd to d5cd00d Aug 18, 2016
@UK992 UK992 force-pushed the UK992:msvc-dependencies branch 2 times, most recently from ba60869 to 0551280 Aug 18, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2016

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

@UK992 UK992 force-pushed the UK992:msvc-dependencies branch from 0551280 to 6d36610 Aug 22, 2016
@jdm jdm removed the S-needs-rebase label Aug 23, 2016
# Don't download CMake if already exists in PATH
if dep_name == "cmake":
if spawn.find_executable(dep_name):
continue

This comment has been minimized.

Copy link
@wafflespeanut

wafflespeanut Aug 23, 2016

Member

This could be moved out of the loop?

This comment has been minimized.

Copy link
@UK992

UK992 Sep 1, 2016

Author Contributor

Hardly, considering that it compares with the names in the list.

@UK992 UK992 force-pushed the UK992:msvc-dependencies branch from 37dc444 to 2814f61 Aug 31, 2016
@UK992
Copy link
Contributor Author

UK992 commented Aug 31, 2016

In last commit i moved to mach bootstrap command based on Mozilla's mozboot, still needs some cleanups, but its for example how could servo bootstrapper looks like. I also added Windows GNU platform.
And can be easily extended to other platforms, since most work it's already done.

@UK992 UK992 force-pushed the UK992:msvc-dependencies branch 5 times, most recently from bbbbbe9 to 0bcb7ac Sep 1, 2016
% __name__)

def which(self, name):
"""Python implementation of which.

This comment has been minimized.

Copy link
@wafflespeanut

wafflespeanut Sep 2, 2016

Member

Can't we just use distutils.spawn.find_executable? (like we do in mach_bootstrap.py)

This comment has been minimized.

Copy link
@UK992

UK992 Sep 2, 2016

Author Contributor

done

@wafflespeanut
Copy link
Member

wafflespeanut commented Sep 2, 2016

I don't have much ideas/suggestions regarding MSVC bootstrapping (apart from python'ish style nits, which I'll address in the end). Pinging @larsbergstrom @vvuk and @metajack to look into this sometime...

$env:FFMPEG_LIBS="avformat:avcodec:avutil" ;
}'
- if %BUILD_ENV%==msvc call "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64\vcvars64.bat"
# - ps: 'if ($env:BUILD_ENV -eq "msvc") {

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Sep 2, 2016

Contributor

Can you change this to instead do ./mach bootstrap-msvc and see if that works on AppVeyor?

This comment has been minimized.

Copy link
@UK992

UK992 Sep 2, 2016

Author Contributor

bootstrap-msvc is replaced by bootstrap in newer commit, which also support gnu builds, but for msvc build, command is being run in ensure_bootstrapped. But i did that for gnu builds.
EDIT: doesn't work for gnu

return print("Unsupported platform.")

msvc_deps_dir = path.join(self.context.sharedir, "msvc-dependencies")
msvc_deps_url = "https://dl.dropboxusercontent.com/u/25971865/msvc-deps/"

This comment has been minimized.

Copy link
@larsbergstrom

larsbergstrom Sep 2, 2016

Contributor

We will definitely want to move these to one of the Servo S3 buckets. I like that you've versioned the files, which is something we've definitely found a need for in the past.

@larsbergstrom larsbergstrom self-assigned this Sep 2, 2016
@UK992 UK992 force-pushed the UK992:msvc-dependencies branch 3 times, most recently from 90dac0b to ecaa97c Sep 2, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Sep 7, 2016

✌️ @UK992 can now approve this pull request

@UK992 UK992 force-pushed the UK992:msvc-dependencies branch from 1bd1fc0 to 922937c Sep 7, 2016
@UK992
Copy link
Contributor Author

UK992 commented Sep 7, 2016

@larsbergstrom done!
@wafflespeanut Can you please take a look again for any python'ish nits?

@UK992 UK992 changed the title WIP: MSVC dependencies auto-download Create `mach bootstrap` based on Mozilla's mozboot bootstrapper Sep 7, 2016
@UK992 UK992 force-pushed the UK992:msvc-dependencies branch from 922937c to eff91be Sep 7, 2016
@UK992
Copy link
Contributor Author

UK992 commented Sep 7, 2016

I've made some renamings:
check_installed_packages -> ensure_system_packages
install_packages -> install_system_packages

@wafflespeanut
Copy link
Member

wafflespeanut commented Sep 8, 2016

I just had a brief overview on this, and I'm cool with most of the stuff. Let's land this thing happily and if there's something dirty, we'll cleanup later 😄

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Sep 8, 2016

@bors-servo r=larsbergstrom,wafflespeanut

@bors-servo
Copy link
Contributor

bors-servo commented Sep 8, 2016

📌 Commit eff91be has been approved by larsbergstrom,wafflespeanut

@bors-servo
Copy link
Contributor

bors-servo commented Sep 8, 2016

Testing commit eff91be with merge 16db0ed...

bors-servo added a commit that referenced this pull request Sep 8, 2016
…espeanut

Create `mach bootstrap` based on Mozilla's mozboot bootstrapper

Fixes #12914
I've made this few weeks ago, its an example how could everything looks like.
It downloads and setup all needed dependencies for MSVC.
It's has version in case if some dependencies need to be updated.
Zip files and folder in zip need to be named ``<dep>-<version>``.
Also if cmake already exist in PATH, it won't download it again.

I want opinion on that, if this is right approaches and how to improve it.

cc @vvuk

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

bors-servo commented Sep 8, 2016

💔 Test failed - mac-rel-wpt

@highfive
Copy link

highfive commented Sep 8, 2016

  ▶ ERROR [expected OK] /webgl/conformance-1.0.3/conformance/ogles/GL/build/build_001_to_008.html
  └   → gl.getProgramInfoLog is not a function
@wafflespeanut
Copy link
Member

wafflespeanut commented Sep 8, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Sep 8, 2016

Testing commit eff91be with merge daa30e6...

bors-servo added a commit that referenced this pull request Sep 8, 2016
…espeanut

Create `mach bootstrap` based on Mozilla's mozboot bootstrapper

Fixes #12914
I've made this few weeks ago, its an example how could everything looks like.
It downloads and setup all needed dependencies for MSVC.
It's has version in case if some dependencies need to be updated.
Zip files and folder in zip need to be named ``<dep>-<version>``.
Also if cmake already exist in PATH, it won't download it again.

I want opinion on that, if this is right approaches and how to improve it.

cc @vvuk

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

bors-servo commented Sep 8, 2016

@bors-servo bors-servo merged commit eff91be into servo:master Sep 8, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@UK992 UK992 deleted the UK992:msvc-dependencies branch Jan 26, 2017
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

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