Skip to content

fix: properly parse conan ref and include user and channel#2034

Merged
wagoodman merged 2 commits into
anchore:mainfrom
Pro:fix-conan-ref-parse
Aug 23, 2023
Merged

fix: properly parse conan ref and include user and channel#2034
wagoodman merged 2 commits into
anchore:mainfrom
Pro:fix-conan-ref-parse

Conversation

@Pro
Copy link
Copy Markdown
Contributor

@Pro Pro commented Aug 17, 2023

The current implementation in syft is not parsing the user and channel from a conan ref.

Both are highly relevant for the resulting package version.

https://github.com/CycloneDX/cyclonedx-conan is parsing the user and channel and putting it in the resulting file.

With this MR the same functionality is also added to syft.

@Pro Pro force-pushed the fix-conan-ref-parse branch from 163e3d7 to 9f32dff Compare August 17, 2023 12:59
Copy link
Copy Markdown
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

This looks like a great start! Would you be able to add a test with some realistic samples of conanfile and lockfile entries?

Comment thread syft/pkg/cataloger/cpp/package.go Outdated
Comment thread syft/pkg/cataloger/cpp/package.go Outdated
Comment thread syft/pkg/cataloger/cpp/package.go Outdated
Comment thread syft/pkg/cataloger/cpp/package.go Outdated
Comment thread syft/pkg/cataloger/cpp/package.go Outdated
@Pro Pro force-pushed the fix-conan-ref-parse branch from 9f32dff to 5a4f877 Compare August 17, 2023 14:10
@Pro
Copy link
Copy Markdown
Contributor Author

Pro commented Aug 17, 2023

This looks like a great start! Would you be able to add a test with some realistic samples of conanfile and lockfile entries?

Done, added some tests @kzantow

@Pro Pro force-pushed the fix-conan-ref-parse branch from 5a4f877 to 9d21265 Compare August 17, 2023 14:11
Copy link
Copy Markdown
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

Thanks @Pro, I think the changes to the tests look sufficient; just one minor issue that wasn't resolved

Comment thread syft/pkg/cataloger/cpp/package.go Outdated
@kzantow
Copy link
Copy Markdown
Contributor

kzantow commented Aug 17, 2023

@Pro -- you should be able to fix the static analysis failure by running make lint-fix

@Pro Pro force-pushed the fix-conan-ref-parse branch from 9d21265 to 97072c5 Compare August 17, 2023 17:33
@kzantow
Copy link
Copy Markdown
Contributor

kzantow commented Aug 17, 2023

@Pro it looks like the last commit was not signed-off; could you rebase against main and force push these changes with sign-off?

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>
@Pro Pro force-pushed the fix-conan-ref-parse branch from 97072c5 to 0b7ad64 Compare August 17, 2023 18:25
@Pro Pro requested a review from kzantow August 22, 2023 13:47
Comment thread syft/pkg/cataloger/cpp/package.go Outdated
@kzantow
Copy link
Copy Markdown
Contributor

kzantow commented Aug 22, 2023

Thanks for this PR @Pro -- there is one very small request to not export the ConanRef type. That's the last hold up for this PR, I think.

Do you happen to know much about Conan lockfiles version 0.5? I've been trying to find information about the schema, but I've been unsuccessful. There is a report that the format may have changed significantly in an incompatible manner, and we'd like to make sure to support both older and newer versions.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@wagoodman wagoodman left a comment

Choose a reason for hiding this comment

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

I made the small tweak that @kzantow was referring to, otherwise it looks great! Thanks for the improvement here!

@wagoodman wagoodman enabled auto-merge (squash) August 23, 2023 17:46
@wagoodman wagoodman merged commit 07ac640 into anchore:main Aug 23, 2023
@kzantow kzantow added the bug Something isn't working label Aug 25, 2023
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
)

* fix: properly parse conan ref and include user and channel

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>

* unexport the conanRef type

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Stefan Profanter <stefan.profanter@agile-robots.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Co-authored-by: Alex Goodman <wagoodman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants