On 2024-08-12 at 09:03:57, Eric Sunshine wrote:
> On Mon, Aug 12, 2024 at 4:15 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > The original iteration had this:
> >
> > * bikeshedding on the name (yes, really). There is an active, unrelated
> > CGit project [4] that we only recently became aware of. We originally
> > took the name "cgit" because at $DAYJOB we sometimes refer to git.git
> > as "cgit" to distinguish it from jgit [5].
>
> A tangent: Speaking of external/other projects, I don't think we've
> seen an explanation yet as to why this Rust wrapper is proposed as a
> `contrib/` item of Git itself, as opposed to being a separate project.
>
> I can only think of two possible reasons why they might want it in the
> Git project itself...
>
> (1) Easier access to the library portions of Git ("libgit") since that
> portion of the code is not otherwise published as a standalone
> library. However, a workable alternative would be for the Rust wrapper
> to carry its own "vendored"[1] copy of Git. This would also ensure
> more reliable builds since they wouldn't have to worry about the
> "libgit" API changing from under them, and can adjust for "libgit" API
> changes when they manually pull in a new vendored copy. Hence, I'm not
> convinced that this is a valid reason to carry the Rust wrapper in
> Git.
Please don't vendor packages like this. This means that every time
there's a security vulnerability in Git, even if we ship a public libgit
and even if the security impact doesn't affect libgit, every security
scanning tool will demand that the crate be updated immediately. This
wastes colossal amounts of work and it's hostile to distros as well, who
will have to repackage the source to remove the vendoring.
At work, I maintain a codebase using Nokogiri, which is a Ruby gem that
vendors libxml2 (which has an absolutely abysmal security record), and I
cannot tell you how much useless work I perform updating this gem
because of security scanners screaming about the vendored libxml2, even
though it's completely possible to use the system library.
> (2) Perhaps the intention is that this Rust wrapper work will allow
> Rust to be used within Git itself[3]? If that's the case, then
> `contrib/` seems the wrong resting place for this code.
I think contrib is the right place for now. We may in the future want
to include some Rust code in the codebase in some context, and this is a
good way for us to experiment with it on an interim basis and discover
whether that's a viable approach for us. Perhaps it is, and perhaps it
is not, but we'll have built the experience to know for certain.
I could imagine (and this is hypothetical and speculative) that we might
allow some sorts of performance optimizations and optional features in
Rust, but require core functionality be in C until Rust is better
supported on some platforms. That might be a nice way to improve things
like multithreading, which Rust excels at, for example.
As an example, I'd like to write a random-access tree parser[0] in Rust
(such as for "dev:README.md"), since our current approach performs
terribly on large trees (it's linear instead of logarithmic). However,
I'm not willing to write that code in C because it's a giant landmine of
memory unsafety and I think it's too likely to be buggy in C.
[0] I am aware that due to the structure of trees, this is not _always_
possible to do, but it is possible in _many_ cases, and optimistically
trying and falling back to the old approach may still be faster.
--
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA
Attachment:
signature.asc
Description: PGP signature
