Skip to content

Add build phase script to remove Wormholy files #15804

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

Closed
wants to merge 2 commits into from
Closed

Add build phase script to remove Wormholy files #15804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e84f472
Add build phase run script to remove Wormholy from release builds
mokagio Jun 24, 2025
ea3e407
HACK to test removal on debug and prototype builds
mokagio Jun 24, 2025
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
13 changes: 13 additions & 0 deletions Scripts/build-phases/strip-wormholy-from-release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/bin/bash -eu

# if [ "${CONFIGURATION}" != "Release" ]; then
# echo "info: Skipping Wormholy removal – not a Release build."
# exit 0
# fi

# FIXME: Delete from all builds to test the implementation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously we should remove the comments before merging.


BUILT_PRODUCTS_DIR=${BUILT_PRODUCTS_DIR:-"$TARGET_BUILD_DIR"}

# Crude way to remove all the Wormholy files from the build.
rm -rf "$BUILT_PRODUCTS_DIR/Wormholy*"
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using find instead (or find + rm), so we can have more control of what we're searching, can add error handling, etc?

if find "$BUILT_PRODUCTS_DIR" -name "*Wormholy*" -print -delete | grep -q .; then
    echo "info: Successfully removed Wormholy files"
else
    echo "warning: No Wormholy files found to remove"
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that find … -delete deletes folders (as opposed to only files)? Or if it does, that it is able to delete the folder recursively, or only in the right order only after *Wormholy* files it contains have been deleted and it happens that there would be no remaining files. We might need a -prune to avoid find to try to scan directories after they've been deleted as well…

Besides, using find like this means it might delete *Wormholy* files other than just the ones at the root of the $BUILT_PRODUCTS_DIR and potentally files deeper in this folder structure. Maybe that's actually why you suggested this @iangmaia … but I'm also not sure this would be the right idea as this feels a tad more "dangerous" than only removing $BUILT_PRODUCTS_DIR/Wormholy* (what if frameworks and targets unrelated with Wormholy itself would include the name "Wormholy" in their file name deep inside a folder that is not $BUILT_PRODUCTS_DIR/Wormholy*/ but $BUILT_PRODUCTS_DIR/Somethingelse/…/BlahWormholyBah…)

Overall I'm not sure there's any perfect solution, unless we find a way to determine the actual list of files generated by the Wormholy package in $BUILT_PRODUCTS_DIR/ by analyzing its Package or SwiftPM target in some clever way (checking the compilation database or whatever?)… which I'm not sure is worth the effort.

Copy link
Contributor

@AliSoftware AliSoftware Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Btw, are we sure that removing the Wormholy* files from BUILT_PRODUCTS_DIR is enough to remove Wormholy from the final binary? I'm sure it would help reducing the binary file size if that works by ensuring the .o is not linked to the final binary if that .o doesn't exist anymore… but I'm worried about other side effects to the compilation process of deleting files from $BUILT_PRODUCTS_DIR/ in the middle of a build there. Like wouldn't the linker risk failing while it'd try to link to the Wormholy.o file and not find it? Or even if it doesn't, maybe there's a risk of crash at runtime on the Release build in case Wormholy is integrated as a dynamic framework (as opposed to statically linked) and dyld would still find instructions to load that Wormholy framework (as per otool -L) in its loading instructions… fail to find the framework in its @rpath…

Which would actually be the worst case scenario, as that would make Debug builds compile and work, and Release builds compile too (making us think they also work) but failed to launch… and it'd be easy to thus submit a Release build to App Store that would in practice fail to launch and not realize it until it's too late…

I don't have any magical solution for that though tbh. The only option I could think of that might feel safe enough to me would be remove Wormholy from the project before building a Release Build. That is, to write a script that would remove Wormholy as a Package Dependency on the project, and run that script on CI before we resolve package dependencies and compile the Release app for submission to TestFlight and AppStore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, using the command as-is would delete only files (and empty directories), but indeed also files deeper in the directory structure. My main idea was to go deeper in the structure and have more control over the search (we could do alternatively something like find "$BUILT_PRODUCTS_DIR" -name "*Wormholy*" -exec rm -rf {})... but 💯 agree it can be dangerous and might not be enough to remove it from the final binary.

remove Wormholy from the project before building a Release Build

This actually doesn't sound too bad given the current constraints 🤔

Copy link
Contributor Author

@mokagio mokagio Jun 25, 2025 •
edited by AliSoftware
Loading

Choose a reason for hiding this comment

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

That is, to write a script that would remove Wormholy as a Package Dependency on the project, and run that script on CI before we resolve package dependencies and compile the Release app for submission to TestFlight and AppStore.

🤔 🤔 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented in #15813.

14 changes: 7 additions & 7 deletions WooCommerce/Classes/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import class Yosemite.ScreenshotStoresManager
// In that way, Inject will be available in the entire target.
@_exported import Inject

#if DEBUG
import WormholySwift
#endif
//#if DEBUG
Copy link
Contributor

Choose a reason for hiding this comment

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

I was gonna say something about trying condition: .when(configuration: .debug) but just seen this discussion 😞

SwiftPM allows conditional platform dependencies, but not conditional configurations.

//import WormholySwift
//#endif

// MARK: - Woo's App Delegate!
//
Expand Down Expand Up @@ -415,10 +415,10 @@ private extension AppDelegate {
/// Set up Wormholy only in Debug build configuration
///
func setupWormholy() {
#if DEBUG
// We want to activate it programmatically, not using the shake.
Wormholy.shakeEnabled = false
#endif
//#if DEBUG
// // We want to activate it programmatically, not using the shake.
// Wormholy.shakeEnabled = false
//#endif
}

/// Set up `KeyboardStateProvider`
Expand Down
19 changes: 19 additions & 0 deletions WooCommerce/WooCommerce.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14487,6 +14487,7 @@
B5650B1020A4CD7F009702D0 /* Embed Frameworks */,
3F1FA85028B60126009E246C /* Embed Foundation Extensions */,
26F81B232BE433A4009EC58E /* Embed Watch Content */,
3F76AD982E0A849700984880 /* Strip Wormholy from Release build */,
);
buildRules = (
);
Expand Down Expand Up @@ -15076,6 +15077,24 @@
shellScript = "\"$SRCROOT/../Scripts/build-phases/LintAppLocalizedStringsUsage.sh\"\n";
showEnvVarsInLog = 0;
};
3F76AD982E0A849700984880 /* Strip Wormholy from Release build */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
files = (
);
inputFileListPaths = (
);
inputPaths = (
);
name = "Strip Wormholy from Release build";
outputFileListPaths = (
);
outputPaths = (
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
shellScript = "$SRCROOT/../Scripts/build-phases/strip-wormholy-from-release.sh\n";
};
57CCFFD4249D2A5700825FCF /* SwiftLint */ = {
isa = PBXShellScriptBuildPhase;
buildActionMask = 2147483647;
Expand Down