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

fix: Make paths in the CmakeLists scripts Windows friendly #1734

Merged
merged 4 commits into from Nov 4, 2022

Conversation

mganandraj
Copy link
Contributor

Summary:

The paths in the CMake script generated for autolinking the modules contains backslashes which CMake don't like. This patch fixes the file paths to replace the backslashes with forward slases.

Test Plan:

Tested autolinking modules locally on windows.

? path.join(sourceDir, userConfig.cmakeListsPath)
: path.join(sourceDir, 'build/generated/source/codegen/jni/CMakeLists.txt');

if(process.platform == "win32") {
cmakeListsPath = cmakeListsPath.replaceAll("\\", "/")
Copy link
Member

Choose a reason for hiding this comment

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

can you do the same for androidMkPath as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi .. Sorry, i missed tracking this PR. The fix may not be required form ndk-build as it can handle the windows file paths i believe .. I will check it out and raise another PR if required.

@thymikee thymikee changed the title Making paths in the cmake scripts windows friendly fix: Make paths in the CmakeLists scripts Windows friendly Nov 4, 2022
Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thank you! I'm not sure whether this change is safe for Android.mk as well. Let's put it into a separate PR, ideally by someone who has a Windows machine to test on, ok @cortinico?

@thymikee thymikee merged commit 7be1d73 into react-native-community:main Nov 4, 2022
thymikee added a commit that referenced this pull request Nov 4, 2022
* Fixing the paths to be windows friendly

* Update packages/cli-platform-android/src/config/index.ts

Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>

* Update packages/cli-platform-android/src/config/index.ts

* fix: lint

Co-authored-by: Michał Pierzchała <thymikee@gmail.com>
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
@mganandraj
Copy link
Contributor Author

The fix may not be required form ndk-build as it can handle the windows file paths i believe .. I will check it out and raise another PR if required.

@mganandraj mganandraj deleted the WindowsPath branch November 9, 2022 21:53
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.

None yet

4 participants