From cf94024c589591e9a5c46a71f8fdce9f8b467161 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 4 Sep 2013 11:42:48 -0700 Subject: [PATCH 01/10] Update clar --- tests-clar/checkout/checkout_helpers.c | 4 +- tests-clar/clar.c | 136 +++++++++++++++++-------- tests-clar/clar.h | 23 +++-- tests-clar/clar/sandbox.h | 6 +- tests-clar/diff/submodules.c | 5 +- 5 files changed, 116 insertions(+), 58 deletions(-) diff --git a/tests-clar/checkout/checkout_helpers.c b/tests-clar/checkout/checkout_helpers.c index 8da024dda..f55f7b611 100644 --- a/tests-clar/checkout/checkout_helpers.c +++ b/tests-clar/checkout/checkout_helpers.c @@ -74,8 +74,8 @@ static void check_file_contents_internal( if (strip_cr) strip_cr_from_buf(&buf); - clar__assert_equal_i((int)expected_len, (int)buf.size, file, line, "strlen(expected_content) != strlen(actual_content)", 1); - clar__assert_equal_s(expected_content, buf.ptr, file, line, msg, 1); + clar__assert_equal(file, line, "strlen(expected_content) != strlen(actual_content)", 1, PRIuZ, expected_len, (size_t)buf.size); + clar__assert_equal(file, line, msg, 1, "%s", expected_content, buf.ptr); } void check_file_contents_at_line( diff --git a/tests-clar/clar.c b/tests-clar/clar.c index 585af8a74..5189e7919 100644 --- a/tests-clar/clar.c +++ b/tests-clar/clar.c @@ -24,28 +24,59 @@ # define _MAIN_CC __cdecl -# define stat(path, st) _stat(path, st) -# define mkdir(path, mode) _mkdir(path) -# define chdir(path) _chdir(path) -# define access(path, mode) _access(path, mode) -# define strdup(str) _strdup(str) -# define strcasecmp(a,b) _stricmp(a,b) +# ifndef stat +# define stat(path, st) _stat(path, st) +# endif +# ifndef mkdir +# define mkdir(path, mode) _mkdir(path) +# endif +# ifndef chdir +# define chdir(path) _chdir(path) +# endif +# ifndef access +# define access(path, mode) _access(path, mode) +# endif +# ifndef strdup +# define strdup(str) _strdup(str) +# endif +# ifndef strcasecmp +# define strcasecmp(a,b) _stricmp(a,b) +# endif # ifndef __MINGW32__ # pragma comment(lib, "shell32") -# define strncpy(to, from, to_size) strncpy_s(to, to_size, from, _TRUNCATE) -# define W_OK 02 -# define S_ISDIR(x) ((x & _S_IFDIR) != 0) -# define snprint_eq(buf,sz,fmt,...) _snprintf_s(buf,sz,_TRUNCATE,fmt,__VA_ARGS__) +# ifndef strncpy +# define strncpy(to, from, to_size) strncpy_s(to, to_size, from, _TRUNCATE) +# endif +# ifndef W_OK +# define W_OK 02 +# endif +# ifndef S_ISDIR +# define S_ISDIR(x) ((x & _S_IFDIR) != 0) +# endif +# define p_snprintf(buf,sz,fmt,...) _snprintf_s(buf,sz,_TRUNCATE,fmt,__VA_ARGS__) # else -# define snprint_eq snprintf +# define p_snprintf snprintf +# endif + +# ifndef PRIuZ +# define PRIuZ "Iu" +# endif +# ifndef PRIxZ +# define PRIxZ "Ix" # endif typedef struct _stat STAT_T; #else # include /* waitpid(2) */ # include # define _MAIN_CC -# define snprint_eq snprintf +# define p_snprintf snprintf +# ifndef PRIuZ +# define PRIuZ "zu" +# endif +# ifndef PRIxZ +# define PRIxZ "zx" +# endif typedef struct stat STAT_T; #endif @@ -406,45 +437,66 @@ void clar__assert( clar__fail(file, line, error_msg, description, should_abort); } -void clar__assert_equal_s( - const char *s1, - const char *s2, +void clar__assert_equal( const char *file, int line, const char *err, - int should_abort) + int should_abort, + const char *fmt, + ...) { - int match = (s1 == NULL || s2 == NULL) ? (s1 == s2) : (strcmp(s1, s2) == 0); + va_list args; + char buf[4096]; + int is_equal = 1; - if (!match) { - char buf[4096]; + va_start(args, fmt); - if (s1 && s2) { - int pos; - for (pos = 0; s1[pos] == s2[pos] && s1[pos] && s2[pos]; ++pos) - /* find differing byte offset */; - snprint_eq(buf, sizeof(buf), "'%s' != '%s' (at byte %d)", s1, s2, pos); - } else { - snprint_eq(buf, sizeof(buf), "'%s' != '%s'", s1, s2); + if (!strcmp("%s", fmt)) { + const char *s1 = va_arg(args, const char *); + const char *s2 = va_arg(args, const char *); + is_equal = (!s1 || !s2) ? (s1 == s2) : !strcmp(s1, s2); + + if (!is_equal) { + if (s1 && s2) { + int pos; + for (pos = 0; s1[pos] == s2[pos] && s1[pos] && s2[pos]; ++pos) + /* find differing byte offset */; + p_snprintf(buf, sizeof(buf), "'%s' != '%s' (at byte %d)", + s1, s2, pos); + } else { + p_snprintf(buf, sizeof(buf), "'%s' != '%s'", s1, s2); + } } - - clar__fail(file, line, err, buf, should_abort); } -} - -void clar__assert_equal_i( - int i1, - int i2, - const char *file, - int line, - const char *err, - int should_abort) -{ - if (i1 != i2) { - char buf[128]; - snprint_eq(buf, sizeof(buf), "%d != %d", i1, i2); - clar__fail(file, line, err, buf, should_abort); + else if (!strcmp(PRIuZ, fmt) || !strcmp(PRIxZ, fmt)) { + size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t); + is_equal = (sz1 == sz2); + if (!is_equal) { + int offset = p_snprintf(buf, sizeof(buf), fmt, sz1); + strncat(buf, " != ", sizeof(buf) - offset); + p_snprintf(buf + offset + 4, sizeof(buf) - offset - 4, fmt, sz2); + } } + else if (!strcmp("%p", fmt)) { + void *p1 = va_arg(args, void *), *p2 = va_arg(args, void *); + is_equal = (p1 == p2); + if (!is_equal) + p_snprintf(buf, sizeof(buf), "%p != %p", p1, p2); + } + else { + int i1 = va_arg(args, int), i2 = va_arg(args, int); + is_equal = (i1 == i2); + if (!is_equal) { + int offset = p_snprintf(buf, sizeof(buf), fmt, i1); + strncat(buf, " != ", sizeof(buf) - offset); + p_snprintf(buf + offset + 4, sizeof(buf) - offset - 4, fmt, i2); + } + } + + va_end(args); + + if (!is_equal) + clar__fail(file, line, err, buf, should_abort); } void cl_set_cleanup(void (*cleanup)(void *), void *opaque) diff --git a/tests-clar/clar.h b/tests-clar/clar.h index d92318bd4..e1f244eba 100644 --- a/tests-clar/clar.h +++ b/tests-clar/clar.h @@ -57,15 +57,17 @@ void cl_fixture_cleanup(const char *fixture_name); /** * Typed assertion macros */ -#define cl_assert_equal_s(s1,s2) clar__assert_equal_s((s1),(s2),__FILE__,__LINE__,"String mismatch: " #s1 " != " #s2, 1) -#define cl_assert_equal_s_(s1,s2,note) clar__assert_equal_s((s1),(s2),__FILE__,__LINE__,"String mismatch: " #s1 " != " #s2 " (" #note ")", 1) +#define cl_assert_equal_s(s1,s2) clar__assert_equal(__FILE__,__LINE__,"String mismatch: " #s1 " != " #s2, 1, "%s", (s1), (s2)) +#define cl_assert_equal_s_(s1,s2,note) clar__assert_equal(__FILE__,__LINE__,"String mismatch: " #s1 " != " #s2 " (" #note ")", 1, "%s", (s1), (s2)) -#define cl_assert_equal_i(i1,i2) clar__assert_equal_i((i1),(i2),__FILE__,__LINE__,#i1 " != " #i2, 1) -#define cl_assert_equal_i_(i1,i2,note) clar__assert_equal_i((i1),(i2),__FILE__,__LINE__,#i1 " != " #i2 " (" #note ")", 1) +#define cl_assert_equal_i(i1,i2) clar__assert_equal(__FILE__,__LINE__,#i1 " != " #i2, 1, "%d", (int)(i1), (int)(i2)) +#define cl_assert_equal_i_(i1,i2,note) clar__assert_equal(__FILE__,__LINE__,#i1 " != " #i2 " (" #note ")", 1, "%d", (i1), (i2)) +#define cl_assert_equal_i_fmt(i1,i2,fmt) clar__assert_equal(__FILE__,__LINE__,#i1 " != " #i2, 1, (fmt), (int)(i1), (int)(i2)) -#define cl_assert_equal_b(b1,b2) clar__assert_equal_i(!!(b1),!!(b2),__FILE__,__LINE__,#b1 " != " #b2, 1) +#define cl_assert_equal_b(b1,b2) clar__assert_equal(__FILE__,__LINE__,#b1 " != " #b2, 1, "%d", (int)((b1) != 0),(int)((b2) != 0)) + +#define cl_assert_equal_p(p1,p2) clar__assert_equal(__FILE__,__LINE__,"Pointer mismatch: " #p1 " != " #p2, 1, "%p", (p1), (p2)) -#define cl_assert_equal_p(p1,p2) cl_assert((p1) == (p2)) void clar__fail( const char *file, @@ -82,7 +84,12 @@ void clar__assert( const char *description, int should_abort); -void clar__assert_equal_s(const char *,const char *,const char *,int,const char *,int); -void clar__assert_equal_i(int,int,const char *,int,const char *,int); +void clar__assert_equal( + const char *file, + int line, + const char *err, + int should_abort, + const char *fmt, + ...); #endif diff --git a/tests-clar/clar/sandbox.h b/tests-clar/clar/sandbox.h index 1ca6fcae8..ee7564148 100644 --- a/tests-clar/clar/sandbox.h +++ b/tests-clar/clar/sandbox.h @@ -43,10 +43,8 @@ find_tmp_path(char *buffer, size_t length) } #else - DWORD env_len; - - if ((env_len = GetEnvironmentVariable("CLAR_TMP", buffer, length)) > 0 && - env_len < length) + DWORD env_len = GetEnvironmentVariable("CLAR_TMP", buffer, (DWORD)length); + if (env_len > 0 && env_len < (DWORD)length) return 0; if (GetTempPath((DWORD)length, buffer)) diff --git a/tests-clar/diff/submodules.c b/tests-clar/diff/submodules.c index 94804db22..9dcf8194e 100644 --- a/tests-clar/diff/submodules.c +++ b/tests-clar/diff/submodules.c @@ -39,8 +39,9 @@ static void check_diff_patches_at_line( cl_git_pass(git_diff_patch_to_str(&patch_text, patch)); - clar__assert_equal_s(expected[d], patch_text, file, line, - "expected diff did not match actual diff", 1); + clar__assert_equal( + file, line, "expected diff did not match actual diff", 1, + "%s", expected[d], patch_text); git__free(patch_text); } From 780f3e540fa6492b244ef2750a71e39407526c1e Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 4 Sep 2013 16:13:18 -0700 Subject: [PATCH 02/10] Make tests take umask into account It seems that libgit2 is correctly applying the umask when initializing a repository from a template and when creating new directories during checkout, but the test suite is not accounting for possible variations due to the umask. This updates that so that the test suite will work regardless of the umask. --- tests-clar/checkout/index.c | 10 +++++++--- tests-clar/clar_libgit2.h | 2 +- tests-clar/repo/init.c | 15 +++++++++++++-- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index 982bf9ee5..a185aec44 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -229,6 +229,7 @@ void test_checkout_index__options_dir_modes(void) struct stat st; git_oid oid; git_commit *commit; + mode_t um; cl_git_pass(git_reference_name_to_id(&oid, g_repo, "refs/heads/dir")); cl_git_pass(git_commit_lookup(&commit, g_repo, &oid)); @@ -240,12 +241,15 @@ void test_checkout_index__options_dir_modes(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); + /* umask will influence actual directory creation mode */ + (void)p_umask(um = p_umask(022)); + cl_git_pass(p_stat("./testrepo/a", &st)); - cl_assert_equal_i(st.st_mode & 0777, 0701); + cl_assert_equal_i_fmt(st.st_mode, GIT_FILEMODE_TREE | 0701 & ~um, "%07o"); /* File-mode test, since we're on the 'dir' branch */ cl_git_pass(p_stat("./testrepo/a/b.txt", &st)); - cl_assert_equal_i(st.st_mode & 0777, 0755); + cl_assert_equal_i_fmt(st.st_mode, GIT_FILEMODE_BLOB_EXECUTABLE, "%07o"); git_commit_free(commit); #endif @@ -263,7 +267,7 @@ void test_checkout_index__options_override_file_modes(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); cl_git_pass(p_stat("./testrepo/new.txt", &st)); - cl_assert_equal_i(st.st_mode & 0777, 0700); + cl_assert_equal_i_fmt(st.st_mode & 0777, 0700, "%07o"); #endif } diff --git a/tests-clar/clar_libgit2.h b/tests-clar/clar_libgit2.h index 8c8357e40..080d32bea 100644 --- a/tests-clar/clar_libgit2.h +++ b/tests-clar/clar_libgit2.h @@ -32,7 +32,7 @@ void cl_git_report_failure(int, const char *, int, const char *); #define cl_assert_at_line(expr,file,line) \ clar__assert((expr) != 0, file, line, "Expression is not true: " #expr, NULL, 1) -#define cl_assert_equal_sz(sz1,sz2) cl_assert_equal_i((int)sz1, (int)(sz2)) +#define cl_assert_equal_sz(sz1,sz2) clar__assert_equal(__FILE__,__LINE__,#sz1 " != " #sz2, 1, PRIuZ, (size_t)(sz1), (size_t)(sz2)) GIT_INLINE(void) clar__assert_in_range( int lo, int val, int hi, diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 5076184b8..4d93f3d0e 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -10,10 +10,17 @@ enum repo_mode { }; static git_repository *_repo = NULL; +static mode_t _umask = 0; void test_repo_init__initialize(void) { _repo = NULL; + + /* load umask if not already loaded */ + if (!_umask) { + _umask = p_umask(022); + (void)p_umask(_umask); + } } static void cleanup_repository(void *path) @@ -377,14 +384,18 @@ static void assert_hooks_match( cl_git_pass(git_buf_joinpath(&actual, repo_dir, hook_path)); cl_git_pass(git_path_lstat(actual.ptr, &st)); - cl_assert(expected_st.st_size == st.st_size); + cl_assert_equal_sz(expected_st.st_size, st.st_size); + + expected_st.st_mode = + (expected_st.st_mode & ~0777) | + (((expected_st.st_mode & 0111) ? 0100777 : 0100666) & ~_umask); if (!core_filemode) { expected_st.st_mode = expected_st.st_mode & ~0111; st.st_mode = st.st_mode & ~0111; } - cl_assert_equal_i((int)expected_st.st_mode, (int)st.st_mode); + cl_assert_equal_i_fmt(expected_st.st_mode, st.st_mode, "%07o"); git_buf_free(&expected); git_buf_free(&actual); From abfed59c2797e9436f2b20fadecd982d6637dc22 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 4 Sep 2013 16:21:18 -0700 Subject: [PATCH 03/10] Clean up one other mode_t assertion --- tests-clar/core/mkdir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests-clar/core/mkdir.c b/tests-clar/core/mkdir.c index 1e50b4336..a969e4de2 100644 --- a/tests-clar/core/mkdir.c +++ b/tests-clar/core/mkdir.c @@ -115,9 +115,9 @@ static void check_mode(mode_t expected, mode_t actual) { #ifdef GIT_WIN32 /* chmod on Win32 doesn't support exec bit, not group/world bits */ - cl_assert((expected & 0600) == (actual & 0777)); + cl_assert_equal_i_fmt((expected & 0600), (actual & 0777), "%07o"); #else - cl_assert(expected == (actual & 0777)); + cl_assert_equal_i_fmt(expected, (actual & 0777), "%07o"); #endif } From 2a54c7f447acfc74368a766369a9923ecdf19b21 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 4 Sep 2013 16:24:36 -0700 Subject: [PATCH 04/10] _umask is function name on Windows --- tests-clar/repo/init.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 4d93f3d0e..aeb35d3b4 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -10,16 +10,16 @@ enum repo_mode { }; static git_repository *_repo = NULL; -static mode_t _umask = 0; +static mode_t g_umask = 0; void test_repo_init__initialize(void) { _repo = NULL; /* load umask if not already loaded */ - if (!_umask) { - _umask = p_umask(022); - (void)p_umask(_umask); + if (!g_umask) { + g_umask = p_umask(022); + (void)p_umask(g_umask); } } @@ -388,7 +388,7 @@ static void assert_hooks_match( expected_st.st_mode = (expected_st.st_mode & ~0777) | - (((expected_st.st_mode & 0111) ? 0100777 : 0100666) & ~_umask); + (((expected_st.st_mode & 0111) ? 0100777 : 0100666) & ~g_umask); if (!core_filemode) { expected_st.st_mode = expected_st.st_mode & ~0111; From 9ce4f7da4a69b3853da587b67e8f137ddf036e4c Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Wed, 4 Sep 2013 16:41:34 -0700 Subject: [PATCH 05/10] Fix tests to use core.filemode correctly Some windows tests were failing --- tests-clar/clar_libgit2.c | 10 ++++++++++ tests-clar/clar_libgit2.h | 1 + tests-clar/repo/init.c | 26 +++++++++++--------------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/tests-clar/clar_libgit2.c b/tests-clar/clar_libgit2.c index bf35a68eb..340943ca8 100644 --- a/tests-clar/clar_libgit2.c +++ b/tests-clar/clar_libgit2.c @@ -344,3 +344,13 @@ void cl_repo_set_bool(git_repository *repo, const char *cfg, int value) cl_git_pass(git_config_set_bool(config, cfg, value != 0)); git_config_free(config); } + +int cl_repo_get_bool(git_repository *repo, const char *cfg) +{ + int val = 0; + git_config *config; + cl_git_pass(git_repository_config(&config, repo)); + cl_git_pass(git_config_get_bool(&val, config, cfg));; + git_config_free(config); + return val; +} diff --git a/tests-clar/clar_libgit2.h b/tests-clar/clar_libgit2.h index 080d32bea..3cb0607f1 100644 --- a/tests-clar/clar_libgit2.h +++ b/tests-clar/clar_libgit2.h @@ -88,5 +88,6 @@ int cl_git_remove_placeholders(const char *directory_path, const char *filename) /* config setting helpers */ void cl_repo_set_bool(git_repository *repo, const char *cfg, int value); +int cl_repo_get_bool(git_repository *repo, const char *cfg); #endif diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index aeb35d3b4..62e4ecd59 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -270,7 +270,6 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void) void test_repo_init__reinit_overwrites_filemode(void) { - git_config *config; int expected, current_value; #ifdef GIT_WIN32 @@ -291,13 +290,10 @@ void test_repo_init__reinit_overwrites_filemode(void) /* Reinit the repository */ cl_git_pass(git_repository_init(&_repo, "overwrite.git", 1)); - git_repository_config(&config, _repo); /* Ensure the "core.filemode" config value has been reset */ - cl_git_pass(git_config_get_bool(¤t_value, config, "core.filemode")); + current_value = cl_repo_get_bool(_repo, "core.filemode"); cl_assert_equal_i(expected, current_value); - - git_config_free(config); } void test_repo_init__sets_logAllRefUpdates_according_to_type_of_repository(void) @@ -391,8 +387,8 @@ static void assert_hooks_match( (((expected_st.st_mode & 0111) ? 0100777 : 0100666) & ~g_umask); if (!core_filemode) { - expected_st.st_mode = expected_st.st_mode & ~0111; - st.st_mode = st.st_mode & ~0111; + expected_st.st_mode = expected_st.st_mode & ~0177; + st.st_mode = st.st_mode & ~0177; } cl_assert_equal_i_fmt(expected_st.st_mode, st.st_mode, "%07o"); @@ -438,6 +434,7 @@ void test_repo_init__extended_with_template(void) git_buf expected = GIT_BUF_INIT; git_buf actual = GIT_BUF_INIT; git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; + int filemode; cl_set_cleanup(&cleanup_repository, "templated.git"); @@ -461,13 +458,15 @@ void test_repo_init__extended_with_template(void) git_buf_free(&expected); git_buf_free(&actual); - assert_hooks_match( - cl_fixture("template"), git_repository_path(_repo), - "hooks/update.sample", true); + filemode = cl_repo_get_bool(_repo, "core.filemode"); assert_hooks_match( cl_fixture("template"), git_repository_path(_repo), - "hooks/link.sample", true); + "hooks/update.sample", filemode); + + assert_hooks_match( + cl_fixture("template"), git_repository_path(_repo), + "hooks/link.sample", filemode); } void test_repo_init__extended_with_template_and_shared_mode(void) @@ -475,7 +474,6 @@ void test_repo_init__extended_with_template_and_shared_mode(void) git_buf expected = GIT_BUF_INIT; git_buf actual = GIT_BUF_INIT; git_repository_init_options opts = GIT_REPOSITORY_INIT_OPTIONS_INIT; - git_config *config; int filemode = true; const char *repo_path = NULL; @@ -491,9 +489,7 @@ void test_repo_init__extended_with_template_and_shared_mode(void) cl_assert(!git_repository_is_bare(_repo)); cl_assert(!git__suffixcmp(git_repository_path(_repo), "/init_shared_from_tpl/.git/")); - cl_git_pass(git_repository_config(&config, _repo)); - cl_git_pass(git_config_get_bool(&filemode, config, "core.filemode")); - git_config_free(config); + filemode = cl_repo_get_bool(_repo, "core.filemode"); cl_git_pass(git_futils_readbuffer( &expected, cl_fixture("template/description"))); From 27061b151a7e0225186365ee0b5ca802d68782a9 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 5 Sep 2013 10:25:16 -0700 Subject: [PATCH 06/10] Fix some newer GCC compiler warnings --- tests-clar/checkout/index.c | 2 +- tests-clar/diff/rename.c | 3 --- tests-clar/odb/backend/nonrefreshing.c | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index a185aec44..c9352d8ce 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -245,7 +245,7 @@ void test_checkout_index__options_dir_modes(void) (void)p_umask(um = p_umask(022)); cl_git_pass(p_stat("./testrepo/a", &st)); - cl_assert_equal_i_fmt(st.st_mode, GIT_FILEMODE_TREE | 0701 & ~um, "%07o"); + cl_assert_equal_i_fmt(st.st_mode, (GIT_FILEMODE_TREE | 0701) & ~um, "%07o"); /* File-mode test, since we're on the 'dir' branch */ cl_git_pass(p_stat("./testrepo/a/b.txt", &st)); diff --git a/tests-clar/diff/rename.c b/tests-clar/diff/rename.c index ac3814d59..b5a9935fd 100644 --- a/tests-clar/diff/rename.c +++ b/tests-clar/diff/rename.c @@ -1100,7 +1100,6 @@ void test_diff_rename__can_rename_from_rewrite(void) { git_index *index; git_tree *tree; - git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT; git_diff_list *diff; git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT; git_diff_find_options findopts = GIT_DIFF_FIND_OPTIONS_INIT; @@ -1110,8 +1109,6 @@ void test_diff_rename__can_rename_from_rewrite(void) const char *targets[] = { "songof7cities.txt", "this-is-a-rename.txt" }; struct rename_expected expect = { 2, status, sources, targets }; - opts.checkout_strategy = GIT_CHECKOUT_FORCE; - cl_git_pass(git_repository_index(&index, g_repo)); cl_git_pass(p_rename("renames/songof7cities.txt", "renames/this-is-a-rename.txt")); diff --git a/tests-clar/odb/backend/nonrefreshing.c b/tests-clar/odb/backend/nonrefreshing.c index abb824d4b..9abca2bd3 100644 --- a/tests-clar/odb/backend/nonrefreshing.c +++ b/tests-clar/odb/backend/nonrefreshing.c @@ -132,8 +132,8 @@ static int build_fake_backend( static void setup_repository_and_backend(git_error_code error_code) { - git_odb *odb; - git_odb_backend *backend; + git_odb *odb = NULL; + git_odb_backend *backend = NULL; _repo = cl_git_sandbox_init("testrepo.git"); From f240acce865ec14df0d517d5000316a933e7ffed Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 5 Sep 2013 11:20:12 -0700 Subject: [PATCH 07/10] Add more file mode permissions macros This adds some more macros for some standard operations on file modes, particularly related to permissions, and then updates a number of places around the code base to use the new macros. --- src/checkout.c | 13 +++++-------- src/diff_print.c | 4 ++-- src/fileops.c | 4 ++-- src/fileops.h | 5 ++++- src/index.c | 2 +- src/tree.c | 12 ++++++------ 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index ec9da7e2e..f3a9b343d 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -693,17 +693,14 @@ static int buffer_to_file( buffer, path, file_open_flags, file_mode)) < 0) return error; - if (st != NULL && (error = p_stat(path, st)) < 0) { - giterr_set(GITERR_OS, "Error while statting '%s'", path); - return error; - } + if (st != NULL && (error = p_stat(path, st)) < 0) + giterr_set(GITERR_OS, "Error statting '%s'", path); - if ((file_mode & 0100) != 0 && (error = p_chmod(path, file_mode)) < 0) { + else if (GIT_PERMS_EXECUTABLE(file_mode) && + (error = p_chmod(path, file_mode)) < 0) giterr_set(GITERR_OS, "Failed to set permissions on '%s'", path); - return error; - } - return 0; + return error; } static int blob_content_to_file( diff --git a/src/diff_print.c b/src/diff_print.c index 4ddd72443..96937d84d 100644 --- a/src/diff_print.c +++ b/src/diff_print.c @@ -7,7 +7,7 @@ #include "common.h" #include "diff.h" #include "diff_patch.h" -#include "buffer.h" +#include "fileops.h" typedef struct { git_diff_list *diff; @@ -46,7 +46,7 @@ static char diff_pick_suffix(int mode) { if (S_ISDIR(mode)) return '/'; - else if (mode & 0100) /* -V536 */ + else if (GIT_PERMS_EXECUTABLE(mode)) /* -V536 */ /* in git, modes are very regular, so we must have 0100755 mode */ return '*'; else diff --git a/src/fileops.c b/src/fileops.c index 76119e02e..92cda82e7 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -110,7 +110,7 @@ git_off_t git_futils_filesize(git_file fd) mode_t git_futils_canonical_mode(mode_t raw_mode) { if (S_ISREG(raw_mode)) - return S_IFREG | GIT_CANONICAL_PERMS(raw_mode); + return S_IFREG | GIT_PERMS_CANONICAL(raw_mode); else if (S_ISLNK(raw_mode)) return S_IFLNK; else if (S_ISGITLINK(raw_mode)) @@ -972,7 +972,7 @@ static int _cp_r_callback(void *ref, git_buf *from) mode_t usemode = from_st.st_mode; if ((info->flags & GIT_CPDIR_SIMPLE_TO_MODE) != 0) - usemode = (usemode & 0111) ? 0777 : 0666; + usemode = GIT_PERMS_FOR_WRITE(usemode); error = git_futils_cp(from->ptr, info->to.ptr, usemode); } diff --git a/src/fileops.h b/src/fileops.h index 5adedfc57..142eb99d2 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -223,8 +223,11 @@ extern int git_futils_open_ro(const char *path); */ extern git_off_t git_futils_filesize(git_file fd); +#define GIT_PERMS_EXECUTABLE(MODE) (((MODE) & 0111) != 0) +#define GIT_PERMS_CANONICAL(MODE) (GIT_PERMS_EXECUTABLE(MODE) ? 0755 : 0644) +#define GIT_PERMS_FOR_WRITE(MODE) (GIT_PERMS_EXECUTABLE(MODE) ? 0777 : 0666) + #define GIT_MODE_PERMS_MASK 0777 -#define GIT_CANONICAL_PERMS(MODE) (((MODE) & 0100) ? 0755 : 0644) #define GIT_MODE_TYPE(MODE) ((MODE) & ~GIT_MODE_PERMS_MASK) #define GIT_MODE_ISBLOB(MODE) (GIT_MODE_TYPE(MODE) == GIT_MODE_TYPE(GIT_FILEMODE_BLOB)) diff --git a/src/index.c b/src/index.c index 17e43903f..9b32222a7 100644 --- a/src/index.c +++ b/src/index.c @@ -284,7 +284,7 @@ static unsigned int index_create_mode(unsigned int mode) if (S_ISDIR(mode) || (mode & S_IFMT) == (S_IFLNK | S_IFDIR)) return (S_IFLNK | S_IFDIR); - return S_IFREG | ((mode & 0100) ? 0755 : 0644); + return S_IFREG | GIT_PERMS_CANONICAL(mode); } static unsigned int index_merge_mode( diff --git a/src/tree.c b/src/tree.c index 65d01b4d5..91309e107 100644 --- a/src/tree.c +++ b/src/tree.c @@ -10,7 +10,7 @@ #include "tree.h" #include "git2/repository.h" #include "git2/object.h" -#include "path.h" +#include "fileops.h" #include "tree-cache.h" #include "index.h" @@ -29,19 +29,19 @@ static bool valid_filemode(const int filemode) GIT_INLINE(git_filemode_t) normalize_filemode(git_filemode_t filemode) { /* Tree bits set, but it's not a commit */ - if (filemode & GIT_FILEMODE_TREE && !(filemode & 0100000)) + if (GIT_MODE_TYPE(filemode) == GIT_FILEMODE_TREE) return GIT_FILEMODE_TREE; - /* If any of the x bits is set */ - if (filemode & 0111) + /* If any of the x bits are set */ + if (GIT_PERMS_EXECUTABLE(filemode)) return GIT_FILEMODE_BLOB_EXECUTABLE; /* 16XXXX means commit */ - if ((filemode & GIT_FILEMODE_COMMIT) == GIT_FILEMODE_COMMIT) + if (GIT_MODE_TYPE(filemode) == GIT_FILEMODE_COMMIT) return GIT_FILEMODE_COMMIT; /* 12XXXX means commit */ - if ((filemode & GIT_FILEMODE_LINK) == GIT_FILEMODE_LINK) + if (GIT_MODE_TYPE(filemode) == GIT_FILEMODE_LINK) return GIT_FILEMODE_LINK; /* Otherwise, return a blob */ From c97d407d9cc54fc99af0a57e09e04e9e0bc68cb6 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 5 Sep 2013 11:45:29 -0700 Subject: [PATCH 08/10] Fix tests of file modes This fixes an issue checking file modes in the tests that initialize a repo from a template directory when a symlink is used in the template. Also, this updates some other places where we are examining file modes to use the new macros. --- tests-clar/checkout/index.c | 2 +- tests-clar/index/addall.c | 7 +++++-- tests-clar/repo/init.c | 33 ++++++++++++++++++--------------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/tests-clar/checkout/index.c b/tests-clar/checkout/index.c index c9352d8ce..73050d08e 100644 --- a/tests-clar/checkout/index.c +++ b/tests-clar/checkout/index.c @@ -267,7 +267,7 @@ void test_checkout_index__options_override_file_modes(void) cl_git_pass(git_checkout_index(g_repo, NULL, &opts)); cl_git_pass(p_stat("./testrepo/new.txt", &st)); - cl_assert_equal_i_fmt(st.st_mode & 0777, 0700, "%07o"); + cl_assert_equal_i_fmt(st.st_mode & GIT_MODE_PERMS_MASK, 0700, "%07o"); #endif } diff --git a/tests-clar/index/addall.c b/tests-clar/index/addall.c index fca6e77fa..e6ce463a3 100644 --- a/tests-clar/index/addall.c +++ b/tests-clar/index/addall.c @@ -1,6 +1,7 @@ #include "clar_libgit2.h" #include "../status/status_helpers.h" #include "posix.h" +#include "fileops.h" git_repository *g_repo = NULL; @@ -108,8 +109,10 @@ static void check_stat_data(git_index *index, const char *path, bool match) cl_assert(st.st_size == entry->file_size); cl_assert(st.st_uid == entry->uid); cl_assert(st.st_gid == entry->gid); - cl_assert_equal_b(st.st_mode & ~0777, entry->mode & ~0777); - cl_assert_equal_b(st.st_mode & 0111, entry->mode & 0111); + cl_assert_equal_i_fmt( + GIT_MODE_TYPE(st.st_mode), GIT_MODE_TYPE(entry->mode), "%07o"); + cl_assert_equal_b( + GIT_PERMS_EXECUTABLE(st.st_mode), GIT_PERMS_EXECUTABLE(entry->mode)); } else { /* most things will still match */ cl_assert(st.st_size != entry->file_size); diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index 62e4ecd59..d7f2524c8 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -364,6 +364,8 @@ void test_repo_init__extended_1(void) cl_fixture_cleanup("root"); } +#define CLEAR_FOR_CORE_FILEMODE(M) ((M) &= ~0177) + static void assert_hooks_match( const char *template_dir, const char *repo_dir, @@ -382,17 +384,19 @@ static void assert_hooks_match( cl_assert_equal_sz(expected_st.st_size, st.st_size); - expected_st.st_mode = - (expected_st.st_mode & ~0777) | - (((expected_st.st_mode & 0111) ? 0100777 : 0100666) & ~g_umask); + if (GIT_MODE_TYPE(expected_st.st_mode) != GIT_FILEMODE_LINK) { + mode_t expected_mode = + GIT_MODE_TYPE(expected_st.st_mode) | + (GIT_PERMS_FOR_WRITE(expected_st.st_mode) & ~g_umask); - if (!core_filemode) { - expected_st.st_mode = expected_st.st_mode & ~0177; - st.st_mode = st.st_mode & ~0177; + if (!core_filemode) { + CLEAR_FOR_CORE_FILEMODE(expected_mode); + CLEAR_FOR_CORE_FILEMODE(st.st_mode); + } + + cl_assert_equal_i_fmt(expected_mode, st.st_mode, "%07o"); } - cl_assert_equal_i_fmt(expected_st.st_mode, st.st_mode, "%07o"); - git_buf_free(&expected); git_buf_free(&actual); } @@ -409,8 +413,8 @@ static void assert_mode_seems_okay( git_buf_free(&full); if (!core_filemode) { - expect_mode = expect_mode & ~0111; - st.st_mode = st.st_mode & ~0111; + CLEAR_FOR_CORE_FILEMODE(expect_mode); + CLEAR_FOR_CORE_FILEMODE(st.st_mode); expect_setgid = false; } @@ -421,12 +425,11 @@ static void assert_mode_seems_okay( cl_assert((st.st_mode & S_ISGID) == 0); } - if ((expect_mode & 0111) != 0) - cl_assert((st.st_mode & 0111) != 0); - else - cl_assert((st.st_mode & 0111) == 0); + cl_assert_equal_b( + GIT_PERMS_EXECUTABLE(expect_mode), GIT_PERMS_EXECUTABLE(st.st_mode)); - cl_assert((expect_mode & 0170000) == (st.st_mode & 0170000)); + cl_assert_equal_i_fmt( + GIT_MODE_TYPE(expect_mode), GIT_MODE_TYPE(st.st_mode), "%07o"); } void test_repo_init__extended_with_template(void) From af22dabb4366f8b2dd4acd5725a25e88842d6938 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 5 Sep 2013 12:01:17 -0700 Subject: [PATCH 09/10] GIT_MODE_TYPE should exclude setgid bits The GIT_MODE_TYPE macro was looking at all bits above the permissions, but it should really just look at the top bits so that it will give the right results for a setgid or setuid entry. Since we're now using these macros in the tests, this was causing a test failure on platforms that don't support setgid. --- src/fileops.h | 3 ++- tests-clar/repo/init.c | 8 ++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/fileops.h b/src/fileops.h index 142eb99d2..f2144566d 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -228,7 +228,8 @@ extern git_off_t git_futils_filesize(git_file fd); #define GIT_PERMS_FOR_WRITE(MODE) (GIT_PERMS_EXECUTABLE(MODE) ? 0777 : 0666) #define GIT_MODE_PERMS_MASK 0777 -#define GIT_MODE_TYPE(MODE) ((MODE) & ~GIT_MODE_PERMS_MASK) +#define GIT_MODE_TYPE_MASK 0170000 +#define GIT_MODE_TYPE(MODE) ((MODE) & GIT_MODE_TYPE_MASK) #define GIT_MODE_ISBLOB(MODE) (GIT_MODE_TYPE(MODE) == GIT_MODE_TYPE(GIT_FILEMODE_BLOB)) /** diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c index d7f2524c8..43bd7afe0 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -418,12 +418,8 @@ static void assert_mode_seems_okay( expect_setgid = false; } - if (S_ISGID != 0) { - if (expect_setgid) - cl_assert((st.st_mode & S_ISGID) != 0); - else - cl_assert((st.st_mode & S_ISGID) == 0); - } + if (S_ISGID != 0) + cl_assert_equal_b(expect_setgid, (st.st_mode & S_ISGID) != 0); cl_assert_equal_b( GIT_PERMS_EXECUTABLE(expect_mode), GIT_PERMS_EXECUTABLE(st.st_mode)); From a7fcc44dcf3b2925ba366543486afd102b41838c Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 5 Sep 2013 16:14:32 -0700 Subject: [PATCH 10/10] Better macro name for is-exec-bit-set test --- src/checkout.c | 2 +- src/diff_print.c | 2 +- src/fileops.h | 6 +++--- src/tree.c | 2 +- tests-clar/index/addall.c | 2 +- tests-clar/repo/init.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index f3a9b343d..aae354ca6 100644 --- a/src/checkout.c +++ b/src/checkout.c @@ -696,7 +696,7 @@ static int buffer_to_file( if (st != NULL && (error = p_stat(path, st)) < 0) giterr_set(GITERR_OS, "Error statting '%s'", path); - else if (GIT_PERMS_EXECUTABLE(file_mode) && + else if (GIT_PERMS_IS_EXEC(file_mode) && (error = p_chmod(path, file_mode)) < 0) giterr_set(GITERR_OS, "Failed to set permissions on '%s'", path); diff --git a/src/diff_print.c b/src/diff_print.c index 96937d84d..ee4b5fc17 100644 --- a/src/diff_print.c +++ b/src/diff_print.c @@ -46,7 +46,7 @@ static char diff_pick_suffix(int mode) { if (S_ISDIR(mode)) return '/'; - else if (GIT_PERMS_EXECUTABLE(mode)) /* -V536 */ + else if (GIT_PERMS_IS_EXEC(mode)) /* -V536 */ /* in git, modes are very regular, so we must have 0100755 mode */ return '*'; else diff --git a/src/fileops.h b/src/fileops.h index f2144566d..02f79b9e7 100644 --- a/src/fileops.h +++ b/src/fileops.h @@ -223,9 +223,9 @@ extern int git_futils_open_ro(const char *path); */ extern git_off_t git_futils_filesize(git_file fd); -#define GIT_PERMS_EXECUTABLE(MODE) (((MODE) & 0111) != 0) -#define GIT_PERMS_CANONICAL(MODE) (GIT_PERMS_EXECUTABLE(MODE) ? 0755 : 0644) -#define GIT_PERMS_FOR_WRITE(MODE) (GIT_PERMS_EXECUTABLE(MODE) ? 0777 : 0666) +#define GIT_PERMS_IS_EXEC(MODE) (((MODE) & 0111) != 0) +#define GIT_PERMS_CANONICAL(MODE) (GIT_PERMS_IS_EXEC(MODE) ? 0755 : 0644) +#define GIT_PERMS_FOR_WRITE(MODE) (GIT_PERMS_IS_EXEC(MODE) ? 0777 : 0666) #define GIT_MODE_PERMS_MASK 0777 #define GIT_MODE_TYPE_MASK 0170000 diff --git a/src/tree.c b/src/tree.c index 91309e107..f9469195a 100644 --- a/src/tree.c +++ b/src/tree.c @@ -33,7 +33,7 @@ GIT_INLINE(git_filemode_t) normalize_filemode(git_filemode_t filemode) return GIT_FILEMODE_TREE; /* If any of the x bits are set */ - if (GIT_PERMS_EXECUTABLE(filemode)) + if (GIT_PERMS_IS_EXEC(filemode)) return GIT_FILEMODE_BLOB_EXECUTABLE; /* 16XXXX means commit */ diff --git a/tests-clar/index/addall.c b/tests-clar/index/addall.c index e6ce463a3..00388ee00 100644 --- a/tests-clar/index/addall.c +++ b/tests-clar/index/addall.c @@ -112,7 +112,7 @@ static void check_stat_data(git_index *index, const char *path, bool match) cl_assert_equal_i_fmt( GIT_MODE_TYPE(st.st_mode), GIT_MODE_TYPE(entry->mode), "%07o"); cl_assert_equal_b( - GIT_PERMS_EXECUTABLE(st.st_mode), GIT_PERMS_EXECUTABLE(entry->mode)); + 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/repo/init.c b/tests-clar/repo/init.c index 43bd7afe0..e3fc112b3 100644 --- a/tests-clar/repo/init.c +++ b/tests-clar/repo/init.c @@ -422,7 +422,7 @@ static void assert_mode_seems_okay( cl_assert_equal_b(expect_setgid, (st.st_mode & S_ISGID) != 0); cl_assert_equal_b( - GIT_PERMS_EXECUTABLE(expect_mode), GIT_PERMS_EXECUTABLE(st.st_mode)); + GIT_PERMS_IS_EXEC(expect_mode), GIT_PERMS_IS_EXEC(st.st_mode)); cl_assert_equal_i_fmt( GIT_MODE_TYPE(expect_mode), GIT_MODE_TYPE(st.st_mode), "%07o");