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

Allow licenses to be specified during cargo new #2013

Closed

Conversation

pwoolcoc
Copy link
Contributor

This pull request would allow a user to specify a license to include
with their project. If the user specifies a license, a line is added
to Cargo.toml specifying the license, and in the case of some
licenses, the actual LICENSE file is added to the initial project.

A user can put their choice(s) of license in a .cargo/config file,
like so:

[cargo-new]
license = "MIT/Apache-2.0"

or it can be specified on the command line when a project is created:

$ cargo new project-name --license "MIT/Apache-2.0"

There are 5 licenses that I have included the actual LICENSE files for:

  • MIT
  • Apache-2.0
  • BSD-3-Clause
  • MPL-2.0
  • GPL-3.0

If the user chooses any of these licenses, the corresponding LICENSE
file is generated and added to their project. In the case of multiple
licenses, each LICENSE file has a suffix added to the file name. So,
if a project was dual-licensed "MIT/Apache-2.0", their project would
include the files LICENSE-MIT and LICENSE-APACHE-2.0.

If the license is not one of these, the entry is still added to
Cargo.toml, and a LICENSE file is still added to the repository,
though it is content-less.

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@pwoolcoc
Copy link
Contributor Author

I did a short survey of the licenses used in projects on crates.io while I was preparing this PR, in case you wanted some reasoning about why I included the licenses I did: http://paul.woolcock.us/posts/crates-io-license-survey.html.

I added the GPL to the list after some feedback on the article.

@pwoolcoc pwoolcoc force-pushed the add-license-config-to-cargo-new branch 2 times, most recently from 70d8bb5 to eca61d5 Compare September 30, 2015 19:06
@alexcrichton
Copy link
Member

Whoa! This is awesome, thanks for doing this @pwoolcoc! Some thoughts of mine:

  • An unknown --license may want to generate an error rather than an empty license file, but perhaps --license-file and [cargo-new.license-file] could be supported options as well? That corresponds to what the manifest accepts as well at least.
  • Things like the MIT license need a year/copyright holder name, and it looks like those aren't filled in just yet?
  • Perhaps all the licenses could be pre-formatted to an 80-character line limit?
  • I wonder if we may want to start warning on a missing --license parameter perhaps?
  • Could you also update the documentation about configuration, and perhaps even about cargo new to indicate that a license is possible to pass?

cc @brson, @wycats, sure you guys will have opinions as well!

@brson
Copy link
Contributor

brson commented Sep 30, 2015

Like the idea. Agree that bogus licenses should generate an error.

Since this includes a bunch 3rd-party text in the product, I am wondering what the copyright status of the licenses themselves is, and how that impacts Cargo's licensing. This isn't a question I've ever considered before.

@eefriedman
Copy link

The license for the GPL is "Everyone is permitted to copy and distribute verbatim copies of this license document, but changing it is not allowed.".

@apasel422
Copy link
Contributor

Perhaps the license files that contain copyright year and holder information could be modified to use the formatting syntax like

Copyright (c) {year} {holder}

along with a method like

impl License {
    fn write_text(&self, f: &mut std::fmt::Formatter, year: u32, holder: Option<&str>)
        -> std::fmt::Result
    {
        match *self {
            License::MIT => write!(f, include_str!("../../../src/etc/licenses/MIT"),
                               year = year, holder = holder.unwrap_or("(Unknown)")),
            License::APACHE2 => write!(f, "{}", include_str!("../../../src/etc/licenses/APACHE2")),
            ...
        }
    }
}

in order to fill in the values where applicable.

@pwoolcoc pwoolcoc force-pushed the add-license-config-to-cargo-new branch from 59f2af7 to 0a27bde Compare October 6, 2015 02:28
@bors
Copy link
Collaborator

bors commented Oct 6, 2015

☔ The latest upstream changes (presumably #2022) made this pull request unmergeable. Please resolve the merge conflicts.

@brson
Copy link
Contributor

brson commented Oct 8, 2015

@gerv I'm interested in your opinion about the legalities of bundling a bunch of license texts into Cargo, re my earlier comment.

@gerv
Copy link

gerv commented Oct 14, 2015

License texts vary - some are "no modifications" (like the GPL), some are "you can make derivative works but you have to change the name" (like the MPL). However, license texts themselves are generally considered exceptions to a project's licensing policy, as long as they are distributable (as all open source license texts are). Some people argue that for software to be open source, it has to use a license whose text is itself open source, but that view is a minority one.

So I'd say bundle away. I think the license list you have is good - I certainly wouldn't add any others. The BSD license and the MIT license are equivalent in effect, and both are deprecated in favour of licenses which actually address patents specifically. The Rust license is partly MIT so you probably have to include that, but if you left out BSD-3-Clause, I would not weep a single tear. The licenses you have left - Apache -> MPL 2 -> GPL 3.0 - cover the broad spectrum of copyleft options pretty well.

Gerv

@pwoolcoc
Copy link
Contributor Author

Thanks @gerv. I need to rebase this branch and finish off a couple minor things from @alexcrichton 's list, and then I will ask about getting it merged again.

@sfackler
Copy link
Member

Have you had a chance to get back to this @pwoolcoc?

@pwoolcoc
Copy link
Contributor Author

Yea, I mentioned in #rust yesterday that I am just getting back to this. I am working on this tonight a bit, hopefully I can get it merge-able either tonight or tomorrow.

Paul Woolcock added 6 commits December 19, 2015 21:37
This pull request would allow a user to specify a license to include
with their project. If the user specifies a license, a line is added
to `Cargo.toml` specifying the license, and in the case of some
licenses, the actual `LICENSE` file is added to the initial project.

A user can put their choice(s) of license in a `.cargo/config` file,
like so:

```toml
[cargo-new]
license = "MIT/Apache-2.0"
```

or it can be specified on the command line when a project is created:

```bash
$ cargo new project-name --license "MIT/Apache-2.0"
```

There are 5 licenses that I have included the actual LICENSE files for:

  * MIT
  * Apache-2.0
  * BSD-3-Clause
  * MPL-2.0
  * GPL-3.0

If the user chooses any of these licenses, the corresponding `LICENSE`
file is generated and added to their project. In the case of multiple
licenses, each `LICENSE` file has a suffix added to the file name. So,
if a project was dual-licensed "MIT/Apache-2.0", their project would
include the files `LICENSE-MIT` and `LICENSE-APACHE-2.0`.

If the license is not one of these, the entry is still added to
`Cargo.toml`, and a `LICENSE` file is still added to the repository,
though it is content-less.
@pwoolcoc pwoolcoc force-pushed the add-license-config-to-cargo-new branch from 0a27bde to 0d77f7e Compare December 20, 2015 05:40
@pwoolcoc
Copy link
Contributor Author

  • An unknown --license may want to generate an error rather than an empty license file, but perhaps --license-file and [cargo-new.license-file] could be supported options as well? That corresponds to what the manifest accepts as well at least.

I have implemented both these.

  • Things like the MIT license need a year/copyright holder name, and it looks like those aren't filled in just yet?

Done.

  • Perhaps all the licenses could be pre-formatted to an 80-character line limit?

Done.

  • I wonder if we may want to start warning on a missing --license parameter perhaps?

Haven't touched this, as I didn't see any additional feedback about this.

  • Could you also update the documentation about configuration, and perhaps even about cargo new to indicate that a license is possible to pass?

I think I got all the docs updated, let me know if I missed something.

@pwoolcoc
Copy link
Contributor Author

r? @alexcrichton

Some(s) => {
let r = &s[..];
let mut licenses: Vec<License> = vec![];
for lic in r.split("/") {
Copy link
Member

Choose a reason for hiding this comment

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

This logic happens in a few places, perhaps it could be consolidated?

@@ -99,6 +166,15 @@ fn existing_vcs_repo(path: &Path, cwd: &Path) -> bool {
GitRepo::discover(path, cwd).is_ok() || HgRepo::discover(path, cwd).is_ok()
}

fn file(p: &Path, contents: &[u8]) -> io::Result<()> {
try!(File::create(p)).write_all(contents)
}
Copy link
Member

Choose a reason for hiding this comment

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

There should be a helper function in util::paths for this I believe

@alexcrichton
Copy link
Member

The logic here for writing out the LICENSE file itself makes me a little uncomfortable, but I'm not sure if there's much we can do about it. It definitely seems to be standard practice to have a file like that, but it involves Cargo parsing licenses and emitting files, both of which are a little unfortunate.

I'd love to take a more conservative approach to start off with by not worrying about foo / bar licenses or things like that and instead just emit one LICENSE file if the string is recognized, and perhaps emitting a warning otherwise. I'm not sure how feasible that'd be though?

@pwoolcoc
Copy link
Contributor Author

I will remove the foo/bar stuff, if necessary, but I had originally implemented it because of the prevalence of the MIT/Apache-2.0 licensed projects on crates.io. I am afraid that, if that is removed, this feature becomes a lot less useful.

@gerv
Copy link

gerv commented Dec 29, 2015

MIT/Apache 2.0 is the license of Rust. It would be a very bad idea not to have that be an option (heck, even the default) for Rust project license generation.

@tbu-
Copy link
Contributor

tbu- commented Jan 11, 2016

One possibility would be to special-case MIT/Apache 2.0 if you don't want to support dual-licensing in general.

@alexcrichton
Copy link
Member

Hm yes in retrospect let's just special case the MIT/Apache-2.0 license for now. The error message can help guide towards that if it's what's intended, and we don't have to worry about parsing license strings in Cargo itself.

@bors
Copy link
Collaborator

bors commented Jan 24, 2016

☔ The latest upstream changes (presumably #2081) made this pull request unmergeable. Please resolve the merge conflicts.

@vi
Copy link
Contributor

vi commented Jan 24, 2016

Shall I try adapting this for cargo init?

@alexcrichton
Copy link
Member

@vi yeah I suspect that this would want to modify cargo init now as well, @pwoolcoc would you be ok rebasing this and addressing the last few comments?

@pwoolcoc
Copy link
Contributor Author

@alexcrichton sure, I'll try to get back to this tonight.

impl License {
fn write_file(&self, path: &Path, holders: &str) -> io::Result<()> {
let mut license_text: Vec<u8> = Vec::new();
let _ = self.license_text(&mut license_text, holders);
Copy link

Choose a reason for hiding this comment

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

This isn't shouldn't be silently ignored.

@alexcrichton
Copy link
Member

This has been inactive for some time now, so I'm going to close this, but feel free to resubmit with a rebase!

@vi
Copy link
Contributor

vi commented Feb 26, 2016

Is it a good idea to "take over" and re-submit this thing myself instead of waiting for original author?

@pwoolcoc
Copy link
Contributor Author

@vi FWIW, I don't have a problem with it if you pick it up -- I just haven't had a chance to get back to it, if you have the time & motivation, awesome!

@vi
Copy link
Contributor

vi commented Apr 1, 2016

Is scanning existing LICENSE or COPYING file and filling in license: or license-file: in cargo init a good idea?

@gerv
Copy link

gerv commented Apr 1, 2016

No. Detecting licenses is the task of a dedicated license scanner; you don't want to try implementing that code. :-) You'd be amazed at how many variants there are.

@ticki
Copy link

ticki commented Apr 1, 2016

@gerv You can easily make a program covering 99% of all cases.

@vi
Copy link
Contributor

vi commented Apr 1, 2016

@gerv, Maybe detect not the license itself, but just license file and then fill in license-file: COPYING?

@gerv
Copy link

gerv commented Apr 1, 2016

Oh, I see. That seems less problematic if you just look for files called COPYING, LICENSE, LICENCE or the equivalent ".txt" versions.

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.