react-native-schemes-manager icon indicating copy to clipboard operation
react-native-schemes-manager copied to clipboard

hide-library-schemes breaks Sentry CLI

Open harunsmrkovic opened this issue 5 years ago • 6 comments

Steps to reproduce the behavior

Have this as "Bundle React native code and images" script in your React-Native build phases:

export DEVELOPMENT_BUILD_CONFIGURATIONS="+(Debug)"
export NODE_BINARY=node
export SENTRY_PROPERTIES=./sentry.properties

$NODE_BINARY ../node_modules/@sentry/cli/bin/sentry-cli react-native xcode ../node_modules/react-native-schemes-manager/lib/react-native-xcode.sh

Expected behavior

Release should be created in the appropriate Sentry project, with main.jsbundle and main.jsbundle.map files uploaded.

Actual behavior

Release is not created, and no files are ever uploaded.

Notes

So, I was moving stuff around in the build script, and noticed that removing these two lines:

cd "$SCHEMES_MANAGER_DIR/../.."
$NODE_BINARY "$SCHEMES_MANAGER_DIR/index.js" hide-library-schemes

actually fixes the issue! :)

Also, it does not have any ill effects to the work of this package itself...

harunsmrkovic avatar Nov 27 '19 09:11 harunsmrkovic

Yeah, the library schemes step just cleans up the menu in Xcode, there are no functional changes to the app or the build, it's just marking the schemes as hidden in the project files in node_modules so they don't clutter up the menu in Xcode.

I'm curious though why that'd fix and/or break it. Does Sentry rely on a particular scheme being visible that we're hiding or something?

thekevinbrown avatar Dec 11 '19 01:12 thekevinbrown

@thekevinbrown I see @matt-oakes seemed to find the issue and why that causes it here

jon-nona avatar Jul 14 '20 09:07 jon-nona

Notes

So, I was moving stuff around in the build script, and noticed that removing these two lines:

cd "$SCHEMES_MANAGER_DIR/../.."
$NODE_BINARY "$SCHEMES_MANAGER_DIR/index.js" hide-library-schemes

actually fixes the issue! :)

Also, it does not have any ill effects to the work of this package itself...

I just faced the same issue. My workaround is similar to yours, but more radical. I am bypassing the whole execution of react-native-schemes-manager xcode script. I say it's similar because the only added logic in the custom script are these 2 lines (or at least these are the only 2 I could identify as different).

- $NODE_BINARY ../node_modules/@sentry/cli/bin/sentry-cli react-native xcode --force-foreground  ../node_modules/react-native-schemes-manager/lib/react-native-xcode.sh
+ $NODE_BINARY ../node_modules/@sentry/cli/bin/sentry-cli react-native xcode --force-foreground  ../node_modules/react-native/scripts/react-native-xcode.sh

I did it with a simple hack: I added a # at the end of the package.json. Which comments out the part that is inserted by react-native-schemes-manager.

    "settings": {
      "fix-script": {
        "nodeCommand": "$NODE_BINARY ../node_modules/@sentry/cli/bin/sentry-cli react-native xcode --force-foreground  ../node_modules/react-native/scripts/react-native-xcode.sh #"
      }
    },

But there is a way more important issue with this custom Xcode script! This script is actually a copy paste from react-native! So, if you use react-native-schemes-manager, you won't get the latest react-native logic regarding the Xcode build.

I am really questioning the need for this script. OK we might need to hide the schemes, but why don't we do a separate Build phase that executes these 2 lines on its own? Does it need the previous stuff?

I think that the current approach is very dangerous. It will lead to many problems without developers knowing.

pierpo avatar Dec 15 '21 10:12 pierpo

When this package was built, React Native did not handle schemes correctly. The script was copy / paste with modifications to enable schemes to work.

In versions of React native >0.6.0, this package is no longer needed at all.

thekevinbrown avatar Dec 15 '21 23:12 thekevinbrown

Very interesting. Thank you!

Shouldn't we document this on the README? 😊

pierpo avatar Dec 16 '21 09:12 pierpo

Sure, just haven't had time myself. PRs are welcome.

thekevinbrown avatar Dec 17 '21 03:12 thekevinbrown