Remove "Check for Nested Frameworks" build phase

[iamgabrielma]

Description

This PR addresses the current warnings regarding the WooCommerce target build phase:

warning build: Run script build phase 'Check for nested frameworks' will be run during every build because it does not specify any outputs. To address this warning, either add output dependencies to the script phase, or configure it to run in every build by unchecking "Based on dependency analysis" in the script phase.

If we do not specify input or output files, Xcode will runs these scripts every time we build the target, regardless of modification status during incremental builds. As this could have a significant impact on compile time, we want Xcode to only run the scripts on the files that actually change.

As this script was added to fix an issue with Xcode 12.4, and is no longer a problem, this PR deals with removing the script and build phase that runs it.

Changes

• Removed "Check for nested frameworks" build phase
• Deleted check-nested-frameworks.sh script

Testing instructions

• Confirm the app builds successfully, and the phase is no longer part of building the WooCommerce target
• Confirm the warning message for "Check for nested frameworks" has disappeared

Screenshots

---

[wpmobilebot]

You can test the changes from this Pull Request by:

• Clicking here or scanning the QR code below to access App Center
• Then installing the build number pr8134-ab50cc1 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

[mokagio]

What would happen on a clean build if the script fails? 🤔 Does Xcode exit as well (equivalent to set -e) or runs the echo command anyway?

woocommerce-ios/Scripts/check-nested-frameworks.sh

Line 24 in 0217932

exit 1

[iamgabrielma]

Oh that's a good point, Xcode will run the echo no matter if check-nested-frameworks.sh exits with 0 or other value, so will always print "SUCCESS" to the output file.

In this case, we could write the result of the input file into the output file.

echo `${SCRIPT_INPUT_FILE_0}` > ${SCRIPT_OUTPUT_FILE_0}

So when we go to the check-for-nested-frameworks.txt file, we can see the actual message that the script returned.

[mokagio]

Oh, interesting.

In this case, we could write the result of the input file into the output file.

echo `${SCRIPT_INPUT_FILE_0}` > ${SCRIPT_OUTPUT_FILE_0}

Have you considered using tee?

./${SCRIPT_INPUT_FILE_0} | tee ${SCRIPT_OUTPUT_FILE_0}

Also, I can't see an input file in the list in this diff. Perhaps this would achieve logging the script run result to file?

${PROJECT_DIR}/../Scripts/check-nested-frameworks.sh | tee ${SCRIPT_OUTPUT_FILE_0}

[iamgabrielma]

Have you considered using tee?

I didn't know that one, thanks!

[mokagio]

Thank you for looking into this @iamgabrielma 🙌

I looked at the script for the nested framework recently, and thought "do we still need it?"

The script states to be addressing an issue in Xcode 12. Is the issue still present in Xcode 14?

woocommerce-ios/Scripts/check-nested-frameworks.sh

Lines 3 to 5 in 0217932

	# Nested frameworks (i.e. having a Frameworks/ folder inside *.app/Frameworks/.framework) is invalid and will make the build be rejected during TestFlight validation.
	# This can happen especially due to an Xcode 12.4 UI bug when linking binary frameworks to the project which always embed the binary (and if you try to change to Do Not Embed it also removes it from linked libraries)
	# This is a bug in Xcode 12.4 UI that is fixed in Xcode 12.5, so fixing nested frameworks to "Do Not Embed" can be fixed using Xcode 12.5, while still continue using Xcode 12.4 for development after the fix.

If the issue is gone. Should we keep the check just in case, or remove it in favor of a slightly faster build and under the assumption that, were the bug to come back, we'll know about it via Test Flight anyway and be able to re-introduce the script?

[iamgabrielma]

Thanks for taking a look @mokagio !

If the issue is gone. Should we keep the check just in case, or remove it in favor of a slightly faster build and under the assumption that, were the bug to come back, we'll know about it via Test Flight anyway and be able to re-introduce the script?

You're right, in this case I'd say it would be preferable to remove it to reduce the clutter and make the build faster, specially since at the moment we're also requiring Xcode 14 or newer.

[iamgabrielma]

@mokagio , from #8098 it would seem that we're gravitating towards running SwiftLint in every build, in this case no changes would be needed besides unchecking the "Based on dependency analysis" in the Build Phase. If that's case, I think I can reduce this PR to only removing the Check for nested frameworks phase. Would you agree?

[mokagio]

@iamgabrielma

I think I can reduce this PR to only removing the Check for nested frameworks phase. Would you agree?

Yes. Thanks!