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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 40 additions & 13 deletions src/bin/new.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::env;
use std::str::{FromStr};

use cargo::ops;
use cargo::util::{CliResult, Config};
use cargo::util::{CliError, CliResult, Config};

#[derive(RustcDecodable)]
struct Options {
Expand All @@ -12,39 +13,65 @@ struct Options {
arg_path: String,
flag_name: Option<String>,
flag_vcs: Option<ops::VersionControl>,
flag_license: Option<String>,
flag_license_file: Option<String>,
}

pub const USAGE: &'static str = "
pub const USAGE: &'static str = r#"
Create a new cargo package at <path>

Usage:
cargo new [options] <path>
cargo new -h | --help

Options:
-h, --help Print this message
--vcs VCS Initialize a new repository for the given version
control system (git or hg) or do not initialize any version
control at all (none) overriding a global configuration.
--bin Use a binary instead of a library template
--name NAME Set the resulting package name
-v, --verbose Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
";
-h, --help Print this message
--vcs VCS Initialize a new repository for the given version
control system (git or hg) or do not initialize any version
control at all (none) overriding a global configuration.
--bin Use a binary instead of a library template
--name NAME Set the resulting package name
-v, --verbose Use verbose output
-q, --quiet No output printed to stdout
--color WHEN Coloring: auto, always, never
--license LICENSES License(s) to add to a project
Multiple licenses should be separated by a '/' character
Copy link
Member

Choose a reason for hiding this comment

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

Right now crates.io actually also supports some more flavorful strings of licenses, and I'd kinda prefer to avoid parsing license strings everywhere and instead just keep it on crates.io for now. Perhaps this help documentation could just be "add a license to a project" and if it's understood we'll write some files out but if not a warning will be emitted that the license files themselves don't exist?

(Supported values, case insensitive: "MIT", "BSD-3-Clause",
"APACHE-2.0", "GPL-3.0", "MPL-2.0")
--license-file FILE Adds the 'license-file' parameter to your Cargo.toml
"#;

pub fn execute(options: Options, config: &Config) -> CliResult<Option<()>> {
debug!("executing; cmd=cargo-new; args={:?}", env::args().collect::<Vec<_>>());
try!(config.shell().set_verbosity(options.flag_verbose, options.flag_quiet));
try!(config.shell().set_color_config(options.flag_color.as_ref().map(|s| &s[..])));

let Options { flag_bin, arg_path, flag_name, flag_vcs, .. } = options;
let Options { flag_bin, arg_path,
flag_name, flag_vcs,
flag_license, flag_license_file, .. } = options;

let opts = ops::NewOptions {
version_control: flag_vcs,
bin: flag_bin,
path: &arg_path,
name: flag_name.as_ref().map(|s| s.as_ref()),
license: match flag_license {
Some(input) => {
let mut licenses: Vec<ops::License> = vec![];
let split = input.split("/").collect::<Vec<_>>();
for l in &split {
let l = l.trim();
licenses.push(match FromStr::from_str(l) {
Ok(lic) => lic,
_ => return Err(CliError::new(&format!("Unrecognised license '{}'", l),
127)),
});
}
Some(licenses)
},
None => None
},
license_file: flag_license_file.as_ref().map(|s| s.as_ref()),
};

try!(ops::new(opts, config));
Expand Down
154 changes: 149 additions & 5 deletions src/cargo/ops/cargo_new.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
use std::env;
use std::fs;
use std::fmt;
use std::fs::{self, File};
use std::io::prelude::*;
use std::io;
use std::path::Path;
use std::str::{FromStr};

use rustc_serialize::{Decodable, Decoder};

use git2::Config as GitConfig;

use term::color::BLACK;
use time::{strftime, now};

use util::{GitRepo, HgRepo, CargoResult, human, ChainError, internal};
use util::{GitRepo, HgRepo, CargoResult, CargoError, human, ChainError, internal};
use util::{Config, paths};

use toml;
Expand All @@ -22,6 +26,8 @@ pub struct NewOptions<'a> {
pub bin: bool,
pub path: &'a str,
pub name: Option<&'a str>,
pub license: Option<Vec<License>>,
Copy link
Member

Choose a reason for hiding this comment

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

This may be best as &'a [License] as it's what most of the rest of the struct looks like

pub license_file: Option<&'a str>,
}

impl Decodable for VersionControl {
Expand All @@ -38,10 +44,71 @@ impl Decodable for VersionControl {
}
}

#[derive(Clone, Debug, PartialEq)]
pub enum License {
MIT,
BSD3,
APACHE2,
MPL2,
GPL3,
}

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.

file(path, &license_text)
}

fn license_text<F: io::Write>(&self, f: &mut F, holders: &str) -> io::Result<()> {
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 it'd be fine for this to just return a String instead of writing into a writer

let tm = now();
let year = strftime("%Y", &tm).unwrap();
match *self {
// Not sure about this, would like to make this prettier
License::MIT => write!(f, include_str!("../../../src/etc/licenses/MIT"),
year = &year, holders = holders),
License::BSD3 => write!(f, include_str!("../../../src/etc/licenses/BSD3"),
year = &year, holders = holders),
License::APACHE2 => write!(f, include_str!("../../../src/etc/licenses/APACHE2")),
License::MPL2 => write!(f, include_str!("../../../src/etc/licenses/MPL2")),
License::GPL3 => write!(f, include_str!("../../../src/etc/licenses/GPL3")),
}
}
}

impl FromStr for License {
type Err = Box<CargoError>;
fn from_str(s: &str) -> CargoResult<License> {
Ok(match s.to_lowercase().as_ref() {
"mit" => License::MIT,
"bsd-3-clause" => License::BSD3,
"apache-2.0" => License::APACHE2,
"mpl-2.0" => License::MPL2,
"gpl-3.0" => License::GPL3,
_ => return Err(internal(format!("Unknown license {}", s))),
Copy link
Member

Choose a reason for hiding this comment

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

Cargo error messages stylistically start with a lowercase letter

Copy link

Choose a reason for hiding this comment

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

Also, the return is not needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's needed due to the Ok() that covers the whole match.

})
}
}

impl fmt::Display for License {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let s = match self {
Copy link
Member

Choose a reason for hiding this comment

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

Stylistically this typically looks like match *self

&License::MIT => "MIT",
&License::BSD3 => "BSD-3-Clause",
&License::APACHE2 => "Apache-2.0",
&License::MPL2 => "MPL-2.0",
&License::GPL3 => "GPL-3.0",
};
write!(f, "{}", s)
}
}

struct CargoNewConfig {
name: Option<String>,
email: Option<String>,
version_control: Option<VersionControl>,
license: Option<Vec<License>>,
license_file: Option<String>
}

pub fn new(opts: NewOptions, config: &Config) -> CargoResult<()> {
Expand Down Expand Up @@ -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


#[allow(deprecated)] // connect => join in 1.3
fn join_licenses(v: Vec<String>) -> String {
v.connect("/")
Copy link

Choose a reason for hiding this comment

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

Why use connect instead of join?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably for 1.2 compatiblity. I think we dropped that, so it's no longer necessary.

}

fn mk(config: &Config, path: &Path, name: &str,
opts: &NewOptions) -> CargoResult<()> {
let cfg = try!(global_config(config));
Expand Down Expand Up @@ -140,14 +216,66 @@ fn mk(config: &Config, path: &Path, name: &str,
(None, None, name, None) => name,
};

try!(paths::write(&path.join("Cargo.toml"), format!(
let license: Option<Vec<License>> = match (&opts.license, cfg.license) {
(&None, None) => None,
(&Some(ref lic), _) => Some(lic.clone()),
(&_, Some(lic)) => Some(lic.clone()),
};

let license_file: Option<String> = match (&opts.license_file, cfg.license_file) {
Copy link
Member

Choose a reason for hiding this comment

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

These type annotations can likely all be elided (here, above, and below), and it'd be good to reduce the number of .clone() or .to_string() calls around here as well.

(&None, None) => None,
(&Some(ref lic_file), _) => Some(lic_file.to_string()),
Copy link

Choose a reason for hiding this comment

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

Use to_owned() instead, or ideally, avoid these allocations.

(&_, Some(lic_file)) => Some(lic_file.to_string()),
};

let license_options: String = match (license.clone(), license_file) {
(Some(license), Some(license_file)) => {
Copy link
Member

Choose a reason for hiding this comment

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

This case should probably be an error actually (to specify both)

let license = join_licenses(license.iter()
.map(|l| format!("{}", l))
.collect::<Vec<_>>());
format!(r#"license = "{}"
license-file = "{}"
"#, license, license_file)
},
(Some(license), None) => {
let license = join_licenses(license.iter()
.map(|l| format!("{}", l))
.collect::<Vec<_>>());
format!(r#"license = "{}"
"#, license)
Copy link
Member

Choose a reason for hiding this comment

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

To be a bit easier on the eyes it'd be fine for this to just be:

format!("license = \"{}\"\n", ...)

},
(None, Some(license_file)) => {
format!(r#"license-file = "{}"
"#, license_file)
},
(None, None) => {
"".to_owned()
Copy link
Member

Choose a reason for hiding this comment

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

This may still want to be a newline to ensure that the generated Cargo.toml without these options is still the same

}
};

// If there is more than one license, we suffix the filename
// with the name of the license
if license.is_some() {
let license = license.unwrap();
if license.len() > 1 {
for l in &license {
let upper = format!("{}", l).to_uppercase();
try!(l.write_file(&path.join(format!("LICENSE-{}", upper)), &author));
}
} else {
let license = license.get(0).unwrap();
try!(license.write_file(&path.join("LICENSE"), &author))
}
}

try!(file(&path.join("Cargo.toml"), format!(
r#"[package]
name = "{}"
version = "0.1.0"
authors = [{}]

{}
[dependencies]
"#, name, toml::Value::String(author)).as_bytes()));
"#, name, toml::Value::String(author.clone()), license_options).as_bytes()));

try!(fs::create_dir(&path.join("src")));

Expand Down Expand Up @@ -196,6 +324,8 @@ fn global_config(config: &Config) -> CargoResult<CargoNewConfig> {
let name = try!(config.get_string("cargo-new.name")).map(|s| s.0);
let email = try!(config.get_string("cargo-new.email")).map(|s| s.0);
let vcs = try!(config.get_string("cargo-new.vcs"));
let license = try!(config.get_string("cargo-new.license"));
let license_file = try!(config.get_string("cargo-new.license-file"));

let vcs = match vcs.as_ref().map(|p| (&p.0[..], &p.1)) {
Some(("git", _)) => Some(VersionControl::Git),
Expand All @@ -208,10 +338,24 @@ fn global_config(config: &Config) -> CargoResult<CargoNewConfig> {
}
None => None
};
let license: Option<Vec<License>> = match license.as_ref().map(|p| &p.0[..]) {
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?

licenses.push(try!(FromStr::from_str(lic)));
}
Some(licenses)
},
_ => None,
};
let license_file: Option<String> = license_file.as_ref().map(|p| p.0.to_owned());
Copy link
Member

Choose a reason for hiding this comment

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

The type annotation here can probably be elided

Ok(CargoNewConfig {
name: name,
email: email,
version_control: vcs,
license: license,
license_file: license_file,
})
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub use self::cargo_rustc::{BuildOutput, BuildConfig, TargetConfig};
pub use self::cargo_rustc::{CommandType, CommandPrototype, ExecEngine, ProcessEngine};
pub use self::cargo_run::run;
pub use self::cargo_install::{install, install_list, uninstall};
pub use self::cargo_new::{new, NewOptions, VersionControl};
pub use self::cargo_new::{new, NewOptions, VersionControl, License};
pub use self::cargo_doc::{doc, DocOptions};
pub use self::cargo_generate_lockfile::{generate_lockfile};
pub use self::cargo_generate_lockfile::{update_lockfile};
Expand Down
18 changes: 18 additions & 0 deletions src/doc/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,24 @@ email = "..."
# set to `none` to disable this behavior.
vcs = "none"

# By default, no license is added to a new project, but by specifying
# one or more here, `cargo` will add the correct entry to `Cargo.toml`, as
# well as a `LICENSE` file (or multiple LICENSE-* files). Currently this
# supports:
#
# * MIT
# * Apache-2.0
# * BSD-3-Clause
# * MPL-2.0
# * GPL-3.0

license = "MIT/Apache-2.0"

# Whenever a new `cargo` project is generated, it's `Cargo.toml` will
# specify that there is a license file located at "..."

license-file = "..."

# For the following sections, $triple refers to any valid target triple, not the
# literal string "$triple", and it will apply whenever that target triple is
# being compiled to.
Expand Down
2 changes: 2 additions & 0 deletions src/etc/_cargo
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ case $state in
'(-h, --help)'{-h,--help}'[show help message]' \
'(-v, --verbose)'{-v,--verbose}'[use verbose output]' \
'--color=:colorization option:(auto always never)' \
'--license=[license choice]' \
'--license-file=[license file location]' \
;;

owner)
Expand Down
2 changes: 1 addition & 1 deletion src/etc/cargo.bashcomp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ _cargo()
local opt__install="$opt_common $opt_feat $opt_jobs --bin --branch --debug --example --git --list --path --rev --root --tag --vers"
local opt__locate_project="$opt_mani -h --help"
local opt__login="$opt_common --host"
local opt__new="$opt_common --vcs --bin --name"
local opt__new="$opt_common --vcs --bin --name --license --license-file"
local opt__owner="$opt_common -a --add -r --remove -l --list --index --token"
local opt__package="$opt_common $opt_mani -l --list --no-verify --no-metadata"
local opt__pkgid="${opt__fetch}"
Expand Down
Loading