Re: [PATCH] status: display the SHA1 of the commit being currently processed

2013-06-17 Thread Johannes Sixt
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

2013-06-13 Thread Johannes Sixt
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

2013-06-11 Thread Johannes Sixt
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

2013-06-10 Thread Johannes Sixt
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

2013-06-09 Thread Johannes Sixt
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

2013-06-09 Thread Johannes Sixt
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

2013-06-08 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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

2013-06-07 Thread Johannes Sixt
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)

2013-06-07 Thread Johannes Sixt
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

2013-06-06 Thread Johannes Sixt
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

2013-06-05 Thread Johannes Sixt
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)

2013-06-05 Thread Johannes Sixt
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)

2013-06-05 Thread Johannes Sixt
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)

2013-06-05 Thread Johannes Sixt
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

2013-06-04 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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)

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-06-01 Thread Johannes Sixt
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

2013-05-31 Thread Johannes Sixt
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

2013-05-30 Thread Johannes Sixt
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

2013-05-30 Thread Johannes Sixt
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

2013-05-29 Thread Johannes Sixt
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

2013-05-29 Thread Johannes Sixt
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

2013-05-29 Thread Johannes Sixt
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

2013-05-29 Thread Johannes Sixt
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

2013-05-26 Thread Johannes Sixt
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

2013-05-18 Thread Johannes Sixt
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

2013-05-18 Thread Johannes Sixt
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

2013-05-17 Thread Johannes Sixt
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

2013-05-16 Thread Johannes Sixt
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

2013-05-15 Thread Johannes Sixt
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

2013-05-14 Thread Johannes Sixt
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

2013-05-10 Thread Johannes Sixt
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

2013-05-10 Thread Johannes Sixt
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

2013-05-10 Thread Johannes Sixt
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

2013-05-08 Thread Johannes Sixt
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?

2013-05-04 Thread Johannes Sixt
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

2013-05-02 Thread Johannes Sixt
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

2013-05-02 Thread Johannes Sixt
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

2013-05-01 Thread Johannes Sixt
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

2013-04-27 Thread Johannes Sixt
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

2013-04-26 Thread Johannes Sixt
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

2013-04-25 Thread Johannes Sixt
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

2013-04-25 Thread Johannes Sixt
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)

2013-04-24 Thread Johannes Sixt
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)

2013-04-24 Thread 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'.
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()

2013-04-24 Thread Johannes Sixt
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

2013-04-24 Thread Johannes Sixt
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

2013-04-22 Thread Johannes Sixt
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

2013-04-19 Thread Johannes Sixt
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

2013-04-19 Thread Johannes Sixt
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

2013-04-19 Thread Johannes Sixt
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

2013-04-18 Thread Johannes Sixt
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

2013-04-18 Thread Johannes Sixt
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

2013-04-18 Thread Johannes Sixt
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

2013-04-18 Thread Johannes Sixt
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

2013-04-16 Thread Johannes Sixt
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

2013-04-16 Thread Johannes Sixt
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

2013-04-16 Thread Johannes Sixt
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

2013-04-16 Thread Johannes Sixt
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

2013-04-15 Thread Johannes Sixt
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

2013-04-09 Thread Johannes Sixt
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

2013-04-02 Thread Johannes Sixt
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

2013-04-02 Thread Johannes Sixt
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

2013-04-02 Thread Johannes Sixt
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

2013-04-01 Thread Johannes Sixt
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

2013-03-26 Thread Johannes Sixt
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

2013-03-26 Thread Johannes Sixt
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

2013-03-26 Thread Johannes Sixt
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

2013-03-25 Thread Johannes Sixt
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

2013-03-25 Thread Johannes Sixt
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

2013-03-25 Thread Johannes Sixt
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

2013-03-22 Thread Johannes Sixt
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

2013-03-22 Thread Johannes Sixt
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

2013-03-22 Thread Johannes Sixt
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

2013-03-22 Thread Johannes Sixt
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.

2013-03-22 Thread Johannes Sixt
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

2013-03-21 Thread Johannes Sixt
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

2013-03-21 Thread Johannes Sixt
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

2013-03-21 Thread 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, 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

2013-03-21 Thread Johannes Sixt
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

2013-03-20 Thread Johannes Sixt
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


<    5   6   7   8   9   10   11   12   >