-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 npm sbom command #6801
Conversation
9ca6625
to
c5bff64
Compare
c3c14d7
to
a0cf3bc
Compare
This looks good from a code standpoint. Waiting on a rough consensus in the rfc before landing. We'll land this, give it a few weeks in case there are implementation bugs we missed, update the RFC to reflect those if needed, then land the RFC. |
a0cf3bc
to
d7785e9
Compare
Signed-off-by: Brian DeHamer <bdehamer@github.com>
d7785e9
to
4fb6e7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for SPDX the directions of the relationships are not consistent (since SPDX provides sometimes both directions, like DEPENDS_ON
and DEPENDENCY_OF
)
const REL_PREREQ = 'HAS_PREREQUISITE' | ||
const REL_OPTIONAL = 'OPTIONAL_DEPENDENCY_OF' | ||
const REL_DEV = 'DEV_DEPENDENCY_OF' | ||
const REL_DEP = 'DEPENDS_ON' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to double check, but the direction might be wrong:
const REL_DEP = 'DEPENDS_ON' | |
const REL_DEP = 'DEPENDENCY_OF' |
At least the other relationships are in the other direction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maxhbr I was chatting with one of the SPDX contributors about this and the guidance I got was that the "directionality" of the relationship is implied by the spdxElementId
/relatedSpdxElement
fields not by the relationshipType
-- many of the types don't have a supported inverse, so I just picked whichever one best conveys the type of the relationship w/o regard for the direction.
Perhaps, I should switch this one just to be consistent, but it was the one case where there was a supported type that actually matched the direction of the relationship.
My understanding is that this is going to get better in SPDX 3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the directionality is implied by the spdxElementId
/relatedSpdxElement
fields, in combination with the type of the relationship.
The following two relationships are equivalent
spdxElementId: PackageA
relatedSpdxElement: PackageB
relationshipType: DEPENDS_ON
and
spdxElementId: PackageB
relatedSpdxElement: PackageA
relationshipType: DEPENDENCY_OF
The direction of the type of relationship is still significant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your case probably the DEPENDS_ON
is the right choice, and the other mappings should be adopted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to clarify the comment above:
this does not resolve the issue. It is just a statement that, when choosing between DEPENDS_ON
and DEPENDENCY_OF
the first should probably be preferred (aligning with #6867 (comment)).
But still the issue is valid and the relationshipTypes
are inconsistently used here.
Signed-off-by: Brian DeHamer <bdehamer@github.com>
Signed-off-by: Brian DeHamer <bdehamer@github.com>
Signed-off-by: Brian DeHamer <bdehamer@github.com>
Adds a new
sbom
command to generate a Software Bill of Materials (SBOM) for the current project.No new dependencies are introduced, however
normalize-package-data
andspdx-expression-parse
are elevated to top-level dependencies.References
Original RFC: npm/rfcs#714