* Re: [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
From: Yaroslav O Halchenko @ 2018-12-13 22:43 UTC (permalink / raw)
To: git
In-Reply-To: <CAGZ79kZb28bCvM7cOYHC4cpJWpA-3_gcbxS_g-rG0yy=9jXquw@mail.gmail.com>
On Thu, 13 Dec 2018, Stefan Beller wrote:
> > and kaboom -- we have a new test. If we decide to test more -- just tune up
> > test_expect_unchanged_submodule_status and done -- all the tests remain
> > sufficiently prescribed.
> > What do you think?
> That is pretty cool. Maybe my gut reaction on the previous patch
> also had to do with the numbers, i.e. having 2 extra function for
> only having 2 tests more legible. A framework is definitely better
> once we have more tests.
cool, thanks for the feedback - I will then try to make it happen
quick one (so when I get to it I know): should I replicate all those
tests you have for other update strategies? (treating of config
specifications etc) There is no easy way to parametrize them somehow?
;) In Python world I might have mocked the actual underlying call to
update, to see what option it would be getting and assure that it is the
one I specified via config, and then sweepped through all of them
to make sure nothing interim changes it. Just wondering if may be
something like that exists in git's tests support.
BTW - sorry if RTFM and unrelated, is there a way to
update --merge
but allowing only fast-forwards? My use case is collection of this
submodules: http://datasets.datalad.org/?dir=/openneuro which all
should come from github and I should not have any changes of my own.
Sure thing if all is clean etc, merge should result in fast-forward. I
just do not want to miss a case where there was some (temporary?) "dirt"
which I forgot to reset and it would then get merged etc.
--
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* Re: [PATCH v3 1/4] pack-protocol.txt: accept error packets in any context
From: Josh Steadmon @ 2018-12-13 22:18 UTC (permalink / raw)
To: Masaya Suzuki; +Cc: peff, git, Junio C Hamano
In-Reply-To: <CAJB1erXRqQW0yQyZutJAJKC7WbdVhBAYUMWM+8ZutxA-W-7S8w@mail.gmail.com>
On 2018.12.12 17:17, Masaya Suzuki wrote:
> On Wed, Dec 12, 2018 at 3:02 AM Jeff King <peff@peff.net> wrote:
> > This ERR handling has been moved to a very low level. What happens if
> > we're passing arbitrary data via the packet_read() code? Could we
> > erroneously trigger an error if a packfile happens to have the bytes
> > "ERR " at a packet boundary?
> >
> > For packfiles via upload-pack, I _think_ we're OK, because we only
> > packetize it when a sideband is in use. In which case this would never
> > match, because we'd have "\1" in the first byte slot.
> >
> > But are there are other cases we need to worry about? Just
> > brainstorming, I can think of:
> >
> > 1. We also pass packetized packfiles between git-remote-https and
> > the stateless-rpc mode of fetch-pack/send-pack. And I don't think
> > we use sidebands there.
> >
> > 2. The packet code is used for long-lived clean/smudge filters these
> > days, which also pass arbitrary data.
> >
> > So I think it's probably not a good idea to unconditionally have callers
> > of packet_read_with_status() handle this. We'd need a flag like
> > PACKET_READ_RESPECT_ERR, and to trigger it from the appropriate callers.
>
> This is outside of the Git pack protocol so having a separate parsing
> mode makes sense to me.
This sounds like it could be a significant refactoring. Should we go
back to V2 of this series, and then work on the new parsing mode
separately?
^ permalink raw reply
* Re: [wishlist] support of cloning recursively from non-bare submodule hierarchies?
From: Stefan Beller @ 2018-12-13 21:59 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git
In-Reply-To: <20181213171917.GC4633@hopa.kiewit.dartmouth.edu>
On Thu, Dec 13, 2018 at 9:19 AM Yaroslav Halchenko <yoh@onerussian.com> wrote:
>
> Example - on http://datasets.datalad.org we have a few hundred datasets
> organized into a hierarchy as git submodules. Each git submodules carries its
> own .git/ directory so they are "self sufficient" and we could readily assess
> their sizes, and "cut the tree" at any level without looking for the
> supermodule somewhere high up in the tree.
>
> .gitmodules typically has relative paths for the url and path for the
> submodules there, the form which I think we chose because it used to work (I
> could be utterly wrong! but I think it was done in an informed fashion)
> for git clone --recursive:
>
> $> curl http://datasets.datalad.org/labs/gobbini/famface/.gitmodules
> [submodule "data"]
> path = data
> url = ./data
>
> and possibly outside:
>
> $> curl http://datasets.datalad.org/labs/gobbini/famface/data/.gitmodules
> [submodule "scripts/mridefacer"]
> path = scripts/mridefacer
> url = https://github.com/yarikoptic/mridefacer
So far so good.
> But unfortunately git doesn't even consider such (valid AFAIK) situation
> while cloning where url has to have .git suffix but repository is not bare and
> a relative "data" path (or "./data" url) is referring to the worktree.
>
> $> git clone --recursive http://datasets.datalad.org/labs/gobbini/famface/.git
[..]
> Submodule 'data' (http://datasets.datalad.org/labs/gobbini/famface/.git/data) registered for path 'data'
and here it goes wrong, and you would have expected to see
.../gobbini/famface/data, eliding the .git ?
I just checked and this did not work neither in v2.18.0 nor v2.0.0 of
Git, so it is
either a real old regression in submodules, or something else.
Is it possible that the clone worked once without the additional .git
in the superproject URL?
> on the server I use the "smart HTTP" git backend, but not sure if that is the one to blame, since
> I do not see in the logs any attempt to get the /data from not under .git/:
If we want to strip off "/.git" of urls to make submodules work,
we'd want to look at builtin/submodule--helper.c::compute_submodule_clone_url
that was recently introduced.
I wonder if we'd just want to cut off the "/.git" and assume the submodule
is there in the worktree. Or if we need to see if the submodule was
absorbed into .git/modules/<name> on the remote side. (But if the
submodule is checked out both would work)
^ permalink raw reply
* [PATCH 4/4] docs/pretty-formats: add explanation + copy edits
From: John Passaro @ 2018-12-13 21:22 UTC (permalink / raw)
To: git; +Cc: gitster, alex.crezoff, peff, mgorny, John Passaro
In-Reply-To: <20181213212256.48122-1-john.a.passaro@gmail.com>
Clarify description of %G? = "U" to say it can mean good signature but
untrusted key.
Make wording consistent between %G* placeholders and other placeholders
by removing the verb "show".
Signed-off-by: John Passaro <john.a.passaro@gmail.com>
---
Documentation/pretty-formats.txt | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 4a83796250..32c2f75060 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -147,18 +147,19 @@ endif::git-rev-list[]
- '%GG': raw verification message from GPG for a signed commit.
This and all the other %G* placeholders, other than %GR, %G+, and
%G?, return blank if GPG cannot be run.
-- '%G?': show "G" for a good (valid) signature,
+- '%G?': "G" for a good (valid) signature,
"B" for a bad signature,
- "U" for a good signature with unknown validity,
+ "U" for a good signature with unknown validity (e.g. key is known but
+ not trusted),
"X" for a good signature that has expired,
"Y" for a good signature made by an expired key,
"R" for a good signature made by a revoked key,
"E" if the signature cannot be checked (e.g. missing key)
and "N" for no signature (e.g. unsigned, or GPG cannot be run)
-- '%GS': show the name of the signer for a signed commit
-- '%GK': show the key used to sign a signed commit
-- '%GF': show the fingerprint of the key used to sign a signed commit
-- '%GP': show the fingerprint of the primary key whose subkey was used
+- '%GS': name of the signer for a signed commit
+- '%GK': key used to sign a signed commit
+- '%GF': fingerprint of the key used to sign a signed commit
+- '%GP': fingerprint of the primary key whose subkey was used
to sign a signed commit
- '%gD': reflog selector, e.g., `refs/stash@{1}` or
`refs/stash@{2 minutes ago`}; the format follows the rules described
--
2.19.1
^ permalink raw reply related
* [PATCH 3/4] doc, tests: pretty behavior when gpg missing
From: John Passaro @ 2018-12-13 21:22 UTC (permalink / raw)
To: git; +Cc: gitster, alex.crezoff, peff, mgorny, John Passaro
In-Reply-To: <20181213212256.48122-1-john.a.passaro@gmail.com>
Test that when GPG cannot be run, new placeholders %GR and %G+ are
unaffected, %G? always returns 'N', and other GPG-related placeholders
return blank.
As of e5a329a279 ("run-command: report exec failure" 2018-12-11), if GPG
cannot be run and placeholders requiring GPG are given, git complains to
stderr that GPG cannot be found. That commit included low-level tests
for this behavior. Now, test it also at the level of everyday user
commands.
Signed-off-by: John Passaro <john.a.passaro@gmail.com>
---
Documentation/pretty-formats.txt | 6 +-
t/t7510-signed-commit.sh | 95 ++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 582454a4f7..4a83796250 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -144,7 +144,9 @@ ifndef::git-rev-list[]
endif::git-rev-list[]
- '%GR': contents of the commits signature (blank when unsigned)
- '%G+': "Y" if the commit is signed, "N" otherwise
-- '%GG': raw verification message from GPG for a signed commit
+- '%GG': raw verification message from GPG for a signed commit.
+ This and all the other %G* placeholders, other than %GR, %G+, and
+ %G?, return blank if GPG cannot be run.
- '%G?': show "G" for a good (valid) signature,
"B" for a bad signature,
"U" for a good signature with unknown validity,
@@ -152,7 +154,7 @@ endif::git-rev-list[]
"Y" for a good signature made by an expired key,
"R" for a good signature made by a revoked key,
"E" if the signature cannot be checked (e.g. missing key)
- and "N" for no signature
+ and "N" for no signature (e.g. unsigned, or GPG cannot be run)
- '%GS': show the name of the signer for a signed commit
- '%GK': show the key used to sign a signed commit
- '%GF': show the fingerprint of the key used to sign a signed commit
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index aff6b1eb3a..d65425eddc 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -170,6 +170,48 @@ test_expect_success GPG 'amending already signed commit' '
! grep "BAD signature from" actual
'
+test_expect_success GPG 'show custom format fields for signed commit if gpg is missing' '
+ cat >expect <<-\EOF &&
+ N
+
+
+
+
+ Y
+ EOF
+ test_config gpg.program this-is-not-a-program &&
+ git log -n1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" sixth-signed >actual 2>/dev/null &&
+ test_cmp expect actual
+'
+
+test_expect_success GPG 'show custom format fields for unsigned commit if gpg is missing' '
+ cat >expect <<-\EOF &&
+ N
+
+
+
+
+ N
+ EOF
+ test_config gpg.program this-is-not-a-program &&
+ git log -n1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" seventh-unsigned >actual 2>/dev/null &&
+ test_cmp expect actual
+'
+
+test_expect_success GPG 'show error for custom format fields on signed commit if gpg is missing' '
+ test_config gpg.program this-is-not-a-program &&
+ git log -n1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" sixth-signed >/dev/null 2>errors &&
+ test $(wc -l <errors) = 1 &&
+ test_i18ngrep "^error: " errors &&
+ grep this-is-not-a-program errors
+'
+
+test_expect_success GPG 'do not run gpg at all for unsigned commit' '
+ test_config gpg.program this-is-not-a-program &&
+ git log -n1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" seventh-unsigned >/dev/null 2>errors &&
+ test_must_be_empty errors
+'
+
test_expect_success GPG 'show good signature with custom format' '
cat >expect <<-\EOF &&
G
@@ -240,6 +282,14 @@ test_expect_success GPG 'show lack of raw signature with custom format' '
test_must_be_empty actual
'
+test_expect_success GPG 'show lack of raw signature with custom format without running GPG' '
+ echo N > expected &&
+ test_config gpg.program this-is-not-a-program &&
+ git log -1 --format="%G+%GR" seventh-unsigned >actual 2>errors &&
+ test_cmp expected actual &&
+ test_must_be_empty errors
+'
+
test_expect_success GPG 'show raw signature with custom format' '
git log -1 --format=format:"%GR" sixth-signed >output &&
cat output &&
@@ -250,6 +300,51 @@ test_expect_success GPG 'show raw signature with custom format' '
tail -n1 output | grep -q "^---*END PGP SIGNATURE---*$"
'
+test_expect_success GPG 'show raw signature with custom format without running GPG' '
+ test_config gpg.program this-is-not-a-program &&
+ git log -1 --format=format:"%GR" sixth-signed >rawsig 2>errors &&
+ cat rawsig &&
+ head -n1 rawsig | grep -q "^---*BEGIN PGP SIGNATURE---*$" &&
+ sed 1d rawsig | grep -q "^$" &&
+ sed "1,/^$/d" rawsig | grep -q "^[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$" &&
+ tail -n2 rawsig | head -n1 | grep -q "^=[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$" &&
+ tail -n1 rawsig | grep -q "^---*END PGP SIGNATURE---*$" &&
+ test_must_be_empty errors
+'
+
+test_expect_success GPG 'show presence of gpgsig with custom format when gpg is missing without errors' '
+ echo Y > expected &&
+ git log -1 --format=%G+ sixth-signed >output 2>errors &&
+ test_cmp expected output &&
+ test_must_be_empty errors
+'
+
+test_expect_success GPG 'show presence of invalid gpgsig header' '
+ printf gpgsig >gpgsig-header &&
+ tee prank-signature <<-\EOF | sed "s/^/ /" >>gpgsig-header &&
+ this is not a signature but an awful...
+ 888
+ 888
+ 888
+ 88888b. 888d888 8888b. 88888b. 888 888
+ 888 "88b 888P" "88b 888 "88b 888 .88P
+ 888 888 888 .d888888 888 888 888888K
+ 888 d88P 888 888 888 888 888 888 "88b
+ 88888P" 888 "Y888888 888 888 888 888
+ 888
+ 888
+ 888
+ EOF
+ git cat-file commit seventh-unsigned >bare-commit-data &&
+ sed "/^committer/r gpgsig-header" bare-commit-data >commit-data &&
+ git hash-object -w -t commit commit-data >commit &&
+ echo Y >expected &&
+ cat prank-signature >>expected &&
+ git log -n1 --format=format:%G+%n%GR $(cat commit) >actual 2>errors &&
+ test_cmp expected actual &&
+ test_must_be_empty errors
+'
+
test_expect_success GPG 'log.showsignature behaves like --show-signature' '
test_config log.showsignature true &&
git show initial >actual &&
--
2.19.1
^ permalink raw reply related
* [PATCH 2/4] t/t7510-signed-commit.sh: test new placeholders
From: John Passaro @ 2018-12-13 21:22 UTC (permalink / raw)
To: git; +Cc: gitster, alex.crezoff, peff, mgorny, John Passaro
In-Reply-To: <20181213212256.48122-1-john.a.passaro@gmail.com>
Test that %GR output ("Raw" contents of "gpgsig" header) looks like
ASCII-armored GPG signature.
Test %G+ (Y/N for presence/absence of "gpgsig" header) by adding it to
existing format tests for signed commits.
Signed-off-by: John Passaro <john.a.passaro@gmail.com>
---
t/t7510-signed-commit.sh | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 86d3f93fa2..aff6b1eb3a 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -177,8 +177,9 @@ test_expect_success GPG 'show good signature with custom format' '
C O Mitter <committer@example.com>
73D758744BE721698EC54E8713B6F51ECDDE430D
73D758744BE721698EC54E8713B6F51ECDDE430D
+ Y
EOF
- git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" sixth-signed >actual &&
+ git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" sixth-signed >actual &&
test_cmp expect actual
'
@@ -189,8 +190,9 @@ test_expect_success GPG 'show bad signature with custom format' '
C O Mitter <committer@example.com>
+ Y
EOF
- git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" $(cat forged1.commit) >actual &&
+ git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" $(cat forged1.commit) >actual &&
test_cmp expect actual
'
@@ -201,8 +203,9 @@ test_expect_success GPG 'show untrusted signature with custom format' '
Eris Discordia <discord@example.net>
F8364A59E07FFE9F4D63005A65A0EEA02E30CAD7
D4BE22311AD3131E5EDA29A461092E85B7227189
+ Y
EOF
- git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" eighth-signed-alt >actual &&
+ git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" eighth-signed-alt >actual &&
test_cmp expect actual
'
@@ -213,8 +216,9 @@ test_expect_success GPG 'show unknown signature with custom format' '
+ Y
EOF
- GNUPGHOME="$GNUPGHOME_NOT_USED" git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" eighth-signed-alt >actual &&
+ GNUPGHOME="$GNUPGHOME_NOT_USED" git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" eighth-signed-alt >actual &&
test_cmp expect actual
'
@@ -225,11 +229,27 @@ test_expect_success GPG 'show lack of signature with custom format' '
+ N
EOF
- git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" seventh-unsigned >actual &&
+ git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP%n%G+" seventh-unsigned >actual &&
test_cmp expect actual
'
+test_expect_success GPG 'show lack of raw signature with custom format' '
+ git log -1 --format=format:"%GR" seventh-unsigned > actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success GPG 'show raw signature with custom format' '
+ git log -1 --format=format:"%GR" sixth-signed >output &&
+ cat output &&
+ head -n1 output | grep -q "^---*BEGIN PGP SIGNATURE---*$" &&
+ sed 1d output | grep -q "^$" &&
+ sed "1,/^$/d" output | grep -q "^[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$" &&
+ tail -n2 output | head -n1 | grep -q "^=[a-zA-Z0-9+/][a-zA-Z0-9+/=]*$" &&
+ tail -n1 output | grep -q "^---*END PGP SIGNATURE---*$"
+'
+
test_expect_success GPG 'log.showsignature behaves like --show-signature' '
test_config log.showsignature true &&
git show initial >actual &&
--
2.19.1
^ permalink raw reply related
* [PATCH 1/4] pretty: expose raw commit signature
From: John Passaro @ 2018-12-13 21:22 UTC (permalink / raw)
To: git; +Cc: gitster, alex.crezoff, peff, mgorny, John Passaro
In-Reply-To: <20181213212256.48122-1-john.a.passaro@gmail.com>
Add new pretty-format placeholders %GR and %G+ to support inspecting
gpgsig commit header in pretty format, even if GPG is not available.
Signed-off-by: John Passaro <john.a.passaro@gmail.com>
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 36 ++++++++++++++++++++++++++++++--
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd8..582454a4f7 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -142,6 +142,8 @@ The placeholders are:
ifndef::git-rev-list[]
- '%N': commit notes
endif::git-rev-list[]
+- '%GR': contents of the commits signature (blank when unsigned)
+- '%G+': "Y" if the commit is signed, "N" otherwise
- '%GG': raw verification message from GPG for a signed commit
- '%G?': show "G" for a good (valid) signature,
"B" for a bad signature,
diff --git a/pretty.c b/pretty.c
index b83a3ecd23..d142b457b5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -768,6 +768,9 @@ struct format_commit_context {
unsigned commit_header_parsed:1;
unsigned commit_message_parsed:1;
struct signature_check signature_check;
+ unsigned signature_checked:2;
+ struct strbuf signature;
+ struct strbuf signature_payload;
enum flush_type flush_type;
enum trunc_type truncate;
const char *message;
@@ -1228,8 +1231,30 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
}
if (placeholder[0] == 'G') {
- if (!c->signature_check.result)
- check_commit_signature(c->commit, &(c->signature_check));
+ if (!c->signature_checked) {
+ parse_signed_commit(c->commit, &(c->signature_payload), &(c->signature));
+ c->signature_checked = 1;
+ }
+ switch (placeholder[1]) {
+ case 'R':
+ strbuf_addbuf(sb, &(c->signature));
+ break;
+ case '+':
+ strbuf_addch(sb, c->signature.len ? 'Y' : 'N');
+ break;
+ default:
+ goto do_signature_check;
+ }
+ return 2;
+
+do_signature_check:
+ if (c->signature_checked < 2) {
+ if (c->signature.len)
+ check_signature(c->signature_payload.buf, c->signature_payload.len,
+ c->signature.buf, c->signature.len,
+ &(c->signature_check));
+ c->signature_checked = 2;
+ }
switch (placeholder[1]) {
case 'G':
if (c->signature_check.gpg_output)
@@ -1246,6 +1271,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
case 'Y':
case 'R':
strbuf_addch(sb, c->signature_check.result);
+ break;
+ case 0: // i.e. no signature so we never ran the check
+ strbuf_addch(sb, 'N');
}
break;
case 'S':
@@ -1527,6 +1555,8 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb->len;
+ strbuf_init(&context.signature, 0);
+ strbuf_init(&context.signature_payload, 0);
/*
* convert a commit message to UTF-8 first
* as far as 'format_commit_item' assumes it in UTF-8
@@ -1556,6 +1586,8 @@ void format_commit_message(const struct commit *commit,
strbuf_attach(sb, out, outsz, outsz + 1);
}
+ strbuf_release(&context.signature);
+ strbuf_release(&context.signature_payload);
free(context.commit_encoding);
unuse_commit_buffer(commit, context.message);
}
--
2.19.1
^ permalink raw reply related
* [PATCH 0/4] Expose gpgsig in pretty-print
From: John Passaro @ 2018-12-13 21:22 UTC (permalink / raw)
To: git; +Cc: gitster, alex.crezoff, peff, mgorny, John Passaro
Currently, users who do not have GPG installed have no way to discern
signed from unsigned commits without examining raw commit data. I
propose two new pretty-print placeholders to expose this information:
%GR: full ("R"aw) contents of gpgsig header
%G+: Y/N if the commit has nonempty gpgsig header or not
The second is of course much more likely to be used, but having exposed
the one, exposing the other too adds almost no complexity.
I'm open to suggestion on the names of these placeholders.
This commit is based on master but e5a329a279 ("run-command: report exec
failure" 2018-12-11) is required for the tests to pass.
One note is that this change touches areas of the pretty-format
documentation that are radically revamped in aw/pretty-trailers: see
42617752d4 ("doc: group pretty-format.txt placeholders descriptions"
2018-12-08). I have another version of this branch based on that branch
as well, so you can use that in case conflicts with aw/pretty-trailers
arise.
See:
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig
- https://github.com/jpassaro/git/tree/jp/pretty-expose-gpgsig--based-on-aw-pretty-trailers
John Passaro (4):
pretty: expose raw commit signature
t/t7510-signed-commit.sh: test new placeholders
doc, tests: pretty behavior when gpg missing
docs/pretty-formats: add explanation + copy edits
Documentation/pretty-formats.txt | 21 ++++--
pretty.c | 36 ++++++++-
t/t7510-signed-commit.sh | 125 +++++++++++++++++++++++++++++--
3 files changed, 167 insertions(+), 15 deletions(-)
base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
prerequisite-patch-id: aedfe228fd293714d9cd0392ac22ff1cba7365db
--
2.19.1
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Mike Rappazzo @ 2018-12-13 21:14 UTC (permalink / raw)
To: Stefan Beller
Cc: Nguyễn Thái Ngọc, gitgitgadget, Git List,
Junio C Hamano
In-Reply-To: <CAGZ79kYnQPhGMStmKSFb5_4Ku-nv54nHwta==jE82ZR4NOPETA@mail.gmail.com>
On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller <sbeller@google.com> wrote:
>
> > > The current situation is definitely a problem. If I am in a worktree,
> > > using "head" should be the same as "HEAD".
>
> By any chance, is your file system case insensitive?
> That is usually the source of confusion for these discussions.
This behavior is the same for MacOS (High Sierra) and Windows 7. I
assume other derivatives of those act the same.
On CentOS "head" is an ambiguous ref. If Windows and Mac resulted in
an ambiguous ref, that would also be OK, but as it is now, they return
the result of "HEAD" on the primary worktree.
>
> Maybe in worktree code we have a spillover between path
> resolution and ref namespace?
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Mike Rappazzo @ 2018-12-13 21:07 UTC (permalink / raw)
To: Nguyễn Thái Ngọc; +Cc: gitgitgadget, Git List, Junio C Hamano
In-Reply-To: <CACsJy8Bj=8xHp3JA8dLbyM=RwJey7utMK6DTVe_0AjBNVHxJyg@mail.gmail.com>
On Thu, Dec 13, 2018 at 3:43 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 9:34 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > >
> > > On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> > > <gitgitgadget@gmail.com> wrote:
> > > >
> > > > From: Michael Rappazzo <rappazzo@gmail.com>
> > > >
> > > > On a worktree which is not the primary, using the symbolic-ref 'head' was
> > > > incorrectly pointing to the main worktree's HEAD. The same was true for
> > > > any other case of the word 'Head'.
> > > >
> > > > Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> > > > ---
> > > > refs.c | 8 ++++----
> > > > t/t1415-worktree-refs.sh | 9 +++++++++
> > > > 2 files changed, 13 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/refs.c b/refs.c
> > > > index f9936355cd..963e786458 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
> > > > *ref = xstrdup(r);
> > > > if (!warn_ambiguous_refs)
> > > > break;
> > > > - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
> > > > + } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {
> > >
> > > This is not going to work. How about ~40 other "strcmp.*HEAD"
> > > instances? All refs are case-sensitive and this probably will not
> > > change even when we introduce new ref backends.
> >
> > The current situation is definitely a problem. If I am in a worktree,
> > using "head" should be the same as "HEAD".
>
> No "head" is not the same as "HEAD". It does not matter if you're in a
> worktree or not.
I was not aware of a difference. Is that spelled out in the docs
somewhere? It seems like a bad idea to have a magical symbolic ref
that _sometimes_ gives you a different answer depending on casing.
What should "head" do in a worktree? Is it supposed to mean the HEAD
of the primary worktree?
>
> > I am not sure if you mean that the fix is too narrow or too wide.
> > Maybe it is only necessary in 'is_per_worktree_ref'. On the other
> > side of the coin, I could change every strcmp to strcasecmp where the
> > comparison is against "HEAD".
>
> If you make "head" work like "HEAD", then it should work for _all_
> commands, not just worktree, and "MASTER" should match
> "refs/heads/master" and so on. I don't think it's as simple as
> changing strcmp to strcasecmp. You would need to make ref management
> case-insensitive (and make sure if still is case-sensitive if
> configured so). I don't think anybody has managed that.
I am all for making "head" work in all cases, not just worktree. I
don't think that this situation applies to non-magical refs
(branches/tags).
> --
> Duy
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Stefan Beller @ 2018-12-13 20:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Mike Rappazzo, gitgitgadget, git, Junio C Hamano
In-Reply-To: <CACsJy8Bj=8xHp3JA8dLbyM=RwJey7utMK6DTVe_0AjBNVHxJyg@mail.gmail.com>
> > The current situation is definitely a problem. If I am in a worktree,
> > using "head" should be the same as "HEAD".
By any chance, is your file system case insensitive?
That is usually the source of confusion for these discussions.
Maybe in worktree code we have a spillover between path
resolution and ref namespace?
^ permalink raw reply
* Re: [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
From: Stefan Beller @ 2018-12-13 20:44 UTC (permalink / raw)
To: debian; +Cc: git
In-Reply-To: <20181213164217.GA4633@hopa.kiewit.dartmouth.edu>
On Thu, Dec 13, 2018 at 8:42 AM Yaroslav O Halchenko
<debian@onerussian.com> wrote:
>
> Thank you Stefan for the review and please pardon my delay with the
> reply, and sorry it got a bit too long by the end ;)
>
> On Wed, 12 Dec 2018, Stefan Beller wrote:
> > Thanks for the patches. The first patch looks good to me!
>
> Great!
>
> > > [PATCH 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
>
> > The subject is a bit cryptic (specifically the first part before the
> > colon), maybe
>
> > t7406: compare entire submodule status for --reset-hard mode
>
> > ?
>
>
> > > For submodule update --reset-hard the best test is comparison of the
> > > entire status as shown by submodule status --recursive. Upon update
> > > --reset-hard we should get back to the original state, with all the
> > > branches being the same (no detached HEAD) and commits identical to
> > > original (so no merges, new commits, etc).
>
> > "original state" can mean different things to different people. I'd think
> > we could be more precise:
>
> > ... we should get to the state that the submodule is reset to the
> > object id as the superprojects gitlink points at, irrespective of the
> > submodule branch.
>
> ok, I will update the description. But I wonder if there could be some
> short term to be used to describe the composite "git submodule status"
> and "git status" (refers to below ;)).
>
> > > test_expect_success 'submodule update --merge staying on master' '
> > > (cd super/submodule &&
> > > - git reset --hard HEAD~1
> > > + git reset --hard HEAD~1
>
> > unrelated white space change?
>
> I was tuning formatting to be uniform and I guess missed that this is in
> the other (not my) test. I will revert that piece, thanks!
The tests in that file are not quite following the coding style that is
currently deemed the best. So if you want to clean that up
as a preparatory patch, feel welcome to do so. :-)
(c.f. t/t7400-submodule-basic.sh for good style, specifically
indentation by tabs and the cd <path> on its own line in
a subshell)
The latest style update I found is
80938c39e2 (pack-objects test: modernize style, 2018-10-30)
and submodule related test style
31158c7efc (t7410: update to new style, 2018-08-15)
So I was not opposed to have style changes, but to have
multiple unrelated things in one patch (feature work vs
cleanup).
> BTW -- should I just squash to PATCHes now? I kept them separate primarily to
> show the use of those helpers:
That would make sense.
> compare_submodules_status is already a compound action, so code would
> become quite more "loaded" if it is expanded, e.g. instead of
...
> it would become something like this I guess?
...
> ! {git submodule status --recursive >actual &&
you could keep the status out of the negation.
> test_i18ncmp expect actual;} &&
> git submodule update --reset-hard submodule &&
> {git submodule status --recursive >actual &&
> test_i18ncmp expect actual;}
> )
>
> IMHO a bit mouth full. I was thinking also to extend compare_ with additional
> testing e.g. using "git status" since "git submodule status" does not care
> about untracked files etc. For --reset-hard I would like to assure that it is
> not just some kind of a mixed reset leaving files behind. That would make
> tests even more overloaded.
ok, that makes sense.
> On that point: Although I also like explicit calls at times, I also do
> like test fixtures as a concept to do more testing around the actual
> test-specific code block, thus minimizing boiler plate, which even if explicit
> makes code actually harder to grasp (at least to me).
>
> Since for the majority of the --reset-hard tests the fixture and test(s) are
> pretty much the same, actually ideally I would have liked to have
> something like this:
>
> test_expect_unchanged_submodule_status 'submodule update --reset-hard staying on master' \
> super \
> '(cd submodule && git reset --hard HEAD~1)' \
> 'git submodule update --reset-hard submodule'
>
> where I just pass
> the path to work in,
> the test setup function,
> and the test action.
>
> The rest (initial cd, record, run setup, verify that there is a change, run
> action, verify there is no changes) is done by the
> test_expect_unchanged_submodule_status in a uniform way, absorbing all the
> boiler plate. (I am not married to the name, could be more descriptive/generic
> may be)
The issue with submodules is that we're already deviating from the
'standard' git test suite at times (See the submodule test suite
lib-submodule-update.sh that is used via t1013, t2013 or t3906
and others).
I guess if we keep the test_expect_unchanged_submodule_status
as a file local function, it could be okay.
> Then we could breed a good number of tests with little to no boiler plate, with
> only relevant pieces and as extended as needed testing done by this
> test_expect_unchanged_submodule_status helper. e.g smth like
>
> test_expect_unchanged_submodule_status 'submodule update --reset-hard staying on master when I do a new commit' \
> super \
> '(cd submodule && git commit --allow-empty -m "new one"' \
In new tests we're a big fan of using -C, as that can save the
subshell, i.e. replace the whole line by
git -C submodule commit --allow-empty -m "new one"' &&
> 'git submodule update --reset-hard submodule'
>
> and kaboom -- we have a new test. If we decide to test more -- just tune up
> test_expect_unchanged_submodule_status and done -- all the tests remain
> sufficiently prescribed.
>
> What do you think?
That is pretty cool. Maybe my gut reaction on the previous patch
also had to do with the numbers, i.e. having 2 extra function for
only having 2 tests more legible. A framework is definitely better
once we have more tests.
Stefan
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Duy Nguyen @ 2018-12-13 20:43 UTC (permalink / raw)
To: Mike Rappazzo; +Cc: gitgitgadget, Git Mailing List, Junio C Hamano
In-Reply-To: <CANoM8SVMYBRg-nL4r=JJDFU_qZ=grzSmRs-B2nLYUnv5kFc00Q@mail.gmail.com>
On Thu, Dec 13, 2018 at 9:34 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> > >
> > > From: Michael Rappazzo <rappazzo@gmail.com>
> > >
> > > On a worktree which is not the primary, using the symbolic-ref 'head' was
> > > incorrectly pointing to the main worktree's HEAD. The same was true for
> > > any other case of the word 'Head'.
> > >
> > > Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> > > ---
> > > refs.c | 8 ++++----
> > > t/t1415-worktree-refs.sh | 9 +++++++++
> > > 2 files changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/refs.c b/refs.c
> > > index f9936355cd..963e786458 100644
> > > --- a/refs.c
> > > +++ b/refs.c
> > > @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
> > > *ref = xstrdup(r);
> > > if (!warn_ambiguous_refs)
> > > break;
> > > - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
> > > + } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {
> >
> > This is not going to work. How about ~40 other "strcmp.*HEAD"
> > instances? All refs are case-sensitive and this probably will not
> > change even when we introduce new ref backends.
>
> The current situation is definitely a problem. If I am in a worktree,
> using "head" should be the same as "HEAD".
No "head" is not the same as "HEAD". It does not matter if you're in a
worktree or not.
> I am not sure if you mean that the fix is too narrow or too wide.
> Maybe it is only necessary in 'is_per_worktree_ref'. On the other
> side of the coin, I could change every strcmp to strcasecmp where the
> comparison is against "HEAD".
If you make "head" work like "HEAD", then it should work for _all_
commands, not just worktree, and "MASTER" should match
"refs/heads/master" and so on. I don't think it's as simple as
changing strcmp to strcasecmp. You would need to make ref management
case-insensitive (and make sure if still is case-sensitive if
configured so). I don't think anybody has managed that.
--
Duy
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Mike Rappazzo @ 2018-12-13 20:34 UTC (permalink / raw)
To: Nguyễn Thái Ngọc; +Cc: gitgitgadget, Git List, Junio C Hamano
In-Reply-To: <CACsJy8AsRT+k4kdwC3gGjDOPiDn-L0GJs7-SQHb88Ra_gt4OcA@mail.gmail.com>
On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Michael Rappazzo <rappazzo@gmail.com>
> >
> > On a worktree which is not the primary, using the symbolic-ref 'head' was
> > incorrectly pointing to the main worktree's HEAD. The same was true for
> > any other case of the word 'Head'.
> >
> > Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> > ---
> > refs.c | 8 ++++----
> > t/t1415-worktree-refs.sh | 9 +++++++++
> > 2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/refs.c b/refs.c
> > index f9936355cd..963e786458 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
> > *ref = xstrdup(r);
> > if (!warn_ambiguous_refs)
> > break;
> > - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
> > + } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {
>
> This is not going to work. How about ~40 other "strcmp.*HEAD"
> instances? All refs are case-sensitive and this probably will not
> change even when we introduce new ref backends.
The current situation is definitely a problem. If I am in a worktree,
using "head" should be the same as "HEAD".
I am not sure if you mean that the fix is too narrow or too wide.
Maybe it is only necessary in 'is_per_worktree_ref'. On the other
side of the coin, I could change every strcmp to strcasecmp where the
comparison is against "HEAD".
>
> > warning(_("ignoring dangling symref %s"), fullref.buf);
> > } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
> > warning(_("ignoring broken ref %s"), fullref.buf);
> --
> Duy
^ permalink raw reply
* Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Duy Nguyen @ 2018-12-13 20:23 UTC (permalink / raw)
To: gitgitgadget; +Cc: Git Mailing List, Junio C Hamano, Mike Rappazzo
In-Reply-To: <13ee60e44f91ca06d182ff50fa4c69e137650fd2.1544730848.git.gitgitgadget@gmail.com>
On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Michael Rappazzo <rappazzo@gmail.com>
>
> On a worktree which is not the primary, using the symbolic-ref 'head' was
> incorrectly pointing to the main worktree's HEAD. The same was true for
> any other case of the word 'Head'.
>
> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
> ---
> refs.c | 8 ++++----
> t/t1415-worktree-refs.sh | 9 +++++++++
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index f9936355cd..963e786458 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
> *ref = xstrdup(r);
> if (!warn_ambiguous_refs)
> break;
> - } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
> + } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {
This is not going to work. How about ~40 other "strcmp.*HEAD"
instances? All refs are case-sensitive and this probably will not
change even when we introduce new ref backends.
> warning(_("ignoring dangling symref %s"), fullref.buf);
> } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
> warning(_("ignoring broken ref %s"), fullref.buf);
--
Duy
^ permalink raw reply
* [PATCH 1/1] worktree refs: fix case sensitivity for 'head'
From: Michael Rappazzo via GitGitGadget @ 2018-12-13 19:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Michael Rappazzo
In-Reply-To: <pull.100.git.gitgitgadget@gmail.com>
From: Michael Rappazzo <rappazzo@gmail.com>
On a worktree which is not the primary, using the symbolic-ref 'head' was
incorrectly pointing to the main worktree's HEAD. The same was true for
any other case of the word 'Head'.
Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
---
refs.c | 8 ++++----
t/t1415-worktree-refs.sh | 9 +++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index f9936355cd..963e786458 100644
--- a/refs.c
+++ b/refs.c
@@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
*ref = xstrdup(r);
if (!warn_ambiguous_refs)
break;
- } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
+ } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {
warning(_("ignoring dangling symref %s"), fullref.buf);
} else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
warning(_("ignoring broken ref %s"), fullref.buf);
@@ -627,7 +627,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
static int is_per_worktree_ref(const char *refname)
{
- return !strcmp(refname, "HEAD") ||
+ return !strcasecmp(refname, "HEAD") ||
starts_with(refname, "refs/worktree/") ||
starts_with(refname, "refs/bisect/") ||
starts_with(refname, "refs/rewritten/");
@@ -847,7 +847,7 @@ int should_autocreate_reflog(const char *refname)
return starts_with(refname, "refs/heads/") ||
starts_with(refname, "refs/remotes/") ||
starts_with(refname, "refs/notes/") ||
- !strcmp(refname, "HEAD");
+ !strcasecmp(refname, "HEAD");
default:
return 0;
}
@@ -855,7 +855,7 @@ int should_autocreate_reflog(const char *refname)
int is_branch(const char *refname)
{
- return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
+ return !strcasecmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
}
struct read_ref_at_cb {
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index b664e51250..e7f8a129fd 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -76,4 +76,13 @@ test_expect_success 'reflog of worktrees/xx/HEAD' '
test_cmp expected actual.wt2
'
+test_expect_success 'head, Head, and HEAD are the same in worktree' '
+ test_cmp_rev worktree/foo initial &&
+ git -C wt1 rev-parse HEAD >uc_ref.wt1 &&
+ git -C wt1 rev-parse Head >mc_ref.wt1 &&
+ git -C wt1 rev-parse head >lc_ref.wt1 &&
+ test_cmp uc_ref.wt1 lc_ref.wt1 &&
+ test_cmp uc_ref.wt1 mc_ref.wt1
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/1] worktree refs: fix case sensitivity for 'head'
From: Michael Rappazzo via GitGitGadget @ 2018-12-13 19:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
On a worktree which is not the primary, using the symbolic-ref 'head' was
incorrectly pointing to the main worktree's HEAD. The same was true for any
other casing of the word 'Head'.
Signed-off-by: Michael Rappazzo rappazzo@gmail.com [rappazzo@gmail.com]
Michael Rappazzo (1):
worktree refs: fix case sensitivity for 'head'
refs.c | 8 ++++----
t/t1415-worktree-refs.sh | 9 +++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-100%2Frappazzo%2Fhead_case_sensitivity_on_worktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-100/rappazzo/head_case_sensitivity_on_worktree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/100
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/3] protocol v2 and hidden refs
From: Jonathan Tan @ 2018-12-13 19:53 UTC (permalink / raw)
To: peff; +Cc: git, bwilliamseng, jonathantanmy
In-Reply-To: <20181211104236.GA6899@sigill.intra.peff.net>
Just realized that I haven't replied to this yet...
> - I'm a little worried this may happen again with future features. The
> root cause is that "ls-refs" follows a different code path than the
> ref advertisement for "upload-pack". So if we add any new config,
> it needs to go both places (non ref-advertisement config is OK, as
> the v2 "fetch" command is a lot closer to a v0 upload-pack).
>
> I think this is just an issue for any future features. I looked for
> other existing features which might be missing in v2, but couldn't
> find any.
>
> I don't know if there's a good solution. I tried running the whole
> test suite with v2 as the default. It does find this bug, but it has
> a bunch of other problems (notably fetch-pack won't run as v2, but
> some other tests I think also depend on v0's reachability rules,
> which v2 is documented not to enforce).
I think Aevar's patches (sent after you wrote this) is a good start, and
I have started looking at it too.
> - The "serve" command is funky, because it has no concept of whether
> the "ls-refs" is for fetching or pushing. Is git-serve even a thing
> that we want to support going forward? I know part of the original
> v2 conception was that one would be able to just connect to
> "git-serve" and do a number of operations. But in practice the v2
> probing requires saying "I'd like to git-upload-pack, and v2 if you
> please". So no client ever calls git-serve.
>
> Is this something we plan to eventually move to? Or can it be
> considered a funny vestige of the development? In the latter case, I
> think we should consider removing it.
Personally, I lean towards removing it, but there are arguments on both
sides. In particular, removing "serve" means that both developers and
users of Git need not be concerned with a 3rd endpoint, but preserving
"serve" (and planning to migrate away from "upload-pack" and
"receive-pack") means that we will only have one endpoint, eliminating
confusion about which endpoint to use when making certain requests (when
we add requests other than "fetch" and "push").
^ permalink raw reply
* Re: [PATCH v2 5/8] tests: add a special setup where for protocol.version
From: Jonathan Tan @ 2018-12-13 19:48 UTC (permalink / raw)
To: avarab; +Cc: git, gitster, peff, bwilliamseng, jonathantanmy
In-Reply-To: <20181213155817.27666-6-avarab@gmail.com>
Regarding the subject, I think you mean "add a special setup var", not
"add a special setup where".
> +GIT_TEST_PROTOCOL_VERSION=<'protocol.version' config value>, when set,
> +runs the test suite with the given protocol.version. E.g. "0", "1" or
> +"2". Can be set to the empty string within tests themselves (e.g. "env
> +GIT_TEST_PROTOCOL_VERSION= <cmd>") to unset the value in the
> +environment as a workaround for "env --unset" not being portable.
Optional: I prefer the "overrides" language used in
GIT_TEST_COMMIT_GRAPH to make it clear that any existing setting is
ignored. E.g.:
GIT_TEST_PROTOCOL_VERSION=<'protocol.version' config value>, when set
to a non-empty string, overrides the 'protocol.version' value. E.g.
"0", ...
Other than that, this patch and the other patches (1-4, 6, 8) look good
to me. My patch 7 looks good too, but it's probably better if someone
else looks at it too :-)
^ permalink raw reply
* [PATCH v4 3/3] Makefile: correct example fuzz build
From: Josh Steadmon @ 2018-12-13 19:43 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, avarab, peff, szeder.dev
In-Reply-To: <cover.1544729841.git.steadmon@google.com>
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 6b72f37c29..bbcfc2bc9f 100644
--- a/Makefile
+++ b/Makefile
@@ -3104,7 +3104,7 @@ cover_db_html: cover_db
# An example command to build against libFuzzer from LLVM 4.0.0:
#
# make CC=clang CXX=clang++ \
-# FUZZ_CXXFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
+# CFLAGS="-fsanitize-coverage=trace-pc-guard -fsanitize=address" \
# LIB_FUZZING_ENGINE=/usr/lib/llvm-4.0/lib/libFuzzer.a \
# fuzz-all
#
--
2.20.0.rc2.10.g21101b961a
^ permalink raw reply related
* [PATCH v4 2/3] commit-graph: fix buffer read-overflow
From: Josh Steadmon @ 2018-12-13 19:43 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, avarab, peff, szeder.dev
In-Reply-To: <cover.1544729841.git.steadmon@google.com>
fuzz-commit-graph identified a case where Git will read past the end of
a buffer containing a commit graph if the graph's header has an
incorrect chunk count. A simple bounds check in parse_commit_graph()
prevents this.
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
commit-graph.c | 14 ++++++++++++--
t/t5318-commit-graph.sh | 16 +++++++++++++---
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 07dd410f3c..836d65a1d3 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -165,10 +165,20 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
last_chunk_offset = 8;
chunk_lookup = data + 8;
for (i = 0; i < graph->num_chunks; i++) {
- uint32_t chunk_id = get_be32(chunk_lookup + 0);
- uint64_t chunk_offset = get_be64(chunk_lookup + 4);
+ uint32_t chunk_id;
+ uint64_t chunk_offset;
int chunk_repeated = 0;
+ if (data + graph_size - chunk_lookup <
+ GRAPH_CHUNKLOOKUP_WIDTH) {
+ error(_("chunk lookup table entry missing; graph file may be incomplete"));
+ free(graph);
+ return NULL;
+ }
+
+ chunk_id = get_be32(chunk_lookup + 0);
+ chunk_offset = get_be64(chunk_lookup + 4);
+
chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;
if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 5fe21db99f..a1b5a75882 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -366,21 +366,26 @@ GRAPH_OCTOPUS_DATA_OFFSET=$(($GRAPH_COMMIT_DATA_OFFSET + \
GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))
GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))
-# usage: corrupt_graph_and_verify <position> <data> <string>
+# usage: corrupt_graph_and_verify <position> <data> <string> [<zero_pos>]
# Manipulates the commit-graph file at the position
-# by inserting the data, then runs 'git commit-graph verify'
+# by inserting the data, optionally zeroing the file
+# starting at <zero_pos>, then runs 'git commit-graph verify'
# and places the output in the file 'err'. Test 'err' for
# the given string.
corrupt_graph_and_verify() {
pos=$1
data="${2:-\0}"
grepstr=$3
+ orig_size=$(wc -c < $objdir/info/commit-graph) &&
+ zero_pos=${4:-${orig_size}} &&
cd "$TRASH_DIRECTORY/full" &&
test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
cp $objdir/info/commit-graph commit-graph-backup &&
printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc &&
+ dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 &&
+ dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) &&
test_must_fail git commit-graph verify 2>test_err &&
- grep -v "^+" test_err >err
+ grep -v "^+" test_err >err &&
test_i18ngrep "$grepstr" err
}
@@ -484,6 +489,11 @@ test_expect_success 'detect invalid checksum hash' '
"incorrect checksum"
'
+test_expect_success 'detect incorrect chunk count' '
+ corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\377" \
+ "chunk lookup table entry missing" $GRAPH_CHUNK_LOOKUP_OFFSET
+'
+
test_expect_success 'git fsck (checks commit-graph)' '
cd "$TRASH_DIRECTORY/full" &&
git fsck &&
--
2.20.0.rc2.10.g21101b961a
^ permalink raw reply related
* [PATCH v4 1/3] commit-graph, fuzz: Add fuzzer for commit-graph
From: Josh Steadmon @ 2018-12-13 19:43 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, avarab, peff, szeder.dev
In-Reply-To: <cover.1544729841.git.steadmon@google.com>
Breaks load_commit_graph_one() into a new function,
parse_commit_graph(). The latter function operates on arbitrary buffers,
which makes it suitable as a fuzzing target. Since parse_commit_graph()
is only called by load_commit_graph_one() (and the fuzzer described
below), we omit error messages that would be duplicated by the caller.
Adds fuzz-commit-graph.c, which provides a fuzzing entry point
compatible with libFuzzer (and possibly other fuzzing engines).
Signed-off-by: Josh Steadmon <steadmon@google.com>
---
.gitignore | 1 +
Makefile | 1 +
commit-graph.c | 53 ++++++++++++++++++++++++++++++---------------
commit-graph.h | 3 +++
fuzz-commit-graph.c | 16 ++++++++++++++
5 files changed, 57 insertions(+), 17 deletions(-)
create mode 100644 fuzz-commit-graph.c
diff --git a/.gitignore b/.gitignore
index 0d77ea5894..8bcf153ed9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
+/fuzz-commit-graph
/fuzz_corpora
/fuzz-pack-headers
/fuzz-pack-idx
diff --git a/Makefile b/Makefile
index 1a44c811aa..6b72f37c29 100644
--- a/Makefile
+++ b/Makefile
@@ -684,6 +684,7 @@ SCRIPTS = $(SCRIPT_SH_INS) \
ETAGS_TARGET = TAGS
+FUZZ_OBJS += fuzz-commit-graph.o
FUZZ_OBJS += fuzz-pack-headers.o
FUZZ_OBJS += fuzz-pack-idx.o
diff --git a/commit-graph.c b/commit-graph.c
index 40c855f185..07dd410f3c 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -84,16 +84,10 @@ static int commit_graph_compatible(struct repository *r)
struct commit_graph *load_commit_graph_one(const char *graph_file)
{
void *graph_map;
- const unsigned char *data, *chunk_lookup;
size_t graph_size;
struct stat st;
- uint32_t i;
- struct commit_graph *graph;
+ struct commit_graph *ret;
int fd = git_open(graph_file);
- uint64_t last_chunk_offset;
- uint32_t last_chunk_id;
- uint32_t graph_signature;
- unsigned char graph_version, hash_version;
if (fd < 0)
return NULL;
@@ -108,27 +102,55 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
die(_("graph file %s is too small"), graph_file);
}
graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ ret = parse_commit_graph(graph_map, fd, graph_size);
+
+ if (!ret) {
+ munmap(graph_map, graph_size);
+ close(fd);
+ exit(1);
+ }
+
+ return ret;
+}
+
+struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ size_t graph_size)
+{
+ const unsigned char *data, *chunk_lookup;
+ uint32_t i;
+ struct commit_graph *graph;
+ uint64_t last_chunk_offset;
+ uint32_t last_chunk_id;
+ uint32_t graph_signature;
+ unsigned char graph_version, hash_version;
+
+ if (!graph_map)
+ return NULL;
+
+ if (graph_size < GRAPH_MIN_SIZE)
+ return NULL;
+
data = (const unsigned char *)graph_map;
graph_signature = get_be32(data);
if (graph_signature != GRAPH_SIGNATURE) {
error(_("graph signature %X does not match signature %X"),
graph_signature, GRAPH_SIGNATURE);
- goto cleanup_fail;
+ return NULL;
}
graph_version = *(unsigned char*)(data + 4);
if (graph_version != GRAPH_VERSION) {
error(_("graph version %X does not match version %X"),
graph_version, GRAPH_VERSION);
- goto cleanup_fail;
+ return NULL;
}
hash_version = *(unsigned char*)(data + 5);
if (hash_version != GRAPH_OID_VERSION) {
error(_("hash version %X does not match version %X"),
hash_version, GRAPH_OID_VERSION);
- goto cleanup_fail;
+ return NULL;
}
graph = alloc_commit_graph();
@@ -152,7 +174,8 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
error(_("improper chunk offset %08x%08x"), (uint32_t)(chunk_offset >> 32),
(uint32_t)chunk_offset);
- goto cleanup_fail;
+ free(graph);
+ return NULL;
}
switch (chunk_id) {
@@ -187,7 +210,8 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
if (chunk_repeated) {
error(_("chunk id %08x appears multiple times"), chunk_id);
- goto cleanup_fail;
+ free(graph);
+ return NULL;
}
if (last_chunk_id == GRAPH_CHUNKID_OIDLOOKUP)
@@ -201,11 +225,6 @@ struct commit_graph *load_commit_graph_one(const char *graph_file)
}
return graph;
-
-cleanup_fail:
- munmap(graph_map, graph_size);
- close(fd);
- exit(1);
}
static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
diff --git a/commit-graph.h b/commit-graph.h
index 9db40b4d3a..813e7c19f1 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -54,6 +54,9 @@ struct commit_graph {
struct commit_graph *load_commit_graph_one(const char *graph_file);
+struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ size_t graph_size);
+
/*
* Return 1 if and only if the repository has a commit-graph
* file and generation numbers are computed in that file.
diff --git a/fuzz-commit-graph.c b/fuzz-commit-graph.c
new file mode 100644
index 0000000000..cf790c9d04
--- /dev/null
+++ b/fuzz-commit-graph.c
@@ -0,0 +1,16 @@
+#include "commit-graph.h"
+
+struct commit_graph *parse_commit_graph(void *graph_map, int fd,
+ size_t graph_size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);
+
+int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
+{
+ struct commit_graph *g;
+
+ g = parse_commit_graph((void *)data, -1, size);
+ free(g);
+
+ return 0;
+}
--
2.20.0.rc2.10.g21101b961a
^ permalink raw reply related
* [PATCH v4 0/3] Add commit-graph fuzzer and fix buffer overflow
From: Josh Steadmon @ 2018-12-13 19:43 UTC (permalink / raw)
To: git; +Cc: gitster, stolee, avarab, peff, szeder.dev
In-Reply-To: <cover.1544048946.git.steadmon@google.com>
Add a new fuzz test for the commit graph and fix a buffer read-overflow
that it discovered. Additionally, fix the Makefile instructions for
building fuzzers.
Changes since V3:
* Improve portability of the new test functionality.
* Fix broken &&-chains in tests.
Changes since V2:
* Avoid pointer arithmetic overflow when checking the graph's chunk
count.
* Merge the corrupt_graph_and_verify and
corrupt_and_zero_graph_then_verify test functions.
Josh Steadmon (3):
commit-graph, fuzz: Add fuzzer for commit-graph
commit-graph: fix buffer read-overflow
Makefile: correct example fuzz build
.gitignore | 1 +
Makefile | 3 +-
commit-graph.c | 67 +++++++++++++++++++++++++++++------------
commit-graph.h | 3 ++
fuzz-commit-graph.c | 16 ++++++++++
t/t5318-commit-graph.sh | 16 ++++++++--
6 files changed, 83 insertions(+), 23 deletions(-)
create mode 100644 fuzz-commit-graph.c
Range-diff against v3:
1: 675d58ecea ! 1: 80b5662f30 commit-graph: fix buffer read-overflow
@@ -55,29 +55,26 @@
pos=$1
data="${2:-\0}"
grepstr=$3
-+ orig_size=$(stat --format=%s $objdir/info/commit-graph)
-+ zero_pos=${4:-${orig_size}}
++ orig_size=$(wc -c < $objdir/info/commit-graph) &&
++ zero_pos=${4:-${orig_size}} &&
cd "$TRASH_DIRECTORY/full" &&
test_when_finished mv commit-graph-backup $objdir/info/commit-graph &&
cp $objdir/info/commit-graph commit-graph-backup &&
printf "$data" | dd of="$objdir/info/commit-graph" bs=1 seek="$pos" conv=notrunc &&
-+ truncate --size=$zero_pos $objdir/info/commit-graph &&
-+ truncate --size=$orig_size $objdir/info/commit-graph &&
++ dd of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=0 &&
++ dd if=/dev/zero of="$objdir/info/commit-graph" bs=1 seek="$zero_pos" count=$(($orig_size - $zero_pos)) &&
test_must_fail git commit-graph verify 2>test_err &&
- grep -v "^+" test_err >err
+- grep -v "^+" test_err >err
++ grep -v "^+" test_err >err &&
test_i18ngrep "$grepstr" err
}
-+
- test_expect_success 'detect bad signature' '
- corrupt_graph_and_verify 0 "\0" \
- "graph signature"
@@
"incorrect checksum"
'
+test_expect_success 'detect incorrect chunk count' '
-+ corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\xff" \
++ corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\377" \
+ "chunk lookup table entry missing" $GRAPH_CHUNK_LOOKUP_OFFSET
+'
+
2: 06a32bfe8b = 2: 21101b961a Makefile: correct example fuzz build
--
2.20.0.rc2.10.g21101b961a
^ permalink raw reply
* [PATCH v2] teach git to support a virtual (partially populated) work directory
From: Ben Peart @ 2018-12-13 19:41 UTC (permalink / raw)
To: git; +Cc: benpeart, pclouds, sandals, avarab, Johannes.Schindelin,
szeder.dev
In-Reply-To: <20181030191608.18716-1-peartben@gmail.com>
From: Ben Peart <benpeart@microsoft.com>
To make git perform well on the very largest repos, we must make git
operations O(modified) instead of O(size of repo). This takes advantage of
the fact that the number of files a developer has modified (especially
in very large repos) is typically a tiny fraction of the overall repo size.
We accomplished this by utilizing the existing internal logic for the skip
worktree bit and excludes to tell git to ignore all files and folders other
than those that have been modified. This logic is driven by an external
process that monitors writes to the repo and communicates the list of files
and folders with changes to git via the virtual work directory hook in this
patch.
The external process maintains a list of files and folders that have been
modified. When git runs, it requests the list of files and folders that
have been modified via the virtual work directory hook. Git then sets/clears
the skip-worktree bit on the cache entries and builds a hashmap of the
modified files/folders that is used by the excludes logic to avoid scanning
the entire repo looking for changes and untracked files.
With this system, we have been able to make local git command performance on
extremely large repos (millions of files, 1/2 million folders) entirely
manageable (30 second checkout, 3.5 seconds status, 4 second add, 7 second
commit, etc).
On index load, clear/set the skip worktree bits based on the virtual
work directory data. Use virtual work directory data to update skip-worktree
bit in unpack-trees. Use virtual work directory data to exclude files and
folders not explicitly requested.
Signed-off-by: Ben Peart <benpeart@microsoft.com>
---
Notes:
Base Ref: v2.20.0
Web-Diff: https://github.com/benpeart/git/commit/acc00a41af
Checkout: git fetch https://github.com/benpeart/git virtual-workdir-v2 && git checkout acc00a41af
### Patches
Documentation/config/core.txt | 9 +
Documentation/githooks.txt | 23 ++
Makefile | 1 +
cache.h | 1 +
config.c | 32 ++-
config.h | 1 +
dir.c | 26 ++-
environment.c | 1 +
read-cache.c | 2 +
t/t1092-virtualworkdir.sh | 390 ++++++++++++++++++++++++++++++++++
unpack-trees.c | 23 +-
virtualworkdir.c | 314 +++++++++++++++++++++++++++
virtualworkdir.h | 25 +++
13 files changed, 840 insertions(+), 8 deletions(-)
create mode 100755 t/t1092-virtualworkdir.sh
create mode 100644 virtualworkdir.c
create mode 100644 virtualworkdir.h
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index d0e6635fe0..49b7699a4e 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -68,6 +68,15 @@ core.fsmonitor::
avoiding unnecessary processing of files that have not changed.
See the "fsmonitor-watchman" section of linkgit:githooks[5].
+core.virtualWorkDir::
+ Please regard this as an experimental feature.
+ If set to true, utilize the virtual-work-dir hook to identify all
+ files and directories that are present in the working directory.
+ Git will only track and update files listed in the virtual work
+ directory. Using the virtual work directory will supersede the
+ sparse-checkout settings which will be ignored.
+ See the "virtual-work-dir" section of linkgit:githooks[6].
+
core.trustctime::
If false, the ctime differences between the index and the
working tree are ignored; useful when the inode change time
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 959044347e..9888d504b4 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -485,6 +485,29 @@ The exit status determines whether git will use the data from the
hook to limit its search. On error, it will fall back to verifying
all files and folders.
+virtual-work-dir
+~~~~~~~~~~~~~~~~
+
+Please regard this as an experimental feature.
+
+The "Virtual Work Directory" hook allows populating the working directory
+sparsely. The virtual work directory data is typically automatically
+generated by an external process. Git will limit what files it checks for
+changes as well as which directories are checked for untracked files based
+on the path names given. Git will also only update those files listed in the
+virtual work directory.
+
+The hook is invoked when the configuration option core.virtualWorkDir is
+set to true. The hook takes one argument, a version (currently 1).
+
+The hook should output to stdout the list of all files in the working
+directory that git should track. The paths are relative to the root
+of the working directory and are separated by a single NUL. Full paths
+('dir1/a.txt') as well as directories are supported (ie 'dir1/').
+
+The exit status determines whether git will use the data from the
+hook. On error, git will abort the command with an error message.
+
p4-pre-submit
~~~~~~~~~~~~~
diff --git a/Makefile b/Makefile
index 1a44c811aa..061f1ab954 100644
--- a/Makefile
+++ b/Makefile
@@ -1012,6 +1012,7 @@ LIB_OBJS += utf8.o
LIB_OBJS += varint.o
LIB_OBJS += version.o
LIB_OBJS += versioncmp.o
+LIB_OBJS += virtualworkdir.o
LIB_OBJS += walker.o
LIB_OBJS += wildmatch.o
LIB_OBJS += worktree.o
diff --git a/cache.h b/cache.h
index ca36b44ee0..39650e6efd 100644
--- a/cache.h
+++ b/cache.h
@@ -886,6 +886,7 @@ extern char *git_replace_ref_base;
extern int fsync_object_files;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
+extern int core_virtualworkdir;
extern int precomposed_unicode;
extern int protect_hfs;
extern int protect_ntfs;
diff --git a/config.c b/config.c
index ff521eb27a..fc0d51aa69 100644
--- a/config.c
+++ b/config.c
@@ -1325,7 +1325,11 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
}
if (!strcmp(var, "core.sparsecheckout")) {
- core_apply_sparse_checkout = git_config_bool(var, value);
+ /* virtual working directory relies on the sparse checkout logic so force it on */
+ if (core_virtualworkdir)
+ core_apply_sparse_checkout = 1;
+ else
+ core_apply_sparse_checkout = git_config_bool(var, value);
return 0;
}
@@ -2315,6 +2319,32 @@ int git_config_get_index_threads(int *dest)
return 1;
}
+int git_config_get_virtualworkdir(void)
+{
+ git_config_get_bool("core.virtualworkdir", &core_virtualworkdir);
+ if (core_virtualworkdir) {
+ /*
+ * Some git commands spawn helpers and redirect the index to a different
+ * location. These include "difftool -d" and the sequencer
+ * (i.e. `git rebase -i`, `git cherry-pick` and `git revert`) and others.
+ * In those instances we don't want to update their temporary index with
+ * our virtualization data.
+ */
+ char *default_index_file = xstrfmt("%s/%s", the_repository->gitdir, "index");
+ int should_run_hook = !strcmp(default_index_file, the_repository->index_file);
+
+ free(default_index_file);
+ if (should_run_hook) {
+ /* virtual working directory relies on the sparse checkout logic so force it on */
+ core_apply_sparse_checkout = 1;
+ return core_virtualworkdir;
+ }
+ core_virtualworkdir = 0;
+ }
+
+ return core_virtualworkdir;
+}
+
NORETURN
void git_die_config_linenr(const char *key, const char *filename, int linenr)
{
diff --git a/config.h b/config.h
index ee5d3fa7b4..e89590603c 100644
--- a/config.h
+++ b/config.h
@@ -251,6 +251,7 @@ extern int git_config_get_untracked_cache(void);
extern int git_config_get_split_index(void);
extern int git_config_get_max_percent_split_change(void);
extern int git_config_get_fsmonitor(void);
+extern int git_config_get_virtualworkdir(void);
/* This dies if the configured or default date is in the future */
extern int git_config_get_expiry(const char *key, const char **output);
diff --git a/dir.c b/dir.c
index ab6477d777..987a3eb17f 100644
--- a/dir.c
+++ b/dir.c
@@ -21,6 +21,7 @@
#include "ewah/ewok.h"
#include "fsmonitor.h"
#include "submodule-config.h"
+#include "virtualworkdir.h"
/*
* Tells read_directory_recursive how a file or directory should be treated.
@@ -1116,6 +1117,14 @@ int is_excluded_from_list(const char *pathname,
struct exclude_list *el, struct index_state *istate)
{
struct exclude *exclude;
+
+ if (core_virtualworkdir) {
+ if (*dtype == DT_UNKNOWN)
+ *dtype = get_dtype(NULL, istate, pathname, pathlen);
+ if (is_excluded_from_virtualworkdir(pathname, pathlen, *dtype) > 0)
+ return 1;
+ }
+
exclude = last_exclude_matching_from_list(pathname, pathlen, basename,
dtype, el, istate);
if (exclude)
@@ -1331,8 +1340,16 @@ struct exclude *last_exclude_matching(struct dir_struct *dir,
int is_excluded(struct dir_struct *dir, struct index_state *istate,
const char *pathname, int *dtype_p)
{
- struct exclude *exclude =
- last_exclude_matching(dir, istate, pathname, dtype_p);
+ struct exclude *exclude;
+
+ if (core_virtualworkdir) {
+ if (*dtype_p == DT_UNKNOWN)
+ *dtype_p = get_dtype(NULL, istate, pathname, strlen(pathname));
+ if (is_excluded_from_virtualworkdir(pathname, strlen(pathname), *dtype_p) > 0)
+ return 1;
+ }
+
+ exclude = last_exclude_matching(dir, istate, pathname, dtype_p);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -1685,6 +1702,9 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
if (dtype != DT_DIR && has_path_in_index)
return path_none;
+ if (is_excluded_from_virtualworkdir(path->buf, path->len, dtype) > 0)
+ return path_excluded;
+
/*
* When we are looking at a directory P in the working tree,
* there are three cases:
@@ -2025,6 +2045,8 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
/* add the path to the appropriate result list */
switch (state) {
case path_excluded:
+ if (is_excluded_from_virtualworkdir(path.buf, path.len, DT_DIR) > 0)
+ break;
if (dir->flags & DIR_SHOW_IGNORED)
dir_add_name(dir, istate, path.buf, path.len);
else if ((dir->flags & DIR_SHOW_IGNORED_TOO) ||
diff --git a/environment.c b/environment.c
index 3465597707..bc0cef4506 100644
--- a/environment.c
+++ b/environment.c
@@ -69,6 +69,7 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
char *notes_ref_name;
int grafts_replace_parents = 1;
int core_apply_sparse_checkout;
+int core_virtualworkdir;
int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
unsigned long pack_size_limit_cfg;
diff --git a/read-cache.c b/read-cache.c
index bd45dc3e24..a2c8027977 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -25,6 +25,7 @@
#include "fsmonitor.h"
#include "thread-utils.h"
#include "progress.h"
+#include "virtualworkdir.h"
/* Mask for the name length in ce_flags in the on-disk index */
@@ -1894,6 +1895,7 @@ static void post_read_index_from(struct index_state *istate)
tweak_untracked_cache(istate);
tweak_split_index(istate);
tweak_fsmonitor(istate);
+ apply_virtualworkdir(istate);
}
static size_t estimate_cache_size_from_compressed(unsigned int entries)
diff --git a/t/t1092-virtualworkdir.sh b/t/t1092-virtualworkdir.sh
new file mode 100755
index 0000000000..752049fbe3
--- /dev/null
+++ b/t/t1092-virtualworkdir.sh
@@ -0,0 +1,390 @@
+#!/bin/sh
+
+test_description='virtual work directory tests'
+
+. ./test-lib.sh
+
+reset_repo () {
+ rm .git/index &&
+ git -c core.virtualworkdir=false reset --hard HEAD &&
+ git -c core.virtualworkdir=false clean -fd &&
+ >untracked.txt &&
+ >dir1/untracked.txt &&
+ >dir2/untracked.txt
+}
+
+test_expect_success 'setup' '
+ mkdir -p .git/hooks/ &&
+ cat >.gitignore <<-\EOF &&
+ .gitignore
+ expect*
+ actual*
+ EOF
+ >file1.txt &&
+ >file2.txt &&
+ mkdir -p dir1 &&
+ >dir1/file1.txt &&
+ >dir1/file2.txt &&
+ mkdir -p dir2 &&
+ >dir2/file1.txt &&
+ >dir2/file2.txt &&
+ git add . &&
+ git commit -m "initial" &&
+ git config --local core.virtualworkdir true
+'
+
+test_expect_success 'test hook parameters and version' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ if test "$#" -ne 1
+ then
+ echo "$0: Exactly 1 argument expected" >&2
+ exit 2
+ fi
+
+ if test "$1" != 1
+ then
+ echo "$0: Unsupported hook version." >&2
+ exit 1
+ fi
+ EOF
+ git status &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ exit 3
+ EOF
+ test_must_fail git status
+'
+
+test_expect_success 'verify status is clean' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir2/file1.txt\0"
+ EOF
+ rm -f .git/index &&
+ git checkout -f &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir2/file1.txt\0"
+ printf "dir1/file1.txt\0"
+ printf "dir1/file2.txt\0"
+ EOF
+ git status >actual &&
+ cat >expected <<-\EOF &&
+ On branch master
+ nothing to commit, working tree clean
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify skip-worktree bit is set for absolute path' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ EOF
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ H dir1/file1.txt
+ S dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify skip-worktree bit is cleared for absolute path' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file2.txt\0"
+ EOF
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ S dir1/file1.txt
+ H dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify folder wild cards' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/\0"
+ EOF
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ H dir1/file1.txt
+ H dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify folders not included are ignored' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ printf "dir1/file2.txt\0"
+ EOF
+ mkdir -p dir1/dir2 &&
+ >dir1/a &&
+ >dir1/b &&
+ >dir1/dir2/a &&
+ >dir1/dir2/b &&
+ git add . &&
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ H dir1/file1.txt
+ H dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify including one file doesnt include the rest' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ printf "dir1/file2.txt\0"
+ printf "dir1/dir2/a\0"
+ EOF
+ mkdir -p dir1/dir2 &&
+ >dir1/a &&
+ >dir1/b &&
+ >dir1/dir2/a &&
+ >dir1/dir2/b &&
+ git add . &&
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ H dir1/dir2/a
+ H dir1/file1.txt
+ H dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify files not listed are ignored by git clean -f -x' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "untracked.txt\0"
+ printf "dir1/\0"
+ EOF
+ mkdir -p dir3 &&
+ >dir3/untracked.txt &&
+ git clean -f -x &&
+ test_path_is_file file1.txt &&
+ test_path_is_file file2.txt &&
+ test_path_is_missing untracked.txt &&
+ test_path_is_dir dir1 &&
+ test_path_is_file dir1/file1.txt &&
+ test_path_is_file dir1/file2.txt &&
+ test_path_is_missing dir1/untracked.txt &&
+ test_path_is_file dir2/file1.txt &&
+ test_path_is_file dir2/file2.txt &&
+ test_path_is_file dir2/untracked.txt &&
+ test_path_is_dir dir3 &&
+ test_path_is_file dir3/untracked.txt
+'
+
+test_expect_success 'verify files not listed are ignored by git clean -f -d -x' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "untracked.txt\0"
+ printf "dir1/\0"
+ printf "dir3/\0"
+ EOF
+ mkdir -p dir3 &&
+ >dir3/untracked.txt &&
+ git clean -f -d -x &&
+ test_path_is_file file1.txt &&
+ test_path_is_file file2.txt &&
+ test_path_is_missing untracked.txt &&
+ test_path_is_dir dir1 &&
+ test_path_is_file dir1/file1.txt &&
+ test_path_is_file dir1/file2.txt &&
+ test_path_is_missing dir1/untracked.txt &&
+ test_path_is_file dir2/file1.txt &&
+ test_path_is_file dir2/file2.txt &&
+ test_path_is_file dir2/untracked.txt &&
+ test ! -d dir3 &&
+ test_path_is_missing dir3/untracked.txt
+'
+
+test_expect_success 'verify folder entries include all files' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/\0"
+ EOF
+ mkdir -p dir1/dir2 &&
+ >dir1/a &&
+ >dir1/b &&
+ >dir1/dir2/a &&
+ >dir1/dir2/b &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ ?? dir1/a
+ ?? dir1/b
+ ?? dir1/dir2/a
+ ?? dir1/dir2/b
+ ?? dir1/untracked.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'verify case insensitivity of virtual work directory entries' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/a\0"
+ printf "Dir1/Dir2/a\0"
+ printf "DIR2/\0"
+ EOF
+ mkdir -p dir1/dir2 &&
+ >dir1/a &&
+ >dir1/b &&
+ >dir1/dir2/a &&
+ >dir1/dir2/b &&
+ git -c core.ignorecase=false status -su >actual &&
+ cat >expected <<-\EOF &&
+ ?? dir1/a
+ EOF
+ test_cmp expected actual &&
+ git -c core.ignorecase=true status -su >actual &&
+ cat >expected <<-\EOF &&
+ ?? dir1/a
+ ?? dir1/dir2/a
+ ?? dir2/untracked.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'on file created' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file3.txt\0"
+ EOF
+ >dir1/file3.txt &&
+ git add . &&
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ S dir1/file1.txt
+ S dir1/file2.txt
+ H dir1/file3.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'on file renamed' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ printf "dir1/file3.txt\0"
+ EOF
+ mv dir1/file1.txt dir1/file3.txt &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ D dir1/file1.txt
+ ?? dir1/file3.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'on file deleted' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ EOF
+ rm dir1/file1.txt &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ D dir1/file1.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'on file overwritten' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/file1.txt\0"
+ EOF
+ echo "overwritten" >dir1/file1.txt &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ M dir1/file1.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'on folder created' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/dir1/\0"
+ EOF
+ mkdir -p dir1/dir1 &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ EOF
+ test_cmp expected actual &&
+ git clean -fd &&
+ test ! -d "/dir1/dir1"
+'
+
+test_expect_success 'on folder renamed' '
+ reset_repo &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir3/\0"
+ printf "dir1/file1.txt\0"
+ printf "dir1/file2.txt\0"
+ printf "dir3/file1.txt\0"
+ printf "dir3/file2.txt\0"
+ EOF
+ mv dir1 dir3 &&
+ git status -su >actual &&
+ cat >expected <<-\EOF &&
+ D dir1/file1.txt
+ D dir1/file2.txt
+ ?? dir3/file1.txt
+ ?? dir3/file2.txt
+ ?? dir3/untracked.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'folder with same prefix as file' '
+ reset_repo &&
+ >dir1.sln &&
+ write_script .git/hooks/virtual-work-dir <<-\EOF &&
+ printf "dir1/\0"
+ printf "dir1.sln\0"
+ EOF
+ git add dir1.sln &&
+ git ls-files -v >actual &&
+ cat >expected <<-\EOF &&
+ H dir1.sln
+ H dir1/file1.txt
+ H dir1/file2.txt
+ S dir2/file1.txt
+ S dir2/file2.txt
+ S file1.txt
+ S file2.txt
+ EOF
+ test_cmp expected actual
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 7570df481b..c6c20c9b61 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -18,6 +18,7 @@
#include "fsmonitor.h"
#include "object-store.h"
#include "fetch-object.h"
+#include "virtualworkdir.h"
/*
* Error messages expected by scripts out of plumbing commands such as
@@ -1363,6 +1364,14 @@ static int clear_ce_flags_1(struct index_state *istate,
continue;
}
+ /* if it's not in the virtual working directory, exit early */
+ if (core_virtualworkdir) {
+ if (is_included_in_virtualworkdir(ce->name, ce->ce_namelen) > 0)
+ ce->ce_flags &= ~clear_mask;
+ cache++;
+ continue;
+ }
+
if (prefix->len && strncmp(ce->name, prefix->buf, prefix->len))
break;
@@ -1481,12 +1490,16 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
if (!core_apply_sparse_checkout || !o->update)
o->skip_sparse_checkout = 1;
if (!o->skip_sparse_checkout) {
- char *sparse = git_pathdup("info/sparse-checkout");
- if (add_excludes_from_file_to_list(sparse, "", 0, &el, NULL) < 0)
- o->skip_sparse_checkout = 1;
- else
+ if (core_virtualworkdir) {
o->el = ⪙
- free(sparse);
+ } else {
+ char *sparse = git_pathdup("info/sparse-checkout");
+ if (add_excludes_from_file_to_list(sparse, "", 0, &el, NULL) < 0)
+ o->skip_sparse_checkout = 1;
+ else
+ o->el = ⪙
+ free(sparse);
+ }
}
memset(&o->result, 0, sizeof(o->result));
diff --git a/virtualworkdir.c b/virtualworkdir.c
new file mode 100644
index 0000000000..f2c8025bf5
--- /dev/null
+++ b/virtualworkdir.c
@@ -0,0 +1,314 @@
+#include "cache.h"
+#include "config.h"
+#include "dir.h"
+#include "hashmap.h"
+#include "run-command.h"
+#include "virtualworkdir.h"
+
+#define HOOK_INTERFACE_VERSION (1)
+
+static struct strbuf virtual_workdir_data = STRBUF_INIT;
+static struct hashmap virtual_workdir_hashmap;
+static struct hashmap parent_directory_hashmap;
+
+struct virtualworkdir {
+ struct hashmap_entry ent; /* must be the first member! */
+ const char *pattern;
+ int patternlen;
+};
+
+static unsigned int(*vwdhash)(const void *buf, size_t len);
+static int(*vwdcmp)(const char *a, const char *b, size_t len);
+
+static int vwd_hashmap_cmp(const void *unused_cmp_data,
+ const void *a, const void *b, const void *key)
+{
+ const struct virtualworkdir *vwd1 = a;
+ const struct virtualworkdir *vwd2 = b;
+
+ return vwdcmp(vwd1->pattern, vwd2->pattern, vwd1->patternlen);
+}
+
+static void get_virtual_workdir_data(struct strbuf *vwd_data)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ const char *p;
+ int err;
+
+ strbuf_init(vwd_data, 0);
+
+ p = find_hook("virtual-work-dir");
+ if (!p)
+ die("unable to find virtual-work-dir hook");
+
+ argv_array_push(&cp.args, p);
+ argv_array_pushf(&cp.args, "%d", HOOK_INTERFACE_VERSION);
+ cp.use_shell = 1;
+ cp.dir = get_git_work_tree();
+
+ err = capture_command(&cp, vwd_data, 1024);
+ if (err)
+ die("unable to load virtual working directory");
+}
+
+static int check_includes_hashmap(struct hashmap *map, const char *pattern, int patternlen)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct virtualworkdir vwd;
+ char *slash;
+
+ /* Check straight mapping */
+ strbuf_reset(&sb);
+ strbuf_add(&sb, pattern, patternlen);
+ vwd.pattern = sb.buf;
+ vwd.patternlen = sb.len;
+ hashmap_entry_init(&vwd, vwdhash(vwd.pattern, vwd.patternlen));
+ if (hashmap_get(map, &vwd, NULL)) {
+ strbuf_release(&sb);
+ return 1;
+ }
+
+ /*
+ * Check to see if it matches a directory or any path
+ * underneath it. In other words, 'a/b/foo.txt' will match
+ * '/', 'a/', and 'a/b/'.
+ */
+ slash = strchr(sb.buf, '/');
+ while (slash) {
+ vwd.pattern = sb.buf;
+ vwd.patternlen = slash - sb.buf + 1;
+ hashmap_entry_init(&vwd, vwdhash(vwd.pattern, vwd.patternlen));
+ if (hashmap_get(map, &vwd, NULL)) {
+ strbuf_release(&sb);
+ return 1;
+ }
+ slash = strchr(slash + 1, '/');
+ }
+
+ strbuf_release(&sb);
+ return 0;
+}
+
+static void includes_hashmap_add(struct hashmap *map, const char *pattern, const int patternlen)
+{
+ struct virtualworkdir *vwd;
+
+ vwd = xmalloc(sizeof(struct virtualworkdir));
+ vwd->pattern = pattern;
+ vwd->patternlen = patternlen;
+ hashmap_entry_init(vwd, vwdhash(vwd->pattern, vwd->patternlen));
+ hashmap_add(map, vwd);
+}
+
+static void initialize_includes_hashmap(struct hashmap *map, struct strbuf *vwd_data)
+{
+ char *buf, *entry;
+ size_t len;
+ int i;
+
+ /*
+ * Build a hashmap of the virtual working directory data we can use to look
+ * for cache entry matches quickly
+ */
+ vwdhash = ignore_case ? memihash : memhash;
+ vwdcmp = ignore_case ? strncasecmp : strncmp;
+ hashmap_init(map, vwd_hashmap_cmp, NULL, 0);
+
+ entry = buf = vwd_data->buf;
+ len = vwd_data->len;
+ for (i = 0; i < len; i++) {
+ if (buf[i] == '\0') {
+ includes_hashmap_add(map, entry, buf + i - entry);
+ entry = buf + i + 1;
+ }
+ }
+}
+
+/*
+ * Return 1 if the requested item is found in the virtual working directory,
+ * 0 for not found and -1 for undecided.
+ */
+int is_included_in_virtualworkdir(const char *pathname, int pathlen)
+{
+ if (!core_virtualworkdir)
+ return -1;
+
+ if (!virtual_workdir_hashmap.tablesize && virtual_workdir_data.len)
+ initialize_includes_hashmap(&virtual_workdir_hashmap, &virtual_workdir_data);
+ if (!virtual_workdir_hashmap.tablesize)
+ return -1;
+
+ return check_includes_hashmap(&virtual_workdir_hashmap, pathname, pathlen);
+}
+
+static void parent_directory_hashmap_add(struct hashmap *map, const char *pattern, const int patternlen)
+{
+ char *slash;
+ struct virtualworkdir *vwd;
+
+ /*
+ * Add any directories leading up to the file as the excludes logic
+ * needs to match directories leading up to the files as well. Detect
+ * and prevent unnecessary duplicate entries which will be common.
+ */
+ if (patternlen > 1) {
+ slash = strchr(pattern + 1, '/');
+ while (slash) {
+ vwd = xmalloc(sizeof(struct virtualworkdir));
+ vwd->pattern = pattern;
+ vwd->patternlen = slash - pattern + 1;
+ hashmap_entry_init(vwd, vwdhash(vwd->pattern, vwd->patternlen));
+ if (hashmap_get(map, vwd, NULL))
+ free(vwd);
+ else
+ hashmap_add(map, vwd);
+ slash = strchr(slash + 1, '/');
+ }
+ }
+}
+
+static void initialize_parent_directory_hashmap(struct hashmap *map, struct strbuf *vwd_data)
+{
+ char *buf, *entry;
+ size_t len;
+ int i;
+
+ /*
+ * Build a hashmap of the parent directories contained in the virtual
+ * file system data we can use to look for matches quickly
+ */
+ vwdhash = ignore_case ? memihash : memhash;
+ vwdcmp = ignore_case ? strncasecmp : strncmp;
+ hashmap_init(map, vwd_hashmap_cmp, NULL, 0);
+
+ entry = buf = vwd_data->buf;
+ len = vwd_data->len;
+ for (i = 0; i < len; i++) {
+ if (buf[i] == '\0') {
+ parent_directory_hashmap_add(map, entry, buf + i - entry);
+ entry = buf + i + 1;
+ }
+ }
+}
+
+static int check_directory_hashmap(struct hashmap *map, const char *pathname, int pathlen)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct virtualworkdir vwd;
+
+ /* Check for directory */
+ strbuf_reset(&sb);
+ strbuf_add(&sb, pathname, pathlen);
+ strbuf_addch(&sb, '/');
+ vwd.pattern = sb.buf;
+ vwd.patternlen = sb.len;
+ hashmap_entry_init(&vwd, vwdhash(vwd.pattern, vwd.patternlen));
+ if (hashmap_get(map, &vwd, NULL)) {
+ strbuf_release(&sb);
+ return 0;
+ }
+
+ strbuf_release(&sb);
+ return 1;
+}
+
+/*
+ * Return 1 for exclude, 0 for include and -1 for undecided.
+ */
+int is_excluded_from_virtualworkdir(const char *pathname, int pathlen, int dtype)
+{
+ if (!core_virtualworkdir)
+ return -1;
+
+ if (dtype != DT_REG && dtype != DT_DIR && dtype != DT_LNK)
+ die(_("is_excluded_from_virtualworkdir passed unhandled dtype"));
+
+ if (dtype == DT_REG || dtype == DT_LNK) {
+ int ret = is_included_in_virtualworkdir(pathname, pathlen);
+ if (ret > 0)
+ return 0;
+ if (ret == 0)
+ return 1;
+ return ret;
+ }
+
+ if (dtype == DT_DIR) {
+ int ret = is_included_in_virtualworkdir(pathname, pathlen);
+ if (ret > 0)
+ return 0;
+
+ if (!parent_directory_hashmap.tablesize && virtual_workdir_data.len)
+ initialize_parent_directory_hashmap(&parent_directory_hashmap, &virtual_workdir_data);
+ if (!parent_directory_hashmap.tablesize)
+ return -1;
+
+ return check_directory_hashmap(&parent_directory_hashmap, pathname, pathlen);
+ }
+
+ return -1;
+}
+
+/*
+ * Update the CE_SKIP_WORKTREE bits based on the virtual working directory.
+ */
+void apply_virtualworkdir(struct index_state *istate)
+{
+ char *buf, *entry;
+ int i;
+
+ if (!git_config_get_virtualworkdir())
+ return;
+
+ if (!virtual_workdir_data.len)
+ get_virtual_workdir_data(&virtual_workdir_data);
+
+ /* set CE_SKIP_WORKTREE bit on all entries */
+ for (i = 0; i < istate->cache_nr; i++)
+ istate->cache[i]->ce_flags |= CE_SKIP_WORKTREE;
+
+ /* clear CE_SKIP_WORKTREE bit for everything in the virtual working directory */
+ entry = buf = virtual_workdir_data.buf;
+ for (i = 0; i < virtual_workdir_data.len; i++) {
+ if (buf[i] == '\0') {
+ int pos, len;
+
+ len = buf + i - entry;
+
+ /* look for a directory wild card (ie "dir1/") */
+ if (buf[i - 1] == '/') {
+ if (ignore_case)
+ adjust_dirname_case(istate, entry);
+
+ pos = index_name_pos(istate, entry, len);
+ if (pos < 0) {
+ pos = -pos - 1;
+ while (pos < istate->cache_nr && !fspathncmp(istate->cache[pos]->name, entry, len)) {
+ istate->cache[pos]->ce_flags &= ~CE_SKIP_WORKTREE;
+ pos++;
+ }
+ }
+ } else {
+ if (ignore_case) {
+ struct cache_entry *ce = index_file_exists(istate, entry, len, ignore_case);
+ if (ce)
+ ce->ce_flags &= ~CE_SKIP_WORKTREE;
+ } else {
+ int pos = index_name_pos(istate, entry, len);
+ if (pos >= 0)
+ istate->cache[pos]->ce_flags &= ~CE_SKIP_WORKTREE;
+ }
+ }
+
+ entry += len + 1;
+ }
+ }
+}
+
+/*
+ * Free the virtual working directory data structures.
+ */
+void free_virtualworkdir(void) {
+ hashmap_free(&virtual_workdir_hashmap, 1);
+ hashmap_free(&parent_directory_hashmap, 1);
+ strbuf_release(&virtual_workdir_data);
+}
diff --git a/virtualworkdir.h b/virtualworkdir.h
new file mode 100644
index 0000000000..139d019d44
--- /dev/null
+++ b/virtualworkdir.h
@@ -0,0 +1,25 @@
+#ifndef VIRTUALWORKDIR_H
+#define VIRTUALWORKDIR_H
+
+/*
+ * Update the CE_SKIP_WORKTREE bits based on the virtual working directory.
+ */
+void apply_virtualworkdir(struct index_state *istate);
+
+/*
+ * Return 1 if the requested item is found in the virtual working directory,
+ * 0 for not found and -1 for undecided.
+ */
+int is_included_in_virtualworkdir(const char *pathname, int pathlen);
+
+/*
+ * Return 1 for exclude, 0 for include and -1 for undecided.
+ */
+int is_excluded_from_virtualworkdir(const char *pathname, int pathlen, int dtype);
+
+/*
+ * Free the virtual working directory data structures.
+ */
+void free_virtualworkdir(void);
+
+#endif
base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
--
2.19.1.gvfs.1.16.g9d1374d
^ permalink raw reply related
* Re: [PATCH 2/2] mingw: allow absolute paths without drive prefix
From: Johannes Sixt @ 2018-12-13 19:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Johannes Schindelin via GitGitGadget, git
In-Reply-To: <8efe7938-12a9-7db3-8f95-825ee2e32247@kdbg.org>
Am 13.12.18 um 07:25 schrieb Johannes Sixt:
> Am 13.12.18 um 03:48 schrieb Junio C Hamano:
>> So,... what's the conclusion? The patch in the context of my tree
>> would be a no-op, and we'd need a prerequisite change to the support
>> function to accompany this patch to be effective?
>
> Correct, that is my conclusion.
Moreover, there is no problem with your tree to begin with. I cloned
into a destination without a drive letter:
C:\>git clone R:\j6t\expat.git \Temp\expat
Cloning into '\Temp\expat'...
done.
and it works fine. If I understood the description in patch 1/2
correctly, this should have failed.
-- Hannes
^ 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