[MIR] mdevctl 1.0.0 (rust switch)

Bug #1942394 reported by Christian Ehrhardt 
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mdevctl
Fix Released
Unknown
mdevctl (Ubuntu)
New
Undecided
Ubuntu Security Team

Bug Description

This template uses the new proposed format that covers Rust packages, submitted
through https://github.com/canonical/ubuntu-mir/pull/1

[Availability]

The package mdevctl is already in main via LP: #1889248, but Version 1.0
switched from the most simple (shell) to the least easy supportable (rust) =>
https://github.com/mdevctl/mdevctl/releases/tag/v1.0.0

The latest version of mdevctl available in Debian unstable was changed to adapt
to the MIR rules, as proposed in
https://code.launchpad.net/~athos-ribeiro/ubuntu/+source/mdevctl/+git/mdevctl/+merge/425351.

The package builds and works for all supported architectures, and is available
at
https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+packages.

The original (shell based) package is available at
https://launchpad.net/ubuntu/+source/mdevctl.

[Rationale]

This has 3 reasons:
1. it is a very nice tool to handle meidiated devices in general.
   It more and more becomes the one tool people refer to (other than fully
   manual working through sysfs)
2. it is a Recomments for libvirt-daemon-system, which is in main.
3. the previous (shell based) version of the package is already in main.

It would be great to have mdevctl in Ubuntu main for kinetic, to avoid more
gaps between Ubuntu and Debian unstable, which could potentialy hinder the
merge processes, but there is no definitive deadline.

[Security]

No CVEs/security issues in this software in the past;
No `suid` or `sgid` binaries;
No executables in `/sbin` and `/usr/sbin`;
The package does not install services, timers or recurring jobs;
The package does not open privileged ports (ports < 1024); and
The package does not contain extensions to security-sensitive software
(filters, scanners, plugins, UI skins, etc).

[Quality assurance - function/usage]

The package works well right after install. It is composed of a single binary
file, a manpage and documentation.

[Quality assurance - maintenance]
The package is maintained well in Debian/Ubuntu and has not too many and long
term critical bugs open.

Ubuntu https://bugs.launchpad.net/ubuntu/+source/mdevctl/+bug

Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=mdevctl

At the moment this was written, the only Ubuntu bug open was this MIR one.
Debian has 2 open bugs, as described below:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013551
This has been fixed in salsa through
https://salsa.debian.org/debian/mdevctl/-/merge_requests/3 and will be
available in the next debian release. It is also already included in the
proposed merge in the PPA at
https://salsa.debian.org/debian/mdevctl/-/merge_requests/3, which is what we
intend to upload to Ubuntu once this MIR is accepted.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1003777
This is valid, but can be fixed in Debian first and then pushed to Ubuntu. The
next upstream version will improve the error message as per
https://github.com/mdevctl/mdevctl/commit/1b880042683879db524c0d74b48bfdf533bda996.
On top of that, we should ensure that /etc/mdevctl.d/ is part of this package.

[Quality assurance - testing]

The package runs a test suite on build time, if it fails it makes the build fail.

You can verify that at https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+build/24166000

The package does not run an autopkgtest because the rust tooling does not provide an out-of-the-box manner to run the test suite for packages with vendorized code as it does for packages without vendorized code. This is something we should pursue in the mid/long term.

[Quality assurance - packaging]

debian/watch is present and works. It levarages the support for Multiple
Upstream Tarballs (MUT) to pull in the vendored sources. This process is
described in debian/README.source.

debian/control defines a correct Maintainer field.

This package does not yield massive lintian Warnings, Errors
A recent build log of the package is available at
https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+build/24120829

A no comprehensive "lintian --pedantic" output (without --no-tag-display-limit) follows:

E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz ar failed for vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_mincore.a
E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz ar failed for vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_mincore_downlevel.a
E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz ar failed for vendor/winapi-i686-pc-windows-gnu/lib/libwinapi_onecore.a
E: mdevctl source: unpack-message-for-orig ... use --no-tag-display-limit to see all (or pipe to a file/program)
P: mdevctl source: update-debian-copyright 2020 vs 2022 [debian/copyright:10]
P: mdevctl source: very-long-line-length-in-source-file vendor/aho-corasick/.cargo-checksum.json line 1 is 2574 characters long (>512)
P: mdevctl source: very-long-line-length-in-source-file vendor/ansi_term/.cargo-checksum.json line 1 is 1075 characters long (>512)
P: mdevctl source: very-long-line-length-in-source-file vendor/anyhow/.cargo-checksum.json line 1 is 3038 characters long (>512)
P: mdevctl source: very-long-line-length-in-source-file ... use --no-tag-display-limit to see all (or pipe to a file/program)

Lintian overrides are not present.

This package does not rely on obsolete or about to be demoted packages.
This package has no python2 or GTK2 dependencies.

The package will not be installed by default. Still, it does not ask debconf
questions.

Packaging is more complex than avarage due to the source vendoring
process, which differs to Debian. This should be ok because
debian/README.source clearly describes the process.

[UI standards]

No end user UI
Just a few CLI bits used by admins and parsable output used by tools.

[Dependencies]

No further depends or recommends dependencies that are not yet in main. Do note
that this package includes vendored Rust code.

[Standards compliance]

This package correctly follows FHS and Debian Policy. Do note that it does
include embedded copies of otehr software (vendorized rust code), which is
discouraged by
https://www.debian.org/doc/debian-policy/ch-source.html#embedded-code-copies.
This is done to the current state of the rust stack/support.

[Maintenance/Owner]

The Server Team is already subscribed to the package and maintains it in Debian
and Ubuntu.

The Server Team is aware of the implications by a static build and
commits to test no-change-rebuilds and to fix any issues found for the
lifetime of the release (including ESM).

The Server Team is aware of the implications of vendored code and (as alerted
by the security team) commits to provide updates and backports to the security
team for any affected vendored code for the lifetime of the release (including
ESM).

This package uses vendored rust code tracked in Cargo.lock as shipped, in the
package (at /usr/share/doc/mdevctl/Cargo.lock.gz - gz compressed),
refreshing that code is outlined in debian/README.source This package uses
vendored code, refreshing that code is outlined in debian/README.source.

This package is rust based and vendors all non language-runtime dependencies.

The package was test rebuilt in a PPA, as pointed out above.

The latest version of mdevctl available in Debian unstable was changed to adapt
to the MIR rules, as proposed in
https://code.launchpad.net/~athos-ribeiro/ubuntu/+source/mdevctl/+git/mdevctl/+merge/425351.

The package builds and works for all supported architectures, and is available
at
https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+packages,
where one can check the build logs for all supported architectures.

[Background information]

The Package description explains the package well:

Mediated device management utility for Linux mdevctl is a utility for managing
and persisting devices in the mediated device framework of the Linux kernel.
Mediated devices are sub-devices of a parent device (ex. a vGPU) which can be
dynamically created and potentially used by drivers like vfio-mdev for
assignment to virtual machines.

Upstream Name is mdevctl, and is available at https://github.com/mdevctl/mdevctl

Note that, for the former MIR process, jq and libonig were included in main
because mdevctl < 1 depends on those packages. This is no longer true for
mdevctl >= 1 and their demotion should be evaluated.

[Former Bug Description - NO LONGER PART OF MIR DOCS]

This is in main via bug 1889248 already, but Version 1.0 switched from the most simple (shell) to the least easy supportable (rust)
=> https://github.com/mdevctl/mdevctl/releases/tag/v1.0.0

This worked fine in Debian
=> https://launchpad.net/debian/+source/mdevctl/1.0.0-1

But for Ubuntu the Server team isn't gonna own the full rust toolchain just because of this helper.
IMHO that needs a discussion how we want to handle rust in general and then the long MIR road for all the way too many dependencies.
I'll start the discussion internally ...

This bug is meant to be a reference from the sync avoidance override as well as the component mismatches - so that everyone can re-check here what the current state is.

Right now it is *intentionally* incomplete and has no full MIR template here.

Changed in mdevctl (Ubuntu):
status: New → Incomplete
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
@ubuntu-archive - since [1] is a junk repo I can't propose an MR to it.
I subscribed you to please merge the attached diff to sync-blacklist.txt to not make this even more pain by auto-syncing 1.0 over once 22.04 opens up.

[1]: https://bazaar.launchpad.net/~ubuntu-archive/+junk/sync-blacklist/view/head:/sync-blacklist.txt

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Done, so now at least we won't get it auto-synced accidentally in the near future.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you Lukasz, I have also contacted Matt/Steve to get a status/ETA on rust-in-main overall.
There isn't much to share yet nor something we could link this bug to :-/

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI I'm sure more will come e.g. virtiofsd-rs https://virtio-fs.gitlab.io/

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (4.1 KiB)

To get a sense of the scale despite mdevctl being so small in it's function here
the rust portion of the build-dependency tree. (Due to the way rust libs work some are from the same source - so two lists):

Maybe some of them are for dh-rust, maybe we need all of them. Since there is no full detection for built-using yet (that I'd know of) this is guessing from what I get when I run apt build-dep in an empty container.

Sources to MIR:
Source: rust-aho-corasick
Source: rust-ansi-term
Source: rust-anyhow
Source: rust-atty
Source: rust-autocfg
Source: rust-bitflags
Source: rust-cfg-if-0.1
Source: rust-clap
Source: rust-cloudabi (0.0.3-1)
Source: rust-env-logger
Source: rust-getrandom
Source: rust-heck
Source: rust-humantime
Source: rust-indexmap
Source: rust-itoa
Source: rust-lazy-static
Source: rust-libc
Source: rust-lock-api
Source: rust-log
Source: rust-memchr
Source: rust-once-cell
Source: rust-parking-lot
Source: rust-parking-lot-core
Source: rust-ppv-lite86
Source: rust-proc-macro-error
Source: rust-proc-macro-error-attr
Source: rust-proc-macro2
Source: rust-quote
Source: rust-rand
Source: rust-rand-chacha
Source: rust-rand-core
Source: rust-rand-hc (0.2.0-1)
Source: rust-redox-syscall
Source: rust-regex
Source: rust-regex-syntax
Source: rust-remove-dir-all
Source: rust-ryu
Source: rust-scopeguard
Source: rust-serde
Source: rust-serde-json
Source: rust-smallvec
Source: rust-strsim
Source: rust-structopt
Source: rust-structopt-derive
Source: rust-syn
Source: rust-syn-mid
Source: rust-tempfile
Source: rust-termcolor
Source: rust-textwrap (0.11.0-1)
Source: rust-thread-local
Source: rust-unicode-segmentation
Source: rust-unicode-width
Source: rust-unicode-xid
Source: rust-uuid
Source: rust-vec-map (0.8.1-2)
Source: rust-version-check
Source: rust-winapi
Source: rust-winapi-i686-pc-windows-gnu (0.4.0-1)
Source: rust-winapi-util
Source: rust-winapi-x86-64-pc-windows-gnu (0.4.0-1)

Effective full build-time dependencies:
librust-aho-corasick+std-dev
librust-aho-corasick-dev
librust-ansi-term-dev
librust-anyhow-dev
librust-atty-dev
librust-autocfg-dev
librust-bitflags-dev
librust-cfg-if-0.1-dev
librust-clap+color-dev
librust-clap+default-dev
librust-clap+strsim-dev
librust-clap-dev
librust-cloudabi+default-dev
librust-cloudabi-dev
librust-env-logger+default-dev
librust-env-logger-dev
librust-getrandom-dev
librust-heck-dev
librust-humantime-dev
librust-indexmap-dev
librust-itoa-dev
librust-lazy-static-dev
librust-libc-dev
librust-lock-api-dev
librust-log+serde-dev
librust-log-dev
librust-memchr-dev
librust-once-cell-dev
librust-parking-lot-core-dev
librust-parking-lot-dev
librust-ppv-lite86-dev
librust-proc-macro-error-attr-dev
librust-proc-macro-error-dev
librust-proc-macro2-dev
librust-quote+proc-macro-dev
librust-quote-dev
librust-rand+alloc-dev
librust-rand+getrandom-dev
librust-rand+std-dev
librust-rand-chacha+std-dev
librust-rand-chacha-dev
librust-rand-core+getrandom-dev
librust-rand-core+std-dev
librust-rand-core-dev
librust-rand-dev
librust-rand-hc-dev
librust-redox-syscall-dev
librust-regex+default-dev
librust-regex+perf-cache-dev
librust-regex+perf-dev
librust-regex+perf-literal-dev
librust-regex+unicode-age-dev
librust-regex+unicode-b...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI (mostly a note to myself I guess) about dynamic rust:

Along the way in I found that rust seems to have dynamic libraries - see
"dylib/cdylib" in [2].
[1] shows -C prefer-dynamic target-feature=-crt-static" which could be our
usual build style for a distro making dependencies and code-resuage through
dynamic libs "normal".

But before I could cheer I found that:
- One argument against it that I found is that two dynamic libs that use the
  same (static) rlib can not work together. So it would have to be make all
  or nothing dynamic. (That would be ok I guess)
- Another I've quckly found is that ABI isn't fixed and thereby a compiler change
  between a rebuild of either component could break things. (This breaks the
  camels back)

[1]: https://rust-lang.github.io/rfcs/0404-change-prefer-dynamic.html
[2]: https://doc.rust-lang.org/reference/linkage.html

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

dh_golang adds misc:Built-Using via `go list -f`.
1. on the target binaries, deriving "{{ range .Deps }}".
2. And then from all Deps deriving the filenames
3. and then it tracks down the package that owns the files
=> That then makes the list for Built-Using

That does not seem to exist for Rust yet.

The build-cache [1] has plenty of information on the build.
Hopefully we might be able to derive somiething from there.
rlibs are part of the linking, think intermediate static
libs [2][3].

Gladly I found that I do not have to re-invent the wheel.
dh-cargo-built-using does what I'd expect.

Along cargo install it dh_cargo runs this already.
The current format in d/control for this seems to be

  Built-Using: ${cargo:Built-Using}
  XB-X-Cargo-Built-Using: ${cargo:X-Cargo-Built-Using}

That might follow the debate in golang to not use Built-Using for
the libraries as it already splits compiler (still useful for the
potential ABI issues) but puts the libs in an extra non-standard field.

I've added this to mdevctl so that we can continue to use this as
test-example for potential e.g. britney extensions to probe component
mismatches based on it.

[1]: https://doc.rust-lang.org/cargo/guide/build-cache.html
[2]: https://doc.rust-lang.org/reference/linkage.html
[3]: https://rustc-dev-guide.rust-lang.org/backend/libs-and-metadata.html

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

With the above I come down further to "just" 36 libraries.
The rest is only indirectly, not effectively or build-time used.

I've not uploaded this to Debian yet as we also have nothing on our end
that would evaluate/block on it.

rust-aho-corasick (= 0.7.10-1)
rust-ansi-term (= 0.12.1-1)
rust-anyhow (= 1.0.31-1)
rust-atty (= 0.2.14-2)
rust-bitflags (= 1.2.1-1)
rust-cfg-if-0.1 (= 0.1.10-2)
rust-clap (= 2.33.3-1)
rust-env-logger (= 0.7.1-2)
rust-getrandom (= 0.1.13-4)
rust-humantime (= 2.0.0-1)
rust-indexmap (= 1.3.2-1)
rust-itoa (= 0.4.3-1)
rust-lazy-static (= 1.4.0-1)
rust-libc (= 0.2.80-1)
rust-log (= 0.4.11-2)
rust-memchr (= 2.3.3-1)
rust-once-cell (= 1.5.2-1)
rust-ppv-lite86 (= 0.2.6-2)
rust-rand-chacha (= 0.2.2-1)
rust-rand-core (= 0.5.1-1)
rust-rand (= 0.7.3-3)
rust-regex (= 1.3.7-1)
rust-regex-syntax (= 0.6.17-1)
rust-remove-dir-all (= 0.5.2-1)
rust-ryu (= 1.0.2-1)
rust-serde (= 1.0.106-1)
rust-serde-json (= 1.0.41-1)
rust-strsim (= 0.9.3-1)
rust-structopt (= 0.3.20-1)
rust-tempfile (= 3.1.0-1)
rust-termcolor (= 1.1.0-1)
rust-textwrap (= 0.11.0-1)
rust-thread-local (= 1.1.3-3)
rust-unicode-width (= 0.1.8-1)
rust-uuid (= 0.8.1-3)
rust-vec-map (= 0.8.1-2)
rustc (= 1.49.0+dfsg1-1)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

As a hint that really more things in main might come up using it, here a follow up to the KVM Forum 2021.
=> https://wiki.qemu.org/RustInQemu

Changed in mdevctl:
status: Unknown → Fix Released
Bryce Harrington (bryce)
tags: added: needs-sync
Changed in mdevctl (Ubuntu):
milestone: none → ubuntu-22.06
Bryce Harrington (bryce)
tags: added: packaging
Changed in mdevctl (Ubuntu):
assignee: nobody → Athos Ribeiro (athos-ribeiro)
description: updated
description: updated
Revision history for this message
Bryce Harrington (bryce) wrote :

 mdevctl | 0.81-1 | jammy
 mdevctl | 0.81-1 | kinetic

mdevctl | 0.81-1 | stable
mdevctl | 1.1.0-1 | testing
mdevctl | 1.1.0-1 | unstable
mdevctl | 1.1.0-1 | unstable-debug

Changed in mdevctl (Ubuntu):
milestone: ubuntu-22.06 → ubuntu-22.07
status: Incomplete → New
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Rust MIR rules landed, this is ready for review.
Un-assigning Athos to be MIR-Team visible.

P.S. Review from the PPA or MP as described, because due to the nature of how propose migration works this would "just migrate" if we upload it as-is.

Changed in mdevctl (Ubuntu):
assignee: Athos Ribeiro (athos-ribeiro) → nobody
Lukas Märdian (slyon)
Changed in mdevctl (Ubuntu):
assignee: nobody → Lukas Märdian (slyon)
Lukas Märdian (slyon)
tags: added: fr-2559
Revision history for this message
Lukas Märdian (slyon) wrote :
Download full text (7.3 KiB)

Review for Package: src:mdevctl

[Summary]
This is the 1st Rust package to be MIRed, using newly adopted MIR rules.
The package looks rather clean and under control, except for all of the vendored
dependencies/sources, which can be overwhelming. But we have the Sever team's
written consent to stay on top of them for the lifetime of the package.
Lot's of "Recommended TODOs" have been stated below, which should be improved
upon step by step, but nothing critical blocking the promotion IMO.

MIR team ACK

This does need a security review, so I'll assign ubuntu-security
That is because it's 1st the Rust package and we want the security-team's input
on this. Doesn't seem super security senstive otherwise (maybe the vendored libc
bindings).

List of specific binary packages to be promoted to main: mdevctl
Specific binary packages built, but NOT to be promoted to main: <None>

Notes:
- This is the 1st Rust package to be MIRed, using newly adopted MIR rules.
- embedded source present
- static linking in use
- handles data formats (JSON output) via serde-rs JSON library

Required TODOs:
- None

Recommended TODOs:
#1:
- dh_auto_test does not recognize the 'nocheck' DEB_BUILD_OPTION
  I: mdevctl source: override_dh_auto_test-does-not-check-DEB_BUILD_OPTIONS [debian/rules:22]
  => This should be fixed according to lintian:
     ifeq (,$(filter nocheck,$(DEB_BUILD_OPTIONS)))

#2:
- does NOT have a non-trivial test suite that runs as autopkgtest
  => as lined out in the bug description, this should be fixed mid/long term.

#3.1: Lintian warnings/errors, some that feel worth mentioning:
- E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-i686-pc-windows-gnu/lib/*.a
- E: mdevctl source: unpack-message-for-orig mdevctl_1.1.0.orig-vendor.tar.gz . ar failed for vendor/winapi-x86_64-pc-windows-gnu/lib/*.a
  => Seems to be OK, as those are only Windows libraries, unused in Ubuntu.

#3.2:
- I: mdevctl source: out-of-date-standards-version 4.6.0 (released 2021-08-18) (current is 4.6.1.0)
  => can be fixed with the next version bump

#3.3:
- P: mdevctl source: very-long-line-length-in-source-file
  => lots of those, maybe they should be muted.
     It's false positives as it is complaining about .a windows binaries

#3.4: (some lintian recommendations)
  X: mdevctl source: prefer-uscan-symlink filenamemangle s/.+\/v?(\d\S+)\.tar\.gz/mdevctl-$1\.tar\.gz/ [debian/watch:3]
  X: mdevctl source: upstream-metadata-file-is-missing

#4: upstream build-time warnings (in vendored code). could be double checked
    and reported to the upstream developers
- warnings during the build (vendorized deps); some deprecations
  warning: `ansi_term` (lib) generated 12 warnings
  warning: `vec_map` (lib) generated 3 warnings
  warning: `log` (lib) generated 1 warning
  warning: `regex-syntax` (lib) generated 2 warnings
  warning: `aho-corasick` (lib) generated 3 warnings
  warning: `clap` (lib) generated 77 warnings
  warning: `proc-macro-error` (lib) generated 1 warning

[Duplication]
There is no other package in main providing the same functionality.
Except the previous (non-rust) version of mdevctl itself.

[Dependencies]
OK:
-...

Read more...

Changed in mdevctl (Ubuntu):
assignee: Lukas Märdian (slyon) → Ubuntu Security Team (ubuntu-security)
tags: added: sec-1214
Changed in mdevctl (Ubuntu):
milestone: ubuntu-22.07 → ubuntu-22.08
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I updated the PPA and the MP to perform the changes on top of the new version available in Debian (1.2.0). It also addresses some of the recommended TODOs pointed by Lukas.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I filed an FFe bug in LP: #1987551 so we can upload this to kinetic once it's complete.

Revision history for this message
Seth Arnold (seth-arnold) wrote :

There's more build warnings than I usually like to see:

$ grep warning: mdevctl_1.1.0-1ubuntu1~ppa2_amd64-2022-08-09T15\:02\:28Z.build | sort | uniq -c | sort -nr
    126 warning: trait objects without an explicit `dyn` are deprecated
     88 = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
     28 = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
     14 warning: unnecessary parentheses around block return value
     10 = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
      9 warning: panic message is not a string literal
      8 warning: use of deprecated associated function `std::error::Error::description`: use the Display impl or to_string()
      8 warning: use of deprecated associated function `bitflags::core::str::<impl str>::trim_left_matches`: superseded by `trim_start_matches`
      6 warning: unnecessary trailing semicolon
      6 warning: unnecessary parentheses around type
      4 = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2018!
      4 warning: anonymous parameters are deprecated and will be removed in the next edition
      3 dpkg-scanpackages: warning: Packages in archive but missing from override file:
...

some of these are obviously a waste of time but some of them might represent challenges backporting fixes from the future.

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Hi Seth,

Thanks for the feedback here!

As mentioned in https://bugs.launchpad.net/ubuntu/+source/mdevctl/+bug/1942394/comments/14, we updated the package in the PPA (https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+packages) to the new version available in Debian (1.2.0).

This new version fixed the majority of the warnings we were seeing in 1.1.0.

Here is the new output for your grep snippet:

$ grep warning: buildlog_ubuntu-kinetic-amd64.mdevctl_1.2.0-1ubuntu1~ppa1_BUILDING.txt | sort | uniq -c | sort -nr
      2 warning: unknown lint: `unused_macro_rules`
      2 warning: `hashbrown` (lib) generated 1 warning
      1 warning: `proc-macro-error` (lib) generated 1 warning
      1 warning: panic message is not a string literal
      1 warning: field is never read: `start`
      1 warning: field is never read: `last_match_end`
      1 warning: field is never read: `config`
      1 warning: be sure to add `debian/mdevctl/usr/bin` to your PATH to be able to run the installed binaries
      1 warning: `aho-corasick` (lib) generated 3 warnings
      1 dpkg: warning: unable to delete old directory '/usr/lib/udev': Directory not empty
      1 dpkg-source: warning: cannot verify signature ./mdevctl_1.2.0-1ubuntu1~ppa1.dsc
      1 dpkg-gencontrol: warning: Suggests field of package mdevctl: substitution variable ${cargo:Suggests} used, but is not defined
      1 dpkg-gencontrol: warning: Recommends field of package mdevctl: substitution variable ${cargo:Recommends} used, but is not defined
      1 dpkg-gencontrol: warning: Provides field of package mdevctl: substitution variable ${cargo:Provides} used, but is not defined
      1 dpkg-gencontrol: warning: Depends field of package mdevctl: substitution variable ${cargo:Depends} used, but is not defined

Now, there are 2 different warnings being raised there:

- warning: panic message is not a string literal
This is related to this Rust 2021 change: https://doc.rust-lang.org/nightly/edition-guide/rust-2021/panic-macro-consistency.html

kinetic ships with rustc 1.59.0, where this warning is not an error. Starting from rustc 1.60.0, this becomes an error and we will need to change that panic!(AbortNow) call to std::panic::panic_any(AbortNow). Since this warning is being issued for a vendorized dependency, I wonder if we should patch it right away and forward a patch upstream, or just patch it upstream and introduce the Delta when/if it is not fixed by the time rustc >= 1.60.0 lands (this would be a delta since the debian package does not vendorize the dependency).

- warning: field is never read: `start`
This is a warning for possible dead code detected by the compiler which could be dealt with upstream in mid/long term.

Finally, a new version of a BuildDep landed in Debian unstable and kinetic-proposed (not migrated yet by the time of this writing). rust-uuid (0.8.1-4 to 1.1.2-2) .

This changes some binary buildDependencies (not used for our build anyway) and leads to FTBFS.
I will forward a fix to Debian and change introduce it as a delta here. Would you also like me to introduce the delta to silence the warning mentioned above?

Thanks!

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

I just pushed a delta to fix the FTBFS issue (mdevctl - 1.2.0-1ubuntu1~ppa2).

For the Debian package, the new dependency is incompatible with mdevctl, so we will need to work further with upstream to add support for the new uuid version in unstable (this should not be an issue here ATM since that dependency is vendorized in this proposal).

https://launchpad.net/~athos-ribeiro/+archive/ubuntu/mdevctl-vendored-plus-lockfile/+packages
https://code.launchpad.net/~athos-ribeiro/ubuntu/+source/mdevctl/+git/mdevctl/+merge/425351

Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

A fix for the Debian package FTBFS is available at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1017369.

I proposed an upstream fix at https://github.com/mdevctl/mdevctl/pull/67.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

We are convinced all that was raised so far is addressed in the new version we already have.
We added block-proposed to have it in kinetic-proposed and stop the PPA indirection.

Expect an update by Athos once it is there.

tags: added: block-proposed
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

As mentioned in the previous comment, we blocked the package from migrating with the "block-proposed" tag in this bug and uploaded it [1].

Once this review process is complete, and if it happens in the next 1 week, we can just remove the tag and let the package migrate. Otherwise, we may want to re-visit the FFe bug at LP: #1987551 to ensure this still acceptable for kinetic.

[1] https://launchpad.net/ubuntu/+source/mdevctl/1.2.0-1ubuntu1

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.