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

(#659) Remove CONFIGURATION_BUILD_DIR default string #671

Closed
wants to merge 1 commit into from

Conversation

ArcTanSusan
Copy link
Contributor

@ArcTanSusan ArcTanSusan commented Sep 19, 2019

###Platforms affected

  • all

Motivation and Context

#659

Description

Remove CONFIGURATION_BUILD_DIR default string. Use the user custom defined CONFIGURATION_BUILD_DIR as the default and if undefined, then the value is an empty string.

Testing

Modified unittests to no longer have a default hard-coded CONFIGURATION_BUILD_DIR value.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • 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)
  • I've updated the documentation if necessary

Use the user custom defined CONFIGURATION_BUILD_DIR as the default and if
undefined, then the value is an empty string.
Copy link
Contributor

@brodycj brodycj left a comment

Choose a reason for hiding this comment

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

Please accept my apologies for lack of review feedback in the past. This definitely looks like a major, breaking change which needs to be described so that we can evaluate whether we can accept the breaking change or consider some other alternatives. Testing seems to be broken and there is now a conflict.

@dpogue dpogue added this to the 7.0.0 milestone Jun 18, 2020
dpogue added a commit to dpogue/cordova-ios that referenced this pull request Apr 16, 2023
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 <[email protected]>
dpogue added a commit to dpogue/cordova-ios that referenced this pull request Apr 16, 2023
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 <[email protected]>
dpogue added a commit to dpogue/cordova-ios that referenced this pull request Apr 16, 2023
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 <[email protected]>
dpogue added a commit to dpogue/cordova-ios that referenced this pull request Apr 16, 2023
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 <[email protected]>
@erisu erisu closed this in #1310 May 28, 2023
erisu pushed a commit that referenced this pull request May 28, 2023
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 #617
Closes #659
Closes #671

Co-authored-by: Susan Tan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants