* Re: Evaluation of ref-api branch status
From: Junio C Hamano @ 2011-12-05 18:26 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git discussion list
In-Reply-To: <4EDAB62E.5070204@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Now that 1.7.8 is out, I wanted to figure out the status of the
> remaining ref-api changes that are in flight, including the differences
> between between my tree and yours.
I understand that the ultimate and largest objective of these series is to
make sure that the performance does not suffer from very many negative
lookups on the refs/replace hierarchy (which almost always is empty in a
sane repository), and I do want to see that happen. I also think it is
good that the series tries to make sure that the various codepaths we
create new refs do not let the user accidentally create refs with bad
names and/or contents.
When we see a questionable ref that is _already_ in the respository,
however, we may warn but it is wrong to declare the repository to be
broken and refuse to work. That would make it cumbersome or impossible to
even _fix_ such breakage. Regressions of that kind wer in the earlier part
of the series already in 1.7.8 and it was rather unpleasant having to
hotfix them. As our test suite does not deliberately create these
questionable refs and/or try to use them (and I personally do not in my
regular workflow either), I am afraid that it is rather hard to realize
what kind of refnames are what we intended to forbid even from earlier
days but did not _bother_ to check and enforce the rule against so far,
but are forbidden by the updated code, until we unleash the new logic to
the end users with various third party tools that created them [*1*]. I
would want to see us get this part right and solid, and include it in
1.7.9.
It would be nice if we can include also the bits to read hierarchies
lazily, but as they come on top of your B & C, it may end up in 1.7.10 and
I do not mind it at all. Reference resolution is one of the central things
in the user experience, and we cannot afford to ship anything that is
half-done-and-mostly-works over slow-but-correct.
Alternatively you _could_ swap the order of your B & C and D and try to
have your D early while giving B & C more time to cook, but I suspect the
order you chose would be better in the longer term.
> I understand that "next" will soon be re-rolled. Will the re-roll be
> based on the current "pu", or will you start from scratch?
As to the two quickfix patches that are on two remaining topics from you
in my tree, I did them merely as a short-term band-aid. I was expecting,
after 1.7.8 when we eject the topics out of 'next', that they will be
rebased on top of 'master' that already contains a proper fix before these
topics started touching the same codepath.
My reading of your summary suggests that it would be easiest to drop the
three mh/ref-api* topics from my tree, especially the 'refs: loosen
over-strict "format" check' band-aid patches, and re-queue a re-roll from
you.
Thanks.
[Footnote]
*1* Trying to be lenient when reading and being strict when writing as the
general principle would be the safe and sane way forward, and making sure
that we warn *loudly* when we think we are merely being lenient (i.e. we
think we found a bad ref with questionable name and/or contents) would be
a good way to catch our mistakes early. E.g. ".git/config does not record
a correct object name" glitch was caught only because you added the
warning so while it was painful to hotfix that late in the cycle, the
warning was worth it.
^ permalink raw reply
* Re: Debugging git-commit slowness on a large repo
From: Junio C Hamano @ 2011-12-05 17:38 UTC (permalink / raw)
To: Carlos Martín Nieto, Thomas Rast
Cc: Joshua Redstone, git@vger.kernel.org
In-Reply-To: <20111203002347.GB2950@centaur.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> ... At one
> point, commit forgot how to write the tree cache to the index (a
> performance optimisation). Do the times improve if you run 'git
> read-tree HEAD' between one commit and another? Note that this will
> reset the index to the last commit, though for the tests I image you
> use some variation of 'git commit -a'.
>
> Thomas Rast wrote a patch to re-teach commit to store the tree cache,
> but there were some issues and never got applied.
Ahh, I forgot all about that exchange.
http://thread.gmane.org/gmane.comp.version-control.git/178480/focus=178515
The cache-tree mechanism has traditionally been one of the more important
optimizations and it would be very nice if we can resurrect the behaviour
for "git commit" too.
^ permalink raw reply
* Re: [PATCH v2 0/3] grep multithreading and scaling
From: Thomas Rast @ 2011-12-05 9:38 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Eric Herman, git, Junio C Hamano
In-Reply-To: <20111202173400.GC23447@sigill.intra.peff.net>
Jeff King wrote:
>
> A quick perf run shows most of the time is spent inflating objects. The
> diff code has a sneaky trick to re-use worktree files when we know they
> are stat-clean (in diff's case it is to avoid writing a tempfile). I
> wonder if we should use the same trick here.
>
> It would hurt the cold cache case, though, as the compressed versions
> require fewer disk accesses, of course.
I just found out that on Linux, there's mincore() that can tell us
(racily, but who cares) whether a given file mapping is in memory. If
you would like to try it, see the source at the end, but I'm getting
things such as
# in a random collection of files, none of which I have accessed lately
$ ls -l
-rw-r--r-- 1 thomas users 116534 Jul 4 2010 IMG_4884.JPG
-rw-r--r-- 1 thomas users 7278081 Aug 25 2010 remoteserverrepo.zip
$ ./mincore IMG_4884.JPG
00000000000000000000000000000
$ cat IMG_4884.JPG > /dev/null
$ ./mincore IMG_4884.JPG
11111111111111111111111111111
$ ./mincore remoteserverrepo.zip
0000000000000000000000[...]
$ head -10 remoteserverrepo.zip >/dev/null
$ ./mincore remoteserverrepo.zip
1111000000000000000000[...]
So that looks fairly promising, and the order would then be:
- if stat-clean, and we have mincore(), and it tells us we can do it
cheaply: grab file from tree
- if it's a loose object: decompress it
- if stat-clean: grab file from tree
- access packs as usual
> PS I suspect your timings are somewhat affected by the simplicity of the
> regex you are asking for. The time to inflate the blobs dominates,
> because the search is just a memmem(). On my quad-core w/
> hyperthreading (i.e., 8 apparent cores):
>
> $ /usr/bin/time git grep INITRAMFS_ROOT_UID >/dev/null
> 0.42user 0.45system 0:00.15elapsed 578%CPU
> $ /usr/bin/time git grep 'a.*b' >/dev/null
> 14.68user 0.50system 0:02.00elapsed 758%CPU
> $ /usr/bin/time git grep --cached INITRAMFS_ROOT_UID >/dev/null
> 7.64user 0.41system 0:07.61elapsed 105%CPU
> $ /usr/bin/time git grep --cached 'a.*b' >/dev/null
> 23.46user 0.47system 0:08.42elapsed 284%CPU
>
> So I think there is value in parallelizing even --cached greps. But
> we could do so much better if blob inflation could be done in
> parallel.
Ok, I see, I missed that part. Perhaps the heuristic should then be
"if the regex boils down to memmem, disable threading", but let's see
what loose object decompression in parallel can give us.
---- 8< ---- mincore.c ---- 8< ----
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <fcntl.h>
void die(const char *s)
{
perror(s);
exit(1);
}
int main (int argc, char *argv[])
{
void *mem;
size_t len;
struct stat st;
int fd;
unsigned char *vec;
int vsize;
int i;
size_t page = sysconf(_SC_PAGESIZE);
if (argc != 2) {
fprintf(stderr, "usage: %s <file>\n", argv[0]);
exit(2);
}
fd = open(argv[1], O_RDONLY);
if (fd == -1)
die("open failed");
if (fstat(fd, &st) == -1)
die("fstat failed");
mem = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
if (mem == (void*) -1)
die("mmap failed");
vsize = (st.st_size+page-1)/page;
vec = malloc(vsize);
if (!vec)
die("malloc failed");
if (mincore(mem, st.st_size, vec) == -1)
die("mincore failed");
for (i = 0; i < vsize; i++)
printf("%d", (int) vec[i]);
printf("\n");
return 0;
}
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH v2 3/3] grep: disable threading in all but worktree case
From: Thomas Rast @ 2011-12-05 9:02 UTC (permalink / raw)
To: René Scharfe; +Cc: Eric Herman, git, Junio C Hamano
In-Reply-To: <4ED8F9AE.8030605@lsrfire.ath.cx>
René Scharfe wrote:
> Am 02.12.2011 14:07, schrieb Thomas Rast:
> > Measuring grep performance showed that in all but the worktree case
> > (as opposed to --cached,<committish> or<treeish>), threading
> > actually slows things down. For example, on my dual-core
> > hyperthreaded i7 in a linux-2.6.git at v2.6.37-rc2, I got:
> >
> > Threads worktree case | --cached case
> > --------------------------------------------------------------------------
> > 8 (default) | 2.17user 0.15sys 0:02.20real | 0.11user 0.26sys 0:00.11real
> > 4 | 2.06user 0.17sys 0:02.08real | 0.11user 0.26sys 0:00.12real
> > 2 | 2.02user 0.25sys 0:02.08real | 0.15user 0.37sys 0:00.28real
> > NO_PTHREADS | 1.57user 0.05sys 0:01.64real | 0.09user 0.12sys 0:00.22real
>
> Are the columns mixed up?
Indeed, sorry.
In case you were wondering why this table is different from the
numbers given in the cover letter: I noticed at some point that I had
an incomplete checkout (apparently 'git checkout -- .' is really not
the same as 'git reset --hard'... sigh). Then I saw that while the
numbers were different, the conclusion was not, so I forgot to update
it.
> This is a bit radical. I think the underlying issue that
> read_sha1_file() is not thread-safe can be solved eventually and then
> we'd need to readd that code.
I'm also scared of sha1_file.c, especially when it gets down to
packfiles. But perhaps it wouldn't be *too* hard to do it in parallel
iff the object can be read from the loose object store.
> PS: Patches one and three missed a signoff.
Oops, thanks, turns out I had a misconfigured alias ...
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] Documentation: fix formatting error in merge-options.txt
From: Jack Nagel @ 2011-12-05 7:53 UTC (permalink / raw)
To: git, gitster; +Cc: jacknagel
The first paragraph inside of a list item does not need a preceding line
consisting of a single '+', and in fact this causes the text to be
misrendered. Fix it.
Signed-off-by: Jack Nagel <jacknagel@gmail.com>
---
Documentation/merge-options.txt | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 6bd0b04..1a5c12e 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -9,7 +9,6 @@ inspect and further tweak the merge result before committing.
--edit::
-e::
-+
Invoke editor before committing successful merge to further
edit the default merge message.
--
1.7.8
^ permalink raw reply related
* [PATCH] Set hard limit on delta chain depth
From: Nguyễn Thái Ngọc Duy @ 2011-12-05 7:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
Too deep delta chains can cause stack overflow in get_base_data(). Set
a hard limit so that index-pack does not run out of stack. Also stop
people from producing such a long delta chains using "pack-object
--depth=<too large>"
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I used to make very long delta chains and triggered this in index-pack.
I did not care reporting because it's my fault anyway. Think again,
index-pack is called at server side and a malicious client can
trigger this. This patch does not improve the situation much, but at
least we won't get sigsegv at server side.
builtin/index-pack.c | 12 ++++++++++--
builtin/pack-objects.c | 3 +++
pack.h | 2 ++
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0945adb..cfb8cb2 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -504,13 +504,16 @@ static int is_delta_type(enum object_type type)
return (type == OBJ_REF_DELTA || type == OBJ_OFS_DELTA);
}
-static void *get_base_data(struct base_data *c)
+static void *get_base_data_1(struct base_data *c, int depth)
{
+ if (depth > MAX_DELTA_DEPTH)
+ die("index-pack: too long delta chain");
+
if (!c->data) {
struct object_entry *obj = c->obj;
if (is_delta_type(obj->type)) {
- void *base = get_base_data(c->base);
+ void *base = get_base_data_1(c->base, depth + 1);
void *raw = get_data_from_pack(obj);
c->data = patch_delta(
base, c->base->size,
@@ -530,6 +533,11 @@ static void *get_base_data(struct base_data *c)
return c->data;
}
+static void *get_base_data(struct base_data *c)
+{
+ return get_base_data_1(c, 0);
+}
+
static void resolve_delta(struct object_entry *delta_obj,
struct base_data *base, struct base_data *result)
{
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 824ecee..85ee04b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2508,6 +2508,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (keep_unreachable && unpack_unreachable)
die("--keep-unreachable and --unpack-unreachable are incompatible.");
+ if (depth > MAX_DELTA_DEPTH)
+ die("--depth exceeds %d", MAX_DELTA_DEPTH);
+
if (progress && all_progress_implied)
progress = 2;
diff --git a/pack.h b/pack.h
index 722a54e..b8f60c3 100644
--- a/pack.h
+++ b/pack.h
@@ -3,6 +3,8 @@
#include "object.h"
+#define MAX_DELTA_DEPTH 128
+
/*
* Packed object header
*/
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Bug: git bus error (stack blowout)
From: Nguyen Thai Ngoc Duy @ 2011-12-05 4:58 UTC (permalink / raw)
To: vsrinivas; +Cc: git
In-Reply-To: <CAAcG=3NfvYSjhHDNb8aZ=_O4661bV7jkZBpmc77ZVCFDQQdHJw@mail.gmail.com>
On Sun, Dec 04, 2011 at 05:50:19PM -0500, Venkatesh Srinivas wrote:
> Hi,
>
> When using git 1.7.6.3 from NetBSD's pkgsrc, on this git tree:
> http://gitweb.dragonflybsd.org/pkgsrcv2.git, I got a bus error when
> switching from the pkgsrc-2011q3 branch to the master branch. I have a
> core file and the git binary if it'd be helpful; it looks like
> mark_parents_uninteresting() was called recursively entirely too many
> times (>60,000), originally from prepare_revision_walk(), from
> stat_tracking_info(), from format_tracking_info(),
> update_revs_for_switch(), from cmd_checkout().
This patch may fix it for you, although it'd be interesting to
understand how you get into this (I'm still cloning pkgsrcv2.git).
-- 8< --
Subject: [PATCH] Eliminate recursion in setting/clearing marks in commit list
Recursion in a DAG is generally a bad idea because it could be very
deep. Be defensive and avoid recursion in mark_parents_uninteresting()
and clear_commit_marks().
mark_parents_uninteresting() learns a trick from clear_commit_marks()
to avoid malloc() in (dorminant) single-parent case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
commit.c | 31 ++++++++++++++++++++-----------
revision.c | 45 +++++++++++++++++++++++++++++----------------
2 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/commit.c b/commit.c
index 0775eec..cd19a69 100644
--- a/commit.c
+++ b/commit.c
@@ -423,22 +423,31 @@ struct commit *pop_most_recent_commit(struct commit_list **list,
void clear_commit_marks(struct commit *commit, unsigned int mark)
{
- while (commit) {
- struct commit_list *parents;
+ struct commit_list *list = NULL, *l;
+ commit_list_insert(commit, &list);
+ while (list) {
+ commit = list->item;
+ l = list;
+ list = list->next;
+ free(l);
- if (!(mark & commit->object.flags))
- return;
+ while (commit) {
+ struct commit_list *parents;
- commit->object.flags &= ~mark;
+ if (!(mark & commit->object.flags))
+ break;
- parents = commit->parents;
- if (!parents)
- return;
+ commit->object.flags &= ~mark;
+
+ parents = commit->parents;
+ if (!parents)
+ break;
- while ((parents = parents->next))
- clear_commit_marks(parents->item, mark);
+ while ((parents = parents->next))
+ commit_list_insert(parents->item, &list);
- commit = commit->parents->item;
+ commit = commit->parents->item;
+ }
}
}
diff --git a/revision.c b/revision.c
index 0aa3638..8d4069e 100644
--- a/revision.c
+++ b/revision.c
@@ -139,11 +139,32 @@ void mark_tree_uninteresting(struct tree *tree)
void mark_parents_uninteresting(struct commit *commit)
{
- struct commit_list *parents = commit->parents;
+ struct commit_list *parents = NULL, *l;
+
+ for (l = commit->parents; l; l = l->next)
+ commit_list_insert(l->item, &parents);
while (parents) {
struct commit *commit = parents->item;
- if (!(commit->object.flags & UNINTERESTING)) {
+ l = parents;
+ parents = parents->next;
+ free(l);
+
+ while (commit) {
+ /*
+ * A missing commit is ok iff its parent is marked
+ * uninteresting.
+ *
+ * We just mark such a thing parsed, so that when
+ * it is popped next time around, we won't be trying
+ * to parse it and get an error.
+ */
+ if (!has_sha1_file(commit->object.sha1))
+ commit->object.parsed = 1;
+
+ if (commit->object.flags & UNINTERESTING)
+ break;
+
commit->object.flags |= UNINTERESTING;
/*
@@ -154,21 +175,13 @@ void mark_parents_uninteresting(struct commit *commit)
* wasn't uninteresting), in which case we need
* to mark its parents recursively too..
*/
- if (commit->parents)
- mark_parents_uninteresting(commit);
- }
+ if (!commit->parents)
+ break;
- /*
- * A missing commit is ok iff its parent is marked
- * uninteresting.
- *
- * We just mark such a thing parsed, so that when
- * it is popped next time around, we won't be trying
- * to parse it and get an error.
- */
- if (!has_sha1_file(commit->object.sha1))
- commit->object.parsed = 1;
- parents = parents->next;
+ for (l = commit->parents->next; l; l = l->next)
+ commit_list_insert(l->item, &parents);
+ commit = commit->parents->item;
+ }
}
}
--
1.7.8.36.g69ee2
-- 8< --
--
Duy
^ permalink raw reply related
* [PATCH v2] git-p4: introduce skipSubmitEdit
From: Pete Wyckoff @ 2011-12-05 0:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Loren A. Linden Levy, Luke Diamand
Add a configuration variable to skip invoking the editor in the
submit path.
The existing variable skipSubmitEditCheck continues to make sure
that the submit template was indeed modified by the editor; but,
it is not considered if skipSubmitEdit is true.
Reported-by: Loren A. Linden Levy <lindenle@gmail.com>
Acked-by: Luke Diamand <luke@diamand.org>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
This was talked about and reviewed back in September 2011.
I think it's ready to include in the next release.
Thanks,
-- Pete
contrib/fast-import/git-p4 | 59 ++++++++++++++++++----------
contrib/fast-import/git-p4.txt | 19 ++++++++-
t/t9805-skip-submit-edit.sh | 82 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 136 insertions(+), 24 deletions(-)
create mode 100755 t/t9805-skip-submit-edit.sh
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index b975d67..7fd8bf0 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -847,6 +847,38 @@ class P4Submit(Command, P4UserMap):
return template
+ def edit_template(self, template_file):
+ """Invoke the editor to let the user change the submission
+ message. Return true if okay to continue with the submit."""
+
+ # if configured to skip the editing part, just submit
+ if gitConfig("git-p4.skipSubmitEdit") == "true":
+ return True
+
+ # look at the modification time, to check later if the user saved
+ # the file
+ mtime = os.stat(template_file).st_mtime
+
+ # invoke the editor
+ if os.environ.has_key("P4EDITOR"):
+ editor = os.environ.get("P4EDITOR")
+ else:
+ editor = read_pipe("git var GIT_EDITOR").strip()
+ system(editor + " " + template_file)
+
+ # If the file was not saved, prompt to see if this patch should
+ # be skipped. But skip this verification step if configured so.
+ if gitConfig("git-p4.skipSubmitEditCheck") == "true":
+ return True
+
+ if os.stat(template_file).st_mtime <= mtime:
+ while True:
+ response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
+ if response == 'y':
+ return True
+ if response == 'n':
+ return False
+
def applyCommit(self, id):
print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
@@ -1001,7 +1033,7 @@ class P4Submit(Command, P4UserMap):
separatorLine = "######## everything below this line is just the diff #######\n"
- [handle, fileName] = tempfile.mkstemp()
+ (handle, fileName) = tempfile.mkstemp()
tmpFile = os.fdopen(handle, "w+")
if self.isWindows:
submitTemplate = submitTemplate.replace("\n", "\r\n")
@@ -1009,25 +1041,9 @@ class P4Submit(Command, P4UserMap):
newdiff = newdiff.replace("\n", "\r\n")
tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
tmpFile.close()
- mtime = os.stat(fileName).st_mtime
- if os.environ.has_key("P4EDITOR"):
- editor = os.environ.get("P4EDITOR")
- else:
- editor = read_pipe("git var GIT_EDITOR").strip()
- system(editor + " " + fileName)
- if gitConfig("git-p4.skipSubmitEditCheck") == "true":
- checkModTime = False
- else:
- checkModTime = True
-
- response = "y"
- if checkModTime and (os.stat(fileName).st_mtime <= mtime):
- response = "x"
- while response != "y" and response != "n":
- response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
-
- if response == "y":
+ if self.edit_template(fileName):
+ # read the edited message and submit
tmpFile = open(fileName, "rb")
message = tmpFile.read()
tmpFile.close()
@@ -1039,11 +1055,12 @@ class P4Submit(Command, P4UserMap):
if self.preserveUser:
if p4User:
# Get last changelist number. Cannot easily get it from
- # the submit command output as the output is unmarshalled.
+ # the submit command output as the output is
+ # unmarshalled.
changelist = self.lastP4Changelist()
self.modifyChangelistUser(changelist, p4User)
-
else:
+ # skip this patch
for f in editedFiles:
p4_revert(f)
for f in filesToAdd:
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 52003ae..5044a12 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -202,11 +202,24 @@ able to find the relevant client. This client spec will be used to
both filter the files cloned by git and set the directory layout as
specified in the client (this implies --keep-path style semantics).
-git-p4.skipSubmitModTimeCheck
+git-p4.skipSubmitEdit
- git config [--global] git-p4.skipSubmitModTimeCheck false
+ git config [--global] git-p4.skipSubmitEdit false
-If true, submit will not check if the p4 change template has been modified.
+Normally, git-p4 invokes an editor after each commit is applied so
+that you can make changes to the submit message. Setting this
+variable to true will skip the editing step, submitting the change as is.
+
+git-p4.skipSubmitEditCheck
+
+ git config [--global] git-p4.skipSubmitEditCheck false
+
+After the editor is invoked, git-p4 normally makes sure you saved the
+change description, as an indication that you did indeed read it over
+and edit it. You can quit without saving to abort the submit (or skip
+this change and continue). Setting this variable to true will cause
+git-p4 not to check if you saved the change description. This variable
+only matters if git-p4.skipSubmitEdit has not been set to true.
git-p4.preserveUser
diff --git a/t/t9805-skip-submit-edit.sh b/t/t9805-skip-submit-edit.sh
new file mode 100755
index 0000000..734ccf2
--- /dev/null
+++ b/t/t9805-skip-submit-edit.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='git-p4 skipSubmitEdit config variables'
+
+. ./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"
+ )
+'
+
+# this works because EDITOR is set to :
+test_expect_success 'no config, unedited, say yes' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ echo line >>file1 &&
+ git commit -a -m "change 2" &&
+ echo y | "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'no config, unedited, say no' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ echo line >>file1 &&
+ git commit -a -m "change 3 (not really)" &&
+ printf "bad response\nn\n" | "$GITP4" submit
+ p4 changes //depot/... >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'skipSubmitEdit' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ # will fail if editor is even invoked
+ git config core.editor /bin/false &&
+ echo line >>file1 &&
+ git commit -a -m "change 3" &&
+ "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 3 wc
+ )
+'
+
+test_expect_success 'skipSubmitEditCheck' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
+ echo line >>file1 &&
+ git commit -a -m "change 4" &&
+ "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 4 wc
+ )
+'
+
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.8.rc4.30.g3c9dc
^ permalink raw reply related
* Bug: git bus error (stack blowout)
From: Venkatesh Srinivas @ 2011-12-04 22:50 UTC (permalink / raw)
To: git
Hi,
When using git 1.7.6.3 from NetBSD's pkgsrc, on this git tree:
http://gitweb.dragonflybsd.org/pkgsrcv2.git, I got a bus error when
switching from the pkgsrc-2011q3 branch to the master branch. I have a
core file and the git binary if it'd be helpful; it looks like
mark_parents_uninteresting() was called recursively entirely too many
times (>60,000), originally from prepare_revision_walk(), from
stat_tracking_info(), from format_tracking_info(),
update_revs_for_switch(), from cmd_checkout().
Thanks,
-- vs;
^ permalink raw reply
* [bug?] checkout -m doesn't work without a base version
From: Pete Harlan @ 2011-12-04 22:31 UTC (permalink / raw)
To: git
Hi,
If during a merge I've resolved conflicts in foo.c but want to start
over with foo.c to resolve them differently, I can say "git checkout
-m foo.c" to restore it to its un-resolved state.
But this only works if there's a base version; if foo.c was added in
each branch, we get:
error: path 'foo.c' does not have all three versions
Git didn't need all three versions to create the original conflicted
file, so why would it need them to recreate it?
(The message is the same if I explicitly tell Git I don't want diff3
via "git checkout --conflict=merge foo.c".)
If this is considered a bug worth fixing I'll write a test that it
fails; if it's expected behavior I think the docs should mention
that.
Thanks,
Pete Harlan
pgit@pcharlan.com
^ permalink raw reply
* Re: [RFC PATCH] git-p4: introduce asciidoc documentation
From: Pete Wyckoff @ 2011-12-04 21:49 UTC (permalink / raw)
To: Frans Klaver; +Cc: git
In-Reply-To: <op.v5zh6udk0aolir@keputer>
fransklaver@gmail.com wrote on Sun, 04 Dec 2011 22:33 +0100:
> >+Clone options
> >+~~~~~~~~~~~~~
> >+These options can used in an initial 'clone', along with the 'sync
>
> s/can used/can be used/
Thanks for checking. I'll roll it in with the next submission.
-- Pete
^ permalink raw reply
* Re: [RFC PATCH] git-p4: introduce asciidoc documentation
From: Frans Klaver @ 2011-12-04 21:33 UTC (permalink / raw)
To: git, Pete Wyckoff
In-Reply-To: <20111203235328.GA3866@arf.padd.com>
> +Clone options
> +~~~~~~~~~~~~~
> +These options can used in an initial 'clone', along with the 'sync
s/can used/can be used/
Cheers,
Frans
^ permalink raw reply
* [RFD] Handling of non-UTF8 data in gitweb
From: Jakub Narebski @ 2011-12-04 16:09 UTC (permalink / raw)
To: git; +Cc: Jürgen Kreileder, John Hawley
Hello!
Currently gitweb converts data it receives from git commands to Perl
internal utf8 representation via to_utf8() subroutine
# decode sequences of octets in utf8 into Perl's internal form,
# which is utf-8 with utf8 flag set if needed. gitweb writes out
# in utf-8 thanks to "binmode STDOUT, ':utf8'" at beginning
sub to_utf8 {
my $str = shift;
return undef unless defined $str;
if (utf8::valid($str)) {
utf8::decode($str);
return $str;
} else {
return decode($fallback_encoding, $str, Encode::FB_DEFAULT);
}
}
Each part of data must be handled separately. It is quite error prone
process, as can be seen from quite a number of patches that fix handling
of UTF-8 data (latest from Jürgen).
Much, much simpler would be to force opening of all files (including
output pipes from git commands) in ':utf8' mode:
use open qw(:std :utf8);
[Note: perhaps instead of ':utf8' it should be ':encoding(UTF-8)'
there...]
But doing this would change gitweb behavior. Currently when
encountering something (usually line of output) that is not valid
UTF-8, we decode it (to UTF-8) using $fallback_encoding, by default
'latin1'. Note however that this value is per gitweb installation,
not per repository.
Using "use open qw(:std :utf8);" would be like changing the value of
$fallback_encoding to 'utf8' -- errors would be ignored, and characters
which are invalid in UTF-8 encoding would get replaced[1] with
substitution character '�' U+FFFD.
Though at least for HTML output we could use Encode::FB_HTMLCREF
handling (which would produce &#NNN;) or Encode::FB_XMLCREF (which
would produce &#xHHHH;), though this must be done after HTML escaping...
and is probaby not worth it (FYI this can be done by setting
$PerlIO::encoding::fallback to either of those values[2])
[1] http://perldoc.perl.org/Encode.html#Handling-Malformed-Data
http://p3rl.org/Encode
[2] http://perldoc.perl.org/PerlIO/encoding.html
http://p3rl.org/PerlIO::encoding
I don't know if people are relying on the old behavior. I guess
it could be emulated by defining our own 'utf-8-with-fallback'
encoding, or by defining our own PerlIO layer with PerlIO::via.
But it no longer be simple solution (though still automatic).
Alternate approach would be to audit gitweb code, and call to_utf8
before storing extracted output of git command in variable (excluding
save types like SHA-1, filemode, timestamp and timezone). The fact
that to_utf8 is idempotent and can be called multiple times would
help here, I think.
The correct solution would be of course to respect `gui.encoding`
per-repository config variable, and `encoding` gitattribute...
though the latter is hampered by the fact that there is currently
no way to read attribute with "git check-attr" from a given tree:
think of a diff of change of encoding of a file!
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv2 0/4] git-p4: small fixes to branches and labels; tests
From: Pete Wyckoff @ 2011-12-04 16:07 UTC (permalink / raw)
To: Vitor Antunes; +Cc: Luke Diamand, git
In-Reply-To: <CAOpHH-U6NxRSioRZg9_+f146vVR+S1hWsVbRmHz+vsqtz+vXiA@mail.gmail.com>
vitor.hda@gmail.com wrote on Thu, 01 Dec 2011 00:37 +0000:
> On Wed, Nov 30, 2011 at 11:00 PM, Pete Wyckoff <pw@padd.com> wrote:
> > And avoids collision with some Vitor code that will get
> > added eventually.
>
> I'm starting to doubt I will ever be able to overcome the fast-import
> limitation on not allowing branch delesetion. Sure, the code I wrote was
> garbage! But they seem to be very relunctant on the concept of deleting
> branches on the fly.
> Did you ever take a look at the patch I sent? Maybe you could help me
> shape it up a bit.
I don't think we necessarily need branch deletion inside the
fast-import. Can you go back and look at my mail from August and
see if that approach is doable? Just make a single commit on a
throwaway branch with no parent, checkpoint, then do diff-tree
for each potential parent until you find a match. Do the commit
for real where it goes. As git-p4 exits, we'll delete the branch
ref of the test commit.
If this works, we can see if fast-import can be taught to
generate a tree object without a commit to save the need for a
temporary branch.
-- Pete
^ permalink raw reply
* Re: [PATCH 2/2] builtin/apply.c: report error on failure to recognize input
From: Brandon Casey @ 2011-12-04 15:39 UTC (permalink / raw)
To: artem.bityutskiy; +Cc: git
In-Reply-To: <1323005418.9400.49.camel@sauron.fi.intel.com>
Artem Bityutskiy <artem.bityutskiy@linux.intel.com> wrote:
>Brandon, Thanks a lot for picking this. I did not reply because I did
>not have time to look at this after your review yet, it was in my TODO
>list. But I am happy you picked this and fixed the issue.
No problem.
>Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Thanks.
-Brandon
^ permalink raw reply
* Re: Debugging git-commit slowness on a large repo
From: Tomas Carnecky @ 2011-12-04 13:54 UTC (permalink / raw)
To: Joshua Redstone; +Cc: git@vger.kernel.org
In-Reply-To: <CAFE9C7B.2BFEC%joshua.redstone@fb.com>
On 12/3/11 12:17 AM, Joshua Redstone wrote:
> Hi,
> I have a git repo with about 300k commits, 150k files totaling maybe 7GB.
> Locally committing a small change - say touching fewer than 300 bytes
> across 4 files - consistently takes over one second, which seems kinda
> slow. This is using git 1.7.7.4 on a linux 2.6 box. The time does not
> improve after doing a git-gc (my .git dir has maybe 250 files after a git
> gc). The same size commit on a brand new repo takes< 10ms. Any thoughts
> on why committing a small change seems to take a long time on larger repos?
>
> Fwiw, I also tried doing the same test using libgit2 (via the pygit2
> wrapper), and it was ever slower (about 6 seconds to commit the same small
> change).
try git commit --no-status
^ permalink raw reply
* Re: [PATCH 2/2] builtin/apply.c: report error on failure to recognize input
From: Artem Bityutskiy @ 2011-12-04 13:30 UTC (permalink / raw)
To: Brandon Casey; +Cc: git
In-Reply-To: <1322944550-27344-2-git-send-email-drafnel@gmail.com>
On Sat, 2011-12-03 at 14:35 -0600, Brandon Casey wrote:
> When git apply is passed something that is not a patch, it does not produce
> an error message or exit with a non-zero status if it was not actually
> "applying" the patch i.e. --check or --numstat etc were supplied on the
> command line.
>
> Fix this by producing an error when apply fails to find any hunks whatsoever
> while parsing the patch.
>
> This will cause some of the output formats (--numstat, --diffstat, etc) to
> produce an error when they formerly would have reported zero changes and
> exited successfully. That seems like the correct behavior though. Failure
> to recognize the input as a patch should be an error.
>
> Plus, add a test.
>
> Reported-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
Brandon, Thanks a lot for picking this. I did not reply because I did
not have time to look at this after your review yet, it was in my TODO
list. But I am happy you picked this and fixed the issue.
Tested-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
--
Best Regards,
Artem Bityutskiy
^ permalink raw reply
* Help with setting up git server
From: git815 @ 2011-12-04 13:22 UTC (permalink / raw)
To: git
After what I thought I have set up a git server under http, I'm getting this
error on client site whenever I issue a git command: "fatal: Not a git
repository (or any of the parent directories): .git".
Here is what I did:
(1) In project directory, clone a git project: "git clone --bare proj.git"
( I noticed no .git subdirectory exists in the cloned directory).
(2) Move the cloned directory proj.git under web server path.
(3) On a separate client machine, clone a local copy of the project: "git
clone http://git-server/proj.git" (This command runs and gives me the actual
source code files and nothing else. The cloned proj.git directories
contains git files and directories).
I'm new and I must have missed something. Can anyone help? Thanks.
--
View this message in context: http://git.661346.n2.nabble.com/Help-with-setting-up-git-server-tp7060154p7060154.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH, v4] git-tag: introduce --cleanup option
From: Kirill A. Shutemov @ 2011-12-04 4:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Kirill A. Shutemov
From: "Kirill A. Shutemov" <kirill@shutemov.name>
Normally git tag stripes tag message lines starting with '#', trailing
spaces from every line and empty lines from the beginning and end.
--cleanup allows to select different cleanup modes for tag message.
It provides the same interface as --cleanup option in git-commit.
Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
Documentation/git-tag.txt | 10 ++++++
builtin/tag.c | 69 +++++++++++++++++++++++++++++++++++---------
2 files changed, 65 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index c83cb13..c2d73f3 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,6 +99,16 @@ OPTIONS
Implies `-a` if none of `-a`, `-s`, or `-u <key-id>`
is given.
+--cleanup=<mode>::
+ This option sets how the tag message is cleaned up.
+ The '<mode>' can be one of 'verbatim', 'whitespace', 'strip',
+ and 'default'. The 'default' mode will strip leading and
+ trailing empty lines and #commentary from the tag message
+ only if the message is to be edited. Otherwise only whitespace
+ removed. The 'verbatim' mode does not change message at all,
+ 'whitespace' removes just leading/trailing whitespace lines
+ and 'strip' removes both whitespace and commentary.
+
<tagname>::
The name of the tag to create, delete, or describe.
The new tag name must pass all checks defined by
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..27a66a3 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -268,6 +268,15 @@ static const char tag_template[] =
N_("\n"
"#\n"
"# Write a tag message\n"
+ "# Lines starting with '#' will be ignored.\n"
+ "#\n");
+
+static const char tag_template_nocleanup[] =
+ N_("\n"
+ "#\n"
+ "# Write a tag message\n"
+ "# Lines starting with '#' will be kept; you may remove them"
+ " yourself if you want to.\n"
"#\n");
static void set_signingkey(const char *value)
@@ -319,8 +328,18 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
return 0;
}
+struct create_tag_options {
+ unsigned int message;
+ unsigned int sign;
+ enum {
+ CLEANUP_NONE,
+ CLEANUP_SPACE,
+ CLEANUP_ALL
+ } cleanup_mode;
+};
+
static void create_tag(const unsigned char *object, const char *tag,
- struct strbuf *buf, int message, int sign,
+ struct strbuf *buf, struct create_tag_options *opt,
unsigned char *prev, unsigned char *result)
{
enum object_type type;
@@ -345,7 +364,7 @@ static void create_tag(const unsigned char *object, const char *tag,
if (header_len > sizeof(header_buf) - 1)
die(_("tag header too big."));
- if (!message) {
+ if (!opt->message) {
int fd;
/* write the template message before editing: */
@@ -356,8 +375,12 @@ static void create_tag(const unsigned char *object, const char *tag,
if (!is_null_sha1(prev))
write_tag_body(fd, prev);
+ else if (opt->cleanup_mode == CLEANUP_ALL)
+ write_or_die(fd, _(tag_template),
+ strlen(_(tag_template)));
else
- write_or_die(fd, _(tag_template), strlen(_(tag_template)));
+ write_or_die(fd, _(tag_template_nocleanup),
+ strlen(_(tag_template_nocleanup)));
close(fd);
if (launch_editor(path, buf, NULL)) {
@@ -367,14 +390,15 @@ static void create_tag(const unsigned char *object, const char *tag,
}
}
- stripspace(buf, 1);
+ if (opt->cleanup_mode != CLEANUP_NONE)
+ stripspace(buf, opt->cleanup_mode == CLEANUP_ALL);
- if (!message && !buf->len)
+ if (opt->message && !buf->len)
die(_("no tag message?"));
strbuf_insert(buf, 0, header_buf, header_len);
- if (build_tag_object(buf, sign, result) < 0) {
+ if (build_tag_object(buf, opt->sign, result) < 0) {
if (path)
fprintf(stderr, _("The tag message has been left in %s\n"),
path);
@@ -422,9 +446,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct ref_lock *lock;
-
- int annotate = 0, sign = 0, force = 0, lines = -1,
- list = 0, delete = 0, verify = 0;
+ struct create_tag_options opt;
+ char *cleanup_arg = NULL;
+ int annotate = 0, force = 0, lines = -1, list = 0,
+ delete = 0, verify = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -442,7 +467,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_CALLBACK('m', "message", &msg, "message",
"tag message", parse_msg_arg),
OPT_FILENAME('F', "file", &msgfile, "read message from file"),
- OPT_BOOLEAN('s', "sign", &sign, "annotated and GPG-signed tag"),
+ OPT_BOOLEAN('s', "sign", &opt.sign, "annotated and GPG-signed tag"),
+ OPT_STRING(0, "cleanup", &cleanup_arg, "default",
+ "how to strip spaces and #comments from message"),
OPT_STRING('u', "local-user", &keyid, "key-id",
"use another key to sign the tag"),
OPT__FORCE(&force, "replace the tag if exists"),
@@ -459,13 +486,15 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
git_config(git_tag_config, NULL);
+ opt.sign = 0;
+
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
if (keyid) {
- sign = 1;
+ opt.sign = 1;
set_signingkey(keyid);
}
- if (sign)
+ if (opt.sign)
annotate = 1;
if (argc == 0 && !(delete || verify))
list = 1;
@@ -523,9 +552,21 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else if (!force)
die(_("tag '%s' already exists"), tag);
+ opt.message = msg.given || msgfile;
+
+ if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
+ opt.cleanup_mode = !opt.message ? CLEANUP_ALL : CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "verbatim"))
+ opt.cleanup_mode = CLEANUP_NONE;
+ else if (!strcmp(cleanup_arg, "whitespace"))
+ opt.cleanup_mode = CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "strip"))
+ opt.cleanup_mode = CLEANUP_ALL;
+ else
+ die(_("Invalid cleanup mode %s"), cleanup_arg);
+
if (annotate)
- create_tag(object, tag, &buf, msg.given || msgfile,
- sign, prev, object);
+ create_tag(object, tag, &buf, &opt, prev, object);
lock = lock_any_ref_for_update(ref.buf, prev, 0);
if (!lock)
--
1.7.7.3
^ permalink raw reply related
* [RFC PATCH] git-p4: introduce asciidoc documentation
From: Pete Wyckoff @ 2011-12-03 23:53 UTC (permalink / raw)
To: git
Add proper documentation for git-p4. Delete the old .txt
documentation from contrib/fast-import.
---
I'd appreciate review by git-p4 people to make sure I captured
everything from the old documentation, and to catch any errors.
There's a fair amount of new content in here, describing all
the options and variables. I left out some obscure commands
on purpose.
Comments from anyone else would be welcome too. Especially
in the area of generic asciidoc formatting or style in git
command pages.
Thanks,
-- Pete
Documentation/git-p4.txt | 456 ++++++++++++++++++++++++++++++++++++++++
contrib/fast-import/git-p4.txt | 289 -------------------------
2 files changed, 456 insertions(+), 289 deletions(-)
create mode 100644 Documentation/git-p4.txt
delete mode 100644 contrib/fast-import/git-p4.txt
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
new file mode 100644
index 0000000..c555dce
--- /dev/null
+++ b/Documentation/git-p4.txt
@@ -0,0 +1,456 @@
+git-p4(1)
+=========
+
+NAME
+----
+git-p4 - Import from and submit to Perforce repositories
+
+
+SYNOPSIS
+--------
+[verse]
+'git p4 clone' [<sync options>] [<clone options>] <p4 depot path>...
+'git p4 sync' [<sync options>] [<p4 depot path>...]
+'git p4 rebase'
+'git p4 submit' [<submit options>] [<master branch name>]
+
+
+DESCRIPTION
+-----------
+This command provides a way to interact with p4 repositories
+using git.
+
+Create a new git repository from an existing p4 repository using
+'git p4 clone', giving it one or more p4 depot paths. Incorporate
+new commits from p4 changes with 'git p4 sync'. The 'sync' command
+is also used to include new branches from other p4 depot paths.
+Submit git changes back to p4 using 'git p4 submit'. The command
+'git p4 rebase' does a sync plus rebases the current branch onto
+the updated p4 remote branch.
+
+
+EXAMPLE
+-------
+* Create an alias for 'git p4', using the full path to the 'git-p4'
+ script if needed:
++
+------------
+$ git config --global alias.p4 '!git-p4'
+------------
+
+* Clone a repository:
++
+------------
+$ git p4 clone //depot/path/project
+------------
+
+* Do some work in the newly created git repository:
++
+------------
+$ cd project
+$ vi foo.h
+$ git commit -a -m "edited foo.h"
+------------
+
+* Update the git repository with recent changes from p4, rebasing your
+ work on top:
++
+------------
+$ git p4 rebase
+------------
+
+* Submit your commits back to p4:
++
+------------
+$ git p4 submit
+------------
+
+
+COMMANDS
+--------
+
+Clone
+~~~~~
+Generally, 'git p4 clone' is used to create a new git directory
+from an existing p4 repository:
+------------
+$ git p4 clone //depot/path/project
+------------
+This:
+
+1. Creates an empty git repository in a subdirectory called 'project'.
++
+2. Imports the full contents of the head revision from the given p4
+depot path into a single commit in the git branch 'refs/remotes/p4/master'.
++
+3. Creates a local branch, 'master' from this remote and checks it out.
+
+To reproduce the entire p4 history in git, use the '@all' modifier on
+the depot path:
+------------
+$ git p4 clone //depot/path/project@all
+------------
+
+
+Sync
+~~~~
+As development continues in the p4 repository, those changes can
+be included in the git repository using:
+------------
+$ git p4 sync
+------------
+This command finds new changes in p4 and imports them as git commits.
+
+P4 repositories can be added to an existing git repository using
+'git p4 sync' too:
+------------
+$ mkdir repo-git
+$ cd repo-git
+$ git init
+$ git p4 sync //path/in/your/perforce/depot
+------------
+This imports the specified depot into
+'refs/remotes/p4/master' in an existing git repository. The
+'--branch' option can be used to specify a different branch to
+be used for the p4 content.
+
+If a git repository includes branches 'refs/remotes/origin/p4', these
+will be fetched and consulted first during a 'git p4 sync'. Since
+importing directly from p4 is considerably slower than pulling changes
+from a git remote, this can be useful in a multi-developer environment.
+
+
+Rebase
+~~~~~~
+A common working pattern is to fetch the latest changes from the p4 depot
+and merge them with local uncommitted changes. Often, the p4 repository
+is the ultimate location for all code, thus a rebase workflow makes
+sense. This command does 'git p4 sync' followed by 'git rebase' to move
+local commits on top of updated p4 changes.
+------------
+$ git p4 rebase
+------------
+
+
+Submit
+~~~~~~
+Submitting changes from a git repository back to the p4 repository
+requires a separate p4 client workspace. This should be specified
+using the 'P4CLIENT' environment variable or the git configuration
+variable 'git-p4.client'. The p4 client must exist, but the client root
+will be created and populated if it does not already exist.
+
+To submit all changes that are in the current git branch but not in
+the 'p4/master' branch, use:
+------------
+$ git p4 submit
+------------
+
+To specify a branch other than the current one, use:
+------------
+$ git p4 submit topicbranch
+------------
+
+The upstream reference is generally 'refs/remotes/p4/master', but can
+be overridden using the '--origin=' command-line option.
+
+The p4 changes will be created as the user invoking 'git p4 submit'. The
+'--preserve-user' option will cause ownership to be modified
+according to the author of the git commit. This option requires admin
+priviliges in p4, which can be granted using 'p4 protect'.
+
+
+OPTIONS
+-------
+
+General options
+~~~~~~~~~~~~~~~
+All commands except clone accept this option.
+
+--git-dir <dir>::
+ Set the 'GIT_DIR' environment variable. See linkgit:git[1].
+
+Sync options
+~~~~~~~~~~~~
+These options can be used in the initial 'clone' as well as in
+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'.
+
+--detect-branches::
+ Use the branch detection algorithm to find new paths in p4. It is
+ documented below in "BRANCH DETECTION".
+
+--changesfile <file>::
+ Import exactly the p4 change numbers listed in 'file', one per
+ line. Normally, 'git p4' inspects the current p4 repository
+ state and detects the changes it should import.
+
+--silent::
+ Do not print any progress information.
+
+--verbose::
+ Provide more progress information.
+
+--detect-labels::
+ Query p4 for labels associated with the depot paths, and add
+ them as tags in git.
+
+--import-local::
+ 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/'.
+
+--max-changes <n>::
+ Limit the number of imported changes to 'n'. Useful to
+ limit the amount of history when using the '@all' p4 revision
+ specifier.
+
+--keep-path::
+ The mapping of file names from the p4 depot path to git, by
+ default, involves removing the entire depot path. With this
+ option, the full p4 depot path is retained in git. For example,
+ path '//depot/main/foo/bar.c', when imported from
+ '//depot/main/', becomes 'foo/bar.c'. With '--keep-path', the
+ git path is instead 'depot/main/foo/bar.c'.
+
+--use-client-spec::
+ 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.
+
+Clone options
+~~~~~~~~~~~~~
+These options can used in an initial 'clone', along with the 'sync'
+options described above.
+
+--destination <directory>::
+ Where to create the git repository. If not provided, the last
+ component in the p4 depot path is used to create a new
+ directory.
+
+--bare::
+ Perform a bare clone. See linkgit:git-clone[1].
+
+-/ <path>::
+ Exclude selected depot paths when cloning.
+
+Submit options
+~~~~~~~~~~~~~~
+These options can be used to modify 'git p4 submit' behavior.
+
+--verbose::
+ Provide more progress information.
+
+--origin <commit>::
+ Upstream location from which commits are identified to submit to
+ p4. By default, this is the most recent p4 commit reachable
+ from 'HEAD'.
+
+-M[<n>]::
+ Detect renames. See linkgit:git-diff[1]. Renames will be
+ represented in p4 using explicit 'move' operations.
+
+--preserve-user::
+ Re-author p4 changes before submitting to p4. This option
+ requires p4 admin priviliges.
+
+
+DEPOT PATH SYNTAX
+-----------------
+The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
+be one or more space-separated p4 depot paths, with an optional
+p4 revision specifier on the end:
+
+"//depot/my/project"::
+ Import one commit with all files in the '#head' change under that tree.
+
+"//depot/my/project@all"::
+ Import one commit for each change in the history of that depot path.
+
+"//depot/my/project@1,6"::
+ Import only changes 1 through 6.
+
+"//depot/proj1 //depot/proj2@all"::
+ Import all changes from both named depot paths.
+
+See 'p4 help revisions' for the full syntax of p4 revision specifiers.
+
+
+BRANCH DETECTION
+----------------
+P4 does not have the same concept of a branch as git. Instead,
+p4 organizes its content as a directory tree, where by convention
+different logical branches are in different locations in the tree.
+The 'p4 branch' command is used to maintain mappings between
+different areas in the tree, and indicate related content. 'git p4'
+can use these mappings to determine branch relationships.
+
+If you have a repository where all the branches of interest exist as
+subdirectories of a single depot path, you can use '--detect-branches'
+when cloning or syncing to have 'git p4' automatically find
+subdirectories in p4, and to generate these as branches in git.
+
+For example, if the P4 repository structure is:
+----
+//depot/main/...
+//depot/branch1/...
+----
+
+And "p4 branch -o branch1" shows a View line that looks like:
+----
+//depot/main/... //depot/branch1/...
+----
+
+Then this 'git p4 clone' command:
+----
+git p4 clone --detect-branches //depot@all
+----
+produces a separate branch in 'refs/remotes/p4/' for //depot/main,
+called 'master', and one for //depot/branch1 called 'depot/branch1'.
+
+However, it is not necessary to create branches in p4 to be able to use
+them like branches. Because it is difficult to infer branch
+relationships automatically, a git configuration setting
+'git-p4.branchList' can be used to explicitly identify branch
+relationships. It is a list of "source:destination" pairs, like a
+simple p4 branch specification, where the "source" and "destination" are
+the path elements in the p4 repository. The example above relied on the
+presence of the p4 branch. Without p4 branches, the same result will
+occur with:
+----
+git config git-p4.branchList main:branch1
+git p4 clone --detect-branches //depot@all
+----
+
+
+PERFORMANCE
+-----------
+The fast-import mechanism used by 'git p4' creates one pack file for
+each invocation of 'git p4 sync'. Normally, git garbage compression
+(linkgit:git-gc[1]) automatically compresses these to fewer pack files,
+but explicit invocation of 'git repack -adf' may improve performance.
+
+
+CONFIGURATION VARIABLES
+-----------------------
+The following config settings can be used to modify 'git p4' behavior.
+They all are in the 'git-p4' section.
+
+General variables
+~~~~~~~~~~~~~~~~~
+git-p4.user::
+ User specified as an option to all p4 commands, with '-u <user>'.
+ The environment variable 'P4USER' can be used instead.
+
+git-p4.password::
+ Password specified as an option to all p4 commands, with
+ '-P <password>'.
+ The environment variable 'P4PASS' can be used instead.
+
+git-p4.port::
+ Port specified as an option to all p4 commands, with
+ '-p <port>'.
+ The environment variable 'P4PORT' can be used instead.
+
+git-p4.host::
+ Host specified as an option to all p4 commands, with
+ '-h <host>'.
+ The environment variable 'P4HOST' can be used instead.
+
+git-p4.client::
+ Client specified as an option to all p4 commands, with
+ '-c <client>'. This can also be used as a way to find
+ the client spec for the 'useClientSpec' option.
+ The environment variable 'P4CLIENT' can be used instead.
+
+Clone and sync variables
+~~~~~~~~~~~~~~~~~~~~~~~~
+git-p4.syncFromOrigin::
+ Because importing commits from other git repositories is much faster
+ than importing them from p4, a mechanism exists to find p4 changes
+ first in git remotes. If branches exist under 'refs/remote/origin/p4',
+ those will be fetched and used when syncing from p4. This
+ variable can be set to 'false' to disable this behavior.
+
+git-p4.branchUser::
+ One phase in branch detection involves looking at p4 branches
+ to find new ones to import. By default, all branches are
+ inspected. This option limits the search to just those owned
+ by the single user named in the variable.
+
+git-p4.branchList::
+ List of branches to be imported when branch detection is
+ enabled. Each entry should be a pair of branch names separated
+ by a colon (:). This example declares that both branchA and
+ branchB were created from main:
+-------------
+git config git-p4.branchList main:branchA
+git config --add git-p4.branchList main:branchB
+-------------
+
+git-p4.useClientSpec::
+ Specify that the p4 client spec to be used to identify p4 depot
+ paths of interest. This is equivalent to specifying the option
+ '--use-client-spec'. The variable 'git-p4.client' can be used
+ to specify the name of the client.
+
+Submit variables
+~~~~~~~~~~~~~~~
+git-p4.detectRenames::
+ Detect renames. See linkgit:git-diff[1].
+
+git-p4.detectCopies::
+ Detect copies. See linkgit:git-diff[1].
+
+git-p4.detectCopiesHarder::
+ Detect copies harder. See linkgit:git-diff[1].
+
+git-p4.preserveUser::
+ On submit, re-author changes to reflect the git author,
+ regardless of who invokes 'git p4 submit'.
+
+git-p4.allowMissingP4Users::
+ When 'preserveUser' is true, 'git p4' normally dies if it
+ cannot find an author in the p4 user map. This setting
+ submits the change regardless.
+
+git-p4.skipSubmitEdit::
+ The submit process invokes the editor before each p4 change
+ is submitted. If this setting is true, though, the editing
+ step is skipped.
+
+git-p4.skipSubmitEditCheck::
+ After editing the p4 change message, 'git p4' makes sure that
+ the description really was changed by looking at the file
+ modification time. This option disables that test.
+
+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.
+
+git-p4.skipUserNameCheck::
+ If the user running 'git p4 submit' does not exist in the p4
+ user map, 'git p4' exits. This option can be used to force
+ submission regardless.
+
+
+IMPLEMENTATION DETAILS
+----------------------
+* Changesets from p4 are imported using git fast-import.
+* Cloning or syncing does not require a p4 client; file contents are
+ collected using 'p4 print'.
+* Submitting requires a p4 client, which is not in the same location
+ as the git repository. Patches are applied, one at a time, to
+ this p4 client and submitted from there.
+* Each commit imported by 'git p4' has a line at the end of the log
+ message indicating the p4 depot location and change number. This
+ line is used by later 'git p4 sync' operations to know which p4
+ changes are new.
+
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
deleted file mode 100644
index 52003ae..0000000
--- a/contrib/fast-import/git-p4.txt
+++ /dev/null
@@ -1,289 +0,0 @@
-git-p4 - Perforce <-> Git converter using git-fast-import
-
-Usage
-=====
-
-git-p4 can be used in two different ways:
-
-1) To import changes from Perforce to a Git repository, using "git-p4 sync".
-
-2) To submit changes from Git back to Perforce, using "git-p4 submit".
-
-Importing
-=========
-
-Simply start with
-
- git-p4 clone //depot/path/project
-
-or
-
- git-p4 clone //depot/path/project myproject
-
-This will:
-
-1) Create an empty git repository in a subdirectory called "project" (or
-"myproject" with the second command)
-
-2) Import the head revision from the given Perforce path into a git branch
-called "p4" (remotes/p4 actually)
-
-3) Create a master branch based on it and check it out.
-
-If you want the entire history (not just the head revision) then you can simply
-append a "@all" to the depot path:
-
- git-p4 clone //depot/project/main@all myproject
-
-
-
-If you want more control you can also use the git-p4 sync command directly:
-
- mkdir repo-git
- cd repo-git
- git init
- git-p4 sync //path/in/your/perforce/depot
-
-This will import the current head revision of the specified depot path into a
-"remotes/p4/master" branch of your git repository. You can use the
---branch=mybranch option to import into a different branch.
-
-If you want to import the entire history of a given depot path simply use:
-
- git-p4 sync //path/in/depot@all
-
-
-Note:
-
-To achieve optimal compression you may want to run 'git repack -a -d -f' after
-a big import. This may take a while.
-
-Incremental Imports
-===================
-
-After an initial import you can continue to synchronize your git repository
-with newer changes from the Perforce depot by just calling
-
- git-p4 sync
-
-in your git repository. By default the "remotes/p4/master" branch is updated.
-
-Advanced Setup
-==============
-
-Suppose you have a periodically updated git repository somewhere, containing a
-complete import of a Perforce project. This repository can be cloned and used
-with git-p4. When updating the cloned repository with the "sync" command,
-git-p4 will try to fetch changes from the original repository first. The git
-protocol used with this is usually faster than importing from Perforce
-directly.
-
-This behaviour can be disabled by setting the "git-p4.syncFromOrigin" git
-configuration variable to "false".
-
-Updating
-========
-
-A common working pattern is to fetch the latest changes from the Perforce depot
-and merge them with local uncommitted changes. The recommended way is to use
-git's rebase mechanism to preserve linear history. git-p4 provides a convenient
-
- git-p4 rebase
-
-command that calls git-p4 sync followed by git rebase to rebase the current
-working branch.
-
-Submitting
-==========
-
-git-p4 has support for submitting changes from a git repository back to the
-Perforce depot. This requires a Perforce checkout separate from your git
-repository. To submit all changes that are in the current git branch but not in
-the "p4" branch (or "origin" if "p4" doesn't exist) simply call
-
- git-p4 submit
-
-in your git repository. If you want to submit changes in a specific branch that
-is not your current git branch you can also pass that as an argument:
-
- git-p4 submit mytopicbranch
-
-You can override the reference branch with the --origin=mysourcebranch option.
-
-The Perforce changelists will be created with the user who ran git-p4. If you
-use --preserve-user then git-p4 will attempt to create Perforce changelists
-with the Perforce user corresponding to the git commit author. You need to
-have sufficient permissions within Perforce, and the git users need to have
-Perforce accounts. Permissions can be granted using 'p4 protect'.
-
-If a submit fails you may have to "p4 resolve" and submit manually. You can
-continue importing the remaining changes with
-
- git-p4 submit --continue
-
-Example
-=======
-
-# Clone a repository
- git-p4 clone //depot/path/project
-# Enter the newly cloned directory
- cd project
-# Do some work...
- vi foo.h
-# ... and commit locally to gi
- git commit foo.h
-# In the meantime somebody submitted changes to the Perforce depot. Rebase your latest
-# changes against the latest changes in Perforce:
- git-p4 rebase
-# Submit your locally committed changes back to Perforce
- git-p4 submit
-# ... and synchronize with Perforce
- git-p4 rebase
-
-
-Configuration parameters
-========================
-
-git-p4.user ($P4USER)
-
-Allows you to specify the username to use to connect to the Perforce repository.
-
- git config [--global] git-p4.user public
-
-git-p4.password ($P4PASS)
-
-Allows you to specify the password to use to connect to the Perforce repository.
-Warning this password will be visible on the command-line invocation of the p4 binary.
-
- git config [--global] git-p4.password public1234
-
-git-p4.port ($P4PORT)
-
-Specify the port to be used to contact the Perforce server. As this will be passed
-directly to the p4 binary, it may be in the format host:port as well.
-
- git config [--global] git-p4.port codes.zimbra.com:2666
-
-git-p4.host ($P4HOST)
-
-Specify the host to contact for a Perforce repository.
-
- git config [--global] git-p4.host perforce.example.com
-
-git-p4.client ($P4CLIENT)
-
-Specify the client name to use
-
- git config [--global] git-p4.client public-view
-
-git-p4.allowSubmit
-
- git config [--global] git-p4.allowSubmit false
-
-git-p4.syncFromOrigin
-
-A useful setup may be that you have a periodically updated git repository
-somewhere that contains a complete import of a Perforce project. That git
-repository can be used to clone the working repository from and one would
-import from Perforce directly after cloning using git-p4. If the connection to
-the Perforce server is slow and the working repository hasn't been synced for a
-while it may be desirable to fetch changes from the origin git repository using
-the efficient git protocol. git-p4 supports this setup by calling "git fetch origin"
-by default if there is an origin branch. You can disable this using:
-
- git config [--global] git-p4.syncFromOrigin false
-
-git-p4.useclientspec
-
- git config [--global] git-p4.useclientspec false
-
-The P4CLIENT environment variable should be correctly set for p4 to be
-able to find the relevant client. This client spec will be used to
-both filter the files cloned by git and set the directory layout as
-specified in the client (this implies --keep-path style semantics).
-
-git-p4.skipSubmitModTimeCheck
-
- git config [--global] git-p4.skipSubmitModTimeCheck false
-
-If true, submit will not check if the p4 change template has been modified.
-
-git-p4.preserveUser
-
- git config [--global] git-p4.preserveUser false
-
-If true, attempt to preserve user names by modifying the p4 changelists. See
-the "--preserve-user" submit option.
-
-git-p4.allowMissingPerforceUsers
-
- git config [--global] git-p4.allowMissingP4Users false
-
-If git-p4 is setting the perforce user for a commit (--preserve-user) then
-if there is no perforce user corresponding to the git author, git-p4 will
-stop. With allowMissingPerforceUsers set to true, git-p4 will use the
-current user (i.e. the behavior without --preserve-user) and carry on with
-the perforce commit.
-
-git-p4.skipUserNameCheck
-
- git config [--global] git-p4.skipUserNameCheck false
-
-When submitting, git-p4 checks that the git commits are authored by the current
-p4 user, and warns if they are not. This disables the check.
-
-git-p4.detectRenames
-
-Detect renames when submitting changes to Perforce server. Will enable -M git
-argument. Can be optionally set to a number representing the threshold
-percentage value of the rename detection.
-
- git config [--global] git-p4.detectRenames true
- git config [--global] git-p4.detectRenames 50
-
-git-p4.detectCopies
-
-Detect copies when submitting changes to Perforce server. Will enable -C git
-argument. Can be optionally set to a number representing the threshold
-percentage value of the copy detection.
-
- git config [--global] git-p4.detectCopies true
- git config [--global] git-p4.detectCopies 80
-
-git-p4.detectCopiesHarder
-
-Detect copies even between files that did not change when submitting changes to
-Perforce server. Will enable --find-copies-harder git argument.
-
- git config [--global] git-p4.detectCopies true
-
-git-p4.branchUser
-
-Only use branch specifications defined by the selected username.
-
- git config [--global] git-p4.branchUser username
-
-git-p4.branchList
-
-List of branches to be imported when branch detection is enabled.
-
- git config [--global] git-p4.branchList main:branchA
- git config [--global] --add git-p4.branchList main:branchB
-
-Implementation Details...
-=========================
-
-* Changesets from Perforce are imported using git fast-import.
-* The import does not require anything from the Perforce client view as it just uses
- "p4 print //depot/path/file#revision" to get the actual file contents.
-* Every imported changeset has a special [git-p4...] line at the
- end of the log message that gives information about the corresponding
- Perforce change number and is also used by git-p4 itself to find out
- where to continue importing when doing incremental imports.
- Basically when syncing it extracts the perforce change number of the
- latest commit in the "p4" branch and uses "p4 changes //depot/path/...@changenum,#head"
- to find out which changes need to be imported.
-* git-p4 submit uses "git rev-list" to pick the commits between the "p4" branch
- and the current branch.
- The commits themselves are applied using git diff/format-patch ... | git apply
-
--
1.7.8.rc4.30.g3c9dc
^ permalink raw reply related
* Evaluation of ref-api branch status
From: Michael Haggerty @ 2011-12-03 23:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git discussion list
Junio,
Now that 1.7.8 is out, I wanted to figure out the status of the
remaining ref-api changes that are in flight, including the differences
between between my tree and yours.
I understand that "next" will soon be re-rolled. Will the re-roll be
based on the current "pu", or will you start from scratch?
AFAIK there are three groups of ref-api patch series in flight, some of
which have gone through multiple iterations:
ref-api-B == mh/ref-api-2
[PATCH v3 00/14] Tidying up references code
ref-api-C == mh/ref-api-3
[PATCH v2 00/12] Use refs API more consistently
ref-api-D == mh/ref-api-take-2
[PATCH 00/28] Store references hierarchically in cache
On 2011-10-28 I sent re-rolled versions of ref-api-B and ref-api-C to
the list followed by the first version of ref-api-D. You committed
these three patch series to your repository as a single branch,
"mh/ref-api-take-2". The mh/ref-api-take-2 branch thus includes the
logical equivalent of all three of the patch series listed above.
Additionally, on 2011-11-15 I sent a fix to patch 26/28 of ref-api-D:
[PATCH] Fix "is_refname_available(): query only possibly-conflicting
references"
I don't see this patch in your tree at all.
Meanwhile, you added the following fix on top of master, mh/ref-api-2,
and ref-api-3:
refs: loosen over-strict "format" check
[Aside: I don't understand why you created three independent commits
rather than create an ur-commit on $(git merge-base master mh/ref-api-2
mh/ref-api-3) then merging it to each of the branches.]
Both mh/ref-api-2 (including your fix) and mh/ref-api-3 (including your
fix) have been merged into "next". You have never merged
mh/ref-api-take-2 anywhere. On the mh/ref-api-take-2 branch:
* ce766d41fa corresponds to mh/ref-api-2 (without your fix)
* 633ebc45c0 corresponds to mh/ref-api-3 (without your fix)
Therefore, your 633ebc45c0..mh/ref-api-take-2 corresponds to my newest
"ref-api-D" patch series. It does not include my fix from 2011-11-15.
Nor does it include any form of your fix "refs: loosen over-strict
"format" check".
What is the difference between the beginning of mh/ref-api-take-2 as
compared to mh/ref-api-2 + mh/ref-api-3? Equivalently, what is the
difference between the last versions of ref-api-B and ref-api-C vs.
earlier versions of those patch series? The differences are relatively
minor:
* A whitespace problem in the earlier version of the series was fixed.
This whitespace problem was introduced in caa80697 and has gotten into
"pu" (in refs.c starting at line 1093).
* resolve_gitlink_packed_ref() was simplified a bit and got a docstring.
* A function do_for_each_ref_in_arrays() was extracted.
* The series were rebased. The changes made to rebase them are roughly
equivalent to the work that you did in merges bea03b2455 and 773b817986.
(BTW, your merges look correct to me.)
* It does *not* include your fix "refs: loosen over-strict "format" check".
SUMMARY:
The patches from mh/ref-api-take-2 through 633ebc45c0 plus your fix are
slightly preferable to the patches in mh/ref-api-2 and mh/ref-api-3, in
the ways listed in the bullet-points above. If you are rewinding "next"
anyway, I suggest that you take the former instead of the latter. If
you prefer to stick with the latter, let me know and I will submit the
remaining differences as patches to be applied on top of mh/ref-api-3.
I hope that this summary is helpful to you. (It certainly helped me
figure out where things stand.)
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: Suggestion on hashing
From: Bill Zaumen @ 2011-12-03 21:21 UTC (permalink / raw)
To: Jeff King; +Cc: git, pclouds
In-Reply-To: <20111203150842.GA4442@sigill.intra.peff.net>
On Sat, 2011-12-03 at 10:08 -0500, Jeff King wrote:
> >
> > Suppose I make the digest pluggable, something I intended to do
> > eventually anyway? Then you just use the existing SHA-1 as an
> > object identifier and the new digest in a signature chain? What I
> > did was essentially to compute the new digest (using a CRC as the
> > trivial case) whenever an object's SHA-1 hash is computed, plus
> > using the new digest for low-cost collision checks.
>
> If you make the digest stronger (or pluggable) and include it in the
> actual objects themselves, then you have a start on (2).
>
> I'd drop all of the digest-exchange bits from the protocol, as the
> actual signatures are the real, trustable verification. I don't think
> you can drop the external storage of the digests, which is one of the
> ugliest bits. You'll be asking for the digests all the time to create
> new commit objects, so you need to have it at hand without rehashing.
The digest-exchange bits, including the tests and response to errors,
is only 222 lines of new code, so its really a minor part. The rest
takes care of what you referred to as "one of the ugliest bits," so
I think it is useful to have available - you can then try various ways
of improving the authentication of commit objects without having to do
a lot of initial work.
I can make those changes - probably over the next couple of weeks or
so (have some other non-related things to take care of) and then send
a new set of patches.
>
> And I wouldn't get my hopes up that this will go into git any time soon.
> At this point, we're really guessing about how broken SHA-1 will be in
> the future, and how much we are going to want to care.
>
> Just my two cents.
Thanks for the discussion. I might add that it is not just a question
of how broken SHA-1 is. If an IT department is considering adopting Git
as the company's revision control system and authentication is important
to the company, an IT manager may not accept SHA-1 for authentication
purposes because NIST claims SHA-1 is not adequate for authentication in
general and explaining to upper management why NIST's statement is not
applicable given the way SHA-1 is used in Git is much harder than
saying, "Git follows the current best practices regarding
authentication." That statement is a simple check-list item one can
show upper management in comparing alternatives.
Such issues (making technical choices for non-technical reasons) have
come up before - I once worked on a high-speed (for the time) networking
project and our manager mentioned that transferring medical records such
as X-ray pictures was one application - they do not accept lossy data
compression because, even if it is completely adequate, in a malpractice
suit, the plaintiff's lawyer would say, "And they purposely threw away
data critical to my client's health," which would sound pretty damning
to a typical jury. The legal risk outweighed the cost of the additional
bandwidth.
^ permalink raw reply
* [PATCH 2/2] builtin/apply.c: report error on failure to recognize input
From: Brandon Casey @ 2011-12-03 20:35 UTC (permalink / raw)
To: git; +Cc: artem.bityutskiy, Brandon Casey
In-Reply-To: <1322944550-27344-1-git-send-email-drafnel@gmail.com>
When git apply is passed something that is not a patch, it does not produce
an error message or exit with a non-zero status if it was not actually
"applying" the patch i.e. --check or --numstat etc were supplied on the
command line.
Fix this by producing an error when apply fails to find any hunks whatsoever
while parsing the patch.
This will cause some of the output formats (--numstat, --diffstat, etc) to
produce an error when they formerly would have reported zero changes and
exited successfully. That seems like the correct behavior though. Failure
to recognize the input as a patch should be an error.
Plus, add a test.
Reported-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
Initially, I was reluctant to change the error message, thinking that
error messages for plumbing commands were not supposed to change. But I
think I was wrong in that thought, so I changed the error message so it
was a more descriptive "unrecognized input".
-Brandon
builtin/apply.c | 10 +++++-----
t/t4136-apply-check.sh | 19 +++++++++++++++++++
2 files changed, 24 insertions(+), 5 deletions(-)
create mode 100755 t/t4136-apply-check.sh
diff --git a/builtin/apply.c b/builtin/apply.c
index 84a8a0b..46dcf3c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3590,15 +3590,12 @@ static int write_out_one_reject(struct patch *patch)
return -1;
}
-static int write_out_results(struct patch *list, int skipped_patch)
+static int write_out_results(struct patch *list)
{
int phase;
int errs = 0;
struct patch *l;
- if (!list && !skipped_patch)
- return error("No changes");
-
for (phase = 0; phase < 2; phase++) {
l = list;
while (l) {
@@ -3724,6 +3721,9 @@ static int apply_patch(int fd, const char *filename, int options)
offset += nr;
}
+ if (!list && !skipped_patch)
+ die("unrecognized input");
+
if (whitespace_error && (ws_error_action == die_on_ws_error))
apply = 0;
@@ -3741,7 +3741,7 @@ static int apply_patch(int fd, const char *filename, int options)
!apply_with_reject)
exit(1);
- if (apply && write_out_results(list, skipped_patch))
+ if (apply && write_out_results(list))
exit(1);
if (fake_ancestor)
diff --git a/t/t4136-apply-check.sh b/t/t4136-apply-check.sh
new file mode 100755
index 0000000..a321f7c
--- /dev/null
+++ b/t/t4136-apply-check.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+
+test_description='git apply should exit non-zero with unrecognized input.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit 1
+'
+
+test_expect_success 'apply --check exits non-zero with unrecognized input' '
+ test_must_fail git apply --check - <<-\EOF
+ I am not a patch
+ I look nothing like a patch
+ git apply must fail
+ EOF
+'
+
+test_done
--
1.7.8
^ permalink raw reply related
* [PATCH 1/2] t/t4131-apply-fake-ancestor.sh: fix broken test
From: Brandon Casey @ 2011-12-03 20:35 UTC (permalink / raw)
To: git; +Cc: artem.bityutskiy, Brandon Casey
The third test "apply --build-fake-ancestor in a subdirectory" has been
broken since it was introduced. It intended to modify a tracked file named
'sub/3.t' and then produce a diff which could be git apply'ed, but the file
named 'sub/3.t' does not exist. The file that exists in the repo is called
'sub/3'. Since no tracked files were modified, an empty diff was produced,
and the test succeeded.
Correct this test by supplying the intended name of the tracked file,
'sub/3.t', to test_commit in the first test.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
t/t4131-apply-fake-ancestor.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t4131-apply-fake-ancestor.sh b/t/t4131-apply-fake-ancestor.sh
index 94373ca..b1361ce 100755
--- a/t/t4131-apply-fake-ancestor.sh
+++ b/t/t4131-apply-fake-ancestor.sh
@@ -11,7 +11,7 @@ test_expect_success 'setup' '
test_commit 1 &&
test_commit 2 &&
mkdir sub &&
- test_commit 3 sub/3 &&
+ test_commit 3 sub/3.t &&
test_commit 4
'
--
1.7.8
^ permalink raw reply related
* Re: [PATCH, v3] git-tag: introduce --[no-]strip options
From: Jeff King @ 2011-12-03 20:04 UTC (permalink / raw)
To: Kirill A. Shutemov; +Cc: git, Junio C Hamano
In-Reply-To: <1322932376-15720-1-git-send-email-kirill@shutemov.name>
On Sat, Dec 03, 2011 at 07:12:56PM +0200, Kirill A. Shutemov wrote:
> From: "Kirill A. Shutemov" <kirill@shutemov.name>
>
> Normally git tag stripes tag message lines starting with '#', trailing
> spaces from every line and empty lines from the beginning and end.
>
> --no-strip is useful if you want to take a tag message as-is, without
> any stripping.
Perhaps this should mirror the "--clean" option from git-commit, as they
are basically doing the same thing? Besides the name difference, --clean
supports three modes: verbatim, whitespace, and strip. And defaults to
strip or whitespace depending on whether we are actually writing into
the editor.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox