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

rust: Add license to our internal crates, check licenses in CI #2073

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

I was playing with
https://github.com/Nemo157/cargo-lichking
in preparation for adding it to our CI.

I thought of this when I came across https://gitlab.com/woshilapin/with_tempdir/
and then noticed it was lgplv3, and wondered whether
we e.g. had any incompatible licenses in our dependency graph.
According to this (and looking at the output of cargo lichking list)
we're good. But we need to keep this in mind.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rust/Cargo.toml Outdated
@@ -2,6 +2,10 @@
name = "rpmostree-rust"
version = "0.1.0"
authors = ["Colin Walters <[email protected]>", "Jonathan Lebon <[email protected]>"]
# This is kind of a lie, we keep the Rust code MIT / Apache 2.0 but we also
# directly depend on the systemd crate, and really this is "license of the whole"
# as opposed "license of the individual code files"
Copy link
Member

Choose a reason for hiding this comment

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

I think I need some help with this. Why can't we put the MIT/Apache 2 license here? If we theoretically published this crate on crates.io, wouldn't that be the license we'd use? Re. systemd/libdnf, it's fine for MIT apps to link to LGPL libraries, no?

Copy link
Member Author

@cgwalters cgwalters May 11, 2020

Choose a reason for hiding this comment

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

So...yeah I kind of tried to do some handwaving here 👋 but you caught me 😉
Basically I tried setting it to "MIT / Apache 2.0" but cargo-lichking didn't like that and I didn't understand why and didn't want to spend the time on it at the time...

Just marking this one as WIP for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I think actually the checker is right here; we want to make a claim on the final license, which is definitely LGPLv2+.

@cgwalters cgwalters changed the title rust: Add license to our internal crates WIP: rust: Add license to our internal crates May 11, 2020
@cgwalters cgwalters force-pushed the license-internals branch 2 times, most recently from 1c50b83 to 3b09007 Compare February 1, 2021 17:46
Start using
https://github.com/Nemo157/cargo-lichking
to validate our crates.

I thought of this when I came across https://gitlab.com/woshilapin/with_tempdir/
and then noticed it was lgplv3, and wondered whether
we e.g. had any incompatible licenses in our dependency graph.
According to this (and looking at the output of `cargo lichking list`)
we're good.  But we need to keep this in mind.
@cgwalters cgwalters changed the title WIP: rust: Add license to our internal crates rust: Add license to our internal crates, check licenses in CI Feb 2, 2021
@cgwalters
Copy link
Member Author

OK I think this works, lifting WIP. But I also think we should move this job to Prow, so
/hold
until openshift/release#15461 merges.

@cgwalters
Copy link
Member Author

I did some more investigation of this space and I think coreos/bootupd#149 is better, will do another PR.

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

Successfully merging this pull request may close these issues.

3 participants