#217 Add %cargo_license and %cargo_license_summary macros
Opened 2 months ago by decathorpe. Modified 2 months ago
fedora-rust/ decathorpe/rust2rpm cargo-license  into  main

file modified
+38
@@ -109,6 +109,44 @@ 

  fi                                                                  \

  )

  

+ # This macro writes license information for all crates in the dependency tree

+ # that are neither dev-dependencies nor proc-macros to a file,

+ # LICENSE.dependencies. Additionally, deprecated SPDX syntax is normalized

+ # before sorting the results to ensure consistent order.

+ %cargo_license(naf:)\

+ %{shrink:\

+     %{__cargo} tree                                                 \

+     --workspace                                                     \

+     --offline                                                       \

+     --edges normal,no-proc-macro                                    \

+     --no-dedupe                                                     \

+     --target all                                                    \

+     %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}}                \

+     --prefix none                                                   \

+     --format "{l}: {p}"                                             \

+     | sed -e "s: ($(pwd)[^)]*)::g" -e "s: / :/:g" -e "s:/: OR :g"   \

+     | sort -u                                                       \

+     > LICENSE.dependencies                                          \

+ }

+ 

+ # This macro prints license summary for all crates in the dependency tree that

+ # are neither dev-dependencies nor proc-macros. Additionally, deprecated SPDX

+ # syntax is normalized before sorting the results to ensure consistent order.

+ %cargo_license_summary(naf:)\

+ %{shrink:\

+     %{__cargo} tree                                                 \

+     --workspace                                                     \

+     --offline                                                       \

+     --edges normal,no-proc-macro                                    \

+     --no-dedupe                                                     \

+     --target all                                                    \

+     %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}}                \

+     --prefix none                                                   \

+     --format "# {l}"                                                \

+     | sed -e "s: / :/:g" -e "s:/: OR :g"                            \

+     | sort -u                                                       \

+ }

+ 

  %__cargo_feature_from_name(n:) %{lua:

  local name = rpm.expand("%{-n*}")

  local feature = string.match(name, "^[^+]+%+(.+)-devel$")

This PR adds %cargo_license macros based on the cargo tree command devised by @alebastr:

  • %cargo_license: writes list of $license: $crate v$version to a file (LICENSE.dependencies)
  • %cargo_license_summary: prints license summary (without crate names / versions)

Both macros replace / with OR and ensure consistent whitespace in SPDX expressions, such that the output of sort -u is consistent and sorted correctly.

Additionally, I've added the no-proc-macro filter to the tree command to also exclude proc-macro dependencies from the query. Procedural macros are used for code generation, but their source code is not linked into final binaries (similar to build-only dependencies).

The macros should also support the -a, -f foo,bar and -n flags (translating to --all-features, --features foo,bar, and --no-default-features, respectively) that the other %cargo_* macros support, though I have not tested this yet.

What I have tested is that using %cargo_license writes the LICENSE.dependencies file correctly, and that the output of %cargo_license_summary is correct and matches what we've done manually until now.

An additional benefit of using these macros would be that they already produce SPDX expressions, even if not all crate dependencies have not been regenerated with rust2rpm v22 for SPDX "License" tags yet.


Caveat: These macros only take the license value in Cargo.toml manifests into account. If there are packages where we apply manual fixes to the "License" tag in the spec file but don't patch the SPDX expression in Cargo.toml, this will not be reflected. But I expect the number of these cases to be small, and we should fix them anyway (or make it a policy that if the License tag needs to be manually modified, the Cargo.toml file also MUST be patched to reflect the correct license).


Fixes: #198

Though I'm not sure how to integrate these macros with rust2rpm yet.

rebased onto 7fd3ec3

2 months ago

rebased onto ae7554c

2 months ago

Rebased to (hopefully) include a fix for workspace crates (where $(pwd) might not be the full path to local crates).

Some clarification of how these macros are intended to be used would be helpful. My understanding is that %cargo_license_summary is just intended to be run once in %prep on each update to determine the license field and then not be included in the specfile when pushed to distgit, but I'm not really not sure about the other one.

I'm not sure that this is the best approach. What do you think about creating a script that create a mock chroot, installs all of the dependencies, and then runs the same commands in there using mock --shell?

What do you think about creating a script that create a mock chroot, installs all of the dependencies, and then runs the same commands in there using mock --shell?

mock -r fedora-rawhide-x86_64 --spec $name.spec --source . -N --enablerepo=local --short-circuit=prep

will setup a clean chroot, install all of the dependencies (including dynmaic ones), and run %prep.

I thought about adding %cargo_license and %cargo_license_summary to %build for packages that build binaries (either at the start or at the end of %build, it really does not matter) on an opt-in basis. That way you could just copy the license summary from the build log of your local test build into the .spec file before committing all changes, and the %cargo_license macro would generate the complete, up-to-date license breakdown for you, so it will always be up-to-date with the latest buildroot content.

You could even keep the macros in the .spec file permanently, since running these commands has no negative effects, except in some rare circumstances. For example, if a binary crate can be built with optional features, but which are not enabled by default - in which case the new macros will complain about those missing dependencies when calculating the dependency tree.

And in many of these "rare" cases, we might even want to enable those optional features (I came across an issue like that with askalono-cli, where I enabled some additional, useful functionality that was previously disabled).

@decathorpe explained to me separately that this approach is simpler for him, especially when doing chainbuilds where the mock --shell approach won't work. In terms of the macro's implementation, it looks okay to me, but I think you can remove all the backslash escapes within the %{shrink: besides the first one (so there'd be one backslash after %cargo_license[...] and then the following line would remain as %{shrink:\).

Hum, I'm not sure about the need for escaped newlines in %shrink. I just followed the example of all other macros in that file, since I don't know RPM macro syntax that well.

Hum, I'm not sure about the need for escaped newlines in %shrink. I just followed the example of all other macros in that file, since I don't know RPM macro syntax that well.

Something that I think we can discuss is whether %cargo_license should print to a file or to stdout by default, and packagers can pipe the output into a file of their choosing if they want to.

Something that I think we can discuss is whether %cargo_license should print to a file or to stdout by default, and packagers can pipe the output into a file of their choosing if they want to.

Something that I think we can discuss is whether %cargo_license should print to a file or to stdout by default, and packagers can pipe the output into a file of their choosing if they want to.

I think it's better to print something that short to stdout. It's tedious to have to go and find the file in the correct mock buildroot directory. I suppose | tee would be the best of both worlds.

Something that short? The output of %cargo_license looks like this, for a package with few dependencies: https://src.fedoraproject.org/rpms/rust-askalono-cli/blob/rawhide/f/LICENSE.dependencies

Printing to stdout would be more flexible, I guess ... and then the packager could pipe the output wherever they wanted (including to tee).

The output of %cargo_license

Ah, I confused this macro with %cargo_license_summary. You are indeed correct.

the packager could pipe the output wherever they wanted (including to tee).

I don't that will work, because the whole line would be passed to the parametric macro, unless you use the %{macroname} syntax. Then, you'd still have to make sure that the macro definition doesn't have any (non-escaped) trailing newlines (that's the case right now). I just meant you could add |tee filename to the end of macro instead of using > filename redirection.

The other thing that I noticed is that %cargo_license_summary does not combine identical yet differently ordered identifiers. E.g. for rust-askalono-cli:

# Apache-2.0
# Apache-2.0 OR BSL-1.0
# Apache-2.0 OR MIT
# MIT
# MIT OR Apache-2.0
# MIT OR Apache-2.0 OR Zlib
# Unlicense OR MIT
# Zlib OR Apache-2.0 OR MIT

Fixing this might be beyond the scope of what you're trying to do here, but I was able to easily combine the list above in Python like this:

licenses = list_from_above.splitlines()
licenses = [license[2:] for license in licenses]
license_set = {frozenset(license.replace(" ", "").split("OR")) for license in licenses}
for line in license_set:
    print("#", " OR ".join(line))
Metadata