From 7073e69e382bc8247c28859d8b0eda2612cd6b94 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Tue, 6 Feb 2007 16:08:06 -0500 Subject: [PATCH] Don't do non-fastforward updates in fast-import. If fast-import is being used to update an existing branch of a repository, the user may not want to lose commits if another process updates the same ref at the same time. For example, the user might be using fast-import to make just one or two commits against a live branch. We now perform a fast-forward check during the ref updating process. If updating a branch would cause commits in that branch to be lost, we skip over it and display the new SHA1 to standard error. This new default behavior can be overridden with `--force`, like git-push and git-fetch. Signed-off-by: Shawn O. Pearce --- Documentation/git-fast-import.txt | 22 +++++++-- fast-import.c | 58 +++++++++++++++++----- t/t9300-fast-import.sh | 80 +++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 17 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 08450de9a..2be6c4b80 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -38,6 +38,11 @@ OPTIONS See ``Date Formats'' below for details about which formats are supported, and their syntax. +--force:: + Force updating modified existing branches, even if doing + so would cause commits to be lost (as the new commit does + not contain the old commit). + --max-pack-size=:: Maximum size of each output packfile, expressed in MiB. The default is 4096 (4 GiB) as that is the maximum allowed @@ -92,11 +97,18 @@ run alongside parallel `git repack -a -d` or `git gc` invocations, or any other Git operation (including `git prune`, as loose objects are never used by gfi). -However, gfi does not lock the branch or tag refs it is actively -importing. After EOF, during its ref update phase, gfi blindly -overwrites each imported branch or tag ref. Consequently it is not -safe to modify refs that are currently being used by a running gfi -instance, as work could be lost when gfi overwrites the refs. +gfi does not lock the branch or tag refs it is actively importing. +After the import, during its ref update phase, gfi tests each +existing branch ref to verify the update will be a fast-forward +update (the commit stored in the ref is contained in the new +history of the commit to be written). If the update is not a +fast-forward update, gfi will skip updating that ref and instead +prints a warning message. gfi will always attempt to update all +branch refs, and does not stop on the first failure. + +Branch updates can be forced with `--force`, but its recommended that +this only be used on an otherwise quiet repository. Using `--force` +is not necessary for an initial import into an empty repository. Technical Discussion diff --git a/fast-import.c b/fast-import.c index ee4777fda..df84e4d87 100644 --- a/fast-import.c +++ b/fast-import.c @@ -121,6 +121,7 @@ Format of STDIN stream: #include "object.h" #include "blob.h" #include "tree.h" +#include "commit.h" #include "delta.h" #include "pack.h" #include "refs.h" @@ -247,6 +248,7 @@ typedef enum { /* Configured limits on output */ static unsigned long max_depth = 10; static unsigned long max_packsize = (1LL << 32) - 1; +static int force_update; /* Stats and misc. counters */ static uintmax_t alloc_count; @@ -257,6 +259,7 @@ static uintmax_t delta_count_by_type[1 << TYPE_BITS]; static unsigned long object_count; static unsigned long branch_count; static unsigned long branch_load_count; +static int failure; /* Memory pools */ static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool); @@ -1278,19 +1281,48 @@ del_entry: return 1; } -static void dump_branches(void) +static int update_branch(struct branch *b) { static const char *msg = "fast-import"; + struct ref_lock *lock; + unsigned char old_sha1[20]; + + if (read_ref(b->name, old_sha1)) + hashclr(old_sha1); + lock = lock_any_ref_for_update(b->name, old_sha1); + if (!lock) + return error("Unable to lock %s", b->name); + if (!force_update && !is_null_sha1(old_sha1)) { + struct commit *old_cmit, *new_cmit; + + old_cmit = lookup_commit_reference_gently(old_sha1, 0); + new_cmit = lookup_commit_reference_gently(b->sha1, 0); + if (!old_cmit || !new_cmit) { + unlock_ref(lock); + return error("Branch %s is missing commits.", b->name); + } + + if (!in_merge_bases(old_cmit, new_cmit)) { + unlock_ref(lock); + warn("Not updating %s" + " (new tip %s does not contain %s)", + b->name, sha1_to_hex(b->sha1), sha1_to_hex(old_sha1)); + return -1; + } + } + if (write_ref_sha1(lock, b->sha1, msg) < 0) + return error("Unable to update %s", b->name); + return 0; +} + +static void dump_branches(void) +{ unsigned int i; struct branch *b; - struct ref_lock *lock; for (i = 0; i < branch_table_sz; i++) { - for (b = branch_table[i]; b; b = b->table_next_branch) { - lock = lock_any_ref_for_update(b->name, NULL); - if (!lock || write_ref_sha1(lock, b->sha1, msg) < 0) - die("Can't write %s", b->name); - } + for (b = branch_table[i]; b; b = b->table_next_branch) + failure |= update_branch(b); } } @@ -1299,13 +1331,13 @@ static void dump_tags(void) static const char *msg = "fast-import"; struct tag *t; struct ref_lock *lock; - char path[PATH_MAX]; + char ref_name[PATH_MAX]; for (t = first_tag; t; t = t->next_tag) { - sprintf(path, "refs/tags/%s", t->name); - lock = lock_any_ref_for_update(path, NULL); + sprintf(ref_name, "tags/%s", t->name); + lock = lock_ref_sha1(ref_name, NULL); if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0) - die("Can't write %s", path); + failure |= error("Unable to update %s", ref_name); } } @@ -1936,6 +1968,8 @@ int main(int argc, const char **argv) max_active_branches = strtoul(a + 18, NULL, 0); else if (!strncmp(a, "--export-marks=", 15)) mark_file = a + 15; + else if (!strcmp(a, "--force")) + force_update = 1; else die("unknown option %s", a); } @@ -2001,5 +2035,5 @@ int main(int argc, const char **argv) fprintf(stderr, "---------------------------------------------------------------------\n"); fprintf(stderr, "\n"); - return 0; + return failure ? 1 : 0; } diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 84b3c12a5..23a2ba78f 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -276,4 +276,84 @@ test_expect_success \ 'git-cat-file commit branch | sed 1,2d >actual && diff -u expect actual' +### +### series F +### + +old_branch=`git-rev-parse --verify branch^0` +test_tick +cat >input < $GIT_COMMITTER_DATE +data <expect < $GIT_COMMITTER_DATE +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + +losing things already? +EOF +test_expect_success \ + 'F: verify other commit' \ + 'git-cat-file commit other >actual && + diff -u expect actual' + +### +### series G +### + +old_branch=`git-rev-parse --verify branch^0` +test_tick +cat >input < $GIT_COMMITTER_DATE +data <