Re: [PATCH] xdiff: trim common tail with -U0 after diff

2017-06-23 Thread Daniel Hahler
On 23.06.2017 22:39, René Scharfe wrote:

> The changed test script passes just fine for me even without your change
> to xdiff-interface.c, which is odd.

Sorry, I've apparently messed this up - it seems to be the case for me, too.

I would assume that using the functions.c context/diff in this test file might 
prove it, but when I wrote this test I was already unsure to base it on an 
experimental feature, that might just get removed again (with its tests).

> Do you have another way to demonstrate the unexpected behavior?

It was clearly reproducible with a local test case, based on "copying an 
existing test case, and modifying the header for it only".

Given the other replies, it seems like it is more a question now if the 
trimming should get moved down, or removed completely it seems.
So I will not update the patch/test for now.


-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


[PATCH] xdiff: trim common tail with -U0 after diff

2017-06-23 Thread Daniel Hahler
When -U0 is used, trim_common_tail should be called after `xdl_diff`, so
that `--indent-heuristic` (and other diff processing) works as expected.

It also removes the check for `!(xecfg->flags & XDL_EMIT_FUNCCONTEXT)`
added in e0876bca4, which does not appear to be necessary anymore after
moving the trimming down.

This only adds a test to t4061-diff-indent.sh, but should also have one for
normal (i.e. non-experimental) diff mode probably?!

Ref: https://github.com/tomtom/quickfixsigns_vim/issues/74#issue-237900460
---
 t/t4061-diff-indent.sh | 15 +++
 xdiff-interface.c  |  7 ---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh
index 2affd7a10..df3151393 100755
--- a/t/t4061-diff-indent.sh
+++ b/t/t4061-diff-indent.sh
@@ -116,6 +116,16 @@ test_expect_success 'prepare' '
 4
EOF
 
+   cat <<-\EOF >spaces-compacted-U0-expect &&
+   diff --git a/spaces.txt b/spaces.txt
+   --- a/spaces.txt
+   +++ b/spaces.txt
+   @@ -4,0 +5,3 @@ a
+   +b
+   +a
+   +
+   EOF
+
tr "_" " " <<-\EOF >functions-expect &&
diff --git a/functions.c b/functions.c
--- a/functions.c
@@ -184,6 +194,11 @@ test_expect_success 'diff: --indent-heuristic with 
--histogram' '
compare_diff spaces-compacted-expect out-compacted4
 '
 
+test_expect_success 'diff: --indent-heuristic with -U0' '
+   git diff -U0 --indent-heuristic old new -- spaces.txt >out-compacted5 &&
+   compare_diff spaces-compacted-U0-expect out-compacted5
+'
+
 test_expect_success 'diff: ugly functions' '
git diff --no-indent-heuristic old new -- functions.c >out &&
compare_diff functions-expect out
diff --git a/xdiff-interface.c b/xdiff-interface.c
index d3f78ca2a..a7e0e3583 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -125,16 +125,17 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b)
 
 int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t 
const *xecfg, xdemitcb_t *xecb)
 {
+   int ret;
mmfile_t a = *mf1;
mmfile_t b = *mf2;
 
if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
return -1;
 
-   if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
+   ret = xdl_diff(, , xpp, xecfg, xecb);
+   if (ret && !xecfg->ctxlen)
trim_common_tail(, );
-
-   return xdl_diff(, , xpp, xecfg, xecb);
+   return ret;
 }
 
 int xdi_diff_outf(mmfile_t *mf1, mmfile_t *mf2,
-- 
2.13.1



Left with empty files after "git stash pop" when system hung

2016-09-13 Thread Daniel Hahler
I have used "git stash --include-untracked", checked out another branch,
went back, and "git stash pop"ed the changes.
Then my system crashed/hung (music that was playing was repeated in a
loop).  I have waited for some minutes, and then turned it off.

Afterwards, the repository in question was in a state where all files
contained in the stash were empty.
"git status" looked good on first sight: all the untracked and modified
files were listed there; but they were empty.

  % git fsck --lost-found
  error: object file .git/objects/04/1e659b5dbfd3f0be351a782b54743692875aec is 
empty
  error: object file .git/objects/04/1e659b5dbfd3f0be351a782b54743692875aec is 
empty
  fatal: loose object 041e659b5dbfd3f0be351a782b54743692875aec (stored in 
.git/objects/04/1e659b5dbfd3f0be351a782b54743692875aec) is corrupt
  % find .git/objects -size 0|wc -l
  12

I would have assumed that the "stash pop" operation would be "atomic",
i.e. it should not remove the stash object before other objects have
been written successfully.

The filesystem in question is ext4, and I am using Arch Linux.

I have removed all empty files in .git/objects and tried to find the
previous stash with `gitk --all $( git fsck | awk '{print $3}' )` then,
but it appears to have disappeared.

Please CC me in replies.


Cheers,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


"git-rebase -i --autostash" will leave dangling stash when editor is aborted

2016-06-15 Thread Daniel Hahler
TEST CASE:

1. Modify a file that need to get stashed on rebasing
2. Run "EDITOR=vim git rebase -i --autostash"
3. Abort with ":cq", which will make Vim exit non-zero

Git then will create an autostash, but aborts with "Could not execute
editor", via the following code in git-rebase--interactive.sh, and does
not restore the autostash - it does not show up with the regular
stashes, like when there is a conflict during the rebase:

git_sequence_editor "$todo" ||
die_abort "Could not execute editor"

Step 2 and 3 and probably merged into a single one for a test case, but
that's how I keep triggering it.

Instead of adding it to the list of stashes, it should probably get
restored/re-applied right away, since no rebase actions have been triggered.


Please CC me in replies.


Cheers,
Daniel.

--
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


Bug: "git rebase --no-autostash" not recognized

2015-09-10 Thread Daniel Hahler
Although the man page mentions the "--no-autostash" option, it is not supported:

% git rebase --no-autostash 
error: unknown option `no-autostash'

% git --version
git version 2.5.1.dirty


Please CC me in case of replies.


Regards,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


Please use distinct messages for unable to read errors (with corrupt repo after git stash -k)

2015-02-18 Thread Daniel Hahler
Hi,

I've just experienced an issue, where the system hung after git stash -k,
and I've had to use Alt-SysRq-REISUB to reboot.

Afterwards git diff and git status failed with:

fatal: unable to read 7eaa1fb32551b16198924bfb8b9d3674fed2a59a

When looking at Git's source I've found several places that use the same call
to `die`:

die(_(unable to read %s)

I think it would be more helpful to have distinct messages for each of them,
which would at least make it easier to find out where it is failing.


As for the issue at hand, I could not find the reference `7eaa1fb3` anywhere,
except for it being an empty object in .git/objects, which I've deleted then.

It looked like 7eaa1fb should have become the new stash reference, but then
the system crash happened before it has been finished/done.

But where did Git pick it from after reboot / with the corrupt repo?


I could fix it by cloning the repository locally again, and then
copying/merging the configuration, stash references etc.

I was using Git v2.3.0 when this happened.


Regards,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


Re: git: regression with mergetool and answering n (backport fix / add tests)

2015-01-23 Thread Daniel Hahler
Hi,

I am a bit surprised that this bug still exists in maint / v2.2.2.

Cherry-picking/merging 0ddedd4 fixes it.


Regards,
Daniel.

On 26.12.2014 02:12, Daniel Hahler wrote:
 Hi David,
 
 sorry for the confusion - the patch / fix I've mentioned was meant to be
 applied on the commit that caused the regression and not current master.
 
 
 Cheers,
 Daniel.
 
 On 26.12.2014 02:00, David Aguilar wrote:
 On Tue, Dec 23, 2014 at 08:08:34PM +0100, Daniel Hahler wrote:
 Hi,

 this is in reply to the commits from David:

 commit 0ddedd4d6b9b3e8eb3557d8ed28e1a0b354a25f8
 Refs: v2.2.0-60-g0ddedd4
 Merge: e886efd 1e86d5b
 Author: Junio C Hamano gits...@pobox.com
 AuthorDate: Fri Dec 12 14:31:39 2014 -0800
 Commit: Junio C Hamano gits...@pobox.com
 CommitDate: Fri Dec 12 14:31:39 2014 -0800

 Merge branch 'da/difftool-mergetool-simplify-reporting-status'

 Code simplification.

 * da/difftool-mergetool-simplify-reporting-status:
   mergetools: stop setting $status in merge_cmd()
   mergetool: simplify conditionals
   difftool--helper: add explicit exit statement
   mergetool--lib: remove use of $status global
   mergetool--lib: remove no-op assignment to $status from 
 setup_user_tool

 I've ran into a problem, where git mergetool (using vimdiff) would add
 the changes to the index, although you'd answered n after not 
 changing/saving
 the merged file.

 Thanks for the heads-up.

 Do you perhaps have mergetool.vimdiff.trustExitCode defined, or
 a similar setting?

 If you saw the prompt then it should have aborted right after
 you answered n.

 The very last thing merge_cmd() for vimdiff does is call
 check_unchanged().  We'll come back to check_unchanged() later.

 I tried to reproduce this issue.  Here's a transcript:

 
 $ git status -s
 UU file.txt

 $ git mergetool -t vimdiff file.txt
 Merging:
 file.txt

 Normal merge conflict for 'file.txt':
   {local}: modified file
   {remote}: modified file
 4 files to edit
  Enter :qall inside vim
 file.txt seems unchanged.
 Was the merge successful? [y/n] n
 merge of file.txt failed
 Continue merging other unresolved paths (y/n) ? n

 $ git status -s
 UU file.txt
 

 That seemed to work fine.  Any clues?
 More notes below...

 This regression has been introduced in:

 commit 99474b6340dbcbe58f6c256fdee231cbadb060f4
 Author: David Aguilar dav...@gmail.com
 Date:   Fri Nov 14 13:33:55 2014 -0800

 difftool: honor --trust-exit-code for builtin tools
 
 run_merge_tool() was not setting $status, which prevented the
 exit code for builtin tools from being forwarded to the caller.
 
 Capture the exit status and add a test to guarantee the behavior.
 
 Reported-by: Adria Farres 14farr...@gmail.com
 Signed-off-by: David Aguilar dav...@gmail.com
 Signed-off-by: Junio C Hamano gits...@pobox.com

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index c45a020..cce4f8c 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -221,6 +221,7 @@ run_merge_tool () {
 else
 run_diff_cmd $1
 fi
 +   status=$?
 return $status
  }


 My fix has been the following, but I agree that the changes from David
 are much better in general.

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index cce4f8c..fa9acb1 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -105,6 +105,7 @@ check_unchanged () {
 esac
 done
 fi
 +   return $status
  }

 I don't think this fix does anything.
 Here is all of check_unchanged() for context:

 check_unchanged () {
  if test $MERGED -nt $BACKUP
  then
  return 0
  else
  while true
  do
  echo $MERGED seems unchanged.
  printf Was the merge successful? [y/n] 
  read answer || return 1
  case $answer in
  y*|Y*) return 0 ;;
  n*|N*) return 1 ;;
  esac
  done
  fi
 }

 The addition of return $status after the fi in the above fix
 won't do anything because that code is unreachable.
 We either return 0 or 1.

 I haven't verified if it really fixes the regression, but if it does it
 should get backported into the branches where the regression is present.

 Also, the $status variable doesn't even exist anymore, so the
 fix is suspect.

 What platform are you on?

 Also, there should be some tests for this.

 I don't disagree with that ;-)

 Let me know if you have any clues.  I don't see anything obvious.

 cheers,

 

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


Re: git: regression with mergetool and answering n (backport fix / add tests)

2014-12-25 Thread Daniel Hahler
Hi David,

sorry for the confusion - the patch / fix I've mentioned was meant to be
applied on the commit that caused the regression and not current master.


Cheers,
Daniel.

On 26.12.2014 02:00, David Aguilar wrote:
 On Tue, Dec 23, 2014 at 08:08:34PM +0100, Daniel Hahler wrote:
 Hi,

 this is in reply to the commits from David:

 commit 0ddedd4d6b9b3e8eb3557d8ed28e1a0b354a25f8
 Refs: v2.2.0-60-g0ddedd4
 Merge: e886efd 1e86d5b
 Author: Junio C Hamano gits...@pobox.com
 AuthorDate: Fri Dec 12 14:31:39 2014 -0800
 Commit: Junio C Hamano gits...@pobox.com
 CommitDate: Fri Dec 12 14:31:39 2014 -0800

 Merge branch 'da/difftool-mergetool-simplify-reporting-status'

 Code simplification.

 * da/difftool-mergetool-simplify-reporting-status:
   mergetools: stop setting $status in merge_cmd()
   mergetool: simplify conditionals
   difftool--helper: add explicit exit statement
   mergetool--lib: remove use of $status global
   mergetool--lib: remove no-op assignment to $status from 
 setup_user_tool

 I've ran into a problem, where git mergetool (using vimdiff) would add
 the changes to the index, although you'd answered n after not 
 changing/saving
 the merged file.
 
 Thanks for the heads-up.
 
 Do you perhaps have mergetool.vimdiff.trustExitCode defined, or
 a similar setting?
 
 If you saw the prompt then it should have aborted right after
 you answered n.
 
 The very last thing merge_cmd() for vimdiff does is call
 check_unchanged().  We'll come back to check_unchanged() later.
 
 I tried to reproduce this issue.  Here's a transcript:
 
 
 $ git status -s
 UU file.txt
 
 $ git mergetool -t vimdiff file.txt
 Merging:
 file.txt
 
 Normal merge conflict for 'file.txt':
   {local}: modified file
   {remote}: modified file
 4 files to edit
  Enter :qall inside vim
 file.txt seems unchanged.
 Was the merge successful? [y/n] n
 merge of file.txt failed
 Continue merging other unresolved paths (y/n) ? n
 
 $ git status -s
 UU file.txt
 
 
 That seemed to work fine.  Any clues?
 More notes below...
 
 This regression has been introduced in:

 commit 99474b6340dbcbe58f6c256fdee231cbadb060f4
 Author: David Aguilar dav...@gmail.com
 Date:   Fri Nov 14 13:33:55 2014 -0800

 difftool: honor --trust-exit-code for builtin tools
 
 run_merge_tool() was not setting $status, which prevented the
 exit code for builtin tools from being forwarded to the caller.
 
 Capture the exit status and add a test to guarantee the behavior.
 
 Reported-by: Adria Farres 14farr...@gmail.com
 Signed-off-by: David Aguilar dav...@gmail.com
 Signed-off-by: Junio C Hamano gits...@pobox.com

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index c45a020..cce4f8c 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -221,6 +221,7 @@ run_merge_tool () {
 else
 run_diff_cmd $1
 fi
 +   status=$?
 return $status
  }


 My fix has been the following, but I agree that the changes from David
 are much better in general.

 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
 index cce4f8c..fa9acb1 100644
 --- a/git-mergetool--lib.sh
 +++ b/git-mergetool--lib.sh
 @@ -105,6 +105,7 @@ check_unchanged () {
 esac
 done
 fi
 +   return $status
  }
 
 I don't think this fix does anything.
 Here is all of check_unchanged() for context:
 
 check_unchanged () {
   if test $MERGED -nt $BACKUP
   then
   return 0
   else
   while true
   do
   echo $MERGED seems unchanged.
   printf Was the merge successful? [y/n] 
   read answer || return 1
   case $answer in
   y*|Y*) return 0 ;;
   n*|N*) return 1 ;;
   esac
   done
   fi
 }
 
 The addition of return $status after the fi in the above fix
 won't do anything because that code is unreachable.
 We either return 0 or 1.
 
 I haven't verified if it really fixes the regression, but if it does it
 should get backported into the branches where the regression is present.
 
 Also, the $status variable doesn't even exist anymore, so the
 fix is suspect.
 
 What platform are you on?
 
 Also, there should be some tests for this.
 
 I don't disagree with that ;-)
 
 Let me know if you have any clues.  I don't see anything obvious.
 
 cheers,
 

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


git: regression with mergetool and answering n (backport fix / add tests)

2014-12-23 Thread Daniel Hahler
Hi,

this is in reply to the commits from David:

commit 0ddedd4d6b9b3e8eb3557d8ed28e1a0b354a25f8
Refs: v2.2.0-60-g0ddedd4
Merge: e886efd 1e86d5b
Author: Junio C Hamano gits...@pobox.com
AuthorDate: Fri Dec 12 14:31:39 2014 -0800
Commit: Junio C Hamano gits...@pobox.com
CommitDate: Fri Dec 12 14:31:39 2014 -0800

Merge branch 'da/difftool-mergetool-simplify-reporting-status'

Code simplification.

* da/difftool-mergetool-simplify-reporting-status:
  mergetools: stop setting $status in merge_cmd()
  mergetool: simplify conditionals
  difftool--helper: add explicit exit statement
  mergetool--lib: remove use of $status global
  mergetool--lib: remove no-op assignment to $status from 
setup_user_tool

I've ran into a problem, where git mergetool (using vimdiff) would add
the changes to the index, although you'd answered n after not changing/saving
the merged file.

This regression has been introduced in:

commit 99474b6340dbcbe58f6c256fdee231cbadb060f4
Author: David Aguilar dav...@gmail.com
Date:   Fri Nov 14 13:33:55 2014 -0800

difftool: honor --trust-exit-code for builtin tools

run_merge_tool() was not setting $status, which prevented the
exit code for builtin tools from being forwarded to the caller.

Capture the exit status and add a test to guarantee the behavior.

Reported-by: Adria Farres 14farr...@gmail.com
Signed-off-by: David Aguilar dav...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index c45a020..cce4f8c 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -221,6 +221,7 @@ run_merge_tool () {
else
run_diff_cmd $1
fi
+   status=$?
return $status
 }


My fix has been the following, but I agree that the changes from David
are much better in general.

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index cce4f8c..fa9acb1 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -105,6 +105,7 @@ check_unchanged () {
esac
done
fi
+   return $status
 }
 
 valid_tool () {

I haven't verified if it really fixes the regression, but if it does it
should get backported into the branches where the regression is present.


Also, there should be some tests for this. I've came up with the
following, which is not finished and left in a state of debugging why it
did not trigger the bug.

In t/t7610-mergetool.sh:

  test_expect_success 'return code with untrusted mergetool' '
  # git config merge.tool untrusted 
  # git config mergetool.untrusted.cmd true 
  # git config mergetool.untrusted.trustExitCode false 
  test_config mergetool.vimdiff.cmd cat \\$REMOTE\ 
  git config merge.tool vimdiff 
  git config --get-regexp mergetool.
  git config --get-regexp merge\.tool.

  git checkout -b test_untrusted branch1 
  test_must_fail git merge master /dev/null 21 
  git status -s 
  test $(git status --short both) = AA both 
  ( echo n | test_must_fail git mergetool both ) 
  git status -s 

  test $(git status --short both) = AA both 
  ( echo y | git mergetool both ) 
  git status -s 
  test $(git status --short both) = M  both 
  git reset --hard 
  git config merge.tool mytool


Cheers,
Daniel.

-- 
http://daniel.hahler.de/




signature.asc
Description: OpenPGP digital signature


diff: use built-in patterns by default via git attributes

2014-12-09 Thread Daniel Hahler
Hi,

I'm wondering why the built-in patterns (defined in userdiff.c) are not
being applied by default, e.g. what you would normally do in
core.attributesfile via:

*.py diff=python

Wouldn't it make sense to provide certain defaults for attributes, where
Git provides enhanced patterns?


Regards,
Daniel.

-- 
http://daniel.hahler.de/




signature.asc
Description: OpenPGP digital signature


diff-index does not consider a removed submodule to be staged with --ignore-submodules

2014-09-20 Thread Daniel Hahler
After staging the removal of a submodule, diff-index does not consider this 
when --ignore-submodules is being used:

# In a repository with submodule sm:
% git rm --cached sm
% git diff-index --cached --quiet --ignore-submodules HEAD
% echo $?
0
% git status
On branch master
Changes to be committed:
  (use git reset HEAD file... to unstage)

deleted:sm

git status --ignore-submodules behaves the same.

From the man page of --ignore-submodules it looks like the option is meant 
to prevent scanning of submodules itself, but in this case the main repository 
is affected.

This command is used by zsh's vcs_info module (in 
Functions/VCS_Info/Backends/VCS_INFO_get_data_git):

if (( querystaged )) ; then
if ${vcs_comm[cmd]} rev-parse --quiet --verify HEAD  /dev/null ; then
${vcs_comm[cmd]} diff-index --cached --quiet --ignore-submodules 
HEAD 2 /dev/null
(( $?  $? != 128 ))  gitstaged=1

Is this a bug/oversight in Git or by design?
Is there a better way to detect if there are any staged changes?


Regards,
Daniel.

-- 
http://daniel.hahler.de/

--
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


Output from rev-parse --resolve-git-dir changed: two lines (prints original argument)

2014-02-17 Thread Daniel Hahler
I have noticed that the output from git rev-parse --resolve-git-dir changed.

While it used to only print the resolved git dir, it now prints the argument 
passed to it itself:

% git rev-parse --resolve-git-dir .git
/path/to/root/.git/modules/vim/bundle/reporoot
.git

This is with Git version 1.8.5.3, but also happens with master.

You can use --flags to avoid this (git rev-parse --flags --resolve-git-dir 
.git), but it is unclear from the documentation if that's the intended 
beahviour.

It seems like the --resolve-git-dir subcommand needs to consume the argument 
passed to it.


Regards,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


Re: Bug: relative core.worktree is resolved from symlink and not its target

2014-02-17 Thread Daniel Hahler
On 09.02.2014 10:08, Duy Nguyen wrote:
 On Tue, Feb 04, 2014 at 11:20:39AM +0100, Daniel Hahler wrote:

Thanks for looking into this.

 when using a submodule sm, there is a relative worktree in its config:

.git/modules/sm/config:
[core]
 worktree = ../../../smworktree

 git-new-worktree (from contrib) symlinks this config the new worktree.

 From inside the new worktree, git reads the config, but resolves the
 relative worktree setting based on the symlink's location.
 
 Hmm.. core.worktree is relative to $GIT_DIR. Whether config is a
 symlink should have no effects.

If config is a symlink, the relative path for worktree is meant to be
resolved based on the config file's location, and not from the symlink
($GIT_DIR).

Here is a test case to reproduce it:

  # Create a submodule repo
  mkdir /tmp/t-sm
  cd /tmp/t-sm
  git init
  touch foo
  git add foo
  git commit -m init

  # Create the root repo
  mkdir /tmp/t-root
  cd /tmp/t-root
  git init
  mkdir submodules
  git submodule add /tmp/t-sm submodules/sm
  git commit -m init

  # Create a new worktree from the submodule
  cd /tmp/t-root/submodules/sm
  git-new-workdir . /tmp/new-workdir

This then fails when checking out:
+ git checkout -f
fatal: Could not chdir to '../../../../submodules/sm': No such file or directory

% ls -l /tmp/new-workdir/.git/config 
[…] /tmp/new-workdir/.git/config - 
/tmp/t-root/.git/modules/submodules/sm/config

% cat /tmp/new-workdir/.git/config 
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
worktree = ../../../../submodules/sm


From inside of /tmp/new-workdir `git rev-parse --git-dir` fails already
(with the same cannot chdir error).

The problem appears to be that it tries to chdir based on
/tmp/new-workdir/.git, but should do so based on
$(readlink -f .git/config).

I recognize that this case is constructed anyway, because even if
`worktree` would get resolved correctly, it would not be what you'd
expect: the point of git-new-workdir is to get a separate worktree, and
not use the existing one.

Therefore I see two problems here:
1. worktree is not resolved correctly by git itself (with .git/config
   being a symlink)
2. git-new-workdir should handle this better, e.g. by creating a copy of
   the config file with the worktree setting removed and printing a
   warning about it.

The workaround appears to be to explicitly set
GIT_WORK_TREE=/tmp/new-workdir.


Regards,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature


Bug: relative core.worktree is resolved from symlink and not its target

2014-02-04 Thread Daniel Hahler
Hi,

when using a submodule sm, there is a relative worktree in its config:

   .git/modules/sm/config:
   [core]
worktree = ../../../smworktree

git-new-worktree (from contrib) symlinks this config the new worktree.

From inside the new worktree, git reads the config, but resolves the
relative worktree setting based on the symlink's location.

A fix would be to resolve any relative worktree setting based on the
symlink target's location (the actual config file), and not from the
symlink.

This is with git version 1.8.5.3.

Please consider fixing this.

(I know about various workarounds, e.g. copying and adjusting config
or manually setting $GIT_WORK_TREE; more relevant discussion would be
at http://comments.gmane.org/gmane.comp.version-control.git/196019)


Thanks,
Daniel.

-- 
http://daniel.hahler.de/



signature.asc
Description: OpenPGP digital signature