Skip to content

SBOMER-79: Check MANIFEST.MF and license text files for licenses #1050

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

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

dwalluck
Copy link
Collaborator

No description provided.

@dwalluck dwalluck force-pushed the license-manifest branch 4 times, most recently from d297e30 to af99827 Compare May 29, 2024 19:03
Copy link
Contributor

@vibe13 vibe13 left a comment

Choose a reason for hiding this comment

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

wow @dwalluck this is a massive improvement you made.
I have added a few comments and some more aliases mapping for the license-mapping.json file, which IIUC will handle unknown aliases. After some tests I have found some license which were not detected correctly so it would be good to add them to the list.

Now, an unfortunately pleasant part .... (can be handled in a separate PR)... sorry for this.
I have seen that you aggregate the licenses for a given build, inside the final "builds.json" file. But, in the intermediate "licenses.json" file which you create, you have the licenses collected per each archive. In order to use the licenses information with precision, I am asking if it's possible to keep the granularity of your license identification, meaning that I would like the final "builds.json" file to have the license information in the corresponding "archive", not in the parent "buildInfo". This is because at the moment if I have a parent pom, whose build will produce a lot of archives, the "build.json" will list all the aggregated licenses in the parent pom "buildInfo", but the parent pom might list only 1 license, while all the others might be related to different submodules/archives. And for the license reports we need to build out of this information, understanding which archive declared which license is a requirement. I apologize if I realized it so late, I will open a new JIRA for this. WRT this JIRA, if you add the additional mappings inside the license-mapping.json, I would be happy to approve! Thanks!!

(created SBOMER-85)

@dwalluck
Copy link
Collaborator Author

dwalluck commented May 30, 2024

archive. In order to use the licenses information with precision, I am asking if it's possible to keep the granularity of your license identification, meaning that I would like the final "builds.json" file to have the license information in the corresponding "archive", not in the parent "buildInfo". This is because at the moment if I have a parent pom, whose build will produce a lot of archives, the "build.json" will list all the aggregated licenses in the parent pom "buildInfo", but the parent pom might list only 1 license, while all the others might be related to different submodules/archives. And for the license reports we need to build out of this information, understanding which archive declared which license is a requirement. I apologize if I realized it so late, I will open a new JIRA for this. WRT this JIRA, if you add the additional mappings inside the license-mapping.json, I would be happy to approve! Thanks!!

(created SBOMER-85)

I think that this should be possible (especially if the licenses.json file already has this information). I agree that it should be in a separate PR/ticket.

I was thinking about why I chose to store it with the build instead.

I think my reasoning was this: Say that a build produces 10 artifacts, but only 1 has the license (e.g., the parent POM). Logically, all 10 artifacts should share this same license, so 1 build = 10/10 = 100% of artifacts have a license. Now, under the new scheme, 1/10 artifacts = 10% have a license.

But this is OK, if this is the way we want it to be.

@vibe13
Copy link
Contributor

vibe13 commented Jun 3, 2024

@dwalluck That is correct and I was thinking about that as well: if a module does not have a license in its pom, but its parent has a license, should we imply that the module has a license? I think we should, but if we lose that information during the merging of results, we lose the ability to chose later which assumption we should make. So let's keep all the info with the most precise granularity and then decide what to do with it. Thanks!!

@dwalluck
Copy link
Collaborator Author

dwalluck commented Jun 3, 2024

@dwalluck That is correct and I was thinking about that as well: if a module does not have a license in its pom, but its parent has a license, should we imply that the module has a license? I think we should, but if we lose that information during the merging of results, we lose the ability to chose later which assumption we should make. So let's keep all the info with the most precise granularity and then decide what to do with it. Thanks!!

Actually, I agree. We should just try to give accurate information (when possible) and let the other tools decide what to do with that information.

@dwalluck dwalluck force-pushed the license-manifest branch from af99827 to 199c026 Compare June 3, 2024 14:06
@vibe13
Copy link
Contributor

vibe13 commented Jun 5, 2024

@dwalluck I have run one more test with the latest fixes and the results are good! However now one license is not correctly detected anymore. Now I get:

    "url" : "http://repository.jboss.org/licenses/cc0-1.0.txt",
    "spdxLicenseId" : "NONE",

while before I correctly got:

    "url" : "http://repository.jboss.org/licenses/cc0-1.0.txt",
    "spdxLicenseId" : "CC0-1.0",

@dwalluck dwalluck force-pushed the license-manifest branch from 199c026 to aed64ac Compare June 7, 2024 14:12
@vibe13
Copy link
Contributor

vibe13 commented Jun 14, 2024

@dwalluck I have done more tests with your latest PRs and everything seems to be great, I got the "CC0-1.0" licenses in consistent way and I don't see anything missing. Excellent job. Thanks !!

Copy link
Contributor

@vibe13 vibe13 left a comment

Choose a reason for hiding this comment

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

Everything look great. Excellent work David!

@dwalluck dwalluck merged commit feb4ae4 into project-ncl:master Jun 14, 2024
11 checks passed
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