From cf711378ce53d6c8d099b1398d68ce3ba91cfd08 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 8 Nov 2016 16:39:44 +0100 Subject: [PATCH 1/9] Really work around "uninitialized value" warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ever since 457f08a (git-rev-list: add --bisect-vars option., 2007-03-21), Git's source code uses the following trick to fool GCC into *not* warning about uninitialized values: int value = value; We use this trick to silence the "warning: ‘x’ is used uninitialized in this function [-Wuninitialized]" when the variables are not really used uninitialized (but it is hard for the compiler to determine that). This trick works well for GCC, and even Clang seems to appease that workaround. Not so Visual C. It does realize that this is just a trick to fool it, and it simply refuses to be fooled. The only way to silence the warning for Visual C would be to write something like this: #pragma warning(suppress: 4700) int value; Obviously this is not portable, and neither is that trick that fools GCC. So let's just introduce a new macro that continues to fool GCC, but simply initializes the values everywhere else. Signed-off-by: Johannes Schindelin --- builtin/rev-list.c | 3 ++- fast-import.c | 4 ++-- git-compat-util.h | 17 +++++++++++++++++ merge-recursive.c | 2 +- read-cache.c | 2 +- 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 0aa93d5891..adde1550aa 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -388,7 +388,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix) mark_edges_uninteresting(&revs, show_edge); if (bisect_list) { - int reaches = reaches, all = all; + FAKE_INIT(int, reaches, 0); + FAKE_INIT(int, all, 0); revs.commits = find_bisection(revs.commits, &reaches, &all, bisect_find_all); diff --git a/fast-import.c b/fast-import.c index 64fe602f0b..da65ba86ac 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2992,7 +2992,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20]) static void parse_get_mark(const char *p) { - struct object_entry *oe = oe; + FAKE_INIT(struct object_entry *, oe, NULL); char output[42]; /* get-mark SP LF */ @@ -3009,7 +3009,7 @@ static void parse_get_mark(const char *p) static void parse_cat_blob(const char *p) { - struct object_entry *oe = oe; + FAKE_INIT(struct object_entry *, oe, NULL); unsigned char sha1[20]; /* cat-blob SP LF */ diff --git a/git-compat-util.h b/git-compat-util.h index 8be0bde057..3522c8430d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -51,6 +51,23 @@ #endif #endif +/* + * Under certain circumstances Git's source code is cleverer than the C + * compiler when the latter warns about some "uninitialized value", e.g. when + * a value is both initialized and used under the same condition. + * + * GCC can be fooled to not spit out this warning by using the construct: + * "int value = value;". Other C compilers are not that easily fooled and would + * require a #pragma (which is not portable, and would litter the source code). + * + * To keep things simple, we only fool GCC, and initialize such values instead + * when compiling with other C compilers. + */ +#ifdef __GNUC__ +#define FAKE_INIT(a, b, c) a b = b +#else +#define FAKE_INIT(a, b, c) a b = c +#endif /* * BUILD_ASSERT_OR_ZERO - assert a build-time dependency, as an expression. diff --git a/merge-recursive.c b/merge-recursive.c index b7ff1ada3c..d5717626b3 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2005,7 +2005,7 @@ int merge_recursive(struct merge_options *o, { struct commit_list *iter; struct commit *merged_common_ancestors; - struct tree *mrtree = mrtree; + FAKE_INIT(struct tree *, mrtree, NULL); int clean; if (show(o, 4)) { diff --git a/read-cache.c b/read-cache.c index 9054369dd0..a9f1f5c487 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1893,7 +1893,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, struct cache_entry *ce, { int size; struct ondisk_cache_entry *ondisk; - int saved_namelen = saved_namelen; /* compiler workaround */ + FAKE_INIT(int, saved_namelen, 0); char *name; int result; From ab3f456a45cb3b3159032eee009a0526cbff5cd1 Mon Sep 17 00:00:00 2001 From: Philip Oakley Date: Sun, 19 Jul 2015 15:59:04 +0100 Subject: [PATCH 2/9] perl/Makefile: treat a missing PM.stamp as if empty 'make clean', or a 'git clean -dfx' will delete the PM stamp file, so it cannot be a direct target in such clean conditions, resulting in an error. Normally the PM.stamp is recreated by the git/Makefile, except when a dry-run is requested, for example, as used in the msysgit msvc-build script which implements the compat/vcbuild/README using contrib/buildsystems. The script msvc-build is introduced later in this series. Protect the PM.stamp target when the PM.stamp file does not exist, allowing a Git 'Makefile -n' to succeed on a clean repo. Signed-off-by: Philip Oakley --- This is development of the original "[PATCH 4/17] Makefile: a dry-run can error out if no perl. Document the issue" 2015-06-25, (http://marc.info/?l=git&m=143519054716960&w=2), which simply documented the issue and then used NO_PERL to avoid the problem. See follow on email thread for some discussion. --- perl/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/perl/Makefile b/perl/Makefile index 15d96fcc7a..5b86aac41c 100644 --- a/perl/Makefile +++ b/perl/Makefile @@ -22,7 +22,9 @@ clean: $(RM) $(makfile).old $(RM) PM.stamp +ifneq (,$(wildcard PM.stamp)) $(makfile): PM.stamp +endif ifdef NO_PERL_MAKEMAKER instdir_SQ = $(subst ','\'',$(prefix)/lib) From a3a51d18d21b82a24a11fa3e3ad996adc64345c8 Mon Sep 17 00:00:00 2001 From: Philip Oakley Date: Fri, 6 May 2016 11:46:05 +0100 Subject: [PATCH 3/9] Avoid multiple PREFIX definitions The short and sweet PREFIX can be confused when used in many places. Rename both usages to better describe their purpose. EXEC_CMD_PREFIX is used in full to disambiguate it from the nearby GIT_EXEC_PATH. The PREFIX in sideband.c, while nominally independant of the exec_cmd PREFIX, does reside within libgit[1], so the definitions would clash when taken together with a PREFIX given on the command line for use by exec_cmd.c. Noticed when compiling Git for Windows using MSVC/Visual Studio [1] which reports the conflict beteeen the command line definition and the definition in sideband.c within the libgit project. [1] the libgit functions are brought into a single sub-project within the Visual Studio construction script provided in contrib, and hence uses a single command for both exec_cmd.c and sideband.c. Signed-off-by: Philip Oakley --- Makefile | 2 +- exec_cmd.c | 4 ++-- sideband.c | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 9b37a96e5b..83ca0b91b4 100644 --- a/Makefile +++ b/Makefile @@ -2014,7 +2014,7 @@ exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ - '-DPREFIX="$(prefix_SQ)"' + '-DFALLBACK_RUNTIME_PREFIX="$(prefix_SQ)"' builtin/init-db.sp builtin/init-db.s builtin/init-db.o: GIT-PREFIX builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \ diff --git a/exec_cmd.c b/exec_cmd.c index fb94aeba9c..4151ffa222 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -12,7 +12,7 @@ char *system_path(const char *path) #ifdef RUNTIME_PREFIX static const char *prefix; #else - static const char *prefix = PREFIX; + static const char *prefix = FALLBACK_RUNTIME_PREFIX; #endif struct strbuf d = STRBUF_INIT; @@ -27,7 +27,7 @@ char *system_path(const char *path) !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) && !(prefix = strip_path_suffix(argv0_path, BINDIR)) && !(prefix = strip_path_suffix(argv0_path, "git"))) { - prefix = PREFIX; + prefix = FALLBACK_RUNTIME_PREFIX; trace_printf("RUNTIME_PREFIX requested, " "but prefix computation failed. " "Using static fallback '%s'.\n", prefix); diff --git a/sideband.c b/sideband.c index 1e4d684d6c..82dda5fa15 100644 --- a/sideband.c +++ b/sideband.c @@ -13,7 +13,7 @@ * the remote died unexpectedly. A flush() concludes the stream. */ -#define PREFIX "remote: " +#define DISPLAY_PREFIX "remote: " #define ANSI_SUFFIX "\033[K" #define DUMB_SUFFIX " " @@ -50,7 +50,7 @@ int recv_sideband(const char *me, int in_stream, int out) switch (band) { case 3: strbuf_addf(&outbuf, "%s%s%s", outbuf.len ? "\n" : "", - PREFIX, buf + 1); + DISPLAY_PREFIX, buf + 1); retval = SIDEBAND_REMOTE_ERROR; break; case 2: @@ -68,7 +68,7 @@ int recv_sideband(const char *me, int in_stream, int out) int linelen = brk - b; if (!outbuf.len) - strbuf_addstr(&outbuf, PREFIX); + strbuf_addstr(&outbuf, DISPLAY_PREFIX); if (linelen > 0) { strbuf_addf(&outbuf, "%.*s%s%c", linelen, b, suffix, *brk); @@ -82,8 +82,8 @@ int recv_sideband(const char *me, int in_stream, int out) } if (*b) - strbuf_addf(&outbuf, "%s%s", - outbuf.len ? "" : PREFIX, b); + strbuf_addf(&outbuf, "%s%s", outbuf.len ? + "" : DISPLAY_PREFIX, b); break; case 1: write_or_die(out, buf + 1, len); From ce196a41b106b5c3cca829ccdea6f3db6f32ce61 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 27 Oct 2016 06:25:56 -0700 Subject: [PATCH 4/9] obstack: fix compiler warning MS Visual C suggests that the construct condition ? (int) i : (ptrdiff_t) d is incorrect. Let's fix this by casting to ptrdiff_t also for the positive arm of the conditional. Signed-off-by: Johannes Schindelin --- compat/obstack.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/obstack.h b/compat/obstack.h index ceb4bdbcdd..892dc28828 100644 --- a/compat/obstack.h +++ b/compat/obstack.h @@ -493,7 +493,7 @@ __extension__ \ ( (h)->temp.tempint = (char *) (obj) - (char *) (h)->chunk, \ ((((h)->temp.tempint > 0 \ && (h)->temp.tempint < (h)->chunk_limit - (char *) (h)->chunk)) \ - ? (int) ((h)->next_free = (h)->object_base \ + ? (ptrdiff_t) ((h)->next_free = (h)->object_base \ = (h)->temp.tempint + (char *) (h)->chunk) \ : (((obstack_free) ((h), (h)->temp.tempint + (char *) (h)->chunk), 0), 0))) From 951751d6c7baa35d7a2e0b96ecb4309d189e9655 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 27 Oct 2016 08:06:29 -0700 Subject: [PATCH 5/9] terminal.c: guard the inclusion of inttypes.h We do have a lovely Makefile option to state that that header file is not available. Let's use it everywhere... Signed-off-by: Johannes Schindelin --- compat/terminal.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compat/terminal.c b/compat/terminal.c index 1d37f0aafb..d9d3945afa 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -1,4 +1,6 @@ +#ifndef NO_INTTYPES_H #include +#endif #include "git-compat-util.h" #include "run-command.h" #include "compat/terminal.h" From f6081edde90398ad8e870bddd3803cbe81c7a667 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 25 Nov 2016 18:29:51 +0100 Subject: [PATCH 6/9] Ensure that bin-wrappers/* targets adds `.exe` if necessary When compiling with Visual Studio, the projects' names are identical to the executables modulo the extensions. Which means that the bin-wrappers *need* to target the .exe files lest they try to execute directories. Signed-off-by: Johannes Schindelin --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 83ca0b91b4..bf5e83a3a3 100644 --- a/Makefile +++ b/Makefile @@ -2276,7 +2276,7 @@ bin-wrappers/%: wrap-for-bin.sh @mkdir -p bin-wrappers $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's|@@BUILD_DIR@@|$(shell pwd)|' \ - -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' < $< > $@ && \ + -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \ chmod +x $@ # GNU make supports exporting all variables by "export" without parameters. From c8dde6714fc3ccac76d98040fd9211a7c9ccc59f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 29 Nov 2016 15:37:01 +0100 Subject: [PATCH 7/9] .gitattributes: fix end-of-lines The test suite does actually not pass when files are checked out with CR/LF line endings, so we have to force them to LF-only. For good measure, we simply force all source files to LF, also to make it easier for Git for Windows contributors to contribute to upstream Git's source code. Signed-off-by: Johannes Schindelin --- .gitattributes | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitattributes b/.gitattributes index 320e33c327..9b742a03a1 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,4 @@ * whitespace=!indent,trail,space -*.[ch] whitespace=indent,trail,space diff=cpp -*.sh whitespace=indent,trail,space +*.[ch] whitespace=indent,trail,space diff=cpp eol=lf +*.sh whitespace=indent,trail,space eol=lf +*.{perl,pm,txt} eol=lf From 735255f47a22ccce59c7657f03512a6440b711b1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 29 Nov 2016 23:07:18 +0100 Subject: [PATCH 8/9] windows: clarify the need for invalidcontinue.obj Git's source code wants to be able to close() the same file descriptor multiple times, ignoring the error returned by the second call (and the ones after that), or to access the osfhandle of an already-closed stdout, among other things that the UCRT does not like. Simply linking invalidcontinue.obj allows such usage without resorting to Debug Assertions (or exiting with exit code 9 in Release Mode). Let's add a note so we don't forget, as suggested by Jeff Hostetler. See https://msdn.microsoft.com/en-us/library/ms235330.aspx for more details. Signed-off-by: Johannes Schindelin --- config.mak.uname | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index 0544660586..fb9973e157 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -379,6 +379,9 @@ ifeq ($(uname_S),Windows) compat/win32/dirent.o compat/win32/fscache.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\" BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE + # invalidcontinue.obj allows Git's source code to close the same file + # handle twice, or to access the osfhandle of an already-closed stdout + # See https://msdn.microsoft.com/en-us/library/ms235330.aspx EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj kernel32.lib ntdll.lib PTHREAD_LIBS = lib = From 5a18234040e330263efb48441e05aeb7460dde2c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 30 Nov 2016 12:00:59 +0100 Subject: [PATCH 9/9] t5505,t5516: create .git/branches/ when needed It is a real old anachronism from the Cogito days to have a .git/branches/ directory. And to have tests that ensure that Cogito users can migrate away from using that directory. But so be it, let's continue testing it. Let's make sure, however, that git init does not need to create that directory. This bug was noticed when testing with templates that had been pre-committed, skipping the empty branches/ directory of course because Git does not track empty directories. Signed-off-by: Johannes Schindelin --- t/t5505-remote.sh | 2 ++ t/t5516-fetch-push.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 3ea27107c2..6b31c7c3fb 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -830,6 +830,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' ' ( cd six && git remote rm origin && + mkdir -p .git/branches && echo "$origin_url" >.git/branches/origin && git remote rename origin origin && test_path_is_missing .git/branches/origin && @@ -844,6 +845,7 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/branches (2)' ( cd seven && git remote rm origin && + mkdir -p .git/branches && echo "quux#foom" > .git/branches/origin && git remote rename origin origin && test_path_is_missing .git/branches/origin && diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 0fc5a7c596..0b6925f1da 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -866,6 +866,7 @@ test_expect_success 'fetch with branches' ' mk_empty testrepo && git branch second $the_first_commit && git checkout second && + mkdir -p testrepo/.git/branches && echo ".." > testrepo/.git/branches/branch1 && ( cd testrepo && @@ -879,6 +880,7 @@ test_expect_success 'fetch with branches' ' test_expect_success 'fetch with branches containing #' ' mk_empty testrepo && + mkdir -p testrepo/.git/branches && echo "..#second" > testrepo/.git/branches/branch2 && ( cd testrepo && @@ -893,6 +895,7 @@ test_expect_success 'fetch with branches containing #' ' test_expect_success 'push with branches' ' mk_empty testrepo && git checkout second && + mkdir -p .git/branches && echo "testrepo" > .git/branches/branch1 && git push branch1 && ( @@ -905,6 +908,7 @@ test_expect_success 'push with branches' ' test_expect_success 'push with branches containing #' ' mk_empty testrepo && + mkdir -p .git/branches && echo "testrepo#branch3" > .git/branches/branch2 && git push branch2 && (