Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(go): use toolchain as stdlib version for go.mod files #7163

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(gomod): parse toolchain to get stdlib version
  • Loading branch information
DmitriyLewen committed Jul 15, 2024
commit f4d7af419827ec2c13503468fd688fd278ef677f
13 changes: 13 additions & 0 deletions pkg/dependency/parser/golang/mod/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,19 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc
skipIndirect = lessThan117(modFileParsed.Go.Version)
}

// Stdlib
if toolchain := modFileParsed.Toolchain; toolchain != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Go uses go line if toolchain is omitted, we probably need to check the go line as well.

If the toolchain line is omitted, the module or workspace is considered to have an implicit toolchain goV line, where V is the Go version from the go line.

https://go.dev/doc/toolchain

But we need to consider how to treat a go line omitting a patch version, like go 1.22. I think we can skip stdlib in this case.

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can skip stdlib in this case.

If module uses version without patch (and child modules don't use patch and toolchain) - go doesn't add patch/toolchain:

➜ cat ../greetings/go.mod 
module github.com/greetings

go 1.22
➜ cat go.mod
module example.com/hello

go 1.22

replace example.com/greetings => ../greetings

require example.com/greetings v0.0.0-00010101000000-000000000000
➜ go version
go version go1.22.0 darwin/arm64

since we say we use minimum required version for stdlib - we can say that v1.x.0 (v1.21.0 for this example) is the minimum required version, no?
I think I'm missing something, but I can't figure out what 😄

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or do you mean that if go version doesn't contain patch - that means it is not a situation where toolchan is omitted?
and we don't need to check for cases where toolchain is not used (or omitted).

But it doesn't work for v1.19 or early: The standard Go toolchains are named goV where V is a Go version denoting a beta release, release candidate, or release. For example, go1.21rc1 and go1.21.0 are toolchain names; go1.21 and go1.22 are not (the initial releases are go1.21.0 and go1.22.0), but go1.20 and go1.19 are.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can say that v1.x.0 (v1.21.0 for this example) is the minimum required version, no?

I found answer - 1.21 != 1.21.0:
For example, 1.21 < 1.21rc1 < 1.21rc2 < 1.21.0 < 1.21.1 < 1.21.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 I updated this PR:

  • if toolchain is omitted - check go line
    • check go version (take only >= 1.21)
    • check patch
    • check rc releases

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detection using the minimum version may be better enabled when this flag is used. For example, django>=3.0.0 in requirements.txt, we can take 3.0.0 as the version even if the project may use newer than 3.0.0. The toolchain version is the same. From toolchain go1.21.4 in go.mod, we consider it Go 1.21.4 even if the project may actually use Go 1.21.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Detection using the minimum version may be better enabled when this flag is used.

Agree with you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've converted this PR to draft. Let's finalize this proposal first and come back here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

django>=3.0.0 in requirements.txt, we can take 3.0.0 as the version even if the project may use newer than 3.0.0

this is good idea 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knqyf263 I updated this PR using --detection-priority flag.
Take a look, when you have time, please.

// `go1.22.5` => `1.22.5`
ver := strings.TrimPrefix(toolchain.Name, "go")
pkgs["stdlib"] = ftypes.Package{
// Add the toolchain version as stdlib version
ID: packageID("stdlib", ver),
Name: "stdlib",
Version: ver,
Relationship: ftypes.RelationshipDirect, // Considered a direct dependency as the main module depends on the standard packages.
}
}

// Main module
if m := modFileParsed.Module; m != nil {
ver := strings.TrimPrefix(m.Mod.Version, "v")
Expand Down