Example of the linting working: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20analyze/73795/overview

 Found lint violations in Kotlin files:
║  20:13:09.867 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
║ /b/s/w/ir/x/w/flutter/examples/api/android/app/src/main/kotlin/dev/flutter/flutter_api_samples/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/tracing_tests/android/app/src/main/kotlin/com/example/tracing_tests/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/manual_tests/android/app/src/main/kotlin/dev/flutter/manual_tests/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/benchmarks/platform_channels_benchmarks/android/app/src/main/kotlin/com/example/platform_channels_benchmarks/MainActivity.kt:5:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/a11y_assessments/android/app/src/main/kotlin/com/example/a11y_assessments/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/integration_tests/deferred_components_test/android/app/src/main/kotlin/io/flutter/integration/deferred_components_test/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/integration_tests/android_embedding_v2_smoke_test/android/app/src/main/kotlin/com/example/android_embedding_v2_smoke_test/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/integration_tests/abstract_method_smoke_test/android/app/src/main/kotlin/com/example/abstract_method_smoke_test/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/integration_tests/spell_check/android/app/src/main/kotlin/com/example/sc_int_test/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ /b/s/w/ir/x/w/flutter/dev/integration_tests/non_nullable/android/app/src/main/kotlin/com/example/non_nullable/MainActivity.kt:1:9: Package name must not contain underscore (standard:package-name)
║ 
║ Summary error count (descending) by rule:
║   standard:package-name: 10

Just need to figure out

1. whether ktlint should live on CIPD, or continue to be downloaded, and
2. If the place where the baseline is is fine.

Baseline is also working, as the check passes now with it included

@reidbaker FYI this is the pr in the state we discussed getting it to, where the decision just needs to be made if the linting tool should be on CIPD, or downloaded (current state) like the packages repo https://github.com/flutter/packages/blob/abb04bb89cca97e653764a60bde0ee4e816db322/script/tool/lib/src/format_command.dart#L39

(mod making some of the paths/executability of the file platform agnostic)

Support for adding a dependency on ktlint just landed in https://flutter-review.googlesource.com/c/recipes/+/56760/, so this should be unblocked now. Just need to test to make sure its getting invoked properly, then clean up and get out of draft

cc @keyonghan (sorry for the continued questions on this!)
I'm trying to depend on ktlint in cipd now in this pr

dependencies: >-
        [
          {"dependency": "ktlint", "version": "version:version_1_1_1"}
        ]

and seeing an error that the version tag doesn't exist
https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8752187117714240737/+/u/Install_ktlint/ensure_installed/stdout
but I thought that I had created it correctly here
https://chrome-infra-packages.appspot.com/p/flutter/ktlint/linux-amd64/+/version_1_1_1

Did I perhaps mess something up in this step?

cc @keyonghan (sorry for the continued questions on this!) I'm trying to depend on ktlint in cipd now in this pr

dependencies: >-
        [
          {"dependency": "ktlint", "version": "version:version_1_1_1"}
        ]

and seeing an error that the version tag doesn't exist https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8752187117714240737/+/u/Install_ktlint/ensure_installed/stdout but I thought that I had created it correctly here https://chrome-infra-packages.appspot.com/p/flutter/ktlint/linux-amd64/+/version_1_1_1

Did I perhaps mess something up in this step?

You need a full tag name: version:version_1_1_1.

So instead of a reference, a tag may be more suitable for this case. (I need to update the doc again).

cipd set-ref flutter/ktlint/linux-amd64 -tag version:version_1_1_1 -version InstanceID

So instead of a reference, a tag may be more suitable for this case. (I need to update the doc again).

cipd set-ref flutter/ktlint/linux-amd64 -tag version:version_1_1_1 -version InstanceID

Hmm so I originally ran

cipd set-ref flutter/ktlint/linux-amd64 -ref version_1_1_1 -version cuHnxtuMe-YkvqnXhXeWPq3i_0TZv4ykCWmDQVxN7f8C

Changing ref to tag gives me an error that the flag doesn't exist

mackall-macbookpro:flutter_tools mackall$ cipd set-ref flutter/ktlint/linux-amd64 -tag=version:version_1_1_1 -version cuHnxtuMe-YkvqnXhXeWPq3i_0TZv4ykCWmDQVxN7f8C
flag provided but not defined: -tag
Moves a ref to point to a given version.

usage:  cipd set-ref <package or package prefix> [options]
  -cache-dir string
    	Directory for the shared cache (can also be set by CIPD_CACHE_DIR env var).
  -json-output path
    	A path to write operation results to.
  -log-level value
    	The logging level. Valid options are: debug, info, warning, error. (default info)
  -ref ref
    	A ref to point to the package instance (can be used multiple times).
  -service-account-json string
    	Path to JSON file with service account credentials to use. Or specify ":gce" to use GCE's default service account.
  -service-url string
    	Backend URL. If provided via an "ensure" file, the URL in the file takes precedence. (default https://chrome-infra-packages.appspot.com)
  -verbose
    	Enable more logging (deprecated, use -log-level=debug).
  -version string
    	Package version to point the ref to. (default "<version>")

I also assume that I'm not going to have permission currently either way, so I'll open another access request

Oh wait its probably supposed to be also set-ref -> set-tag, that works (at least up until permissions)

Okay nevermind, I was setting the version wrong 🤦. Sorry about that! Looks like the step is succeeding now https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20analyze/76227/overview

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

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

I'm pretty certain this does not affect g3 in any way (it only affects ci config, benchmarks, and a test file), so I'm going to override the google testing. I'm sorry if I was wrong 🙏

I'm pretty certain this does not affect g3 in any way (it only affects ci config, benchmarks, and a test file), so I'm going to override the google testing. I'm sorry if I was wrong 🙏

Post submit test passed https://critique.corp.google.com/cl/621639365

This new test is breaking on the Dart CI, where we run the flutter analysis test with tip-of-tree Dart SDK.
Whatever is added to the Flutter recipes to install klint also needs to be added to the Dart script at https://dart.googlesource.com/sdk/+/refs/heads/main/tools/bots/flutter/analyze_flutter_flutter.sh

Alternatively, if there is a command-line option or environment variable to set to disable the klint checks, or if these could be made a separate test script, rather than adding them into the existing analyze.dart test, which otherwise just requires a normal framework installation.

This new test is breaking on the Dart CI, where we run the flutter analysis test with tip-of-tree Dart SDK. Whatever is added to the Flutter recipes to install klint also needs to be added to the Dart script at https://dart.googlesource.com/sdk/+/refs/heads/main/tools/bots/flutter/analyze_flutter_flutter.sh

Alternatively, if there is a command-line option or environment variable to set to disable the klint checks, or if these could be made a separate test script, rather than adding them into the existing analyze.dart test, which otherwise just requires a normal framework installation.

I'm fine reverting this and moving the new portion of the test to a different check/shard.

We should also definitely add documentation noting that this check is copied and run in the dart ci as well. Are there other tests that also get copied? Is there a testowner that should be added for this check? I'm not particularly sure how one could know of or anticipate this problem.

Can I ask why you run this test in the dart ci, with the latest dart? It seems like a significant portion of the checks it makes are functionality I wouldn't expect to be run anywhere else, or to be particularly tied to dart versions (things like 'Links for creating GitHub issues', 'Unexpected binaries...', 'All tool test files end in _test.dart...', etc).

We only want to run any changes in the Dart analyzer and linter against the framework code, and this is the Flutter framework test that does that. We could extract just the relevant parts into commands run directly in our script, but then it has the danger of getting out-of-sync with what the flutter framework runs. But it does make sense to skip all of the non-relevant tests run by analyze.sh.

OK, looking at it, I agree that we should extract the analyzer tests out to a separate set, since there are so many non-relevant tests in this suite. Either this suite could be split into two parts, or some option added to only run the analysis tests (but also on gallery and code snippets), or we would have to clone all the analysis test code into our script, with the disadvantages of code clones. So I don't think you should revert.