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

Show licenses of crates on a per version basis #736

Closed
12 tasks done
est31 opened this issue May 22, 2017 · 9 comments
Closed
12 tasks done

Show licenses of crates on a per version basis #736

est31 opened this issue May 22, 2017 · 9 comments
Labels
C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear E-has-mentor E-help-wanted

Comments

@est31
Copy link
Member

est31 commented May 22, 2017

Follow up of #227.

Currently, crates.io maintains license information on a per crate basis. I'd propose to base it on the (crate, version) tuple instead.

The issue is that it might mislead people to believe that the license of a crate is something different from what it actually is.

Like for example, take someone creating a crate with portions of code from someone else, licensed under the MPL. Now assume this was the reason to license the remainder of the crate under the MPL as well, and version 0.1 is published as MPL. Now the author finds out that the functionality of that external code is already implemented by someone else on crates.io and removes it, adding a dependency instead. This allows the author to relicense under the more custom Apache2/MIT duo, which what the author then does, releasing version 0.2 of the crate under Apache2/MIT.

Now, take some user who for some reason thinks that version 0.1 is better than version 0.2 and wants to use it instead. The user looks at the crates.io page of the version of the crate, and sees its licensed under Apache2/MIT which is okay, while the MPL would be not okay for them. They then mistakenly use the 0.1 version without knowing its in fact MPL licensed!

Or take another case (inspired by the winehq story): Some person happily releases version after version of their crate under the Apache2/MIT license, until they discover that someone else took their crate, improved it and now sells the library in exchange for money. The author doesn't like that and decides to relicense the crate under the LGPL license, in order to be able to compete with the proprietary library (so that not every improvement can automatically be imported to the proprietary library). Now the future versions of the crate are LGPL licensed. Does that change the license of the old versions though? No it doesn't! Once you license some code under an open source license, it can be continued to be used under that license, even if you reconsider. Only changes you make to that code can be put under a new license. So crates.io saying that the older versions are LGPL as well might scare away users who would use that crate if it were Apache2/MIT licensed and can live with older versions.


Tasks

Ok so I (@carols10cents) think what needs to be done here is:

  • Make a diesel migration to add a license column to the versions table
  • Write a script in src/bin that, for all current crates. will copy the value of crate.license to each of that crate's version.license values to backfill that column for existing crates
  • Instead of passing license and license file to NewCrate and NewCrate.create_or_update (here), pass license and license file to NewVersion::new (here)
  • Instead of passing the license file to NewCrate.validate and calling validate_license from NewCrate.validate, move the validate_license method to be a private method on NewVersion instead (in here), and call validate_license from NewVersion::new. If the license/license file isn't valid, this should make NewVersion::new fail, which should make the crate publish fail.
  • Instead of returning license as part of EncodableCrate, return it as part of EncodableVersion
  • Update the existing tests as necessary with the above changes
  • Add some tests to the version tests and the crate tests that create two versions of a crate with different licenses and test that the different licenses show up in the JSON returned from version index example test, version show example test, and crate show example test
  • Update the mirage fixtures used by the frontend tests to have the license under version instead of under crate
    • Remove from crate in crate.js, search.js, and summary.js
    • Add to versions in crate.js and crate_versions.js
  • Change the Ember models to remove license from crate and add it to version
  • Change the version template from displaying the license from crate.license to, I think, currentVersion.license (it might be requestedVersion? I forget)
  • I don't think any of the frontend tests are testing the display of the license, if it's not too terrible, add some frontend tests that check that the license displayed changes when you look at different versions of a crate that has different licenses. If it is terrible to add tests for this, check this by hand.
  • Definitely in a separate PR from any and all of the above, and definitely after all of the above are complete, add a migration to remove the license column from the crates table

I will happily accept PRs that complete parts of this, since we can just have the frontend continue to display the crate.license while this work is partially complete.

@withoutboats
Copy link
Contributor

This seems like a good change to make. We don't wnat people to be mislead about what the licensing is if they're considering using an outdated version of a crate.

@Nemo157
Copy link
Member

Nemo157 commented Jun 2, 2017

Now the future versions of the crate are LGPL licensed. Does that change the license of the old versions though? No it doesn't! Once you license some code under an open source license, it can be continued to be used under that license, even if you reconsider. Only changes you make to that code can be put under a new license.

Technically you can stop publishing old versions of the code under the old license. Anyone that already has the code under the old license (crates.io in this case) can continue to use the code (and re-publish if allowed by the license) under the old license. So, since the code has already been provided to crates.io under license terms that allow crates.io to re-publish the code under the given license we're fine, but in general you can change the license under which you publish existing code.

This actually makes me wonder if crates.io should have some sort of "by publishing your code here you're giving us a perpetual license to re-publish your code" TOS, as far as I can see there's nothing stopping someone publishing a crate that lacks a license allowing crates.io to re-publish it. (I took a brief look at the crates.io footer, "Publishing on crates.io", "FAQ" and "Policies" docs pages and found nothing related to this.)

@est31
Copy link
Member Author

est31 commented Jun 2, 2017

@Nemo157 afaik you need to provide an open source license when uploading to crates.io.

@Nemo157
Copy link
Member

Nemo157 commented Jun 4, 2017

That was my assumption as well, but where is this enforced/mentioned at all? I just checked the source and it definitely allows non-open source licenses to be uploaded. As I mentioned above I could not find anything mentioned in the docs on crates.io itself. To make sure I just searched the source of both crates.io and cargo for the string license but couldn't find anything saying you must provide an open source license.

@steveklabnik
Copy link
Member

This actually makes me wonder if crates.io should have some sort of "by publishing your code here you're giving us a perpetual license to re-publish your code" TOS

It should; it's been on my plate for a while but I have not gotten to it.

@carols10cents carols10cents added A-versions E-help-wanted C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works E-has-mentor labels Jun 6, 2017
@carols10cents
Copy link
Member

I've added to the description a bunch of tasks that I think need to happen to complete this, and I've marked this issue medium because of all these tasks. I think each individual task should be fairly straightforward and E-easy though, and I'm happy to merge PRs that take care of one or a few of these tasks-- we can have the frontend keep using crate.license until the backend changes are complete.

As always, please let me know if you are thinking about working on this and have questions! I will make sure PRs get linked to this and try to keep the checkboxes in the description up-to-date.

@carols10cents
Copy link
Member

@baileyn finished most of this and it's deployed!! Keeping this open as a reminder to get rid of the crates table license column since we're not using it anymore.

Thank you @baileyn!!! 🎉 🌮

@carols10cents carols10cents added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear and removed C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works labels Jul 23, 2017
bors added a commit that referenced this issue Sep 5, 2019
Remove `license` column on `crates` table

Seemingly the last step in the checklist on #736, this PR removes the `license` column on the `crates` table, and all references thereto. (In fact, I think #736 can be closed once this PR is merged.)

As of #803, the front-end no longer really used this field, and the front-end was also changed to render the license of the _current_ version, instead of the value coming from this now-obsolete `license` column on the crate. Thus, this column likely contains stale data and, to my knowledge, isn't changed at all when a new version is uploaded.

I wasn't sure what requirements are enforced for backwards-compatibility, or whether my changes here would actually affect behavior of e.g. if there are old versions of `cargo` that will upload with these values. I am happy for these changes to be scrutinized until this is verified to be safe.
@rye
Copy link
Contributor

rye commented Sep 6, 2019

Was this finished by #1815? (I didn't set it up to auto-close in case there were more things to get done, but that PR did remove the table.)

@carols10cents
Copy link
Member

Yep, I think this is done! Thank you everyone!!! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear E-has-mentor E-help-wanted
Projects
None yet
Development

No branches or pull requests

6 participants