Skip to content
Browse files

Merge branch 'nd/fetch-into-shallow'

When there is no sufficient overlap between old and new history
during a fetch into a shallow repository, we unnecessarily sent
objects the sending side knows the receiving end has.

* nd/fetch-into-shallow:
  Add testcase for needless objects during a shallow fetch
  list-objects: mark more commits as edges in mark_edges_uninteresting
  list-objects: reduce one argument in mark_edges_uninteresting
  upload-pack: delegate rev walking in shallow fetch to pack-objects
  shallow: add setup_temporary_shallow()
  shallow: only add shallow graft points to new shallow file
  move setup_alternate_shallow and write_shallow_commits to shallow.c
  • Loading branch information...
2 parents 79e46c9 + f21d2a7 commit 238504b014230d0bc244fb0de84990863fcddd59 @gitster gitster committed Sep 20, 2013
Showing with 152 additions and 160 deletions.
  1. +1 −1 bisect.c
  2. +1 −1 builtin/pack-objects.c
  3. +1 −1 builtin/rev-list.c
  4. +4 −0 commit.h
  5. +1 −52 fetch-pack.c
  6. +1 −1 http-push.c
  7. +20 −4 list-objects.c
  8. +1 −1 list-objects.h
  9. +79 −0 shallow.c
  10. +11 −0 t/t5500-fetch-pack.sh
  11. +0 −3 t/t5530-upload-pack-error.sh
  12. +32 −96 upload-pack.c
View
2 bisect.c
@@ -624,7 +624,7 @@ static void bisect_common(struct rev_info *revs)
if (prepare_revision_walk(revs))
die("revision walk setup failed");
if (revs->tree_objects)
- mark_edges_uninteresting(revs->commits, revs, NULL);
+ mark_edges_uninteresting(revs, NULL);
}
static void exit_if_skipped_commits(struct commit_list *tried,
View
2 builtin/pack-objects.c
@@ -2378,7 +2378,7 @@ static void get_object_list(int ac, const char **av)
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
- mark_edges_uninteresting(revs.commits, &revs, show_edge);
+ mark_edges_uninteresting(&revs, show_edge);
traverse_commit_list(&revs, show_commit, show_object, NULL);
if (keep_unreachable)
View
2 builtin/rev-list.c
@@ -336,7 +336,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
if (revs.tree_objects)
- mark_edges_uninteresting(revs.commits, &revs, show_edge);
+ mark_edges_uninteresting(&revs, show_edge);
if (bisect_list) {
int reaches = reaches, all = all;
View
4 commit.h
@@ -201,6 +201,10 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag);
extern void check_shallow_file_for_update(void);
extern void set_alternate_shallow_file(const char *path);
+extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol);
+extern void setup_alternate_shallow(struct lock_file *shallow_lock,
+ const char **alternate_shallow_file);
+extern char *setup_temporary_shallow(void);
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit *);
View
53 fetch-pack.c
@@ -185,36 +185,6 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd)
}
}
-struct write_shallow_data {
- struct strbuf *out;
- int use_pack_protocol;
- int count;
-};
-
-static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
-{
- struct write_shallow_data *data = cb_data;
- const char *hex = sha1_to_hex(graft->sha1);
- data->count++;
- if (data->use_pack_protocol)
- packet_buf_write(data->out, "shallow %s", hex);
- else {
- strbuf_addstr(data->out, hex);
- strbuf_addch(data->out, '\n');
- }
- return 0;
-}
-
-static int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
-{
- struct write_shallow_data data;
- data.out = out;
- data.use_pack_protocol = use_pack_protocol;
- data.count = 0;
- for_each_commit_graft(write_one_shallow, &data);
- return data.count;
-}
-
static enum ack_type get_ack(int fd, unsigned char *result_sha1)
{
int len;
@@ -796,27 +766,6 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
return strcmp(a->name, b->name);
}
-static void setup_alternate_shallow(void)
-{
- struct strbuf sb = STRBUF_INIT;
- int fd;
-
- check_shallow_file_for_update();
- fd = hold_lock_file_for_update(&shallow_lock, git_path("shallow"),
- LOCK_DIE_ON_ERROR);
- if (write_shallow_commits(&sb, 0)) {
- if (write_in_full(fd, sb.buf, sb.len) != sb.len)
- die_errno("failed to write to %s", shallow_lock.filename);
- alternate_shallow_file = shallow_lock.filename;
- } else
- /*
- * is_repository_shallow() sees empty string as "no
- * shallow file".
- */
- alternate_shallow_file = "";
- strbuf_release(&sb);
-}
-
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
@@ -897,7 +846,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
if (args->stateless_rpc)
packet_flush(fd[1]);
if (args->depth > 0)
- setup_alternate_shallow();
+ setup_alternate_shallow(&shallow_lock, &alternate_shallow_file);
else
alternate_shallow_file = NULL;
if (get_pack(args, fd, pack_lockfile))
View
2 http-push.c
@@ -1975,7 +1975,7 @@ int main(int argc, char **argv)
pushing = 0;
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
- mark_edges_uninteresting(revs.commits, &revs, NULL);
+ mark_edges_uninteresting(&revs, NULL);
objects_to_send = get_delta(&revs, ref_lock);
finish_all_active_slots();
View
24 list-objects.c
@@ -144,19 +144,35 @@ static void mark_edge_parents_uninteresting(struct commit *commit,
}
}
-void mark_edges_uninteresting(struct commit_list *list,
- struct rev_info *revs,
- show_edge_fn show_edge)
+void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)
{
- for ( ; list; list = list->next) {
+ struct commit_list *list;
+ int i;
+
+ for (list = revs->commits; list; list = list->next) {
struct commit *commit = list->item;
if (commit->object.flags & UNINTERESTING) {
mark_tree_uninteresting(commit->tree);
+ if (revs->edge_hint && !(commit->object.flags & SHOWN)) {
+ commit->object.flags |= SHOWN;
+ show_edge(commit);
+ }
continue;
}
mark_edge_parents_uninteresting(commit, revs, show_edge);
}
+ for (i = 0; i < revs->cmdline.nr; i++) {
+ struct object *obj = revs->cmdline.rev[i].item;
+ struct commit *commit = (struct commit *)obj;
+ if (obj->type != OBJ_COMMIT || !(obj->flags & UNINTERESTING))
+ continue;
+ mark_tree_uninteresting(commit->tree);
+ if (revs->edge_hint && !(obj->flags & SHOWN)) {
+ obj->flags |= SHOWN;
+ show_edge(commit);
+ }
+ }
}
static void add_pending_tree(struct rev_info *revs, struct tree *tree)
View
2 list-objects.h
@@ -6,6 +6,6 @@ typedef void (*show_object_fn)(struct object *, const struct name_path *, const
void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);
typedef void (*show_edge_fn)(struct commit *);
-void mark_edges_uninteresting(struct commit_list *, struct rev_info *, show_edge_fn);
+void mark_edges_uninteresting(struct rev_info *, show_edge_fn);
#endif
View
79 shallow.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "commit.h"
#include "tag.h"
+#include "pkt-line.h"
static int is_shallow = -1;
static struct stat shallow_stat;
@@ -141,3 +142,81 @@ void check_shallow_file_for_update(void)
)
die("shallow file was changed during fetch");
}
+
+struct write_shallow_data {
+ struct strbuf *out;
+ int use_pack_protocol;
+ int count;
+};
+
+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
+{
+ struct write_shallow_data *data = cb_data;
+ const char *hex = sha1_to_hex(graft->sha1);
+ if (graft->nr_parent != -1)
+ return 0;
+ data->count++;
+ if (data->use_pack_protocol)
+ packet_buf_write(data->out, "shallow %s", hex);
+ else {
+ strbuf_addstr(data->out, hex);
+ strbuf_addch(data->out, '\n');
+ }
+ return 0;
+}
+
+int write_shallow_commits(struct strbuf *out, int use_pack_protocol)
+{
+ struct write_shallow_data data;
+ data.out = out;
+ data.use_pack_protocol = use_pack_protocol;
+ data.count = 0;
+ for_each_commit_graft(write_one_shallow, &data);
+ return data.count;
+}
+
+char *setup_temporary_shallow(void)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+
+ if (write_shallow_commits(&sb, 0)) {
+ struct strbuf path = STRBUF_INIT;
+ strbuf_addstr(&path, git_path("shallow_XXXXXX"));
+ fd = xmkstemp(path.buf);
+ if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+ die_errno("failed to write to %s",
+ path.buf);
+ close(fd);
+ strbuf_release(&sb);
+ return strbuf_detach(&path, NULL);
+ }
+ /*
+ * is_repository_shallow() sees empty string as "no shallow
+ * file".
+ */
+ return xstrdup("");
+}
+
+void setup_alternate_shallow(struct lock_file *shallow_lock,
+ const char **alternate_shallow_file)
+{
+ struct strbuf sb = STRBUF_INIT;
+ int fd;
+
+ check_shallow_file_for_update();
+ fd = hold_lock_file_for_update(shallow_lock, git_path("shallow"),
+ LOCK_DIE_ON_ERROR);
+ if (write_shallow_commits(&sb, 0)) {
+ if (write_in_full(fd, sb.buf, sb.len) != sb.len)
+ die_errno("failed to write to %s",
+ shallow_lock->filename);
+ *alternate_shallow_file = shallow_lock->filename;
+ } else
+ /*
+ * is_repository_shallow() sees empty string as "no
+ * shallow file".
+ */
+ *alternate_shallow_file = "";
+ strbuf_release(&sb);
+}
View
11 t/t5500-fetch-pack.sh
@@ -393,6 +393,17 @@ test_expect_success 'fetch in shallow repo unreachable shallow objects' '
git fsck --no-dangling
)
'
+test_expect_success 'fetch creating new shallow root' '
+ (
+ git clone "file://$(pwd)/." shallow10 &&
+ git commit --allow-empty -m empty &&
+ cd shallow10 &&
+ git fetch --depth=1 --progress 2>actual &&
+ # This should fetch only the empty commit, no tree or
+ # blob objects
+ grep "remote: Total 1" actual
+ )
+'
test_expect_success 'setup tests for the --stdin parameter' '
for head in C D E F
View
3 t/t5530-upload-pack-error.sh
@@ -54,9 +54,6 @@ test_expect_success 'upload-pack fails due to error in rev-list' '
printf "0032want %s\n0034shallow %s00000009done\n0000" \
$(git rev-parse HEAD) $(git rev-parse HEAD^) >input &&
test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
- # pack-objects survived
- grep "Total.*, reused" output.err &&
- # but there was an error, which must have been in rev-list
grep "bad tree object" output.err
'
View
128 upload-pack.c
@@ -69,87 +69,28 @@ static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
return sz;
}
-static FILE *pack_pipe = NULL;
-static void show_commit(struct commit *commit, void *data)
-{
- if (commit->object.flags & BOUNDARY)
- fputc('-', pack_pipe);
- if (fputs(sha1_to_hex(commit->object.sha1), pack_pipe) < 0)
- die("broken output pipe");
- fputc('\n', pack_pipe);
- fflush(pack_pipe);
- free(commit->buffer);
- commit->buffer = NULL;
-}
-
-static void show_object(struct object *obj,
- const struct name_path *path, const char *component,
- void *cb_data)
-{
- show_object_with_name(pack_pipe, obj, path, component);
-}
-
-static void show_edge(struct commit *commit)
-{
- fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1));
-}
-
-static int do_rev_list(int in, int out, void *user_data)
-{
- int i;
- struct rev_info revs;
-
- pack_pipe = xfdopen(out, "w");
- init_revisions(&revs, NULL);
- revs.tag_objects = 1;
- revs.tree_objects = 1;
- revs.blob_objects = 1;
- if (use_thin_pack)
- revs.edge_hint = 1;
-
- for (i = 0; i < want_obj.nr; i++) {
- struct object *o = want_obj.objects[i].item;
- /* why??? */
- o->flags &= ~UNINTERESTING;
- add_pending_object(&revs, o, NULL);
- }
- for (i = 0; i < have_obj.nr; i++) {
- struct object *o = have_obj.objects[i].item;
- o->flags |= UNINTERESTING;
- add_pending_object(&revs, o, NULL);
- }
- setup_revisions(0, NULL, &revs, NULL);
- if (prepare_revision_walk(&revs))
- die("revision walk setup failed");
- mark_edges_uninteresting(revs.commits, &revs, show_edge);
- if (use_thin_pack)
- for (i = 0; i < extra_edge_obj.nr; i++)
- fprintf(pack_pipe, "-%s\n", sha1_to_hex(
- extra_edge_obj.objects[i].item->sha1));
- traverse_commit_list(&revs, show_commit, show_object, NULL);
- fflush(pack_pipe);
- fclose(pack_pipe);
- return 0;
-}
-
static void create_pack_file(void)
{
- struct async rev_list;
struct child_process pack_objects;
char data[8193], progress[128];
char abort_msg[] = "aborting due to possible repository "
"corruption on the remote side.";
int buffered = -1;
ssize_t sz;
- const char *argv[10];
- int arg = 0;
+ const char *argv[12];
+ int i, arg = 0;
+ FILE *pipe_fd;
+ char *shallow_file = NULL;
- argv[arg++] = "pack-objects";
- if (!shallow_nr) {
- argv[arg++] = "--revs";
- if (use_thin_pack)
- argv[arg++] = "--thin";
+ if (shallow_nr) {
+ shallow_file = setup_temporary_shallow();
+ argv[arg++] = "--shallow-file";
+ argv[arg++] = shallow_file;
}
+ argv[arg++] = "pack-objects";
+ argv[arg++] = "--revs";
+ if (use_thin_pack)
+ argv[arg++] = "--thin";
argv[arg++] = "--stdout";
if (!no_progress)
@@ -170,29 +111,21 @@ static void create_pack_file(void)
if (start_command(&pack_objects))
die("git upload-pack: unable to fork git-pack-objects");
- if (shallow_nr) {
- memset(&rev_list, 0, sizeof(rev_list));
- rev_list.proc = do_rev_list;
- rev_list.out = pack_objects.in;
- if (start_async(&rev_list))
- die("git upload-pack: unable to fork git-rev-list");
- }
- else {
- FILE *pipe_fd = xfdopen(pack_objects.in, "w");
- int i;
-
- for (i = 0; i < want_obj.nr; i++)
- fprintf(pipe_fd, "%s\n",
- sha1_to_hex(want_obj.objects[i].item->sha1));
- fprintf(pipe_fd, "--not\n");
- for (i = 0; i < have_obj.nr; i++)
- fprintf(pipe_fd, "%s\n",
- sha1_to_hex(have_obj.objects[i].item->sha1));
- fprintf(pipe_fd, "\n");
- fflush(pipe_fd);
- fclose(pipe_fd);
- }
-
+ pipe_fd = xfdopen(pack_objects.in, "w");
+
+ for (i = 0; i < want_obj.nr; i++)
+ fprintf(pipe_fd, "%s\n",
+ sha1_to_hex(want_obj.objects[i].item->sha1));
+ fprintf(pipe_fd, "--not\n");
+ for (i = 0; i < have_obj.nr; i++)
+ fprintf(pipe_fd, "%s\n",
+ sha1_to_hex(have_obj.objects[i].item->sha1));
+ for (i = 0; i < extra_edge_obj.nr; i++)
+ fprintf(pipe_fd, "%s\n",
+ sha1_to_hex(extra_edge_obj.objects[i].item->sha1));
+ fprintf(pipe_fd, "\n");
+ fflush(pipe_fd);
+ fclose(pipe_fd);
/* We read from pack_objects.err to capture stderr output for
* progress bar, and pack_objects.out to capture the pack data.
@@ -291,8 +224,11 @@ static void create_pack_file(void)
error("git upload-pack: git-pack-objects died with error.");
goto fail;
}
- if (shallow_nr && finish_async(&rev_list))
- goto fail; /* error was already reported */
+ if (shallow_file) {
+ if (*shallow_file)
+ unlink(shallow_file);
+ free(shallow_file);
+ }
/* flush the data */
if (0 <= buffered) {

0 comments on commit 238504b

Please sign in to comment.
Something went wrong with that request. Please try again.