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

build improvements for packagers of deno #2423

Merged
merged 2 commits into from May 30, 2019

Conversation

4 participants
@chrmoritz
Copy link
Contributor

commented May 29, 2019

This PR adds some improvements to the build tooling of deno which could be useful for packagers of deno. These ideas came up during the creating of the homebrew formula in Homebrew/homebrew-core#35645. With is being merged the homebrew formula could make use of your build scripts (tools/setup.py and tools/build.py) and be even more simple as shown in this gist: https://gist.github.com/chrmoritz/3dfc9fa7acc720cf5e1289e51662f69c#file-deno-rb-L40-L46

Refs: #1474

The first commit 4205d06 to ./tools/third_party.py simply adds the option to override the default gn_path and ninja_path by setting the newly added DENO_GN_PATH and DENO_NINJA_PATH environment variables. (This is useful when not using the prebuilt binaries of both tools and instead pointing the build system to the system provided ones.)

The second commit d7bb9bb fixes ./tools/setup.py's --no-binary-download option to also disable the download of the sccache binary. To make it work I had to refactor the ./tools/prebuilt.py a bit to separate the getting the platform specific path to the binary from actually downloading said binary.

@CLAassistant

This comment has been minimized.

Copy link

commented May 29, 2019

CLA assistant check
All committers have signed the CLA.

@ry

This comment has been minimized.

Copy link
Collaborator

commented May 30, 2019

Looks good! Thanks!

Would you mind adding some documentation for DENO_GN_PATH and DENO_NINJA_PATH to the manual? I think this section is appropriate:
https://github.com/denoland/deno/blob/master/website/manual.md#other-useful-commands

@chrmoritz chrmoritz force-pushed the chrmoritz:buildOptions branch from d7bb9bb to c45ac2c May 30, 2019

@chrmoritz

This comment has been minimized.

Copy link
Contributor Author

commented May 30, 2019

Would you mind adding some documentation for DENO_GN_PATH and DENO_NINJA_PATH to the manual? I think this section is appropriate:
https://github.com/denoland/deno/blob/master/website/manual.md#other-useful-commands

Sure. I though about adding some documentation there yesterday, but I think I've forgotten it because it was already late and I wanted to submit this PR.

@ry

ry approved these changes May 30, 2019

Copy link
Collaborator

left a comment

LGTM

@ry ry merged commit 0e780e8 into denoland:master May 30, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
@hayd

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

This is now used in homebrew! 😎 Homebrew/homebrew-core#40811
Thanks again @chrmoritz.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.