* [PATCH v7 p1 10/13] remote-testgit: report success after an import
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
Doesn't make a difference for the tests, but it does for the ones
seeking reference.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-remote-testgit | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/git-remote-testgit b/git-remote-testgit
index efda74b..6fb8780 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -65,7 +65,20 @@ do
sed -e "s#refs/heads/#${prefix}/heads/#g"
;;
export)
+ before=$(git for-each-ref --format='%(refname) %(objectname)')
+
git fast-import "${testgitmarks_args[@]}" --quiet
+
+ after=$(git for-each-ref --format='%(refname) %(objectname)')
+
+ # figure out which refs were updated
+ join -e 0 -o '0 1.2 2.2' -a 2 <(echo "$before") <(echo "$after") |
+ while read ref a b
+ do
+ test $a == $b && continue
+ echo "ok $ref"
+ done
+
echo
;;
'')
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p1 09/13] remote-testgit: exercise more features
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
Unfortunately a lot of these tests fail.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-remote-testgit | 38 +++++++++++++++++++++++-----------
t/t5801-remote-helpers.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 12 deletions(-)
diff --git a/git-remote-testgit b/git-remote-testgit
index 5117ab5..efda74b 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -6,17 +6,25 @@ url=$2
dir="$GIT_DIR/testgit/$alias"
prefix="refs/testgit/$alias"
-refspec="refs/heads/*:${prefix}/heads/*"
-gitmarks="$dir/git.marks"
-testgitmarks="$dir/testgit.marks"
+default_refspec="refs/heads/*:${prefix}/heads/*"
+
+refspec="${GIT_REMOTE_TESTGIT_REFSPEC-$default_refspec}"
+
+test -z "$refspec" && prefix="refs"
export GIT_DIR="$url/.git"
mkdir -p "$dir"
-test -e "$gitmarks" || > "$gitmarks"
-test -e "$testgitmarks" || > "$testgitmarks"
+if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
+then
+ gitmarks="$dir/git.marks"
+ testgitmarks="$dir/testgit.marks"
+ test -e "$gitmarks" || >"$gitmarks"
+ test -e "$testgitmarks" || >"$testgitmarks"
+ testgitmarks_args=( "--"{import,export}"-marks=$testgitmarks" )
+fi
while read line
do
@@ -24,9 +32,12 @@ do
capabilities)
echo 'import'
echo 'export'
- echo "refspec $refspec"
- echo "*import-marks $gitmarks"
- echo "*export-marks $gitmarks"
+ test -n "$refspec" && echo "refspec $refspec"
+ if test -n "$gitmarks"
+ then
+ echo "*import-marks $gitmarks"
+ echo "*export-marks $gitmarks"
+ fi
echo
;;
list)
@@ -45,13 +56,16 @@ do
test "${line%% *}" != "import" && break
done
- echo "feature import-marks=$gitmarks"
- echo "feature export-marks=$gitmarks"
- git fast-export --use-done-feature --{import,export}-marks="$testgitmarks" $refs |
+ if test -n "$gitmarks"
+ then
+ echo "feature import-marks=$gitmarks"
+ echo "feature export-marks=$gitmarks"
+ fi
+ git fast-export --use-done-feature "${testgitmarks_args[@]}" $refs |
sed -e "s#refs/heads/#${prefix}/heads/#g"
;;
export)
- git fast-import --{import,export}-marks="$testgitmarks" --quiet
+ git fast-import "${testgitmarks_args[@]}" --quiet
echo
;;
'')
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index bc0b5f7..12ae256 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -110,4 +110,56 @@ test_expect_failure 'push new branch with old:new refspec' '
compare_refs local HEAD server refs/heads/new-refspec
'
+test_expect_success 'cloning without refspec' '
+ GIT_REMOTE_TESTGIT_REFSPEC="" \
+ git clone "testgit::${PWD}/server" local2 &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without refspecs' '
+ (cd local2 &&
+ git reset --hard &&
+ GIT_REMOTE_TESTGIT_REFSPEC="" git pull) &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without refspecs' '
+ test_when_finished "(cd local2 && git reset --hard origin)" &&
+ (cd local2 &&
+ echo content >>file &&
+ git commit -a -m ten &&
+ GIT_REMOTE_TESTGIT_REFSPEC="" git push) &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling with straight refspec' '
+ (cd local2 &&
+ GIT_REMOTE_TESTGIT_REFSPEC="*:*" git pull) &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing with straight refspec' '
+ test_when_finished "(cd local2 && git reset --hard origin)" &&
+ (cd local2 &&
+ echo content >>file &&
+ git commit -a -m eleven &&
+ GIT_REMOTE_TESTGIT_REFSPEC="*:*" git push) &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_success 'pulling without marks' '
+ (cd local2 &&
+ GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) &&
+ compare_refs local2 HEAD server HEAD
+'
+
+test_expect_failure 'pushing without marks' '
+ test_when_finished "(cd local2 && git reset --hard origin)" &&
+ (cd local2 &&
+ echo content >>file &&
+ git commit -a -m twelve &&
+ GIT_REMOTE_TESTGIT_NO_MARKS=1 git push) &&
+ compare_refs local2 HEAD server HEAD
+'
+
test_done
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p1 12/13] fast-export: trivial cleanup
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
Setting 'commit' to 'commit' is a no-op. It might have been there to
avoid a compiler warning, but if so, it was the compiler to blame, and
it's certainly not there any more.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/fast-export.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 9b70ec1..191936c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -483,7 +483,7 @@ static void get_tags_and_duplicates(struct object_array *pending,
for (i = 0; i < pending->nr; i++) {
struct object_array_entry *e = pending->objects + i;
unsigned char sha1[20];
- struct commit *commit = commit;
+ struct commit *commit;
char *full_name;
if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p1 11/13] remote-testgit: implement the "done" feature manually
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
People who want to write their own remote-helper will find it more
useful to see clearly how they are supposed to advertise and implement
the "done" feature themselves.
Right now we are relying on fast-export to do that by using the
--use-done-feature argument. However, people writing their own
remote-helper would probably not have such an option, as they would
probably be writing the fast-export functionality themselves.
It should now be clearer to them.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-remote-testgit | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-remote-testgit b/git-remote-testgit
index 6fb8780..b395c8d 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -61,8 +61,10 @@ do
echo "feature import-marks=$gitmarks"
echo "feature export-marks=$gitmarks"
fi
- git fast-export --use-done-feature "${testgitmarks_args[@]}" $refs |
+ echo "feature done"
+ git fast-export "${testgitmarks_args[@]}" $refs |
sed -e "s#refs/heads/#${prefix}/heads/#g"
+ echo "done"
;;
export)
before=$(git for-each-ref --format='%(refname) %(objectname)')
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p1 13/13] fast-export: fix comparison in tests
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
First the expected, then the actual, otherwise the diff would be the
opposite of what we want.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9350-fast-export.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 5948b65..1f59862 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -303,7 +303,7 @@ test_expect_success 'dropping tag of filtered out object' '
(
cd limit-by-paths &&
git fast-export --tag-of-filtered-object=drop mytag -- there > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
@@ -320,7 +320,7 @@ test_expect_success 'rewriting tag of filtered out object' '
(
cd limit-by-paths &&
git fast-export --tag-of-filtered-object=rewrite mytag -- there > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
@@ -351,7 +351,7 @@ test_expect_failure 'no exact-ref revisions included' '
(
cd limit-by-paths &&
git fast-export master~2..master~1 > output &&
- test_cmp output expected
+ test_cmp expected output
)
'
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p1 04/13] Rename git-remote-testgit to git-remote-testpy
From: Felipe Contreras @ 2012-11-28 22:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354140669-23533-1-git-send-email-felipe.contreras@gmail.com>
This script is not really exercising the remote-helper functionality,
but more the python framework for remote helpers that live in
git_remote_helpers.
It's also not a good example of how to write remote-helpers, unless you
are planning to use python, and even then you might not want to use this
framework.
So let's use a more appropriate name: git-remote-testpy.
A patch that replaces git-remote-testgit with a simpler version is on
the way.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
.gitignore | 2 +-
Makefile | 2 +-
git-remote-testgit.py | 272 ----------------------------------------------
git-remote-testpy.py | 272 ++++++++++++++++++++++++++++++++++++++++++++++
t/t5800-remote-helpers.sh | 148 -------------------------
t/t5800-remote-testpy.sh | 148 +++++++++++++++++++++++++
6 files changed, 422 insertions(+), 422 deletions(-)
delete mode 100644 git-remote-testgit.py
create mode 100644 git-remote-testpy.py
delete mode 100755 t/t5800-remote-helpers.sh
create mode 100755 t/t5800-remote-testpy.sh
diff --git a/.gitignore b/.gitignore
index eb5114f..827fe48 100644
--- a/.gitignore
+++ b/.gitignore
@@ -124,7 +124,7 @@
/git-remote-ftps
/git-remote-fd
/git-remote-ext
-/git-remote-testgit
+/git-remote-testpy
/git-remote-testsvn
/git-repack
/git-replace
diff --git a/Makefile b/Makefile
index 2ab0747..83a36b7 100644
--- a/Makefile
+++ b/Makefile
@@ -470,7 +470,7 @@ SCRIPT_PERL += git-relink.perl
SCRIPT_PERL += git-send-email.perl
SCRIPT_PERL += git-svn.perl
-SCRIPT_PYTHON += git-remote-testgit.py
+SCRIPT_PYTHON += git-remote-testpy.py
SCRIPT_PYTHON += git-p4.py
SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
diff --git a/git-remote-testgit.py b/git-remote-testgit.py
deleted file mode 100644
index ade797b..0000000
--- a/git-remote-testgit.py
+++ /dev/null
@@ -1,272 +0,0 @@
-#!/usr/bin/env python
-
-# This command is a simple remote-helper, that is used both as a
-# testcase for the remote-helper functionality, and as an example to
-# show remote-helper authors one possible implementation.
-#
-# This is a Git <-> Git importer/exporter, that simply uses git
-# fast-import and git fast-export to consume and produce fast-import
-# streams.
-#
-# To understand better the way things work, one can activate debug
-# traces by setting (to any value) the environment variables
-# GIT_TRANSPORT_HELPER_DEBUG and GIT_DEBUG_TESTGIT, to see messages
-# from the transport-helper side, or from this example remote-helper.
-
-# hashlib is only available in python >= 2.5
-try:
- import hashlib
- _digest = hashlib.sha1
-except ImportError:
- import sha
- _digest = sha.new
-import sys
-import os
-import time
-sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
-
-from git_remote_helpers.util import die, debug, warn
-from git_remote_helpers.git.repo import GitRepo
-from git_remote_helpers.git.exporter import GitExporter
-from git_remote_helpers.git.importer import GitImporter
-from git_remote_helpers.git.non_local import NonLocalGit
-
-def get_repo(alias, url):
- """Returns a git repository object initialized for usage.
- """
-
- repo = GitRepo(url)
- repo.get_revs()
- repo.get_head()
-
- hasher = _digest()
- hasher.update(repo.path)
- repo.hash = hasher.hexdigest()
-
- repo.get_base_path = lambda base: os.path.join(
- base, 'info', 'fast-import', repo.hash)
-
- prefix = 'refs/testgit/%s/' % alias
- debug("prefix: '%s'", prefix)
-
- repo.gitdir = os.environ["GIT_DIR"]
- repo.alias = alias
- repo.prefix = prefix
-
- repo.exporter = GitExporter(repo)
- repo.importer = GitImporter(repo)
- repo.non_local = NonLocalGit(repo)
-
- return repo
-
-
-def local_repo(repo, path):
- """Returns a git repository object initalized for usage.
- """
-
- local = GitRepo(path)
-
- local.non_local = None
- local.gitdir = repo.gitdir
- local.alias = repo.alias
- local.prefix = repo.prefix
- local.hash = repo.hash
- local.get_base_path = repo.get_base_path
- local.exporter = GitExporter(local)
- local.importer = GitImporter(local)
-
- return local
-
-
-def do_capabilities(repo, args):
- """Prints the supported capabilities.
- """
-
- print "import"
- print "export"
- print "refspec refs/heads/*:%s*" % repo.prefix
-
- dirname = repo.get_base_path(repo.gitdir)
-
- if not os.path.exists(dirname):
- os.makedirs(dirname)
-
- path = os.path.join(dirname, 'git.marks')
-
- print "*export-marks %s" % path
- if os.path.exists(path):
- print "*import-marks %s" % path
-
- print # end capabilities
-
-
-def do_list(repo, args):
- """Lists all known references.
-
- Bug: This will always set the remote head to master for non-local
- repositories, since we have no way of determining what the remote
- head is at clone time.
- """
-
- for ref in repo.revs:
- debug("? refs/heads/%s", ref)
- print "? refs/heads/%s" % ref
-
- if repo.head:
- debug("@refs/heads/%s HEAD" % repo.head)
- print "@refs/heads/%s HEAD" % repo.head
- else:
- debug("@refs/heads/master HEAD")
- print "@refs/heads/master HEAD"
-
- print # end list
-
-
-def update_local_repo(repo):
- """Updates (or clones) a local repo.
- """
-
- if repo.local:
- return repo
-
- path = repo.non_local.clone(repo.gitdir)
- repo.non_local.update(repo.gitdir)
- repo = local_repo(repo, path)
- return repo
-
-
-def do_import(repo, args):
- """Exports a fast-import stream from testgit for git to import.
- """
-
- if len(args) != 1:
- die("Import needs exactly one ref")
-
- if not repo.gitdir:
- die("Need gitdir to import")
-
- ref = args[0]
- refs = [ref]
-
- while True:
- line = sys.stdin.readline()
- if line == '\n':
- break
- if not line.startswith('import '):
- die("Expected import line.")
-
- # strip of leading 'import '
- ref = line[7:].strip()
- refs.append(ref)
-
- repo = update_local_repo(repo)
- repo.exporter.export_repo(repo.gitdir, refs)
-
- print "done"
-
-
-def do_export(repo, args):
- """Imports a fast-import stream from git to testgit.
- """
-
- if not repo.gitdir:
- die("Need gitdir to export")
-
- update_local_repo(repo)
- changed = repo.importer.do_import(repo.gitdir)
-
- if not repo.local:
- repo.non_local.push(repo.gitdir)
-
- for ref in changed:
- print "ok %s" % ref
- print
-
-
-COMMANDS = {
- 'capabilities': do_capabilities,
- 'list': do_list,
- 'import': do_import,
- 'export': do_export,
-}
-
-
-def sanitize(value):
- """Cleans up the url.
- """
-
- if value.startswith('testgit::'):
- value = value[9:]
-
- return value
-
-
-def read_one_line(repo):
- """Reads and processes one command.
- """
-
- sleepy = os.environ.get("GIT_REMOTE_TESTGIT_SLEEPY")
- if sleepy:
- debug("Sleeping %d sec before readline" % int(sleepy))
- time.sleep(int(sleepy))
-
- line = sys.stdin.readline()
-
- cmdline = line
-
- if not cmdline:
- warn("Unexpected EOF")
- return False
-
- cmdline = cmdline.strip().split()
- if not cmdline:
- # Blank line means we're about to quit
- return False
-
- cmd = cmdline.pop(0)
- debug("Got command '%s' with args '%s'", cmd, ' '.join(cmdline))
-
- if cmd not in COMMANDS:
- die("Unknown command, %s", cmd)
-
- func = COMMANDS[cmd]
- func(repo, cmdline)
- sys.stdout.flush()
-
- return True
-
-
-def main(args):
- """Starts a new remote helper for the specified repository.
- """
-
- if len(args) != 3:
- die("Expecting exactly three arguments.")
- sys.exit(1)
-
- if os.getenv("GIT_DEBUG_TESTGIT"):
- import git_remote_helpers.util
- git_remote_helpers.util.DEBUG = True
-
- alias = sanitize(args[1])
- url = sanitize(args[2])
-
- if not alias.isalnum():
- warn("non-alnum alias '%s'", alias)
- alias = "tmp"
-
- args[1] = alias
- args[2] = url
-
- repo = get_repo(alias, url)
-
- debug("Got arguments %s", args[1:])
-
- more = True
-
- sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)
- while (more):
- more = read_one_line(repo)
-
-if __name__ == '__main__':
- sys.exit(main(sys.argv))
diff --git a/git-remote-testpy.py b/git-remote-testpy.py
new file mode 100644
index 0000000..ade797b
--- /dev/null
+++ b/git-remote-testpy.py
@@ -0,0 +1,272 @@
+#!/usr/bin/env python
+
+# This command is a simple remote-helper, that is used both as a
+# testcase for the remote-helper functionality, and as an example to
+# show remote-helper authors one possible implementation.
+#
+# This is a Git <-> Git importer/exporter, that simply uses git
+# fast-import and git fast-export to consume and produce fast-import
+# streams.
+#
+# To understand better the way things work, one can activate debug
+# traces by setting (to any value) the environment variables
+# GIT_TRANSPORT_HELPER_DEBUG and GIT_DEBUG_TESTGIT, to see messages
+# from the transport-helper side, or from this example remote-helper.
+
+# hashlib is only available in python >= 2.5
+try:
+ import hashlib
+ _digest = hashlib.sha1
+except ImportError:
+ import sha
+ _digest = sha.new
+import sys
+import os
+import time
+sys.path.insert(0, os.getenv("GITPYTHONLIB","."))
+
+from git_remote_helpers.util import die, debug, warn
+from git_remote_helpers.git.repo import GitRepo
+from git_remote_helpers.git.exporter import GitExporter
+from git_remote_helpers.git.importer import GitImporter
+from git_remote_helpers.git.non_local import NonLocalGit
+
+def get_repo(alias, url):
+ """Returns a git repository object initialized for usage.
+ """
+
+ repo = GitRepo(url)
+ repo.get_revs()
+ repo.get_head()
+
+ hasher = _digest()
+ hasher.update(repo.path)
+ repo.hash = hasher.hexdigest()
+
+ repo.get_base_path = lambda base: os.path.join(
+ base, 'info', 'fast-import', repo.hash)
+
+ prefix = 'refs/testgit/%s/' % alias
+ debug("prefix: '%s'", prefix)
+
+ repo.gitdir = os.environ["GIT_DIR"]
+ repo.alias = alias
+ repo.prefix = prefix
+
+ repo.exporter = GitExporter(repo)
+ repo.importer = GitImporter(repo)
+ repo.non_local = NonLocalGit(repo)
+
+ return repo
+
+
+def local_repo(repo, path):
+ """Returns a git repository object initalized for usage.
+ """
+
+ local = GitRepo(path)
+
+ local.non_local = None
+ local.gitdir = repo.gitdir
+ local.alias = repo.alias
+ local.prefix = repo.prefix
+ local.hash = repo.hash
+ local.get_base_path = repo.get_base_path
+ local.exporter = GitExporter(local)
+ local.importer = GitImporter(local)
+
+ return local
+
+
+def do_capabilities(repo, args):
+ """Prints the supported capabilities.
+ """
+
+ print "import"
+ print "export"
+ print "refspec refs/heads/*:%s*" % repo.prefix
+
+ dirname = repo.get_base_path(repo.gitdir)
+
+ if not os.path.exists(dirname):
+ os.makedirs(dirname)
+
+ path = os.path.join(dirname, 'git.marks')
+
+ print "*export-marks %s" % path
+ if os.path.exists(path):
+ print "*import-marks %s" % path
+
+ print # end capabilities
+
+
+def do_list(repo, args):
+ """Lists all known references.
+
+ Bug: This will always set the remote head to master for non-local
+ repositories, since we have no way of determining what the remote
+ head is at clone time.
+ """
+
+ for ref in repo.revs:
+ debug("? refs/heads/%s", ref)
+ print "? refs/heads/%s" % ref
+
+ if repo.head:
+ debug("@refs/heads/%s HEAD" % repo.head)
+ print "@refs/heads/%s HEAD" % repo.head
+ else:
+ debug("@refs/heads/master HEAD")
+ print "@refs/heads/master HEAD"
+
+ print # end list
+
+
+def update_local_repo(repo):
+ """Updates (or clones) a local repo.
+ """
+
+ if repo.local:
+ return repo
+
+ path = repo.non_local.clone(repo.gitdir)
+ repo.non_local.update(repo.gitdir)
+ repo = local_repo(repo, path)
+ return repo
+
+
+def do_import(repo, args):
+ """Exports a fast-import stream from testgit for git to import.
+ """
+
+ if len(args) != 1:
+ die("Import needs exactly one ref")
+
+ if not repo.gitdir:
+ die("Need gitdir to import")
+
+ ref = args[0]
+ refs = [ref]
+
+ while True:
+ line = sys.stdin.readline()
+ if line == '\n':
+ break
+ if not line.startswith('import '):
+ die("Expected import line.")
+
+ # strip of leading 'import '
+ ref = line[7:].strip()
+ refs.append(ref)
+
+ repo = update_local_repo(repo)
+ repo.exporter.export_repo(repo.gitdir, refs)
+
+ print "done"
+
+
+def do_export(repo, args):
+ """Imports a fast-import stream from git to testgit.
+ """
+
+ if not repo.gitdir:
+ die("Need gitdir to export")
+
+ update_local_repo(repo)
+ changed = repo.importer.do_import(repo.gitdir)
+
+ if not repo.local:
+ repo.non_local.push(repo.gitdir)
+
+ for ref in changed:
+ print "ok %s" % ref
+ print
+
+
+COMMANDS = {
+ 'capabilities': do_capabilities,
+ 'list': do_list,
+ 'import': do_import,
+ 'export': do_export,
+}
+
+
+def sanitize(value):
+ """Cleans up the url.
+ """
+
+ if value.startswith('testgit::'):
+ value = value[9:]
+
+ return value
+
+
+def read_one_line(repo):
+ """Reads and processes one command.
+ """
+
+ sleepy = os.environ.get("GIT_REMOTE_TESTGIT_SLEEPY")
+ if sleepy:
+ debug("Sleeping %d sec before readline" % int(sleepy))
+ time.sleep(int(sleepy))
+
+ line = sys.stdin.readline()
+
+ cmdline = line
+
+ if not cmdline:
+ warn("Unexpected EOF")
+ return False
+
+ cmdline = cmdline.strip().split()
+ if not cmdline:
+ # Blank line means we're about to quit
+ return False
+
+ cmd = cmdline.pop(0)
+ debug("Got command '%s' with args '%s'", cmd, ' '.join(cmdline))
+
+ if cmd not in COMMANDS:
+ die("Unknown command, %s", cmd)
+
+ func = COMMANDS[cmd]
+ func(repo, cmdline)
+ sys.stdout.flush()
+
+ return True
+
+
+def main(args):
+ """Starts a new remote helper for the specified repository.
+ """
+
+ if len(args) != 3:
+ die("Expecting exactly three arguments.")
+ sys.exit(1)
+
+ if os.getenv("GIT_DEBUG_TESTGIT"):
+ import git_remote_helpers.util
+ git_remote_helpers.util.DEBUG = True
+
+ alias = sanitize(args[1])
+ url = sanitize(args[2])
+
+ if not alias.isalnum():
+ warn("non-alnum alias '%s'", alias)
+ alias = "tmp"
+
+ args[1] = alias
+ args[2] = url
+
+ repo = get_repo(alias, url)
+
+ debug("Got arguments %s", args[1:])
+
+ more = True
+
+ sys.stdin = os.fdopen(sys.stdin.fileno(), 'r', 0)
+ while (more):
+ more = read_one_line(repo)
+
+if __name__ == '__main__':
+ sys.exit(main(sys.argv))
diff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh
deleted file mode 100755
index d46fa40..0000000
--- a/t/t5800-remote-helpers.sh
+++ /dev/null
@@ -1,148 +0,0 @@
-#!/bin/sh
-#
-# Copyright (c) 2010 Sverre Rabbelier
-#
-
-test_description='Test remote-helper import and export commands'
-
-. ./test-lib.sh
-
-if ! test_have_prereq PYTHON ; then
- skip_all='skipping remote-testgit tests, python not available'
- test_done
-fi
-
-"$PYTHON_PATH" -c '
-import sys
-if sys.hexversion < 0x02040000:
- sys.exit(1)
-' || {
- skip_all='skipping remote-testgit tests, python version < 2.4'
- test_done
-}
-
-compare_refs() {
- git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
- git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
- test_cmp expect actual
-}
-
-test_expect_success 'setup repository' '
- git init --bare server/.git &&
- git clone server public &&
- (cd public &&
- echo content >file &&
- git add file &&
- git commit -m one &&
- git push origin master)
-'
-
-test_expect_success 'cloning from local repo' '
- git clone "testgit::${PWD}/server" localclone &&
- test_cmp public/file localclone/file
-'
-
-test_expect_success 'cloning from remote repo' '
- git clone "testgit::file://${PWD}/server" clone &&
- test_cmp public/file clone/file
-'
-
-test_expect_success 'create new commit on remote' '
- (cd public &&
- echo content >>file &&
- git commit -a -m two &&
- git push)
-'
-
-test_expect_success 'pulling from local repo' '
- (cd localclone && git pull) &&
- test_cmp public/file localclone/file
-'
-
-test_expect_success 'pulling from remote remote' '
- (cd clone && git pull) &&
- test_cmp public/file clone/file
-'
-
-test_expect_success 'pushing to local repo' '
- (cd localclone &&
- echo content >>file &&
- git commit -a -m three &&
- git push) &&
- compare_refs localclone HEAD server HEAD
-'
-
-# Generally, skip this test. It demonstrates a now-fixed race in
-# git-remote-testgit, but is too slow to leave in for general use.
-: test_expect_success 'racily pushing to local repo' '
- test_when_finished "rm -rf server2 localclone2" &&
- cp -R server server2 &&
- git clone "testgit::${PWD}/server2" localclone2 &&
- (cd localclone2 &&
- echo content >>file &&
- git commit -a -m three &&
- GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
- compare_refs localclone2 HEAD server2 HEAD
-'
-
-test_expect_success 'synch with changes from localclone' '
- (cd clone &&
- git pull)
-'
-
-test_expect_success 'pushing remote local repo' '
- (cd clone &&
- echo content >>file &&
- git commit -a -m four &&
- git push) &&
- compare_refs clone HEAD server HEAD
-'
-
-test_expect_success 'fetch new branch' '
- (cd public &&
- git checkout -b new &&
- echo content >>file &&
- git commit -a -m five &&
- git push origin new
- ) &&
- (cd localclone &&
- git fetch origin new
- ) &&
- compare_refs public HEAD localclone FETCH_HEAD
-'
-
-test_expect_success 'fetch multiple branches' '
- (cd localclone &&
- git fetch
- ) &&
- compare_refs server master localclone refs/remotes/origin/master &&
- compare_refs server new localclone refs/remotes/origin/new
-'
-
-test_expect_success 'push when remote has extra refs' '
- (cd clone &&
- echo content >>file &&
- git commit -a -m six &&
- git push
- ) &&
- compare_refs clone master server master
-'
-
-test_expect_success 'push new branch by name' '
- (cd clone &&
- git checkout -b new-name &&
- echo content >>file &&
- git commit -a -m seven &&
- git push origin new-name
- ) &&
- compare_refs clone HEAD server refs/heads/new-name
-'
-
-test_expect_failure 'push new branch with old:new refspec' '
- (cd clone &&
- git push origin new-name:new-refspec
- ) &&
- compare_refs clone HEAD server refs/heads/new-refspec
-'
-
-test_done
diff --git a/t/t5800-remote-testpy.sh b/t/t5800-remote-testpy.sh
new file mode 100755
index 0000000..6750961
--- /dev/null
+++ b/t/t5800-remote-testpy.sh
@@ -0,0 +1,148 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Sverre Rabbelier
+#
+
+test_description='Test python remote-helper framework'
+
+. ./test-lib.sh
+
+if ! test_have_prereq PYTHON ; then
+ skip_all='skipping python remote-helper tests, python not available'
+ test_done
+fi
+
+"$PYTHON_PATH" -c '
+import sys
+if sys.hexversion < 0x02040000:
+ sys.exit(1)
+' || {
+ skip_all='skipping python remote-helper tests, python version < 2.4'
+ test_done
+}
+
+compare_refs() {
+ git --git-dir="$1/.git" rev-parse --verify $2 >expect &&
+ git --git-dir="$3/.git" rev-parse --verify $4 >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'setup repository' '
+ git init --bare server/.git &&
+ git clone server public &&
+ (cd public &&
+ echo content >file &&
+ git add file &&
+ git commit -m one &&
+ git push origin master)
+'
+
+test_expect_success 'cloning from local repo' '
+ git clone "testpy::${PWD}/server" localclone &&
+ test_cmp public/file localclone/file
+'
+
+test_expect_success 'cloning from remote repo' '
+ git clone "testpy::file://${PWD}/server" clone &&
+ test_cmp public/file clone/file
+'
+
+test_expect_success 'create new commit on remote' '
+ (cd public &&
+ echo content >>file &&
+ git commit -a -m two &&
+ git push)
+'
+
+test_expect_success 'pulling from local repo' '
+ (cd localclone && git pull) &&
+ test_cmp public/file localclone/file
+'
+
+test_expect_success 'pulling from remote remote' '
+ (cd clone && git pull) &&
+ test_cmp public/file clone/file
+'
+
+test_expect_success 'pushing to local repo' '
+ (cd localclone &&
+ echo content >>file &&
+ git commit -a -m three &&
+ git push) &&
+ compare_refs localclone HEAD server HEAD
+'
+
+# Generally, skip this test. It demonstrates a now-fixed race in
+# git-remote-testpy, but is too slow to leave in for general use.
+: test_expect_success 'racily pushing to local repo' '
+ test_when_finished "rm -rf server2 localclone2" &&
+ cp -R server server2 &&
+ git clone "testpy::${PWD}/server2" localclone2 &&
+ (cd localclone2 &&
+ echo content >>file &&
+ git commit -a -m three &&
+ GIT_REMOTE_TESTGIT_SLEEPY=2 git push) &&
+ compare_refs localclone2 HEAD server2 HEAD
+'
+
+test_expect_success 'synch with changes from localclone' '
+ (cd clone &&
+ git pull)
+'
+
+test_expect_success 'pushing remote local repo' '
+ (cd clone &&
+ echo content >>file &&
+ git commit -a -m four &&
+ git push) &&
+ compare_refs clone HEAD server HEAD
+'
+
+test_expect_success 'fetch new branch' '
+ (cd public &&
+ git checkout -b new &&
+ echo content >>file &&
+ git commit -a -m five &&
+ git push origin new
+ ) &&
+ (cd localclone &&
+ git fetch origin new
+ ) &&
+ compare_refs public HEAD localclone FETCH_HEAD
+'
+
+test_expect_success 'fetch multiple branches' '
+ (cd localclone &&
+ git fetch
+ ) &&
+ compare_refs server master localclone refs/remotes/origin/master &&
+ compare_refs server new localclone refs/remotes/origin/new
+'
+
+test_expect_success 'push when remote has extra refs' '
+ (cd clone &&
+ echo content >>file &&
+ git commit -a -m six &&
+ git push
+ ) &&
+ compare_refs clone master server master
+'
+
+test_expect_success 'push new branch by name' '
+ (cd clone &&
+ git checkout -b new-name &&
+ echo content >>file &&
+ git commit -a -m seven &&
+ git push origin new-name
+ ) &&
+ compare_refs clone HEAD server refs/heads/new-name
+'
+
+test_expect_failure 'push new branch with old:new refspec' '
+ (cd clone &&
+ git push origin new-name:new-refspec
+ ) &&
+ compare_refs clone HEAD server refs/heads/new-refspec
+'
+
+test_done
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p2 0/2] fast-export fixes
From: Felipe Contreras @ 2012-11-28 22:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
Hi,
Here's version 7 part 2; I've dropped all the unnecessary patches, nobody seems
to care about the current brokedness, and I did them only to show that these
are correct, and that remote helpers without marks just don't work. These are
the ones I care about.
Below is a summary of what happens when you apply both patches, and how the new
behavior is obviously correct. All the refs point to the same object.
== before ==
% git fast-export --export--marks=marks master
# exported stuff
% git fast-export --{import,export}-marks=marks test
# nothing
% git fast-export --{import,export}-marks=marks master ^uninteresting
reset refs/heads/uninteresting
from :6
% git fast-export --{import,export}-marks=marks ^uninteresting master ^foo test
reset refs/heads/test
from :6
reset refs/heads/foo
from :6
reset refs/heads/master
from :6
% git fast-export --{import,export}-marks=marks uninteresting..master
== after ==
% git fast-export --export--marks=marks master
# exported stuff
% git fast-export --{import,export}-marks=marks test
reset refs/heads/test
from :6
% git fast-export --{import,export}-marks=marks master ^uninteresting
reset refs/heads/master
from :6
% git fast-export --{import,export}-marks=marks ^uninteresting master ^foo test
reset refs/heads/test
from :6
reset refs/heads/master
from :6
% git fast-export --{import,export}-marks=marks uninteresting..master
reset refs/heads/master
from :6
Changes since v6:
* Drop all the extra patches
* Reorder patches so tests never fail
Felipe Contreras (2):
fast-export: don't handle uninteresting refs
fast-export: make sure updated refs get updated
builtin/fast-export.c | 21 ++++++++++++++-------
t/t5801-remote-helpers.sh | 28 ++++++++++++++++------------
t/t9350-fast-export.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 75 insertions(+), 19 deletions(-)
--
1.8.0.1
^ permalink raw reply
* [PATCH v7 p2 1/2] fast-export: don't handle uninteresting refs
From: Felipe Contreras @ 2012-11-28 22:23 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354141440-26534-1-git-send-email-felipe.contreras@gmail.com>
They have been marked as UNINTERESTING for a reason, lets respect that.
Currently the first ref is handled properly, but not the rest:
% git fast-export master ^uninteresting ^foo ^bar
reset refs/heads/bar
from :0
reset refs/heads/foo
from :0
reset refs/heads/uninteresting
from :0
% git fast-export ^uninteresting ^foo ^bar master
reset refs/heads/master
from :0
reset refs/heads/bar
from :0
reset refs/heads/foo
from :0
Clearly this is wrong; the negative refs should be ignored.
After this patch:
% git fast-export ^uninteresting ^foo ^bar master
# nothing
% git fast-export master ^uninteresting ^foo ^bar
# nothing
And even more, it would only happen if the ref is pointing to exactly
the same commit, but not otherwise:
% git fast-export ^next next
reset refs/heads/next
from :0
% git fast-export ^next next^{commit}
# nothing
% git fast-export ^next next~0
# nothing
% git fast-export ^next next~1
# nothing
% git fast-export ^next next~2
# nothing
The reason this happens is that before traversing the commits,
fast-export checks if any of the refs point to the same object, and any
duplicated ref gets added to a list in order to issue 'reset' commands
after the traversing. Unfortunately, it's not even checking if the
commit is flagged as UNINTERESTING. The fix of course, is to do
precisely that.
However, in order to do it properly we need to get the UNINTERESTING flag
from the command line ref, not from the commit object. Fortunately we
can simply use revs.pending, which contains all the information we need
for get_tags_and_duplicates(), plus the ref flag. This way the rest of
the positive refs will remain untouched; it's only the negative ones
that change in behavior.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/fast-export.c | 11 +++++++----
t/t5801-remote-helpers.sh | 8 ++++++++
t/t9350-fast-export.sh | 30 ++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 191936c..2547e6c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag *tag)
(int)message_size, (int)message_size, message ? message : "");
}
-static void get_tags_and_duplicates(struct object_array *pending,
+static void get_tags_and_duplicates(struct rev_cmdline_info *info,
struct string_list *extra_refs)
{
struct tag *tag;
int i;
- for (i = 0; i < pending->nr; i++) {
- struct object_array_entry *e = pending->objects + i;
+ for (i = 0; i < info->nr; i++) {
+ struct rev_cmdline_entry *e = info->rev + i;
unsigned char sha1[20];
struct commit *commit;
char *full_name;
+ if (e->flags & UNINTERESTING)
+ continue;
+
if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
continue;
@@ -681,7 +684,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
if (import_filename && revs.prune_data.nr)
full_tree = 1;
- get_tags_and_duplicates(&revs.pending, &extra_refs);
+ get_tags_and_duplicates(&revs.cmdline, &extra_refs);
if (prepare_revision_walk(&revs))
die("revision walk setup failed");
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 12ae256..ece8fd5 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -162,4 +162,12 @@ test_expect_failure 'pushing without marks' '
compare_refs local2 HEAD server HEAD
'
+test_expect_success 'push all with existing object' '
+ (cd local &&
+ git branch dup2 master &&
+ git push origin --all
+ ) &&
+ compare_refs local dup2 server dup2
+'
+
test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 1f59862..c8e41c1 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -454,4 +454,34 @@ test_expect_success 'test bidirectionality' '
git fast-import --export-marks=marks-cur --import-marks=marks-cur
'
+cat > expected << EOF
+blob
+mark :13
+data 5
+bump
+
+commit refs/heads/master
+mark :14
+author A U Thor <author@example.com> 1112912773 -0700
+committer C O Mitter <committer@example.com> 1112912773 -0700
+data 5
+bump
+from :12
+M 100644 :13 file
+
+EOF
+
+test_expect_success 'avoid uninteresting refs' '
+ > tmp-marks &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks master > /dev/null &&
+ git tag v1.0 &&
+ git branch uninteresting &&
+ echo bump > file &&
+ git commit -a -m bump &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks ^uninteresting ^v1.0 master > actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.0.1
^ permalink raw reply related
* [PATCH v7 p2 2/2] fast-export: make sure updated refs get updated
From: Felipe Contreras @ 2012-11-28 22:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Felipe Contreras
In-Reply-To: <1354141440-26534-1-git-send-email-felipe.contreras@gmail.com>
When an object has already been exported (and thus is in the marks) it's
flagged as SHOWN, so it will not be exported again, even if in a later
time it's exported through a different ref.
We don't need the object to be exported again, but we want the ref
updated, which doesn't happen.
Since we can't know if a ref was exported or not, let's just assume that
if the commit was marked (flags & SHOWN), the user still wants the ref
updated.
IOW: If it's specified in the command line, it will get updated,
regardless of whether or not the object was marked.
So:
% git branch test master
% git fast-export $mark_flags master
% git fast-export $mark_flags test
Would export 'test' properly.
Additionally, this fixes issues with remote helpers; now they can push
refs whose objects have already been exported, and a few other issues as
well. Update the tests accordingly.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/fast-export.c | 10 +++++++---
t/t5801-remote-helpers.sh | 20 ++++++++------------
t/t9350-fast-export.sh | 15 +++++++++++++++
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 2547e6c..77dffd1 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -526,10 +526,14 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info,
typename(e->item->type));
continue;
}
- if (commit->util)
- /* more than one name for the same object */
+
+ /*
+ * This ref will not be updated through a commit, lets make
+ * sure it gets properly updated eventually.
+ */
+ if (commit->util || commit->object.flags & SHOWN)
string_list_append(extra_refs, full_name)->util = commit;
- else
+ if (!commit->util)
commit->util = full_name;
}
}
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index ece8fd5..f387027 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -63,18 +63,6 @@ test_expect_success 'fetch new branch' '
compare_refs server HEAD local FETCH_HEAD
'
-#
-# This is only needed because of a bug not detected by this script. It will be
-# fixed shortly, but for now lets not cause regressions.
-#
-test_expect_success 'bump commit in server' '
- (cd server &&
- git checkout master &&
- echo content >>file &&
- git commit -a -m four) &&
- compare_refs server HEAD server HEAD
-'
-
test_expect_success 'fetch multiple branches' '
(cd local &&
git fetch
@@ -170,4 +158,12 @@ test_expect_success 'push all with existing object' '
compare_refs local dup2 server dup2
'
+test_expect_success 'push ref with existing object' '
+ (cd local &&
+ git branch dup master &&
+ git push origin dup
+ ) &&
+ compare_refs local dup server dup
+'
+
test_done
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index c8e41c1..9320b4f 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -484,4 +484,19 @@ test_expect_success 'avoid uninteresting refs' '
test_cmp expected actual
'
+cat > expected << EOF
+reset refs/heads/master
+from :14
+
+EOF
+
+test_expect_success 'refs are updated even if no commits need to be exported' '
+ > tmp-marks &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks master > /dev/null &&
+ git fast-export --import-marks=tmp-marks \
+ --export-marks=tmp-marks master > actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.0.1
^ permalink raw reply related
* Re: git-svn: What is --follow-parent / --no-follow-parent for?
From: Sebastian Leske @ 2012-11-28 23:59 UTC (permalink / raw)
To: git
In-Reply-To: <20121120073153.GA340@localhost>
Hi Steven, hi Eric,
thanks for your explanations. I'll try to update my doc patch to include
them.
^ permalink raw reply
* Re: [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Junio C Hamano @ 2012-11-29 0:02 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, Brandon Casey
In-Reply-To: <1353894359-6733-5-git-send-email-drafnel@gmail.com>
Brandon Casey <drafnel@gmail.com> writes:
> -static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
> +static int is_rfc2822_line(const char *buf, int len)
> +{
> + int i;
> +
> + for (i = 0; i < len; i++) {
> + int ch = buf[i];
> + if (ch == ':')
> + break;
> + if (isalnum(ch) || (ch == '-'))
> + continue;
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> +static int is_cherry_pick_from_line(const char *buf, int len)
> +{
> + return (strlen(cherry_picked_prefix) + 41) <= len &&
> + !prefixcmp(buf, cherry_picked_prefix);
> +}
> +
> +static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
> {
> - int ch;
> int hit = 0;
> - int i, j, k;
> + int i, k;
> int len = sb->len - ignore_footer;
> const char *buf = sb->buf;
>
> @@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
> ; /* do nothing */
> k++;
>
> - for (j = 0; i + j < len; j++) {
> - ch = buf[i + j];
> - if (ch == ':')
> - break;
> - if (isalnum(ch) ||
> - (ch == '-'))
> - continue;
> + if (!(is_rfc2822_line(buf+i, k-i) ||
> + is_cherry_pick_from_line(buf+i, k-i)))
> return 0;
> - }
> }
> return 1;
> }
Refactored code looks vastly more readable, but I think the
is_cherry_pick_from_line() function (by the way, shouldn't it be
named is_cherry_picked_from_line()?) shows its ambivalence. It
insists that the line has to be long enough to hold 40-hex object
name plus a closing parenthesis, but it only makes sure that the
prefix matches, without checking if the line has 40-hex object name,
or the object name is immediately followed by a closing parenthesis.
It also does not care if there are other garbage after it.
If the code is trying to be strict to avoid misidentification, then
the check should be tightened (i.e. require the known fixed length,
make sure get_sha1_hex() is happy, 41st byte is a close parenthesis
that is followed by the end of line). If the code is trying to be
more lenient to allow people hand-editing the cherry-picked-from
line that was generated, the check could be loosened (people may
truncate the 40-hex down to 12-hex or something).
I cannot read from this code which one was intended; the code must
make up its mind, whichever way (I do not have a strong preference).
> +test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' '
Is the title of this test appropriate? It looks like it is making
sure we do not add an extra blank line after the existing footer to
me.
> + pristine_detach initial &&
> + sha1=`git rev-parse mesg-with-footer^0` &&
> + git cherry-pick -x -s mesg-with-footer &&
> + cat <<-EOF >expect &&
> + $mesg_with_footer
> + (cherry picked from commit $sha1)
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> + EOF
> + git log -1 --pretty=format:%B >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' '
> + pristine_detach initial &&
> + sha1=`git rev-parse mesg-with-footer-sob^0` &&
> + git cherry-pick -x -s mesg-with-footer-sob &&
> + cat <<-EOF >expect &&
> + $mesg_with_footer_sob
> + (cherry picked from commit $sha1)
> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
> + EOF
> + git log -1 --pretty=format:%B >actual &&
> + test_cmp expect actual
> +'
For people on the sideline, $mesg_with_footer_sob ends with s-o-b by
the same "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" we are adding
here. This is probably a sensible thing to do.
One thing I am not so happy about this whole "(cherry picked from"
thing (and I am again agreeing with Linus who made me change the
default long time ago not to add this line by default) is this. If
you use "cherry-pick -s -x $commit" to transplant a commit from an
unrelated part of the history, you will get the object name in the
resulting commit. But you can do the same in at least two different
ways. The easiest is:
git format-patch -1 --stdout $commit | git am -s3
and a bit closer to what "cherry-pick" does is:
git checkout $commit^0 &&
git rebase --onto @{-1} HEAD^ &&
git checkout -B @{-1}
i.e. rebase the single commit on top of the branch you were on.
In either way, you don't get "cherry picked from", even though you
just did the moral equivalent of it. Also we need to realize that
the first one is essentially what happens all the time here; the "|"
is implemented by the mailing list. And nobody misses "cherry
picked from" to point at the original commit object, which is
useless for the recipient of the resulting commit.
But that is an orthogonal issue.
^ permalink raw reply
* Re: [PATCH 06/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Junio C Hamano @ 2012-11-29 0:35 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, pclouds, Brandon Casey
In-Reply-To: <1353894359-6733-7-git-send-email-drafnel@gmail.com>
Brandon Casey <drafnel@gmail.com> writes:
> +/* Returns 0 for non-conforming footer
Please format it like this:
/*
* Returns 0 for ...
> + * Returns 1 for conforming footer
> + * Returns 2 when sob exists within conforming footer
> + * Returns 3 when sob exists within conforming footer as last entry
> + */
> +static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
> + int ignore_footer)
> {
> int hit = 0;
> - int i, k;
> + int i, k = 0;
This is not wrong per-se, but I do not think it is necessary ('k' is
always initialized at least to 'i' before it gets used). If we need
to do this, I'd prefer to see it done in 1/11 that cleaned up the
containing function as a preparation for this series.
> int len = sb->len - ignore_footer;
> const char *buf = sb->buf;
> + int found_sob = 0;
>
> for (i = len - 1; i > 0; i--) {
> if (hit && buf[i] == '\n')
> @@ -63,14 +70,24 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
> i++;
>
> for (; i < len; i = k) {
> + int found_rfc2822;
> +
> for (k = i; k < len && buf[k] != '\n'; k++)
> ; /* do nothing */
> k++;
>
> - if (!(is_rfc2822_line(buf+i, k-i) ||
> - is_cherry_pick_from_line(buf+i, k-i)))
> + found_rfc2822 = is_rfc2822_line(buf+i, k-i);
Not limited to this place but please have SP around binary operators, i.e.
found_rfc2822 = is_rfc2822_line(buf + i, k - i);
> + if (found_rfc2822 && sob &&
> + !strncasecmp(buf+i, sob->buf, sob->len))
> + found_sob = k;
Are we sure we want strncasecmp() here? I *think* you are trying to
catch "signed-off-By:" and other misspellings, and even though I
know in practice we know "Brandon Casey" and "brandon casey" are the
same person, it still feels somewhat sloppy. Perhaps it is just me.
> +
> + if (!(found_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
> return 0;
> }
> + if (found_sob == i)
> + return 3;
> + if (found_sob)
> + return 2;
> return 1;
> }
>
> @@ -291,7 +308,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> rollback_lock_file(&index_lock);
>
> if (opts->signoff)
> - append_signoff(msgbuf, 0);
> + append_signoff(msgbuf, 0, 0);
>
> if (!clean) {
> int i;
> @@ -547,7 +564,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
> }
>
> if (opts->record_origin) {
> - if (!has_conforming_footer(&msgbuf, 0))
> + if (!has_conforming_footer(&msgbuf, NULL, 0))
> strbuf_addch(&msgbuf, '\n');
> strbuf_addstr(&msgbuf, cherry_picked_prefix);
> strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
> @@ -1074,9 +1091,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
> return pick_commits(todo_list, opts);
> }
>
> -void append_signoff(struct strbuf *msgbuf, int ignore_footer)
> +void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
> {
> struct strbuf sob = STRBUF_INIT;
> + int has_footer = 0;
> int i;
>
> strbuf_addstr(&sob, sign_off_header);
> @@ -1085,10 +1103,11 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
> strbuf_addch(&sob, '\n');
> for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
> ; /* do nothing */
> - if (prefixcmp(msgbuf->buf + i, sob.buf)) {
> - if (!i || !has_conforming_footer(msgbuf, ignore_footer))
> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
> - strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
> - }
> + if (!i || !(has_footer =
> + has_conforming_footer(msgbuf, &sob, ignore_footer)))
> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
> + if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
> + strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
> + sob.buf, sob.len);
Avoid assignment inside if () conditional. It is not immediately
obvious what value is compared against 3 in the second one, as the
above makes it appear as if has_footer is uninitialized when i is
zero.
Thanks.
^ permalink raw reply
* Re: [PATCH v7 p2 1/2] fast-export: don't handle uninteresting refs
From: Max Horn @ 2012-11-29 1:16 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1354141440-26534-2-git-send-email-felipe.contreras@gmail.com>
On 28.11.2012, at 23:23, Felipe Contreras wrote:
> They have been marked as UNINTERESTING for a reason, lets respect that.
>
> Currently the first ref is handled properly, but not the rest:
>
> % git fast-export master ^uninteresting ^foo ^bar
All these refs are assumed to point to the same object, right? I think it would be better if the commit message stated that explicitly. To make up for the lost space, you could then get rid of one of the four refs, I think three are sufficient to drive the message home ;-).
<snip>
> The reason this happens is that before traversing the commits,
> fast-export checks if any of the refs point to the same object, and any
> duplicated ref gets added to a list in order to issue 'reset' commands
> after the traversing. Unfortunately, it's not even checking if the
> commit is flagged as UNINTERESTING. The fix of course, is to do
> precisely that.
Hm... So this might be me being a stupid n00b (I am not yet that familiar with the internal rep of things in git and all...)... but I found the "precisely that" par very confusing, because right afterwards, you say:
>
> However, in order to do it properly we need to get the UNINTERESTING flag
> from the command line ref, not from the commit object.
So this sounds like you are saying "we do *precisely* that, except we don't, because it is more complicated, so we actually don't do this *precisely*, just manner of speaking..."
Some details here are beyond my knowledge, I am afraid, so I have to resort to guess: In particular it is not clear to me why the "however" part pops up: Reading it makes it sound as if the commit object also carries an UNINTERESTING flag, but we can't use it because of some reason (perhaps it doesn't have the semantics we need?), so we have to look at revs.pending instead. Right? Wrong? Or is it because the commit objects actually do *not* carry the UNINTERESTING bits, hence we need to look at revs.pending. Or is it due to yet another reason?
I would find it helpful if that could be clarified. E.g. like so:
"The fix is to add such a check. However, we cannot just use the UNINTERESTING flag of the commit object, because INSERT-REASON."
or
"The fix is to add such a check. However, the commit object does not contain the UNINTERESTING flag directly."
or something.
Anyway, other than these nitpicky questions, this whole thing looks very logical to me, description and code alike. I also played around with tons of "fast-export" invocations, with and without this patch, and it seems to do what the description says. Finally, I went to the various long threads discussion prior versions of this patch, in particular those starting at
http://thread.gmane.org/gmane.comp.version-control.git/208725
and
http://thread.gmane.org/gmane.comp.version-control.git/209355/focus=209370
These contained some concerns. Sadly, several of those discussions ultimately degenerated into not-so-pleasant exchanges :-(, and my impression is that as a result some people are not so inclined to comment on these patches anymore at all. Which is a pity :-(. But overall, it seems this patch makes nothing worse, but fixes some things; and it is simple enough that it shouldn't make future improvements harder.
So *I* at least am quite happy with this, it helps me! My impression is that Felipe's latest patch addresses most concerns people raised by means of an improved description. I couldn't find any in those threads that I feel still applies -- but of course those people should speak for themselves, I am simply afraid they don't want to be part of this anymore :-(.
Still, for what little it might be worth, I think this patch is good and a real improvement. I hope it can be merged soon.
Cheers,
Max
> Fortunately we
> can simply use revs.pending, which contains all the information we need
> for get_tags_and_duplicates(), plus the ref flag. This way the rest of
> the positive refs will remain untouched; it's only the negative ones
> that change in behavior.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> builtin/fast-export.c | 11 +++++++----
> t/t5801-remote-helpers.sh | 8 ++++++++
> t/t9350-fast-export.sh | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 191936c..2547e6c 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -474,18 +474,21 @@ static void handle_tag(const char *name, struct tag *tag)
> (int)message_size, (int)message_size, message ? message : "");
> }
>
> -static void get_tags_and_duplicates(struct object_array *pending,
> +static void get_tags_and_duplicates(struct rev_cmdline_info *info,
> struct string_list *extra_refs)
> {
> struct tag *tag;
> int i;
>
> - for (i = 0; i < pending->nr; i++) {
> - struct object_array_entry *e = pending->objects + i;
> + for (i = 0; i < info->nr; i++) {
> + struct rev_cmdline_entry *e = info->rev + i;
> unsigned char sha1[20];
> struct commit *commit;
> char *full_name;
>
> + if (e->flags & UNINTERESTING)
> + continue;
> +
> if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
> continue;
>
> @@ -681,7 +684,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
> if (import_filename && revs.prune_data.nr)
> full_tree = 1;
>
> - get_tags_and_duplicates(&revs.pending, &extra_refs);
> + get_tags_and_duplicates(&revs.cmdline, &extra_refs);
>
> if (prepare_revision_walk(&revs))
> die("revision walk setup failed");
> diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
> index 12ae256..ece8fd5 100755
> --- a/t/t5801-remote-helpers.sh
> +++ b/t/t5801-remote-helpers.sh
> @@ -162,4 +162,12 @@ test_expect_failure 'pushing without marks' '
> compare_refs local2 HEAD server HEAD
> '
>
> +test_expect_success 'push all with existing object' '
> + (cd local &&
> + git branch dup2 master &&
> + git push origin --all
> + ) &&
> + compare_refs local dup2 server dup2
> +'
> +
> test_done
> diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
> index 1f59862..c8e41c1 100755
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -454,4 +454,34 @@ test_expect_success 'test bidirectionality' '
> git fast-import --export-marks=marks-cur --import-marks=marks-cur
> '
>
> +cat > expected << EOF
> +blob
> +mark :13
> +data 5
> +bump
> +
> +commit refs/heads/master
> +mark :14
> +author A U Thor <author@example.com> 1112912773 -0700
> +committer C O Mitter <committer@example.com> 1112912773 -0700
> +data 5
> +bump
> +from :12
> +M 100644 :13 file
> +
> +EOF
> +
> +test_expect_success 'avoid uninteresting refs' '
> + > tmp-marks &&
> + git fast-export --import-marks=tmp-marks \
> + --export-marks=tmp-marks master > /dev/null &&
> + git tag v1.0 &&
> + git branch uninteresting &&
> + echo bump > file &&
> + git commit -a -m bump &&
> + git fast-export --import-marks=tmp-marks \
> + --export-marks=tmp-marks ^uninteresting ^v1.0 master > actual &&
> + test_cmp expected actual
> +'
> +
> test_done
> --
> 1.8.0.1
>
> --
> 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
* Please pull l10n updates for 1.8.1 round 1
From: Jiang Xin @ 2012-11-29 1:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Peter Krefting, Trần Ngọc Quân,
Nguyễn Thái Ngọc Duy
Hi, Junio
The following changes since commit 2d242fb3fc19fc9ba046accdd9210be8b9913f64:
Update draft release notes for 1.8.1 (2012-11-21 13:32:58 -0800)
are available in the git repository at:
git://github.com/git-l10n/git-po.git master
for you to fetch changes up to 647d5183b8dc36b38d19c7a3f388108f245b11d3:
l10n: Update Swedish translation (1975t0f0u) (2012-11-23 08:59:11 +0100)
----------------------------------------------------------------
Jiang Xin (1):
l10n: Update git.pot (14 new, 3 removed messages)
Peter Krefting (1):
l10n: Update Swedish translation (1975t0f0u)
Tran Ngoc Quan (1):
l10n: vi.po: update to git-v1.7.12-437-g1084f
po/git.pot | 1224 ++++++++++++++++++++++++----------------------
po/sv.po | 1246 +++++++++++++++++++++++++----------------------
po/vi.po | 1597 ++++++++++++++++++++++++++++++------------------------------
3 files changed, 2097 insertions(+), 1970 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2012-11-29 1:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Casey, git@vger.kernel.org, pclouds@gmail.com
In-Reply-To: <7v8v9ltgxn.fsf@alter.siamese.dyndns.org>
On 11/28/2012 4:02 PM, Junio C Hamano wrote:
> Brandon Casey <drafnel@gmail.com> writes:
>
>> -static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
>> +static int is_rfc2822_line(const char *buf, int len)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < len; i++) {
>> + int ch = buf[i];
>> + if (ch == ':')
>> + break;
>> + if (isalnum(ch) || (ch == '-'))
>> + continue;
>> + return 0;
>> + }
>> +
>> + return 1;
>> +}
>> +
>> +static int is_cherry_pick_from_line(const char *buf, int len)
>> +{
>> + return (strlen(cherry_picked_prefix) + 41) <= len &&
>> + !prefixcmp(buf, cherry_picked_prefix);
>> +}
>> +
>> +static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
>> {
>> - int ch;
>> int hit = 0;
>> - int i, j, k;
>> + int i, k;
>> int len = sb->len - ignore_footer;
>> const char *buf = sb->buf;
>>
>> @@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
>> ; /* do nothing */
>> k++;
>>
>> - for (j = 0; i + j < len; j++) {
>> - ch = buf[i + j];
>> - if (ch == ':')
>> - break;
>> - if (isalnum(ch) ||
>> - (ch == '-'))
>> - continue;
>> + if (!(is_rfc2822_line(buf+i, k-i) ||
>> + is_cherry_pick_from_line(buf+i, k-i)))
>> return 0;
>> - }
>> }
>> return 1;
>> }
>
> Refactored code looks vastly more readable, but I think the
> is_cherry_pick_from_line() function (by the way, shouldn't it be
> named is_cherry_picked_from_line()?
Yes.
> ) shows its ambivalence. It
> insists that the line has to be long enough to hold 40-hex object
> name plus a closing parenthesis, but it only makes sure that the
> prefix matches, without checking if the line has 40-hex object name,
> or the object name is immediately followed by a closing parenthesis.
> It also does not care if there are other garbage after it.
>
> If the code is trying to be strict to avoid misidentification, then
> the check should be tightened (i.e. require the known fixed length,
> make sure get_sha1_hex() is happy, 41st byte is a close parenthesis
> that is followed by the end of line). If the code is trying to be
> more lenient to allow people hand-editing the cherry-picked-from
> line that was generated, the check could be loosened (people may
> truncate the 40-hex down to 12-hex or something).
>
> I cannot read from this code which one was intended; the code must
> make up its mind, whichever way (I do not have a strong preference).
The intention was a stricter-type match, but implemented somewhat
lazily. Part of me doesn't think that anyone should need to modify
the string that 'cherry-pick -x' adds and that a strict match is
appropriate. The other part of me doesn't want to reject a line that
looks like "(cherry picked from ...)" line that has a trimmed sha1
inside.
I think I'll submit a somewhat loosened check.
>> +test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' '
>
> Is the title of this test appropriate? It looks like it is making
> sure we do not add an extra blank line after the existing footer to
> me.
It's really just part of the series of tests that checks for correct
operation when "the last sob doesn't match committer". This test
has a few elements in it:
* existing sob footer contains the committer's sob, but not last.
ensure that we don't refrain from appending a sob just because
one already exists in the footer.
* ensure extra blank isn't inserted after existing footer and
"(cherry picked from..."
* ensure a blank isn't inserted between "(cherry picked from..."
and new sob
The title of the test is me trying to fit "correctly adds \"(cherry
picked from...\" and sob when footer contains committer's sob but
not last" within a single line.
>> + pristine_detach initial &&
>> + sha1=`git rev-parse mesg-with-footer^0` &&
>> + git cherry-pick -x -s mesg-with-footer &&
>> + cat <<-EOF >expect &&
>> + $mesg_with_footer
>> + (cherry picked from commit $sha1)
>> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
>> + EOF
>> + git log -1 --pretty=format:%B >actual &&
>> + test_cmp expect actual
>> +'
>> +
>
>> +test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' '
>> + pristine_detach initial &&
>> + sha1=`git rev-parse mesg-with-footer-sob^0` &&
>> + git cherry-pick -x -s mesg-with-footer-sob &&
>> + cat <<-EOF >expect &&
>> + $mesg_with_footer_sob
>> + (cherry picked from commit $sha1)
>> + Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
>> + EOF
>> + git log -1 --pretty=format:%B >actual &&
>> + test_cmp expect actual
>> +'
>
> For people on the sideline, $mesg_with_footer_sob ends with s-o-b by
> the same "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" we are adding
> here. This is probably a sensible thing to do.
>
> One thing I am not so happy about this whole "(cherry picked from"
> thing (and I am again agreeing with Linus who made me change the
> default long time ago not to add this line by default) is this. If
> you use "cherry-pick -s -x $commit" to transplant a commit from an
> unrelated part of the history, you will get the object name in the
> resulting commit. But you can do the same in at least two different
> ways. The easiest is:
>
> git format-patch -1 --stdout $commit | git am -s3
>
> and a bit closer to what "cherry-pick" does is:
>
> git checkout $commit^0 &&
> git rebase --onto @{-1} HEAD^ &&
> git checkout -B @{-1}
>
> i.e. rebase the single commit on top of the branch you were on.
>
> In either way, you don't get "cherry picked from", even though you
> just did the moral equivalent of it. Also we need to realize that
> the first one is essentially what happens all the time here; the "|"
> is implemented by the mailing list. And nobody misses "cherry
> picked from" to point at the original commit object, which is
> useless for the recipient of the resulting commit.
>
> But that is an orthogonal issue.
Are you suggesting that people shouldn't use 'cherry-pick -x' at all?
I agree that it is useless to use -x when the commit that will be
referenced will not be available as part of the permanent history
of the project.
I do however think it is useful to add a reference to the original
commit when back- or forward- porting a commit. I think it adds
a little visibility to the commit to say that it was not originally
implemented on top of the base it is built on. I find it helpful
to examine the original implementation (the referenced commit) when
verifying the correctness or attempting to understand the ported
commit.
-Brandon
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH 00/11] alternative unify appending of sob
From: Junio C Hamano @ 2012-11-29 5:56 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Brandon Casey, git, Brandon Casey
In-Reply-To: <CACsJy8AfaETfikvB+zON4iiX2HBehJg-xX2GGmKYHueX8rv3qw@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> On Mon, Nov 26, 2012 at 8:45 AM, Brandon Casey <drafnel@gmail.com> wrote:
>
>> I've integrated Duy's series with a few minor tweaks. I added a couple
>> of additional tests to t4014 and corrected one of the tests which had
>> incorrect behavior. I think his sign-off's should still be valid, so I
>> kept them in. Sorry that I've been slow, and now the two of us are stepping
>> on each other's toes, but Duy please take a look and let me know if there's
>> anything you dislike.
>
> I'm still not sure whether format-patch should follow cherry-pick's
> rule in appending sob. If it does, it probably makes more sense to fix
> the sequencer.c code then delete log-tree.c (not fixes on log-tree.c
> then delete it). I see that your changes pass all the new tests I
> added in format-patch so sequencer.c is probably good enough,
> log-tree.c changes are probably not needed. Feel free take over the
> series :-)
After reading the series over, I agree with the above.
Patch #9 that fixes the copy in log-tree.c only to discard it in
patch #11 does not seem to be the best organization of the series.
Instead, perhaps we can salvage the tests in patch #9 (but mark
failing ones as expecting failure) without updating the one in
log-tree.c, adjust prototype in patch #10 (still broken in
log-tree.c) to avoid having to make changes to the callers in patch
#11, and then conclude the series with #11?
Other than the code in patches #06 and #07 that I already commented
on, i.e. assignments in if () condition that make it harder to
follow the logic, I did not find anything majorly objectionable in
the series.
Thanks for a pleasant read.
^ permalink raw reply
* Re: Please pull l10n updates for 1.8.1 round 1
From: Junio C Hamano @ 2012-11-29 5:59 UTC (permalink / raw)
To: Jiang Xin
Cc: Git List, Peter Krefting, Trần Ngọc Quân,
Nguyễn Thái Ngọc Duy
In-Reply-To: <CANYiYbHfhBXbLEEKdpJkhp+wY9-AJ1JkjfO62W3c+dm8qpk7_g@mail.gmail.com>
Thanks.
^ permalink raw reply
* reposurgeon now writes Subversion repositories
From: Eric S. Raymond @ 2012-11-29 5:59 UTC (permalink / raw)
To: dev, git
This is something that probably doesn't happen very often -
cross-posting to the Subversion and git dev lists that is on-topic for
both :-).
The repo head version of reposurgeon can now write Subversion
repositories from its common git-import-stream-based representation of
repository histories, as well as reading them in. This joins full
support for git, hg, and bzr; it means that in theory reposurgeon
could now be used to move revision histories from these systems to
Subversion, as well as the other way around.
(For those of you who have been living under a rock, reposurgeon is a
multi-VCS surgery and conversion tool. Since 2.x it does a more
intelligent job of lifting from Subversion to anything else than any
other tool I know of. Much more at <http://www.catb.org/esr/reposurgeon/>.)
Presently, writing (as opposed to reading) Subversion repos is more of
a stunt than a real production technique, and may always remain so.
It has serious limitations. I am posting because I think the details
of those limitations will be of some technical interest to both
Subversion and git developers.
Indented paragraphs is the documentation from reposurgeon's manual
page. I have added some further notes.
In summary, Subversion repository histories do not round-trip through
reposurgeon editing. File content changes are preserved but some
metadata is unavoidably lost. Furthermore, writing out a DVCS history
in Subversion also loses significant portions of its metadata.
Writing a Subversion repository or dump stream discards author
information, the committer's name, and the hostname part of the commit
address; only the commit timestamp and the local part of the
committer's email address are preserved, the latter becoming the
Subversion author field. However, reading a Subversion repository and
writing it out again will preserve the author fields.
Subversion's metadata doesn't have separate author and committer
properties, and doesn't store anything but a Unix user ID as
attribution. I don't see any way around this.
Import-stream timestamps have 1-second granularity. The subsecond
parts of Subversion commit timestamps will be lost on their way through
reposurgeon.
Unavoidable in moving from Subversion to git import streams, and one
of two places where git's data model requires us to throw away
information.
However, I think I could preserve this information in a
Subversion-to-Subversion editing scenario by storing the incoming
timestamps as floats and only truncating them on import-stream output,
leaving the subseconds in place for Subversion output.
Empty directories aren't represented in import streams. Consequently,
reading and writing Subversion repositories preserves file content,
but not empty directories. It is also not guaranteed that after
editing a Subverson repository that the sequence of directory
creations and deletions relative to other operations will be
identical; the only guarantee is that enclosing directories will be
created before any files in them are.
When reading a Subversion repository, reposurgeon discards the special
directory-copy nodes associated with branch creations. These can't be
recreated if and when the repository is written back out to
Subversion; rather, each branch copy node from the original translates
into a branch creation plus the first set of file modifications on the
branch.
In theory, I could relax the rules of reposurgeon's internal
representation so that empty directory-creation and deletion nodes are
not discarded at read time but only when outputting a git event stream.
That would bring Subversion repositories closer to round-tripping, but
not get all the way there. One problem is botched branch copies -
directory copies with cp(1) followed by Subversion add operations.
This is not an uncommon malformation; reposurgeon takes it in stride,
treating these as though they had been real branch copies and
simplifying the backlinks appropriately.
When reading a Subversion repository, reposurgeon also automatically
breaks apart mixed-branch commits.
It has to. These just can't be represented in the import-stream model of
branching.
Because of the preceding two points, it is not guaranteed that
even revision numbers will be stable when a Subversion repository
is read in and then written out!
So not only can Subversion repos fail to round-trip exactly, in the
presence of lots of branch copies and mixed-branch commits the
relationship between the read-in and written out revision numbers
could get pretty unpredictable.
Subversion repositories are always written with a standard
(trunk/tags/branches) layout. Thus, a repository with a nonstandard
shape that has been analyzed by reposurgeon won't be written out with
the same shape.
In particular, this means linear Subversion repositories with no trunk
(an organization some smaller projects used to use and might still)
will turn into branchy repos with trunk on the way out.
Subversion has a concept of "flows"; that is, named segments of
history corresponding to files or directories that are created when
the path is added, cloned when the path is copied, and deleted when
the path is deleted. This information is not preserved in import
streams or the internal representation that reposurgeon uses. Thus,
after editing, the flow boundaries of a Subversion history may be
arbitrarily changed.
This is me being obsessive about documenting the details. I think it
is doubtful that most Subversion users even know flows exist.
Bugs: Presently, writing out a history to a Subversion repository does
not create mergeinfo properties representing branch merges. It also
loses all information about lightweight tags (though annotated tags
are turned into Subversion-style directory copies). These bugs will
probably be fixed in future reposurgeon releases.
I'm also not sure the present code handles branchiness exactly right.
My next task is to write a test suite for this new feature.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
The Constitution is not neutral. It was designed to take the
government off the backs of the people.
-- Justice William O. Douglas
^ permalink raw reply
* Re: Millisecond precision in timestamps?
From: Eric S. Raymond @ 2012-11-29 6:16 UTC (permalink / raw)
To: Steven Michalske
Cc: Thomas Berg, Felipe Contreras, Junio C Hamano, Jeff King,
Shawn Pearce, git
In-Reply-To: <E4C993F4-B7A4-4CB6-A9EA-BFE98BE3A381@gmail.com>
Steven Michalske <smichalske@gmail.com>:
> Would having arbitrary key value pairs be useful in the git data
> model? We could have ones that affect the sha1 and others that are
> transparent.
My tools would have several uses for these.
bzr's implementation of import streams has a commit-propperties extension.
reposurgeon can read, display.. and manipulate these key/value pairs.
I do wish they were in core git.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Millisecond precision in timestamps?
From: Junio C Hamano @ 2012-11-29 7:11 UTC (permalink / raw)
To: Steven Michalske
Cc: Thomas Berg, Felipe Contreras, Jeff King, Shawn Pearce,
Eric Raymond, git
In-Reply-To: <E4C993F4-B7A4-4CB6-A9EA-BFE98BE3A381@gmail.com>
Steven Michalske <smichalske@gmail.com> writes:
> Would having arbitrary key value pairs be useful in the git data
> model?
My answer to the question is that it is harmful to the data model,
but the benefit of going against the data model _may_ outweigh the
downside. It is all relative.
The first of very small number of principles of the git data model
is that the object name is derived solely from the contents, hence
we can tell two different things apart with object names without
looking at object contents.
This is actively broken by adding "junk" fields left and right.
Adding arbitrary pieces of data that are optional (and largely
ignored by core operations) means you can record objects with
essentially the same contents under different object names, so
object names no longer help us telling two moral-equivalent objects
apart.
But "if two objects have different names, they are not the same"
does not have to be the only and the absolute truth in all contexts;
the world is not so black and white. Depending on the application
and the context, you may want to treat two things that are not the
same as equivalents.
For example, at the blob level, two blob objects that store the same
text (say, one original and the other typed in double-space) would
be different objects and have different object names, but you may
want to treat them as "equivalents" (not same but interchangeable),
by applying textconv filter to normalize their contents when
comparing them. We still keep the "two objects with different names
are different" principle, but at the same time, allow users to treat
them as equivalent in specific contexts.
Introducing a hack to exclude selective "junk" fields from hashing
done for object name computation is not a solution and is out of the
question, but that does not necessarily mean that commit objects
should never be extended with new types of header fields. When a
commit object is made with a "junk" field, it will have a name that
is different from the one it would get without the "junk" field, but
the benefit of the ability to store extra data _may_ outweigh the
downside of having to always compare the contents of two objects
with different names to find out that they are different but
equivalent.
^ permalink raw reply
* Re: Millisecond precision in timestamps?
From: Felipe Contreras @ 2012-11-29 7:22 UTC (permalink / raw)
To: Junio C Hamano
Cc: Steven Michalske, Thomas Berg, Jeff King, Shawn Pearce,
Eric Raymond, git
In-Reply-To: <7va9u0sx26.fsf@alter.siamese.dyndns.org>
On Thu, Nov 29, 2012 at 8:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Steven Michalske <smichalske@gmail.com> writes:
>
>> Would having arbitrary key value pairs be useful in the git data
>> model?
>
> My answer to the question is that it is harmful to the data model,
> but the benefit of going against the data model _may_ outweigh the
> downside. It is all relative.
If git doesn't provide the capability, people will keep using the
commit message to store that extra information, which I would think is
even more harmful. An standard 'commit-extra' note or something would
help deal with that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* RE: Please pull l10n updates for 1.8.1 round 1
From: Trần Ngọc Quân @ 2012-11-29 7:28 UTC (permalink / raw)
To: 'Jiang Xin', 'Junio C Hamano'
Cc: 'Git List', 'Peter Krefting',
'Nguyễn Thái Ngọc Duy'
In-Reply-To: <CANYiYbHfhBXbLEEKdpJkhp+wY9-AJ1JkjfO62W3c+dm8qpk7_g@mail.gmail.com>
Hello JX,
You missing pull from my repo (2 commits instead of one, v1.7 and v1.8): dcc52a0449c7ee10690e23152e63b9798f8a332f
$ git log -n 2
commit dcc52a0449c7ee10690e23152e63b9798f8a332f
Author: Tran Ngoc Quan <vnwildman@gmail.com>
Date: Sat Nov 24 07:37:35 2012 +0700
l10n: vi.po: Update follow git-v1.8.0-273-g2d242
Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>
commit 131fa518f10521b4a534863331decbfef2875f24
Author: Tran Ngoc Quan <vnwildman@gmail.com>
Date: Wed Oct 31 08:19:59 2012 +0700
l10n: vi.po: update to git-v1.7.12-437-g1084f
* updated all new messages (1967t0f0u)
* make quote become more good-looking
Signed-off-by: Tran Ngoc Quan <vnwildman@gmail.com>
https://github.com/vnwildman/git.git master
Thanks,
Trần Ngọc Quân
-----Original Message-----
From: Jiang Xin [mailto:worldhello.net@gmail.com]
Sent: Thursday, November 29, 2012 8:19 AM
To: Junio C Hamano
Cc: Git List; Peter Krefting; Trần Ngọc Quân; Nguyễn Thái Ngọc Duy
Subject: Please pull l10n updates for 1.8.1 round 1
Hi, Junio
The following changes since commit 2d242fb3fc19fc9ba046accdd9210be8b9913f64:
Update draft release notes for 1.8.1 (2012-11-21 13:32:58 -0800)
are available in the git repository at:
git://github.com/git-l10n/git-po.git master
for you to fetch changes up to 647d5183b8dc36b38d19c7a3f388108f245b11d3:
l10n: Update Swedish translation (1975t0f0u) (2012-11-23 08:59:11 +0100)
----------------------------------------------------------------
Jiang Xin (1):
l10n: Update git.pot (14 new, 3 removed messages)
Peter Krefting (1):
l10n: Update Swedish translation (1975t0f0u)
Tran Ngoc Quan (1):
l10n: vi.po: update to git-v1.7.12-437-g1084f
po/git.pot | 1224 ++++++++++++++++++++++++----------------------
po/sv.po | 1246 +++++++++++++++++++++++++----------------------
po/vi.po | 1597 ++++++++++++++++++++++++++++++------------------------------
3 files changed, 2097 insertions(+), 1970 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Re: reposurgeon now writes Subversion repositories
From: Daniel Shahaf @ 2012-11-29 7:58 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: dev, git
In-Reply-To: <20121129055946.2D7B84065F@snark.thyrsus.com>
Eric S. Raymond wrote on Thu, Nov 29, 2012 at 00:59:45 -0500:
> In summary, Subversion repository histories do not round-trip through
> reposurgeon editing. File content changes are preserved but some
> metadata is unavoidably lost. Furthermore, writing out a DVCS history
> in Subversion also loses significant portions of its metadata.
>
> Writing a Subversion repository or dump stream discards author
> information, the committer's name, and the hostname part of the commit
> address; only the commit timestamp and the local part of the
> committer's email address are preserved, the latter becoming the
> Subversion author field. However, reading a Subversion repository and
> writing it out again will preserve the author fields.
>
> Subversion's metadata doesn't have separate author and committer
> properties, and doesn't store anything but a Unix user ID as
> attribution. I don't see any way around this.
You're not fully informed, then.
1) svn:author revprops can contain any UTF-8 string. They are not
restricted to Unix user id's. (For example, they can contain full
names, if the administrator so chooses.)
2) You can define custom revision properties. In your case, the easiest
way would be to set an reposurgeon:author property, alongside the
svn:author property.
You might also seek community consensus to reserve an svn:foo name for
the "original author" property --- perhaps svn:original-author --- so
that reposurgeon and other git->svn tools can interoperate in the way
they transfer the "original author" information.
I note that one can set revision properties at commit time:
svn commit -m logmsg --with-revprop svn:original-author="Patch Submitter <foo@bar.example>"
> Empty directories aren't represented in import streams. Consequently,
> reading and writing Subversion repositories preserves file content,
> but not empty directories. It is also not guaranteed that after
> editing a Subverson repository that the sequence of directory
> creations and deletions relative to other operations will be
> identical; the only guarantee is that enclosing directories will be
> created before any files in them are.
How does reposurgeon handle empty directories with (node) properties?
% svnadmin create r
% svnmucc -mm -U file://$PWD/r mkdir foo propset k v foo
> Subversion has a concept of "flows"; that is, named segments of
> history corresponding to files or directories that are created when
> the path is added, cloned when the path is copied, and deleted when
> the path is deleted. This information is not preserved in import
> streams or the internal representation that reposurgeon uses. Thus,
> after editing, the flow boundaries of a Subversion history may be
> arbitrarily changed.
>
> This is me being obsessive about documenting the details. I think it
> is doubtful that most Subversion users even know flows exist.
>
I think you're saying that adds might turn into copies, and vice-versa.
That is something users would notice --- it is certainly exposed in the
UI --- even though node-id's are not exposed to clients.
>
Cheers
Daniel
^ permalink raw reply
* [PATCH] completion: fix warning for zsh
From: Felipe Contreras @ 2012-11-29 8:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
Otherwise the user might get something like:
git-completion.sh:2466: command not found: compdef
If this script is loaded before compinit. The script would work either
way, but let's not be more annoying to the user.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
contrib/completion/git-completion.bash | 2 ++
1 file changed, 2 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index af13fcc..0b77eb1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2404,6 +2404,8 @@ __gitk_main ()
if [[ -n ${ZSH_VERSION-} ]]; then
echo "WARNING: this script is deprecated, please see git-completion.zsh" 1>&2
+ autoload -U +X compinit && compinit
+
__gitcomp ()
{
emulate -L zsh
--
1.8.0.1
^ permalink raw reply related
* [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)
From: Johannes Sixt @ 2012-11-29 8:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Antoine Pelisse
In-Reply-To: <1354051310-29093-1-git-send-email-gitster@pobox.com>
From: Johannes Sixt <j6t@kdbg.org>
The earlier change 74faaa16 (Fix "git diff --stat" for interesting - but
empty - file changes) needed to change the count of differing files
because the executable-bit changes of two empty files are now counted.
On file systems that do not record the executable bit, however, the old
file count was actually correct (and the updated tests fail) because the
mode change cannot be diagnosed by looking at the file system alone.
Change the mode not only on the file system, but also in the index;
compare the new state against the commit, so that the tests do not depend
on the file system's ability to record the executable bit, when possible.
The exception is the test for unmerged entries, which does depend on the
file system; we have to skip it.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 11/27/2012 22:21, schrieb Junio C Hamano:
> It turns out that there are at least two bugs in the diffstat
> counting code. This series comes on top of the earlier 74faaa1 (Fix
> "git diff --stat" for interesting - but empty - file changes,
> 2012-10-17) to fix them.
The tests still fail on Windows. I am not sure whether there is a
difference in comparing the file system against the index or a commit.
If there is, then the updated tests might not test the same thing.
-- Hannes
t/t4049-diff-stat-count.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 37f50cd..9e29e71 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup' '
test_expect_success 'limit output to 2 (simple)' '
git reset --hard &&
- chmod +x c d &&
+ test_chmod +x c d &&
echo a >a &&
echo b >b &&
cat >expect <<-\EOF
@@ -24,13 +24,13 @@ test_expect_success 'limit output to 2 (simple)' '
...
4 files changed, 2 insertions(+)
EOF
- git diff --stat --stat-count=2 >actual &&
+ git diff --stat --stat-count=2 HEAD >actual &&
test_i18ncmp expect actual
'
test_expect_success 'binary changes do not count in lines' '
git reset --hard &&
- chmod +x c d &&
+ test_chmod +x c d &&
echo a >a &&
echo b >b &&
cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
@@ -40,11 +40,11 @@ test_expect_success 'binary changes do not count in lines' '
...
4 files changed, 2 insertions(+)
EOF
- git diff --stat --stat-count=2 >actual &&
+ git diff --stat --stat-count=2 HEAD >actual &&
test_i18ncmp expect actual
'
-test_expect_success 'exclude unmerged entries from total file count' '
+test_expect_success FILEMODE 'exclude unmerged entries from total file count' '
git reset --hard &&
echo a >a &&
echo b >b &&
--
1.8.0.1.1524.gaf6675c
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox