Re: feature suggestion: optimize common parts for checkout --conflict=diff3
On Wed, Mar 06, 2013 at 01:32:28PM -0800, Junio C Hamano wrote: We show both sides added, either identically or differently as noteworthy events, but the patched code pushes both sides added identically case outside the conflicting hunk, as if what was added relative to the common ancestor version (in Uwe's case, is it 1-14 that is common, or just 10-14?) is not worth looking at when considering what the right resolution is. If it is not worth looking at what was in the original for the conflicting part, why would we be even using diff3 mode in the first place? I vaguely recall we did this clip to eager as an explicit bugfix at 83133740d9c8 (xmerge.c: diff3 -m style clips merge reduction level to EAGER or less, 2008-08-29). The list archive around that time may give us more contexts. Thanks for the pointer. The relevant threads are: http://article.gmane.org/gmane.comp.version-control.git/94228 and http://thread.gmane.org/gmane.comp.version-control.git/94339 There is not much discussion beyond what ended up in 8313374; both Linus and Dscho question whether level and output format are orthogonal, but seem to accept the explanation you give in the commit message. Having read that commit and the surrounding thread, I think I stand by my argument that zdiff3 is a useful tool to have, as long as the user understands what the hunks mean. It should never replace diff3, but I think it makes sense as a separate format. I was also curious whether it would the diff3/zealous combination would trigger any weird corner cases. In particular, I wanted to know how the example you gave in that commit of: postimage#1: 1234ABCDE789 |/ | / preimage:123456789 | \ |\ postimage#2: 1234AXCYE789 would react with diff3 (this is not the original example, but one with an extra C in the middle of postimage#2, which could in theory be presented as split hunks). However, it seems that we do not do such hunk splitting at all, neither for diff3 nor for the merge representation. -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] git p4: chdir resolves symlinks only for relative paths
Sorry for the late turnaround here is an improved version. Now chdir has an optional argument client_path, if it's true then we don't do os.getcwd. I think that my first patch is also valid too - when the path is absolute no need for getcwd no matter what is the context, when it's relative we have to use os.getcwd() no matter of the context. --- If p4 client is configured to /p/foo which is a symlink: /p/foo - /vol/barvol/projects/foo. Then resolving the symlink will confuse p4: Path /vol/barvol/projects/foo/... is not under client root /p/foo. While AltRoots in p4 client specification can be used as a workaround on p4 side, git-p4 should not resolve symlinks in client paths. chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve relative paths, but as a sideeffect it resolves symlinks too. Now for client paths we don't call os.getcwd(). Signed-off-by: Miklós Fazekas mfaze...@szemafor.com --- git-p4.py | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index 0682e61..2bd8cc2 100755 --- a/git-p4.py +++ b/git-p4.py @@ -68,12 +68,17 @@ def p4_build_cmd(cmd): real_cmd += cmd return real_cmd -def chdir(dir): +def chdir(dir,client_path=False): # P4 uses the PWD environment variable rather than getcwd(). Since we're # not using the shell, we have to set it ourselves. This path could # be relative, so go there first, then figure out where we ended up. +# os.getcwd() will resolve symlinks, so we should avoid it for +# client_paths. os.chdir(dir) -os.environ['PWD'] = os.getcwd() +if client_path: +os.environ['PWD'] = dir +else: + os.environ['PWD'] = os.getcwd() def die(msg): if verbose: @@ -1554,7 +1559,7 @@ class P4Submit(Command, P4UserMap): new_client_dir = True os.makedirs(self.clientPath) -chdir(self.clientPath) +chdir(self.clientPath,client_path=True) if self.dry_run: print Would synchronize p4 checkout in %s % self.clientPath else: -- 1.7.10.2 (Apple Git-33) On Mon, Feb 4, 2013 at 12:08 AM, Pete Wyckoff p...@padd.com wrote: mfaze...@szemafor.com wrote on Tue, 29 Jan 2013 09:37 +0100: If a p4 client is configured to /p/foo which is a symlink to /vol/bar/projects/foo, then resolving symlink, which is done by git-p4's chdir will confuse p4: Path /vol/bar/projects/foo/... is not under client root /p/foo While AltRoots in p4 client specification can be used as a workaround on p4 side, git-p4 should not resolve symlinks in client paths. chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve relative paths, but as a side effect it resolves symlinks too. Now it checks if the dir is relative before resolving. Signed-off-by: Miklós Fazekas mfaze...@szemafor.com --- git-p4.py |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 2da5649..5d74649 100755 --- a/git-p4.py +++ b/git-p4.py @@ -64,7 +64,10 @@ def chdir(dir): # not using the shell, we have to set it ourselves. This path could # be relative, so go there first, then figure out where we ended up. os.chdir(dir) -os.environ['PWD'] = os.getcwd() +if os.path.isabs(dir): +os.environ['PWD'] = dir +else: +os.environ['PWD'] = os.getcwd() def die(msg): if verbose: Thanks, this is indeed a bug and I have reproduced it with a test case. Your patch works, but I think it would be better to separate the callers of chdir(): those that know they are cd-ing to a path from a p4 client, and everybody else. The former should not use os.getcwd(), as you show. I'll whip something up soon, unless you beat me to it. -- Pete -- 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-scm.com/book/ru -- incorrect next link containing a question mark
On Thu, 7 Mar 2013 00:01:31 -0800 (PST) Aleksey Rozhkov ekker...@gmail.com wrote: The page http://git-scm.com/book/ru/ Введение-Первоначальная-настройка-Git contains incorrect link next Now this link to the page http://git-scm.com/book/ru/Введение-Как-получить-помощь? , but this page does not exist I would say ? is just interpreted as a request part of the URL. Indeed, the page source contains: a href=/book/ru/Введение-Установка-Gitprev/a | a href=/book/ru/Введение-Как-получить-помощь?next/a so the question mark is really included verbatim. So, correct link is http://git-scm.com/book/ru/Введение-Как-получить-помощь%3F Good point, thanks. I Cc'ed the main Git list and made the message title a bit more clear. To the Pro Git book maintainer: is it possible to somehow fix the HTML generation script to URL-encode any special characters if they're to appear in an URL? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What I want rebase to do
wor...@alum.mit.edu (Dale R. Worley) writes:
[...snip...]
Isn't that just a very long-winded way of restating what Junio said
earlier:
It was suggested to make it apply the first-parent diff and record
the result, I think. If that were an acceptable approach (I didn't
think about it through myself, though), that would automatically
cover the evil-merge case as well.
You can fake that with something like
git rev-list --first-parent --reverse RANGE_TO_REBASE |
while read rev; do
if git rev-parse $rev^2 /dev/null 21; then
git cherry-pick -n -m1 $rev
git rev-parse $rev^2 .git/MERGE_HEAD
git commit -C$rev
else
git cherry-pick $rev
fi
done
Only tested very lightly. Dealing with octopii, conflicts and actually
preserving the commit's attributes is left as an exercise to the
reader[1].
I still think that the _right_ solution is first redoing the merge on
its original parents and then seeing how the actual merge differs from
that. --preserve-merges has bigger issues though, like Junio said.
Perhaps a new option to git-rebase could trigger the above behavior for
merges, who knows. (It could be called --first-parent.)
[1] If you don't get the sarcasm: that would amount to reinventing
large parts of git-rebase.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] git p4: chdir resolves symlinks only for relative paths
On Thu, Mar 07, 2013 at 09:36:06AM +0100, Miklós Fazekas wrote: Sorry for the late turnaround here is an improved version. Now chdir has an optional argument client_path, if it's true then we don't do os.getcwd. I think that my first patch is also valid too - when the path is absolute no need for getcwd no matter what is the context, when it's relative we have to use os.getcwd() no matter of the context. --- If p4 client is configured to /p/foo which is a symlink: /p/foo - /vol/barvol/projects/foo. Then resolving the symlink will confuse p4: Path /vol/barvol/projects/foo/... is not under client root /p/foo. While AltRoots in p4 client specification can be used as a workaround on p4 side, git-p4 should not resolve symlinks in client paths. chdir(dir) uses os.getcwd() after os.chdir(dir) to resolve relative paths, but as a sideeffect it resolves symlinks too. Now for client paths we don't call os.getcwd(). Signed-off-by: Miklós Fazekas mfaze...@szemafor.com --- git-p4.py | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/git-p4.py b/git-p4.py index 0682e61..2bd8cc2 100755 --- a/git-p4.py +++ b/git-p4.py @@ -68,12 +68,17 @@ def p4_build_cmd(cmd): real_cmd += cmd return real_cmd -def chdir(dir): +def chdir(dir,client_path=False): Style (space after comma): def chdir(dir, client_path=False): # P4 uses the PWD environment variable rather than getcwd(). Since we're # not using the shell, we have to set it ourselves. This path could # be relative, so go there first, then figure out where we ended up. +# os.getcwd() will resolve symlinks, so we should avoid it for +# client_paths. os.chdir(dir) -os.environ['PWD'] = os.getcwd() +if client_path: +os.environ['PWD'] = dir +else: + os.environ['PWD'] = os.getcwd() Indentation seems to have gone a bit wrong here... def die(msg): if verbose: @@ -1554,7 +1559,7 @@ class P4Submit(Command, P4UserMap): new_client_dir = True os.makedirs(self.clientPath) -chdir(self.clientPath) +chdir(self.clientPath,client_path=True) Again, there should be a space after the comma here. if self.dry_run: print Would synchronize p4 checkout in %s % self.clientPath else: -- 1.7.10.2 (Apple Git-33) -- 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: Merging submodules - best merge-base
Den 2013-03-06 19:12:05 skrev Heiko Voigt hvo...@hvoigt.net: On Mon, Feb 25, 2013 at 05:44:05PM +0100, Daniel Bratell wrote: I can phrase this in two ways and I'll start with the short way: Why does a merge of a git submodule use as merge-base the commit that was active in the merge-base of the parent repo, rather than the merge-base of the two commits that are being merged? The long question is: A submodule change can be merged, but only if the merge is a fast-forward which I think is a fair demand, but currently it checks if it's a fast-forward from a commit that might not be very interesting anymore. If two branches A and B split at a point when they used submodule commit S1 (based on S), and both then switched to S2 (also based on S) and B then switched to S21, then it's today not possible to merge B into A, despite S21 being a descendant of S2 and you get a conflict and this warning: warning: Failed to merge submodule S (commits don't follow merge-base) (attempt at ASCII gfx: Submodule tree: S S1 \ \ - S2 -- S21 Main tree: A' (uses S1) --- A (uses S2) \ \ --- B' (uses S2) -- B (uses S21) I would like it to end up as: A' (uses S1) --- A (uses S2) A+ (uses S21) \ / \ --- B' (uses S2) -- B (uses S21)- / And that should be legal since S21 is a descendant of S2. So to summarize what you are requesting: You want a submodule merge be two way in the view of the superproject and calculate the merge base in the submodule from the two commits that are going to be merged? It currently sounds logical but I have to think about it further and whether that might break other use cases. Maybe both could be legal even. The current code can't be all wrong, and this case also seems to be straightforward. /Daniel -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] p4merge: create a virtual base if none available
On 07/03/2013 04:23, David Aguilar wrote:
On Wed, Mar 6, 2013 at 12:32 PM, Kevin Bracey ke...@bracey.fi wrote:
+make_virtual_base() {
+ # Copied from git-merge-one-file.sh.
I think the reasoning behind these patches is good.
How do we feel about this duplication?
Bad.
Should we make a common function in the git-sh-setup.sh,
or is it okay to have a slightly modified version of this
function in two places?
I'd prefer to have a common function, I just didn't know if there was
somewhere appropriate to place it, available from both files. And I'm
going to have to learn a bit more sh to get it right.
Also, the @@DIFF@@ string may not work here.
This is a template string that is replaced by the Makefile.
It does work in git-mergetool--lib.sh, but not in mergetools/p4merge.
We prefer $(command) instead of `command`.
These should be adjusted.
Can the same thing be accomplished using git diff --no-index
so that we do not need a dependency on an external diff command here?
Do these comments still apply if it's a common function in
git-sh-setup.sh that git-one-merge-file.sh will use? I'm wary of
layering violations.
Kevin
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What I want rebase to do
Am 3/7/2013 9:48, schrieb Thomas Rast: wor...@alum.mit.edu (Dale R. Worley) writes: [...snip...] Isn't that just a very long-winded way of restating what Junio said earlier: It was suggested to make it apply the first-parent diff and record the result, I think. If that were an acceptable approach (I didn't think about it through myself, though), that would automatically cover the evil-merge case as well. You can fake that with something like git rev-list --first-parent --reverse RANGE_TO_REBASE | while read rev; do if git rev-parse $rev^2 /dev/null 21; then git cherry-pick -n -m1 $rev git rev-parse $rev^2 .git/MERGE_HEAD git commit -C$rev else git cherry-pick $rev fi done Only tested very lightly. Dealing with octopii, conflicts and actually preserving the commit's attributes is left as an exercise to the reader[1]. I proposed this long ago, but by modifying preserve-merges rather than with a new option (--first-parent): http://thread.gmane.org/gmane.comp.version-control.git/198125 It works very well. I'm using it frequently in the field. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add: Clarify documentation of -A and -u
On Wed, Mar 06, 2013 at 09:10:57AM -0800, Junio C Hamano wrote: I do not know if mentioning what -A does in the description for -u (and vice versa) makes it easier to understand or more confusing (not rhetorical: I suspect some may find it easier and others not). But and the default behaviour will... here _is_ confusing. After reading this patch three times, I still cannot tell what default you are trying to describe. Is it -u without arguments? Is it add without -u nor -A? Is it something else??? I meant the behavior without -A or -u. This could be fixed directly by s/the default behavior will/with neither -A nor -u we/. But we can also keep revising -- there's definitely more room to make this clearer! I suggest new versions at the bottom of this message. I do think that it's important that the reader be able to see clearly what the option does as a contrast with what the command does without the option. Normally in a man page this is how the option is described in the first place -- for example, the next entry in this very man page says Record *only* the fact that the path will be added later, rather than Record the fact that ... These two descriptions suffer from not doing that, so that it's not clear which parts of the behavior they describe are just what 'add' does with no options and which parts are actually due to the option. Whenever you see an incomprehensible technobabble followed by That means, In other words, etc., you (not limited to Greg-you but figuratively everybody) should see if it makes it easier to read to remove everything up to that That means [...] Seems like a reasonable heuristic. -u:: --update:: Update modified and/or removed paths in the index that match pathspec with the current state of the working tree files. No new path is added because this considers only the paths that are already in the index. -A:: --all:: Update the index to record the current state of the working tree files that match pathspec. Note that new paths will be added to the index, in addition to modified and/or removed paths. These are good -- I especially like that they're shorter. I do think they're still likely to be confusing. The lead sentences are hard to tell apart from each other or one's mental model of what 'add' alone does, though the contrasts that follow them help. I also think the lead sentence for '--all' isn't really correct -- we update the index not only for the working tree files that match pathspec, but also where there is no working tree file, only an index entry. (So the sentence actually describes what 'add' with neither option does.) Maybe it's worth taking a step back. The overall taxonomy is * 'add' alone considers matching filenames in the working tree * 'add -u' considers matching filenames in the index * 'add -A' considers matching filenames in both the index and the working tree and in each case we make the index match the working tree on those files. Or, put another way, * 'add' alone modifies and adds files * 'add -u' modifies and removes files * 'add -A' modifies, adds, and removes files Here's a crack at making those distinctions clear. I've also tried to make the descriptions as parallel as possible, as what they're saying is very similar. -u:: --update:: Update the index just where it already has an entry matching pathspec. This removes as well as modifies index entries to match the working tree, but adds no new files. -A:: --all:: Update the index not only where the working tree has a file matching pathspec but also where the index already has an entry. This adds, modifies, and removes index entries to match the working tree. These are the shortest in the discussion so far, and I think they're also the clearest. Then follow both with the If no pathspec paragraph. I just noticed that the paragraph actually needs a small modification to fit '-A', too. New patch below. Greg From: Greg Price pr...@mit.edu Date: Thu, 7 Mar 2013 02:08:21 -0800 Subject: [PATCH] add: Clarify documentation of -A and -u The documentation of '-A' and '-u' is very confusing for someone who doesn't already know what they do. Describe them with fewer words and clearer parallelism to each other and to the behavior of plain 'add'. Also mention the default pathspec for '-A' as well as '-u', because it applies to both. Signed-off-by: Greg Price pr...@mit.edu --- Documentation/git-add.txt | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 388a225..b0944e5 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -100,12 +100,9 @@ apply to the index. See EDITING PATCHES below. -u:: --update:: -
rebase: strange failures to apply patc 3-way
Recently I have observed very strange failures in git rebase that cause it to fail to work automatically in situations where it should trivially be able to do so. In case it matter, here's my setup: git 1.8.2.rc2.4.g7799588 (i.e. git.git master branch) on Mac OS X. The repos clone is on a HFS+ partition, not on a network volume. No gitattributes are being used. Regarding the origin of the repos (I hope it doesn't matter, but just in case): The repository in question used to be a CVS repository; it was decided to switch to Mercurial (not my decision ;-) and I performed the conversion; I first converted it to git using cvs2git (from the cvs2svn suite), then performed some history cleanup, and finally used hg convert to convert it to git. Recently, I have been accessing the repository from git via the gitifyhg tool https://github.com/buchuki/gitifyhg. Anyway, several strange things just happened (and I had similar experiences in the past days and weeks, but that was using an older git, and I thought I was just doing something wrong). Specifically, I wanted to rebase a branch with some experimental commits. The strange things started with this: $ git rebase MY-NEW-BASE First, rewinding head to replay your work on top of it... Applying: SOME COMMIT Applying: SOME OTHER COMMIT ... Applying: COMMIT A Applying: COMMIT B Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... error: Your local changes to the following files would be overwritten by merge: some/source.file Please, commit your changes or stash them before you can merge. Aborting Failed to merge in the changes. Patch failed at 0014 COMMIT B The copy of the patch that failed is found in: /path/to/my/repo/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. Now, what is strange about this is that the failed patch actually applies cleanly: $ patch -p1 /path/to/my/repo/.git/rebase-apply/patch patching file some/source.file $ And there is no subtle merge issue here, either: That patch is the only one to have touched the surrounding code since 1999! There is no source of conflict there! Anyway. The tale gets stranger, as I was trying to do this again (no changes were made to the repos in between, this is a straight continuation from above): $ git rebase --abort $ git rebase MY-NEW-BASE First, rewinding head to replay your work on top of it... Applying: SOME COMMIT Applying: SOME OTHER COMMIT ... Applying: COMMIT A Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... error: Your local changes to the following files would be overwritten by merge: some/othersource.file some/yetanother.file Please, commit your changes or stash them before you can merge. Aborting Failed to merge in the changes. Patch failed at 0013 COMMIT A The copy of the patch that failed is found in: /path/to/my/repo/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. So suddenly it fails to apply the commit A, the one before the previously failing commit. Huh? But again, the failing patch applies cleanly (and after all, rebase was able to apply it in my previous attempt). And again, the patch actually applies cleanly. So one more try: $ git rebase --abort $ git rebase MY-NEW-BASE First, rewinding head to replay your work on top of it... Applying: SOME COMMIT Applying: SOME OTHER COMMIT ... Applying: COMMIT A Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... error: Your local changes to the following files would be overwritten by merge: some/othersource.file Please, commit your changes or stash them before you can merge. Aborting Failed to merge in the changes. Patch failed at 0013 COMMIT A The copy of the patch that failed is found in: /path/to/my/repo/.git/rebase-apply/patch When you have resolved this problem, run git rebase --continue. If you prefer to skip this patch, run git rebase --skip instead. To check out the original branch and stop rebasing, run git rebase --abort. Again it fails in commit A -- but this time, it only thinks one file is problematic. HUH? Again, the patch actually applies cleanly: $ patch -p1 /path/to/my/repo/.git/rebase-apply/patch patching file some/othersource.file patching file some/yetanother.file At this point, things stabilized, and when I now abort and reattempt the merge, it always fails in the same way. This time trying to apply a change to a code comment that was last changed in 1997 (though for one hunk, a few lines before the changed lines, there is a line that was changed in 2008... but
Questions/investigations on git-subtree and tags
Hello everybody
I am trying to use git-subtree to follow a subproject but I have a couple of
problems and I am not sure if I am doing something wrong
Basically I am trying to use a tag on the subproject as my base for the
subproject but subtree doesn't seem to handle that properly
my first attempt was to simply do
git subtree add --squash git://git.buildroot.net/buildroot 2013.02
but subtree refused, telling me that the SHA of the tag is not a valid commit.
Ok it makes sense, though I think this is a very valid use-case...
so I tried
git subtree add git://git.buildroot.net/buildroot 2013.02^{commit}
which was refused because git fetch can't parse the ^{commit} marker.
Again it makes sense, but I really want to start from that tag.
so I tried
git fetch git://git.buildroot.net/buildroot 2013.02
git subtree add --squash FETCH_HEAD
which worked. Ok at that point I am slightly abusing git subtree, but it seems
a valid usage
except that this last attempt causes serious problems when trying to split out
the tree again
the call to git commit-tree within git subtree split complains that the SHA
of the parent
is not a valid commit SHA. Which is true, it's the SHA of the tag.
At this point I am not sure if I am abusing subtree, if I have a legitimate but
unimplemented use-case and how to
fix/implement it.
the squash-commit message only contains the SHA of the tag, should it contain
the SHA of the commit ?
subtree split can only handle commit SHA, should it somehow follow tag SHA
too ? how ?
this is probably a trivial fix for someone with good knowledge of git-subtree
but i'm not there yet, so any hint would be welcomed
Regards
Jérémy Rosen
fight key loggers : write some perl using vim
--
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] In partial SVN merges, the ranges contains additional character *
(adding Sam to the Cc:, I rely on him for SVN merge knowledge)
Jan Pešta jan.pe...@certicon.cz wrote:
See http://www.open.collab.net/community/subversion/articles/merge-info.html
Extract:
The range r30430:30435 that was added to 1.5.x in this merge has a '*'
suffix for 1.5.x\www.
This '*' is the marker for a non-inheritable mergeinfo range.
The '*' means that only the path on which the mergeinfo is explicitly set
has had this range merged into it.
Jan: can you write a better commit message to explain what your
patch fixes/changes, and why we do it?
Something like:
Subject: [PATCH] git svn: ignore partial svn:mergeinfo
explain why we ignore partial svn:mergeinfo in the body
See Documentation/SubmittingPatches for hints. Thanks!
Signed-off-by: Jan Pesta jan.pe...@certicon.cz
---
perl/Git/SVN.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 0ebc68a..74d49bb 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1493,6 +1493,11 @@ sub lookup_svn_merge {
my @merged_commit_ranges;
# find the tip
for my $range ( @ranges ) {
+ if ($range =~ /[*]$/) {
+ warn W:Ignoring partial merge in svn:mergeinfo
+ .dirprop: $source:$range\n;
+ next;
+ }
my ($bottom, $top) = split -, $range;
$top ||= $bottom;
my $bottom_commit = $gs-find_rev_after( $bottom, 1, $top );
--
--
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] format-patch: RFC 2047 says multi-octet character may not be split
Junio,
On Wed, Mar 06, 2013 at 09:47:53AM -0800, Junio C Hamano wrote:
Kirill Smelkov k...@mns.spb.ru writes:
Intro
-
Drop this. We know the beginning part is intro already ;-)
:)
Subject: föö bar
encoding
Subject: =?UTF-8?q?=20f=C3=B6=C3=B6?=
=?UTF-8?q?=20bar?=
is correct, and
Subject: =?UTF-8?q?=20f=C3=B6=C3?= -- NOTE ö is broken here
=?UTF-8?q?=B6=20bar?=
is not, because ö character UTF-8 encoding C3 B6 is split here across
adjacent encoded words.
The above is an important part to keep in the log message.
Everything above that I snipped can be left out for brevity.
As it is now, format-patch does not respect multi-octet charactes may
not be split rule, and so sending patches with non-english subject has
issues:
The problematic case shows in mail readers as fö?? bar.
But the log message lacks crucial bits of information before you
start talking about your solution. Where does it go wrong? What
did the earlier attempt bafc478..41dd00bad miss? This can be fixed
trivially by replacing the above (and the solution section),
perhaps like this:
Even though an earlier attempt (bafc478..41dd00bad) cleaned
up RFC 2047 encoding, pretty.c::add_rfc2047() still decides
where to split the output line by going through the input
one byte at a time, and potentially splits a character in
the middle. A subject line may end up showing like this:
The problematic case shows in mail readers as fö?? bar.
Instead, make the loop grab one _character_ at a time and
determine its output length to see where to break the output
line. Note that this version only knows about UTF-8, but the
logic to grab one character is abstracted out in mbs_chrlen()
function to make it possible to extend it to other encodings.
I agree my description was messy and thanks for reworking and clarifying
it - your version is much better.
I'll use its slight variation for the updated patch.
+ while (len) {
+ /*
+* RFC 2047, section 5 (3):
+*
+* Each 'encoded-word' MUST represent an integral number of
+* characters. A multi-octet character may not be split across
+* adjacent 'encoded- word's.
+*/
+ const unsigned char *p = (const unsigned char *)line;
+ int chrlen = mbs_chrlen(line, len, encoding);
+ int is_special = (chrlen 1) || is_rfc2047_special(*p, type);
/*
* According to RFC 2047, we could encode the special character
@@ -367,16 +376,18 @@ static void add_rfc2047(struct strbuf *sb, const char
*line, int len,
* causes ' ' to be encoded as '=20', avoiding this problem.
*/
+ if (line_len + 2 + (is_special ? 3*chrlen : 1)
max_encoded_length) {
Always have SP around binary operators such as '*' (multiplication).
ok, but note that's just a matter of style, and if one is used to code
formulas, _not_ having SP is more convenient sometimes.
I would actually suggest adding an extra variable encoded_len and
do something like this:
/* =%02X times num_char, or the byte itself */
encoded_len = is_special ? 3 * num_char : 1;
if (max_encoded_length line_len + 2 + encoded_len) {
/* It will not fit---break the line */
...
Right. Actually if we add encoded_len, adding encoded_fmt is tempting
const char *encoded_fmt = is_special ? =%02X: %c;
and then encoding part simplifies to just unconditional
for (i = 0; i chrlen; i++)
strbuf_addf(sb, encoded_fmt, p[i]);
line_len += encoded_len;
You may also want to say what the hardcoded 2 is about in the
comment there.
ok.
diff --git a/utf8.c b/utf8.c
index 8f6e84b..7911b58 100644
--- a/utf8.c
+++ b/utf8.c
@@ -531,3 +531,42 @@ char *reencode_string(const char *in, const char
*out_encoding, const char *in_e
return out;
}
#endif
+
+/*
+ * Returns first character length in bytes for multi-byte `text` according
to
+ * `encoding`.
+ *
+ * - The `text` pointer is updated to point at the next character.
+ * - When `remainder_p` is not NULL, on entry `*remainder_p` is how much
bytes
+ * we can consume from text, and on exit `*remainder_p` is reduced by
returned
+ * character length. Otherwise `text` is treated as limited by NUL.
+ */
+int mbs_chrlen(const char **text, size_t *remainder_p, const char
*encoding)
+{
+ int chrlen;
+ const char *p = *text;
+ size_t r = (remainder_p ? *remainder_p : INT_MAX);
Ugly, and more importantly I suspect this is wrong because size_t is
not signed and INT_MAX is.
Why is it ugly? There is similiar snippet in pick_one_utf8_char():
/*
* A caller that assumes NUL terminated text can choose
Re: Questions/investigations on git-subtree and tags
On Thu, Mar 7, 2013 at 10:25 AM, Jeremy Rosen jeremy.ro...@openwide.fr wrote:
Hello everybody
I am trying to use git-subtree to follow a subproject but I have a couple of
problems and I am not sure if I am doing something wrong
Basically I am trying to use a tag on the subproject as my base for the
subproject but subtree doesn't seem to handle that properly
my first attempt was to simply do
git subtree add --squash git://git.buildroot.net/buildroot 2013.02
but subtree refused, telling me that the SHA of the tag is not a valid commit.
Ok it makes sense, though I think this is a very valid use-case...
so I tried
git subtree add git://git.buildroot.net/buildroot 2013.02^{commit}
which was refused because git fetch can't parse the ^{commit} marker.
Again it makes sense, but I really want to start from that tag.
so I tried
git fetch git://git.buildroot.net/buildroot 2013.02
git subtree add --squash FETCH_HEAD
which worked. Ok at that point I am slightly abusing git subtree, but it
seems a valid usage
except that this last attempt causes serious problems when trying to split
out the tree again
the call to git commit-tree within git subtree split complains that the
SHA of the parent
is not a valid commit SHA. Which is true, it's the SHA of the tag.
At this point I am not sure if I am abusing subtree, if I have a legitimate
but unimplemented use-case and how to
fix/implement it.
the squash-commit message only contains the SHA of the tag, should it contain
the SHA of the commit ?
subtree split can only handle commit SHA, should it somehow follow tag SHA
too ? how ?
this is probably a trivial fix for someone with good knowledge of git-subtree
but i'm not there yet, so any hint would be welcomed
Regards
Jérémy Rosen
Hi Jérémy,
Git subtree ignores tags from the remote repo.
To follow a project in a subdirectory I would use git-subtree add
selecting a branch, not a tag, from the other repo. Then use
git-subtree pull to keep yourself updated.
e.g.
To add:
git subtree add --prefix=$subdir $repo $branch
Then to update:
git subtree pull --prefix=$subdir $repo $branch
If you make any changes on the branch and wanted to push them back you
could do that with:
git subtree pull --prefix=$subdir $repo2 $branch2
$repo2 and $branch2 would be different from $repo and $branch if you
wanted to push to your own fork before submitting a pull request.
--
Paul [W] Campbell
--
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: Questions/investigations on git-subtree and tags
Hi Jérémy, Git subtree ignores tags from the remote repo. is that a design decision or a case of not implemented yet To follow a project in a subdirectory I would use git-subtree add selecting a branch, not a tag, from the other repo. Then use git-subtree pull to keep yourself updated. well... yes, but releases are marked by tags, not branches so what I really want is a tag. I still use git so I have the possibility to update and can traceback what happened later e.g. To add: git subtree add --prefix=$subdir $repo $branch Then to update: git subtree pull --prefix=$subdir $repo $branch ok, that probably works with branches (didn't test) If you make any changes on the branch and wanted to push them back you could do that with: git subtree pull --prefix=$subdir $repo2 $branch2 $repo2 and $branch2 would be different from $repo and $branch if you wanted to push to your own fork before submitting a pull request. shouldn't there be a subtree split somewhere ? IIUC pull is only merge from the remote to my local repo, not the other way round -- Paul [W] Campbell -- 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 svn: ignore partial svn:mergeinfo
Currently this is cosmetic change - the merges are ignored, becuase the methods
(lookup_svn_merge, find_rev_before, find_rev_after) are failing on comparing
text with number.
See http://www.open.collab.net/community/subversion/articles/merge-info.html
Extract:
The range r30430:30435 that was added to 1.5.x in this merge has a '*' suffix
for 1.5.x\www.
This '*' is the marker for a non-inheritable mergeinfo range.
The '*' means that only the path on which the mergeinfo is explicitly set has
had this range merged into it.
Signed-off-by: Jan Pesta jan.pe...@certicon.cz
---
perl/Git/SVN.pm | 5 +
1 file changed, 5 insertions(+)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 0ebc68a..74d49bb 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1493,6 +1493,11 @@ sub lookup_svn_merge {
my @merged_commit_ranges;
# find the tip
for my $range ( @ranges ) {
+ if ($range =~ /[*]$/) {
+ warn W:Ignoring partial merge in svn:mergeinfo
+ .dirprop: $source:$range\n;
+ next;
+ }
my ($bottom, $top) = split -, $range;
$top ||= $bottom;
my $bottom_commit = $gs-find_rev_after( $bottom, 1, $top );
--
1.8.1.msysgit.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Questions/investigations on git-subtree and tags
On Thu, Mar 7, 2013 at 11:05 AM, Jeremy Rosen jeremy.ro...@openwide.fr wrote: Hi Jérémy, Git subtree ignores tags from the remote repo. is that a design decision or a case of not implemented yet I'm not sure. If you imported all the tags from all your subtrees repos, you could easily end up with duplicate tags from different repos. They could be namespaced, but there is no concept of namespace in git-subtree. That even assumes that you can tag a subtree (I've not tried). To follow a project in a subdirectory I would use git-subtree add selecting a branch, not a tag, from the other repo. Then use git-subtree pull to keep yourself updated. well... yes, but releases are marked by tags, not branches so what I really want is a tag. I still use git so I have the possibility to update and can traceback what happened later e.g. To add: git subtree add --prefix=$subdir $repo $branch Then to update: git subtree pull --prefix=$subdir $repo $branch ok, that probably works with branches (didn't test) If you make any changes on the branch and wanted to push them back you could do that with: git subtree pull --prefix=$subdir $repo2 $branch2 $repo2 and $branch2 would be different from $repo and $branch if you wanted to push to your own fork before submitting a pull request. shouldn't there be a subtree split somewhere ? IIUC pull is only merge from the remote to my local repo, not the other way round Oops, that should have been git subtree push, which uses git subtree split internally. -- Paul [W] Campbell -- 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: Questions/investigations on git-subtree and tags
On Thu, Mar 7, 2013 at 12:50 PM, Jeremy Rosen jeremy.ro...@openwide.fr wrote: Git subtree ignores tags from the remote repo. is that a design decision or a case of not implemented yet I'm not sure. If you imported all the tags from all your subtrees repos, you could easily end up with duplicate tags from different repos. They could be namespaced, but there is no concept of namespace in git-subtree. That even assumes that you can tag a subtree (I've not tried). Ok, I can understand that you don't want to import tags for namespace reason, but in that case shouldn't git subtree add refuse to create a subtree when the tag isn't a commit It shouldn't and tries not to, but is limited in it's ability to identify if a refspec points to a commit or not in the remote repo. or if it allows it, what would be the gracefull way to handle that ? I've posted a patch (which is pending a lot of other changes to git-subtree that I'm corralling) that tries to prevent some obvious errors in the refspec. But letting the git fetch used by git-subtree add and git-subtree pull catch the error and report it may be the best option. i'm quite new to git's internals, so I don't really know if/what the right approch would be. note that all those problems seems to disapear when squash is not used I've never really tried using --squash, I don't see that it adds any value for me. -- Paul [W] Campbell -- 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: Questions/investigations on git-subtree and tags
Ok, I can understand that you don't want to import tags for
namespace reason, but in that case shouldn't
git subtree add refuse to create a subtree when the tag isn't a
commit
It shouldn't and tries not to, but is limited in it's ability to
identify if a refspec points to a commit or not in the remote repo.
ok, i've studied a little more
* the target for git subtree add url refspec can only be a remote branch
or tag, since we git fetch
can only target remote refs.
* in case of a branch, git subtree forgets the branch and only use the commit
linked to the branch. for
tags, the fetch part is ok, it's the merge part that fail. adding ^{} at the
right place would probably fix that
I've posted a patch (which is pending a lot of other changes to
git-subtree that I'm corralling) that tries to prevent some obvious
errors in the refspec. But letting the git fetch used by git-subtree
add and git-subtree pull catch the error and report it may be the
best
option.
that's interesting... do you have a link ?
I've never really tried using --squash, I don't see that it adds any
value for me.
my project has a git subtree for a linux kernel and another subtree for
buildroot,
a default .git is about 1.5G, squashing it reduces it to 200M so it's worth it
for 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: Questions/investigations on git-subtree and tags
On Thu, Mar 7, 2013 at 3:15 PM, Jeremy Rosen jeremy.ro...@openwide.fr wrote:
Ok, I can understand that you don't want to import tags for
namespace reason, but in that case shouldn't
git subtree add refuse to create a subtree when the tag isn't a
commit
It shouldn't and tries not to, but is limited in it's ability to
identify if a refspec points to a commit or not in the remote repo.
ok, i've studied a little more
* the target for git subtree add url refspec can only be a remote branch
or tag, since we git fetch
can only target remote refs.
* in case of a branch, git subtree forgets the branch and only use the commit
linked to the branch. for
tags, the fetch part is ok, it's the merge part that fail. adding ^{} at the
right place would probably fix that
I think I tried adding the ^{} syntax, but I don't think it works on
remote repos. Or I couldn't get the right syntax.
I've posted a patch (which is pending a lot of other changes to
git-subtree that I'm corralling) that tries to prevent some obvious
errors in the refspec. But letting the git fetch used by git-subtree
add and git-subtree pull catch the error and report it may be the
best
option.
that's interesting... do you have a link ?
Latest patch:
http://thread.gmane.org/gmane.comp.version-control.git/217257
Prior patch with comments from Junio on what was probably going on
with the old tests:
http://thread.gmane.org/gmane.comp.version-control.git/217227
I've never really tried using --squash, I don't see that it adds any
value for me.
my project has a git subtree for a linux kernel and another subtree for
buildroot,
a default .git is about 1.5G, squashing it reduces it to 200M so it's worth
it for me :)
If disk space is the issue, or bandwidth for initial cloning, then
sure, but I thought Git was efficient enough that a large repo
wouldn't give much of a performance hit. Unless you use git-subtree
split or push, they are slow.
If git-subtree split could be optimised then --squash wouldn't be
needed as much. It does take an age compared to other Git operations.
--
Paul [W] Campbell
--
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: Questions/investigations on git-subtree and tags
I think I tried adding the ^{} syntax, but I don't think it works on
remote repos. Or I couldn't get the right syntax.
indeed, it doesn't work on fetch, but it could be used somewhere between the
fetch and the commit-tree to move from the ref to the associated commit
Latest patch:
http://thread.gmane.org/gmane.comp.version-control.git/217257
oh, that patch, yes I found it while looking around it is a step in the right
direction but it doesn't help in my case since i'm using a valid remote ref
that can be fetched
(on a side note you could use git ls-remote to check for the remote ref and
avoid a fetch in case of an incorrect ref, but that's just a detail)
I just tested with it and here is what happens
git subtree add --squash -P br2 git://git.buildroot.net/buildroot 2013.02 =
works ok, br2 is created
however the message of the squash commit is
Squashed 'br2/' content from commit f1d2c19
git-subtree-dir: br2
git-subtree-split: f1d2c19091e1c2ef803ec3267fe71cf6ce7dd948
which is not correct :
git ls-remote git://git.buildroot.net/buildroot 2013.02
f1d2c19091e1c2ef803ec3267fe71cf6ce7dd948refs/tags/2013.02
git ls-remote git://git.buildroot.net/buildroot 2013.02^{}
15ace1a845c9e7fc65b648bbaf4dd14e03d938fdrefs/tags/2013.02^{}
as you can see git subtee thinks it splited from the tag SHA instead of the
tag's commit SHA
this is incorrect because the tag isn't here, and at split time git subtree
won't be able to find the correct ancestor. We just need to make sure we use
the tag's commit instead
of the tag
changing
revs=FETCH_HEAD
to
revs=FETCH_HEAD^{}
in cmd_add_repository
seems to fix it, both for remote branch pull and remote tag pull
we still have a bug lurking around it's the case where the user does the fetch
himself then use subtree add with a tag SHA. but let's discuss problems one at
a time :)
--
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] setup.c: Fix prefix_pathspec from looping pass end of string
The previous code was assuming length ends at either `)` or `,`, and was
not handling the case where strcspn returns length due to end of string.
So specifying :(top as pathspec will cause the loop to go pass the end
of string.
Signed-off-by: Andrew Wong andrew.k...@gmail.com
---
setup.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 1dee47e..f4c4e73 100644
--- a/setup.c
+++ b/setup.c
@@ -207,9 +207,11 @@ static const char *prefix_pathspec(const char *prefix, int
prefixlen, const char
*copyfrom *copyfrom != ')';
copyfrom = nextat) {
size_t len = strcspn(copyfrom, ,));
- if (copyfrom[len] == ')')
+ if (copyfrom[len] == '\0')
nextat = copyfrom + len;
- else
+ else if (copyfrom[len] == ')')
+ nextat = copyfrom + len;
+ else if (copyfrom[len] == ',')
nextat = copyfrom + len + 1;
if (!len)
continue;
--
1.8.2.rc0.22.gb3600c3
--
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: feature suggestion: optimize common parts for checkout --conflict=diff3
Jeff King p...@peff.net writes: I was also curious whether it would the diff3/zealous combination would trigger any weird corner cases. In particular, I wanted to know how the example you gave in that commit of: postimage#1: 1234ABCDE789 |/ | / preimage:123456789 | \ |\ postimage#2: 1234AXCYE789 would react with diff3 (this is not the original example, but one with an extra C in the middle of postimage#2, which could in theory be presented as split hunks). However, it seems that we do not do such hunk splitting at all, neither for diff3 nor for the merge representation. Without thinking about it too deeply,... I think the RCS merge _could_ show it as 1234AB=XCD=YE789 without losing any information (as it is already discarding what was in the original in the part that is affected by the conflict, i.e. 56 was there). Let's think aloud how diff3 -m _should_ split this. The most straight-forward representation would be 1234ABCDE|56=AXCYE789, that is, where 56 was originally there, one side made it to ABCDE and the other AXCYE. You could make it 1234AB|5=AXC|=CDE|6=YE789, and that is technically correct (what there were in the shared original for the conflicted part is 5 and then 6), but the representation pretends that it knows more than there actually is information, which may be somewhat misleading. All these three are equally plausible split of the original 56: 1234AB|=AXC|=CDE|56=YE789 1234AB|5=AXC|=CDE|6=YE789 1234AB|56=AXC|=CDE|=YE789 and picking one over others would be a mere heuristic. All three are technically correct representations and it is just the matter of which one is the easiest to understand. So, this is the kind of misleading but not incorrect. In all these cases, the middle part would look like this: ours C ||| base === C theirs in order to honor the explicit I want to view all three versions to examine the situation aka --conflict=diff3 option. We cannot reduce it to just C. That will make it not just misleading but is actively wrong. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What I want rebase to do
Thomas Rast tr...@student.ethz.ch writes: I still think that the _right_ solution is first redoing the merge on its original parents and then seeing how the actual merge differs from that. I think that is what was suggested in http://article.gmane.org/gmane.comp.version-control.git/198316 Perhaps a new option to git-rebase could trigger the above behavior for merges, who knows. (It could be called --first-parent.) Yeah, I think that is what the old thread concluded to be the way to move forward: http://thread.gmane.org/gmane.comp.version-control.git/198125 I'll throw it in to the leftover bits. http://git-blame.blogspot.com/2013/02/more-leftover-bits.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
Re: feature suggestion: optimize common parts for checkout --conflict=diff3
On Thu, Mar 07, 2013 at 09:26:05AM -0800, Junio C Hamano wrote: Without thinking about it too deeply,... I think the RCS merge _could_ show it as 1234AB=XCD=YE789 without losing any information (as it is already discarding what was in the original in the part that is affected by the conflict, i.e. 56 was there). Right, I think that is sane, though we do not do that at this point. Let's think aloud how diff3 -m _should_ split this. The most straight-forward representation would be 1234ABCDE|56=AXCYE789, that is, where 56 was originally there, one side made it to ABCDE and the other AXCYE. Yes, that is what diff3 would do now (because it does not do any hunk refinement at all), and should continue doing. You could make it 1234AB|5=AXC|=CDE|6=YE789, and that is technically correct (what there were in the shared original for the conflicted part is 5 and then 6), but the representation pretends that it knows more than there actually is information, which may be somewhat misleading. All these three are equally plausible split of the original 56: 1234AB|=AXC|=CDE|56=YE789 1234AB|5=AXC|=CDE|6=YE789 1234AB|56=AXC|=CDE|=YE789 and picking one over others would be a mere heuristic. All three are technically correct representations and it is just the matter of which one is the easiest to understand. So, this is the kind of misleading but not incorrect. Yes, I agree it is a heuristic about which part of a split hunk to place deleted preimage lines in. Conceptually, I'm OK with that; the point of zdiff3 is to try to make the conflict easier to read by eliminating possibly uninteresting parts. It doesn't have to be right all the time; it just has to be useful most of the time. But it's not clear how true that would be in real life. I think this is somewhat a moot point, though. We do not do this splitting now. If we later learn to do it, there is nothing to say that zdiff3 would have to adopt it also; it could stop at a lower zealous-level than the regular merge markers. I think I'd want to experiment with it and see some real-world examples before making a decision on that. In all these cases, the middle part would look like this: ours C ||| base === C theirs in order to honor the explicit I want to view all three versions to examine the situation aka --conflict=diff3 option. We cannot reduce it to just C. That will make it not just misleading but is actively wrong. I'm not sure I agree. In this output (which does the zealous simplification, the splitting, and arbitrarily assigns deleted preimage to the first of the split hunks): 1234AB|56=XCD|YE789 I do not see the promotion of C to already resolved, you cannot tell if it was really in the preimage or not as any more or less misleading or wrong than that of A or E. It is no more misleading than what the merge-marker case would do, which would be: 1234AB=XCD=YE789 The wrong thing to me is the arbitrary choice about how to distribute the preimage lines. In this example, it is not a big deal for the heuristic to be wrong; you can see both of the hunks. But if C is long, and you do not even see D=Y while resolving B=X, seeing the preimage there may become nonsensical. But again, we don't do this splitting now. So I don't think it's something that should make or break a decision to have zdiff3. Without the splitting, I can see it being quite useful. I'm going to carry the patch in my tree for a while and try using it in practice for a while. -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: feature suggestion: optimize common parts for checkout --conflict=diff3
Junio C Hamano gits...@pobox.com writes: You could make it 1234AB|5=AXC|=CDE|6=YE789, and that is technically correct (what there were in the shared original for the conflicted part is 5 and then 6), but the representation pretends that it knows more than there actually is information, which may be somewhat misleading. All these three are equally plausible split of the original 56: 1234AB|=AXC|=CDE|56=YE789 1234AB|5=AXC|=CDE|6=YE789 1234AB|56=AXC|=CDE|=YE789 and picking one over others would be a mere heuristic. All three are technically correct representations and it is just the matter of which one is the easiest to understand. So, this is the kind of misleading but not incorrect. I forgot to say that youu could even do something silly like: 1234AB|=AXC|56=CDE|=YE789 ;-) In all these cases, the middle part would look like this: ours C ||| base === C theirs in order to honor the explicit I want to view all three versions to examine the situation aka --conflict=diff3 option. We cannot reduce it to just C. That will make it not just misleading but is actively wrong. I also forgot to say that the issue is the same to reduce 1234AB|=AXC|=CDE|56=YE789 to 1234A|=AB|=XC|=CD|56=YE|=E789 which is unconditionally correct and then for all x reduce x|=x to x, yielding 1234AB|=XCD|56=YE789 which your zealous-diff3 would do. So squashing that C|=C in the middle would be consistent if you take the zealous-diff3 route. But again, that is discarding the information of the original, which the user explicitly asked from diff3 -m, i.e. show all three to examine the situation. If the user wants to operate _without_ the original, the user would have asked for RCS merge style output, so I am still not sure if that is a sensible mode of operation for diff3 to begin with. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool
On 07/03/2013 09:23, Junio C Hamano wrote: If p4merge GUI labels one side clearly as theirs and the other ours, and the way we feed the inputs to it makes the side that is actually ours appear in p4merge GUI labelled as theirs, then I do not think backward compatibility argument does not hold water. It is just correcting a longstanding 3-4 year old bug in a tool that nobody noticed. It's not quite that clear-cut. Some years ago, and before p4merge was added as a Git mergetool, P4Merge was changed so its main GUI text says left and right instead of theirs and ours when invoked manually. But it appears that's as far as they went. It doesn't seem any of its asymmetric diff display logic was changed; it works better with ours on the right, and the built-in help all remains written on the theirs/ours basis. And even little details like the icons imply it (a square for the base, a downward-pointing triangle for their incoming stuff, and a circle for the version we hold). For people who are very used to the way p4merge shows the merged contents by theirs-base-yours order in side-by-side view, I do not think it is unreasonable to introduce the mergetool.$name.reverse configuration variable and teach the mergetool frontend to pay attention to it. That will allow them to see their merge in reverse order even when they are using a backend other than p4merge. With such a mechanism in place, by default, we can just declare that mergetool.p4merge.reverse is true when unset, while making mergetool.$name.reverse for all the other tools default to false. People who are already used to the way our p4merge integration works can set mergetool.p4merge.reverse to false explicitly to retain the historical behaviour that you are declaring buggy with such a change. I like this idea as a user - having made this change to p4merge, it does throw me when I decide to attempt a particularly tricky merge with bc3 instead, and get the other order. The user config options you suggest sound good to me. For completion on this idea, I'd suggest difftool.xxx.reverse, to allow the orientation for 0- and 1-revision diffs to be chosen - allow the implied working tree version to be on the left or right. That would allow ours-theirs order, which some would view as being more consistent with the ours-base-theirs default for mergetool. Would it be going too far to also have xxxtool.reverse to choose the global default? Then the choice hierarchy would be xxxtool.xxx.reverse if set optional inbuilt tool preference xxxtool.reverse if set false. So the user could request a global swap, except that they'd have to explicitly override any tools that have a preferred orientation. My only reservation is that I assume it would be implemented by swapping what's passed in $LOCAL and $REMOTE. Which seems a bit icky: $LOCAL=a.REMOTE.1234.c. On the other hand, $LOCAL and $REMOTE are already not very meaningful names for difftool... Maybe we should change to using $LEFT and $RIGHT, acknowledging the existing difftool situation, and that the user can now swap merges too. I'd be happy to prepare a fuller patch on this sort of basis. Kevin -- 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] Fix revision walk for commits with the same dates
git rev-list A^! --not B provides wrong answer if all commits in the range A..B had the same commit times and there are more then 8 of them. This commits fixes the logic in still_interesting function to prevent this error. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- revision.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/revision.c b/revision.c index ef60205..cf620c6 100644 --- a/revision.c +++ b/revision.c @@ -709,7 +709,7 @@ static int still_interesting(struct commit_list *src, unsigned long date, int sl * Does the destination list contain entries with a date * before the source list? Definitely _not_ done. */ - if (date src-item-date) + if (date = src-item-date) return SLOP; /* -- 1.8.2.rc2 -- 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: feature suggestion: optimize common parts for checkout --conflict=diff3
Jeff King p...@peff.net writes: I'm not sure I agree. In this output (which does the zealous simplification, the splitting, and arbitrarily assigns deleted preimage to the first of the split hunks): 1234AB|56=XCD|YE789 I do not see the promotion of C to already resolved, you cannot tell if it was really in the preimage or not as any more or less misleading or wrong than that of A or E. It is no more misleading than what the merge-marker case would do, which would be: 1234AB=XCD=YE789 That is exactly my point and I think we are in complete agreement. While the intended difference between RCS merge and diff3 -m is for the latter not to lose information on the original, zealous-diff3 chooses to lose information in both sides added, identically case. Where we differ is if such information loss is a good thing to have. We could say both sides added, identically is auto-resolved when you use the zealous option, and do so regardless of how the merge conflicts are presented. Then it becomes perfectly fine to eject A and E out of the conflicted block and merge them to be part of pre/post contexts. The same goes for reducing C|=C to C. As long as we clearly present the users what the option does and what its implications are, it is not bad to have such an option, I think. The wrong thing to me is the arbitrary choice about how to distribute the preimage lines. Yeah, but that is not diff3 -m vs zealous-diff3 issue, is it? If you value the original and want to show it somewhere, you cannot avoid making the choice whether you are zealous or not if you split such a hunk. -- 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] line-log: Fix sparse warnings
Sparse issues the following warnings:
line-log.c:17:6: warning: symbol 'range_set_grow' was not declared. Should
it be static?
line-log.c:25:6: warning: symbol 'range_set_init' was not declared. Should
it be static?
line-log.c:33:6: warning: symbol 'range_set_release' was not declared.
Should it be static?
line-log.c:41:6: warning: symbol 'range_set_copy' was not declared. Should
it be static?
line-log.c:47:6: warning: symbol 'range_set_move' was not declared. Should
it be static?
line-log.c:58:6: warning: symbol 'range_set_append' was not declared.
Should it be static?
line-log.c:299:46: warning: Using plain integer as NULL pointer
line-log.c:444:12: warning: symbol 'parse_loc' was not declared. Should it
be static?
line-log.c:681:49: warning: Using plain integer as NULL pointer
line-log.c:684:58: warning: Using plain integer as NULL pointer
builtin/log.c:120:57: warning: Using plain integer as NULL pointer
builtin/log.c:120:60: warning: Using plain integer as NULL pointer
In order to suppress the ... was not declared warnings, we simply
add the static modifier to the declarations of those symbols, since
they do not need more than file scope.
In order to suppress the NULL pointer warnings, we simply replace
the use of the integer constant zero as a representation of the null
pointer with the NULL symbol.
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---
Hi Thomas,
If you need to re-roll the patches in your 'tr/line-log' branch, could
you please squash these changes in. [Note that this patch applies to the
tip of the pu branch ;-) ]
Also, I noticed some things in passing, viz:
971 static void load_tree_desc(struct tree_desc *desc, void **tree,
972 const unsigned char *sha1)
973 {
974 unsigned long size;
975 *tree = read_object_with_reference(sha1, tree_type, size,
NULL);
976 if (!tree)
977 die(Unable to read tree (%s), sha1_to_hex(sha1));
978 init_tree_desc(desc, *tree, size);
979 }
The die() on line 977 will never trigger, since (given that !tree
holds) we will already have dumped core on line 975! ;-)
1401 int line_log_filter(struct rev_info *rev)
1402 {
1403 struct commit *commit;
1404 struct commit_list *list = rev-commits;
1405 struct commit_list *out = NULL, *cur = NULL;
1406
1407 list = rev-commits;
1408 while (list) {
Note that the assignment on line 1407 is redundant and can be
removed; list has been initialized to the same value in it's
declaration on line 1404.
HTH
ATB,
Ramsay Jones
builtin/log.c | 2 +-
line-log.c| 20 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index c5d2313..a551d8d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -117,7 +117,7 @@ static void cmd_log_init_finish(int argc, const char
**argv, const char *prefix,
{
struct userformat_want w;
int quiet = 0, source = 0, mailmap = 0;
- static struct line_opt_callback_data line_cb = {0, 0,
STRING_LIST_INIT_DUP};
+ static struct line_opt_callback_data line_cb = {NULL, NULL,
STRING_LIST_INIT_DUP};
const struct option builtin_log_options[] = {
OPT_BOOLEAN(0, quiet, quiet, N_(suppress diff output)),
diff --git a/line-log.c b/line-log.c
index a74bbaf..4d01798 100644
--- a/line-log.c
+++ b/line-log.c
@@ -14,7 +14,7 @@
#include userdiff.h
#include line-log.h
-void range_set_grow (struct range_set *rs, size_t extra)
+static void range_set_grow (struct range_set *rs, size_t extra)
{
ALLOC_GROW(rs-ranges, rs-nr + extra, rs-alloc);
}
@@ -22,7 +22,7 @@ void range_set_grow (struct range_set *rs, size_t extra)
/* Either initialization would be fine */
#define RANGE_SET_INIT {0}
-void range_set_init (struct range_set *rs, size_t prealloc)
+static void range_set_init (struct range_set *rs, size_t prealloc)
{
rs-alloc = rs-nr = 0;
rs-ranges = NULL;
@@ -30,7 +30,7 @@ void range_set_init (struct range_set *rs, size_t prealloc)
range_set_grow(rs, prealloc);
}
-void range_set_release (struct range_set *rs)
+static void range_set_release (struct range_set *rs)
{
free(rs-ranges);
rs-alloc = rs-nr = 0;
@@ -38,13 +38,13 @@ void range_set_release (struct range_set *rs)
}
/* dst must be uninitialized! */
-void range_set_copy (struct range_set *dst, struct range_set *src)
+static void range_set_copy (struct range_set *dst, struct range_set *src)
{
range_set_init(dst, src-nr);
memcpy(dst-ranges, src-ranges, src-nr*sizeof(struct range_set));
dst-nr = src-nr;
}
-void range_set_move (struct range_set *dst, struct range_set *src)
+static void range_set_move (struct range_set *dst, struct range_set *src)
{
range_set_release(dst);
dst-ranges = src-ranges;
@@ -55,7 +55,7 @@ void
Re: feature suggestion: optimize common parts for checkout --conflict=diff3
On Thu, Mar 07, 2013 at 10:40:46AM -0800, Junio C Hamano wrote: Where we differ is if such information loss is a good thing to have. We could say both sides added, identically is auto-resolved when you use the zealous option, and do so regardless of how the merge conflicts are presented. Then it becomes perfectly fine to eject A and E out of the conflicted block and merge them to be part of pre/post contexts. The same goes for reducing C|=C to C. As long as we clearly present the users what the option does and what its implications are, it is not bad to have such an option, I think. Exactly. I do think it has real-world uses (see the example script I posted yesterday), but it would never replace diff3. I'm going to try it out for a bit. As I mentioned yesterday, I see those sorts of cherry-pick-with-something-on-top conflicts when I am rebasing onto or merging my topics into what you have picked up from the same topic on the list. I think the code in Uwe's patch looked fine, but it definitely needs a documentation change to explain the new mode and its caveats. I'd also be happy with a different name, if you think it implies that it is too related to zdiff3, but I cannot think of anything better at the moment. The wrong thing to me is the arbitrary choice about how to distribute the preimage lines. Yeah, but that is not diff3 -m vs zealous-diff3 issue, is it? If you value the original and want to show it somewhere, you cannot avoid making the choice whether you are zealous or not if you split such a hunk. Right, but I meant that we would never split a hunk like that with diff3, because we would not do any hunk refinement at all. Splitting a hunk with merge is OK, because the where does the preimage go problem does not exist there. zdiff3 is the only problematic case, because it would be the only one that (potentially) splits and cares about how the preimage maps to each hunk. But we can deal with that if and when we ever do such splitting. -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: Merging submodules - best merge-base
On Thu, Mar 07, 2013 at 10:49:09AM +0100, Daniel Bratell wrote: Den 2013-03-06 19:12:05 skrev Heiko Voigt hvo...@hvoigt.net: On Mon, Feb 25, 2013 at 05:44:05PM +0100, Daniel Bratell wrote: A submodule change can be merged, but only if the merge is a fast-forward which I think is a fair demand, but currently it checks if it's a fast-forward from a commit that might not be very interesting anymore. If two branches A and B split at a point when they used submodule commit S1 (based on S), and both then switched to S2 (also based on S) and B then switched to S21, then it's today not possible to merge B into A, despite S21 being a descendant of S2 and you get a conflict and this warning: warning: Failed to merge submodule S (commits don't follow merge-base) (attempt at ASCII gfx: Submodule tree: S S1 \ \ - S2 -- S21 Main tree: A' (uses S1) --- A (uses S2) \ \ --- B' (uses S2) -- B (uses S21) I would like it to end up as: A' (uses S1) --- A (uses S2) A+ (uses S21) \ / \ --- B' (uses S2) -- B (uses S21)- / And that should be legal since S21 is a descendant of S2. So to summarize what you are requesting: You want a submodule merge be two way in the view of the superproject and calculate the merge base in the submodule from the two commits that are going to be merged? It currently sounds logical but I have to think about it further and whether that might break other use cases. Maybe both could be legal even. The current code can't be all wrong, and this case also seems to be straightforward. Ok I have thought about it further and I did not come up with a simple (and stable) enough strategy that would allow your use case to merge cleanly without user interaction. The problem is that your are actually doing a rewind from base to both tips. The fact that a rewind is there makes git suspicious and we simply give up. IMO, thats the right thing to do in such a situation. What should a merge strategy do? It infers from two changes what the final intention might be. For submodules we can do that when the changes on both sides point forward. Since thats the typical progress of development. If not there is some reason for it we do not know about. So the merge gives up. Please see this post about why we need to forbid rewinds from the initial design discussion: http://article.gmane.org/gmane.comp.version-control.git/149003 I am not saying that the current behavior is perfect but I think a merge containing a rewind needs user support. We could give the user a hint and say: Hey I gave up but the two sides are contained in each other and this is the commit containing both. Then the user can choose to use that suggested solution. We already do the same for the merge commit search. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool
Kevin Bracey ke...@bracey.fi writes: On 07/03/2013 09:23, Junio C Hamano wrote: If p4merge GUI labels one side clearly as theirs and the other ours, and the way we feed the inputs to it makes the side that is actually ours appear in p4merge GUI labelled as theirs, then I do not think backward compatibility argument does not hold water. It is just correcting a longstanding 3-4 year old bug in a tool that nobody noticed. It's not quite that clear-cut. Some years ago, and before p4merge was added as a Git mergetool, P4Merge was changed so its main GUI text says left and right instead of theirs and ours when invoked manually. But it appears that's as far as they went. It doesn't seem any of its asymmetric diff display logic was changed; it works better with ours on the right, and the built-in help all remains written on the theirs/ours basis. And even little details like the icons imply it (a square for the base, a downward-pointing triangle for their incoming stuff, and a circle for the version we hold). So in short, a user of p4merge can see that left side is intended as theirs, even though recent p4merge sometimes calls it left. And your description on the coloring (green vs blue) makes it clear that left and theirs are still intended to be synonyms. If that is the case I would think you can still argue such a change as correcting a 3-4-year old bug. Would it be going too far to also have xxxtool.reverse to choose the global default? It would be a natural thing to do. I left it out because I thought it would go without saying, given that precedences already exist, e.g. mergetool.keepBackup etc. My only reservation is that I assume it would be implemented by swapping what's passed in $LOCAL and $REMOTE. Which seems a bit icky: $LOCAL=a.REMOTE.1234.c. Doesn't the UI show the actual temporary filename? When merging my version of hello.c with your version, showing them as hello.LOCAL.c and hello.REMOTE.c is an integral part of the UI experience, I think, even if the GUI tool does not give its own labels (and behaviour differences as you mentioned for p4merge) to mark which side is theirs and which side is ours. The temporary file that holds their version should still be named with REMOTE, even when the mergetool.reverse option is in effect. As to the name of the variable, I do not care too deeply about it myself, but I think keeping the current LOCAL and REMOTE would help people following the code, especially given the option is called reverse, meaning that there is an internal convention that the order is LOCAL and then REMOTE. One thing to watch out for is from which temporary file we take the merged results. You can present the two sides swapped, but if the tool always writes the results out by updating the second file, the caller needs to be prepared to read from the one that gets changed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool
On Thu, Mar 7, 2013 at 11:10 AM, Junio C Hamano gits...@pobox.com wrote: Kevin Bracey ke...@bracey.fi writes: On 07/03/2013 09:23, Junio C Hamano wrote: If p4merge GUI labels one side clearly as theirs and the other ours, and the way we feed the inputs to it makes the side that is actually ours appear in p4merge GUI labelled as theirs, then I do not think backward compatibility argument does not hold water. It is just correcting a longstanding 3-4 year old bug in a tool that nobody noticed. It's not quite that clear-cut. Some years ago, and before p4merge was added as a Git mergetool, P4Merge was changed so its main GUI text says left and right instead of theirs and ours when invoked manually. But it appears that's as far as they went. It doesn't seem any of its asymmetric diff display logic was changed; it works better with ours on the right, and the built-in help all remains written on the theirs/ours basis. And even little details like the icons imply it (a square for the base, a downward-pointing triangle for their incoming stuff, and a circle for the version we hold). So in short, a user of p4merge can see that left side is intended as theirs, even though recent p4merge sometimes calls it left. And your description on the coloring (green vs blue) makes it clear that left and theirs are still intended to be synonyms. If that is the case I would think you can still argue such a change as correcting a 3-4-year old bug. I would prefer to treat this as a bugfix rather than introducing a new set of configuration knobs if possible. It really does seem like a correction. Users that want the traditional behavior can get that by configuring a custom mergetool.p4merge.cmd, so we're not completely losing the ability to get at the old behavior. Users that want to see a reverse diff with difftool can already say --reverse, so there's even less reason to have it there (though I know we're talking about mergetool only). Would it be going too far to also have xxxtool.reverse to choose the global default? It would be a natural thing to do. I left it out because I thought it would go without saying, given that precedences already exist, e.g. mergetool.keepBackup etc. Medium NACK. If we can do without configuration all the better. I would much rather prefer to have the default/mainstream behavior be the best out-of-the-box sans configuration. The reasoning behind swapping them for p4merge makes sense for p4merge only. I don't think we're quite ready to declare that all the merge tools need to be swapped or that we need a mechanism for swapping the order. My only reservation is that I assume it would be implemented by swapping what's passed in $LOCAL and $REMOTE. Which seems a bit icky: $LOCAL=a.REMOTE.1234.c. Doesn't the UI show the actual temporary filename? When merging my version of hello.c with your version, showing them as hello.LOCAL.c and hello.REMOTE.c is an integral part of the UI experience, I think, even if the GUI tool does not give its own labels (and behaviour differences as you mentioned for p4merge) to mark which side is theirs and which side is ours. The temporary file that holds their version should still be named with REMOTE, even when the mergetool.reverse option is in effect. As to the name of the variable, I do not care too deeply about it myself, but I think keeping the current LOCAL and REMOTE would help people following the code, especially given the option is called reverse, meaning that there is an internal convention that the order is LOCAL and then REMOTE. One thing to watch out for is from which temporary file we take the merged results. You can present the two sides swapped, but if the tool always writes the results out by updating the second file, the caller needs to be prepared to read from the one that gets changed. -- 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
Re: [PATCH 1/2] p4merge: swap LOCAL and REMOTE for mergetool
David Aguilar dav...@gmail.com writes: I would prefer to treat this as a bugfix rather than introducing a new set of configuration knobs if possible. It really does seem like a correction. Users that want the traditional behavior can get that by configuring a custom mergetool.p4merge.cmd, so we're not completely losing the ability to get at the old behavior. Users that want to see a reverse diff with difftool can already say --reverse, so there's even less reason to have it there (though I know we're talking about mergetool only). ... I would much rather prefer to have the default/mainstream behavior be the best out-of-the-box sans configuration. The reasoning behind swapping them for p4merge makes sense for p4merge only. I don't think we're quite ready to declare that all the merge tools need to be swapped or that we need a mechanism for swapping the order. Thanks for an injection of sanity. -- 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 l10n updates for 1.8.2 round 4
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 2/2] bundle: Add colons to list headings in verify
Lukas Fleischer g...@cryptocrack.de writes:
These slightly improve the reading flow by making it obvious that a list
follows.
Signed-off-by: Lukas Fleischer g...@cryptocrack.de
Perhaps.
The earlier message says contains X ref(s) while the later one
says requires this/these X ref(s). Do we want to make them
consistent, too?
---
bundle.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/bundle.c b/bundle.c
index 65db53b..8cd8b4f 100644
--- a/bundle.c
+++ b/bundle.c
@@ -183,8 +183,8 @@ int verify_bundle(struct bundle_header *header, int
verbose)
struct ref_list *r;
r = header-references;
- printf_ln(Q_(The bundle contains %d ref,
- The bundle contains %d refs,
+ printf_ln(Q_(The bundle contains %d ref:,
+ The bundle contains %d refs:,
r-nr),
r-nr);
list_refs(r, 0, NULL);
@@ -192,8 +192,8 @@ int verify_bundle(struct bundle_header *header, int
verbose)
if (!r-nr) {
printf_ln(_(The bundle records a complete history.));
} else {
- printf_ln(Q_(The bundle requires this ref,
- The bundle requires these %d refs,
+ printf_ln(Q_(The bundle requires this ref:,
+ The bundle requires these %d refs:,
r-nr),
r-nr);
list_refs(r, 0, NULL);
--
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] setup.c: Fix prefix_pathspec from looping pass end of string
Andrew Wong andrew.k...@gmail.com writes:
The previous code was assuming length ends at either `)` or `,`, and was
not handling the case where strcspn returns length due to end of string.
So specifying :(top as pathspec will cause the loop to go pass the end
of string.
Thanks.
The parser that goes past the end of the string may be a bug worth
fixing, but is this patch sufficient to diagnose such an input as an
error?
Signed-off-by: Andrew Wong andrew.k...@gmail.com
---
setup.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 1dee47e..f4c4e73 100644
--- a/setup.c
+++ b/setup.c
@@ -207,9 +207,11 @@ static const char *prefix_pathspec(const char *prefix,
int prefixlen, const char
*copyfrom *copyfrom != ')';
copyfrom = nextat) {
size_t len = strcspn(copyfrom, ,));
- if (copyfrom[len] == ')')
+ if (copyfrom[len] == '\0')
nextat = copyfrom + len;
- else
+ else if (copyfrom[len] == ')')
+ nextat = copyfrom + len;
+ else if (copyfrom[len] == ',')
nextat = copyfrom + len + 1;
if (!len)
continue;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What I want rebase to do
From: Thomas Rast tr...@student.ethz.ch
wor...@alum.mit.edu (Dale R. Worley) writes:
[...snip...]
Isn't that just a very long-winded way of restating what Junio said
earlier:
It was suggested to make it apply the first-parent diff and record
the result, I think. If that were an acceptable approach (I didn't
think about it through myself, though), that would automatically
cover the evil-merge case as well.
Well, I believe what I said was a fleshed-out way of saying what I
*think* Junio said, but...
You can fake that with something like
git rev-list --first-parent --reverse RANGE_TO_REBASE |
while read rev; do
if git rev-parse $rev^2 /dev/null 21; then
git cherry-pick -n -m1 $rev
git rev-parse $rev^2 .git/MERGE_HEAD
git commit -C$rev
else
git cherry-pick $rev
fi
done
This code doesn't do that. I don't want something that rebases a
single thread of the current branch, I want something that rebases
*all* the commits between the head commit and the merge base. Which
is what is illustrated in my message.
[1] If you don't get the sarcasm: that would amount to reinventing
large parts of git-rebase.
Yes, that is the point of the exercise.
I've done a proof-of-concept implementation of what I want to see,
calling it git-rebase--merge-safe. But I'm new here and likely that
is a pretty crude solution. I suspect that a real implementation
could be done by inserting this logic into the framework of
git-filter-tree. Following is git-rebase--merge-safe, and the script
I use to test it (and explore rebase problems).
Dale
--
git-rebase--merge-safe
#!/bin/bash
. git-sh-setup
prec=4
set -ex
# Ensure the work tree is clean.
require_clean_work_tree rebase Please commit or stash them.
onto_name=$1
onto=$(git rev-parse --verify ${onto_name}^0) ||
die Does not point to a valid commit: $1
head_name=$( git symbolic-ref HEAD )
orig_head=$(git rev-parse --verify $head_name) ||
exit 1
echo onto=$onto
echo head_name=$head_name
echo orig_head=$orig_head
# Get the merge base, which is the root of the branch that we are rebasing.
# (For now, ignore the question of whether there is more than one merge base.)
mb=$(git merge-base $onto $orig_head)
echo mb=$mb
# Get the list of commits to rebase, which is everything between $mb and
# $orig_head.
# Note that $mb is not included.
revisions=`git rev-list --reverse --ancestry-path $mb..$orig_head`
echo revisions=$revisions
# Set up the list mapping the commits on the original branch to the commits
# on the branch we are creating.
# Its format is ,old-hash1/new-hash1,old-hash2/new-hash2,...,.
# The initial value maps $mb to $onto.
map=,$mb/$onto,
# Export these so git commit can see them.
export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
# Process each commit in forward topological order.
for cmt in $revisions
do
# Examine the commit to extract information we will need to reconstruct it.
# First parent of the commit that has a mapping, i.e., is part of the
# branch (and has thus been rebuilt already.
first_mapped_parent=
# The new commit that was made of $first_mapped_parent.
first_mapped_parent_mapped=
# List of -p options naming the parent commits, or their new commits if they
# are in the branch.
parents=
# Dissect the old commit's data.
# Output the commit data into FD 3.
exec 3 ( git cat-file commit $cmt )
while read keyword rest 3
do
case $keyword in
tree)
# Ignored
;;
parent)
# See if the parent is mapped, i.e., is in the
# original branch.
if [[ $map == *,$rest/* ]]
then
# This parent has been mapped. Get the new commit.
parent_mapped=${map#*,$rest/}
parent_mapped=${parent_mapped%%,*}
if test -z $first_mapped_parent
then
first_mapped_parent=$rest
first_mapped_parent_mapped=$parent_mapped
fi
else
# This parent has not been mapped.
parent_mapped=$rest
fi
# $parent_mapped is a parent of the new commit.
parents=$parents -p $parent_mapped
;;
author)
# Extract the information about the author.
GIT_AUTHOR_NAME=${rest%% *}
GIT_AUTHOR_EMAIL=${rest##* }
GIT_AUTHOR_EMAIL=${GIT_AUTHOR_EMAIL%% *}
GIT_AUTHOR_DATE=${rest##* }
;;
committer)
# Ignored: The new commit will have this use's name
# as committer.
;;
'')
# End of fixed fields, remainder is the
Re: inotify to minimize stat() calls
On 11.02.13 03:56, Duy Nguyen wrote:
On Mon, Feb 11, 2013 at 3:16 AM, Junio C Hamano gits...@pobox.com wrote:
The other lstat() experiment was a very interesting one, but this
is not yet an interesting experiment to see where in the ignore
codepath we are spending times.
We know that we can tell wt_status_collect_untracked() not to bother
with the untracked or ignored files with !s-show_untracked_files
already, but I think the more interesting question is if we can show
the untracked files with less overhead.
If we want to show untrackedd files, it is a given that we need to
read directories to see what paths there are on the filesystem. Is
the opendir/readdir cost dominating in the process? Are we spending
a lot of time sifting the result of opendir/readdir via the ignore
mechanism? Is reading the ignore files costing us much to prime
the ignore mechanism?
If readdir cost is dominant, then that makes cache gitignore a
nonsense proposition, I think. If you really want to cache
something, you need to have somebody (i.e. a daemon) who constantly
keeps an eye on the filesystem changes and can respond with the up
to date result directly to fill_directory(). I somehow doubt that
it is a direction we would want to go in, though.
Yeah, it did not cut out syscall cost, I also cut a lot of user-space
processing (plus .gitignore content access). From the timings I posted
earlier,
unmodified dir.c
real0m0.550s0m0.287s
user0m0.305s0m0.201s
sys 0m0.240s0m0.084s
sys time is reduced from 0.24s to 0.08s, so readdir+opendir definitely
has something to do with it (and perhaps reading .gitignore). But it
also reduces user time from 0.305 to 0.201s. I don't think avoiding
readdir+openddir will bring us this gain. It's probably the cost of
matching .gitignore. I'll try to replace opendir+readdir with a
no-syscall version. At this point untracked caching sounds more
feasible (and less complex) than .gitignore cachine.
Thanks for Duy for the measurements, and patches.
I took the freedom to convert the patched dir.c into a
runtime configurable git status option.
I'm not sure if the following copy-and-paste work applies,
(it is based on Git 1.8.1.3), but the time spend for
git status --changed-only is basically half the time of
git status, similar to what Duy has measured.
I did a test both on a Linux box and Mac OS.
And the speedup is so impressive, that I am tempted to submit a patch simlar
to the following, what do we think about it?
/Torsten
-- 8 --
[PATCH] git status: add option changed-only
git status may be run faster if
- we only check if files are changed which are already known to git.
- we don't check if there are untracked files.
git status --changed-only (or the short form git status -c)
will only check for changed files which are already known to git,
and which are in the index.
The call to read_directory_recursive() is skipped and untracked files
in the working tree are not reported.
Inspired-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
builtin/commit.c | 2 ++
dir.c| 5 +++--
dir.h| 3 ++-
wt-status.c | 3 +++
wt-status.h | 1 +
5 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..6a5ba11 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1158,6 +1158,8 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
unsigned char sha1[20];
static struct option builtin_status_options[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
+ OPT_BOOLEAN('c', changed-only, s.check_changed_only,
+ N_(Ignore untracked files. Check if files known to
git are modified)),
OPT_SET_INT('s', short, status_format,
N_(show status concisely), STATUS_FORMAT_SHORT),
OPT_BOOLEAN('b', branch, s.show_branch,
diff --git a/dir.c b/dir.c
index a473ca2..555b652 100644
--- a/dir.c
+++ b/dir.c
@@ -1274,8 +1274,9 @@ int read_directory(struct dir_struct *dir, const char
*path, int len, const char
return dir-nr;
simplify = create_simplify(pathspec);
- if (!len || treat_leading_path(dir, path, len, simplify))
- read_directory_recursive(dir, path, len, 0, simplify);
+ if ((!(dir-flags DIR_CHECK_CHANGED_ONLY))
+ (!len || treat_leading_path(dir, path, len, simplify)))
o
+ read_directory_recursive(dir, path, len, 0, simplify);
free_simplify(simplify);
qsort(dir-entries, dir-nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir-ignored, dir-ignored_nr, sizeof(struct dir_entry *),
cmp_name);
diff --git a/dir.h b/dir.h
index f5c89e3..1a915a7 100644
--- a/dir.h
+++ b/dir.h
@@ -41,7 +41,8 @@ struct dir_struct {
DIR_SHOW_OTHER_DIRECTORIES = 11,
Re: [PATCH] setup.c: Fix prefix_pathspec from looping pass end of string
On 3/7/13, Junio C Hamano gits...@pobox.com wrote: The parser that goes past the end of the string may be a bug worth fixing, but is this patch sufficient to diagnose such an input as an error? Yea, the patch should fix the passing end of string too. The parser was going past end of string because the nextat is set to copyfrom + len + 1 for the '\0' case too. Then + 1 causes the parser to go pass end of string. If we handle the '\0' case separately, then the parser ends properly, and shouldn't be able to go pass the end of string. Hm, should I be paranoid and put an else clause to call die() as well? In case there's a scenario where none of the 3 cases is true... Andrew -- 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
[ANNOUNCE] Git v1.8.2-rc3
A release candidate Git v1.8.2-rc3 is now available for testing at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 3fe30d85cea78a388d61ba79fe3a106fca41cfbe git-1.8.2.rc3.tar.gz 4b378cf6129fa4c9355436b93a698dde2ed4ce7a git-htmldocs-1.8.2.rc3.tar.gz b18a1f2e70920b5028f1179cc4b362ad78a6f34c git-manpages-1.8.2.rc3.tar.gz Also the following public repositories all have a copy of the v1.8.2-rc3 tag and the master branch that the tag points at: url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.8.2 Release Notes (draft) Backward compatibility notes In the next major release Git 2.0 (not *this* one), we will change the behavior of the git push command. When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). We will use the simple semantics that pushes the current branch to the branch with the same name, only when the current branch is set to integrate with that remote branch. There is a user preference configuration variable push.default to change this. git push $there tag v1.2.3 used to allow replacing a tag v1.2.3 that already exists in the repository $there, if the rewritten tag you are pushing points at a commit that is a descendant of a commit that the old tag v1.2.3 points at. This was found to be error prone and starting with this release, any attempt to update an existing ref under refs/tags/ hierarchy will fail, without --force. When git add -u and git add -A, that does not specify what paths to add on the command line, is run from inside a subdirectory, the scope of the operation has always been limited to the subdirectory. Many users found this counter-intuitive, given that git commit -a and other commands operate on the entire tree regardless of where you are. In this release, these commands give warning in such a case and encourage the user to say git add -u/-A . instead when restricting the scope to the current directory. At Git 2.0 (not *this* one), we plan to change these commands without pathspec to operate on the entire tree. Forming a habit to type . when you mean to limit the command to the current working directory will protect you against the planned future change, and that is the whole point of the new message (there will be no configuration variable to squelch this warning---it goes against the habit forming objective). Updates since v1.8.1 UI, Workflows Features * Initial ports to QNX and z/OS UNIX System Services have started. * Output from the tests is coloured using green is okay, yellow is questionable, red is bad and blue is informative scheme. * Mention of GIT/Git/git in the documentation have been updated to be more uniform and consistent. The name of the system and the concept it embodies is Git; the command the users type is git. All-caps GIT was merely a way to imitate Git typeset in small caps in our ASCII text only documentation and to be avoided. * The completion script (in contrib/completion) used to let the default completer to suggest pathnames, which gave too many irrelevant choices (e.g. git add would not want to add an unmodified path). It learnt to use a more git-aware logic to enumerate only relevant ones. * In bare repositories, git shortlog and other commands now read mailmap files from the tip of the history, to help running these tools in server settings. * Color specifiers, e.g. %C(blue)Hello%C(reset), used in the --format= option of git log and friends can be disabled when the output is not sent to a terminal by prefixing them with auto,, e.g. %C(auto,blue)Hello%C(auto,reset). * Scripts can ask Git that wildcard patterns in pathspecs they give do not have any significance, i.e. take them as literal strings. * The patterns in .gitignore and .gitattributes files can have **/, as a pattern that matches 0 or more levels of subdirectory. E.g. foo/**/bar matches bar in foo itself or in a subdirectory of foo. * When giving arguments without -- disambiguation, object names that come earlier on the command line must not be interpretable as pathspecs and pathspecs that come later on the command line must not be interpretable as object names. This disambiguation rule has been tweaked so that :/ (no other string before or after) is always interpreted as a pathspec; git cmd -- :/ is no longer needed, you can just say git cmd :/. * Various hint lines Git gives when it asks the user to edit messages in the editor are commented out with '#'
[PATCH 0/3] fix git-p4 client root symlink problems
Miklós pointed out in http://thread.gmane.org/gmane.comp.version-control.git/214915 that when the p4 client root included a symlink, bad things happen. It is fixable, but inconvenient, to use an absolute path in one's p4 client. It's not too hard to be smarter about this in git-p4. Thanks to Miklós for the patch, and to John for the style suggestions. I wrote a couple of tests to make sure this part doesn't break again. This is maybe a bug introduced by bf1d68f (git-p4: use absolute directory for PWD env var, 2011-12-09), but that's so long ago that I don't think this is a candidate for maint. -- Pete Miklós Fazekas (1): git p4: avoid expanding client paths in chdir Pete Wyckoff (2): git p4 test: make sure P4CONFIG relative path works git p4 test: should honor symlink in p4 client root git-p4.py | 29 ++--- t/t9808-git-p4-chdir.sh | 41 + 2 files changed, 63 insertions(+), 7 deletions(-) -- 1.8.2.rc2.64.g8335025 -- 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 1/3] git p4 test: make sure P4CONFIG relative path works
This adds a test for the fix in bf1d68f (git-p4: use absolute directory for PWD env var, 2011-12-09). It is necessary to set PWD to an absolute path so that p4 can find files referenced by non-absolute paths, like the value of the P4CONFIG environment variable. P4 does not open files directly; it builds a path by prepending the contents of the PWD environment variable. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9808-git-p4-chdir.sh | 14 ++ 1 file changed, 14 insertions(+) diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh index dc92e60..55c5e36 100755 --- a/t/t9808-git-p4-chdir.sh +++ b/t/t9808-git-p4-chdir.sh @@ -42,6 +42,20 @@ test_expect_success 'P4CONFIG and relative dir clone' ' ) ' +# Common setup using .p4config to set P4CLIENT and P4PORT breaks +# if clone destination is relative. Make sure that chdir() expands +# the relative path in --dest to absolute. +test_expect_success 'p4 client root would be relative due to clone --dest' ' + test_when_finished cleanup_git + ( + echo P4PORT=$P4PORT git/.p4config + P4CONFIG=.p4config + export P4CONFIG + unset P4PORT + git p4 clone --dest=git //depot + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.2.rc2.64.g8335025 -- 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 2/3] git p4 test: should honor symlink in p4 client root
This test fails when the p4 client root includes a symlink. It complains: Path /vol/bar/projects/foo/... is not under client root /p/foo and dumps a traceback. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9808-git-p4-chdir.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh index 55c5e36..af8bd8a 100755 --- a/t/t9808-git-p4-chdir.sh +++ b/t/t9808-git-p4-chdir.sh @@ -56,6 +56,33 @@ test_expect_success 'p4 client root would be relative due to clone --dest' ' ) ' +# When the p4 client Root is a symlink, make sure chdir() does not use +# getcwd() to convert it to a physical path. +test_expect_failure 'p4 client root symlink should stay symbolic' ' + physical=$TRASH_DIRECTORY/physical + symbolic=$TRASH_DIRECTORY/symbolic + test_when_finished rm -rf \$physical\ + test_when_finished rm \$symbolic\ + mkdir -p $physical + ln -s $physical $symbolic + test_when_finished cleanup_git + ( + P4CLIENT=client-sym + p4 client -i -EOF + Client: $P4CLIENT + Description: $P4CLIENT + Root: $symbolic + LineEnd: unix + View: //depot/... //$P4CLIENT/... + EOF + git p4 clone --dest=$git //depot + cd $git + test_commit file2 + git config git-p4.skipSubmitEdit true + git p4 submit + ) +' + test_expect_success 'kill p4d' ' kill_p4d ' -- 1.8.2.rc2.64.g8335025 -- 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 3/3] git p4: avoid expanding client paths in chdir
From: Miklós Fazekas mfaze...@szemafor.com The generic chdir() helper sets the PWD environment variable, as that is what is used by p4 to know its current working directory. Normally the shell would do this, but in git-p4, we must do it by hand. However, when the path contains a symbolic link, os.getcwd() will return the physical location. If the p4 client specification includes symlinks, setting PWD to the physical location causes p4 to think it is not inside the client workspace. It complains, e.g. Path /vol/bar/projects/foo/... is not under client root /p/foo One workaround is to use AltRoots in the p4 client specification, but it is cleaner to handle it directly in git-p4. Other uses of chdir still require setting PWD to an absolute path so p4 features like P4CONFIG work. See bf1d68f (git-p4: use absolute directory for PWD env var, 2011-12-09). [ pw: tweak patch and commit message ] Thanks-to: John Keeping j...@keeping.me.uk Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 29 ++--- t/t9808-git-p4-chdir.sh | 2 +- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/git-p4.py b/git-p4.py index 647f110..7288c0b 100755 --- a/git-p4.py +++ b/git-p4.py @@ -79,12 +79,27 @@ def p4_build_cmd(cmd): real_cmd += cmd return real_cmd -def chdir(dir): -# P4 uses the PWD environment variable rather than getcwd(). Since we're -# not using the shell, we have to set it ourselves. This path could -# be relative, so go there first, then figure out where we ended up. -os.chdir(dir) -os.environ['PWD'] = os.getcwd() +def chdir(path, is_client_path=False): +Do chdir to the given path, and set the PWD environment + variable for use by P4. It does not look at getcwd() output. + Since we're not using the shell, it is necessary to set the + PWD environment variable explicitly. + + Normally, expand the path to force it to be absolute. This + addresses the use of relative path names inside P4 settings, + e.g. P4CONFIG=.p4config. P4 does not simply open the filename + as given; it looks for .p4config using PWD. + + If is_client_path, the path was handed to us directly by p4, + and may be a symbolic link. Do not call os.getcwd() in this + case, because it will cause p4 to think that PWD is not inside + the client path. + + +os.chdir(path) +if not is_client_path: +path = os.getcwd() +os.environ['PWD'] = path def die(msg): if verbose: @@ -1624,7 +1639,7 @@ class P4Submit(Command, P4UserMap): new_client_dir = True os.makedirs(self.clientPath) -chdir(self.clientPath) +chdir(self.clientPath, is_client_path=True) if self.dry_run: print Would synchronize p4 checkout in %s % self.clientPath else: diff --git a/t/t9808-git-p4-chdir.sh b/t/t9808-git-p4-chdir.sh index af8bd8a..09b2cc4 100755 --- a/t/t9808-git-p4-chdir.sh +++ b/t/t9808-git-p4-chdir.sh @@ -58,7 +58,7 @@ test_expect_success 'p4 client root would be relative due to clone --dest' ' # When the p4 client Root is a symlink, make sure chdir() does not use # getcwd() to convert it to a physical path. -test_expect_failure 'p4 client root symlink should stay symbolic' ' +test_expect_success 'p4 client root symlink should stay symbolic' ' physical=$TRASH_DIRECTORY/physical symbolic=$TRASH_DIRECTORY/symbolic test_when_finished rm -rf \$physical\ -- 1.8.2.rc2.64.g8335025 -- 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: [feature request] 2) Remove many tags at once and 1) Prune tags on old-branch-before-rebase
Eric Chamberland eric.chamberl...@giref.ulaval.ca writes: 1) git tag --delete-tags-to-danglings-and-unnamed-banches This would be able to remove all tags that refers to commits which are on branches that are no more referenced by any branch name. This is happening when you tag something, then git rebase. Your tag will still be there on the old-and-before-rebase branch and won't be pruned by any git command... (that I know of...) Not interesting for at least two reasons. Why are tags any special? git branch --delete-merged may also be of interest, and for that matter git update-ref -d to deal with any ref in general would be equally valid if such an option were a good idea. What you want is a way to compute, given a set of tags (or refs in general) and a set of branches (or another set of refs in general), find the ones in the former that none of the latter can reach. With that, you can drive git tag -d $(that way). In other words, the feature does not belong to git tag command. 2) git tag -d TOKEN* Again, not interesting. You already have: git for-each-ref --format='%(refname:short)' refs/tags/TOKEN\* | xargs -r git tag -d -- 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] setup.c: Fix prefix_pathspec from looping pass end of string
Andrew Wong andrew.k...@gmail.com writes: On 3/7/13, Junio C Hamano gits...@pobox.com wrote: The parser that goes past the end of the string may be a bug worth fixing, but is this patch sufficient to diagnose such an input as an error? Yea, the patch should fix the passing end of string too. The parser was going past end of string because the nextat is set to copyfrom + len + 1 for the '\0' case too. Then + 1 causes the parser to go pass end of string. If we handle the '\0' case separately, then the parser ends properly, and shouldn't be able to go pass the end of string. This did not error out for me, though. $ cd t git ls-files :(top -- 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: inotify to minimize stat() calls
Torsten Bögershausen tbo...@web.de writes:
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..6a5ba11 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1158,6 +1158,8 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
unsigned char sha1[20];
static struct option builtin_status_options[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
+ OPT_BOOLEAN('c', changed-only, s.check_changed_only,
+ N_(Ignore untracked files. Check if files known to
git are modified)),
Doesn't this make one wonder why a separate bit and implementation
is necessary to say I am not interested in untracked files when
-uno option is already 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: [PATCH] setup.c: Fix prefix_pathspec from looping pass end of string
On 3/7/13, Junio C Hamano gits...@pobox.com wrote: This did not error out for me, though. $ cd t git ls-files :(top No error message at all? Hm, maybe in your case, the byte after the end of string happens to be '\0' and the loop ended by chance? git doesn't crash for me, but it generates this error: $ git ls-files :(top fatal: Invalid pathspec magic 'LS_COLORS=' in ':(top' The loop runs for a second time after parsing top, and copyfrom now points to the byte after :(top, which is coming from argv. And in my distribution/platform, it looks like the envp, the third param of main(), is packed right after the argv strings, because: $ env | head -n 1 LS_COLORS= -- 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] setup.c: Fix prefix_pathspec from looping pass end of string
Andrew Wong andrew.k...@gmail.com writes: On 3/7/13, Junio C Hamano gits...@pobox.com wrote: This did not error out for me, though. $ cd t git ls-files :(top No error message at all? Hm, maybe in your case, the byte after the end of string happens to be '\0' and the loop ended by chance? git doesn't crash for me, but it generates this error: $ git ls-files :(top fatal: Invalid pathspec magic 'LS_COLORS=' in ':(top' What I meant was that I do not get any error _after_ applying your patch. It is broken to behave as if LS_COLORS=... (which is totally unrelated string that happens to be laid out next in the memory) is a part of the pathspec magic specification your :(top started. Your patch makes the code stop doing that. But it is equally broken to behave as if there is nothing wrong in the incomplete magic :(top that is not closed, isn't 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: [BUG] bare repository detection does not work with aliases
The $GIT_BARE idea sounds very sensible to me.
Jeff King p...@peff.net wrote:
On Thu, Mar 07, 2013 at 05:47:45PM -0500, Mark Lodato wrote:
It seems that the fallback bare repository detection in the absence
of
core.bare fails for aliases.
This triggered some deja vu for me, so I went digging. And indeed, this
has been a bug since at least 2008. This patch (which never got
applied)
fixed it:
http://thread.gmane.org/gmane.comp.version-control.git/72792
The issue is that we treat:
GIT_DIR=/some/path git ...
as if the current directory is the work tree, unless core.bare is
explicitly set, or unless an explicit work tree is given (via
GIT_WORK_TREE, git --work-tree, or in the config). This is handy,
and
backwards compatible.
Inside setup_git_directory, when we find the directory we put it in
$GIT_DIR for later reference by ourselves or sub-programs (since we are
typically moving to the top of the working tree next, we need to record
the original path, and can't rely on discovery finding the same path
again). But we don't set $GIT_WORK_TREE. So if you don't have core.bare
set, the above rule will kick in for sub-programs, or for aliases
(which
will call setup_git_directory again).
The solution is that when we set $GIT_DIR like this, we need to also
say
no, there is no working tree; we are bare. And that's what that patch
does. It's 5 years old now, so not surprisingly, it does not apply
cleanly. The moral equivalent in today's code base would be something
like:
diff --git a/environment.c b/environment.c
index 89d6c70..8edaedd 100644
--- a/environment.c
+++ b/environment.c
@@ -200,7 +200,8 @@ void set_git_work_tree(const char *new_work_tree)
return;
}
git_work_tree_initialized = 1;
- work_tree = xstrdup(real_path(new_work_tree));
+ if (*new_work_tree)
+ work_tree = xstrdup(real_path(new_work_tree));
}
const char *get_git_work_tree(void)
diff --git a/setup.c b/setup.c
index e1cfa48..f0e1251 100644
--- a/setup.c
+++ b/setup.c
@@ -544,7 +544,7 @@ static const char *setup_explicit_git_dir(const
char *gitdirenv,
worktree = get_git_work_tree();
/* both get_git_work_tree() and cwd are already normalized */
- if (!strcmp(cwd, worktree)) { /* cwd == worktree */
+ if (!worktree || !strcmp(cwd, worktree)) { /* cwd == worktree */
set_git_dir(gitdirenv);
free(gitfile);
return NULL;
@@ -636,6 +636,8 @@ static const char *setup_bare_git_dir(char *cwd,
int offset, int len, int *nongi
}
else
set_git_dir(.);
+
+ setenv(GIT_WORK_TREE_ENVIRONMENT, , 1);
return NULL;
}
which passes your test. Unfortunately, this patch runs afoul of the
same
complaints that prevented the original from being acceptable (weirdness
on Windows with empty environment variables).
Having read the discussion again, I _think_ the more sane thing is to
actually just have a new variable, $GIT_BARE, which overrides any
core.bare config (just as $GIT_WORK_TREE override core.worktree). And
then we set that explicitly when we are in a bare $GIT_DIR, propagating
our auto-detection to sub-processes.
-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
--
Pardon terseness, typo and HTML from a tablet.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] git p4 test: should honor symlink in p4 client root
Am 3/8/2013 0:19, schrieb Pete Wyckoff: +# When the p4 client Root is a symlink, make sure chdir() does not use +# getcwd() to convert it to a physical path. +test_expect_failure 'p4 client root symlink should stay symbolic' ' + physical=$TRASH_DIRECTORY/physical + symbolic=$TRASH_DIRECTORY/symbolic + test_when_finished rm -rf \$physical\ + test_when_finished rm \$symbolic\ + mkdir -p $physical + ln -s $physical $symbolic This test needs a SYMLINKS prerequisite to future-proof it, in case the Windows port gains p4 support some time. + test_when_finished cleanup_git + ( + P4CLIENT=client-sym + p4 client -i -EOF + Client: $P4CLIENT + Description: $P4CLIENT + Root: $symbolic + LineEnd: unix + View: //depot/... //$P4CLIENT/... + EOF + git p4 clone --dest=$git //depot + cd $git + test_commit file2 + git config git-p4.skipSubmitEdit true + git p4 submit + ) +' -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: inotify to minimize stat() calls
On 08.03.13 01:04, Junio C Hamano wrote:
Torsten Bögershausen tbo...@web.de writes:
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..6a5ba11 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1158,6 +1158,8 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
unsigned char sha1[20];
static struct option builtin_status_options[] = {
OPT__VERBOSE(verbose, N_(be verbose)),
+OPT_BOOLEAN('c', changed-only, s.check_changed_only,
+N_(Ignore untracked files. Check if files known to
git are modified)),
Doesn't this make one wonder why a separate bit and implementation
is necessary to say I am not interested in untracked files when
-uno option is already there?
Thanks Junio,
this is good news.
I need to admit that I wasn't aware about git status -uno.
Thinking about it, how many git users are aware of the speed penalty
when running git status to find out which (tracked) files they had changed?
Or to put it the other way, when a developer wants a quick overview
about the files she changed, then git status -uno may be a good and fast friend.
Does it make sence to stress put that someway in the documentation?
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 9f1ef9a..360d813 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -51,13 +51,18 @@ default is 'normal', i.e. show untracked files and directori
+
The possible options are:
+
- - 'no' - Show no untracked files
+ - 'no' - Show no untracked files (this is fastest)
- 'normal' - Shows untracked files and directories
- 'all'- Also shows individual files in untracked directories.
+
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
++
+Note: Searching for untracked files or directories may take some time.
+A fast way to get a status of files tracked by git is to use
+'git status -uno'
+
--
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] setup: suppress implicit . work-tree for bare repos
If an explicit GIT_DIR is given without a working tree, we
implicitly assume that the current working directory should
be used as the working tree. E.g.,:
GIT_DIR=/some/repo.git git status
would compare against the cwd.
Unfortunately, we fool this rule for sub-invocations of git
by setting GIT_DIR internally ourselves. For example:
git init foo
cd foo/.git
git status ;# fails, as we expect
git config alias.st status
git status ;# does not fail, but should
What happens is that we run setup_git_directory when doing
alias lookup (since we need to see the config), set GIT_DIR
as a result, and then leave GIT_WORK_TREE blank (because we
do not have one). Then when we actually run the status
command, we do setup_git_directory again, which sees our
explicit GIT_DIR and uses the cwd as an implicit worktree.
It's tempting to argue that we should be suppressing that
second invocation of setup_git_directory, as it could use
the values we already found in memory. However, the problem
still exists for sub-processes (e.g., if git status were
an external command).
You can see another example with the --bare option, which
sets GIT_DIR explicitly. For example:
git init foo
cd foo/.git
git status ;# fails
git --bare status ;# does NOT fail
We need some way of telling sub-processes even though
GIT_DIR is set, do not use cwd as an implicit working tree.
We could do it by putting a special token into
GIT_WORK_TREE, but the obvious choice (an empty string) has
some portability problems, and could potentially be
triggered accidentally by a user.
Instead, we add a new boolean variable, GIT_IMPLICIT_WORK_TREE,
which suppresses the use of cwd as a working tree when
GIT_DIR is set. We trigger the new variable when we know we
are in a bare setting.
The variable is left intentionally undocumented, as this is
an internal detail (for now, anyway). If somebody comes up
with a good alternate use for it, and once we are confident
we have shaken any bugs out of it, we can consider promoting
it further.
Signed-off-by: Jeff King p...@peff.net
---
So I think this just ends up being a cleaner and smaller change than
trying to support $GIT_BARE. I think $GIT_BARE could allow more
flexibility, but it's flexibility nobody is particularly asking for, and
there are lots of nasty corner cases around it. I'm pretty sure this is
doing the right thing.
Having written this, I'm still tempted to signal the same thing by
putting /dev/null into GIT_WORK_TREE (Junio's suggestion from the old
thread). This one works OK because we only check GIT_WORK_TREE_IMPLICIT
_after_ exhausting all of the other working tree options, so it is
always subordinate to a later setting of GIT_WORK_TREE. But it seems a
little cleaner for somebody setting GIT_WORK_TREE To clear this
implicit flag automatically.
At the same time, I would wonder how other git implementations would
react to GIT_WORK_TREE=/dev/null. Would they try to chdir() there and
barf, when they could happily exist without a working tree? Doing it
this way seems a bit safer from regressions (those other implementations
do not get the _benefit_ of this patch unless they support
GIT_WORK_TREE_IMPLICIT, of course, but at least we are not breaking
them).
cache.h | 1 +
git.c | 1 +
setup.c | 8
t/t1510-repo-setup.sh | 19 +++
4 files changed, 29 insertions(+)
diff --git a/cache.h b/cache.h
index e493563..070169a 100644
--- a/cache.h
+++ b/cache.h
@@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int
mode)
#define GIT_DIR_ENVIRONMENT GIT_DIR
#define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
#define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE
+#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE
#define DEFAULT_GIT_DIR_ENVIRONMENT .git
#define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY
#define INDEX_ENVIRONMENT GIT_INDEX_FILE
diff --git a/git.c b/git.c
index b10c18b..24b7984 100644
--- a/git.c
+++ b/git.c
@@ -125,6 +125,7 @@ static int handle_options(const char ***argv, int *argc,
int *envchanged)
static char git_dir[PATH_MAX+1];
is_bare_repository_cfg = 1;
setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
sizeof(git_dir)), 0);
+ setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 0, 1);
if (envchanged)
*envchanged = 1;
} else if (!strcmp(cmd, -c)) {
diff --git a/setup.c b/setup.c
index 1dee47e..6c87660 100644
--- a/setup.c
+++ b/setup.c
@@ -523,6 +523,12 @@ static const char *setup_explicit_git_dir(const char
*gitdirenv,
set_git_work_tree(core_worktree);
}
}
+ else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {
+ /* #16d */
+ set_git_dir(gitdirenv);
+ free(gitfile);
+ return NULL;
+ }
else /* #2,
Re: [PATCH] setup: suppress implicit . work-tree for bare repos
Am 3/8/2013 8:15, schrieb Jeff King: --- a/cache.h +++ b/cache.h @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_DIR_ENVIRONMENT GIT_DIR #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE #define DEFAULT_GIT_DIR_ENVIRONMENT .git #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY #define INDEX_ENVIRONMENT GIT_INDEX_FILE This new variable needs to be added to environment.c:local_repo_env, right? -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] setup: suppress implicit . work-tree for bare repos
Jeff King p...@peff.net writes: diff --git a/cache.h b/cache.h index e493563..070169a 100644 --- a/cache.h +++ b/cache.h @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_DIR_ENVIRONMENT GIT_DIR #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE #define DEFAULT_GIT_DIR_ENVIRONMENT .git #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY #define INDEX_ENVIRONMENT GIT_INDEX_FILE Not adding any user documentation is fine (you explained why in the log message), but I would really prefer to have some in-code comment to clarify its meaning. Is it Please do use implicit work tree boolean? Is it This is the path to the work tree we have already figured out string? Is it something else? What is it used for, who sets it, what other codepath that will be invented in the future need to be careful to set it (or unset it) and how does one who writes that new codepath decides that he needs to do so (or shouldn't)? I would know *today* that it is a bool to affect us, after having discovered that we are in bare and we have set GIT_DIR (so if the end user already had GIT_DIR, we shouldn't set it ourselves), and also our child processes, but I am not confident that I will remember this thread 6 months down the road. -- 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
