Spec URL: https://gitlab.com/luigileonardi/libigvm/-/raw/no_vendor/libigvm.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/lleonard/sev-snp-coconut/fedora-rawhide-x86_64/08918482-libigvm/libigvm-0.3.4-1.fc43.src.rpm Description: IGVM Library is an implementation of a parser for the Independent Guest Virtual Machine Fedora Account System Username: lleonard This is part of the effort to add the Coconut-SVSM paravisor to Fedora. It would enables Fedora virtualization hosts to launch confidential virtual machines using AMD's SEV-SNP technology. See https://fedoraproject.org/wiki/Changes/ConfidentialVirtHostAMDSEVSNP This static library is required by qemu.
Cannot find any valid SRPM URL for this ticket. Common causes are: - You didn't specify `SRPM URL: ...` in the ticket description or any of your comments - The URL schema isn't HTTP or HTTPS - The SRPM package linked in your URL doesn't match the package name specified in the ticket summary --- 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.
[fedora-review-service-build]
Copr build: https://copr.fedorainfracloud.org/coprs/build/8951388 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2361600-libigvm/fedora-rawhide-x86_64/08951388-libigvm/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.
I think my original suggestion to call this `libigvm` is wrong, sorry. yeah a libigvm.a is being installed, but the .pc file is `igvm.pc` and the upstream repo is called `igvm`. So I think this package should be called `igvm`. igvm here is _only_ a static library. So the package layout is a bit interesting. With libs like glib2, pcre2, glibc you'll have package hierarchy like glib2 (or some packages call this NAME-libs) files: lib/*.so.* glib2-devel files: lib/*.so , pkgconfig, /usr/include Requires: glib2 glib2-static files: lib/*.a Requires: glib2-devel But since libigvm doesn't have .so files at all, `libigvm` doesn't exist, and `libigvm-devel` on its own is pointless. So I _think_ this should stuff pkgconfig and /usr/include bits into libigvm-static, and kill the libigvm-devel package too. If something changed and libigvm ever becomes dynamically linkable as well, the package can switch to the above ideal layout easypeasy. @berrange can you sanity check me here, does all that sound reasonable?
Some other bits: + if the dump_igvm tool is just a helper tool, I think it should go in a `NAME-tools` package + rust-bitfield-struct is at version 0.11 in Fedora, so you need to adjust your version patch. I confirmed it's still building with that + take a look at /usr/lib/rpm/macros.d/macros.cargo %__cargo macro. I think you want to export `CARGO_HOME=.cargo` and `RUSTFLAGS='%{build_rustflags}'` too so that trickles down to cargo called by the Makefile.
> But since libigvm doesn't have .so files at all, `libigvm` doesn't exist, and `libigvm-devel` on its own is pointless. > So I _think_ this should stuff pkgconfig and /usr/include bits into libigvm-static, and kill the libigvm-devel package too. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_packaging_static_libraries Says that for the "static only" case don't create a -static, put the .a file in the -devel + if the dump_igvm tool is just a helper tool, I think it should go in a `NAME-tools` package Agreed > Spec URL: https://gitlab.com/luigileonardi/libigvm/-/raw/no_vendor/libigvm.spec > SRPM URL: https://download.copr.fedorainfracloud.org/results/lleonard/sev-snp-coconut/fedora-rawhide-x86_64/08918482-libigvm/libigvm-0.3.4-1.fc43.src.rpm The src.rpm has gone, and the only build I see in copr has a spec file that is different from the above one, so I'm unclear as to what this ticket should be reviewing as the "latest". > export EXTRA_PARAMS="--profile rpm -Z avoid-dev-deps" What does the 'avoid-dev-deps' flag skip ? > # Run test just when necessary. This is not upstream > Patch6: 0007-Remove-test-from-all-target.patch Seems like upstream should define a top level target 'build' and have 'all' depend on 'build' + 'test'. > %if %{with check} Is that needed ? If building locally you can give an arg to RPM to skip checks. If building in Fedora I'd expects checks to always be run & passing
Hi Cole and Daniel, > So I think this package should be called `igvm`. Should I create a new ticket or just rename this one? > rust-bitfield-struct is at version 0.11 in Fedora, so you need to adjust your version patch. I confirmed it's still building with that good catch, thanks for checking! > take a look at /usr/lib/rpm/macros.d/macros.cargo %__cargo macro. I think you want to export `CARGO_HOME=.cargo` and `RUSTFLAGS='%{build_rustflags}'` too so that trickles down to cargo called by the Makefile. good point, will do! > What does the 'avoid-dev-deps' flag skip ? It's a flag it's normally used by %cargo_build (it's in __cargo_common_opts) but yes, it's useless in this case since there are no dev-dependencies. I can drop RUSTC_BOOTSTRAP too. > Is that needed ? If building locally you can give an arg to RPM to skip checks. If building in Fedora I'd expects checks to always be run & passing It's from the default cargo2rpm template but I can drop it. > The src.rpm has gone, and the only build I see in copr has a spec file that is different from the above one, so I'm unclear as to what this ticket should be reviewing as the "latest". Where can I upload the file so that it's not removed after a while? > Seems like upstream should define a top level target 'build' and have 'all' depend on 'build' + 'test'. I'll send a PR upstream So to recap, I need to create a `igvm-devel` package that contains .a and headers etc and a `igmm-tool` just with the `dump_igvm`? Thanks!
> Where can I upload the file so that it's not removed after a while? Typically you can use fedorapeople.org https://fedoraproject.org/wiki/Infrastructure/fedorapeople.org#Accessing_your_fedorapeople.org_space
Spec URL: https://lleonard.fedorapeople.org/igvm.spec SRPM URL: https://lleonard.fedorapeople.org/igvm-0.3.4-1.fc42.src.rpm changes: - Renamed to igvm, moved .a and igvm.pc to devel - Created -tools package that contains dump_igvm - Makefile target change has been merged upstream. - exported cargo macros and updated flags - bumped rust-bitfield-struct to v0.11
Copr build: https://copr.fedorainfracloud.org/coprs/build/9100128 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2361600-igvm/fedora-rawhide-x86_64/09100128-igvm/fedora-review/review.txt Found issues: - Package has .a files: igvm-devel. Does not provide -static: igvm-devel. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/#packaging-static-libraries Please know that there can be false-positives. --- 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.
Spec URL: https://lleonard.fedorapeople.org/igvm.spec SRPM URL: https://lleonard.fedorapeople.org/igvm-0.3.4-2.fc42.src.rpm changes: - Adding Provides for the -static package. I don't think we need `Requires: %{name} = %{version}-%{release}` for `devel` and `tools` since the `igvm` package is not created. WDYT?
Created attachment 2092116 [details] The .spec file difference from Copr build 9100128 to 9100453
Copr build: https://copr.fedorainfracloud.org/coprs/build/9100453 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2361600-igvm/fedora-rawhide-x86_64/09100453-igvm/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.
> I don't think we need `Requires: %{name} = %{version}-%{release}` for > `devel` and `tools` since the `igvm` package is not created. WDYT? Yeah that's fine. I tried a scratch build, and i686 failed, the .a file wasn't generated. Not worth figuring out, just add `ExcludeArch: %{ix86}" rpmlint points out some issues with dump_igvm: igvm-tools.x86_64: W: unused-direct-shlib-dependency /usr/bin/dump_igvm /lib64/libm.so.6 igvm-tools.x86_64: W: unstripped-binary-or-object /usr/bin/dump_igvm igvm-tools.x86_64: W: position-independent-executable-suggested /usr/bin/dump_igvm I _think_ these are all due to igvm_c/Makefile ignoring CFLAGS and friends when building dump_igvm. rpm exports CFLAGS at beginning of %build/%install/%check phases but the Makefile isn't adhering to them. Is dump_igvm useful? The code is in a `sample/` directory upstream so not sure if it's just a demo app or not. Maybe it's simpler to delete it from packaging. After those bits I think everything is good to go
Spec URL: https://lleonard.fedorapeople.org/igvm.spec SRPM URL: https://lleonard.fedorapeople.org/igvm-0.3.4-3.fc42.src.rpm > Not worth figuring out, just add `ExcludeArch: %{ix86}" done! > Is dump_igvm useful? It is kind of useful, so I troubleshooted and fixed it: I opened an upstream PR to add CFLAGS and LDFLAGS into the Makefile I also had to remove %global debug_package %{nil}, in this way the binaries are now stripped. changes: - Added ExcludeArch: %{ix86} - Applied an upstream patch for the makefile - Added README for -tools package Thanks!
Created attachment 2093326 [details] The .spec file difference from Copr build 9100453 to 9140866
Copr build: https://copr.fedorainfracloud.org/coprs/build/9140866 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2361600-igvm/fedora-rawhide-x86_64/09140866-igvm/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.
LGTM now! setting fedora-review+
The Pagure repository was created at https://src.fedoraproject.org/rpms/igvm
FEDORA-2025-1d89eb2177 (igvm-0.3.4-1.fc43) has been submitted as an update to Fedora 43. https://bodhi.fedoraproject.org/updates/FEDORA-2025-1d89eb2177
FEDORA-2025-1d89eb2177 (igvm-0.3.4-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report.