Proposal/Discussion: Turning parts of Git into libraries

>
> On 2023-02-17 at 21:12:23, Emily Shaffer wrote:
> > Hi folks,
>
> Hey,
>
> > I'm looking forward to the discussion!
>
> While I'm not personally interested in the VFS work, I think it's a
> great idea to turn more of the code into libraries (or at least make it
> more library-like), and so I'm fully in support of this approach.

>
> On 2023-02-17 at 21:38:34, Emily Shaffer wrote:
> > For example, I seem to remember you saying during the SHA-256 series
> > that the next hashing algorithm would also be painful to implement;
> > would that still be true if the hashing algorithm is encapsulated well
> > by a library interface? Or is it for a different reason?
>
> Right now, most of the code for a future hash algorithm wouldn't be too
> difficult to implement, I'd think, because we can already support two of
> them.  If we decide, say, to implement SHA-3-512, we basically just add
> that algorithm, update all the entries in the tests (which is kind of a
> pain since there's a lot of them, but not really difficult), and then
> move on with our lives.
>
> The difficulty is dealing with interop work, which is basically
> switching from dealing with just one algorithm to rewriting things
> between the two on the fly.  I think _that_ work would be made easier by
> library work because sometimes it involves working with submodules, such
> as when updating the submodule commit, and being able to deal with both
> object stores more easily at the same time would be very helpful in that
> regard.

> I can imagine there are other things that would be easier as well, and I
> can also imagine that we'll have better control over memory allocations
> and leak less, which would be nice.  If we can get leaks low enough, we
> could even add CI jobs to catch them and fail, which I think would be
> super valuable, especially since I find even after over two decades of C
> that I'm still not very good about catching all the leaks (which is one
> of the reasons I've mostly switched to Rust).  We might also be able to
> make nicer steps on multithreading our code as well.
>
> Personally, I'd like to see some sort of standard error type (whether
> integral or not) that would let us do more bubbling up of errors and
> less die().  I don't know if that's in the cards, but I thought I'd
> suggest it in case other folks are interested.

> > Personally, I'd like to see some sort of standard error type (whether
> > integral or not) that would let us do more bubbling up of errors and
> > less die().  I don't know if that's in the cards, but I thought I'd
> > suggest it in case other folks are interested.
> 
> Yes!!! We have talked about this a lot internally - but this is one
> thing that will be difficult to introduce into Git without making
> parts of the codebase a little uglier. Since obviously C doesn't have
> an intrinsic to do this, we'll have to roll our own, which means that
> manipulating it consistently at function exits might end up pretty
> ugly. So hearing that there's interest outside of my team to come up
> with such a type makes me optimistic that we can figure out a
> neat-enough solution.

>
> On Fri, Feb 17, 2023 at 02:49:51PM -0800, Emily Shaffer wrote:
>
> > > Personally, I'd like to see some sort of standard error type (whether
> > > integral or not) that would let us do more bubbling up of errors and
> > > less die().  I don't know if that's in the cards, but I thought I'd
> > > suggest it in case other folks are interested.
> >
> > Yes!!! We have talked about this a lot internally - but this is one
> > thing that will be difficult to introduce into Git without making
> > parts of the codebase a little uglier. Since obviously C doesn't have
> > an intrinsic to do this, we'll have to roll our own, which means that
> > manipulating it consistently at function exits might end up pretty
> > ugly. So hearing that there's interest outside of my team to come up
> > with such a type makes me optimistic that we can figure out a
> > neat-enough solution.
>
> Here are some past discussions on what I thought would be a good
> approach to error handling. The basic idea is to replace the "pass a
> strbuf that people shove error messages into" approach with an error
> context struct that has a callback. And that callback can then stuff
> them into a strbuf, or report them directly, or even die.

>
> This thread sketches out the idea, though sadly I no longer have the
> more fleshed-out patches I mentioned there:
>
>   https://lore.kernel.org/git/20160927191955.mympqgylrxhkp24n@sigill.intra.peff.net/
>
> And then the sub-thread starting here discusses a similar approach:
>
>   https://lore.kernel.org/git/20171103191309.sth4zjokgcupvk2e@sigill.intra.peff.net/
>
> It does mean passing a "struct error_context" just about everywhere.
> Though since the context doesn't change very much and most calls are
> just forwarding it along, it would probably also be reasonable to have a
> thread-local global context, and push/pop from it (sort of a poor man's
> dynamic scoping).
>
> One thing that strategy doesn't help with, though, that your
> libification might want: it's not very machine-readable. The error
> reporters would still fundamentally be working with strings. So a
> libified process can know "OK, writing this ref failed, and I have some
> error messages in a buffer". But the calling code can't know specifics
> like "it failed because we tried to open file 'foo' and it got EPERM".
> We _could_ design an error context that stores individual errno values
> or codes in a list, but having each caller report those specific errors
> is a much bigger job (and ongoing maintenance burden as we catalogue and
> give an identifier to each error).

> > One thing that strategy doesn't help with, though, that your
> > libification might want: it's not very machine-readable. The error
> > reporters would still fundamentally be working with strings. So a
> > libified process can know "OK, writing this ref failed, and I have some
> > error messages in a buffer". But the calling code can't know specifics
> > like "it failed because we tried to open file 'foo' and it got EPERM".
> > We _could_ design an error context that stores individual errno values
> > or codes in a list, but having each caller report those specific errors
> > is a much bigger job (and ongoing maintenance burden as we catalogue and
> > give an identifier to each error).
> 
> Is there a reason not to use this kind of struct and provide
> library-specific error code enums, though, I wonder? You're right that
> parsing the error string is really bad for the caller, for anything
> besides just logging it. But it seems somewhat reasonable to expect
> that any call from config library returning an integer error code is
> referring to enum config_errors...

> Is there a reason not to use this kind of struct and provide
> library-specific error code enums, though, I wonder? You're right that
> parsing the error string is really bad for the caller, for anything
> besides just logging it. But it seems somewhat reasonable to expect
> that any call from config library returning an integer error code is
> referring to enum config_errors...

>On 2023-02-17 at 21:12:23, Emily Shaffer wrote:
>> Hi folks,
>
>Hey,
>
>> I'm looking forward to the discussion!
>
>While I'm not personally interested in the VFS work, I think it's a great idea to turn
>more of the code into libraries (or at least make it more library-like), and so I'm fully
>in support of this approach.  When I send patches in the future, I'll try to make sure
>that they're friendly to this goal.

> I am uncertain about this, from a licensing standpoint. Typically,
> when one links in a library from one project, the license from that
> project may inherit into your own project. AFAIK, GPLv3 has this
> implied provision - I do not think it is explicit, but the implication
> seems to be there. Making git libraries has the potential to cause
> git's license rights to be incorporated into other products. I am
> suggesting that we would need to tread carefully in this area. Using
> someone else's DLL is not so bad, as the code is not bound together,
> but may also cause ambiguities depending on whether the licenses are
> conflicting or not. I am not suggesting that this is a bad idea, just
> one that should be handled carefully.

> Basically, if this effort turns out not to be fruitful as a whole, I'd
> like for us to still have left a positive impact on the codebase.
> ...
> So what's next? Naturally, I'm looking forward to a spirited
> discussion about this topic - I'd like to know which concerns haven't
> been addressed and figure out whether we can find a way around them,
> and generally build awareness of this effort with the community.

>
> Emily Shaffer <nasamuffin@google.com> writes:
>
> > Basically, if this effort turns out not to be fruitful as a whole, I'd
> > like for us to still have left a positive impact on the codebase.
> > ...
> > So what's next? Naturally, I'm looking forward to a spirited
> > discussion about this topic - I'd like to know which concerns haven't
> > been addressed and figure out whether we can find a way around them,
> > and generally build awareness of this effort with the community.
>
> On of the gravest concerns is that the devil is in the details.
>
> For example, "die() is inconvenient to callers, let's propagate
> errors up the callchain" is an easy thing to say, but it would take
> much more than "let's propagate errors up" to libify something like
> check_connected() to do the same thing without spawning a separate
> process that is expected to exit with failure.

> On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Emily Shaffer <nasamuffin@google.com> writes:
>>
>>> Basically, if this effort turns out not to be fruitful as a whole, I'd
>>> like for us to still have left a positive impact on the codebase.
>>> ...
>>> So what's next? Naturally, I'm looking forward to a spirited
>>> discussion about this topic - I'd like to know which concerns haven't
>>> been addressed and figure out whether we can find a way around them,
>>> and generally build awareness of this effort with the community.
>>
>> On of the gravest concerns is that the devil is in the details.
>>
>> For example, "die() is inconvenient to callers, let's propagate
>> errors up the callchain" is an easy thing to say, but it would take
>> much more than "let's propagate errors up" to libify something like
>> check_connected() to do the same thing without spawning a separate
>> process that is expected to exit with failure.
> 
> 
> What does "propagate errors up the callchain" mean?  One
> interpretation I can think of seems quite horrible, but another seems
> quite doable and reasonable and likely not even very invasive of the
> existing code:
> 
> You can use setjmp/longjmp to implement a form of "try", so that
> errors dont have to be *explicitly* returned *in* the call chain. And
> you could probably do so without changing very much of the existing
> code at all, and maintain a high level of conceptual alignment with
> the current code strategy.

> To do this you need to set up a globally available linked list of
> jmp_env data (see `man setjmp` for jmp_env), and a global error
> object, and make the existing "die" functions populate the global
> error object, and then pop the most recent jmp_env data and longjmp to
> it.
> 
> At the top of any git invocation you would set up the topmost jmp_env
> "frame". Any code that wants to "try" existing logic pushes a new
> jmp_env (using a wrapper around setjmp), and prepares to be longjmp'ed
> to. If the code does not die then it pops the jmp_env it just pushed
> and returns as normal, if it is longjmp'ed to you can detect this and
> do some other behavior to handle the exception (by reading the global
> error object). If the code that died *really* wants to exit, then it
> returns the appropriate code as part of the longjmp, and the try
> handler longjmps again propagating up the chain. Eventually you either
> have an error that "propagates to the top" which results in an exit
> with an appropriate error message, or you have an error that is
> trapped and the code does something else, and then eventually returns
> normally.
> 
> FWIW, this is essentially a loose description of how Perl handles the
> execution part of "eval" and supports exception handling internally.
> Most of the perl internals do not know anything about exceptions, they
> just call functions similar to gits die functions if they need to,
> which then call into Perl_die_unwind(). which then calls the
> JUMPENV_JUMP() macro which does the "pop and longjmp" dance.
> 
> Seems to me that it wouldn't be very difficult nor particularly
> invasive to implement this in git. Much of the logic in the perl
> project to do this is at the top of cop.h,  see the macros
> JMPENV_PUSH(), JMPENV_POP(), JMPENV_JUMP(). Obviously this code
> contains a bunch of perl specific logic, but the general gist of it
> should be easily understood and easily converted to a more git like
> context:
> 
> struct jmpenv: https://github.com/Perl/perl5/blob/blead/cop.h#L32
> JMPENV_BOOTSTRAP: https://github.com/Perl/perl5/blob/blead/cop.h#L66
> JMPENV_PUSH: https://github.com/Perl/perl5/blob/blead/cop.h#L113
> JMPENV_POP: https://github.com/Perl/perl5/blob/blead/cop.h#L147
> JMPENV_JUMP: https://github.com/Perl/perl5/blob/blead/cop.h#L159
> 
> Perl_die_unwind: https://github.com/Perl/perl5/blob/blead/pp_ctl.c#L1741
> Where Perl_die_unwind() calls JMPENV_JUMP:
> https://github.com/Perl/perl5/blob/blead/pp_ctl.c#L1865
> 
> You can also grep for functions of the form S_try_ in the perl code
> base to find examples where the C code explicitly sets up an "eval
> frame" to interoperate with the functionality above.
> 
> git grep -nP '^S_try_'
> pp_ctl.c:3548:S_try_yyparse(pTHX_ int gramtype, OP *caller_op)
> pp_ctl.c:3604:S_try_run_unitcheck(pTHX_ OP* caller_op)
> pp_sys.c:3120:S_try_amagic_ftest(pTHX_ char chr) {
> 
> Seems to me that this gives enough prior art to convert git to use the
> same strategy, and that doing so would not actually be that big a
> change to the existing code.  Both environments are fairly similar if
> you look at them from the right perspective. Both are C, and both have
> a lot of global state, and both have lots of functions which you
> really dont want to have to change to understand about exception
> objects..
> 
> Here is an example of how a C function might be written to use this
> kind of infrastructure to "try" functionality that might call die. In
> this case there is no need for the code to inspect the global error
> object, but the basic pattern is consistent. The "default" case below
> handles the situation where the "tried" function is signalling an
> "untrappable error" that needs to be rethrown to ultimately unwind the
> entire try/catch chain and exit the program. It is derived and
> simplified from S_try_yyparse mentioned above. This function handles
> the "compile the code" part of an `eval EXPR`, and traps exceptions
> from the parser so that they can be handled properly and distinctly
> from errors trapped during execution of the compiled code. [ I am
> assuming that given the historical relationship between git and perl
> these concepts are not alien to everybody on this list. ]
> 
> /* S_try_yyparse():
>   *
>   * Run yyparse() in a setjmp wrapper. Returns:
>   *   0: yyparse() successful
>   *   1: yyparse() failed
>   *   3: yyparse() died
>   *
>   * ...
>   */
> STATIC int
> S_try_yyparse(pTHX_ int gramtype, ...)
> {
>      dJMPENV;
> 
>      JMPENV_PUSH(ret);
>      switch (ret) {
>      case 0:
>          ret = yyparse(gramtype) ? 1 : 0;
>          break;
>      case 3:
>          /* yyparse() died and we trapped the error. */
>          ....
>          break;
>      default:
>          JMPENV_POP;          /* remove our own setjmp data */
>          JMPENV_JUMP(ret); /* RETHROW */
>      }
>      JMPENV_POP;
>      return ret;
> }
> 

>
> On 18/02/2023 01:59, demerphq wrote:
> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Emily Shaffer <nasamuffin@google.com> writes:
> >>
> >>> Basically, if this effort turns out not to be fruitful as a whole, I'd
> >>> like for us to still have left a positive impact on the codebase.
> >>> ...
> >>> So what's next? Naturally, I'm looking forward to a spirited
> >>> discussion about this topic - I'd like to know which concerns haven't
> >>> been addressed and figure out whether we can find a way around them,
> >>> and generally build awareness of this effort with the community.
> >>
> >> On of the gravest concerns is that the devil is in the details.
> >>
> >> For example, "die() is inconvenient to callers, let's propagate
> >> errors up the callchain" is an easy thing to say, but it would take
> >> much more than "let's propagate errors up" to libify something like
> >> check_connected() to do the same thing without spawning a separate
> >> process that is expected to exit with failure.
> >
> >
> > What does "propagate errors up the callchain" mean?  One
> > interpretation I can think of seems quite horrible, but another seems
> > quite doable and reasonable and likely not even very invasive of the
> > existing code:
> >
> > You can use setjmp/longjmp to implement a form of "try", so that
> > errors dont have to be *explicitly* returned *in* the call chain. And
> > you could probably do so without changing very much of the existing
> > code at all, and maintain a high level of conceptual alignment with
> > the current code strategy.
>
> Using setjmp/longjmp is an interesting suggestion, I think lua does
> something similar to what you describe for perl. However I think both of
> those use a allocator with garbage collection. I worry that using
> longjmp in git would be more invasive (or result in more memory leaks)
> as we'd need to to guard each allocation with some code to clean it up
> and then propagate the error. That means we're back to manually
> propagating errors up the call chain in many cases.

>On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> On 18/02/2023 01:59, demerphq wrote:
>> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
>> >>
>> >> Emily Shaffer <nasamuffin@google.com> writes:
>> >>
>> >>> Basically, if this effort turns out not to be fruitful as a whole,
>> >>> I'd like for us to still have left a positive impact on the codebase.
>> >>> ...
>> >>> So what's next? Naturally, I'm looking forward to a spirited
>> >>> discussion about this topic - I'd like to know which concerns
>> >>> haven't been addressed and figure out whether we can find a way
>> >>> around them, and generally build awareness of this effort with the community.
>> >>
>> >> On of the gravest concerns is that the devil is in the details.
>> >>
>> >> For example, "die() is inconvenient to callers, let's propagate
>> >> errors up the callchain" is an easy thing to say, but it would take
>> >> much more than "let's propagate errors up" to libify something like
>> >> check_connected() to do the same thing without spawning a separate
>> >> process that is expected to exit with failure.
>> >
>> >
>> > What does "propagate errors up the callchain" mean?  One
>> > interpretation I can think of seems quite horrible, but another
>> > seems quite doable and reasonable and likely not even very invasive
>> > of the existing code:
>> >
>> > You can use setjmp/longjmp to implement a form of "try", so that
>> > errors dont have to be *explicitly* returned *in* the call chain.
>> > And you could probably do so without changing very much of the
>> > existing code at all, and maintain a high level of conceptual
>> > alignment with the current code strategy.
>>
>> Using setjmp/longjmp is an interesting suggestion, I think lua does
>> something similar to what you describe for perl. However I think both
>> of those use a allocator with garbage collection. I worry that using
>> longjmp in git would be more invasive (or result in more memory leaks)
>> as we'd need to to guard each allocation with some code to clean it up
>> and then propagate the error. That means we're back to manually
>> propagating errors up the call chain in many cases.
>
>We could just use talloc [1].

>
> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> On 18/02/2023 01:59, demerphq wrote:
> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
> >> >>
> >> >> Emily Shaffer <nasamuffin@google.com> writes:
> >> >>
> >> >>> Basically, if this effort turns out not to be fruitful as a whole,
> >> >>> I'd like for us to still have left a positive impact on the codebase.
> >> >>> ...
> >> >>> So what's next? Naturally, I'm looking forward to a spirited
> >> >>> discussion about this topic - I'd like to know which concerns
> >> >>> haven't been addressed and figure out whether we can find a way
> >> >>> around them, and generally build awareness of this effort with the community.
> >> >>
> >> >> On of the gravest concerns is that the devil is in the details.
> >> >>
> >> >> For example, "die() is inconvenient to callers, let's propagate
> >> >> errors up the callchain" is an easy thing to say, but it would take
> >> >> much more than "let's propagate errors up" to libify something like
> >> >> check_connected() to do the same thing without spawning a separate
> >> >> process that is expected to exit with failure.
> >> >
> >> >
> >> > What does "propagate errors up the callchain" mean?  One
> >> > interpretation I can think of seems quite horrible, but another
> >> > seems quite doable and reasonable and likely not even very invasive
> >> > of the existing code:
> >> >
> >> > You can use setjmp/longjmp to implement a form of "try", so that
> >> > errors dont have to be *explicitly* returned *in* the call chain.
> >> > And you could probably do so without changing very much of the
> >> > existing code at all, and maintain a high level of conceptual
> >> > alignment with the current code strategy.
> >>
> >> Using setjmp/longjmp is an interesting suggestion, I think lua does
> >> something similar to what you describe for perl. However I think both
> >> of those use a allocator with garbage collection. I worry that using
> >> longjmp in git would be more invasive (or result in more memory leaks)
> >> as we'd need to to guard each allocation with some code to clean it up
> >> and then propagate the error. That means we're back to manually
> >> propagating errors up the call chain in many cases.
> >
> >We could just use talloc [1].
>
> talloc is not portable.

>On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
>>
>> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
>> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood <phillip.wood123@gmail.com>
>wrote:
>> >>
>> >> On 18/02/2023 01:59, demerphq wrote:
>> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
>> >> >>
>> >> >> Emily Shaffer <nasamuffin@google.com> writes:
>> >> >>
>> >> >>> Basically, if this effort turns out not to be fruitful as a
>> >> >>> whole, I'd like for us to still have left a positive impact on the codebase.
>> >> >>> ...
>> >> >>> So what's next? Naturally, I'm looking forward to a spirited
>> >> >>> discussion about this topic - I'd like to know which concerns
>> >> >>> haven't been addressed and figure out whether we can find a way
>> >> >>> around them, and generally build awareness of this effort with the
>community.
>> >> >>
>> >> >> On of the gravest concerns is that the devil is in the details.
>> >> >>
>> >> >> For example, "die() is inconvenient to callers, let's propagate
>> >> >> errors up the callchain" is an easy thing to say, but it would
>> >> >> take much more than "let's propagate errors up" to libify
>> >> >> something like
>> >> >> check_connected() to do the same thing without spawning a
>> >> >> separate process that is expected to exit with failure.
>> >> >
>> >> >
>> >> > What does "propagate errors up the callchain" mean?  One
>> >> > interpretation I can think of seems quite horrible, but another
>> >> > seems quite doable and reasonable and likely not even very
>> >> > invasive of the existing code:
>> >> >
>> >> > You can use setjmp/longjmp to implement a form of "try", so that
>> >> > errors dont have to be *explicitly* returned *in* the call chain.
>> >> > And you could probably do so without changing very much of the
>> >> > existing code at all, and maintain a high level of conceptual
>> >> > alignment with the current code strategy.
>> >>
>> >> Using setjmp/longjmp is an interesting suggestion, I think lua does
>> >> something similar to what you describe for perl. However I think
>> >> both of those use a allocator with garbage collection. I worry that
>> >> using longjmp in git would be more invasive (or result in more
>> >> memory leaks) as we'd need to to guard each allocation with some
>> >> code to clean it up and then propagate the error. That means we're
>> >> back to manually propagating errors up the call chain in many cases.
>> >
>> >We could just use talloc [1].
>>
>> talloc is not portable.
>
>What makes you say that?

>
> On Thursday, March 23, 2023 7:35 PM, Felipe Contreras wrote:
> >On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
> >>
> >> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
> >> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood <phillip.wood123@gmail.com>
> >wrote:
> >> >>
> >> >> On 18/02/2023 01:59, demerphq wrote:
> >> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com> wrote:
> >> >> >>
> >> >> >> Emily Shaffer <nasamuffin@google.com> writes:
> >> >> >>
> >> >> >>> Basically, if this effort turns out not to be fruitful as a
> >> >> >>> whole, I'd like for us to still have left a positive impact on the codebase.
> >> >> >>> ...
> >> >> >>> So what's next? Naturally, I'm looking forward to a spirited
> >> >> >>> discussion about this topic - I'd like to know which concerns
> >> >> >>> haven't been addressed and figure out whether we can find a way
> >> >> >>> around them, and generally build awareness of this effort with the
> >community.
> >> >> >>
> >> >> >> On of the gravest concerns is that the devil is in the details.
> >> >> >>
> >> >> >> For example, "die() is inconvenient to callers, let's propagate
> >> >> >> errors up the callchain" is an easy thing to say, but it would
> >> >> >> take much more than "let's propagate errors up" to libify
> >> >> >> something like
> >> >> >> check_connected() to do the same thing without spawning a
> >> >> >> separate process that is expected to exit with failure.
> >> >> >
> >> >> >
> >> >> > What does "propagate errors up the callchain" mean?  One
> >> >> > interpretation I can think of seems quite horrible, but another
> >> >> > seems quite doable and reasonable and likely not even very
> >> >> > invasive of the existing code:
> >> >> >
> >> >> > You can use setjmp/longjmp to implement a form of "try", so that
> >> >> > errors dont have to be *explicitly* returned *in* the call chain.
> >> >> > And you could probably do so without changing very much of the
> >> >> > existing code at all, and maintain a high level of conceptual
> >> >> > alignment with the current code strategy.
> >> >>
> >> >> Using setjmp/longjmp is an interesting suggestion, I think lua does
> >> >> something similar to what you describe for perl. However I think
> >> >> both of those use a allocator with garbage collection. I worry that
> >> >> using longjmp in git would be more invasive (or result in more
> >> >> memory leaks) as we'd need to to guard each allocation with some
> >> >> code to clean it up and then propagate the error. That means we're
> >> >> back to manually propagating errors up the call chain in many cases.
> >> >
> >> >We could just use talloc [1].
> >>
> >> talloc is not portable.
> >
> >What makes you say that?
>
> talloc is not part of a POSIX standard I could find.

>On Thu, Mar 23, 2023 at 5:43 PM <rsbecker@nexbridge.com> wrote:
>>
>> On Thursday, March 23, 2023 7:35 PM, Felipe Contreras wrote:
>> >On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
>> >>
>> >> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
>> >> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood
>> >> ><phillip.wood123@gmail.com>
>> >wrote:
>> >> >>
>> >> >> On 18/02/2023 01:59, demerphq wrote:
>> >> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com>
>wrote:
>> >> >> >>
>> >> >> >> Emily Shaffer <nasamuffin@google.com> writes:
>> >> >> >>
>> >> >> >>> Basically, if this effort turns out not to be fruitful as a
>> >> >> >>> whole, I'd like for us to still have left a positive impact on the codebase.
>> >> >> >>> ...
>> >> >> >>> So what's next? Naturally, I'm looking forward to a spirited
>> >> >> >>> discussion about this topic - I'd like to know which
>> >> >> >>> concerns haven't been addressed and figure out whether we
>> >> >> >>> can find a way around them, and generally build awareness of
>> >> >> >>> this effort with the
>> >community.
>> >> >> >>
>> >> >> >> On of the gravest concerns is that the devil is in the details.
>> >> >> >>
>> >> >> >> For example, "die() is inconvenient to callers, let's
>> >> >> >> propagate errors up the callchain" is an easy thing to say,
>> >> >> >> but it would take much more than "let's propagate errors up"
>> >> >> >> to libify something like
>> >> >> >> check_connected() to do the same thing without spawning a
>> >> >> >> separate process that is expected to exit with failure.
>> >> >> >
>> >> >> >
>> >> >> > What does "propagate errors up the callchain" mean?  One
>> >> >> > interpretation I can think of seems quite horrible, but
>> >> >> > another seems quite doable and reasonable and likely not even
>> >> >> > very invasive of the existing code:
>> >> >> >
>> >> >> > You can use setjmp/longjmp to implement a form of "try", so
>> >> >> > that errors dont have to be *explicitly* returned *in* the call chain.
>> >> >> > And you could probably do so without changing very much of the
>> >> >> > existing code at all, and maintain a high level of conceptual
>> >> >> > alignment with the current code strategy.
>> >> >>
>> >> >> Using setjmp/longjmp is an interesting suggestion, I think lua
>> >> >> does something similar to what you describe for perl. However I
>> >> >> think both of those use a allocator with garbage collection. I
>> >> >> worry that using longjmp in git would be more invasive (or
>> >> >> result in more memory leaks) as we'd need to to guard each
>> >> >> allocation with some code to clean it up and then propagate the
>> >> >> error. That means we're back to manually propagating errors up the call
>chain in many cases.
>> >> >
>> >> >We could just use talloc [1].
>> >>
>> >> talloc is not portable.
>> >
>> >What makes you say that?
>>
>> talloc is not part of a POSIX standard I could find.
>
>It's a library, like: z, ssl, curl, pcre2-8, etc. Libraries can be compiled on different
>platforms.

>
> On Thursday, March 23, 2023 7:55 PM, Felipe Contreras wrote:
> >On Thu, Mar 23, 2023 at 5:43 PM <rsbecker@nexbridge.com> wrote:
> >>
> >> On Thursday, March 23, 2023 7:35 PM, Felipe Contreras wrote:
> >> >On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
> >> >>
> >> >> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
> >> >> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood
> >> >> ><phillip.wood123@gmail.com>
> >> >wrote:
> >> >> >>
> >> >> >> On 18/02/2023 01:59, demerphq wrote:
> >> >> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano <gitster@pobox.com>
> >wrote:
> >> >> >> >>
> >> >> >> >> Emily Shaffer <nasamuffin@google.com> writes:
> >> >> >> >>
> >> >> >> >>> Basically, if this effort turns out not to be fruitful as a
> >> >> >> >>> whole, I'd like for us to still have left a positive impact on the codebase.
> >> >> >> >>> ...
> >> >> >> >>> So what's next? Naturally, I'm looking forward to a spirited
> >> >> >> >>> discussion about this topic - I'd like to know which
> >> >> >> >>> concerns haven't been addressed and figure out whether we
> >> >> >> >>> can find a way around them, and generally build awareness of
> >> >> >> >>> this effort with the
> >> >community.
> >> >> >> >>
> >> >> >> >> On of the gravest concerns is that the devil is in the details.
> >> >> >> >>
> >> >> >> >> For example, "die() is inconvenient to callers, let's
> >> >> >> >> propagate errors up the callchain" is an easy thing to say,
> >> >> >> >> but it would take much more than "let's propagate errors up"
> >> >> >> >> to libify something like
> >> >> >> >> check_connected() to do the same thing without spawning a
> >> >> >> >> separate process that is expected to exit with failure.
> >> >> >> >
> >> >> >> >
> >> >> >> > What does "propagate errors up the callchain" mean?  One
> >> >> >> > interpretation I can think of seems quite horrible, but
> >> >> >> > another seems quite doable and reasonable and likely not even
> >> >> >> > very invasive of the existing code:
> >> >> >> >
> >> >> >> > You can use setjmp/longjmp to implement a form of "try", so
> >> >> >> > that errors dont have to be *explicitly* returned *in* the call chain.
> >> >> >> > And you could probably do so without changing very much of the
> >> >> >> > existing code at all, and maintain a high level of conceptual
> >> >> >> > alignment with the current code strategy.
> >> >> >>
> >> >> >> Using setjmp/longjmp is an interesting suggestion, I think lua
> >> >> >> does something similar to what you describe for perl. However I
> >> >> >> think both of those use a allocator with garbage collection. I
> >> >> >> worry that using longjmp in git would be more invasive (or
> >> >> >> result in more memory leaks) as we'd need to to guard each
> >> >> >> allocation with some code to clean it up and then propagate the
> >> >> >> error. That means we're back to manually propagating errors up the call
> >chain in many cases.
> >> >> >
> >> >> >We could just use talloc [1].
> >> >>
> >> >> talloc is not portable.
> >> >
> >> >What makes you say that?
> >>
> >> talloc is not part of a POSIX standard I could find.
> >
> >It's a library, like: z, ssl, curl, pcre2-8, etc. Libraries can be compiled on different
> >platforms.
>
> talloc adds additional *required* dependencies to git, including python3 - required to configure and build talloc - which is not available on the NonStop ia64 platform (required support through end of 2025). I must express my resistance to what would amount to losing support for git on this NonStop platform.

>On Fri, Mar 24, 2023 at 1:28 PM <rsbecker@nexbridge.com> wrote:
>>
>> On Thursday, March 23, 2023 7:55 PM, Felipe Contreras wrote:
>> >On Thu, Mar 23, 2023 at 5:43 PM <rsbecker@nexbridge.com> wrote:
>> >>
>> >> On Thursday, March 23, 2023 7:35 PM, Felipe Contreras wrote:
>> >> >On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
>> >> >>
>> >> >> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
>> >> >> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood
>> >> >> ><phillip.wood123@gmail.com>
>> >> >wrote:
>> >> >> >>
>> >> >> >> On 18/02/2023 01:59, demerphq wrote:
>> >> >> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano
>> >> >> >> > <gitster@pobox.com>
>> >wrote:
>> >> >> >> >>
>> >> >> >> >> Emily Shaffer <nasamuffin@google.com> writes:
>> >> >> >> >>
>> >> >> >> >>> Basically, if this effort turns out not to be fruitful as
>> >> >> >> >>> a whole, I'd like for us to still have left a positive impact on the
>codebase.
>> >> >> >> >>> ...
>> >> >> >> >>> So what's next? Naturally, I'm looking forward to a
>> >> >> >> >>> spirited discussion about this topic - I'd like to know
>> >> >> >> >>> which concerns haven't been addressed and figure out
>> >> >> >> >>> whether we can find a way around them, and generally
>> >> >> >> >>> build awareness of this effort with the
>> >> >community.
>> >> >> >> >>
>> >> >> >> >> On of the gravest concerns is that the devil is in the details.
>> >> >> >> >>
>> >> >> >> >> For example, "die() is inconvenient to callers, let's
>> >> >> >> >> propagate errors up the callchain" is an easy thing to
>> >> >> >> >> say, but it would take much more than "let's propagate errors up"
>> >> >> >> >> to libify something like
>> >> >> >> >> check_connected() to do the same thing without spawning a
>> >> >> >> >> separate process that is expected to exit with failure.
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > What does "propagate errors up the callchain" mean?  One
>> >> >> >> > interpretation I can think of seems quite horrible, but
>> >> >> >> > another seems quite doable and reasonable and likely not
>> >> >> >> > even very invasive of the existing code:
>> >> >> >> >
>> >> >> >> > You can use setjmp/longjmp to implement a form of "try", so
>> >> >> >> > that errors dont have to be *explicitly* returned *in* the call chain.
>> >> >> >> > And you could probably do so without changing very much of
>> >> >> >> > the existing code at all, and maintain a high level of
>> >> >> >> > conceptual alignment with the current code strategy.
>> >> >> >>
>> >> >> >> Using setjmp/longjmp is an interesting suggestion, I think
>> >> >> >> lua does something similar to what you describe for perl.
>> >> >> >> However I think both of those use a allocator with garbage
>> >> >> >> collection. I worry that using longjmp in git would be more
>> >> >> >> invasive (or result in more memory leaks) as we'd need to to
>> >> >> >> guard each allocation with some code to clean it up and then
>> >> >> >> propagate the error. That means we're back to manually
>> >> >> >> propagating errors up the call
>> >chain in many cases.
>> >> >> >
>> >> >> >We could just use talloc [1].
>> >> >>
>> >> >> talloc is not portable.
>> >> >
>> >> >What makes you say that?
>> >>
>> >> talloc is not part of a POSIX standard I could find.
>> >
>> >It's a library, like: z, ssl, curl, pcre2-8, etc. Libraries can be
>> >compiled on different platforms.
>>
>> talloc adds additional *required* dependencies to git, including python3 - required
>to configure and build talloc - which is not available on the NonStop ia64 platform
>(required support through end of 2025). I must express my resistance to what would
>amount to losing support for git on this NonStop platform.
>
>That is not true. You don't need python3 for talloc, not even to build it, it's just a
>single simple c file, it's easy to compile.
>
>The only reason python is used is to run waf, which is used to build Samba, which is
>much more complex, but you don't need to run it, especially if you know the
>characteristics of your system.
>
>This simple Makefile builds libtalloc.so just fine:
>
>  CC := gcc
>  CFLAGS := -fPIC -I./lib/replace
>  LDFLAGS := -Wl,--no-undefined
>
>  # For talloc.c
>  CFLAGS += -DTALLOC_BUILD_VERSION_MAJOR=2
>-DTALLOC_BUILD_VERSION_MINOR=4 -DTALLOC_BUILD_VERSION_RELEASE=0
>  CFLAGS += -DHAVE_CONSTRUCTOR_ATTRIBUTE -DHAVE_VA_COPY -
>DHAVE_VALGRIND_MEMCHECK_H -DHAVE_INTPTR_T
>
>  # For replace.h
>  CFLAGS += -DNO_CONFIG_H -D__STDC_WANT_LIB_EXT1__=1
>  CFLAGS += -DHAVE_STDBOOL_H -DHAVE_BOOL -DHAVE_STRING_H -
>DHAVE_LIMITS_H -DHAVE_STDINT_H
>  CFLAGS += -DHAVE_DLFCN_H -DHAVE_UINTPTR_T -DHAVE_C99_VSNPRINTF -
>DHAVE_MEMMOVE -DHAVE_STRNLEN -DHAVE_VSNPRINTF
>
>  libtalloc.so: talloc.o
>    $(CC) $(LDFLAGS) -shared -o $@ $^
>
>But of course, most of those defines are not even needed with a simple "replace.h"
>that is less than 10 lines of code.

>
> On Friday, March 24, 2023 5:22 PM, Felipe Contreras wrote:
> >On Fri, Mar 24, 2023 at 1:28 PM <rsbecker@nexbridge.com> wrote:
> >>
> >> On Thursday, March 23, 2023 7:55 PM, Felipe Contreras wrote:
> >> >On Thu, Mar 23, 2023 at 5:43 PM <rsbecker@nexbridge.com> wrote:
> >> >>
> >> >> On Thursday, March 23, 2023 7:35 PM, Felipe Contreras wrote:
> >> >> >On Thu, Mar 23, 2023 at 5:30 PM <rsbecker@nexbridge.com> wrote:
> >> >> >>
> >> >> >> On Thursday, March 23, 2023 7:22 PM, Felipe Contreras wrote:
> >> >> >> >On Sat, Feb 18, 2023 at 5:12 AM Phillip Wood
> >> >> >> ><phillip.wood123@gmail.com>
> >> >> >wrote:
> >> >> >> >>
> >> >> >> >> On 18/02/2023 01:59, demerphq wrote:
> >> >> >> >> > On Sat, 18 Feb 2023 at 00:24, Junio C Hamano
> >> >> >> >> > <gitster@pobox.com>
> >> >wrote:
> >> >> >> >> >>
> >> >> >> >> >> Emily Shaffer <nasamuffin@google.com> writes:
> >> >> >> >> >>
> >> >> >> >> >>> Basically, if this effort turns out not to be fruitful as
> >> >> >> >> >>> a whole, I'd like for us to still have left a positive impact on the
> >codebase.
> >> >> >> >> >>> ...
> >> >> >> >> >>> So what's next? Naturally, I'm looking forward to a
> >> >> >> >> >>> spirited discussion about this topic - I'd like to know
> >> >> >> >> >>> which concerns haven't been addressed and figure out
> >> >> >> >> >>> whether we can find a way around them, and generally
> >> >> >> >> >>> build awareness of this effort with the
> >> >> >community.
> >> >> >> >> >>
> >> >> >> >> >> On of the gravest concerns is that the devil is in the details.
> >> >> >> >> >>
> >> >> >> >> >> For example, "die() is inconvenient to callers, let's
> >> >> >> >> >> propagate errors up the callchain" is an easy thing to
> >> >> >> >> >> say, but it would take much more than "let's propagate errors up"
> >> >> >> >> >> to libify something like
> >> >> >> >> >> check_connected() to do the same thing without spawning a
> >> >> >> >> >> separate process that is expected to exit with failure.
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > What does "propagate errors up the callchain" mean?  One
> >> >> >> >> > interpretation I can think of seems quite horrible, but
> >> >> >> >> > another seems quite doable and reasonable and likely not
> >> >> >> >> > even very invasive of the existing code:
> >> >> >> >> >
> >> >> >> >> > You can use setjmp/longjmp to implement a form of "try", so
> >> >> >> >> > that errors dont have to be *explicitly* returned *in* the call chain.
> >> >> >> >> > And you could probably do so without changing very much of
> >> >> >> >> > the existing code at all, and maintain a high level of
> >> >> >> >> > conceptual alignment with the current code strategy.
> >> >> >> >>
> >> >> >> >> Using setjmp/longjmp is an interesting suggestion, I think
> >> >> >> >> lua does something similar to what you describe for perl.
> >> >> >> >> However I think both of those use a allocator with garbage
> >> >> >> >> collection. I worry that using longjmp in git would be more
> >> >> >> >> invasive (or result in more memory leaks) as we'd need to to
> >> >> >> >> guard each allocation with some code to clean it up and then
> >> >> >> >> propagate the error. That means we're back to manually
> >> >> >> >> propagating errors up the call
> >> >chain in many cases.
> >> >> >> >
> >> >> >> >We could just use talloc [1].
> >> >> >>
> >> >> >> talloc is not portable.
> >> >> >
> >> >> >What makes you say that?
> >> >>
> >> >> talloc is not part of a POSIX standard I could find.
> >> >
> >> >It's a library, like: z, ssl, curl, pcre2-8, etc. Libraries can be
> >> >compiled on different platforms.
> >>
> >> talloc adds additional *required* dependencies to git, including python3 - required
> >to configure and build talloc - which is not available on the NonStop ia64 platform
> >(required support through end of 2025). I must express my resistance to what would
> >amount to losing support for git on this NonStop platform.
> >
> >That is not true. You don't need python3 for talloc, not even to build it, it's just a
> >single simple c file, it's easy to compile.
> >
> >The only reason python is used is to run waf, which is used to build Samba, which is
> >much more complex, but you don't need to run it, especially if you know the
> >characteristics of your system.
> >
> >This simple Makefile builds libtalloc.so just fine:
> >
> >  CC := gcc
> >  CFLAGS := -fPIC -I./lib/replace
> >  LDFLAGS := -Wl,--no-undefined
> >
> >  # For talloc.c
> >  CFLAGS += -DTALLOC_BUILD_VERSION_MAJOR=2
> >-DTALLOC_BUILD_VERSION_MINOR=4 -DTALLOC_BUILD_VERSION_RELEASE=0
> >  CFLAGS += -DHAVE_CONSTRUCTOR_ATTRIBUTE -DHAVE_VA_COPY -
> >DHAVE_VALGRIND_MEMCHECK_H -DHAVE_INTPTR_T
> >
> >  # For replace.h
> >  CFLAGS += -DNO_CONFIG_H -D__STDC_WANT_LIB_EXT1__=1
> >  CFLAGS += -DHAVE_STDBOOL_H -DHAVE_BOOL -DHAVE_STRING_H -
> >DHAVE_LIMITS_H -DHAVE_STDINT_H
> >  CFLAGS += -DHAVE_DLFCN_H -DHAVE_UINTPTR_T -DHAVE_C99_VSNPRINTF -
> >DHAVE_MEMMOVE -DHAVE_STRNLEN -DHAVE_VSNPRINTF
> >
> >  libtalloc.so: talloc.o
> >    $(CC) $(LDFLAGS) -shared -o $@ $^
> >
> >But of course, most of those defines are not even needed with a simple "replace.h"
> >that is less than 10 lines of code.
>
> The 2.4.0 version is substantially more complex than one .c file. The version I just unpacked has 61 C and H files, and requires more than a trivial makefile.

>
> Emily Shaffer <nasamuffin@google.com> writes:
>
> > Basically, if this effort turns out not to be fruitful as a whole, I'd
> > like for us to still have left a positive impact on the codebase.
> > ...
> > So what's next? Naturally, I'm looking forward to a spirited
> > discussion about this topic - I'd like to know which concerns haven't
> > been addressed and figure out whether we can find a way around them,
> > and generally build awareness of this effort with the community.
>
> On of the gravest concerns is that the devil is in the details.
>
> For example, "die() is inconvenient to callers, let's propagate
> errors up the callchain" is an easy thing to say, but it would take
> much more than "let's propagate errors up" to libify something like
> check_connected() to do the same thing without spawning a separate
> process that is expected to exit with failure.

>
> It is not clear if we can start small, work on a subset of the
> things and still reap the benefit of libification.  Is there an
> existing example that we have successfully modularlized the API into
> one subsystem?  Offhand, I suspect that the refs API with its two
> implementations may be reasonably close, but is the inteface into
> that subsystem the granularity of the library interface you guys
> have in mind?

>> For example, "die() is inconvenient to callers, let's propagate
>> errors up the callchain" is an easy thing to say, but it would take
>> much more than "let's propagate errors up" to libify something like
>> check_connected() to do the same thing without spawning a separate
>> process that is expected to exit with failure.
>
> Because the error propagation path is complicated, you mean? Or
> because the cleanup is painful?

> This turned out pretty long-winded, so a quick summary before I dive in:
>
> - We want to compile parts of Git as independent libraries
> - We want to do it by making incremental code quality improvements to Git
> - Let's avoid promising stability of the interfaces of those libraries
> - We think it'll let Git do cool stuff like unit tests and allowing
> purpose-built plugins
> - Hopefully by example we can convince the rest of the project to join
> in the effort

> The good news is that for practical near-term purposes, "libification"
> mostly means cleanups to the Git codebase, and continuing code health
> work that the project has already cared about doing:
>
> - Removing references to global variables and instead piping them
> through arguments
> - Finding and fixing memory leaks, especially in widely-used low-level code

> - Clarifying the scope and layering of existing modules, for example
> by moving single-use helpers from the shared module's scope into the
> single user's scope
> - Making module interfaces more consistent and easier to understand,
> including moving "private" functions out of headers and into source
> files and improving in-header documentation

> I'm also planning to send a proposal for a document full of "best
> practices" for turning Git code into libraries (and have quite a lot
> of discussion around that document, too). My hope is that we can use
> that document to help us during implementation as well as during
> review, and refine it over time as we learn more about what works and
> what doesn't. Having this kind of documentation will make it easy for
> others to join us in moving Git's codebase towards a clean set of
> libraries. I hope that, as a project, we can settle on some tenets
> that we all agree would make Git nicer.

> After that, we're still hoping to target low-level libraries first - I
> certainly don't think it will make sense to ship a high-level `git
> commit` library in the near future, if ever - in the order that
> they're required from the VFS project we're working closely with. As
> far as I can tell right now, that's likely to cover object store and
> worktree access, as well as commit creation and pushing, but we'll see
> how planning shakes out over the next month or so. But Google's
> schedule should have no bearing on what others in the Git project feel
> is important to clean up and libify, and if there is interest in the
> rest of the project in converting other existing modules into
> libraries, my team and I are excited to participate in the review.

> Much, much later on, I'm expecting us to form a plan around allowing
> "plugins" - that is, replacing library functionality we use today with
> an alternative library, such as an object store relying on a
> distributed file store like S3. Making that work well will also likely
> involve us coming up with a solution for dependency injection, and to
> begin using vtables for some libraries. I'm hoping that we can figure
> out a way to do that that won't make the Git source ugly. Around this
> time, I think it will make sense to buy into unit tests even more and
> start using an approach like mocking to test various edge cases. And
> at some point, it's likely that we'll want to make the interfaces to
> various Git libraries consistent with each other, which would involve
> some large-scale but hopefully-mechanical refactors.

>
> On Fri, Feb 17, 2023 at 1:45 PM Emily Shaffer <nasamuffin@google.com> wrote:
> [...]
> > The good news is that for practical near-term purposes, "libification"
> > mostly means cleanups to the Git codebase, and continuing code health
> > work that the project has already cared about doing:
> >
> > - Removing references to global variables and instead piping them
> > through arguments
> > - Finding and fixing memory leaks, especially in widely-used low-level code
>
> Does removing memory leaks also mean converting UNLEAK to free()?

>
> > - Reducing or removing `die()` invocations in low-level code, and
> > instead reporting errors back to callers in a consistent way
>
> What delinates "low-level" code?  (A "we don't know yet but we'll
> start with obvious places and plan to have good discussions on the
> appropriate boundary in the future as we submit patches" is a fine
> answer, I'm just curious if you already have a rough idea of where you
> intend that boundary to lie.)

> > - Clarifying the scope and layering of existing modules, for example
> > by moving single-use helpers from the shared module's scope into the
> > single user's scope
> > - Making module interfaces more consistent and easier to understand,
> > including moving "private" functions out of headers and into source
> > files and improving in-header documentation
>
> I think these are very positive directions.  I like the fact that your
> initial plan benefits all of us, whether or not libification is
> ultimately achieved.
>
> [...]
> > So what's next? Naturally, I'm looking forward to a spirited
> > discussion about this topic - I'd like to know which concerns haven't
> > been addressed and figure out whether we can find a way around them,
> > and generally build awareness of this effort with the community.
>
> I'm curious whether clarifying scope/layering and cleaning up
> interfaces might mean you'd be interested in things like:
>   * https://github.com/newren/git/commits/header-cleanups (which was
> still WIP; I paused working on it because I figured people would see
> it as big "cleanup" patches with no practical benefit)
>   * https://github.com/gitgitgadget/git/pull/1149 (which has been
> ready to submit for a _long_ time, but I just haven't yet)
> or if these two things are orthogonal to what you have in mind.

>
> > I'm also planning to send a proposal for a document full of "best
> > practices" for turning Git code into libraries (and have quite a lot
> > of discussion around that document, too). My hope is that we can use
> > that document to help us during implementation as well as during
> > review, and refine it over time as we learn more about what works and
> > what doesn't. Having this kind of documentation will make it easy for
> > others to join us in moving Git's codebase towards a clean set of
> > libraries. I hope that, as a project, we can settle on some tenets
> > that we all agree would make Git nicer.
>
> I like the sound of this.
>
> > After that, we're still hoping to target low-level libraries first - I
> > certainly don't think it will make sense to ship a high-level `git
> > commit` library in the near future, if ever - in the order that
> > they're required from the VFS project we're working closely with. As
> > far as I can tell right now, that's likely to cover object store and
> > worktree access, as well as commit creation and pushing, but we'll see
> > how planning shakes out over the next month or so. But Google's
> > schedule should have no bearing on what others in the Git project feel
> > is important to clean up and libify, and if there is interest in the
> > rest of the project in converting other existing modules into
> > libraries, my team and I are excited to participate in the review.
>
> If we can't libify something like commit, does that prevent libifying
> higher level things like merge?
>
> I spent some time thinking about this a while back.  I tried to
> carefully design merge-ort to improve the odds it could be used
> elsewhere, maybe even libgit2.  (I hope it shows in the many comments
> in merge-ort.h, and I think the "priv" field in particular allowing me
> to hide the first ~300 lines of merge-ort.c declaring data structures
> from users was really nice.)  However, I still had to accept data in
> some known format.  So input parameters are things like trees and
> commits.  But tree.h and commit.h both include object.h first, which
> includes cache.h, which is basically all of Git.  And the functions I
> call to interoperate with the system are similarly entangled.  So, the
> odds of merge-ort being reused by libgit2 or otherwise used in a
> library seems essentially nil, at least without some broader
> libification effort.

>
> I'd like to make that story better, time permitting (which is much
> more of a challenge these days than it was a couple years ago), but
> I'm curious if you or others have thoughts on something like that.
>
> > Much, much later on, I'm expecting us to form a plan around allowing
> > "plugins" - that is, replacing library functionality we use today with
> > an alternative library, such as an object store relying on a
> > distributed file store like S3. Making that work well will also likely
> > involve us coming up with a solution for dependency injection, and to
> > begin using vtables for some libraries. I'm hoping that we can figure
> > out a way to do that that won't make the Git source ugly. Around this
> > time, I think it will make sense to buy into unit tests even more and
> > start using an approach like mocking to test various edge cases. And
> > at some point, it's likely that we'll want to make the interfaces to
> > various Git libraries consistent with each other, which would involve
> > some large-scale but hopefully-mechanical refactors.
>
> Would these plugins resemble the pluggable merge backends that was
> added to builtin/merge.c?  Would it replace that mechanism with a
> different one?  Would it be more like the refs backends?

>
> Would this plugin scheme allow us to, for example, use gitoxide[1] as
> a clone replacement to make clones 2x as fast (and with half the
> memory -- although I suspect they cheated and used sha1 instead of
> sha1dc, so maybe it wouldn't really be 2x)?

>
> Oh, and it's totally okay if you don't know the answers to any or all
> of my questions right now.  I'm just curious, because I've long
> thought these kinds of directions would be good.  Since I've spent
> time thinking about it, I have questions that I don't know the answers
> to, but I figured it couldn't hurt to bounce them off others who are
> thinking about this area.
>
> Anyway, it's a large pile of work that you're undertaking, and as
> Junio comments elsewhere in this thread it's unclear if libification
> can be achieved for a big enough component (and you seem to admit as
> much in your email as well), but I applaud the general direction and
> your initial plans.

> > > So what's next? Naturally, I'm looking forward to a spirited
> > > discussion about this topic - I'd like to know which concerns haven't
> > > been addressed and figure out whether we can find a way around them,
> > > and generally build awareness of this effort with the community.
> >
> > I'm curious whether clarifying scope/layering and cleaning up
> > interfaces might mean you'd be interested in things like:
> >   * https://github.com/newren/git/commits/header-cleanups (which was
> > still WIP; I paused working on it because I figured people would see
> > it as big "cleanup" patches with no practical benefit)
> >   * https://github.com/gitgitgadget/git/pull/1149 (which has been
> > ready to submit for a _long_ time, but I just haven't yet)
> > or if these two things are orthogonal to what you have in mind.
>
> Extremely yes. :) Even "small" stuff like the need for header cleanups
> have already come up for Glen and Calvin working on config and strbuf.

> > > Much, much later on, I'm expecting us to form a plan around allowing
> > > "plugins" - that is, replacing library functionality we use today with
> > > an alternative library, such as an object store relying on a
> > > distributed file store like S3. Making that work well will also likely
> > > involve us coming up with a solution for dependency injection, and to
> > > begin using vtables for some libraries. I'm hoping that we can figure
> > > out a way to do that that won't make the Git source ugly. Around this
> > > time, I think it will make sense to buy into unit tests even more and
> > > start using an approach like mocking to test various edge cases. And
> > > at some point, it's likely that we'll want to make the interfaces to
> > > various Git libraries consistent with each other, which would involve
> > > some large-scale but hopefully-mechanical refactors.
> >
> > Would these plugins resemble the pluggable merge backends that was
> > added to builtin/merge.c?  Would it replace that mechanism with a
> > different one?  Would it be more like the refs backends?
>
> I suspect that it's likely to be most similar to the refs backend
> replacement, although I investigated it only a little bit just now.
>
> The pluggable merge backends are an interesting thought - right now
> all those alternatives are built in and we decide based on config,
> right?

> > Thinking of things in a library context probably pushes us in that
> > direction (though, alternatively, it might just highlight the question
> > of what is considered "low-level" instead).
> 
> I'm not sure whether use of UNLEAK has so much to do with "low-level"
> or not. In cases when Git is being called as an ephemeral single-run
> process, UNLEAK makes a lot of sense. In cases when Git is being
> called in a long-lived process, UNLEAK is just a sign that says
> "there's a leak here".  So I think the distinction is not low-level or
> high-level, but more simply, within a library or not.

> This turned out pretty long-winded, so a quick summary before I dive in:
>
> - We want to compile parts of Git as independent libraries
> - We want to do it by making incremental code quality improvements to Git
> - Let's avoid promising stability of the interfaces of those libraries
> - We think it'll let Git do cool stuff like unit tests and allowing
>   purpose-built plugins
> - Hopefully by example we can convince the rest of the project to join
>   in the effort

>
> On Fri, Feb 17, 2023 at 01:12:23PM -0800, Emily Shaffer wrote:
> > This turned out pretty long-winded, so a quick summary before I dive in:
> >
> > - We want to compile parts of Git as independent libraries
> > - We want to do it by making incremental code quality improvements to Git
> > - Let's avoid promising stability of the interfaces of those libraries
> > - We think it'll let Git do cool stuff like unit tests and allowing
> >   purpose-built plugins
> > - Hopefully by example we can convince the rest of the project to join
> >   in the effort
>
> Like others, I am less interested in the VFS-specific components you
> mention here, but I suspect that is just one particular instance of
> something that would be benefited by making git internals exposed via a
> linkable library.
>
> I don't have objections to things like reducing our usage of globals,
> making fewer internal functions die() when they encounter an error, and
> so on. But like Junio, I suspect that this is definitely an instance of
> a "devil's in the details" kind of problem.
>
> That's definitely my main concern: that this turns out to be much more
> complicated than imagined and that we leave the codebase in a worse
> state without much to show.

>
> So I think we'd want to see some more concrete examples with clear
> benefits to gauge whether this is a worthwhile direction. I think that
> strbuf.h is too trivial an example to demonstrate anything useful. Being
> able to extract config.h into its own library so that another non-Git
> program could link against it and implement 'git config'-like
> functionality would be much more interesting.

> Hi folks,
> 
> As I mentioned in standup this week[1], my colleagues and I at Google
> have become very interested in converting parts of Git into libraries
> usable by external programs. In other words, for some modules which
> already have clear boundaries inside of Git - like config.[ch],
> strbuf.[ch], etc. - we want to remove some implicit dependencies, like
> references to globals, and make explicit other dependencies, like
> references to other modules within Git. Eventually, we'd like both for
> an external program to use Git libraries within its own process, and
> for Git to be given an alternative implementation of a library it uses
> internally (like a plugin at runtime).

> 
> This turned out pretty long-winded, so a quick summary before I dive in:
> 
> - We want to compile parts of Git as independent libraries
> - We want to do it by making incremental code quality improvements to Git
> - Let's avoid promising stability of the interfaces of those libraries
> - We think it'll let Git do cool stuff like unit tests and allowing
> purpose-built plugins
> - Hopefully by example we can convince the rest of the project to join
> in the effort
> 
> My team has spent the past year or so trying to make improvements to
> Git's behavior with submodules, and we found that the current
> structure of Git is quite tricky to work with: because Git doesn't
> execute on second repositories in the same process well, recursing
> into submodules typically involves spawning child processes, and
> piping new arguments through the helpers around those child processes,
> and then through Git's typical codepaths, is very tricky. After
> spending more than a year trying to make improvements, we have very
> little to show for it, largely as a result of the difficulty of
> passing information between superprojects and submodules.
> 
> It seems like being able to invoke parts of Git as a library, or Git
> being able to invoke custom libraries, does a lot of good for the Git
> project:
> 
> - Having clear, modular libraries makes it easy to find the code
> responsible for some behavior, or determine where to add something
> new.

> - Operations recursing into submodules can be run in-process, and the
> callsite makes it very clear which repository context is being used;
> if we're not subprocessing to recurse, then we can lose the awkward
> git-submodule.sh -> builtin/submodule__helper.c -> top-level Git
> process codepath.

> 
> The ability to use Git as a library also makes it easier for other
> tooling to interact with a Git repository *correctly*. As an example,
> `repo` has a long history of abusing Git by directly manipulating the
> gitdir[2], but if it were written in a world where Git exists as
> easy-to-use libraries, it probably wouldn't have needed to, as it
> could have invoked Git directly or replaced the relevant modules with
> its own implementation. Both `repo`[3] and `git-gui[4]` have
> reimplemented logic from git.git. Other interfaces that cooperate with
> Git's filesystem storage layer, like `scalar` or `jj`[5], would be
> able to interop with a Git repository without having to reimplement
> custom logic or keep up with new Git changes.

> 
> Of course, there's a reason Google wants it, too. We've talked
> previously about wanting better integration between Git and something
> like a VFS; as we've experimented with it internally, we've found a
> couple of tricky areas:
> 
> - The VFS relies on running Git commands to determine the state of the
> repository. However, these Git commands interact with the gitdir or
> worktree, which is populated by the VFS. For example, parsing a
> .gitattributes or .gitmodules which is already stored in the VFS
> requires the VFS to provide a POSIX file handle, spawn a Git
> subprocess, populate other files needed by that subprocess (like
> .git/config), and finally collect the output stream of the subprocess.
> As you can imagine, this interaction of VFS -> Git -> VFS [-> Git]
> creates all sort of complications. The alternative is for the VFS to
> write its own parser (or use a library like libgit2; more on that
> later). But having a Git library means that a subset of Git
> functionality can happen in-process, and that filesystem access could
> be replaced by the VFS directly providing high-level objects or plain
> bytestreams.
> 
> - A user running `git status` in a directory controlled by the VFS
> will require the VFS to populate the entire (large) worktree - even
> though the VFS is sure that only one file has been modified. The
> closest we've come with an alternative is an orchestrated use of
> sparse-checkout - but modifying the sparse-checkout configs
> automatically in response to the user's filesystem operations takes us
> right back to the previous point. If Git could take a plugin and
> replacement for the object store that directly interacts with the VFS
> daemon, a layer of complexity would disappear and performance would
> improve.

> The good news is that for practical near-term purposes, "libification"
> mostly means cleanups to the Git codebase, and continuing code health
> work that the project has already cared about doing:
> 
> - Removing references to global variables and instead piping them
> through arguments
> - Finding and fixing memory leaks, especially in widely-used low-level code
> - Reducing or removing `die()` invocations in low-level code, and
> instead reporting errors back to callers in a consistent way
> - Clarifying the scope and layering of existing modules, for example
> by moving single-use helpers from the shared module's scope into the
> single user's scope
> - Making module interfaces more consistent and easier to understand,
> including moving "private" functions out of headers and into source
> files and improving in-header documentation
> 
> Basically, if this effort turns out not to be fruitful as a whole, I'd
> like for us to still have left a positive impact on the codebase.

> 
> In the longer term, if Git has libraries with easily-replaced
> dependencies, we get a few more benefits:
> 
> - Unit tests. We already have some in t/helper/, but if we can replace
> all the dependencies of a certain library with simple stubs, it's
> easier for us to write comprehensive unit tests, in addition to the
> work we already do introducing edge cases in bash integration tests.

> 
> Of course, we haven't maintained any guarantee about the consistency
> of our implementation between releases. I don't anticipate that we'll
> write the perfect library interface on our first try. So I hope that
> we can be very explicit about refusing to provide any compatibility
> guarantee whatsoever between versions for quite a long time. On
> Google's end, that's well-understood and accepted. As I understand,
> some other projects already use Git's codebase as a "library" by
> including it as a submodule and using the code directly[6]; even a
> breakable API seems like an improvement over that, too.

> 
> So what's next? Naturally, I'm looking forward to a spirited
> discussion about this topic - I'd like to know which concerns haven't
> been addressed and figure out whether we can find a way around them,
> and generally build awareness of this effort with the community.
> 
> I'm also planning to send a proposal for a document full of "best
> practices" for turning Git code into libraries (and have quite a lot
> of discussion around that document, too). My hope is that we can use
> that document to help us during implementation as well as during
> review, and refine it over time as we learn more about what works and
> what doesn't. Having this kind of documentation will make it easy for
> others to join us in moving Git's codebase towards a clean set of
> libraries. I hope that, as a project, we can settle on some tenets
> that we all agree would make Git nicer.

> 
> From the rest of my own team, we're planning on working first on some
> limited scope, low-level libraries so that we can all see how the
> process works. We're starting with strbuf.[ch] (as it's used
> everywhere with few or no dependencies and helps us guarantee string
> safety at API boundaries), config.[ch] (as many external tools are
> probably interested in parsing Git config formatted files directly),
> and a subset of operations related to the object store. These starting
> points are intended to have a small impact on the codebase and teach
> us a lot about logistics and best practices while doing these kinds of
> conversions.
> 
> After that, we're still hoping to target low-level libraries first - I
> certainly don't think it will make sense to ship a high-level `git
> commit` library in the near future, if ever - in the order that
> they're required from the VFS project we're working closely with. As
> far as I can tell right now, that's likely to cover object store and
> worktree access, as well as commit creation and pushing, but we'll see
> how planning shakes out over the next month or so. But Google's
> schedule should have no bearing on what others in the Git project feel
> is important to clean up and libify, and if there is interest in the
> rest of the project in converting other existing modules into
> libraries, my team and I are excited to participate in the review.

> 
> Much, much later on, I'm expecting us to form a plan around allowing
> "plugins" - that is, replacing library functionality we use today with
> an alternative library, such as an object store relying on a
> distributed file store like S3. Making that work well will also likely
> involve us coming up with a solution for dependency injection, and to
> begin using vtables for some libraries. I'm hoping that we can figure
> out a way to do that that won't make the Git source ugly. Around this
> time, I think it will make sense to buy into unit tests even more and
> start using an approach like mocking to test various edge cases. And
> at some point, it's likely that we'll want to make the interfaces to
> various Git libraries consistent with each other, which would involve
> some large-scale but hopefully-mechanical refactors.

>  - Emily
> 
> 1: https://colabti.org/irclogger/irclogger_log/git-devel?date=2023-02-13#l29
> 2: https://gerrit.googlesource.com/git-repo/+/refs/heads/main/docs/internal-fs-layout.md
> 3: https://gerrit.googlesource.com/git-repo/+/refs/heads/main/git_config.py
> 4: https://github.com/git/git/blob/master/git-gui/git-gui.sh#L305
> 5: https://github.com/martinvonz/jj
> 6: https://github.com/glandium/git-cinnabar

> Emily Shaffer wrote:
> > Hi folks,
> > 
> > As I mentioned in standup this week[1], my colleagues and I at Google
> > have become very interested in converting parts of Git into libraries
> > usable by external programs. In other words, for some modules which
> > already have clear boundaries inside of Git - like config.[ch],
> > strbuf.[ch], etc. - we want to remove some implicit dependencies, like
> > references to globals, and make explicit other dependencies, like
> > references to other modules within Git. Eventually, we'd like both for
> > an external program to use Git libraries within its own process, and
> > for Git to be given an alternative implementation of a library it uses
> > internally (like a plugin at runtime).
> 
> I see why you've linked these two objectives (there could be some shared
> infrastructure between them), but they're ultimately separate concerns, with
> separate needs & tradeoffs. 

> By trying to design with a unified solution for
> both, though, this proposal's solution may be broader than it needs to be
> and comes with some serious developer experience ramifications.
> 
> For example, looking at the  "call libgit.so from other applications"
> objective: one way I could imagine implementing this is by creating a stable
> API wrapper around various hand-picked internal functions. If those internal
> functions change (e.g., adding a function argument to handle a new flag in a
> builtin), the API can remain stable (by setting a sane default) without
> really limiting the contributor adding a new feature. 
> 
> However, my reading of this proposal (although correct me if I'm wrong) is
> that you want to use that same external-facing API as a plugin interface for
> Git. In that case, the contributor can't just add a new argument and
> trivially modify the wrapper; they either break the API contract, or create
> a new version of the API and continue to support the old one. While that's
> technically doable, it's a pretty sizeable increase to the amount of "stuff"
> contributors need to keep track of and limits the project's flexibility. 

> > - Having clear, modular libraries makes it easy to find the code
> > responsible for some behavior, or determine where to add something
> > new.
> 
> This is more an issue of code & repository organization, and isn't really an
> inherent outcome of lib-ifying Git. That said, like the cleanup you mention
> later, I would not be opposed to a dedicated code reorganization proposal.

> > - Operations recursing into submodules can be run in-process, and the
> > callsite makes it very clear which repository context is being used;
> > if we're not subprocessing to recurse, then we can lose the awkward
> > git-submodule.sh -> builtin/submodule__helper.c -> top-level Git
> > process codepath.
> 
> Interesting - I wasn't aware this was how submodules worked internally. Is
> there a specific reason this can't be refactored to perform the recursion
> in-process?

> > - Being able to test libraries in isolation via unit tests or mocks
> > speeds up determining the root cause of bugs.
> 
> The addition of unit tests is a project of its own, and one I don't believe
> is intrinsically tied to this one. More on this later.

> > - If we can swap out an entire library, or just a single function of
> > one, it's easy to experiment with the entire codebase without sweeping
> > changes.
> 
> What sort of "experiment" are you thinking of here? Anyone can make internal
> changes to Git in their own clone of the repository, then build and test it.
> Shipping a custom fork of Git doesn't seem any less complex than building a
> plugin library, shipping that, and injecting it into an existing Git
> install.

> > The ability to use Git as a library also makes it easier for other
> > tooling to interact with a Git repository *correctly*. As an example,
> > `repo` has a long history of abusing Git by directly manipulating the
> > gitdir[2], but if it were written in a world where Git exists as
> > easy-to-use libraries, it probably wouldn't have needed to, as it
> > could have invoked Git directly or replaced the relevant modules with
> > its own implementation. Both `repo`[3] and `git-gui[4]` have
> > reimplemented logic from git.git. Other interfaces that cooperate with
> > Git's filesystem storage layer, like `scalar` or `jj`[5], would be
> > able to interop with a Git repository without having to reimplement
> > custom logic or keep up with new Git changes.
> 
> I'd alternatively suggest that these use cases could be addressed with
> better plumbing command support and better (public) documentation of our
> on-disk data structures. In many cases, we're treating on-disk data
> structures as external-facing APIs anyway - the index [1], packfiles [2],
> etc. are versioned. We could definitely add documentation that's more 
> directly targeted at external integrators.
> 
> [1] https://git-scm.com/docs/index-format
> [2] https://git-scm.com/docs/gitformat-pack

> Both of these points sound more like implementation details/quirks of the
> VFS prototype you're building than hard blockers coming from Git. For
> example, you mention 'git status' needing to populate a full worktree -
> could it instead use a mechanism similar to FSMonitor to avoid scanning for
> files you know are virtualized? While there'd likely be some Git changes
> involved (some of which might be a bit plugin-y), they'd be much more
> tightly scoped than full libification.

> > In the longer term, if Git has libraries with easily-replaced
> > dependencies, we get a few more benefits:
> > 
> > - Unit tests. We already have some in t/helper/, but if we can replace
> > all the dependencies of a certain library with simple stubs, it's
> > easier for us to write comprehensive unit tests, in addition to the
> > work we already do introducing edge cases in bash integration tests.
> 
> This doesn't really follow as a direct consequence of making libgit
> externally facing. AFAIK, there's nothing explicitly stopping us from
> writing or integrating a unit test framework now.
> 
> Like the "make libgit public" vs. "allow for custom backends in Git", I
> think this is a separate project with its own tradeoffs to evaluate.

> > - If our users can use plugins to improve performance in specific
> > scenarios (like a VFS-aware object store in the VFS case I cited
> > above), then Git works better for them without having to adopt a
> > different workflow, such as using an alternative tool or wrapper.
> 
> I'm curious as to what you want the user experience for this to look like.
> How would Git know to dynamically load a plugin? How does a user configure
> (or disable) plugins? Will a plugin need to replace all of the functions in
> a given library, or would Git be able to "fall back" on its own internal
> implementation?

> > - An easy-to-understand modular codebase makes it easy for new
> > contributors to start hacking and understand the consequences of their
> > patch.
> 
> As noted earlier, I think improving the developer experience in this way is
> independent of the development of external APIs.

> > Of course, we haven't maintained any guarantee about the consistency
> > of our implementation between releases. I don't anticipate that we'll
> > write the perfect library interface on our first try. So I hope that
> > we can be very explicit about refusing to provide any compatibility
> > guarantee whatsoever between versions for quite a long time. On
> > Google's end, that's well-understood and accepted. As I understand,
> > some other projects already use Git's codebase as a "library" by
> > including it as a submodule and using the code directly[6]; even a
> > breakable API seems like an improvement over that, too.
> 
> This hints at, but sidesteps, a really important aspect of the long-term
> goals of this project - are you planning on having us start guaranteeing
> consistency once there's an external-facing API available? 
> 
> If not, that sounds like an unpleasant user experience (and one prone to
> Hyrum's Law [3] at that). 
> 
> If so, Git contributors will either be much more constrained in the
> introduction of new features, or we'll end up with a mess of
> backward-compatibility APIs.
> 
> [3] https://www.hyrumslaw.com/

> > So what's next? Naturally, I'm looking forward to a spirited
> > discussion about this topic - I'd like to know which concerns haven't
> > been addressed and figure out whether we can find a way around them,
> > and generally build awareness of this effort with the community.
> > 
> > I'm also planning to send a proposal for a document full of "best
> > practices" for turning Git code into libraries (and have quite a lot
> > of discussion around that document, too). My hope is that we can use
> > that document to help us during implementation as well as during
> > review, and refine it over time as we learn more about what works and
> > what doesn't. Having this kind of documentation will make it easy for
> > others to join us in moving Git's codebase towards a clean set of
> > libraries. I hope that, as a project, we can settle on some tenets
> > that we all agree would make Git nicer.
> 
> I think the use of multiple libraries is part of the potentially suboptimal
> balance between the "load Git's internals as a library" and "let Git use
> plugins in place of its own implementations" goals. The former could leave a
> user in dependency hell if there are lots of small libraries to load, while
> the latter may work better with smaller-scoped libraries (to avoid needing
> to implement more than is useful). And, the functionality that's useful to a
> program invoking Git via library may not be the same as what someone would
> want to replace via plugin. 

> As a general note, this libification project - its justification,
> milestones, organization, etc. - seems to be primarily driven by the needs
> of a VFS that is entirely opaque to the Git community you're proposing to.
> As it stands, reviewers are put in a position of needing to accept, without
> much evidence, that this is the best (or only) possible approach for meeting
> those needs. 
> 
> While I'm normally content with "scratch your own itch"-type changes, what
> you're proposing is a major paradigm shift in how Git is written,
> maintained, and used. I'm not comfortable accepting that level of impact
> without at least being able to evaluate whether the problem can be solved
> some other way.

> Implementing dependency injection (via vtable or otherwise) and unit test
> mocking is a massive undertaking on its own, let alone everything else
> described so far. You've outlined some clear, fairly unobtrusive first steps
> to the overarching proposal, but there's a lot of detail missing from the
> plans for later steps. While there's always risk of a project getting
> derailed or blocked later on due to some unforeseen issue, that risk seems
> particularly high here given the size & scope of all of these components. 

> So, to summarize my thoughts into some (hopefully) actionable feedback:
> 
> - It's really important that the proposal presents a clear, concrete
>   long-term vision for this project. 
>   - What is the desired end state, in terms of what is built and installed
>     with Git?

> This turned out pretty long-winded, so a quick summary before I dive in:
>
> - We want to compile parts of Git as independent libraries
> - We want to do it by making incremental code quality improvements to Git
> - Let's avoid promising stability of the interfaces of those libraries
> - We think it'll let Git do cool stuff like unit tests and allowing
> purpose-built plugins
> - Hopefully by example we can convince the rest of the project to join
> in the effort

> - Removing references to global variables and instead piping them
> through arguments
> - Finding and fixing memory leaks, especially in widely-used low-level code
> - Reducing or removing `die()` invocations in low-level code, and
> instead reporting errors back to callers in a consistent way

> - Operations recursing into submodules can be run in-process, and the
> callsite makes it very clear which repository context is being used;
> if we're not subprocessing to recurse, then we can lose the awkward
> git-submodule.sh -> builtin/submodule__helper.c -> top-level Git
> process codepath.

> Of course, we haven't maintained any guarantee about the consistency
> of our implementation between releases. I don't anticipate that we'll
> write the perfect library interface on our first try. So I hope that
> we can be very explicit about refusing to provide any compatibility
> guarantee whatsoever between versions for quite a long time. On
> Google's end, that's well-understood and accepted. As I understand,
> some other projects already use Git's codebase as a "library" by
> including it as a submodule and using the code directly[6]; even a
> breakable API seems like an improvement over that, too.

> Of course, there's a reason Google wants it, too. We've talked
> previously about wanting better integration between Git and something
> like a VFS; as we've experimented with it internally, we've found a
> couple of tricky areas:
>
> - The VFS relies on running Git commands to determine the state of the
> repository. However, these Git commands interact with the gitdir or
> worktree, which is populated by the VFS.

> - A user running `git status` in a directory controlled by the VFS
> will require the VFS to populate the entire (large) worktree - even
> though the VFS is sure that only one file has been modified. The
> closest we've come with an alternative is an orchestrated use of
> sparse-checkout - but modifying the sparse-checkout configs
> automatically in response to the user's filesystem operations takes us
> right back to the previous point. If Git could take a plugin and
> replacement for the object store that directly interacts with the VFS
> daemon, a layer of complexity would disappear and performance would
> improve.

> I'm also planning to send a proposal for a document full of "best
> practices" for turning Git code into libraries (and have quite a lot
> of discussion around that document, too). My hope is that we can use
> that document to help us during implementation as well as during
> review, and refine it over time as we learn more about what works and
> what doesn't. Having this kind of documentation will make it easy for
> others to join us in moving Git's codebase towards a clean set of
> libraries. I hope that, as a project, we can settle on some tenets
> that we all agree would make Git nicer.

> From the rest of my own team, we're planning on working first on some
> limited scope, low-level libraries so that we can all see how the
> process works. We're starting with strbuf.[ch] (as it's used
> everywhere with few or no dependencies and helps us guarantee string
> safety at API boundaries), config.[ch] (as many external tools are
> probably interested in parsing Git config formatted files directly),
> and a subset of operations related to the object store. These starting
> points are intended to have a small impact on the codebase and teach
> us a lot about logistics and best practices while doing these kinds of
> conversions.

>
> On 2/17/2023 4:12 PM, Emily Shaffer wrote:
>
> > This turned out pretty long-winded, so a quick summary before I dive in:
> >
> > - We want to compile parts of Git as independent libraries
> > - We want to do it by making incremental code quality improvements to Git
> > - Let's avoid promising stability of the interfaces of those libraries
> > - We think it'll let Git do cool stuff like unit tests and allowing
> > purpose-built plugins
> > - Hopefully by example we can convince the rest of the project to join
> > in the effort
>
> As I was thinking about this proposal, I realized that my biggest concern
> is that we are jumping to the "what" and "how" without fully exploring the
> "why"s, so I've reorganized some of your points along those lines, so I
> can respond to each in turn.

>
> ## Why: Modular code
>
> > - Having clear, modular libraries makes it easy to find the code
> > responsible for some behavior, or determine where to add something
> > new.
>
> > - An easy-to-understand modular codebase makes it easy for new
> > contributors to start hacking and understand the consequences of their
> > patch.
>
> Generally, having a modular codebase does not guarantee that things are
> easy to find or that consequences are understood. The correct abstractions
> are key in this, as well as developing boundaries that do not leak into
> each other.

>
> Unless done correctly, we can have significant issues with how things are
> communicated through layers of abstraction. We already have that problem
> when we want to add a new option to a builtin and need to modify three or
> four method prototypes in order to communicate a change of behavior to the
> right layer.

>
> I've also found that where we have abstractions, such as the refs code or
> the transport code, that the indirection created by the vtables is more
> confusing to discover by reading the code. I've needed the use of a
> debugger to even discover the call stack for certain operations.

>
> > - If we can swap out an entire library, or just a single function of
> > one, it's easy to experiment with the entire codebase without sweeping
> > changes.
>
> I don't understand this one too much. We already have this ability through
> forking the repository and mutating the code to do different things. Why
> do we need modules (with vtables and all the rest of the pain that goes
> into it) to make such swaps?
>
> This only really matters in the fullness of the libification effort: if
> everything is mocked, then subsystems can be replaced wholesale. That's a
> lot of work just to avoid experimenting with a fork.

>
> ...
>
> You said this about how to achieve modular code:
>
> > - Removing references to global variables and instead piping them
> > through arguments
> > - Finding and fixing memory leaks, especially in widely-used low-level code
> > - Reducing or removing `die()` invocations in low-level code, and
> > instead reporting errors back to callers in a consistent way
> ...
> > - Making module interfaces more consistent and easier to understand,
> > including moving "private" functions out of headers and into source
> > files and improving in-header documentation
>
> I think these mechanisms should be universally welcomed. One thing that is
> tricky is how we can guarantee that an API can be marked as "done" and
> maintained that way in perpetuity. (Unit tests may help with that, so
> let's come back to this then.)

>
> > - Clarifying the scope and layering of existing modules, for example
> > by moving single-use helpers from the shared module's scope into the
> > single user's scope
>
> This one is a bit murky to me. It sounds like that if there is only one
> caller to a strbuf_*() method that it should be made 'static' inside the
> caller instead of global in strbuf.h for use by future callers. Making
> this a goal in itself is probably more likely to see methods moving around
> more frequently due to the frequency of their use, rather than natural
> groupings based on which data structures are being mutated.

>
> What measurement would we optimize for? How could we maintain such
> standards as we go? The Git codebase doesn't really have a firm
> "architecture" other than "builtin code calls libgit.a code, which calls
> other libgit.a code as necessary". There aren't really strong layers
> between things. Everything assumes it can look up an object or load
> config. Are there organizational things that we can do in this space that
> would help decoupling things before jumping to libification?

>
> ## Why: Submodules
>
> > - Operations recursing into submodules can be run in-process, and the
> > callsite makes it very clear which repository context is being used;
> > if we're not subprocessing to recurse, then we can lose the awkward
> > git-submodule.sh -> builtin/submodule__helper.c -> top-level Git
> > process codepath.
>
> Previously, there was an effort to replace dependencies on the_repository
> in favor of repository structs being passed through method parameters.
> This seems to have fallen off of the priority list, and prevous APIs that
> were converted have regressed in their dependencies.
>
> Should we consider restarting that effort as an early goal?

> ## Why: Unit testing
>
> > - Unit tests. We already have some in t/helper/, but if we can replace
> > all the dependencies of a certain library with simple stubs, it's
> > easier for us to write comprehensive unit tests, in addition to the
> > work we already do introducing edge cases in bash integration tests.
>
> Unit tests are very valuable in keeping the codebase stable and reducing
> the chance that code was mutated by accident. This is achieved by very
> rigid test infrastructure, requiring _replacing_ methods with mocks in
> careful ways in order to test that behavior matches expectation. The
> "simple stubs" are actually carefully crafted to verify their inputs and
> provide a carefully selected return value.
>
> The difficulty of writing unit tests (or mutating the code after writing
> unit tests) is balanced by the ability to more explicitly create error
> conditions that are unlikely to occur in real test cases. Cases such as
> connection errors, file I/O problems, or other unexpected responses from
> the mocked methods, are all relatively difficult to check via end-to-end
> tests.
>
> I've personally found that the hardest unit test to write is the _first_
> one, and after that the rest are simpler. The difficulty arises in making
> the code testable in the first place, as well as creating infrastructure
> for the most-common unit test scenarios. My expectation is that it will
> take significant changes to the Git codebase to make any non-trivial unit
> tests outside of these very isolated cases that are presented here: strbuf
> manipulation is easy to unit-test and config methods operating on
> config_set structs should be similar. However, config methods that
> interact with a repository and its config file(s) will be much harder to
> test unless we start mocking filesystem interactions. We could create
> test-tool helpers that load a full repository and check the config files
> present on the filesystem, but now we're talking about integration tests
> instead of unit tests.

>
> > - Being able to test libraries in isolation via unit tests or mocks
> > speeds up determining the root cause of bugs.
>
> I'm not sure I completely agree with this statement. Unit tests can help
> prevent introducing a new fault when mutating code to adjust for a bug,
> but unit tests only guarantee expected behavior on the expected
> preconditions. None of this necessarily helps finding a root cause,
> especially in the likely event that the root cause is due to combining
> units and thus not covered by unit tests.

>
> ## Why: Correct use
>
> > The ability to use Git as a library also makes it easier for other
> > tooling to interact with a Git repository *correctly*.
>
> Is there is a correct way to interact with a Git repository? We definitely
> prefer that the repository is updated by executing Git processes, leaving
> all internals up to Git. Libification thus has a purpose for scenarios
> that do not have an appropriate Git builtin or where the process startup
> time is too expensive for that use.

>
> ## How to achieve these goals
>
> > I'm also planning to send a proposal for a document full of "best
> > practices" for turning Git code into libraries (and have quite a lot
> > of discussion around that document, too). My hope is that we can use
> > that document to help us during implementation as well as during
> > review, and refine it over time as we learn more about what works and
> > what doesn't. Having this kind of documentation will make it easy for
> > others to join us in moving Git's codebase towards a clean set of
> > libraries. I hope that, as a project, we can settle on some tenets
> > that we all agree would make Git nicer.
>
> I like the idea of a "best practices" document, but I would hesitate to
> focus on the libification and instead aim for the high-value items such
> as lack of globals and reduced memory leaks. How do we write such code?
> How do we write (unit?) tests that guarantee those properties will be
> maintained into the future?
>
> > From the rest of my own team, we're planning on working first on some
> > limited scope, low-level libraries so that we can all see how the
> > process works. We're starting with strbuf.[ch] (as it's used
> > everywhere with few or no dependencies and helps us guarantee string
> > safety at API boundaries), config.[ch] (as many external tools are
> > probably interested in parsing Git config formatted files directly),
> > and a subset of operations related to the object store. These starting
> > points are intended to have a small impact on the codebase and teach
> > us a lot about logistics and best practices while doing these kinds of
> > conversions.
>
> I can see that making a library version of config.h could be helpful to
> third parties wanting to read Git config. In particular, I know that Git
> Credential Manager runs multiple 'git config' calls to get multiple values
> out. Perhaps time would be better spent creating a 'git config --batch'
> mode so these consumers could call one process, pass a list of config keys
> as input, and get a list of key-value pairs out? (Use a '-z' mode to allow
> newlines in the values.)
>
> However, I'm not seeing value to anyone else to have strbuf.h available
> externally. I'm also not seeing any value to the Git project by having
> either of these available as a library. I can only see increased developer
> friction due to the new restrictions on making changes in these areas.

>
> If this first effort is instead swapped to say "we plan on introducing
> unit tests for these libraries" then I think that is an easier thing to
> support.
>
> It happens that these efforts will make it easier to make the jump to
> providing APIs as a library. Not only are unit tests _necessary_ to
> support libification, they also make a smaller lift. Unit tests will
> already create more friction when changing the API, since either new tests
> must be written or old tests must be modified.
>
>
> ### Summary
>
> Generally, making architectural change is difficult. In addition to the
> amount of code that needs to be moved, adjusted, and tested in new ways,
> there is a cultural element that needs to be adjusted. Expecting Git's
> code to be used as a library is a fundamentally different use case than we
> have supported in the past, and most of our developer guidelines reflect
> that. We don't hesitate to modify APIs. We prefer end-to-end tests over
> unit tests whenever possible.

>
> There are focused bits of your proposal that I think will be universally
> welcomed, and I think the best course of action in the short term is to
> demonstrate improvements in those areas:
>
>  * Reduced use of globals.
>  * Reduced memory leaks.
>  * Use error responses instead of die()s within low-level code.
>  * Update header files to have appropriate visibility and documentation.
>
> The things that we need to really focus on are how we can measure progress
> in these areas as well as how can we prevent regression in the future.
> Unit tests are one way to do this (especially with leak detection), but
> also documentation and coding guidelines need to be updated based on the
> new patterns discovered in this process.
>
> I mentioned the_repository earlier as a partially-complete architectural
> change. I think the_index compatibility macros are in a similar boat of
> being half-completed (though, the last time I tried to remove these macros
> I was told that it wasn't a goal to remove the macros from builtins). The
> sparse-index's command_requires_full_index checks are also in this boat,
> so I understand the difficulty of getting a big change 100% complete (and
> in this case, the plan is to do at least one more GSoC project in the area
> to see what progress can be made that way).
>
> We should be careful in committing to a very long-term direction without
> first delivering value on a shorter timeline.
>
> Thanks,
> -Stolee

> As I mentioned in standup this week[1], my colleagues and I at Google
> have become very interested in converting parts of Git into libraries
> usable by external programs. In other words, for some modules which
> already have clear boundaries inside of Git - like config.[ch],
> strbuf.[ch], etc. - we want to remove some implicit dependencies, like
> references to globals, and make explicit other dependencies, like
> references to other modules within Git. Eventually, we'd like both for
> an external program to use Git libraries within its own process, and
> for Git to be given an alternative implementation of a library it uses
> internally (like a plugin at runtime).

>On Fri, Feb 17, 2023 at 3:45 PM Emily Shaffer <nasamuffin@google.com> wrote:
>
>> As I mentioned in standup this week[1], my colleagues and I at Google
>> have become very interested in converting parts of Git into libraries
>> usable by external programs. In other words, for some modules which
>> already have clear boundaries inside of Git - like config.[ch],
>> strbuf.[ch], etc. - we want to remove some implicit dependencies, like
>> references to globals, and make explicit other dependencies, like
>> references to other modules within Git. Eventually, we'd like both for
>> an external program to use Git libraries within its own process, and
>> for Git to be given an alternative implementation of a library it uses
>> internally (like a plugin at runtime).
>
>This is obviously the way it should have been done from the beginning, but
>unfortunately at this point the Git project has too much inertia and too many vested
>interests from multi-billion dollar corporations to change.
>
>I wonder if a single person who isn't paid to work on Git commented on this thread.