Re: [PATCH RFC v3 1/8] meson: add infrastructure to build internal Rust library

[Patrick Steinhardt <ps@pks.im>]

On Mon, Sep 08, 2025 at 10:09:37PM +0000, brian m. carlson wrote:
> On 2025-09-08 at 14:13:08, Patrick Steinhardt wrote:
> > diff --git a/Cargo.lock b/Cargo.lock
> > new file mode 100644
> > index 00000000000..2b80a01e22a
> > --- /dev/null
> > +++ b/Cargo.lock
> > @@ -0,0 +1,22 @@
> > +# This file is automatically @generated by Cargo.
> > +# It is not intended for manual editing.
> > +# Fix this to version 3. This is required so that older toolchains can still
> > +# read the lock file. Furthermore, while an argument could be made that we
> > +# should not even commit the "Cargo.lock" file in the first place, there's two
> > +# reasons to still do so:
> > +#
> > +#   - It thwarts supply-chain attacks by committing checksums into the
> > +#     repository.
> > +#
> > +#   - It is required by Meson so that it can extract Cargo dependencies.
> 
> If we check this in, then we basically cannot use any dependencies.  As
> I mentioned elsewhere, the problem is that invariably, if we're going to
> pin to an older version of Rust, we're going to be faced with the
> problem that some crate is going to require a security update that is
> also going to break older versions of Rust, and we will then have users
> aggressively demanding on the list that we update it immediately and
> ship a new release, breaking those older compilers.  (And yes, I've seen
> this happen with Go dependencies on Git LFS, even when the vulnerable
> code is not used.)

Hm. This one just feels weird to me. Doesn't it break reproducible
builds and create new attack vectors for supply-chain attacks?

> This is made worse by the fact that you want to support Rust 1.49
> instead of Rust 1.63, as I proposed.  Absent some compelling proposal on
> how we're going to deal with this situation, I think we need to omit
> `Cargo.lock`.

Just to clarify: this is only initially, until we have a good reason to
pick a later version of Rust. Right now, to the best of my knowledge
(and please correct me if I'm wrong), we don't have any reason to use
Rust 1.63 yet.

I'd like to pick the minimum version with a certain intent, where the
current intent is that 1.49 may ease the pressure on downstream users of
Git via gcc-rs at one point in time. Once there are reasons for why we
want a newer version of Rust though we should definitely discuss whether
it makes sense for us to bump the requirements.

Does that make sense?

> I think the better approach is to leave it out and use Cargo to build
> the Rust code instead of having Meson do it directly.
> 
> > +# Starting with Meson 1.5, it knows to parse the "Cargo.lock" file and extract
> > +# dependencies from it. So from hereon we don't need Cargo anymore to build
> > +# Git.
> 
> Ah, yes, I've already broken this in my branch (early this morning, in
> fact).  I've added a `build.rs` file (used by Cargo) which is necessary
> to properly link the tests against `libgit.a`.  (I'm using the hashing
> code in some of my tests.) Meson fails to honour that and so the
> compilation breaks.

Too bad.

> I don't think it's going to be viable to try to maintain two separate
> build systems that build the Rust code.  Everyone who uses rust-analyzer
> (the Rust LSP) will use Cargo because that's the build system it uses,
> and everyone uses Cargo anyway, so as a practical matter we need to
> support it.  Trying to have Meson do its own thing is unlikely to work
> here, and it demands that we use the `Cargo.lock` file, which we'd like
> to avoid.

Unfortunate, but probably fair. Let's take the simple route for now and
potentially iterate down the road.

> > +  cargo_command = [
> > +    cargo,
> > +    'build',
> > +    '--lib',
> > +    '--quiet',
> > +    '--manifest-path',
> > +    meson.project_source_root() / 'Cargo.toml',
> > +    '--target-dir',
> > +    meson.current_build_dir() / 'target',
> > +    # `--out-dir` is unstable, but supported since 2018. It's been recently
> > +    # renamed to `--artifact-dir`, but for now both options are supported.
> > +    '-Z',
> > +    'unstable-options',
> 
> `-Z` is only accepted in nightly versions of the compiler.  This won't
> work with stable Rust and it definitely won't work with either 1.63 or
> 1.49.  It didn't work for me using Rust 1.89.0 when I removed the other
> branch.

Huh, weird. No idea why it works on my system with Rust 1.89.0 then.

It's kind of puzzling that something as simple as specifying where Cargo
puts the build artifacts is a nightly feature. All I really want is to
say `cargo build -o $PATH`. Oh, well...

Patrick