From 6b7991e264b2fb0448e3dc47f972bafabf38c1fa Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 30 Sep 2013 16:13:53 -0700 Subject: [PATCH 01/14] Add check if we need to precompose unicode on Mac This adds initialization of core.precomposeunicode to repo init on Mac. This is necessary because when a Mac accesses a repo on a VFAT or SAMBA file system, it will return directory entries in decomposed unicode even if the filesystem entry is precomposed. This also removes caching of a number of repo properties from the repo init pipeline because these are properties of the specific filesystem on which the repo is created, not of the system as a whole. --- src/repository.c | 101 ++++++++++++++++++++++++++++------------- tests-clar/repo/init.c | 10 ++++ 2 files changed, 79 insertions(+), 32 deletions(-) diff --git a/src/repository.c b/src/repository.c index 0d7c09484..64f13978d 100644 --- a/src/repository.c +++ b/src/repository.c @@ -843,10 +843,6 @@ fail: static bool is_chmod_supported(const char *file_path) { struct stat st1, st2; - static int _is_supported = -1; - - if (_is_supported > -1) - return _is_supported; if (p_stat(file_path, &st1) < 0) return false; @@ -857,27 +853,19 @@ static bool is_chmod_supported(const char *file_path) if (p_stat(file_path, &st2) < 0) return false; - _is_supported = (st1.st_mode != st2.st_mode); - - return _is_supported; + return (st1.st_mode != st2.st_mode); } static bool is_filesystem_case_insensitive(const char *gitdir_path) { git_buf path = GIT_BUF_INIT; - static int _is_insensitive = -1; + int is_insensitive = -1; - if (_is_insensitive > -1) - return _is_insensitive; + if (!git_buf_joinpath(&path, gitdir_path, "CoNfIg")) + is_insensitive = git_path_exists(git_buf_cstr(&path)); - if (git_buf_joinpath(&path, gitdir_path, "CoNfIg") < 0) - goto cleanup; - - _is_insensitive = git_path_exists(git_buf_cstr(&path)); - -cleanup: git_buf_free(&path); - return _is_insensitive; + return is_insensitive; } static bool are_symlinks_supported(const char *wd_path) @@ -885,24 +873,69 @@ static bool are_symlinks_supported(const char *wd_path) git_buf path = GIT_BUF_INIT; int fd; struct stat st; - static int _symlinks_supported = -1; - - if (_symlinks_supported > -1) - return _symlinks_supported; + int symlinks_supported = -1; if ((fd = git_futils_mktmp(&path, wd_path)) < 0 || p_close(fd) < 0 || p_unlink(path.ptr) < 0 || p_symlink("testing", path.ptr) < 0 || p_lstat(path.ptr, &st) < 0) - _symlinks_supported = false; + symlinks_supported = false; else - _symlinks_supported = (S_ISLNK(st.st_mode) != 0); + symlinks_supported = (S_ISLNK(st.st_mode) != 0); (void)p_unlink(path.ptr); git_buf_free(&path); - return _symlinks_supported; + return symlinks_supported; +} + +static const char *nfc_file = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D.XXXXXX"; +static const char *nfd_file = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D.XXXXXX"; + +/* On Mac, HDFS always stores files using decomposed unicode, but when + * writing to VFAT or SAMBA file systems, filenames may be kept as + * precomposed unicode, but will be converted to decomposed form when + * reading the directory entries. This can cause file name mismatches. + * The solution is to convert directory entries to precomposed form if we + * cannot look up the file from the decomposed path. + */ +static bool should_precompose_unicode_paths(const char *wd_path) +{ + git_buf path = GIT_BUF_INIT; + int fd; + bool need_precompose = false; + char tmp[6]; + + /* Create a file using a precomposed path and then try to find it + * using the decomposed name. If the lookup fails, then we will mark + * that we should precompose unicode for this repository. + */ + if (git_buf_joinpath(&path, wd_path, nfc_file) < 0 || + (fd = p_mkstemp(path.ptr)) < 0) + goto fail; + p_close(fd); + + /* record trailing digits generated by mkstemp */ + memcpy(tmp, path.ptr + path.size - sizeof(tmp), sizeof(tmp)); + + /* try to look up as NFD path */ + if (git_buf_joinpath(&path, wd_path, nfd_file) < 0) + goto fail; + memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp)); + + need_precompose = !git_path_exists(path.ptr); + + /* remove temporary file */ + if (git_buf_joinpath(&path, wd_path, nfc_file) < 0) + goto fail; + memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp)); + + (void)p_unlink(path.ptr); + +fail: + git_buf_free(&path); + return need_precompose; } static int create_empty_file(const char *path, mode_t mode) @@ -930,6 +963,7 @@ static int repo_init_config( int error = 0; git_buf cfg_path = GIT_BUF_INIT; git_config *config = NULL; + bool is_bare = ((opts->flags & GIT_REPOSITORY_INIT_BARE) != 0); #define SET_REPO_CONFIG(TYPE, NAME, VAL) do {\ if ((error = git_config_set_##TYPE(config, NAME, VAL)) < 0) \ @@ -954,17 +988,23 @@ static int repo_init_config( goto cleanup; SET_REPO_CONFIG( - bool, "core.bare", (opts->flags & GIT_REPOSITORY_INIT_BARE) != 0); + bool, "core.bare", is_bare); SET_REPO_CONFIG( int32, "core.repositoryformatversion", GIT_REPO_VERSION); SET_REPO_CONFIG( bool, "core.filemode", is_chmod_supported(git_buf_cstr(&cfg_path))); - if (!(opts->flags & GIT_REPOSITORY_INIT_BARE)) { - SET_REPO_CONFIG(bool, "core.logallrefupdates", true); +#if __APPLE__ + SET_REPO_CONFIG( + bool, "core.precomposeunicode", + should_precompose_unicode_paths(is_bare ? repo_dir : work_dir)); +#endif - if (!are_symlinks_supported(work_dir)) - SET_REPO_CONFIG(bool, "core.symlinks", false); + if (!are_symlinks_supported(is_bare ? repo_dir : work_dir)) + SET_REPO_CONFIG(bool, "core.symlinks", false); + + if (!is_bare) { + SET_REPO_CONFIG(bool, "core.logallrefupdates", true); if (!(opts->flags & GIT_REPOSITORY_INIT__NATURAL_WD)) { SET_REPO_CONFIG(string, "core.worktree", work_dir); @@ -973,9 +1013,6 @@ static int repo_init_config( if (git_config_delete_entry(config, "core.worktree") < 0) giterr_clear(); } - } else { - if (!are_symlinks_supported(repo_dir)) - SET_REPO_CONFIG(bool, "core.symlinks", false); } if (!(opts->flags & GIT_REPOSITORY_INIT__IS_REINIT) && diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 392be205b..6f3c84175 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -241,6 +241,16 @@ void test_repo_init__detect_ignorecase(void) #endif } +void test_repo_init__detect_precompose_unicode_required(void) +{ +#ifdef __APPLE__ + /* hard to test "true" case without SAMBA or VFAT file system available */ + assert_config_entry_on_init("core.precomposeunicode", false); +#else + assert_config_entry_on_init("core.precomposeunicode", GIT_ENOTFOUND); +#endif +} + void test_repo_init__reinit_doesnot_overwrite_ignorecase(void) { git_config *config; From 2fe54afa2a8f87d03d2d550dcde7718f27e40967 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Mon, 30 Sep 2013 16:58:33 -0700 Subject: [PATCH 02/14] Put hooks in place for precompose in dirload fn This doesn't actual do string precompose but it puts the hooks in place into the iterators and the git_path_dirload function so that the actual precompose work is ready to go. --- src/config_cache.c | 1 + src/iterator.c | 14 +++++++++++++- src/iterator.h | 10 ++++++---- src/path.c | 14 +++++++++++--- src/path.h | 10 ++++++++-- src/refdb_fs.c | 10 ++++++++-- src/repository.h | 3 +++ 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/config_cache.c b/src/config_cache.c index 84de3a5ed..6808521a3 100644 --- a/src/config_cache.c +++ b/src/config_cache.c @@ -67,6 +67,7 @@ static struct map_data _cvar_maps[] = { {"core.ignorestat", NULL, 0, GIT_IGNORESTAT_DEFAULT }, {"core.trustctime", NULL, 0, GIT_TRUSTCTIME_DEFAULT }, {"core.abbrev", _cvar_map_int, 1, GIT_ABBREV_DEFAULT }, + {"core.precomposeunicode", NULL, 0, GIT_PRECOMPOSE_DEFAULT }, }; int git_repository__cvar(int *out, git_repository *repo, git_cvar_cached cvar) diff --git a/src/iterator.c b/src/iterator.c index bdc98d22b..946790449 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -986,7 +986,10 @@ static int fs_iterator__expand_dir(fs_iterator *fi) GITERR_CHECK_ALLOC(ff); error = git_path_dirload_with_stat( - fi->path.ptr, fi->root_len, iterator__ignore_case(fi), + fi->path.ptr, fi->root_len, + (iterator__ignore_case(fi) ? GIT_PATH_DIRLOAD_IGNORE_CASE : 0) | + (iterator__flag(fi, PRECOMPOSE_UNICODE) ? + GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE : 0), fi->base.start, fi->base.end, &ff->entries); if (error < 0) { @@ -1356,6 +1359,15 @@ int git_iterator_for_workdir_ext( return error; } + /* try to look up precompose and set flag if appropriate */ + { + int precompose = 0; + if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0) + giterr_clear(); + else if (precompose) + wi->fi.base.flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; + } + return fs_iterator__initialize(out, &wi->fi, repo_workdir); } diff --git a/src/iterator.h b/src/iterator.h index ea88fa6a2..751e139d0 100644 --- a/src/iterator.h +++ b/src/iterator.h @@ -24,13 +24,15 @@ typedef enum { typedef enum { /** ignore case for entry sort order */ - GIT_ITERATOR_IGNORE_CASE = (1 << 0), + GIT_ITERATOR_IGNORE_CASE = (1u << 0), /** force case sensitivity for entry sort order */ - GIT_ITERATOR_DONT_IGNORE_CASE = (1 << 1), + GIT_ITERATOR_DONT_IGNORE_CASE = (1u << 1), /** return tree items in addition to blob items */ - GIT_ITERATOR_INCLUDE_TREES = (1 << 2), + GIT_ITERATOR_INCLUDE_TREES = (1u << 2), /** don't flatten trees, requiring advance_into (implies INCLUDE_TREES) */ - GIT_ITERATOR_DONT_AUTOEXPAND = (1 << 3), + GIT_ITERATOR_DONT_AUTOEXPAND = (1u << 3), + /** convert precomposed unicode to decomposed unicode */ + GIT_ITERATOR_PRECOMPOSE_UNICODE = (1u << 4), } git_iterator_flag_t; typedef struct { diff --git a/src/path.c b/src/path.c index 9d8e90361..c4ab57136 100644 --- a/src/path.c +++ b/src/path.c @@ -781,6 +781,7 @@ int git_path_dirload( const char *path, size_t prefix_len, size_t alloc_extra, + unsigned int flags, git_vector *contents) { int error, need_slash; @@ -816,6 +817,12 @@ int git_path_dirload( entry_len = strlen(de->d_name); + /* if we read decomposed unicode and precompose flag is set, + * then precompose it now so app code sees it as precomposed + */ + if ((flags & GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE) != 0) { + } + entry_path = git__malloc( path_len + need_slash + entry_len + 1 + alloc_extra); GITERR_CHECK_ALLOC(entry_path); @@ -858,7 +865,7 @@ int git_path_with_stat_cmp_icase(const void *a, const void *b) int git_path_dirload_with_stat( const char *path, size_t prefix_len, - bool ignore_case, + unsigned int flags, const char *start_stat, const char *end_stat, git_vector *contents) @@ -875,13 +882,14 @@ int git_path_dirload_with_stat( return -1; error = git_path_dirload( - path, prefix_len, sizeof(git_path_with_stat) + 1, contents); + path, prefix_len, sizeof(git_path_with_stat) + 1, flags, contents); if (error < 0) { git_buf_free(&full); return error; } - strncomp = ignore_case ? git__strncasecmp : git__strncmp; + strncomp = (flags & GIT_PATH_DIRLOAD_IGNORE_CASE) != 0 ? + git__strncasecmp : git__strncmp; /* stat struct at start of git_path_with_stat, so shift path text */ git_vector_foreach(contents, i, ps) { diff --git a/src/path.h b/src/path.h index b2899e97f..feacc99d0 100644 --- a/src/path.h +++ b/src/path.h @@ -290,6 +290,11 @@ extern int git_path_walk_up( int (*fn)(void *state, git_buf *), void *state); +enum { + GIT_PATH_DIRLOAD_IGNORE_CASE = (1u << 0), + GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE = (1u << 1), +}; + /** * Load all directory entries (except '.' and '..') into a vector. * @@ -310,6 +315,7 @@ extern int git_path_dirload( const char *path, size_t prefix_len, size_t alloc_extra, + unsigned int flags, git_vector *contents); @@ -336,7 +342,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b); * * @param path The directory to read from * @param prefix_len The trailing part of path to prefix to entry paths - * @param ignore_case How to sort and compare paths with start/end limits + * @param flags GIT_PATH_DIRLOAD flags from above * @param start_stat As optimization, only stat values after this prefix * @param end_stat As optimization, only stat values before this prefix * @param contents Vector to fill with git_path_with_stat structures @@ -344,7 +350,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b); extern int git_path_dirload_with_stat( const char *path, size_t prefix_len, - bool ignore_case, + unsigned int flags, const char *start_stat, const char *end_stat, git_vector *contents); diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 0ba711eb7..715b311a4 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -458,17 +458,23 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) { - int error = 0; + int error = 0, t; git_buf path = GIT_BUF_INIT; + git_iterator_flag_t flags = 0; git_iterator *fsit = NULL; const git_index_entry *entry = NULL; if (!backend->path) /* do nothing if no path for loose refs */ return 0; + if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_IGNORECASE) && t) + flags |= GIT_ITERATOR_IGNORE_CASE; + if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_PRECOMPOSE) && t) + flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; + if ((error = git_buf_printf(&path, "%s/refs", backend->path)) < 0 || (error = git_iterator_for_filesystem( - &fsit, git_buf_cstr(&path), 0, NULL, NULL)) < 0) { + &fsit, git_buf_cstr(&path), flags, NULL, NULL)) < 0) { git_buf_free(&path); return error; } diff --git a/src/repository.h b/src/repository.h index 12dc50d51..832df3bd2 100644 --- a/src/repository.h +++ b/src/repository.h @@ -37,6 +37,7 @@ typedef enum { GIT_CVAR_IGNORESTAT, /* core.ignorestat */ GIT_CVAR_TRUSTCTIME, /* core.trustctime */ GIT_CVAR_ABBREV, /* core.abbrev */ + GIT_CVAR_PRECOMPOSE, /* core.precomposeunicode */ GIT_CVAR_CACHE_MAX } git_cvar_cached; @@ -86,6 +87,8 @@ typedef enum { GIT_TRUSTCTIME_DEFAULT = GIT_CVAR_TRUE, /* core.abbrev */ GIT_ABBREV_DEFAULT = 7, + /* core.precomposeunicode */ + GIT_PRECOMPOSE_DEFAULT = GIT_CVAR_FALSE, } git_cvar_value; From 219d3457324e8c2652e3462cedaf648912b40281 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 1 Oct 2013 16:12:15 -0700 Subject: [PATCH 03/14] Initial iconv hookup for precomposed unicode This hooks up git_path_direach and git_path_dirload so that they will take a flag indicating if directory entry names should be tested and converted from decomposed unicode to precomposed form. This code will only come into play on the Apple platform and even then, only when certain types of filesystems are used. This involved adding a flag to these functions which involved changing a lot of places in the code. This was an opportunity to do a bit of code cleanup here and there, for example, getting rid of the git_futils_cleanupdir_r function in favor of a simple flag to git_futils_rmdir_r to not remove the top level entry. That ended up adding depth tracking during rmdir_r which led to a safety check for infinite directory recursion. Yay. This hasn't actually been tested on the Mac filesystems where the issue occurs. I still need to get test environment for that. --- CMakeLists.txt | 5 +- src/clone.c | 42 ++++--- src/fileops.c | 73 +++++------- src/fileops.h | 16 +-- src/iterator.c | 24 ++-- src/odb_loose.c | 6 +- src/odb_pack.c | 2 +- src/path.c | 186 +++++++++++++++++++++++-------- src/path.h | 49 ++++---- src/refdb_fs.c | 28 +++-- tests-clar/clar_libgit2.c | 2 +- tests-clar/clone/nonetwork.c | 29 ++--- tests-clar/core/dirent.c | 24 +--- tests-clar/merge/merge_helpers.c | 2 +- tests-clar/status/worktree.c | 2 +- 15 files changed, 273 insertions(+), 217 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index df3936496..62ebbf4e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,7 +58,10 @@ FUNCTION(TARGET_OS_LIBRARIES target) TARGET_LINK_LIBRARIES(${target} socket nsl) ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux") TARGET_LINK_LIBRARIES(${target} rt) - ENDIF () + ELSEIF(APPLE) + TARGET_LINK_LIBRARIES(${target} iconv) + ENDIF() + IF(THREADSAFE) TARGET_LINK_LIBRARIES(${target} ${CMAKE_THREAD_LIBS_INIT}) ENDIF() diff --git a/src/clone.c b/src/clone.c index f3e365c07..657243945 100644 --- a/src/clone.c +++ b/src/clone.c @@ -391,11 +391,11 @@ int git_clone( const char *local_path, const git_clone_options *_options) { - int retcode = GIT_ERROR; + int error = 0; git_repository *repo = NULL; git_remote *origin; git_clone_options options = GIT_CLONE_OPTIONS_INIT; - int remove_directory_on_failure = 0; + uint32_t rmdir_flags = GIT_RMDIR_REMOVE_FILES; assert(out && url && local_path); @@ -408,33 +408,29 @@ int git_clone( if (git_path_exists(local_path) && !git_path_is_empty_dir(local_path)) { giterr_set(GITERR_INVALID, "'%s' exists and is not an empty directory", local_path); - return GIT_ERROR; + return GIT_EEXISTS; } - /* Only remove the directory on failure if we create it */ - remove_directory_on_failure = !git_path_exists(local_path); + /* Only remove the root directory on failure if we create it */ + if (git_path_exists(local_path)) + rmdir_flags |= GIT_RMDIR_SKIP_ROOT; - if ((retcode = git_repository_init(&repo, local_path, options.bare)) < 0) - return retcode; + if ((error = git_repository_init(&repo, local_path, options.bare)) < 0) + return error; - if ((retcode = create_and_configure_origin(&origin, repo, url, &options)) < 0) - goto cleanup; + if (!(error = create_and_configure_origin(&origin, repo, url, &options))) { + error = git_clone_into( + repo, origin, &options.checkout_opts, options.checkout_branch); - retcode = git_clone_into(repo, origin, &options.checkout_opts, options.checkout_branch); - git_remote_free(origin); + git_remote_free(origin); + } - if (retcode < 0) - goto cleanup; + if (error < 0) { + git_repository_free(repo); + repo = NULL; + (void)git_futils_rmdir_r(local_path, NULL, rmdir_flags); + } *out = repo; - return 0; - -cleanup: - git_repository_free(repo); - if (remove_directory_on_failure) - git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES); - else - git_futils_cleanupdir_r(local_path); - - return retcode; + return error; } diff --git a/src/fileops.c b/src/fileops.c index 5a5041cc3..be2e53ca8 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -398,8 +398,11 @@ typedef struct { size_t baselen; uint32_t flags; int error; + int depth; } futils__rmdir_data; +#define FUTILS_MAX_DEPTH 100 + static int futils__error_cannot_rmdir(const char *path, const char *filemsg) { if (filemsg) @@ -441,7 +444,12 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) struct stat st; futils__rmdir_data *data = opaque; - if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) { + if (data->depth > FUTILS_MAX_DEPTH) { + data->error = + futils__error_cannot_rmdir(path->ptr, "directory nesting too deep"); + } + + else if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) { if (errno == ENOENT) data->error = 0; else if (errno == ENOTDIR) { @@ -457,9 +465,19 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) } 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; + data->depth++; + + { + int error = + git_path_direach(path, 0, futils__rmdir_recurs_foreach, data); + if (error < 0) + return (error == GIT_EUSER) ? data->error : error; + } + + data->depth--; + + if (data->depth == 0 && (data->flags & GIT_RMDIR_SKIP_ROOT) != 0) + return data->error; data->error = p_rmdir(path->ptr); @@ -517,7 +535,7 @@ int git_futils_rmdir_r( { int error; git_buf fullpath = GIT_BUF_INIT; - futils__rmdir_data data; + futils__rmdir_data data = { 0 }; /* build path and find "root" where we should start calling mkdir */ if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0) @@ -526,7 +544,6 @@ int git_futils_rmdir_r( data.base = base ? base : ""; data.baselen = base ? strlen(base) : 0; data.flags = flags; - data.error = 0; error = futils__rmdir_recurs_foreach(&data, &fullpath); @@ -544,41 +561,6 @@ int git_futils_rmdir_r( return error; } -int git_futils_cleanupdir_r(const char *path) -{ - int error; - git_buf fullpath = GIT_BUF_INIT; - futils__rmdir_data data; - - if ((error = git_buf_put(&fullpath, path, strlen(path))) < 0) - goto clean_up; - - data.base = ""; - data.baselen = 0; - data.flags = GIT_RMDIR_REMOVE_FILES; - data.error = 0; - - if (!git_path_exists(path)) { - giterr_set(GITERR_OS, "Path does not exist: %s" , path); - error = GIT_ERROR; - goto clean_up; - } - - if (!git_path_isdir(path)) { - giterr_set(GITERR_OS, "Path is not a directory: %s" , path); - error = GIT_ERROR; - goto clean_up; - } - - error = git_path_direach(&fullpath, futils__rmdir_recurs_foreach, &data); - if (error == GIT_EUSER) - error = data.error; - -clean_up: - git_buf_free(&fullpath); - return error; -} - static int git_futils_guess_system_dirs(git_buf *out) { @@ -948,9 +930,12 @@ static int _cp_r_callback(void *ref, git_buf *from) error = _cp_r_mkdir(info, from); /* recurse onto target directory */ - if (!error && (!exists || S_ISDIR(to_st.st_mode)) && - ((error = git_path_direach(from, _cp_r_callback, info)) == GIT_EUSER)) - error = info->error; + if (!error && (!exists || S_ISDIR(to_st.st_mode))) { + error = git_path_direach(from, 0, _cp_r_callback, info); + + if (error == GIT_EUSER) + error = info->error; + } if (oldmode != 0) info->dirmode = oldmode; diff --git a/src/fileops.h b/src/fileops.h index 6bd693b99..1b2728e58 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -115,12 +115,7 @@ extern int git_futils_mkpath2file(const char *path, const mode_t mode); * * 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: - * - * * 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 + * * GIT_RMDIR_SKIP_ROOT - don't remove root directory itself */ typedef enum { GIT_RMDIR_EMPTY_HIERARCHY = 0, @@ -128,6 +123,7 @@ typedef enum { GIT_RMDIR_SKIP_NONEMPTY = (1 << 1), GIT_RMDIR_EMPTY_PARENTS = (1 << 2), GIT_RMDIR_REMOVE_BLOCKERS = (1 << 3), + GIT_RMDIR_SKIP_ROOT = (1 << 4), } git_futils_rmdir_flags; /** @@ -140,14 +136,6 @@ typedef enum { */ extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags); -/** - * Remove all files and directories beneath the specified path. - * - * @param path Path to the top level directory to process. - * @return 0 on success; -1 on error. - */ -extern int git_futils_cleanupdir_r(const char *path); - /** * Create and open a temporary file with a `_git2_` suffix. * Writes the filename into path_out. diff --git a/src/iterator.c b/src/iterator.c index 946790449..ea6b45e88 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -893,6 +893,7 @@ struct fs_iterator { git_index_entry entry; git_buf path; size_t root_len; + uint32_t dirload_flags; int depth; int (*enter_dir_cb)(fs_iterator *self); @@ -986,10 +987,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi) GITERR_CHECK_ALLOC(ff); error = git_path_dirload_with_stat( - fi->path.ptr, fi->root_len, - (iterator__ignore_case(fi) ? GIT_PATH_DIRLOAD_IGNORE_CASE : 0) | - (iterator__flag(fi, PRECOMPOSE_UNICODE) ? - GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE : 0), + fi->path.ptr, fi->root_len, fi->dirload_flags, fi->base.start, fi->base.end, &ff->entries); if (error < 0) { @@ -1210,6 +1208,11 @@ static int fs_iterator__initialize( } fi->root_len = fi->path.size; + fi->dirload_flags = + (iterator__ignore_case(fi) ? GIT_PATH_DIR_IGNORE_CASE : 0) | + (iterator__flag(fi, PRECOMPOSE_UNICODE) ? + GIT_PATH_DIR_PRECOMPOSE_UNICODE : 0); + if ((error = fs_iterator__expand_dir(fi)) < 0) { if (error == GIT_ENOTFOUND || error == GIT_ITEROVER) { giterr_clear(); @@ -1332,7 +1335,7 @@ int git_iterator_for_workdir_ext( const char *start, const char *end) { - int error; + int error, precompose = 0; workdir_iterator *wi; if (!repo_workdir) { @@ -1360,13 +1363,10 @@ int git_iterator_for_workdir_ext( } /* try to look up precompose and set flag if appropriate */ - { - int precompose = 0; - if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0) - giterr_clear(); - else if (precompose) - wi->fi.base.flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; - } + if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0) + giterr_clear(); + else if (precompose) + wi->fi.base.flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; return fs_iterator__initialize(out, &wi->fi, repo_workdir); } diff --git a/src/odb_loose.c b/src/odb_loose.c index 4ff57158d..07dfae539 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -544,7 +544,7 @@ static int locate_object_short_oid( /* Explore directory to find a unique object matching short_oid */ error = git_path_direach( - object_location, fn_locate_object_short_oid, &state); + object_location, 0, fn_locate_object_short_oid, &state); if (error && error != GIT_EUSER) return error; @@ -745,7 +745,7 @@ static int foreach_cb(void *_state, git_buf *path) { struct foreach_state *state = (struct foreach_state *) _state; - return git_path_direach(path, foreach_object_dir_cb, state); + return git_path_direach(path, 0, foreach_object_dir_cb, state); } static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb cb, void *data) @@ -768,7 +768,7 @@ static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb state.data = data; state.dir_len = git_buf_len(&buf); - error = git_path_direach(&buf, foreach_cb, &state); + error = git_path_direach(&buf, 0, foreach_cb, &state); git_buf_free(&buf); diff --git a/src/odb_pack.c b/src/odb_pack.c index cadc93a65..897bddf32 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -331,7 +331,7 @@ static int pack_backend__refresh(git_odb_backend *_backend) git_buf_sets(&path, backend->pack_folder); /* reload all packs */ - error = git_path_direach(&path, packfile_load__cb, (void *)backend); + error = git_path_direach(&path, 0, packfile_load__cb, backend); git_buf_free(&path); diff --git a/src/path.c b/src/path.c index c4ab57136..857cdfb5d 100644 --- a/src/path.c +++ b/src/path.c @@ -18,6 +18,12 @@ #define LOOKS_LIKE_DRIVE_PREFIX(S) (git__isalpha((S)[0]) && (S)[1] == ':') +#if __APPLE__ +#include +#endif +#define ICONV_REPO_ENCODING "UTF-8" +#define ICONV_PATH_ENCODING "UTF-8-MAC" + #ifdef GIT_WIN32 static bool looks_like_network_computer_name(const char *path, int pos) { @@ -724,14 +730,83 @@ int git_path_cmp( return (c1 < c2) ? -1 : (c1 > c2) ? 1 : 0; } +static bool path_has_non_ascii(const char *path, size_t pathlen) +{ + const uint8_t *scan = (const uint8_t *)path, *end; + + for (end = scan + pathlen; scan < end; ++scan) + if (*scan & 0x80) + return true; + + return false; +} + +#ifdef __APPLE__ +static int path_iconv(iconv_t map, git_buf *out, char **in, size_t *inlen) +{ + char *nfd = *in, *nfc; + size_t nfdlen = *inlen, nfclen, wantlen = nfdlen, rv; + int retry = 1; + + if (!path_has_non_ascii(*in, *inlen)) + return 0; + + while (1) { + if (git_buf_grow(out, wantlen) < 0) + return -1; + + nfc = out->ptr + out->size; + nfclen = out->asize - out->size; + + rv = iconv(map, &nfd, &nfdlen, &nfc, &nfclen); + + out->size = (nfc - out->ptr); + + if (rv != (size_t)-1) + break; + + if (errno != E2BIG) + return -1; + + /* make space for 2x the remaining data to be converted + * (with per retry overhead to avoid infinite loops) + */ + wantlen = out->size + max(nfclen, nfdlen) * 2 + (size_t)(retry * 4); + + if (retry++ > 4) + return -1; + } + + out->ptr[out->size] = '\0'; + + *in = out->ptr; + *inlen = out->size; + + return 0; +} +#endif + +#if defined(__sun) || defined(__GNU__) +typedef char path_dirent_data[sizeof(struct dirent) + FILENAME_MAX + 1]; +#else +typedef struct dirent path_dirent_data; +#endif + int git_path_direach( git_buf *path, + uint32_t flags, int (*fn)(void *, git_buf *), void *arg) { + int error = 0; ssize_t wd_len; DIR *dir; - struct dirent *de, *de_buf; + path_dirent_data de_data; + struct dirent *de, *de_buf = (struct dirent *)&de_data; +#ifdef __APPLE__ + iconv_t nfd2nfc = (iconv_t)-1; + git_buf nfc_path = GIT_BUF_INIT; +#endif if (git_path_to_dir(path) < 0) return -1; @@ -743,38 +818,46 @@ int git_path_direach( return -1; } -#if defined(__sun) || defined(__GNU__) - de_buf = git__malloc(sizeof(struct dirent) + FILENAME_MAX + 1); -#else - de_buf = git__malloc(sizeof(struct dirent)); +#ifdef __APPLE__ + if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0) + nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING); #endif while (p_readdir_r(dir, de_buf, &de) == 0 && de != NULL) { - int result; + char *de_path = de->d_name; + size_t de_len = strlen(de_path); - if (git_path_is_dot_or_dotdot(de->d_name)) + if (git_path_is_dot_or_dotdot(de_path)) continue; - if (git_buf_puts(path, de->d_name) < 0) { - closedir(dir); - git__free(de_buf); - return -1; - } +#if __APPLE__ + if (nfd2nfc != (iconv_t)-1 && + (error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0) + break; +#endif - result = fn(arg, path); + if ((error = git_buf_put(path, de_path, de_len)) < 0) + break; + + error = fn(arg, path); git_buf_truncate(path, wd_len); /* restore path */ - if (result) { - closedir(dir); - git__free(de_buf); - return GIT_EUSER; + if (error) { + error = GIT_EUSER; + break; } } closedir(dir); - git__free(de_buf); - return 0; + +#ifdef __APPLE__ + if (nfd2nfc != (iconv_t)-1) + iconv_close(nfd2nfc); + git_buf_free(&nfc_path); +#endif + + return error; } int git_path_dirload( @@ -786,22 +869,30 @@ int git_path_dirload( { int error, need_slash; DIR *dir; - struct dirent *de, *de_buf; size_t path_len; + path_dirent_data de_data; + struct dirent *de, *de_buf = (struct dirent *)&de_data; +#ifdef __APPLE__ + iconv_t nfd2nfc = (iconv_t)-1; + git_buf nfc_path = GIT_BUF_INIT; +#endif + + assert(path && contents); - assert(path != NULL && contents != NULL); path_len = strlen(path); - assert(path_len > 0 && path_len >= prefix_len); + if (!path_len || path_len < prefix_len) { + giterr_set(GITERR_INVALID, "Invalid directory path '%s'", path); + return -1; + } if ((dir = opendir(path)) == NULL) { giterr_set(GITERR_OS, "Failed to open directory '%s'", path); return -1; } -#if defined(__sun) || defined(__GNU__) - de_buf = git__malloc(sizeof(struct dirent) + FILENAME_MAX + 1); -#else - de_buf = git__malloc(sizeof(struct dirent)); +#ifdef __APPLE__ + if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0) + nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING); #endif path += prefix_len; @@ -809,40 +900,41 @@ int git_path_dirload( need_slash = (path_len > 0 && path[path_len-1] != '/') ? 1 : 0; while ((error = p_readdir_r(dir, de_buf, &de)) == 0 && de != NULL) { - char *entry_path; - size_t entry_len; + char *entry_path, *de_path = de->d_name; + size_t alloc_size, de_len = strlen(de_path); - if (git_path_is_dot_or_dotdot(de->d_name)) + if (git_path_is_dot_or_dotdot(de_path)) continue; - entry_len = strlen(de->d_name); +#if __APPLE__ + if (nfd2nfc != (iconv_t)-1 && + (error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0) + break; +#endif - /* if we read decomposed unicode and precompose flag is set, - * then precompose it now so app code sees it as precomposed - */ - if ((flags & GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE) != 0) { + alloc_size = path_len + need_slash + de_len + 1 + alloc_extra; + if ((entry_path = git__calloc(alloc_size, 1)) == NULL) { + error = -1; + break; } - entry_path = git__malloc( - path_len + need_slash + entry_len + 1 + alloc_extra); - GITERR_CHECK_ALLOC(entry_path); - if (path_len) memcpy(entry_path, path, path_len); if (need_slash) entry_path[path_len] = '/'; - memcpy(&entry_path[path_len + need_slash], de->d_name, entry_len); - entry_path[path_len + need_slash + entry_len] = '\0'; + memcpy(&entry_path[path_len + need_slash], de_path, de_len); - if (git_vector_insert(contents, entry_path) < 0) { - closedir(dir); - git__free(de_buf); - return -1; - } + if ((error = git_vector_insert(contents, entry_path)) < 0) + break; } closedir(dir); - git__free(de_buf); + +#ifdef __APPLE__ + if (nfd2nfc != (iconv_t)-1) + iconv_close(nfd2nfc); + git_buf_free(&nfc_path); +#endif if (error != 0) giterr_set(GITERR_OS, "Failed to process directory entry in '%s'", path); @@ -888,7 +980,7 @@ int git_path_dirload_with_stat( return error; } - strncomp = (flags & GIT_PATH_DIRLOAD_IGNORE_CASE) != 0 ? + strncomp = (flags & GIT_PATH_DIR_IGNORE_CASE) != 0 ? git__strncasecmp : git__strncmp; /* stat struct at start of git_path_with_stat, so shift path text */ diff --git a/src/path.h b/src/path.h index feacc99d0..534cfe50b 100644 --- a/src/path.h +++ b/src/path.h @@ -242,21 +242,28 @@ extern int git_path_resolve_relative(git_buf *path, size_t ceiling); */ extern int git_path_apply_relative(git_buf *target, const char *relpath); +enum { + GIT_PATH_DIR_IGNORE_CASE = (1u << 0), + GIT_PATH_DIR_PRECOMPOSE_UNICODE = (1u << 1), +}; + /** * Walk each directory entry, except '.' and '..', calling fn(state). * - * @param pathbuf buffer the function reads the initial directory + * @param pathbuf Buffer the function reads the initial directory * path from, and updates with each successive entry's name. - * @param fn function to invoke with each entry. The first arg is - * the input state and the second arg is pathbuf. The function - * may modify the pathbuf, but only by appending new text. - * @param state to pass to fn as the first arg. + * @param flags Combination of GIT_PATH_DIR flags. + * @param callback Callback for each entry. Passed the `payload` and each + * successive path inside the directory as a full path. This may + * safely append text to the pathbuf if needed. + * @param payload Passed to callback as first argument. * @return 0 on success, GIT_EUSER on non-zero callback, or error code */ extern int git_path_direach( git_buf *pathbuf, - int (*fn)(void *, git_buf *), - void *state); + uint32_t flags, + int (*callback)(void *payload, git_buf *path), + void *payload); /** * Sort function to order two paths @@ -276,24 +283,19 @@ extern int git_path_cmp( * @param pathbuf Buffer the function reads the directory from and * and updates with each successive name. * @param ceiling Prefix of path at which to stop walking up. If NULL, - * this will walk all the way up to the root. If not a prefix of - * pathbuf, the callback will be invoked a single time on the - * original input path. - * @param fn Function to invoke on each path. The first arg is the - * input satte and the second arg is the pathbuf. The function - * should not modify the pathbuf. + * this will walk all the way up to the root. If not a prefix of + * pathbuf, the callback will be invoked a single time on the + * original input path. + * @param callback Function to invoke on each path. Passed the `payload` + * and the buffer containing the current path. The path should not + * be modified in any way. * @param state Passed to fn as the first ath. */ extern int git_path_walk_up( git_buf *pathbuf, const char *ceiling, - int (*fn)(void *state, git_buf *), - void *state); - -enum { - GIT_PATH_DIRLOAD_IGNORE_CASE = (1u << 0), - GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE = (1u << 1), -}; + int (*callback)(void *payload, git_buf *path), + void *payload); /** * Load all directory entries (except '.' and '..') into a vector. @@ -309,13 +311,14 @@ enum { * prefix_len 3, the entries will look like "b/e1", "b/e2", etc. * @param alloc_extra Extra bytes to add to each string allocation in * case you want to append anything funny. + * @param flags Combination of GIT_PATH_DIR flags. * @param contents Vector to fill with directory entry names. */ extern int git_path_dirload( const char *path, size_t prefix_len, size_t alloc_extra, - unsigned int flags, + uint32_t flags, git_vector *contents); @@ -342,7 +345,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b); * * @param path The directory to read from * @param prefix_len The trailing part of path to prefix to entry paths - * @param flags GIT_PATH_DIRLOAD flags from above + * @param flags GIT_PATH_DIR flags from above * @param start_stat As optimization, only stat values after this prefix * @param end_stat As optimization, only stat values before this prefix * @param contents Vector to fill with git_path_with_stat structures @@ -350,7 +353,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b); extern int git_path_dirload_with_stat( const char *path, size_t prefix_len, - unsigned int flags, + uint32_t flags, const char *start_stat, const char *end_stat, git_vector *contents); diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 715b311a4..afe1a2a42 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -56,6 +56,8 @@ typedef struct refdb_fs_backend { git_sortedcache *refcache; int peeling_mode; + git_iterator_flag_t iterator_flags; + uint32_t direach_flags; } refdb_fs_backend; static int packref_cmp(const void *a_, const void *b_) @@ -269,7 +271,8 @@ static int _dirent_loose_load(void *data, git_buf *full_path) return 0; if (git_path_isdir(full_path->ptr)) - return git_path_direach(full_path, _dirent_loose_load, backend); + return git_path_direach( + full_path, backend->direach_flags, _dirent_loose_load, backend); file_path = full_path->ptr + strlen(backend->path); @@ -295,7 +298,8 @@ static int packed_loadloose(refdb_fs_backend *backend) * This will overwrite any old packed entries with their * updated loose versions */ - error = git_path_direach(&refs_path, _dirent_loose_load, backend); + error = git_path_direach( + &refs_path, backend->direach_flags, _dirent_loose_load, backend); git_buf_free(&refs_path); @@ -458,23 +462,17 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter) static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter) { - int error = 0, t; + int error = 0; git_buf path = GIT_BUF_INIT; - git_iterator_flag_t flags = 0; git_iterator *fsit = NULL; const git_index_entry *entry = NULL; if (!backend->path) /* do nothing if no path for loose refs */ return 0; - if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_IGNORECASE) && t) - flags |= GIT_ITERATOR_IGNORE_CASE; - if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_PRECOMPOSE) && t) - flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; - if ((error = git_buf_printf(&path, "%s/refs", backend->path)) < 0 || (error = git_iterator_for_filesystem( - &fsit, git_buf_cstr(&path), flags, NULL, NULL)) < 0) { + &fsit, path.ptr, backend->iterator_flags, NULL, NULL)) < 0) { git_buf_free(&path); return error; } @@ -1077,6 +1075,7 @@ int git_refdb_backend_fs( git_refdb_backend **backend_out, git_repository *repository) { + int t = 0; git_buf path = GIT_BUF_INIT; refdb_fs_backend *backend; @@ -1098,6 +1097,15 @@ int git_refdb_backend_fs( git_buf_free(&path); + if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_IGNORECASE) && t) { + backend->iterator_flags |= GIT_ITERATOR_IGNORE_CASE; + backend->direach_flags |= GIT_PATH_DIR_IGNORE_CASE; + } + if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_PRECOMPOSE) && t) { + backend->iterator_flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE; + backend->direach_flags |= GIT_PATH_DIR_PRECOMPOSE_UNICODE; + } + backend->parent.exists = &refdb_fs_backend__exists; backend->parent.lookup = &refdb_fs_backend__lookup; backend->parent.iterator = &refdb_fs_backend__iterator; diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index 6063bf91c..0f8f26efb 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -301,7 +301,7 @@ static int remove_placeholders_recurs(void *_data, git_buf *path) size_t pathlen; if (git_path_isdir(path->ptr) == true) - return git_path_direach(path, remove_placeholders_recurs, data); + return git_path_direach(path, 0, remove_placeholders_recurs, data); pathlen = path->size; diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index 4bcb5be1e..c4d75a8dc 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -63,34 +63,27 @@ static int dont_call_me(void *state, git_buf *path) return GIT_ERROR; } +static void assert_empty_directory(const char *path) +{ + git_buf buf = GIT_BUF_INIT; + cl_assert(git_path_exists(path)); + cl_git_pass(git_buf_sets(&buf, path)); + cl_git_pass(git_path_direach(&buf, 0, dont_call_me, NULL)); + git_buf_free(&buf); +} + void test_clone_nonetwork__do_not_clean_existing_directory(void) { - git_buf path_buf = GIT_BUF_INIT; - - git_buf_put(&path_buf, "./foo", 5); - /* Clone should not remove the directory if it already exists, but * Should clean up entries it creates. */ p_mkdir("./foo", GIT_DIR_MODE); cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options)); - cl_assert(git_path_exists("./foo")); - - /* Make sure the directory is empty. */ - cl_git_pass(git_path_direach(&path_buf, - dont_call_me, - NULL)); + assert_empty_directory("./foo"); /* Try again with a bare repository. */ g_options.bare = true; cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options)); - cl_assert(git_path_exists("./foo")); - - /* Make sure the directory is empty. */ - cl_git_pass(git_path_direach(&path_buf, - dont_call_me, - NULL)); - - git_buf_free(&path_buf); + assert_empty_directory("./foo"); } void test_clone_nonetwork__local(void) diff --git a/tests-clar/core/dirent.c b/tests-clar/core/dirent.c index 5a7859d1b..a9a83e827 100644 --- a/tests-clar/core/dirent.c +++ b/tests-clar/core/dirent.c @@ -115,9 +115,7 @@ void test_core_dirent__dont_traverse_dot(void) cl_set_cleanup(&dirent_cleanup__cb, &dot); setup(&dot); - cl_git_pass(git_path_direach(&dot.path, - one_entry, - &dot)); + cl_git_pass(git_path_direach(&dot.path, 0, one_entry, &dot)); check_counts(&dot); } @@ -141,9 +139,7 @@ void test_core_dirent__traverse_subfolder(void) cl_set_cleanup(&dirent_cleanup__cb, &sub); setup(&sub); - cl_git_pass(git_path_direach(&sub.path, - one_entry, - &sub)); + cl_git_pass(git_path_direach(&sub.path, 0, one_entry, &sub)); check_counts(&sub); } @@ -161,9 +157,7 @@ void test_core_dirent__traverse_slash_terminated_folder(void) cl_set_cleanup(&dirent_cleanup__cb, &sub_slash); setup(&sub_slash); - cl_git_pass(git_path_direach(&sub_slash.path, - one_entry, - &sub_slash)); + cl_git_pass(git_path_direach(&sub_slash.path, 0, one_entry, &sub_slash)); check_counts(&sub_slash); } @@ -184,16 +178,12 @@ void test_core_dirent__dont_traverse_empty_folders(void) cl_set_cleanup(&dirent_cleanup__cb, &empty); setup(&empty); - cl_git_pass(git_path_direach(&empty.path, - one_entry, - &empty)); + cl_git_pass(git_path_direach(&empty.path, 0, one_entry, &empty)); check_counts(&empty); /* make sure callback not called */ - cl_git_pass(git_path_direach(&empty.path, - dont_call_me, - &empty)); + cl_git_pass(git_path_direach(&empty.path, 0, dont_call_me, &empty)); } static name_data odd_names[] = { @@ -216,9 +206,7 @@ void test_core_dirent__traverse_weird_filenames(void) cl_set_cleanup(&dirent_cleanup__cb, &odd); setup(&odd); - cl_git_pass(git_path_direach(&odd.path, - one_entry, - &odd)); + cl_git_pass(git_path_direach(&odd.path, 0, one_entry, &odd)); check_counts(&odd); } diff --git a/tests-clar/merge/merge_helpers.c b/tests-clar/merge/merge_helpers.c index e4092787c..ddcb93ff6 100644 --- a/tests-clar/merge/merge_helpers.c +++ b/tests-clar/merge/merge_helpers.c @@ -291,7 +291,7 @@ int merge_test_workdir(git_repository *repo, const struct merge_index_entry expe git_buf wd = GIT_BUF_INIT; git_buf_puts(&wd, repo->workdir); - git_path_direach(&wd, dircount, &actual_len); + git_path_direach(&wd, 0, dircount, &actual_len); if (actual_len != expected_len) return 0; diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c index 135a95871..3b569c7ba 100644 --- a/tests-clar/status/worktree.c +++ b/tests-clar/status/worktree.c @@ -119,7 +119,7 @@ void test_status_worktree__purged_worktree(void) /* first purge the contents of the worktree */ cl_git_pass(git_buf_sets(&workdir, git_repository_workdir(repo))); - cl_git_pass(git_path_direach(&workdir, remove_file_cb, NULL)); + cl_git_pass(git_path_direach(&workdir, 0, remove_file_cb, NULL)); git_buf_free(&workdir); /* now get status */ From 966bb17a57d0c1128261b7575db245f5bc3cf41d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 1 Oct 2013 16:41:07 -0700 Subject: [PATCH 04/14] Add to Git authors who have agreed to relicense MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In email, Torsten Bögershausen agreed that we could use his code from core Git in libgit2 under the modified license. Also, since his work is the basis for much of the precompose unicode support, I have added him to the AUTHORS file as well. --- AUTHORS | 1 + git.git-authors | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index 587da249d..f3a03ee74 100644 --- a/AUTHORS +++ b/AUTHORS @@ -68,5 +68,6 @@ Sven Strickroth Tim Branyen Tim Clem Tim Harder +Torsten Bögershausen Trent Mick Vicent Marti diff --git a/git.git-authors b/git.git-authors index 3fcf27ffc..7d4c95bb3 100644 --- a/git.git-authors +++ b/git.git-authors @@ -68,3 +68,4 @@ ok Sebastian Schuberth ok Shawn O. Pearce ok Steffen Prohaska ok Sven Verdoolaege +ok Torsten Bögershausen From d0849f830f494445e4ef9f04d32be1a2d10b89b3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 2 Oct 2013 11:07:18 -0700 Subject: [PATCH 05/14] Simplify git_path_is_empty_dir implementation This simplifies git_path_is_empty_dir on both Windows (getting rid of git_buf allocation inside the function) and other platforms (by just using git_path_direach), and adds tests for the function, and uses the function to simplify some existing tests. --- src/path.c | 51 ++++++++++++++++-------------------- tests-clar/clone/nonetwork.c | 20 ++------------ tests-clar/core/dirent.c | 31 +++++++++++++++------- 3 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/path.c b/src/path.c index 857cdfb5d..c8d6efb30 100644 --- a/src/path.c +++ b/src/path.c @@ -489,23 +489,23 @@ bool git_path_isfile(const char *path) bool git_path_is_empty_dir(const char *path) { - git_buf pathbuf = GIT_BUF_INIT; HANDLE hFind = INVALID_HANDLE_VALUE; git_win32_path wbuf; + int wbufsz; WIN32_FIND_DATAW ffd; bool retval = true; - if (!git_path_isdir(path)) return false; + if (!git_path_isdir(path)) + return false; - git_buf_printf(&pathbuf, "%s\\*", path); - git_win32_path_from_c(wbuf, git_buf_cstr(&pathbuf)); + wbufsz = git_win32_path_from_c(wbuf, path); + if (!wbufsz || wbufsz + 2 > GIT_WIN_PATH_UTF16) + return false; + memcpy(&wbuf[wbufsz - 1], L"\\*", 3 * sizeof(wchar_t)); hFind = FindFirstFileW(wbuf, &ffd); - if (INVALID_HANDLE_VALUE == hFind) { - giterr_set(GITERR_OS, "Couldn't open '%s'", path); - git_buf_free(&pathbuf); + if (INVALID_HANDLE_VALUE == hFind) return false; - } do { if (!git_path_is_dot_or_dotdotW(ffd.cFileName)) { @@ -515,38 +515,33 @@ bool git_path_is_empty_dir(const char *path) } while (FindNextFileW(hFind, &ffd) != 0); FindClose(hFind); - git_buf_free(&pathbuf); return retval; } #else +static int path_found_entry(void *payload, git_buf *path) +{ + GIT_UNUSED(payload); + return !git_path_is_dot_or_dotdot(path->ptr); +} + bool git_path_is_empty_dir(const char *path) { - DIR *dir = NULL; - struct dirent *e; - bool retval = true; + int error; + git_buf dir = GIT_BUF_INIT; - if (!git_path_isdir(path)) return false; - - dir = opendir(path); - if (!dir) { - giterr_set(GITERR_OS, "Couldn't open '%s'", path); + if (!git_path_isdir(path)) return false; - } - while ((e = readdir(dir)) != NULL) { - if (!git_path_is_dot_or_dotdot(e->d_name)) { - giterr_set(GITERR_INVALID, - "'%s' exists and is not an empty directory", path); - retval = false; - break; - } - } - closedir(dir); + if (!(error = git_buf_sets(&dir, path))) + error = git_path_direach(&dir, 0, path_found_entry, NULL); - return retval; + git_buf_free(&dir); + + return !error; } + #endif int git_path_lstat(const char *path, struct stat *st) diff --git a/tests-clar/clone/nonetwork.c b/tests-clar/clone/nonetwork.c index c4d75a8dc..071e3d09f 100644 --- a/tests-clar/clone/nonetwork.c +++ b/tests-clar/clone/nonetwork.c @@ -56,34 +56,18 @@ void test_clone_nonetwork__bad_url(void) cl_assert(!git_path_exists("./foo")); } -static int dont_call_me(void *state, git_buf *path) -{ - GIT_UNUSED(state); - GIT_UNUSED(path); - return GIT_ERROR; -} - -static void assert_empty_directory(const char *path) -{ - git_buf buf = GIT_BUF_INIT; - cl_assert(git_path_exists(path)); - cl_git_pass(git_buf_sets(&buf, path)); - cl_git_pass(git_path_direach(&buf, 0, dont_call_me, NULL)); - git_buf_free(&buf); -} - void test_clone_nonetwork__do_not_clean_existing_directory(void) { /* Clone should not remove the directory if it already exists, but * Should clean up entries it creates. */ p_mkdir("./foo", GIT_DIR_MODE); cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options)); - assert_empty_directory("./foo"); + cl_assert(git_path_is_empty_dir("./foo")); /* Try again with a bare repository. */ g_options.bare = true; cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options)); - assert_empty_directory("./foo"); + cl_assert(git_path_is_empty_dir("./foo")); } void test_clone_nonetwork__local(void) diff --git a/tests-clar/core/dirent.c b/tests-clar/core/dirent.c index a9a83e827..f17260362 100644 --- a/tests-clar/core/dirent.c +++ b/tests-clar/core/dirent.c @@ -88,14 +88,6 @@ static int one_entry(void *state, git_buf *path) return GIT_ERROR; } -static int dont_call_me(void *state, git_buf *path) -{ - GIT_UNUSED(state); - GIT_UNUSED(path); - return GIT_ERROR; -} - - static name_data dot_names[] = { { 0, "./a" }, @@ -183,7 +175,7 @@ void test_core_dirent__dont_traverse_empty_folders(void) check_counts(&empty); /* make sure callback not called */ - cl_git_pass(git_path_direach(&empty.path, 0, dont_call_me, &empty)); + cl_assert(git_path_is_empty_dir(empty.path.ptr)); } static name_data odd_names[] = { @@ -219,5 +211,26 @@ void test_core_dirent__length_limits(void) big_filename[FILENAME_MAX] = 0; cl_must_fail(p_creat(big_filename, 0666)); + git__free(big_filename); } + +void test_core_dirent__empty_dir(void) +{ + cl_must_pass(p_mkdir("empty_dir", 0777)); + cl_assert(git_path_is_empty_dir("empty_dir")); + + cl_git_mkfile("empty_dir/content", "whatever\n"); + cl_assert(!git_path_is_empty_dir("empty_dir")); + cl_assert(!git_path_is_empty_dir("empty_dir/content")); + + cl_must_pass(p_unlink("empty_dir/content")); + + cl_must_pass(p_mkdir("empty_dir/content", 0777)); + cl_assert(!git_path_is_empty_dir("empty_dir")); + cl_assert(git_path_is_empty_dir("empty_dir/content")); + + cl_must_pass(p_rmdir("empty_dir/content")); + + cl_must_pass(p_rmdir("empty_dir")); +} From 618b7689e1cdd4ebd956949a95038fd49592a187 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 2 Oct 2013 12:06:26 -0700 Subject: [PATCH 06/14] Wrap iconv stuff and write tests This adds a simple wrapper around the iconv APIs and uses it instead of the old code that was inlining the iconv stuff. This makes it possible for me to test the iconv logic in isolation. A "no iconv" version of the API was defined with macros so that I could have fewer ifdefs in the code itself. --- src/path.c | 105 ++++++++++++++++++---------------------- src/path.h | 52 ++++++++++++++++++++ tests-clar/core/iconv.c | 57 ++++++++++++++++++++++ tests-clar/core/path.c | 2 +- 4 files changed, 157 insertions(+), 59 deletions(-) create mode 100644 tests-clar/core/iconv.c diff --git a/src/path.c b/src/path.c index c8d6efb30..27abd062b 100644 --- a/src/path.c +++ b/src/path.c @@ -18,12 +18,6 @@ #define LOOKS_LIKE_DRIVE_PREFIX(S) (git__isalpha((S)[0]) && (S)[1] == ':') -#if __APPLE__ -#include -#endif -#define ICONV_REPO_ENCODING "UTF-8" -#define ICONV_PATH_ENCODING "UTF-8-MAC" - #ifdef GIT_WIN32 static bool looks_like_network_computer_name(const char *path, int pos) { @@ -725,7 +719,7 @@ int git_path_cmp( return (c1 < c2) ? -1 : (c1 > c2) ? 1 : 0; } -static bool path_has_non_ascii(const char *path, size_t pathlen) +bool git_path_has_non_ascii(const char *path, size_t pathlen) { const uint8_t *scan = (const uint8_t *)path, *end; @@ -736,49 +730,72 @@ static bool path_has_non_ascii(const char *path, size_t pathlen) return false; } -#ifdef __APPLE__ -static int path_iconv(iconv_t map, git_buf *out, char **in, size_t *inlen) +#ifdef GIT_USE_ICONV + +int git_path_iconv_init_precompose(git_path_iconv_t *ic) +{ + git_buf_init(&ic->buf, 0); + ic->map = iconv_open(GIT_PATH_REPO_ENCODING, GIT_PATH_NATIVE_ENCODING); + return 0; +} + +void git_path_iconv_clear(git_path_iconv_t *ic) +{ + if (ic) { + if (ic->map != (iconv_t)-1) + iconv_close(ic->map); + git_buf_free(&ic->buf); + } +} + +int git_path_iconv(git_path_iconv_t *ic, char **in, size_t *inlen) { char *nfd = *in, *nfc; size_t nfdlen = *inlen, nfclen, wantlen = nfdlen, rv; int retry = 1; - if (!path_has_non_ascii(*in, *inlen)) + if (!ic || ic->map == (iconv_t)-1 || + !git_path_has_non_ascii(*in, *inlen)) return 0; while (1) { - if (git_buf_grow(out, wantlen) < 0) + if (git_buf_grow(&ic->buf, wantlen) < 0) return -1; - nfc = out->ptr + out->size; - nfclen = out->asize - out->size; + nfc = ic->buf.ptr + ic->buf.size; + nfclen = ic->buf.asize - ic->buf.size; - rv = iconv(map, &nfd, &nfdlen, &nfc, &nfclen); + rv = iconv(ic->map, &nfd, &nfdlen, &nfc, &nfclen); - out->size = (nfc - out->ptr); + ic->buf.size = (nfc - ic->buf.ptr); if (rv != (size_t)-1) break; if (errno != E2BIG) - return -1; + goto fail; /* make space for 2x the remaining data to be converted * (with per retry overhead to avoid infinite loops) */ - wantlen = out->size + max(nfclen, nfdlen) * 2 + (size_t)(retry * 4); + wantlen = ic->buf.size + max(nfclen, nfdlen) * 2 + (size_t)(retry * 4); if (retry++ > 4) - return -1; + goto fail; } - out->ptr[out->size] = '\0'; + ic->buf.ptr[ic->buf.size] = '\0'; - *in = out->ptr; - *inlen = out->size; + *in = ic->buf.ptr; + *inlen = ic->buf.size; return 0; + +fail: + giterr_set(GITERR_OS, "Unable to convert unicode path data"); + return -1; } + #endif #if defined(__sun) || defined(__GNU__) @@ -798,10 +815,7 @@ int git_path_direach( DIR *dir; path_dirent_data de_data; struct dirent *de, *de_buf = (struct dirent *)&de_data; -#ifdef __APPLE__ - iconv_t nfd2nfc = (iconv_t)-1; - git_buf nfc_path = GIT_BUF_INIT; -#endif + git_path_iconv_t ic = GIT_PATH_ICONV_INIT; if (git_path_to_dir(path) < 0) return -1; @@ -813,10 +827,8 @@ int git_path_direach( return -1; } -#ifdef __APPLE__ if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0) - nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING); -#endif + (void)git_path_iconv_init_precompose(&ic); while (p_readdir_r(dir, de_buf, &de) == 0 && de != NULL) { char *de_path = de->d_name; @@ -825,13 +837,8 @@ int git_path_direach( if (git_path_is_dot_or_dotdot(de_path)) continue; -#if __APPLE__ - if (nfd2nfc != (iconv_t)-1 && - (error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0) - break; -#endif - - if ((error = git_buf_put(path, de_path, de_len)) < 0) + if ((error = git_path_iconv(&ic, &de_path, &de_len)) < 0 || + (error = git_buf_put(path, de_path, de_len)) < 0) break; error = fn(arg, path); @@ -845,12 +852,7 @@ int git_path_direach( } closedir(dir); - -#ifdef __APPLE__ - if (nfd2nfc != (iconv_t)-1) - iconv_close(nfd2nfc); - git_buf_free(&nfc_path); -#endif + git_path_iconv_clear(&ic); return error; } @@ -867,10 +869,7 @@ int git_path_dirload( size_t path_len; path_dirent_data de_data; struct dirent *de, *de_buf = (struct dirent *)&de_data; -#ifdef __APPLE__ - iconv_t nfd2nfc = (iconv_t)-1; - git_buf nfc_path = GIT_BUF_INIT; -#endif + git_path_iconv_t ic = GIT_PATH_ICONV_INIT; assert(path && contents); @@ -885,10 +884,8 @@ int git_path_dirload( return -1; } -#ifdef __APPLE__ if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0) - nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING); -#endif + (void)git_path_iconv_init_precompose(&ic); path += prefix_len; path_len -= prefix_len; @@ -901,11 +898,8 @@ int git_path_dirload( if (git_path_is_dot_or_dotdot(de_path)) continue; -#if __APPLE__ - if (nfd2nfc != (iconv_t)-1 && - (error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0) + if ((error = git_path_iconv(&ic, &de_path, &de_len)) < 0) break; -#endif alloc_size = path_len + need_slash + de_len + 1 + alloc_extra; if ((entry_path = git__calloc(alloc_size, 1)) == NULL) { @@ -924,12 +918,7 @@ int git_path_dirload( } closedir(dir); - -#ifdef __APPLE__ - if (nfd2nfc != (iconv_t)-1) - iconv_close(nfd2nfc); - git_buf_free(&nfc_path); -#endif + git_path_iconv_clear(&ic); if (error != 0) giterr_set(GITERR_OS, "Failed to process directory entry in '%s'", path); diff --git a/src/path.h b/src/path.h index 534cfe50b..2cfd714cd 100644 --- a/src/path.h +++ b/src/path.h @@ -358,4 +358,56 @@ extern int git_path_dirload_with_stat( const char *end_stat, git_vector *contents); +/* check if non-ascii characters are present in filename */ +extern bool git_path_has_non_ascii(const char *path, size_t pathlen); + +/* only enable iconv on Mac for now */ +#ifdef __APPLE__ +#define GIT_USE_ICONV 1 +#endif + +#define GIT_PATH_REPO_ENCODING "UTF-8" + +#ifdef __APPLE__ +#define GIT_PATH_NATIVE_ENCODING "UTF-8-MAC" +#else +#define GIT_PATH_NATIVE_ENCODING "UTF-8" +#endif + +#ifdef GIT_USE_ICONV + +#include + +typedef struct { + iconv_t map; + git_buf buf; +} git_path_iconv_t; + +#define GIT_PATH_ICONV_INIT { (iconv_t)-1, GIT_BUF_INIT } + +/* Init iconv data for converting decomposed UTF-8 to precomposed */ +extern int git_path_iconv_init_precompose(git_path_iconv_t *ic); + +/* Clear allocated iconv data */ +extern void git_path_iconv_clear(git_path_iconv_t *ic); + +/* + * Rewrite `in` buffer using iconv map if necessary, replacing `in` + * pointer internal iconv buffer if rewrite happened. The `in` pointer + * will be left unchanged if no rewrite was needed. + */ +extern int git_path_iconv(git_path_iconv_t *ic, char **in, size_t *inlen); + +#else + +typedef struct { + int unused; +} git_path_iconv_t; +#define GIT_PATH_ICONV_INIT { 0 } +#define git_path_iconv_init_precompose(X) 0 +#define git_path_iconv_clear(X) (void)(X) +#define git_path_iconv(X,Y,Z) 0 + +#endif /* GIT_USE_ICONV */ + #endif diff --git a/tests-clar/core/iconv.c b/tests-clar/core/iconv.c new file mode 100644 index 000000000..5a3e1dee0 --- /dev/null +++ b/tests-clar/core/iconv.c @@ -0,0 +1,57 @@ +#include "clar_libgit2.h" +#include "path.h" + +static git_path_iconv_t ic; +static char *nfc = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D"; +static char *nfd = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D"; + +void test_core_iconv__initialize(void) +{ + cl_git_pass(git_path_iconv_init_precompose(&ic)); +} + +void test_core_iconv__cleanup(void) +{ + git_path_iconv_clear(&ic); +} + +void test_core_iconv__unchanged(void) +{ + char *data = "Ascii data", *original = data; + size_t datalen = strlen(data); + + cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + + /* There are no high bits set, so this should leave data untouched */ + cl_assert(data == original); +} + +void test_core_iconv__decomposed_to_precomposed(void) +{ + char *data = nfd; + size_t datalen = strlen(nfd); + + cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + + /* The decomposed nfd string should be transformed to the nfc form + * (on platforms where iconv is enabled, of course). + */ +#ifdef GIT_USE_ICONV + cl_assert_equal_s(nfc, data); +#else + cl_assert_equal_s(nfd, data); +#endif +} + +void test_core_iconv__precomposed_is_unmodified(void) +{ + char *data = nfc; + size_t datalen = strlen(nfc); + + cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + + /* data is already in precomposed form, so even though some bytes have + * the high-bit set, the iconv transform should result in no change. + */ + cl_assert_equal_s(nfc, data); +} diff --git a/tests-clar/core/path.c b/tests-clar/core/path.c index e584d6115..cf2d5e944 100644 --- a/tests-clar/core/path.c +++ b/tests-clar/core/path.c @@ -1,5 +1,5 @@ #include "clar_libgit2.h" -#include +#include "fileops.h" static void check_dirname(const char *A, const char *B) From af302acaee95ff2ca3379b93cd886ebe96adff10 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 2 Oct 2013 14:13:11 -0700 Subject: [PATCH 07/14] Clean up annoying warnings The indexer code was generating warnings on Windows 64-bit. I looked closely at the logic and was able to simplify it a bit. Also this fixes some other Windows and Linux warnings. --- src/indexer.c | 10 +++++----- src/repository.c | 6 +++++- tests-clar/core/iconv.c | 3 +++ tests-clar/online/push.c | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 4ce69fc8d..1270488f0 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -50,7 +50,7 @@ struct git_indexer_stream { /* Fields for calculating the packfile trailer (hash of everything before it) */ char inbuf[GIT_OID_RAWSZ]; - int inbuf_len; + size_t inbuf_len; git_hash_ctx trailer; }; @@ -378,13 +378,13 @@ static int do_progress_callback(git_indexer_stream *idx, git_transfer_progress * /* Hash everything but the last 20B of input */ static void hash_partially(git_indexer_stream *idx, const uint8_t *data, size_t size) { - int to_expell, to_keep; + size_t to_expell, to_keep; if (size == 0) return; /* Easy case, dump the buffer and the data minus the last 20 bytes */ - if (size >= 20) { + if (size >= GIT_OID_RAWSZ) { git_hash_update(&idx->trailer, idx->inbuf, idx->inbuf_len); git_hash_update(&idx->trailer, data, size - GIT_OID_RAWSZ); @@ -402,8 +402,8 @@ static void hash_partially(git_indexer_stream *idx, const uint8_t *data, size_t } /* We need to partially drain the buffer and then append */ - to_expell = abs(size - (GIT_OID_RAWSZ - idx->inbuf_len)); - to_keep = abs(idx->inbuf_len - to_expell); + to_keep = GIT_OID_RAWSZ - size; + to_expell = idx->inbuf_len - to_keep; git_hash_update(&idx->trailer, idx->inbuf, to_expell); diff --git a/src/repository.c b/src/repository.c index 64f13978d..51cc76db4 100644 --- a/src/repository.c +++ b/src/repository.c @@ -890,6 +890,8 @@ static bool are_symlinks_supported(const char *wd_path) return symlinks_supported; } +#ifdef GIT_USE_ICONV + static const char *nfc_file = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D.XXXXXX"; static const char *nfd_file = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D.XXXXXX"; @@ -938,6 +940,8 @@ fail: return need_precompose; } +#endif + static int create_empty_file(const char *path, mode_t mode) { int fd; @@ -994,7 +998,7 @@ static int repo_init_config( SET_REPO_CONFIG( bool, "core.filemode", is_chmod_supported(git_buf_cstr(&cfg_path))); -#if __APPLE__ +#ifdef GIT_USE_ICONV SET_REPO_CONFIG( bool, "core.precomposeunicode", should_precompose_unicode_paths(is_bare ? repo_dir : work_dir)); diff --git a/tests-clar/core/iconv.c b/tests-clar/core/iconv.c index 5a3e1dee0..73bc99a23 100644 --- a/tests-clar/core/iconv.c +++ b/tests-clar/core/iconv.c @@ -21,6 +21,7 @@ void test_core_iconv__unchanged(void) size_t datalen = strlen(data); cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + GIT_UNUSED(datalen); /* There are no high bits set, so this should leave data untouched */ cl_assert(data == original); @@ -32,6 +33,7 @@ void test_core_iconv__decomposed_to_precomposed(void) size_t datalen = strlen(nfd); cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + GIT_UNUSED(datalen); /* The decomposed nfd string should be transformed to the nfc form * (on platforms where iconv is enabled, of course). @@ -49,6 +51,7 @@ void test_core_iconv__precomposed_is_unmodified(void) size_t datalen = strlen(nfc); cl_git_pass(git_path_iconv(&ic, &data, &datalen)); + GIT_UNUSED(datalen); /* data is already in precomposed form, so even though some bytes have * the high-bit set, the iconv transform should result in no change. diff --git a/tests-clar/online/push.c b/tests-clar/online/push.c index 4c2bec7ca..957cef7ca 100644 --- a/tests-clar/online/push.c +++ b/tests-clar/online/push.c @@ -356,7 +356,7 @@ static int push_pack_progress_cb(int stage, unsigned int current, unsigned int t static int push_transfer_progress_cb(unsigned int current, unsigned int total, size_t bytes, void* payload) { int *was_called = (int *) payload; - GIT_UNUSED(current); GIT_UNUSED(total); GIT_UNUSED(bytes); + GIT_UNUSED(current); GIT_UNUSED(total); GIT_UNUSED(bytes); *was_called = 1; return 0; } From 840fb4fc4346580a6398856b3fa2f1226b3a365d Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 3 Oct 2013 14:42:37 -0700 Subject: [PATCH 08/14] Update repo init with fewer platform assumptions The repo init code was assuming Windows == no filemode, and Mac or Windows == no case sensitivity. Those assumptions are not consistently true depending on the mounted file system. This is a first step to removing those assumptions. It focuses on the repo init code and the tests of that code. There are still many other tests that are broken when those assumptions don't hold true, but this clears up one area of the code. Also, this moves the core.precomposeunicode logic to be closer to the current logic in core Git where it will be set to true on any filesystem where composed unicode is decomposed when read back. --- src/repository.c | 34 ++++++++-------- tests-clar/repo/init.c | 92 ++++++++++++++++++++++-------------------- 2 files changed, 66 insertions(+), 60 deletions(-) diff --git a/src/repository.c b/src/repository.c index 51cc76db4..f609e5779 100644 --- a/src/repository.c +++ b/src/repository.c @@ -895,18 +895,20 @@ static bool are_symlinks_supported(const char *wd_path) static const char *nfc_file = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D.XXXXXX"; static const char *nfd_file = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D.XXXXXX"; -/* On Mac, HDFS always stores files using decomposed unicode, but when - * writing to VFAT or SAMBA file systems, filenames may be kept as - * precomposed unicode, but will be converted to decomposed form when - * reading the directory entries. This can cause file name mismatches. - * The solution is to convert directory entries to precomposed form if we - * cannot look up the file from the decomposed path. +/* Check if the platform is decomposing unicode data for us. We will + * emulate core Git and prefer to use precomposed unicode data internally + * on these platforms, composing the decomposed unicode on the fly. + * + * This mainly happens on the Mac where HDFS stores filenames as + * decomposed unicode. Even on VFAT and SAMBA file systems, the Mac will + * return decomposed unicode from readdir() even when the actual + * filesystem is storing precomposed unicode. */ -static bool should_precompose_unicode_paths(const char *wd_path) +static bool does_fs_decompose_unicode_paths(const char *wd_path) { git_buf path = GIT_BUF_INIT; int fd; - bool need_precompose = false; + bool found_decomposed = false; char tmp[6]; /* Create a file using a precomposed path and then try to find it @@ -915,7 +917,7 @@ static bool should_precompose_unicode_paths(const char *wd_path) */ if (git_buf_joinpath(&path, wd_path, nfc_file) < 0 || (fd = p_mkstemp(path.ptr)) < 0) - goto fail; + goto done; p_close(fd); /* record trailing digits generated by mkstemp */ @@ -923,21 +925,21 @@ static bool should_precompose_unicode_paths(const char *wd_path) /* try to look up as NFD path */ if (git_buf_joinpath(&path, wd_path, nfd_file) < 0) - goto fail; + goto done; memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp)); - need_precompose = !git_path_exists(path.ptr); + found_decomposed = git_path_exists(path.ptr); - /* remove temporary file */ + /* remove temporary file (using original precomposed path) */ if (git_buf_joinpath(&path, wd_path, nfc_file) < 0) - goto fail; + goto done; memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp)); (void)p_unlink(path.ptr); -fail: +done: git_buf_free(&path); - return need_precompose; + return found_decomposed; } #endif @@ -1001,7 +1003,7 @@ static int repo_init_config( #ifdef GIT_USE_ICONV SET_REPO_CONFIG( bool, "core.precomposeunicode", - should_precompose_unicode_paths(is_bare ? repo_dir : work_dir)); + does_fs_decompose_unicode_paths(is_bare ? repo_dir : work_dir)); #endif if (!are_symlinks_supported(is_bare ? repo_dir : work_dir)) diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 6f3c84175..061e444b5 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -179,41 +179,32 @@ void test_repo_init__additional_templates(void) git_buf_free(&path); } -static void assert_config_entry_on_init_bytype(const char *config_key, int expected_value, bool is_bare) +static void assert_config_entry_on_init_bytype( + const char *config_key, int expected_value, bool is_bare) { git_config *config; - int current_value; - git_buf repo_path = GIT_BUF_INIT; + int error, current_value; + const char *repo_path = is_bare ? + "config_entry/test.bare.git" : "config_entry/test.non.bare.git"; cl_set_cleanup(&cleanup_repository, "config_entry"); - cl_git_pass(git_buf_puts(&repo_path, "config_entry/test.")); + cl_git_pass(git_repository_init(&_repo, repo_path, is_bare)); - if (!is_bare) - cl_git_pass(git_buf_puts(&repo_path, "non.")); - - cl_git_pass(git_buf_puts(&repo_path, "bare.git")); - - cl_git_pass(git_repository_init(&_repo, git_buf_cstr(&repo_path), is_bare)); - - git_buf_free(&repo_path); - - git_repository_config(&config, _repo); + cl_git_pass(git_repository_config(&config, _repo)); + error = git_config_get_bool(¤t_value, config, config_key); + git_config_free(config); if (expected_value >= 0) { - cl_git_pass(git_config_get_bool(¤t_value, config, config_key)); - + cl_assert_equal_i(0, error); cl_assert_equal_i(expected_value, current_value); } else { - int error = git_config_get_bool(¤t_value, config, config_key); - cl_assert_equal_i(expected_value, error); } - - git_config_free(config); } -static void assert_config_entry_on_init(const char *config_key, int expected_value) +static void assert_config_entry_on_init( + const char *config_key, int expected_value) { assert_config_entry_on_init_bytype(config_key, expected_value, true); git_repository_free(_repo); @@ -221,31 +212,47 @@ static void assert_config_entry_on_init(const char *config_key, int expected_val assert_config_entry_on_init_bytype(config_key, expected_value, false); } -void test_repo_init__detect_filemode(void) +static int expect_filemode_support(void) { -#ifdef GIT_WIN32 - assert_config_entry_on_init("core.filemode", false); -#else - assert_config_entry_on_init("core.filemode", true); -#endif + struct stat st; + + cl_git_write2file("testmode", "whatever\n", 0, O_CREAT | O_WRONLY, 0767); + cl_must_pass(p_stat("testmode", &st)); + cl_must_pass(p_unlink("testmode")); + + return (st.st_mode & 0111) == 0101; } -#define CASE_INSENSITIVE_FILESYSTEM (defined GIT_WIN32 || defined __APPLE__) +void test_repo_init__detect_filemode(void) +{ + assert_config_entry_on_init("core.filemode", expect_filemode_support()); +} void test_repo_init__detect_ignorecase(void) { -#if CASE_INSENSITIVE_FILESYSTEM - assert_config_entry_on_init("core.ignorecase", true); -#else - assert_config_entry_on_init("core.ignorecase", GIT_ENOTFOUND); -#endif + struct stat st; + bool found_without_match; + + cl_git_write2file("testCAPS", "whatever\n", 0, O_CREAT | O_WRONLY, 0666); + found_without_match = (p_stat("Testcaps", &st) == 0); + cl_must_pass(p_unlink("testCAPS")); + + assert_config_entry_on_init( + "core.ignorecase", found_without_match ? true : GIT_ENOTFOUND); } void test_repo_init__detect_precompose_unicode_required(void) { -#ifdef __APPLE__ - /* hard to test "true" case without SAMBA or VFAT file system available */ - assert_config_entry_on_init("core.precomposeunicode", false); + char *composed = "ḱṷṓn", *decomposed = "ḱṷṓn"; + struct stat st; + bool found_with_nfd; + + cl_git_write2file(composed, "whatever\n", 0, O_CREAT | O_WRONLY, 0666); + found_with_nfd = (p_stat(decomposed, &st) == 0); + cl_must_pass(p_unlink(composed)); + +#ifdef GIT_USE_ICONV + assert_config_entry_on_init("core.precomposeunicode", found_with_nfd); #else assert_config_entry_on_init("core.precomposeunicode", GIT_ENOTFOUND); #endif @@ -280,13 +287,7 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void) void test_repo_init__reinit_overwrites_filemode(void) { - int expected, current_value; - -#ifdef GIT_WIN32 - expected = false; -#else - expected = true; -#endif + int expected = expect_filemode_support(), current_value; /* Init a new repo */ cl_set_cleanup(&cleanup_repository, "overwrite.git"); @@ -358,7 +359,10 @@ void test_repo_init__extended_1(void) cl_git_pass(git_path_lstat(git_repository_path(_repo), &st)); cl_assert(S_ISDIR(st.st_mode)); - cl_assert((S_ISGID & st.st_mode) == S_ISGID); + if (expect_filemode_support()) + cl_assert((S_ISGID & st.st_mode) == S_ISGID); + else + cl_assert((S_ISGID & st.st_mode) == 0); cl_git_pass(git_reference_lookup(&ref, _repo, "HEAD")); cl_assert(git_reference_type(ref) == GIT_REF_SYMBOLIC); From b8f9059d6330d1b0113910eb4c6f049ea5a150a4 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 3 Oct 2013 15:16:06 -0700 Subject: [PATCH 09/14] More cleanups to remove WIN assumptions This cleans up more of the test suite to check actual filesystem behavior instead of relying on Windows vs. Mac vs. Linux to test. --- tests-clar/checkout/index.c | 10 ++++++---- tests-clar/core/mkdir.c | 20 +++++++++++++------- tests-clar/repo/init.c | 17 +++-------------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 73050d08e..48d6d79f9 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -224,13 +224,15 @@ void test_checkout_index__options_disable_filters(void) void test_checkout_index__options_dir_modes(void) { -#ifndef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; struct stat st; git_oid oid; git_commit *commit; mode_t um; + if (!cl_is_chmod_supported()) + return; + cl_git_pass(git_reference_name_to_id(&oid, g_repo, "refs/heads/dir")); cl_git_pass(git_commit_lookup(&commit, g_repo, &oid)); @@ -252,15 +254,16 @@ void test_checkout_index__options_dir_modes(void) cl_assert_equal_i_fmt(st.st_mode, GIT_FILEMODE_BLOB_EXECUTABLE, "%07o"); git_commit_free(commit); -#endif } void test_checkout_index__options_override_file_modes(void) { -#ifndef GIT_WIN32 git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; struct stat st; + if (!cl_is_chmod_supported()) + return; + opts.checkout_strategy = GIT_CHECKOUT_SAFE_CREATE; opts.file_mode = 0700; @@ -268,7 +271,6 @@ void test_checkout_index__options_override_file_modes(void) cl_git_pass(p_stat("./testrepo/new.txt", &st)); cl_assert_equal_i_fmt(st.st_mode & GIT_MODE_PERMS_MASK, 0700, "%07o"); -#endif } void test_checkout_index__options_open_flags(void) diff --git a/tests-clar/core/mkdir.c b/tests-clar/core/mkdir.c index a969e4de2..a8c5b10ae 100644 --- a/tests-clar/core/mkdir.c +++ b/tests-clar/core/mkdir.c @@ -111,14 +111,20 @@ static void cleanup_chmod_root(void *ref) git_futils_rmdir_r("r", NULL, GIT_RMDIR_EMPTY_HIERARCHY); } -static void check_mode(mode_t expected, mode_t actual) +#define check_mode(X,A) check_mode_at_line((X), (A), __FILE__, __LINE__) + +static void check_mode_at_line( + mode_t expected, mode_t actual, const char *file, int line) { -#ifdef GIT_WIN32 - /* chmod on Win32 doesn't support exec bit, not group/world bits */ - cl_assert_equal_i_fmt((expected & 0600), (actual & 0777), "%07o"); -#else - cl_assert_equal_i_fmt(expected, (actual & 0777), "%07o"); -#endif + /* FAT filesystems don't support exec bit, nor group/world bits */ + if (!cl_is_chmod_supported()) { + expected &= 0600; + actual &= 0600; + } + + clar__assert_equal( + file, line, "expected_mode != actual_mode", 1, + "%07o", (int)expected, (int)(actual & 0777)); } void test_core_mkdir__chmods(void) diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 061e444b5..617fdf879 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -212,20 +212,9 @@ static void assert_config_entry_on_init( assert_config_entry_on_init_bytype(config_key, expected_value, false); } -static int expect_filemode_support(void) -{ - struct stat st; - - cl_git_write2file("testmode", "whatever\n", 0, O_CREAT | O_WRONLY, 0767); - cl_must_pass(p_stat("testmode", &st)); - cl_must_pass(p_unlink("testmode")); - - return (st.st_mode & 0111) == 0101; -} - void test_repo_init__detect_filemode(void) { - assert_config_entry_on_init("core.filemode", expect_filemode_support()); + assert_config_entry_on_init("core.filemode", cl_is_chmod_supported()); } void test_repo_init__detect_ignorecase(void) @@ -287,7 +276,7 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void) void test_repo_init__reinit_overwrites_filemode(void) { - int expected = expect_filemode_support(), current_value; + int expected = cl_is_chmod_supported(), current_value; /* Init a new repo */ cl_set_cleanup(&cleanup_repository, "overwrite.git"); @@ -359,7 +348,7 @@ void test_repo_init__extended_1(void) cl_git_pass(git_path_lstat(git_repository_path(_repo), &st)); cl_assert(S_ISDIR(st.st_mode)); - if (expect_filemode_support()) + if (cl_is_chmod_supported()) cl_assert((S_ISGID & st.st_mode) == S_ISGID); else cl_assert((S_ISGID & st.st_mode) == 0); From 5173ea921d4ccbbe7d61ddce9a0920c2e1c82035 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Fri, 4 Oct 2013 16:32:16 -0700 Subject: [PATCH 10/14] Add git_repository_reset_filesystem and fix tests When a repository is transferred from one file system to another, many of the config settings that represent the properties of the file system may be wrong. This adds a new public API that will refresh the config settings of the repository to account for the change of file system. This doesn't do a full "reinitialize" and operates on a existing git_repository object refreshing the config when done. This commit then makes use of the new API in clar as each test repository is set up. This commit also has a number of other clar test fixes where we were making assumptions about the type of filesystem, either based on outdated config data or based on the OS instead of the FS. --- include/git2/repository.h | 17 ++++++ src/config.h | 2 +- src/repository.c | 102 +++++++++++++++++++++++---------- tests-clar/clar_libgit2.c | 3 + tests-clar/diff/diff_helpers.c | 43 ++++++++++++-- tests-clar/diff/diffiter.c | 10 ++-- tests-clar/diff/drivers.c | 7 +++ tests-clar/diff/patch.c | 8 ++- tests-clar/index/filemodes.c | 30 ++++++---- 9 files changed, 168 insertions(+), 54 deletions(-) diff --git a/include/git2/repository.h b/include/git2/repository.h index b4d561992..74ab65bf7 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -287,6 +287,23 @@ GIT_EXTERN(int) git_repository_init_ext( const char *repo_path, git_repository_init_options *opts); +/** + * Update the filesystem config settings for an open repository + * + * When a repository is initialized, config values are set based on the + * properties of the filesystem that the repository is on, such as + * "core.ignorecase", "core.filemode", "core.symlinks", etc. If the + * repository is moved to a new filesystem, these properties may no + * longer be correct and API calls may not behave as expected. This + * call reruns the phase of repository initialization that sets those + * properties to compensate for the current filesystem of the repo. + * + * @param repo A repository object + * @returrn 0 on success, < 0 on error + */ +GIT_EXTERN(int) git_repository_reset_filesystem( + git_repository *repo); + /** * Retrieve and resolve the reference pointed at by HEAD. * diff --git a/src/config.h b/src/config.h index 85db5e3e1..01e8465cc 100644 --- a/src/config.h +++ b/src/config.h @@ -47,7 +47,7 @@ extern int git_config_rename_section( * @param out the new backend * @param path where the config file is located */ -extern int git_config_file__ondisk(struct git_config_backend **out, const char *path); +extern int git_config_file__ondisk(git_config_backend **out, const char *path); extern int git_config__normalize_name(const char *in, char **out); diff --git a/src/repository.c b/src/repository.c index f609e5779..52509ffc1 100644 --- a/src/repository.c +++ b/src/repository.c @@ -962,34 +962,46 @@ static int create_empty_file(const char *path, mode_t mode) } static int repo_init_config( + git_config *parent, const char *repo_dir, const char *work_dir, - git_repository_init_options *opts) + uint32_t flags, + uint32_t mode) { int error = 0; - git_buf cfg_path = GIT_BUF_INIT; + git_buf buf = GIT_BUF_INIT; + const char *cfg_path = NULL; git_config *config = NULL; - bool is_bare = ((opts->flags & GIT_REPOSITORY_INIT_BARE) != 0); + bool is_bare = ((flags & GIT_REPOSITORY_INIT_BARE) != 0); #define SET_REPO_CONFIG(TYPE, NAME, VAL) do {\ if ((error = git_config_set_##TYPE(config, NAME, VAL)) < 0) \ goto cleanup; } while (0) - if (git_buf_joinpath(&cfg_path, repo_dir, GIT_CONFIG_FILENAME_INREPO) < 0) + if (git_buf_joinpath(&buf, repo_dir, GIT_CONFIG_FILENAME_INREPO) < 0) return -1; + cfg_path = git_buf_cstr(&buf); - if (!git_path_isfile(git_buf_cstr(&cfg_path)) && - create_empty_file(git_buf_cstr(&cfg_path), GIT_CONFIG_FILE_MODE) < 0) { - git_buf_free(&cfg_path); - return -1; + if (!git_path_isfile(cfg_path) && + (error = create_empty_file(cfg_path, GIT_CONFIG_FILE_MODE)) < 0) + goto cleanup; + + if (!parent) + error = git_config_open_ondisk(&config, cfg_path); + else if ((error = git_config_open_level( + &config, parent, GIT_CONFIG_LEVEL_LOCAL)) < 0) + { + giterr_clear(); + + if (!(error = git_config_add_file_ondisk( + parent, cfg_path, GIT_CONFIG_LEVEL_LOCAL, false))) + error = git_config_open_level( + &config, parent, GIT_CONFIG_LEVEL_LOCAL); } + if (error < 0) + goto cleanup; - if (git_config_open_ondisk(&config, git_buf_cstr(&cfg_path)) < 0) { - git_buf_free(&cfg_path); - return -1; - } - - if ((opts->flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0 && + if ((flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0 && (error = check_repositoryformatversion(config)) < 0) goto cleanup; @@ -998,7 +1010,7 @@ static int repo_init_config( SET_REPO_CONFIG( int32, "core.repositoryformatversion", GIT_REPO_VERSION); SET_REPO_CONFIG( - bool, "core.filemode", is_chmod_supported(git_buf_cstr(&cfg_path))); + bool, "core.filemode", is_chmod_supported(cfg_path)); #ifdef GIT_USE_ICONV SET_REPO_CONFIG( @@ -1009,38 +1021,70 @@ static int repo_init_config( if (!are_symlinks_supported(is_bare ? repo_dir : work_dir)) SET_REPO_CONFIG(bool, "core.symlinks", false); + /* core git does not do this on a reinit, but it is a property of + * the filesystem, so I think we should... + */ + if (!(flags & GIT_REPOSITORY_INIT__IS_REINIT) && + is_filesystem_case_insensitive(repo_dir)) + SET_REPO_CONFIG(bool, "core.ignorecase", true); + if (!is_bare) { SET_REPO_CONFIG(bool, "core.logallrefupdates", true); - if (!(opts->flags & GIT_REPOSITORY_INIT__NATURAL_WD)) { + if (!(flags & GIT_REPOSITORY_INIT__NATURAL_WD)) { SET_REPO_CONFIG(string, "core.worktree", work_dir); } - else if ((opts->flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0) { + else if ((flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0) { if (git_config_delete_entry(config, "core.worktree") < 0) giterr_clear(); } } - if (!(opts->flags & GIT_REPOSITORY_INIT__IS_REINIT) && - is_filesystem_case_insensitive(repo_dir)) - SET_REPO_CONFIG(bool, "core.ignorecase", true); - - if (opts->mode == GIT_REPOSITORY_INIT_SHARED_GROUP) { + if (mode == GIT_REPOSITORY_INIT_SHARED_GROUP) { SET_REPO_CONFIG(int32, "core.sharedrepository", 1); SET_REPO_CONFIG(bool, "receive.denyNonFastforwards", true); } - else if (opts->mode == GIT_REPOSITORY_INIT_SHARED_ALL) { + else if (mode == GIT_REPOSITORY_INIT_SHARED_ALL) { SET_REPO_CONFIG(int32, "core.sharedrepository", 2); SET_REPO_CONFIG(bool, "receive.denyNonFastforwards", true); } cleanup: - git_buf_free(&cfg_path); + git_buf_free(&buf); git_config_free(config); return error; } +int git_repository_reset_filesystem(git_repository *repo) +{ + int error = 0; + uint32_t flags = 0; + const char *repo_dir, *work_dir; + git_config *cfg; + + assert(repo); + + repo_dir = git_repository_path(repo); + work_dir = git_repository_workdir(repo); + + if (git_repository_is_bare(repo)) + flags |= GIT_REPOSITORY_INIT_BARE; + else if (!git__prefixcmp(repo_dir, work_dir) && + !strcmp(repo_dir + strlen(work_dir), DOT_GIT "/")) + flags |= GIT_REPOSITORY_INIT__NATURAL_WD; + + if ((error = git_repository_config(&cfg, repo)) < 0) + return error; + + error = repo_init_config(cfg, repo_dir, work_dir, flags, 0); + + git_config_free(cfg); + git_repository__cvar_cache_clear(repo); + + return error; +} + static int repo_write_template( const char *git_dir, bool allow_overwrite, @@ -1429,22 +1473,22 @@ int git_repository_init_ext( opts->flags |= GIT_REPOSITORY_INIT__IS_REINIT; error = repo_init_config( - git_buf_cstr(&repo_path), git_buf_cstr(&wd_path), opts); + NULL, repo_path.ptr, wd_path.ptr, opts->flags, opts->mode); /* TODO: reinitialize the templates */ } else { if (!(error = repo_init_structure( - git_buf_cstr(&repo_path), git_buf_cstr(&wd_path), opts)) && + repo_path.ptr, wd_path.ptr, opts)) && !(error = repo_init_config( - git_buf_cstr(&repo_path), git_buf_cstr(&wd_path), opts))) + NULL, repo_path.ptr, wd_path.ptr, opts->flags, opts->mode))) error = repo_init_create_head( - git_buf_cstr(&repo_path), opts->initial_head); + repo_path.ptr, opts->initial_head); } if (error < 0) goto cleanup; - error = git_repository_open(out, git_buf_cstr(&repo_path)); + error = git_repository_open(out, repo_path.ptr); if (!error && opts->origin_url) error = repo_init_create_origin(*out, opts->origin_url); diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index 0f8f26efb..c3abc1f95 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -190,6 +190,9 @@ git_repository *cl_git_sandbox_init(const char *sandbox) /* Now open the sandbox repository and make it available for tests */ cl_git_pass(git_repository_open(&_cl_repo, sandbox)); + /* Adjust configs after copying to new filesystem */ + cl_git_pass(git_repository_reset_filesystem(_cl_repo)); + return _cl_repo; } diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c index a5c322b4d..3452f231d 100644 --- a/tests-clar/diff/diff_helpers.c +++ b/tests-clar/diff/diff_helpers.c @@ -21,6 +21,35 @@ git_tree *resolve_commit_oid_to_tree( return tree; } +static char diff_pick_suffix(int mode) +{ + if (S_ISDIR(mode)) + return '/'; + else if (GIT_PERMS_IS_EXEC(mode)) + return '*'; + else + return ' '; +} + +static void fprintf_delta(FILE *fp, const git_diff_delta *delta, float progress) +{ + char code = git_diff_status_char(delta->status); + char old_suffix = diff_pick_suffix(delta->old_file.mode); + char new_suffix = diff_pick_suffix(delta->new_file.mode); + + fprintf(fp, "%c\t%s", code, delta->old_file.path); + + if ((delta->old_file.path != delta->new_file.path && + strcmp(delta->old_file.path, delta->new_file.path) != 0) || + (delta->old_file.mode != delta->new_file.mode && + delta->old_file.mode != 0 && delta->new_file.mode != 0)) + fprintf(fp, "%c %s%c", old_suffix, delta->new_file.path, new_suffix); + else if (old_suffix != ' ') + fprintf(fp, "%c", old_suffix); + + fprintf(fp, "\t[%.2f]\n", progress); +} + int diff_file_cb( const git_diff_delta *delta, float progress, @@ -29,9 +58,7 @@ int diff_file_cb( diff_expects *e = payload; if (e->debug) - fprintf(stderr, "%c %s (%.3f)\n", - git_diff_status_char(delta->status), - delta->old_file.path, progress); + fprintf_delta(stderr, delta, progress); if (e->names) cl_assert_equal_s(e->names[e->files], delta->old_file.path); @@ -55,8 +82,14 @@ int diff_print_file_cb( float progress, void *payload) { - fprintf(stderr, "%c %s\n", - git_diff_status_char(delta->status), delta->old_file.path); + if (!payload) { + fprintf_delta(stderr, delta, progress); + return 0; + } + + if (!((diff_expects *)payload)->debug) + fprintf_delta(stderr, delta, progress); + return diff_file_cb(delta, progress, payload); } diff --git a/tests-clar/diff/diffiter.c b/tests-clar/diff/diffiter.c index 932d720f2..ea5908475 100644 --- a/tests-clar/diff/diffiter.c +++ b/tests-clar/diff/diffiter.c @@ -27,25 +27,25 @@ void test_diff_diffiter__create(void) git_diff_list_free(diff); } -void test_diff_diffiter__iterate_files(void) +void test_diff_diffiter__iterate_files_1(void) { git_repository *repo = cl_git_sandbox_init("attr"); git_diff_list *diff; size_t d, num_d; - int count = 0; + diff_expects exp = { 0 }; cl_git_pass(git_diff_index_to_workdir(&diff, repo, NULL, NULL)); num_d = git_diff_num_deltas(diff); - cl_assert_equal_i(6, (int)num_d); for (d = 0; d < num_d; ++d) { const git_diff_delta *delta; cl_git_pass(git_diff_get_patch(NULL, &delta, diff, d)); cl_assert(delta != NULL); - count++; + + diff_file_cb(delta, (float)d / (float)num_d, &exp); } - cl_assert_equal_i(6, count); + cl_assert_equal_sz(6, exp.files); git_diff_list_free(diff); } diff --git a/tests-clar/diff/drivers.c b/tests-clar/diff/drivers.c index e02dd5c68..719d229fc 100644 --- a/tests-clar/diff/drivers.c +++ b/tests-clar/diff/drivers.c @@ -147,6 +147,13 @@ void test_diff_drivers__long_lines(void) cl_git_pass(git_diff_get_patch(&patch, NULL, diff, 0)); cl_git_pass(git_diff_patch_to_str(&actual, patch)); + /* if chmod not supported, overwrite mode bits since anything is possible */ + if (!cl_is_chmod_supported()) { + size_t actual_len = strlen(actual); + if (actual_len > 72 && memcmp(&actual[66], "100644", 6) != 0) + memcpy(&actual[66], "100644", 6); + } + cl_assert_equal_s(expected, actual); free(actual); diff --git a/tests-clar/diff/patch.c b/tests-clar/diff/patch.c index 6a33fa990..7aab8f409 100644 --- a/tests-clar/diff/patch.c +++ b/tests-clar/diff/patch.c @@ -238,6 +238,9 @@ void test_diff_patch__hunks_have_correct_line_numbers(void) cl_git_pass(git_config_new(&cfg)); git_repository_set_config(g_repo, cfg); + git_config_free(cfg); + + git_repository_reset_filesystem(g_repo); cl_git_pass( git_futils_readbuffer(&old_content, "renames/songof7cities.txt")); @@ -408,7 +411,6 @@ void test_diff_patch__hunks_have_correct_line_numbers(void) git_buf_free(&actual); git_buf_free(&old_content); git_tree_free(head); - git_config_free(cfg); } static void check_single_patch_stats( @@ -520,6 +522,9 @@ void test_diff_patch__line_counts_with_eofnl(void) cl_git_pass(git_config_new(&cfg)); git_repository_set_config(g_repo, cfg); + git_config_free(cfg); + + git_repository_reset_filesystem(g_repo); cl_git_pass(git_futils_readbuffer(&content, "renames/songof7cities.txt")); @@ -574,5 +579,4 @@ void test_diff_patch__line_counts_with_eofnl(void) g_repo, 1, 1, 1, 6, expected_sizes, expected); git_buf_free(&content); - git_config_free(cfg); } diff --git a/tests-clar/index/filemodes.c b/tests-clar/index/filemodes.c index 02f83efba..8cbd6ff3a 100644 --- a/tests-clar/index/filemodes.c +++ b/tests-clar/index/filemodes.c @@ -51,18 +51,24 @@ static void replace_file_with_mode( git_buf_free(&content); } -static void add_and_check_mode( - git_index *index, const char *filename, unsigned int expect_mode) +#define add_and_check_mode(I,F,X) add_and_check_mode_(I,F,X,__FILE__,__LINE__) + +static void add_and_check_mode_( + git_index *index, const char *filename, unsigned int expect_mode, + const char *file, int line) { size_t pos; const git_index_entry *entry; cl_git_pass(git_index_add_bypath(index, filename)); - cl_assert(!git_index_find(&pos, index, filename)); + clar__assert(!git_index_find(&pos, index, filename), + file, line, "Cannot find index entry", NULL, 1); entry = git_index_get_byindex(index, pos); - cl_assert(entry->mode == expect_mode); + + clar__assert_equal(file, line, "Expected mode does not match index", + 1, "%07o", (unsigned int)entry->mode, (unsigned int)expect_mode); } void test_index_filemodes__untrusted(void) @@ -91,16 +97,16 @@ void test_index_filemodes__untrusted(void) replace_file_with_mode("exec_on", "filemodes/exec_on.1", 0755); add_and_check_mode(index, "exec_on", GIT_FILEMODE_BLOB_EXECUTABLE); - /* 5 - add new 0644 -> expect 0644 */ - cl_git_write2file("filemodes/new_off", "blah", 0, - O_WRONLY | O_CREAT | O_TRUNC, 0644); - add_and_check_mode(index, "new_off", GIT_FILEMODE_BLOB); - - /* this test won't give predictable results on a platform - * that doesn't support filemodes correctly, so skip it. + /* these tests of newly added files won't give predictable results on + * filesystems without actual filemode support, so skip them. */ if (can_filemode) { - /* 6 - add 0755 -> expect 0755 */ + /* 5 - add new 0644 -> expect 0644 */ + cl_git_write2file("filemodes/new_off", "blah", 0, + O_WRONLY | O_CREAT | O_TRUNC, 0644); + add_and_check_mode(index, "new_off", GIT_FILEMODE_BLOB); + + /* 6 - add new 0755 -> expect 0755 */ cl_git_write2file("filemodes/new_on", "blah", 0, O_WRONLY | O_CREAT | O_TRUNC, 0755); add_and_check_mode(index, "new_on", GIT_FILEMODE_BLOB_EXECUTABLE); From 14997dc5f69e7ceebe502b32087d809a8482bf78 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 8 Oct 2013 12:45:43 -0700 Subject: [PATCH 11/14] More filemode cleanups for FAT on MacOS This cleans up some additional issues. The main change is that on a filesystem that doesn't support mode bits, libgit2 will now create new blobs with GIT_FILEMODE_BLOB always instead of being at the mercy to the filesystem driver to report executable or not. This means that if "core.filemode" lies and claims that filemode is not supported, then we will ignore the executable bit from the filesystem. Previously we would have allowed it. This adds an option to the new git_repository_reset_filesystem to recurse through submodules if desired. There may be other types of APIs that would like a "recurse submodules" option, but this one is particularly useful. This also has a number of cleanups, etc., for related things including trying to give better error messages when problems come up from the filesystem. For example, the FAT filesystem driver on MacOS appears to return errno EINVAL if you attempt to write a filename with invalid UTF-8 in it. We try to capture that with a better error message now. --- include/git2/repository.h | 4 +- src/checkout.c | 3 +- src/errors.c | 1 - src/fileops.c | 85 ++++------ src/index.c | 8 +- src/index.h | 2 +- src/iterator.c | 2 +- src/path.c | 33 +++- src/path.h | 4 + src/repository.c | 199 ++++++++++++++--------- src/submodule.c | 3 +- src/win32/posix_w32.c | 4 +- tests-clar/clar_libgit2.c | 2 +- tests-clar/diff/patch.c | 4 +- tests-clar/diff/workdir.c | 11 +- tests-clar/index/addall.c | 5 +- tests-clar/index/filemodes.c | 22 +-- tests-clar/refs/unicode.c | 27 ++- tests-clar/submodule/lookup.c | 15 +- tests-clar/submodule/modify.c | 15 +- tests-clar/submodule/submodule_helpers.c | 4 + 21 files changed, 244 insertions(+), 209 deletions(-) diff --git a/include/git2/repository.h b/include/git2/repository.h index 74ab65bf7..28d8400f2 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -299,10 +299,12 @@ GIT_EXTERN(int) git_repository_init_ext( * properties to compensate for the current filesystem of the repo. * * @param repo A repository object + * @param recurse_submodules Should submodules be reset recursively * @returrn 0 on success, < 0 on error */ GIT_EXTERN(int) git_repository_reset_filesystem( - git_repository *repo); + git_repository *repo, + int recurse_submodules); /** * Retrieve and resolve the reference pointed at by HEAD. diff --git a/src/checkout.c b/src/checkout.c index 5d741d393..d3f673d40 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -783,7 +783,8 @@ static int checkout_update_index( memset(&entry, 0, sizeof(entry)); entry.path = (char *)file->path; /* cast to prevent warning */ - git_index_entry__init_from_stat(&entry, st); + git_index_entry__init_from_stat( + &entry, st, !(git_index_caps(data->index) & GIT_INDEXCAP_NO_FILEMODE)); git_oid_cpy(&entry.oid, &file->oid); return git_index_add(data->index, &entry); diff --git a/src/errors.c b/src/errors.c index e2629f69e..c9d9e4e37 100644 --- a/src/errors.c +++ b/src/errors.c @@ -116,4 +116,3 @@ const git_error *giterr_last(void) { return GIT_GLOBAL->last_error; } - diff --git a/src/fileops.c b/src/fileops.c index be2e53ca8..63aedc6d0 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -78,11 +78,8 @@ int git_futils_creat_locked_withpath(const char *path, const mode_t dirmode, con int git_futils_open_ro(const char *path) { int fd = p_open(path, O_RDONLY); - if (fd < 0) { - if (errno == ENOENT || errno == ENOTDIR) - fd = GIT_ENOTFOUND; - giterr_set(GITERR_OS, "Failed to open '%s'", path); - } + if (fd < 0) + return git_path_set_error(errno, path, "open"); return fd; } @@ -138,7 +135,6 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) int git_futils_readbuffer_updated( git_buf *buf, const char *path, time_t *mtime, size_t *size, int *updated) { - int error = 0; git_file fd; struct stat st; bool changed = false; @@ -148,13 +144,8 @@ int git_futils_readbuffer_updated( if (updated != NULL) *updated = 0; - if (p_stat(path, &st) < 0) { - error = errno; - giterr_set(GITERR_OS, "Failed to stat '%s'", path); - if (error == ENOENT || error == ENOTDIR) - return GIT_ENOTFOUND; - return -1; - } + if (p_stat(path, &st) < 0) + return git_path_set_error(errno, path, "stat"); if (S_ISDIR(st.st_mode) || !git__is_sizet(st.st_size+1)) { giterr_set(GITERR_OS, "Invalid regular file stat for '%s'", path); @@ -441,66 +432,60 @@ static int futils__rm_first_parent(git_buf *path, const char *ceiling) static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path) { - struct stat st; futils__rmdir_data *data = opaque; + int error = data->error; + struct stat st; - if (data->depth > FUTILS_MAX_DEPTH) { - data->error = - futils__error_cannot_rmdir(path->ptr, "directory nesting too deep"); - } + if (data->depth > FUTILS_MAX_DEPTH) + error = futils__error_cannot_rmdir( + path->ptr, "directory nesting too deep"); - else if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) { + else if ((error = p_lstat_posixly(path->ptr, &st)) < 0) { if (errno == ENOENT) - data->error = 0; + 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); + 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"); + error = git_path_set_error(errno, path->ptr, "rmdir"); } else if (S_ISDIR(st.st_mode)) { data->depth++; - { - int error = - git_path_direach(path, 0, futils__rmdir_recurs_foreach, data); - if (error < 0) - return (error == GIT_EUSER) ? data->error : error; - } + error = git_path_direach(path, 0, futils__rmdir_recurs_foreach, data); + if (error < 0) + return (error == GIT_EUSER) ? data->error : error; data->depth--; if (data->depth == 0 && (data->flags & GIT_RMDIR_SKIP_ROOT) != 0) return data->error; - data->error = p_rmdir(path->ptr); - - if (data->error < 0) { + if ((error = p_rmdir(path->ptr)) < 0) { if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) != 0 && (errno == ENOTEMPTY || errno == EEXIST || errno == EBUSY)) - data->error = 0; + error = 0; else - futils__error_cannot_rmdir(path->ptr, NULL); + error = git_path_set_error(errno, path->ptr, "rmdir"); } } 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"); + if (p_unlink(path->ptr) < 0) + error = git_path_set_error(errno, path->ptr, "remove"); } else if ((data->flags & GIT_RMDIR_SKIP_NONEMPTY) == 0) - data->error = futils__error_cannot_rmdir(path->ptr, "still present"); + error = futils__error_cannot_rmdir(path->ptr, "still present"); - return data->error; + data->error = error; + return error; } static int futils__rmdir_empty_parent(void *opaque, git_buf *path) @@ -523,7 +508,7 @@ static int futils__rmdir_empty_parent(void *opaque, git_buf *path) giterr_clear(); error = GIT_ITEROVER; } else { - futils__error_cannot_rmdir(git_buf_cstr(path), NULL); + error = git_path_set_error(errno, git_buf_cstr(path), "rmdir"); } } @@ -818,11 +803,8 @@ int git_futils_cp(const char *from, const char *to, mode_t filemode) return ifd; if ((ofd = p_open(to, O_WRONLY | O_CREAT | O_EXCL, filemode)) < 0) { - if (errno == ENOENT || errno == ENOTDIR) - ofd = GIT_ENOTFOUND; - giterr_set(GITERR_OS, "Failed to open '%s' for writing", to); p_close(ifd); - return ofd; + return git_path_set_error(errno, to, "open for writing"); } return cp_by_fd(ifd, ofd, true); @@ -905,15 +887,14 @@ static int _cp_r_callback(void *ref, git_buf *from) goto exit; } - if (p_lstat(info->to.ptr, &to_st) < 0) { - if (errno != ENOENT && errno != ENOTDIR) { - giterr_set(GITERR_OS, - "Could not access %s while copying files", info->to.ptr); - error = -1; - goto exit; - } - } else + if (!(error = git_path_lstat(info->to.ptr, &to_st))) exists = true; + else if (error != GIT_ENOTFOUND) + goto exit; + else { + giterr_clear(); + error = 0; + } if ((error = git_path_lstat(from->ptr, &from_st)) < 0) goto exit; diff --git a/src/index.c b/src/index.c index 21a8d31d1..06394e6db 100644 --- a/src/index.c +++ b/src/index.c @@ -579,7 +579,8 @@ const git_index_entry *git_index_get_bypath( return git_index_get_byindex(index, pos); } -void git_index_entry__init_from_stat(git_index_entry *entry, struct stat *st) +void git_index_entry__init_from_stat( + git_index_entry *entry, struct stat *st, bool trust_mode) { entry->ctime.seconds = (git_time_t)st->st_ctime; entry->mtime.seconds = (git_time_t)st->st_mtime; @@ -587,7 +588,8 @@ void git_index_entry__init_from_stat(git_index_entry *entry, struct stat *st) /* entry->ctime.nanoseconds = st->st_ctimensec; */ entry->dev = st->st_rdev; entry->ino = st->st_ino; - entry->mode = index_create_mode(st->st_mode); + entry->mode = (!trust_mode && S_ISREG(st->st_mode)) ? + index_create_mode(0666) : index_create_mode(st->st_mode); entry->uid = st->st_uid; entry->gid = st->st_gid; entry->file_size = st->st_size; @@ -631,7 +633,7 @@ static int index_entry_init( entry = git__calloc(1, sizeof(git_index_entry)); GITERR_CHECK_ALLOC(entry); - git_index_entry__init_from_stat(entry, &st); + git_index_entry__init_from_stat(entry, &st, !index->distrust_filemode); entry->oid = oid; entry->path = git__strdup(rel_path); diff --git a/src/index.h b/src/index.h index 40577e105..4c448fabf 100644 --- a/src/index.h +++ b/src/index.h @@ -48,7 +48,7 @@ struct git_index_conflict_iterator { }; extern void git_index_entry__init_from_stat( - git_index_entry *entry, struct stat *st); + git_index_entry *entry, struct stat *st, bool trust_mode); extern size_t git_index__prefix_position(git_index *index, const char *path); diff --git a/src/iterator.c b/src/iterator.c index ea6b45e88..c0d7862ff 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -1175,7 +1175,7 @@ static int fs_iterator__update_entry(fs_iterator *fi) return GIT_ITEROVER; fi->entry.path = ps->path; - git_index_entry__init_from_stat(&fi->entry, &ps->st); + git_index_entry__init_from_stat(&fi->entry, &ps->st, true); /* need different mode here to keep directories during iteration */ fi->entry.mode = git_futils_canonical_mode(ps->st.st_mode); diff --git a/src/path.c b/src/path.c index 27abd062b..d45751cd1 100644 --- a/src/path.c +++ b/src/path.c @@ -538,16 +538,35 @@ bool git_path_is_empty_dir(const char *path) #endif +int git_path_set_error(int errno_value, const char *path, const char *action) +{ + switch (errno_value) { + case ENOENT: + case ENOTDIR: + giterr_set(GITERR_OS, "Could not find '%s' to %s", path, action); + return GIT_ENOTFOUND; + + case EINVAL: + case ENAMETOOLONG: + giterr_set(GITERR_OS, "Invalid path for filesystem '%s'", path); + return GIT_EINVALIDSPEC; + + case EEXIST: + giterr_set(GITERR_OS, "Failed %s - '%s' already exists", action, path); + return GIT_EEXISTS; + + default: + giterr_set(GITERR_OS, "Could not %s '%s'", action, path); + return -1; + } +} + int git_path_lstat(const char *path, struct stat *st) { - int err = 0; + if (p_lstat(path, st) == 0) + return 0; - if (p_lstat(path, st) < 0) { - err = (errno == ENOENT) ? GIT_ENOTFOUND : -1; - giterr_set(GITERR_OS, "Failed to stat file '%s'", path); - } - - return err; + return git_path_set_error(errno, path, "stat"); } static bool _check_dir_contents( diff --git a/src/path.h b/src/path.h index 2cfd714cd..175756938 100644 --- a/src/path.h +++ b/src/path.h @@ -358,6 +358,10 @@ extern int git_path_dirload_with_stat( const char *end_stat, git_vector *contents); +/* translate errno to libgit2 error code and set error message */ +extern int git_path_set_error( + int errno_value, const char *path, const char *action); + /* check if non-ascii characters are present in filename */ extern bool git_path_has_non_ascii(const char *path, size_t pathlen); diff --git a/src/repository.c b/src/repository.c index 52509ffc1..23cafe05a 100644 --- a/src/repository.c +++ b/src/repository.c @@ -961,80 +961,121 @@ static int create_empty_file(const char *path, mode_t mode) return 0; } +static int repo_local_config( + git_config **out, + git_buf *config_dir, + git_repository *repo, + const char *repo_dir) +{ + int error = 0; + git_config *parent; + const char *cfg_path; + + if (git_buf_joinpath(config_dir, repo_dir, GIT_CONFIG_FILENAME_INREPO) < 0) + return -1; + cfg_path = git_buf_cstr(config_dir); + + /* make LOCAL config if missing */ + if (!git_path_isfile(cfg_path) && + (error = create_empty_file(cfg_path, GIT_CONFIG_FILE_MODE)) < 0) + return error; + + /* if no repo, just open that file directly */ + if (!repo) + return git_config_open_ondisk(out, cfg_path); + + /* otherwise, open parent config and get that level */ + if ((error = git_repository_config__weakptr(&parent, repo)) < 0) + return error; + + if (git_config_open_level(out, parent, GIT_CONFIG_LEVEL_LOCAL) < 0) { + giterr_clear(); + + if (!(error = git_config_add_file_ondisk( + parent, cfg_path, GIT_CONFIG_LEVEL_LOCAL, false))) + error = git_config_open_level(out, parent, GIT_CONFIG_LEVEL_LOCAL); + } + + git_config_free(parent); + + return error; +} + +static int repo_init_fs_configs( + git_config *cfg, + const char *cfg_path, + const char *repo_dir, + const char *work_dir, + bool update_ignorecase) +{ + int error = 0; + + if (!work_dir) + work_dir = repo_dir; + + if ((error = git_config_set_bool( + cfg, "core.filemode", is_chmod_supported(cfg_path))) < 0) + return error; + + if (!are_symlinks_supported(work_dir)) { + if ((error = git_config_set_bool(cfg, "core.symlinks", false)) < 0) + return error; + } else if (git_config_delete_entry(cfg, "core.symlinks") < 0) + giterr_clear(); + + if (update_ignorecase) { + if (is_filesystem_case_insensitive(repo_dir)) { + if ((error = git_config_set_bool(cfg, "core.ignorecase", true)) < 0) + return error; + } else if (git_config_delete_entry(cfg, "core.ignorecase") < 0) + giterr_clear(); + } + +#ifdef GIT_USE_ICONV + if ((error = git_config_set_bool( + cfg, "core.precomposeunicode", + does_fs_decompose_unicode_paths(work_dir))) < 0) + return error; +#endif + + return 0; +} + static int repo_init_config( - git_config *parent, const char *repo_dir, const char *work_dir, uint32_t flags, uint32_t mode) { int error = 0; - git_buf buf = GIT_BUF_INIT; - const char *cfg_path = NULL; + git_buf cfg_path = GIT_BUF_INIT; git_config *config = NULL; bool is_bare = ((flags & GIT_REPOSITORY_INIT_BARE) != 0); + bool is_reinit = ((flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0); -#define SET_REPO_CONFIG(TYPE, NAME, VAL) do {\ + if ((error = repo_local_config(&config, &cfg_path, NULL, repo_dir)) < 0) + goto cleanup; + + if (is_reinit && (error = check_repositoryformatversion(config)) < 0) + goto cleanup; + +#define SET_REPO_CONFIG(TYPE, NAME, VAL) do { \ if ((error = git_config_set_##TYPE(config, NAME, VAL)) < 0) \ goto cleanup; } while (0) - if (git_buf_joinpath(&buf, repo_dir, GIT_CONFIG_FILENAME_INREPO) < 0) - return -1; - cfg_path = git_buf_cstr(&buf); + SET_REPO_CONFIG(bool, "core.bare", is_bare); + SET_REPO_CONFIG(int32, "core.repositoryformatversion", GIT_REPO_VERSION); - if (!git_path_isfile(cfg_path) && - (error = create_empty_file(cfg_path, GIT_CONFIG_FILE_MODE)) < 0) + if ((error = repo_init_fs_configs( + config, cfg_path.ptr, repo_dir, work_dir, !is_reinit)) < 0) goto cleanup; - if (!parent) - error = git_config_open_ondisk(&config, cfg_path); - else if ((error = git_config_open_level( - &config, parent, GIT_CONFIG_LEVEL_LOCAL)) < 0) - { - giterr_clear(); - - if (!(error = git_config_add_file_ondisk( - parent, cfg_path, GIT_CONFIG_LEVEL_LOCAL, false))) - error = git_config_open_level( - &config, parent, GIT_CONFIG_LEVEL_LOCAL); - } - if (error < 0) - goto cleanup; - - if ((flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0 && - (error = check_repositoryformatversion(config)) < 0) - goto cleanup; - - SET_REPO_CONFIG( - bool, "core.bare", is_bare); - SET_REPO_CONFIG( - int32, "core.repositoryformatversion", GIT_REPO_VERSION); - SET_REPO_CONFIG( - bool, "core.filemode", is_chmod_supported(cfg_path)); - -#ifdef GIT_USE_ICONV - SET_REPO_CONFIG( - bool, "core.precomposeunicode", - does_fs_decompose_unicode_paths(is_bare ? repo_dir : work_dir)); -#endif - - if (!are_symlinks_supported(is_bare ? repo_dir : work_dir)) - SET_REPO_CONFIG(bool, "core.symlinks", false); - - /* core git does not do this on a reinit, but it is a property of - * the filesystem, so I think we should... - */ - if (!(flags & GIT_REPOSITORY_INIT__IS_REINIT) && - is_filesystem_case_insensitive(repo_dir)) - SET_REPO_CONFIG(bool, "core.ignorecase", true); - if (!is_bare) { SET_REPO_CONFIG(bool, "core.logallrefupdates", true); - if (!(flags & GIT_REPOSITORY_INIT__NATURAL_WD)) { + if (!(flags & GIT_REPOSITORY_INIT__NATURAL_WD)) SET_REPO_CONFIG(string, "core.worktree", work_dir); - } - else if ((flags & GIT_REPOSITORY_INIT__IS_REINIT) != 0) { + else if (is_reinit) { if (git_config_delete_entry(config, "core.worktree") < 0) giterr_clear(); } @@ -1050,38 +1091,44 @@ static int repo_init_config( } cleanup: - git_buf_free(&buf); + git_buf_free(&cfg_path); git_config_free(config); return error; } -int git_repository_reset_filesystem(git_repository *repo) +static int repo_reset_submodule_fs(git_submodule *sm, const char *n, void *p) +{ + git_repository *smrepo = NULL; + GIT_UNUSED(n); GIT_UNUSED(p); + + if (git_submodule_open(&smrepo, sm) < 0 || + git_repository_reset_filesystem(smrepo, true) < 0) + giterr_clear(); + git_repository_free(smrepo); + + return 0; +} + +int git_repository_reset_filesystem(git_repository *repo, int recurse) { int error = 0; - uint32_t flags = 0; - const char *repo_dir, *work_dir; - git_config *cfg; + git_buf path = GIT_BUF_INIT; + git_config *config = NULL; + const char *repo_dir = git_repository_path(repo); - assert(repo); + if (!(error = repo_local_config(&config, &path, repo, repo_dir))) + error = repo_init_fs_configs( + config, path.ptr, repo_dir, git_repository_workdir(repo), true); - repo_dir = git_repository_path(repo); - work_dir = git_repository_workdir(repo); + git_config_free(config); + git_buf_free(&path); - if (git_repository_is_bare(repo)) - flags |= GIT_REPOSITORY_INIT_BARE; - else if (!git__prefixcmp(repo_dir, work_dir) && - !strcmp(repo_dir + strlen(work_dir), DOT_GIT "/")) - flags |= GIT_REPOSITORY_INIT__NATURAL_WD; - - if ((error = git_repository_config(&cfg, repo)) < 0) - return error; - - error = repo_init_config(cfg, repo_dir, work_dir, flags, 0); - - git_config_free(cfg); git_repository__cvar_cache_clear(repo); + if (!repo->is_bare && recurse) + (void)git_submodule_foreach(repo, repo_reset_submodule_fs, NULL); + return error; } @@ -1473,7 +1520,7 @@ int git_repository_init_ext( opts->flags |= GIT_REPOSITORY_INIT__IS_REINIT; error = repo_init_config( - NULL, repo_path.ptr, wd_path.ptr, opts->flags, opts->mode); + repo_path.ptr, wd_path.ptr, opts->flags, opts->mode); /* TODO: reinitialize the templates */ } @@ -1481,7 +1528,7 @@ int git_repository_init_ext( if (!(error = repo_init_structure( repo_path.ptr, wd_path.ptr, opts)) && !(error = repo_init_config( - NULL, repo_path.ptr, wd_path.ptr, opts->flags, opts->mode))) + repo_path.ptr, wd_path.ptr, opts->flags, opts->mode))) error = repo_init_create_head( repo_path.ptr, opts->initial_head); } diff --git a/src/submodule.c b/src/submodule.c index 121383b9c..12ade83fe 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -372,7 +372,8 @@ int git_submodule_add_to_index(git_submodule *sm, int write_index) memset(&entry, 0, sizeof(entry)); entry.path = sm->path; - git_index_entry__init_from_stat(&entry, &st); + git_index_entry__init_from_stat( + &entry, &st, !(git_index_caps(index) & GIT_INDEXCAP_NO_FILEMODE)); /* 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/win32/posix_w32.c b/src/win32/posix_w32.c index 2f490529c..18f717b0f 100644 --- a/src/win32/posix_w32.c +++ b/src/win32/posix_w32.c @@ -125,8 +125,8 @@ static int do_lstat( errno = ENOENT; - /* We need POSIX behavior, then ENOTDIR must set when any of the folders in the - * file path is a regular file,otherwise ENOENT must be set. + /* To match POSIX behavior, set ENOTDIR when any of the folders in the + * file path is a regular file, otherwise set ENOENT. */ if (posix_enotdir) { /* scan up path until we find an existing item */ diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index c3abc1f95..82ec5c065 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -191,7 +191,7 @@ git_repository *cl_git_sandbox_init(const char *sandbox) cl_git_pass(git_repository_open(&_cl_repo, sandbox)); /* Adjust configs after copying to new filesystem */ - cl_git_pass(git_repository_reset_filesystem(_cl_repo)); + cl_git_pass(git_repository_reset_filesystem(_cl_repo, 0)); return _cl_repo; } diff --git a/tests-clar/diff/patch.c b/tests-clar/diff/patch.c index 7aab8f409..1dbfc9ac5 100644 --- a/tests-clar/diff/patch.c +++ b/tests-clar/diff/patch.c @@ -240,7 +240,7 @@ void test_diff_patch__hunks_have_correct_line_numbers(void) git_repository_set_config(g_repo, cfg); git_config_free(cfg); - git_repository_reset_filesystem(g_repo); + git_repository_reset_filesystem(g_repo, false); cl_git_pass( git_futils_readbuffer(&old_content, "renames/songof7cities.txt")); @@ -524,7 +524,7 @@ void test_diff_patch__line_counts_with_eofnl(void) git_repository_set_config(g_repo, cfg); git_config_free(cfg); - git_repository_reset_filesystem(g_repo); + git_repository_reset_filesystem(g_repo, false); cl_git_pass(git_futils_readbuffer(&content, "renames/songof7cities.txt")); diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c index 6c17b41c6..aeef7b963 100644 --- a/tests-clar/diff/workdir.c +++ b/tests-clar/diff/workdir.c @@ -761,16 +761,7 @@ void test_diff_workdir__submodules(void) git_diff_list *diff = NULL; diff_expects exp; - g_repo = cl_git_sandbox_init("submod2"); - - cl_fixture_sandbox("submod2_target"); - p_rename("submod2_target/.gitted", "submod2_target/.git"); - - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); - p_rename("submod2/not/.gitted", "submod2/not/.git"); - - cl_fixture_cleanup("submod2_target"); + g_repo = setup_fixture_submod2(); a = resolve_commit_oid_to_tree(g_repo, a_commit); diff --git a/tests-clar/index/addall.c b/tests-clar/index/addall.c index f46a1e16c..3b5f5f22d 100644 --- a/tests-clar/index/addall.c +++ b/tests-clar/index/addall.c @@ -111,8 +111,9 @@ static void check_stat_data(git_index *index, const char *path, bool match) cl_assert(st.st_gid == entry->gid); cl_assert_equal_i_fmt( GIT_MODE_TYPE(st.st_mode), GIT_MODE_TYPE(entry->mode), "%07o"); - cl_assert_equal_b( - GIT_PERMS_IS_EXEC(st.st_mode), GIT_PERMS_IS_EXEC(entry->mode)); + if (cl_is_chmod_supported()) + cl_assert_equal_b( + GIT_PERMS_IS_EXEC(st.st_mode), GIT_PERMS_IS_EXEC(entry->mode)); } else { /* most things will still match */ cl_assert(st.st_size != entry->file_size); diff --git a/tests-clar/index/filemodes.c b/tests-clar/index/filemodes.c index 8cbd6ff3a..013932696 100644 --- a/tests-clar/index/filemodes.c +++ b/tests-clar/index/filemodes.c @@ -74,7 +74,6 @@ static void add_and_check_mode_( void test_index_filemodes__untrusted(void) { git_index *index; - bool can_filemode = cl_is_chmod_supported(); cl_repo_set_bool(g_repo, "core.filemode", false); @@ -97,20 +96,15 @@ void test_index_filemodes__untrusted(void) replace_file_with_mode("exec_on", "filemodes/exec_on.1", 0755); add_and_check_mode(index, "exec_on", GIT_FILEMODE_BLOB_EXECUTABLE); - /* these tests of newly added files won't give predictable results on - * filesystems without actual filemode support, so skip them. - */ - if (can_filemode) { - /* 5 - add new 0644 -> expect 0644 */ - cl_git_write2file("filemodes/new_off", "blah", 0, - O_WRONLY | O_CREAT | O_TRUNC, 0644); - add_and_check_mode(index, "new_off", GIT_FILEMODE_BLOB); + /* 5 - add new 0644 -> expect 0644 */ + cl_git_write2file("filemodes/new_off", "blah", 0, + O_WRONLY | O_CREAT | O_TRUNC, 0644); + add_and_check_mode(index, "new_off", GIT_FILEMODE_BLOB); - /* 6 - add new 0755 -> expect 0755 */ - cl_git_write2file("filemodes/new_on", "blah", 0, - O_WRONLY | O_CREAT | O_TRUNC, 0755); - add_and_check_mode(index, "new_on", GIT_FILEMODE_BLOB_EXECUTABLE); - } + /* 6 - add new 0755 -> expect 0644 if core.filemode == false */ + cl_git_write2file("filemodes/new_on", "blah", 0, + O_WRONLY | O_CREAT | O_TRUNC, 0755); + add_and_check_mode(index, "new_on", GIT_FILEMODE_BLOB); git_index_free(index); } diff --git a/tests-clar/refs/unicode.c b/tests-clar/refs/unicode.c index 2ec103275..b29c63e2b 100644 --- a/tests-clar/refs/unicode.c +++ b/tests-clar/refs/unicode.c @@ -22,23 +22,38 @@ void test_refs_unicode__create_and_lookup(void) git_reference *ref0, *ref1, *ref2; git_repository *repo2; - const char *REFNAME = "refs/heads/" "\305" "ngstr" "\366" "m"; + const char *REFNAME = "refs/heads/" "\303\205" "ngstr" "\303\266" "m"; + const char *REFNAME_DECOMPOSED = + "refs/heads/" "A" "\314\212" "ngstro" "\314\210" "m"; const char *master = "refs/heads/master"; /* Create the reference */ cl_git_pass(git_reference_lookup(&ref0, repo, master)); cl_git_pass(git_reference_create(&ref1, repo, REFNAME, git_reference_target(ref0), 0)); cl_assert_equal_s(REFNAME, git_reference_name(ref1)); + git_reference_free(ref0); /* Lookup the reference in a different instance of the repository */ cl_git_pass(git_repository_open(&repo2, "testrepo.git")); + cl_git_pass(git_reference_lookup(&ref2, repo2, REFNAME)); - - cl_assert(git_oid_cmp(git_reference_target(ref1), git_reference_target(ref2)) == 0); + cl_assert_equal_i( + 0, git_oid_cmp(git_reference_target(ref1), git_reference_target(ref2))); cl_assert_equal_s(REFNAME, git_reference_name(ref2)); - - git_reference_free(ref0); - git_reference_free(ref1); git_reference_free(ref2); + +#if GIT_USE_ICONV + /* Lookup reference by decomposed unicode name */ + + cl_git_pass(git_reference_lookup(&ref2, repo2, REFNAME_DECOMPOSED)); + cl_assert_equal_i( + 0, git_oid_cmp(git_reference_target(ref1), git_reference_target(ref2))); + cl_assert_equal_s(REFNAME, git_reference_name(ref2)); + git_reference_free(ref2); +#endif + + /* Cleanup */ + + git_reference_free(ref1); git_repository_free(repo2); } diff --git a/tests-clar/submodule/lookup.c b/tests-clar/submodule/lookup.c index b626cdf04..5f320e702 100644 --- a/tests-clar/submodule/lookup.c +++ b/tests-clar/submodule/lookup.c @@ -7,20 +7,7 @@ static git_repository *g_repo = NULL; void test_submodule_lookup__initialize(void) { - g_repo = cl_git_sandbox_init("submod2"); - - cl_fixture_sandbox("submod2_target"); - p_rename("submod2_target/.gitted", "submod2_target/.git"); - - /* must create submod2_target before rewrite so prettify will work */ - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); -} - -void test_submodule_lookup__cleanup(void) -{ - cl_git_sandbox_cleanup(); - cl_fixture_cleanup("submod2_target"); + g_repo = setup_fixture_submod2(); } void test_submodule_lookup__simple_lookup(void) diff --git a/tests-clar/submodule/modify.c b/tests-clar/submodule/modify.c index c0498ce6e..e326287a6 100644 --- a/tests-clar/submodule/modify.c +++ b/tests-clar/submodule/modify.c @@ -11,20 +11,7 @@ static git_repository *g_repo = NULL; void test_submodule_modify__initialize(void) { - g_repo = cl_git_sandbox_init("submod2"); - - cl_fixture_sandbox("submod2_target"); - p_rename("submod2_target/.gitted", "submod2_target/.git"); - - /* must create submod2_target before rewrite so prettify will work */ - rewrite_gitmodules(git_repository_workdir(g_repo)); - p_rename("submod2/not-submodule/.gitted", "submod2/not-submodule/.git"); -} - -void test_submodule_modify__cleanup(void) -{ - cl_git_sandbox_cleanup(); - cl_fixture_cleanup("submod2_target"); + g_repo = setup_fixture_submod2(); } void test_submodule_modify__add(void) diff --git a/tests-clar/submodule/submodule_helpers.c b/tests-clar/submodule/submodule_helpers.c index 3e79c77fd..cc3b7a2e3 100644 --- a/tests-clar/submodule/submodule_helpers.c +++ b/tests-clar/submodule/submodule_helpers.c @@ -102,6 +102,8 @@ git_repository *setup_fixture_submodules(void) cl_set_cleanup(cleanup_fixture_submodules, "testrepo.git"); + cl_git_pass(git_repository_reset_filesystem(repo, 1)); + return repo; } @@ -118,5 +120,7 @@ git_repository *setup_fixture_submod2(void) cl_set_cleanup(cleanup_fixture_submodules, "submod2_target"); + cl_git_pass(git_repository_reset_filesystem(repo, 1)); + return repo; } From d5e83627e4ed764115175dc42090afe0df332fe3 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 8 Oct 2013 14:41:32 -0700 Subject: [PATCH 12/14] Case sensitivity issues on Linux A couple of tests were actually dealing incorrectly with case sensitivity issues on Linux because they were relying on having core.ignorecase set to true. Now that the fixture initialization sets the case sensitivity to be accurate for the platform, it exposed bugs in these tests. --- tests-clar/status/renames.c | 14 +++++++------- tests-clar/submodule/status.c | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/tests-clar/status/renames.c b/tests-clar/status/renames.c index de84a574d..16fd02676 100644 --- a/tests-clar/status/renames.c +++ b/tests-clar/status/renames.c @@ -69,14 +69,14 @@ static void test_status( actual = git_status_byindex(status_list, i); expected = &expected_list[i]; - cl_assert_equal_i_fmt(expected->status, actual->status, "%04x"); - oldname = actual->head_to_index ? actual->head_to_index->old_file.path : actual->index_to_workdir ? actual->index_to_workdir->old_file.path : NULL; newname = actual->index_to_workdir ? actual->index_to_workdir->new_file.path : actual->head_to_index ? actual->head_to_index->new_file.path : NULL; + cl_assert_equal_i_fmt(expected->status, actual->status, "%04x"); + if (oldname) cl_assert(git__strcmp(oldname, expected->oldname) == 0); else @@ -507,14 +507,14 @@ void test_status_renames__both_casechange_two(void) "untimely.txt", "untimeliest.txt" } }; struct status_entry expected_case[] = { - { GIT_STATUS_INDEX_RENAMED | GIT_STATUS_INDEX_MODIFIED | - GIT_STATUS_WT_RENAMED | GIT_STATUS_WT_MODIFIED, - "ikeepsix.txt", "ikeepsix.txt" }, - { GIT_STATUS_INDEX_MODIFIED | GIT_STATUS_WT_RENAMED, - "sixserving.txt", "SixServing.txt" }, { GIT_STATUS_INDEX_RENAMED | GIT_STATUS_WT_MODIFIED | GIT_STATUS_WT_RENAMED, "songof7cities.txt", "SONGOF7.txt" }, + { GIT_STATUS_INDEX_MODIFIED | GIT_STATUS_WT_RENAMED, + "sixserving.txt", "SixServing.txt" }, + { GIT_STATUS_INDEX_RENAMED | GIT_STATUS_INDEX_MODIFIED | + GIT_STATUS_WT_RENAMED | GIT_STATUS_WT_MODIFIED, + "ikeepsix.txt", "ikeepsix.txt" }, { GIT_STATUS_INDEX_RENAMED | GIT_STATUS_WT_RENAMED, "untimely.txt", "untimeliest.txt" } }; diff --git a/tests-clar/submodule/status.c b/tests-clar/submodule/status.c index f1227a575..f5111c84f 100644 --- a/tests-clar/submodule/status.c +++ b/tests-clar/submodule/status.c @@ -388,7 +388,8 @@ void test_submodule_status__iterator(void) opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_UNMODIFIED | GIT_STATUS_OPT_INCLUDE_IGNORED | - GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS; + GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS | + GIT_STATUS_OPT_SORT_CASE_INSENSITIVELY; cl_git_pass(git_status_foreach_ext( g_repo, &opts, confirm_submodule_status, &exp)); From 92dac975869bf6207eca0754345dc9aa7fec8992 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 8 Oct 2013 16:35:57 -0700 Subject: [PATCH 13/14] Make reference lookups apply precomposeunicode Before these changes, looking up a reference would return the same precomposed or decomposed form of the reference name that was used to look it up, so on MacOS which ignores the difference between the two, a single reference could be looked up either way and git_reference_name would return the form of the name that was used to look it up! This change makes lookup always return the precomposed name if core.precomposeunicode is set regardless of which version was used to look it up. The reference iterator was already returning the precomposed form from earlier work. This also updates the CMakeLists.txt rules for enabling iconv usage because the clar tests for this code were actually not being activated properly with the old version. Finally, this moves git_repository_reset_filesystem from include/ git2/repository.h to include/git2/sys/repository.h since it is not really a function that normal library users should have to think about very often. --- CMakeLists.txt | 14 ++++++-- include/git2/refs.h | 8 ++--- include/git2/repository.h | 19 ---------- include/git2/sys/repository.h | 20 ++++++++++- src/path.h | 5 --- src/refs.c | 67 +++++++++++++++++++++++------------ src/refs.h | 2 ++ tests-clar/clar_libgit2.c | 1 + tests-clar/refs/unicode.c | 15 ++++---- 9 files changed, 87 insertions(+), 64 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 62ebbf4e5..c7414996b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,9 +27,14 @@ OPTION( BUILD_EXAMPLES "Build library usage example apps" OFF ) OPTION( TAGS "Generate tags" OFF ) OPTION( PROFILE "Generate profiling information" OFF ) OPTION( ENABLE_TRACE "Enables tracing support" OFF ) -OPTION( LIBGIT2_FILENAME "Name of the produced binary" OFF ) +OPTION( LIBGIT2_FILENAME "Name of the produced binary" OFF ) -OPTION( ANDROID "Build for android NDK" OFF ) +OPTION( ANDROID "Build for android NDK" OFF ) + +OPTION( USE_ICONV "Link with and use iconv library" OFF ) +IF(APPLE) + SET( USE_ICONV ON ) +ENDIF() IF(MSVC) # This option is only available when building with MSVC. By default, libgit2 @@ -58,8 +63,11 @@ FUNCTION(TARGET_OS_LIBRARIES target) TARGET_LINK_LIBRARIES(${target} socket nsl) ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux") TARGET_LINK_LIBRARIES(${target} rt) - ELSEIF(APPLE) + ENDIF() + + IF(USE_ICONV) TARGET_LINK_LIBRARIES(${target} iconv) + ADD_DEFINITIONS(-DGIT_USE_ICONV) ENDIF() IF(THREADSAFE) diff --git a/include/git2/refs.h b/include/git2/refs.h index 4871e9820..4041947f6 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -453,7 +453,7 @@ GIT_EXTERN(int) git_reference_is_remote(git_reference *ref); GIT_EXTERN(int) git_reference_is_tag(git_reference *ref); typedef enum { - GIT_REF_FORMAT_NORMAL = 0, + GIT_REF_FORMAT_NORMAL = 0u, /** * Control whether one-level refnames are accepted @@ -461,7 +461,7 @@ typedef enum { * components). Those are expected to be written only using * uppercase letters and underscore (FETCH_HEAD, ...) */ - GIT_REF_FORMAT_ALLOW_ONELEVEL = (1 << 0), + GIT_REF_FORMAT_ALLOW_ONELEVEL = (1u << 0), /** * Interpret the provided name as a reference pattern for a @@ -470,14 +470,14 @@ typedef enum { * in place of a one full pathname component * (e.g., foo//bar but not foo/bar). */ - GIT_REF_FORMAT_REFSPEC_PATTERN = (1 << 1), + GIT_REF_FORMAT_REFSPEC_PATTERN = (1u << 1), /** * Interpret the name as part of a refspec in shorthand form * so the `ONELEVEL` naming rules aren't enforced and 'master' * becomes a valid name. */ - GIT_REF_FORMAT_REFSPEC_SHORTHAND = (1 << 2), + GIT_REF_FORMAT_REFSPEC_SHORTHAND = (1u << 2), } git_reference_normalize_t; /** diff --git a/include/git2/repository.h b/include/git2/repository.h index 28d8400f2..b4d561992 100644 --- a/include/git2/repository.h +++ b/include/git2/repository.h @@ -287,25 +287,6 @@ GIT_EXTERN(int) git_repository_init_ext( const char *repo_path, git_repository_init_options *opts); -/** - * Update the filesystem config settings for an open repository - * - * When a repository is initialized, config values are set based on the - * properties of the filesystem that the repository is on, such as - * "core.ignorecase", "core.filemode", "core.symlinks", etc. If the - * repository is moved to a new filesystem, these properties may no - * longer be correct and API calls may not behave as expected. This - * call reruns the phase of repository initialization that sets those - * properties to compensate for the current filesystem of the repo. - * - * @param repo A repository object - * @param recurse_submodules Should submodules be reset recursively - * @returrn 0 on success, < 0 on error - */ -GIT_EXTERN(int) git_repository_reset_filesystem( - git_repository *repo, - int recurse_submodules); - /** * Retrieve and resolve the reference pointed at by HEAD. * diff --git a/include/git2/sys/repository.h b/include/git2/sys/repository.h index ba3d65ae5..e77b1dcba 100644 --- a/include/git2/sys/repository.h +++ b/include/git2/sys/repository.h @@ -27,7 +27,6 @@ GIT_BEGIN_DECL */ GIT_EXTERN(int) git_repository_new(git_repository **out); - /** * Reset all the internal state in a repository. * @@ -41,6 +40,25 @@ GIT_EXTERN(int) git_repository_new(git_repository **out); */ GIT_EXTERN(void) git_repository__cleanup(git_repository *repo); +/** + * Update the filesystem config settings for an open repository + * + * When a repository is initialized, config values are set based on the + * properties of the filesystem that the repository is on, such as + * "core.ignorecase", "core.filemode", "core.symlinks", etc. If the + * repository is moved to a new filesystem, these properties may no + * longer be correct and API calls may not behave as expected. This + * call reruns the phase of repository initialization that sets those + * properties to compensate for the current filesystem of the repo. + * + * @param repo A repository object + * @param recurse_submodules Should submodules be reset recursively + * @returrn 0 on success, < 0 on error + */ +GIT_EXTERN(int) git_repository_reset_filesystem( + git_repository *repo, + int recurse_submodules); + /** * Set the configuration file for this repository * diff --git a/src/path.h b/src/path.h index 175756938..eaf94d486 100644 --- a/src/path.h +++ b/src/path.h @@ -365,11 +365,6 @@ extern int git_path_set_error( /* check if non-ascii characters are present in filename */ extern bool git_path_has_non_ascii(const char *path, size_t pathlen); -/* only enable iconv on Mac for now */ -#ifdef __APPLE__ -#define GIT_USE_ICONV 1 -#endif - #define GIT_PATH_REPO_ENCODING "UTF-8" #ifdef __APPLE__ diff --git a/src/refs.c b/src/refs.c index c045ab9dc..0da02a666 100644 --- a/src/refs.c +++ b/src/refs.c @@ -138,6 +138,22 @@ int git_reference_name_to_id( return 0; } +static int reference_normalize_for_repo( + char *out, + size_t out_size, + git_repository *repo, + const char *name) +{ + int precompose; + unsigned int flags = GIT_REF_FORMAT_ALLOW_ONELEVEL; + + if (!git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) && + precompose) + flags |= GIT_REF_FORMAT__PRECOMPOSE_UNICODE; + + return git_reference_normalize_name(out, out_size, name, flags); +} + int git_reference_lookup_resolved( git_reference **ref_out, git_repository *repo, @@ -159,13 +175,13 @@ int git_reference_lookup_resolved( else if (max_nesting < 0) max_nesting = DEFAULT_NESTING_LEVEL; - strncpy(scan_name, name, GIT_REFNAME_MAX); scan_type = GIT_REF_SYMBOLIC; - if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0) - return -1; + if ((error = reference_normalize_for_repo( + scan_name, sizeof(scan_name), repo, name)) < 0) + return error; - if ((error = git_reference__normalize_name_lax(scan_name, GIT_REFNAME_MAX, name)) < 0) + if ((error = git_repository_refdb__weakptr(&refdb, repo)) < 0) return error; for (nesting = max_nesting; @@ -173,7 +189,7 @@ int git_reference_lookup_resolved( nesting--) { if (nesting != max_nesting) { - strncpy(scan_name, ref->target.symbolic, GIT_REFNAME_MAX); + strncpy(scan_name, ref->target.symbolic, sizeof(scan_name)); git_reference_free(ref); } @@ -711,17 +727,18 @@ static bool is_all_caps_and_underscore(const char *name, size_t len) return true; } +/* Inspired from https://github.com/git/git/blob/f06d47e7e0d9db709ee204ed13a8a7486149f494/refs.c#L36-100 */ int git_reference__normalize_name( git_buf *buf, const char *name, unsigned int flags) { - // Inspired from https://github.com/git/git/blob/f06d47e7e0d9db709ee204ed13a8a7486149f494/refs.c#L36-100 - char *current; int segment_len, segments_count = 0, error = GIT_EINVALIDSPEC; unsigned int process_flags; bool normalize = (buf != NULL); + git_path_iconv_t ic = GIT_PATH_ICONV_INIT; + assert(name); process_flags = flags; @@ -733,6 +750,13 @@ int git_reference__normalize_name( if (normalize) git_buf_clear(buf); + if ((flags & GIT_REF_FORMAT__PRECOMPOSE_UNICODE) != 0) { + size_t namelen = strlen(current); + if ((error = git_path_iconv_init_precompose(&ic)) < 0 || + (error = git_path_iconv(&ic, ¤t, &namelen)) < 0) + goto cleanup; + } + while (true) { segment_len = ensure_segment_validity(current); if (segment_len < 0) { @@ -809,6 +833,8 @@ cleanup: if (error && normalize) git_buf_free(buf); + git_path_iconv_clear(&ic); + return error; } @@ -983,9 +1009,9 @@ static int peel_error(int error, git_reference *ref, const char* msg) } int git_reference_peel( - git_object **peeled, - git_reference *ref, - git_otype target_type) + git_object **peeled, + git_reference *ref, + git_otype target_type) { git_reference *resolved = NULL; git_object *target = NULL; @@ -1027,24 +1053,19 @@ cleanup: return error; } -int git_reference__is_valid_name( - const char *refname, - unsigned int flags) +int git_reference__is_valid_name(const char *refname, unsigned int flags) { - int error; + if (git_reference__normalize_name(NULL, refname, flags) < 0) { + giterr_clear(); + return false; + } - error = git_reference__normalize_name(NULL, refname, flags) == 0; - giterr_clear(); - - return error; + return true; } -int git_reference_is_valid_name( - const char *refname) +int git_reference_is_valid_name(const char *refname) { - return git_reference__is_valid_name( - refname, - GIT_REF_FORMAT_ALLOW_ONELEVEL); + return git_reference__is_valid_name(refname, GIT_REF_FORMAT_ALLOW_ONELEVEL); } const char *git_reference_shorthand(git_reference *ref) diff --git a/src/refs.h b/src/refs.h index 4b91c25e8..80c7703fc 100644 --- a/src/refs.h +++ b/src/refs.h @@ -46,6 +46,8 @@ #define GIT_STASH_FILE "stash" #define GIT_REFS_STASH_FILE GIT_REFS_DIR GIT_STASH_FILE +#define GIT_REF_FORMAT__PRECOMPOSE_UNICODE (1u << 16) + #define GIT_REFNAME_MAX 1024 struct git_reference { diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index 82ec5c065..9f65953f5 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -1,6 +1,7 @@ #include "clar_libgit2.h" #include "posix.h" #include "path.h" +#include "git2/sys/repository.h" void cl_git_report_failure( int error, const char *file, int line, const char *fncall) diff --git a/tests-clar/refs/unicode.c b/tests-clar/refs/unicode.c index b29c63e2b..b56012869 100644 --- a/tests-clar/refs/unicode.c +++ b/tests-clar/refs/unicode.c @@ -4,17 +4,13 @@ static git_repository *repo; void test_refs_unicode__initialize(void) { - cl_fixture_sandbox("testrepo.git"); - - cl_git_pass(git_repository_open(&repo, "testrepo.git")); + repo = cl_git_sandbox_init("testrepo.git"); } void test_refs_unicode__cleanup(void) { - git_repository_free(repo); + cl_git_sandbox_cleanup(); repo = NULL; - - cl_fixture_cleanup("testrepo.git"); } void test_refs_unicode__create_and_lookup(void) @@ -23,13 +19,12 @@ void test_refs_unicode__create_and_lookup(void) git_repository *repo2; const char *REFNAME = "refs/heads/" "\303\205" "ngstr" "\303\266" "m"; - const char *REFNAME_DECOMPOSED = - "refs/heads/" "A" "\314\212" "ngstro" "\314\210" "m"; const char *master = "refs/heads/master"; /* Create the reference */ cl_git_pass(git_reference_lookup(&ref0, repo, master)); - cl_git_pass(git_reference_create(&ref1, repo, REFNAME, git_reference_target(ref0), 0)); + cl_git_pass(git_reference_create( + &ref1, repo, REFNAME, git_reference_target(ref0), 0)); cl_assert_equal_s(REFNAME, git_reference_name(ref1)); git_reference_free(ref0); @@ -45,6 +40,8 @@ void test_refs_unicode__create_and_lookup(void) #if GIT_USE_ICONV /* Lookup reference by decomposed unicode name */ +#define REFNAME_DECOMPOSED "refs/heads/" "A" "\314\212" "ngstro" "\314\210" "m" + cl_git_pass(git_reference_lookup(&ref2, repo2, REFNAME_DECOMPOSED)); cl_assert_equal_i( 0, git_oid_cmp(git_reference_target(ref1), git_reference_target(ref2))); From 867f7c9b3329952dc0aebf9770019a7a01ff2691 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 8 Oct 2013 16:59:59 -0700 Subject: [PATCH 14/14] Rename new fn to git_repository_reinit_filesystem --- include/git2/sys/repository.h | 4 ++-- src/repository.c | 8 ++++---- tests-clar/clar_libgit2.c | 2 +- tests-clar/diff/patch.c | 4 ++-- tests-clar/submodule/submodule_helpers.c | 5 +++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/git2/sys/repository.h b/include/git2/sys/repository.h index e77b1dcba..36f8b5836 100644 --- a/include/git2/sys/repository.h +++ b/include/git2/sys/repository.h @@ -52,10 +52,10 @@ GIT_EXTERN(void) git_repository__cleanup(git_repository *repo); * properties to compensate for the current filesystem of the repo. * * @param repo A repository object - * @param recurse_submodules Should submodules be reset recursively + * @param recurse_submodules Should submodules be updated recursively * @returrn 0 on success, < 0 on error */ -GIT_EXTERN(int) git_repository_reset_filesystem( +GIT_EXTERN(int) git_repository_reinit_filesystem( git_repository *repo, int recurse_submodules); diff --git a/src/repository.c b/src/repository.c index 23cafe05a..c5ce8425f 100644 --- a/src/repository.c +++ b/src/repository.c @@ -1097,20 +1097,20 @@ cleanup: return error; } -static int repo_reset_submodule_fs(git_submodule *sm, const char *n, void *p) +static int repo_reinit_submodule_fs(git_submodule *sm, const char *n, void *p) { git_repository *smrepo = NULL; GIT_UNUSED(n); GIT_UNUSED(p); if (git_submodule_open(&smrepo, sm) < 0 || - git_repository_reset_filesystem(smrepo, true) < 0) + git_repository_reinit_filesystem(smrepo, true) < 0) giterr_clear(); git_repository_free(smrepo); return 0; } -int git_repository_reset_filesystem(git_repository *repo, int recurse) +int git_repository_reinit_filesystem(git_repository *repo, int recurse) { int error = 0; git_buf path = GIT_BUF_INIT; @@ -1127,7 +1127,7 @@ int git_repository_reset_filesystem(git_repository *repo, int recurse) git_repository__cvar_cache_clear(repo); if (!repo->is_bare && recurse) - (void)git_submodule_foreach(repo, repo_reset_submodule_fs, NULL); + (void)git_submodule_foreach(repo, repo_reinit_submodule_fs, NULL); return error; } diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index 9f65953f5..50762cdb8 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -192,7 +192,7 @@ git_repository *cl_git_sandbox_init(const char *sandbox) cl_git_pass(git_repository_open(&_cl_repo, sandbox)); /* Adjust configs after copying to new filesystem */ - cl_git_pass(git_repository_reset_filesystem(_cl_repo, 0)); + cl_git_pass(git_repository_reinit_filesystem(_cl_repo, 0)); return _cl_repo; } diff --git a/tests-clar/diff/patch.c b/tests-clar/diff/patch.c index 1dbfc9ac5..171abc819 100644 --- a/tests-clar/diff/patch.c +++ b/tests-clar/diff/patch.c @@ -240,7 +240,7 @@ void test_diff_patch__hunks_have_correct_line_numbers(void) git_repository_set_config(g_repo, cfg); git_config_free(cfg); - git_repository_reset_filesystem(g_repo, false); + git_repository_reinit_filesystem(g_repo, false); cl_git_pass( git_futils_readbuffer(&old_content, "renames/songof7cities.txt")); @@ -524,7 +524,7 @@ void test_diff_patch__line_counts_with_eofnl(void) git_repository_set_config(g_repo, cfg); git_config_free(cfg); - git_repository_reset_filesystem(g_repo, false); + git_repository_reinit_filesystem(g_repo, false); cl_git_pass(git_futils_readbuffer(&content, "renames/songof7cities.txt")); diff --git a/tests-clar/submodule/submodule_helpers.c b/tests-clar/submodule/submodule_helpers.c index cc3b7a2e3..d5750675c 100644 --- a/tests-clar/submodule/submodule_helpers.c +++ b/tests-clar/submodule/submodule_helpers.c @@ -4,6 +4,7 @@ #include "util.h" #include "posix.h" #include "submodule_helpers.h" +#include "git2/sys/repository.h" /* rewrite gitmodules -> .gitmodules * rewrite the empty or relative urls inside each module @@ -102,7 +103,7 @@ git_repository *setup_fixture_submodules(void) cl_set_cleanup(cleanup_fixture_submodules, "testrepo.git"); - cl_git_pass(git_repository_reset_filesystem(repo, 1)); + cl_git_pass(git_repository_reinit_filesystem(repo, 1)); return repo; } @@ -120,7 +121,7 @@ git_repository *setup_fixture_submod2(void) cl_set_cleanup(cleanup_fixture_submodules, "submod2_target"); - cl_git_pass(git_repository_reset_filesystem(repo, 1)); + cl_git_pass(git_repository_reinit_filesystem(repo, 1)); return repo; }