* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-11-03 18:38 UTC (permalink / raw)
To: Andy Koppe
Cc: Thomas Guyot, Junio C Hamano, Jeff King, Christoph Anton Mitterer,
git
In-Reply-To: <CAHWeT-aftfawZ9DhqC1NkQrJAuArJZC65ZDr4FyPwEc7aXGVuA@mail.gmail.com>
On 2023-11-03 16:28, Andy Koppe wrote:
> Thomas Guyot wrote:
>> >> I actually have one major issue with it, it's that displaying anything
>> >> less than a full page will fill the screen with ~ on the bottom, just
>> >> like when scrolling up on a partial page without -F.
>
> 'less' has the '-~' (or --tilde) option to suppress that.
Good to know, thanks. However, with the "-F" option in place, the
situation in which less than a full page is displayed within less(1)
shouldn't be encountered.
> For what it's worth, the 'mintty' terminal used by default for Git for
> Windows as well as MSYS and Cygwin has another approach to the whole
> problem. Its rather flippantly named 'Flip Screen' context menu
> command with Alt+F12 or Ctrl+Shift+S shortcut lets users temporarily
> look at the alternate screen buffer while the main screen buffer is
> active, and vice versa.
This is a rather neat feature. Though, I wonder how many users are
actually aware of this feature, and how frequently is it used. I wasn't
aware of it, and I used Git CLI on Windows for some time.
> If 'less' is invoked without the -X option, it will switch to the
> alternate screen, where mousewheel scrolling works by sending cursor
> up/down keycodes. While in 'less', you can temporarily flip to the
> main screen to look up something in the shell session there or copy
> something for searching in 'less'. While looking at the main screen,
> the mousewheel will scroll the scrollback buffer. Keyboard input
> that's sent to 'less' will flip back to the alternate screen.
To me, this is another confirmation that (ab)using the "-X" option is
something that we need to get rid of, as I already described earlier in
this email thread.
> Quitting 'less' switches back to the main screen, so the 'less' output
> disappears and you're back in the shell session with the command that
> invoked 'less' as the last thing shown. But again, the 'Flip Screen'
> command or shortcuts can be used to temporarily look at or copy from
> the alternate screen, which will contain the last page displayed by
> 'less'. (The alternate screen does not have a scrollback buffer.)
>
> The 'Flip Screen' feature of course also works with other
> alternate-screen applications, for example editors.
>
> Apparently the Mac terminal has such a feature as well:
> https://support.apple.com/en-ie/guide/terminal/trmld1f46097/mac
Maybe there is some data available about how frequently this neat
feature is used? It would be really good to know how much is it
actually used.
> (Full disclosure: I originally made mintty, from PuTTY.)
Thanks for your work!
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-11-03 18:22 UTC (permalink / raw)
To: Thomas Guyot; +Cc: Junio C Hamano, Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <24c46f25-6b31-437d-9f89-1e8eb74136c8@gmail.com>
On 2023-11-03 12:47, Thomas Guyot wrote:
> On 2023-11-02 10:19, Dragan Simic wrote:
>> On 2023-11-02 14:19, Thomas Guyot wrote:
>>> That's correct, you need both and also -y0
>>
>> Hmm, I tried the following:
>>
>> GIT_PAGER='less -R -F -X -c -y0'
>>
>> In my environment (Xfce), the result after scrolling the output of
>> "git
>> log -p" up and down a bit was about 20 copies of the same screen
>> "page"
>> in the scrollback, plus a couple of blank "pages". Not good,
>> unfortunately, and actually much worse than having just "-R -F -X".
>
> Indeed, I did notice it with xfce4-term - I suspect different controls
> are used to clear the screen, with rxvt-unicode the initial one
> scrolls anything in the current display to the scrollback (which is
> important to avoid clearing the last commands/output form the
> scrollback), but afterward screen updates do not update the
> scrollback, it remains within the screen.
>
> These options may actually affect the behavior (I have put my current
> values there)
>
> secondaryScreen: True
> Turn on/off secondary screen (default enabled).
>
> secondaryScroll: True
> Turn on/off secondary screen scroll (default enabled). If this
> option is enabled, scrolls on the secondary screen will change
> the
> scrollback buffer and, when secondaryScreen is off, switching
> to/from the secondary screen will instead scroll the screen up.
>
> So it appears I could disable secondaryScroll to avoid getting
> scrolled lines into the scrollback buffer. It's also noteworthy that
> with secondaryScreen disabled, rxvt-unicode instead scroll things up
> to avoid the current screen form being wiped out.
This is quite interesting. However, we can't expect that the users
perform such adjustments to their environments, if you agree. Tweaking
one's own environment is fine, but the shipped solution should "just
work" in most environments, or ideally in all environments in their
original form.
>>> Indeed, but when less update from the bottom, it can add new lines
>>> and
>>> let the overflow lines scroll up into the scrollback buffer.
>>>
>>> Then updating it from the top, it draws the whole page, top to
>>> bottom.
>>> That's fine for a full page but not desired for a partial one. Also
>>> note that on my terminal (rxvt-unicode) when less clears the screen
>>> to
>>> draw the first page the current screen is rolled up into scrollback -
>>> iirc that's a configurable option, it would be worth testing other
>>> terminal's behavior on that. IIRC it may also erase it when using the
>>> wrong termcap file.
>>>
>>> I haven't looked at the code, but I think it could be possibly to
>>> start the -c behavior only after a full page is drawn, after exiting
>>> on partial pages, which would give us the best of both worlds.
>>
>> Does the GIT_PAGER setup, as I described it above, work for you
>> without
>> the described artifacts, in any of the environments you have access
>> to?
>
> Yes, rxvt-unicode, with the settings mentioned above.
Good to know, thanks. Looking forward to see will the
"--redraw-on-quit" option work as expected in your rxvt-unicode
terminal.
>> One of the solutions is to edit the terminfo entry manually and remove
>> the escape codes that cause the switching to and from alternate
>> screen,
>> which I tested, but that also introduced another issue -- the screen
>> contents was always present after less(1) exited, which isn't always
>> the
>> desired behavior.
>
> But when less scrolls down (line by line, not page by page), it always
> append lines and let them scroll up. Won't you see these?
> (page-by-page otoh will redraw the full screen without scrolling).
>
> Also won't it wipe the *current* screen so that you won't see the
> commands/output you had *before* running less?
>
> This is why rxvt-unicode has an option to disable secondary screen,
> and scroll contents all the way to the buffer on switch to/from
> secondary screen.
IIRC, when I tested by removing the escape codes manually from the
terminfo entry, it all worked fine, but the screen contents always
remained displayed after less(1) exited. That isn't always the desired
behavior.
>>> Actually I just tested my
>>> solution on xfce4-terminal and it doesn't work, the terminal still
>>> push up stuff above on redraw (noteworthy is with rxvt-unicode the
>>> first draw pushes the current screen contents up but no other redraw
>>> does, which is what makes it work so well - I haven't tried to find
>>> out what is being done exactly... OTOH the redraw on scroll down is
>>> slightly noticeable there, while impossible to see on xfce4-terminal.
>>> I'll install the latest less and see what happens with --redraw on
>>
>> Please test the "--redraw-on-quit" option, so far it's the best
>> available solution, IMHO.
>
> I will, not now though - need it compile less form source I guess...
Great, thanks. Looking forward to the results of your testing. As a
reminder, this is what I use:
GIT_PAGER='less -R -F --redraw-on-quit'
My LESS environment variable contains only some coloring-related
options, which don't matter in this case.
>>> Now, if all other terms misbehave here, that's an issue, making this
>>> suggestion mostly useless. And considering the number of Windows
>>> users
>>> we absolutely need to test Windows Terminal, and should probably test
>>> MacOS's term too (whatever that is).
>>
>> Quite frankly, I think that such a solution would be like "fixing the
>> fix, which is actually an abuse", as I described it above, eventually
>> introducing even more issues, instead of solving the original issue.
>
> I'll let you know my findings... I'm not convinced --redraw-on-quit is
> actually going to fix it for all unless this does a lot more than the
> option name implies (but quite happy if it does).
Actually, it's more about not (ab)using the "-X" option, because
skipping the terminal initialization may cause various issues. The
"--redraw-on-quit" option is there just to have the screen contents
preserved after less(1) exits.
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Kousik Sanagavarapu @ 2023-11-03 17:20 UTC (permalink / raw)
To: Andy Koppe; +Cc: Liam Beguin, git
In-Reply-To: <4e3a5661-cba0-4849-8340-06d8d4094f48@gmail.com>
On Fri, Nov 03, 2023 at 08:22:05AM +0000, Andy Koppe wrote:
>
> On 27/10/2023 19:40, Kousik Sanagavarapu wrote:
> > So, if we have a domain-name, we grab it, else (the case where we don't
> > have '@') we grab it as-is. Looks good.
>
> I'm not sure that this is the right way to handle a missing '@' here
> actually, because %al already returns the whole email field in that case,
> which makes sense as the likes of the 'mail' command would interpret it as a
> local username.
>
> And if someone was going to use %al and the new specifier together to format
> the parts of the email field differently, they probably wouldn't want the
> field to appear twice.
>
> Therefore I think it would be more appropriate to expand to nothing in that
> case. Tools that consume this output would already need to be able to deal
> with the empty case, as it could also happen if there's a single '@' at the
> end of the email field, or if the field is empty.
I originally thought since localpart and the new domainpart are like
counterparts (or are symmetrical, like Peff mentioned), falling back
like in the case of localpart was the correct way here (again, symmetry).
Having read your reasoning though I think it makes sense to not fall back the
same way as localpart (%al) and return empty instead.
This goes in favor of what Liam said in the original commit message of this
feature being used to keep track of commits from different organizations (as
in from GitHub or GitLab) or at least domains that make sense. As, if we
return the whole email (where the email is with no domain-part or '@' is at
the end of the email), the whole purpose of our new feature is lost.
Thanks
^ permalink raw reply
* [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Joanna Wang @ 2023-11-03 16:34 UTC (permalink / raw)
To: jojwang; +Cc: git, gitster
In-Reply-To: <20231103143508.896745-1-jojwang@google.com>
This lets users limit files or exclude files based on file
attributes during git-add and git-stash.
For example, the chromium project would like to use this like
"git add --all ':(exclude,attr:submodule)'", as submodules are managed in a
unique way and often results in submodule changes that users do not want in
their commits.
This does not change any attr magic implementation. It is only adding
attr as an allowed pathspec in git-add and git-stash, which was previously
blocked by GUARD_PATHSPEC and a pathspec mask in parse_pathspec()).
However, this does fix a bug in prefix_magic() where attr values were unintentionally removed.
This was hit whenever parse_pathspec() is called with PATHSPEC_PREFIX_ORIGIN as a flag,
which was the case for git-stash.
(Bug originally filed here:
https://lore.kernel.org/git/CAMmZTi-0QKtj7Q=sbC5qhipGsQxJFOY-Qkk1jfkRYwfF5FcUVg@mail.gmail.com/)
Furthermore, while other commands hit this code path it did not result in unexpected
behavior because this bug only impacts the pathspec->items->original field which is
NOT used to filter paths. However, git-stash does use pathspec->items->original when
building args used to call other git commands.
(See add_pathspecs() usage and implementation in stash.c)
It is possible that when the attr pathspec feature was first added in
b0db704652 (pathspec: allow querying for attributes, 2017-03-13),
"PATHSPEC_ATTR" was just unintentionally left out of a few GUARD_PATHSPEC() invocations.
Later, to get a more user-friendly error message when attr was used with git-add,
PATHSPEC_ATTR was added as a mask to git-add's invocation of parse_pathspec()
84d938b732 (add: do not accept pathspec magic 'attr', 2018-09-18).
However, this user-friendly error message was never added for git-stash.
Signed-off-by: Joanna Wang <jojwang@google.com>
---
I fixed a typo above. "(But originally" -> "(Bug originally"
> So if element magic is not 0, we know the body is in the expected
> form (either long or short).
Also while this is still true AFAICT, I found an existing bug in parse_short_magic()
where it does not handle paths that start with ":" where ":" is part of the
file name. e.g. "git stash push -- ':chicken'" is not handled correcly. But I feel
this is a seperate topic from this patch. So I'll file a bug to address it seperately.
builtin/add.c | 7 ++-
dir.c | 3 +-
pathspec.c | 36 ++++++++---
t/t6135-pathspec-with-attrs.sh | 108 +++++++++++++++++++++++++++++++--
4 files changed, 136 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 5126d2ede3..d46e4d10e9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -424,7 +424,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
* Check the "pathspec '%s' did not match any files" block
* below before enabling new magic.
*/
- parse_pathspec(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, argv);
@@ -433,7 +433,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (pathspec.nr)
die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
- parse_pathspec_file(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec_file(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, pathspec_from_file, pathspec_file_nul);
@@ -504,7 +504,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
diff --git a/dir.c b/dir.c
index 16fdb03f2a..4d1cd039be 100644
--- a/dir.c
+++ b/dir.c
@@ -2179,7 +2179,8 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec->nr; i++) {
const struct pathspec_item *item = &pathspec->items[i];
diff --git a/pathspec.c b/pathspec.c
index bb1efe1f39..2f8721cc15 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -109,16 +109,34 @@ static struct pathspec_magic {
{ PATHSPEC_ATTR, '\0', "attr" },
};
-static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
+static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic, const char *element)
{
- int i;
- strbuf_addstr(sb, ":(");
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
- if (magic & pathspec_magic[i].bit) {
- if (sb->buf[sb->len - 1] != '(')
- strbuf_addch(sb, ',');
- strbuf_addstr(sb, pathspec_magic[i].name);
+ /* No magic was found in element, just add prefix magic */
+ if (magic == 0) {
+ strbuf_addf(sb, ":(prefix:%d)", prefixlen);
+ return;
+ }
+
+ /*
+ * At this point we known that parse_element_magic() was able to extract some pathspec
+ * magic from element. So we know element is correctly formatted in either shorthand
+ * or longhand form
+ */
+ if (element[1] != '(') {
+ /* Process an element in shorthand form (e.g. ":!/<match>") */
+ strbuf_addstr(sb, ":(");
+ for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
+ if (sb->buf[sb->len - 1] != '(')
+ strbuf_addch(sb, ',');
+ strbuf_addstr(sb, pathspec_magic[i].name);
+ }
}
+ } else {
+ /* For the longhand form, we copy everything up to the final ')' */
+ size_t len = strchr(element, ')') - element;
+ strbuf_add(sb, element, len);
+ }
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
@@ -493,7 +511,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
struct strbuf sb = STRBUF_INIT;
/* Preserve the actual prefix length of each pattern */
- prefix_magic(&sb, prefixlen, element_magic);
+ prefix_magic(&sb, prefixlen, element_magic, elt);
strbuf_addstr(&sb, match);
item->original = strbuf_detach(&sb, NULL);
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index f70c395e75..e46fa176ed 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -64,12 +64,24 @@ test_expect_success 'setup .gitattributes' '
fileSetLabel label
fileValue label=foo
fileWrongLabel label☺
+ newFileA* labelA
+ newFileB* labelB
EOF
echo fileSetLabel label1 >sub/.gitattributes &&
git add .gitattributes sub/.gitattributes &&
git commit -m "add attributes"
'
+test_expect_success 'setup .gitignore' '
+ cat <<-\EOF >.gitignore &&
+ actual
+ expect
+ pathspec_file
+ EOF
+ git add .gitignore &&
+ git commit -m "add gitignore"
+'
+
test_expect_success 'check specific set attr' '
cat <<-\EOF >expect &&
fileSetLabel
@@ -150,6 +162,7 @@ test_expect_success 'check specific value attr (2)' '
test_expect_success 'check unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileA
fileAB
fileAC
@@ -175,6 +188,7 @@ test_expect_success 'check unspecified attr' '
test_expect_success 'check unspecified attr (2)' '
cat <<-\EOF >expect &&
HEAD:.gitattributes
+ HEAD:.gitignore
HEAD:fileA
HEAD:fileAB
HEAD:fileAC
@@ -200,6 +214,7 @@ test_expect_success 'check unspecified attr (2)' '
test_expect_success 'check multiple unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileC
fileNoLabel
fileWrongLabel
@@ -239,16 +254,99 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
test_i18ngrep "Only one" actual
'
-test_expect_success 'fail if attr magic is used places not implemented' '
+test_expect_success 'fail if attr magic is used in places not implemented' '
# The main purpose of this test is to check that we actually fail
# when you attempt to use attr magic in commands that do not implement
- # attr magic. This test does not advocate git-add to stay that way,
- # though, but git-add is convenient as it has its own internal pathspec
- # parsing.
- test_must_fail git add ":(attr:labelB)" 2>actual &&
+ # attr magic. This test does not advocate check-ignore to stay that way.
+ # When you teach the command to grok the pathspec, you need to find
+ # another command to replace it for the test.
+ test_must_fail git check-ignore ":(attr:labelB)" 2>actual &&
test_i18ngrep "magic not supported" actual
'
+test_expect_success 'check that attr magic works for git stash push' '
+ cat <<-\EOF >expect &&
+ A sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git stash push --include-untracked -- ":(exclude,attr:labelB)" &&
+ git stash show --include-untracked --name-status >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --all' '
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -W -S . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add -u' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add -u ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . && rm sub/new* &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add <path>' '
+ cat <<-\EOF >expect &&
+ fileA
+ fileB
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add ":(exclude,attr:labelB)sub/*" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git -add .' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ cd sub &&
+ git add . ":(exclude,attr:labelB)" &&
+ cd .. &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --pathspec-from-file' '
+ cat <<-\EOF >pathspec_file &&
+ :(exclude,attr:labelB)
+ EOF
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all --pathspec-from-file=pathspec_file &&
+ git diff --name-only --cached >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'abort on giving invalid label on the command line' '
test_must_fail git ls-files . ":(attr:☺)"
'
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: Jeff King @ 2023-11-03 16:21 UTC (permalink / raw)
To: rsbecker; +Cc: git
In-Reply-To: <007f01da0e6f$d5031a40$7f094ec0$@nexbridge.com>
On Fri, Nov 03, 2023 at 12:07:17PM -0400, rsbecker@nexbridge.com wrote:
> What I get from Perl is
> $ perl -e '
> > my $bytes = "\1\2\3\4\5\6\7\8";
> > my $q = eval { unpack("Q>", $bytes) };
> > print "Q = ", defined($q) ? $q : "($@)", "\n";
> > my ($n1, $n2) = unpack("NN", $bytes);
> > print "n1 = $n1\n";
> > print "n2 = $n2\n";
> > print "computed quad = ", ($n1 << 32) | $n2, "\n"; '
> Q = (Invalid type 'Q' in unpack at -e line 3.
> )
> n1 = 16909060
> n2 = 84281144
> computed quad = 84281144
OK, that matches what I expected. Hopefully the patch I just sent (our
mails just crossed) will fix it for you.
> Because perl itself is 32-bit, not 64-bit on this platform. So even
> moving git to 64-bit will not correct the issue.
Yep, exactly.
-Peff
^ permalink raw reply
* [PATCH] t: avoid perl's pack/unpack "Q" specifier
From: Jeff King @ 2023-11-03 16:20 UTC (permalink / raw)
To: rsbecker; +Cc: git
In-Reply-To: <007c01da0e6f$014e2c30$03ea8490$@nexbridge.com>
On Fri, Nov 03, 2023 at 12:01:22PM -0400, rsbecker@nexbridge.com wrote:
> Explains a lot. We are only able to build git in 32-bit mode because
> of OS dependencies (only available in 32-bit). I did not know that
> 64-bit was now required for git. We will get there, but it will
> probably take years.
It's not required for Git. And even on 32-bit systems there is usually
some compiler support for 64-bit types via "uint64_t", etc. And likewise
perl can probably be built for 64-bit types even on a 32-bit system (the
tests do pass on our Linux 32-bit build), but your perl simply wasn't.
I think we can accommodate it, though. Did you try the snippet I showed?
If it behaves as I think it does, then this patch should hopefully fix
things for you:
-- >8 --
Subject: [PATCH] t: avoid perl's pack/unpack "Q" specifier
The perl script introduced by 86b008ee61 (t: add library for munging
chunk-format files, 2023-10-09) uses pack("Q") and unpack("Q") to read
and write 64-bit values ("quadwords" in perl parlance) from the on-disk
chunk files. However, some builds of perl may not support 64-bit
integers at all, and throw an exception here. While some 32-bit
platforms may still support 64-bit integers in perl (such as our linux32
CI environment), others reportedly don't (the NonStop 32-bit builds).
We can work around this by treating the 64-bit values as two 32-bit
values. We can't ever combine them into a single 64-bit value, but in
practice this is OK. These are representing file offsets, and our files
are much smaller than 4GB. So the upper half of the 64-bit value will
always be 0.
We can just introduce a few helper functions which perform the
translation and double-check our assumptions.
Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>
Signed-off-by: Jeff King <peff@peff.net>
---
If this does work, it should go on top of jk/chunk-bounds.
t/lib-chunk/corrupt-chunk-file.pl | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/t/lib-chunk/corrupt-chunk-file.pl b/t/lib-chunk/corrupt-chunk-file.pl
index cd6d386fef..b024bbdcb5 100644
--- a/t/lib-chunk/corrupt-chunk-file.pl
+++ b/t/lib-chunk/corrupt-chunk-file.pl
@@ -21,14 +21,38 @@ sub copy {
return $buf;
}
+# Some platforms' perl builds don't support 64-bit integers, and hence do not
+# allow packing/unpacking quadwords with "Q". The chunk format uses 64-bit file
+# offsets to support files of any size, but in practice our test suite will
+# only use small files. So we can fake it by asking for two 32-bit values and
+# discarding the first (most significant) one, which is equivalent as long as
+# it's just zero.
+sub unpack_quad {
+ my $bytes = shift;
+ my ($n1, $n2) = unpack("NN", $bytes);
+ die "quad value exceeds 32 bits" if $n1;
+ return $n2;
+};
+sub pack_quad {
+ my $n = shift;
+ my $ret = pack("NN", 0, $n);
+ # double check that our original $n did not exceed the 32-bit limit.
+ # This is presumably impossible on a 32-bit system (which would have
+ # truncated much earlier), but would still alert us on a 64-bit build
+ # of a new test that would fail on a 32-bit build (though we'd
+ # presumably see the die() from unpack_quad() in such a case).
+ die "quad round-trip failed" if unpack_quad($ret) != $n;
+ return $ret;
+}
+
# read until we find table-of-contents entry for chunk;
# note that we cheat a bit by assuming 4-byte alignment and
# that no ToC entry will accidentally look like a header.
#
# If we don't find the entry, copy() will hit EOF and exit
# (which should cause the caller to fail the test).
while (copy(4) ne $chunk) { }
-my $offset = unpack("Q>", copy(8));
+my $offset = unpack_quad(copy(8));
# In clear mode, our length will change. So figure out
# the length by comparing to the offset of the next chunk, and
@@ -38,11 +62,11 @@ sub copy {
my $id;
do {
$id = copy(4);
- my $next = unpack("Q>", get(8));
+ my $next = unpack_quad(get(8));
if (!defined $len) {
$len = $next - $offset;
}
- print pack("Q>", $next - $len + length($bytes));
+ print pack_quad($next - $len + length($bytes));
} while (unpack("N", $id));
}
--
2.42.0.1037.g1c5f731f9d
^ permalink raw reply related
* RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: rsbecker @ 2023-11-03 16:07 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git
In-Reply-To: <20231103155220.GA1470570@coredump.intra.peff.net>
On November 3, 2023, Jeff King <peff@peff.net> wrote:
>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:
>
>> In RC0, the following tests are failing (with verbose). They look like
>> the same root cause. Unpack("Q>".... What version does git now require for perl?
>> I have v5.30.3 available, nothing more recent.
>
>The perl used in the test suite is supposed to be vanilla enough to support any
>ancient version. The perl5 Git import doesn't have version tags that go back that far,
>but the quadwords in pack/unpack go back at least to a commit from 1998.
>
>So I suspect this is not a version issue, but rather a build-time config one. The docs
>say:
>
> Q An unsigned quad value.
> (Quads are available only if your system supports 64-bit integer
> values _and_ if Perl has been compiled to support those. Raises
> an exception otherwise.)
>
>It would probably be possible to rewrite the use of "Q" here to grab two 32-bit
>values instead. But I'd guess that on your system it is not as simple as a shift-and-
>add to then treat them as a 64-bit value, since presumably the problem is that perl's
>ints are all strictly 32-bit.
>
>What does this script produce for you:
>
> perl -e '
> my $bytes = "\1\2\3\4\5\6\7\8";
> my $q = eval { unpack("Q>", $bytes) };
> print "Q = ", defined($q) ? $q : "($@)", "\n";
> my ($n1, $n2) = unpack("NN", $bytes);
> print "n1 = $n1\n";
> print "n2 = $n2\n";
> print "computed quad = ", ($n1 << 32) | $n2, "\n";
> '
>
>I get:
>
> Q = 72623859790382904
> n1 = 16909060
> n2 = 84281144
> computed quad = 72623859790382904
>
>but I'm guessing you get an exception report for Q, and that the computed quad is
>probably equal to n2 (the shift of n1 goes totally off the end).
>
>We may not be without hope, though. These 64-bit values are file offsets we're
>reading from the chunk files. The format naturally uses 64-bit values here to
>accommodate arbitrarily large files. But in our tests, the offsets are all going to be
>relatively small. So our "$n1" in practice will always be 0.
>
>> This same problem also happens in t5318, t5319, t5324
>
>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those
>scripts which use it will run into the same problem.
What I get from Perl is
$ perl -e '
> my $bytes = "\1\2\3\4\5\6\7\8";
> my $q = eval { unpack("Q>", $bytes) };
> print "Q = ", defined($q) ? $q : "($@)", "\n";
> my ($n1, $n2) = unpack("NN", $bytes);
> print "n1 = $n1\n";
> print "n2 = $n2\n";
> print "computed quad = ", ($n1 << 32) | $n2, "\n"; '
Q = (Invalid type 'Q' in unpack at -e line 3.
)
n1 = 16909060
n2 = 84281144
computed quad = 84281144
Because perl itself is 32-bit, not 64-bit on this platform. So even moving git to 64-bit will not correct the issue.
^ permalink raw reply
* RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: rsbecker @ 2023-11-03 16:01 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git
In-Reply-To: <20231103155220.GA1470570@coredump.intra.peff.net>
On Jeff King <peff@peff.net> wrote:
>On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:
>> In RC0, the following tests are failing (with verbose). They look like
>> the same root cause. Unpack("Q>".... What version does git now require for perl?
>> I have v5.30.3 available, nothing more recent.
>
>The perl used in the test suite is supposed to be vanilla enough to support any
>ancient version. The perl5 Git import doesn't have version tags that go back that far,
>but the quadwords in pack/unpack go back at least to a commit from 1998.
>
>So I suspect this is not a version issue, but rather a build-time config one. The docs
>say:
>
> Q An unsigned quad value.
> (Quads are available only if your system supports 64-bit integer
> values _and_ if Perl has been compiled to support those. Raises
> an exception otherwise.)
>
>It would probably be possible to rewrite the use of "Q" here to grab two 32-bit
>values instead. But I'd guess that on your system it is not as simple as a shift-and-
>add to then treat them as a 64-bit value, since presumably the problem is that perl's
>ints are all strictly 32-bit.
>
>What does this script produce for you:
>
> perl -e '
> my $bytes = "\1\2\3\4\5\6\7\8";
> my $q = eval { unpack("Q>", $bytes) };
> print "Q = ", defined($q) ? $q : "($@)", "\n";
> my ($n1, $n2) = unpack("NN", $bytes);
> print "n1 = $n1\n";
> print "n2 = $n2\n";
> print "computed quad = ", ($n1 << 32) | $n2, "\n";
> '
>
>I get:
>
> Q = 72623859790382904
> n1 = 16909060
> n2 = 84281144
> computed quad = 72623859790382904
>
>but I'm guessing you get an exception report for Q, and that the computed quad is
>probably equal to n2 (the shift of n1 goes totally off the end).
>
>We may not be without hope, though. These 64-bit values are file offsets we're
>reading from the chunk files. The format naturally uses 64-bit values here to
>accommodate arbitrarily large files. But in our tests, the offsets are all going to be
>relatively small. So our "$n1" in practice will always be 0.
>
>> This same problem also happens in t5318, t5319, t5324
>
>Yep. The offending code is in lib-chunk.sh, so the new tests added in all of those
>scripts which use it will run into the same problem.
Explains a lot. We are only able to build git in 32-bit mode because of OS dependencies (only available in 32-bit). I did not know that 64-bit was now required for git. We will get there, but it will probably take years.
^ permalink raw reply
* Re: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: Jeff King @ 2023-11-03 15:52 UTC (permalink / raw)
To: rsbecker; +Cc: git
In-Reply-To: <007101da0e65$13e3c170$3bab4450$@nexbridge.com>
On Fri, Nov 03, 2023 at 10:50:19AM -0400, rsbecker@nexbridge.com wrote:
> In RC0, the following tests are failing (with verbose). They look like the
> same root cause. Unpack("Q>".... What version does git now require for perl?
> I have v5.30.3 available, nothing more recent.
The perl used in the test suite is supposed to be vanilla enough to
support any ancient version. The perl5 Git import doesn't have version
tags that go back that far, but the quadwords in pack/unpack go back at
least to a commit from 1998.
So I suspect this is not a version issue, but rather a build-time config
one. The docs say:
Q An unsigned quad value.
(Quads are available only if your system supports 64-bit integer
values _and_ if Perl has been compiled to support those. Raises
an exception otherwise.)
It would probably be possible to rewrite the use of "Q" here to grab two
32-bit values instead. But I'd guess that on your system it is not as
simple as a shift-and-add to then treat them as a 64-bit value, since
presumably the problem is that perl's ints are all strictly 32-bit.
What does this script produce for you:
perl -e '
my $bytes = "\1\2\3\4\5\6\7\8";
my $q = eval { unpack("Q>", $bytes) };
print "Q = ", defined($q) ? $q : "($@)", "\n";
my ($n1, $n2) = unpack("NN", $bytes);
print "n1 = $n1\n";
print "n2 = $n2\n";
print "computed quad = ", ($n1 << 32) | $n2, "\n";
'
I get:
Q = 72623859790382904
n1 = 16909060
n2 = 84281144
computed quad = 72623859790382904
but I'm guessing you get an exception report for Q, and that the
computed quad is probably equal to n2 (the shift of n1 goes totally off
the end).
We may not be without hope, though. These 64-bit values are file offsets
we're reading from the chunk files. The format naturally uses 64-bit
values here to accommodate arbitrarily large files. But in our tests,
the offsets are all going to be relatively small. So our "$n1" in
practice will always be 0.
> This same problem also happens in t5318, t5319, t5324
Yep. The offending code is in lib-chunk.sh, so the new tests added in
all of those scripts which use it will run into the same problem.
-Peff
^ permalink raw reply
* Re: [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Ruben Safir @ 2023-11-03 15:31 UTC (permalink / raw)
To: Joanna Wang; +Cc: gitster, git
In-Reply-To: <20231103143508.896745-1-jojwang@google.com>
Is that really you name or is it a bad joke?
On Fri, Nov 03, 2023 at 02:35:07PM +0000, Joanna Wang wrote:
> This lets users limit files or exclude files based on file
> attributes during git-add and git-stash.
> For example, the chromium project would like to use this like
> "git add --all ':(exclude,attr:submodule)'", as submodules are managed in a
> unique way and often results in submodule changes that users do not want in
> their commits.
>
> This does not change any attr magic implementation. It is only adding
> attr as an allowed pathspec in git-add and git-stash, which was previously
> blocked by GUARD_PATHSPEC and a pathspec mask in parse_pathspec()).
> However, this does fix a bug in prefix_magic() where attr values were unintentionally removed.
> This was hit whenever parse_pathspec() is called with PATHSPEC_PREFIX_ORIGIN as a flag,
> which was the case for git-stash.
> (But originally filed here:
> https://lore.kernel.org/git/CAMmZTi-0QKtj7Q=sbC5qhipGsQxJFOY-Qkk1jfkRYwfF5FcUVg@mail.gmail.com/)
>
> Furthermore, while other commands hit this code path it did not result in unexpected
> behavior because this bug only impacts the pathspec->items->original field which is
> NOT used to filter paths. However, git-stash does use pathspec->items->original when
> building args used to call other git commands.
> (See add_pathspecs() usage and implementation in stash.c)
>
> It is possible that when the attr pathspec feature was first added in
> b0db704652 (pathspec: allow querying for attributes, 2017-03-13),
> "PATHSPEC_ATTR" was just unintentionally left out of a few GUARD_PATHSPEC() invocations.
>
> Later, to get a more user-friendly error message when attr was used with git-add,
> PATHSPEC_ATTR was added as a mask to git-add's invocation of parse_pathspec()
> 84d938b732 (add: do not accept pathspec magic 'attr', 2018-09-18).
> However, this user-friendly error message was never added for git-stash.
>
> Signed-off-by: Joanna Wang <jojwang@google.com>
>
> ---
>
> > At this point in the code, is it guaranteed that element[0] is ':'
> > and never a NUL? Also is it guaranteed that element has ')'
> > somewhere later if element[1] is '('?
> No sorry, we can only assume this if there was element magic detected
> by parse_element_magic(). So if element magic is not 0, we know the body
> is in the expected form (either long or short).
> I have added comments and a check for magic to guard against this.
>
> > I wonder if this existing bug caused by
> > failing to copy the value of "attr:" is triggerable from a codepath
> > that already allows PATHSPEC_ATTR magic.
> While there are other commands that go through the prefix_magic path (like `git-add -i`),
> AFAICT they are not actually impacted by the bug. The bug only impacts the value of
> pathspec->items->original which is not used during pathspec matching. But git-stash
> uses `original` to pass in as args to other commands it calls. I've included this
> detail in the description above.
>
> > but as the third parameter strbuf_add() takes is of type size_t, it would not
> > hurt to define "len" as the same type as well.
> Thanks for spotting. fixed.
>
> builtin/add.c | 7 ++-
> dir.c | 3 +-
> pathspec.c | 36 ++++++++---
> t/t6135-pathspec-with-attrs.sh | 108 +++++++++++++++++++++++++++++++--
> 4 files changed, 136 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 5126d2ede3..d46e4d10e9 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -424,7 +424,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> * Check the "pathspec '%s' did not match any files" block
> * below before enabling new magic.
> */
> - parse_pathspec(&pathspec, PATHSPEC_ATTR,
> + parse_pathspec(&pathspec, 0,
> PATHSPEC_PREFER_FULL |
> PATHSPEC_SYMLINK_LEADING_PATH,
> prefix, argv);
> @@ -433,7 +433,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> if (pathspec.nr)
> die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
>
> - parse_pathspec_file(&pathspec, PATHSPEC_ATTR,
> + parse_pathspec_file(&pathspec, 0,
> PATHSPEC_PREFER_FULL |
> PATHSPEC_SYMLINK_LEADING_PATH,
> prefix, pathspec_from_file, pathspec_file_nul);
> @@ -504,7 +504,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> PATHSPEC_LITERAL |
> PATHSPEC_GLOB |
> PATHSPEC_ICASE |
> - PATHSPEC_EXCLUDE);
> + PATHSPEC_EXCLUDE |
> + PATHSPEC_ATTR);
>
> for (i = 0; i < pathspec.nr; i++) {
> const char *path = pathspec.items[i].match;
> diff --git a/dir.c b/dir.c
> index 16fdb03f2a..4d1cd039be 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2179,7 +2179,8 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
> PATHSPEC_LITERAL |
> PATHSPEC_GLOB |
> PATHSPEC_ICASE |
> - PATHSPEC_EXCLUDE);
> + PATHSPEC_EXCLUDE |
> + PATHSPEC_ATTR);
>
> for (i = 0; i < pathspec->nr; i++) {
> const struct pathspec_item *item = &pathspec->items[i];
> diff --git a/pathspec.c b/pathspec.c
> index bb1efe1f39..2f8721cc15 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -109,16 +109,34 @@ static struct pathspec_magic {
> { PATHSPEC_ATTR, '\0', "attr" },
> };
>
> -static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
> +static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic, const char *element)
> {
> - int i;
> - strbuf_addstr(sb, ":(");
> - for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
> - if (magic & pathspec_magic[i].bit) {
> - if (sb->buf[sb->len - 1] != '(')
> - strbuf_addch(sb, ',');
> - strbuf_addstr(sb, pathspec_magic[i].name);
> + /* No magic was found in element, just add prefix magic */
> + if (magic == 0) {
> + strbuf_addf(sb, ":(prefix:%d)", prefixlen);
> + return;
> + }
> +
> + /*
> + * At this point we known that parse_element_magic() was able to extract some pathspec
> + * magic from element. So we know element is correctly formatted in either shorthand
> + * or longhand form
> + */
> + if (element[1] != '(') {
> + /* Process an element in shorthand form (e.g. ":!/<match>") */
> + strbuf_addstr(sb, ":(");
> + for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
> + if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
> + if (sb->buf[sb->len - 1] != '(')
> + strbuf_addch(sb, ',');
> + strbuf_addstr(sb, pathspec_magic[i].name);
> + }
> }
> + } else {
> + /* For the longhand form, we copy everything up to the final ')' */
> + size_t len = strchr(element, ')') - element;
> + strbuf_add(sb, element, len);
> + }
> strbuf_addf(sb, ",prefix:%d)", prefixlen);
> }
>
> @@ -493,7 +511,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
> struct strbuf sb = STRBUF_INIT;
>
> /* Preserve the actual prefix length of each pattern */
> - prefix_magic(&sb, prefixlen, element_magic);
> + prefix_magic(&sb, prefixlen, element_magic, elt);
>
> strbuf_addstr(&sb, match);
> item->original = strbuf_detach(&sb, NULL);
> diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
> index f70c395e75..e46fa176ed 100755
> --- a/t/t6135-pathspec-with-attrs.sh
> +++ b/t/t6135-pathspec-with-attrs.sh
> @@ -64,12 +64,24 @@ test_expect_success 'setup .gitattributes' '
> fileSetLabel label
> fileValue label=foo
> fileWrongLabel label☺
> + newFileA* labelA
> + newFileB* labelB
> EOF
> echo fileSetLabel label1 >sub/.gitattributes &&
> git add .gitattributes sub/.gitattributes &&
> git commit -m "add attributes"
> '
>
> +test_expect_success 'setup .gitignore' '
> + cat <<-\EOF >.gitignore &&
> + actual
> + expect
> + pathspec_file
> + EOF
> + git add .gitignore &&
> + git commit -m "add gitignore"
> +'
> +
> test_expect_success 'check specific set attr' '
> cat <<-\EOF >expect &&
> fileSetLabel
> @@ -150,6 +162,7 @@ test_expect_success 'check specific value attr (2)' '
> test_expect_success 'check unspecified attr' '
> cat <<-\EOF >expect &&
> .gitattributes
> + .gitignore
> fileA
> fileAB
> fileAC
> @@ -175,6 +188,7 @@ test_expect_success 'check unspecified attr' '
> test_expect_success 'check unspecified attr (2)' '
> cat <<-\EOF >expect &&
> HEAD:.gitattributes
> + HEAD:.gitignore
> HEAD:fileA
> HEAD:fileAB
> HEAD:fileAC
> @@ -200,6 +214,7 @@ test_expect_success 'check unspecified attr (2)' '
> test_expect_success 'check multiple unspecified attr' '
> cat <<-\EOF >expect &&
> .gitattributes
> + .gitignore
> fileC
> fileNoLabel
> fileWrongLabel
> @@ -239,16 +254,99 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
> test_i18ngrep "Only one" actual
> '
>
> -test_expect_success 'fail if attr magic is used places not implemented' '
> +test_expect_success 'fail if attr magic is used in places not implemented' '
> # The main purpose of this test is to check that we actually fail
> # when you attempt to use attr magic in commands that do not implement
> - # attr magic. This test does not advocate git-add to stay that way,
> - # though, but git-add is convenient as it has its own internal pathspec
> - # parsing.
> - test_must_fail git add ":(attr:labelB)" 2>actual &&
> + # attr magic. This test does not advocate check-ignore to stay that way.
> + # When you teach the command to grok the pathspec, you need to find
> + # another command to replace it for the test.
> + test_must_fail git check-ignore ":(attr:labelB)" 2>actual &&
> test_i18ngrep "magic not supported" actual
> '
>
> +test_expect_success 'check that attr magic works for git stash push' '
> + cat <<-\EOF >expect &&
> + A sub/newFileA-foo
> + EOF
> + >sub/newFileA-foo &&
> + >sub/newFileB-foo &&
> + git stash push --include-untracked -- ":(exclude,attr:labelB)" &&
> + git stash show --include-untracked --name-status >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check that attr magic works for git add --all' '
> + cat <<-\EOF >expect &&
> + sub/newFileA-foo
> + EOF
> + >sub/newFileA-foo &&
> + >sub/newFileB-foo &&
> + git add --all ":(exclude,attr:labelB)" &&
> + git diff --name-only --cached >actual &&
> + git restore -W -S . &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check that attr magic works for git add -u' '
> + cat <<-\EOF >expect &&
> + sub/fileA
> + EOF
> + >sub/newFileA-foo &&
> + >sub/newFileB-foo &&
> + >sub/fileA &&
> + >sub/fileB &&
> + git add -u ":(exclude,attr:labelB)" &&
> + git diff --name-only --cached >actual &&
> + git restore -S -W . && rm sub/new* &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check that attr magic works for git add <path>' '
> + cat <<-\EOF >expect &&
> + fileA
> + fileB
> + sub/fileA
> + EOF
> + >fileA &&
> + >fileB &&
> + >sub/fileA &&
> + >sub/fileB &&
> + git add ":(exclude,attr:labelB)sub/*" &&
> + git diff --name-only --cached >actual &&
> + git restore -S -W . &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check that attr magic works for git -add .' '
> + cat <<-\EOF >expect &&
> + sub/fileA
> + EOF
> + >fileA &&
> + >fileB &&
> + >sub/fileA &&
> + >sub/fileB &&
> + cd sub &&
> + git add . ":(exclude,attr:labelB)" &&
> + cd .. &&
> + git diff --name-only --cached >actual &&
> + git restore -S -W . &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'check that attr magic works for git add --pathspec-from-file' '
> + cat <<-\EOF >pathspec_file &&
> + :(exclude,attr:labelB)
> + EOF
> + cat <<-\EOF >expect &&
> + sub/newFileA-foo
> + EOF
> + >sub/newFileA-foo &&
> + >sub/newFileB-foo &&
> + git add --all --pathspec-from-file=pathspec_file &&
> + git diff --name-only --cached >actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'abort on giving invalid label on the command line' '
> test_must_fail git ls-files . ":(attr:☺)"
> '
> --
> 2.42.0.869.gea05f2083d-goog
>
--
So many immigrant groups have swept through our town
that Brooklyn, like Atlantis, reaches mythological
proportions in the mind of the world - RI Safir 1998
http://www.mrbrklyn.com
DRM is THEFT - We are the STAKEHOLDERS - RI Safir 2002
http://www.nylxs.com - Leadership Development in Free Software
http://www2.mrbrklyn.com/resources - Unpublished Archive
http://www.coinhangout.com - coins!
http://www.brooklyn-living.com
Being so tracked is for FARM ANIMALS and extermination camps,
but incompatible with living as a free human being. -RI Safir 2013
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Andy Koppe @ 2023-11-03 15:28 UTC (permalink / raw)
To: Thomas Guyot
Cc: Dragan Simic, Junio C Hamano, Jeff King, Christoph Anton Mitterer,
git
In-Reply-To: <24c46f25-6b31-437d-9f89-1e8eb74136c8@gmail.com>
Thomas Guyot wrote:
> >> I actually have one major issue with it, it's that displaying anything
> >> less than a full page will fill the screen with ~ on the bottom, just
> >> like when scrolling up on a partial page without -F.
'less' has the '-~' (or --tilde) option to suppress that.
> I mentioned it slightly above, to be clear it would mean that:
>
> 1. less starts by just writing lined down as usual, making any lines
> above scroll up and overflow into the scrollback buffer as usual
> 2. If less draws less than a page, exits as before - the effective
> result is as if pager was cat
> 3. If less reaches a full page and still has lines to write, it turns on
> -c's behavior and further updates happen from the top of the screen,
> preventing scroll up (at least on rxvt-unicode)
>
> Now, if all other terms misbehave here, that's an issue, making this
> suggestion mostly useless. And considering the number of Windows users
> we absolutely need to test Windows Terminal, and should probably test
> MacOS's term too (whatever that is).
For what it's worth, the 'mintty' terminal used by default for Git for
Windows as well as MSYS and Cygwin has another approach to the whole
problem. Its rather flippantly named 'Flip Screen' context menu
command with Alt+F12 or Ctrl+Shift+S shortcut lets users temporarily
look at the alternate screen buffer while the main screen buffer is
active, and vice versa.
If 'less' is invoked without the -X option, it will switch to the
alternate screen, where mousewheel scrolling works by sending cursor
up/down keycodes. While in 'less', you can temporarily flip to the
main screen to look up something in the shell session there or copy
something for searching in 'less'. While looking at the main screen,
the mousewheel will scroll the scrollback buffer. Keyboard input
that's sent to 'less' will flip back to the alternate screen.
Quitting 'less' switches back to the main screen, so the 'less' output
disappears and you're back in the shell session with the command that
invoked 'less' as the last thing shown. But again, the 'Flip Screen'
command or shortcuts can be used to temporarily look at or copy from
the alternate screen, which will contain the last page displayed by
'less'. (The alternate screen does not have a scrollback buffer.)
The 'Flip Screen' feature of course also works with other
alternate-screen applications, for example editors.
Apparently the Mac terminal has such a feature as well:
https://support.apple.com/en-ie/guide/terminal/trmld1f46097/mac
(Full disclosure: I originally made mintty, from PuTTY.)
Kind regards,
Andy
^ permalink raw reply
* RE: [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: rsbecker @ 2023-11-03 15:01 UTC (permalink / raw)
To: git
In-Reply-To: <007101da0e65$13e3c170$3bab4450$@nexbridge.com>
On Friday, November 3, 2023 10:50 AM, I wrote:
>In RC0, the following tests are failing (with verbose). They look like the
same root
>cause. Unpack("Q>".... What version does git now require for perl?
>I have v5.30.3 available, nothing more recent.
>
>expecting success of 4216.141 'Bloom reader notices too-small data chunk':
>check_corrupt_graph BDAT clear 00000000 && echo "warning: ignoring
too-small
>changed-path chunk" \
>"(4 < 12) in commit-graph file" >expect.err && test_cmp expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 141 - Bloom reader notices too-small data chunk #
#check_corrupt_graph
>BDAT clear 00000000 && #echo "warning: ignoring too-small changed-path
chunk"
>\
>#"(4 < 12) in commit-graph file" >expect.err && #test_cmp expect.err err #
>
>expecting success of 4216.142 'Bloom reader notices out-of-bounds filter
>offsets':
>check_corrupt_graph BIDX 12 FFFFFFFF &&
># use grep to avoid depending on exact chunk size grep "warning: ignoring
out-of-
>range offset (4294967295) for changed-path filter at pos 3 of
>.git/objects/info/commit-graph" err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 142 - Bloom reader notices out-of-bounds filter offsets #
>#check_corrupt_graph BIDX 12 FFFFFFFF && ## use grep to avoid depending on
>exact chunk size #grep "warning: ignoring out-of-range offset (4294967295)
for
>changed-path filter at pos 3 of .git/objects/info/commit-graph" err #
>
>expecting success of 4216.143 'Bloom reader notices too-small index chunk':
># replace the index with a single entry, making most # lookups
out-of-bounds
>check_corrupt_graph BIDX clear 00000000 && echo "warning: commit-graph
>changed-path index chunk" \ "is too small" >expect.err && test_cmp
expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 143 - Bloom reader notices too-small index chunk # ## replace the
index with
>a single entry, making most ## lookups out-of-bounds #check_corrupt_graph
BIDX
>clear 00000000 && #echo "warning: commit-graph changed-path index chunk" \
>#"is too small" >expect.err && #test_cmp expect.err err #
>
>expecting success of 4216.144 'Bloom reader notices out-of-order index
>offsets':
># we do not know any real offsets, but we can pick # something plausible;
we
>should not get to the point of # actually reading from the bogus offsets
anyway.
>corrupt_graph BIDX 4 0000000c00000005 && echo "warning: ignoring decreasing
>changed-path index offsets" \
>"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>>expect.err &&
>git -c core.commitGraph=false log -- A/B/file2 >expect.out && git -c
>core.commitGraph=true log -- A/B/file2 >out 2>err && test_cmp expect.out
out &&
>test_cmp expect.err err
>
>Invalid type 'Q' in unpack at
>/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chun
k
>-file.pl line 31.
>not ok 144 - Bloom reader notices out-of-order index offsets # ## we do not
know
>any real offsets, but we can pick ## something plausible; we should not get
to the
>point of ## actually reading from the bogus offsets anyway.
>#corrupt_graph BIDX 4 0000000c00000005 && #echo "warning: ignoring
>decreasing changed-path index offsets" \
>#"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>>expect.err &&
>#git -c core.commitGraph=false log -- A/B/file2 >expect.out && #git -c
>core.commitGraph=true log -- A/B/file2 >out 2>err && #test_cmp expect.out
out
>&& #test_cmp expect.err err #
>
This same problem also happens in t5318, t5319, t5324
--Randall
^ permalink raw reply
* Re: mintty bug in current git bash (Git-2.42.0.2-64-bit.exe)
From: Andy Koppe @ 2023-11-03 14:58 UTC (permalink / raw)
To: Rudisill, Steven L. (Booz Allen Hamilton); +Cc: git@vger.kernel.org
In-Reply-To: <SJ0PR09MB99693718DDA17BC15532A3099CA5A@SJ0PR09MB9969.namprd09.prod.outlook.com>
On Fri, 3 Nov 2023 at 14:11, Rudisill, Steven L. (Booz Allen Hamilton) wrote:
> Mintty has a bug - when double-clicking on text, the preceding '~' or '.' is not selected.
> https://github.com/mintty/mintty/issues/1237
>
> This really slows down interacting in git bash because this is a very frequent console interaction:
> Double-clicking to select a path (which copies it to the paste buffer since the text is highlighted), then easily paste to manipulate text.
> The pasted text is omitting the preceding '~' or '.' character.
>
> Please release a new git bash version that uses the updated mintty with the fix for this bug. The issue link above indicates that this is a bug and has been fixed.
This is fixed in mintty 3.6.5, which is part of the Git for Windows
2.43.0 release candidate announced earlier today:
https://lore.kernel.org/git/20231103085526.3655-1-johannes.schindelin@gmx.de
Regards,
Andy
^ permalink raw reply
* [BUG] Git 2.43.0-rc0 - t4216 unpack(Q) invalid type
From: rsbecker @ 2023-11-03 14:50 UTC (permalink / raw)
To: git
In RC0, the following tests are failing (with verbose). They look like the
same root cause. Unpack("Q>".... What version does git now require for perl?
I have v5.30.3 available, nothing more recent.
expecting success of 4216.141 'Bloom reader notices too-small data chunk':
check_corrupt_graph BDAT clear 00000000 &&
echo "warning: ignoring too-small changed-path chunk" \
"(4 < 12) in commit-graph file" >expect.err &&
test_cmp expect.err err
Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 141 - Bloom reader notices too-small data chunk
#
#check_corrupt_graph BDAT clear 00000000 &&
#echo "warning: ignoring too-small changed-path chunk" \
#"(4 < 12) in commit-graph file" >expect.err &&
#test_cmp expect.err err
#
expecting success of 4216.142 'Bloom reader notices out-of-bounds filter
offsets':
check_corrupt_graph BIDX 12 FFFFFFFF &&
# use grep to avoid depending on exact chunk size
grep "warning: ignoring out-of-range offset (4294967295) for changed-path
filter at pos 3 of .git/objects/info/commit-graph" err
Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 142 - Bloom reader notices out-of-bounds filter offsets
#
#check_corrupt_graph BIDX 12 FFFFFFFF &&
## use grep to avoid depending on exact chunk size
#grep "warning: ignoring out-of-range offset (4294967295) for changed-path
filter at pos 3 of .git/objects/info/commit-graph" err
#
expecting success of 4216.143 'Bloom reader notices too-small index chunk':
# replace the index with a single entry, making most
# lookups out-of-bounds
check_corrupt_graph BIDX clear 00000000 &&
echo "warning: commit-graph changed-path index chunk" \
"is too small" >expect.err &&
test_cmp expect.err err
Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 143 - Bloom reader notices too-small index chunk
#
## replace the index with a single entry, making most
## lookups out-of-bounds
#check_corrupt_graph BIDX clear 00000000 &&
#echo "warning: commit-graph changed-path index chunk" \
#"is too small" >expect.err &&
#test_cmp expect.err err
#
expecting success of 4216.144 'Bloom reader notices out-of-order index
offsets':
# we do not know any real offsets, but we can pick
# something plausible; we should not get to the point of
# actually reading from the bogus offsets anyway.
corrupt_graph BIDX 4 0000000c00000005 &&
echo "warning: ignoring decreasing changed-path index offsets" \
"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>expect.err &&
git -c core.commitGraph=false log -- A/B/file2 >expect.out &&
git -c core.commitGraph=true log -- A/B/file2 >out 2>err &&
test_cmp expect.out out &&
test_cmp expect.err err
Invalid type 'Q' in unpack at
/home/jenkinsbuild/.jenkins/workspace/Git_Pipeline/t/lib-chunk/corrupt-chunk
-file.pl line 31.
not ok 144 - Bloom reader notices out-of-order index offsets
#
## we do not know any real offsets, but we can pick
## something plausible; we should not get to the point of
## actually reading from the bogus offsets anyway.
#corrupt_graph BIDX 4 0000000c00000005 &&
#echo "warning: ignoring decreasing changed-path index offsets" \
#"(12 > 5) for positions 1 and 2 of .git/objects/info/commit-graph"
>expect.err &&
#git -c core.commitGraph=false log -- A/B/file2 >expect.out &&
#git -c core.commitGraph=true log -- A/B/file2 >out 2>err &&
#test_cmp expect.out out &&
#test_cmp expect.err err
#
--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.
^ permalink raw reply
* [PATCH 1/1] attr: enable attr pathspec magic for git-add and git-stash
From: Joanna Wang @ 2023-11-03 14:35 UTC (permalink / raw)
To: gitster; +Cc: git, jojwang
In-Reply-To: <xmqq34xnlny7.fsf@gitster.g>
This lets users limit files or exclude files based on file
attributes during git-add and git-stash.
For example, the chromium project would like to use this like
"git add --all ':(exclude,attr:submodule)'", as submodules are managed in a
unique way and often results in submodule changes that users do not want in
their commits.
This does not change any attr magic implementation. It is only adding
attr as an allowed pathspec in git-add and git-stash, which was previously
blocked by GUARD_PATHSPEC and a pathspec mask in parse_pathspec()).
However, this does fix a bug in prefix_magic() where attr values were unintentionally removed.
This was hit whenever parse_pathspec() is called with PATHSPEC_PREFIX_ORIGIN as a flag,
which was the case for git-stash.
(But originally filed here:
https://lore.kernel.org/git/CAMmZTi-0QKtj7Q=sbC5qhipGsQxJFOY-Qkk1jfkRYwfF5FcUVg@mail.gmail.com/)
Furthermore, while other commands hit this code path it did not result in unexpected
behavior because this bug only impacts the pathspec->items->original field which is
NOT used to filter paths. However, git-stash does use pathspec->items->original when
building args used to call other git commands.
(See add_pathspecs() usage and implementation in stash.c)
It is possible that when the attr pathspec feature was first added in
b0db704652 (pathspec: allow querying for attributes, 2017-03-13),
"PATHSPEC_ATTR" was just unintentionally left out of a few GUARD_PATHSPEC() invocations.
Later, to get a more user-friendly error message when attr was used with git-add,
PATHSPEC_ATTR was added as a mask to git-add's invocation of parse_pathspec()
84d938b732 (add: do not accept pathspec magic 'attr', 2018-09-18).
However, this user-friendly error message was never added for git-stash.
Signed-off-by: Joanna Wang <jojwang@google.com>
---
> At this point in the code, is it guaranteed that element[0] is ':'
> and never a NUL? Also is it guaranteed that element has ')'
> somewhere later if element[1] is '('?
No sorry, we can only assume this if there was element magic detected
by parse_element_magic(). So if element magic is not 0, we know the body
is in the expected form (either long or short).
I have added comments and a check for magic to guard against this.
> I wonder if this existing bug caused by
> failing to copy the value of "attr:" is triggerable from a codepath
> that already allows PATHSPEC_ATTR magic.
While there are other commands that go through the prefix_magic path (like `git-add -i`),
AFAICT they are not actually impacted by the bug. The bug only impacts the value of
pathspec->items->original which is not used during pathspec matching. But git-stash
uses `original` to pass in as args to other commands it calls. I've included this
detail in the description above.
> but as the third parameter strbuf_add() takes is of type size_t, it would not
> hurt to define "len" as the same type as well.
Thanks for spotting. fixed.
builtin/add.c | 7 ++-
dir.c | 3 +-
pathspec.c | 36 ++++++++---
t/t6135-pathspec-with-attrs.sh | 108 +++++++++++++++++++++++++++++++--
4 files changed, 136 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 5126d2ede3..d46e4d10e9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -424,7 +424,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
* Check the "pathspec '%s' did not match any files" block
* below before enabling new magic.
*/
- parse_pathspec(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, argv);
@@ -433,7 +433,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (pathspec.nr)
die(_("'%s' and pathspec arguments cannot be used together"), "--pathspec-from-file");
- parse_pathspec_file(&pathspec, PATHSPEC_ATTR,
+ parse_pathspec_file(&pathspec, 0,
PATHSPEC_PREFER_FULL |
PATHSPEC_SYMLINK_LEADING_PATH,
prefix, pathspec_from_file, pathspec_file_nul);
@@ -504,7 +504,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec.nr; i++) {
const char *path = pathspec.items[i].match;
diff --git a/dir.c b/dir.c
index 16fdb03f2a..4d1cd039be 100644
--- a/dir.c
+++ b/dir.c
@@ -2179,7 +2179,8 @@ static int exclude_matches_pathspec(const char *path, int pathlen,
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE |
- PATHSPEC_EXCLUDE);
+ PATHSPEC_EXCLUDE |
+ PATHSPEC_ATTR);
for (i = 0; i < pathspec->nr; i++) {
const struct pathspec_item *item = &pathspec->items[i];
diff --git a/pathspec.c b/pathspec.c
index bb1efe1f39..2f8721cc15 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -109,16 +109,34 @@ static struct pathspec_magic {
{ PATHSPEC_ATTR, '\0', "attr" },
};
-static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic)
+static void prefix_magic(struct strbuf *sb, int prefixlen, unsigned magic, const char *element)
{
- int i;
- strbuf_addstr(sb, ":(");
- for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
- if (magic & pathspec_magic[i].bit) {
- if (sb->buf[sb->len - 1] != '(')
- strbuf_addch(sb, ',');
- strbuf_addstr(sb, pathspec_magic[i].name);
+ /* No magic was found in element, just add prefix magic */
+ if (magic == 0) {
+ strbuf_addf(sb, ":(prefix:%d)", prefixlen);
+ return;
+ }
+
+ /*
+ * At this point we known that parse_element_magic() was able to extract some pathspec
+ * magic from element. So we know element is correctly formatted in either shorthand
+ * or longhand form
+ */
+ if (element[1] != '(') {
+ /* Process an element in shorthand form (e.g. ":!/<match>") */
+ strbuf_addstr(sb, ":(");
+ for (int i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+ if ((magic & pathspec_magic[i].bit) && (pathspec_magic[i].mnemonic != '\0')) {
+ if (sb->buf[sb->len - 1] != '(')
+ strbuf_addch(sb, ',');
+ strbuf_addstr(sb, pathspec_magic[i].name);
+ }
}
+ } else {
+ /* For the longhand form, we copy everything up to the final ')' */
+ size_t len = strchr(element, ')') - element;
+ strbuf_add(sb, element, len);
+ }
strbuf_addf(sb, ",prefix:%d)", prefixlen);
}
@@ -493,7 +511,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,
struct strbuf sb = STRBUF_INIT;
/* Preserve the actual prefix length of each pattern */
- prefix_magic(&sb, prefixlen, element_magic);
+ prefix_magic(&sb, prefixlen, element_magic, elt);
strbuf_addstr(&sb, match);
item->original = strbuf_detach(&sb, NULL);
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index f70c395e75..e46fa176ed 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -64,12 +64,24 @@ test_expect_success 'setup .gitattributes' '
fileSetLabel label
fileValue label=foo
fileWrongLabel label☺
+ newFileA* labelA
+ newFileB* labelB
EOF
echo fileSetLabel label1 >sub/.gitattributes &&
git add .gitattributes sub/.gitattributes &&
git commit -m "add attributes"
'
+test_expect_success 'setup .gitignore' '
+ cat <<-\EOF >.gitignore &&
+ actual
+ expect
+ pathspec_file
+ EOF
+ git add .gitignore &&
+ git commit -m "add gitignore"
+'
+
test_expect_success 'check specific set attr' '
cat <<-\EOF >expect &&
fileSetLabel
@@ -150,6 +162,7 @@ test_expect_success 'check specific value attr (2)' '
test_expect_success 'check unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileA
fileAB
fileAC
@@ -175,6 +188,7 @@ test_expect_success 'check unspecified attr' '
test_expect_success 'check unspecified attr (2)' '
cat <<-\EOF >expect &&
HEAD:.gitattributes
+ HEAD:.gitignore
HEAD:fileA
HEAD:fileAB
HEAD:fileAC
@@ -200,6 +214,7 @@ test_expect_success 'check unspecified attr (2)' '
test_expect_success 'check multiple unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
+ .gitignore
fileC
fileNoLabel
fileWrongLabel
@@ -239,16 +254,99 @@ test_expect_success 'fail on multiple attr specifiers in one pathspec item' '
test_i18ngrep "Only one" actual
'
-test_expect_success 'fail if attr magic is used places not implemented' '
+test_expect_success 'fail if attr magic is used in places not implemented' '
# The main purpose of this test is to check that we actually fail
# when you attempt to use attr magic in commands that do not implement
- # attr magic. This test does not advocate git-add to stay that way,
- # though, but git-add is convenient as it has its own internal pathspec
- # parsing.
- test_must_fail git add ":(attr:labelB)" 2>actual &&
+ # attr magic. This test does not advocate check-ignore to stay that way.
+ # When you teach the command to grok the pathspec, you need to find
+ # another command to replace it for the test.
+ test_must_fail git check-ignore ":(attr:labelB)" 2>actual &&
test_i18ngrep "magic not supported" actual
'
+test_expect_success 'check that attr magic works for git stash push' '
+ cat <<-\EOF >expect &&
+ A sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git stash push --include-untracked -- ":(exclude,attr:labelB)" &&
+ git stash show --include-untracked --name-status >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --all' '
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -W -S . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add -u' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add -u ":(exclude,attr:labelB)" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . && rm sub/new* &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add <path>' '
+ cat <<-\EOF >expect &&
+ fileA
+ fileB
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ git add ":(exclude,attr:labelB)sub/*" &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git -add .' '
+ cat <<-\EOF >expect &&
+ sub/fileA
+ EOF
+ >fileA &&
+ >fileB &&
+ >sub/fileA &&
+ >sub/fileB &&
+ cd sub &&
+ git add . ":(exclude,attr:labelB)" &&
+ cd .. &&
+ git diff --name-only --cached >actual &&
+ git restore -S -W . &&
+ test_cmp expect actual
+'
+
+test_expect_success 'check that attr magic works for git add --pathspec-from-file' '
+ cat <<-\EOF >pathspec_file &&
+ :(exclude,attr:labelB)
+ EOF
+ cat <<-\EOF >expect &&
+ sub/newFileA-foo
+ EOF
+ >sub/newFileA-foo &&
+ >sub/newFileB-foo &&
+ git add --all --pathspec-from-file=pathspec_file &&
+ git diff --name-only --cached >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'abort on giving invalid label on the command line' '
test_must_fail git ls-files . ":(attr:☺)"
'
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related
* [PATCH 2/2] RelNotes: improve wording of credential helper notes
From: Todd Zullinger @ 2023-11-03 14:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20231103141759.864875-1-tmz@pobox.com>
Offer a slightly more verbose description of the issue fixed by
7144dee3ec (credential/libsecret: erase matching creds only, 2023-07-26)
and cb626f8e5c (credential/wincred: erase matching creds only,
2023-07-26).
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
I had trouble parsing the original. Hopefully this is an improvement. :)
Documentation/RelNotes/2.43.0.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/RelNotes/2.43.0.txt b/Documentation/RelNotes/2.43.0.txt
index e66d691f4e..6db6865861 100644
--- a/Documentation/RelNotes/2.43.0.txt
+++ b/Documentation/RelNotes/2.43.0.txt
@@ -37,8 +37,9 @@ UI, Workflows & Features
oath token and password expiration data) are stored in libsecret
keyrings has been rethought.
- * Update two credential helpers to correctly match which credential
- to erase; they dropped not the ones with stale password.
+ * Update the libsecret and wincred credential helpers to correctly
+ match which credential to erase; they erased the wrong entry in
+ some cases.
* Git GUI updates.
--
2.43.0.rc0
^ permalink raw reply related
* [PATCH 1/2] RelNotes: minor typo fixes in 2.43.0 draft
From: Todd Zullinger @ 2023-11-03 14:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20231103141759.864875-1-tmz@pobox.com>
Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
Documentation/RelNotes/2.43.0.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/RelNotes/2.43.0.txt b/Documentation/RelNotes/2.43.0.txt
index ad3b82fe0a..e66d691f4e 100644
--- a/Documentation/RelNotes/2.43.0.txt
+++ b/Documentation/RelNotes/2.43.0.txt
@@ -16,7 +16,7 @@ Backward Compatibility Notes
rule is that "--not" given from the command line only affects revs
given from the command line that comes but not revs read from the
standard input, and "--not" read from the standard input affects
- revs given from the stanrdard input and not revs given from the
+ revs given from the standard input and not revs given from the
command line.
UI, Workflows & Features
@@ -62,7 +62,7 @@ UI, Workflows & Features
describe complex "revert of revert of revert" situation better in
their own words.
- * The command-line complation support (in contrib/) learned to
+ * The command-line completion support (in contrib/) learned to
complete "git commit --trailer=" for possible trailer keys.
* "git update-index" learns "--show-index-version" to inspect
--
2.43.0.rc0
^ permalink raw reply related
* [PATCH 0/2] RelNotes: minor changes in 2.43.0 draft
From: Todd Zullinger @ 2023-11-03 14:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
These are minor changes from a quick reading of the 2.43.0
release notes draft.
Todd Zullinger (2):
RelNotes: minor typo fixes in 2.43.0 draft
RelNotes: improve wording of credential helper notes
Documentation/RelNotes/2.43.0.txt | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--
2.43.0.rc0
^ permalink raw reply
* RE: mintty bug in current git bash (Git-2.42.0.2-64-bit.exe)
From: Rudisill, Steven L. (Booz Allen Hamilton) @ 2023-11-03 14:10 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <SJ0PR09MB99699E1D1F577734F535CB1D9CA6A@SJ0PR09MB9969.namprd09.prod.outlook.com>
Hi git,
Mintty has a bug - when double-clicking on text, the preceding '~' or '.' is not selected.
https://github.com/mintty/mintty/issues/1237
This really slows down interacting in git bash because this is a very frequent console interaction:
Double-clicking to select a path (which copies it to the paste buffer since the text is highlighted), then easily paste to manipulate text.
The pasted text is omitting the preceding '~' or '.' character.
Please release a new git bash version that uses the updated mintty with the fix for this bug. The issue link above indicates that this is a bug and has been fixed.
Further, it might be nice if git bash had an update feature?
Regards,
Steven
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Thomas Guyot @ 2023-11-03 11:47 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <79cf1bf35ba6c9348735685b01e0f2f9@manjaro.org>
On 2023-11-02 10:19, Dragan Simic wrote:
> On 2023-11-02 14:19, Thomas Guyot wrote:
>> That's correct, you need both and also -y0
> Hmm, I tried the following:
>
> GIT_PAGER='less -R -F -X -c -y0'
>
> In my environment (Xfce), the result after scrolling the output of "git
> log -p" up and down a bit was about 20 copies of the same screen "page"
> in the scrollback, plus a couple of blank "pages". Not good,
> unfortunately, and actually much worse than having just "-R -F -X".
Indeed, I did notice it with xfce4-term - I suspect different controls
are used to clear the screen, with rxvt-unicode the initial one scrolls
anything in the current display to the scrollback (which is important to
avoid clearing the last commands/output form the scrollback), but
afterward screen updates do not update the scrollback, it remains within
the screen.
These options may actually affect the behavior (I have put my current
values there)
secondaryScreen: True
Turn on/off secondary screen (default enabled).
secondaryScroll: True
Turn on/off secondary screen scroll (default enabled). If this
option is enabled, scrolls on the secondary screen will change the
scrollback buffer and, when secondaryScreen is off, switching
to/from the secondary screen will instead scroll the screen up.
So it appears I could disable secondaryScroll to avoid getting scrolled
lines into the scrollback buffer. It's also noteworthy that with
secondaryScreen disabled, rxvt-unicode instead scroll things up to avoid
the current screen form being wiped out.
>> Indeed, but when less update from the bottom, it can add new lines and
>> let the overflow lines scroll up into the scrollback buffer.
>>
>> Then updating it from the top, it draws the whole page, top to bottom.
>> That's fine for a full page but not desired for a partial one. Also
>> note that on my terminal (rxvt-unicode) when less clears the screen to
>> draw the first page the current screen is rolled up into scrollback -
>> iirc that's a configurable option, it would be worth testing other
>> terminal's behavior on that. IIRC it may also erase it when using the
>> wrong termcap file.
>>
>> I haven't looked at the code, but I think it could be possibly to
>> start the -c behavior only after a full page is drawn, after exiting
>> on partial pages, which would give us the best of both worlds.
> Does the GIT_PAGER setup, as I described it above, work for you without
> the described artifacts, in any of the environments you have access to?
Yes, rxvt-unicode, with the settings mentioned above.
>>> Just to clarify, it's the "-X" option that creates all the issues, and
>>> the "--redraw-on-quit" option is already there to replace it with no
>>> associated issues, but the trouble is that only newer versions of
>>> less(1) support the "--redraw-on-quit" option. IOW, it's all about
>>> improving less(1) to avoid complex workarounds required to handle
>>> different versions, such as the workarounds used in bat(1).
>> TBH I haven't tested --redraw-on-quit, even on Debian Bookworm which
>> was just released a couple months ago this option isn't available. I
>> suspect that the issue isn't -X, but the scrolling behavior controlled
>> by -y and the full redraw controlled by -c.
> When you get into the terminfo entry definitions, the root cause is that
> the terminal initialization sequences contain switching to alternate
> screen, which causes screen contents to be lost when less(1) exits.
> Thus, "-X" has been actually abused in the pager setups to skip the
> terminal initialization sequences, which may also result in other
> issues.
Right - this is something that rxvt-unicode addresses with the correct
settings.
IIRC I disabled the secondasyScreen to be able to use the scrollback
buffer (unavailable otherwise), which probably also means that with it
enabled I would also have no issue without -X, but that would need
testing. Also note that rxvt-unicode has its own terminfo file
(xfce4-term uses xterm), and in some places I'm forced to use xterm as a
fallback I get issues like current screen being wiped when switching
to/from secondary screen.
> One of the solutions is to edit the terminfo entry manually and remove
> the escape codes that cause the switching to and from alternate screen,
> which I tested, but that also introduced another issue -- the screen
> contents was always present after less(1) exited, which isn't always the
> desired behavior.
But when less scrolls down (line by line, not page by page), it always
append lines and let them scroll up. Won't you see these? (page-by-page
otoh will redraw the full screen without scrolling).
Also won't it wipe the *current* screen so that you won't see the
commands/output you had *before* running less?
This is why rxvt-unicode has an option to disable secondary screen, and
scroll contents all the way to the buffer on switch to/from secondary
screen.
>> Actually I just tested my
>> solution on xfce4-terminal and it doesn't work, the terminal still
>> push up stuff above on redraw (noteworthy is with rxvt-unicode the
>> first draw pushes the current screen contents up but no other redraw
>> does, which is what makes it work so well - I haven't tried to find
>> out what is being done exactly... OTOH the redraw on scroll down is
>> slightly noticeable there, while impossible to see on xfce4-terminal.
>> I'll install the latest less and see what happens with --redraw on
> Please test the "--redraw-on-quit" option, so far it's the best
> available solution, IMHO.
I will, not now though - need it compile less form source I guess...
>>>> If less could only enable this behavior after the first full page
>>>> draw, that would be perfect!
>>> Could you, please, elaborate a bit on that?
>> I mentioned it slightly above, to be clear it would mean that:
>>
>> 1. less starts by just writing lined down as usual, making any lines
>> above scroll up and overflow into the scrollback buffer as usual
>> 2. If less draws less than a page, exits as before - the effective
>> result is as if pager was cat
>> 3. If less reaches a full page and still has lines to write, it turns
>> on -c's behavior and further updates happen from the top of the
>> screen, preventing scroll up (at least on rxvt-unicode)
>>
>> Now, if all other terms misbehave here, that's an issue, making this
>> suggestion mostly useless. And considering the number of Windows users
>> we absolutely need to test Windows Terminal, and should probably test
>> MacOS's term too (whatever that is).
> Quite frankly, I think that such a solution would be like "fixing the
> fix, which is actually an abuse", as I described it above, eventually
> introducing even more issues, instead of solving the original issue.
I'll let you know my findings... I'm not convinced --redraw-on-quit is
actually going to fix it for all unless this does a lot more than the
option name implies (but quite happy if it does).
Regards.
--
Thomas
^ permalink raw reply
* [L10N] Kickoff for Git 2.43.0 round #1
From: Jiang Xin @ 2023-11-03 11:41 UTC (permalink / raw)
To: Git List, Git l10n discussion group, Alexander Shopov, Jordi Mas,
Ralf Thielow, Jimmy Angelakos, Christopher Díaz,
Jean-Noël Avila, Bagas Sanjaya, Alessandro Menti,
Gwan-gyeong Mun, Arusekk, Daniel Santos, Dimitriy Ryazantcev,
Peter Krefting, Emir SARI, Arkadii Yakovets,
Trần Ngọc Quân, Teng Long, Yi-Jyun Pan
Cc: Jiang Xin
Hi,
Git v2.43.0-rc0 has been released, and it's time to start new round of
git l10n. This time there are 84 updated messages need to be translated
since last release. Please send your pull request to the l10n coordinator's
repository below before this update window closes on Sun, 19 Nov 2023.
https://github.com/git-l10n/git-po/
As of git 2.37, we (git l10n contributors) have a new l10n workflow. The
following description of the new l10n workflow is from the "po/README.md"
file.
## The "po/git.pot" file is a generated file, no longer in the repository
The l10n coordinator does not need to generate the "po/git.pot" file every
time to start a new l10n workflow, and there is no "po/git.pot" file at all.
Everyone can generate the "po/git.pot" file with the command below:
make po/git.pot
But we can also forget about it. By updating our corresponding "po/XX.po"
file, the "po/git.pot" file is automatically generated.
## Update the "po/XX.po" file, and start to translate
Before updating the "po/XX.po" file, l10n contributors should pull the latest
commits from the master branch of "git.git". E.g.:
git pull --rebase git@github.com:git/git.git master
Then update the cooresponding "po/XX.po" file using the following command:
make po-update PO_FILE=po/XX.po
Translate the uptodate "po/XX.po" file, and create a new commit.
## Refine your commits, send pull requests
In the "po/XX.po" file, there are location lines in comments like below:
#: add-interactive.c:535 add-interactive.c:836 reset.c:136 sequencer.c:3505
#: sequencer.c:3970 sequencer.c:4127 builtin/rebase.c:1261
#: builtin/rebase.c:1671
These comments with file locations are useful for l10n contributors to locate
the context easily during translation. But these file locations introduce a
lot of noise and will consume a lot of repository storage. Therefore, we
should remove these file locations from the "po/XX.po" file.
To remove file locations in the "po/XX.po" file, you can use one of the
following two ways, but don't switch back and forth.
* Keep the filenames, only remove locations (need gettext 0.19 and above):
msgcat --add-location=file po/XX.po >po/XX.po.new
mv po/XX.po.new po/XX.po
* Remove both filenames and locations:
msgcat --no-location po/XX.po >po/XX.po.new
mv po/XX.po.new po/XX.po
After squashing trivial commits and removing file locations in the "po/XX.po"
file, send pull request to the l10n coordinator's repository below:
https://github.com/git-l10n/git-po/
## Resolve errors found by the l10n CI pipeline for the pull request
A helper program hosted on "https://github.com/git-l10n/git-po-helper" can
help git l10n coordinator and git l10n contributors to check the conventions
of git l10n contributions, and it is also used in GitHub actions as l10n CI
pipeline to validate each pull request in the "git-l10n/git-po" repository.
Please fix the issues found by the helper program.
** Please note: The update window will close on Sun, 19 Nov 2023. **
--
Jiang Xin
^ permalink raw reply
* [PATCH] diff: implement config.diff.renames=copies-harder
From: Sam James via GitGitGadget @ 2023-11-03 11:25 UTC (permalink / raw)
To: git; +Cc: Sam James, Sam James
From: Sam James <sam@gentoo.org>
This patch adds a config value for 'diff.renames' called 'copies-harder'
which make it so '-C -C' is in effect always passed for 'git log -p',
'git diff', etc.
This allows specifying that 'git log -p', 'git diff', etc should always act
as if '-C --find-copies-harder' was passed.
I've found this especially useful for certain types of repository (like
Gentoo's ebuild repositories) because files are often copies of a previous
version.
Signed-off-by: Sam James <sam@gentoo.org>
---
diff: implement config.diff.renames=copies-harder
This patch adds a config value for 'diff.renames' called 'copies-harder'
which make it so '-C -C' is in effect always passed for 'git log -p',
'git diff', etc.
This allows specifying that 'git log -p', 'git diff', etc should always
act as if '-C --find-copies-harder' was passed.
I've found this especially useful for certain types of repository (like
Gentoo's ebuild repositories) because files are often copies of a
previous version.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1606%2Fthesamesam%2Fconfig-copies-harder-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1606/thesamesam/config-copies-harder-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1606
Documentation/config/diff.txt | 3 ++-
Documentation/config/status.txt | 3 ++-
diff.c | 12 +++++++++---
diff.h | 1 +
diffcore-rename.c | 4 ++--
merge-ort.c | 2 +-
merge-recursive.c | 2 +-
7 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index bd5ae0c3378..d2ff3c62d41 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -131,7 +131,8 @@ diff.renames::
Whether and how Git detects renames. If set to "false",
rename detection is disabled. If set to "true", basic rename
detection is enabled. If set to "copies" or "copy", Git will
- detect copies, as well. Defaults to true. Note that this
+ detect copies, as well. If set to "copies-harder", Git will try harder
+ to detect copies. Defaults to true. Note that this
affects only 'git diff' Porcelain like linkgit:git-diff[1] and
linkgit:git-log[1], and not lower level commands such as
linkgit:git-diff-files[1].
diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
index 2ff8237f8fc..7ca7a4becd7 100644
--- a/Documentation/config/status.txt
+++ b/Documentation/config/status.txt
@@ -33,7 +33,8 @@ status.renames::
Whether and how Git detects renames in linkgit:git-status[1] and
linkgit:git-commit[1] . If set to "false", rename detection is
disabled. If set to "true", basic rename detection is enabled.
- If set to "copies" or "copy", Git will detect copies, as well.
+ If set to "copies" or "copy", Git will detect copies, as well. If
+ set to "copies-harder", Git will try harder to detect copies.
Defaults to the value of diff.renames.
status.showStash::
diff --git a/diff.c b/diff.c
index 2c602df10a3..0ca906611f5 100644
--- a/diff.c
+++ b/diff.c
@@ -206,8 +206,11 @@ int git_config_rename(const char *var, const char *value)
{
if (!value)
return DIFF_DETECT_RENAME;
+ if (!strcasecmp(value, "copies-harder"))
+ return DIFF_DETECT_COPY_HARDER;
if (!strcasecmp(value, "copies") || !strcasecmp(value, "copy"))
- return DIFF_DETECT_COPY;
+ return DIFF_DETECT_COPY;
+
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
@@ -4832,8 +4835,11 @@ void diff_setup_done(struct diff_options *options)
else
options->flags.diff_from_contents = 0;
- if (options->flags.find_copies_harder)
+ /* Just fold this in as it makes the patch-to-git smaller */
+ if (options->flags.find_copies_harder || options->detect_rename == DIFF_DETECT_COPY_HARDER) {
+ options->flags.find_copies_harder = 1;
options->detect_rename = DIFF_DETECT_COPY;
+ }
if (!options->flags.relative_name)
options->prefix = NULL;
@@ -5264,7 +5270,7 @@ static int diff_opt_find_copies(const struct option *opt,
if (*arg != 0)
return error(_("invalid argument to %s"), opt->long_name);
- if (options->detect_rename == DIFF_DETECT_COPY)
+ if (options->detect_rename == DIFF_DETECT_COPY || options->detect_rename == DIFF_DETECT_COPY_HARDER)
options->flags.find_copies_harder = 1;
else
options->detect_rename = DIFF_DETECT_COPY;
diff --git a/diff.h b/diff.h
index 66bd8aeb293..b29e5b777f8 100644
--- a/diff.h
+++ b/diff.h
@@ -555,6 +555,7 @@ int git_config_rename(const char *var, const char *value);
#define DIFF_DETECT_RENAME 1
#define DIFF_DETECT_COPY 2
+#define DIFF_DETECT_COPY_HARDER 3
#define DIFF_PICKAXE_ALL 1
#define DIFF_PICKAXE_REGEX 2
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 5a6e2bcac71..856291d66f2 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -299,7 +299,7 @@ static int find_identical_files(struct hashmap *srcs,
}
/* Give higher scores to sources that haven't been used already */
score = !source->rename_used;
- if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY)
+ if (source->rename_used && options->detect_rename != DIFF_DETECT_COPY && options->detect_rename != DIFF_DETECT_COPY_HARDER)
continue;
score += basename_same(source, target);
if (score > best_score) {
@@ -1405,7 +1405,7 @@ void diffcore_rename_extended(struct diff_options *options,
trace2_region_enter("diff", "setup", options->repo);
info.setup = 0;
assert(!dir_rename_count || strmap_empty(dir_rename_count));
- want_copies = (detect_rename == DIFF_DETECT_COPY);
+ want_copies = (detect_rename == DIFF_DETECT_COPY || detect_rename == DIFF_DETECT_COPY_HARDER);
if (dirs_removed && (break_idx || want_copies))
BUG("dirs_removed incompatible with break/copy detection");
if (break_idx && relevant_sources)
diff --git a/merge-ort.c b/merge-ort.c
index 6491070d965..77498354652 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4782,7 +4782,7 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
* sanity check them anyway.
*/
assert(opt->detect_renames >= -1 &&
- opt->detect_renames <= DIFF_DETECT_COPY);
+ opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
assert(opt->verbosity >= 0 && opt->verbosity <= 5);
assert(opt->buffer_output <= 2);
assert(opt->obuf.len == 0);
diff --git a/merge-recursive.c b/merge-recursive.c
index e3beb0801b1..d52dd536606 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3708,7 +3708,7 @@ static int merge_start(struct merge_options *opt, struct tree *head)
assert(opt->branch1 && opt->branch2);
assert(opt->detect_renames >= -1 &&
- opt->detect_renames <= DIFF_DETECT_COPY);
+ opt->detect_renames <= DIFF_DETECT_COPY_HARDER);
assert(opt->detect_directory_renames >= MERGE_DIRECTORY_RENAMES_NONE &&
opt->detect_directory_renames <= MERGE_DIRECTORY_RENAMES_TRUE);
assert(opt->rename_limit >= -1);
base-commit: 692be87cbba55e8488f805d236f2ad50483bd7d5
--
gitgitgadget
^ permalink raw reply related
* [ANNOUNCE] Git for Windows 2.43.0-rc0
From: Johannes Schindelin @ 2023-11-03 8:55 UTC (permalink / raw)
To: git-for-windows, git, git-packagers; +Cc: Johannes Schindelin
Dear Git users,
I hereby announce that Git for Windows 2.43.0-rc0 is available from:
https://github.com/git-for-windows/git/releases/tag/v2.43.0-rc0.windows.1
Changes since Git for Windows v2.42.0(2) (August 30th 2023)
New Features
* Comes with Git v2.43.0-rc0.
* Comes with MinTTY v3.6.5.
* Comes with MSYS2 runtime v3.4.9.
* Comes with GNU TLS v3.8.1.
* When installing into a Windows setup with Mandatory Address Space
Layout Randomization (ASLR) enabled, which is incompatible with the
MSYS2 runtime powering Git Bash, SSH and some other programs
distributed with Git for Windows, the Git for Windows installer now
offers to add exceptions that will allow those programs to work as
expected.
* Comes with OpenSSH v9.5.P1.
* Comes with cURL v8.4.0.
* Comes with OpenSSL v3.1.4.
* Comes with Git Credential Manager v2.4.1.
Bug Fixes
* Symbolic links whose target is an absolute path without the drive
prefix accidentally had a drive prefix added when checked out,
rendering them "eternally modified". This bug has been fixed.
* Git for Windows's installer is no longer confused by global GIT_*
environment variables.
* The installer no longer claims that "fast-forward or merge" is the
default git pull behavior: The default behavior has changed in Git
a while ago, to "fast-forward only".
Git-2.43.0-rc0-64-bit.exe | 11a41993a28a7f6dcf0381dd6b92b0ee013c1acf227a332074bb323c0a479e2a
Git-2.43.0-rc0-32-bit.exe | 924e10b97571ce4810434fc1464a63f97946def7a0492283abc03bc84cba35e7
PortableGit-2.43.0-rc0-64-bit.7z.exe | 697bcdf0e182a551221048f4447baf2f4cbc9bc54718830dde33a497192f0bd3
PortableGit-2.43.0-rc0-32-bit.7z.exe | 35eaed402eb541c7892323eae5b85604a5fa1f86741850c43fcd7745ab14bf31
MinGit-2.43.0-rc0-64-bit.zip | 216006c249857c6c2aa9b2d44b06b0dbc89d4bd52202dae6a3201cfa432fc430
MinGit-2.43.0-rc0-32-bit.zip | 50fea5aeee4b2ba97e300ca973b1a85c3bd9618e030079a49aa849cbbef1db17
MinGit-2.43.0-rc0-busybox-64-bit.zip | c633050b43896e8bfd318f2251b3e66a4161287e00e0ad2b92de25c8c6ee511b
MinGit-2.43.0-rc0-busybox-32-bit.zip | cbfdbf1f148aefb04c51327ef6642e4c6366b86c105c3de0fd846f33dd79c120
Git-2.43.0-rc0-64-bit.tar.bz2 | bdd7e890f297c351f16e262ab94ad1e77d18c5b647e54d06a1b80a7dda6ac60f
Git-2.43.0-rc0-32-bit.tar.bz2 | 1765bc7a4f33984a209dca4570ff964af3f4ca68e00d4528c50ce32a9e955e0c
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses
From: Andy Koppe @ 2023-11-03 8:22 UTC (permalink / raw)
To: Kousik Sanagavarapu, Liam Beguin; +Cc: git
In-Reply-To: <20231027184357.21049-1-five231003@gmail.com>
On 27/10/2023 19:40, Kousik Sanagavarapu wrote:
> Liam Beguin <liambeguin@gmail.com> wrote:
>> @@ -808,6 +808,17 @@ static size_t format_person_part(struct strbuf *sb, char part,
>> strbuf_add(sb, mail, maillen);
>> return placeholder_len;
>> }
>> + if (part == 'a' || part == 'A') { /* domain-part */
>> + const char *at = memchr(mail, '@', maillen);
>> + if (at) {
>> + at += 1;
>> + maillen -= at - mail;
>> + strbuf_add(sb, at, maillen);
>> + } else {
>> + strbuf_add(sb, mail, maillen);
>> + }
>> + return placeholder_len;
>> + }
>>
>> if (!s.date_begin)
>> goto skip;
>
> So, if we have a domain-name, we grab it, else (the case where we don't
> have '@') we grab it as-is. Looks good.
I'm not sure that this is the right way to handle a missing '@' here
actually, because %al already returns the whole email field in that
case, which makes sense as the likes of the 'mail' command would
interpret it as a local username.
And if someone was going to use %al and the new specifier together to
format the parts of the email field differently, they probably wouldn't
want the field to appear twice.
Therefore I think it would be more appropriate to expand to nothing in
that case. Tools that consume this output would already need to be able
to deal with the empty case, as it could also happen if there's a single
'@' at the end of the email field, or if the field is empty.
Regards,
Andy
^ permalink raw reply
* Re: [PATCH] ci: upgrade to using macos-13
From: Junio C Hamano @ 2023-11-03 8:16 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
In-Reply-To: <pull.1607.git.1698996455218.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In April, GitHub announced that the `macos-13` pool is available:
> https://github.blog/changelog/2023-04-24-github-actions-macos-13-is-now-available/.
> It is only a matter of time until the `macos-12` pool is going away,
> therefore we should switch now, without pressure of a looming deadline.
> ...
> To reinstate the spirit of that commit _and_ to fix that build failure,
> let's switch to the now-newest GCC version: v13.x.
Thank you very much for a well reasoned patch that is very timely.
I am tempted to fast-track this one down from 'seen' to 'maint'
before we tag -rc1.
Will queue.
^ 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