Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

zanderso
Copy link
Member

@zanderso zanderso commented Mar 16, 2024

Part 1 of flutter/flutter#145263

This PR updates the names of builds outside of local_engine.json to be prefixed with the string ci/ (or ci\ on Windows). For better or worse, the "name" field of a build is used to construct a path used as the source directory of a copy operation (I think the CAS archive step?). Because of that, changing the name of a build also requires updating the build output directory of the ninja build.

This PR also adds tests to make sure the naming of these builds remains consistent.

@zanderso zanderso force-pushed the namespace-ci-builds branch 7 times, most recently from 9f33041 to 4f078ae Compare March 17, 2024 17:13
Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

No recipes change should be expected. LGTM for configs change.

@johnmccutchan
Copy link
Contributor

I'm fine with this as a temporary measure but I'd like to understand the plan to get us into a place where there is only one definition of each build config (aka out directory).

@zanderso
Copy link
Member Author

I'm fine with this as a temporary measure but I'd like to understand the plan to get us into a place where there is only one definition of each build config (aka out directory).

There are a few different ways to accomplish that that we'll need to discuss. I would like to consider approaches where the exact contents and layout of out/ is an implementation detail of et and CI, and in which human readable descriptions of the build configs provide a good guide for which config a developer needs at any time. (If you need to know the fine-grained details, that could be an et query subcommand.) As an example, if we choose to use the GN toolchain feature to build multiple configurations as part of the same ninja build as I've heard a few folks discuss, then the output for some build configurations would shift from the top-level out/ directory into a subdirectory 2-3 layers deep. The user shouldn't have to worry about that.

I think maybe the spirit of your question is not exactly about the out/ directory, though, but more about having a one-to-one mapping between configurations and names. If/when we design a configuration language for generating the .json files instead of hand-coding them, we should ensure that we get that one-to-one property by construction. But exactly how to do that, I don't have a detailed design for.

@zanderso zanderso force-pushed the namespace-ci-builds branch 3 times, most recently from 9f7136a to 2ce8e6f Compare March 24, 2024 01:11
zanderso added a commit to flutter/buildroot that referenced this pull request Mar 26, 2024
…841)

The relative path in the config file is incorrect if the working
directory is not directly nested under `out/`, as will happen with
flutter/engine#51474.
@zanderso zanderso force-pushed the namespace-ci-builds branch 4 times, most recently from 9f3fc0d to b428642 Compare March 31, 2024 17:56
@zanderso zanderso force-pushed the namespace-ci-builds branch from b428642 to b11d69a Compare March 31, 2024 18:08
@zanderso zanderso merged commit 4f6b832 into flutter:main Mar 31, 2024
@zanderso zanderso deleted the namespace-ci-builds branch March 31, 2024 19:43
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 31, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 31, 2024
…146055)

flutter/engine@9689390...4f6b832

2024-03-31 [email protected] Prefix non-local build config names with ci/ (flutter/engine#51474)

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

To file a bug in Flutter: 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants