Re: [PATCH] xdiff: trim common tail with -U0 after diff
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
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
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
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
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)
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)
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)
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)
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
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
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)
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
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
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