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

Jump to bottom

Fix VPN uninstall not working for App Store users. #2951

Merged
merged 1 commit into from
Jul 9, 2024
Merged

Fix VPN uninstall not working for App Store users. #2951

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion Configuration/App/NetworkProtection/DuckDuckGoVPNAppStore.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ CODE_SIGN_IDENTITY[config=CI][sdk=macosx*] =
PRODUCT_NAME_PREFIX = $(AGENT_PRODUCT_NAME)
PRODUCT_NAME = $(AGENT_PRODUCT_NAME)
PRODUCT_NAME[config=Review][arch=*][sdk=*] = $(AGENT_PRODUCT_NAME)
PRODUCT_NAME[config=Release][arch=*][sdk=*] = $(AGENT_RELEASE_PRODUCT_NAME)
PRODUCT_NAME[config=Release][arch=*][sdk=*] = $(AGENT_PRODUCT_NAME)
Copy link
Contributor Author

@diegoreymendez diegoreymendez Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Read my comment below. Even though we always use AGENT_PRODUCT_NAME this is specifically expanded because the value of AGENT_PRODUCT_NAME is different for release builds, and Xcode otherwise seems to miss it.


PROVISIONING_PROFILE_SPECIFIER[sdk=macosx*] =
PROVISIONING_PROFILE_SPECIFIER[config=Release][sdk=macosx*] = match AppStore com.duckduckgo.mobile.ios.vpn.agent macos
Expand Down
2 changes: 1 addition & 1 deletion Configuration/AppStore.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ AGENT_BUNDLE_ID[config=CI][sdk=*] = $(AGENT_BUNDLE_ID_BASE).debug
AGENT_BUNDLE_ID[config=Review][sdk=*] = $(AGENT_BUNDLE_ID_BASE).review

AGENT_PRODUCT_NAME = DuckDuckGo VPN App Store
AGENT_RELEASE_PRODUCT_NAME = DuckDuckGo VPN
AGENT_PRODUCT_NAME[config=Release][sdk=*] = DuckDuckGo VPN
Copy link
Contributor Author

@diegoreymendez diegoreymendez Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Quite honestly there should be no difference in just setting this directly, because in the end we were loading this value directly into the product name.

Additionally AGENT_PRODUCT_NAME is saved in Info.plist so that the code knows what the agent product name is - this is what was causing the issue since the app was trying to launch the VPN menu app with the wrong product name (loaded from info.plist).


// Extensions

Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9531,7 +9531,7 @@
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "# Embeds login items for the App Store build.\n\n# Skip login item embedding for release builds until they're ready to go live.\nif [ \"${CONFIGURATION}\" = \"Release\" ]; then\n VPN_AGENT_NAME=\"${AGENT_RELEASE_PRODUCT_NAME}\"\n PIR_AGENT_NAME=\"${DBP_BACKGROUND_AGENT_RELEASE_PRODUCT_NAME}\"\nelse\n VPN_AGENT_NAME=\"${AGENT_PRODUCT_NAME}\"\n PIR_AGENT_NAME=\"${DBP_BACKGROUND_AGENT_PRODUCT_NAME}\"\nfi\n\nVPN_AGENT_ORIGIN=$(readlink -f \"${CONFIGURATION_BUILD_DIR}/${VPN_AGENT_NAME}.app\")\nPIR_AGENT_ORIGIN=$(readlink -f \"${CONFIGURATION_BUILD_DIR}/${PIR_AGENT_NAME}.app\")\nAGENT_DESTINATION=\"${CONFIGURATION_BUILD_DIR}/${CONTENTS_FOLDER_PATH}/Library/LoginItems\"\n \n# Make sure that Library/LoginItems exists before copying\nmkdir -p \"$AGENT_DESTINATION\"\n \necho \"Copying VPN agent from $VPN_AGENT_ORIGIN to $AGENT_DESTINATION\"\nrsync -r --links \"$VPN_AGENT_ORIGIN\" \"$AGENT_DESTINATION\"\n \necho \"Copying Personal Information Removal agent from $PIR_AGENT_ORIGIN to $AGENT_DESTINATION\"\nrsync -r --links \"$PIR_AGENT_ORIGIN\" \"$AGENT_DESTINATION\"\n";
shellScript = "# Embeds login items for the App Store build.\n\n# Skip login item embedding for release builds until they're ready to go live.\nif [ \"${CONFIGURATION}\" = \"Release\" ]; then\n PIR_AGENT_NAME=\"${DBP_BACKGROUND_AGENT_RELEASE_PRODUCT_NAME}\"\nelse\n PIR_AGENT_NAME=\"${DBP_BACKGROUND_AGENT_PRODUCT_NAME}\"\nfi\n\nVPN_AGENT_NAME=\"${AGENT_PRODUCT_NAME}\"\n\nif [ -z \"$VPN_AGENT_NAME\" ]; then\n echo \"VPN_AGENT_NAME is empty.\"\n exit 1\nelif [ -z \"$PIR_AGENT_NAME\" ]; then\n echo \"PIR_AGENT_NAME is empty.\"\n exit 1\nelse\n echo \"Both VPN_AGENT_NAME and PIR_AGENT_NAME are set.\"\n echo \"VPN_AGENT_NAME is set to '$VPN_AGENT_NAME'.\"\n echo \"PIR_AGENT_NAME is set to '$PIR_AGENT_NAME'.\"\nfi\n\nVPN_AGENT_ORIGIN=$(readlink -f \"${CONFIGURATION_BUILD_DIR}/${VPN_AGENT_NAME}.app\")\nPIR_AGENT_ORIGIN=$(readlink -f \"${CONFIGURATION_BUILD_DIR}/${PIR_AGENT_NAME}.app\")\nAGENT_DESTINATION=\"${CONFIGURATION_BUILD_DIR}/${CONTENTS_FOLDER_PATH}/Library/LoginItems\"\n \n# Make sure that Library/LoginItems exists before copying\nmkdir -p \"$AGENT_DESTINATION\"\n \necho \"Copying VPN agent from $VPN_AGENT_ORIGIN to $AGENT_DESTINATION\"\nrsync -r --links \"$VPN_AGENT_ORIGIN\" \"$AGENT_DESTINATION\"\n \necho \"Copying Personal Information Removal agent from $PIR_AGENT_ORIGIN to $AGENT_DESTINATION\"\nrsync -r --links \"$PIR_AGENT_ORIGIN\" \"$AGENT_DESTINATION\"\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You might want to look at this in Xcode. Basically removed the override and just use AGENT_PRODUCT_NAME.

I've also added a check to ensure that VPN_AGENT_NAME and PIR_AGENT_NAME are not empty, since if they're empty some really odd stuff happens later in the build.

};
6A8856B31B2BC5078B61ED81 /* Run swiftlint */ = {
isa = PBXShellScriptBuildPhase;
Expand Down
Loading