XcodeBuild: pass deployment target from profile #18496

kambala-decapitator · 2025-06-19T10:38:02Z

Changelog: Fix: Pass deployment target from profile to XcodeBuild.
Changelog: Fix: Project path and target name are quoted now for XcodeBuild.
Docs: conan-io/docs#4129

Refer to the issue that supports this Pull Request.
If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

memsharded

This might be doing too much in the same PR, specially when there are some potentially controversial things such as harcoding RUN_CLANG_STATIC_ANALYZER=NO in the command line.

Maybe it is better to split this PR, addressing first the bugfix, then the potential improvements in a separate one. Also, some test that covers the changes would be greatly appreciated, please ask us for support or guidance if you need, we might even contribute the tests ourselves.

conan/tools/apple/xcodebuild.py

memsharded · 2025-06-19T11:21:40Z

conan/tools/apple/xcodebuild.py

+                                self._sdkroot, self._verbosity, target)
+
+        deployment_target_key = xcodebuild_deployment_target_key(self._os)
+        if deployment_target_key and self._os_version:


This is probably the main and necessary fix?
I understand that if this is not provided, it will fail to build for iOS, watchOS, etc?

if it's not provided, the setting from the Xcode project will be used, which may be not what the library user desires. Since profile defines OS version, it's natural to use it.

memsharded · 2025-06-19T11:28:43Z

It also seems the PR broke some existing OSX-functional tests, please have a look https://github.com/conan-io/conan/actions/runs/15756189684/job/44412185234?pr=18496 (you an ignore the Windows-unit tests, that seems a temporary glitch).

kambala-decapitator · 2025-06-19T12:03:07Z

Maybe it is better to split this PR, addressing first the bugfix, then the potential improvements in a separate one. Also, some test that covers the changes would be greatly appreciated, please ask us for support or guidance if you need, we might even contribute the tests ourselves.

thanks, I fully agree with splitting. Are you fine with keeping the SYMROOT/OBJROOT change in this PR or it should also go to the other one?

memsharded · 2025-06-19T12:09:59Z

thanks, I fully agree with splitting. Are you fine with keeping the SYMROOT/OBJROOT change in this PR or it should also go to the other one?

You convinced me that if not defining this it might pollute the source directory when no_copy_source=True, then I am willing to consider this a bugfix and move forward.

But please wait a bit for the split, I am going to ask some colleague more knowledgeable than me in Apple for their feedback.

kambala-decapitator · 2025-07-13T08:05:02Z

FYI I'm heavily using changes from this PR quite successfully so far, would be great to get a review that I could proceed

memsharded

I still need to check with @jcar87 about this, but it would be nice to get the minor build_options=[] and the RUN_CLANG_STATIC_ANALYZER fixed in the meantime too.

memsharded · 2025-07-14T15:15:24Z

Also it seems there are a couple of builds broken, I haven't check them yet, but it would be necessary to have them green too.

memsharded

I have checked with the team, and the issue seems that this PR is being a bit confusing, doing different things:

adding new optional arguments, not fully clear all use cases
changing some command layout things with SYMROOT, OBJROOT. These seems should be defined at the project level, or in any case at the toolchain level. Because otherwise, normal flow like conan install + IDE/xcodebuild direct user invocation will do something different
The main fix which is passing the deployment target to the command line

I'd say to remove all the other things, and leave just the definition of the deployment target, which could be considered a bug fix if I understood correctly, if this is not passed, the conan build will not build the intended thing? That way we can move the PR forward, the other improvements can be discussed later in a different issue/PR, as they seem less priority

kambala-decapitator · 2025-07-17T06:46:27Z

Removed all the new features from the PR but added 2 small fixes - quoting project path and target name as they can have spaces.

changing some command layout things with SYMROOT, OBJROOT. These seems should be defined at the project level, or in any case at the toolchain level. Because otherwise, normal flow like conan install + IDE/xcodebuild direct user invocation will do something different

I can't really agree with this.

When building from Xcode IDE, those variables are ignored by default and Xcode places all the stuff in ~/Library/Developer/Xcode/DerivedData/<dir>. They will be used only when project is configured in such a way, but AFAIK it's rarely done nowadays. This is configured in File - Project settings... - Advanced - set to Legacy. This behavior was changed to Legacy like 10 years ago or even more.

Suppose we don't touch those settings. How can a recipe figure out where the build artifacts are? The only reliable way would be to invoke xcodebuild -showBuildSettings to see the actual value of all settings and grep SYMROOT. Do you really expect every recipe to include such a piece of code (or have a Conan helper function)? To me it looks like an unnecessary complication when you can set a reliable path at build time. And also polluting source directory by default which we have already discussed. And one more point here: as soon as I have redirected SYMROOT+OBJROOT, tests started failing because they rely on hardcoded default value, see kambala-decapitator@e538411

Will open a separate PR with all the new features.

memsharded

Looking good, thanks for doing the changes!

We can continue in other threads for the other features

memsharded · 2025-07-17T17:30:03Z

conan/tools/apple/xcodebuild.py

        """
-        target = "-target {}".format(target) if target else "-alltargets"
-        cmd = "xcodebuild -project {} -configuration {} -arch {} " \
+        target = "-target '{}'".format(target) if target else "-alltargets"


We usually quote with double quotes, it is a bit more cross-platform, though I understand this isn't cross-platform at all, so most likely not an issue. Wdyt @czoido?

I think it should be fine to use single quotes as this should never run on windows cmd

memsharded · 2025-07-17T17:31:30Z

I can't really agree with this.

It would be great to have the feedback of @jcar87 here, I am not a OSX user (it can be discussed in the new PR too)

kambala-decapitator · 2025-07-20T09:17:02Z

We can continue in other threads for the other features

opened #18668 with the new features

memsharded · 2025-07-20T09:27:00Z

opened #18668 with the new features

Thanks. I am aiming to include this PR in the next release, this split will certainly help in this.

czoido · 2025-07-21T13:17:37Z

Hi @kambala-decapitator,
I think this looks good. Thanks a lot for the contribution. I have added some basic tests over these changes.

pass deployment target based on profile to xcodebuild

4eac1e5

kambala-decapitator mentioned this pull request Jun 19, 2025

[XcodeBuild] update docs conan-io/docs#4129

Merged

memsharded reviewed Jun 19, 2025

View reviewed changes

memsharded requested a review from jcar87 June 19, 2025 12:10

memsharded added this to the 2.19.0 milestone Jul 2, 2025

memsharded reviewed Jul 14, 2025

View reviewed changes

memsharded requested a review from czoido July 14, 2025 15:21

memsharded added the type: look into label Jul 14, 2025

memsharded reviewed Jul 16, 2025

View reviewed changes

kambala-decapitator added 2 commits July 17, 2025 09:25

quote project path

6ec2454

quote target param

0ac6270

kambala-decapitator force-pushed the xcodebuild-extend branch from 9fa5da6 to 0ac6270 Compare July 17, 2025 06:26

memsharded approved these changes Jul 17, 2025

View reviewed changes

kambala-decapitator changed the title ~~XcodeBuild: extend build parameters~~ XcodeBuild: pass deployment target from profile Jul 20, 2025

kambala-decapitator mentioned this pull request Jul 20, 2025

XcodeBuild: extend build parameters #18668

Merged

5 tasks

memsharded removed the type: look into label Jul 20, 2025

memsharded assigned czoido Jul 20, 2025

add tests

6318357

czoido approved these changes Jul 21, 2025

View reviewed changes

memsharded merged commit bc8ad50 into conan-io:develop2 Jul 21, 2025
15 checks passed

kambala-decapitator deleted the xcodebuild-extend branch July 21, 2025 17:32

XcodeBuild: pass deployment target from profile #18496

XcodeBuild: pass deployment target from profile #18496

Uh oh!

Conversation

kambala-decapitator commented Jun 19, 2025 • edited by czoido Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

memsharded Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

kambala-decapitator Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

memsharded commented Jun 19, 2025

Uh oh!

kambala-decapitator commented Jun 19, 2025

Uh oh!

memsharded commented Jun 19, 2025

Uh oh!

kambala-decapitator commented Jul 13, 2025

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

memsharded commented Jul 14, 2025

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

kambala-decapitator commented Jul 17, 2025

Uh oh!

memsharded left a comment

Choose a reason for hiding this comment

Uh oh!

memsharded Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

czoido Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

memsharded commented Jul 17, 2025

Uh oh!

kambala-decapitator commented Jul 20, 2025

Uh oh!

memsharded commented Jul 20, 2025

Uh oh!

czoido commented Jul 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kambala-decapitator commented Jun 19, 2025 •

edited by czoido

Loading