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

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]

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.)

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`.

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.

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.

> +  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.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]