Buildifier configuration? #1080
Assignees
Comments
|
Which Buildifier version did you use on your machine? |
|
I'm installing via % buildifier -mode=diff "test/starlark_tests/ios_dynamic_framework_tests.bzl"
--- test/starlark_tests/ios_dynamic_framework_tests.bzl 2021-01-19 09:22:00.700099519 -0800
+++ /var/folders/h6/6btvg47n7y1f5xxz4_n9rsd40000gn/T/buildifier-tmp-881139563 2021-01-19 09:22:52.394393220 -0800
@@ -43,11 +43,11 @@
"$BUNDLE_ROOT/Info.plist",
"$BUNDLE_ROOT/Modules/module.modulemap",
"$BUNDLE_ROOT/Modules/BasicFramework.swiftmodule/x86_64.swiftdoc",
- "$BUNDLE_ROOT/Modules/BasicFramework.swiftmodule/x86_64.swiftmodule"
+ "$BUNDLE_ROOT/Modules/BasicFramework.swiftmodule/x86_64.swiftmodule",
],
tags = [name],
)
-
+
infoplist_contents_test(
name = "{}_plist_test".format(name),
target_under_test = "//test/starlark_tests/targets_under_test/ios:basic_framework",
@@ -84,7 +84,7 @@
"$BUNDLE_ROOT/Info.plist",
"$BUNDLE_ROOT/Modules/module.modulemap",
"$BUNDLE_ROOT/Modules/DirectDependencyTest.swiftmodule/x86_64.swiftdoc",
- "$BUNDLE_ROOT/Modules/DirectDependencyTest.swiftmodule/x86_64.swiftmodule"
+ "$BUNDLE_ROOT/Modules/DirectDependencyTest.swiftmodule/x86_64.swiftmodule",
],
tags = [name],
)
@@ -102,7 +102,7 @@
"$BUNDLE_ROOT/Info.plist",
"$BUNDLE_ROOT/Modules/module.modulemap",
"$BUNDLE_ROOT/Modules/TransitiveDependencyTest.swiftmodule/x86_64.swiftdoc",
- "$BUNDLE_ROOT/Modules/TransitiveDependencyTest.swiftmodule/x86_64.swiftmodule"
+ "$BUNDLE_ROOT/Modules/TransitiveDependencyTest.swiftmodule/x86_64.swiftmodule",
],
tags = [name],
)
exit status 1 |
|
Another report about Buildifier behaving badly:
|
|
Another link: bazelbuild/rules_rust@7579c34 |
|
Any progress? We regularly miss buildifier lints in PRs and end up having |
fweikert
added a commit
to fweikert/continuous-integration
that referenced
this issue
Mar 31, 2021
There have been many problems with Buildifier on Bazel CI, mostly stemming from the fact that Buildifier has evolved a lot, but our CI support hasn't kept up.
Ideally there would have been many smaller commits over time, but well...
This commit implements the following changes:
1. The code now uses a single Buildifier invocation for linting and format checks.
2. We now call Buildifier with --format=json, thus removing the need for error-prone parsing of unstructured text.
3. The old code checked stderr for Buildifier warnings, but at some point Buildifier started printing them to stdout only.
4. Finally we no longer determine the list of source files that should be passed to Buildifier, but simply use Buildifier's recursive mode ("-r .").
Progress towards bazelbuild#1080
|
@hlopko Sorry for the delay. There were so many problems that a significant rewrite was needed. |
|
No worries, thank you for your work!! |
fweikert
added a commit
that referenced
this issue
Apr 1, 2021
There have been many problems with Buildifier on Bazel CI, mostly stemming from the fact that Buildifier has evolved a lot, but our CI support hasn't kept up.
Ideally there would have been many smaller commits over time, but well...
This commit implements the following changes:
1. The code now uses a single Buildifier invocation for linting and format checks.
2. We now call Buildifier with --format=json, thus removing the need for error-prone parsing of unstructured text.
3. The old code checked stderr for Buildifier warnings, but at some point Buildifier started printing them to stdout only.
4. Finally we no longer determine the list of source files that should be passed to Buildifier, but simply use Buildifier's recursive mode ("-r .").
Progress towards #1080
fweikert
added a commit
to fweikert/continuous-integration
that referenced
this issue
Apr 1, 2021
Previously CI used the Buildifier binary from the Docker container if the user did not request a specific version. However, since we forgot to update the Docker container, it still contained an ancient version (0.22.0 - for comparison, the most recent release is 4.0.1). With this commit CI will always download a Buildifier binary from GitHub, with "no version" implying "latest". Moreover, this commit fixes the download logic to recognize binaries of older releases such as 0.4.3. Consequently, we no longer ship a Buildifier binary as part of the Docker container. More progress towards bazelbuild#1080.
fweikert
added a commit
to fweikert/continuous-integration
that referenced
this issue
Apr 1, 2021
Previously CI used the Buildifier binary from the Docker container if the user did not request a specific version. However, since we forgot to update the Docker container, it still contained an ancient version (0.22.0 - for comparison, the most recent release is 4.0.1). With this commit CI will always download a Buildifier binary from GitHub, with "no version" implying "latest". Moreover, this commit fixes the download logic to recognize binaries of older releases such as 0.4.3. Consequently, we no longer ship a Buildifier binary as part of the Docker container. More progress towards bazelbuild#1080.
|
Had to fix two other problems, but it seems to work now: https://buildkite.com/bazel/rules-rust-rustlang/builds/2602 |
keith
added a commit
to bazelbuild/rules_apple
that referenced
this issue
Apr 1, 2021
CI was previously broken, these were the violations that snuck in bazelbuild/continuous-integration#1080
|
Thanks! I'm seeing failures on our side too now |
|
Great! I'll close this issue - please let me know if you run into other problems. |
keith
added a commit
to bazelbuild/rules_apple
that referenced
this issue
Apr 1, 2021
CI was previously broken, these were the violations that snuck in bazelbuild/continuous-integration#1080
13 tasks
fweikert
added a commit
that referenced
this issue
Apr 8, 2021
Previously CI used the Buildifier binary from the Docker container if the user did not request a specific version. However, since we forgot to update the Docker container, it still contained an ancient version (0.22.0 - for comparison, the most recent release is 4.0.1). With this commit CI will always download a Buildifier binary from GitHub, with "no version" implying "latest". Moreover, this commit fixes the download logic to recognize binaries of older releases such as 0.4.3. Consequently, we no longer ship a Buildifier binary as part of the Docker container. More progress towards #1080.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I noticed after this commit landed bazelbuild/rules_apple@572aee2 there were some buildifier issues I see locally:
That are not being caught on CI. Is there some configuration of paths we should be doing to include these files?
The text was updated successfully, but these errors were encountered: