[PATCH 0/3] lazily load commit-buffer
On Fri, Jan 25, 2013 at 07:36:18AM -0800, Junio C Hamano wrote:
Jonathon Mah j...@me.com writes:
Just to note, the proposals so far don't prevent a smart-ass
function from freeing the buffer when it's called underneath the
use/release scope, as in:
with_commit_buffer(commit); {
fn1_needing_buffer(commit);
walk_rev_tree_or_something();
fn2_needing_buffer(commit);
} done_with_commit_buffer(commit);
I think the goal of everybody discussing these ideas is to make sure
that all code follows the simple ownership policy proposed at the
beginning of this subthread: commit-buffer belongs to the revision
traversal machinery, and other users could borrow it when available.
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.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] commit: drop useless xstrdup of commit message
When git-commit is asked to reuse a commit message via -c, we call read_commit_message, which looks up the commit and hands back either the re-encoded result, or a copy of the original. We make a copy in the latter case so that the ownership semantics of the return value are clear (in either case, it can be freed). However, since we return a const char *, and since the resulting buffer's lifetime is the same as that of the whole program, we never bother to free it at all. Let's just drop the copy. That saves us a copy in the common case. While it does mean we leak in the re-encode case, it doesn't matter, since we are relying on program exit to free the memory anyway. Signed-off-by: Jeff King p...@peff.net --- This one isn't strictly necessary, but it makes it a lot more obvious what is going on with the memory ownership of this code in the next patch. builtin/commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index 38b9a9c..fbbb40f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -962,7 +962,7 @@ static const char *read_commit_message(const char *name) * encodings are identical. */ if (out == NULL) - out = xstrdup(commit-buffer); + out = commit-buffer; return out; } -- 1.8.0.2.16.g72e2fc9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] logmsg_reencode: lazily load missing commit buffers
Usually a commit that makes it to logmsg_reencode will have
been parsed, and the commit-buffer struct member will be
valid. However, some code paths will free commit buffers
after having used them (for example, the log traversal
machinery will do so to keep memory usage down).
Most of the time this is fine; log should only show a commit
once, and then exits. However, there are some code paths
where this does not work. At least two are known:
1. A commit may be shown as part of a regular ref, and
then it may be shown again as part of a submodule diff
(e.g., if a repo contains refs to both the superproject
and subproject).
2. A notes-cache commit may be shown during log --all,
and then later used to access a textconv cache during a
diff.
Lazily loading in logmsg_reencode does not necessarily catch
all such cases, but it should catch most of them. Users of
the commit buffer tend to be either parsing for structure
(in which they will call parse_commit, and either we will
already have parsed, or we will load commit-buffer lazily
there), or outputting (either to the user, or fetching a
part of the commit message via format_commit_message). In
the latter case, we should always be using logmsg_reencode
anyway (and typically we do so via the pretty-print
machinery).
If there are any cases that this misses, we can fix them up
to use logmsg_reencode (or handle them on a case-by-case
basis if that is inappropriate).
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 13 -
pretty.c | 57 ++--
t/t4042-diff-textconv-caching.sh | 8 ++
3 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 962e4e3..86100e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1424,19 +1424,6 @@ static void get_commit_info(struct commit *commit,
commit_info_init(ret);
- /*
-* We've operated without save_commit_buffer, so
-* we now need to populate them for output.
-*/
- if (!commit-buffer) {
- enum object_type type;
- unsigned long size;
- commit-buffer =
- read_sha1_file(commit-object.sha1, type, size);
- if (!commit-buffer)
- die(Cannot read commit %s,
- sha1_to_hex(commit-object.sha1));
- }
encoding = get_log_output_encoding();
message = logmsg_reencode(commit, encoding);
get_ac_line(message, \nauthor ,
diff --git a/pretty.c b/pretty.c
index c675349..eae57ad 100644
--- a/pretty.c
+++ b/pretty.c
@@ -592,18 +592,59 @@ char *logmsg_reencode(const struct commit *commit,
char *msg = commit-buffer;
char *out;
+ if (!msg) {
+ enum object_type type;
+ unsigned long size;
+
+ msg = read_sha1_file(commit-object.sha1, type, size);
+ if (!msg)
+ die(Cannot read commit object %s,
+ sha1_to_hex(commit-object.sha1));
+ if (type != OBJ_COMMIT)
+ die(Expected commit for '%s', got %s,
+ sha1_to_hex(commit-object.sha1), typename(type));
+ }
+
if (!output_encoding || !*output_encoding)
return msg;
encoding = get_header(commit, msg, encoding);
use_encoding = encoding ? encoding : utf8;
- if (same_encoding(use_encoding, output_encoding))
- if (encoding) /* we'll strip encoding header later */
- out = xstrdup(commit-buffer);
- else
- return msg; /* nothing to do */
- else
- out = reencode_string(commit-buffer,
- output_encoding, use_encoding);
+ if (same_encoding(use_encoding, output_encoding)) {
+ /*
+* No encoding work to be done. If we have no encoding header
+* at all, then there's nothing to do, and we can return the
+* message verbatim (whether newly allocated or not).
+*/
+ if (!encoding)
+ return msg;
+
+ /*
+* Otherwise, we still want to munge the encoding header in the
+* result, which will be done by modifying the buffer. If we
+* are using a fresh copy, we can reuse it. But if we are using
+* the cached copy from commit-buffer, we need to duplicate it
+* to avoid munging commit-buffer.
+*/
+ out = msg;
+ if (out == commit-buffer)
+ out = xstrdup(out);
+ }
+ else {
+ /*
+* There's actual encoding work to do. Do the reencoding, which
+* still leaves the header to be
Re: git merge error question: The following untracked working tree files would be overwritten by merge
Am 2013-01-25 19:07, schrieb Junio C Hamano: Carsten Fuchs carsten.fu...@cafu.de writes: [...] $ git merge origin/master --ff-only Updating f419d57..2da6052 error: The following untracked working tree files would be overwritten by merge: obsolete/e107/Readme.txt obsolete/e107/article.php obsolete/e107/backend.php [...] ... Compare with what Subversion did in an analogous case: When I ran svn update and the update brought new files for which there already was an untracked copy in the working directory, Subversion: - started to consider the file as tracked, - but left the file in the working-copy alone. As a result, a subsequent svn status might a) no longer show the file at all, if the foreign copy in the working directory happened to be the same as the one brought by the svn update, or b) flag the file as modified, if different from the one that svn update would have created in its place. Interesting. So before running status, the merge is recorded (in this particular case you are doing ff-only so there is nothing new to record, but if the rest of the tree merges cleanly, the new tree that contains obsolete from the other branch you just merged will be the contents you record in the merge commit), and working tree is immediately dirty? Yes. But I don't think it's the (svn) status command that does anything special. In Git, if I understand it correctly, the final step of a merge, checkout, reset, etc. is a move of the HEAD to the resulting or specified commit. I imagine that it is here where the diff of the dirty working tree is re-applied to the newly checked out commit (and if this is not possible cleanly, probably [a] the whole operation must abort, or [b] leave files in the working tree with conflict markers), and where a decision must be made about obstructing paths (svn lingo): [c] abort the whole operation, or [d] version them (but don't modify them in any way). I'm not sure if Subversion does [a] and [c] (abort) without the --force option, and [b] and [d] with --force, or any other combination, but at least TortoiseSVN seems to use [d] by default (which seems safe enough). Despite a thorough search, I've not been able to find much reference about this behaviour: http://svnbook.red-bean.com/en/1.6/svn.ref.svn.c.switch.html http://markphip.blogspot.de/2007/01/subversion-15-tolerate-obstructions.html However, as the blog article mentions, I too have found this treatment of obstructing paths very natural and helpful in several occasions. (Because without it, we must manually rename the obstructing paths, re-start the previously aborted operation, and then take diffs or somehow else compare the renamed obstructing and newly added paths manually, and possible merge them manually; or at least copy the renamed edition over the newly added edition to get back into Git for the job.) So my real question is, why does Git not do something analogous? The answer to that question is because nobody thought that doing so would help users very much and bothered to implement it; it is not people thought about doing so and found reasons why that would not help users. Thanks, it's very good to hear this! :-) Best regards, Carsten -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 v2] mergetool--lib: don't call exit in setup_tool
This will make it easier to use setup_tool in places where we expect
that the selected tool will not support the current mode.
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Fri, Jan 25, 2013 at 04:24:03PM -0800, Junio C Hamano wrote:
Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break
t7610 rather badly.
Sorry about that. The 'setup_tool' function should really be called
'setup_builtin_tool' - it isn't necessary when a custom mergetool is
configured and will return 1 when called with an argument that isn't a
builtin tool from $GIT_EXEC_PATH/mergetools.
The change is the second hunk below which now wraps the call to
setup_tool in an if block as well as adding the || return.
git-mergetool--lib.sh | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4c1e129..8a5eaff 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -67,11 +67,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
}
@@ -100,7 +100,10 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool $1
+ if test -z $merge_tool_path
+ then
+ setup_tool $1 || return
+ fi
if merge_mode
then
--
1.8.1.1.367.ga9c3dd4.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
On Fri, Jan 25, 2013 at 10:50:58PM -0800, David Aguilar wrote:
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 dav...@gmail.com
---
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.
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 aa38bd1..c866ed8 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,25 +46,14 @@ valid_tool () {
}
setup_tool () {
- case $1 in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool=$1
- ;;
- esac
- mergetools=$(git --exec-path)/mergetools
+ tool=$1
Unnecessary quoting.
- # Load the default definitions
- . $mergetools/defaults
- if ! test -f $mergetools/$tool
+ if ! test -f $MERGE_TOOLS_DIR/$tool
then
return 1
fi
-
- # Load the redefined functions
- . $mergetools/$tool
+ . $MERGE_TOOLS_DIR/include/defaults.sh
+ . $MERGE_TOOLS_DIR/$tool
if merge_mode ! can_merge
then
@@ -99,7 +90,7 @@ run_merge_tool () {
base_present=$2
status=0
- # Bring tool-specific functions into scope
+ # Bring tool specific functions into scope
This isn't related to this change (and I think tool-specific is more
correct anyway).
setup_tool $1
if merge_mode
@@ -177,18 +168,17 @@ 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
+ if test -d $i
then
continue
- elif merge_mode ! can_merge
+ fi
+
+ . $MERGE_TOOLS_DIR/include/defaults.sh
+ . $i
+
+ if merge_mode ! can_merge
then
continue
elif diff_mode ! can_diff
@@ -196,6 +186,7 @@ show_tool_help () {
continue
fi
I'd prefer to see my change to setup_tool done before this so that we
can just use:
setup_tool $tool 2/dev/null || continue
for the above block. I'll send a fixed version in a couple of minutes.
+ tool=$(basename $i)
merge_tool_path=$(translate_merge_tool_path $tool)
if type $merge_tool_path /dev/null 21
then
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. $MERGE_TOOLS_DIR/include/vim.sh
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. $MERGE_TOOLS_DIR/include/vim.sh
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/include/vim.sh
similarity index 100%
rename from mergetools/vim
rename to mergetools/include/vim.sh
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/vimdiff
@@ -0,0 +1 @@
+. $MERGE_TOOLS_DIR/include/vim.sh
diff --git
Re: [PATCH 1/2] git-p4.py: support Python 2.5
draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800: Python 2.5 and older do not accept None as the first argument to translate() and complain with: TypeError: expected a character buffer object Satisfy this older python by calling maketrans() to generate an empty translation table and supplying that to translate(). This allows git-p4 to be used with Python 2.5. This was a lot easier than I imagined! def wildcard_present(path): -return path.translate(None, *#@%) != path +from string import maketrans +return path.translate(maketrans(,), *#@%) != path translate() was a bit too subtle already. Could you try something like this instead? m = re.search([*#@%], path) return m is not None I think that'll work everywhere and not force people to look up how translate and maketrans work. -- Pete -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-p4.py: support Python 2.4
draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800:
Python 2.4 lacks the following features:
subprocess.check_call
struct.pack_into
Take a cue from 460d1026 and provide an implementation of the
CalledProcessError exception. Then replace the calls to
subproccess.check_call with calls to subprocess.call that check the return
status and raise a CalledProcessError exception if necessary.
The struct.pack_into in t/9802 can be converted into a single struct.pack
call which is available in Python 2.4.
Excellent. Should have used struct.pack() from the get-go.
Acked-by: Pete Wyckoff p...@padd.com
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 21924df..be299dc 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -105,12 +105,13 @@ build_gendouble() {
cat gendouble.py -\EOF
import sys
import struct
- import array
- s = array.array(c, '\0' * 26)
- struct.pack_into(L, s, 0, 0x00051607) # AppleDouble
- struct.pack_into(L, s, 4, 0x0002) # version 2
- s.tofile(sys.stdout)
+ s = struct.pack(LL18s,
+ 0x00051607, # AppleDouble
+ 0x0002, # version 2
+# pad to 26 bytes
+ )
+ sys.stdout.write(s);
EOF
One stray semicolon.
In terms of maintenance, I'll not run tests with 2.4 or 2.5
myself, but maybe you would be willing to check an RC candidate
each release?
-- Pete
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2 v3] mergetool--lib: don't call exit in setup_tool
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 j...@keeping.me.uk
---
On Fri, Jan 25, 2013 at 04:24:03PM -0800, Junio C Hamano wrote:
Applying this one on top of 1/7 thru 5/7 and 7/7 seems to break
t7610 rather badly.
Sorry about that. The 'setup_tool' function should really be called
'setup_builtin_tool' - it isn't necessary when a custom mergetool is
configured and will return 1 when called with an argument that isn't a
builtin tool from $GIT_EXEC_PATH/mergetools.
The change is the second hunk below which now wraps the call to
setup_tool in an if block as well as adding the || return.
Now that I've run the entire test suite, that still wasn't correct since
it did not correctly handle the case where the user overrides the path
for one of the built-in mergetools.
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 4c1e129..dd4f088 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.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 01/25/2013 08:03 PM, Jonathan Nieder wrote:
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
Maybe WINDOWS_NATIVE should be defined in config.mak.uname?
Otherwise, I tested the patch and it does build / run on Cygwin, but I
cannot run a test suite until next week. I am concerned about subtle
changes due to the various WIN32 tests that were not guarded by
__CYGWIN__ before, haven't stared at the code enough to see if there
could be an issue.
Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 26.01.13 02:03, Jonathan Nieder wrote:
Throughout git, it is assumed that the WIN32 preprocessor symbol is
defined on native Windows setups (mingw and msvc) and not on Cygwin.
On Cygwin, most of the time git can pretend this is just another Unix
machine, and Windows-specific magic is generally counterproductive.
Unfortunately Cygwin *does* define the WIN32 symbol in some headers.
Best to rely on a new git-specific symbol NATIVE_WINDOWS instead,
defined as follows:
#if defined(WIN32) !defined(__CYGWIN__)
# define NATIVE_WINDOWS
#endif
After this change, it should be possible to drop the
CYGWIN_V15_WIN32API setting without any negative effect.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Eric Blake wrote:
Which is why other projects, like gnulib, have
# if (defined _WIN32 || defined __WIN32__) ! defined __CYGWIN__
all over the place.
So, how about this?
Completely untested.
abspath.c | 2 +-
compat/terminal.c | 4 ++--
compat/win32.h| 2 +-
diff-no-index.c | 2 +-
git-compat-util.h | 3 ++-
help.c| 2 +-
run-command.c | 10 +-
test-chmtime.c| 2 +-
thread-utils.c| 2 +-
9 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -3,7 +3,7 @@
#include sigchain.h
#include strbuf.h
-#if defined(HAVE_DEV_TTY) || defined(WIN32)
+#if defined(HAVE_DEV_TTY) || defined(WINDOWS_NATIVE)
static void restore_term(void);
@@ -53,7 +53,7 @@ error:
return -1;
}
-#elif defined(WIN32)
+#elif defined(WINDOWS_NATIVE)
#define INPUT_PATH CONIN$
#define OUTPUT_PATH CONOUT$
diff --git a/compat/win32.h b/compat/win32.h
index 8ce91048..31dd30f7 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,7 +2,7 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
+#ifndef WINDOWS_NATIVE /* Not defined for Cygwin */
#include windows.h
#endif
diff --git a/diff-no-index.c b/diff-no-index.c
index 74da6593..a0bc9c50 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -45,7 +45,7 @@ static int get_mode(const char *path, int *mode)
if (!path || !strcmp(path, /dev/null))
*mode = 0;
-#ifdef _WIN32
+#ifdef WINDOWS_NATIVE
else if (!strcasecmp(path, nul))
*mode = 0;
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b745..ebbdff53 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,10 +85,11 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
+#if defined(WIN32) !defined(__CYGWIN__) /* Both MinGW and MSVC */
#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
#include winsock2.h
#include windows.h
+#define WINDOWS_NATIVE
#endif
#include unistd.h
diff --git a/help.c b/help.c
index 2a42ec6d..cc1e63f7 100644
--- a/help.c
+++ b/help.c
@@ -106,7 +106,7 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
+#if defined(WINDOWS_NATIVE) || defined(__CYGWIN__)
#if defined(__CYGWIN__)
if ((st.st_mode S_IXUSR) == 0)
#endif
diff --git a/run-command.c b/run-command.c
index 04712191..04ac6181 100644
--- a/run-command.c
+++ b/run-command.c
@@ -72,7 +72,7 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static inline void dup_devnull(int to)
{
int fd = open(/dev/null, O_RDWR);
@@ -159,7 +159,7 @@ static const char **prepare_shell_cmd(const char **argv)
die(BUG: shell command is empty);
if (strcspn(argv[0], |;()$`\\\' \t\n*?[#~=%) != strlen(argv[0])) {
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;
#else
nargv[nargc++] = sh;
@@ -182,7 +182,7 @@ static const char **prepare_shell_cmd(const char **argv)
return nargv;
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int execv_shell_cmd(const char **argv)
{
const char **nargv = prepare_shell_cmd(argv);
@@ -193,7 +193,7 @@ static int execv_shell_cmd(const char **argv)
}
#endif
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int child_err = 2;
static int child_notifier = -1;
@@ -330,7 +330,7 @@ fail_pipe:
trace_argv_printf(cmd-argv,
Re: [PATCH 1/2] git-p4.py: support Python 2.5
On Sat, Jan 26, 2013 at 4:45 AM, Pete Wyckoff p...@padd.com wrote: draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800: Python 2.5 and older do not accept None as the first argument to translate() and complain with: TypeError: expected a character buffer object Satisfy this older python by calling maketrans() to generate an empty translation table and supplying that to translate(). This allows git-p4 to be used with Python 2.5. This was a lot easier than I imagined! def wildcard_present(path): -return path.translate(None, *#@%) != path +from string import maketrans +return path.translate(maketrans(,), *#@%) != path translate() was a bit too subtle already. Could you try something like this instead? m = re.search([*#@%], path) return m is not None I think that'll work everywhere and not force people to look up how translate and maketrans work. Yes that's simpler and works fine. -Brandon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Port 22
I am currently a student at Brigham Young University - Idaho and we are use Pagoda Box and Git for our Mobile Apps class. However, the school's network has blocked incoming trafic on port 22. I have been searching through all the tutorials and documents provided by Pagoda Box and Git but have not been able to find a solution to solve this problem. We can use sftp but we then have to manually deploy the latest using the admin panel. Can you help provide a simple solution? Thanks, Craig W Christensen cwcra...@gmail.com chr07...@byui.edu-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-p4.py: support Python 2.4
On Sat, Jan 26, 2013 at 4:48 AM, Pete Wyckoff p...@padd.com wrote: draf...@gmail.com wrote on Fri, 25 Jan 2013 12:44 -0800: + sys.stdout.write(s); One stray semicolon. Whoops. Thanks. In terms of maintenance, I'll not run tests with 2.4 or 2.5 myself, but maybe you would be willing to check an RC candidate each release? Not a problem. I'm sure it will get run much more often then that. -Brandon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] git-p4.py: support Python 2.4
From: Brandon Casey draf...@gmail.com
Python 2.4 lacks the following features:
subprocess.check_call
struct.pack_into
Take a cue from 460d1026 and provide an implementation of the
CalledProcessError exception. Then replace the calls to
subproccess.check_call with calls to subprocess.call that check the return
status and raise a CalledProcessError exception if necessary.
The struct.pack_into in t/9802 can be converted into a single struct.pack
call which is available in Python 2.4.
Signed-off-by: Brandon Casey bca...@nvidia.com
Acked-by: Pete Wyckoff p...@padd.com
---
On 1/26/2013 4:48 AM, Pete Wyckoff wrote:
One stray semicolon.
Fixed.
-Brandon
INSTALL| 2 +-
git-p4.py | 27 ---
t/t9802-git-p4-filetype.sh | 11 ++-
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/INSTALL b/INSTALL
index fc723b3..b96e16d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -131,7 +131,7 @@ Issues of note:
use English. Under autoconf the configure script will do this
automatically if it can't find libintl on the system.
- - Python version 2.5 or later is needed to use the git-p4
+ - Python version 2.4 or later is needed to use the git-p4
interface to Perforce.
- Some platform specific issues are dealt with Makefile rules,
diff --git a/git-p4.py b/git-p4.py
index de1a0b9..625a396 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -18,6 +18,21 @@ import optparse, os, marshal, subprocess, shelve
import tempfile, getopt, os.path, time, platform
import re, shutil
+try:
+from subprocess import CalledProcessError
+except ImportError:
+# from python2.7:subprocess.py
+# Exception classes used by this module.
+class CalledProcessError(Exception):
+This exception is raised when a process run by check_call() returns
+a non-zero exit status. The exit status will be stored in the
+returncode attribute.
+def __init__(self, returncode, cmd):
+self.returncode = returncode
+self.cmd = cmd
+def __str__(self):
+return Command '%s' returned non-zero exit status %d %
(self.cmd, self.returncode)
+
verbose = False
# Only labels/tags matching this will be imported/exported
@@ -158,13 +173,17 @@ def system(cmd):
expand = isinstance(cmd,basestring)
if verbose:
sys.stderr.write(executing %s\n % str(cmd))
-subprocess.check_call(cmd, shell=expand)
+retcode = subprocess.call(cmd, shell=expand)
+if retcode:
+raise CalledProcessError(retcode, cmd)
def p4_system(cmd):
Specifically invoke p4 as the system command.
real_cmd = p4_build_cmd(cmd)
expand = isinstance(real_cmd, basestring)
-subprocess.check_call(real_cmd, shell=expand)
+retcode = subprocess.call(real_cmd, shell=expand)
+if retcode:
+raise CalledProcessError(retcode, real_cmd)
def p4_integrate(src, dest):
p4_system([integrate, -Dt, wildcard_encode(src),
wildcard_encode(dest)])
@@ -3174,7 +3193,9 @@ class P4Clone(P4Sync):
init_cmd = [ git, init ]
if self.cloneBare:
init_cmd.append(--bare)
-subprocess.check_call(init_cmd)
+retcode = subprocess.call(init_cmd)
+if retcode:
+raise CalledProcessError(retcode, init_cmd)
if not P4Sync.run(self, depotPaths):
return False
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 21924df..aae1a3f 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -105,12 +105,13 @@ build_gendouble() {
cat gendouble.py -\EOF
import sys
import struct
- import array
- s = array.array(c, '\0' * 26)
- struct.pack_into(L, s, 0, 0x00051607) # AppleDouble
- struct.pack_into(L, s, 4, 0x0002) # version 2
- s.tofile(sys.stdout)
+ s = struct.pack(LL18s,
+ 0x00051607, # AppleDouble
+ 0x0002, # version 2
+ # pad to 26 bytes
+ )
+ sys.stdout.write(s)
EOF
}
--
1.8.1.1.442.g413e803
---
This email message is for the sole use of the intended recipient(s) and may
contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact the
sender by
reply email and destroy all copies of the original message.
---
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] git-p4.py: support Python 2.5
From: Brandon Casey draf...@gmail.com Python 2.5 and older do not accept None as the first argument to translate() and complain with: TypeError: expected a character buffer object As suggested by Pete Wyckoff, let's just replace the call to translate() with a regex search which should be more clear and more portable. This allows git-p4 to be used with Python 2.5. Signed-off-by: Brandon Casey bca...@nvidia.com --- INSTALL | 2 +- git-p4.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/INSTALL b/INSTALL index 28f34bd..fc723b3 100644 --- a/INSTALL +++ b/INSTALL @@ -131,7 +131,7 @@ Issues of note: use English. Under autoconf the configure script will do this automatically if it can't find libintl on the system. - - Python version 2.6 or later is needed to use the git-p4 + - Python version 2.5 or later is needed to use the git-p4 interface to Perforce. - Some platform specific issues are dealt with Makefile rules, diff --git a/git-p4.py b/git-p4.py index 2da5649..de1a0b9 100755 --- a/git-p4.py +++ b/git-p4.py @@ -768,7 +768,8 @@ def wildcard_encode(path): return path def wildcard_present(path): -return path.translate(None, *#@%) != path +m = re.search([*#@%], path) +return m is not None class Command: def __init__(self): -- 1.8.1.1.442.g413e803 --- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. --- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
On Sat, Jan 26, 2013 at 4:12 AM, John Keeping j...@keeping.me.uk wrote:
On Fri, Jan 25, 2013 at 10:50:58PM -0800, David Aguilar wrote:
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 dav...@gmail.com
---
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.
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 aa38bd1..c866ed8 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,25 +46,14 @@ valid_tool () {
}
setup_tool () {
- case $1 in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool=$1
- ;;
- esac
- mergetools=$(git --exec-path)/mergetools
+ tool=$1
Unnecessary quoting.
- # Load the default definitions
- . $mergetools/defaults
- if ! test -f $mergetools/$tool
+ if ! test -f $MERGE_TOOLS_DIR/$tool
then
return 1
fi
-
- # Load the redefined functions
- . $mergetools/$tool
+ . $MERGE_TOOLS_DIR/include/defaults.sh
+ . $MERGE_TOOLS_DIR/$tool
if merge_mode ! can_merge
then
@@ -99,7 +90,7 @@ run_merge_tool () {
base_present=$2
status=0
- # Bring tool-specific functions into scope
+ # Bring tool specific functions into scope
This isn't related to this change (and I think tool-specific is more
correct anyway).
setup_tool $1
if merge_mode
@@ -177,18 +168,17 @@ 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
+ if test -d $i
then
continue
- elif merge_mode ! can_merge
+ fi
+
+ . $MERGE_TOOLS_DIR/include/defaults.sh
+ . $i
+
+ if merge_mode ! can_merge
then
continue
elif diff_mode ! can_diff
@@ -196,6 +186,7 @@ show_tool_help () {
continue
fi
I'd prefer to see my change to setup_tool done before this so that we
can just use:
setup_tool $tool 2/dev/null || continue
for the above block. I'll send a fixed version in a couple of minutes.
I'll reroll this patch with your notes on top of your new patch later today.
Thanks (and thanks for the Makefile hint.. I knew it was wrong! ;-).
+ tool=$(basename $i)
merge_tool_path=$(translate_merge_tool_path $tool)
if type $merge_tool_path /dev/null 21
then
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+. $MERGE_TOOLS_DIR/include/vim.sh
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 100644
index 000..f5890b1
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+. $MERGE_TOOLS_DIR/include/vim.sh
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/include/vim.sh
similarity index 100%
rename from mergetools/vim
rename to
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
On Sat, Jan 26, 2013 at 12:40:23PM -0800, David Aguilar wrote: On Sat, Jan 26, 2013 at 4:12 AM, John Keeping j...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] lazily load commit-buffer
Jeff King p...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tests: turn on test-lint-shell-syntax by default
Torsten Bögershausen tbo...@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. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
John Keeping j...@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 0x0200:
+# 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(
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] lazily load commit-buffer
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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] fetch: run gc --auto after fetching
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] fetch-pack: avoid repeatedly re-scanning pack directory
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-remote-testpy: fix patch hashing on Python 3
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 j...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mergetool--lib: don't call exit in setup_tool
From: John Keeping j...@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 j...@keeping.me.uk
Signed-off-by: David Aguilar dav...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fetch: run gc --auto after fetching
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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/21] git p4 test: use client_view in t9806
Yes, this really is four months later. Somehow I forgot all
about this series.
gits...@pobox.com wrote on Fri, 28 Sep 2012 12:11 -0700:
Pete Wyckoff p...@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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 00/21] git p4: work on cygwin
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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 01/21] git p4: temp branch name should use / even on windows
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.mcgi...@gmail.com Signed-off-by: Pete Wyckoff p...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 02/21] git p4: remove unused imports
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 p...@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 0x0204: # 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 03/21] git p4: generate better error message for bad depot path
Depot paths must start with //. Exit with a better explanation
when a bad depot path is supplied.
Signed-off-by: Pete Wyckoff p...@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 2errs
+ 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 04/21] git p4 test: use client_view to build the initial client
Simplify the code a bit by using an existing function.
Signed-off-by: Pete Wyckoff p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 05/21] git p4 test: avoid loop in client_view
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 gits...@pobox.com
Signed-off-by: Pete Wyckoff p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 06/21] git p4 test: use client_view in t9806
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 07/21] git p4 test: start p4d inside its db dir
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 08/21] git p4 test: translate windows paths for cygwin
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 sschube...@gmail.com
Thanks-to: Johannes Sixt j...@kdbg.org
fixup! git p4 test: translate windows paths for cygwin
Signed-off-by: Pete Wyckoff p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 09/21] git p4: remove unreachable windows \r\n conversion code
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 p...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 10/21] git p4: scrub crlf for utf16 files on windows
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 p...@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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 11/21] git p4 test: newline handling
P4 stores newlines in the depos as \n. By default, git does this too, both on unix and windows. Test to make sure that this stays true. Both git and p4 have mechanisms to use \r\n in the working directory. Exercise these. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9802-git-p4-filetype.sh | 117 + 1 file changed, 117 insertions(+) diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh index 21924df..c5ab626 100755 --- a/t/t9802-git-p4-filetype.sh +++ b/t/t9802-git-p4-filetype.sh @@ -8,6 +8,123 @@ test_expect_success 'start p4d' ' start_p4d ' +# +# This series of tests checks newline handling Both p4 and +# git store newlines as \n, and have options to choose how +# newlines appear in checked-out files. +# +test_expect_success 'p4 client newlines, unix' ' + ( + cd $cli + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + printf unix\ncrlf\n f-unix + printf unix\r\ncrlf\r\n f-unix-as-crlf + p4 add -t text f-unix + p4 submit -d f-unix + + # LineEnd: unix; should be no change after sync + cp f-unix f-unix-orig + p4 sync -f + test_cmp f-unix-orig f-unix + + # make sure stored in repo as unix newlines + # use sed to eat python-appened newline + p4 -G print //depot/f-unix | marshal_dump data 2 |\ + sed \$d f-unix-p4-print + test_cmp f-unix-orig f-unix-p4-print + + # switch to win, make sure lf - crlf + p4 client -o | sed /LineEnd/s/:.*/:win/ | p4 client -i + p4 sync -f + test_cmp f-unix-as-crlf f-unix + ) +' + +test_expect_success 'p4 client newlines, win' ' + ( + cd $cli + p4 client -o | sed /LineEnd/s/:.*/:win/ | p4 client -i + printf win\r\ncrlf\r\n f-win + printf win\ncrlf\n f-win-as-lf + p4 add -t text f-win + p4 submit -d f-win + + # LineEnd: win; should be no change after sync + cp f-win f-win-orig + p4 sync -f + test_cmp f-win-orig f-win + + # make sure stored in repo as unix newlines + # use sed to eat python-appened newline + p4 -G print //depot/f-win | marshal_dump data 2 |\ + sed \$d f-win-p4-print + test_cmp f-win-as-lf f-win-p4-print + + # switch to unix, make sure lf - crlf + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + p4 sync -f + test_cmp f-win-as-lf f-win + ) +' + +test_expect_success 'ensure blobs store only lf newlines' ' + test_when_finished cleanup_git + ( + cd $git + git init + git p4 sync //depot@all + + # verify the files in .git are stored only with newlines + o=$(git ls-tree p4/master -- f-unix | cut -f1 | cut -d\ -f3) + git cat-file blob $o f-unix-blob + test_cmp $cli/f-unix-orig f-unix-blob + + o=$(git ls-tree p4/master -- f-win | cut -f1 | cut -d\ -f3) + git cat-file blob $o f-win-blob + test_cmp $cli/f-win-as-lf f-win-blob + + rm f-unix-blob f-win-blob + ) +' + +test_expect_success 'gitattributes setting eol=lf produces lf newlines' ' + test_when_finished cleanup_git + ( + # checkout the files and make sure core.eol works as planned + cd $git + git init + echo * eol=lf .gitattributes + git p4 sync //depot@all + git checkout master + test_cmp $cli/f-unix-orig f-unix + test_cmp $cli/f-win-as-lf f-win + ) +' + +test_expect_success 'gitattributes setting eol=crlf produces crlf newlines' ' + test_when_finished cleanup_git + ( + # checkout the files and make sure core.eol works as planned + cd $git + git init + echo * eol=crlf .gitattributes + git p4 sync //depot@all + git checkout master + test_cmp $cli/f-unix-as-crlf f-unix + test_cmp $cli/f-win-orig f-win + ) +' + +test_expect_success 'crlf cleanup' ' + ( + cd $cli + rm f-unix-orig f-unix-as-crlf + rm f-win-orig f-win-as-lf + p4 client -o | sed /LineEnd/s/:.*/:unix/ | p4 client -i + p4 sync -f + ) +' + test_expect_success 'utf-16 file create' ' ( cd $cli -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org
Re: [PATCH v3 2/2] mergetools: Simplify how we handle vim and defaults
David Aguilar dav...@gmail.com writes:
@@ -44,19 +46,9 @@ valid_tool () {
}
setup_tool () {
- case $1 in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool=$1
- ;;
- esac
This part was an eyesore every time I looked at mergetools scripts.
Good riddance.
Is there still other special case like this, or was this the last
one?
Thanks, both of you, for working on this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 12/21] git p4 test: use LineEnd unix in windows tests too
In all clients, even those created on windows, use unix line
endings. This makes it possible to verify file contents without
doing OS-specific comparisons in all the tests.
Tests in t9802-git-p4-filetype.sh are used to make sure that
the other LineEnd options continue to work.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index d5596de..67101b1 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -142,6 +142,7 @@ client_view() {
Description: $P4CLIENT
Root: $cli
AltRoots: $(native_path $cli)
+ LineEnd: unix
View:
EOF
printf \t%s\n $@
--
1.8.1.1.460.g6fa8886
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 13/21] git p4 test: avoid wildcard * in windows
This character is not valid in windows filenames, even though it can appear in p4 depot paths. Avoid using it in tests on windows, both mingw and cygwin. Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9809-git-p4-client-view.sh | 10 -- t/t9812-git-p4-wildcards.sh | 37 + 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh index 0b58fb9..a911988 100755 --- a/t/t9809-git-p4-client-view.sh +++ b/t/t9809-git-p4-client-view.sh @@ -365,7 +365,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' ' ( cd $git echo git-wild-hash dir1/git-wild#hash - echo git-wild-star dir1/git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo git-wild-star dir1/git-wild\*star + fi echo git-wild-at dir1/git-wild@at echo git-wild-percent dir1/git-wild%percent git add dir1/git-wild* @@ -376,7 +379,10 @@ test_expect_success 'wildcard files submit back to p4, client-spec case' ' ( cd $cli test_path_is_file dir1/git-wild#hash - test_path_is_file dir1/git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_path_is_file dir1/git-wild\*star + fi test_path_is_file dir1/git-wild@at test_path_is_file dir1/git-wild%percent ) diff --git a/t/t9812-git-p4-wildcards.sh b/t/t9812-git-p4-wildcards.sh index 143d413..6763325 100755 --- a/t/t9812-git-p4-wildcards.sh +++ b/t/t9812-git-p4-wildcards.sh @@ -14,7 +14,10 @@ test_expect_success 'add p4 files with wildcards in the names' ' printf file2\nhas\nsome\nrandom\ntext\n file2 p4 add file2 echo file-wild-hash file-wild#hash - echo file-wild-star file-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo file-wild-star file-wild\*star + fi echo file-wild-at file-wild@at echo file-wild-percent file-wild%percent p4 add -f file-wild* @@ -28,7 +31,10 @@ test_expect_success 'wildcard files git p4 clone' ' ( cd $git test -f file-wild#hash - test -f file-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test -f file-wild\*star + fi test -f file-wild@at test -f file-wild%percent ) @@ -40,7 +46,10 @@ test_expect_success 'wildcard files submit back to p4, add' ' ( cd $git echo git-wild-hash git-wild#hash - echo git-wild-star git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo git-wild-star git-wild\*star + fi echo git-wild-at git-wild@at echo git-wild-percent git-wild%percent git add git-wild* @@ -51,7 +60,10 @@ test_expect_success 'wildcard files submit back to p4, add' ' ( cd $cli test_path_is_file git-wild#hash - test_path_is_file git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_path_is_file git-wild\*star + fi test_path_is_file git-wild@at test_path_is_file git-wild%percent ) @@ -63,7 +75,10 @@ test_expect_success 'wildcard files submit back to p4, modify' ' ( cd $git echo new-line git-wild#hash - echo new-line git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + echo new-line git-wild\*star + fi echo new-line git-wild@at echo new-line git-wild%percent git add git-wild* @@ -74,7 +89,10 @@ test_expect_success 'wildcard files submit back to p4, modify' ' ( cd $cli test_line_count = 2 git-wild#hash - test_line_count = 2 git-wild\*star + if test_have_prereq NOT_MINGW NOT_CYGWIN + then + test_line_count = 2 git-wild\*star + fi test_line_count = 2 git-wild@at test_line_count = 2 git-wild%percent ) @@ -87,7 +105,7 @@ test_expect_success 'wildcard files submit back to p4, copy' ' cd $git cp file2 git-wild-cp#hash git add git-wild-cp#hash - cp
[PATCHv2 14/21] git p4: cygwin p4 client does not mark read-only
There are some old versions of p4, compiled for cygwin, that
treat read-only files differently.
Normally, a file that is not open is read-only, meaning that
test -w on the file is false. This works on unix, and it works
on windows using the NT version of p4. The cygwin version
of p4, though, changes the permissions, but does not set the
windows read-only attribute, so test -w returns false.
Notice this oddity and make the tests work, even on cygiwn.
Signed-off-by: Pete Wyckoff p...@padd.com
---
t/lib-git-p4.sh | 13 +
t/t9807-git-p4-submit.sh | 14 --
t/t9809-git-p4-client-view.sh | 4 ++--
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
index 67101b1..2098b9b 100644
--- a/t/lib-git-p4.sh
+++ b/t/lib-git-p4.sh
@@ -148,3 +148,16 @@ client_view() {
printf \t%s\n $@
) | p4 client -i
}
+
+is_cli_file_writeable() {
+ # cygwin version of p4 does not set read-only attr,
+ # will be marked 444 but -w is true
+ file=$1
+ if test_have_prereq CYGWIN p4 -V | grep -q CYGWIN
+ then
+ stat=$(stat --format=%a $file)
+ test $stat = 644
+ else
+ test -w $file
+ fi
+}
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 0ae048f..1fb7bc7 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -17,6 +17,16 @@ test_expect_success 'init depot' '
)
'
+test_expect_failure 'is_cli_file_writeable function' '
+ (
+ cd $cli
+ echo a a
+ is_cli_file_writeable a
+ ! is_cli_file_writeable file1
+ rm a
+ )
+'
+
test_expect_success 'submit with no client dir' '
test_when_finished cleanup_git
git p4 clone --dest=$git //depot
@@ -200,7 +210,7 @@ test_expect_success 'submit copy' '
(
cd $cli
test_path_is_file file5.ta
- test ! -w file5.ta
+ ! is_cli_file_writeable file5.ta
)
'
@@ -219,7 +229,7 @@ test_expect_success 'submit rename' '
cd $cli
test_path_is_missing file6.t
test_path_is_file file6.ta
- test ! -w file6.ta
+ ! is_cli_file_writeable file6.ta
)
'
diff --git a/t/t9809-git-p4-client-view.sh b/t/t9809-git-p4-client-view.sh
index a911988..77f6349 100755
--- a/t/t9809-git-p4-client-view.sh
+++ b/t/t9809-git-p4-client-view.sh
@@ -333,7 +333,7 @@ test_expect_success 'subdir clone, submit copy' '
(
cd $cli
test_path_is_file dir1/file11a
- test ! -w dir1/file11a
+ ! is_cli_file_writeable dir1/file11a
)
'
@@ -353,7 +353,7 @@ test_expect_success 'subdir clone, submit rename' '
cd $cli
test_path_is_missing dir1/file13
test_path_is_file dir1/file13a
- test ! -w dir1/file13a
+ ! is_cli_file_writeable dir1/file13a
)
'
--
1.8.1.1.460.g6fa8886
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 15/21] git p4 test: use test_chmod for cygwin
This test does a commit that is a pure mode change, submits it to p4 but causes the submit to fail. It verifies that the state in p4 as well as the client directory are both unmodified after the failed submit. On cygwin, chmod +x does nothing, so use the test_chmod function to modify the index directly too. Also on cygwin, the executable bit cannot be seen in the filesystem, so avoid that part of the test. The checks of p4 state are still valid, though. Thanks-to: Johannes Sixt j...@kdbg.org Signed-off-by: Pete Wyckoff p...@padd.com --- t/t9815-git-p4-submit-fail.sh | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh index d2b7b3d..1243d96 100755 --- a/t/t9815-git-p4-submit-fail.sh +++ b/t/t9815-git-p4-submit-fail.sh @@ -405,8 +405,8 @@ test_expect_success 'cleanup chmod after submit cancel' ' git p4 clone --dest=$git //depot ( cd $git - chmod u+x text - chmod u-x text+x + test_chmod +x text + test_chmod -x text+x git add text text+x git commit -m chmod texts echo n | test_expect_code 1 git p4 submit @@ -415,10 +415,13 @@ test_expect_success 'cleanup chmod after submit cancel' ' cd $cli test_path_is_file text ! p4 fstat -T action text - stat --format=%A text | egrep ^-r-- test_path_is_file text+x ! p4 fstat -T action text+x - stat --format=%A text+x | egrep ^-r-x + if test_have_prereq NOT_CYGWIN + then + stat --format=%A text | egrep ^-r-- + stat --format=%A text+x | egrep ^-r-x + fi ) ' -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 16/21] git p4: disable read-only attribute before deleting
On windows, p4 marks un-edited files as read-only. Not only are they read-only, but also they cannot be deleted. Remove the read-only attribute before deleting in both the copy and rename cases. This also happens in the RCS cleanup code, where a file is marked to be deleted, but must first be edited to remove adjust the keyword lines. Make sure it is editable before patching. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 10 ++ 1 file changed, 10 insertions(+) diff --git a/git-p4.py b/git-p4.py index c62b2ca..a989704 100755 --- a/git-p4.py +++ b/git-p4.py @@ -21,6 +21,7 @@ import time import platform import re import shutil +import stat verbose = False @@ -1231,6 +1232,9 @@ class P4Submit(Command, P4UserMap): p4_edit(dest) pureRenameCopy.discard(dest) filesToChangeExecBit[dest] = diff['dst_mode'] +if self.isWindows: +# turn off read-only attribute +os.chmod(dest, stat.S_IWRITE) os.unlink(dest) editedFiles.add(dest) elif modifier == R: @@ -1249,6 +1253,8 @@ class P4Submit(Command, P4UserMap): p4_edit(dest) # with move: already open, writable filesToChangeExecBit[dest] = diff['dst_mode'] if not self.p4HasMoveCommand: +if self.isWindows: +os.chmod(dest, stat.S_IWRITE) os.unlink(dest) filesToDelete.add(src) editedFiles.add(dest) @@ -1289,6 +1295,10 @@ class P4Submit(Command, P4UserMap): for file in kwfiles: if verbose: print zapping %s with %s % (line,pattern) +# File is being deleted, so not open in p4. Must +# disable the read-only bit on windows. +if self.isWindows and file not in editedFiles: +os.chmod(file, stat.S_IWRITE) self.patchRCSKeywords(file, kwfiles[file]) fixed_rcs_keywords = True -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 17/21] git p4: avoid shell when mapping users
The extra quoting and double-% are unneeded, just to work around the shell. Instead, avoid the shell indirection. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index a989704..c43d044 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1050,7 +1050,8 @@ class P4Submit(Command, P4UserMap): def p4UserForCommit(self,id): # Return the tuple (perforce user,git email) for a given git commit id self.getUserMapFromPerforceServer() -gitEmail = read_pipe(git log --max-count=1 --format='%%ae' %s % id) +gitEmail = read_pipe([git, log, --max-count=1, + --format=%ae, id]) gitEmail = gitEmail.strip() if not self.emails.has_key(gitEmail): return (None,gitEmail) -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 18/21] git p4: avoid shell when invoking git rev-list
Invoke git rev-list directly, avoiding the shell, in P4Submit and P4Sync. The overhead of starting extra processes is significant in cygwin; this speeds things up on that platform. Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/git-p4.py b/git-p4.py index c43d044..c8ae83d 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1606,7 +1606,7 @@ class P4Submit(Command, P4UserMap): self.check() commits = [] -for line in read_pipe_lines(git rev-list --no-merges %s..%s % (self.origin, self.master)): +for line in read_pipe_lines([git, rev-list, --no-merges, %s..%s % (self.origin, self.master)]): commits.append(line.strip()) commits.reverse() @@ -2644,7 +2644,8 @@ class P4Sync(Command, P4UserMap): def searchParent(self, parent, branch, target): parentFound = False -for blob in read_pipe_lines([git, rev-list, --reverse, --no-merges, parent]): +for blob in read_pipe_lines([git, rev-list, --reverse, + --no-merges, parent]): blob = blob.strip() if len(read_pipe([git, diff-tree, blob, target])) == 0: parentFound = True -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 19/21] git p4: avoid shell when invoking git config --get-all
Signed-off-by: Pete Wyckoff p...@padd.com --- git-p4.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index c8ae83d..7efa9a8 100755 --- a/git-p4.py +++ b/git-p4.py @@ -571,7 +571,8 @@ def gitConfig(key, args = None): # set args to --bool, for instance def gitConfigList(key): if not _gitConfig.has_key(key): -_gitConfig[key] = read_pipe(git config --get-all %s % key, ignore_error=True).strip().split(os.linesep) +s = read_pipe([git, config, --get-all, key], ignore_error=True) +_gitConfig[key] = s.strip().split(os.linesep) return _gitConfig[key] def p4BranchesInGit(branchesAreInRemotes=True): -- 1.8.1.1.460.g6fa8886 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 20/21] git p4: avoid shell when calling git config
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 7efa9a8..ff3e8c9 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -560,13 +560,16 @@ def gitBranchExists(branch):
return proc.wait() == 0;
_gitConfig = {}
-def gitConfig(key, args = None): # set args to --bool, for instance
+
+def gitConfig(key, args=None): # set args to --bool, for instance
if not _gitConfig.has_key(key):
-argsFilter =
-if args != None:
-argsFilter = %s % args
-cmd = git config %s%s % (argsFilter, key)
-_gitConfig[key] = read_pipe(cmd, ignore_error=True).strip()
+cmd = [ git, config ]
+if args:
+assert(args == --bool)
+cmd.append(args)
+cmd.append(key)
+s = read_pipe(cmd, ignore_error=True)
+_gitConfig[key] = s.strip()
return _gitConfig[key]
def gitConfigList(key):
--
1.8.1.1.460.g6fa8886
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 21/21] git p4: introduce gitConfigBool
Make the intent of --bool more obvious by returning a direct True
or False value. Convert a couple non-bool users with obvious bool
intent.
Signed-off-by: Pete Wyckoff p...@padd.com
---
git-p4.py | 45 ++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index ff3e8c9..955a5dd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -561,17 +561,25 @@ def gitBranchExists(branch):
_gitConfig = {}
-def gitConfig(key, args=None): # set args to --bool, for instance
+def gitConfig(key):
if not _gitConfig.has_key(key):
-cmd = [ git, config ]
-if args:
-assert(args == --bool)
-cmd.append(args)
-cmd.append(key)
+cmd = [ git, config, key ]
s = read_pipe(cmd, ignore_error=True)
_gitConfig[key] = s.strip()
return _gitConfig[key]
+def gitConfigBool(key):
+Return a bool, using git config --bool. It is True only if the
+ variable is set to true, and False if set to false or not present
+ in the config.
+
+if not _gitConfig.has_key(key):
+cmd = [ git, config, --bool, key ]
+s = read_pipe(cmd, ignore_error=True)
+v = s.strip()
+_gitConfig[key] = v == true
+return _gitConfig[key]
+
def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe([git, config, --get-all, key], ignore_error=True)
@@ -722,8 +730,7 @@ def p4PathStartsWith(path, prefix):
#
# we may or may not have a problem. If you have core.ignorecase=true,
# we treat DirA and dira as the same directory
-ignorecase = gitConfig(core.ignorecase, --bool) == true
-if ignorecase:
+if gitConfigBool(core.ignorecase):
return path.lower().startswith(prefix.lower())
return path.startswith(prefix)
@@ -959,7 +966,7 @@ class P4Submit(Command, P4UserMap):
self.usage += [name of git branch to submit into perforce depot]
self.origin =
self.detectRenames = False
-self.preserveUser = gitConfig(git-p4.preserveUser).lower() == true
+self.preserveUser = gitConfigBool(git-p4.preserveUser)
self.dry_run = False
self.prepare_p4_only = False
self.conflict_behavior = None
@@ -1068,7 +1075,7 @@ class P4Submit(Command, P4UserMap):
(user,email) = self.p4UserForCommit(id)
if not user:
msg = Cannot find p4 user for email %s in commit %s. %
(email, id)
-if gitConfig('git-p4.allowMissingP4Users').lower() == true:
+if gitConfigBool(git-p4.allowMissingP4Users):
print %s % msg
else:
die(Error: %s\nSet git-p4.allowMissingP4Users to true to
allow this. % msg)
@@ -1163,7 +1170,7 @@ class P4Submit(Command, P4UserMap):
message. Return true if okay to continue with the submit.
# if configured to skip the editing part, just submit
-if gitConfig(git-p4.skipSubmitEdit) == true:
+if gitConfigBool(git-p4.skipSubmitEdit):
return True
# look at the modification time, to check later if the user saved
@@ -1179,7 +1186,7 @@ class P4Submit(Command, P4UserMap):
# If the file was not saved, prompt to see if this patch should
# be skipped. But skip this verification step if configured so.
-if gitConfig(git-p4.skipSubmitEditCheck) == true:
+if gitConfigBool(git-p4.skipSubmitEditCheck):
return True
# modification time updated means user saved the file
@@ -1279,7 +1286,7 @@ class P4Submit(Command, P4UserMap):
# Patch failed, maybe it's just RCS keyword woes. Look through
# the patch to see if that's possible.
-if gitConfig(git-p4.attemptRCSCleanup,--bool) == true:
+if gitConfigBool(git-p4.attemptRCSCleanup):
file = None
pattern = None
kwfiles = {}
@@ -1574,7 +1581,7 @@ class P4Submit(Command, P4UserMap):
sys.exit(128)
self.useClientSpec = False
-if gitConfig(git-p4.useclientspec, --bool) == true:
+if gitConfigBool(git-p4.useclientspec):
self.useClientSpec = True
if self.useClientSpec:
self.clientSpecDirs = getClientSpec()
@@ -1614,7 +1621,7 @@ class P4Submit(Command, P4UserMap):
commits.append(line.strip())
commits.reverse()
-if self.preserveUser or (gitConfig(git-p4.skipUserNameCheck) ==
true):
+if self.preserveUser or gitConfigBool(git-p4.skipUserNameCheck):
self.checkAuthorship = False
else:
self.checkAuthorship = True
@@ -1650,7 +1657,7 @@ class P4Submit(Command, P4UserMap):
else:
self.diffOpts += -C%s % detectCopies
-if gitConfig(git-p4.detectCopiesHarder, --bool) == true:
+if gitConfigBool(git-p4.detectCopiesHarder):
Re: [PATCH v3 2/2] mergetools: Simplify how we handle vim and defaults
On Sat, Jan 26, 2013 at 7:15 PM, Junio C Hamano gits...@pobox.com wrote:
David Aguilar dav...@gmail.com writes:
@@ -44,19 +46,9 @@ valid_tool () {
}
setup_tool () {
- case $1 in
- vim*|gvim*)
- tool=vim
- ;;
- *)
- tool=$1
- ;;
- esac
This part was an eyesore every time I looked at mergetools scripts.
Good riddance.
Is there still other special case like this, or was this the last
one?
Thanks, both of you, for working on this.
I believe that was the last special case. :-) It should be easier
to auto-generate a list of tools for use in the documentation now.
That'll be the the next topic I look into.
I have a question. John mentioned that we can replace the use of
$(..) with $(..) in shell.
I have a trivial style patches to replace $(..) with $(..)
sitting uncommitted in my tree for mergetool--lib.
Grepping the rest of the tree shows that there are many
occurrences of the $(..) idiom in the shell code.
Is this a general rule that should be in CodingStyle,
or is it better left as-is? Are there cases where DQ should
be used around $(..)? My understanding is no, but I don't
know whether that is correct.
Doing the documentation stuff is more important IMO so I probably
shouldn't let myself get too distracted by it, though I am curious.
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
On 01/26/2013 10:44 PM, Junio C Hamano wrote:
John Keeping j...@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 0x0200:
+# 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(
This will still fail under Python 2.x if repo.path is a byte string that
contains non-ASCII characters. And it will fail under Python 3.1 and
later if repo.path contains characters using the surrogateescape
encoding option [1], as it will if the original command-line argument
contained bytes that cannot be decoded into Unicode using the user's
default encoding:
$ python3 --version
Python 3.2.3
$ python3 -c
import sys
print(repr(sys.argv[1]))
print(repr(sys.argv[1].encode('utf-8')))
$(echo français|iconv -t latin1)
'fran\udce7ais'
Traceback (most recent call last):
File string, line 4, in module
UnicodeEncodeError: 'utf-8' codec can't encode character '\udce7' in
position 4: surrogates not allowed
I'm not sure what happens in Python 3.0.
I think the modern way to handle this situation in Python 3.1+ is via
PEP 383's surrogateescape encoding option [1]:
repo.path.encode('utf-8', 'surrogateescape')
Basically, byte strings that come from the OS are automatically decoded
into Unicode strings using
s = b.decode(sys.getfilesystemencoding(), 'surrogateescape')
If the string needs to be passed back to the filesystem as a byte string
it is via
b = s.encode(sys.getfilesystemencoding(), 'surrogateescape')
My understanding is that the surrogateescape mechanism guarantees that
the round-trip bytestring - string - bytestring gives back the
original byte string, which is what you want for things like filenames.
But a Unicode string that contains surrogate escape characters *cannot*
be encoded without the 'surrogateescape' option.
'surrogateescape' is not supported in Python 3.0, but I think it would
be quite acceptable only to support Python 3.x for x = 1.
But 'surrogateescape' doesn't seem to be supported at all in Python 2.x
(I tested 2.7.3 and it's not there).
Here you don't really need byte-for-byte correctness; it would be enough
to get *some* byte string that is unique for a given input (ideally,
consistent with ASCII or UTF-8 for backwards compatibility). So you
could use
b = s.encode('utf-8', 'backslashreplace')
Unfortunately, this doesn't work under Python 2.x:
$ python2 -c
import sys
print(repr(sys.argv[1]))
print(repr(sys.argv[1].encode('utf-8', 'backslashreplace')))
$(echo français|iconv -t latin1)
'fran\xe7ais'
Traceback (most recent call last):
File string, line 4, in module
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe7 in position
4: ordinal not in range(128)
Apparently when you call bytestring.encode(), Python first tries to
decode the string to Unicode using the 'ascii' encoding.
So to handle all of the cases across Python versions as closely as
possible to the old 2.x code, it might be necessary to make the code
explicitly depend on the Python version number, like:
hasher = _digest()
if sys.hexversion 0x0300:
pathbytes = repo.path
elif sys.hexversion 0x0301:
# If support for Python 3.0.x is desired (note: result can
# be different in this case than under 2.x or 3.1+):
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
'backslashreplace')
else
pathbytes = repo.path.encode(sys.getfilesystemencoding(),
'surrogateescape')
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
John Keeping j...@keeping.me.uk writes:
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.
Is 'include' really used for such a purpose? It only houses defaults.sh
as far as I can see.
As that defaults.sh file is used only to define trivially empty
shell functions, I wonder if it is better to get rid of it, and
define these functions in line in git-mergetool--lib.sh. Such a
change would like the attached on top of the entire series.
Makefile | 6 +-
git-mergetool--lib.sh | 24 ++--
mergetools/include/defaults.sh | 22 --
3 files changed, 23 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index 26f217f..f69979e 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,11 +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 $(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'
+ $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
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 7ea7510..1d0fb12 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -57,8 +57,28 @@ setup_tool () {
return 2
fi
- # Load the default functions
- . $MERGE_TOOLS_DIR/include/defaults.sh
+ # Fallback definitions, to be overriden by tools.
+ can_merge () {
+ return 0
+ }
+
+ can_diff () {
+ return 0
+ }
+
+ diff_cmd () {
+ status=1
+ return $status
+ }
+
+ merge_cmd () {
+ status=1
+ return $status
+ }
+
+ translate_merge_tool_path () {
+ echo $1
+ }
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
diff --git a/mergetools/include/defaults.sh b/mergetools/include/defaults.sh
deleted file mode 100644
index 21e63ec..000
--- a/mergetools/include/defaults.sh
+++ /dev/null
@@ -1,22 +0,0 @@
-# Redefined by builtin tools
-can_merge () {
- return 0
-}
-
-can_diff () {
- return 0
-}
-
-diff_cmd () {
- status=1
- return $status
-}
-
-merge_cmd () {
- status=1
- return $status
-}
-
-translate_merge_tool_path () {
- echo $1
-}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
On Sat, Jan 26, 2013 at 8:57 PM, Junio C Hamano gits...@pobox.com wrote:
John Keeping j...@keeping.me.uk writes:
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.
Is 'include' really used for such a purpose? It only houses defaults.sh
as far as I can see.
As that defaults.sh file is used only to define trivially empty
shell functions, I wonder if it is better to get rid of it, and
define these functions in line in git-mergetool--lib.sh. Such a
change would like the attached on top of the entire series.
I think that's much better.
Makefile | 6 +-
git-mergetool--lib.sh | 24 ++--
mergetools/include/defaults.sh | 22 --
3 files changed, 23 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index 26f217f..f69979e 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,11 +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 $(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'
+ $(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
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 7ea7510..1d0fb12 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -57,8 +57,28 @@ setup_tool () {
return 2
fi
- # Load the default functions
- . $MERGE_TOOLS_DIR/include/defaults.sh
+ # Fallback definitions, to be overriden by tools.
+ can_merge () {
+ return 0
+ }
+
+ can_diff () {
+ return 0
+ }
+
+ diff_cmd () {
+ status=1
+ return $status
+ }
+
+ merge_cmd () {
+ status=1
+ return $status
+ }
+
+ translate_merge_tool_path () {
+ echo $1
+ }
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
diff --git a/mergetools/include/defaults.sh b/mergetools/include/defaults.sh
deleted file mode 100644
index 21e63ec..000
--- a/mergetools/include/defaults.sh
+++ /dev/null
@@ -1,22 +0,0 @@
-# Redefined by builtin tools
-can_merge () {
- return 0
-}
-
-can_diff () {
- return 0
-}
-
-diff_cmd () {
- status=1
- return $status
-}
-
-merge_cmd () {
- status=1
- return $status
-}
-
-translate_merge_tool_path () {
- echo $1
-}
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
Michael Haggerty mhag...@alum.mit.edu writes: This will still fail under Python 2.x if repo.path is a byte string that contains non-ASCII characters. And it will fail under Python 3.1 and later if repo.path contains characters using the surrogateescape encoding option [1],... Here you don't really need byte-for-byte correctness; it would be enough to get *some* byte string that is unique for a given input ... Yeek. As we do not care about the actual value at all, how about doing something like this instead? git-remote-testgit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-remote-testgit.py b/git-remote-testgit.py index 5f3ebd2..705750d 100644 --- a/git-remote-testgit.py +++ b/git-remote-testgit.py @@ -40,7 +40,7 @@ def get_repo(alias, url): repo.get_head() hasher = _digest() -hasher.update(repo.path) +hasher.update(..join([str(ord(c)) for c in repo.path])) repo.hash = hasher.hexdigest() repo.get_base_path = lambda base: os.path.join( -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/8] git-remote-testpy: hash bytes explicitly
On Sat, Jan 26, 2013 at 8:44 PM, Michael Haggerty mhag...@alum.mit.edu wrote: So to handle all of the cases across Python versions as closely as possible to the old 2.x code, it might be necessary to make the code explicitly depend on the Python version number, like: Does this all go away if we restrict ourselves to python 2.6 and just use the b prefix? -- Cheers, Sverre Rabbelier -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] lazily load commit-buffer
Jeff King p...@peff.net writes: 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. ... I'd be in favor of dropping the lines just to decrease complexity of the code. I think we are in agreement, then. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
On Sat, Jan 26, 2013 at 9:07 PM, David Aguilar dav...@gmail.com wrote:
On Sat, Jan 26, 2013 at 8:57 PM, Junio C Hamano gits...@pobox.com wrote:
John Keeping j...@keeping.me.uk writes:
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.
Is 'include' really used for such a purpose? It only houses defaults.sh
as far as I can see.
As that defaults.sh file is used only to define trivially empty
shell functions, I wonder if it is better to get rid of it, and
define these functions in line in git-mergetool--lib.sh. Such a
change would like the attached on top of the entire series.
I think that's much better.
Would you like me to put this together into a proper patch?
You can also squash it in (along with a removal of the
last line of the commit message) if you prefer.
Makefile | 6 +-
git-mergetool--lib.sh | 24 ++--
mergetools/include/defaults.sh | 22 --
3 files changed, 23 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index 26f217f..f69979e 100644
--- a/Makefile
+++ b/Makefile
@@ -2724,11 +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 $(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'
+ $(INSTALL) -m 644 mergetools/*
'$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
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 7ea7510..1d0fb12 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -57,8 +57,28 @@ setup_tool () {
return 2
fi
- # Load the default functions
- . $MERGE_TOOLS_DIR/include/defaults.sh
+ # Fallback definitions, to be overriden by tools.
+ can_merge () {
+ return 0
+ }
+
+ can_diff () {
+ return 0
+ }
+
+ diff_cmd () {
+ status=1
+ return $status
+ }
+
+ merge_cmd () {
+ status=1
+ return $status
+ }
+
+ translate_merge_tool_path () {
+ echo $1
+ }
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
diff --git a/mergetools/include/defaults.sh b/mergetools/include/defaults.sh
deleted file mode 100644
index 21e63ec..000
--- a/mergetools/include/defaults.sh
+++ /dev/null
@@ -1,22 +0,0 @@
-# Redefined by builtin tools
-can_merge () {
- return 0
-}
-
-can_diff () {
- return 0
-}
-
-diff_cmd () {
- status=1
- return $status
-}
-
-merge_cmd () {
- status=1
- return $status
-}
-
-translate_merge_tool_path () {
- echo $1
-}
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mergetools: Simplify how we handle vim and defaults
David Aguilar dav...@gmail.com writes: I think that's much better. Would you like me to put this together into a proper patch? You can also squash it in (along with a removal of the last line of the commit message) if you prefer. I was lazy and didn't want to think about rewriting your log message, so I just queued it with this log message on top. mergetools: remove mergetools/include/defaults.sh This and its containing directory are used only to define trivial fallback definition of five shell functions in a single script. Defining them in-line at the location that wants to have them makes the result much easier to read. Signed-off-by: Junio C Hamano gits...@pobox.com But as you said, removing the last line of your log message and squashing the change into it would be more preferrable. Let me do that later. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] optimizing pack access on read only fetch repos
Jeff King p...@peff.net writes: 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 Both makes sense to me. I also wonder if we would be helped by another repack mode that coalesces small packs into a single one with minimum overhead, and run that often from gc --auto, so that we do not end up having to have 50 packfiles. When we have 2 or more small and young packs, we could: - iterate over idx files for these packs to enumerate the objects to be packed, replacing read_object_list_from_stdin() step; - always choose to copy the data we have in these existing packs, instead of doing a full prepare_pack(); and - use the order the objects appear in the original packs, bypassing compute_write_order(). The procedure cannot be a straight byte-for-byte copy, because some objects may appear in multiple packs, and extra copies of the same object have to be excised from the result. OFS_DELTA offsets need to be adjusted for objects that appear later in the output and for objects that were deltified against such an object that recorded its base with OFS_DELTA format. But other than such OFS_DELTA adjustments, it feels that such an only coalesce multiple packs into one mode should be fairly quick. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Reduce false positive in check-non-portable-shell.pl
check-non-portable-shell.pl is using simple regular expressions to
find illegal shell syntax.
Improve the expressions and reduce the chance for false positves:
sed -i must be followed by 1..n whitespace and 1 non whitespace
declare must be followed by 1..n whitespace and 1 non whitespace
echo -n must be followed by 1..n whitespace and 1 non whitespace
which: catch lines like if which foo
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
t/check-non-portable-shell.pl | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 8b5a71d..d9ddcdf 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl
@@ -16,10 +16,10 @@ sub err {
while () {
chomp;
- /^\s*sed\s+-i/ and err 'sed -i is not portable';
- /^\s*echo\s+-n/ and err 'echo -n is not portable (please use printf)';
- /^\s*declare\s+/ and err 'arrays/declare not portable';
- /^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
+ /^\s*sed\s+-i\s+\S/ and err 'sed -i is not portable';
+ /^\s*echo\s+-n\s+\S/ and err 'echo -n is not portable (please use
printf)';
+ /^\s*declare\s+\S/ and err 'arrays/declare not portable';
+ /^\s*if\s+which\s+\S/ and err 'which is not portable (please use type)';
/test\s+[^=]*==/ and err 'test a == b is not portable (please use =)';
# this resets our $. for each file
close ARGV if eof;
--
1.8.1.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
