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

NativeExecutableSpec should have default toolchains which target MacOS Intel and ARM #1096

Open
prbprbprb opened this issue Mar 29, 2022 · 0 comments · Fixed by gradle/gradle#20310

Comments

@prbprbprb
Copy link

prbprbprb commented Mar 29, 2022

Expected Behavior

Consider a trivial native project:

plugins {
    id 'cpp'
}

model {
    components {
        hello(NativeExecutableSpec) {
            targetPlatform "osx_x86-64"
            targetPlatform "osx_aarch64"

            sources {
                cpp {
                   source {
                     srcDir "src/cpp"
                     include "hello.cc"
                   }
                }
            }
        }
    }
}

Ideally this should build out of the box on either Intel or ARM based Macs with no further configuration.

Current Behavior

  1. Failure with Invalid NativePlatform: osx_aarch64
  2. After removing targetPlatform "osx_aarch64" then gradle components thinks it is targetting osx_x86-64 (expected) but on an ARM based Mac gradle build will actually build native ARM executables as no -arch flag is passed to Clang.

This can be overcome by adding the toolchain definitions manually (see attached project) but ideally shouldn't be necessary,
e.g.

    toolChains {
        clang(Clang) {
            target("osx_aarch64") {
                cppCompiler.withArguments { args ->
                    args << "-arch" << "arm64"
                }
     [... and so on for linker and Intel targets ...]

Context

Adding ARM support for another project: google/conscrypt#1034 - both the JNI libs and the generated source need local toolchain definitions to build at all on MacOS ARM.

In general it makes developing on an M1 based Mac more painful.

Steps to Reproduce (for bugs)

Simple project attached. hellonative.tar.gz

Will fail out of the box or work after uncommenting the toolchain definitions.

I also have a PR I am about to submit with a working fix for this.

Your Environment

Gradle 7.4.1 and locally built from source.

Also affects 6.x, and 6.x has the further issue that the target platform type for ARM-based Macs is inferred as osx_arm-v8 rather than osx_aarch64. [Update: looks like this is fixed in 6.9.1]

bot-gradle added a commit to gradle/gradle that referenced this issue Sep 9, 2022
Fixes [gradle-native #1096](gradle/gradle-native#1096)

Adds two new default toolchain targets for MacOS x86_64 and
arm64 which use Clang's `-arch` argument to target the correct
architecture.  On MacOS, this is used in preference to the
existing `Intel64Architecture` target which passes `-m64` to the
toolchain.

This is sufficient for `NativeExecutableSpec` to target
`osx_x86-64` and `osx_aarch64` on both Intel and M1 Macs.

I haven't (yet!) located any tests for this functionality, and
so this PR doesn't include any test changes.  Happy to add them
if you can point me in the right direction.

Signed-off-by: Pete Bentley <[email protected]>

<!--- The issue this PR addresses -->
Fixes [gradle-native #1096](gradle/gradle-native#1096)

### Context
Ease of use on both Intel and ARM based Macs.

### Contributor Checklist
- [x] [Review Contribution Guidelines](https://github.com/gradle/gradle/blob/master/CONTRIBUTING.md)
- [x] Make sure that all commits are [signed off](https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff) to indicate that you agree to the terms of [Developer Certificate of Origin](https://developercertificate.org/).
- [x] Make sure all contributed code can be distributed under the terms of the [Apache License 2.0](https://github.com/gradle/gradle/blob/master/LICENSE), e.g. the code was written by yourself or the original code is licensed under [a license compatible to Apache License 2.0](https://apache.org/legal/resolved.html).
- [x] Check ["Allow edit from maintainers" option](https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/) in pull request so that additional changes can be pushed by Gradle team
- [ ] Provide integration tests (under `<subproject>/src/integTest`) to verify changes from a user perspective
- [x] Provide unit tests (under `<subproject>/src/test`) to verify logic
- [x] Update User Guide, DSL Reference, and Javadoc for public-facing changes
- [x] Ensure that tests pass sanity check: `./gradlew sanityCheck`
- [x] Ensure that tests pass locally: `./gradlew <changed-subproject>:quickTest`

### Gradle Core Team Checklist
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation
- [ ] Recognize contributor in release notes

Co-authored-by: Pete Bentley <[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 a pull request may close this issue.

1 participant