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

getBuildProperty: get from default or specific build of first target #126

Closed
wants to merge 1 commit into from
Closed

getBuildProperty: get from default or specific build of first target #126

wants to merge 1 commit into from

Conversation

ZauberNerd
Copy link

@ZauberNerd ZauberNerd commented Feb 21, 2017

Before this change the getBuildProperty method would iterate over all
targets and their build configurations without an early exit when the
property was found - that sometimes lead to having the value of a build
property from a later project.

After this change the getBuildProperty method will search through all
buildConfigurations (or a single one, if specified) of the first project
target and return as soon as the requested property has been found.

This PR can also be used as basis for follow-up PRs, for example: The
productName getter can use this internally to get the PRODUCT_NAME
of the correct target. (which in turn would fix: #91 / #99)

This PR would probably make this workaround in react-native obsolete.

@ZauberNerd
Copy link
Author

ZauberNerd commented Feb 21, 2017

I'm not too well versed in the .pbxproj file format, but is the assumption correct, that the XCConfigurationList of the "first target" is always the same as the XCConfigurationList of the PBXProject that the rootObject points to?
And would it make sense then (in a follow-up PR) to use the rootObject instead of the first target throughout the entire project?
Nevermind - after inspecting the file a bit more I figured out, that there are build configuration lists for native targets and for the project itself.

@ZauberNerd
Copy link
Author

ZauberNerd commented Feb 21, 2017

After thinking about this a bit more - I think this change doesn't make much sense. It would need the ability to select which target it should read instead of always using the first target.
So the API of getBuildProperty would need to have another parameter for the target:

// option 1 - would be a breaking API change
project.getBuildProperty(property: string, target: uuid, build: string) => string|void;

// option 2 - feels a bit unnatural in my opinion, because target is a parent of build
project.getBuildProperty(property: string, build: string, target: uuid) => string|void;

// option 3 - would be in line with other method signatures, but feels clumsy to have an options object
// for just one property
project.getBuildProperty(property: string, build: string, opts: { target: uuid }) => string|void;

@alunny @imhotep what are your thoughts on this?

Before this change the `getBuildProperty` method would iterate over all
targets and their build configurations without an early exit when the
property was found - that sometimes lead to having the value of a build
property from a later project.

After this change the `getBuildProperty` method will search through all
buildConfigurations (or a single one, if specified) of the first project
target and return as soon as the requested property has been found.

This PR can also be used as basis for follow-up PRs, for example: The
`productName` getter can use this internally to get the `PRODUCT_NAME`
of the correct target. (which in turn would fix: #91 / #99)

This PR would probably make [this workaround in react-native](https://github.com/facebook/react-native/blob/0af640b/local-cli/link/ios/getBuildProperty.js#L4) obsolete.
@ZauberNerd ZauberNerd closed this Jun 21, 2024
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.

addStaticLibrary problem
1 participant