Review for Package: rustc
[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.
This does need a security review, so I'll assign ubuntu-security
In addition security has to check and state if keeping the embeeded
llvm is ok for them.
List of specific binary packages to be promoted to main:
rustc, libstd-rust-dev, libstd-rust-1.57
Specific binary packages built, but NOT to be promoted to main:
rust-doc, rust-src, rust-all
Specific binary packages built yet unsure if they will be promoted,
@Foundations please clarify if you plan to seed any of those in main as well:
rust-gdb, rust-lldb, rust-clippy, rustfmt
This qas quite a complex case (as expected), I hope I didn't miss too much.
In any case if there are comments by others in regard to the case that you think
need to be done for full support that I missed to mention feel free to speak up
please and add a comment.
This case has rather many notes, required and recommended todos.
For a better reference to them as you are going to anser/implement them
I'm numbering them. Please when fixin/answering them refer to them by that
number so that we can more easily check later if all got adressed.
## Notes:
#1 - libstd-rust is known to be very light and almost whatever you do you need
further libs. We have in the context of this done an evaluation and identified
some very common libs (log, serde, tempfile, structopt -open for suggstions)
of almost always used libraries which I'd highly recommend to eventually MIR
as part of the base rust support to make sense overall and to provide support
for commonly used bits.
OTOH the current draft [1] of MIR rules for other rust using packages suggests
to vendor everything - until that changes the current situation is ok. But
when this ever changes the general rust support will need a set of the most
common libraries in main as the stdlib never is enough.
#2 - llvm-13 is needed in main which should soon happen anyway, not an extra
task
here. The one potential problem there is that so far never lldb-$ver was in
main but with rustc it will be needed. I didn't see an issue as the source is
in main and the further deps as well, but I have no deep insight into llvm,
there might be issues if one goes down that path in more detail.
## Required TODOs:
#3 - As part of the rules draft for later MIR of rust based packages [1] a few
structural requirements were identified which aren't part of rustc but the
rustc ecosystem. In particular that would include the following two tasks
that would need to be compleded to support rustc in a useful way for the
distribution overall. Those likely need to happen in dh-cargo:
#3a - the proper creation of a post-build .lock file and providing it in a
standard path like /usr/share/doc/$pkg/
#3b - Current build environments create X-Cargo-Built-Using but for plenty of
Ubuntu mechanisms it would be required to put that data in Built-Using
e.g. Component mismatch checks
#3c - Even rustc itself has 288 vendored rust sources, so we will need both of
the above .lock and built-using for rustc itself as well, not just other
packages using rust.
Both need to be implemented OR the rules need to be changed to end up in a
state that a developer can upload and support a rust based package without
re-inventing the wheel over and over again.
#4 - dh-cargo in general should be added to the MIR as an extra task as it is
part of the build environment just as much as cargo itself.
#5 - Such toolchains are prone to break if the underlying libs are changed e.g.
libc or binutils updates. So if no other tests are available, at least
leveraging the self-tests ran at build time should be promoted to also
run as autopkgtest to spot these kind of issues early on and avoid spreading
potential issues especially since in the rust ecosystem you can't just
rebuild the lib, you have to rebuild anyone using it.
#6 - as part of fixing/improving the tests please re-check bug 1688120 as for
full support no architecture should be left behind (maybe it is closed anyway
nowadays?)
#7 - define an exclude for rust-doc to avoid js dependencies in component
mismatches
#8 - rustc itself and dependencies are obvious to go to main; some others are
obvious to not go to main - like -doc, but to be sure are you planning
to also seed rust-gdb, rust-lldb, rust-clippy, rustfmt ?
Please state that clearly so that we can adapt the list of binary packages we
expect to be promoted when this is complete.
#9 - rustc is on the lto-disable-list but per request by doko/foundations and
thereby the MIR rules to be in main it has to do the LTO disabling in the
package. That shall remind and encourage the maintainer to actively try to
get it supported.
#10 - So far the statement in the MIR request about the embedded llvm lists
options
but was not clear. Do you want to keep the embedded llvm (to avoid version
mismatches, to keep modifications to rusts special llvm not conflicting with
others) or do you want to modify rustc to use the systems llvm toolchain.
Either way - we need a clear statement so that further steps are based
on the right assumption. For now I have assumed you'll want to keep the
embedded llvm. But as a required task - please state clearly which path you
choose.
## Recommended TODOs:
#11 - I appreciate that it was identified that the non-failing build tests
should be re-evaluated now that more focus is put onto it. I'd hope that after
identifying a few tests which happen to be more flaky then others the next
step could be to split things into "has to work, and if not build fails" as
well as "at least no more htan 20% of these shall fail". And over time
that should gradually be improved until all tests are reliable or disabled
for a documented reason.
#12 - The package does need to get a team bug subscriber before being
promoted
#13 - Unless there is a reason against it, please apply the fix for CVE
https://www.cve.org/CVERecord?id=CVE-2022-21658
As an alternative If you prefer that - I'm not suggesting this as I expect
it might cause way too much fallout - consider going fully to 1.58.1 being
the latest stable release
[Duplication]
There is no other package in main providing the same functionality.
[Dependencies]
OK:
- no other Dependencies to MIR due to this
- some dependencies to main packages
- many internal cross dependencies to src:rustc itself
- lldb-13 will be needed, that is part of llvm-toolchain which is in main
although v13 needs to be promoted in jammy (known)
- No dependencies in main that are only superficially tested requiring
more tests now.
Problems:
- you'll need to exclude the rust-doc package from auto-inclusion to avoid
the additional javascript dependencies
- check with the owning team of llvm-toolchain-13 (foundations as well) if
it would be a problem to promote lldb-13 as so far that binary pkg is only
in universe
[Embedded sources and static linking]
OK:
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard
Problems:
- static linking (ok for rust libs for now, but as identified by the reporter
libgit2 and llvm could be a problem here)
- embedded source present - again libgit2 / llvm
IIRC the rust llvm isn't only new, but also sometimes modified. Given
that we can expect rust (just like go) to need regular backports I assume
we will have to keep the embedded llvm, if that is fine for foundations to
maintain it twice that is fine as long as security agrees.
- embedded source present - in addition to non-rust libs mentioned above there
are a whopping 288 sources in vendor/*. That is just the way the rust
ecosystem works - this means that not only for rust-based packages, but also
for rustc and the stdlib itself we will need proper .lock to track the used
built-in components and their versions as well as proper built-using
statements. At least on the build all those code blocks are touched (we see
many deprecations and warnings which isn't reassuring since it is bundled
with it you'd expect it matches what is needed).
[Security]
OK:
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)
Problems:
- history of CVEs does look concerning
- does parse data formats (the code)
- does process arbitrary web content as in a usual build things are loaded from
external sources like crates.io
For this and the many other reasons outlined here this surely needs a
security review.
[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
- no special HW required
- no micro lib where tests make no sense
- no new python2 dependency
- no go package for extra constraints
Problems:
- test suite does not fail the build upon error.
- does not have a non-trivial test suite that runs as autopkgtest
[Packaging red flags]
OK:
- Ubuntu does carry a delta, but it is reasonable and maintenance under
control
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok
- Upstream update history is good
- Debian/Ubuntu update history is good and already including the expected
regular backports
- promoting this does not seem to cause issues for MOTUs that so far
maintained the package
Problems:
- the current release is not packaged, there is 1.58.1 (Jan 22) but we are
on 1.57 (Dec 21). That is sort of ok for a toolchain, as such bumps always
include potential to require updates in many depending packages.
https://blog.rust-lang.org/2022/01/13/Rust-1.58.0.html
https://blog.rust-lang.org/2022/01/20/Rust-1.58.1.html
But that also includes a CVE fix, and at least that we should apply now
I'd think: https://www.cve.org/CVERecord?id=CVE-2022-21658
I added a hint to recommended todos.
- d/rules are not clean (almost 500 LOC), this won't be an easy maintenance
but this also isn't a random MIR, foundations is aware that this is complex
and has the resources and people to do it - so that isn't a blocker here.
- Lintian warnings are rather massive, but I agree that so far for now none
of them are critical, so this isn't great but ok
- It is on the lto-disabled list, packages in main are expected to actively
try to support LTO. Therefore for those it is required to do the lto
disabling directly in the package to remind/force maintainers to try getting
it working (as requested by Doko/Foundations).
[Upstream red flags]
OK:
- no incautious use of malloc/sprintf
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH (usage is OK inside
tests)
- no use of user nobody
- no use of setuid
- no important open bugs (crashers, etc) in Debian or Ubuntu
- there are issues for sure, like a whole set of issues affecting backports
with bad dependencies that have to get under control
https://bugs.launchpad.net/ubuntu/+source/rustc/+bug/1721425
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=972829
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=928422
But none of them is for jammy or an issue right now.
- no dependency on webkit, qtwebkit, seed or libgoa-*
- not part of the UI for extra checks
- no translation present, but none needed for this case (dev tool)
Problems:
- Errors/warnings during the build actually a lot of them.
Some are by design like not found crates which we do not want to by
dynamically downloaded. But there are also plenty of deprecation warnings
even in the very sources they bundle. I guess that is nothing we can fix
on the Ubuntu side, but it will make debugging FTBFS harder as things are
very noisy even on the good case.
[1]: https://github.com/cpaelzer/ubuntu-mir/pull/3
** Bug watch added: Debian Bug tracker #972829
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=972829
** Bug watch added: Debian Bug tracker #928422
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=928422
** CVE added: https://cve.mitre.org/cgi-bin/cvename.cgi?name=2022-21658
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1957932
Title:
[MIR] rustc, cargo
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/cargo/+bug/1957932/+subscriptions
--
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
