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

Use -target instead of -arch in compiling commands in Apple crosstool #13466

Conversation

thii
Copy link
Member

@thii thii commented May 11, 2021

This allows using this crosstool file for cross-compiling from non-Apple
host platforms. On macOS, -arch <arch> and
-m<platform>-version-min=<version_min> will be expanded to -triple <arch>-apple-<platform><version_min> in the frontend invocation, but on
a Linux host, without -target, the expanded triple flags would be
-triple <arch>-pc-linux-gnu.

Fixes #11986

@google-cla google-cla bot added the cla: yes label May 11, 2021
@thii thii force-pushed the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch from c090df3 to ae52523 Compare May 11, 2021 06:58
@thii thii marked this pull request as ready for review May 11, 2021 07:16
@keith
Copy link
Member

keith commented May 11, 2021

If we're passing target we can probably drop the arch arg? Also see #13448 which should make this necessary in fewer places

@thii
Copy link
Member Author

thii commented May 11, 2021

I'll rebase this after your PR is merged.

I've actually never seen the target being rewritten to aarch64-apple-darwin as mentioned in the linked commit, maybe it's only with older clang? If there's no problem I can drop the -arch argument.

@keith
Copy link
Member

keith commented May 11, 2021

hrm i wonder if that has something to do with the host OS? so maybe that's only a problem with clang on non macOS hosts?

@thii thii force-pushed the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch from ae52523 to da54989 Compare May 11, 2021 21:48
@thii thii changed the title Pass -target in addition to -arch in compiling commands in Apple crosstool Use -target instead of -arch in compiling commands in Apple crosstool May 11, 2021
@thii
Copy link
Member Author

thii commented May 11, 2021

Couldn't reproduce myself. Rebased and removed -arch.

Also it seems this will fix #11986.

@keith
Copy link
Member

keith commented May 11, 2021

looks like the failure is related

@thii
Copy link
Member Author

thii commented May 11, 2021

There's still some places in this file that can be simplified. I did a cleanup of it a while ago while implementing another toolchain, and the target system name can already be derived from cpu like:

def _target_system_name(cpu):
    platform, _, cpu = cpu.partition("_")
    if platform == "darwin":
        platform = "macosx"
    return "{}-apple-{}".format(cpu, platform)

https://github.com/apple-cross-toolchain/rules_applecross/blob/e59e4a02aa41b918b919d3711bb9c167b4474961/toolchain/cc_toolchain_config.bzl.tpl#L31-L35

@keith
Copy link
Member

keith commented May 11, 2021

doing some of that in this #13449

@keith
Copy link
Member

keith commented Feb 22, 2022

I think this is good to go, wdyt?

@thii thii force-pushed the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch from ccd22e1 to 2edf407 Compare February 23, 2022 12:51
@thii
Copy link
Member Author

thii commented Feb 23, 2022

I think we should land this. I would prefer the default crosstool to just work in most environments as much as it can.

Rebased and updated the target triple to also minimum OS version and target environment. With this, the version_min feature should be no longer needed, but we can clean it up in a follow-up.

@keith
Copy link
Member

keith commented Feb 23, 2022

will this usage

target_system_name = target_system_name,
be happy with that change? not sure how it gets consumed later.

@thii
Copy link
Member Author

thii commented Feb 24, 2022

It's what returned by cc_toolchain.target_gnu_system_name.

@thii thii marked this pull request as draft February 24, 2022 01:27
@thii thii force-pushed the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch from 2edf407 to cea78ff Compare February 24, 2022 05:09
@thii thii marked this pull request as ready for review February 24, 2022 05:54
crosstool

This allows using this crosstool file for cross-compiling from non-Apple
host platforms. On macOS, `-arch <arch>`  and
`-m<platform>-version-min=<version_min>` will be expanded to `-triple
<arch>-apple-<platform><version_min>` in the frontend invocation, but on
a Linux host, without `-target`, the expanded triple flags would be
`-triple <arch>-pc-linux-gnu`.
@thii thii force-pushed the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch from cea78ff to bdcdfac Compare February 24, 2022 06:25
@thii
Copy link
Member Author

thii commented Mar 29, 2022

Superseded by #14898.

@thii thii closed this Mar 29, 2022
@thii thii deleted the pass-target-in-addition-to-arch-in-compiling-commands-in-apple branch March 29, 2022 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing -target arg in cpp compilation for native cpp rules
3 participants