Skip to content

Fix #351 License override does not work for third-party dependencies with multiple licenses - #374

Merged
slawekjaranowski merged 11 commits into
mojohaus:masterfrom
lama0206:master
Jun 30, 2023
Merged

slawekjaranowski merged 11 commits into
mojohaus:masterfrom
lama0206:master

Conversation

@lama0206

Copy link
Copy Markdown
Contributor

As discussed in #351, the current license override overrides only the first license of a multi-license artifact. The update - also as discussed in #351 - removes/overrides all licenses and logs this action on the INFO level.

@lama0206 lama0206 closed this Apr 20, 2020
@lama0206

lama0206 commented Apr 20, 2020 •

Copy link
Copy Markdown
Contributor Author

Sorry for spamming the pull request with failing integration tests. Figured out how to run the integration tests locally :)

This is how the log messages look like (taken from the ISSUE-351 integration test's log file:

[INFO] Overriding license(s) for dependency [org.glassfish.jersey.containers--jersey-container-servlet--2.30.1] with [(Eclipse Public License 2.0)], overriden license(s): [(Apache License, 2.0) (BSD 2-Clause) (EDL 1.0) (EPL 2.0) (GPL2 w/ CPE) (MIT license) (Modified BSD) (Public Domain) (W3C license) (jQuery license)]

@lama0206 lama0206 reopened this Apr 20, 2020
Comment thread src/it/ISSUE-351/postbuild.groovy Resolved
@lama0206

Copy link
Copy Markdown
Contributor Author

Ye, lines 178-181 don't make sense. I'll take a look at it this weekend.

@lama0206 lama0206 changed the title Override all licenses for projects from override Fix #351 License override does not work for third-party dependencies with multiple licenses Jun 20, 2021
licenseMap.removeProject( project );
// remove project only removes first occurrence of project from license -> project[] map.
List<String> removedFrom = licenseMap.removeProject( project );
LOG.info( "Overriding license(s) for dependency [{}] with [{}], overriden license(s): [{}]",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not good print these information to log as info. This plugin is already much verbose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I agree - yes, it could be considered verbose yet overriding a license can be a major issue which is why I agree with digulla's comment in general.

I'd rather print one summary line (for example, "Overriding license(s) for 123 dependencies. Enable debug log for details.") iff a license has been overridden instead of silently pushing the info down to LOG.debug entirely.

What do you think?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree with @lama0206

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After testing with multi module projects with long override list I think that log in debug level will be enough.

@profTwinglings

Copy link
Copy Markdown

Hi do you think this will be merged?

@ClintonFowler

Copy link
Copy Markdown

Interested in this Fix as well. Wondering if this could be prioritized and moved into the next patch.

@slawekjaranowski slawekjaranowski self-assigned this Jun 29, 2023
@slawekjaranowski

Copy link
Copy Markdown
Member

merged with master
simple cleanups

@slawekjaranowski slawekjaranowski added the bug Something isn't working label Jun 30, 2023
@slawekjaranowski
slawekjaranowski merged commit 82f70ae into mojohaus:master Jun 30, 2023
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.

License override does not work for third-party dependencies with multiple licenses

6 participants