* Re: [PATCH] tag: add tag.createReflog option
From: Jeff King @ 2017-01-25 21:33 UTC (permalink / raw)
To: Cornelius Weig; +Cc: git, novalis, pclouds
In-Reply-To: <00712f81-e0ba-52e6-77bc-095a2ed706c4@tngtech.com>
On Wed, Jan 25, 2017 at 10:21:48PM +0100, Cornelius Weig wrote:
> On 01/25/2017 07:00 PM, Jeff King wrote:
>
> > - Is that the end of it, or is the desire really "I want reflogs for
> > _everything_"? That seems like a sane thing to want.
> >
> > If so, then the update to core.logallrefupdates should turn it into
> > a tri-state:
> >
> > - false; no reflogs
> >
> > - true; reflogs for branches, remotes, notes, as now
> >
> > - always; reflogs for all refs under "refs/"
> >
>
> I think you nailed it. This is much more useful than what I suggested.
> I'll see if I can code it up.
I cheated a little. I actually wrote the "always" patch 3 years ago as
part of another thing I was working on. But in the end I didn't need it,
and never submitted it.
The patch is below for reference. I have no idea whether it even applies
now, let alone runs and does the right thing. But perhaps you can
salvage bits of it (but feel free to ignore it if it makes things
harder).
-- >8 --
From: Jeff King <peff@peff.net>
Date: Mon, 15 Apr 2013 23:31:05 -0400
Subject: [PATCH] teach core.logallrefupdates an "always" mode
When core.logallrefupdates is true, we only create a new
reflog for refs that are under certain well-known
hierarchies. The reason is that we know that some
hierarchies (like refs/tags) do not typically change, and
that unknown hierarchies might not want reflogs at all
(e.g., a hypothetical refs/foo might be meant to change
often and drop old history immediately).
However, sometimes it is useful to override this decision
and simply log for all refs, because the safety and audit
trail is more important than the performance implications of
keeping the log around.
This patch introduces a new "always" mode for the
core.logallrefupdates option which will log updates to
everything under refs/, regardless where in the hierarchy it
is (we still will not log things like ORIG_HEAD and
FETCH_HEAD, which are known to be transient).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/config.txt | 8 +++++---
branch.c | 2 +-
builtin/checkout.c | 2 +-
builtin/init-db.c | 2 +-
cache.h | 9 ++++++++-
config.c | 7 ++++++-
environment.c | 2 +-
refs.c | 23 +++++++++++++++++------
t/t1400-update-ref.sh | 32 ++++++++++++++++++++++++++++++++
9 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e37ba94a72..cb72e559ec 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -390,10 +390,12 @@ core.logAllRefUpdates::
"$GIT_DIR/logs/<ref>", by appending the new and old
SHA1, the date/time and the reason of the update, but
only when the file exists. If this configuration
- variable is set to true, missing "$GIT_DIR/logs/<ref>"
+ variable is set to `true`, a missing "$GIT_DIR/logs/<ref>"
file is automatically created for branch heads (i.e. under
- refs/heads/), remote refs (i.e. under refs/remotes/),
- note refs (i.e. under refs/notes/), and the symbolic ref HEAD.
+ `refs/heads/`), remote refs (i.e. under `refs/remotes/`),
+ note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/branch.c b/branch.c
index 2bef1e7e71..c11880b181 100644
--- a/branch.c
+++ b/branch.c
@@ -259,7 +259,7 @@ void create_branch(const char *head,
}
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (forcing)
snprintf(msg, sizeof msg, "branch: Reset to %s",
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a95f..00e231d83b 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -564,7 +564,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
temp = log_all_ref_updates;
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (log_ref_setup(ref_name, log_file, sizeof(log_file))) {
fprintf(stderr, _("Can not do reflog for '%s'\n"),
opts->new_orphan_branch);
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 78aa3872dd..0ebad0b37d 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -264,7 +264,7 @@ static int create_default_files(const char *template_path)
const char *work_tree = get_git_work_tree();
git_config_set("core.bare", "false");
/* allow template config file to override the default */
- if (log_all_ref_updates == -1)
+ if (log_all_ref_updates == LOG_REFS_UNSET)
git_config_set("core.logallrefupdates", "true");
if (prefixcmp(git_dir, work_tree) ||
strcmp(git_dir + strlen(work_tree), "/.git")) {
diff --git a/cache.h b/cache.h
index 2b192d24ac..d2bfabc67f 100644
--- a/cache.h
+++ b/cache.h
@@ -536,7 +536,6 @@ extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
extern int prefer_symlink_refs;
-extern int log_all_ref_updates;
extern int warn_ambiguous_refs;
extern int shared_repository;
extern const char *apply_default_whitespace;
@@ -556,6 +555,14 @@ extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+enum log_refs_config {
+ LOG_REFS_UNSET = -1,
+ LOG_REFS_NONE = 0,
+ LOG_REFS_NORMAL, /* see should_create_reflog for rules */
+ LOG_REFS_ALWAYS
+};
+extern enum log_refs_config log_all_ref_updates;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index b5696354fa..ffb892c0a0 100644
--- a/config.c
+++ b/config.c
@@ -601,7 +601,12 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.logallrefupdates")) {
- log_all_ref_updates = git_config_bool(var, value);
+ if (value && !strcasecmp(value, "always"))
+ log_all_ref_updates = LOG_REFS_ALWAYS;
+ else if (git_config_bool(var, value))
+ log_all_ref_updates = LOG_REFS_NORMAL;
+ else
+ log_all_ref_updates = LOG_REFS_NONE;
return 0;
}
diff --git a/environment.c b/environment.c
index 85edd7f95a..1867d31d75 100644
--- a/environment.c
+++ b/environment.c
@@ -19,7 +19,7 @@ int ignore_case;
int assume_unchanged;
int prefer_symlink_refs;
int is_bare_repository_cfg = -1; /* unspecified */
-int log_all_ref_updates = -1; /* unspecified */
+enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
int warn_ambiguous_refs = 1;
int repository_format_version;
const char *git_commit_encoding;
diff --git a/refs.c b/refs.c
index 541fec2065..3cd203ef87 100644
--- a/refs.c
+++ b/refs.c
@@ -1896,7 +1896,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
lock->force_write = 1;
flag = log_all_ref_updates;
- log_all_ref_updates = 0;
+ log_all_ref_updates = LOG_REFS_NONE;
if (write_ref_sha1(lock, orig_sha1, NULL))
error("unable to write current sha1 into %s", oldrefname);
log_all_ref_updates = flag;
@@ -1965,16 +1965,27 @@ static int copy_msg(char *buf, const char *msg)
return cp - buf;
}
+int should_create_reflog(const char *refname)
+{
+ switch (log_all_ref_updates) {
+ case LOG_REFS_ALWAYS:
+ return 1;
+ case LOG_REFS_NORMAL:
+ return !prefixcmp(refname, "refs/heads/") ||
+ !prefixcmp(refname, "refs/remotes/") ||
+ !prefixcmp(refname, "refs/notes/") ||
+ !strcmp(refname, "HEAD");
+ default:
+ return 0;
+ }
+}
+
int log_ref_setup(const char *refname, char *logfile, int bufsize)
{
int logfd, oflags = O_APPEND | O_WRONLY;
git_snpath(logfile, bufsize, "logs/%s", refname);
- if (log_all_ref_updates &&
- (!prefixcmp(refname, "refs/heads/") ||
- !prefixcmp(refname, "refs/remotes/") ||
- !prefixcmp(refname, "refs/notes/") ||
- !strcmp(refname, "HEAD"))) {
+ if (should_create_reflog(refname)) {
if (safe_create_leading_directories(logfile) < 0)
return error("unable to create directory for %s",
logfile);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e415ee0bbf..f60196d294 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -302,4 +302,36 @@ test_expect_success \
'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER)' \
'test OTHER = $(git cat-file blob "master@{2005-05-26 23:42}:F")'
+test_expect_success 'core.logAllRefUpdates=true does not log refs/foo/' '
+ test_config core.logAllRefUpdates true &&
+ test_commit log-true &&
+ git update-ref -m reflog-message refs/heads/logme HEAD &&
+ git update-ref -m reflog-message refs/foo/logme HEAD &&
+ {
+ echo "refs/heads/logme@{0} reflog-message"
+ } >expect &&
+ {
+ git log -g -1 --format="%gD %gs" refs/heads/logme &&
+ git log -g -1 --format="%gD %gs" refs/foo/logme
+ } >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'core.logAllRefUpdates=always logs refs/foo/' '
+ test_config core.logAllRefUpdates always &&
+ test_commit log-always &&
+ git update-ref -m reflog-message refs/heads/logme HEAD &&
+ git update-ref -m reflog-message refs/foo/logme HEAD &&
+ {
+ echo "refs/heads/logme@{0} reflog-message"
+ echo "refs/foo/logme@{0} reflog-message"
+ } >expect &&
+ {
+ git log -g -1 --format="%gD %gs" refs/heads/logme &&
+ git log -g -1 --format="%gD %gs" refs/foo/logme
+ } >actual &&
+ test_cmp expect actual
+'
+
+
test_done
--
2.11.0.840.gd37c5973a
^ permalink raw reply related
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: Jeff King @ 2017-01-25 21:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: brian m. carlson, git
In-Reply-To: <alpine.DEB.2.20.1701251425080.3469@virtualbox>
On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
> > The need for the extensions could be replaced with a small amount of
> > Ruby code, if that's considered desirable. Previous opinions on doing
> > so were negative, however.
>
> Quite frankly, it is annoying to be forced to install the extensions. I
> would much rather have the small amount of Ruby code in Git's repository.
Me too. Dependencies can be a big annoyance. I'd reserve judgement until
I saw the actual Ruby code, though. :)
-Peff
^ permalink raw reply
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Junio C Hamano @ 2017-01-25 21:39 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <dc388b2df3baee83972f007cd77a57410553a411.1485362812.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> This change is necessary to allow the files in .git/hooks/ to optionally
> have the file extension `.exe` on Windows, as the file names are
> hardcoded otherwise.
Hmph. There is no longer ".com"?
I briefly wondered if hooks/post-receive.{py,rb,...} would be good
things to support, but I do not think we want to go there, primarily
because we do not want to deal with "what happens when there are
many?"
As Peff pointed out while I was typing this message, ".exe" would be
better spelled as STRIP_EXTENSION, I think. The resulting code
would read naturally when you read the macro not as "please do strip
extensions" boolean, but as "this extension is to be stripped"
string, which is how it is used in the other site the macro is used
(namely, strip_extension() called by handle_builtin()).
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/exe-as-hook-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git exe-as-hook-v1
>
> run-command.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index 73bfba7ef9..45229ef052 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -871,8 +871,14 @@ const char *find_hook(const char *name)
>
> strbuf_reset(&path);
> strbuf_git_path(&path, "hooks/%s", name);
> - if (access(path.buf, X_OK) < 0)
> + if (access(path.buf, X_OK) < 0) {
> +#ifdef STRIP_EXTENSION
> + strbuf_addstr(&path, ".exe");
> + if (access(path.buf, X_OK) >= 0)
> + return path.buf;
> +#endif
> return NULL;
> + }
> return path.buf;
> }
>
>
> base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
^ permalink raw reply
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Junio C Hamano @ 2017-01-25 21:39 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125211529.jq4halip4ndbff5k@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 25, 2017 at 05:58:42PM +0100, Johannes Schindelin wrote:
>
>> This change is necessary to allow the files in .git/hooks/ to optionally
>> have the file extension `.exe` on Windows, as the file names are
>> hardcoded otherwise.
>
> Make sense as a goal.
>
>> - if (access(path.buf, X_OK) < 0)
>> + if (access(path.buf, X_OK) < 0) {
>> +#ifdef STRIP_EXTENSION
>> + strbuf_addstr(&path, ".exe");
>
> I think STRIP_EXTENSION is a string. Should this line be:
>
> strbuf_addstr(&path, STRIP_EXTENSION);
>
> ?
Yup, I think that is more in line with how git.c (the other user of
the macro) uses it.
^ permalink raw reply
* Re: [PATCH] Retire the `relink` command
From: Junio C Hamano @ 2017-01-25 21:41 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125211331.ts4ohigz5z6ugj6q@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 25, 2017 at 05:58:57PM +0100, Johannes Schindelin wrote:
>
>> Back in the olden days, when all objects were loose and rubber boots were
>> made out of wood, it made sense to try to share (immutable) objects
>> between repositories.
>>
>> Ever since the arrival of pack files, it is but an anachronism.
>
> Yes, this is a good idea. I could _almost_ see its utility if it linked
> packfiles, too, but then it is very unlikely that two repos have the
> exact same packfile.
>
>> Let's move the script to the contrib/examples/ directory and no longer
>> offer it.
>
> I am OK with this, but perhaps we should go even further and just delete
> it entirely. The point of contrib/examples is to show people "this is
> how you could script plumbing to implement a porcelain". But this script
> does not call a single plumbing script. It just looks directly in
> objects/, which is probably not something we would want to encourage. :)
Yeah. I am OK with this patch as the first step of "first move to
contrib and then remove" two step process, but I suspect we may
forget to follow through.
^ permalink raw reply
* Re: [PATCH] tag: add tag.createReflog option
From: Junio C Hamano @ 2017-01-25 21:43 UTC (permalink / raw)
To: Jeff King; +Cc: Cornelius Weig, git, novalis, pclouds
In-Reply-To: <20170125213328.meehgxvzuajjgvag@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> +enum log_refs_config {
> + LOG_REFS_UNSET = -1,
> + LOG_REFS_NONE = 0,
> + LOG_REFS_NORMAL, /* see should_create_reflog for rules */
> + LOG_REFS_ALWAYS
> +};
> +extern enum log_refs_config log_all_ref_updates;
> +...
> +int should_create_reflog(const char *refname)
> +{
> + switch (log_all_ref_updates) {
> + case LOG_REFS_ALWAYS:
> + return 1;
> + case LOG_REFS_NORMAL:
> + return !prefixcmp(refname, "refs/heads/") ||
> + !prefixcmp(refname, "refs/remotes/") ||
> + !prefixcmp(refname, "refs/notes/") ||
> + !strcmp(refname, "HEAD");
> + default:
> + return 0;
> + }
> +}
Yup, this is how I expected for the feature to be done.
Just a hint for Cornelius; prefixcmp() is an old name for what is
called starts_with() these days.
^ permalink raw reply
* Re: [PATCH v2 25/27] attr: store attribute stack in attr_check structure
From: Brandon Williams @ 2017-01-25 21:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, pclouds
In-Reply-To: <xmqq1svqc2ww.fsf@gitster.mtv.corp.google.com>
On 01/25, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> >> In my mind, the value of having a constant check_attr is primarily
> >> that it gives us a stable pointer to serve as a hashmap key,
> >> i.e. the identifier for each call site, in a later iteration.
> >
> > We didn't really discuss this notion of having the pointer be a key into
> > a hashmap, what sort of information are you envisioning being stored in
> > this sort of hashmap?
>
> The "entries relevant to this attr_check() call, that is specific to
> the <check_attr instance, the thread> tuple" (aka "what used to be
> called the global attr_stack") we discussed would be the primary
> example. A thread is likely be looping in a caller that has many
> paths inside a directory, calling a function that has a call to
> attr_check() for each path. Having something that can use to
> identify the check_attr instance in a stable way, even when the
> inner function is called and returns many times, would allow us to
> populate the "attr stack" just once for the thread when it enters a
> directory for the first time (remember, another thread may be
> executing the same codepath, checking for paths in a different
> directory) and keep using it. There may be other mechanisms you can
> come up with, so I wouldn't say it is the only valid way, but it is
> a way. That is why I said:
>
> >> But all of the above comes from my intuition, and I'll very much
> >> welcome to be proven wrong with an alternative design, or better
> >> yet, a working code based on an alternative design ;-).
>
> near the end of my message.
>
> > One issue I can see with this is that the
> > functions which have a static attr_check struct would still not be thread
> > safe if the initialization of the structure isn't surrounded by a mutex
> > itself. ie
>
> Yes, that goes without saying. That is why I suggested Stefan to do
> not this:
>
> > static struct attr_check *check;
> >
> > if (!check)
> > init(check);
> >
> > would need to be:
> >
> > lock()
> > if (!check)
> > init(check);
> > unlock();
>
> but this:
>
> static struct attr_check *check;
> init(&check);
>
> and hide the lock/unlock gymnastics inside the API. I thought that
> already was in what you inherited from him and started your work
> on top of?
I essentially built off of the series you had while using Stefan's
patches as inspiration, but I don't believe the kind of mechanism you
are describing existed in Stefan's series. His series had a single lock
for the entire system, only allowing a single caller to be in it at any
given time. This definitely isn't ideal, hence why I picked it up.
Implementation aside I want to try and nail down what the purpose of
this refactor is. There are roughly two notions of being "thread-safe".
1. The first is that the subsystem itself is thread safe, that is
multiple threads can be executing inside the subsystem without stepping
on each others work.
2. The second is that the object itself is thread safe or that multiple
threads can use the same object.
I thought that the main purpose of this was to achieve (1) since
currently that is not the case.
--
Brandon Williams
^ permalink raw reply
* [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Stefan Beller @ 2017-01-25 21:59 UTC (permalink / raw)
To: peff; +Cc: git, Stefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Peff,
This applies to the repo at https://github.com/git/git.github.io
If you're looking for a co-admin/mentors, I can help out.
Thanks,
Stefan
SoC-2017-Ideas.md | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/SoC-2017-Ideas.md b/SoC-2017-Ideas.md
index df98919..6d554b8 100644
--- a/SoC-2017-Ideas.md
+++ b/SoC-2017-Ideas.md
@@ -162,3 +162,60 @@ See discussion in:
The goal is to better format object related information as discussed in:
[https://public-inbox.org/git/CA+P7+xr4ZNCCJkS0=yR-FNu+MrL60YX-+Wsz9L_5LCNhnY_d=A@mail.gmail.com/](https://public-inbox.org/git/CA+P7+xr4ZNCCJkS0=yR-FNu+MrL60YX-+Wsz9L_5LCNhnY_d=A@mail.gmail.com/)
+
+### Submodule related work:
+
+* Cleanup our test suite. Do not use a repo itself as a submodule for itself
+ (Search for "git submodule add ./. <name>")
+
+* Fix the ./ bug for submodule URL handling.
+ (c.f. https://public-inbox.org/git/20161021235939.20792-4-sbeller@google.com/)
+
+* Teach "git -C <submodule-path> status" in an un-populated submodule
+ to report the submodule being un-populated, do not fall back to the
+ superproject.
+
+* "git -C sub add ." might behave just like "git add sub"
+
+* Teach "git log -- <path/into/submodule/and/further>" to behave
+ like "git -C <path/into/submodule> log -- <and/further>
+
+* git archive(/bundle) to have a --recurse-submodules flag to
+ include the submodule contents.
+
+* Convert a submodule subcommand to C (c.f. 3604242f080a8,
+ submodule: port init from shell to C, 2016-04-15)
+ I'd propose to go for "foreach" first, as that will
+ have most performance impact and is one of the shortest
+
+* (Advanced datastructure knowledge required?)
+ Protect submodule from gc-ing interesting HEADS.
+ Given that the the modules file has a ‘branch’ field, we may want checkout
+ to have the ability to checkout the branch specified in this ‘branch’ field.
+ This can be especially useful when making a brand new branch in the
+ superproject which can then make corresponding branches in the submodules.
+ Or if we are tracking a particular branch, we can checkout that branch
+ (given HEAD of that branch is pointing to the same SHA1 that is checked
+ into the superproject). This may be needed to avoid unintended garbage
+ collection of commits in the submodules which aren’t reachable by the
+ standard refs/branches.
+
+* (Advanced understanding of usability:)
+ Design and implement an "overlay" for .gitmodules as a ref.
+ To get submodules to usable state, you need to configure a lot. To aid with
+ this the file ".gitmodules" in the repository provides some defaults that
+ are copied to the actual config e.g. in "git submodule init".
+ These defaults are not always the right choice (e.g. when working in a
+ large organisation, you may have an internal git mirror site, that
+ you rather want to clone/fetch from; This can be helped with by configuring
+ e.g. url."<pattern>".insteadOf; But generally this is a pain for users; this
+ large organisation could provide such a configuration as a ref as well,
+ which has higher priority than the .gitmodules file, but lower priority
+ than the .git/config file.)
+
+### Discourage pushing annotated tag to a branch ref
+ If I run
+
+ git push origin v1.0:refs/heads/master
+
+ and v1.0 is an annotated tag, then I probably meant v1.0^{commit}, not ^{tag}.
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: Fixing the warning about warning(""); was: Re: [PATCH] difftool.c: mark a file-local symbol with static
From: Jeff King @ 2017-01-25 22:01 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, David Aguilar, Ramsay Jones,
GIT Mailing-list
In-Reply-To: <xmqq7f5iakxw.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 25, 2017 at 01:28:27PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The only advantage is that it is self-documenting, so somebody does not
> > come through later and convert ("%s", "") back to (""). We could also
> > write a comment. But I am happy if we simply catch it in review (or
> > preferably the person is clueful enough to read the output of git-blame
> > and see why it is that way in the first place).
>
> And the last sentence unfortunatly does not reflect reality.
>
> I would prefer something self-documenting, like your wrapper with a
> comment. Then somebody who is looking at the implementation of
> warning_blank_line() will not get tempted to turn "%s", "" into ""
> because of the comment. And somebody who is looking at the callsite
> of warning_blank_line() will think twice before suggesting to turn
> it into warning("").
I am OK with it either way. I was mostly responding to Dscho's
complaint, and I would just like to get this resolved so we never have
to revisit it again. :)
> In any case, the patch is a minimum effort band-aid that lets us
> punt on the whole issue for now, so I'll queue it as-is.
Here's one other option that I came across. Pragmas feel gross, but I
think it will behave as we want, and it doesn't require cooperation from
the callsites at all.
-- >8 --
Subject: [PATCH] disable -Wzero-length-format via #pragma
Building with "gcc -Wall" will complain that the format in:
warning("")
is empty. Which is true, but the warning is over-eager. We
are calling the function for its side effect of printing
"warning:", even with an empty string.
We disable this warning already with the DEVELOPER Makefile
knob. But we can't unconditionally add -Wno-format-zero-length
to the normal CFLAGS variable, because not all compilers will
understand it. So we may get reports about the warning from
non-developer users who compile with our default of -Wall.
Instead, we can disable the warning using a gcc-specific
#pragma. This should be ignored by non-gcc compilers, and do
what we want for gcc.
I tested also with clang, which often implements gcc
compatible extensions like this. Clang does not generate the
warning in the first place, but also does not complain about
our pragma.
Signed-off-by: Jeff King <peff@peff.net>
---
git-compat-util.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 325950426..a6558930d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -413,6 +413,8 @@ extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2
extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
+#pragma GCC diagnostic ignored "-Wformat-zero-length"
+
#ifndef NO_OPENSSL
#ifdef APPLE_COMMON_CRYPTO
#include "compat/apple-common-crypto.h"
--
2.11.0.840.gd37c5973a
^ permalink raw reply related
* [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
Hello everyone - this is a proposal for a protocol change to allow the
fetch-pack/upload-pack to converse in terms of ref names (globs
allowed), and also an implementation of the server (upload-pack) and
fetch-from-HTTP client (fetch-pack invoked through fetch).
Negotiation currently happens by upload-pack initially sending a list of
refs with names and SHA-1 hashes, and then several request/response
pairs in which the request from fetch-pack consists of SHA-1 hashes
(selected from the initial list). Allowing the request to consist of
names instead of SHA-1 hashes increases tolerance to refs changing
(due to time, and due to having load-balanced servers without strong
consistency), and is a step towards eliminating the need for the server
to send the list of refs first (possibly improving performance).
The protocol is extended by allowing fetch-pack to send
"want-ref <name>", where <name> is a full name (refs/...) [1], possibly
including glob characters. Upload-pack will inform the client of the
refs actually matched by sending "wanted-ref <SHA-1> <name>" before
sending the last ACK or NAK.
This patch set is laid out in this way:
1-2:
Upload-pack, protocol documentation, tests that test upload-pack
independently. A configuration option is added to control if the
"ref-in-want" capability is advertised. (It is always supported even
if not advertised - this is so that this feature in multiple
load-balanced servers can be switched on or off without needing any
atomic switching.)
3:
Mechanism to test a repo that changes over the negotiation (currently,
only with the existing mechanism).
4-9:
The current transport mechanism takes in an array of refs which is
used as both input and output. Since we are planning to extend the
transport mechanism to also allow the specification of ref names
(which may include globs, and thus do not have a 1-1 correspondence to
refs), refactor to make this parameter to be solely an input
parameter.
10-11:
Changes to fetch-pack (which is used by remote-curl) to support
"want-ref".
12-13:
Changes to the rest (fetch -> transport -> transport-helper ->
remote-curl) to support "want-ref" when fetching from HTTP. The
transport fetch function signature has been widened to allow passing
in ref names - transports may use those ref names instead of or in
addition to refs if they support it. (I chose to preserve refs in the
function signature because many parts of Git, including remote
helpers, only understand the old functionality anyway, and because
precalculating the refs allows some optimizations.)
14:
This is not meant for submission - this is just to show that the tests
pass if ref-in-want was advertised by default (except for some newly
added tests that explicitly check for the old behavior).
[1] There has been some discussion about whether the server should
accept partial ref names, e.g. [2]. In this patch set, I have made the
server only accept full names, and it is the responsibility of the
client to send the multiple patterns which it wants to match. Quoting
from the commit message of the second patch:
For example, a client could reasonably expand an abbreviated
name "foo" to "want-ref foo", "want-ref refs/heads/foo", "want-ref
refs/tags/foo", among others, and ensure that at least one such ref has
been fetched.
[2] <20161024132932.i42rqn2vlpocqmkq@sigill.intra.peff.net>
Jonathan Tan (14):
upload-pack: move parsing of "want" line
upload-pack: allow ref name and glob requests
upload-pack: test negotiation with changing repo
fetch: refactor the population of hashes
fetch: refactor fetch_refs into two functions
fetch: refactor to make function args narrower
fetch-pack: put shallow info in out param
fetch-pack: check returned refs for matches
transport: put ref oid in out param
fetch-pack: support partial names and globs
fetch-pack: support want-ref
fetch-pack: do not printf after closing stdout
fetch: send want-ref and receive fetched refs
DONT USE advertise_ref_in_want=1
Documentation/technical/http-protocol.txt | 20 +-
Documentation/technical/pack-protocol.txt | 24 +-
Documentation/technical/protocol-capabilities.txt | 6 +
builtin/clone.c | 16 +-
builtin/fetch-pack.c | 64 ++--
builtin/fetch.c | 178 +++++++---
fetch-pack.c | 226 +++++++++----
fetch-pack.h | 6 +-
remote-curl.c | 91 +++--
remote.c | 67 +++-
remote.h | 20 +-
t/lib-httpd.sh | 1 +
t/lib-httpd/apache.conf | 8 +
t/lib-httpd/one-time-sed.sh | 8 +
t/t5500-fetch-pack.sh | 82 +++++
t/t5536-fetch-conflicts.sh | 2 +
t/t5552-upload-pack-ref-in-want.sh | 386 ++++++++++++++++++++++
transport-helper.c | 105 ++++--
transport.c | 40 ++-
transport.h | 23 +-
upload-pack.c | 117 +++++--
21 files changed, 1232 insertions(+), 258 deletions(-)
create mode 100644 t/lib-httpd/one-time-sed.sh
create mode 100755 t/t5552-upload-pack-ref-in-want.sh
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply
* [RFC 02/14] upload-pack: allow ref name and glob requests
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Currently, while performing packfile negotiation [1], upload-pack allows
clients to specify their desired objects only as SHA-1s. This causes:
(a) vulnerability to failure when an object turns non-existent during
negotiation, which may happen if, for example, upload-pack is
provided by multiple Git servers in a load-balancing arrangement,
and
(b) dependence on the server first publishing a list of refs with
associated objects.
To eliminate (a) and take a step towards eliminating (b), teach
upload-pack to support requests in the form of ref names and globs (in
addition to the existing support for SHA-1s) through a new line of the
form "want-ref <ref>" where ref is the full name of a ref, a glob
pattern, or a SHA-1. At the conclusion of negotiation, the server will
write "wanted-ref <SHA-1> <name>" for all requests that have been
specified this way.
The server indicates that it supports this feature by advertising the
capability "ref-in-want". Advertisement of this capability is by default
disabled, but can be enabled through a configuration option.
To be flexible with respect to client needs, the server does not
indicate an error if a "want-ref" line corresponds to no refs, but
instead relies on the client to ensure that what the user needs has been
fetched. For example, a client could reasonably expand an abbreviated
name "foo" to "want-ref foo", "want-ref refs/heads/foo", "want-ref
refs/tags/foo", among others, and ensure that at least one such ref has
been fetched.
[1] pack-protocol.txt
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Documentation/technical/http-protocol.txt | 20 +-
Documentation/technical/pack-protocol.txt | 24 +-
Documentation/technical/protocol-capabilities.txt | 6 +
t/t5552-upload-pack-ref-in-want.sh | 295 ++++++++++++++++++++++
upload-pack.c | 89 ++++++-
5 files changed, 411 insertions(+), 23 deletions(-)
create mode 100755 t/t5552-upload-pack-ref-in-want.sh
diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt
index 1c561bdd9..162d6bc07 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -316,7 +316,8 @@ to prevent caching of the response.
Servers SHOULD support all capabilities defined here.
-Clients MUST send at least one "want" command in the request body.
+Clients MUST send at least one "want" or "want-ref" command in the
+request body.
Clients MUST NOT reference an id in a "want" command which did not
appear in the response obtained through ref discovery unless the
server advertises capability `allow-tip-sha1-in-want` or
@@ -330,7 +331,7 @@ server advertises capability `allow-tip-sha1-in-want` or
want_list = PKT-LINE(want NUL cap_list LF)
*(want_pkt)
want_pkt = PKT-LINE(want LF)
- want = "want" SP id
+ want = "want" SP id / "want-ref" SP name
cap_list = *(SP capability) SP
have_list = *PKT-LINE("have" SP id LF)
@@ -352,7 +353,8 @@ C: Build an empty set, `common`, to hold the objects that are later
determined to be on both ends.
C: Build a set, `want`, of the objects from `advertised` the client
- wants to fetch, based on what it saw during ref discovery.
+ wants to fetch, based on what it saw during ref discovery. This is to
+ be used if the server does not support the ref-in-want capability.
C: Start a queue, `c_pending`, ordered by commit time (popping newest
first). Add all client refs. When a commit is popped from
@@ -363,8 +365,8 @@ C: Start a queue, `c_pending`, ordered by commit time (popping newest
C: Send one `$GIT_URL/git-upload-pack` request:
- C: 0032want <want #1>...............................
- C: 0032want <want #2>...............................
+ C: <size>want-ref <want #1>...............................
+ C: <size>want-ref <want #2>...............................
....
C: 0032have <common #1>.............................
C: 0032have <common #2>.............................
@@ -384,7 +386,7 @@ the pkt-line value.
Commands MUST appear in the following order, if they appear
at all in the request stream:
-* "want"
+* "want" or "want-ref"
* "have"
The stream is terminated by a pkt-line flush (`0000`).
@@ -393,6 +395,9 @@ A single "want" or "have" command MUST have one hex formatted
SHA-1 as its value. Multiple SHA-1s MUST be sent by sending
multiple commands.
+A "want-ref" command MUST be followed by a ref name (which may include
+shell glob characters) or a hex formatted SHA-1.
+
The `have` list is created by popping the first 32 commits
from `c_pending`. Less can be supplied if `c_pending` empties.
@@ -410,6 +415,9 @@ Verify all objects in `want` are directly reachable from refs.
The server MAY walk backwards through history or through
the reflog to permit slightly stale requests.
+Treat "want-ref" requests as if the equivalent 0 or more "want" commands
+(according to the server's refs) were given instead.
+
If no "want" objects are received, send an error:
TODO: Define error if no "want" lines are requested.
diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt
index c59ac9936..b8b2ffdf9 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -223,15 +223,20 @@ out of what the server said it could do with the first 'want' line.
PKT-LINE("deepen-since" SP timestamp) /
PKT-LINE("deepen-not" SP ref)
- first-want = PKT-LINE("want" SP obj-id SP capability-list)
- additional-want = PKT-LINE("want" SP obj-id)
+ first-want = PKT-LINE(want SP capability-list)
+ additional-want = PKT-LINE(want)
+
+ want = "want" SP obj-id / "want-ref" SP name
depth = 1*DIGIT
----
-Clients MUST send all the obj-ids it wants from the reference
-discovery phase as 'want' lines. Clients MUST send at least one
-'want' command in the request body. Clients MUST NOT mention an
+Clients may specify the objects it wants by selecting obj-ids from the
+refs that have been advertised (using "want") or by specifying ref names
+and ref patterns directly (using "want-ref"). If the latter, the server
+will return the list of effective refs used using "wanted-ref" lines.
+Either way, clients MUST send at least one
+'want' or 'want-ref' command in the request body. Clients MUST NOT mention an
obj-id in a 'want' command which did not appear in the response
obtained through ref discovery.
@@ -249,7 +254,7 @@ complete those commits. Commits whose parents are not received as a
result are defined as shallow and marked as such in the server. This
information is sent back to the client in the next step.
-Once all the 'want's and 'shallow's (and optional 'deepen') are
+Once all the wants and 'shallow's (and optional 'deepen') are
transferred, clients MUST send a flush-pkt, to tell the server side
that it is done sending the list.
@@ -344,7 +349,9 @@ implementation if we have received at least one "ACK %s continue"
during a prior round. This helps to ensure that at least one common
ancestor is found before we give up entirely.
-Once the 'done' line is read from the client, the server will either
+Once the 'done' line is read from the client, the server will send all the
+refs corresponding to "want-ref" lines from the client (prefixed by
+"wanted-ref"), and then either
send a final 'ACK obj-id' or it will send a 'NAK'. 'obj-id' is the object
name of the last commit determined to be common. The server only sends
ACK after 'done' if there is at least one common base and multi_ack or
@@ -354,9 +361,10 @@ if there is no common base found.
Then the server will start sending its packfile data.
----
- server-response = *ack_multi ack / nak
+ server-response = *ack_multi *wanted_ref ack / nak
ack_multi = PKT-LINE("ACK" SP obj-id ack_status)
ack_status = "continue" / "common" / "ready"
+ wanted_ref = PKT-LINE("wanted-ref" SP obj-id SP name)
ack = PKT-LINE("ACK" SP obj-id)
nak = PKT-LINE("NAK")
----
diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index 26dcc6f50..72a2ff907 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -309,3 +309,9 @@ to accept a signed push certificate, and asks the <nonce> to be
included in the push certificate. A send-pack client MUST NOT
send a push-cert packet unless the receive-pack server advertises
this capability.
+
+ref-in-want
+-----------
+
+If the upload-pack server advertises this capability, fetch-pack may
+send "want-ref" lines.
diff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh
new file mode 100755
index 000000000..3496af641
--- /dev/null
+++ b/t/t5552-upload-pack-ref-in-want.sh
@@ -0,0 +1,295 @@
+#!/bin/sh
+
+test_description='upload-pack ref-in-want'
+
+. ./test-lib.sh
+
+line() {
+ len=$(printf '%s' "$1" | wc -c) &&
+ len=$((len + 5)) &&
+ printf '%04x%s\n' $len "$1" >>input
+}
+
+flush() {
+ printf '0000'
+}
+
+get_actual_refs() {
+ grep -a wanted-ref output | sed "s/^.*wanted-ref //" >actual_refs
+}
+
+get_actual_commits() {
+ perl -0777 -p -e "s/^.*PACK/PACK/s" <output >o.pack &&
+ git index-pack o.pack &&
+ git verify-pack -v o.idx | grep commit | cut -c-40 | sort >actual_commits
+}
+
+check_output() {
+ get_actual_refs &&
+ test_cmp expected_refs actual_refs &&
+ get_actual_commits &&
+ test_cmp expected_commits actual_commits
+}
+
+# c(o/foo) d(o/bar)
+# \ /
+# b e(baz) f(master)
+# \__ | __/
+# \ | /
+# a
+test_expect_success 'setup repository' '
+ test_commit a &&
+ git checkout -b o/foo &&
+ test_commit b &&
+ test_commit c &&
+ git checkout -b o/bar b &&
+ test_commit d &&
+ git checkout -b baz a &&
+ test_commit e &&
+ git checkout master &&
+ test_commit f
+'
+
+test_expect_success 'config controls ref-in-want advertisement' '
+ test_config uploadpack.advertiseRefInWant false &&
+ printf "0000" | git upload-pack . >output &&
+ ! grep -a ref-in-want output &&
+ test_config uploadpack.advertiseRefInWant true &&
+ printf "0000" | git upload-pack . >output &&
+ grep -a ref-in-want output
+'
+
+test_expect_success 'mix want and want-ref' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse f) refs/heads/master
+ EOF
+ git rev-parse e f | sort >expected_commits &&
+
+ line "want-ref refs/heads/master" >input &&
+ line "want $(git rev-parse e)" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with glob and non-glob' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse f) refs/heads/master
+ $(git rev-parse d) refs/heads/o/bar
+ $(git rev-parse c) refs/heads/o/foo
+ EOF
+ git rev-parse b c d f | sort >expected_commits &&
+
+ line "want-ref refs/head*/[op]/*" >input &&
+ line "want-ref refs/heads/master" >>input &&
+ line "want-ref refs/heads/non-existent/*" >>input &&
+ line "want-ref refs/heads/also-non-existent" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with SHA-1' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse f) $(git rev-parse f)
+ EOF
+ git rev-parse f | sort >expected_commits &&
+
+ line "want-ref $(git rev-parse f)" >input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with overlapping glob and non-glob' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse d) refs/heads/o/bar
+ $(git rev-parse c) refs/heads/o/foo
+ EOF
+ git rev-parse b c d | sort >expected_commits &&
+
+ line "want-ref refs/heads/o/*" >input &&
+ line "want-ref refs/heads/o/foo" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with overlapping non-glob and SHA-1' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse f) $(git rev-parse f)
+ $(git rev-parse f) refs/heads/master
+ EOF
+ git rev-parse f | sort >expected_commits &&
+
+ line "want-ref $(git rev-parse f)" >input &&
+ line "want-ref refs/heads/master" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with overlapping glob and SHA-1' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse f) $(git rev-parse f)
+ $(git rev-parse f) refs/heads/master
+ EOF
+ git rev-parse f | sort >expected_commits &&
+
+ line "want-ref $(git rev-parse f)" >input &&
+ line "want-ref refs/heads/mas*" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with overlapping globs' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse d) refs/heads/o/bar
+ $(git rev-parse c) refs/heads/o/foo
+ EOF
+ git rev-parse b c d | sort >expected_commits &&
+
+ line "want-ref refs/heads/o/*" >input &&
+ line "want-ref refs/heads/o/f*" >>input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'want-ref with ref we already have commit for' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse c) refs/heads/o/foo
+ EOF
+ >expected_commits &&
+
+ line "want-ref refs/heads/o/foo" >input &&
+ flush >>input &&
+ line "have $(git rev-parse c)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'send wanted-ref only at the end of negotiation' '
+ # Incomplete input; acknowledge "have" with NAK, but no wanted-ref
+ line "want-ref refs/heads/o/foo" >input &&
+ flush >>input &&
+ line "have 1234567890123456789012345678901234567890" >>input &&
+ flush >>input &&
+ test_must_fail git upload-pack . <input >output &&
+ grep -a "0008NAK" output &&
+ test_must_fail grep -a "wanted-ref" output &&
+
+ # Complete the input, and try again
+ line "have $(git rev-parse c)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ flush >>input &&
+ git upload-pack . <input >output &&
+ grep -a "wanted-ref" output
+'
+
+test_expect_success 'want-ref with capability declaration' '
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse c) refs/heads/o/foo
+ EOF
+ git rev-parse b c | sort >expected_commits &&
+
+ line "want-ref refs/heads/o/foo no_progress" >input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'hideRefs' '
+ test_config transfer.hideRefs refs/heads/o/foo &&
+
+ cat >expected_refs <<-EOF &&
+ $(git rev-parse d) refs/heads/o/bar
+ EOF
+ git rev-parse b d | sort >expected_commits &&
+
+ line "want-ref refs/heads/o/*" >input &&
+ flush >>input &&
+ line "have $(git rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'setup namespaced repo' '
+ git init n &&
+ (
+ cd n &&
+ test_commit a &&
+ test_commit b &&
+ git checkout a &&
+ test_commit c &&
+ git checkout a &&
+ test_commit d &&
+ git update-ref refs/heads/ns-no b
+ git update-ref refs/namespaces/ns/refs/heads/ns-yes c
+ git update-ref refs/namespaces/ns/refs/heads/another d
+ )
+'
+
+test_expect_success 'want-ref with namespaces' '
+ cat >expected_refs <<-EOF &&
+ $(git -C n rev-parse c) refs/heads/ns-yes
+ EOF
+ git -C n rev-parse c | sort >expected_commits &&
+
+ line "want-ref refs/heads/ns-*" >input &&
+ flush >>input &&
+ line "have $(git -C n rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git --namespace=ns -C n upload-pack . <input >output &&
+ check_output
+'
+
+test_expect_success 'hideRefs with namespaces' '
+ test_config transfer.hideRefs refs/heads/another &&
+
+ cat >expected_refs <<-EOF &&
+ $(git -C n rev-parse c) refs/heads/ns-yes
+ EOF
+ git -C n rev-parse c | sort >expected_commits &&
+
+ line "want-ref refs/heads/ns-*" >input &&
+ flush >>input &&
+ line "have $(git -C n rev-parse a)" >>input &&
+ flush >>input &&
+ line "done" >>input &&
+ git --namespace=ns -C n upload-pack . <input >output &&
+ check_output
+'
+
+test_done
diff --git a/upload-pack.c b/upload-pack.c
index 15c60a204..b88ed8e26 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -62,6 +62,7 @@ static int use_sideband;
static int advertise_refs;
static int stateless_rpc;
static const char *pack_objects_hook;
+static int advertise_ref_in_want;
static void reset_timeout(void)
{
@@ -380,7 +381,25 @@ static int ok_to_give_up(void)
return 1;
}
-static int get_common_commits(void)
+static void write_wanted_ns_refs(const struct string_list *wanted_ns_refs)
+{
+ const struct string_list_item *item;
+ for_each_string_list_item(item, wanted_ns_refs) {
+ unsigned char sha1[GIT_SHA1_RAWSZ];
+ if (!get_sha1_hex(item->string, sha1)) {
+ packet_write_fmt(1, "wanted-ref %s %s\n", item->string,
+ item->string);
+ } else {
+ if (read_ref(item->string, sha1))
+ die("unable to read ref %s", item->string);
+ packet_write_fmt(1, "wanted-ref %s %s\n",
+ sha1_to_hex(sha1),
+ strip_namespace(item->string));
+ }
+ }
+}
+
+static int get_common_commits(const struct string_list *wanted_ns_refs)
{
unsigned char sha1[20];
char last_hex[41];
@@ -442,6 +461,7 @@ static int get_common_commits(void)
continue;
}
if (!strcmp(line, "done")) {
+ write_wanted_ns_refs(wanted_ns_refs);
if (have_obj.nr > 0) {
if (multi_ack)
packet_write_fmt(1, "ACK %s\n", last_hex);
@@ -729,7 +749,26 @@ static void deepen_by_rev_list(int ac, const char **av,
packet_flush(1);
}
-static void receive_needs(void)
+static int mark_ref_wanted(const char *ns_ref,
+ const struct object_id *oid, int flags,
+ void *wanted_ns_refs_)
+{
+ struct string_list *wanted_ns_refs = wanted_ns_refs_;
+ struct object *o;
+
+ if (ref_is_hidden(strip_namespace(ns_ref), ns_ref))
+ return 0;
+
+ o = parse_object_or_die(oid->hash, ns_ref);
+ if (!(o->flags & WANTED)) {
+ o->flags |= WANTED;
+ add_object_array(o, NULL, &want_obj);
+ }
+ string_list_insert(wanted_ns_refs, ns_ref);
+ return 0;
+}
+
+static void receive_needs(struct string_list *wanted_ns_refs)
{
struct object_array shallows = OBJECT_ARRAY_INIT;
struct string_list deepen_not = STRING_LIST_INIT_DUP;
@@ -793,8 +832,35 @@ static void receive_needs(void)
deepen_rev_list = 1;
continue;
}
- if (skip_prefix(line, "want ", &arg) &&
- !get_sha1_hex(arg, sha1_buf)) {
+ if (skip_prefix(line, "want-ref ", &arg)) {
+ struct object_id oid;
+
+ char *space = strchrnul(arg, ' ');
+ char *ns_ref = xstrfmt("%s%.*s",
+ get_git_namespace(),
+ (int)(space - arg),
+ arg);
+ if (has_glob_specials(ns_ref))
+ for_each_glob_ref(mark_ref_wanted, ns_ref,
+ wanted_ns_refs);
+ else if (!get_oid_hex(arg, &oid)) {
+ o = parse_object(oid.hash);
+ if (o && !(o->flags & WANTED)) {
+ o->flags |= WANTED;
+ if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
+ || is_our_ref(o)))
+ has_non_tip = 1;
+ add_object_array(o, NULL, &want_obj);
+ }
+ mark_ref_wanted(ns_ref, &oid, 0,
+ wanted_ns_refs);
+ } else if (!read_ref(ns_ref, oid.hash))
+ mark_ref_wanted(ns_ref, &oid, 0,
+ wanted_ns_refs);
+ free(ns_ref);
+ features = space;
+ } else if (skip_prefix(line, "want ", &arg) &&
+ !get_sha1_hex(arg, sha1_buf)) {
o = parse_object(sha1_buf);
if (!o)
die("git upload-pack: not our ref %s",
@@ -806,12 +872,11 @@ static void receive_needs(void)
has_non_tip = 1;
add_object_array(o, NULL, &want_obj);
}
+ features = arg + 40;
} else
die("git upload-pack: protocol error, "
"expected to get sha, not '%s'", line);
- features = arg + 40;
-
if (parse_feature_request(features, "deepen-relative"))
deepen_relative = 1;
if (parse_feature_request(features, "multi_ack_detailed"))
@@ -935,7 +1000,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
struct strbuf symref_info = STRBUF_INIT;
format_symref_info(&symref_info, cb_data);
- packet_write_fmt(1, "%s %s%c%s%s%s%s%s agent=%s\n",
+ packet_write_fmt(1, "%s %s%c%s%s%s%s%s%s agent=%s\n",
oid_to_hex(oid), refname_nons,
0, capabilities,
(allow_unadvertised_object_request & ALLOW_TIP_SHA1) ?
@@ -943,6 +1008,8 @@ static int send_ref(const char *refname, const struct object_id *oid,
(allow_unadvertised_object_request & ALLOW_REACHABLE_SHA1) ?
" allow-reachable-sha1-in-want" : "",
stateless_rpc ? " no-done" : "",
+ advertise_ref_in_want ?
+ " ref-in-want" : "",
symref_info.buf,
git_user_agent_sanitized());
strbuf_release(&symref_info);
@@ -975,6 +1042,7 @@ static int find_symref(const char *refname, const struct object_id *oid,
static void upload_pack(void)
{
struct string_list symref = STRING_LIST_INIT_DUP;
+ struct string_list wanted_ns_refs = STRING_LIST_INIT_DUP;
head_ref_namespaced(find_symref, &symref);
@@ -992,11 +1060,12 @@ static void upload_pack(void)
if (advertise_refs)
return;
- receive_needs();
+ receive_needs(&wanted_ns_refs);
if (want_obj.nr) {
- get_common_commits();
+ get_common_commits(&wanted_ns_refs);
create_pack_file();
}
+ string_list_clear(&wanted_ns_refs, 0);
}
static int upload_pack_config(const char *var, const char *value, void *unused)
@@ -1020,6 +1089,8 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
keepalive = git_config_int(var, value);
if (!keepalive)
keepalive = -1;
+ } else if (!strcmp("uploadpack.advertiserefinwant", var)) {
+ advertise_ref_in_want = git_config_bool(var, value);
} else if (current_config_scope() != CONFIG_SCOPE_REPO) {
if (!strcmp("uploadpack.packobjectshook", var))
return git_config_string(&pack_objects_hook, var, value);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 01/14] upload-pack: move parsing of "want" line
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Refactor to parse "want" lines when the prefix is found. This makes it
easier to add support for another prefix, which will be done in a
subsequent commit.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
upload-pack.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 7597ba340..15c60a204 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -793,8 +793,20 @@ static void receive_needs(void)
deepen_rev_list = 1;
continue;
}
- if (!skip_prefix(line, "want ", &arg) ||
- get_sha1_hex(arg, sha1_buf))
+ if (skip_prefix(line, "want ", &arg) &&
+ !get_sha1_hex(arg, sha1_buf)) {
+ o = parse_object(sha1_buf);
+ if (!o)
+ die("git upload-pack: not our ref %s",
+ sha1_to_hex(sha1_buf));
+ if (!(o->flags & WANTED)) {
+ o->flags |= WANTED;
+ if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
+ || is_our_ref(o)))
+ has_non_tip = 1;
+ add_object_array(o, NULL, &want_obj);
+ }
+ } else
die("git upload-pack: protocol error, "
"expected to get sha, not '%s'", line);
@@ -820,18 +832,6 @@ static void receive_needs(void)
no_progress = 1;
if (parse_feature_request(features, "include-tag"))
use_include_tag = 1;
-
- o = parse_object(sha1_buf);
- if (!o)
- die("git upload-pack: not our ref %s",
- sha1_to_hex(sha1_buf));
- if (!(o->flags & WANTED)) {
- o->flags |= WANTED;
- if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
- || is_our_ref(o)))
- has_non_tip = 1;
- add_object_array(o, NULL, &want_obj);
- }
}
/*
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 12/14] fetch-pack: do not printf after closing stdout
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
In fetch-pack, during a stateless RPC, printf is invoked after stdout is
closed. Update the code to not do this, preserving the existing
behavior.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index ae073ab24..24af3b7c5 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -191,10 +191,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
printf("connectivity-ok\n");
fflush(stdout);
}
- close(fd[0]);
- close(fd[1]);
- if (finish_connect(conn))
- return 1;
+ if (finish_connect(conn)) {
+ ret = 1;
+ goto cleanup;
+ }
ret = !ref;
@@ -218,11 +218,17 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
ret = 1;
}
+ if (args.stateless_rpc)
+ goto cleanup;
+
while (ref) {
printf("%s %s\n",
oid_to_hex(&ref->old_oid), ref->name);
ref = ref->next;
}
+cleanup:
+ close(fd[0]);
+ close(fd[1]);
return ret;
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 13/14] fetch: send want-ref and receive fetched refs
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Teach fetch to send refspecs to the underlying transport, and teach all
components used by the HTTP transport (remote-curl, transport-helper,
fetch-pack) to understand and propagate the names and SHA-1s of the refs
fetched.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/clone.c | 4 +-
builtin/fetch-pack.c | 8 ++-
builtin/fetch.c | 100 ++++++++++++++++++++++++++++++-------
remote-curl.c | 91 ++++++++++++++++++++-------------
t/t5552-upload-pack-ref-in-want.sh | 4 +-
transport-helper.c | 74 +++++++++++++++++++++------
transport.c | 10 ++--
transport.h | 20 +++++---
8 files changed, 229 insertions(+), 82 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 3191da391..765a3a3b6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1078,7 +1078,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (!is_local && !complete_refs_before_fetch) {
- transport_fetch_refs(transport, mapped_refs,
+ transport_fetch_refs(transport, NULL, 0, mapped_refs,
&new_remote_refs);
if (new_remote_refs) {
refs = new_remote_refs;
@@ -1124,7 +1124,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs, NULL);
+ transport_fetch_refs(transport, NULL, 0, mapped_refs, NULL);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
branch_top.buf, reflog_msg.buf, transport, !is_local);
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 24af3b7c5..ed1608c12 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -35,6 +35,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
struct fetch_pack_args args;
struct sha1_array shallow = SHA1_ARRAY_INIT;
struct string_list deepen_not = STRING_LIST_INIT_DUP;
+ int always_print_refs = 0;
packet_trace_identity("fetch-pack");
@@ -126,6 +127,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
args.update_shallow = 1;
continue;
}
+ if (!strcmp("--always-print-refs", arg)) {
+ always_print_refs = 1;
+ continue;
+ }
usage(fetch_pack_usage);
}
if (deepen_not.nr)
@@ -218,7 +223,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
ret = 1;
}
- if (args.stateless_rpc)
+ if (args.stateless_rpc && !always_print_refs)
goto cleanup;
while (ref) {
@@ -226,6 +231,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
oid_to_hex(&ref->old_oid), ref->name);
ref = ref->next;
}
+ fflush(stdout);
cleanup:
close(fd[0]);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 19e3f40a0..87de00e49 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -302,10 +302,75 @@ static void find_non_local_tags(const struct ref *refs,
string_list_clear(&remote_refs, 0);
}
+static void get_effective_refspecs(struct refspec **e_rs, int *e_rs_nr,
+ const struct remote *remote,
+ const struct refspec *cli_rs, int cli_rs_nr,
+ int tags, int *autotags)
+{
+ static struct refspec head_refspec;
+
+ const struct refspec *base_rs;
+ int base_rs_nr;
+ struct branch *merge_branch = NULL;
+ int i;
+
+ struct refspec *rs;
+ int nr, alloc;
+
+ if (cli_rs_nr) {
+ base_rs = cli_rs;
+ base_rs_nr = cli_rs_nr;
+ } else if (refmap_array) {
+ die("--refmap option is only meaningful with command-line refspec(s).");
+ } else {
+ /* Use the defaults */
+ struct branch *branch = branch_get(NULL);
+ int has_merge = branch_has_merge_config(branch);
+ /* Note: has_merge implies non-NULL branch->remote_name */
+ if (has_merge && !strcmp(branch->remote_name, remote->name))
+ /*
+ * if the remote we're fetching from is the same
+ * as given in branch.<name>.remote, we add the
+ * ref given in branch.<name>.merge, too.
+ */
+ merge_branch = branch;
+ if (remote &&
+ (remote->fetch_refspec_nr || merge_branch)) {
+ base_rs = remote->fetch;
+ base_rs_nr = remote->fetch_refspec_nr;
+ } else {
+ head_refspec.src = "HEAD";
+ base_rs = &head_refspec;
+ base_rs_nr = 1;
+ }
+ }
+
+ for (i = 0; i < base_rs_nr; i++)
+ if (base_rs[i].dst && base_rs[i].dst[0]) {
+ *autotags = 1;
+ break;
+ }
+
+ alloc = base_rs_nr +
+ (merge_branch ? merge_branch->merge_nr : 0) +
+ (tags == TAGS_SET);
+ rs = xcalloc(alloc, sizeof(*rs));
+ memcpy(rs, base_rs, base_rs_nr * sizeof(*rs));
+ nr = base_rs_nr;
+ if (merge_branch)
+ for (i = 0; i < merge_branch->merge_nr; i++)
+ rs[nr++].src = merge_branch->merge[i]->src;
+ if (tags == TAGS_SET)
+ rs[nr++] = *tag_refspec;
+
+ *e_rs = rs;
+ *e_rs_nr = nr;
+}
+
static struct ref *get_ref_map(const struct remote *remote,
const struct ref *remote_refs,
struct refspec *refspecs, int refspec_count,
- int tags, int *autotags)
+ int tags, int autotags)
{
int i;
struct ref *rm;
@@ -321,11 +386,8 @@ static struct ref *get_ref_map(const struct remote *remote,
struct refspec *fetch_refspec;
int fetch_refspec_nr;
- for (i = 0; i < refspec_count; i++) {
+ for (i = 0; i < refspec_count; i++)
get_fetch_map(remote_refs, &refspecs[i], &tail, 0);
- if (refspecs[i].dst && refspecs[i].dst[0])
- *autotags = 1;
- }
/* Merge everything on the command line (but not --tags) */
for (rm = ref_map; rm; rm = rm->next)
rm->fetch_head_status = FETCH_HEAD_MERGE;
@@ -372,9 +434,6 @@ static struct ref *get_ref_map(const struct remote *remote,
(has_merge && !strcmp(branch->remote_name, remote->name)))) {
for (i = 0; i < remote->fetch_refspec_nr; i++) {
get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
- if (remote->fetch[i].dst &&
- remote->fetch[i].dst[0])
- *autotags = 1;
if (!i && !has_merge && ref_map &&
!remote->fetch[0].pattern)
ref_map->fetch_head_status = FETCH_HEAD_MERGE;
@@ -401,7 +460,7 @@ static struct ref *get_ref_map(const struct remote *remote,
if (tags == TAGS_SET)
/* also fetch all tags */
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
- else if (tags == TAGS_DEFAULT && *autotags)
+ else if (tags == TAGS_DEFAULT && autotags)
find_non_local_tags(remote_refs, &ref_map, &tail);
/* Now append any refs to be updated opportunistically: */
@@ -911,13 +970,14 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, &rm, &opt);
}
-static int fetch_refs(struct transport *transport, struct ref *ref_map,
- struct ref **updated_remote_refs)
+static int fetch_refs(struct transport *transport,
+ struct refspec *refspecs, int refspec_nr,
+ struct ref *ref_map, struct ref **updated_remote_refs)
{
int ret = quickfetch(ref_map);
if (ret)
- ret = transport_fetch_refs(transport, ref_map,
- updated_remote_refs);
+ ret = transport_fetch_refs(transport, refspecs, refspec_nr,
+ ref_map, updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1068,7 +1128,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- if (!fetch_refs(transport, ref_map, NULL))
+ if (!fetch_refs(transport, NULL, 0, ref_map, NULL))
consume_refs(transport, ref_map);
if (gsecondary) {
@@ -1083,6 +1143,10 @@ static int do_fetch(struct transport *transport,
struct ref *ref_map;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
+
+ struct refspec *e_rs;
+ int e_rs_nr;
+
const struct ref *remote_refs;
struct ref *new_remote_refs = NULL;
@@ -1103,9 +1167,11 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
+ get_effective_refspecs(&e_rs, &e_rs_nr, transport->remote,
+ refs, ref_count, tags, &autotags);
remote_refs = transport_get_remote_refs(transport);
ref_map = get_ref_map(transport->remote, remote_refs, refs, ref_count,
- tags, &autotags);
+ tags, autotags);
if (!update_head_ok)
check_not_current_branch(ref_map);
@@ -1126,7 +1192,7 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map, &new_remote_refs)) {
+ if (fetch_refs(transport, e_rs, e_rs_nr, ref_map, &new_remote_refs)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
@@ -1134,7 +1200,7 @@ static int do_fetch(struct transport *transport,
if (new_remote_refs) {
free_refs(ref_map);
ref_map = get_ref_map(transport->remote, new_remote_refs,
- refs, ref_count, tags, &autotags);
+ refs, ref_count, tags, autotags);
free_refs(new_remote_refs);
}
diff --git a/remote-curl.c b/remote-curl.c
index 34a97e732..e78959d47 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -12,6 +12,7 @@
#include "credential.h"
#include "sha1-array.h"
#include "send-pack.h"
+#include "refs.h"
static struct remote *remote;
/* always ends with a trailing slash */
@@ -31,7 +32,8 @@ struct options {
thin : 1,
/* One of the SEND_PACK_PUSH_CERT_* constants. */
push_cert : 2,
- deepen_relative : 1;
+ deepen_relative : 1,
+ echo_refs : 1;
};
static struct options options;
static struct string_list cas_options = STRING_LIST_INIT_DUP;
@@ -139,6 +141,14 @@ static int set_option(const char *name, const char *value)
else
return -1;
return 0;
+ } else if (!strcmp(name, "echo-refs")) {
+ if (!strcmp(value, "true"))
+ options.echo_refs = 1;
+ else if (!strcmp(value, "false"))
+ options.echo_refs = 0;
+ else
+ return -1;
+ return 0;
#if LIBCURL_VERSION_NUM >= 0x070a08
} else if (!strcmp(name, "family")) {
@@ -750,7 +760,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
return err;
}
-static int fetch_dumb(int nr_heads, struct ref **to_fetch)
+static int fetch_dumb(int nr_heads, const struct ref **to_fetch)
{
struct walker *walker;
char **targets;
@@ -775,11 +785,24 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch)
free(targets[i]);
free(targets);
+ if (options.echo_refs) {
+ struct strbuf sb = STRBUF_INIT;
+ for (i = 0; i < nr_heads; i++) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb,
+ "%s %s\n",
+ oid_to_hex(&to_fetch[i]->old_oid),
+ to_fetch[i]->name);
+ write_or_die(1, sb.buf, sb.len);
+ }
+ }
+
return ret ? error("fetch failed.") : 0;
}
static int fetch_git(struct discovery *heads,
- int nr_heads, struct ref **to_fetch)
+ int nr_refspec, const struct refspec *refspecs,
+ int nr_heads, const struct ref **to_fetch)
{
struct rpc_state rpc;
struct strbuf preamble = STRBUF_INIT;
@@ -811,10 +834,15 @@ static int fetch_git(struct discovery *heads,
options.deepen_not.items[i].string);
if (options.deepen_relative && options.depth)
argv_array_push(&args, "--deepen-relative");
+ if (options.echo_refs)
+ argv_array_push(&args, "--always-print-refs");
argv_array_push(&args, url.buf);
- for (i = 0; i < nr_heads; i++) {
- struct ref *ref = to_fetch[i];
+ if (refspecs) {
+ for (i = 0; i < nr_refspec; i++)
+ packet_buf_write(&preamble, "%s\n", refspecs[i].src);
+ } else {
+ const struct ref *ref = to_fetch[i];
if (!*ref->name)
die("cannot fetch by sha1 over smart http");
packet_buf_write(&preamble, "%s %s\n",
@@ -837,46 +865,38 @@ static int fetch_git(struct discovery *heads,
return err;
}
-static int fetch(int nr_heads, struct ref **to_fetch)
+static int fetch(int nr_refspec, const struct refspec *refspecs)
{
+ const struct ref **to_fetch;
+ int nr;
+ int ret, i;
struct discovery *d = discover_refs("git-upload-pack", 0);
+ get_ref_array(&to_fetch, &nr, d->refs, refspecs, nr_refspec);
+
if (d->proto_git)
- return fetch_git(d, nr_heads, to_fetch);
+ ret = fetch_git(d, nr_refspec, refspecs, nr, to_fetch);
else
- return fetch_dumb(nr_heads, to_fetch);
+ ret = fetch_dumb(nr, to_fetch);
+
+ for (i = 0; i < nr; i++) {
+ free((void *) to_fetch[i]);
+ }
+ free(to_fetch);
+
+ return ret;
}
static void parse_fetch(struct strbuf *buf)
{
- struct ref **to_fetch = NULL;
- struct ref *list_head = NULL;
- struct ref **list = &list_head;
- int alloc_heads = 0, nr_heads = 0;
+ struct refspec *to_fetch = NULL;
+ int alloc = 0, nr = 0;
do {
const char *p;
if (skip_prefix(buf->buf, "fetch ", &p)) {
- const char *name;
- struct ref *ref;
- struct object_id old_oid;
-
- if (get_oid_hex(p, &old_oid))
- die("protocol error: expected sha/ref, got %s'", p);
- if (p[GIT_SHA1_HEXSZ] == ' ')
- name = p + GIT_SHA1_HEXSZ + 1;
- else if (!p[GIT_SHA1_HEXSZ])
- name = "";
- else
- die("protocol error: expected sha/ref, got %s'", p);
-
- ref = alloc_ref(name);
- oidcpy(&ref->old_oid, &old_oid);
-
- *list = ref;
- list = &ref->next;
-
- ALLOC_GROW(to_fetch, nr_heads + 1, alloc_heads);
- to_fetch[nr_heads++] = ref;
+ nr++;
+ ALLOC_GROW(to_fetch, nr, alloc);
+ parse_ref_or_pattern(&to_fetch[nr - 1], p);
}
else
die("http transport does not support %s", buf->buf);
@@ -888,10 +908,8 @@ static void parse_fetch(struct strbuf *buf)
break;
} while (1);
- if (fetch(nr_heads, to_fetch))
+ if (fetch(nr, to_fetch))
exit(128); /* error already reported */
- free_refs(list_head);
- free(to_fetch);
printf("\n");
fflush(stdout);
@@ -1084,6 +1102,7 @@ int cmd_main(int argc, const char **argv)
printf("option\n");
printf("push\n");
printf("check-connectivity\n");
+ printf("echo-refs\n");
printf("\n");
fflush(stdout);
} else {
diff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh
index 80cf2263a..26e785f3b 100755
--- a/t/t5552-upload-pack-ref-in-want.sh
+++ b/t/t5552-upload-pack-ref-in-want.sh
@@ -345,7 +345,7 @@ test_expect_success 'server is initially ahead - no ref in want' '
grep "ERR upload-pack: not our ref" err
'
-test_expect_failure 'server is initially ahead - ref in want' '
+test_expect_success 'server is initially ahead - ref in want' '
git -C "$REPO" config uploadpack.advertiseRefInWant true &&
rm -rf local &&
cp -r "$LOCAL_PRISTINE" local &&
@@ -369,7 +369,7 @@ test_expect_success 'server is initially behind - no ref in want' '
test_cmp expected actual
'
-test_expect_failure 'server is initially behind - ref in want' '
+test_expect_success 'server is initially behind - ref in want' '
git -C "$REPO" config uploadpack.advertiseRefInWant true &&
rm -rf local &&
cp -r "$LOCAL_PRISTINE" local &&
diff --git a/transport-helper.c b/transport-helper.c
index be0aa6d39..fcd9edcdc 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -28,7 +28,8 @@ struct helper_data {
signed_tags : 1,
check_connectivity : 1,
no_disconnect_req : 1,
- no_private_update : 1;
+ no_private_update : 1,
+ echo_refs : 1;
char *export_marks;
char *import_marks;
/* These go from remote name (as in "list") to private name */
@@ -195,6 +196,8 @@ static struct child_process *get_helper(struct transport *transport)
data->import_marks = xstrdup(arg);
} else if (starts_with(capname, "no-private-update")) {
data->no_private_update = 1;
+ } else if (!strcmp(capname, "echo-refs")) {
+ data->echo_refs = 1;
} else if (mandatory) {
die("Unknown mandatory capability %s. This remote "
"helper probably needs newer version of Git.",
@@ -383,27 +386,49 @@ static int release_helper(struct transport *transport)
return res;
}
+static struct ref *copy_ref_array(struct ref **array, int nr)
+{
+ struct ref *head = NULL, **tail = &head;
+ int i;
+ for (i = 0; i < nr; i++) {
+ *tail = copy_ref(array[i]);
+ tail = &(*tail)->next;
+ }
+ return head;
+}
+
static int fetch_with_fetch(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_refspec, const struct refspec *refspecs,
+ int nr_heads, const struct ref **to_fetch,
+ struct ref **fetched_refs)
{
struct helper_data *data = transport->data;
int i;
struct strbuf buf = STRBUF_INIT;
-
- for (i = 0; i < nr_heads; i++) {
- const struct ref *posn = to_fetch[i];
- if (posn->status & REF_STATUS_UPTODATE)
- continue;
-
- strbuf_addf(&buf, "fetch %s %s\n",
- oid_to_hex(&posn->old_oid),
- posn->symref ? posn->symref : posn->name);
+ int use_echo_refs = data->echo_refs && refspecs;
+ struct ref *fetched = NULL;
+
+ if (use_echo_refs) {
+ set_helper_option(transport, "echo-refs", "true");
+ for (i = 0; i < nr_refspec; i++)
+ strbuf_addf(&buf, "fetch %s\n", refspecs[i].src);
+ } else {
+ for (i = 0; i < nr_heads; i++) {
+ const struct ref *posn = to_fetch[i];
+ if (posn->status & REF_STATUS_UPTODATE)
+ continue;
+
+ strbuf_addf(&buf, "fetch %s %s\n",
+ oid_to_hex(&posn->old_oid),
+ posn->symref ? posn->symref : posn->name);
+ }
}
strbuf_addch(&buf, '\n');
sendline(data, &buf);
while (1) {
+ struct object_id oid;
if (recvline(data, &buf))
exit(128);
@@ -418,12 +443,29 @@ static int fetch_with_fetch(struct transport *transport,
data->transport_options.check_self_contained_and_connected &&
!strcmp(buf.buf, "connectivity-ok"))
data->transport_options.self_contained_and_connected = 1;
- else if (!buf.len)
+ else if (use_echo_refs && !get_oid_hex(buf.buf, &oid)
+ && buf.buf[GIT_SHA1_HEXSZ] == ' ') {
+ struct ref *ref = alloc_ref(buf.buf + GIT_SHA1_HEXSZ + 1);
+ oidcpy(&ref->old_oid, &oid);
+ ref->next = fetched;
+ fetched = ref;
+ } else if (!buf.len)
break;
else
warning("%s unexpectedly said: '%s'", data->name, buf.buf);
}
strbuf_release(&buf);
+
+ if (use_echo_refs) {
+ if (fetched_refs)
+ *fetched_refs = fetched;
+ } else {
+ assert(fetched == NULL);
+ if (fetched_refs)
+ *fetched_refs = copy_ref_array((struct ref **) to_fetch,
+ nr_heads);
+ }
+
return 0;
}
@@ -657,6 +699,7 @@ static int connect_helper(struct transport *transport, const char *name,
}
static int fetch(struct transport *transport,
+ int nr_refspec, const struct refspec *refspecs,
int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
@@ -665,8 +708,8 @@ static int fetch(struct transport *transport,
if (process_connect(transport, 0)) {
do_take_over(transport);
- return transport->fetch(transport, nr_heads, to_fetch,
- fetched_refs);
+ return transport->fetch(transport, nr_refspec, refspecs,
+ nr_heads, to_fetch, fetched_refs);
}
count = 0;
@@ -688,7 +731,8 @@ static int fetch(struct transport *transport,
set_helper_option(transport, "update-shallow", "true");
if (data->fetch)
- return fetch_with_fetch(transport, nr_heads, to_fetch);
+ return fetch_with_fetch(transport, nr_refspec, refspecs,
+ nr_heads, to_fetch, fetched_refs);
if (data->import)
return fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);
diff --git a/transport.c b/transport.c
index 85a4c5369..734c605b1 100644
--- a/transport.c
+++ b/transport.c
@@ -95,6 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
+ int nr_refspec, const struct refspec *refspecs,
int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
@@ -203,6 +204,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
}
static int fetch_refs_via_pack(struct transport *transport,
+ int nr_refspec, const struct refspec *refspecs,
int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
@@ -1096,8 +1098,9 @@ const struct ref *transport_get_remote_refs(struct transport *transport)
return transport->remote_refs;
}
-int transport_fetch_refs(struct transport *transport, struct ref *refs,
- struct ref **fetched_refs)
+int transport_fetch_refs(struct transport *transport,
+ const struct refspec *refspecs, int refspec_nr,
+ struct ref *refs, struct ref **fetched_refs)
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
@@ -1136,7 +1139,8 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
heads[nr_heads++] = rm;
}
- rc = transport->fetch(transport, nr_heads, heads, fetched_refs);
+ rc = transport->fetch(transport, refspec_nr, refspecs,
+ nr_heads, heads, fetched_refs);
if (nop_head) {
*nop_tail = *fetched_refs;
*fetched_refs = nop_head;
diff --git a/transport.h b/transport.h
index 326ff9bd6..d7a007d21 100644
--- a/transport.h
+++ b/transport.h
@@ -82,13 +82,20 @@ struct transport {
* Fetch the objects for the given refs. Note that this gets
* an array, and should ignore the list structure.
*
+ * The user may provide the array of refspecs used to generate the
+ * given refs. If provided, the transport should prefer the refspecs if
+ * possible (but may still use the refs for pre-fetch optimizations,
+ * for example).
+ *
* The transport *may* provide, in fetched_refs, the list of refs that
- * it fetched. If the transport knows anything about the fetched refs
- * that the caller does not know (for example, shallow status or ref
- * hashes), it should provide that list of refs and include that
- * information in the list.
+ * it fetched, and must do so if it is different from the given refs.
+ * If the transport knows anything about the fetched refs that the
+ * caller does not know (for example, shallow status or ref hashes), it
+ * should provide that list of refs and include that information in the
+ * list.
**/
int (*fetch)(struct transport *transport,
+ int refspec_nr, const struct refspec *refspecs,
int refs_nr, const struct ref **refs,
struct ref **fetched_refs);
@@ -234,8 +241,9 @@ int transport_push(struct transport *connection,
const struct ref *transport_get_remote_refs(struct transport *transport);
-int transport_fetch_refs(struct transport *transport, struct ref *refs,
- struct ref **fetched_refs);
+int transport_fetch_refs(struct transport *transport,
+ const struct refspec *refspecs, int refspec_nr,
+ struct ref *refs, struct ref **fetched_refs);
void transport_unlock_pack(struct transport *transport);
int transport_disconnect(struct transport *transport);
char *transport_anonymize_url(const char *url);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 04/14] fetch: refactor the population of hashes
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Populate SHA-1 ref hashes in get_ref_map instead of do_fetch. Besides
tightening scopes of variables in the code, this also prepares for a
future patch where get_ref_map is called multiple times within do_fetch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f1570e346..c71d5eb9b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -316,6 +316,8 @@ static struct ref *get_ref_map(struct transport *transport,
const struct ref *remote_refs = transport_get_remote_refs(transport);
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
+
if (refspec_count) {
struct refspec *fetch_refspec;
int fetch_refspec_nr;
@@ -411,7 +413,23 @@ static struct ref *get_ref_map(struct transport *transport,
tail = &rm->next;
}
- return ref_remove_duplicates(ref_map);
+ ref_map = ref_remove_duplicates(ref_map);
+
+ for_each_ref(add_existing, &existing_refs);
+ for (rm = ref_map; rm; rm = rm->next) {
+ if (rm->peer_ref) {
+ struct string_list_item *peer_item =
+ string_list_lookup(&existing_refs,
+ rm->peer_ref->name);
+ if (peer_item) {
+ struct object_id *old_oid = peer_item->util;
+ oidcpy(&rm->peer_ref->old_oid, old_oid);
+ }
+ }
+ }
+ string_list_clear(&existing_refs, 1);
+
+ return ref_map;
}
#define STORE_REF_ERROR_OTHER 1
@@ -1055,14 +1073,10 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
static int do_fetch(struct transport *transport,
struct refspec *refs, int ref_count)
{
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
struct ref *ref_map;
- struct ref *rm;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
- for_each_ref(add_existing, &existing_refs);
-
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
tags = TAGS_SET;
@@ -1084,18 +1098,6 @@ static int do_fetch(struct transport *transport,
if (!update_head_ok)
check_not_current_branch(ref_map);
- for (rm = ref_map; rm; rm = rm->next) {
- if (rm->peer_ref) {
- struct string_list_item *peer_item =
- string_list_lookup(&existing_refs,
- rm->peer_ref->name);
- if (peer_item) {
- struct object_id *old_oid = peer_item->util;
- oidcpy(&rm->peer_ref->old_oid, old_oid);
- }
- }
- }
-
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
if (prune) {
@@ -1132,7 +1134,6 @@ static int do_fetch(struct transport *transport,
}
cleanup:
- string_list_clear(&existing_refs, 1);
return retcode;
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 03/14] upload-pack: test negotiation with changing repo
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Make upload-pack report "not our ref" errors to the client. (If not, the
client would be left waiting for a response when the server is already
dead.)
Add tests to check the behavior of upload-pack and fetch-pack when
upload-pack is served from a changing repository (for example, when
different servers in a load-balancing agreement participate in the same
stateless RPC negotiation). This forms a baseline of comparison to the
ref-in-want functionality (which will be introduced in subsequent
commits), and ensures that subsequent commits do not change existing
behavior.
As part of this effort, a mechanism to substitute strings in an HTTP
response only on the first invocation is added.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
t/lib-httpd.sh | 1 +
t/lib-httpd/apache.conf | 8 ++++
t/lib-httpd/one-time-sed.sh | 8 ++++
t/t5552-upload-pack-ref-in-want.sh | 91 ++++++++++++++++++++++++++++++++++++++
upload-pack.c | 6 ++-
5 files changed, 113 insertions(+), 1 deletion(-)
create mode 100644 t/lib-httpd/one-time-sed.sh
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 435a37465..84f8efdd4 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -132,6 +132,7 @@ prepare_httpd() {
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
install_script broken-smart-http.sh
install_script error.sh
+ install_script one-time-sed.sh
ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules"
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 69174c6e3..ef218ff15 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -106,9 +106,14 @@ Alias /auth/dumb/ www/auth/dumb/
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
SetEnv GIT_HTTP_EXPORT_ALL
</LocationMatch>
+<LocationMatch /one_time_sed/>
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+</LocationMatch>
ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
ScriptAlias /broken_smart/ broken-smart-http.sh/
ScriptAlias /error/ error.sh/
+ScriptAliasMatch /one_time_sed/(.*) one-time-sed.sh/$1
<Directory ${GIT_EXEC_PATH}>
Options FollowSymlinks
</Directory>
@@ -118,6 +123,9 @@ ScriptAlias /error/ error.sh/
<Files error.sh>
Options ExecCGI
</Files>
+<Files one-time-sed.sh>
+ Options ExecCGI
+</Files>
<Files ${GIT_EXEC_PATH}/git-http-backend>
Options ExecCGI
</Files>
diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
new file mode 100644
index 000000000..060ec0300
--- /dev/null
+++ b/t/lib-httpd/one-time-sed.sh
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+if [ -e one-time-sed ]; then
+ "$GIT_EXEC_PATH/git-http-backend" | sed "$(cat one-time-sed)"
+ rm one-time-sed
+else
+ "$GIT_EXEC_PATH/git-http-backend"
+fi
diff --git a/t/t5552-upload-pack-ref-in-want.sh b/t/t5552-upload-pack-ref-in-want.sh
index 3496af641..80cf2263a 100755
--- a/t/t5552-upload-pack-ref-in-want.sh
+++ b/t/t5552-upload-pack-ref-in-want.sh
@@ -292,4 +292,95 @@ test_expect_success 'hideRefs with namespaces' '
check_output
'
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
+LOCAL_PRISTINE="$(pwd)/local_pristine"
+
+test_expect_success 'setup repos for change-while-negotiating test' '
+ (
+ git init "$REPO" &&
+ cd "$REPO" &&
+ >.git/git-daemon-export-ok &&
+ test_commit m1 &&
+ git tag -d m1 &&
+
+ # Local repo with many commits (so that negotiation will take
+ # more than 1 request/response pair)
+ git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" &&
+ cd "$LOCAL_PRISTINE" &&
+ git checkout -b side &&
+ for i in $(seq 1 33); do test_commit s$i; done &&
+
+ # Add novel commits to upstream
+ git checkout master &&
+ cd "$REPO" &&
+ test_commit m2 &&
+ test_commit m3 &&
+ git tag -d m2 m3
+ ) &&
+ git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo"
+'
+
+inconsistency() {
+ # Simulate that the server initially reports $2 as the ref
+ # corresponding to $1, and after that, $1 as the ref corresponding to
+ # $1. This corresponds to the real-life situation where the server's
+ # repository appears to change during negotiation, for example, when
+ # different servers in a load-balancing arrangement serve (stateless)
+ # RPCs during a single negotiation.
+ printf "s/%s/%s/" \
+ $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
+ $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
+ >"$HTTPD_ROOT_PATH/one-time-sed"
+}
+
+test_expect_success 'server is initially ahead - no ref in want' '
+ git -C "$REPO" config uploadpack.advertiseRefInWant false &&
+ rm -rf local &&
+ cp -r "$LOCAL_PRISTINE" local &&
+ inconsistency master 1234567890123456789012345678901234567890 &&
+ test_must_fail git -C local fetch 2> err &&
+ grep "ERR upload-pack: not our ref" err
+'
+
+test_expect_failure 'server is initially ahead - ref in want' '
+ git -C "$REPO" config uploadpack.advertiseRefInWant true &&
+ rm -rf local &&
+ cp -r "$LOCAL_PRISTINE" local &&
+ inconsistency master 1234567890123456789012345678901234567890 &&
+ git -C local fetch &&
+
+ git -C "$REPO" rev-parse --verify master > expected &&
+ git -C local rev-parse --verify refs/remotes/origin/master > actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'server is initially behind - no ref in want' '
+ git -C "$REPO" config uploadpack.advertiseRefInWant false &&
+ rm -rf local &&
+ cp -r "$LOCAL_PRISTINE" local &&
+ inconsistency master "master^" &&
+ git -C local fetch &&
+
+ git -C "$REPO" rev-parse --verify "master^" > expected &&
+ git -C local rev-parse --verify refs/remotes/origin/master > actual &&
+ test_cmp expected actual
+'
+
+test_expect_failure 'server is initially behind - ref in want' '
+ git -C "$REPO" config uploadpack.advertiseRefInWant true &&
+ rm -rf local &&
+ cp -r "$LOCAL_PRISTINE" local &&
+ inconsistency master "master^" &&
+ git -C local fetch &&
+
+ git -C "$REPO" rev-parse --verify "master" > expected &&
+ git -C local rev-parse --verify refs/remotes/origin/master > actual &&
+ test_cmp expected actual
+'
+
+stop_httpd
+
test_done
diff --git a/upload-pack.c b/upload-pack.c
index b88ed8e26..0678c53d6 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)
} else if (skip_prefix(line, "want ", &arg) &&
!get_sha1_hex(arg, sha1_buf)) {
o = parse_object(sha1_buf);
- if (!o)
+ if (!o) {
+ packet_write_fmt(1,
+ "ERR upload-pack: not our ref %s",
+ sha1_to_hex(sha1_buf));
die("git upload-pack: not our ref %s",
sha1_to_hex(sha1_buf));
+ }
if (!(o->flags & WANTED)) {
o->flags |= WANTED;
if (!((allow_unadvertised_object_request & ALLOW_ANY_SHA1) == ALLOW_ANY_SHA1
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 11/14] fetch-pack: support want-ref
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Teach fetch-pack to use the want-ref mechanism whenever the server
advertises it.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 5 +-
fetch-pack.c | 173 ++++++++++++++++++++++++++++++++++++--------------
fetch-pack.h | 2 +
t/t5500-fetch-pack.sh | 42 ++++++++++++
transport.c | 2 +-
5 files changed, 175 insertions(+), 49 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 5f14242ae..ae073ab24 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -179,8 +179,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
get_ref_array(&sought_refs, &nr_sought_refs, ref, sought, nr_sought);
- ref = fetch_pack(&args, fd, conn, ref, dest, sought_refs, nr_sought_refs,
- &shallow, pack_lockfile_ptr);
+ ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,
+ sought_refs, nr_sought_refs, &shallow,
+ pack_lockfile_ptr);
if (pack_lockfile) {
printf("lock %s\n", pack_lockfile);
fflush(stdout);
diff --git a/fetch-pack.c b/fetch-pack.c
index 8cc85c19f..02149c930 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -219,11 +219,19 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd)
}
}
-static enum ack_type get_ack(int fd, unsigned char *result_sha1)
+/*
+ * Reads an ACK or NAK from fd. If wanted_ref_tail is not NULL, also accepts
+ * any "wanted-ref" lines before that ACK or NAK, writing them to
+ * wanted_ref_tail.
+ */
+static enum ack_type get_ack(int fd, unsigned char *result_sha1,
+ struct ref ***wanted_ref_tail)
{
int len;
- char *line = packet_read_line(fd, &len);
+ char *line;
const char *arg;
+start:
+ line = packet_read_line(fd, &len);
if (!len)
die(_("git fetch-pack: expected ACK/NAK, got EOF"));
@@ -244,7 +252,19 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
return ACK;
}
}
- die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
+ if (wanted_ref_tail) {
+ struct object_id oid;
+ if (skip_prefix(line, "wanted-ref ", &arg) &&
+ !get_sha1_hex(arg, oid.hash) && arg[40] == ' ' && arg[41]) {
+ struct ref *ref = alloc_ref(arg + 41);
+ oidcpy(&ref->old_oid, &oid);
+ **wanted_ref_tail = ref;
+ *wanted_ref_tail = &ref->next;
+ goto start;
+ }
+ die(_("git fetch_pack: expected ACK/NAK or wanted-ref, got '%s'"), line);
+ }
+ die(_("git fetch_pack: expected ACK/NAK, got '%s'"), line);
}
static void send_request(struct fetch_pack_args *args,
@@ -282,29 +302,55 @@ static int next_flush(struct fetch_pack_args *args, int count)
return count;
}
-static int find_common(struct fetch_pack_args *args,
- int fd[2], unsigned char *result_sha1,
- struct ref *refs)
+static void write_capabilities(struct strbuf *sb,
+ const struct fetch_pack_args *args)
{
- int fetching;
- int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;
- const unsigned char *sha1;
- unsigned in_vain = 0;
- int got_continue = 0;
- int got_ready = 0;
- struct strbuf req_buf = STRBUF_INIT;
- size_t state_len = 0;
+ if (multi_ack == 2) strbuf_addstr(sb, " multi_ack_detailed");
+ if (multi_ack == 1) strbuf_addstr(sb, " multi_ack");
+ if (no_done) strbuf_addstr(sb, " no-done");
+ if (use_sideband == 2) strbuf_addstr(sb, " side-band-64k");
+ if (use_sideband == 1) strbuf_addstr(sb, " side-band");
+ if (args->deepen_relative) strbuf_addstr(sb, " deepen-relative");
+ if (args->use_thin_pack) strbuf_addstr(sb, " thin-pack");
+ if (args->no_progress) strbuf_addstr(sb, " no-progress");
+ if (args->include_tag) strbuf_addstr(sb, " include-tag");
+ if (prefer_ofs_delta) strbuf_addstr(sb, " ofs-delta");
+ if (deepen_since_ok) strbuf_addstr(sb, " deepen-since");
+ if (deepen_not_ok) strbuf_addstr(sb, " deepen-not");
+ if (agent_supported) strbuf_addf(sb, " agent=%s",
+ git_user_agent_sanitized());
+}
- if (args->stateless_rpc && multi_ack == 1)
- die(_("--stateless-rpc requires multi_ack_detailed"));
- if (marked)
- for_each_ref(clear_marks, NULL);
- marked = 1;
+static void write_wants(struct strbuf *sb, const struct fetch_pack_args *args,
+ const struct refspec *refspecs, int nr_refspec,
+ struct ref *refs)
+{
+ int capabilities_written = 0;
- for_each_ref(rev_list_insert_ref_oid, NULL);
- for_each_alternate_ref(insert_one_alternate_ref, NULL);
+ if (refspecs) {
+ int i;
+ for (i = 0; i < nr_refspec; i++) {
+ const char *to_send = (refspecs[i].src && refspecs[i].src[0])
+ ? refspecs[i].src : "HEAD";
+ if (i == 0) {
+ struct strbuf c = STRBUF_INIT;
+ write_capabilities(&c, args);
+ packet_buf_write(sb, "want-ref %s%s\n",
+ to_send, c.buf);
+ strbuf_release(&c);
+ } else
+ packet_buf_write(sb, "want-ref %s\n", to_send);
+
+ /* write everything that refname_match supports */
+ packet_buf_write(sb, "want-ref refs/%s\n", to_send);
+ packet_buf_write(sb, "want-ref refs/tags/%s\n", to_send);
+ packet_buf_write(sb, "want-ref refs/heads/%s\n", to_send);
+ packet_buf_write(sb, "want-ref refs/remotes/%s\n", to_send);
+ packet_buf_write(sb, "want-ref refs/remotes/%s/HEAD\n", to_send);
+ }
+ return;
+ }
- fetching = 0;
for ( ; refs ; refs = refs->next) {
unsigned char *remote = refs->old_oid.hash;
const char *remote_hex;
@@ -326,30 +372,41 @@ static int find_common(struct fetch_pack_args *args,
}
remote_hex = sha1_to_hex(remote);
- if (!fetching) {
+ if (!capabilities_written) {
struct strbuf c = STRBUF_INIT;
- if (multi_ack == 2) strbuf_addstr(&c, " multi_ack_detailed");
- if (multi_ack == 1) strbuf_addstr(&c, " multi_ack");
- if (no_done) strbuf_addstr(&c, " no-done");
- if (use_sideband == 2) strbuf_addstr(&c, " side-band-64k");
- if (use_sideband == 1) strbuf_addstr(&c, " side-band");
- if (args->deepen_relative) strbuf_addstr(&c, " deepen-relative");
- if (args->use_thin_pack) strbuf_addstr(&c, " thin-pack");
- if (args->no_progress) strbuf_addstr(&c, " no-progress");
- if (args->include_tag) strbuf_addstr(&c, " include-tag");
- if (prefer_ofs_delta) strbuf_addstr(&c, " ofs-delta");
- if (deepen_since_ok) strbuf_addstr(&c, " deepen-since");
- if (deepen_not_ok) strbuf_addstr(&c, " deepen-not");
- if (agent_supported) strbuf_addf(&c, " agent=%s",
- git_user_agent_sanitized());
- packet_buf_write(&req_buf, "want %s%s\n", remote_hex, c.buf);
+ write_capabilities(&c, args);
+ packet_buf_write(sb, "want %s%s\n", remote_hex, c.buf);
strbuf_release(&c);
+ capabilities_written = 1;
} else
- packet_buf_write(&req_buf, "want %s\n", remote_hex);
- fetching++;
+ packet_buf_write(sb, "want %s\n", remote_hex);
}
+}
+
+static int find_common(struct fetch_pack_args *args,
+ int fd[2], unsigned char *result_sha1,
+ struct strbuf *wants, struct ref **wanted_refs)
+{
+ int count = 0, flushes = 0, flush_at = INITIAL_FLUSH, retval;
+ const unsigned char *sha1;
+ unsigned in_vain = 0;
+ int got_continue = 0;
+ int got_ready = 0;
+ struct strbuf req_buf = STRBUF_INIT;
+ size_t state_len = 0;
+
+ if (args->stateless_rpc && multi_ack == 1)
+ die(_("--stateless-rpc requires multi_ack_detailed"));
+ if (marked)
+ for_each_ref(clear_marks, NULL);
+ marked = 1;
- if (!fetching) {
+ for_each_ref(rev_list_insert_ref_oid, NULL);
+ for_each_alternate_ref(insert_one_alternate_ref, NULL);
+
+ strbuf_swap(&req_buf, wants);
+
+ if (!req_buf.len) {
strbuf_release(&req_buf);
packet_flush(fd[1]);
return 1;
@@ -435,7 +492,7 @@ static int find_common(struct fetch_pack_args *args,
consume_shallow_list(args, fd[0]);
do {
- ack = get_ack(fd[0], result_sha1);
+ ack = get_ack(fd[0], result_sha1, NULL);
if (ack)
print_verbose(args, _("got %s %d %s"), "ack",
ack, sha1_to_hex(result_sha1));
@@ -504,7 +561,9 @@ static int find_common(struct fetch_pack_args *args,
if (!got_ready || !no_done)
consume_shallow_list(args, fd[0]);
while (flushes || multi_ack) {
- int ack = get_ack(fd[0], result_sha1);
+ struct ref *wr = NULL, **wr_tail = ≀
+ int ack = get_ack(fd[0], result_sha1, &wr_tail);
+ *wanted_refs = wr;
if (ack) {
print_verbose(args, _("got %s (%d) %s"), "ack",
ack, sha1_to_hex(result_sha1));
@@ -835,6 +894,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
+ const struct refspec *refspecs, int nr_refspec,
const struct ref **sought, int nr_sought,
struct shallow_info *si,
char **pack_lockfile)
@@ -843,6 +903,10 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
unsigned char sha1[20];
const char *agent_feature;
int agent_len;
+ int ref_in_want = 0;
+ struct strbuf wants = STRBUF_INIT;
+ struct ref *wanted_refs = NULL;
+ int want_ref_used = 0;
sort_ref_list(&ref, ref_compare_name);
QSORT(sought, nr_sought, cmp_ref_by_name);
@@ -907,17 +971,26 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
die(_("Server does not support --shallow-exclude"));
if (!server_supports("deepen-relative") && args->deepen_relative)
die(_("Server does not support --deepen"));
+ if (server_supports("ref-in-want"))
+ ref_in_want = 1;
if (everything_local(args, &ref, sought, nr_sought)) {
packet_flush(fd[1]);
goto all_done;
}
- if (find_common(args, fd, sha1, ref) < 0)
+
+ if (ref_in_want && refspecs) {
+ write_wants(&wants, args, refspecs, nr_refspec, NULL);
+ want_ref_used = 1;
+ } else
+ write_wants(&wants, args, NULL, 0, ref);
+ if (find_common(args, fd, sha1, &wants, &wanted_refs) < 0)
if (!args->keep_pack)
/* When cloning, it is not unusual to have
* no common commit.
*/
warning(_("no common commits"));
+ strbuf_release(&wants);
if (args->stateless_rpc)
packet_flush(fd[1]);
@@ -932,6 +1005,13 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
die(_("git fetch-pack: fetch failed."));
all_done:
+ if (want_ref_used) {
+ free_refs(ref);
+ return wanted_refs;
+ }
+
+ if (wanted_refs)
+ die("Protocol error: we are not using ref-in-want but server still sends wanted-ref");
return ref;
}
@@ -1082,6 +1162,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
+ const struct refspec *refspecs, int nr_refspec,
const struct ref **sought, int nr_sought,
struct sha1_array *shallow,
char **pack_lockfile)
@@ -1098,8 +1179,8 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
die(_("no matching remote head"));
}
prepare_shallow_info(&si, shallow);
- ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
- &si, pack_lockfile);
+ ref_cpy = do_fetch_pack(args, fd, ref, refspecs, nr_refspec,
+ sought, nr_sought, &si, pack_lockfile);
reprepare_packed_git();
update_shallow(args, ref_cpy, &si);
clear_shallow_info(&si);
diff --git a/fetch-pack.h b/fetch-pack.h
index 6e4fdbb68..06eb0fb28 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -5,6 +5,7 @@
#include "run-command.h"
struct sha1_array;
+struct refspec;
struct fetch_pack_args {
const char *uploadpack;
@@ -38,6 +39,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
+ const struct refspec *refspecs, int nr_refspec,
const struct ref **sought,
int nr_sought,
struct sha1_array *shallow,
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index cb1b7d949..18fe23c97 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -563,6 +563,25 @@ test_expect_success 'fetch-pack can fetch refs using a partial name' '
grep "$(printf "%s refs/heads/one" $(git -C server rev-parse --verify one))" actual
'
+test_expect_success 'fetch-pack can fetch refs using a partial name using want-ref' '
+ rm -rf server &&
+ git init server &&
+ (
+ cd server &&
+ git config uploadpack.advertiseRefInWant true
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b one
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server one >actual &&
+ echo here &&
+ grep " want-ref " trace &&
+ ! grep " want " trace &&
+
+ grep "$(printf "%s refs/heads/one" $(git -C server rev-parse --verify one))" actual
+'
+
test_expect_success 'fetch-pack can fetch refs using a glob' '
rm -rf server &&
git init server &&
@@ -585,6 +604,29 @@ test_expect_success 'fetch-pack can fetch refs using a glob' '
grep "$(printf "%s refs/heads/onc" $(git -C server rev-parse --verify onc))" actual
'
+test_expect_success 'fetch-pack can fetch refs using a glob using want-ref' '
+ rm -rf server &&
+ git init server &&
+ (
+ cd server &&
+ git config uploadpack.advertiseRefInWant true
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b ona &&
+ git checkout -b onb &&
+ test_commit 3 &&
+ git checkout -b onc
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server "refs/heads/on*" >actual &&
+ grep " want-ref " trace &&
+ ! grep " want " trace &&
+
+ grep "$(printf "%s refs/heads/ona" $(git -C server rev-parse --verify ona))" actual &&
+ grep "$(printf "%s refs/heads/onb" $(git -C server rev-parse --verify onb))" actual &&
+ grep "$(printf "%s refs/heads/onc" $(git -C server rev-parse --verify onc))" actual
+'
+
check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
diff --git a/transport.c b/transport.c
index 5ed3fc68e..85a4c5369 100644
--- a/transport.c
+++ b/transport.c
@@ -239,7 +239,7 @@ static int fetch_refs_via_pack(struct transport *transport,
refs = fetch_pack(&args, data->fd, data->conn,
refs_tmp ? refs_tmp : transport->remote_refs,
- dest, to_fetch, nr_heads, &data->shallow,
+ dest, NULL, 0, to_fetch, nr_heads, &data->shallow,
&transport->pack_lockfile);
close(data->fd[0]);
close(data->fd[1]);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 07/14] fetch-pack: put shallow info in out param
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Expand the transport fetch method signature, by adding an out parameter,
to allow transports to return information about the refs they have
fetched. Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.
This does require clients to sometimes generate the ref map twice: one
generation from the list of refs provided by the remote (as is currently
done) and potentially one regeneration from the new list of refs that
the fetch mechanism provides (added in this patch). The double
generation may result in duplicate error messages when a remote-tracking
branch seems to track more than one remote branch.
This is the 1st of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/clone.c | 4 ++--
builtin/fetch.c | 23 +++++++++++++++++++----
fetch-pack.c | 18 +++++++++++-------
remote.c | 12 +++++++++++-
remote.h | 1 +
t/t5536-fetch-conflicts.sh | 2 ++
transport-helper.c | 5 +++--
transport.c | 32 ++++++++++++++++++++++++++------
transport.h | 12 ++++++++++--
9 files changed, 85 insertions(+), 24 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5ef81927a..0135c5f1c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1076,7 +1076,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (!is_local && !complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs);
+ transport_fetch_refs(transport, mapped_refs, NULL);
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1115,7 +1115,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs);
+ transport_fetch_refs(transport, mapped_refs, NULL);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
branch_top.buf, reflog_msg.buf, transport, !is_local);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ae7c6daa1..19e3f40a0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -911,11 +911,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, &rm, &opt);
}
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
{
int ret = quickfetch(ref_map);
if (ret)
- ret = transport_fetch_refs(transport, ref_map);
+ ret = transport_fetch_refs(transport, ref_map,
+ updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1066,7 +1068,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- if (!fetch_refs(transport, ref_map))
+ if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
if (gsecondary) {
@@ -1082,6 +1084,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+ struct ref *new_remote_refs = NULL;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1123,7 +1126,19 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
+ if (fetch_refs(transport, ref_map, &new_remote_refs)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
+ if (new_remote_refs) {
+ free_refs(ref_map);
+ ref_map = get_ref_map(transport->remote, new_remote_refs,
+ refs, ref_count, tags, &autotags);
+ free_refs(new_remote_refs);
+ }
+
+ if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f0779a..9a87ddd3d 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -974,12 +974,13 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)
}
static void update_shallow(struct fetch_pack_args *args,
- struct ref **sought, int nr_sought,
+ struct ref *refs,
struct shallow_info *si)
{
struct sha1_array ref = SHA1_ARRAY_INIT;
int *status;
int i;
+ struct ref *r;
if (args->deepen && alternate_shallow_file) {
if (*alternate_shallow_file == '\0') { /* --unshallow */
@@ -1021,8 +1022,8 @@ static void update_shallow(struct fetch_pack_args *args,
remove_nonexistent_theirs_shallow(si);
if (!si->nr_ours && !si->nr_theirs)
return;
- for (i = 0; i < nr_sought; i++)
- sha1_array_append(&ref, sought[i]->old_oid.hash);
+ for (r = refs; r; r = r->next)
+ sha1_array_append(&ref, r->old_oid.hash);
si->ref = &ref;
if (args->update_shallow) {
@@ -1056,12 +1057,15 @@ static void update_shallow(struct fetch_pack_args *args,
* remote is also shallow, check what ref is safe to update
* without updating .git/shallow
*/
- status = xcalloc(nr_sought, sizeof(*status));
+ status = xcalloc(ref.nr, sizeof(*status));
assign_shallow_commits_to_refs(si, NULL, status);
if (si->nr_ours || si->nr_theirs) {
- for (i = 0; i < nr_sought; i++)
+ i = 0;
+ for (r = refs; r; r = r->next) {
if (status[i])
- sought[i]->status = REF_STATUS_REJECT_SHALLOW;
+ r->status = REF_STATUS_REJECT_SHALLOW;
+ i++;
+ }
}
free(status);
sha1_array_clear(&ref);
@@ -1090,7 +1094,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
ref_cpy = do_fetch_pack(args, fd, ref, sought, nr_sought,
&si, pack_lockfile);
reprepare_packed_git();
- update_shallow(args, sought, nr_sought, &si);
+ update_shallow(args, ref_cpy, &si);
clear_shallow_info(&si);
return ref_cpy;
}
diff --git a/remote.c b/remote.c
index d5eaec737..725a2d39a 100644
--- a/remote.c
+++ b/remote.c
@@ -929,7 +929,7 @@ struct ref *alloc_ref(const char *name)
return alloc_ref_with_prefix("", 0, name);
}
-struct ref *copy_ref(const struct ref *ref)
+struct ref *copy_ref_peerless(const struct ref *ref)
{
struct ref *cpy;
size_t len;
@@ -941,6 +941,16 @@ struct ref *copy_ref(const struct ref *ref)
cpy->next = NULL;
cpy->symref = xstrdup_or_null(ref->symref);
cpy->remote_status = xstrdup_or_null(ref->remote_status);
+ cpy->peer_ref = NULL;
+ return cpy;
+}
+
+struct ref *copy_ref(const struct ref *ref)
+{
+ struct ref *cpy;
+ if (!ref)
+ return NULL;
+ cpy = copy_ref_peerless(ref);
cpy->peer_ref = copy_ref(ref->peer_ref);
return cpy;
}
diff --git a/remote.h b/remote.h
index 924881169..57d059431 100644
--- a/remote.h
+++ b/remote.h
@@ -131,6 +131,7 @@ struct ref {
extern struct ref *find_ref_by_name(const struct ref *list, const char *name);
struct ref *alloc_ref(const char *name);
+struct ref *copy_ref_peerless(const struct ref *ref);
struct ref *copy_ref(const struct ref *ref);
struct ref *copy_ref_list(const struct ref *ref);
void sort_ref_list(struct ref **, int (*cmp)(const void *, const void *));
diff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh
index 2e42cf331..0cb380795 100755
--- a/t/t5536-fetch-conflicts.sh
+++ b/t/t5536-fetch-conflicts.sh
@@ -93,6 +93,8 @@ test_expect_success 'fetch conflict: criss-cross args' '
verify_stderr <<-\EOF
warning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2
warning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1
+ warning: refs/remotes/origin/branch1 usually tracks refs/heads/branch1, not refs/heads/branch2
+ warning: refs/remotes/origin/branch2 usually tracks refs/heads/branch2, not refs/heads/branch1
EOF
)
'
diff --git a/transport-helper.c b/transport-helper.c
index 91aed35eb..f3d78bb9e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -646,14 +646,15 @@ static int connect_helper(struct transport *transport, const char *name,
}
static int fetch(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch, struct ref **fetched_refs)
{
struct helper_data *data = transport->data;
int i, count;
if (process_connect(transport, 0)) {
do_take_over(transport);
- return transport->fetch(transport, nr_heads, to_fetch);
+ return transport->fetch(transport, nr_heads, to_fetch,
+ fetched_refs);
}
count = 0;
diff --git a/transport.c b/transport.c
index c86ba2eb8..80e007f2f 100644
--- a/transport.c
+++ b/transport.c
@@ -95,7 +95,8 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch,
+ struct ref **fetched_refs)
{
struct bundle_transport_data *data = transport->data;
return unbundle(&data->header, data->fd,
@@ -202,7 +203,8 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
}
static int fetch_refs_via_pack(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch,
+ struct ref **fetched_refs)
{
struct git_transport_data *data = transport->data;
struct ref *refs;
@@ -250,8 +252,12 @@ static int fetch_refs_via_pack(struct transport *transport,
data->options.self_contained_and_connected =
args.self_contained_and_connected;
+ if (fetched_refs)
+ *fetched_refs = refs;
+ else
+ free_refs(refs);
+
free_refs(refs_tmp);
- free_refs(refs);
free(dest);
return (refs ? 0 : -1);
}
@@ -1090,19 +1096,29 @@ const struct ref *transport_get_remote_refs(struct transport *transport)
return transport->remote_refs;
}
-int transport_fetch_refs(struct transport *transport, struct ref *refs)
+int transport_fetch_refs(struct transport *transport, struct ref *refs,
+ struct ref **fetched_refs)
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
struct ref **heads = NULL;
+ struct ref *nop_head = NULL, **nop_tail = &nop_head;
struct ref *rm;
for (rm = refs; rm; rm = rm->next) {
nr_refs++;
if (rm->peer_ref &&
!is_null_oid(&rm->old_oid) &&
- !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid))
+ !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) {
+ /* These need to be reported as fetched, but we do not
+ * actually need to fetch them. */
+ if (fetched_refs) {
+ struct ref *nop_ref = copy_ref_peerless(rm);
+ *nop_tail = nop_ref;
+ nop_tail = &nop_ref->next;
+ }
continue;
+ }
ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
heads[nr_heads++] = rm;
}
@@ -1120,7 +1136,11 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
heads[nr_heads++] = rm;
}
- rc = transport->fetch(transport, nr_heads, heads);
+ rc = transport->fetch(transport, nr_heads, heads, fetched_refs);
+ if (nop_head) {
+ *nop_tail = *fetched_refs;
+ *fetched_refs = nop_head;
+ }
free(heads);
return rc;
diff --git a/transport.h b/transport.h
index 9820f10b8..b9e7e4656 100644
--- a/transport.h
+++ b/transport.h
@@ -82,11 +82,18 @@ struct transport {
* Fetch the objects for the given refs. Note that this gets
* an array, and should ignore the list structure.
*
+ * The transport *may* provide, in fetched_refs, the list of refs that
+ * it fetched. If the transport knows anything about the fetched refs
+ * that the caller does not know (for example, shallow status), it
+ * should provide that list of refs and include that information in the
+ * list.
+ *
* If the transport did not get hashes for refs in
* get_refs_list(), it should set the old_sha1 fields in the
* provided refs now.
**/
- int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
+ int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,
+ struct ref **fetched_refs);
/**
* Push the objects and refs. Send the necessary objects, and
@@ -230,7 +237,8 @@ int transport_push(struct transport *connection,
const struct ref *transport_get_remote_refs(struct transport *transport);
-int transport_fetch_refs(struct transport *transport, struct ref *refs);
+int transport_fetch_refs(struct transport *transport, struct ref *refs,
+ struct ref **fetched_refs);
void transport_unlock_pack(struct transport *transport);
int transport_disconnect(struct transport *transport);
char *transport_anonymize_url(const char *url);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 14/14] DONT USE advertise_ref_in_want=1
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
---
t/t5500-fetch-pack.sh | 2 ++
upload-pack.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 18fe23c97..f39dbcab8 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -551,6 +551,7 @@ test_expect_success 'fetch-pack can fetch refs using a partial name' '
git init server &&
(
cd server &&
+ git config uploadpack.advertiseRefInWant false
test_commit 1 &&
test_commit 2 &&
git checkout -b one
@@ -587,6 +588,7 @@ test_expect_success 'fetch-pack can fetch refs using a glob' '
git init server &&
(
cd server &&
+ git config uploadpack.advertiseRefInWant false
test_commit 1 &&
test_commit 2 &&
git checkout -b ona &&
diff --git a/upload-pack.c b/upload-pack.c
index 0678c53d6..4998a8c7e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -62,7 +62,7 @@ static int use_sideband;
static int advertise_refs;
static int stateless_rpc;
static const char *pack_objects_hook;
-static int advertise_ref_in_want;
+static int advertise_ref_in_want = 1;
static void reset_timeout(void)
{
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 05/14] fetch: refactor fetch_refs into two functions
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them. This prepares for a
future patch where some processing may be done between those tasks.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c71d5eb9b..43e35c494 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -918,10 +918,16 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
int ret = quickfetch(ref_map);
if (ret)
ret = transport_fetch_refs(transport, ref_map);
- if (!ret)
- ret |= store_updated_refs(transport->url,
- transport->remote->name,
- ref_map);
+ if (ret)
+ transport_unlock_pack(transport);
+ return ret;
+}
+
+static int consume_refs(struct transport *transport, struct ref *ref_map)
+{
+ int ret = store_updated_refs(transport->url,
+ transport->remote->name,
+ ref_map);
transport_unlock_pack(transport);
return ret;
}
@@ -1062,7 +1068,8 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- fetch_refs(transport, ref_map);
+ if (!fetch_refs(transport, ref_map))
+ consume_refs(transport, ref_map);
if (gsecondary) {
transport_disconnect(gsecondary);
@@ -1115,7 +1122,7 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map)) {
+ if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 06/14] fetch: refactor to make function args narrower
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Refactor find_non_local_tags and get_ref_map to only take the
information they need instead of the entire transport struct. Besides
improving code clarity, this also improves their flexibility, which will
be needed in a future patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 43e35c494..ae7c6daa1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -220,7 +220,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
return 0;
}
-static void find_non_local_tags(struct transport *transport,
+static void find_non_local_tags(const struct ref *refs,
struct ref **head,
struct ref ***tail)
{
@@ -230,7 +230,7 @@ static void find_non_local_tags(struct transport *transport,
struct string_list_item *item = NULL;
for_each_ref(add_existing, &existing_refs);
- for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
+ for (ref = refs; ref; ref = ref->next) {
if (!starts_with(ref->name, "refs/tags/"))
continue;
@@ -302,7 +302,8 @@ static void find_non_local_tags(struct transport *transport,
string_list_clear(&remote_refs, 0);
}
-static struct ref *get_ref_map(struct transport *transport,
+static struct ref *get_ref_map(const struct remote *remote,
+ const struct ref *remote_refs,
struct refspec *refspecs, int refspec_count,
int tags, int *autotags)
{
@@ -314,8 +315,6 @@ static struct ref *get_ref_map(struct transport *transport,
/* opportunistically-updated references: */
struct ref *orefs = NULL, **oref_tail = &orefs;
- const struct ref *remote_refs = transport_get_remote_refs(transport);
-
struct string_list existing_refs = STRING_LIST_INIT_DUP;
if (refspec_count) {
@@ -355,8 +354,8 @@ static struct ref *get_ref_map(struct transport *transport,
fetch_refspec = parse_fetch_refspec(refmap_nr, refmap_array);
fetch_refspec_nr = refmap_nr;
} else {
- fetch_refspec = transport->remote->fetch;
- fetch_refspec_nr = transport->remote->fetch_refspec_nr;
+ fetch_refspec = remote->fetch;
+ fetch_refspec_nr = remote->fetch_refspec_nr;
}
for (i = 0; i < fetch_refspec_nr; i++)
@@ -365,7 +364,6 @@ static struct ref *get_ref_map(struct transport *transport,
die("--refmap option is only meaningful with command-line refspec(s).");
} else {
/* Use the defaults */
- struct remote *remote = transport->remote;
struct branch *branch = branch_get(NULL);
int has_merge = branch_has_merge_config(branch);
if (remote &&
@@ -404,7 +402,7 @@ static struct ref *get_ref_map(struct transport *transport,
/* also fetch all tags */
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
else if (tags == TAGS_DEFAULT && *autotags)
- find_non_local_tags(transport, &ref_map, &tail);
+ find_non_local_tags(remote_refs, &ref_map, &tail);
/* Now append any refs to be updated opportunistically: */
*tail = orefs;
@@ -1083,6 +1081,7 @@ static int do_fetch(struct transport *transport,
struct ref *ref_map;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
+ const struct ref *remote_refs;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1101,7 +1100,9 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
- ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
+ remote_refs = transport_get_remote_refs(transport);
+ ref_map = get_ref_map(transport->remote, remote_refs, refs, ref_count,
+ tags, &autotags);
if (!update_head_ok)
check_not_current_branch(ref_map);
@@ -1134,7 +1135,7 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT && autotags) {
struct ref **tail = &ref_map;
ref_map = NULL;
- find_non_local_tags(transport, &ref_map, &tail);
+ find_non_local_tags(remote_refs, &ref_map, &tail);
if (ref_map)
backfill_tags(transport, ref_map);
free_refs(ref_map);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 09/14] transport: put ref oid in out param
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Return new OID information obtained through fetching in new structs
instead of reusing the existing ones. With this change, the input
structs are no longer used for output, and are thus marked const.
This is the 3rd of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/clone.c | 14 ++++++++++++--
builtin/fetch-pack.c | 4 ++--
fetch-pack.c | 26 +++++++++++++++-----------
fetch-pack.h | 2 +-
transport-helper.c | 34 +++++++++++++++++++++++-----------
transport.c | 6 +++---
transport.h | 13 +++++--------
7 files changed, 61 insertions(+), 38 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 0135c5f1c..3191da391 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -858,6 +858,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
struct refspec *refspec;
const char *fetch_pattern;
+ struct ref *new_remote_refs = NULL;
+
packet_trace_identity("clone");
argc = parse_options(argc, argv, prefix, builtin_clone_options,
builtin_clone_usage, 0);
@@ -1075,8 +1077,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
break;
}
- if (!is_local && !complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs, NULL);
+ if (!is_local && !complete_refs_before_fetch) {
+ transport_fetch_refs(transport, mapped_refs,
+ &new_remote_refs);
+ if (new_remote_refs) {
+ refs = new_remote_refs;
+ free_refs(mapped_refs);
+ mapped_refs = wanted_peer_refs(refs, refspec);
+ }
+ }
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1148,5 +1157,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
junk_mode = JUNK_LEAVE_ALL;
free(refspec);
+ free_refs(new_remote_refs);
return err;
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f31f874a0..a18fd0c44 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -11,7 +11,7 @@ static const char fetch_pack_usage[] =
"[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
-static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
+static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,
const char *name)
{
struct ref *ref;
@@ -44,7 +44,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
int i, ret;
struct ref *ref = NULL;
const char *dest = NULL;
- struct ref **sought = NULL;
+ const struct ref **sought = NULL;
int nr_sought = 0, alloc_sought = 0;
int fd[2];
char *pack_lockfile = NULL;
diff --git a/fetch-pack.c b/fetch-pack.c
index d4642b05c..8cc85c19f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -52,6 +52,10 @@ static int non_common_revs, multi_ack, use_sideband;
#define ALLOW_REACHABLE_SHA1 02
static unsigned int allow_unadvertised_object_request;
+/* An arbitrary non-NULL non-const pointer to assign to the util field of
+ * string list items when we need one. */
+#define ARBITRARY (&transfer_unpack_limit)
+
__attribute__((format (printf, 2, 3)))
static inline void print_verbose(const struct fetch_pack_args *args,
const char *fmt, ...)
@@ -556,7 +560,7 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,
static void filter_refs(struct fetch_pack_args *args,
struct ref **refs,
- struct ref **sought, int nr_sought)
+ const struct ref **sought, int nr_sought)
{
struct ref *newlist = NULL;
struct ref **newtail = &newlist;
@@ -604,16 +608,16 @@ static void filter_refs(struct fetch_pack_args *args,
for (i = 0; i < nr_sought; i++) {
unsigned char sha1[20];
- ref = sought[i];
+ const struct ref *sref = sought[i];
if (matched[i])
continue;
- if (get_sha1_hex(ref->name, sha1) ||
- ref->name[40] != '\0' ||
- hashcmp(sha1, ref->old_oid.hash))
+ if (get_sha1_hex(sref->name, sha1) ||
+ sref->name[40] != '\0' ||
+ hashcmp(sha1, sref->old_oid.hash))
continue;
matched[i] = 1;
- *newtail = copy_ref(ref);
+ *newtail = copy_ref(sref);
newtail = &(*newtail)->next;
}
}
@@ -629,7 +633,7 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)
static int everything_local(struct fetch_pack_args *args,
struct ref **refs,
- struct ref **sought, int nr_sought)
+ const struct ref **sought, int nr_sought)
{
struct ref *ref;
int retval;
@@ -831,7 +835,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
- struct ref **sought, int nr_sought,
+ const struct ref **sought, int nr_sought,
struct shallow_info *si,
char **pack_lockfile)
{
@@ -955,7 +959,7 @@ static void fetch_pack_setup(void)
did_setup = 1;
}
-static int remove_duplicates_in_refs(struct ref **ref, int nr)
+static int remove_duplicates_in_refs(const struct ref **ref, int nr)
{
struct string_list names = STRING_LIST_INIT_NODUP;
int src, dst;
@@ -965,7 +969,7 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)
item = string_list_insert(&names, ref[src]->name);
if (item->util)
continue; /* already have it */
- item->util = ref[src];
+ item->util = ARBITRARY;
if (src != dst)
ref[dst] = ref[src];
dst++;
@@ -1078,7 +1082,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
- struct ref **sought, int nr_sought,
+ const struct ref **sought, int nr_sought,
struct sha1_array *shallow,
char **pack_lockfile)
{
diff --git a/fetch-pack.h b/fetch-pack.h
index 76f7c719c..6e4fdbb68 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -38,7 +38,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
- struct ref **sought,
+ const struct ref **sought,
int nr_sought,
struct sha1_array *shallow,
char **pack_lockfile);
diff --git a/transport-helper.c b/transport-helper.c
index f3d78bb9e..be0aa6d39 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -384,7 +384,7 @@ static int release_helper(struct transport *transport)
}
static int fetch_with_fetch(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, const struct ref **to_fetch)
{
struct helper_data *data = transport->data;
int i;
@@ -477,13 +477,14 @@ static int get_exporter(struct transport *transport,
}
static int fetch_with_import(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, const struct ref **to_fetch, struct ref **fetched_refs)
{
struct child_process fastimport;
struct helper_data *data = transport->data;
int i;
- struct ref *posn;
+ const struct ref *posn;
struct strbuf buf = STRBUF_INIT;
+ struct ref *fr_head = NULL, **fr_tail = &fr_head;
get_helper(transport);
@@ -522,28 +523,38 @@ static int fetch_with_import(struct transport *transport,
* (If no "refspec" capability was specified, for historical
* reasons we default to the equivalent of *:*.)
*
- * Store the result in to_fetch[i].old_sha1. Callers such
+ * Store the result in old_oid in fetched_refs. Callers such
* as "git fetch" can use the value to write feedback to the
* terminal, populate FETCH_HEAD, and determine what new value
* should be written to peer_ref if the update is a
* fast-forward or this is a forced update.
*/
+ if (!fetched_refs)
+ goto cleanup;
for (i = 0; i < nr_heads; i++) {
- char *private, *name;
- posn = to_fetch[i];
- if (posn->status & REF_STATUS_UPTODATE)
+ struct ref *ref;
+ char *private;
+ const char *name;
+
+ ref = copy_ref_peerless(to_fetch[i]);
+ *fr_tail = ref;
+ fr_tail = &ref->next;
+ if (ref->status & REF_STATUS_UPTODATE)
continue;
- name = posn->symref ? posn->symref : posn->name;
+ name = ref->symref ? ref->symref : ref->name;
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, name);
else
private = xstrdup(name);
if (private) {
- if (read_ref(private, posn->old_oid.hash) < 0)
+ if (read_ref(private, ref->old_oid.hash) < 0)
die("Could not read ref %s", private);
free(private);
}
}
+ *fetched_refs = fr_head;
+
+cleanup:
strbuf_release(&buf);
return 0;
}
@@ -646,7 +657,8 @@ static int connect_helper(struct transport *transport, const char *name,
}
static int fetch(struct transport *transport,
- int nr_heads, struct ref **to_fetch, struct ref **fetched_refs)
+ int nr_heads, const struct ref **to_fetch,
+ struct ref **fetched_refs)
{
struct helper_data *data = transport->data;
int i, count;
@@ -679,7 +691,7 @@ static int fetch(struct transport *transport,
return fetch_with_fetch(transport, nr_heads, to_fetch);
if (data->import)
- return fetch_with_import(transport, nr_heads, to_fetch);
+ return fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);
return -1;
}
diff --git a/transport.c b/transport.c
index 80e007f2f..5ed3fc68e 100644
--- a/transport.c
+++ b/transport.c
@@ -95,7 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
- int nr_heads, struct ref **to_fetch,
+ int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
struct bundle_transport_data *data = transport->data;
@@ -203,7 +203,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
}
static int fetch_refs_via_pack(struct transport *transport,
- int nr_heads, struct ref **to_fetch,
+ int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
struct git_transport_data *data = transport->data;
@@ -1101,7 +1101,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
- struct ref **heads = NULL;
+ const struct ref **heads = NULL;
struct ref *nop_head = NULL, **nop_tail = &nop_head;
struct ref *rm;
diff --git a/transport.h b/transport.h
index b9e7e4656..326ff9bd6 100644
--- a/transport.h
+++ b/transport.h
@@ -84,15 +84,12 @@ struct transport {
*
* The transport *may* provide, in fetched_refs, the list of refs that
* it fetched. If the transport knows anything about the fetched refs
- * that the caller does not know (for example, shallow status), it
- * should provide that list of refs and include that information in the
- * list.
- *
- * If the transport did not get hashes for refs in
- * get_refs_list(), it should set the old_sha1 fields in the
- * provided refs now.
+ * that the caller does not know (for example, shallow status or ref
+ * hashes), it should provide that list of refs and include that
+ * information in the list.
**/
- int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,
+ int (*fetch)(struct transport *transport,
+ int refs_nr, const struct ref **refs,
struct ref **fetched_refs);
/**
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 08/14] fetch-pack: check returned refs for matches
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Instead of setting "matched" on matched refs, detect matches by checking
the sought refs against the returned refs. Also, since the "matched"
field in struct ref is now no longer used, remove it.
This is the 2nd of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.
(There are possibly ways more efficient than a nested for loop to
accomplish this. However, since a subsequent patch will compare the
user-provided refspecs against the fetched refs directly, and thus
necessitating the nested for loop anyway, I decided to use the nested
for loop in this patch.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 7 ++++++-
fetch-pack.c | 9 ++++++---
fetch-pack.h | 2 --
remote.h | 3 +--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e447c..f31f874a0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -4,6 +4,7 @@
#include "remote.h"
#include "connect.h"
#include "sha1-array.h"
+#include "refs.h"
static const char fetch_pack_usage[] =
"git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] "
@@ -220,7 +221,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
* an error.
*/
for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
+ struct ref *r;
+ for (r = ref; r; r = r->next)
+ if (!sought[i] || refname_match(sought[i]->name, r->name))
+ break;
+ if (r)
continue;
error("no such remote ref %s", sought[i]->name);
ret = 1;
diff --git a/fetch-pack.c b/fetch-pack.c
index 9a87ddd3d..d4642b05c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -562,6 +562,7 @@ static void filter_refs(struct fetch_pack_args *args,
struct ref **newtail = &newlist;
struct ref *ref, *next;
int i;
+ int *matched = xcalloc(nr_sought, sizeof(*matched));
i = 0;
for (ref = *refs; ref; ref = next) {
@@ -578,7 +579,7 @@ static void filter_refs(struct fetch_pack_args *args,
break; /* definitely do not have it */
else if (cmp == 0) {
keep = 1; /* definitely have it */
- sought[i]->matched = 1;
+ matched[i] = 1;
}
i++;
}
@@ -604,19 +605,21 @@ static void filter_refs(struct fetch_pack_args *args,
unsigned char sha1[20];
ref = sought[i];
- if (ref->matched)
+ if (matched[i])
continue;
if (get_sha1_hex(ref->name, sha1) ||
ref->name[40] != '\0' ||
hashcmp(sha1, ref->old_oid.hash))
continue;
- ref->matched = 1;
+ matched[i] = 1;
*newtail = copy_ref(ref);
newtail = &(*newtail)->next;
}
}
*refs = newlist;
+
+ free(matched);
}
static void mark_alternate_complete(const struct ref *ref, void *unused)
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d32..76f7c719c 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -33,8 +33,6 @@ struct fetch_pack_args {
/*
* sought represents remote references that should be updated from.
- * On return, the names that were found on the remote will have been
- * marked as such.
*/
struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
diff --git a/remote.h b/remote.h
index 57d059431..2f7f23d47 100644
--- a/remote.h
+++ b/remote.h
@@ -89,8 +89,7 @@ struct ref {
force:1,
forced_update:1,
expect_old_sha1:1,
- deletion:1,
- matched:1;
+ deletion:1;
/*
* Order is important here, as we write to FETCH_HEAD
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 10/14] fetch-pack: support partial names and globs
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Teach fetch-pack to support partial ref names and ref patterns as input.
This does not use "want-ref" yet - support for that will be added in a
future patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 40 ++++++++++++-------------------------
remote.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
remote.h | 16 +++++++++++++++
t/t5500-fetch-pack.sh | 38 +++++++++++++++++++++++++++++++++++
4 files changed, 122 insertions(+), 27 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index a18fd0c44..5f14242ae 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -11,32 +11,12 @@ static const char fetch_pack_usage[] =
"[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
-static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,
+static void add_sought_entry(struct refspec **sought, int *nr, int *alloc,
const char *name)
{
- struct ref *ref;
- struct object_id oid;
-
- if (!get_oid_hex(name, &oid)) {
- if (name[GIT_SHA1_HEXSZ] == ' ') {
- /* <sha1> <ref>, find refname */
- name += GIT_SHA1_HEXSZ + 1;
- } else if (name[GIT_SHA1_HEXSZ] == '\0') {
- ; /* <sha1>, leave sha1 as name */
- } else {
- /* <ref>, clear cruft from oid */
- oidclr(&oid);
- }
- } else {
- /* <ref>, clear cruft from get_oid_hex */
- oidclr(&oid);
- }
-
- ref = alloc_ref(name);
- oidcpy(&ref->old_oid, &oid);
(*nr)++;
ALLOC_GROW(*sought, *nr, *alloc);
- (*sought)[*nr - 1] = ref;
+ parse_ref_or_pattern(&(*sought)[*nr - 1], name);
}
int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
@@ -44,8 +24,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
int i, ret;
struct ref *ref = NULL;
const char *dest = NULL;
- const struct ref **sought = NULL;
+ struct refspec *sought = NULL;
int nr_sought = 0, alloc_sought = 0;
+ const struct ref **sought_refs;
+ int nr_sought_refs;
int fd[2];
char *pack_lockfile = NULL;
char **pack_lockfile_ptr = NULL;
@@ -195,8 +177,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
return args.diag_url ? 0 : 1;
}
get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
+ get_ref_array(&sought_refs, &nr_sought_refs, ref, sought, nr_sought);
- ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,
+ ref = fetch_pack(&args, fd, conn, ref, dest, sought_refs, nr_sought_refs,
&shallow, pack_lockfile_ptr);
if (pack_lockfile) {
printf("lock %s\n", pack_lockfile);
@@ -222,12 +205,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
*/
for (i = 0; i < nr_sought; i++) {
struct ref *r;
- for (r = ref; r; r = r->next)
- if (!sought[i] || refname_match(sought[i]->name, r->name))
+ if (sought[i].pattern)
+ continue; /* patterns do not need to match anything */
+ for (r = ref; r; r = r->next) {
+ if (refname_match(sought[i].src, r->name))
break;
+ }
if (r)
continue;
- error("no such remote ref %s", sought[i]->name);
+ error("no such remote ref %s", sought[i].src);
ret = 1;
}
diff --git a/remote.c b/remote.c
index 725a2d39a..08f3c910e 100644
--- a/remote.c
+++ b/remote.c
@@ -612,6 +612,39 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
die("Invalid refspec '%s'", refspec[i]);
}
+void parse_ref_or_pattern(struct refspec *refspec, const char *str)
+{
+ struct object_id oid;
+ memset(refspec, 0, sizeof(*refspec));
+
+ if (!get_oid_hex(str, &oid)) {
+ if (str[GIT_SHA1_HEXSZ] == ' ') {
+ struct object_id oid2;
+ /* <sha1> <ref>, find refname */
+ refspec->src = xstrdup(str + GIT_SHA1_HEXSZ + 1);
+ if (!get_oid_hex(refspec->src, &oid2)
+ && !oidcmp(&oid, &oid2))
+ /* The name is actually a SHA-1 */
+ refspec->exact_sha1 = 1;
+ } else if (str[GIT_SHA1_HEXSZ] == '\0') {
+ ; /* <sha1>, leave sha1 as name */
+ refspec->src = xstrdup(str);
+ refspec->exact_sha1 = 1;
+ } else {
+ /* <ref> */
+ refspec->src = xstrdup(str);
+ }
+ } else {
+ /* <ref> */
+ refspec->src = xstrdup(str);
+ }
+
+ if (has_glob_specials(refspec->src)) {
+ refspec->pattern = 1;
+ refspec->dst = refspec->src;
+ }
+}
+
int valid_fetch_refspec(const char *fetch_refspec_str)
{
struct refspec *refspec;
@@ -1924,6 +1957,28 @@ int get_fetch_map(const struct ref *remote_refs,
return 0;
}
+void get_ref_array(const struct ref ***refs, int *nr_ref,
+ const struct ref *remote_refs,
+ const struct refspec *refspecs, int nr_refspecs)
+{
+ struct ref *head = NULL, **tail = &head;
+ const struct ref **array = NULL;
+ int nr = 0, alloc = 0;
+
+ struct ref *r;
+ int i;
+
+ for (i = 0; i < nr_refspecs; i++)
+ get_fetch_map(remote_refs, &refspecs[i], &tail, 1);
+ for (r = head; r; r = r->next) {
+ nr++;
+ ALLOC_GROW(array, nr, alloc);
+ array[nr - 1] = r;
+ }
+ *refs = array;
+ *nr_ref = nr;
+}
+
int resolve_remote_symref(struct ref *ref, struct ref *list)
{
if (!ref->symref)
diff --git a/remote.h b/remote.h
index 2f7f23d47..daca1c65e 100644
--- a/remote.h
+++ b/remote.h
@@ -162,6 +162,13 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
*/
struct ref *ref_remove_duplicates(struct ref *ref_map);
+/*
+ * Parse the given ref or ref pattern. If a ref, write a refspec with that ref
+ * as src, and with an empty dst. If a ref pattern, write a glob refspec with
+ * that pattern as src and dst.
+ */
+void parse_ref_or_pattern(struct refspec *refspec, const char *str);
+
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
@@ -192,6 +199,15 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
int get_fetch_map(const struct ref *remote_refs, const struct refspec *refspec,
struct ref ***tail, int missing_ok);
+/*
+ * Convenience function to generate an array of refs corresponding to the given
+ * refspecs. This is equivalent to repeatedly calling get_fetch_map and
+ * rearranging the returned refs as an array.
+ */
+void get_ref_array(const struct ref ***refs, int *nr_ref,
+ const struct ref *remote_refs,
+ const struct refspec *refspecs, int nr_refspecs);
+
struct ref *get_remote_ref(const struct ref *remote_refs, const char *name);
/*
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4a7..cb1b7d949 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,6 +547,44 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
'
+test_expect_success 'fetch-pack can fetch refs using a partial name' '
+ git init server &&
+ (
+ cd server &&
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b one
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server one >actual &&
+ grep " want " trace &&
+ ! grep " want-ref " trace &&
+
+ grep "$(printf "%s refs/heads/one" $(git -C server rev-parse --verify one))" actual
+'
+
+test_expect_success 'fetch-pack can fetch refs using a glob' '
+ rm -rf server &&
+ git init server &&
+ (
+ cd server &&
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b ona &&
+ git checkout -b onb &&
+ test_commit 3 &&
+ git checkout -b onc
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server "refs/heads/on*" >actual &&
+ grep " want " trace &&
+ ! grep " want-ref " trace &&
+
+ grep "$(printf "%s refs/heads/ona" $(git -C server rev-parse --verify ona))" actual &&
+ grep "$(printf "%s refs/heads/onb" $(git -C server rev-parse --verify onb))" actual &&
+ grep "$(printf "%s refs/heads/onc" $(git -C server rev-parse --verify onc))" actual
+'
+
check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Jeff King @ 2017-01-25 22:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20170125215931.26339-1-sbeller@google.com>
On Wed, Jan 25, 2017 at 01:59:31PM -0800, Stefan Beller wrote:
> This applies to the repo at https://github.com/git/git.github.io
Thanks. I've applied and pushed, though I'll admit I didn't really read
it carefully for content. A few of the ideas look like they would need
to be aggregated to be big enough for a SoC project, but that can be
fleshed out in future patches (i.e., I don't think we care enough about
history to have people polish and re-roll what are essentially wiki
edits).
If you (or anybody interested in working on this) would prefer direct
push access to the git.github.io repo, I'm happy to set that up.
> If you're looking for a co-admin/mentors, I can help out.
I may take you up on the co-admin thing. I think it's good to have a
backup, just in case.
Anything you put on the Ideas page, you should probably be willing to
mentor. We definitely _don't_ want Ideas that don't have a mentor.
I think in general that each idea should have the possible mentors
listed below it.
-Peff
^ 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