[S] Change in dart/sdk[main]: [build] Make `build_analyze_snapshot` consistent between the host and...
Ryan Macnak (Gerrit)
Attention is currently required from: Daco Harkes.
1 comment:
File runtime/runtime_args.gni:
Patch Set #1, Line 95:
(target_os == "linux" || target_os == "android") &&is_android means current_os == "android"
"runtime" deps on "analyze_snapshot" and "analyze_snapshot($host_toolchain)"
the latter was not defined under the host toolchain because for it is_android is false
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Ryan Macnak (Gerrit)
Attention is currently required from: Daco Harkes.
Ryan Macnak would like Daco Harkes to review this change.
[build] Make `build_analyze_snapshot` consistent between the host and target toolchains.
Bug: https://github.com/dart-lang/sdk/issues/51242
Change-Id: I7a0ad1f0bc94c4593412945e4887f9045693fd40
---
M runtime/runtime_args.gni
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/runtime/runtime_args.gni b/runtime/runtime_args.gni
index 0c4f1ea..b3eac69 100644
--- a/runtime/runtime_args.gni
+++ b/runtime/runtime_args.gni
@@ -92,6 +92,6 @@
# The analyze_snapshot tool is only supported on 64 bit AOT builds running
# under linux and android platforms
build_analyze_snapshot =
- (is_linux || is_android) &&
+ (target_os == "linux" || target_os == "android") &&
(dart_target_arch == "x64" || dart_target_arch == "arm64")
}
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Daco Harkes (Gerrit)
Attention is currently required from: Ryan Macnak.
Patch set 1:Code-Review +1
2 comments:
Commit Message:
Patch Set #1, Line 7:
[build] Make `build_analyze_snapshot` consistent between the host and target toolchains.Should we consider running the build on an M1 bot so that we ensure we don't break the build again?
Patchset:
Thank you Ryan!
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Ryan Macnak (Gerrit)
Attention is currently required from: Daco Harkes.
Patch set 1:Commit-Queue +2
1 comment:
Commit Message:
Patch Set #1, Line 7:
[build] Make `build_analyze_snapshot` consistent between the host and target toolchains.Should we consider running the build on an M1 bot so that we ensure we don't break the build again?
(In this case both x64 and ARM64 Macs were affected.)
I don't think there are enough Android devices for a full mac-host-android-target bot, but maybe a build-only bot? We don't have anything for windows-host-android-target either.
And now that I think about it, we're still using an NDK that requires Rosetta for ARM64 Mac.
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Daco Harkes (Gerrit)
Attention is currently required from: Ryan Macnak.
1 comment:
Commit Message:
Patch Set #1, Line 7:
[build] Make `build_analyze_snapshot` consistent between the host and target toolchains.
(In this case both x64 and ARM64 Macs were affected.) […]
I believe we have plenty of cycles on the M1 bots, they always run immediately when I add them to my CQ on CLs. (The Windows ones I'm not so sure about.)
We would definitely catch these build configurations errors with a build-only bot 👍
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Commit Queue (Gerrit)
Commit Queue submitted this change.
[build] Make `build_analyze_snapshot` consistent between the host and target toolchains.
Bug: https://github.com/dart-lang/sdk/issues/51242
Change-Id: I7a0ad1f0bc94c4593412945e4887f9045693fd40
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280782
Reviewed-by: Daco Harkes <dacoh...@google.com>
Commit-Queue: Ryan Macnak <rma...@google.com>
---
M runtime/runtime_args.gni
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/runtime/runtime_args.gni b/runtime/runtime_args.gni
index 0c4f1ea..b3eac69 100644
--- a/runtime/runtime_args.gni
+++ b/runtime/runtime_args.gni
@@ -92,6 +92,6 @@
# The analyze_snapshot tool is only supported on 64 bit AOT builds running
# under linux and android platforms
build_analyze_snapshot =
- (is_linux || is_android) &&
+ (target_os == "linux" || target_os == "android") &&
(dart_target_arch == "x64" || dart_target_arch == "arm64")
}
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
CBuild (Gerrit)
go/dart-cbuild result: SUCCESS
Details: https://goto.google.com/dart-cbuild/find/881825980e7b5217a7c8cbd219970a42eb644db9
William Hesse (Gerrit)
1 comment:
Patchset:
This change seems to have broken the Fuchsia build, because the analyze_snapshot target is no longer present in the Fuchsia build.
//third_party/dart/runtime/bin:analyze_snapshot(//build/toolchain/linux:clang_x64)
is needed by
//flutter/lib/snapshot:generate_snapshot_bins(//build/toolchain/fuchsia:fuchsia)
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
William Hesse (Gerrit)
1 comment:
Patchset:
This change seems to have broken the Fuchsia build, because the analyze_snapshot target is no longer […]
The Fuchsia builder is failing in the gn step, to generate the build files. See the HHH builder https://ci.chromium.org/p/dart/builders/ci.sandbox/flutter-engine-linux.
Flutter has requested a revert, will do so after checking nothing depending on this has landed.
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
William Hesse (Gerrit)
William Hesse has created a revert of this change.
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.
Ryan Macnak (Gerrit)
1 comment:
Patchset:
Thanks, Bill.
Reland that also handles Fuchsia at https://dart-review.googlesource.com/c/sdk/+/281460
To view, visit change 280782. To unsubscribe, or for help writing mail filters, visit settings.