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.

Sign up for GitHub

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

Added option to disable xip #13326

Closed
wants to merge 1 commit into from
Closed

Added option to disable xip #13326

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions packager/react-native-xcode.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,14 @@ if [[ "$CONFIGURATION" = "Debug" && ! "$PLATFORM_NAME" == *simulator ]]; then
if [ -z "$IP" ]; then
IP=$(ifconfig | grep 'inet ' | grep -v 127.0.0.1 | cut -d\ -f2 | awk 'NR==1{print $1}')
fi

if [ -z ${DISABLE_XIP+x} ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, I'm working on this file and don't know what "+x" in ${DISABLE_XIP+x} does. (AFAICS, it is also not mentioned in the Bash manual.) Could you please explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's parameter expansion:

${parameter:+word}
If parameter is null or unset, nothing is substituted, otherwise the expansion of word is substituted.

So if DISABLE_XIP is null or unset then the check will fail otherwise it will pass. x is used here just as a placeholder.

Copy link
Contributor

@AlicanC AlicanC Oct 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Turns out I've missed the part where it explains that the colon can be omitted:

Omitting the colon results in a test only for a parameter that is unset. Put another way, if the colon is included, the operator tests for both parameter’s existence and that its value is not null; if the colon is omitted, the operator tests only for existence.

Thanks!

I think I will change this to [[ $DISABLE_XIP != true ]] like I did with all the other boolean-y variables.

To be honest, ${DISABLE_XIP+x} looks ugly. I have completely rewritten this script and this is the last part of the whole thing that I could not understand. I don't like saying "If that was hard for me, then it will be for other people." but I guess this one deserves an exception.

Also, correct me if I'm wrong, but if a variable is set and I want to run a single command with that variable being unset, I have to use unset DISABLE_XIP; command which unsets the variable for the whole env and not just the command. A subshell can be used to avoid that but it just gets uglier. With a != true check, you can just do DISABLE_XIP= command; to turn it off and DISABLE_XIP=true command; to turn it on. This doesn't really matter in this use case, but I guess I will adopt this as one of my "Bash Best Practices".

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that sounds completely reasonable and a nicer solution all round. Thanks!

IP="$IP.xip.io"
fi

$PLISTBUDDY -c "Add NSAppTransportSecurity:NSExceptionDomains:localhost:NSTemporaryExceptionAllowsInsecureHTTPLoads bool true" "$PLIST"
$PLISTBUDDY -c "Add NSAppTransportSecurity:NSExceptionDomains:$IP.xip.io:NSTemporaryExceptionAllowsInsecureHTTPLoads bool true" "$PLIST"
echo "$IP.xip.io" > "$DEST/ip.txt"
$PLISTBUDDY -c "Add NSAppTransportSecurity:NSExceptionDomains:$IP:NSTemporaryExceptionAllowsInsecureHTTPLoads bool true" "$PLIST"
echo "$IP" > "$DEST/ip.txt"
fi

BUNDLE_FILE="$DEST/main.jsbundle"
Expand Down