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: add parser for canon.lock files #128

Merged
merged 7 commits into from Sep 4, 2022
Merged

feat: add parser for canon.lock files #128

merged 7 commits into from Sep 4, 2022

Conversation

DmitriyLewen
Copy link
Collaborator

@DmitriyLewen DmitriyLewen commented Aug 24, 2022

Description

Added parser for canon.lock files

Related issues

Comment on lines 7 to 10
dio "github.com/aquasecurity/go-dep-parser/pkg/io"
"github.com/aquasecurity/go-dep-parser/pkg/types"
"golang.org/x/xerrors"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't golangci-lint show goimport error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

// 'pkgc/system'
ref := strings.Split(strings.Split(nod.Ref, "@")[0], "/")
if len(ref) != 2 {
return nil, nil, xerrors.Errorf("unable to parse ref: %s", nod.Ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe i miss something.
there will be an error even for one broken ref. right?

I'd prefer to write logs and continue. but if we don't have logging, breaking is better way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It makes sense.
I have added a debug message. Can you take a look?

Comment on lines 49 to 52
// skip system dependencies
if ref[1] == "system" {
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why should we skip system deps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

conan system dependencies have a system only version (e.g. glu)

At first I thought that we need to skip these dependencies, but currently I think we need to keep all dependencies.

Thank for your note! Changed it.

Nodes map[string]Nod `json:"nodes"`
}

type Nod struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we cannot use Node?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is my typo, fixed. Thanks.

Comment on lines 50 to 51
f, err := os.Open(tt.inputFile)
require.NoError(t, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
f, err := os.Open(tt.inputFile)
require.NoError(t, err)
f, err := os.Open(tt.inputFile)
require.NoError(t, err)
defer f.Close()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,75 @@
package lock
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
package lock
package lock_test

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

// 'pkgc/system'
ref := strings.Split(strings.Split(nod.Ref, "@")[0], "/")
if len(ref) != 2 {
once.Do(func() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need once.Do here? If there are multiple invalid versions, don't we display them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right. User should see all wrong dependencies.
Removed this.

Comment on lines 8 to 12
dio "github.com/aquasecurity/go-dep-parser/pkg/io"
"github.com/aquasecurity/go-dep-parser/pkg/log"
"github.com/aquasecurity/go-dep-parser/pkg/types"

"golang.org/x/xerrors"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit

Suggested change
dio "github.com/aquasecurity/go-dep-parser/pkg/io"
"github.com/aquasecurity/go-dep-parser/pkg/log"
"github.com/aquasecurity/go-dep-parser/pkg/types"
"golang.org/x/xerrors"
"golang.org/x/xerrors"
dio "github.com/aquasecurity/go-dep-parser/pkg/io"
"github.com/aquasecurity/go-dep-parser/pkg/log"
"github.com/aquasecurity/go-dep-parser/pkg/types"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@knqyf263 knqyf263 merged commit d2cb7a4 into main Sep 4, 2022
@knqyf263 knqyf263 deleted the feat/conan branch September 4, 2022 09:05
Sq34sy pushed a commit to Sq34sy/go-dep-parser that referenced this pull request Jul 28, 2023
Co-authored-by: knqyf263 <knqyf263@gmail.com>
Sq34sy pushed a commit to Sq34sy/go-dep-parser that referenced this pull request Jul 28, 2023
Co-authored-by: knqyf263 <knqyf263@gmail.com>
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.

None yet

3 participants