* 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
* Re: enhancement: support for author.email and author.name in "git config"
From: Jonathan Nieder @ 2018-12-13 19:37 UTC (permalink / raw)
To: William Hubbs
Cc: Johannes Schindelin, Martin Ågren, Git Mailing List,
chutzpah
In-Reply-To: <20181213192930.GA21006@whubbs1.gaikai.biz>
William Hubbs wrote:
> Is the git team on an irc channel somewhere,
Some like to use #git-devel on freenode.
> or should I start sending
> patches that I know are incomplete to the list?
Yeah, that's fine. Include [WIP/PATCH] in the subject line to set
appropriate expectations. :)
Thanks,
Jonathan
^ permalink raw reply
* Re: enhancement: support for author.email and author.name in "git config"
From: William Hubbs @ 2018-12-13 19:29 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Johannes Schindelin, Martin Ågren, Git Mailing List,
chutzpah
In-Reply-To: <20181207224428.GE73340@google.com>
Hi Jonathan,
On Fri, Dec 07, 2018 at 02:44:28PM -0800, Jonathan Nieder wrote:
> William Hubbs wrote:
> > On Thu, Dec 06, 2018 at 11:20:07PM +0100, Johannes Schindelin wrote:
>
> >> There *is* a way to get what you want that is super easy and will
> >> definitely work: if you sit down and do it ;-)
> >>
> >> Please let us know if you need any additional information before you can
> >> start.
> >
> > Which branch should I work off of in the repo?
>
> "master".
>
> Also, please make sure the documentation (e.g. in
> Documentation/config/user.txt) describes when a user would want to set
> this option.
Is the git team on an irc channel somewhere, or should I start sending
patches that I know are incomplete to the list?
I would like to work with someone on this since I'm not that familiar
with the git sources. :-)
Thanks,
William
^ permalink raw reply
* Re: Pre-commit hook does not work properly in recent version
From: Johannes Schindelin @ 2018-12-13 19:19 UTC (permalink / raw)
To: Andrew Kharchenko; +Cc: git
In-Reply-To: <678FE577-1708-4B7A-8EE6-EBE2514A4D04@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
Hi Andrew,
On Thu, 13 Dec 2018, Andrew Kharchenko wrote:
> I found a bug in Git v. 2.20.0 and 2.19.x at least, where pre-commit hook does not work properly.
>
> OS: macOS High Sierra
> Git: 2.20.0, 2.19.x
>
> Description: pre-commit file should be marked as executable in order to recreate the bug. When executing “git commit”, it silently exits without any message.
> It was found, that Git version 2.17.1 still works as expected.
Have you dug deeper? Like, add a `set -x` at the beginning of that hook to
find out why it is exiting early?
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] submodule update: run at most one fetch job unless otherwise set
From: Eric Sunshine @ 2018-12-13 19:04 UTC (permalink / raw)
To: Stefan Beller; +Cc: Git List, Junio C Hamano, sjon
In-Reply-To: <20181213190248.247083-1-sbeller@google.com>
On Thu, Dec 13, 2018 at 2:03 PM Stefan Beller <sbeller@google.com> wrote:
> In a028a1930c (fetching submodules: respect `submodule.fetchJobs`
> config option, 2016-02-29), we made sure to keep the default behavior
> of a fetching at most one submodule at once when not setting the
s/of a/of/
> newly introduced `submodule.fetchJobs` config.
>
> This regressed in 90efe595c5 (builtin/submodule--helper: factor
> out submodule updating, 2018-08-03). Fix it.
>
> Reported-by: Sjon Hortensius <sjon@parse.nl>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
^ permalink raw reply
* [PATCH] submodule update: run at most one fetch job unless otherwise set
From: Stefan Beller @ 2018-12-13 19:02 UTC (permalink / raw)
To: sbeller; +Cc: git, gitster, sjon
In-Reply-To: <CAGZ79kYsk8YEUUhMVF9fBC++fop3CPyobXTgHTuF2Lgikf9CJA@mail.gmail.com>
From: Junio C Hamano <gitster@pobox.com>
In a028a1930c (fetching submodules: respect `submodule.fetchJobs`
config option, 2016-02-29), we made sure to keep the default behavior
of a fetching at most one submodule at once when not setting the
newly introduced `submodule.fetchJobs` config.
This regressed in 90efe595c5 (builtin/submodule--helper: factor
out submodule updating, 2018-08-03). Fix it.
Reported-by: Sjon Hortensius <sjon@parse.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/submodule--helper.c | 2 +-
t/t5526-fetch-submodules.sh | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d38113a31a..1f8a4a9d52 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1551,7 +1551,7 @@ struct submodule_update_clone {
#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
NULL, NULL, NULL, \
- NULL, 0, 0, 0, NULL, 0, 0, 0}
+ NULL, 0, 0, 0, NULL, 0, 0, 1}
static void next_submodule_warn_missing(struct submodule_update_clone *suc,
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 6c2f9b2ba2..a0317556c6 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -524,6 +524,8 @@ test_expect_success 'fetching submodules respects parallel settings' '
git config fetch.recurseSubmodules true &&
(
cd downstream &&
+ GIT_TRACE=$(pwd)/trace.out git fetch &&
+ grep "1 tasks" trace.out &&
GIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&
grep "7 tasks" trace.out &&
git config submodule.fetchJobs 8 &&
--
2.20.0.rc2.230.gc28305e538
^ permalink raw reply related
* [PATCH v3] revision: use commit graph in get_reference()
From: Jonathan Tan @ 2018-12-13 18:54 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan, peff, stolee, gitster
In-Reply-To: <20181204224238.50966-1-jonathantanmy@google.com>
When fetching into a repository, a connectivity check is first made by
check_exist_and_connected() in builtin/fetch.c that runs:
git rev-list --objects --stdin --not --all --quiet <(list of objects)
If the client repository has many refs, this command can be slow,
regardless of the nature of the server repository or what is being
fetched. A profiler reveals that most of the time is spent in
setup_revisions() (approx. 60/63), and of the time spent in
setup_revisions(), most of it is spent in parse_object() (approx.
49/60). This is because setup_revisions() parses the target of every ref
(from "--all"), and parse_object() reads the buffer of the object.
Reading the buffer is unnecessary if the repository has a commit graph
and if the ref points to a commit (which is typically the case). This
patch uses the commit graph wherever possible; on my computer, when I
run the above command with a list of 1 object on a many-ref repository,
I get a speedup from 1.8s to 1.0s.
Another way to accomplish this effect would be to modify parse_object()
to use the commit graph if possible; however, I did not want to change
parse_object()'s current behavior of always checking the object
signature of the returned object.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
This patch is still on master. Junio, let me know if you would rather
have me base it on sb/more-repo-in-api instead.
I mentioned [1] that I would unify parse_commit_in_graph_one() and
parse_commit_in_graph() so that one documentation comment could cover
all the functionality, but with the simpler API, I decided not to do
that to minimize the diff.
Change in v3: Now uses a simpler API with the algorithm suggested by
Peff in [2], except that I also retain the existing optimization that
checks if graph_pos is already set.
[1] https://public-inbox.org/git/20181212195812.232726-1-jonathantanmy@google.com/
[2] https://public-inbox.org/git/20181213012707.GC26210@sigill.intra.peff.net/
---
commit-graph.c | 44 ++++++++++++++++++++++++++------------
commit-graph.h | 11 +++++-----
commit.c | 4 +++-
revision.c | 5 ++++-
t/helper/test-repository.c | 8 ++-----
5 files changed, 45 insertions(+), 27 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 40c855f185..0aca7ec0fe 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -286,7 +286,8 @@ void close_commit_graph(struct repository *r)
r->objects->commit_graph = NULL;
}
-static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos)
+static int bsearch_graph(struct commit_graph *g, const struct object_id *oid,
+ uint32_t *pos)
{
return bsearch_hash(oid->hash, g->chunk_oid_fanout,
g->chunk_oid_lookup, g->hash_len, pos);
@@ -374,24 +375,42 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin
}
}
-static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item)
+static struct commit *parse_commit_in_graph_one(struct repository *r,
+ struct commit_graph *g,
+ const struct object_id *oid)
{
+ struct object *obj;
+ struct commit *commit;
uint32_t pos;
- if (item->object.parsed)
- return 1;
+ obj = lookup_object(r, oid->hash);
+ commit = obj && obj->type == OBJ_COMMIT ? (struct commit *) obj : NULL;
+ if (commit && obj->parsed)
+ return commit;
- if (find_commit_in_graph(item, g, &pos))
- return fill_commit_in_graph(item, g, pos);
+ if (commit && commit->graph_pos != COMMIT_NOT_FROM_GRAPH)
+ pos = commit->graph_pos;
+ else if (bsearch_graph(g, oid, &pos))
+ ; /* bsearch_graph sets pos */
+ else
+ return NULL;
- return 0;
+ if (!commit) {
+ commit = lookup_commit(r, oid);
+ if (!commit)
+ return NULL;
+ }
+
+ fill_commit_in_graph(commit, g, pos);
+ return commit;
}
-int parse_commit_in_graph(struct repository *r, struct commit *item)
+struct commit *parse_commit_in_graph(struct repository *r,
+ const struct object_id *oid)
{
if (!prepare_commit_graph(r))
- return 0;
- return parse_commit_in_graph_one(r->objects->commit_graph, item);
+ return NULL;
+ return parse_commit_in_graph_one(r, r->objects->commit_graph, oid);
}
void load_commit_graph_info(struct repository *r, struct commit *item)
@@ -1004,8 +1023,6 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
}
for (i = 0; i < g->num_commits; i++) {
- struct commit *graph_commit;
-
hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
if (i && oidcmp(&prev_oid, &cur_oid) >= 0)
@@ -1024,8 +1041,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
cur_fanout_pos++;
}
- graph_commit = lookup_commit(r, &cur_oid);
- if (!parse_commit_in_graph_one(g, graph_commit))
+ if (!parse_commit_in_graph_one(r, g, &cur_oid))
graph_report("failed to parse %s from commit-graph",
oid_to_hex(&cur_oid));
}
diff --git a/commit-graph.h b/commit-graph.h
index 9db40b4d3a..b67aac1125 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -13,16 +13,17 @@ struct commit;
char *get_commit_graph_filename(const char *obj_dir);
/*
- * Given a commit struct, try to fill the commit struct info, including:
+ * If the given commit is in the commit graph, returns a commit struct
+ * including the following info:
* 1. tree object
* 2. date
* 3. parents.
*
- * Returns 1 if and only if the commit was found in the packed graph.
- *
- * See parse_commit_buffer() for the fallback after this call.
+ * If not, returns NULL. See parse_commit_buffer() for the fallback after this
+ * call.
*/
-int parse_commit_in_graph(struct repository *r, struct commit *item);
+struct commit *parse_commit_in_graph(struct repository *r,
+ const struct object_id *oid);
/*
* It is possible that we loaded commit contents from the commit buffer,
diff --git a/commit.c b/commit.c
index d13a7bc374..19ce5e34a2 100644
--- a/commit.c
+++ b/commit.c
@@ -456,7 +456,9 @@ int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_com
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(the_repository, item))
+ if (use_commit_graph &&
+ parse_commit_in_graph(the_repository, &item->object.oid) &&
+ item->object.parsed)
return 0;
buffer = read_object_file(&item->object.oid, &type, &size);
if (!buffer)
diff --git a/revision.c b/revision.c
index 13e0519c02..7f54f3b4c7 100644
--- a/revision.c
+++ b/revision.c
@@ -213,7 +213,10 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
{
struct object *object;
- object = parse_object(revs->repo, oid);
+ object = (struct object *) parse_commit_in_graph(revs->repo, oid);
+ if (!object)
+ object = parse_object(revs->repo, oid);
+
if (!object) {
if (revs->ignore_missing)
return object;
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index 6a84a53efb..35bfd1233d 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -20,9 +20,7 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,
if (repo_init(&r, gitdir, worktree))
die("Couldn't init repo");
- c = lookup_commit(&r, commit_oid);
-
- if (!parse_commit_in_graph(&r, c))
+ if (!(c = parse_commit_in_graph(&r, commit_oid)))
die("Couldn't parse commit");
printf("%"PRItime, c->date);
@@ -46,13 +44,11 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
if (repo_init(&r, gitdir, worktree))
die("Couldn't init repo");
- c = lookup_commit(&r, commit_oid);
-
/*
* get_commit_tree_in_graph does not automatically parse the commit, so
* parse it first.
*/
- if (!parse_commit_in_graph(&r, c))
+ if (!(c = parse_commit_in_graph(&r, commit_oid)))
die("Couldn't parse commit");
tree = get_commit_tree_in_graph(&r, c);
if (!tree)
--
2.19.0.271.gfe8321ec05.dirty
^ permalink raw reply related
* Re: 2.20.0 - Undocumented change in submodule update wrt # parallel jobs
From: Stefan Beller @ 2018-12-13 18:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sjon
In-Reply-To: <xmqqva3xecjz.fsf@gitster-ct.c.googlers.com>
On Thu, Dec 13, 2018 at 6:17 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Sjon Hortensius <sjon@parse.nl> writes:
>
> > When switching to 2.20 our `git submodule update' (which clones
> > through ssh) broke because our ssh-server rejected the ~20
> > simultaneous connections the git-client makes. This seems to be caused
> > by a (possibly unintended) change in
> > https://github.com/git/git/commit/90efe595c53f4bb1851371344c35eff71f604d2b
> > which removed the default of max_jobs=1
> >
> > While this can easily be fixed by configuring submodule.fetchJobs I
> > think this change should be documented - or reverted back to it's
> > previous default of 1
>
> The commit in question does not look like it _wanted_ to change the
> default; rather, it appears to me that it wanted to be bug-to-bug
> compatible with the original, and any such change of behaviour is
> entirely unintended.
Indeed.
> I think the attached may be sufficient to change the default
> max_jobs back to 1.
I think so, too. I can wrap it into a commit with a proper message.
>
> By the way, is there a place where we document that the default
> value for fetchjobs, when unconfigured, is 1?
`man git config`
submodule.fetchJobs
Specifies how many submodules are fetched/cloned at the
same time. A positive integer allows up to that number of
submodules fetched in parallel. A value of 0 will give some
reasonable default. If unset, it defaults to 1.
and that seems to be the only place, other places only reference
this place:
Documentation$ git grep submodule.fetch
config/submodule.txt:66:submodule.fetchJobs::
git-clone.txt:259: Defaults to the `submodule.fetchJobs` option.
git-submodule.txt:408: Defaults to the `submodule.fetchJobs` option.
The behavior of that seems to have been there since the beginning of
a028a1930c (fetching submodules: respect `submodule.fetchJobs`
config option, 2016-02-29)
> If we are not making
> such a concrete promise, then I would think it is OK to update the
> default without any fanfare, as long as we have good reasons to do
> so. For this particular one, however, as I already said, I do not
> think we wanted to change the default to unlimited or anything like
> that, so...
We definitely want the diff below as a proper patch.
> builtin/submodule--helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 789d00d87d..e8cdf84f1c 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1552,7 +1552,7 @@ struct submodule_update_clone {
> #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
> SUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \
> NULL, NULL, NULL, \
> - NULL, 0, 0, 0, NULL, 0, 0, 0}
> + NULL, 0, 0, 0, NULL, 0, 0, 1}
>
>
> static void next_submodule_warn_missing(struct submodule_update_clone *suc,
^ permalink raw reply
* Re: Preparing for 2.20.1 brown-paper-bag maintenance release
From: Duy Nguyen @ 2018-12-13 18:17 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, Git Mailing List, Johannes Schindelin,
Derrick Stolee
In-Reply-To: <CACsJy8AB0gQAvAWh3vtiSFnZWXtdvQdi4czBoR2B8TkECMrQtQ@mail.gmail.com>
On Thu, Dec 13, 2018 at 7:08 PM Duy Nguyen <pclouds@gmail.com> wrote:
> There's also a bug in my patch (-2 is already being used by
> parse_opt_unknown_cb and my patch will change behavior of git-blame at
> least in theory).
Ah no. Too many magic numbers in parse-options.c code. It's working
fine but I'll need to give these some names to avoid confusion in the
future.
--
Duy
^ permalink raw reply
* Re: Preparing for 2.20.1 brown-paper-bag maintenance release
From: Duy Nguyen @ 2018-12-13 18:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, Git Mailing List, Johannes Schindelin,
Derrick Stolee
In-Reply-To: <87r2el1q0g.fsf@evledraar.gmail.com>
On Thu, Dec 13, 2018 at 3:05 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Thu, Dec 13 2018, Junio C Hamano wrote:
>
> > Here is an excerpt from a draft edition of "What's cooking" report
> > for topics that are about an immediate 2.20.1 maintenance release,
> > with five topics that I plan to merge to 'next' and then to 'maint'
> > soonish (they're all marked as "Will merge to 'next' and then to
> > 'master'").
> >
> > They'll be in 'pu' but not in 'next' in today's pushout, but unless
> > I hear breakage reports in time, I am hoping to merge them to 'next'
> > during tomorrow's integration cycle, so that we can start the new
> > week with 2.20.1.
> > [...]
> > * nd/show-gitcomp-compilation-fix (2018-12-12) 1 commit
> > - parse-options: fix SunCC compiler warning
> >
> > Portability fix for a recent update to parse-options API.
>
> Since I reported this, just a clarification: Unlike 46c0eb5843
> ("files-backend.c: fix build error on Solaris", 2018-11-25) this one's
> not an error on suncc, just a warning (and we have 20-30 of those exact
> warnings in our code).
>
> So if you wanted to minimize 2.20.1 this could be held back, but also it
> looks obviously correct so it's fine that it makes it into that point
> release. Just FYI.
There's also a bug in my patch (-2 is already being used by
parse_opt_unknown_cb and my patch will change behavior of git-blame at
least in theory). I'll reroll later. I think it's better to hold it
back.
--
Duy
^ permalink raw reply
* [PATCH] Simplify handling of setup_git_directory_gently() failure cases.
From: Erin Dahlgren @ 2018-12-13 17:30 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Junio C Hamano, Erin Dahlgren
setup_git_directory_gently() expects two types of failures to
discover a git directory (e.g. .git/):
- GIT_DIR_HIT_CEILING: could not find a git directory in any
parent directories of the cwd.
- GIT_DIR_HIT_MOUNT_POINT: could not find a git directory in
any parent directories up to the mount point of the cwd.
Both cases are handled in a similar way, but there are misleading
and unimportant differences. In both cases, setup_git_directory_gently()
should:
- Die if we are not in a git repository. Otherwise:
- Set nongit_ok = 1, indicating that we are not in a git repository
but this is ok.
- Call strbuf_release() on any non-static struct strbufs that we
allocated.
Before this change are two misleading additional behaviors:
- GIT_DIR_HIT_CEILING: setup_nongit() changes to the cwd for no
apparent reason. We never had the chance to change directories
up to this point so chdir(current cwd) is pointless.
- GIT_DIR_HIT_MOUNT_POINT: strbuf_release() frees the buffer
of a static struct strbuf (cwd). This is unnecessary because the
struct is static so its buffer is always reachable. This is also
misleading because nowhere else in the function is this buffer
released.
This change eliminates these two misleading additional behaviors and
deletes setup_nogit() because the code is clearer without it. The
result is that we can see clearly that GIT_DIR_HIT_CEILING and
GIT_DIR_HIT_MOUNT_POINT lead to the same behavior (ignoring the
different help messages).
Thanks-to: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Erin Dahlgren <eedahlgren@gmail.com>
---
setup.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/setup.c b/setup.c
index 1be5037..b441e39 100644
--- a/setup.c
+++ b/setup.c
@@ -831,16 +831,6 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,
return NULL;
}
-static const char *setup_nongit(const char *cwd, int *nongit_ok)
-{
- if (!nongit_ok)
- die(_("not a git repository (or any of the parent directories): %s"), DEFAULT_GIT_DIR_ENVIRONMENT);
- if (chdir(cwd))
- die_errno(_("cannot come back to cwd"));
- *nongit_ok = 1;
- return NULL;
-}
-
static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_len)
{
struct stat buf;
@@ -1097,18 +1087,20 @@ const char *setup_git_directory_gently(int *nongit_ok)
prefix = setup_bare_git_dir(&cwd, dir.len, &repo_fmt, nongit_ok);
break;
case GIT_DIR_HIT_CEILING:
- prefix = setup_nongit(cwd.buf, nongit_ok);
- break;
+ if (!nongit_ok)
+ die(_("not a git repository (or any of the parent directories): %s"),
+ DEFAULT_GIT_DIR_ENVIRONMENT);
+ *nongit_ok = 1;
+ strbuf_release(&dir);
+ return NULL;
case GIT_DIR_HIT_MOUNT_POINT:
- if (nongit_ok) {
- *nongit_ok = 1;
- strbuf_release(&cwd);
- strbuf_release(&dir);
- return NULL;
- }
- die(_("not a git repository (or any parent up to mount point %s)\n"
- "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
- dir.buf);
+ if (!nongit_ok)
+ die(_("not a git repository (or any parent up to mount point %s)\n"
+ "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
+ dir.buf);
+ *nongit_ok = 1;
+ strbuf_release(&dir);
+ return NULL;
default:
BUG("unhandled setup_git_directory_1() result");
}
--
2.7.4
^ permalink raw reply related
* [wishlist] support of cloning recursively from non-bare submodule hierarchies?
From: Yaroslav Halchenko @ 2018-12-13 17:19 UTC (permalink / raw)
To: git
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
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
Cloning into 'famface'...
remote: Counting objects: 61, done.
remote: Compressing objects: 100% (54/54), done.
remote: Total 61 (delta 14), reused 0 (delta 0)
Unpacking objects: 100% (61/61), done.
Submodule 'data' (http://datasets.datalad.org/labs/gobbini/famface/.git/data) registered for path 'data'
Cloning into '/tmp/famface/data'...
fatal: repository 'http://datasets.datalad.org/labs/gobbini/famface/.git/data/' not found
fatal: clone of 'http://datasets.datalad.org/labs/gobbini/famface/.git/data' into submodule path '/tmp/famface/data' failed
Failed to clone 'data'. Retry scheduled
Cloning into '/tmp/famface/data'...
fatal: repository 'http://datasets.datalad.org/labs/gobbini/famface/.git/data/' not found
fatal: clone of 'http://datasets.datalad.org/labs/gobbini/famface/.git/data' into submodule path '/tmp/famface/data' failed
Failed to clone 'data' a second time, aborting
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/:
10.31.188.88 - - [13/Dec/2018:12:18:38 -0500] "GET /labs/gobbini/famface/.git/info/refs?service=git-upload-pack HTTP/1.1" 200 681 "-" "git/2.20.0.rc2.403.gdbc3b29805"
10.31.188.88 - - [13/Dec/2018:12:18:38 -0500] "POST /labs/gobbini/famface/.git/git-upload-pack HTTP/1.1" 200 69276 "-" "git/2.20.0.rc2.403.gdbc3b29805"
==> datasets.datalad.org-error.log <==
[Thu Dec 13 12:18:38.673447 2018] [core:info] [pid 7570:tid 140683541153536] [client 10.31.188.88:32794] AH00128: File does not exist: /srv/datasets.datalad.org/www/labs/gobbini/famface/.git/data/info/refs
==> datasets.datalad.org-access-comb.log <==
10.31.188.88 - - [13/Dec/2018:12:18:38 -0500] "GET /labs/gobbini/famface/.git/data/info/refs?service=git-upload-pack HTTP/1.1" 404 485 "-" "git/2.20.0.rc2.403.gdbc3b29805"
==> datasets.datalad.org-error.log <==
[Thu Dec 13 12:18:38.689277 2018] [core:info] [pid 7572:tid 140683574724352] [client 10.31.188.88:32796] AH00128: File does not exist: /srv/datasets.datalad.org/www/labs/gobbini/famface/.git/data/info/refs
==> datasets.datalad.org-access-comb.log <==
10.31.188.88 - - [13/Dec/2018:12:18:38 -0500] "GET /labs/gobbini/famface/.git/data/info/refs?service=git-upload-pack HTTP/1.1" 404 485 "-" "git/2.20.0.rc2.403.gdbc3b29805"
--
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] Re: [wishlist] submodule.update config
From: Yaroslav Halchenko @ 2018-12-13 16:50 UTC (permalink / raw)
To: git
In-Reply-To: <CAGZ79kaka_DTUMkGdSbYW7Vam3XSWcdqxPrzFDXZJSsQC1zHYQ@mail.gmail.com>
On Wed, 12 Dec 2018, Stefan Beller wrote:
> > But again, I must confess, that either I forgot or just do not see a
> > clear use-case/demand for submodule.update config myself any longer,
> ok, let's drop that patch then.
ok, But I will cherish it in my memory so whenever the use case
comes back to me -- I will be back too ;)
> > Probably I need to try "submodules update --merge" to see what is that
> > rough edge which makes it different from the potential "merge
> > --recurse-submodules", or is it easy to describe? ;-)
> I think the branch handling would be the difference. I'd expect
> "merge --recurse-submodules" to be sensible about staying on
> the branch both in the superproject and submodule, whereas
> "submodule update --merge" is too much plumbing, that we'd
> expect a sensible branch handling (detached HEAD is just fine,
> right?)
re "detached HEAD is just fine" -- I guess "it depends"... E.g. why
should it get detached if it was not detached to start with? Why not
just to perform a regular "git merge --recurse-submodules" within
the submodule thus making it all consistent across?
If there is a need in detached HEADs handling of merges etc, get
them detached and then they would stay detached - no surprises.
> The merge result would be the same, I'd think.
it better be ;)
> > I wonder if may be instead of pestering you about this config one, I
> > should ask about pointers on how to accomplish "revert
> > --recurse-submodules"
> What do you want to do in revert --recurse-submodules?
> When you have "revert --recurse-submodules $COMMIT",
> would that revert all submodule commits introduced in
> that commit as well as the regular superproject revert?
That is correct
> This would require either opening multiple editors
> (once per submodule and at last for the superproject)
> or we'd have to do fancy snip-snapping of the user input,
> e.g. providing a template like:
> Revert "$title"
> This reverts commit $COMMIT.
> # The above is for the superproject commit
> # Please enter the commit message ...
> # Changes to be committed:
> # ...
> # --8<-- DO NOT DELETE THIS LINE
> # Below is the commit for submodule $submodule:
> Revert $submodule_range
> This reverts commits $maybe_many
> # The above is for the submodule commit
> # Please ...
> I guess it may be easier to just have multiple
> editors opened sequentially to give a commit
> message.
yeap - that would be beautiful. Now I just need to do that all manually
;)
> > or where to poke to make it possible to clone
> > recursively from http://datasets.datalad.org/ where we do not place
> > submodules all under the very top /.git/modules ;-)
> Not sure what you mean there?
sorry I was not clear... I will start a new thread for a complete
description.
--
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 2/2] RF+ENH(TST): compare the entire list of submodule status --recursive to stay intact
From: Yaroslav O Halchenko @ 2018-12-13 16:42 UTC (permalink / raw)
To: git
In-Reply-To: <CAGZ79kY17gmEh5Sawa+1fG5cXjOReOgCjDyEmGbbpJ5EE1APdw@mail.gmail.com>
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!
BTW -- should I just squash to PATCHes now? I kept them separate primarily to
show the use of those helpers:
> > ) &&
> > (cd super &&
> > (cd submodule &&
> > @@ -307,16 +318,28 @@ test_expect_success 'submodule update --merge staying on master' '
> > '
> > test_expect_success 'submodule update --reset-hard staying on master' '
> > [..]
> > +'
> > +
> The tests look good to me, though I wonder if we'd rather want to inline
> {record/compare}_submodule_status as then you'd not need to look it up
> and the functions are rather short?
compare_submodules_status is already a compound action, so code would
become quite more "loaded" if it is expanded, e.g. instead of
(cd super &&
record_submodules_status &&
(cd submodule &&
git reset --hard HEAD~1
) &&
! compare_submodules_status &&
git submodule update --reset-hard submodule &&
compare_submodules_status
)
it would become something like this I guess?
(cd super &&
git submodule status --recursive >expect &&
(cd submodule &&
git reset --hard HEAD~1
) &&
! {git submodule status --recursive >actual &&
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.
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)
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"' \
'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?
--
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 v2 1/2] .gitattributes: ensure t/oid-info/* has eol=lf
From: Derrick Stolee @ 2018-12-13 16:22 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, Derrick Stolee, SZEDER Gábor
In-Reply-To: <xmqqva3ygnh3.fsf@gitster-ct.c.googlers.com>
On 12/12/2018 9:38 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Derrick Stolee <dstolee@microsoft.com>
>>
>> The new test_oid machinery in the test library requires reading
>> some information from t/oid-info/hash-info and t/oid-info/oid.
>> The shell logic that reads from these files is sensitive to CRLF
>> line endings, causing a problem when the test suite is run on a
>> Windows machine that converts LF to CRLF.
>>
>> Exclude the files in this folder from this conversion.
>>
>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>> ---
> It seems that this step is identical to v1, to which SZEDER Gábor
> had trouble with (cf. <20181212133945.GV30222@szeder.dev>). I am
> guessing that the review and v2 e-mails have crossed.
>
> FWIW, I personally do not think "is sensitive to CRLF" is too bad,
> as my attempt to clarify it does not make it much better, e.g.
>
> The logic to read from these files in shell uses built-in
> "read" command, which leaves CR at the end of these text
> files when they are checked out with CRLF line endings, at
> least when run with bash shipped with Git for Windows. This
> results in an unexpected value in the variable these lines
> are read into, leading the tests to fail.
>
> So, I'll keep what I queued when I received v1 for now.
Sorry for missing the edit to the message. You are correct that v2 just
added one commit.
I like your rewording, if you feel like editing it.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH on master v2] revision: use commit graph in get_reference()
From: Derrick Stolee @ 2018-12-13 16:20 UTC (permalink / raw)
To: Jeff King, Jonathan Tan; +Cc: gitster, git
In-Reply-To: <20181213012707.GC26210@sigill.intra.peff.net>
On 12/12/2018 8:27 PM, Jeff King wrote:
> On Wed, Dec 12, 2018 at 11:58:12AM -0800, Jonathan Tan wrote:
>
>>> Yeah, this was the part that took me a bit to figure out, as well. The
>>> optimization here is really just avoiding a call to lookup_commit(),
>>> which will do a single hash-table lookup. I wonder if that's actually
>>> worth this more complex interface (as opposed to just always taking an
>>> oid and then always returning a "struct commit", which could be old or
>>> new).
>> Avoidance of lookup_commit() is more important than an optimization, I
>> think. Here, we call lookup_commit() only when we know that that object
>> is a commit (by its presence in a commit graph). If we just called it
>> blindly, we might mistakenly create a commit for that hash when it is
>> actually an object of another type. (We could inline lookup_commit() in
>> parse_commit_in_graph_one(), removing the object creation part, but that
>> adds complexity as well.)
> I was thinking we would only do so in the happy path when we find a
> commit. I.e., something like:
>
> obj = lookup_object(oid); /* does not auto-vivify */
> if (obj && obj->parsed)
> return obj;
>
> if (we_have_it_in_commit_graph) {
> commit = obj || lookup_commit(oid);
> fill_in_details_from_commit_graph(commit);
> return &commit->obj;
> } else {
> return parse_object(oid);
> }
>
> which is more along the lines of that parse_probably_commit() that
> Stolee mentioned.
This approach is what I had in mind. Thanks for making it more concrete!
-Stolee
^ permalink raw reply
* Re: [PATCHSET] git-reverse-trailer-xrefs: Reverse map cherry-picks and other cross-references
From: Tejun Heo @ 2018-12-13 16:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, kernel-team, Stefan Xenos
In-Reply-To: <xmqqftv2f05j.fsf@gitster-ct.c.googlers.com>
Hello, Junio.
On Thu, Dec 13, 2018 at 02:47:36PM +0900, Junio C Hamano wrote:
> Tejun Heo <tj@kernel.org> writes:
>
> > Hmmm... I see. I still have a bit of trouble seeing why doing it that
> > way is better tho. Wouldn't new-object-hook be simpler? They'll
> > achieve about the same thing but one would need to keep the states in
> > two places.
>
> The new-object-hook thing will not have keep the states. Whenever
> Git realizes that it created a new object, it must call that hook,
> and at that point in time, without keeping any state, it knows which
> objects are what it has just created. So "in two places" is not a
Yeap, that's what I was trying to say.
> problem at all. There is only one place (i.e. the place the sweeper
> would record what it just did to communicate with its future
> invocation).
>
> A new-object-hook that will always be called any time a new object
> enters the picture would be a nightmare to maintain up-to-date. One
But I didn't know this. I probably am too naive in thinking so but
it's a bit surprising that there's no single / few chokepoints for
repo updates that we can attach to.
> missed codepath and your coverage will have holes. Having a back-up
> mechanism to sweep for new objects will give you a better chance of
> staying correct as the system evolves, I'd think.
The duplicate state tracking still bothers me a bit (duplicate in the
sense that the end results are the same) especially as this would mean
any similar mechanism would need to track their own states too, but I
really don't have the full (not even close) picture here and it can
easily be me missing something.
Anyways, I'll wait for Stefan to chime in.
Thanks.
--
tejun
^ permalink raw reply
* RE: [Question] Complex textconv text
From: Randall S. Becker @ 2018-12-13 16:14 UTC (permalink / raw)
To: git
In-Reply-To: <004501d492f5$98f0fcc0$cad2f640$@nexbridge.com>
On December 13, 2018 10:08, I wrote:
> I have a strange situation and need help with resolving funky characters
in
> .git/config. My situation is this:
>
> [diff "*.dat"]
> textconv = enscribe-conv
> --format=-a1\(A=-a1,-a16,-a32\|P=-a1,-a32,-a16\|=-a1,-d,a14\),-a224
>
> Basically this is a formatter for diff so that I can show structured
binary data.
> The unquoted syntax of the format string is:
> --format=-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224
>
> Content is not really important. The issue is that git is reporting fatal:
> bad config line 2 in file .git/config when I escape the (, ), and |
characters. I
> get syntax errors otherwise from the shell running the textconv. I have
tried -
> -format="-a1(A=-a1,-a16,-a32|P=-a1,-a32,-a16|=-a1,-d,a14),-a224", to no
> avail. How should I safely escape the characters in here?
Easiest bypass was to wrap the mess above into a shell script and execute it
that way, which works just fine. I'm not sure it's possible to escape all of
the special characters. Seems like the safest approach anyway.
Cheers,
Randall
^ permalink raw reply
* Re: [PATCH v2 8/8] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 16:08 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan
In-Reply-To: <20181213155817.27666-9-avarab@gmail.com>
On Thu, Dec 13 2018, Ævar Arnfjörð Bjarmason wrote:
Now that we have this maybe we should discuss why these tests show
different things:
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 086f2c40f6..8b1217ea26 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -628,7 +628,10 @@ test_expect_success 'fetch-pack cannot fetch a raw sha1 that is not advertised a
> test_commit -C server 6 &&
>
> git init client &&
> - test_must_fail git -C client fetch-pack ../server \
> +
> + # Other protocol versions (e.g. 2) allow fetching an unadvertised
> + # object, so run this test with the default protocol version (0).
> + test_must_fail env GIT_TEST_PROTOCOL_VERSION= git -C client fetch-pack ../server \
> $(git -C server rev-parse refs/heads/master^) 2>err &&
What? So the equivalent of uploadpack.allowAnySHA1InWant=true is on for
v2 all the time?
> test_i18ngrep "Server does not allow request for unadvertised object" err
> '
> @@ -788,7 +791,7 @@ test_expect_success 'shallow clone exclude tag two' '
> '
>
> test_expect_success 'fetch exclude tag one' '
> - git -C shallow12 fetch --shallow-exclude one origin &&
> + GIT_TEST_PROTOCOL_VERSION= git -C shallow12 fetch --shallow-exclude one origin &&
> git -C shallow12 log --pretty=tformat:%s origin/master >actual &&
> test_write_lines three two >expected &&
> test_cmp expected actual
> @@ -806,7 +809,7 @@ test_expect_success 'fetching deepen' '
> git -C deepen log --pretty=tformat:%s master >actual &&
> echo three >expected &&
> test_cmp expected actual &&
> - git -C deepen fetch --deepen=1 &&
> + GIT_TEST_PROTOCOL_VERSION= git -C deepen fetch --deepen=1 &&
> git -C deepen log --pretty=tformat:%s origin/master >actual &&
> cat >expected <<-\EOF &&
> four
> diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
> index 4ca48f0276..0e90a90294 100755
> --- a/t/t5503-tagfollow.sh
> +++ b/t/t5503-tagfollow.sh
> @@ -56,7 +56,7 @@ test_expect_success 'fetch A (new commit : 1 connection)' '
> rm -f $U &&
> (
> cd cloned &&
> - GIT_TRACE_PACKET=$UPATH git fetch &&
> + GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
> test $A = $(git rev-parse --verify origin/master)
> ) &&
> get_needs $U >actual &&
> @@ -86,7 +86,7 @@ test_expect_success 'fetch C, T (new branch, tag : 1 connection)' '
> rm -f $U &&
> (
> cd cloned &&
> - GIT_TRACE_PACKET=$UPATH git fetch &&
> + GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
> test $C = $(git rev-parse --verify origin/cat) &&
> test $T = $(git rev-parse --verify tag1) &&
> test $A = $(git rev-parse --verify tag1^0)
> @@ -122,7 +122,7 @@ test_expect_success 'fetch B, S (commit and tag : 1 connection)' '
> rm -f $U &&
> (
> cd cloned &&
> - GIT_TRACE_PACKET=$UPATH git fetch &&
> + GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
> test $B = $(git rev-parse --verify origin/master) &&
> test $B = $(git rev-parse --verify tag2^0) &&
> test $S = $(git rev-parse --verify tag2)
> @@ -146,7 +146,7 @@ test_expect_success 'new clone fetch master and tags' '
> cd clone2 &&
> git init &&
> git remote add origin .. &&
> - GIT_TRACE_PACKET=$UPATH git fetch &&
> + GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
> test $B = $(git rev-parse --verify origin/master) &&
> test $S = $(git rev-parse --verify tag2) &&
> test $B = $(git rev-parse --verify tag2^0) &&
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index ca69636fd5..7b480587c9 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -223,7 +223,7 @@ test_expect_success 'ls-remote --symref' '
> $(git rev-parse refs/tags/mark1.10) refs/tags/mark1.10
> $(git rev-parse refs/tags/mark1.2) refs/tags/mark1.2
> EOF
> - git ls-remote --symref >actual &&
> + GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref >actual &&
> test_cmp expect actual
> '
>
> @@ -243,7 +243,7 @@ test_expect_failure 'ls-remote with filtered symref (--heads)' '
> 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/foo
> 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/master
> EOF
> - git ls-remote --symref --heads . >actual &&
> + GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
> test_cmp expect actual
> '
>
> @@ -252,9 +252,9 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
> 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/foo
> 1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/master
> EOF
> - git ls-remote --symref --heads . >actual &&
> + GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
> test_cmp expect actual &&
> - git ls-remote --symref . "refs/heads/*" >actual &&
> + GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref . "refs/heads/*" >actual &&
> test_cmp expect actual
> '
>
> diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
> index 36b0dbc01c..f095555c3e 100755
> --- a/t/t5515-fetch-merge-logic.sh
> +++ b/t/t5515-fetch-merge-logic.sh
This one should really be looked at by someone more familiar with
v2. Looks scary that we have different merge results with it.
> @@ -147,7 +147,7 @@ do
> do
> git update-ref -d "$refname" "$val"
> done
> - git fetch "$@" >/dev/null
> + GIT_TEST_PROTOCOL_VERSION= git fetch "$@" >/dev/null
> cat .git/FETCH_HEAD
> } >"$actual_f" &&
> git show-ref >"$actual_r" &&
> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
> index 08cdee0b95..1d1b717cd5 100755
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -1129,7 +1129,7 @@ do
> '
> done
>
> -test_expect_success 'fetch exact SHA1' '
> +test_expect_success 'fetch exact SHA1 in protocol v0' '
> mk_test testrepo heads/master hidden/one &&
> git push testrepo master:refs/hidden/one &&
> (
> @@ -1148,7 +1148,8 @@ test_expect_success 'fetch exact SHA1' '
> test_must_fail git cat-file -t $the_commit &&
>
> # fetching the hidden object should fail by default
> - test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
> + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
> + git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
> test_i18ngrep "Server does not allow request for unadvertised object" err &&
> test_must_fail git rev-parse --verify refs/heads/copy &&
>
> @@ -1205,7 +1206,8 @@ do
> mk_empty shallow &&
> (
> cd shallow &&
> - test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
> + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
> + git fetch --depth=1 ../testrepo/.git $SHA1 &&
> git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
> git fetch --depth=1 ../testrepo/.git $SHA1 &&
> git cat-file commit $SHA1
> @@ -1233,15 +1235,18 @@ do
> mk_empty shallow &&
> (
> cd shallow &&
> - test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3 &&
> - test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_1 &&
> + test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
> + git fetch ../testrepo/.git $SHA1_3 &&
> + test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
> + git fetch ../testrepo/.git $SHA1_1 &&
> git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
> git fetch ../testrepo/.git $SHA1_1 &&
> git cat-file commit $SHA1_1 &&
> test_must_fail git cat-file commit $SHA1_2 &&
> git fetch ../testrepo/.git $SHA1_2 &&
> git cat-file commit $SHA1_2 &&
> - test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3
> + test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
> + git fetch ../testrepo/.git $SHA1_3
> )
> '
Ditto all this stuff.
> [...]
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index e87164aa8f..a555e38845 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -943,7 +943,8 @@ test_expect_success 'submodule update clone shallow submodule outside of depth'
> cd super3 &&
> sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
> mv -f .gitmodules.tmp .gitmodules &&
> - test_must_fail git submodule update --init --depth=1 2>actual &&
> + test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
> + git submodule update --init --depth=1 2>actual &&
> test_i18ngrep "Direct fetching of that commit failed." actual &&
> git -C ../submodule config uploadpack.allowReachableSHA1InWant true &&
> git submodule update --init --depth=1 >actual &&
And this one and various other shallow things look odd, are shallow
fetches different under v2?
^ permalink raw reply
* [PATCH v2 8/8] tests: mark tests broken under GIT_TEST_PROTOCOL_VERSION=2
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
Mark those tests that have behavior differences or bugs under
protocol.version=2.
Whether or not these tests should exhibit different behavior is
outside the scope of this change. Some (such as t5500-fetch-pack.sh)
are intentionally different, but others (such as
t7406-submodule-update.sh and t5515-fetch-merge-logic.sh) might
indicate bugs in the protocol v2 code.
Tracking down which is which is outside the scope of this
change. Let's first exhaustively annotate where the differences are,
so that we can spot future behavior differences or regressions.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
t/t5500-fetch-pack.sh | 9 ++++++---
t/t5503-tagfollow.sh | 8 ++++----
t/t5512-ls-remote.sh | 8 ++++----
t/t5515-fetch-merge-logic.sh | 2 +-
t/t5516-fetch-push.sh | 17 +++++++++++------
t/t5537-fetch-shallow.sh | 3 ++-
t/t5539-fetch-http-shallow.sh | 9 +++++----
t/t5541-http-push-smart.sh | 9 +++++++--
t/t5551-http-fetch-smart.sh | 19 +++++++++++--------
t/t5552-skipping-fetch-negotiator.sh | 4 ++--
t/t5570-git-daemon.sh | 2 +-
t/t7406-submodule-update.sh | 3 ++-
12 files changed, 56 insertions(+), 37 deletions(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 086f2c40f6..8b1217ea26 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -628,7 +628,10 @@ test_expect_success 'fetch-pack cannot fetch a raw sha1 that is not advertised a
test_commit -C server 6 &&
git init client &&
- test_must_fail git -C client fetch-pack ../server \
+
+ # Other protocol versions (e.g. 2) allow fetching an unadvertised
+ # object, so run this test with the default protocol version (0).
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= git -C client fetch-pack ../server \
$(git -C server rev-parse refs/heads/master^) 2>err &&
test_i18ngrep "Server does not allow request for unadvertised object" err
'
@@ -788,7 +791,7 @@ test_expect_success 'shallow clone exclude tag two' '
'
test_expect_success 'fetch exclude tag one' '
- git -C shallow12 fetch --shallow-exclude one origin &&
+ GIT_TEST_PROTOCOL_VERSION= git -C shallow12 fetch --shallow-exclude one origin &&
git -C shallow12 log --pretty=tformat:%s origin/master >actual &&
test_write_lines three two >expected &&
test_cmp expected actual
@@ -806,7 +809,7 @@ test_expect_success 'fetching deepen' '
git -C deepen log --pretty=tformat:%s master >actual &&
echo three >expected &&
test_cmp expected actual &&
- git -C deepen fetch --deepen=1 &&
+ GIT_TEST_PROTOCOL_VERSION= git -C deepen fetch --deepen=1 &&
git -C deepen log --pretty=tformat:%s origin/master >actual &&
cat >expected <<-\EOF &&
four
diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh
index 4ca48f0276..0e90a90294 100755
--- a/t/t5503-tagfollow.sh
+++ b/t/t5503-tagfollow.sh
@@ -56,7 +56,7 @@ test_expect_success 'fetch A (new commit : 1 connection)' '
rm -f $U &&
(
cd cloned &&
- GIT_TRACE_PACKET=$UPATH git fetch &&
+ GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
test $A = $(git rev-parse --verify origin/master)
) &&
get_needs $U >actual &&
@@ -86,7 +86,7 @@ test_expect_success 'fetch C, T (new branch, tag : 1 connection)' '
rm -f $U &&
(
cd cloned &&
- GIT_TRACE_PACKET=$UPATH git fetch &&
+ GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
test $C = $(git rev-parse --verify origin/cat) &&
test $T = $(git rev-parse --verify tag1) &&
test $A = $(git rev-parse --verify tag1^0)
@@ -122,7 +122,7 @@ test_expect_success 'fetch B, S (commit and tag : 1 connection)' '
rm -f $U &&
(
cd cloned &&
- GIT_TRACE_PACKET=$UPATH git fetch &&
+ GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
test $B = $(git rev-parse --verify origin/master) &&
test $B = $(git rev-parse --verify tag2^0) &&
test $S = $(git rev-parse --verify tag2)
@@ -146,7 +146,7 @@ test_expect_success 'new clone fetch master and tags' '
cd clone2 &&
git init &&
git remote add origin .. &&
- GIT_TRACE_PACKET=$UPATH git fetch &&
+ GIT_TRACE_PACKET=$UPATH GIT_TEST_PROTOCOL_VERSION= git fetch &&
test $B = $(git rev-parse --verify origin/master) &&
test $S = $(git rev-parse --verify tag2) &&
test $B = $(git rev-parse --verify tag2^0) &&
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index ca69636fd5..7b480587c9 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -223,7 +223,7 @@ test_expect_success 'ls-remote --symref' '
$(git rev-parse refs/tags/mark1.10) refs/tags/mark1.10
$(git rev-parse refs/tags/mark1.2) refs/tags/mark1.2
EOF
- git ls-remote --symref >actual &&
+ GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref >actual &&
test_cmp expect actual
'
@@ -243,7 +243,7 @@ test_expect_failure 'ls-remote with filtered symref (--heads)' '
1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/foo
1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/master
EOF
- git ls-remote --symref --heads . >actual &&
+ GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
test_cmp expect actual
'
@@ -252,9 +252,9 @@ test_expect_success 'ls-remote --symref omits filtered-out matches' '
1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/foo
1bd44cb9d13204b0fe1958db0082f5028a16eb3a refs/heads/master
EOF
- git ls-remote --symref --heads . >actual &&
+ GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref --heads . >actual &&
test_cmp expect actual &&
- git ls-remote --symref . "refs/heads/*" >actual &&
+ GIT_TEST_PROTOCOL_VERSION= git ls-remote --symref . "refs/heads/*" >actual &&
test_cmp expect actual
'
diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 36b0dbc01c..f095555c3e 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -147,7 +147,7 @@ do
do
git update-ref -d "$refname" "$val"
done
- git fetch "$@" >/dev/null
+ GIT_TEST_PROTOCOL_VERSION= git fetch "$@" >/dev/null
cat .git/FETCH_HEAD
} >"$actual_f" &&
git show-ref >"$actual_r" &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 08cdee0b95..1d1b717cd5 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1129,7 +1129,7 @@ do
'
done
-test_expect_success 'fetch exact SHA1' '
+test_expect_success 'fetch exact SHA1 in protocol v0' '
mk_test testrepo heads/master hidden/one &&
git push testrepo master:refs/hidden/one &&
(
@@ -1148,7 +1148,8 @@ test_expect_success 'fetch exact SHA1' '
test_must_fail git cat-file -t $the_commit &&
# fetching the hidden object should fail by default
- test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
test_i18ngrep "Server does not allow request for unadvertised object" err &&
test_must_fail git rev-parse --verify refs/heads/copy &&
@@ -1205,7 +1206,8 @@ do
mk_empty shallow &&
(
cd shallow &&
- test_must_fail git fetch --depth=1 ../testrepo/.git $SHA1 &&
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ git fetch --depth=1 ../testrepo/.git $SHA1 &&
git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
git fetch --depth=1 ../testrepo/.git $SHA1 &&
git cat-file commit $SHA1
@@ -1233,15 +1235,18 @@ do
mk_empty shallow &&
(
cd shallow &&
- test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3 &&
- test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_1 &&
+ test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+ git fetch ../testrepo/.git $SHA1_3 &&
+ test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+ git fetch ../testrepo/.git $SHA1_1 &&
git --git-dir=../testrepo/.git config uploadpack.allowreachablesha1inwant true &&
git fetch ../testrepo/.git $SHA1_1 &&
git cat-file commit $SHA1_1 &&
test_must_fail git cat-file commit $SHA1_2 &&
git fetch ../testrepo/.git $SHA1_2 &&
git cat-file commit $SHA1_2 &&
- test_must_fail ok=sigpipe git fetch ../testrepo/.git $SHA1_3
+ test_must_fail ok=sigpipe env GIT_TEST_PROTOCOL_VERSION= \
+ git fetch ../testrepo/.git $SHA1_3
)
'
done
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 6faf17e17a..85b3022ce6 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -127,7 +127,8 @@ test_expect_success 'fetch that requires changes in .git/shallow is filtered' '
git init notshallow &&
(
cd notshallow &&
- git fetch ../shallow/.git refs/heads/*:refs/remotes/shallow/*&&
+ GIT_TEST_PROTOCOL_VERSION= \
+ git fetch ../shallow/.git refs/heads/*:refs/remotes/shallow/* &&
git for-each-ref --format="%(refname)" >actual.refs &&
cat <<EOF >expect.refs &&
refs/remotes/shallow/no-shallow
diff --git a/t/t5539-fetch-http-shallow.sh b/t/t5539-fetch-http-shallow.sh
index 5fbf67c446..a121e19bc7 100755
--- a/t/t5539-fetch-http-shallow.sh
+++ b/t/t5539-fetch-http-shallow.sh
@@ -67,7 +67,8 @@ test_expect_success 'no shallow lines after receiving ACK ready' '
cd clone &&
git checkout --orphan newnew &&
test_commit new-too &&
- GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" git fetch --depth=2 &&
+ GIT_TRACE_PACKET="$TRASH_DIRECTORY/trace" GIT_TEST_PROTOCOL_VERSION= \
+ git fetch --depth=2 &&
grep "fetch-pack< ACK .* ready" ../trace &&
! grep "fetch-pack> done" ../trace
)
@@ -114,7 +115,7 @@ test_expect_success 'shallow clone exclude tag two' '
'
test_expect_success 'fetch exclude tag one' '
- git -C shallow12 fetch --shallow-exclude one origin &&
+ GIT_TEST_PROTOCOL_VERSION= git -C shallow12 fetch --shallow-exclude one origin &&
git -C shallow12 log --pretty=tformat:%s origin/master >actual &&
test_write_lines three two >expected &&
test_cmp expected actual
@@ -128,14 +129,14 @@ test_expect_success 'fetching deepen' '
test_commit two &&
test_commit three &&
mv .git "$HTTPD_DOCUMENT_ROOT_PATH/shallow-deepen.git" &&
- git clone --depth 1 $HTTPD_URL/smart/shallow-deepen.git deepen &&
+ GIT_TEST_PROTOCOL_VERSION= git clone --depth 1 $HTTPD_URL/smart/shallow-deepen.git deepen &&
mv "$HTTPD_DOCUMENT_ROOT_PATH/shallow-deepen.git" .git &&
test_commit four &&
git -C deepen log --pretty=tformat:%s master >actual &&
echo three >expected &&
test_cmp expected actual &&
mv .git "$HTTPD_DOCUMENT_ROOT_PATH/shallow-deepen.git" &&
- git -C deepen fetch --deepen=1 &&
+ GIT_TEST_PROTOCOL_VERSION= git -C deepen fetch --deepen=1 &&
git -C deepen log --pretty=tformat:%s origin/master >actual &&
cat >expected <<-\EOF &&
four
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 5475afc052..180c9005b7 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -45,14 +45,19 @@ test_expect_success 'no empty path components' '
# In the URL, add a trailing slash, and see if git appends yet another
# slash.
cd "$ROOT_PATH" &&
- git clone $HTTPD_URL/smart/test_repo.git/ test_repo_clone &&
+ # Other protocol versions may make different requests, so perform this
+ # clone with the default protocol.
+ GIT_TEST_PROTOCOL_VERSION= git clone $HTTPD_URL/smart/test_repo.git/ test_repo_clone &&
check_access_log exp
'
test_expect_success 'clone remote repository' '
rm -rf test_repo_clone &&
- git clone $HTTPD_URL/smart/test_repo.git test_repo_clone &&
+ # Other protocol versions may make different requests, so perform this
+ # clone with the default protocol.
+ GIT_TEST_PROTOCOL_VERSION= git clone $HTTPD_URL/smart/test_repo.git test_repo_clone &&
+
(
cd test_repo_clone && git config push.default matching
)
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 8630b0cc39..a51993f35a 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -43,7 +43,8 @@ test_expect_success 'clone http repository' '
< Cache-Control: no-cache, max-age=0, must-revalidate
< Content-Type: application/x-git-upload-pack-result
EOF
- GIT_TRACE_CURL=true git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
+ GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION= \
+ git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err &&
test_cmp file clone/file &&
tr '\''\015'\'' Q <err |
sed -e "
@@ -92,7 +93,7 @@ test_expect_success 'fetch changes via http' '
echo content >>file &&
git commit -a -m two &&
git push public &&
- (cd clone && git pull) &&
+ (cd clone && GIT_TEST_PROTOCOL_VERSION= git pull) &&
test_cmp file clone/file
'
@@ -143,7 +144,7 @@ test_expect_success 'clone from auth-only-for-push repository' '
test_expect_success 'clone from auth-only-for-objects repository' '
echo two >expect &&
set_askpass user@host pass@host &&
- git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth &&
+ GIT_TEST_PROTOCOL_VERSION= git clone --bare "$HTTPD_URL/auth-fetch/smart/repo.git" half-auth &&
expect_askpass both user@host &&
git --git-dir=half-auth log -1 --format=%s >actual &&
test_cmp expect actual
@@ -151,7 +152,7 @@ test_expect_success 'clone from auth-only-for-objects repository' '
test_expect_success 'no-op half-auth fetch does not require a password' '
set_askpass wrong &&
- git --git-dir=half-auth fetch &&
+ GIT_TEST_PROTOCOL_VERSION= git --git-dir=half-auth fetch &&
expect_askpass none
'
@@ -187,7 +188,7 @@ test_expect_success 'create namespaced refs' '
'
test_expect_success 'smart clone respects namespace' '
- git clone "$HTTPD_URL/smart_namespace/repo.git" ns-smart &&
+ GIT_TEST_PROTOCOL_VERSION= git clone "$HTTPD_URL/smart_namespace/repo.git" ns-smart &&
echo namespaced >expect &&
git --git-dir=ns-smart/.git log -1 --format=%s >actual &&
test_cmp expect actual
@@ -214,7 +215,7 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set
EOF
git config http.cookiefile cookies.txt &&
git config http.savecookies true &&
- git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
+ GIT_TEST_PROTOCOL_VERSION= git ls-remote $HTTPD_URL/smart_cookies/repo.git master &&
tail -3 cookies.txt | sort >cookies_tail.txt &&
test_cmp expect_cookies.txt cookies_tail.txt
'
@@ -306,7 +307,8 @@ test_expect_success 'test allowreachablesha1inwant with unreachable' '
git init --bare test_reachable.git &&
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
- test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
'
test_expect_success 'test allowanysha1inwant with unreachable' '
@@ -325,7 +327,8 @@ test_expect_success 'test allowanysha1inwant with unreachable' '
git init --bare test_reachable.git &&
git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" &&
- test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ git -C test_reachable.git fetch origin "$(git rev-parse HEAD)" &&
git -C "$server" config uploadpack.allowanysha1inwant 1 &&
git -C test_reachable.git fetch origin "$(git rev-parse HEAD)"
diff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh
index 30857b84a8..979e6583d4 100755
--- a/t/t5552-skipping-fetch-negotiator.sh
+++ b/t/t5552-skipping-fetch-negotiator.sh
@@ -127,7 +127,7 @@ test_expect_success 'use ref advertisement to filter out commits' '
# not need to send any ancestors of "c3", but we still need to send "c3"
# itself.
test_config -C client fetch.negotiationalgorithm skipping &&
- trace_fetch client origin to_fetch &&
+ GIT_TEST_PROTOCOL_VERSION= trace_fetch client origin to_fetch &&
have_sent c5 c4^ c2side &&
have_not_sent c4 c4^^ c4^^^
'
@@ -189,7 +189,7 @@ test_expect_success 'do not send "have" with ancestors of commits that server AC
test_commit -C server commit-on-b1 &&
test_config -C client fetch.negotiationalgorithm skipping &&
- trace_fetch client "$(pwd)/server" to_fetch &&
+ GIT_TEST_PROTOCOL_VERSION= trace_fetch client "$(pwd)/server" to_fetch &&
grep " fetch" trace &&
# fetch-pack sends 2 requests each containing 16 "have" lines before
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 7466aad111..c42ee245cd 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -190,7 +190,7 @@ test_expect_success 'daemon log records all attributes' '
EOF
>daemon.log &&
GIT_OVERRIDE_VIRTUAL_HOST=localhost \
- git -c protocol.version=1 \
+ GIT_TEST_PROTOCOL_VERSION= git -c protocol.version=1 \
ls-remote "$GIT_DAEMON_URL/interp.git" &&
grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&
test_cmp expect actual
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index e87164aa8f..a555e38845 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -943,7 +943,8 @@ test_expect_success 'submodule update clone shallow submodule outside of depth'
cd super3 &&
sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
mv -f .gitmodules.tmp .gitmodules &&
- test_must_fail git submodule update --init --depth=1 2>actual &&
+ test_must_fail env GIT_TEST_PROTOCOL_VERSION= \
+ git submodule update --init --depth=1 2>actual &&
test_i18ngrep "Direct fetching of that commit failed." actual &&
git -C ../submodule config uploadpack.allowReachableSHA1InWant true &&
git submodule update --init --depth=1 >actual &&
--
2.20.0.405.gbc1bbc6f85
^ permalink raw reply related
* [PATCH v2 7/8] builtin/fetch-pack: support protocol version 2
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
From: Jonathan Tan <jonathantanmy@google.com>
Currently, if support for running Git's entire test suite with protocol
v2 were added, some tests would fail because the fetch-pack CLI command
dies if it encounters protocol v2. To avoid this, teach fetch-pack
support for protocol v2.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/fetch-pack.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 63e69a5801..f6a513495e 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -55,6 +55,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
struct oid_array shallow = OID_ARRAY_INIT;
struct string_list deepen_not = STRING_LIST_INIT_DUP;
struct packet_reader reader;
+ enum protocol_version version;
fetch_if_missing = 0;
@@ -219,9 +220,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_GENTLE_ON_EOF);
- switch (discover_version(&reader)) {
+ version = discover_version(&reader);
+ switch (version) {
case protocol_v2:
- die("support for protocol v2 not implemented yet");
+ get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL);
+ break;
case protocol_v1:
case protocol_v0:
get_remote_heads(&reader, &ref, 0, NULL, &shallow);
@@ -231,7 +234,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
}
ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,
- &shallow, pack_lockfile_ptr, protocol_v0);
+ &shallow, pack_lockfile_ptr, version);
if (pack_lockfile) {
printf("lock %s\n", pack_lockfile);
fflush(stdout);
--
2.20.0.405.gbc1bbc6f85
^ permalink raw reply related
* [PATCH v2 6/8] tests: mark & fix tests broken under GIT_TEST_PROTOCOL_VERSION=1
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
There's one t5400-send-pack.sh test which is broken under
GIT_TEST_PROTOCOL_VERSION=1. It's easier to just coerce it to run
under protocol 1. The difference in the output is that there'll be a
"version 1" line which'll make the subsequent "test_cmp". See
protocol.version in git-config(1) which notes that "1" is just "0"
with a version number. the trace output will include fail.
Similarly in t5601-clone.sh we'll be passing an option to ssh, but
since so many tests would fail in this file let's go above & beyond
and make them pass by only testing the relevant part of the output.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/t5400-send-pack.sh | 2 +-
t/t5601-clone.sh | 11 +++++++++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index f1932ea431..571d620aed 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -288,7 +288,7 @@ test_expect_success 'receive-pack de-dupes .have lines' '
$shared .have
EOF
- GIT_TRACE_PACKET=$(pwd)/trace \
+ GIT_TRACE_PACKET=$(pwd)/trace GIT_TEST_PROTOCOL_VERSION= \
git push \
--receive-pack="unset GIT_TRACE_PACKET; git-receive-pack" \
fork HEAD:foo &&
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 8bbc7068ac..a7b7ab327c 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -325,7 +325,7 @@ copy_ssh_wrapper_as () {
expect_ssh () {
test_when_finished '
- (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
+ (cd "$TRASH_DIRECTORY" && rm -f ssh-expect ssh-output.munged && >ssh-output)
' &&
{
case "$#" in
@@ -341,7 +341,14 @@ expect_ssh () {
echo "ssh: $1 $2 git-upload-pack '$3' $4"
esac
} >"$TRASH_DIRECTORY/ssh-expect" &&
- (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
+ (
+ cd "$TRASH_DIRECTORY" &&
+ # We don't care about this trivial difference in
+ # output with GIT_TEST_PROTOCOL_VERSION=[12]
+ sed 's/ssh: -o SendEnv=GIT_PROTOCOL /ssh: /' <ssh-output >ssh-output.munged &&
+ mv ssh-output.munged ssh-output &&
+ test_cmp ssh-expect ssh-output
+ )
}
test_expect_success 'clone myhost:src uses ssh' '
--
2.20.0.405.gbc1bbc6f85
^ permalink raw reply related
* [PATCH v2 5/8] tests: add a special setup where for protocol.version
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
Add a GIT_TEST_PROTOCOL_VERSION=X test mode which is equivalent to
running with protocol.version=X. This is needed to spot regressions
and differences such as "ls-refs" behaving differently with
transfer.hideRefs. See
https://public-inbox.org/git/20181211104236.GA6899@sigill.intra.peff.net/
for a fix for that regression.
With this all tests pass with GIT_TEST_PROTOCOL_VERSION=0, but fail
with GIT_TEST_PROTOCOL_VERSION=[1|2]. That's OK since this is a new
test mode, subsequent patches will fix up these test failures.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
protocol.c | 13 ++++++++++++-
t/README | 6 ++++++
t/t0410-partial-clone.sh | 3 ++-
t/t5516-fetch-push.sh | 3 ++-
t/t5700-protocol-v1.sh | 1 +
t/t5702-protocol-v2.sh | 1 +
6 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/protocol.c b/protocol.c
index 5e636785d1..aa06c3b5bc 100644
--- a/protocol.c
+++ b/protocol.c
@@ -17,7 +17,18 @@ static enum protocol_version parse_protocol_version(const char *value)
enum protocol_version get_protocol_version_config(void)
{
const char *value;
- if (!git_config_get_string_const("protocol.version", &value)) {
+ const char *git_test_k = "GIT_TEST_PROTOCOL_VERSION";
+ const char *git_test_v = getenv(git_test_k);
+
+ if (git_test_v && strlen(git_test_v)) {
+ enum protocol_version version = parse_protocol_version(git_test_v);
+
+ if (version == protocol_unknown_version)
+ die("unknown value for %s: %s", git_test_k,
+ git_test_v);
+
+ return version;
+ } else if (!git_config_get_string_const("protocol.version", &value)) {
enum protocol_version version = parse_protocol_version(value);
if (version == protocol_unknown_version)
diff --git a/t/README b/t/README
index 28711cc508..89629c5818 100644
--- a/t/README
+++ b/t/README
@@ -311,6 +311,12 @@ marked strings" in po/README for details.
GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
test suite. Accept any boolean values that are accepted by git-config.
+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.
+
GIT_TEST_FULL_IN_PACK_ARRAY=<boolean> exercises the uncommon
pack-objects code path where there are more than 1024 packs even if
the actual number of packs in repository is below this limit. Accept
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index ba3887f178..8ba3d9b5ab 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -178,7 +178,8 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled'
rm -rf repo/.git/objects/* &&
rm -f trace &&
- GIT_TRACE_PACKET="$(pwd)/trace" git -C repo cat-file -p "$HASH" &&
+ GIT_TRACE_PACKET="$(pwd)/trace" env GIT_TEST_PROTOCOL_VERSION= \
+ git -C repo cat-file -p "$HASH" &&
grep "git< fetch=.*ref-in-want" trace
'
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 37e8e80893..08cdee0b95 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1187,7 +1187,8 @@ test_expect_success 'fetch exact SHA1 in protocol v2' '
# fetching the hidden object succeeds by default
# NEEDSWORK: should this match the v0 behavior instead?
- git -C child fetch -v ../testrepo $the_commit:refs/heads/copy
+ env GIT_TEST_PROTOCOL_VERSION= \
+ git -C child fetch -v ../testrepo $the_commit:refs/heads/copy
'
for configallowtipsha1inwant in true false
diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh
index ba86a44eb1..e4d375c462 100755
--- a/t/t5700-protocol-v1.sh
+++ b/t/t5700-protocol-v1.sh
@@ -8,6 +8,7 @@ TEST_NO_CREATE_REPO=1
# Test protocol v1 with 'git://' transport
#
+unset GIT_TEST_PROTOCOL_VERSION
. "$TEST_DIRECTORY"/lib-git-daemon.sh
start_git_daemon --export-all --enable=receive-pack
daemon_parent=$GIT_DAEMON_DOCUMENT_ROOT_PATH/parent
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 0f2b09ebb8..d1549f294e 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -8,6 +8,7 @@ TEST_NO_CREATE_REPO=1
# Test protocol v2 with 'git://' transport
#
+unset GIT_TEST_PROTOCOL_VERSION
. "$TEST_DIRECTORY"/lib-git-daemon.sh
start_git_daemon --export-all --enable=receive-pack
daemon_parent=$GIT_DAEMON_DOCUMENT_ROOT_PATH/parent
--
2.20.0.405.gbc1bbc6f85
^ permalink raw reply related
* [PATCH v2 4/8] tests: add a check for unportable env --unset
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
The "env --unset=*" argument isn't portable. Neither Solaris or AIX
have it, and probably not a bunch of other POSIX-like OS's.
Using this was suggested on-list[1]. Let's add a check for it so it
doesn't sneak into the codebase in the future.
1. https://public-inbox.org/git/cover.1544573604.git.jonathantanmy@google.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
t/check-non-portable-shell.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index b45bdac688..1cfb4608ee 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -35,6 +35,7 @@ sub err {
chomp;
}
+ /\benv (?:-u|--unset)\b/ and err 'env [-u|--unset] is not portable';
/\bsed\s+-i/ and err 'sed -i is not portable';
/\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)';
/^\s*declare\s+/ and err 'arrays/declare not portable';
--
2.20.0.405.gbc1bbc6f85
^ permalink raw reply related
* [PATCH v2 3/8] upload-pack: support hidden refs with protocol v2
From: Ævar Arnfjörð Bjarmason @ 2018-12-13 15:58 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Brandon Williams, Jonathan Tan,
Ævar Arnfjörð Bjarmason
In-Reply-To: <xmqqimzygmz6.fsf@gitster-ct.c.googlers.com>
From: Jeff King <peff@peff.net>
In the v2 protocol, upload-pack's advertisement has been moved to the
"ls-refs" command. That command does not respect hidden-ref config (like
transfer.hiderefs) at all, and advertises everything.
While there are some features that are not supported in v2 (e.g., v2
always allows fetching any sha1 without respect to advertisements), the
lack of this feature is not documented and is likely just a bug. Let's
make it work, as otherwise upgrading a server to a v2-capable git will
start exposing these refs that the repository admin has asked to remain
hidden.
Note that we depend on the config_context from the caller here to
realize that we should respect uploadpack.hiderefs. When run from the
"git-serve" tool, we won't have that context and will only respect
transfer.hiderefs. This should be OK, as git-serve is not actually used
for normal protocol operations.
Note also that we don't need to worry about the "An attempt to update
or delete a hidden ref by git push is rejected" feature of
receive.hideRefs, see git-config(1). This is because there's no v2
push protocol yet.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
ls-refs.c | 12 ++++++++++++
t/t5512-ls-remote.sh | 6 ++++++
2 files changed, 18 insertions(+)
diff --git a/ls-refs.c b/ls-refs.c
index e8e31475f0..8a8143338b 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -5,6 +5,7 @@
#include "argv-array.h"
#include "ls-refs.h"
#include "pkt-line.h"
+#include "config.h"
/*
* Check if one of the prefixes is a prefix of the ref.
@@ -40,6 +41,9 @@ static int send_ref(const char *refname, const struct object_id *oid,
const char *refname_nons = strip_namespace(refname);
struct strbuf refline = STRBUF_INIT;
+ if (ref_is_hidden(refname_nons, refname))
+ return 0;
+
if (!ref_match(&data->prefixes, refname))
return 0;
@@ -69,6 +73,12 @@ static int send_ref(const char *refname, const struct object_id *oid,
return 0;
}
+static int ls_refs_config(const char *var, const char *value,
+ void *config_context)
+{
+ return parse_hide_refs_config(var, value, config_context);
+}
+
int ls_refs(struct repository *r,
const char *config_section,
struct argv_array *keys,
@@ -78,6 +88,8 @@ int ls_refs(struct repository *r,
memset(&data, 0, sizeof(data));
+ git_config(ls_refs_config, (void *)config_section);
+
while (packet_reader_read(request) != PACKET_READ_FLUSH) {
const char *arg = request->line;
const char *out;
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 32e722db2e..ca69636fd5 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -204,6 +204,12 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs'
grep refs/tags/magic actual
'
+test_expect_success 'protocol v2 supports hiderefs' '
+ test_config uploadpack.hiderefs refs/tags &&
+ git -c protocol.version=2 ls-remote . >actual &&
+ ! grep refs/tags actual
+'
+
test_expect_success 'ls-remote --symref' '
git fetch origin &&
cat >expect <<-EOF &&
--
2.20.0.405.gbc1bbc6f85
^ 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