Skip to content

Commit

Permalink
fix: ensure gazelle config overrides are local (#2638)
Browse files Browse the repository at this point in the history
The change to add custom targets caused this issue. The inherited
targets configuration was not deep cloned, so modifications to the
targets configurations would modify the config of parent BUILDs.

<!-- Delete this comment! Follow our PR template instructions:
https://github.com/aspect-build/.github/blob/main/pull_requests.md -->

### Type of change

- Bug fix (change which fixes an issue)

### Test plan

- New test cases added

GitOrigin-RevId: bba27992671afa344077b8ef40be142f583075fa
  • Loading branch information
jbedard authored and gregmagolan committed Jun 18, 2023
1 parent e7bda75 commit b077571
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 5 deletions.
17 changes: 16 additions & 1 deletion gazelle/js/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,15 @@ func newRootConfig() *JsGazelleConfig {
}
}

func (g *TargetGroup) copy() *TargetGroup {
return &TargetGroup{
name: g.name,
customSources: g.customSources[:],
defaultSources: g.defaultSources,
testonly: g.testonly,
}
}

// NewChild creates a new child JsGazelleConfig. It inherits desired values from the
// current JsGazelleConfig and sets itself as the parent to the child.
func (c *JsGazelleConfig) NewChild(childPath string) *JsGazelleConfig {
Expand All @@ -194,8 +203,14 @@ func (c *JsGazelleConfig) NewChild(childPath string) *JsGazelleConfig {
cCopy.excludes = make([]string, 0)
cCopy.ignoreDependencies = make([]string, 0)
cCopy.resolves = linkedhashmap.New()
cCopy.targets = c.targets[:]

// Copy the targets, any modifications will be local.
cCopy.targets = make([]*TargetGroup, 0, len(c.targets))
for _, target := range c.targets {
cCopy.targets = append(cCopy.targets, target.copy())
}

// Copy the overrides, any modifications will be local.
cCopy.targetNamingOverrides = make(map[string]string)
for k, v := range c.targetNamingOverrides {
cCopy.targetNamingOverrides[k] = v
Expand Down
5 changes: 1 addition & 4 deletions gazelle/js/tests/simple_globs/BUILD.out
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ load("@aspect_rules_ts//ts:defs.bzl", "ts_project")

ts_project(
name = "simple_globs",
srcs = [
"other.ts",
"srcs/main.ts",
],
srcs = ["srcs/main.ts"],
)

ts_project(
Expand Down
5 changes: 5 additions & 0 deletions gazelle/js/tests/simple_globs/e2e/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Modify the globs within a subdirectory.
# Should not effect the js_test_files of parent/sibling directories.
# Example bug: https://github.com/sourcegraph/sourcegraph/commit/87a70c7c1ff0328540af1b2d77eb3b035b41b1d3

# gazelle:js_test_files **/*.{ts,tsx}
16 changes: 16 additions & 0 deletions gazelle/js/tests/simple_globs/e2e/BUILD.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")

# Modify the globs within a subdirectory.
# Should not effect the js_test_files of parent/sibling directories.
# Example bug: https://github.com/sourcegraph/sourcegraph/commit/87a70c7c1ff0328540af1b2d77eb3b035b41b1d3

# gazelle:js_test_files **/*.{ts,tsx}

ts_project(
name = "e2e_tests",
testonly = True,
srcs = [
"a.ts",
"r.ts",
],
)
Empty file.
Empty file.

0 comments on commit b077571

Please sign in to comment.