* Re: [PATCH] mergetools: Simplify how we handle "vim" and "defaults"
From: John Keeping @ 2013-01-26 20:54 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <CAJDDKr5UMyJOthSOPuChx7BCpcGwtmYcnVMh83q9kgCWSxDp4w@mail.gmail.com>
On Sat, Jan 26, 2013 at 12:40:23PM -0800, David Aguilar wrote:
> On Sat, Jan 26, 2013 at 4:12 AM, John Keeping <john@keeping.me.uk> wrote:
> > On Fri, Jan 25, 2013 at 10:50:58PM -0800, David Aguilar wrote:
> >> diff --git a/Makefile b/Makefile
> >> index f69979e..3bc6eb5 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -2724,7 +2724,7 @@ install: all
> >> $(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
> >> $(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
> >> $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> >> - $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> >> + cp -R mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> >
> > Shouldn't this be more like this?
> >
> > $(INSTALL) -m 644 $(subst include,,$(wildcard mergetools/*)) \
> > '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
> > $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
> > $(INSTALL) -m 644 mergetools/include/* \
> > '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
> >
> > I'm not sure creating an 'include' directory actually buys us much over
> > declaring that 'vimdiff' is the real script and the others just source
> > it. Either way there is a single special entry in the mergetools
> > directory.
>
> Thanks (and thanks for the Makefile hint.. I knew it was wrong! ;-).
I think that version's still not right actually, the first line should
probably use filter-out not subst:
$(INSTALL) -m 644 $(filter-out include,$(wildcard mergetools/*)) \
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
John
^ permalink raw reply
* Re: [PATCH 0/3] lazily load commit->buffer
From: Junio C Hamano @ 2013-01-26 21:26 UTC (permalink / raw)
To: Jeff King
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <20130126094026.GA9646@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah, agreed. I started to fix this up with a use/unuse pattern and
> realized something: all of the call sites are calling logmsg_reencode
> anyway, because that is the next logical step in doing anything with the
> buffer that is not just parsing out the parent/timestamp/tree info. And
> since that function already might allocate (for the re-encoded copy),
> callers have to handle the maybe-borrowed-maybe-free situation already.
>
> So I came up with this patch series, which I think should fix the
> problem, and actually makes the call-sites easier to read, rather than
> harder.
>
> [1/3]: commit: drop useless xstrdup of commit message
> [2/3]: logmsg_reencode: never return NULL
> [3/3]: logmsg_reencode: lazily load missing commit buffers
>
> Here's the diffstat:
>
> builtin/blame.c | 22 ++-------
> builtin/commit.c | 14 +-----
> commit.h | 1 +
> pretty.c | 93 ++++++++++++++++++++++++++---------
> t/t4042-diff-textconv-caching.sh | 8 +++
> 5 files changed, 85 insertions(+), 53 deletions(-)
>
> Not too bad, and 27 of the lines added in pretty.c are new comments
> explaining the flow of logmsg_reencode. So even if this doesn't get
> every case, I think it's a nice cleanup.
This looks very good.
I wonder if this lets us get rid of the hack in cmd_log_walk() that
does this:
while ((commit = get_revision(rev)) != NULL) {
if (!log_tree_commit(rev, commit) &&
rev->max_count >= 0)
rev->max_count++;
! if (!rev->reflog_info) {
! /* we allow cycles in reflog ancestry */
free(commit->buffer);
commit->buffer = NULL;
! }
free_commit_list(commit->parents);
commit->parents = NULL;
After log_tree_commit() handles the commit, using the buffer, we
discard the memory associated to it because we know we no longer
will use it in normal cases.
The "do not do that if rev->reflog_info is true" was added in
a6c7306 (--walk-reflogs: do not crash with cyclic reflog ancestry,
2007-01-20) because the second and subsequent display of "commit"
(which happens to occur only when walking reflogs) needs to look at
commit->buffer again, and this hack forces us to retain the buffer
for _all_ commit objects.
But your patches could be seen as a different (and more correct) way
to fix the same issue. Once the display side learns how to re-read
the log text of the commit object, the above becomes unnecessary, no?
We may still be helped if majority of commit objects that appear in
the reflog appear more than once, in which case retaining the buffer
for _all_ commits could be an overall win. Not having to read the
buffer for the same commit each time it is shown for majority of
multiply-appearing commits, in exchange for having to keep the
buffer for commits that appears only once that are minority and
suffering increasted page cache pressure. That could be seen as an
optimization.
But that is a performance thing, not a correctness issue, so "we
allow cycles" implying "therefore if we discard the buffer, we will
show wrong output" becomes an incorrect justification.
I happen to have HEAD reflog that is 30k entries long; more than 26k
represent a checkout of unique commit. So I suspect that the above
hack to excessive retain commit->buffer for already used commits will
not help us performance-wise, either.
^ permalink raw reply
* Re: [PATCH] tests: turn on test-lint-shell-syntax by default
From: Junio C Hamano @ 2013-01-26 21:43 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Jonathan Nieder, git, kraai
In-Reply-To: <51037E5F.8090506@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> Do we really need "which" to detect if frotz is installed?
I think we all know the answer to that question is no, but why is
that a relevant question in the context of this discussion? One of
us may be very confused.
I thought the topic of this discussion was that, already knowing
that "which" should never be used anywhere in our scripts, you are
trying to devise a mechanical way to catch newcomers' attempts to
use it in their changes, in order to prevent patches that add use of
"which" to be sent for review to waste our time. I was illustrating
that the approach to override "which" in a shell function for test
scripts will not be a useful solution for that goal.
^ permalink raw reply
* Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
From: Junio C Hamano @ 2013-01-26 21:44 UTC (permalink / raw)
To: John Keeping; +Cc: git, Sverre Rabbelier
In-Reply-To: <20130126175158.GK7498@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
> Junio, can you replace the queued 0846b0c (git-remote-testpy: hash bytes
> explicitly) with this?
>
> I hadn't realised that the "hex" encoding we chose before is a "bytes to
> bytes" encoding so it just fails with an error on Python 3 in the same
> way as the original code.
>
> Since we want to convert a Unicode string to bytes I think UTF-8 really
> is the best option here.
Ahh. I think it is already in "next", so this needs to be turned
into an incremental to flip 'hex' to 'utf-8', with the justification
being these five lines above.
Thanks for catching.
>
> git-remote-testpy.py | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/git-remote-testpy.py b/git-remote-testpy.py
> index d94a66a..f8dc196 100644
> --- a/git-remote-testpy.py
> +++ b/git-remote-testpy.py
> @@ -31,9 +31,9 @@ from git_remote_helpers.git.exporter import GitExporter
> from git_remote_helpers.git.importer import GitImporter
> from git_remote_helpers.git.non_local import NonLocalGit
>
> -if sys.hexversion < 0x01050200:
> - # os.makedirs() is the limiter
> - sys.stderr.write("git-remote-testgit: requires Python 1.5.2 or later.\n")
> +if sys.hexversion < 0x02000000:
> + # string.encode() is the limiter
> + sys.stderr.write("git-remote-testgit: requires Python 2.0 or later.\n")
> sys.exit(1)
>
> def get_repo(alias, url):
> @@ -45,7 +45,7 @@ def get_repo(alias, url):
> repo.get_head()
>
> hasher = _digest()
> - hasher.update(repo.path)
> + hasher.update(repo.path.encode('utf-8'))
> repo.hash = hasher.hexdigest()
>
> repo.get_base_path = lambda base: os.path.join(
^ permalink raw reply
* Re: [PATCH 0/3] lazily load commit->buffer
From: Jeff King @ 2013-01-26 22:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathon Mah, Jonathan Nieder, Duy Nguyen, Stefan Näwe,
Armin, git@vger.kernel.org
In-Reply-To: <7v8v7f1vqa.fsf@alter.siamese.dyndns.org>
On Sat, Jan 26, 2013 at 01:26:53PM -0800, Junio C Hamano wrote:
> This looks very good.
>
> I wonder if this lets us get rid of the hack in cmd_log_walk() that
> does this:
>
> while ((commit = get_revision(rev)) != NULL) {
> if (!log_tree_commit(rev, commit) &&
> rev->max_count >= 0)
> rev->max_count++;
> ! if (!rev->reflog_info) {
> ! /* we allow cycles in reflog ancestry */
> free(commit->buffer);
> commit->buffer = NULL;
> ! }
> free_commit_list(commit->parents);
> commit->parents = NULL;
>
> After log_tree_commit() handles the commit, using the buffer, we
> discard the memory associated to it because we know we no longer
> will use it in normal cases.
> [...]
> But that is a performance thing, not a correctness issue, so "we
> allow cycles" implying "therefore if we discard the buffer, we will
> show wrong output" becomes an incorrect justification.
Right. I think the correctness issue goes away with my patches, and it
is just a question of estimating the workload for performance. I doubt
it makes a big difference either way, especially when compared to
actually showing the commit (even a single pathspec limiter, or doing
"-p", would likely dwarf a few extra commit decompressions).
My HEAD has about 400/3000 non-unique commits, which matches your
numbers percentage-wise. Dropping the lines above (and always freeing)
takes my best-of-five for "git log -g" from 0.085s to 0.080s. Which is
well within the noise. Doing "git log -g Makefile" ended up at 0.183s
both before and after.
So I suspect it does not matter at all in normal cases, and the time is
indeed dwarfed by adding even a rudimentary pathspec. I'd be in favor of
dropping the lines just to decrease complexity of the code.
-Peff
^ permalink raw reply
* [PATCH 0/2] optimizing pack access on "read only" fetch repos
From: Jeff King @ 2013-01-26 22:40 UTC (permalink / raw)
To: git
This is a repost from here:
http://thread.gmane.org/gmane.comp.version-control.git/211176
which got no response initially. Basically the issue is that read-only
repos (e.g., a CI server) whose workflow is something like:
git fetch $some_branch &&
git checkout -f $some_branch &&
make test
will never run git-gc, and will accumulate a bunch of small packs and
loose objects, leading to poor performance.
Patch 1 runs "gc --auto" on fetch, which I think is sane to do.
Patch 2 optimizes our pack dir re-scanning for fetch-pack (which, unlike
the rest of git, should expect to be missing lots of objects, since we
are deciding what to fetch).
I think 1 is a no-brainer. If your repo is packed, patch 2 matters less,
but it still seems like a sensible optimization to me.
[1/2]: fetch: run gc --auto after fetching
[2/2]: fetch-pack: avoid repeatedly re-scanning pack directory
-Peff
^ permalink raw reply
* [PATCH 1/2] fetch: run gc --auto after fetching
From: Jeff King @ 2013-01-26 22:40 UTC (permalink / raw)
To: git
In-Reply-To: <20130126224011.GA20675@sigill.intra.peff.net>
We generally try to run "gc --auto" after any commands that
might introduce a large number of new objects. An obvious
place to do so is after running "fetch", which may introduce
new loose objects or packs (depending on the size of the
fetch).
While an active developer repository will probably
eventually trigger a "gc --auto" on another action (e.g.,
git-rebase), there are two good reasons why it is nicer to
do it at fetch time:
1. Read-only repositories which track an upstream (e.g., a
continuous integration server which fetches and builds,
but never makes new commits) will accrue loose objects
and small packs, but never coalesce them into a more
efficient larger pack.
2. Fetching is often already perceived to be slow to the
user, since they have to wait on the network. It's much
more pleasant to include a potentially slow auto-gc as
part of the already-long network fetch than in the
middle of productive work with git-rebase or similar.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/fetch.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4b5a898..1ddbf0d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -959,6 +959,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
struct string_list list = STRING_LIST_INIT_NODUP;
struct remote *remote;
int result = 0;
+ static const char *argv_gc_auto[] = {
+ "gc", "--auto", NULL,
+ };
packet_trace_identity("fetch");
@@ -1026,5 +1029,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
list.strdup_strings = 1;
string_list_clear(&list, 0);
+ run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+
return result;
}
--
1.8.0.2.16.g72e2fc9
^ permalink raw reply related
* [PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
From: Jeff King @ 2013-01-26 22:40 UTC (permalink / raw)
To: git
In-Reply-To: <20130126224011.GA20675@sigill.intra.peff.net>
When we look up a sha1 object for reading, we first check
packfiles, and then loose objects. If we still haven't found
it, we re-scan the list of packfiles in `objects/pack`. This
final step ensures that we can co-exist with a simultaneous
repack process which creates a new pack and then prunes the
old object.
This extra re-scan usually does not have a performance
impact for two reasons:
1. If an object is missing, then typically the re-scan
will find a new pack, then no more misses will occur.
Or if it truly is missing, then our next step is
usually to die().
2. Re-scanning is cheap enough that we do not even notice.
However, these do not always hold. The assumption in (1) is
that the caller is expecting to find the object. This is
usually the case, but the call to `parse_object` in
`everything_local` does not follow this pattern. It is
looking to see whether we have objects that the remote side
is advertising, not something we expect to have. Therefore
if we are fetching from a remote which has many refs
pointing to objects we do not have, we may end up
re-scanning the pack directory many times.
Even with this extra re-scanning, the impact is often not
noticeable due to (2); we just readdir() the packs directory
and skip any packs that are already loaded. However, if
there are a large number of packs, then even enumerating the
directory directory can be expensive (especially if we do it
repeatedly). Having this many packs is a good sign the user
should run `git gc`, but it would still be nice to avoid
having to scan the directory at all.
Signed-off-by: Jeff King <peff@peff.net>
---
fetch-pack.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fetch-pack.c b/fetch-pack.c
index f0acdf7..6d8926a 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -594,6 +594,9 @@ static int everything_local(struct fetch_pack_args *args,
for (ref = *refs; ref; ref = ref->next) {
struct object *o;
+ if (!has_sha1_file(ref->old_sha1))
+ continue;
+
o = parse_object(ref->old_sha1);
if (!o)
continue;
--
1.8.0.2.16.g72e2fc9
^ permalink raw reply related
* [PATCH] git-remote-testpy: fix patch hashing on Python 3
From: John Keeping @ 2013-01-26 23:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Sverre Rabbelier
In-Reply-To: <7vwquzzkiw.fsf@alter.siamese.dyndns.org>
When this change was originally made (0846b0c - git-remote-testpy: hash bytes
explicitly , I didn't realised that the "hex" encoding we chose is a "bytes to
bytes" encoding so it just fails with an error on Python 3 in the same way as
the original code.
Since we want to convert a Unicode string to bytes, UTF-8 really is the best
option here.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
On Sat, Jan 26, 2013 at 01:44:55PM -0800, Junio C Hamano wrote:
> Ahh. I think it is already in "next", so this needs to be turned
> into an incremental to flip 'hex' to 'utf-8', with the justification
> being these five lines above.
Here it is, based on next obviously.
git-remote-testpy.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
index c7a04ec..4713363 100644
--- a/git-remote-testpy.py
+++ b/git-remote-testpy.py
@@ -45,7 +45,7 @@ def get_repo(alias, url):
repo.get_head()
hasher = _digest()
- hasher.update(repo.path.encode('hex'))
+ hasher.update(repo.path.encode('utf-8'))
repo.hash = hasher.hexdigest()
repo.get_base_path = lambda base: os.path.join(
--
1.8.1.1
^ permalink raw reply related
* [PATCH 1/2] mergetool--lib: don't call "exit" in setup_tool
From: David Aguilar @ 2013-01-27 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
From: John Keeping <john@keeping.me.uk>
This will make it easier to use setup_tool in places where we expect
that the selected tool will not support the current mode.
We need to introduce a new return code for setup_tool to differentiate
between the case of "the selected tool is invalid" and "the selected
tool is not a built-in" since we must call setup_tool when a custom
'merge.<tool>.path' is configured for a built-in tool but avoid failing
when the configured tool is not a built-in.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This series is based on jk/mergetool in "pu".
This patch is unchanged from $gmane/214624.
git-mergetool--lib.sh | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index aa38bd1..f1bb372 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -58,7 +58,11 @@ setup_tool () {
. "$mergetools/defaults"
if ! test -f "$mergetools/$tool"
then
- return 1
+ # Use a special return code for this case since we want to
+ # source "defaults" even when an explicit tool path is
+ # configured since the user can use that to override the
+ # default path in the scriptlet.
+ return 2
fi
# Load the redefined functions
@@ -67,11 +71,11 @@ setup_tool () {
if merge_mode && ! can_merge
then
echo "error: '$tool' can not be used to resolve merges" >&2
- exit 1
+ return 1
elif diff_mode && ! can_diff
then
echo "error: '$tool' can only be used to resolve merges" >&2
- exit 1
+ return 1
fi
return 0
}
@@ -101,6 +105,19 @@ run_merge_tool () {
# Bring tool-specific functions into scope
setup_tool "$1"
+ exitcode=$?
+ case $exitcode in
+ 0)
+ :
+ ;;
+ 2)
+ # The configured tool is not a built-in tool.
+ test -n "$merge_tool_path" || return 1
+ ;;
+ *)
+ return $exitcode
+ ;;
+ esac
if merge_mode
then
--
1.8.0.8.g9bc9422
^ permalink raw reply related
* [PATCH v2 2/2] mergetools: Simplify how we handle "vim" and "defaults"
From: David Aguilar @ 2013-01-27 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359247207-71819-1-git-send-email-davvid@gmail.com>
Remove the exceptions for "vim" and "defaults" in the mergetool library
so that every filename in mergetools/ matches 1:1 with the name of a
valid built-in tool.
Make common functions available in $MERGE_TOOLS_DIR/include/.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This diffstat is much nicer now thanks to John's setup_tool rework in 1/1.
Makefile | 6 +++-
git-mergetool--lib.sh | 41 ++++++++--------------------
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2 | 1 +
mergetools/{defaults => include/defaults.sh} | 0
mergetools/{vim => vimdiff} | 0
mergetools/vimdiff2 | 1 +
7 files changed, 19 insertions(+), 31 deletions(-)
create mode 100644 mergetools/gvimdiff
create mode 100644 mergetools/gvimdiff2
rename mergetools/{defaults => include/defaults.sh} (100%)
rename mergetools/{vim => vimdiff} (100%)
create mode 100644 mergetools/vimdiff2
diff --git a/Makefile b/Makefile
index f69979e..0f89032 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,7 +2724,11 @@ install: all
$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
- $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ $(INSTALL) -m 644 $(filter-out include,$(wildcard mergetools/*)) \
+ '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
+ $(INSTALL) -m 644 mergetools/include/* \
+ '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
ifndef NO_GETTEXT
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale && $(TAR) cf - .) | \
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f1bb372..7ea7510 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,7 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -44,19 +46,9 @@ valid_tool () {
}
setup_tool () {
- case "$1" in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool="$1"
- ;;
- esac
- mergetools="$(git --exec-path)/mergetools"
+ tool="$1"
- # Load the default definitions
- . "$mergetools/defaults"
- if ! test -f "$mergetools/$tool"
+ if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
# Use a special return code for this case since we want to
# source "defaults" even when an explicit tool path is
@@ -65,8 +57,11 @@ setup_tool () {
return 2
fi
+ # Load the default functions
+ . "$MERGE_TOOLS_DIR/include/defaults.sh"
+
# Load the redefined functions
- . "$mergetools/$tool"
+ . "$MERGE_TOOLS_DIR/$tool"
if merge_mode && ! can_merge
then
@@ -194,24 +189,10 @@ list_merge_tool_candidates () {
show_tool_help () {
unavailable= available= LF='
'
-
- scriptlets="$(git --exec-path)"/mergetools
- for i in "$scriptlets"/*
+ for i in "$MERGE_TOOLS_DIR"/*
do
- . "$scriptlets"/defaults
- . "$i"
-
- tool="$(basename "$i")"
- if test "$tool" = "defaults"
- then
- continue
- elif merge_mode && ! can_merge
- then
- continue
- elif diff_mode && ! can_diff
- then
- continue
- fi
+ tool=$(basename "$i")
+ setup_tool "$tool" 2>/dev/null || continue
merge_tool_path=$(translate_merge_tool_path "$tool")
if type "$merge_tool_path" >/dev/null 2>&1
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/defaults b/mergetools/include/defaults.sh
similarity index 100%
rename from mergetools/defaults
rename to mergetools/include/defaults.sh
diff --git a/mergetools/vim b/mergetools/vimdiff
similarity index 100%
rename from mergetools/vim
rename to mergetools/vimdiff
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/vimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
--
1.8.0.8.g9bc9422
^ permalink raw reply related
* [PATCH v3 2/2] mergetools: Simplify how we handle "vim" and "defaults"
From: David Aguilar @ 2013-01-27 0:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, John Keeping
Remove the exceptions for "vim" and "defaults" in the mergetool library
so that every filename in mergetools/ matches 1:1 with the name of a
valid built-in tool.
Make common functions available in $MERGE_TOOLS_DIR/include/.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
v2 used "include" instead of "mergetools/include"
in the Makefile. Please ignore it.
Makefile | 6 +++-
git-mergetool--lib.sh | 41 ++++++++--------------------
mergetools/gvimdiff | 1 +
mergetools/gvimdiff2 | 1 +
mergetools/{defaults => include/defaults.sh} | 0
mergetools/{vim => vimdiff} | 0
mergetools/vimdiff2 | 1 +
7 files changed, 19 insertions(+), 31 deletions(-)
create mode 100644 mergetools/gvimdiff
create mode 100644 mergetools/gvimdiff2
rename mergetools/{defaults => include/defaults.sh} (100%)
rename mergetools/{vim => vimdiff} (100%)
create mode 100644 mergetools/vimdiff2
diff --git a/Makefile b/Makefile
index f69979e..26f217f 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,7 +2724,11 @@ install: all
$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
- $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ $(INSTALL) -m 644 $(filter-out mergetools/include,$(wildcard mergetools/*)) \
+ '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
+ $(INSTALL) -m 644 mergetools/include/* \
+ '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'
ifndef NO_GETTEXT
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'
(cd po/build/locale && $(TAR) cf - .) | \
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f1bb372..7ea7510 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -1,5 +1,7 @@
#!/bin/sh
# git-mergetool--lib is a library for common merge tool functions
+MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
+
diff_mode() {
test "$TOOL_MODE" = diff
}
@@ -44,19 +46,9 @@ valid_tool () {
}
setup_tool () {
- case "$1" in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool="$1"
- ;;
- esac
- mergetools="$(git --exec-path)/mergetools"
+ tool="$1"
- # Load the default definitions
- . "$mergetools/defaults"
- if ! test -f "$mergetools/$tool"
+ if ! test -f "$MERGE_TOOLS_DIR/$tool"
then
# Use a special return code for this case since we want to
# source "defaults" even when an explicit tool path is
@@ -65,8 +57,11 @@ setup_tool () {
return 2
fi
+ # Load the default functions
+ . "$MERGE_TOOLS_DIR/include/defaults.sh"
+
# Load the redefined functions
- . "$mergetools/$tool"
+ . "$MERGE_TOOLS_DIR/$tool"
if merge_mode && ! can_merge
then
@@ -194,24 +189,10 @@ list_merge_tool_candidates () {
show_tool_help () {
unavailable= available= LF='
'
-
- scriptlets="$(git --exec-path)"/mergetools
- for i in "$scriptlets"/*
+ for i in "$MERGE_TOOLS_DIR"/*
do
- . "$scriptlets"/defaults
- . "$i"
-
- tool="$(basename "$i")"
- if test "$tool" = "defaults"
- then
- continue
- elif merge_mode && ! can_merge
- then
- continue
- elif diff_mode && ! can_diff
- then
- continue
- fi
+ tool=$(basename "$i")
+ setup_tool "$tool" 2>/dev/null || continue
merge_tool_path=$(translate_merge_tool_path "$tool")
if type "$merge_tool_path" >/dev/null 2>&1
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/defaults b/mergetools/include/defaults.sh
similarity index 100%
rename from mergetools/defaults
rename to mergetools/include/defaults.sh
diff --git a/mergetools/vim b/mergetools/vimdiff
similarity index 100%
rename from mergetools/vim
rename to mergetools/vimdiff
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 100644
index 0000000..04a5bb0
--- /dev/null
+++ b/mergetools/vimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
--
1.8.0.8.gd6b90fb.dirty
^ permalink raw reply related
* Re: [PATCH 1/2] fetch: run gc --auto after fetching
From: Jonathan Nieder @ 2013-01-27 1:51 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20130126224038.GA20849@sigill.intra.peff.net>
Jeff King wrote:
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -959,6 +959,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> struct string_list list = STRING_LIST_INIT_NODUP;
> struct remote *remote;
> int result = 0;
> + static const char *argv_gc_auto[] = {
> + "gc", "--auto", NULL,
> + };
>
> packet_trace_identity("fetch");
>
> @@ -1026,5 +1029,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
> list.strdup_strings = 1;
> string_list_clear(&list, 0);
>
> + run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
> +
> return result;
Good idea, and the execution is obviously correct. Thanks.
^ permalink raw reply
* Re: [PATCH 06/21] git p4 test: use client_view in t9806
From: Pete Wyckoff @ 2013-01-27 1:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nmiklbt.fsf@alter.siamese.dyndns.org>
Yes, this really is four months later. Somehow I forgot all
about this series.
gitster@pobox.com wrote on Fri, 28 Sep 2012 12:11 -0700:
> Pete Wyckoff <pw@padd.com> writes:
>
> > Use the standard client_view function from lib-git-p4.sh
> > instead of building one by hand. This requires a bit of
> > rework, using the current value of $P4CLIENT for the client
> > name. It also reorganizes the test to isolate changes to
> > $P4CLIENT and $cli in a subshell.
> >
> > Signed-off-by: Pete Wyckoff <pw@padd.com>
> > ---
> > t/lib-git-p4.sh | 4 ++--
> > t/t9806-git-p4-options.sh | 50 ++++++++++++++++++++++-------------------------
> > 2 files changed, 25 insertions(+), 29 deletions(-)
> >
> > diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
> > index 890ee60..d558dd0 100644
> > --- a/t/lib-git-p4.sh
> > +++ b/t/lib-git-p4.sh
> > @@ -116,8 +116,8 @@ marshal_dump() {
> > client_view() {
> > (
> > cat <<-EOF &&
> > - Client: client
> > - Description: client
> > + Client: $P4CLIENT
> > + Description: $P4CLIENT
> > Root: $cli
> > View:
> > EOF
> > diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
> > index fa40cc8..37ca30a 100755
> > --- a/t/t9806-git-p4-options.sh
> > +++ b/t/t9806-git-p4-options.sh
> > @@ -126,37 +126,33 @@ test_expect_success 'clone --use-client-spec' '
> > exec >/dev/null &&
> > test_must_fail git p4 clone --dest="$git" --use-client-spec
> > ) &&
> > - cli2=$(test-path-utils real_path "$TRASH_DIRECTORY/cli2") &&
> > + # build a different client
> > + cli2="$TRASH_DIRECTORY/cli2" &&
> > mkdir -p "$cli2" &&
> > test_when_finished "rmdir \"$cli2\"" &&
> > test_when_finished cleanup_git &&
> > ...
> > - # same thing again, this time with variable instead of option
> > (
> > ...
> > + # group P4CLIENT and cli changes in a sub-shell
> > + P4CLIENT=client2 &&
> > + cli="$cli2" &&
> > + client_view "//depot/sub/... //client2/bus/..." &&
> > + git p4 clone --dest="$git" --use-client-spec //depot/... &&
> > + (
> > + cd "$git" &&
> > + test_path_is_file bus/dir/f4 &&
> > + test_path_is_missing file1
> > + ) &&
> > + cleanup_git &&
>
> Hmm, the use of "test-path-utils real_path" to form cli2 in the
> original was not necessary at all?
Thanks, I will make this removal more explicit, putting it in
with 8/21 where it belongs, with explanation.
> > + # same thing again, this time with variable instead of option
> > + (
> > + cd "$git" &&
> > + git init &&
> > + git config git-p4.useClientSpec true &&
> > + git p4 sync //depot/... &&
> > + git checkout -b master p4/master &&
> > + test_path_is_file bus/dir/f4 &&
> > + test_path_is_missing file1
> > + )
>
> Do you need a separate sub-shell inside a sub-shell we are already
> in that you called client_view in?
>
> > )
> > '
The first subshell is to hide P4CLIENT and cli variable changes
from the rest of the tests.
The second is to keep the "cd $git" from changing behavior of the
following "cleanup_git" call. That does "rm -rf $git" which
would fail on some file systems if cwd is still in there. With
just one subshell it would look like:
(
P4CLIENT=client2 &&
git p4 clone .. &&
cd "$git" &&
... do test
cd "$TRASH_DIRECTORY" &&
cleanup_git &&
cd "$git" &&
... more test
)
It's a bit easier to understand with an extra level of shell,
and sticks to the pattern used in the rest of the t98*.
-- Pete
^ permalink raw reply
* [PATCHv2 00/21] git p4: work on cygwin
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
Junio and Hannes: thanks for the comments four months ago; I've
been slow getting back to this. I incorporated all your
suggestions.
Junio: this merges okay with Brandon's v2.4 support series.
This series fixes problems in git-p4, and its tests, so that
git-p4 works on the cygwin platform.
See the wiki for info on how to get started on cygwin:
https://git.wiki.kernel.org/index.php/GitP4
Testing by people who use cygwin would be appreciated. It would
be good to support cygwin more regularly. Anyone who had time
to contribute to testing on cygwin, and reporting problems, would
be welcome.
There's more work requried to support msysgit. Those patches
are not in good enough shape to ship out yet, but a lot of what
is in this series is required for msysgit too.
These patches:
- fix bugs in git-p4 related to issues found on cygwin
- cleanup some ugly code in git-p4 observed in error paths while
getting tests to work on cygwin
- simplify and refactor code and tests to make cygwin changes easier
- handle newline and path issues for cygwin platform
- speed up some aspects of git-p4 by removing extra shell invocations
Changes from v1:
http://thread.gmane.org/gmane.comp.version-control.git/206557
- Addressed comments from Junio and Hannes:
- Removed "git p4: fix error message when "describe -s" fails";
it was fixed as part of 18fa13d (git p4: catch p4 describe
errors, 2012-11-23), with messages like "p4 describe -s ...
failed".
- Removed extranneous "grep -q" in "git p4: generate better
error message for bad depot path".
- Added "git p4 test: avoid loop in client_view" after a
suggestion from Junio.
- Made the test-path-utils removal explicit.
- Modify the chmod test to use test_chmod, and verify at
least the p4 bits on cygwin, although not the filesystem.
- Retested on latest cygwin
Pete Wyckoff (21):
git p4: temp branch name should use / even on windows
git p4: remove unused imports
git p4: generate better error message for bad depot path
git p4 test: use client_view to build the initial client
git p4 test: avoid loop in client_view
git p4 test: use client_view in t9806
git p4 test: start p4d inside its db dir
git p4 test: translate windows paths for cygwin
git p4: remove unreachable windows \r\n conversion code
git p4: scrub crlf for utf16 files on windows
git p4 test: newline handling
git p4 test: use LineEnd unix in windows tests too
git p4 test: avoid wildcard * in windows
git p4: cygwin p4 client does not mark read-only
git p4 test: use test_chmod for cygwin
git p4: disable read-only attribute before deleting
git p4: avoid shell when mapping users
git p4: avoid shell when invoking git rev-list
git p4: avoid shell when invoking git config --get-all
git p4: avoid shell when calling git config
git p4: introduce gitConfigBool
git-p4.py | 119 ++++++++++++++++++++++++++++--------------
t/lib-git-p4.sh | 64 ++++++++++++++++-------
t/t9800-git-p4-basic.sh | 5 ++
t/t9802-git-p4-filetype.sh | 117 +++++++++++++++++++++++++++++++++++++++++
t/t9806-git-p4-options.sh | 51 ++++++++----------
t/t9807-git-p4-submit.sh | 14 ++++-
t/t9809-git-p4-client-view.sh | 16 ++++--
t/t9812-git-p4-wildcards.sh | 37 ++++++++++---
t/t9815-git-p4-submit-fail.sh | 11 ++--
t/test-lib.sh | 3 ++
10 files changed, 332 insertions(+), 105 deletions(-)
--
1.8.1.1.460.g6fa8886
^ permalink raw reply
* [PATCHv2 01/21] git p4: temp branch name should use / even on windows
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Commit fed2369 (git-p4: Search for parent commit on branch creation,
2012-01-25) uses temporary branches to help find the parent of a
new p4 branch. The temp branches are of the form "git-p4-tmp/%d"
for some p4 change number. Mistakenly, this string was made
using os.path.join() instead of just string concatenation. On
windows, this turns into a backslash (\), which is not allowed in
git branch names.
Reported-by: Casey McGinty <casey.mcginty@gmail.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 2da5649..fb77c56 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2687,7 +2687,7 @@ class P4Sync(Command, P4UserMap):
blob = None
if len(parent) > 0:
- tempBranch = os.path.join(self.tempBranchLocation, "%d" % (change))
+ tempBranch = "%s/%d" % (self.tempBranchLocation, change)
if self.verbose:
print "Creating temporary branch: " + tempBranch
self.commit(description, filesForCommit, tempBranch)
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 02/21] git p4: remove unused imports
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Found by "pyflakes" checker tool.
Modules shelve, getopt were unused.
Module os.path is exported by os.
Reformat one-per-line as is PEP008 suggested style.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fb77c56..47d092d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -7,16 +7,20 @@
# 2007 Trolltech ASA
# License: MIT <http://www.opensource.org/licenses/mit-license.php>
#
-
import sys
if sys.hexversion < 0x02040000:
# The limiter is the subprocess module
sys.stderr.write("git-p4: requires Python 2.4 or later.\n")
sys.exit(1)
-
-import optparse, os, marshal, subprocess, shelve
-import tempfile, getopt, os.path, time, platform
-import re, shutil
+import os
+import optparse
+import marshal
+import subprocess
+import tempfile
+import time
+import platform
+import re
+import shutil
verbose = False
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 03/21] git p4: generate better error message for bad depot path
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Depot paths must start with //. Exit with a better explanation
when a bad depot path is supplied.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 1 +
t/t9800-git-p4-basic.sh | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index 47d092d..cbf8525 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3163,6 +3163,7 @@ class P4Clone(P4Sync):
self.cloneExclude = ["/"+p for p in self.cloneExclude]
for p in depotPaths:
if not p.startswith("//"):
+ sys.stderr.write('Depot paths must start with "//": %s\n' % p)
return False
if not self.cloneDestination:
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 166e752..665607c 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -30,6 +30,11 @@ test_expect_success 'basic git p4 clone' '
)
'
+test_expect_success 'depot typo error' '
+ test_must_fail git p4 clone --dest="$git" /depot 2>errs &&
+ grep "Depot paths must start with" errs
+'
+
test_expect_success 'git p4 clone @all' '
git p4 clone --dest="$git" //depot@all &&
test_when_finished cleanup_git &&
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 04/21] git p4 test: use client_view to build the initial client
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Simplify the code a bit by using an existing function.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 7061dce..890ee60 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -74,15 +74,8 @@ start_p4d() {
fi
# build a client
- (
- cd "$cli" &&
- p4 client -i <<-EOF
- Client: client
- Description: client
- Root: $cli
- View: //depot/... //client/...
- EOF
- )
+ client_view "//depot/... //client/..." &&
+
return 0
}
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 05/21] git p4 test: avoid loop in client_view
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
The printf command re-interprets the format string as
long as there are arguments to consume. Use this to
simplify a for loop in the client_view() library function.
This requires a fix to one of the client_view callers.
An errant \n in the string was converted into a harmless
newline in the input to "p4 client -i", but now shows up
as a literal \n as passed through by "%s". Remove the \n.
Based-on-patch-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 4 +---
t/t9809-git-p4-client-view.sh | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 890ee60..b1dbded 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -121,8 +121,6 @@ client_view() {
Root: $cli
View:
EOF
- for arg ; do
- printf "\t$arg\n"
- done
+ printf "\t%s\n" "$@"
) | p4 client -i
}
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index 281be29..0b58fb9 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -196,7 +196,7 @@ test_expect_success 'exclusion single file' '
test_expect_success 'overlay wildcard' '
client_view "//depot/dir1/... //client/cli/..." \
- "+//depot/dir2/... //client/cli/...\n" &&
+ "+//depot/dir2/... //client/cli/..." &&
files="cli/file11 cli/file12 cli/file21 cli/file22" &&
client_verify $files &&
test_when_finished cleanup_git &&
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 06/21] git p4 test: use client_view in t9806
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Use the standard client_view function from lib-git-p4.sh
instead of building one by hand. This requires a bit of
rework, using the current value of $P4CLIENT for the client
name. It also reorganizes the test to isolate changes to
$P4CLIENT and $cli in a subshell.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 4 ++--
t/t9806-git-p4-options.sh | 49 ++++++++++++++++++++---------------------------
2 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index b1dbded..c5d1f4d 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -116,8 +116,8 @@ marshal_dump() {
client_view() {
(
cat <<-EOF &&
- Client: client
- Description: client
+ Client: $P4CLIENT
+ Description: $P4CLIENT
Root: $cli
View:
EOF
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 4f077ee..564fc80 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -214,40 +214,33 @@ test_expect_success 'clone --use-client-spec' '
exec >/dev/null &&
test_must_fail git p4 clone --dest="$git" --use-client-spec
) &&
+ # build a different client
cli2=$(test-path-utils real_path "$TRASH_DIRECTORY/cli2") &&
mkdir -p "$cli2" &&
test_when_finished "rmdir \"$cli2\"" &&
- (
- cd "$cli2" &&
- p4 client -i <<-EOF
- Client: client2
- Description: client2
- Root: $cli2
- View: //depot/sub/... //client2/bus/...
- EOF
- ) &&
test_when_finished cleanup_git &&
(
+ # group P4CLIENT and cli changes in a sub-shell
P4CLIENT=client2 &&
- git p4 clone --dest="$git" --use-client-spec //depot/...
- ) &&
- (
- cd "$git" &&
- test_path_is_file bus/dir/f4 &&
- test_path_is_missing file1
- ) &&
- cleanup_git &&
-
- # same thing again, this time with variable instead of option
- (
- cd "$git" &&
- git init &&
- git config git-p4.useClientSpec true &&
- P4CLIENT=client2 &&
- git p4 sync //depot/... &&
- git checkout -b master p4/master &&
- test_path_is_file bus/dir/f4 &&
- test_path_is_missing file1
+ cli="$cli2" &&
+ client_view "//depot/sub/... //client2/bus/..." &&
+ git p4 clone --dest="$git" --use-client-spec //depot/... &&
+ (
+ cd "$git" &&
+ test_path_is_file bus/dir/f4 &&
+ test_path_is_missing file1
+ ) &&
+ cleanup_git &&
+ # same thing again, this time with variable instead of option
+ (
+ cd "$git" &&
+ git init &&
+ git config git-p4.useClientSpec true &&
+ git p4 sync //depot/... &&
+ git checkout -b master p4/master &&
+ test_path_is_file bus/dir/f4 &&
+ test_path_is_missing file1
+ )
)
'
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 07/21] git p4 test: start p4d inside its db dir
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
This will avoid having to do native path conversion for
windows. Also may be a bit cleaner always to know that p4d
has that working directory, instead of wherever the function
was called from.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index c5d1f4d..185f6f1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -40,8 +40,11 @@ start_p4d() {
mkdir -p "$db" "$cli" "$git" &&
rm -f "$pidfile" &&
(
- p4d -q -r "$db" -p $P4DPORT &
- echo $! >"$pidfile"
+ cd "$db" &&
+ {
+ p4d -q -p $P4DPORT &
+ echo $! >"$pidfile"
+ }
) &&
# This gives p4d a long time to start up, as it can be
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 08/21] git p4 test: translate windows paths for cygwin
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Native windows binaries do not understand posix-like
path mapping offered by cygwin. Convert paths to native
using "cygpath --windows" before presenting them to p4d.
This is done using the AltRoots mechanism of p4. Both the
posix and windows forms are put in the client specification,
allowing p4 to find its location by native path even though
the environment reports a different PWD.
Shell operations in tests will use the normal form of $cli,
which will look like a posix path in cygwin, while p4 will
use AltRoots to match against the windows form of the working
directory.
This mechanism also handles the symlink issue that was fixed in
23bd0c9 (git p4 test: use real_path to resolve p4 client
symlinks, 2012-06-27). Now that every p4 client view has
an AltRoots with the real_path in it, explicitly calculating
the real_path elsewhere is not necessary.
Thanks-to: Sebastian Schuberth <sschuberth@gmail.com>
Thanks-to: Johannes Sixt <j6t@kdbg.org>
fixup! git p4 test: translate windows paths for cygwin
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
t/lib-git-p4.sh | 24 ++++++++++++++++++++++--
t/t9806-git-p4-options.sh | 2 +-
t/test-lib.sh | 3 +++
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 185f6f1..d5596de 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -8,7 +8,8 @@ TEST_NO_CREATE_REPO=NoThanks
. ./test-lib.sh
-if ! test_have_prereq PYTHON; then
+if ! test_have_prereq PYTHON
+then
skip_all='skipping git p4 tests; python not available'
test_done
fi
@@ -17,6 +18,24 @@ fi
test_done
}
+# On cygwin, the NT version of Perforce can be used. When giving
+# it paths, either on the command-line or in client specifications,
+# be sure to use the native windows form.
+#
+# Older versions of perforce were available compiled natively for
+# cygwin. Those do not accept native windows paths, so make sure
+# not to convert for them.
+native_path() {
+ path="$1" &&
+ if test_have_prereq CYGWIN && ! p4 -V | grep -q CYGWIN
+ then
+ path=$(cygpath --windows "$path")
+ else
+ path=$(test-path-utils real_path "$path")
+ fi &&
+ echo "$path"
+}
+
# Try to pick a unique port: guess a large number, then hope
# no more than one of each test is running.
#
@@ -32,7 +51,7 @@ P4EDITOR=:
export P4PORT P4CLIENT P4EDITOR
db="$TRASH_DIRECTORY/db"
-cli=$(test-path-utils real_path "$TRASH_DIRECTORY/cli")
+cli="$TRASH_DIRECTORY/cli"
git="$TRASH_DIRECTORY/git"
pidfile="$TRASH_DIRECTORY/p4d.pid"
@@ -122,6 +141,7 @@ client_view() {
Client: $P4CLIENT
Description: $P4CLIENT
Root: $cli
+ AltRoots: $(native_path "$cli")
View:
EOF
printf "\t%s\n" "$@"
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 564fc80..254d428 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -215,7 +215,7 @@ test_expect_success 'clone --use-client-spec' '
test_must_fail git p4 clone --dest="$git" --use-client-spec
) &&
# build a different client
- cli2=$(test-path-utils real_path "$TRASH_DIRECTORY/cli2") &&
+ cli2="$TRASH_DIRECTORY/cli2" &&
mkdir -p "$cli2" &&
test_when_finished "rmdir \"$cli2\"" &&
test_when_finished cleanup_git &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a6c4ab..9e7f6b4 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -666,12 +666,14 @@ case $(uname -s) in
# backslashes in pathspec are converted to '/'
# exec does not inherit the PID
test_set_prereq MINGW
+ test_set_prereq NOT_CYGWIN
test_set_prereq SED_STRIPS_CR
;;
*CYGWIN*)
test_set_prereq POSIXPERM
test_set_prereq EXECKEEPSPID
test_set_prereq NOT_MINGW
+ test_set_prereq CYGWIN
test_set_prereq SED_STRIPS_CR
;;
*)
@@ -679,6 +681,7 @@ case $(uname -s) in
test_set_prereq BSLASHPSPEC
test_set_prereq EXECKEEPSPID
test_set_prereq NOT_MINGW
+ test_set_prereq NOT_CYGWIN
;;
esac
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 09/21] git p4: remove unreachable windows \r\n conversion code
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Replacing \r\n with \n on windows was added in c1f9197 (Replace
\r\n with \n when importing from p4 on Windows, 2007-05-24), to
work around an oddity with "p4 print" on windows. Text files
are printed with "\r\r\n" endings, regardless of whether they
were created on unix or windows, and regardless of the client
LineEnd setting.
As of d2c6dd3 (use p4CmdList() to get file contents in Python
dicts. This is more robust., 2007-05-23), git-p4 uses "p4 -G
print", which generates files in a raw format. As the native
line ending format if p4 is \n, there will be no \r\n in the
raw text.
Actually, it is possible to generate a text file so that the
p4 representation includes embedded \r\n, even though this is not
normal on either windows or unix. In that case the code would
have mistakenly stripped them out, but now they will be left
intact.
More information on how p4 deals with line endings is here:
http://kb.perforce.com/article/63
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index cbf8525..445d704 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2134,15 +2134,6 @@ class P4Sync(Command, P4UserMap):
print "\nIgnoring apple filetype file %s" % file['depotFile']
return
- # Perhaps windows wants unicode, utf16 newlines translated too;
- # but this is not doing it.
- if self.isWindows and type_base == "text":
- mangled = []
- for data in contents:
- data = data.replace("\r\n", "\n")
- mangled.append(data)
- contents = mangled
-
# Note that we do not try to de-mangle keywords on utf16 files,
# even though in theory somebody may want that.
pattern = p4_keywords_regexp_for_type(type_base, type_mods)
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
* [PATCHv2 10/21] git p4: scrub crlf for utf16 files on windows
From: Pete Wyckoff @ 2013-01-27 3:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Sixt
In-Reply-To: <1359256284-5660-1-git-send-email-pw@padd.com>
Files of type utf16 are handled with "p4 print" instead of the
normal "p4 -G print" interface due to how the latter does not
produce correct output. See 55aa571 (git-p4: handle utf16
filetype properly, 2011-09-17) for details.
On windows, though, "p4 print" can not be told which line
endings to use, as there is no underlying client, and always
chooses crlf, even for utf16 files. Convert the \r\n into \n
when importing utf16 files.
The fix for this is complex, in that the problem is a property
of the NT version of p4. There are old versions of p4 that
were compiled directly for cygwin that should not be subjected
to text replacement. The right check here, then, is to look
at the p4 version, not the OS version. Note also that on cygwin,
platform.system() is "CYGWIN_NT-5.1" or similar, not "Windows".
Add a function to memoize the p4 version string and use it to
check for "/NT", indicating the Windows build of p4.
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 445d704..c62b2ca 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -170,6 +170,22 @@ def p4_system(cmd):
expand = isinstance(real_cmd, basestring)
subprocess.check_call(real_cmd, shell=expand)
+_p4_version_string = None
+def p4_version_string():
+ """Read the version string, showing just the last line, which
+ hopefully is the interesting version bit.
+
+ $ p4 -V
+ Perforce - The Fast Software Configuration Management System.
+ Copyright 1995-2011 Perforce Software. All rights reserved.
+ Rev. P4/NTX86/2011.1/393975 (2011/12/16).
+ """
+ global _p4_version_string
+ if not _p4_version_string:
+ a = p4_read_pipe_lines(["-V"])
+ _p4_version_string = a[-1].rstrip()
+ return _p4_version_string
+
def p4_integrate(src, dest):
p4_system(["integrate", "-Dt", wildcard_encode(src), wildcard_encode(dest)])
@@ -1973,7 +1989,6 @@ class P4Sync(Command, P4UserMap):
self.syncWithOrigin = True
self.importIntoRemotes = True
self.maxChanges = ""
- self.isWindows = (platform.system() == "Windows")
self.keepRepoPath = False
self.depotPaths = None
self.p4BranchesInGit = []
@@ -2118,7 +2133,14 @@ class P4Sync(Command, P4UserMap):
# operations. utf16 is converted to ascii or utf8, perhaps.
# But ascii text saved as -t utf16 is completely mangled.
# Invoke print -o to get the real contents.
+ #
+ # On windows, the newlines will always be mangled by print, so put
+ # them back too. This is not needed to the cygwin windows version,
+ # just the native "NT" type.
+ #
text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+ if p4_version_string().find("/NT") >= 0:
+ text = text.replace("\r\n", "\n")
contents = [ text ]
if type_base == "apple":
--
1.8.1.1.460.g6fa8886
^ permalink raw reply related
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