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 support for --sourcemap-output path containing spaces in Xcode projects #30981

Closed
wants to merge 2 commits into from

Conversation

nickdowell
Copy link

Summary

This change fixes the generation of source maps for Xcode projects where the output path contains spaces.

The EXTRA_ARGS environment variable, being a plain string, would be split into arguments by whitespace - so a path containing spaces was being treated as several arguments rather than one.

This change uses an array to contain the arguments instead, allowing the proper handling of arguments that may contain spaces.

Changelog

[iOS] [Fixed] - Fix support for --sourcemap-output path containing spaces

Test Plan

Tested using a sample project with the following "Bundle React Native code and images" Xcode build phase

export SOURCEMAP_FILE="$CONFIGURATION_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH/main.jsbundle.map"
set -e
export NODE_BINARY=node
../node_modules/react-native/scripts/react-native-xcode.sh

and a CONFIGURATION_BUILD_DIR that contains spaces - ~/Library/Xcode/Derived Data

Before

+ EXTRA_ARGS=
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS=' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
                    Welcome to Metro!
              Fast - Scalable - Integrated


info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived

Note the incorrect sourcemap output path.

After

+ EXTRA_ARGS=()
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS+=("--sourcemap-output")
+ EXTRA_ARGS+=("$PACKAGER_SOURCEMAP_FILE")
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
                    Welcome to Metro!
              Fast - Scalable - Integrated


info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map

sourcemap output path fixed 🎉

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 11, 2021
@analysis-bot
Copy link

analysis-bot commented Feb 11, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: cdf3182

@analysis-bot
Copy link

analysis-bot commented Feb 11, 2021

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,908,406 +0
android hermes armeabi-v7a 8,407,477 +0
android hermes x86 9,398,953 +0
android hermes x86_64 9,342,879 +0
android jsc arm64-v8a 10,642,320 +0
android jsc armeabi-v7a 10,124,479 +0
android jsc x86 10,694,366 +0
android jsc x86_64 11,278,558 +0

Base commit: cdf3182

scripts/react-native-xcode.sh Outdated Show resolved Hide resolved
scripts/react-native-xcode.sh Outdated Show resolved Hide resolved
nickdowell and others added 2 commits March 10, 2021 13:31
Co-authored-by: Nurlan Suyundukov <nurlansu@users.noreply.github.com>
@nickdowell
Copy link
Author

Closing because there's clearly no interest in merging this fix, and I'm no longer working with react-native so not in a position to resolve the merge conflict or do any testing.

@nickdowell nickdowell closed this Aug 9, 2023
@joren-vos-aca
Copy link

joren-vos-aca commented Oct 13, 2023

Why is this one closed without being merged? This is still a bug for every project that has a space in -for example- its scheme name.

facebook-github-bot pushed a commit that referenced this pull request Dec 28, 2023
…ojects (#40937)

Summary:
This PR contains the changes from #30981 that got closed due to inactivity.

Many thanks to nickdowell for this bug report & fix. We encountered this error in our project when we had an Xcode scheme that contains a space (like `AppName alpha`).

This change fixes the generation of source maps for Xcode projects where the output path contains spaces.

The `EXTRA_ARGS` environment variable, being a plain string, would be split into arguments by whitespace - so a path containing spaces was being treated as several arguments rather than one.

This change uses an array to contain the arguments instead, allowing the proper handling of arguments that may contain spaces.

bypass-github-export-checks

## Changelog:

[iOS] [Fixed] - Fix support for --sourcemap-output path containing spaces

Pull Request resolved: #40937

Test Plan:
Tested using a sample project with the following "Bundle React Native code and images" Xcode build phase

```
export SOURCEMAP_FILE="$CONFIGURATION_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH/main.jsbundle.map"
set -e
export NODE_BINARY=node
../node_modules/react-native/scripts/react-native-xcode.sh
```

and a `CONFIGURATION_BUILD_DIR` that contains spaces - `~/Library/Xcode/Derived Data`. **You can also try an XCode-scheme that contains a space.**

### Before

```
+ EXTRA_ARGS=
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS=' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
                    Welcome to Metro!
              Fast - Scalable - Integrated

info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived
```
Note the incorrect sourcemap output path.

### After

```
+ EXTRA_ARGS=()
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS+=("--sourcemap-output")
+ EXTRA_ARGS+=("$PACKAGER_SOURCEMAP_FILE")
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
                    Welcome to Metro!
              Fast - Scalable - Integrated

info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
```
sourcemap output path fixed 🎉

Reviewed By: arushikesarwani94

Differential Revision: D52431057

Pulled By: cipolleschi

fbshipit-source-id: 528217c84fe3f467a30baa15cfa4dcb2ed713165
Othinn pushed a commit to Othinn/react-native that referenced this pull request Jan 9, 2024
…ojects (facebook#40937)

Summary:
This PR contains the changes from facebook#30981 that got closed due to inactivity.

Many thanks to nickdowell for this bug report & fix. We encountered this error in our project when we had an Xcode scheme that contains a space (like `AppName alpha`).

This change fixes the generation of source maps for Xcode projects where the output path contains spaces.

The `EXTRA_ARGS` environment variable, being a plain string, would be split into arguments by whitespace - so a path containing spaces was being treated as several arguments rather than one.

This change uses an array to contain the arguments instead, allowing the proper handling of arguments that may contain spaces.

bypass-github-export-checks

## Changelog:

[iOS] [Fixed] - Fix support for --sourcemap-output path containing spaces

Pull Request resolved: facebook#40937

Test Plan:
Tested using a sample project with the following "Bundle React Native code and images" Xcode build phase

```
export SOURCEMAP_FILE="$CONFIGURATION_BUILD_DIR/$UNLOCALIZED_RESOURCES_FOLDER_PATH/main.jsbundle.map"
set -e
export NODE_BINARY=node
../node_modules/react-native/scripts/react-native-xcode.sh
```

and a `CONFIGURATION_BUILD_DIR` that contains spaces - `~/Library/Xcode/Derived Data`. **You can also try an XCode-scheme that contains a space.**

### Before

```
+ EXTRA_ARGS=
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS=' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
                    Welcome to Metro!
              Fast - Scalable - Integrated

info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived
```
Note the incorrect sourcemap output path.

### After

```
+ EXTRA_ARGS=()
+ case "$PLATFORM_NAME" in
+ BUNDLE_PLATFORM=ios
+ EMIT_SOURCEMAP=
+ [[ ! -z /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map ]]
+ EMIT_SOURCEMAP=true
+ PACKAGER_SOURCEMAP_FILE=
+ [[ true == true ]]
+ [[ '' == true ]]
+ PACKAGER_SOURCEMAP_FILE='/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
+ EXTRA_ARGS+=("--sourcemap-output")
+ EXTRA_ARGS+=("$PACKAGER_SOURCEMAP_FILE")
+ node /Users/nick/Desktop/RN064/node_modules/react-native/cli.js bundle --entry-file index.js --platform ios --dev false --reset-cache --bundle-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle' --assets-dest '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app' --sourcemap-output '/Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map'
                    Welcome to Metro!
              Fast - Scalable - Integrated

info Writing bundle output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/main.jsbundle
info Writing sourcemap output to:, /Users/nick/Library/Developer/Xcode/Derived Data/RN064-cpnwckdferodycbevupbrkjydate/Build/Products/Release-iphonesimulator/RN064.app/main.jsbundle.map
```
sourcemap output path fixed 🎉

Reviewed By: arushikesarwani94

Differential Revision: D52431057

Pulled By: cipolleschi

fbshipit-source-id: 528217c84fe3f467a30baa15cfa4dcb2ed713165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants