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

Introduce minimal source coordinates #623

Merged
merged 3 commits into from Nov 18, 2021
Merged

Introduce minimal source coordinates #623

merged 3 commits into from Nov 18, 2021

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Nov 10, 2021

Today we have source.Location to represent any and all location information. This includes the real path (path with no links), virtual path (path that may have links), filesystem ID (image layer ID), and file.Reference resolved from stereoscope. All of this information is useful in different circumstances.

However, there are critical use cases for having an object that has the minimal amount of information to be able to find a location within a source. This minimal information is real path and filesystem ID. The use case in question is being able to craft an index used within sbom.SBOM.Artifacts without necessarily having virtual path or a file.Reference. If you've indexed a location with a virtual path but want to access that index with only real path information then it would not be possible without creating additional indexes.

This PR introduces a new source.Coordinates object that contains this minimal information and changes all maps within sbom.SBOM.Aritfacts to use source.Coordinates as keys (instead of source.Location).

This PR helps to unblock anchore/grype#395 and #476 while further helping along #556.

Note: this PR base branch is first-class-relationships which is under review under PR #607 . This PR will remain in draft until #607 is merged.

@github-actions
Copy link

github-actions bot commented Nov 10, 2021

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                                   old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2          1.68ms ± 1%    1.71ms ±11%    ~     (p=1.000 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2        4.28ms ± 9%    4.10ms ± 5%    ~     (p=0.095 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     978µs ± 1%     977µs ± 3%    ~     (p=0.690 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                1.17ms ± 1%    1.16ms ± 3%    ~     (p=0.841 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  986µs ± 1%    1019µs ± 3%  +3.39%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                  13.4ms ± 1%    13.2ms ± 3%    ~     (p=0.095 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                 1.60ms ± 1%    1.58ms ± 3%    ~     (p=0.841 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2      1.75µs ± 1%    1.61µs ± 3%  -7.77%  (p=0.008 n=5+5)

name                                                   old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           285kB ± 0%     286kB ± 0%  +0.37%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2        1.29MB ± 0%    1.29MB ± 0%  +0.22%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     211kB ± 0%     212kB ± 0%  +0.31%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 269kB ± 0%     269kB ± 0%  +0.32%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  234kB ± 0%     234kB ± 0%  +0.10%  (p=0.016 n=5+4)
ImagePackageCatalogers/java-cataloger-2                  3.37MB ± 0%    3.36MB ± 0%  -0.20%  (p=0.016 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                 1.34MB ± 0%    1.35MB ± 0%  +0.03%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2        480B ± 0%      480B ± 0%    ~     (all equal)

name                                                   old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           8.90k ± 0%     8.93k ± 0%  +0.32%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2         38.3k ± 0%     38.5k ± 0%  +0.34%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     5.80k ± 0%     5.82k ± 0%  +0.24%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 8.51k ± 0%     8.55k ± 0%  +0.49%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  7.09k ± 0%     7.10k ± 0%  +0.20%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                   66.8k ± 0%     66.9k ± 0%  +0.08%  (p=0.008 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                  11.1k ± 0%     11.1k ± 0%  +0.26%  (p=0.016 n=4+5)
ImagePackageCatalogers/go-module-binary-cataloger-2        11.0 ± 0%      11.0 ± 0%    ~     (all equal)

Base automatically changed from first-class-relationships to main November 16, 2021 19:14
@wagoodman wagoodman requested a review from a team November 16, 2021 19:20
@wagoodman wagoodman self-assigned this Nov 16, 2021
@wagoodman wagoodman marked this pull request as ready for review November 16, 2021 19:24
Copy link
Contributor

@luhring luhring left a comment

Choose a reason for hiding this comment

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

Looks cool!

syft/source/coordinates.go Outdated Show resolved Hide resolved
syft/source/location.go Show resolved Hide resolved
syft/source/location.go Show resolved Hide resolved
…addressing

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman enabled auto-merge (squash) November 18, 2021 18:08
@wagoodman wagoodman merged commit e38cde3 into main Nov 18, 2021
@wagoodman wagoodman deleted the split-location branch November 18, 2021 18:13
spiffcs added a commit that referenced this pull request Nov 18, 2021
* main:
  Introduce minimal source coordinates (#623)
fengshunli pushed a commit to fengshunli/syft that referenced this pull request Jan 24, 2022
* split source.Location and create source.Coordinates for minimal path addressing

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* move coordinates into separate file

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* Update syft/source/coordinates.go

Co-authored-by: Dan Luhring <luhring@users.noreply.github.com>
Signed-off-by: fsl <1171313930@qq.com>
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* split source.Location and create source.Coordinates for minimal path addressing

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* move coordinates into separate file

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>

* Update syft/source/coordinates.go

Co-authored-by: Dan Luhring <luhring@users.noreply.github.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