* Re: [Outreachy][PATCH] Port helper/test-advise.c to unit-tests/t-advise.c
From: Rubén Justo @ 2024-01-15 19:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Achu Luma, git, christian.couder, Christian Couder
In-Reply-To: <xmqqy1cq4ide.fsf@gitster.g>
On 15-ene-2024 09:27:25, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> >> To test the effect of setting one configuration variable, and ensure
> >> it results in a slightly different advice message output to the
> >> standard error stream, "test-tool advice" needs only a single line
> >> of patch, but if we started with this version, how much work does it
> >> take to run the equivalent test in the other patch if it were to be
> >> rebased on top of this change? It won't be just the matter of
> >> adding a new TEST(check_advise_if_enabled()) call to cmd_main(),
> >> will it?
> >
> > Maybe something like this will do the trick:
> >
> > diff --git a/t/unit-tests/t-advise.c b/t/unit-tests/t-advise.c
> > index 15df29c955..ac7d2620ef 100644
> > --- a/t/unit-tests/t-advise.c
> > +++ b/t/unit-tests/t-advise.c
> > @@ -6,6 +6,7 @@
> >
> > static const char expect_advice_msg[] = "hint: This is a piece of advice\n"
> > "hint: Disable this message with \"git config advice.nestedTag false\"\n";
> > +static const char expect_advice_msg_without_disable_hint[] = "hint: This is a piece of advice\n";
> > static const char advice_msg[] = "This is a piece of advice";
> > static const char out_file[] = "./output.txt";
>
> Yup, but ...
>
> > @@ -44,7 +45,7 @@ int cmd_main(int argc, const char **argv) {
> >
> > TEST(check_advise_if_enabled(advice_msg, NULL, expect_advice_msg),
> > "advice should be printed when config variable is unset");
> > - TEST(check_advise_if_enabled(advice_msg, "true", expect_advice_msg),
> > + TEST(check_advise_if_enabled(advice_msg, "true", expect_advice_msg_without_disable_hint),
> > "advice should be printed when config variable is set to true");
> > TEST(check_advise_if_enabled(advice_msg, "false", ""),
> > "advice should not be printed when config variable is set to false");
>
> ... I cannot shake this feeling that the next person who comes to
> this code and stares at advice.c would be very tempted to "refactor"
> the messages, so that there is only one instance of the same string
> in advice.c that is passed to TEST() above. After all, you can
> change only one place to update the message and avoid triggering
> test failures that way, right?
I see. Maybe you're expecting something like:
diff --git a/t/unit-tests/t-advise.c b/t/unit-tests/t-advise.c
index 15df29c955..15e293fa82 100644
--- a/t/unit-tests/t-advise.c
+++ b/t/unit-tests/t-advise.c
@@ -4,14 +4,15 @@
#include "setup.h"
#include "strbuf.h"
-static const char expect_advice_msg[] = "hint: This is a piece of advice\n"
- "hint: Disable this message with \"git config advice.nestedTag false\"\n";
+static const char expect_advice_msg[] = "hint: This is a piece of advice\n";
+static const char expect_hint_msg[] = "hint: Disable this message with \"git config advice.nestedTag false\"\n";
static const char advice_msg[] = "This is a piece of advice";
static const char out_file[] = "./output.txt";
static void check_advise_if_enabled(const char *argv, const char *conf_val, const char *expect) {
FILE *file;
+ const char *hint;
struct strbuf actual = STRBUF_INIT;
if (conf_val)
@@ -32,7 +33,9 @@ static void check_advise_if_enabled(const char *argv, const char *conf_val, cons
return;
}
- check_str(actual.buf, expect);
+ check_str_len(actual.buf, expect, strlen(expect));
+ if (!conf_val && skip_prefix(actual.buf, expect, &hint))
+ check_str_len(hint, expect_hint_msg, strlen(expect_hint_msg));
strbuf_release(&actual);
if (!check(remove(out_file) == 0))
This implies a new check_str_len() helper, which I'm not including here
but it's a trivial copy of check_str() but using strncmp().
Maybe we can turn the screw a little more.
I'm still not sure of the value in the changes in this series, though.
^ permalink raw reply related
* git-range-diff ignoring commit message changes
From: Gwyneth Morgan @ 2024-01-15 19:40 UTC (permalink / raw)
To: git
Is there a way to make git-range-diff to ignore commit messages when
considering if commits are identical? When range-diffing long series
there are cases where I would like to check at a glance whether the code
has changed, and only when the code has changed do I want to see the
change in commit message too.
It seems I can approximate what I want by tweaking range-diff's source
like this but I couldn't tell if there was an actual option:
diff --git a/range-diff.c b/range-diff.c
index c45b6d849c..fd421b7b99 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -59,7 +59,7 @@ static int read_patches(const char *range, struct string_list *list,
"--output-indicator-old=<",
"--output-indicator-context=#",
"--no-abbrev-commit",
- "--pretty=medium",
+ "--pretty=format:commit %H",
"--show-notes-by-default",
NULL);
strvec_push(&cp.args, range);
This ignores commit message changes entirely, but it would be nice to
have an option to only see commit message changes when the code diff has
changed. It would also be convenient to have a way to only consider
changes in the title of commits but ignore the message body (equivalent
to "--pretty=short" above).
You could get some of this information with git-cherry, but that is
suited for different uses (only cares about new commits on one side,
doesn't show diffs) and would take more effort than just ignoring the
commit messages in the current range-diff output.
^ permalink raw reply related
* Re: [PATCH 1/7] chunk-format: introduce `pair_chunk_expect()` helper
From: Linus Arver @ 2024-01-15 22:31 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <af5fe3b7237caeba8f970e967933db96c83a230e.1699569246.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
> +static int pair_chunk_expect_fn(const unsigned char *chunk_start,
> + size_t chunk_size,
> + void *data)
> +{
> + struct pair_chunk_data *pcd = data;
> + if (chunk_size / pcd->record_size != pcd->record_nr)
> + return -1;
> + *pcd->p = chunk_start;
> + return 0;
> +}
> +
I don't think this function should assume anything about the inputs
(chunk_size, record_size, nor record_nr). If we are asking the helper to
be the common tool for multiple locations, it should assume even less
about what these inputs could look like.
For example, if record_size is 0 this will lead to a
divide-by-zero. Likewise, if record_nr is zero it doesn't make
sense to check if chunk_size fits into record_size * record_nr.
And even if there are no (unexpected) zero-values involved, shouldn't we
also check for nonsensical comparisons, such as if chunk_size is smaller
than record_size?
I think there are several possibilities:
CHUNK_MISSING (chunk_size == 0)
CHUNK_TOO_SMALL (chunk_size < record_size)
CHUNK_OK (chunk_size == record_nr * record_size)
CHUNK_TOO_BIG (chunk_size > record_size, record_nr * record_size < chunk_size)
And for the CHUNK_TOO_BIG case there are two possibilities --- the
leftover parts of chunk_size that are not accounted by "record_nr *
record_size" are (or are not) "aligned" such that increasing the
record_size by some positive increment could exactly match the
chunk_size. For example, chunk_size = 128, record_size = 8, record_nr =
8. In this case the records account for 64 bytes so we have 128 - 64 =
64 bytes left over, and simply increasing record_nr to 16 could account
for every bytes in chunk_size. If chunk_size in this example was 120 or
130, there would be bytes left over that cannot be accounted for by
adjusting record_size. This divisibility-of-leftover-bytes check could
be done with '%' as mentioned already by others.
So in summary there appear to be the following possibilities:
CHUNK_MISSING
CHUNK_TOO_SMALL
CHUNK_OK
CHUNK_TOO_BIG_ALIGNED
CHUNK_TOO_BIG_MISALIGNED
(on top of the cases where record_* inputs are zero).
And it seems prudent to treat each of the not-OK cases differently
(including how we report error messages) once we know which category we
fall into.
^ permalink raw reply
* Re: [PATCH 1/7] chunk-format: introduce `pair_chunk_expect()` helper
From: Linus Arver @ 2024-01-15 22:53 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <owly8r4qi5zj.fsf@fine.c.googlers.com>
Linus Arver <linusa@google.com> writes:
> So in summary there appear to be the following possibilities:
>
> CHUNK_MISSING
> CHUNK_TOO_SMALL
> CHUNK_OK
> CHUNK_TOO_BIG_ALIGNED
> CHUNK_TOO_BIG_MISALIGNED
On second thought, it appears that CHUNK_TOO_SMALL has three cases:
(1) chunk_size < record_size
(2) chunk_size >= record_size, but chunk_size < record_size * record_nr
and decreasing record_nr can make chunk_size "fit"
(3) chunk_size >= record_size, but chunk_size < record_size * record_nr
and decreasing record_nr cannot make chunk_size "fit"
where (2) and (3) are just like the *_(MIS)ALIGNED cases above when
chunk_size is too big.
My default position is that these additional cases might need to be
treated differently, although maybe it's overkill also.
Thanks.
^ permalink raw reply
* Re: Suggested clarification for .gitattributes reference documentation
From: Michael Litwak @ 2024-01-16 0:19 UTC (permalink / raw)
To: Torsten Bögershausen, Matthias Aßhauer
Cc: brian m. carlson, git@vger.kernel.org
In-Reply-To: <20240113074323.GA6819@tb-raspi4>
> To my knowledge the binary iconv.exe (or just iconv under non-Windows) is never called from Git itself.
> I can't find a single instance of Git for Windows calling iconv.exe instead of using the corresponding library functions.
Thank you for your responses. I think you are both right. Git must instead call methods in libiconv-2.dll to do encoding conversions.
I have no idea why my Windows 10 PC could add a UTF-16LE with BOM file, but then fail to later successfully "decode" it, when running Git from an ordinary Command Prompt (cmd.exe). I assume this failure was a fluke, since I cannot replicate the failure on my other (Windows 11) PC. So I am withdrawing my concerns about:
1) Git for Windows failing to support UTF-16LE with BOM.
2) Git for Windows installer being misleading in its "recommended" PATH modification option.
As for documentation clarifications for the .gitattributes manpage at https://git-scm.com/docs/gitattributes, I still suggest adding an explicit example for UTF-16LE with BOM, and/or adding a table listing which working-tree-encoding value to use for each of the following UTF-16 text encodings:
ENCODING 'working-tree-encoding' VALUE
------------------- -----------------------------
UTF-16LE with BOM UTF-16LE-BOM
UTF-16BE with BOM UTF-16
UTF-16LE no BOM UTF-16LE
UTF-16BE no BOM UTF-16BE
Why bother clarifying the documentation? Because These UTF-16 encodings are commonly found on Windows systems. Notepad supports the first two, and many Visual Studio project wizards add various files using these encodings as well. Older versions of PowerShell saved new .ps1 scripts using UTF-16BE with BOM as the default encoding.
Also, the current .gitattributes documentation makes frequent reference to "UTF-16" as an encoding but fails to be clear that the working-tree-encoding value "UTF-16" is now only for UTF-16BE with BOM. It would be easy to assume that the working-tree-encoding value "UTF-16" meant any UTF-16 file with a BOM (either LE or BE), which was the original meaning of this value before UTF-16LE-BOM was added to Git.
Finally, I am not sure how to use git add --renormalize to correct a UTF-16 file that was previously added incorrectly (i.e. with a missing or incorrect working-tree-encoding entry in .gitattributes). The git add documentation at https://git-scm.com/docs/git-add implies 'renormalize' resets only the end-of-line values; however, I suspect it also re-converts text encoding when a working-tree-encoding property is set. It would be helpful to know one way or the other.
- Michael Litwak
-----Original Message-----
From: Torsten Bögershausen <tboegi@web.de>
Sent: Friday, January 12, 2024 11:43 PM
To: Michael Litwak <michael.litwak@nuix.com>
Cc: brian m. carlson <sandals@crustytoothpaste.net>; git@vger.kernel.org
Subject: [EXTERNAL]Re: Suggested clarification for .gitattributes reference documentation
[You don't often get email from tboegi@web.de. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
On Sat, Jan 13, 2024 at 02:56:27AM +0000, Michael Litwak wrote:
> I just installed Git for Windows 2.43.0 and noticed the installer offers three options for altering the PATH:
>
> 1) Run git from git bash only
>
> 2) Run git from git bash, cmd.exe and PowerShell (RECOMMENDED)
>
> 3) Run git from git bash, cmd.exe and PowerShell with optional utilities (warning: will override find, sort and other system utilities).
>
> It turns out iconv.exe is accessible from cmd.exe (Command Prompt) only when you take the third option. But iconv.exe is NOT optional. It is required for git to deal with UTF-16LE with BOM text conversions (and probably for numerous other encoding conversions).
Plese wait a second - and thanks for bringing this up.
To my knowledge the binary iconv.exe (or just iconv under non-Windows) is never called from Git itself.
Git is using iconv_open() and friends, which are all inside a library, either the C-library "libc", or "libiconv"
(not 100% sure about the naming here)
iconv.exe is not needed in everyday life, or is it ?
If yes, when ?
iconv.exe is used when you run the test-suite, to verify what Git is doing.
Could you elaborate a little bit more,
when iconv.exe is missing, and what is happening, please ?
>
> But when PATH option #2 is chosen, and iconv.exe is unreachable from a Windows Command Prompt, the git commands which call upon iconv.exe do NOT indicate the error. The call to iconv.exe fails silently. It is only later after you commit, push and clone the repo again that you see the encoding failures.
>
> And the warning about overriding find and sort must be taken with a grain of salt, since the Windows versions of those programs are accessed via a Windows folder which appears earlier in the PATH.
>
> So this Git for Windows installer screen is misleading. And perhaps iconv.exe should be relocated so it is accessible even when PATH option #2 is chosen. I intend to submit an issue on the Git for Windows issue tracker regarding this. I'll also submit an issue about the lack of an error when running 'git add' for a UTF-16LE with BOM file under PATH option #2.
>
> Thanks,
> - Michael
>
[]
CAUTION:This email originated from outside of Nuix. Do not click links or open attachments unless you recognise the sender and know the content is safe.
^ permalink raw reply
* Re: Suggested clarification for .gitattributes reference documentation
From: brian m. carlson @ 2024-01-16 2:06 UTC (permalink / raw)
To: Michael Litwak
Cc: Torsten Bögershausen, Matthias Aßhauer,
git@vger.kernel.org
In-Reply-To: <SJ0PR10MB5693A19B0B66F47B2A985739FA732@SJ0PR10MB5693.namprd10.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 6194 bytes --]
On 2024-01-16 at 00:19:20, Michael Litwak wrote:
> As for documentation clarifications for the .gitattributes manpage at
> https://git-scm.com/docs/gitattributes, I still suggest adding an
> explicit example for UTF-16LE with BOM, and/or adding a table listing
> which working-tree-encoding value to use for each of the following
> UTF-16 text encodings:
>
> ENCODING 'working-tree-encoding' VALUE
> ------------------- -----------------------------
> UTF-16LE with BOM UTF-16LE-BOM
I should point out that this encoding, while very common on Windows, is
also nonstandard. The standard says that UTF-16LE and UTF-16BE don't
include a BOM and are always the respective endianness. UTF-16 can have
a BOM or not, and if it doesn't, it's big-endian.
There is no standard-conforming way to force the use of little-endian
with a BOM. The problem is that many Windows programs insist on the
BOM, but also refuse to read big-endian data in violation of the
standard[0]. That's why this nonstandard variant exists in Git.
I'll also note that this particular nonstandard variant is essentially
impossible to encode reliably on Unix outside of Git because it's
nonstandard, so it's an extremely unportable choice. In fact, I'm not
aware of _any_ tool on my Debian system other than Git that will
guarantee a UTF-16 little-endian stream with BOM. My editor (Neovim)
certainly doesn't. (Apparently Emacs, which is not on my system, may
permit that, which does not surprise me in the least.)
> UTF-16BE with BOM UTF-16
It's a little more complicated than that. "UTF-16" would allow UTF-16
big-endian with BOM, UTF-16 little-endian with BOM, or UTF-16 big-endian
without BOM. In other words, UTF-16 is big-endian by default and
otherwise requires a BOM, which may be included even if not required.
A reader must handle every variant of this, and must honour the BOM if
set and default to big-endian if not. A writer may write whichever
variant pleases it most as long as it's consistent within the same
message.
> UTF-16LE no BOM UTF-16LE
> UTF-16BE no BOM UTF-16BE
I think the addition of this table is too much. UTF-16LE-BOM is common
on Windows, and the rest are substantially less common. It's also very
difficult to explain in a table what "UTF-16" means in an understandable
way. And I also think it's also pretty clear that users should be using
UTF-8 without BOM where possible.
We do already mention both UTF-16, UTF-16LE, and UTF-16LE-BOM as options
in the gitattributes manual page, and it's up to the user to know what
their program wants and supports if that's not UTF-8. (I would say that
the user wants a new program that _does_ support UTF-8, but perhaps I'm
being unrealistically harsh.) I agree it's difficult because the
documentation usually doesn't indicate what's supported and all the
variants are hard to understand, but that's a huge part of the reason
that we recommend UTF-8.
I'll also add that in general, when you do have Unix systems that read
or write data in UTF-16, they handle every variant correctly. Thus, the
practical choice if you steadfastly refuse to use UTF-8 is either
UTF-16LE-BOM (if your Windows program has the bug I mentioned above) or
UTF-16, both of which we mention already in the manual page.
I'm explicitly ignoring non-file contexts here, where one may use
UTF-16LE or UTF-16BE, but those are substantially less common in actual
files, which is what this feature describes.
> Why bother clarifying the documentation? Because These UTF-16
> encodings are commonly found on Windows systems. Notepad supports the
> first two, and many Visual Studio project wizards add various files
> using these encodings as well. Older versions of PowerShell saved new
> .ps1 scripts using UTF-16BE with BOM as the default encoding.
True, but Notepad also supports UTF-8 and has for quite a while.
According to the Powershell documentation[1], there is no portable
character set option for non-ASCII characters, so in general it's
impossible to know. I suspect that a simple "UTF-16" will be fine here,
though, since it clearly doesn't have the bug mentioned above.
> Also, the current .gitattributes documentation makes frequent
> reference to "UTF-16" as an encoding but fails to be clear that the
> working-tree-encoding value "UTF-16" is now only for UTF-16BE with
> BOM. It would be easy to assume that the working-tree-encoding value
> "UTF-16" meant any UTF-16 file with a BOM (either LE or BE), which was
> the original meaning of this value before UTF-16LE-BOM was added to
> Git.
As I said, your statement isn't correct. That's what libiconv does on
Windows. On Linux, glibc uses a little-endian variant with BOM on
little-endian machines. musl, if memory serves me, always uses
big-endian without a BOM. All of those are valid encodings, and a
UTF-16 reader must handle all of them.
> Finally, I am not sure how to use git add --renormalize to correct a
> UTF-16 file that was previously added incorrectly (i.e. with a missing
> or incorrect working-tree-encoding entry in .gitattributes). The git
> add documentation at https://git-scm.com/docs/git-add implies
> 'renormalize' resets only the end-of-line values; however, I suspect
> it also re-converts text encoding when a working-tree-encoding
> property is set. It would be helpful to know one way or the other.
It does indeed affect the working-tree-encoding. If you wanted to send
an inline patch created with git format-patch, it would probably be
welcome to mention that. However, because in this project we typically
scratch our own itch, if you don't send one, it's likely nobody else
will, either.
[0] https://datatracker.ietf.org/doc/html/rfc2781 § 4.1: “All
applications that process text with the "UTF-16" charset label
MUST be able to interpret both big- endian and little-endian text.”
[1] https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_character_encoding?view=powershell-7.4
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply
* Re: [PATCH] advice: allow disabling the automatic hint in advise_if_enabled()
From: Junio C Hamano @ 2024-01-16 4:56 UTC (permalink / raw)
To: Jeff King; +Cc: Rubén Justo, Git List, Taylor Blau, Dragan Simic
In-Reply-To: <20240113073828.GB657764@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> For a tri-state we often use "-1" for "not specified". That has the nice
> side effect in this case that "if (level)" shows the advice (that works
> because "unspecified" and "explicitly true" both show the advice. And
> then "if (level < 0)" is used for just the hint. But maybe that is too
> clever/fragile.
>
> Of course that means that all of the initializers have to use "-1"
> explicitly. So zero-initialization is sometimes nice, too.
;-) 100% agreed.
^ permalink raw reply
* Custom merge drivers: accessing the pathnames and revisions of the files being merged
From: Antonin Delpeuch @ 2024-01-16 8:44 UTC (permalink / raw)
To: git
Hello,
Custom merge drivers [1] provide a convenient way to extend Git's merge
algorithms. But as far as I can tell, there is no way to access the
pathnames and revisions of the files to merge. Those would be very
useful to generate informative conflict markers, like the built-in merge
strategies do.
For instance, when merging with the ort strategy and diff3 conflict
style, I get markers such as:
<<<<<<< HEAD:main/my_file.txt
…
||||||| 194c4190b:main/my_file.txt
…
=======
…
>>>>>>> origin/branch:main/my_renamed_file.txt
Those strings at the end of conflict markers, with the revision and the
pathname, are very useful to understand which parts are coming from where.
When implementing a custom merge driver, I don't see how to access this
information to include it in the conflict markers. Custom merge drivers
are typically invoked on temporary files generated by Git with
uninformative paths, such as ".merge_file_NgiKjJ".
Therefore, my merge driver is only able to generate conflicts which look
like this:
<<<<<<< .merge_file_NgiKjJ
…
||||||| .merge_file_D1XtCW
…
=======
…
>>>>>>> .merge_file_WbmrBA
Of course, in a given rebase/merge session, the order in which the
conflicting parts will be presented will remain consistent, so I will
generally be able to remember which revision each part corresponds to,
but it's still a mental load I would ideally try to avoid. Also, if the
rename detection heuristics have false positives, then I can get merge
conflicts which come from unrelated files: in that case it is very
useful to see the pathnames, to understand this situation better.
So, I wonder: would people be open to exposing additional parameters to
merge drivers? For instance we could add parameters "%X", "%Y" "%Z" to
expose those "revision:pathname" strings for each part. (I think colons
cannot be part of revision names, so this can be parsed unambiguously by
the custom merge driver to recover the revision and pathname
independently, if needed.)
I would be happy to submit a patch for this if you think this makes
sense. If it is already possible to access this information in another
way, I would like to work on documenting that.
Best wishes,
Antonin
[1]: https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
^ permalink raw reply
* Re: [PATCH v4 1/4] transport-helper: no connection restriction in connect_helper
From: Jiang Xin @ 2024-01-16 9:04 UTC (permalink / raw)
To: Linus Arver; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <owlyy1cvhua5.fsf@fine.c.googlers.com>
On Fri, Jan 12, 2024 at 3:42 PM Linus Arver <linusa@google.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > When commit b236752a (Support remote archive from all smart transports,
> > 2009-12-09) added "remote archive" support for "smart transports", it
> > was for transport that supports the ".connect" method. The
> > "connect_helper()" function protected itself from getting called for a
> > transport without the method before calling process_connect_service(),
>
> OK.
>
> > which did not work with such a transport.
>
> How about 'which only worked with the ".connect" method.' ?
>
> >
> > Later, commit edc9caf7 (transport-helper: introduce stateless-connect,
> > 2018-03-15) added a way for a transport without the ".connect" method
> > to establish a "stateless" connection in protocol-v2, which
>
> s/which/where
>
> > process_connect_service() was taught to handle the "stateless"
> > connection,
>
> I think using 'the ".stateless_connect" method' is more consistent with
> the rest of this text.
>
> > making the old safety valve in its caller that insisted
> > that ".connect" method must be defined too strict, and forgot to loosen
> > it.
>
> I think just "...making the old protection too strict. But edc9caf7
> forgot to adjust this protection accordingly." is simpler to read.
Thanks for the above suggestions, and will update in next reroll.
> > Remove the restriction in the "connect_helper()" function and give the
> > function "process_connect_service()" the opportunity to establish a
> > connection using ".connect" or ".stateless_connect" for protocol v2. So
> > we can connect with a stateless-rpc and do something useful. E.g., in a
> > later commit, implements remote archive for a repository over HTTP
> > protocol.
> >
> > Helped-by: Junio C Hamano <gitster@pobox.com>
> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> > ---
> > transport-helper.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/transport-helper.c b/transport-helper.c
> > index 49811ef176..2e127d24a5 100644
> > --- a/transport-helper.c
> > +++ b/transport-helper.c
> > @@ -662,8 +662,6 @@ static int connect_helper(struct transport *transport, const char *name,
> >
> > /* Get_helper so connect is inited. */
> > get_helper(transport);
> > - if (!data->connect)
> > - die(_("operation not supported by protocol"));
>
> Should we still terminate early here if both data->connect and
> data->stateless_connect are not truthy? It would save a few CPU cycles,
> but even better, remain true to the the original intent of the code.
> Maybe there was a really good reason to terminate early here that we're
> not aware of?
>
It's not necessary to check data->connect here, because it will
terminate if fail to connect by calling the function
"process_connect_service()".
> But also, what about the case where both are enabled? Should we print an
> error message? (Maybe this concern is outside the scope of this series?)
In the function "process_connect_service()", we can see that "connect"
has a higher priority than "stateless-connect".
>
> > if (!process_connect_service(transport, name, exec))
> > die(_("can't connect to subservice %s"), name);
Regardless of whether "connect" or "stateless-connect" is used, the
function process_connect_service() will return 1 if the connection is
successful. If the connection fails, it will terminate here.
^ permalink raw reply
* Re: [PATCH v2 0/6] worktree: initialize refdb via ref backends
From: Karthik Nayak @ 2024-01-16 9:17 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Junio C Hamano, Eric Sunshine
In-Reply-To: <cover.1704705733.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 543 bytes --]
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this is the second version of my patch series that refactors the
> initialization of worktree refdbs to use the refs API.
>
> Changes compared to v1:
>
> - Improved commit messages.
>
> - This series is now based on `ps/refstorage-extension`, 1b2234079b
> (t9500: write "extensions.refstorage" into config, 2023-12-29).
> While there is no functional dependency between those series,
> merging both topics causes a merge conflict.
>
This looks good to me now. Thanks Patrick!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/4] transport-helper: call do_take_over() in connect_helper
From: Jiang Xin @ 2024-01-16 9:41 UTC (permalink / raw)
To: Linus Arver; +Cc: Git List, Junio C Hamano, Jiang Xin
In-Reply-To: <owlyttnjhtmz.fsf@fine.c.googlers.com>
On Fri, Jan 12, 2024 at 3:56 PM Linus Arver <linusa@google.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > After successfully connecting to the smart transport by calling
> > process_connect_service() in connect_helper(), run do_take_over() to
> > replace the old vtable with a new one which has methods ready for the
> > smart transport connection.
> >
> >
> > The connect_helper() function is used as the connect method of the
> > vtable in "transport-helper.c", and it is called by transport_connect()
> > in "transport.c" to setup a connection. The only place that we call
> > transport_connect() so far is in "builtin/archive.c". Without running
> > do_take_over(), it may fail to call transport_disconnect() in
> > run_remote_archiver() of "builtin/archive.c". This is because for a
> > stateless connection or a service like "git-upload-pack-archive", the
>
> There is "git-upload-pack" and "git-upload-archive". Which one did you
> mean here? Or did you mean both?
>
Should be "git-upload-archive".
> > remote helper may receive a SIGPIPE signal and exit early. To have a
> > graceful disconnect method by calling do_take_over() will solve this
> > issue.
>
> Are you saying that this patch fixes an existing bug? That is, is this
> patch independent of the first patch (transport-helper: no connection
> restriction in connect_helper) in this series?
>
> > The subsequent commit will introduce remote archive over a stateless-rpc
> > connection.
>
> Does the next commit depend on this patch? If not, I think you can drop
> this paragraph.
One test case in next commit will break without this patch. I will
move this patch to the end of this series.
^ permalink raw reply
* Re: Strange behaviour when pushing a commit object to remote's refs/HEAD
From: Karthik Nayak @ 2024-01-16 9:54 UTC (permalink / raw)
To: Pratyush Yadav, git
In-Reply-To: <mafs0fryypg82.fsf@yadavpratyush.com>
[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]
Pratyush Yadav <me@yadavpratyush.com> writes:
> Hi,
>
Hello,
> I ran into a strange Magit bug, where when I ran magit-show-refs on a
> particular repo it threw an error. The details of the Magit bug are not
> very interesting, but when attempting to reproduce it, I also saw git
> misbehaving for such repos.
>
> The strange behaviour happens when you push a commit object to remote's
> refs/HEAD instead of pushing a symbolic ref. Such a repository can be
> found at https://github.com/prati0100/magit-reproducer. I roughly used
> the below steps to create such a repo:
>
> $ git init
> $ echo 1 > foo && git add foo && git commit
> $ echo 2 > bar && git add bar && git commit
> $ git push
> $ git checkout 79264c3
> $ echo 2.1 > bar && git add bar && git commit
> $ git push origin 707a3d5:refs/heads/HEAD
>
Just to note here that pushing to "refs/heads/HEAD" is not actually
updating the remote repositories $GIT_DIR/HEAD file, rather it creates a
new reference $GIT_DIR/refs/heads/HEAD.
With this understanding you'll see that this is not a bug, because the
remote HEAD was never updated, but only a new branch called HEAD was
created [0].
> Now with such a repo, if you do `git log --all --oneline` it would look
> something like:
>
> 707a3d5 (origin/HEAD) 2.1
> 86e1c97 (HEAD -> main, origin/main) 2
> 79264c3 1
>
> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>
> ,origin,refs/remotes/origin/HEAD,2.1
> ,origin/main,refs/remotes/origin/main,2
>
> All well and good so far. Now delete the repo and attempt to clone it.
> This time `git log --all --oneline` gives:
>
> 86e1c97 (HEAD -> main, origin/main, origin/HEAD) 2
> 79264c3 1
>
This is expected since you cloned the repository and you got the default
branch 'main'.
> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>
> origin/main,origin,refs/remotes/origin/HEAD,2
> ,origin/main,refs/remotes/origin/main,2
>
> So suddenly the remote's HEAD becomes origin/main (symbolic ref) and the
> commit (707a3d5, "2.1") is nowhere to be found. It neither shows up in
> `git rev-list --all` nor in `git log --all`. The files and trees
> associated with it also do not show up in `git rev-list --all --object`.
Because rev-list's `--all`, iterates over all refs. Since you only
cloned, the HEAD branch is not pulled.
Everything else is a consequence of the subtle but important difference
between updating $GIT_DIR/HEAD vs creating $GIT_DIR/refs/heads/HEAD.
[0]: https://github.com/prati0100/magit-reproducer/branches/all
Thanks,
Karthik
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [Outreachy][PATCH] Port helper/test-advise.c to unit-tests/t-advise.c
From: Rubén Justo @ 2024-01-16 11:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Achu Luma, git, christian.couder, Christian Couder
In-Reply-To: <6b6b455e-26b8-442e-828e-506f9a152407@gmail.com>
On 15-ene-2024 20:24:47, Rubén Justo wrote:
> - check_str(actual.buf, expect);
> + check_str_len(actual.buf, expect, strlen(expect));
> + if (!conf_val && skip_prefix(actual.buf, expect, &hint))
> + check_str_len(hint, expect_hint_msg, strlen(expect_hint_msg));
> strbuf_release(&actual);
>
> if (!check(remove(out_file) == 0))
>
> This implies a new check_str_len() helper, which I'm not including here
> but it's a trivial copy of check_str() but using strncmp().
>
> Maybe we can turn the screw a little more.
>
> I'm still not sure of the value in the changes in this series, though.
I hope, no one has wasted time with the code above. It is not testing
correctly the conditions being probed in t-advice.c.
Take it as a way to see if this is how we want to avoid multiple
instances of similar literals that might be tempting to refactor.
^ permalink raw reply
* Re: Strange behaviour when pushing a commit object to remote's refs/HEAD
From: Pratyush Yadav @ 2024-01-16 11:33 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Pratyush Yadav, git
In-Reply-To: <CAOLa=ZS8YBhzaYx=9016KxErsMsazsF09rcuPs=-WpEGjV+ruw@mail.gmail.com>
On Tue, Jan 16 2024, Karthik Nayak wrote:
> Pratyush Yadav <me@yadavpratyush.com> writes:
>
>> Hi,
>>
>
> Hello,
>
>> I ran into a strange Magit bug, where when I ran magit-show-refs on a
>> particular repo it threw an error. The details of the Magit bug are not
>> very interesting, but when attempting to reproduce it, I also saw git
>> misbehaving for such repos.
>>
>> The strange behaviour happens when you push a commit object to remote's
>> refs/HEAD instead of pushing a symbolic ref. Such a repository can be
>> found at https://github.com/prati0100/magit-reproducer. I roughly used
>> the below steps to create such a repo:
>>
>> $ git init
>> $ echo 1 > foo && git add foo && git commit
>> $ echo 2 > bar && git add bar && git commit
>> $ git push
>> $ git checkout 79264c3
>> $ echo 2.1 > bar && git add bar && git commit
>> $ git push origin 707a3d5:refs/heads/HEAD
>>
>
> Just to note here that pushing to "refs/heads/HEAD" is not actually
> updating the remote repositories $GIT_DIR/HEAD file, rather it creates a
> new reference $GIT_DIR/refs/heads/HEAD.
Yes, that is what I would also expect. I checked one of the Git servers
we have and this is exactly what happens. $GIT_DIR/HEAD is a symref
pointing to refs/heads/main and $GIT_DIR/refs/heads/HEAD points to the
commit. But behaviour from client side is not consistent.
>
> With this understanding you'll see that this is not a bug, because the
> remote HEAD was never updated, but only a new branch called HEAD was
> created [0].
GitHub thinks so but try opening the branch. It won't show you the
commit (707a3d5, "2.1") but instead shows you 86e1c97 ("2"). So
something is wrong _at least_ with Github.
>
>> Now with such a repo, if you do `git log --all --oneline` it would look
>> something like:
>>
>> 707a3d5 (origin/HEAD) 2.1
>> 86e1c97 (HEAD -> main, origin/main) 2
>> 79264c3 1
>>
>> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>>
>> ,origin,refs/remotes/origin/HEAD,2.1
>> ,origin/main,refs/remotes/origin/main,2
>>
>> All well and good so far. Now delete the repo and attempt to clone it.
>> This time `git log --all --oneline` gives:
>>
>> 86e1c97 (HEAD -> main, origin/main, origin/HEAD) 2
>> 79264c3 1
>>
>
> This is expected since you cloned the repository and you got the default
> branch 'main'.
No.
First, if I clone a repo with multiple branches (say
https://github.com/prati0100/git-gui) I get _all_ the remote branches.
Yet here I clearly don't get the so called "HEAD" branch. This is not
expected behaviour.
Second, git really does misunderstand refs/remotes/origin/HEAD. For
example, when running git for-each-ref command with the clone method, I
get:
origin/main,origin,refs/remotes/origin/HEAD,2
So it clearly thinks refs/remotes/origin/HEAD is at 86e1c97 ("2"). Or,
to be more specific, it thinks the ref points to origin/main which is at
86e1c97 ("2"). But we set it at (707a3d5, "2.1"). So it tells me the
wrong thing. Now if I do the git remote add && git remote update method,
git for-each-ref says:
,origin,refs/remotes/origin/HEAD,2.1
So now it thinks refs/remotes/origin/HEAD points at (707a3d5, "2.1"). I
do not see it as expected behaviour.
We can also see this when inspecting the contents of
.git/refs/remotes/origin/HEAD. With clone it says:
ref: refs/remotes/origin/main
With git remote add && git remote update it says:
707a3d587c61c089710e3924eb63a51763b5a4c8
The same ref points to different places based on how you pull the repo.
Looking deeper, if you clone a repo that does not have a branch called
"HEAD" (like git-gui), git creates a file in
.git/refs/remotes/origin/HEAD that says:
ref: refs/remotes/origin/master
So it certainly seems to use refs/remotes/origin/HEAD to point to the
remote's HEAD, and not as a regular branch.
I find this to be inconsistent behaviour on git's part and do not think
it is (or should be) expected behaviour.
>
>> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>>
>> origin/main,origin,refs/remotes/origin/HEAD,2
>> ,origin/main,refs/remotes/origin/main,2
>>
>> So suddenly the remote's HEAD becomes origin/main (symbolic ref) and the
>> commit (707a3d5, "2.1") is nowhere to be found. It neither shows up in
>> `git rev-list --all` nor in `git log --all`. The files and trees
>> associated with it also do not show up in `git rev-list --all --object`.
>
>
> Because rev-list's `--all`, iterates over all refs. Since you only
> cloned, the HEAD branch is not pulled.
Why not? When you clone all branches should get pulled.
>
> Everything else is a consequence of the subtle but important difference
> between updating $GIT_DIR/HEAD vs creating $GIT_DIR/refs/heads/HEAD.
>
> [0]: https://github.com/prati0100/magit-reproducer/branches/all
--
Regards,
Pratyush Yadav
^ permalink raw reply
* Re: Strange behaviour when pushing a commit object to remote's refs/HEAD
From: Karthik Nayak @ 2024-01-16 13:24 UTC (permalink / raw)
To: Pratyush Yadav; +Cc: git
In-Reply-To: <mafs0a5p5pl6y.fsf@yadavpratyush.com>
[-- Attachment #1: Type: text/plain, Size: 5062 bytes --]
Pratyush Yadav <me@yadavpratyush.com> writes:
>> Just to note here that pushing to "refs/heads/HEAD" is not actually
>> updating the remote repositories $GIT_DIR/HEAD file, rather it creates a
>> new reference $GIT_DIR/refs/heads/HEAD.
>
> Yes, that is what I would also expect. I checked one of the Git servers
> we have and this is exactly what happens. $GIT_DIR/HEAD is a symref
> pointing to refs/heads/main and $GIT_DIR/refs/heads/HEAD points to the
> commit. But behaviour from client side is not consistent.
>
What is the non _consistent_ part?
>>
>> With this understanding you'll see that this is not a bug, because the
>> remote HEAD was never updated, but only a new branch called HEAD was
>> created [0].
>
> GitHub thinks so but try opening the branch. It won't show you the
> commit (707a3d5, "2.1") but instead shows you 86e1c97 ("2"). So
> something is wrong _at least_ with Github.
>
I don't know how GitHub operates, but I'm guessing because there is
ambiguity between a branch called HEAD and the actual HEAD. So this is
probably the reason.
>>
>>> Now with such a repo, if you do `git log --all --oneline` it would look
>>> something like:
>>>
>>> 707a3d5 (origin/HEAD) 2.1
>>> 86e1c97 (HEAD -> main, origin/main) 2
>>> 79264c3 1
>>>
>>> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>>>
>>> ,origin,refs/remotes/origin/HEAD,2.1
>>> ,origin/main,refs/remotes/origin/main,2
>>>
>>> All well and good so far. Now delete the repo and attempt to clone it.
>>> This time `git log --all --oneline` gives:
>>>
>>> 86e1c97 (HEAD -> main, origin/main, origin/HEAD) 2
>>> 79264c3 1
>>>
>>
>> This is expected since you cloned the repository and you got the default
>> branch 'main'.
>
> No.
>
> First, if I clone a repo with multiple branches (say
> https://github.com/prati0100/git-gui) I get _all_ the remote branches.
> Yet here I clearly don't get the so called "HEAD" branch. This is not
> expected behaviour.
>
You're right, I meant to say that the remote branches don't have the
corresponding local branches. But that does not matter here.
I'm not saying that there is a path for git to work properly when
creating a branch called "HEAD". It's just that "HEAD" is more of a
reserved word for git and creating a branch with the same name has
unintended effects.
> Second, git really does misunderstand refs/remotes/origin/HEAD. For
> example, when running git for-each-ref command with the clone method, I
> get:
>
> origin/main,origin,refs/remotes/origin/HEAD,2
>
> So it clearly thinks refs/remotes/origin/HEAD is at 86e1c97 ("2"). Or,
> to be more specific, it thinks the ref points to origin/main which is at
> 86e1c97 ("2"). But we set it at (707a3d5, "2.1"). So it tells me the
> wrong thing. Now if I do the git remote add && git remote update method,
> git for-each-ref says:
>
> ,origin,refs/remotes/origin/HEAD,2.1
>
This is one of those ambiguities, we store HEAD for remotes as
$GIT_DIR/refs/remotes/<remote>/HEAD
and remote branches as
$GIT_DIR/refs/remotes/<remote>/<branch>
So what happens if there is a branch named HEAD? This is the problem
you're facing...
> So now it thinks refs/remotes/origin/HEAD points at (707a3d5, "2.1"). I
> do not see it as expected behaviour.
>
> We can also see this when inspecting the contents of
> .git/refs/remotes/origin/HEAD. With clone it says:
>
> ref: refs/remotes/origin/main
>
> With git remote add && git remote update it says:
>
> 707a3d587c61c089710e3924eb63a51763b5a4c8
>
> The same ref points to different places based on how you pull the repo.
>
> Looking deeper, if you clone a repo that does not have a branch called
> "HEAD" (like git-gui), git creates a file in
> .git/refs/remotes/origin/HEAD that says:
>
> ref: refs/remotes/origin/master
>
> So it certainly seems to use refs/remotes/origin/HEAD to point to the
> remote's HEAD, and not as a regular branch.
>
> I find this to be inconsistent behaviour on git's part and do not think
> it is (or should be) expected behaviour.
>
Maybe we should explicitly mention that using HEAD as the branch name
has unintended effects and should be avoided.
>>
>>> And running `git for-each-ref --format='%(symref:short),%(refname:short),%(refname),%(subject)' refs/remotes/origin` gives:
>>>
>>> origin/main,origin,refs/remotes/origin/HEAD,2
>>> ,origin/main,refs/remotes/origin/main,2
>>>
>>> So suddenly the remote's HEAD becomes origin/main (symbolic ref) and the
>>> commit (707a3d5, "2.1") is nowhere to be found. It neither shows up in
>>> `git rev-list --all` nor in `git log --all`. The files and trees
>>> associated with it also do not show up in `git rev-list --all --object`.
>>
>>
>> Because rev-list's `--all`, iterates over all refs. Since you only
>> cloned, the HEAD branch is not pulled.
>
> Why not? When you clone all branches should get pulled.
>
I think I jumped too quick here, it is because the branch HEAD is never
realized locally as I explained above.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH v5 0/6] support remote archive via stateless transport
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1702562879.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
# Changes since v4
1. Change commit messages and order of commits.
2. Split the last commit of v4 into three seperate commits.
# range-diff v4...v5
1: da80391037 ! 1: f3fef46c05 transport-helper: no connection restriction in connect_helper
@@ Commit message
was for transport that supports the ".connect" method. The
"connect_helper()" function protected itself from getting called for a
transport without the method before calling process_connect_service(),
- which did not work with such a transport.
+ which only worked with the ".connect" method.
Later, commit edc9caf7 (transport-helper: introduce stateless-connect,
2018-03-15) added a way for a transport without the ".connect" method
- to establish a "stateless" connection in protocol-v2, which
- process_connect_service() was taught to handle the "stateless"
- connection, making the old safety valve in its caller that insisted
- that ".connect" method must be defined too strict, and forgot to loosen
- it.
+ to establish a "stateless" connection in protocol-v2, where
+ process_connect_service() was taught to handle the ".stateless_connect"
+ method, making the old protection too strict. But commit edc9caf7 forgot
+ to adjust this protection accordingly.
Remove the restriction in the "connect_helper()" function and give the
function "process_connect_service()" the opportunity to establish a
@@ Commit message
protocol.
Helped-by: Junio C Hamano <gitster@pobox.com>
+ Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
## transport-helper.c ##
-: ---------- > 2: 6be331b22d remote-curl: supports git-upload-archive service
-: ---------- > 3: aabc8e1a2a transport-helper: protocol-v2 supports upload-archive
4: a21a80dae9 ! 4: fdab4abb43 archive: support remote archive from stateless transport
@@ Metadata
Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
## Commit message ##
- archive: support remote archive from stateless transport
+ http-backend: new rpc-service for git-upload-archive
- Even though we can establish a stateless connection, we still cannot
- archive the remote repository using a stateless HTTP protocol. Try the
- following steps to make it work.
+ Add new rpc-service "upload-archive" in http-backend to add server side
+ support for remote archive over HTTP/HTTPS protocols.
- 1. Add support for "git-upload-archive" service in "http-backend".
-
- 2. Use the URL ".../info/refs?service=git-upload-pack" to detect the
- protocol version, instead of use the "git-upload-archive" service.
-
- 3. "git-archive" does not expect to see protocol version and
- capabilities when connecting to remote-helper, so do not send them
- in "remote-curl.c" for the "git-upload-archive" service.
+ Also add new test cases in t5003. In the test case "archive remote http
+ repository", git-archive exits with a non-0 exit code even though we
+ create the archive correctly. It will be fixed in a later commit.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
if (ret)
... (omitted) ...
3: 870dc5fd21 ! 5: 6ac0c8e105 transport-helper: call do_take_over() in connect_helper
@@ Commit message
After successfully connecting to the smart transport by calling
process_connect_service() in connect_helper(), run do_take_over() to
replace the old vtable with a new one which has methods ready for the
- smart transport connection.
+ smart transport connection. This will fix the exit code of git-archive
+ in test case "archive remote http repository" of t5003.
The connect_helper() function is used as the connect method of the
vtable in "transport-helper.c", and it is called by transport_connect()
@@ Commit message
transport_connect() so far is in "builtin/archive.c". Without running
do_take_over(), it may fail to call transport_disconnect() in
run_remote_archiver() of "builtin/archive.c". This is because for a
- stateless connection or a service like "git-upload-pack-archive", the
+ stateless connection and a service like "git-upload-archive", the
remote helper may receive a SIGPIPE signal and exit early. To have a
graceful disconnect method by calling do_take_over() will solve this
issue.
- The subsequent commit will introduce remote archive over a stateless-rpc
- connection.
-
+ Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
+ ## t/t5003-archive-zip.sh ##
+@@ t/t5003-archive-zip.sh: test_expect_success 'remote archive does not work with protocol v1' '
+ '
+
+ test_expect_success 'archive remote http repository' '
+- test_must_fail git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
++ git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
+ --output=remote-http.zip HEAD &&
+ test_cmp_bin d.zip remote-http.zip
+ '
+
## transport-helper.c ##
@@ transport-helper.c: static int connect_helper(struct transport *transport, const char *name,
2: 2f7060f7c5 = 6: 423a89c593 transport-helper: call do_take_over() in process_connect
Jiang Xin (6):
transport-helper: no connection restriction in connect_helper
remote-curl: supports git-upload-archive service
transport-helper: protocol-v2 supports upload-archive
http-backend: new rpc-service for git-upload-archive
transport-helper: call do_take_over() in connect_helper
transport-helper: call do_take_over() in process_connect
http-backend.c | 13 ++++++++++---
remote-curl.c | 14 +++++++++++---
t/t5003-archive-zip.sh | 34 ++++++++++++++++++++++++++++++++++
transport-helper.c | 29 +++++++++++++----------------
4 files changed, 68 insertions(+), 22 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v5 1/6] transport-helper: no connection restriction in connect_helper
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
When commit b236752a (Support remote archive from all smart transports,
2009-12-09) added "remote archive" support for "smart transports", it
was for transport that supports the ".connect" method. The
"connect_helper()" function protected itself from getting called for a
transport without the method before calling process_connect_service(),
which only worked with the ".connect" method.
Later, commit edc9caf7 (transport-helper: introduce stateless-connect,
2018-03-15) added a way for a transport without the ".connect" method
to establish a "stateless" connection in protocol-v2, where
process_connect_service() was taught to handle the ".stateless_connect"
method, making the old protection too strict. But commit edc9caf7 forgot
to adjust this protection accordingly.
Remove the restriction in the "connect_helper()" function and give the
function "process_connect_service()" the opportunity to establish a
connection using ".connect" or ".stateless_connect" for protocol v2. So
we can connect with a stateless-rpc and do something useful. E.g., in a
later commit, implements remote archive for a repository over HTTP
protocol.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
transport-helper.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 49811ef176..2e127d24a5 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -662,8 +662,6 @@ static int connect_helper(struct transport *transport, const char *name,
/* Get_helper so connect is inited. */
get_helper(transport);
- if (!data->connect)
- die(_("operation not supported by protocol"));
if (!process_connect_service(transport, name, exec))
die(_("can't connect to subservice %s"), name);
--
2.43.0
^ permalink raw reply related
* [PATCH v5 2/6] remote-curl: supports git-upload-archive service
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Add new service (git-upload-archive) support in remote-curl, so we can
support remote archive over HTTP/HTTPS protocols. Differences between
git-upload-archive and other serices:
1. The git-archive command does not expect to see protocol version and
capabilities when connecting to remote-helper, so do not send them
in remote-curl for the git-upload-archive service.
2. We need to detect protocol version by calling discover_refs(),
Fallback to use the git-upload-pack service (which, like
git-upload-archive, is a read-only operation) to discover protocol
version.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
remote-curl.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index ef05752ca5..ce6cb8ac05 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1447,8 +1447,14 @@ static int stateless_connect(const char *service_name)
* establish a stateless connection, otherwise we need to tell the
* client to fallback to using other transport helper functions to
* complete their request.
+ *
+ * The "git-upload-archive" service is a read-only operation. Fallback
+ * to use "git-upload-pack" service to discover protocol version.
*/
- discover = discover_refs(service_name, 0);
+ if (!strcmp(service_name, "git-upload-archive"))
+ discover = discover_refs("git-upload-pack", 0);
+ else
+ discover = discover_refs(service_name, 0);
if (discover->version != protocol_v2) {
printf("fallback\n");
fflush(stdout);
@@ -1486,9 +1492,11 @@ static int stateless_connect(const char *service_name)
/*
* Dump the capability listing that we got from the server earlier
- * during the info/refs request.
+ * during the info/refs request. This does not work with the
+ * "git-upload-archive" service.
*/
- write_or_die(rpc.in, discover->buf, discover->len);
+ if (strcmp(service_name, "git-upload-archive"))
+ write_or_die(rpc.in, discover->buf, discover->len);
/* Until we see EOF keep sending POSTs */
while (1) {
--
2.43.0
^ permalink raw reply related
* [PATCH v5 3/6] transport-helper: protocol-v2 supports upload-archive
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
We used to support only git-upload-pack service for protocol-v2. In
order to support remote archive over HTTP/HTTPS protocols, add new
service support for git-upload-archive in protocol-v2.
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
transport-helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index 2e127d24a5..6fe9f4f208 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -628,7 +628,8 @@ static int process_connect_service(struct transport *transport,
ret = run_connect(transport, &cmdbuf);
} else if (data->stateless_connect &&
(get_protocol_version_config() == protocol_v2) &&
- !strcmp("git-upload-pack", name)) {
+ (!strcmp("git-upload-pack", name) ||
+ !strcmp("git-upload-archive", name))) {
strbuf_addf(&cmdbuf, "stateless-connect %s\n", name);
ret = run_connect(transport, &cmdbuf);
if (ret)
--
2.43.0
^ permalink raw reply related
* [PATCH v5 4/6] http-backend: new rpc-service for git-upload-archive
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin, Eric Sunshine
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Add new rpc-service "upload-archive" in http-backend to add server side
support for remote archive over HTTP/HTTPS protocols.
Also add new test cases in t5003. In the test case "archive remote http
repository", git-archive exits with a non-0 exit code even though we
create the archive correctly. It will be fixed in a later commit.
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
http-backend.c | 13 ++++++++++---
t/t5003-archive-zip.sh | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index ff07b87e64..1ed1e29d07 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -38,6 +38,7 @@ struct rpc_service {
static struct rpc_service rpc_service[] = {
{ "upload-pack", "uploadpack", 1, 1 },
{ "receive-pack", "receivepack", 0, -1 },
+ { "upload-archive", "uploadarchive", 0, -1 },
};
static struct string_list *get_parameters(void)
@@ -639,10 +640,15 @@ static void check_content_type(struct strbuf *hdr, const char *accepted_type)
static void service_rpc(struct strbuf *hdr, char *service_name)
{
- const char *argv[] = {NULL, "--stateless-rpc", ".", NULL};
+ struct strvec argv = STRVEC_INIT;
struct rpc_service *svc = select_service(hdr, service_name);
struct strbuf buf = STRBUF_INIT;
+ strvec_push(&argv, svc->name);
+ if (strcmp(service_name, "git-upload-archive"))
+ strvec_push(&argv, "--stateless-rpc");
+ strvec_push(&argv, ".");
+
strbuf_reset(&buf);
strbuf_addf(&buf, "application/x-git-%s-request", svc->name);
check_content_type(hdr, buf.buf);
@@ -655,9 +661,9 @@ static void service_rpc(struct strbuf *hdr, char *service_name)
end_headers(hdr);
- argv[0] = svc->name;
- run_service(argv, svc->buffer_input);
+ run_service(argv.v, svc->buffer_input);
strbuf_release(&buf);
+ strvec_clear(&argv);
}
static int dead;
@@ -723,6 +729,7 @@ static struct service_cmd {
{"GET", "/objects/pack/pack-[0-9a-f]{64}\\.idx$", get_idx_file},
{"POST", "/git-upload-pack$", service_rpc},
+ {"POST", "/git-upload-archive$", service_rpc},
{"POST", "/git-receive-pack$", service_rpc}
};
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index fc499cdff0..6f85bd3463 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -239,4 +239,38 @@ check_zip with_untracked2
check_added with_untracked2 untracked one/untracked
check_added with_untracked2 untracked two/untracked
+# Test remote archive over HTTP protocol.
+#
+# Note: this should be the last part of this test suite, because
+# by including lib-httpd.sh, the test may end early if httpd tests
+# should not be run.
+#
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success "setup for HTTP protocol" '
+ cp -R bare.git "$HTTPD_DOCUMENT_ROOT_PATH/bare.git" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/bare.git" \
+ config http.uploadpack true &&
+ set_askpass user@host pass@host
+'
+
+setup_askpass_helper
+
+test_expect_success 'remote archive does not work with protocol v1' '
+ test_must_fail git -c protocol.version=1 archive \
+ --remote="$HTTPD_URL/auth/smart/bare.git" \
+ --output=remote-http.zip HEAD >actual 2>&1 &&
+ cat >expect <<-EOF &&
+ fatal: can${SQ}t connect to subservice git-upload-archive
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'archive remote http repository' '
+ test_must_fail git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
+ --output=remote-http.zip HEAD &&
+ test_cmp_bin d.zip remote-http.zip
+'
+
test_done
--
2.43.0
^ permalink raw reply related
* [PATCH v5 5/6] transport-helper: call do_take_over() in connect_helper
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
After successfully connecting to the smart transport by calling
process_connect_service() in connect_helper(), run do_take_over() to
replace the old vtable with a new one which has methods ready for the
smart transport connection. This will fix the exit code of git-archive
in test case "archive remote http repository" of t5003.
The connect_helper() function is used as the connect method of the
vtable in "transport-helper.c", and it is called by transport_connect()
in "transport.c" to setup a connection. The only place that we call
transport_connect() so far is in "builtin/archive.c". Without running
do_take_over(), it may fail to call transport_disconnect() in
run_remote_archiver() of "builtin/archive.c". This is because for a
stateless connection and a service like "git-upload-archive", the
remote helper may receive a SIGPIPE signal and exit early. To have a
graceful disconnect method by calling do_take_over() will solve this
issue.
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
t/t5003-archive-zip.sh | 2 +-
transport-helper.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh
index 6f85bd3463..961c6aac25 100755
--- a/t/t5003-archive-zip.sh
+++ b/t/t5003-archive-zip.sh
@@ -268,7 +268,7 @@ test_expect_success 'remote archive does not work with protocol v1' '
'
test_expect_success 'archive remote http repository' '
- test_must_fail git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
+ git archive --remote="$HTTPD_URL/auth/smart/bare.git" \
--output=remote-http.zip HEAD &&
test_cmp_bin d.zip remote-http.zip
'
diff --git a/transport-helper.c b/transport-helper.c
index 6fe9f4f208..91381be622 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -669,6 +669,8 @@ static int connect_helper(struct transport *transport, const char *name,
fd[0] = data->helper->out;
fd[1] = data->helper->in;
+
+ do_take_over(transport);
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v5 6/6] transport-helper: call do_take_over() in process_connect
From: Jiang Xin @ 2024-01-16 13:39 UTC (permalink / raw)
To: Git List, Junio C Hamano, Linus Arver; +Cc: Jiang Xin
In-Reply-To: <cover.1705411391.git.zhiyou.jx@alibaba-inc.com>
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
The existing pattern among all callers of process_connect() seems to be
if (process_connect(...)) {
do_take_over();
... dispatch to the underlying method ...
}
... otherwise implement the fallback ...
where the return value from process_connect() is the return value of the
call it makes to process_connect_service().
Move the call of do_take_over() inside process_connect(), so that
calling the process_connect() function is more concise and will not
miss do_take_over().
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
transport-helper.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 91381be622..566f7473df 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -646,6 +646,7 @@ static int process_connect(struct transport *transport,
struct helper_data *data = transport->data;
const char *name;
const char *exec;
+ int ret;
name = for_push ? "git-receive-pack" : "git-upload-pack";
if (for_push)
@@ -653,7 +654,10 @@ static int process_connect(struct transport *transport,
else
exec = data->transport_options.uploadpack;
- return process_connect_service(transport, name, exec);
+ ret = process_connect_service(transport, name, exec);
+ if (ret)
+ do_take_over(transport);
+ return ret;
}
static int connect_helper(struct transport *transport, const char *name,
@@ -685,10 +689,8 @@ static int fetch_refs(struct transport *transport,
get_helper(transport);
- if (process_connect(transport, 0)) {
- do_take_over(transport);
+ if (process_connect(transport, 0))
return transport->vtable->fetch_refs(transport, nr_heads, to_fetch);
- }
/*
* If we reach here, then the server, the client, and/or the transport
@@ -1145,10 +1147,8 @@ static int push_refs(struct transport *transport,
{
struct helper_data *data = transport->data;
- if (process_connect(transport, 1)) {
- do_take_over(transport);
+ if (process_connect(transport, 1))
return transport->vtable->push_refs(transport, remote_refs, flags);
- }
if (!remote_refs) {
fprintf(stderr,
@@ -1189,11 +1189,9 @@ static struct ref *get_refs_list(struct transport *transport, int for_push,
{
get_helper(transport);
- if (process_connect(transport, for_push)) {
- do_take_over(transport);
+ if (process_connect(transport, for_push))
return transport->vtable->get_refs_list(transport, for_push,
transport_options);
- }
return get_refs_list_using_list(transport, for_push);
}
@@ -1277,10 +1275,8 @@ static int get_bundle_uri(struct transport *transport)
{
get_helper(transport);
- if (process_connect(transport, 0)) {
- do_take_over(transport);
+ if (process_connect(transport, 0))
return transport->vtable->get_bundle_uri(transport);
- }
return -1;
}
--
2.43.0
^ permalink raw reply related
* [PATCH] rebase: Fix documentation about used shell in -x
From: Nikolay Borisov @ 2024-01-16 14:18 UTC (permalink / raw)
To: git; +Cc: Nikolay Borisov
The shell used when using the -x option is the one pointed to by the
SHELL_PATH constant at build time. This erroneous statement in the
documentation sent me on a 10 minute wild goose chase wondering why my
$SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
using dash and not bash.
Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
Documentation/git-rebase.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 25516c45d8b8..08cf52daf39e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can
continue with `git rebase --continue`.
The "exec" command launches the command in a shell (the one specified
-in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
+by the build-time SHELL_PATH variable, usually /bin/sh), so you can
use shell features (like "cd", ">", ";" ...). The command is run from
the root of the working tree.
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] rebase: Fix documentation about used shell in -x
From: Jeff King @ 2024-01-16 14:37 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: git
In-Reply-To: <20240116141842.193151-1-nik.borisov@suse.com>
On Tue, Jan 16, 2024 at 04:18:42PM +0200, Nikolay Borisov wrote:
> The shell used when using the -x option is the one pointed to by the
> SHELL_PATH constant at build time. This erroneous statement in the
> documentation sent me on a 10 minute wild goose chase wondering why my
> $SHELL was pointing to /bin/bash and my /bin/sh to dash and git was
> using dash and not bash.
Good catch. It originally used $SHELL when the documentation was added
in cd035b1cef (rebase -i: add exec command to launch a shell command,
2010-08-10). But that was lost when it was converted to C (which is
perhaps a regression, but nobody seems to have noticed or cared until
now, and at this point we should stick with the new behavior).
(I don't have an exact date since the conversion was somewhat piecemeal,
but it was done by 2018).
Since then, we use the code in run-command.c's prepare_shell_cmd().
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 25516c45d8b8..08cf52daf39e 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -964,7 +964,7 @@ non-0 status) to give you an opportunity to fix the problem. You can
> continue with `git rebase --continue`.
>
> The "exec" command launches the command in a shell (the one specified
> -in `$SHELL`, or the default shell if `$SHELL` is not set), so you can
> +by the build-time SHELL_PATH variable, usually /bin/sh), so you can
> use shell features (like "cd", ">", ";" ...). The command is run from
> the root of the working tree.
Avoiding $SHELL is obviously correct, but I think mentioning SHELL_PATH
is a little hairy. It is not used on Windows; see 776297548e (Do not use
SHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17).
Maybe it makes sense to just say:
...in a shell (the default one, usually /bin/sh), ...
It might even make sense to just drop the parenthetical phrase entirely.
Git executes lots of things using a shell, and it is always "the default
one", but we don't bother saying so in most places.
-Peff
^ permalink raw reply
* Re: [PATCH 3/3] ci: add macOS jobs to GitLab CI
From: Phillip Wood @ 2024-01-16 14:58 UTC (permalink / raw)
To: Patrick Steinhardt, git
In-Reply-To: <d196cfd9d01fe3b52c75a1e4e0aca9f67567ab43.1705318985.git.ps@pks.im>
Hi Patrick
On 15/01/2024 11:45, Patrick Steinhardt wrote:
> Add two macOS-based jobs to GitLab CI, one for Clang and one for GCC.
This doesn't match whats in the rest of the commit message where you
explain why there is no gcc job. The patch itself looks good to me and
it is nice that we'll now be testing on arm64 with the GitLab runners.
> This matches equivalent jobs we have for GitHub Workflows, except that
> we use macOS 14 instead of macOS 13.
>
> Note that one test marked as `test_must_fail` is surprisingly passing:
>
> t7815-grep-binary.sh (Wstat: 0 Tests: 22 Failed: 0)
> TODO passed: 12
>
> This seems to boil down to an unexpected difference in how regcomp(1)
nit: regcomp(3)?
Best Wishes
Phillip
> works when matching NUL bytes. Cross-checking with the respective GitHub
> job shows though that this is not an issue unique to the GitLab CI job
> as it passes in the same way there.
>
> Further note that we do not include the equivalent for the "osx-gcc" job
> that we use with GitHub Workflows. This is because the runner for macOS
> on GitLab is running on Apple M1 machines and thus uses the "arm64"
> architecture. GCC does not support this platform yet.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> .gitlab-ci.yml | 26 +++++++++++++++++++++++++-
> ci/lib.sh | 9 ++++++++-
> 2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 793243421c..9748970798 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -7,7 +7,7 @@ workflow:
> - if: $CI_COMMIT_TAG
> - if: $CI_COMMIT_REF_PROTECTED == "true"
>
> -test:
> +test:linux:
> image: $image
> before_script:
> - ./ci/install-docker-dependencies.sh
> @@ -52,6 +52,30 @@ test:
> - t/failed-test-artifacts
> when: on_failure
>
> +test:osx:
> + image: $image
> + tags:
> + - saas-macos-medium-m1
> + before_script:
> + - ./ci/install-dependencies.sh
> + script:
> + - ./ci/run-build-and-tests.sh
> + after_script:
> + - |
> + if test "$CI_JOB_STATUS" != 'success'
> + then
> + ./ci/print-test-failures.sh
> + fi
> + parallel:
> + matrix:
> + - jobname: osx-clang
> + image: macos-13-xcode-14
> + CC: clang
> + artifacts:
> + paths:
> + - t/failed-test-artifacts
> + when: on_failure
> +
> static-analysis:
> image: ubuntu:22.04
> variables:
> diff --git a/ci/lib.sh b/ci/lib.sh
> index f631206a44..d5dd2f2697 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -252,7 +252,14 @@ then
> CI_COMMIT="$CI_COMMIT_SHA"
> case "$CI_JOB_IMAGE" in
> macos-*)
> - CI_OS_NAME=osx;;
> + # GitLab CI has Python installed via multiple package managers,
> + # most notably via asdf and Homebrew. Ensure that our builds
> + # pick up the Homebrew one by prepending it to our PATH as the
> + # asdf one breaks tests.
> + export PATH="$(brew --prefix)/bin:$PATH"
> +
> + CI_OS_NAME=osx
> + ;;
> alpine:*|fedora:*|ubuntu:*)
> CI_OS_NAME=linux;;
> *)
^ permalink raw reply
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