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

source: plumb sym link base through to syft CLI #1830

Closed
wants to merge 1 commit into from

Conversation

jsquyres
Copy link
Contributor

Continuing the work of b81c980, change the behavior of the syft command: the root for symbol link resolution of directory scans is now the directory root, not "/".

Do this pedantically, by separating the directory root to the scanned from the directory sym link base. As of this commit, these two values will always be the same, but this strategy leaves the door open for them to be different someday, if that would ever be useful.

Also remove some unused functions and consolidate down to just the NewFromDirectoryBaseWithName() function.

Refs #1485, #1359

Continuing the work of b81c980, change the behavior of the syft
command: the root for symbol link resolution of directory scans is now
the directory root, not "/".

Do this pedantically, by separating the directory root to the scanned
from the directory sym link base.  As of this commit, these two values
will always be the same, but this strategy leaves the door open for
them to be different someday, if that would ever be useful.

Also remove some unused functions and consolidate down to just the
NewFromDirectoryBaseWithName() function.

Signed-off-by: Jeff Squyres <jeff@squyres.com>
@jsquyres
Copy link
Contributor Author

Looks like some of those "unused" functions I removed are used by tests. I'll investigate...

@kzantow
Copy link
Contributor

kzantow commented Jun 15, 2023

Hi @jsquyres and thanks for the contribution here -- we have a bit of a different change that does this in PR #1867. It should be noted the referenced PR has a prerequisite of some other changes and we're looking at getting these done hopefully in a week-ish timeframe. That said, I'm going to close this PR due to the overlap. Please let me know if I've misunderstood something and would be happy to reopen it!

@jsquyres
Copy link
Contributor Author

@kzantow Sorry for my long silence on this -- I got distracted by vacation, holidays, and other shiny objects. If #1867 implements effectively the same kind of functionality as I did in this PR, we can certainly close this PR. I'd just like the functionality, regardless of whether it's my PR or someone else's. 😄

@kzantow
Copy link
Contributor

kzantow commented Jun 22, 2023

Thanks @jsquyres! I'm going to close this as noted -- we're making progress

@kzantow kzantow closed this Jun 22, 2023
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.

2 participants