Skip to content

Commit

Permalink
Support using configured rule inputs from 7.0.0 prereleases (#74)
Browse files Browse the repository at this point in the history
This allows for more precise dependency analysis as we no longer need to
guess configurations (where we over-estimate), and avoids potential
deadlocks if we hit dependency cycles.

Fixes #72
  • Loading branch information
illicitonion authored Aug 30, 2023
1 parent c3c8572 commit cfae1f3
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 18 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/bazelbuild/bazel-gazelle v0.32.0
github.com/google/btree v1.1.2
github.com/google/uuid v1.3.0
github.com/hashicorp/go-version v1.6.0
github.com/otiai10/copy v1.7.1-0.20211223015809-9aae5f77261f
github.com/wI2L/jsondiff v0.2.0
google.golang.org/protobuf v1.27.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=
github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/otiai10/copy v1.7.1-0.20211223015809-9aae5f77261f h1:P7Ab27T4In6ExIHmjOe88b1BHpuHlr4Vr75hX2QKAXw=
github.com/otiai10/copy v1.7.1-0.20211223015809-9aae5f77261f/go.mod h1:rmRl6QPdJj6EiUqXQ/4Nn2lLXoNQjFCQbbNrxgc/t3U=
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
Expand Down
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ go_library(
"//third_party/protobuf/bazel/build",
"@com_github_aristanetworks_goarista//path",
"@com_github_bazelbuild_bazel_gazelle//label",
"@com_github_hashicorp_go_version//:go-version",
"@com_github_wi2l_jsondiff//:jsondiff",
"@org_golang_google_protobuf//encoding/protojson",
"@org_golang_google_protobuf//proto",
Expand Down
28 changes: 26 additions & 2 deletions pkg/hash_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"log"
"os"
"path/filepath"
"sort"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/bazel-contrib/target-determinator/third_party/protobuf/bazel/analysis"
"github.com/bazel-contrib/target-determinator/third_party/protobuf/bazel/build"
gazelle_label "github.com/bazelbuild/bazel-gazelle/label"
"github.com/hashicorp/go-version"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
)
Expand All @@ -38,8 +40,18 @@ func NewTargetHashCache(context map[gazelle_label.Label]map[Configuration]*analy
}

func isConfiguredRuleInputsSupported(releaseString string) bool {
// Disabled until https://github.com/bazelbuild/bazel/issues/17916 is resolved
return false
releasePrefix := "release "
explanation := " - assuming cquery does not support configured rule inputs (which is supported from bazel 7), which may lead to over-estimates of affected targets"
if !strings.HasPrefix(releaseString, releasePrefix) {
log.Printf("Bazel wasn't a released version%s", explanation)
return false
}
v, err := version.NewVersion(releaseString[len(releasePrefix):])
if err != nil {
log.Printf("Failed to parse Bazel version %q: %s", releaseString, explanation)
return false
}
return v.GreaterThanOrEqual(version.Must(version.NewVersion("7.0.0-pre.20230628.2")))
}

// TargetHashCache caches hash computations for targets and files, so that transitive hashes can be
Expand Down Expand Up @@ -492,6 +504,8 @@ func hashRule(thc *TargetHashCache, rule *build.Rule, configuration *analysis.Co
hasher.Write(protoBytes)
}

ownConfiguration := NormalizeConfiguration(configuration.GetChecksum())

// Hash rule inputs
if thc.bazelVersionSupportsConfiguredRuleInputs {
for _, configuredRuleInput := range rule.ConfiguredRuleInput {
Expand All @@ -500,6 +514,16 @@ func hashRule(thc *TargetHashCache, rule *build.Rule, configuration *analysis.Co
return nil, fmt.Errorf("failed to parse configuredRuleInput label %s: %w", configuredRuleInput.GetLabel(), err)
}
ruleInputConfiguration := NormalizeConfiguration(configuredRuleInput.GetConfigurationChecksum())
if ruleInputConfiguration.String() == "" {
// Configured Rule Inputs which aren't transitioned end up with an empty string as their configuration.
// This _either_ means there was no transition, _or_ means that the input was a source file (so didn't have a configuration at all).
// Fortunately, these are mutually exclusive - a target either has a configuration or doesn't, so we look up which one exists.
if _, ok := thc.context[ruleInputLabel][ownConfiguration]; ok {
ruleInputConfiguration = ownConfiguration
} else if _, ok := thc.context[ruleInputLabel][ruleInputConfiguration]; !ok {
return nil, fmt.Errorf("configuredRuleInputs for %s included %s in configuration %s but it couldn't be found either unconfigured or in the depending target's configuration %s. This probably indicates a bug in Bazel - please report it with a git repo that reproduces at https://github.com/bazel-contrib/target-determinator/issues so we can investigate", rule.GetName(), ruleInputLabel, ruleInputConfiguration, ownConfiguration)
}
}
ruleInputHash, err := thc.Hash(LabelAndConfiguration{Label: ruleInputLabel, Configuration: ruleInputConfiguration})
if err != nil {
return nil, fmt.Errorf("failed to hash configuredRuleInput %s %s which is a dependency of %s %s: %w", ruleInputLabel, ruleInputConfiguration, rule.GetName(), configuration.GetChecksum(), err)
Expand Down
48 changes: 32 additions & 16 deletions pkg/hash_cache_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package pkg_test
package pkg

import (
"encoding/hex"
Expand All @@ -9,7 +9,6 @@ import (
"reflect"
"testing"

"github.com/bazel-contrib/target-determinator/pkg"
"github.com/bazel-contrib/target-determinator/third_party/protobuf/bazel/analysis"
"github.com/bazel-contrib/target-determinator/third_party/protobuf/bazel/build"
"github.com/bazelbuild/bazel-gazelle/label"
Expand All @@ -29,7 +28,7 @@ func TestAbsolutifiesSourceFileInBuildDirBazel4(t *testing.T) {
},
}
const want = "/some/path/to/java/example/simple/Dep.java"
got := pkg.AbsolutePath(&target)
got := AbsolutePath(&target)
if want != got {
t.Fatalf("Wrong absolute path: want %v got %v", want, got)
}
Expand All @@ -45,7 +44,7 @@ func TestAbsolutifiesSourceFileInNestedDirBazel4(t *testing.T) {
},
}
const want = "/some/path/to/java/example/simple/just/a/File.java"
got := pkg.AbsolutePath(&target)
got := AbsolutePath(&target)
if want != got {
t.Fatalf("Wrong absolute path: want %v got %v", want, got)
}
Expand All @@ -61,7 +60,7 @@ func TestAbsolutifiesSourceFileInBuildDirBazel5(t *testing.T) {
},
}
const want = "/some/path/to/java/example/simple/Dep.java"
got := pkg.AbsolutePath(&target)
got := AbsolutePath(&target)
if want != got {
t.Fatalf("Wrong absolute path: want %v got %v", want, got)
}
Expand All @@ -77,7 +76,7 @@ func TestAbsolutifiesSourceFileInNestedDirBazel5(t *testing.T) {
},
}
const want = "/some/path/to/java/example/simple/just/a/File.java"
got := pkg.AbsolutePath(&target)
got := AbsolutePath(&target)
if want != got {
t.Fatalf("Wrong absolute path: want %v got %v", want, got)
}
Expand All @@ -95,7 +94,7 @@ func TestAbsolutifiesBuildFile(t *testing.T) {
},
}
const want = "/some/path/to/BUILD.bazel"
got := pkg.AbsolutePath(&target)
got := AbsolutePath(&target)
if want != got {
t.Fatalf("Wrong absolute path: want %v got %v", want, got)
}
Expand All @@ -105,7 +104,7 @@ func TestDigestsSingleSourceFile(t *testing.T) {
_, cqueryResult := layoutProject(t)
thc := parseResult(t, cqueryResult, "release 5.1.1")

hash, err := thc.Hash(pkg.LabelAndConfiguration{
hash, err := thc.Hash(LabelAndConfiguration{
Label: mustParseLabel("//HelloWorld:HelloWorld.java"),
})
if err != nil {
Expand All @@ -125,7 +124,7 @@ func TestDigestingMissingSourceFileIsNotError(t *testing.T) {
_, cqueryResult := layoutProject(t)
thc := parseResult(t, cqueryResult, "release 5.1.1")

_, err := thc.Hash(pkg.LabelAndConfiguration{
_, err := thc.Hash(LabelAndConfiguration{
Label: mustParseLabel("//HelloWorld:ThereIsNoWorld.java"),
})
if err != nil {
Expand All @@ -144,7 +143,7 @@ func TestDigestingDirectoryIsNotError(t *testing.T) {
_, cqueryResult := layoutProject(t)
thc := parseResult(t, cqueryResult, "release 5.1.1")

_, err := thc.Hash(pkg.LabelAndConfiguration{
_, err := thc.Hash(LabelAndConfiguration{
Label: mustParseLabel("//HelloWorld:InhabitedPlanets"),
})
if err != nil {
Expand All @@ -158,9 +157,9 @@ func TestDigestTree(t *testing.T) {
// v
// HelloWorld.java

labelAndConfiguration := pkg.LabelAndConfiguration{
labelAndConfiguration := LabelAndConfiguration{
Label: mustParseLabel("//HelloWorld:HelloWorld"),
Configuration: pkg.NormalizeConfiguration(configurationChecksum),
Configuration: NormalizeConfiguration(configurationChecksum),
}

const defaultBazelVersion = "release 5.1.1"
Expand Down Expand Up @@ -380,22 +379,39 @@ func layoutProject(t *testing.T) (string, *analysis.CqueryResult) {
return dir, &cqueryResult
}

func parseResult(t *testing.T, result *analysis.CqueryResult, bazelRelease string) *pkg.TargetHashCache {
cqueryResult, err := pkg.ParseCqueryResult(result)
func parseResult(t *testing.T, result *analysis.CqueryResult, bazelRelease string) *TargetHashCache {
cqueryResult, err := ParseCqueryResult(result)
if err != nil {
t.Fatalf("Failed to parse cquery result: %v", err)
}
return pkg.NewTargetHashCache(cqueryResult, bazelRelease)
return NewTargetHashCache(cqueryResult, bazelRelease)
}

func areHashesEqual(left, right []byte) bool {
return reflect.DeepEqual(left, right)
}

func mustParseLabel(s string) label.Label {
l, err := pkg.ParseCanonicalLabel(s)
l, err := ParseCanonicalLabel(s)
if err != nil {
panic(err)
}
return l
}

func Test_isConfiguredRuleInputsSupported(t *testing.T) {
for version, want := range map[string]bool{
"release 6.3.1": false,
"release 7.0.0-pre.20230530.3": false,
"release 7.0.0-pre.20230628.2": true,
"release 7.0.0-pre.20230816.3": true,
"release 7.0.0": true,
} {
t.Run(version, func(t *testing.T) {
got := isConfiguredRuleInputsSupported(version)
if want != got {
t.Fatalf("Incorrect isConfiguredRuleInputsSupported: want %v got %v", want, got)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public void tryImportInBazelrcAffectingJava() throws Exception {
super.tryImportInBazelrcAffectingJava();
}

// TODO: Stop ignoring this test when our tests use Bazel 7.0.0
@Override
public void addingTargetUsedInHostConfiguration() throws Exception {
allowOverBuilds(
Expand Down Expand Up @@ -108,6 +109,7 @@ public void reducingVisibilityOnDependencyAffectsTarget() throws Exception {
}
}

// TODO: Stop ignoring this test when our tests use Bazel 7.0.0
@Override
public void ignoredPlatformSpecificDepChanged() throws Exception {
allowOverBuilds("Platform-specific narrowing is disabled due to https://github.com/bazelbuild/bazel/issues/17916");
Expand Down
6 changes: 6 additions & 0 deletions third_party/go/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ def go_dependencies():
sum = "h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE=",
version = "v1.0.2",
)
go_repository(
name = "com_github_hashicorp_go_version",
importpath = "github.com/hashicorp/go-version",
sum = "h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=",
version = "v1.6.0",
)
go_repository(
name = "com_github_influxdata_influxdb1_client",
importpath = "github.com/influxdata/influxdb1-client",
Expand Down

0 comments on commit cfae1f3

Please sign in to comment.