Bug 2332550 - Review Request: trustee-guest-components - attest and get secrets from Trustee
Summary: Review Request: trustee-guest-components - attest and get secrets from Trustee
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Cole Robinson
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/confidential-conta...
Whiteboard:
: 2328647 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-12-16 09:49 UTC by Uri Lublin
Modified: 2025-01-21 14:46 UTC (History)
12 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2025-01-21 14:46:59 UTC
Type: ---
Embargoed:
crobinso: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8397231 to 8401489 (1.99 KB, patch)
2024-12-16 23:33 UTC, Fedora Review Service
no flags Details | Diff
A simple python script to sort spec-file license lines of Rust packages (733 bytes, text/x-python3)
2025-01-20 11:28 UTC, Uri Lublin
no flags Details

Description Uri Lublin 2024-12-16 09:49:56 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08395873-trustee-guest-components/trustee-guest-components.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08395873-trustee-guest-components/trustee-guest-components-0.10.0%5E124.git0061d03-1.fc42.src.rpm

Description: Running in a confidential VM, gather confidential-computing evidence,
send it to Trustee and get secrets.
A part of the confidential-containers project

Fedora Account System Username: uril

Notes:
- Upstream git repository is https://github.com/confidential-containers/guest-components.
- There are not yet crates of this project in crates.io.

Comment 1 Fedora Review Service 2024-12-16 09:52:19 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8397231
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2332550-trustee-guest-components/fedora-rawhide-x86_64/08397231-trustee-guest-components/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Uri Lublin 2024-12-16 09:56:08 UTC
*** Bug 2328647 has been marked as a duplicate of this bug. ***

Comment 3 Uri Lublin 2024-12-16 13:39:50 UTC
rust2rpm.toml:
[package]
summary = "Tools that run in confidential VMs, attest and get secrets from Trustee"
description = "Running in a confidential VM, gather confidential-computing evidence,\nsend it to Trustee and get secrets.\nA part of the confidential-containers project"
url = "https://github.com/confidential-containers/guest-components"
source-url = "https://github.com/confidential-containers/guest-components/archive/0061d036509e451597f5f61492b41849b36a32a1.zip"
extra-sources = [
              ]

extra-patches = [
              { "number" = 1, "file" = "0001-Fedora-Remove-workspace-members-which-are-not-built.patch" , "comments" = [ "Remove workspace members which are not built" ] },
              { "number" = 2, "file" = "0002-Fedora-AA-deps-crypto-default-to-openssl.patch" , "comments" = [ "deps/crypto defaults to openssl" ] },
              { "number" = 3, "file" = "0003-Fedora-kbs_protocol-Cargo.toml-reqwest-use-native-TL.patch" , "comments" = [ "use native-tls for reqwest" ] },
              { "number" = 4, "file" = "0004-Fedora-remove-jwt-simple-dependency.patch" , "comments" = [ "remove dependency jwt-simple - not in Fedora" ] },
              { "number" = 5, "file" = "0005-Fedora-remove-ttrpc-dependency.patch" , "comments" = [ "remove dependency ttrpc - not in Fedora" ] },
              { "number" = 6, "file" = "0006-Fedora-attester-pick-attesters-in-all-attesters.patch" , "comments" = [ "pick attesters to build" ] },
              { "number" = 7, "file" = "0007-Fedora-remove-testcontainers-dependency.patch" , "comments" = [ "remove dependency testcontainers - not in Fedora" ] },
              { "number" = 8, "file" = "0008-Fedora-kbs_protocol-default-to-openssl.patch" , "comments" = [ "kbs_protocol defaults to openssl" ] },
              { "number" = 9, "file" = "0009-Fedora-rstest-0.23.patch" , "comments" = [ "rstest version is 0.23" ] },
              { "number" = 10, "file" = "0010-Fedora-use-clap-4.5.19.patch" , "comments" = [ "clap version is 4.5.19" ] },
              { "number" = 11, "file" = "0011-Fedora-kbs-types-0.8.0.patch" , "comments" = [ "kbs-types version is 0.8.0" ] },
              { "number" = 12, "file" = "0012-Fedora-kbs_protocol-Cargo.toml-add-package.license.patch" , "comments" = [ "add package.license to kbs_protocol/Cargo.toml" ] },
              { "number" = 13, "file" = "0013-Fedora-tokio-version-is-1.41.patch" , "comments" = [ "tokio version is 1.41" ] },
              { "number" = 14, "file" = "0014-Fedora-url-version-is-2.5.2.patch" , "comments" = [ "url version is 2.5.2" ] },
              ]

extra-files = [
              "%doc trustee-attester-README.md",
              ]
supported-arches = ["x86_64"] # == exclusive-arch # or add it manually
cargo-install-bin = true
cargo-install-lib = false


[scripts]
prep.pre  =   [
    "# Cargo.lock isn't uptodate and rpmbuild set --offline", 
    "rm -f Cargo.lock",
    "# Work in kbs_protocol directory",
    "cd attestation-agent/kbs_protocol",
]
build.pre = [
    "cd attestation-agent/kbs_protocol",
]
build.post = [
    "mv LICENSE.dependencies ../../",
]
install.pre = [
    "cd attestation-agent/kbs_protocol",
    "mkdir -p %{buildroot}%{_docdir}/%{name}",
    "install -m 0644 src/bin/trustee-attester/README.md %{buildroot}%{_docdir}/%{name}/trustee-attester-README.md",
]
check.pre = [
    "cd attestation-agent/kbs_protocol",
]

# Manually fix what's missing:
# name:
# vesion:
# version (snapshot) is of format <latest-release>^<number>.<scm><revision>
# where the last part is built out of: git describe --tags
# matching the tarball name
#
# license:
# %cargo_generate_buildrequires
# Copy all comments with license strings
# Fix LICENSE at the top by concatenating these strings with AND between them
#
# files:
# Fix %files:
# Add LICENSE, README.md, trustee-attester
# Remove any binary or other files.
# ---
# %doc     README.md
# %doc     trustee-attester-README.md
# %license LICENSE
# %license LICENSE.dependencies

Comment 4 Uri Lublin 2024-12-16 13:52:28 UTC
How was the spec-file created:
1. Clone upstream git
git clone https://github.com/confidential-containers/guest-components
2. cd guest-components
3. Decide on a commit
c=main # or a specific commit e.g. to create the same spec-file as was uploaded
c=$(git log -1 --pretty=%H $c)
4. Check out the commit into a new branch 'fedora_packaging'
git checkout -b fedora_packaging $c
5. Find the version relative to last release
v=$(git describe $c --tags | sed 's/^v//')
echo $v
6. Download the zip file of $c from github:
curl -LO https://github.com/confidential-containers/guest-components/archive/$c.zip
8. Apply all the Fedora specific patches
# git am <all patches>
# git format-patch -N $c..HEAD # or copy the patches over
7. Edit the rust2rpm.toml (or verify it contains the right values for)
# a. source-url contains the tarball
# b. extra-patches contains all the patches
10. rust2rpm --path . kbs_protocol
11. mv kbs_protocol.spec trustee-guest-components.spec
12. edit trustee-guest-components.spec and fix name, version, license, and files
# a. Name is trustee-guest-components
# b. Version is a snapshot so replace in $v the first '-' with '^' and the '-g' with '.git'
#    echo $v | sed 's/\-/^/; s/\-g/.git/;'
# c. License: TEMPORARILY write "Apache-2.0" (to be fixed later, in step 15)
# d. %prep: fix autosetup: replace kbs_protocol-%{vesion} with the output of $(echo guest-components-$c)
# NOT DONE e. %generate_buildrequires: add 'cd attestation-agent/kbs_protocol' above %cargo_generate_buildrequires
# NOT DONE    (It's probably not important, but that's what I did -- similar to other sections)
# f. %files:
#   I) move trustee-attester-README.md a bit up - together with README.md
#  II) Remove evidence_getter (possibly keep it now?)
# III) Make sure LICENSE exists
13. rpmbuild ... |& tee rpmbuild.OUT
14. Fix all dependencies that require vendor fixing (if any).
    This can happen if a dependency package was upgraded in Fedora.
    for each such fix: git commit + git format-patch + add to extra-patches in rust2rpm.toml
    go back to 13
15. Fix License:
  a. copy from rpmbuild.OUT the license comments (search for Apache-2.0)
  b. "License" is a concatenation of them in a single line.
     Add parenthesis around every line and AND between them.
     Leave a space before/after every license string -- e.g. ( Apache-2.0 OR ... ) and not (Apache-2.0 OR ...)
  If you modified the spec-file, rebuild the package (go back to 13)
16. Build on copr (local build was successful)

Comment 5 Cole Robinson 2024-12-16 16:04:49 UTC
(In reply to Uri Lublin from comment #3)
> rust2rpm.toml:
...

> extra-sources = [
>               ]

guess you can drop this bit, or is there a reason it's empty?

> , "comments" = [ "clap version is 4.5.19" ] },

Interesting that this seems to work fine when upstream's version string is says they explicitly want 4.2. Do you know what that's about? Maybe want to raise with upstream and mention in the patch comment. 


>               { "number" = 12, "file" =
> "0012-Fedora-kbs_protocol-Cargo.toml-add-package.license.patch" , "comments"
> = [ "add package.license to kbs_protocol/Cargo.toml" ] },

Something to raise with upstream?

>               { "number" = 13, "file" =
> "0013-Fedora-tokio-version-is-1.41.patch" , "comments" = [ "tokio version is
> 1.41" ] },

tokio 1.42 is in fedora now, you can drop this



Does the package build without the un-reviewed cvm/vtpm bits enabled? If it's simple to drop those, maybe do that, and then enable them in a follow up commit once packages land, so this package isn't blocked.

Comment 6 Uri Lublin 2024-12-16 18:24:02 UTC
Thank you Cole for reviewing !

(In reply to Cole Robinson from comment #5)
> (In reply to Uri Lublin from comment #3)
> > rust2rpm.toml:
> ...
> 
> > extra-sources = [
> >               ]
> 
> guess you can drop this bit, or is there a reason it's empty?

Yes, it can be dropped.

> 
> > , "comments" = [ "clap version is 4.5.19" ] },
> 
> Interesting that this seems to work fine when upstream's version string is
> says they explicitly want 4.2. Do you know what that's about? Maybe want to
> raise with upstream and mention in the patch comment. 

Without it (commenting out Patch10 in the spec-file) the build fails with:
	(crate(clap/default) >= 4.2.7 with crate(clap/default) < 4.3.0~) is needed by trustee-guest-components-0.10.0^124.git0061d03-1.fc42.x86_64
	(crate(clap/derive) >= 4.2.7 with crate(clap/derive) < 4.3.0~) is needed by trustee-guest-components-0.10.0^124.git0061d03-1.fc42.x86_64


Running "dnf list rust-clap*-devel" shows there are
rust-clap2 and rust-clap3 packages but no rust-clap4.2 packages.


Commit 2d8dcd3 message says:
     versions: Downgrade clap
    
    - Downgrade clap to get to a version that builds on rust 1.69
    - clap 4.3 states that it requires rust 1.65, but it pulls in
    clap_lex 0.5 as a dependency, which requires rust 1.70.
    The newest version of clap_lex that will build on 1.69 is 0.4 and
    the newest version of clap that depends on 0.4 is currently 4.2.7,
    which is how I got to this version
    
    Fixes: #336
    Signed-off-by: stevenhorsman <steven.com>
---

In Fedora rust is 1.83 and rust-clap_lex-devel is 0.7.4.

I can change it back to "4" instead of "4.5.19" if it's better.


> 
> 
> >               { "number" = 12, "file" =
> > "0012-Fedora-kbs_protocol-Cargo.toml-add-package.license.patch" , "comments"
> > = [ "add package.license to kbs_protocol/Cargo.toml" ] },
> 
> Something to raise with upstream?

I'll ask upstream about it.

> 
> >               { "number" = 13, "file" =
> > "0013-Fedora-tokio-version-is-1.41.patch" , "comments" = [ "tokio version is
> > 1.41" ] },
> 
> tokio 1.42 is in fedora now, you can drop this

Yes, I'll drop it.

> 
> Does the package build without the un-reviewed cvm/vtpm bits enabled? If
> it's simple to drop those, maybe do that, and then enable them in a follow
> up commit once packages land, so this package isn't blocked.

It can be built with an added patch to remove the Azure attesters, so that
only snp-attester is enabled, or by modifying
0006-Fedora-attester-pick-attesters-in-all-attesters.patch
diff --git a/attestation-agent/attester/Cargo.toml b/attestation-agent/attester/Cargo.toml
index 4e16347..1451560 100644
--- a/attestation-agent/attester/Cargo.toml
+++ b/attestation-agent/attester/Cargo.toml
@@ -34,8 +34,6 @@ required-features = ["bin"]
 [features]
 default = ["all-attesters"]
 all-attesters = [
-    "az-snp-vtpm-attester",
-    "az-tdx-vtpm-attester",
     "snp-attester",
 ]

Comment 7 Fabio Valentini 2024-12-16 19:09:19 UTC
(In reply to Uri Lublin from comment #6)

(...)

> > > , "comments" = [ "clap version is 4.5.19" ] },
> > 
> > Interesting that this seems to work fine when upstream's version string is
> > says they explicitly want 4.2. Do you know what that's about? Maybe want to
> > raise with upstream and mention in the patch comment. 
> 
> Without it (commenting out Patch10 in the spec-file) the build fails with:
> 	(crate(clap/default) >= 4.2.7 with crate(clap/default) < 4.3.0~) is needed
> by trustee-guest-components-0.10.0^124.git0061d03-1.fc42.x86_64
> 	(crate(clap/derive) >= 4.2.7 with crate(clap/derive) < 4.3.0~) is needed by
> trustee-guest-components-0.10.0^124.git0061d03-1.fc42.x86_64
> 
> 
> Running "dnf list rust-clap*-devel" shows there are
> rust-clap2 and rust-clap3 packages but no rust-clap4.2 packages.
> 
> 
> Commit 2d8dcd3 message says:
>      versions: Downgrade clap
>     
>     - Downgrade clap to get to a version that builds on rust 1.69
>     - clap 4.3 states that it requires rust 1.65, but it pulls in
>     clap_lex 0.5 as a dependency, which requires rust 1.70.
>     The newest version of clap_lex that will build on 1.69 is 0.4 and
>     the newest version of clap that depends on 0.4 is currently 4.2.7,
>     which is how I got to this version
>     
>     Fixes: #336
>     Signed-off-by: stevenhorsman <steven.com>
> ---
> 
> In Fedora rust is 1.83 and rust-clap_lex-devel is 0.7.4.
> 
> I can change it back to "4" instead of "4.5.19" if it's better.

We can only provide one package per *major version*. So clap v2, clap v3, and clap v4.

The reason for a projects to restrict to a specific *minor* version of clap (compatibility with older versions of the Rust compiler) do not apply for packaging purposes, so these can be relaxed from ">=4.2.7,<4.3" to "^4.2.7" (equivalent to ">=4.2.7,<5").

Comment 8 Uri Lublin 2024-12-16 19:46:38 UTC
Thanks Fabio, I'll change it to "^4.2.7"

Comment 9 Cole Robinson 2024-12-16 20:08:32 UTC
(In reply to Uri Lublin from comment #6)

> > Does the package build without the un-reviewed cvm/vtpm bits enabled? If
> > it's simple to drop those, maybe do that, and then enable them in a follow
> > up commit once packages land, so this package isn't blocked.
> 
> It can be built with an added patch to remove the Azure attesters, so that
> only snp-attester is enabled, or by modifying
> 0006-Fedora-attester-pick-attesters-in-all-attesters.patch
> diff --git a/attestation-agent/attester/Cargo.toml
> b/attestation-agent/attester/Cargo.toml
> index 4e16347..1451560 100644
> --- a/attestation-agent/attester/Cargo.toml
> +++ b/attestation-agent/attester/Cargo.toml
> @@ -34,8 +34,6 @@ required-features = ["bin"]
>  [features]
>  default = ["all-attesters"]
>  all-attesters = [
> -    "az-snp-vtpm-attester",
> -    "az-tdx-vtpm-attester",
>      "snp-attester",
>  ]

snp-attester is enough for initial package review IMO, that's useful functionality on its own.
Will let you parallellize the review process a bit.

Comment 10 Uri Lublin 2024-12-16 23:20:08 UTC
New build with the following changes:
- rust2rpm.toml: removed "extra-sources = []"
- Patch0006: pick only snp-attester, remove az-???-vtpm attesters
  + This changed the License: no line with a single ISC in it.
  + Make it possible to build the package in Fedora without additional dependencies.
- Patch0010: clap version is "^4.2.7" instead of "4.5.19"
- Previous patch 0013-Fedora-tokio-version-is-1.41.patch dropped

Spec URL: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08399419-trustee-guest-components/trustee-guest-components.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08399419-trustee-guest-components/trustee-guest-components-0.10.0%5E124.git0061d03-1.fc42.src.rpm

Comment 11 Uri Lublin 2024-12-16 23:28:48 UTC
(In reply to Uri Lublin from comment #10)
> New build with the following changes:
> - rust2rpm.toml: removed "extra-sources = []"

rust2rpm.toml:
[package]
summary = "Tools that run in confidential VMs, attest and get secrets from Trustee"
description = "Running in a confidential VM, gather confidential-computing evidence,\nsend it to Trustee and get secrets.\nA part of the confidential-containers project"
url = "https://github.com/confidential-containers/guest-components"
source-url = "https://github.com/confidential-containers/guest-components/archive/0061d036509e451597f5f61492b41849b36a32a1.zip"
extra-patches = [
              { "number" = 1, "file" = "0001-Fedora-Remove-workspace-members-which-are-not-built.patch" , "comments" = [ "Remove workspace members which are not built" ] },
              { "number" = 2, "file" = "0002-Fedora-AA-deps-crypto-default-to-openssl.patch" , "comments" = [ "deps/crypto defaults to openssl" ] },
              { "number" = 3, "file" = "0003-Fedora-kbs_protocol-Cargo.toml-reqwest-use-native-TL.patch" , "comments" = [ "use native-tls for reqwest" ] },
              { "number" = 4, "file" = "0004-Fedora-remove-jwt-simple-dependency.patch" , "comments" = [ "remove dependency jwt-simple - not in Fedora" ] },
              { "number" = 5, "file" = "0005-Fedora-remove-ttrpc-dependency.patch" , "comments" = [ "remove dependency ttrpc - not in Fedora" ] },
              { "number" = 6, "file" = "0006-Fedora-attester-pick-attesters-in-all-attesters.patch" , "comments" = [ "pick attesters to build" ] },
              { "number" = 7, "file" = "0007-Fedora-remove-testcontainers-dependency.patch" , "comments" = [ "remove dependency testcontainers - not in Fedora" ] },
              { "number" = 8, "file" = "0008-Fedora-kbs_protocol-default-to-openssl.patch" , "comments" = [ "kbs_protocol defaults to openssl" ] },
              { "number" = 9, "file" = "0009-Fedora-rstest-0.23.patch" , "comments" = [ "rstest version is 0.23" ] },
              { "number" = 10, "file" = "0010-Fedora-use-clap-4.2.7.patch" , "comments" = [ "clap version is ^4.2.7 -- see patch for more info" ] },
              { "number" = 11, "file" = "0011-Fedora-kbs-types-0.8.0.patch" , "comments" = [ "kbs-types version is 0.8.0" ] },
              { "number" = 12, "file" = "0012-Fedora-kbs_protocol-Cargo.toml-add-package.license.patch" , "comments" = [ "add package.license to kbs_protocol/Cargo.toml" ] },
              { "number" = 13, "file" = "0013-Fedora-url-version-is-2.5.2.patch" , "comments" = [ "url version is 2.5.2" ] },
              ]
extra-files = [
              "%doc trustee-attester-README.md",
              ]
supported-arches = ["x86_64"] # == exclusive-arch # or add it manually
cargo-install-bin = true
cargo-install-lib = false


[scripts]
prep.pre  =   [
    "# Cargo.lock isn't uptodate and rpmbuild set --offline", 
    "rm -f Cargo.lock",
    "# Work in kbs_protocol directory",
    "cd attestation-agent/kbs_protocol",
]
build.pre = [
    "cd attestation-agent/kbs_protocol",
]
build.post = [
    "mv LICENSE.dependencies ../../",
]
install.pre = [
    "cd attestation-agent/kbs_protocol",
    "mkdir -p %{buildroot}%{_docdir}/%{name}",
    "install -m 0644 src/bin/trustee-attester/README.md %{buildroot}%{_docdir}/%{name}/trustee-attester-README.md",
]
check.pre = [
    "cd attestation-agent/kbs_protocol",
]

# Manually fix what's missing:
# name:
# vesion:
# version (snapshot) is of format <latest-release>^<number>.<scm><revision>
# where the last part is built out of: git describe --tags
# matching the tarball name
#
# license:
# %cargo_generate_buildrequires
# Copy all comments with license strings
# Fix LICENSE at the top by concatenating these strings with AND between them
#
# files:
# Fix %files:
# Add LICENSE, README.md, trustee-attester
# Remove any binary or other files.
# ---
# %doc     README.md
# %doc     trustee-attester-README.md
# %license LICENSE
# %license LICENSE.dependencies

Comment 12 Fedora Review Service 2024-12-16 23:33:32 UTC
Created attachment 2062725 [details]
The .spec file difference from Copr build 8397231 to 8401489

Comment 13 Fedora Review Service 2024-12-16 23:33:34 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8401489
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2332550-trustee-guest-components/fedora-rawhide-x86_64/08401489-trustee-guest-components/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 15 Fedora Review Service 2025-01-14 18:04:13 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8513091
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2332550-trustee-guest-components/fedora-rawhide-x86_64/08513091-trustee-guest-components/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 16 Cole Robinson 2025-01-17 19:09:28 UTC
Looks good to me now, setting fedora-review+. Package is no longer blocked by other reviews, so removing those from `Depends On`

Template courtesy of decathorpe:

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
✅ test suite is run and all unit tests pass
✅ latest version of the crate is packaged
✅ license matches upstream specification and is acceptable for Fedora
✅ license file is included with %license in %files
✅ package complies with Rust Packaging Guidelines

Comment 17 Fabio Valentini 2025-01-18 15:58:59 UTC
Note that using a rust2rpm.toml config file is usually not necessary for packages like this,
since they don't need to be regenerated with rust2rpm for every new upstream version.
It doesn't *hurt*, but it's also not as helpful for non-crate packages as it is for crates.

> License:       ( ( MIT OR Apache-2.0 ) AND Unicode-DFS-2016 ) AND Apache-2.0 AND ( Apache-2.0 OR BSL-1.0 ) AND ( Apache-2.0 OR ISC OR MIT ) AND ( Apache-2.0 OR MIT ) AND ( BSD-2-Clause OR Apache-2.0 OR MIT ) AND MIT AND ( MIT OR Apache-2.0 ) AND ( MIT OR Apache-2.0 OR Zlib ) AND MPL-2.0 AND ( Unlicense OR MIT ) AND ( Zlib OR Apache-2.0 OR MIT )
> # LICENSE.dependencies contains a full license breakdown

This is also more verbose than it needs to be. While mechanically applying the

`" AND ".join(clause) for license in licenses`

"algorithm" for getting this string is "technically not wrong", it contains duplicates and unnecessary parentheses.

According to Red Hat Legal, the AND and OR operators in SPDX are associative and commutative, so this could be simplified to:

"""
License:       Apache-2.0 AND MIT AND MPL-2.0 AND Unicode-DFS-2016 AND (Apache-2.0 OR BSL-1.0) AND (Apache-2.0 OR ISC OR MIT) AND (Apache-2.0 OR MIT) AND (BSD-2-Clause OR Apache-2.0 OR MIT) AND (MIT OR Apache-2.0 OR Zlib) AND (Unlicense OR MIT)
"""

Note that it's also unusual to put additional whitespace around "(" and ")", I've never seen SPDX expressions written like that. I don't know if that would be *wrong*, but it looks odd.

Comment 18 Fabio Valentini 2025-01-18 16:02:03 UTC
Side note:

> 0.10.0^124.git0061d03

I don't know where "124" comes from, but this part would usually be a date in YYYYMMDD format (i.e. the date of the commit).

https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots

Using "<number>" instead of "<date>" only makes sense for VCSs with strictly linearly numbered revisions like bzr, I think.

Comment 19 Fabio Valentini 2025-01-18 16:05:11 UTC
I would also recommend to set the packaged commit in a macro like here:

https://src.fedoraproject.org/rpms/wingpanel/blob/rawhide/f/wingpanel.spec#_3-5

The SourceURL is also not in the recommended format for GitHub commit snapshot archives, which should be:

https://github.com/OWNER/PROJECT/archive/%{commit}/%{name}-%{shortcommit}.tar.gz

c.f. https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_commit_revision

Right now you need to update the commit hash in multiple places in the spec file (*and* in rust2rpm.toml) when making an update. Putting this into a macro would mean you only need to update *one* line in the spec file.

Comment 20 Uri Lublin 2025-01-19 14:39:14 UTC
(In reply to Fabio Valentini from comment #17)
> Note that using a rust2rpm.toml config file is usually not necessary for
> packages like this,
> since they don't need to be regenerated with rust2rpm for every new upstream
> version.
> It doesn't *hurt*, but it's also not as helpful for non-crate packages as it
> is for crates.

Noted, thanks.
I did it for the other Rust packages, so I did the same for this package too.

> 
> > License:       ( ( MIT OR Apache-2.0 ) AND Unicode-DFS-2016 ) AND Apache-2.0 AND ( Apache-2.0 OR BSL-1.0 ) AND ( Apache-2.0 OR ISC OR MIT ) AND ( Apache-2.0 OR MIT ) AND ( BSD-2-Clause OR Apache-2.0 OR MIT ) AND MIT AND ( MIT OR Apache-2.0 ) AND ( MIT OR Apache-2.0 OR Zlib ) AND MPL-2.0 AND ( Unlicense OR MIT ) AND ( Zlib OR Apache-2.0 OR MIT )
> > # LICENSE.dependencies contains a full license breakdown
> 
> This is also more verbose than it needs to be. While mechanically applying
> the
> 
> `" AND ".join(clause) for license in licenses`
> 
> "algorithm" for getting this string is "technically not wrong", it contains
> duplicates and unnecessary parentheses.
> 
> According to Red Hat Legal, the AND and OR operators in SPDX are associative
> and commutative, so this could be simplified to:
> 
> """
> License:       Apache-2.0 AND MIT AND MPL-2.0 AND Unicode-DFS-2016 AND
> (Apache-2.0 OR BSL-1.0) AND (Apache-2.0 OR ISC OR MIT) AND (Apache-2.0 OR
> MIT) AND (BSD-2-Clause OR Apache-2.0 OR MIT) AND (MIT OR Apache-2.0 OR Zlib)
> AND (Unlicense OR MIT)
> """

With that many expressions, I did not change anything so it's easy to
see that it's the same as the comments above in the spec file.
It takes a bit more time to compare the original and simplified version.
It seems the expression order is sorted.

If preferred, I'll use your simplified License tag.

> 
> Note that it's also unusual to put additional whitespace around "(" and ")",
> I've never seen SPDX expressions written like that. I don't know if that
> would be *wrong*, but it looks odd.

I'll remove the spaces.

Comment 21 Uri Lublin 2025-01-19 14:54:40 UTC
(In reply to Fabio Valentini from comment #18)
> Side note:
> 
> > 0.10.0^124.git0061d03
> 
> I don't know where "124" comes from, but this part would usually be a date
> in YYYYMMDD format (i.e. the date of the commit).

It's from "git describe --tags <commit>"
$ git describe --tags 0061d03
v0.10.0-124-g0061d03

'v' is removed
0.10.0 is the version
"-124-g0061d03" -> "^124.git0061d03" ( <number>.<scm><revision> )

> 
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_snapshots
> 
> Using "<number>" instead of "<date>" only makes sense for VCSs with strictly
> linearly numbered revisions like bzr, I think.

It (git describe) is linearly numbered according to the number of commits
since latest tag.

Comment 22 Fabio Valentini 2025-01-19 15:09:19 UTC
> If preferred, I'll use your simplified License tag.

This is for you to decide. Most maintainers so far just preferred the shorter result :)

> It's from "git describe --tags <commit>"
> $ git describe --tags 0061d03
> v0.10.0-124-g0061d03

Thanks, I did not know this existed, and haven't seen it used for packages yet. But that should indeed be fine.

Comment 23 Uri Lublin 2025-01-19 16:39:51 UTC
(In reply to Fabio Valentini from comment #19)
> I would also recommend to set the packaged commit in a macro like here:
> 
> https://src.fedoraproject.org/rpms/wingpanel/blob/rawhide/f/wingpanel.
> spec#_3-5
> 
> The SourceURL is also not in the recommended format for GitHub commit
> snapshot archives, which should be:
> 
> https://github.com/OWNER/PROJECT/archive/%{commit}/%{name}-%{shortcommit}.
> tar.gz
> 
> c.f.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
> #_commit_revision
> 
> Right now you need to update the commit hash in multiple places in the spec
> file (*and* in rust2rpm.toml) when making an update. Putting this into a
> macro would mean you only need to update *one* line in the spec file.

I'll add it in the spec file and change Source URL too.

Thanks !

Comment 24 Uri Lublin 2025-01-20 11:17:44 UTC
New build with the following changes (spec file only):
 - %commit, %shortcommit and more definition and usage
 - SourceLicense: tag removed
 - Simplified License: tag
   + First the lines are sorted, using a python script (attached)
   + Then License is simplify.
 - Source: is a tar.gz file (and URL)

SRPM: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08549695-trustee-guest-components/trustee-guest-components-0.10.0%5E124.git0061d03-1.fc42.src.rpm
SPEC: https://download.copr.fedorainfracloud.org/results/uril/trustee-attester/fedora-rawhide-x86_64/08549695-trustee-guest-components/trustee-guest-components.spec

Comment 25 Uri Lublin 2025-01-20 11:28:54 UTC
Created attachment 2066780 [details]
A simple python script to sort spec-file license lines of Rust packages

1. Copy the license lines from cargo build and add (or compare) them to the spec-file
2. run python rust_package_spec_file_license_sort.py
3. Paste the lines
4. Copy the output lines and add them to the spec-file (possibly skip this step)
5. Simplify License by
5.a Removing duplicate lines
5.b Removing A in "# A AND B" lines if A already exists (same for B)
5.c Removing (A OR B) AND C if "A OR B" already exists (a special case of 5.b)

Comment 26 Fedora Admin user for bugzilla script actions 2025-01-21 13:58:18 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/trustee-guest-components

Comment 27 Fedora Update System 2025-01-21 14:41:54 UTC
FEDORA-2025-2a0827553a (trustee-guest-components-0.10.0^124.git0061d03-1.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-2a0827553a

Comment 28 Fedora Update System 2025-01-21 14:46:59 UTC
FEDORA-2025-2a0827553a (trustee-guest-components-0.10.0^124.git0061d03-1.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.