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.

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

feat!: remove default build dir overrides #1310

Merged
merged 1 commit into from
May 28, 2023
Merged

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Apr 15, 2023

Platforms affected

iOS

Motivation and Context

Specifying CONFIGURATION_BUILD_DIR can cause issues with Cocoapods.

Closes #617
Closes #659
Closes #671

Description

Once upon a time, Xcode's default directory values had issues when there were spaces in project names, so we override the CONFIGURATION_BUILD_DIR and SHARED_PRECOMPS_DIR variables with our own paths.

However, this can interfere with Cocoapods, and Xcode should be able to handle things sensibly nowadays.

Unfortunately, that results in our build artifacts living somewhere in a randomly-named Xcode DerivedData directory, and then we can't do things like run the app in a simulator or on a device because we don't know the path to it. 🤦🏼‍♂️

Setting SYMROOT allows us to control the output directory of the built products, with the caveat that Xcode always creates a subdirectory named with the current configuration.

So instead of build/emulator, we'll have build/Debug-iphonesimulator and instead of build/device, we'll have build/Release-iphoneos.
Hypothetically this is better because now we are sure that debug and release files never get mixed up in the same output directory.

The downside is that this is a breaking change because it alters the path for the output .ipa files.

Testing

  • Unit tests pass
  • Created test project with spaces in the project name and confirmed that it builds successfully
  • Created a test project with Cocoapods that previously failed and confirmed that it builds successfully
  • Run a test project in the simulator (debug configuration)
  • Run a test project in the simulator (release configuration)
  • Run a test project on a device (debug configuration)
  • Run a test project on a device (release configuration)

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)

@dpogue dpogue added this to the 7.0.0 milestone Apr 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2023

Codecov Report

Merging #1310 (91dbae5) into master (3d6c71a) will increase coverage by 0.00%.
The diff coverage is 64.70%.

@@           Coverage Diff           @@
##           master    #1310   +/-   ##
=======================================
  Coverage   78.48%   78.49%           
=======================================
  Files          15       15           
  Lines        1780     1790   +10     
=======================================
+ Hits         1397     1405    +8     
- Misses        383      385    +2     
Impacted Files Coverage Δ
lib/run.js 21.73% <0.00%> (-0.24%) ⬇️
lib/build.js 68.04% <84.61%> (+1.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@dpogue dpogue changed the title feat!: Remove default build dir overrides BREAKING -- feat!: Remove default build dir overrides Apr 16, 2023
@dpogue dpogue force-pushed the dir-settings branch 2 times, most recently from 7e70eea to 0da4b51 Compare April 16, 2023 07:02
Once upon a time, Xcode's default directory values had issues when there
were spaces in project names, so we override the CONFIGURATION_BUILD_DIR
and SHARED_PRECOMPS_DIR variables with our own paths.

However, this can interfere with Cocoapods, and Xcode should be able to
handle things sensibly nowadays.

Unfortunately, that results in our build artifacts living somewhere in a
randomly-named Xcode DerivedData directory, and then we can't do things
like run the app in a simulator or on a device because we don't know the
path to it.

Setting SYMROOT allows us to control the output directory of the built
products, with the caveat that Xcode always creates a subdirectory named
with the current configuration.

So instead of `build/emulator`, we'll have `build/Debug-iphonesimulator`
and instead of `build/device`, we'll have `build/Release-iphoneos`.

Hypothetically this is better because now we are sure that debug and
release files never get mixed up in the same output directory.

The downside is that this is a breaking change because it alters the
path for the output .ipa files.

Closes apache#617.
Closes apache#659.
Closes apache#671.

Co-Authored-By: Susan Tan <onceuponatimeforever@gmail.com>
@dpogue dpogue marked this pull request as ready for review April 16, 2023 07:41
@dpogue dpogue mentioned this pull request Apr 16, 2023
3 tasks
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

Code Changes LGTM

@erisu erisu changed the title BREAKING -- feat!: Remove default build dir overrides feat!: remove default build dir overrides May 26, 2023
@erisu erisu merged commit 58df099 into apache:master May 28, 2023
6 checks passed
@dpogue dpogue deleted the dir-settings branch March 8, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants