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

Automatically include license files in .dist-info/license_files #862

Merged
merged 10 commits into from
Apr 2, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 2 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

* Automatically include license files in `.dist-info/license_files` following PEP 639 in [#862](https://github.com/PyO3/maturin/pull/862)

## [0.12.11] - 2022-03-15

* Package license files in `.dist-info/license_files` following PEP 639 in [#837](https://github.com/PyO3/maturin/pull/837)
Expand Down
59 changes: 59 additions & 0 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,25 @@ impl Metadata21 {
}
}

// Until PEP 639 is approved with metadata 2.3, we can assume a
// dynamic license-files (also awaiting full 2.2 metadata support)
// We're already emitting the License-Files metadata without issue.
// license-files.globs = ["LICEN[CS]E*", "COPYING*", "NOTICE*", "AUTHORS*"]
let license_include_targets = ["LICEN[CS]E*", "COPYING*", "NOTICE*", "AUTHORS*"];
for pattern in license_include_targets.iter() {
for license_path in glob::glob(&manifest_path.join(pattern).to_string_lossy())
.expect("No license files found for pattern")
Copy link
Member

Choose a reason for hiding this comment

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

the manifest_path needs to be escaped with glob::escape or it'll become part of the pattern. The expect shouldn't happen anyway, but a? would be better than possibly panicking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a fix for the ? usage instead of the expect. I followed the pattern from source_distribution.rs#L317-L328. I can update the code to used Pattern::escape if you like.

    if let Some(include_targets) = sdist_include {
        for pattern in include_targets {
            println!("📦 Including files matching \"{}\"", pattern);
            for source in glob::glob(&manifest_dir.join(pattern).to_string_lossy())
                .expect("No files found for pattern")
                .filter_map(Result::ok)
            {
                let target = root_dir.join(&source.strip_prefix(manifest_dir)?);
                writer.add_file(target, source)?;
            }
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@konstin Is this what you are looking for?

let license_include_targets = ["LICEN[CS]E*", "COPYING*", "NOTICE*", "AUTHORS*"];
let escaped_manifest_string = glob::Pattern::escape(manifest_path.to_str().unwrap());
let escaped_manifest_path = Path::new(&escaped_manifest_string);
for pattern in license_include_targets.iter() {
    for license_path in
        glob::glob(&escaped_manifest_path.join(pattern).to_string_lossy())?
            .filter_map(Result::ok)
    {
//....

Copy link
Member

Choose a reason for hiding this comment

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

yep that looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@konstin Just pushed the change. Thanks for the tip.

.filter_map(Result::ok)
{
// if the pyproject.toml specified the license file,
// then we won't list it as automatically included
if !self.license_files.contains(&license_path) {
println!("📦 Including license file \"{}\"", license_path.display());
self.license_files.push(license_path);
}
}
}

if let Some(authors) = &project.authors {
let mut names = Vec::with_capacity(authors.len());
let mut emails = Vec::with_capacity(authors.len());
Expand Down Expand Up @@ -835,4 +854,44 @@ mod test {
"text/markdown; charset=UTF-8; variant=GFM"
);
}

#[test]
fn test_merge_metadata_from_pyproject_dynamic_license_test() {
let manifest_path = PathBuf::from("test-crates").join("license-test");
let cargo_toml_str = fs_err::read_to_string(&manifest_path.join("Cargo.toml")).unwrap();
let cargo_toml: CargoToml = toml_edit::easy::from_str(&cargo_toml_str).unwrap();
let metadata = Metadata21::from_cargo_toml(&cargo_toml, &manifest_path).unwrap();

// verify Cargo.toml value came through
assert_eq!(metadata.license.as_ref().unwrap(), "MIT");

let license_files_strings: Vec<_> = metadata
.license_files
.iter()
.map(|v| v.to_str().unwrap())
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can just keep everything as PathBuf and drop the unwrap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed fix, thank you!

.collect();

// verify we have the total number of expected licenses
assert_eq!(4, license_files_strings.len());

// Verify pyproject.toml license = {file = ...} worked
assert_eq!(
license_files_strings[0],
manifest_path.join("LICENCE.txt").to_str().unwrap()
);

// Verify the default licenses were included
assert_eq!(
license_files_strings[1],
manifest_path.join("LICENSE").to_str().unwrap()
);
assert_eq!(
license_files_strings[2],
manifest_path.join("NOTICE.md").to_str().unwrap()
);
assert_eq!(
license_files_strings[3],
manifest_path.join("AUTHORS.txt").to_str().unwrap()
);
}
}
1 change: 1 addition & 0 deletions test-crates/license-test/AUTHORS.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
AUTHORS
6 changes: 6 additions & 0 deletions test-crates/license-test/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions test-crates/license-test/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "license-test"
version = "0.1.0"
authors = ["konstin <konstin@mailbox.org>"]
edition = "2018"
license = "MIT"

[dependencies]
1 change: 1 addition & 0 deletions test-crates/license-test/LICENCE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENCE
1 change: 1 addition & 0 deletions test-crates/license-test/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENSE
1 change: 1 addition & 0 deletions test-crates/license-test/NOTICE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# NOTICE
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/AUTHORS.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
AUTHORS
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/COPYING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# COPYING
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/LICENCE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENCE
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/LICENSE-APACHE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENSE-APACHE
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/NOTICE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
NOTICE
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/nested/LICENSE-BSD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENSE-BSD
1 change: 1 addition & 0 deletions test-crates/license-test/_vendor/nested/NOTICE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# NOTICE
12 changes: 12 additions & 0 deletions test-crates/license-test/check_installed/check_installed.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from subprocess import check_output


def main():
output = check_output(["hello-world"]).decode("utf-8").strip()
if not output == "Hello, world!":
raise Exception(output)
print("SUCCESS")


if __name__ == "__main__":
main()
10 changes: 10 additions & 0 deletions test-crates/license-test/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[build-system]
requires = ["maturin>=0.12,<0.13"]
build-backend = "maturin"

[project]
name = "license-test"
license = { file = "LICENCE.txt" }

[tool.maturin]
bindings = "bin"
3 changes: 3 additions & 0 deletions test-crates/license-test/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}