* Re: Suggested clarification for .gitattributes reference documentation
From: Michael Litwak @ 2024-01-12 22:36 UTC (permalink / raw)
To: brian m. carlson; +Cc: git@vger.kernel.org
In-Reply-To: <ZaG0EkADl8hQZaqf@tapette.crustytoothpaste.net>
Regarding Git for Windows requiring 'git bash' console to perform text conversion...
>This sounds like a Git for Windows bug. Rather than documenting it, could you open an issue for it on their project?
>That sounds like a PATH misconfiguration on your part. Have you checked your PATH settings to make sure that the path including the binary is included?
If it is merely that I need to adjust my PATH so iconv.exe is accessible, that simplifies everything; however, it could possibly be a Git for Windows installer bug (or perhaps the installer offered to change my PATH and I declined).
I'll check out both possibilities.
>We have also documented the UTF-16LE-BOM case specifically in the Git FAQ (git help gitfaq) under "I'm on Windows and my text files are detected as binary".
I'll take a look and perhaps revise my suggested documentation edits.
Thanks,
- Michael
-----Original Message-----
From: brian m. carlson <sandals@crustytoothpaste.net>
Sent: Friday, January 12, 2024 1:50 PM
To: Michael Litwak <michael.litwak@nuix.com>
Cc: git@vger.kernel.org
Subject: [EXTERNAL]Re: Suggested clarification for .gitattributes reference documentation
On 2024-01-12 at 21:25:19, Michael Litwak wrote:
> Please note, Git for Windows does not support UTF-16LE encoding when running git
> commands from an ordinary Command Prompt. Use a git bash console instead.
This sounds like a Git for Windows bug. Rather than documenting it, could you open an issue for it on their project?
> NEW TEXT:
>
> You can get a list of all available encodings on your platform with the following command:
>
> iconv --list
>
> For Git for Windows users the command, above, is only supported when running in a 'git bash' console.
That sounds like a PATH misconfiguration on your part. Have you checked your PATH settings to make sure that the path including the binary is included?
> CONCLUSION:
>
> Text files encoded with UTF-16LE with BOM are common in the Windows
> world, as some versions of Visual Studio will use this as the default
> encoding for .rc or .mc files. Solution files, project files and
> other Visual Studio files can also be in this format. Other encodings
> are common, too, e.g. some older versions of PowerShell defaulted to
> UTF-16BE with BOM for new .ps1 files. Yet users continue to experience
> encoding errors even when they are using the proper
> working-tree-encoding in their .gitattributes file. Part of this is
> due to the complexity of Git and the number of different platforms it
> supports.
I should point out that UTF-8 is pretty much the standard these days in many domains, even on Windows. For example, nobody is going to be pleased if you write a web page in any variant of UTF-16, and some languages, such as Rust, are simply defined to be in UTF-8 and won't work if you put them in any other encoding. Almost all editors these days do support UTF-8 (without BOM), even on Windows, so we do want to strongly encourage that rather than having people use UTF-16. The Git FAQ specifically outlines UTF-8 as the recommended way, which is most portable and most functional.
We have also documented the UTF-16LE-BOM case specifically in the Git FAQ (git help gitfaq) under "I'm on Windows and my text files are detected as binary". Answering questions on Stack Overflow, I realize that nobody actually reads the FAQ, but we did clearly document how to do it. That being said, I'm not opposed to an additional mention in the
gitattributes(5) page if you want to send an actual patch.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
^ permalink raw reply
* Re: [Outreachy][PATCH] Port helper/test-advise.c to unit-tests/t-advise.c
From: Junio C Hamano @ 2024-01-12 22:44 UTC (permalink / raw)
To: Achu Luma, Rubén Justo; +Cc: git, christian.couder, Christian Couder
In-Reply-To: <20240112102122.1422-1-ach.lumap@gmail.com>
Achu Luma <ach.lumap@gmail.com> writes:
> In the recent codebase update (8bf6fbd00d (Merge branch
> 'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
> merged, providing a standardized approach for testing C code. Prior to
> this update, some unit tests relied on the test helper mechanism,
> lacking a dedicated unit testing framework. It's more natural to perform
> these unit tests using the new unit test framework.
>
> This commit migrates the unit tests for advise_if_enabled functionality
> from the legacy approach using the test-tool command `test-tool advise`
> in t/helper/test-advise.c to the new unit testing framework
> (t/unit-tests/test-lib.h).
>
> The migration involves refactoring the tests to utilize the testing
> macros provided by the framework (TEST() and check_*()).
In the light of the presense of work like this one
https://lore.kernel.org/git/c870a0b6-9fa8-4d00-a5a6-661ca175805f@gmail.com/
I am not sure if moving this to unit-test framework is a good thing,
at least right at this moment.
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?
I doubt the issue is not about "picking the right moment" to
transition from the test-tool to unit testing framework in this
particular case. Is "check-advice-if-enabled" a bit too high level
a feature to be a good match to "unit" testing, I have to wonder?
Thanks.
^ permalink raw reply
* Re: [PATCH v4 1/2] t7501: add tests for --include and --only
From: Junio C Hamano @ 2024-01-12 23:10 UTC (permalink / raw)
To: Ghanshyam Thakkar; +Cc: git, phillip.wood123, christian.couder
In-Reply-To: <20240112180109.59350-2-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> @@ -92,6 +92,19 @@ test_expect_success '--long fails with nothing to commit' '
> test_must_fail git commit -m initial --long
> '
>
> +test_expect_success 'fail to commit untracked file' '
> + echo content >baz &&
> + test_must_fail git commit -m "baz" baz
> +'
> +
> +test_expect_success '--only also fail to commit untracked file' '
> + test_must_fail git commit --only -m "baz" baz
> +'
> +
> +test_expect_success '--include also fail to commit untracked file' '
> + test_must_fail git commit --include -m "baz" baz
> +'
As the latter two depends on the first one's side effect of leaving
an untracked 'baz' file in the working tree, I do not know if it is
sensible to split these into three tests. An obvious alternative is
to have a single test
test_expect_success 'pathspec that do not match any tracked path' '
echo content >baz &&
test_must_fail git commit -m baz baz &&
test_must_fail git commit -o -m baz baz &&
test_must_fail git commit -i -m baz baz
'
By the way, I do not think presence of 'baz' in the working tree
matters in the failures from these tests all that much, as the
reason they fail is because the pathspec does not match any tracked
file, whose contents in the index to be updated before made into a
commit.
> @@ -117,6 +130,70 @@ test_expect_success '--long with stuff to commit returns ok' '
> git commit -m next -a --long
> '
>
> +test_expect_success 'only commit given path (also excluding additional staged changes)' '
> + echo content >file &&
> + echo content >baz &&
> + git add baz &&
> + git commit -m "file" file &&
> +
> + git diff --name-only >actual &&
> + test_must_be_empty actual &&
> +
> + git diff --name-only --staged >actual &&
> + test_cmp - actual <<-EOF &&
> + baz
> + EOF
> +
> + git diff --name-only HEAD^ HEAD >actual &&
> + test_cmp - actual <<-EOF
> + file
> + EOF
> +'
OK. The change to baz is already in the index, but "commit file"
will skip over it and record only the changes to file in the commit.
Very much makes sense.
> +test_expect_success 'same as above with -o/--only' '
> + echo change >file &&
> + echo change >baz &&
> + git add baz &&
> + git commit --only -m "file" file &&
> +
> + git diff --name-only >actual &&
> + test_must_be_empty actual &&
> +
> + git diff --name-only --staged >actual &&
> + test_cmp - actual <<-EOF &&
> + baz
> + EOF
> +
> + git diff --name-only HEAD^ HEAD >actual &&
> + test_cmp - actual <<-EOF
> + file
> + EOF
> +'
Likewise. An obvious thing to notice is that this cannot use the
same "contents" text as before, even though it claims to be "same as
above". If the final contents of "file" and "baz" does not matter,
but it matters more that these files have been changed, it often is
a good idea to append to the file. That way, you can ensure that
you will be making them different, no matter what the initial
condition was, i.e.,
for opt in "" "-o" "--only"
do
test_expect_success 'skip over already added change' '
echo more >>file &&
echo more >>baz &&
git add baz &&
git commit $opt -m "file" file &&
... ensure that changes to file are committed
... and changes to baz is only in the index
'
done
let's you test all three combinations.
Thanks.
^ permalink raw reply
* Re: [PATCH] strvec: use correct member name in comments
From: Linus Arver @ 2024-01-13 0:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Linus Arver via GitGitGadget, git
In-Reply-To: <xmqqjzoe8br0.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Linus Arver <linusa@google.com> writes:
>
>> Side note: should we start naming the parameters in strvec.h? I would
>> think that it wouldn't hurt at this point (as the API is pretty stable).
>> If you think that's worth it, I could reroll to include that in this
>> series (and also improve my commit message for this patch).
>
> I am not sure if it adds more value to outweigh the cost of
> churning. When the meaning of the parameters are obvious only by
> looking at their types, a prototype without parameter names is
> easier to maintain, by allowing the parameters to be renamed only
> once in the implementation. When the meaning of parameters are not
> obvious from their types, we do want them to be named so that you
> only have to refer to the header files to know the argument order.
This sounds like a good rule to me.
> "void *calloc(size_t, size_t)" would not tell us if we should pass
> the size of individual element or the number of elements first, and
> writing "void *calloc(size_t nmemb, size_t size)" to make it more
> obvious is a good idea.
>
> On the other hand, "void *realloc(void *, size_t)" is sufficient to
> tell us that we are passing a pointer as the first parameter and the
> desired size as the second parameter, without them having any name.
Thanks for the illuminating examples. Agreed.
> Are there functions declared in strvec.h you have in mind that their
> parameters are confusing and hard to guess what they mean?
TBH I only learned recently (while writing the patch in this thread)
that parameter names in prototypes were optional. I got a little
confused initially when looking at strvec.h for the first time because
none of the parameters were named. Having thought a bit more about these
functions, none of them have repeated types like in your example where
naming is warranted, so I think they're fine as is.
OTOH if we were treating these .h files as something meant for direct
external consumption (that is, if strvec.h is libified and external
users outside of Git are expected to use it directly as their first
point of documentation), at that point it might make sense to name the
parameters (akin to the style of manpages for syscalls). But I imagine
at that point we would have some other means of developer docs (beyond
raw header files) for libified parts of Git, so even in that case it's
probably fine to keep things as is.
Thanks.
^ permalink raw reply
* Re: Help: Trying to setup triangular workflow
From: Matthew B. Gray @ 2024-01-13 0:45 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20240112073136.GG618729@coredump.intra.peff.net>
Hi Peff,
> This push step is rewriting your upstream config. Do you have
> push.autoSetupRemote configured? In general you wouldn't want that for a
> triangular flow.
Thanks for the pointer, yes have both push.autoSetupRemote and
branch.autoSetupMerge set. Turning these off has fixed my problem.
With neither option set the example yields:
@{upstream}: refs/remotes/origin/main
@{push}: refs/remotes/myfork/mybranch
With push.autosetupremote=true set:
@{upstream}: refs/remotes/origin/main
@{push}: refs/remotes/myfork/mybranch
With branch.autoSetupMerge=simple set:
@{upstream}:
@{push}: refs/remotes/myfork/mybranch
With branch.autoSetupMerge=simple and push.autosetupremote=true:
@{upstream}: refs/remotes/myfork/mybranch
@{push}: refs/remotes/myfork/mybranch
> Though I think it also is only supposed to kick in if there is no
> tracking configured already. Why did the "git switch" invocation not set
> up tracking itself? When I run those commands it does. Do you have
> branch.autoSetupMerge turned off in your config?
If you're interested in looking at my full gitconfig it's hosted here:
https://github.com/heymatthew/dotfiles/blob/trunk/softlinks/.gitconfig
However looks like taking those out of global config has fixed my issue.
Thanks for the help!
Ngā mihi,
Matthew
^ permalink raw reply
* Re: [PATCH v4 1/2] t7501: add tests for --include and --only
From: Ghanshyam Thakkar @ 2024-01-13 1:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, phillip.wood123, christian.couder
In-Reply-To: <xmqqbk9q6tcs.fsf@gitster.g>
On Sat Jan 13, 2024 at 4:40 AM IST, Junio C Hamano wrote:
> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
>
> > @@ -92,6 +92,19 @@ test_expect_success '--long fails with nothing to commit' '
> > test_must_fail git commit -m initial --long
> > '
> >
> > +test_expect_success 'fail to commit untracked file' '
> > + echo content >baz &&
> > + test_must_fail git commit -m "baz" baz
> > +'
> > +
> > +test_expect_success '--only also fail to commit untracked file' '
> > + test_must_fail git commit --only -m "baz" baz
> > +'
> > +
> > +test_expect_success '--include also fail to commit untracked file' '
> > + test_must_fail git commit --include -m "baz" baz
> > +'
>
> As the latter two depends on the first one's side effect of leaving
> an untracked 'baz' file in the working tree, I do not know if it is
> sensible to split these into three tests. An obvious alternative is
> to have a single test
>
> test_expect_success 'pathspec that do not match any tracked path' '
> echo content >baz &&
> test_must_fail git commit -m baz baz &&
> test_must_fail git commit -o -m baz baz &&
> test_must_fail git commit -i -m baz baz
> '
>
> By the way, I do not think presence of 'baz' in the working tree
> matters in the failures from these tests all that much, as the
> reason they fail is because the pathspec does not match any tracked
> file, whose contents in the index to be updated before made into a
> commit.
Yes, that is true. However, as per your prior email in which you stated
about --include:
"Now which behaviour between "error out because there is no path in
the index that matches pathspec 'baz'" and "add baz to the index and
commit that addition, together with what is already in the index" we
would want to take is probably open for discussion. Such a discussion
may decide that the current behaviour is fine. Until then..."
If in such a discussion it is decided that -i should add to index and
commit, then by not having 'baz' in the working tree, the -i test
would still error out regardless of whether its behaviour is to [add
to the index and commit] or [error out]. Therefore, by having 'baz'
we can detect the change between [-i adds to index and commits] or
[errors out].
> Likewise. An obvious thing to notice is that this cannot use the
> same "contents" text as before, even though it claims to be "same as
> above". If the final contents of "file" and "baz" does not matter,
> but it matters more that these files have been changed, it often is
> a good idea to append to the file. That way, you can ensure that
> you will be making them different, no matter what the initial
> condition was, i.e.,
>
> for opt in "" "-o" "--only"
> do
> test_expect_success 'skip over already added change' '
> echo more >>file &&
> echo more >>baz &&
> git add baz &&
> git commit $opt -m "file" file &&
>
> ... ensure that changes to file are committed
> ... and changes to baz is only in the index
> '
> done
>
> let's you test all three combinations.
Yeah, that is a more effective approach. I will change it and reroll
quickly.
>
> Thanks.
Thank you for all the help!
^ permalink raw reply
* Re: [PATCH v4 1/2] t7501: add tests for --include and --only
From: Junio C Hamano @ 2024-01-13 1:16 UTC (permalink / raw)
To: Ghanshyam Thakkar; +Cc: git, phillip.wood123, christian.couder
In-Reply-To: <20240112180109.59350-2-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> +test_expect_success '-i/--include includes staged changes' '
> + echo newcontent >file &&
> + echo newcontent >baz &&
> + git add file &&
> + git commit --include -m "file baz" baz &&
I may have said this already, but the command invocation that does
not result in an error smells like a bug, and I doubt that we want
to etch the current behaviour into stone, which may make it harder
to fix [*].
Another related behaviour that I suspect is a bug is that if you did
git add -u baz
instead of this "git commit -i baz", I think the command will
silently succeed without doing anything. They may be the same bug,
because "git commit -i <pathspec>" is an equivalent to "git add -u
<pathspec>" followed by "git commit" after all. Both should say
"there is no such path to update that matches the pathspec <baz>"
and error out, I suspect.
Thanks.
[Footnote]
* A reasonable way out to unblock this particular patch may be to
clarify that this test is only documenting the current behaviour
without necessarily endorsing it. Perhaps
echo more >>file &&
echo more >>baz &&
git add file &&
# Note: "git commit -i baz" is like "git add -u baz"
# followed by "git commit" but because baz is untracked,
# only "file" is committed.
# This test only documents this current behaviour, which we
# may want to fix, and when it happens, this needs to be
# adjusted to the new behaviour.
git commit -i -m "file and baz" baz &&
or something, probably.
^ permalink raw reply
* Re: [PATCH 00/10] Enrich Trailer API
From: Linus Arver @ 2024-01-13 1:35 UTC (permalink / raw)
To: Junio C Hamano, Linus Arver via GitGitGadget
Cc: git, Emily Shaffer, Christian Couder
In-Reply-To: <xmqqy1cx9dm4.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> This patch series is the first 10 patches of a much larger series I've been
>> working. The main goal of this series is to enrich the API in trailer.h. The
>> larger series brings a number of additional code simplifications and
>> cleanups (exposing and fixing some bugs along the way), and builds on top of
>> this series. The goal of the larger series is to make the trailer interface
>> ready for unit testing. By "trailer API" I mean those functions exposed in
>> trailer.h.
>
> Are there places in the current code that deals with trailers but
> does not use the trailer API (e.g., manually parse and/or insert the
> trailer in an in-core buffer)?
While working on this series I unfortunately did not search for such use
cases (I limited the scope of my work to mainly the interpret-trailers
builtin). But, a quick search just now on master branch turned up
append_signoff() in sequencer.c which constructs a Signed-off-by trailer
manually with a raw strbuf [1].
This is somewhat understandable though, because AFAICS the current
trailer API does not support creating and formatting single trailer
objects.
> Is it part of the larger goal to
> update these places so that we will always use the trailer API to
> touch trailers
That sounds like The Right Thing to do.
> , and if so, have these places been identified?
Not yet, but, it should be easy to check any remaining cases other than
the one I identified up above.
> Obviously the reason why I ask is that testing cannot be the goal by
> itself.
I now seem to recall an offline discussion where I said I wanted to
enrich the API to make other parts of Git also use this new API. Somehow
I've left that motivation out of the cover letter (will include in the
next reroll).
> The "alternative" ...
>
>> As an alternative to this patch series, we could keep trailer.h intact and
>> decide to unit-test the existing "trailer_info_get()" function which does
>> most of the trailer parsing work.
>
> ... may allow you to "test", but it would make it more difficult in
> the future to revamp the trailer API, if it is needed, in order to
> cover code paths that ought to be using but currently bypassing the
> trailer API.
Agreed. I should include this rationale as well in the next cover
letter, thanks.
>> This series breaks up "process_trailers()" into smaller pieces, exposing
>> many of the parts relevant to trailer-related processing in trailer.h. This
>> forces us to start writing unit tests for these now public functions, but
>> that is a good thing because those same unit tests should be easy to write
>> (due to their small(er) sizes), but also, because those unit tests will now
>> ensure some degree of stability across new versions of trailer.h (we will
>> start noticing when the behavior of any of these API functions change).
>
> And helper functions, each of which does one small thing well, may
> be more applicable to other code paths that are currently bypassing
> the API.
Yep.
>> Thanks to the aggressive refactoring in this series, I've been able to
>> identify and fix a couple bugs in our existing implementation. Those fixes
>> build on top of this series but were not included here, in order to keep
>> this series small.
>
> It would be nicer to have a concise list of these fixes (in the form
> of "git shortlog") as a teaser here ;-). That would hopefully
> entice others into reviewing this part that forms the foundation.
Ah, good idea. Here's a shortlog (with a short summary of each one) of
bug fixes that are forthcoming:
trailer: trailer replacement should not change its position
(Summary: If we found a trailer we'd like to replace, preserve its
position relative to the other trailers found in the trailer
block, instead of always moving it to the beginning or end of the
entire trailer block.)
interpret-trailers: preserve trailers coming from the input
(Summary: Sometimes, the parsed trailers from the input will be
formatted differently depending on whether we provide
--only-trailers or not. Make the trailers that were not modified
and are coming directly from the input get formatted the same way,
regardless of this flag.)
interpret-trailers: fail if given unrecognized arguments
(Summary: E.g., for "--where", only accept recognized WHERE_* enum
values. If we get something unrecognized, fail with an error
instead of silently doing nothing. Ditto for "--if-exists" and
"--if-missing".)
The last one is a different class of bug than the first two, and perhaps
less interesting.
Thanks.
[1] https://github.com/git/git/blob/d4dbce1db5cd227a57074bcfc7ec9f0655961bba/sequencer.c#L5299-L5301
^ permalink raw reply
* Re: [PATCH] diffcore-delta: avoid ignoring final 'line' of file
From: Elijah Newren @ 2024-01-13 1:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <xmqqedenearc.fsf@gitster.g>
On Thu, Jan 11, 2024 at 3:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > diff --git a/diffcore-delta.c b/diffcore-delta.c
> > index c30b56e983b..7136c3dd203 100644
> > --- a/diffcore-delta.c
> > +++ b/diffcore-delta.c
> > @@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r,
> > n = 0;
> > accum1 = accum2 = 0;
> > }
> > + if (n > 0) {
> > + hashval = (accum1 + accum2 * 0x61) % HASHBASE;
> > + hash = add_spanhash(hash, hashval, n);
> > + }
>
> OK, so we were ignoring the final short bit that is not terminated
> with LF due to the "continue". Nicely found.
>
> > diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> > index 85be1367de6..29299acbce7 100755
> > --- a/t/t4001-diff-rename.sh
> > +++ b/t/t4001-diff-rename.sh
> > @@ -286,4 +286,23 @@ test_expect_success 'basename similarity vs best similarity' '
> > test_cmp expected actual
> > '
> >
> > +test_expect_success 'last line matters too' '
> > + test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
> > + printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
> > + git add nonewline &&
> > + git commit -m "original version of file with no final newline" &&
>
> I found it misleading that the file whose name is nonewline has
> bunch of LF including on its last line ;-).
Yeah, sorry. It's been a while, but I think I started with a file
with only one line that had no LF, but then realized for inexact
rename detection to kick in I needed some other lines, at least one of
which didn't match...but I forgot to update the filename after adding
the other lines...
> > + # Change ONLY the first character of the whole file
> > + test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&
>
> Same here, but it is too much to bother rewriting it ...
>
> {
> test_write_lines ...
> printf ...
> } >incomplete
>
> ... like so ("incomplete" stands for "file ending with an incomplete line"),
> so I'll let it pass.
>
> > + printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
>
>
> > + git add nonewline &&
> > + git mv nonewline still-no-newline &&
> > + git commit -a -m "rename nonewline -> still-no-newline" &&
> > + git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
> > + cat >expected <<-\EOF &&
> > + R097 nonewline still-no-newline
>
> I am not very happy with the hardcoded 97. You are already using
> the non-standard 10% threshold. If the delta detection that
> forgets about the last line is so broken as your proposed log
> message noted, shouldn't you be able to construct a sample pair of
> preimage and postimage for which the broken version gives so low
> similarity to be judged not worth treating as a rename, while the
> fixed version gives reasonable similarity to be made into a rename,
> by the default threshold? That way, the test only needs to see if
> we got a rename (with any similarity) or a delete and an add.
Oops, the threshold is entirely unnecessary here; not sure why I
didn't remember to take it out (originally used the threshold while
testing without the fix to just how low of a similarity git thought
these nearly identical files had).
Since you don't like the threshold, and since we don't seem to have a
summary format that reports on the rename without the percentage, I
guess I need to munge the output with sed:
sed -e "s/^R[0-9]* /R /" actual >actual.munged &&
and then compare the expected output to that. I'll send in a patch
doing so and fix up the filenames and drop the rename threshold while
at it.
^ permalink raw reply
* Re: [PATCH v4 1/2] t7501: add tests for --include and --only
From: Ghanshyam Thakkar @ 2024-01-13 1:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, phillip.wood123, christian.couder
In-Reply-To: <xmqqmsta58ya.fsf@gitster.g>
On Sat Jan 13, 2024 at 6:46 AM IST, Junio C Hamano wrote:
> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
>
> > +test_expect_success '-i/--include includes staged changes' '
> > + echo newcontent >file &&
> > + echo newcontent >baz &&
> > + git add file &&
> > + git commit --include -m "file baz" baz &&
>
> I may have said this already, but the command invocation that does
> not result in an error smells like a bug, and I doubt that we want
> to etch the current behaviour into stone, which may make it harder
> to fix [*].
Yeah, that is why baz is added in the index in the test before the one
you quoted. And as I understand it, when everything is in the index, it
works as intended. Therefore to not get in the way of amending this
behaviour, no untracked files are being (attempted to be) committed in
these tests (except 'fail to commit untracked files' test, but that is
not what you quoted above).
> Another related behaviour that I suspect is a bug is that if you did
>
> git add -u baz
>
> instead of this "git commit -i baz", I think the command will
> silently succeed without doing anything. They may be the same bug,
> because "git commit -i <pathspec>" is an equivalent to "git add -u
> <pathspec>" followed by "git commit" after all. Both should say
> "there is no such path to update that matches the pathspec <baz>"
> and error out, I suspect.
Yeah, just checked, that also succeeds silently.
> Thanks.
>
> [Footnote]
>
> * A reasonable way out to unblock this particular patch may be to
> clarify that this test is only documenting the current behaviour
> without necessarily endorsing it. Perhaps
>
> echo more >>file &&
> echo more >>baz &&
> git add file &&
>
> # Note: "git commit -i baz" is like "git add -u baz"
> # followed by "git commit" but because baz is untracked,
> # only "file" is committed.
> # This test only documents this current behaviour, which we
> # may want to fix, and when it happens, this needs to be
> # adjusted to the new behaviour.
> git commit -i -m "file and baz" baz &&
>
> or something, probably.
as stated above, baz is tracked from the previous test. In any case,
I will add a note just to document that untracked files also do not
give any error (when there are staged changes) but are not committed.
Thanks.
^ permalink raw reply
* RE: Suggested clarification for .gitattributes reference documentation
From: Michael Litwak @ 2024-01-13 2:56 UTC (permalink / raw)
To: brian m. carlson; +Cc: git@vger.kernel.org
In-Reply-To: <SJ0PR10MB56937175632E5D82793CE13EFA6F2@SJ0PR10MB5693.namprd10.prod.outlook.com>
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).
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
-----Original Message-----
From: brian m. carlson <sandals@crustytoothpaste.net>
Sent: Friday, January 12, 2024 1:50 PM
To: Michael Litwak <michael.litwak@nuix.com>
Cc: git@vger.kernel.org
Subject: [EXTERNAL]Re: Suggested clarification for .gitattributes reference documentation
On 2024-01-12 at 21:25:19, Michael Litwak wrote:
> Please note, Git for Windows does not support UTF-16LE encoding when running git
> commands from an ordinary Command Prompt. Use a git bash console instead.
This sounds like a Git for Windows bug. Rather than documenting it, could you open an issue for it on their project?
> NEW TEXT:
>
> You can get a list of all available encodings on your platform with the following command:
>
> iconv --list
>
> For Git for Windows users the command, above, is only supported when running in a 'git bash' console.
That sounds like a PATH misconfiguration on your part. Have you checked your PATH settings to make sure that the path including the binary is included?
> CONCLUSION:
>
> Text files encoded with UTF-16LE with BOM are common in the Windows
> world, as some versions of Visual Studio will use this as the default
> encoding for .rc or .mc files. Solution files, project files and
> other Visual Studio files can also be in this format. Other encodings
> are common, too, e.g. some older versions of PowerShell defaulted to
> UTF-16BE with BOM for new .ps1 files. Yet users continue to experience
> encoding errors even when they are using the proper
> working-tree-encoding in their .gitattributes file. Part of this is
> due to the complexity of Git and the number of different platforms it
> supports.
I should point out that UTF-8 is pretty much the standard these days in many domains, even on Windows. For example, nobody is going to be pleased if you write a web page in any variant of UTF-16, and some languages, such as Rust, are simply defined to be in UTF-8 and won't work if you put them in any other encoding. Almost all editors these days do support UTF-8 (without BOM), even on Windows, so we do want to strongly encourage that rather than having people use UTF-16. The Git FAQ specifically outlines UTF-8 as the recommended way, which is most portable and most functional.
We have also documented the UTF-16LE-BOM case specifically in the Git FAQ (git help gitfaq) under "I'm on Windows and my text files are detected as binary". Answering questions on Stack Overflow, I realize that nobody actually reads the FAQ, but we did clearly document how to do it. That being said, I'm not opposed to an additional mention in the
gitattributes(5) page if you want to send an actual patch.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
^ permalink raw reply
* [PATCH v5 0/2] t7501: add tests for --include, --only, --signoff
From: Ghanshyam Thakkar @ 2024-01-13 4:21 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood123, christian.couder, Ghanshyam Thakkar
In-Reply-To: <20240112180109.59350-1-shyamthakkar001@gmail.com>
The v5 of this series updates the '--only' test to use the for loop
and >> (append) operator to test all three cases (-o/--only/none) via a
single test.
This version also documents a bug found via the previous iteration of
this series, which does not report error when --include is used with
invalid pathspec.
Signoff tests remain unchanged.
Ghanshyam Thakkar (2):
t7501: add tests for --include and --only
t7501: add tests for --amend --signoff
t/t7501-commit-basic-functionality.sh | 100 +++++++++++++++++++++++++-
1 file changed, 98 insertions(+), 2 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v5 1/2] t7501: add tests for --include and --only
From: Ghanshyam Thakkar @ 2024-01-13 4:21 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood123, christian.couder, Ghanshyam Thakkar
In-Reply-To: <20240113042254.38602-1-shyamthakkar001@gmail.com>
Add tests for --only (-o) and --include (-i). This include testing
with or without staged changes for both -i and -o. Also to test
for committing untracked files with -i, -o and without -i/-o.
Some tests already exist in t7501 for testing --only, however,
it is tested in combination with --amend and --allow-empty and on
to-be-born branch. The addition of these tests check, when the
pathspec is provided, that only the files matching the pathspec
get committed. This behavior is same when we provide --only and
checked by the tests.
(as --only is the default mode of operation when pathspec is
provided.)
As for --include, there is no prior test for checking if --include
also commits staged changes, thus add test for that. Along with
the test also document a potential bug, in which, when provided
with -i and an untracked pathspec, commit does not fail if there
are staged changes. And when there are no staged changes commit
fails. However, no error is returned to stderr in either of the
cases.
Thus, these tests belong in t7501 with other similar existing tests,
as described in the case of --only.
And also add a test for checking incompatibilty when using -o and
-i together.
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
t/t7501-commit-basic-functionality.sh | 77 ++++++++++++++++++++++++++-
1 file changed, 76 insertions(+), 1 deletion(-)
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index 3d8500a52e..3e18b879b5 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -3,7 +3,7 @@
# Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
#
-# FIXME: Test the various index usages, -i and -o, test reflog,
+# FIXME: Test the various index usages, test reflog,
# signoff
test_description='git commit'
@@ -92,6 +92,32 @@ test_expect_success '--long fails with nothing to commit' '
test_must_fail git commit -m initial --long
'
+test_expect_success 'fail to commit untracked pathspec (even with --include/--only)' '
+ echo content >baz &&
+ error="error: pathspec .baz. did not match any file(s) known to git" &&
+
+ test_must_fail git commit -m "baz" baz 2>err &&
+ test_grep -e "$error" err &&
+
+ test_must_fail git commit --only -m "baz" baz 2>err &&
+ test_grep -e "$error" err &&
+
+ # TODO: as for --include, the below command will fail because nothing is
+ # staged. If something was staged, it would not fail even if the
+ # pathspec was untracked (however, pathspec will remain untracked and
+ # staged changes would be committed.) In either cases, no error is
+ # returned to stderr like in (-o and without -o/-i) cases. In a
+ # similar manner, "git add -u baz" also does not error out.
+ #
+ # Therefore, the below test is just to document the current behavior
+ # and is not an endorsement to the current behavior, and we may
+ # want to fix this. And when that happens, this test should be
+ # updated accordingly.
+
+ test_must_fail git commit --include -m "baz" baz 2>err &&
+ test_must_be_empty err
+'
+
test_expect_success 'setup: non-initial commit' '
echo bongo bongo bongo >file &&
git commit -m next -a
@@ -117,6 +143,55 @@ test_expect_success '--long with stuff to commit returns ok' '
git commit -m next -a --long
'
+for opt in "" "-o" "--only"
+do
+ test_expect_success 'exclude additional staged changes when given pathspec' '
+ echo content >>file &&
+ echo content >>baz &&
+ git add baz &&
+ git commit $opt -m "file" file &&
+
+ git diff --name-only >actual &&
+ test_must_be_empty actual &&
+
+ git diff --name-only --staged >actual &&
+ test_cmp - actual <<-EOF &&
+ baz
+ EOF
+
+ git diff --name-only HEAD^ HEAD >actual &&
+ test_cmp - actual <<-EOF
+ file
+ EOF
+ '
+done
+
+test_expect_success '-i/--include includes staged changes' '
+ echo content >>file &&
+ echo content >>baz &&
+ git add file &&
+
+ # baz is in the index, therefore, it will be committed
+ git commit --include -m "file and baz" baz &&
+
+ git diff --name-only HEAD >remaining &&
+ test_must_be_empty remaining &&
+
+ git diff --name-only HEAD^ HEAD >changes &&
+ test_cmp - changes <<-EOF
+ baz
+ file
+ EOF
+'
+
+test_expect_success '--include and --only do not mix' '
+ test_when_finished "git reset --hard" &&
+ echo content >>file &&
+ echo content >>baz &&
+ test_must_fail git commit --include --only -m "file baz" file baz 2>actual &&
+ test_grep -e "fatal: options .-i/--include. and .-o/--only. cannot be used together" actual
+'
+
test_expect_success 'commit message from non-existing file' '
echo more bongo: bongo bongo bongo bongo >file &&
test_must_fail git commit -F gah -a
--
2.43.0
^ permalink raw reply related
* [PATCH v5 2/2] t7501: add tests for --amend --signoff
From: Ghanshyam Thakkar @ 2024-01-13 4:21 UTC (permalink / raw)
To: git; +Cc: gitster, phillip.wood123, christian.couder, Ghanshyam Thakkar
In-Reply-To: <20240113042254.38602-1-shyamthakkar001@gmail.com>
Add tests for amending the commit to add Signed-off-by trailer. And
also to check if it does not add another trailer if one already exists.
Currently, there are tests for --signoff separately in t7501, however,
they are not tested with --amend.
Therefore, these tests belong with other similar tests of --amend in
t7501-commit-basic-functionality.
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
t/t7501-commit-basic-functionality.sh | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh
index 3e18b879b5..db37e9051b 100755
--- a/t/t7501-commit-basic-functionality.sh
+++ b/t/t7501-commit-basic-functionality.sh
@@ -3,8 +3,7 @@
# Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
#
-# FIXME: Test the various index usages, test reflog,
-# signoff
+# FIXME: Test the various index usages, test reflog
test_description='git commit'
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
@@ -464,6 +463,28 @@ test_expect_success 'amend commit to fix date' '
'
+test_expect_success 'amend commit to add signoff' '
+
+ test_commit "msg" file content &&
+ git commit --amend --signoff &&
+ test_commit_message HEAD <<-EOF
+ msg
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+'
+
+test_expect_success 'amend does not add signoff if it already exists' '
+
+ test_commit --signoff "tenor" file newcontent &&
+ git commit --amend --signoff &&
+ test_commit_message HEAD <<-EOF
+ tenor
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+'
+
test_expect_success 'commit mentions forced date in output' '
git commit --amend --date=2010-01-02T03:04:05 >output &&
grep "Date: *Sat Jan 2 03:04:05 2010" output
--
2.43.0
^ permalink raw reply related
* [PATCH v2] diffcore-delta: avoid ignoring final 'line' of file
From: Elijah Newren via GitGitGadget @ 2024-01-13 4:26 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1637.git.1705006074626.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
hash_chars() would hash lines to integers, and store them in a spanhash,
but cut lines at 64 characters. Thus, whenever it reached 64 characters
or a newline, it would create a new spanhash. The problem is, the final
part of the file might not end 64 characters after the previous 'line'
and might not end with a newline. This could, for example, cause an
85-byte file with 12 lines and only the first character in the file
differing to appear merely 23% similar rather than the expected 97%.
Ensure the last line is included, and add a testcase that would have
caught this problem.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
diffcore-delta: avoid ignoring final 'line' of file
Found while experimenting with converting portions of diffcore-delta to
Rust.
Changes since v1:
* Removed the unnecessary similarity threshold specification
* Munged the discovered similarity percentage so we are only checking
that a rename is detected
* Fixed up filenames
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1637%2Fnewren%2Ffix-diffcore-final-line-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1637/newren/fix-diffcore-final-line-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1637
Range-diff vs v1:
1: f62b22a54c3 ! 1: e0223bbfeb5 diffcore-delta: avoid ignoring final 'line' of file
@@ t/t4001-diff-rename.sh: test_expect_success 'basename similarity vs best similar
'
+test_expect_success 'last line matters too' '
-+ test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
-+ printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
-+ git add nonewline &&
++ {
++ test_write_lines a 0 1 2 3 4 5 6 7 8 9 &&
++ printf "git ignores final up to 63 characters if not newline terminated"
++ } >no-final-lf &&
++ git add no-final-lf &&
+ git commit -m "original version of file with no final newline" &&
+
+ # Change ONLY the first character of the whole file
-+ test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&
-+ printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
-+ git add nonewline &&
-+ git mv nonewline still-no-newline &&
-+ git commit -a -m "rename nonewline -> still-no-newline" &&
-+ git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
++ {
++ test_write_lines b 0 1 2 3 4 5 6 7 8 9 &&
++ printf "git ignores final up to 63 characters if not newline terminated"
++ } >no-final-lf &&
++ git add no-final-lf &&
++ git mv no-final-lf still-absent-final-lf &&
++ git commit -a -m "rename no-final-lf -> still-absent-final-lf" &&
++ git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
++ sed -e "s/^R[0-9]* /R /" actual >actual.munged &&
+ cat >expected <<-\EOF &&
-+ R097 nonewline still-no-newline
++ R no-final-lf still-absent-final-lf
+ EOF
-+ test_cmp expected actual
++ test_cmp expected actual.munged
+'
+
test_done
diffcore-delta.c | 4 ++++
t/t4001-diff-rename.sh | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/diffcore-delta.c b/diffcore-delta.c
index c30b56e983b..7136c3dd203 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r,
n = 0;
accum1 = accum2 = 0;
}
+ if (n > 0) {
+ hashval = (accum1 + accum2 * 0x61) % HASHBASE;
+ hash = add_spanhash(hash, hashval, n);
+ }
QSORT(hash->data, (size_t)1ul << hash->alloc_log2, spanhash_cmp);
return hash;
}
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 85be1367de6..49c042a38ae 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -286,4 +286,28 @@ test_expect_success 'basename similarity vs best similarity' '
test_cmp expected actual
'
+test_expect_success 'last line matters too' '
+ {
+ test_write_lines a 0 1 2 3 4 5 6 7 8 9 &&
+ printf "git ignores final up to 63 characters if not newline terminated"
+ } >no-final-lf &&
+ git add no-final-lf &&
+ git commit -m "original version of file with no final newline" &&
+
+ # Change ONLY the first character of the whole file
+ {
+ test_write_lines b 0 1 2 3 4 5 6 7 8 9 &&
+ printf "git ignores final up to 63 characters if not newline terminated"
+ } >no-final-lf &&
+ git add no-final-lf &&
+ git mv no-final-lf still-absent-final-lf &&
+ git commit -a -m "rename no-final-lf -> still-absent-final-lf" &&
+ git diff-tree -r -M --name-status HEAD^ HEAD >actual &&
+ sed -e "s/^R[0-9]* /R /" actual >actual.munged &&
+ cat >expected <<-\EOF &&
+ R no-final-lf still-absent-final-lf
+ EOF
+ test_cmp expected actual.munged
+'
+
test_done
base-commit: 055bb6e9969085777b7fab83e3fee0017654f134
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] diffcore-delta: avoid ignoring final 'line' of file
From: Junio C Hamano @ 2024-01-13 6:21 UTC (permalink / raw)
To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git
In-Reply-To: <CABPp-BGp0NMQKLYg=OxJgnVxARffNF57B_N2bLmwT2R2EZqhdA@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> I am not very happy with the hardcoded 97. You are already using
>> the non-standard 10% threshold. If the delta detection that
>> forgets about the last line is so broken as your proposed log
>> message noted, shouldn't you be able to construct a sample pair of
>> preimage and postimage for which the broken version gives so low
>> similarity to be judged not worth treating as a rename, while the
>> fixed version gives reasonable similarity to be made into a rename,
>> by the default threshold? That way, the test only needs to see if
>> we got a rename (with any similarity) or a delete and an add.
>
> Oops, the threshold is entirely unnecessary here; not sure why I
> didn't remember to take it out (originally used the threshold while
> testing without the fix to just how low of a similarity git thought
> these nearly identical files had).
>
> Since you don't like the threshold, and since we don't seem to have a
> summary format that reports on the rename without the percentage, I
> guess I need to munge the output with sed:
>
> sed -e "s/^R[0-9]* /R /" actual >actual.munged &&
Heh, I was hoping that we should be able to use "diff --name-only".
$ git mv Makefile Breakfile
$ git diff --name-only -M HEAD
Breakfile
$ git reset --hard
$ git rm Makefile
$ >Breakfile && git add Breakfile
$ git diff --name-only -M HEAD
Breakfile
Makefile
$ git reset --hard
^ permalink raw reply
* Re: [PATCH] strvec: use correct member name in comments
From: Jeff King @ 2024-01-13 7:31 UTC (permalink / raw)
To: Linus Arver; +Cc: Junio C Hamano, Linus Arver via GitGitGadget, git
In-Reply-To: <owlyle8uhxut.fsf@fine.c.googlers.com>
On Fri, Jan 12, 2024 at 04:37:46PM -0800, Linus Arver wrote:
> OTOH if we were treating these .h files as something meant for direct
> external consumption (that is, if strvec.h is libified and external
> users outside of Git are expected to use it directly as their first
> point of documentation), at that point it might make sense to name the
> parameters (akin to the style of manpages for syscalls). But I imagine
> at that point we would have some other means of developer docs (beyond
> raw header files) for libified parts of Git, so even in that case it's
> probably fine to keep things as is.
I think this is mostly orthogonal to libification. Whether the audience
is other parts of Git or users outside of Git, they need to know how to
call the function. Our main source of documentation there is comments
above the declaration (we've marked these with "/**" which would allow a
parser to pull them into a separate doc file, but AFAIK in the 9 years
since we started that convention, nobody has bothered to write such a
script).
Naming the parameters can help when writing those comments, because you
can then refer to them (e.g., see the comment above strbuf_addftime).
Even without that, I think they can be helpful, but I don't think I'd
bother adding them in unless taking a pass over the whole file, looking
for comments that do not sufficiently explain their matching functions.
I don't doubt that some of that would be necessary for libification,
just to increase the quality of the documentation. But I think it's
largely separate from the patch in this thread.
-Peff
^ permalink raw reply
* Re: [PATCH] advice: allow disabling the automatic hint in advise_if_enabled()
From: Jeff King @ 2024-01-13 7:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Rubén Justo, Git List, Taylor Blau, Dragan Simic
In-Reply-To: <xmqqsf326vpn.fsf@gitster.g>
On Fri, Jan 12, 2024 at 02:19:32PM -0800, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > advice.c | 109 +++++++++++++++++++++++++---------------------
> > t/t0018-advice.sh | 1 -
> > 2 files changed, 59 insertions(+), 51 deletions(-)
> >
> > diff --git a/advice.c b/advice.c
> > index f6e4c2f302..8474966ce1 100644
> > --- a/advice.c
> > +++ b/advice.c
> > @@ -33,50 +33,56 @@ static const char *advise_get_color(enum color_advice ix)
> > return "";
> > }
> >
> > +enum advice_level {
> > + ADVICE_LEVEL_DEFAULT = 0,
>
> We may want to say "_NONE" not "_DEFAULT" to match the convention
> used elsewhere, but I have no strong preference. I do have a
> preference to see that, when we explicitly say "In this enum, there
> is ADVICE_LEVEL_DEFAULT and its value is zero" with "= 0" in the
> definition, the places we use a variable of this enum type, we say
>
> if (!variable)
>
> and not
>
> if (variable == ADVICE_LEVEL_DEFAULT)
This may be getting into the realm of bikeshedding, but... ;)
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.
-Peff
^ permalink raw reply
* Re: Suggested clarification for .gitattributes reference documentation
From: Torsten Bögershausen @ 2024-01-13 7:43 UTC (permalink / raw)
To: Michael Litwak; +Cc: brian m. carlson, git@vger.kernel.org
In-Reply-To: <SJ0PR10MB56932ABBEEEC6F8ADE23995AFA6E2@SJ0PR10MB5693.namprd10.prod.outlook.com>
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
>
[]
^ permalink raw reply
* Re: Suggested clarification for .gitattributes reference documentation
From: Matthias Aßhauer @ 2024-01-13 9:24 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Michael Litwak, brian m. carlson, git@vger.kernel.org
In-Reply-To: <20240113074323.GA6819@tb-raspi4>
On Sat, 13 Jan 2024, Torsten Bögershausen wrote:
> 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).
For end users directly calling iconv.exe is definitely optional.
> 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)
Exactly. I can't find a single instance of Git for Windows calling
iconv.exe instead of using the corresponding library functions. [1]
And even if it did, iconv.exe is definitely on the path for git.exe [2]
unless you're calling /(mingw|clangarm)(64|32)/bin/git.exe directly, in
which case the solution is to call /cmd/git.exe instead.
[1]
https://github.com/search?q=repo%3Agit-for-windows%2Fgit%20iconv%20NOT%20path%3A%2F%5Et%5C%2F%2F%20NOT%20path%3A%2F%5EDocumentation%5C%2F%2F&type=code
[2]
https://github.com/git-for-windows/MINGW-packages/blob/0c91cf2079184ae6a604e8f7a406a47d39305e72/mingw-w64-git/git-wrapper.c#L166-L258
> 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.
We should probably consider rewording that warning.
>> 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
>>
> []
>
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2024, #01; Tue, 2)
From: SZEDER Gábor @ 2024-01-13 18:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Taylor Blau, git
In-Reply-To: <xmqqa5pm9tnx.fsf@gitster.g>
On Wed, Jan 03, 2024 at 10:08:18AM -0800, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> >> * tb/path-filter-fix (2023-10-18) 17 commits
> >> - bloom: introduce `deinit_bloom_filters()`
> >> ...
> >> - t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
> >>
> >> The Bloom filter used for path limited history traversal was broken
> >> on systems whose "char" is unsigned; update the implementation and
> >> bump the format version to 2.
> >>
> >> Expecting a reroll.
> >> cf. <20231023202212.GA5470@szeder.dev>
> >> source: <cover.1697653929.git.me@ttaylorr.com>
> >
> > I was confused by this one, since I couldn't figure out which tests
> > Gábor was referring to here. I responded in [1], but haven't heard back
> > since the end of October.
> > ...
> > [1]: https://lore.kernel.org/git/ZUARCJ1MmqgXfS4i@nand.local/
I keep referring to the test in:
https://public-inbox.org/git/20230830200218.GA5147@szeder.dev/
which, rather disappointingly, is still the only test out there
exercising the interaction between split commit graphs and different
modified path Bloom filter versions. Note that in that message I
mentioned that merging layers with differenet Bloom filter versions
seemed to work, but that, alas, is no longer the case, because it's
now broken in Taylor's recent iterations of the patch series.
At the risk of sounding like a broken record: the interaction of split
commit graphs and different Bloom filter versions should be thoroughly
tested.
^ permalink raw reply
* Re: [PATCH 2/2] completion: silence pseudoref existence check
From: SZEDER Gábor @ 2024-01-13 19:17 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Stan Hu
In-Reply-To: <24563975fca8df6ae73917e9ee3534933d47c429.1704969119.git.ps@pks.im>
On Thu, Jan 11, 2024 at 11:41:59AM +0100, Patrick Steinhardt wrote:
> In 44dbb3bf29 (completion: support pseudoref existence checks for
> reftables, 2023-12-19), we have extended the Bash completion script to
> support future ref backends better by using git-rev-parse(1) to check
> for pseudo-ref existence. This conversion has introduced a bug, because
> even though we pass `--quiet` to git-rev-parse(1) it would still output
> the resolved object ID of the ref in question if it exists.
>
> Fix this by redirecting its stdout to `/dev/null` and add a test that
> catches this behaviour. Note that the test passes even without the fix
> for the "files" backend because we parse pseudo refs via the filesystem
> directly in that case. But the test will fail with the "reftable"
> backend.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> contrib/completion/git-completion.bash | 2 +-
> t/t9902-completion.sh | 8 ++++++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8c40ade494..8872192e2b 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -146,7 +146,7 @@ __git_pseudoref_exists ()
> if __git_eread "$__git_repo_path/HEAD" head; then
> b="${head#ref: }"
> if [ "$b" == "refs/heads/.invalid" ]; then
Nit: I guess these last two lines above came from the git prompt
script, where we do need the name of the ref, but here we don't need
that, so the condition could have simply been
if [ "$head" = "ref: refs/heads/.invalid" ]
With a single = instead of ==, because there is no pattern matching
here.
> - __git -C "$__git_repo_path" rev-parse --verify --quiet "$ref" 2>/dev/null
> + __git -C "$__git_repo_path" rev-parse --verify --quiet "$ref" >/dev/null 2>&1
You don't need the '-C $__git_repo_path' option and you don't have to
redirect stderr either, because the purpose of the __git wrapper
function is to take care of both.
> return $?
> fi
> fi
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 78cb93bea7..b14ae4de14 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1927,6 +1927,14 @@ test_expect_success 'git checkout - --orphan with branch already provided comple
> EOF
> '
>
> +test_expect_success 'git reset completes modified files' '
The description of the test case mentions 'git reset' ...
> + test_commit A a.file &&
> + echo B >a.file &&
> + test_completion "git restore a." <<-\EOF
... but it invokes 'git restore'.
Anyway, I think it would be better to add a dedicated test or two to
exercise the __git_pseudoref_exists helper function instead (or
perhaps in addition).
> + a.file
> + EOF
> +'
> +
> test_expect_success 'teardown after ref completion' '
> git branch -d matching-branch &&
> git tag -d matching-tag &&
> --
> 2.43.GIT
>
^ permalink raw reply
* Re: [PATCH v3 1/2] completion: refactor existence checks for pseudorefs
From: SZEDER Gábor @ 2024-01-13 21:07 UTC (permalink / raw)
To: Stan Hu; +Cc: git, Patrick Steinhardt, Christian Couder
In-Reply-To: <29c928649aba7dfce4dab1b5d923bc23b7af2d32.1703022850.git.stanhu@gmail.com>
On Tue, Dec 19, 2023 at 02:14:17PM -0800, Stan Hu wrote:
> In preparation for the reftable backend, this commit introduces a
> '__git_pseudoref_exists' function that continues to use 'test -f' to
> determine whether a given pseudoref exists in the local filesystem.
>
> Signed-off-by: Stan Hu <stanhu@gmail.com>
> ---
> contrib/completion/git-completion.bash | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 13a39ebd2e..8edd002eed 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -122,6 +122,15 @@ __git ()
> ${__git_dir:+--git-dir="$__git_dir"} "$@" 2>/dev/null
> }
>
> +# Runs git in $__git_repo_path to determine whether a pseudoref exists.
> +# 1: The pseudo-ref to search
> +__git_pseudoref_exists ()
> +{
> + local ref=$1
> +
> + [ -f "$__git_repo_path/$ref" ]
This new helper function relies on $__git_repo_path being set, but it
doesn't ensure that it's actually set by calling __git_find_repo_path.
Instead, it relies on its callers calling __git_find_repo_path,
although after this patch none of those callers directly access
$__git_repo_path anymore.
It would be better to call __git_find_repo_path in this helper to make
it more self-contained, and then the now unnecessary
__git_find_repo_path calls from the three callers can be removed.
Yeah, I know it's late, because this patch is already in master, but
this would be a good preparation step for eventual dedicated tests of
__git_pseudoref_exists that came up in:
https://public-inbox.org/git/20240113191749.GB3000857@szeder.dev/
> +}
> +
> # Removes backslash escaping, single quotes and double quotes from a word,
> # stores the result in the variable $dequoted_word.
> # 1: The word to dequote.
> @@ -1625,7 +1634,7 @@ __git_cherry_pick_inprogress_options=$__git_sequencer_inprogress_options
> _git_cherry_pick ()
> {
> __git_find_repo_path
> - if [ -f "$__git_repo_path"/CHERRY_PICK_HEAD ]; then
> + if __git_pseudoref_exists CHERRY_PICK_HEAD; then
> __gitcomp "$__git_cherry_pick_inprogress_options"
> return
> fi
> @@ -2067,7 +2076,7 @@ _git_log ()
> __git_find_repo_path
>
> local merge=""
> - if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> + if __git_pseudoref_exists MERGE_HEAD; then
> merge="--merge"
> fi
> case "$prev,$cur" in
> @@ -2934,6 +2943,7 @@ _git_reset ()
>
> _git_restore ()
> {
> + __git_find_repo_path
> case "$prev" in
> -s)
> __git_complete_refs
> @@ -2952,7 +2962,7 @@ _git_restore ()
> __gitcomp_builtin restore
> ;;
> *)
> - if __git rev-parse --verify --quiet HEAD >/dev/null; then
> + if __git_pseudoref_exists HEAD; then
> __git_complete_index_file "--modified"
> fi
> esac
> @@ -2963,7 +2973,7 @@ __git_revert_inprogress_options=$__git_sequencer_inprogress_options
> _git_revert ()
> {
> __git_find_repo_path
> - if [ -f "$__git_repo_path"/REVERT_HEAD ]; then
> + if __git_pseudoref_exists REVERT_HEAD; then
> __gitcomp "$__git_revert_inprogress_options"
> return
> fi
> @@ -3592,7 +3602,7 @@ __gitk_main ()
> __git_find_repo_path
>
> local merge=""
> - if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
> + if __git_pseudoref_exists MERGE_HEAD; then
> merge="--merge"
> fi
> case "$cur" in
> --
> 2.42.0
>
>
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2024, #01; Tue, 2)
From: Taylor Blau @ 2024-01-13 22:06 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, git
In-Reply-To: <20240113183544.GA3000857@szeder.dev>
Hi Gábor,
Thanks very much for your patience while I figure all of this out. I'm
confident that with the fixup! below that your test passes in the next
round of this series.
On Sat, Jan 13, 2024 at 07:35:44PM +0100, SZEDER Gábor wrote:
> On Wed, Jan 03, 2024 at 10:08:18AM -0800, Junio C Hamano wrote:
> > Taylor Blau <me@ttaylorr.com> writes:
> >
> > >> * tb/path-filter-fix (2023-10-18) 17 commits
> > >> - bloom: introduce `deinit_bloom_filters()`
> > >> ...
> > >> - t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
> > >>
> > >> The Bloom filter used for path limited history traversal was broken
> > >> on systems whose "char" is unsigned; update the implementation and
> > >> bump the format version to 2.
> > >>
> > >> Expecting a reroll.
> > >> cf. <20231023202212.GA5470@szeder.dev>
> > >> source: <cover.1697653929.git.me@ttaylorr.com>
> > >
> > > I was confused by this one, since I couldn't figure out which tests
> > > Gábor was referring to here. I responded in [1], but haven't heard back
> > > since the end of October.
> > > ...
> > > [1]: https://lore.kernel.org/git/ZUARCJ1MmqgXfS4i@nand.local/
>
> I keep referring to the test in:
>
> https://public-inbox.org/git/20230830200218.GA5147@szeder.dev/
>
> which, rather disappointingly, is still the only test out there
> exercising the interaction between split commit graphs and different
> modified path Bloom filter versions. Note that in that message I
> mentioned that merging layers with differenet Bloom filter versions
> seemed to work, but that, alas, is no longer the case, because it's
> now broken in Taylor's recent iterations of the patch series.
Thanks for clarifying. With all of the different versions of this series
and the couple of tests that you and I were talking about, I think that
I got confused and thought you were referring to a different test.
Indeed, the current version of this series (v4) does not pass the test
in <20230830200218.GA5147@szeder.dev>, but the fix in order for it to
pass is relatively straightforward.
I have this on top of my local branch as a fixup! and I'll squash it
down on Tuesday[^1] and send a reroll after double-checking that the fix
is correct and doesn't conflict with any other parts of the series.
Since it's been so long since I've looked at this, I'll re-read the
series as a whole with fresh eyes to make sure that everything still
makes sense.
In any case, here's the patch on top (with a lightly modified version of
the test you wrote in <20230830200218.GA5147@szeder.dev>:
Subject: [PATCH] fixup! commit-graph: ensure Bloom filters are read with
consistent settings
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
commit-graph.c | 3 ++-
t/t4216-log-bloom.sh | 29 ++++++++++++++++++++++++++++-
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 60fa64d956..82a4632c4e 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -507,7 +507,8 @@ static void validate_mixed_bloom_settings(struct commit_graph *g)
}
if (g->bloom_filter_settings->bits_per_entry != settings->bits_per_entry ||
- g->bloom_filter_settings->num_hashes != settings->num_hashes) {
+ g->bloom_filter_settings->num_hashes != settings->num_hashes ||
+ g->bloom_filter_settings->hash_version != settings->hash_version) {
g->chunk_bloom_indexes = NULL;
g->chunk_bloom_data = NULL;
FREE_AND_NULL(g->bloom_filter_settings);
diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh
index 569f2b6f8b..d8c42e2e27 100755
--- a/t/t4216-log-bloom.sh
+++ b/t/t4216-log-bloom.sh
@@ -438,7 +438,7 @@ test_expect_success 'setup for mixed Bloom setting tests' '
done
'
-test_expect_success 'ensure incompatible Bloom filters are ignored' '
+test_expect_success 'ensure Bloom filters with incompatible settings are ignored' '
# Compute Bloom filters with "unusual" settings.
git -C $repo rev-parse one >in &&
GIT_TEST_BLOOM_SETTINGS_NUM_HASHES=3 git -C $repo commit-graph write \
@@ -488,6 +488,33 @@ test_expect_success 'merge graph layers with incompatible Bloom settings' '
test_must_be_empty err
'
+test_expect_success 'ensure Bloom filter with incompatible versions are ignored' '
+ rm "$repo/$graph" &&
+
+ git -C $repo log --oneline --no-decorate -- $CENT >expect &&
+
+ # Compute v1 Bloom filters for commits at the bottom.
+ git -C $repo rev-parse HEAD^ >in &&
+ git -C $repo commit-graph write --stdin-commits --changed-paths \
+ --split <in &&
+
+ # Compute v2 Bloomfilters for the rest of the commits at the top.
+ git -C $repo rev-parse HEAD >in &&
+ git -C $repo -c commitGraph.changedPathsVersion=2 commit-graph write \
+ --stdin-commits --changed-paths --split=no-merge <in &&
+
+ test_line_count = 2 $repo/$chain &&
+
+ git -C $repo log --oneline --no-decorate -- $CENT >actual 2>err &&
+ test_cmp expect actual &&
+
+ layer="$(head -n 1 $repo/$chain)" &&
+ cat >expect.err <<-EOF &&
+ warning: disabling Bloom filters for commit-graph layer $SQ$layer$SQ due to incompatible settings
+ EOF
+ test_cmp expect.err err
+'
+
get_first_changed_path_filter () {
test-tool read-graph bloom-filters >filters.dat &&
head -n 1 filters.dat
--
2.38.0.16.g393fd4c6db
(Excuse the old version of Git here, I'm in the middle of a long-running
process which checks out older versions of the tree to count the number
of unused-parameters over time.)
Thanks,
Taylor
[^1]: Monday is a US holiday, so I likely won't be at my computer.
^ permalink raw reply related
* Re: [PATCH v2 2/2] t5541: remove lockfile creation
From: Justin Tobler @ 2024-01-13 22:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Justin Tobler via GitGitGadget, git,
Patrick Steinhardt
In-Reply-To: <xmqqo7dqbfin.fsf@gitster.g>
Great! Thanks everyone for the help!
-Justin
On Fri, Jan 12, 2024 at 11:58 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > On Thu, Jan 11, 2024 at 08:24:30PM +0000, Justin Tobler via GitGitGadget wrote:
> >
> >> - # the new branch should not have been created upstream
> >> - test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
> >> -
> >> - # upstream should still reflect atomic2, the last thing we pushed
> >> - # successfully
> >> - git rev-parse atomic2 >expected &&
> >> - # ...to other.
> >> - git -C "$d" rev-parse refs/heads/other >actual &&
> >> - test_cmp expected actual &&
> >> -
> >> - # the new branch should not have been created upstream
> >> + # The atomic and other branches should be created upstream.
> >> test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
> >> + test_must_fail git -C "$d" show-ref --verify refs/heads/other &&
> >
> > This last comment should say "should not be created", I think?
> >
> > Other than that, both patches look good to me.
>
> Thanks. Will queue with the following and "Acked-by: peff".
>
> diff --git c/t/t5541-http-push-smart.sh w/t/t5541-http-push-smart.sh
> index 9a8bed6c32..71428f3d5c 100755
> --- c/t/t5541-http-push-smart.sh
> +++ w/t/t5541-http-push-smart.sh
> @@ -242,7 +242,7 @@ test_expect_success 'push --atomic fails on server-side errors' '
> # --atomic should cause entire push to be rejected
> test_must_fail git push --atomic "$up" atomic other 2>output &&
>
> - # The atomic and other branches should be created upstream.
> + # The atomic and other branches should not be created upstream.
> test_must_fail git -C "$d" show-ref --verify refs/heads/atomic &&
> test_must_fail git -C "$d" show-ref --verify refs/heads/other &&
>
>
^ 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