* Re: [PATCH] revision: allow missing promisor objects on CLI
From: Jonathan Tan @ 2019-12-30 18:38 UTC (permalink / raw)
To: gitster; +Cc: jonathantanmy, git, matvore
In-Reply-To: <xmqqlfqxhzvu.fsf@gitster-ct.c.googlers.com>
> Jonathan Tan <jonathantanmy@google.com> writes:
>
> > object = get_reference(revs, arg, &oid, flags ^ local_flags);
> > if (!object)
> > - return revs->ignore_missing ? 0 : -1;
> > + /*
> > + * Either this object is missing and ignore_missing is true, or
> > + * this object is a (missing) promisor object and
> > + * exclude_promisor_objects is true.
>
> I had to guess and dig where these assertions are coming from; we
> should not force future readers of the code to.
>
> At least this comment must say why these assertions hold. Say
> something like "get_reference() yields NULL on only such and such
> cases" before concluding with "and in any of these cases, we can
> safely ignore it because ...".
OK, will do.
> I think the two cases the comment covers are safe for this caller to
> silently return 0. Another case get_reference() yields NULL is when
> oid_object_info() says it is a commit but it turns out that the
> object is found by repo_parse_commit() to be a non-commit, isn't it?
> I am not sure if it is safe for this caller to just return 0. There
> may be some other "unusual-but-not-fatal" cases where get_reference()
> does not hit a die() but returns NULL.
I don't think there is any other case where get_reference() yields NULL,
at least where I based my patch (99c33bed56 ("Git 2.25-rc0",
2019-12-25)). Quoting the entire get_reference():
> static struct object *get_reference(struct rev_info *revs, const char *name,
> const struct object_id *oid,
> unsigned int flags)
> {
> struct object *object;
>
> /*
> * If the repository has commit graphs, repo_parse_commit() avoids
> * reading the object buffer, so use it whenever possible.
> */
> if (oid_object_info(revs->repo, oid, NULL) == OBJ_COMMIT) {
> struct commit *c = lookup_commit(revs->repo, oid);
> if (!repo_parse_commit(revs->repo, c))
> object = (struct object *) c;
> else
> object = NULL;
> } else {
> object = parse_object(revs->repo, oid);
> }
No return statements at all prior to this line.
> if (!object) {
> if (revs->ignore_missing)
> return object;
Return NULL (the value of object).
> if (revs->exclude_promisor_objects && is_promisor_object(oid))
> return NULL;
Return NULL.
> die("bad object %s", name);
Die (so this function invocation never returns). In conclusion, if
object is NULL at this point in time, get_reference() either returns
NULL or dies.
> }
Since get_reference() did not return NULL or die, object is non-NULL
here.
> object->flags |= flags;
> return object;
Nothing has overridden object since, so we're returning non-NULL here.
> }
So I think get_reference() only returns NULL in those two safe cases.
(Or did I miss something?)
^ permalink raw reply
* Re: ERANGE strikes again on my Windows build; RFH
From: Jonathan Nieder @ 2019-12-30 18:06 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <6eb02a73-9dcb-f1fc-f015-80e71e9910d6@kdbg.org>
Hi,
Johannes Sixt wrote:
> In sha1-file.c:read_object_file_extended() we have the following pattern:
>
> errno = 0;
> data = read_object(r, repl, type, size);
> if (data)
> return data;
>
> if (errno && errno != ENOENT)
> die_errno(_("failed to read object %s"), oid_to_hex(oid));
>
> That is, it is expected that read_object() does not change the value of
> errno in the non-error case. I find it intriguing that we expect a quite
> large call graph that is behind read_object() to behave this way.
Yes, this seems dubious.
In fact this is only inspecting errno in the returned-NULL case. If I
look only at the code above and not at the implementation of
read_object, then I would say that the bug is the 'errno &&' part: when
errno is meaningful for a function for a given return value, the usual
convention is
(1) it *always* sets errno for errors, not conditionally
(2) it never sets errno to 0
There are some exceptions (like strtoul) but they are few and
unfortunate, not something to be imitated.
Do you have more details about the case where read_object is expected
to produce errno == 0? I'm wondering whether we forgot to set 'errno
= ENOENT' explicitly somewhere.
Thanks and hope that helps,
Jonathan
^ permalink raw reply
* Re: ERANGE strikes again on my Windows build; RFH
From: Junio C Hamano @ 2019-12-30 17:42 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <6eb02a73-9dcb-f1fc-f015-80e71e9910d6@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> In sha1-file.c:read_object_file_extended() we have the following pattern:
>
> errno = 0;
> data = read_object(r, repl, type, size);
> if (data)
> return data;
>
> if (errno && errno != ENOENT)
> die_errno(_("failed to read object %s"), oid_to_hex(oid));
>
> That is, it is expected that read_object() does not change the value of
> errno in the non-error case. I find it intriguing that we expect a quite
> large call graph that is behind read_object() to behave this way.
Yeah, that is somewhat unfortunate and dubious, but as long as it
works (i.e. various system library calls the codepaths involved
behave sensibly), ...
> What if a subordinate callee starts doing
>
> if (some_syscall(...) < 0) {
> if (errno == EEXIST) {
> /* never mind, that's OK */
> ...
> }
> }
>
> Would it be required to reset errno to its previous value in this
> failure-is-not-an-error case?
... that would be the logical conclusion, I think.
As a longer term fix for better portability (i.e. system libraries
may not behave exactly the way how the codepaths and POSIX expects
wrt to the errors detected), it may become necessary to change the
function to yield "how the call failed" information in addition to
"here is the block of bytes I read for the object", without relying
on particular errno. But as a shorter term solution, ...
> The problem in my Windows build is that one of these subordinate
> syscalls is vsnprintf() (as part of a strbuf_add variant, I presume),
> and it fails with ERANGE when the buffer is too short. Do I have to
> modify the vsnprintf emulation to restore errno?
... I think this is a reasonable thing to do regardless.
Thanks.
^ permalink raw reply
* [PATCH 3/3] t: drop copy&pasted tests for --pathspec-from-file
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
In-Reply-To: <pull.503.git.1577727747.gitgitgadget@gmail.com>
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
With direct tests for `parse_pathspec_file()` already in place, it is
not very reasonable to copy&paste 6 tests for `parse_pathspec_file()`
for every git command that uses it (I counted 13 commands that could use
it eventually).
I believe that indirect tests are redundant because I don't expect
direct tests to ever disagree with indirect tests.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 75 +--------------------------
t/t2072-restore-pathspec-file.sh | 75 +--------------------------
t/t3704-add-pathspec-file.sh | 75 +--------------------------
t/t7107-reset-pathspec-file.sh | 84 +++----------------------------
t/t7526-commit-pathspec-file.sh | 75 +--------------------------
5 files changed, 16 insertions(+), 368 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index 2dc8901bca..559b4528d7 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git checkout --pathspec-from-file=- HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git checkout --pathspec-from-file=list HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git checkout --pathspec-from-file=- --pathspec-file-nul HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index 70e95ef3b6..9b3125d582 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -35,7 +35,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git restore --pathspec-from-file=- --source=HEAD^1 &&
@@ -46,19 +46,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git restore --pathspec-from-file=list --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git restore --pathspec-from-file=- --pathspec-file-nul --source=HEAD^1 &&
@@ -70,65 +58,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- M fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
-
- cat >expect <<-\EOF &&
- M fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 2e0141fcce..9009f8a9ac 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -23,7 +23,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git add --pathspec-from-file=- &&
@@ -34,19 +34,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git add --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul &&
@@ -58,65 +46,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 52a44f033d..5b845f4f7c 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -25,7 +25,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
git rm fileA.t &&
@@ -37,20 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- git reset --pathspec-from-file=list &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
git rm fileA.t fileB.t &&
@@ -63,76 +50,21 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- git rm fileA.t fileB.t &&
- printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- D fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
+test_expect_success 'only touches what was listed' '
restore_checkpoint &&
- git rm fileA.t fileB.t &&
- printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
+ git rm fileA.t fileB.t fileC.t fileD.t &&
+ printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
- D fileA.t
+ D fileA.t
D fileB.t
+ D fileC.t
+ D fileD.t
EOF
verify_expect
'
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- git rm fileA.t &&
- printf "\"file\\\\101.t\"" >list &&
- # Note: "git reset" has not yet learned to fail on wrong pathspecs
- git reset --pathspec-from-file=list --pathspec-file-nul &&
-
- cat >expect <<-\EOF &&
- D fileA.t
- EOF
- test_must_fail verify_expect
-'
-
test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
restore_checkpoint &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index e7dc2ff8b1..8d6c652690 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -26,7 +26,7 @@ verify_expect () {
test_cmp expect actual
}
-test_expect_success '--pathspec-from-file from stdin' '
+test_expect_success 'simplest' '
restore_checkpoint &&
echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
@@ -37,19 +37,7 @@ test_expect_success '--pathspec-from-file from stdin' '
verify_expect
'
-test_expect_success '--pathspec-from-file from file' '
- restore_checkpoint &&
-
- echo fileA.t >list &&
- git commit --pathspec-from-file=list -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect
-'
-
-test_expect_success 'NUL delimiters' '
+test_expect_success '--pathspec-file-nul' '
restore_checkpoint &&
printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
@@ -61,65 +49,6 @@ test_expect_success 'NUL delimiters' '
verify_expect
'
-test_expect_success 'LF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'no trailing delimiter' '
- restore_checkpoint &&
-
- printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'CRLF delimiters' '
- restore_checkpoint &&
-
- printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- A fileB.t
- EOF
- verify_expect
-'
-
-test_expect_success 'quotes' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- # git takes \101 and spits A
- printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
-
- cat >expect <<-\EOF &&
- A fileA.t
- EOF
- verify_expect expect
-'
-
-test_expect_success 'quotes not compatible with --pathspec-file-nul' '
- restore_checkpoint &&
-
- # shell takes \\\\101 and spits \\101
- # printf takes \\101 and spits \101
- printf "\"file\\\\101.t\"" >list &&
- test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] t: directly test parse_pathspec_file()
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
In-Reply-To: <pull.503.git.1577727747.gitgitgadget@gmail.com>
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Previously, `parse_pathspec_file()` was tested indirectly by invoking
git commands with properly crafted inputs. As demonstrated by the
previous bugfix, testing complicated black boxes indirectly can lead to
tests that silently test the wrong thing.
Introduce direct tests for `parse_pathspec_file()`.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 34 +++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++
5 files changed, 126 insertions(+)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
diff --git a/Makefile b/Makefile
index 09f98b777c..0061f96e8a 100644
--- a/Makefile
+++ b/Makefile
@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-oidmap.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-parse-options.o
+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-prio-queue.o
diff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c
new file mode 100644
index 0000000000..e7f525feb9
--- /dev/null
+++ b/t/helper/test-parse-pathspec-file.c
@@ -0,0 +1,34 @@
+#include "test-tool.h"
+#include "parse-options.h"
+#include "pathspec.h"
+#include "gettext.h"
+
+int cmd__parse_pathspec_file(int argc, const char **argv)
+{
+ struct pathspec pathspec;
+ const char *pathspec_from_file = 0;
+ int pathspec_file_nul = 0, i;
+
+ static const char *const usage[] = {
+ "test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]",
+ NULL
+ };
+
+ struct option options[] = {
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
+ OPT_END()
+ };
+
+ parse_options(argc, argv, 0, options, usage, 0);
+
+ parse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,
+ pathspec_file_nul);
+
+ for (i = 0; i < pathspec.nr; i++) {
+ printf("%s\n", pathspec.items[i].original);
+ }
+
+ clear_pathspec(&pathspec);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f20989d449..c9a232d238 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "oidmap", cmd__oidmap },
{ "online-cpus", cmd__online_cpus },
{ "parse-options", cmd__parse_options },
+ { "parse-pathspec-file", cmd__parse_pathspec_file },
{ "path-utils", cmd__path_utils },
{ "pkt-line", cmd__pkt_line },
{ "prio-queue", cmd__prio_queue },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 8ed2af71d1..c8549fd87f 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);
int cmd__oidmap(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__parse_options(int argc, const char **argv);
+int cmd__parse_pathspec_file(int argc, const char** argv);
int cmd__path_utils(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
int cmd__prio_queue(int argc, const char **argv);
diff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh
new file mode 100755
index 0000000000..df7b319713
--- /dev/null
+++ b/t/t0067-parse_pathspec_file.sh
@@ -0,0 +1,89 @@
+#!/bin/sh
+
+test_description='Test parse_pathspec_file()'
+
+. ./test-lib.sh
+
+test_expect_success 'one item from stdin' '
+ echo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'one item from file' '
+ echo fileA.t >list &&
+ test-tool parse-pathspec-file --pathspec-from-file=list >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'NUL delimiters' '
+ printf "fileA.t\0fileB.t\0" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'LF delimiters' '
+ printf "fileA.t\nfileB.t\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'no trailing delimiter' '
+ printf "fileA.t\nfileB.t" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'CRLF delimiters' '
+ printf "fileA.t\r\nfileB.t\r\n" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ fileB.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'quotes' '
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&
+
+ cat >expect <<-\EOF &&
+ fileA.t
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--pathspec-file-nul takes quotes literally' '
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&
+
+ cat >expect <<-\EOF &&
+ "file\101.t"
+ EOF
+ test_cmp expect actual
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/3] t: fix quotes tests for --pathspec-from-file
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano, Alexandr Miloslavskiy
In-Reply-To: <pull.503.git.1577727747.gitgitgadget@gmail.com>
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
While working on the next patch, I also noticed that quotes testing via
`"\"file\\101.t\""` was somewhat incorrect: I escaped `\` one time while
I had to escape it two times! Tests still worked due to `"` being
preserved which in turn prevented pathspec from matching files.
Fix this by properly escaping one more time.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 9 +++++++--
t/t2072-restore-pathspec-file.sh | 9 +++++++--
t/t3704-add-pathspec-file.sh | 9 +++++++--
t/t7107-reset-pathspec-file.sh | 9 +++++++--
t/t7526-commit-pathspec-file.sh | 9 +++++++--
5 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..2dc8901bca 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git checkout --pathspec-from-file=- HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git checkout --pathspec-from-file=list --pathspec-file-nul HEAD^1
'
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..70e95ef3b6 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -109,7 +109,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git restore --pathspec-from-file=- --source=HEAD^1 &&
cat >expect <<-\EOF &&
M fileA.t
@@ -120,7 +123,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git restore --pathspec-from-file=list --pathspec-file-nul --source=HEAD^1
'
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..2e0141fcce 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -97,7 +97,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git add --pathspec-from-file=- &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git add --pathspec-from-file=- &&
cat >expect <<-\EOF &&
A fileA.t
@@ -108,7 +111,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git add --pathspec-from-file=list --pathspec-file-nul
'
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..52a44f033d 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -105,8 +105,11 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
git rm fileA.t &&
- printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
+ printf "\"file\\\\101.t\"" | git reset --pathspec-from-file=- &&
cat >expect <<-\EOF &&
D fileA.t
@@ -117,8 +120,10 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
git rm fileA.t &&
- printf "\"file\\101.t\"" >list &&
+ printf "\"file\\\\101.t\"" >list &&
# Note: "git reset" has not yet learned to fail on wrong pathspecs
git reset --pathspec-from-file=list --pathspec-file-nul &&
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..e7dc2ff8b1 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -100,7 +100,10 @@ test_expect_success 'CRLF delimiters' '
test_expect_success 'quotes' '
restore_checkpoint &&
- printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ # git takes \101 and spits A
+ printf "\"file\\\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
cat >expect <<-\EOF &&
A fileA.t
@@ -111,7 +114,9 @@ test_expect_success 'quotes' '
test_expect_success 'quotes not compatible with --pathspec-file-nul' '
restore_checkpoint &&
- printf "\"file\\101.t\"" >list &&
+ # shell takes \\\\101 and spits \\101
+ # printf takes \\101 and spits \101
+ printf "\"file\\\\101.t\"" >list &&
test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/3] t: rework tests for --pathspec-from-file
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 17:42 UTC (permalink / raw)
To: git; +Cc: Alexandr Miloslavskiy, Junio C Hamano
Please refer to commit messages for rationale.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
Previously in [3] there were some concerns on whether removing
copy&pasted tests is good. I still think that yes, it 's a good thing,
mostly because of high volume of potential 13*6=78 duplicate tests.
Still, I separated this change as last patch, so that the remaining
part of the branch can be taken without it.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://lore.kernel.org/git/xmqqwoatcn5u.fsf@gitster-ct.c.googlers.com/
Alexandr Miloslavskiy (3):
t: fix quotes tests for --pathspec-from-file
t: directly test parse_pathspec_file()
t: drop copy&pasted tests for --pathspec-from-file
Makefile | 1 +
t/helper/test-parse-pathspec-file.c | 34 +++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++++++++
t/t2026-checkout-pathspec-file.sh | 70 +----------------------
t/t2072-restore-pathspec-file.sh | 70 +----------------------
t/t3704-add-pathspec-file.sh | 70 +----------------------
t/t7107-reset-pathspec-file.sh | 79 +++----------------------
t/t7526-commit-pathspec-file.sh | 70 +----------------------
10 files changed, 142 insertions(+), 343 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-503%2FSyntevoAlex%2F%230207(git)_2b_test_parse_directly-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-503/SyntevoAlex/#0207(git)_2b_test_parse_directly-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/503
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v3 0/2] sparse-checkout: list directories in cone mode
From: Elijah Newren @ 2019-12-30 17:18 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: Git Mailing List, SZEDER Gábor, jon, Derrick Stolee,
Junio C Hamano
In-Reply-To: <pull.500.v3.git.1577719993.gitgitgadget@gmail.com>
Hi,
Thanks for the fixups. Just two small comments.
On Mon, Dec 30, 2019 at 7:33 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> When in cone mode, "git sparse-checkout set" takes a list of folders and
> constructs an ordered list of patterns for the sparse-checkout file. The
> "git sparse-checkout list" subcommand outputs the contents of the
> sparse-checkout file in a very basic way.
>
> This patch changes the behavior of "git sparse-checkout list" when
> core.sparseCheckoutCone=true. It will output the folders that were used in
> "git sparse-checkout set" to create the patterns, instead of the patterns
> themselves.
>
> I believe this was requested in the initial review, but I cannot find that
> message now.
This sentence isn't still true, is it?
> I was going to include this as part of a longer follow-up series, but I
> think this may be worth considering for the 2.25.0 release. Hence, it is
> included by itself.
>
> Update in V2:
>
> * Fixed typos/word choice in commit message.
>
>
> * Added a second commit including clarification on interactions with
> submodules.
>
>
>
> Thanks, -Stolee
>
> Derrick Stolee (2):
> sparse-checkout: list directories in cone mode
> sparse-checkout: document interactions with submodules
>
> Documentation/git-sparse-checkout.txt | 21 ++++++++++++++-
> builtin/sparse-checkout.c | 21 +++++++++++++++
> t/t1091-sparse-checkout-builtin.sh | 39 +++++++++++++++++++++++++++
> 3 files changed, 80 insertions(+), 1 deletion(-)
>
>
> base-commit: 761e3d26bbe44c51f83c4f1ad198461f57029ebd
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-500%2Fderrickstolee%2Fsparse-checkout-list-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-500/derrickstolee/sparse-checkout-list-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/500
>
> Range-diff vs v2:
>
> 1: d6f4f40486 ! 1: 7d4295bd06 sparse-checkout: list folders in cone mode
> @@ -1,17 +1,17 @@
> Author: Derrick Stolee <dstolee@microsoft.com>
>
> - sparse-checkout: list folders in cone mode
> + sparse-checkout: list directories in cone mode
>
> When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
> - command takes a list of folders as input, then creates an ordered
> - list of sparse-checkout patterns such that those folders are
> - recursively included and all sibling entries along the parent folders
> + command takes a list of directories as input, then creates an ordered
> + list of sparse-checkout patterns such that those directories are
> + recursively included and all sibling entries along the parent directories
> are also included. Listing the patterns is less user-friendly than the
> - folders themselves.
> + directories themselves.
>
> In cone mode, and as long as the patterns match the expected cone-mode
> pattern types, change the output of 'git sparse-checkout list' to only
> - show the folders that created the patterns.
> + show the directories that created the patterns.
>
> With this change, the following piped commands would not change the
> working directory:
> @@ -41,8 +41,8 @@
> based algorithms to compute inclusion in the sparse-checkout.
>
> +In the cone mode case, the `git sparse-checkout list` subcommand will list the
> -+folders that define the recursive patterns. For the example sparse-checkout file
> -+above, the output is as follows:
> ++directories that define the recursive patterns. For the example sparse-checkout
> ++file above, the output is as follows:
> +
> +--------------------------
> +$ git sparse-checkout list
> 2: 331bb7d6fb ! 2: 74bbd0f84d sparse-checkout: document interactions with submodules
> @@ -2,11 +2,6 @@
>
> sparse-checkout: document interactions with submodules
>
> - Junio asked what the behavior is between the sparse-checkout feature
> - and the submodule feature. The sparse-checkout builtin has not changed
> - the way these features interact, but we may as well document it in
> - the builtin docs.
> -
I actually liked the second sentence of this paragraph and thought it
provided useful information for future readers; I only thought the
first sentence should be removed.
> Using 'git submodule (init|deinit)' a user can select a subset of
> submodules to populate. This behaves very similar to the sparse-checkout
> feature, but those directories contain their own .git directory
> @@ -61,14 +56,14 @@
> + git sparse-checkout set folder1
> + ) &&
> + list_files super >dir &&
> -+ cat >expect <<-EOF &&
> ++ cat >expect <<-\EOF &&
> + a
> + folder1
> + modules
> + EOF
> + test_cmp expect dir &&
> + list_files super/modules/child >dir &&
> -+ cat >expect <<-EOF &&
> ++ cat >expect <<-\EOF &&
> + a
> + deep
> + folder1
>
> --
> gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/9] [RFC] Changed Paths Bloom Filters
From: Junio C Hamano @ 2019-12-30 17:02 UTC (permalink / raw)
To: Jeff King
Cc: Derrick Stolee, Garima Singh via GitGitGadget, git, szeder.dev,
jonathantanmy, jeffhost, me
In-Reply-To: <20191229062414.GC220034@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This is holding internal state in diff_options, but the same
> diff_options is often used for multiple diffs (e.g., "git log --raw"
> would use the same rev_info.diffopt over and over again).
>
> So it would need to be cleared between diffs. There's a similar feature
> in the "has_changes" flag, though it looks like it is cleared manually
> by callers. Yuck.
Do you mean we want reset_per_invocation_part_of_diff_options()
helper or something?
> I actually wonder if this could be rolled into the has_changes and
> diff_can_quit_early() feature. This really just a generalization of that
> feature (which is like setting max_changes to "1").
Yeah, I wondered about the same thing, after seeing the impressive
numbers ;-)
^ permalink raw reply
* Re: Updating the commit message for reverts
From: Junio C Hamano @ 2019-12-30 16:52 UTC (permalink / raw)
To: Gal Paikin; +Cc: git
In-Reply-To: <CAEsQYpMJGbw3L66vCd25Ht0bTBzvvt1yMRd2U3=u3U-BZukyzg@mail.gmail.com>
Gal Paikin <paiking@google.com> writes:
> So the idea of changing from "Revert Revert" to "Reland", "reapply"
> has a big problem: sometimes Revert^2 actually means 'reverting
> "Revert"' since "Revert" introduced a bug that wasn't in the original
> change.
Sorry, I do not see a relevance of the above in this discussion, as
the situation does not improve if you phrase it as "Revert^2" or
"Second Revert".
Also as somebody else said in downthread, the phrasing "second
revert" would typically mean "a patch gets applied, proves to be
premature and gets reverted, the revert is reverted because the
situation in the rest of the system improved to make the orignal
patch viable, and then gets reverted again due to some other
issues", i.e. "Revert Revert Revert do something", so it is even
worse.
^ permalink raw reply
* [RFC PATCH 2/2 v2] Add the configuration parameter core.branchnameincommit
From: Arnaud Bertrand @ 2019-12-30 16:32 UTC (permalink / raw)
To: arnaud.bertrand; +Cc: git, gitster, xda
In-Reply-To: <20191230163256.14749-1-xda@abalgo.com>
With this parameter, which is 0 by default (no change compare
to the normal behaviour) you have the possibility to activate
this feature to have the branchname in the header commit
When it exists, the branchname is accesible in the git log
with the pretty format placehoder "%Xb".
---
cache.h | 1 +
commit.c | 24 +++++++++++++++++-------
config.c | 5 +++++
environment.c | 1 +
4 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 1554488d66..dd7188a4e4 100644
--- a/cache.h
+++ b/cache.h
@@ -949,6 +949,7 @@ void reset_shared_repository(void);
* commands that do not want replace references to be active.
*/
extern int read_replace_refs;
+extern int branchname_in_commit;
extern char *git_replace_ref_base;
extern int fsync_object_files;
diff --git a/commit.c b/commit.c
index f64a0698be..e63d97d308 100644
--- a/commit.c
+++ b/commit.c
@@ -1428,6 +1428,7 @@ int commit_tree_extended(const char *msg, size_t msg_len,
const char *branch = "Unknown";
int flags;
const char *lbranch =resolve_ref_unsafe("HEAD",0,NULL,&flags);
+ int flbranchinextra = 0;
assert_oid_type(tree, OBJ_TREE);
@@ -1456,21 +1457,30 @@ int commit_tree_extended(const char *msg, size_t msg_len,
author = git_author_info(IDENT_STRICT);
strbuf_addf(&buffer, "author %s\n", author);
strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
- if (lbranch) {
- skip_prefix(lbranch,"refs/heads/",&branch);
- strbuf_addf(&buffer, "branch %s\n", branch);
- }
- else {
- strbuf_addf(&buffer, "branch Unknown\n");
- }
if (!encoding_is_utf8)
strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
while (extra) {
+ /* when commit is reworked - e.g. amend, the banch is already
+ * in extra-header and must not be modified
+ */
+ if (!strcmp(extra->key,"branch"))
+ flbranchinextra=1;
add_extra_header(&buffer, extra);
extra = extra->next;
}
+
+ if (branchname_in_commit && !flbranchinextra) {
+ if (lbranch) {
+ skip_prefix(lbranch,"refs/heads/",&branch);
+ strbuf_addf(&buffer, "branch %s\n", branch);
+ }
+ else {
+ strbuf_addf(&buffer, "branch Unknown\n");
+ }
+ }
+
strbuf_addch(&buffer, '\n');
/* And add the comment */
diff --git a/config.c b/config.c
index d75f88ca0c..bec1b5c3af 100644
--- a/config.c
+++ b/config.c
@@ -1389,6 +1389,11 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
return 0;
}
+ if (!strcmp(var, "core.branchnameincommit")) {
+ branchname_in_commit = git_config_bool(var, value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return platform_core_config(var, value, cb);
}
diff --git a/environment.c b/environment.c
index e72a02d0d5..1d266a91cf 100644
--- a/environment.c
+++ b/environment.c
@@ -52,6 +52,7 @@ const char *askpass_program;
const char *excludes_file;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
int read_replace_refs = 1;
+int branchname_in_commit = 0;
char *git_replace_ref_base;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
--
2.25.0.rc0.7.g17b02bf28a
^ permalink raw reply related
* [RFC PATCH 1/2 v2] Add branchname in commit header
From: Arnaud Bertrand @ 2019-12-30 16:32 UTC (permalink / raw)
To: arnaud.bertrand; +Cc: git, gitster, xda
In-Reply-To: <20191230163256.14749-1-xda@abalgo.com>
Add the branchname in the commit header before the commit message
the following line is added:
branch <branchname>
where <branchname> comes from the function resolve_ref_unsafe("HEAD",...)
without the prefix refs/heads/
A placeholder is added to the pretty format "%Xb" to print the branch information,
X if for "extra-header" and can be use in the future for new features
b is of course for "branch"
the %Xb returns an empty string when branchname information is not found
---
Documentation/pretty-formats.txt | 1 +
commit.c | 11 +++++++++++
pretty.c | 15 +++++++++++++++
3 files changed, 27 insertions(+)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 1a7212ce5a..bd52908f53 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -241,6 +241,7 @@ endif::git-rev-list[]
'%gE':: reflog identity email (respecting .mailmap, see
linkgit:git-shortlog[1] or linkgit:git-blame[1])
'%gs':: reflog subject
+'%Xb':: branchname in which commit was done
'%(trailers[:options])':: display the trailers of the body as
interpreted by
linkgit:git-interpret-trailers[1]. The
diff --git a/commit.c b/commit.c
index 434ec030d6..f64a0698be 100644
--- a/commit.c
+++ b/commit.c
@@ -1425,6 +1425,9 @@ int commit_tree_extended(const char *msg, size_t msg_len,
int result;
int encoding_is_utf8;
struct strbuf buffer;
+ const char *branch = "Unknown";
+ int flags;
+ const char *lbranch =resolve_ref_unsafe("HEAD",0,NULL,&flags);
assert_oid_type(tree, OBJ_TREE);
@@ -1453,6 +1456,14 @@ int commit_tree_extended(const char *msg, size_t msg_len,
author = git_author_info(IDENT_STRICT);
strbuf_addf(&buffer, "author %s\n", author);
strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_STRICT));
+ if (lbranch) {
+ skip_prefix(lbranch,"refs/heads/",&branch);
+ strbuf_addf(&buffer, "branch %s\n", branch);
+ }
+ else {
+ strbuf_addf(&buffer, "branch Unknown\n");
+ }
+
if (!encoding_is_utf8)
strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
diff --git a/pretty.c b/pretty.c
index 305e903192..5961c39398 100644
--- a/pretty.c
+++ b/pretty.c
@@ -804,6 +804,7 @@ struct format_commit_context {
/* The following ones are relative to the result struct strbuf. */
size_t wrap_start;
+ char *branch;
};
static void parse_commit_header(struct format_commit_context *context)
@@ -1367,6 +1368,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
return 1;
}
+
+ /* Now add extra header info */
+ if (placeholder[0] == 'X') {
+ switch (placeholder[1]) {
+ case 'b': /* branch ... */
+ c->branch = get_header(msg,"branch");
+ if (c->branch)
+ strbuf_addstr(sb, c->branch);
+ free(c->branch);
+ return 2;
+ }
+ }
+
+
/* Now we need to parse the commit message. */
if (!c->commit_message_parsed)
parse_commit_message(c);
--
2.25.0.rc0.7.g17b02bf28a
^ permalink raw reply related
* [RFC PATCH 0/2 v2] *** Add branchname in commit when core.branchnameincommit is set ***
From: Arnaud Bertrand @ 2019-12-30 16:32 UTC (permalink / raw)
To: arnaud.bertrand; +Cc: git, gitster, xda
In-Reply-To: <20191229222633.23815-1-arnaud.bertrand@abalgo.com>
To avoid any change in the current git behaviour by default, I've added
a configuration variable that allow to activate the feature for those
who want to see the branchname in commit.
By default, this feature is disabled
Arnaud Bertrand (2):
Add branchname in commit header
Add the configuration parameter core.branchnameincommit
Documentation/pretty-formats.txt | 1 +
cache.h | 1 +
commit.c | 21 +++++++++++++++++++++
config.c | 5 +++++
environment.c | 1 +
pretty.c | 15 +++++++++++++++
6 files changed, 44 insertions(+)
--
2.25.0.rc0.7.g17b02bf28a
^ permalink raw reply
* Re: Re: [PATCH 1/1] git-gui: add possibility to open currently selected file
From: Zoli Szabó @ 2019-12-30 16:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pratyush Yadav, git
In-Reply-To: <xmqqh81iiv18.fsf@gitster-ct.c.googlers.com>
Adapted the commit message once again (PATCH v3).
On 2019.12.30 01:14, Junio C Hamano wrote:
> Zoli Szabó <zoli.szabo@gmail.com> writes:
>
>> On 2019.12.28 00:32, Junio C Hamano wrote:
>>
>>> The phrasing on the title is a bit awkward. "add possibility to do
>>> X" is better spelled "allow doing X", no?
>>
>> Thank you, Junio, for the hint. Updated the commit message accordingly
>> (PATCH v2).
>
> Also, do not start the body of the proposed log message with half
> sentence. The title is supposed to be a full sentence.
>
>
>
^ permalink raw reply
* Re: [PATCH 0/9] [RFC] Changed Paths Bloom Filters
From: Derrick Stolee @ 2019-12-30 16:04 UTC (permalink / raw)
To: Jeff King
Cc: Garima Singh via GitGitGadget, git, szeder.dev, jonathantanmy,
jeffhost, me, Junio C Hamano
In-Reply-To: <20191229062414.GC220034@coredump.intra.peff.net>
On 12/29/2019 1:24 AM, Jeff King wrote:
> On Fri, Dec 27, 2019 at 11:11:37AM -0500, Derrick Stolee wrote:
>
>>> So here are a few patches to reduce the CPU and memory usage. They could
>>> be squashed in at the appropriate spots, or perhaps taken as inspiration
>>> if there are better solutions (especially for the first one).
>>
>> I tested these patches with the Linux kernel repo and reported my results
>> on each patch. However, I wanted to also test on a larger internal repo
>> (the AzureDevOps repo), which has ~500 commits with more than 512 changes,
>> and generally has larger diffs than the Linux kernel repo.
>>
>> | Version | Time | Memory |
>> |----------|--------|--------|
>> | Garima | 16m36s | 27.0gb |
>> | Peff 1 | 6m32s | 28.0gb |
>> | Peff 2 | 6m48s | 5.6gb |
>> | Peff 3 | 6m14s | 4.5gb |
>> | Shortcut | 3m47s | 4.5gb |
>>
>> For reference, I found the time and memory information using
>> "/usr/bin/time --verbose" in a bash script.
>
> Thanks for giving it more exercise. My heap profiling was done with
> massif, which measures the heap directly. Measuring RSS would cover
> that, but will also include the mmap'd packfiles. That's probably why
> your linux.git numbers were slightly higher than mine.
That's interesting. I initially avoided massif because it is so much
slower than /usr/bin/time. However, the inflated numbers could be
explained by that. Also, the distinction between mem_heap and
mem_heap_extra may have interesting implications. Looking online, it
seems that large mem_heap_extra implies the heap is fragmented from
many small allocations.
Here are my findings on the Linux repo:
| Version | mem_heap | mem_heap_extra |
|----------|----------|----------------|
| Peff 1 | 6,500mb | 913mb |
| Peff 2 | 3,100mb | 286mb |
| Peff 3 | 781mb | 235mb |
These numbers more closely match your numbers (in sum of the two
columns).
> (massif is a really great tool if you haven't used it, as it also shows
> which allocations were using the memory. But it's part of valgrind, so
> it definitely doesn't run on native Windows. It might work under WSL,
> though. I'm sure there are also other heap profilers on Windows).
I am using my Linux machine for my tests. Garima is using her Windows
machine.
>> By "Shortcut" in the table above, I mean the following patch on top of
>> Garima's and Peff's changes. It inserts a max_changes option into struct
>> diff_options to halt the diff early. This seemed like an easier change
>> than creating a new tree diff algorithm wholesale.
>
> Yeah, I'm not opposed to a diff feature like this.
>
> But be careful, because...
>
>> diff --git a/diff.h b/diff.h
>> index 6febe7e365..9443dc1b00 100644
>> --- a/diff.h
>> +++ b/diff.h
>> @@ -285,6 +285,11 @@ struct diff_options {
>> /* Number of hexdigits to abbreviate raw format output to. */
>> int abbrev;
>>
>> + /* If non-zero, then stop computing after this many changes. */
>> + int max_changes;
>> + /* For internal use only. */
>> + int num_changes;
>
> This is holding internal state in diff_options, but the same
> diff_options is often used for multiple diffs (e.g., "git log --raw"
> would use the same rev_info.diffopt over and over again).
>
> So it would need to be cleared between diffs. There's a similar feature
> in the "has_changes" flag, though it looks like it is cleared manually
> by callers. Yuck.
You're right about this. What if we initialize it in diff_tree_paths()
before it calls the recursive ll_difF_tree_paths()?
> This isn't a problem for commit-graph right now, but:
>
> - it actually could be using a single diff_options, which would be
> slightly simpler (it doesn't seem to save much CPU, though, because
> the initialization is relatively cheap)
>
> - it's a bit of a subtle bug to leave hanging around for the next
> person who tries to use the feature
>
> I actually wonder if this could be rolled into the has_changes and
> diff_can_quit_early() feature. This really just a generalization of that
> feature (which is like setting max_changes to "1").
I thought about this at first, but it only takes a struct diff_options
right now. It does have an internally-mutated member (flags.has_changes)
but it also seems a bit wrong to add a uint32_t of the count in this.
Changing the prototype could be messy, too.
There are also multiple callers, and limiting everything to tree-diff.c
limits the impact.
Thanks,
-Stolee
^ permalink raw reply
* [PATCH v3 1/1] git-gui: allow opening currently selected file in default app
From: Zoli Szabó via GitGitGadget @ 2019-12-30 15:56 UTC (permalink / raw)
To: git; +Cc: Pratyush Yadav, Zoli Szabó
In-Reply-To: <pull.499.v3.git.1577721419.gitgitgadget@gmail.com>
From: =?UTF-8?q?Zoli=20Szab=C3=B3?= <zoli.szabo@gmail.com>
Many times there's the need to quickly open a source file (the one you're
looking at in Git GUI) in the predefined text editor / IDE. Of course,
the file can be searched for in your preferred file manager or directly
in the text editor, but having the option to directly open the current
file from Git GUI would be just faster. This change enables just that by:
- clicking the diff header path (which is now highlighted as a hyperlink)
- or diff header path context menu -> Open;
Note: executable files will be run and not opened for editing.
Signed-off-by: Zoli Szabó <zoli.szabo@gmail.com>
---
git-gui.sh | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index c1be733e3e..8920e4ddb0 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2248,9 +2248,8 @@ proc do_git_gui {} {
}
}
-proc do_explore {} {
- global _gitworktree
- set explorer {}
+# Get the system-specific explorer app/command.
+proc get_explorer {} {
if {[is_Cygwin] || [is_Windows]} {
set explorer "explorer.exe"
} elseif {[is_MacOSX]} {
@@ -2259,9 +2258,23 @@ proc do_explore {} {
# freedesktop.org-conforming system is our best shot
set explorer "xdg-open"
}
+ return $explorer
+}
+
+proc do_explore {} {
+ global _gitworktree
+ set explorer [get_explorer]
eval exec $explorer [list [file nativename $_gitworktree]] &
}
+# Open file relative to the working tree by the default associated app.
+proc do_file_open {file} {
+ global _gitworktree
+ set explorer [get_explorer]
+ set full_file_path [file join $_gitworktree $file]
+ eval exec $explorer [list [file nativename $full_file_path]] &
+}
+
set is_quitting 0
set ret_code 1
@@ -3513,9 +3526,11 @@ tlabel .vpane.lower.diff.header.file \
-justify left
tlabel .vpane.lower.diff.header.path \
-background gold \
- -foreground black \
+ -foreground blue \
-anchor w \
- -justify left
+ -justify left \
+ -font [eval font create [font configure font_ui] -underline 1] \
+ -cursor hand2
pack .vpane.lower.diff.header.status -side left
pack .vpane.lower.diff.header.file -side left
pack .vpane.lower.diff.header.path -fill x
@@ -3530,8 +3545,12 @@ $ctxm add command \
-type STRING \
-- $current_diff_path
}
+$ctxm add command \
+ -label [mc Open] \
+ -command {do_file_open $current_diff_path}
lappend diff_actions [list $ctxm entryconf [$ctxm index last] -state]
bind_button3 .vpane.lower.diff.header.path "tk_popup $ctxm %X %Y"
+bind .vpane.lower.diff.header.path <Button-1> {do_file_open $current_diff_path}
# -- Diff Body
#
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/1] git-gui: allow opening currently selected file
From: Zoli Szabó via GitGitGadget @ 2019-12-30 15:56 UTC (permalink / raw)
To: git; +Cc: Pratyush Yadav
In-Reply-To: <pull.499.v2.git.1577647930.gitgitgadget@gmail.com>
...in the default associated app (e.g. in a text editor / IDE).
Many times there's the need to quickly open a source file (the one you're
looking at in Git GUI) in the predefined text editor / IDE. Of course, the
file can be searched for in your preferred file manager or directly in the
text editor, but having the option to directly open the current file from
Git GUI would be just faster. This change enables just that by:
* clicking the diff header path (which is now highlighted as a hyperlink)
* or diff header path context menu -> Open;
Note: executable files will be run and not opened for editing.
Signed-off-by: Zoli Szabó zoli.szabo@gmail.com [zoli.szabo@gmail.com]
Zoli Szabó (1):
git-gui: allow opening currently selected file in default app
git-gui.sh | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
base-commit: 23cbe427c44645a3ab0449919e55bade5eb264bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-499%2Fzoliszabo%2Fgit-gui%2Fopen-current-file-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-499/zoliszabo/git-gui/open-current-file-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/499
Range-diff vs v2:
1: a6fde256f8 ! 1: 1b2363be72 git-gui: allow opening currently selected file
@@ -1,8 +1,6 @@
Author: Zoli Szabó <zoli.szabo@gmail.com>
- git-gui: allow opening currently selected file
-
- ...in the default associated app (e.g. in a text editor / IDE).
+ git-gui: allow opening currently selected file in default app
Many times there's the need to quickly open a source file (the one you're
looking at in Git GUI) in the predefined text editor / IDE. Of course,
--
gitgitgadget
^ permalink raw reply
* Re: Feature request: add a metadata in the commit: the "commited in branch" information
From: Paul Smith @ 2019-12-30 15:15 UTC (permalink / raw)
To: Arnaud Bertrand; +Cc: git
In-Reply-To: <CAEW0o+gtya5tm6Wb474Srmb2j4E9ocm9p75=aZWjTASbApsb1A@mail.gmail.com>
On Mon, 2019-12-30 at 12:59 +0100, Arnaud Bertrand wrote:
> > Why does it need to be the branch name? You can add your own extra
> > metadata to the git description.
>
> That's typically my problem. It is not possible "by default", I mean
> - It is only possible if the developer configure something
> - or if there is an upper layer that guarantee this
> By default, there is no hook embedded with the clone. So, as far as I
> know (and I hope I'm wrong!), you have to use upper layer tools or to
> change environment variables to activate this feature.
In general I have found that trying to mandate what users do in their own
repositories on their own systems is a losing proposition.
Instead, we put requirements on what content is pushed to the central
repository. Because the central repository is managed by the SCM admin
team we always know only properly-constructed commits can appear there,
without having to assume that every individual developer's local
environment has been set up in a specific way.
This can be done with hooks in the central repository: there are Git hooks
that are run before any push is accepted, which can cause the push to be
rejected, and hooks that are run after a push is accepted, which can be
used for triggering other operations.
So if you have a requirement about contents of Git commit message format,
for example, you can enforce that via these hooks. If someone attempts to
push commits to the central repository and the commit message has an
incorrect format then the push is rejected and they'll have to fix it
before they can proceed to push.
In the environments I've been associated with we don't care about branch
names; instead everything is based on bug tracker identifiers. Every
commit needs to be associated with a valid bug ID (added to the commit
message) and the pre-push hook verifies this and rejects the commit if not.
Then after the push is accepted, post-push hooks will update the bug
tracker with information about the push (SHA, software version, etc.) This
ensures that development and management can use the bug tracker as their
primary planning tool to know what has been accomplished and what is left
to accomplish. Since the commit message is persisted through cherry-picks,
etc. it allows us to know which bugs were fixed in which different patch
release branches as well.
^ permalink raw reply
* [PATCH 1/1] t: add tests for error conditions with --pathspec-from-file
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 15:38 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Alexandr Miloslavskiy, Junio C Hamano,
Alexandr Miloslavskiy
In-Reply-To: <pull.502.git.1577720318.gitgitgadget@gmail.com>
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Also move some old tests into the new tests: it doesn't seem reasonable
to have individual error condition tests.
Old test for `git commit` was corrected, previously it was instructed
to use stdin but wasn't provided with any stdin. While this works at
the moment, it's not exactly perfect.
Old tests for `git reset` were improved to test for a specific error
message.
Suggested-By: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
t/t2026-checkout-pathspec-file.sh | 17 +++++++++++++++++
t/t2072-restore-pathspec-file.sh | 18 ++++++++++++++++++
t/t3704-add-pathspec-file.sh | 25 +++++++++++++++++++++++++
t/t7107-reset-pathspec-file.sh | 30 +++++++++++++++++++++---------
t/t7526-commit-pathspec-file.sh | 27 ++++++++++++++++++++++++---
5 files changed, 105 insertions(+), 12 deletions(-)
diff --git a/t/t2026-checkout-pathspec-file.sh b/t/t2026-checkout-pathspec-file.sh
index f62fd27440..0926312370 100755
--- a/t/t2026-checkout-pathspec-file.sh
+++ b/t/t2026-checkout-pathspec-file.sh
@@ -136,4 +136,21 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
+test_expect_success 'error conditions' '
+ restore_checkpoint &&
+ echo fileA.t >list &&
+
+ test_must_fail git checkout --pathspec-from-file=list --detach 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --detach" err &&
+
+ test_must_fail git checkout --pathspec-from-file=list --patch 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git checkout --pathspec-from-file=list -- fileA.t 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git checkout --pathspec-file-nul 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err
+'
+
test_done
diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh
index db58e83735..5c7abbbce3 100755
--- a/t/t2072-restore-pathspec-file.sh
+++ b/t/t2072-restore-pathspec-file.sh
@@ -136,4 +136,22 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
+test_expect_success 'error conditions' '
+ restore_checkpoint &&
+ echo fileA.t >list &&
+ >empty_list &&
+
+ test_must_fail git restore --pathspec-from-file=list --patch --source=HEAD^1 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git restore --pathspec-from-file=list --source=HEAD^1 -- fileA.t 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git restore --pathspec-file-nul --source=HEAD^1 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ test_must_fail git restore --pathspec-from-file=empty_list --source=HEAD^1 2>err &&
+ test_i18ngrep -e "you must specify path(s) to restore" err
+'
+
test_done
diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh
index 3cfdb669b7..a1696e1a39 100755
--- a/t/t3704-add-pathspec-file.sh
+++ b/t/t3704-add-pathspec-file.sh
@@ -124,4 +124,29 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
+test_expect_success 'error conditions' '
+ restore_checkpoint &&
+ echo fileA.t >list &&
+ >empty_list &&
+
+ test_must_fail git add --pathspec-from-file=list --interactive 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git add --pathspec-from-file=list --patch 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git add --pathspec-from-file=list --edit 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --edit" err &&
+
+ test_must_fail git add --pathspec-from-file=list -- fileA.t 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git add --pathspec-file-nul 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ # This case succeeds, but still prints to stderr
+ git add --pathspec-from-file=empty_list 2>err &&
+ test_i18ngrep -e "Nothing specified, nothing added." err
+'
+
test_done
diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh
index 6b1a731fff..975a9a930a 100755
--- a/t/t7107-reset-pathspec-file.sh
+++ b/t/t7107-reset-pathspec-file.sh
@@ -128,15 +128,6 @@ test_expect_success 'quotes not compatible with --pathspec-file-nul' '
test_must_fail verify_expect
'
-test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
- restore_checkpoint &&
-
- git rm fileA.t &&
- echo fileA.t >list &&
- test_must_fail git reset --soft --pathspec-from-file=list &&
- test_must_fail git reset --hard --pathspec-from-file=list
-'
-
test_expect_success 'only touches what was listed' '
restore_checkpoint &&
@@ -152,4 +143,25 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
+test_expect_success 'error conditions' '
+ restore_checkpoint &&
+ echo fileA.t >list &&
+ git rm fileA.t &&
+
+ test_must_fail git reset --pathspec-from-file=list --patch 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git reset --pathspec-from-file=list -- fileA.t 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git reset --pathspec-file-nul 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ test_must_fail git reset --soft --pathspec-from-file=list 2>err &&
+ test_i18ngrep -e "fatal: Cannot do soft reset with paths" err &&
+
+ test_must_fail git reset --hard --pathspec-from-file=list 2>err &&
+ test_i18ngrep -e "fatal: Cannot do hard reset with paths" err
+'
+
test_done
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
index 4b58901ed6..336197449f 100755
--- a/t/t7526-commit-pathspec-file.sh
+++ b/t/t7526-commit-pathspec-file.sh
@@ -127,10 +127,31 @@ test_expect_success 'only touches what was listed' '
verify_expect
'
-test_expect_success '--pathspec-from-file and --all cannot be used together' '
+test_expect_success 'error conditions' '
restore_checkpoint &&
- test_must_fail git commit --pathspec-from-file=- --all -m "Commit" 2>err &&
- test_i18ngrep "[-]-pathspec-from-file with -a does not make sense" err
+ echo fileA.t >list &&
+ >empty_list &&
+
+ test_must_fail git commit --pathspec-from-file=list --interactive -m "Commit" 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git commit --pathspec-from-file=list --patch -m "Commit" 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git commit --pathspec-from-file=list --all -m "Commit" 2>err &&
+ test_i18ngrep -e "--pathspec-from-file with -a does not make sense" err &&
+
+ test_must_fail git commit --pathspec-from-file=list -m "Commit" -- fileA.t 2>err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git commit --pathspec-file-nul -m "Commit" 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ test_must_fail git commit --pathspec-from-file=empty_list --include -m "Commit" 2>err &&
+ test_i18ngrep -e "No paths with --include/--only does not make sense." err &&
+
+ test_must_fail git commit --pathspec-from-file=empty_list --only -m "Commit" 2>err &&
+ test_i18ngrep -e "No paths with --include/--only does not make sense." err
'
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/1] t: add tests for error conditions with --pathspec-from-file
From: Alexandr Miloslavskiy via GitGitGadget @ 2019-12-30 15:38 UTC (permalink / raw)
To: git; +Cc: Phillip Wood, Alexandr Miloslavskiy, Junio C Hamano
This patch adds tests for various cases where using `--pathspec-from-file` would result in a git error, such as using incompatible options.
This branch is a follow-up for [1] where part of branch was merged into `master` via [2].
The idea for these tests is from [3] where another error condition was added together with a test.
[1] https://lore.kernel.org/git/pull.490.git.1576161385.gitgitgadget@gmail.com/
[2] https://public-inbox.org/git/pull.445.v4.git.1575381738.gitgitgadget@gmail.com/
[3] https://public-inbox.org/git/4401823b-8039-99b4-2436-ed2f1a571d78@gmail.com/
Alexandr Miloslavskiy (1):
t: add tests for error conditions with --pathspec-from-file
t/t2026-checkout-pathspec-file.sh | 17 +++++++++++++++++
t/t2072-restore-pathspec-file.sh | 18 ++++++++++++++++++
t/t3704-add-pathspec-file.sh | 25 +++++++++++++++++++++++++
t/t7107-reset-pathspec-file.sh | 30 +++++++++++++++++++++---------
t/t7526-commit-pathspec-file.sh | 27 ++++++++++++++++++++++++---
5 files changed, 105 insertions(+), 12 deletions(-)
base-commit: 0a76bd7381ec0dbb7c43776eb6d1ac906bca29e6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-502%2FSyntevoAlex%2F%230207(git)_2a_test_error_conditions-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-502/SyntevoAlex/#0207(git)_2a_test_error_conditions-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/502
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 2/2] sparse-checkout: document interactions with submodules
From: Derrick Stolee via GitGitGadget @ 2019-12-30 15:33 UTC (permalink / raw)
To: git; +Cc: szeder.dev, newren, jon, Derrick Stolee, Junio C Hamano,
Derrick Stolee
In-Reply-To: <pull.500.v3.git.1577719993.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
Using 'git submodule (init|deinit)' a user can select a subset of
submodules to populate. This behaves very similar to the sparse-checkout
feature, but those directories contain their own .git directory
including an object database and ref space. To have the sparse-checkout
file also determine if those files should exist would easily cause
problems. Therefore, keeping these features independent in this way
is the best way forward.
Also create a test that demonstrates this behavior to make sure
it doesn't change as the sparse-checkout feature evolves.
Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-sparse-checkout.txt | 10 ++++++++++
t/t1091-sparse-checkout-builtin.sh | 28 +++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 67be5247b9..3b341cf0fc 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -164,6 +164,16 @@ case-insensitive check. This corrects for case mismatched filenames in the
'git sparse-checkout set' command to reflect the expected cone in the working
directory.
+
+SUBMODULES
+----------
+
+If your repository contains one or more submodules, then those submodules will
+appear based on which you initialized with the `git submodule` command. If
+your sparse-checkout patterns exclude an initialized submodule, then that
+submodule will still appear in your working directory.
+
+
SEE ALSO
--------
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 37f6d8fa90..ff7f8f7a1f 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -340,4 +340,32 @@ test_expect_success 'cone mode: set with core.ignoreCase=true' '
test_cmp expect dir
'
+test_expect_success 'interaction with submodules' '
+ git clone repo super &&
+ (
+ cd super &&
+ mkdir modules &&
+ git submodule add ../repo modules/child &&
+ git add . &&
+ git commit -m "add submodule" &&
+ git sparse-checkout init --cone &&
+ git sparse-checkout set folder1
+ ) &&
+ list_files super >dir &&
+ cat >expect <<-\EOF &&
+ a
+ folder1
+ modules
+ EOF
+ test_cmp expect dir &&
+ list_files super/modules/child >dir &&
+ cat >expect <<-\EOF &&
+ a
+ deep
+ folder1
+ folder2
+ EOF
+ test_cmp expect dir
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/2] sparse-checkout: list directories in cone mode
From: Derrick Stolee via GitGitGadget @ 2019-12-30 15:33 UTC (permalink / raw)
To: git; +Cc: szeder.dev, newren, jon, Derrick Stolee, Junio C Hamano,
Derrick Stolee
In-Reply-To: <pull.500.v3.git.1577719993.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>
When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
command takes a list of directories as input, then creates an ordered
list of sparse-checkout patterns such that those directories are
recursively included and all sibling entries along the parent directories
are also included. Listing the patterns is less user-friendly than the
directories themselves.
In cone mode, and as long as the patterns match the expected cone-mode
pattern types, change the output of 'git sparse-checkout list' to only
show the directories that created the patterns.
With this change, the following piped commands would not change the
working directory:
git sparse-checkout list | git sparse-checkout set --stdin
The only time this would not work is if core.sparseCheckoutCone is
true, but the sparse-checkout file contains patterns that do not
match the expected pattern types for cone mode.
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
Documentation/git-sparse-checkout.txt | 11 ++++++++++-
builtin/sparse-checkout.c | 21 +++++++++++++++++++++
t/t1091-sparse-checkout-builtin.sh | 11 +++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-sparse-checkout.txt b/Documentation/git-sparse-checkout.txt
index 9c3c66cc37..67be5247b9 100644
--- a/Documentation/git-sparse-checkout.txt
+++ b/Documentation/git-sparse-checkout.txt
@@ -28,7 +28,7 @@ THE FUTURE.
COMMANDS
--------
'list'::
- Provide a list of the contents in the sparse-checkout file.
+ Describe the patterns in the sparse-checkout file.
'init'::
Enable the `core.sparseCheckout` setting. If the
@@ -150,6 +150,15 @@ expecting patterns of these types. Git will warn if the patterns do not match.
If the patterns do match the expected format, then Git will use faster hash-
based algorithms to compute inclusion in the sparse-checkout.
+In the cone mode case, the `git sparse-checkout list` subcommand will list the
+directories that define the recursive patterns. For the example sparse-checkout
+file above, the output is as follows:
+
+--------------------------
+$ git sparse-checkout list
+A/B/C
+--------------------------
+
If `core.ignoreCase=true`, then the pattern-matching algorithm will use a
case-insensitive check. This corrects for case mismatched filenames in the
'git sparse-checkout set' command to reflect the expected cone in the working
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 5d62f7a66d..b3bed891cb 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -53,6 +53,8 @@ static int sparse_checkout_list(int argc, const char **argv)
memset(&pl, 0, sizeof(pl));
+ pl.use_cone_patterns = core_sparse_checkout_cone;
+
sparse_filename = get_sparse_checkout_filename();
res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL);
free(sparse_filename);
@@ -62,6 +64,25 @@ static int sparse_checkout_list(int argc, const char **argv)
return 0;
}
+ if (pl.use_cone_patterns) {
+ int i;
+ struct pattern_entry *pe;
+ struct hashmap_iter iter;
+ struct string_list sl = STRING_LIST_INIT_DUP;
+
+ hashmap_for_each_entry(&pl.recursive_hashmap, &iter, pe, ent) {
+ /* pe->pattern starts with "/", skip it */
+ string_list_insert(&sl, pe->pattern + 1);
+ }
+
+ string_list_sort(&sl);
+
+ for (i = 0; i < sl.nr; i++)
+ printf("%s\n", sl.items[i].string);
+
+ return 0;
+ }
+
write_patterns_to_file(stdout, &pl);
clear_pattern_list(&pl);
diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
index 6f7e2d0c9e..37f6d8fa90 100755
--- a/t/t1091-sparse-checkout-builtin.sh
+++ b/t/t1091-sparse-checkout-builtin.sh
@@ -246,6 +246,17 @@ test_expect_success 'cone mode: init and set' '
test_cmp expect dir
'
+test_expect_success 'cone mode: list' '
+ cat >expect <<-EOF &&
+ folder1
+ folder2
+ EOF
+ git -C repo sparse-checkout set --stdin <expect &&
+ git -C repo sparse-checkout list >actual 2>err &&
+ test_must_be_empty err &&
+ test_cmp expect actual
+'
+
test_expect_success 'cone mode: set with nested folders' '
git -C repo sparse-checkout set deep deep/deeper1/deepest 2>err &&
test_line_count = 0 err &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] sparse-checkout: list directories in cone mode
From: Derrick Stolee via GitGitGadget @ 2019-12-30 15:33 UTC (permalink / raw)
To: git; +Cc: szeder.dev, newren, jon, Derrick Stolee, Junio C Hamano
In-Reply-To: <pull.500.v2.git.1577472469.gitgitgadget@gmail.com>
When in cone mode, "git sparse-checkout set" takes a list of folders and
constructs an ordered list of patterns for the sparse-checkout file. The
"git sparse-checkout list" subcommand outputs the contents of the
sparse-checkout file in a very basic way.
This patch changes the behavior of "git sparse-checkout list" when
core.sparseCheckoutCone=true. It will output the folders that were used in
"git sparse-checkout set" to create the patterns, instead of the patterns
themselves.
I believe this was requested in the initial review, but I cannot find that
message now.
I was going to include this as part of a longer follow-up series, but I
think this may be worth considering for the 2.25.0 release. Hence, it is
included by itself.
Update in V2:
* Fixed typos/word choice in commit message.
* Added a second commit including clarification on interactions with
submodules.
Thanks, -Stolee
Derrick Stolee (2):
sparse-checkout: list directories in cone mode
sparse-checkout: document interactions with submodules
Documentation/git-sparse-checkout.txt | 21 ++++++++++++++-
builtin/sparse-checkout.c | 21 +++++++++++++++
t/t1091-sparse-checkout-builtin.sh | 39 +++++++++++++++++++++++++++
3 files changed, 80 insertions(+), 1 deletion(-)
base-commit: 761e3d26bbe44c51f83c4f1ad198461f57029ebd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-500%2Fderrickstolee%2Fsparse-checkout-list-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-500/derrickstolee/sparse-checkout-list-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/500
Range-diff vs v2:
1: d6f4f40486 ! 1: 7d4295bd06 sparse-checkout: list folders in cone mode
@@ -1,17 +1,17 @@
Author: Derrick Stolee <dstolee@microsoft.com>
- sparse-checkout: list folders in cone mode
+ sparse-checkout: list directories in cone mode
When core.sparseCheckoutCone is enabled, the 'git sparse-checkout set'
- command takes a list of folders as input, then creates an ordered
- list of sparse-checkout patterns such that those folders are
- recursively included and all sibling entries along the parent folders
+ command takes a list of directories as input, then creates an ordered
+ list of sparse-checkout patterns such that those directories are
+ recursively included and all sibling entries along the parent directories
are also included. Listing the patterns is less user-friendly than the
- folders themselves.
+ directories themselves.
In cone mode, and as long as the patterns match the expected cone-mode
pattern types, change the output of 'git sparse-checkout list' to only
- show the folders that created the patterns.
+ show the directories that created the patterns.
With this change, the following piped commands would not change the
working directory:
@@ -41,8 +41,8 @@
based algorithms to compute inclusion in the sparse-checkout.
+In the cone mode case, the `git sparse-checkout list` subcommand will list the
-+folders that define the recursive patterns. For the example sparse-checkout file
-+above, the output is as follows:
++directories that define the recursive patterns. For the example sparse-checkout
++file above, the output is as follows:
+
+--------------------------
+$ git sparse-checkout list
2: 331bb7d6fb ! 2: 74bbd0f84d sparse-checkout: document interactions with submodules
@@ -2,11 +2,6 @@
sparse-checkout: document interactions with submodules
- Junio asked what the behavior is between the sparse-checkout feature
- and the submodule feature. The sparse-checkout builtin has not changed
- the way these features interact, but we may as well document it in
- the builtin docs.
-
Using 'git submodule (init|deinit)' a user can select a subset of
submodules to populate. This behaves very similar to the sparse-checkout
feature, but those directories contain their own .git directory
@@ -61,14 +56,14 @@
+ git sparse-checkout set folder1
+ ) &&
+ list_files super >dir &&
-+ cat >expect <<-EOF &&
++ cat >expect <<-\EOF &&
+ a
+ folder1
+ modules
+ EOF
+ test_cmp expect dir &&
+ list_files super/modules/child >dir &&
-+ cat >expect <<-EOF &&
++ cat >expect <<-\EOF &&
+ a
+ deep
+ folder1
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 1/3] commit-graph: examine changed-path objects in pack order
From: Derrick Stolee @ 2019-12-30 14:51 UTC (permalink / raw)
To: Jeff King
Cc: Garima Singh via GitGitGadget, git, szeder.dev, jonathantanmy,
jeffhost, me, Junio C Hamano
In-Reply-To: <b9bd0c2e-63fc-5658-7a24-b8ab078acd44@gmail.com>
On 12/30/2019 9:37 AM, Derrick Stolee wrote:
> On the Linux kernel repository, this change reduced the computation
> time for 'git commit-graph write --reachable --changed-paths' from
> 6m30s to 4m50s.
I apologize, these numbers are based on the AzureDevOps repo, not the
Linux kernel repo. After re-running with the Linux kernel repo my
times improve from 3m00s to 1m37s.
-Stolee
^ permalink raw reply
* Re: git filters don't get applied to dotfiles
From: Adrien LEMAIRE @ 2019-12-30 14:42 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: git
In-Reply-To: <ea322b4c06dce0332ead3521e45514d10f2a76b8.camel@kaarsemaker.net>
Hi Dennis, and thanks for looking into this.
I cannot reproduce this issue anymore, and it works as expected:
$ GIT_TRACE=2 config add .mailrc
23:31:05.135580 git.c:439 trace: built-in: git add .mailrc
23:31:05.135902 run-command.c:663 trace: run_command: 'sed -e
'\''s/gmail.com:.*@smtp/gmail.com:PASSWORD@smtp/'\'''
$ config diff --cached
diff --git a/.mailrc b/.mailrc
new file mode 100644
index 0000000..2698128
--- /dev/null
+++ b/.mailrc
@@ -0,0 +1,4 @@
+account gmail {
+ set v15-compat
+ set mta=smtp://lemaire.adrien%40gmail.com:PASSWORD@smtp.gmail.com:587
smtp-use-starttls
+}
To answer your question, yes I first added the file without a filter.
But I'm pretty sure I did a `config restore --staged .mailrc` after
creating the filter (and I actually repeated the operation several
times before contacting you the other day), but I must have been wrong
about that.
I didn't know about the GIT_TRACE environment variable. Thank you for
teaching me something, and sorry about the false bug report.
Best regards
Adrien
On Mon, Dec 30, 2019 at 1:02 AM Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
>
> On Fri, 2019-12-27 at 16:51 +0900, Adrien LEMAIRE wrote:
> > I'd like to report a bug regarding git filters not being applied to
> > files beginning with a dot character "."
> > Using git version 2.24.1
> > Please let me know if there is a better way to report bugs. The github
> > page only mentions this email.
>
> <snip reproduction recipe>
>
> I was not able to reproduce this in the git test suite with a quick
> patch (see below). Your output does not show any git add command, is it
> possible that you added the changes before configuring the filter?
>
> If you set GIT_TRACE=2 in your environment before doing the git add of
> the .mailrc file, you should see it run the filter command. It should
> look something like:
>
> + git add test test.t test.i .mailrc
> trace: built-in: git add test test.t test.i .mailrc
> trace: run_command: ./rot13.sh
> trace: run_command: ./rot13.sh
>
> (which is a part of the output of GIT_TRACE=2 ./t0021-conversion.sh -x
> -v -i)
>
>
> diff --git t/t0021-conversion.sh t/t0021-conversion.sh
> index 6c6d77b51a..32c27d513b 100755
> --- t/t0021-conversion.sh
> +++ t/t0021-conversion.sh
> @@ -77,6 +77,7 @@ test_expect_success setup '
>
> {
> echo "*.t filter=rot13"
> + echo ".mailrc filter=rot13"
> echo "*.i ident"
> } >.gitattributes &&
>
> @@ -88,9 +89,10 @@ test_expect_success setup '
> cat test >test.t &&
> cat test >test.o &&
> cat test >test.i &&
> - git add test test.t test.i &&
> + cat test >.mailrc &&
> + git add test test.t test.i .mailrc &&
> rm -f test test.t test.i &&
> - git checkout -- test test.t test.i &&
> + git checkout -- test test.t test.i .mailrc &&
>
> echo "content-test2" >test2.o &&
> echo "content-test3 - filename with special characters" >"test3 '\''sq'\'',\$x=.o"
> @@ -102,6 +104,7 @@ test_expect_success check '
>
> test_cmp test.o test &&
> test_cmp test.o test.t &&
> + test_cmp test.o .mailrc &&
>
> # ident should be stripped in the repository
> git diff --raw --exit-code :test :test.i &&
> @@ -110,9 +113,12 @@ test_expect_success check '
> test "z$id" = "z$embedded" &&
>
> git cat-file blob :test.t >test.r &&
> + git cat-file blob :.mailrc >.mailrc.r &&
>
> ./rot13.sh <test.o >test.t &&
> - test_cmp test.r test.t
> + ./rot13.sh <test.o >.mailrc &&
> + test_cmp test.r test.t &&
> + test_cmp .mailrc.r .mailrc
> '
>
> # If an expanded ident ever gets into the repository, we want to make sure that
>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox