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

Fix .ci.yaml validation for Fusion (the monorepo) #4137

Merged
merged 6 commits into from
Dec 26, 2024

Conversation

matanlurey
Copy link
Contributor

Closes flutter/flutter#160707.

Fixes a bug where DEPS was accidentally (or purposefully, there were no tests to verify) not checked for framework builds, and then expands on the validation by adding a check for engine/** for framework builds, adding a check nobody uses runIfNot, which is extremely broken and should not be used, and then lastly refactors the code and adds tests for the next shmuck to want to change this.

exceptions.add('ERROR: ${target.name} is missing `.ci.yaml` in runIf');
}

// 2. The engine repo must additionally depend on DEPS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please leave a TODO with a link to an issue reminding us to clean this up once we're ready to archive the engine repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

final ciYamlPath = switch (type) {
CiType.fusionEngine => 'engine/src/flutter/.ci.yaml',
_ => '.ci.yaml',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these be added automatically by the .ci.yaml parser (or by the logic that evaluates runIf conditions)? This way we don't have to specify anything in the yaml files. Seems like they add a bunch of noise. Totally fine we want to take this one step at a time though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App. label Dec 26, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Dec 26, 2024
Copy link
Contributor

auto-submit bot commented Dec 26, 2024

auto label is removed for flutter/cocoon/4137, due to - The status or check suite Linux Cocoon has failed. Please fix the issues identified (or deflake) before re-applying this label.

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App. label Dec 26, 2024
@auto-submit auto-submit bot merged commit cd0e4b1 into flutter:main Dec 26, 2024
4 checks passed
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 27, 2024
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 27, 2024
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Jan 7, 2025
Towards #160915 (future
cleanup: remove the validation from `flutter/cocoon`).

This PR effectively duplicates the logic in
flutter/cocoon#4137, in `flutter/flutter`.

/cc @jtmcdole
srujzs pushed a commit to srujzs/flutter that referenced this pull request Jan 12, 2025
…61249)

Towards flutter#160915 (future
cleanup: remove the validation from `flutter/cocoon`).

This PR effectively duplicates the logic in
flutter/cocoon#4137, in `flutter/flutter`.

/cc @jtmcdole
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a test to validate .ci.yaml builds that use runIf: do not omit DEPS and engine/**
2 participants