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

GO LIST JSON? WHY NOT!? #150

Merged
merged 9 commits into from Jun 11, 2020
Merged

GO LIST JSON? WHY NOT!? #150

merged 9 commits into from Jun 11, 2020

Conversation

DarthHater
Copy link
Member

This PR handles:

go list -m all and go list -json -m all commands piped in to Nancy

I went with this approach as the Go Mod list command stuff is all internal. I've copied the struct for Module however, and trimmed it down for now to the fields we likely will ever need.

The approach I took was to:

  • Read the stdIn into a []byte, as I don't anticipate it being TOO big (I tried at first to do this by buffering it, but the code was getting extremely complicated for something that should never be like, massive in memory footprint)
  • Attempt to read from the []byte as json. If it errors, break the loop, if it doesn't, return everything, GREAT!
  • If it exits the loop due to an error (not EOF), I attempt to read it as if it was just a regular list like what we had before
  • The function that reads the parsed JSON can handle mods that were replaced now, giving us the accurate version being used

This new method GoParseAgnostic takes an io.Reader as well, so that we can test it a bit easier.

This pull request makes the following changes:

  • Switches main.go to use GoListAgnostic
  • Implements func GoListAgnostic(stdIn io.Reader) (deps types.ProjectList, err error)
  • Implements a private function to parse a GoListModule struct, which has a signature of func modToProjectList(mod types.GoListModule) (dep types.Projects, err error)

It relates to the following issue #s:

cc @bhamail / @DarthHater

Copy link
Contributor

@MichelKazi MichelKazi left a comment

Choose a reason for hiding this comment

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

LGTM!
Just a question though, does this mean parse.GoList is no more?

@DarthHater
Copy link
Member Author

@MichelKazi yep! We would remove that (or rename the other to GoList) when we do a 1.0.0 release

Comment on lines +90 to +98
if mod.Replace != nil {
if mod.Replace.Version == "" {
err = &NoVersionError{err: fmt.Errorf("No version found for mod")}
return
}
dep.Name = mod.Replace.Path
dep.Version = mod.Replace.Version
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since #142 called this out as an issue i think we should add a test around it to make sure it is working as expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad i should have been more clear :) that just handles replace for go list and not go list -json. We should probably have a replace test for the json path too.

{
        "Path": "github.com/gorilla/websocket",
        "Version": "v1.4.0",
        "Replace": {
                "Path": "github.com/gorilla/websocket",
                "Version": "v1.4.2",
                "Time": "2020-03-19T17:50:51Z",
                "GoMod": "/Users/ME/go/pkg/mod/cache/download/github.com/gorilla/websocket/@v/v1.4.2.mod"
        },
        "Update": {
                "Path": "github.com/gorilla/websocket",
                "Version": "v1.4.2",
                "Time": "2020-03-19T17:50:51Z"
        },
        "Indirect": true,
        "GoMod": "/Users/ME/go/pkg/mod/cache/download/github.com/gorilla/websocket/@v/v1.4.2.mod"
}

One of those testdata files but with that and maybe another in it.

Copy link
Member Author

Choose a reason for hiding this comment

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

In: 64079e0

Copy link
Contributor

@zendern zendern left a comment

Choose a reason for hiding this comment

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

Code looks good to me....probably need to update the README too. And add something to the extent that you will get better results if you use go list -m -json

@DarthHater
Copy link
Member Author

@zendern , README updated!

@DarthHater DarthHater merged commit 8d6b87d into master Jun 11, 2020
@DarthHater DarthHater deleted the GoModJsonAndReplace branch June 11, 2020 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle replace (old version to other version)
3 participants