Skip to content

Improve hydro_deploy build error reporting, fix stageleft windows paths #1010

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

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

MingweiSamuel
Copy link
Member

No description provided.

@MingweiSamuel MingweiSamuel changed the title ci: add window ci testing Updates for Windows Jan 3, 2024
@MingweiSamuel MingweiSamuel marked this pull request as draft January 3, 2024 20:17
@MingweiSamuel MingweiSamuel marked this pull request as ready for review January 3, 2024 20:33
@MingweiSamuel MingweiSamuel requested a review from shadaj January 3, 2024 20:33
@shadaj
Copy link
Member

shadaj commented Jan 3, 2024

These test failures are strange, it seems that the cfg(target_os = ...) is behaving differently when cargo is invoked by Hydro Deploy. Hydro Deploy shouldn't be specifying any target though, we leave it as the default for a Localhost host, so I'm a bit confused why this is happening.

@MingweiSamuel
Copy link
Member Author

MingweiSamuel commented Jan 3, 2024

@shadaj I also can't repro (anymore) on my local windows, lol. Removing the CI for now and testing more

@shadaj
Copy link
Member

shadaj commented Jan 3, 2024

I wonder if we need to be using which to grab the right cargo from the PATH, I vaguely remember needing to do something similar when invoking processes in another project.

@MingweiSamuel
Copy link
Member Author

Actually can reproduce again locally. Build seems to be broken in typenum paholg/typenum#195

Will merge this and continue

@MingweiSamuel MingweiSamuel merged commit 8df66f8 into hydro-project:main Jan 3, 2024
@MingweiSamuel MingweiSamuel changed the title Updates for Windows Improve hydro_deploy build error reporting, stageleft windows paths Jan 3, 2024
@MingweiSamuel MingweiSamuel changed the title Improve hydro_deploy build error reporting, stageleft windows paths Improve hydro_deploy build error reporting, fix stageleft windows paths Jan 3, 2024
@MingweiSamuel MingweiSamuel deleted the windows-ci branch January 3, 2024 23:39
@MingweiSamuel
Copy link
Member Author

Seems like OUT_DIR is getting mutilated:
D:\Projects\hydroflow\target\debug\build\typenum-27ab1f3e5e6288a0\out normally
vs
\\?\D:\Projects\hydroflow\target\release\build\typenum-123f6570ec25fa24\out when the error occurs

@MingweiSamuel
Copy link
Member Author

paholg/typenum#195 (comment)

include!(r"D:\Projects\typenum_test/include.rs"); // works
include!(r"\\?\D:\Projects\typenum_test\include.rs"); // works
include!(r"\\?\D:\Projects\typenum_test/include.rs"); // ERROR

@shadaj
Copy link
Member

shadaj commented Jan 3, 2024

Yep, https://stackoverflow.com/questions/50322817/how-do-i-remove-the-prefix-from-a-canonical-windows-path. We canonicalize the src path before using it as the current dir for the command.

nickjiang2378 pushed a commit to nickjiang2378/hydroflow that referenced this pull request Jan 25, 2024
nickjiang2378 pushed a commit to nickjiang2378/hydroflow that referenced this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants