Skip to content

Conversation

@GregoryConrad
Copy link
Contributor

@GregoryConrad GregoryConrad commented Sep 14, 2025

Sometimes the necessary binaries code_assets expects alongside clang++ have differing names than the current code expects (e.g., ld instead of ld.lld) on Linux. This change is necessary on some systems with atypical installs, such as NixOS.

These binaries are gathered by the flutter tool and are eventually sent to the build hook for native code assets as the CCompilerConfig.

Note that the only other reference to clang/clang++ in the linux build system is where it is used to invoke cmake, where they are set as the values of the CC/CXX environment variables.

Fixes #175311

CC @dcharkes, this is take two of #175312

This PR also fixes my CI using only the llvmPackages_20.clangUseLLVM Nix package, for reference

Pre-launch Checklist

@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Sep 14, 2025
@GregoryConrad GregoryConrad marked this pull request as ready for review September 14, 2025 15:45
Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes the necessary code_assets binaries alongside clang++ have differing names than the current code expects.

Nit: Maybe be explicit about "CCompilerConfig binaries in package:code_assets passed into build hooks". (The flutter tools passes in the config, but the code assets package defines what the possible config is.)

(Also the PR title could be something like [native_assets] Find more CCompilerConfig on Linux.)

This change is necessary on some systems with atypical installs, such as NixOS.

Maybe repeat precisely what is different. The explicit executable names.

This PR also fixes my CI using only the llvmPackages_20.clangUseLLVM Nix package, for reference

Sweet! 👍

One question that should be answered in the PR description is:

  • Where in flutter tools is clang used? And does flutter tools use only clang or does it also use ld and ar? And if so, is it consistent in all places. (I think it's basically only clang and clang++ from PATH: packages/flutter_tools/lib/src/linux/build_linux.dart.)

Maybe add comments there and here to say that these should be kept consistent.

const kArBinary = 'llvm-ar';
const kLdBinary = 'ld.lld';
const kClangBinaryOptions = ['clang'];
const kArBinaryOptions = ['llvm-ar', 'ar'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add comments what is the default name, and where the alternative names occur.

Copy link
Contributor Author

@GregoryConrad GregoryConrad Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I:

  • Updated PR title
  • Updated PR description
  • Added code comment here to address concern

@dcharkes @bkonyi can I please get a re-review (and if all looks good, queue this to merge?)

@GregoryConrad GregoryConrad changed the title Search for additional code_assets exec dependencies [native_assets] Find more CCompilerConfig on Linux Sep 15, 2025
@dcharkes
Copy link
Contributor

One question that should be answered in the PR description is:

  • Where in flutter tools is clang used? And does flutter tools use only clang or does it also use ld and ar? And if so, is it consistent in all places. (I think it's basically only clang and clang++ from PATH: packages/flutter_tools/lib/src/linux/build_linux.dart.)

Maybe add comments there and here to say that these should be kept consistent.

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the comment of how flutter_tools uses clang and friends outside of native assets is answered and cross-linked with comments so that it stays consistent.

@GregoryConrad
Copy link
Contributor Author

LGTM once the comment of how flutter_tools uses clang and friends outside of native assets is answered and cross-linked with comments so that it stays consistent.

Ahh, gotcha now--will add another commit for that later today/tomorrow

@GregoryConrad
Copy link
Contributor Author

GregoryConrad commented Sep 16, 2025

@dcharkes the only relevant reference I could find to building with clang/clang++ was in the file you mentioned, but the reference is just here where CC/CXX environment variable are set to clang/clang++:

result = await globals.processUtils.stream(
<String>[
'cmake',
'-G',
'Ninja',
'-DCMAKE_BUILD_TYPE=$buildFlag',
'-DFLUTTER_TARGET_PLATFORM=${getNameForTargetPlatform(targetPlatform)}',
// Support cross-building for arm64 targets on x64 hosts.
// (Cross-building for x64 on arm64 hosts isn't supported now.)
if (needCrossBuild) '-DFLUTTER_TARGET_PLATFORM_SYSROOT=$targetSysroot',
if (needCrossBuildOptionsForArm64) '-DCMAKE_C_COMPILER_TARGET=aarch64-linux-gnu',
if (needCrossBuildOptionsForArm64) '-DCMAKE_CXX_COMPILER_TARGET=aarch64-linux-gnu',
sourceDir.path,
],
workingDirectory: buildDir.path,
environment: <String, String>{'CC': 'clang', 'CXX': 'clang++'},
trace: true,
);
if (result != 0) {
throwToolExit('Unable to generate build files');
}

That doesn't seem like it needs a comment linking it to where it's used in Native Assets, as it won't get out of sync? Also didn't see any references to ld or ar at first glance (for Linux).

@dcharkes dcharkes added this pull request to the merge queue Sep 16, 2025
Merged via the queue into flutter:master with commit bbb2362 Sep 16, 2025
146 checks passed
@GregoryConrad GregoryConrad deleted the native-assets-exec branch September 16, 2025 12:30
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 16, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 16, 2025
flutter/flutter@29a238d...8d0b31d

2025-09-16 [email protected] Roll Packages from fcd5f68 to 0255ac9 (2 revisions) (flutter/flutter#175427)
2025-09-16 [email protected] Adds a11y section locale support for iOS (flutter/flutter#175005)
2025-09-16 [email protected] Roll Skia from 4e9c86d4a6d9 to 7d160bbf9403 (3 revisions) (flutter/flutter#175404)
2025-09-16 [email protected] [native_assets] Find more `CCompilerConfig` on Linux (flutter/flutter#175323)
2025-09-16 [email protected] Roll Dart SDK from 50e61e5bff51 to 700de52f29a9 (3 revisions) (flutter/flutter#175395)
2025-09-16 [email protected] Fix default overlay color in `TabBar` (flutter/flutter#175270)
2025-09-16 [email protected] Migrate to widget state (flutter/flutter#175242)
2025-09-16 [email protected] Roll Skia from 01b0ede33ae9 to 4e9c86d4a6d9 (1 revision) (flutter/flutter#175387)
2025-09-15 [email protected] Merge the engine README into the README of the old buildroot. (flutter/flutter#175384)
2025-09-15 [email protected] Marks Mac_ios microbenchmarks_ios to be unflaky (flutter/flutter#171146)
2025-09-15 [email protected] Deprecate Objective-C plugin template (flutter/flutter#174003)
2025-09-15 [email protected] Add a gn --ccache argument (flutter/flutter#174621)
2025-09-15 [email protected] Update `build.gradle` to remove deprecation warning in `flutter\engine\src\flutter\shell\platform\android` (flutter/flutter#175305)
2025-09-15 [email protected] Show cursor after swipe only if TextField has focus (flutter/flutter#175044)
2025-09-15 [email protected] Roll Skia from f950263bb3d4 to 01b0ede33ae9 (7 revisions) (flutter/flutter#175373)
2025-09-15 [email protected] Update Chromium sysroot to pick up RISC-V support. (flutter/flutter#173671)
2025-09-15 [email protected] Set Gemini Code Assist `include_drafts` to false (flutter/flutter#175098)
2025-09-15 [email protected] Roll Packages from 15e7e89 to fcd5f68 (3 revisions) (flutter/flutter#175366)
2025-09-15 [email protected] Remove 'v' Open DevTools from help on web in profile/release mode (flutter/flutter#172829)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
dixita0607 pushed a commit to dixita0607/flutter that referenced this pull request Sep 17, 2025
Sometimes the necessary binaries `code_assets` expects alongside
`clang++` have differing names than the current code expects (e.g., `ld`
instead of `ld.lld`) on Linux. This change is necessary on some systems
with atypical installs, such as NixOS.

These binaries are gathered by the flutter tool and are eventually sent
to the build hook for native code assets as the `CCompilerConfig`.

Note that the only other reference to `clang`/`clang++` in the linux
build system is where it is used to invoke `cmake`, where they are set
as the values of the `CC`/`CXX` environment variables.

Fixes flutter#175311

CC @dcharkes, this is take two of flutter#175312

> This PR also fixes my CI using only the `llvmPackages_20.clangUseLLVM`
Nix package, for reference

## Pre-launch Checklist

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
Sometimes the necessary binaries `code_assets` expects alongside
`clang++` have differing names than the current code expects (e.g., `ld`
instead of `ld.lld`) on Linux. This change is necessary on some systems
with atypical installs, such as NixOS.

These binaries are gathered by the flutter tool and are eventually sent
to the build hook for native code assets as the `CCompilerConfig`.

Note that the only other reference to `clang`/`clang++` in the linux
build system is where it is used to invoke `cmake`, where they are set
as the values of the `CC`/`CXX` environment variables.

Fixes flutter#175311

CC @dcharkes, this is take two of flutter#175312

> This PR also fixes my CI using only the `llvmPackages_20.clangUseLLVM`
Nix package, for reference

## Pre-launch Checklist

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
Sometimes the necessary binaries `code_assets` expects alongside
`clang++` have differing names than the current code expects (e.g., `ld`
instead of `ld.lld`) on Linux. This change is necessary on some systems
with atypical installs, such as NixOS.

These binaries are gathered by the flutter tool and are eventually sent
to the build hook for native code assets as the `CCompilerConfig`.

Note that the only other reference to `clang`/`clang++` in the linux
build system is where it is used to invoke `cmake`, where they are set
as the values of the `CC`/`CXX` environment variables.

Fixes flutter#175311

CC @dcharkes, this is take two of flutter#175312

> This PR also fixes my CI using only the `llvmPackages_20.clangUseLLVM`
Nix package, for reference

## Pre-launch Checklist

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
Sometimes the necessary binaries `code_assets` expects alongside
`clang++` have differing names than the current code expects (e.g., `ld`
instead of `ld.lld`) on Linux. This change is necessary on some systems
with atypical installs, such as NixOS.

These binaries are gathered by the flutter tool and are eventually sent
to the build hook for native code assets as the `CCompilerConfig`.

Note that the only other reference to `clang`/`clang++` in the linux
build system is where it is used to invoke `cmake`, where they are set
as the values of the `CC`/`CXX` environment variables.

Fixes flutter#175311

CC @dcharkes, this is take two of flutter#175312

> This PR also fixes my CI using only the `llvmPackages_20.clangUseLLVM`
Nix package, for reference

## Pre-launch Checklist

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C Compiler dependencies for code_assets cannot always be found on all Linux systems in current implementation

3 participants