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

hide-library-schemes breaks Sentry CLI #80

Open
harunsmrkovic opened this issue Nov 27, 2019 · 6 comments
Open

hide-library-schemes breaks Sentry CLI #80

harunsmrkovic opened this issue Nov 27, 2019 · 6 comments

Comments

@harunsmrkovic
Copy link

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...

@thekevinbrown
Copy link
Owner

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?

@jon-nona
Copy link

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

@pierpo
Copy link
Contributor

pierpo commented Dec 15, 2021

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.

@thekevinbrown
Copy link
Owner

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.

@pierpo
Copy link
Contributor

pierpo commented Dec 16, 2021

Very interesting. Thank you!

Shouldn't we document this on the README? 😊

@thekevinbrown
Copy link
Owner

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

pierpo added a commit to pierpo/react-native-schemes-manager that referenced this issue Dec 24, 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

No branches or pull requests

4 participants