I seek your consent
I am the Chairman of the Audit Committee of a renowned Bank in Singapore. I have a project worth lots of money and I believe you will be interested in it. If indeed you are, reply for specifics chmtsoo...@yahoo.com.sg This message was sent using IMP, the Internet Messaging Program. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-subtree: Avoid using echo -n even indirectly
On Wed, Oct 9, 2013 at 11:57 AM, Paolo G. Giarrusso
p.giarru...@gmail.com wrote:
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7d7af03..ebfb78f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -592,7 +592,9 @@ cmd_split()
eval $grl |
while read rev parents; do
revcount=$(($revcount + 1))
- say -n $revcount/$revmax ($createcount)
+ if [ -z $quiet ]; then
+ printf %s $revcount/$revmax ($createcount)
2
+ fi
Reviewers might wish to know that say in git-subtree is defined as
say()
{
if [ -z $quiet ]; then
echo $@ 2
fi
}
Hence the if and the redirect.
--
Cheers,
Ray Chuan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] diffcore-delta: optimize renames and copies detection (git diff -M/-C)
diffcore_count_changes() can return -1 when src_copied is greater than
delta_limit, without counting all the src_copied.
By that, performance of diff -M/-C can be improved.
Signed-off-by: Tsuneo Yoshioka yoshiokatsu...@gmail.com
---
diffcore-delta.c | 11 ---
diffcore-rename.c | 2 +-
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/diffcore-delta.c b/diffcore-delta.c
index 7cf431d..0a9290e 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -173,7 +173,7 @@ int diffcore_count_changes(struct diff_filespec *src,
{
struct spanhash *s, *d;
struct spanhash_top *src_count, *dst_count;
- unsigned long sc, la;
+ unsigned long sc, not_sc, la;
src_count = dst_count = NULL;
if (src_count_p)
@@ -190,7 +190,7 @@ int diffcore_count_changes(struct diff_filespec *src,
if (dst_count_p)
*dst_count_p = dst_count;
}
- sc = la = 0;
+ sc = not_sc = la = 0;
s = src_count-data;
d = dst_count-data;
@@ -214,8 +214,13 @@ int diffcore_count_changes(struct diff_filespec *src,
la += dst_cnt - src_cnt;
sc += src_cnt;
}
- else
+ else{
sc += dst_cnt;
+ not_sc += (src_cnt - dst_cnt);
+ if(delta_limit != 0 not_sc delta_limit){
+ return -1;
+ }
+ }
s++;
}
while (d-cnt) {
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 6c7a72f..d52b2c8 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -181,7 +181,7 @@ static int estimate_similarity(struct diff_filespec *src,
return 0;
delta_limit = (unsigned long)
- (base_size * (MAX_SCORE-minimum_score) / MAX_SCORE);
+ (max_size * (MAX_SCORE-minimum_score) / MAX_SCORE);
if (diffcore_count_changes(src, dst,
src-cnt_data, dst-cnt_data,
delta_limit,
--
1.7.12.4 (Apple Git-37)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
(Resending without HTML).
On Wed, Oct 9, 2013 at 12:20 PM, Tay Ray Chuan rcta...@gmail.com wrote:
On Wed, Oct 9, 2013 at 11:57 AM, Paolo G. Giarrusso
p.giarru...@gmail.com wrote:
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7d7af03..ebfb78f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -592,7 +592,9 @@ cmd_split()
eval $grl |
while read rev parents; do
revcount=$(($revcount + 1))
- say -n $revcount/$revmax ($createcount)
+ if [ -z $quiet ]; then
+ printf %s $revcount/$revmax ($createcount)
2
An additional note for reviewers and appliers: the original and the
patched codeboth embed a literal ^M, not a new line, go to back to the
beginning of the line and overwrite it, so the above is not a
consequence of line-wrap.
I used git-format-patch and git-send-email, and the ^M is visible in
Vim in the exported patch (that's why I didn't remark on it).
Seeing the email, I wonder whether there's hope something like that
can be preserved in an email, and whether the code should use some
escape sequence instead.
+ fi
Reviewers might wish to know that say in git-subtree is defined as
say()
{
if [ -z $quiet ]; then
echo $@ 2
fi
}
Hence the if and the redirect.
Indeed. I considered having a variant of `say` instead of inlining and
customizing it, but for once I decided to keep this simple, since this
variant of `say` is currently used only once. Otherwise, one could
change say to use printf, but that's more invasive.
Cheers,
--
Paolo G. Giarrusso - Ph.D. Student, Philipps-University Marburg
http://www.informatik.uni-marburg.de/~pgiarrusso/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
Am 10/9/2013 12:32, schrieb Paolo Giarrusso: On Wed, Oct 9, 2013 at 12:20 PM, Tay Ray Chuan rcta...@gmail.com wrote: On Wed, Oct 9, 2013 at 11:57 AM, Paolo G. Giarrusso p.giarru...@gmail.com wrote: diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh index 7d7af03..ebfb78f 100755 --- a/contrib/subtree/git-subtree.sh +++ b/contrib/subtree/git-subtree.sh @@ -592,7 +592,9 @@ cmd_split() eval $grl | while read rev parents; do revcount=$(($revcount + 1)) - say -n $revcount/$revmax ($createcount) + if [ -z $quiet ]; then + printf %s $revcount/$revmax ($createcount) 2 An additional note for reviewers and appliers: the original and the patched codeboth embed a literal ^M, ... whether the code should use some escape sequence instead. As you are using printf, you can easily do: printf '%s\r' ... without using ^M. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
Paolo Giarrusso p.giarru...@gmail.com writes: Otherwise, one could change say to use printf, but that's more invasive. invasive in the sense that it impacts indirectly more callers, but are there really cases where echo is needed when calling say? Aren't there other potential bugs when arbitrary strings are passed to say, that would be fixed by using printf once and for all? The patch would look like the one I did in 89b0230a20 (Wed Aug 7 2013, die_with_status: use printf '%s\n', not echo). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
On Wed, Oct 9, 2013 at 1:26 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Paolo Giarrusso p.giarru...@gmail.com writes: Otherwise, one could change say to use printf, but that's more invasive. invasive in the sense that it impacts indirectly more callers, but are there really cases where echo is needed when calling say? Aren't there other potential bugs when arbitrary strings are passed to say, that would be fixed by using printf once and for all? (1) Changing the implementation of say to use printf %s\n would be trivial, and I think would address your concerns. But I was concerned about code duplication; one could additionally make say reusable in this single call site, instead of inlining and customizing it by replacing the \n with \r. But for that, you need to either (2) add an explicit \n to all callers (invasive error prone), or (3) make `say` parse the `-n` option and conditionally add \n to the format string or to a final argument, if -n is not specified; this would affect no current caller, but complicate the implementation of say. Doing that for just one call site has too much potential for breakage, so I'm not sure I'd do it. (I'm not even sure on what should `say` do when `-n` is not the first argument). Options (1), (2) and (3) are mutually alternative; my favorite is (1). I can see your points about opportunity, especially after looking at the commit message of the patch of yours you linked. The patch would look like the one I did in 89b0230a20 (Wed Aug 7 2013, die_with_status: use printf '%s\n', not echo). I see your point. But note that using printf like in die_with_status after that commit wouldn't be reusable here in all call sites, because it always prints a newline. Cheers, -- Paolo G. Giarrusso - Ph.D. Student, Philipps-University Marburg http://www.informatik.uni-marburg.de/~pgiarrusso/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCHv2 1/3] Documentation/git-svn: Promote the use of --prefix in docs + examples
On Wed, Oct 9, 2013 at 3:33 AM, Eric Sunshine sunsh...@sunshineco.com wrote: On Sat, Oct 5, 2013 at 7:30 PM, Johan Herland jo...@herland.net wrote: diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 4dd3bcb..da00671 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -973,6 +979,15 @@ without giving any repository layout options. If the full history with branches and tags is required, the options '--trunk' / '--branches' / '--tags' must be used. +When using the options for describing the repository layout (--trunk, +--tags, --branches, --stdlayout), please also specify the --prefix +option (e.g. '--prefix=origin/') to cause your SVN-tracking refs to be +placed at refs/remotes/origin/* rather than the default refs/remotes/*. +The former is more compatible with the layout of Git's regular +remote-tracking refs (refs/remotes/$remote/*), and may potentially +prevent similarly named SVN branches and Git remotes from clobbering +eachother. s/eachother/each other/ Thanks, will fix. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCHv2 2/3] git-svn: Warn about changing default for --prefix in Git v2.0
On Wed, Oct 9, 2013 at 3:34 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Sat, Oct 5, 2013 at 7:30 PM, Johan Herland jo...@herland.net wrote:
diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
index b7ef9e2..1c8d049 100755
--- a/t/t9117-git-svn-init-clone.sh
+++ b/t/t9117-git-svn-init-clone.sh
@@ -52,4 +52,71 @@ test_expect_success 'clone to target directory with
--stdlayout' '
rm -rf target
'
+test_expect_success 'clone without -s/-T/-b/-t does not warn' '
+ test ! -d trunk
+ git svn clone $svnrepo/project/trunk 2warning
+ test_must_fail grep -q prefix warning
+ rm -rf trunk
+ rm -f warning
+ '
+
+test_svn_configured_prefix () {
+ prefix=$1
Did you want to maintain the -chain here?
+ cat expect EOF
And here?
Yes, will add in both places. Thanks.
...Johan
--
Johan Herland, jo...@herland.net
www.herland.net
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] checkout tests: enable test with complex relative path
This test was put in, but commented out in fed1b5ca (2007-11-09, git-checkout: Test for relative path use.) It's been a while since 2007 and the intended test case works now. (I could not find the enabling commit in ls-files however.) The code in question however did not change into the sub directory, so we still need to add a 'cd'. Also a test for the file content has been added. This is already part of the other tests for checkout. Signed-off-by: Stefan Beller stefanbel...@googlemail.com --- t/t2008-checkout-subdir.sh | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh index 3e098ab..399655f 100755 --- a/t/t2008-checkout-subdir.sh +++ b/t/t2008-checkout-subdir.sh @@ -58,13 +58,14 @@ test_expect_success 'checkout with simple prefix' ' ' -# This is not expected to work as ls-files was not designed -# to deal with such. Enable it when ls-files is updated. -: test_expect_success 'checkout with complex relative path' ' - - rm file1 - git checkout HEAD -- ../dir1/../dir1/file1 test -f ./file1 - +test_expect_success 'checkout with complex relative path' ' + ( + cd dir1 + rm file1 + git checkout HEAD -- ../dir1/../dir1/file1 + test -f file1 + test hello = $(cat file1) + ) ' test_expect_success 'relative path outside tree should fail' \ -- 1.8.4.1.471.g53f64e4.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] pv4_tree_desc: don't bother looking for v4 trees if no v4 packs are present
This is mostly to avoid overhead on v2 only systems.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 5 -
sha1_file.c| 5 +
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index 7d257af..4354ee3 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -12,6 +12,8 @@
#include packv4-parse.h
#include varint.h
+int packv4_available;
+
const unsigned char *get_sha1ref(struct packed_git *p,
const unsigned char **bufp)
{
@@ -749,7 +751,8 @@ int pv4_tree_desc_from_sha1(struct pv4_tree_desc *desc,
strbuf_init(desc-buf, 0);
memset(oi, 0, sizeof(oi));
- if (!sha1_object_info_extended(sha1, oi)
+ if (packv4_available
+ !sha1_object_info_extended(sha1, oi)
oi.whence == OI_PACKED
oi.u.packed.real_type == OBJ_PV4_TREE
oi.u.packed.pack-version = 4) {
diff --git a/sha1_file.c b/sha1_file.c
index 5848008..4744132 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -40,6 +40,7 @@ const unsigned char null_sha1[20];
static const char *no_log_pack_access = no_log_pack_access;
static const char *log_pack_access;
+extern int packv4_available;
/*
* This is meant to hold a *small* number of objects that you would
@@ -763,6 +764,8 @@ void free_pack_by_name(const char *pack_name)
close(p-pack_fd);
pack_open_fds--;
}
+ if (p-version = 4)
+ packv4_available--;
close_pack_index(p);
free(p-bad_object_sha1);
pv4_free_dict(p-ident_dict);
@@ -856,6 +859,8 @@ static int open_packed_git_1(struct packed_git *p)
supported (try upgrading GIT to a newer version),
p-pack_name, ntohl(hdr.hdr_version));
p-version = ntohl(hdr.hdr_version);
+ if (p-version = 4)
+ packv4_available++;
/* Verify the pack matches its index. */
if (p-num_objects != ntohl(hdr.hdr_entries))
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] pv4_tree_desc: introduce new struct for pack v4 tree walker
This struct is intended to be the successor of struct tree_desc. For
now it only holds a buffer for converting pv4 tree to canonical format.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 73 --
packv4-parse.h | 9
2 files changed, 44 insertions(+), 38 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index 7b096cb..f9db364 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -344,9 +344,8 @@ void *pv4_get_commit(struct packed_git *p, struct
pack_window **w_curs,
return dst;
}
-static int copy_canonical_tree_entries(struct packed_git *p, off_t offset,
- unsigned int start, unsigned int count,
- unsigned char **dstp, unsigned long
*sizep)
+static int copy_canonical_tree_entries(struct pv4_tree_desc *v4, off_t offset,
+ unsigned int start, unsigned int count)
{
void *data;
const unsigned char *from, *end;
@@ -354,7 +353,7 @@ static int copy_canonical_tree_entries(struct packed_git
*p, off_t offset,
unsigned long size;
struct tree_desc desc;
- data = unpack_entry(p, offset, type, size);
+ data = unpack_entry(v4-p, offset, type, size);
if (!data)
return -1;
if (type != OBJ_TREE) {
@@ -372,13 +371,11 @@ static int copy_canonical_tree_entries(struct packed_git
*p, off_t offset,
update_tree_entry(desc);
end = desc.buffer;
- if (end - from *sizep) {
+ if (end - from strbuf_avail(v4-buf)) {
free(data);
return -1;
}
- memcpy(*dstp, from, end - from);
- *dstp += end - from;
- *sizep -= end - from;
+ strbuf_add(v4-buf, from, end - from);
free(data);
return 0;
}
@@ -417,7 +414,7 @@ static struct pv4_tree_cache *get_tree_offset_cache(struct
packed_git *p, off_t
return c;
}
-static int tree_entry_prefix(unsigned char *buf, unsigned long size,
+static int tree_entry_prefix(char *buf, unsigned long size,
const unsigned char *path, int path_len,
unsigned mode)
{
@@ -443,35 +440,33 @@ static int tree_entry_prefix(unsigned char *buf, unsigned
long size,
return len;
}
-static int generate_tree_entry(struct packed_git *p,
+static int generate_tree_entry(struct pv4_tree_desc *desc,
const unsigned char **bufp,
- unsigned char **dstp, unsigned long *sizep,
int what)
{
const unsigned char *path, *sha1;
+ char *buf = desc-buf.buf + desc-buf.len;
unsigned mode;
int len, pathlen;
- path = get_pathref(p, what 1, pathlen);
- sha1 = get_sha1ref(p, bufp);
+ path = get_pathref(desc-p, what 1, pathlen);
+ sha1 = get_sha1ref(desc-p, bufp);
if (!path || !sha1)
return -1;
mode = (path[0] 8) | path[1];
- len = tree_entry_prefix(*dstp, *sizep,
+ len = tree_entry_prefix(buf, strbuf_avail(desc-buf),
path + 2, pathlen - 2, mode);
- if (!len || len + 20 *sizep)
+ if (!len || len + 20 strbuf_avail(desc-buf))
return -1;
- hashcpy(*dstp + len, sha1);
- len += 20;
- *dstp += len;
- *sizep -= len;
+ memcpy(buf + len, sha1, 20);
+ desc-buf.len += len + 20;
return 0;
}
-static int decode_entries(struct packed_git *p, struct pack_window **w_curs,
- off_t obj_offset, unsigned int start, unsigned int
count,
- unsigned char **dstp, unsigned long *sizep)
+static int decode_entries(struct pv4_tree_desc *desc, off_t obj_offset,
+ unsigned int start, unsigned int count)
{
+ struct packed_git *p = desc-p;
unsigned long avail;
const unsigned char *src, *scp;
unsigned int curpos;
@@ -490,7 +485,7 @@ static int decode_entries(struct packed_git *p, struct
pack_window **w_curs,
} else {
unsigned int nb_entries;
- src = use_pack(p, w_curs, obj_offset, avail);
+ src = use_pack(p, desc-w_curs, obj_offset, avail);
scp = src;
/* we need to skip over the object header */
@@ -502,9 +497,8 @@ static int decode_entries(struct packed_git *p, struct
pack_window **w_curs,
/* is this a canonical tree object? */
case OBJ_TREE:
case OBJ_REF_DELTA:
- return copy_canonical_tree_entries(p, obj_offset,
- start, count,
- dstp, sizep);
+ return copy_canonical_tree_entries(desc, obj_offset,
+
[PATCH 6/9] pv4_tree_desc: complete interface
Best explained with an example
void walk(const unsigned char *sha1)
{
struct pv4_tree_desc desc;
/*
* Start pv4_tree_desc from an SHA-1. If it's a v4 tree, v4 walker
* will be used. Otherwise v2 is walked.
*/
pv4_tree_desc_from_sha1(desc, sha1, 0);
recurse(desc);
pv4_release_tree_desc(desc);
}
void recurse(struct pv4_tree_desc *desc)
{
/*
* Then you can go over entries, one by one, similar to the
* current tree walker. Current entry is in desc-v2.entry.
* Pathlen in desc-pathlen. Do not use tree_entry_len() because
* that one is only correct for v2 entries
*/
while (pv4_get_entry(desc)) {
printf(%s %s\n, sha1_to_hex(desc-v2.entry.sha1),
desc-v2.entry.path);
/*
* Once you have an initialized pv4_tree_desc you may skip the
* SHA-1 lookup step if the next tree is in the same pack.
*/
if (S_ISDIR(desc-v2.entry.mode)) {
struct pv4_tree_desc new_desc;
pv4_tree_desc_from_entry(new_desc, desc);
recurse(new_desc);
/* Finally release everything */
pv4_release_tree_desc(new_desc);
}
}
}
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 80 ++
packv4-parse.h | 12 +
2 files changed, 92 insertions(+)
diff --git a/packv4-parse.c b/packv4-parse.c
index f222456..7d257af 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -732,3 +732,83 @@ unsigned long pv4_unpack_object_header_buffer(const
unsigned char *base,
*sizep = val 4;
return cp - base;
}
+
+int pv4_tree_desc_from_sha1(struct pv4_tree_desc *desc,
+ const unsigned char *sha1,
+ unsigned flags)
+{
+ unsigned long size;
+ enum object_type type;
+ void *data;
+ struct object_info oi;
+
+ assert(!(flags ~0xff)
+ you are not supposed to set these from outside!);
+
+ memset(desc, 0, sizeof(*desc));
+ strbuf_init(desc-buf, 0);
+
+ memset(oi, 0, sizeof(oi));
+ if (!sha1_object_info_extended(sha1, oi)
+ oi.whence == OI_PACKED
+ oi.u.packed.real_type == OBJ_PV4_TREE
+ oi.u.packed.pack-version = 4) {
+ desc-p = oi.u.packed.pack;
+ desc-obj_offset = oi.u.packed.offset;
+ desc-flags = flags;
+ return 0;
+ }
+
+ data = read_sha1_file(sha1, type, size);
+ if (!data || type != OBJ_TREE) {
+ free(data);
+ return -1;
+ }
+ desc-flags = flags;
+ desc-flags |= PV4_TREE_CANONICAL;
+ init_tree_desc(desc-v2, data, size);
+ /*
+* we can attach to strbuf because read_sha1_file always
+* appends NUL at the end
+*/
+ strbuf_attach(desc-buf, data, size, size + 1);
+ return 0;
+}
+
+int pv4_tree_desc_from_entry(struct pv4_tree_desc *desc,
+const struct pv4_tree_desc *src,
+unsigned flags)
+{
+ if (!src-sha1_index)
+ return pv4_tree_desc_from_sha1(desc,
+ src-v2.entry.sha1,
+ flags);
+ assert(!(flags ~0xff)
+ you are not supposed to set these from outside!);
+ memset(desc, 0, sizeof(*desc));
+ strbuf_init(desc-buf, 0);
+ desc-p = src-p;
+ desc-obj_offset =
+ nth_packed_object_offset(desc-p, src-sha1_index - 1);
+ desc-flags = flags;
+ return 0;
+}
+
+void pv4_release_tree_desc(struct pv4_tree_desc *desc)
+{
+ strbuf_release(desc-buf);
+ unuse_pack(desc-w_curs);
+}
+
+int pv4_tree_entry(struct pv4_tree_desc *desc)
+{
+ if (desc-flags PV4_TREE_CANONICAL) {
+ if (!desc-v2.size)
+ return 0;
+ if (desc-start)
+ update_tree_entry(desc-v2);
+ desc-start++;
+ return 1;
+ }
+ return !decode_entries(desc, desc-obj_offset, desc-start++, 1);
+}
diff --git a/packv4-parse.h b/packv4-parse.h
index fe0ea38..874f57c 100644
--- a/packv4-parse.h
+++ b/packv4-parse.h
@@ -36,6 +36,8 @@ struct pv4_tree_desc {
/* v4 entry */
struct packed_git *p;
struct pack_window *w_curs;
+ off_t obj_offset;
+ unsigned start;
unsigned int sha1_index;
int pathlen;
@@ -46,4 +48,14 @@ struct pv4_tree_desc {
struct strbuf buf;
};
+int pv4_tree_desc_from_sha1(struct pv4_tree_desc *desc,
+ const unsigned char *sha1,
+ unsigned flags);
+int pv4_tree_desc_from_entry(struct pv4_tree_desc *desc,
+const struct pv4_tree_desc *src,
+unsigned flags);
+void pv4_release_tree_desc(struct pv4_tree_desc *desc);
+
[PATCH 2/9] pack v4: move v2 tree entry generation code out of decode_entries
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index 31c89c7..7b096cb 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -443,6 +443,31 @@ static int tree_entry_prefix(unsigned char *buf, unsigned
long size,
return len;
}
+static int generate_tree_entry(struct packed_git *p,
+ const unsigned char **bufp,
+ unsigned char **dstp, unsigned long *sizep,
+ int what)
+{
+ const unsigned char *path, *sha1;
+ unsigned mode;
+ int len, pathlen;
+
+ path = get_pathref(p, what 1, pathlen);
+ sha1 = get_sha1ref(p, bufp);
+ if (!path || !sha1)
+ return -1;
+ mode = (path[0] 8) | path[1];
+ len = tree_entry_prefix(*dstp, *sizep,
+ path + 2, pathlen - 2, mode);
+ if (!len || len + 20 *sizep)
+ return -1;
+ hashcpy(*dstp + len, sha1);
+ len += 20;
+ *dstp += len;
+ *sizep -= len;
+ return 0;
+}
+
static int decode_entries(struct packed_git *p, struct pack_window **w_curs,
off_t obj_offset, unsigned int start, unsigned int
count,
unsigned char **dstp, unsigned long *sizep)
@@ -543,22 +568,8 @@ static int decode_entries(struct packed_git *p, struct
pack_window **w_curs,
/*
* This is an actual tree entry to recreate.
*/
- const unsigned char *path, *sha1;
- unsigned mode;
- int len, pathlen;
-
- path = get_pathref(p, what 1, pathlen);
- sha1 = get_sha1ref(p, scp);
- if (!path || !sha1)
- return -1;
- mode = (path[0] 8) | path[1];
- len = tree_entry_prefix(*dstp, *sizep,
- path + 2, pathlen - 2, mode);
- if (!len || len + 20 *sizep)
+ if (generate_tree_entry(p, scp, dstp, sizep, what))
return -1;
- hashcpy(*dstp + len, sha1);
- *dstp += len + 20;
- *sizep -= len + 20;
count--;
curpos++;
} else if (what 1) {
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] pv4_tree_desc: allow decode_entries to return v4 trees, one at a time
When PV4_TREE_CANONICAL is passed, decode_entries() generates count
tree entries in canonical format. When this flag is not passed _and_
count is 1, decode_entries fills struct name_entry and saves
sha1_index.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 44 ++--
packv4-parse.h | 10 ++
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index f5c486e..f222456 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -365,6 +365,12 @@ static int copy_canonical_tree_entries(struct
pv4_tree_desc *v4, off_t offset,
while (start--)
update_tree_entry(desc);
+ if (!(v4-flags PV4_TREE_CANONICAL)) {
+ v4-sha1_index = 0;
+ v4-pathlen = tree_entry_len(desc-entry);
+ return 0;
+ }
+
from = desc-buffer;
while (count--)
update_tree_entry(desc);
@@ -462,6 +468,33 @@ static int generate_tree_entry(struct pv4_tree_desc *desc,
return 0;
}
+static int get_tree_entry_v4(struct pv4_tree_desc *desc,
+const unsigned char **bufp,
+int what)
+{
+ const unsigned char *path;
+
+ path = get_pathref(desc-p, what 1, desc-pathlen);
+ if (!path)
+ return -1;
+ desc-v2.entry.mode = (path[0] 8) | path[1];
+ desc-v2.entry.path = (const char *)path + 2;
+
+ if (**bufp) {
+ desc-sha1_index = decode_varint(bufp);
+ if (desc-sha1_index 1 ||
+ desc-sha1_index - 1 desc-p-num_objects)
+ return error(bad index in get_sha1ref);
+ desc-v2.entry.sha1 = desc-p-sha1_table + (desc-sha1_index -
1) * 20;
+ } else {
+ desc-sha1_index = 0;
+ desc-v2.entry.sha1 = *bufp + 1;
+ *bufp += 21;
+ }
+
+ return 0;
+}
+
static int decode_entries(struct pv4_tree_desc *desc, off_t obj_offset,
unsigned int start, unsigned int count)
{
@@ -561,8 +594,14 @@ static int decode_entries(struct pv4_tree_desc *desc,
off_t obj_offset,
/*
* This is an actual tree entry to recreate.
*/
- if (generate_tree_entry(desc, scp, what))
- return -1;
+ if (desc-flags PV4_TREE_CANONICAL) {
+ if (generate_tree_entry(desc, scp, what))
+ return -1;
+ } else if (count == 1) {
+ if (get_tree_entry_v4(desc, scp, what))
+ return -1;
+ } else
+ die(generating multiple v4 entries is not
supported);
count--;
curpos++;
} else if (what 1) {
@@ -668,6 +707,7 @@ void *pv4_get_tree(struct packed_git *p, struct pack_window
**w_curs,
int ret;
memset(desc, 0, sizeof(desc));
+ desc.flags = PV4_TREE_CANONICAL;
desc.p = p;
desc.w_curs = *w_curs;
strbuf_init(desc.buf, size);
diff --git a/packv4-parse.h b/packv4-parse.h
index 04b9a59..fe0ea38 100644
--- a/packv4-parse.h
+++ b/packv4-parse.h
@@ -24,10 +24,20 @@ void *pv4_get_commit(struct packed_git *p, struct
pack_window **w_curs,
void *pv4_get_tree(struct packed_git *p, struct pack_window **w_curs,
off_t obj_offset, unsigned long size);
+/*
+ * These are private flags, never pass them directly to
+ * pv4_tree_desc_*
+ */
+#define PV4_TREE_CANONICAL 0x800
+
struct pv4_tree_desc {
+ unsigned flags;
+
/* v4 entry */
struct packed_git *p;
struct pack_window *w_curs;
+ unsigned int sha1_index;
+ int pathlen;
/* v2 entry */
struct tree_desc v2;
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] pv4_tree_desc: use struct tree_desc from pv4_tree_desc
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 13 ++---
packv4-parse.h | 5 +
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index f9db364..f5c486e 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -10,7 +10,6 @@
#include cache.h
#include packv4-parse.h
-#include tree-walk.h
#include varint.h
const unsigned char *get_sha1ref(struct packed_git *p,
@@ -351,7 +350,7 @@ static int copy_canonical_tree_entries(struct pv4_tree_desc
*v4, off_t offset,
const unsigned char *from, *end;
enum object_type type;
unsigned long size;
- struct tree_desc desc;
+ struct tree_desc *desc = v4-v2;
data = unpack_entry(v4-p, offset, type, size);
if (!data)
@@ -361,15 +360,15 @@ static int copy_canonical_tree_entries(struct
pv4_tree_desc *v4, off_t offset,
return -1;
}
- init_tree_desc(desc, data, size);
+ init_tree_desc(desc, data, size);
while (start--)
- update_tree_entry(desc);
+ update_tree_entry(desc);
- from = desc.buffer;
+ from = desc-buffer;
while (count--)
- update_tree_entry(desc);
- end = desc.buffer;
+ update_tree_entry(desc);
+ end = desc-buffer;
if (end - from strbuf_avail(v4-buf)) {
free(data);
diff --git a/packv4-parse.h b/packv4-parse.h
index cad7a82..04b9a59 100644
--- a/packv4-parse.h
+++ b/packv4-parse.h
@@ -1,6 +1,8 @@
#ifndef PACKV4_PARSE_H
#define PACKV4_PARSE_H
+#include tree-walk.h
+
struct packv4_dict {
const unsigned char *data;
unsigned int nb_entries;
@@ -27,6 +29,9 @@ struct pv4_tree_desc {
struct packed_git *p;
struct pack_window *w_curs;
+ /* v2 entry */
+ struct tree_desc v2;
+
/* full canonical tree */
struct strbuf buf;
};
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] pv4_tree_desc: avoid lookup_object() when possible
pv4_tree_desc_from_entry() cuts out SHA-1 index lookups when
possible. This patch provides a new set of lookup functions that avoid
looking up object hash table.
We maintain an object pointer array and use SHA-1 table as
key. Because we know index in SHA-1 table in v4 trees, we can skip
binary search and go straight to the object.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h| 1 +
packv4-parse.c | 33 +
packv4-parse.h | 12
sha1_file.c| 1 +
4 files changed, 47 insertions(+)
diff --git a/cache.h b/cache.h
index 5028ded..da65063 100644
--- a/cache.h
+++ b/cache.h
@@ -1035,6 +1035,7 @@ extern struct packed_git {
struct packv4_dict *ident_dict;
off_t ident_dict_end;
struct packv4_dict *path_dict;
+ struct object **objs;
time_t mtime;
int pack_fd;
unsigned pack_local:1,
diff --git a/packv4-parse.c b/packv4-parse.c
index 4354ee3..6f6152c 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -11,6 +11,10 @@
#include cache.h
#include packv4-parse.h
#include varint.h
+#include commit.h
+#include tree.h
+#include blob.h
+#include tag.h
int packv4_available;
@@ -815,3 +819,32 @@ int pv4_tree_entry(struct pv4_tree_desc *desc)
}
return !decode_entries(desc, desc-obj_offset, desc-start++, 1);
}
+
+static struct object **get_packed_objs(struct pv4_tree_desc *desc)
+{
+ if (!desc-p || !desc-sha1_index)
+ return NULL;
+ if (desc-p-version = 4 !desc-p-objs)
+ desc-p-objs =
+ xmalloc(sizeof(struct object *) * desc-p-num_objects);
+ return desc-p-objs;
+}
+
+#define DEFINE_LOOKUP(TYPE)\
+struct TYPE *pv4_lookup_##TYPE(struct pv4_tree_desc *desc) \
+{ \
+ struct object **objs = get_packed_objs(desc); \
+ if (!objs) \
+ return lookup_##TYPE(desc-v2.entry.sha1); \
+ objs += desc-sha1_index - 1; \
+ if (!*objs) \
+ *objs = (struct object *) \
+ lookup_##TYPE(desc-v2.entry.sha1); \
+ return (struct TYPE *)objs[0]; \
+}
+
+DEFINE_LOOKUP(object)
+DEFINE_LOOKUP(commit)
+DEFINE_LOOKUP(tree)
+DEFINE_LOOKUP(blob)
+DEFINE_LOOKUP(tag)
diff --git a/packv4-parse.h b/packv4-parse.h
index 874f57c..3bf69bc 100644
--- a/packv4-parse.h
+++ b/packv4-parse.h
@@ -3,6 +3,12 @@
#include tree-walk.h
+struct object;
+struct commit;
+struct tree;
+struct blob;
+struct tag;
+
struct packv4_dict {
const unsigned char *data;
unsigned int nb_entries;
@@ -58,4 +64,10 @@ void pv4_release_tree_desc(struct pv4_tree_desc *desc);
int pv4_tree_entry(struct pv4_tree_desc *desc);
+struct object *pv4_lookup_object(struct pv4_tree_desc *desc);
+struct commit *pv4_lookup_commit(struct pv4_tree_desc *desc);
+struct tree *pv4_lookup_tree(struct pv4_tree_desc *desc);
+struct blob *pv4_lookup_blob(struct pv4_tree_desc *desc);
+struct tag*pv4_lookup_tag(struct pv4_tree_desc *desc);
+
#endif
diff --git a/sha1_file.c b/sha1_file.c
index 4744132..88a6273 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -773,6 +773,7 @@ void free_pack_by_name(const char *pack_name)
*pp = p-next;
if (last_found_pack == p)
last_found_pack = NULL;
+ free(p-objs);
free(p);
return;
}
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[BAD PATCH 0/9] v4-aware tree walker API
I know I still have a lot of holes to plug, but this was more interesting because we could see some encouraging numbers. Unfortunately the result is disappointing. Maybe I did it in a stupid way and need to restart with a totally different way. rev-list --objects on v2 takes 4 secs, v4 with current walker 11s and the new walker 16s (worst!). perf's top functions with v2 are 23,51% git libz.so.1.2.7 [.] inflate 16,66% git git [.] lookup_object 11,46% git libz.so.1.2.7 [.] inflate_fast 6,89% git libc-2.16.so[.] __memcpy_ssse3_back 4,19% git libz.so.1.2.7 [.] inflate_table 4,15% git git [.] find_pack_entry_one 3,84% git git [.] decode_tree_entry and with new walker 58,61% git git[.] decode_entries 18,66% git git[.] decode_varint 9,73% git git[.] use_pack 3,31% git git[.] nth_packed_object_offset 1,73% git git[.] process_tree 1,66% git git[.] pv4_lookup_blob 1,09% git git[.] get_pathref 1,03% git libc-2.16.so [.] __memcpy_ssse3_back 0,90% git libz.so.1.2.7 [.] inflate 0,50% git libz.so.1.2.7 [.] inflate_table It's no surprise that lookup_object is no longer hot. The closet is pv4_lookup_blob. nth_packed_object_offset is getting hotter as it's used extensively by decode_entries. And decode_entries is getting t hot. This function is now called for each tree entry of every tree. And it does get_tree_offset_cache() lookup for every call (ironically we try hard to avoid hash lookup in lookup_object). The only bit I haven't done is avoid checking if a tree is already examined, if so do not bother with copy sequences referring to it. That should cut down the number of decode_entries but not sure how much because there's no relation between tree traversing order and how copy sequences are made. Maybe we could make an exception and allow the tree walker to pass pv4_tree_cache* directly to decode_entries so it does not need to do the first lookup every time.. Suggestions? Nguyễn Thái Ngọc Duy (9): sha1_file: provide real packed type in object_info_extended pack v4: move v2 tree entry generation code out of decode_entries pv4_tree_desc: introduce new struct for pack v4 tree walker pv4_tree_desc: use struct tree_desc from pv4_tree_desc pv4_tree_desc: allow decode_entries to return v4 trees, one at a time pv4_tree_desc: complete interface pv4_tree_desc: don't bother looking for v4 trees if no v4 packs are present pv4_tree_desc: avoid lookup_object() when possible list-object.c: take advantage of new pv4_tree_desc interface cache.h| 3 +- list-objects.c | 38 + packv4-parse.c | 263 ++--- packv4-parse.h | 48 +++ sha1_file.c| 9 +- streaming.c| 9 +- 6 files changed, 300 insertions(+), 70 deletions(-) -- 1.8.2.83.gc99314b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] list-object.c: take advantage of new pv4_tree_desc interface
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
list-objects.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index 6def897..39ad3e6 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -7,6 +7,7 @@
#include tree-walk.h
#include revision.h
#include list-objects.h
+#include packv4-parse.h
static void process_blob(struct rev_info *revs,
struct blob *blob,
@@ -60,6 +61,7 @@ static void process_gitlink(struct rev_info *revs,
}
static void process_tree(struct rev_info *revs,
+struct pv4_tree_desc *desc,
struct tree *tree,
show_tree_entry_fn show_tree_entry,
show_object_fn show,
@@ -69,8 +71,6 @@ static void process_tree(struct rev_info *revs,
void *cb_data)
{
struct object *obj = tree-object;
- struct tree_desc desc;
- struct name_entry entry;
struct name_path me;
enum interesting match = revs-diffopt.pathspec.nr == 0 ?
all_entries_interesting: entry_not_interesting;
@@ -96,11 +96,10 @@ static void process_tree(struct rev_info *revs,
strbuf_addch(base, '/');
}
- init_tree_desc(desc, tree-buffer, tree-size);
-
- while (tree_entry(desc, entry)) {
+ while (pv4_tree_entry(desc)) {
+ struct name_entry *entry = desc-v2.entry;
if (match != all_entries_interesting) {
- match = tree_entry_interesting(entry, base, 0,
+ match = tree_entry_interesting(entry, base, 0,
revs-diffopt.pathspec);
if (match == all_entries_not_interesting)
break;
@@ -109,22 +108,26 @@ static void process_tree(struct rev_info *revs,
}
if (show_tree_entry)
- show_tree_entry(entry, cb_data);
+ show_tree_entry(entry, cb_data);
- if (S_ISDIR(entry.mode))
+ if (S_ISDIR(entry-mode)) {
+ struct pv4_tree_desc sub_desc;
+ pv4_tree_desc_from_entry(sub_desc, desc, 0);
process_tree(revs,
-lookup_tree(entry.sha1),
+sub_desc,
+pv4_lookup_tree(desc),
show_tree_entry,
-show, me, base, entry.path,
+show, me, base, entry-path,
cb_data);
- else if (S_ISGITLINK(entry.mode))
- process_gitlink(revs, entry.sha1,
- show, me, entry.path,
+ pv4_release_tree_desc(sub_desc);
+ } else if (S_ISGITLINK(entry-mode))
+ process_gitlink(revs, entry-sha1,
+ show, me, entry-path,
cb_data);
else
process_blob(revs,
-lookup_blob(entry.sha1),
-show, me, entry.path,
+pv4_lookup_blob(desc),
+show, me, entry-path,
cb_data);
}
strbuf_setlen(base, baselen);
@@ -202,9 +205,12 @@ void traverse_commit_list(struct rev_info *revs,
continue;
}
if (obj-type == OBJ_TREE) {
- process_tree(revs, (struct tree *)obj,
+ struct pv4_tree_desc desc;
+ pv4_tree_desc_from_sha1(desc, obj-sha1, 0);
+ process_tree(revs, desc, (struct tree *)obj,
show_tree_entry, show_object,
NULL, base, name, data);
+ pv4_release_tree_desc(desc);
continue;
}
if (obj-type == OBJ_BLOB) {
--
1.8.2.83.gc99314b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] use parse_commit_or_die instead of custom message
Many calls to parse_commit detect errors and die. In some
cases, the custom error messages are more useful than what
parse_commit_or_die could produce, because they give some
context, like which ref the commit came from. Some, however,
just say invalid commit. Let's convert the latter to use
parse_commit_or_die; its message is slightly more informative,
and it makes the error more consistent throughout git.
Signed-off-by: Jeff King p...@peff.net
---
shallow.c | 3 +--
upload-pack.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/shallow.c b/shallow.c
index cdf37d6..961cf6f 100644
--- a/shallow.c
+++ b/shallow.c
@@ -90,8 +90,7 @@ struct commit_list *get_shallow_commits(struct object_array
*heads, int depth,
cur_depth = *(int *)commit-util;
}
}
- if (parse_commit(commit))
- die(invalid commit);
+ parse_commit_or_die(commit);
cur_depth++;
if (cur_depth = depth) {
commit_list_insert(commit, result);
diff --git a/upload-pack.c b/upload-pack.c
index a6c54e0..abe6636 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -649,8 +649,7 @@ static void receive_needs(void)
/* make sure the real parents are parsed */
unregister_shallow(object-sha1);
object-parsed = 0;
- if (parse_commit((struct commit *)object))
- die(invalid commit);
+ parse_commit_or_die((struct commit *)object);
parents = ((struct commit *)object)-parents;
while (parents) {
add_object_array(parents-item-object,
--
1.8.4.1.4.gf327177
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] use parse_commit_or_die instead of segfaulting
Some unchecked calls to parse_commit should obviously die on
error, because their next step is to start looking at the
parsed fields, which will cause a segfault. These are
obvious candidates for parse_commit_or_die, which will be a
strict improvement in behavior.
Signed-off-by: Jeff King p...@peff.net
---
builtin/checkout.c| 4 ++--
builtin/fast-export.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0f57397..34a2e43 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -789,7 +789,7 @@ static int switch_branches(const struct checkout_opts *opts,
new-commit = old.commit;
if (!new-commit)
die(_(You are on a branch yet to be born));
- parse_commit(new-commit);
+ parse_commit_or_die(new-commit);
}
ret = merge_working_tree(opts, old, new, writeout_error);
@@ -952,7 +952,7 @@ static int parse_branchname_arg(int argc, const char **argv,
/* not a commit */
*source_tree = parse_tree_indirect(rev);
} else {
- parse_commit(new-commit);
+ parse_commit_or_die(new-commit);
*source_tree = new-commit-tree;
}
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 78250ea..ea63052 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -287,7 +287,7 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev)
rev-diffopt.output_format = DIFF_FORMAT_CALLBACK;
- parse_commit(commit);
+ parse_commit_or_die(commit);
author = strstr(commit-buffer, \nauthor );
if (!author)
die (Could not find author in commit %s,
@@ -308,7 +308,7 @@ static void handle_commit(struct commit *commit, struct
rev_info *rev)
if (commit-parents
get_object_mark(commit-parents-item-object) != 0
!full_tree) {
- parse_commit(commit-parents-item);
+ parse_commit_or_die(commit-parents-item);
diff_tree_sha1(commit-parents-item-tree-object.sha1,
commit-tree-object.sha1, , rev-diffopt);
}
--
1.8.4.1.4.gf327177
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] parse_commit cleanups
On Sun, Oct 06, 2013 at 12:48:56AM -0400, Jeff King wrote: Instead of a segfault, let's print an error message and die a little more gracefully. [...] --- Not a huge deal, since we are terminating the program either way. There are other places in the code with a bare parse_commit that could probably use the same treatment. I didn't investigate them, but they could easily build on the parse_commit_or_die here if somebody wants to follow up. Here are some follow-up patches that go on top. The first two are noop cleanups. The third and fourth are not strictly noops, but are minor behavior changes that should be strict improvements. There are a number of unchecked parse_commit calls whose fallout is more complicated. In many cases, we want to ignore an error (keeping in mind that parse_commit will already have printed a message to stderr) and keep going, in order to allow users to to get as much done as they can in a broken repository. The fifth patch deals with one of these cases. There are 8 more unchecked calls after this series, some of which may want to die(), or may want to be left alone; I didn't investigate deeply. There are also some cases where we do not die, but perhaps should. In general, I think it makes sense to let fetch-pack and rev-list work around broken history, but probably not things like blame, which would then provide subtly wrong answers. Still, I doubt it's a big deal in practice, since corrupted repositories are relatively rare (and the message to stderr does inform the user that something is wrong). [1/5]: assume parse_commit checks commit-object.parsed [2/5]: assume parse_commit checks for NULL commit [3/5]: use parse_commit_or_die instead of segfaulting [4/5]: use parse_commit_or_die instead of custom message [5/5]: checkout: do not die when leaving broken detached HEAD -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] checkout: do not die when leaving broken detached HEAD
If we move away from a detached HEAD that has broken or
corrupted commits, we might die in two places:
1. Printing the old HEAD was... message.
2. Printing the list of orphaned commits.
In both cases, we ignore the return value of parse_commit
and feed the resulting commit to the pretty-print machinery,
which will die() upon failing to read the commit object
itself.
Since both cases are ancillary to the real operation being
performed, let's be more robust and keep going. This lets
user more easily checkout away from broken history.
Note that the call in describe_detached_head is also used to
describe the new commit we are moving to. We would want to
die in that case, but that is already handled much earlier,
when we parse the commit to get the tree to move to.
Signed-off-by: Jeff King p...@peff.net
---
builtin/checkout.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 34a2e43..1df55c0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -380,8 +380,8 @@ static void describe_detached_head(const char *msg, struct
commit *commit)
static void describe_detached_head(const char *msg, struct commit *commit)
{
struct strbuf sb = STRBUF_INIT;
- parse_commit(commit);
- pp_commit_easy(CMIT_FMT_ONELINE, commit, sb);
+ if (!parse_commit(commit))
+ pp_commit_easy(CMIT_FMT_ONELINE, commit, sb);
fprintf(stderr, %s %s... %s\n, msg,
find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV),
sb.buf);
strbuf_release(sb);
@@ -677,12 +677,12 @@ static void describe_one_orphan(struct strbuf *sb, struct
commit *commit)
static void describe_one_orphan(struct strbuf *sb, struct commit *commit)
{
- parse_commit(commit);
strbuf_addstr(sb, );
strbuf_addstr(sb,
find_unique_abbrev(commit-object.sha1, DEFAULT_ABBREV));
strbuf_addch(sb, ' ');
- pp_commit_easy(CMIT_FMT_ONELINE, commit, sb);
+ if (!parse_commit(commit))
+ pp_commit_easy(CMIT_FMT_ONELINE, commit, sb);
strbuf_addch(sb, '\n');
}
--
1.8.4.1.4.gf327177
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] assume parse_commit checks commit-object.parsed
The parse_commit function will check the parsed flag of
the object and do nothing if it is set. There is no need
for callers to check the flag themselves, and doing so only
clutters the code.
Signed-off-by: Jeff King p...@peff.net
---
builtin/blame.c | 3 +--
builtin/name-rev.c| 3 +--
builtin/show-branch.c | 3 +--
fetch-pack.c | 8 +++-
4 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 6da7233..5f1cb09 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1549,8 +1549,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
*/
origin_incref(suspect);
commit = suspect-commit;
- if (!commit-object.parsed)
- parse_commit(commit);
+ parse_commit(commit);
if (reverse ||
(!(commit-object.flags UNINTERESTING)
!(revs-max_age != -1 commit-date revs-max_age)))
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 20fcf8c..23daaa7 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -27,8 +27,7 @@ static void name_rev(struct commit *commit,
struct commit_list *parents;
int parent_number = 1;
- if (!commit-object.parsed)
- parse_commit(commit);
+ parse_commit(commit);
if (commit-date cutoff)
return;
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 001f29c..46902c3 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -227,8 +227,7 @@ static void join_revs(struct commit_list **list_p,
parents = parents-next;
if ((this_flag flags) == flags)
continue;
- if (!p-object.parsed)
- parse_commit(p);
+ parse_commit(p);
if (mark_seen(p, seen_p) !still_interesting)
extra--;
p-object.flags |= flags;
diff --git a/fetch-pack.c b/fetch-pack.c
index a0e0350..a141eb4 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -47,9 +47,8 @@ static void rev_list_push(struct commit *commit, int mark)
if (!(commit-object.flags mark)) {
commit-object.flags |= mark;
- if (!(commit-object.parsed))
- if (parse_commit(commit))
- return;
+ if (parse_commit(commit))
+ return;
prio_queue_put(rev_list, commit);
@@ -128,8 +127,7 @@ static const unsigned char *get_rev(void)
return NULL;
commit = prio_queue_get(rev_list);
- if (!commit-object.parsed)
- parse_commit(commit);
+ parse_commit(commit);
parents = commit-parents;
commit-object.flags |= POPPED;
--
1.8.4.1.4.gf327177
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] clone: do not segfault when specifying a nonexistent branch
On Sat, Oct 5, 2013 at 1:55 AM, Duy Nguyen pclo...@gmail.com wrote:
On Fri, Oct 4, 2013 at 9:20 PM, Stefan Beller
stefanbel...@googlemail.com wrote:
I think we should emit a warning additionally?
Signed-off-by: Stefan Beller stefanbel...@googlemail.com
I think it's nice to credit Robert for reporting the fault in the
commit message (something like reported-by: or noticed-by:...)
---
builtin/clone.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 0aff974..b764ad0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -688,7 +688,7 @@ static void write_refspec_config(const char*
src_ref_prefix,
if (option_mirror || !option_bare) {
if (option_single_branch !option_mirror) {
- if (option_branch) {
+ if (option_branch our_head_points_at) {
if (strstr(our_head_points_at-name,
refs/tags/))
strbuf_addf(value, +%s:%s,
our_head_points_at-name,
our_head_points_at-name);
This prevents the segfault, but what about remote.*.fetch? Should we
setup standard refspec for fetch or..?
--
Duy
This segfault only happens when cloning an empty repository and only with option
--single-branch. Or do I miss something?
If we call git clone for a non-empty repository with a non-existing branch
using [--single-branch] --branch foo then Git will abort with a message that
the branch doesn't exist in upstream.
In an empty upstream repo the branch doesn't exist, either. So why not
abort with
the same message? That would be consistent. Otherwise I'd just
override the options
--single-branch and --branch to not set.
Ralf
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BAD PATCH 0/9] v4-aware tree walker API
On Wed, 9 Oct 2013, Nguyễn Thái Ngọc Duy wrote: I know I still have a lot of holes to plug, but this was more interesting because we could see some encouraging numbers. Unfortunately the result is disappointing. Maybe I did it in a stupid way and need to restart with a totally different way. rev-list --objects on v2 takes 4 secs, v4 with current walker 11s and the new walker 16s (worst!). Clear indication that something is wrong if the intermediate step of converting to the canonical object representation is faster than your attempt at a native pv4 walker. However here's the numbers I get here after a fresh clone of git.git $ time git revlist --objects --all /dev/null real0m2.619s user0m2.577s sys 0m0.033s $ mkdir orig $ mv .git/objects/pack/pack-* orig/ $ ../../git/test-packv4 orig/pack-*.pack .git/objects/pack/pack-foo.pack Scanning objects: 100% (162785/162785), done. Writing objects: 100% (162785/162785), done. $ time git rev-list --objects --all /dev/null real0m6.210s user0m6.140s sys 0m0.027s [installing git with your latest patches applied here] $ time git rev-list --objects --all /dev/null real0m20.409s user0m20.337s sys 0m0.029s So... I get even *worse* numbers relative to v2 than you do! Now let's mitigate the deep delta chaining effect in the tree encoding: $ rm .git/objects/pack/pack-foo.* $ ../../git/test-packv4 --min-tree-copy=50 orig/pack-*.pack .git/objects/pack/pack-foo.pack Scanning objects: 100% (162785/162785), done. Writing objects: 100% (162785/162785), done. $ time git rev-list --objects --all /dev/null real0m9.451s user0m9.393s sys 0m0.036s Using --min-tree-copy=50 produces a pack which is still smaller than pack v2 but any tree copy sequence must refer to a minimum of 50 entries. This significantly reduces the CPU usage in decode_entries() by reducing the needless chaining effect I mentioned here: http://article.gmane.org/gmane.comp.version-control.git/234975 Now let's keep that pack and back out your patches. [installing git with your latest patches reverted here] $ time git rev-list --objects --all /dev/null real0m3.751s user0m3.711s sys 0m0.029s So I shoved almost 2.5 seconds of runtime here. Not the half of the original runtime, but still significant. Let's push the chaining theory even further: $ rm .git/objects/pack/pack-foo.* $ ../../git/test-packv4 --min-tree-copy=100 orig/pack-*.pack .git/objects/pack/pack-foo.pack Scanning objects: 100% (162785/162785), done. Writing objects: 100% (162785/162785), done. $ time git rev-list --objects --all /dev/null real0m3.445s user0m3.406s sys 0m0.029s With --min-tree-copy=100 the resulting pack gets somewhat larger than the v2 equivalent, but still smaller if we take into account the size of both the pack and index files. However the runtime benefit is no more significant. So, there are 2 conclusions here: 1: The current tree delta euristic is inefficient for pack v4. 2- Something must be very wrong in your latest patches as they make it close to 3 times more expensive than without them. The only bit I haven't done is avoid checking if a tree is already examined, if so do not bother with copy sequences referring to it. That should cut down the number of decode_entries but not sure how much because there's no relation between tree traversing order and how copy sequences are made. I'm sure it would help mitigate the deep delta chaining effect as well. Maybe we could make an exception and allow the tree walker to pass pv4_tree_cache* directly to decode_entries so it does not need to do the first lookup every time.. Suggestions? I'll try to have a look at your patches in more details soon. Nicolas
Make git status show if on tag
Hello Git, I was wondering if a patch that adds the tag information (something like what git log --decorate produces) to the git status would be welcome? All the best, Bartek -- 本日も小田急線をご利用いただきまして、どうもありがとうございました。 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bisect needing to be at repo top-level?
On Wed, Oct 09, 2013 at 08:27:58PM +0200, Stefan Beller wrote: At least on Linux, if you checkout a revision with foo/ directory, chdir to it and then checkout a revision with foo file to nuke your current place, I know git checkout will happily do so and you will still be in a directory that is connected nowhere. Your .. is probably pointing at the top-level, but there is no reverse, so cd ../foo may or may not work from that state, and it would lead to an interesting confusion. We may want to check the condition and forbid such a checkout. I think forbidding such a checkout is a bit hard: $ git checkout branch fatal: checkout not possible, because of said reason (dangling pwd) $ cd ../.. # go to top level or somewhere else unaffected $ git checkout branch # this will work Wouldn't it be better to navigate to the 'nearest' possible working dir on checkout? Such a workflow would emerge: $ git checkout branch # this includes the cd .. of the previous step, it just went the dir structure up, until a valid dir was found. warning: the current working directory is not part of the tree, navigating to $(PWD) The problem is that the program calling git checkout (e.g., the shell) is in the directory that is going away, and git cannot impact the working directory of its parent. So there is no way to fix it here. Our only options are to proceed and hope the user can figure it out, or to warn/forbid. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make git status show if on tag
On Wed, Oct 09, 2013 at 08:54:25PM +0200, bpuzon wrote: I was wondering if a patch that adds the tag information (something like what git log --decorate produces) to the git status would be welcome? Do you mean when you are on a branch that also happens to point at the same commit found at a tag? Or do you mean when you have detached your HEAD at a tag (e.g., by doing git checkout v1.0). In the latter case, git v1.8.3 and later will print the tag name already. If the former, I don't have a strong opinion (it is not something I would find useful, but I can see how others might). If it were not intrusive, like: # On branch master (v1.0) I don't think anybody would complain. It does involve enumerating all of the refs to calculate it, which might be noticeable. OTOH, status is not exactly a lightweight operation already, since it has to refresh the whole index. You might also consider just showing git describe output, which will show a tag if you are on it, but also show your distance to the nearest tag otherwise. It is more expensive to calculate, though. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make git status show if on tag
Hi Bartek, bpuzon wrote: I was wondering if a patch that adds the tag information (something like what git log --decorate produces) to the git status would be welcome? It would slow down git status a little. I haven't thought carefully about whether that cost is worth it --- it's hard to know without a rough patch to try it out. ;-) So the patch would be welcome, as a way to find out. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: Make git status show if on tag
Do you mean when you are on a branch that also happens to point at the same commit found at a tag? Or do you mean when you have detached your HEAD at a tag (e.g., by doing git checkout v1.0). I meant the latter. So I will just update git then. Thank you! -- 本日も小田急線をご利用いただきまして、どうもありがとうございました。 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
On Wed, Oct 09, 2013 at 02:03:24PM +0200, Paolo Giarrusso wrote: On Wed, Oct 9, 2013 at 1:26 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Paolo Giarrusso p.giarru...@gmail.com writes: Otherwise, one could change say to use printf, but that's more invasive. invasive in the sense that it impacts indirectly more callers, but are there really cases where echo is needed when calling say? Aren't there other potential bugs when arbitrary strings are passed to say, that would be fixed by using printf once and for all? (1) Changing the implementation of say to use printf %s\n would be trivial, and I think would address your concerns. Yeah, I think we should do that. I had the same thought as Matthieu when I read your initial patch; there are real portability bugs caused by using echo that would be fixed. However, that is somewhat orthogonal to the bug you are fixing. For handling this one site, I think it would be fine to just convert it to use printf, as your patch did. As you noted, the alternatives: (2) add an explicit \n to all callers (invasive error prone), or (3) make `say` parse the `-n` option and conditionally add \n to the format string or to a final argument, if -n is not specified; this would affect no current caller, but complicate the implementation of say. Doing that for just one call site has too much potential for breakage, so I'm not sure I'd do it. (I'm not even sure on what should `say` do when `-n` is not the first argument). ...are either annoying or complicated (and in particular, parsing -n means that callers need to be aware that 'say $foo' might accidentally trigger -n if $foo comes from the user). So the sanest interface is probably say_nonl or something similar. But since there would only be one caller, I don't see much point in factoring it out. Options (1), (2) and (3) are mutually alternative; my favorite is (1). Me too. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
NEW! The Best Selected Papers will be promoted for (possible) publication in WSEAS Transactions
18th International Conference on Applied Mathematics (AMATH '13) Budapest, Hungary, December 10-12, 2013 http://www.wseas.org/cms.action?id=6258 NEW! The Best Selected Papers will be promoted for (possible) publication in WSEAS Transactions (From the 16 WSEAS Journals, the 15 are indexed in SCOPUS See: http://www.wseas.org/cms.action?id=43 ) Scientific Sponsors - Co-organizers: 1) University of Bologna, ITALY 2) Technical University of Ostrava, CZECH REPUBLIC 3) University of Petrosani, ROMANIA 4) University of Kassel, GERMANY 5) Music Academy Studio Musica, ITALY 6) Melbourne Institute of Technology, AUSTRALIA Plenary Lectures: === Plenary Lecture 1: Numerical Solution to Maxwell's Equations by a Subdomain Method by Prof. Franck Assous, Ariel University Center Bar-Ilan University, ISRAEL. Plenary Lecture 2: Use of Orthogonal Arrays and Design of Experiments via Taguchi Methods in Software Testing by Assis. Prof. Ljubomir Lazic, State University of Novi Pazar, SERBIA. Plenary Lecture 3: Solution of Eddy Current Testing Problems for Media with Axisymmetric Flaws of Finite Size by Prof. Andrei Kolyshkin, Riga Technical University, LATVIA. Plenary Lecture 4: Tensional Psychological Dynamics between Determinism and Predeterminism by Prof. Alin Gilbert Sumedrea, Lucian Blaga University of Sibiu, ROMANIA. Publications = Accepted Papers are going to be published in (a) Book, Hard-Copy Proceedings (Indexed in ISI, SCOPUS, AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS etc) (b) CD-ROM Proceedings (Indexed in ISI, SCOPUS, AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS etc) Review Procees for Book and CD-ROM: Papers will undergo thorough peer review by 3 international experts. Nobody can qualify to become Reviewer for our Conference Proceedings (Book and CD-ROM) without proper academic qualifications (recent publications in SCOPUS and ISI). You will receive comments from those 3 reviewers and your paper might be * a) accepted as it is (unaltered) * b) accepted after minor revision * c) accepted after major revision * d) rejected We cannot guarantee acceptance for cases b) and c), if the reviewers are not satisfied by your response and your (minor or major) modifications to your conference paper. (c) After new peer thorough review of their extended versions in a well-known Journal (Indexed in SCOPUS, AMS, ACS, CiteSeerX, Zentralblatt, British Library, EBSCO, SWETS, EMBASE, CAS etc) Review Process for Extended Version in collaborating journals: Extended Versions of high-quality journals will undergo again thorough peer review by 3 other international experts. Nobody can qualify to become Reviewer for these journals without proper academic qualifications (recent publications in SCOPUS and ISI). You will receive comments from those other reviewers and your extended paper might be * a) accepted as it is (unaltered) * b) accepted after minor revision * c) accepted after major revision * d) rejected We cannot guarantee acceptance for cases b) and c), if the reviewers are not satisfied by your response and your (minor or major) modifications to your extended paper. (d) E-Library with free access Review Process === Papers in WSEAS Conferences and Journals are subject to peer, thorough review. The names of the Reviewers appear in the Proceedings and are, consequently, sent to all collaborating indexes. Qualified colleagues are always invited to participate in the review process. See the names of the Reviewers in our recent conferences of 2013 by conference here: http://www.wseas.org/wseas/cms.action?id=5628 Nobody can qualify to become a WSEAS Reviewer without proper academic qualifications (i.e. recent publications in SCOPUS and ISI). Reviewers whose review work is not thorough or who are not longer active are immediately removed from ourreviewers' list. See: http://www.wseas.org/wseas/cms.action?id=5321 Therefore, this list only includes active reviewers. The names of all conference reviewers are also published in the WSEAS post-conference reports: http://www.wseas.org/wseas/cms.action?id=87 Additionally, WSEAS is the only academic publisher with open access journals (public PDF on the web plus hard copy), where the authors do not pay any article processing fees. More details: http://www.wseas.org/wseas/cms.action?id=43 Indexes The Proceedings (CD-ROM and Books) will be send for indexing to: ISI (Thomson Reuters) ELSEVIER SCOPUS ACM - Association for Computing Machinery Zentralblatt MATH British Library EBSCO SWETS EMBASE CAS - American Chemical Society CiteSeerx Cabell Publishing Electronic Journals Library SAO/NASA Astrophysics Data System EI Compendex Engineering Village CSA Cambridge Scientific Abstracts DoPP GEOBASE Biobase American
Re: [PATCH 1/2] http: add option to enable 100 Continue responses
On Wed, Oct 9, 2013 at 12:30 PM, Jeff King p...@peff.net wrote: On Tue, Oct 08, 2013 at 08:48:06PM +, brian m. carlson wrote: When using GSS-Negotiate authentication with libcurl, the authentication provided will change every time, and so the probe that git uses to determine if authentication is needed is not sufficient to guarantee that data can be sent. If the data fits entirely in http.postBuffer bytes, the data can be rewound and resent if authentication fails; otherwise, a 100 Continue must be requested in this case. By default, curl will send an Expect: 100-continue if a certain amount of data is to be uploaded, but when using chunked data this is never triggered. Add an option http.continue, which defaults to enabled, to control whether this header is sent. The addition of an option is necessary because some proxies break badly if sent this header. [+cc spearce] I'd be more comfortable defaulting this to on if I understood more about the original problem that led to 959dfcf and 206b099. It sounds like enabling this all the time will cause annoying stalls in the protocol, unless the number of non-crappy proxy implementations has gotten smaller over the past few years. It actually hasn't, not significantly. 206b099 was written because the Google web servers for android.googlesource.com and code.google.com do not support 100-continue semantics. This caused the client to stall a full 1 second before each POST exchange. If ancestor negotiation required O(128) have lines to be advertised I think this was 2 or 4 POSTs, resulting in 2-4 second stalls above the other latency of the network and the server. The advice I received from the Google web server developers was to not rely on 100-continue, because a large number of corporate proxy servers do not support it correctly. HTTP 100-continue is just pretty broken in the wild. If Expect: 100-continue is required for GSS-Negotiate to work then Git should only enable the option if the server is demanding GSS-Negotiate for authentication. Per 206b099 the default should still be off for anonymous and HTTP basic, digest, and SSL certificate authentication. diff --git a/remote-curl.c b/remote-curl.c index b5ebe01..3b5e160 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -470,7 +470,12 @@ static int post_rpc(struct rpc_state *rpc) headers = curl_slist_append(headers, rpc-hdr_content_type); headers = curl_slist_append(headers, rpc-hdr_accept); - headers = curl_slist_append(headers, Expect:); + + /* Force it either on or off, since curl will try to decide based on how + * much data is to be uploaded and we want consistency. + */ + headers = curl_slist_append(headers, http_use_100_continue ? + Expect: 100-continue : Expect:); Is there any point in sending the Expect: header in cases where curl would not send it, though? It seems like we should assume curl does the right thing most of the time, and have our option only be to override curl in the negative direction. Adding a header of Expect: causes curl to disable the header and never use it. Always supplying the header with no value prevents libcurl from using 100-continue on its own, which is what I fixed in 959dfcf. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] http: add option to enable 100 Continue responses
On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote:
I'd be more comfortable defaulting this to on if I understood more
about the original problem that led to 959dfcf and 206b099. It sounds
like enabling this all the time will cause annoying stalls in the
protocol, unless the number of non-crappy proxy implementations has
gotten smaller over the past few years.
It actually hasn't, not significantly.
Thanks for update; my pessimistic side assumed this was the case, but it
is always good to check. :)
206b099 was written because the Google web servers for
android.googlesource.com and code.google.com do not support
100-continue semantics. This caused the client to stall a full 1
second before each POST exchange. If ancestor negotiation required
O(128) have lines to be advertised I think this was 2 or 4 POSTs,
resulting in 2-4 second stalls above the other latency of the network
and the server.
Yuck.
If Expect: 100-continue is required for GSS-Negotiate to work then
Git should only enable the option if the server is demanding
GSS-Negotiate for authentication. Per 206b099 the default should still
be off for anonymous and HTTP basic, digest, and SSL certificate
authentication.
Part of the problem is that curl is the one handling the negotiation.
When we get a 401, I think we can ask curl_easy_getinfo to tell us which
auth methods are available (via CURLINFO_HTTPAUTH_AVAIL). But I don't
know how we decide that GSS is what's going to be used. I guess if it is
the only option, and there is no basic-auth offered?
And then in that case turn on Expect (or more accurately, stop
disabling it).
I don't have a GSS-enabled server to test on. Brian, can you try the
patch at the end of this message on your non-working server and see what
it outputs?
+ headers = curl_slist_append(headers, http_use_100_continue ?
+ Expect: 100-continue : Expect:);
Is there any point in sending the Expect: header in cases where curl
would not send it, though? It seems like we should assume curl does the
right thing most of the time, and have our option only be to override
curl in the negative direction.
Adding a header of Expect: causes curl to disable the header and
never use it. Always supplying the header with no value prevents
libcurl from using 100-continue on its own, which is what I fixed in
959dfcf.
Right. What I meant is that we do not want to unconditionally send the
Expect: 100-continue in the other case. IOW, we would just want:
if (!http_use_100_continue)
headers = curl_slist_append(headers, Expect:);
-Peff
-- 8 --
diff --git a/http.c b/http.c
index f3e1439..e7257d7 100644
--- a/http.c
+++ b/http.c
@@ -889,6 +889,12 @@ static int http_request(const char *url, struct strbuf
*type,
if (start_active_slot(slot)) {
run_active_slot(slot);
ret = handle_curl_result(results);
+ if (ret == HTTP_REAUTH) {
+ long auth_avail;
+ curl_easy_getinfo(slot-curl, CURLINFO_HTTPAUTH_AVAIL,
+ auth_avail);
+ fprintf(stderr, offered auth: %ld\n, auth_avail);
+ }
} else {
snprintf(curl_errorstr, sizeof(curl_errorstr),
failed to start HTTP request);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Documentation: --options in man-pages synopsys
From: Loyall, David david.loy...@nebraska.gov Sent: Tuesday, October 08, 2013 3:03 PM As a unix user I'd expect the SYNOPSIS section at the top of the man page to include all options that the command accepts. Mutually exclusive options are expected to be in the form [-q | --progress | --all-progress], such is already done. I believe that you'd be safe in following http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html unless the git-* family of commands and documentation deviate from it in a way that I am not aware of. For an example of a command with a long list of options try `git rev-parse --help`. SYNOPSIS git rev-parse [ --option ] args. Philip Hope this helps, --Dave -Original Message- From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Dmitry Ivankov Sent: Tuesday, October 08, 2013 7:07 AM To: Git List Subject: [RFC] Documentation: --options in man-pages synopsys Hi, I've noticed that man git-pack-objects describes cmdline as following SYNOPSYS 'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied] [-- no-reuse-delta] [--delta-base-offset] [--non-empty] [--local] [--incremental] [--window=n] [--depth=n] [--revs [--unpacked | --all]] [--stdout | base- name] [--keep-true-parents] object-list while OPTIONS sections has even more options, --no-reuse-objects for instance. Should it be dealt with and how? - add smth like ... at the tail of options in synopsys to indicate that there are more options - add all the [--options] to synopsys - drop all the [--options] as they all are optional - pick only the most common/important ones like -q --progress, per command or per command classes (hard to maintain and/or verify?) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error in creating git over http
On Tue, Oct 08, 2013 at 03:05:22PM +, Shlomit Afgin wrote: I do the following on the git server: cd /var/www/html/ git clone --bare /path/to/dir/ gitproject.git cd gitproject.git/ mv hooks/post-update.sample hooks/post-update chmod a+x hooks/post-update OK, so on the next push, the info/refs file should be updated. Note that this file is only necessary if you want to run the dumb http protocol (i.e., the less efficient one that does not require git on the server). You didn't say whether you are trying to set up a dumb or smart git-http server. These days you almost certainly want to set up a smart one, and you do not need to care about info/refs or running update-server-info. on the local machine run: git clone http://server.name/gitproject.git I got the error: Initialized empty Git repository in /local/path/gitproject/.git/ fatal: http://server.name/gitproject.git/info/refs not found: did you run git update-server-info on the server? I saw that the file does not exist, it seem that the file post-update is not execute. Yes, if you didn't push yet, then it won't have been created. I run it on the git server: git update-server-info Now the info/ref is created. OK, good. On local machine I run again : git clone http://server.name/gitproject.git Now I get the error: Initialized empty Git repository in /local/path/gitproject/.git/ error: The requested URL returned error: 403 (curl_result = 22, http_code = 403, sha1 = 9d83b83df9fbc75ecd754264f95793fca93ccf93) error: Unable to find 9d83b83df9fbc75ecd754264f95793fca93ccf93 under http://server.name/gitproject.git Cannot obtain needed object 9d83b83df9fbc75ecd754264f95793fca93ccf93 403 is an HTTP Forbidden. Have you configured your web server to allow access to the project? Have you marked the repository as git-daemon-export-ok, as described in git help http-backend (or set GIT_HTTP_EXPORT_ALL in the environment)? Is there anything interesting in the webserver's error logs? If it is still not working after checking those things, can you show us how you have configured your webserver (presumably apache?). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: A workflow for local patch maintenance
On Tue, Oct 08, 2013 at 07:12:22PM +0100, Tony Finch wrote: We often need to patch the software that we run in order to fix bugs quickly rather than wait for an official release, or to add functionality that we need. In many cases we have to maintain a locally-developed patch for a significant length of time, across multiple upstream releases, either because it is not yet ready for incorporation into a stable upstream version, or because it is too specific to our setup so will not be suitable for passing upstream without significant extra work. Do you need to keep the modifications you make on top of upstream as a nice, clean series of rebased patches? If not, then you can avoid the repeated rebasing, and just use a more traditional topic-branch workflow. Treat modifications from upstream as just another topic. For example, start with some version (let's say 1.0) of the upstream software as your master branch. If it's kept in git, build on upstream's git history. If all you get are tarballs, create an upstream branch with v1.0, and fork master from it. Build on master as you would if it were your own. Fork topic branches, develop the topics, test them, and then merge them back to master when they're ready (or do development straight on master, or whatever workflow you're accustomed to). When v1.1 of the upstream software comes out, create a merge-upstream topic branch from the tip of your master. If upstream is in git, just git merge v1.1 from upstream. If not, then checkout your pristine upstream branch (which should still be sitting at the v1.0 commit), and build a v1.1 commit on top of it. And then git merge upstream to pick up the new changes. Test your merge-upstream topic in isolation, and when you think it's ready merge it into master and deploy. The most difficult part is the merge of upstream into the topic branch. But git's 3-way merge tends to do a pretty good job (e.g., if you contributed your patches upstream, then there should be no conflict). You can also break up the work by keeping the merge topic running for a long time, and merging as often as possible from upstream. That breaks the conflict resolution into smaller chunks, and lets you do it closer to when the conflicting patches were actually made, when they are hopefully closer in your mind. And you don't have to worry about having a broken intermediate result, because you're not deploying it; you're just keeping the topic up to date until you're ready to test it. You can also try git-imerge, which can make big merges a little more manageable (though it can also make them harder sometimes...): https://github.com/mhagger/git-imerge The history for such a repository might look like: o--o--B--o--o--C -- upstream branch / \\ o--o---o--o--o--o--D -- upstream-merge branch / //\ A--o---E--o--o--F--o---G -- master branch \/ \ / o--o oo -- topic branches where: - A is the v1.0 commit you start at - B and C are milestones where you merged upstream into your upstream-merge topic branch. These could be releases (like v1.1), or they could just be random spots where you felt like merging to keep things up to date. It depends how you want to break up the conflict resolution - D is a state of the upstream-merge branch that you test to make sure the merge happened OK - E and F are merges of regular topic branches (i.e., the patches you are working on locally). Note that we also merge those up to the upstream-merge branch, so that we can resolve early any conflicts between what's happening on master and what's happening upstream. - G is the merge of D into the master branch, after we have decided it's good to deploy This all assumes that master is your known-good state that you deploy or ship. If you prefer to have a deploy or maint branch for hotfixes, you can do that too. Hope that helps, -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] http: add option to enable 100 Continue responses
On Wed, Oct 09, 2013 at 05:37:42PM -0400, Jeff King wrote: On Wed, Oct 09, 2013 at 02:19:36PM -0700, Shawn O. Pearce wrote: 206b099 was written because the Google web servers for android.googlesource.com and code.google.com do not support 100-continue semantics. This caused the client to stall a full 1 second before each POST exchange. If ancestor negotiation required O(128) have lines to be advertised I think this was 2 or 4 POSTs, resulting in 2-4 second stalls above the other latency of the network and the server. Yuck. Shame on Google. Of all people, they should be able to implement HTTP 1.1 properly. Part of the problem is that curl is the one handling the negotiation. When we get a 401, I think we can ask curl_easy_getinfo to tell us which auth methods are available (via CURLINFO_HTTPAUTH_AVAIL). But I don't know how we decide that GSS is what's going to be used. I guess if it is the only option, and there is no basic-auth offered? And then in that case turn on Expect (or more accurately, stop disabling it). I don't have a GSS-enabled server to test on. Brian, can you try the patch at the end of this message on your non-working server and see what it outputs? It doesn't trigger. My server only requires authentication for the actual push operation, and it looks like your code doesn't trigger in that case. + headers = curl_slist_append(headers, http_use_100_continue ? + Expect: 100-continue : Expect:); Is there any point in sending the Expect: header in cases where curl would not send it, though? It seems like we should assume curl does the right thing most of the time, and have our option only be to override curl in the negative direction. I think curl expects that data in the POSTFIELDS option in order to trigger. I wasn't able to get it to trigger on its own. Adding a header of Expect: causes curl to disable the header and never use it. Always supplying the header with no value prevents libcurl from using 100-continue on its own, which is what I fixed in 959dfcf. Right. What I meant is that we do not want to unconditionally send the Expect: 100-continue in the other case. IOW, we would just want: if (!http_use_100_continue) headers = curl_slist_append(headers, Expect:); I tried that. It doesn't work, since it never sends the Expect: 100-continue. I made libcurl dump all the headers it sends, and it doesn't send them in that case. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 1/2] http: add option to enable 100 Continue responses
On Thu, Oct 10, 2013 at 01:35:47AM +, brian m. carlson wrote: I don't have a GSS-enabled server to test on. Brian, can you try the patch at the end of this message on your non-working server and see what it outputs? It doesn't trigger. My server only requires authentication for the actual push operation, and it looks like your code doesn't trigger in that case. Can you describe the sequence of requests, then? Do you not have to auth for the info/refs request, and then have to auth later for the git-receive-pack request? Does the auth trigger for the probe_rpc call? Can you show us GIT_CURL_VERBOSE=1 output for a session that fails (do note that it will dump your auth headers, so you may want to redact them before sharing)? Is there any point in sending the Expect: header in cases where curl would not send it, though? It seems like we should assume curl does the right thing most of the time, and have our option only be to override curl in the negative direction. I think curl expects that data in the POSTFIELDS option in order to trigger. I wasn't able to get it to trigger on its own. Was your POST perhaps not big enough? My impression is that it only sends it if the POST is too large to rewind (which seems like a good thing in general), and I thought earlier in the thread that command-line curl was sending one. I'm not sure what would be different for us here, once our manual Expect suppression is turned off. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Documentation: --options in man-pages synopsys
On Wed, Oct 09, 2013 at 11:35:19PM +0100, Philip Oakley wrote:
As a unix user I'd expect the SYNOPSIS section at the top of the
man page to include all options that the command accepts. Mutually
exclusive options are expected to be in the form [-q | --progress |
--all-progress], such is already done.
I believe that you'd be safe in following
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap12.html
unless the git-* family of commands and documentation deviate from
it in a way that I am not aware of.
For an example of a command with a long list of options try `git
rev-parse --help`.
SYNOPSIS
git rev-parse [ --option ] args.
The current documentation is quite inconsistent between the two forms.
Personally, I favor the shorter form as I think the longer ones end up
quite unwieldy (see git help rev-list for example). But I do think the
synopsis should show the major modes of the command. You can see
examples of both in the GNU pages for cat and sort:
$ man cat | sed -n '/SYNOPSIS/,${p; /^$/q}'
SYNOPSIS
cat [OPTION]... [FILE]...
$ man sort | sed -n '/SYNOPSIS/,${p; /^$/q}'
SYNOPSIS
sort [OPTION]... [FILE]...
sort [OPTION]... --files0-from=F
A similar example in git would be git help branch, whose synopsis
should include listing mode, creation mode, deleting mode, etc.
However, I am not sure everyone on the list agrees with me on this. Last
time it came up (which was probably several years now) there was some
discussion but not enough consensus for somebody to actually go through
and standardize it.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Error in creating git over http
Thanks for your answer. I did not know about dumb and smart I will read on those in the future. I found in google that the problem of Error: 403 can be solved. I run on the git server, in the directory that had the repository: 'git gc' which do git cleanup and the problem solved. Thanks you very much. On 10/10/13 3:06 AM, Jeff King p...@peff.net wrote: On Tue, Oct 08, 2013 at 03:05:22PM +, Shlomit Afgin wrote: I do the following on the git server: cd /var/www/html/ git clone --bare /path/to/dir/ gitproject.git cd gitproject.git/ mv hooks/post-update.sample hooks/post-update chmod a+x hooks/post-update OK, so on the next push, the info/refs file should be updated. Note that this file is only necessary if you want to run the dumb http protocol (i.e., the less efficient one that does not require git on the server). You didn't say whether you are trying to set up a dumb or smart git-http server. These days you almost certainly want to set up a smart one, and you do not need to care about info/refs or running update-server-info. on the local machine run: git clone http://server.name/gitproject.git I got the error: Initialized empty Git repository in /local/path/gitproject/.git/ fatal: http://server.name/gitproject.git/info/refs not found: did you run git update-server-info on the server? I saw that the file does not exist, it seem that the file post-update is not execute. Yes, if you didn't push yet, then it won't have been created. I run it on the git server: git update-server-info Now the info/ref is created. OK, good. On local machine I run again : git clone http://server.name/gitproject.git Now I get the error: Initialized empty Git repository in /local/path/gitproject/.git/ error: The requested URL returned error: 403 (curl_result = 22, http_code = 403, sha1 = 9d83b83df9fbc75ecd754264f95793fca93ccf93) error: Unable to find 9d83b83df9fbc75ecd754264f95793fca93ccf93 under http://server.name/gitproject.git Cannot obtain needed object 9d83b83df9fbc75ecd754264f95793fca93ccf93 403 is an HTTP Forbidden. Have you configured your web server to allow access to the project? Have you marked the repository as git-daemon-export-ok, as described in git help http-backend (or set GIT_HTTP_EXPORT_ALL in the environment)? Is there anything interesting in the webserver's error logs? If it is still not working after checking those things, can you show us how you have configured your webserver (presumably apache?). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
