Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: <rsbecker@nexbridge.com>
To: "'Felipe Contreras'" <felipe.contreras@gmail.com>
Cc: <phillip.wood@dunelm.org.uk>, "'demerphq'" <demerphq@gmail.com>,
	"'Junio C Hamano'" <gitster@pobox.com>,
	"'Emily Shaffer'" <nasamuffin@google.com>,
	"'Git List'" <git@vger.kernel.org>,
	"'Jonathan Nieder'" <jrn@google.com>,
	"'Jose Lopes'" <jabolopes@google.com>,
	"'Aleksandr Mikhailov'" <avmikhailov@google.com>
Subject: RE: Proposal/Discussion: Turning parts of Git into libraries
Date: Fri, 24 Mar 2023 18:06:12 -0400	[thread overview]
Message-ID: <005601d95e9c$dc20b360$94621a20$@nexbridge.com> (raw)
In-Reply-To: <CAMP44s3fXYcOsq-XRNZX0y6D=W37=ONELUpTBtkzv4KLbym2iA@mail.gmail.com>

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. 

config.h requires that the ./configure script runs (which it does not because of python3). replace.h is over 1000 lines in this version.

This also gets into git supply chain issues where people who want to build git, outside of specific platforms, need to modify the build environment associated with talloc, which changes the delivered signature.

Speaking purely from a selfish standpoint, this is more than a trivial amount of work at least for me (the above Makefile does not satisfy 2.4.0) that is proposed in order to make talloc a seamless addition. Running the talloc test suite, which I consider a requirement to adding this as a dependency, is also problematic for the reasons I previously indicated.

We may just have to agree to disagree, but I stand by my concern that this suggestion will cause maintenance issues given the current state of the talloc code - and I do not have the cycles to become a community maintainer of that code also.

--Randall


  reply	other threads:[~2023-03-24 22:06 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-17 21:12 Proposal/Discussion: Turning parts of Git into libraries Emily Shaffer
2023-02-17 21:21 ` brian m. carlson
2023-02-17 21:38   ` Emily Shaffer
2023-02-17 22:41     ` brian m. carlson
2023-02-17 22:49       ` Emily Shaffer
2023-02-22 19:34         ` Jeff King
2023-02-24 20:31           ` Emily Shaffer
2023-02-24 21:41             ` Jeff King
2023-02-24 22:59             ` Junio C Hamano
2023-02-17 22:04   ` rsbecker
2023-02-17 22:48     ` brian m. carlson
2023-02-17 22:57 ` Junio C Hamano
2023-02-18  1:59   ` demerphq
2023-02-18 10:36     ` Phillip Wood
2023-03-23 23:22       ` Felipe Contreras
2023-03-23 23:30         ` rsbecker
2023-03-23 23:34           ` Felipe Contreras
2023-03-23 23:42             ` rsbecker
2023-03-23 23:55               ` Felipe Contreras
2023-03-24 19:27                 ` rsbecker
2023-03-24 21:21                   ` Felipe Contreras
2023-03-24 22:06                     ` rsbecker [this message]
2023-03-24 22:29                       ` Felipe Contreras
2023-02-21 21:42   ` Emily Shaffer
2023-02-22  0:22     ` Junio C Hamano
2023-02-18  4:05 ` Elijah Newren
2023-02-21 22:06   ` Emily Shaffer
2023-02-22  8:23     ` Elijah Newren
2023-02-22 19:25     ` Jeff King
2023-02-21 19:09 ` Taylor Blau
2023-02-21 22:27   ` Emily Shaffer
2023-02-22  1:44 ` Victoria Dye
2023-02-25  1:48   ` Jonathan Tan
2023-02-22 14:55 ` Derrick Stolee
2023-02-24 21:06   ` Emily Shaffer
2023-03-23 23:37 ` Felipe Contreras
2023-03-23 23:44   ` rsbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='005601d95e9c$dc20b360$94621a20$@nexbridge.com' \
    --to=rsbecker@nexbridge.com \
    --cc=avmikhailov@google.com \
    --cc=demerphq@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jabolopes@google.com \
    --cc=jrn@google.com \
    --cc=nasamuffin@google.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).