* [PATCH 1/3] mailinfo documentation: accurately describe non -k case
From: Thomas Rast @ 2012-01-11 20:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Since its very first description of -k, the documentation for
git-mailinfo claimed that (in the case without -k) after cleaning up
bracketed strings [blah], it would insert [PATCH].
It doesn't; on the contrary, one of the important jobs of mailinfo is
to remove those strings.
Since we're already there, rewrite the paragraph to give a complete
enumeration of all the transformations. Specifically, it was missing
the whitespace normalization (run of isspace(c) -> ' ') and the
removal of leading ':'.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Documentation/git-mailinfo.txt | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-mailinfo.txt b/Documentation/git-mailinfo.txt
index 51dc325..97e7a8e 100644
--- a/Documentation/git-mailinfo.txt
+++ b/Documentation/git-mailinfo.txt
@@ -25,13 +25,24 @@ command directly. See linkgit:git-am[1] instead.
OPTIONS
-------
-k::
- Usually the program 'cleans up' the Subject: header line
- to extract the title line for the commit log message,
- among which (1) remove 'Re:' or 're:', (2) leading
- whitespaces, (3) '[' up to ']', typically '[PATCH]', and
- then prepends "[PATCH] ". This flag forbids this
- munging, and is most useful when used to read back
- 'git format-patch -k' output.
+ Usually the program removes email cruft from the Subject:
+ header line to extract the title line for the commit log
+ message. This option prevents this munging, and is most
+ useful when used to read back 'git format-patch -k' output.
++
+Specifically, the following are removed until none of them remain:
++
+--
+* Leading and trailing whitespace.
+
+* Leading `Re:`, `re:`, and `:`.
+
+* Leading bracketed strings (between `[` and `]`, usually
+ `[PATCH]`).
+--
++
+Finally, runs of whitespace are normalized to a single ASCII space
+character.
-b::
When -k is not in effect, all leading strings bracketed with '['
--
1.7.9.rc0.168.g3847c
^ permalink raw reply related
* [PATCH 3/3] mailinfo: with -b, keep space after [foo]
From: Thomas Rast @ 2012-01-11 20:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <e915a551c9bbf12f4d8fd929e9ed24f3223790ee.1326312730.git.trast@student.ethz.ch>
The logic for the -b mode, where [PATCH] is dropped but [foo] is not,
silently ate all spaces after the ].
Fix this by keeping the next isspace() character, if there is any.
Being more thorough is pointless, as the later cleanup_space() call
will normalize any sequence of whitespace to a single ' '.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
builtin/mailinfo.c | 11 ++++++++++-
t/t4150-am.sh | 2 +-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index bfb32b7..eaf9e15 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -250,8 +250,17 @@ static void cleanup_subject(struct strbuf *subject)
(7 <= remove &&
memmem(subject->buf + at, remove, "PATCH", 5)))
strbuf_remove(subject, at, remove);
- else
+ else {
at += remove;
+ /*
+ * If the input had a space after the ], keep
+ * it. We don't bother with finding the end of
+ * the space, since we later normalize it
+ * anyway.
+ */
+ if (isspace(subject->buf[at]))
+ at += 1;
+ }
continue;
}
break;
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 7e7c83c..8807b60 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -262,7 +262,7 @@ test_expect_success 'am --keep really keeps the subject' '
grep "Re: Re: Re: \[PATCH 1/5 v2\] \[foo\] third" actual
'
-test_expect_failure 'am --keep-non-patch really keeps the non-patch part' '
+test_expect_success 'am --keep-non-patch really keeps the non-patch part' '
rm -fr .git/rebase-apply &&
git reset --hard &&
git checkout HEAD^ &&
--
1.7.9.rc0.168.g3847c
^ permalink raw reply related
* [PATCH 2/3] am: learn passing -b to mailinfo
From: Thomas Rast @ 2012-01-11 20:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <e915a551c9bbf12f4d8fd929e9ed24f3223790ee.1326312730.git.trast@student.ethz.ch>
git-am could pass -k to mailinfo, but not -b. Introduce an option
that does so. We change the meaning of the 'keep' state file, but are
careful not to cause a problem unless you downgrade in the middle of
an 'am' run.
This uncovers a bug in mailinfo -b, hence the failing test.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Someone on IRC asked about git-am support for passing through the -b
flag to mailinfo. And so it began...
Documentation/git-am.txt | 3 +++
git-am.sh | 11 +++++++----
t/t4150-am.sh | 14 ++++++++++++--
3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 887466d..ee6cca2 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -40,6 +40,9 @@ OPTIONS
--keep::
Pass `-k` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
+--keep-non-patch::
+ Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
+
--keep-cr::
--no-keep-cr::
With `--keep-cr`, call 'git mailsplit' (see linkgit:git-mailsplit[1])
diff --git a/git-am.sh b/git-am.sh
index 1c13b13..8dfad7a 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -15,6 +15,7 @@ q,quiet be quiet
s,signoff add a Signed-off-by line to the commit message
u,utf8 recode into utf8 (default)
k,keep pass -k flag to git-mailinfo
+keep-non-patch pass -b flag to git-mailinfo
keep-cr pass --keep-cr flag to git-mailsplit for mbox format
no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr
c,scissors strip everything before a scissors line
@@ -386,7 +387,9 @@ do
--no-utf8)
utf8= ;;
-k|--keep)
- keep=t ;;
+ keep=-k ;;
+ --keep-non-patch)
+ keep=-b ;;
-c|--scissors)
scissors=t ;;
--no-scissors)
@@ -398,7 +401,7 @@ do
--abort)
abort=t ;;
--rebasing)
- rebasing=t threeway=t keep=t scissors=f no_inbody_headers=t ;;
+ rebasing=t threeway=t keep=-k scissors=f no_inbody_headers=t ;;
-d|--dotest)
die "$(gettext "-d option is no longer supported. Do not use.")"
;;
@@ -571,8 +574,8 @@ then
else
utf8=-n
fi
-if test "$(cat "$dotest/keep")" = t
-then
+keep=$(cat "$dotest/keep")
+if test "$keep" = t
keep=-k
fi
case "$(cat "$dotest/scissors")" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index d7d9ccc..7e7c83c 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -237,7 +237,7 @@ test_expect_success 'am stays in branch' '
test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '
git format-patch --stdout HEAD^ >patch3 &&
- sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2," patch3 >patch4 &&
+ sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," patch3 >patch4 &&
rm -fr .git/rebase-apply &&
git reset --hard &&
git checkout HEAD^ &&
@@ -259,7 +259,17 @@ test_expect_success 'am --keep really keeps the subject' '
git am --keep patch4 &&
! test -d .git/rebase-apply &&
git cat-file commit HEAD >actual &&
- grep "Re: Re: Re: \[PATCH 1/5 v2\] third" actual
+ grep "Re: Re: Re: \[PATCH 1/5 v2\] \[foo\] third" actual
+'
+
+test_expect_failure 'am --keep-non-patch really keeps the non-patch part' '
+ rm -fr .git/rebase-apply &&
+ git reset --hard &&
+ git checkout HEAD^ &&
+ git am --keep-non-patch patch4 &&
+ ! test -d .git/rebase-apply &&
+ git cat-file commit HEAD >actual &&
+ grep "^\[foo\] third" actual
'
test_expect_success 'am -3 falls back to 3-way merge' '
--
1.7.9.rc0.168.g3847c
^ permalink raw reply related
* Re: [PATCH 3/3] diff-index: pass pathspec down to unpack-trees machinery
From: Junio C Hamano @ 2012-01-11 20:40 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jonathan Nieder, git, Linus Torvalds
In-Reply-To: <CACsJy8D7EnOebAxBYF8ua7htu-81nKY=ghUMgg=JOe4Fc1uigQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> This seems to fix this.
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 7c9ecf6..5cf58b6 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1042,6 +1042,7 @@ int unpack_trees(unsigned len, struct tree_desc
> *t, struct unpack_trees_options
> info.data = o;
> info.show_all_errors = o->show_all_errors;
> info.pathspec = o->pathspec;
> + info.pathspec->recursive = 1;
>
> if (o->prefix) {
> /*
>
> Still scratching my head why this flag is zero by default, which would
> affect all other places.
Ahh, thanks for diagnosing.
> ... Or perhaps the right fix would be this
> instead
>
> diff --git a/tree-walk.c b/tree-walk.c
> index f82dba6..0345938 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -634,7 +634,7 @@ enum interesting tree_entry_interesting(const
> struct name_entry *entry,
> * Match all directories. We'll try to
> * match files later on.
> */
> - if (ps->recursive && S_ISDIR(entry->mode))
> + if (S_ISDIR(entry->mode))
> return entry_interesting;
> }
>
> @@ -662,7 +662,7 @@ match_wildcards:
> * Match all directories. We'll try to match files
> * later on.
> */
> - if (ps->recursive && S_ISDIR(entry->mode))
> + if (S_ISDIR(entry->mode))
> return entry_interesting;
> }
> return never_interesting; /* No matches */
Doesn't that break "git diff-tree A B" without the "-r" option?
^ permalink raw reply
* Re: [PATCH RFC] commit: allow to commit even if there are intent-to-add entries
From: Junio C Hamano @ 2012-01-11 21:08 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Nguyễn Thái Ngọc Duy, git, Jeff King,
Will Palmer
In-Reply-To: <20120111110222.GA32173@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
>> When running "commit" and "status" with files marked with "intent to add",
>> I think there are three possible interpretations of what the user
>> wants to do.
> [ (1) thanks for stopping me, I had forgotten about that file;
> (2) I changed my mind, please leave that file out; or (3) please
> dwim and add the file ]
>
> I think (3) was a trick --- no one that does not use the "-a" option
> would want that. :)
I really wish it were the case, but I doubt it.
People from other VCS background seem to still think that "commit" without
paths should commit everything; after getting told that "what you add to
the index is what you will commit", I can easily see this commint: but but
but I told Git that I _want_ to add with -N! Why aren't you committing it?
> At the time, I did not understand what (2) meant. Now I see why ---
> in interpretation (2), the user did not change her mind at all.
You are correct. "I still cannot make up my mind" is what is happening in
that situation.
The user explicitly said "I cannot decide about this path right now" when
she said "add -N". And we haven't heard from the user what should happen
to the path. Now we have to make a commit so somebody needs to decide.
> She
> said "I will be adding this file at some point, so please keep track
> of it along with the others for the sake of commands like 'git diff'
> and 'git add -u', but that does not mean "I will be adding this file
> at some point _before the next commit_".
Correct. She only said "I cannot decide right now" when she said "add -N"
and hasn't gave us any more hint as to what should happen now we have to
make a commit.
It is _wrong_ for us to unilaterally decide for the user that she does not
want the path in the commit. The last we heard from her was that she does
not know what should happen to the path.
> (2) makes intent-to-add entries just like any other tracked index
> entry with some un-added content.
You are comparing files edited in the working tree without the user
telling anything about them to Git (both tracked and untracked) and files
the user explicitly told Git that the user hasn't made up her mind
about. Why is it a good thing to make the latter behave "just like any
other"?
^ permalink raw reply
* Re: git svn clone terminating prematurely (I think)
From: Steven Line @ 2012-01-11 21:52 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0nyc6NVE7Qpvbc0dXb1UHGM_=uYbCS+a53HZxiBRG9HvQ@mail.gmail.com>
Hi Ram, thank you for the reply. Se below . . .
>
>> I need some help getting my subversion repository cloned over to git.
>> Our svn repository has about 12,000 commits, when I run
>> git svn clone -s -A authors.txt
>> svn+ssh://csvn <at> source.res.ourdomain.com/home/svn/sem sem
>> It runs for about 2h 15m then completes with no error messages. I have
>> also cloned starting at revision 6300, about the middle of the svn
>> repository, and I get the same results as below.
>
>> $ git branch -a # shows only about half the branches that should have
>> been cloned
>
> Interesting. From the git-svn-id of the most recent commit, can you
> tell if there's anything especially fishy about the revision where
> git-svn stops? Your Subversion repository is probably broken in some
> way, but git-svn should not use that as an excuse for appearing to
> finish successfully while failing in reality.
Well by your question it seems you expect this clone to fail at the
same svn revision number
each time. I spent all day today trying to figure out what that
revision was. However it doesn't
seem to be failing at the same svn revision each time.
There are about 12813 revisions in our svn repository so I started
attempting clones from successively later
and later revisions both to figure which revision they repeatedly
failed on (it was never the same one) and to see
if I could coax a successful clone to occur. Originally I started at
revision 1, then incremented to 6300, then 8000, then 10000, then
12,500, then finally 12,700. Finally the attempt starting at svn
revision 12,700 succeeded and the resulting git repository
seems to work. All the attempts prior to 12,700 (starting at 1, 6300,
8000, 10000, and 12,500) failed while
importing different revisions, none of them showed error messages.
One thing that I am suspect of is that I'm not able to log directly
into the machine running git, it's on a remote server in
Atlanta while I'm in Colorado. I'm running the 'git svn clone' using
nohup, but in every case of a corrupt git repository
the connection between me and the server dies before the git completes
(the connection dies, but I log in again and run
ps -ef and I see the multiple git processes still running. They run
for up to several hours longer, then terminate). I should be
ok since I'm using nohup, but coincidentally the only clone that
succeeded was the one where the network connection
never disconnected during the clone
Not sure if this helps but here are some numbers:
Both the svn repository and git machine are Solaris 10
Svn revision 1.6.12 CollabNet
Git version 1.7.6.1
Thank you.
--
Steven Line
303-910-1212
sline00@gmail.com
^ permalink raw reply
* Re: git svn clone terminating prematurely (I think)
From: Jonathan Nieder @ 2012-01-11 22:48 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Steven Line, git
In-Reply-To: <CALkWK0nyc6NVE7Qpvbc0dXb1UHGM_=uYbCS+a53HZxiBRG9HvQ@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Interesting. From the git-svn-id of the most recent commit, can you
> tell if there's anything especially fishy about the revision where
> git-svn stops? Your Subversion repository is probably broken in some
> way,
I wouldn't necessarily assume that. My first hunch would have been
that this is some variation on the SIGPIPE bug[1]. The usual
workaround for that[2] is to get git-svn to continue by running "git
svn fetch" again. Nobody's fixed it because nobody's stared at the
SVN perl bindings for long enough to find the cause.
Modern git versions produce a message in that case, though:
error: git-svn died of signal 13
So you are probably running into something else. I only mention this
for context.
Does svnsync work for making a local copy of whatever subset of the
repository is relevant? Alternatively (this is basically the same),
is the network connection stable enough for e.g. svnrdump[3] to get a
dump?
Sorry for the trouble, and hope that helps,
Jonathan
[1] http://thread.gmane.org/gmane.comp.version-control.git/134936/focus=134940
[2] Maybe it deserves a note in the manpage.
[3] http://repo.or.cz/w/svnrdump.git
^ permalink raw reply
* [PATCH 0/5] git-p4: view spec review fixes
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
I received some helpful review comments offline regarding the
series recently merged into master (8cbfc11
(Merge branch 'pw/p4-view-updates', 2012-01-06)).
It would be nice to see these go into 1.7.9 as well.
Even though they belong there logically, they are
minor fixes, and not critical.
I added a bunch of new tests documenting further problems,
but will not fix those here as the changes will be more
invasive.
Thanks,
Pete Wyckoff (5):
git-p4: only a single ... wildcard is supported
git-p4: fix verbose comment typo
git-p4: clarify comment
git-p4: adjust test to adhere to stricter useClientSpec
git-p4: add tests demonstrating spec overlay ambiguities
Documentation/git-p4.txt | 5 +
contrib/fast-import/git-p4 | 9 +-
t/t9806-git-p4-options.sh | 4 +-
t/t9809-git-p4-client-view.sh | 395 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 406 insertions(+), 7 deletions(-)
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply
* [PATCH 1/5] git-p4: only a single ... wildcard is supported
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
In-Reply-To: <1326324670-15967-1-git-send-email-pw@padd.com>
Catch the case where a ... exists at the end, and also elsehwere.
Reported-by: Gary Gibbons <ggibbons@perforce.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 4 ++--
t/t9809-git-p4-client-view.sh | 8 +++++++-
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 3e1aa27..20208bf 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1207,8 +1207,8 @@ class View(object):
die("Can't handle * wildcards in view: %s" % self.path)
triple_dot_index = self.path.find("...")
if triple_dot_index >= 0:
- if not self.path.endswith("..."):
- die("Can handle ... wildcard only at end of path: %s" %
+ if triple_dot_index != len(self.path) - 3:
+ die("Can handle only single ... wildcard, at end: %s" %
self.path)
self.ends_triple_dot = True
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index c9471d5..54204af 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -101,12 +101,18 @@ test_expect_success 'unsupported view wildcard *' '
test_must_fail "$GITP4" clone --use-client-spec --dest="$git" //depot
'
-test_expect_success 'wildcard ... only supported at end of spec' '
+test_expect_success 'wildcard ... only supported at end of spec 1' '
client_view "//depot/.../file11 //client/.../file11" &&
test_when_finished cleanup_git &&
test_must_fail "$GITP4" clone --use-client-spec --dest="$git" //depot
'
+test_expect_success 'wildcard ... only supported at end of spec 2' '
+ client_view "//depot/.../a/... //client/.../a/..." &&
+ test_when_finished cleanup_git &&
+ test_must_fail "$GITP4" clone --use-client-spec --dest="$git" //depot
+'
+
test_expect_success 'basic map' '
client_view "//depot/dir1/... //client/cli1/..." &&
files="cli1/file11 cli1/file12" &&
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply related
* [PATCH 2/5] git-p4: fix verbose comment typo
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
In-Reply-To: <1326324670-15967-1-git-send-email-pw@padd.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 20208bf..e267f31 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1263,7 +1263,7 @@ class View(object):
if self.exclude:
c = "-"
return "View.Mapping: %s%s -> %s" % \
- (c, self.depot_side, self.client_side)
+ (c, self.depot_side.path, self.client_side.path)
def map_depot_to_client(self, depot_path):
"""Calculate the client path if using this mapping on the
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply related
* [PATCH 3/5] git-p4: clarify comment
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
In-Reply-To: <1326324670-15967-1-git-send-email-pw@padd.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
contrib/fast-import/git-p4 | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index e267f31..e11e15b 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1363,7 +1363,8 @@ class View(object):
else:
# This mapping matched; no need to search any further.
# But, the mapping could be rejected if the client path
- # has already been claimed by an earlier mapping.
+ # has already been claimed by an earlier mapping (i.e.
+ # one later in the list, which we are walking backwards).
already_mapped_in_client = False
for f in paths_filled:
# this is View.Path.match
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply related
* [PATCH 4/5] git-p4: adjust test to adhere to stricter useClientSpec
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
In-Reply-To: <1326324670-15967-1-git-send-email-pw@padd.com>
This test relied on what now is seen as broken behavior
in --use-client-spec. Change it to make sure it works
according to the new behavior as described in
ecb7cf9 (git-p4: rewrite view handling, 2012-01-02) and
c700b68 (git-p4: test client view handling, 2012-01-02).
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/t9806-git-p4-options.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 1f1952a..0571602 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -146,7 +146,7 @@ test_expect_success 'clone --use-client-spec' '
(
cd "$git" &&
test_path_is_file bus/dir/f4 &&
- test_path_is_file file1
+ test_path_is_missing file1
) &&
cleanup_git &&
@@ -159,7 +159,7 @@ test_expect_success 'clone --use-client-spec' '
"$GITP4" sync //depot/... &&
git checkout -b master p4/master &&
test_path_is_file bus/dir/f4 &&
- test_path_is_file file1
+ test_path_is_missing file1
)
'
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply related
* [PATCH 5/5] git-p4: add tests demonstrating spec overlay ambiguities
From: Pete Wyckoff @ 2012-01-11 23:31 UTC (permalink / raw)
To: git; +Cc: Gary Gibbons
In-Reply-To: <1326324670-15967-1-git-send-email-pw@padd.com>
Introduce new tests that look more closely at overlay situations
when there are conflicting files. Five of these are broken.
Document the brokenness.
This is a fundamental problem with how git-p4 only "borrows" a
client spec. At some sync operation, a new change can contain
a file which is already in the repo or explicitly deleted through
another mapping. To sort this out would involve listing all the
files in the client spec to find one with a higher priority.
While this is not too hard for the initial import, subsequent
sync operations would be very costly.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
Documentation/git-p4.txt | 5 +
t/t9809-git-p4-client-view.sh | 387 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 392 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 78938b2..8b92cc0 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -314,6 +314,11 @@ around whitespace. Of the possible wildcards, git-p4 only handles
'...', and only when it is at the end of the path. Git-p4 will complain
if it encounters an unhandled wildcard.
+Bugs in the implementation of overlap mappings exist. If multiple depot
+paths map through overlays to the same location in the repository,
+git-p4 can choose the wrong one. This is hard to solve without
+dedicating a client spec just for git-p4.
+
The name of the client can be given to git-p4 in multiple ways. The
variable 'git-p4.client' takes precedence if it exists. Otherwise,
normal p4 mechanisms of determining the client are used: environment
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 54204af..ae9145e 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -247,6 +247,393 @@ test_expect_success 'quotes on rhs only' '
'
#
+# What happens when two files of the same name are overlayed together?
+# The last-listed file should take preference.
+#
+# //depot
+# - dir1
+# - file11
+# - file12
+# - filecollide
+# - dir2
+# - file21
+# - file22
+# - filecollide
+#
+test_expect_success 'overlay collision setup' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir1/filecollide >dir1/filecollide &&
+ p4 add dir1/filecollide &&
+ p4 submit -d dir1/filecollide &&
+ echo dir2/filecollide >dir2/filecollide &&
+ p4 add dir2/filecollide &&
+ p4 submit -d dir2/filecollide
+ )
+'
+
+test_expect_success 'overlay collision 1 to 2' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22 filecollide" &&
+ echo dir2/filecollide >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/filecollide &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files &&
+ test_cmp actual "$git"/filecollide
+'
+
+test_expect_failure 'overlay collision 2 to 1' '
+ client_view "//depot/dir2/... //client/..." \
+ "+//depot/dir1/... //client/..." &&
+ files="file11 file12 file21 file22 filecollide" &&
+ echo dir1/filecollide >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/filecollide &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files &&
+ test_cmp actual "$git"/filecollide
+'
+
+test_expect_success 'overlay collision delete 2' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ p4 delete dir2/filecollide &&
+ p4 submit -d "remove dir2/filecollide"
+ )
+'
+
+# no filecollide, got deleted with dir2
+test_expect_failure 'overlay collision 1 to 2, but 2 deleted' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22" &&
+ client_verify $files &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files
+'
+
+test_expect_success 'overlay collision update 1' '
+ client_view "//depot/dir1/... //client/dir1/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ p4 open dir1/filecollide &&
+ echo dir1/filecollide update >dir1/filecollide &&
+ p4 submit -d "update dir1/filecollide"
+ )
+'
+
+# still no filecollide, dir2 still wins with the deletion even though the
+# change to dir1 is more recent
+test_expect_failure 'overlay collision 1 to 2, but 2 deleted, then 1 updated' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22" &&
+ client_verify $files &&
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files
+'
+
+test_expect_success 'overlay collision delete filecollides' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ p4 delete dir1/filecollide dir2/filecollide &&
+ p4 submit -d "remove filecollides"
+ )
+'
+
+#
+# Overlays as part of sync, rather than initial checkout:
+# 1. add a file in dir1
+# 2. sync to include it
+# 3. add same file in dir2
+# 4. sync, make sure content switches as dir2 has priority
+# 5. add another file in dir1
+# 6. sync
+# 7. add/delete same file in dir2
+# 8. sync, make sure it disappears, again dir2 wins
+# 9. cleanup
+#
+# //depot
+# - dir1
+# - file11
+# - file12
+# - colA
+# - colB
+# - dir2
+# - file21
+# - file22
+# - colA
+# - colB
+#
+test_expect_success 'overlay sync: add colA in dir1' '
+ client_view "//depot/dir1/... //client/dir1/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir1/colA >dir1/colA &&
+ p4 add dir1/colA &&
+ p4 submit -d dir1/colA
+ )
+'
+
+test_expect_success 'overlay sync: initial git checkout' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22 colA" &&
+ echo dir1/colA >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colA &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files &&
+ test_cmp actual "$git"/colA
+'
+
+test_expect_success 'overlay sync: add colA in dir2' '
+ client_view "//depot/dir2/... //client/dir2/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir2/colA >dir2/colA &&
+ p4 add dir2/colA &&
+ p4 submit -d dir2/colA
+ )
+'
+
+test_expect_success 'overlay sync: colA content switch' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22 colA" &&
+ echo dir2/colA >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colA &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files &&
+ test_cmp actual "$git"/colA
+'
+
+test_expect_success 'overlay sync: add colB in dir1' '
+ client_view "//depot/dir1/... //client/dir1/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir1/colB >dir1/colB &&
+ p4 add dir1/colB &&
+ p4 submit -d dir1/colB
+ )
+'
+
+test_expect_success 'overlay sync: colB appears' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22 colA colB" &&
+ echo dir1/colB >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colB &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files &&
+ test_cmp actual "$git"/colB
+'
+
+test_expect_success 'overlay sync: add/delete colB in dir2' '
+ client_view "//depot/dir2/... //client/dir2/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir2/colB >dir2/colB &&
+ p4 add dir2/colB &&
+ p4 submit -d dir2/colB &&
+ p4 delete dir2/colB &&
+ p4 submit -d "delete dir2/colB"
+ )
+'
+
+test_expect_success 'overlay sync: colB disappears' '
+ client_view "//depot/dir1/... //client/..." \
+ "+//depot/dir2/... //client/..." &&
+ files="file11 file12 file21 file22 colA" &&
+ client_verify $files &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files
+'
+
+test_expect_success 'overlay sync: cleanup' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ p4 delete dir1/colA dir2/colA dir1/colB &&
+ p4 submit -d "remove overlay sync files"
+ )
+'
+
+#
+# Overlay tests again, but swapped so dir1 has priority.
+# 1. add a file in dir1
+# 2. sync to include it
+# 3. add same file in dir2
+# 4. sync, make sure content does not switch
+# 5. add another file in dir1
+# 6. sync
+# 7. add/delete same file in dir2
+# 8. sync, make sure it is still there
+# 9. cleanup
+#
+# //depot
+# - dir1
+# - file11
+# - file12
+# - colA
+# - colB
+# - dir2
+# - file21
+# - file22
+# - colA
+# - colB
+#
+test_expect_success 'overlay sync swap: add colA in dir1' '
+ client_view "//depot/dir1/... //client/dir1/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir1/colA >dir1/colA &&
+ p4 add dir1/colA &&
+ p4 submit -d dir1/colA
+ )
+'
+
+test_expect_success 'overlay sync swap: initial git checkout' '
+ client_view "//depot/dir2/... //client/..." \
+ "+//depot/dir1/... //client/..." &&
+ files="file11 file12 file21 file22 colA" &&
+ echo dir1/colA >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colA &&
+ "$GITP4" clone --use-client-spec --dest="$git" //depot &&
+ git_verify $files &&
+ test_cmp actual "$git"/colA
+'
+
+test_expect_success 'overlay sync swap: add colA in dir2' '
+ client_view "//depot/dir2/... //client/dir2/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir2/colA >dir2/colA &&
+ p4 add dir2/colA &&
+ p4 submit -d dir2/colA
+ )
+'
+
+test_expect_failure 'overlay sync swap: colA no content switch' '
+ client_view "//depot/dir2/... //client/..." \
+ "+//depot/dir1/... //client/..." &&
+ files="file11 file12 file21 file22 colA" &&
+ echo dir1/colA >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colA &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files &&
+ test_cmp actual "$git"/colA
+'
+
+test_expect_success 'overlay sync swap: add colB in dir1' '
+ client_view "//depot/dir1/... //client/dir1/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir1/colB >dir1/colB &&
+ p4 add dir1/colB &&
+ p4 submit -d dir1/colB
+ )
+'
+
+test_expect_success 'overlay sync swap: colB appears' '
+ client_view "//depot/dir2/... //client/..." \
+ "+//depot/dir1/... //client/..." &&
+ files="file11 file12 file21 file22 colA colB" &&
+ echo dir1/colB >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colB &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files &&
+ test_cmp actual "$git"/colB
+'
+
+test_expect_success 'overlay sync swap: add/delete colB in dir2' '
+ client_view "//depot/dir2/... //client/dir2/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ echo dir2/colB >dir2/colB &&
+ p4 add dir2/colB &&
+ p4 submit -d dir2/colB &&
+ p4 delete dir2/colB &&
+ p4 submit -d "delete dir2/colB"
+ )
+'
+
+test_expect_failure 'overlay sync swap: colB no change' '
+ client_view "//depot/dir2/... //client/..." \
+ "+//depot/dir1/... //client/..." &&
+ files="file11 file12 file21 file22 colA colB" &&
+ echo dir1/colB >actual &&
+ client_verify $files &&
+ test_cmp actual "$cli"/colB &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ "$GITP4" sync --use-client-spec &&
+ git merge --ff-only p4/master
+ ) &&
+ git_verify $files &&
+ test_cmp actual "$cli"/colB
+'
+
+test_expect_success 'overlay sync swap: cleanup' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ p4 delete dir1/colA dir2/colA dir1/colB &&
+ p4 submit -d "remove overlay sync files"
+ )
+'
+
+#
# Rename directories to test quoting in depot-side mappings
# //depot
# - "dir 1"
--
1.7.8.1.409.g3e338.dirty
^ permalink raw reply related
* Re: [PATCHv3 10/13] credentials: add "cache" helper
From: Jonathan Nieder @ 2012-01-11 23:50 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120110175312.GA7289@sigill.intra.peff.net>
Jeff King wrote:
> Still, it would be slightly more robust. I wonder how portable fchdir
> is in practice (I guess we could always fall back to the getcwd code
> path). Do you want to prepare a patch on top?
I've been wanting to get around to doing something similar for setup.c
for a while. I'm happy enough to forget about it for now. ;-)
Thanks again for the fix. Here's another quick nit.
-- >8 --
Subject: unix-socket: do not let close() or chdir() clobber errno during cleanup
unix_stream_connect and unix_stream_listen return -1 on error, with
errno set by the failing underlying call to allow the caller to write
a useful diagnosis.
Unfortunately the error path involves a few system calls itself, such
as close(), that can themselves touch errno.
This is not as worrisome as it might sound. If close() fails, this
just means substituting one meaningful error message for another,
which is perfectly fine. However, when the call _succeeds_, it is
allowed to (and sometimes might) clobber errno along the way with some
undefined value, so it is good higiene to save errno and restore it
immediately before returning to the caller. Do so.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
unix-socket.c | 39 ++++++++++++++++++++++-----------------
1 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/unix-socket.c b/unix-socket.c
index 7d8bec61..01f119f9 100644
--- a/unix-socket.c
+++ b/unix-socket.c
@@ -73,25 +73,29 @@ static int unix_sockaddr_init(struct sockaddr_un *sa, const char *path,
int unix_stream_connect(const char *path)
{
- int fd;
+ int fd, saved_errno;
struct sockaddr_un sa;
struct unix_sockaddr_context ctx;
if (unix_sockaddr_init(&sa, path, &ctx) < 0)
return -1;
fd = unix_stream_socket();
- if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
- unix_sockaddr_cleanup(&ctx);
- close(fd);
- return -1;
- }
+ if (connect(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+ goto fail;
unix_sockaddr_cleanup(&ctx);
return fd;
+
+fail:
+ saved_errno = errno;
+ unix_sockaddr_cleanup(&ctx);
+ close(fd);
+ errno = saved_errno;
+ return -1;
}
int unix_stream_listen(const char *path)
{
- int fd;
+ int fd, saved_errno;
struct sockaddr_un sa;
struct unix_sockaddr_context ctx;
@@ -100,18 +104,19 @@ int unix_stream_listen(const char *path)
fd = unix_stream_socket();
unlink(path);
- if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
- unix_sockaddr_cleanup(&ctx);
- close(fd);
- return -1;
- }
+ if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+ goto fail;
- if (listen(fd, 5) < 0) {
- unix_sockaddr_cleanup(&ctx);
- close(fd);
- return -1;
- }
+ if (listen(fd, 5) < 0)
+ goto fail;
unix_sockaddr_cleanup(&ctx);
return fd;
+
+fail:
+ saved_errno = errno;
+ unix_sockaddr_cleanup(&ctx);
+ close(fd);
+ errno = saved_errno;
+ return -1;
}
--
1.7.8.3
^ permalink raw reply related
* Re: [PATCH 1/2] cache-tree: update API to take abitrary flags
From: Junio C Hamano @ 2012-01-11 23:48 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326275982-29866-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> ---
Thanks; this one looks very sensible regardless of what follows (or does
not follow). Forgot to sign-off?
^ permalink raw reply
* Re: [PATCH 2/2] commit: add --skip-intent-to-add to allow commit with i-t-a entries in index
From: Junio C Hamano @ 2012-01-11 23:55 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326275982-29866-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> ---
> builtin/commit.c | 10 +++++++---
> cache-tree.c | 8 +++++---
> cache-tree.h | 1 +
> t/t2203-add-intent.sh | 17 +++++++++++++++++
> 4 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index bf42bb3..021206e 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
> static int edit_flag = -1; /* unspecified */
> static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
> static int no_post_rewrite, allow_empty_message;
> +static int cache_tree_flags, skip_intent_to_add;
> static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
> static char *sign_commit;
>
> @@ -170,6 +171,7 @@ static struct option builtin_commit_options[] = {
> OPT_BOOLEAN(0, "amend", &amend, "amend previous commit"),
> OPT_BOOLEAN(0, "no-post-rewrite", &no_post_rewrite, "bypass post-rewrite hook"),
> { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg, "mode", "show untracked files, optional modes: all, normal, no. (Default: all)", PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
> + OPT_BOOL(0, "skip-intent-to-add", &skip_intent_to_add, "allow intent-to-add entries in index"),
This is more like "ignore", not "allow", from end user's point of view,
no? The user earlier said "I cannot decide what contents to put in the
commit yet for this path", and normally we catch it and remind the user
that she needs to decide. This option gives her a quick way to say "I
decide that I do not want to add this path at all to this commit I am
creating, so please ignore it in the meantime."
> @@ -1088,6 +1090,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
> cleanup_mode = CLEANUP_ALL;
> else
> die(_("Invalid cleanup mode %s"), cleanup_arg);
> + if (skip_intent_to_add)
> + cache_tree_flags = WRITE_TREE_INTENT_TO_ADD_OK;
The name WRITE_TREE_INTENT_TO_ADD_OK says "it is OK to call write-tree
with i-t-a entries in the index, please do not barf", but I think "when
writing a tree, ignore i-t-a entries" would be a more appropriate way to
say the same thing, i.e. WRITE_TREE_IGNORE_INTENT_TO_ADD.
Other than that, I do not see an issue in the implementation of the
patch. It is a separate design level issue if we want to worsen
proliferation of the options, though.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 09/10] clone: allow --branch to take a tag
From: Junio C Hamano @ 2012-01-11 23:57 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jeff King
In-Reply-To: <1326189427-20800-10-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> @@ -766,6 +771,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> find_ref_by_name(mapped_refs, head.buf);
> strbuf_release(&head);
>
> + if (!our_head_points_at) {
> + strbuf_addstr(&head, "refs/tags/");
> + strbuf_addstr(&head, option_branch);
> + our_head_points_at =
> + find_ref_by_name(mapped_refs, head.buf);
> + strbuf_release(&head);
> + }
> +
Ok. I think this makes sense.
^ permalink raw reply
* Re: leaky cherry-pick
From: Junio C Hamano @ 2012-01-12 0:00 UTC (permalink / raw)
To: Jeff King
Cc: Ramkumar Ramachandra, Pete Wyckoff, git,
Nguyễn Thái Ngọc
In-Reply-To: <20120111195605.GB12333@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Maybe this?
>
> diff --git a/attr.c b/attr.c
> index 76b079f..1656db4 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -301,6 +301,7 @@ static void free_attr_elem(struct attr_stack *e)
> }
> free(a);
> }
> + free(e->attrs);
> free(e);
> }
Yeah, that is definitely a leak.
^ permalink raw reply
* Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default
From: Tay Ray Chuan @ 2012-01-12 0:52 UTC (permalink / raw)
To: Thomas Rast; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <87lipexawp.fsf@thomas.inf.ethz.ch>
Hi,
Thomas, first off, thanks for looking through this.
On Thu, Jan 12, 2012 at 4:05 AM, Thomas Rast <trast@student.ethz.ch> wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>
>> Factor out the comprehensive non-whitespace regex in use by PATTERNS and
>> IPATTERN and use it as the word-diff regex for the default diff driver.
>
> Why?
>
> I seem to recall that the motivation for keeping the original code as-is
> instead of just emulating its behavior with a default regex was that it
> is faster. So disabling the default mode should at least have an
> advantage?
>
> </devils-advocate>
If you're talking about speed, yeah, that's probably true.
But I think it's worthwhile to trade-off performance for a sensible
default. Something like
matrix[a,b,c]
matrix[d,b,c]
gives
matrix[[-a-]{+d+},b,c]
and when we have
ImagineALanguageLikeFoo
ImagineALanguageLikeBar
we get
ImagineALanguageLike[-Foo-]{+Bar+}
(But I cheated. Foo and Bar have no common characters in common; if
they did, the word diff would be messy.)
Both of which seem sensible. From a usability/effectiveness
standpoint, I think it's more useful than what the current word-diff
defaults to - the whole line is taken as a "word", with the pre-image
shown as deleted and the post-image as added; we don't even try to run
LCS on it.
Examples are lifted from:
[1] http://article.gmane.org/gmane.comp.version-control.git/105896
[2] http://article.gmane.org/gmane.comp.version-control.git/105237
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH 1/2] cache-tree: update API to take abitrary flags
From: Nguyen Thai Ngoc Duy @ 2012-01-12 1:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmx9t6bs5.fsf@alter.siamese.dyndns.org>
2012/1/12 Junio C Hamano <gitster@pobox.com>:
> Thanks; this one looks very sensible regardless of what follows (or does
> not follow). Forgot to sign-off?
Deliberately to stop you from using it because I did not test it
carefully. It was created as material for the discussion only. Will
resubmit later.
--
Duy
^ permalink raw reply
* Re: [PATCH v3 06/10] clone: delay cloning until after remote HEAD checking
From: Nguyen Thai Ngoc Duy @ 2012-01-12 1:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7v8vle6ng3.fsf@alter.siamese.dyndns.org>
2012/1/12 Junio C Hamano <gitster@pobox.com>:
>> @@ -938,6 +940,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
>> char *handler = xmalloc(len + 1);
>> handler[len] = 0;
>> strncpy(handler, url, len);
>> + remote->foreign_vcs = helper;
>> transport_helper_init(ret, handler);
>> }
>
> This I am not sure. What value does "helper" variable have at this point
> in the flow? Wouldn't it be a NULL? Or did you mean "handler"?
Ah yes "handler", my bad.
--
Duy
^ permalink raw reply
* Re: [PATCH 2/3] am: learn passing -b to mailinfo
From: Junio C Hamano @ 2012-01-12 1:35 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <19539098c07a207f3bd24f5a145ba3b6c5e46766.1326312730.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> @@ -571,8 +574,8 @@ then
> else
> utf8=-n
> fi
> -if test "$(cat "$dotest/keep")" = t
> -then
> +keep=$(cat "$dotest/keep")
> +if test "$keep" = t
> keep=-k
> fi
Curious.
Who writes 't' to $dotest/keep after this patch is applied?
I also do not want to worry about "echo" portability issues that may come
from an existing
echo "$keep" >"$dotest/keep"
that this patch does not touch.
I suspect that this patch was not tested in a way to exercise this
codepath; shell would have barfed when seeing the lack of "then" here, no?
^ permalink raw reply
* Re: [PATCH 1/2] t9200: On MSYS, do not pass Windows-style paths to CVS
From: Junio C Hamano @ 2012-01-12 2:06 UTC (permalink / raw)
To: Sebastian Schuberth; +Cc: msysgit, git
In-Reply-To: <4F0D544E.6010105@gmail.com>
Sebastian Schuberth <sschuberth@gmail.com> writes:
> For details, see the commit message of 4114156ae9. Note that while using
> $PWD as part of GIT_DIR is not required here, it does no harm and it is
> more consistent. In addition, on MSYS using an environment variable should
> be slightly faster than spawning an external executable.
Thanks. It seems that the "Dos and Dont's" from t/README needs to be
stressed a bit more strongly.
^ permalink raw reply
* Re: git diff <file> HEAD^:<file> error message
From: Junio C Hamano @ 2012-01-12 2:26 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <20120111111831.GB15232@beez.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> I was trying to figure out why running
>
> git diff HEAD^:RelNotes RelNotes
>
> gives the expected output (on maint it tells me that the stable
> version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> doesn't.
>
> git diff RelNotes HEAD^:RelNotes
>
> doesn't show the opposite patch ...
That comes from the general argument parsing rules of Git, namely, global
options (e.g. --paginate) first, then subcommand name, followed by dashed
options, revs and finally the paths. Once you give "RelNotes", which
cannot be a rev, you cannot give a rev.
We _could_ special case the rule for "diff", but we simply didn't bother,
as the resulting code (and the implications of special casing) would be
too ugly to live to support such a corner case usage, especially when you
could always say "-R" to reverse the output.
^ permalink raw reply
* Re: [PATCH 1/2] get_sha1_with_context: report features used in resolution
From: Junio C Hamano @ 2012-01-12 2:36 UTC (permalink / raw)
To: Jeff King; +Cc: Carlos Martín Nieto, git, Albert Astals Cid
In-Reply-To: <20120111194210.GA12441@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Most callers generally treat get_sha1 as a black box, giving
> it a string from the user and expecting to get a sha1 in
> return. The get_sha1_with_context function gives callers
> more information about what happened while resolving the
> object name so they can make better decisions about how to
> use the result. We currently use this only to provide
> information about the path entry used to find a blob.
>
> We don't currently provide any information about the
> resolution rules that were used to reach the final object.
> Some callers may want these in order to enforce a policy
> that a particular subset of the lookup rules are used (e.g.,
> when serving remote requests).
>
> This patch adds a set of bit-fields that document the use of
> particular features during an object lookup.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> The diffstat looks a little scary, but it is mostly just the internal
> get_sha1 functions learning to pass the object_context around.
Hmm, shouldn't this also cover peel_to_type()? That would have made it
also apply to the maintenance track.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox