* Re: [PATCH] contrib: added git-diffall
From: Stefano Lattarini @ 2012-02-22 10:05 UTC (permalink / raw)
To: Tim Henigan; +Cc: git, gitster
In-Reply-To: <1329785969-828-1-git-send-email-tim.henigan@gmail.com>
On 02/21/2012 01:59 AM, Tim Henigan wrote:
> test -z $(which mktemp 2>/dev/null)
>
This is wrong: if mktemp is not avilable, the expression above will
become, after command substitution and word splitting have taken pace,
equivalent to:
test -z
which, per POSIX, must return 0 (and does with at least bash 4.1.5 and
dash 0.5.5.1). You should just use this instead:
which mktemp 2>/dev/null
OK, technically you could also fix your idiom above a little and use:
test -z "$(which mktemp 2>/dev/null)"
but seems like a useless use of indirections to me.
And all of this is naturally render moot by Junio's advice of not using
which(1) in the first place ;-)
Regards,
Stefano
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Jeff King @ 2012-02-22 10:13 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <1327079011-24788-1-git-send-email-spearce@spearce.org>
On Fri, Jan 20, 2012 at 09:03:31AM -0800, Shawn O. Pearce wrote:
> diff --git a/remote-curl.c b/remote-curl.c
> index 48c20b8..25c1af7 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -822,12 +822,13 @@ static void parse_push(struct strbuf *buf)
> break;
> } while (1);
>
> - if (push(nr_spec, specs))
> - exit(128); /* error already reported */
> -
> + ret = push(nr_spec, specs);
> printf("\n");
> fflush(stdout);
>
> + if (ret)
> + exit(128); /* error already reported */
> +
This hunk is causing intermittent failures of t5541 for me, especially
when the system is under heavy load (e.g., make -j32 test). Before your
patch, this is what happened:
1. remote-curl relays the status lines from send-pack, then sees that
send-pack reported error, and it exits
2. push reads the status lines, looking for a blank line to terminate
them. It sees EOF instead of the blank line and exits(128) itself.
After your patch, this happens:
1. remote-curl relays the status lines, alway appends the blank line
terminator, and then exits
2. push reads the status lines, including the blank line terminator,
and reports them to the user.
3. push then disconnects the remote-curl helper by writing a blank
line to it (to signal end-of-input), followed by finish_command().
The latter propagates the error code from the exit in step 1, and
we use that to signal failure from "git push".
There's a race condition now in step 3. The push process may write to
the pipe going to remote-curl after it has exited, causing it to receive
SIGPIPE and die. We can block SIGPIPE, but that's not sufficient; we'll
still notice that our write() returns EPIPE and die.
Obviously we can't not print the post-push "\n" in remote-curl, for the
reasons you outlined in the commit message of this patch. We also can't
not exit from remote-curl on error. Even though in the test in t5541 we
have signaled error via the ref statuses, we might have received an
error that does not come through a ref status (e.g., if we couldn't run
send-pack at all).
We can't not write the "\n" to signal end-of-input to remote-curl,
because we don't actually know yet that there's an error (we find out
when we wait() on the process). Barring any asynchronous SIGCHLD
handling, of course, but I don't think we want to get into that.
So it's kind of a bug in the remote helper protocol. The helpers can
signal failure only by dying, but we can find out about that failure
only after disconnecting, which involves writing to them. It would be
much more sane if the helpers returned an overall text status from each
command (e.g., printed "error push failed" instead of dying).
But that would involve changing the protocol, of course. I think our
best option is to work around it by considering the final blank line we
send before disconnect as "best effort". That is, it is a courtesy to
the remote helper to tell it we are hanging up cleanly, and if it does
not arrive, then we can ignore the problem and proceed with closing the
pipe. I.e., something like:
diff --git a/transport-helper.c b/transport-helper.c
index 6f227e2..f6b3b1f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -9,6 +9,7 @@
#include "remote.h"
#include "string-list.h"
#include "thread-utils.h"
+#include "sigchain.h"
static int debug;
@@ -220,15 +221,21 @@ static struct child_process *get_helper(struct transport *transport)
static int disconnect_helper(struct transport *transport)
{
struct helper_data *data = transport->data;
- struct strbuf buf = STRBUF_INIT;
int res = 0;
if (data->helper) {
if (debug)
fprintf(stderr, "Debug: Disconnecting.\n");
if (!data->no_disconnect_req) {
- strbuf_addf(&buf, "\n");
- sendline(data, &buf);
+ /*
+ * Ignore write errors; there's nothing we can do,
+ * since we're about to close the pipe anyway. And the
+ * most likely error is EPIPE due to the helper dying
+ * to report an error itself.
+ */
+ sigchain_push(SIGPIPE, SIG_IGN);
+ xwrite(data->helper->in, "\n", 1);
+ sigchain_pop(SIGPIPE);
}
close(data->helper->in);
close(data->helper->out);
which makes the t5541 failures go away for me. What do you think?
-Peff
^ permalink raw reply related
* [PATCHv4] git-p4: RCS keyword handling
From: Luke Diamand @ 2012-02-22 10:15 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Eric Scouten, Junio C Hamano, Luke Diamand
Updated patch incorporating comments from Pete and Junio to
handle RCS keyword expansion in git-p4.
Note: The current patch fails to to handle the case where a user
adds an expanded RCS keyword in *git* (e.g. via cut-n-paste).
I'll try to address that separately. There's a failing test case
for this.
This version:
- uses "p4 fstat" to get the filetype
- uses Junio's suggested regexp to match $Keyword:$
- uses the sets of added/deleted files rather than parsing diff output
- various other small fixes spotted by Pete
- adds additional test cases
Luke Diamand (1):
git-p4: add initial support for RCS keywords
Documentation/git-p4.txt | 5 +
contrib/fast-import/git-p4 | 118 ++++++++++++--
t/t9810-git-p4-rcs.sh | 388 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+), 10 deletions(-)
create mode 100755 t/t9810-git-p4-rcs.sh
--
1.7.9.259.ga92e
^ permalink raw reply
* [PATCHv4] git-p4: add initial support for RCS keywords
From: Luke Diamand @ 2012-02-22 10:15 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Eric Scouten, Junio C Hamano, Luke Diamand
In-Reply-To: <1329905741-2092-1-git-send-email-luke@diamand.org>
RCS keywords cause problems for git-p4 as perforce always
expands them (if +k is set) and so when applying the patch,
git reports that the files have been modified by both sides,
when in fact they haven't.
This change means that when git-p4 detects a problem applying
a patch, it will check to see if keyword expansion could be
the culprit. If it is, it strips the keywords in the p4
repository so that they match what git is expecting. It then
has another go at applying the patch.
This behaviour is enabled with a new git-p4 configuration
option and is off by default.
Improved-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Luke Diamand <luke@diamand.org>
---
Documentation/git-p4.txt | 5 +
contrib/fast-import/git-p4 | 118 ++++++++++++--
t/t9810-git-p4-rcs.sh | 388 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 501 insertions(+), 10 deletions(-)
create mode 100755 t/t9810-git-p4-rcs.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 8b92cc0..3fecefa 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -483,6 +483,11 @@ git-p4.skipUserNameCheck::
user map, 'git p4' exits. This option can be used to force
submission regardless.
+git-p4.attemptRCSCleanup:
+ If enabled, 'git p4 submit' will attempt to cleanup RCS keywords
+ ($Header$, etc). These would otherwise cause merge conflicts and prevent
+ the submit going ahead. This option should be considered experimental at
+ present.
IMPLEMENTATION DETAILS
----------------------
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a78d9c5..c8b6c8a 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -10,7 +10,7 @@
import optparse, sys, os, marshal, subprocess, shelve
import tempfile, getopt, os.path, time, platform
-import re
+import re, shutil
verbose = False
@@ -186,6 +186,47 @@ def split_p4_type(p4type):
mods = s[1]
return (base, mods)
+#
+# return the raw p4 type of a file (text, text+ko, etc)
+#
+def p4_type(file):
+ results = p4CmdList(["fstat", "-T", "headType", file])
+ return results[0]['headType']
+
+#
+# Given a type base and modifier, return a regexp matching
+# the keywords that can be expanded in the file
+#
+def p4_keywords_regexp_for_type(base, type_mods):
+ if base in ("text", "unicode", "binary"):
+ kwords = None
+ if "ko" in type_mods:
+ kwords = 'Id|Header'
+ elif "k" in type_mods:
+ kwords = 'Id|Header|Author|Date|DateTime|Change|File|Revision'
+ else:
+ return None
+ pattern = r"""
+ \$ # Starts with a dollar, followed by...
+ (%s) # one of the keywords, followed by...
+ (:[^$]+)? # possibly an old expansion, followed by...
+ \$ # another dollar
+ """ % kwords
+ return pattern
+ else:
+ return None
+
+#
+# Given a file, return a regexp matching the possible
+# RCS keywords that will be expanded, or None for files
+# with kw expansion turned off.
+#
+def p4_keywords_regexp_for_file(file):
+ if not os.path.exists(file):
+ return None
+ else:
+ (type_base, type_mods) = split_p4_type(p4_type(file))
+ return p4_keywords_regexp_for_type(type_base, type_mods)
def setP4ExecBit(file, mode):
# Reopens an already open file and changes the execute bit to match
@@ -753,6 +794,29 @@ class P4Submit(Command, P4UserMap):
return result
+ def patchRCSKeywords(self, file, pattern):
+ # Attempt to zap the RCS keywords in a p4 controlled file matching the given pattern
+ (handle, outFileName) = tempfile.mkstemp(dir='.')
+ try:
+ outFile = os.fdopen(handle, "w+")
+ inFile = open(file, "r")
+ regexp = re.compile(pattern, re.VERBOSE)
+ for line in inFile.readlines():
+ line = regexp.sub(r'$\1$', line)
+ outFile.write(line)
+ inFile.close()
+ outFile.close()
+ # Forcibly overwrite the original file
+ os.unlink(file)
+ shutil.move(outFileName, file)
+ except:
+ # cleanup our temporary file
+ os.unlink(outFileName)
+ print "Failed to strip RCS keywords in %s" % file
+ raise
+
+ print "Patched up RCS keywords in %s" % file
+
def p4UserForCommit(self,id):
# Return the tuple (perforce user,git email) for a given git commit id
self.getUserMapFromPerforceServer()
@@ -918,6 +982,7 @@ class P4Submit(Command, P4UserMap):
filesToDelete = set()
editedFiles = set()
filesToChangeExecBit = {}
+
for line in diff:
diff = parseDiffTreeEntry(line)
modifier = diff['status']
@@ -964,9 +1029,45 @@ class P4Submit(Command, P4UserMap):
patchcmd = diffcmd + " | git apply "
tryPatchCmd = patchcmd + "--check -"
applyPatchCmd = patchcmd + "--check --apply -"
+ patch_succeeded = True
if os.system(tryPatchCmd) != 0:
+ fixed_rcs_keywords = False
+ patch_succeeded = False
print "Unfortunately applying the change failed!"
+
+ # 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":
+ file = None
+ pattern = None
+ kwfiles = {}
+ for file in editedFiles | filesToDelete:
+ # did this file's delta contain RCS keywords?
+ pattern = p4_keywords_regexp_for_file(file)
+
+ if pattern:
+ # this file is a possibility...look for RCS keywords.
+ regexp = re.compile(pattern, re.VERBOSE)
+ for line in read_pipe_lines(["git", "diff", "%s^..%s" % (id, id), file]):
+ if regexp.search(line):
+ if verbose:
+ print "got keyword match on %s in %s in %s" % (pattern, line, file)
+ kwfiles[file] = pattern
+ break
+
+ for file in kwfiles:
+ if verbose:
+ print "zapping %s with %s" % (line,pattern)
+ self.patchRCSKeywords(file, kwfiles[file])
+ fixed_rcs_keywords = True
+
+ if fixed_rcs_keywords:
+ print "Retrying the patch with RCS keywords cleaned up"
+ if os.system(tryPatchCmd) == 0:
+ patch_succeeded = True
+
+ if not patch_succeeded:
print "What do you want to do?"
response = "x"
while response != "s" and response != "a" and response != "w":
@@ -1585,15 +1686,12 @@ class P4Sync(Command, P4UserMap):
# Note that we do not try to de-mangle keywords on utf16 files,
# even though in theory somebody may want that.
- if type_base in ("text", "unicode", "binary"):
- if "ko" in type_mods:
- text = ''.join(contents)
- text = re.sub(r'\$(Id|Header):[^$]*\$', r'$\1$', text)
- contents = [ text ]
- elif "k" in type_mods:
- text = ''.join(contents)
- text = re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', text)
- contents = [ text ]
+ pattern = p4_keywords_regexp_for_type(type_base, type_mods)
+ if pattern:
+ regexp = re.compile(pattern, re.VERBOSE)
+ text = ''.join(contents)
+ text = regexp.sub(r'$\1$', text)
+ contents = [ text ]
self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
new file mode 100755
index 0000000..9e84b5a
--- /dev/null
+++ b/t/t9810-git-p4-rcs.sh
@@ -0,0 +1,388 @@
+#!/bin/sh
+
+test_description='git-p4 rcs keywords'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+#
+# Make one file with keyword lines at the top, and
+# enough plain text to be able to test modifications
+# far away from the keywords.
+#
+test_expect_success 'init depot' '
+ (
+ cd "$cli" &&
+ cat <<-\EOF >filek &&
+ $Id$
+ /* $Revision$ */
+ # $Change$
+ line4
+ line5
+ line6
+ line7
+ line8
+ EOF
+ cp filek fileko &&
+ sed -i "s/Revision/Revision: do not scrub me/" fileko
+ cp fileko file_text &&
+ sed -i "s/Id/Id: do not scrub me/" file_text
+ p4 add -t text+k filek &&
+ p4 submit -d "filek" &&
+ p4 add -t text+ko fileko &&
+ p4 submit -d "fileko" &&
+ p4 add -t text file_text &&
+ p4 submit -d "file_text"
+ )
+'
+
+#
+# Generate these in a function to make it easy to use single quote marks.
+#
+write_scrub_scripts() {
+ cat >"$TRASH_DIRECTORY/scrub_k.py" <<-\EOF &&
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+ cat >"$TRASH_DIRECTORY/scrub_ko.py" <<-\EOF
+ import re, sys
+ sys.stdout.write(re.sub(r'(?i)\$(Id|Header):[^$]*\$', r'$\1$', sys.stdin.read()))
+ EOF
+}
+
+test_expect_success 'scrub scripts' '
+ write_scrub_scripts
+'
+
+#
+# Compare $cli/file to its scrubbed version, should be different.
+# Compare scrubbed $cli/file to $git/file, should be same.
+#
+scrub_k_check() {
+ file=$1 &&
+ scrub="$TRASH_DIRECTORY/$file" &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/scrub_k.py" <"$git/$file" >"$scrub" &&
+ ! test_cmp "$cli/$file" "$scrub" &&
+ test_cmp "$git/$file" "$scrub" &&
+ rm "$scrub"
+}
+scrub_ko_check() {
+ file=$1 &&
+ scrub="$TRASH_DIRECTORY/$file" &&
+ "$PYTHON_PATH" "$TRASH_DIRECTORY/scrub_ko.py" <"$git/$file" >"$scrub" &&
+ ! test_cmp "$cli/$file" "$scrub" &&
+ test_cmp "$git/$file" "$scrub" &&
+ rm "$scrub"
+}
+
+#
+# Modify far away from keywords. If no RCS lines show up
+# in the diff, there is no conflict.
+#
+test_expect_success 'edit far away from RCS lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ sed -i "s/^line7/line7 edit/" filek &&
+ git commit -m "filek line7 edit" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Modify near the keywords. This will require RCS scrubbing.
+#
+test_expect_success 'edit near RCS lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" filek &&
+ git commit -m "filek line4 edit" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Modify the keywords themselves. This also will require RCS scrubbing.
+#
+test_expect_success 'edit keyword lines' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "/Revision/d" filek &&
+ git commit -m "filek remove Revision line" filek &&
+ "$GITP4" submit &&
+ scrub_k_check filek
+ )
+'
+
+#
+# Scrubbing text+ko files should not alter all keywords, just Id, Header.
+#
+test_expect_success 'scrub ko files differently' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" fileko &&
+ git commit -m "fileko line4 edit" fileko &&
+ "$GITP4" submit &&
+ scrub_ko_check fileko &&
+ ! scrub_k_check fileko
+ )
+'
+
+# hack; git-p4 submit should do it on its own
+test_expect_success 'cleanup after failure' '
+ (
+ cd "$cli" &&
+ p4 revert ...
+ )
+'
+
+#
+# Do not scrub anything but +k or +ko files. Sneak a change into
+# the cli file so that submit will get a conflict. Make sure that
+# scrubbing doesn't make a mess of things.
+#
+# Assumes that git-p4 exits leaving the p4 file open, with the
+# conflict-generating patch unapplied.
+#
+# This might happen only if the git repo is behind the p4 repo at
+# submit time, and there is a conflict.
+#
+test_expect_success 'do not scrub plain text' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ sed -i "s/^line4/line4 edit/" file_text &&
+ git commit -m "file_text line4 edit" file_text &&
+ (
+ cd "$cli" &&
+ p4 open file_text &&
+ sed -i "s/^line5/line5 p4 edit/" file_text &&
+ p4 submit -d "file5 p4 edit"
+ ) &&
+ ! "$GITP4" submit &&
+ (
+ # exepct something like:
+ # file_text - file(s) not opened on this client
+ # but not copious diff output
+ cd "$cli" &&
+ p4 diff file_text >wc &&
+ test_line_count = 1 wc
+ )
+ )
+'
+
+# hack; git-p4 submit should do it on its own
+test_expect_success 'cleanup after failure 2' '
+ (
+ cd "$cli" &&
+ p4 revert ...
+ )
+'
+
+create_kw_file() {
+ cat <<\EOF >"$1"
+/* A file
+ Id: $Id$
+ Revision: $Revision$
+ File: $File$
+ */
+int main(int argc, const char **argv) {
+ return 0;
+}
+EOF
+}
+
+test_expect_success 'add kwfile' '
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "file 1" &&
+ create_kw_file kwfile1.c &&
+ p4 add kwfile1.c &&
+ p4 submit -d "Add rcw kw file" kwfile1.c
+ )
+'
+
+p4_append_to_file() {
+ f=$1 &&
+ p4 edit -t ktext "$f" &&
+ echo "/* $(date) */" >>"$f" &&
+ p4 submit -d "appending a line in p4"
+}
+
+# Create some files with RCS keywords. If they get modified
+# elsewhere then the version number gets bumped which then
+# results in a merge conflict if we touch the RCS kw lines,
+# even though the change itself would otherwise apply cleanly.
+test_expect_success 'cope with rcs keyword expansion damage' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ (cd ../cli && p4_append_to_file kwfile1.c) &&
+ old_lines=$(wc -l <kwfile1.c) &&
+ perl -n -i -e "print unless m/Revision:/" kwfile1.c &&
+ new_lines=$(wc -l <kwfile1.c) &&
+ expr $new_lines == $old_lines - 1 &&
+
+ git add kwfile1.c &&
+ git commit -m "Zap an RCS kw line" &&
+ "$GITP4" submit &&
+ "$GITP4" rebase &&
+ git diff p4/master &&
+ "$GITP4" commit &&
+ echo "try modifying in both" &&
+ cd "$cli" &&
+ p4 edit kwfile1.c &&
+ echo "line from p4" >>kwfile1.c &&
+ p4 submit -d "add a line in p4" kwfile1.c &&
+ cd "$git" &&
+ echo "line from git at the top" | cat - kwfile1.c >kwfile1.c.new &&
+ mv kwfile1.c.new kwfile1.c &&
+ git commit -m "Add line in git at the top" kwfile1.c &&
+ "$GITP4" rebase &&
+ "$GITP4" submit
+ )
+'
+
+test_expect_success 'cope with rcs keyword file deletion' '
+ test_when_finished cleanup_git &&
+ (
+ cd "$cli" &&
+ echo "\$Revision\$" >kwdelfile.c &&
+ p4 add -t ktext kwdelfile.c &&
+ p4 submit -d "Add file to be deleted" &&
+ cat kwdelfile.c &&
+ grep -q 1 kwdelfile.c
+ ) &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ grep -q Revision kwdelfile.c &&
+ git rm -f kwdelfile.c &&
+ git commit -m "Delete a file containing RCS keywords" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ [ ! -f kwdelfile.c ]
+ )
+'
+
+# If you add keywords in git of the form $Header$ then everything should
+# work fine without any special handling.
+test_expect_success 'Add keywords in git which match the default p4 values' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ echo "NewKW: \$Revision\$" >>kwfile1.c &&
+ git add kwfile1.c &&
+ git commit -m "Adding RCS keywords in git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ [ -f kwfile1.c ] &&
+ grep 'NewKW.*Revision.*[0-9]' kwfile1.c
+
+ )
+'
+
+# If you add keywords in git of the form $Header:#1$ then things will fail
+# unless git-p4 takes steps to scrub the *git* commit.
+#
+test_expect_failure 'Add keywords in git which do not match the default p4 values' '
+ test_when_finished cleanup_git &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ echo "NewKW2: \$Revision:1\$" >>kwfile1.c &&
+ git add kwfile1.c &&
+ git commit -m "Adding RCS keywords in git" &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ "$GITP4" submit
+ ) &&
+ (
+ cd "$cli" &&
+ p4 sync &&
+ grep 'NewKW2.*Revision.*[0-9]' kwfile1.c
+
+ )
+'
+
+# Check that the existing merge conflict handling still works.
+# Modify kwfile1.c in git, and delete in p4. We should be able
+# to skip the git commit.
+#
+test_expect_success 'merge conflict handling still works' '
+ test_when_finished cleanup_git &&
+ (
+ cd "$cli" &&
+ echo "Hello:\$Id\$" >merge2.c &&
+ echo "World" >>merge2.c &&
+ p4 add -t ktext merge2.c &&
+ p4 submit -d "add merge test file"
+ ) &&
+ "$GITP4" clone --dest="$git" //depot &&
+ (
+ cd "$git" &&
+ sed -e "/Hello/d" merge2.c >merge2.c.tmp &&
+ mv merge2.c.tmp merge2.c &&
+ git add merge2.c &&
+ git commit -m "Modifying merge2.c"
+ ) &&
+ (
+ cd "$cli" &&
+ p4 delete merge2.c &&
+ p4 submit -d "remove merge test file"
+ ) &&
+ (
+ cd "$git" &&
+ [ -f merge2.c ] &&
+ git config git-p4.skipSubmitEdit true &&
+ git config git-p4.attemptRCSCleanup true &&
+ !(echo "s" | "$GITP4" submit) &&
+ git rebase --skip &&
+ [ ! -f merge2.c ]
+ )
+'
+
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.9.259.ga92e
^ permalink raw reply related
* Re: git status: small difference between stating whole repository and small subdirectory
From: Nguyen Thai Ngoc Duy @ 2012-02-22 10:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Jeff King, Piotr Krukowiecki, Git Mailing List
In-Reply-To: <7v8vjwgfoq.fsf@alter.siamese.dyndns.org>
On Tue, Feb 21, 2012 at 11:16:37AM -0800, Junio C Hamano wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
> > I'm aware that Jeff's tackling at lower level, which retains
> > cache-tree for many more cases.
> >
> > But this patch seems simple and safe
> > to me, and in my experience this case happens quite often (or maybe I
> > tend to keep my index clean). Junio, any chance this patch may get in?
>
> I do not think we are talking about a duplicated effort here.
>
> By definition, the change to hook into unpack_trees() and making sure we
> invalidate all the necessary subtrees in the cache cannot give you a cache
> tree that is more populated than what you started with. And the train of
> thought in Peff's message is to improve this invalidation---we currently
> invalidate everything ;-)
>
> Somebody has to populate the cache tree fully when we _know_ the index
> matches a certain tree, and adding a call to prime_cache_tree() in
> strategic places is a way to do so. The most obvious is write-tree, but
> there are a few other existing codepaths that do so.
>
> Because prime_cache_tree() by itself is a fairly expensive operation that
> reads all the trees recursively, its benefits need to be evaluated. It
> should to happen only in an operation that is already heavy-weight, is
> likely to have read all the trees and have many of them in-core cache, and
> also relatively rarely happens compared to "git add" so that the cost can
> be amortised over time, such as "reset --(hard|mixed)".
It's tradeoff. As you said prime_cache_tree() is expensive.
cache_tree_update is supposed to be cheap. But cache_tree_update() when
empty is even more expensive than prime_cache_tree(). So it boils down
how much cache-tree we have after unpack_trees() and whether it's worth
repopulate cache-tree again.
> Switching branches is likely to fall into that category, but that is just
> my gut feeling. I would feel better at night if somebody did a benchmark
> ;-)
I timed prime_cache_tree() and cache_tree_update() while switching branch
on linux-2.6, between v2.6.32 and a quite recent version. prime_cache_tree()
took ~55ms while cache_tree_update() 150ms or 90ms (depending on final tree).
It depends on your view, whether 55ms is expensive on such a reasonably large
repository. I took several seconds for me to complete the checkout though.
Checking out with "-q" prime_cache_tree() took 145ms and 80ms respectively,
as expensive as cache_tree_update()
If cache-tree is only used at commit time, I think we could delay
prime_cache_tree() until absolutely needed. We could add an optional index
extension recording the fact that index matches certain tree.
On the first cache_tree_invalidate_path(), if cache-tree is still
empty, we prime cache-tree, then invalidate the requested path.
It would then add no cost to a quick branch switch.
But if diff-cached also takes advantage of cache-tree, it's a different story.
Anyway, I think this patch does better than my last one
-- 8< --
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6b9061f..e7eaeec 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -387,6 +387,7 @@ static int merge_working_tree(struct checkout_opts *opts,
int ret;
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
int newfd = hold_locked_index(lock_file, 1);
+ int head_index_mismatch = 1;
if (read_cache_preload(NULL) < 0)
return error(_("corrupt index file"));
@@ -396,6 +397,7 @@ static int merge_working_tree(struct checkout_opts *opts,
ret = reset_tree(new->commit->tree, opts, 1);
if (ret)
return ret;
+ head_index_mismatch = 0;
} else {
struct tree_desc trees[2];
struct tree *tree;
@@ -490,7 +492,27 @@ static int merge_working_tree(struct checkout_opts *opts,
ret = reset_tree(new->commit->tree, opts, 0);
if (ret)
return ret;
- }
+ } else
+ head_index_mismatch = topts.head_index_mismatch;
+ }
+
+ /*
+ * Currently cache-tree is always destroyed after
+ * unpack_trees(). It's probably a good idea to repopulate
+ * cache-tree. If the user makes a few modifications and
+ * commits, tree generation woulda be cheap. If they switch
+ * away again, not so cheap.
+ *
+ * When unpack_trees() learns to retains as much cache-tree as
+ * possible, this code probably does not help much on tree
+ * generation, unless the tree difference between to heads are
+ * too far, little cache-tree can be kept.
+ */
+ if (!head_index_mismatch &&
+ !cache_tree_fully_valid(active_cache_tree)) {
+ if (!new->commit->tree->object.parsed)
+ parse_tree(new->commit->tree);
+ prime_cache_tree(&active_cache_tree, new->commit->tree);
}
if (write_cache(newfd, active_cache, active_nr) ||
diff --git a/unpack-trees.c b/unpack-trees.c
index 7c9ecf6..f2c518f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1022,6 +1022,8 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
o->result.timestamp.nsec = o->src_index->timestamp.nsec;
o->merge_size = len;
mark_all_ce_unused(o->src_index);
+ if (o->fn != twoway_merge)
+ o->head_index_mismatch = 1;
/*
* Sparse checkout loop #1: set NEW_SKIP_WORKTREE on existing entries
@@ -1736,6 +1738,8 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)
(oldtree && newtree &&
!same(oldtree, newtree) && /* 18 and 19 */
same(current, newtree))) {
+ if (!newtree || (newtree && !same(current, newtree)))
+ o->head_index_mismatch = 1;
return keep_entry(current, o);
}
else if (oldtree && !newtree && same(current, oldtree)) {
diff --git a/unpack-trees.h b/unpack-trees.h
index 5e432f5..b75b64e 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -48,7 +48,8 @@ struct unpack_trees_options {
gently,
exiting_early,
show_all_errors,
- dry_run;
+ dry_run,
+ head_index_mismatch;
const char *prefix;
int cache_bottom;
struct dir_struct *dir;
-- 8< --
^ permalink raw reply related
* Re: [PATCH 0/8 v6] diff --stat: use the full terminal width
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-22 11:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael J Gruber, pclouds, j.sixt
In-Reply-To: <7v1upogd6w.fsf@alter.siamese.dyndns.org>
On 02/21/2012 09:10 PM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl> writes:
>
>> This seem overly complex. A nice property to have would be
>> "if the window is wide enough so there's enough space for full
>> filenames, the graph part scales monotonically with the change count".
>> (If there's filename truncation, than there just isn't enough space
>> for everything and the graph may be compressed. But otherwise, if we
>> have two graphs which do not end at the edge of the screen, and the
>> second one is wider than the first one, then without looking at the
>> change counts we know that the second one has more changes).
>>
>> For this property to be satisfied, the graph_width limit would have to
>> be independent of the filename width.
>>
>> So maybe it should be ...
>
> Sorry, the desired property I would understand, but that does not click
> with your "have to be independent" conclusion, so I do not have comment on
> the "maybe it should be..." part.
Hi,
by "scales monotonically with the change count" I meant with two
different commits. Image that there are two commits
a | 300 ++++++++++++++++++++++
and
a/a/a/b | 300 ++++++++++++++++++++++
Both commits have the same change count, but filenames of different
length. If the filename length can influence the number of "+" in the
graph, then the scaling is not monotonic. There would always be cases
when a bigger change with longer filenames has a narrower graph.
> The resolution requirement may want to set a "desired lower limit" for the
> width of the graph, but it is only "desired" because it is possible that
> you have to bust the limit if you have three files with 1, 9999 and 10000
> changed lines and your terminal is only 200 columns wide.
>
> The current code caps name part to 50/80, but allows the graph to use more
> when you have only shorter names. Perhaps you can follow the same logic
> in the first part of your [7/8] (which needs to be separated to at least
> in two pieces, as it conflates the "lift 50-column cap from the name width
> and make it proportional to the term_width()" part and "but cap the graph
> part to 40-column" part, that are separate topics)? Then we can try
> different heuristics to find a better way to cap the length of the graph
> on top?
Sure. I'll be replying to this mail with patches
[7.1/8] use a maximum of 5/8 for the filename part
[7.2/8] add a test for output with COLUMNS=40
[7.3/8] limit graph part to 40 columns
--
Zbyszek
^ permalink raw reply
* contrib/git-move-refs.py not exists in cvs2svn-2.3.0
From: supadhyay @ 2012-02-22 11:25 UTC (permalink / raw)
To: git
Hi All,
I have run the cvs2git tool to migrate my CVS repository to git. I am able
to finish successfully. Now as the last recommended steps from
http://cvs2svn.tigris.org/cvs2git.html , I found to run
"contrib/git-move-refs.py" but in my cvs2svn-2.3.0 I can not see any file
name like "git-move-refs.ph", I have one file "git-move-tags.py". Is this
is the same file ?
Can anyone please update me?
thanks.
--
View this message in context: http://git.661346.n2.nabble.com/contrib-git-move-refs-py-not-exists-in-cvs2svn-2-3-0-tp7308023p7308023.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH 7.3/8] diff --stat: limit graph part to 40 columns
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-22 11:51 UTC (permalink / raw)
To: git, gitster
Cc: Michael J Gruber, pclouds, j.sixt,
Zbigniew Jędrzejewski-Szmek
In-Reply-To: <1329911507-10587-1-git-send-email-zbyszek@in.waw.pl>
The way that available columns are divided between the filename part
and the graph part is modified to use as many columns as necessary for
the filenames and up to 40 columns for the graph.
If commits changing a lot of lines are displayed in a wide terminal
window (200 or more columns), and the +- graph would use the full
width, the output would look bad. Messages wrapped to about 80 columns
would be interspersed with very long +- lines. It makes sense to limit
the width of the graph part to a fixed value, even if more columns are
available. This fixed value is subjectively hard-coded to be 40
columns, which seems to work well for git.git and linux-2.6.git and
some other repositories.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
Documentation/diff-options.txt | 2 +-
diff.c | 8 ++++++--
t/t4052-stat-output.sh | 16 ++++++----------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git Documentation/diff-options.txt Documentation/diff-options.txt
index 29647e5..e4d0e3e 100644
--- Documentation/diff-options.txt
+++ Documentation/diff-options.txt
@@ -54,7 +54,7 @@ endif::git-format-patch[]
--stat[=<width>[,<name-width>[,<count>]]]::
Generate a diffstat. By default, as much space as necessary
- will be used for the filename part, and the rest for
+ will be used for the filename part, and up to 40 columns for
the graph part. Maximum width defaults to terminal width,
or 80 columns if not connected to a terminal, and can be
overriden by `<width>`. The width of the filename part can be
diff --git diff.c diff.c
index f1c278f..8a9a387 100644
--- diff.c
+++ diff.c
@@ -1421,13 +1421,15 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
/*
* We have width = stat_width or term_columns() columns total.
* We want a maximum of min(max_len, stat_name_width) for the name part.
+ * We want a maximum of min(max_change, 40) for the +- part.
* We also need 1 for " " and 4 + decimal_width(max_change)
* for " | NNNN " and one the empty column at the end, altogether
* 6 + decimal_width(max_change).
*
* If there's not enough space, we will use the smaller of
* stat_name_width (if set) and 5/8*width for the filename,
- * and the rest for constant elements + graph part.
+ * and the rest for constant elements + graph part, but no more
+ * than 40 for the graph part.
* (5/8 gives 50 for filename and 30 for the constant parts + graph
* for the standard terminal size).
*
@@ -1452,7 +1454,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
/*
* First assign sizes that are wanted, ignoring available width.
*/
- graph_width = max_change;
+ graph_width = max_change < 40 ? max_change : 40;
name_width = (options->stat_name_width > 0 &&
options->stat_name_width < max_len) ?
options->stat_name_width : max_len;
@@ -1463,6 +1465,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
if (name_width + number_width + 6 + graph_width > width) {
if (graph_width > width * 3/8 - number_width - 6)
graph_width = width * 3/8 - number_width - 6;
+ if (graph_width > 40)
+ graph_width = 40;
if (name_width > width - number_width - 6 - graph_width)
name_width = width - number_width - 6 - graph_width;
else
diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh
index 84be8bd..1b237b7 100755
--- t/t4052-stat-output.sh
+++ t/t4052-stat-output.sh
@@ -78,11 +78,7 @@ test_expect_success 'preparation for big change tests' '
'
cat >expect80 <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
-EOF
-
-cat >expect200 <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
while read verb expect cmd args
@@ -94,9 +90,9 @@ do
'
done <<\EOF
ignores expect80 format-patch -1 --stdout
-respects expect200 diff HEAD^ HEAD --stat
-respects expect200 show --stat
-respects expect200 log -1 --stat
+respects expect80 diff HEAD^ HEAD --stat
+respects expect80 show --stat
+respects expect80 log -1 --stat
EOF
cat >expect40 <<'EOF'
@@ -170,7 +166,7 @@ cat >expect80 <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++
EOF
cat >expect200 <<'EOF'
- aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
while read verb expect cmd args
do
@@ -187,7 +183,7 @@ respects expect200 log -1 --stat
EOF
cat >expect <<'EOF'
- abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ abcd | 1000 ++++++++++++++++++++++++++++++++++++++++
EOF
test_expect_success 'merge --stat respects COLUMNS (big change)' '
git checkout -b branch HEAD^^ &&
--
1.7.9.1.355.ge8a9f
^ permalink raw reply related
* [PATCH 7.2/8] diff --stat: add a test for output with COLUMNS=40
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-22 11:51 UTC (permalink / raw)
To: git, gitster
Cc: Michael J Gruber, pclouds, j.sixt,
Zbigniew Jędrzejewski-Szmek
In-Reply-To: <1329911507-10587-1-git-send-email-zbyszek@in.waw.pl>
In preparation for the introduction on the limit of the width of the
graph part, a new test with COLUMNS=40 is added to check that the
environment variable influences diff, show, log, but not format-patch.
A new test is added because limiting the graph part makes COLUMNS=200
stop influencing diff --stat behaviour, which isn't wide enough now.
The old test with COLUMNS=200 is retained to check for regressions.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
t/t4052-stat-output.sh | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh
index c250744..84be8bd 100755
--- t/t4052-stat-output.sh
+++ t/t4052-stat-output.sh
@@ -99,6 +99,25 @@ respects expect200 show --stat
respects expect200 log -1 --stat
EOF
+cat >expect40 <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++
+EOF
+
+while read verb expect cmd args
+do
+ test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
+ COLUMNS=40 git $cmd $args >output
+ grep " | " output >actual &&
+ test_cmp "$expect" actual
+ '
+done <<\EOF
+ignores expect80 format-patch -1 --stdout
+respects expect40 diff HEAD^ HEAD --stat
+respects expect40 show --stat
+respects expect40 log -1 --stat
+EOF
+
+
cat >expect <<'EOF'
abcd | 1000 ++++++++++++++++++++++++++
EOF
--
1.7.9.1.355.ge8a9f
^ permalink raw reply related
* [PATCH 7.1/8] diff --stat: use a maximum of 5/8 for the filename part
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-22 11:51 UTC (permalink / raw)
To: git, gitster
Cc: Michael J Gruber, pclouds, j.sixt,
Zbigniew Jędrzejewski-Szmek
In-Reply-To: <4F44D084.7030308@in.waw.pl>
The way that available columns are divided between the filename part
and the graph part is modified to use as many columns as necessary for
the filenames and the rest for the graph.
If there isn't enough columns to print both the filename and the
graph, at least 5/8 of available space is devoted to filenames. On a
standard 80 column terminal, or if not connected to a terminal and
using the default of 80 columns, this gives the same partition as
before.
The effect of this change is visible in the patch to t4052 that fixes a
few tests marked with test_expect_failure.
Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---
Documentation/diff-options.txt | 14 ++++---
diff.c | 90 ++++++++++++++++++++++++++--------------
t/t4052-stat-output.sh | 14 +++----
3 files changed, 75 insertions(+), 43 deletions(-)
diff --git Documentation/diff-options.txt Documentation/diff-options.txt
index 9ed78c9..29647e5 100644
--- Documentation/diff-options.txt
+++ Documentation/diff-options.txt
@@ -53,13 +53,15 @@ endif::git-format-patch[]
Generate a diff using the "patience diff" algorithm.
--stat[=<width>[,<name-width>[,<count>]]]::
- Generate a diffstat. You can override the default
- output width for 80-column terminal by `--stat=<width>`.
- The width of the filename part can be controlled by
- giving another width to it separated by a comma.
+ Generate a diffstat. By default, as much space as necessary
+ will be used for the filename part, and the rest for
+ the graph part. Maximum width defaults to terminal width,
+ or 80 columns if not connected to a terminal, and can be
+ overriden by `<width>`. The width of the filename part can be
+ limited by giving another width `<name-width>` after a comma.
By giving a third parameter `<count>`, you can limit the
- output to the first `<count>` lines, followed by
- `...` if there are more.
+ output to the first `<count>` lines, followed by `...` if
+ there are more.
+
These parameters can also be set individually with `--stat-width=<width>`,
`--stat-name-width=<name-width>` and `--stat-count=<count>`.
diff --git diff.c diff.c
index 1db46a4..f1c278f 100644
--- diff.c
+++ diff.c
@@ -1375,7 +1375,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
int i, len, add, del, adds = 0, dels = 0;
uintmax_t max_change = 0, max_len = 0;
int total_files = data->nr;
- int width, name_width, count;
+ int width, name_width, graph_width, number_width = 4, count;
const char *reset, *add_c, *del_c;
const char *line_prefix = "";
int extra_shown = 0;
@@ -1389,28 +1389,15 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
line_prefix = msg->buf;
}
- if (options->stat_width == -1)
- width = term_columns();
- else
- width = options->stat_width ? options->stat_width : 80;
- name_width = options->stat_name_width ? options->stat_name_width : 50;
count = options->stat_count ? options->stat_count : data->nr;
- /* Sanity: give at least 5 columns to the graph,
- * but leave at least 10 columns for the name.
- */
- if (width < 25)
- width = 25;
- if (name_width < 10)
- name_width = 10;
- else if (width < name_width + 15)
- name_width = width - 15;
-
- /* Find the longest filename and max number of changes */
reset = diff_get_color_opt(options, DIFF_RESET);
add_c = diff_get_color_opt(options, DIFF_FILE_NEW);
del_c = diff_get_color_opt(options, DIFF_FILE_OLD);
+ /*
+ * Find the longest filename and max number of changes
+ */
for (i = 0; (i < count) && (i < data->nr); i++) {
struct diffstat_file *file = data->files[i];
uintmax_t change = file->added + file->deleted;
@@ -1431,19 +1418,62 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
}
count = i; /* min(count, data->nr) */
- /* Compute the width of the graph part;
- * 10 is for one blank at the beginning of the line plus
- * " | count " between the name and the graph.
+ /*
+ * We have width = stat_width or term_columns() columns total.
+ * We want a maximum of min(max_len, stat_name_width) for the name part.
+ * We also need 1 for " " and 4 + decimal_width(max_change)
+ * for " | NNNN " and one the empty column at the end, altogether
+ * 6 + decimal_width(max_change).
+ *
+ * If there's not enough space, we will use the smaller of
+ * stat_name_width (if set) and 5/8*width for the filename,
+ * and the rest for constant elements + graph part.
+ * (5/8 gives 50 for filename and 30 for the constant parts + graph
+ * for the standard terminal size).
*
- * From here on, name_width is the width of the name area,
- * and width is the width of the graph area.
+ * In other words: stat_width limits the maximum width, and
+ * stat_name_width fixes the maximum width of the filename,
+ * and is also used to divide available columns if there
+ * aren't enough.
*/
- name_width = (name_width < max_len) ? name_width : max_len;
- if (width < (name_width + 10) + max_change)
- width = width - (name_width + 10);
+
+ if (options->stat_width == -1)
+ width = term_columns();
else
- width = max_change;
+ width = options->stat_width ? options->stat_width : 80;
+ /*
+ * Guarantee 3/8*16==6 for the graph part
+ * and 5/8*16==10 for the filename part
+ */
+ if (width < 16 + 6 + number_width)
+ width = 16 + 6 + number_width;
+
+ /*
+ * First assign sizes that are wanted, ignoring available width.
+ */
+ graph_width = max_change;
+ name_width = (options->stat_name_width > 0 &&
+ options->stat_name_width < max_len) ?
+ options->stat_name_width : max_len;
+
+ /*
+ * Adjust adjustable widths not to exceed maximum width
+ */
+ if (name_width + number_width + 6 + graph_width > width) {
+ if (graph_width > width * 3/8 - number_width - 6)
+ graph_width = width * 3/8 - number_width - 6;
+ if (name_width > width - number_width - 6 - graph_width)
+ name_width = width - number_width - 6 - graph_width;
+ else
+ graph_width = width - number_width - 6 - name_width;
+ }
+
+ /*
+ * From here name_width is the width of the name area,
+ * and graph_width is the width of the graph area.
+ * max_change is used to scale graph properly.
+ */
for (i = 0; i < count; i++) {
const char *prefix = "";
char *name = data->files[i]->print_name;
@@ -1499,18 +1529,18 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
adds += add;
dels += del;
- if (width <= max_change) {
+ if (graph_width <= max_change) {
int total = add + del;
- total = scale_linear(add + del, width, max_change);
+ total = scale_linear(add + del, graph_width, max_change);
if (total < 2 && add && del)
/* width >= 2 due to the sanity check */
total = 2;
if (add < del) {
- add = scale_linear(add, width, max_change);
+ add = scale_linear(add, graph_width, max_change);
del = total - add;
} else {
- del = scale_linear(del, width, max_change);
+ del = scale_linear(del, graph_width, max_change);
add = total - del;
}
}
diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh
index 2b4510c..c250744 100755
--- t/t4052-stat-output.sh
+++ t/t4052-stat-output.sh
@@ -28,19 +28,19 @@ EOF
while read cmd args
do
- test_expect_failure "$cmd graph width defaults to 80 columns" '
+ test_expect_success "$cmd graph width defaults to 80 columns" '
git $cmd $args >output &&
grep " | " output >actual &&
test_cmp expect80 actual
'
- test_expect_failure "$cmd --stat=width with long name" '
+ test_expect_success "$cmd --stat=width with long name" '
git $cmd $args --stat=40 >output &&
grep " | " output >actual &&
test_cmp expect40 actual
'
- test_expect_failure "$cmd --stat-width=width with long name" '
+ test_expect_success "$cmd --stat-width=width with long name" '
git $cmd $args --stat-width=40 >output &&
grep " | " output >actual &&
test_cmp expect40 actual
@@ -87,7 +87,7 @@ EOF
while read verb expect cmd args
do
- test_expect_success "$cmd $verb COLUMNS (big change)" '
+ test_expect_success "$cmd $verb too many COLUMNS (big change)" '
COLUMNS=200 git $cmd $args >output
grep " | " output >actual &&
test_cmp "$expect" actual
@@ -130,7 +130,7 @@ test_expect_success 'preparation for long filename tests' '
'
cat >expect <<'EOF'
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++
+ ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
EOF
while read cmd args
@@ -151,7 +151,7 @@ cat >expect80 <<'EOF'
...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++
EOF
cat >expect200 <<'EOF'
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
EOF
while read verb expect cmd args
do
@@ -178,7 +178,7 @@ test_expect_success 'merge --stat respects COLUMNS (big change)' '
'
cat >expect <<'EOF'
- ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++
+ aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++
EOF
test_expect_success 'merge --stat respects COLUMNS (long filename)' '
COLUMNS=100 git merge --stat --no-ff master >output &&
--
1.7.9.1.355.ge8a9f
^ permalink raw reply related
* Re: [PATCHv4] git-p4: add initial support for RCS keywords
From: Pete Wyckoff @ 2012-02-22 12:53 UTC (permalink / raw)
To: Luke Diamand; +Cc: git, Eric Scouten, Junio C Hamano
In-Reply-To: <1329905741-2092-2-git-send-email-luke@diamand.org>
luke@diamand.org wrote on Wed, 22 Feb 2012 10:15 +0000:
> RCS keywords cause problems for git-p4 as perforce always
> expands them (if +k is set) and so when applying the patch,
> git reports that the files have been modified by both sides,
> when in fact they haven't.
>
> This change means that when git-p4 detects a problem applying
> a patch, it will check to see if keyword expansion could be
> the culprit. If it is, it strips the keywords in the p4
> repository so that they match what git is expecting. It then
> has another go at applying the patch.
>
> This behaviour is enabled with a new git-p4 configuration
> option and is off by default.
>
> Improved-by: Pete Wyckoff <pw@padd.com>
> Signed-off-by: Luke Diamand <luke@diamand.org>
Looks brilliant. Ack. Thanks for suffering through N rounds of
review. :)
-- Pete
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Nguyen Thai Ngoc Duy @ 2012-02-22 12:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Jeff King, Piotr Krukowiecki, Git Mailing List
In-Reply-To: <7v7gzfefw1.fsf@alter.siamese.dyndns.org>
On Wed, Feb 22, 2012 at 9:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> That makes me think if "diff --cached" can take advantage of
>> cache-tree to avoid walking down valid cached trees and do tree-tree
>> diff in those cases instead. Not sure if it gains us anything but code
>> complexity.
>
> Why do I have this funny feeling that we saw that comment in this thread
> already?
Simple. You wrote it and I missed it.
On Sat, Feb 18, 2012 at 5:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> That being said, we do have an index extension to store the tree sha1 of
>> whole directories (i.e., we populate it when we write a whole tree or
>> subtree into the index from the object db, and it becomes invalidated
>> when a file becomes modified). This optimization is used by things like
>> "git commit" to avoid having to recreate the same sub-trees over and
>> over when creating tree objects from the index. But we could also use it
>> here to avoid having to even read the sub-tree objects from the object
>> db.
>
> Like b65982b (Optimize "diff-index --cached" using cache-tree, 2009-05-20)
> perhaps?
This optimizes the case when a cached tree matches entirely.I wonder
whether it's faster if we switch to tree-tree diff whenever we find
valid cached trees. If cache-tree is fully valid, "git diff --cached
foo" would be equivalent to "git diff HEAD foo".
I tried "git diff --raw HEAD HEAD~100" (where HEAD was
v3.1-rc1-272-g73e0881 on linux-2.6) and "git diff --cached --raw
HEAD~100" with no cache-tree. The former is a little bit faster than
the latter (177ms vs 275ms). On gentoo-x86, 70k worktree files, it's
4.33s vs 4.45s. But in tree-tree diff we pay high in cold cache case
for loading trees from "HEAD". So no, probably not worth more code
changes. Your optimization is good enough.
--
Duy
^ permalink raw reply
* Re: git status: small difference between stating whole repository and small subdirectory
From: Thomas Rast @ 2012-02-22 13:17 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: Junio C Hamano, Jeff King, Piotr Krukowiecki, Git Mailing List
In-Reply-To: <CACsJy8BZyokLSpbA=C6YEO1JajyemT9vLG=C9W4E4coA5OfszQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Sat, Feb 18, 2012 at 5:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jeff King <peff@peff.net> writes:
>>
>>> That being said, we do have an index extension to store the tree sha1 of
>>> whole directories (i.e., we populate it when we write a whole tree or
>>> subtree into the index from the object db, and it becomes invalidated
>>> when a file becomes modified). This optimization is used by things like
>>> "git commit" to avoid having to recreate the same sub-trees over and
>>> over when creating tree objects from the index. But we could also use it
>>> here to avoid having to even read the sub-tree objects from the object
>>> db.
>>
>> Like b65982b (Optimize "diff-index --cached" using cache-tree, 2009-05-20)
>> perhaps?
>
> This optimizes the case when a cached tree matches entirely.I wonder
> whether it's faster if we switch to tree-tree diff whenever we find
> valid cached trees. If cache-tree is fully valid, "git diff --cached
> foo" would be equivalent to "git diff HEAD foo".
Not necessarily; the cache-tree is valid if it faithfully represents
what is in the index. It does not have any direct relation to HEAD.
> I tried "git diff --raw HEAD HEAD~100" (where HEAD was
> v3.1-rc1-272-g73e0881 on linux-2.6) and "git diff --cached --raw
> HEAD~100" with no cache-tree. The former is a little bit faster than
> the latter (177ms vs 275ms). On gentoo-x86, 70k worktree files, it's
> 4.33s vs 4.45s. But in tree-tree diff we pay high in cold cache case
> for loading trees from "HEAD". So no, probably not worth more code
> changes. Your optimization is good enough.
I'm still wondering about using mincore() to good effect. I tried it
for git-grep, but it ended up slowing things down. However, it irks me
that in some cases a clueful use of one form over the other can really
make a huge performance difference, e.g.,
git grep stuff
git grep HEAD stuff
If I am in a big repository that I haven't used in a while, the HEAD
form will be much faster as the worktree search would fault many files.
OTOH if I am in a heavily-used repository (and perhaps just said 'make'
minutes ago) the worktree version will avoid the pack decompression
effort.
Sadly this also has the problem that we must first determine whether
substituting HEAD for the worktree (or vice versa) is valid at all. For
grep perhaps there could be a "just do a fast search somewhere" option
since usually you are looking for something that hasn't changed in ages.
Ok, that was almost completely beside the point of this thread.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] Improve http proxy support
From: Nelson Benítez León @ 2012-02-22 14:04 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 884 bytes --]
Hi, my initial motivation for this patch was to add NTLM proxy
authentication so I could 'git clone' from inside my employers
network, but apart from doing that, I also added two more features,
so, this patch adds the following improvements to the http proxy
support:
- Support NTLM proxy authentication (as well as other authentication
methods) by setting CURLOPT_PROXYAUTH[1] to CURLAUTH_ANY.
- Look up environment vars http_proxy and HTTP_PROXY in case git
http.proxy config option is not set. This supports system wide proxy
support in terminals.
- Support proxy urls with username but without a password, in which
case we interactively ask for the password (as it's already done in
http auth code). This makes possible to not have the password written
down in git config files or in env vars.
Thanks!
[1] http://curl.haxx.se/libcurl/c/curl_easy_setopt.html#CURLOPTPROXYAUTH
[-- Attachment #2: gitproxyauthv2.patch --]
[-- Type: application/octet-stream, Size: 1701 bytes --]
--- http.c 2012-01-19 01:19:22.000000000 +0100
+++ http.mod2.c 2012-02-22 14:44:11.727773038 +0100
@@ -299,8 +299,54 @@
if (curl_ftp_no_epsv)
curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
- if (curl_http_proxy)
- curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ if (!curl_http_proxy) {
+ const char *env_proxy;
+ env_proxy = getenv("HTTP_PROXY");
+ if (!env_proxy) {
+ env_proxy = getenv("http_proxy");
+ }
+ if (env_proxy) {
+ curl_http_proxy = xstrdup(env_proxy);
+ }
+ }
+ if (curl_http_proxy) {
+ char *at, *colon, *proxyuser;
+ const char *cp;
+ cp = strstr(curl_http_proxy, "://");
+ if (cp == NULL) {
+ cp = curl_http_proxy;
+ } else {
+ cp += 3;
+ }
+ at = strchr(cp, '@');
+ colon = strchr(cp, ':');
+ if (at && (!colon || at < colon)) {
+ /* proxy string has username but no password, ask for password */
+ char *ask_str, *proxyuser, *proxypass;
+ int len;
+ struct strbuf pbuf = STRBUF_INIT;
+ len = at - cp;
+ proxyuser = xmalloc(len + 1);
+ memcpy(proxyuser, cp, len);
+ proxyuser[len] = '\0';
+
+ strbuf_addf(&pbuf, "Enter password for proxy %s...", at+1);
+ ask_str = strbuf_detach(&pbuf, NULL);
+ proxypass = xstrdup(git_getpass(ask_str));
+
+ strbuf_insert(&pbuf, 0, curl_http_proxy, cp - curl_http_proxy);
+ strbuf_addf(&pbuf, "%s:%s", proxyuser, proxypass);
+ strbuf_add(&pbuf, at, strlen(at));
+ curl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&pbuf, NULL));
+
+ free(ask_str);
+ free(proxyuser);
+ free(proxypass);
+ } else {
+ curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ }
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
+ }
return result;
}
^ permalink raw reply
* Re: [PATCH] Improve http proxy support
From: Matthieu Moy @ 2012-02-22 14:13 UTC (permalink / raw)
To: Nelson Benítez León; +Cc: git
In-Reply-To: <CAAUd640GaLz4TGs_Lz6KbSFK0VcEVxGfO6PpSCdhch+fYwVovw@mail.gmail.com>
Nelson Benítez León <nbenitezl@gmail.com> writes:
> Hi, my initial motivation for this patch was to add NTLM proxy
> authentication [...]
That sounds interesting, but please read Documentation/SubmittingPatches
in Git's tree. The formatting of your email is wrong (giving more work
for your maintainer) and you need to sign-off your patch to allow your
code to be legally included.
Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: Ambiguous reference weirdness
From: Phil Hord @ 2012-02-22 14:48 UTC (permalink / raw)
To: Jeff King; +Cc: git, Ramkumar Ramachandra, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20120222070034.GA17015@sigill.intra.peff.net>
On Wed, Feb 22, 2012 at 2:00 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Feb 21, 2012 at 08:46:24PM -0500, Phil Hord wrote:
>
>> I accidentally ran into this today:
>> $ git cherry-pick 1147
>> fatal: BUG: expected exactly one commit from walk
>>
>> git log shows no output:
>> $ git log 1147
>
> What is 1147? Is it supposed to be a partial sha1, or is it a ref you
> have?
1147 was a typo. It was a Gerritt changeset ID I forgot to expand.
When I type "git cherry-pick 1147<TAB>", autocompletion expands this
for me to "git cherry-pick origin/changes/47/1147/1". Except in this
case I misfired the TAB and got the weird "BUG:" report. But I didn't
see the same problem with other invalid refs, so I went searching for
the variants and to see what caused it.
> Have you looked at the object that it resolves to? I suspect it is the
> partial sha1 of a non-commit object. E.g.:
All of these examples were run in current git.git, so you can try them
yourself if needed. But I did figure out that 1147 resolves to a
blob, and that's apparently the difference between these three:
$ git cherry-pick 1147
fatal: BUG: expected exactly one commit from walk
$ git cherry-pick 1146
error: short SHA1 1146 is ambiguous.
error: short SHA1 1146 is ambiguous.
fatal: ambiguous argument '1146': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
$ git cherry-pick 114333
fatal: ambiguous argument '114333': unknown revision or path not in
the working tree.
Use '--' to separate paths from revisions
I consider the first two responses to be UI bugs. The second one is
minor (the twice-reported error message), and the first one is pretty
rude. I would expect all three to report the same conclusion,
"fatal: ambiguous argument 'XXXXX': unknown revision or path not in
the working tree." But the first one doesn't.
> $ git cat-file -t HEAD^{tree}
> tree
> $ git cherry-pick HEAD^{tree}
> fatal: BUG: expected exactly one commit from walk
> $ git log HEAD^{tree} | wc -l
> 0
Thanks. I'm not familiar with the {tree} syntax -- in fact I'd like
to find a dictionary for all the reference spelling variants -- but
this is elucidating.
> In the cherry-pick case, the code is checking the right thing, but the
> message is horrible. It is not a bug, but merely unexpected input, and
> it should provide a usage message.
Bug is too strong a word in one sense, but from the user perspective I
consider this "horrible message" a bug.
> In the log case, we totally ignore any pending non-revision arguments.
> So it is correct to produce no output (there is nothing to show, which
> is not unusual in itself; many queries end up producing empty output).
> But we should probably notice that there are pending objects left over
> and produce some kind of diagnostic.
I think you're right about this. I tried log as an afterthought and
was surprised by the varying results.
> I've reordered some of your example commands below to fit the flow of my
> explanation better.
>
>> $git log 114
>> fatal: ambiguous argument '114': unknown revision or path not in the
>> working tree.
>> Use '--' to separate paths from revisions
>
> Right. I think we require at least 4 characters in a partial sha1, so we
> don't treat that as a possibility. So we are left guessing whether you
> mean to do:
>
> git log 114 --
>
> or
>
> git log HEAD -- 114
>
> since it exists as neither a revision nor a path, and the error message
> reflects that (the first one is an error, as there is no such revision.
> The second is a correct query, though one that does not produce any
> results).
>
>> $ git checkout 114
>> error: pathspec '114' did not match any file(s) known to git.
>
> I think checkout has the same "is this a path or a revision" ambiguity
> to resolve. But rather than be explicit that you might have meant "114"
> as a tree, the error message assumes you meant a path. That might be
> worth improving, similar to the above example.
>
> Again, you can disambiguate with:
>
> $ git checkout -- 1147
> error: pathspec '1147' did not match any file(s) known to git.
>
> $ git checkout 1147 --
> fatal: reference is not a tree: 1147
>
>> $ git checkout 1147
>> fatal: reference is not a tree: 1147
Yes, I understand this. This was a typo and it was ambiguous. But
shouldn't we tell the user the same thing when encountering the same
failure?
> In this case the name does resolve to an object, so we try to use it as
> such (even though we later find that it is useless for the operation).
> We _could_ realize that it is not a tree and disambiguate to:
>
> $ git checkout -- 1147
>
> but the current rule is at least consistent and simple.
>
>> $ git checkout 1146
>> error: short SHA1 1146 is ambiguous.
>> error: pathspec '1146' did not match any file(s) known to git.
>
> The sha1 is ambiguous, and therefore it does not resolve to anything. So
> you get the same case as "git checkout 1147", but with the extra
> ambiguity warning.
This one I like. I only included it to demonstrate the different
error messages git produces.
>> $ git merge 114
>> fatal: '114' does not point to a commit
>
> It might be nice for this error message to be split into two cases:
>
> 1. the name does not resolve _at all_ (i.e., you made a typo)
>
> 2. the name does resolve to something, but it is not a commit
>
> In the latter case, we actually do get an extra error message from
> elsewhere in the code:
>
>> $ git merge 1147
>> error: 1147: expected commit type, but the object dereferences to blob type
>> fatal: '1147' does not point to a commit
>
> But in case 1, it's not clear which is which (maybe even rewording it as
> "114 cannot be resolved as a commit" would be less confusing).
>
>> $ git cherry-pick 114
>> fatal: ambiguous argument '114': unknown revision or path not in the
>> working tree.
>> Use '--' to separate paths from revisions
>> [...]
>> $ git cherry-pick 1147
>> fatal: BUG: expected exactly one commit from walk
>
> This is the "does not resolve" versus "is not actually a commit". In the
> first case, though, I wonder if the error message is accurate. I'm not
> sure if you can do "git cherry-pick <rev> -- <paths>", so the error
> message is misleading
Good point. I missed that one completely.
> (if anything, I would expect it to limit the
> revision walk, but trying "git cherry-pick HEAD -- 114" seems to still
> complain about the absence of 114).
>
>> [more examples]
>
> These are all variants that hopefully make sense in light of the
> explanations above.
>
>> I can understand some of the inconsistent error reporting (checkout
>> may expect filenames, but cherry-pick and merge do not). But this
>> seems too varied to me.
>>
>> And the first two look like bugs.
>>
>> Any comments or suggestions?
>
> I think the outcomes are all working as intended, but the error messages
> could stand to be improved.
Yes, I agree. I only meant to complain about the error messages, not
the results. Thanks for the discussion. I'll try to look for where
these come from and see if they can be improved within reason.
Phil
^ permalink raw reply
* Re: [PATCH] remote-curl: Fix push status report when all branches fail
From: Shawn Pearce @ 2012-02-22 15:22 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20120222101302.GA11606@sigill.intra.peff.net>
On Wed, Feb 22, 2012 at 02:13, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 20, 2012 at 09:03:31AM -0800, Shawn O. Pearce wrote:
> This hunk is causing intermittent failures of t5541 for me, especially
> when the system is under heavy load (e.g., make -j32 test).
...
> @@ -220,15 +221,21 @@ static struct child_process *get_helper(struct transport *transport)
> static int disconnect_helper(struct transport *transport)
> {
> struct helper_data *data = transport->data;
> - struct strbuf buf = STRBUF_INIT;
> int res = 0;
>
> if (data->helper) {
> if (debug)
> fprintf(stderr, "Debug: Disconnecting.\n");
> if (!data->no_disconnect_req) {
> - strbuf_addf(&buf, "\n");
> - sendline(data, &buf);
> + /*
> + * Ignore write errors; there's nothing we can do,
> + * since we're about to close the pipe anyway. And the
> + * most likely error is EPIPE due to the helper dying
> + * to report an error itself.
> + */
> + sigchain_push(SIGPIPE, SIG_IGN);
> + xwrite(data->helper->in, "\n", 1);
> + sigchain_pop(SIGPIPE);
> }
> close(data->helper->in);
> close(data->helper->out);
>
> which makes the t5541 failures go away for me. What do you think?
This sounds right to me. Its unfortunate that we missed the error
status output when we built the remote helper protocol, but your patch
above might be the best we can do now.
Eh, well, actually we could have the helper advertise a new capability
that can be enabled to return exit status. That is a much bigger
change, and even if we do it for remote-curl (since that is in tree
and easy to update) we still need your patch for the same race
condition for out of tree helpers (which Google actually has so I care
about out of tree helpers too).
^ permalink raw reply
* Re: how do you review auto-resolved files
From: Zbigniew Jędrzejewski-Szmek @ 2012-02-22 15:24 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Neal Kreitzinger
In-Reply-To: <7vhayjga0a.fsf@alter.siamese.dyndns.org>
On 02/21/2012 10:19 PM, Junio C Hamano wrote:
> Imagine that you are the maintainer of the mainline and are reviewing the
> work made on a side branch that you just merged, but pretend that the
> contribution came as a patch instead. How would you assess the damage to
> your mainline?
>
> You would use "git show --first-parent $commit" for that.
Hi,
it seems that git show --first-parent is not documented in the man page.
This option is only documented for rev-list and log. I think that
- this example should land in Examples in git-show.txt
- --first-parent should be documented in git-show.txt because it (at
least to me, but I guess that for other people also) it isn't
immediately obvious that it means to _diff_ with the first parent.
--
Zbyszek
^ permalink raw reply
* Re: git-svn show-externals and svn version
From: Nikolaus Demmel @ 2012-02-22 15:27 UTC (permalink / raw)
To: git
In-Reply-To: <994D6101-DD43-4CD9-BB96-34807E3087C4@nikolaus-demmel.de>
Hi,
I feel a bit like I am talking to myself, but I see from the high
traffic on this list that people are busy doing great things :-). I will
write anyway in case someone interested in git-svn listens.
Is there an appropriate place to file these kinds of feature/enhancement
requests?
So I've investigated the matter a bit further. Turns out in the
subversion SWIG language bindings there is an API function that parses
svn:externals definitions for you. See [1] for a recent (minimal) change
to make this function available in python. I guess supporting Perl
requires equally minimal changes. I haven't attempted it myself since I
know neither Perl nor SWIG.
How could this be used in git-svn show-externals? As layed out before, I
believe that the current output for the svn1.5 syntax is inherently
broken and we should not worry about backwards compatibility for
that. To maintain backwards compatibility with the output for the old
format and to give a canonical, easy to parse, output for any external
definition, I suggest sticking to the current format, just inserting the
parsed definition at the appropriate place with relative URLs completely
resolved to absolute ones.
The pre-svn1.5 syntax for external definitions was:
LOCAL-PATH [-r REVISION] ABSOLUTE-URL
The output for show-externals was thus (note that there is no parsing of
the external definition going on yet):
DIRECTORY-PREFIX/LOCAL-PATH [-rREVISION] ABSOLUTE-URL
The DIRECTORY-PREFIX was added because show-externals shows the external
definitions for all subdirectories recursively. With this prefix, every
line can be processed on its own. I suggest extending this output to:
DIRECTORY-PREFIX/LOCAL-PATH [-rREVISION] ABSOLUTE-URL[@PEG-REV]
Again, as mentioned above, show-externals should parse the definitions
and resolve relative URLs. Any lines that the svn API call cannot parse
should be completely ommited (e.g. commented lines and empty lines).
As I understand it show-externals is intended primarily for scripts for
further processing. With this extension existing scripts for the old
syntax should keep working also long as they don't feature
peg-revisions. With relative URLs resolved and a standard ordering old
and new syntax cannot be distinguished in terms of show-externals output
(except when there are peg-revsion are there).
Thoughts, comments, opinions?
Cheers,
Nikolaus
[1] http://thread.gmane.org/gmane.comp.version-control.subversion.user/109033/focus=109036
Am 21.02.2012 um 12:14 schrieb Nikolaus Demmel:
> Hi,
>
> as a followup just another example of when the current show-externals gives a flaky output, namely when the line in the external definition is commented.
>
>
> $ git svn show-externals
> [...]
> # /src/
> /src/#https://codex.cs.bham.ac.uk/svn/nah/cogx/code/subarchitectures/vision.sa/tags/matlab-cosy-2008 matlab_cosy_2008
> /src/#https://codex.cs.bham.ac.uk/svn/nah/cogx/code/subarchitectures/vision.sa/tags/matlab-review-2009 matlab_review_2009
>
>
> Regards,
> Nikolaus
>
>
> Am 19.02.2012 um 19:53 schrieb Nikolaus Demmel:
>
>> Hi,
>>
>> I am currently investigating getting support for svn externals in git-svn (you might have noticed my other mails).
>>
>> It turns out that there are quite a few scripts floating around that use the output of show-externals and then try to pull these externals with git-svn into independent repositories and add the folders as submodules to the root repository.
>>
>> However, none of them work for me, and the primary reason AFAICT is that they were written for the pre svn1.5 format of svn:externals. From 1.5 svn supports a new format of svn:externals, which changes the order of revision, repository-url, and local folder, and also adds the posibility to add relative urls, peg-revisions, etc [1].
>>
>> On top that it seems to me that the output of show-externals was purely designed for the old format. For example, if you compare the output of "git svn show-externals" with "git svn propget svn:externals" in an example repository using the new format [2], you find that the in the show-externals output the prepended "/" and "/instantiations/" at the beginning of each line does not make sense. If the target url (all relative with the ^ syntax in this case) and the sub-folder were swapped in order, as of pre 1.5 svn, it would make much more sense. Also apparently the code for show-externals was added in 2007 and not changed since, whereas svn 1.5 was released in 2008.
>>
>> What I am not completely clear about is, whether svn 1.5 and later enforces the new syntax, or whether it just adds the new syntax and still has to support the old syntax (which could be distinguished, I guess, by checking of the last part on an entry is an absolute URL instead of a subfolder). Also, I'm not sure if the format depends on the version of the svn-server or the client. I would assume you can check out a repository hosted with svn 1.4 with a 1.5 client. Does the client process the svn:externals and present it in the new format, or is this the text string just taken from the server unaltered (I have not much knowledge of how svn actually works internally)?
>>
>> Another question is whether the perl svn bindings present the svn:externals in some parsed, standard format, or do they just give you the raw text string?
>>
>> In order to make show-externals more useful with the svn 1.5 and later syntax, one would maybe need to check the underlying svn version. I guess it is also quite important to retain backwards compatibility, such that users of externals with the old syntax would still get the same output as before.
>>
>> I would suggest that the show-externals output should be as close as possible to the svn:externals syntax, possibly adapting the subfolder path for nested folders. However here the recursive display of externals for subfolders becomes a bit more tricky, since the URL can also be relative to the subfolder as of the new syntax. Maybe the easiest way to deal with the new syntax in show-externals would be to have each line like it is in the svn-properties, but add a space separated relative path to the corresponding subfolder at the beginning. A tool that uses this is then responsible for making sure the relative URLs are resolved correctly.
>>
>> To sum up, given that all the questions I have are answered like I think is most likely, it would boil down to changing the output of show-externals for svn 1.5 and later just slightly, namely by inserting an additional space between the prepended subfolder and the actual svn:externals definition in each line.
>>
>> Any thoughts and/or answers?
>>
>> Cheers,
>> Nikolaus
>>
>>
>> [1] http://svnbook.red-bean.com/en/1.7/svn.advanced.externals.html
>> [2] http://paste.lisp.org/display/127858--
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [FYI] very large text files and their problems.
From: Ian Kumlien @ 2012-02-22 15:49 UTC (permalink / raw)
To: git
Hi,
We just saw a interesting issue, git compressed a ~3.4 gb project to ~57
mb. But when we tried to clone it on a big machine we got:
fatal: Out of memory, malloc failed (tried to allocate
18446744072724798634 bytes)
This is already fixed in the 1.7.10 mainline - but it also seems like
git needs to have atleast the same ammount of memory as the largest
file free... Couldn't this be worked around?
On a (32 bit) machine with 4GB memory - results in:
fatal: Out of memory, malloc failed (tried to allocate 3310214313 bytes)
(and i see how this could be a problem, but couldn't it be mitigated? or
is it bydesign and intended behaviour?)
I'm not subscribed to please keep me in CC.
/Ian Kumlien
^ permalink raw reply
* Re: [FYI] very large text files and their problems.
From: Nguyen Thai Ngoc Duy @ 2012-02-22 16:18 UTC (permalink / raw)
To: Ian Kumlien; +Cc: git
In-Reply-To: <20120222154926.GC11202@pomac.netswarm.net>
On Wed, Feb 22, 2012 at 10:49 PM, Ian Kumlien <pomac@vapor.com> wrote:
> Hi,
>
> We just saw a interesting issue, git compressed a ~3.4 gb project to ~57 mb.
How big are those files? How many of them? How often do they change?
> But when we tried to clone it on a big machine we got:
>
> fatal: Out of memory, malloc failed (tried to allocate
> 18446744072724798634 bytes)
>
> This is already fixed in the 1.7.10 mainline - but it also seems like
Does 1.7.9 have this problem?
> git needs to have atleast the same ammount of memory as the largest
> file free... Couldn't this be worked around?
>
> On a (32 bit) machine with 4GB memory - results in:
> fatal: Out of memory, malloc failed (tried to allocate 3310214313 bytes)
>
> (and i see how this could be a problem, but couldn't it be mitigated? or
> is it bydesign and intended behaviour?)
I think that it's delta resolving that hogs all your memory. If your
files are smaller than 512M, try lower core.bigFileThreshold. The
topic jc/split-blob, which stores a big file are several smaller
pieces, might solve your problem. Unfortunately the topic is not
complete yet.
--
Duy
^ permalink raw reply
* Re: [PATCH] Improve http proxy support
From: Thomas Rast @ 2012-02-22 16:43 UTC (permalink / raw)
To: Nelson Benítez León; +Cc: Matthieu Moy, git
In-Reply-To: <vpqd397x8fc.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Nelson Benítez León <nbenitezl@gmail.com> writes:
>
>> Hi, my initial motivation for this patch was to add NTLM proxy
>> authentication [...]
>
> That sounds interesting, but please read Documentation/SubmittingPatches
> in Git's tree. The formatting of your email is wrong (giving more work
> for your maintainer) and you need to sign-off your patch to allow your
> code to be legally included.
Judging from the message, it also conflates three changes into one
patch. Don't do that.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Problems with unrecognized headers in git bundles
From: Jannis Pohlmann @ 2012-02-22 16:05 UTC (permalink / raw)
To: git
Hi,
creating bundles from some repositories seems to lead to bundles with
incorrectly formatted headers, at least with git >= 1.7.2. When cloning
from such bundles, git prints the following error/warning:
$ git clone perl-clone.bundle perl-clone
Cloning into 'perl-clone'...
warning: unrecognized header: --work around mangled archname on...
This can be reproduced easily with git from any version >= 1.7.2 or from
master, using the following steps:
git clone git://perl5.git.perl.org/perl.git perl
GIT_DIR=perl/.git git bundle create perl-clone.bundle --all
git clone perl-clone.bundle perl-clone
The content of the bundle is:
# v2 git bundle
-- work around mangled archname on win32 while finding...
39ec54a59ce332fc44e553f4e5eeceef88e8369e refs/heads/blead
39ec54a59ce332fc44e553f4e5eeceef88e8369e refs/remotes/origin/HEAD
...
The "--work around mangled archname..." line is rather long, so I've
omitted most of it. What it contains is a series of commit messages all
combined into a single line. It appears that this line is the problem
because it's neither a comment (like '#v2 git bundle') nor a SHA1
followed by a ref name.
Note that this problem does not occur with all repositories. A bundle
created from a test repository with a single text file and just one
commit does not have this problem.
Note also that "git clone <bundle> <dest>" does not fail with corrupted
bundles if the git version is something like 1.7.2 or 1.7.7.6. Those
version print the above warning but still succeed in cloning. "git
clone" from master however treats this as an error and fails.
Without any knowledge of how git works internally, I would assume that
this is either a bug in how bundles are created, or a hint at a slightly
broken perl repository (other's have the same thing though, perhaps it
is because of a conversion from another SCM software to git in the
past). Does this sound reasonable?
Is there a way to work around this or fix it properly? I'm not sure what
lead to the decision to no longer ignore unrecognized headers in git
master, would it be sensible to revert this change if nothing can be
done to solve this during bundle creation?
- Jannis
^ permalink raw reply
* Re: [PATCH 4/5] xdiff: introduce XDF_IGNORE_CASE
From: Jakub Narebski @ 2012-02-22 18:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1329704188-9955-5-git-send-email-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> writes:
> Teach the hash function and per-line comparison logic to compare lines
> while ignoring the differences in case. It is not an ignore-whitespace
> option but still needs to trigger the inexact match logic, and that is
> why the previous step introduced XDF_INEXACT_MATCH mask.
Nb. how it compares with ignore case in filesystem paths?
> Assign the 7th bit for this option, and move the bits to select diff
> algorithms out of the way in order to leave room for a few bits to add
> more variants of ignore-whitespace, such as --ignore-tab-expansion, if
> somebody else is inclined to do so later.
Or do a proper Unicode sorting / collation algorithm, with different
levels
(4.3 Form a sort key for each string, UTS #10.):
Level 1: alphabetic ordering
Level 2: diacritic ordering
Level 3: case ordering
Level 4: tie-breaking (e.g. in the case when variable is 'shifted')
> We would still need to teach the front-end to flip this bit, for this
> change to be any useful.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> +static inline int match_a_byte(char ch1, char ch2, long flags)
> +{
> + if (ch1 == ch2)
> + return 1;
> + if (!(flags & XDF_IGNORE_CASE) || ((ch1 | ch2) & 0x80))
> + return 0;
> + if (isupper(ch1))
> + ch1 = tolower(ch1);
> + if (isupper(ch2))
> + ch2 = tolower(ch2);
> + return (ch1 == ch2);
> +}
<del>
Wouldn't a better solution be a collate algorithm rather than changing
a sorting function? Or is it a performance hack on typical body of
text under version control (mainly lowercase)?
</del>
"(libc.info)Collation Fuctions" says:
The functions `strcoll' and `wcscoll' perform this translation
implicitly, in order to do one comparison. By contrast, `strxfrm' and
`wcsxfrm' perform the mapping explicitly. If you are making multiple
comparisons using the same string or set of strings, it is likely to be
more efficient to use `strxfrm' or `wcsxfrm' to transform all the
strings just once, and subsequently compare the transformed strings
with `strcmp' or `wcscmp'.
The function match_a_byte (memcoll?) defined here is similar to strcoll;
do we compare single line with more than one other line?
> +static inline unsigned long hash_a_byte(const char ch_, long flags)
> +{
> + unsigned long ch = ch_ & 0xFF;
> + if ((flags & XDF_IGNORE_CASE) && !(ch & 0x80) && isupper(ch))
> + ch = tolower(ch);
> + return ch;
> +}
> +
Hmmm... hash_a_byte (memxfrm?) is similar to strxfrm, so you do use
one or the other...
--
Jakub Narebski
^ permalink raw reply
* Re: [FYI] very large text files and their problems.
From: Ian Kumlien @ 2012-02-22 18:39 UTC (permalink / raw)
To: git; +Cc: pclouds
[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]
Seems like i ruined my dovecot config in a recent upgrade - which also
affected my mail... =/
Anyway, it's all fixed now.
from: Nguyen Thai Ngoc Duy <pclouds () gmail ! com>
> On Wed, Feb 22, 2012 at 10:49 PM, Ian Kumlien <pomac@vapor.com> wrote:
> > Hi,
> >
> > We just saw a interesting issue, git compressed a ~3.4 gb project to
> ~57 mb.
>
> How big are those files? How many of them? How often do they change?
This is the initial check in, one of the files is a 3.3 gb text file.
> > But when we tried to clone it on a big machine we got:
> >
> > fatal: Out of memory, malloc failed (tried to allocate
> > 18446744072724798634 bytes)
> >
> > This is already fixed in the 1.7.10 mainline - but it also seems
> like
>
> Does 1.7.9 have this problem?
I've tested with 1.7.9.1, haven't downgraded to test with 1.7.9...
> > git needs to have atleast the same ammount of memory as the largest
> > file free... Couldn't this be worked around?
> >
> > On a (32 bit) machine with 4GB memory - results in:
> > fatal: Out of memory, malloc failed (tried to allocate 3310214313
> bytes)
> >
> > (and i see how this could be a problem, but couldn't it be
> mitigated? or
> > is it bydesign and intended behaviour?)
>
> I think that it's delta resolving that hogs all your memory. If your
> files are smaller than 512M, try lower core.bigFileThreshold. The
> topic jc/split-blob, which stores a big file are several smaller
> pieces, might solve your problem. Unfortunately the topic is not
> complete yet.
the problem here is that there is one file that is exactly: 3310214313
bytes, so it should all be one "blob".
split-blob would be really interesting for several reasons though =)
> --
> Duy
> --
--
Ian Kumlien -- http://demius.net || http://pomac.netswarm.net
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox