[PATCH v2] git svn: reset invalidates the memoized mergeinfo caches
On Wed, Aug 08, 2012 at 10:52:58PM +, Eric Wong wrote:
Peter Baumann waste.mana...@gmx.de wrote:
On Tue, Aug 07, 2012 at 08:45:10PM +, Eric Wong wrote:
Peter Baumann waste.mana...@gmx.de wrote:
+ for my $suffix (qw(yaml db)) {
+ unlink($cache_file.$suffix);
Need to check for unlink() errors (and ignore ENOENT).
I'm not sure what you mean here: Aren't we screwed either way if unlinking
the file failed? There is nothhing we can do about it if e.g. the user
doesn't
have the permissions to delete the file, besides terminating, e.g.
for my $cache_file (($cache_path/lookup_svn_merge,
$cache_path/check_cherry_pick,
$cache_path/has_no_changes)) {
for my $suffix (qw(yaml db)) {
next unless (-e $cache_file.$suffix);
unlink($cache_file.$suffix) or
die Failed to delete $cache_file.$suffix;
}
Yes we're screwed, but silent failure is the worst way to fail,
especially if it can lead us back to the problems your patch is meant to
address.
Perhaps something like this (with $! to show the error):
my $file = $cache_file.$suffix;
next unless -e $file;
unlink($file) or die unlink($file) failed: $!\n;
First, let me thank you for your review and your detailed explanation.
I really appreciate it.
I changed svn to svn_cmd in the test (and also some minor changes to
the comments and removing trailing whitespace) and switched the order of
clear_memoized_mergeinfo_caches and _rev_map_reset as you asked for, so
hopefully this is ready to go in.
-- 8 --
From: Peter Baumann waste.mana...@gmx.de
Subject: [PATCH] git svn: reset invalidates the memoized mergeinfo caches
Since v1.7.0-rc2~11 (git-svn: persistent memoization, 2010-01-30),
git-svn has maintained some private per-repository caches in
.git/svn/.caches to avoid refetching and recalculating some
mergeinfo-related information with every 'git svn fetch'.
This memoization can cause problems, e.g consider the following case:
SVN repo:
... - a - b - c - m - trunk
\/
d - e- branch1
The Git import of the above repo is at commit 'a' and doesn't know about
the branch1. In case of an 'git svn rebase', only the trunk of the
SVN repo is imported. During the creation of the git commit 'm', git svn
uses the svn:mergeinfo property and tries to find the corresponding git
commit 'e' to create 'm' with 'c' and 'e' as parents. But git svn rebase
only imports the current branch so commit 'e' is not imported.
Therefore git svn fails to create commit 'm' as a merge commit, because one
of its parents is not known to git. The imported history looks like this:
... - a - b - c - m - trunk
A later 'git svn fetch' to import all branches can't rewrite the commit 'm'
to add 'e' as a parent and to make it a real git merge commit, because it
was already imported.
That's why the imported history misses the merge and looks like this:
... - a - b - c - m - trunk
\
d - e- branch1
Right now the only known workaround for importing 'm' as a merge is to
force reimporting 'm' again from SVN, e.g. via
$ git svn reset --revision $(git find-rev $c)
$ git svn fetch
Sadly, this is where the behavior has regressed: git svn reset doesn't
invalidate the old mergeinfo cache, which is no longer valid for the
reimport, which leads to 'm' beeing imprted with only 'c' as parent.
As solution to this problem, this commit invalidates the mergeinfo cache
to force correct recalculation of the parents.
During development of this patch, several ways for invalidating the cache
where considered. One of them is to use Memoize::flush_cache, which will
call the CLEAR method on the underlying Memoize persistency implementation.
Sadly, neither Memoize::Storable nor the newer Memoize::YAML module
introduced in 68f532f4ba888 could optionally be used implement the
CLEAR method, so this is not an option.
Reseting the internal hash used to store the memoized values has the same
problem, because it calls the non-existing CLEAR method of the
underlying persistency layer, too.
Considering this and taking into account the different implementations
of the memoization modules, where Memoize::Storable is not in our control,
implementing the missing CLEAR method is not an option, at least not if
Memoize::Storable is still used.
Therefore the easiest solution to clear the cache is to delete the files
on disk in 'git svn reset'. Normally, deleting the files behind the back
of the memoization module would be problematic, because the in-memory
representation would still exist and contain wrong data. Fortunately, the
memoization is active in memory only for a small portion of the code.
Invalidating the cache by deleting the files on
Re: [PATCH v2 resend] gitk: Use an external icon file on Windows
On Wed, Aug 8, 2012 at 11:13 PM, Junio C Hamano gits...@pobox.com wrote: Forwarding a misdirected patch to the maintainer who is free to pick or ignore. How am I supposed to know if a patch has been ignored as an oversight (in which case I would resend), or because the maintainer decided not to include it (in which case I would not resend in order to not annoy the maintainer)? Personally I am negative on it (nobody on the list asked for the new Git icon as far as I recall), but my voice on this counts just as little as others. I guess most patches on the list come in unasked, so I cannot follow your negative attitude because of this. Also, the patch does not enforce the new icon on any platform. From a user's perspective, [1] as mentioned on [2] is the official Git homepage, thus the icon used on the homepage can be regarded as the official Git icon. In Git for Windows, we try to have a consistent user experience, and we had a report about gitk still using the old icon [3]. This patch fixes that, and I'm sending it upstream in an effort to not let msysgit's fork of git diverge too much from upstream. [1] http://git-scm.com/ [2] http://en.wikipedia.org/wiki/Git_%28software%29#External_links [3] https://github.com/msysgit/msysgit/issues/44 -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add Code Compare v2.80.4 as a merge / diff tool for Windows
On Wed, Aug 8, 2012 at 11:26 PM, Junio C Hamano gits...@pobox.com wrote: I do not have a strong reason to vote for or against inclusion of yet another tool as mergetool backends (read: Meh), but what this That sounds almost as if you'd like to keep the number of directly supported mergetools small (I'm not talking about about the length of the list of mergetools in the docs right now). I was always thinking, the more mergetools, the better. Do you think differently? patch does to Documentation/merge-config.txt is actively unwelcome. As we discussed earlier in http://thread.gmane.org/gmane.comp.version-control.git/201913/focus=201976 the longer term direction is to reduce the names of tools listed there. I am somewhat saddened to find your name in that thread; you should have been aware of that discussion when you wrote this patch. I still agree that not listing all mergetools in multiple places is a good thing. But doing the whole stuff of extending --tool-help for git-mergetool and git-difftool to return a simple list that can be used in git-completion.bash etc. IMHO is a separate topic and out of scope of this patch. If it helps to accept the patch, I can send a v2 that changes merge-config.txt and diff-config.txt to refer to git-mergetool --tool-help and git-difftool --tool-help instead of naming any actual tools. -- Sebastian Schuberth -- 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
Please pull git-l10n updates for git v1.7.12-rc2
Hi Junio, The following changes since commit bfbf4d477a33be413800f531c3ac0d227f46ab44: Merge git://github.com/git-l10n/git-po (2012-08-05 20:51:05 -0700) are available in the git repository at: git://github.com/git-l10n/git-po master for you to fetch changes up to cc2f50dafe28fda6652e1ab78034aae49b495b08: l10n: Update Swedish translation (1168t0f0u) (2012-08-09 06:39:17 +0100) Jiang Xin (3): l10n: Update one message in git.pot l10n: zh_CN.po: update one translation Merge git://github.com/ralfth/git-po-de Peter Krefting (1): l10n: Update Swedish translation (1168t0f0u) Ralf Thielow (1): l10n: de.po: translate 77 new messages Tran Ngoc Quan (1): l10n: vi.po: update one message po/de.po| 488 +++-- po/git.pot | 16 +- po/sv.po| 1413 ++- po/vi.po| 20 +- po/zh_CN.po | 18 +- 5 files changed, 1375 insertions(+), 580 deletions(-) -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] command-list: mention git-credential-* helpers
Jeff King p...@peff.net writes: These commands were never added to the command-list. Adding them makes make check-docs run without complaint. While we're at it, let's capitalize the first letter of their one-line summaries to match the rest of the git manpages. You may want to squash my patch in this one, since they really do the same thing. In any case, thanks for taking care of this, the patches sound good. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: fast-import error: fatal: 'refs/heads/master' - not a valid ref
git --version git version 1.7.11.3 On Wed, Aug 8, 2012 at 9:54 PM, Jeff King p...@peff.net wrote: On Wed, Aug 08, 2012 at 11:25:02AM +0400, Andrey Pavlenko wrote: I'm developing a remote helper which uses the fast-import stream for fetching. When I perform cloning git prints error message - fatal: 'refs/heads/master' - not a valid ref, however the clonning completes normally. Each my fast-import commit command starts with commit refs/heads/master header. What does this error message mean and how can I fix it? What version of git are you using? The only command which produces that exact message is git show-ref, and it is not called by current versions of the cloning process (but it used to be in old versions). -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: fast-import error: fatal: 'refs/heads/master' - not a valid ref
My helper really executes git show-ref before writting the export stream... Thank you for pointing that out and sorry for disturbing. On Thu, Aug 9, 2012 at 12:22 PM, Andrey Pavlenko andrey.a.pavle...@gmail.com wrote: git --version git version 1.7.11.3 On Wed, Aug 8, 2012 at 9:54 PM, Jeff King p...@peff.net wrote: On Wed, Aug 08, 2012 at 11:25:02AM +0400, Andrey Pavlenko wrote: I'm developing a remote helper which uses the fast-import stream for fetching. When I perform cloning git prints error message - fatal: 'refs/heads/master' - not a valid ref, however the clonning completes normally. Each my fast-import commit command starts with commit refs/heads/master header. What does this error message mean and how can I fix it? What version of git are you using? The only command which produces that exact message is git show-ref, and it is not called by current versions of the cloning process (but it used to be in old versions). -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: [PATCH/RFC v2 06/16] t3700: sleep for 1 second, to avoid interfering with the racy code
On 08/08, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: So whether done with sleep or test-chmtime, avoiding a racily clean situation sounds like sweeping a bug in the v5 code in racy situation under the rug to me (unless I am misunderstanding what you are doing with this change and in your explanation, or the test was checking a wrong thing, that is). Even more confused OK, after staring this test for a long time, and going back to 3d1f148 (refresh_index: do not show unmerged path that is outside pathspec, 2012-02-17), I give up. Let me ask the same question in a more direct way. Which part of this test break with your series? test_expect_success 'git add --refresh with pathspec' ' git reset --hard echo foo echo bar echo baz git add foo bar baz H=$(git rev-parse :foo) git rm -f foo echo 100644 $H 3 foo | git update-index --index-info # sleep 1 in the update here ... test-chmtime -60 bar baz expect git add --refresh bar actual test_cmp expect actual git diff-files --name-only actual ! grep bar actual grep baz actual ' We prepare an index with bunch of paths, we make foo unmerged, we smudge bar and baz stat-dirty, so that diff-files would report them, even though their contents match what is recorded in the index. After getting confused a bit myself, I now think here is the problem. The v5 code smudges baz when doing git add --refresh bar. Therefore baz isn't considered stat-dirty by the code, but a racily smudged entry and therefore its content gets checked, thus not showing up in git diff-files. The mtime doesn't get checked anymore as it is used as smudge marker and thus 0. Adding sleep just avoids smudging the entry. The alternative would be to use the size or the crc as smudge marker but I don't think they are good canidates, as they can still be used by the reader to avoid checking the filesystem. Another alternative would be to introduce a CE_SMUDGED flag as it was suggested by Thomas on irc IIRC, but we chose to use the mtime as smudge marker instead. Then we say git add --refresh bar. As far as I know, the output from git add --refresh pathspec is limited to foo: needs merge if and only if foo is covered by pathspec and foo is unmerged. Side note: If --verbose is given to the same command, we also give Unstaged changes after refreshing the index: followed by M foo or U foo if foo does not match the index but not unmerged, or if foo is unmerged, again if and only if foo is covered by pathspec. But that is not how we invoke git add --refresh in this test. So if you are getting a test failure from the test_cmp, wouldn't it mean that your series broke what 3d1f148 did (namely, make sure we report only on paths that are covered by pathspec, in this case bar), as the contents of bar in the working tree matches what is recorded in the index? If the failure you are seeing is that bar appears in the output of git diff-files --name-only, it means that diff-files noticed that bar is stat-dirty after git add --refresh bar. Wouldn't it mean that the series broke git add --refresh bar in such a way that it does not to refresh what it was told to refresh? Another test that could fail after the point you added sleep 1 is that the output from git diff-files --name-only fails to list baz in its output, but with test-chmtime -60 bar baz, we made sure that bar and baz are stat-dirty, and we only refreshed bar and not baz. If that is the case, then would it mean that the series broke git add --refresh bar in such a way that it refreshes something other than what it was told to refresh? In any case, having to change this test in any way smells like there is some breakage in the series; it is not immediately obvious to me that the current test is checking anything wrong as I suspected in the earlier message. So,... I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] add tests for 'git rebase --keep-empty'
Add test cases for 'git rebase --keep-empty' with and without an empty commit already in upstream. The empty commit that is about to be rebased should be kept in both cases. Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- Added another test for when the upstream already has an empty commit. The test case protects the current behavior; I just assume the current behavior is what we want. While writing the test case, I also noticed that an interrupted 'git rebase --keep-empty' can not be continued 'git rebase --continue', but instead needs 'git cherry-pick --continue'. I guess this shouldn't really be surprising given that it's implemented in terms of cherry-pick. This should be fixed once all the different kinds of rebase use the same way of finding the commits to rebase, so I wouldn't worry about fixing this specific problem right now. t/t3401-rebase-partial.sh | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh index 7f8693b..58f4823 100755 --- a/t/t3401-rebase-partial.sh +++ b/t/t3401-rebase-partial.sh @@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' ' git commit --allow-empty -m empty test_commit D git rebase C - test $(git log --format=%s C..) = D + test $(git log --format=%s C..) = D +' + +test_expect_success 'rebase --keep-empty' ' + git reset --hard D + git rebase --keep-empty C + test $(git log --format=%s C..) = D +empty +' + +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' ' + git reset --hard A + git commit --allow-empty -m also-empty + git rebase --keep-empty D + test $(git log --format=%s A..) = also-empty +D +empty ' test_done -- 1.7.11.1.104.ge7b44f1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 resend] gitk: Use an external icon file on Windows
Sebastian Schuberth sschube...@gmail.com writes: On Wed, Aug 8, 2012 at 11:13 PM, Junio C Hamano gits...@pobox.com wrote: Forwarding a misdirected patch to the maintainer who is free to pick or ignore. How am I supposed to know if a patch has been ignored as an oversight (in which case I would resend), or because the maintainer decided not to include it (in which case I would not resend in order to not annoy the maintainer)? By hearing from Paul (or asking directly), perhaps? Personally I am negative on it (nobody on the list asked for the new Git icon as far as I recall), but my voice on this counts just as little as others. I guess most patches on the list come in unasked, so I cannot follow your negative attitude because of this. Many patches do come after a thread of discussion, and at the end of such a discussion I do not often explicitly ask yeah, that sounds like a good way to go, please make it so, so technically it is correct that many patches come unasked, but my nobody asked for was not about your patch. It was about the new logo. Cf. http://thread.gmane.org/gmane.comp.version-control.git/197082 The site may be called official, but Scott exercises fairly large amount of editorial discretion without community input on occasions, and because he is such a nice guy, we let some questionable things go from time to time. Some things on that site are not always the community concensus, not necessarily in the sense that Scott decided against community concensus but in the sense that the community did not feel a need to even have a concensus and Scott chose to go one way. The new logo is one of these things. Some in the community may not be enthusiastic about it and prefer the original, but they do not feel strongly enough to send in patches and gather the community support to fix it. And that goes both ways. If the logo of the official site is not something so important that git-scm.com can change without gaining community consensus first, then it equally is within the editorial discretion of individual git package's maintainer to keep using the original not the updated logo. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add Code Compare v2.80.4 as a merge / diff tool for Windows
Sebastian Schuberth sschube...@gmail.com writes: On Wed, Aug 8, 2012 at 11:26 PM, Junio C Hamano gits...@pobox.com wrote: I do not have a strong reason to vote for or against inclusion of yet another tool as mergetool backends (read: Meh), but what this That sounds almost as if you'd like to keep the number of directly supported mergetools small (I'm not talking about about the length of the list of mergetools in the docs right now). I was always thinking, the more mergetools, the better. Do you think differently? It depends on the pros-and-cons between the cost of forcing many people to see a list of supported backends with yet one more item (the list only grows and rarely shrinks) and the benefit of the subset of users who can now use it. I couldn't measure how widely the particular commercial program is used from your proposed commit log message, and that is where my for or against comes from. patch does to Documentation/merge-config.txt is actively unwelcome. As we discussed earlier in http://thread.gmane.org/gmane.comp.version-control.git/201913/focus=201976 the longer term direction is to reduce the names of tools listed there. I am somewhat saddened to find your name in that thread; you should have been aware of that discussion when you wrote this patch. I still agree that not listing all mergetools in multiple places is a good thing. But doing the whole stuff of extending --tool-help for git-mergetool and git-difftool to return a simple list that can be used in git-completion.bash etc. IMHO is a separate topic and out of scope of this patch. Exactly. If you know that is the long term direction, I would have preferred you _not_ to touch any existing descriptions of the tools (not even changing them to refer to --tool-help) in this patch, in order to avoid unnecessary conflicts with the topic of unifying the list of tool backends, which can be written and cooked separately. 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: [PATCH v2 resend] gitk: Use an external icon file on Windows
On Thu, Aug 9, 2012 at 5:54 PM, Junio C Hamano gits...@pobox.com wrote: Forwarding a misdirected patch to the maintainer who is free to pick or ignore. How am I supposed to know if a patch has been ignored as an oversight (in which case I would resend), or because the maintainer decided not to include it (in which case I would not resend in order to not annoy the maintainer)? By hearing from Paul (or asking directly), perhaps? I did that, in a private mail due to some temporary mailing list problems, but did not get an answer. So my question was referring to Paul. How I should know whether Paul ignores my patch due to an oversight, or because he is not interested? And because of this ambiguity in ignoring a patch I think an explicit NACK should be mandatory if a maintainer is not interested in a patch. And that goes both ways. If the logo of the official site is not something so important that git-scm.com can change without gaining community consensus first, then it equally is within the editorial discretion of individual git package's maintainer to keep using the original not the updated logo. I admit that I was assuming community consensus on the new logo exactly *because* I saw it on git-scm.com. On the other hand, from my experience the Git community is not exactly one where is it particularly easy to get consensus on something, so I completely understand and support Scott's habit to sometimes go one way without consulting the community first. Anyway, I'll include the patch into Git for Windows, because that's what our users ask for. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add Code Compare v2.80.4 as a merge / diff tool for Windows
On Thu, Aug 9, 2012 at 6:03 PM, Junio C Hamano gits...@pobox.com wrote: I still agree that not listing all mergetools in multiple places is a good thing. But doing the whole stuff of extending --tool-help for git-mergetool and git-difftool to return a simple list that can be used in git-completion.bash etc. IMHO is a separate topic and out of scope of this patch. Exactly. If you know that is the long term direction, I would have preferred you _not_ to touch any existing descriptions of the tools (not even changing them to refer to --tool-help) in this patch, in order to avoid unnecessary conflicts with the topic of unifying the list of tool backends, which can be written and cooked separately. To the the best of my knowledge there currently no such topic underway, and even if it was, it would be unclear how long it would take for integration. If I was not touching the existing descriptions of the tools, and a Git version was to be released after accepting my patch but before the --tool-help topic is merged, that would leave the documentation in a wrong state. I was just trying to be consistent by also touching the descriptions, which IMHO is the correct thing to do in the short term, as you yourself say the topic to make use of --tool-help is a long term goal. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 06/16] t3700: sleep for 1 second, to avoid interfering with the racy code
Thomas Gummerer t.gumme...@gmail.com writes: On 08/08, Junio C Hamano wrote: ... Let me ask the same question in a more direct way. Which part of this test break with your series? test_expect_success 'git add --refresh with pathspec' ' git reset --hard echo foo echo bar echo baz git add foo bar baz H=$(git rev-parse :foo) git rm -f foo echo 100644 $H 3foo | git update-index --index-info # sleep 1 in the update here ... test-chmtime -60 bar baz expect git add --refresh bar actual test_cmp expect actual git diff-files --name-only actual ! grep bar actual grep baz actual ' We prepare an index with bunch of paths, we make foo unmerged, we smudge bar and baz stat-dirty, so that diff-files would report them, even though their contents match what is recorded in the index. After getting confused a bit myself, I now think here is the problem. The v5 code smudges baz when doing git add --refresh bar. Therefore baz isn't considered stat-dirty by the code, but a racily smudged entry and therefore its content gets checked, thus not showing up in git diff-files. So in short, the breakage is the last one among the three choices I gave you in my message you are responding to. The user asked to refresh bar so that later diff-files won't report a false change on it, but baz effectively ends up getting refreshed at the same time and a false change is not reported. That breakage is, from the correctness point of view, not a breakage. As the primary purpose of refreshing is to support commands that want to rely on a quick ce_modified() call to tell files that are modified in the working tree since it was last added to the index---you refresh once, and then you call such commands many times without having to worry about having to compare the contents between the indexed objects and the working tree files. But from the performance point of view, which is the whole point of refresh, the behaviour of the new code is dubious. If the user is working in a large working tree (which automatically means large index, the primary reason we are doing this v5 experiment), the user often is working in a deep and narrow subdirectory of it, and a path limited refresh (the test names a specific file bar, but imagine it were . to limit it to the directory the user is working in) may be a cheap way not to bother even checking outside the area the user currently is working in. Also, smudging more entries than necessary to be checked by ce_modified_check_fs() later at runtime may mean that it defeats the refresh once and then compare cheaply many times pattern that is employed by existing scripts. Is the root cause really where the racily-clean so smudge to tell later runtime to check contents bit goes? I am hoping that the issue is not coming from the difference between the current code and your code when they decide to smudge, what entries they decide to smudge and based on what condition. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] rebase -i: use full onto sha1 in reflog
Michael J Gruber g...@drmicha.warpmail.net writes:
'git rebase' uses the full onto sha1 for the reflog message whereas 'git
rebase -i' uses the short sha1. This is not only inconsistent, but can
lead to problems when the reflog is inspected at a later time at which
that abbreviation may have become ambiguous.
Make 'rebase -i' use the full onto sha1, as well.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
I don't remember having sent this out before but have been running with
it since (including tests). I don't see it on the list either.
While not for 1.7.12 obviously, it might still be good to have.
Makes sense, I agree it is not that urgent to include it in 1.7.12,
and I think it is not just good to have but we should have done
so from day one and it cannot break anybody who is reading the
reflog.
Thanks.
git-rebase--interactive.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0d2056f..dbc9de6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -573,7 +573,7 @@ do_next () {
newhead=$(git rev-parse HEAD)
case $head_name in
refs/*)
- message=$GIT_REFLOG_ACTION: $head_name onto $shortonto
+ message=$GIT_REFLOG_ACTION: $head_name onto $onto
git update-ref -m $message $head_name $newhead $orig_head
git symbolic-ref \
-m $GIT_REFLOG_ACTION: returning to $head_name \
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] add tests for 'git rebase --keep-empty'
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes: Add test cases for 'git rebase --keep-empty' with and without an empty commit already in upstream. The empty commit that is about to be rebased should be kept in both cases. Signed-off-by: Martin von Zweigbergk martin.von.zweigbe...@gmail.com --- Added another test for when the upstream already has an empty commit. The test case protects the current behavior; I just assume the current behavior is what we want. Thanks. I think it makes sense, as upstream already has an empty commit together with want to keep empty while rebasing is a strong sign that the user wants to have a history littered with many empty commits. Unlike a normal commit whose patch-id identity may have meaningful significance (i.e. the change to do X is already in, or not yet in, this branch), all the empty commits share the same emptiness, so having one empty somewhere long time ago in the history of where we are transplanting the commits shouldn't be a reason to countermand the want to keep empty wish by the user. And I do not think the conclusion would change even if we changed the definition of identity for empty commits so that two empty commits with the same message are detected as equal. The only semi sensible justification I heard from people who want to have empty commits in their history is to keep in-history notes (e.g. at this point in the series, the code stops to compile, but the next one fixes it, it is possible that we may want to redo the previous patch but I dunno), and it may not make sense to drop such an empty commit under --keep-empty mode if there are similar or identical looking notes in the upstream part of the history. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git svn: handle errors and concurrent commits in dcommit
Eric Wong normalper...@yhbt.net writes: Robert Luberda rob...@debian.org wrote: Eric Wong wrote: + echo PATH=\$PATH\; export PATH $hook + echo svnconf=\$svnconf\ $hook + cat $hook - 'EOF2' + cd work-auto-commits.svn + svn up --config-dir $svnconf That doesn't seem to interact well with users who depend on svn_cmd pointing to something non-standard. Not sure what to do about it, though I have no idea how to change it either. I've tried to source the lib-git-svn.sh file inside the hook, but it sources test-lib.sh, and the latter script doesn't work well if it is sourced by non-test script. Anyway I the part of my original patch unchanged. Ah, so svn_cmd only cares about --config-dir and you already handled that :) I misremembered it also allowed for non-standard SVN installations :x I've pushed your updated patch to my maint branch on git://bogomips.org/git-svn since master has larger pending changes. I should have asked this yesterday, but do you mean you want to have your maint in the upcoming 1.7.12? This does look like a useful thing to do, but does not seem like a regression fix to me. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] rebase -i: use full onto sha1 in reflog
Martin von Zweigbergk martin.von.zweigbe...@gmail.com writes:
On Thu, Aug 9, 2012 at 9:05 AM, Michael J Gruber
g...@drmicha.warpmail.net wrote:
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0d2056f..dbc9de6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -573,7 +573,7 @@ do_next () {
newhead=$(git rev-parse HEAD)
case $head_name in
refs/*)
- message=$GIT_REFLOG_ACTION: $head_name onto $shortonto
+ message=$GIT_REFLOG_ACTION: $head_name onto $onto
After this patch, is there any point in the
shortonto=$(git rev-parse --short $onto)
line just before the context? (I can't see any.)
Good point; the function always exits after reaching this point, so
it should be safe to omit it.
--
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: Please pull git-l10n updates for git v1.7.12-rc2
Jiang Xin worldhello@gmail.com writes: Hi Junio, The following changes since commit bfbf4d477a33be413800f531c3ac0d227f46ab44: Merge git://github.com/git-l10n/git-po (2012-08-05 20:51:05 -0700) are available in the git repository at: git://github.com/git-l10n/git-po master for you to fetch changes up to cc2f50dafe28fda6652e1ab78034aae49b495b08: l10n: Update Swedish translation (1168t0f0u) (2012-08-09 06:39:17 +0100) Thanks, 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 with uucp for deployment
Thomas Hochstein t...@inter.net: Randal L. Schwartz schrieb: I am unaware of *anyone* using uucp these days, UUCP over TCP/IP is still in use for transmission of mail and Usenet news. No, it isn't. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitweb: Add support for gravatar-ssl
Teach gitweb to allow 'avatar' to be set to 'gravatar-ssl', switching
to the https://secure.gravatar.com url form, to avoid mixed content
warnings when serving gitweb over https, with gravatar enabled.
---
I'd alternatively propose always using the https:// form of the URL,
but it seems significantly slower, so it's probably best to let people
pick to continue using the insecure version.
gitweb/gitweb.perl | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3d6a705..d70e8b8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -482,7 +482,7 @@ our %feature = (
# To enable system wide have in $GITWEB_CONFIG
# $feature{'avatar'}{'default'} = ['provider'];
- # where provider is either gravatar or picon.
+ # where provider is either gravatar, gravatar-ssl or picon.
# To have project specific config enable override in $GITWEB_CONFIG
# $feature{'avatar'}{'override'} = 1;
# and in project config gitweb.avatar = provider;
@@ -1117,7 +1117,7 @@ sub configure_gitweb_features {
# if the provider name is invalid or the dependencies are not met,
# reset $git_avatar to the empty string.
our ($git_avatar) = gitweb_get_feature('avatar');
- if ($git_avatar eq 'gravatar') {
+ if ($git_avatar eq 'gravatar' || $git_avatar eq 'gravatar-ssl') {
$git_avatar = '' unless (eval { require Digest::MD5; 1; });
} elsif ($git_avatar eq 'picon') {
# no dependencies
@@ -2079,7 +2079,10 @@ sub gravatar_url {
my $email = lc shift;
my $size = shift;
$avatar_cache{$email} ||=
- http://www.gravatar.com/avatar/; .
+ ($git_avatar eq 'gravatar-ssl' ?
+ https://secure; :
+ http://www;)
+ . .gravatar.com/avatar/ .
Digest::MD5::md5_hex($email) . ?s=;
return $avatar_cache{$email} . $size;
}
@@ -2093,7 +2096,7 @@ sub git_get_avatar {
$opts{-size} ||= 'default';
my $size = $avatar_size{$opts{-size}} || $avatar_size{'default'};
my $url = ;
- if ($git_avatar eq 'gravatar') {
+ if ($git_avatar eq 'gravatar' || $git_avatar eq 'gravatar-ssl') {
$url = gravatar_url($email, $size);
} elsif ($git_avatar eq 'picon') {
$url = picon_url($email);
--
1.7.10
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: Add support for gravatar-ssl
Chris West (Faux) wrote: Teach gitweb to allow 'avatar' to be set to 'gravatar-ssl', switching to the https://secure.gravatar.com url form, to avoid mixed content warnings when serving gitweb over https, with gravatar enabled. --- I'd alternatively propose always using the https:// form of the URL, but it seems significantly slower, so it's probably best to let people pick to continue using the insecure version. Rather than introducing a new, separate setting, wouldn't it be better to have the gravatar URL be automatically decided based on the current gitweb URL? (use the ssl gravar url when the current protocol is https, and the standard http url otherwise) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: Add support for gravatar-ssl
On Thu, Aug 09, 2012 at 09:36:46PM +0200, Giuseppe Bilotta wrote: Chris West (Faux) wrote: Teach gitweb to allow 'avatar' to be set to 'gravatar-ssl', switching to the https://secure.gravatar.com url form, to avoid mixed content warnings when serving gitweb over https, with gravatar enabled. --- I'd alternatively propose always using the https:// form of the URL, but it seems significantly slower, so it's probably best to let people pick to continue using the insecure version. Rather than introducing a new, separate setting, wouldn't it be better to have the gravatar URL be automatically decided based on the current gitweb URL? (use the ssl gravar url when the current protocol is https, and the standard http url otherwise) I don't believe it's possible to reliably detect this kind of thing (on the server side), think ssl terminators / reverse proxies / etc., although you can probably get 99% of the way there, which might be good enough. An alternative would be to dump some JS on the page to convert the SSL links to non-SSL links if the page hasn't been loaded over SSL (according to the browser)? Or do it the other way around and ignore the mixed content warnings that ssl users with no javascript will see. I'd rather just hardcode the https version than do anything with JS. -- 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Let submodule command exit with error status if path does not exist
Previously the exit status of git submodule was zero for various
subcommands even though the user specified an unknown path.
The reason behind that was that they all pipe the output of module_list
into the while loop which then does the action on the paths specified by
the commandline. Since piped commands are run in parallel the status
code of module_list was swallowed.
We work around this by introducing a new function module_list_valid
which is used to check the leftover commandline parameters passed to
module_list.
Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
git-submodule.sh | 19 ++-
t/t7400-submodule-basic.sh | 26 ++
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index aac575e..1fd21da 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -103,13 +103,21 @@ resolve_relative_url ()
echo ${is_relative:+${up_path}}${remoteurl#./}
}
+module_list_ls_files() {
+ git ls-files --error-unmatch --stage -- $@
+}
+
+module_list_valid() {
+ module_list_ls_files $@ /dev/null
+}
+
#
# Get submodule info for registered submodules
# $@ = path to limit submodule list
#
module_list()
{
- git ls-files --error-unmatch --stage -- $@ |
+ module_list_ls_files $@ |
perl -e '
my %unmerged = ();
my ($null_sha1) = (0 x 40);
@@ -434,6 +442,8 @@ cmd_init()
shift
done
+ module_list_valid $@ || exit 1
+
module_list $@ |
while read mode sha1 stage sm_path
do
@@ -532,6 +542,8 @@ cmd_update()
cmd_init -- $@ || return
fi
+ module_list_valid $@ || exit 1
+
cloned_modules=
module_list $@ | {
err=
@@ -929,6 +941,8 @@ cmd_status()
shift
done
+ module_list_valid $@ || exit 1
+
module_list $@ |
while read mode sha1 stage sm_path
do
@@ -996,6 +1010,9 @@ cmd_sync()
;;
esac
done
+
+ module_list_valid $@ || exit 1
+
cd_to_toplevel
module_list $@ |
while read mode sha1 stage sm_path
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index c73bec9..3a40334 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -258,6 +258,27 @@ test_expect_success 'init should register submodule url in
.git/config' '
test_cmp expect url
'
+test_failure_with_unknown_submodule() {
+ test_must_fail git submodule $1 no-such-submodule 2output.err
+ grep ^error: .*no-such-submodule output.err
+}
+
+test_expect_success 'init should fail with unknown submodule' '
+ test_failure_with_unknown_submodule init
+'
+
+test_expect_success 'update should fail with unknown submodule' '
+ test_failure_with_unknown_submodule update
+'
+
+test_expect_success 'status should fail with unknown submodule' '
+ test_failure_with_unknown_submodule status
+'
+
+test_expect_success 'sync should fail with unknown submodule' '
+ test_failure_with_unknown_submodule sync
+'
+
test_expect_success 'update should fail when path is used by a file' '
echo hello expect
@@ -418,10 +439,7 @@ test_expect_success 'moving to a commit without submodule
does not leave empty d
'
test_expect_success 'submodule invalid-path warns' '
-
- git submodule no-such-submodule 2 output.err
- grep ^error: .*no-such-submodule output.err
-
+ test_failure_with_unknown_submodule
'
test_expect_success 'add submodules without specifying an explicit path' '
--
1.7.12.rc2.10.g45a4861
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Let submodule command exit with error status if path does not exist
Heiko Voigt hvo...@hvoigt.net writes:
Previously the exit status of git submodule was zero for various
subcommands even though the user specified an unknown path.
The reason behind that was that they all pipe the output of module_list
into the while loop which then does the action on the paths specified by
the commandline. Since piped commands are run in parallel the status
code of module_list was swallowed.
It is more like that the shell ignores the exit status of command
that is on the upstream side of a pipeline.
We work around this by introducing a new function module_list_valid
which is used to check the leftover commandline parameters passed to
module_list.
Doesn't it slow things down for the normal case, though?
A plausible hack, assuming all the problematic readers of the pipe
are of the form ... | while read mode sha1 stage sm_path, might be
to update module_list () to do something like:
(
git ls-files --error-unmatch ... ||
echo #unmatched
)
and then update the readers to catch #unmatched token, e.g.
module_list $@ |
while read mode sha1 stage sm_path
do
if test $mode = #unmatched
then
... do the necessary error thing ...
continue
fi
... whatever the loop originally did ...
done
One thing to note is that the above is not good if you want to
atomically reject
git submodule foo module1 moduel2
and error the whole thing out without touching module1 (which
exists) because of misspelt module2.
But is it what we want to see happen in these codepaths?
diff --git a/git-submodule.sh b/git-submodule.sh
index aac575e..1fd21da 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -103,13 +103,21 @@ resolve_relative_url ()
echo ${is_relative:+${up_path}}${remoteurl#./}
}
+module_list_ls_files() {
+ git ls-files --error-unmatch --stage -- $@
+}
+
+module_list_valid() {
+ module_list_ls_files $@ /dev/null
+}
+
This is a tangent, but among the 170 hits
git grep -e '^[a-z][a-z0-9A-Z_]* *(' -- './git-*.sh'
gives, about 120 have SP after funcname, i.e.
funcname () {
and 50 don't, i.e.
funcname() {
This file has 12 such definitions, among which 10 are the latter
form. There is no rational reason to choose between the two, but
having two forms in the same project hurts greppability. Updating
the style of existing code shouldn't be done in the same patch, but
please do not make things worse.
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index c73bec9..3a40334 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -258,6 +258,27 @@ test_expect_success 'init should register submodule url
in .git/config' '
test_cmp expect url
'
+test_failure_with_unknown_submodule() {
Likewise, even though inside t/ directory we seem to have more
offenders (190/480 ~ 40%, vs 50/170 ~ 30%).
+ test_must_fail git submodule $1 no-such-submodule 2output.err
+ grep ^error: .*no-such-submodule output.err
+}
I think the latter half already passes with the current code, but
the exit code from git submodule $1 would be corrected with this
patch, which is good.
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
[PATCH] git bash completion handles alias options
git bash completion now handles git alias options. It means that options which change the completion behavior for complete commands will also change the behavior of completion for aliased command. bash completion now handle git alias options. I did a use case in another commit: a checkout alias which only complete local branches, ie. only 'refs' ones, not 'remotes'. With this new alias completion, I added an option to do that in checkout command and hacked _git_checkout complete function. Best, Jerome Jerome Reybert (1): git bash completion handles alias options contrib/completion/git-completion.bash | 31 +-- 1 file changed, 25 insertions(+), 6 deletions(-) -- 1.7.12.rc0.91.gf4edd99 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git bash completion handles alias options
git bash completion now handles git alias options. It means that options
which change the completion behavior for complete commands will also
change the behavior of completion for aliased command.
bash completion now handle git alias options. I did a use case in another
commit: a checkout alias which only complete local branches, ie. only
'refs' ones, not 'remotes'. With this new alias completion, I added an
option to do that in checkout command and hacked _git_checkout complete
function.
This is a draft version, bash usage should be improved. To achieve this:
* a global array $alias_words is filled. As it is global and this file
sourced, this variable can be seen from the terminal
* __git_aliased_command fills $alias_words
* __git_aliased_command must be called differently (not in a subshell)
to fill the global variable. The return value $expansion is
passed by reference as a parameter
* __git_find_on_cmdline now search in $words and $alias_words
Signed-off-by: Jerome Reybert jreyb...@gmail.com
---
contrib/completion/git-completion.bash | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index ffedce7..dfceda0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -23,6 +23,9 @@
#3) Consider changing your PS1 to also show the current branch,
# see git-prompt.sh for details.
+alias_words=()
+alias_cword=0
+
if [[ -n ${ZSH_VERSION-} ]]; then
autoload -U +X bashcompinit bashcompinit
fi
@@ -708,19 +711,22 @@ __git_aliased_command ()
{
local word cmdline=$(git --git-dir=$(__gitdir) \
config --get alias.$1)
+
for word in $cmdline; do
case $word in
\!gitk|gitk)
- echo gitk
+ eval $2='gitk'
return
;;
\!*): shell command alias ;;
- -*) : option ;;
+ -*) : option
+ alias_words=(${alias_words[@]} $word)
+ ((alias_cword++))
+ ;;
*=*): setting env ;;
git): git itself ;;
*)
- echo $word
- return
+ eval $2='$word'
esac
done
}
@@ -739,6 +745,17 @@ __git_find_on_cmdline ()
done
((c++))
done
+ c=0
+ while [ $c -lt $alias_cword ]; do
+ word=${alias_words[c]}
+ for subcommand in $1; do
+ if [ $subcommand = $word ]; then
+ echo $subcommand
+ return
+ fi
+ done
+ ((c++))
+ done
}
__git_has_doubledash ()
@@ -2353,6 +2370,8 @@ _git_whatchanged ()
__git_main ()
{
+ alias_words=()
+ alias_cword=0
local i c=1 command __git_dir
while [ $c -lt $cword ]; do
@@ -2394,8 +2413,8 @@ __git_main ()
local completion_func=_git_${command//-/_}
declare -f $completion_func /dev/null $completion_func return
-
- local expansion=$(__git_aliased_command $command)
+ local expansion=''
+ __git_aliased_command $command expansion
if [ -n $expansion ]; then
completion_func=_git_${expansion//-/_}
declare -f $completion_func /dev/null $completion_func
--
1.7.12.rc0.91.gf4edd99
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 01/13] Move index v2 specific functions to their own file
Thomas Gummerer t.gumme...@gmail.com writes:
/* remember to discard_cache() before reading a different cache! */
int read_index_from(struct index_state *istate, const char *path)
{
...
mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd,
0);
- close(fd);
if (mmap == MAP_FAILED)
die_errno(unable to map index file);
hdr = mmap;
- if (verify_hdr(hdr, mmap_size) 0)
+ if (verify_hdr_version(istate, hdr, mmap_size) 0)
goto unmap;
...
+ if (istate-ops-verify_hdr(mmap, mmap_size) 0)
+ goto unmap;
+ istate-ops-read_index(istate, mmap, mmap_size, fd);
...
+ close(fd);
This looks utterly wrong.
You already have mapped the whole thing, so there is nothing to be
read from fd. You have everything in-core. Leaving fd open and
pass it around looks like it is asking for trouble and confusion.
If you found that an entry you read halfway has an inconsistent crc,
and if you suspect that is because somebody else was writing to the
same index, it is a _sure_ sign that you are not alone, and all the
entries you read so far to the core, even if they weren't touched by
that sombody else when you read them, may be stale, and worse yet,
what you are going to read may be inconsistent with what you've read
and have in-core (e.g. you may have read f before somebody else
that is racing with you have turned it into a directory, and your
next read may find f/d in the index without crc error).
One sane way to avoid reading such an inconsistent state may be to
redo this whole function, starting from the part that calls mmap().
IOW,
do {
fd = open()
mmap = xmmap(fd);
close(fd);
verify_various_fields(mmap);
status = istate-ops-read_index(istate, mmap, mmap_size));
} while (status == READ_AGAIN);
I do not think the pass fd around so that we can redo the mapping
deep inside the callchain is either a good idea or necessary.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 04/13] Add documentation of the index-v5 file format
Thomas Gummerer t.gumme...@gmail.com writes:
+GIT index format
+
+
+== The git index file format
+
+ The git index file (.git/index) documents the status of the files
+ in the git staging area.
+
+ The staging area is used for preparing commits, merging, etc.
The above two are not about index file format. It is an
explanation of what the index is.
+ All binary numbers are in network byte order. Version 5 is described
+ here.
I had to read between these two lines something like
The index file consists of various sections; the sections
appear in the following order in the file.
to make sense of the document.
+ - A 20-byte header consisting of
+
+ sig (32-bits): Signature:
+ The signature is { 'D', 'I', 'R', 'C' } (stands for dircache)
+
+ vnr (32-bits): Version number:
+ The current supported versions are 2, 3, 4 and 5.
+
+ ndir (32-bits): number of directories in the index.
+
+ nfile (32-bits): number of file entries in the index.
+
+ fblockoffset (32-bits): offset to the file block, relative to the
+ beginning of the file.
Ok.
+ - Offset to the extensions.
+ nextensions (32-bits): number of extensions.
+
+ extoffset (32-bits): offset to the extension. (Possibly none, as
+ many as indicated in the 4-byte number of extensions)
OK.
+ headercrc (32-bits): crc checksum for the header and extension
+ offsets
This may have to have the same - section title at the same
level as A 20-byte header and Offset to the ext; as it stands,
it looks as if it is part of Offset to the ext which consists of
12 bytes.
+ - diroffsets (ndir * directory offsets): A directory offset for each
+ of the ndir directories in the index, sorted by pathname (of the
+ directory it's pointing to) (see below). The diroffsets are relative
+ to the beginning of the direntries block. [1]
ndir * diroffsets confused me. I think you meant to say that this
diroffsets section consists of ndir entries of something and that
each of that something is a directory offset. It is unclear how a
directory offset is represented, except that it is relative to the
beginning of direntry block (and it is unclear what and where the
direntry block is from the information given up to this point) and
the reader can guess it is in network byte order (assuming it is a
binary number). Perhaps
diroffsets (ndir entries of directory offset): A 4-byte
offset relative to the beginning of the direntries block
(see below) for each of the ...
and drop the last sentence?
Other tables may want to be adjusted in a similar fashion.
+== Directory offsets (diroffsets)
+
+ diroffset (32-bits): offset to the directory relative to the beginning
+of the index file. There are ndir + 1 offsets in the diroffset table,
+the last is pointing to the end of the last direntry. With this last
+entry, we can replace the strlen when reading each filename, by
+calculating its length with the offsets.
The mention of strlen looks very out of place. The reader may be
able to guess that you want to say that the nth string is between
diroffset[n] and diroffset[n+1], and these strings are densely
packed so strlen(diroffset[n]) and diroffset[n+1]-diroffset[n] are
either the same thing (or with a fixed difference, if each string
is accompanied by some fixed-length data), but it is unclear what
these strings represent, especially because the name of the table
implies that you are talking about directories but strlen talks
about filename.
+== Design explanations
+ ...
+[3] The data of the cache-tree extension and the resolve undo
+extension is now part of the index itself, but if other extensions
+come up in the future, there is no need to change the index, they
+can simply be added at the end.
Interesting. When we added extensions, we said that there is no
need to change the index to add new features, they can simply be
added at the end. Perhaps the file offset table can be added as an
extension to v2 to give us the same bisectability, allowing us a
single entry in-place replacementability, without defining an
entirely different format?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 06/16] t3700: sleep for 1 second, to avoid interfering with the racy code
On 08/09, Junio C Hamano wrote: Thomas Gummerer t.gumme...@gmail.com writes: On 08/08, Junio C Hamano wrote: ... Let me ask the same question in a more direct way. Which part of this test break with your series? test_expect_success 'git add --refresh with pathspec' ' git reset --hard echo foo echo bar echo baz git add foo bar baz H=$(git rev-parse :foo) git rm -f foo echo 100644 $H 3 foo | git update-index --index-info # sleep 1 in the update here ... test-chmtime -60 bar baz expect git add --refresh bar actual test_cmp expect actual git diff-files --name-only actual ! grep bar actual grep baz actual ' We prepare an index with bunch of paths, we make foo unmerged, we smudge bar and baz stat-dirty, so that diff-files would report them, even though their contents match what is recorded in the index. After getting confused a bit myself, I now think here is the problem. The v5 code smudges baz when doing git add --refresh bar. Therefore baz isn't considered stat-dirty by the code, but a racily smudged entry and therefore its content gets checked, thus not showing up in git diff-files. So in short, the breakage is the last one among the three choices I gave you in my message you are responding to. The user asked to refresh bar so that later diff-files won't report a false change on it, but baz effectively ends up getting refreshed at the same time and a false change is not reported. Exactly. That breakage is, from the correctness point of view, not a breakage. As the primary purpose of refreshing is to support commands that want to rely on a quick ce_modified() call to tell files that are modified in the working tree since it was last added to the index---you refresh once, and then you call such commands many times without having to worry about having to compare the contents between the indexed objects and the working tree files. But from the performance point of view, which is the whole point of refresh, the behaviour of the new code is dubious. If the user is working in a large working tree (which automatically means large index, the primary reason we are doing this v5 experiment), the user often is working in a deep and narrow subdirectory of it, and a path limited refresh (the test names a specific file bar, but imagine it were . to limit it to the directory the user is working in) may be a cheap way not to bother even checking outside the area the user currently is working in. That's true, but once we have the partial reader/writer, we do not bother checking outside the area the user is currently working in anyway. Also and probably more importantly, this will only affect a *very* small number of entries, because timestamps outside of the directory in which the user is working in are rarely updated recently and thus racy. Also, smudging more entries than necessary to be checked by ce_modified_check_fs() later at runtime may mean that it defeats the refresh once and then compare cheaply many times pattern that is employed by existing scripts. The new racy code also calls ce_modified_check_fs() only if the size and the stat_crc are not changed. It's true that ce_modified_check_fs() can be called multiple times, when match_stat_crc() is called, but that could be solved by adding an additional flag CE_IS_MODIFIED, which indicates that ce_modified_check_fs() was already run. Is the root cause really where the racily-clean so smudge to tell later runtime to check contents bit goes? I am hoping that the issue is not coming from the difference between the current code and your code when they decide to smudge, what entries they decide to smudge and based on what condition. I just gave it a try using a CE_SMUDGED flag, instead of the mtime as smudge marker, which which this test works without any problems. It doesn't work the other way round, the test as the test doesn't break when using mtime as smudge marker in v2, because we do the ce_modified_check_fs() test earlier. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 07/13] Read resolve-undo data
Thomas Gummerer t.gumme...@gmail.com writes:
Make git read the resolve-undo data from the index.
Since the resolve-undo data is joined with the conflicts in
the ondisk format of the index file version 5, conflicts and
resolved data is read at the same time, and the resolve-undo
data is then converted to the in-memory format.
This, and the next one, are both about reading extension data from
the v2 formatted index, no?
Again, mild NAK.
I think it is a lot more logical for the v5 code to read data stored
in the resolve-undo and cache-tree extensions using the public API
just like other users of these data do, and write out whatever in a
way that is specific to the v5 index format.
If the v5 codepath needs some information that is not exposed to
other users of istate-resolve_undo and istate-cache_tree, then the
story is different, but I do not think that is the case.
Helped-by: Thomas Rast tr...@student.ethz.ch
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
read-cache-v5.c |1 +
resolve-undo.c | 36
resolve-undo.h |2 ++
3 files changed, 39 insertions(+)
diff --git a/read-cache-v5.c b/read-cache-v5.c
index ec1201d..b47398d 100644
--- a/read-cache-v5.c
+++ b/read-cache-v5.c
@@ -494,6 +494,7 @@ static struct directory_entry *read_entries(struct
index_state *istate,
int i;
conflict_queue = read_conflicts(de, mmap, mmap_size, fd);
+ resolve_undo_convert_v5(istate, conflict_queue);
for (i = 0; i de-de_nfiles; i++) {
ce = read_entry(de,
entry_offset,
diff --git a/resolve-undo.c b/resolve-undo.c
index 72b4612..f96c6ba 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -170,3 +170,39 @@ void unmerge_index(struct index_state *istate, const
char **pathspec)
i = unmerge_index_entry_at(istate, i);
}
}
+
+void resolve_undo_convert_v5(struct index_state *istate,
+ struct conflict_entry *ce)
+{
+ int i;
+
+ while (ce) {
+ struct string_list_item *lost;
+ struct resolve_undo_info *ui;
+ struct conflict_part *cp;
+
+ if (ce-entries (ce-entries-flags CONFLICT_CONFLICTED)
!= 0) {
+ ce = ce-next;
+ continue;
+ }
+ if (!istate-resolve_undo) {
+ istate-resolve_undo = xcalloc(1, sizeof(struct
string_list));
+ istate-resolve_undo-strdup_strings = 1;
+ }
+
+ lost = string_list_insert(istate-resolve_undo, ce-name);
+ if (!lost-util)
+ lost-util = xcalloc(1, sizeof(*ui));
+ ui = lost-util;
+
+ cp = ce-entries;
+ for (i = 0; i 3; i++)
+ ui-mode[i] = 0;
+ while (cp) {
+ ui-mode[conflict_stage(cp) - 1] = cp-entry_mode;
+ hashcpy(ui-sha1[conflict_stage(cp) - 1], cp-sha1);
+ cp = cp-next;
+ }
+ ce = ce-next;
+ }
+}
diff --git a/resolve-undo.h b/resolve-undo.h
index 8458769..ab660a6 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -13,4 +13,6 @@ extern void resolve_undo_clear_index(struct index_state *);
extern int unmerge_index_entry_at(struct index_state *, int);
extern void unmerge_index(struct index_state *, const char **);
+extern void resolve_undo_convert_v5(struct index_state *, struct
conflict_entry *);
+
#endif
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 01/13] Move index v2 specific functions to their own file
On 08/09, Junio C Hamano wrote:
Thomas Gummerer t.gumme...@gmail.com writes:
/* remember to discard_cache() before reading a different cache! */
int read_index_from(struct index_state *istate, const char *path)
{
...
mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd,
0);
- close(fd);
if (mmap == MAP_FAILED)
die_errno(unable to map index file);
hdr = mmap;
- if (verify_hdr(hdr, mmap_size) 0)
+ if (verify_hdr_version(istate, hdr, mmap_size) 0)
goto unmap;
...
+ if (istate-ops-verify_hdr(mmap, mmap_size) 0)
+ goto unmap;
+ istate-ops-read_index(istate, mmap, mmap_size, fd);
...
+ close(fd);
This looks utterly wrong.
You already have mapped the whole thing, so there is nothing to be
read from fd. You have everything in-core. Leaving fd open and
pass it around looks like it is asking for trouble and confusion.
If you found that an entry you read halfway has an inconsistent crc,
and if you suspect that is because somebody else was writing to the
same index, it is a _sure_ sign that you are not alone, and all the
entries you read so far to the core, even if they weren't touched by
that sombody else when you read them, may be stale, and worse yet,
what you are going to read may be inconsistent with what you've read
and have in-core (e.g. you may have read f before somebody else
that is racing with you have turned it into a directory, and your
next read may find f/d in the index without crc error).
One sane way to avoid reading such an inconsistent state may be to
redo this whole function, starting from the part that calls mmap().
IOW,
do {
fd = open()
mmap = xmmap(fd);
close(fd);
verify_various_fields(mmap);
status = istate-ops-read_index(istate, mmap, mmap_size));
} while (status == READ_AGAIN);
I do not think the pass fd around so that we can redo the mapping
deep inside the callchain is either a good idea or necessary.
Thanks, that looks better. I'll change it for the re-roll.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 07/13] Read resolve-undo data
On 08/09, Junio C Hamano wrote:
Thomas Gummerer t.gumme...@gmail.com writes:
Make git read the resolve-undo data from the index.
Since the resolve-undo data is joined with the conflicts in
the ondisk format of the index file version 5, conflicts and
resolved data is read at the same time, and the resolve-undo
data is then converted to the in-memory format.
This, and the next one, are both about reading extension data from
the v2 formatted index, no?
Yes, exactly.
Again, mild NAK.
I think it is a lot more logical for the v5 code to read data stored
in the resolve-undo and cache-tree extensions using the public API
just like other users of these data do, and write out whatever in a
way that is specific to the v5 index format.
If the v5 codepath needs some information that is not exposed to
other users of istate-resolve_undo and istate-cache_tree, then the
story is different, but I do not think that is the case.
Sorry it's not clear to me what you mean with using the public API here.
Do you mean using resolve_undo_write() and resolve_undo_read()? I
wouldn't think those two methods would be really useful, as they expect
the data mangled in to a char* or return it as struct strbuf*. And I
don't see the other methods doing something more useful. Or I could
use the resolve-undo string_list directly, and just move the function
to read-cache-v5.c, or am I missing something here?
Helped-by: Thomas Rast tr...@student.ethz.ch
Signed-off-by: Thomas Gummerer t.gumme...@gmail.com
---
read-cache-v5.c |1 +
resolve-undo.c | 36
resolve-undo.h |2 ++
3 files changed, 39 insertions(+)
diff --git a/read-cache-v5.c b/read-cache-v5.c
index ec1201d..b47398d 100644
--- a/read-cache-v5.c
+++ b/read-cache-v5.c
@@ -494,6 +494,7 @@ static struct directory_entry *read_entries(struct
index_state *istate,
int i;
conflict_queue = read_conflicts(de, mmap, mmap_size, fd);
+ resolve_undo_convert_v5(istate, conflict_queue);
for (i = 0; i de-de_nfiles; i++) {
ce = read_entry(de,
entry_offset,
diff --git a/resolve-undo.c b/resolve-undo.c
index 72b4612..f96c6ba 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -170,3 +170,39 @@ void unmerge_index(struct index_state *istate, const
char **pathspec)
i = unmerge_index_entry_at(istate, i);
}
}
+
+void resolve_undo_convert_v5(struct index_state *istate,
+ struct conflict_entry *ce)
+{
+ int i;
+
+ while (ce) {
+ struct string_list_item *lost;
+ struct resolve_undo_info *ui;
+ struct conflict_part *cp;
+
+ if (ce-entries (ce-entries-flags CONFLICT_CONFLICTED)
!= 0) {
+ ce = ce-next;
+ continue;
+ }
+ if (!istate-resolve_undo) {
+ istate-resolve_undo = xcalloc(1, sizeof(struct
string_list));
+ istate-resolve_undo-strdup_strings = 1;
+ }
+
+ lost = string_list_insert(istate-resolve_undo, ce-name);
+ if (!lost-util)
+ lost-util = xcalloc(1, sizeof(*ui));
+ ui = lost-util;
+
+ cp = ce-entries;
+ for (i = 0; i 3; i++)
+ ui-mode[i] = 0;
+ while (cp) {
+ ui-mode[conflict_stage(cp) - 1] = cp-entry_mode;
+ hashcpy(ui-sha1[conflict_stage(cp) - 1], cp-sha1);
+ cp = cp-next;
+ }
+ ce = ce-next;
+ }
+}
diff --git a/resolve-undo.h b/resolve-undo.h
index 8458769..ab660a6 100644
--- a/resolve-undo.h
+++ b/resolve-undo.h
@@ -13,4 +13,6 @@ extern void resolve_undo_clear_index(struct index_state
*);
extern int unmerge_index_entry_at(struct index_state *, int);
extern void unmerge_index(struct index_state *, const char **);
+extern void resolve_undo_convert_v5(struct index_state *, struct
conflict_entry *);
+
#endif
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 07/13] Read resolve-undo data
Thomas Gummerer t.gumme...@gmail.com writes: On 08/09, Junio C Hamano wrote: Thomas Gummerer t.gumme...@gmail.com writes: Make git read the resolve-undo data from the index. Since the resolve-undo data is joined with the conflicts in the ondisk format of the index file version 5, conflicts and resolved data is read at the same time, and the resolve-undo data is then converted to the in-memory format. This, and the next one, are both about reading extension data from the v2 formatted index, no? Yes, exactly. Again, mild NAK. I think it is a lot more logical for the v5 code to read data stored in the resolve-undo and cache-tree extensions using the public API just like other users of these data do, and write out whatever in a way that is specific to the v5 index format. If the v5 codepath needs some information that is not exposed to other users of istate-resolve_undo and istate-cache_tree, then the story is different, but I do not think that is the case. Sorry it's not clear to me what you mean with using the public API here. Do you mean using resolve_undo_write() and resolve_undo_read()? The code that reads from istate-resolve_undo is fine to do the v5 specific conversion, but it does not belong to resolve-undo.c file which is about the resolve-undo extension. Moving it to v5 specific file you added for this topic, read-cache-v5.c, and everything looks more logical. When we taught ls-files to show the paths with resolve-undo data, we didn't add any function to resolve-undo.c that does ls-files's work for it. Instead, ls-files just uses the public API (the data structure you find at the_index.resolve_undo is part of the API) to find what it needs to learn, and I think v5 code can do the same. then the story is different comment refers to a possibilty that v5 code might need something more than callers outside resolve-undo.c can find from its public interface, but I do not think it is the 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
git am and the wrong chunk of ---
Hello, I have some contributors who consistently put their commentary *before* the --- line rather than *after* it, presumably with the notion that it is some kind of cover text. This messes with git am, and so I end up having to edit those posts manually. I have tried git am --scissors and it doesn't seem to solve the problem. Is there any other option which can be used to automatically process such a patch? -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] [git-subtree.sh] Use raw subject and body modifier %B instead of %s%n%n%b for commit
I don't know if it is the right place to post this patch, I have sended an email to the original author apenwarr and have no response. According to https://github.com/apenwarr/git-subtree/blob/master/THIS-REPO-IS-OBSOLETE, this is the place, but https://github.com/git/git/blob/master/contrib/README says different, which is really confusing. Anyway, here I am. Recently, I imported a foreign git project as a sub directory into a main repo which I intend to maintain as primary. Due to the project I imported has its own remote repo which hosted on the github, I expected after a 'git-subtree.sh split' the newly generated subtree branch would be exactly identical to the original branch. Unfortunately, it is not. I have fixed the committer date and make everything looks the same with the original branch, but they just did not end up with same commit sha1 hash. Then, I used `git cat-file -p` to view the raw output of the both commits and found that the commit generate by git-subtree has a extra 'new-line' character appended at the end of the subject which causes the problem. I checked the source and found %s%n%n%b were used to generate the commit message, this works the fine when a commit has a subject as well as a body, but most of my commits only have a subject under which condition a extra 'new-line' character is appended. Instead, a raw subject and body message modifier '%B' should be used. Though I think this patch should be applied by default, but the mistake has been there for a long time, applying this patch may cause the patched git-subtree generate a different branch for those whose subtree branch has already been generated using the old git-subtree. Maybe this should be explained in the help or man page, and add a condition check or a compatible mode somehow. Techlive Zheng (1): subtree.sh: Use raw subject and body modifier %B instead of %s%n%n%b contrib/subtree/git-subtree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] subtree.sh: Use raw subject and body modifier %B instead of %s%n%n%b
%s%n%n%b is not always equal to %B. If the commit msg does not have
a body, this will append an extra new-line character to the msg title
which would cause the splited commit has a new sha1 hash. In most cases,
this does not matter, but for a project which did not merged using this
script initially, the 'split' command would not genereate the same
commits as the orginal which may cause conflicts.
Signed-off-by: Techlive Zheng techlivezh...@gmail.com
---
contrib/subtree/git-subtree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 920c664..5598210 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,7 +296,7 @@ copy_commit()
# We're going to set some environment vars here, so
# do it in a subshell to get rid of them safely later
debug copy_commit {$1} {$2} {$3}
- git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b'
$1 |
+ git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' $1 |
(
read GIT_AUTHOR_NAME
read GIT_AUTHOR_EMAIL
--
1.7.11.4
--
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 am and the wrong chunk of ---
H. Peter Anvin h...@zytor.com writes: Hello, I have some contributors who consistently put their commentary *before* the --- line rather than *after* it, presumably with the notion that it is some kind of cover text. This messes with git am, and so I end up having to edit those posts manually. I have tried git am --scissors and it doesn't seem to solve the problem. Is there any other option which can be used to automatically process such a patch? I hate to be the one who is telling you this, but if the submitter cannot be trained to write supporting material after --- as the convention across git using projects suggest him to do, it is likely that he didn't write supporting material before the scissors, or did not resist the temptation to deviate from the accepted shape of the scissors (e.g. -- 8 --) just to be creative. For that matter, I would be mildly surprised if the material in the middle is usable as is as an acceptable log message from such a submitter X-. So in short, no, --scissors (or -c in short) is not any more magical than the traditional ---. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v3 01/13] Move index v2 specific functions to their own file
On Fri, Aug 10, 2012 at 7:13 AM, Junio C Hamano gits...@pobox.com wrote: By the way, you can only detect such inconsistency when you are lucky enough that you catch the other person in the middle of writing. If the index you are looking at holds a large tree with very many paths, it is possible that there are two large directories, and after you read all entries from one, the other process starts modifying the paths in that directory, without you ever finding it out. If the goal of the topic is to make the index work better in projects with large trees, it may be wise to think about locking the whole thing, so that you do not have to rely on the per-entry crc and you being lucky to detect such a race. The per-entry crc, as far as I understand, may have been introduced primarily to detect on-disk data corruption; it is not a suitable mechanism to detect conflicting accesses. So we acquire the lock just before we need to write, or at the time we open for reading? -- 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: [PATCH] Add Code Compare v2.80.4 as a merge / diff tool for Windows
On Thu, Aug 9, 2012 at 9:22 AM, Sebastian Schuberth sschube...@gmail.com wrote: On Thu, Aug 9, 2012 at 6:03 PM, Junio C Hamano gits...@pobox.com wrote: I still agree that not listing all mergetools in multiple places is a good thing. But doing the whole stuff of extending --tool-help for git-mergetool and git-difftool to return a simple list that can be used in git-completion.bash etc. IMHO is a separate topic and out of scope of this patch. Exactly. If you know that is the long term direction, I would have preferred you _not_ to touch any existing descriptions of the tools (not even changing them to refer to --tool-help) in this patch, in order to avoid unnecessary conflicts with the topic of unifying the list of tool backends, which can be written and cooked separately. To the the best of my knowledge there currently no such topic underway, and even if it was, it would be unclear how long it would take for integration. If I was not touching the existing descriptions of the tools, and a Git version was to be released after accepting my patch but before the --tool-help topic is merged, that would leave the documentation in a wrong state. I was just trying to be consistent by also touching the descriptions, which IMHO is the correct thing to do in the short term, as you yourself say the topic to make use of --tool-help is a long term goal. Thanks Sebastian. I think your patch would be good, so long as we leave the descriptions alone. If you could please re-roll to add the new scriptlet without touching the docs then that would be very helpful. I have a separate patch to update the mergetool documentation for --tool-help which I will send shortly. I have another topic (--symlinks) in next upon which it is based. -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mergetool,difftool: Document --tool-help consistently
Add an entry for --tool-help to the mergetool documentation. Move --tool-help in the difftool documentation so that it is listed immediately after --tool so that it is easier to find. Signed-off-by: David Aguilar dav...@gmail.com --- Based on work in next. This is not urgent as the current difftool topics are cooking and will not be in the current release cycle. Documentation/git-difftool.txt | 6 +++--- Documentation/git-mergetool.txt | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 313d54e..73ca702 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -66,6 +66,9 @@ of the diff post-image. `$MERGED` is the name of the file which is being compared. `$BASE` is provided for compatibility with custom merge tool commands and has the same value as `$MERGED`. +--tool-help:: + Print a list of diff tools that may be used with `--tool`. + --symlinks:: --no-symlinks:: 'git difftool''s default behavior is create symlinks to the @@ -74,9 +77,6 @@ with custom merge tool commands and has the same value as `$MERGED`. Specifying `--no-symlinks` instructs 'git difftool' to create copies instead. `--no-symlinks` is the default on Windows. ---tool-help:: - Print a list of diff tools that may be used with `--tool`. - -x command:: --extcmd=command:: Specify a custom command for viewing diffs. diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index d7207bd..7100237 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -64,6 +64,9 @@ variable `mergetool.tool.trustExitCode` can be set to `true`. Otherwise, 'git mergetool' will prompt the user to indicate the success of the resolution after the custom tool has exited. +--tool-help:: + Print a list of diff tools that may be used with `--tool`. + -y:: --no-prompt:: Don't prompt before each invocation of the merge resolution -- 1.7.12.rc2.16.g034161a -- 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
