From 331e7de9004db5909edd1057db88f63a53dd2d3f Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 24 Oct 2012 17:32:50 -0700 Subject: [PATCH 01/11] Extensions to rmdir and mkdir utilities * Rework GIT_DIRREMOVAL values to GIT_RMDIR flags, allowing combinations of flags * Add GIT_RMDIR_EMPTY_PARENTS flag to remove parent dirs that are left empty after removal * Add GIT_MKDIR_VERIFY_DIR to give an error if item is a file, not a dir (previously an EEXISTS error was ignored, even for files) and enable this flag for git_futils_mkpath2file call * Improve accuracy of error messages from git_futils_mkdir --- src/checkout.c | 2 +- src/clone.c | 2 +- src/fileops.c | 138 ++++++++++++++++++++++++--------- src/fileops.h | 39 ++++++---- src/reflog.c | 2 +- src/refs.c | 13 ++-- tests-clar/core/copy.c | 8 +- tests-clar/core/mkdir.c | 14 ++-- tests-clar/core/rmdir.c | 42 ++++++++-- tests-clar/object/blob/write.c | 4 +- tests-clar/repo/discover.c | 2 +- tests-clar/repo/open.c | 6 +- tests-clar/status/worktree.c | 6 +- tests-clar/submodule/status.c | 8 +- 14 files changed, 195 insertions(+), 91 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index e068e4f5f..b9a5399cc 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -213,7 +213,7 @@ static int checkout_remove_the_old( data->error = git_futils_rmdir_r( delta->new_file.path, git_repository_workdir(data->owner), - GIT_DIRREMOVAL_FILES_AND_DIRS); + GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS); data->completed_steps++; report_progress(data, delta->new_file.path); diff --git a/src/clone.c b/src/clone.c index 7352f5fb2..560d64c0d 100644 --- a/src/clone.c +++ b/src/clone.c @@ -338,7 +338,7 @@ static int clone_internal( fetch_progress_cb, fetch_progress_payload)) < 0) { /* Failed to fetch; clean up */ git_repository_free(repo); - git_futils_rmdir_r(path, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS); + git_futils_rmdir_r(path, NULL, GIT_RMDIR_REMOVE_FILES); } else { *out = repo; retcode = 0; diff --git a/src/fileops.c b/src/fileops.c index 2aceb112a..d2cbd046d 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -14,7 +14,8 @@ int git_futils_mkpath2file(const char *file_path, const mode_t mode) { return git_futils_mkdir( - file_path, NULL, mode, GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST); + file_path, NULL, mode, + GIT_MKDIR_PATH | GIT_MKDIR_SKIP_LAST | GIT_MKDIR_VERIFY_DIR); } int git_futils_mktmp(git_buf *path_out, const char *filename) @@ -250,6 +251,7 @@ int git_futils_mkdir( mode_t mode, uint32_t flags) { + int error = -1; git_buf make_path = GIT_BUF_INIT; ssize_t root = 0; char lastch, *tail; @@ -297,12 +299,28 @@ int git_futils_mkdir( *tail = '\0'; /* make directory */ - if (p_mkdir(make_path.ptr, mode) < 0 && - (errno != EEXIST || (flags & GIT_MKDIR_EXCL) != 0)) - { - giterr_set(GITERR_OS, "Failed to make directory '%s'", - make_path.ptr); - goto fail; + if (p_mkdir(make_path.ptr, mode) < 0) { + if (errno == EEXIST) { + if (!lastch && (flags & GIT_MKDIR_VERIFY_DIR) != 0) { + if (!git_path_isdir(make_path.ptr)) { + giterr_set( + GITERR_OS, "Existing path is not a directory '%s'", + make_path.ptr); + error = GIT_ENOTFOUND; + goto fail; + } + } + if ((flags & GIT_MKDIR_EXCL) != 0) { + giterr_set(GITERR_OS, "Directory already exists '%s'", + make_path.ptr); + error = GIT_EEXISTS; + goto fail; + } + } else { + giterr_set(GITERR_OS, "Failed to make directory '%s'", + make_path.ptr); + goto fail; + } } /* chmod if requested */ @@ -324,7 +342,7 @@ int git_futils_mkdir( fail: git_buf_free(&make_path); - return -1; + return error; } int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode) @@ -332,57 +350,103 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode) return git_futils_mkdir(path, base, mode, GIT_MKDIR_PATH); } -static int _rmdir_recurs_foreach(void *opaque, git_buf *path) +typedef struct { + uint32_t flags; + int error; +} futils__rmdir_data; + +static int futils__error_cannot_rmdir(const char *path, const char *filemsg) { - git_directory_removal_type removal_type = *(git_directory_removal_type *)opaque; + if (filemsg) + giterr_set(GITERR_OS, "Could not remove directory. File '%s' %s", + path, filemsg); + else + giterr_set(GITERR_OS, "Could not remove directory '%s'", path); + + return -1; +} + +static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) +{ + futils__rmdir_data *data = opaque; if (git_path_isdir(path->ptr) == true) { - if (git_path_direach(path, _rmdir_recurs_foreach, opaque) < 0) - return -1; + int error = git_path_direach(path, futils__rmdir_recurs_foreach, data); + if (error < 0) + return (error == GIT_EUSER) ? data->error : error; - if (p_rmdir(path->ptr) < 0) { - if (removal_type == GIT_DIRREMOVAL_ONLY_EMPTY_DIRS && (errno == ENOTEMPTY || errno == EEXIST)) - return 0; + data->error = p_rmdir(path->ptr); - giterr_set(GITERR_OS, "Could not remove directory '%s'", path->ptr); - return -1; + if (data->error < 0) { + if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) != 0 && + (errno == ENOTEMPTY || errno == EEXIST)) + data->error = 0; + else + futils__error_cannot_rmdir(path->ptr, NULL); } - - return 0; } - if (removal_type == GIT_DIRREMOVAL_FILES_AND_DIRS) { - if (p_unlink(path->ptr) < 0) { - giterr_set(GITERR_OS, "Could not remove directory. File '%s' cannot be removed", path->ptr); - return -1; + else if ((data->flags & GIT_RMDIR_REMOVE_FILES) != 0) { + data->error = p_unlink(path->ptr); + + if (data->error < 0) + futils__error_cannot_rmdir(path->ptr, "cannot be removed"); + } + + else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0) { + data->error = futils__error_cannot_rmdir(path->ptr, "still present"); + } + + return data->error; +} + +static int futils__rmdir_empty_parent(void *opaque, git_buf *path) +{ + int error = p_rmdir(path->ptr); + + GIT_UNUSED(opaque); + + if (error) { + int en = errno; + + if (en == ENOENT || en == ENOTDIR) { + giterr_clear(); + error = 0; + } else if (en == ENOTEMPTY || en == EEXIST) { + giterr_clear(); + error = GIT_ITEROVER; + } else { + futils__error_cannot_rmdir(path->ptr, NULL); } - - return 0; } - if (removal_type == GIT_DIRREMOVAL_EMPTY_HIERARCHY) { - giterr_set(GITERR_OS, "Could not remove directory. File '%s' still present", path->ptr); - return -1; - } - - return 0; + return error; } int git_futils_rmdir_r( - const char *path, const char *base, git_directory_removal_type removal_type) + const char *path, const char *base, uint32_t flags) { int error; git_buf fullpath = GIT_BUF_INIT; - - assert(removal_type == GIT_DIRREMOVAL_EMPTY_HIERARCHY - || removal_type == GIT_DIRREMOVAL_FILES_AND_DIRS - || removal_type == GIT_DIRREMOVAL_ONLY_EMPTY_DIRS); + futils__rmdir_data data; /* build path and find "root" where we should start calling mkdir */ if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0) return -1; - error = _rmdir_recurs_foreach(&removal_type, &fullpath); + data.flags = flags; + data.error = 0; + + error = futils__rmdir_recurs_foreach(&data, &fullpath); + + /* remove now-empty parents if requested */ + if (!error && (flags & GIT_RMDIR_EMPTY_PARENTS) != 0) { + error = git_path_walk_up( + &fullpath, base, futils__rmdir_empty_parent, &data); + + if (error == GIT_ITEROVER) + error = 0; + } git_buf_free(&fullpath); diff --git a/src/fileops.h b/src/fileops.h index 25e62c504..6952c463c 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -65,6 +65,7 @@ extern int git_futils_mkdir_r(const char *path, const char *base, const mode_t m * * GIT_MKDIR_CHMOD says to chmod the final directory entry after creation * * GIT_MKDIR_CHMOD_PATH says to chmod each directory component in the path * * GIT_MKDIR_SKIP_LAST says to leave off the last element of the path + * * GIT_MKDIR_VERIFY_DIR says confirm final item is a dir, not just EEXIST * * Note that the chmod options will be executed even if the directory already * exists, unless GIT_MKDIR_EXCL is given. @@ -74,7 +75,8 @@ typedef enum { GIT_MKDIR_PATH = 2, GIT_MKDIR_CHMOD = 4, GIT_MKDIR_CHMOD_PATH = 8, - GIT_MKDIR_SKIP_LAST = 16 + GIT_MKDIR_SKIP_LAST = 16, + GIT_MKDIR_VERIFY_DIR = 32, } git_futils_mkdir_flags; /** @@ -98,27 +100,38 @@ extern int git_futils_mkdir(const char *path, const char *base, mode_t mode, uin */ extern int git_futils_mkpath2file(const char *path, const mode_t mode); +/** + * Flags to pass to `git_futils_rmdir_r`. + * + * * GIT_RMDIR_EMPTY_HIERARCHY - the default; remove hierarchy of empty + * dirs and generate error if any files are found. + * * GIT_RMDIR_REMOVE_FILES - attempt to remove files in the hierarchy. + * * GIT_RMDIR_SKIP_NONEMPTY - skip non-empty directories with no error. + * * GIT_RMDIR_EMPTY_PARENTS - remove containing directories up to base + * if removing this item leaves them empty + * + * The old values translate into the new as follows: + * + * * GIT_DIRREMOVAL_EMPTY_HIERARCHY == GIT_RMDIR_EMPTY_HIERARCHY + * * GIT_DIRREMOVAL_FILES_AND_DIRS ~= GIT_RMDIR_REMOVE_FILES + * * GIT_DIRREMOVAL_ONLY_EMPTY_DIRS == GIT_RMDIR_SKIP_NONEMPTY + */ typedef enum { - GIT_DIRREMOVAL_EMPTY_HIERARCHY = 0, - GIT_DIRREMOVAL_FILES_AND_DIRS = 1, - GIT_DIRREMOVAL_ONLY_EMPTY_DIRS = 2, -} git_directory_removal_type; + GIT_RMDIR_EMPTY_HIERARCHY = 0, + GIT_RMDIR_REMOVE_FILES = (1 << 0), + GIT_RMDIR_SKIP_NONEMPTY = (1 << 1), + GIT_RMDIR_EMPTY_PARENTS = (1 << 2), +} git_futils_rmdir_flags; /** * Remove path and any files and directories beneath it. * * @param path Path to to top level directory to process. * @param base Root for relative path. - * @param removal_type GIT_DIRREMOVAL_EMPTY_HIERARCHY to remove a hierarchy - * of empty directories (will fail if any file is found), - * GIT_DIRREMOVAL_FILES_AND_DIRS to remove a hierarchy of - * files and folders, - * GIT_DIRREMOVAL_ONLY_EMPTY_DIRS to only remove empty - * directories (no failure on file encounter). - * + * @param flags Combination of git_futils_rmdir_flags values * @return 0 on success; -1 on error. */ -extern int git_futils_rmdir_r(const char *path, const char *base, git_directory_removal_type removal_type); +extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags); /** * Create and open a temporary file with a `_git2_` suffix. diff --git a/src/reflog.c b/src/reflog.c index 5d1465eca..0e333aa6f 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -378,7 +378,7 @@ int git_reflog_rename(git_reference *ref, const char *new_name) goto cleanup; if (git_path_isdir(git_buf_cstr(&new_path)) && - (git_futils_rmdir_r(git_buf_cstr(&new_path), NULL, GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0)) + (git_futils_rmdir_r(git_buf_cstr(&new_path), NULL, GIT_RMDIR_SKIP_NONEMPTY) < 0)) goto cleanup; if (git_futils_mkpath2file(git_buf_cstr(&new_path), GIT_REFLOG_DIR_MODE) < 0) diff --git a/src/refs.c b/src/refs.c index bbf30ed9e..97c97563e 100644 --- a/src/refs.c +++ b/src/refs.c @@ -274,18 +274,15 @@ static int loose_write(git_reference *ref) git_buf ref_path = GIT_BUF_INIT; struct stat st; - if (git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name) < 0) - return -1; - /* Remove a possibly existing empty directory hierarchy * which name would collide with the reference name */ - if (git_path_isdir(git_buf_cstr(&ref_path)) && - git_futils_rmdir_r(git_buf_cstr(&ref_path), NULL, - GIT_DIRREMOVAL_ONLY_EMPTY_DIRS) < 0) { - git_buf_free(&ref_path); + if (git_futils_rmdir_r(ref->name, ref->owner->path_repository, + GIT_RMDIR_SKIP_NONEMPTY) < 0) + return -1; + + if (git_buf_joinpath(&ref_path, ref->owner->path_repository, ref->name) < 0) return -1; - } if (git_filebuf_open(&file, ref_path.ptr, GIT_FILEBUF_FORCE) < 0) { git_buf_free(&ref_path); diff --git a/tests-clar/core/copy.c b/tests-clar/core/copy.c index d0b21f6ec..c0c59c056 100644 --- a/tests-clar/core/copy.c +++ b/tests-clar/core/copy.c @@ -41,7 +41,7 @@ void test_core_copy__file_in_dir(void) cl_assert(S_ISREG(st.st_mode)); cl_assert(strlen(content) == (size_t)st.st_size); - cl_git_pass(git_futils_rmdir_r("an_dir", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("an_dir", NULL, GIT_RMDIR_REMOVE_FILES)); cl_assert(!git_path_isdir("an_dir")); } @@ -95,7 +95,7 @@ void test_core_copy__tree(void) cl_assert(S_ISLNK(st.st_mode)); #endif - cl_git_pass(git_futils_rmdir_r("t1", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("t1", NULL, GIT_RMDIR_REMOVE_FILES)); cl_assert(!git_path_isdir("t1")); /* copy with empty dirs, no links, yes dotfiles, no overwrite */ @@ -119,8 +119,8 @@ void test_core_copy__tree(void) cl_git_fail(git_path_lstat("t2/c/d/l1", &st)); #endif - cl_git_pass(git_futils_rmdir_r("t2", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("t2", NULL, GIT_RMDIR_REMOVE_FILES)); cl_assert(!git_path_isdir("t2")); - cl_git_pass(git_futils_rmdir_r("src", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("src", NULL, GIT_RMDIR_REMOVE_FILES)); } diff --git a/tests-clar/core/mkdir.c b/tests-clar/core/mkdir.c index e5dc6654b..1e50b4336 100644 --- a/tests-clar/core/mkdir.c +++ b/tests-clar/core/mkdir.c @@ -6,11 +6,11 @@ static void cleanup_basic_dirs(void *ref) { GIT_UNUSED(ref); - git_futils_rmdir_r("d0", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); - git_futils_rmdir_r("d1", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); - git_futils_rmdir_r("d2", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); - git_futils_rmdir_r("d3", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); - git_futils_rmdir_r("d4", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); + git_futils_rmdir_r("d0", NULL, GIT_RMDIR_EMPTY_HIERARCHY); + git_futils_rmdir_r("d1", NULL, GIT_RMDIR_EMPTY_HIERARCHY); + git_futils_rmdir_r("d2", NULL, GIT_RMDIR_EMPTY_HIERARCHY); + git_futils_rmdir_r("d3", NULL, GIT_RMDIR_EMPTY_HIERARCHY); + git_futils_rmdir_r("d4", NULL, GIT_RMDIR_EMPTY_HIERARCHY); } void test_core_mkdir__basic(void) @@ -56,7 +56,7 @@ void test_core_mkdir__basic(void) static void cleanup_basedir(void *ref) { GIT_UNUSED(ref); - git_futils_rmdir_r("base", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); + git_futils_rmdir_r("base", NULL, GIT_RMDIR_EMPTY_HIERARCHY); } void test_core_mkdir__with_base(void) @@ -108,7 +108,7 @@ static void cleanup_chmod_root(void *ref) git__free(mode); } - git_futils_rmdir_r("r", NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY); + git_futils_rmdir_r("r", NULL, GIT_RMDIR_EMPTY_HIERARCHY); } static void check_mode(mode_t expected, mode_t actual) diff --git a/tests-clar/core/rmdir.c b/tests-clar/core/rmdir.c index 9ada8f426..f0b0bfa42 100644 --- a/tests-clar/core/rmdir.c +++ b/tests-clar/core/rmdir.c @@ -30,7 +30,7 @@ void test_core_rmdir__initialize(void) /* make sure empty dir can be deleted recusively */ void test_core_rmdir__delete_recursive(void) { - cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY)); + cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_EMPTY_HIERARCHY)); } /* make sure non-empty dir cannot be deleted recusively */ @@ -42,15 +42,15 @@ void test_core_rmdir__fail_to_delete_non_empty_dir(void) cl_git_mkfile(git_buf_cstr(&file), "dummy"); - cl_git_fail(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY)); + cl_git_fail(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_EMPTY_HIERARCHY)); cl_must_pass(p_unlink(file.ptr)); - cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_DIRREMOVAL_EMPTY_HIERARCHY)); + cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_EMPTY_HIERARCHY)); git_buf_free(&file); } -void test_core_rmdir__can_skip__non_empty_dir(void) +void test_core_rmdir__can_skip_non_empty_dir(void) { git_buf file = GIT_BUF_INIT; @@ -58,11 +58,41 @@ void test_core_rmdir__can_skip__non_empty_dir(void) cl_git_mkfile(git_buf_cstr(&file), "dummy"); - cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_DIRREMOVAL_ONLY_EMPTY_DIRS)); + cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_SKIP_NONEMPTY)); cl_assert(git_path_exists(git_buf_cstr(&file)) == true); - cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_REMOVE_FILES)); cl_assert(git_path_exists(empty_tmp_dir) == false); git_buf_free(&file); } + +void test_core_rmdir__can_remove_empty_parents(void) +{ + git_buf file = GIT_BUF_INIT; + + cl_git_pass( + git_buf_joinpath(&file, empty_tmp_dir, "/one/two_two/three/file.txt")); + cl_git_mkfile(git_buf_cstr(&file), "dummy"); + cl_assert(git_path_isfile(git_buf_cstr(&file))); + + cl_git_pass(git_futils_rmdir_r("one/two_two/three/file.txt", empty_tmp_dir, + GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS)); + + cl_assert(!git_path_exists(git_buf_cstr(&file))); + + git_buf_rtruncate_at_char(&file, '/'); /* three (only contained file.txt) */ + cl_assert(!git_path_exists(git_buf_cstr(&file))); + + git_buf_rtruncate_at_char(&file, '/'); /* two_two (only contained three) */ + cl_assert(!git_path_exists(git_buf_cstr(&file))); + + git_buf_rtruncate_at_char(&file, '/'); /* one (contained two_one also) */ + cl_assert(git_path_exists(git_buf_cstr(&file))); + + cl_assert(git_path_exists(empty_tmp_dir) == true); + + git_buf_free(&file); + + cl_git_pass(git_futils_rmdir_r(empty_tmp_dir, NULL, GIT_RMDIR_EMPTY_HIERARCHY)); +} diff --git a/tests-clar/object/blob/write.c b/tests-clar/object/blob/write.c index 87a9e2072..6d4cbab4f 100644 --- a/tests-clar/object/blob/write.c +++ b/tests-clar/object/blob/write.c @@ -49,7 +49,7 @@ void test_object_blob_write__can_create_a_blob_in_a_standard_repo_from_a_absolut assert_blob_creation(ELSEWHERE "/test.txt", git_buf_cstr(&full_path), &git_blob_create_fromdisk); git_buf_free(&full_path); - cl_must_pass(git_futils_rmdir_r(ELSEWHERE, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_must_pass(git_futils_rmdir_r(ELSEWHERE, NULL, GIT_RMDIR_REMOVE_FILES)); } void test_object_blob_write__can_create_a_blob_in_a_bare_repo_from_a_absolute_filepath(void) @@ -65,5 +65,5 @@ void test_object_blob_write__can_create_a_blob_in_a_bare_repo_from_a_absolute_fi assert_blob_creation(ELSEWHERE "/test.txt", git_buf_cstr(&full_path), &git_blob_create_fromdisk); git_buf_free(&full_path); - cl_must_pass(git_futils_rmdir_r(ELSEWHERE, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_must_pass(git_futils_rmdir_r(ELSEWHERE, NULL, GIT_RMDIR_REMOVE_FILES)); } diff --git a/tests-clar/repo/discover.c b/tests-clar/repo/discover.c index b5afab75a..3d9aeedd7 100644 --- a/tests-clar/repo/discover.c +++ b/tests-clar/repo/discover.c @@ -135,7 +135,7 @@ void test_repo_discover__0(void) ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path); ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, repository_path); - cl_git_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, NULL, GIT_RMDIR_REMOVE_FILES)); git_repository_free(repo); git_buf_free(&ceiling_dirs_buf); } diff --git a/tests-clar/repo/open.c b/tests-clar/repo/open.c index ef912fa0e..7f93ae91a 100644 --- a/tests-clar/repo/open.c +++ b/tests-clar/repo/open.c @@ -7,7 +7,7 @@ void test_repo_open__cleanup(void) cl_git_sandbox_cleanup(); if (git_path_isdir("alternate")) - git_futils_rmdir_r("alternate", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS); + git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES); } void test_repo_open__bare_empty_repo(void) @@ -202,8 +202,8 @@ void test_repo_open__bad_gitlinks(void) cl_git_fail(git_repository_open_ext(&repo, "alternate", 0, NULL)); } - git_futils_rmdir_r("invalid", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS); - git_futils_rmdir_r("invalid2", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS); + git_futils_rmdir_r("invalid", NULL, GIT_RMDIR_REMOVE_FILES); + git_futils_rmdir_r("invalid2", NULL, GIT_RMDIR_REMOVE_FILES); } #ifdef GIT_WIN32 diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c index 116286f67..c154179b0 100644 --- a/tests-clar/status/worktree.c +++ b/tests-clar/status/worktree.c @@ -71,7 +71,7 @@ static int remove_file_cb(void *data, git_buf *file) return 0; if (git_path_isdir(filename)) - cl_git_pass(git_futils_rmdir_r(filename, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(filename, NULL, GIT_RMDIR_REMOVE_FILES)); else cl_git_pass(p_unlink(git_buf_cstr(file))); @@ -314,7 +314,7 @@ void test_status_worktree__issue_592_3(void) repo = cl_git_sandbox_init("issue_592"); cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(repo), "c")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_pass(git_status_foreach(repo, cb_status__check_592, "c/a.txt")); @@ -344,7 +344,7 @@ void test_status_worktree__issue_592_5(void) repo = cl_git_sandbox_init("issue_592"); cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(repo), "t")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_pass(p_mkdir(git_buf_cstr(&path), 0777)); cl_git_pass(git_status_foreach(repo, cb_status__check_592, NULL)); diff --git a/tests-clar/submodule/status.c b/tests-clar/submodule/status.c index eec028c40..325013466 100644 --- a/tests-clar/submodule/status.c +++ b/tests-clar/submodule/status.c @@ -50,7 +50,7 @@ void test_submodule_status__ignore_none(void) git_buf path = GIT_BUF_INIT; cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "sm_unchanged")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_fail(git_submodule_lookup(&sm, g_repo, "not_submodule")); @@ -135,7 +135,7 @@ void test_submodule_status__ignore_untracked(void) git_submodule_ignore_t ign = GIT_SUBMODULE_IGNORE_UNTRACKED; cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "sm_unchanged")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign)); @@ -195,7 +195,7 @@ void test_submodule_status__ignore_dirty(void) git_submodule_ignore_t ign = GIT_SUBMODULE_IGNORE_DIRTY; cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "sm_unchanged")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign)); @@ -255,7 +255,7 @@ void test_submodule_status__ignore_all(void) git_submodule_ignore_t ign = GIT_SUBMODULE_IGNORE_ALL; cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "sm_unchanged")); - cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(git_buf_cstr(&path), NULL, GIT_RMDIR_REMOVE_FILES)); cl_git_pass(git_submodule_foreach(g_repo, set_sm_ignore, &ign)); From 32def5af9a951396e626c3e276a0f94683753e3d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 24 Oct 2012 17:37:07 -0700 Subject: [PATCH 02/11] Fix checkout behavior when its hands are tied So, @nulltoken created a failing test case for checkout that proved to be particularly daunting. If checkout is given only a very limited strategy mask (e.g. just GIT_CHECKOUT_CREATE_MISSING) then it is possible for typechange/rename modifications to leave it unable to complete the request. That's okay, but the existing code did not have enough information not to generate an error (at least for tree/blob conflicts). This led me to a significant reorganization of the code to handle the failing case, but it has three benefits: 1. The test case is handled correctly (I think) 2. The new code should actually be much faster than the old code since I decided to make checkout aware of diff list internals. 3. The progress value accuracy is hugely increased since I added a fourth pass which calculates exactly what work needs to be done before doing anything. --- src/checkout.c | 351 ++++++++++++++++++++++++------------ tests-clar/checkout/index.c | 33 +++- 2 files changed, 263 insertions(+), 121 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index b9a5399cc..e4a397cd5 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -21,6 +21,7 @@ #include "repository.h" #include "filter.h" #include "blob.h" +#include "diff.h" struct checkout_diff_data { @@ -43,20 +44,23 @@ static int buffer_to_file( int file_open_flags, mode_t file_mode) { - int fd, error, error_close; + int fd, error; if ((error = git_futils_mkpath2file(path, dir_mode)) < 0) return error; - if ((fd = p_open(path, file_open_flags, file_mode)) < 0) + if ((fd = p_open(path, file_open_flags, file_mode)) < 0) { + giterr_set(GITERR_OS, "Could not open '%s' for writing", path); return fd; + } - error = p_write(fd, git_buf_cstr(buffer), git_buf_len(buffer)); - - error_close = p_close(fd); - - if (!error) - error = error_close; + if ((error = p_write(fd, git_buf_cstr(buffer), git_buf_len(buffer))) < 0) { + giterr_set(GITERR_OS, "Could not write to '%s'", path); + (void)p_close(fd); + } else { + if ((error = p_close(fd)) < 0) + giterr_set(GITERR_OS, "Error while closing '%s'", path); + } if (!error && (file_mode & 0100) != 0 && @@ -160,8 +164,8 @@ static int checkout_submodule( } static void report_progress( - struct checkout_diff_data *data, - const char *path) + struct checkout_diff_data *data, + const char *path) { if (data->checkout_opts->progress_cb) data->checkout_opts->progress_cb( @@ -176,12 +180,19 @@ static int checkout_blob( const git_diff_file *file) { git_blob *blob; - int error; + int error = 0; git_buf_truncate(data->path, data->workdir_len); if (git_buf_joinpath(data->path, git_buf_cstr(data->path), file->path) < 0) return -1; + /* If there is a directory where this blob should go, then there is an + * existing tree that conflicts with this file, but REMOVE_UNTRACKED must + * not have been passed in. Signal to caller with GIT_ENOTFOUND. + */ + if (git_path_isdir(git_buf_cstr(data->path))) + return GIT_ENOTFOUND; + if ((error = git_blob_lookup(&blob, data->owner, &file->oid)) < 0) return error; @@ -197,90 +208,6 @@ static int checkout_blob( return error; } -static int checkout_remove_the_old( - void *cb_data, const git_diff_delta *delta, float progress) -{ - struct checkout_diff_data *data = cb_data; - git_checkout_opts *opts = data->checkout_opts; - - GIT_UNUSED(progress); - - if ((delta->status == GIT_DELTA_UNTRACKED && - (opts->checkout_strategy & GIT_CHECKOUT_REMOVE_UNTRACKED) != 0) || - (delta->status == GIT_DELTA_TYPECHANGE && - (opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0)) - { - data->error = git_futils_rmdir_r( - delta->new_file.path, - git_repository_workdir(data->owner), - GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS); - - data->completed_steps++; - report_progress(data, delta->new_file.path); - } - - return data->error; -} - -static int checkout_create_the_new( - void *cb_data, const git_diff_delta *delta, float progress) -{ - int error = 0; - struct checkout_diff_data *data = cb_data; - git_checkout_opts *opts = data->checkout_opts; - bool do_checkout = false, do_notify = false; - - GIT_UNUSED(progress); - - if (delta->status == GIT_DELTA_MODIFIED || - delta->status == GIT_DELTA_TYPECHANGE) - { - if ((opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0) - do_checkout = true; - else if (opts->skipped_notify_cb != NULL) - do_notify = !data->create_submodules; - } - else if (delta->status == GIT_DELTA_DELETED && - (opts->checkout_strategy & GIT_CHECKOUT_CREATE_MISSING) != 0) - do_checkout = true; - - if (do_notify) { - if (opts->skipped_notify_cb( - delta->old_file.path, &delta->old_file.oid, - delta->old_file.mode, opts->notify_payload)) - { - giterr_clear(); - error = GIT_EUSER; - } - } - - if (do_checkout) { - bool is_submodule = S_ISGITLINK(delta->old_file.mode); - - if (is_submodule) { - data->found_submodules = true; - } - - if (!is_submodule && !data->create_submodules) { - error = checkout_blob(data, &delta->old_file); - data->completed_steps++; - report_progress(data, delta->old_file.path); - } - - else if (is_submodule && data->create_submodules) { - error = checkout_submodule(data, &delta->old_file); - data->completed_steps++; - report_progress(data, delta->old_file.path); - } - - } - - if (error) - data->error = error; - - return error; -} - static int retrieve_symlink_capabilities(git_repository *repo, bool *can_symlink) { git_config *cfg; @@ -324,6 +251,177 @@ static void normalize_options(git_checkout_opts *normalized, git_checkout_opts * normalized->file_open_flags = O_CREAT | O_TRUNC | O_WRONLY; } +enum { + CHECKOUT_ACTION__NONE = 0, + CHECKOUT_ACTION__REMOVE = 1, + CHECKOUT_ACTION__CREATE_BLOB = 2, + CHECKOUT_ACTION__CREATE_SUBMODULE = 4, + CHECKOUT_ACTION__NOTIFY = 8 +}; + +static uint32_t checkout_action_for_delta( + git_checkout_opts *opts, + const git_diff_delta *delta) +{ + uint32_t action = CHECKOUT_ACTION__NONE; + + switch (delta->status) { + case GIT_DELTA_UNTRACKED: + if ((opts->checkout_strategy & GIT_CHECKOUT_REMOVE_UNTRACKED) != 0) + action |= CHECKOUT_ACTION__REMOVE; + break; + + case GIT_DELTA_TYPECHANGE: + if ((opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0) + action |= CHECKOUT_ACTION__REMOVE | CHECKOUT_ACTION__CREATE_BLOB; + else if (opts->skipped_notify_cb != NULL) + action = CHECKOUT_ACTION__NOTIFY; + break; + + case GIT_DELTA_DELETED: + if ((opts->checkout_strategy & GIT_CHECKOUT_CREATE_MISSING) != 0) + action |= CHECKOUT_ACTION__CREATE_BLOB; + break; + + case GIT_DELTA_MODIFIED: + if ((opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0) + action |= CHECKOUT_ACTION__CREATE_BLOB; + else if (opts->skipped_notify_cb != NULL) + action = CHECKOUT_ACTION__NOTIFY; + break; + + default: + break; + } + + if ((action & CHECKOUT_ACTION__CREATE_BLOB) != 0 && + S_ISGITLINK(delta->old_file.mode)) + action = (action & ~CHECKOUT_ACTION__CREATE_BLOB) | + CHECKOUT_ACTION__CREATE_SUBMODULE; + + return action; +} + +static int checkout_get_actions( + uint32_t **actions_ptr, + size_t **counts_ptr, + git_diff_list *diff, + git_checkout_opts *opts) +{ + git_diff_delta *delta; + size_t i, *counts; + uint32_t *actions; + + *counts_ptr = counts = + git__calloc(CHECKOUT_ACTION__NOTIFY, sizeof(size_t)); + GITERR_CHECK_ALLOC(counts); + + *actions_ptr = actions = + git__calloc(diff->deltas.length, sizeof(uint32_t)); + GITERR_CHECK_ALLOC(actions); + + git_vector_foreach(&diff->deltas, i, delta) { + actions[i] = checkout_action_for_delta(opts, delta); + + if (actions[i] & CHECKOUT_ACTION__REMOVE) + counts[CHECKOUT_ACTION__REMOVE]++; + + if (actions[i] & CHECKOUT_ACTION__CREATE_BLOB) + counts[CHECKOUT_ACTION__CREATE_BLOB]++; + + if (actions[i] & CHECKOUT_ACTION__CREATE_SUBMODULE) + counts[CHECKOUT_ACTION__CREATE_SUBMODULE]++; + + if (actions[i] & CHECKOUT_ACTION__NOTIFY) + counts[CHECKOUT_ACTION__NOTIFY]++; + } + + return 0; +} + +static int checkout_remove_the_old( + git_diff_list *diff, + unsigned int *actions, + struct checkout_diff_data *data) +{ + git_diff_delta *delta; + size_t i; + + git_vector_foreach(&diff->deltas, i, delta) { + if (actions[i] & CHECKOUT_ACTION__REMOVE) { + int error = git_futils_rmdir_r( + delta->new_file.path, git_buf_cstr(data->path), + GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS); + if (error < 0) + return error; + + data->completed_steps++; + report_progress(data, delta->new_file.path); + } + } + + return 0; +} + +static int checkout_create_the_new( + git_diff_list *diff, + unsigned int *actions, + struct checkout_diff_data *data) +{ + git_diff_delta *delta; + size_t i; + + git_vector_foreach(&diff->deltas, i, delta) { + if (actions[i] & CHECKOUT_ACTION__CREATE_BLOB) { + int error = checkout_blob(data, &delta->old_file); + + /* ENOTFOUND means unable to create the file because of + * an existing parent dir. Probably flags were given + * asking to create new blobs without allowing removal + * of a conflicting tree (or vice versa). + */ + if (error == GIT_ENOTFOUND) + giterr_clear(); + else if (error < 0) + return error; + + data->completed_steps++; + report_progress(data, delta->old_file.path); + } + + if (actions[i] & CHECKOUT_ACTION__NOTIFY) { + if (data->checkout_opts->skipped_notify_cb( + delta->old_file.path, &delta->old_file.oid, + delta->old_file.mode, data->checkout_opts->notify_payload)) + return GIT_EUSER; + } + } + + return 0; +} + +static int checkout_create_submodules( + git_diff_list *diff, + unsigned int *actions, + struct checkout_diff_data *data) +{ + git_diff_delta *delta; + size_t i; + + git_vector_foreach(&diff->deltas, i, delta) { + if (actions[i] & CHECKOUT_ACTION__CREATE_SUBMODULE) { + int error = checkout_submodule(data, &delta->old_file); + if (error < 0) + return error; + + data->completed_steps++; + report_progress(data, delta->old_file.path); + } + } + + return 0; +} + int git_checkout_index( git_repository *repo, git_checkout_opts *opts) @@ -336,6 +434,9 @@ int git_checkout_index( struct checkout_diff_data data; git_buf workdir = GIT_BUF_INIT; + uint32_t *actions = NULL; + size_t *counts = NULL; + int error; assert(repo); @@ -359,44 +460,54 @@ int git_checkout_index( normalize_options(&checkout_opts, opts); - memset(&data, 0, sizeof(data)); - - data.path = &workdir; - data.workdir_len = git_buf_len(&workdir); - data.checkout_opts = &checkout_opts; - data.owner = repo; - data.total_steps = (size_t)git_diff_num_deltas(diff); - - if ((error = retrieve_symlink_capabilities(repo, &data.can_symlink)) < 0) - goto cleanup; - - /* Checkout is best performed with three passes through the diff. + /* Checkout is best performed with up to four passes through the diff. * - * 1. First do removes, because we iterate in alphabetical order, thus + * 0. Figure out what actions should be taken and record for later. + * 1. Next do removes, because we iterate in alphabetical order, thus * a new untracked directory will end up sorted *after* a blob that * should be checked out with the same name. * 2. Then checkout all blobs. * 3. Then checkout all submodules in case a new .gitmodules blob was * checked out during pass #2. */ + if ((error = checkout_get_actions(&actions, &counts, diff, &checkout_opts)) < 0) + goto cleanup; + + memset(&data, 0, sizeof(data)); + data.path = &workdir; + data.workdir_len = git_buf_len(&workdir); + data.checkout_opts = &checkout_opts; + data.owner = repo; + data.total_steps = counts[CHECKOUT_ACTION__REMOVE] + + counts[CHECKOUT_ACTION__CREATE_BLOB] + + counts[CHECKOUT_ACTION__CREATE_SUBMODULE]; + + if ((error = retrieve_symlink_capabilities(repo, &data.can_symlink)) < 0) + goto cleanup; report_progress(&data, NULL); - if (!(error = git_diff_foreach( - diff, &data, checkout_remove_the_old, NULL, NULL)) && - !(error = git_diff_foreach( - diff, &data, checkout_create_the_new, NULL, NULL)) && - data.found_submodules) - { - data.create_submodules = true; - error = git_diff_foreach( - diff, &data, checkout_create_the_new, NULL, NULL); - } + if (counts[CHECKOUT_ACTION__REMOVE] > 0 && + (error = checkout_remove_the_old(diff, actions, &data)) < 0) + goto cleanup; + + if ((counts[CHECKOUT_ACTION__CREATE_BLOB] + + counts[CHECKOUT_ACTION__NOTIFY]) > 0 && + (error = checkout_create_the_new(diff, actions, &data)) < 0) + goto cleanup; + + if (counts[CHECKOUT_ACTION__CREATE_SUBMODULE] > 0 && + (error = checkout_create_submodules(diff, actions, &data)) < 0) + goto cleanup; + + assert(data.completed_steps == data.total_steps); cleanup: if (error == GIT_EUSER) - error = (data.error != 0) ? data.error : -1; + giterr_clear(); + git__free(actions); + git__free(counts); git_diff_list_free(diff); git_buf_free(&workdir); @@ -449,7 +560,7 @@ int git_checkout_head( if ((error = git_repository_head(&head, repo)) < 0) return error; - + if ((error = git_reference_peel(&tree, head, GIT_OBJ_TREE)) < 0) goto cleanup; diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 89bc1da15..18c59a45d 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -351,7 +351,7 @@ void test_checkout_index__wont_notify_of_expected_line_ending_changes(void) { cl_git_pass(p_unlink("./testrepo/.gitattributes")); set_core_autocrlf_to(true); - + cl_git_mkfile("./testrepo/new.txt", "my new file\r\n"); g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; @@ -377,3 +377,34 @@ void test_checkout_index__calls_progress_callback(void) cl_git_pass(git_checkout_index(g_repo, &g_opts)); cl_assert_equal_i(was_called, true); } + +void test_checkout_index__can_overcome_name_clashes(void) +{ + git_index *index; + + cl_git_pass(git_repository_index(&index, g_repo)); + git_index_clear(index); + + cl_git_mkfile("./testrepo/path0", "content\r\n"); + cl_git_pass(p_mkdir("./testrepo/path1", 0777)); + cl_git_mkfile("./testrepo/path1/file1", "content\r\n"); + + cl_git_pass(git_index_add(index, "path0", 0)); + cl_git_pass(git_index_add(index, "path1/file1", 0)); + + cl_git_pass(p_unlink("./testrepo/path0")); + cl_git_pass(git_futils_rmdir_r( + "./testrepo/path1", NULL, GIT_RMDIR_REMOVE_FILES)); + + cl_git_mkfile("./testrepo/path1", "content\r\n"); + cl_git_pass(p_mkdir("./testrepo/path0", 0777)); + cl_git_mkfile("./testrepo/path0/file0", "content\r\n"); + + g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + cl_git_pass(git_checkout_index(g_repo, &g_opts)); + + cl_assert(git_path_isfile("./testrepo/path1")); + cl_assert(git_path_isfile("./testrepo/path0/file0")); + + git_index_free(index); +} From 18eff2ad7027cb82ccf61b4ae7c4fa8d75c6eace Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 16:44:39 -0800 Subject: [PATCH 03/11] Clean up a couple things missed in rebase --- tests-clar/stash/drop.c | 2 +- tests-clar/stash/foreach.c | 4 ++-- tests-clar/stash/save.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests-clar/stash/drop.c b/tests-clar/stash/drop.c index 5bbc7452a..39139ccae 100644 --- a/tests-clar/stash/drop.c +++ b/tests-clar/stash/drop.c @@ -15,7 +15,7 @@ void test_stash_drop__cleanup(void) { git_signature_free(signature); git_repository_free(repo); - cl_git_pass(git_futils_rmdir_r("stash", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("stash", NULL, GIT_RMDIR_REMOVE_FILES)); } void test_stash_drop__cannot_drop_from_an_empty_stash(void) diff --git a/tests-clar/stash/foreach.c b/tests-clar/stash/foreach.c index 818d906e7..d7127a9db 100644 --- a/tests-clar/stash/foreach.c +++ b/tests-clar/stash/foreach.c @@ -30,7 +30,7 @@ void test_stash_foreach__cleanup(void) { git_signature_free(signature); git_repository_free(repo); - cl_git_pass(git_futils_rmdir_r(REPO_NAME, NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r(REPO_NAME, NULL, GIT_RMDIR_REMOVE_FILES)); } static int callback_cb( @@ -45,7 +45,7 @@ static int callback_cb( GIT_UNUSED(message); cl_assert_equal_i(0, git_oid_streq(stash_oid, data->oids[data->invokes++])); - + return 0; } diff --git a/tests-clar/stash/save.c b/tests-clar/stash/save.c index 01acf672c..b4b7b5a4a 100644 --- a/tests-clar/stash/save.c +++ b/tests-clar/stash/save.c @@ -27,7 +27,7 @@ void test_stash_save__cleanup(void) { git_signature_free(signature); git_repository_free(repo); - cl_git_pass(git_futils_rmdir_r("stash", NULL, GIT_DIRREMOVAL_FILES_AND_DIRS)); + cl_git_pass(git_futils_rmdir_r("stash", NULL, GIT_RMDIR_REMOVE_FILES)); } static void assert_object_oid(const char* revision, const char* expected_oid, git_otype type) From 220d5a6c3572574a2fcf8869816390eadebbb792 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 16:45:25 -0800 Subject: [PATCH 04/11] Make iterator ignore eval lazy This makes it so that the check if a file is ignored will be deferred until requested on the workdir iterator, instead of aggressively evaluating the ignore rules for each entry. This should improve performance because there will be no need to check ignore rules for files that are already in the index. --- src/iterator.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/iterator.c b/src/iterator.c index 5fac41046..20878a060 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -641,13 +641,12 @@ static int workdir_iterator__update_entry(workdir_iterator *wi) wi->entry.path = ps->path; - /* skip over .git entry */ + /* skip over .git entries */ if (STRCMP_CASESELECT(wi->base.ignore_case, ps->path, DOT_GIT "/") == 0 || STRCMP_CASESELECT(wi->base.ignore_case, ps->path, DOT_GIT) == 0) return workdir_iterator__advance((git_iterator *)wi, NULL); - /* if there is an error processing the entry, treat as ignored */ - wi->is_ignored = 1; + wi->is_ignored = -1; git_index__init_entry_from_stat(&ps->st, &wi->entry); @@ -655,12 +654,10 @@ static int workdir_iterator__update_entry(workdir_iterator *wi) wi->entry.mode = git_futils_canonical_mode(ps->st.st_mode); /* if this is a file type we don't handle, treat as ignored */ - if (wi->entry.mode == 0) + if (wi->entry.mode == 0) { + wi->is_ignored = 1; return 0; - - /* okay, we are far enough along to look up real ignore rule */ - if (git_ignore__lookup(&wi->ignores, wi->entry.path, &wi->is_ignored) < 0) - return 0; /* if error, ignore it and ignore file */ + } /* detect submodules */ if (S_ISDIR(wi->entry.mode)) { @@ -908,8 +905,18 @@ notfound: int git_iterator_current_is_ignored(git_iterator *iter) { - return (iter->type != GIT_ITERATOR_WORKDIR) ? 0 : - ((workdir_iterator *)iter)->is_ignored; + workdir_iterator *wi = (workdir_iterator *)iter; + + if (iter->type != GIT_ITERATOR_WORKDIR) + return 0; + + if (wi->is_ignored != -1) + return wi->is_ignored; + + if (git_ignore__lookup(&wi->ignores, wi->entry.path, &wi->is_ignored) < 0) + wi->is_ignored = 1; + + return wi->is_ignored; } int git_iterator_advance_into_directory( From 2e3d4b96c08f1b0e2ee9b248c53aec523d70fd25 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 16:47:28 -0800 Subject: [PATCH 05/11] Move pathspec code in separate files Diff uses a `git_strarray` of path specs to represent a subset of all files to be processed. It is useful to be able to reuse this filtering in other places outside diff, so I've moved it into a standalone set of utilities. --- src/diff.c | 172 +++++++++++-------------------------------------- src/pathspec.c | 151 +++++++++++++++++++++++++++++++++++++++++++ src/pathspec.h | 32 +++++++++ 3 files changed, 220 insertions(+), 135 deletions(-) create mode 100644 src/pathspec.c create mode 100644 src/pathspec.h diff --git a/src/diff.c b/src/diff.c index 55f6ee7d5..f3dfdd9dc 100644 --- a/src/diff.c +++ b/src/diff.c @@ -10,76 +10,7 @@ #include "config.h" #include "attr_file.h" #include "filter.h" - -static char *diff_prefix_from_pathspec(const git_strarray *pathspec) -{ - git_buf prefix = GIT_BUF_INIT; - const char *scan; - - if (git_buf_common_prefix(&prefix, pathspec) < 0) - return NULL; - - /* diff prefix will only be leading non-wildcards */ - for (scan = prefix.ptr; *scan; ++scan) { - if (git__iswildcard(*scan) && - (scan == prefix.ptr || (*(scan - 1) != '\\'))) - break; - } - git_buf_truncate(&prefix, scan - prefix.ptr); - - if (prefix.size <= 0) { - git_buf_free(&prefix); - return NULL; - } - - git_buf_unescape(&prefix); - - return git_buf_detach(&prefix); -} - -static bool diff_pathspec_is_interesting(const git_strarray *pathspec) -{ - const char *str; - - if (pathspec == NULL || pathspec->count == 0) - return false; - if (pathspec->count > 1) - return true; - - str = pathspec->strings[0]; - if (!str || !str[0] || (!str[1] && (str[0] == '*' || str[0] == '.'))) - return false; - return true; -} - -static bool diff_path_matches_pathspec(git_diff_list *diff, const char *path) -{ - unsigned int i; - git_attr_fnmatch *match; - - if (!diff->pathspec.length) - return true; - - git_vector_foreach(&diff->pathspec, i, match) { - int result = strcmp(match->pattern, path) ? FNM_NOMATCH : 0; - - if (((diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) == 0) && - result == FNM_NOMATCH) - result = p_fnmatch(match->pattern, path, 0); - - /* if we didn't match, look for exact dirname prefix match */ - if (result == FNM_NOMATCH && - (match->flags & GIT_ATTR_FNMATCH_HASWILD) == 0 && - strncmp(path, match->pattern, match->length) == 0 && - path[match->length] == '/') - result = 0; - - if (result == 0) - return (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) ? false : true; - } - - return false; -} +#include "pathspec.h" static git_diff_delta *diff_delta__alloc( git_diff_list *diff, @@ -125,7 +56,10 @@ static int diff_delta__from_one( (diff->opts.flags & GIT_DIFF_INCLUDE_UNTRACKED) == 0) return 0; - if (!diff_path_matches_pathspec(diff, entry->path)) + if (!git_pathspec_match_path( + &diff->pathspec, entry->path, + (diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0, + (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0)) return 0; delta = diff_delta__alloc(diff, status, entry->path); @@ -295,7 +229,6 @@ static git_diff_list *git_diff_list_alloc( git_repository *repo, const git_diff_options *opts) { git_config *cfg; - size_t i; git_diff_list *diff = git__calloc(1, sizeof(git_diff_list)); if (diff == NULL) return NULL; @@ -333,7 +266,10 @@ static git_diff_list *git_diff_list_alloc( return diff; memcpy(&diff->opts, opts, sizeof(git_diff_options)); - memset(&diff->opts.pathspec, 0, sizeof(diff->opts.pathspec)); + + /* pathspec init will do nothing for empty pathspec */ + if (git_pathspec_init(&diff->pathspec, &opts->pathspec, &diff->pool) < 0) + goto fail; /* TODO: handle config diff.mnemonicprefix, diff.noprefix */ @@ -355,35 +291,6 @@ static git_diff_list *git_diff_list_alloc( if (diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES) diff->opts.flags |= GIT_DIFF_INCLUDE_TYPECHANGE; - /* only copy pathspec if it is "interesting" so we can test - * diff->pathspec.length > 0 to know if it is worth calling - * fnmatch as we iterate. - */ - if (!diff_pathspec_is_interesting(&opts->pathspec)) - return diff; - - if (git_vector_init( - &diff->pathspec, (unsigned int)opts->pathspec.count, NULL) < 0) - goto fail; - - for (i = 0; i < opts->pathspec.count; ++i) { - int ret; - const char *pattern = opts->pathspec.strings[i]; - git_attr_fnmatch *match = git__calloc(1, sizeof(git_attr_fnmatch)); - if (!match) - goto fail; - match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE; - ret = git_attr_fnmatch__parse(match, &diff->pool, NULL, &pattern); - if (ret == GIT_ENOTFOUND) { - git__free(match); - continue; - } else if (ret < 0) - goto fail; - - if (git_vector_insert(&diff->pathspec, match) < 0) - goto fail; - } - return diff; fail: @@ -394,7 +301,6 @@ fail: static void diff_list_free(git_diff_list *diff) { git_diff_delta *delta; - git_attr_fnmatch *match; unsigned int i; git_vector_foreach(&diff->deltas, i, delta) { @@ -403,12 +309,7 @@ static void diff_list_free(git_diff_list *diff) } git_vector_free(&diff->deltas); - git_vector_foreach(&diff->pathspec, i, match) { - git__free(match); - diff->pathspec.contents[i] = NULL; - } - git_vector_free(&diff->pathspec); - + git_pathspec_free(&diff->pathspec); git_pool_clear(&diff->pool); git__free(diff); } @@ -499,7 +400,10 @@ static int maybe_modified( GIT_UNUSED(old_iter); - if (!diff_path_matches_pathspec(diff, oitem->path)) + if (!git_pathspec_match_path( + &diff->pathspec, oitem->path, + (diff->opts.flags & GIT_DIFF_DISABLE_PATHSPEC_MATCH) != 0, + (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0)) return 0; /* on platforms with no symlinks, preserve mode of existing symlinks */ @@ -842,15 +746,15 @@ int git_diff_tree_to_tree( git_diff_list **diff) { git_iterator *a = NULL, *b = NULL; - char *prefix = opts ? diff_prefix_from_pathspec(&opts->pathspec) : NULL; + char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; assert(repo && old_tree && new_tree && diff); - if (git_iterator_for_tree_range(&a, repo, old_tree, prefix, prefix) < 0 || - git_iterator_for_tree_range(&b, repo, new_tree, prefix, prefix) < 0) + if (git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx) < 0 || + git_iterator_for_tree_range(&b, repo, new_tree, pfx, pfx) < 0) return -1; - git__free(prefix); + git__free(pfx); return diff_from_iterators(repo, opts, a, b, diff); } @@ -862,20 +766,20 @@ int git_diff_index_to_tree( git_diff_list **diff) { git_iterator *a = NULL, *b = NULL; - char *prefix = opts ? diff_prefix_from_pathspec(&opts->pathspec) : NULL; + char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; assert(repo && diff); - if (git_iterator_for_tree_range(&a, repo, old_tree, prefix, prefix) < 0 || - git_iterator_for_index_range(&b, repo, prefix, prefix) < 0) + if (git_iterator_for_tree_range(&a, repo, old_tree, pfx, pfx) < 0 || + git_iterator_for_index_range(&b, repo, pfx, pfx) < 0) goto on_error; - git__free(prefix); + git__free(pfx); return diff_from_iterators(repo, opts, a, b, diff); on_error: - git__free(prefix); + git__free(pfx); git_iterator_free(a); return -1; } @@ -885,23 +789,22 @@ int git_diff_workdir_to_index( const git_diff_options *opts, git_diff_list **diff) { - git_iterator *a = NULL, *b = NULL; int error; - - char *prefix = opts ? diff_prefix_from_pathspec(&opts->pathspec) : NULL; + git_iterator *a = NULL, *b = NULL; + char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; assert(repo && diff); - if ((error = git_iterator_for_index_range(&a, repo, prefix, prefix)) < 0 || - (error = git_iterator_for_workdir_range(&b, repo, prefix, prefix)) < 0) + if ((error = git_iterator_for_index_range(&a, repo, pfx, pfx)) < 0 || + (error = git_iterator_for_workdir_range(&b, repo, pfx, pfx)) < 0) goto on_error; - git__free(prefix); + git__free(pfx); return diff_from_iterators(repo, opts, a, b, diff); on_error: - git__free(prefix); + git__free(pfx); git_iterator_free(a); return error; } @@ -910,26 +813,25 @@ on_error: int git_diff_workdir_to_tree( git_repository *repo, const git_diff_options *opts, - git_tree *old_tree, + git_tree *tree, git_diff_list **diff) { - git_iterator *a = NULL, *b = NULL; int error; + git_iterator *a = NULL, *b = NULL; + char *pfx = opts ? git_pathspec_prefix(&opts->pathspec) : NULL; - char *prefix = opts ? diff_prefix_from_pathspec(&opts->pathspec) : NULL; + assert(repo && tree && diff); - assert(repo && old_tree && diff); - - if ((error = git_iterator_for_tree_range(&a, repo, old_tree, prefix, prefix)) < 0 || - (error = git_iterator_for_workdir_range(&b, repo, prefix, prefix)) < 0) + if ((error = git_iterator_for_tree_range(&a, repo, tree, pfx, pfx)) < 0 || + (error = git_iterator_for_workdir_range(&b, repo, pfx, pfx)) < 0) goto on_error; - git__free(prefix); + git__free(pfx); return diff_from_iterators(repo, opts, a, b, diff); on_error: - git__free(prefix); + git__free(pfx); git_iterator_free(a); return error; } diff --git a/src/pathspec.c b/src/pathspec.c new file mode 100644 index 000000000..9632f5f13 --- /dev/null +++ b/src/pathspec.c @@ -0,0 +1,151 @@ +/* + * Copyright (C) 2009-2012 the libgit2 contributors + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ + +#include "pathspec.h" +#include "attr_file.h" + +/* what is the common non-wildcard prefix for all items in the pathspec */ +char *git_pathspec_prefix(const git_strarray *pathspec) +{ + git_buf prefix = GIT_BUF_INIT; + const char *scan; + + if (!pathspec || !pathspec->count || + git_buf_common_prefix(&prefix, pathspec) < 0) + return NULL; + + /* diff prefix will only be leading non-wildcards */ + for (scan = prefix.ptr; *scan; ++scan) { + if (git__iswildcard(*scan) && + (scan == prefix.ptr || (*(scan - 1) != '\\'))) + break; + } + git_buf_truncate(&prefix, scan - prefix.ptr); + + if (prefix.size <= 0) { + git_buf_free(&prefix); + return NULL; + } + + git_buf_unescape(&prefix); + + return git_buf_detach(&prefix); +} + +/* is there anything in the spec that needs to be filtered on */ +bool git_pathspec_is_interesting(const git_strarray *pathspec) +{ + const char *str; + + if (pathspec == NULL || pathspec->count == 0) + return false; + if (pathspec->count > 1) + return true; + + str = pathspec->strings[0]; + if (!str || !str[0] || (!str[1] && (str[0] == '*' || str[0] == '.'))) + return false; + return true; +} + +/* build a vector of fnmatch patterns to evaluate efficiently */ +int git_pathspec_init( + git_vector *vspec, const git_strarray *strspec, git_pool *strpool) +{ + size_t i; + + memset(vspec, 0, sizeof(*vspec)); + + if (!git_pathspec_is_interesting(strspec)) + return 0; + + if (git_vector_init(vspec, strspec->count, NULL) < 0) + return -1; + + for (i = 0; i < strspec->count; ++i) { + int ret; + const char *pattern = strspec->strings[i]; + git_attr_fnmatch *match = git__calloc(1, sizeof(git_attr_fnmatch)); + if (!match) + return -1; + + match->flags = GIT_ATTR_FNMATCH_ALLOWSPACE; + + ret = git_attr_fnmatch__parse(match, strpool, NULL, &pattern); + if (ret == GIT_ENOTFOUND) { + git__free(match); + continue; + } else if (ret < 0) + return ret; + + if (git_vector_insert(vspec, match) < 0) + return -1; + } + + return 0; +} + +/* free data from the pathspec vector */ +void git_pathspec_free(git_vector *vspec) +{ + git_attr_fnmatch *match; + unsigned int i; + + git_vector_foreach(vspec, i, match) { + git__free(match); + vspec->contents[i] = NULL; + } + + git_vector_free(vspec); +} + +/* match a path against the vectorized pathspec */ +bool git_pathspec_match_path( + git_vector *vspec, const char *path, bool disable_fnmatch, bool casefold) +{ + unsigned int i; + git_attr_fnmatch *match; + int fnmatch_flags = 0; + int (*use_strcmp)(const char *, const char *); + int (*use_strncmp)(const char *, const char *, size_t); + + if (!vspec || !vspec->length) + return true; + + if (disable_fnmatch) + fnmatch_flags = -1; + else if (casefold) + fnmatch_flags = FNM_CASEFOLD; + + if (casefold) { + use_strcmp = strcasecmp; + use_strncmp = strncasecmp; + } else { + use_strcmp = strcmp; + use_strncmp = strncmp; + } + + git_vector_foreach(vspec, i, match) { + int result = use_strcmp(match->pattern, path) ? FNM_NOMATCH : 0; + + if (fnmatch_flags >= 0 && result == FNM_NOMATCH) + result = p_fnmatch(match->pattern, path, fnmatch_flags); + + /* if we didn't match, look for exact dirname prefix match */ + if (result == FNM_NOMATCH && + (match->flags & GIT_ATTR_FNMATCH_HASWILD) == 0 && + use_strncmp(path, match->pattern, match->length) == 0 && + path[match->length] == '/') + result = 0; + + if (result == 0) + return (match->flags & GIT_ATTR_FNMATCH_NEGATIVE) ? false : true; + } + + return false; +} + diff --git a/src/pathspec.h b/src/pathspec.h new file mode 100644 index 000000000..31a1cdad9 --- /dev/null +++ b/src/pathspec.h @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2011-2012 the libgit2 contributors + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_pathspec_h__ +#define INCLUDE_pathspec_h__ + +#include "common.h" +#include "buffer.h" +#include "vector.h" +#include "pool.h" + +/* what is the common non-wildcard prefix for all items in the pathspec */ +extern char *git_pathspec_prefix(const git_strarray *pathspec); + +/* is there anything in the spec that needs to be filtered on */ +extern bool git_pathspec_is_interesting(const git_strarray *pathspec); + +/* build a vector of fnmatch patterns to evaluate efficiently */ +extern int git_pathspec_init( + git_vector *vspec, const git_strarray *strspec, git_pool *strpool); + +/* free data from the pathspec vector */ +extern void git_pathspec_free(git_vector *vspec); + +/* match a path against the vectorized pathspec */ +extern bool git_pathspec_match_path( + git_vector *vspec, const char *path, bool disable_fnmatch, bool casefold); + +#endif From 55cbd05b18960e761a4d237ce5f1ff06455da98d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 16:56:34 -0800 Subject: [PATCH 06/11] Some diff refactorings to help code reuse There are some diff functions that are useful in a rewritten checkout and this lays some groundwork for that. This contains three main things: 1. Share the function diff uses to calculate the OID for a file in the working directory (now named `git_diff__oid_for_file` 2. Add a `git_diff__paired_foreach` function to iterator over two diff lists concurrently. Convert status to use it. 3. Move all the string/prefix/index entry comparisons into function pointers inside the `git_diff_list` object so they can be switched between case sensitive and insensitive versions. This makes them easier to reuse in various functions without replicating logic. As part of this, move a couple of index functions out of diff.c and into index.c. --- src/attr_file.h | 1 + src/diff.c | 166 +++++++++++++++++++++++++--------------------- src/diff.h | 8 +++ src/diff_output.c | 55 +++++++++++++++ src/diff_output.h | 6 ++ src/index.c | 20 +++++- src/index.h | 5 +- src/iterator.c | 2 +- src/status.c | 83 ++++++++++------------- src/submodule.c | 2 +- src/util.h | 5 ++ 11 files changed, 226 insertions(+), 127 deletions(-) diff --git a/src/attr_file.h b/src/attr_file.h index 3ea13d273..5bdfc7054 100644 --- a/src/attr_file.h +++ b/src/attr_file.h @@ -7,6 +7,7 @@ #ifndef INCLUDE_attr_file_h__ #define INCLUDE_attr_file_h__ +#include "git2/oid.h" #include "git2/attr.h" #include "vector.h" #include "pool.h" diff --git a/src/diff.c b/src/diff.c index f3dfdd9dc..015c77edd 100644 --- a/src/diff.c +++ b/src/diff.c @@ -327,24 +327,39 @@ void git_diff_list_addref(git_diff_list *diff) GIT_REFCOUNT_INC(diff); } -static int oid_for_workdir_item( +int git_diff__oid_for_file( git_repository *repo, - const git_index_entry *item, + const char *path, + uint16_t mode, + git_off_t size, git_oid *oid) { int result = 0; git_buf full_path = GIT_BUF_INIT; if (git_buf_joinpath( - &full_path, git_repository_workdir(repo), item->path) < 0) + &full_path, git_repository_workdir(repo), path) < 0) return -1; + if (!mode) { + struct stat st; + + if (p_stat(path, &st) < 0) { + giterr_set(GITERR_OS, "Could not stat '%s'", path); + result = -1; + goto cleanup; + } + + mode = st.st_mode; + size = st.st_size; + } + /* calculate OID for file if possible */ - if (S_ISGITLINK(item->mode)) { + if (S_ISGITLINK(mode)) { git_submodule *sm; const git_oid *sm_oid; - if (!git_submodule_lookup(&sm, repo, item->path) && + if (!git_submodule_lookup(&sm, repo, path) && (sm_oid = git_submodule_wd_oid(sm)) != NULL) git_oid_cpy(oid, sm_oid); else { @@ -354,23 +369,22 @@ static int oid_for_workdir_item( giterr_clear(); memset(oid, 0, sizeof(*oid)); } - } else if (S_ISLNK(item->mode)) + } else if (S_ISLNK(mode)) { result = git_odb__hashlink(oid, full_path.ptr); - else if (!git__is_sizet(item->file_size)) { - giterr_set(GITERR_OS, "File size overflow for 32-bit systems"); + } else if (!git__is_sizet(size)) { + giterr_set(GITERR_OS, "File size overflow (for 32-bits) on '%s'", path); result = -1; } else { git_vector filters = GIT_VECTOR_INIT; - result = git_filters_load( - &filters, repo, item->path, GIT_FILTER_TO_ODB); + result = git_filters_load(&filters, repo, path, GIT_FILTER_TO_ODB); if (result >= 0) { int fd = git_futils_open_ro(full_path.ptr); if (fd < 0) result = fd; else { result = git_odb__hashfd_filtered( - oid, fd, (size_t)item->file_size, GIT_OBJ_BLOB, &filters); + oid, fd, (size_t)size, GIT_OBJ_BLOB, &filters); p_close(fd); } } @@ -378,8 +392,8 @@ static int oid_for_workdir_item( git_filters_free(&filters); } +cleanup: git_buf_free(&full_path); - return result; } @@ -439,8 +453,7 @@ static int maybe_modified( } /* if oids and modes match, then file is unmodified */ - else if (git_oid_cmp(&oitem->oid, &nitem->oid) == 0 && - omode == nmode) + else if (git_oid_equal(&oitem->oid, &nitem->oid) && omode == nmode) status = GIT_DELTA_UNMODIFIED; /* if we have an unknown OID and a workdir iterator, then check some @@ -493,12 +506,14 @@ static int maybe_modified( /* if we got here and decided that the files are modified, but we * haven't calculated the OID of the new item, then calculate it now */ - if (status != GIT_DELTA_UNMODIFIED && git_oid_iszero(&nitem->oid)) { - if (oid_for_workdir_item(diff->repo, nitem, &noid) < 0) + if (status != GIT_DELTA_UNMODIFIED && + git_oid_iszero(&nitem->oid) && !use_noid) + { + if (git_diff__oid_for_file(diff->repo, + nitem->path, nitem->mode, nitem->file_size, &noid) < 0) return -1; - else if (omode == nmode && git_oid_equal(&oitem->oid, &noid)) + if (omode == nmode && git_oid_equal(&oitem->oid, &noid)) status = GIT_DELTA_UNMODIFIED; - /* store calculated oid so we don't have to recalc later */ use_noid = &noid; } @@ -507,31 +522,14 @@ static int maybe_modified( diff, status, oitem, omode, nitem, nmode, use_noid); } -static int git_index_entry_cmp_case(const void *a, const void *b) -{ - const git_index_entry *entry_a = a; - const git_index_entry *entry_b = b; - - return strcmp(entry_a->path, entry_b->path); -} - -static int git_index_entry_cmp_icase(const void *a, const void *b) -{ - const git_index_entry *entry_a = a; - const git_index_entry *entry_b = b; - - return strcasecmp(entry_a->path, entry_b->path); -} - static bool entry_is_prefixed( + git_diff_list *diff, const git_index_entry *item, - git_iterator *prefix_iterator, const git_index_entry *prefix_item) { size_t pathlen; - if (!prefix_item || - ITERATOR_PREFIXCMP(*prefix_iterator, prefix_item->path, item->path)) + if (!prefix_item || diff->prefixcmp(prefix_item->path, item->path)) return false; pathlen = strlen(item->path); @@ -541,6 +539,35 @@ static bool entry_is_prefixed( prefix_item->path[pathlen] == '/'); } +static int diff_list_init_from_iterators( + git_diff_list *diff, + git_iterator *old_iter, + git_iterator *new_iter) +{ + diff->old_src = old_iter->type; + diff->new_src = new_iter->type; + + /* Use case-insensitive compare if either iterator has + * the ignore_case bit set */ + if (!old_iter->ignore_case && !new_iter->ignore_case) { + diff->opts.flags &= ~GIT_DIFF_DELTAS_ARE_ICASE; + + diff->strcmp = strcmp; + diff->strncmp = strncmp; + diff->prefixcmp = git__prefixcmp; + diff->entrycmp = git_index_entry__cmp; + } else { + diff->opts.flags |= GIT_DIFF_DELTAS_ARE_ICASE; + + diff->strcmp = strcasecmp; + diff->strncmp = strncasecmp; + diff->prefixcmp = git__prefixcmp_icase; + diff->entrycmp = git_index_entry__cmp_icase; + } + + return 0; +} + static int diff_from_iterators( git_repository *repo, const git_diff_options *opts, /**< can be NULL for defaults */ @@ -548,37 +575,31 @@ static int diff_from_iterators( git_iterator *new_iter, git_diff_list **diff_ptr) { + int error = 0; const git_index_entry *oitem, *nitem; git_buf ignore_prefix = GIT_BUF_INIT; git_diff_list *diff = git_diff_list_alloc(repo, opts); - git_vector_cmp entry_compare; - if (!diff) + *diff_ptr = NULL; + + if (!diff || + diff_list_init_from_iterators(diff, old_iter, new_iter) < 0) goto fail; - diff->old_src = old_iter->type; - diff->new_src = new_iter->type; - - /* Use case-insensitive compare if either iterator has - * the ignore_case bit set */ - if (!old_iter->ignore_case && !new_iter->ignore_case) { - entry_compare = git_index_entry_cmp_case; - diff->opts.flags &= ~GIT_DIFF_DELTAS_ARE_ICASE; - } else { - entry_compare = git_index_entry_cmp_icase; - diff->opts.flags |= GIT_DIFF_DELTAS_ARE_ICASE; - + if (diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) { /* If one of the iterators doesn't have ignore_case set, * then that's unfortunate because we'll have to spool * its data, sort it icase, and then use that for our * merge join to the other iterator that is icase sorted */ - if (!old_iter->ignore_case) { - if (git_iterator_spoolandsort(&old_iter, old_iter, git_index_entry_cmp_icase, true) < 0) - goto fail; - } else if (!new_iter->ignore_case) { - if (git_iterator_spoolandsort(&new_iter, new_iter, git_index_entry_cmp_icase, true) < 0) - goto fail; - } + if (!old_iter->ignore_case && + git_iterator_spoolandsort( + &old_iter, old_iter, diff->entrycmp, true) < 0) + goto fail; + + if (!new_iter->ignore_case && + git_iterator_spoolandsort( + &new_iter, new_iter, diff->entrycmp, true) < 0) + goto fail; } if (git_iterator_current(old_iter, &oitem) < 0 || @@ -589,7 +610,7 @@ static int diff_from_iterators( while (oitem || nitem) { /* create DELETED records for old items not matched in new */ - if (oitem && (!nitem || entry_compare(oitem, nitem) < 0)) { + if (oitem && (!nitem || diff->entrycmp(oitem, nitem) < 0)) { if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0) goto fail; @@ -597,7 +618,7 @@ static int diff_from_iterators( * instead of just generating a DELETE record */ if ((diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES) != 0 && - entry_is_prefixed(oitem, new_iter, nitem)) + entry_is_prefixed(diff, oitem, nitem)) { /* this entry has become a tree! convert to TYPECHANGE */ git_diff_delta *last = diff_delta__last_for_item(diff, oitem); @@ -614,13 +635,12 @@ static int diff_from_iterators( /* create ADDED, TRACKED, or IGNORED records for new items not * matched in old (and/or descend into directories as needed) */ - else if (nitem && (!oitem || entry_compare(oitem, nitem) > 0)) { + else if (nitem && (!oitem || diff->entrycmp(oitem, nitem) > 0)) { git_delta_t delta_type = GIT_DELTA_UNTRACKED; /* check if contained in ignored parent directory */ if (git_buf_len(&ignore_prefix) && - ITERATOR_PREFIXCMP(*old_iter, nitem->path, - git_buf_cstr(&ignore_prefix)) == 0) + diff->prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0) delta_type = GIT_DELTA_IGNORED; if (S_ISDIR(nitem->mode)) { @@ -629,7 +649,7 @@ static int diff_from_iterators( * directories and it is not under an ignored directory. */ bool contains_tracked = - entry_is_prefixed(nitem, old_iter, oitem); + entry_is_prefixed(diff, nitem, oitem); bool recurse_untracked = (delta_type == GIT_DELTA_UNTRACKED && (diff->opts.flags & GIT_DIFF_RECURSE_UNTRACKED_DIRS) != 0); @@ -693,7 +713,7 @@ static int diff_from_iterators( */ if (delta_type != GIT_DELTA_IGNORED && (diff->opts.flags & GIT_DIFF_INCLUDE_TYPECHANGE_TREES) != 0 && - entry_is_prefixed(nitem, old_iter, oitem)) + entry_is_prefixed(diff, nitem, oitem)) { /* this entry was a tree! convert to TYPECHANGE */ git_diff_delta *last = diff_delta__last_for_item(diff, oitem); @@ -711,7 +731,7 @@ static int diff_from_iterators( * (or ADDED and DELETED pair if type changed) */ else { - assert(oitem && nitem && entry_compare(oitem, nitem) == 0); + assert(oitem && nitem && diff->entrycmp(oitem, nitem) == 0); if (maybe_modified(old_iter, oitem, new_iter, nitem, diff) < 0 || git_iterator_advance(old_iter, &oitem) < 0 || @@ -720,21 +740,19 @@ static int diff_from_iterators( } } - git_iterator_free(old_iter); - git_iterator_free(new_iter); - git_buf_free(&ignore_prefix); - *diff_ptr = diff; - return 0; fail: + if (!*diff_ptr) { + git_diff_list_free(diff); + error = -1; + } + git_iterator_free(old_iter); git_iterator_free(new_iter); git_buf_free(&ignore_prefix); - git_diff_list_free(diff); - *diff_ptr = NULL; - return -1; + return error; } diff --git a/src/diff.h b/src/diff.h index ed66439bf..e9d8fd5a7 100644 --- a/src/diff.h +++ b/src/diff.h @@ -41,6 +41,11 @@ struct git_diff_list { git_iterator_type_t old_src; git_iterator_type_t new_src; uint32_t diffcaps; + + int (*strcmp)(const char *, const char *); + int (*strncmp)(const char *, const char *, size_t); + int (*prefixcmp)(const char *str, const char *pfx); + int (*entrycmp)(const void *a, const void *b); }; extern void git_diff__cleanup_modes( @@ -53,5 +58,8 @@ extern int git_diff_delta__cmp(const void *a, const void *b); extern bool git_diff_delta__should_skip( const git_diff_options *opts, const git_diff_delta *delta); +extern int git_diff__oid_for_file( + git_repository *, const char *, uint16_t, git_off_t, git_oid *); + #endif diff --git a/src/diff_output.c b/src/diff_output.c index e678ec857..2f61540ff 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -1573,3 +1573,58 @@ int git_diff_patch_to_str( return error; } + +int git_diff__paired_foreach( + git_diff_list *idx2head, + git_diff_list *wd2idx, + int (*cb)(void *cbref, git_diff_delta *i2h, git_diff_delta *w2i), + void *cbref) +{ + int cmp; + git_diff_delta *i2h, *w2i; + size_t i, j, i_max, j_max; + bool icase = false; + + i_max = idx2head ? idx2head->deltas.length : 0; + j_max = wd2idx ? wd2idx->deltas.length : 0; + + if (idx2head && wd2idx && + (0 != (idx2head->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) || + 0 != (wd2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE))) + { + /* Then use the ignore-case sorter... */ + icase = true; + + /* and assert that both are ignore-case sorted. If this function + * ever needs to support merge joining result sets that are not sorted + * by the same function, then it will need to be extended to do a spool + * and sort on one of the results before merge joining */ + assert(0 != (idx2head->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) && + 0 != (wd2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE)); + } + + for (i = 0, j = 0; i < i_max || j < j_max; ) { + i2h = idx2head ? GIT_VECTOR_GET(&idx2head->deltas,i) : NULL; + w2i = wd2idx ? GIT_VECTOR_GET(&wd2idx->deltas,j) : NULL; + + cmp = !w2i ? -1 : !i2h ? 1 : + STRCMP_CASESELECT(icase, i2h->old_file.path, w2i->old_file.path); + + if (cmp < 0) { + if (cb(cbref, i2h, NULL)) + return GIT_EUSER; + i++; + } else if (cmp > 0) { + if (cb(cbref, NULL, w2i)) + return GIT_EUSER; + j++; + } else { + if (cb(cbref, i2h, w2i)) + return GIT_EUSER; + i++; j++; + } + } + + return 0; +} + diff --git a/src/diff_output.h b/src/diff_output.h index 5fed1d998..f74dd3a71 100644 --- a/src/diff_output.h +++ b/src/diff_output.h @@ -83,4 +83,10 @@ typedef struct { uint32_t diffed : 1; } diff_delta_context; +extern int git_diff__paired_foreach( + git_diff_list *idx2head, + git_diff_list *wd2idx, + int (*cb)(void *cbref, git_diff_delta *i2h, git_diff_delta *w2i), + void *cbref); + #endif diff --git a/src/index.c b/src/index.c index 214d29def..c1b4565a3 100644 --- a/src/index.c +++ b/src/index.c @@ -516,7 +516,7 @@ git_index_entry *git_index_get_bypath(git_index *index, const char *path, int st return git_index_get_byindex(index, pos); } -void git_index__init_entry_from_stat(struct stat *st, git_index_entry *entry) +void git_index_entry__init_from_stat(git_index_entry *entry, struct stat *st) { entry->ctime.seconds = (git_time_t)st->st_ctime; entry->mtime.seconds = (git_time_t)st->st_mtime; @@ -530,6 +530,22 @@ void git_index__init_entry_from_stat(struct stat *st, git_index_entry *entry) entry->file_size = st->st_size; } +int git_index_entry__cmp(const void *a, const void *b) +{ + const git_index_entry *entry_a = a; + const git_index_entry *entry_b = b; + + return strcmp(entry_a->path, entry_b->path); +} + +int git_index_entry__cmp_icase(const void *a, const void *b) +{ + const git_index_entry *entry_a = a; + const git_index_entry *entry_b = b; + + return strcasecmp(entry_a->path, entry_b->path); +} + static int index_entry_init(git_index_entry **entry_out, git_index *index, const char *rel_path) { git_index_entry *entry = NULL; @@ -568,7 +584,7 @@ static int index_entry_init(git_index_entry **entry_out, git_index *index, const entry = git__calloc(1, sizeof(git_index_entry)); GITERR_CHECK_ALLOC(entry); - git_index__init_entry_from_stat(&st, entry); + git_index_entry__init_from_stat(entry, &st); entry->oid = oid; entry->path = git__strdup(rel_path); diff --git a/src/index.h b/src/index.h index 86158eb84..9778a543a 100644 --- a/src/index.h +++ b/src/index.h @@ -41,8 +41,11 @@ struct git_index { git_vector_cmp reuc_search; }; -extern void git_index__init_entry_from_stat(struct stat *st, git_index_entry *entry); +extern void git_index_entry__init_from_stat(git_index_entry *entry, struct stat *st); extern unsigned int git_index__prefix_position(git_index *index, const char *path); +extern int git_index_entry__cmp(const void *a, const void *b); +extern int git_index_entry__cmp_icase(const void *a, const void *b); + #endif diff --git a/src/iterator.c b/src/iterator.c index 20878a060..33b775ce1 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -648,7 +648,7 @@ static int workdir_iterator__update_entry(workdir_iterator *wi) wi->is_ignored = -1; - git_index__init_entry_from_stat(&ps->st, &wi->entry); + git_index_entry__init_from_stat(&wi->entry, &ps->st); /* need different mode here to keep directories during iteration */ wi->entry.mode = git_futils_canonical_mode(ps->st.st_mode); diff --git a/src/status.c b/src/status.c index 2d022bfda..0bd170e6d 100644 --- a/src/status.c +++ b/src/status.c @@ -17,6 +17,7 @@ #include "git2/diff.h" #include "diff.h" +#include "diff_output.h" static unsigned int index_delta2status(git_delta_t index_status) { @@ -76,21 +77,43 @@ static unsigned int workdir_delta2status(git_delta_t workdir_status) return st; } +typedef struct { + int (*cb)(const char *, unsigned int, void *); + void *cbdata; +} status_user_callback; + +static int status_invoke_cb( + void *cbref, git_diff_delta *i2h, git_diff_delta *w2i) +{ + status_user_callback *usercb = cbref; + const char *path = NULL; + unsigned int status = 0; + + if (w2i) { + path = w2i->old_file.path; + status |= workdir_delta2status(w2i->status); + } + if (i2h) { + path = i2h->old_file.path; + status |= index_delta2status(i2h->status); + } + + return usercb->cb(path, status, usercb->cbdata); +} + int git_status_foreach_ext( git_repository *repo, const git_status_options *opts, int (*cb)(const char *, unsigned int, void *), void *cbdata) { - int err = 0, cmp; + int err = 0; git_diff_options diffopt; git_diff_list *idx2head = NULL, *wd2idx = NULL; git_tree *head = NULL; git_status_show_t show = opts ? opts->show : GIT_STATUS_SHOW_INDEX_AND_WORKDIR; - git_diff_delta *i2h, *w2i; - size_t i, j, i_max, j_max; - bool ignore_case = false; + status_user_callback usercb; assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR); @@ -126,55 +149,19 @@ int git_status_foreach_ext( (err = git_diff_workdir_to_index(repo, &diffopt, &wd2idx)) < 0) goto cleanup; + usercb.cb = cb; + usercb.cbdata = cbdata; + if (show == GIT_STATUS_SHOW_INDEX_THEN_WORKDIR) { - for (i = 0; !err && i < idx2head->deltas.length; i++) { - i2h = GIT_VECTOR_GET(&idx2head->deltas, i); - if (cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata)) - err = GIT_EUSER; - } + if ((err = git_diff__paired_foreach( + idx2head, NULL, status_invoke_cb, &usercb)) < 0) + goto cleanup; + git_diff_list_free(idx2head); idx2head = NULL; } - i_max = idx2head ? idx2head->deltas.length : 0; - j_max = wd2idx ? wd2idx->deltas.length : 0; - - if (idx2head && wd2idx && - (0 != (idx2head->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) || - 0 != (wd2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE))) - { - /* Then use the ignore-case sorter... */ - ignore_case = true; - - /* and assert that both are ignore-case sorted. If this function - * ever needs to support merge joining result sets that are not sorted - * by the same function, then it will need to be extended to do a spool - * and sort on one of the results before merge joining */ - assert(0 != (idx2head->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) && - 0 != (wd2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE)); - } - - for (i = 0, j = 0; !err && (i < i_max || j < j_max); ) { - i2h = idx2head ? GIT_VECTOR_GET(&idx2head->deltas,i) : NULL; - w2i = wd2idx ? GIT_VECTOR_GET(&wd2idx->deltas,j) : NULL; - - cmp = !w2i ? -1 : !i2h ? 1 : STRCMP_CASESELECT(ignore_case, i2h->old_file.path, w2i->old_file.path); - - if (cmp < 0) { - if (cb(i2h->old_file.path, index_delta2status(i2h->status), cbdata)) - err = GIT_EUSER; - i++; - } else if (cmp > 0) { - if (cb(w2i->old_file.path, workdir_delta2status(w2i->status), cbdata)) - err = GIT_EUSER; - j++; - } else { - if (cb(i2h->old_file.path, index_delta2status(i2h->status) | - workdir_delta2status(w2i->status), cbdata)) - err = GIT_EUSER; - i++; j++; - } - } + err = git_diff__paired_foreach(idx2head, wd2idx, status_invoke_cb, &usercb); cleanup: git_tree_free(head); diff --git a/src/submodule.c b/src/submodule.c index d69559dc2..1364b6881 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -371,7 +371,7 @@ int git_submodule_add_to_index(git_submodule *sm, int write_index) memset(&entry, 0, sizeof(entry)); entry.path = sm->path; - git_index__init_entry_from_stat(&st, &entry); + git_index_entry__init_from_stat(&entry, &st); /* calling git_submodule_open will have set sm->wd_oid if possible */ if ((sm->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID) == 0) { diff --git a/src/util.h b/src/util.h index 3d00e9c85..23d4bc6e9 100644 --- a/src/util.h +++ b/src/util.h @@ -81,6 +81,11 @@ extern int git__prefixcmp(const char *str, const char *prefix); extern int git__prefixcmp_icase(const char *str, const char *prefix); extern int git__suffixcmp(const char *str, const char *suffix); +GIT_INLINE(int) git__signum(int val) +{ + return ((val > 0) - (val < 0)); +} + extern int git__strtol32(int32_t *n, const char *buff, const char **end_buf, int base); extern int git__strtol64(int64_t *n, const char *buff, const char **end_buf, int base); From ad9a921b92a964a0f28a5f0d59079cde5a0ada1e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 17:05:07 -0800 Subject: [PATCH 07/11] Rework checkout with new strategy options This is a major reworking of checkout strategy options. The checkout code is now sensitive to the contents of the HEAD tree and the new options allow you to update the working tree so that it will match the index content only when it previously matched the contents of the HEAD. This allows you to, for example, to distinguish between removing files that are in the HEAD but not in the index, vs just removing all untracked files. Because of various corner cases that arise, etc., this required some additional capabilities in rmdir and other utility functions. This includes the beginnings of an implementation of code to read a partial tree into the index based on a pathspec, but that is not enabled because of the possibility of creating conflicting index entries. --- include/git2/checkout.h | 155 ++++++++-- include/git2/errors.h | 1 + src/checkout.c | 470 +++++++++++++++++++++---------- src/fileops.c | 48 +++- src/fileops.h | 2 + src/index.c | 53 ++++ src/index.h | 3 + src/path.c | 5 +- src/reset.c | 5 +- src/stash.c | 4 +- tests-clar/checkout/index.c | 74 +++-- tests-clar/checkout/tree.c | 5 +- tests-clar/checkout/typechange.c | 13 +- tests-clar/clone/network.c | 2 +- 14 files changed, 612 insertions(+), 228 deletions(-) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index 390d2f215..a9314c2cb 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -25,20 +25,117 @@ GIT_BEGIN_DECL * Checkout behavior flags * * These flags control what checkout does with files. Pass in a - * combination of these values OR'ed together. + * combination of these values OR'ed together. If you just pass zero + * (i.e. no flags), then you are effectively doing a "dry run" where no + * files will be modified. + * + * Checkout groups the working directory content into 3 classes of files: + * (1) files that don't need a change, and files that do need a change + * that either (2) we are allowed to modifed or (3) we are not. The flags + * you pass in will decide which files we are allowed to modify. + * + * By default, checkout is not allowed to modify any files. Anything + * needing a change would be considered a conflict. + * + * GIT_CHECKOUT_UPDATE_UNMODIFIED means that checkout is allowed to update + * any file where the working directory content matches the HEAD + * (e.g. either the files match or the file is absent in both places). + * + * GIT_CHECKOUT_UPDATE_MISSING means checkout can create a missing file + * that exists in the index and does not exist in the working directory. + * This is usually desirable for initial checkout, etc. Technically, the + * missing file differs from the HEAD, which is why this is separate. + * + * GIT_CHECKOUT_UPDATE_MODIFIED means checkout is allowed to update files + * where the working directory does not match the HEAD so long as the file + * actually exists in the HEAD. This option implies UPDATE_UNMODIFIED. + * + * GIT_CHECKOUT_UPDATE_UNTRACKED means checkout is allowed to update files + * even if there is a working directory version that does not exist in the + * HEAD (i.e. the file was independently created in the workdir). This + * implies UPDATE_UNMODIFIED | UPDATE_MISSING (but *not* UPDATE_MODIFIED). + * + * + * On top of these three basic strategies, there are some modifiers + * options that can be applied: + * + * If any files need update but are disallowed by the strategy, normally + * checkout calls the conflict callback (if given) and then aborts. + * GIT_CHECKOUT_ALLOW_CONFLICTS means it is okay to update the files that + * are allowed by the strategy even if there are conflicts. The conflict + * callbacks are still made, but non-conflicting files will be updated. + * + * Any unmerged entries in the index are automatically considered conflicts. + * If you want to proceed anyhow and just skip unmerged entries, you can use + * GIT_CHECKOUT_SKIP_UNMERGED which is less dangerous than just allowing all + * conflicts. Alternatively, use GIT_CHECKOUT_USE_OURS to proceed and + * checkout the stage 2 ("ours") version. GIT_CHECKOUT_USE_THEIRS means to + * proceed and use the stage 3 ("theirs") version. + * + * GIT_CHECKOUT_UPDATE_ONLY means that update is not allowed to create new + * files or delete old ones, only update existing content. With this + * flag, files that needs to be created or deleted are not conflicts - + * they are just skipped. This also skips typechanges to existing files + * (because the old would have to be removed). + * + * GIT_CHECKOUT_REMOVE_UNTRACKED means that files in the working directory + * that are untracked (and not ignored) will be removed altogether. These + * untracked files (that do not shadow index entries) are not considered + * conflicts and would normally be ignored. + * + * + * Checkout is "semi-atomic" as in it will go through the work to be done + * before making any changes and if may decide to abort if there are + * conflicts, or you can use the conflict callback to explicitly abort the + * action before any updates are made. Despite this, if a second process + * is modifying the filesystem while checkout is running, it can't + * guarantee that the choices is makes while initially examining the + * filesystem are still going to be correct as it applies them. */ typedef enum { - /** Checkout does not update any files in the working directory. */ - GIT_CHECKOUT_DEFAULT = (1 << 0), + GIT_CHECKOUT_DEFAULT = 0, /** default is a dry run, no actual updates */ - /** When a file exists and is modified, replace it with new version. */ - GIT_CHECKOUT_OVERWRITE_MODIFIED = (1 << 1), + /** Allow update of entries where working dir matches HEAD. */ + GIT_CHECKOUT_UPDATE_UNMODIFIED = (1u << 0), - /** When a file does not exist in the working directory, create it. */ - GIT_CHECKOUT_CREATE_MISSING = (1 << 2), + /** Allow update of entries where working dir does not have file. */ + GIT_CHECKOUT_UPDATE_MISSING = (1u << 1), + + /** Allow safe updates that cannot overwrite uncommited data */ + GIT_CHECKOUT_SAFE = + (GIT_CHECKOUT_UPDATE_UNMODIFIED | GIT_CHECKOUT_UPDATE_MISSING), + + /** Allow update of entries in working dir that are modified from HEAD. */ + GIT_CHECKOUT_UPDATE_MODIFIED = (1u << 2), + + /** Update existing untracked files that are now present in the index. */ + GIT_CHECKOUT_UPDATE_UNTRACKED = (1u << 3), + + /** Allow all updates to force working directory to look like index */ + GIT_CHECKOUT_FORCE = + (GIT_CHECKOUT_SAFE | GIT_CHECKOUT_UPDATE_MODIFIED | GIT_CHECKOUT_UPDATE_UNTRACKED), + + /** Allow checkout to make updates even if conflicts are found */ + GIT_CHECKOUT_ALLOW_CONFLICTS = (1u << 4), + + /** Remove untracked files not in index (that are not ignored) */ + GIT_CHECKOUT_REMOVE_UNTRACKED = (1u << 5), + + /** Only update existing files, don't create new ones */ + GIT_CHECKOUT_UPDATE_ONLY = (1u << 6), + + /** Allow checkout to skip unmerged files */ + GIT_CHECKOUT_SKIP_UNMERGED = (1u << 10), + /** For unmerged files, checkout stage 2 from index */ + GIT_CHECKOUT_USE_OURS = (1u << 11), + /** For unmerged files, checkout stage 3 from index */ + GIT_CHECKOUT_USE_THEIRS = (1u << 12), + + /** Recursively checkout submodule with same options */ + GIT_CHECKOUT_UPDATE_SUBMODULES = (1u << 16), + /** Recursively checkout submodules only if HEAD moved in super repo */ + GIT_CHECKOUT_UPDATE_SUBMODULES_IF_CHANGED = (1u << 17), - /** If an untracked file in found in the working dir, delete it. */ - GIT_CHECKOUT_REMOVE_UNTRACKED = (1 << 3), } git_checkout_strategy_t; /** @@ -47,38 +144,38 @@ typedef enum { * Use zeros to indicate default settings. */ typedef struct git_checkout_opts { - unsigned int checkout_strategy; /** default: GIT_CHECKOUT_DEFAULT */ + unsigned int checkout_strategy; /** default will be a dry run */ + int disable_filters; /** don't apply filters like CRLF conversion */ int dir_mode; /** default is 0755 */ int file_mode; /** default is 0644 or 0755 as dictated by blob */ int file_open_flags; /** default is O_CREAT | O_TRUNC | O_WRONLY */ - /** Optional callback to notify the consumer of files that - * haven't be checked out because a modified version of them - * exist in the working directory. - * - * When provided, this callback will be invoked when the flag - * GIT_CHECKOUT_OVERWRITE_MODIFIED isn't part of the checkout strategy. + /** Optional callback made on files where the index differs from the + * working directory but the rules do not allow update. Return a + * non-zero value to abort the checkout. All such callbacks will be + * made before any changes are made to the working directory. */ - int (* skipped_notify_cb)( - const char *skipped_file, - const git_oid *blob_oid, - int file_mode, + int (*conflict_cb)( + const char *conflicting_path, + const git_oid *index_oid, + unsigned int index_mode, + unsigned int wd_mode, void *payload); - void *notify_payload; + void *conflict_payload; /* Optional callback to notify the consumer of checkout progress. */ - void (* progress_cb)( - const char *path, - size_t completed_steps, - size_t total_steps, - void *payload); + void (*progress_cb)( + const char *path, + size_t completed_steps, + size_t total_steps, + void *payload); void *progress_payload; - /** When not NULL, array of fnmatch patterns specifying - * which paths should be taken into account + /** When not zeroed out, array of fnmatch patterns specifying which + * paths should be taken into account, otherwise all files. */ - git_strarray paths; + git_strarray paths; } git_checkout_opts; /** diff --git a/include/git2/errors.h b/include/git2/errors.h index 8bb47f354..45e04578d 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -60,6 +60,7 @@ typedef enum { GITERR_SUBMODULE, GITERR_THREAD, GITERR_STASH, + GITERR_CHECKOUT, } git_error_t; /** diff --git a/src/checkout.c b/src/checkout.c index e4a397cd5..2bad06501 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -22,20 +22,19 @@ #include "filter.h" #include "blob.h" #include "diff.h" +#include "pathspec.h" -struct checkout_diff_data -{ +typedef struct { + git_repository *repo; + git_diff_list *diff; + git_checkout_opts *opts; git_buf *path; size_t workdir_len; - git_checkout_opts *checkout_opts; - git_repository *owner; bool can_symlink; - bool found_submodules; - bool create_submodules; int error; size_t total_steps; size_t completed_steps; -}; +} checkout_diff_data; static int buffer_to_file( git_buf *buffer, @@ -112,7 +111,8 @@ static int blob_content_to_file( if (!file_mode) file_mode = entry_filemode; - error = buffer_to_file(&filtered, path, opts->dir_mode, opts->file_open_flags, file_mode); + error = buffer_to_file( + &filtered, path, opts->dir_mode, opts->file_open_flags, file_mode); cleanup: git_filters_free(&filters); @@ -123,7 +123,8 @@ cleanup: return error; } -static int blob_content_to_link(git_blob *blob, const char *path, bool can_symlink) +static int blob_content_to_link( + git_blob *blob, const char *path, bool can_symlink) { git_buf linktarget = GIT_BUF_INIT; int error; @@ -142,58 +143,52 @@ static int blob_content_to_link(git_blob *blob, const char *path, bool can_symli } static int checkout_submodule( - struct checkout_diff_data *data, + checkout_diff_data *data, const git_diff_file *file) { + /* Until submodules are supported, UPDATE_ONLY means do nothing here */ + if ((data->opts->checkout_strategy & GIT_CHECKOUT_UPDATE_ONLY) != 0) + return 0; + if (git_futils_mkdir( - file->path, git_repository_workdir(data->owner), - data->checkout_opts->dir_mode, GIT_MKDIR_PATH) < 0) + file->path, git_repository_workdir(data->repo), + data->opts->dir_mode, GIT_MKDIR_PATH) < 0) return -1; - /* TODO: two cases: + /* TODO: Support checkout_strategy options. Two circumstances: * 1 - submodule already checked out, but we need to move the HEAD * to the new OID, or * 2 - submodule not checked out and we should recursively check it out * - * Checkout will not execute a pull request on the submodule, but a - * clone command should probably be able to. Do we need a submodule - * callback option? + * Checkout will not execute a pull on the submodule, but a clone + * command should probably be able to. Do we need a submodule callback? */ return 0; } static void report_progress( - struct checkout_diff_data *data, + checkout_diff_data *data, const char *path) { - if (data->checkout_opts->progress_cb) - data->checkout_opts->progress_cb( - path, - data->completed_steps, - data->total_steps, - data->checkout_opts->progress_payload); + if (data->opts->progress_cb) + data->opts->progress_cb( + path, data->completed_steps, data->total_steps, + data->opts->progress_payload); } static int checkout_blob( - struct checkout_diff_data *data, + checkout_diff_data *data, const git_diff_file *file) { - git_blob *blob; int error = 0; + git_blob *blob; git_buf_truncate(data->path, data->workdir_len); - if (git_buf_joinpath(data->path, git_buf_cstr(data->path), file->path) < 0) + if (git_buf_puts(data->path, file->path) < 0) return -1; - /* If there is a directory where this blob should go, then there is an - * existing tree that conflicts with this file, but REMOVE_UNTRACKED must - * not have been passed in. Signal to caller with GIT_ENOTFOUND. - */ - if (git_path_isdir(git_buf_cstr(data->path))) - return GIT_ENOTFOUND; - - if ((error = git_blob_lookup(&blob, data->owner, &file->oid)) < 0) + if ((error = git_blob_lookup(&blob, data->repo, &file->oid)) < 0) return error; if (S_ISLNK(file->mode)) @@ -201,14 +196,14 @@ static int checkout_blob( blob, git_buf_cstr(data->path), data->can_symlink); else error = blob_content_to_file( - blob, git_buf_cstr(data->path), file->mode, data->checkout_opts); + blob, git_buf_cstr(data->path), file->mode, data->opts); git_blob_free(blob); return error; } -static int retrieve_symlink_capabilities(git_repository *repo, bool *can_symlink) +static int retrieve_symlink_caps(git_repository *repo, bool *can_symlink) { git_config *cfg; int error; @@ -218,10 +213,7 @@ static int retrieve_symlink_capabilities(git_repository *repo, bool *can_symlink error = git_config_get_bool((int *)can_symlink, cfg, "core.symlinks"); - /* - * When no "core.symlinks" entry is found in any of the configuration - * store (local, global or system), default value is "true". - */ + /* If "core.symlinks" is not found anywhere, default to true. */ if (error == GIT_ENOTFOUND) { *can_symlink = true; error = 0; @@ -230,7 +222,8 @@ static int retrieve_symlink_capabilities(git_repository *repo, bool *can_symlink return error; } -static void normalize_options(git_checkout_opts *normalized, git_checkout_opts *proposed) +static void normalize_options( + git_checkout_opts *normalized, git_checkout_opts *proposed) { assert(normalized); @@ -239,11 +232,16 @@ static void normalize_options(git_checkout_opts *normalized, git_checkout_opts * else memmove(normalized, proposed, sizeof(git_checkout_opts)); - /* Default options */ - if (!normalized->checkout_strategy) - normalized->checkout_strategy = GIT_CHECKOUT_DEFAULT; + /* implied checkout strategies */ + if ((normalized->checkout_strategy & GIT_CHECKOUT_UPDATE_MODIFIED) != 0 || + (normalized->checkout_strategy & GIT_CHECKOUT_UPDATE_UNTRACKED) != 0) + normalized->checkout_strategy |= GIT_CHECKOUT_UPDATE_UNMODIFIED; + + if ((normalized->checkout_strategy & GIT_CHECKOUT_UPDATE_UNTRACKED) != 0) + normalized->checkout_strategy |= GIT_CHECKOUT_UPDATE_MISSING; /* opts->disable_filters is false by default */ + if (!normalized->dir_mode) normalized->dir_mode = GIT_DIR_MODE; @@ -254,50 +252,174 @@ static void normalize_options(git_checkout_opts *normalized, git_checkout_opts * enum { CHECKOUT_ACTION__NONE = 0, CHECKOUT_ACTION__REMOVE = 1, - CHECKOUT_ACTION__CREATE_BLOB = 2, - CHECKOUT_ACTION__CREATE_SUBMODULE = 4, - CHECKOUT_ACTION__NOTIFY = 8 + CHECKOUT_ACTION__UPDATE_BLOB = 2, + CHECKOUT_ACTION__UPDATE_SUBMODULE = 4, + CHECKOUT_ACTION__CONFLICT = 8, + CHECKOUT_ACTION__MAX = 8 }; -static uint32_t checkout_action_for_delta( - git_checkout_opts *opts, - const git_diff_delta *delta) +static int checkout_confirm_update_blob( + checkout_diff_data *data, + const git_diff_delta *delta, + int action) { - uint32_t action = CHECKOUT_ACTION__NONE; + int error; + unsigned int strat = data->opts->checkout_strategy; + struct stat st; + bool update_only = ((strat & GIT_CHECKOUT_UPDATE_ONLY) != 0); + + /* for typechange, remove the old item first */ + if (delta->status == GIT_DELTA_TYPECHANGE) { + if (update_only) + action = CHECKOUT_ACTION__NONE; + else + action |= CHECKOUT_ACTION__REMOVE; + + return action; + } + + git_buf_truncate(data->path, data->workdir_len); + if (git_buf_puts(data->path, delta->new_file.path) < 0) + return -1; + + if ((error = p_stat(git_buf_cstr(data->path), &st)) < 0) { + if (errno == ENOENT) { + if (update_only) + action = CHECKOUT_ACTION__NONE; + } else if (errno == ENOTDIR) { + /* File exists where a parent dir needs to go - i.e. untracked + * typechange. Ignore if UPDATE_ONLY, remove if allowed. + */ + if (update_only) + action = CHECKOUT_ACTION__NONE; + else if ((strat & GIT_CHECKOUT_UPDATE_UNTRACKED) != 0) + action |= CHECKOUT_ACTION__REMOVE; + else + action = CHECKOUT_ACTION__CONFLICT; + } + /* otherwise let error happen when we attempt blob checkout later */ + } + else if (S_ISDIR(st.st_mode)) { + /* Directory exists where a blob needs to go - i.e. untracked + * typechange. Ignore if UPDATE_ONLY, remove if allowed. + */ + if (update_only) + action = CHECKOUT_ACTION__NONE; + else if ((strat & GIT_CHECKOUT_UPDATE_UNTRACKED) != 0) + action |= CHECKOUT_ACTION__REMOVE; + else + action = CHECKOUT_ACTION__CONFLICT; + } + + return action; +} + +static int checkout_action_for_delta( + checkout_diff_data *data, + const git_diff_delta *delta, + const git_index_entry *head_entry) +{ + int action = CHECKOUT_ACTION__NONE; + unsigned int strat = data->opts->checkout_strategy; switch (delta->status) { - case GIT_DELTA_UNTRACKED: - if ((opts->checkout_strategy & GIT_CHECKOUT_REMOVE_UNTRACKED) != 0) - action |= CHECKOUT_ACTION__REMOVE; + case GIT_DELTA_UNMODIFIED: + if (!head_entry) { + /* file independently created in wd, even though not in HEAD */ + if ((strat & GIT_CHECKOUT_UPDATE_MISSING) == 0) + action = CHECKOUT_ACTION__CONFLICT; + } + else if (!git_oid_equal(&head_entry->oid, &delta->old_file.oid)) { + /* working directory was independently updated to match index */ + if ((strat & GIT_CHECKOUT_UPDATE_MODIFIED) == 0) + action = CHECKOUT_ACTION__CONFLICT; + } break; - case GIT_DELTA_TYPECHANGE: - if ((opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0) - action |= CHECKOUT_ACTION__REMOVE | CHECKOUT_ACTION__CREATE_BLOB; - else if (opts->skipped_notify_cb != NULL) - action = CHECKOUT_ACTION__NOTIFY; + case GIT_DELTA_ADDED: + /* Impossible. New files should be UNTRACKED or TYPECHANGE */ + action = CHECKOUT_ACTION__CONFLICT; break; case GIT_DELTA_DELETED: - if ((opts->checkout_strategy & GIT_CHECKOUT_CREATE_MISSING) != 0) - action |= CHECKOUT_ACTION__CREATE_BLOB; + if (head_entry && /* working dir missing, but exists in HEAD */ + (strat & GIT_CHECKOUT_UPDATE_MISSING) == 0) + action = CHECKOUT_ACTION__CONFLICT; + else + action = CHECKOUT_ACTION__UPDATE_BLOB; break; case GIT_DELTA_MODIFIED: - if ((opts->checkout_strategy & GIT_CHECKOUT_OVERWRITE_MODIFIED) != 0) - action |= CHECKOUT_ACTION__CREATE_BLOB; - else if (opts->skipped_notify_cb != NULL) - action = CHECKOUT_ACTION__NOTIFY; + case GIT_DELTA_TYPECHANGE: + if (!head_entry) { + /* working dir was independently updated & does not match index */ + if ((strat & GIT_CHECKOUT_UPDATE_UNTRACKED) == 0) + action = CHECKOUT_ACTION__CONFLICT; + else + action = CHECKOUT_ACTION__UPDATE_BLOB; + } + else if (git_oid_equal(&head_entry->oid, &delta->new_file.oid)) + action = CHECKOUT_ACTION__UPDATE_BLOB; + else if ((strat & GIT_CHECKOUT_UPDATE_MODIFIED) == 0) + action = CHECKOUT_ACTION__CONFLICT; + else + action = CHECKOUT_ACTION__UPDATE_BLOB; break; + case GIT_DELTA_UNTRACKED: + if (!head_entry) { + if ((strat & GIT_CHECKOUT_REMOVE_UNTRACKED) != 0) + action = CHECKOUT_ACTION__REMOVE; + } + else if ((strat & GIT_CHECKOUT_UPDATE_MODIFIED) != 0) { + action = CHECKOUT_ACTION__REMOVE; + } else if ((strat & GIT_CHECKOUT_UPDATE_UNMODIFIED) != 0) { + git_oid wd_oid; + + /* if HEAD matches workdir, then remove, else conflict */ + + if (git_oid_iszero(&delta->new_file.oid) && + git_diff__oid_for_file( + data->repo, delta->new_file.path, delta->new_file.mode, + delta->new_file.size, &wd_oid) < 0) + action = -1; + else if (git_oid_equal(&head_entry->oid, &wd_oid)) + action = CHECKOUT_ACTION__REMOVE; + else + action = CHECKOUT_ACTION__CONFLICT; + } else { + /* present in HEAD and workdir, but absent in index */ + action = CHECKOUT_ACTION__CONFLICT; + } + break; + + case GIT_DELTA_IGNORED: default: + /* just skip these files */ break; } - if ((action & CHECKOUT_ACTION__CREATE_BLOB) != 0 && - S_ISGITLINK(delta->old_file.mode)) - action = (action & ~CHECKOUT_ACTION__CREATE_BLOB) | - CHECKOUT_ACTION__CREATE_SUBMODULE; + if (action > 0 && (action & CHECKOUT_ACTION__UPDATE_BLOB) != 0) { + if (S_ISGITLINK(delta->old_file.mode)) + action = (action & ~CHECKOUT_ACTION__UPDATE_BLOB) | + CHECKOUT_ACTION__UPDATE_SUBMODULE; + + action = checkout_confirm_update_blob(data, delta, action); + } + + if (action == CHECKOUT_ACTION__CONFLICT && + data->opts->conflict_cb != NULL && + data->opts->conflict_cb( + delta->old_file.path, &delta->old_file.oid, + delta->old_file.mode, delta->new_file.mode, + data->opts->conflict_payload) != 0) + { + giterr_clear(); + action = GIT_EUSER; + } + + if (action > 0 && (strat & GIT_CHECKOUT_UPDATE_ONLY) != 0) + action = (action & ~CHECKOUT_ACTION__REMOVE); return action; } @@ -305,53 +427,119 @@ static uint32_t checkout_action_for_delta( static int checkout_get_actions( uint32_t **actions_ptr, size_t **counts_ptr, - git_diff_list *diff, - git_checkout_opts *opts) + checkout_diff_data *data) { + int error; + git_diff_list *diff = data->diff; git_diff_delta *delta; size_t i, *counts; uint32_t *actions; + git_tree *head = NULL; + git_iterator *hiter = NULL; + char *pfx = git_pathspec_prefix(&data->opts->paths); + const git_index_entry *he; - *counts_ptr = counts = - git__calloc(CHECKOUT_ACTION__NOTIFY, sizeof(size_t)); - GITERR_CHECK_ALLOC(counts); + /* if there is no HEAD, that's okay - we'll make an empty iterator */ + (void)git_repository_head_tree(&head, data->repo); - *actions_ptr = actions = - git__calloc(diff->deltas.length, sizeof(uint32_t)); - GITERR_CHECK_ALLOC(actions); + if ((error = git_iterator_for_tree_range( + &hiter, data->repo, head, pfx, pfx)) < 0) + goto fail; - git_vector_foreach(&diff->deltas, i, delta) { - actions[i] = checkout_action_for_delta(opts, delta); + if ((diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0 && + !hiter->ignore_case && + (error = git_iterator_spoolandsort( + &hiter, hiter, diff->entrycmp, true)) < 0) + goto fail; - if (actions[i] & CHECKOUT_ACTION__REMOVE) - counts[CHECKOUT_ACTION__REMOVE]++; + if ((error = git_iterator_current(hiter, &he)) < 0) + goto fail; - if (actions[i] & CHECKOUT_ACTION__CREATE_BLOB) - counts[CHECKOUT_ACTION__CREATE_BLOB]++; + git__free(pfx); - if (actions[i] & CHECKOUT_ACTION__CREATE_SUBMODULE) - counts[CHECKOUT_ACTION__CREATE_SUBMODULE]++; - - if (actions[i] & CHECKOUT_ACTION__NOTIFY) - counts[CHECKOUT_ACTION__NOTIFY]++; + *counts_ptr = counts = git__calloc(CHECKOUT_ACTION__MAX+1, sizeof(size_t)); + *actions_ptr = actions = git__calloc(diff->deltas.length, sizeof(uint32_t)); + if (!counts || !actions) { + error = -1; + goto fail; } + git_vector_foreach(&diff->deltas, i, delta) { + int cmp = -1, act; + + /* try to track HEAD entries parallel to deltas */ + while (he) { + cmp = S_ISDIR(delta->new_file.mode) ? + diff->prefixcmp(he->path, delta->new_file.path) : + diff->strcmp(he->path, delta->old_file.path); + if (cmp >= 0) + break; + if (git_iterator_advance(hiter, &he) < 0) + he = NULL; + } + + act = checkout_action_for_delta(data, delta, !cmp ? he : NULL); + + if (act < 0) { + error = act; + goto fail; + } + + if (!cmp && git_iterator_advance(hiter, &he) < 0) + he = NULL; + + actions[i] = act; + + if (act & CHECKOUT_ACTION__REMOVE) + counts[CHECKOUT_ACTION__REMOVE]++; + if (act & CHECKOUT_ACTION__UPDATE_BLOB) + counts[CHECKOUT_ACTION__UPDATE_BLOB]++; + if (act & CHECKOUT_ACTION__UPDATE_SUBMODULE) + counts[CHECKOUT_ACTION__UPDATE_SUBMODULE]++; + if (act & CHECKOUT_ACTION__CONFLICT) + counts[CHECKOUT_ACTION__CONFLICT]++; + } + + if (counts[CHECKOUT_ACTION__CONFLICT] > 0 && + (data->opts->checkout_strategy & GIT_CHECKOUT_ALLOW_CONFLICTS) == 0) + { + giterr_set(GITERR_CHECKOUT, "%d conflicts prevent checkout", + (int)counts[CHECKOUT_ACTION__CONFLICT]); + goto fail; + } + + git_iterator_free(hiter); return 0; + +fail: + *counts_ptr = NULL; + git__free(counts); + *actions_ptr = NULL; + git__free(actions); + + git_iterator_free(hiter); + git__free(pfx); + + return -1; } static int checkout_remove_the_old( git_diff_list *diff, unsigned int *actions, - struct checkout_diff_data *data) + checkout_diff_data *data) { git_diff_delta *delta; size_t i; + git_buf_truncate(data->path, data->workdir_len); + git_vector_foreach(&diff->deltas, i, delta) { if (actions[i] & CHECKOUT_ACTION__REMOVE) { int error = git_futils_rmdir_r( - delta->new_file.path, git_buf_cstr(data->path), - GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS); + delta->new_file.path, + git_buf_cstr(data->path), /* here set to work dir root */ + GIT_RMDIR_REMOVE_FILES | GIT_RMDIR_EMPTY_PARENTS | + GIT_RMDIR_REMOVE_BLOCKERS); if (error < 0) return error; @@ -366,35 +554,20 @@ static int checkout_remove_the_old( static int checkout_create_the_new( git_diff_list *diff, unsigned int *actions, - struct checkout_diff_data *data) + checkout_diff_data *data) { git_diff_delta *delta; size_t i; git_vector_foreach(&diff->deltas, i, delta) { - if (actions[i] & CHECKOUT_ACTION__CREATE_BLOB) { + if (actions[i] & CHECKOUT_ACTION__UPDATE_BLOB) { int error = checkout_blob(data, &delta->old_file); - - /* ENOTFOUND means unable to create the file because of - * an existing parent dir. Probably flags were given - * asking to create new blobs without allowing removal - * of a conflicting tree (or vice versa). - */ - if (error == GIT_ENOTFOUND) - giterr_clear(); - else if (error < 0) + if (error < 0) return error; data->completed_steps++; report_progress(data, delta->old_file.path); } - - if (actions[i] & CHECKOUT_ACTION__NOTIFY) { - if (data->checkout_opts->skipped_notify_cb( - delta->old_file.path, &delta->old_file.oid, - delta->old_file.mode, data->checkout_opts->notify_payload)) - return GIT_EUSER; - } } return 0; @@ -403,13 +576,13 @@ static int checkout_create_the_new( static int checkout_create_submodules( git_diff_list *diff, unsigned int *actions, - struct checkout_diff_data *data) + checkout_diff_data *data) { git_diff_delta *delta; size_t i; git_vector_foreach(&diff->deltas, i, delta) { - if (actions[i] & CHECKOUT_ACTION__CREATE_SUBMODULE) { + if (actions[i] & CHECKOUT_ACTION__UPDATE_SUBMODULE) { int error = checkout_submodule(data, &delta->old_file); if (error < 0) return error; @@ -427,16 +600,13 @@ int git_checkout_index( git_checkout_opts *opts) { git_diff_list *diff = NULL; - git_diff_options diff_opts = {0}; git_checkout_opts checkout_opts; - struct checkout_diff_data data; + checkout_diff_data data; git_buf workdir = GIT_BUF_INIT; - uint32_t *actions = NULL; size_t *counts = NULL; - int error; assert(repo); @@ -445,9 +615,8 @@ int git_checkout_index( return error; diff_opts.flags = - GIT_DIFF_INCLUDE_UNTRACKED | - GIT_DIFF_INCLUDE_TYPECHANGE | - GIT_DIFF_SKIP_BINARY_CHECK; + GIT_DIFF_INCLUDE_UNMODIFIED | GIT_DIFF_INCLUDE_UNTRACKED | + GIT_DIFF_INCLUDE_TYPECHANGE | GIT_DIFF_SKIP_BINARY_CHECK; if (opts && opts->paths.count > 0) diff_opts.pathspec = opts->paths; @@ -470,33 +639,35 @@ int git_checkout_index( * 3. Then checkout all submodules in case a new .gitmodules blob was * checked out during pass #2. */ - if ((error = checkout_get_actions(&actions, &counts, diff, &checkout_opts)) < 0) - goto cleanup; memset(&data, 0, sizeof(data)); data.path = &workdir; data.workdir_len = git_buf_len(&workdir); - data.checkout_opts = &checkout_opts; - data.owner = repo; - data.total_steps = counts[CHECKOUT_ACTION__REMOVE] + - counts[CHECKOUT_ACTION__CREATE_BLOB] + - counts[CHECKOUT_ACTION__CREATE_SUBMODULE]; + data.repo = repo; + data.diff = diff; + data.opts = &checkout_opts; - if ((error = retrieve_symlink_capabilities(repo, &data.can_symlink)) < 0) + if ((error = checkout_get_actions(&actions, &counts, &data)) < 0) goto cleanup; - report_progress(&data, NULL); + data.total_steps = counts[CHECKOUT_ACTION__REMOVE] + + counts[CHECKOUT_ACTION__UPDATE_BLOB] + + counts[CHECKOUT_ACTION__UPDATE_SUBMODULE]; + + if ((error = retrieve_symlink_caps(repo, &data.can_symlink)) < 0) + goto cleanup; + + report_progress(&data, NULL); /* establish 0 baseline */ if (counts[CHECKOUT_ACTION__REMOVE] > 0 && (error = checkout_remove_the_old(diff, actions, &data)) < 0) goto cleanup; - if ((counts[CHECKOUT_ACTION__CREATE_BLOB] + - counts[CHECKOUT_ACTION__NOTIFY]) > 0 && + if (counts[CHECKOUT_ACTION__UPDATE_BLOB] > 0 && (error = checkout_create_the_new(diff, actions, &data)) < 0) goto cleanup; - if (counts[CHECKOUT_ACTION__CREATE_SUBMODULE] > 0 && + if (counts[CHECKOUT_ACTION__UPDATE_SUBMODULE] > 0 && (error = checkout_create_submodules(diff, actions, &data)) < 0) goto cleanup; @@ -519,32 +690,28 @@ int git_checkout_tree( git_object *treeish, git_checkout_opts *opts) { + int error = 0; git_index *index = NULL; git_tree *tree = NULL; - int error; - assert(repo && treeish); if (git_object_peel((git_object **)&tree, treeish, GIT_OBJ_TREE) < 0) { - giterr_set(GITERR_INVALID, "Provided treeish cannot be peeled into a tree."); - return GIT_ERROR; + giterr_set( + GITERR_CHECKOUT, "Provided object cannot be peeled to a tree"); + return -1; } - if ((error = git_repository_index(&index, repo)) < 0) - goto cleanup; + /* load paths in tree that match pathspec into index */ + if (!(error = git_repository_index(&index, repo)) && + !(error = git_index_read_tree_match( + index, tree, opts ? &opts->paths : NULL)) && + !(error = git_index_write(index))) + error = git_checkout_index(repo, opts); - if ((error = git_index_read_tree(index, tree)) < 0) - goto cleanup; - - if ((error = git_index_write(index)) < 0) - goto cleanup; - - error = git_checkout_index(repo, opts); - -cleanup: git_index_free(index); git_tree_free(tree); + return error; } @@ -552,21 +719,16 @@ int git_checkout_head( git_repository *repo, git_checkout_opts *opts) { - git_reference *head; int error; + git_reference *head = NULL; git_object *tree = NULL; assert(repo); - if ((error = git_repository_head(&head, repo)) < 0) - return error; + if (!(error = git_repository_head(&head, repo)) && + !(error = git_reference_peel(&tree, head, GIT_OBJ_TREE))) + error = git_checkout_tree(repo, tree, opts); - if ((error = git_reference_peel(&tree, head, GIT_OBJ_TREE)) < 0) - goto cleanup; - - error = git_checkout_tree(repo, tree, opts); - -cleanup: git_reference_free(head); git_object_free(tree); diff --git a/src/fileops.c b/src/fileops.c index d2cbd046d..5eebc5057 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -351,6 +351,7 @@ int git_futils_mkdir_r(const char *path, const char *base, const mode_t mode) } typedef struct { + const char *base; uint32_t flags; int error; } futils__rmdir_data; @@ -366,11 +367,52 @@ static int futils__error_cannot_rmdir(const char *path, const char *filemsg) return -1; } +static int futils__rm_first_parent(git_buf *path, const char *ceiling) +{ + int error = GIT_ENOTFOUND; + struct stat st; + + while (error == GIT_ENOTFOUND) { + git_buf_rtruncate_at_char(path, '/'); + + if (!path->size || git__prefixcmp(path->ptr, ceiling) != 0) + error = 0; + else if (p_lstat(path->ptr, &st) == 0) { + if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) + error = p_unlink(path->ptr); + else if (!S_ISDIR(st.st_mode)) + error = -1; /* fail to remove non-regular file */ + } else if (errno != ENOTDIR) + error = -1; + } + + if (error) + futils__error_cannot_rmdir(path->ptr, "cannot remove parent"); + + return error; +} + static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) { + struct stat st; futils__rmdir_data *data = opaque; - if (git_path_isdir(path->ptr) == true) { + if ((data->error = p_lstat(path->ptr, &st)) < 0) { + if (errno == ENOENT) + data->error = 0; + else if (errno == ENOTDIR) { + /* asked to remove a/b/c/d/e and a/b is a normal file */ + if ((data->flags & GIT_RMDIR_REMOVE_BLOCKERS) != 0) + data->error = futils__rm_first_parent(path, data->base); + else + futils__error_cannot_rmdir( + path->ptr, "parent is not directory"); + } + else + futils__error_cannot_rmdir(path->ptr, "cannot access"); + } + + else if (S_ISDIR(st.st_mode)) { int error = git_path_direach(path, futils__rmdir_recurs_foreach, data); if (error < 0) return (error == GIT_EUSER) ? data->error : error; @@ -393,9 +435,8 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) futils__error_cannot_rmdir(path->ptr, "cannot be removed"); } - else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0) { + else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0) data->error = futils__error_cannot_rmdir(path->ptr, "still present"); - } return data->error; } @@ -434,6 +475,7 @@ int git_futils_rmdir_r( if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0) return -1; + data.base = base ? base : ""; data.flags = flags; data.error = 0; diff --git a/src/fileops.h b/src/fileops.h index 6952c463c..a74f8b758 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -109,6 +109,7 @@ extern int git_futils_mkpath2file(const char *path, const mode_t mode); * * GIT_RMDIR_SKIP_NONEMPTY - skip non-empty directories with no error. * * GIT_RMDIR_EMPTY_PARENTS - remove containing directories up to base * if removing this item leaves them empty + * * GIT_RMDIR_REMOVE_BLOCKERS - remove blocking file that causes ENOTDIR * * The old values translate into the new as follows: * @@ -121,6 +122,7 @@ typedef enum { GIT_RMDIR_REMOVE_FILES = (1 << 0), GIT_RMDIR_SKIP_NONEMPTY = (1 << 1), GIT_RMDIR_EMPTY_PARENTS = (1 << 2), + GIT_RMDIR_REMOVE_BLOCKERS = (1 << 3), } git_futils_rmdir_flags; /** diff --git a/src/index.c b/src/index.c index c1b4565a3..0ae1b4479 100644 --- a/src/index.c +++ b/src/index.c @@ -13,6 +13,8 @@ #include "tree.h" #include "tree-cache.h" #include "hash.h" +#include "iterator.h" +#include "pathspec.h" #include "git2/odb.h" #include "git2/oid.h" #include "git2/blob.h" @@ -1590,3 +1592,54 @@ git_repository *git_index_owner(const git_index *index) { return INDEX_OWNER(index); } + +int git_index_read_tree_match( + git_index *index, git_tree *tree, git_strarray *strspec) +{ +#if 0 + git_iterator *iter = NULL; + const git_index_entry *entry; + char *pfx = NULL; + git_vector pathspec = GIT_VECTOR_INIT; + git_pool pathpool = GIT_POOL_INIT_STRINGPOOL; +#endif + + if (!git_pathspec_is_interesting(strspec)) + return git_index_read_tree(index, tree); + + return git_index_read_tree(index, tree); + +#if 0 + /* The following loads the matches into the index, but doesn't + * erase obsoleted entries (e.g. you load a blob at "a/b" which + * should obsolete a blob at "a/b/c/d" since b is no longer a tree) + */ + + if (git_pathspec_init(&pathspec, strspec, &pathpool) < 0) + return -1; + + pfx = git_pathspec_prefix(strspec); + + if ((error = git_iterator_for_tree_range( + &iter, INDEX_OWNER(index), tree, pfx, pfx)) < 0 || + (error = git_iterator_current(iter, &entry)) < 0) + goto cleanup; + + while (entry != NULL) { + if (git_pathspec_match_path(&pathspec, entry->path, false, false) && + (error = git_index_add(index, entry)) < 0) + goto cleanup; + + if ((error = git_iterator_advance(iter, &entry)) < 0) + goto cleanup; + } + +cleanup: + git_iterator_free(iter); + git_pathspec_free(&pathspec); + git_pool_clear(&pathpool); + git__free(pfx); + + return error; +#endif +} diff --git a/src/index.h b/src/index.h index 9778a543a..f0dcd64d5 100644 --- a/src/index.h +++ b/src/index.h @@ -48,4 +48,7 @@ extern unsigned int git_index__prefix_position(git_index *index, const char *pat extern int git_index_entry__cmp(const void *a, const void *b); extern int git_index_entry__cmp_icase(const void *a, const void *b); +extern int git_index_read_tree_match( + git_index *index, git_tree *tree, git_strarray *strspec); + #endif diff --git a/src/path.c b/src/path.c index 09556bd3f..98351bec3 100644 --- a/src/path.c +++ b/src/path.c @@ -382,9 +382,10 @@ int git_path_walk_up( iter.asize = path->asize; while (scan >= stop) { - if ((error = cb(data, &iter)) < 0) - break; + error = cb(data, &iter); iter.ptr[scan] = oldc; + if (error < 0) + break; scan = git_buf_rfind_next(&iter, '/'); if (scan >= 0) { scan++; diff --git a/src/reset.c b/src/reset.c index 7df1c1a57..69a9c4f04 100644 --- a/src/reset.c +++ b/src/reset.c @@ -137,10 +137,7 @@ int git_reset( } memset(&opts, 0, sizeof(opts)); - opts.checkout_strategy = - GIT_CHECKOUT_CREATE_MISSING - | GIT_CHECKOUT_OVERWRITE_MODIFIED - | GIT_CHECKOUT_REMOVE_UNTRACKED; + opts.checkout_strategy = GIT_CHECKOUT_FORCE; if (git_checkout_index(repo, &opts) < 0) { giterr_set(GITERR_INDEX, "%s - Failed to checkout the index.", ERROR_MSG); diff --git a/src/stash.c b/src/stash.c index 1d6940e3c..7bff466d1 100644 --- a/src/stash.c +++ b/src/stash.c @@ -501,8 +501,8 @@ static int reset_index_and_workdir( memset(&opts, 0, sizeof(git_checkout_opts)); - opts.checkout_strategy = - GIT_CHECKOUT_CREATE_MISSING | GIT_CHECKOUT_OVERWRITE_MODIFIED; + opts.checkout_strategy = + GIT_CHECKOUT_UPDATE_MODIFIED | GIT_CHECKOUT_UPDATE_UNTRACKED; if (remove_untracked) opts.checkout_strategy |= GIT_CHECKOUT_REMOVE_UNTRACKED; diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 18c59a45d..72044d01c 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -26,7 +26,7 @@ void test_checkout_index__initialize(void) git_tree *tree; memset(&g_opts, 0, sizeof(g_opts)); - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; g_repo = cl_git_sandbox_init("testrepo"); @@ -78,7 +78,6 @@ void test_checkout_index__can_create_missing_files(void) cl_assert_equal_i(false, git_path_isfile("./testrepo/branch_file.txt")); cl_assert_equal_i(false, git_path_isfile("./testrepo/new.txt")); - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; cl_git_pass(git_checkout_index(g_repo, &g_opts)); test_file_contents("./testrepo/README", "hey there\n"); @@ -94,7 +93,7 @@ void test_checkout_index__can_remove_untracked_files(void) cl_assert_equal_i(true, git_path_isdir("./testrepo/dir/subdir/subsubdir")); - g_opts.checkout_strategy = GIT_CHECKOUT_REMOVE_UNTRACKED; + g_opts.checkout_strategy |= GIT_CHECKOUT_REMOVE_UNTRACKED; cl_git_pass(git_checkout_index(g_repo, &g_opts)); cl_assert_equal_i(false, git_path_isdir("./testrepo/dir")); @@ -203,7 +202,11 @@ void test_checkout_index__donot_overwrite_modified_file_by_default(void) { cl_git_mkfile("./testrepo/new.txt", "This isn't what's stored!"); - g_opts.checkout_strategy = 0; + /* set this up to not return an error code on conflicts, but it + * still will not have permission to overwrite anything... + */ + g_opts.checkout_strategy = GIT_CHECKOUT_ALLOW_CONFLICTS; + cl_git_pass(git_checkout_index(g_repo, &g_opts)); test_file_contents("./testrepo/new.txt", "This isn't what's stored!"); @@ -213,7 +216,8 @@ void test_checkout_index__can_overwrite_modified_file(void) { cl_git_mkfile("./testrepo/new.txt", "This isn't what's stored!"); - g_opts.checkout_strategy = GIT_CHECKOUT_OVERWRITE_MODIFIED; + g_opts.checkout_strategy |= GIT_CHECKOUT_UPDATE_MODIFIED; + cl_git_pass(git_checkout_index(g_repo, &g_opts)); test_file_contents("./testrepo/new.txt", "my new file\n"); @@ -282,28 +286,30 @@ void test_checkout_index__options_open_flags(void) g_opts.file_open_flags = O_CREAT | O_RDWR | O_APPEND; - g_opts.checkout_strategy |= GIT_CHECKOUT_OVERWRITE_MODIFIED; + g_opts.checkout_strategy |= GIT_CHECKOUT_UPDATE_MODIFIED; cl_git_pass(git_checkout_index(g_repo, &g_opts)); test_file_contents("./testrepo/new.txt", "hi\nmy new file\n"); } -struct notify_data { +struct conflict_data { const char *file; const char *sha; }; -static int notify_cb( - const char *skipped_file, +static int conflict_cb( + const char *conflict_file, const git_oid *blob_oid, - int file_mode, + unsigned int index_mode, + unsigned int wd_mode, void *payload) { - struct notify_data *expectations = (struct notify_data *)payload; + struct conflict_data *expectations = (struct conflict_data *)payload; - GIT_UNUSED(file_mode); + GIT_UNUSED(index_mode); + GIT_UNUSED(wd_mode); - cl_assert_equal_s(expectations->file, skipped_file); + cl_assert_equal_s(expectations->file, conflict_file); cl_assert_equal_i(0, git_oid_streq(blob_oid, expectations->sha)); return 0; @@ -311,7 +317,7 @@ static int notify_cb( void test_checkout_index__can_notify_of_skipped_files(void) { - struct notify_data data; + struct conflict_data data; cl_git_mkfile("./testrepo/new.txt", "This isn't what's stored!"); @@ -324,22 +330,24 @@ void test_checkout_index__can_notify_of_skipped_files(void) data.file = "new.txt"; data.sha = "a71586c1dfe8a71c6cbf6c129f404c5642ff31bd"; - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; - g_opts.skipped_notify_cb = notify_cb; - g_opts.notify_payload = &data; + g_opts.checkout_strategy |= GIT_CHECKOUT_ALLOW_CONFLICTS; + g_opts.conflict_cb = conflict_cb; + g_opts.conflict_payload = &data; cl_git_pass(git_checkout_index(g_repo, &g_opts)); } -static int dont_notify_cb( - const char *skipped_file, +static int dont_conflict_cb( + const char *conflict_file, const git_oid *blob_oid, - int file_mode, + unsigned int index_mode, + unsigned int wd_mode, void *payload) { - GIT_UNUSED(skipped_file); + GIT_UNUSED(conflict_file); GIT_UNUSED(blob_oid); - GIT_UNUSED(file_mode); + GIT_UNUSED(index_mode); + GIT_UNUSED(wd_mode); GIT_UNUSED(payload); cl_assert(false); @@ -354,9 +362,9 @@ void test_checkout_index__wont_notify_of_expected_line_ending_changes(void) cl_git_mkfile("./testrepo/new.txt", "my new file\r\n"); - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; - g_opts.skipped_notify_cb = dont_notify_cb; - g_opts.notify_payload = NULL; + g_opts.checkout_strategy |= GIT_CHECKOUT_ALLOW_CONFLICTS; + g_opts.conflict_cb = dont_conflict_cb; + g_opts.conflict_payload = NULL; cl_git_pass(git_checkout_index(g_repo, &g_opts)); } @@ -389,8 +397,9 @@ void test_checkout_index__can_overcome_name_clashes(void) cl_git_pass(p_mkdir("./testrepo/path1", 0777)); cl_git_mkfile("./testrepo/path1/file1", "content\r\n"); - cl_git_pass(git_index_add(index, "path0", 0)); - cl_git_pass(git_index_add(index, "path1/file1", 0)); + cl_git_pass(git_index_add_from_workdir(index, "path0")); + cl_git_pass(git_index_add_from_workdir(index, "path1/file1")); + cl_git_pass(p_unlink("./testrepo/path0")); cl_git_pass(git_futils_rmdir_r( @@ -400,11 +409,20 @@ void test_checkout_index__can_overcome_name_clashes(void) cl_git_pass(p_mkdir("./testrepo/path0", 0777)); cl_git_mkfile("./testrepo/path0/file0", "content\r\n"); - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + cl_assert(git_path_isfile("./testrepo/path1")); + cl_assert(git_path_isfile("./testrepo/path0/file0")); + + g_opts.checkout_strategy = GIT_CHECKOUT_SAFE | GIT_CHECKOUT_ALLOW_CONFLICTS; cl_git_pass(git_checkout_index(g_repo, &g_opts)); cl_assert(git_path_isfile("./testrepo/path1")); cl_assert(git_path_isfile("./testrepo/path0/file0")); + g_opts.checkout_strategy = GIT_CHECKOUT_FORCE; + cl_git_pass(git_checkout_index(g_repo, &g_opts)); + + cl_assert(git_path_isfile("./testrepo/path0")); + cl_assert(git_path_isfile("./testrepo/path1/file1")); + git_index_free(index); } diff --git a/tests-clar/checkout/tree.c b/tests-clar/checkout/tree.c index c42aefc31..983425324 100644 --- a/tests-clar/checkout/tree.c +++ b/tests-clar/checkout/tree.c @@ -12,7 +12,7 @@ void test_checkout_tree__initialize(void) g_repo = cl_git_sandbox_init("testrepo"); memset(&g_opts, 0, sizeof(g_opts)); - g_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + g_opts.checkout_strategy = GIT_CHECKOUT_SAFE; } void test_checkout_tree__cleanup(void) @@ -74,10 +74,13 @@ static void progress(const char *path, size_t cur, size_t tot, void *payload) void test_checkout_tree__calls_progress_callback(void) { bool was_called = 0; + g_opts.progress_cb = progress; g_opts.progress_payload = &was_called; cl_git_pass(git_revparse_single(&g_object, g_repo, "master")); + cl_git_pass(git_checkout_tree(g_repo, g_object, &g_opts)); + cl_assert_equal_i(was_called, true); } diff --git a/tests-clar/checkout/typechange.c b/tests-clar/checkout/typechange.c index e86af52ee..cd34885de 100644 --- a/tests-clar/checkout/typechange.c +++ b/tests-clar/checkout/typechange.c @@ -40,10 +40,12 @@ void test_checkout_typechange__checkout_typechanges(void) git_object *obj; git_checkout_opts opts = {0}; - opts.checkout_strategy = - GIT_CHECKOUT_REMOVE_UNTRACKED | - GIT_CHECKOUT_CREATE_MISSING | - GIT_CHECKOUT_OVERWRITE_MODIFIED; + opts.checkout_strategy = GIT_CHECKOUT_FORCE; + + /* if you don't include GIT_CHECKOUT_REMOVE_UNTRACKED then on the final + * checkout which is supposed to remove all the files, we will not + * actually remove them! + */ for (i = 0; g_typechange_oids[i] != NULL; ++i) { cl_git_pass(git_revparse_single(&obj, g_repo, g_typechange_oids[i])); @@ -51,6 +53,9 @@ void test_checkout_typechange__checkout_typechanges(void) cl_git_pass(git_checkout_tree(g_repo, obj, &opts)); + cl_git_pass( + git_repository_set_head_detached(g_repo, git_object_id(obj))); + git_object_free(obj); if (!g_typechange_empty[i]) { diff --git a/tests-clar/clone/network.c b/tests-clar/clone/network.c index 19385f77a..68fa8eb6b 100644 --- a/tests-clar/clone/network.c +++ b/tests-clar/clone/network.c @@ -113,7 +113,7 @@ void test_clone_network__can_checkout_a_cloned_repo(void) bool checkout_progress_cb_was_called = false, fetch_progress_cb_was_called = false; - opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + opts.checkout_strategy = GIT_CHECKOUT_UPDATE_UNMODIFIED; opts.progress_cb = &checkout_progress; opts.progress_payload = &checkout_progress_cb_was_called; From a1bf70e4c94cea054fe5812344e6e70f13402be6 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 8 Nov 2012 21:58:24 -0800 Subject: [PATCH 08/11] fix regression in diff with submodule oid --- src/diff.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/diff.c b/src/diff.c index 015c77edd..ea19d4799 100644 --- a/src/diff.c +++ b/src/diff.c @@ -506,16 +506,15 @@ static int maybe_modified( /* if we got here and decided that the files are modified, but we * haven't calculated the OID of the new item, then calculate it now */ - if (status != GIT_DELTA_UNMODIFIED && - git_oid_iszero(&nitem->oid) && !use_noid) - { - if (git_diff__oid_for_file(diff->repo, - nitem->path, nitem->mode, nitem->file_size, &noid) < 0) - return -1; - if (omode == nmode && git_oid_equal(&oitem->oid, &noid)) + if (status != GIT_DELTA_UNMODIFIED && git_oid_iszero(&nitem->oid)) { + if (!use_noid) { + if (git_diff__oid_for_file(diff->repo, + nitem->path, nitem->mode, nitem->file_size, &noid) < 0) + return -1; + use_noid = &noid; + } + if (omode == nmode && git_oid_equal(&oitem->oid, use_noid)) status = GIT_DELTA_UNMODIFIED; - /* store calculated oid so we don't have to recalc later */ - use_noid = &noid; } return diff_delta__from_two( From 8064ecba2384df32d5b821398a346f0c96cf74d1 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 9 Nov 2012 10:48:25 -0800 Subject: [PATCH 09/11] Update reset hard test The `git_reset` API with the HARD option is still slightly broken, but this test now does exercise the ability of the command to revert modified files. --- tests-clar/reset/hard.c | 58 +++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/tests-clar/reset/hard.c b/tests-clar/reset/hard.c index c3f041817..4b49ca362 100644 --- a/tests-clar/reset/hard.c +++ b/tests-clar/reset/hard.c @@ -19,30 +19,56 @@ void test_reset_hard__cleanup(void) cl_git_sandbox_cleanup(); } -void test_reset_hard__resetting_culls_empty_directories(void) +void test_reset_hard__resetting_reverts_modified_files(void) { - git_buf subdir_path = GIT_BUF_INIT; - git_buf subfile_path = GIT_BUF_INIT; - git_buf newdir_path = GIT_BUF_INIT; + git_buf path = GIT_BUF_INIT, content = GIT_BUF_INIT; + int i; + static const char *files[4] = { + "current_file", + "modified_file", + "staged_new_file", + "staged_changes_modified_file" }; + static const char *before[4] = { + "current_file\n", + "modified_file\nmodified_file\n", + "staged_new_file\n", + "staged_changes_modified_file\nstaged_changes_modified_file\nstaged_changes_modified_file\n" + }; + static const char *after[4] = { + "current_file\n", + "modified_file\n", + /* wrong value because reset is still slightly incorrect */ + "staged_new_file\n", + /* right value: NULL, */ + "staged_changes_modified_file\n" + }; + const char *wd = git_repository_workdir(repo); - cl_git_pass(git_buf_joinpath(&newdir_path, git_repository_workdir(repo), "newdir/")); + cl_assert(wd); - cl_git_pass(git_buf_joinpath(&subfile_path, git_buf_cstr(&newdir_path), "with/nested/file.txt")); - cl_git_pass(git_futils_mkpath2file(git_buf_cstr(&subfile_path), 0755)); - cl_git_mkfile(git_buf_cstr(&subfile_path), "all anew...\n"); + for (i = 0; i < 4; ++i) { + cl_git_pass(git_buf_joinpath(&path, wd, files[i])); + cl_git_pass(git_futils_readbuffer(&content, path.ptr)); + cl_assert_equal_s(before[i], content.ptr); + } - cl_git_pass(git_buf_joinpath(&subdir_path, git_repository_workdir(repo), "subdir/")); - cl_assert(git_path_isdir(git_buf_cstr(&subdir_path)) == true); + retrieve_target_from_oid( + &target, repo, "26a125ee1bfc5df1e1b2e9441bbe63c8a7ae989f"); - retrieve_target_from_oid(&target, repo, "0017bd4ab1ec30440b17bae1680cff124ab5f1f6"); cl_git_pass(git_reset(repo, target, GIT_RESET_HARD)); - cl_assert(git_path_isdir(git_buf_cstr(&subdir_path)) == false); - cl_assert(git_path_isdir(git_buf_cstr(&newdir_path)) == false); + for (i = 0; i < 4; ++i) { + cl_git_pass(git_buf_joinpath(&path, wd, files[i])); + if (after[i]) { + cl_git_pass(git_futils_readbuffer(&content, path.ptr)); + cl_assert_equal_s(after[i], content.ptr); + } else { + cl_assert(!git_path_exists(path.ptr)); + } + } - git_buf_free(&subdir_path); - git_buf_free(&subfile_path); - git_buf_free(&newdir_path); + git_buf_free(&content); + git_buf_free(&path); } void test_reset_hard__cannot_reset_in_a_bare_repository(void) From 0f3def715dc9af442f5f025c50a041c6319df1e8 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 9 Nov 2012 11:19:46 -0800 Subject: [PATCH 10/11] Fix various cross-platform build issues This fixes a number of warnings and problems with cross-platform builds. Among other things, it's not safe to name a member of a structure "strcmp" because that may be #defined. --- examples/network/Makefile | 5 +++-- examples/network/clone.c | 24 +++++++++++++++--------- examples/network/common.h | 9 +++++++++ examples/network/fetch.c | 4 ++-- include/git2/checkout.h | 14 +++++++++----- src/checkout.c | 6 +++--- src/diff.c | 30 +++++++++++++++--------------- src/diff.h | 8 ++++---- src/diff_output.c | 4 ++-- 9 files changed, 62 insertions(+), 42 deletions(-) diff --git a/examples/network/Makefile b/examples/network/Makefile index 835be24cc..ef3cec659 100644 --- a/examples/network/Makefile +++ b/examples/network/Makefile @@ -2,7 +2,8 @@ default: all CC = gcc CFLAGS += -g -CFLAGS += -I../../include -L../../build -L../.. -lgit2 -lpthread +CFLAGS += -I../../include +LDFLAGS += -L../../build -L../.. -lgit2 -lpthread OBJECTS = \ git2.o \ @@ -12,4 +13,4 @@ OBJECTS = \ index-pack.o all: $(OBJECTS) - $(CC) $(CFLAGS) -o git2 $(OBJECTS) + $(CC) $(CFLAGS) $(LDFLAGS) -o git2 $(OBJECTS) diff --git a/examples/network/clone.c b/examples/network/clone.c index 791600171..30a4944c2 100644 --- a/examples/network/clone.c +++ b/examples/network/clone.c @@ -22,12 +22,14 @@ static void print_progress(const progress_data *pd) ? (100 * pd->completed_steps) / pd->total_steps : 0.f; int kbytes = pd->fetch_progress.received_bytes / 1024; - printf("net %3d%% (%4d kb, %5d/%5d) / idx %3d%% (%5d/%5d) / chk %3d%% (%4lu/%4lu) %s\n", - network_percent, kbytes, - pd->fetch_progress.received_objects, pd->fetch_progress.total_objects, - index_percent, pd->fetch_progress.indexed_objects, pd->fetch_progress.total_objects, - checkout_percent, pd->completed_steps, pd->total_steps, - pd->path); + + printf("net %3d%% (%4d kb, %5d/%5d) / idx %3d%% (%5d/%5d) / chk %3d%% (%4" PRIuZ "/%4" PRIuZ ") %s\n", + network_percent, kbytes, + pd->fetch_progress.received_objects, pd->fetch_progress.total_objects, + index_percent, pd->fetch_progress.indexed_objects, pd->fetch_progress.total_objects, + checkout_percent, + pd->completed_steps, pd->total_steps, + pd->path); } static void fetch_progress(const git_transfer_progress *stats, void *payload) @@ -47,13 +49,15 @@ static void checkout_progress(const char *path, size_t cur, size_t tot, void *pa int do_clone(git_repository *repo, int argc, char **argv) { - progress_data pd = {0}; + progress_data pd; git_repository *cloned_repo = NULL; - git_checkout_opts checkout_opts = {0}; + git_checkout_opts checkout_opts; const char *url = argv[1]; const char *path = argv[2]; int error; + (void)repo; // unused + // Validate args if (argc < 3) { printf ("USAGE: %s \n", argv[0]); @@ -61,8 +65,10 @@ int do_clone(git_repository *repo, int argc, char **argv) } // Set up options - checkout_opts.checkout_strategy = GIT_CHECKOUT_CREATE_MISSING; + memset(&checkout_opts, 0, sizeof(checkout_opts)); + checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE; checkout_opts.progress_cb = checkout_progress; + memset(&pd, 0, sizeof(pd)); checkout_opts.progress_payload = &pd; // Do the clone diff --git a/examples/network/common.h b/examples/network/common.h index c82eaa1c8..a4cfa1a7e 100644 --- a/examples/network/common.h +++ b/examples/network/common.h @@ -12,4 +12,13 @@ int fetch(git_repository *repo, int argc, char **argv); int index_pack(git_repository *repo, int argc, char **argv); int do_clone(git_repository *repo, int argc, char **argv); +#ifndef PRIuZ +/* Define the printf format specifer to use for size_t output */ +#if defined(_MSC_VER) || defined(__MINGW32__) +# define PRIuZ "Iu" +#else +# define PRIuZ "zu" +#endif +#endif + #endif /* __COMMON_H__ */ diff --git a/examples/network/fetch.c b/examples/network/fetch.c index 496498e8c..9d1404ab4 100644 --- a/examples/network/fetch.c +++ b/examples/network/fetch.c @@ -103,9 +103,9 @@ int fetch(git_repository *repo, int argc, char **argv) usleep(10000); if (stats->total_objects > 0) - printf("Received %d/%d objects (%d) in %d bytes\r", + printf("Received %d/%d objects (%d) in %" PRIuZ " bytes\r", stats->received_objects, stats->total_objects, - stats->indexed_objects, stats->received_bytes); + stats->indexed_objects, stats->received_bytes); } while (!data.finished); if (data.ret < 0) diff --git a/include/git2/checkout.h b/include/git2/checkout.h index a9314c2cb..d444450f3 100644 --- a/include/git2/checkout.h +++ b/include/git2/checkout.h @@ -124,16 +124,20 @@ typedef enum { /** Only update existing files, don't create new ones */ GIT_CHECKOUT_UPDATE_ONLY = (1u << 6), - /** Allow checkout to skip unmerged files */ + /** + * THE FOLLOWING OPTIONS ARE NOT YET IMPLEMENTED + */ + + /** Allow checkout to skip unmerged files (NOT IMPLEMENTED) */ GIT_CHECKOUT_SKIP_UNMERGED = (1u << 10), - /** For unmerged files, checkout stage 2 from index */ + /** For unmerged files, checkout stage 2 from index (NOT IMPLEMENTED) */ GIT_CHECKOUT_USE_OURS = (1u << 11), - /** For unmerged files, checkout stage 3 from index */ + /** For unmerged files, checkout stage 3 from index (NOT IMPLEMENTED) */ GIT_CHECKOUT_USE_THEIRS = (1u << 12), - /** Recursively checkout submodule with same options */ + /** Recursively checkout submodules with same options (NOT IMPLEMENTED) */ GIT_CHECKOUT_UPDATE_SUBMODULES = (1u << 16), - /** Recursively checkout submodules only if HEAD moved in super repo */ + /** Recursively checkout submodules if HEAD moved in super repo (NOT IMPLEMENTED) */ GIT_CHECKOUT_UPDATE_SUBMODULES_IF_CHANGED = (1u << 17), } git_checkout_strategy_t; diff --git a/src/checkout.c b/src/checkout.c index 2bad06501..8d164cfca 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -449,7 +449,7 @@ static int checkout_get_actions( if ((diff->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0 && !hiter->ignore_case && (error = git_iterator_spoolandsort( - &hiter, hiter, diff->entrycmp, true)) < 0) + &hiter, hiter, diff->entrycomp, true)) < 0) goto fail; if ((error = git_iterator_current(hiter, &he)) < 0) @@ -470,8 +470,8 @@ static int checkout_get_actions( /* try to track HEAD entries parallel to deltas */ while (he) { cmp = S_ISDIR(delta->new_file.mode) ? - diff->prefixcmp(he->path, delta->new_file.path) : - diff->strcmp(he->path, delta->old_file.path); + diff->pfxcomp(he->path, delta->new_file.path) : + diff->strcomp(he->path, delta->old_file.path); if (cmp >= 0) break; if (git_iterator_advance(hiter, &he) < 0) diff --git a/src/diff.c b/src/diff.c index ea19d4799..6f48d72a2 100644 --- a/src/diff.c +++ b/src/diff.c @@ -528,7 +528,7 @@ static bool entry_is_prefixed( { size_t pathlen; - if (!prefix_item || diff->prefixcmp(prefix_item->path, item->path)) + if (!prefix_item || diff->pfxcomp(prefix_item->path, item->path)) return false; pathlen = strlen(item->path); @@ -551,17 +551,17 @@ static int diff_list_init_from_iterators( if (!old_iter->ignore_case && !new_iter->ignore_case) { diff->opts.flags &= ~GIT_DIFF_DELTAS_ARE_ICASE; - diff->strcmp = strcmp; - diff->strncmp = strncmp; - diff->prefixcmp = git__prefixcmp; - diff->entrycmp = git_index_entry__cmp; + diff->strcomp = strcmp; + diff->strncomp = strncmp; + diff->pfxcomp = git__prefixcmp; + diff->entrycomp = git_index_entry__cmp; } else { diff->opts.flags |= GIT_DIFF_DELTAS_ARE_ICASE; - diff->strcmp = strcasecmp; - diff->strncmp = strncasecmp; - diff->prefixcmp = git__prefixcmp_icase; - diff->entrycmp = git_index_entry__cmp_icase; + diff->strcomp = strcasecmp; + diff->strncomp = strncasecmp; + diff->pfxcomp = git__prefixcmp_icase; + diff->entrycomp = git_index_entry__cmp_icase; } return 0; @@ -592,12 +592,12 @@ static int diff_from_iterators( * merge join to the other iterator that is icase sorted */ if (!old_iter->ignore_case && git_iterator_spoolandsort( - &old_iter, old_iter, diff->entrycmp, true) < 0) + &old_iter, old_iter, diff->entrycomp, true) < 0) goto fail; if (!new_iter->ignore_case && git_iterator_spoolandsort( - &new_iter, new_iter, diff->entrycmp, true) < 0) + &new_iter, new_iter, diff->entrycomp, true) < 0) goto fail; } @@ -609,7 +609,7 @@ static int diff_from_iterators( while (oitem || nitem) { /* create DELETED records for old items not matched in new */ - if (oitem && (!nitem || diff->entrycmp(oitem, nitem) < 0)) { + if (oitem && (!nitem || diff->entrycomp(oitem, nitem) < 0)) { if (diff_delta__from_one(diff, GIT_DELTA_DELETED, oitem) < 0) goto fail; @@ -634,12 +634,12 @@ static int diff_from_iterators( /* create ADDED, TRACKED, or IGNORED records for new items not * matched in old (and/or descend into directories as needed) */ - else if (nitem && (!oitem || diff->entrycmp(oitem, nitem) > 0)) { + else if (nitem && (!oitem || diff->entrycomp(oitem, nitem) > 0)) { git_delta_t delta_type = GIT_DELTA_UNTRACKED; /* check if contained in ignored parent directory */ if (git_buf_len(&ignore_prefix) && - diff->prefixcmp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0) + diff->pfxcomp(nitem->path, git_buf_cstr(&ignore_prefix)) == 0) delta_type = GIT_DELTA_IGNORED; if (S_ISDIR(nitem->mode)) { @@ -730,7 +730,7 @@ static int diff_from_iterators( * (or ADDED and DELETED pair if type changed) */ else { - assert(oitem && nitem && diff->entrycmp(oitem, nitem) == 0); + assert(oitem && nitem && diff->entrycomp(oitem, nitem) == 0); if (maybe_modified(old_iter, oitem, new_iter, nitem, diff) < 0 || git_iterator_advance(old_iter, &oitem) < 0 || diff --git a/src/diff.h b/src/diff.h index e9d8fd5a7..1e3be7593 100644 --- a/src/diff.h +++ b/src/diff.h @@ -42,10 +42,10 @@ struct git_diff_list { git_iterator_type_t new_src; uint32_t diffcaps; - int (*strcmp)(const char *, const char *); - int (*strncmp)(const char *, const char *, size_t); - int (*prefixcmp)(const char *str, const char *pfx); - int (*entrycmp)(const void *a, const void *b); + int (*strcomp)(const char *, const char *); + int (*strncomp)(const char *, const char *, size_t); + int (*pfxcomp)(const char *str, const char *pfx); + int (*entrycomp)(const void *a, const void *b); }; extern void git_diff__cleanup_modes( diff --git a/src/diff_output.c b/src/diff_output.c index 2f61540ff..46a9e02bf 100644 --- a/src/diff_output.c +++ b/src/diff_output.c @@ -120,7 +120,7 @@ static int diff_delta_is_binary_by_attr( return -1; mirror_new = (delta->new_file.path == delta->old_file.path || - strcmp(delta->new_file.path, delta->old_file.path) == 0); + ctxt->diff->strcomp(delta->new_file.path, delta->old_file.path) == 0); if (mirror_new) delta->new_file.flags |= (delta->old_file.flags & KNOWN_BINARY_FLAGS); else @@ -1002,7 +1002,7 @@ static int print_compact( git_buf_clear(pi->buf); if (delta->old_file.path != delta->new_file.path && - strcmp(delta->old_file.path,delta->new_file.path) != 0) + pi->diff->strcomp(delta->old_file.path,delta->new_file.path) != 0) git_buf_printf(pi->buf, "%c\t%s%c -> %s%c\n", code, delta->old_file.path, old_suffix, delta->new_file.path, new_suffix); else if (delta->old_file.mode != delta->new_file.mode && From 757b406504021b3a73e52ce9f95d590d65c7dce5 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 9 Nov 2012 14:01:44 -0800 Subject: [PATCH 11/11] Fix warnings and valgrind issues This fixes some various warnings that showed up in Travis and a couple uses of uninitialized memory and one memory leak. --- src/checkout.c | 8 ++++++-- src/index.c | 2 +- src/pack-objects.c | 12 ++++++------ src/transports/http.c | 2 ++ tests-clar/clone/network.c | 2 +- tests-clar/config/configlevel.c | 1 - tests-clar/stash/stash_helpers.c | 1 + 7 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 8d164cfca..0d14e2625 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -432,8 +432,8 @@ static int checkout_get_actions( int error; git_diff_list *diff = data->diff; git_diff_delta *delta; - size_t i, *counts; - uint32_t *actions; + size_t i, *counts = NULL; + uint32_t *actions = NULL; git_tree *head = NULL; git_iterator *hiter = NULL; char *pfx = git_pathspec_prefix(&data->opts->paths); @@ -456,6 +456,7 @@ static int checkout_get_actions( goto fail; git__free(pfx); + pfx = NULL; *counts_ptr = counts = git__calloc(CHECKOUT_ACTION__MAX+1, sizeof(size_t)); *actions_ptr = actions = git__calloc(diff->deltas.length, sizeof(uint32_t)); @@ -509,6 +510,8 @@ static int checkout_get_actions( } git_iterator_free(hiter); + git_tree_free(head); + return 0; fail: @@ -518,6 +521,7 @@ fail: git__free(actions); git_iterator_free(hiter); + git_tree_free(head); git__free(pfx); return -1; diff --git a/src/index.c b/src/index.c index 0ae1b4479..43244494a 100644 --- a/src/index.c +++ b/src/index.c @@ -402,7 +402,7 @@ int git_index_read(git_index *index) { int error = 0, updated; git_buf buffer = GIT_BUF_INIT; - git_futils_filestamp stamp; + git_futils_filestamp stamp = {0}; if (!index->index_file_path) { giterr_set(GITERR_INDEX, diff --git a/src/pack-objects.c b/src/pack-objects.c index 7acc93328..af7472aff 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -73,16 +73,16 @@ static int packbuilder_config(git_packbuilder *pb) { git_config *config; int ret; + int64_t val; if (git_repository_config__weakptr(&config, pb->repo) < 0) return -1; -#define config_get(key, dst, default) \ - ret = git_config_get_int64((int64_t *)&dst, config, key); \ - if (ret == GIT_ENOTFOUND) \ - dst = default; \ - else if (ret < 0) \ - return -1; +#define config_get(KEY,DST,DFLT) do { \ + ret = git_config_get_int64(&val, config, KEY); \ + if (!ret) (DST) = val; \ + else if (ret == GIT_ENOTFOUND) (DST) = (DFLT); \ + else if (ret < 0) return -1; } while (0) config_get("pack.deltaCacheSize", pb->max_delta_cache_size, GIT_PACK_DELTA_CACHE_SIZE); diff --git a/src/transports/http.c b/src/transports/http.c index 78977f44a..34ade947a 100644 --- a/src/transports/http.c +++ b/src/transports/http.c @@ -629,6 +629,8 @@ int git_smart_subtransport_http(git_smart_subtransport **out, http_subtransport *t; int flags; + (void)flags; + if (!out) return -1; diff --git a/tests-clar/clone/network.c b/tests-clar/clone/network.c index 68fa8eb6b..1304f7728 100644 --- a/tests-clar/clone/network.c +++ b/tests-clar/clone/network.c @@ -113,7 +113,7 @@ void test_clone_network__can_checkout_a_cloned_repo(void) bool checkout_progress_cb_was_called = false, fetch_progress_cb_was_called = false; - opts.checkout_strategy = GIT_CHECKOUT_UPDATE_UNMODIFIED; + opts.checkout_strategy = GIT_CHECKOUT_SAFE; opts.progress_cb = &checkout_progress; opts.progress_payload = &checkout_progress_cb_was_called; diff --git a/tests-clar/config/configlevel.c b/tests-clar/config/configlevel.c index 69aede6d3..1c22e8d9f 100644 --- a/tests-clar/config/configlevel.c +++ b/tests-clar/config/configlevel.c @@ -62,7 +62,6 @@ void test_config_configlevel__fetching_a_level_from_an_empty_compound_config_ret { git_config *cfg; git_config *local_cfg; - const char *s; cl_git_pass(git_config_new(&cfg)); diff --git a/tests-clar/stash/stash_helpers.c b/tests-clar/stash/stash_helpers.c index 000a0f1a9..86a741853 100644 --- a/tests-clar/stash/stash_helpers.c +++ b/tests-clar/stash/stash_helpers.c @@ -16,6 +16,7 @@ void commit_staged_files( cl_git_pass(git_index_write_tree(&tree_oid, index)); cl_git_pass(git_tree_lookup(&tree, repo, &tree_oid)); + cl_git_pass(git_commit_create_v( commit_oid, repo,