Re: [PATCH 1/2] reset: handle submodule with trailing slash
Am 10.09.2013 21:13, schrieb John Keeping: When using tab-completion, a directory path will often end with a trailing slash which currently confuses git rm when dealing with submodules. Now that we have parse_pathspec we can easily handle this by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag. Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/reset.c| 5 + t/t7400-submodule-basic.sh | 6 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 5e4c551..9efac0f 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec, } } *rev_ret = rev; + + if (read_cache() 0) + die(_(index file corrupt)); When the index is now read here, I would have expected hunk in this patch that removes a read_cache() invocation. + parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL | +PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP | (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0), prefix, argv); } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 4192fe0..c268d3c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -481,7 +481,7 @@ test_expect_success 'do not add files from a submodule' ' ' -test_expect_success 'gracefully add submodule with a trailing slash' ' +test_expect_success 'gracefully add/reset submodule with a trailing slash' ' git reset --hard git commit -m commit subproject init @@ -495,7 +495,9 @@ test_expect_success 'gracefully add submodule with a trailing slash' ' git add init/ test_must_fail git diff --exit-code --cached init test $commit = $(git ls-files --stage | - sed -n s/^16 \([^ ]*\).*/\1/p) + sed -n s/^16 \([^ ]*\).*/\1/p) + git reset init/ + git diff --exit-code --cached init ' -- 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 00/21] np/pack-v4 updates
This contains fixups for some of my patches, some of Nico's, adds v4 support to unpack-objects because the test suite needs it. With these, when force generating pack v4 unconditionally, the remaining failed tests are: - t5300-pack-object: ofs-delta tests fail (not surprising). core.packsizelimit also fails. Kinda expected, but not my top priority. - t5302-pack-index: mainly to test .idx v2, expected - t5303-pack-corruption-resilience: if I force generating .idx v2 with .pack v4, I could get to 1/3 of it. Need a deeper look. So v4 code is in pretty good shape in terms of correctness and near function complete. Brave souls should try it out. Nguyễn Thái Ngọc Duy (21): fixup! pack-objects: prepare SHA-1 table in v4 fixup! pack-objects: support writing pack v4 fixup! pack v4: support end-of-pack indicator in index-pack and pack-objects fixup! index-pack: parse v4 header and dictionaries fixup! index-pack: record all delta bases in v4 (tree and ref-delta) pack v4: lift dict size check in load_dict() pack v4: move pv4 objhdr parsing code to packv4-parse.c pack-objects: respect compression level in v4 pack-objects: recognize v4 as pack source pack v4: add a note that streaming does not support OBJ_PV4_* unpack-objects: report missing object name unpack-objects: recognize end-of-pack in v4 thin pack unpack-objects: read v4 dictionaries unpack-objects: decode v4 object header unpack-objects: decode v4 ref-delta unpack-objects: decode v4 commits unpack-objects: allow to save processed bytes to a buffer unpack-objects: decode v4 trees index-pack, pack-objects: allow creating .idx v2 with .pack v4 show-index: acknowledge that it does not read .idx v3 t1050, t5500: replace the use of show-index|wc -l with verify-pack builtin/index-pack.c | 19 ++- builtin/pack-objects.c | 60 +-- builtin/unpack-objects.c | 395 --- packv4-create.c | 17 +- packv4-create.h | 6 +- packv4-parse.c | 16 +- packv4-parse.h | 7 + sha1_file.c | 9 +- show-index.c | 4 +- streaming.c | 2 +- t/t1050-large.sh | 9 +- t/t5500-fetch-pack.sh| 4 +- test-packv4.c| 9 +- 13 files changed, 480 insertions(+), 77 deletions(-) -- 1.8.2.82.gc24b958 -- 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 06/21] pack v4: lift dict size check in load_dict()
A pack with no trees (or an empty pack) could have zero-sized name
dictionary.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 4
1 file changed, 4 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index f96acc1..80ad6fc 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -87,10 +87,6 @@ static struct packv4_dict *load_dict(struct packed_git *p,
off_t *offset)
src = use_pack(p, w_curs, curpos, avail);
cp = src;
dict_size = decode_varint(cp);
- if (dict_size 3) {
- error(bad dict size);
- return NULL;
- }
curpos += cp - src;
data = xmallocz(dict_size);
--
1.8.2.82.gc24b958
--
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 01/21] fixup! pack-objects: prepare SHA-1 table in v4
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
remove debugging code
builtin/pack-objects.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1efb728..945b817 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -816,7 +816,6 @@ static void prepare_sha1_table(uint32_t start, struct
object_entry **write_order
struct object_entry *e = write_order[i];
if (e-idx.offset 0) {
v4.all_objs[v4.all_objs_nr++] = e-idx;
- fprintf(stderr, %s in\n, sha1_to_hex(e-idx.sha1));
e-idx.offset = 0;
}
}
--
1.8.2.82.gc24b958
--
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 07/21] pack v4: move pv4 objhdr parsing code to packv4-parse.c
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
packv4-parse.c | 12
packv4-parse.h | 5 +
sha1_file.c| 9 ++---
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/packv4-parse.c b/packv4-parse.c
index 80ad6fc..7a43635 100644
--- a/packv4-parse.c
+++ b/packv4-parse.c
@@ -570,3 +570,15 @@ void *pv4_get_tree(struct packed_git *p, struct
pack_window **w_curs,
}
return dst;
}
+
+unsigned long pv4_unpack_object_header_buffer(const unsigned char *base,
+ unsigned long len,
+ enum object_type *type,
+ unsigned long *sizep)
+{
+ const unsigned char *cp = base;
+ uintmax_t val = decode_varint(cp);
+ *type = val 0xf;
+ *sizep = val 4;
+ return cp - base;
+}
diff --git a/packv4-parse.h b/packv4-parse.h
index e6719f6..52f52f5 100644
--- a/packv4-parse.h
+++ b/packv4-parse.h
@@ -10,6 +10,11 @@ struct packv4_dict {
struct packv4_dict *pv4_create_dict(const unsigned char *data, int dict_size);
void pv4_free_dict(struct packv4_dict *dict);
+unsigned long pv4_unpack_object_header_buffer(const unsigned char *base,
+ unsigned long len,
+ enum object_type *type,
+ unsigned long *sizep);
+
void *pv4_get_commit(struct packed_git *p, struct pack_window **w_curs,
off_t offset, unsigned long size);
void *pv4_get_tree(struct packed_git *p, struct pack_window **w_curs,
diff --git a/sha1_file.c b/sha1_file.c
index 1528e28..038e22e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1736,13 +1736,8 @@ int unpack_object_header(struct packed_git *p,
base = use_pack(p, w_curs, *curpos, left);
if (p-version 4) {
used = unpack_object_header_buffer(base, left, type, sizep);
- } else {
- const unsigned char *cp = base;
- uintmax_t val = decode_varint(cp);
- used = cp - base;
- type = val 0xf;
- *sizep = val 4;
- }
+ } else
+ used = pv4_unpack_object_header_buffer(base, left, type,
sizep);
if (!used) {
type = OBJ_BAD;
} else
--
1.8.2.82.gc24b958
--
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 03/21] fixup! pack v4: support end-of-pack indicator in index-pack and pack-objects
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
nr_objects contains a lot more than the number of objects to be
written.
builtin/pack-objects.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b60b1a0..39d1e08 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -872,7 +872,7 @@ static void write_pack_file(void)
* Pack v4 thin pack is terminated by a type
* 0, size 0 in variable length encoding
*/
- if (pack_version == 4 nr_written nr_objects)
+ if (pack_version == 4 nr_written v4.all_objs_nr)
sha1write(f, type_zero, 1);
sha1close(f, sha1, CSUM_CLOSE);
} else if (nr_written == nr_remaining) {
--
1.8.2.82.gc24b958
--
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 02/21] fixup! pack-objects: support writing pack v4
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
by setting usable_delta to zero, I disable tree delta in
pack-objects. Some test cases spotted this.
builtin/pack-objects.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 945b817..b60b1a0 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -256,7 +256,12 @@ static unsigned long write_no_reuse_object(struct sha1file
*f, struct object_ent
struct git_istream *st = NULL;
char *result = OK;
- if (!usable_delta) {
+ if (!usable_delta ||
+ /*
+* Force loading canonical tree. In future we may want to
+* read v4 trees directly instead.
+*/
+ (pack_version == 4 entry-type == OBJ_TREE)) {
if (entry-type == OBJ_BLOB
entry-size big_file_threshold
(st = open_istream(entry-idx.sha1, type, size, NULL)) !=
NULL)
@@ -518,9 +523,6 @@ static unsigned long write_object(struct sha1file *f,
else
usable_delta = 0; /* base could end up in another pack */
- if (pack_version == 4 entry-type == OBJ_TREE)
- usable_delta = 0;
-
if (!reuse_object)
to_reuse = 0; /* explicit */
else if (!entry-in_pack)
--
1.8.2.82.gc24b958
--
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 04/21] fixup! index-pack: parse v4 header and dictionaries
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
empty pack case
builtin/index-pack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 8a6e2a3..89bc708 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1474,7 +1474,8 @@ static void parse_dictionaries(void)
return;
sha1_table = xmalloc(20 * nr_objects_final);
- hashcpy(sha1_table, fill_and_use(20));
+ if (nr_objects_final)
+ hashcpy(sha1_table, fill_and_use(20));
for (i = 1; i nr_objects_final; i++) {
unsigned char *p = sha1_table + i * 20;
hashcpy(p, fill_and_use(20));
--
1.8.2.82.gc24b958
--
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 10/21] pack v4: add a note that streaming does not support OBJ_PV4_*
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
streaming.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/streaming.c b/streaming.c
index debe904..c7edebb 100644
--- a/streaming.c
+++ b/streaming.c
@@ -437,7 +437,7 @@ static open_method_decl(pack_non_delta)
unuse_pack(window);
switch (in_pack_type) {
default:
- return -1; /* we do not do deltas for now */
+ return -1; /* we do not do deltas nor pv4 types for now */
case OBJ_COMMIT:
case OBJ_TREE:
case OBJ_BLOB:
--
1.8.2.82.gc24b958
--
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 14/21] unpack-objects: decode v4 object header
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 1a3c30e..a906a98 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -448,32 +448,38 @@ static void unpack_delta_entry(enum object_type type,
unsigned long delta_size,
free(base);
}
-static int unpack_one(unsigned nr)
+static void read_typesize_v2(enum object_type *type, unsigned long *size)
{
+ unsigned char c = *(char*)fill_and_use(1);
unsigned shift;
- unsigned char *pack;
- unsigned long size, c;
+
+ *type = (c 4) 7;
+ *size = (c 15);
+ shift = 4;
+ while (c 128) {
+ c = *(char*)fill_and_use(1);
+ *size += (c 0x7f) shift;
+ shift += 7;
+ }
+}
+
+static int unpack_one(unsigned nr)
+{
+ unsigned long size;
enum object_type type;
obj_list[nr].offset = consumed_bytes;
- pack = fill(1);
if (packv4 *(char*)fill(1) == 0) {
use(1);
return -1;
}
- c = *pack;
- use(1);
- type = (c 4) 7;
- size = (c 15);
- shift = 4;
- while (c 0x80) {
- pack = fill(1);
- c = *pack;
- use(1);
- size += (c 0x7f) shift;
- shift += 7;
- }
+ if (packv4) {
+ uintmax_t val = read_varint();
+ type = val 15;
+ size = val 4;
+ } else
+ read_typesize_v2(type, size);
switch (type) {
case OBJ_COMMIT:
--
1.8.2.82.gc24b958
--
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 13/21] unpack-objects: read v4 dictionaries
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 50
1 file changed, 50 insertions(+)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index c9eb31d..1a3c30e 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -10,6 +10,7 @@
#include tree-walk.h
#include progress.h
#include decorate.h
+#include packv4-parse.h
#include fsck.h
static int dry_run, quiet, recover, has_errors, strict;
@@ -20,7 +21,10 @@ static unsigned char buffer[4096];
static unsigned int offset, len;
static off_t consumed_bytes;
static git_SHA_CTX ctx;
+
static int packv4;
+static unsigned char *sha1_table;
+static struct packv4_dict *name_dict, *path_dict;
/*
* When running under --strict mode, objects whose reachability are
@@ -89,6 +93,28 @@ static void use(int bytes)
consumed_bytes += bytes;
}
+static inline void *fill_and_use(int bytes)
+{
+ void *p = fill(bytes);
+ use(bytes);
+ return p;
+}
+
+static uintmax_t read_varint(void)
+{
+ unsigned char c = *(char*)fill_and_use(1);
+ uintmax_t val = c 127;
+ while (c 128) {
+ val += 1;
+ if (!val || MSB(val, 7))
+ die(offset overflow in read_varint at %lu,
+ (unsigned long)consumed_bytes);
+ c = *(char*)fill_and_use(1);
+ val = (val 7) + (c 127);
+ }
+ return val;
+}
+
static void *get_data(unsigned long size)
{
git_zstream stream;
@@ -470,6 +496,20 @@ static int unpack_one(unsigned nr)
return 0;
}
+static struct packv4_dict *read_dict(void)
+{
+ unsigned long size;
+ unsigned char *data;
+ struct packv4_dict *dict;
+
+ size = read_varint();
+ data = get_data(size);
+ dict = pv4_create_dict(data, size);
+ if (!dict)
+ die(unable to parse dictionary);
+ return dict;
+}
+
static void unpack_all(void)
{
int i;
@@ -486,6 +526,16 @@ static void unpack_all(void)
packv4 = ntohl(hdr-hdr_version) == 4;
use(sizeof(struct pack_header));
+ if (packv4) {
+ sha1_table = xmalloc(20 * nr_objects);
+ for (i = 0; i nr_objects; i++) {
+ unsigned char *p = sha1_table + i * 20;
+ hashcpy(p, fill_and_use(20));
+ }
+ name_dict = read_dict();
+ path_dict = read_dict();
+ }
+
if (!quiet)
progress = start_progress(Unpacking objects, nr_objects);
obj_list = xcalloc(nr_objects, sizeof(*obj_list));
--
1.8.2.82.gc24b958
--
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 15/21] unpack-objects: decode v4 ref-delta
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 25 ++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index a906a98..f8442f4 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -23,6 +23,7 @@ static off_t consumed_bytes;
static git_SHA_CTX ctx;
static int packv4;
+static unsigned nr_objects;
static unsigned char *sha1_table;
static struct packv4_dict *name_dict, *path_dict;
@@ -115,6 +116,21 @@ static uintmax_t read_varint(void)
return val;
}
+static const unsigned char *read_sha1ref(void)
+{
+ unsigned int index = read_varint();
+ if (!index) {
+ static unsigned char sha1[20];
+ hashcpy(sha1, fill_and_use(20));
+ return sha1;
+ }
+ index--;
+ if (index = nr_objects)
+ die(bad index in read_sha1ref at %lu,
+ (unsigned long)consumed_bytes);
+ return sha1_table + index * 20;
+}
+
static void *get_data(unsigned long size)
{
git_zstream stream;
@@ -185,7 +201,6 @@ struct obj_info {
#define FLAG_WRITTEN (1u21)
static struct obj_info *obj_list;
-static unsigned nr_objects;
/*
* Called only from check_object() after it verified this object
@@ -361,8 +376,12 @@ static void unpack_delta_entry(enum object_type type,
unsigned long delta_size,
unsigned char base_sha1[20];
if (type == OBJ_REF_DELTA) {
- hashcpy(base_sha1, fill(20));
- use(20);
+ if (packv4)
+ hashcpy(base_sha1, read_sha1ref());
+ else {
+ hashcpy(base_sha1, fill(20));
+ use(20);
+ }
delta_data = get_data(delta_size);
if (dry_run || !delta_data) {
free(delta_data);
--
1.8.2.82.gc24b958
--
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 11/21] unpack-objects: report missing object name
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- builtin/unpack-objects.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 2217d7b..6d0a65c 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -193,7 +193,7 @@ static int check_object(struct object *obj, int type, void *data) unsigned long size; int type = sha1_object_info(obj-sha1, size); if (type != obj-type || type = 0) - die(object of unexpected type); + die(object %s of unexpected type, sha1_to_hex(obj-sha1)); obj-flags |= FLAG_WRITTEN; return 0; } -- 1.8.2.82.gc24b958 -- 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 12/21] unpack-objects: recognize end-of-pack in v4 thin pack
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6d0a65c..c9eb31d 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -20,6 +20,7 @@ static unsigned char buffer[4096];
static unsigned int offset, len;
static off_t consumed_bytes;
static git_SHA_CTX ctx;
+static int packv4;
/*
* When running under --strict mode, objects whose reachability are
@@ -421,7 +422,7 @@ static void unpack_delta_entry(enum object_type type,
unsigned long delta_size,
free(base);
}
-static void unpack_one(unsigned nr)
+static int unpack_one(unsigned nr)
{
unsigned shift;
unsigned char *pack;
@@ -431,6 +432,10 @@ static void unpack_one(unsigned nr)
obj_list[nr].offset = consumed_bytes;
pack = fill(1);
+ if (packv4 *(char*)fill(1) == 0) {
+ use(1);
+ return -1;
+ }
c = *pack;
use(1);
type = (c 4) 7;
@@ -450,18 +455,19 @@ static void unpack_one(unsigned nr)
case OBJ_BLOB:
case OBJ_TAG:
unpack_non_delta_entry(type, size, nr);
- return;
+ break;
case OBJ_REF_DELTA:
case OBJ_OFS_DELTA:
unpack_delta_entry(type, size, nr);
- return;
+ break;
default:
error(bad object type %d, type);
has_errors = 1;
if (recover)
- return;
+ break;
exit(1);
}
+ return 0;
}
static void unpack_all(void)
@@ -477,13 +483,15 @@ static void unpack_all(void)
if (!pack_version_ok(hdr-hdr_version))
die(unknown pack file version %PRIu32,
ntohl(hdr-hdr_version));
+ packv4 = ntohl(hdr-hdr_version) == 4;
use(sizeof(struct pack_header));
if (!quiet)
progress = start_progress(Unpacking objects, nr_objects);
obj_list = xcalloc(nr_objects, sizeof(*obj_list));
for (i = 0; i nr_objects; i++) {
- unpack_one(i);
+ if (unpack_one(i))
+ break;
display_progress(progress, i + 1);
}
stop_progress(progress);
--
1.8.2.82.gc24b958
--
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 18/21] unpack-objects: decode v4 trees
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 191 ++-
1 file changed, 189 insertions(+), 2 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 044a087..9fd5640 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -12,6 +12,7 @@
#include decorate.h
#include packv4-parse.h
#include fsck.h
+#include varint.h
static int dry_run, quiet, recover, has_errors, strict;
static const char unpack_usage[] = git unpack-objects [-n] [-q] [-r]
[--strict] pack-file;
@@ -148,6 +149,27 @@ static const unsigned char *read_sha1ref(void)
return sha1_table + index * 20;
}
+static void check_against_sha1table(const unsigned char *sha1)
+{
+ const unsigned char *found;
+ if (!packv4)
+ return;
+
+ found = bsearch(sha1, sha1_table, nr_objects, 20,
+ (int (*)(const void *, const void *))hashcmp);
+ if (!found)
+ die(_(object %s not found in SHA-1 table),
+ sha1_to_hex(sha1));
+}
+
+static const unsigned char *read_sha1table_ref(void)
+{
+ const unsigned char *sha1 = read_sha1ref();
+ if (sha1 sha1_table || sha1 = sha1_table + nr_objects * 20)
+ check_against_sha1table(sha1);
+ return sha1;
+}
+
static const unsigned char *read_dictref(struct packv4_dict *dict)
{
unsigned int index = read_varint();
@@ -327,6 +349,84 @@ static void write_object(unsigned nr, enum object_type
type,
}
}
+static void resolve_tree_v4(unsigned long nr_obj,
+ const void *tree,
+ unsigned long tree_len,
+ const unsigned char *base_sha1,
+ const void *base,
+ unsigned long base_size)
+{
+ int nr;
+ struct strbuf sb = STRBUF_INIT;
+ const unsigned char *p = tree;
+ const unsigned char *end = p + tree_len;
+
+ nr = decode_varint(p);
+ while (nr 0 p end) {
+ unsigned int copy_start_or_path = decode_varint(p);
+ if (copy_start_or_path 1) { /* copy_start */
+ struct tree_desc desc;
+ struct name_entry entry;
+ unsigned int copy_count = decode_varint(p);
+ unsigned int copy_start = copy_start_or_path 1;
+ if (!base_sha1)
+ die(we are not supposed to copy from another
tree!);
+ if (copy_count 1) { /* first delta */
+ unsigned int id = decode_varint(p);
+ const unsigned char *last_base;
+ if (!id) {
+ last_base = p;
+ p += 20;
+ } else
+ last_base = sha1_table + (id - 1) * 20;
+ if (hashcmp(last_base, base_sha1))
+ die(bad base tree in resolve_tree_v4);
+ }
+
+ copy_count = 1;
+ nr -= copy_count;
+
+ init_tree_desc(desc, base, base_size);
+ while (tree_entry(desc, entry)) {
+ if (copy_start)
+ copy_start--;
+ else if (copy_count) {
+ strbuf_addf(sb, %o %s%c,
+ entry.mode, entry.path,
'\0');
+ strbuf_add(sb, entry.sha1, 20);
+ copy_count--;
+ } else
+ break;
+ }
+ } else {/* path */
+ unsigned int path_idx = copy_start_or_path 1;
+ const unsigned char *path;
+ unsigned mode;
+ unsigned int id;
+ const unsigned char *entry_sha1;
+
+ id = decode_varint(p);
+ if (!id) {
+ entry_sha1 = p;
+ p += 20;
+ } else
+ entry_sha1 = sha1_table + (id - 1) * 20;
+ nr--;
+
+ path = path_dict-data + path_dict-offsets[path_idx];
+ mode = (path[0] 8) | path[1];
+ strbuf_addf(sb, %o %s%c, mode, path+2, '\0');
+ strbuf_add(sb, entry_sha1, 20);
+ }
+ }
+ if (nr != 0 || p != end)
+ die(_(bad delta tree));
+ if (!dry_run)
+
[PATCH 17/21] unpack-objects: allow to save processed bytes to a buffer
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 6fc72c1..044a087 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -54,6 +54,9 @@ static void add_object_buffer(struct object *object, char
*buffer, unsigned long
die(object %s tried to add buffer twice!,
sha1_to_hex(object-sha1));
}
+static struct strbuf back_buffer = STRBUF_INIT;
+static int save_to_back_buffer;
+
/*
* Make sure at least min bytes are available in the buffer, and
* return the pointer to the buffer.
@@ -66,6 +69,8 @@ static void *fill(int min)
die(cannot fill %d bytes, min);
if (offset) {
git_SHA1_Update(ctx, buffer, offset);
+ if (save_to_back_buffer)
+ strbuf_add(back_buffer, buffer, offset);
memmove(buffer, buffer + offset, len);
offset = 0;
}
@@ -81,6 +86,18 @@ static void *fill(int min)
return buffer;
}
+static void copy_back_buffer(int set)
+{
+ if (offset) {
+ git_SHA1_Update(ctx, buffer, offset);
+ if (save_to_back_buffer)
+ strbuf_add(back_buffer, buffer, offset);
+ memmove(buffer, buffer + offset, len);
+ offset = 0;
+ }
+ save_to_back_buffer = set;
+}
+
static void use(int bytes)
{
if (bytes len)
--
1.8.2.82.gc24b958
--
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 16/21] unpack-objects: decode v4 commits
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/unpack-objects.c | 60
1 file changed, 60 insertions(+)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index f8442f4..6fc72c1 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -131,6 +131,15 @@ static const unsigned char *read_sha1ref(void)
return sha1_table + index * 20;
}
+static const unsigned char *read_dictref(struct packv4_dict *dict)
+{
+ unsigned int index = read_varint();
+ if (index = dict-nb_entries)
+ die(bad index in read_dictref at %lu,
+ (unsigned long)consumed_bytes);
+ return dict-data + dict-offsets[index];
+}
+
static void *get_data(unsigned long size)
{
git_zstream stream;
@@ -467,6 +476,54 @@ static void unpack_delta_entry(enum object_type type,
unsigned long delta_size,
free(base);
}
+static void unpack_commit_v4(unsigned long size, unsigned long nr)
+{
+ unsigned int nb_parents;
+ const unsigned char *committer, *author, *ident;
+ unsigned long author_time, committer_time;
+ int16_t committer_tz, author_tz;
+ struct strbuf dst;
+ char *remaining;
+
+ strbuf_init(dst, size);
+
+ strbuf_addf(dst, tree %s\n, sha1_to_hex(read_sha1ref()));
+ nb_parents = read_varint();
+ while (nb_parents--)
+ strbuf_addf(dst, parent %s\n, sha1_to_hex(read_sha1ref()));
+
+ committer_time = read_varint();
+ ident = read_dictref(name_dict);
+ committer_tz = (ident[0] 8) | ident[1];
+ committer = ident + 2;
+
+ author_time = read_varint();
+ ident = read_dictref(name_dict);
+ author_tz = (ident[0] 8) | ident[1];
+ author = ident + 2;
+
+ if (author_time 1)
+ author_time = committer_time + (author_time 1);
+ else
+ author_time = committer_time - (author_time 1);
+
+ strbuf_addf(dst,
+ author %s %lu %+05d\n
+ committer %s %lu %+05d\n,
+ author, author_time, author_tz,
+ committer, committer_time, committer_tz);
+
+ if (dst.len size)
+ die(bad commit);
+
+ remaining = get_data(size - dst.len);
+ strbuf_add(dst, remaining, size - dst.len);
+ if (!dry_run)
+ write_object(nr, OBJ_COMMIT, dst.buf, dst.len);
+ else
+ strbuf_release(dst);
+}
+
static void read_typesize_v2(enum object_type *type, unsigned long *size)
{
unsigned char c = *(char*)fill_and_use(1);
@@ -511,6 +568,9 @@ static int unpack_one(unsigned nr)
case OBJ_OFS_DELTA:
unpack_delta_entry(type, size, nr);
break;
+ case OBJ_PV4_COMMIT:
+ unpack_commit_v4(size, nr);
+ break;
default:
error(bad object type %d, type);
has_errors = 1;
--
1.8.2.82.gc24b958
--
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 20/21] show-index: acknowledge that it does not read .idx v3
show-index takes .idx from stdin while v3 requires the .pack. It's
used for testing purposes only. Let those test scripts force .idx v2
with index-pack.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
show-index.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/show-index.c b/show-index.c
index 5a9eed7..2028e02 100644
--- a/show-index.c
+++ b/show-index.c
@@ -19,8 +19,10 @@ int main(int argc, char **argv)
die(unable to read header);
if (top_index[0] == htonl(PACK_IDX_SIGNATURE)) {
version = ntohl(top_index[1]);
- if (version 2 || version 2)
+ if (version 2 || version 3)
die(unknown index version);
+ if (version == 3)
+ die(show-index does not support .idx v3, convert to v2
instead);
if (fread(top_index, 256 * 4, 1, stdin) != 1)
die(unable to read index);
} else {
--
1.8.2.82.gc24b958
--
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 19/21] index-pack, pack-objects: allow creating .idx v2 with .pack v4
While .idx v3 is recommended because it's smaller, there is no reason
why .idx v2 can't use with .pack v4. Enable it, at least for the test
suite as some tests need to this kind of information from show-index
and show-index does not support .idx v3.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/index-pack.c | 14 ++
builtin/pack-objects.c | 14 ++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1895adf..4607dc6 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -89,7 +89,7 @@ static int verbose;
static int show_stat;
static int check_self_contained_and_connected;
static int packv4;
-
+static int idx_version_set;
static struct progress *progress;
/* We always read in 4kB chunks. */
@@ -1892,8 +1892,9 @@ static int git_index_pack_config(const char *k, const
char *v, void *cb)
if (!strcmp(k, pack.indexversion)) {
opts-version = git_config_int(k, v);
- if (opts-version 2)
+ if (opts-version 3)
die(_(bad pack.indexversion=%PRIu32), opts-version);
+ idx_version_set = 1;
return 0;
}
if (!strcmp(k, pack.threads)) {
@@ -2107,12 +2108,13 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
} else if (!prefixcmp(arg, --index-version=)) {
char *c;
opts.version = strtoul(arg + 16, c, 10);
- if (opts.version 2)
+ if (opts.version 3)
die(_(bad %s), arg);
if (*c == ',')
opts.off32_limit = strtoul(c+1, c, 0);
if (*c || opts.off32_limit 0x8000)
die(_(bad %s), arg);
+ idx_version_set = 1;
} else
usage(index_pack_usage);
continue;
@@ -2151,6 +2153,7 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
if (!index_name)
die(_(--verify with no packfile name given));
read_idx_option(opts, index_name);
+ idx_version_set = 1;
opts.flags |= WRITE_IDX_VERIFY | WRITE_IDX_STRICT;
}
if (strict)
@@ -2167,6 +2170,9 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
curr_pack = open_pack_file(pack_name);
parse_pack_header();
+ if (!packv4 opts.version = 3)
+ die(_(pack idx version %d does not work with pack version %d),
+ opts.version, 4);
objects = xcalloc(nr_objects + 1, sizeof(struct object_entry));
deltas = xcalloc(nr_objects, sizeof(struct delta_entry));
parse_dictionaries();
@@ -2180,7 +2186,7 @@ int cmd_index_pack(int argc, const char **argv, const
char *prefix)
if (show_stat)
show_pack_info(stat_only);
- if (packv4)
+ if (packv4 !idx_version_set)
opts.version = 3;
idx_objects = xmalloc((nr_objects) * sizeof(struct pack_idx_entry *));
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ac25973..f604fa5 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -66,7 +66,7 @@ static uint32_t nr_objects, nr_alloc, nr_result, nr_written;
static struct packv4_tables v4;
-static int non_empty;
+static int non_empty, idx_version_set;
static int reuse_delta = 1, reuse_object = 1;
static int keep_unreachable, unpack_unreachable, include_tag;
static unsigned long unpack_unreachable_expiration;
@@ -2205,7 +2205,8 @@ static void prepare_pack(int window, int depth)
sort_dict_entries_by_hits(v4.commit_ident_table);
sort_dict_entries_by_hits(v4.tree_path_table);
v4.all_objs = xmalloc(nr_objects * sizeof(*v4.all_objs));
- pack_idx_opts.version = 3;
+ if (!idx_version_set)
+ pack_idx_opts.version = 3;
allow_ofs_delta = 0;
}
@@ -2319,9 +2320,10 @@ static int git_pack_config(const char *k, const char *v,
void *cb)
}
if (!strcmp(k, pack.indexversion)) {
pack_idx_opts.version = git_config_int(k, v);
- if (pack_idx_opts.version 2)
+ if (pack_idx_opts.version 3)
die(bad pack.indexversion=%PRIu32,
pack_idx_opts.version);
+ idx_version_set = 1;
return 0;
}
return git_default_config(k, v, cb);
@@ -2604,12 +2606,13 @@ static int option_parse_index_version(const struct
option *opt,
char *c;
const char *val = arg;
[PATCH 21/21] t1050, t5500: replace the use of show-index|wc -l with verify-pack
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
t/t1050-large.sh | 9 +
t/t5500-fetch-pack.sh | 4 ++--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index fd10528..829030b 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -32,7 +32,7 @@ test_expect_success 'add a large file or two' '
done
test -z $bad
test $count = 1
- cnt=$(git show-index $idx | wc -l)
+ cnt=$(git verify-pack -v ${idx/idx/pack} | grep ^[0-9a-f]\{40\} |
wc -l)
test $cnt = 2
for l in .git/objects/??/??
do
@@ -93,11 +93,12 @@ test_expect_success 'packsize limit' '
) |
sort expect
- for pi in .git/objects/pack/pack-*.idx
+ for pi in .git/objects/pack/pack-*.pack
do
- git show-index $pi
+ git verify-pack -v $pi
done |
- sed -e s/^[0-9]* \([0-9a-f]*\) .*/\1/ |
+ grep ^[0-9a-f]\{40\} |
+ sed -e s/^\([0-9a-f]\{40\}\) .*/\1/ |
sort actual
test_cmp expect actual
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index fd2598e..f99cd14 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -60,8 +60,8 @@ pull_to_client () {
git unpack-objects $p
git fsck --full
- idx=`echo pack-*.idx`
- pack_count=`git show-index $idx | wc -l`
+ pack=`echo pack-*.pack`
+ pack_count=`git verify-pack -v $pack | grep
^[0-9a-f]\{40\} | wc -l`
test $pack_count = $count
rm -f pack-*
)
--
1.8.2.82.gc24b958
--
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] Disabling status hints in COMMIT_EDITMSG
Junio C Hamano gits...@pobox.com writes: But at the same time, I feel that these redundant lines, especially the latter one, would give the users a stronger cue than just saying that bar is Untracked; do X to include reminds that bar will not be included if nothing is done. The one which draw my attention was (use git commit to conclude merge) which is particularly counter-productive when you are already doing a git commit. The advice for untracked files is less counter-productive, but while we're removing the non-sensical ones, I think it makes sense to remove the essentially-useless ones too. -- 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: [RFC] Disabling status hints in COMMIT_EDITMSG
IMHO, It is alright as it is. I have been using git for 4~ years now, and I still find very useful those lines. They are like a git status while committing, and it's the key to avoid accidental commits of objects or forgetting files in a commit. Between that and that the commit message can't be empty, I can abort a commit and correct the staging area. Cheers, Javier Domingo -- 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 2/2] rm: re-use parse_pathspec's trailing-slash removal
On Wed, Sep 11, 2013 at 2:13 AM, John Keeping j...@keeping.me.uk wrote:
Instead of re-implementing the remove trailing slashes loop in
builtin/rm.c just pass PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to
parse_pathspec.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 9b59ab3..3a0e0ea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -298,22 +298,10 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
if (read_cache() 0)
die(_(index file corrupt));
- /*
-* Drop trailing directory separators from directories so we'll find
-* submodules in the index.
-*/
- for (i = 0; i argc; i++) {
- size_t pathlen = strlen(argv[i]);
- if (pathlen is_dir_sep(argv[i][pathlen - 1])
- is_directory(argv[i])) {
- do {
- pathlen--;
- } while (pathlen is_dir_sep(argv[i][pathlen - 1]));
- argv[i] = xmemdupz(argv[i], pathlen);
- }
- }
-
- parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
I notice that _CHEAP implementation and the removed code are not
exactly the same. But I think they have the same purpose so it's
probably ok even there are some subtle behavioral changes.
You may want to improve _CHEAP to remove consecutive trailing slashes
(i.e. foo - foo) too. And maybe is is_dir_sep() instead of
explicit == '/' comparison in there.
+ prefix, argv);
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
seen = xcalloc(pathspec.nr, 1);
--
Duy
--
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] Disabling status hints in COMMIT_EDITMSG
Javier Domingo javier...@gmail.com writes: IMHO, It is alright as it is. I have been using git for 4~ years now, and I still find very useful those lines. They are like a git status while committing, and it's the key to avoid accidental commits of objects or forgetting files in a commit. Having the list of staged/unstaged/untracked is the key to that, and I'm not planning on changing that. Having advices on how to change it (run ... to ...) is another matter. -- 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: [PATCH 1/2] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 08:05:44AM +0200, Johannes Sixt wrote:
Am 10.09.2013 21:13, schrieb John Keeping:
When using tab-completion, a directory path will often end with a
trailing slash which currently confuses git rm when dealing with
submodules. Now that we have parse_pathspec we can easily handle this
by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/reset.c| 5 +
t/t7400-submodule-basic.sh | 6 --
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5e4c551..9efac0f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
+
+ if (read_cache() 0)
+ die(_(index file corrupt));
When the index is now read here, I would have expected hunk in this
patch that removes a read_cache() invocation.
I think that needs to look like this on top - there's also a
read_cache_unmerged() around line 68 but I don't think we can remove
that one.
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec *pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const
char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
--
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 2/2] rm: re-use parse_pathspec's trailing-slash removal
On Wed, Sep 11, 2013 at 02:48:51PM +0700, Duy Nguyen wrote:
On Wed, Sep 11, 2013 at 2:13 AM, John Keeping j...@keeping.me.uk wrote:
Instead of re-implementing the remove trailing slashes loop in
builtin/rm.c just pass PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to
parse_pathspec.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 9b59ab3..3a0e0ea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -298,22 +298,10 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
if (read_cache() 0)
die(_(index file corrupt));
- /*
-* Drop trailing directory separators from directories so we'll find
-* submodules in the index.
-*/
- for (i = 0; i argc; i++) {
- size_t pathlen = strlen(argv[i]);
- if (pathlen is_dir_sep(argv[i][pathlen - 1])
- is_directory(argv[i])) {
- do {
- pathlen--;
- } while (pathlen is_dir_sep(argv[i][pathlen -
1]));
- argv[i] = xmemdupz(argv[i], pathlen);
- }
- }
-
- parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
I notice that _CHEAP implementation and the removed code are not
exactly the same. But I think they have the same purpose so it's
probably ok even there are some subtle behavioral changes.
Providing that there's only one trailing slash, the user-visible effect
should be the same since the only case affected by that is submodules.
In fact _CHEAP does better in the case where the submodule does not
exist in the working tree.
You may want to improve _CHEAP to remove consecutive trailing slashes
(i.e. foo - foo) too. And maybe is is_dir_sep() instead of
explicit == '/' comparison in there.
Sounds good, I'll try to look at that tonight.
+ prefix, argv);
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
seen = xcalloc(pathspec.nr, 1);
--
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/2] wt-status: turn advice_status_hints into a field of wt_status
No behavior change in this patch, but this makes the display of status
hints more flexible as they can be enabled or disabled for individual
calls to commit.c:run_status().
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
builtin/commit.c | 10 --
wt-status.c | 38 +++---
wt-status.h | 1 +
3 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 60812b5..388acde 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -163,6 +163,12 @@ static void determine_whence(struct wt_status *s)
s-whence = whence;
}
+static void status_finalize(struct wt_status *s)
+{
+ determine_whence(s);
+ s-hints = advice_status_hints;
+}
+
static void rollback_index_files(void)
{
switch (commit_style) {
@@ -1249,7 +1255,7 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
wt_status_prepare(s);
gitmodules_config();
git_config(git_status_config, s);
- determine_whence(s);
+ status_finalize(s);
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
@@ -1494,7 +1500,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
gitmodules_config();
git_config(git_commit_config, s);
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
- determine_whence(s);
+ status_finalize(s);
s.colopts = 0;
if (get_sha1(HEAD, sha1))
diff --git a/wt-status.c b/wt-status.c
index cb24f1f..885ee66 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -160,7 +160,7 @@ static void wt_status_print_unmerged_header(struct
wt_status *s)
}
}
- if (!advice_status_hints)
+ if (!s-hints)
return;
if (s-whence != FROM_COMMIT)
;
@@ -187,7 +187,7 @@ static void wt_status_print_cached_header(struct wt_status
*s)
const char *c = color(WT_STATUS_HEADER, s);
status_printf_ln(s, c, _(Changes to be committed:));
- if (!advice_status_hints)
+ if (!s-hints)
return;
if (s-whence != FROM_COMMIT)
; /* NEEDSWORK: use git reset --unresolve??? */
@@ -205,7 +205,7 @@ static void wt_status_print_dirty_header(struct wt_status
*s,
const char *c = color(WT_STATUS_HEADER, s);
status_printf_ln(s, c, _(Changes not staged for commit:));
- if (!advice_status_hints)
+ if (!s-hints)
return;
if (!has_deleted)
status_printf_ln(s, c, _( (use \git add file...\ to
update what will be committed)));
@@ -223,7 +223,7 @@ static void wt_status_print_other_header(struct wt_status
*s,
{
const char *c = color(WT_STATUS_HEADER, s);
status_printf_ln(s, c, %s:, what);
- if (!advice_status_hints)
+ if (!s-hints)
return;
status_printf_ln(s, c, _( (use \git %s file...\ to include in
what will be committed)), how);
status_printf_ln(s, c, );
@@ -801,13 +801,13 @@ static void show_merge_in_progress(struct wt_status *s,
{
if (has_unmerged(s)) {
status_printf_ln(s, color, _(You have unmerged paths.));
- if (advice_status_hints)
+ if (s-hints)
status_printf_ln(s, color,
_( (fix conflicts and run \git commit\)));
} else {
status_printf_ln(s, color,
_(All conflicts fixed but you are still merging.));
- if (advice_status_hints)
+ if (s-hints)
status_printf_ln(s, color,
_( (use \git commit\ to conclude merge)));
}
@@ -823,7 +823,7 @@ static void show_am_in_progress(struct wt_status *s,
if (state-am_empty_patch)
status_printf_ln(s, color,
_(The current patch is empty.));
- if (advice_status_hints) {
+ if (s-hints) {
if (!state-am_empty_patch)
status_printf_ln(s, color,
_( (fix conflicts and then run \git am
--continue\)));
@@ -896,7 +896,7 @@ static void show_rebase_in_progress(struct wt_status *s,
else
status_printf_ln(s, color,
_(You are currently rebasing.));
- if (advice_status_hints) {
+ if (s-hints) {
status_printf_ln(s, color,
_( (fix conflicts and then run \git rebase
--continue\)));
status_printf_ln(s, color,
@@ -913,7 +913,7 @@ static void show_rebase_in_progress(struct wt_status *s,
else
status_printf_ln(s, color,
_(You are
[PATCH 2/2] commit: disable status hints when writing to COMMIT_EDITMSG
This turns the template COMMIT_EDITMSG from e.g # [...] # Changes to be committed: # (use git reset HEAD file... to unstage) # # modified: builtin/commit.c # # Untracked files: # (use git add file... to include in what will be committed) # # t/foo # to # [...] # Changes to be committed: # modified: builtin/commit.c # # Untracked files: # t/foo # Most status hints were written to be accurate when running git status before running a commit. Many of them are not applicable when the commit has already been started, and should not be shown in COMMIT_EDITMSG. The most obvious are hints advising to run git commit, git rebase/am/cherry-pick --continue, which do not make sense when the command has already been ran. Other messages become slightly inaccurate (e.g. hint to use git add to add untracked files), as the suggested commands are not immediately applicable during the edition of COMMIT_EDITMSG, but would be applicable if the commit is aborted. These messages are both potentially helpful and slightly misleading. This patch chose to remove them too, to avoid introducing too much complexity in the status code. Signed-off-by: Matthieu Moy matthieu@imag.fr --- Junio, you'll get a trivial merge conflict with my other status series, as they both add a few lines of code at the same location. There should be no semantic conflict so I didn't consider the branches as dependant. builtin/commit.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index 388acde..6251d29 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -702,6 +702,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (s-fp == NULL) die_errno(_(could not open '%s'), git_path(commit_editmsg)); + /* +* Most hints are counter-productive when the commit has +* already started. +*/ + s-hints = 0; + if (clean_message_contents) stripspace(sb, 0); -- 1.8.4.8.g834017f -- 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 2/2] commit: disable status hints when writing to COMMIT_EDITMSG
On Wed, Sep 11, 2013 at 5:08 AM, Matthieu Moy matthieu@imag.fr wrote: Most status hints were written to be accurate when running git status before running a commit. Many of them are not applicable when the commit has already been started, and should not be shown in COMMIT_EDITMSG. The most obvious are hints advising to run git commit, git rebase/am/cherry-pick --continue, which do not make sense when the command has already been ran. s/ran/run/ Other messages become slightly inaccurate (e.g. hint to use git add to add untracked files), as the suggested commands are not immediately applicable during the edition of COMMIT_EDITMSG, but would be applicable s/edition/editing/ if the commit is aborted. These messages are both potentially helpful and slightly misleading. This patch chose to remove them too, to avoid introducing too much complexity in the status code. Signed-off-by: Matthieu Moy matthieu@imag.fr -- 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] Disabling status hints in COMMIT_EDITMSG
On Wed, Sep 11, 2013 at 3:24 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Junio C Hamano gits...@pobox.com writes: But at the same time, I feel that these redundant lines, especially the latter one, would give the users a stronger cue than just saying that bar is Untracked; do X to include reminds that bar will not be included if nothing is done. The one which draw my attention was (use git commit to conclude merge) which is particularly counter-productive when you are already doing a git commit. The advice for untracked files is less counter-productive, but while we're removing the non-sensical ones, I think it makes sense to remove the essentially-useless ones too. FWIW, I think it makes sense to remove the extra advice too. -John -- 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 0/3] Reject non-ff pulls by default
On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: The problem is the newcomers, and the newcomers will most definitely not activate a configuration option to tell them that they are doing something potentially undesirable. I teach Git to 200 newcommers each year. All of them run git pull the first day, but believe me, very few of them want to know what a rebase is at that time. And who says they have to? This is a straw man argument. May of them don't want to know what the staging area is, that's why they run 'git commit --all', and just like that they can run 'git pull --merge'. By the time they learn about pull.mode, they probably already know what a rebase is. So what is the point of the configuration in the first place? [...] That doesn't mean anything, you are assuming the user will do 'git pull --rebase', and there's no rationale as to why they would end up doing that. So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? Because as you said, they don't know what that is. 'git commit' by default prevents users from creating commits without first adding changes to the staging area, and since it's a concept unique to Git, it's fair to say that none of the newcomers understand why 'git commit' is failing, the error messages is not particularly useful either. I don't particularly agree that not defaulting to --all was a good idea, but that's another topic. It the same topic, the project already made a choice, and precisely because of the same reasoning that 'git commit --all' is required, 'git pull --merge' should be required. But the error message is rather clear: no changes added to commit (use git add and/or git commit -a) And we can do the same: Read more with 'git pull --help' or do 'git pull --merge'. -- Felipe Contreras -- 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] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 3:20 PM, John Keeping j...@keeping.me.uk wrote:
On Wed, Sep 11, 2013 at 08:05:44AM +0200, Johannes Sixt wrote:
Am 10.09.2013 21:13, schrieb John Keeping:
When using tab-completion, a directory path will often end with a
trailing slash which currently confuses git rm when dealing with
submodules. Now that we have parse_pathspec we can easily handle this
by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/reset.c| 5 +
t/t7400-submodule-basic.sh | 6 --
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5e4c551..9efac0f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
+
+ if (read_cache() 0)
+ die(_(index file corrupt));
When the index is now read here, I would have expected hunk in this
patch that removes a read_cache() invocation.
I think that needs to look like this on top - there's also a
read_cache_unmerged() around line 68 but I don't think we can remove
that one.
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec *pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const
char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
reset --soft does not go through these code paths (i.e. it does not
need index at all). If we fail to load index index in reset --soft I
think it's ok to die(). Corrupt index is fatal anyway. But reset
--soft now has to pay the cost to load index, which could be slow
when the index is big. Assuming nobody does reset --soft that often
I think this is OK.
Alternatively we could load index lazily in _CHEAP code only when we
see trailing slashes, then replace these read_cache() with
read_cache_unless_its_already_loaded_earlier() or something.
--
Duy
--
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] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 05:54:48PM +0700, Duy Nguyen wrote:
On Wed, Sep 11, 2013 at 3:20 PM, John Keeping j...@keeping.me.uk wrote:
On Wed, Sep 11, 2013 at 08:05:44AM +0200, Johannes Sixt wrote:
Am 10.09.2013 21:13, schrieb John Keeping:
When using tab-completion, a directory path will often end with a
trailing slash which currently confuses git rm when dealing with
submodules. Now that we have parse_pathspec we can easily handle this
by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/reset.c| 5 +
t/t7400-submodule-basic.sh | 6 --
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5e4c551..9efac0f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
+
+ if (read_cache() 0)
+ die(_(index file corrupt));
When the index is now read here, I would have expected hunk in this
patch that removes a read_cache() invocation.
I think that needs to look like this on top - there's also a
read_cache_unmerged() around line 68 but I don't think we can remove
that one.
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec
*pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const
char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
reset --soft does not go through these code paths (i.e. it does not
need index at all). If we fail to load index index in reset --soft I
think it's ok to die(). Corrupt index is fatal anyway. But reset
--soft now has to pay the cost to load index, which could be slow
when the index is big. Assuming nobody does reset --soft that often
I think this is OK.
Alternatively we could load index lazily in _CHEAP code only when we
see trailing slashes, then replace these read_cache() with
read_cache_unless_its_already_loaded_earlier() or something.
read_cache() already has an early return if the index is already loaded
so I don't think we need to worry about a special function for that.
I'm not sure it's worth optimizing this case too heavily, but it might
be a nice change to make parse_pathspec() not rely on the index being
loaded before it is called with certain flags.
--
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] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 6:08 PM, John Keeping j...@keeping.me.uk wrote:
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec
*pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb,
const char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
reset --soft does not go through these code paths (i.e. it does not
need index at all). If we fail to load index index in reset --soft I
think it's ok to die(). Corrupt index is fatal anyway. But reset
--soft now has to pay the cost to load index, which could be slow
when the index is big. Assuming nobody does reset --soft that often
I think this is OK.
Alternatively we could load index lazily in _CHEAP code only when we
see trailing slashes, then replace these read_cache() with
read_cache_unless_its_already_loaded_earlier() or something.
read_cache() already has an early return if the index is already loaded
so I don't think we need to worry about a special function for that.
I'm not sure it's worth optimizing this case too heavily, but it might
be a nice change to make parse_pathspec() not rely on the index being
loaded before it is called with certain flags.
Yeah I ddin't check. I agree putting read_cache() in _CHEAP code
sounds nice. We won't need to worry about forgotten read_cache()
elsewhere.
--
Duy
--
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-Transmission of blobs?
On Di, Sep 10, 2013 at 10:51:02 -0700, Junio C Hamano wrote: Consider this simple history with only a handful of commits (as usual, time flows from left to right): E / A---B---C---D where D is at the tip of the sending side, E is at the tip of the receiving side. The exchange goes roughly like this: (receiving side): what do you have? (sending side): my tip is at D. (receiving side): D? I've never heard of it --- please give it to me. I have E. At this point, why would the receiving side not tell all the heads it knows about? That would enable the sending side to see whether it knows any of those commits. It then would be able to remove from the sending list all the objects that can be reached from the commits it knows about. (sending side): E? I don't know about it; must be something you created since you forked from me. Tell me about its ancestors. This is not exactly true. In the example I had given, the sending side has all three commits: C, D and E. So the sending side has no reason to say that it doesn't know anything about E. Therefore the sending side has all information it needs to deduce exactly which objects need to be sent to the receiving side. What needs to be sent are all the objects in C..D but without all the objects in C..E. I guess this operation would be called set-difference in english. And if the receiving side would have told that it has heads X Y Z in addition, and the sending side happens to have Y, then the sending side could in addition remove any objects that can be reached from Y from the sending list. -- Josef Wolf j...@raven.inka.de -- 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 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? Because as you said, they don't know what that is. That does not answer my question: why ask? Look around you what people say about Git. See how many complain about Git not exposing enough complexity to the user. See how many would complain about Git not advertising rebase enough. Then, look how many complain about Git exposing too much complexity and making it too easy to use features like rebase. -- 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: [PATCH v6 7/8] update-ref: support multiple simultaneous updates
On 09/10/2013 06:51 PM, Eric Sunshine wrote:
On Mon, Sep 9, 2013 at 8:57 PM, Brad King brad.k...@kitware.com wrote:
+Use 40 0 or the empty string to specify a zero value, except that
Did you want an 's' after the 0?
The same description without 's' already appears in git-update-ref.txt
above this location in the existing documentation of the command-line
option behavior. I see 0{40} in git-receive-pack.txt and also in
howto/update-hook-example.txt. Perhaps a follow-up change can be made
to choose a consistent way to describe 40 0s.
-Brad
--
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 v6.1 8/8] update-ref: add test cases covering --stdin signature
Extend t/t1400-update-ref.sh to cover cases using the --stdin option.
Signed-off-by: Brad King brad.k...@kitware.com
---
On 09/10/2013 06:46 PM, Eric Sunshine wrote:
Thus printf provides all the functionality you require, and
print_nul() function can be dropped. So:
printf '%s\0' foo bar baz
Wonderful, thanks! The single-quotes do not fit easily inside
test code blocks that are themselves single-quoted, so I packaged
the format up in a variable. Here is a revised patch.
-Brad
t/t1400-update-ref.sh | 632 ++
1 file changed, 632 insertions(+)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index e415ee0..6ffd82f 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -302,4 +302,636 @@ test_expect_success \
'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER)' \
'test OTHER = $(git cat-file blob master@{2005-05-26 23:42}:F)'
+a=refs/heads/a
+b=refs/heads/b
+c=refs/heads/c
+E=''
+F='%s\0'
+pws='path with space'
+
+test_expect_success 'stdin test setup' '
+ echo $pws $pws
+ git add -- $pws
+ git commit -m $pws
+'
+
+test_expect_success '-z fails without --stdin' '
+ test_must_fail git update-ref -z $m $m $m 2err
+ grep usage: git update-ref err
+'
+
+test_expect_success 'stdin works with no input' '
+ stdin
+ git update-ref --stdin stdin
+ git rev-parse --verify -q $m
+'
+
+test_expect_success 'stdin fails on empty line' '
+ echo stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: empty command in input err
+'
+
+test_expect_success 'stdin fails on only whitespace' '
+ echo stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: whitespace before command: err
+'
+
+test_expect_success 'stdin fails on leading whitespace' '
+ echo create $a $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: whitespace before command: create $a $m err
+'
+
+test_expect_success 'stdin fails on unknown command' '
+ echo unknown $a stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: unknown command: unknown $a err
+'
+
+test_expect_success 'stdin fails on badly quoted input' '
+ echo create $a \master stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: badly quoted argument: \\\master err
+'
+
+test_expect_success 'stdin fails on arguments not separated by space' '
+ echo create \$a\master stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: expected SP but got: master err
+'
+
+test_expect_success 'stdin fails create with no ref' '
+ echo create stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: create line missing ref err
+'
+
+test_expect_success 'stdin fails create with bad ref name' '
+ echo create ~a $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: invalid ref format: ~a err
+'
+
+test_expect_success 'stdin fails create with no new value' '
+ echo create $a stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: create $a missing newvalue err
+'
+
+test_expect_success 'stdin fails create with too many arguments' '
+ echo create $a $m $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: create $a has extra input: $m err
+'
+
+test_expect_success 'stdin fails update with no ref' '
+ echo update stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: update line missing ref err
+'
+
+test_expect_success 'stdin fails update with bad ref name' '
+ echo update ~a $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: invalid ref format: ~a err
+'
+
+test_expect_success 'stdin fails update with no new value' '
+ echo update $a stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: update $a missing newvalue err
+'
+
+test_expect_success 'stdin fails update with too many arguments' '
+ echo update $a $m $m $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: update $a has extra input: $m err
+'
+
+test_expect_success 'stdin fails delete with no ref' '
+ echo delete stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: delete line missing ref err
+'
+
+test_expect_success 'stdin fails delete with bad ref name' '
+ echo delete ~a $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: invalid ref format: ~a err
+'
+
+test_expect_success 'stdin fails delete with too many arguments' '
+ echo delete $a $m $m stdin
+ test_must_fail git update-ref --stdin stdin 2err
+ grep fatal: delete $a has extra input: $m err
+'
+
Re: git-cvsserver strips exec bit
Junio C Hamano wrote: Then what I wrote was actually relevant;-) I am not sure if we want to use the owner bit (i.e. 4th place) instead of the other bit (i.e. the last place) like this patch does, though. The old code in 1.8.1.x would have produced either r (for 100644) or wx (for 100755); I think that the result of applying this patch would give us r (for 100644) or rx (for 100755). This should then work, I would think. Yes, this new patch fixes the issue. Thanks! Michael -- 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: Git tag output order is incorrect (IMHO)
Someone at $work asked me this week how to find the current and previous tags on his branch so he could generate release notes. I just need last two tags on head in topo-order. I was surprised by how complicated this turned out to be. I ended up with this: git log --decorate=full --pretty=format:'%d' HEAD | sed -n -e 's-^.* refs/tags/\(.*\)[ )].*$-\1-p' | head -2 Surely there's a cleaner way, right? Phil On Sun, Sep 8, 2013 at 6:49 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Thu, Jul 18, 2013 at 10:27 AM, Rahul Bansal rahul.ban...@rtcamp.com wrote: I am posting here first time, so please excuse me if this is not right place to send something like this. Please check - http://stackoverflow.com/questions/6091306/can-i-make-git-print-x-y-z-style-tag-names-in-a-sensible-order And also - https://github.com/gitlabhq/gitlabhq/issues/4565 IMHO git tag is expected to show tag-list ordered by versions. It may be case, that people do not follow same version numbering convention. Most people after x.9.x increment major version (that is why they may not be affected because of this) Another option like git tag --date-asc can be added which will print tags by creation date. (As long as people do not create backdated tag, this will work). I completely agree, and there was a proposal to an option like this a long time ago: http://article.gmane.org/gmane.comp.version-control.git/111032 -- Felipe Contreras -- 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: [RFC] Disabling status hints in COMMIT_EDITMSG
That extra info doesn't occupy too much, and helps distinguish between sections. They do also remember you the commands to use (thought after some time using git, you may not need it). Cheers, Javier -- 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 00/21] np/pack-v4 updates
Nico, if you have time you may want to look into this. The result v4 pack from pack-objects on git.git for me is 35MB (one branch) while packv4-create produces 30MB (v2 is 40MB). I don't know why there is such a big difference in size. I compared. Ident dict is identical. Tree dict is a bit different (some that have same hits are ordered differently). Delta chains do not differ much. Many groups of entries in the pack are displaced though. I guess I turned a wrong knob or something in pack-objects in v4 code.. -- DUy -- 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 19/21] index-pack, pack-objects: allow creating .idx v2 with .pack v4
On Wed, 11 Sep 2013, Nguyễn Thái Ngọc Duy wrote: While .idx v3 is recommended because it's smaller, there is no reason why .idx v2 can't use with .pack v4. Enable it, at least for the test suite as some tests need to this kind of information from show-index and show-index does not support .idx v3. FYI, I've added that ability to show-index in my tree. The output does not include the actual object SHA1 though. [...] @@ -2167,6 +2170,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) curr_pack = open_pack_file(pack_name); parse_pack_header(); + if (!packv4 opts.version = 3) + die(_(pack idx version %d does not work with pack version %d), + opts.version, 4); I don't think this is what you really meant here. I've amended this patch with: diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 4607dc6..f071ed9 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -2171,8 +2171,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) curr_pack = open_pack_file(pack_name); parse_pack_header(); if (!packv4 opts.version = 3) - die(_(pack idx version %d does not work with pack version %d), - opts.version, 4); + die(_(pack idx version %d requires at least pack version 4), + opts.version); objects = xcalloc(nr_objects + 1, sizeof(struct object_entry)); deltas = xcalloc(nr_objects, sizeof(struct delta_entry)); parse_dictionaries(); Nicolas
[PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory
Git use, as many project that use autoconf, private m4 macros.
When not using automake, and just relying on autoconf, the macro
files are not picked up by default.
A possibility, as git do today, is to put the private m4 macro
in the configure.ac file, so they will copied over the final configure
when calling autoreconf(that call also autoconf).
But this makes configure.ac difficult to read and maintain,
especially if you want to introduce new macros later. By separating
the definitions of the macros from configure.ac file the build system
would be more modular.
Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR
to declare where additional macro files are to be put and found by aclocal.
The argument passed to this macro is commonly m4. Despite the documentation,
autoconf do nothing with it, only aclocal can use directly if invoked by
-I m4 or indirectly using automake. But autoreconf don't invoke aclocal
in this way. So in summary you can not use this macro in a useful
way if you only use autoconf, as git does.
Another historical possibility is to list all your macros in acinclude.m4.
This file will be included in aclocal.m4 when you run aclocal, and its macro(s)
will henceforth be visible to autoconf. However if it contains numerous macros,
it will rapidly become difficult to maintain, and for git this don't provide
any benefits or very little.
The actual autotool documentation recommend to write each
macro in its own file and gather all these files in a separate directory.
Given the limitations i mentioned earlier, the only possibility is to use the
m4_include
for including every macro file. The m4_include directive works quite like the
#include directive of the C programming language, and simply copies over the
content
of the file(s).
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
This is a second version of this patch
http://article.gmane.org/gmane.comp.version-control.git/231984.
The first was plain wrong, my bad. I am sorry for the long delay.
Sure it is something low-hanging fruit
configure.ac | 148 +++--
m4/git_arg_set_path.m4| 14
m4/git_check_func.m4 | 13
m4/git_conf_append_path.m4| 30
m4/git_conf_subst.m4 | 10 +++
m4/git_conf_subst_init.m4 | 15
m4/git_parse_with.m4 | 22 ++
m4/git_parse_with_set_make_var.m4 | 20 +
m4/git_stash_flags.m4 | 15
m4/git_unstash_flags.m4 | 13
10 files changed, 162 insertions(+), 138 deletions(-)
create mode 100644 m4/git_arg_set_path.m4
create mode 100644 m4/git_check_func.m4
create mode 100644 m4/git_conf_append_path.m4
create mode 100644 m4/git_conf_subst.m4
create mode 100644 m4/git_conf_subst_init.m4
create mode 100644 m4/git_parse_with.m4
create mode 100644 m4/git_parse_with_set_make_var.m4
create mode 100644 m4/git_stash_flags.m4
create mode 100644 m4/git_unstash_flags.m4
diff --git a/configure.ac b/configure.ac
index 2f43393..81a876f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,144 +1,6 @@
# -*- Autoconf -*-
# Process this file with autoconf to produce a configure script.
-## Definitions of private macros.
-
-# GIT_CONF_SUBST(VAL, VAR)
-#
-# Cause the line VAR=VAL to be eventually appended to ${config_file}.
-AC_DEFUN([GIT_CONF_SUBST],
-[AC_REQUIRE([GIT_CONF_SUBST_INIT])
-config_appended_defs=$config_appended_defs${newline}dnl
-$1=m4_if([$#],[1],[${$1}],[$2])])
-
-# GIT_CONF_SUBST_INIT
-# ---
-# Prepare shell variables and autoconf machine required by later calls
-# to GIT_CONF_SUBST.
-AC_DEFUN([GIT_CONF_SUBST_INIT],
-[config_appended_defs=; newline='
-'
-AC_CONFIG_COMMANDS([$config_file],
- [echo $config_appended_defs $config_file],
- [config_file=$config_file
-config_appended_defs=$config_appended_defs])])
-
-# GIT_ARG_SET_PATH(PROGRAM)
-# -
-# Provide --with-PROGRAM=PATH option to set PATH to PROGRAM
-# Optional second argument allows setting NO_PROGRAM=YesPlease if
-# --without-PROGRAM version used.
-AC_DEFUN([GIT_ARG_SET_PATH],
-[AC_ARG_WITH([$1],
-[AS_HELP_STRING([--with-$1=PATH],
-[provide PATH to $1])],
-[GIT_CONF_APPEND_PATH([$1], [$2])],
-[])])
-
-# GIT_CONF_APPEND_PATH(PROGRAM)
-# -
-# Parse --with-PROGRAM=PATH option to set PROGRAM_PATH=PATH
-# Used by GIT_ARG_SET_PATH(PROGRAM)
-# Optional second argument allows setting NO_PROGRAM=YesPlease if
-# --without-PROGRAM is used.
-AC_DEFUN([GIT_CONF_APPEND_PATH],
-[m4_pushdef([GIT_UC_PROGRAM], m4_toupper([$1]))dnl
-if test $withval = no; then
- if test -n $2; then
- GIT_UC_PROGRAM[]_PATH=$withval
- AC_MSG_NOTICE([Disabling use of
Re: [PATCH-v3] Allow git-filter-branch to process large repositories with lots of branches.
Lee Carver lee.car...@servicenow.com writes:
It is using the same ${tempdir} working directory that git rev-list uses
below for the ../revs file
Ah, I missed that; then that should be safe. The patch looks sane.
Can we have your sign-off, too, please?
It's normally .git-rewrite/t, following the normal working directory setup
near line 205.
case $filter_subdir in
)
@@ -268,7 +268,7 @@ case $filter_subdir in
esac
git rev-list --reverse --topo-order --default HEAD \
- --parents --simplify-merges $rev_args $@ ../revs ||
+ --parents --simplify-merges --stdin $@ ../parse ../revs ||
die Could not get the commits
commits=$(wc -l ../revs | tr -d )
--
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 v6 7/8] update-ref: support multiple simultaneous updates
On Wed, Sep 11, 2013 at 8:36 AM, Brad King brad.k...@kitware.com wrote:
On 09/10/2013 06:51 PM, Eric Sunshine wrote:
On Mon, Sep 9, 2013 at 8:57 PM, Brad King brad.k...@kitware.com wrote:
+Use 40 0 or the empty string to specify a zero value, except that
Did you want an 's' after the 0?
The same description without 's' already appears in git-update-ref.txt
above this location in the existing documentation of the command-line
Thanks for the explanation. (I could have checked the surrounding text
but didn't think to do so.)
option behavior. I see 0{40} in git-receive-pack.txt and also in
howto/update-hook-example.txt. Perhaps a follow-up change can be made
to choose a consistent way to describe 40 0s.
-Brad
--
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] git-compat-util: Avoid strcasecmp() being inlined
This is necessary so that read_mailmap() can obtain a pointer to the function. Signed-off-by: Sebastian Schuberth sschube...@gmail.com --- git-compat-util.h | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index be1c494..664305c 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -85,6 +85,13 @@ #define _NETBSD_SOURCE 1 #define _SGI_SOURCE 1 +#define __NO_INLINE__ /* do not inline strcasecmp() */ +#include string.h +#ifdef HAVE_STRINGS_H +#include strings.h /* for strcasecmp() */ +#endif +#undef __NO_INLINE__ + #ifdef WIN32 /* Both MinGW and MSVC */ #define _WIN32_WINNT 0x0502 #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ @@ -99,10 +106,6 @@ #include stddef.h #include stdlib.h #include stdarg.h -#include string.h -#ifdef HAVE_STRINGS_H -#include strings.h /* for strcasecmp() */ -#endif #include errno.h #include limits.h #ifdef NEEDS_SYS_PARAM_H -- 1.8.3.mingw.1.2.g56240b5.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
Re: [PATCH 20/21] show-index: acknowledge that it does not read .idx v3
On Wed, 11 Sep 2013, Nguyễn Thái Ngọc Duy wrote: show-index takes .idx from stdin while v3 requires the .pack. It's used for testing purposes only. Let those test scripts force .idx v2 with index-pack. Since I have a patch adding (partial) index v3 support to show-index in my tree, I've dropped this patch. Many tests use show-index only to manually count the number of objects and the added support in show-index is good enough for that. I've therefore reduced your next patch only to those tests where the actual list of object names is expected. Whether or not we'd wish to get rid of show-index completely at some point is a separate matter. Nicolas
Re: [PATCH 0/3] Fix MSVC compile errors and cleanup stat definitions
Karsten Blees karsten.bl...@gmail.com writes: A few minor fixes for the MSVC build. Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions Karsten Blees (3): MSVC: fix compile errors due to missing libintl.h MSVC: fix compile errors due to macro redefinitions MSVC: fix stat definition hell compat/mingw.h | 21 + compat/msvc.h| 15 --- config.mak.uname | 1 + 3 files changed, 18 insertions(+), 19 deletions(-) I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack. Thanks. -- 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] Windows: Do not redefine _WIN32_WINNT
With MinGW runtime version 4.0 this interferes with the previous definition from sdkddkver.h. Signed-off-by: Sebastian Schuberth sschube...@gmail.com --- compat/nedmalloc/malloc.c.h | 2 ++ git-compat-util.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h index 1401a67..930d03b 100644 --- a/compat/nedmalloc/malloc.c.h +++ b/compat/nedmalloc/malloc.c.h @@ -495,7 +495,9 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #endif /* WIN32 */ #ifdef WIN32 #define WIN32_LEAN_AND_MEAN +#ifndef _WIN32_WINNT #define _WIN32_WINNT 0x403 +#endif #include windows.h #define HAVE_MMAP 1 #define HAVE_MORECORE 0 diff --git a/git-compat-util.h b/git-compat-util.h index 664305c..f5c756d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -93,7 +93,9 @@ #undef __NO_INLINE__ #ifdef WIN32 /* Both MinGW and MSVC */ +#ifndef _WIN32_WINNT #define _WIN32_WINNT 0x0502 +#endif #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ #include winsock2.h #include windows.h -- 1.8.3.mingw.1.2.g56240b5.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
Re: [PATCH 3/3] MSVC: fix stat definition hell
On 11.09.2013 01:23, Karsten Blees wrote:
In msvc.h, there's a couple of stat related functions defined diffently
from mingw.h. When we remove these definitions, the only problem we get is
warning C4005: '_stati64' : macro redefinition for this line in mingw.h:
#define _stati64(x,y) mingw_stat(x,y)
I have a similar patch at [1] to fix similar compilation issues with MinGW
runtime version 4.0, which was recently released. I like your patch better, so
I've rebased mine on top of yours:
[PATCH] MinGW: Fix stat definitions to work with MinGW runtime version 4.0
For an overview of changes in mingwrt-4.0 see:
http://sourceforge.net/p/mingw/mingw-org-wsl/ci/4.0.0/tree/NEWS
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
compat/mingw.c | 1 -
compat/mingw.h | 9 +
config.mak.uname | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 96d9ac4..29c051f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -616,7 +616,6 @@ int mingw_stat(const char *file_name, struct stat *buf)
return do_stat_internal(1, file_name, buf);
}
-#undef fstat
int mingw_fstat(int fd, struct stat *buf)
{
HANDLE fh = (HANDLE)_get_osfhandle(fd);
diff --git a/compat/mingw.h b/compat/mingw.h
index b521900..0d2faac 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -278,11 +278,20 @@ static inline int getrlimit(int resource, struct rlimit
*rlp)
#define lseek _lseeki64
/* use struct stat with 64 bit st_size */
+#ifdef stat
+#undef stat
+#endif
#define stat _stati64
int mingw_lstat(const char *file_name, struct stat *buf);
int mingw_stat(const char *file_name, struct stat *buf);
int mingw_fstat(int fd, struct stat *buf);
+#ifdef fstat
+#undef fstat
+#endif
#define fstat mingw_fstat
+#ifdef lstat
+#undef lstat
+#endif
#define lstat mingw_lstat
#ifndef _stati64
diff --git a/config.mak.uname b/config.mak.uname
index 9249ee3..983ecc1 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -499,7 +499,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
NO_D_INO_IN_DIRENT = YesPlease
- COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/win32
+ COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI
-Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
--
1.8.3.mingw.1.2.g56240b5.dirty
I don't think we should squash this patch to yours, however, because yours
addressed MSVC compilation issues, and mine address MinGW compilation issues.
But my patch could go to your branch.
[1]
https://github.com/sschuberth/git/commit/841cdf60faa134eef031a7cf6d6692473a18cb65
--
Sebastian Schuberth
--
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 0/3] Fix MSVC compile errors and cleanup stat definitions
On Wed, Sep 11, 2013 at 6:09 PM, Junio C Hamano gits...@pobox.com wrote: Karsten Blees karsten.bl...@gmail.com writes: A few minor fixes for the MSVC build. Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions Karsten Blees (3): MSVC: fix compile errors due to missing libintl.h MSVC: fix compile errors due to macro redefinitions MSVC: fix stat definition hell compat/mingw.h | 21 + compat/msvc.h| 15 --- config.mak.uname | 1 + 3 files changed, 18 insertions(+), 19 deletions(-) I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack. Acked-by: Sebastian Schuberth sschube...@gmail.com -- Sebastian Schuberth -- 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 00/21] np/pack-v4 updates
On Wed, 11 Sep 2013, Duy Nguyen wrote: Nico, if you have time you may want to look into this. The result v4 pack from pack-objects on git.git for me is 35MB (one branch) while packv4-create produces 30MB (v2 is 40MB). I don't know why there is such a big difference in size. I compared. Ident dict is identical. Tree dict is a bit different (some that have same hits are ordered differently). Delta chains do not differ much. Many groups of entries in the pack are displaced though. I guess I turned a wrong knob or something in pack-objects in v4 code.. Will try to have a closer look. Thanks for your dedication so far. Nicolas -- 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 00/21] np/pack-v4 updates
On Wed, 11 Sep 2013, Nguyễn Thái Ngọc Duy wrote: This contains fixups for some of my patches, some of Nico's, adds v4 support to unpack-objects because the test suite needs it. With these, when force generating pack v4 unconditionally, the remaining failed tests are: [...] @junio: I've folded those patches into my branch, along with the better fix for the compilation issue you found. So you may simply replace the branch you have in pu with mine directly if you wish. git://git.linaro.org/people/nico/git Nicolas
Re: [PATCH 1/2] reset: handle submodule with trailing slash
Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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] Disabling status hints in COMMIT_EDITMSG
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: But at the same time, I feel that these redundant lines, especially the latter one, would give the users a stronger cue than just saying that bar is Untracked; do X to include reminds that bar will not be included if nothing is done. The one which draw my attention was (use git commit to conclude merge) which is particularly counter-productive when you are already doing a git commit. Oh, no question about that. Nobody would object to the removal of that one; it is clearly nonsense. I was commented on the value of keeping hints like this: # Untracked files: # (use git add file... to include in what will be committed) The primary value of the hint in the context of commit message buffer *NOT* being what exactly do I need to do after I abort this commit?, but being Ahh, this 'Untracked' section is showing me files that I may have forgotten to 'git add'. If new users do not benefit from the latter, I am perfectly fine with the removal, but I suspect it may not be the case, hence my earlier comment. And the user can see these hints by running another 'git status' after aborting the commit anyway is an irrelevant counter-argument, exactly because my point is that I suspect having them in the commit template comment may help the users to *decide* if they want to continue with or abort the commit. But as I said already, I do not have a strong preference either way. Will queue the two patches (but I see there are already some obvious fixes suggested). Thanks. -- 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-Transmission of blobs?
Josef Wolf j...@raven.inka.de writes: On Di, Sep 10, 2013 at 10:51:02 -0700, Junio C Hamano wrote: Consider this simple history with only a handful of commits (as usual, time flows from left to right): E / A---B---C---D where D is at the tip of the sending side, E is at the tip of the receiving side. The exchange goes roughly like this: (receiving side): what do you have? (sending side): my tip is at D. (receiving side): D? I've never heard of it --- please give it to me. I have E. At this point, why would the receiving side not tell all the heads it knows about? It did. The receiving end had only one branch whose tip is E. It may have a tracking branch that knows where the tip of the sending end used to be when it forked (which is C), so the above may say I have E and C. It actually would say I have B and A and ... for a bounded number of commits, but that does not fundamentally change the picture---the important point is it is bounded and there is a horizon. There are some work being done to optimize this further using various techniques, but they are not ready yet. And this still stands. -- 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] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? Duy's suggestion below is necessary to avoid this then I think - we'll die if the user has a corrupt index and gives a path with a trailing slash, but without that path we won't try to load the index. But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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] Documentation/git-checkout: Move `--detach` flag in synopsis to correct command
Benjamin Bergman b...@benbergman.ca writes: From a33659535cb0eac92bed42d5e494dbb8f5d9ab20 Mon Sep 17 00:00:00 2001 From: Benjamin Bergman b...@benbergman.ca Date: Tue, 10 Sep 2013 16:00:29 -0500 Subject: [PATCH] Documentation/git-checkout: Move `--detach` flag in synopsis to correct command These (except the first one that is a separator in the format-patch output) go to your e-mail headers, not to the body of the message. Detailed description of `--detach` states that it is default for `commit` but needs to be specified for `branch`. The old man page synopsis showed the reverse. --- Please sign-off your patch. -'git checkout' [-q] [-f] [-m] [branch] -'git checkout' [-q] [-f] [-m] [--detach] [commit] +'git checkout' [-q] [-f] [-m] [--detach] [branch] +'git checkout' [-q] [-f] [-m] [commit] I am actually on the fence on this change. The original shows two operations that do different things (one goes to a named branch, the other goes to the state in which the HEAD is detached at the named commit). With the updated synopsis, even with the first form, if the user uses --detach, the end result is like the second one. I personally find the synopsis with the patch a bit more confusing, not less. Can we update the second form (without touching the one that shows how to check out a named branch at all) in a different way to clarify? E.g. 'git checkout' [-q] [-f] [-m] [commit | --detach branch] might be one way. If you want to detach at a named commit, you can directly give the commit object name; a branch name usually can be used as a commit object name, but that use conflicts with the more usual check out that branch usage, so you need to give --detach explicitly if you use a branch name to name that commit you want to detach at. One drawback this has is that it does not capture that you could say --detach (even though you do not have to) when using a form that is _not_ a branch name to name your commit, e.g. git checkout --detach master^0 ... thinks a bit more ... Or we can split the detaching usage into two, which is how the DESCRIPTION section does. I.e. 'git checkout' [-q] [-f] [-m] [branch] -'git checkout' [-q] [-f] [-m] --detach [commit] +'git checkout' [-q] [-f] [-m] --detach [branch] +'git checkout' [-q] [-f] [-m] commit 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] new_branch] [st... This would tell the reader that: - You can give --detach and no branch name---you would detach at the tip of your current branch; - You can give --detach and a branch name; - You can give a commit object name. We could also throw in [--detach] in front of the commit to say that it is accepted (even though it is not necessary. 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] new_branch] [start_point] 'git checkout' [-f|--ours|--theirs|-m|--conflict=style] [tree-ish] [--] paths... Your MUA or editor line-wrapped the patch; please do not let them. 'git checkout' [-p|--patch] [tree-ish] [--] [paths...] How about doing it this way? -- 8 -- Subject: checkout: update synopsys and documentation on detaching HEAD In the synopsis, the second form to detach HEAD at the named commit labelled the argument as 'commit'. While this is technically more correct, because the feature to detach is not limited to the tip of a named branch, it was found confusing and did not express the fact that you have to give `--detach` if you are naming the commit you want to detach HEAD at with a branch name. Separate this case into two syntactical forms, mimicking the way how the DESCRIPTION section shows this usage. Also update the text that explains the syntax to name the commit to detach HEAD at to clarify. Suggested-by: Benjamin Bergman b...@benbergman.ca Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-checkout.txt | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ca118ac..01d9b85 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -9,7 +9,8 @@ SYNOPSIS [verse] 'git checkout' [-q] [-f] [-m] [branch] -'git checkout' [-q] [-f] [-m] [--detach] [commit] +'git checkout' [-q] [-f] [-m] --detach [branch] +'git checkout' [-q] [-f] [-m] [--detach] commit 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] new_branch] [start_point] 'git checkout' [-f|--ours|--theirs|-m|--conflict=style] [tree-ish] [--] paths... 'git checkout' [-p|--patch] [tree-ish] [--] [paths...] @@ -62,7 +63,7 @@ that is to say, the branch is not reset/created unless git checkout is successful. 'git checkout' --detach [branch]:: -'git checkout' commit:: +'git checkout' [--detach] commit:: Prepare to work on top of commit, by detaching HEAD at it (see DETACHED HEAD section), and updating the index and the @@ -71,10 +72,13 @@ successful. tree will be the state recorded in the commit plus
Re: breakage in revision traversal with pathspec
On 11/09/2013 01:23, Junio C Hamano wrote: Kevin Bracey ke...@bracey.fi writes: On 10/09/2013 20:19, Junio C Hamano wrote: This command $ git log v1.8.3.1..v1.8.4 -- git-cvsserver.perl reports that a merge 766f0f8ef7 (which did not touch the specified path at all) touches it. Bisecting points at d0af663e (revision.c: Make --full-history consider more merges, 2013-05-16). That merge appearing *with* --full-history would seem like correct behaviour to me. Or at least it's what I intended. ... But it shouldn't appear if the user does not ask for --full-history. Well, there is a functioning semi-work-around for now: avoid difficult non-linear questions like v1.8.3.1..v1.8.4. A question like v1.8.3..v1.8.4 is a lot easier to visualise, and it does already omit the merge. On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x In effect, we do have a linear section of history to follow, and the file does change in the middle of that line. It may be quite hard to come up with a solid rule to hide the merge that doesn't go wrong somewhere else. The current rules for this are 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. For this query, the mainline link to 1.8.3 is the uninteresting side branch! If you do specify v1.8.3..v1.8.4, then v1.8.3 becomes on-graph thanks to other new rules, and this rule does kick in, hiding the merge. 2) If rule 1 doesn't activate, and it remains as a merge, hide it if treesame to all on-graph parents. Previously this rule was hide if treesame to any parent, and so that would have hidden the merge. Now, when I changed rule 2, I did not think this would affect the default log. See my commit message: Now redefine a commit's TREESAME flag to be true only if a commit is TREESAME to _all_ of its [later: on-graph] parent. This doesn't affect ... the default simplify_history behaviour (because partially TREESAME merges are turned into normal commits)... Whoops - partially TREESAME merges are not always turned into normal commits. Maybe the fix is to define TREESAME differently for simplify_history - to use the old definition of identical to any parent in that case. I'm not sure that's right though. I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Kevin -- 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] Disabling status hints in COMMIT_EDITMSG
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: But at the same time, I feel that these redundant lines, especially the latter one, would give the users a stronger cue than just saying that bar is Untracked; do X to include reminds that bar will not be included if nothing is done. The one which draw my attention was (use git commit to conclude merge) which is particularly counter-productive when you are already doing a git commit. Oh, no question about that. Nobody would object to the removal of that one; it is clearly nonsense. I was commented on the value of keeping hints like this: # Untracked files: # (use git add file... to include in what will be committed) Yes, I understood your argument. I have no strong opinion on whether they should be removed either, but I went for the removal essentially because it keeps the code simple. If we want to keep the advices, and if we want them to be really sound, then for example the advice for Changes to be committed: should be changed when running git commit --amend (we currently hint git reset even for files which are not in the index in this case). Same for --only/--include. So, giving accurate hints in all cases seems non-trivial. I think the value of these messages is smaller than the potential confusion and/or the code complexity to select and possibly modify the hints. -- 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: [PATCH v2] configure.ac: move the private git m4 macros to a dedicated directory
Hi Elia. Sorry, but I have to give my NAK to this patch. On 09/11/2013 04:46 PM, Elia Pinto wrote: Git use, as many project that use autoconf, private m4 macros. When not using automake, and just relying on autoconf, the macro files are not picked up by default. A possibility, as git do today, is to put the private m4 macro in the configure.ac file, so they will copied over the final configure when calling autoreconf(that call also autoconf). But this makes configure.ac difficult to read and maintain, especially if you want to introduce new macros later. By separating the definitions of the macros from configure.ac file the build system would be more modular. In which sense are we being more modular exactly? After all: - the configure.ac of Git is the only user of these macros, - using m4_include doesn't offer any performance improvement, and - m4 doesn't offer any namespace granularity anyway. So it seems to me that this patch only adds extra indirections without adding any real benefit. Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR to declare where additional macro files are to be put and found by aclocal. The argument passed to this macro is commonly m4. Despite the documentation, autoconf do nothing with it, only aclocal can use directly if invoked by -I m4 or indirectly using automake. But autoreconf don't invoke aclocal in this way. So in summary you can not use this macro in a useful way if you only use autoconf, as git does. Another historical possibility is to list all your macros in acinclude.m4. This file will be included in aclocal.m4 when you run aclocal, and its macro(s) will henceforth be visible to autoconf. However if it contains numerous macros, it will rapidly become difficult to maintain, and for git this don't provide any benefits or very little. The actual autotool documentation recommend to write each macro in its own file and gather all these files in a separate directory. Where exactly id you find that recommendation? If the autotools docs tell to do so *unconditionally*, they are wrong and should be fixed. In fact, even the configure.ac from Automake itself keeps definition of private macros in configure.ac... Given the limitations i mentioned earlier, the only possibility is to use the m4_include for including every macro file. The m4_include directive works quite like the #include directive of the C programming language, and simply copies over the content of the file(s). Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- This is a second version of this patch http://article.gmane.org/gmane.comp.version-control.git/231984. The first was plain wrong, my bad. I am sorry for the long delay. Sure it is something low-hanging fruit configure.ac | 148 +++-- m4/git_arg_set_path.m4| 14 m4/git_check_func.m4 | 13 m4/git_conf_append_path.m4| 30 m4/git_conf_subst.m4 | 10 +++ m4/git_conf_subst_init.m4 | 15 m4/git_parse_with.m4 | 22 ++ m4/git_parse_with_set_make_var.m4 | 20 + m4/git_stash_flags.m4 | 15 m4/git_unstash_flags.m4 | 13 10 files changed, 162 insertions(+), 138 deletions(-) create mode 100644 m4/git_arg_set_path.m4 create mode 100644 m4/git_check_func.m4 create mode 100644 m4/git_conf_append_path.m4 create mode 100644 m4/git_conf_subst.m4 create mode 100644 m4/git_conf_subst_init.m4 create mode 100644 m4/git_parse_with.m4 create mode 100644 m4/git_parse_with_set_make_var.m4 create mode 100644 m4/git_stash_flags.m4 create mode 100644 m4/git_unstash_flags.m4 [SNIP] Regards, Stefano -- 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: Git tag output order is incorrect (IMHO)
Phil Hord phil.h...@gmail.com writes: Someone at $work asked me this week how to find the current and previous tags on his branch so he could generate release notes. I just need last two tags on head in topo-order. I was surprised by how complicated this turned out to be. I ended up with this: git log --decorate=full --pretty=format:'%d' HEAD | sed -n -e 's-^.* refs/tags/\(.*\)[ )].*$-\1-p' | head -2 Surely there's a cleaner way, right? That looks clean enough (I would have used head -n 2 though) and in line with the way how you can exercise the flexibility of the system, at least to me ;-). Joking aside, I agree that a --merged X primitive, i.e. what refs can be reachable from commit X?, in the listing mode of git tag or git for-each-ref would have helped. As the sorting and formatting primitives are already there in for-each-ref, it would have been git for-each-ref \ --format='%(refname:short)' \ --sort='-*committerdate' \ --count=2 \ --merged my-branch \ refs/tags/ or something like that. -- 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 0/3] Fix MSVC compile errors and cleanup stat definitions
Sebastian Schuberth sschube...@gmail.com writes: On Wed, Sep 11, 2013 at 6:09 PM, Junio C Hamano gits...@pobox.com wrote: Karsten Blees karsten.bl...@gmail.com writes: A few minor fixes for the MSVC build. Also here: https://github.com/kblees/git/tree/kb/fix-msvc-stat-definitions Karsten Blees (3): MSVC: fix compile errors due to missing libintl.h MSVC: fix compile errors due to macro redefinitions MSVC: fix stat definition hell compat/mingw.h | 21 + compat/msvc.h| 15 --- config.mak.uname | 1 + 3 files changed, 18 insertions(+), 19 deletions(-) I won't be a good person to review, suggest improvements on, and/or judge these patches. I'm Cc'ing regulars who work on mingw port for their help and Ack. Acked-by: Sebastian Schuberth sschube...@gmail.com Thanks; will queue these three and your follow-up on top. -- 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] reset: handle submodule with trailing slash
John Keeping j...@keeping.me.uk writes: On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? Duy's suggestion below is necessary to avoid this then I think - we'll die if the user has a corrupt index and gives a path with a trailing slash, but without that path we won't try to load the index. Sorry, but I don't quite follow. Isn't git reset --soft path a nonsense, with or without a trailing slash at the end of path? But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 11:14:57AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? Duy's suggestion below is necessary to avoid this then I think - we'll die if the user has a corrupt index and gives a path with a trailing slash, but without that path we won't try to load the index. Sorry, but I don't quite follow. Isn't git reset --soft path a nonsense, with or without a trailing slash at the end of path? Yes, but my point was that providing the user doesn't give a path with a trailing slash we will get past the option parser if we take the approach below. However, I think we do do a read_cache when using reset --soft since we go through builtin/reset.c::die_if_unmerged_cache() which dies if read_cache fails. So I don't think we are losing anything by moving this check earlier. But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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: breakage in revision traversal with pathspec
Kevin Bracey wrote: On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x [...] 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. [...] 2) If rule 1 doesn't activate, and it remains as a merge, hide it if treesame to all on-graph parents. Previously this rule was hide if treesame to any parent, and so that would have hidden the merge. Now, when I changed rule 2, I did not think this would affect the default log. See my commit message: [...] I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Thanks for this analysis. Interesting. The rule (1) comes from v1.3.0-rc1~13^2~6: commit f3219fbbba32b5100430c17468524b776eb869d6 Author: Junio C Hamano jun...@cox.net Date: Fri Mar 10 21:59:37 2006 -0800 try_to_simplify_commit(): do not skip inspecting tree change at boundary. When git-rev-list (and git-log) collapsed ancestry chain to commits that touch specified paths, we failed to inspect and notice tree changes when we are about to hit uninteresting parent. This resulted in git rev-list since.. -- file to always show the child commit after the lower bound, even if it does not touch the file. This commit fixes it. Thanks for Catalin for reporting this. See also: 461cf59f8924f174d7a0dcc3d77f576d93ed29a4 Signed-off-by: Junio C Hamano jun...@cox.net I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. 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: [PATCH] Windows: Do not redefine _WIN32_WINNT
Sebastian Schuberth sschube...@gmail.com writes: diff --git a/git-compat-util.h b/git-compat-util.h index 664305c..f5c756d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -93,7 +93,9 @@ #undef __NO_INLINE__ #ifdef WIN32 /* Both MinGW and MSVC */ +#ifndef _WIN32_WINNT #define _WIN32_WINNT 0x0502 +#endif #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ #include winsock2.h #include windows.h This unfortunately does not seem to match what I have. I think the patch is based on the codebase before these two: 380395d0 (mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE, 2013-05-02) 41f29991 (msvc: Fix compilation errors caused by poll.h emulation, 2013-01-31) I could of course wiggle it in, if you want, but I wanted to know what is going on. Is it a pre-release freeze period on your side or something? -- 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-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth wrote: This is necessary so that read_mailmap() can obtain a pointer to the function. Hm, what platform has strcasecmp() as an inline function? Is this allowed by POSIX? Even if it isn't, should we perhaps just work around it by providing our own thin static function wrapper in mailmap.c? Curious, 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: [PATCH 1/2] wt-status: turn advice_status_hints into a field of wt_status
On Wed, Sep 11, 2013 at 11:08:58AM +0200, Matthieu Moy wrote:
No behavior change in this patch, but this makes the display of status
hints more flexible as they can be enabled or disabled for individual
calls to commit.c:run_status().
[...]
+static void status_finalize(struct wt_status *s)
+{
+ determine_whence(s);
+ s-hints = advice_status_hints;
+}
Is a finalize the right place to put the status hint tweak? I would
expect the order for callers to be:
wt_status_prepare(s);
/* manual tweaks of fields in s */
wt_status_finalize(s);
but the finalize would overwrite any tweak of the hints field. So it
would make more sense to me for it to go in prepare().
But the current callsites look like this:
@@ -1249,7 +1255,7 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
wt_status_prepare(s);
gitmodules_config();
git_config(git_status_config, s);
- determine_whence(s);
+ status_finalize(s);
argc = parse_options(argc, argv, prefix,
builtin_status_options,
builtin_status_usage, 0);
We do not actually read the config until after _prepare, because the
config is what is doing the tweaking of s in this case. But we cannot
trust advice_* until we have read the config.
The problem is that we are doing two things in that git_config call:
1. Loading basic git-wide core config.
2. Priming the wt_status struct with options specific to git status
So the cleanest thing would be something like:
/* load basic config */
git_config(git_diff_ui_config, NULL);
/* initialize the status-run struct; this would probably be better named as
* _init to match the rest of the code */
wt_status_prepare(s);
/* now tweak the defaults using status-specific config, which does
* not need to chain to other config callbacks anymore */
git_config(git_status_config, s);
/* and then tweak further with command line options */
argc = parse_options(...);
/* and now finally ask wt-status to finalize any setup we've put into
the struct (e.g., calling determine_whence, though I do not
actually see it depending on any of the fields we set. Should it
be part of _prepare? */
wt_status_finalize(s);
That would follow our more usual object init-tweak-finalize-use
patterns. Hrm. To make matters more complicated, we have
finalize_deferred_config, too. I think that could be rolled into
wt_status_finalize.
Perhaps that is getting a bit complicated as a refactor. If you don't
want to do it, I understand, but I think you should probably avoid the
name _finalize here, as it is a bit mis-leading.
-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] git-compat-util: Avoid strcasecmp() being inlined
Sebastian Schuberth sschube...@gmail.com writes: This is necessary so that read_mailmap() can obtain a pointer to the function. Whoa, I didn't think it is even legal for a C library to supply strcmp() or strcasecmp() that are purely inline you cannot take the address of. The solution looks a bit too large a hammer that affects everybody, not just those who have such a set of header files. +#define __NO_INLINE__ /* do not inline strcasecmp() */ +#include string.h +#ifdef HAVE_STRINGS_H +#include strings.h /* for strcasecmp() */ +#endif +#undef __NO_INLINE__ + #ifdef WIN32 /* Both MinGW and MSVC */ #define _WIN32_WINNT 0x0502 #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ @@ -99,10 +106,6 @@ #include stddef.h #include stdlib.h #include stdarg.h -#include string.h -#ifdef HAVE_STRINGS_H -#include strings.h /* for strcasecmp() */ -#endif #include errno.h #include limits.h #ifdef NEEDS_SYS_PARAM_H -- 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] Documentation/git-checkout: Move `--detach` flag in synopsis to correct command
Hi,
Junio C Hamano wrote:
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -9,7 +9,8 @@ SYNOPSIS
[verse]
'git checkout' [-q] [-f] [-m] [branch]
-'git checkout' [-q] [-f] [-m] [--detach] [commit]
+'git checkout' [-q] [-f] [-m] --detach [branch]
+'git checkout' [-q] [-f] [-m] [--detach] commit
'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] new_branch] [start_point]
'git checkout' [-f|--ours|--theirs|-m|--conflict=style] [tree-ish] [--]
paths...
'git checkout' [-p|--patch] [tree-ish] [--] [paths...]
@@ -62,7 +63,7 @@ that is to say, the branch is not reset/created unless git
checkout is
successful.
'git checkout' --detach [branch]::
-'git checkout' commit::
+'git checkout' [--detach] commit::
Looks sensible.
[...]
@@ -71,10 +72,13 @@ successful.
tree will be the state recorded in the commit plus the local
modifications.
+
-Passing `--detach` forces this behavior in the case of a branch (without
-the option, giving a branch name to the command would check out the branch,
-instead of detaching HEAD at it), or the current commit,
-if no branch is specified.
+Even though a branch name can be used to name a commit, you have to
+explicitly say `git checkout --detach branch` when you want to
+detach HEAD at the tip of the branch (`git checkout branch` will
+check out that branch without detaching HEAD). Omitting branch,
+i.e. `git checkout --detach` detaches HEAD at the tip of the current
+branch. When naming the commit in a form other than just a branch
+name, e.g. `master^0`, `HEAD~4`, `c2f3bf071e`, you can omit --detach.
Hm. I agree that the old explanation is overly convoluted, but I don't
think the replacement is clear enough yet. The Even though a branch
name can be used to name a commit, part forced me to pause for too
long --- why is this telling me that a branch can be used to name a
commit, and in what context?
I think the main problem with the old text is that it tried to say too
much in one sentence.
The explanation lower down of the --detach option does this rather
well:
--detach
Rather than checking out a branch to work on it, check
out a commit for inspection and discardable
experiments. This is the default behavior of
git checkout commit when commit is not a branch
name. See the DETACHED HEAD section below for
details.
How about splitting this into multiple paragraphs, like so? In the
suggestion below I also cleaned up the language a little.
git checkout --detach [branch], git checkout [--detach] commit
Prepare to work on top of commit, by detaching [...]
When the commit argument is a branch name, the --detach
option can be used to detach HEAD at the tip of the
branch ('git checkout branch' would check out that
branch without detaching HEAD).
Omitting branch detaches HEAD at the tip of the
current branch.
I'd leave out the last sentence about commits other than branch names,
since it is already implied by the [--detach] in the syntax.
Thanks and hope that helps,
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: [PATCH 1/2] reset: handle submodule with trailing slash
John Keeping j...@keeping.me.uk writes: However, I think we do do a read_cache when using reset --soft since we go through builtin/reset.c::die_if_unmerged_cache() which dies if read_cache fails. So I don't think we are losing anything by moving this check earlier. Thanks. -- 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: breakage in revision traversal with pathspec
Jonathan Nieder jrnie...@gmail.com writes: I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. Sounds sensible. Thanks. -- 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] Improve documentation concerning the status.submodulesummary setting
'git status' and 'git commit' can be told to also show the output of git submodule summary by setting the status.submodulesummary config option. But status and commit also honor the diff.ignoreSubmodules and the submodule.name.ignore settings, which then disable the summary partly or completely. This - and the fact that the last two settings do not affect the git submodule commands at all - is not well documented. Extend the documentation in those places where status.submodulesummary, diff.ignoreSubmodules and submodule.name.ignore are described to better explain these dependencies. Thanks-to: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Am 10.09.2013 18:27, schrieb Jens Lehmann: Am 10.09.2013 00:53, schrieb Junio C Hamano: * bc/submodule-status-ignored (2013-09-04) 2 commits - submodule: don't print status output with ignore=all - submodule: fix confusing variable name Originally merged to 'next' on 2013-08-22 Will merge to 'next'. I propose to cook this some time in next to give submodule users who have configured ignore=all the opportunity to test and comment on that. And as Matthieu noticed the documentation is not terribly clear here, I'll prepare one or two patches to fix that which should go in together with these changes. And here we go. Matthieu, does that make it more obvious? Documentation/config.txt | 12 ++-- Documentation/diff-config.txt | 6 +- Documentation/git-status.txt | 8 +++- Documentation/gitmodules.txt | 3 ++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 44e7873..52f20ab 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2207,7 +2207,14 @@ status.submodulesummary:: If this is set to a non zero number or true (identical to -1 or an unlimited number), the submodule summary will be enabled and a summary of commits for modified submodules will be shown (see - --summary-limit option of linkgit:git-submodule[1]). + --summary-limit option of linkgit:git-submodule[1]). Please note + that the summary output command will be suppressed for all + submodules when `diff.ignoreSubmodules` is set to 'all' or only + for those submodules where `submodule.name.ignore=all`. To + also view the summary for ignored submodules you can either use + the --ignore-submodules=dirty command line option or the 'git + submodule summary' command, which shows a similar output but does + not honor these settings. submodule.name.path:: submodule.name.url:: @@ -2242,7 +2249,8 @@ submodule.name.ignore:: submodules that have untracked files in their work tree as changed. This setting overrides any setting made in .gitmodules for this submodule, both settings can be overridden on the command line by using the - --ignore-submodules option. + --ignore-submodules option. The 'git submodule' commands are not + affected by this setting. tar.umask:: This variable can be used to restrict the permission bits of diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index ac77050..223b931 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -73,7 +73,11 @@ diff.ignoreSubmodules:: Sets the default value of --ignore-submodules. Note that this affects only 'git diff' Porcelain, and not lower level 'diff' commands such as 'git diff-files'. 'git checkout' also honors - this setting when reporting uncommitted changes. + this setting when reporting uncommitted changes. Setting it to + 'all' disables the submodule summary normally shown by 'git commit' + and 'git status' when 'status.submodulesummary' is set unless it is + overridden by using the --ignore-submodules command line option. + The 'git submodule' commands are not affected by this setting. diff.mnemonicprefix:: If set, 'git diff' uses a prefix pair that is different from the diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 9046df9..a4acaa0 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -210,7 +210,13 @@ directory. If `status.submodulesummary` is set to a non zero number or true (identical to -1 or an unlimited number), the submodule summary will be enabled for the long format and a summary of commits for modified submodules will be -shown (see --summary-limit option of linkgit:git-submodule[1]). +shown (see --summary-limit option of linkgit:git-submodule[1]). Please note +that the summary output from the status command will be suppressed for all +submodules when `diff.ignoreSubmodules` is set to 'all' or only for those +submodules where `submodule.name.ignore=all`. To also view the summary for +ignored submodules you can either use the --ignore-submodules=dirty
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Wed, Sep 11, 2013 at 11:29:21AM -0700, Jonathan Nieder wrote: Sebastian Schuberth wrote: This is necessary so that read_mailmap() can obtain a pointer to the function. Hm, what platform has strcasecmp() as an inline function? Is this allowed by POSIX? Even if it isn't, should we perhaps just work around it by providing our own thin static function wrapper in mailmap.c? Environments can implement library functions as macros or even intrinsics, but C99 requires that they still allow you to access a function pointer. And if my reading of C99 6.7.4 is correct, it should apply to inlines, too, because you should always be able to take the address of an inline function (though it is a little subtle). But that does not mean there are not popular platforms that we do not have to workaround (and the inline keyword is C99 anyway, so all bets are off for pre-C99 inline implementations). I would prefer the static wrapper solution you suggest, though. It leaves the compiler free to optimize the common case of normal strcasecmp calls, and only introduces an extra function indirection when using it as a callback (and even then, if we can inline the strcasecmp, it still ends up as a single function call). The downside is that it has to be remembered at each site that uses strcasecmp, but we do not use pointers to standard library functions very often. -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] lookup_object: remove hashtable_index() and optimize hash_obj()
On Tue, Sep 10, 2013 at 06:17:12PM -0400, Nicolas Pitre wrote:
hashtable_index() appears to be a close duplicate of hash_obj().
Keep only the later and make it usable for all cases.
Thanks. This duplication has often bugged me when looking at that
hash table, but I just never actually wrote the patch.
Also remove the modulus as this is an expansive operation.
The size argument is always a power of 2 anyway, so a simple
mask operation provides the same result.
On a 'git rev-list --all --objects' run this decreased the time spent
in lookup_object from 27.5% to 24.1%.
Nice. This is a tiny bit subtle, though, as the power-of-2 growth
happens elsewhere, and we may want to tweak it later (the decorate.c
hash, for example, grows by 3/2).
Maybe it's worth squashing in one or both of the comments below as a
warning to anybody who tries to tweak it.
---
diff --git a/object.c b/object.c
index e2dae22..5f792cb 100644
--- a/object.c
+++ b/object.c
@@ -47,6 +47,7 @@ static unsigned int hash_obj(const unsigned char *sha1,
unsigned int n)
{
unsigned int hash;
memcpy(hash, sha1, sizeof(unsigned int));
+ /* Assumes power-of-2 hash sizes in grow_object_hash */
return hash (n - 1);
}
@@ -94,6 +95,10 @@ static void grow_object_hash(void)
static void grow_object_hash(void)
{
int i;
+ /*
+* Note that this size must always be power-of-2 to match hash_obj
+* above.
+*/
int new_hash_size = obj_hash_size 32 ? 32 : 2 * obj_hash_size;
struct object **new_hash;
--
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] Windows: Do not redefine _WIN32_WINNT
On Wed, Sep 11, 2013 at 8:29 PM, Junio C Hamano gits...@pobox.com wrote: This unfortunately does not seem to match what I have. I think the patch is based on the codebase before these two: 380395d0 (mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE, 2013-05-02) 41f29991 (msvc: Fix compilation errors caused by poll.h emulation, 2013-01-31) I could of course wiggle it in, if you want, but I wanted to know what is going on. Is it a pre-release freeze period on your side or something? That's right, I currently have a code freeze at Git 1.8.3 because I need to solve several other issues with Git 1.8.4 on Windows first. I'd be grateful if you could wiggle it in. -- Sebastian Schuberth -- 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: breakage in revision traversal with pathspec
On 11/09/2013 21:24, Jonathan Nieder wrote: Kevin Bracey wrote: On reflection I'm not sure what we should for the simple history view of v1.8.3.1..v1.8.4. We're not rewriting parents, so we don't get a chance to reconsider the merge as being zero-parent, and we do have this little section of graph to traverse at the bottom: 1.8.3 oxxxx---x--- (x = included, o = excluded, *=!treesame) / /* o--x--x--x--x [...] 1) if identical to any on-graph parent, follow that one, and rewrite the merge as a non-merge. We currently do not follow to an identical off-graph parent. This long-standing comment in try_to_simplify_commit applies: Even if a merge with an uninteresting side branch brought the entire change we are interested in, we do not want to lose the other branches of this merge, so we just keep going. [...] I currently feel instinctively more disposed to dropping the older don't follow off-graph identical parents rule. Let the default history go straight to v1.8.3 even though it goes off the graph, stopping us traversing the topic branch. Thanks for this analysis. Interesting. The rule (1) comes from v1.3.0-rc1~13^2~6: ... I think you're right that dropping the don't follow off-graph treesame parents rule would be a sensible change. The usual point of the follow the treesame parent rule is to avoid drawing undue attention to merges of ancient history where some of the parents are side-branches with an old version of the files being tracked and did not actually change those files. That rationale applies just as much for a merge on top of an UNINTERESTING rev as any other merge. I agree about the rationale still applying - why not follow off-graph, unless you're doing --ancestry-path? (Fortunately ancestry_path already disables simplify_history). That makes more sense if you try to ignore the misleading comment. In a typical v1..v3 range, the temporal limiting means that it's paths to the mainline that will tend to be marked UNINTERESTING, not to the topic branches... But I can imagine going off graph it may previously have tripped up other parts of the code. It could be that this Git 1.3.0 rule ended up covering over some of the older merge hiding logic flakiness. Maybe it's no longer necessary. I'll do some experiments. Now, one bit of news - I have just figured out why gitk is behaving differently. It transforms .. before it reaches git. To see the effect at the command line: git log v1.8.3..v.1.8.4 hides the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new example of a dotty shorthand not being exactly equivalent. In the .. case the v1.8.3 tag gets peeled before being sent to add_rev_cmdline , and the mark bottom commits logic works. But in the ^ case, the v1.8.3 doesn't get peeled. Junio - any thoughts on the correct place to fix that? (And gitk actually does ^tag-sha, just to be odd, so that needs to be handled too). Should these things be peeled in revs-cmdline or not? We should be consistent. Kevin -- 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] Improve documentation concerning the status.submodulesummary setting
Jens Lehmann jens.lehm...@web.de writes: And here we go. Matthieu, does that make it more obvious? Seems OK. -- 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: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder jrnie...@gmail.com wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is this
allowed by POSIX? Even if it isn't, should we perhaps just work
around it by providing our own thin static function wrapper in
mailmap.c?
I'm on Windows using MSYS / MinGW. Since MinGW runtime version 4.0,
string.h contains the following code (see [1]):
#ifndef __NO_INLINE__
__CRT_INLINE int __cdecl __MINGW_NOTHROW
strncasecmp (const char * __sz1, const char * __sz2, size_t __sizeMaxCompare)
{return _strnicmp (__sz1, __sz2, __sizeMaxCompare);}
#else
#define strncasecmp _strnicmp
#endif
[1]
http://sourceforge.net/p/mingw/mingw-org-wsl/ci/master/tree/include/string.h#l107
--
Sebastian Schuberth
--
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 v2] configure.ac: move the private git m4 macros to a dedicated directory
2013/9/11 Stefano Lattarini stefano.lattar...@gmail.com: Hi Elia. Sorry, but I have to give my NAK to this patch. I hold in great consideration the comments of Stephen in this area. On 09/11/2013 04:46 PM, Elia Pinto wrote: Git use, as many project that use autoconf, private m4 macros. When not using automake, and just relying on autoconf, the macro files are not picked up by default. A possibility, as git do today, is to put the private m4 macro in the configure.ac file, so they will copied over the final configure when calling autoreconf(that call also autoconf). But this makes configure.ac difficult to read and maintain, especially if you want to introduce new macros later. By separating the definitions of the macros from configure.ac file the build system would be more modular. In which sense are we being more modular exactly? After all: - the configure.ac of Git is the only user of these macros, - using m4_include doesn't offer any performance improvement, and - m4 doesn't offer any namespace granularity anyway. So it seems to me that this patch only adds extra indirections without adding any real benefit. Having a big #include have the same effect di reduced modularization, larger coupling. Don't splitting header in include in general is not a good thing. If you introduce new macros and start using it in configure.ac in the same commit it becomes more apparent the significance of their use. less probability of mistake, ecc. But it is my opinion. Performarce wasn't my goal. Starting from version 2.58, autoconf provide the macro AC_CONFIG_MACRO_DIR to declare where additional macro files are to be put and found by aclocal. The argument passed to this macro is commonly m4. Despite the documentation, autoconf do nothing with it, only aclocal can use directly if invoked by -I m4 or indirectly using automake. But autoreconf don't invoke aclocal in this way. So in summary you can not use this macro in a useful way if you only use autoconf, as git does. Another historical possibility is to list all your macros in acinclude.m4. This file will be included in aclocal.m4 when you run aclocal, and its macro(s) will henceforth be visible to autoconf. However if it contains numerous macros, it will rapidly become difficult to maintain, and for git this don't provide any benefits or very little. The actual autotool documentation recommend to write each macro in its own file and gather all these files in a separate directory. Where exactly id you find that recommendation? If the autotools docs tell to do so *unconditionally*, they are wrong and should be fixed. In fact, even the configure.ac from Automake itself keeps definition of private macros in configure.ac... Maybe I misinterpreted the ufficial documentation (sure it is for automake ) http://www.gnu.org/software/automake/manual/html_node/Local-Macros.html And this a good reference for me but it is not ufficial https://www.flameeyes.eu/autotools-mythbuster/autoconf/macros.html However, if an automake maintainer can not find the right the patch certainly has the definitive voice. Given the limitations i mentioned earlier, the only possibility is to use the m4_include for including every macro file. The m4_include directive works quite like the #include directive of the C programming language, and simply copies over the content of the file(s). Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- This is a second version of this patch http://article.gmane.org/gmane.comp.version-control.git/231984. The first was plain wrong, my bad. I am sorry for the long delay. Sure it is something low-hanging fruit configure.ac | 148 +++-- m4/git_arg_set_path.m4| 14 m4/git_check_func.m4 | 13 m4/git_conf_append_path.m4| 30 m4/git_conf_subst.m4 | 10 +++ m4/git_conf_subst_init.m4 | 15 m4/git_parse_with.m4 | 22 ++ m4/git_parse_with_set_make_var.m4 | 20 + m4/git_stash_flags.m4 | 15 m4/git_unstash_flags.m4 | 13 10 files changed, 162 insertions(+), 138 deletions(-) create mode 100644 m4/git_arg_set_path.m4 create mode 100644 m4/git_check_func.m4 create mode 100644 m4/git_conf_append_path.m4 create mode 100644 m4/git_conf_subst.m4 create mode 100644 m4/git_conf_subst_init.m4 create mode 100644 m4/git_parse_with.m4 create mode 100644 m4/git_parse_with_set_make_var.m4 create mode 100644 m4/git_stash_flags.m4 create mode 100644 m4/git_unstash_flags.m4 [SNIP] Regards, Stefano -- 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: breakage in revision traversal with pathspec
Kevin Bracey ke...@bracey.fi writes: To see the effect at the command line: git log v1.8.3..v.1.8.4 hides the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new example of a dotty shorthand not being exactly equivalent. In the .. case the v1.8.3 tag gets peeled before being sent to add_rev_cmdline , and the mark bottom commits logic works. But in the ^ case, the v1.8.3 doesn't get peeled. That sounds like a bug. ^v1.8.3 should mark v1.8.3^0 as uninteresting. -- 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: What's cooking in git.git (Sep 2013, #02; Mon, 9)
On 09/10/2013 12:53 AM, Junio C Hamano wrote: * sb/repack-in-c (2013-08-30) 2 commits - repack: retain the return value of pack-objects - repack: rewrite the shell script in C Any further reviews? Just came home from holiday (with no internet ;) but I'll review my code now that I have quite a mental distance to that code I wrote earlier. Stefan signature.asc Description: OpenPGP digital signature
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Wed, Sep 11, 2013 at 09:59:53PM +0200, Sebastian Schuberth wrote:
On Wed, Sep 11, 2013 at 8:29 PM, Jonathan Nieder jrnie...@gmail.com wrote:
This is necessary so that read_mailmap() can obtain a pointer to the
function.
Hm, what platform has strcasecmp() as an inline function? Is this
allowed by POSIX? Even if it isn't, should we perhaps just work
around it by providing our own thin static function wrapper in
mailmap.c?
I'm on Windows using MSYS / MinGW. Since MinGW runtime version 4.0,
string.h contains the following code (see [1]):
#ifndef __NO_INLINE__
__CRT_INLINE int __cdecl __MINGW_NOTHROW
strncasecmp (const char * __sz1, const char * __sz2, size_t __sizeMaxCompare)
{return _strnicmp (__sz1, __sz2, __sizeMaxCompare);}
#else
#define strncasecmp _strnicmp
#endif
What is the error the compiler reports? Can it take the address of other
inline functions? For example, can it compile:
inline int foo(void) { return 5; }
extern int bar(int (*cb)(void));
int call(void) { return bar(foo); }
Just wondering if that is the root of the problem, or if maybe there is
something else subtle going on. Also, does __CRT_INLINE just turn into
inline, or is there perhaps some other pre-processor magic going on?
-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] Windows: Do not redefine _WIN32_WINNT
Sebastian Schuberth sschube...@gmail.com writes: On Wed, Sep 11, 2013 at 8:29 PM, Junio C Hamano gits...@pobox.com wrote: This unfortunately does not seem to match what I have. I think the patch is based on the codebase before these two: 380395d0 (mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE, 2013-05-02) 41f29991 (msvc: Fix compilation errors caused by poll.h emulation, 2013-01-31) I could of course wiggle it in, if you want, but I wanted to know what is going on. Is it a pre-release freeze period on your side or something? That's right, I currently have a code freeze at Git 1.8.3 because I need to solve several other issues with Git 1.8.4 on Windows first. I'd be grateful if you could wiggle it in. It seems that compat/poll/poll.c also defines _WIN32_WINNT (but only with _MSC_VER defined). The change to git-compat-util.h in this patch avoids redefinition for both MinGW and MSVC case. Do you also need to have this, too? Here is what I tentatively queued on top of the three from Karsten, and your Fix stat definitions. -- 8 -- From: Sebastian Schuberth sschube...@gmail.com Date: Wed, 11 Sep 2013 18:06:31 +0200 Subject: [PATCH] Windows: do not redefine _WIN32_WINNT With MinGW runtime version 4.0 this interferes with the previous definition from sdkddkver.h. Signed-off-by: Sebastian Schuberth sschube...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- compat/nedmalloc/malloc.c.h | 2 ++ compat/poll/poll.c | 2 +- git-compat-util.h | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h index ed4f1fa..f216a2a 100644 --- a/compat/nedmalloc/malloc.c.h +++ b/compat/nedmalloc/malloc.c.h @@ -499,7 +499,9 @@ MAX_RELEASE_CHECK_RATE default: 4095 unless not HAVE_MMAP #endif /* WIN32 */ #ifdef WIN32 #define WIN32_LEAN_AND_MEAN +#ifndef _WIN32_WINNT #define _WIN32_WINNT 0x403 +#endif #include windows.h #define HAVE_MMAP 1 #define HAVE_MORECORE 0 diff --git a/compat/poll/poll.c b/compat/poll/poll.c index 4410310..31163f2 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -39,7 +39,7 @@ #if (defined _WIN32 || defined __WIN32__) ! defined __CYGWIN__ # define WIN32_NATIVE -# if defined (_MSC_VER) +# if defined (_MSC_VER) !defined(_WIN32_WINNT) # define _WIN32_WINNT 0x0502 # endif # include winsock2.h diff --git a/git-compat-util.h b/git-compat-util.h index 9549de6..7776f12 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -86,7 +86,7 @@ #define _SGI_SOURCE 1 #if defined(WIN32) !defined(__CYGWIN__) /* Both MinGW and MSVC */ -# if defined (_MSC_VER) +# if defined (_MSC_VER) !defined(_WIN32_WINNT) # define _WIN32_WINNT 0x0502 # endif #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ -- 1.8.4-469-g57f7e3a -- 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
Specifying a private key when connecting to a remote SSH repo
It would be very helpful if you could specify the path to the private key to use for ssh remotes just like in ssh. ``` git push origin master -i 'path_to_key' ``` Althought there are workarounds involving ssh config, if you have a server that has hundreds of git repos, each with the own private key, those workarounds become unusable. This is a very popular request with thousands of comments about it, for example: http://superuser.com/questions/232373/tell-git-which-private-key-to-use http://stackoverflow.com/questions/3496037/how-to-specify-which-ssh-key-to-use-within-git-for-git-push-in-order-to-have-git Thoughts? Thanks! Breck Yunits bre...@gmail.com -- 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
What's cooking in git.git (Sep 2013, #03; Wed, 11)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The second batch of topics are now in 'master'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * es/rebase-i-no-abbrev (2013-08-25) 3 commits (merged to 'next' on 2013-09-04 at 6027805) + rebase -i: fix short SHA-1 collision + t3404: rebase -i: demonstrate short SHA-1 collision + t3404: make tests more self-contained Originally merged to 'next' on 2013-08-26 The commit object names in the insn sheet that was prepared at the beginning of rebase -i session can become ambiguous as the rebasing progresses and the repository gains more commits. Make sure the internal record is kept with full 40-hex object names. * es/rebase-i-respect-core-commentchar (2013-08-18) 1 commit (merged to 'next' on 2013-09-04 at 8c1ce68) + rebase -i: fix cases ignoring core.commentchar Originally merged to 'next' on 2013-08-20 rebase -i forgot that the comment character can be configurable while reading its insn sheet. * jc/ls-files-killed-optim (2013-08-23) 4 commits (merged to 'next' on 2013-09-04 at 20c2304) + dir.c::test_one_path(): work around directory_exists_in_index_icase() breakage + t3010: update to demonstrate ls-files -k optimization pitfalls + ls-files -k: a directory only can be killed if the index has a non-directory + dir.c: use the cache_* macro to access the current index Originally merged to 'next' on 2013-08-27 git ls-files -k needs to crawl only the part of the working tree that may overlap the paths in the index to find killed files, but shared code with the logic to find all the untracked files, which made it unnecessarily inefficient. * jn/post-receive-utf8 (2013-08-05) 3 commits (merged to 'next' on 2013-09-04 at 3a3f480) + hooks/post-receive-email: set declared encoding to utf-8 + hooks/post-receive-email: force log messages in UTF-8 + hooks/post-receive-email: use plumbing instead of git log/show Originally merged to 'next' on 2013-08-20 Update post-receive-email script to make sure the message contents and pathnames are encoded consistently in UTF-8. * js/xread-in-full (2013-08-20) 1 commit (merged to 'next' on 2013-09-04 at 5bfb049) + stream_to_pack: xread does not guarantee to read all requested bytes Originally merged to 'next' on 2013-08-20 A call to xread() was used without a loop around to cope with short read in the codepath to stream new contents to a pack. * nd/push-no-thin (2013-08-13) 1 commit (merged to 'next' on 2013-09-04 at faa8c02) + push: respect --no-thin Originally merged to 'next' on 2013-08-14 git push --no-thin was a no-op by mistake. * rt/rebase-p-no-merge-summary (2013-08-21) 1 commit (merged to 'next' on 2013-09-04 at d8d89ee) + rebase --preserve-merges: ignore merge.log config Originally merged to 'next' on 2013-08-22 git rebase -p internally used the merge machinery, but when rebasing, there should not be a need for merge summary. * sb/mailmap-freeing-NULL-is-ok (2013-08-20) 1 commit (merged to 'next' on 2013-09-04 at c831015) + mailmap: remove redundant check for freeing memory Originally merged to 'next' on 2013-08-20 * sh/pull-rebase-preserve (2013-09-04) 1 commit (merged to 'next' on 2013-09-04 at 32a93bb) + pull: allow pull to preserve merges when rebasing Originally merged to 'next' on 2013-08-14 git pull --rebase always flattened the history; pull.rebase can now be set to preserve to invoke rebase --preserve-merges. * tf/gitweb-ss-tweak (2013-08-20) 4 commits (merged to 'next' on 2013-09-04 at 774bfbe) + gitweb: make search help link less ugly + gitweb: omit the repository owner when it is unset + gitweb: vertically centre contents of page footer + gitweb: ensure OPML text fits inside its box Originally merged to 'next' on 2013-08-22 Tweak Gitweb CSS to layout some elements better. -- [New Topics] * bc/send-email-ssl-die-message-fix (2013-09-10) 1 commit - send-email: don't call methods on undefined values When send-email comes up with an error message to die with upon failure to start an SSL session, it tried to read the error string from a wrong place. Will merge to 'next'. * jc/checkout-detach-doc (2013-09-11) 1 commit - checkout: update synopsys and documentation on detaching HEAD git checkout [--detach] commit was listed poorly in the synopsis section of its documentation. * jc/cvsserver-perm-bit-fix (2013-09-11) 1 commit - cvsserver: pick up the right mode bits git cvsserver computed the permission mode bits incorrectly for executable files. Will merge to 'next'. * jk/trailing-slash-in-pathspec (2013-09-10) 2 commits - rm: re-use parse_pathspec's
Re: [PATCH/RFC 1/5] add a hashtable implementation that supports O(1) removal
Karsten Blees karsten.bl...@gmail.com writes:
+#define FNV32_BASE ((unsigned int) 0x811c9dc5)
+#define FNV32_PRIME ((unsigned int) 0x01000193)
+ ...
+static inline unsigned int bucket(const hashmap *map, const hashmap_entry
*key)
+{
+ return key-hash (map-tablesize - 1);
+}
As tablesize would hopefully be reasonably small, not worrying about
platforms' unsigned int being 64-bit (in which case it would be
more appropriate to compute with FNV64_PRIME) should be fine.
+static inline hashmap_entry **find_entry(const hashmap *map,
+ const hashmap_entry *key)
+{
+ hashmap_entry **e = map-table[bucket(map, key)];
+ while (*e !entry_equals(map, *e, key))
+ e = (*e)-next;
+ return e;
+}
(mental note) This finds the location the pointer to the entry is
stored, not the entry itself.
+void *hashmap_get(const hashmap *map, const void *key)
+{
+ return *find_entry(map, key);
+}
... which is consistent with this, and more importantly, it is
crucial for hashmap_remove()'s implementation, because...
+void *hashmap_remove(hashmap *map, const void *key)
+{
+ hashmap_entry *old;
+ hashmap_entry **e = find_entry(map, key);
+ if (!*e)
+ return NULL;
+
+ /* remove existing entry */
+ old = *e;
+ *e = old-next;
... this wants to update the linked list in place.
Looking good.
I however wonder if the singly linked linear chain is a really good
alternative for the access pattern of the hashes we use, though. Do
we really want to trigger growth on the bucket load factor, not the
length of the longest chain, for example?
+ old-next = NULL;
+
+ /* fix size and rehash if appropriate */
+ map-size--;
+ if (map-tablesize HASHMAP_INITIAL_SIZE
+ map-size * HASHMAP_SHRINK_AT map-tablesize)
+ rehash(map, map-tablesize HASHMAP_GROW);
Please align the first two lines so that the first non-whitespace on
the second line of the condition part of the if statement
(i.e. 'm') aligns with the first non-whitespace inside the '(' open
parenthesis (i.e. 'm').
--
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 v2] http-backend: provide Allow header for 405
The HTTP 1.1 standard requires an Allow header for 405 Method Not Allowed:
The response MUST include an Allow header containing a list of valid methods
for the requested resource.
So provide such a header when we return a 405 to the user agent.
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
http-backend.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/http-backend.c b/http-backend.c
index 0324417..8c464bd 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -594,9 +594,11 @@ int main(int argc, char **argv)
if (strcmp(method, c-method)) {
const char *proto = getenv(SERVER_PROTOCOL);
- if (proto !strcmp(proto, HTTP/1.1))
+ if (proto !strcmp(proto, HTTP/1.1)) {
http_status(405, Method Not Allowed);
- else
+ hdr_str(Allow, !strcmp(c-method,
GET) ?
+ GET, HEAD : c-method);
+ } else
http_status(400, Bad Request);
hdr_nocache();
end_headers();
--
1.8.4.rc3
--
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 v2] http-backend: provide Allow header for 405
brian m. carlson wrote: Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Thanks again for noticing. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.com -- 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 00/21] np/pack-v4 updates
On Wed, Sep 11, 2013 at 11:25 PM, Nicolas Pitre n...@fluxnic.net wrote: On Wed, 11 Sep 2013, Duy Nguyen wrote: Nico, if you have time you may want to look into this. The result v4 pack from pack-objects on git.git for me is 35MB (one branch) while packv4-create produces 30MB (v2 is 40MB). I don't know why there is such a big difference in size. I compared. Ident dict is identical. Tree dict is a bit different (some that have same hits are ordered differently). Delta chains do not differ much. Many groups of entries in the pack are displaced though. I guess I turned a wrong knob or something in pack-objects in v4 code.. Will try to have a closer look. Problem found. I encoded some trees as ref-delta instead of pv4-tree :( Something like this brings the size back to packv4-create output diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index f604fa5..3d9ab0e 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1490,7 +1490,8 @@ static void check_object(struct object_entry *entry) * deltify other objects against, in order to avoid * circular deltas. */ - entry-type = entry-in_pack_type; + if (pack_version 4) + entry-type = entry-in_pack_type; entry-delta = base_entry; entry-delta_size = entry-size; entry-delta_sibling = base_entry-delta_child; -- Duy -- 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
[GIT PULL] updates of German translation for maint branch
Hi, Junio Would you please pull the following into maint branch. And it can be merged to the master branch. This isn't really a bugfix but a nice to have in a maintenance release. ($gmane/233807) The following changes since commit 21860882c8782771e99aa68fab6e365c628ff39d: l10n: fr.po: hotfix for commit 6b388fc (2013-08-30 16:59:29 +0800) are available in the git repository at: git://github.com/git-l10n/git-po maint for you to fetch changes up to 8766343faff88c303917944fb771471ec0fdbec1: l10n: de.po: use das Tag instead of der Tag (2013-09-08 18:37:13 +0200) Ralf Thielow (1): l10n: de.po: use das Tag instead of der Tag po/de.po | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) -- 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: This is sequel to your non-response of my earlier letter to you
Dear Friend After five years of waiting the crown (British government) has given me the power to contact you so that you become the beneficiary to total amount £15,000,000.00 GBP in the intent of the deceased who died without a will. I am contacting you because I believe you are related. Please if you are interested respond immediately with your full names, physical address and cell number. Yours Truly, Brenda Hoffman -- DISCLAIMER: --- This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify the system manager. This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately by e-mail if you have received this e-mail by mistake and delete this e-mail from your system. Before opening any mail and attachments please check them for viruses and defect If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this information is strictly prohibited. - This message has been scanned for viruses and dangerous content, and is believed to be clean. Regional Cancer Centre, Thiruvananthapuram www.rcctvm.org -- 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/RFC 1/5] add a hashtable implementation that supports O(1) removal
Karsten Blees karsten.bl...@gmail.com writes:
+/*
+ * Hashmap entry data structure, intended to be used as first member of user
+ * data structures. Consists of a pointer and an int. Ideally it should be
It is technically correct to say this is intended to be used, but
to those who are using this API, it would be more helpful to say a
user data structure that uses this API *must* have this as its first
member field.
+ * followed by an int-sized member to prevent unused memory on 64-bit systems
+ * due to alignment.
+ */
+typedef struct hashmap_entry {
+ struct hashmap_entry *next;
+ unsigned int hash;
+} hashmap_entry;
+ ...
+typedef struct hashmap {
+ hashmap_entry **table;
+ hashmap_cmp_fn cmpfn;
+ unsigned int size, tablesize;
+} hashmap;
I forgot to mention in my previous message, but we find that the
code tends to be easier to read if we avoid using typedef'ed struct
like these. E.g. in 2/5 we see something like this:
static int abbrev = -1; /* unspecified */
static int max_candidates = 10;
-static struct hash_table names;
+static hashmap names;
static int have_util;
static const char *pattern;
static int always;
@@ -38,7 +38,7 @@ static const char *diff_index_args[] = {
struct commit_name {
- struct commit_name *next;
+ hashmap_entry entry;
unsigned char peeled[20];
The version before the patch is preferrable.
Thanks.
--
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: [GIT PULL] updates of German translation for maint branch
Thanks, will do. -- 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: Specifying a private key when connecting to a remote SSH repo
Breck Yunits bre...@gmail.com writes:
It would be very helpful if you could specify the path to the private
key to use for ssh remotes just like in ssh.
You could add a support for the remote.name.sshIdentityFile
configuration variable, i.e.
[remote origin]
url = br...@example.com:projects/w.git
sshIdentityFile = ~/.ssh/id_for_example_com
and then teach the codepath that leads to connect.c::git_connect()
to pass it down, and add -o 'IdentityFile %s' to the command line
that invokes ssh.
But you would need the same number of configuration as you have
remotes that need such custom identity files, which happens to be
the same number of entries you would normally configure with the
standard ~/.ssh/config file, so I do not think it is worth it.
If the only thing you are interested in supporting is a one-shot
invocation, i.e. giving which identity file to use from the command
line when you run either git push or git fetch, I suspect that
you could play with GIT_SSH environment variable, e.g.
GIT_SSH_IDENTITY_FILE=$HOME/.ssh/id_for_example_com git push
and do something ugly like the attached, I suppose.
It also crossed my mind that you could (ab)use the credential helper
framework and ask it to return not the password but the identity
filename, and pass it down the callchain to git_connect(), but again
you will have to teach the credential helper as many settings as you
would need to make in ~/.ssh/config anyway, so I find it dubious how
it would be a win.
connect.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/connect.c b/connect.c
index a80ebd3..87b7ceb 100644
--- a/connect.c
+++ b/connect.c
@@ -623,9 +623,10 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
die(command line too long);
conn-in = conn-out = -1;
- conn-argv = arg = xcalloc(7, sizeof(*arg));
+ conn-argv = arg = xcalloc(20, sizeof(*arg));
if (protocol == PROTO_SSH) {
const char *ssh = getenv(GIT_SSH);
+ const char *ssh_ident = getenv(GIT_SSH_IDENTITY_FILE);
int putty = ssh strcasestr(ssh, plink);
if (!ssh) ssh = ssh;
@@ -637,6 +638,12 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
*arg++ = putty ? -P : -p;
*arg++ = port;
}
+ if (ssh_ident) {
+ char *ident_arg = xmalloc(strlen(ssh_ident) + 50);
+ sprintf(ident_arg, IdentityFile %s, ssh_ident);
+ *arg++ = -o;
+ *arg++ = ident_arg;
+ }
*arg++ = host;
}
else {
--
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 v2] http-backend: provide Allow header for 405
On Thu, Sep 12, 2013 at 12:30:01AM +, brian m. carlson wrote: The HTTP 1.1 standard requires an Allow header for 405 Method Not Allowed: The response MUST include an Allow header containing a list of valid methods for the requested resource. So provide such a header when we return a 405 to the user agent. Signed-off-by: brian m. carlson sand...@crustytoothpaste.net Thanks, this looks good to me. -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: Specifying a private key when connecting to a remote SSH repo
On Wed, Sep 11, 2013 at 09:39:55PM -0700, Junio C Hamano wrote: If the only thing you are interested in supporting is a one-shot invocation, i.e. giving which identity file to use from the command line when you run either git push or git fetch, I suspect that you could play with GIT_SSH environment variable, e.g. GIT_SSH_IDENTITY_FILE=$HOME/.ssh/id_for_example_com git push and do something ugly like the attached, I suppose. We already have GIT_SSH, so I would expect: GIT_SSH='ssh -i $HOME/.ssh/id_for_example_com' git push to work. But sadly, GIT_SSH does not use the shell, unlike most other configure git commands. :( We could consider it a consistency bug and fix it, though I suspect we may be annoying people on Windows who have spaces in their paths. If we do go the route of adding a new variable, it would make sense to add something for specifying arbitrary arguments, not just the identity file. Something like GIT_SSH_ARGS would be enough, though once you start handling splitting, dequoting, and interpreting variables, you're better off using the shell. So maybe GIT_SSH_SHELL or similar as a preferred version of GIT_SSH that uses the shell. It also crossed my mind that you could (ab)use the credential helper framework and ask it to return not the password but the identity filename, and pass it down the callchain to git_connect(), but again you will have to teach the credential helper as many settings as you would need to make in ~/.ssh/config anyway, so I find it dubious how it would be a win. You could write a credential helper shell script that knows about classes of remotes (e.g., selecting an identity file based on the hostname), and write only a few lines to cover a large number of hosts. For example: #!/bin/sh test $1 = get || exit 0 while IFS== read key val; do test $key = host || continue case $val in *.example.com) echo sshident=com_key ;; *.example.net) echo sshident=net_key ;; esac done But it feels a bit hacky to be using the credential helpers at all for ssh connections. -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: What's cooking in git.git (Sep 2013, #03; Wed, 11)
On Wed, Sep 11, 2013 at 04:32:23PM -0700, Junio C Hamano wrote: * jk/config-int-range-check (2013-09-09) 5 commits (merged to 'next' on 2013-09-09 at 9ab779d) + git-config: always treat --int as 64-bit internally + config: make numeric parsing errors more clear + config: set errno in numeric git_parse_* functions + config: properly range-check integer values + config: factor out integer parsing from range checks Originally merged to 'next' on 2013-08-22 git config --int section.var 3g should somehow diagnose that the number does not fit in int (on 32-bit platforms anyway) but it did not. This description is slightly inaccurate since the re-roll. I think it is now: git config did not provide a way to set or access numbers larger than a native int on the platform; it now provides 64-bit signed integers on all platforms. Not a big deal, but it may make your life easier if this description ends up in the merge commit, and then you later try to write ReleaseNotes off of it. It also fixes the mis-detection of 32-bit overflow on certain platforms, but the only likely way to trigger that was via config --int (unless you are crazy enough to set gc.auto to 2g or something). But now that git-config is 64-bit that does not even matter, and it is probably not even worth mentioning in the release notes. -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
