From 3c96aa97232367a24eef8b3b387f5ddae5b0c10f Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 12 Sep 2017 10:30:27 -0700 Subject: [PATCH] push, fetch: error out for submodule entries not pointing to commits The check_has_commit helper uses resolves a submodule entry to a commit, when validating its existence. As a side effect this means tolerates a submodule entry pointing to a tag, which is not a valid submodule entry that git commands would know how to cope with. Tighten the check to require an actual commit, not a tag pointing to a commit. Also improve the error handling when a submodule entry points to non-commit (e.g., a blob) to error out instead of warning and pretending the pointed to object doesn't exist. Signed-off-by: Stefan Beller Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- submodule.c | 33 +++++++++++++++++++++++++-------- t/t5531-deep-submodule-push.sh | 10 ++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/submodule.c b/submodule.c index 3cea8221e0..e0da55920d 100644 --- a/submodule.c +++ b/submodule.c @@ -767,19 +767,36 @@ static int append_oid_to_argv(const struct object_id *oid, void *data) return 0; } +struct has_commit_data { + int result; + const char *path; +}; + static int check_has_commit(const struct object_id *oid, void *data) { - int *has_commit = data; + struct has_commit_data *cb = data; - if (!lookup_commit_reference(oid)) - *has_commit = 0; + enum object_type type = sha1_object_info(oid->hash, NULL); - return 0; + switch (type) { + case OBJ_COMMIT: + return 0; + case OBJ_BAD: + /* + * Object is missing or invalid. If invalid, an error message + * has already been printed. + */ + cb->result = 0; + return 0; + default: + die(_("submodule entry '%s' (%s) is a %s, not a commit"), + cb->path, oid_to_hex(oid), typename(type)); + } } static int submodule_has_commits(const char *path, struct oid_array *commits) { - int has_commit = 1; + struct has_commit_data has_commit = { 1, path }; /* * Perform a cheap, but incorrect check for the existence of 'commits'. @@ -795,7 +812,7 @@ static int submodule_has_commits(const char *path, struct oid_array *commits) oid_array_for_each_unique(commits, check_has_commit, &has_commit); - if (has_commit) { + if (has_commit.result) { /* * Even if the submodule is checked out and the commit is * present, make sure it exists in the submodule's object store @@ -814,12 +831,12 @@ static int submodule_has_commits(const char *path, struct oid_array *commits) cp.dir = path; if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1) || out.len) - has_commit = 0; + has_commit.result = 0; strbuf_release(&out); } - return has_commit; + return has_commit.result; } static int submodule_needs_pushing(const char *path, struct oid_array *commits) diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index 0f84a53146..39cb2c1c34 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -298,6 +298,16 @@ test_expect_success 'push succeeds if submodule commit disabling recursion from ) ' +test_expect_success 'submodule entry pointing at a tag is error' ' + git -C work/gar/bage tag -a test1 -m "tag" && + tag=$(git -C work/gar/bage rev-parse test1^{tag}) && + git -C work update-index --cacheinfo 160000 "$tag" gar/bage && + git -C work commit -m "bad commit" && + test_when_finished "git -C work reset --hard HEAD^" && + test_must_fail git -C work push --recurse-submodules=on-demand ../pub.git master 2>err && + test_i18ngrep "is a tag, not a commit" err +' + test_expect_success 'push fails if recurse submodules option passed as yes' ' ( cd work/gar/bage && -- 2.11.4.GIT