* Re: [PATCH v5 02/10] daemon: libify child process handling functions
From: Matthew John Cheetham @ 2023-01-17 21:14 UTC (permalink / raw)
To: Victoria Dye, Matthew John Cheetham via GitGitGadget, git
Cc: Derrick Stolee, Lessley Dennington, M Hickford, Jeff Hostetler,
Glen Choo, Matthew John Cheetham
In-Reply-To: <3a8d1b66-ed06-16a3-5459-9381faa69420@github.com>
On 2023-01-12 11:35, Victoria Dye wrote:
> Matthew John Cheetham via GitGitGadget wrote:
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
>>
>> Extract functions and structures for managing child processes started
>> from the parent daemon-like process from `daemon.c` to the new shared
>> `daemon-utils.{c,h}` files.
>
> As with patch 1, it looks like the main changes here are changing global
> references to function arguments. Specifically, those variables are
> 'firstborn', 'live_children', and 'loginfo':
>
>> -static void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen)
>> +void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen,
>> + struct child *firstborn , unsigned int *live_children)
>
>> -static void kill_some_child(void)
>> +void kill_some_child(struct child *firstborn)
>
>> -static void check_dead_children(void)
>> +void check_dead_children(struct child *firstborn, unsigned int *live_children,
>> + log_fn loginfo)
>
> Those values are provided by the callers in 'daemon.c'. The major change
> here is that 'live_children' is passed as a pointer, since its value is
> updated by difference is passing 'live_children' as a pointer, since its
> value is updated by 'check_dead_children()' and 'add_child()':
>
>> @@ -879,9 +797,9 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
>> struct child_process cld = CHILD_PROCESS_INIT;
>>
>> if (max_connections && live_children >= max_connections) {
>> - kill_some_child();
>> + kill_some_child(firstborn);
>> sleep(1); /* give it some time to die */
>> - check_dead_children();
>> + check_dead_children(firstborn, &live_children, loginfo);
>> if (live_children >= max_connections) {
>> close(incoming);
>> logerror("Too many children, dropping connection");
>> @@ -914,7 +832,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
>> if (start_command(&cld))
>> logerror("unable to fork");
>> else
>> - add_child(&cld, addr, addrlen);
>> + add_child(&cld, addr, addrlen, firstborn, &live_children);
>> }
>>
>> static void child_handler(int signo)
>> @@ -944,7 +862,7 @@ static int service_loop(struct socketlist *socklist)
>> for (;;) {
>> int i;
>>
>> - check_dead_children();
>> + check_dead_children(firstborn, &live_children, loginfo);
>>
>> if (poll(pfd, socklist->nr, -1) < 0) {
>> if (errno != EINTR) {
>
> However, I think that change to 'live_children' may have caused a bug. In
> 'check_dead_children()', you decrement the 'live_children' *pointer*. That
> changes its address, not its value:
>
>> +void check_dead_children(struct child *firstborn, unsigned int *live_children,
>> + log_fn loginfo)
>> +{
> ...
>> + live_children--;
> ...
>> +}
>
> Same thing in 'add_child()':
>
>> +void add_child(struct child_process *cld, struct sockaddr *addr, socklen_t addrlen,
>> + struct child *firstborn , unsigned int *live_children)
>> +{
> ...
>> + live_children++;
> ...
>> +}
>
> These should be changed to '(*live_children)--' and '(*live_children)++',
> respectively.
Ah! You are correct; my bad. Will correct this in v6.
> There's also one minor functional change in 'check_dead_children()', where
> an 'if (loginfo)' check is added guarding the call to 'loginfo()':
>
>> +void check_dead_children(struct child *firstborn, unsigned int *live_children,
>> + log_fn loginfo)
>> +{
> ...
>> + if (loginfo) {
>> + const char *dead = "";
>> + if (status)
>> + dead = " (with error)";
>> + loginfo("[%"PRIuMAX"] Disconnected%s",
>> + (uintmax_t)pid, dead);
>> + }
> ...
>> +}
>
> I'm guessing this is done because a caller later in the series won't provide
> a 'loginfo', but if that's the case, it would help to note that in this
> patch's commit message.
Will call this out in the commit message in v6.
> The one other thing I noticed is that you removed the function documentation
> for 'kill_some_child()':
>
>> -/*
>> - * This gets called if the number of connections grows
>> - * past "max_connections".
>> - *
>> - * We kill the newest connection from a duplicate IP.
>> - */
>
> Is there a reason why you removed it? Otherwise, it should be added back in
> - probably in 'daemon-utils.h'?
I removed it initially as it was referencing things like `max_connections`
which no longer existed in the context of `daemon-utils.{c,h}`.
Next iteration I can restore the spirit of the comment, that this should be
called when the maximimum number of connections has been reached, in order
to kill the newest connection from a duplicate IP.
> Everything else here looks good.
Thanks,
Matthew
^ permalink raw reply
* Re: [PATCH 1/8] t5558: add tests for creationToken heuristic
From: Derrick Stolee @ 2023-01-17 21:00 UTC (permalink / raw)
To: Victoria Dye, Derrick Stolee via GitGitGadget, git
Cc: gitster, me, avarab, steadmon, chooglen
In-Reply-To: <4d13cea1-e9f6-381f-14ef-8a5a645e2a8f@github.com>
On 1/17/2023 1:17 PM, Victoria Dye wrote:
> Derrick Stolee via GitGitGadget wrote:
>> + for b in 1 2 3 4
>> + do
>> + test_bundle_downloaded bundle-$b.bundle trace-clone.txt ||
>> + return 1
>> + done
>
> Because the current state of bundle list handling is equivalent to "no
> heuristic", this pre-existing test is just updated to verify all bundles are
> downloaded. This isn't new behavior, but it'll be relevant to compare with
> the behavior of the 'creationToken' heuristic.
>
> I was going to ask how the tests verify that *only* the expected bundles are
> downloaded, and it looks like later patches [1] handle that with
> '! test_bundle_downloaded' checks. That approach seems a bit fragile (if a
> bundle's name doesn't match the '! test_bundle_downloaded' check for some
> reason, the bundle can be either downloaded or not with no effect on the
> test result). Would something like a 'test_downloaded_bundle_count' work
> instead?
Or, perhaps we could check the exact list _and order_ using this slightly
more generic helper?
# Given a GIT_TRACE2_EVENT log over stdin, writes to stdout a list of URLs
# sent to git-remote-https child processes.
test_remote_https_urls() {
grep -e '"event":"child_start".*"argv":\["git-remote-https",".*"\]' |
sed -e 's/{"event":"child_start".*"argv":\["git-remote-https","//g' \
-e 's/"\]}//g'
}
With a test example looking a lot like this:
cat >expect <<-EOF &&
$HTTPD_URL/newest.bundle
$HTTPD_URL/new.bundle
$HTTPD_URL/everything.bundle
EOF
test_remote_https_urls <trace-clone.txt >actual &&
test_cmp expect actual
Thanks for the inspiration.
>> +test_expect_success 'clone bundle list (http, creationToken)' '
>> + cp clone-from/bundle-*.bundle "$HTTPD_DOCUMENT_ROOT_PATH/" &&
>> + cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
>> + [bundle]
>> + version = 1
>> + mode = all
>> + heuristic = creationToken
>> +
>> + [bundle "bundle-1"]
>> + uri = bundle-1.bundle
>> + creationToken = 1
>> +
>> + [bundle "bundle-2"]
>> + uri = bundle-2.bundle
>> + creationToken = 2
>> +
>> + [bundle "bundle-3"]
>> + uri = bundle-3.bundle
>> + creationToken = 3
>> +
>> + [bundle "bundle-4"]
>> + uri = bundle-4.bundle
>> + creationToken = 4
>> + EOF
>> +
>> + git clone --bundle-uri="$HTTPD_URL/bundle-list" . clone-list-http-2 &&
>> +
>> + git -C clone-from for-each-ref --format="%(objectname)" >oids &&
>> + git -C clone-list-http-2 cat-file --batch-check <oids
>
> This test looks like the one that was updated above, but adds the
> 'creationToken' heuristic key. However, the 'test_bundle_downloaded' check
> isn't included - if it were, it would need to verify that all bundles were
> downloaded, with the heuristic being ignored, all bundles will be downloaded
> (which isn't consistent with what the 'creationToken' heuristic will
> *eventually* do).
>
> As a matter of personal preference (so no pressure to change if you
> disagree), I find this test in its current state a bit misleading; because
> it's a 'test_expect_success' and there's no "NEEDSWORK" or "TODO", I could
> easily assume that cloning from a bundle list with the 'creationToken'
> heuristic is working as-intended at this point (that is, there's no
> indication that it's not implemented).
It's true that it's not implemented right now, and it is a bit misleading
because of that. At clone time, the only thing that will change with the
implementation is possibly the order of the files being downloaded (and
the order is not predictable before that implementation).
The restriction of _not_ downloading some files comes only for the 'git
fetch' implementation, so these test changes are only foundations for those
future tests.
The only benefit of having these tests right now is that we get some
demonstration that the existing implementation ignores unknown properties
in the bundle list.
> If you did want to change it, adding a 'NEEDSWORK' comment, changing to
> 'test_expect_failure' & including the appropriate 'test_bundle_downloaded'
> check, or moving this test to the patch where the heuristic is implemented
> would mitigate any confusion. That said, this "issue" is resolved by the
> end of the series anyway, so it's really a low priority fix.
I think if we wanted to go this route, we could do it with the "download
the bundles in this order" check, or possibly by adding the 'git fetch'
behavior into the test at this point. I'll consider these options for v2.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v5 10/10] credential: add WWW-Authenticate header to cred requests
From: Matthew John Cheetham @ 2023-01-17 21:35 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason,
Matthew John Cheetham via GitGitGadget
Cc: git, Derrick Stolee, Lessley Dennington, M Hickford,
Jeff Hostetler, Glen Choo, Victoria Dye, Matthew John Cheetham
In-Reply-To: <230112.86sfgg2kuj.gmgdl@evledraar.gmail.com>
On 2023-01-12 00:48, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Jan 11 2023, Matthew John Cheetham via GitGitGadget wrote:
>
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
>> [...]
>> +static void credential_write_strvec(FILE *fp, const char *key,
>> + const struct strvec *vec)
>> +{
>> + int i = 0;
>> + const char *full_key = xstrfmt("%s[]", key);
>> + for (; i < vec->nr; i++) {
>> + credential_write_item(fp, full_key, vec->v[i], 0);
>
> Style: Don't mismatch types if there's no good reason. Use "size_t i" here, also let's do:
>
> for (size_t i = 0; ....
>
> I.e. no reason to declare it earlier.
>
>> + }
>> + free((void*)full_key);
>
> Just don't add a "const" to that "full_key" and skip the cast with
> free() here.
Both good points! Thanks - will take this onboard in next iteration.
>> +++ b/t/helper/test-credential-helper-replay.sh
>
> I see to my surprise that we have one existing *.sh helper in that
> directory, but in any case...
>
>> @@ -0,0 +1,14 @@
>> +cmd=$1
>> +teefile=$cmd-actual.cred
>> +catfile=$cmd-response.cred
>> +rm -f $teefile
>> +while read line;
>> +do
>> + if test -z "$line"; then
>> + break;
>> + fi
>> + echo "$line" >> $teefile
>> +done
>
> It looks like you're re-inventing "sed" here, isn't this whole loop just
>
> sed -n -e '/^$/q' -n 'p'
True; `sed -n -e '/^$/q' -e 'p'` is equivalent here.
> And then you can skip the "rm" before, as you could just clobber the
> thing.
>
>> +if test "$cmd" = "get"; then
>> + cat $catfile
>> +fi
>> diff --git a/t/t5556-http-auth.sh b/t/t5556-http-auth.sh
>> index 65105a5a6a9..a8dbee6ca40 100755
>> --- a/t/t5556-http-auth.sh
>> +++ b/t/t5556-http-auth.sh
>> @@ -27,6 +27,8 @@ PID_FILE="$TRASH_DIRECTORY"/pid-file.pid
>> SERVER_LOG="$TRASH_DIRECTORY"/OUT.server.log
>>
>> PATH="$GIT_BUILD_DIR/t/helper/:$PATH" && export PATH
>> +CREDENTIAL_HELPER="$GIT_BUILD_DIR/t/helper/test-credential-helper-replay.sh" \
>> + && export CREDENTIAL_HELPER
>
> ...(continued from above): Let's just use write_script() here or
> whatever, i.e. no reason to make this a global script, it's just used in
> this one test, so it can set it up.
In the next iteration I will move to using write_script; thanks!
>> test_expect_success 'setup repos' '
>> test_create_repo "$REPO_DIR" &&
>> @@ -92,15 +94,279 @@ start_http_server () {
>>
>> per_test_cleanup () {
>> stop_http_server &&
>> - rm -f OUT.*
>> + rm -f OUT.* &&
>> + rm -f *.cred &&
>> + rm -f auth.config
>> }
>>
>> test_expect_success 'http auth anonymous no challenge' '
>> test_when_finished "per_test_cleanup" &&
>> - start_http_server &&
>> +
>> + cat >auth.config <<-EOF &&
>> + [auth]
>> + allowAnonymous = true
>
> Mixed tab/space. Use "\t" not 4x " " (ditto below).
Sure!
^ permalink raw reply
* [PATCH v5 1/2] send-email: refactor header generation functions
From: Strawbridge, Michael @ 2023-01-17 21:36 UTC (permalink / raw)
To: avarab@gmail.com
Cc: Tuikov, Luben, Strawbridge, Michael, git@vger.kernel.org,
gitster@pobox.com, Strawbridge, Michael
In-Reply-To: <230117.86wn5lxpl0.gmgdl@evledraar.gmail.com>
On Tue, Jan 17 2023, Ævar Arnfjörð Bjarmason wrote:
>On Tue, Jan 10 2023, Strawbridge, Michael wrote:
>
>> Split process_file and send_message into easier to use functions.
>> Making SMTP header information more widely available.
>>
>> Cc: Luben Tuikov <luben.tuikov@amd.com>
>> Cc: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
>> ---
>> git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
>> 1 file changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 5861e99a6e..810dd1f1ce 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
>> return File::Spec::Functions::file_name_is_absolute($path);
>> }
>>
>> -# Prepares the email, then asks the user what to do.
>> -#
>> -# If the user chooses to send the email, it's sent and 1 is returned.
>> -# If the user chooses not to send the email, 0 is returned.
>> -# If the user decides they want to make further edits, -1 is returned and the
>> -# caller is expected to call send_message again after the edits are performed.
>> -#
>> -# If an error occurs sending the email, this just dies.
>> -
>> -sub send_message {
>> +sub gen_header {
>> my @recipients = unique_email_list(@to);
>> @cc = (grep { my $cc = extract_valid_address_or_die($_);
>> not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
>> @@ -1546,6 +1537,22 @@ sub send_message {
>> if (@xh) {
>> $header .= join("\n", @xh) . "\n";
>> }
>> + my $recipients_ref = \@recipients;
>> + return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
>> +}
>> +
>> +# Prepares the email, then asks the user what to do.
>> +#
>> +# If the user chooses to send the email, it's sent and 1 is returned.
>> +# If the user chooses not to send the email, 0 is returned.
>> +# If the user decides they want to make further edits, -1 is returned and the
>> +# caller is expected to call send_message again after the edits are performed.
>> +#
>> +# If an error occurs sending the email, this just dies.
>> +
>> +sub send_message {
>> + my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
>
>This makes the diff smaller, but if we're refactoring these functions to
>return arguments it's probably better to return a hash reference rather
>than remembering all the parameter names.
>
>But in this case it's probably fine...
>
I hadn't known about passing a hash reference on perl. Although after
looking into it, I'm not sure I will go that way this time.
>> + my @recipients = @$recipients_ref;
>>
>> my @sendmail_parameters = ('-i', @recipients);
>> my $raw_from = $sender;
>> @@ -1735,11 +1742,8 @@ sub send_message {
>> $references = $initial_in_reply_to || '';
>> $message_num = 0;
>>
>> -# Prepares the email, prompts the user, sends it out
>> -# Returns 0 if an edit was done and the function should be called again, or 1
>> -# otherwise.
>> -sub process_file {
>> - my ($t) = @_;
>> +sub pre_process_file {
>> + my ($t, $quiet) = @_;
>
>This, I think, is an anti-pattern in this file. We can just read the
>"$quiet" and other things that we set up when we parse the parameters as
>globals, we don't need to pass them as named parameters.
>
>It doesn't help readability to shadow that variable with another lexical
>here below:
>
I am open to guidance here. The reason I included quiet as an argument
was because I wanted to override the global quiet value for the call of
pre_process_file from the validation section (line 1736). This is needed
otherwise the user gets spammed with double print statements. Once in
the validation loop and another time in the actual sending message part.
The paths forward that I can currently see are:
1) a) remain as it is
b) rename quiet in pre_process_file but keep the logic the same
2) in the validation section, temporarily save the quiet value, alter it to
indicate quieting of the print statements, and then put the quiet value back
3) separate out the print statements inside pre_prcoess_file into a different
function and only call that function in the send message code.
>> [...]
>> +}
>> +
>> +# Prepares the email, prompts the user, sends it out
>> +# Returns 0 if an edit was done and the function should be called again, or 1
>> +# otherwise.
>> +sub process_file {
>> + my ($t) = @_;
>> +
>> + pre_process_file($t, $quiet);
>>
>> my $message_was_sent = send_message();
>> if ($message_was_sent == -1) {
>> @@ -2002,7 +2015,7 @@ sub process_file {
>> # Execute a command (e.g. $to_cmd) to get a list of email addresses
>> # and return a results array
>> sub recipients_cmd {
>> - my ($prefix, $what, $cmd, $file) = @_;
>> + my ($prefix, $what, $cmd, $file, $quiet) = @_;
>>
>> my @addresses = ();
>> open my $fh, "-|", "$cmd \Q$file\E"
I appreciate your feedback!
--
2.34.1
^ permalink raw reply
* Re: ctrl-z ignored by git; creates blobs from non-existent repos
From: Crls @ 2023-01-17 22:40 UTC (permalink / raw)
To: Jeff King; +Cc: Theodore Ts'o, git
In-Reply-To: <Y8SNNeQ3fQdrf5Fi@coredump.intra.peff.net>
On Sun, Jan 15, 2023 at 06:33:09PM -0500, Jeff King wrote:
> On Sun, Jan 15, 2023 at 05:47:02PM -0500, Crls wrote:
>
> > > Now, when you type ^Z, the git processes are stopped --- but the
> > > objects are created already.
> >
> > The directories exist not because ^Z is used, but because by the time
> > git-clone prompts for a username, git is already set on what to do
> > next. Correct? in other words, the process is shoved down my throat.
> > Or the user's throat in this case. Or going by another analogy, it
> > certainly sounds as if I meant: «Git, please git-clone such and such
> > repo, but let me fix just a typo on the repo name before submitting
> > it, pretty please» and then Git replies: «too late for that
> > chick-a-doodle» and there it goes. It injects blobs all over (well,
> > not all over but on the dir specified).
>
> I don't know what you mean by "blobs" here, since we fail to download
> any Git objects at all, let alone blobs. But yes, Git creates the local
> repository, and then tries to fetch into it. So the directory is created
> before it even contacts the remote server at all, and you will see the
> same behavior whether the repository exists or not. And then if an error
> occurs, it will rolls back by deleting the newly-created repository.
>
> It _could_ be possible to contact the server first, and only when things
> looked successful-ish, to create the local repository. But:
>
> 1. The clone command is simply not written that way, and converting it
> now would be tricky. The clone command's view of the world is that
> it makes a new repository, sets up config, etc, then fetches into
> it.
>
> 2. It would not fix all cases anyway. At some point we have to say
> "this looks close enough to success to create the directory", but
> things can still go wrong after that.
>
What I meant by blobs is in the following:
Ending up with a list of directories and subdirectories, is not unusual, is actually very common really with every git-clone while fetching a non-existent repo with a git clone operation either through github or gitlab. First it comes with two (2) git, or one git instance first, then another and finally one (1) git-remote-helper instance.
ps -a | grep 'git' can confirm it
You'd be able to see
19159 pts/9 00:00:00 git
19160 pts/9 00:00:00 git
19161 pts/9 00:00:00 git-remote-http <defunct>
19188 pts/9 00:00:00 git
19189 pts/9 00:00:00 git
19190 pts/9 00:00:00 git-remote-http <defunct>
Obviously, its <defunct processs > part was only made possible just right after and invoking a `kill -9 process id`
And if you wanted to find out the leftovers behind through a `du -h <reponame>` by either by git/git-remote-helper (it doesn't really matter at this point which one out of the two may have been the culprit here):
8.0K /.git/info
4.0K /.git/objects/info
4.0K /.git/objects/pack
12K /.git/objects
64K /.git/hooks
4.0K /.git/refs/tags
4.0K /.git/refs/heads
12K /.git/refs
4.0K /.git/branches
116K /.git
120K /
These are the default values. Any curl operation through git with a non-existent repo will get those values.
Now. We can all proceed trying to sugar coat these leftovers behind an explanation of whatever git-remote-http is actually doing or for that matter, as I said before and without being redundant here, the git-clone processs itself, but it doesn't realyl matter, the end results are the same. Are there leftovers? You can bet on it. It may not happen — and borowwing someone else's phrasing, wording, — «instantaneous», for it's actually quite clever really, to the detriment of an end-user.
I've read tibbits from gitremote-helpers(7) quote: When Git encounters a URL of the form <transport>://<address>, where <transport> is a protocol that it cannot handle natively, it automatically invokes git remote-<transport> with the full URL as thesecond argument. End quote. But it's unsettling. The whole issue. The gitremote-helper is not helping, no pun intended. Luckily for these use cases ^C and ^\ seem to be functioning.
> Now if you have a problem with the rollback, there might be a bug there.
> But it sounds like you are simply stopping the process and not letting
> it finish. It should roll back even if it receives a signal death, but
> ^Z is stopping the job and putting it in limbo. If it hurts, don't do it
> (or use "fg" or "bg" to let it finish).
>
> > > So what's going on is that github.com is not returning a non-existent
> > > repo error; it's prompting for a username/password, as _if_ the
> > > repository exists. That's presumably to prevent disclosing
> > > information as to whether or not a private repository exists or not.
> >
> > I agree with you there. Coincidentally speaking, why does a username
> > warrants a prompt from git, is simply beyond me. I mean, that is
> > certainly the more far-fetched reasoning of implementation I've read
> > in a long long time.
> >
> > Can you git-clone a user? What about the user's settings? What about
> > the remainder its gpg tokens and so forth? In other words, if a user's
> > repo is not found, why even prompting for a username? The latter, that
> > is, the user's repo, is redundant, when the prompt is clearly asking
> > for a username, and not a repo.
>
> Huh? GitHub cannot tell you whether you have access to a repository (or,
> for privacy reasons of the owner of the hypothetical repository, whether
> a repository of that name might exist) unless you authenticate. So it
> returns an HTTP 401. Your authentication in this case requires a
> username and password. Git asks for the username first, then the
> password.
>
> Is there something else you think GitHub should be returning there? Or
> something else you think Git should do with an HTTP 401?
>
> -Peff
--
Don't hit the keys so hard, it hurts.
^ permalink raw reply
* [PATCH v8] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 21:41 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v7.git.git.1673987583356.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v8
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v8
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v7:
1: 23094afb2e6 ! 1: 5e44592695e curl: resolve deprecated curl declarations
@@ http.c: void setup_curl_trace(CURL *handle)
+ }
+ if (is_transport_allowed("https", from_user)) {
-+ strbuf_addstr(proto_buf, proto_buf->len ? "https" : ",https");
++ strbuf_addstr(proto_buf, proto_buf->len ? ",https" : "https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
-+ strbuf_addstr(proto_buf, proto_buf->len ? "ftp" : ",ftp");
++ strbuf_addstr(proto_buf, proto_buf->len ? ",ftp" : "ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
-+ strbuf_addstr(proto_buf, proto_buf->len ? "ftps" : ",ftps");
++ strbuf_addstr(proto_buf, proto_buf->len ? ",ftps" : "ftps");
+ }
+}
+#else
INSTALL | 2 +-
git-curl-compat.h | 8 +++++++
http-push.c | 6 ++---
http.c | 57 +++++++++++++++++++++++++++++++++++++----------
http.h | 2 +-
remote-curl.c | 31 +++++++++++++-------------
6 files changed, 73 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..79ea98d7d54 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +763,26 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
+ if (is_transport_allowed("http", from_user)) {
+ strbuf_addstr(proto_buf, "http");
+ }
+ if (is_transport_allowed("https", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? ",https" : "https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? ",ftp" : "ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? ",ftps" : "ftps");
+ }
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +798,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -923,10 +941,25 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ {
+ struct strbuf buf = STRBUF_INIT;
+
+ get_curl_allowed_protocols(&buf, 0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+ strbuf_reset(&buf);
+
+ get_curl_allowed_protocols(&buf, -1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+ strbuf_release(&buf);
+ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* Re: [ANNOUNCE] Git 2.39.1 and others
From: Ramsay Jones @ 2023-01-17 22:35 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: linux-kernel, git-packagers, lwn
In-Reply-To: <xmqq7cxl9h0i.fsf@gitster.g>
On 17/01/2023 18:02, Junio C Hamano wrote:
> A maintenance release v2.39.1, together with releases for older
> maintenance tracks v2.38.3, v2.37.5, v2.36.4, v2.35.6, v2.34.6,
> v2.33.6, v2.32.5, v2.31.6, and v2.30.7, are now available at the
> usual places.
FYI:
$ git tag -v v2.30.7
object b7b37a33711e64bf580ef3141878b12a36e28833
type commit
tag v2.30.7
tagger Junio C Hamano <gitster@pobox.com> 1670933134 +0900
Git 2.30.7
gpg: Signature made 13 Dec 2022 12:05:34 GMT
gpg: using RSA key B0B5E88696AFE6CB
gpg: Good signature from "Junio C Hamano <gitster@pobox.com>"
gpg: aka "Junio C Hamano <jch@google.com>"
gpg: aka "Junio C Hamano <junio@pobox.com>"
gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.
Primary key fingerprint: 96E0 7AF2 5771 9559 80DA D100 20D0 4E5A 7136 60A7
Subkey fingerprint: E1F0 36B1 FEE7 221F C778 ECEF B0B5 E886 96AF E6CB
$ git tag -v v2.31.6
object 82689d5e5d3f41da2ab1fbf9fbe7aacfd6da74c1
type commit
tag v2.31.6
tagger Junio C Hamano <gitster@pobox.com> 1670933242 +0900
Git 2.31.6
error: no signature found
$ git tag -v v2.32.5
object d96ea538e8dd0fcf381089a3b09c0a9af3617351
type commit
tag v2.32.5
tagger Junio C Hamano <gitster@pobox.com> 1670933456 +0900
Git 2.32.5
error: no signature found
$ git tag -v v2.33.6
object 7fe9bf55b84d2610a7ac09893b25ef188f145a21
type commit
tag v2.33.6
tagger Junio C Hamano <gitster@pobox.com> 1670933643 +0900
Git 2.33.6
error: no signature found
$ git tag -v v2.34.6
object 6c9466944c90c236217ec6f9ce5ed6b0e73903f9
type commit
tag v2.34.6
tagger Junio C Hamano <gitster@pobox.com> 1670933752 +0900
Git 2.34.6
error: no signature found
$ git tag -v v2.35.6
object 02f498172348f7ba9dceb169305b74c7eca7a38d
type commit
tag v2.35.6
tagger Junio C Hamano <gitster@pobox.com> 1670933877 +0900
Git 2.35.6
error: no signature found
$ git tag -v v2.36.4
object ad949b24f8d6ee4767c07794a2f01ada91b46b74
type commit
tag v2.36.4
tagger Junio C Hamano <gitster@pobox.com> 1670933982 +0900
Git 2.36.4
error: no signature found
$ git tag -v v2.37.5
object e43ac5f23d3231f6433558931296d555592e978a
type commit
tag v2.37.5
tagger Junio C Hamano <gitster@pobox.com> 1670934063 +0900
Git 2.37.5
error: no signature found
$ git tag -v v2.38.3
object 37ed7bf0f13d204ccb6b8e7e2fbcf7b3bf13e25c
type commit
tag v2.38.3
tagger Junio C Hamano <gitster@pobox.com> 1670934268 +0900
Git 2.38.3
error: no signature found
$ git tag -v v2.39.1
object 01443f01b7c6a3c6ef03268b649b119027743115
type commit
tag v2.39.1
tagger Junio C Hamano <gitster@pobox.com> 1670934343 +0900
Git 2.39.1
error: no signature found
$
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH v5 09/10] http: read HTTP WWW-Authenticate response headers
From: Matthew John Cheetham @ 2023-01-17 21:51 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason,
Matthew John Cheetham via GitGitGadget
Cc: git, Derrick Stolee, Lessley Dennington, M Hickford,
Jeff Hostetler, Glen Choo, Victoria Dye, Matthew John Cheetham
In-Reply-To: <230112.86wn5s2l5r.gmgdl@evledraar.gmail.com>
On 2023-01-12 00:41, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Jan 11 2023, Matthew John Cheetham via GitGitGadget wrote:
>
>> From: Matthew John Cheetham <mjcheetham@outlook.com>
>> [...]
>> + } else if (buf.len) {
>> + const char *prev = values->v[values->nr - 1];
>> + struct strbuf append = STRBUF_INIT;
>> + strbuf_addstr(&append, prev);
>> +
>> + /* Join two non-empty values with a single space. */
>> + if (append.len)
>> + strbuf_addch(&append, ' ');
>> +
>> + strbuf_addbuf(&append, &buf);
>> +
>> + strvec_pop(values);
>> + strvec_push_nodup(values, strbuf_detach(&append, NULL));
>> + }
>> +
>
> I've written something like the strvec_push_nodup() patch that preceded
> this myself for similar reasons, and as recently noted in [1] I think
> such a thing (although I implemented a different interface) might be
> useful in general.
A fair point, and reading [1] I see there's some concerns about making the
strvec interface more complicated w.r.t. ownership vs saving a `xstrdup`.
In light of this, I'll drop the commit to add `strvec_push_nodup`.
> But this really doesn't seem like a good justification for adding this
> new API. Let's instead do:
>
> } else if (buf.len) {
> const char *prev = values->v[values->nr - 1];
> /* Join two non-empty values with a single space. */
> const char *const sp = *prev ? " " : ""
>
> strvec_pop(values);
> strvec_pushf(values, "%s%s%s", prev, sp, buf.buf);
> }
>
> There may be cases where a public strvec_push_nodup() simplifies things,
> but this doesn't seem like such a case, just use strvec_pushf() directly
> instead, and skip the strbuf & strbuf_detach().
>
> I haven't compiled/tested the above, so there may e.g. be a typo in
> there. But I think the general concept should work in this case.
>
> 1. https://lore.kernel.org/git/RFC-cover-0.5-00000000000-20221215T090226Z-avarab@gmail.com/
There's a bug in your suggestion. We're `strvec_pop`-ing from the array
which also frees the previous value that we want to use to append to in
the next call to `strvec_pushf`. We need to keep a copy of the previous
header value around.
This should work instead (adding an `xstrdup` and `free`):
} else if (buf.len) {
char *prev = xstrdup(values->v[values->nr - 1]);
/* Join two non-empty values with a single space. */
const char *const sp = *prev ? " " : "";
strvec_pop(values);
strvec_pushf(values, "%s%s%s", prev, sp, buf.buf);
free(prev);
}
Thanks,
Matthew
^ permalink raw reply
* [PATCH v2] fsm-listen-darwin: combine bit operations
From: Rose via GitGitGadget @ 2023-01-17 21:54 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1437.git.git.1673990756466.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
fsm-listen-darwin: combine bit operations
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1437%2FAtariDreams%2Fdarwin-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1437/AtariDreams/darwin-v2
Pull-Request: https://github.com/git/git/pull/1437
Range-diff vs v1:
1: a98654c7507 ! 1: 9943d52654f fsm-listen-daarwin: combine bit operations
@@ Metadata
Author: Seija Kijin <doremylover123@gmail.com>
## Commit message ##
- fsm-listen-daarwin: combine bit operations
+ fsm-listen-darwin: combine bit operations
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
compat/fsmonitor/fsm-listen-darwin.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
index 97a55a6f0a4..fccdd21d858 100644
--- a/compat/fsmonitor/fsm-listen-darwin.c
+++ b/compat/fsmonitor/fsm-listen-darwin.c
@@ -129,9 +129,9 @@ static int ef_is_root_renamed(const FSEventStreamEventFlags ef)
static int ef_is_dropped(const FSEventStreamEventFlags ef)
{
- return (ef & kFSEventStreamEventFlagMustScanSubDirs ||
- ef & kFSEventStreamEventFlagKernelDropped ||
- ef & kFSEventStreamEventFlagUserDropped);
+ return (ef & (kFSEventStreamEventFlagMustScanSubDirs |
+ kFSEventStreamEventFlagKernelDropped |
+ kFSEventStreamEventFlagUserDropped));
}
/*
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* [PATCH] git: replace strbuf_addstr with strbuf_addch for all strings of length 2
From: Rose via GitGitGadget @ 2023-01-17 21:54 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
From: Seija Kijin <doremylover123@gmail.com>
This helps reduce overhead of calculating the length
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
git: replace strbuf_addstr with strbuf_addch for all strings of length 2
This helps reduce overhead of calculating the length
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1436%2FAtariDreams%2Fstrbuf-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1436/AtariDreams/strbuf-v1
Pull-Request: https://github.com/git/git/pull/1436
bisect.c | 2 +-
builtin/am.c | 6 ++----
builtin/blame.c | 8 +++-----
builtin/ls-tree.c | 2 +-
diff.c | 2 +-
log-tree.c | 2 +-
merge-ort.c | 3 +--
path.c | 2 +-
protocol-caps.c | 2 +-
reftable/readwrite_test.c | 2 +-
reftable/refname.c | 2 +-
reftable/stack.c | 12 ++++++------
reftable/stack_test.c | 2 +-
reftable/writer.c | 2 +-
sequencer.c | 2 +-
setup.c | 2 +-
trace2/tr2_tgt_normal.c | 2 +-
17 files changed, 25 insertions(+), 30 deletions(-)
diff --git a/bisect.c b/bisect.c
index ec7487e6836..ea534ad3777 100644
--- a/bisect.c
+++ b/bisect.c
@@ -443,7 +443,7 @@ static int register_ref(const char *refname, const struct object_id *oid,
{
struct strbuf good_prefix = STRBUF_INIT;
strbuf_addstr(&good_prefix, term_good);
- strbuf_addstr(&good_prefix, "-");
+ strbuf_addch(&good_prefix, '-');
if (!strcmp(refname, term_bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
diff --git a/builtin/am.c b/builtin/am.c
index 7e88d2426d7..c96886e0433 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -329,13 +329,11 @@ static void write_author_script(const struct am_state *state)
strbuf_addstr(&sb, "GIT_AUTHOR_NAME=");
sq_quote_buf(&sb, state->author_name);
- strbuf_addch(&sb, '\n');
- strbuf_addstr(&sb, "GIT_AUTHOR_EMAIL=");
+ strbuf_addstr(&sb, "\nGIT_AUTHOR_EMAIL=");
sq_quote_buf(&sb, state->author_email);
- strbuf_addch(&sb, '\n');
- strbuf_addstr(&sb, "GIT_AUTHOR_DATE=");
+ strbuf_addstr(&sb, "\nGIT_AUTHOR_DATE=");
sq_quote_buf(&sb, state->author_date);
strbuf_addch(&sb, '\n');
diff --git a/builtin/blame.c b/builtin/blame.c
index 71f925e456c..3ab4cc0a56b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -143,11 +143,9 @@ static void get_ac_line(const char *inbuf, const char *what,
if (split_ident_line(&ident, tmp, len)) {
error_out:
- /* Ugh */
- tmp = "(unknown)";
- strbuf_addstr(name, tmp);
- strbuf_addstr(mail, tmp);
- strbuf_addstr(tz, tmp);
+ strbuf_addstr(name, "(unknown)");
+ strbuf_addstr(mail, "(unknown)");
+ strbuf_addstr(tz, "(unknown)");
*time = 0;
return;
}
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index c3ea09281af..73b755029ee 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -60,7 +60,7 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
} else if (padded) {
strbuf_addf(line, "%7s", "-");
} else {
- strbuf_addstr(line, "-");
+ strbuf_addch(line, '-');
}
}
diff --git a/diff.c b/diff.c
index 329eebf16a0..b379660c42b 100644
--- a/diff.c
+++ b/diff.c
@@ -1702,7 +1702,7 @@ static void add_line_count(struct strbuf *out, int count)
strbuf_addstr(out, "0,0");
break;
case 1:
- strbuf_addstr(out, "1");
+ strbuf_addch(out, '1');
break;
default:
strbuf_addf(out, "1,%d", count);
diff --git a/log-tree.c b/log-tree.c
index 1dd5fcbf7be..23f2a62c5ac 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -388,7 +388,7 @@ void fmt_output_subject(struct strbuf *filename,
strbuf_addf(&temp, "v%s", info->reroll_count);
format_sanitized_subject(filename, temp.buf, temp.len);
- strbuf_addstr(filename, "-");
+ strbuf_addch(filename, '-');
strbuf_release(&temp);
}
strbuf_addf(filename, "%04d-%s", nr, subject);
diff --git a/merge-ort.c b/merge-ort.c
index d1611ca400a..3132ac22aba 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -801,8 +801,7 @@ static void path_msg(struct merge_options *opt,
va_start(ap, fmt);
if (opt->priv->call_depth) {
- strbuf_addchars(dest, ' ', 2);
- strbuf_addstr(dest, "From inner merge:");
+ strbuf_addstr(dest, " From inner merge:");
strbuf_addchars(dest, ' ', opt->priv->call_depth * 2);
}
strbuf_vaddf(dest, fmt, ap);
diff --git a/path.c b/path.c
index 492e17ad121..05d3b6d9059 100644
--- a/path.c
+++ b/path.c
@@ -1082,7 +1082,7 @@ const char *remove_leading_path(const char *in, const char *prefix)
strbuf_reset(&buf);
if (!in[j])
- strbuf_addstr(&buf, ".");
+ strbuf_addch(&buf, '.');
else
strbuf_addstr(&buf, in + j);
return buf.buf;
diff --git a/protocol-caps.c b/protocol-caps.c
index bbde91810ac..80ec75e1131 100644
--- a/protocol-caps.c
+++ b/protocol-caps.c
@@ -63,7 +63,7 @@ static void send_info(struct repository *r, struct packet_writer *writer,
if (info->size) {
if (oid_object_info(r, &oid, &object_size) < 0) {
- strbuf_addstr(&send_buffer, " ");
+ strbuf_addch(&send_buffer, ' ');
} else {
strbuf_addf(&send_buffer, " %lu", object_size);
}
diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index 469ab79a5ad..ed0f6058ba9 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -483,7 +483,7 @@ static void test_table_read_write_seek(int index, int hash_id)
}
strbuf_addstr(&pastLast, names[N - 1]);
- strbuf_addstr(&pastLast, "/");
+ strbuf_addch(&pastLast, '/');
err = reftable_reader_seek_ref(&rd, &it, pastLast.buf);
if (err == 0) {
diff --git a/reftable/refname.c b/reftable/refname.c
index 95734969324..b6d5b76a8fe 100644
--- a/reftable/refname.c
+++ b/reftable/refname.c
@@ -179,7 +179,7 @@ int modification_validate(struct modification *mod)
goto done;
strbuf_reset(&slashed);
strbuf_addstr(&slashed, mod->add[i]);
- strbuf_addstr(&slashed, "/");
+ strbuf_addch(&slashed, '/');
err = modification_has_ref_with_prefix(mod, slashed.buf);
if (err == 0) {
diff --git a/reftable/stack.c b/reftable/stack.c
index ddbdf1b9c8b..479658e428d 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -35,7 +35,7 @@ static void stack_filename(struct strbuf *dest, struct reftable_stack *st,
{
strbuf_reset(dest);
strbuf_addstr(dest, st->reftable_dir);
- strbuf_addstr(dest, "/");
+ strbuf_addch(dest, '/');
strbuf_addstr(dest, name);
}
@@ -547,11 +547,11 @@ int reftable_addition_commit(struct reftable_addition *add)
for (i = 0; i < add->stack->merged->stack_len; i++) {
strbuf_addstr(&table_list, add->stack->readers[i]->name);
- strbuf_addstr(&table_list, "\n");
+ strbuf_addch(&table_list, '\n');
}
for (i = 0; i < add->new_tables_len; i++) {
strbuf_addstr(&table_list, add->new_tables[i]);
- strbuf_addstr(&table_list, "\n");
+ strbuf_addch(&table_list, '\n');
}
err = write(add->lock_file_fd, table_list.buf, table_list.len);
@@ -1013,15 +1013,15 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last,
for (i = 0; i < first; i++) {
strbuf_addstr(&ref_list_contents, st->readers[i]->name);
- strbuf_addstr(&ref_list_contents, "\n");
+ strbuf_addch(&ref_list_contents, '\n');
}
if (!is_empty_table) {
strbuf_addbuf(&ref_list_contents, &new_table_name);
- strbuf_addstr(&ref_list_contents, "\n");
+ strbuf_addch(&ref_list_contents, '\n');
}
for (i = last + 1; i < st->merged->stack_len; i++) {
strbuf_addstr(&ref_list_contents, st->readers[i]->name);
- strbuf_addstr(&ref_list_contents, "\n");
+ strbuf_addch(&ref_list_contents, '\n');
}
err = write(lock_file_fd, ref_list_contents.buf, ref_list_contents.len);
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index d0b717510fa..4fcaefd3ddb 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -181,7 +181,7 @@ static void test_reftable_stack_add_one(void)
strbuf_reset(&scratch);
strbuf_addstr(&scratch, dir);
- strbuf_addstr(&scratch, "/");
+ strbuf_addch(&scratch, '/');
/* do not try at home; not an external API for reftable. */
strbuf_addstr(&scratch, st->readers[0]->name);
err = stat(scratch.buf, &stat_result);
diff --git a/reftable/writer.c b/reftable/writer.c
index 2e322a5683d..61d6f3229f3 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -351,7 +351,7 @@ int reftable_writer_add_log(struct reftable_writer *w,
err = REFTABLE_API_ERROR;
goto done;
}
- strbuf_addstr(&cleaned_message, "\n");
+ strbuf_addch(&cleaned_message, '\n');
log->value.update.message = cleaned_message.buf;
}
diff --git a/sequencer.c b/sequencer.c
index bcb662e23be..27f41a027d2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2233,7 +2233,7 @@ static int do_pick_commit(struct repository *r,
} else {
strbuf_addstr(&msgbuf, "Revert \"");
strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"");
+ strbuf_addch(&msgbuf, '\"');
}
strbuf_addstr(&msgbuf, "\n\nThis reverts commit ");
refer_to_commit(opts, &msgbuf, commit);
diff --git a/setup.c b/setup.c
index cefd5f63c46..865ed8fbe98 100644
--- a/setup.c
+++ b/setup.c
@@ -1349,7 +1349,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
return GIT_DIR_DISALLOWED_BARE;
if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
return GIT_DIR_INVALID_OWNERSHIP;
- strbuf_addstr(gitdir, ".");
+ strbuf_addch(gitdir, '.');
return GIT_DIR_BARE;
}
diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index fbbef68dfc0..e99ee58ca38 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -224,7 +224,7 @@ static void fn_child_start_fl(const char *file, int line,
if (cmd->dir) {
strbuf_addstr(&buf_payload, " cd ");
sq_quote_buf_pretty(&buf_payload, cmd->dir);
- strbuf_addstr(&buf_payload, ";");
+ strbuf_addch(&buf_payload, ';');
}
/*
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* Join us for Review Club! (possibly from other timezones)
From: Glen Choo @ 2023-01-17 22:07 UTC (permalink / raw)
To: git, Ævar Arnfjörð Bjarmason; +Cc: Calvin Wan
Hi everyone, and happy belated new year!
Review Club is happening this Wednesday at 14:00 Pacific time (UTC-8).
You can find more info at [1] and on gitcal [2]. We run a session every
other week, and you can find the full schedule on gitcal.
This week, we'll be discussing Ævar Arnfjörð Bjarmason's improvements to
the config API. [3]. Let me know if you're interested and would like
to join (off-list is fine), and I'll send you an invite :) If you're
interested but can't make that time, let me know too!
As an aside, I'll be working from a different timezone for ~1 month
starting next week. I'm considering doing some Europe/APAC-friendly
sessions if there are folks who would love to join but can't make the
usual time. Do let me know on this thread if you're interested! The
normal Review Club sessions will not be affected; many thanks to Calvin
for offering to conduct the sessions in my absence :)
See you there!
[1] https://lore.kernel.org/git/Yfl1%2FZN%2FtaYwfGD0@google.com/
[2] http://tinyurl.com/gitcal
[3] https://lore.kernel.org/git/cover-v3-0.9-00000000000-20221125T093158Z-avarab@gmail.com/
^ permalink raw reply
* [BUG?] 'git rebase --update-refs --whitespace=fix' incompatible, but not enforced
From: Philippe Blain @ 2023-01-17 22:02 UTC (permalink / raw)
To: Git mailing list, Derrick Stolee, Elijah Newren
Hi Elijah and Stolee,
First, Stolee, thanks a lot for adding '--update-refs', it is very useful (I've
added it to my config so I don't forget to use it).
I recently learned that 'git rebase --whitespace=fix' exists, which is also
great but since it uses the apply backend, it can't be used with --update-refs.
I understand this, and the fact that adding '--whitespace=fix' to the merge
backend would be a big task; this is not what this email is about.
I think there is a bug in that the code does not enforce the fact that
both options can't be used together. Whenever '--whitespace' or '-C' are used,
the code defaults to the apply backend:
```builtin/rebase +1502
if (options.git_am_opts.nr || options.type == REBASE_APPLY) {
/* all am options except -q are compatible only with --apply */
for (i = options.git_am_opts.nr - 1; i >= 0; i--)
if (strcmp(options.git_am_opts.v[i], "-q"))
break;
if (i >= 0) {
if (is_merge(&options))
die(_("apply options and merge options "
"cannot be used together"));
else
options.type = REBASE_APPLY;
}
```
but 'is_merge' only checks if 'opts->type == REBASE_MERGE', so the check only
works if --merge was given explicitely, but not when none of '--merge' or '--apply'
were given (and so the default "merge" backend is used).
I would have expected the code to die telling me --update-refs and --whitespace
are incompatible. But instead it defaulted to --apply, and (of course) did not
update the refs in my history (which I found out later).
Thanks,
Philippe.
^ permalink raw reply
* [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook
From: Strawbridge, Michael @ 2023-01-17 21:58 UTC (permalink / raw)
To: avarab@gmail.com
Cc: Tuikov, Luben, Strawbridge, Michael, git@vger.kernel.org,
gitster@pobox.com, Strawbridge, Michael
In-Reply-To: <230117.86sfg9xp98.gmgdl@evledraar.gmail.com>
On Tue, Jan 17 2023, Ævar Arnfjörð Bjarmason wrote:
>On Tue, Jan 10 2023, Strawbridge, Michael wrote:
>
>> To allow further flexibility in the git hook, the SMTP header
>> information of the email that git-send-email intends to send, is now
>> passed as a 2nd argument to the sendemail-validate hook.
>>
>> As an example, this can be useful for acting upon keywords in the
>> subject or specific email addresses.
>
>Okey, but...
>
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index a16e62bc8c..2b5c6640cc 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -583,10 +583,19 @@ processed by rebase.
>> sendemail-validate
>> ~~~~~~~~~~~~~~~~~~
>>
>> -This hook is invoked by linkgit:git-send-email[1]. It takes a single parameter,
>> -the name of the file that holds the e-mail to be sent. Exiting with a
>> -non-zero status causes `git send-email` to abort before sending any
>> -e-mails.
>> +This hook is invoked by linkgit:git-send-email[1].
>> +
>> +It takes these command line arguments:
>> +1. the name of the file that holds the e-mail to be sent.
>> +2. the name of the file that holds the SMTP headers to be used.
>> +
>> +The hook doesn't need to support multiple header names (for example only Cc
>> +is passed). However, it does need to understand that lines beginning with
>> +whitespace belong to the previous header. The header information follows
>> +the same format as the confirmation given at the end of send-email.
>> +
>> +Exiting with a non-zero status causes `git send-email` to abort
>> +before sending any e-mails.
>>
>> fsmonitor-watchman
>> ~~~~~~~~~~~~~~~~~~
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 810dd1f1ce..b2adca515e 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -787,14 +787,6 @@ sub is_format_patch_arg {
>>
>> @files = handle_backup_files(@files);
>>
>> -if ($validate) {
>> - foreach my $f (@files) {
>> - unless (-p $f) {
>> - validate_patch($f, $target_xfer_encoding);
>> - }
>> - }
>> -}
>> -
>> if (@files) {
>> unless ($quiet) {
>> print $_,"\n" for (@files);
>> @@ -1738,6 +1730,16 @@ sub send_message {
>> return 1;
>> }
>>
>> +if ($validate) {
>> + foreach my $f (@files) {
>> + unless (-p $f) {
>> + pre_process_file($f, 1);
>> +
>> + validate_patch($f, $target_xfer_encoding);
>> + }
>> + }
>> +}
>
>...here we have the seemingly unrelated change of first doing the
>validation before this, and if we pass it we'll print the names of the
>files we're sending unless --quiet.
>
>Now we'll do it the other way around, maybe that's good, or maybe not,
>but your updated docs don't say.
>
>Also (and I didn't look at this all that carefully), why are you moving
>the control logic to between the later function declarations?
>
>Perl isn't a language where you need to arrange your source in that way
>(unless a bareword is involved, or if this happens at BEGIN time or
>whatever). The current structure is:
>
> <use & imports>
> <basic setup (getopts etc)>
> <main logic>
> <helper function>
>
>Here you're moving part of the main logic to in-between two helper
>function, why?
>
In general I understand your point. However, I found in
git-send-email that code outside of functions is sprinkled in between
function declarations in several areas. Lines 790-797 and 1011-1058 are
examples of some of what I mean. Also, the original process_file call is
actually out of place (roughly line 2011-2015). Unfortunately some of this
sprinkled code has an impact on the header variables and so I needed to
move the validation code lower, closer to the process_file section but
before the intialization of in_reply_to, references, and message_num.
I was attempting to reduce the number of code changes.
You do bring up a good point about printing of the names of files
before validation causing a difference in output. I can attempt to move
some more of this code as well, if the change of output is deemed
important enough.
>> $in_reply_to = $initial_in_reply_to;
>> $references = $initial_in_reply_to || '';
>> $message_num = 0;
>> @@ -2101,11 +2103,20 @@ sub validate_patch {
>> chdir($repo->wc_path() or $repo->repo_path())
>> or die("chdir: $!");
>> local $ENV{"GIT_DIR"} = $repo->repo_path();
>> +
>> + my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
>> +
>> + require File::Temp;
>> + my ($header_filehandle, $header_filename) = File::Temp::tempfile(
>> + ".gitsendemail.header.XXXXXX", DIR => $repo->repo_path());
>> + print $header_filehandle $header;
>> +
>> my @cmd = ("git", "hook", "run", "--ignore-missing",
>> $hook_name, "--");
>> - my @cmd_msg = (@cmd, "<patch>");
>> - my @cmd_run = (@cmd, $target);
>> + my @cmd_msg = (@cmd, "<patch>", "<header>");
>> + my @cmd_run = (@cmd, $target, $header_filename);
>> $hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
>> + unlink($header_filehandle);
>> chdir($cwd_save) or die("chdir: $!");
>
>I know "git hook run" doesn't support input on stdin yet, but isn't this
>just working around it not supporting that? That seems like a much
>better & natural interface than what we're doing here.
>
>I have out-of-tree patches for that (or rather, a re-roll of Emily's
>patches to do that), if that landed in-tree could this use that
>interface, do you think?
>
>I'd rather that we didn't forever codify a strange interface here due to
>a temporary limitation in "git hook" and the hook API...
I was trying to follow the convention that the original hook was using.
I'm not against changing this if the out of tree patches you speak of
are going to be rolled in soon. However, I'd prefer not to delay this
patch if these other patches are far off. Thanks.
--
2.34.1
^ permalink raw reply
* [PATCH v7] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 20:33 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v6.git.git.1673986937738.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v7
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v6:
1: 6b471a867d5 ! 1: 23094afb2e6 curl: resolve deprecated curl declarations
@@ http.c: void setup_curl_trace(CURL *handle)
+ }
+ if (is_transport_allowed("https", from_user)) {
-+ if (proto_buf->len)
-+ strbuf_addch(proto_buf, ',');
-+ strbuf_addstr(proto_buf, "https");
++ strbuf_addstr(proto_buf, proto_buf->len ? "https" : ",https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
-+ if (proto_buf->len)
-+ strbuf_addch(proto_buf, ',');
-+ strbuf_addstr(proto_buf, "ftp");
++ strbuf_addstr(proto_buf, proto_buf->len ? "ftp" : ",ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
-+ if (proto_buf->len)
-+ strbuf_addch(proto_buf, ',');
-+ strbuf_addstr(proto_buf, "ftps");
++ strbuf_addstr(proto_buf, proto_buf->len ? "ftps" : ",ftps");
+ }
+}
+#else
INSTALL | 2 +-
git-curl-compat.h | 8 +++++++
http-push.c | 6 ++---
http.c | 57 +++++++++++++++++++++++++++++++++++++----------
http.h | 2 +-
remote-curl.c | 31 +++++++++++++-------------
6 files changed, 73 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..6dfbae96ac6 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +763,26 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
+ if (is_transport_allowed("http", from_user)) {
+ strbuf_addstr(proto_buf, "http");
+ }
+ if (is_transport_allowed("https", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? "https" : ",https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? "ftp" : ",ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
+ strbuf_addstr(proto_buf, proto_buf->len ? "ftps" : ",ftps");
+ }
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +798,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -923,10 +941,25 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ {
+ struct strbuf buf = STRBUF_INIT;
+
+ get_curl_allowed_protocols(&buf, 0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+ strbuf_reset(&buf);
+
+ get_curl_allowed_protocols(&buf, -1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+ strbuf_release(&buf);
+ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 20:19 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v4.git.git.1673986152672.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v5
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v4:
1: ebe36ad23fa ! 1: c8628c53d22 curl: resolve deprecated curl declarations
@@ http.c: void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+static void get_curl_allowed_protocols(char* protocol_buff, int from_user)
++static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
+ unsigned int i = 0;
+
+ if (is_transport_allowed("http", from_user)) {
-+ protocol_buff[i++] = 'h';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 'p';
++ strbuf_addstr(proto_buf, "http");
+ }
+
+ if (is_transport_allowed("https", from_user)) {
-+ if (i != 0) {
-+ protocol_buff[i++] = ',';
-+ }
-+
-+ protocol_buff[i++] = 'h';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 'p';
-+ protocol_buff[i++] = 's';
++ if (proto_buf->len)
++ strbuf_addch(proto_buf, ',');
++ strbuf_addstr(proto_buf, "https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
-+ if (i != 0) {
-+ protocol_buff[i++] = ',';
-+ }
-+
-+ protocol_buff[i++] = 'f';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 'p';
++ if (proto_buf->len)
++ strbuf_addch(proto_buf, ',');
++ strbuf_addstr(proto_buf, "ftp");
+ }
-+ if (is_transport_allowed("ftps", from_user)) {
-+ if (i != 0) {
-+ protocol_buff[i++] = ',';
-+ }
+
-+ protocol_buff[i++] = 'f';
-+ protocol_buff[i++] = 't';
-+ protocol_buff[i++] = 'p';
-+ protocol_buff[i++] = 's';
++ if (is_transport_allowed("ftps", from_user)) {
++ if (proto_buf->len)
++ strbuf_addch(proto_buf, ',');
++ strbuf_addstr(proto_buf, "ftps");
+ }
-
-+ protocol_buff[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
@@ http.c: static long get_curl_allowed_protocols(int from_user)
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
-@@ http.c: static int get_curl_http_version_opt(const char *version_string, long *opt)
-
- static CURL *get_curl_handle(void)
- {
-+
-+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+char protocol_buff[20];
-+#endif
-+
- CURL *result = curl_easy_init();
-
- if (!result)
@@ http.c: static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+ get_curl_allowed_protocols(protocol_buff, 0);
-+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, protocol_buff);
-+ get_curl_allowed_protocols(protocol_buff, -1);
-+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol_buff);
++ {
++ struct strbuf buf = STRBUF_INIT;
++
++ get_curl_allowed_protocols(&buf, 0);
++ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
++ strbuf_reset(&buf);
++
++ get_curl_allowed_protocols(&buf, -1);
++ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
++ strbuf_release(&buf);
++ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
INSTALL | 2 +-
git-curl-compat.h | 8 ++++++
http-push.c | 6 ++---
http.c | 65 ++++++++++++++++++++++++++++++++++++++---------
http.h | 2 +-
remote-curl.c | 31 +++++++++++-----------
6 files changed, 81 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..bc343656d5d 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +763,34 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
+ unsigned int i = 0;
+
+ if (is_transport_allowed("http", from_user)) {
+ strbuf_addstr(proto_buf, "http");
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "https");
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "ftps");
+ }
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +806,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -923,10 +949,25 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ {
+ struct strbuf buf = STRBUF_INIT;
+
+ get_curl_allowed_protocols(&buf, 0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+ strbuf_reset(&buf);
+
+ get_curl_allowed_protocols(&buf, -1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+ strbuf_release(&buf);
+ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 20:22 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v5.git.git.1673986764553.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v6
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v5:
1: c8628c53d22 ! 1: 6b471a867d5 curl: resolve deprecated curl declarations
@@ http.c: void setup_curl_trace(CURL *handle)
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
-+ unsigned int i = 0;
-+
+ if (is_transport_allowed("http", from_user)) {
+ strbuf_addstr(proto_buf, "http");
+ }
-+
+
+ if (is_transport_allowed("https", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "https");
+ }
-
++
+ if (is_transport_allowed("ftp", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
INSTALL | 2 +-
git-curl-compat.h | 8 ++++++
http-push.c | 6 ++---
http.c | 63 ++++++++++++++++++++++++++++++++++++++---------
http.h | 2 +-
remote-curl.c | 31 +++++++++++------------
6 files changed, 79 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..74aa6edd1fd 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +763,32 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(struct strbuf *proto_buf, int from_user)
+{
+ if (is_transport_allowed("http", from_user)) {
+ strbuf_addstr(proto_buf, "http");
+ }
+ if (is_transport_allowed("https", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "https");
+ }
+
+ if (is_transport_allowed("ftp", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "ftp");
+ }
+
+ if (is_transport_allowed("ftps", from_user)) {
+ if (proto_buf->len)
+ strbuf_addch(proto_buf, ',');
+ strbuf_addstr(proto_buf, "ftps");
+ }
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +804,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -923,10 +947,25 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ {
+ struct strbuf buf = STRBUF_INIT;
+
+ get_curl_allowed_protocols(&buf, 0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, buf.buf);
+ strbuf_reset(&buf);
+
+ get_curl_allowed_protocols(&buf, -1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, buf.buf);
+ strbuf_release(&buf);
+ }
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 20:09 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v3.git.git.1673985725868.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v4
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v3:
1: 14ca56fe608 ! 1: ebe36ad23fa curl: resolve deprecated curl declarations
@@ http-push.c: static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
## http.c ##
-@@ http.c: static const char *http_proxy_ssl_ca_info;
- static struct credential proxy_cert_auth = CREDENTIAL_INIT;
- static int proxy_ssl_cert_password_required;
-
-+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+static char protocol_list[20];
-+#endif
-+
- static struct {
- const char *name;
- long curlauth_param;
@@ http.c: size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
@@ http.c: size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffe
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
--
++ struct buffer *buffer = userp;
+
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
@@ http.c: size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffe
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-+ struct buffer *buffer = userp;
-
+-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
@@ http.c: void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+static void get_curl_allowed_protocols(int from_user)
++static void get_curl_allowed_protocols(char* protocol_buff, int from_user)
+{
+ unsigned int i = 0;
-
++
+ if (is_transport_allowed("http", from_user)) {
-+ protocol_list[i++] = 'h';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 'p';
++ protocol_buff[i++] = 'h';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 'p';
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (i != 0) {
-+ protocol_list[i++] = ',';
++ protocol_buff[i++] = ',';
+ }
+
-+ protocol_list[i++] = 'h';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 'p';
-+ protocol_list[i++] = 's';
++ protocol_buff[i++] = 'h';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 'p';
++ protocol_buff[i++] = 's';
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (i != 0) {
-+ protocol_list[i++] = ',';
++ protocol_buff[i++] = ',';
+ }
+
-+ protocol_list[i++] = 'f';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 'p';
++ protocol_buff[i++] = 'f';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 'p';
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ if (i != 0) {
-+ protocol_list[i++] = ',';
++ protocol_buff[i++] = ',';
+ }
+
-+ protocol_list[i++] = 'f';
-+ protocol_list[i++] = 't';
-+ protocol_list[i++] = 'p';
-+ protocol_list[i++] = 's';
++ protocol_buff[i++] = 'f';
++ protocol_buff[i++] = 't';
++ protocol_buff[i++] = 'p';
++ protocol_buff[i++] = 's';
+ }
-+
-+ protocol_list[i] = '\0';
+
++ protocol_buff[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
@@ http.c: static long get_curl_allowed_protocols(int from_user)
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
+@@ http.c: static int get_curl_http_version_opt(const char *version_string, long *opt)
+
+ static CURL *get_curl_handle(void)
+ {
++
++#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
++char protocol_buff[20];
++#endif
++
+ CURL *result = curl_easy_init();
+
+ if (!result)
@@ http.c: static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+ get_curl_allowed_protocols(0);
-+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, redir_protocol);
-+ get_curl_allowed_protocols(-1);
-+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol);
++ get_curl_allowed_protocols(protocol_buff, 0);
++ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, protocol_buff);
++ get_curl_allowed_protocols(protocol_buff, -1);
++ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol_buff);
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
INSTALL | 2 +-
git-curl-compat.h | 8 +++++
http-push.c | 6 ++--
http.c | 81 ++++++++++++++++++++++++++++++++++++++++-------
http.h | 2 +-
remote-curl.c | 31 +++++++++---------
6 files changed, 97 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..e51a9a61a20 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +763,52 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(char* protocol_buff, int from_user)
+{
+ unsigned int i = 0;
+
+ if (is_transport_allowed("http", from_user)) {
+ protocol_buff[i++] = 'h';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 'p';
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (i != 0) {
+ protocol_buff[i++] = ',';
+ }
+
+ protocol_buff[i++] = 'h';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 'p';
+ protocol_buff[i++] = 's';
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (i != 0) {
+ protocol_buff[i++] = ',';
+ }
+
+ protocol_buff[i++] = 'f';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 'p';
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ if (i != 0) {
+ protocol_buff[i++] = ',';
+ }
+
+ protocol_buff[i++] = 'f';
+ protocol_buff[i++] = 't';
+ protocol_buff[i++] = 'p';
+ protocol_buff[i++] = 's';
+ }
+ protocol_buff[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +824,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -809,6 +853,11 @@ static int get_curl_http_version_opt(const char *version_string, long *opt)
static CURL *get_curl_handle(void)
{
+
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+char protocol_buff[20];
+#endif
+
CURL *result = curl_easy_init();
if (!result)
@@ -923,10 +972,18 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ get_curl_allowed_protocols(protocol_buff, 0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, protocol_buff);
+ get_curl_allowed_protocols(protocol_buff, -1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol_buff);
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 20:02 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.v2.git.git.1673984591615.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v3
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v2:
1: c40fb2de13d ! 1: 14ca56fe608 curl: resolve deprecated curl declarations
@@ Commit message
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
+ ## INSTALL ##
+@@ INSTALL: Issues of note:
+ not need that functionality, use NO_CURL to build without
+ it.
+
+- Git requires version "7.19.4" or later of "libcurl" to build
++ Git requires version "7.19.5" or later of "libcurl" to build
+ without NO_CURL. This version requirement may be bumped in
+ the future.
+
+
## git-curl-compat.h ##
@@
#endif
@@ http-push.c: static void curl_setup_http(CURL *curl, const char *url,
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
-+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, ioctl_buffer);
++ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
## http.c ##
+@@ http.c: static const char *http_proxy_ssl_ca_info;
+ static struct credential proxy_cert_auth = CREDENTIAL_INIT;
+ static int proxy_ssl_cert_password_required;
+
++#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
++static char protocol_list[20];
++#endif
++
+ static struct {
+ const char *name;
+ long curlauth_param;
@@ http.c: size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
-+int ioctl_buffer(void *userp, curl_off_t offset, int origin)
++int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
-+ struct buffer *buffer = userp;
-
+-
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
@@ http.c: size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffe
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
--
++ struct buffer *buffer = userp;
+
- default:
- return CURLIOE_UNKNOWNCMD;
-- }
-+ buffer->posn = 0;
++ if (origin != SEEK_SET)
++ BUG("seek_buffer only handles SEEK_SET");
++ if (offset < 0 || offset >= buffer->buf.len) {
++ error("curl seek would be outside of buffer");
++ return CURL_SEEKFUNC_FAIL;
+ }
++
++ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
@@ http.c: void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+static void get_curl_allowed_protocols(int from_user, char *protocol)
++static void get_curl_allowed_protocols(int from_user)
+{
+ unsigned int i = 0;
-+
+
+ if (is_transport_allowed("http", from_user)) {
-+ protocol[i++] = 'h';
-+ protocol[i++] = 't';
-+ protocol[i++] = 't';
-+ protocol[i++] = 'p';
++ protocol_list[i++] = 'h';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 'p';
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (i != 0) {
-+ protocol[i++] = ',';
++ protocol_list[i++] = ',';
+ }
+
-+ protocol[i++] = 'h';
-+ protocol[i++] = 't';
-+ protocol[i++] = 't';
-+ protocol[i++] = 'p';
-+ protocol[i++] = 's';
++ protocol_list[i++] = 'h';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 'p';
++ protocol_list[i++] = 's';
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (i != 0) {
-+ protocol[i++] = ',';
++ protocol_list[i++] = ',';
+ }
-
-+ protocol[i++] = 'f';
-+ protocol[i++] = 't';
-+ protocol[i++] = 'p';
++
++ protocol_list[i++] = 'f';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 'p';
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ if (i != 0) {
-+ protocol[i++] = ',';
++ protocol_list[i++] = ',';
+ }
+
-+ protocol[i++] = 'f';
-+ protocol[i++] = 't';
-+ protocol[i++] = 'p';
-+ protocol[i++] = 's';
++ protocol_list[i++] = 'f';
++ protocol_list[i++] = 't';
++ protocol_list[i++] = 'p';
++ protocol_list[i++] = 's';
+ }
+
-+ protocol[i] = '\0';
++ protocol_list[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
@@ http.c: static long get_curl_allowed_protocols(int from_user)
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
-@@ http.c: static int get_curl_http_version_opt(const char *version_string, long *opt)
- static CURL *get_curl_handle(void)
- {
- CURL *result = curl_easy_init();
-+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+ static char protocol[20], redir_protocol[20];
-+#endif
-
- if (!result)
- die("curl_easy_init failed");
@@ http.c: static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
-+ get_curl_allowed_protocols(0, redir_protocol);
++ get_curl_allowed_protocols(0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, redir_protocol);
-+ get_curl_allowed_protocols(-1, protocol);
++ get_curl_allowed_protocols(-1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol);
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
@@ http.h: struct buffer {
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
-+int ioctl_buffer(void *userp, curl_off_t offset, int origin);
++int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
@@ remote-curl.c: static size_t rpc_out(void *ptr, size_t eltsize,
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
-+static int rpc_ioctl(void *userp, curl_off_t offset, int origin)
++static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
@@ remote-curl.c: static size_t rpc_out(void *ptr, size_t eltsize,
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
--
++ if (origin != SEEK_SET)
++ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
+
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
-- }
++ if (rpc->initial_buffer) {
++ if (offset < 0 || offset > rpc->len) {
++ error("curl seek would be outside of rpc buffer");
++ return CURL_SEEKFUNC_FAIL;
+ }
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
-+ if (rpc->initial_buffer) {
-+ rpc->pos = 0;
++ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
@@ remote-curl.c: retry:
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
-+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_ioctl);
++ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
INSTALL | 2 +-
git-curl-compat.h | 8 +++++
http-push.c | 6 ++--
http.c | 80 ++++++++++++++++++++++++++++++++++++++++-------
http.h | 2 +-
remote-curl.c | 31 +++++++++---------
6 files changed, 96 insertions(+), 33 deletions(-)
diff --git a/INSTALL b/INSTALL
index 33447883974..d5694f8c470 100644
--- a/INSTALL
+++ b/INSTALL
@@ -139,7 +139,7 @@ Issues of note:
not need that functionality, use NO_CURL to build without
it.
- Git requires version "7.19.4" or later of "libcurl" to build
+ Git requires version "7.19.5" or later of "libcurl" to build
without NO_CURL. This version requirement may be bumped in
the future.
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..7f71316456c 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..671bfd834f3 100644
--- a/http.c
+++ b/http.c
@@ -76,6 +76,10 @@ static const char *http_proxy_ssl_ca_info;
static struct credential proxy_cert_auth = CREDENTIAL_INIT;
static int proxy_ssl_cert_password_required;
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static char protocol_list[20];
+#endif
+
static struct {
const char *name;
long curlauth_param;
@@ -157,21 +161,19 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int seek_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
-
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
+ struct buffer *buffer = userp;
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (origin != SEEK_SET)
+ BUG("seek_buffer only handles SEEK_SET");
+ if (offset < 0 || offset >= buffer->buf.len) {
+ error("curl seek would be outside of buffer");
+ return CURL_SEEKFUNC_FAIL;
}
+
+ buffer->posn = offset;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +767,52 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(int from_user)
+{
+ unsigned int i = 0;
+ if (is_transport_allowed("http", from_user)) {
+ protocol_list[i++] = 'h';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 'p';
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (i != 0) {
+ protocol_list[i++] = ',';
+ }
+
+ protocol_list[i++] = 'h';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 'p';
+ protocol_list[i++] = 's';
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (i != 0) {
+ protocol_list[i++] = ',';
+ }
+
+ protocol_list[i++] = 'f';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 'p';
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ if (i != 0) {
+ protocol_list[i++] = ',';
+ }
+
+ protocol_list[i++] = 'f';
+ protocol_list[i++] = 't';
+ protocol_list[i++] = 'p';
+ protocol_list[i++] = 's';
+ }
+
+ protocol_list[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +828,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -923,10 +971,18 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ get_curl_allowed_protocols(0);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, redir_protocol);
+ get_curl_allowed_protocols(-1);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol);
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0be9400ef53 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int seek_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..540da2b7989 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,24 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_seek(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
+ if (origin != SEEK_SET)
+ BUG("rpc_seek only handles SEEK_SET, not %d", origin);
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
+ if (rpc->initial_buffer) {
+ if (offset < 0 || offset > rpc->len) {
+ error("curl seek would be outside of rpc buffer");
+ return CURL_SEEKFUNC_FAIL;
}
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ rpc->pos = offset;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +958,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v4 15/19] object-file.c: release the "tag" in check_tag()
From: René Scharfe @ 2023-01-17 19:58 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano, Eric Sunshine
In-Reply-To: <patch-v4-15.19-66c24afb893-20230117T151202Z-avarab@gmail.com>
Am 17.01.23 um 18:11 schrieb Ævar Arnfjörð Bjarmason:
> Fix a memory leak that's been with us ever since c879daa2372 (Make
> hash-object more robust against malformed objects, 2011-02-05). With
> "HASH_FORMAT_CHECK" (used by "hash-object" and "replace") we'll parse
> tags into a throwaway variable on the stack, but weren't freeing the
> "item->tag" we might malloc() when doing so.
>
> The clearing that release_tag_memory() does for us is redundant here,
> but let's use it as-is anyway. It only has one other existing caller,
> which does need the tag to be cleared.
Calling it is better than getting our hands dirty with tag internals
here.
There's similar leak in check_commit() in the same file, but plugging it
would require exporting unparse_commit(). Or perhaps using the heavy
hammer that is release_commit_memory()? Anyway, it doesn't seem simple
to me, so that would be a patch for a separate series.
>
> Mark the tests that now pass in their entirety as passing under
> "SANITIZE=leak", which means we'll test them as part of the
> "linux-leaks" CI job.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> object-file.c | 1 +
> t/t3800-mktag.sh | 1 +
> t/t5302-pack-index.sh | 2 ++
> 3 files changed, 4 insertions(+)
>
> diff --git a/object-file.c b/object-file.c
> index 80a0cd3b351..b554266aff4 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2324,6 +2324,7 @@ static void check_tag(const void *buf, size_t size)
> memset(&t, 0, sizeof(t));
> if (parse_tag_buffer(the_repository, &t, buf, size))
> die(_("corrupt tag"));
> + release_tag_memory(&t);
> }
>
> static int index_mem(struct index_state *istate,
> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
> index e3cf0ffbe59..d3e428ff46e 100755
> --- a/t/t3800-mktag.sh
> +++ b/t/t3800-mktag.sh
> @@ -4,6 +4,7 @@
>
> test_description='git mktag: tag object verify test'
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> ###########################################################
> diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
> index b0095ab41d3..54b11f81c63 100755
> --- a/t/t5302-pack-index.sh
> +++ b/t/t5302-pack-index.sh
> @@ -4,6 +4,8 @@
> #
>
> test_description='pack index with 64-bit offsets and object CRC'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup' '
^ permalink raw reply
* Re: [PATCH v4 19/19] push: free_refs() the "local_refs" in set_refspecs()
From: René Scharfe @ 2023-01-17 19:58 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano, Eric Sunshine
In-Reply-To: <patch-v4-19.19-b3aee41d0b4-20230117T151202Z-avarab@gmail.com>
Am 17.01.23 um 18:11 schrieb Ævar Arnfjörð Bjarmason:
> Fix a memory leak that's been with us since this code was added in
> ca02465b413 (push: use remote.$name.push as a refmap, 2013-12-03).
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> builtin/push.c | 1 +
> t/t1416-ref-transaction-hooks.sh | 1 +
> t/t2402-worktree-list.sh | 1 +
> t/t5504-fetch-receive-strict.sh | 1 +
> t/t5523-push-upstream.sh | 1 +
> t/t5529-push-errors.sh | 2 ++
> t/t5546-receive-limits.sh | 2 ++
> t/t5547-push-quarantine.sh | 2 ++
> t/t5606-clone-options.sh | 1 +
> t/t5810-proto-disable-local.sh | 2 ++
> t/t5813-proto-disable-ssh.sh | 2 ++
> t/t7409-submodule-detached-work-tree.sh | 1 +
> t/t7416-submodule-dash-url.sh | 2 ++
> t/t7450-bad-git-dotfiles.sh | 2 ++
> 14 files changed, 21 insertions(+)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 60ac8017e52..f48e4c6a856 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -129,6 +129,7 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
> } else
> refspec_append(&rs, ref);
> }
> + free_refs(local_refs);
OK.
This can still leak local_refs if remote_get() returns NULL and lazy-
loading is done over and over. Unlikely to occur in the wild, I bet --
who pushes without a remote? Does it make sense to also check
local_refs for NULL already in this patch or is it worth its own series?
Not sure.
remote is still leaked if it isn't NULL, though. We'd need to export
remote_clear() to release it properly, no? And shouldn't
remotes_remote_get_1() call remote_clear() itself before returning
NULL? Not simple, separate series.
> }
>
> static int push_url_of_remote(struct remote *remote, const char ***url_p)
> diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh
> index 27731722a5b..b32ca798f9f 100755
> --- a/t/t1416-ref-transaction-hooks.sh
> +++ b/t/t1416-ref-transaction-hooks.sh
> @@ -5,6 +5,7 @@ test_description='reference transaction hooks'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success setup '
> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
> index 79e0fce2d90..9ad9be0c208 100755
> --- a/t/t2402-worktree-list.sh
> +++ b/t/t2402-worktree-list.sh
> @@ -5,6 +5,7 @@ test_description='test git worktree list'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
> index ac4099ca893..14e8af1f3b7 100755
> --- a/t/t5504-fetch-receive-strict.sh
> +++ b/t/t5504-fetch-receive-strict.sh
> @@ -4,6 +4,7 @@ test_description='fetch/receive strict mode'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup and inject "corrupt or missing" object' '
> diff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh
> index fdb42920564..c9acc076353 100755
> --- a/t/t5523-push-upstream.sh
> +++ b/t/t5523-push-upstream.sh
> @@ -4,6 +4,7 @@ test_description='push with --set-upstream'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
> . "$TEST_DIRECTORY"/lib-terminal.sh
>
> diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
> index ce85fd30ad1..0247137cb36 100755
> --- a/t/t5529-push-errors.sh
> +++ b/t/t5529-push-errors.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='detect some push errors early (before contacting remote)'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup commits' '
> diff --git a/t/t5546-receive-limits.sh b/t/t5546-receive-limits.sh
> index 0b0e987fdb7..eed3c9d81ab 100755
> --- a/t/t5546-receive-limits.sh
> +++ b/t/t5546-receive-limits.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='check receive input limits'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> # Let's run tests with different unpack limits: 1 and 10000
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> index 1876fb34e51..9f899b8c7d7 100755
> --- a/t/t5547-push-quarantine.sh
> +++ b/t/t5547-push-quarantine.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='check quarantine of objects during push'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'create picky dest repo' '
> diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
> index cf221e92c4d..27f9f776389 100755
> --- a/t/t5606-clone-options.sh
> +++ b/t/t5606-clone-options.sh
> @@ -4,6 +4,7 @@ test_description='basic clone options'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> diff --git a/t/t5810-proto-disable-local.sh b/t/t5810-proto-disable-local.sh
> index c1ef99b85c2..862610256fb 100755
> --- a/t/t5810-proto-disable-local.sh
> +++ b/t/t5810-proto-disable-local.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='test disabling of local paths in clone/fetch'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
> . "$TEST_DIRECTORY/lib-proto-disable.sh"
>
> diff --git a/t/t5813-proto-disable-ssh.sh b/t/t5813-proto-disable-ssh.sh
> index 3f084ee3065..2e975dc70ec 100755
> --- a/t/t5813-proto-disable-ssh.sh
> +++ b/t/t5813-proto-disable-ssh.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='test disabling of git-over-ssh in clone/fetch'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
> . "$TEST_DIRECTORY/lib-proto-disable.sh"
>
> diff --git a/t/t7409-submodule-detached-work-tree.sh b/t/t7409-submodule-detached-work-tree.sh
> index 374ed481e9c..574a6fc526e 100755
> --- a/t/t7409-submodule-detached-work-tree.sh
> +++ b/t/t7409-submodule-detached-work-tree.sh
> @@ -13,6 +13,7 @@ TEST_NO_CREATE_REPO=1
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh
> index 3ebd9859814..7cf72b9a076 100755
> --- a/t/t7416-submodule-dash-url.sh
> +++ b/t/t7416-submodule-dash-url.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='check handling of disallowed .gitmodule urls'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup' '
> diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
> index ba1f569bcbb..0d0c3f2c683 100755
> --- a/t/t7450-bad-git-dotfiles.sh
> +++ b/t/t7450-bad-git-dotfiles.sh
> @@ -12,6 +12,8 @@ Such as:
>
> - symlinked .gitmodules, etc
> '
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
> . "$TEST_DIRECTORY"/lib-pack.sh
>
^ permalink raw reply
* [PATCH v2] curl: resolve deprecated curl declarations
From: Rose via GitGitGadget @ 2023-01-17 19:43 UTC (permalink / raw)
To: git; +Cc: Rose, Seija Kijin
In-Reply-To: <pull.1435.git.git.1673983640663.gitgitgadget@gmail.com>
From: Seija Kijin <doremylover123@gmail.com>
Fix CI-Alpine build by replacing deprecated
declarations with their suggested replacements
Note that this required changing the
callbacks of functions because the replacement
for these deprecations require a different function
signature for the callback and different parameters.
Every change done was made as to minimize
changed behavior as well as get the CI to pass again.
Signed-off-by: Seija Kijin <doremylover123@gmail.com>
---
curl: resolve deprecated curl declarations
Fix CI-Alpine build by replacing deprecated declarations with their
suggested replacements
Signed-off-by: Seija Kijin doremylover123@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1435%2FAtariDreams%2Fcurl-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1435/AtariDreams/curl-v2
Pull-Request: https://github.com/git/git/pull/1435
Range-diff vs v1:
1: d84e7e003da ! 1: c40fb2de13d curl: resolve deprecated curl declarations
@@ git-curl-compat.h
#endif
+
+/**
-+ * CURLOPT_REDIR_PROTOCOLS_STR was added in 7.83.0, released in August
++ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
-+#define GIT_CURL_HAVE_OPT_REDIR_PROTOCOLS_STR 1
++#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
## http-push.c ##
@@ http.c: void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
-+#ifdef GIT_CURL_HAVE_OPT_REDIR_PROTOCOLS_STR
++#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(int from_user, char *protocol)
+{
+ unsigned int i = 0;
@@ http.c: static int get_curl_http_version_opt(const char *version_string, long *o
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
-+#ifdef GIT_CURL_HAVE_OPT_REDIR_PROTOCOLS_STR
++#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ static char protocol[20], redir_protocol[20];
+#endif
@@ http.c: static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
-+#ifdef GIT_CURL_HAVE_OPT_REDIR_PROTOCOLS_STR
++#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ get_curl_allowed_protocols(0, redir_protocol);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, redir_protocol);
+ get_curl_allowed_protocols(-1, protocol);
git-curl-compat.h | 8 +++++
http-push.c | 6 ++--
http.c | 74 ++++++++++++++++++++++++++++++++++++++---------
http.h | 2 +-
remote-curl.c | 28 +++++++-----------
5 files changed, 83 insertions(+), 35 deletions(-)
diff --git a/git-curl-compat.h b/git-curl-compat.h
index 56a83b6bbd8..38a2237c8fe 100644
--- a/git-curl-compat.h
+++ b/git-curl-compat.h
@@ -127,3 +127,11 @@
#endif
#endif
+
+/**
+ * CURLOPT_PROTOCOLS_STR was added in 7.83.0, released in August
+ * 2022.
+ */
+#if LIBCURL_VERSION_NUM >= 0x075500
+#define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+#endif
diff --git a/http-push.c b/http-push.c
index 5f4340a36e6..ab458d4d062 100644
--- a/http-push.c
+++ b/http-push.c
@@ -198,13 +198,13 @@ static void curl_setup_http(CURL *curl, const char *url,
const char *custom_req, struct buffer *buffer,
curl_write_callback write_fn)
{
- curl_easy_setopt(curl, CURLOPT_PUT, 1);
+ curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
curl_easy_setopt(curl, CURLOPT_URL, url);
curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLFUNCTION, ioctl_buffer);
- curl_easy_setopt(curl, CURLOPT_IOCTLDATA, buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, ioctl_buffer);
+ curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
diff --git a/http.c b/http.c
index 8a5ba3f4776..ee5f063e5b0 100644
--- a/http.c
+++ b/http.c
@@ -157,21 +157,12 @@ size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
return size / eltsize;
}
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp)
+int ioctl_buffer(void *userp, curl_off_t offset, int origin)
{
- struct buffer *buffer = clientp;
+ struct buffer *buffer = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- buffer->posn = 0;
- return CURLIOE_OK;
-
- default:
- return CURLIOE_UNKNOWNCMD;
- }
+ buffer->posn = 0;
+ return CURL_SEEKFUNC_OK;
}
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
@@ -765,7 +756,52 @@ void setup_curl_trace(CURL *handle)
curl_easy_setopt(handle, CURLOPT_DEBUGFUNCTION, curl_trace);
curl_easy_setopt(handle, CURLOPT_DEBUGDATA, NULL);
}
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+static void get_curl_allowed_protocols(int from_user, char *protocol)
+{
+ unsigned int i = 0;
+
+ if (is_transport_allowed("http", from_user)) {
+ protocol[i++] = 'h';
+ protocol[i++] = 't';
+ protocol[i++] = 't';
+ protocol[i++] = 'p';
+ }
+
+ if (is_transport_allowed("https", from_user)) {
+ if (i != 0) {
+ protocol[i++] = ',';
+ }
+
+ protocol[i++] = 'h';
+ protocol[i++] = 't';
+ protocol[i++] = 't';
+ protocol[i++] = 'p';
+ protocol[i++] = 's';
+ }
+ if (is_transport_allowed("ftp", from_user)) {
+ if (i != 0) {
+ protocol[i++] = ',';
+ }
+ protocol[i++] = 'f';
+ protocol[i++] = 't';
+ protocol[i++] = 'p';
+ }
+ if (is_transport_allowed("ftps", from_user)) {
+ if (i != 0) {
+ protocol[i++] = ',';
+ }
+
+ protocol[i++] = 'f';
+ protocol[i++] = 't';
+ protocol[i++] = 'p';
+ protocol[i++] = 's';
+ }
+
+ protocol[i] = '\0';
+}
+#else
static long get_curl_allowed_protocols(int from_user)
{
long allowed_protocols = 0;
@@ -781,6 +817,7 @@ static long get_curl_allowed_protocols(int from_user)
return allowed_protocols;
}
+#endif
#ifdef GIT_CURL_HAVE_CURL_HTTP_VERSION_2
static int get_curl_http_version_opt(const char *version_string, long *opt)
@@ -810,6 +847,9 @@ static int get_curl_http_version_opt(const char *version_string, long *opt)
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ static char protocol[20], redir_protocol[20];
+#endif
if (!result)
die("curl_easy_init failed");
@@ -923,10 +963,18 @@ static CURL *get_curl_handle(void)
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
+#ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR
+ get_curl_allowed_protocols(0, redir_protocol);
+ curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS_STR, redir_protocol);
+ get_curl_allowed_protocols(-1, protocol);
+ curl_easy_setopt(result, CURLOPT_PROTOCOLS_STR, protocol);
+#else
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
get_curl_allowed_protocols(-1));
+#endif
+
if (getenv("GIT_CURL_VERBOSE"))
http_trace_curl_no_data();
setup_curl_trace(result);
diff --git a/http.h b/http.h
index 3c94c479100..0ec572d4a06 100644
--- a/http.h
+++ b/http.h
@@ -40,7 +40,7 @@ struct buffer {
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_buffer(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
size_t fwrite_null(char *ptr, size_t eltsize, size_t nmemb, void *strbuf);
-curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);
+int ioctl_buffer(void *userp, curl_off_t offset, int origin);
/* Slot lifecycle functions */
struct active_request_slot *get_active_slot(void);
diff --git a/remote-curl.c b/remote-curl.c
index 72dfb8fb86a..ae69dcb70d5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -717,25 +717,17 @@ static size_t rpc_out(void *ptr, size_t eltsize,
return avail;
}
-static curlioerr rpc_ioctl(CURL *handle, int cmd, void *clientp)
+static int rpc_ioctl(void *userp, curl_off_t offset, int origin)
{
- struct rpc_state *rpc = clientp;
+ struct rpc_state *rpc = userp;
- switch (cmd) {
- case CURLIOCMD_NOP:
- return CURLIOE_OK;
-
- case CURLIOCMD_RESTARTREAD:
- if (rpc->initial_buffer) {
- rpc->pos = 0;
- return CURLIOE_OK;
- }
- error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
- return CURLIOE_FAILRESTART;
-
- default:
- return CURLIOE_UNKNOWNCMD;
+ if (rpc->initial_buffer) {
+ rpc->pos = 0;
+ return CURL_SEEKFUNC_OK;
}
+
+ error(_("unable to rewind rpc post data - try increasing http.postBuffer"));
+ return CURL_SEEKFUNC_FAIL;
}
struct check_pktline_state {
@@ -959,8 +951,8 @@ retry:
rpc->initial_buffer = 1;
curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLFUNCTION, rpc_ioctl);
- curl_easy_setopt(slot->curl, CURLOPT_IOCTLDATA, rpc);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_ioctl);
+ curl_easy_setopt(slot->curl, CURLOPT_SEEKDATA, rpc);
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (chunked)\n", rpc->service_name);
fflush(stderr);
base-commit: a7caae2729742fc80147bca1c02ae848cb55921a
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v4 0/5] submodule: parallelize diff
From: Calvin Wan @ 2023-01-17 19:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, emilyshaffer, avarab, phillip.wood123, myriamanis
In-Reply-To: <xmqqv8l8f8j8.fsf@gitster.g>
Hi Junio
I've sent out a reroll to fix this. Thanks!
Passing leaks CI at:
https://github.com/calvin-wan-google/git/actions/runs/3942292098
(linux-musl technically failed, but it looks like for other reasons)
On Sun, Jan 15, 2023 at 1:31 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Calvin Wan <calvinwan@google.com> writes:
>
> > Original cover letter for context:
> > https://lore.kernel.org/git/20221011232604.839941-1-calvinwan@google.com/
> > ...
> > Calvin Wan (5):
> > run-command: add duplicate_output_fn to run_processes_parallel_opts
> > submodule: strbuf variable rename
> > submodule: move status parsing into function
> > diff-lib: refactor match_stat_with_submodule
> > diff-lib: parallelize run_diff_files for submodules
> >
> > Documentation/config/submodule.txt | 12 ++
> > diff-lib.c | 103 +++++++++++--
> > run-command.c | 13 +-
> > run-command.h | 24 +++
> > submodule.c | 229 +++++++++++++++++++++++++----
> > submodule.h | 9 ++
> > t/helper/test-run-command.c | 21 +++
> > t/t0061-run-command.sh | 39 +++++
> > t/t4027-diff-submodule.sh | 19 +++
> > t/t7506-status-submodule.sh | 19 +++
> > 10 files changed, 441 insertions(+), 47 deletions(-)
>
> While the topic is marked as "Needs review" in the recent "What's
> cooking" reports, merging this topic also breaks the "linux-leaks"
> job by causing many tests fail:
>
> t3040-subprojects-basic.sh
> t4010-diff-pathspec.sh
> t4015-diff-whitespace.sh
> t4027-diff-submodule.sh
> t7403-submodule-sync.sh
> t7409-submodule-detached-work-tree.sh
> t7416-submodule-dash-url.sh
> t7450-bad-git-dotfiles.sh
> t7506-status-submodule.sh
>
> Two of the test scripts are touched by this topic, and their
> breakage could be caused by newly using other git subcommands that
> were known to be leaking (iow, not because this series introduced
> new leaks). It also is possible that they fail because this series
> added new leaks to the commands these two test scripts use. In
> either case, other tests that haven't been touched by this topic
> were definitely broken by new leaks introduced by the changes made
> by this series.
>
> Anybody interested should be able to see the breakage themselves by
> checking out 'seen' and running
>
> SANTIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=true \
> make test
>
> to see the tree with all in-flight topics are clean, and then by
> running the same test after merging this topic to 'seen'.
>
> Thanks.
^ permalink raw reply
* [PATCH v6 6/6] submodule: call parallel code from serial status
From: Calvin Wan @ 2023-01-17 19:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, emilyshaffer, avarab, phillip.wood123, chooglen,
newren, jonathantanmy
In-Reply-To: <20230104215415.1083526-1-calvinwan@google.com>
Remove the serial implementation of status inside of
is_submodule_modified since the parallel implementation of status with
one job accomplishes the same task.
Combine parse_status_porcelain and parse_status_porcelain_strbuf since
the only other caller of parse_status_porcelain was in
is_submodule_modified
Signed-off-by: Calvin Wan <calvinwan@google.com>
---
submodule.c | 146 ++++++++++++++++++----------------------------------
1 file changed, 51 insertions(+), 95 deletions(-)
diff --git a/submodule.c b/submodule.c
index da95ea1f5e..2009748d9f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1887,46 +1887,7 @@ int fetch_submodules(struct repository *r,
return spf.result;
}
-static int parse_status_porcelain(char *str, size_t len,
- unsigned *dirty_submodule,
- int ignore_untracked)
-{
- /* regular untracked files */
- if (str[0] == '?')
- *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
-
- if (str[0] == 'u' ||
- str[0] == '1' ||
- str[0] == '2') {
- /* T = line type, XY = status, SSSS = submodule state */
- if (len < strlen("T XY SSSS"))
- BUG("invalid status --porcelain=2 line %s",
- str);
-
- if (str[5] == 'S' && str[8] == 'U')
- /* nested untracked file */
- *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
-
- if (str[0] == 'u' ||
- str[0] == '2' ||
- memcmp(str + 5, "S..U", 4))
- /* other change */
- *dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
- }
-
- if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) &&
- ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ||
- ignore_untracked)) {
- /*
- * We're not interested in any further information from
- * the child any more, neither output nor its exit code.
- */
- return 1;
- }
- return 0;
-}
-
-static void parse_status_porcelain_strbuf(struct strbuf *buf,
+static void parse_status_porcelain(struct strbuf *buf,
unsigned *dirty_submodule,
int ignore_untracked)
{
@@ -1936,65 +1897,60 @@ static void parse_status_porcelain_strbuf(struct strbuf *buf,
string_list_split(&list, buf->buf, '\n', -1);
for_each_string_list_item(item, &list) {
- if (parse_status_porcelain(item->string,
- strlen(item->string),
- dirty_submodule,
- ignore_untracked))
+ char *str = item->string;
+ /* regular untracked files */
+ if (str[0] == '?')
+ *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
+
+ if (str[0] == 'u' ||
+ str[0] == '1' ||
+ str[0] == '2') {
+ /* T = line type, XY = status, SSSS = submodule state */
+ if (strlen(str) < strlen("T XY SSSS"))
+ BUG("invalid status --porcelain=2 line %s",
+ str);
+
+ if (str[5] == 'S' && str[8] == 'U')
+ /* nested untracked file */
+ *dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
+
+ if (str[0] == 'u' ||
+ str[0] == '2' ||
+ memcmp(str + 5, "S..U", 4))
+ /* other change */
+ *dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
+ }
+
+ if ((*dirty_submodule & DIRTY_SUBMODULE_MODIFIED) &&
+ ((*dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ||
+ ignore_untracked)) {
+ /*
+ * We're not interested in any further information from
+ * the child any more, neither output nor its exit code.
+ */
break;
+ }
}
string_list_clear(&list, 0);
}
unsigned is_submodule_modified(const char *path, int ignore_untracked)
{
- struct child_process cp = CHILD_PROCESS_INIT;
- struct strbuf buf = STRBUF_INIT;
- FILE *fp;
- unsigned dirty_submodule = 0;
- const char *git_dir;
- int ignore_cp_exit_code = 0;
-
- strbuf_addf(&buf, "%s/.git", path);
- git_dir = read_gitfile(buf.buf);
- if (!git_dir)
- git_dir = buf.buf;
- if (!is_git_directory(git_dir)) {
- if (is_directory(git_dir))
- die(_("'%s' not recognized as a git repository"), git_dir);
- strbuf_release(&buf);
- /* The submodule is not checked out, so it is not modified */
- return 0;
- }
- strbuf_reset(&buf);
-
- strvec_pushl(&cp.args, "status", "--porcelain=2", NULL);
- if (ignore_untracked)
- strvec_push(&cp.args, "-uno");
-
- prepare_submodule_repo_env(&cp.env);
- cp.git_cmd = 1;
- cp.no_stdin = 1;
- cp.out = -1;
- cp.dir = path;
- if (start_command(&cp))
- die(_("Could not run 'git status --porcelain=2' in submodule %s"), path);
-
- fp = xfdopen(cp.out, "r");
- while (strbuf_getwholeline(&buf, fp, '\n') != EOF) {
- char *str = buf.buf;
- const size_t len = buf.len;
-
- ignore_cp_exit_code = parse_status_porcelain(str, len, &dirty_submodule,
- ignore_untracked);
- if (ignore_cp_exit_code)
- break;
- }
- fclose(fp);
-
- if (finish_command(&cp) && !ignore_cp_exit_code)
- die(_("'git status --porcelain=2' failed in submodule %s"), path);
-
- strbuf_release(&buf);
+ struct submodule_status_util util = {
+ .dirty_submodule = 0,
+ .ignore_untracked = ignore_untracked,
+ .path = path,
+ };
+ struct string_list sub = STRING_LIST_INIT_NODUP;
+ struct string_list_item *item;
+ int dirty_submodule;
+
+ item = string_list_append(&sub, path);
+ item->util = &util;
+ if (get_submodules_status(&sub, 1))
+ die(_("submodule status failed"));
+ dirty_submodule = util.dirty_submodule;
+ string_list_clear(&sub, 0);
return dirty_submodule;
}
@@ -2096,9 +2052,9 @@ static int status_finish(int retvalue, struct strbuf *err,
task->path);
}
- parse_status_porcelain_strbuf(&task->out,
- &util->dirty_submodule,
- util->ignore_untracked);
+ parse_status_porcelain(&task->out,
+ &util->dirty_submodule,
+ util->ignore_untracked);
strbuf_release(&task->out);
free(task);
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related
* [PATCH v6 5/6] diff-lib: parallelize run_diff_files for submodules
From: Calvin Wan @ 2023-01-17 19:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, emilyshaffer, avarab, phillip.wood123, chooglen,
newren, jonathantanmy
In-Reply-To: <20230104215415.1083526-1-calvinwan@google.com>
During the iteration of the index entries in run_diff_files, whenever
a submodule is found and needs its status checked, a subprocess is
spawned for it. Instead of spawning the subprocess immediately and
waiting for its completion to continue, hold onto all submodules and
relevant information in a list. Then use that list to create tasks for
run_processes_parallel. Subprocess output is duplicated and passed to
status_pipe_output which stores it to be parsed on completion of the
subprocess.
Add config option submodule.diffJobs to set the maximum number
of parallel jobs. The option defaults to 1 if unset. If set to 0, the
number of jobs is set to online_cpus().
Since run_diff_files is called from many different commands, I chose
to grab the config option in the function rather than adding variables
to every git command and then figuring out how to pass them all in.
Signed-off-by: Calvin Wan <calvinwan@google.com>
---
Documentation/config/submodule.txt | 12 ++
diff-lib.c | 84 ++++++++++++--
submodule.c | 169 +++++++++++++++++++++++++++++
submodule.h | 9 ++
t/t4027-diff-submodule.sh | 19 ++++
t/t7506-status-submodule.sh | 19 ++++
6 files changed, 305 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/submodule.txt b/Documentation/config/submodule.txt
index 6490527b45..3209eb8117 100644
--- a/Documentation/config/submodule.txt
+++ b/Documentation/config/submodule.txt
@@ -93,6 +93,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
+submodule.diffJobs::
+ Specifies how many submodules are diffed at the same time. A
+ positive integer allows up to that number of submodules diffed
+ in parallel. A value of 0 will give some reasonable default.
+ If unset, it defaults to 1. The diff operation is used by many
+ other git commands such as add, merge, diff, status, stash and
+ more. Note that the expensive part of the diff operation is
+ reading the index from cache or memory. Therefore multiple jobs
+ may be detrimental to performance if your hardware does not
+ support parallel reads or if the number of jobs greatly exceeds
+ the amount of supported reads.
+
submodule.alternateLocation::
Specifies how the submodules obtain alternates when submodules are
cloned. Possible values are `no`, `superproject`.
diff --git a/diff-lib.c b/diff-lib.c
index 64583fded0..f51ea07f36 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -14,6 +14,7 @@
#include "dir.h"
#include "fsmonitor.h"
#include "commit-reach.h"
+#include "config.h"
/*
* diff-files
@@ -65,18 +66,23 @@ static int check_removed(const struct index_state *istate, const struct cache_en
* Return 1 when changes are detected, 0 otherwise. If the DIRTY_SUBMODULES
* option is set, the caller does not only want to know if a submodule is
* modified at all but wants to know all the conditions that are met (new
- * commits, untracked content and/or modified content).
+ * commits, untracked content and/or modified content). If
+ * defer_submodule_status bit is set, dirty_submodule will be left to the
+ * caller to set. defer_submodule_status can also be set to 0 in this
+ * function if there is no need to check if the submodule is modified.
*/
static int match_stat_with_submodule(struct diff_options *diffopt,
const struct cache_entry *ce,
struct stat *st, unsigned ce_option,
- unsigned *dirty_submodule)
+ unsigned *dirty_submodule, int *defer_submodule_status,
+ unsigned *ignore_untracked)
{
int changed = ie_match_stat(diffopt->repo->index, ce, st, ce_option);
struct diff_flags orig_flags;
+ int defer = 0;
if (!S_ISGITLINK(ce->ce_mode))
- return changed;
+ goto ret;
orig_flags = diffopt->flags;
if (!diffopt->flags.override_submodule_config)
@@ -86,11 +92,20 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
goto cleanup;
}
if (!diffopt->flags.ignore_dirty_submodules &&
- (!changed || diffopt->flags.dirty_submodules))
- *dirty_submodule = is_submodule_modified(ce->name,
+ (!changed || diffopt->flags.dirty_submodules)) {
+ if (defer_submodule_status && *defer_submodule_status) {
+ defer = 1;
+ *ignore_untracked = diffopt->flags.ignore_untracked_in_submodules;
+ } else {
+ *dirty_submodule = is_submodule_modified(ce->name,
diffopt->flags.ignore_untracked_in_submodules);
+ }
+ }
cleanup:
diffopt->flags = orig_flags;
+ret:
+ if (defer_submodule_status)
+ *defer_submodule_status = defer;
return changed;
}
@@ -102,6 +117,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
? CE_MATCH_RACY_IS_DIRTY : 0);
uint64_t start = getnanotime();
struct index_state *istate = revs->diffopt.repo->index;
+ struct string_list submodules = STRING_LIST_INIT_NODUP;
diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
@@ -226,6 +242,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
newmode = ce->ce_mode;
} else {
struct stat st;
+ unsigned ignore_untracked = 0;
+ int defer_submodule_status = !!revs->repo;
changed = check_removed(istate, ce, &st);
if (changed) {
@@ -247,8 +265,26 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
}
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
- ce_option, &dirty_submodule);
+ ce_option, &dirty_submodule,
+ &defer_submodule_status,
+ &ignore_untracked);
newmode = ce_mode_from_stat(ce, st.st_mode);
+ if (defer_submodule_status) {
+ struct submodule_status_util tmp = {
+ .changed = changed,
+ .dirty_submodule = 0,
+ .ignore_untracked = ignore_untracked,
+ .newmode = newmode,
+ .ce = ce,
+ .path = ce->name,
+ };
+ struct string_list_item *item;
+
+ item = string_list_append(&submodules, ce->name);
+ item->util = xmalloc(sizeof(tmp));
+ memcpy(item->util, &tmp, sizeof(tmp));
+ continue;
+ }
}
if (!changed && !dirty_submodule) {
@@ -267,6 +303,40 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
ce->name, 0, dirty_submodule);
}
+ if (submodules.nr > 0) {
+ int parallel_jobs;
+ if (git_config_get_int("submodule.diffjobs", ¶llel_jobs))
+ parallel_jobs = 1;
+ else if (!parallel_jobs)
+ parallel_jobs = online_cpus();
+ else if (parallel_jobs < 0)
+ die(_("submodule.diffjobs cannot be negative"));
+
+ if (get_submodules_status(&submodules, parallel_jobs))
+ die(_("submodule status failed"));
+ for (size_t i = 0; i < submodules.nr; i++) {
+ struct submodule_status_util *util = submodules.items[i].util;
+ struct cache_entry *ce = util->ce;
+ unsigned int oldmode;
+ const struct object_id *old_oid, *new_oid;
+
+ if (!util->changed && !util->dirty_submodule) {
+ ce_mark_uptodate(ce);
+ mark_fsmonitor_valid(istate, ce);
+ if (!revs->diffopt.flags.find_copies_harder)
+ continue;
+ }
+ oldmode = ce->ce_mode;
+ old_oid = &ce->oid;
+ new_oid = util->changed ? null_oid() : &ce->oid;
+ diff_change(&revs->diffopt, oldmode, util->newmode,
+ old_oid, new_oid,
+ !is_null_oid(old_oid),
+ !is_null_oid(new_oid),
+ ce->name, 0, util->dirty_submodule);
+ }
+ }
+ string_list_clear(&submodules, 1);
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
trace_performance_since(start, "diff-files");
@@ -314,7 +384,7 @@ static int get_stat_data(const struct index_state *istate,
return -1;
}
changed = match_stat_with_submodule(diffopt, ce, &st,
- 0, dirty_submodule);
+ 0, dirty_submodule, NULL, NULL);
if (changed) {
mode = ce_mode_from_stat(ce, st.st_mode);
oid = null_oid();
diff --git a/submodule.c b/submodule.c
index 768d4b4cd7..da95ea1f5e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1369,6 +1369,17 @@ int submodule_touches_in_range(struct repository *r,
return ret;
}
+struct submodule_parallel_status {
+ size_t index_count;
+ int result;
+
+ struct string_list *submodule_names;
+
+ /* Pending statuses by OIDs */
+ struct status_task **oid_status_tasks;
+ int oid_status_tasks_nr, oid_status_tasks_alloc;
+};
+
struct submodule_parallel_fetch {
/*
* The index of the last index entry processed by
@@ -1451,6 +1462,12 @@ struct fetch_task {
struct oid_array *commits; /* Ensure these commits are fetched */
};
+struct status_task {
+ const char *path;
+ struct strbuf out;
+ int ignore_untracked;
+};
+
/**
* When a submodule is not defined in .gitmodules, we cannot access it
* via the regular submodule-config. Create a fake submodule, which we can
@@ -1909,6 +1926,25 @@ static int parse_status_porcelain(char *str, size_t len,
return 0;
}
+static void parse_status_porcelain_strbuf(struct strbuf *buf,
+ unsigned *dirty_submodule,
+ int ignore_untracked)
+{
+ struct string_list list = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+
+ string_list_split(&list, buf->buf, '\n', -1);
+
+ for_each_string_list_item(item, &list) {
+ if (parse_status_porcelain(item->string,
+ strlen(item->string),
+ dirty_submodule,
+ ignore_untracked))
+ break;
+ }
+ string_list_clear(&list, 0);
+}
+
unsigned is_submodule_modified(const char *path, int ignore_untracked)
{
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1962,6 +1998,139 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
return dirty_submodule;
}
+static struct status_task *
+get_status_task_from_index(struct submodule_parallel_status *sps,
+ struct strbuf *err)
+{
+ for (; sps->index_count < sps->submodule_names->nr; sps->index_count++) {
+ struct submodule_status_util *util = sps->submodule_names->items[sps->index_count].util;
+ struct status_task *task;
+ struct strbuf buf = STRBUF_INIT;
+ const char *git_dir;
+
+ strbuf_addf(&buf, "%s/.git", util->path);
+ git_dir = read_gitfile(buf.buf);
+ if (!git_dir)
+ git_dir = buf.buf;
+ if (!is_git_directory(git_dir)) {
+ if (is_directory(git_dir))
+ die(_("'%s' not recognized as a git repository"), git_dir);
+ strbuf_release(&buf);
+ /* The submodule is not checked out, so it is not modified */
+ util->dirty_submodule = 0;
+ continue;
+ }
+ strbuf_release(&buf);
+
+ task = xmalloc(sizeof(*task));
+ task->path = util->path;
+ task->ignore_untracked = util->ignore_untracked;
+ strbuf_init(&task->out, 0);
+ sps->index_count++;
+ return task;
+ }
+ return NULL;
+}
+
+static int get_next_submodule_status(struct child_process *cp,
+ struct strbuf *err, void *data,
+ void **task_cb)
+{
+ struct submodule_parallel_status *sps = data;
+ struct status_task *task = get_status_task_from_index(sps, err);
+
+ if (!task)
+ return 0;
+
+ child_process_init(cp);
+ prepare_submodule_repo_env_in_gitdir(&cp->env);
+
+ strvec_init(&cp->args);
+ strvec_pushl(&cp->args, "status", "--porcelain=2", NULL);
+ if (task->ignore_untracked)
+ strvec_push(&cp->args, "-uno");
+
+ prepare_submodule_repo_env(&cp->env);
+ cp->git_cmd = 1;
+ cp->dir = task->path;
+ *task_cb = task;
+ return 1;
+}
+
+static int status_start_failure(struct strbuf *err,
+ void *cb, void *task_cb)
+{
+ struct submodule_parallel_status *sps = cb;
+ struct status_task *task = task_cb;
+
+ sps->result = 1;
+ strbuf_addf(err,
+ _("could not run 'git status --porcelain=2' in submodule %s"),
+ task->path);
+ return 0;
+}
+
+static void status_duplicate_output(struct strbuf *out,
+ size_t offset,
+ void *cb, void *task_cb)
+{
+ struct status_task *task = task_cb;
+
+ strbuf_add(&task->out, out->buf + offset, out->len - offset);
+ strbuf_setlen(out, offset);
+}
+
+static int status_finish(int retvalue, struct strbuf *err,
+ void *cb, void *task_cb)
+{
+ struct submodule_parallel_status *sps = cb;
+ struct status_task *task = task_cb;
+ struct string_list_item *it =
+ string_list_lookup(sps->submodule_names, task->path);
+ struct submodule_status_util *util = it->util;
+
+ if (retvalue) {
+ sps->result = 1;
+ strbuf_addf(err,
+ _("'git status --porcelain=2' failed in submodule %s"),
+ task->path);
+ }
+
+ parse_status_porcelain_strbuf(&task->out,
+ &util->dirty_submodule,
+ util->ignore_untracked);
+
+ strbuf_release(&task->out);
+ free(task);
+
+ return 0;
+}
+
+int get_submodules_status(struct string_list *submodules,
+ int max_parallel_jobs)
+{
+ struct submodule_parallel_status sps = {
+ .submodule_names = submodules,
+ };
+ const struct run_process_parallel_opts opts = {
+ .tr2_category = "submodule",
+ .tr2_label = "parallel/status",
+
+ .processes = max_parallel_jobs,
+
+ .get_next_task = get_next_submodule_status,
+ .start_failure = status_start_failure,
+ .duplicate_output = status_duplicate_output,
+ .task_finished = status_finish,
+ .data = &sps,
+ };
+
+ string_list_sort(sps.submodule_names);
+ run_processes_parallel(&opts);
+
+ return sps.result;
+}
+
int submodule_uses_gitfile(const char *path)
{
struct child_process cp = CHILD_PROCESS_INIT;
diff --git a/submodule.h b/submodule.h
index b52a4ff1e7..08d278a414 100644
--- a/submodule.h
+++ b/submodule.h
@@ -41,6 +41,13 @@ struct submodule_update_strategy {
.type = SM_UPDATE_UNSPECIFIED, \
}
+struct submodule_status_util {
+ int changed, ignore_untracked;
+ unsigned dirty_submodule, newmode;
+ struct cache_entry *ce;
+ const char *path;
+};
+
int is_gitmodules_unmerged(struct index_state *istate);
int is_writing_gitmodules_ok(void);
int is_staging_gitmodules_ok(struct index_state *istate);
@@ -94,6 +101,8 @@ int fetch_submodules(struct repository *r,
int command_line_option,
int default_option,
int quiet, int max_parallel_jobs);
+int get_submodules_status(struct string_list *submodules,
+ int max_parallel_jobs);
unsigned is_submodule_modified(const char *path, int ignore_untracked);
int submodule_uses_gitfile(const char *path);
diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 40164ae07d..e08ee315a7 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -34,6 +34,25 @@ test_expect_success setup '
subtip=$3 subprev=$2
'
+test_expect_success 'diff in superproject with submodules respects parallel settings' '
+ test_when_finished "rm -f trace.out" &&
+ (
+ GIT_TRACE=$(pwd)/trace.out git diff &&
+ grep "1 tasks" trace.out &&
+ >trace.out &&
+
+ git config submodule.diffJobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git diff &&
+ grep "8 tasks" trace.out &&
+ >trace.out &&
+
+ GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 diff &&
+ grep "preparing to run up to [0-9]* tasks" trace.out &&
+ ! grep "up to 0 tasks" trace.out &&
+ >trace.out
+ )
+'
+
test_expect_success 'git diff --raw HEAD' '
hexsz=$(test_oid hexsz) &&
git diff --raw --abbrev=$hexsz HEAD >actual &&
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index d050091345..52a82b703f 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -412,4 +412,23 @@ test_expect_success 'status with added file in nested submodule (short)' '
EOF
'
+test_expect_success 'status in superproject with submodules respects parallel settings' '
+ test_when_finished "rm -f trace.out" &&
+ (
+ GIT_TRACE=$(pwd)/trace.out git status &&
+ grep "1 tasks" trace.out &&
+ >trace.out &&
+
+ git config submodule.diffJobs 8 &&
+ GIT_TRACE=$(pwd)/trace.out git status &&
+ grep "8 tasks" trace.out &&
+ >trace.out &&
+
+ GIT_TRACE=$(pwd)/trace.out git -c submodule.diffJobs=0 status &&
+ grep "preparing to run up to [0-9]* tasks" trace.out &&
+ ! grep "up to 0 tasks" trace.out &&
+ >trace.out
+ )
+'
+
test_done
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related
* [PATCH v6 4/6] diff-lib: refactor match_stat_with_submodule
From: Calvin Wan @ 2023-01-17 19:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, emilyshaffer, avarab, phillip.wood123, chooglen,
newren, jonathantanmy
In-Reply-To: <20230104215415.1083526-1-calvinwan@google.com>
Flatten out the if statements in match_stat_with_submodule so the
logic is more readable and easier for future patches to add to.
orig_flags didn't need to be set if the cache entry wasn't a
GITLINK so defer setting it.
Signed-off-by: Calvin Wan <calvinwan@google.com>
---
diff-lib.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index dec040c366..64583fded0 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -73,18 +73,24 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
unsigned *dirty_submodule)
{
int changed = ie_match_stat(diffopt->repo->index, ce, st, ce_option);
- if (S_ISGITLINK(ce->ce_mode)) {
- struct diff_flags orig_flags = diffopt->flags;
- if (!diffopt->flags.override_submodule_config)
- set_diffopt_flags_from_submodule_config(diffopt, ce->name);
- if (diffopt->flags.ignore_submodules)
- changed = 0;
- else if (!diffopt->flags.ignore_dirty_submodules &&
- (!changed || diffopt->flags.dirty_submodules))
- *dirty_submodule = is_submodule_modified(ce->name,
- diffopt->flags.ignore_untracked_in_submodules);
- diffopt->flags = orig_flags;
+ struct diff_flags orig_flags;
+
+ if (!S_ISGITLINK(ce->ce_mode))
+ return changed;
+
+ orig_flags = diffopt->flags;
+ if (!diffopt->flags.override_submodule_config)
+ set_diffopt_flags_from_submodule_config(diffopt, ce->name);
+ if (diffopt->flags.ignore_submodules) {
+ changed = 0;
+ goto cleanup;
}
+ if (!diffopt->flags.ignore_dirty_submodules &&
+ (!changed || diffopt->flags.dirty_submodules))
+ *dirty_submodule = is_submodule_modified(ce->name,
+ diffopt->flags.ignore_untracked_in_submodules);
+cleanup:
+ diffopt->flags = orig_flags;
return changed;
}
--
2.39.0.314.g84b9a713c41-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox