-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: windows support #3107
feat: windows support #3107
Conversation
a51c0d7
to
8b6c291
Compare
8b6c291
to
ed0ee5b
Compare
a0cf87a
to
ed0ee5b
Compare
You can find the image built from this PR at
Built from 5ad9045 |
5bd7f38
to
553c728
Compare
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.
LGTM! Thanks for it! 💯
I just added some minor comments that I hope you find useful :)
submodule_version=$(cargo metadata --format-version=1 --no-deps --manifest-path "${build_dir}/rln/Cargo.toml" | sed -n 's/.*"name":"rln","version":"\([^"]*\)".*/\1/p') | ||
else | ||
submodule_version=$(cargo metadata --format-version=1 --no-deps --manifest-path "${build_dir}/rln/Cargo.toml" | jq -r '.packages[] | select(.name == "rln") | .version') |
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.
Are the sed
and jq
available by default on Windows?
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 msys built in contains bash and all the main *nix utilities... at least how I remember...
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.
Yes, jq, sed and all other linux utilities are come long with msys ( mingw )
scripts/windows_setup.sh
Outdated
build_component "vendor/nim-libbacktrace" "make install/usr/lib/libunwind.a" "libunwind" | ||
|
||
echo "7. Building wakunode2" | ||
execute_command "make wakunode2 V=1" |
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'd add some more NIMFLAGS to prevent colors in chronicles
execute_command "make wakunode2 V=1" | |
execute_command 'make wakunode2 V=1 NIMFLAGS="-d:disableMarchNative -d:postgres -d:chronicles_colors:none" ' |
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.
Thanks so much! Looks amazing! 🔥
Added a couple comments to further understand :)
Makefile
Outdated
detected_OS := Darwin | ||
else ifeq ($(detected_OS),Linux) | ||
detected_OS := Linux | ||
else ifneq (,$(findstring MINGW,$(detected_OS))) |
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.
Can't the OS be Windows even if the OS doesn't contain MINGW on its name? I think in other places we use ifeq ($(OS),Windows_NT)
for example
It would probably make sense to also search for the string Windows
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 windows systems, the OS variable is usually empty by default, so I use the uname
command for information. Since MINGW is very common in windows, use that idea to differentiate windows and non-windows. but yeah i got your point, i will try make more simplified.
Related to waku-org/pm#239 |
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.
Great work!
Would you mind add a section into Readme.md describing Windows build?
submodule_version=$(cargo metadata --format-version=1 --no-deps --manifest-path "${build_dir}/rln/Cargo.toml" | sed -n 's/.*"name":"rln","version":"\([^"]*\)".*/\1/p') | ||
else | ||
submodule_version=$(cargo metadata --format-version=1 --no-deps --manifest-path "${build_dir}/rln/Cargo.toml" | jq -r '.packages[] | select(.name == "rln") | .version') |
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 msys built in contains bash and all the main *nix utilities... at least how I remember...
Sure, Thanks for suggestion |
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.
LGTM
6f24838
to
8e86fac
Compare
8e86fac
to
3dc3fc8
Compare
Windows support for nwaku - Initial pull request
Overview
This pull request (PR) introduces official Windows support for nwaku. Our ultimate goal is to achieve parity with Linux and macOS, allowing Windows users to build and run nwaku using familiar
make
targets, with minimal platform-specific steps.Current status and objectives
Setup procedure
The current setup procedure is as follows:
windows_support
branchscripts/windows_setup.sh
make wakunode2
(additional targets coming soon)Note: This procedure will be updated as we progress towards our goal of making the windows build process similar to linux and mac. Eventually, users will be able to use standard
make
targets without additional setup steps.Achievements