oss-review-toolkit / ort

A suite of tools to automate software compliance checks.
https://oss-review-toolkit.org
Apache License 2.0
1.57k stars 308 forks source link

[BUG] Analyzer fails with go.mod #6615

Closed qequ closed 1 year ago

qequ commented 1 year ago

Trying to analyze a Go project like this https://github.com/parvez3019/go-swagger3 ort crashes with this message

[DefaultDispatcher-worker-1] ERROR org.ossreviewtoolkit.analyzer.PackageManager - Resolving GoMod dependencies for path 'go.mod' failed with: MissingKotlinParameterException: Instantiation of [simple type, class org.ossreviewtoolkit.analyzer.managers.ModuleInfoFile] value failed for JSON property Origin due to missing (therefore NULL) value for creator parameter origin which is a non-nullable type
 at [Source: (File); line: 1, column: 50] (through reference chain: org.ossreviewtoolkit.analyzer.managers.ModuleInfoFile["Origin"])

The Analyzer used by ort was GoMod and the command used for testing was

./cli/build/install/ort/bin/ort analyze -i ~/go-swagger3/ -o ~/ort_results/

(built from sources)

qequ commented 1 year ago

Checking on getModuleInfos the line that parses to objects ModuleInfo found out that some modules info lack GoMod data, like for example

{
    "Path": "golang.org/x/term",
    "Version": "v0.0.0-20210927222741-03fcf44c2211",
    "Time": "2021-09-27T22:27:41Z",
    "Indirect": true
}
{
    "Path": "golang.org/x/text",
    "Version": "v0.3.7",
    "Time": "2021-08-10T18:28:16Z",
    "Indirect": true
}

but in the ModuleInfo class the GoMod property is mandatory

    @JsonProperty("GoMod")
    val goMod: String

that's the cause of the crashing. If setting a default paramater like ".../dummy.mod" after that it's another crash when parsing data to create the ModuleInfoFile object

@JsonIgnoreProperties(ignoreUnknown = true)
private data class ModuleInfoFile(
    @JsonProperty("Origin")
    val origin: Origin
) {
    @JsonIgnoreProperties(ignoreUnknown = true)
    data class Origin(
        @JsonProperty("VCS")
        val vcs: String? = null,
        @JsonProperty("URL")
        val url: String? = null,
        @JsonProperty("Ref")
        val ref: String? = null,
        @JsonProperty("Hash")
        val hash: String? = null,
        @JsonProperty("Subdir")
        val subdir: String? = null
    )
}

due that it lacks an Origin

@fviernau @sschuberth

dgutson commented 1 year ago

@arieltorti

dgutson commented 1 year ago

We concluded that there are valid cases where the GoMod can be missing (Ariel can explain further). Minimum steps to reproduce a project without GoMod:

mkdir pepe
cd pepe
go mod init ariel
go get  golang.org/x/net@v0.0.0-20220812174116-3211cb980234
go list -m -json all

There you will get the many entries without GoMod, for example

{
        "Path": "golang.org/x/sys",
        "Version": "v0.0.0-20220728004956-3c1f35247d10",
        "Time": "2022-07-28T00:49:56Z",
        "Indirect": true
}
qequ commented 1 year ago

PATCH; updated go to 1.20 (last version) and set ModuleInfo's GoMod json property default to "".

--- a/analyzer/src/main/kotlin/managers/GoMod.kt
+++ b/analyzer/src/main/kotlin/managers/GoMod.kt
@@ -483,7 +483,7 @@ private data class ModuleInfo(
     val main: Boolean = false,

     @JsonProperty("GoMod")
-    val goMod: String
+    val goMod: String = ""
 )

 private data class DepInfo(

Altough this works when analyzing with the docker package there is no dependency tree generated for it

dgutson commented 1 year ago

Note: when analyzing the docker package ORT shows no dependencies of docker.

fviernau commented 1 year ago

Thanks for reporting this @qequ . I haven't finished investigating this yet.

Anyhow, I wonder if this "bug" is impossible to happen if the dependencies are kind of consistent in go.mod and go.sum with what go mod tidy would output. IIUC go mod tidy adds all dependencies, including indirect ones, to go.mod / go.sum. So, it basically makes all dependencies direct.

I now wonder if this problem cannot happen if there is no "indirect" dependencies. Idea came when reading https://groups.google.com/g/golang-codereviews/c/I3VInl2hCZk.

Would you mind trying to find an example (including a main.go as well as a go.mod and go.sum) which is consistent with tidy which reproduces this problem? @qequ or @dgutson ?

sschuberth commented 1 year ago

@fviernau, looks like @oheger-bosch is also working on this in https://github.com/oss-review-toolkit/ort/compare/main...bosch-io:oss-review-toolkit:oheger-bosch/analyzer/gomod_modules. It probably makes sense to align.

oheger-bosch commented 1 year ago

@sschuberth, @fviernau: Yes, this problem occurred in one of our projects yesterday. I was testing an approach to make the goMod property in the ModuleInfo structure optional. This seems to fix the issue. I will open a draft PR, so you can have a look.

oheger-bosch commented 1 year ago

See https://github.com/oss-review-toolkit/ort/pull/6638

malmor commented 1 year ago

Hey @fviernau, here is our example project that produces this error (Note: the Go code itself does not work, but contains the necessary dependencies 😉).

example-project-missing-gomod-field.zip

What I did to test this:

# Run docker container because Go is not installed on my pc
$ docker run -it --rm -v path/to/project:/test --entrypoint="" golang:1.19 bash

# Switch to mounted directory
root@fbb823281ed9: $ cd /test

# Run 'go mod tidy' - which does not change the 'go.mod' or 'go.sum' (they are up to date)
root@fbb823281ed9: $ go mod tidy

# Verify that the graph can be generated - works
root@fbb823281ed9: $ go mod graph

# Install jq for json parsing
root@fbb823281ed9: $ apt-get update && apt-get install -y jq

# Run 'go list' for the full output
root@fbb823281ed9: $ go list -m -json -buildvcs=false all

# Run 'go list' and only select packages that do not contain the 'GoMod' field
root@fbb823281ed9: $ go list -m -json -buildvcs=false all | jq '. | select(.GoMod == null)'

For our project these dependencies seem to be missing the GoMod field:

{
  "Path": "github.com/davecgh/go-spew",
  "Version": "v1.1.1",
  "Time": "2018-02-21T23:26:28Z",
  "Indirect": true,
  "Dir": "/go/pkg/mod/github.com/davecgh/go-spew@v1.1.1"
}
{
  "Path": "github.com/stretchr/testify",
  "Version": "v1.7.2",
  "Time": "2022-06-06T09:52:45Z",
  "Indirect": true,
  "Dir": "/go/pkg/mod/github.com/stretchr/testify@v1.7.2"
}
{
  "Path": "gopkg.in/yaml.v3",
  "Version": "v3.0.1",
  "Time": "2022-05-27T08:35:30Z",
  "Indirect": true,
  "Dir": "/go/pkg/mod/gopkg.in/yaml.v3@v3.0.1"
}

Apparently they are all indirect dependencies used by github.com/aws/aws-go-lambda - because once you remove it the issue can no longer be reproduced (see the comment in the main.go).

To reproduce the error inside ORT I used revision d949be19c1c99f8abd07d30bbe5d2ce480744bed (released 2023-03-06 11:06 PM GMT+1). Running the analyze phase then produces this error:

11:24:10.595 [DefaultDispatcher-worker-2] DEBUG org.eclipse.jgit.util.SystemReader - loading config FileBasedConfig[/home/ort/.gitconfig]
11:24:10.663 [DefaultDispatcher-worker-2] ERROR org.ossreviewtoolkit.analyzer.PackageManager - Resolving GoMod dependencies for path 'go.mod' failed with: MissingKotlinParameterException: Instantiation of [simple type, class org.ossreviewtoolkit.analyzer.managers.ModuleInfo] value failed for JSON property GoMod due to missing (therefore NULL) value for creator parameter goMod which is a non-nullable type
 at [Source: (String)"{
\u0009"Path": "my.company.local/team-a/app-a",
\u0009"Main": true,
\u0009"Dir": "/builds/team-a/app-a",
\u0009"GoMod": "/builds/team-a/app-a/go.mod",
\u0009"GoVersion": "1.19"
}
{
\u0009"Path": "github.com/aws/aws-lambda-go",
\u0009"Version": "v1.35.0",
\u0009"Time": "2022-11-16T11:12:14Z",
\u0009"GoMod": "/tmp/ort-GoMod15285485639793805450/pkg/mod/cache/download/github.com/aws/aws-lambda-go/@v/v1.35.0.mod",
\u0009"GoVersion": "1.18"
}
{
\u0009"Path": "github.com/aws/aws-sdk-go-v2",
\u0009"Versi"[truncated 7185 chars]; line: 162, column: 1] (through reference chain: org.ossreviewtoolkit.analyzer.managers.ModuleInfo["GoMod"])
11:24:10.666 [DefaultDispatcher-worker-2] INFO  org.ossreviewtoolkit.analyzer.PackageManager - Resolving GoMod dependencies for path 'go.mod' took 1m 16.851894495s.
11:24:10.669 [DefaultDispatcher-worker-2] INFO  org.ossreviewtoolkit.analyzer.Analyzer - Finished GoMod analysis.

If there is anything else I can provide then please let me know.

fviernau commented 1 year ago

Awsome @malmor this was very helpful. I've just tested my 2 open PRs against your example and it seems to work now. It also works against the go.mod file` described in the ticket description.

malmor commented 1 year ago

Thanks @fviernau - looking forward to testing the new version once it's released!

fviernau commented 1 year ago

@malmor just FYI: the dependencies of your example projects which have GoMod == null are test dependencies of one of your dependencies. These are neither required for building your main module nor for building your tests. They are only required for building the tests of that aws package and thus are irrelevant / unused. (They should not appear in the result at all) My theory that GoMod is null only for such unused deps seems to play out...

go mod why -m github.com/davecgh/go-spew
# github.com/davecgh/go-spew
my.company.local/team-a/app-a
github.com/aws/aws-lambda-go/lambda
github.com/aws/aws-lambda-go/lambda.test
github.com/stretchr/testify/assert
github.com/davecgh/go-spew/spew

go mod why -m gopkg.in/yaml.v3
# gopkg.in/yaml.v3
my.company.local/team-a/app-a
github.com/aws/aws-lambda-go/lambda
github.com/aws/aws-lambda-go/lambda.test
github.com/stretchr/testify/assert
gopkg.in/ya

go mod why -m github.com/stretchr/testify
# github.com/stretchr/testify
my.company.local/team-a/app-a
github.com/aws/aws-lambda-go/lambda
github.com/aws/aws-lambda-go/lambda.test
github.com/stretchr/testify/assert
dgutson commented 1 year ago

Thanks!

malmor commented 1 year ago

Thanks a lot @fviernau (and everybody else who reviewed the PRs and shared their thoughts)!

malmor commented 1 year ago

I just re-tested this issue running revision 314c95804246f2b668a2eb8ce046590b9216af84 (released 2023-03-13 10:01 PM GMT+1) and the issue does no longer occur - neither for the small reproduction project nor for our main project 🚀

dgutson commented 1 year ago

@fviernau there's something weird here, ORT is running for almost 3x time and still running for the same input as before. More than 1 day (with 5 / 6 hours last time). @franco0700 pls check the other commits that you got from the master branch and list here?

sschuberth commented 1 year ago

@Franco0700 pls check the other commits that you got from the master branch and list here?

Please not here. If you encounter a new issue like a regression in runtime, please open a new issue.

dgutson commented 1 year ago

I would like to ask @fviernau if he thinks this fix may had something to do with the delay?

sschuberth commented 1 year ago

I would like to ask @fviernau if he thinks this fix may had something to do with the delay?

If you're not certain the discussion belongs to this issue or if it's a question, better head at our Slack workspace or GitHub discussions.

fviernau commented 1 year ago

I would like to ask @fviernau if he thinks this fix may had something to do with the delay?

@dgutson I'm not sure what you're referring to with "the delay". However, never say never but I believe it's highly unlikely that the fixes I made for this issue changed anything in terms of execution time.

dgutson commented 1 year ago

@fviernau understood.