Stop downloading new relay list on every build when building ios 1397

[pinkisemils]

I've changed the app to always download the relay list if the bundled one is empty in debug builds. Having an empty prebundled relay list will now fail the build. I've achieved this by changing the relay downloading script to never download relays and instead validate the list itself if it doesn't exist. This way, we can keep the file referenced in the project, and we can stop updating it all the time. MockRelease will require a checked out relay list since it will not include the code that will fetch the list by default.

I've tried to scope all the app changes to only be there in debug builds. The relay list downloading should take place soon after the app delegate has initialized.

To test this out, you can try building a MockRelease build without a relay list.
Then you can set an invalid one and see it fail still:

echo '{}' > ios/MullvadREST/Assets/relays.json

And you should be able to make it build:

curl https://api.mullvad.net/app/v1/relays > ios/MullvadREST/Assets/relays.json

And after removing the relay list, you should still be able to do a fresh Debug install on a device and see it work as expected. This does mean that fresh debug installs will only work when the device can reach the API to fetch a new list.

---

This change is 

[linear]

IOS-1397 Stop downloading new relay list on every build when building production builds

[rablador]

I like the idea here, but I think all the debug flags pollute the code a bit. Also, what are the consequences if we accidentally remove one that shouldn't be removed?

@rablador partially reviewed 15 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pinkisemils).

---

ios/MullvadVPN.xcodeproj/xcshareddata/xcbaselines/58B0A29F238EE67E00BC001D.xcbaseline/0B028554-E830-4027-9640-5C31EBD6454D.plist line 1 at r4 (raw file):

<?xml version="1.0" encoding="UTF-8"?>

What's this file?

---

ios/MullvadVPN.xcodeproj/xcshareddata/xcbaselines/58B0A29F238EE67E00BC001D.xcbaseline/Info.plist line 1 at r4 (raw file):

<?xml version="1.0" encoding="UTF-8"?>

And this?

[pinkisemils]

I can revise the changes such that instead of having the debug flags being a part of a long function, the optional code can be stuffed into a function/method and the whole body of that method can be conditional. What I don't like about this is that the call site will look weird, unless we call the function something like downloadRelayListIfDebug.

@pinkisemils made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador).

---

ios/MullvadVPN.xcodeproj/xcshareddata/xcbaselines/58B0A29F238EE67E00BC001D.xcbaseline/0B028554-E830-4027-9640-5C31EBD6454D.plist line 1 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

What's this file?

This is a baseline for a benchmark for the AllLocationDataSourceBenchmarkTests.

---

ios/MullvadVPN.xcodeproj/xcshareddata/xcbaselines/58B0A29F238EE67E00BC001D.xcbaseline/Info.plist line 1 at r4 (raw file):

Previously, rablador (Jon Petersson) wrote…

And this?

Same as above. I should've added these earlier.

[acb-mv]

@acb-mv partially reviewed 15 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador).

---

ios/checkRelays.swift line 22 at r4 (raw file):

    }

    print("Validating list")

Any reason for using both print and fputs?

[rablador]

Do you mean for all flags in each flag?

@rablador made 1 comment and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved.

[rablador]

*in each file

@rablador made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved.

[mojganii]

@mojganii made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils).

---

ios/checkRelays.swift line 22 at r4 (raw file):

Previously, acb-mv wrote…

Any reason for using both print and fputs?

+1

---

ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift line 69 at r4 (raw file):

            try hotfixRelaysThatDoNotHaveFeatures()

            #if DEBUG

wasn't it suuposed to create another compiler falg for it?

[pinkisemils]

Not all flags for each file, but only where it makes sense. As discussed, I've changed to use a different flag and removed some use of the flags where they weren't absolutely necessary.

@pinkisemils made 2 comments.
Reviewable status: 7 of 16 files reviewed, 1 unresolved discussion (waiting on @acb-mv, @mojganii, and @rablador).

---

ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift line 69 at r4 (raw file):

Previously, mojganii wrote…

wasn't it suuposed to create another compiler falg for it?

You're right. Should be fixed now.

[rablador]

@rablador reviewed 9 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mojganii and @pinkisemils).

---

ios/Configurations/Base.xcconfig.template line 27 at r5 (raw file):

// Flag used to conditionally show features that are in development
SWIFT_ACTIVE_COMPILATION_CONDITIONS[config=Debug] = DEBUG NOT_PRODUCTION_PERMANENT

Nit: I can't think of something better, but reading #if NOT_PRODUCTION_PERMANENT in one swoop makes it sound like it's not production permanent, rather than not production and here to stay.

[mojganii]

@mojganii made 2 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @pinkisemils).

---

ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift line 69 at r4 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

You're right. Should be fixed now.

shouldn't it be replaced with the new flag which is NOT_PRODUCTION_PERMANENT?

---

ios/Configurations/Base.xcconfig.template line 28 at r5 (raw file):

// Flag used to conditionally show features that are in development
SWIFT_ACTIVE_COMPILATION_CONDITIONS[config=Debug] = DEBUG NOT_PRODUCTION_PERMANENT
SWIFT_ACTIVE_COMPILATION_CONDITIONS[config=Staging] = DEBUG NOT_PRODUCTION_PERMANENT

nit: : NON_PRODUCTION_ONLY, INTERNAL_ONLY

[pinkisemils]

@pinkisemils made 3 comments.
Reviewable status: 15 of 16 files reviewed, 3 unresolved discussions (waiting on @mojganii and @rablador).

---

ios/Configurations/Base.xcconfig.template line 27 at r5 (raw file):

Previously, rablador (Jon Petersson) wrote…

Nit: I can't think of something better, but reading #if NOT_PRODUCTION_PERMANENT in one swoop makes it sound like it's not production permanent, rather than not production and here to stay.

How about NEVER_IN_PRODUCTION ?

---

ios/Configurations/Base.xcconfig.template line 28 at r5 (raw file):

Previously, mojganii wrote…

nit: : NON_PRODUCTION_ONLY, INTERNAL_ONLY

How about NEVER_IN_PRODUCTION ?

---

ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift line 69 at r4 (raw file):

Previously, mojganii wrote…

shouldn't it be replaced with the new flag which is NOT_PRODUCTION_PERMANENT?

Done.

[mojganii]

@mojganii made 2 comments and resolved 1 discussion.
Reviewable status: 14 of 16 files reviewed, 2 unresolved discussions (waiting on @pinkisemils and @rablador).

---

ios/Configurations/Base.xcconfig.template line 28 at r5 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

How about NEVER_IN_PRODUCTION ?

looks good to me.

[rablador]

@rablador reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pinkisemils).

---

ios/Configurations/Base.xcconfig.template line 27 at r5 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

How about NEVER_IN_PRODUCTION ?

Let's go with that.