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

GH-579: Add and fix LICENSE.txt and NOTICE.txt in the distributed artifacts #578

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

jbonofre
Copy link
Member

@jbonofre jbonofre commented Jan 31, 2025

Fixes #579.

In our distributed artifacts, especially the shading ones, we have to provide LICENSE.txt and NOTICE.txt with all "bundled" dependencies.

@lidavidm @kou This is a draft PR as I have to fix the non-shading artifacts.

@jbonofre jbonofre marked this pull request as draft January 31, 2025 13:08
@jbonofre
Copy link
Member Author

@kou @lidavidm if you have time, you can already take a look on the content. Please let me know if you have any question.

@lidavidm
Copy link
Member

BTW, how were these generated? Or do they have to be assembled by hand?

@jbonofre
Copy link
Member Author

BTW, how were these generated? Or do they have to be assembled by hand?

For this release, I did that "manually". After the release, I will add the maven tooling to at least check the content.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

Thanks!

@kou kou changed the title Add and fix LICENSE.txt and NOTICE.txt in the distributed artifacts GH-579: Add and fix LICENSE.txt and NOTICE.txt in the distributed artifacts Feb 1, 2025
@@ -202,16 +202,21 @@
limitations under the License.

--------------------------------------------------------------------------------
vector/src/main/java/org/apache/arrow/vector/util/IntObjectHashMap.java
vector/src/main/java/org/apache/arrow/vector/util/IntObjectMap.java
This product includes code from Netty 4.1.117.Final:
Copy link
Member

Choose a reason for hiding this comment

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

Is it required to include the specific version? It would be easily out of sync since the dependabot does not maintain this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately (or not 😄 ), yes, It's important to specify the version of the dependency as licenses sometimes change as product versions change.
Also the NOTICE can change from a version to another.
So, it's better to document to actual version bundled in our distributed jar.

So source distribution, we should document the version where the code has been copied from.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation! If we don't have good automation tools to keep them in sync, at least we need to make sure they are accurate in the release process.

@jbonofre
Copy link
Member Author

jbonofre commented Feb 2, 2025

I'm fixing the location and other jar resources.

@kou
Copy link
Member

kou commented Feb 3, 2025

Should we merge PRs from Dependabot such as #584 after the next release?
I think that they may change dependency versions in binary artifacts.

@jbonofre
Copy link
Member Author

jbonofre commented Feb 3, 2025

Should we merge PRs from Dependabot such as #584 after the next release? I think that they may change dependency versions in binary artifacts.

We can merge dependabot PRs, I will update this PR accordingly. That's totally fine for me. I need ~ 1 day to fix the last "jar" creation.

@jbonofre
Copy link
Member Author

jbonofre commented Feb 4, 2025

@kou thanks ! I'm updating this PR accordingly.

@jbonofre jbonofre marked this pull request as ready for review February 4, 2025 10:10
@jbonofre
Copy link
Member Author

jbonofre commented Feb 4, 2025

I'm fixing the endline thing.

@kou
Copy link
Member

kou commented Feb 5, 2025

@jbonofre Can we merge this?

@jbonofre
Copy link
Member Author

jbonofre commented Feb 5, 2025

@jbonofre Can we merge this?

Yes, we are good 👍

@lidavidm lidavidm merged commit 993536f into apache:main Feb 5, 2025
25 checks passed
@jbonofre
Copy link
Member Author

jbonofre commented Feb 5, 2025

@lidavidm @kou thanks for the review and merge. I'm doing a quick new run but I think we are good for the release.

@kou
Copy link
Member

kou commented Feb 5, 2025

Thanks! I'll create cut an RC.

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.

LICENSE.txt/NOTICE.txt miss dependencies for binary artifacts
4 participants