-
Notifications
You must be signed in to change notification settings - Fork 27
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: Support building in windows #16
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Thanks for your contribution. Very productive working! I will check it on Windows ASAP. |
Thank you. I've had to change it to work for both compared to the working version the night before. So something could of slipped through due to caching |
Signed-off-by: Jordan Hall <[email protected]>
I've got my linux laptop tomorrow. I'll check it out but i think its no longer macos and windows. Looking at what i had to change, it should work on linux. Hence i've added linux build script |
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.
Could you please write the building guide for windows in README?
In your MR, can we get rid of WSL and use PowerShell with MinGW directly to build? |
Sure i'll add that later today. I assume you are referring to use of rm to delete? Sure i'll change these to native windows command. When i install git I override the command with unix. I dont actually use WSL on my machine |
Signed-off-by: Jordan Hall <[email protected]>
Dear @Jordan-Hall how to fix this, i'm trying your's on windows and getting error The system cannot find the path specified. |
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Signed-off-by: Jordan Hall <[email protected]>
Please accept my apologies. Needed to run pnpm install before the build. I've changed the setup.bat file. I've also changed the make file to do the same thing as it had the same issue (I copied order from that as i forgot exactly the order i got it to build in) |
Signed-off-by: Jordan Hall <[email protected]>
First .sh files wont run on windows so made them in JS so it's cross platform. Next gclient wouldn't run directly. Could used setx to add to path but thats then two workflows to run. Created a run gn to run the gclient from the path. Only way to work in windows. |
"generate-protocol-resources": "python ./scripts/deps/generate_protocol_resources.py && git cl format --js", | ||
"install-deps": "python ./scripts/deps/manage_node_deps.py", |
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.
There's a shebang in the top of these python file.
#!/usr/bin/env vpython
Therefore, we should not specify python commands explicitly.
https://chromium.googlesource.com/chromium/tools/depot_tools/master/vpython3
Maybe we should make sure that we can use vpython.
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.
On windows using pnpm to run python script you cant just use the .py file. It requires to know what to use. I could do same as gclient and wrap it in JS to run. Alternative have a windows command. I cant see what actually calls these though on a search of the repo
I've not sorted the actual build release because im unsure about the CI pipeline. But you should be able to check this out and run on windows.
#5 tested on windows - should be close for linux too