Skip to content

add support for resolving jars' licenses#53

Merged
kezhenxu94 merged 2 commits intoapache:mainfrom
MoGuGuai-hzr:tony
Aug 12, 2021
Merged

add support for resolving jars' licenses#53
kezhenxu94 merged 2 commits intoapache:mainfrom
MoGuGuai-hzr:tony

Conversation

@MoGuGuai-hzr
Copy link
Contributor

No description provided.

@wu-sheng
Copy link
Member

Could you be clear in the PR comments? What is your resolving way?

@kezhenxu94 kezhenxu94 added this to the 0.2.0 milestone Aug 11, 2021
@kezhenxu94 kezhenxu94 self-requested a review August 11, 2021 15:20
@MoGuGuai-hzr
Copy link
Contributor Author

In fact, this is not one new implement. In the last pr, we searched for the license files that the maven project depends on from two places:

  • searching in jar file for license from license[.txt] copying, manifest file and so on;
  • searching in AID-Version.pom file for license from comments the head of the file and license element in xml

Now we separate jar file resolver from maven resolver to support that resolves a series of jar files entered by the user

@MoGuGuai-hzr
Copy link
Contributor Author

Could you be clear in the PR comments? What is your resolving way?

I just submitted a description

@wu-sheng
Copy link
Member

We need a document to list the license resolving mechanism. Not just for Maven, for all supported language ecosystems.
This would be very helpful for users when they face license check fails. Such as having wrong mixed licenses, wrong in higher priority place, but a right and compatible one in lower priority place.

FYI @kezhenxu94

@wu-sheng
Copy link
Member

Let's create an issue to track this document requirements.

Copy link
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

This feature is for a scenario where users already have their distribution packages and check the final .jar files, the key point is that we need to expand all .jar files in the patter like:

dependency:
  files:
    - go.mod
    - dist/**/*.jar

return nil, err
}

if _, err := exec.Command("mvn", "dependency:copy-dependencies", "-DoutputDirectory=./lib", "-DincludeScope=runtime").Output(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This is not portable, users / developers don't have mvn installed won't be able to run this test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The purpose of this test is to check the usability of the JarResolver. Can I put the jar files directly in the test/testdata?

Copy link
Member

Choose a reason for hiding this comment

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

The purpose of this test is to check the usability of the JarResolver. Can I put the jar files directly in the test/testdata?

you can put binary files (.jar) files in the codebase but we can't release them, this is unusual case, maybe we can only run this test case when mvn is available, otherwise we skip this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can put binary files (.jar) files in the codebase but we can't release them, this is unusual case, maybe we can only run this test case when mvn is available, otherwise we skip this test?

ok, i will check the availability of mvn first


func (resolver *JarResolver) Resolve(jarFile string, report *Report) error {
state := NotFound
if err := resolver.ResolveJar(&state, jarFile, report); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

In this resolver (jar resolver), we should support a path pattern like dist/**/*.jar and expand all the .jar files and resolve their licenses, the current feature in this PR only support a singe .jar file, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, JarResolver is only responsible for resolve the licenses of jar files. And deps/config.go will enter all jar files in the dist/**/*.jar

Copy link
Member

Choose a reason for hiding this comment

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

From my understanding, JarResolver is only responsible for resolve the licenses of jar files. And deps/config.go will enter all jar files in the dist/**/*.jar

Well, now, it seems deps/config.go doesn't expand the pattern dist/**/*.jar to all .jar files (like dist/module1/log4j.jar, dist/module2/junit.jar), we should do this, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, now, it seems deps/config.go doesn't expand the pattern dist/**/*.jar to all .jar files (like dist/module1/log4j.jar, dist/module2/junit.jar), we should do this, right?

Yes, it will be complete in the next pr

Copy link
Member

Choose a reason for hiding this comment

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

Well, now, it seems deps/config.go doesn't expand the pattern dist/**/*.jar to all .jar files (like dist/module1/log4j.jar, dist/module2/junit.jar), we should do this, right?

Yes, it will be complete in the next pr

Nice

@kezhenxu94 kezhenxu94 merged commit 03dbef4 into apache:main Aug 12, 2021
@wu-sheng
Copy link
Member

How far are we from using this in main repo and the next release?

@MoGuGuai-hzr MoGuGuai-hzr deleted the tony branch August 12, 2021 05:28
@kezhenxu94
Copy link
Member

kezhenxu94 commented Aug 12, 2021

How far are we from using this in main repo and the next release?

For now this feature is only helpful to assist human to audit the licenses, the key to make it available in main repo (automatically report conflict licenses and fail the CI) is to provide a license compatibility matrix and check the final resolved licenses. We might provide another command license-eye dependency check, (the current command is license-eye dependency resolve for human usage)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants