Re: [PATCH] status: display the SHA1 of the commit being currently processed
Am 6/17/2013 15:57, schrieb Mathieu Liénard--Mayor: Le 2013-06-17 15:54, Peter Krefting a écrit : Mathieu Liénard--Mayor: Actually, at first I dealt with it this way: status_printf_ln(s, color, _(Splitting %s while rebasing branch '%s' on '%s'.), stopped_sha ? stopped_sha : _(a commit), ); Would this be more suitable for translators ? Not really, the text surrounding a commit might need to be inflected differently depending on whether it is a SHA-1 or the a commit string. Word order might also be different. Okay, I'll use what you suggested then. That's not a good idea. Do we already use %1$ style formats elsewhere? I'm afraid that they are not supported everywhere. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH jx/clean-interactive] t0060: skip a few relative_path tests on Windows
From: Johannes Sixt j...@kdbg.org The bash on Windows rewrites paths that look like absolute POSIX paths when they are a command-line argument of a regular Windows program, such as git and the test helpers. As a consequence, the actual tests performed are not what the tests scripts expect. The tests that need *not* be skipped are those where the two paths passed to 'test-path-utils relative_path' have the same prefix and the result is expected to be a relative path. This is because the rewriting changes /a/b to D:/Src/MSysGit/a/b, and when both inputs are extended the same way, this just cancels out in the relative path computation. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t0060-path-utils.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index dfe4747..4deec52 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -190,15 +190,15 @@ test_expect_success SYMLINKS 'real path works on symlinks' ' relative_path /a/b/c/ /a/b/ c/ relative_path /a/b/c/ /a/bc/ -relative_path /a//b//c///a/b// c/ +relative_path /a//b//c///a/b// c/ POSIX relative_path /a/b /a/b./ relative_path /a/b//a/b./ relative_path /a /a/b../ relative_path //a/b/ ../../ relative_path /a/c /a/b/ ../c relative_path /a/c /a/b../c -relative_path /a/b empty /a/b -relative_path /a/b null/a/b +relative_path /a/b empty /a/bPOSIX +relative_path /a/b null/a/bPOSIX relative_path empty/a/b./ relative_path emptyempty ./ relative_path emptynull./ -- 1.8.3.1.1670.g1dbc49e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Tracking vendor release with Git
Am 11.06.2013 19:06, schrieb Yann Droneaud: Hi, I'm trying to setup a workflow to track vendor releases (upstream). Each new release are provided as an archive of source code, data, documentation, etc. For each vendor releases, fixes need to be applied before making them available to users (downstream). Seems to be a rather common use case, applied by most Linux distribution for decades. In my case, on top of each releases, a common set of patches will be applied, the biggest, the most intrusive one, being converting CRLF to LF using dos2unix, the others being small portability fixes. In this case, fixes are not going to be applied by upstream. I'm trying to design (copy ;) a workflow with following properties, in order of importance: 1- I wish to keep a branch with each new vendor release as a commit. This branch's history is only about vendor releases, so it's easy to read the changelog of the vendor releases with command such as git log vendor-release-branch 2- I'd like to ease the process of applying our patches on top of each new vendor release, eg. reduces the likeliness of conflicts. 3- I wish to keep a branch with each new fixed vendor release as a commit. Just like the upstream vendor-release-branch, only one commit per release, so it's easy to read the changelog of the vendor releases with command such as git log patched-release-branch I suggest you aim for the following history (time flows from left to right): U---V-W -- upstream branch \ \ \ C---D-E-- CRLF conversion branch \ \ \ K---L--M--N--O -- downstream branch U, V, W are the upstream releases. C is the initial CRLF-LF conversion. D merges the second upstream release into the CRLF branch, E the third upstream release. These merges very likely create tons of conflicts. But that does not matter, because you know that the only change in our side is CRLF conversion. The commits on this branch can easily be automated. That's the primary motivation for this scheme. K is your first small bugfix and also your first downstream release. After merging L, the second, CRLF-converted, upstream release, you make your second small change, M, which is also your second downstream release. Rinse and repeat with N and O for the third release. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] test: improve rebase -q test
Am 10.06.2013 19:27, schrieb SZEDER Gábor: My main motivation is that, like in your example, in the bash prompt tests I only have to check a single line of output, but because of debuggability I always did: echo (master) expected __git_ps1 actual test_cmp expected actual Chained by , I presume. With such a helper function this could be reduced to a single line: test_string_equal (master) $(__git_ps1) without loss of functionality Not quite: A non-zero exit code of the $(__git_ps1) is lost. (It probably doesn't matter here, but it certainly does if the command is $(git rev-parse foo) or similar.) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
Am 6/9/2013 22:31, schrieb Junio C Hamano: Jeff King p...@peff.net writes: I'm a little negative on handling just SIGTERM. That would make the test pass, but does it really address the overall issue? To me, the usefulness is having exit values with consistent meanings. Yes. Unless the goal is to give Windows port pratically the same signal semantics as ports on other platforms, I do not think special casing SIGTERM (unless it is a very common signal on Windows and others are unlikely to be useful) buys us much. I'm thinking the same. And, no, SIGTERM is not very common on Windows. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mingw: make mingw_signal return the correct handler
From: Erik Faye-Lund kusmab...@gmail.com Returning the SIGALRM handler for SIGINT is not very useful. Signed-off-by: Erik Faye-Lund kusmab...@gmail.com Signed-off-by: Johannes Sixt j...@kdbg.org --- Am 6/7/2013 16:20, schrieb Erik Faye-Lund: On Fri, Jun 7, 2013 at 3:07 PM, Johannes Sixt j.s...@viscovery.net wrote: BTW, isn't mingw_signal() bogus in that it returns the SIGALRM handler even if a SIGINT handler is installed? Yep, that's a bug. Thanks for noticing. This is your patch to address it. I've pushed out a branch here that tries to address these issues, but I haven't had time to test them. I'll post the series when I have. In the mean time: https://github.com/kusma/git/tree/win32-signal-raise Concerning the other two patches: * SIGINT: perhaps handle only the SIG_DFL case (for the exit code) and forward all other cases to MSVCRT? * SIGTERM: it papers only over a general issue and should be dropped. IMO. -- Hannes compat/mingw.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index b295e2f..bb92c43 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1677,14 +1677,16 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out) #undef signal sig_handler_t mingw_signal(int sig, sig_handler_t handler) { - sig_handler_t old = timer_fn; + sig_handler_t old; switch (sig) { case SIGALRM: + old = timer_fn; timer_fn = handler; break; case SIGINT: + old = sigint_fn; sigint_fn = handler; break; -- 1.8.3.1504.g78dbf7a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0070: Use precondition CANNOTWRITE
Am 08.06.2013 08:51, schrieb Torsten Bögershausen: Filesystems like VFAT or NTFS allow to create files regardless of the write permissions of the directory. Therefore mktemp to unwritable directory in t0700 will always fail on Windows using NTFS. This TC has been disabled for MINGW, and needs to be disabled for CYGWIN. Use the precondition CANNOTWRITE which is probing the file system and works for MINGW, CYGWIN and even for Linux using VFAT. Shouldn't it be a matter of -test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' ' +test_expect_success SANITY 'mktemp to unwritable directory prints filename' ' It probably wouldn't catch Linux VFAT, but there're already a lot of tests that don't pass on Linux VFAT. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
Am 6/6/2013 19:40, schrieb Jeff King: On Thu, Jun 06, 2013 at 10:21:47AM -0700, Junio C Hamano wrote: The particular deficiency is that when a signal is raise()d whose SIG_DFL action will cause process death (SIGTERM in this case), the implementation of raise() just calls exit(3). After a bit of web searching, it seems to me that this behaviour of raise() is in msvcrt, and compat/mingw.c::mingw_raise() just calls that. In other words, the implementation of raise() is at an even lower level than mingw/msys, and I would agree that it is a platform issue. Yeah, if it were mingw_raise responsible for this, I would suggest using the POSIX shell 128+sig instead. We could potentially check for SIG_DFL[1] mingw_raise and intercept and exit there. I don't know if that would create headaches or confusion for other msys programs, though. I'd leave that up to the msysgit people to decide whether it is worth the trouble. Even if we move the signal emulation closer to POSIX in the way that you suggested (if that were possible, I haven't checked), the emulation is still not complete, because we would have addressed only raise(). Therefore, personally I would like to delay the issue until there is a user (script) that depends on POSIXly exit codes. As far as t0005.2 is concerned, its the best to just concede that Windows does not have POSIX-like behavior as far as signals are concerned, and skip the test. We could also sweep the issue under the rug with the patch below, which works because SIGALRM does not exist in MSVCRT and is handled entirely by compat/mingw.c. But it goes into the wrong direction, IMO. diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index ad9e604..68b6c3b 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -12,9 +12,8 @@ EOF test_expect_success 'sigchain works' ' test-sigchain actual case $? in - 143) true ;; # POSIX w/ SIGTERM=15 - 271) true ;; # ksh w/ SIGTERM=15 - 3) true ;; # Windows + 142) true ;; # POSIX w/ SIGALRM=14 + 270) true ;; # ksh w/ SIGTERM=14 *) false ;; esac test_cmp expect actual @@ -23,8 +22,8 @@ test_expect_success 'sigchain works' ' test_expect_success 'signals are propagated using shell convention' ' # we use exec here to avoid any sub-shell interpretation # of the exit code - git config alias.sigterm !exec test-sigchain - test_expect_code 143 git sigterm + git config alias.sigalrm !exec test-sigchain + test_expect_code 142 git sigalrm ' test_done diff --git a/test-sigchain.c b/test-sigchain.c index 42db234..0233a39 100644 --- a/test-sigchain.c +++ b/test-sigchain.c @@ -14,9 +14,9 @@ X(three) #undef X int main(int argc, char **argv) { - sigchain_push(SIGTERM, one); - sigchain_push(SIGTERM, two); - sigchain_push(SIGTERM, three); - raise(SIGTERM); + sigchain_push(SIGALRM, one); + sigchain_push(SIGALRM, two); + sigchain_push(SIGALRM, three); + raise(SIGALRM); return 0; } -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
Am 6/7/2013 14:00, schrieb Erik Faye-Lund: On Fri, Jun 7, 2013 at 12:24 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 6/7/2013 12:12, schrieb Erik Faye-Lund: On Thu, Jun 6, 2013 at 7:40 PM, Jeff King p...@peff.net wrote: On Thu, Jun 06, 2013 at 10:21:47AM -0700, Junio C Hamano wrote: The particular deficiency is that when a signal is raise()d whose SIG_DFL action will cause process death (SIGTERM in this case), the implementation of raise() just calls exit(3). After a bit of web searching, it seems to me that this behaviour of raise() is in msvcrt, and compat/mingw.c::mingw_raise() just calls that. In other words, the implementation of raise() is at an even lower level than mingw/msys, and I would agree that it is a platform issue. Yeah, if it were mingw_raise responsible for this, I would suggest using the POSIX shell 128+sig instead. We could potentially check for SIG_DFL[1] mingw_raise and intercept and exit there. I don't know if that would create headaches or confusion for other msys programs, though. I'd leave that up to the msysgit people to decide whether it is worth the trouble. ...and here's the code to do just that: diff --git a/compat/mingw.c b/compat/mingw.c index b295e2f..8b3c1b4 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1573,7 +1573,8 @@ static HANDLE timer_event; static HANDLE timer_thread; static int timer_interval; static int one_shot; -static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL; +static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL, +sigterm_fn = SIG_DFL; /* The timer works like this: * The thread, ticktack(), is a trivial routine that most of the time @@ -1688,6 +1689,10 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler) sigint_fn = handler; break; + case SIGTERM: + sigterm_fn = handler; + break; + default: return signal(sig, handler); } @@ -1715,6 +1720,13 @@ int mingw_raise(int sig) sigint_fn(SIGINT); return 0; + case SIGTERM: + if (sigterm_fn == SIG_DFL) + exit(128 + SIGTERM); + else if (sigterm_fn != SIG_IGN) + sigterm_fn(SIGTERM); + return 0; + default: return raise(sig); } That's pointless and does not work. The handler would only be called when raise() is called, but not when a SIGTERM is received, e.g., via Ctrl-C from the command line, because that route ends up in MSVCRT, which does not know about this handler. That's not entirely true. On Windows, there's only *one* way to generate SIGTERM; signal(SIGTERM). Ctrl+C does not generate SIGTERM. We generate SIGINT on Ctrl+C in mingw_fgetc, but the default Control+C handler routine calls ExitProcess(): http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx But a call to signal(SIGTERM, my_handler) should divert Ctrl+C to my_handler. The unpatched version does, because MSVCRT now knows about my_handler and sets things up so that the event handler calls my_handler. But your patched version bypasses MSVCRT, and the default (whatever MSVCRT has set up) happens, and my_handler is not called. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t0005: skip signal death exit code test on Windows
Am 6/7/2013 14:46, schrieb Erik Faye-Lund: On Fri, Jun 7, 2013 at 2:19 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 6/7/2013 14:00, schrieb Erik Faye-Lund: On Fri, Jun 7, 2013 at 12:24 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 6/7/2013 12:12, schrieb Erik Faye-Lund: diff --git a/compat/mingw.c b/compat/mingw.c index b295e2f..8b3c1b4 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1573,7 +1573,8 @@ static HANDLE timer_event; static HANDLE timer_thread; static int timer_interval; static int one_shot; -static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL; +static sig_handler_t timer_fn = SIG_DFL, sigint_fn = SIG_DFL, +sigterm_fn = SIG_DFL; /* The timer works like this: * The thread, ticktack(), is a trivial routine that most of the time @@ -1688,6 +1689,10 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler) sigint_fn = handler; break; + case SIGTERM: + sigterm_fn = handler; + break; + default: return signal(sig, handler); } @@ -1715,6 +1720,13 @@ int mingw_raise(int sig) sigint_fn(SIGINT); return 0; + case SIGTERM: + if (sigterm_fn == SIG_DFL) + exit(128 + SIGTERM); + else if (sigterm_fn != SIG_IGN) + sigterm_fn(SIGTERM); + return 0; + default: return raise(sig); } That's pointless and does not work. The handler would only be called when raise() is called, but not when a SIGTERM is received, e.g., via Ctrl-C from the command line, because that route ends up in MSVCRT, which does not know about this handler. That's not entirely true. On Windows, there's only *one* way to generate SIGTERM; signal(SIGTERM). Ctrl+C does not generate SIGTERM. We generate SIGINT on Ctrl+C in mingw_fgetc, but the default Control+C handler routine calls ExitProcess(): http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx But a call to signal(SIGTERM, my_handler) should divert Ctrl+C to my_handler. The unpatched version does, because MSVCRT now knows about my_handler and sets things up so that the event handler calls my_handler. No, it does not: Ctrl+C raises SIGINT, not SIGTERM. action type=slap destination=forehead/ You are right. Your change would fix SIGTERM as it can be raised only via raise() on Windows nor can it be caught when a process is killed via mingw_kill(...,SIGTERM) by another process. But then the current handling of SIGINT in compat/mingw.c is broken. The handler is not propagated to MSVCRT, and after a SIGINT handler is installed, Ctrl+C still terminates the process. No? BTW, isn't mingw_signal() bogus in that it returns the SIGALRM handler even if a SIGINT handler is installed? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/8] Rebase topology test
Am 07.06.2013 08:11, schrieb Martin von Zweigbergk: Changes since v5: * Improved test_linear_range * Changed TODOs to be about consistency, not --topo-order Martin von Zweigbergk (7): add simple tests of consistency across rebase types add tests for rebasing with patch-equivalence present add tests for rebasing of empty commits add tests for rebasing root add tests for rebasing merged history t3406: modernize style tests: move test for rebase messages from t3400 to t3406 I looked at the interdiff to v3 and have nothing to add. Well done! Thanks. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/10] test-chmtime: Fix exit code on Windows
MinGW's bash does not recognize an exit code -1 as failure. See also 47e3de0e (MinGW: truncate exit()'s argument to lowest 8 bits) and 2488df84 (builtin run_command: do not exit with -1). Exit code 1 is good enough. Signed-off-by: Johannes Sixt j...@kdbg.org --- test-chmtime.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test-chmtime.c b/test-chmtime.c index 02b42ba..2e601a8 100644 --- a/test-chmtime.c +++ b/test-chmtime.c @@ -84,7 +84,7 @@ int main(int argc, const char *argv[]) if (stat(argv[i], sb) 0) { fprintf(stderr, Failed to stat %s: %s\n, argv[i], strerror(errno)); - return -1; + return 1; } #ifdef WIN32 @@ -92,7 +92,7 @@ int main(int argc, const char *argv[]) chmod(argv[i], sb.st_mode | S_IWUSR)) { fprintf(stderr, Could not make user-writable %s: %s, argv[i], strerror(errno)); - return -1; + return 1; } #endif @@ -107,7 +107,7 @@ int main(int argc, const char *argv[]) if (utb.modtime != sb.st_mtime utime(argv[i], utb) 0) { fprintf(stderr, Failed to modify time on %s: %s\n, argv[i], strerror(errno)); - return -1; + return 1; } } @@ -115,5 +115,5 @@ int main(int argc, const char *argv[]) usage: fprintf(stderr, usage: %s %s\n, argv[0], usage_str); - return -1; + return 1; } -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/10] t3010: modernize style
In particular: - move test preparations inside test_expect_success - place test description on the test_expect_success line - indent with a tab Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3010-ls-files-killed-modified.sh | 123 ++-- 1 file changed, 61 insertions(+), 62 deletions(-) diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 95671c2..2d0ff2d 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -37,71 +37,70 @@ modified without reporting path9 and path10. ' . ./test-lib.sh -date path0 -if test_have_prereq SYMLINKS -then - ln -s xyzzy path1 -else - date path1 -fi -mkdir path2 path3 -date path2/file2 -date path3/file3 -: path7 -date path8 -: path9 -date path10 -test_expect_success \ -'git update-index --add to add various paths.' \ -git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 - -rm -fr path? ;# leave path10 alone -date path2 -if test_have_prereq SYMLINKS -then - ln -s frotz path3 - ln -s nitfol path5 -else - date path3 - date path5 -fi -mkdir path0 path1 path6 -date path0/file0 -date path1/file1 -date path6/file6 -date path7 -: path8 -: path9 -touch path10 +test_expect_success 'git update-index --add to add various paths.' ' + date path0 + if test_have_prereq SYMLINKS + then + ln -s xyzzy path1 + else + date path1 + fi + mkdir path2 path3 + date path2/file2 + date path3/file3 + : path7 + date path8 + : path9 + date path10 + git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 + rm -fr path?# leave path10 alone +' -test_expect_success \ -'git ls-files -k to show killed files.' \ -'git ls-files -k .output' -cat .expected EOF -path0/file0 -path1/file1 -path2 -path3 -EOF +test_expect_success 'git ls-files -k to show killed files.' ' + date path2 + if test_have_prereq SYMLINKS + then + ln -s frotz path3 + ln -s nitfol path5 + else + date path3 + date path5 + fi + mkdir path0 path1 path6 + date path0/file0 + date path1/file1 + date path6/file6 + date path7 + : path8 + : path9 + touch path10 + git ls-files -k .output +' -test_expect_success \ -'validate git ls-files -k output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -k output.' ' + cat .expected -\EOF + path0/file0 + path1/file1 + path2 + path3 + EOF + test_cmp .expected .output +' -test_expect_success \ -'git ls-files -m to show modified files.' \ -'git ls-files -m .output' -cat .expected EOF -path0 -path1 -path2/file2 -path3/file3 -path7 -path8 -EOF +test_expect_success 'git ls-files -m to show modified files.' ' + git ls-files -m .output +' -test_expect_success \ -'validate git ls-files -m output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -m output.' ' + cat .expected -\EOF + path0 + path1 + path2/file2 + path3/file3 + path7 + path8 + EOF + test_cmp .expected .output +' test_done -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/10] t3030: use test_ln_s_add to remove SYMLINKS prerequisite
The test cases include many corner-cases of merge-recursive's behavior, some of them involve type changes and symbolic links. All cases, including those that are protected by SYMLINKS check only whether the result of merge-recursive is correctly stored in the database and the index; the file system is not investigated. Use test_ln_s_add to enter a symbolic link in the index in the test setup and run the tests without the SYMLINKS prerequisite. Notice that one test that has the SYMLINKS protection removed is an expect_failure. There is a possibility that the test fails differently depending on whether SYMLINKS is present or not; but this is not the case presently. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3030-merge-recursive.sh | 62 +++--- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index a5e3da7..2f96100 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -25,10 +25,7 @@ test_expect_success 'setup 1' ' git branch submod git branch copy git branch rename - if test_have_prereq SYMLINKS - then - git branch rename-ln - fi + git branch rename-ln echo hello a cp a d/e @@ -260,16 +257,12 @@ test_expect_success 'setup 8' ' git add e test_tick git commit -m rename a-e - if test_have_prereq SYMLINKS - then - git checkout rename-ln - git mv a e - ln -s e a - git add a e - test_tick - git commit -m rename a-e, symlink a-e - oln=`printf e | git hash-object --stdin` - fi + git checkout rename-ln + git mv a e + test_ln_s_add e a + test_tick + git commit -m rename a-e, symlink a-e + oln=`printf e | git hash-object --stdin` ' test_expect_success 'setup 9' ' @@ -569,28 +562,25 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -if test_have_prereq SYMLINKS -then - test_expect_failure 'merge-recursive rename vs. rename/symlink' ' - - git checkout -f rename - git merge rename-ln - ( git ls-tree -r HEAD ; git ls-files -s ) actual - ( - echo 12 blob $oln a - echo 100644 blob $o0 b - echo 100644 blob $o0 c - echo 100644 blob $o0 d/e - echo 100644 blob $o0 e - echo 12 $oln 0 a - echo 100644 $o0 0 b - echo 100644 $o0 0 c - echo 100644 $o0 0 d/e - echo 100644 $o0 0 e - ) expected - test_cmp expected actual - ' -fi +test_expect_failure 'merge-recursive rename vs. rename/symlink' ' + + git checkout -f rename + git merge rename-ln + ( git ls-tree -r HEAD ; git ls-files -s ) actual + ( + echo 12 blob $oln a + echo 100644 blob $o0 b + echo 100644 blob $o0 c + echo 100644 blob $o0 d/e + echo 100644 blob $o0 e + echo 12 $oln 0 a + echo 100644 $o0 0 b + echo 100644 $o0 0 c + echo 100644 $o0 0 d/e + echo 100644 $o0 0 e + ) expected + test_cmp expected actual +' test_done -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/10] t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite
In t4023 and t4114, we have to remove the entries using 'git rm' because otherwise the entries that must turn from symbolic links to regular files would stay symbolic links in the index. For the same reason, we have to use 'git mv' instead of plain 'mv' in t3509. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3509-cherry-pick-merge-df.sh | 12 +--- t/t4023-diff-rename-typechange.sh | 28 ++-- t/t4114-apply-typechange.sh | 29 ++--- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh index df921d1..a5b6a5f 100755 --- a/t/t3509-cherry-pick-merge-df.sh +++ b/t/t3509-cherry-pick-merge-df.sh @@ -10,17 +10,15 @@ test_expect_success 'Initialize repository' ' git commit -m a ' -test_expect_success SYMLINKS 'Setup rename across paths each below D/F conflicts' ' +test_expect_success 'Setup rename across paths each below D/F conflicts' ' mkdir b - ln -s ../a b/a - git add b + test_ln_s_add ../a b/a git commit -m b git checkout -b branch rm b/a - mv a b/a - ln -s b/a a - git add . + git mv a b/a + test_ln_s_add b/a a git commit -m swap f1 @@ -28,7 +26,7 @@ test_expect_success SYMLINKS 'Setup rename across paths each below D/F conflicts git commit -m f1 ' -test_expect_success SYMLINKS 'Cherry-pick succeeds with rename across D/F conflicts' ' +test_expect_success 'Cherry-pick succeeds with rename across D/F conflicts' ' git reset --hard git checkout master^0 git cherry-pick branch diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh index 5d20acf..55d549f 100755 --- a/t/t4023-diff-rename-typechange.sh +++ b/t/t4023-diff-rename-typechange.sh @@ -4,44 +4,44 @@ test_description='typechange rename detection' . ./test-lib.sh -test_expect_success SYMLINKS setup ' +test_expect_success setup ' rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo - ln -s linklink bar - git add foo bar + test_ln_s_add linklink bar + git add foo git commit -a -m Initial git tag one - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING bar - ln -s linklink foo - git add foo bar + test_ln_s_add linklink foo + git add bar git commit -a -m Second git tag two - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo git add foo git commit -a -m Third git tag three mv foo bar - ln -s linklink foo - git add foo bar + test_ln_s_add linklink foo + git add bar git commit -a -m Fourth git tag four # This is purely for sanity check - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo cat $TEST_DIRECTORY/../Makefile bar git add foo bar git commit -a -m Fifth git tag five - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../Makefile foo cat $TEST_DIRECTORY/../COPYING bar git add foo bar @@ -50,7 +50,7 @@ test_expect_success SYMLINKS setup ' ' -test_expect_success SYMLINKS 'cross renames to be detected for regular files' ' +test_expect_success 'cross renames to be detected for regular files' ' git diff-tree five six -r --name-status -B -M | sort actual { @@ -61,7 +61,7 @@ test_expect_success SYMLINKS 'cross renames to be detected for regular files' ' ' -test_expect_success SYMLINKS 'cross renames to be detected for typechange' ' +test_expect_success 'cross renames to be detected for typechange' ' git diff-tree one two -r --name-status -B -M | sort actual { @@ -72,7 +72,7 @@ test_expect_success SYMLINKS 'cross renames to be detected for typechange' ' ' -test_expect_success SYMLINKS 'moves and renames' ' +test_expect_success 'moves and renames' ' git diff-tree three four -r --name-status -B -M | sort actual { diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh index f12826f..ebadbc3 100755 --- a/t/t4114-apply-typechange.sh +++ b/t/t4114-apply-typechange.sh @@ -9,20 +9,19 @@ test_description='git apply should not get confused with type changes. . ./test-lib.sh -test_expect_success SYMLINKS 'setup repository and commits' ' +test_expect_success 'setup repository and commits' ' echo hello world foo echo hi planet bar git update-index --add foo bar git commit -m initial git branch initial rm -f foo - ln -s bar foo - git update-index foo + test_ln_s_add bar foo git commit -m foo symlinked to bar git branch foo-symlinked-to-bar - rm
[PATCH v2 00/10] Increase test coverage on Windows by removing SYMLINKS from many tests
Many tests that involve symbolic links actually check only whether our algorithms are correct by investigating the contents of the object database and the index. Only some of them check the filesystem. This series introduces a function test_ln_s_add that inserts a symbolic link in the index even if the filesystem does not support symbolic links. By using this function, many more tests can be run when the filesystem does not have symblic links, aka Windows. Changes since v1: - Ripped out test_ln_s and corresponding conversions; they were dubious. - There are no changes to t2100 anymore; the corresponding modernization patch is gone. - Moved the t4011 change from the trivial cases to its own patch. It still contains the effects of the former test_ln_s, but open-coded and with a comment. Johannes Sixt (10): test-chmtime: Fix exit code on Windows t3010: modernize style tests: introduce test_ln_s_add tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases) t: use test_ln_s_add to remove SYMLINKS prerequisite t3030: use test_ln_s_add to remove SYMLINKS prerequisite t3100: use test_ln_s_add to remove SYMLINKS prerequisite t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite t6035: use test_ln_s_add to remove SYMLINKS prerequisite t4011: remove SYMLINKS prerequisite t/README | 14 t/t-basic.sh | 39 +++ t/t1004-read-tree-m-u-wf.sh| 7 +- t/t2001-checkout-cache-clash.sh| 7 +- t/t2004-checkout-cache-temp.sh | 5 +- t/t2007-checkout-symlink.sh| 12 ++-- t/t2021-checkout-overwrite.sh | 12 ++-- t/t2200-add-update.sh | 5 +- t/t3010-ls-files-killed-modified.sh| 118 - t/t3030-merge-recursive.sh | 62 - t/t3100-ls-tree-restrict.sh| 42 +--- t/t3509-cherry-pick-merge-df.sh| 12 ++-- t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 --- t/t4008-diff-break-rewrite.sh | 12 ++-- t/t4011-diff-symlink.sh| 35 +++--- t/t4023-diff-rename-typechange.sh | 28 t/t4030-diff-textconv.sh | 8 +-- t/t4114-apply-typechange.sh| 29 t/t4115-apply-symlink.sh | 10 ++- t/t4122-apply-symlink-inside.sh| 8 +-- t/t6035-merge-dir-to-symlink.sh| 73 t/t7001-mv.sh | 18 +++-- t/t7607-merge-overwrite.sh | 5 +- t/t8006-blame-textconv.sh | 14 ++-- t/t8007-cat-file-textconv.sh | 10 ++- t/t9350-fast-export.sh | 5 +- t/t9500-gitweb-standalone-no-errors.sh | 15 ++--- t/test-lib-functions.sh| 17 + test-chmtime.c | 8 +-- 30 files changed, 351 insertions(+), 333 deletions(-) -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/10] t4011: remove SYMLINKS prerequisite
The part of the test that is about symbolic links in the index does not require that the corresponding file system entry is actually a symbolic link. Use test_ln_s_add to insert a symbolic link in the index. When the file system does not support symbolic links, we actually have a regular file in the worktree, which we can update as if it were a symbolic link. diff-index picks up the symbolic link property from the index. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t4011-diff-symlink.sh | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh index f0d5041..13e7f62 100755 --- a/t/t4011-diff-symlink.sh +++ b/t/t4011-diff-symlink.sh @@ -9,7 +9,7 @@ test_description='Test diff of symlinks. . ./test-lib.sh . $TEST_DIRECTORY/diff-lib.sh -test_expect_success SYMLINKS 'diff new symlink and file' ' +test_expect_success 'diff new symlink and file' ' cat expected -\EOF diff --git a/frotz b/frotz new file mode 12 @@ -27,22 +27,25 @@ test_expect_success SYMLINKS 'diff new symlink and file' ' @@ -0,0 +1 @@ +xyzzy EOF - ln -s xyzzy frotz - echo xyzzy nitfol + + # the empty tree git update-index tree=$(git write-tree) - git update-index --add frotz nitfol + + test_ln_s_add xyzzy frotz + echo xyzzy nitfol + git update-index --add nitfol GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree current compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff unchanged symlink and file' ' +test_expect_success 'diff unchanged symlink and file' ' tree=$(git write-tree) git update-index frotz nitfol test -z $(git diff-index --name-only $tree) ' -test_expect_success SYMLINKS 'diff removed symlink and file' ' +test_expect_success 'diff removed symlink and file' ' cat expected -\EOF diff --git a/frotz b/frotz deleted file mode 12 @@ -66,12 +69,18 @@ test_expect_success SYMLINKS 'diff removed symlink and file' ' compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff identical, but newly created symlink and file' ' +test_expect_success 'diff identical, but newly created symlink and file' ' expected rm -f frotz nitfol echo xyzzy nitfol test-chmtime +10 nitfol - ln -s xyzzy frotz + if test_have_prereq SYMLINKS + then + ln -s xyzzy frotz + else + printf xyzzy frotz + # the symlink property propagates from the index + fi git diff-index -M -p $tree current compare_diff_patch expected current @@ -80,7 +89,7 @@ test_expect_success SYMLINKS 'diff identical, but newly created symlink and file compare_diff_patch expected current ' -test_expect_success SYMLINKS 'diff different symlink and file' ' +test_expect_success 'diff different symlink and file' ' cat expected -\EOF diff --git a/frotz b/frotz index 7c465af..df1db54 12 @@ -100,7 +109,13 @@ test_expect_success SYMLINKS 'diff different symlink and file' ' +yxyyz EOF rm -f frotz - ln -s yxyyz frotz + if test_have_prereq SYMLINKS + then + ln -s yxyyz frotz + else + printf yxyyz frotz + # the symlink property propagates from the index + fi echo yxyyz nitfol git diff-index -M -p $tree current compare_diff_patch expected current -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/10] t0000: use test_ln_s_add to remove SYMLINKS prerequisite
t-basic hard-codes many object IDs. To cater to file systems that do not support symbolic links, different IDs are used depending on the SYMLINKS prerequisite. But we can observe the symbolic links are only needed to generate index entries. Use test_ln_s_add to generate the index entries and get rid of explicit SYMLINKS checks. This undoes the special casing introduced in this test by 704a3143 (Use prerequisite tags to skip tests that depend on symbolic links, 2009-03-04). Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t-basic.sh | 39 ++- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index cefe33d..0f13180 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -367,22 +367,6 @@ test_expect_success 'validate object ID of a known tree' ' # Various types of objects -# Some filesystems do not support symblic links; on such systems -# some expected values are different -if test_have_prereq SYMLINKS -then - expectfilter=cat - expectedtree=087704a96baf1c2d1c869a8b084481e121c88b5b - expectedptree1=21ae8269cacbe57ae09138dcc3a2887f904d02b3 - expectedptree2=3c5e5399f3a333eddecce7a9b9465b63f65f51e2 -else - expectfilter='grep -v sym' - expectedtree=8e18edf7d7edcf4371a3ac6ae5f07c2641db7c46 - expectedptree1=cfb8591b2f65de8b8cc1020cd7d9e67e7793b325 - expectedptree2=ce580448f0148b985a513b693fdf7d802cacb44f -fi - - test_expect_success 'adding various types of objects with git update-index --add' ' mkdir path2 path3 path3/subp3 paths=path0 path2/file2 path3/file3 path3/subp3/file3 @@ -390,10 +374,7 @@ test_expect_success 'adding various types of objects with git update-index --add for p in $paths do echo hello $p $p || exit 1 - if test_have_prereq SYMLINKS - then - ln -s hello $p ${p}sym || exit 1 - fi + test_ln_s_add hello $p ${p}sym || exit 1 done ) find path* ! -type d -print | xargs git update-index --add @@ -405,7 +386,7 @@ test_expect_success 'showing stage with git ls-files --stage' ' ' test_expect_success 'validate git ls-files output for a known tree' ' - $expectfilter expected -\EOF + cat expected -\EOF 100644 f87290f8eb2cbbea7857214459a0739927eab154 0 path0 12 15a98433ae33114b085f3eb3bb03b832b3180a01 0 path0sym 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0 path2/file2 @@ -423,14 +404,14 @@ test_expect_success 'writing tree out with git write-tree' ' ' test_expect_success 'validate object ID for a known tree' ' - test $tree = $expectedtree + test $tree = 087704a96baf1c2d1c869a8b084481e121c88b5b ' test_expect_success 'showing tree with git ls-tree' ' git ls-tree $tree current ' -test_expect_success SYMLINKS 'git ls-tree output for a known tree' ' +test_expect_success 'git ls-tree output for a known tree' ' cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym @@ -447,7 +428,7 @@ test_expect_success 'showing tree with git ls-tree -r' ' ' test_expect_success 'git ls-tree -r output for a known tree' ' - $expectfilter expected -\EOF + cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym 100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7path2/file2 @@ -465,7 +446,7 @@ test_expect_success 'showing tree with git ls-tree -r -t' ' git ls-tree -r -t $tree current ' -test_expect_success SYMLINKS 'git ls-tree -r output for a known tree' ' +test_expect_success 'git ls-tree -r output for a known tree' ' cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym @@ -487,7 +468,7 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' ' ' test_expect_success 'validate object ID for a known tree' ' - test $ptree = $expectedptree1 + test $ptree = 21ae8269cacbe57ae09138dcc3a2887f904d02b3 ' test_expect_success 'writing partial tree out with git write-tree --prefix' ' @@ -495,7 +476,7 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' ' ' test_expect_success 'validate object ID for a known tree' ' - test $ptree = $expectedptree2 + test $ptree = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2 ' test_expect_success 'put invalid objects into the index' ' @@ -529,7 +510,7 @@ test_expect_success 'git read-tree followed by write-tree should be idempotent' ' test_expect_success 'validate git diff-files
[PATCH v2 04/10] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
There are many instances where the treatment of symbolic links in the object model and the algorithms are tested, but where it is not necessary to actually have a symbolic link in the worktree. Make adjustments to the tests and remove the SYMLINKS prerequisite when appropriate in trivial cases, where trivial means: - merely a replacement of 'ln -s a b git add b' by test_ln_s_add is needed; - a test for symbolic link on the file system can be split off (and remains protected by SYMLINKS); - existing code is equivalent to test_ln_s_add. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t1004-read-tree-m-u-wf.sh| 7 +++--- t/t2001-checkout-cache-clash.sh| 7 +++--- t/t2004-checkout-cache-temp.sh | 5 ++--- t/t2007-checkout-symlink.sh| 12 +-- t/t2021-checkout-overwrite.sh | 12 +++ t/t2200-add-update.sh | 5 ++--- t/t3010-ls-files-killed-modified.sh| 9 ++-- t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 -- t/t4008-diff-break-rewrite.sh | 12 +-- t/t4030-diff-textconv.sh | 8 +++ t/t4115-apply-symlink.sh | 10 - t/t4122-apply-symlink-inside.sh| 8 +++ t/t7001-mv.sh | 18 ++-- t/t7607-merge-overwrite.sh | 5 ++--- t/t8006-blame-textconv.sh | 14 +--- t/t8007-cat-file-textconv.sh | 10 - t/t9350-fast-export.sh | 5 ++--- t/t9500-gitweb-standalone-no-errors.sh | 15 + 19 files changed, 106 insertions(+), 110 deletions(-) diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index b3ae7d5..3e72aff 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -158,7 +158,7 @@ test_expect_success '3-way not overwriting local changes (their side)' ' ' -test_expect_success SYMLINKS 'funny symlink in work tree' ' +test_expect_success 'funny symlink in work tree' ' git reset --hard git checkout -b sym-b side-b @@ -170,15 +170,14 @@ test_expect_success SYMLINKS 'funny symlink in work tree' ' rm -fr a git checkout -b sym-a side-a mkdir -p a - ln -s ../b a/b - git add a/b + test_ln_s_add ../b a/b git commit -m we add a/b read_tree_u_must_succeed -m -u sym-a sym-a sym-b ' -test_expect_success SYMLINKS,SANITY 'funny symlink in work tree, un-unlink-able' ' +test_expect_success SANITY 'funny symlink in work tree, un-unlink-able' ' rm -fr a b git reset --hard diff --git a/t/t2001-checkout-cache-clash.sh b/t/t2001-checkout-cache-clash.sh index 98aa73e..1fc8e63 100755 --- a/t/t2001-checkout-cache-clash.sh +++ b/t/t2001-checkout-cache-clash.sh @@ -59,10 +59,9 @@ test_expect_success \ 'git read-tree -m $tree1 git checkout-index -f -a' test_debug 'show_files $tree1' -test_expect_success SYMLINKS \ -'git update-index --add a symlink.' \ -'ln -s path0 path1 - git update-index --add path1' +test_expect_success \ +'add a symlink' \ +'test_ln_s_add path0 path1' test_expect_success \ 'writing tree out with git write-tree' \ 'tree3=$(git write-tree)' diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh index 0f4b289..f171a55 100755 --- a/t/t2004-checkout-cache-temp.sh +++ b/t/t2004-checkout-cache-temp.sh @@ -194,11 +194,10 @@ test_expect_success \ test $(cat ../$s1) = tree1asubdir/path5) )' -test_expect_success SYMLINKS \ +test_expect_success \ 'checkout --temp symlink' ' rm -f path* .merge_* out .git/index -ln -s b a -git update-index --add a +test_ln_s_add b a t4=$(git write-tree) rm -f .git/index git read-tree $t4 diff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh index e6f59f1..fc9aad5 100755 --- a/t/t2007-checkout-symlink.sh +++ b/t/t2007-checkout-symlink.sh @@ -6,7 +6,7 @@ test_description='git checkout to switch between branches with symlink-dir' . ./test-lib.sh -test_expect_success SYMLINKS setup ' +test_expect_success setup ' mkdir frotz echo hello frotz/filfre @@ -25,25 +25,25 @@ test_expect_success SYMLINKS setup ' git rm --cached frotz/filfre mv frotz xyzzy - ln -s xyzzy frotz - git add xyzzy/filfre frotz + test_ln_s_add xyzzy frotz + git add xyzzy/filfre test_tick git commit -m side moves frotz/ to xyzzy/ and adds frotz-xyzzy/ ' -test_expect_success SYMLINKS 'switch from symlink to dir' ' +test_expect_success 'switch from symlink to dir' ' git checkout master ' -test_expect_success SYMLINKS 'Remove temporary directories switch to master' ' +test_expect_success 'Remove temporary directories switch to master' ' rm -fr frotz xyzzy nitfol git checkout -f master
[PATCH] t0005: skip signal death exit code test on Windows
From: Johannes Sixt j...@kdbg.org The test case depends on that test-sigchain can commit suicide by a call to raise(SIGTERM) in a way that run-command.c::wait_or_whine() can detect as death through a signal. There are no POSIX signals on Windows, and a sufficiently close emulation is not available in the Microsoft C runtime (and probably not even possible). The particular deficiency is that when a signal is raise()d whose SIG_DFL action will cause process death (SIGTERM in this case), the implementation of raise() just calls exit(3). We could check for exit code 3 in addition to 143, but that would miss the point of the test entirely. Hence, just skip it on Windows. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t0005-signals.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh index ad9e604..981437b 100755 --- a/t/t0005-signals.sh +++ b/t/t0005-signals.sh @@ -20,7 +20,7 @@ test_expect_success 'sigchain works' ' test_cmp expect actual ' -test_expect_success 'signals are propagated using shell convention' ' +test_expect_success !MINGW 'signals are propagated using shell convention' ' # we use exec here to avoid any sub-shell interpretation # of the exit code git config alias.sigterm !exec test-sigchain -- 1.8.3.1489.g15123b5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/7] add tests for rebasing merged history
Am 6/4/2013 19:18, schrieb Junio C Hamano: Martin von Zweigbergk martinv...@gmail.com writes: --- +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i I do not quite follow this TODO. While I think it would be nice to update rebase so that all variants consider replaying the commits in the same order, in this history you have: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z as long as o comes after n and e is shown before n or after o, which all three expected results satisify, it is in --topo-order, isn't it? The comment is really just about the inconsistency, not about a request to have a guaranteed order among the parents of a merge commit. Having said that, wouldn't it be useful (generally, not just in this context) to have a guarantee in which order --topo-order lists parents of a merge? +test_expect_success rebase -p re-creates merge from side branch + reset_rebase + git rebase -p z w + test_cmp_rev z HEAD^ + test_cmp_rev w^2 HEAD^2 + Hmm, turning the left one to the right one? +# d---e d---e +#\ \ \ \ +# n---o---w === n---o \ +# \ \ \ +# z z---W If w were a merge of o into e (i.e. w^1 were e not o), what should happen? Would we get the same topology? 'git rebase -p z w' is a nonsense request in this situation. (I.e., there is no requirement on the result.) At best, we could detect it and bail out or warn. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2013, #02; Tue, 4)
Am 6/5/2013 1:45, schrieb Junio C Hamano: * jk/test-exit-code-by-signal (2013-06-02) 1 commit (merged to 'next' on 2013-06-03 at 25af892) + t0005: test git exit code from signal death Will merge to 'master'. I haven't gotten around to run this new test on Windows. I've reason to believe that it won't pass as is. Please don't let it graduate, yet. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
Am 04.06.2013 23:06, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: There are many instances where the treatment of symbolic links in the object model and the algorithms are tested, but where it is not necessary to actually have a symbolic link in the worktree. Make adjustments to the tests and remove the SYMLINKS prerequisite when appropriate in trivial cases, where trivial means: - merely a replacement of 'ln -s a b' to test_ln_s or of 'ln -s a b git add b' to test_ln_s_add is needed; - a test for symbolic link on the file system can be split off (and remains protected by SYMLINKS); - existing code is equivalent to test_ln_s[_add]. This is too big to review in one go, so I may have separate messages later on this same patch. diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh index ff163cf..bd17ba2 100755 --- a/t/t2003-checkout-cache-mkdir.sh +++ b/t/t2003-checkout-cache-mkdir.sh @@ -19,10 +19,10 @@ test_expect_success 'setup' ' git update-index --add path0 path1/file1 ' -test_expect_success SYMLINKS 'have symlink in place where dir is expected.' ' +test_expect_success 'have symlink in place where dir is expected.' ' rm -fr path0 path1 mkdir path2 -ln -s path2 path1 +test_ln_s path2 path1 git checkout-index -f -a test ! -h path1 test -d path1 test -f path1/file1 test ! -f path2/file1 I do not think this hunk is correct. [...] Under the precondition checkout-index runs in this test, a casual echo rezrov path1/file1 would leave path1 as a symlink without turning it into a real directory, and we will end up creating path2/file1. We are making sure that checkout-index does not behave that way, and it is essential to have symlink support in the working tree for the bug to trigger. @@ -79,10 +79,10 @@ test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' ' test -h tmp ' -test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' ' +test_expect_success 'use --prefix=tmp- where tmp-path1 is a symlink' ' rm -fr path0 path1 path2 tmp* mkdir tmp1 -ln -s tmp1 tmp-path1 +test_ln_s tmp1 tmp-path1 git checkout-index --prefix=tmp- -f -a test -f tmp-path0 test ! -h tmp-path1 This change has the same issue, I think. Yes, agreed. The converted tests -- when SYMLINKS are not available -- just repeat what is tested elsewhere. Nice catch. After I've found a hammer (test_ln_s) I was mindlessly looking for nails and found two of them in these two instances. ;-) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
Am 04.06.2013 23:55, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh index 88be904..563ac7f 100755 --- a/t/t3000-ls-files-others.sh +++ b/t/t3000-ls-files-others.sh @@ -19,12 +19,7 @@ filesystem. test_expect_success 'setup ' ' date path0 -if test_have_prereq SYMLINKS -then -ln -s xyzzy path1 -else -date path1 -fi +test_ln_s xyzzy path1 mkdir path2 path3 path4 date path2/file2 date path2-junk This also is not appropriate, but it is not as bad as the one in t2003 I earlier commented on. This test wants an untracked symbolic link in the working tree as path1 and wants to see ls-files -o report it as other. On a filesystem that lack symbolic link, we obviously cannot have one, so as a substitute we just create another regular file to make the expected output and comparison simpler. Exactly. This is just a convenience. The issue is not introduced by the conversion, but dates back 4 years when I added the SYMLINKS check. We now only use a less random string on !SYMLINKS filesystems. test_expect_success 'git ls-files -k to show killed files.' ' date path2 -if test_have_prereq SYMLINKS -then -ln -s frotz path3 -ln -s nitfol path5 -else -date path3 -date path5 -fi +test_ln_s frotz path3 +test_ln_s nitfol path5 This falls into the same category as the one in t3000 above. The only thing that matters in this test is path3 and path5 are non directories so that the former is killed when path3/file3 needs to be checked out, while path5 will not appear in ls-files -k output. Ideally we would want to test regular files and symlinks as two different kinds of non directory filesystem entities, but on platforms that lack symbolic links we cannot do so, hence we punt and create a regular file. Indeed. Same answer. diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh -test_expect_success SYMLINKS 'diff identical, but newly created symlink and file' ' +test_expect_success 'diff identical, but newly created symlink and file' ' expected rm -f frotz nitfol echo xyzzy nitfol test-chmtime +10 nitfol -ln -s xyzzy frotz +test_ln_s xyzzy frotz git diff-index -M -p $tree current compare_diff_patch expected current As the point of test is to compare $tree that has symlink with another symlink that is identical but newly created one, I think this _does_ want the filesystem entity to be a symbolic link, but the index has frotz as a symlink and the mode propagates to what we read from the filesystem on !has_symlinks systems, so this conversion may be a correct one, though it is a bit tricky. Yes, this test depends on the mode propagation. I'll add a comment along these lines and keep the change in this patch with a title marked trivial cases. @@ -100,7 +103,7 @@ test_expect_success SYMLINKS 'diff different symlink and file' ' +yxyyz EOF rm -f frotz -ln -s yxyyz frotz +test_ln_s yxyyz frotz echo yxyyz nitfol git diff-index -M -p $tree current compare_diff_patch expected current Likewise. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/7] add simple tests of consistency across rebase types
Am 6/4/2013 7:14, schrieb Martin von Zweigbergk: On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano gits...@pobox.com wrote: + +# checks that the revisions in $2 represent a linear range with the +# subjects in $1 +test_linear_range () { + ! { git log --format=%p $2 | sane_grep ;} An interesting way to spell: test $(git rev-list --merges $2 | wc -l) = 0 Heh, true. I'll change that. Then I think it would be even better written as revlist_merges=$(git rev-list --merges $2) test -z $revlist_merges so as not to ignore errors in the git invocation (and at least one less fork()). -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/11] t3030: use test_ln_s_add to remove SYMLINKS prerequisite
The test cases include many corner-cases of merge-recursive's behavior, some of them involve type changes and symbolic links. All cases, including those that are protected by SYMLINKS check only whether the result of merge-recursive is correctly stored in the database and the index; the file system is not investigated. Use test_ln_s_add to enter a symbolic link in the index in the test setup and run the tests without the SYMLINKS prerequisite. Notice that one test that has the SYMLINKS protection removed is an expect_failure. There is a possibility that the test fails differently depending on whether SYMLINKS is present or not; but this is not the case presently. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3030-merge-recursive.sh | 62 +++--- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index a5e3da7..2f96100 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -25,10 +25,7 @@ test_expect_success 'setup 1' ' git branch submod git branch copy git branch rename - if test_have_prereq SYMLINKS - then - git branch rename-ln - fi + git branch rename-ln echo hello a cp a d/e @@ -260,16 +257,12 @@ test_expect_success 'setup 8' ' git add e test_tick git commit -m rename a-e - if test_have_prereq SYMLINKS - then - git checkout rename-ln - git mv a e - ln -s e a - git add a e - test_tick - git commit -m rename a-e, symlink a-e - oln=`printf e | git hash-object --stdin` - fi + git checkout rename-ln + git mv a e + test_ln_s_add e a + test_tick + git commit -m rename a-e, symlink a-e + oln=`printf e | git hash-object --stdin` ' test_expect_success 'setup 9' ' @@ -569,28 +562,25 @@ test_expect_success 'merge-recursive copy vs. rename' ' test_cmp expected actual ' -if test_have_prereq SYMLINKS -then - test_expect_failure 'merge-recursive rename vs. rename/symlink' ' - - git checkout -f rename - git merge rename-ln - ( git ls-tree -r HEAD ; git ls-files -s ) actual - ( - echo 12 blob $oln a - echo 100644 blob $o0 b - echo 100644 blob $o0 c - echo 100644 blob $o0 d/e - echo 100644 blob $o0 e - echo 12 $oln 0 a - echo 100644 $o0 0 b - echo 100644 $o0 0 c - echo 100644 $o0 0 d/e - echo 100644 $o0 0 e - ) expected - test_cmp expected actual - ' -fi +test_expect_failure 'merge-recursive rename vs. rename/symlink' ' + + git checkout -f rename + git merge rename-ln + ( git ls-tree -r HEAD ; git ls-files -s ) actual + ( + echo 12 blob $oln a + echo 100644 blob $o0 b + echo 100644 blob $o0 c + echo 100644 blob $o0 d/e + echo 100644 blob $o0 e + echo 12 $oln 0 a + echo 100644 $o0 0 b + echo 100644 $o0 0 c + echo 100644 $o0 0 d/e + echo 100644 $o0 0 e + ) expected + test_cmp expected actual +' test_done -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/11] t3100: use test_ln_s_add to remove SYMLINKS prerequisite
This undoes the special casing introduced in this test by 704a3143 (Use prerequisite tags to skip tests that depend on symbolic links, 2009-03-04). Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3100-ls-tree-restrict.sh | 42 +++--- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/t/t3100-ls-tree-restrict.sh b/t/t3100-ls-tree-restrict.sh index 81d90b6..eb73c06 100755 --- a/t/t3100-ls-tree-restrict.sh +++ b/t/t3100-ls-tree-restrict.sh @@ -22,20 +22,8 @@ test_expect_success \ 'setup' \ 'mkdir path2 path2/baz echo Hi path0 - if test_have_prereq SYMLINKS - then - ln -s path0 path1 - ln -s ../path1 path2/bazbo - make_expected () { - cat expected - } - else - printf path0 path1 - printf ../path1 path2/bazbo - make_expected () { - sed -e s/12 /100644 / expected - } - fi + test_ln_s_add path0 path1 + test_ln_s_add ../path1 path2/bazbo echo Lo path2/foo echo Mi path2/baz/b find path? \( -type f -o -type l \) -print | @@ -51,7 +39,7 @@ test_output () { test_expect_success \ 'ls-tree plain' \ 'git ls-tree $tree current - make_expected \EOF + cat expected \EOF 100644 blob X path0 12 blob X path1 04 tree X path2 @@ -61,7 +49,7 @@ EOF test_expect_success \ 'ls-tree recursive' \ 'git ls-tree -r $tree current - make_expected \EOF + cat expected \EOF 100644 blob X path0 12 blob X path1 100644 blob X path2/baz/b @@ -73,7 +61,7 @@ EOF test_expect_success \ 'ls-tree recursive with -t' \ 'git ls-tree -r -t $tree current - make_expected \EOF + cat expected \EOF 100644 blob X path0 12 blob X path1 04 tree X path2 @@ -87,7 +75,7 @@ EOF test_expect_success \ 'ls-tree recursive with -d' \ 'git ls-tree -r -d $tree current - make_expected \EOF + cat expected \EOF 04 tree X path2 04 tree X path2/baz EOF @@ -96,7 +84,7 @@ EOF test_expect_success \ 'ls-tree filtered with path' \ 'git ls-tree $tree path current - make_expected \EOF + cat expected \EOF EOF test_output' @@ -106,7 +94,7 @@ EOF test_expect_success \ 'ls-tree filtered with path1 path0' \ 'git ls-tree $tree path1 path0 current - make_expected \EOF + cat expected \EOF 100644 blob X path0 12 blob X path1 EOF @@ -115,7 +103,7 @@ EOF test_expect_success \ 'ls-tree filtered with path0/' \ 'git ls-tree $tree path0/ current - make_expected \EOF + cat expected \EOF EOF test_output' @@ -124,7 +112,7 @@ EOF test_expect_success \ 'ls-tree filtered with path2' \ 'git ls-tree $tree path2 current - make_expected \EOF + cat expected \EOF 04 tree X path2 EOF test_output' @@ -133,7 +121,7 @@ EOF test_expect_success \ 'ls-tree filtered with path2/' \ 'git ls-tree $tree path2/ current - make_expected \EOF + cat expected \EOF 04 tree X path2/baz 12 blob X path2/bazbo 100644 blob X path2/foo @@ -145,7 +133,7 @@ EOF test_expect_success \ 'ls-tree filtered with path2/baz' \ 'git ls-tree $tree path2/baz current - make_expected \EOF + cat expected \EOF 04 tree X path2/baz EOF test_output' @@ -153,14 +141,14 @@ EOF test_expect_success \ 'ls-tree filtered with path2/bak' \ 'git ls-tree $tree path2/bak current - make_expected \EOF + cat expected \EOF EOF test_output' test_expect_success \ 'ls-tree -t filtered with path2/bak' \ 'git ls-tree -t $tree path2/bak current - make_expected \EOF + cat expected \EOF 04 tree X path2 EOF test_output' @@ -168,7 +156,7 @@ EOF test_expect_success \ 'ls-tree with one path a prefix of the other' \ 'git ls-tree $tree path2/baz path2/bazbo current - make_expected \EOF + cat expected \EOF 04 tree X path2/baz 12 blob X path2/bazbo EOF -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 00/11] Increase test coverage on Windows by removing SYMLINKS from many tests
Many tests that involve symbolic links actually check only whether our algorithms are correct by investigating the contents of the object database and the index. Only some of them check the filesystem. This series introduces a function test_ln_s_add that inserts a symbolic link in the index even if the filesystem does not support symbolic links. By using this function, many more tests can be run when the filesystem does not have symblic links, aka Windows. The patches touch a number of test files that do not follow the modern style. But I modernized only the two test files where the subsequent change to use test_ln_s_add would otherwise be rather inconvenient or obscure. Johannes Sixt (11): test-chmtime: Fix exit code on Windows t2100: modernize style and unroll a loop of test cases t3010: modernize style tests: introduce test_ln_s and test_ln_s_add tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases) t: use test_ln_s_add to remove SYMLINKS prerequisite t2100: use test_ln_s_add to remove SYMLINKS prerequisite t3030: use test_ln_s_add to remove SYMLINKS prerequisite t3100: use test_ln_s_add to remove SYMLINKS prerequisite t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite t6035: use test_ln_s_add to remove SYMLINKS prerequisite t/README | 17 + t/t-basic.sh | 39 +++- t/t1004-read-tree-m-u-wf.sh| 7 +-- t/t2001-checkout-cache-clash.sh| 7 +-- t/t2003-checkout-cache-mkdir.sh| 8 +-- t/t2004-checkout-cache-temp.sh | 5 +- t/t2007-checkout-symlink.sh| 12 ++-- t/t2021-checkout-overwrite.sh | 12 ++-- t/t2100-update-cache-badpath.sh| 71 +++-- t/t2200-add-update.sh | 5 +- t/t3000-ls-files-others.sh | 7 +-- t/t3010-ls-files-killed-modified.sh| 112 +++-- t/t3030-merge-recursive.sh | 62 -- t/t3100-ls-tree-restrict.sh| 42 + t/t3509-cherry-pick-merge-df.sh| 12 ++-- t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 t/t4008-diff-break-rewrite.sh | 12 ++-- t/t4011-diff-symlink.sh| 23 --- t/t4023-diff-rename-typechange.sh | 28 - t/t4030-diff-textconv.sh | 8 +-- t/t4114-apply-typechange.sh| 29 + t/t4115-apply-symlink.sh | 10 ++- t/t4122-apply-symlink-inside.sh| 8 +-- t/t6035-merge-dir-to-symlink.sh| 73 + t/t7001-mv.sh | 18 +++--- t/t7607-merge-overwrite.sh | 5 +- t/t8006-blame-textconv.sh | 14 ++--- t/t8007-cat-file-textconv.sh | 10 ++- t/t9350-fast-export.sh | 5 +- t/t9500-gitweb-standalone-no-errors.sh | 15 ++--- t/test-lib-functions.sh| 30 + test-chmtime.c | 8 +-- 33 files changed, 391 insertions(+), 377 deletions(-) -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/11] t2100: modernize style and unroll a loop of test cases
In particular, move all test preparations inside test_expect_success. In a subsequent patch, we are going to move test case path3 out of the loop of test cases. Use this opportunity to linearize the tests. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t2100-update-cache-badpath.sh | 81 - 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/t/t2100-update-cache-badpath.sh b/t/t2100-update-cache-badpath.sh index 2df3fdd..a3f9255 100755 --- a/t/t2100-update-cache-badpath.sh +++ b/t/t2100-update-cache-badpath.sh @@ -24,38 +24,51 @@ All of the attempts should fail. . ./test-lib.sh -mkdir path2 path3 -date path0 -if test_have_prereq SYMLINKS -then - ln -s xyzzy path1 -else - date path1 -fi -date path2/file2 -date path3/file3 - -test_expect_success \ -'git update-index --add to add various paths.' \ -'git update-index --add -- path0 path1 path2/file2 path3/file3' - -rm -fr path? - -mkdir path0 path1 -date path2 -if test_have_prereq SYMLINKS -then - ln -s frotz path3 -else - date path3 -fi -date path0/file0 -date path1/file1 - -for p in path0/file0 path1/file1 path2 path3 -do - test_expect_success \ - git update-index to add conflicting path $p should fail. \ - test_must_fail git update-index --add -- $p -done +test_expect_success 'git update-index --add to add various paths' ' + + mkdir path2 path3 + date path0 + if test_have_prereq SYMLINKS + then + ln -s xyzzy path1 + else + date path1 + fi + date path2/file2 + date path3/file3 + test_when_finished rm -fr path0 path1 path2 path3 + git update-index --add -- path0 path1 path2/file2 path3/file3 +' + +test_expect_success 'git update-index to add conflicting path path0/file0 should fail' ' + + mkdir path0 + date path0/file0 + test_must_fail git update-index --add -- path0/file0 +' + +test_expect_success 'git update-index to add conflicting path path1/file1 should fail' ' + + mkdir path1 + date path1/file1 + test_must_fail git update-index --add -- path1/file1 +' + +test_expect_success 'git update-index to add conflicting file path2 should fail' ' + + date path2 + test_must_fail git update-index --add -- path2 +' + +test_expect_success 'git update-index to add conflicting symlink path3 should fail' ' + + if test_have_prereq SYMLINKS + then + ln -s xyzzy path3 + else + date path3 + fi + test_must_fail git update-index --add -- path3 +' + test_done -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/11] t3509, t4023, t4114: use test_ln_s_add to remove SYMLINKS prerequisite
In t4023 and t4114, we have to remove the entries using 'git rm' because otherwise the entries that must turn from symbolic links to regular files would stay symbolic links in the index. For the same reason, we have to use 'git mv' instead of plain 'mv' in t3509. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3509-cherry-pick-merge-df.sh | 12 +--- t/t4023-diff-rename-typechange.sh | 28 ++-- t/t4114-apply-typechange.sh | 29 ++--- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/t/t3509-cherry-pick-merge-df.sh b/t/t3509-cherry-pick-merge-df.sh index df921d1..a5b6a5f 100755 --- a/t/t3509-cherry-pick-merge-df.sh +++ b/t/t3509-cherry-pick-merge-df.sh @@ -10,17 +10,15 @@ test_expect_success 'Initialize repository' ' git commit -m a ' -test_expect_success SYMLINKS 'Setup rename across paths each below D/F conflicts' ' +test_expect_success 'Setup rename across paths each below D/F conflicts' ' mkdir b - ln -s ../a b/a - git add b + test_ln_s_add ../a b/a git commit -m b git checkout -b branch rm b/a - mv a b/a - ln -s b/a a - git add . + git mv a b/a + test_ln_s_add b/a a git commit -m swap f1 @@ -28,7 +26,7 @@ test_expect_success SYMLINKS 'Setup rename across paths each below D/F conflicts git commit -m f1 ' -test_expect_success SYMLINKS 'Cherry-pick succeeds with rename across D/F conflicts' ' +test_expect_success 'Cherry-pick succeeds with rename across D/F conflicts' ' git reset --hard git checkout master^0 git cherry-pick branch diff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh index 5d20acf..55d549f 100755 --- a/t/t4023-diff-rename-typechange.sh +++ b/t/t4023-diff-rename-typechange.sh @@ -4,44 +4,44 @@ test_description='typechange rename detection' . ./test-lib.sh -test_expect_success SYMLINKS setup ' +test_expect_success setup ' rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo - ln -s linklink bar - git add foo bar + test_ln_s_add linklink bar + git add foo git commit -a -m Initial git tag one - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING bar - ln -s linklink foo - git add foo bar + test_ln_s_add linklink foo + git add bar git commit -a -m Second git tag two - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo git add foo git commit -a -m Third git tag three mv foo bar - ln -s linklink foo - git add foo bar + test_ln_s_add linklink foo + git add bar git commit -a -m Fourth git tag four # This is purely for sanity check - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../COPYING foo cat $TEST_DIRECTORY/../Makefile bar git add foo bar git commit -a -m Fifth git tag five - rm -f foo bar + git rm -f foo bar cat $TEST_DIRECTORY/../Makefile foo cat $TEST_DIRECTORY/../COPYING bar git add foo bar @@ -50,7 +50,7 @@ test_expect_success SYMLINKS setup ' ' -test_expect_success SYMLINKS 'cross renames to be detected for regular files' ' +test_expect_success 'cross renames to be detected for regular files' ' git diff-tree five six -r --name-status -B -M | sort actual { @@ -61,7 +61,7 @@ test_expect_success SYMLINKS 'cross renames to be detected for regular files' ' ' -test_expect_success SYMLINKS 'cross renames to be detected for typechange' ' +test_expect_success 'cross renames to be detected for typechange' ' git diff-tree one two -r --name-status -B -M | sort actual { @@ -72,7 +72,7 @@ test_expect_success SYMLINKS 'cross renames to be detected for typechange' ' ' -test_expect_success SYMLINKS 'moves and renames' ' +test_expect_success 'moves and renames' ' git diff-tree three four -r --name-status -B -M | sort actual { diff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh index f12826f..ebadbc3 100755 --- a/t/t4114-apply-typechange.sh +++ b/t/t4114-apply-typechange.sh @@ -9,20 +9,19 @@ test_description='git apply should not get confused with type changes. . ./test-lib.sh -test_expect_success SYMLINKS 'setup repository and commits' ' +test_expect_success 'setup repository and commits' ' echo hello world foo echo hi planet bar git update-index --add foo bar git commit -m initial git branch initial rm -f foo - ln -s bar foo - git update-index foo + test_ln_s_add bar foo git commit -m foo symlinked to bar git branch foo-symlinked-to-bar - rm
[PATCH 05/11] tests: use test_ln_s_add to remove SYMLINKS prerequisite (trivial cases)
There are many instances where the treatment of symbolic links in the object model and the algorithms are tested, but where it is not necessary to actually have a symbolic link in the worktree. Make adjustments to the tests and remove the SYMLINKS prerequisite when appropriate in trivial cases, where trivial means: - merely a replacement of 'ln -s a b' to test_ln_s or of 'ln -s a b git add b' to test_ln_s_add is needed; - a test for symbolic link on the file system can be split off (and remains protected by SYMLINKS); - existing code is equivalent to test_ln_s[_add]. Signed-off-by: Johannes Sixt j...@kdbg.org --- The changes in t9500-gitweb-* were not tested on a system that does not have SYMLINKS. t/t1004-read-tree-m-u-wf.sh| 7 +++--- t/t2001-checkout-cache-clash.sh| 7 +++--- t/t2003-checkout-cache-mkdir.sh| 8 +++ t/t2004-checkout-cache-temp.sh | 5 ++--- t/t2007-checkout-symlink.sh| 12 +-- t/t2021-checkout-overwrite.sh | 12 +++ t/t2200-add-update.sh | 5 ++--- t/t3000-ls-files-others.sh | 7 +- t/t3010-ls-files-killed-modified.sh| 19 - t/t3700-add.sh | 15 ++--- t/t3903-stash.sh | 39 -- t/t4008-diff-break-rewrite.sh | 12 +-- t/t4011-diff-symlink.sh| 23 +++- t/t4030-diff-textconv.sh | 8 +++ t/t4115-apply-symlink.sh | 10 - t/t4122-apply-symlink-inside.sh| 8 +++ t/t7001-mv.sh | 18 ++-- t/t7607-merge-overwrite.sh | 5 ++--- t/t8006-blame-textconv.sh | 14 +--- t/t8007-cat-file-textconv.sh | 10 - t/t9350-fast-export.sh | 5 ++--- t/t9500-gitweb-standalone-no-errors.sh | 15 + 22 files changed, 126 insertions(+), 138 deletions(-) diff --git a/t/t1004-read-tree-m-u-wf.sh b/t/t1004-read-tree-m-u-wf.sh index b3ae7d5..3e72aff 100755 --- a/t/t1004-read-tree-m-u-wf.sh +++ b/t/t1004-read-tree-m-u-wf.sh @@ -158,7 +158,7 @@ test_expect_success '3-way not overwriting local changes (their side)' ' ' -test_expect_success SYMLINKS 'funny symlink in work tree' ' +test_expect_success 'funny symlink in work tree' ' git reset --hard git checkout -b sym-b side-b @@ -170,15 +170,14 @@ test_expect_success SYMLINKS 'funny symlink in work tree' ' rm -fr a git checkout -b sym-a side-a mkdir -p a - ln -s ../b a/b - git add a/b + test_ln_s_add ../b a/b git commit -m we add a/b read_tree_u_must_succeed -m -u sym-a sym-a sym-b ' -test_expect_success SYMLINKS,SANITY 'funny symlink in work tree, un-unlink-able' ' +test_expect_success SANITY 'funny symlink in work tree, un-unlink-able' ' rm -fr a b git reset --hard diff --git a/t/t2001-checkout-cache-clash.sh b/t/t2001-checkout-cache-clash.sh index 98aa73e..1fc8e63 100755 --- a/t/t2001-checkout-cache-clash.sh +++ b/t/t2001-checkout-cache-clash.sh @@ -59,10 +59,9 @@ test_expect_success \ 'git read-tree -m $tree1 git checkout-index -f -a' test_debug 'show_files $tree1' -test_expect_success SYMLINKS \ -'git update-index --add a symlink.' \ -'ln -s path0 path1 - git update-index --add path1' +test_expect_success \ +'add a symlink' \ +'test_ln_s_add path0 path1' test_expect_success \ 'writing tree out with git write-tree' \ 'tree3=$(git write-tree)' diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh index ff163cf..bd17ba2 100755 --- a/t/t2003-checkout-cache-mkdir.sh +++ b/t/t2003-checkout-cache-mkdir.sh @@ -19,10 +19,10 @@ test_expect_success 'setup' ' git update-index --add path0 path1/file1 ' -test_expect_success SYMLINKS 'have symlink in place where dir is expected.' ' +test_expect_success 'have symlink in place where dir is expected.' ' rm -fr path0 path1 mkdir path2 - ln -s path2 path1 + test_ln_s path2 path1 git checkout-index -f -a test ! -h path1 test -d path1 test -f path1/file1 test ! -f path2/file1 @@ -79,10 +79,10 @@ test_expect_success SYMLINKS 'use --prefix=tmp/orary- where tmp is a symlink' ' test -h tmp ' -test_expect_success SYMLINKS 'use --prefix=tmp- where tmp-path1 is a symlink' ' +test_expect_success 'use --prefix=tmp- where tmp-path1 is a symlink' ' rm -fr path0 path1 path2 tmp* mkdir tmp1 - ln -s tmp1 tmp-path1 + test_ln_s tmp1 tmp-path1 git checkout-index --prefix=tmp- -f -a test -f tmp-path0 test ! -h tmp-path1 diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh index 0f4b289..f171a55 100755 --- a/t/t2004-checkout-cache-temp.sh +++ b/t/t2004-checkout-cache
[PATCH 07/11] t2100: use test_ln_s_add to remove SYMLINKS prerequisite
One converted case expects 'git update-index' to fail. We cannot use 'test_must_fail test_ln_s_add ...', because that would mistakenly pass if a command other than the final git-update-index of test_ln_s_add failed. Hence, use test_ln_s to create the symbolic link and keep the explicit 'test_must_fail git update-index'. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t2100-update-cache-badpath.sh | 14 ++ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/t/t2100-update-cache-badpath.sh b/t/t2100-update-cache-badpath.sh index a3f9255..47a9b05 100755 --- a/t/t2100-update-cache-badpath.sh +++ b/t/t2100-update-cache-badpath.sh @@ -28,12 +28,7 @@ test_expect_success 'git update-index --add to add various paths' ' mkdir path2 path3 date path0 - if test_have_prereq SYMLINKS - then - ln -s xyzzy path1 - else - date path1 - fi + test_ln_s_add xyzzy path1 date path2/file2 date path3/file3 test_when_finished rm -fr path0 path1 path2 path3 @@ -62,12 +57,7 @@ test_expect_success 'git update-index to add conflicting file path2 should fail' test_expect_success 'git update-index to add conflicting symlink path3 should fail' ' - if test_have_prereq SYMLINKS - then - ln -s xyzzy path3 - else - date path3 - fi + test_ln_s xyzzy path3 test_must_fail git update-index --add -- path3 ' -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/11] t3010: modernize style
In particular: - move test preparations inside test_expect_success - place test description on the test_expect_success line - indent with a tab Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t3010-ls-files-killed-modified.sh | 123 ++-- 1 file changed, 61 insertions(+), 62 deletions(-) diff --git a/t/t3010-ls-files-killed-modified.sh b/t/t3010-ls-files-killed-modified.sh index 95671c2..2d0ff2d 100755 --- a/t/t3010-ls-files-killed-modified.sh +++ b/t/t3010-ls-files-killed-modified.sh @@ -37,71 +37,70 @@ modified without reporting path9 and path10. ' . ./test-lib.sh -date path0 -if test_have_prereq SYMLINKS -then - ln -s xyzzy path1 -else - date path1 -fi -mkdir path2 path3 -date path2/file2 -date path3/file3 -: path7 -date path8 -: path9 -date path10 -test_expect_success \ -'git update-index --add to add various paths.' \ -git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 - -rm -fr path? ;# leave path10 alone -date path2 -if test_have_prereq SYMLINKS -then - ln -s frotz path3 - ln -s nitfol path5 -else - date path3 - date path5 -fi -mkdir path0 path1 path6 -date path0/file0 -date path1/file1 -date path6/file6 -date path7 -: path8 -: path9 -touch path10 +test_expect_success 'git update-index --add to add various paths.' ' + date path0 + if test_have_prereq SYMLINKS + then + ln -s xyzzy path1 + else + date path1 + fi + mkdir path2 path3 + date path2/file2 + date path3/file3 + : path7 + date path8 + : path9 + date path10 + git update-index --add -- path0 path1 path?/file? path7 path8 path9 path10 + rm -fr path?# leave path10 alone +' -test_expect_success \ -'git ls-files -k to show killed files.' \ -'git ls-files -k .output' -cat .expected EOF -path0/file0 -path1/file1 -path2 -path3 -EOF +test_expect_success 'git ls-files -k to show killed files.' ' + date path2 + if test_have_prereq SYMLINKS + then + ln -s frotz path3 + ln -s nitfol path5 + else + date path3 + date path5 + fi + mkdir path0 path1 path6 + date path0/file0 + date path1/file1 + date path6/file6 + date path7 + : path8 + : path9 + touch path10 + git ls-files -k .output +' -test_expect_success \ -'validate git ls-files -k output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -k output.' ' + cat .expected -\EOF + path0/file0 + path1/file1 + path2 + path3 + EOF + test_cmp .expected .output +' -test_expect_success \ -'git ls-files -m to show modified files.' \ -'git ls-files -m .output' -cat .expected EOF -path0 -path1 -path2/file2 -path3/file3 -path7 -path8 -EOF +test_expect_success 'git ls-files -m to show modified files.' ' + git ls-files -m .output +' -test_expect_success \ -'validate git ls-files -m output.' \ -'test_cmp .expected .output' +test_expect_success 'validate git ls-files -m output.' ' + cat .expected -\EOF + path0 + path1 + path2/file2 + path3/file3 + path7 + path8 + EOF + test_cmp .expected .output +' test_done -- 1.8.3.rc1.32.g8b61cbb -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] t0000: use test_ln_s_add to remove SYMLINKS prerequisite
t-basic hard-codes many object IDs. To cater to file systems that do not support symbolic links, different IDs are used depending on the SYMLINKS prerequisite. But we can observe the symbolic links are only needed to generate index entries. Use test_ln_s_add to generate the index entries and get rid of explicit SYMLINKS checks. This undoes the special casing introduced in this test by 704a3143 (Use prerequisite tags to skip tests that depend on symbolic links, 2009-03-04). Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t-basic.sh | 39 ++- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index cefe33d..0f13180 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -367,22 +367,6 @@ test_expect_success 'validate object ID of a known tree' ' # Various types of objects -# Some filesystems do not support symblic links; on such systems -# some expected values are different -if test_have_prereq SYMLINKS -then - expectfilter=cat - expectedtree=087704a96baf1c2d1c869a8b084481e121c88b5b - expectedptree1=21ae8269cacbe57ae09138dcc3a2887f904d02b3 - expectedptree2=3c5e5399f3a333eddecce7a9b9465b63f65f51e2 -else - expectfilter='grep -v sym' - expectedtree=8e18edf7d7edcf4371a3ac6ae5f07c2641db7c46 - expectedptree1=cfb8591b2f65de8b8cc1020cd7d9e67e7793b325 - expectedptree2=ce580448f0148b985a513b693fdf7d802cacb44f -fi - - test_expect_success 'adding various types of objects with git update-index --add' ' mkdir path2 path3 path3/subp3 paths=path0 path2/file2 path3/file3 path3/subp3/file3 @@ -390,10 +374,7 @@ test_expect_success 'adding various types of objects with git update-index --add for p in $paths do echo hello $p $p || exit 1 - if test_have_prereq SYMLINKS - then - ln -s hello $p ${p}sym || exit 1 - fi + test_ln_s_add hello $p ${p}sym || exit 1 done ) find path* ! -type d -print | xargs git update-index --add @@ -405,7 +386,7 @@ test_expect_success 'showing stage with git ls-files --stage' ' ' test_expect_success 'validate git ls-files output for a known tree' ' - $expectfilter expected -\EOF + cat expected -\EOF 100644 f87290f8eb2cbbea7857214459a0739927eab154 0 path0 12 15a98433ae33114b085f3eb3bb03b832b3180a01 0 path0sym 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0 path2/file2 @@ -423,14 +404,14 @@ test_expect_success 'writing tree out with git write-tree' ' ' test_expect_success 'validate object ID for a known tree' ' - test $tree = $expectedtree + test $tree = 087704a96baf1c2d1c869a8b084481e121c88b5b ' test_expect_success 'showing tree with git ls-tree' ' git ls-tree $tree current ' -test_expect_success SYMLINKS 'git ls-tree output for a known tree' ' +test_expect_success 'git ls-tree output for a known tree' ' cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym @@ -447,7 +428,7 @@ test_expect_success 'showing tree with git ls-tree -r' ' ' test_expect_success 'git ls-tree -r output for a known tree' ' - $expectfilter expected -\EOF + cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym 100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7path2/file2 @@ -465,7 +446,7 @@ test_expect_success 'showing tree with git ls-tree -r -t' ' git ls-tree -r -t $tree current ' -test_expect_success SYMLINKS 'git ls-tree -r output for a known tree' ' +test_expect_success 'git ls-tree -r output for a known tree' ' cat expected -\EOF 100644 blob f87290f8eb2cbbea7857214459a0739927eab154path0 12 blob 15a98433ae33114b085f3eb3bb03b832b3180a01path0sym @@ -487,7 +468,7 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' ' ' test_expect_success 'validate object ID for a known tree' ' - test $ptree = $expectedptree1 + test $ptree = 21ae8269cacbe57ae09138dcc3a2887f904d02b3 ' test_expect_success 'writing partial tree out with git write-tree --prefix' ' @@ -495,7 +476,7 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' ' ' test_expect_success 'validate object ID for a known tree' ' - test $ptree = $expectedptree2 + test $ptree = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2 ' test_expect_success 'put invalid objects into the index' ' @@ -529,7 +510,7 @@ test_expect_success 'git read-tree followed by write-tree should be idempotent' ' test_expect_success 'validate git diff-files
[PATCH 11/11] t6035: use test_ln_s_add to remove SYMLINKS prerequisite
All tests in t6035 are protected by SYMLINKS. But that is not necessary, because a lot of the functionality can be tested provided symbolic link entries enter the index and object data base. Use test_ln_s_add for this purpose. Some test cases do test the presence of symbolic links on the file system. Move these tests into separate test cases that remain protected by SYMLINKS. There is one instance of expect_failure. There is a possibility that this test case fails differently depending on whether SYMLINKS is present or not; but this is not the case. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t6035-merge-dir-to-symlink.sh | 73 ++--- 1 file changed, 47 insertions(+), 26 deletions(-) diff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh index 2599ae5..9324ea4 100755 --- a/t/t6035-merge-dir-to-symlink.sh +++ b/t/t6035-merge-dir-to-symlink.sh @@ -3,7 +3,7 @@ test_description='merging when a directory was replaced with a symlink' . ./test-lib.sh -test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' ' +test_expect_success 'create a commit where dir a/b changed to symlink' ' mkdir -p a/b/c a/b-2/c a/b/c/d a/b-2/c/d @@ -12,12 +12,12 @@ test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' git commit -m base git tag start rm -rf a/b - ln -s b-2 a/b git add -A + test_ln_s_add b-2 a/b git commit -m dir to symlink ' -test_expect_success SYMLINKS 'checkout does not clobber untracked symlink' ' +test_expect_success 'checkout does not clobber untracked symlink' ' git checkout HEAD^0 git reset --hard master git rm --cached a/b @@ -25,7 +25,7 @@ test_expect_success SYMLINKS 'checkout does not clobber untracked symlink' ' test_must_fail git checkout start^0 ' -test_expect_success SYMLINKS 'a/b-2/c/d is kept when clobbering symlink b' ' +test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' ' git checkout HEAD^0 git reset --hard master git rm --cached a/b @@ -34,14 +34,14 @@ test_expect_success SYMLINKS 'a/b-2/c/d is kept when clobbering symlink b' ' test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'checkout should not have deleted a/b-2/c/d' ' +test_expect_success 'checkout should not have deleted a/b-2/c/d' ' git checkout HEAD^0 git reset --hard master git checkout start^0 test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'setup for merge test' ' +test_expect_success 'setup for merge test' ' git reset --hard test -f a/b-2/c/d echo x a/x @@ -50,39 +50,51 @@ test_expect_success SYMLINKS 'setup for merge test' ' git tag baseline ' -test_expect_success SYMLINKS 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' ' +test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (resolve)' ' git reset --hard git checkout baseline^0 git merge -s resolve master - test -h a/b test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle D/F conflict, do not lose a/b-2/c/d in merge (recursive)' ' git reset --hard git checkout baseline^0 git merge -s recursive master - test -h a/b test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (resolve)' ' git reset --hard git checkout master^0 git merge -s resolve baseline^0 - test -h a/b test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_success 'Handle F/D conflict, do not lose a/b-2/c/d in merge (recursive)' ' git reset --hard git checkout master^0 git merge -s recursive baseline^0 - test -h a/b test -f a/b-2/c/d ' -test_expect_failure SYMLINKS 'do not lose untracked in merge (resolve)' ' +test_expect_success SYMLINKS 'a/b was resolved as symlink' ' + test -h a/b +' + +test_expect_failure 'do not lose untracked in merge (resolve)' ' git reset --hard git checkout baseline^0 a/b/c/e @@ -91,7 +103,7 @@ test_expect_failure SYMLINKS 'do not lose untracked in merge (resolve)' ' test -f a/b-2/c/d ' -test_expect_success SYMLINKS 'do not lose untracked in merge (recursive)' ' +test_expect_success 'do not lose
Re: [PATCH 04/11] tests: introduce test_ln_s and test_ln_s_add
Am 01.06.2013 13:11, schrieb Ramkumar Ramachandra: Johannes Sixt wrote: +test_ln_s () { + if test_have_prereq SYMLINKS + then + ln -s $1 $2 + else + printf '%s' $1 $2 + fi +} What is this? We can't test_ln_s something and then 'git add' it, so what purpose does this serve? Sure, we can 'git add' it: # - Use test_ln_s instead of ln -s x y when y has been added as a # symbolic link entry earlier. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/7] add tests for rebasing merged history
Am 31.05.2013 08:49, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i +#\ \ +# e---u +# +# uppercase = cherry-picked +# h = reverted g ... +test_expect_success rebase -p --onto in merged history drops patches in upstream + reset_rebase + git rebase -p --onto f h u + test_cmp_rev f HEAD~3 + test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD + + +test_expect_success rebase -p --onto in merged history does not drop patches in onto + reset_rebase + git rebase -p --onto h f u + test_cmp_rev h HEAD~3 + test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD + I noticed one new aspect: The interdiff between v2 and v3 looks like this: -test_expect_failure rebase -p --onto in merged history does not lose patches in upstream +test_expect_success rebase -p --onto in merged history drops patches in upstream reset_rebase git rebase -p --onto f h u test_cmp_rev f HEAD~3 - test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD + test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD -test_expect_success rebase -p --onto in merged history drops patches in onto +test_expect_success rebase -p --onto in merged history does not drop patches in onto reset_rebase git rebase -p --onto h f u test_cmp_rev h HEAD~3 - test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD + test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD The expectations that these two tests check changed from v2 to v3. Notice that former test goes from expect_failure to expect_success, as it should, but the latter does not change. Strange, isn't it? The reason is that this check is incomplete: test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD and allowed the latter test in the v2 form to pass. It should be test_revision_subjects 'd i d e u' HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD The check of the latter test should be: test_revision_subjects 'd G i d e u' HEAD^2~2 HEAD^2^ HEAD^2 HEAD~2 HEAD^ HEAD i.e. check all the way back to the mergebase via both branches. This can be extrapolated to all tests that reconstruct mergy history (not just these two cases). -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: t0008-ignores failure
Am 30.05.2013 04:55, schrieb Jeff King: So while it would be nice to work on paths with colons everywhere, I doubt it is worth the effort to start checking it through the whole test suite. And on top of it, on Windows, you can't have a path component or file name that contains a colon... -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/7] add tests for rebasing with patch-equivalence present
Am 30.05.2013 07:30, schrieb Martin von Zweigbergk: On Wed, May 29, 2013 at 12:09 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i ... +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto drops patches in onto + reset_rebase + git rebase $* --onto h f i + test_cmp_rev h HEAD~2 + test_linear_range 'd i' h.. Isn't this expectation wrong? The upstream of the rebased branch is f, and it does not contain G. Hence, G should be replayed. Since h is the reversal of g, the state at h is the same as at c, and applying G should succeed (it is the same change as g). Therefore, I think the correct expectation is: test_linear_range 'd G i' h.. Good question! It is really not obvious what the right answer is. Some arguments in favor of dropping 'G': 1. Let's say origin/master points to 'b' when you start the 'd G i' branch. You then send the 'G' patch to Junio who applies it as 'g' (cherry-picking direction is reversed compared to figure, but same effect). You then git pull --rebase when master on origin points to 'h'. Because of the cleverness in 'git pull --rebase', it issues 'git rebase --onto h b i'. The reason for this git pull cleverness is to be prepared for rewritten history: b'--c'--g'--h' / a---b \ d---G---i to avoid that b is rebased. In this case it's clearly useful to have the patch dropped. 2. In the test a little before the above one, we instead do 'git rebase --onto f h i' and make sure that the 'G' is _not_ lost. In that case it doesn't matter what's in $branch..$upstream. Do we agree that $branch..$upstream should never matter (instead, $upstream is only used to find merge base with $branch)? No, we do not agree. $branch..$upstream should be the set of patches that should be omitted. $branch..$onto should not matter. $onto is only used to specify the destination of the rebased commits. Do we also agree that 'git rebase a b' should be identical to 'git rebase --onto a a b'? Absolutely! Because 'git rebase h i' should clearly drop 'G', then so should 'git rebase --onto h h i'. Yes! Then, if we agreed that $branch..$upstream doesn't matter, 'git rebase --onto h f i' should behave the same, no? Correct in the mathematically logical sense. ;) But we do not agree that $branch..$upstream doesn't matter. The set of commits to rebase that I was thinking of using was $upstream..$branch, unless equivalent with patch in $branch..$onto. But I'm not very confident about my conclusions above :-) At least the man page says that ..$upstream counts and $onto tells just the new base. The way how git pull calls rebase should be revisited, I think. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: zsh: improve bash script loading
Am 5/29/2013 5:24, schrieb Felipe Contreras: +if [ -z $script ]; then + local -a locations + locations=( + '/etc/bash_completion.d/git' # fedora, old debian + '/usr/share/bash-completion/completions/git' # arch, ubuntu, new debian + '/usr/share/bash-completion/git' # gentoo + $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash + ) Won't you need local e here, or does it not matter? + for e in $locations; do + test -f $e script=$e break + done +fi -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/7] add tests for rebasing root
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +test_run_rebase () { + result=$1 + shift + test_expect_$result rebase $* --onto --root with merge-base does not go to root + reset_rebase + git rebase $* --onto m --root g + test_cmp_rev m HEAD~2 + test_linear_range 'c g' m.. Here you check the outcome. There is no explicit check whether the rebase attempted to replay a and b. But that check is implicit: If a or b were attempted to replay, the rebase would have been interrupted with no new changes. Right? + +} + +test_run_rebase success '' +test_run_rebase success -m +test_run_rebase success -i +test_run_rebase failure -p Just curious: Does the last one fail because the result is not correct or because it does go to the root? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] add tests for rebasing merged history
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# a---b---c +# \ \ +# d---e \ +#\ \ \ +# n---o---w---v +# \ +# z +#TODO: make all flavors of rebase use --topo-order +test_run_rebase success 'e n o' '' +test_run_rebase success 'e n o' -m +test_run_rebase success 'n o e' -i As test_commit offers predictable timestamps, I think you can work around this discrepancy by generating commits n and o before e. (That is not a solution--just a workaround that depends on the current implementation--because the order in which parents of a merge are listed is unspecified.) +test_expect_success rebase -p re-creates internal merge + reset_rebase + git rebase -p c w + test_revision_subjects 'c d n e o w' HEAD~4 HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD Shouldn't this better be test_cmp_rev c HEAD~4 test_revision_subjects 'd n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD to ensure that c is not a rewritten commit? + + +test_expect_success rebase -p can re-create two branches on onto + reset_rebase + git rebase -p --onto c d w + test_revision_subjects 'c n e o w' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD + Same here. Time is fleeting. I have to stop here. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/7] add tests for rebasing merged history
Am 5/29/2013 8:39, schrieb Martin von Zweigbergk: +# f +# / +# a---b---c---g---h +# \ +# d---G---i +#\ \ +# e---u +# +# uppercase = cherry-picked +# h = reverted g +test_expect_failure rebase -p --onto in merged history does not lose patches in upstream + reset_rebase + git rebase -p --onto f h u + test_cmp_rev f HEAD~3 + test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD + My expectations are different: When a patch is in upstream, then it is not to be rebased, even --onto somewhere else than upstream. But take this with a grain of salt, as I never encounter(ed) this use-case in practice. +test_expect_success rebase -p --onto in merged history drops patches in onto + reset_rebase + git rebase -p --onto h f u + test_cmp_rev h HEAD~3 + test_revision_subjects 'd i e u' HEAD~2 HEAD^2 HEAD^ HEAD + And this is just the opposite case, where I think the patch should be kept. +# a---b---c +# \ +# d---e +#\ \ +# n---r +# \ +# o +# +# r = tree-same with n +# uppercace = cherry-picked I do not see any upper-cased letters in this graph. ;) +test_expect_success rebase -p re-creates empty internal merge commit + reset_rebase + git rebase -p c r + test_revision_subjects 'c d e n r' HEAD~3 HEAD~2 HEAD^2 HEAD^ HEAD Again, check c with test_cmp_rev. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff: add --ignore-blank-lines option
Am 26.05.2013 19:58, schrieb Antoine Pelisse: The goal of the patch is to introduce the GNU diff -B/--ignore-blank-lines as closely as possible. The short option is not available because it's already used for break-rewrites. When this option is used, git-diff will not create hunks that simply adds or removes empty lines, but will still show empty lines addition/suppression if they are close enough to valuable changes. So when an addition or removal of a blank line appears in a hunk that also has non-blank-line changes, the addition or removal is not treated specially? How is a blank line defined? What happens if a line that has only whitespace is added or removed? I'm thinking of diffs of files with CRLF line breaks, where the CR would count as whitespace in the line, I think. +--ignore-blank-lines:: + Ignore changes whose lines are all blank. I think this is too terse and does not convey what the option really does. +test_expect_success 'ignore-blank-lines: only new lines' ' + seq 5 x Please use test_seq instead of seq in all new tests. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make git ignore the timestamp embedded in PDFs
Am 18.05.2013 09:42, schrieb Andreas Leha: Am 14.05.2013 15:17, schrieb Andreas Leha: Hi all, how can I make git ignore the time stamp(s) in a PDF. Two PDFs that differ only in these time stamps should be considered identical. ... What I tried is a filter: ,[ ~/.gitconfig ] | [filter pdfresetdate] | clean = pdfresetdate ` This 'works' as far as the committed pdf indeed has the date reset to my default value. However, when I re-checkout the files, they are marked modified by git. I'm using cleaned files every now and then, but not on Linux. I have never observed this behavior recently. If you 'git add' the file, does it keep its modified state? Does 'git yes. diff' tell a difference? no. I do not believe you. I'm sure that Binary files differ was reported. The reason is that your pdfresetdate script is not idempotent. Look: $ pdfresetdate x.pdf y.pdf $ pdfresetdate y.pdf z.pdf $ md5sum x.pdf y.pdf z.pdf c46a7097574a035e89d1a46d93c83528 x.pdf 8e6d942b4cc7d8a4dfe6898867573617 y.pdf e6333bc0f8ab9781d3e1d811a392d516 z.pdf A file that was already cleaned by the clean filter must not be modified, i.e., the y.pdf and z.pdf should be identical. But they are not. Fix your clean filter. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: .gitignore behavior on Mac
Am 18.05.2013 20:55, schrieb John Keeping: On Sat, May 18, 2013 at 08:43:57PM +0200, Peter Lauri wrote: But I just don't want to see that darn file. It is a config file that I have changed, and I don't want to need to stash it for each git svn action I want to perform... Any solution for that? Read about --assume-unchanged in git-update-index(1). Beware!! --assume-unchanged is a promise not to modify a file, but that is not true in this case, because it *was* modified. It might hide the file from the git-status output, but then git might do something unexpected sometimes, because a promise was not kept. See last paragraph of http://article.gmane.org/gmane.comp.version-control.git/146353 -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching
Am 17.05.2013 19:02, schrieb Thomas Rast: At this point the splitting has already happened in the caller when it does the (refactored) + if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS So $@ and $@ is actually the same thing. Not in general: If you omit the double-quotes, the individual words still undergo path name expansion and the result depends on the files that happen to match patterns given in $GIT_SKIP_TESTS. This is not a new problem at the call site of the new function, but we shouldn't duplicate the same problem in the implementation of the function. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/6] test-lib: refactor $GIT_SKIP_TESTS matching
Am 5/16/2013 22:50, schrieb Thomas Rast: +match_pattern_list () { + arg=$1 + shift + test -z $* return 1 + for pat in $@ You should have double-quotes around $@ here, but then you can just as well abbreviate to for pat and you don't need the 'test -z $*' check anymore. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Trouble with case insensitive filesystem
Am 5/15/2013 10:40, schrieb Luc Bourhis: I work on a case insensitive filesystem and I have core.ignorecase set to true. ... So I thought it was a job for git filter-branch, ... However because of those two blobs, I have: ~ git status # modified: .../fourCircles.py and git filter-branch therefore refuses to run. Make a commit that has neither file, run git filter-branch, then throw away the commit with git reset --hard HEAD~. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: make git ignore the timestamp embedded in PDFs
Am 14.05.2013 15:17, schrieb Andreas Leha: Hi all, how can I make git ignore the time stamp(s) in a PDF. Two PDFs that differ only in these time stamps should be considered identical. ... What I tried is a filter: ,[ ~/.gitconfig ] | [filter pdfresetdate] | clean = pdfresetdate ` This 'works' as far as the committed pdf indeed has the date reset to my default value. However, when I re-checkout the files, they are marked modified by git. I'm using cleaned files every now and then, but not on Linux. I have never observed this behavior recently. If you 'git add' the file, does it keep its modified state? Does 'git diff' tell a difference? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Avoiding broken Gitweb links and deleted objects
Am 5/8/2013 18:16, schrieb Matt McClure: That begs a follow-up question. It sounds as though Git will typically delete unreachable objects. My team often shares links like https://git.example.com/foo.git/log/d59051721bb0a3758f7c6ea0452bac122a377645?hp=0055e0959cd13780494fe33832bae9bcf91e4a90 . If I later rebase the branch containing those commits and d590517 becomes unreachable, do I risk that link breaking when Git deletes d590517? Yes. When we explain 'rebase', we usually say you make the life hard for people who build on (published) history that you later rebase. But you inconvenience not only people who build their own history on top of your outdated history, but also those who operate with (web) links into that history. What's a good strategy for avoiding breaking those links? Do not rebase published history. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] fast-export: add new --refspec option
Am 5/10/2013 3:13, schrieb Junio C Hamano: * On the other hand, git log 'fc/*' might be a handy thing for any command that wants to have multiple starting points for revision traversal, so in principle I would not mind such an enhancement to rev-list machinery. Currently, we spell this as git log --branches=fc or git log --branches='*export*'. But I do not mean to say that git log fc/\* would be a bad thing to have. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Avoiding broken Gitweb links and deleted objects
Am 5/10/2013 8:37, schrieb Junio C Hamano: What if we teach git rebase to record, perhaps by default, an ours merge on top of Y that takes the tree state of Y but has X as its second parent, ... Please let's not go that route... -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: `git prune` doc or implementation defect, or user misunderstanding
Am 5/8/2013 16:19, schrieb Matt McClure: My interpretation of that is that `git prune` will not prune packed objects by default. The following behavior seems inconsistent with that interpretation. [git@438587-beefcake01 panama.git]$ git prune -n | wc -l 9210 You have 9210 unreachable, loose objects. [git@438587-beefcake01 panama.git]$ git fsck --unreachable | wc -l 9468 You have 9468 unreachable objects in total. [git@438587-beefcake01 panama.git]$ git gc --no-prune Counting objects: 531223, done. Delta compression using up to 24 threads. Compressing objects: 100% (109848/109848), done. Writing objects: 100% (531223/531223), done. Total 531223 (delta 405288), reused 530894 (delta 404961) Only reachable objects go into the new pack. Unreachable objects that were in the pack before, are evicted and are now loose. [git@438587-beefcake01 panama.git]$ git prune -n | wc -l 9468 [git@438587-beefcake01 panama.git]$ git fsck --unreachable | wc -l 9468 Now all 9468 unreachable objects are loose and eligible for being pruned. It looks like `git prune -n` is telling me that it would prune the objects that I just packed. What am I misunderstanding? git gc moves unreachable objects that were packed before to the loose object store, from where they can be pruned. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pitfalls in auto-fast-forwarding heads that are not checked out?
Am 04.05.2013 00:46, schrieb Martin Langhoff: I am building a small git wrapper around puppet, and one of the actions it performs is auto-fastforwarding of branches without checking them out. In simplified code... we ensure that we are on a head called master, and in some cases ppg commit, will commit to master and... ## early on # sanity-check we are on master headname=$(git rev-parse --symbolic-full-name --revs-only HEAD) if [ $headname -ne refs/heads/headname ]; then You mean refs/heads/master and != here because -ne is numeric comparison in a shell script. echo 2 ERROR: can only issue --immediate commit from the master branch! exit 1 fi ## then git commit -bla blarg baz ## and then... # ensure we can ff head_sha1=$(git rev-parse --revs-only master) mb=$(git merge-base $production_sha1 refs/heads/master) if [[ $mb -ne $production_sha1 ]]; then Your approach looks OK (but note again the incorrect -ne). Since git 1.8.0 you can express this check as if git merge-base --is-ancestor $production_sha1 refs/heads/master echo 2 ERROR: cannot fast-forward master to production echo 2 ERROR: cannot fast-forward production to master exit 1 fi $GIT_EXEC_PATH/git-update-ref -m ppg immediate commit refs/heads/production $head_sha1 $production_sha1 || exit 1 Are there major pitfalls in this approach? I don't think there are. I cannot think of any, but git has stayed away from updating my local tracking branches; so maybe there's a reason for that... I don't understand what you are saying here. What is that? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lookup_object: prioritize recently found objects
Am 5/1/2013 22:34, schrieb Jeff King: struct object *lookup_object(const unsigned char *sha1) { - unsigned int i; + unsigned int i, first; struct object *obj; if (!obj_hash) return NULL; - i = hashtable_index(sha1); + first = i = hashtable_index(sha1); while ((obj = obj_hash[i]) != NULL) { if (!hashcmp(sha1, obj-sha1)) break; @@ -85,6 +85,11 @@ struct object *lookup_object(const unsigned char *sha1) if (i == obj_hash_size) i = 0; } + if (obj i != first) { + struct object *tmp = obj_hash[i]; + obj_hash[i] = obj_hash[first]; + obj_hash[first] = tmp; + } return obj; } This is one of the places where I think the code does not speak for itself and a comment is warranted: The new if statement is not about correctness, but about optimization: /* * Move object to where we started to look for it * so that we do not need to walk the hash table * the next time we look for it. */ or something. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lookup_object: prioritize recently found objects
Am 5/2/2013 8:46, schrieb Jeff King: On Thu, May 02, 2013 at 08:44:07AM +0200, Johannes Sixt wrote: Am 5/1/2013 22:34, schrieb Jeff King: struct object *lookup_object(const unsigned char *sha1) { - unsigned int i; + unsigned int i, first; struct object *obj; if (!obj_hash) return NULL; - i = hashtable_index(sha1); + first = i = hashtable_index(sha1); while ((obj = obj_hash[i]) != NULL) { if (!hashcmp(sha1, obj-sha1)) break; @@ -85,6 +85,11 @@ struct object *lookup_object(const unsigned char *sha1) if (i == obj_hash_size) i = 0; } + if (obj i != first) { + struct object *tmp = obj_hash[i]; + obj_hash[i] = obj_hash[first]; + obj_hash[first] = tmp; + } return obj; } This is one of the places where I think the code does not speak for itself and a comment is warranted: The new if statement is not about correctness, but about optimization: I figured the lengthy description in the commit message would be sufficient, It's absolutely sufficient *if* one reads the commit message. In this case, though it goes more like this function should be trivial, and it is -- up to this if statement; what the heck is it good for? and the reader is forced to dig the history. BTW, do you notice that the function is now modifying an object (the hash table) even though this is rather unexpected from a lookup function? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] help: add help_unknown_ref
Am 01.05.2013 21:55, schrieb Vikrant Varma: On 01-05-2013 17:53, Ramkumar Ramachandra wrote: Vikrant Varma wrote: +void help_unknown_ref(const char* ref) { +int i; +struct similar_ref_cb ref_cb; +ref_cb.similar_refs = (struct string_list)STRING_LIST_INIT_NODUP; Why are you casting STRING_LIST_INIT_NODUP? +ref_cb.base_ref = ref; ref_cb.similar_refs has already been defined. The compiler won't let me assign to it unless I cast first. However, I think compound literals are a C99/gcc feature. Is this better? struct similar_ref_cb ref_cb = {ref, STRING_LIST_INIT_NODUP}; No. There are compilers that can initialize a struct only with constant data, but ref is not a constant. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* [PATCH] git-remote-testgit: avoid process substitution
Am 27.04.2013 01:26, schrieb Junio C Hamano: Felipe Contreras felipe.contre...@gmail.com writes: No, it wouldn't, but I don't think there's any way to do \\ or \b in globs. This should do in the meantime, but it further needs: - J6t's sign off for the follow-up part to remove remaining bash-isms to complete this patch (the last part of the patch is from 5178c583.6000...@viscovery.net and we can take half the log message from there); The patch below doesn't remove the bash dependency, yet, but it addresses the problematic mismatch you noticed without the need for $LF. Can you please queue it to move the topic forward? Removing the remaining bashisms and the following two can come later: - Rename it to git-remote-testgit.sh and tell Makefile to replace the shebang line with SHELL_PATH like other scripts; - Remove the we need to have bash because we will run remote-testgit logic from t5801 Here's my Signed-off-by: Johannes Sixt j...@kdbg.org for this part in case someone wants to pick it up: diff --git a/git-remote-testgit b/git-remote-testgit index b395c8d..ffac950 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +#!/bin/sh # Copyright (c) 2012 Felipe Contreras alias=$1 @@ -23,7 +23,6 @@ then testgitmarks=$dir/testgit.marks test -e $gitmarks || $gitmarks test -e $testgitmarks || $testgitmarks - testgitmarks_args=( --{import,export}-marks=$testgitmarks ) fi while read line @@ -62,22 +61,31 @@ do echo feature export-marks=$gitmarks fi echo feature done - git fast-export ${testgitmarks_args[@]} $refs | + git fast-export \ + ${testgitmarks:+--import-marks=$testgitmarks} \ + ${testgitmarks:+--export-marks=$testgitmarks} \ + $refs | sed -e s#refs/heads/#${prefix}/heads/#g echo done ;; export) before=$(git for-each-ref --format='%(refname) %(objectname)') - git fast-import ${testgitmarks_args[@]} --quiet + git fast-import \ + ${testgitmarks:+--import-marks=$testgitmarks} \ + ${testgitmarks:+--export-marks=$testgitmarks} \ + --quiet --- 8 --- Subject: [PATCH] git-remote-testgit: avoid process substitution The implementation of bash on Windows does not offer process substitution. Signed-off-by: Johannes Sixt j...@kdbg.org --- git-remote-testgit | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/git-remote-testgit b/git-remote-testgit index 23c9d40..979b13e 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -87,17 +87,18 @@ do exit 1 fi - before=$(git for-each-ref --format='%(refname) %(objectname)') + before=$(git for-each-ref --format=' %(refname) %(objectname) ') git fast-import ${testgitmarks_args[@]} --quiet - after=$(git for-each-ref --format='%(refname) %(objectname)') - # figure out which refs were updated - join -e 0 -o '0 1.2 2.2' -a 2 (echo $before) (echo $after) | - while read ref a b + git for-each-ref --format='%(refname) %(objectname)' | + while read ref a do - test $a == $b continue + case $before in + * $ref $a *) + continue ;; # unchanged + esac echo ok $ref done -- 1.8.2.388.g36592d7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Itches with the current rev spec
Am 4/26/2013 10:19, schrieb Yann Dirson: 2. git rebase -i master fails unless I've rebased my branch on top of master. I always wished I could do the equivalent of 'git rebase -i master..', but I can't. Can we give the A..B syntax a new meaning in the context of rebase, namely $(git merge-base A B)? If I understand well, you're refering to a problem that also annoys me, ie. using rebase -i to just edit your local commits, without rebasing onto the lastest revision on the upstream branch, right ? That is, just another wart of having a single command for arguably-different use cases, or of having the single-argument version of rebase use that argument for 2 very different things (cut-off point and destination), but I won't try to address either of these today :) In that case, what about just adding a new flag to rebase -i, that would prevent the single-argument to be interpreted as destination ? I really consider this a workaround for a suboptimal CLI, but since we don't want to change the rebase CLI before at least 2.0, that could fill the gap for now. As for the flag itself, what about --here ? Obviously it would only be meaninglful together with -i, and be exclusive with --onto. How about this: Allow alternative spelling of git rebase -i master topic like this: git rebase -i master..topic (as always, the default for topic is HEAD). Then by extension (cf. git diff, where A...B shows the diff between the mergebase and B) git rebase -i master...topic would rebase onto the mergebase, which in practice will be the fork point of topic, i.e., a non-rebasing rebase. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t5801: properly test the test shell
Am 4/25/2013 12:09, schrieb Michael J Gruber: fc407f9 (Add new simplified git-remote-testgit, 2012-11-28) introduced a test which was meant to skip the test unless the test shell is bash. Unfortunately, it tests for the availability of bash only. But users can opt to use a different shell (using SHELL_PATH) for the tests even though bash is available. After my patch this morning (avoid process substitution), there is not much bashism left in git-remote-testgit: diff --git a/git-remote-testgit b/git-remote-testgit index e99d5fa..178d030 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +#!/bin/sh # Copyright (c) 2012 Felipe Contreras alias=$1 @@ -23,7 +23,6 @@ then testgitmarks=$dir/testgit.marks test -e $gitmarks || $gitmarks test -e $testgitmarks || $testgitmarks - testgitmarks_args=( --{import,export}-marks=$testgitmarks ) fi while read line @@ -70,7 +69,10 @@ do fi echo feature done - git fast-export ${testgitmarks_args[@]} $refs | + git fast-export \ + ${testgitmarks:+--import-marks=$testgitmarks} \ + ${testgitmarks:+--export-marks=$testgitmarks} \ + $refs | sed -e s#refs/heads/#${prefix}/heads/#g echo done ;; @@ -89,7 +91,10 @@ do before=$(git for-each-ref --format='%(refname) %(objectname)') - git fast-import ${testgitmarks_args[@]} --quiet + git fast-import \ + ${testgitmarks:+--import-marks=$testgitmarks} \ + ${testgitmarks:+--export-marks=$testgitmarks} \ + --quiet # figure out which refs were updated git for-each-ref --format='%(refname) %(objectname)' | What's left is to take care of the shbang line, remove the bash check from t5801, make a proper commit from this patch. But I leave that to the interested reader. :-) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t5801: properly test the test shell
Am 4/25/2013 13:21, schrieb Michael J Gruber: Johannes Sixt venit, vidit, dixit 25.04.2013 12:59: Am 4/25/2013 12:09, schrieb Michael J Gruber: fc407f9 (Add new simplified git-remote-testgit, 2012-11-28) introduced a test which was meant to skip the test unless the test shell is bash. Unfortunately, it tests for the availability of bash only. But users can opt to use a different shell (using SHELL_PATH) for the tests even though bash is available. After my patch this morning (avoid process substitution), there is not much bashism left in git-remote-testgit: Is that a patch you submitted? It is not the patch I submitted this morning, but a patch on top that removes the remaining bashisms from git-remote-testgit. No, the problem (that I'm adressing) is not git-remote-testgit which uses bash unconditionally, independent of SHELL_PATH. The problem is bashism(s) in t5801 itself. That is completely orthogonal to your (non-) patch. OK. But wouldn't it be nicer to remove that bashism as well and have portable t5801 and git-remote-testgit? :-) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #08; Tue, 23)
Am 4/23/2013 21:31, schrieb Junio C Hamano: * fc/transport-helper-error-reporting (2013-04-17) 9 commits (merged to 'next' on 2013-04-22 at 5ba6467) + transport-helper: update remote helper namespace + transport-helper: trivial code shuffle + transport-helper: warn when refspec is not used + transport-helper: clarify pushing without refspecs + transport-helper: update refspec documentation + transport-helper: clarify *:* refspec + transport-helper: improve push messages + transport-helper: mention helper name when it dies + transport-helper: report errors properly Update transport helper to report errors and maintain ref hierarchy used to keep track of remote helper state better. Will merge to 'master'. Please don't, yet. There is a new test case that fails on Windows. I'll have to figure out a work-around. In git-remote-testgit we have this code: before=$(git for-each-ref --format='%(refname) %(objectname)') git fast-import ${testgitmarks_args[@]} --quiet after=$(git for-each-ref --format='%(refname) %(objectname)') # figure out which refs were updated join -e 0 -o '0 1.2 2.2' -a 2 (echo $before) (echo $after) | while read ref a b do test $a == $b continue echo ok $ref done The failure is in the 'join' line: Bash on Windows does not implement process substitution, and we do not have 'join'. This failing code exists since 93b5cf9c (remote-testgit: report success after an import, 2012-11-28), but apparently, it did not matter so far. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Apr 2013, #08; Tue, 23)
Am 4/24/2013 10:04, schrieb Felipe Contreras: On Wed, Apr 24, 2013 at 2:57 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 4/23/2013 21:31, schrieb Junio C Hamano: * fc/transport-helper-error-reporting (2013-04-17) 9 commits (merged to 'next' on 2013-04-22 at 5ba6467) + transport-helper: update remote helper namespace + transport-helper: trivial code shuffle + transport-helper: warn when refspec is not used + transport-helper: clarify pushing without refspecs + transport-helper: update refspec documentation + transport-helper: clarify *:* refspec + transport-helper: improve push messages + transport-helper: mention helper name when it dies + transport-helper: report errors properly Update transport helper to report errors and maintain ref hierarchy used to keep track of remote helper state better. Will merge to 'master'. Please don't, yet. There is a new test case that fails on Windows. I'll have to figure out a work-around. Which test case? If it it failed, it failed before this series. I don't see how this new series would affect anything. The test introduced in the commit at the tip: 'push update refs'. More precisely: 8 D:\Src\mingw-git\tsh t5801-remote-helpers.sh ok 1 - setup repository ok 2 - cloning from local repo ok 3 - create new commit on remote ok 4 - pulling from local repo ok 5 - pushing to local repo ok 6 - fetch new branch ok 7 - fetch multiple branches ok 8 - push when remote has extra refs ok 9 - push new branch by name not ok 10 - push new branch with old:new refspec # TODO known breakage ok 11 - cloning without refspec ok 12 - pulling without refspecs ok 13 - pushing without refspecs ok 14 - pulling without marks not ok 15 - pushing without marks # TODO known breakage ok 16 - push all with existing object ok 17 - push ref with existing object not ok 18 - push update refs # # (cd local # git checkout -b update master # echo update file # git commit -a -m update # git push origin update # git rev-parse --verify remotes/origin/update expect # git rev-parse --verify testgit/origin/heads/update actual # test_cmp expect actual # ) # ok 19 - proper failure checks for fetching ok 20 - proper failure checks for pushing ok 21 - push messages ok 22 - push signed tag ok 23 - push signed tag with signed-tags capability # still have 2 known breakage(s) # failed 1 among remaining 21 test(s) 1..23 8 The verbose failure is: 8 expecting success: (cd local git checkout -b update master echo update file git commit -a -m update git push origin update git rev-parse --verify remotes/origin/update expect git rev-parse --verify testgit/origin/heads/update actual test_cmp expect actual ) Switched to a new branch 'update' [update 86cfeec] update Author: A U Thor aut...@example.com 1 file changed, 1 insertion(+) d:/Src/mingw-git/git-remote-testgit: cannot make pipe for process substitution: Function not implemented d:/Src/mingw-git/git-remote-testgit: cannot make pipe for process substitution: Function not implemented d:/Src/mingw-git/git-remote-testgit: line 97: join: command not found Everything up-to-date fatal: Needed a single revision not ok 18 - push update refs 8 An example of a successful test is this: 8 expecting success: (cd local git checkout -b new-name echo content file git commit -a -m seven git push origin new-name ) compare_refs local HEAD server refs/heads/new-name Switched to a new branch 'new-name' [new-name 455466e] seven Author: A U Thor aut...@example.com 1 file changed, 1 insertion(+) d:/Src/mingw-git/git-remote-testgit: cannot make pipe for process substitution: Function not implemented d:/Src/mingw-git/git-remote-testgit: cannot make pipe for process substitution: Function not implemented d:/Src/mingw-git/git-remote-testgit: line 97: join: command not found Everything up-to-date ok 9 - push new branch by name 8 -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: fix quoting in relative_path()
Am 24.04.2013 18:28, schrieb John Keeping: On Wed, Apr 24, 2013 at 09:21:38AM -0700, Junio C Hamano wrote: J6t meant a patch to remove the entire case...esac and replace it with a single liner (target=${target#$curdir/}). Ah, I missed the six-liner part. But that doesn't work because we break out of the do...while loop when $target starts with $curdir/ and replacing the case statement will remove that. Yeah, right, _that_ is what I missed ;-) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote-testgit: avoid process substitution
From: Johannes Sixt j...@kdbg.org Bash on Windows does not implement process substitution. Signed-off-by: Johannes Sixt j...@kdbg.org --- Am 4/24/2013 10:30, schrieb Johannes Sixt: Am 4/24/2013 10:04, schrieb Felipe Contreras: On Wed, Apr 24, 2013 at 2:57 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 4/23/2013 21:31, schrieb Junio C Hamano: * fc/transport-helper-error-reporting (2013-04-17) 9 commits (merged to 'next' on 2013-04-22 at 5ba6467) + transport-helper: update remote helper namespace + transport-helper: trivial code shuffle + transport-helper: warn when refspec is not used + transport-helper: clarify pushing without refspecs + transport-helper: update refspec documentation + transport-helper: clarify *:* refspec + transport-helper: improve push messages + transport-helper: mention helper name when it dies + transport-helper: report errors properly Update transport helper to report errors and maintain ref hierarchy used to keep track of remote helper state better. Will merge to 'master'. Please don't, yet. There is a new test case that fails on Windows. I'll have to figure out a work-around. Which test case? If it it failed, it failed before this series. I don't see how this new series would affect anything. The test introduced in the commit at the tip: 'push update refs'. Here is a fix. It assumes that the list of refs after the import is a superset of the refs before the import. (Can refs be deleted via fast-import?) git-remote-testgit | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/git-remote-testgit b/git-remote-testgit index 23c9d40..e99d5fa 100755 --- a/git-remote-testgit +++ b/git-remote-testgit @@ -91,13 +91,15 @@ do git fast-import ${testgitmarks_args[@]} --quiet - after=$(git for-each-ref --format='%(refname) %(objectname)') - # figure out which refs were updated - join -e 0 -o '0 1.2 2.2' -a 2 (echo $before) (echo $after) | - while read ref a b + git for-each-ref --format='%(refname) %(objectname)' | + while read ref a do - test $a == $b continue + case $before in + *$ref $a*) + continue + ;; + esac echo ok $ref done -- 1.8.2.388.g36592d7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Silent data loss on merge with uncommited changes + renames
Am 4/22/2013 11:24, schrieb Matthieu Moy: Following the discussion on merge with uncommited changes inside the git pull --autostash thread, I did a bit of testing, and encountered a case with silent data loss. In short: merge a branch introducing changes to a file. If the file has been renamed in the current branch, then git merge follows the rename and brings changes to the renamed file, but uncommited changes in this file are overriden silently. Can you check whether your case is already covered by one of: git grep expect_failure t/*merge* and if not, contribute a test case? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
Am 19.04.2013 18:45, schrieb Junio C Hamano: John Keeping j...@keeping.me.uk writes: On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: +relative_path () +{ + local target curdir result + target=$1 + curdir=${2-$wt_prefix} + curdir=${curdir%/} + result= + + while test -n $curdir + do + case $target in + $curdir/*) + target=${target#$curdir/} + break + ;; + esac Could $curdir have glob wildcard to throw this part of the logic off? It is OK to have limitations like you cannot have a glob characters in your path to submodule working tree (at least until we start rewriting these in C or Perl or Python), but we need to be aware of them. I think the use of # instead of ## saves us here because even with a wildcard in $curdir the case statement matches literally, If you have curdir=a*b and target=adropb/c/d/e, the chopping itself target=${target#$curdir/} would happily chop adropb/ from the target, but because the dq around $curdir/* in the case arm label enforces that target must literally match curdir followed by a slash, we do not even come to the chomping part. I still have not convinced myself that it is impossible for somebody more clever than I to craft a pair of target and curdir that breaks it, though. (target=a*b/c/d, curdir=a*b) is correctly chopped, so that is not it. Why not just replace the six-liner by this one-liner: target=${target#$curdir/} -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t6200: avoid path mangling issue on Windows
Am 19.04.2013 18:33, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: Am 4/18/2013 19:05, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: From: Johannes Sixt j...@kdbg.org MSYS bash interprets the slash in the argument core.commentchar=/ as root directory and mangles it into a Windows style path. Use a different core.commentchar to dodge the issue. Signed-off-by: Johannes Sixt j...@kdbg.org ... - git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD actual + git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD actual Sigh... Again? Are folks working on Msys bash aware that sometimes the users may want to say key=value on their command line without the value getting molested in any way and giving them some escape hatch would help them? Perhaps they have already decided that it is not feasible after thinking about the issue, in which case I do not have new ideas to offer. What is the issue? And in which way would an escape hatch help us here? When the user passes key=value and value begins with a slash, value may be a path in the filesystem very often, and adjusting it to the local filesystem convention helps Windows users a lot. But there are cases outside that very often when the user wants the value passed literally. There seems to be no way to do so. ... if bash could be told with a very unnatural and not so hard to type way that the particular value is not to be mangled, e.g. xyzzy key=/a/b/c I'll not argue whether such a feature would make sense or not, or whether it can be implemented, because it is aimed at the user, but misses one important point: It does in no way help our development process. A patch auther whose first instinct is to write 'foo=/' will never write 'foo=x', let alone 'foo=/'. Someone will have to discover the issue eventually and write a patch to fix it, and someone will have to apply it. I don't think that we can do anything about it. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib
Am 19.04.2013 21:24, schrieb Junio C Hamano: Felipe Contreras felipe.contre...@gmail.com writes: On Fri, Apr 19, 2013 at 12:08 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: The code finds the changes of a commit, runs 'git blame' for each chunk to see which other commits are relevant, and then reports the author and signers. But I think it can be useful outside the context of send-email as well, and having one independent tool that does one single job well is a better design. Perhaps it is better to name it less specific to send-email's cc-cmd option. git people? git whom? git reviewers? I dunno, but along those lines. Would it make sense to integrate this in git shortlog, which already does something similar? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: put THEIR commits AFTER my commits with a single rebase command
Am 4/18/2013 7:18, schrieb Ilya Basin: desired result: A---B---C origin/master / D---E---F---G---A'---B'---C' *master Variant 1: git branch -f tmp git reset --hard origin/master git rebase tmp Variant 1a: git reset --hard origin/master git rebase @{1} This variant is bad, because 'git reset --hard' checks out some files and 'git rebase' rewrites them again before applying commits. It's a redundant job. Variant 2: git branch -f tmp origin/master git rebase --onto master master tmp git branch -f master git checkout master Too many commands. I want to do this with just one command. And I want to stay be on branch master in case of rebase conflicts. Perhaps this one: git merge origin/master git rebase ORIG_HEAD -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
t6200: avoid path mangling issue on Windows
From: Johannes Sixt j...@kdbg.org MSYS bash interprets the slash in the argument core.commentchar=/ as root directory and mangles it into a Windows style path. Use a different core.commentchar to dodge the issue. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t6200-fmt-merge-msg.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index e7e945d..54b5744 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -179,8 +179,8 @@ test_expect_success '--log=5 with custom comment character' ' cat expected -EOF Merge branch ${apos}left${apos} - / By Another Author (3) and A U Thor (2) - / Via Another Committer + x By Another Author (3) and A U Thor (2) + x Via Another Committer * left: Left #5 Left #4 @@ -189,7 +189,7 @@ test_expect_success '--log=5 with custom comment character' ' Common #1 EOF - git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD actual + git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD actual test_cmp expected actual ' -- 1.8.2.1.1678.gf713add -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: put THEIR commits AFTER my commits with a single rebase command
Am 4/18/2013 10:33, schrieb Ilya Basin: JS Perhaps this one: JSgit merge origin/master JSgit rebase ORIG_HEAD JS -- Hannes Wouldn't I have to resolve conflicts twice? Yes. But you did run 'git config rerere.enabled true' when you started with git, didn't you? ;-) Anyway, Johan's idea to use git cherry-pick is much better. BTW, during the rebase, can I tell git to rewrite a different branch upon rebase success or abort? git branch -f tmp origin/master git rebase --onto master master tmp if [ $? -ne 0 ]; then # modify some file in .git/ ? What do you expect here? Failure of git rebase means that it is not complete, yet. So far, nothing has been rewritten. So what? Perhaps you mean: # never mind git rebase --abort else git branch -f master git checkout master fi -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t6200: avoid path mangling issue on Windows
Am 4/18/2013 19:05, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: From: Johannes Sixt j...@kdbg.org MSYS bash interprets the slash in the argument core.commentchar=/ as root directory and mangles it into a Windows style path. Use a different core.commentchar to dodge the issue. Signed-off-by: Johannes Sixt j...@kdbg.org ... -git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD actual +git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD actual Sigh... Again? Are folks working on Msys bash aware that sometimes the users may want to say key=value on their command line without the value getting molested in any way and giving them some escape hatch would help them? Perhaps they have already decided that it is not feasible after thinking about the issue, in which case I do not have new ideas to offer. What is the issue? And in which way would an escape hatch help us here? We would have to apply a patch anyway after a glitch like this shows up, because disabling path mangling whole-sale (if there were a method -- there is none currently) is a no-go in the context of our test suite, let a lone in our scripted tool set. When foo=/ appears on the command line, the most obvious interpretation of the slash for a program without mind-reading mode is that it is an absolute path, and then path mangling must happen (if and only if the invoked program is a non-MSYS program such as git). I'll apply the patch as-is, but this feels really painful to the users. No, generally, path mangling is a service for the user. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usage: refactor die-recursion checks
Am 4/16/2013 4:50, schrieb Jeff King: On Mon, Apr 15, 2013 at 07:34:07PM -0700, Brandon Casey wrote: Right. My assumption was that we are primarily interested in protecting against the die_routine. Compat functions should never be calling die. I think the rule we have been enforcing is less strict than that. We have only said that any compat function in a die handler path should never call die. But maybe that's what you meant. No, I assumed we were following the stronger rule. If you are a compat function for a C library function, then you should never need to die. You should be conforming to the existing interface, which will have some mechanism for passing back an error. This rule has been violated LNG ago, and not only in compat/mingw.c (see xmalloc in compat/qsort.c, for example). The primary motivation was that Hannes Sixt had to step in and point out yet again that the high-level memory allocators should not be called in anything that could be in a die handler code path. I was on the thread, but I don't remember the topic (ah, Jonathan has stepped in with the answer). I do remember that I was not the only one who had forgotten about that rule though. Yeah, it is subtle enough that it may be worth protecting against. Too late. To implement this check correctly/completely (i.e. detect recursion in the main thread as well as in any child threads), I think you really do need to use thread-local storage as you mentioned in 3/3 which could look something like: static pthread_key_t dying; static pthread_once_t dying_once = PTHREAD_ONCE_INIT; void setup_die_counter(void) { pthread_key_create(dying, NULL); } check_die_recursion(void) { pthread_once(dying_once, setup_die_counter); if (pthread(getspecific(dying)) { puts(BUG: recursion...); exit(128); } pthread_setspecific(dying, dying); } Yeah, that seems sane; my biggest worry was that it would create headaches for Windows folks, who would have to emulate pthread_key. But it seems like we already added support in 9ba604a. pthread_key is not a problem, but pthread_once is. It's certainly solvable, but do we really have to? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usage: refactor die-recursion checks
Am 4/16/2013 15:01, schrieb Jeff King: On Tue, Apr 16, 2013 at 09:18:46AM +0200, Johannes Sixt wrote: Yeah, that seems sane; my biggest worry was that it would create headaches for Windows folks, who would have to emulate pthread_key. But it seems like we already added support in 9ba604a. pthread_key is not a problem, but pthread_once is. It's certainly solvable, but do we really have to? I'm not clear on what you are suggesting. That we protect only the main thread from recursion, or that we drop the check entirely? Or that we implement thread-local storage for this case without using pthread_once? Anything(*) that does not require pthread_once. A pthread_once implementation on Windows would be tricky and voluminous and and on top of it very likely to be done differently for gcc and MSVC. I don't like to go there if we can avoid it. (*) That includes doing nothing, but does not include ripping out the recursion check, as it protects us from crashes. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH master] convert: The native line-ending is \r\n on MinGW
Am 16.04.2013 16:39, schrieb Erik Faye-Lund: On Mon, Apr 15, 2013 at 11:43 PM, Junio C Hamano gits...@pobox.com wrote: Erik Faye-Lund kusmab...@gmail.com writes: This is absolutely the right thing to do. However, stuff have changed a bit since the patch was written; this change now needs to go in config.mak.uname instead of config.mak. Thanks for a quick response. What's your preference? I could just ignore a patch I won't be able to test myself and have you guys carry it in your tree forever, but I do not think that is necessary for something small like this. I should probably clarify; conceptually, this is the right thing to do. Git for Windows is a Windows application, and should have CRLF as the native newline. I hadn't tested this patch myself, though. Our tree is currently way behind yours, and I tried to do a rebase, but it turned out much trickier than I was hoping for. I've given it a go on top of your tree + some essential patches I'll need to get things to run, and it seems to do what it claims to do. However, I haven't been able to run the test-suite, because I need a bunch more patches from the msysGit-tree for that. I have been using this patch or an equivalent one since at least one and a half years (until a month or two ago, as I discovered today, but that is only by accident). But I do not use any text attributes or eol configuration, so I can only say that it does not regress this use case. I think this is low impact enough that it can directly go to 'master' or even 'maint' if I were to apply to my tree. I agree. I don't think we need it in maint; we don't track that branch for msysGit. Yes, master is good enough. Thanks. Thanks. -- 8 -- From: Jonathan Nieder jrnie...@gmail.com Date: Sat, 4 Sep 2010 03:25:09 -0500 Subject: [PATCH] convert: The native line-ending is \r\n on MinGW If you try this: 1. Install Git for Windows (from the msysgit project) 2. Put [core] autocrlf = false eol = native in your .gitconfig. 3. Clone a project with *.txt text in its .gitattributes. Then with current git, any text files checked out have LF line endings, instead of the expected CRLF. Cc: Johannes Schindelin johannes.schinde...@gmx.de Cc: Johannes Sixt j...@kdbg.org Signed-off-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- config.mak.uname | 1 + 1 file changed, 1 insertion(+) diff --git a/config.mak.uname b/config.mak.uname index 9080054..d78fd3d 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -507,6 +507,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) compat/win32/dirent.o EXTLIBS += -lws2_32 PTHREAD_LIBS = + NATIVE_CRLF = YesPlease X = .exe SPARSE_FLAGS = -Wno-one-bit-signed-bitfield ifneq (,$(wildcard ../THIS_IS_MSYSGIT)) -- 1.8.2.1-542-g3613165 Looks fine to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Splitting a commit with rebase -i and keeping a commit message
Am 4/17/2013 3:38, schrieb Tim Chase: I asked this on IRC and played with some of their ideas, but struck out with anything satisfying. I walked through [1] with the following setup: git init foo cd foo touch a.txt b.txt git add a.txt b.txt git commit -m Initial checkin echo Modify A a.txt git commit -am Modified A echo Modify B b.txt git commit -am Modified B echo Modify A2 a.txt echo Modify B2 b.txt git commit -am Modified B git commit -am Long-bodied commit comment about b.txt changes # whoops, just wanted B git rebase -i HEAD^^ # change the Added b.txt... commit to edit # and duplicate the instruction line git checkout HEAD^ b.txt # undo b.txt git commit --amend -m Tweaked a.txt git rebase --continue # in real world cases, you are likely to see conflicts here # when the commit is applied a second time, # but not in this toy example git rebase --continue -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] push: introduce implicit push
Am 4/15/2013 5:04, schrieb Junio C Hamano: Ramkumar Ramachandra artag...@gmail.com writes: ... In my proposal, the precedence order branch.name.pushremote, remote.pushdefault, branch.name.remote, remote.default, origin, remains the same: we just want to change which branch that name refers to. That changing the meaning of name in the middle, and doing so will be confusing to the users, is exactly the issue, isn't it? In my opinion, it is a much more subtle change than the entirely new precedence order that you're inventing. Adding -- has never been my itch. I just brought it up out of thin air as a possible alternative that is less confusing. User says: git push -- master docs release Then git pushes the three branches to three different upstreams. You find that confusing. Do I understanding correctly so far? If I were a push.default=(simple|upstream) type, then I would be totally aware that there are three different upstreams involved because I had had to configure them manually and explicitly (correct?), and I would be completely surprised if the push would *not* go to three different upstreams. Just my 2 cents. (But I'm a traditional matching type, so take this with a grain of salt. Or I may be missing the point of this thread, as I haven't followed closely.) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: commit-message attack for extracting sensitive data from rewritten Git history
Am 4/8/2013 23:54, schrieb Jeff King: Yeah, it would make sense for filter-branch to have a --map-commit-ids option or similar that does the update. At first I thought it might take two passes, but I don't think it is necessary, as long as we traverse the commits topologically (i.e., you cannot have mentioned X in a commit that is an ancestor of X, so you do not have to worry about mapping it until after it has been processed). Topological traversal is not sufficient. Consider this history: o--A--o-- / / --o--B--o If A mentions B (think of cherry-pick -x), then you must ensure that the branch containing B was traversed first. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rerere forget: grok files containing NUL
Am 02.04.2013 00:48, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Using 'git rerere forget .' after a merge that involved binary files runs into an infinite loop if the binary file contains a zero byte. Replace a strchrnul by memchr because the former does not make progress as soon as the NUL is encountered. Hmph, thanks. Is it the right behaviour for rerere to even attempt to interfere with a merge that involves binary files in the first place? Why not? And how could rerere tell the difference? It would have to do the same check for binary-ness as the merge driver before it even starts looking closer at the files. Does the three-way merge machinery replay recorded resolution for such a binary file correctly (after your fix, that is)? Yes, it does. It recognizes the binary-ness and picks 'our' side. Only then comes rerere_mem_getline into play. diff --git a/rerere.c b/rerere.c index a6a5cd5..4d940cd 100644 --- a/rerere.c +++ b/rerere.c @@ -284,8 +284,10 @@ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_) strbuf_release(sb); if (!io-input.len) return -1; -ep = strchrnul(io-input.buf, '\n'); -if (*ep == '\n') +ep = memchr(io-input.buf, '\n', io-input.len); +if (!ep) +ep = io-input.buf + io-input.len; +else if (*ep == '\n') ep++; len = ep - io-input.buf; strbuf_add(sb, io-input.buf, len); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] run-command: add new check_command helper
Am 02.04.2013 12:31, schrieb Felipe Contreras: And persistent_waitpid() to recover the information from the last run. I'm not a fan of this new API, because it looks like a workaround for a problem that should have been solved in a cleaner way. But if we can't avoid it, please also add a paragraph to Documentation/technical/api-run-command.txt +int check_command(struct child_process *cmd) +{ + int status; + pid_t waiting; + + if (cmd-last_status.valid) + return 0; + + while ((waiting = waitpid(cmd-pid, status, WNOHANG)) 0 errno == EINTR) + ; /* nothing */ + + if (!waiting) + return 1; + + if (waiting == cmd-pid) { + cmd-last_status.valid = 1; + cmd-last_status.status = status; + return 0; + } + + if (waiting 0) + die(BUG: waitpid reported a random pid?); + + return 0; +} + static void prepare_run_command_v_opt(struct child_process *cmd, const char **argv, int opt) @@ -729,7 +770,7 @@ error: int finish_async(struct async *async) { #ifdef NO_PTHREADS - return wait_or_whine(async-pid, child process); + return wait_or_whine(cmd, async-pid, child process); This breaks the NO_PTHREADS build because cmd is undeclared. Perhaps this on top: diff --git a/run-command.c b/run-command.c index a9fa779..a02ef62 100644 --- a/run-command.c +++ b/run-command.c @@ -230,7 +230,7 @@ static pid_t persistent_waitpid(struct child_process *cmd, pid_t pid, int *statu { pid_t waiting; - if (cmd-last_status.valid) { + if (cmd cmd-last_status.valid) { *status = cmd-last_status.status; return pid; } @@ -771,7 +771,7 @@ int start_async(struct async *async) int finish_async(struct async *async) { #ifdef NO_PTHREADS - return wait_or_whine(cmd, async-pid, child process); + return wait_or_whine(NULL, async-pid, child process); #else void *ret = (void *)(intptr_t)(-1); -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rerere forget: grok files containing NUL
Am 02.04.2013 21:18, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Does the three-way merge machinery replay recorded resolution for such a binary file correctly (after your fix, that is)? Yes, it does. It recognizes the binary-ness and picks 'our' side. Only then comes rerere_mem_getline into play. Surely getline() needs to be fixed not to loop forever regardless of the binary-ness, but I was more worried about our additions of lines that satisfy is_cmarker(), counting of them in the callchain from handle_file() to handle_path() to decide if a path has already been resolved by the user, and recording of an resolution based on the return value of that callchain, all of which relies on the merged contents being textual and marked with the conflict marker. Of course, it would make sense to exclude binary files from rerere's operation. But that's an independent issue, and it is not new. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] rerere forget: grok files containing NUL
Using 'git rerere forget .' after a merge that involved binary files runs into an infinite loop if the binary file contains a zero byte. Replace a strchrnul by memchr because the former does not make progress as soon as the NUL is encountered. Signed-off-by: Johannes Sixt j...@kdbg.org --- The new test case runs into the infinite loop if you back out the code change. There's another bug where an uninitialized pointer is accessed in the second for-loop in handle_cache(), presumably for a file with ADD-ADD conflicts. Will look into that one later this week. -- Hannes rerere.c | 6 -- t/t2030-unresolve-info.sh | 12 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/rerere.c b/rerere.c index a6a5cd5..4d940cd 100644 --- a/rerere.c +++ b/rerere.c @@ -284,8 +284,10 @@ static int rerere_mem_getline(struct strbuf *sb, struct rerere_io *io_) strbuf_release(sb); if (!io-input.len) return -1; - ep = strchrnul(io-input.buf, '\n'); - if (*ep == '\n') + ep = memchr(io-input.buf, '\n', io-input.len); + if (!ep) + ep = io-input.buf + io-input.len; + else if (*ep == '\n') ep++; len = ep - io-input.buf; strbuf_add(sb, io-input.buf, len); diff --git a/t/t2030-unresolve-info.sh b/t/t2030-unresolve-info.sh index f262065..0b699f5 100755 --- a/t/t2030-unresolve-info.sh +++ b/t/t2030-unresolve-info.sh @@ -44,9 +44,13 @@ prime_resolve_undo () { test_expect_success setup ' mkdir fi + printf a\0a binary + git add binary test_commit initial fi/le first git branch side git branch another + printf a\0b binary + git add binary test_commit second fi/le second git checkout side test_commit third fi/le third @@ -167,4 +171,12 @@ test_expect_success 'rerere and rerere forget (subdirectory)' ' test_cmp expect actual ' +test_expect_success 'rerere forget (binary)' ' + git checkout -f side + printf a\0c binary + git commit -a -m binary + test_must_fail git merge second + git rerere forget binary +' + test_done -- 1.8.2.383.g5f2fd52 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool: don't overwrite modified files
Am 3/25/2013 22:44, schrieb John Keeping: After running the user's diff tool, git-difftool will copy any files that differ between the working tree and the temporary tree. This is useful when the user edits the file in their diff tool but is wrong if they edit the working tree file while examining the diff. Instead of copying unconditionally when the files differ, create and index from the working tree files and only copy the temporary file back if it was modified and the working tree file was not. If both files have been modified, print a warning and exit with an error. Note that we cannot use an existing index in git-difftool since those contain the modified files that need to be checked out but here we are looking at those files which are copied from the working tree and not checked out. These are precisely the files which are not in the existing indices. Signed-off-by: John Keeping j...@keeping.me.uk --- On Mon, Mar 25, 2013 at 10:42:19AM +, John Keeping wrote: On Mon, Mar 25, 2013 at 08:41:59AM +0100, Johannes Sixt wrote: This is gross. Can't we do much better here? Difftool already keeps a GIT_INDEX of the files in the temporary tree ($tmpdir/rindex). Running git-diff-files should be sufficient to tell which ones where edited via the users's diff-tool. Then you can restrict calling hash-object to only those worktree files where an edit collision needs to be checked for. That's only the case for files that are not copied from the working tree, so the temporary index doesn't contain the files that are of interest here. You could also keep a parallel index that keeps the state of the same set of files in the worktree. Then another git-diff-files call could replace the other half of hash-object calls. I like the idea of creating an index from the working tree files and using it here. If we create a starting state index for these files, we should be able to run git-diff-files against both the working tree and the temporary tree at this point and compare the output. Here's an attempt at taking this approach, built on jk/difftool-dir-diff-edit-fix. git-difftool.perl | 73 +++-- t/t7800-difftool.sh | 26 +++ 2 files changed, 85 insertions(+), 14 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index c433e86..d10f7d2 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -13,9 +13,9 @@ use 5.008; use strict; use warnings; +use Error qw(:try); use File::Basename qw(dirname); use File::Copy; -use File::Compare; use File::Find; use File::stat; use File::Path qw(mkpath rmtree); @@ -88,14 +88,45 @@ sub use_wt_file my ($repo, $workdir, $file, $sha1, $symlinks) = @_; my $null_sha1 = '0' x 40; - if ($sha1 eq $null_sha1) { - return 1; - } elsif (not $symlinks) { + if ($sha1 ne $null_sha1 and not $symlinks) { return 0; } my $wt_sha1 = $repo-command_oneline('hash-object', $workdir/$file); - return $sha1 eq $wt_sha1; + my $use = ($sha1 eq $null_sha1) || ($sha1 eq $wt_sha1); + return ($use, $wt_sha1); +} + +sub changed_files +{ + my ($repo_path, $index, $worktree) = @_; + $ENV{GIT_INDEX_FILE} = $index; + $ENV{GIT_WORK_TREE} = $worktree; + my $must_unset_git_dir = 0; + if (not defined($ENV{GIT_DIR})) { + $must_unset_git_dir = 1; + $ENV{GIT_DIR} = $repo_path; + } + + my @refreshargs = qw/update-index --really-refresh -q --unmerged/; + my @gitargs = qw/diff-files --name-only -z/; + try { + Git::command_oneline(@refreshargs); + } catch Git::Error::Command with {}; + + my $line = Git::command_oneline(@gitargs); + my @files; + if (defined $line) { + @files = split('\0', $line); + } else { + @files = (); + } + + delete($ENV{GIT_INDEX_FILE}); + delete($ENV{GIT_WORK_TREE}); + delete($ENV{GIT_DIR}) if ($must_unset_git_dir); + + return map { $_ = 1 } @files; } sub setup_dir_diff @@ -121,6 +152,7 @@ sub setup_dir_diff my $null_sha1 = '0' x 40; my $lindex = ''; my $rindex = ''; + my $wtindex = ''; my %submodule; my %symlink; my @working_tree = (); @@ -174,8 +206,12 @@ EOF } if ($rmode ne $null_mode) { - if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) { - push(@working_tree, $dst_path); + my ($use, $wt_sha1) = use_wt_file($repo, $workdir, + $dst_path, $rsha1, + $symlinks); + if ($use) { + push @working_tree, $dst_path; + $wtindex .= $rmode $wt_sha1\t
Re: [PATCH v2] difftool: don't overwrite modified files
Forgot to mention: The patch passes t7800 on Windows. Thanks, -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] difftool: don't overwrite modified files
Am 3/26/2013 10:31, schrieb John Keeping: On Tue, Mar 26, 2013 at 09:38:42AM +0100, Johannes Sixt wrote: One question though: Do I understand correctly that the temporary directories are leaked in the case of an edit conflict? If so, is it worth a warning for the user to clean up the garbage? Do you mean for normal users or for those running the tests? In normal usage we do print a warning - it's in the existing code, triggered by setting $error = 1 - you can see that if you run the tests with -v. I meant for normal users. I see the error now. Thanks. The last test does result in /tmp filling up with temporary directories though, it would be good if the test could clean up after itself. The best I can come up with is adding something like this immediately after running difftool but I'm not entirely happy with the .. in the argument to rm: test_when_finished rm -rf $(cat tmpdir)/.. Wrap the test in ( TMPDIR=$TRASH_DIRECTORY export TMPDIR ... ) It works for me. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] t7800: run --dir-diff tests with and without symlinks
The series looks good, but I can't test it because it does not apply anywhere here. Am 3/23/2013 14:31, schrieb John Keeping: Currently the difftool --dir-diff tests may or may not use symlinks depending on the operating system on which they are run. In one case this has caused a test failure to be noticed only on Windows when the test also fails on Linux when difftool is invoked with --no-symlinks. Rewrite these tests so that they do not depend on the environment but run explicitly with both --symlinks and --no-symlinks, protecting the --symlinks version with a SYMLINKS prerequisite. At first, I wondered what the point of having --symlinks and --no-symlinks was when there is no discernable difference. But 1f229345 (difftool: Use symlinks when diffing against the worktree) makes it pretty clear: It's an optimization, and --no-symlinks is only intended as an escape hatch. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] t7800: fix tests when difftool uses --no-symlinks
Am 3/24/2013 16:15, schrieb John Keeping: Subject: [PATCH] difftool: don't overwrite modified files After running the user's diff tool, git-difftool will copy any files that differ between the working tree and the temporary tree. This is useful when the user edits the file in their diff tool but is wrong if they edit the working tree file while examining the diff. Instead of copying unconditionally when the files differ, store the initial hash of the working tree file and only copy the temporary file back if it was modified and the working tree file was not. If both files have been modified, print a warning and exit with an error. Signed-off-by: John Keeping j...@keeping.me.uk --- git-difftool.perl | 35 +-- t/t7800-difftool.sh | 26 ++ 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index c433e86..be82b5a 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -15,7 +15,6 @@ use strict; use warnings; use File::Basename qw(dirname); use File::Copy; -use File::Compare; use File::Find; use File::stat; use File::Path qw(mkpath rmtree); @@ -123,7 +122,7 @@ sub setup_dir_diff my $rindex = ''; my %submodule; my %symlink; - my @working_tree = (); + my %working_tree; my @rawdiff = split('\0', $diffrtn); my $i = 0; @@ -175,7 +174,9 @@ EOF if ($rmode ne $null_mode) { if (use_wt_file($repo, $workdir, $dst_path, $rsha1, $symlinks)) { - push(@working_tree, $dst_path); + $working_tree{$dst_path} = + $repo-command_oneline('hash-object', + $workdir/$dst_path); } else { $rindex .= $rmode $rsha1\t$dst_path\0; } @@ -227,7 +228,7 @@ EOF # not part of the index. Remove any trailing slash from $workdir # before starting to avoid double slashes in symlink targets. $workdir =~ s|/$||; - for my $file (@working_tree) { + for my $file (keys %working_tree) { my $dir = dirname($file); unless (-d $rdir/$dir) { mkpath($rdir/$dir) or @@ -278,7 +279,7 @@ EOF exit_cleanup($tmpdir, 1) if not $ok; } - return ($ldir, $rdir, $tmpdir, @working_tree); + return ($ldir, $rdir, $tmpdir, %working_tree); } sub write_to_file @@ -376,7 +377,7 @@ sub dir_diff my $error = 0; my $repo = Git-repository(); my $workdir = find_worktree($repo); - my ($a, $b, $tmpdir, @worktree) = + my ($a, $b, $tmpdir, %worktree) = setup_dir_diff($repo, $workdir, $symlinks); if (defined($extcmd)) { @@ -390,19 +391,25 @@ sub dir_diff # should be copied back to the working tree. # Do not copy back files when symlinks are used and the # external tool did not replace the original link with a file. - for my $file (@worktree) { + for my $file (keys %worktree) { next if $symlinks -l $b/$file; next if ! -f $b/$file; - my $diff = compare($b/$file, $workdir/$file); - if ($diff == 0) { - next; - } elsif ($diff == -1) { - my $errmsg = warning: Could not compare ; - $errmsg += '$b/$file' with '$workdir/$file'\n; + my $wt_hash = $repo-command_oneline('hash-object', + $workdir/$file); + my $tmp_hash = $repo-command_oneline('hash-object', + $b/$file); This is gross. Can't we do much better here? Difftool already keeps a GIT_INDEX of the files in the temporary tree ($tmpdir/rindex). Running git-diff-files should be sufficient to tell which ones where edited via the users's diff-tool. Then you can restrict calling hash-object to only those worktree files where an edit collision needs to be checked for. You could also keep a parallel index that keeps the state of the same set of files in the worktree. Then another git-diff-files call could replace the other half of hash-object calls. + my $wt_modified = $wt_hash ne $worktree{$file}; + my $tmp_modified = $tmp_hash ne $worktree{$file}; + + if ($wt_modified and $tmp_modified) { + my $errmsg = warning: Both files modified: ; + $errmsg .= '$workdir/$file' and '$b/$file'.\n; + $errmsg .= warning: Working tree file has been left.\n; + $errmsg .= warning:\n; warn $errmsg; $error = 1; - } elsif ($diff == 1) { + } elsif ($tmp_modified) { my $mode =
Re: [PATCH v2 3/3] t7800: run --dir-diff tests with and without symlinks
Am 3/25/2013 11:35, schrieb John Keeping: On Mon, Mar 25, 2013 at 08:26:52AM +0100, Johannes Sixt wrote: The series looks good, but I can't test it because it does not apply anywhere here. It's built on top of da/difftool-fixes, is there some problem that stops it applying cleanly on top of that? Thanks. I had only tried trees that were contaminated by jk/difftool-dir-diff-edit-fix, which is in conflict with da/difftool-fixes. t7800 passes on Windows with these patches. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] t7800: modernize tests
Am 3/21/2013 8:41, schrieb Johannes Sixt: Am 3/20/2013 23:59, schrieb David Aguilar: I started digging in and the @worktree_files (aka @worktree above) is populated from the output of git diff --raw Seeing the output filename in diff --raw implies that one of the tests added output to the index somehow. I do not see that happening anywhere, though, so I do not know how it would end up in the @worktree array if it is not reported by diff --raw. My current understanding of how it could possibly be open twice: 1. via the output redirect 2. via the copy() perl code which is fed by @worktree So I'm confused. Why would we get different results on Windows? I tracked down the difference between Windows and Linux, and it is... for my $file (@worktree) { next if $symlinks -l $b/$file; ... this line in sub dir_diff. On Linux, we take the short-cut, but on Windows we proceed through the rest of the loop, And that is likely by design. From the docs: --symlinks --no-symlinks git difftool's default behavior is create symlinks to the working tree when run in --dir-diff mode. Specifying `--no-symlinks` instructs 'git difftool' to create copies instead. `--no-symlinks` is the default on Windows. And indeed, we have this initialization: my %opts = ( ... symlinks = $^O ne 'cygwin' $^O ne 'MSWin32' $^O ne 'msys', ... ); Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor --no-symlinks is passed? Perhaps the right solution is this: diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index c6d6b1c..19238f6 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -328,14 +328,16 @@ test_expect_success PERL 'setup change in subdirectory' ' git commit -m modified both ' -test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output +# passing --symlinks helps Cygwin, which defaults to --no-symlinks + +test_expect_success PERL,SYMLINKS 'difftool -d' ' + git difftool -d --symlinks --extcmd ls branch output stdin_contains sub output stdin_contains file output ' -test_expect_success PERL 'difftool --dir-diff' ' - git difftool --dir-diff --extcmd ls branch output +test_expect_success PERL,SYMLINKS 'difftool --dir-diff' ' + git difftool --dir-diff --symlinks --extcmd ls branch output stdin_contains sub output stdin_contains file output ' @@ -362,16 +364,16 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without unstage test_cmp actual expect ' -test_expect_success PERL 'difftool --dir-diff ignores --prompt' ' - git difftool --dir-diff --prompt --extcmd ls branch output +test_expect_success PERL,SYMLINKS 'difftool --dir-diff ignores --prompt' ' + git difftool --dir-diff --symlinks --prompt --extcmd ls branch output stdin_contains sub output stdin_contains file output ' -test_expect_success PERL 'difftool --dir-diff from subdirectory' ' +test_expect_success PERL,SYMLINKS 'difftool --dir-diff from subdirectory' ' ( cd sub - git difftool --dir-diff --extcmd ls branch output + git difftool --dir-diff --symlinks --extcmd ls branch output stdin_contains sub output stdin_contains file output ) (Only tested on MinGW, which skips the tests.) I leave it to you to write --no-symlinks tests. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] t7800: modernize tests
Am 3/22/2013 11:00, schrieb John Keeping: On Fri, Mar 22, 2013 at 08:13:46AM +0100, Johannes Sixt wrote: Am 3/21/2013 8:41, schrieb Johannes Sixt: Am 3/20/2013 23:59, schrieb David Aguilar: I started digging in and the @worktree_files (aka @worktree above) is populated from the output of git diff --raw Seeing the output filename in diff --raw implies that one of the tests added output to the index somehow. I do not see that happening anywhere, though, so I do not know how it would end up in the @worktree array if it is not reported by diff --raw. My current understanding of how it could possibly be open twice: 1. via the output redirect 2. via the copy() perl code which is fed by @worktree So I'm confused. Why would we get different results on Windows? I tracked down the difference between Windows and Linux, and it is... for my $file (@worktree) { next if $symlinks -l $b/$file; ... this line in sub dir_diff. On Linux, we take the short-cut, but on Windows we proceed through the rest of the loop, And that is likely by design. From the docs: --symlinks --no-symlinks git difftool's default behavior is create symlinks to the working tree when run in --dir-diff mode. Specifying `--no-symlinks` instructs 'git difftool' to create copies instead. `--no-symlinks` is the default on Windows. And indeed, we have this initialization: my %opts = ( ... symlinks = $^O ne 'cygwin' $^O ne 'MSWin32' $^O ne 'msys', ... ); Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor --no-symlinks is passed? Perhaps the right solution is this: We already have tests that explicitly pass '--symlinks'. I wonder if it would be better to change output to .git/output, which should avoid the problem by moving the output file out of the working tree. The point of using --symlinks is not to test the effect of the option, but to test the same thing on Unix and on Cygwin, because the latter uses --no-symlinks by default. Therefore, I think that this sketch is the right thing to do. But the real problem seems to be that output should not be among the files treated in the cited pieces of code (unless I'm wrong, of course; I know next to nothing about git-difftool). It should not matter where the file lives. Just add --no-symlinks to the difftool invocation of test difftool -d and watch it fail on Linux, too. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t7800: fix tests when difftool uses --no-symlinks
Am 22.03.2013 20:36, schrieb John Keeping: When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory But this should not happen, should it? 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. Isn't this just painting over the bug that output is incorrectly copied? In the longer term, difftool probably needs to learn to warn the user instead of overwrite any changes that have been made to the working tree file. Sure, but this is an independent issue. diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e694972..1eed439 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -319,29 +319,29 @@ test_expect_success PERL 'setup change in subdirectory' ' ' test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output - grep sub output - grep file output + git difftool -d --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t7800: don't hide grep output
Am 22.03.2013 20:36, schrieb John Keeping: Remove the stdin_contains and stdin_doesnt_contain helper functions which add nothing but hide the output of grep, hurting debugging. Thanks. Patch looks good. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid false positives in label detection in cpp diff hunk header regex.
Am 22.03.2013 23:32, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Am 22.03.2013 16:02, schrieb Junio C Hamano: Vadim Zeitlin vz-...@zeitlins.org writes: A C++ method start such as void foo::bar() wasn't recognized by cpp diff driver as it mistakenly included foo::bar as a label. However the colon in a label can't be followed by another colon, so recognize this case specially to correctly detect C++ methods using this style. Much appreciated! PATTERNS(cpp, /* Jump targets or access declarations */ -!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n +!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n Hmm. Wouldn't find a word (possibly after indentation), colon and then either a non-colon or end of line be sufficient and simpler? iow, something like... !^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$) Yes, indeed. We don't need to match more than necessary in a negative pattern. The \n must still remain, though. ... because \n is not for matching against the text, but merely to separate the regular expressions, right? Correct. I also wonder if label : should also be caught, or is it too weird format to be worth supporting? It's easy to support, by inserting another [ \t] before the first colon. So, why not? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] do not use GIT_TRACE_PACKET=3 in tests
Am 3/20/2013 18:43, schrieb Jeff King: Now that we are using the GIT_TRACE mechanism, we can improve both scripts by asking git to write directly to a file rather than a descriptor. That fixes the hang in t5700, and should allow t5503 to successfully run on Windows. Well spotted, and, right, both tests pass with this patch. Tested-by: Johannes Sixt j...@kdbg.org Thanks, -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH jk/checkout-attribute-lookup] t2003: work around path mangling issue on Windows
Am 3/20/2013 18:10, schrieb Junio C Hamano: Johannes Sixt j.s...@viscovery.net writes: From: Johannes Sixt j...@kdbg.org MSYS bash considers the part /g in the sed expression s/./=/g as an absolute path after an assignment, and mangles it to a C:/something string. Do not attract bash's attention by avoiding the equals sign. If this breakage is about path mangling, I suspect it may be cleaner to work it around by not using / as the pattern separator, e.g. sed -e s!.!=!g Half a year down the road you'd scratch your head why you were not using '/' as separator. As the replacement character is irrelevant here, it's better to exchange that. Therefore, I still prefer my version. Or perhaps use SHELL_PATH to point at a more reasonable implementation of shell that does not have such an idiocy? Well, POSIX and DOS paths look inherently different, particularly absolute paths. You can't write a reasonably portable shell script if the shell doesn't help in some way. Not to mention that the supply of POSIX shells on Windows is inherently scarce. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] t7800: modernize tests
Am 3/20/2013 23:59, schrieb David Aguilar: I started digging in and the @worktree_files (aka @worktree above) is populated from the output of git diff --raw Seeing the output filename in diff --raw implies that one of the tests added output to the index somehow. I do not see that happening anywhere, though, so I do not know how it would end up in the @worktree array if it is not reported by diff --raw. My current understanding of how it could possibly be open twice: 1. via the output redirect 2. via the copy() perl code which is fed by @worktree So I'm confused. Why would we get different results on Windows? I tracked down the difference between Windows and Linux, and it is... for my $file (@worktree) { next if $symlinks -l $b/$file; ... this line in sub dir_diff. On Linux, we take the short-cut, but on Windows we proceed through the rest of the loop, which ultimately finds a difference here: my $diff = compare($b/$file, $workdir/$file); and attempts to copy a file here: copy($b/$file, $workdir/$file) or where one of the files is the locked output file. I don't know how essential symlinks are for the operation of git-difftool and whether something can be done about it. The immediate fix is apparently to protect the tests with SYMLINKS. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] drop some int x = x hacks to silence gcc warnings
Am 3/21/2013 12:03, schrieb Jeff King: I was fooling around with clang and noticed that it complains about the int x = x construct under -Wall. That is IMHO a deficiency in clang, since the idiom has a well-defined use in silencing -Wuninitialized warnings. IMO, that's a myth. The construct invokes undefined behavior at least since C99, and the compilers are right to complain about it. But you might just say that standards are not worth the paper they are printed on, and you may possibly be right for practical reasons. But I still consider it a myth that int x = x is an idiom. I'm in the C business since more than 25 years, and the first time I saw the idiom was in git code. Is there any evidence that the construct is used elsewhere? Have I been in the wrong corner of the C world for such a long time? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH jk/pkt-line-cleanup] t5700-clone-reference: send trace to fd 2, not 3, to please Windows git
From: Johannes Sixt j...@kdbg.org Two tests use GIT_TRACE=3 to dump debugging information of git. On Windows, however, bash is unable to set up file descriptor 3 correctly for its child process, so that git reports Bad file descriptor on every trace attempt. The 'git clone' test succeeds nevertheless because an empty trace file remains, and there is only a check for the absence of a particular line. But the 'git fetch' process ultimately hangs (the dynamics that lead to this surprising result have not been investigated). Since we do not otherwise use stderr in the test cases, divert the trace dump to stderr. Signed-off-by: Johannes Sixt j...@kdbg.org --- This fixes a regression introduced in t/t5700-clone-reference.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh index 9cd3b4d..8b5c58e 100755 --- a/t/t5700-clone-reference.sh +++ b/t/t5700-clone-reference.sh @@ -55,7 +55,7 @@ cd $base_dir rm -f $U.D test_expect_success 'cloning with reference (no -l -s)' \ -'GIT_TRACE_PACKET=3 git clone --reference B file://$(pwd)/A D 3$U.D' +'GIT_TRACE_PACKET=2 git clone --reference B file://$(pwd)/A D 2$U.D' test_expect_success 'fetched no objects' \ '! grep want $U.D' @@ -173,7 +173,7 @@ test_expect_success 'fetch with incomplete alternates' ' ( cd K git remote add J file://$base_dir/J - GIT_TRACE_PACKET=3 git fetch J 3$U.K + GIT_TRACE_PACKET=2 git fetch J 2$U.K ) master_object=$(cd A git for-each-ref --format=%(objectname) refs/heads/master) ! grep want $master_object $U.K -- 1.8.2.1298.g2825a8e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html