Skip to content

Commit

Permalink
refactor(cli): upgrade configure directive suggestions to use aspect …
Browse files Browse the repository at this point in the history
…directives (#7025)

This is only error/suggestion messaging that the CLI outputs and does
not have any functional change, `# gazelle:` is still supported. Note
that we still refer to all the gazelle builtin directives, refer people
to gazelle docs etc, those docs will still have `# gazelle:` which could
cause some confusion? 🤷

Is this the direction we'd like to go? We are already requiring our
gazelle patches in order to use our gazelle plugins, support for `#
aspect:` is enabled with a 1-line patch. We could also do this only for
orion and not our existing js/kotlin extensions.

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 94a01922edd9ae69532fbf75105470075327d756
  • Loading branch information
jbedard committed Oct 29, 2024
1 parent e6863d9 commit 4dbada5
Show file tree
Hide file tree
Showing 25 changed files with 48 additions and 123 deletions.
2 changes: 1 addition & 1 deletion bazel/action_cache/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "bazel_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/analysis/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "bazel_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/buildeventstream/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "buildeventstream_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/command_line/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "command_line_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/failure_details/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "failure_details_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/flags/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "bazel_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/invocation_policy/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "invocation_policy_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/options/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "options_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/packages_metrics/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "devtools_build_lib_packages_metrics_proto",
Expand Down
2 changes: 1 addition & 1 deletion bazel/query/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")
load("//bazel/ts:defs.bzl", "ts_proto_library")
load("//bazel/go:write_go_generated_source_files.bzl", "write_go_generated_source_files")

proto_library(
name = "blaze_query_aspect_mirror_proto",
Expand Down
Empty file removed bazel/ts/BUILD.bazel
Empty file.
75 changes: 0 additions & 75 deletions bazel/ts/defs.bzl

This file was deleted.

2 changes: 1 addition & 1 deletion docs/bazelrc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ go_library(
"convenience.bazelrc",
"correctness.bazelrc",
"debug.bazelrc",
"java.bazelrc",
"javascript.bazelrc",
"performance.bazelrc",
"java.bazelrc",
],
importpath = "aspect.build/cli/docs/bazelrc",
visibility = ["//visibility:public"],
Expand Down
2 changes: 1 addition & 1 deletion gazelle/js/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ Generated targets include:

### Directives

See [Aspect CLI Directives](/docs/help/topics/directives.md#JavaScript) for a list of supported directives.
See [Aspect CLI Directives](/cli/core/docs/help/topics/directives.md#JavaScript) for a list of supported directives.
6 changes: 3 additions & 3 deletions gazelle/js/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,10 +384,10 @@ func (ts *typeScriptLang) addProjectRule(cfg *JsGazelleConfig, tsconfigRel strin
colError := gazelle.CheckCollisionErrors(targetName, TsProjectKind, sourceRuleKinds, args)
if colError != nil {
return nil, fmt.Errorf(colError.Error()+" "+
"Use the '# gazelle:%s' directive to change the naming convention.\n\n"+
"Use the '# aspect:%s' directive to change the naming convention.\n\n"+
"For example:\n"+
"\t# gazelle:%s {dirname}_js\n"+
"\t# gazelle:%s {dirname}_js_tests",
"\t# aspect:%s {dirname}_js\n"+
"\t# aspect:%s {dirname}_js_tests",
Directive_LibraryNamingConvention,
Directive_LibraryNamingConvention,
Directive_TestsNamingConvention,
Expand Down
10 changes: 5 additions & 5 deletions gazelle/js/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,11 @@ func (ts *typeScriptLang) resolveImports(
notFound := fmt.Errorf(
"Import %[1]q from %[2]q is an unknown dependency. Possible solutions:\n"+
"\t1. Instruct Gazelle to resolve to a known dependency using a directive:\n"+
"\t\t# gazelle:resolve [src-lang] js import-string label\n"+
"\t\t# aspect:resolve [src-lang] js import-string label\n"+
"\t\t or\n"+
"\t\t# gazelle:js_resolve import-string-glob label\n"+
"\t2. Ignore the dependency using the '# gazelle:%[3]s %[1]s' directive.\n"+
"\t3. Disable Gazelle resolution validation using '# gazelle:%[4]s off'",
"\t\t# aspect:js_resolve import-string-glob label\n"+
"\t2. Ignore the dependency using the '# aspect:%[3]s %[1]s' directive.\n"+
"\t3. Disable Gazelle resolution validation using '# aspect:%[4]s off'",
imp.ImportPath, imp.SourcePath, Directive_IgnoreImports, Directive_ValidateImportStatements,
)
resolutionErrors = append(resolutionErrors, notFound)
Expand Down Expand Up @@ -429,7 +429,7 @@ func (ts *typeScriptLang) resolveImportFromIndex(
// Too many results, don't know which is correct
if len(filteredMatches) > 1 {
return Resolution_Error, nil, fmt.Errorf(
"Import %q from %q resolved to multiple targets (%s) - this must be fixed using the \"gazelle:resolve\" directive",
"Import %q from %q resolved to multiple targets (%s) - this must be fixed using the \"aspect:resolve\" directive",
impStm.ImportPath, impStm.SourcePath, targetListFromResults(matches))
}

Expand Down
6 changes: 3 additions & 3 deletions gazelle/js/tests/rules_conflicting_name/expectedStderr.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Source rule generation error: failed to generate target "//:rules_conflicting_name" of kind "ts_project": a target of kind "filegroup" with the same name already exists. Use the '# gazelle:js_project_naming_convention' directive to change the naming convention.
Source rule generation error: failed to generate target "//:rules_conflicting_name" of kind "ts_project": a target of kind "filegroup" with the same name already exists. Use the '# aspect:js_project_naming_convention' directive to change the naming convention.

For example:
# gazelle:js_project_naming_convention {dirname}_js
# gazelle:js_tests_naming_convention {dirname}_js_tests
# aspect:js_project_naming_convention {dirname}_js
# aspect:js_tests_naming_convention {dirname}_js_tests

Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Source rule generation error: failed to generate target "//:rules_conflicting_name_mapped_kind" of kind "ts_override": a target of kind "asdf" with the same name already exists. Use the '# gazelle:js_project_naming_convention' directive to change the naming convention.
Source rule generation error: failed to generate target "//:rules_conflicting_name_mapped_kind" of kind "ts_override": a target of kind "asdf" with the same name already exists. Use the '# aspect:js_project_naming_convention' directive to change the naming convention.

For example:
# gazelle:js_project_naming_convention {dirname}_js
# gazelle:js_tests_naming_convention {dirname}_js_tests
# aspect:js_project_naming_convention {dirname}_js
# aspect:js_tests_naming_convention {dirname}_js_tests

16 changes: 8 additions & 8 deletions gazelle/js/tests/validate_import_statements/expectedStderr.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ Failed to validate dependencies for target "@validate_import_statements//:valida

Import "bad-import" from "main.ts" is an unknown dependency. Possible solutions:
1. Instruct Gazelle to resolve to a known dependency using a directive:
# gazelle:resolve [src-lang] js import-string label
# aspect:resolve [src-lang] js import-string label
or
# gazelle:js_resolve import-string-glob label
2. Ignore the dependency using the '# gazelle:js_ignore_imports bad-import' directive.
3. Disable Gazelle resolution validation using '# gazelle:js_validate_import_statements off'
# aspect:js_resolve import-string-glob label
2. Ignore the dependency using the '# aspect:js_ignore_imports bad-import' directive.
3. Disable Gazelle resolution validation using '# aspect:js_validate_import_statements off'

Import "bad-import2" from "main.ts" is an unknown dependency. Possible solutions:
1. Instruct Gazelle to resolve to a known dependency using a directive:
# gazelle:resolve [src-lang] js import-string label
# aspect:resolve [src-lang] js import-string label
or
# gazelle:js_resolve import-string-glob label
2. Ignore the dependency using the '# gazelle:js_ignore_imports bad-import2' directive.
3. Disable Gazelle resolution validation using '# gazelle:js_validate_import_statements off'
# aspect:js_resolve import-string-glob label
2. Ignore the dependency using the '# aspect:js_ignore_imports bad-import2' directive.
3. Disable Gazelle resolution validation using '# aspect:js_validate_import_statements off'
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ Warning: Failed to validate dependencies for target "@validate_import_statements

Import "bad-import" from "main.ts" is an unknown dependency. Possible solutions:
1. Instruct Gazelle to resolve to a known dependency using a directive:
# gazelle:resolve [src-lang] js import-string label
# aspect:resolve [src-lang] js import-string label
or
# gazelle:js_resolve import-string-glob label
2. Ignore the dependency using the '# gazelle:js_ignore_imports bad-import' directive.
3. Disable Gazelle resolution validation using '# gazelle:js_validate_import_statements off'
# aspect:js_resolve import-string-glob label
2. Ignore the dependency using the '# aspect:js_ignore_imports bad-import' directive.
3. Disable Gazelle resolution validation using '# aspect:js_validate_import_statements off'
4 changes: 2 additions & 2 deletions gazelle/kotlin/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (kt *kotlinLang) resolveImports(
notFound := fmt.Errorf(
"Import %[1]q from %[2]q is an unknown dependency. Possible solutions:\n"+
"\t1. Instruct Gazelle to resolve to a known dependency using a directive:\n"+
"\t\t# gazelle:resolve [src-lang] kotlin import-string label\n",
"\t\t# aspect:resolve [src-lang] kotlin import-string label\n",
mod.Imp, mod.SourcePath,
)

Expand Down Expand Up @@ -162,7 +162,7 @@ func (kt *kotlinLang) resolveImport(
if len(filteredMatches) > 1 {
return Resolution_Error, nil, fmt.Errorf(
"Import %q from %q resolved to multiple targets (%s)"+
" - this must be fixed using the \"gazelle:resolve\" directive",
" - this must be fixed using the \"aspect:resolve\" directive",
impt.Imp, impt.SourcePath, targetListFromResults(matches))
}

Expand Down
4 changes: 2 additions & 2 deletions gazelle/kotlin/tests/unknown_imports/expectedStdout.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Resolution error Import "lib.not" from "lib.kt" is an unknown dependency. Possible solutions:
1. Instruct Gazelle to resolve to a known dependency using a directive:
# gazelle:resolve [src-lang] kotlin import-string label
# aspect:resolve [src-lang] kotlin import-string label

Resolution error Import "foo" from "main.kt" is an unknown dependency. Possible solutions:
1. Instruct Gazelle to resolve to a known dependency using a directive:
# gazelle:resolve [src-lang] kotlin import-string label
# aspect:resolve [src-lang] kotlin import-string label

4 changes: 2 additions & 2 deletions integration_tests/aspect/common.bats
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ aspect() {
# but the configured path is only writable by specific users which we can't run as within the sandbox.
# these system wide flag affect the output path of the bazel-in-bazel that is run inside the tests leading to errors.
# NOTE: the output files left by bazel-in-bazel should be discarded as they are not part of the action.
"$TEST_SRCDIR/$TEST_WORKSPACE/cmd/aspect/aspect_/aspect" --aspect:nosystem_config --nosystem_rc --nohome_rc "$@"
"$TEST_SRCDIR/$TEST_WORKSPACE/cli/core/cmd/aspect/aspect_/aspect" --aspect:nosystem_config --nosystem_rc --nohome_rc "$@"
}

aspect_vanilla() {
# for use with --version, -v and --bazel-version
"$TEST_SRCDIR/$TEST_WORKSPACE/cmd/aspect/aspect_/aspect" "$@"
"$TEST_SRCDIR/$TEST_WORKSPACE/cli/core/cmd/aspect/aspect_/aspect" "$@"
}

setup_file() {
Expand Down
6 changes: 3 additions & 3 deletions pkg/aspect/outputs/hash_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 Aspect Build Systems, Inc.
* Copyright 2022 Aspect Build Systems, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,8 +37,8 @@ func TestHash(t *testing.T) {
t.Run("hashMurmur3Sync and hashMurmur3Concurrent return the same hash for the same set of files", func(t *testing.T) {
g := NewGomegaWithT(t)

const testTarget1 = "//pkg/aspect/outputs:test_label"
const testTarget2 = "//pkg/aspect/outputs:test_label"
const testTarget1 = "//cli/core/pkg/aspect/outputs:test_label"
const testTarget2 = "//cli/core/pkg/aspect/outputs:test_label"

hashFiles := make(map[string][]string)
hashFiles["//:test_label_1"] = testFixtures(1)
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugin/system/besproxy/bes_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (bp *besProxy) Send(req *buildv1.PublishBuildToolEventStreamRequest) error
}

// If we want to mutate the BES events in the future before they are sent out to external consumers, this is the place
// to do it. See https://github.com/aspect-build/silo/blob/7f13ab16fa10ffcec71b09737f0370f22a508823/pkg/plugin/system/besproxy/bes_proxy.go#L103
// to do it. See https://github.com/aspect-build/silo/blob/7f13ab16fa10ffcec71b09737f0370f22a508823/cli/core/pkg/plugin/system/besproxy/bes_proxy.go#L103
// as an example.

return bp.stream.Send(req)
Expand Down

0 comments on commit 4dbada5

Please sign in to comment.