All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Elia Pinto <gitter.spiros@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] git-curl-compat.h: addition of all symbols defined by curl
Date: Tue, 15 Mar 2022 20:30:21 +0100	[thread overview]
Message-ID: <220315.86pmmndmre.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20220315131638.924669-1-gitter.spiros@gmail.com>


On Tue, Mar 15 2022, Elia Pinto wrote:

> This file was produced from a modified version of symbols.pl
> (https://github.com/curl/curl/blob/master/docs/libcurl/symbols.pl) and
> by manually adding the previous comments describing the dates of release
> of some curl versions not currently reported in the symbols-in-versions.
>
> To do this the symbols are listed in the order defined in the file
> symbols-in-versions rather than as they were previously inserted based
> on release dates.
>
> Most of these symbols are not used by git today. However, inserting
> them all starting from an automatic tool makes it largely unnecessary
> to update this file and therefore reduces the possibility
> of introducing possible errors in the future.
> [...]
>  1 file changed, 2875 insertions(+), 69 deletions(-)

*gulp* :)

> +#define LIBCURL_HAS(x) \
> +  (defined(x ## _FIRST) && (x ## _FIRST <= LIBCURL_VERSION_NUM) && \
> +   (!defined(x ## _LAST) || ( x ## _LAST >= LIBCURL_VERSION_NUM)))

As the recent author of this file I think there's certainly space to
improve it, e.g. perhaps this macro & the change Junio suggests
downthread, but...

> +#define CURLALTSVC_H1_FIRST 0x074001 /* Added in 7.64.1 */
> +#define GIT_CURL_HAVE_CURLALTSVC_H1 \

[snip ~3k lines lines]

I'd really prefer for us not to go down this route. If you look at the
patches I did where I removed the version comparisons in favor of these
GIT_* constants the improvement was in readability & discovery. I.e. to
see at a glance where this project was on curl compatibility &
compatibility shims.

If we add a new version-dependant curl feature the time saving is really
not going to be in adding a trivial GIT_CURL_* line to the file. It's
always going to be the compatibility shim itself that takes *much*
longer.

Right now you can easily discover where we need those shims with "git
grep GIT_?CURL" (the "?" needing fixing, as noted downthread), after
this...


      parent reply	other threads:[~2022-03-15 19:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-15 13:16 [PATCH] git-curl-compat.h: addition of all symbols defined by curl Elia Pinto
2022-03-15 15:47 ` Junio C Hamano
2022-03-16 10:33   ` Elia Pinto
2022-03-15 19:30 ` Ævar Arnfjörð Bjarmason [this message]

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=220315.86pmmndmre.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitter.spiros@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.