-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Fix building on Windows #275
Conversation
- cmd doesn't support semicolons, use && instead. - Run `python` explicitly. - Use quote escaping that works in both bash and cmd. - Set `DEPOT_TOOLS_WIN_TOOLCHAIN=0` by default (to avoid non-googlers being asked for credentials). - Pass `shell=True` to run .bat files. See also: v8/node#60
@seishun Does ccache work for you at all? Where did you get it from? |
Nope. As far as I can tell, |
@seishun appreciate the effort but we need to get these tools building in linux first. I'm going to close this. We will be compatible with windows - just everything is in flux right now. |
|
||
gn gen out/Debug --args='cc_wrapper="ccache" is_debug=true ' | ||
gn gen out/Debug --args="cc_wrapper=\"ccache\" is_debug=true " |
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.
Works, but unnecessarily complex. Just remove the quotes around ccache completely.
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.
That doesn't work here:
~/deno/deno2$ gn gen out/Debug --args="cc_wrapper=ccache is_debug=true "
ERROR at the command-line "--args":1:12: Undefined identifier
cc_wrapper=ccache is_debug=true
^-----
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.
Looking good overall, but see comments.
@@ -23,6 +23,9 @@ def main(): | |||
os.chdir(root_path) | |||
buildName = "Debug" if args.debug else "Default" | |||
buildDir = os.path.join(root_path, "out", buildName) | |||
# Default to non-Googler configuration. | |||
if 'DEPOT_TOOLS_WIN_TOOLCHAIN' not in os.environ: |
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.
In addition to doing this, also print out a me.ssage. Otherwise people are going to scratch their head too much when running gn directly and it doesn't work.
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.
Do you think the note I added in README.md is insufficient?
@ry I don't think it's unreasonable to land this. I've been meaning to write this down, but didn't. |
I would rather remove |
Yeah, let's remove build.py |
#276 doesn't include the README.md changes though. |
python
explicitly.DEPOT_TOOLS_WIN_TOOLCHAIN=0
by default (to avoid non-googlersbeing asked for credentials).
shell=True
to run .bat files.See also: v8/node#60