Specify architecture when building for macOS #148920

zhibisora · 2024-05-22T23:28:34Z

use

final String arch = globals.platform.version.contains('arm64') ? 'arm64' : globals.platform.version.contains('x86_64') ? 'x86_64' : '';
final String destination = 'platform=macOS${arch.isEmpty?'':',arch=$arch'}';

and

'-destination', destination,

make Xcode know which device to choose for the build.

This will not affect the architecture version of the build result, the actual build version is still determined by the xcode settings. This is simply a more precise specification of the build target to minimize Xcode complaints.

If there is no matching architectural information in the version, the code behavior falls back to what it was before this pr.

fixes #86590

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

flutter-dashboard · 2024-05-22T23:28:38Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

google-cla · 2024-05-22T23:28:38Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

andrewkolos · 2024-05-30T20:26:09Z

@cbracken is this something you could review?

andrewkolos · 2024-06-13T20:13:34Z

@cbracken is this something you could review?

FYI: Chris will likely be out of action for the next 10 days or so.

jmagman · 2024-06-24T17:56:43Z

packages/flutter_tools/lib/src/macos/build_macos.dart

    throwToolExit('Unable to find expected configuration in Xcode project.');
  }
+  // Specifying the architecture makes the destination clear to Xcode
+  final String arch = globals.platform.version.contains('arm64') ? 'arm64' : globals.platform.version.contains('x86_64') ? 'x86_64' : '';


Sorry for the delay on this review.
Instead of string parsing, you should check OperatingSystemUtils.hostPlatform:

Suggested change

final String arch = globals.platform.version.contains('arm64') ? 'arm64' : globals.platform.version.contains('x86_64') ? 'x86_64' : '';

final String arch = globals.os.hostPlatform == HostPlatform.darwin_arm64 ? 'arm64' : 'x86_64';

What would the empty string fallback do?

The empty string fallback is just in case it's not fetched, it does nothing and keeps the behaviour before this pr.

Using an empty string also allows the old tests to pass normally. If I switch to using globals.os.hostPlatform, I might need to modify the existing tests, as the destination parameter is hard-coded in the current tests. I'm not sure if this is an acceptable practice.

In build_macos_test.dart on line 127, '-destination', 'platform=macOS' is written as a string. I might need to modify it to pass this PR.

cbracken · 2024-07-11T17:53:06Z

@zhibisora are you still working on this?

zhibisora · 2024-07-11T21:57:30Z

@zhibisora are you still working on this?

Yes, I had some other things to handle before, but I will continue working on it from now on.

cbracken · 2024-07-16T22:02:42Z

Right on. Give us a shout when this is passing tests and ready for review! Thanks!

jmagman · 2024-07-17T20:09:21Z

Example failure:

[2024-07-11 15:55:28.438858] [STDOUT] run:stderr: 2024-07-11 15:55:28.435 xcodebuild[19672:128982] Writing error result bundle to /var/folders/cs/cqkgmb4n27n6k5bhvx0_34d00000gp/T/ResultBundle_2024-11-07_15-55-0028.xcresult
[2024-07-11 15:55:28.470962] [STDOUT] run:stderr: xcodebuild: error: unreadable input ' arch=arm64' at end of value for option 'Destination'

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8742668911423530689/+/u/run_run_debug_test_macos/stdout

jmagman · 2024-07-25T23:43:03Z

Heads up, after #152347 you'll be able to grab the ARCH build settings before the actual compilation happens.

LouiseHsu · 2024-08-07T21:14:34Z

Are you still working on this? Would you like any help?
Also heads up #152347 has landed :)

cbracken · 2024-08-14T20:54:28Z

Converting to draft for the time being. @zhibisora is this still something you're working on?

flutter-dashboard · 2024-08-14T20:54:59Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

jmagman · 2024-08-21T21:26:43Z

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to submit a new PR if you have the time to address the review comments. Thanks!

@zhibisora

@zhibisora

github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop framework flutter/packages/flutter repository. See also f: labels. labels May 22, 2024

zhibisora force-pushed the specify_arch_macos branch from d3d4bd0 to 40c323a Compare May 23, 2024 21:10

github-actions bot removed the framework flutter/packages/flutter repository. See also f: labels. label May 23, 2024

loic-sharma added the platform-mac Building on or for macOS specifically label Jun 24, 2024

jmagman reviewed Jun 24, 2024

View reviewed changes

zhibisora force-pushed the specify_arch_macos branch from 40c323a to 4f06fef Compare July 11, 2024 22:15

zhibisora added 8 commits July 12, 2024 06:48

Specify architecture when building for macOS

6826c92

add test

d0f8140

small change

cad4ebe

more tests

2c8d4e1

use stronger way to impl, fix tests and add tests

832b08b

use hostPlatform instead

1137c10

some fix

a4aea79

change tests

0531557

zhibisora force-pushed the specify_arch_macos branch from 312e981 to 0531557 Compare July 11, 2024 22:49

cbracken marked this pull request as draft August 14, 2024 20:53

jmagman closed this Aug 21, 2024

Sameri11 mentioned this pull request Mar 31, 2025

Fix warning when building for macOS desktop #165996

Merged

9 tasks

	final String arch = globals.platform.version.contains('arm64') ? 'arm64' : globals.platform.version.contains('x86_64') ? 'x86_64' : '';
	final String arch = globals.os.hostPlatform == HostPlatform.darwin_arm64 ? 'arm64' : 'x86_64';

Specify architecture when building for macOS #148920

Specify architecture when building for macOS #148920

Uh oh!

Conversation

zhibisora commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented May 22, 2024

Uh oh!

google-cla bot commented May 22, 2024

Uh oh!

andrewkolos commented May 30, 2024

Uh oh!

andrewkolos commented Jun 13, 2024

Uh oh!

jmagman Jun 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhibisora Jul 11, 2024

Choose a reason for hiding this comment

Uh oh!

zhibisora Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbracken commented Jul 11, 2024

Uh oh!

zhibisora commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbracken commented Jul 16, 2024

Uh oh!

jmagman commented Jul 17, 2024

Uh oh!

jmagman commented Jul 25, 2024

Uh oh!

LouiseHsu commented Aug 7, 2024

Uh oh!

cbracken commented Aug 14, 2024

Uh oh!

flutter-dashboard bot commented Aug 14, 2024

Uh oh!

jmagman commented Aug 21, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

zhibisora commented May 22, 2024 •

edited

Loading

jmagman Jun 24, 2024 •

edited

Loading

zhibisora Jul 11, 2024 •

edited

Loading

zhibisora commented Jul 11, 2024 •

edited

Loading