Re: Git diff|status against remote repo
Am 16.09.19 um 12:45 schrieb Pratyush Yadav: On 16/09/19 11:02AM, Stefanie Leisestreichler wrote: Hi. I am far from being a pro in git. There is something I do not understand. This is my git config: [core] repositoryformatversion = 0 filemode = true bare = false logallrefupdates = true [remote "origin"] url = ssh://git@192.168.2.2:/home/git/PROJECT.git fetch = +refs/heads/*:refs/remotes/origin/* [branch "master"] remote = origin merge = refs/heads/master [branch "develop"] remote = origin merge = refs/heads/develop I have a local repo called "develop" also on each involved dev machine. Developer A has made changes on nearly each file of the project by changing the namespace and locally commited those changes to his local branch "develop". After that he did a git push. On a not involved machine M I did a git clone ssh://git@192.168.2.2:/home/git/PROJECT.git and after that I did a "git checkout develop" which was followed by a message like "Branch develop is following remote branch develop from origin". I can see all the changes Developer A has made. On my machine DEV I am on my local branch develop. Since I did not pull or merge from origin already, I am not able to see any of those changes developer A has made. So far so good. But I would expect, that git status (which results in "On branch develop. Your branch is up to date with origin/develop) or "git diff origin/develop develop" or "git diff origin/develop...develop" (both no output at all) would give me a hint that I am x commits behind origin/master or (git diff) will show me the changes Developer A committed to the Repro. But nothing... What am I doing wrong? You haven't gotten the new commits from origin, so your local repo can't know how ahead/behind it is from its remote version. Run `git fetch origin`. Now if you run `git status`, it should show you that you are X commits behind origin. `git pull` does two things: `git fetch` and `git merge`. fetch "downloads" all the commits from remote, and merge then puts them in your local branch. So if you do a `git pull`, developer A's changes are now merged into your tree. But if you only do a fetch, origin/master gets updated, but not your local master. So now status can know how far you are behind, but your local branch is not changed yet. Once you are ready to finally get those changes in your local branch, run `git merge origin/master`. Thanks a lot. git fetch did the trick.
Re: Git diff|status against remote repo
On 16/09/19 11:02AM, Stefanie Leisestreichler wrote: > Hi. > > I am far from being a pro in git. > There is something I do not understand. > > This is my git config: > [core] > repositoryformatversion = 0 > filemode = true > bare = false > logallrefupdates = true > [remote "origin"] > url = ssh://git@192.168.2.2:/home/git/PROJECT.git > fetch = +refs/heads/*:refs/remotes/origin/* > [branch "master"] > remote = origin > merge = refs/heads/master > [branch "develop"] > remote = origin > merge = refs/heads/develop > > I have a local repo called "develop" also on each involved dev machine. > > Developer A has made changes on nearly each file of the project by changing > the namespace and locally commited those changes to his local branch > "develop". After that he did a git push. > > On a not involved machine M I did a git clone > ssh://git@192.168.2.2:/home/git/PROJECT.git and after that I did a "git > checkout develop" which was followed by a message like "Branch develop is > following remote branch develop from origin". I can see all the changes > Developer A has made. > > On my machine DEV I am on my local branch develop. Since I did not pull or > merge from origin already, I am not able to see any of those changes > developer A has made. So far so good. But I would expect, that git status > (which results in "On branch develop. Your branch is up to date with > origin/develop) or "git diff origin/develop develop" or "git diff > origin/develop...develop" (both no output at all) would give me a hint that > I am x commits behind origin/master or (git diff) will show me the changes > Developer A committed to the Repro. But nothing... > > What am I doing wrong? You haven't gotten the new commits from origin, so your local repo can't know how ahead/behind it is from its remote version. Run `git fetch origin`. Now if you run `git status`, it should show you that you are X commits behind origin. `git pull` does two things: `git fetch` and `git merge`. fetch "downloads" all the commits from remote, and merge then puts them in your local branch. So if you do a `git pull`, developer A's changes are now merged into your tree. But if you only do a fetch, origin/master gets updated, but not your local master. So now status can know how far you are behind, but your local branch is not changed yet. Once you are ready to finally get those changes in your local branch, run `git merge origin/master`. -- Regards, Pratyush Yadav
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
SZEDER Gábor writes:
> Another Python 2 vs 3 issue, perhaps?
>
> # Python2, good:
>...
>'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',
>'100644',
>'file1-mv.txt',
>'similarity index 90%\nrename from file1.txt\nrename to
> file1-mv.txt\nindex 2bef330..f8fd673 100644\n']
> # Python3, bad:
> ...
>'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd',
>'100644',
>'file1-mv.txt',
>'similarity index 90%\n'
>'rename from file1.txt\n'
>'rename to file1-mv.txt\n'
>'index 2bef330..f8fd673 100644\n']
>
> Let's just stick to plain old printf for now, as suggested by Phillip
> earlier, to reduce unnecessary confusion.
I notice that the latter is pretty-printed. Pay attention to the
last few lines that end without a trailing comma. Literal string
concatenation taking place to form a single string here, in which
case both are giving us the same string?
By the way, sorry for an earlier response based on what I
misremembered, which may have confused the discussion unnecessarily.
Ever since 65056021 ("built-in diff.", 2006-04-28) made "git diff" a
built-in, we had used (or at least attempted to use) from
:, so it is not unexpected to see paths in two blob
diff output.
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
I've figured it out. It's not a bug, it's a peculiarity of pprint(). It splits strings to avoid long lines and abuses the fact that in Python strings split with whitespace are concatenated by the parser. Also, in my example newlines are not parsed correctly in the shell. Escaping them, I get the exact same behavior as in subprocess.check_call(): $ python2 print_argv.py $'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n' ['print_argv.py', 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n'] $ python3 print_argv.py $'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n' ['print_argv.py', 'similarity index 90%\n' 'rename from file1.txt\n' 'rename to file1-mv.txt\n' 'index 2bef330..f8fd673 100644\n']
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
On Fri, 30 Aug 2019 at 17:27, Jeff King wrote: > I think you have an extra "old-filename" in the second list. Agreed. My misunderstanding was that I had thought that when documentation said "path", it meant "argv[0], the path to the diff executable". > Interesting. I _don't_ see that splitting when I run the same command in > your demo repo (nor, looking at Git's code, do I see how it could > happen; we always add the metainfo as a single argument). Szeder Gábor below has guessed at the cause. I have tested it a little more: # Test script to ensure that shell does not get in the way $ cat test.py import sys import subprocess subprocess.check_call([ 'python%s' % sys.argv[1], './print_argv.py', 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n', ]) # Python 2, expected behavior $ python3 test.py 2 ['./print_argv.py', 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n'] # Python 3, broken behavior observed before $ python3 test.py 3 ['./print_argv.py', 'similarity index 90%\n' 'rename from file1.txt\n' 'rename to file1-mv.txt\n' 'index 2bef330..f8fd673 100644\n'] # Directly via shell. Python 2, expected behavior $ python2 print_argv.py 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n' ['print_argv.py', 'similarity index 90%\\nrename from file1.txt\\nrename to file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n'] # Python 3, WTF $ python3 print_argv.py 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n' ['print_argv.py', 'similarity index 90%\\nrename from file1.txt\\nrename to ' 'file1-mv.txt\\nindex 2bef330..f8fd673 100644\\n']
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
On Fri, Aug 30, 2019 at 04:23:13PM +0300, Dmitry Nikulin wrote: > On Fri, 30 Aug 2019 at 13:16, Phillip Wood wrote: > > I'm not sure why the last argument is being split in > > your example. It is not split in the example below > > I have replicated the splitting issue on my small demo repo [1]: > > $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1 > origin/branch1-mv -- file1.txt file1-mv.txt > ['./print_argv.py', > 'file1.txt', > '/tmp/EWaCSc_file1.txt', > '2bef330804cb3f6962e45a72a12a3071ee9b5888', > '100644', > '/tmp/mtEiSc_file1-mv.txt', > 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', > '100644', > 'file1-mv.txt', > 'similarity index 90%\n' > 'rename from file1.txt\n' > 'rename to file1-mv.txt\n' > 'index 2bef330..f8fd673 100644\n'] Another Python 2 vs 3 issue, perhaps? # Python2, good: $ GIT_EXTERNAL_DIFF='python2 ./print_argv.py' git --no-pager diff -M origin/branch1 origin/branch1-mv -- file1.txt file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/ViB58B_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/p6OH9B_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'similarity index 90%\nrename from file1.txt\nrename to file1-mv.txt\nindex 2bef330..f8fd673 100644\n'] # Python3, bad: $ GIT_EXTERNAL_DIFF='python3 ./print_argv.py' git --no-pager diff -M origin/branch1 origin/branch1-mv -- file1.txt file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/5xD2DS_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/vvHQGS_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'similarity index 90%\n' 'rename from file1.txt\n' 'rename to file1-mv.txt\n' 'index 2bef330..f8fd673 100644\n'] Let's just stick to plain old printf for now, as suggested by Phillip earlier, to reduce unnecessary confusion.
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
On Fri, Aug 30, 2019 at 04:23:13PM +0300, Dmitry Nikulin wrote: > On Fri, 30 Aug 2019 at 13:16, Phillip Wood wrote: > > I'm not sure why the last argument is being split in > > your example. It is not split in the example below > > I have replicated the splitting issue on my small demo repo [1]: > > $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1 > origin/branch1-mv -- file1.txt file1-mv.txt > ['./print_argv.py', > 'file1.txt', > '/tmp/EWaCSc_file1.txt', > '2bef330804cb3f6962e45a72a12a3071ee9b5888', > '100644', > '/tmp/mtEiSc_file1-mv.txt', > 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', > '100644', > 'file1-mv.txt', > 'similarity index 90%\n' > 'rename from file1.txt\n' > 'rename to file1-mv.txt\n' > 'index 2bef330..f8fd673 100644\n'] Interesting. I _don't_ see that splitting when I run the same command in your demo repo (nor, looking at Git's code, do I see how it could happen; we always add the metainfo as a single argument). > This is, however, tangential to the original problem: documenting the > external diff CLI interface for diffing two blobs. Here is what I am > seeing: > > $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff > origin/branch1:file1.txt origin/branch1-mv:file1-mv.txt > ['./print_argv.py', > 'file1.txt', > '/tmp/n9USvy_file1.txt', > '2bef330804cb3f6962e45a72a12a3071ee9b5888', > '100644', > '/tmp/Zst0uy_file1-mv.txt', > 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', > '100644', > 'file1-mv.txt', > 'index 2bef330..f8fd673 100644\n'] > > The meaning and origin of the last arg remains mysterious, and the > other args do not conform to the published documentation[2], which > states that the args should be: > > path old-file old-hex old-mode new-file new-hex new-mode > > Instead the args that are passed are: > > path old-filename old-file old-hex old-mode new-file new-hex > new-mode new-filename something I think you have an extra "old-filename" in the second list. Without that, the first 7 arguments are as documented. The last two are: - when the destination path differs from the source path, we append it. This is generally a sign of a rename/copy, though it triggers in the blob case because Git has been manually given a pair of files with non-matching names. - the final one is metainfo that Git typically prints between the "diff" header and the diff itself. This is added only when we add the filename, and would generally carry information about the rename (but of course since there isn't one, it has only the index line). These were both added by 427dcb4bca ([PATCH] Diff overhaul, adding half of copy detection., 2005-05-21). I think the intent was that existing diff commands would/could ignore the extra arguments. Certainly the world would be a better place if those were described in the external-diff documentation (specifically in relation to renames, which is their intended use). As for the behavior in the blob-diff case, I think it's _pretty_ reasonable. It's certainly useful to give the new-filename. The metainfo is mostly useless in this case, and potentially could be suppressed if the pair is not an actual rename/copy. But I also think it's not hurting much (and a script can tell that's what's going on by the lack of rename lines in the metainfo). -Peff
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
On 30/08/2019 14:23, Dmitry Nikulin wrote: On Fri, 30 Aug 2019 at 13:16, Phillip Wood wrote: I'm not sure why the last argument is being split in your example. It is not split in the example below I have replicated the splitting issue on my small demo repo [1]: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1 origin/branch1-mv -- file1.txt file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/EWaCSc_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/mtEiSc_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'similarity index 90%\n' 'rename from file1.txt\n' 'rename to file1-mv.txt\n' 'index 2bef330..f8fd673 100644\n'] That's strange - What OS are you using? Does python do any pre-processing of arguments with newlines in them? This is, however, tangential to the original problem: documenting the external diff CLI interface for diffing two blobs. Here is what I am seeing: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff origin/branch1:file1.txt origin/branch1-mv:file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/n9USvy_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/Zst0uy_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'index 2bef330..f8fd673 100644\n'] The meaning and origin of the last arg remains mysterious, and the other args do not conform to the published documentation[2], which states that the args should be: The documentation is incomplete it should document the extra fields passed when it detects renames. path old-file old-hex old-mode new-file new-hex new-mode Instead the args that are passed are: path old-filename old-file old-hex old-mode new-file new-hex new-mode new-filename something I think what is happening is that because you're passing different filenames in to get the blobs the 'pass rename information' code is being triggered so it shows the new filename as the name used to get the second blob and then passes the header which only has an index line as there isn't any real rename information. Best Wishes Phillip [1]: https://github.com/dniku/git-external-diff-argv [2]: https://www.git-scm.com/docs/git#Documentation/git.txt-codeGITEXTERNALDIFFcode
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
On Fri, 30 Aug 2019 at 13:16, Phillip Wood wrote: > I'm not sure why the last argument is being split in > your example. It is not split in the example below I have replicated the splitting issue on my small demo repo [1]: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1 origin/branch1-mv -- file1.txt file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/EWaCSc_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/mtEiSc_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'similarity index 90%\n' 'rename from file1.txt\n' 'rename to file1-mv.txt\n' 'index 2bef330..f8fd673 100644\n'] This is, however, tangential to the original problem: documenting the external diff CLI interface for diffing two blobs. Here is what I am seeing: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff origin/branch1:file1.txt origin/branch1-mv:file1-mv.txt ['./print_argv.py', 'file1.txt', '/tmp/n9USvy_file1.txt', '2bef330804cb3f6962e45a72a12a3071ee9b5888', '100644', '/tmp/Zst0uy_file1-mv.txt', 'f8fd6737fbe5a45c97ba9c9de495dc46ff11eccd', '100644', 'file1-mv.txt', 'index 2bef330..f8fd673 100644\n'] The meaning and origin of the last arg remains mysterious, and the other args do not conform to the published documentation[2], which states that the args should be: path old-file old-hex old-mode new-file new-hex new-mode Instead the args that are passed are: path old-filename old-file old-hex old-mode new-file new-hex new-mode new-filename something [1]: https://github.com/dniku/git-external-diff-argv [2]: https://www.git-scm.com/docs/git#Documentation/git.txt-codeGITEXTERNALDIFFcode
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
Hi Dmitry On 29/08/2019 15:36, Dmitry Nikulin wrote: Thank you for the reply. [...] However, for the original repository where I first faced this problem (https://github.com/yandexdataschool/Practical_RL), Git passes a very weird set of args to the external diff: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M master coursera -- week02_value_based/seminar_vi.ipynb week2_model_based/practice_vi.ipynb ['./print_argv.py', 'week02_value_based/seminar_vi.ipynb', '/tmp/amudWz_seminar_vi.ipynb', '8f8016963c888b7dd8dd20f60b7d6fdb41b26c1d', '100644', '/tmp/Ub7zPz_practice_vi.ipynb', '21db80f53b632d975a9af0acbaf397eb717cde2c', '100644', 'week2_model_based/practice_vi.ipynb', 'similarity index 82%\n' 'rename from week02_value_based/seminar_vi.ipynb\n' 'rename to week2_model_based/practice_vi.ipynb\n' 'index 8f80169..21db80f 100644\n'] I would guess that this is a bug. There can clearly be a hotfix (after all, Git passes all of the information to the external that it should per the spec, that is, -path, -hex, -mode; adding, however, some garbage). When git detects a rename it adds two parameters to the end of the argument list for the external diff program. The first extra argument is the new name and the second is the header with the similarity information[1]. I'm not sure why the last argument is being split in your example. It is not split in the example below $git mv git.c renamed.c $env GIT_EXTERNAL_DIFF='printf "|%s|\\n"' git diff HEAD |git.c| |/tmp/lMQpP8_git.c| |c1ee7124edcfb0417539134d50212e997dc71c1f| |100644| |renamed.c| |c1ee7124edcfb0417539134d50212e997dc71c1f| |100644| |renamed.c| |similarity index 100% rename from git.c rename to renamed.c | Best Wishes Phillip [1] https://github.com/git/git/blob/master/diff.c#L4204 I do not know though to what extent this information is correct. You say that this information is lost when I use the : notation; however, Git seems to pass paths and hexes correctly. This only leaves open the question of file mode. Perhaps it could be preserved at least for some cases, such as when the blob is retrieved from a path in a tree?
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
Thank you for the reply. On Thu, 29 Aug 2019 at 06:54, Junio C Hamano wrote: > $ git diff -M branch1 branch2 -- file1 file2 > > if file1 and file2 have similar-enough contents, may have a better > chance of what you wanted to ask Git (if I am guessing what it is, > that is). The context here is that I am trying to diff two Jupyter notebooks using an external tool (git-nbdiffdriver in my case). Therefore, for me it is crucial to use the external tool, and not Git's internal machinery. For the particular command that you suggested as the replacement, on my demo repository it does not produce anything interesting, as it does not detect renames and calls my honeypot twice: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M origin/branch1 origin/branch2 -- file1.txt file2.txt ['./print_argv.py', 'file1.txt', '/tmp/2IEKCw_file1.txt', '802b1c4ed7b06162b2ce09b7db72a576695b96e5', '100644', '/dev/null', '.', '.'] ['./print_argv.py', 'file2.txt', '/dev/null', '.', '.', '/tmp/oAMdDx_file2.txt', '076e8e37a712d8a66c0c3d1a103050dc509ca6ff', '100644'] However, for the original repository where I first faced this problem (https://github.com/yandexdataschool/Practical_RL), Git passes a very weird set of args to the external diff: $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff -M master coursera -- week02_value_based/seminar_vi.ipynb week2_model_based/practice_vi.ipynb ['./print_argv.py', 'week02_value_based/seminar_vi.ipynb', '/tmp/amudWz_seminar_vi.ipynb', '8f8016963c888b7dd8dd20f60b7d6fdb41b26c1d', '100644', '/tmp/Ub7zPz_practice_vi.ipynb', '21db80f53b632d975a9af0acbaf397eb717cde2c', '100644', 'week2_model_based/practice_vi.ipynb', 'similarity index 82%\n' 'rename from week02_value_based/seminar_vi.ipynb\n' 'rename to week2_model_based/practice_vi.ipynb\n' 'index 8f80169..21db80f 100644\n'] I would guess that this is a bug. There can clearly be a hotfix (after all, Git passes all of the information to the external that it should per the spec, that is, -path, -hex, -mode; adding, however, some garbage). I do not know though to what extent this information is correct. You say that this information is lost when I use the : notation; however, Git seems to pass paths and hexes correctly. This only leaves open the question of file mode. Perhaps it could be preserved at least for some cases, such as when the blob is retrieved from a path in a tree?
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
Dmitry Nikulin writes: > $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff > origin/branch1:file1.txt origin/branch2:file2.txt I didn't even know external-diff driver is called (and does not even segfaut) in the "compare two blobs" hack codepath. The syntax : you have on the command line resolves to a blob object name. There is no leading directory name, there is no permission bits or executable bit, there is no filename, when "diff" is told to compare two blob objects this way. THe "diff" machinery that drives the external (or internal for that matter) diff will only get two 40-hex blob object names and nothing else. The only pieces of information you can trust among those the external program may receive are the blob object name(s) and the contents stored in the temporary files given to it. The location of these temporary files or their mode bits have no relation to the "files" in some tree in the original repository, as that information is long lost when you write : to tell Git to use that as a blob object name. $ git diff -M branch1 branch2 -- file1 file2 if file1 and file2 have similar-enough contents, may have a better chance of what you wanted to ask Git (if I am guessing what it is, that is).
Re: git-diff passes : args to GIT_EXTERNAL_DIFF incorrectly?
I have put up a demo repo here: https://github.com/dniku/git-external-diff-argv On Tue, 27 Aug 2019 at 21:24, Dmitry Nikulin wrote: > > I wrote a very simple Python script to see which arguments git-diff > passes to the external diff program when comparing files across > branches: > > $ env GIT_EXTERNAL_DIFF=./print_argv.py git diff > origin/branch1:file1.txt origin/branch2:file2.txt > ['./print_argv.py', > 'file1.txt', > '/tmp/QRaIJ1_file1.txt', > '802b1c4ed7b06162b2ce09b7db72a576695b96e5', > '100644', > '/tmp/AZuOJ1_file2.txt', > '076e8e37a712d8a66c0c3d1a103050dc509ca6ff', > '100644', > 'file2.txt', > 'index 802b1c4..076e8e3 100644\n'] > > According to the docs > (https://www.git-scm.com/docs/git/2.22.0#Documentation/git.txt-codeGITEXTERNALDIFFcode), > git-diff is supposed to pass 7 parameters: > > path old-file old-hex old-mode new-file new-hex new-mode > > This is not what I am seeing here. Is this a bug or > incorrect/incomplete documentation? > > Tested with git 2.22.0 and 2.17.1.
Re: git diff autocomplete
On Aug 26 2019, Dhaval Patel wrote: > If it is only about files and revisions both being handled by git > diff, would it not ne possible to do something like this? > > For files > > git diff -f a[PRESS TAB] > > For revisions > > git diff -r a[PRESS TAB] > > Some sort of flag which says we are handling files or revisions. That already exists, it is spelt `--'. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: git diff autocomplete
On Sun, Aug 18, 2019 at 08:14:15AM +0530, Dhaval Patel wrote: > How to reproduce > > [snip] > Suggested feature - > > when I press tab, git diff should autocomplete just like git add. I think this is somewhat harder of a usecase, as git add generally takes paths and diff can compare revisions *and* paths. I see that if I do git diff -- [TAB] I get all the paths as an autocomplete option, which I think is desired behavior. Thanks, -Santiago. signature.asc Description: PGP signature
Re: git diff-tree ignores --textconv
On 2018-09-23 06:23 PM, Jeff King wrote: > On Sun, Sep 23, 2018 at 05:56:03PM -0700, Stas Bekman wrote: > >>> You probably want "--ext-diff", not "--textconv". >> [...] >> Would it be safe to ask the maintainer of the application to include >> both --textconv and --ext-diff in that 'git diff-tree' call? I only need >> the latter, but someone needed --textconv there as it's in the code. > > I think so. The main reason that they are not the default for plumbing > commands such as diff-tree is that the output may be quite surprising to > anything trying to parse the output. Using --textconv will always > produce a diff, but one that may not be applied to the original content. > Using --ext-diff may produce output that doesn't even look like a diff, > though in practice they often do. > >> This is for this package: >> https://github.com/rsmmr/git-notifier > > It looks like the output is meant to be read by humans, so yeah, I think > it would be fine (and preferred) to enable both. Fantastic. Thank you so much, Jeff. -- Stas Bekman <'>< <'>< https://stasosphere.com https://chestofbooks.com https://experientialsexlab.com https://stason.org https://stasosphere.com/experience-life/my-books
Re: git diff-tree ignores --textconv
On Sun, Sep 23, 2018 at 05:56:03PM -0700, Stas Bekman wrote: > > You probably want "--ext-diff", not "--textconv". > [...] > Would it be safe to ask the maintainer of the application to include > both --textconv and --ext-diff in that 'git diff-tree' call? I only need > the latter, but someone needed --textconv there as it's in the code. I think so. The main reason that they are not the default for plumbing commands such as diff-tree is that the output may be quite surprising to anything trying to parse the output. Using --textconv will always produce a diff, but one that may not be applied to the original content. Using --ext-diff may produce output that doesn't even look like a diff, though in practice they often do. > This is for this package: > https://github.com/rsmmr/git-notifier It looks like the output is meant to be read by humans, so yeah, I think it would be fine (and preferred) to enable both. -Peff
Re: git diff-tree ignores --textconv
On 2018-09-23 05:43 PM, Jeff King wrote: > On Sun, Sep 23, 2018 at 03:41:45PM -0700, Stas Bekman wrote: > >> $ git config --get diff.jupyternotebook.command >> git-nbdiffdriver diff > > That's an "external diff driver", not a textconv driver. > > So here: > >> $ GIT_TRACE=1 git diff-tree -p HEAD --textconv test/test.ipynb >> > > You probably want "--ext-diff", not "--textconv". > > There's some discussion in the gitattributes manpage, but the short of > it is that textconv converts binary input to text, which is then fed > through the normal diff mechanism. Whereas an external diff driver is > given both sides and can produce whatever output it wants. Textconv is > less flexible, but generally way easier to write. Thank you, Jeff, for explaining my misunderstanding and how to fix it. Would it be safe to ask the maintainer of the application to include both --textconv and --ext-diff in that 'git diff-tree' call? I only need the latter, but someone needed --textconv there as it's in the code. This is for this package: https://github.com/rsmmr/git-notifier It was added here: https://github.com/rsmmr/git-notifier/search?q=textconv&unscoped_q=textconv -- Stas Bekman <'>< <'>< https://stasosphere.com https://chestofbooks.com https://experientialsexlab.com https://stason.org https://stasosphere.com/experience-life/my-books
Re: git diff-tree ignores --textconv
On Sun, Sep 23, 2018 at 03:41:45PM -0700, Stas Bekman wrote: > $ git config --get diff.jupyternotebook.command > git-nbdiffdriver diff That's an "external diff driver", not a textconv driver. So here: > $ GIT_TRACE=1 git diff-tree -p HEAD --textconv test/test.ipynb > You probably want "--ext-diff", not "--textconv". There's some discussion in the gitattributes manpage, but the short of it is that textconv converts binary input to text, which is then fed through the normal diff mechanism. Whereas an external diff driver is given both sides and can produce whatever output it wants. Textconv is less flexible, but generally way easier to write. -Peff
Re: Git diff --no-index --no-prefix output loses leading slash in paths
This is a feature request; sorry for the confusion. My guess is that it's a corner case that was not considered due to the default prefixing. > On Jun 18, 2018, at 10:59 AM, Duy Nguyen wrote: > > On Mon, Jun 18, 2018 at 4:36 PM George King wrote: >> >> As of 2.17.1, `git diff --no-index --no-prefix relative/path /absolute/path` >> produces the following: > > I checked as far back as v1.4.0 and git behaved the same way too. What > version did it work for you? Or is this not a regression, rather a > feature request? > >> diff --git relative/path absolute/path >> index XXX..YYY ZZ >> --- relative/path >> +++ absolute/path >> >> The leading slash on `absolute/path` is lost. This is unfortunate; my use >> case is a diff highlighter that parses and reformats paths so that code >> editors can autodetect them and link to the files. >> >> Would the maintainers please consider fixing the output to preserve absolute >> paths? >> >> Thank you, >> George King >> > -- > Duy
Re: Git diff --no-index --no-prefix output loses leading slash in paths
On Mon, Jun 18, 2018 at 4:36 PM George King wrote: > > As of 2.17.1, `git diff --no-index --no-prefix relative/path /absolute/path` > produces the following: I checked as far back as v1.4.0 and git behaved the same way too. What version did it work for you? Or is this not a regression, rather a feature request? > diff --git relative/path absolute/path > index XXX..YYY ZZ > --- relative/path > +++ absolute/path > > The leading slash on `absolute/path` is lost. This is unfortunate; my use > case is a diff highlighter that parses and reformats paths so that code > editors can autodetect them and link to the files. > > Would the maintainers please consider fixing the output to preserve absolute > paths? > > Thank you, > George King > -- Duy
Re: git diff: meaning of ^M at line ends ?
On 15.05.18 20:04, Frank Schäfer wrote: > Am 14.05.2018 um 20:13 schrieb Torsten Bögershausen: >> ^M is the representation of a "Carriage Return" or CR. >> Under Linux/Unix/Mac OS X a line is terminated with a single >> "line feed", LF. >> >> Windows typically uses CRLF at the end of the line. >> "git diff" uses the LF to detect the end of line, >> leaving the CR alone. >> >> Nothing to worry about. > Thanks, I already suspected something like that. > Has this behavior been changed/added recently ? That is a good question. There is, to my knowledge, no intentional change. > I didn't observe it before, although the project I'm currently looking > into has always been using CR+LF... Do you mean that older versions did behave differently ? Do you have a version number for the "old" handling ? > > Why does the ^M only show up in '+' lines ? > When changing the line end from CR+LF to LF, the diff looks like this: > > -blahblah > +blahblah > > But I would expect it to be > > -blahblah^M > +blahblah May be this helps (I haven't tested it) ? git config core.whitespace cr-at-eol
Re: git diff: meaning of ^M at line ends ?
Am 14.05.2018 um 20:13 schrieb Torsten Bögershausen: > ^M is the representation of a "Carriage Return" or CR. > Under Linux/Unix/Mac OS X a line is terminated with a single > "line feed", LF. > > Windows typically uses CRLF at the end of the line. > "git diff" uses the LF to detect the end of line, > leaving the CR alone. > > Nothing to worry about. Thanks, I already suspected something like that. Has this behavior been changed/added recently ? I didn't observe it before, although the project I'm currently looking into has always been using CR+LF... Why does the ^M only show up in '+' lines ? When changing the line end from CR+LF to LF, the diff looks like this: -blahblah +blahblah But I would expect it to be -blahblah^M +blahblah Regards, Frank > If you want, you can commit those files with > CRLF in the working tree, and LF in the repo. > > More information may be found here: > > https://git-scm.com/docs/gitattributes > > (Or ask more questions here, if needed)
Re: git diff: meaning of ^M at line ends ?
On 14.05.18 18:08, Frank Schäfer wrote: > What does ^M at the end of lines in the output of 'git diff' mean ? > > Thanks, > Frank > ^M is the representation of a "Carriage Return" or CR. Under Linux/Unix/Mac OS X a line is terminated with a single "line feed", LF. Windows typically uses CRLF at the end of the line. "git diff" uses the LF to detect the end of line, leaving the CR alone. Nothing to worry about. If you want, you can commit those files with CRLF in the working tree, and LF in the repo. More information may be found here: https://git-scm.com/docs/gitattributes (Or ask more questions here, if needed)
Re: Git diff --no-index and file descriptor inputs
On Thu, 2017-09-07 at 15:55 -0400, Jason Pyeron wrote: > > -Original Message- > > From: Martin Ågren > > Sent: Thursday, September 7, 2017 1:48 PM > > > > On 7 September 2017 at 18:00, Jason Pyeron wrote: > > > > > > I was hoping to leverage --word-diff-regex, but the > > > > --no-index option > > > does not seem to work with <(...) notation. > > > > > > I am I doing something wrong or is this a bug? > > > > There were some patches floating around half a year ago, I > > don't know what happened to them. > > > > From: Dennis Kaarsemaker > > Sent: Friday, January 13, 2017 5:20 AM > > Subject: [PATCH v2 0/2] diff --no-index: support symlinks and pipes > > https://public-inbox.org/git/20170324213110.4331-1-den...@kaarsemaker.net/ > > I see, it goes back to 2016... > > > From: Dennis Kaarsemaker > > Subject: [RFC/PATCH 0/2] git diff <(command1) <(command2) > > Date: Fri, 11 Nov 2016 21:19:56 +0100 > > https://public-inbox.org/git/2016201958.2175-1-den...@kaarsemaker.net/ > > I will read up on those threads weekend, then try to apply the patches and > see what happens. > > Thanks for the google fu help. I hope to send a new version of this soon. I had almost no time to do anything git related in the last year, trying to catch up with mailing list traffic now. D.
Re: git diff --name-status for deleted files
Gene Thomas writes: > Junio, >Thanks for your reply. So git is essentially doing a >"git commit" when I "git rm". No. You'd probably need to read a bit more on Git; unlike other systems like CVS and SVN, where you only have two states (i.e. committed contents vs files on the filesystem), we have three states (i.e. the index in addition to the above two). "git add file" and "git rm file" make the index match what's on the filesystem wrt "file". They never touch committed contents, which "git commit" command is about.
RE: git diff --name-status for deleted files
Junio, Thanks for your reply. So git is essentially doing a "git commit" when I "git rm". Gene. -Original Message- From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Friday, 15 September 2017 2:58 PM To: Gene Thomas Cc: git@vger.kernel.org Subject: Re: git diff --name-status for deleted files Gene Thomas writes: > Hello, > "git diff -name-status" is useful to list the files one > has changed but it does not list file that one has > deleted with "git rm". It would be really handy if it > did. I am using git 2.9.3 on Ubuntu Linux 16.10. With or without --name-status option, "git diff" compares between the contents you have in the index and in your working tree. After you modify contents of a file, i.e. edit file git add file you would not see that file edited exactly because the file on the filesystem is identical to what you added to the index with "git add". Your example works exactly the same way. Instead of modifying the contents of a file, removing the presense of the file and recording that fact to the index (i.e. removing the path from the index, too) is done with "git rm", so after running git rm file your working tree would lack "file" and so would your index. Hence you wouldn't see "git diff" report any difference on "file". Perhaps you wanted "git diff HEAD", which is a way to compare between the contents you have in the tip commit and the paths in your working tree thru the index?
Re: git diff --name-status for deleted files
Gene Thomas writes: > Hello, > "git diff -name-status" is useful to list the files one > has changed but it does not list file that one has > deleted with "git rm". It would be really handy if it > did. I am using git 2.9.3 on Ubuntu Linux 16.10. With or without --name-status option, "git diff" compares between the contents you have in the index and in your working tree. After you modify contents of a file, i.e. edit file git add file you would not see that file edited exactly because the file on the filesystem is identical to what you added to the index with "git add". Your example works exactly the same way. Instead of modifying the contents of a file, removing the presense of the file and recording that fact to the index (i.e. removing the path from the index, too) is done with "git rm", so after running git rm file your working tree would lack "file" and so would your index. Hence you wouldn't see "git diff" report any difference on "file". Perhaps you wanted "git diff HEAD", which is a way to compare between the contents you have in the tip commit and the paths in your working tree thru the index?
Re: git diff doesn't quite work as documented?
2017-09-08 0:31 GMT+08:00 Olaf Klischat : > oklischat@oklischat:/tmp$ mkdir gittest > oklischat@oklischat:/tmp$ cd gittest/ > oklischat@oklischat:/tmp/gittest$ git init > Initialized empty Git repository in /private/tmp/gittest/.git/ > oklischat@oklischat:/tmp/gittest$ echo foo > foo.txt > oklischat@oklischat:/tmp/gittest$ git add foo.txt > oklischat@oklischat:/tmp/gittest$ git commit -m foo > [master (root-commit) 54d55f2] foo > 1 file changed, 1 insertion(+) > create mode 100644 foo.txt > oklischat@oklischat:/tmp/gittest$ echo bar > bar.txt > oklischat@oklischat:/tmp/gittest$ git add bar.txt > oklischat@oklischat:/tmp/gittest$ git commit -m bar > [master 83b2e74] bar > 1 file changed, 1 insertion(+) > create mode 100644 bar.txt > oklischat@oklischat:/tmp/gittest$ git tag bar-added > oklischat@oklischat:/tmp/gittest$ git rm bar.txt > rm 'bar.txt' > oklischat@oklischat:/tmp/gittest$ git commit -m 'rm bar' > [master 3ca4ff9] rm bar > 1 file changed, 1 deletion(-) > delete mode 100644 bar.txt > oklischat@oklischat:/tmp/gittest$ > oklischat@oklischat:/tmp/gittest$ git checkout bar-added -- bar.txt > oklischat@oklischat:/tmp/gittest$ git reset HEAD > oklischat@oklischat:/tmp/gittest$ git status > On branch master > Untracked files: > (use "git add ..." to include in what will be committed) > > bar.txt > > nothing added to commit but untracked files present (use "git add" to track) > oklischat@oklischat:/tmp/gittest$ git diff bar-added # would expect this to > show no differences > diff --git a/bar.txt b/bar.txt > deleted file mode 100644 > index 5716ca5..000 > --- a/bar.txt > +++ /dev/null > @@ -1 +0,0 @@ > -bar > oklischat@oklischat:/tmp/gittest$ > oklischat@oklischat:/tmp/gittest$ git diff bar-added -- bar.txt # dito > diff --git a/bar.txt b/bar.txt > deleted file mode 100644 > index 5716ca5..000 > --- a/bar.txt > +++ /dev/null > @@ -1 +0,0 @@ > -bar Michael J Gruber is correct about the working-dir/working-tree things. A quick example: add a new file with $ echo bzz > bzz.txt then do "git diff bar-added". The result is the same, because "bzz.txt" is not in the working-tree. Yubin > oklischat@oklischat:/tmp/gittest$ > > > `git diff --help' says: > > git diff [--options] [--] [...] >This form is to view the changes you have in your working tree > relative to the named . > > If that were entirely true, the last two commands shouldn't have shown > any differences, right? > > On closer inspection, it seems that what `git diff ' really > does is take only those paths in the working directory that are also > in and compare the resulting tree against . > > We should add some option to that git diff form to make it really do > what the docs claim it does. > > Or am I missing something?
Re: git diff doesn't quite work as documented?
Junio C Hamano venit, vidit, dixit 08.09.2017 03:26: > Olaf Klischat writes: > >> `git diff --help' says: >> >> git diff [--options] [--] [...] >>This form is to view the changes you have in your >>working tree relative to the named . > > That help text is poorly phrased. > > When "git diff" talks about files in your working tree, it always > looks them _through_ the index. As far as the command is concerned, > a cruft left in your working tree that is not in the index does > *not* exist. > > So when your index does not have bar.txt, even if you have an > untracked bar.txt in your directory, i.e. > >> oklischat@oklischat:/tmp/gittest$ git status >> On branch master >> Untracked files: >> (use "git add ..." to include in what will be committed) >> >> bar.txt > > and you have a commit that _has_ that file, then the command thinks > has the path, and your working tree does *not*. IOW, this > is... > >> oklischat@oklischat:/tmp/gittest$ git diff bar-added >> diff --git a/bar.txt b/bar.txt >> deleted file mode 100644 > > ... totally expected and intended output. > > Hope the above explanation clarifies. A documentation update might > be helpful to new users. Well, there's a difference between "working tree" and "working dir". The wt is "the tree of actual checked out files" per our glossary. So maybe the doc could point to the glossary (see the glossary for the difference to the work dir). But really, this type of misunderstandings comes up often: people try to understand the doc based on common language terms (which is okay, of course) and are unaware of the defined meanings of technical terms. Explaining them in every place where they are used simply does not scale. Maybe we should make more use of our glossary (extend it, enhance it, promote it) and somehow mark all technical terms as such when they are used (say, italics in HTML), or at least when the exact meaning is relevant like in the case above, and possibly link to the glossary (-item). Michael
Re: git diff doesn't quite work as documented?
Olaf Klischat writes: > `git diff --help' says: > > git diff [--options] [--] [...] >This form is to view the changes you have in your >working tree relative to the named . That help text is poorly phrased. When "git diff" talks about files in your working tree, it always looks them _through_ the index. As far as the command is concerned, a cruft left in your working tree that is not in the index does *not* exist. So when your index does not have bar.txt, even if you have an untracked bar.txt in your directory, i.e. > oklischat@oklischat:/tmp/gittest$ git status > On branch master > Untracked files: > (use "git add ..." to include in what will be committed) > > bar.txt and you have a commit that _has_ that file, then the command thinks has the path, and your working tree does *not*. IOW, this is... > oklischat@oklischat:/tmp/gittest$ git diff bar-added > diff --git a/bar.txt b/bar.txt > deleted file mode 100644 ... totally expected and intended output. Hope the above explanation clarifies. A documentation update might be helpful to new users. Thanks.
RE: Git diff --no-index and file descriptor inputs
> -Original Message- > From: Martin Ågren > Sent: Thursday, September 7, 2017 1:48 PM > > On 7 September 2017 at 18:00, Jason Pyeron wrote: > > > > I was hoping to leverage --word-diff-regex, but the > --no-index option > > does not seem to work with <(...) notation. > > > > I am I doing something wrong or is this a bug? > > There were some patches floating around half a year ago, I > don't know what happened to them. > | From: Dennis Kaarsemaker | Sent: Friday, January 13, 2017 5:20 AM | Subject: [PATCH v2 0/2] diff --no-index: support symlinks and pipes > https://public-inbox.org/git/20170324213110.4331-1-den...@kaarsemaker.net/ I see, it goes back to 2016... | From: Dennis Kaarsemaker | Subject: [RFC/PATCH 0/2] git diff <(command1) <(command2) | Date: Fri, 11 Nov 2016 21:19:56 +0100 https://public-inbox.org/git/2016201958.2175-1-den...@kaarsemaker.net/ I will read up on those threads weekend, then try to apply the patches and see what happens. Thanks for the google fu help. -Jason -- -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- - - - Jason Pyeron PD Inc. http://www.pdinc.us - - Principal Consultant 10 West 24th Street #100- - +1 (443) 269-1555 x333Baltimore, Maryland 21218 - - - -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Re: Git diff --no-index and file descriptor inputs
On 7 September 2017 at 18:00, Jason Pyeron wrote: > > I was hoping to leverage --word-diff-regex, but the --no-index option does > not seem to work with <(...) notation. > > I am I doing something wrong or is this a bug? There were some patches floating around half a year ago, I don't know what happened to them. https://public-inbox.org/git/20170324213110.4331-1-den...@kaarsemaker.net/ > -Jason (reply to list please) Martin (not CC-ing the OP, though I'm not sure why they'd want that -- it's usually the other way round)
Re: git diff sometimes brings up buggy pager
Any chance you can tell us what repo this happens on? + git version, OS, and what the triggering diff invocation is. On Thu, Jun 15, 2017 at 12:19 PM, Matthew Groth wrote: > When I do `git diff` sometimes I get this: > > > ...skipping... > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ...skipping... > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ...skipping... > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > ~ > > > it goes on like this for about 10 times the length. Looks like this > happens exclusively when I use git diff with a github remote that is at the > same commit. I will update if I find any other case where this happens. > > > >
[PATCH 0/2] Re: git diff --submodule=diff fails with submodules in a submodule
I came up with a patch that fixes the issue locally. (It is the first patch, such that we can track the first patch down to maint) While doing so, I noticed 2 issues: * when having nested submodules, we probably want to have --submodule=diff to recurse into the nested submodules, so pass on the option. (2nd patch) * the tests in t4060 hardcode hashes literally. I do not have a patch for that yet, but I will send patches later. (c.f. "sanitize_output" in t4202) Thanks, Stefan Stefan Beller (2): diff: submodule inline diff to initialize env array. diff: recurse into nested submodules for inline diff submodule.c | 4 +- t/t4060-diff-submodule-option-diff-format.sh | 70 2 files changed, 73 insertions(+), 1 deletion(-) -- 2.12.2.576.g7be6e4ba40.dirty
Re: git diff --submodule=diff fails with submodules in a submodule
On Fri, Mar 31, 2017 at 10:07 AM, Stefan Beller wrote: > +cc Jacob, who implemented --submodule=diff > > On Fri, Mar 31, 2017 at 8:40 AM, David Parrish wrote: >> When I try to run `git diff --submodule=diff` in a submodule which has >> it's own submodules that have changes I get the error: fatal: bad >> object > > Thanks for the bug report! > >> Let me know if you need an example reproduce the issue. > > I could reproduce it when playing around locally with a submodule in > submodules. I think sub-submodule needs to have its HEAD moved from > the recorded commit. > > Thanks, > Stefan Hmm. An example reproduction would be helpful. Ideally in the form of a test ;) But otherwise whatever helps. I will try to look at this, but I'm busy for a few days. Thanks, Jake
Re: git diff --submodule=diff fails with submodules in a submodule
+cc Jacob, who implemented --submodule=diff On Fri, Mar 31, 2017 at 8:40 AM, David Parrish wrote: > When I try to run `git diff --submodule=diff` in a submodule which has > it's own submodules that have changes I get the error: fatal: bad > object Thanks for the bug report! > Let me know if you need an example reproduce the issue. I could reproduce it when playing around locally with a submodule in submodules. I think sub-submodule needs to have its HEAD moved from the recorded commit. Thanks, Stefan
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
Torsten Bögershausen writes: > On 2017-02-27 21:17, Junio C Hamano wrote: > >> Torsten, you've been quite active in fixing various glitches around >> the EOL conversion in the latter half of last year. Have any >> thoughts to share on this topic? >> >> Thanks. > > Sorry for the delay, being too busy with other things. > I followed the discussion, but didn't have good things to contribute. > I am not an expert in diff.c, but there seems to be a bug, thanks everybody > for digging. > > Back to business: > > My understanding is that git diff --quiet should be quiet, when > git add will not do anything. Yes, I think that is a sensible criterion. What I was interested to hear from you the most was to double check if Mike's expectation is reasonable. Earlier we had a lengthy discussion on what to do when convert-to-git and convert-to-working-tree conversions do not round trip, and I was wondering if this was one of those cases.
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
On 2017-02-27 21:17, Junio C Hamano wrote: > Torsten, you've been quite active in fixing various glitches around > the EOL conversion in the latter half of last year. Have any > thoughts to share on this topic? > > Thanks. Sorry for the delay, being too busy with other things. I followed the discussion, but didn't have good things to contribute. I am not an expert in diff.c, but there seems to be a bug, thanks everybody for digging. Back to business: My understanding is that git diff --quiet should be quiet, when git add will not do anything (but the file is "touched". The touched means that Git will detect e.g a new mtime or inode or file size when doing lstat(). mtime is tricky, inodes are problematic under Windows. What is easy to change is the file length. I don't thing that we need a test file with LF, nor do we need the sleep, touch or anything. Would the the following work ? (This is copy-paste, so the TABs may be corrupted) #!/bin/sh # # Copyright (c) 2017 Mike Crowe # # These tests ensure that files changing line endings in the presence # of .gitattributes to indicate that line endings should be ignored # don't cause 'git diff' or 'git diff --quiet' to think that they have # been changed. test_description='git diff with files that require CRLF conversion' . ./test-lib.sh test_expect_success setup ' echo "* text=auto" > .gitattributes && printf "Hello\r\nWorld\r\n" >crlf.txt && git add .gitattributes crlf.txt lf.txt && git commit -m "initial" ' test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' ' printf "Hello\r\nWorld\n" >crlf.txt && git status && git diff --quiet ' test_done
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
Torsten, you've been quite active in fixing various glitches around the EOL conversion in the latter half of last year. Have any thoughts to share on this topic? Thanks. Mike Crowe writes: > On Monday 20 February 2017 at 13:25:01 -0800, Junio C Hamano wrote: >> This almost makes me suspect that the place that checks lengths of >> one and two in order to refrain from running more expensive content >> comparison you found earlier need to ask would_convert_to_git() >> before taking the short-cut, something along the lines of this (for >> illustration purposes only, not even compile-tested). The "almost" >> comes to me because I do not offhand know the performance implications >> of making calls to would_convert_to_git() here. >> >> diff.c | 18 ++ >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/diff.c b/diff.c >> index 051761be40..094d5913da 100644 >> --- a/diff.c >> +++ b/diff.c >> @@ -4921,9 +4921,10 @@ static int diff_filespec_check_stat_unmatch(struct >> diff_filepair *p) >> *differences. >> * >> * 2. At this point, the file is known to be modified, >> - *with the same mode and size, and the object >> - *name of one side is unknown. Need to inspect >> - *the identical contents. >> + *with the same mode and size, the object >> + *name of one side is unknown, or size comparison >> + *cannot be depended upon. Need to inspect the >> + *contents. >> */ >> if (!DIFF_FILE_VALID(p->one) || /* (1) */ >> !DIFF_FILE_VALID(p->two) || >> @@ -4931,7 +4932,16 @@ static int diff_filespec_check_stat_unmatch(struct >> diff_filepair *p) >> (p->one->mode != p->two->mode) || >> diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || >> diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || >> -(p->one->size != p->two->size) || >> + >> +/* >> + * only if eol and other conversions are not involved, >> + * we can say that two contents of different sizes >> + * cannot be the same without checking their contents. >> + */ >> +(!would_convert_to_git(p->one->path) && >> + !would_convert_to_git(p->two->path) && >> + (p->one->size != p->two->size)) || >> + >> !diff_filespec_is_identical(p->one, p->two)) /* (2) */ >> p->skip_stat_unmatch_result = 1; >> return p->skip_stat_unmatch_result; >> >> > > Thanks for investigating this. I think you are correct that I was misguided > in my previous "fix". However, your change above does fix the problem for > me. > > It looks like the main cost of convert_to_git is in convert_attrs which > ends up doing various path operations in attr.c. After that, both > apply_filter and crlf_to_git return straight away if there's nothing to do. > > I experimented several times with running "git diff -quiet" after touching > every file in Git's own worktree and any difference in total time was lost > in the noise. > > I've further improved my test case. Tests 3 and 4 fail without the above > change but pass with it. Unfortunately I'm still unable to get those tests > to fail without the above fix unless the sleeps are present. I've tried > using the "touch -r .datetime" technique from racy-git.txt but it doesn't > help. It seems that I'm unable to stop Git from using its cache without > sleeping. :( > > diff --git a/t/t4063-diff-converted.sh b/t/t4063-diff-converted.sh > new file mode 100755 > index 000..31a730d > --- /dev/null > +++ b/t/t4063-diff-converted.sh > @@ -0,0 +1,44 @@ > +#!/bin/sh > +# > +# Copyright (c) 2017 Mike Crowe > +# > +# These tests ensure that files changing line endings in the presence > +# of .gitattributes to indicate that line endings should be ignored > +# don't cause 'git diff' or 'git diff --quiet' to think that they have > +# been changed. > +# > +# The sleeps are necessary to reproduce the problem for reasons that I > +# don't understand. > + > +test_description='git diff with files that require CRLF conversion' > + > +. ./test-lib.sh > + > +test_expect_success setup ' > + echo "* text=auto" > .gitattributes && > + printf "Hello\r\nWorld\r\n" > crlf.txt && > + printf "Hello\nWorld\n" > lf.txt && > + git add .gitattributes crlf.txt lf.txt && > + git commit -m "initial" && echo three > +' > +test_expect_success 'noisy diff works on file that requires CRLF conversion' > ' > + git status >/dev/null && > + git diff >/dev/null && > + sleep 1 && > + touch crlf.txt lf.txt && > + git diff >/dev/null > +' > +test_expect_success 'quiet diff works on file that requires CRLF conversion > with no changes' ' > + git status && > + git diff --quiet && > + sleep 1 && > + touch crlf.txt lf.txt && > + git diff --quiet > +' > + > +test_expect_success 'quiet diff works on file with line-ending change that > has no effect on repository' ' > + printf "Hello\nWorld\n" >
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
On Monday 20 February 2017 at 13:25:01 -0800, Junio C Hamano wrote: > This almost makes me suspect that the place that checks lengths of > one and two in order to refrain from running more expensive content > comparison you found earlier need to ask would_convert_to_git() > before taking the short-cut, something along the lines of this (for > illustration purposes only, not even compile-tested). The "almost" > comes to me because I do not offhand know the performance implications > of making calls to would_convert_to_git() here. > > diff.c | 18 ++ > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/diff.c b/diff.c > index 051761be40..094d5913da 100644 > --- a/diff.c > +++ b/diff.c > @@ -4921,9 +4921,10 @@ static int diff_filespec_check_stat_unmatch(struct > diff_filepair *p) >*differences. >* >* 2. At this point, the file is known to be modified, > - *with the same mode and size, and the object > - *name of one side is unknown. Need to inspect > - *the identical contents. > + *with the same mode and size, the object > + *name of one side is unknown, or size comparison > + *cannot be depended upon. Need to inspect the > + *contents. >*/ > if (!DIFF_FILE_VALID(p->one) || /* (1) */ > !DIFF_FILE_VALID(p->two) || > @@ -4931,7 +4932,16 @@ static int diff_filespec_check_stat_unmatch(struct > diff_filepair *p) > (p->one->mode != p->two->mode) || > diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || > diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || > - (p->one->size != p->two->size) || > + > + /* > + * only if eol and other conversions are not involved, > + * we can say that two contents of different sizes > + * cannot be the same without checking their contents. > + */ > + (!would_convert_to_git(p->one->path) && > + !would_convert_to_git(p->two->path) && > + (p->one->size != p->two->size)) || > + > !diff_filespec_is_identical(p->one, p->two)) /* (2) */ > p->skip_stat_unmatch_result = 1; > return p->skip_stat_unmatch_result; > > Thanks for investigating this. I think you are correct that I was misguided in my previous "fix". However, your change above does fix the problem for me. It looks like the main cost of convert_to_git is in convert_attrs which ends up doing various path operations in attr.c. After that, both apply_filter and crlf_to_git return straight away if there's nothing to do. I experimented several times with running "git diff -quiet" after touching every file in Git's own worktree and any difference in total time was lost in the noise. I've further improved my test case. Tests 3 and 4 fail without the above change but pass with it. Unfortunately I'm still unable to get those tests to fail without the above fix unless the sleeps are present. I've tried using the "touch -r .datetime" technique from racy-git.txt but it doesn't help. It seems that I'm unable to stop Git from using its cache without sleeping. :( diff --git a/t/t4063-diff-converted.sh b/t/t4063-diff-converted.sh new file mode 100755 index 000..31a730d --- /dev/null +++ b/t/t4063-diff-converted.sh @@ -0,0 +1,44 @@ +#!/bin/sh +# +# Copyright (c) 2017 Mike Crowe +# +# These tests ensure that files changing line endings in the presence +# of .gitattributes to indicate that line endings should be ignored +# don't cause 'git diff' or 'git diff --quiet' to think that they have +# been changed. +# +# The sleeps are necessary to reproduce the problem for reasons that I +# don't understand. + +test_description='git diff with files that require CRLF conversion' + +. ./test-lib.sh + +test_expect_success setup ' + echo "* text=auto" > .gitattributes && + printf "Hello\r\nWorld\r\n" > crlf.txt && + printf "Hello\nWorld\n" > lf.txt && + git add .gitattributes crlf.txt lf.txt && + git commit -m "initial" && echo three +' +test_expect_success 'noisy diff works on file that requires CRLF conversion' ' + git status >/dev/null && + git diff >/dev/null && + sleep 1 && + touch crlf.txt lf.txt && + git diff >/dev/null +' +test_expect_success 'quiet diff works on file that requires CRLF conversion with no changes' ' + git status && + git diff --quiet && + sleep 1 && + touch crlf.txt lf.txt && + git diff --quiet +' + +test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' ' + printf "Hello\nWorld\n" > crlf.txt && + printf "Hello\r\nWorld\r\n" > lf.txt && + git diff --quiet +' +test_done Mike.
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
Mike Crowe writes:
> I think that if there's a risk that file contents will undergo conversion
> then this should force the diff to check the file contents. Something like:
>
> diff --git a/diff.c b/diff.c
> index 051761b..bee1662 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3413,13 +3413,14 @@ void diff_setup_done(struct diff_options *options)
> /*
>* Most of the time we can say "there are changes"
>* only by checking if there are changed paths, but
> - * --ignore-whitespace* options force us to look
> - * inside contents.
> + * --ignore-whitespace* options or text conversion
> + * force us to look inside contents.
>*/
>
> if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
> DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
> - DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
> + DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL) ||
> + DIFF_OPT_TST(options, ALLOW_TEXTCONV))
> DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
> else
> DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
Thanks.
You may be on the right track to find FROM_CONTENTS bit, but
I think TEXTCONV bit is a red-herring.
This part of diff.c caught my attention, while thinking about this
topic:
if (output_format & DIFF_FORMAT_NO_OUTPUT &&
DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
/*
* run diff_flush_patch for the exit status. setting
* options->file to /dev/null should be safe, because we
* aren't supposed to produce any output anyway.
*/
and the body of this "if" statement loops over q->queue[]. It is
about the "even though we prefer not having to format the patch
because we are doing --quiet, we need to see if contents of one and
two that we _know_ are different are made into the same thing when
we compare them while ignoring various forms of whitespace changes".
So one and two that are removed in earlier step because they were
truly identical may not be penalized if you flip FROM_CONTENTS bit
early on.
Having said that, DIFF_FROM_CONTENTS is about all paths this options
structure governs, not just paths that have eol conversion defined.
When you say "diff --ignore-whitespace-change", that applies to all
paths. The eol conversion is specified per-path, so ideally the
FROM_CONTENTS bit should be flipped if and only if one or more of
the paths would need the conversion (i.e. does the helper function
would_convert_to_git() say "yes" to the path?). I however suspect
that necessary information to do so (i.e. "which paths we are
looking at?") has not been generated yet at the point of the code
you quoted. setup comes (and must come) very early, and then
q->queue[] is populated by different front-end functions that
compare trees, the index, and the working tree, depending on the
"git diff" option or "git diff-{tree,index,files}" plumbing command,
and you can ask "would one of these paths need conversion?" only
after q->queue[] is populated. Hmm.
Another thing is that ALLOW_TEXTCONV is not a right bit to check for
your example. It is "do we use textconv filters to turn binary
files into a (phony) text representation before comparing?". People
use the mechanism to throw JPEG photos in Git and have textconv
filter to extract only EXIF data, and "diff --textconv" would let us
textually compare only the EXIF data (which is the only human
readable part of the contents anyway).
It might be a good idea to also flip FROM_CONTENTS bit under "diff
--textconv", and ...
> This solves the problem for me and my test case now passes.
... but I suspect that you were misled to think it fixes your issue,
only because "--textconv" is by default enabled for "git diff" and
"git log" (see "git diff --help"). I think you are saying that if
you always set FROM_CONTENTS bit on, you get what you want. But
that is to be expected and it unfortunately penalizes everybody by
turning an obvious optimization permanently off.
Also "--textconv" is not on by default for the plumbing "git
diff-index" command and its friends, so it is likely that "git
diff-index HEAD" with your change will still not work as you expect.
A cheap (from code-change point of view) band-aid might be to flip
FROM_CONTENTS on if we know the repository has _some_ paths that
need eol conversion, even when the particular "diff" we are taking
does not involve any eol conversion (e.g. "is core.crlf set?").
While it may be better than "if we are porcelain (aka ALLOW_TEXTCONV
is set), unconditionally flip FROM_CONTENTS on", it is not ideal,
either.
This almost makes me suspect that the place that checks lengths of
one and two in order to refrain from running more expensive content
comparison you found earlier need to ask would_convert_to_git()
before taking the short-cut, something along the lines of this (for
illustrat
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
On Friday 17 February 2017 at 22:19:58 +, Mike Crowe wrote: > On Friday 17 February 2017 at 14:05:17 -0800, Junio C Hamano wrote: > > Mike Crowe writes: > > > > > If "git diff --quiet" finds it necessary to compare actual file contents, > > > and a file requires CRLF conversion, then it incorrectly exits with an > > > exit > > > code of 1 even if there have been no changes. > > > > > > The patch below adds a test file that shows the problem. > > > > If "git diff" does not show any output and "git diff --exit-code" or > > "git diff --quiet" says there are differences, then it is a bug. > > > > I would however have expected that any culprit would involve a code > > that says "under QUICK option, we do not have to bother doing > > this". The part you quoted: > > > > > if (!DIFF_FILE_VALID(p->one) || /* (1) */ > > > !DIFF_FILE_VALID(p->two) || > > > (p->one->oid_valid && p->two->oid_valid) || > > > (p->one->mode != p->two->mode) || > > > diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || > > > diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || > > > (p->one->size != p->two->size) || > > > !diff_filespec_is_identical(p->one, p->two)) /* (2) */ > > > p->skip_stat_unmatch_result = 1; > > > > is used by "git diff" with and without "--quiet", afacr, so I > > suspect that the bug lies somewhere else. > > I can't say that I really understand the code fully, but it appears that > the first pass generates a queue of files that contain differences. The > result of the quiet diff comes from the size of that queue, > diff_queued_diff.nr, being non-zero in diffcore_std. I'm assuming that the > result of the noisy diff comes from actually comparing the files. > > But, I've only spent a short while looking so I may have got the wrong end > of the stick. Tricking Git into checking the actual file contents (by passing --ignore-space-change for example) is sufficient to ensure that the exit status is never 1 when it should be zero. (Of course that option has other unwanted effects in this case.) I think that if there's a risk that file contents will undergo conversion then this should force the diff to check the file contents. Something like: diff --git a/diff.c b/diff.c index 051761b..bee1662 100644 --- a/diff.c +++ b/diff.c @@ -3413,13 +3413,14 @@ void diff_setup_done(struct diff_options *options) /* * Most of the time we can say "there are changes" * only by checking if there are changed paths, but -* --ignore-whitespace* options force us to look -* inside contents. +* --ignore-whitespace* options or text conversion +* force us to look inside contents. */ if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) || DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) || - DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL)) + DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL) || + DIFF_OPT_TST(options, ALLOW_TEXTCONV)) DIFF_OPT_SET(options, DIFF_FROM_CONTENTS); else DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS); This solves the problem for me and my test case now passes. Unfortunately it breaks the 'removing and adding subproject' test case in t3040-subprojects-basic at the line: test_expect_code 1 git diff -M --name-status --exit-code HEAD^ HEAD presumably because after the rename has been detected the file contents are identical. :( A rename of a single file appears to still be handled correctly. Mike.
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
On Friday 17 February 2017 at 14:05:17 -0800, Junio C Hamano wrote: > Mike Crowe writes: > > > If "git diff --quiet" finds it necessary to compare actual file contents, > > and a file requires CRLF conversion, then it incorrectly exits with an exit > > code of 1 even if there have been no changes. > > > > The patch below adds a test file that shows the problem. > > If "git diff" does not show any output and "git diff --exit-code" or > "git diff --quiet" says there are differences, then it is a bug. > > I would however have expected that any culprit would involve a code > that says "under QUICK option, we do not have to bother doing > this". The part you quoted: > > > if (!DIFF_FILE_VALID(p->one) || /* (1) */ > > !DIFF_FILE_VALID(p->two) || > > (p->one->oid_valid && p->two->oid_valid) || > > (p->one->mode != p->two->mode) || > > diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || > > diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || > > (p->one->size != p->two->size) || > > !diff_filespec_is_identical(p->one, p->two)) /* (2) */ > > p->skip_stat_unmatch_result = 1; > > is used by "git diff" with and without "--quiet", afacr, so I > suspect that the bug lies somewhere else. I can't say that I really understand the code fully, but it appears that the first pass generates a queue of files that contain differences. The result of the quiet diff comes from the size of that queue, diff_queued_diff.nr, being non-zero in diffcore_std. I'm assuming that the result of the noisy diff comes from actually comparing the files. But, I've only spent a short while looking so I may have got the wrong end of the stick. Thanks. Mike.
Re: git diff --quiet exits with 1 on clean tree with CRLF conversions
Mike Crowe writes: > If "git diff --quiet" finds it necessary to compare actual file contents, > and a file requires CRLF conversion, then it incorrectly exits with an exit > code of 1 even if there have been no changes. > > The patch below adds a test file that shows the problem. If "git diff" does not show any output and "git diff --exit-code" or "git diff --quiet" says there are differences, then it is a bug. I would however have expected that any culprit would involve a code that says "under QUICK option, we do not have to bother doing this". The part you quoted: > if (!DIFF_FILE_VALID(p->one) || /* (1) */ > !DIFF_FILE_VALID(p->two) || > (p->one->oid_valid && p->two->oid_valid) || > (p->one->mode != p->two->mode) || > diff_populate_filespec(p->one, CHECK_SIZE_ONLY) || > diff_populate_filespec(p->two, CHECK_SIZE_ONLY) || > (p->one->size != p->two->size) || > !diff_filespec_is_identical(p->one, p->two)) /* (2) */ > p->skip_stat_unmatch_result = 1; is used by "git diff" with and without "--quiet", afacr, so I suspect that the bug lies somewhere else.
Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
Jeff King writes: > On Wed, Jan 18, 2017 at 12:57:12PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > So I dunno. A sensible rule to me is "iff -p would show a diff header, >> > then --stat should mention it". >> >> True but tricky (you need a better definition of "a diff header"). >> >> In addition to a new and deleted file, does a file whose executable >> bit was flipped need mention? If so, then "diff --git" is the diff >> header in the above. Otherwise "@@ ... @@", iow, "iff -p would show >> any hunk". >> >> I think the patch implements the latter, which I think is sensible. > > I would think the former is more sensible (and is what my patch is > working towards). Doh (yes, "diff --git" was what I meant). As a mode-flipping patch does not have hunk but does show the header, it wants to be included in "git diff --stat" output, I would think, independent of -b issue. In fact chmod +x README.md git diff --stat does show a 0-line diffstat. > Doing: > > >empty > git add empty > git diff --cached > > shows a "diff --git" header, but no hunk. I think it should show a > diffstat (and does with my patch). > > I was thinking the rule should be something like: > > if (p->status == DIFF_STATUS_MODIFIED && > !file->added && !file->deleted)) > > and otherwise include the entry, since it would be an add, delete, > rename, etc, which carries useful information. > > Though a pure rename would not hit this code path at all, I would think > (it would not trigger "!same_contents"). And a rename where there was a > whitespace only change probably _should_ be omitted from "-b". > > Ditto for a pure mode change, I think. We do not run the contents > through diff at all, so it does not hit this code path. > > -Peff
Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
Jeff King writes:
> So I dunno. A sensible rule to me is "iff -p would show a diff header,
> then --stat should mention it".
True but tricky (you need a better definition of "a diff header").
In addition to a new and deleted file, does a file whose executable
bit was flipped need mention? If so, then "diff --git" is the diff
header in the above. Otherwise "@@ ... @@", iow, "iff -p would show
any hunk".
I think the patch implements the latter, which I think is sensible.
> + /*
> + * Omit diffstats where nothing changed. Even if
> + * !same_contents, this might be the case due to ignoring
> + * whitespace changes, etc.
> + *
> + * But note that we special-case additions and deletions,
> + * as adding an empty file, for example, is still of interest.
> + */
> + if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) {
> + struct diffstat_file *file =
> + diffstat->files[diffstat->nr - 1];
> + if (!file->added && !file->deleted) {
> + free_diffstat_file(file);
> + diffstat->nr--;
> + }
> + }
> }
Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
On Wed, Jan 18, 2017 at 12:57:12PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > So I dunno. A sensible rule to me is "iff -p would show a diff header, > > then --stat should mention it". > > True but tricky (you need a better definition of "a diff header"). > > In addition to a new and deleted file, does a file whose executable > bit was flipped need mention? If so, then "diff --git" is the diff > header in the above. Otherwise "@@ ... @@", iow, "iff -p would show > any hunk". > > I think the patch implements the latter, which I think is sensible. I would think the former is more sensible (and is what my patch is working towards). Doing: >empty git add empty git diff --cached shows a "diff --git" header, but no hunk. I think it should show a diffstat (and does with my patch). I was thinking the rule should be something like: if (p->status == DIFF_STATUS_MODIFIED && !file->added && !file->deleted)) and otherwise include the entry, since it would be an add, delete, rename, etc, which carries useful information. Though a pure rename would not hit this code path at all, I would think (it would not trigger "!same_contents"). And a rename where there was a whitespace only change probably _should_ be omitted from "-b". Ditto for a pure mode change, I think. We do not run the contents through diff at all, so it does not hit this code path. -Peff
Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"
On Tue, Jan 17, 2017 at 09:01:55PM -0500, Matt McCutchen wrote:
> A bug report: I noticed that "git diff --ignore-space-change --stat"
> lists files with only whitespace differences as having changed with 0
> differing lines. This is inconsistent with the behavior without --
> stat, which doesn't list such files at all. (Same behavior with all
> the --ignore*space* flags.) I can reproduce this with the current
> "next", af746e4. Quick test case:
Hmm. This is pretty easy to do naively, but the special-casing for
addition/deletion (which I think we _do_ need, and which certainly we
fail t4205 without) makes me feel dirty. I'd worry there are other
cases, too (perhaps renames?). And I also notice that the
binary-diffstat code path just above my changes explicitly creates 0/0
diffstats, but I'm not even sure how one would trigger that.
So I dunno. A sensible rule to me is "iff -p would show a diff header,
then --stat should mention it". I think we'd want to somehow extract the
logic from builtin_diff() and reuse it.
---
diff --git a/diff.c b/diff.c
index e2eb6d66a..57ff5c1dc 100644
--- a/diff.c
+++ b/diff.c
@@ -2105,17 +2105,20 @@ static void show_dirstat_by_line(struct diffstat_t
*data, struct diff_options *o
gather_dirstat(options, &dir, changed, "", 0);
}
+static void free_diffstat_file(struct diffstat_file *f)
+{
+ if (f->name != f->print_name)
+ free(f->print_name);
+ free(f->name);
+ free(f->from_name);
+ free(f);
+}
+
static void free_diffstat_info(struct diffstat_t *diffstat)
{
int i;
- for (i = 0; i < diffstat->nr; i++) {
- struct diffstat_file *f = diffstat->files[i];
- if (f->name != f->print_name)
- free(f->print_name);
- free(f->name);
- free(f->from_name);
- free(f);
- }
+ for (i = 0; i < diffstat->nr; i++)
+ free_diffstat_file(diffstat->files[i]);
free(diffstat->files);
}
@@ -2603,6 +2606,23 @@ static void builtin_diffstat(const char *name_a, const
char *name_b,
if (xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
&xpp, &xecfg))
die("unable to generate diffstat for %s", one->path);
+
+ /*
+* Omit diffstats where nothing changed. Even if
+* !same_contents, this might be the case due to ignoring
+* whitespace changes, etc.
+*
+* But note that we special-case additions and deletions,
+* as adding an empty file, for example, is still of interest.
+*/
+ if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) {
+ struct diffstat_file *file =
+ diffstat->files[diffstat->nr - 1];
+ if (!file->added && !file->deleted) {
+ free_diffstat_file(file);
+ diffstat->nr--;
+ }
+ }
}
diff_free_filespec_data(one);
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 289806d0c..2805db411 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -736,7 +736,7 @@ test_expect_success 'checkdiff allows new blank lines' '
cat x "hello world" &&
git add x &&
@@ -746,6 +746,12 @@ test_expect_success 'whitespace-only changes not reported'
'
test_cmp expect actual
'
+test_expect_success 'whitespace-only changes not reported (diffstat)' '
+ # reuse state from previous test
+ git diff --stat -b >actual &&
+ test_cmp expect actual
+'
+
cat
Re: git diff
Thanks, Mike. Now I understood. -fuz > Mike Rappazzo hat am 12. Oktober 2016 um 13:06 > geschrieben: > > On Wed, Oct 12, 2016 at 6:50 AM, wrote: > > Hi. > > > > I created a new branch named hotfix from master. > > I switched to the branch, changed 1 file. > > > > Now I want to see the diff from the both using > > > > git diff hotfix master > > > > I do not see any output (difference). > > When I do a git status I see my file with status mofified, not staged for > > commit. > > Since you just created the branch, and did not add any content, there > is no difference to see. A branch is just a pointer to a commit. You > now have two pointers pointing at the same commit. > > If you want to see the difference between your changes and the master > branch, you can omit the first reference: > > git diff master > > When you start adding commits to your hotfix branch, you will be able > to see the diff between that and master with the command that you > gave. However, your arguments may be in the reverse order than what > you expect. You want to specify master first because that is the > mainline branch (I presume). > > When you have several commits on your hotfix branch, you can refer to > older commits to diff against. There are several ways to refer back, > but the simplest is to use a tilde '~' followed by a number to count > back. For example 'hotfix~1' refers to the parent commit on the > hotfix branch. There is a lot in the documentation[1], so take a look > there for more info. > > Good luck. > _Mike > > [1] https://git-scm.com/doc > > > Also, I can see that I am working with the correct branch, hotfix > > > > What am I doing wrong? > > > > -fuz > > On Wed, Oct 12, 2016 at 6:50 AM, wrote: > > Hi. > > > > I created a new branch named hotfix from master. > > I switched to the branch, changed 1 file. > > > > Now I want to see the diff from the both using > > > > git diff hotfix master > > > > I do not see any output (difference). > > When I do a git status I see my file with status mofified, not staged for > > commit. > > Also, I can see that I am working with the correct branch, hotfix > > > > What am I doing wrong? > > > > -fuz >
Re: git diff
On Wed, Oct 12, 2016 at 6:50 AM, wrote: > Hi. > > I created a new branch named hotfix from master. > I switched to the branch, changed 1 file. > > Now I want to see the diff from the both using > > git diff hotfix master > > I do not see any output (difference). > When I do a git status I see my file with status mofified, not staged for > commit. Since you just created the branch, and did not add any content, there is no difference to see. A branch is just a pointer to a commit. You now have two pointers pointing at the same commit. If you want to see the difference between your changes and the master branch, you can omit the first reference: git diff master When you start adding commits to your hotfix branch, you will be able to see the diff between that and master with the command that you gave. However, your arguments may be in the reverse order than what you expect. You want to specify master first because that is the mainline branch (I presume). When you have several commits on your hotfix branch, you can refer to older commits to diff against. There are several ways to refer back, but the simplest is to use a tilde '~' followed by a number to count back. For example 'hotfix~1' refers to the parent commit on the hotfix branch. There is a lot in the documentation[1], so take a look there for more info. Good luck. _Mike [1] https://git-scm.com/doc > Also, I can see that I am working with the correct branch, hotfix > > What am I doing wrong? > > -fuz On Wed, Oct 12, 2016 at 6:50 AM, wrote: > Hi. > > I created a new branch named hotfix from master. > I switched to the branch, changed 1 file. > > Now I want to see the diff from the both using > > git diff hotfix master > > I do not see any output (difference). > When I do a git status I see my file with status mofified, not staged for > commit. > Also, I can see that I am working with the correct branch, hotfix > > What am I doing wrong? > > -fuz
Re: 'git diff-index' doesn't honor the 'diff.algorithm' variable
On 05/15/2016 09:43 PM, Junio C Hamano wrote: I think the paragraph is shared among the "diff" family of commands both plumbing and Porcelain, so I'd say "patches welcome" at this point ;-). I think I've done my part here. It's not like this is a feature request. The script was an illustration of the logic--I am sure elisp is much core capable scripting environment than POSIX shell. Perhaps (setq vc-diff-git-diff-use-histogram t) in ~/.emacs is not too bad ;-) Yes, doing it via a user option is already possible in Emacs. I was concerned that the user has to configure it twice (once for console, two for Emacs), but if you think it's fine, let's leave it at that. The Porcelain "git diff" command is not bound by any promise of stable output and reserves the right to change the default to better support human users. I think the upcoming version of Git turns the diff.renames setting on by default, for example. We might even add a side-by-side diff and make it the default someday. You do not want to be reading these "fancy" output, and you cannot keep updating the invocation of "git diff" by vc-diff with unbounded number options, e.g. --no-side-by-side, that will be added to defeat configuration variables that will be invented in the future. Fair enough. Thanks, Dmitry. -- 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 diff-index' doesn't honor the 'diff.algorithm' variable
Dmitry Gutov writes: > OK, that makes sense. You might want to fix the man page, though, it > says, like the 'git diff' one, "For instance, if you configured > diff.algorithm variable to a non-default value and want to use the > default one, then you have to use --diff-algorithm=default option.". Thanks; I think the paragraph is shared among the "diff" family of commands both plumbing and Porcelain, so I'd say "patches welcome" at this point ;-). > Thanks, but we don't distribute any custom Git porcelains with > Emacs. We usually can't rely on bash being available either. The script was an illustration of the logic--I am sure elisp is much core capable scripting environment than POSIX shell. Perhaps (setq vc-diff-git-diff-use-histogram t) in ~/.emacs is not too bad ;-) > I'll have to see why we using 'git diff-index' there directly. Maybe > we could switch to 'git diff'. I do not think it is a good idea. Depending on how much understanding vc-diff wants to have on what the diff output is saying, it may want to be read and parse parts of the output; an end user who has diff.color=always can easily ruin your day. And no, running "git diff --color=never" is not a solution for that. The Porcelain "git diff" command is not bound by any promise of stable output and reserves the right to change the default to better support human users. I think the upcoming version of Git turns the diff.renames setting on by default, for example. We might even add a side-by-side diff and make it the default someday. You do not want to be reading these "fancy" output, and you cannot keep updating the invocation of "git diff" by vc-diff with unbounded number options, e.g. --no-side-by-side, that will be added to defeat configuration variables that will be invented in the future. -- 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 diff-index' doesn't honor the 'diff.algorithm' variable
Hi Junio, On 05/14/2016 09:40 PM, Junio C Hamano wrote: The variable belongs to UI config, meant for Porcelain "git diff", together with things like "diff.color", "diff.context", etc. OK, that makes sense. You might want to fix the man page, though, it says, like the 'git diff' one, "For instance, if you configured diff.algorithm variable to a non-default value and want to use the default one, then you have to use --diff-algorithm=default option.". A script that calls diff-index, if it wants to honor end-users' UI config variables, is allowed to use 'git config' to read them and turn them into appropriate command line options. e.g. algo=$(git config diff.algorithm) case "$algo" in minimal|histogram|patience) algo=--$algo ;; *) algo= ;; esac ... git diff-index $algo ... other args ... or something like that. Thanks, but we don't distribute any custom Git porcelains with Emacs. We usually can't rely on bash being available either. Doing an extra process call from Emacs for this niche a feature doesn't seem like a great idea either. To clarify, the problem is that `M-x vc-diff' doesn't honor the diff.algorithm option. I'll have to see why we using 'git diff-index' there directly. Maybe we could switch to 'git diff'. -- 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 diff-index' doesn't honor the 'diff.algorithm' variable
Dmitry Gutov writes: > Hi all, > > Subj. ...even though it's explicitly mentioned in the subcommand's man > page. Git version 2.7.4 here. > > To elaborate: > > - Call 'git config --global diff.algorithm histogram'. The variable belongs to UI config, meant for Porcelain "git diff", together with things like "diff.color", "diff.context", etc. As the point of lower-level plumbing commands in the diff family, i.e. diff-files, diff-index and diff-tree, are about giving stable output, which are _not_ affected by random end-user configuration, for scripted use, it is very much deliberate design decision that they ignore the UI config variables. A script that calls diff-index, if it wants to honor end-users' UI config variables, is allowed to use 'git config' to read them and turn them into appropriate command line options. e.g. algo=$(git config diff.algorithm) case "$algo" in minimal|histogram|patience) algo=--$algo ;; *) algo= ;; esac ... git diff-index $algo ... other args ... or something like that. -- 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 diff --exit-code does not honour textconv setting
Junio C Hamano venit, vidit, dixit 06.04.2016 01:16:
> Michael J Gruber writes:
>
>>> The "test" command is used as it does not generate any output on stdout.
>>
>> "test" is a bit of a red herring here since it will receive commands.
>> But your example works even with "true" which ignores its commands and
>> produces no output.
>
> Either sounds strange, as they exit without reading their input at
> all, so the other side of the pipe may get an write error it has to
> handle ;-)
>
>> The description doesn't make it clear whether exit-code refers to
>> the actual diff (foo vs. bar) or to the diff after textconv (empty
>> vs. empty). In any case, "-b" should not make a difference for
>> your example.
>>
>> diff_flush() in diff.c has this piece of code:
>>
>
> It is understandable that "-b" makes difference. The actual
> short-cut happens a bit before the code you quoted.
>
> if (output_format & DIFF_FORMAT_NO_OUTPUT &&
> DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
> DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
> /*
>* run diff_flush_patch for the exit status. setting
>* options->file to /dev/null should be safe, because we
>* aren't supposed to produce any output anyway.
>*/
> if (options->close_file)
> fclose(options->file);
> options->file = fopen("/dev/null", "w");
> if (!options->file)
> die_errno("Could not open /dev/null");
> options->close_file = 1;
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> if (check_pair_status(p))
> diff_flush_patch(p, options);
> if (options->found_changes)
> break;
> }
> }
>
> When running without producing output but we need the exit status,
> unless DIFF_FROM_CONTENTS is set (e.g. "-b" in use), we do not run
> the code that would inspect the blob contents that would have
> produced the actual patch. When DIFF_FROM_CONTENTS is set, we need
> to dig into the blob contents and the body of the above if() gets
> run only to trigger o->found_changes (e.g. in builtin_diff() that
> sets ecbdata.found_changesp to point at o->found_changes before
> calling into the xdiff machinery), but the output is discarded to
> /dev/null.
>
> The textconv filter is an unfortunate beast, in that while some
> paths use one while others don't, so it won't be as simple as "-b"
> that flips DIFF_FROM_CONTENTS to say "We need to inspect blobs for
> ALL FILEPAIRS to see if there is any difference" if we want to keep
> the optimization as much as possible.
>
> Without DIFF_FROM_CONTENTS set, any filepair that point at two
> different blobs that might end up to be the same after applying
> textconv will flip the o->found_changes bit on, and with
> EXIT_WITH_STATUS bit on, we have another optimization to skip
> checking the remainder of the filepairs. So if you want to support
> textconv with QUICK, you would also need to disable that
> optimization by teaching diff_change() to refrain from setting
> HAS_CHANGES bit, i.e.
>
> diff --git a/diff.h b/diff.h
> index 125447b..f6c0c07 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct
> diff_options *opt, void *data)
> #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)
> #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)
> #define DIFF_OPT_RENAME_EMPTY(1 << 8)
> -/* (1 << 9) unused */
> +#define DIFF_OPT_WITH_TEXTCONV (1 << 9)
> #define DIFF_OPT_HAS_CHANGES (1 << 10)
> #define DIFF_OPT_QUICK (1 << 11)
> #define DIFF_OPT_NO_INDEX(1 << 12)
>
> diff --git a/diff.c b/diff.c
> index 4dfe660..0016ad2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5018,6 +5018,11 @@ void diff_change(struct diff_options *options,
> two->dirty_submodule = new_dirty_submodule;
> p = diff_queue(&diff_queued_diff, one, two);
>
> + if (either path one or path two has textconv defined) {
> + DIFF_OPT_SET(options, DIFF_WITH_TEXTCONV);
> + return;
> + }
> +
> if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
> return;
>
> And then in order to keep the optimization, add this to the above
> codepath:
>
> diff --git a/diff.c b/diff.c
> index 4dfe660..7b318cc 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4598,6 +4598,7 @@ void diff_flush(struct diff_options *options)
> int i, output_format = options->output_format;
> int separator = 0;
> int dirstat_by_line = 0;
> + int textconv_hack;
>
> /*
> * Order: raw, stat, summary, patch
Re: git diff --exit-code does not honour textconv setting
Michael J Gruber writes:
>> The "test" command is used as it does not generate any output on stdout.
>
> "test" is a bit of a red herring here since it will receive commands.
> But your example works even with "true" which ignores its commands and
> produces no output.
Either sounds strange, as they exit without reading their input at
all, so the other side of the pipe may get an write error it has to
handle ;-)
> The description doesn't make it clear whether exit-code refers to
> the actual diff (foo vs. bar) or to the diff after textconv (empty
> vs. empty). In any case, "-b" should not make a difference for
> your example.
>
> diff_flush() in diff.c has this piece of code:
>
It is understandable that "-b" makes difference. The actual
short-cut happens a bit before the code you quoted.
if (output_format & DIFF_FORMAT_NO_OUTPUT &&
DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
/*
* run diff_flush_patch for the exit status. setting
* options->file to /dev/null should be safe, because we
* aren't supposed to produce any output anyway.
*/
if (options->close_file)
fclose(options->file);
options->file = fopen("/dev/null", "w");
if (!options->file)
die_errno("Could not open /dev/null");
options->close_file = 1;
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (check_pair_status(p))
diff_flush_patch(p, options);
if (options->found_changes)
break;
}
}
When running without producing output but we need the exit status,
unless DIFF_FROM_CONTENTS is set (e.g. "-b" in use), we do not run
the code that would inspect the blob contents that would have
produced the actual patch. When DIFF_FROM_CONTENTS is set, we need
to dig into the blob contents and the body of the above if() gets
run only to trigger o->found_changes (e.g. in builtin_diff() that
sets ecbdata.found_changesp to point at o->found_changes before
calling into the xdiff machinery), but the output is discarded to
/dev/null.
The textconv filter is an unfortunate beast, in that while some
paths use one while others don't, so it won't be as simple as "-b"
that flips DIFF_FROM_CONTENTS to say "We need to inspect blobs for
ALL FILEPAIRS to see if there is any difference" if we want to keep
the optimization as much as possible.
Without DIFF_FROM_CONTENTS set, any filepair that point at two
different blobs that might end up to be the same after applying
textconv will flip the o->found_changes bit on, and with
EXIT_WITH_STATUS bit on, we have another optimization to skip
checking the remainder of the filepairs. So if you want to support
textconv with QUICK, you would also need to disable that
optimization by teaching diff_change() to refrain from setting
HAS_CHANGES bit, i.e.
diff --git a/diff.h b/diff.h
index 125447b..f6c0c07 100644
--- a/diff.h
+++ b/diff.h
@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct
diff_options *opt, void *data)
#define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)
#define DIFF_OPT_FOLLOW_RENAMES (1 << 7)
#define DIFF_OPT_RENAME_EMPTY(1 << 8)
-/* (1 << 9) unused */
+#define DIFF_OPT_WITH_TEXTCONV (1 << 9)
#define DIFF_OPT_HAS_CHANGES (1 << 10)
#define DIFF_OPT_QUICK (1 << 11)
#define DIFF_OPT_NO_INDEX(1 << 12)
diff --git a/diff.c b/diff.c
index 4dfe660..0016ad2 100644
--- a/diff.c
+++ b/diff.c
@@ -5018,6 +5018,11 @@ void diff_change(struct diff_options *options,
two->dirty_submodule = new_dirty_submodule;
p = diff_queue(&diff_queued_diff, one, two);
+ if (either path one or path two has textconv defined) {
+ DIFF_OPT_SET(options, DIFF_WITH_TEXTCONV);
+ return;
+ }
+
if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
return;
And then in order to keep the optimization, add this to the above
codepath:
diff --git a/diff.c b/diff.c
index 4dfe660..7b318cc 100644
--- a/diff.c
+++ b/diff.c
@@ -4598,6 +4598,7 @@ void diff_flush(struct diff_options *options)
int i, output_format = options->output_format;
int separator = 0;
int dirstat_by_line = 0;
+ int textconv_hack;
/*
* Order: raw, stat, summary, patch
@@ -4652,9 +4653,17 @@ void diff_flush(struct diff_options *options)
separator++;
}
+ /*
+* If there is any path that needs textconv and we haven't
+* found an
Re: git diff --exit-code does not honour textconv setting
Georg Pichler venit, vidit, dixit 20.03.2016 13:43:
> Hi,
>
> I realized that "git diff --exit-code" does not honour textconv settings.
> Maybe this behaviour is desired. It can be partially circumvented by using
> the "-b" flag if one does not care about whitespace changes.
> To reproduce this, create an empty repository and run the following commands:
>
> (I was using git version 2.7.3)
>
> $ git config --add diff.void.textconv test
> $ echo "foo diff=void" >.gitattributes
> $ echo foo >foo
> $ git add . && git commit -m "Init"
> [master (root-commit) 70c39d9] Init
> 2 files changed, 2 insertions(+)
> create mode 100644 .gitattributes
> create mode 100644 foo
> $ echo bar >foo
> $ git status
> On branch master
> Changes not staged for commit:
> (use "git add ..." to update what will be committed)
> (use "git checkout -- ..." to discard changes in working directory)
>
> modified: foo
>
> no changes added to commit (use "git add" and/or "git commit -a")
> $ git diff
> $ git diff --exit-code
> [exits with 1, no output]
> $ git diff --exit-code -b
> [exits with 0, no output]
>
> The "test" command is used as it does not generate any output on stdout.
"test" is a bit of a red herring here since it will receive commands.
But your example works even with "true" which ignores its commands and
produces no output.
> I would expect "git diff --exit-code" to return with exit code 0. If this is
> not desired, it should be clearly stated in the man page,
> that "--exit-code" does not honour the textconv setting, except if "-b" is
> given. Currently this is not clear:
>
>--exit-code
>Make the program exit with codes similar to diff(1). That is, it
> exits
>with 1 if there were differences and 0 means no differences.
>
> Best,
> Georg Pichler
The description doesn't make it clear whether exit-code refers to the
actual diff (foo vs. bar) or to the diff after textconv (empty vs.
empty). In any case, "-b" should not make a difference for your example.
diff_flush() in diff.c has this piece of code:
/*
* Report the content-level differences with HAS_CHANGES;
* diff_addremove/diff_change does not set the bit when
* DIFF_FROM_CONTENTS is in effect (e.g. with -w).
*/
if (DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
if (options->found_changes)
DIFF_OPT_SET(options, HAS_CHANGES);
else
DIFF_OPT_CLR(options, HAS_CHANGES);
}
So it's clear that depending on "-b" (or "-w") or not, it's taking
shortcuts or looking at the textconved diff but I'm not sure where's the
proper place to fix that.
Michael
--
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 diff does not precompose unicode file paths (OS X)
> On 15 Mar 2016, at 06:45, Torsten Bögershausen wrote: > > >I created a test case but git diff exits with 0 if it does not recognize the > >file >path so the test case always succeeds. Can you give me a hint or one > >>example test case? > > The most clean (?) is to compare "git diff" NFC and git diff NFD, they should > give the same result: > for "git diff" something like this would do: > + > +# This will test git diff > +test_expect_success "git diff f.Adiar" ' > + echo "Modified" >f.$Adiarnfd && > + git diff f.$Adiarnfd >expect && > + git diff f.$Adiarnfc >actual && > + git checkout f.$Adiarnfd && > + test_cmp expect actual > +’ Thank you! I had to tweak it a little but it now reproduces the issue and confirms the fix for diff, diff-index, diff-files and diff-tree. I have just sent in the full patch. -- 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 diff does not precompose unicode file paths (OS X)
>I created a test case but git diff exits with 0 if it does not recognize the file >path so the test case always succeeds. Can you give me a hint or one >example test case? The most clean (?) is to compare "git diff" NFC and git diff NFD, they should give the same result: for "git diff" something like this would do: + +# This will test git diff +test_expect_success "git diff f.Adiar" ' + echo "Modified" >f.$Adiarnfd && + git diff f.$Adiarnfd >expect && + git diff f.$Adiarnfc >actual && + git checkout f.$Adiarnfd && + test_cmp expect actual +' HTH /Torsten -- 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 diff does not precompose unicode file paths (OS X)
> On 08 Mar 2016, at 13:30, Torsten Bögershausen wrote: > >>> And if not, I can put it on my TODO-stack. >> I have read through the official contribution guidelines and I think I can >> send an official patch. >> >> In this case, would you prefer to have a single commit since the change >> is related? Or would you prefer keeping it in separate commits, since >> they are different commands and I can use commit subjects like “diff:” >> and “diff-index:”, etc.? >> > Thanks for the work. > The same issue fixed at different places: > I personally would prefer a single commit. > > Another thing is, if we want to add TC in t3910, > to avoid future regressions. > (Otherwise I can help with those) I created a test case but git diff exits with 0 if it does not recognize the file path so the test case always succeeds. Can you give me a hint or one example test case? -- 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 diff does not precompose unicode file paths (OS X)
And if not, I can put it on my TODO-stack. I have read through the official contribution guidelines and I think I can send an official patch. In this case, would you prefer to have a single commit since the change is related? Or would you prefer keeping it in separate commits, since they are different commands and I can use commit subjects like “diff:” and “diff-index:”, etc.? Thanks for the work. The same issue fixed at different places: I personally would prefer a single commit. Another thing is, if we want to add TC in t3910, to avoid future regressions. (Otherwise I can help with those) -- 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 diff does not precompose unicode file paths (OS X)
On 03/07/2016 08:47 AM, Alexander Rinass wrote: On 04 Mar 2016, at 19:49, Ramsay Jones wrote: On 04/03/16 14:37, Alexander Rinass wrote: On 04 Mar 2016, at 13:16, Torsten Bögershausen wrote: On 03/04/2016 10:07 AM, Alexander Rinass wrote: [snip] Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes the issue. But I had to disable the check (precomposed_unicode != 1) in precompose_argv to make it work. That’s probably because precompose_argv is usually called from parse_options and is missing some other call before it? Yes, you need to place it after the configuration is read, but before calls to diff_no_index() or setup_revisions(). Directly after the call to git_config() should be fine. [But this begs the question about other commands, including plumbing, which don't call parse_options().] Maybe this will work for you (I can't test it, since I don't have any access to a Mac): diff --git a/builtin/diff.c b/builtin/diff.c index 343c6b8..b7a9405 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -320,6 +320,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) gitmodules_config(); init_diff_ui_defaults(); git_config(git_diff_ui_config, NULL); + precompose_argv(argc, argv); init_revisions(&rev, prefix); Your patch fixes it for the diff command without further modifications. I have also modified diff-tree, diff-index and diff-files by adding the precompose_argv call and successfully verified it. I have attached the full patch. If there is anything else I can test, let me know. Thanks for reporting, fixing, testing - Do you think you can send an official patch to the list ? If not, push your patches to some public repo ? And if not, I can put it on my TODO-stack. -- 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 diff does not precompose unicode file paths (OS X)
> On 04 Mar 2016, at 19:49, Ramsay Jones wrote:
>
>
>
> On 04/03/16 14:37, Alexander Rinass wrote:
>>
>>> On 04 Mar 2016, at 13:16, Torsten Bögershausen wrote:
>>>
>>> On 03/04/2016 10:07 AM, Alexander Rinass wrote:
> [snip]
>
>>
>> Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes
>> the issue.
>>
>> But I had to disable the check (precomposed_unicode != 1) in precompose_argv
>> to make it work. That’s probably because precompose_argv is usually called
>> from parse_options and is missing some other call before it?
>>
>
> Yes, you need to place it after the configuration is read, but before
> calls to diff_no_index() or setup_revisions(). Directly after the call
> to git_config() should be fine. [But this begs the question about other
> commands, including plumbing, which don't call parse_options().]
>
> Maybe this will work for you (I can't test it, since I don't have any
> access to a Mac):
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 343c6b8..b7a9405 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -320,6 +320,7 @@ int cmd_diff(int argc, const char **argv, const char
> *prefix)
> gitmodules_config();
> init_diff_ui_defaults();
> git_config(git_diff_ui_config, NULL);
> + precompose_argv(argc, argv);
>
> init_revisions(&rev, prefix);
Your patch fixes it for the diff command without further modifications.
I have also modified diff-tree, diff-index and diff-files by adding the
precompose_argv call and successfully verified it.
I have attached the full patch. If there is anything else I can test, let me
know.
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 8ed2eb8..15c61fd 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -24,6 +24,7 @@ int cmd_diff_files(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
+ precompose_argv(argc, argv);
argc = setup_revisions(argc, argv, &rev, NULL);
while (1 < argc && argv[1][0] == '-') {
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index d979824..1af373d 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -21,6 +21,7 @@ int cmd_diff_index(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
rev.abbrev = 0;
+ precompose_argv(argc, argv);
argc = setup_revisions(argc, argv, &rev, NULL);
for (i = 1; i < argc; i++) {
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 2a12b81..806dd7a 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -114,6 +114,8 @@ int cmd_diff_tree(int argc, const char **argv, const char
*prefix)
opt->disable_stdin = 1;
memset(&s_r_opt, 0, sizeof(s_r_opt));
s_r_opt.tweak = diff_tree_tweak_rev;
+
+ precompose_argv(argc, argv);
argc = setup_revisions(argc, argv, opt, &s_r_opt);
while (--argc > 0) {
diff --git a/builtin/diff.c b/builtin/diff.c
index 52c98a9..d6b8f98 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -319,6 +319,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
if (!no_index)
gitmodules_config();
git_config(git_diff_ui_config, NULL);
+ precompose_argv(argc, argv);
init_revisions(&rev, prefix);
--
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 diff does not precompose unicode file paths (OS X)
On 04/03/16 14:37, Alexander Rinass wrote: > >> On 04 Mar 2016, at 13:16, Torsten Bögershausen wrote: >> >> On 03/04/2016 10:07 AM, Alexander Rinass wrote: [snip] > > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes > the issue. > > But I had to disable the check (precomposed_unicode != 1) in precompose_argv > to make it work. That’s probably because precompose_argv is usually called > from parse_options and is missing some other call before it? > Yes, you need to place it after the configuration is read, but before calls to diff_no_index() or setup_revisions(). Directly after the call to git_config() should be fine. [But this begs the question about other commands, including plumbing, which don't call parse_options().] Maybe this will work for you (I can't test it, since I don't have any access to a Mac): diff --git a/builtin/diff.c b/builtin/diff.c index 343c6b8..b7a9405 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -320,6 +320,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) gitmodules_config(); init_diff_ui_defaults(); git_config(git_diff_ui_config, NULL); + precompose_argv(argc, argv); init_revisions(&rev, prefix); ATB, Ramsay Jones -- 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 diff does not precompose unicode file paths (OS X)
Alexander Rinass writes: > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff > function fixes the issue. > > But I had to disable the check (precomposed_unicode != 1) in > precompose_argv to make it work. That’s probably because > precompose_argv is usually called from parse_options and is > missing some other call before it? The "precomposed_unicode" bit comes from your configuration file, so it won't be usable before you call into git_default_core_config and that happens via a call to "git_config(git_diff_ui_config, NULL)". So perhaps you'd want to add the argv munging immediately before init_revisions() call there? -- 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 diff does not precompose unicode file paths (OS X)
> On 04 Mar 2016, at 13:16, Torsten Bögershausen wrote: > > On 03/04/2016 10:07 AM, Alexander Rinass wrote: >> Hallo, >> >> It appears that the git diff command does not precompose file path >> arguments, even if the option core.precomposeunicode is set to true (which >> is the default on OS X). >> >> Passing the decomposed form of a file path to the git diff command will >> yield no diff for a modified file. >> >> In my case, the decomposed form of the file path is sent by the OS X Cocoa >> framework's NSTask, wich I am using in an application. It can be simulated >> on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path >> argument on the shell. >> >> Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path >> forms, git diff does not. >> >> It can be tested with the following setup on OS X (as iconv's utf-8-mac >> encoding is only available on OS X): >> >> git init . >> git config core.quotepath true >> git config core.precomposeunicode true # (default on OS X) >> touch .gitignore && git add .gitignore && git commit -m "Initial commit" >> echo "." >> Ä >> git add Ä >> git commit -m "Create commit with unicode file path" >> echo "." >> Ä >> This gives the following status, showing the precomposed form of "Ä": >> >> git status --short >> M "\303\204" >> Running git add with both forms does work as expected: >> >> git add Ä >> git status --short >> M "\303\204" >> git reset HEAD -- Ä >> git add $(iconv -f utf-8 -t utf-8-mac <<< Ä) >> git status --short >> M "\303\204" >> git reset HEAD -- Ä >> However, running git diff only works with the precomposed form: >> >> git status --short >> M "\303\204" >> git --no-pager diff -- Ä >> [...shows diff...] >> git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä) >> [...shows NO diff...] >> >> I took a look at the Git source code, and the builtin/diff*.c do not contain >> the parse_options call (which does the precompose_argv call) that the other >> builtins use. >> >> But I am not really familiar with either C or the Git project structure, so >> this may not mean anything. >> >> Best regards, >> Alexander Rinass >> > Good analyzes, and thanks for the report. > It should be possible to stick in a > > precompose_arrgv(argc, argv) > > into builtin/diff.c > > Do you you can test that ? > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes the issue. But I had to disable the check (precomposed_unicode != 1) in precompose_argv to make it work. That’s probably because precompose_argv is usually called from parse_options and is missing some other call before it? I think it is clear that diff.c and friends are definitely missing the precomposing step. I am not sure about the right way to fix though (should parse_options be used in the end?) and my C skills are basic at best, otherwise I would create a patch. -- 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 diff does not precompose unicode file paths (OS X)
On 03/04/2016 10:07 AM, Alexander Rinass wrote: Hallo, It appears that the git diff command does not precompose file path arguments, even if the option core.precomposeunicode is set to true (which is the default on OS X). Passing the decomposed form of a file path to the git diff command will yield no diff for a modified file. In my case, the decomposed form of the file path is sent by the OS X Cocoa framework's NSTask, wich I am using in an application. It can be simulated on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path argument on the shell. Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path forms, git diff does not. It can be tested with the following setup on OS X (as iconv's utf-8-mac encoding is only available on OS X): git init . git config core.quotepath true git config core.precomposeunicode true # (default on OS X) touch .gitignore && git add .gitignore && git commit -m "Initial commit" echo "." >> Ä git add Ä git commit -m "Create commit with unicode file path" echo "." >> Ä This gives the following status, showing the precomposed form of "Ä": git status --short M "\303\204" Running git add with both forms does work as expected: git add Ä git status --short M "\303\204" git reset HEAD -- Ä git add $(iconv -f utf-8 -t utf-8-mac <<< Ä) git status --short M "\303\204" git reset HEAD -- Ä However, running git diff only works with the precomposed form: git status --short M "\303\204" git --no-pager diff -- Ä [...shows diff...] git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä) [...shows NO diff...] I took a look at the Git source code, and the builtin/diff*.c do not contain the parse_options call (which does the precompose_argv call) that the other builtins use. But I am not really familiar with either C or the Git project structure, so this may not mean anything. Best regards, Alexander Rinass Good analyzes, and thanks for the report. It should be possible to stick in a precompose_arrgv(argc, argv) into builtin/diff.c Do you you can test that ? -- 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 diff HEAD^(255) fails
On Sat, Feb 06, 2016 at 10:56:46PM +0100, Ole Tange wrote: > git diff first looks for a file, then looks if it is a reference to a > revision. If the file fails due to being too long, the diff fails: > > $ git init > $ git diff > 'HEAD^^^' > HEAD > fatal: failed to stat > 'HEAD^^^': > File name too long > > If file name too long it should just try to see if it is a reference > to a revision. > Is there a reason you are repeating 255 "^" instead of using HEAD~255? -- 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-diff: unable to turn off diff.autorefreshindex with command line switch
> On 21 Jan 2016, at 9:38 pm, Jeff King wrote: > But note that your whole test is going to be racy, and possibly depend > on the resolution of your filesystem's timestamps. Did you run your test > as a single script, where the initial index write and the `touch` may > happen in the same second? If so, trying running it slowly by hand, > or inserting "sleep 1" between the commands. I ran my tests by hand, taking more than 1s between commands. I also used `stat README` and `ls -l README` on occasion to confirm that `touch` was updating the file's timestamps. -- Andrew Stewart-- 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-diff: unable to turn off diff.autorefreshindex with command line switch
On Thu, Jan 21, 2016 at 10:29:49AM +, Andrew Stewart wrote: > # Now the strange behaviour: no output from the next command > $ git -c diff.autorefreshindex=0 diff --raw -- README I can't reproduce here (v2.7.0, Linux). But note that your whole test is going to be racy, and possibly depend on the resolution of your filesystem's timestamps. Did you run your test as a single script, where the initial index write and the `touch` may happen in the same second? If so, trying running it slowly by hand, or inserting "sleep 1" between the commands. -Peff -- 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-diff-tree --root
Roman Neuhauser writes: > ... I like > my scripts as simple as possible, so I'd like to use --root *always*. With "--root", the command does not do anything different without for commits that are not root commits, and it shows "everything created from nothing" for root commits. The option was invented to help those who want that behaviour, so if you'd like to use it always, then you are welcome to do so ;-) -- 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-diff-tree --root
# gits...@pobox.com / 2014-09-12 10:31:30 -0700: > Roman Neuhauser writes: > > git-diff-tree without --root is absolutely silent for the root commit, > > and i see no bad effects of --root on non-root commits. are there any > > hidden gotchas? IOW, why is the --root behavior not the default? > > Because tools that was written before you proposed that change > expect to see nothing for the root commit, and then you are suddenly > breaking their expectations? i'm not proposing anything, i'm just curious why it is this way. my line of thinking: there must be (or have been) a grave reason to break the simple consistency, or the current behavior must be very useful for something and i'm just missing what it is. the reasons for the behavior may have been invalidated by further developments, or it may have been a wrong decision we're stuck with for BC; i'm just curious about history. motivation for my question is that i'm scripting git-diff-tree and i need it to produce the diff for root commits as well. i like my scripts as simple as possible, so i'd like to use --root *always*. is it safe? -- roman -- 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-diff-tree --root
Roman Neuhauser writes: > git-diff-tree without --root is absolutely silent for the root commit, > and i see no bad effects of --root on non-root commits. are there any > hidden gotchas? IOW, why is the --root behavior not the default? Because tools that was written before you proposed that change expect to see nothing for the root commit, and then you are suddenly breaking their expectations? -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Tue, Jul 29, 2014 at 11:06:25AM +1000, Bryan Turner wrote: > On Tue, Jul 29, 2014 at 10:11 AM, Junio C Hamano wrote: > > > OK, I pushed out updated 'maint' and 'master'. The former merges > > a rebased version of jk/alloc-commit-id in to make the "reorganize > > the way we manage the in-core commit data" topic, and the latter > > reverts the "Use SSE to micro-optimize a leaf function to check the > > format of a ref string". > > > > Please give them some quick sanity check. They both look OK to me. > Thanks both of you; I appreciate your efforts! I've run my suite of > tests against the tips of master and maint and all 681 pass for each. > Looks good to me. So what suite of tests is this? Is it worth getting you to fold it into our regular test suite? :) -Peff -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Tue, Jul 29, 2014 at 10:11 AM, Junio C Hamano wrote: > Junio C Hamano writes: > Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. >>> >>> Whatever we do, perhaps it is worth applying the test below on top? >> >> Yeah, thanks. I think that is a good idea. I was preparing a patch >> to tuck your minimum reproduction at the end of 4202, but your version >> and placement makes good sense. > > OK, I pushed out updated 'maint' and 'master'. The former merges > a rebased version of jk/alloc-commit-id in to make the "reorganize > the way we manage the in-core commit data" topic, and the latter > reverts the "Use SSE to micro-optimize a leaf function to check the > format of a ref string". > > Please give them some quick sanity check. > > Thanks. Thanks both of you; I appreciate your efforts! I've run my suite of tests against the tips of master and maint and all 681 pass for each. Looks good to me. Best regards, Bryan Turner -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
Junio C Hamano writes: >>> Yeah, I'm fine with a straight revert, too (I think it is fine to keep >>> in master, though). I think jk/alloc-commit-id is built right on top of >>> the original commit-slab topic, so it should be easy to do either way. >>> >>> Thanks for dealing with it. >> >> Whatever we do, perhaps it is worth applying the test below on top? > > Yeah, thanks. I think that is a good idea. I was preparing a patch > to tuck your minimum reproduction at the end of 4202, but your version > and placement makes good sense. OK, I pushed out updated 'maint' and 'master'. The former merges a rebased version of jk/alloc-commit-id in to make the "reorganize the way we manage the in-core commit data" topic, and the latter reverts the "Use SSE to micro-optimize a leaf function to check the format of a ref string". Please give them some quick sanity check. Thanks. -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
Jeff King writes: > On Mon, Jul 28, 2014 at 01:37:34PM -0400, Jeff King wrote: > >> On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: >> >> > > Junio, we should consider a v2.0.4 with that series, I think. This is a >> > > pretty serious regression in diff-tree (I didn't even realize that the >> > > buffer-slab work went into the maint series; that may have been a little >> > > ambitious). >> > >> > Or v2.0.4 without that series, which is how we usually do things, >> > but let me see if jk/alloc-commit-id is easily applicable there >> > first. >> >> Yeah, I'm fine with a straight revert, too (I think it is fine to keep >> in master, though). I think jk/alloc-commit-id is built right on top of >> the original commit-slab topic, so it should be easy to do either way. >> >> Thanks for dealing with it. > > Whatever we do, perhaps it is worth applying the test below on top? Yeah, thanks. I think that is a good idea. I was preparing a patch to tuck your minimum reproduction at the end of 4202, but your version and placement makes good sense. > -- >8 -- > Subject: t4013: test diff-tree's --stdin commit formatting > > Once upon a time, git-log was just "rev-list | diff-tree", > and we did not bother to test it separately. These days git-log > is implemented internally, but we want to make sure that the > rev-list to diff-tree pipeline continues to function. Let's > add a basic sanity test. > > Signed-off-by: Jeff King > --- > t/t4013-diff-various.sh | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index 805b055..6ec6072 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -324,4 +324,14 @@ test_expect_success 'diff --cached -- file on unborn > branch' ' > test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result > ' > > +test_expect_success 'diff-tree --stdin with log formatting' ' > + cat >expect <<-\EOF && > + Side > + Third > + Second > + EOF > + git rev-list master | git diff-tree --stdin --format=%s -s >actual && > + test_cmp expect actual > +' > + > test_done -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 01:37:34PM -0400, Jeff King wrote: > On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: > > > > Junio, we should consider a v2.0.4 with that series, I think. This is a > > > pretty serious regression in diff-tree (I didn't even realize that the > > > buffer-slab work went into the maint series; that may have been a little > > > ambitious). > > > > Or v2.0.4 without that series, which is how we usually do things, > > but let me see if jk/alloc-commit-id is easily applicable there > > first. > > Yeah, I'm fine with a straight revert, too (I think it is fine to keep > in master, though). I think jk/alloc-commit-id is built right on top of > the original commit-slab topic, so it should be easy to do either way. > > Thanks for dealing with it. Whatever we do, perhaps it is worth applying the test below on top? -- >8 -- Subject: t4013: test diff-tree's --stdin commit formatting Once upon a time, git-log was just "rev-list | diff-tree", and we did not bother to test it separately. These days git-log is implemented internally, but we want to make sure that the rev-list to diff-tree pipeline continues to function. Let's add a basic sanity test. Signed-off-by: Jeff King --- t/t4013-diff-various.sh | 10 ++ 1 file changed, 10 insertions(+) diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 805b055..6ec6072 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -324,4 +324,14 @@ test_expect_success 'diff --cached -- file on unborn branch' ' test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result ' +test_expect_success 'diff-tree --stdin with log formatting' ' + cat >expect <<-\EOF && + Side + Third + Second + EOF + git rev-list master | git diff-tree --stdin --format=%s -s >actual && + test_cmp expect actual +' + test_done -- 2.0.0.566.gfe3e6b2 -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 10:32:45AM -0700, Junio C Hamano wrote: > > Junio, we should consider a v2.0.4 with that series, I think. This is a > > pretty serious regression in diff-tree (I didn't even realize that the > > buffer-slab work went into the maint series; that may have been a little > > ambitious). > > Or v2.0.4 without that series, which is how we usually do things, > but let me see if jk/alloc-commit-id is easily applicable there > first. Yeah, I'm fine with a straight revert, too (I think it is fine to keep in master, though). I think jk/alloc-commit-id is built right on top of the original commit-slab topic, so it should be easy to do either way. Thanks for dealing with it. -Peff -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
Jeff King writes: > On Mon, Jul 28, 2014 at 07:42:16PM +1000, Bryan Turner wrote: > >> Running a git bisect between v2.0.1, which does not manifest this >> issue, and v2.0.2 fingers the following commit: >> bturner@ubuntu:~/Development/oss/git/git$ git bisect bad >> c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit >> commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 >> Author: Jeff King >> Date: Tue Jun 10 17:43:02 2014 -0400 >> >> commit: convert commit->buffer to a slab > > I haven't reproduced here yet, but this is almost certainly the bug > where lookup_unknown_object causes a bogus commit->index field (and > prior to the commit you found, diff-tree did not use commit->index). > > The series that Junio has in jk/alloc-commit-id should fix the problem > (it's in master already, and slated for v2.1.0). > > Junio, we should consider a v2.0.4 with that series, I think. This is a > pretty serious regression in diff-tree (I didn't even realize that the > buffer-slab work went into the maint series; that may have been a little > ambitious). Or v2.0.4 without that series, which is how we usually do things, but let me see if jk/alloc-commit-id is easily applicable there first. Thanks. -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 08:35:52AM -0700, Junio C Hamano wrote: > I am tempted to revert that series; it already caused "oops, this > needs a further fix" before it hit 'master' at least once, and we do > not want any more headaches at this point in the release cycle. Yeah, that sounds reasonable to me. I'm a little doubtful of its value (and maintainability) at all, but certainly I do not think it would be a big deal to push it off for one more cycle if David wants to rework it. If you do revert it, we may want a test like below. That will make sure the case is covered for future attempts. -- >8 -- Subject: [PATCH] t1402: check for refs ending with a dot This has been illegal since cbdffe4 (check_ref_format(): tighten refname rules, 2009-03-21), but we never tested it. Signed-off-by: Jeff King --- t/t1402-check-ref-format.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh index 9aeb352..8d2f75f 100755 --- a/t/t1402-check-ref-format.sh +++ b/t/t1402-check-ref-format.sh @@ -48,6 +48,7 @@ invalid_ref './foo/bar' invalid_ref 'foo/./bar' invalid_ref 'foo/bar/.' invalid_ref '.refs/foo' +invalid_ref 'refs/heads/foo.' invalid_ref 'heads/foo..bar' invalid_ref 'heads/foo?bar' valid_ref 'foo./bar' -- 2.0.0.566.gfe3e6b2 -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
Bryan Turner writes: > It looks like refs ending in a dot are now legal in 2.1.0? Is that > intentional? A quick git bisect is fingering: > bturner@ubuntu:~/Development/oss/git/git$ git bisect bad > 745224e04a03e4544c58d5d38d3c54f67100f8eb is the first bad commit > commit 745224e04a03e4544c58d5d38d3c54f67100f8eb > Author: David Turner > Date: Wed Jun 18 01:54:42 2014 -0400 Thanks for a report. I am tempted to revert that series; it already caused "oops, this needs a further fix" before it hit 'master' at least once, and we do not want any more headaches at this point in the release cycle. -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 8:44 PM, Jeff King wrote: > On Mon, Jul 28, 2014 at 06:35:04AM -0400, Jeff King wrote: > >> I haven't reproduced here yet, but this is almost certainly the bug >> where lookup_unknown_object causes a bogus commit->index field (and >> prior to the commit you found, diff-tree did not use commit->index). >> >> The series that Junio has in jk/alloc-commit-id should fix the problem >> (it's in master already, and slated for v2.1.0). > > Yep, that's definitely it. Here's the minimum reproduction: > > git init > git commit --allow-empty -m one > git commit --allow-empty -m two > git rev-list HEAD | git diff-tree --stdin --always --format=%s > > That yields: > > one > one > > on v2.0.3, but merging in jk/alloc-commit-id yields: > > two > one > > -Peff Thanks for digging into it, Jeff. I should have tried it against 2.1.0 myself. I've run my entire matrix of tests now against 2.1.0-rc0 and the diff-tree bug appears fixed on that tag. I noticed a different change, though: bturner@ubuntu:~/tmp/test$ /opt/git/2.1.0-rc0/bin/git check-ref-format ref/with/trailing/dot. bturner@ubuntu:~/tmp/test$ echo $? 0 bturner@ubuntu:~/tmp/test$ /opt/git/2.0.3/bin/git check-ref-format ref/with/trailing/dot. bturner@ubuntu:~/tmp/test$ echo $? 1 It looks like refs ending in a dot are now legal in 2.1.0? Is that intentional? A quick git bisect is fingering: bturner@ubuntu:~/Development/oss/git/git$ git bisect bad 745224e04a03e4544c58d5d38d3c54f67100f8eb is the first bad commit commit 745224e04a03e4544c58d5d38d3c54f67100f8eb Author: David Turner Date: Wed Jun 18 01:54:42 2014 -0400 Best regards, Bryan Turner -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 06:35:04AM -0400, Jeff King wrote: > I haven't reproduced here yet, but this is almost certainly the bug > where lookup_unknown_object causes a bogus commit->index field (and > prior to the commit you found, diff-tree did not use commit->index). > > The series that Junio has in jk/alloc-commit-id should fix the problem > (it's in master already, and slated for v2.1.0). Yep, that's definitely it. Here's the minimum reproduction: git init git commit --allow-empty -m one git commit --allow-empty -m two git rev-list HEAD | git diff-tree --stdin --always --format=%s That yields: one one on v2.0.3, but merging in jk/alloc-commit-id yields: two one -Peff -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On Mon, Jul 28, 2014 at 07:42:16PM +1000, Bryan Turner wrote: > Running a git bisect between v2.0.1, which does not manifest this > issue, and v2.0.2 fingers the following commit: > bturner@ubuntu:~/Development/oss/git/git$ git bisect bad > c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit > commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 > Author: Jeff King > Date: Tue Jun 10 17:43:02 2014 -0400 > > commit: convert commit->buffer to a slab I haven't reproduced here yet, but this is almost certainly the bug where lookup_unknown_object causes a bogus commit->index field (and prior to the commit you found, diff-tree did not use commit->index). The series that Junio has in jk/alloc-commit-id should fix the problem (it's in master already, and slated for v2.1.0). Junio, we should consider a v2.0.4 with that series, I think. This is a pretty serious regression in diff-tree (I didn't even realize that the buffer-slab work went into the maint series; that may have been a little ambitious). -Peff -- 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 diff-tree commit detail bug in 2.0.2 and 2.0.3
On 28/07/14 10:42, Bryan Turner wrote: > Using git diff-tree --stdin on 2.0.2 and 2.0.3 produces incorrect > commit messages. > > Here's an example to reproduce the issue: > > bturner@ubuntu:/tmp$ git init --bare test.git > Initialized empty Git repository in /tmp/test.git/ > bturner@ubuntu:/tmp$ git clone test.git > Cloning into 'test'... > warning: You appear to have cloned an empty repository. > done. > bturner@ubuntu:/tmp$ cd test > bturner@ubuntu:/tmp/test$ echo "Hello" > file.txt > bturner@ubuntu:/tmp/test$ git add file.txt > bturner@ubuntu:/tmp/test$ git commit -m "Initial commit" > [master (root-commit) c5e16f3] Initial commit > 1 file changed, 1 insertion(+) > create mode 100644 file.txt > bturner@ubuntu:/tmp/test$ echo "World" >> file.txt > bturner@ubuntu:/tmp/test$ git commit -am "Second commit" > [master 9214ac7] Second commit > 1 file changed, 1 insertion(+) > bturner@ubuntu:/tmp/test$ git push origin HEAD > Counting objects: 6, done. > Delta compression using up to 4 threads. > Compressing objects: 100% (2/2), done. > Writing objects: 100% (6/6), 446 bytes | 0 bytes/s, done. > Total 6 (delta 0), reused 0 (delta 0) > To /tmp/test.git > * [new branch] HEAD -> master > bturner@ubuntu:/tmp/test$ cd ../test.git/ > bturner@ubuntu:/tmp/test.git$ git rev-list -1 > --format="%H|%h|%P|%p|%aN|%aE|%at%n%B%n" 9214ac7 > commit 9214ac79728424a971244c34432c6d948754198d > 9214ac79728424a971244c34432c6d948754198d|9214ac7|c5e16f37164f1b7411685def64d7390775437f07|c5e16f3|Bryan > Turner|btur...@atlassian.com|1406539558 > Second commit > > > bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree ^^^ You appear to have used v2.0.3 on both invocations of diff-tree (see also below); cut-n-paste error? > --no-renames --always --format="commit > %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n" --root > 9214ac79728424a971244c34432c6d948754198d > commit 9214ac79728424a971244c34432c6d948754198d > 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan > Turner|btur...@atlassian.com|1406539558|Second commit > > > > :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 > f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt > bturner@ubuntu:/tmp/test.git$ /opt/git/2.0.3/bin/git diff-tree > --no-renames --always --format="commit > %H%n%H|%h|%P|%p|%aN|%aE|%at|%B%n" --root --stdin > --9214ac79728424a971244c34432c6d948754198d > commit 9214ac79728424a971244c34432c6d948754198d > 9214ac79728424a971244c34432c6d948754198d|9214ac79728424a971244c34432c6d948754198d|c5e16f37164f1b7411685def64d7390775437f07|c5e16f37164f1b7411685def64d7390775437f07|Bryan > Turner|btur...@atlassian.com|1406539543|Initial commit ---^^ The timestamp is also different than the above. > > > > :100644 100644 e965047ad7c57865823c7d992b1d046ea66edf78 > f9264f7fbd31ae7a18b7931ed8946fb0aebb0af3 Mfile.txt > bturner@ubuntu:/tmp/test.git$ > > Running a git bisect between v2.0.1, which does not manifest this > issue, and v2.0.2 fingers the following commit: > bturner@ubuntu:~/Development/oss/git/git$ git bisect bad > c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 is the first bad commit > commit c1b3c71f4b4571abb2b2a457122fd100dc9f7eb0 > Author: Jeff King > Date: Tue Jun 10 17:43:02 2014 -0400 > > commit: convert commit->buffer to a slab > ATB, Ramsay Jones -- 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 diff' command doesn't honor utf8 symbol boundaries, and doesn't use actual terminal width
On Sat, Jan 25, 2014 at 10:34 AM, Yuri wrote: > The fragment of 'git diff' output looks like this: > - Ошибка: адрес %1 содержит запрещенные символы. Пожалуйста, > перепро� > + Ошибка: адрес %1 содержит запрещённые символы. Пожалуйста, > перепро� > > Two issues here: > 1. Cyrilic text in utf8 got truncated not at utf8 boundary, causing this > garbage symbol in the end > 2. Truncation is done at somewhat ~70% of the actual screen width. git could > go all the way to the whole screen with, but it didn't. Shrinking terminal > width causes the output truncation limit to shrink in the same proportion. > So some bad decision about truncation size is made somewhere in 'git diff' > command. > > Suggested behavior: > 1. git should respect utf8, and in case of truncation it should add … > symbol. > 2. truncation algorithm should read current terminal width and truncate at > width-1 length to allow for the above-mentioned symbol I don't think git diff truncates its output (it does truncate @@ lines, but not the real diff). Perhaps your pager did that? -- Duy -- 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 diff returns fatal error with core.safecrlf is set to true.
Torsten Bögershausen writes:
>> +++ b/diff.c
>> @@ -2647,6 +2647,10 @@ static int diff_populate_gitlink(struct diff_filespec
>> *s, int size_only)
>> int diff_populate_filespec(struct diff_filespec *s, int size_only)
>> {
>> int err = 0;
>> +enum safe_crlf crlf_warn = (safe_crlf != SAFE_CRLF_FAIL
>> +? safe_crlf
>> +: SAFE_CRLF_WARN);
>
> Thanks,
> Does it makes sense to write it the other way around?
>
> enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
>? SAFE_CRLF_WARN
>: safe_crlf);
I didn't see much difference either way, but between "FAIL needs to
be demoted to WARN, everything else goes as-is" and the original "We
do not care about anything other than FAIL, so use it as-is, but
demote FAIL to WARN", yours look shorter. Will replace.
--
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 diff returns fatal error with core.safecrlf is set to true.
> +++ b/diff.c
> @@ -2647,6 +2647,10 @@ static int diff_populate_gitlink(struct diff_filespec
> *s, int size_only)
> int diff_populate_filespec(struct diff_filespec *s, int size_only)
> {
> int err = 0;
> + enum safe_crlf crlf_warn = (safe_crlf != SAFE_CRLF_FAIL
> + ? safe_crlf
> + : SAFE_CRLF_WARN);
Thanks,
Does it makes sense to write it the other way around?
enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
? SAFE_CRLF_WARN
: safe_crlf);
--
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 diff returns fatal error with core.safecrlf is set to true.
Junio C Hamano writes:
> Care to turn it into an appliable patch with tests?
In the meantime, here is a quick-and-dirty one. I am not proud of
it; it was just something to keep in 'pu' let it gets lost.
A better replacement is very much welcomed.
-- >8 --
Subject: [PATCH] diff: demote core.safecrlf=true to core.safecrlf=warn
Otherwise the user will not be able to start to guess where in the
contents in the working tree the offending unsafe CR lies.
Signed-off-by: Junio C Hamano
---
diff.c | 6 +-
t/t0020-crlf.sh | 8
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 32142db..155857c 100644
--- a/diff.c
+++ b/diff.c
@@ -2647,6 +2647,10 @@ static int diff_populate_gitlink(struct diff_filespec
*s, int size_only)
int diff_populate_filespec(struct diff_filespec *s, int size_only)
{
int err = 0;
+ enum safe_crlf crlf_warn = (safe_crlf != SAFE_CRLF_FAIL
+ ? safe_crlf
+ : SAFE_CRLF_WARN);
+
if (!DIFF_FILE_VALID(s))
die("internal error: asking to populate invalid file.");
if (S_ISDIR(s->mode))
@@ -2702,7 +2706,7 @@ int diff_populate_filespec(struct diff_filespec *s, int
size_only)
/*
* Convert from working tree format to canonical git format
*/
- if (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf))
{
+ if (convert_to_git(s->path, s->data, s->size, &buf, crlf_warn))
{
size_t size = 0;
munmap(s->data, s->size);
s->should_munmap = 0;
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 1a8f44c..e526184 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -81,6 +81,14 @@ test_expect_success 'safecrlf: print warning only once' '
test $(git add doublewarn 2>&1 | grep "CRLF will be replaced by LF" |
wc -l) = 1
'
+
+test_expect_success 'safecrlf: git diff demotes safecrlf=true to warn' '
+ git config core.autocrlf input &&
+ git config core.safecrlf true &&
+ git diff HEAD
+'
+
+
test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '
git config core.autocrlf false &&
git config core.safecrlf false &&
--
1.8.3.1-771-gb7c3f25
--
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 diff returns fatal error with core.safecrlf is set to true.
Yann Droneaud writes: > Hi, > > Le 21.06.2013 23:57, Junio C Hamano a écrit : >> Junio C Hamano writes: >> >>> The helper may want to learn a way to be told to demote that error >>> to a warning. >> >> Perhaps something like this? >> > > Thanks for the patch. Care to turn it into an appliable patch with tests? Thanks. -- 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 diff returns fatal error with core.safecrlf is set to true.
Hi, Le 21.06.2013 23:57, Junio C Hamano a écrit : Junio C Hamano writes: The helper may want to learn a way to be told to demote that error to a warning. Perhaps something like this? Thanks for the patch. I run my test again, eg. run git diff after a rebase failure (see my other mail about core.safecrlf), I'm able to run git diff a get a meaningful output: # git version 1.8.1.4 fatal: CRLF would be replaced by LF in test. # git version 1.8.3.1.741.g635527f.dirty (eg. next + your patch) warning: CRLF will be replaced by LF in test. The file will have its original line endings in your working directory. diff --git a/test b/test index b043836..63ba10f 100644 --- a/test +++ b/test @@ -1,4 +1,4 @@ -Hello World 1 -Hello World 2 -Hello World 3 +Hello World 1 +Hello World 2 +Hello World 3 Hello World 4 \ No newline at end of file It seems better. The removed lines have CRLF EOL while the added line have LF line ending characters. Tested-By: Yann Droneaud Regards. -- Yann Droneaud OPTEYA -- 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 diff returns fatal error with core.safecrlf is set to true.
Junio C Hamano writes:
> The helper may want to learn a way to be told to demote that error
> to a warning.
Perhaps something like this?
diff.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index f0b3e7c..9b4f3ac 100644
--- a/diff.c
+++ b/diff.c
@@ -2677,6 +2677,10 @@ static int diff_populate_gitlink(struct diff_filespec
*s, int size_only)
int diff_populate_filespec(struct diff_filespec *s, int size_only)
{
int err = 0;
+ enum safe_crlf crlf_warn = (safe_crlf != SAFE_CRLF_FAIL
+ ? safe_crlf
+ : SAFE_CRLF_WARN);
+
if (!DIFF_FILE_VALID(s))
die("internal error: asking to populate invalid file.");
if (S_ISDIR(s->mode))
@@ -2732,7 +2736,7 @@ int diff_populate_filespec(struct diff_filespec *s, int
size_only)
/*
* Convert from working tree format to canonical git format
*/
- if (convert_to_git(s->path, s->data, s->size, &buf, safe_crlf))
{
+ if (convert_to_git(s->path, s->data, s->size, &buf, crlf_warn))
{
size_t size = 0;
munmap(s->data, s->size);
s->should_munmap = 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: git diff returns fatal error with core.safecrlf is set to true.
Yann Droneaud writes: > While testing the behavor of Git regarding CRLF handling, > when core.safecrlf is set to true, I've found that "git diff" is > returning > "fatal: CRLF would be replaced by LF" without returning any kind of > diff. > > This make me wonder if its the correct behavor for git diff to (only) > fail: > It should be fatal for git add / git commit ( / git cherry-pick / > ... ?), > but non fatal for git diff. Yeah, I agree. This is a diff between something and the working tree file, right? It needs to convert from the working tree representation to the canonical repository representation before doing the actual comparison, and most likely the same helper function that is reused for the check-in codepath, which needs to error out, is erroring out after finding an input in your working tree that cannot safely round-trip between LF/CRLF world. The helper may want to learn a way to be told to demote that error to a warning. -- 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 diff bug?
On Mon, Jun 10, 2013 at 8:44 AM, Célestin Matte wrote: > Since nobody answered you (publicly at least), I will try doing it myself: > I think the best thing to do if you want a feature to be added is to > come with a patch and request for comments on it. Then, people will > discuss it and decide whether it's worth adding it to git. So yes, you > can try implementing it - all work is welcome :) That sounds great. I will try implementing and send out a patch soon! -- 010 001 111 -- 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 diff bug?
Le 07/06/2013 18:01, Sarma Tangirala a écrit : > On Thu, Jun 6, 2013 at 6:17 PM, Junio C Hamano wrote: >> Célestin Matte writes: >> > >> But for a two-endpoint diff Porcelain (not the plumbing diff-files, >> diff-index and diff-tree), I do not think it is particularly a bad >> idea to add such a "typo-detection" feature. > > I was wondering if this feature is going to be added and if I could > try implementing it. Since nobody answered you (publicly at least), I will try doing it myself: I think the best thing to do if you want a feature to be added is to come with a patch and request for comments on it. Then, people will discuss it and decide whether it's worth adding it to git. So yes, you can try implementing it - all work is welcome :) -- Célestin Matte -- 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 diff bug?
On Thu, Jun 6, 2013 at 6:17 PM, Junio C Hamano wrote: > Célestin Matte writes: > > But for a two-endpoint diff Porcelain (not the plumbing diff-files, > diff-index and diff-tree), I do not think it is particularly a bad > idea to add such a "typo-detection" feature. I was wondering if this feature is going to be added and if I could try implementing it. -- 010 001 111 -- 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 diff bug?
Célestin Matte writes: > Le 06/06/2013 23:26, Sarma Tangirala a écrit : >> Hello All, >> >> If I did 'git diff HEAD^..HEAD -- file' should git not report some >> kind of warning if it could not match the file? For example, if 'file' >> were infact 'dir/file' and 'file' were unique, would it not be a good >> idea to report that in the present working directory 'file' were not >> found but 'dir/file' were a match? > > I don't know any program doing such a thing, and I don't think it is the > role of the program to predict which file the user actually wanted to > provide in the command line. > That would imply looking for files with the same name or a close name in > the current directory and its subdirectories - and maybe even in the > superdirectory? It is hard to decide when you have to stop looking for > the file. The parameters after "--" are pathspecs, which is a set of patterns the paths discovered by the operation (in this case "diff" that finds paths in HEAD^ and HEAD) are matched against. They are used to filter out uninteresting paths. If HEAD^ and HEAD does not have anything that match the given pattern (in this case, literal four-letter string "file"), the set of interesting paths may become empty and that is perfectly normal. So this is working as designed. Having said that, we do detect typo by noticing when a pathspec did not find _any_ path that matched it in some front-end Porcelain commands, e.g. $ git add 'foo*' fatal: pathspec 'foo*' did not match any files It is unreasonable to do the same in "git log old..new -- path" and error out when the pathspec does not match, because it is normal for some revisions to have path while some other revisions to lack it. But for a two-endpoint diff Porcelain (not the plumbing diff-files, diff-index and diff-tree), I do not think it is particularly a bad idea to add such a "typo-detection" feature. -- 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 diff bug?
Le 06/06/2013 23:26, Sarma Tangirala a écrit : > Hello All, > > If I did 'git diff HEAD^..HEAD -- file' should git not report some > kind of warning if it could not match the file? For example, if 'file' > were infact 'dir/file' and 'file' were unique, would it not be a good > idea to report that in the present working directory 'file' were not > found but 'dir/file' were a match? I don't know any program doing such a thing, and I don't think it is the role of the program to predict which file the user actually wanted to provide in the command line. That would imply looking for files with the same name or a close name in the current directory and its subdirectories - and maybe even in the superdirectory? It is hard to decide when you have to stop looking for the file. -- 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
