Skip to content

Commit fbb4bd4

Browse files
authored
Fix cross repo Go references when not pointing at a tagged version (#99)
This PR resolves a mismatch with how scip-go would generate the version string when not pointing at a tagged module version string. This mismatch would break cross-repo references.
1 parent 3e3635f commit fbb4bd4

File tree

5 files changed

+111
-11
lines changed

5 files changed

+111
-11
lines changed

internal/git/infer_module_version.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88

99
// InferModuleVersion returns the version of the module declared in the given
1010
// directory. This will be either the work tree commit's tag, or it will be the
11-
// most recent tag with a short revhash appended to it.
11+
// short revhash of the HEAD commit.
1212
func InferModuleVersion(dir string) (string, error) {
1313
version, err := command.Run(dir, "git", "tag", "-l", "--points-at", "HEAD")
1414
if err != nil {

internal/loader/loader.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"github.com/sourcegraph/scip-go/internal/newtypes"
1111
"github.com/sourcegraph/scip-go/internal/output"
1212
"golang.org/x/mod/modfile"
13+
"golang.org/x/mod/module"
14+
"golang.org/x/mod/semver"
1315
"golang.org/x/tools/go/packages"
1416
)
1517

@@ -219,6 +221,27 @@ func normalizePackage(opts *config.IndexOpts, pkg *packages.Package) *packages.P
219221

220222
pkg.Module.Version = "."
221223
}
224+
} else if module.IsPseudoVersion(pkg.Module.Version) {
225+
// Unpublished versions of dependencies have pseudo-versions in go.mod.
226+
// When the dependency itself is indexed, only the revision will be used.
227+
// For correct cross-repo navigation to such dependencies, only use
228+
// the revision from a pseudo-version.
229+
rev, err := module.PseudoVersionRev(pkg.Module.Version)
230+
if err != nil {
231+
// Only panic when running in debug mode.
232+
fmt.Println(handler.ErrOrPanic(
233+
"Unable to find rev from pseudo-version: %s %s",
234+
pkg.Module.Path,
235+
pkg.Module.Version,
236+
))
237+
} else {
238+
pkg.Module.Version = rev
239+
}
240+
} else if build := semver.Build(pkg.Module.Version); build != "" {
241+
// The revision can also have build metadata following a `+`. Drop that,
242+
// similar to official Go tooling. (https://go.dev/ref/mod#versions)
243+
// > The build metadata suffix is ignored for the purpose of comparing versions
244+
pkg.Module.Version = strings.TrimSuffix(pkg.Module.Version, build)
222245
}
223246

224247
return pkg

internal/loader/loader_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"testing"
88

99
"github.com/sourcegraph/scip-go/internal/config"
10+
"github.com/stretchr/testify/require"
11+
"golang.org/x/mod/module"
1012
"golang.org/x/tools/go/packages"
1113
)
1214

@@ -39,6 +41,81 @@ func TestBuiltinFormat(t *testing.T) {
3941
}
4042
}
4143

44+
type normalizeTestCase struct {
45+
Raw string
46+
Normalized string
47+
}
48+
49+
func TestNormalizePackageModuleVersion(t *testing.T) {
50+
cases := []normalizeTestCase{
51+
{
52+
Raw: "v0.0.0-20180920160851-f15b22f93c73",
53+
Normalized: "f15b22f93c73",
54+
},
55+
{
56+
Raw: "v0.3.1-0.20230414160720-beea233bdc0b",
57+
Normalized: "beea233bdc0b",
58+
},
59+
{
60+
Raw: "v2.0.0-20180818164646-67afb5ed74ec",
61+
Normalized: "67afb5ed74ec",
62+
},
63+
{
64+
Raw: "v1.1.1",
65+
Normalized: "v1.1.1",
66+
},
67+
{
68+
Raw: "v1.0.0-beta.1",
69+
Normalized: "v1.0.0-beta.1",
70+
},
71+
{
72+
Raw: "v0.0.0",
73+
Normalized: "v0.0.0",
74+
},
75+
{
76+
Raw: "v2.0.0+incompatible",
77+
Normalized: "v2.0.0",
78+
},
79+
{
80+
Raw: "",
81+
Normalized: ".",
82+
},
83+
}
84+
85+
for _, testCase := range cases {
86+
pkg := &packages.Package{
87+
PkgPath: "github.com/fake_name/fake_module/fake_package",
88+
Module: &packages.Module{
89+
Path: "github.com/fake_name/fake_module",
90+
Version: testCase.Raw,
91+
},
92+
}
93+
normalizePackage(&config.IndexOpts{}, pkg)
94+
95+
require.Equal(t, testCase.Normalized, pkg.Module.Version)
96+
}
97+
}
98+
99+
func TestPackagePseudoVersion(t *testing.T) {
100+
wd, _ := os.Getwd()
101+
root, _ := filepath.Abs(filepath.Join(wd, "../../"))
102+
pkgConfig := getConfig(root, config.IndexOpts{})
103+
pkgConfig.Tests = false
104+
105+
pkgs, err := packages.Load(pkgConfig, "github.com/efritz/pentimento")
106+
require.Nil(t, err)
107+
108+
require.Equal(t, 1, len(pkgs), "Too many packages")
109+
110+
pkg := pkgs[0]
111+
112+
require.True(t, module.IsPseudoVersion(pkg.Module.Version), "Package did not have a pseudo version: pre ensure")
113+
114+
normalizePackage(&config.IndexOpts{}, pkg)
115+
116+
require.Equal(t, "ade47d831101", pkg.Module.Version, "Package pseudo-version was not extracted into a sha: post ensure")
117+
}
118+
42119
func TestPackageWithinModule(t *testing.T) {
43120
wd, _ := os.Getwd()
44121
root, _ := filepath.Abs(filepath.Join(wd, "../../"))

internal/testdata/snapshots/output/generallyeric/new_operators.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
22
// ^^^^^^^^^^^^^ reference 0.1.test `sg/generallyeric`/
33

44
import "golang.org/x/exp/constraints"
5-
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/
5+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/
66

77
type Number interface {
88
// ^^^^^^ definition 0.1.test `sg/generallyeric`/Number#
99
// documentation ```go
1010
// documentation ```go
1111
constraints.Float | constraints.Integer | constraints.Complex
12-
// ^^^^^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/
13-
// ^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/Float#
14-
// ^^^^^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/
15-
// ^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/Integer#
16-
// ^^^^^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/
17-
// ^^^^^^^ reference golang.org/x/exp v0.0.0-20221205204356-47842c84f3db `golang.org/x/exp/constraints`/Complex#
12+
// ^^^^^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/
13+
// ^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/Float#
14+
// ^^^^^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/
15+
// ^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/Integer#
16+
// ^^^^^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/
17+
// ^^^^^^^ reference golang.org/x/exp 47842c84f3db `golang.org/x/exp/constraints`/Complex#
1818
}
1919

2020
func Double[T Number](value T) T {

internal/testdata/snapshots/output/replace-directives/replace_directives.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
// ^^^ reference github.com/golang/go/src go1.22 fmt/
88

99
"github.com/dghubble/gologin"
10-
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference github.com/sourcegraph/gologin v1.0.2-0.20181110030308-c6f1b62954d8 `github.com/dghubble/gologin`/
10+
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ reference github.com/sourcegraph/gologin c6f1b62954d8 `github.com/dghubble/gologin`/
1111
)
1212

1313
func Something() {
@@ -16,7 +16,7 @@
1616
fmt.Println(gologin.DefaultCookieConfig)
1717
// ^^^ reference github.com/golang/go/src go1.22 fmt/
1818
// ^^^^^^^ reference github.com/golang/go/src go1.22 fmt/Println().
19-
// ^^^^^^^ reference github.com/sourcegraph/gologin v1.0.2-0.20181110030308-c6f1b62954d8 `github.com/dghubble/gologin`/
20-
// ^^^^^^^^^^^^^^^^^^^ reference github.com/sourcegraph/gologin v1.0.2-0.20181110030308-c6f1b62954d8 `github.com/dghubble/gologin`/DefaultCookieConfig.
19+
// ^^^^^^^ reference github.com/sourcegraph/gologin c6f1b62954d8 `github.com/dghubble/gologin`/
20+
// ^^^^^^^^^^^^^^^^^^^ reference github.com/sourcegraph/gologin c6f1b62954d8 `github.com/dghubble/gologin`/DefaultCookieConfig.
2121
}
2222

0 commit comments

Comments
 (0)