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

Ensure Windows paths provided to CMake contain forward slashes only #807

Merged
merged 2 commits into from
Nov 22, 2021

Conversation

jheaff1
Copy link
Collaborator

@jheaff1 jheaff1 commented Oct 29, 2021

CMake will fail if the cross tool file generated by rules_foreign_cc contains backslashes.

@jheaff1 jheaff1 force-pushed the forward_slashes_in_windows_paths branch 3 times, most recently from c7337fc to 7db5217 Compare October 29, 2021 23:37
@jheaff1 jheaff1 changed the title Ensure Windows paths in MSYS2 contain forward slashes only Ensure Windows paths provided to CMake contain forward slashes only Oct 29, 2021
@jheaff1 jheaff1 marked this pull request as ready for review October 29, 2021 23:42
@jheaff1
Copy link
Collaborator Author

jheaff1 commented Nov 6, 2021

@jsharpe @UebelAndre May I please request a review of this PR 😁

@UebelAndre
Copy link
Collaborator

UebelAndre commented Nov 11, 2021

Sorry for the delay. Can you maybe share the callstack of some issue this is solving? So I understand the issue?

@jheaff1
Copy link
Collaborator Author

jheaff1 commented Nov 18, 2021

@UebelAndre Apologies for the delay in getting back to you.

I am trying to build zlib using a hermetic Android toolchain. The crosstool_bazel.cmake file that rules_foreign_cc generates contains the following:

set(CMAKE_CXX_FLAGS_INIT "-isystem C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/c++/v1/ -isystem C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/ -isystem C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/sysroot/usr/include/arm-linux-androideabi/ -target armv7a-linux-androideabi21 -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -fpic -no-canonical-prefixes --sysroot=C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/sysroot -std=c++17")
set(CMAKE_C_COMPILER "C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/bin/clang.exe")

Notice how the paths start with backward slashes, i.e. `C:\msys64\home........".

zlib fails to build because cmake reports a syntax error in crosstool_bazel.cmake:

  Syntax error in cmake code at

    C:/msys64/home/jheaf/_bazel_jheaf/mnvfelgc/execroot/bazel-example/bazel-out/x64_windows-fastbuild/bin/external/zlib/zlib.build_tmpdir/CMakeFiles/3.21.2/CMakeCCompiler.cmake:1

  when parsing string

    C:\msys64\home\jheaf\_bazel_jheaf\mnvfelgc\execroot\bazel-example/external/customandroidsdk/ndk/toolchains/llvm/prebuilt/windows-x86_64/bin/clang.exe

  Invalid escape sequence \m

This PR changes rules_foreign_cc so that all backslashes in paths are replaced with forward slashes, which resolves the issue.

Hope that made sense

CMake will fail if the toolchain fail generated by rules_foreign_cc
contains backslashes.
@jheaff1 jheaff1 force-pushed the forward_slashes_in_windows_paths branch from 7db5217 to 169167b Compare November 18, 2021 19:35
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this looks fine to me but would like @jsharpe to also take a look

Copy link
Member

@jsharpe jsharpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jsharpe jsharpe enabled auto-merge (squash) November 22, 2021 19:23
@jsharpe jsharpe merged commit 7205619 into bazel-contrib:main Nov 22, 2021
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.

3 participants