Git development
 help / color / mirror / Atom feed
* [PATCH 03/11] git-p4: clone does not use --git-dir
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Complain if --git-dir is given during a clone.  It has no
effect.  Only --destination and --bare can change where the newly
cloned git dir will be.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |    3 ++-
 t/t9806-git-p4-options.sh  |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletions(-)
 create mode 100755 t/t9806-git-p4-options.sh

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 5949803..dafc4a2 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -2335,7 +2335,8 @@ def main():
     args = sys.argv[2:]
 
     if len(options) > 0:
-        options.append(optparse.make_option("--git-dir", dest="gitdir"))
+        if cmd.needsGit:
+            options.append(optparse.make_option("--git-dir", dest="gitdir"))
 
         parser = optparse.OptionParser(cmd.usage.replace("%prog", "%prog " + cmdName),
                                        options,
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
new file mode 100755
index 0000000..8044fb0
--- /dev/null
+++ b/t/t9806-git-p4-options.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='git-p4 options'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+	start_p4d
+'
+
+test_expect_success 'init depot' '
+	(
+		cd "$cli" &&
+		echo file1 >file1 &&
+		p4 add file1 &&
+		p4 submit -d "change 1" &&
+		echo file2 >file2 &&
+		p4 add file2 &&
+		p4 submit -d "change 2" &&
+		echo file3 >file3 &&
+		p4 add file3 &&
+		p4 submit -d "change 3"
+	)
+'
+
+test_expect_success 'clone no --git-dir' '
+	test_must_fail "$GITP4" clone --git-dir=xx //depot
+'
+
+test_expect_success 'kill p4d' '
+	kill_p4d
+'
+
+test_done
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 04/11] git-p4: test cloning with two dirs, clarify doc
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Document how git-p4 currently works when specifying multiple
depot paths:

1.  No branches or directories are named.

2.  Conflicting files are silently ignored---the last change
    wins.

2.  Option --destination is required, else the last path is construed
    to be a directory.

3.  Revision specifiers must be the same on all paths for them to
    take effect.

Test this behavior.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt |   11 +++++++-
 t/t9800-git-p4-basic.sh  |   60 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index c981407..c15b3b7 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -276,8 +276,15 @@ p4 revision specifier on the end:
 "//depot/my/project@1,6"::
     Import only changes 1 through 6.
 
-"//depot/proj1 //depot/proj2@all"::
-    Import all changes from both named depot paths.
+"//depot/proj1@all //depot/proj2@all"::
+    Import all changes from both named depot paths into a single
+    repository.  Only files below these directories are included.
+    There is not a subdirectory in git for each "proj1" and "proj2".
+    You must use the '--destination' option when specifying more
+    than one depot path.  The revision specifier must be specified
+    identically on each depot path.  If there are files in the
+    depot paths with the same name, the path with the most recently
+    updated version of the file is the one that appears in git.
 
 See 'p4 help revisions' for the full syntax of p4 revision specifiers.
 
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 272de3f..04ee20e 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -65,6 +65,66 @@ test_expect_success 'git-p4 sync new branch' '
 	)
 '
 
+test_expect_success 'clone two dirs' '
+	(
+		cd "$cli" &&
+		mkdir sub1 sub2 &&
+		echo sub1/f1 >sub1/f1 &&
+		echo sub2/f2 >sub2/f2 &&
+		p4 add sub1/f1 &&
+		p4 submit -d "sub1/f1" &&
+		p4 add sub2/f2 &&
+		p4 submit -d "sub2/f2"
+	) &&
+	"$GITP4" clone --dest="$git" //depot/sub1 //depot/sub2 &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git ls-files >lines &&
+		test_line_count = 2 lines &&
+		git log --oneline p4/master >lines &&
+		test_line_count = 1 lines
+	)
+'
+
+test_expect_success 'clone two dirs, @all' '
+	(
+		cd "$cli" &&
+		echo sub1/f3 >sub1/f3 &&
+		p4 add sub1/f3 &&
+		p4 submit -d "sub1/f3"
+	) &&
+	"$GITP4" clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git ls-files >lines &&
+		test_line_count = 3 lines &&
+		git log --oneline p4/master >lines &&
+		test_line_count = 3 lines
+	)
+'
+
+test_expect_success 'clone two dirs, @all, conflicting files' '
+	(
+		cd "$cli" &&
+		echo sub2/f3 >sub2/f3 &&
+		p4 add sub2/f3 &&
+		p4 submit -d "sub2/f3"
+	) &&
+	"$GITP4" clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git ls-files >lines &&
+		test_line_count = 3 lines &&
+		git log --oneline p4/master >lines &&
+		test_line_count = 4 lines &&
+		echo sub2/f3 >expected &&
+		test_cmp expected f3
+	)
+'
+
 test_expect_success 'exit when p4 fails to produce marshaled output' '
 	badp4dir="$TRASH_DIRECTORY/badp4dir" &&
 	mkdir "$badp4dir" &&
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 05/11] git-p4: document and test clone --branch
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Clone with --branch will not checkout HEAD, unless the branch
happens to be called the default refs/remotes/p4/master.  The
--branch option is most useful with sync; give an example of
that.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt  |   10 +++++++++-
 t/t9806-git-p4-options.sh |   11 +++++++++++
 2 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index c15b3b7..a5d3d81 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -178,7 +178,15 @@ subsequent 'sync' operations.
 --branch <branch>::
 	Import changes into given branch.  If the branch starts with
 	'refs/', it will be used as is, otherwise the path 'refs/heads/'
-	will be prepended.  The default branch is 'master'.
+	will be prepended.  The default branch is 'master'.  If used
+	with an initial clone, no HEAD will be checked out.
++
+This example imports a new remote "p4/proj2" into an existing
+git repository:
+----
+    $ git init
+    $ git p4 sync --branch=refs/remotes/p4/proj2 //depot/proj2
+----
 
 --detect-branches::
 	Use the branch detection algorithm to find new paths in p4.  It is
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 8044fb0..7e2e45a 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -27,6 +27,17 @@ test_expect_success 'clone no --git-dir' '
 	test_must_fail "$GITP4" clone --git-dir=xx //depot
 '
 
+test_expect_success 'clone --branch' '
+	"$GITP4" clone --branch=refs/remotes/p4/sb --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git ls-files >files &&
+		test_line_count = 0 files &&
+		test_path_is_file .git/refs/remotes/p4/sb
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 06/11] git-p4: honor --changesfile option and test
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

When an explicit list of changes is given, it makes no sense to
use @all or @3,5 or any of the other p4 revision specifiers.
Make the code notice when this happens, instead of just ignoring
--changesfile.  Test it.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |   16 +++++++++++++++-
 t/t9806-git-p4-options.sh  |   23 +++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index dafc4a2..d0a9b0d 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -2024,6 +2024,17 @@ class P4Sync(Command, P4UserMap):
         revision = ""
         self.users = {}
 
+        # Make sure no revision specifiers are used when --changesfile
+        # is specified.
+        bad_changesfile = False
+        if len(self.changesFile) > 0:
+            for p in self.depotPaths:
+                if p.find("@") >= 0 or p.find("#") >= 0:
+                    bad_changesfile = True
+                    break
+        if bad_changesfile:
+            die("Option --changesfile is incompatible with revision specifiers")
+
         newPaths = []
         for p in self.depotPaths:
             if p.find("@") != -1:
@@ -2040,7 +2051,10 @@ class P4Sync(Command, P4UserMap):
                 revision = p[hashIdx:]
                 p = p[:hashIdx]
             elif self.previousDepotPaths == []:
-                revision = "#head"
+                # pay attention to changesfile, if given, else import
+                # the entire p4 tree at the head revision
+                if len(self.changesFile) == 0:
+                    revision = "#head"
 
             p = re.sub ("\.\.\.$", "", p)
             if not p.endswith("/"):
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 7e2e45a..7a1dba6 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -38,6 +38,29 @@ test_expect_success 'clone --branch' '
 	)
 '
 
+test_expect_success 'clone --changesfile' '
+	cf="$TRASH_DIRECTORY/cf" &&
+	test_when_finished "rm \"$cf\"" &&
+	printf "1\n3\n" >"$cf" &&
+	"$GITP4" clone --changesfile="$cf" --dest="$git" //depot &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git log --oneline p4/master >lines &&
+		test_line_count = 2 lines
+		test_path_is_file file1 &&
+		test_path_is_missing file2 &&
+		test_path_is_file file3
+	)
+'
+
+test_expect_success 'clone --changesfile, @all' '
+	cf="$TRASH_DIRECTORY/cf" &&
+	test_when_finished "rm \"$cf\"" &&
+	printf "1\n3\n" >"$cf" &&
+	test_must_fail "$GITP4" clone --changesfile="$cf" --dest="$git" //depot@all
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 07/11] git-p4: document and test --import-local
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Explain that it is needed on future syncs to find p4 branches
in refs/heads.  Test this behavior.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt  |    4 +++-
 t/t9806-git-p4-options.sh |   22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index a5d3d81..2885b82 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -211,7 +211,9 @@ git repository:
 	By default, p4 branches are stored in 'refs/remotes/p4/',
 	where they will be treated as remote-tracking branches by
 	linkgit:git-branch[1] and other commands.  This option instead
-	puts p4 branches in 'refs/heads/p4/'.
+	puts p4 branches in 'refs/heads/p4/'.  Note that future
+	sync operations must specify '--import-local' as well so that
+	they can find the p4 branches in refs/heads.
 
 --max-changes <n>::
 	Limit the number of imported changes to 'n'.  Useful to
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 7a1dba6..6770326 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -61,6 +61,28 @@ test_expect_success 'clone --changesfile, @all' '
 	test_must_fail "$GITP4" clone --changesfile="$cf" --dest="$git" //depot@all
 '
 
+# imports both master and p4/master in refs/heads
+# requires --import-local on sync to find p4 refs/heads
+# does not update master on sync, just p4/master
+test_expect_success 'clone/sync --import-local' '
+	"$GITP4" clone --import-local --dest="$git" //depot@1,2 &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git log --oneline refs/heads/master >lines &&
+		test_line_count = 2 lines &&
+		git log --oneline refs/heads/p4/master >lines &&
+		test_line_count = 2 lines &&
+		test_must_fail "$GITP4" sync &&
+
+		"$GITP4" sync --import-local &&
+		git log --oneline refs/heads/master >lines &&
+		test_line_count = 2 lines &&
+		git log --oneline refs/heads/p4/master >lines &&
+		test_line_count = 3 lines
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 08/11] git-p4: test --max-changes
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>


Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9806-git-p4-options.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 6770326..cc0fd26 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -83,6 +83,16 @@ test_expect_success 'clone/sync --import-local' '
 	)
 '
 
+test_expect_success 'clone --max-changes' '
+	"$GITP4" clone --dest="$git" --max-changes 2 //depot@all &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		git log --oneline refs/heads/master >lines &&
+		test_line_count = 2 lines
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 09/11] git-p4: test --keep-path
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Make sure it leaves the path, below //depot, in git.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 t/t9806-git-p4-options.sh |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index cc0fd26..6b288ac 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -93,6 +93,30 @@ test_expect_success 'clone --max-changes' '
 	)
 '
 
+test_expect_success 'clone --keep-path' '
+	(
+		cd "$cli" &&
+		mkdir -p sub/dir &&
+		echo f4 >sub/dir/f4 &&
+		p4 add sub/dir/f4 &&
+		p4 submit -d "change 4"
+	) &&
+	"$GITP4" clone --dest="$git" --keep-path //depot/sub/dir@all &&
+	test_when_finished cleanup_git &&
+	(
+		cd "$git" &&
+		test_path_is_missing f4 &&
+		test_path_is_file sub/dir/f4
+	) &&
+	cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot/sub/dir@all &&
+	(
+		cd "$git" &&
+		test_path_is_file f4 &&
+		test_path_is_missing sub/dir/f4
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 10/11] git-p4: test and document --use-client-spec
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

The depot path is required, even with this option.  Make sure
git-p4 fails and exits with non-zero.

Contents in the specified depot path will be rearranged according
to the client spec.  Test this and add a note in the docs.

Leave an XXX suggesting that this is somewhat confusing behavior
that might be good to fix later.

Function stripRepoPath() looks at self.useClientSpec.  Make sure
this is set both for command-line option --use-client-spec and
for configuration variable git-p4.useClientSpec.  Test this.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt   |    5 +++-
 contrib/fast-import/git-p4 |    6 ++++-
 t/t9806-git-p4-options.sh  |   46 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 2885b82..3092571 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -232,7 +232,10 @@ git repository:
 	Use a client spec to find the list of interesting files in p4.
 	The client spec is discovered using 'p4 client -o' which checks
 	the 'P4CLIENT' environment variable and returns a mapping of
-	depot files to workspace files.
+	depot files to workspace files.  Note that a depot path is
+	still required, but files found in the path that match in
+	the client spec view will be laid out according to the client
+	spec.
 
 Clone options
 ~~~~~~~~~~~~~
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index d0a9b0d..5420bf1 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1951,7 +1951,10 @@ class P4Sync(Command, P4UserMap):
             if not gitBranchExists(self.refPrefix + "HEAD") and self.importIntoRemotes and gitBranchExists(self.branch):
                 system("git symbolic-ref %sHEAD %s" % (self.refPrefix, self.branch))
 
-        if self.useClientSpec or gitConfig("git-p4.useclientspec") == "true":
+        if not self.useClientSpec:
+            if gitConfig("git-p4.useclientspec", "--bool") == "true":
+                self.useClientSpec = True
+        if self.useClientSpec:
             self.getClientSpec()
 
         # TODO: should always look at previous commits,
@@ -2380,6 +2383,7 @@ def main():
 
     if not cmd.run(args):
         parser.print_help()
+        sys.exit(2)
 
 
 if __name__ == '__main__':
diff --git a/t/t9806-git-p4-options.sh b/t/t9806-git-p4-options.sh
index 6b288ac..1f1952a 100755
--- a/t/t9806-git-p4-options.sh
+++ b/t/t9806-git-p4-options.sh
@@ -117,6 +117,52 @@ test_expect_success 'clone --keep-path' '
 	)
 '
 
+# clone --use-client-spec must still specify a depot path
+# if given, it should rearrange files according to client spec
+# when it has view lines that match the depot path
+# XXX: should clone/sync just use the client spec exactly, rather
+# than needing depot paths?
+test_expect_success 'clone --use-client-spec' '
+	(
+		# big usage message
+		exec >/dev/null &&
+		test_must_fail "$GITP4" clone --dest="$git" --use-client-spec
+	) &&
+	cli2="$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
+	) &&
+	P4CLIENT=client2 &&
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" --use-client-spec //depot/... &&
+	(
+		cd "$git" &&
+		test_path_is_file bus/dir/f4 &&
+		test_path_is_file file1
+	) &&
+	cleanup_git &&
+
+	# same thing again, this time with variable instead of option
+	mkdir "$git" &&
+	(
+		cd "$git" &&
+		git init &&
+		git config git-p4.useClientSpec true &&
+		"$GITP4" sync //depot/... &&
+		git checkout -b master p4/master &&
+		test_path_is_file bus/dir/f4 &&
+		test_path_is_file file1
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* [PATCH 11/11] git-p4: document and test submit options
From: Pete Wyckoff @ 2011-12-25  2:07 UTC (permalink / raw)
  To: git
In-Reply-To: <1324778860-4821-1-git-send-email-pw@padd.com>

Clarify there is a -M option, but no -C.  These are both
configurable through variables.

Explain that the allowSubmit variable takes a comma-separated
list of branch names.

Catch earlier an invalid branch name given as an argument to
"git p4 clone".

Test option --origin, variable allowSubmit, and explicit master
branch name.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 Documentation/git-p4.txt   |    8 +++++-
 contrib/fast-import/git-p4 |    7 +++++
 t/t9807-git-p4-submit.sh   |   54 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 3092571..f97b1c5 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -267,7 +267,9 @@ These options can be used to modify 'git p4 submit' behavior.
 
 -M[<n>]::
 	Detect renames.  See linkgit:git-diff[1].  Renames will be
-	represented in p4 using explicit 'move' operations.
+	represented in p4 using explicit 'move' operations.  There
+	is no corresponding option to detect copies, but there are
+	variables for both moves and copies.
 
 --preserve-user::
 	Re-author p4 changes before submitting to p4.  This option
@@ -453,7 +455,9 @@ git-p4.skipSubmitEditCheck::
 git-p4.allowSubmit::
 	By default, any branch can be used as the source for a 'git p4
 	submit' operation.  This configuration variable, if set, permits only
-	the named branches to be used as submit sources.
+	the named branches to be used as submit sources.  Branch names
+	must be the short names (no "refs/heads/"), and should be
+	separated by commas (","), with no spaces.
 
 git-p4.skipUserNameCheck::
 	If the user running 'git p4 submit' does not exist in the p4
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 5420bf1..d3c3ad8 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -362,6 +362,11 @@ def isValidGitDir(path):
 def parseRevision(ref):
     return read_pipe("git rev-parse %s" % ref).strip()
 
+def branchExists(ref):
+    rev = read_pipe(["git", "rev-parse", "-q", "--verify", ref],
+                     ignore_error=True)
+    return len(rev) > 0
+
 def extractLogMessageFromGitCommit(commit):
     logMessage = ""
 
@@ -1089,6 +1094,8 @@ class P4Submit(Command, P4UserMap):
                 die("Detecting current git branch failed!")
         elif len(args) == 1:
             self.master = args[0]
+            if not branchExists(self.master):
+                die("Branch %s does not exist" % self.master)
         else:
             return False
 
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index 2cb724e..b1f61e3 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -31,6 +31,60 @@ test_expect_success 'submit with no client dir' '
 	)
 '
 
+# make two commits, but tell it to apply only from HEAD^
+test_expect_success 'submit --origin' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_commit "file3" &&
+		test_commit "file4" &&
+		git config git-p4.skipSubmitEdit true &&
+		"$GITP4" submit --origin=HEAD^
+	) &&
+	(
+		cd "$cli" &&
+		p4 sync &&
+		test_path_is_missing "file3.t" &&
+		test_path_is_file "file4.t"
+	)
+'
+
+test_expect_success 'submit with allowSubmit' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_commit "file5" &&
+		git config git-p4.skipSubmitEdit true &&
+		git config git-p4.allowSubmit "nobranch" &&
+		test_must_fail "$GITP4" submit &&
+		git config git-p4.allowSubmit "nobranch,master" &&
+		"$GITP4" submit
+	)
+'
+
+test_expect_success 'submit with master branch name from argv' '
+	test_when_finished cleanup_git &&
+	"$GITP4" clone --dest="$git" //depot &&
+	(
+		cd "$git" &&
+		test_commit "file6" &&
+		git config git-p4.skipSubmitEdit true &&
+		test_must_fail "$GITP4" submit nobranch &&
+		git branch otherbranch &&
+		git reset --hard HEAD^ &&
+		test_commit "file7" &&
+		"$GITP4" submit otherbranch
+	) &&
+	(
+		cd "$cli" &&
+		p4 sync &&
+		test_path_is_file "file6.t" &&
+		test_path_is_missing "file7.t"
+	)
+'
+
 test_expect_success 'kill p4d' '
 	kill_p4d
 '
-- 
1.7.8.534.g03ab.dirty

^ permalink raw reply related

* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-25  3:13 UTC (permalink / raw)
  To: Joey Hess; +Cc: git
In-Reply-To: <20111224234212.GA21533@gnu.kitenet.net>

Joey Hess <joey@kitenet.net> writes:

> The post-fetch hook is fed on its stdin all refs that were newly fetched.
> It is not allowed to abort the fetch (or pull), but can modify what
> was fetched or take other actions.
>
> One example use of this hook is to automatically merge certain remote
> branches into a local branch. Another is to update a local cache
> (such as a search index) with the fetched refs. No other hook is run
> near fetch time, except for post-merge, which doesn't always run after a
> fetch, which is why this additional hook is useful.
>
> Signed-off-by: Joey Hess <joey@kitenet.net>

A typical "'git pull' invokes 'git fetch' and then lets 'git merge' (or
'git rebase') integrate the work on the current branch with what was
fetched" sequence goes like this:

 - 'git fetch':
  . grabs the necessary objects from the remote;
  . decides what remote tracking branches are updated to point
    at what objects, and what updates are to be denied;
  . updates remote tracking branches accordingly; and
  . writes $GIT_DIR/FETCH_HEAD to communicate what have been fetched
    and what are to be merged.

 - 'git merge':
  . reads $GIT_DIR/FETCH_HEAD to learn what commits to be merged; and
  . merges the commits to the current branch.

Even though we do not add arbitrary hooks on the client side that could
easily be implemented by wrapping the client side commands (i.e. you could
implement "git myfetch" that runs "git fetch" followed by whatever script
that mucks with the result of the fetch) in general, I can see that it
would be useful to have a hook that can tweak the result of the fetch run
inside of the "git pull", because you cannot tell "git pull" to run "git
myfetch" instead of "git fetch".

Because the sequence of "git fetch" followed by "git merge", both commands
issued by the end user, should be equivalent to "git pull" from an end
user's point of view, the hook must be called from near the end of "git
fetch" if we were to have such a hook that tweaks the result of the fetch
inside "pull". IOW, the implementation, even though logically it belongs
to "pull", has to be inside "fetch", not "pull".

In that sense, I am not fundamentally opposed to the idea of adding a post
fetch hook that allows tweaking of the result.

*HOWEVER*

If we _were_ to sanction the use of the hook to tweak the result, I do not
want to see it implemented as an ad-hoc hack that tells the hook writers
that it is _entirely_ their responsiblity to update the remote tracking
branches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to
maintain consistency between these two places.

A very cursory look at the patch tells me that there are a few problems
with it.  It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD
at all, and hence it does not have any way to affect the result of the
fetch that does not store it to any of our remote tracking branches.

> The #1 point of confusion for git-annex users is the need to run
> "git annex merge" after fetching. That does a union merge of newly
> fetched remote git-annex branches into the local git-annex branch.

That use case sounds like that "git fetch" is called as a first class UI,
which is covered by "git myfetch" (you can call it "git annex fetch")
wrapper approach, the canonical example of a hook that we explicitly do
not want to add. It also does not seem to call for mucking with the result
of the fetch at all.

Perhaps the two concepts should be separated into different hooks?

^ permalink raw reply

* Re: [PATCH v2 3/3] grep: disable threading in all but worktree case
From: Nguyen Thai Ngoc Duy @ 2011-12-25  3:32 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð, Thomas Rast, René Scharfe,
	Eric Herman, git, Junio C Hamano, Fredrik Kuivinen
In-Reply-To: <20111224070715.GA32267@sigill.intra.peff.net>

On Sat, Dec 24, 2011 at 2:07 PM, Jeff King <peff@peff.net> wrote:
> The case where we would most expect the setup cost to be drowned out is
> using a more complex regex, grepping tree objects. There we have a
> baseline of:
>
>  $ time git grep 'a.*c' HEAD >/dev/null
>  real    0m5.684s
>  user    0m5.472s
>  sys     0m0.196s
>
>  $ time git ls-tree --name-only -r HEAD |
>      xargs git grep 'a.*c' HEAD -- >/dev/null
>  real    0m10.906s
>  user    0m10.725s
>  sys     0m0.240s
>
> Here, we still almost double our time. It looks like we don't use the
> same pathspec matching code in this case. But we do waste a lot of extra
> time zlib-inflating the trees in "ls-tree", only to do it separately in
> "grep".

Or you could pass blob SHA-1 to git grep to avoid reinflating trees

$ time git ls-tree -r HEAD|cut -c 13-52|xargs git grep 'a.*c' >/dev/null

Doing it in parallel does not seem to save time for me though.
-- 
Duy

^ permalink raw reply

* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-25  3:50 UTC (permalink / raw)
  To: git
In-Reply-To: <7v4nwpbaxq.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

Junio C Hamano wrote:
> If we _were_ to sanction the use of the hook to tweak the result, I do not
> want to see it implemented as an ad-hoc hack that tells the hook writers
> that it is _entirely_ their responsiblity to update the remote tracking
> branches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to
> maintain consistency between these two places.
> 
> A very cursory look at the patch tells me that there are a few problems
> with it.  It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD
> at all, and hence it does not have any way to affect the result of the
> fetch that does not store it to any of our remote tracking branches.

True, it does not update FETCH_HEAD. I had not considered using the hook
that way.

I suppose that after running the hook, fetch could check each remote
tracking branch for a new value, and only then write to FETCH_HEAD.

> > The #1 point of confusion for git-annex users is the need to run
> > "git annex merge" after fetching. That does a union merge of newly
> > fetched remote git-annex branches into the local git-annex branch.
> 
> That use case sounds like that "git fetch" is called as a first class UI,
> which is covered by "git myfetch" (you can call it "git annex fetch")
> wrapper approach, the canonical example of a hook that we explicitly do
> not want to add. It also does not seem to call for mucking with the result
> of the fetch at all.

Most users are fetching by calling git pull as part of their normal
workflow. I would like to avoid git-annex needing its own special pull
command. For one thing, there can be many programs that use git branches
in similar ways (another one is pristine-tar), and a user shouldn't have
to run multiple wrapped versions of git fetch or pull when using
multiple such programs.

-- 
see shy jo

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* [PATCH] Support wrapping commit messages when you read them
From: Sidney San Martín @ 2011-12-25  5:05 UTC (permalink / raw)
  To: git

Fairly simpleminded line wrapping that makes commit messages
readable if they weren’t wrapped by the committer.

- Use strbuf_add_wrapped_text() to do the dirty work
- Detect simple lists which begin with "+ ", "* ", or "- " and indent
  them appropriately (like this line)
- Print lines which begin with whitespace as-is (e.g. code samples)

Add --wrap[=<width>] and --no-wrap to commands that pretty-print commit
messages, and add log.wrap and log.wrap.width configuration options.

log.wrap defaults to never, and can be set to never/false, auto/true,
or always. If auto, hijack want_color() to decide whether it’s
appropriate to use line wrapping. (This is a little hacky, but as far
as I can tell the conditions for auto color and auto wrapping are the
same. Maybe it would make sense to rename want_color()?)

log.wrap.width defaults to 80.

Signed-off-by: Sidney San Martín <s@sidneysm.com>
---

I hope I’m doing this right!

Consider this a first draft of the new feature I brought up a few weeks ago.


 Documentation/config.txt         |    8 ++++
 Documentation/pretty-options.txt |   14 +++++++
 commit.h                         |    6 +++
 log-tree.c                       |    1 +
 pretty.c                         |   71 +++++++++++++++++++++++++++++++++++++-
 revision.c                       |   10 +++++
 revision.h                       |    3 ++
 7 files changed, 112 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6e63b59..b8c1a81 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1403,6 +1403,14 @@ log.showroot::
 	Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
 	normally hide the root commit will now show it. True by default.
 
+log.wrap::
+	Commit message wrapping. May be set to always, false (or never) or
+	auto (or true), in which case commit messages are only wrapped when
+	the output is to a terminal. Defaults to false. See linkgit:git-log[1].
+
+log.wrap.width::
+	Line width for commit message wrapping. Defaults to 80 characters.
+
 mailmap.file::
 	The location of an augmenting mailmap file. The default
 	mailmap, located in the root of the repository, is loaded
diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 2a3dc86..7601a43 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -10,6 +10,20 @@
 Note: you can specify the default pretty format in the repository
 configuration (see linkgit:git-config[1]).
 
+--wrap[=<width>]::
+	Word-wrap commit messages to the specified width, 80 characters
+	by default. Lines beginning with +, *, or - and a space, or with a
+	number followed by a period and a space, are interpreted as lists
+	and wrapped with a hanging indent. Lines beginning with whitespace
+	(e.g. code samples) are left as-is.
++
+Note: you can specify the default wrapping behavior in the repository
+configuration (see linkgit:git-config[1]).
+
+--no-wrap::
+	Turn off commit message wrapping. This is the default (unless
+	otherwise specified in the repository configuration).
+
 --abbrev-commit::
 	Instead of showing the full 40-byte hexadecimal commit object
 	name, show only a partial prefix.  Non default number of
diff --git a/commit.h b/commit.h
index 4df3978..1321666 100644
--- a/commit.h
+++ b/commit.h
@@ -86,6 +86,12 @@ struct pretty_print_context {
 	int show_notes;
 	struct reflog_walk_info *reflog_info;
 	const char *output_encoding;
+	struct wrap_options {
+		unsigned int
+			width,
+			wrap:1,
+			wrap_given:1;
+	} wrap;
 };
 
 struct userformat_want {
diff --git a/log-tree.c b/log-tree.c
index 319bd31..3dfa944 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -414,6 +414,7 @@ void show_log(struct rev_info *opt)
 
 	opt->loginfo = NULL;
 	ctx.show_notes = opt->show_notes;
+	ctx.wrap = opt->wrap;
 	if (!opt->verbose_header) {
 		graph_show_commit(opt->graph);
 
diff --git a/pretty.c b/pretty.c
index 1580299..133bc53 100644
--- a/pretty.c
+++ b/pretty.c
@@ -23,6 +23,10 @@ static size_t commit_formats_len;
 static size_t commit_formats_alloc;
 static struct cmt_fmt_map *find_commit_format(const char *sought);
 
+static unsigned int wrap_configured = 0;
+static unsigned int wrap = 0;
+static unsigned int wrap_width = 80;
+
 static void save_user_format(struct rev_info *rev, const char *cp, int is_tformat)
 {
 	free(user_format);
@@ -172,6 +176,63 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 	}
 }
 
+static int git_wrap_config(const char *name, const char *value, void *cb)
+{
+	wrap_configured = 1;
+
+	if (prefixcmp(name, "log.wrap"))
+		return 0;
+
+	if (name[8] == '\0') {
+		if (value && !strcmp(value, "always")) {
+			wrap = 1;
+		} else if (value && !strcmp(value, "never")) {
+			wrap = 0;
+		} else if (!value || !strcmp(value, "auto") || git_config_bool(name, value)) {
+			wrap = want_color(GIT_COLOR_AUTO);
+		}
+	} else if (name[8] == '.') {
+		name += 9;
+		if (!strcmp(name, "width")) {
+			wrap_width = git_config_int(name, value);
+		}
+	}
+	return 0;
+}
+
+static unsigned int want_wrap(struct wrap_options options)
+{
+	if(!wrap_configured)
+		git_config(git_wrap_config, NULL);
+	return (options.wrap_given ? options.wrap : wrap);
+}
+static unsigned int get_wrap_width(struct wrap_options options)
+{
+	if(!wrap_configured)
+		git_config(git_wrap_config, NULL);
+	return options.width ? options.width : wrap_width;
+}
+
+static int line_list_prefix(const char *line, int len)
+{
+	unsigned int numberLength = 0;
+	const char *pos = line;
+	if (len < 3) {
+		return 0;
+	} else if ((line[0] == '*' || line[0] == '+' || line[0] == '-') && line[1] == ' ') {
+		return 2;
+	} else {
+		while (pos - line < len && pos[0] >= '0' && pos[0] <= '9'){
+			numberLength++;
+			pos++;
+		}
+		if (numberLength && pos - line + 1 < len && pos[0] == '.' && pos[1] == ' ') {
+			return numberLength + 2;
+		}
+	}
+	return 0;
+}
+
 /*
  * Generic support for pretty-printing the header
  */
@@ -1246,6 +1307,8 @@ void pp_remainder(const struct pretty_print_context *pp,
 		  struct strbuf *sb,
 		  int indent)
 {
+	unsigned int wrap = want_wrap(pp->wrap);
+	unsigned int width = get_wrap_width(pp->wrap);
 	int first = 1;
 	for (;;) {
 		const char *line = *msg_p;
@@ -1268,7 +1331,13 @@ void pp_remainder(const struct pretty_print_context *pp,
 			memset(sb->buf + sb->len, ' ', indent);
 			strbuf_setlen(sb, sb->len + indent);
 		}
-		strbuf_add(sb, line, linelen);
+		if (wrap && linelen && line[0] != ' ' && line[0] != '\t') {
+			struct strbuf wrapped = STRBUF_INIT;
+			strbuf_add(&wrapped, line, linelen);
+			strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + line_list_prefix(line, linelen), width - indent);
+		} else {
+			strbuf_add(sb, line, linelen);
+		}
 		strbuf_addch(sb, '\n');
 	}
 }
diff --git a/revision.c b/revision.c
index 8764dde..ca4b386 100644
--- a/revision.c
+++ b/revision.c
@@ -1465,6 +1465,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->verbose_header = 1;
 		revs->pretty_given = 1;
 		get_commit_format(arg+9, revs);
+	} else if (!strcmp(arg, "--wrap")) {
+		revs->wrap.wrap = 1;
+		revs->wrap.wrap_given = 1;
+	} else if (!prefixcmp(arg, "--wrap=")) {
+		revs->wrap.wrap = 1;
+		revs->wrap.wrap_given = 1;
+		revs->wrap.width = atoi(arg+7);
+	} else if (!prefixcmp(arg, "--no-wrap")) {
+		revs->wrap.wrap = 0;
+		revs->wrap.wrap_given = 1;
 	} else if (!strcmp(arg, "--show-notes") || !strcmp(arg, "--notes")) {
 		revs->show_notes = 1;
 		revs->show_notes_given = 1;
diff --git a/revision.h b/revision.h
index 6aa53d1..f812685 100644
--- a/revision.h
+++ b/revision.h
@@ -117,6 +117,9 @@ struct rev_info {
 			missing_newline:1,
 			date_mode_explicit:1,
 			preserve_subject:1;
+
+	struct wrap_options wrap;
+
 	unsigned int	disable_stdin:1;
 	unsigned int	leak_pending:1;
 
-- 
1.7.8.1

^ permalink raw reply related

* Re: [PATCH] add post-fetch hook
From: Junio C Hamano @ 2011-12-25  9:30 UTC (permalink / raw)
  To: Joey Hess; +Cc: git
In-Reply-To: <20111225035059.GA29852@gnu.kitenet.net>

Joey Hess <joey@kitenet.net> writes:

> Junio C Hamano wrote:
>> If we _were_ to sanction the use of the hook to tweak the result, I do not
>> want to see it implemented as an ad-hoc hack that tells the hook writers
>> that it is _entirely_ their responsiblity to update the remote tracking
>> branches from what it fetched, and also update $GIT_DIR/FETCH_HEAD to
>> maintain consistency between these two places.
>> 
>> A very cursory look at the patch tells me that there are a few problems
>> with it.  It does not seem to affect what will go to $GIT_DIR/FETCH_HEAD
>> at all, and hence it does not have any way to affect the result of the
>> fetch that does not store it to any of our remote tracking branches.
>
> True, it does not update FETCH_HEAD. I had not considered using the hook
> that way.

Perhaps I misread what you wrote in the commit log message then.  I
somehow got an impression that one of the advertised way for the hook to
be used was to lie which commits the remote tracking refs point at by
letting it run "update-ref refs/*/*" and the lie is later picked up by
re-reading them, but I wasn't reading the patch very carefully.

If your use case does not involve updating the remote tracking refs nor
FETCH_HEAD (updating only one and not the other is a no-starter), then we
should explicitly forbid it in the documentation, as allowing updates will
invite inconsistencies.

Although I do not deeply care between such a "trigger to only notify, no
touching" hook and a full-blown "allow hook writers to easily lie about
what happened in the fetch" hook, I was hoping that we would get this
right and useful if we were spending our brain bandwidth on it. I am not
very fond of an easier "trigger to only notify" hook because people are
bound to misuse the interface and try updating the refs anyway, making it
easy to introduce inconsistencies between refs and FETCH_HEAD that will
confuse the later "merge" step.

As hook writers are more prone to write such an incorrect code than people
who implement the mechanism to call hooks on the git-core side, the more
the hook interface helps hook writers to avoid such mistakes, the better.

So if we were to allow the hook to lie what commits were fetched and store
something different from what we fetched in the remote tracking refs, I
think the correct place to do so would be in store_updated_refs(),
immediately before we call check_everything_connected().

 - Feed the contents of the ref_map to the hook. For each entry, the hook
   would get (at least):
   . the object name;
   . the refname at the remote;
   . the refname at the local (which could be empty when we are not
     copying it to any of our local ref); and
   . if the entry is to be used for merge.

 - The hook must read _everything_ from its standard input, and then
   return the
   re-written result in the same format as its input. The hook could
   . update the object name (i.e. re-point the remote tracking ref);
   . update the local refname (i.e. use different remote tracking ref);
   . change "merge" flag between true/false; and/or
   . add or remove entries

 - You read from the hook and replace the ref_map list that is fed to
   check_everything_connected(). This ref_map list is what is used in the
   next for() loop that calls update_local_ref() to update the remote
   tracking ref, records the entry in FETCH_HEAD, and produces the report.

This way, the hook cannot screw up, as what it tells us will consistently
be written by us to where it should go.

^ permalink raw reply

* Re: [PATCH] Support wrapping commit messages when you read them
From: Junio C Hamano @ 2011-12-25  9:57 UTC (permalink / raw)
  To: Sidney San Martín; +Cc: git
In-Reply-To: <8DE6E894-B50D-4F7E-AE18-C10E7E40A550@sidneysm.com>

Sidney San Martín <s@sidneysm.com> writes:

> Fairly simpleminded line wrapping that makes commit messages
> readable if they weren’t wrapped by the committer.

This does not say anything useful, other than "this is a naïve
implementation of message wrapper" and invites "So what?".

The most simple-minded solution is to reject such commits with crappy log
message.

After all, SCM is merely a method to help communication between
developers, and sticking to the common denominator is a proven good way to
make sure everybody involved in the project can use what is recorded in
the repository. This is not limited only to the log message, but equally
applies to filenames (e.g. don't create xt_tcpmss.c and xt_TCPMSS.c in the
same directory if you want your project extractable on case insensitive
filesystems) and even to the sources.

You need to justify the cause a bit better. Why is such a new logic
justified?

> - Use strbuf_add_wrapped_text() to do the dirty work
> - Detect simple lists which begin with "+ ", "* ", or "- " and indent
>   them appropriately (like this line)
> - Print lines which begin with whitespace as-is (e.g. code samples)

I suspect the above would make it more palatable than format=flowed
brought up in earlier discussions, which is unsuitable for nothing but
straight text.

> Add --wrap[=<width>] and --no-wrap to commands that pretty-print commit
> messages, and add log.wrap and log.wrap.width configuration options.

Why do you need two separate options and configurations that look as if
they are independent but in reality not?  If you say "no wrap", there is
no room for you to say "wrap width is 72".

I would expect something like:

 --log-message-wrap, --log-message-wrap=72, --log-message-wrap=no 

with --log-message-wrap=yes as a synonym for --log-message-wrap to give
consistency. The corresponding configuraiton would be log.messageWrap
whose values could be the usual bool-or-int.

> log.wrap defaults to never, and can be set to never/false, auto/true,
> or always. If auto, hijack want_color() to decide whether it’s
> appropriate to use line wrapping. (This is a little hacky, but as far
> as I can tell the conditions for auto color and auto wrapping are the
> same.

Why does coloring have _anything_ to do with line wrapping? Maybe your
personaly preference might be "wrap and color if interactive terminal" but
that is conflating two unrelated concepts. A user may not expect coloring
on a dumb interactive terminal, but wrapping may still be useful.

> log.wrap.width defaults to 80.

This does not deserve a comment as I already rejected the "two
configuration" approach, but do not use three-level names this way. We try
to reserve three-level names only for cases where the second level is used
for an unbound collection (e.g. "remote.$name.url", "branch.$name.merge").
that is user-specified.

^ permalink raw reply

* Re: [PATCH] add post-fetch hook
From: Jakub Narebski @ 2011-12-25 16:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Joey Hess, git
In-Reply-To: <7vsjk99exw.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> [...] if we were to allow the hook to lie what commits were fetched and store
> something different from what we fetched in the remote tracking refs, I
> think the correct place to do so would be in store_updated_refs(),
> immediately before we call check_everything_connected().
> 
>  - Feed the contents of the ref_map to the hook. For each entry, the hook
>    would get (at least):
>    . the object name;
>    . the refname at the remote;
>    . the refname at the local (which could be empty when we are not
>      copying it to any of our local ref); and
>    . if the entry is to be used for merge.
> 
>  - The hook must read _everything_ from its standard input, and then
>    return the
>    re-written result in the same format as its input. The hook could
>    . update the object name (i.e. re-point the remote tracking ref);
>    . update the local refname (i.e. use different remote tracking ref);
>    . change "merge" flag between true/false; and/or
>    . add or remove entries

This is a very nice idea, IMHO, both because it makes it simple to
implement no-op (example) hook by just using "cat", and beause it
makes possible to stack such hooks (e.g. one from git-annex with the
one from pristine-tar etc.).

One thing that needs to be specified is what should happen if the hook
changes "the refname at the remote" part...
 
>  - You read from the hook and replace the ref_map list that is fed to
>    check_everything_connected(). This ref_map list is what is used in the
>    next for() loop that calls update_local_ref() to update the remote
>    tracking ref, records the entry in FETCH_HEAD, and produces the report.
> 
> This way, the hook cannot screw up, as what it tells us will consistently
> be written by us to where it should go.

-- 
Jakub Narebski

^ permalink raw reply

* Re: [RFC PATCH] Allow cloning branches selectively
From: Jakub Narebski @ 2011-12-25 16:28 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1324671199-7074-1-git-send-email-cmn@elego.de>

Carlos Martín Nieto <cmn@elego.de> writes:

> Sometimes it's useful to clone only a subset of branches from a remote
> we're cloning. Teach clone the --fetch option to select which branches
> should get fetched.
> 
> Each --fetch sets up a fetch refspec for that branch. Previously this
> was only possible by initializing a repo and manually setting up the
> config.

I haven't read the code, but I guess it should be possible to share
code with "git remote add", which allows to select which branches to
track, and which branch is to be 'main' on remote.

  git remote add [-t <branch>] [-m <master>] [-f] ...

-- 
Jakub Narebski

^ permalink raw reply

* FETCH_HEAD documentation vs reality
From: Joey Hess @ 2011-12-25 17:39 UTC (permalink / raw)
  To: git


[-- Attachment #1.1: Type: text/plain, Size: 2428 bytes --]

While trying to find some documentation of the format of .git/FETCH_HEAD,
I found this example in git-read-tree.txt, which I think will no longer
work. Probably when this was written, .git/FETCH_HEAD contained only a single
SHA; it's much more complicated now.

        $ JC=`git rev-parse --verify "HEAD^0"`
        $ git checkout-index -f -u -a $JC
        ...
        $ git fetch git://.... linus
        $ LT=`cat .git/FETCH_HEAD`
        ...
        $ git read-tree -m -u `git merge-base $JC $LT` $JC $LT

It's also common for the first line of .git/FETCH_HEAD to be an
arbitrary branch that was fetched (as part of an unqualified "git
pull"), marked not-for-merge. So using "FETCH_HEAD" as a refname will
refer to such a branch unintentionally. There are several places in the
docs that seem to expect FETCH_HEAD to always refer to the one that was
fetched and will be merged (ie, master):

revisions.txt:

	'FETCH_HEAD' records the branch which you fetched from a remote repository
	with your last `git fetch` invocation.

git-pull.txt:

	In its default mode, `git pull` is shorthand for
	`git fetch` followed by `git merge FETCH_HEAD`.

gittutorial.txt:

	alice$ git log -p HEAD..FETCH_HEAD
	$ gitk HEAD..FETCH_HEAD

howto/rebase-from-internal-branch.txt:

	You fetch from upstream, but not merge.
	
	    $ git fetch upstream
	
	This leaves the updated upstream head in .git/FETCH_HEAD but
	does not touch your .git/HEAD nor .git/refs/heads/master.
	You run "git rebase" now.
	
	    $ git rebase FETCH_HEAD master

All this documentation could be changed, or resolve_ref_unsafe in refs.c
could be changed to have a special case parser for .git/FETCH_HEAD,
that finds the first branch that is marked for merge, where it now has
this minor special case for it:

        /* Please note that FETCH_HEAD has a second line containing other data. */
        if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {

Or yet another way to fix it would be to make git fetch always write the
intended FETCH_HEAD first into .git/FETCH_HEAD. (When not in --append mode.)
This seems like perhaps the best fix, although it does mean that if a
fetch is done of only not-for-merge refs, without --append, FETCH_HEAD
will still refer to one of them. 

I've attached a minimal proof-of-concept patch implementing this last
option.

-- 
see shy jo

[-- Attachment #1.2: patch --]
[-- Type: text/plain, Size: 1221 bytes --]

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..e2f2c69 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -376,6 +376,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	struct strbuf note = STRBUF_INIT;
 	const char *what, *kind;
 	struct ref *rm;
+	int top = 1;
 	char *url, *filename = dry_run ? "/dev/null" : git_path("FETCH_HEAD");
 
 	fp = fopen(filename, "a");
@@ -393,6 +394,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		goto abort;
 	}
 
+ write:
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
 
@@ -408,6 +410,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		if (!commit)
 			rm->merge = 0;
 
+		if (top != rm->merge)
+			continue;
+
 		if (!strcmp(rm->name, "HEAD")) {
 			kind = "";
 			what = "";
@@ -474,6 +479,11 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		}
 	}
 
+	if (top) {
+		top = 0;
+		goto write;
+	}
+
 	if (rc & STORE_REF_ERROR_DF_CONFLICT)
 		error(_("some local refs could not be updated; try running\n"
 		      " 'git remote prune %s' to remove any old, conflicting "

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related

* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 17:43 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <m3hb0ofwem.fsf@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Sun, Dec 25, 2011 at 08:28:39AM -0800, Jakub Narebski wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
> > 
> > Each --fetch sets up a fetch refspec for that branch. Previously this
> > was only possible by initializing a repo and manually setting up the
> > config.
> 
> I haven't read the code, but I guess it should be possible to share
> code with "git remote add", which allows to select which branches to
> track, and which branch is to be 'main' on remote.
> 
>   git remote add [-t <branch>] [-m <master>] [-f] ...

Ah, very nice. I didn't know about those options. Hopefully I can use
it. Thanks for pointing it out.

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-25 17:54 UTC (permalink / raw)
  To: git
In-Reply-To: <7vsjk99exw.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]

Junio C Hamano wrote:
> So if we were to allow the hook to lie what commits were fetched and store
> something different from what we fetched in the remote tracking refs, I
> think the correct place to do so would be in store_updated_refs(),
> immediately before we call check_everything_connected().
> 
>  - Feed the contents of the ref_map to the hook. For each entry, the hook
>    would get (at least):
>    . the object name;
>    . the refname at the remote;
>    . the refname at the local (which could be empty when we are not
>      copying it to any of our local ref); and
>    . if the entry is to be used for merge.
> 
>  - The hook must read _everything_ from its standard input, and then
>    return the
>    re-written result in the same format as its input. The hook could
>    . update the object name (i.e. re-point the remote tracking ref);
>    . update the local refname (i.e. use different remote tracking ref);
>    . change "merge" flag between true/false; and/or
>    . add or remove entries
> 
>  - You read from the hook and replace the ref_map list that is fed to
>    check_everything_connected(). This ref_map list is what is used in the
>    next for() loop that calls update_local_ref() to update the remote
>    tracking ref, records the entry in FETCH_HEAD, and produces the report.
> 
> This way, the hook cannot screw up, as what it tells us will consistently
> be written by us to where it should go.

This is a good plan, the only problem I see with it is that
store_updated_refs is potentially called twice in a fetch, when the
automated tag following is done. I don't see that as a large problem,
perhaps it could even be optimised away.

The format of .git/FETCH_HEAD does not seem appropriate for this hook
to use (it's not documented, and it doesn't quite have all the necessary
fields, particularly missing the local refname). Instead, how about this,
for the hook's input/output format?

<sha1> SP <not-for-merge|merge> SP <remote-refname> SP <local-refname> LF

Example:

5d6dfc7cb140a6eb90138334fab2245b69bc8bc4 merge refs/heads/master refs/remotes/origin/master
f95247ea15bc62a2dab0f6ae3cd247267a0639b8 not-for-merge refs/heads/pu refs/remotes/origin/pu
2ce0edcd786b790fed580e7df56291619834d276 not-for-merge refs/tags/v1.7.8.1 refs/tags/v1.7.8.1

Allowing the hook to change the merge flag does open up some other
interesting uses of the hook. I can now think of three use cases for it:

1. Only accepting tags that meet some criteria, such as being signed
   by a trusted signature.
2. Causing branches that would not normally be merged to get merged.
   For example, a hook could set the merge flag on a branch when it was
   pulled from a remote other than branch.master.remote. This could be useful
   when using git without a single central origin and with a number of
   repositories that are always wanted to be kept merged.
3. My git annex merge case.

-- 
see shy jo

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-25 18:06 UTC (permalink / raw)
  To: git
In-Reply-To: <m3liq0fwkz.fsf@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

Jakub Narebski wrote:
> This is a very nice idea, IMHO, both because it makes it simple to
> implement no-op (example) hook by just using "cat", and beause it
> makes possible to stack such hooks (e.g. one from git-annex with the
> one from pristine-tar etc.).

Indeed.

> One thing that needs to be specified is what should happen if the hook
> changes "the refname at the remote" part...

I'm not sure what the use case is for including the remote's refname is yet.
Changing it would allow changing what's written into FETCH_HEAD though.
For example:

-2ce0edcd786b790fed580e7df56291619834d276        not-for-merge   branch 'maint' of git://git.kernel.org/pub/scm/git/git
+2ce0edcd786b790fed580e7df56291619834d276        not-for-merge   branch 'jc-maint' of git://git.kernel.org/pub/scm/git/git

And that would in turn be used by a few things that consume that
information. Whether that's useful, I don't know.

-- 
see shy jo

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 19:00 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8ANZvkY+na-tM95prHEfXD+N2OT8+3NLeccycGL1BmbCg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 596 bytes --]

On Sat, Dec 24, 2011 at 11:31:51AM +0700, Nguyen Thai Ngoc Duy wrote:
> On Sat, Dec 24, 2011 at 3:13 AM, Carlos Martín Nieto <cmn@elego.de> wrote:
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
> 
> What about tags? Are all tags fetched or only ones part of the
> selected branches?

I haven't really touched that part of the code, so I think it'll fetch
every tag, as that's what clone does by default. Certainly something
that should get fixed.

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: Gitk: shortcut to jump to the current HEAD (yellow spot)?
From: Dirk Süsserott @ 2011-12-25 19:33 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Pat Thoyts, Git Mailing List
In-Reply-To: <CAOeW2eGCKxYW1TT-HPoSCO0_PsQPX5C-bcHLUy73MTd7=CsqRA@mail.gmail.com>

Am 24.12.2011 05:22 schrieb Martin von Zweigbergk:
> 2011/12/23 Dirk Süsserott <newsletter@dirk.my1.cc>:
>>
>> That's because gitk behaves odd (at least to me) when not run from the
>> top-level directory. E.g. the "touching paths" box won't find files in
>> the top dir if you don't prefix them with a slash.
> 
> This should be fixed in c332f44 (gitk: Fix file highlight when run in
> subdirectory, 2011-04-04), which is in the current master and thus, I
> believe, to be released in Git 1.7.9.
> 
> Martin

Ahh, cool. I wouldn't have noticed because I'm so used to my "cd $TOP &&
gitk". I thought it was by intention because it just behaves like "git
log": When run from subdirs it doesn't know about topdir files: Assume
README.txt is in the topdir and current dir is some subdir:

$ git log -- README.txt    # fails
$ git log -- ../README.txt # works

My alias (or function) was just a helper to avoid remembering where I
started gitk from.

Cheers,
    Dirk

BTW, Merry X-Mas to you and all others on the list :-)

^ permalink raw reply

* Re: [RFC PATCH] Allow cloning branches selectively
From: Carlos Martín Nieto @ 2011-12-25 20:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmxajaswj.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]

On Fri, Dec 23, 2011 at 01:18:36PM -0800, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > Sometimes it's useful to clone only a subset of branches from a remote
> > we're cloning. Teach clone the --fetch option to select which branches
> > should get fetched.
> 
> This is just a knee-jerk reaction without reading the patch text (which I
> won't be doing over the holiday weekend anyway), but is the workflow of
> the primarly intended audience to clone "a subset of branches" or "a
> single branch"?
> 
> I have a slight suspicion that this started out as "I often want to create
> a clone to _track_ a single branch, but because I am mucking with the code
> related to cloning anyway, I might as well allow more than one to be
> fetched, even though I do not have any need for that, somebody might find
> it useful". And that is why it is important to answer the first question.

The main usefulness is indeed to track single branches. Limiting it to
one looked to me to be an arbitrary limitation and I don't like
those. Tracking between two and all branches is probably quite a niche
however, so I'll go with allowing one -t/--track option and if enough
people want to extend it, we'll see what we do then.

> 
> If the primary motivation is for a single branch, I suspect supporting
> only a single branch and advertising the feature as "tracking only one
> branch" might make it much easier to understand to the end users.
> 
> Upon "git clone --track cn/single-clone $there x.git", you would do
> something like:
> 
>   it=cn/single-clone &&
>   git init x.git &&
>   cd x.git &&
> 
>   # configure "git fetch origin" to only get branch $it
>   git config remote.origin.url "$there" &&
>   git config remote.origin.fetch refs/heads/$it:refs/remotes/origin/$it 
> 
>   # declare that the primary branch at origin is $it as far as we are concerned
>   git symbolic-ref -m clone refs/remotes/origin/HEAD refs/remotes/origin/$it &&

As Jakub pointed out, remote already has this option, so you can
replace these calls with

    git remote add origin $there -t $it -m $it

> 
>   # Git aware prompt reminds us that this repository is to track branch $it
>   git symbolic-ref -m clone HEAD refs/heads/$it &&
> 
>   # And Go!
>   git fetch origin &&
>   git reset --hard remotes/origin/$it &&
>   git config branch.$it.remote origin &&
>   git config branch.$it.merge $it

A lazier man (or one who doesn't work with internals every day) might do

    git checkout -t origin/$it

which the documentation claims would do the same thing. So it boils
down to doing three commands and it feels like it'd be much easier to
just write a small script than to modify the clone code. Putting it in
C will hopefully make the code a bit cleaner, if I use the code that
already exists in remote.

> 
> Of course you _could_ support more than one pretty easily, but the point
> is that it is unclear how you explain to the end user what the feature
> does and what it is for in easily understoodd terms, once you start doing
> so. It will no longer be "this new clone is to track that branch", but
> something else, and I do not know what that something else is.

Looking at the explanation for the -t (--track) option in git-remote,
it's certainly not very friendly unless you already know exactly what
a -t option would do if you were to implement it.

   cmn

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* [PATCH] add post-fetch hook
From: Joey Hess @ 2011-12-26  2:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsjk99exw.fsf@alter.siamese.dyndns.org>

The post-fetch hook is fed lines on stdin for all refs that were fetched, and
outputs on stdout possibly modified lines. Its output is parsed and used
when git fetch updates the remote tracking refs, records the entries in
FETCH_HEAD, and produces its report.

---

Not quite ready to sign off on this yet, but it does work.
Comments and code review appreciated.

Demo:

joey@gnu:~/tmp/demo>cat .git/hooks/post-fetch
#!/bin/sh
# Rename branches, and block all tags.
sed 's!foo!bar!g' | grep -v refs/tags/
joey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git pull
From /home/joey/tmp/a
 * [new branch]      bar        -> origin/bar
Already up-to-date.
joey@gnu:~/tmp/demo>chmod -x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git pull
From /home/joey/tmp/a
 * [new branch]      foo        -> origin/foo
 * [new tag]         v1.0       -> v1.0
Already up-to-date.
joey@gnu:~/tmp/demo>chmod +x .git/hooks/post-fetch
joey@gnu:~/tmp/demo>git remote update --prune
Fetching origin
 x [deleted]         (none)     -> origin/foo


 Documentation/githooks.txt |   29 ++++++
 builtin/fetch.c            |  228 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 238 insertions(+), 19 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 28edefa..9c2b6bf 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -162,6 +162,35 @@ This hook can be used to perform repository validity checks, auto-display
 differences from the previous HEAD if different, or set working dir metadata
 properties.
 
+post-fetch
+~~~~~~~~~~
+
+This hook is invoked by 'git fetch' (commonly called by 'git pull'), after
+refs have been fetched from the remote repository. It is not executed, if
+nothing was fetched.
+
+It takes no arguments, but is fed a line of the following format on
+its standard input for each ref that was fetched.
+
+  <sha1> SP not-for-merge|merge SP <remote-refname> SP <local-refname> LF
+
+Where the "not-for-merge" flag indicates the ref is not to be merged into the
+current branch, and the "merge" flag indicates that 'git merge' should
+later merge it. The `<remote-refname>` is the remote's name for the ref
+that was pulled, and `<local-refname>` is a name of a remote-tracking branch,
+like "refs/remotes/origin/master", or can be empty if the fetched ref is not
+being stored in a local refname.
+
+The hook must consume all of its standard input, and output back lines
+of the same format. It can modify its input as desired, including
+adding or removing lines, updating the sha1 (i.e. re-point the
+remote-tracking branch), changing the merge flag, and changing the
+`<local-refname>` (i.e. use different remote-tracking branch).
+
+The output of the hook is used to update the remote-tracking branches, and
+`.git/FETCH_HEAD`, in preparation for for a later merge operation done by
+'git merge'.
+
 post-merge
 ~~~~~~~~~~
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 33ad3aa..aa401b2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -89,6 +89,188 @@ static struct option builtin_fetch_options[] = {
 	OPT_END()
 };
 
+static int add_existing(const char *refname, const unsigned char *sha1,
+			int flag, void *cbdata)
+{
+	struct string_list *list = (struct string_list *)cbdata;
+	struct string_list_item *item = string_list_insert(list, refname);
+	item->util = (void *)sha1;
+	return 0;
+}
+	
+static const char post_fetch_hook[] = "post-fetch";
+struct ref *post_fetch_hook_refs;
+
+int feed_post_fetch_hook (int in, int out, void *data)
+{
+	struct ref *ref;
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	for (ref = post_fetch_hook_refs; ref; ref = ref->next) {
+		strbuf_addstr(&buf, sha1_to_hex(ref->old_sha1));
+		strbuf_addch(&buf, ' ');
+		strbuf_addstr(&buf, ref->merge ? "merge" : "not-for-merge");
+		strbuf_addch(&buf, ' ');
+		if (ref->name)
+			strbuf_addstr(&buf, ref->name);
+		strbuf_addch(&buf, ' ');
+		if (ref->peer_ref && ref->peer_ref->name)
+			strbuf_addstr(&buf, ref->peer_ref->name);
+		strbuf_addch(&buf, '\n');
+	}
+
+	ret = write_in_full(out, buf.buf, buf.len) != buf.len;
+	if (ret)
+		warning("%s hook failed to consume all its input",
+				post_fetch_hook);
+	close(out);
+	strbuf_release(&buf);
+	return ret;
+}
+	
+struct ref *parse_post_fetch_hook_line (char *l, 
+		struct string_list *existing_refs)
+{
+	struct ref *ref = NULL, *peer_ref = NULL;
+	struct string_list_item *peer_item = NULL;
+	char *words[4];
+	int i, word=0;
+	char *problem;
+
+	for (i=0; l[i]; i++) {
+		if (isspace(l[i])) {
+			l[i]='\0';
+			words[word]=l;
+			l+=i+1;
+			i=0;
+			word++;
+			if (word > 3) {
+				problem="too many words";
+				goto unparsable;
+			}
+		}
+	}
+	if (word < 3) {
+		problem="not enough words";
+		goto unparsable;
+	}
+	
+	ref = alloc_ref(words[2]);
+	peer_ref = ref->peer_ref = alloc_ref(l);
+	ref->peer_ref->force=1;
+
+	if (get_sha1_hex(words[0], ref->old_sha1)) {
+		problem="bad sha1";
+		goto unparsable;
+	}
+
+	if (strcmp(words[1], "merge") == 0) {
+		ref->merge=1;
+	}
+	else if (strcmp(words[1], "not-for-merge") != 0) {
+		problem="bad merge flag";
+		goto unparsable;
+	}
+
+	peer_item = string_list_lookup(existing_refs, peer_ref->name);
+	if (peer_item)
+		hashcpy(peer_ref->old_sha1, peer_item->util);
+
+	return ref;
+
+ unparsable:
+	warning("%s hook output a wrongly formed line: %s",
+			post_fetch_hook, problem);
+	free(ref);
+	free(peer_ref);
+	return NULL;
+}
+
+/* The hook is fed lines of the form:
+ * <sha1> SP <not-for-merge|merge> SP <remote-refname> SP <local-refname> LF
+ * And should output rewritten lines of the same form.
+ */
+struct ref *run_post_fetch_hook (struct ref *fetched_refs)
+{
+	struct ref *new_refs = NULL;
+	struct string_list existing_refs = STRING_LIST_INIT_NODUP;
+	struct child_process hook;
+	struct async async;
+	const char *argv[2];
+	FILE *f;
+	struct strbuf buf;
+	struct ref *ref, *prevref=NULL;
+	int ok = 1;
+
+	if (! fetched_refs)
+		return fetched_refs;
+
+	argv[0] = git_path("hooks/%s", post_fetch_hook);
+	if (access(argv[0], X_OK) < 0)
+		return fetched_refs;
+	argv[1] = NULL;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.in = -1;
+	hook.out = -1;
+	if (start_command(&hook) != 0)
+		return fetched_refs;
+
+	/* Use an async writer to feed the hook process.
+	 * This allows the hook to read and write a line at
+	 * a time without blocking. */
+	memset(&async, 0, sizeof(async));
+	async.proc = feed_post_fetch_hook;
+	post_fetch_hook_refs = fetched_refs;
+	async.out = hook.in;
+	if (start_async(&async))
+		goto failed_hook;
+
+	for_each_ref(add_existing, &existing_refs);
+
+	f = fdopen(hook.out, "r");
+	if (f == NULL)
+		goto failed_hook;
+	strbuf_init(&buf, 128);
+	while (strbuf_getline(&buf, f, '\n') != EOF) {
+		char *l = strbuf_detach(&buf, NULL);
+		ref = parse_post_fetch_hook_line(l, &existing_refs);
+		if (ref) {
+			if (prevref) {
+				prevref->next=ref;
+				prevref=ref;
+			}
+			else {
+				new_refs = prevref = ref;
+			}
+		}
+		else {
+			ok = 0; /* ignore the other output */
+		}
+		free(l);
+	}
+	strbuf_release(&buf);
+	fclose(f);
+
+	if (finish_async(&async))
+		goto failed_hook;
+	finish_command(&hook);
+
+	if (! ok)
+		return fetched_refs;
+	/* The new_refs are returned, to be used in place of fetched_refs,
+	 * so it is not needed anymore and can be freed here. */
+	free_refs(fetched_refs);
+	return new_refs;
+
+failed_hook:
+	close(hook.out);
+	finish_command(&hook);
+	return fetched_refs;
+}
+
 static void unlock_pack(void)
 {
 	if (transport)
@@ -507,17 +689,30 @@ static int quickfetch(struct ref *ref_map)
 	return check_everything_connected(iterate_ref_map, 1, &rm);
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+struct fetch_refs_result {
+	struct ref *new_refs;
+	int status;
+};
+
+static struct fetch_refs_result fetch_refs(struct transport *transport,
+		struct ref *ref_map)
 {
-	int ret = quickfetch(ref_map);
-	if (ret)
-		ret = transport_fetch_refs(transport, ref_map);
-	if (!ret)
-		ret |= store_updated_refs(transport->url,
+	struct fetch_refs_result res;
+	res.status = quickfetch(ref_map);
+	if (res.status)
+		res.status = transport_fetch_refs(transport, ref_map);
+	if (!res.status) {
+		res.new_refs = run_post_fetch_hook(ref_map);
+
+		res.status |= store_updated_refs(transport->url,
 				transport->remote->name,
-				ref_map);
+				res.new_refs);
+	}
+	else {
+		res.new_refs = ref_map;
+	}
 	transport_unlock_pack(transport);
-	return ret;
+	return res;
 }
 
 static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
@@ -542,15 +737,6 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
 	return result;
 }
 
-static int add_existing(const char *refname, const unsigned char *sha1,
-			int flag, void *cbdata)
-{
-	struct string_list *list = (struct string_list *)cbdata;
-	struct string_list_item *item = string_list_insert(list, refname);
-	item->util = (void *)sha1;
-	return 0;
-}
-
 static int will_fetch(struct ref **head, const unsigned char *sha1)
 {
 	struct ref *rm = *head;
@@ -673,6 +859,7 @@ static int do_fetch(struct transport *transport,
 	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
 	struct ref *rm;
+	struct fetch_refs_result res;
 	int autotags = (transport->remote->fetch_tags == 1);
 
 	for_each_ref(add_existing, &existing_refs);
@@ -710,7 +897,9 @@ static int do_fetch(struct transport *transport,
 
 	if (tags == TAGS_DEFAULT && autotags)
 		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
-	if (fetch_refs(transport, ref_map)) {
+	res = fetch_refs(transport, ref_map);
+	ref_map = res.new_refs;
+	if (res.status) {
 		free_refs(ref_map);
 		return 1;
 	}
@@ -750,7 +939,8 @@ static int do_fetch(struct transport *transport,
 		if (ref_map) {
 			transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
 			transport_set_option(transport, TRANS_OPT_DEPTH, "0");
-			fetch_refs(transport, ref_map);
+			res = fetch_refs(transport, ref_map);
+			ref_map = res.new_refs;
 		}
 		free_refs(ref_map);
 	}

-- 
1.7.7.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox