Re: [PATCH v2 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
Hi Junio,
On Wed, 10 Sep 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
diff --git a/fsck.c b/fsck.c
index dd77628..9dd7d12 100644
--- a/fsck.c
+++ b/fsck.c
@@ -237,6 +237,26 @@ static int fsck_tree(struct tree *item, int strict,
fsck_error error_func)
return retval;
}
+static int require_end_of_header(const void *data, unsigned long size,
+ struct object *obj, fsck_error error_func)
+{
+ const char *buffer = (const char *)data;
+ int i;
+
+ for (i = 0; i size; i++) {
+ switch (buffer[i]) {
+ case '\0':
+ return error_func(obj, FSCK_ERROR,
+ invalid message: NUL at offset %d, i);
Isn't this invalid header? After all we haven't escaped this loop
and haven't seen the message part of the commit object (and it is
the same if you are going to later reuse this for tag objects).
My reasoning for keeping it saying message was that a message consists
of a header and a body. I will change it to unterminated header instead,
also in the error message when no NUL was found.
Ciao,
Dscho
--
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 5/6] Add regression tests for stricter tag fsck'ing
Hi Junio, On Wed, 10 Sep 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: + test_when_finished git update-ref -d refs/tags/wrong + git fsck --tags 2out I wonder what the command does with or without --tags option (applies to both tests added by this patch)? Does running fsck without the option not to report broken tags detected by the new checks added in this series? Should it? fsck fails perfectly fine without the --tags option; this option just triggers that fsck will show also the objects referenced by the tag objects (which is nice for debugging). Ciao, Dscho -- 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 6/6] Make sure that index-pack --strict fails upon invalid tag objects
Hi,
On Wed, 10 Sep 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
+tag=$(git hash-object -t tag -w --stdin wrong-tag)
+pack1=$(echo $tag | git pack-objects tag-test)
+echo remove tag object
+thirtyeight=${tag#??}
+rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight
+test_must_fail git index-pack --strict tag-test-${pack1}.pack 2 err
I had to drop must fail from this one (will amend the SQUASH???
again).
Funny. It failed here, but for the wrong reason: index-pack --strict
failed here because the object referenced by the tag object was not in the
pack.
Ciao,
Dscho
--
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 v3 2/6] Accept object data in the fsck_object() function
When fsck'ing an incoming pack, we need to fsck objects that cannot be
read via read_sha1_file() because they are not local yet (and might even
be rejected if transfer.fsckobjects is set to 'true').
For commits, there is a hack in place: we basically cache commit
objects' buffers anyway, but the same is not true, say, for tag objects.
By refactoring fsck_object() to take the object buffer and size as
optional arguments -- optional, because we still fall back to the
previous method to look at the cached commit objects if the caller
passes NULL -- we prepare the machinery for the upcoming handling of tag
objects.
The assumption that such buffers are inherently NUL terminated is now
wrong, of course, hence we pass the size of the buffer so that we can
add a sanity check later, to prevent running past the end of the buffer.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
builtin/fsck.c | 2 +-
builtin/index-pack.c | 3 ++-
builtin/unpack-objects.c | 14 ++
fsck.c | 24 +++-
fsck.h | 4 +++-
5 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index d42a27d..d9f4e6e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -298,7 +298,7 @@ static int fsck_obj(struct object *obj)
if (fsck_walk(obj, mark_used, NULL))
objerror(obj, broken links);
- if (fsck_object(obj, check_strict, fsck_error_func))
+ if (fsck_object(obj, NULL, 0, check_strict, fsck_error_func))
return -1;
if (obj-type == OBJ_TREE) {
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 5568a5b..f2465ff 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -773,7 +773,8 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
if (!obj)
die(_(invalid %s), typename(type));
if (do_fsck_object
- fsck_object(obj, 1, fsck_error_function))
+ fsck_object(obj, buf, size, 1,
+ fsck_error_function))
die(_(Error in object));
if (fsck_walk(obj, mark_link, NULL))
die(_(Not all child objects of %s are
reachable), sha1_to_hex(obj-sha1));
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 99cde45..855d94b 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -164,10 +164,10 @@ static unsigned nr_objects;
* Called only from check_object() after it verified this object
* is Ok.
*/
-static void write_cached_object(struct object *obj)
+static void write_cached_object(struct object *obj, struct obj_buffer *obj_buf)
{
unsigned char sha1[20];
- struct obj_buffer *obj_buf = lookup_object_buffer(obj);
+
if (write_sha1_file(obj_buf-buffer, obj_buf-size,
typename(obj-type), sha1) 0)
die(failed to write object %s, sha1_to_hex(obj-sha1));
obj-flags |= FLAG_WRITTEN;
@@ -180,6 +180,8 @@ static void write_cached_object(struct object *obj)
*/
static int check_object(struct object *obj, int type, void *data)
{
+ struct obj_buffer *obj_buf;
+
if (!obj)
return 1;
@@ -198,11 +200,15 @@ static int check_object(struct object *obj, int type,
void *data)
return 0;
}
- if (fsck_object(obj, 1, fsck_error_function))
+ obj_buf = lookup_object_buffer(obj);
+ if (!obj_buf)
+ die(Whoops! Cannot find object '%s', sha1_to_hex(obj-sha1));
+ if (fsck_object(obj, obj_buf-buffer, obj_buf-size, 1,
+ fsck_error_function))
die(Error in object);
if (fsck_walk(obj, check_object, NULL))
die(Error on reachable objects of %s, sha1_to_hex(obj-sha1));
- write_cached_object(obj);
+ write_cached_object(obj, obj_buf);
return 0;
}
diff --git a/fsck.c b/fsck.c
index 56156ff..dd77628 100644
--- a/fsck.c
+++ b/fsck.c
@@ -277,7 +277,7 @@ static int fsck_ident(const char **ident, struct object
*obj, fsck_error error_f
}
static int fsck_commit_buffer(struct commit *commit, const char *buffer,
- fsck_error error_func)
+ unsigned long size, fsck_error error_func)
{
unsigned char tree_sha1[20], sha1[20];
struct commit_graft *graft;
@@ -322,15 +322,18 @@ static int fsck_commit_buffer(struct commit *commit,
const char *buffer,
return 0;
}
-static int fsck_commit(struct commit *commit, fsck_error error_func)
+static int fsck_commit(struct commit *commit, const char *data,
+ unsigned long size, fsck_error error_func)
{
- const char *buffer = get_commit_buffer(commit, NULL);
- int ret = fsck_commit_buffer(commit, buffer, error_func);
- unuse_commit_buffer(commit,
[PATCH v3 1/6] Refactor type_from_string() to avoid die()ing in case of errors
In the next commits, we will enhance the fsck_tag() function to check
tag objects more thoroughly. To this end, we need a function to verify
that a given string is a valid object type, but that does not die() in
the negative case.
While at it, prepare type_from_string() for counted strings, i.e. strings
with an explicitly specified length rather than a NUL termination.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
object.c | 11 +--
object.h | 3 ++-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/object.c b/object.c
index a16b9f9..aedac24 100644
--- a/object.c
+++ b/object.c
@@ -33,13 +33,20 @@ const char *typename(unsigned int type)
return object_type_strings[type];
}
-int type_from_string(const char *str)
+int type_from_string_gently(const char *str, ssize_t len, int gentle)
{
int i;
+ if (len 0)
+ len = strlen(str);
+
for (i = 1; i ARRAY_SIZE(object_type_strings); i++)
- if (!strcmp(str, object_type_strings[i]))
+ if (!strncmp(str, object_type_strings[i], len))
return i;
+
+ if (gentle)
+ return -1;
+
die(invalid object type \%s\, str);
}
diff --git a/object.h b/object.h
index 5e8d8ee..e028ced 100644
--- a/object.h
+++ b/object.h
@@ -53,7 +53,8 @@ struct object {
};
extern const char *typename(unsigned int type);
-extern int type_from_string(const char *str);
+extern int type_from_string_gently(const char *str, ssize_t, int gentle);
+#define type_from_string(str) type_from_string_gently(str, -1, 0)
/*
* Return the current number of buckets in the object hashmap.
--
2.0.0.rc3.9669.g840d1f9
--
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 v3 0/6] Improve tag checking in fsck and with transfer.fsckobjects
This patch series introduces detailed checking of tag objects when calling git fsck, and also when transfer.fsckobjects is set to true. To this end, the fsck machinery is reworked to accept the buffer and size of the object to check, and for commit and tag objects, we verify that the buffers contain an end of header (i.e. an empty line) to guarantee that our checks do not run beyond the buffer. This work was sponsored by GitHub. Changes since v2: - replaced 'invalid message' with 'unterminated header' - avoided comparison between int and unsigned long (thanks, Eric Sunshine) - made ident parsing conditional on finding the optional 'tagger' line - added forgotten strbuf_release() Still unaddressed: - getting rid of struct object altogether in fsck (I felt this was quite a big task, getting much more familiar with the non-tag code paths, and I did not want to delay this patch series up any further) - ensuring that index-pack passes only NUL-terminated buffers to fsck (again, I am not familiar enough with the code, and IIRC the problematic unit test that revealed that these buffers are not always NUL-terminated exercised the unpack-objects code path, not index-pack, again nothing I wanted to let delay this patch series any further). Johannes Schindelin (6): Refactor type_from_string() to avoid die()ing in case of errors Accept object data in the fsck_object() function Make sure fsck_commit_buffer() does not run out of the buffer fsck: check tag objects' headers Add regression tests for stricter tag fsck'ing Make sure that index-pack --strict checks tag objects builtin/fsck.c | 2 +- builtin/index-pack.c | 3 +- builtin/unpack-objects.c | 14 +++-- fsck.c | 133 +++ fsck.h | 4 +- object.c | 11 +++- object.h | 3 +- t/t1450-fsck.sh | 20 +++ t/t5302-pack-index.sh| 19 +++ 9 files changed, 189 insertions(+), 20 deletions(-) -- 2.0.0.rc3.9669.g840d1f9 -- 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 v3 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
So far, we assumed that the buffer is NUL terminated, but this is not
a safe assumption, now that we opened the fsck_object() API to pass a
buffer directly.
So let's make sure that there is at least an empty line in the buffer.
That way, our checks would fail if the empty line was encountered
prematurely, and consequently we can get away with the current string
comparisons even with non-NUL-terminated buffers are passed to
fsck_object().
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
fsck.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/fsck.c b/fsck.c
index dd77628..73da6f8 100644
--- a/fsck.c
+++ b/fsck.c
@@ -237,6 +237,26 @@ static int fsck_tree(struct tree *item, int strict,
fsck_error error_func)
return retval;
}
+static int require_end_of_header(const void *data, unsigned long size,
+ struct object *obj, fsck_error error_func)
+{
+ const char *buffer = (const char *)data;
+ unsigned long i;
+
+ for (i = 0; i size; i++) {
+ switch (buffer[i]) {
+ case '\0':
+ return error_func(obj, FSCK_ERROR,
+ unterminated header: NUL at offset %d, i);
+ case '\n':
+ if (i + 1 size buffer[i + 1] == '\n')
+ return 0;
+ }
+ }
+
+ return error_func(obj, FSCK_ERROR, unterminated header);
+}
+
static int fsck_ident(const char **ident, struct object *obj, fsck_error
error_func)
{
char *end;
@@ -284,6 +304,9 @@ static int fsck_commit_buffer(struct commit *commit, const
char *buffer,
unsigned parent_count, parent_line_count = 0;
int err;
+ if (require_end_of_header(buffer, size, commit-object, error_func))
+ return -1;
+
if (!skip_prefix(buffer, tree , buffer))
return error_func(commit-object, FSCK_ERROR, invalid format
- expected 'tree' line);
if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')
--
2.0.0.rc3.9669.g840d1f9
--
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 v3 6/6] Make sure that index-pack --strict checks tag objects
One of the most important use cases for the strict tag object checking
is when transfer.fsckobjects is set to true to catch invalid objects
early on. This new regression test essentially tests the same code path
by directly calling 'index-pack --strict' on a pack containing an
tag object without a 'tagger' line.
Technically, this test is not enough: it only exercises a code path that
*warns*, not one that *fails*. The reason is that it would be exquisitely
convoluted to test that: not only hash-object, but also pack-index
actually *parse* tag objects when encountering them. Therefore we would
have to actively *break* pack-index in order to test this. Or rewrite
both hash-object and pack-index in shell script. Ain't gonna happen.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
t/t5302-pack-index.sh | 19 +++
1 file changed, 19 insertions(+)
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 4bbb718..4d033df 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -243,4 +243,23 @@ test_expect_success 'running index-pack in the object
store' '
test -f .git/objects/pack/pack-${pack1}.idx
'
+test_expect_success 'index-pack --strict warns upon missing tagger in tag' '
+sha=$(git rev-parse HEAD)
+cat wrong-tag EOF
+object $sha
+type commit
+tag guten tag
+
+This is an invalid tag.
+EOF
+
+tag=$(git hash-object -t tag -w --stdin wrong-tag)
+pack1=$(echo $tag $sha | git pack-objects tag-test)
+echo remove tag object
+thirtyeight=${tag#??}
+rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight
+git index-pack --strict tag-test-${pack1}.pack 2 err
+grep ^error:.* expected .tagger. line err
+'
+
test_done
--
2.0.0.rc3.9669.g840d1f9
--
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 0/6] Improve tag checking in fsck and with transfer.fsckobjects
Hi Junio, On Wed, 10 Sep 2014, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Johannes Schindelin johannes.schinde...@gmx.de writes: This patch series introduces detailed checking of tag objects when calling git fsck, and also when transfer.fsckobjects is set to true. To this end, the fsck machinery is reworked to accept the buffer and size of the object to check, and for commit and tag objects, we verify that the buffers contain an end of header (i.e. an empty line) to guarantee that our checks do not run beyond the buffer. Overall they looked sensible and I am trying to queue them on 'pu' for today's pushout. Thanks. I was expecting to see interesting interactions with the oops, fsck was not exiting with non-zero status in some cases fix by Peff with the patch 5/6 of this series that adds two new tests to t1450, but because the breakage in these two cases are both only warning-events and not error-events, I didn't prefix git fsck with test_must_fail after all. There were a couple of issues, thanks for pointing out that I missed something. Short story: hash-object, fsck *and* pack-objects parse tag objects as they are encountered. Therefore, it would be a bit hard to test because we would have to emulate broken hash-object and pack-objects to generate a pack containing an invalid tag object. As a compromise, I now test for the warnings to make sure that the code path is triggered, but do not explicitly test with a pack that would make index-pack --strict *fail*. Okay? Ciao, Dscho P.S.: I squashed your changes in slightly modified form. -- 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 v3 4/6] fsck: check tag objects' headers
We inspect commit objects pretty much in detail in git-fsck, but we just
glanced over the tag objects. Let's be stricter.
Since we do not want to limit 'tag' lines unduly, values that would fail
the refname check only result in warnings, not errors.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
fsck.c | 86 +-
1 file changed, 85 insertions(+), 1 deletion(-)
diff --git a/fsck.c b/fsck.c
index 73da6f8..2fffa43 100644
--- a/fsck.c
+++ b/fsck.c
@@ -6,6 +6,7 @@
#include commit.h
#include tag.h
#include fsck.h
+#include refs.h
static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)
{
@@ -355,6 +356,88 @@ static int fsck_commit(struct commit *commit, const char
*data,
return ret;
}
+static int fsck_tag_buffer(struct tag *tag, const char *data,
+ unsigned long size, fsck_error error_func)
+{
+ unsigned char sha1[20];
+ int ret = 0;
+ const char *buffer;
+ char *to_free = NULL, *eol;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (data)
+ buffer = data;
+ else {
+ enum object_type type;
+
+ buffer = to_free =
+ read_sha1_file(tag-object.sha1, type, size);
+ if (!buffer)
+ return error_func(tag-object, FSCK_ERROR,
+ cannot read tag object);
+
+ if (type != OBJ_TAG) {
+ ret = error_func(tag-object, FSCK_ERROR,
+ expected tag got %s,
+ typename(type));
+ goto done;
+ }
+ }
+
+ if (require_end_of_header(buffer, size, tag-object, error_func))
+ goto done;
+
+ if (!skip_prefix(buffer, object , buffer)) {
+ ret = error_func(tag-object, FSCK_ERROR, invalid format -
expected 'object' line);
+ goto done;
+ }
+ if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n') {
+ ret = error_func(tag-object, FSCK_ERROR, invalid 'object'
line format - bad sha1);
+ goto done;
+ }
+ buffer += 41;
+
+ if (!skip_prefix(buffer, type , buffer)) {
+ ret = error_func(tag-object, FSCK_ERROR, invalid format -
expected 'type' line);
+ goto done;
+ }
+ eol = strchr(buffer, '\n');
+ if (!eol) {
+ ret = error_func(tag-object, FSCK_ERROR, invalid format -
unexpected end after 'type' line);
+ goto done;
+ }
+ if (type_from_string_gently(buffer, eol - buffer, 1) 0)
+ ret = error_func(tag-object, FSCK_ERROR, invalid 'type'
value);
+ if (ret)
+ goto done;
+ buffer = eol + 1;
+
+ if (!skip_prefix(buffer, tag , buffer)) {
+ ret = error_func(tag-object, FSCK_ERROR, invalid format -
expected 'tag' line);
+ goto done;
+ }
+ eol = strchr(buffer, '\n');
+ if (!eol) {
+ ret = error_func(tag-object, FSCK_ERROR, invalid format -
unexpected end after 'type' line);
+ goto done;
+ }
+ strbuf_addf(sb, refs/tags/%.*s, (int)(eol - buffer), buffer);
+ if (check_refname_format(sb.buf, 0))
+ error_func(tag-object, FSCK_WARN, invalid 'tag' name: %s,
buffer);
+ buffer = eol + 1;
+
+ if (!skip_prefix(buffer, tagger , buffer))
+ /* early tags do not contain 'tagger' lines; warn only */
+ error_func(tag-object, FSCK_WARN, invalid format - expected
'tagger' line);
+ else
+ ret = fsck_ident(buffer, tag-object, error_func);
+
+done:
+ strbuf_release(sb);
+ free(to_free);
+ return ret;
+}
+
static int fsck_tag(struct tag *tag, const char *data,
unsigned long size, fsck_error error_func)
{
@@ -362,7 +445,8 @@ static int fsck_tag(struct tag *tag, const char *data,
if (!tagged)
return error_func(tag-object, FSCK_ERROR, could not load
tagged object);
- return 0;
+
+ return fsck_tag_buffer(tag, data, size, error_func);
}
int fsck_object(struct object *obj, void *data, unsigned long size,
--
2.0.0.rc3.9669.g840d1f9
--
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 v3 5/6] Add regression tests for stricter tag fsck'ing
The intent of the new test case is to catch general breakages in
the fsck_tag() function, not so much to test it extensively, trying to
strike the proper balance between thoroughness and speed.
While it *would* have been nice to test the code path where fsck_object()
encounters an invalid tag object, this is not possible using git fsck: tag
objects are parsed already before fsck'ing (and the parser already fails
upon such objects).
Even worse: we would not even be able write out invalid tag objects
because git hash-object parses those objects, too, unless we resorted to
really ugly hacks such as using something like this in the unit tests
(essentially depending on Perl *and* Compress::Zlib):
hash_invalid_object () {
contents=$(printf '%s %d\0%s' $1 ${#2} $2)
sha1=$(echo $contents | test-sha1)
suffix=${sha1#??}
mkdir -p .git/objects/${sha1%$suffix}
echo $contents |
perl -MCompress::Zlib -e 'undef $/; print compress()' \
.git/objects/${sha1%$suffix}/$suffix
echo $sha1
}
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
t/t1450-fsck.sh | 20
1 file changed, 20 insertions(+)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 8c739c9..2b6a6f2 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -194,6 +194,26 @@ test_expect_success 'tag pointing to something else than
its type' '
test_must_fail git fsck --tags
'
+test_expect_success 'tag with incorrect tag name missing tagger' '
+ sha=$(git rev-parse HEAD)
+ cat wrong-tag -EOF
+ object $sha
+ type commit
+ tag wrong name format
+
+ This is an invalid tag.
+ EOF
+
+ tag=$(git hash-object -t tag -w --stdin wrong-tag)
+ test_when_finished remove_object $tag
+ echo $tag .git/refs/tags/wrong
+ test_when_finished git update-ref -d refs/tags/wrong
+ git fsck --tags 2out
+ cat out
+ grep invalid .tag. name out
+ grep expected .tagger. line out
+'
+
test_expect_success 'cleaned up' '
git fsck actual 21
test_cmp empty actual
--
2.0.0.rc3.9669.g840d1f9
--
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 22/32] checkout: support checking out into a new working directory
On 14-09-10 06:41 PM, Nguyễn Thái Ngọc Duy wrote: git checkout --to sets up a new working directory with a .git file pointing to $GIT_DIR/worktrees/id. It then executes git checkout again on the new worktree with the same arguments except --to is taken out. The second checkout execution, which is not contaminated with any info from the current repository, will actually check out and everything that normal git checkout does. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-checkout.txt | 43 +++ Documentation/git.txt | 3 +- Documentation/gitrepository-layout.txt | 7 +++ builtin/checkout.c | 95 +- path.c | 2 +- t/t2025-checkout-to.sh (new +x)| 63 ++ 6 files changed, 209 insertions(+), 4 deletions(-) create mode 100755 t/t2025-checkout-to.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 33ad2ad..bd0fc1d 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -225,6 +225,13 @@ This means that you can use `git checkout -p` to selectively discard edits from your current working tree. See the ``Interactive Mode'' section of linkgit:git-add[1] to learn how to operate the `--patch` mode. +--to=path:: + Check out a branch in a separate working directory at + `path`. A new working directory is linked to the current + repository, sharing everything except working directory + specific files such as HEAD, index... See MULTIPLE CHECKOUT + MODE section for more information. + branch:: Branch to checkout; if it refers to a branch (i.e., a name that, when prepended with refs/heads/, is a valid ref), then that @@ -388,6 +395,42 @@ $ git reflog -2 HEAD # or $ git log -g -2 HEAD +MULTIPLE CHECKOUT MODE +-- +Normally a working directory is attached to repository. When git +checkout --to is used, a new working directory is attached to the +current repository. This new working directory is called linked +checkout as compared to the main checkout prepared by git init or +git clone. A repository has one main checkout and zero or more +linked checkouts. + +Each linked checkout has a private directory in $GIT_DIR/worktrees in +the main checkout, usually named after the base name of the new +working directory, optionally with a number added to make it +unique. For example, the command `git checkout --to ../test-next next` +running with $GIT_DIR=/path/main may create the directory +`$GIT_DIR/worktrees/test-next` (or `$GIT_DIR/worktrees/test-next1` if +`test-next` is already taken). + +Within a linked checkout, $GIT_DIR is set to point to this private +directory (e.g. `/path/main/worktrees/test-next` in the example) and +$GIT_COMMON_DIR is set to point back to the main checkout's $GIT_DIR +(e.g. `/path/main`). Setting is done via a .git file located at the +top directory of the linked checkout. + +Path resolution via `git rev-parse --git-path` would use either +$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, the +linked checkout's `$GIT_DIR/HEAD` resolve to +`/path/main/worktrees/test-next/HEAD` (not `/path/main/HEAD` which is +the main checkout's HEAD) while `$GIT_DIR/refs/heads/master` would use +$GIT_COMMON_DIR and resolve to `/path/main/refs/heads/master`, which +is shared across checkouts. + +See linkgit:gitrepository-layout[5] for more information. The rule of +thumb is do not make any assumption about whether a path belongs to +$GIT_DIR or $GIT_COMMON_DIR when you need to directly access something +inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. + Um, didn't you say in [1] that you'd use the text I posted in [2]? [1] http://article.gmane.org/gmane.comp.version-control.git/256446 [2] http://article.gmane.org/gmane.comp.version-control.git/256323 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] Documentation: use single-parameter --cacheinfo in example
The single-parameter form is described as the preferred way. Separate arguments are only supported for backward compatibility. Update the example to the recommended form. Signed-off-by: Steffen Prohaska proha...@zib.de --- Documentation/git-update-index.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index dfc09d9..82eca6f 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -202,7 +202,7 @@ merging. To pretend you have a file with mode and sha1 at path, say: -$ git update-index --cacheinfo mode sha1 path +$ git update-index --cacheinfo mode,sha1,path '--info-only' is used to register files without placing them in the object -- 2.1.0.137.gbf198b9 -- 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 23/32] prune: strategies for linked checkouts
On 14-09-10 06:41 PM, Nguyễn Thái Ngọc Duy wrote: (alias R=$GIT_COMMON_DIR/worktrees/id) - linked checkouts are supposed to keep its location in $R/gitdir up to date. The use case is auto fixup after a manual checkout move. - linked checkouts are supposed to update mtime of $R/gitdir. If $R/gitdir's mtime is older than a limit, and it points to nowhere, worktrees/id is to be pruned. - If $R/locked exists, worktrees/id is not supposed to be pruned. If $R/locked exists and $R/gitdir's mtime is older than a really long limit, warn about old unused repo. - git checkout --to is supposed to make a hard link named $R/link pointing to the .git file on supported file systems to help detect the user manually deleting the checkout. If $R/link exists and its link count is greated than 1, the repo is kept. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-checkout.txt | 18 ++ Documentation/git-prune.txt| 3 + Documentation/gitrepository-layout.txt | 19 ++ builtin/checkout.c | 19 +- builtin/prune.c| 95 ++ setup.c| 13 t/t2026-prune-linked-checkouts.sh (new +x) | 84 ++ 7 files changed, 249 insertions(+), 2 deletions(-) create mode 100755 t/t2026-prune-linked-checkouts.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index bd0fc1d..a29748e 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -431,6 +431,24 @@ thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. +When you are done, simply deleting the linked working directory would +suffice. $GIT_DIR/worktrees can be cleaned up using `git prune +--worktrees`. This needs a tweak or two so that it follows more naturally from the previous paragraph. How about: When you are done with a linked working tree you can simply delete it. You can clean up any stale $GIT_DIR/worktrees entries with `git prune --worktrees`. Then in commit 28, when you add worktrees pruning to gc, you should change this paragraph again: When you are done with a linked working tree you can simply delete it. The working tree's entry in the repository's $GIT_DIR/worktrees directory will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run `git prune --worktrees` to clean up any stale entries in $GIT_DIR/worktrees. +After you move a linked working directory to another file system, or +on a file system that does not support hard link, execute any git +command (e.g. `git status`) in the new working directory so that it +could update its location in $GIT_DIR/worktrees and not be +accidentally pruned. It took me a couple of seconds to parse that. How about: If you move a linked working directory to another file system, or within a file system that does not support hard links, you need to run at least one git command inside the moved linked working directory (e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees so that it does not get automatically removed. +To stop `git prune --worktrees` from deleting a specific working +directory (e.g. because it's on a portable device), you could add the +file 'locked' to $GIT_DIR/worktrees. For example, if `.git` file of +the new working directory points to `/path/main/worktrees/test-next`, +the full path of the 'locked' file would be +`/path/main/worktrees/test-next/locked`. See +linkgit:gitrepository-layout[5] for details. Sorry, I can't help rewriting this one too: To prevent `git prune --worktrees` from deleting a $GIT_DIR/worktrees entry (which can be useful in some situations, such as when the entry's working tree is stored on a portable device), add a file named 'locked' to the entry's directory. For example, if a linked working tree's `.git` file points to `/path/main/.git/worktrees/test-next` then a file named `/path/main/.git/worktrees/test-next/locked` will prevent the `test-next` entry from being pruned. See linkgit:gitrepository-layout[5] for details. I also suggest this paragraph get updated in commit 28, but just change the first clause, before (which can be ...: To prevent a $GIT_DIR/worktrees entry from being pruned (which ... Thanks for all this work! 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
Re: [PATCH v2 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
Johannes Schindelin johannes.schinde...@gmx.de writes:
+ for (i = 0; i size; i++) {
+ switch (buffer[i]) {
+ case '\0':
+ return error_func(obj, FSCK_ERROR,
+ invalid message: NUL at offset %d, i);
Isn't this invalid header? After all we haven't escaped this loop
and haven't seen the message part of the commit object (and it is
the same if you are going to later reuse this for tag objects).
My reasoning for keeping it saying message was that a message consists
of a header and a body. I will change it to unterminated header instead,
also in the error message when no NUL was found.
Because end users think of a message in the context of discussing
either a commit or a tag as what they give as the value to the -m
option, the object payload consists of a header and a body the
latter of which *is* the message to them. Choosing a word that is
not message is a good way to avoid potential confusion here.
--
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 6/6] Make sure that index-pack --strict fails upon invalid tag objects
Johannes Schindelin johannes.schinde...@gmx.de writes:
Hi,
On Wed, 10 Sep 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
+tag=$(git hash-object -t tag -w --stdin wrong-tag)
+pack1=$(echo $tag | git pack-objects tag-test)
+echo remove tag object
+thirtyeight=${tag#??}
+rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight
+test_must_fail git index-pack --strict tag-test-${pack1}.pack 2 err
I had to drop must fail from this one (will amend the SQUASH???
again).
Funny. It failed here, but for the wrong reason: index-pack --strict
failed here because the object referenced by the tag object was not in the
pack.
That is strange. It failed with the version you sent to the list
for me for a different reason---it tried to verify the ident that
did not exist in the tested object (which we fixed in the squash).
--
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] pretty: add %D format specifier
Because patch 1/2 alone does not make much sense without 2/2, it probably would have been better to do these as a single patch. And of course a few additional tests to t4205 would not hurt ;-) 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: [PATCH v2 6/6] Make sure that index-pack --strict fails upon invalid tag objects
Hi Junio,
On Thu, 11 Sep 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
On Wed, 10 Sep 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
+tag=$(git hash-object -t tag -w --stdin wrong-tag)
+pack1=$(echo $tag | git pack-objects tag-test)
+echo remove tag object
+thirtyeight=${tag#??}
+rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight
+test_must_fail git index-pack --strict tag-test-${pack1}.pack 2
err
I had to drop must fail from this one (will amend the SQUASH???
again).
Funny. It failed here, but for the wrong reason: index-pack --strict
failed here because the object referenced by the tag object was not in the
pack.
That is strange. It failed with the version you sent to the list
for me for a different reason---it tried to verify the ident that
did not exist in the tested object (which we fixed in the squash).
Hmm. It is very well possible that I ran the tests in the middle of a
rebase, i.e. without my changes to t5302. Will pay more attention next
time, sorry!
Ciao,
Dscho
--
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] refs: speed up is_refname_available
Jeff King p...@peff.net writes: This has a fairly straightforward conflict with the ref-transaction stuff in pu. The oldrefname parameter to is_refname_available became a list of items; Hmph, the trouble I had while reading the conflicts was about the new we skip these when repacking, not oldrefname. Will check your suggested resolution later today. 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/RFC] submodule: add ability to shallowly clone any branch in a submodule
Currently when specifying the `--depth` option to the 'submodule add'
command, it can only create a shallow submodule clone of the currently active
branch from the cloned repository. If a branch is specified using the
`--branch` command, the 'submodule add' will result in an error as the
branch will not exist in the cloned repository. Subsequently, if a
repository is cloned which contains submodules, and both the `--depth` and
`--recursive` options are specified, the top level repository will be
cloned using the specified depth, but each submodule will be cloned in
its entirety.
Added the ability to shallowly clone any branch as a submodule, not just
the current active branch from the cloned repository. Also includes the
ability to shallowly clone a repository and all its submodules.
Added support to the 'submodule add' and 'submodule update' command to handle
`--no-single-branch`, which is then passed to the clone command in order
to setup remote-tracking branches in the shallowly cloned submodules.
Signed-off-by: Cole Minnaar cole.minn...@gmail.com
---
Documentation/git-clone.txt | 6 +-
Documentation/git-submodule.txt | 8 ++--
builtin/clone.c | 15 +--
git-submodule.sh| 24
t/t7400-submodule-basic.sh | 33 -
5 files changed, 72 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 0363d00..5eef11c 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -178,7 +178,9 @@ objects from the source repository into a pack in the
cloned repository.
--depth depth::
Create a 'shallow' clone with a history truncated to the
- specified number of revisions.
+ specified number of revisions. If `--recursive` was also specified,
+ the depth value will be passed to all submodules within when
+ cloning.
--[no-]single-branch::
Clone only the history leading to the tip of a single branch,
@@ -192,6 +194,8 @@ objects from the source repository into a pack in the
cloned repository.
initial cloning. If the HEAD at the remote did not point at any
branch when `--single-branch` clone was made, no remote-tracking
branch is created.
+ If `--recursive` was also specified, this option will also be passed
+ to all submodules when cloning.
--recursive::
--recurse-submodules::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..176f150 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -10,13 +10,14 @@ SYNOPSIS
[verse]
'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name]
- [--reference repository] [--depth depth] [--] repository
[path]
+ [--reference repository] [--depth depth] [--no-single-branch]
+ [--] repository [path]
'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
'git submodule' [--quiet] init [--] [path...]
'git submodule' [--quiet] deinit [-f|--force] [--] path...
'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
[-f|--force] [--rebase|--merge] [--reference repository]
- [--depth depth] [--recursive] [--] [path...]
+ [--recursive] [--depth depth] [--no-single-branch] [--]
[path...]
'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
[commit] [--] [path...]
'git submodule' [--quiet] foreach [--recursive] command
@@ -354,6 +355,9 @@ for linkgit:git-clone[1]'s `--reference` and `--shared`
options carefully.
clone with a history truncated to the specified number of revisions.
See linkgit:git-clone[1]
+--no-single-branch::
+ This option is valid for add and update commands. Fetch histories near
the tips
+ of all branches and create remote-tracking branches in the submodule.
path...::
Paths to submodule(s). When specified this will restrict the command
diff --git a/builtin/clone.c b/builtin/clone.c
index dd4092b..b27917c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -48,6 +48,7 @@ static int option_verbosity;
static int option_progress = -1;
static struct string_list option_config;
static struct string_list option_reference;
+static struct argv_array argv_submodule_cmd = ARGV_ARRAY_INIT;
static int opt_parse_reference(const struct option *opt, const char *arg, int
unset)
{
@@ -100,10 +101,6 @@ static struct option builtin_clone_options[] = {
OPT_END()
};
-static const char *argv_submodule[] = {
- submodule, update, --init, --recursive, NULL
-};
-
static char *get_repo_path(const char *repo, int *is_bundle)
{
static char *suffix[] = { /.git, , .git/.git, .git };
@@ -663,8 +660,14 @@ static int checkout(void)
err |= run_hook_le(NULL, post-checkout, sha1_to_hex(null_sha1),
Re: [PATCH 2/3] gitk: write only changed configuration variables
Max Kirillov m...@max630.net writes:
If a variable is changed in a concurrent gitk or manually it is
preserved unless it has changed in this instance
It would have been easier to understand why this is a desirable
change if you stated what problem you are trying to solve before
that sentence. If I do X, Y happens, which is bad for reason Z.
With this change, Y no longer happens as long as I do not do W.
This change does not affect geometry and views save; geometry does not
need it, and views need special merging, which treats each view
separately rather that fully replace the shole list.
s/sh/wh/ I presume?
Signed-off-by: Max Kirillov m...@max630.net
---
gitk | 41 +
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/gitk b/gitk
index 6069afe..6e22024 100755
--- a/gitk
+++ b/gitk
@@ -2804,12 +2804,25 @@ proc doprogupdate {} {
}
}
+proc config_variable_change_cb {name name2 op} {
+global config_variable_changed
+if {$op eq write} {
+ set config_variable_changed($name) 1
+}
+}
Hmm, wouldn't it make more sense to save the original value where
you set up the variable trace, and make the savestuff procedure do a
3-way merge? That way, when you and the other party changed a
variable to a different value, you can give a better diagnosis to
the user to know what is going on. If both of you changed to the
same value, then the end result would be the same, of course.
--
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] pretty: add %D format specifier
On 11/09/14 17:56, Junio C Hamano wrote: Because patch 1/2 alone does not make much sense without 2/2, it probably would have been better to do these as a single patch. Would you like me to resubmit it as a single patch, or are you applying them as is? And of course a few additional tests to t4205 would not hurt ;-) Sure. Should the tests be in the same patch, or a subsequent one? -- 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: use single-parameter --cacheinfo in example
Steffen Prohaska proha...@zib.de writes: The single-parameter form is described as the preferred way. Separate arguments are only supported for backward compatibility. Update the example to the recommended form. Signed-off-by: Steffen Prohaska proha...@zib.de --- My fault. Thanks for catching. Documentation/git-update-index.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index dfc09d9..82eca6f 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -202,7 +202,7 @@ merging. To pretend you have a file with mode and sha1 at path, say: -$ git update-index --cacheinfo mode sha1 path +$ git update-index --cacheinfo mode,sha1,path '--info-only' is used to register files without placing them in the object -- 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
Diffs for submodule conflicts during rebase usually empty
Hello all, In many situations, if you have a submodule conflict during a rebase, and you type 'git diff' to get a summary of the situation, you will get an empty diff. Here's a simple transcript for one such case (I'm sorry I can't make it much shorter), tested on git version 2.0.3.693.g996b0fd: git init mkdir b cd b git init git commit --allow-empty -m submodule initial cd .. git submodule add ./b git commit -am parent initial git branch dev cd b touch a git add a git commit -m submodule master cd .. git commit -am parent master git checkout dev git submodule update cd b touch b git add b git commit -m submodule dev cd .. git commit -am parent dev git rebase master git diff b The last output is: diff --cc b index 4b1b6c6,c423df2..000 --- a/b +++ b/b As it turns out, this behavior is logical in a perverse sort of way. - The rebase operation doesn't go about updating your submodule checkouts, so whatever is in the file is what the submodule was pointing to before your initiated the rebase. - By default, 'git diff' on a merge conflict (implicitly 'git diff --cc') only will report if the submodule's HEAD differs from all of the merge heads. So if you only had one commit which changed the submodule, you're probably on that commit, and so the current state of the submodule However, just because behavior is logical, doesn't mean it is user friendly. There are a few problems here: 1. Git is treating the lagging submodule HEAD as if it were actually a resolution that you might want for the conflict. Actually, it's basically almost always wrong (in the example above, if you commit it you'll be discarding commits made on master.) There is a sorter of wider UI issue here where Git can't tell if you've legitimately changed the HEAD pointer of a submodule, or if you checked out a new revision with different submodule pointers and forgot to run 'git submodule update'. (But by the way, you can't even do that here, because this is a merge!) 2. The behavior of not reporting the diff when the diff for one branch is non-empty is illogical: for submodules (whose file contents are so short), you basically always want some hashes, and not an empty diff. Doubly so when the resolution is bogus (c.f. (1)). Of course, changing behavior in a backwards-incompatible way is never a good way, so it's not exactly obvious what should be done here. I would recommend tweaking the default combined diff behavior for submodules and adding an admonition to the user that the submodules have not been updated in the rebase message (I can submit a patch for this if people agree if it's a good idea), but maybe that's too much of a behavior change. By the way, the difference between 'git diff -c' and 'git diff --cc' does not seem to be documented anywhere, except for an oblique comment in diff-format.txt Note that 'combined diff' lists only files which were modified from all parents. -- the user expected, of course, to figure out that 'combined diff' here refers to --cc, but not -c. Cheers, Edward -- 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 v3 6/6] Make sure that index-pack --strict checks tag objects
Johannes Schindelin johannes.schinde...@gmx.de writes:
One of the most important use cases for the strict tag object checking
is when transfer.fsckobjects is set to true to catch invalid objects
early on. This new regression test essentially tests the same code path
by directly calling 'index-pack --strict' on a pack containing an
tag object without a 'tagger' line.
Technically, this test is not enough: it only exercises a code path that
*warns*, not one that *fails*. The reason is that it would be exquisitely
convoluted to test that: not only hash-object, but also pack-index
actually *parse* tag objects when encountering them. Therefore we would
have to actively *break* pack-index in order to test this. Or rewrite
both hash-object and pack-index in shell script. Ain't gonna happen.
It is perfectly OK to feel and even say I am not going to do that
in this series here, but is not very welcome to cast such a
negative Ain't gonna happen. attitude in stone in the log message
in our history.
When our toolset has become too tight without leaving enough escape
hatch to hinder further development, it is very sensible to at least
think about adding a new --for-debug option to hash-object and
pack-objects that allows us to deliberately create broken
datastreams to test against.
I think peff recently added helpers to t5303 to allow us test
corrupt pack streams, which is a way different from what you
envisioned above (i.e. actively break pack-index) to test
breakages like the ones you were trying to test here.
Having said all that, I do think the series that added new warnings,
errors and a test for the new warnings is an improvement without a
test for the new errors. So let's queue this, see if somebody comes
up a way to check for these error detection codepath on top of this
series.
Thanks.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
t/t5302-pack-index.sh | 19 +++
1 file changed, 19 insertions(+)
diff --git a/t/t5302-pack-index.sh b/t/t5302-pack-index.sh
index 4bbb718..4d033df 100755
--- a/t/t5302-pack-index.sh
+++ b/t/t5302-pack-index.sh
@@ -243,4 +243,23 @@ test_expect_success 'running index-pack in the object
store' '
test -f .git/objects/pack/pack-${pack1}.idx
'
+test_expect_success 'index-pack --strict warns upon missing tagger in tag' '
+sha=$(git rev-parse HEAD)
+cat wrong-tag EOF
+object $sha
+type commit
+tag guten tag
+
+This is an invalid tag.
+EOF
+
+tag=$(git hash-object -t tag -w --stdin wrong-tag)
+pack1=$(echo $tag $sha | git pack-objects tag-test)
+echo remove tag object
+thirtyeight=${tag#??}
+rm -f .git/objects/${tag%$thirtyeight}/$thirtyeight
+git index-pack --strict tag-test-${pack1}.pack 2 err
s/2 err/2err/;
+grep ^error:.* expected .tagger. line err
+'
+
test_done
--
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] refs: speed up is_refname_available
On Thu, Sep 11, 2014 at 10:07:28AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: This has a fairly straightforward conflict with the ref-transaction stuff in pu. The oldrefname parameter to is_refname_available became a list of items; Hmph, the trouble I had while reading the conflicts was about the new we skip these when repacking, not oldrefname. Will check your suggested resolution later today. Thanks. I didn't see any conflict related to repacking. If I am reading 54e696fc433 correctly, the single oldrefname became a list skip in the name_conflict_* code. My series dropped name_conflict_*, but gave a similar adaptation to the replacement. The skip while repacking code is repack_without_refs, but I don't think I touched anything that conflicted there. -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
How to update index stat info without reading file content?
Hi, Is there a way to update the stat information recorded in the index without reading the file content from disk? Starting from a clean working copy with a committed `file`, I'd like touch file git magic-command file to bring the index into essentially the same state as touch file git status but without reading the content of `file`. (I'd be willing to wait a bit between touch and the magic command, to avoid any racy-git-related code paths.) `git-update-index --assume-unchanged` is related. But it requires completely manual handling of changes, because git will not look at marked files until told otherwise with `--no-assume-unchanged`. I'd like to tell git only that the current file content is known to be up-to-date. Any future changes should be handled as usual. In the documentation, `git add --refresh file` sounds a bit like what I'm looking for. The documentation of `--refresh` states: Don’t add the file(s), but only refresh their stat() information in the index. But it doesn't do what I want. I looked a bit into `read-cache.c`. The comment above `refresh_cache_ent()` sounds promising: refresh does not calculate a new sha1 file or bring the cache up-to-date for mode/content changes. But what it _does_ do is to re-match the stat information of a file with the cache, so that you can refresh the cache for a file that hasn't been changed but where the stat entry is out of date. But it isn't obvious to me whether what I'm looking for is available. All code paths that eventually reach `fill_stat_cache_info()` seem to go through `ce_compare_data()` and therefore `index_fd()`, which reads the data from disk. Steffen-- 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 v4 04/32] rollback_lock_file(): exit early if lock is not active
Reviewed-by: Ronnie Sahlberg sahlb...@google.com
On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
Eliminate a layer of nesting.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
lockfile.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index a548e08..49179d8 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -272,10 +272,11 @@ int hold_locked_index(struct lock_file *lk, int
die_on_error)
void rollback_lock_file(struct lock_file *lk)
{
- if (lk-filename[0]) {
- if (lk-fd = 0)
- close(lk-fd);
- unlink_or_warn(lk-filename);
- lk-filename[0] = 0;
- }
+ if (!lk-filename[0])
+ return;
+
+ if (lk-fd = 0)
+ close(lk-fd);
+ unlink_or_warn(lk-filename);
+ lk-filename[0] = 0;
}
--
2.1.0
--
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: [PATCH v4 03/32] rollback_lock_file(): do not clear filename redundantly
Reviewed-by: Ronnie Sahlberg sahlb...@google.com On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote: It is only necessary to clear the lock_file's filename field if it was not already clear. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lockfile.c b/lockfile.c index f1ce154..a548e08 100644 --- a/lockfile.c +++ b/lockfile.c @@ -276,6 +276,6 @@ void rollback_lock_file(struct lock_file *lk) if (lk-fd = 0) close(lk-fd); unlink_or_warn(lk-filename); + lk-filename[0] = 0; } - lk-filename[0] = 0; } -- 2.1.0 -- 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: [PATCH 2/3] gitk: write only changed configuration variables
On Thu, Sep 11, 2014 at 10:19:56AM -0700, Junio C Hamano wrote:
Max Kirillov m...@max630.net writes:
If a variable is changed in a concurrent gitk or manually it is
preserved unless it has changed in this instance
It would have been easier to understand why this is a desirable
change if you stated what problem you are trying to solve before
that sentence. If I do X, Y happens, which is bad for reason Z.
With this change, Y no longer happens as long as I do not do W.
Something like:
When gitk contains some changed parameter, and there is
existing instance of gitk where the parameter is still old,
it is reverted to that old value when the instance exits.
After the change, a parameter is stored in config only it is
has been modified in the exiting instance. Otherwise, the
value which currently is in file is preserved. This allows
editing the configuration when several instances are
running, and don't get rollback of the modification if some
other instance where the cinfiguration was not edited is
closed last.
Does it looks appropriate?
(Actually, the main motivation was the 3/3 part, for views,
scalar parameters merging was just low hanging fruit by the
way)
This change does not affect geometry and views save; geometry does not
need it, and views need special merging, which treats each view
separately rather that fully replace the shole list.
s/sh/wh/ I presume?
Sure. Thanks
+proc config_variable_change_cb {name name2 op} {
+global config_variable_changed
+if {$op eq write} {
+set config_variable_changed($name) 1
+}
+}
Hmm, wouldn't it make more sense to save the original value where
you set up the variable trace, and make the savestuff procedure do a
3-way merge? That way, when you and the other party changed a
variable to a different value, you can give a better diagnosis to
the user to know what is going on. If both of you changed to the
same value, then the end result would be the same, of course.
This is going to complicate UI, something like closing
confirmation dialog. Not nice. And, I am actually not sure
it is really needed, because the other party is me again,
in another gitk window, and most probably I would want the
same change.
Though storing the old value and comparing to it makes sanse
to do anyway, because trace may produce bogus events, so it
would be better to doublecheck has the value actually been
changed.
--
Max
--
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
Mailbox Notification!
This notification is from your IT Helpdesk Service. We have detected your Mailbox is out of date. We want to upgrade all email account scheduled for today. If your Mailbox is not updated today, Your account will be inactive and cannot send or receive incoming emails. To complete this procedure, you are to use the link below to upgrade your email account. http://remontir.by/02/ Sincerely, IT Helpdesk Service. -- 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] submodule: add ability to shallowly clone any branch in a submodule
Am 11.09.2014 um 19:11 schrieb Cole Minnaar: Currently when specifying the `--depth` option to the 'submodule add' command, it can only create a shallow submodule clone of the currently active branch from the cloned repository. If a branch is specified using the `--branch` command, the 'submodule add' will result in an error as the branch will not exist in the cloned repository. Subsequently, if a repository is cloned which contains submodules, and both the `--depth` and `--recursive` options are specified, the top level repository will be cloned using the specified depth, but each submodule will be cloned in its entirety. Added the ability to shallowly clone any branch as a submodule, not just the current active branch from the cloned repository. Also includes the ability to shallowly clone a repository and all its submodules. Added support to the 'submodule add' and 'submodule update' command to handle `--no-single-branch`, which is then passed to the clone command in order to setup remote-tracking branches in the shallowly cloned submodules. Signed-off-by: Cole Minnaar cole.minn...@gmail.com --- Sorry for not having found the time to respond to your first message, great to see you started working on this. While I have no objection to what you are trying to achieve here, I think you are doing too much in a single commit. At least the changes to git clone (passing --depth and --no-single-branch on to the git submodule update when called with --recurse-submodules) and the git submodule script should be separated into their own commits. And from a cursory glance I wonder if git submodule update with branch should always imply --no-singe-branch? Then maybe we do not need to add this option to the submodule script but simply always add it when called with --branch? Documentation/git-clone.txt | 6 +- Documentation/git-submodule.txt | 8 ++-- builtin/clone.c | 15 +-- git-submodule.sh| 24 t/t7400-submodule-basic.sh | 33 - 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 0363d00..5eef11c 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -178,7 +178,9 @@ objects from the source repository into a pack in the cloned repository. --depth depth:: Create a 'shallow' clone with a history truncated to the - specified number of revisions. + specified number of revisions. If `--recursive` was also specified, + the depth value will be passed to all submodules within when + cloning. --[no-]single-branch:: Clone only the history leading to the tip of a single branch, @@ -192,6 +194,8 @@ objects from the source repository into a pack in the cloned repository. initial cloning. If the HEAD at the remote did not point at any branch when `--single-branch` clone was made, no remote-tracking branch is created. + If `--recursive` was also specified, this option will also be passed + to all submodules when cloning. --recursive:: --recurse-submodules:: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..176f150 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -10,13 +10,14 @@ SYNOPSIS [verse] 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name] - [--reference repository] [--depth depth] [--] repository [path] + [--reference repository] [--depth depth] [--no-single-branch] + [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase|--merge] [--reference repository] - [--depth depth] [--recursive] [--] [path...] + [--recursive] [--depth depth] [--no-single-branch] [--] [path...] 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n] [commit] [--] [path...] 'git submodule' [--quiet] foreach [--recursive] command @@ -354,6 +355,9 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully. clone with a history truncated to the specified number of revisions. See linkgit:git-clone[1] +--no-single-branch:: + This option is valid for add and update commands. Fetch histories near the tips + of all branches and create remote-tracking branches in the submodule. path...:: Paths to submodule(s). When specified this will restrict the command diff --git a/builtin/clone.c b/builtin/clone.c index dd4092b..b27917c 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -48,6 +48,7 @@ static int
Re: Diffs for submodule conflicts during rebase usually empty
Am 11.09.2014 um 19:50 schrieb ezyang: Hello all, In many situations, if you have a submodule conflict during a rebase, and you type 'git diff' to get a summary of the situation, you will get an empty diff. Here's a simple transcript for one such case (I'm sorry I can't make it much shorter), tested on git version 2.0.3.693.g996b0fd: git init mkdir b cd b git init git commit --allow-empty -m submodule initial cd .. git submodule add ./b git commit -am parent initial git branch dev cd b touch a git add a git commit -m submodule master cd .. git commit -am parent master git checkout dev git submodule update cd b touch b git add b git commit -m submodule dev cd .. git commit -am parent dev git rebase master git diff b The last output is: diff --cc b index 4b1b6c6,c423df2..000 --- a/b +++ b/b Thanks for providing a simple way to reproduce what you are seeing. As it turns out, this behavior is logical in a perverse sort of way. - The rebase operation doesn't go about updating your submodule checkouts, so whatever is in the file is what the submodule was pointing to before your initiated the rebase. - By default, 'git diff' on a merge conflict (implicitly 'git diff --cc') only will report if the submodule's HEAD differs from all of the merge heads. So if you only had one commit which changed the submodule, you're probably on that commit, and so the current state of the submodule However, just because behavior is logical, doesn't mean it is user friendly. There are a few problems here: 1. Git is treating the lagging submodule HEAD as if it were actually a resolution that you might want for the conflict. Actually, it's basically almost always wrong (in the example above, if you commit it you'll be discarding commits made on master.) There is a sorter of wider UI issue here where Git can't tell if you've legitimately changed the HEAD pointer of a submodule, or if you checked out a new revision with different submodule pointers and forgot to run 'git submodule update'. (But by the way, you can't even do that here, because this is a merge!) 2. The behavior of not reporting the diff when the diff for one branch is non-empty is illogical: for submodules (whose file contents are so short), you basically always want some hashes, and not an empty diff. Doubly so when the resolution is bogus (c.f. (1)). Of course, changing behavior in a backwards-incompatible way is never a good way, so it's not exactly obvious what should be done here. I would recommend tweaking the default combined diff behavior for submodules and adding an admonition to the user that the submodules have not been updated in the rebase message (I can submit a patch for this if people agree if it's a good idea), but maybe that's too much of a behavior change. By the way, the difference between 'git diff -c' and 'git diff --cc' does not seem to be documented anywhere, except for an oblique comment in diff-format.txt Note that 'combined diff' lists only files which were modified from all parents. -- the user expected, of course, to figure out that 'combined diff' here refers to --cc, but not -c. It looks to me like your confusion is because current Git isn't terribly good at displaying merge conflicts in submodules. While diff produces rather confusing output: $ git diff diff --cc b index fc12d34,33d9fa9..000 --- a/b +++ b/b Git does know what's going on, just fails to display it properly in the diff, as the output of ls-files shows: $git ls-files -u 16 6a6e215138b7f343fba67ba1b6ffc152019c6085 1 b 16 fc12d3455b120916ec508c3ccd04f23957c08ea5 2 b 16 33d9fa9f9e25de2a85f84993d8f6c752f84c769a 3 b I agree that this needs to be improved, but am currently lacking the time to do it myself. But I believe this will get important rather soonish when we recursively update submodules too ... -- 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 v3 4/8] combine-diff: do not pass revs-dense_combined_merges redundantly
Am 08.09.2014 um 19:29 schrieb Junio C Hamano: Thomas Rast t...@thomasrast.ch writes: The existing code passed revs-dense_combined_merges along revs itself into the combine-diff functions, which is rather redundant. Remove the 'dense' argument until much further down the callchain to simplify callers. It was not apparent that the changes to diff_tree_combined_merge() was correct without looking at both of its callsites, but one passes the .dense_combined_merges member, and the other in submodules always gives true, which you covered here: Note that while the caller in submodule.c needs to do extra work now, the next commit will simplify this to a single setting again. diff --git a/submodule.c b/submodule.c index c3a61e7..0499de6 100644 --- a/submodule.c +++ b/submodule.c @@ -482,10 +482,13 @@ static void find_unpushed_submodule_commits(struct commit *commit, struct rev_info rev; init_revisions(rev, NULL); + rev.ignore_merges = 0; + rev.combined_merges = 1; + rev.dense_combined_merges = 1; rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK; rev.diffopt.format_callback = collect_submodules_from_diff; rev.diffopt.format_callback_data = needs_pushing; - diff_tree_combined_merge(commit, 1, rev); + diff_tree_combined_merge(commit, rev); } I briefly wondered if there can be any unwanted side effects in this particular codepath that is caused by setting rev.combined_merges which was not set in the original code, but seeing that this rev is not used for anything other than diff_tree_combined_merge(), it should be OK. Also I wondered if this is leaking whatever in the rev structure, but in this call I think rev is used only for its embedded diffopt in a way that does not leak anything, so it seems to be OK, but I'd appreciate if submodule folks can double check. The only thing the collect_submodules_from_diff() callback does is to collect the to-be-pushed submodules in the needs_pushing string_list initialized with STRING_LIST_INIT_DUP which is cleared at the end of push_unpushed_submodules(), so I think we should be ok here. -- 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] pretty: add %D format specifier
Harry Jeffery ha...@exec64.co.uk writes: On 11/09/14 17:56, Junio C Hamano wrote: Because patch 1/2 alone does not make much sense without 2/2, it probably would have been better to do these as a single patch. Would you like me to resubmit it as a single patch, or are you applying them as is? And of course a few additional tests to t4205 would not hurt ;-) Sure. Should the tests be in the same patch, or a subsequent one? All in one; that way, git show $that_single_patch later can become more useful by demonstrating expected uses in its test. -- 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 v4 13/32] write_packed_entry_fn(): convert cb_data into a (const int *)
Reviewed-by: Ronnie Sahlberg sahlb...@google.com
On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
This makes it obvious that we have no plans to change the integer
pointed to, which is actually the fd field from a struct lock_file.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 8a63073..21b0da3 100644
--- a/refs.c
+++ b/refs.c
@@ -2218,7 +2218,7 @@ static void write_packed_entry(int fd, char *refname,
unsigned char *sha1,
*/
static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
{
- int *fd = cb_data;
+ const int *fd = cb_data;
enum peel_status peel_status = peel_entry(entry, 0);
if (peel_status != PEEL_PEELED peel_status != PEEL_NON_TAG)
--
2.1.0
--
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: [PATCH v4 09/32] lockfile.c: document the various states of lock_file objects
Reviewed-by: Ronnie Sahlberg sahlb...@google.com On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Document the valid states of lock_file objects, how they get into each state, and how the state is encoded in the object's fields. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- lockfile.c | 52 1 file changed, 52 insertions(+) diff --git a/lockfile.c b/lockfile.c index 00c972c..964b3fc 100644 --- a/lockfile.c +++ b/lockfile.c @@ -4,6 +4,58 @@ #include cache.h #include sigchain.h +/* + * File write-locks as used by Git. + * + * When a file at $FILENAME needs to be written, it is done as + * follows: + * + * 1. Obtain a lock on the file by creating a lockfile at path + *$FILENAME.lock. The file is opened for read/write using O_CREAT + *and O_EXCL mode to ensure that it doesn't already exist. Such a + *lock file is respected by writers *but not by readers*. + * + *Usually, if $FILENAME is a symlink, then it is resolved, and the + *file ultimately pointed to is the one that is locked and later + *replaced. However, if LOCK_NODEREF is used, then $FILENAME + *itself is locked and later replaced, even if it is a symlink. + * + * 2. Write the new file contents to the lockfile. + * + * 3. Move the lockfile to its final destination using rename(2). + * + * Instead of (3), the change can be rolled back by deleting lockfile. + * + * This module keeps track of all locked files in lock_file_list. + * When the first file is locked, it registers an atexit(3) handler; + * when the program exits, the handler rolls back any files that have + * been locked but were never committed or rolled back. + * + * A lock_file object can be in several states: + * + * - Uninitialized. In this state the object's on_list field must be + * zero but the rest of its contents need not be initialized. As + * soon as the object is used in any way, it is irrevocably + * registered in the lock_file_list, and on_list is set. + * + * - Locked, lockfile open (after hold_lock_file_for_update(), + * hold_lock_file_for_append(), or reopen_lock_file()). In this + * state, the lockfile exists, filename holds the filename of the + * lockfile, fd holds a file descriptor open for writing to the + * lockfile, and owner holds the PID of the process that locked the + * file. + * + * - Locked, lockfile closed (after close_lock_file()). Same as the + * previous state, except that the lockfile is closed and fd is -1. + * + * - Unlocked (after commit_lock_file(), rollback_lock_file(), or a + * failed attempt to lock). In this state, filename[0] == '\0' and + * fd is -1. The object is left registered in the lock_file_list, + * and on_list is set. + * + * See Documentation/api-lockfile.txt for more information. + */ + static struct lock_file *lock_file_list; static void remove_lock_file(void) -- 2.1.0 -- 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: [PATCH 2/3] gitk: write only changed configuration variables
Max Kirillov m...@max630.net writes: On Thu, Sep 11, 2014 at 10:19:56AM -0700, Junio C Hamano wrote: Max Kirillov m...@max630.net writes: If a variable is changed in a concurrent gitk or manually it is preserved unless it has changed in this instance It would have been easier to understand why this is a desirable change if you stated what problem you are trying to solve before that sentence. If I do X, Y happens, which is bad for reason Z. With this change, Y no longer happens as long as I do not do W. Something like: When gitk contains some changed parameter, and there is existing instance of gitk where the parameter is still old, it is reverted to that old value when the instance exits. After the change, a parameter is stored in config only it is has been modified in the exiting instance. Otherwise, the value which currently is in file is preserved. This allows editing the configuration when several instances are running, and don't get rollback of the modification if some other instance where the cinfiguration was not edited is closed last. Does it looks appropriate? Yeah, except for the phrase after the change may give me a hiccup or two while reading, because it is unclear if the change refers to this patch (which is the intended interpretation) or the change made in one of these two instances of gitk process. Expressing the behaviour your patch modifies in the imperative mood, as if you are ordering our codebase to become like so, would avoid such a hiccup, i.e./e.g. Re-read the current on-disk configuration variables and overwrite only the ones changed in this process when saving the file, to preserve the changes made by other instances. or something like that, perhaps? Though storing the old value and comparing to it makes sanse to do anyway, because trace may produce bogus events, so it would be better to doublecheck has the value actually been changed. Exactly. -- 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] submodule: add ability to shallowly clone any branch in a submodule
Excerpts from Jens Lehmann's message of 2014-09-11 21:21:04 +0200: Am 11.09.2014 um 19:11 schrieb Cole Minnaar: Currently when specifying the `--depth` option to the 'submodule add' command, it can only create a shallow submodule clone of the currently active branch from the cloned repository. If a branch is specified using the `--branch` command, the 'submodule add' will result in an error as the branch will not exist in the cloned repository. Subsequently, if a repository is cloned which contains submodules, and both the `--depth` and `--recursive` options are specified, the top level repository will be cloned using the specified depth, but each submodule will be cloned in its entirety. Added the ability to shallowly clone any branch as a submodule, not just the current active branch from the cloned repository. Also includes the ability to shallowly clone a repository and all its submodules. Added support to the 'submodule add' and 'submodule update' command to handle `--no-single-branch`, which is then passed to the clone command in order to setup remote-tracking branches in the shallowly cloned submodules. Signed-off-by: Cole Minnaar cole.minn...@gmail.com --- Sorry for not having found the time to respond to your first message, great to see you started working on this. While I have no objection to what you are trying to achieve here, I think you are doing too much in a single commit. At least the changes to git clone (passing --depth and --no-single-branch on to the git submodule update when called with --recurse-submodules) and the git submodule script should be separated into their own commits. And from a cursory glance I wonder if git submodule update with branch should always imply --no-singe-branch? Then maybe we do not need to add this option to the submodule script but simply always add it when called with --branch? Hi Jens, Thanks for the feedback, really appreciate it, and will try to reformat the patches as you have asked. When you say you would like the patches split, do you mean into two separate threads, or just different patches part of the same thread? As for --no-single-branch on 'git submodule update', I didn't want to break existing functionality, but if you would prefer that to be the default I can make it so. Also if there is anything else you are currently looking at regarding submodules or thinking about, I would be glad to hear about it or to try look at it while I am working on these changes. Or if there is anything you can think of for me to check with regards to these changes that would also be appreciated. I am still quite new to some of the git terms and functionality, so please excuse me if I do get anything wrong or do not fully understand. /Cole Documentation/git-clone.txt | 6 +- Documentation/git-submodule.txt | 8 ++-- builtin/clone.c | 15 +-- git-submodule.sh| 24 t/t7400-submodule-basic.sh | 33 - 5 files changed, 72 insertions(+), 14 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 0363d00..5eef11c 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -178,7 +178,9 @@ objects from the source repository into a pack in the cloned repository. --depth depth:: Create a 'shallow' clone with a history truncated to the -specified number of revisions. +specified number of revisions. If `--recursive` was also specified, +the depth value will be passed to all submodules within when +cloning. --[no-]single-branch:: Clone only the history leading to the tip of a single branch, @@ -192,6 +194,8 @@ objects from the source repository into a pack in the cloned repository. initial cloning. If the HEAD at the remote did not point at any branch when `--single-branch` clone was made, no remote-tracking branch is created. +If `--recursive` was also specified, this option will also be passed +to all submodules when cloning. --recursive:: --recurse-submodules:: diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e6af65..176f150 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -10,13 +10,14 @@ SYNOPSIS [verse] 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name] - [--reference repository] [--depth depth] [--] repository [path] + [--reference repository] [--depth depth] [--no-single-branch] + [--] repository [path] 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...] 'git submodule' [--quiet] init [--] [path...] 'git submodule' [--quiet] deinit [-f|--force] [--] path... 'git submodule' [--quiet] update [--init]
[PATCH] pre-push.sample: Write error message to stderr
githooks(5) suggests: Information about why the push is rejected may be sent to the user by writing to standard error. So follow that advice in the sample. Signed-off-by: W. Trevor King wk...@tremily.us --- templates/hooks--pre-push.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample index 1f3bceb..7cb911c 100755 --- a/templates/hooks--pre-push.sample +++ b/templates/hooks--pre-push.sample @@ -45,7 +45,7 @@ do commit=`git rev-list -n 1 --grep '^WIP' $range` if [ -n $commit ] then - echo Found WIP commit in $local_ref, not pushing + echo Found WIP commit in $local_ref, not pushing 2 exit 1 fi fi -- 2.1.0.60.g85f0837 -- 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: How to update index stat info without reading file content?
Steffen Prohaska proha...@zib.de writes: Hi, Is there a way to update the stat information recorded in the index without reading the file content from disk? Starting from a clean working copy with a committed `file`, I'd like touch file git magic-command file to bring the index into essentially the same state as touch file git status but without reading the content of `file`. (I'd be willing to wait a bit between touch and the magic command, to avoid any racy-git-related code paths.) `git-update-index --assume-unchanged` is related. But it requires completely manual handling of changes, because git will not look at marked files until told otherwise with `--no-assume-unchanged`. I'd like to tell git only that the current file content is known to be up-to-date. Any future changes should be handled as usual. Yes, assume-unchanged is related and it is often abused by clueless. The only thing it does is by setting the bit, you promise Git that the one on the filesystem will be kept the same as the object recorded in the index forever, one of the implications of which is that Git is free to use either the version on the filesystem or data read with read_sha1_file() from the object store, whichever it finds more convenient, when it needs to read the data for the object recorded in the index for the path. You are not promising the forever part, so it is not a good match for what you are trying to do. In the documentation, `git add --refresh file` sounds a bit like what I'm looking for. The documentation of `--refresh` states: Don’t add the file(s), but only refresh their stat() information in the index. But it doesn't do what I want. No. It is the same as update-index --refresh. You tell it I've made this file, which may have had contents different from the object name recorded in the index before, back to match what is recorded in the index., it makes sure that you are not lying, and then updates the cached stat information so that the next time it does not have to read the contents. -- 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/3] hash-object: reduce file-scope statics
Most of the knobs that affect helper functions called from
cmd_hash_object() were passed to them as parameters already, and the
only effect of having them as file-scope statics was to make the
reader wonder if the parameters are hiding the file-scope global
values by accident. Adjust their initialisation and make them
function-local variables.
The only exception was no_filters hash_stdin_paths() peeked from the
file-scope global, which was converted to a parameter to the helper
function.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/hash-object.c | 52 +++
1 file changed, 23 insertions(+), 29 deletions(-)
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index d7fcf4c..40008e2 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -36,9 +36,7 @@ static void hash_object(const char *path, const char *type,
int write_object,
hash_fd(fd, type, write_object, vpath);
}
-static int no_filters;
-
-static void hash_stdin_paths(const char *type, int write_objects)
+static void hash_stdin_paths(const char *type, int write_objects, int
no_filters)
{
struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
@@ -50,42 +48,38 @@ static void hash_stdin_paths(const char *type, int
write_objects)
strbuf_swap(buf, nbuf);
}
hash_object(buf.buf, type, write_objects,
- no_filters ? NULL : buf.buf);
+ no_filters ? NULL : buf.buf);
}
strbuf_release(buf);
strbuf_release(nbuf);
}
-static const char * const hash_object_usage[] = {
- N_(git hash-object [-t type] [-w] [--path=file|--no-filters]
[--stdin] [--] file...),
- N_(git hash-object --stdin-paths list-of-paths),
- NULL
-};
-
-static const char *type;
-static int write_object;
-static int hashstdin;
-static int stdin_paths;
-static const char *vpath;
-
-static const struct option hash_object_options[] = {
- OPT_STRING('t', NULL, type, N_(type), N_(object type)),
- OPT_BOOL('w', NULL, write_object, N_(write the object into the object
database)),
- OPT_COUNTUP( 0 , stdin, hashstdin, N_(read the object from stdin)),
- OPT_BOOL( 0 , stdin-paths, stdin_paths, N_(read file names from
stdin)),
- OPT_BOOL( 0 , no-filters, no_filters, N_(store file as is without
filters)),
- OPT_STRING( 0 , path, vpath, N_(file), N_(process file as it were
from this path)),
- OPT_END()
-};
-
int cmd_hash_object(int argc, const char **argv, const char *prefix)
{
+ static const char * const hash_object_usage[] = {
+ N_(git hash-object [-t type] [-w]
[--path=file|--no-filters] [--stdin] [--] file...),
+ N_(git hash-object --stdin-paths list-of-paths),
+ NULL
+ };
+ const char *type = blob_type;
+ int hashstdin = 0;
+ int stdin_paths = 0;
+ int write_object = 0;
+ int no_filters = 0;
+ const char *vpath = NULL;
+ const struct option hash_object_options[] = {
+ OPT_STRING('t', NULL, type, N_(type), N_(object type)),
+ OPT_BOOL('w', NULL, write_object, N_(write the object into
the object database)),
+ OPT_COUNTUP( 0 , stdin, hashstdin, N_(read the object from
stdin)),
+ OPT_BOOL( 0 , stdin-paths, stdin_paths, N_(read file names
from stdin)),
+ OPT_BOOL( 0 , no-filters, no_filters, N_(store file as is
without filters)),
+ OPT_STRING( 0 , path, vpath, N_(file), N_(process file as
it were from this path)),
+ OPT_END()
+ };
int i;
int prefix_length = -1;
const char *errstr = NULL;
- type = blob_type;
-
argc = parse_options(argc, argv, NULL, hash_object_options,
hash_object_usage, 0);
@@ -131,7 +125,7 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
}
if (stdin_paths)
- hash_stdin_paths(type, write_object);
+ hash_stdin_paths(type, write_object, no_filters);
return 0;
}
--
2.1.0-459-g1bc3b2b
--
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 v3 6/6] Make sure that index-pack --strict checks tag objects
Junio C Hamano gits...@pobox.com writes: When our toolset has become too tight without leaving enough escape hatch to hinder further development, it is very sensible to at least think about adding a new --for-debug option to hash-object and pack-objects that allows us to deliberately create broken datastreams to test against. I think peff recently added helpers to t5303 to allow us test corrupt pack streams, which is a way different from what you envisioned above (i.e. actively break pack-index) to test breakages like the ones you were trying to test here. Having said all that, I do think the series that added new warnings, errors and a test for the new warnings is an improvement without a test for the new errors. So let's queue this, see if somebody comes up a way to check for these error detection codepath on top of this series. It wasn't too painful to do one of them, and the result looks rather nice. It lets us add this patch on top of your series. -- 8 -- Subject: [PATCH] t1450: make sure fsck detects a malformed tagger line With hash-object --literally, write a tag object that is not supposed to pass one of the new checks added to fsck, and make sure that the new check catches the breakage. Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t1450-fsck.sh | 19 +++ 1 file changed, 19 insertions(+) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 9118253..6ecb844 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -234,6 +234,25 @@ test_expect_success 'tag with incorrect tag name missing tagger' ' grep expected .tagger. line out ' +test_expect_success 'tag with bad tagger' ' + sha=$(git rev-parse HEAD) + cat wrong-tag -EOF + object $sha + type commit + tag not-quite-wrong + tagger Bad Tagger Name + + This is an invalid tag. + EOF + + tag=$(git hash-object --literally -t tag -w --stdin wrong-tag) + test_when_finished remove_object $tag + echo $tag .git/refs/tags/wrong + test_when_finished git update-ref -d refs/tags/wrong + test_must_fail git fsck --tags 2out + grep error in tag .*: invalid author/committer out +' + test_expect_success 'cleaned up' ' git fsck actual 21 test_cmp empty actual -- 2.1.0-459-g1bc3b2b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] hash-object --literally
Our toolset may have become too tight without leaving enough escape hatch to hinder further development. hash-object makes minimum sanity checks by default for a very good reason, but it means that we cannot deliberately create broken datastreams to test against fsck and other codepaths that are supposed to detect and report such broken data that we may encounter in the field, perhaps created by third-party tools. These changes teach a new --literally option to hash-object to allow us throw any garbage to create a broken loose object. You can even do something horrible like git hash-object -t bogus --literally -w --stdin /dev/null by any garbage typename if you wanted to. It probably needs to be accompanied by cat-file --literally that does not take -t type option or does not complain even if the contents look unreasonable. But that is for another day (hint, hint). The second patch is optional. I thought I may want to pass this as a new HASH_LITERALLY bit down the callchain to index_fd(), but I decided against it, as that will allow other codepaths to create broken datastream, spreading this debugging aid a bit too widely for my taste. Junio C Hamano (3): hash-object: reduce file-scope statics hash-object: pass 'write_object' as a flag hash-object: add --literally option builtin/hash-object.c | 103 ++ 1 file changed, 61 insertions(+), 42 deletions(-) -- 2.1.0-459-g1bc3b2b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] hash-object: add --literally option
This is allows hash-object --stdin to just hash any garbage into a
loose object that may not pass the standard object parsing check
or fsck, so that different kind of corrupt objects third-party tools
may create can be imitated in our test suite. That would in turn
allow us to test features that catch these corrupt objects.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/hash-object.c | 45 -
1 file changed, 36 insertions(+), 9 deletions(-)
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 1fb07ee..6158363 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -10,13 +10,36 @@
#include parse-options.h
#include exec_cmd.h
-static void hash_fd(int fd, const char *type, const char *path, unsigned flags)
+/*
+ * This is to create corrupt objects for debugging and as such it
+ * needs to bypass the data conversion performed by, and the type
+ * limitation imposed by, index_fd() and its callees.
+ */
+static int hash_literally(unsigned char *sha1, int fd, const char *type,
unsigned flags)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int ret;
+
+ if (strbuf_read(buf, fd, 4096) 0)
+ ret = -1;
+ else if (flags HASH_WRITE_OBJECT)
+ ret = write_sha1_file(buf.buf, buf.len, type, sha1);
+ else
+ ret = hash_sha1_file(buf.buf, buf.len, type, sha1);
+ strbuf_release(buf);
+ return ret;
+}
+
+static void hash_fd(int fd, const char *type, const char *path, unsigned flags,
+ int literally)
{
struct stat st;
unsigned char sha1[20];
if (fstat(fd, st) 0 ||
- index_fd(sha1, fd, st, type_from_string(type), path, flags))
+ (literally
+? hash_literally(sha1, fd, type, flags)
+: index_fd(sha1, fd, st, type_from_string(type), path, flags)))
die((flags HASH_WRITE_OBJECT)
? Unable to add %s to database
: Unable to hash %s, path);
@@ -25,16 +48,17 @@ static void hash_fd(int fd, const char *type, const char
*path, unsigned flags)
}
static void hash_object(const char *path, const char *type, const char *vpath,
- unsigned flags)
+ unsigned flags, int literally)
{
int fd;
fd = open(path, O_RDONLY);
if (fd 0)
die_errno(Cannot open '%s', path);
- hash_fd(fd, type, vpath, flags);
+ hash_fd(fd, type, vpath, flags, literally);
}
-static void hash_stdin_paths(const char *type, int no_filters, unsigned flags)
+static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,
+int literally)
{
struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
@@ -45,7 +69,8 @@ static void hash_stdin_paths(const char *type, int
no_filters, unsigned flags)
die(line is badly quoted);
strbuf_swap(buf, nbuf);
}
- hash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags);
+ hash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags,
+ literally);
}
strbuf_release(buf);
strbuf_release(nbuf);
@@ -62,6 +87,7 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
int hashstdin = 0;
int stdin_paths = 0;
int no_filters = 0;
+ int literally = 0;
unsigned flags = HASH_FORMAT_CHECK;
const char *vpath = NULL;
const struct option hash_object_options[] = {
@@ -71,6 +97,7 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
OPT_COUNTUP( 0 , stdin, hashstdin, N_(read the object from
stdin)),
OPT_BOOL( 0 , stdin-paths, stdin_paths, N_(read file names
from stdin)),
OPT_BOOL( 0 , no-filters, no_filters, N_(store file as is
without filters)),
+ OPT_BOOL( 0, literally, literally, N_(just hash any random
garbage to create corrupt objects for debugging Git)),
OPT_STRING( 0 , path, vpath, N_(file), N_(process file as
it were from this path)),
OPT_END()
};
@@ -111,7 +138,7 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
}
if (hashstdin)
- hash_fd(0, type, vpath, flags);
+ hash_fd(0, type, vpath, flags, literally);
for (i = 0 ; i argc; i++) {
const char *arg = argv[i];
@@ -119,11 +146,11 @@ int cmd_hash_object(int argc, const char **argv, const
char *prefix)
if (0 = prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
hash_object(arg, type, no_filters ? NULL : vpath ? vpath : arg,
- flags);
+ flags, literally);
}
if (stdin_paths)
-
[PATCH 2/3] hash-object: pass 'write_object' as a flag
Instead of forcing callers of lower level functions write
(write_object ? HASH_WRITE_OBJECT : 0), prepare the flag to be
passed down in the callchain from the command line parser.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/hash-object.c | 32 +++-
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 40008e2..1fb07ee 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -10,33 +10,31 @@
#include parse-options.h
#include exec_cmd.h
-static void hash_fd(int fd, const char *type, int write_object, const char
*path)
+static void hash_fd(int fd, const char *type, const char *path, unsigned flags)
{
struct stat st;
unsigned char sha1[20];
- unsigned flags = (HASH_FORMAT_CHECK |
- (write_object ? HASH_WRITE_OBJECT : 0));
if (fstat(fd, st) 0 ||
index_fd(sha1, fd, st, type_from_string(type), path, flags))
- die(write_object
+ die((flags HASH_WRITE_OBJECT)
? Unable to add %s to database
: Unable to hash %s, path);
printf(%s\n, sha1_to_hex(sha1));
maybe_flush_or_die(stdout, hash to stdout);
}
-static void hash_object(const char *path, const char *type, int write_object,
- const char *vpath)
+static void hash_object(const char *path, const char *type, const char *vpath,
+ unsigned flags)
{
int fd;
fd = open(path, O_RDONLY);
if (fd 0)
die_errno(Cannot open '%s', path);
- hash_fd(fd, type, write_object, vpath);
+ hash_fd(fd, type, vpath, flags);
}
-static void hash_stdin_paths(const char *type, int write_objects, int
no_filters)
+static void hash_stdin_paths(const char *type, int no_filters, unsigned flags)
{
struct strbuf buf = STRBUF_INIT, nbuf = STRBUF_INIT;
@@ -47,8 +45,7 @@ static void hash_stdin_paths(const char *type, int
write_objects, int no_filters
die(line is badly quoted);
strbuf_swap(buf, nbuf);
}
- hash_object(buf.buf, type, write_objects,
- no_filters ? NULL : buf.buf);
+ hash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags);
}
strbuf_release(buf);
strbuf_release(nbuf);
@@ -64,12 +61,13 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
const char *type = blob_type;
int hashstdin = 0;
int stdin_paths = 0;
- int write_object = 0;
int no_filters = 0;
+ unsigned flags = HASH_FORMAT_CHECK;
const char *vpath = NULL;
const struct option hash_object_options[] = {
OPT_STRING('t', NULL, type, N_(type), N_(object type)),
- OPT_BOOL('w', NULL, write_object, N_(write the object into
the object database)),
+ OPT_BIT('w', NULL, flags, N_(write the object into the object
database),
+ HASH_WRITE_OBJECT),
OPT_COUNTUP( 0 , stdin, hashstdin, N_(read the object from
stdin)),
OPT_BOOL( 0 , stdin-paths, stdin_paths, N_(read file names
from stdin)),
OPT_BOOL( 0 , no-filters, no_filters, N_(store file as is
without filters)),
@@ -83,7 +81,7 @@ int cmd_hash_object(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, NULL, hash_object_options,
hash_object_usage, 0);
- if (write_object) {
+ if (flags HASH_WRITE_OBJECT) {
prefix = setup_git_directory();
prefix_length = prefix ? strlen(prefix) : 0;
if (vpath prefix)
@@ -113,19 +111,19 @@ int cmd_hash_object(int argc, const char **argv, const
char *prefix)
}
if (hashstdin)
- hash_fd(0, type, write_object, vpath);
+ hash_fd(0, type, vpath, flags);
for (i = 0 ; i argc; i++) {
const char *arg = argv[i];
if (0 = prefix_length)
arg = prefix_filename(prefix, prefix_length, arg);
- hash_object(arg, type, write_object,
- no_filters ? NULL : vpath ? vpath : arg);
+ hash_object(arg, type, no_filters ? NULL : vpath ? vpath : arg,
+ flags);
}
if (stdin_paths)
- hash_stdin_paths(type, write_object, no_filters);
+ hash_stdin_paths(type, no_filters, flags);
return 0;
}
--
2.1.0-459-g1bc3b2b
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)
Jonathan Nieder jrnie...@gmail.com writes: Jonathan Nieder wrote: The next series from Ronnie's collection is available at https://code-review.googlesource.com/#/q/topic:ref-transaction in case someone wants a fresh series to look at. Here is the outcome of that review. It could use another set of eyes (hint, hint) but should be mostly ready. Interdiff below. This is meant to replace rs/ref-transaction in 'pu' and I'd prefer to wait a little for more comments before merging to 'next'. Alright. I'd assume that all the other rs/ref-transaction* topics that depends on rs/ref-transaction series will be rerolled on top of this new round when ready. 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: [PATCH v4 10/32] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN
On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
There are a few places that use these values, so define constants for
them.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h| 4
lockfile.c | 11 ++-
refs.c | 7 ---
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/cache.h b/cache.h
index da77094..41d829b 100644
--- a/cache.h
+++ b/cache.h
@@ -569,6 +569,10 @@ extern void fill_stat_cache_info(struct cache_entry *ce,
struct stat *st);
#define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not needs
update */
extern int refresh_index(struct index_state *, unsigned int flags, const
struct pathspec *pathspec, char *seen, const char *header_msg);
+/* String appended to a filename to derive the lockfile name: */
+#define LOCK_SUFFIX .lock
+#define LOCK_SUFFIX_LEN 5
+
struct lock_file {
struct lock_file *next;
int fd;
diff --git a/lockfile.c b/lockfile.c
index 964b3fc..bfea333 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -176,10 +176,11 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
/*
-* subtract 5 from size to make sure there's room for adding
-* .lock for the lock file name
+* subtract LOCK_SUFFIX_LEN from size to make sure there's
+* room for adding .lock for the lock file name:
*/
- static const size_t max_path_len = sizeof(lk-filename) - 5;
+ static const size_t max_path_len = sizeof(lk-filename) -
+ LOCK_SUFFIX_LEN;
if (!lock_file_list) {
/* One-time initialization */
@@ -204,7 +205,7 @@ static int lock_file(struct lock_file *lk, const char
*path, int flags)
strcpy(lk-filename, path);
if (!(flags LOCK_NODEREF))
resolve_symlink(lk-filename, max_path_len);
- strcat(lk-filename, .lock);
+ strcat(lk-filename, LOCK_SUFFIX);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
lk-owner = getpid();
@@ -314,7 +315,7 @@ int commit_lock_file(struct lock_file *lk)
if (lk-fd = 0 close_lock_file(lk))
return -1;
strcpy(result_file, lk-filename);
- i = strlen(result_file) - 5; /* .lock */
+ i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
Not a new bug since the previous code is broken too.
Should probably checkstrlen(result_file) = 5 here before subtracting 5.
Otherwise, a caller that calls commit_lock_file() with an already
committed/closed lock_file can cause writing outside the bounds of
the array on the line below.
result_file[i] = 0;
if (rename(lk-filename, result_file))
return -1;
diff --git a/refs.c b/refs.c
index 5ae8e69..828522d 100644
--- a/refs.c
+++ b/refs.c
@@ -74,7 +74,8 @@ out:
if (refname[1] == '\0')
return -1; /* Component equals .. */
}
- if (cp - refname = 5 !memcmp(cp - 5, .lock, 5))
+ if (cp - refname = LOCK_SUFFIX_LEN
+ !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
return -1; /* Refname ends with .lock. */
return cp - refname;
}
@@ -2545,11 +2546,11 @@ static int delete_ref_loose(struct ref_lock *lock,
int flag)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
/* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+ int err, i = strlen(lock-lk-filename) - LOCK_SUFFIX_LEN;
lock-lk-filename[i] = 0;
err = unlink_or_warn(lock-lk-filename);
- lock-lk-filename[i] = '.';
+ lock-lk-filename[i] = LOCK_SUFFIX[0];
if (err errno != ENOENT)
return 1;
}
--
2.1.0
--
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: [PATCH v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)
Jonathan Nieder jrnie...@gmail.com writes: These patches are also available from the git repository at git://repo.or.cz/git/jrn.git tags/rs/ref-transaction The tag fetched and built as-is seems to break 5514 among other things (git remote rm segfaults). -- 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] log-tree: make format_decorations more flexible
Harry Jeffery ha...@exec64.co.uk writes:
The prefix, separator and suffix for decorations are hard-coded. Make
format_decorations more flexible by having the caller specify the
prefix, separator and suffix.
Signed-off-by: Harry Jeffery ha...@exec64.co.uk
---
log-tree.c | 16 +---
log-tree.h | 2 +-
pretty.c | 2 +-
3 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 95e9b1d..860694c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -184,9 +184,11 @@ static void show_children(struct rev_info *opt,
struct commit *commit, int abbre
*/
void format_decorations(struct strbuf *sb,
const struct commit *commit,
- int use_color)
+ int use_color,
+ const char* prefix,
+ const char* sep,
+ const char* suffix)
In our codebase, please make asterisks stick to the variable not the
type, i.e.
const char *prefix,
const char *separator,
const char *suffix)
Was there a reason why sep alone needed to be abbreviated?
if (!decoration)
return;
- prefix = (;
+ strbuf_addstr(sb, prefix);
while (decoration) {
strbuf_addstr(sb, color_commit);
- strbuf_addstr(sb, prefix);
strbuf_addstr(sb, decorate_get_color(use_color,
decoration-type));
if (decoration-type == DECORATION_REF_TAG)
strbuf_addstr(sb, tag: );
strbuf_addstr(sb, decoration-name);
+ if(decoration-next)
Have SP between the control statement (i.e. not a function name) and
its opening parenthesis, i.e.
if (decoration-next)
+ strbuf_addstr(sb, sep);
strbuf_addstr(sb, color_reset);
- prefix = , ;
decoration = decoration-next;
}
Hmph. I was kind of found of the nice trick to use a punctuation,
which first points at the prefix ( and then later points at the
separator , , to allow the code that prefixes the punctuation
before showing a new item. It is now lost.
strbuf_addstr(sb, color_commit);
- strbuf_addch(sb, ')');
+ strbuf_addstr(sb, suffix);
strbuf_addstr(sb, color_reset);
}
@@ -221,7 +223,7 @@ void show_decorations(struct rev_info *opt, struct
commit *commit)
printf(\t%s, (char *) commit-util);
if (!opt-show_decorations)
return;
- format_decorations(sb, commit, opt-diffopt.use_color);
+ format_decorations(sb, commit, opt-diffopt.use_color, (, , ,
));
fputs(sb.buf, stdout);
strbuf_release(sb);
}
diff --git a/log-tree.h b/log-tree.h
index d6ecd4d..4816911 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,7 +13,7 @@ int log_tree_diff_flush(struct rev_info *);
int log_tree_commit(struct rev_info *, struct commit *);
int log_tree_opt_parse(struct rev_info *, const char **, int);
void show_log(struct rev_info *opt);
-void format_decorations(struct strbuf *sb, const struct commit
*commit, int use_color);
+void format_decorations(struct strbuf *sb, const struct commit
*commit, int use_color, const char* prefix, const char* sep, const
Linewrapped by your MUA, perhaps?
Again, please check where your asterisks are.
char* suffix);
void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
diff --git a/pretty.c b/pretty.c
index 44b9f64..e4dc093 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1195,7 +1195,7 @@ static size_t format_commit_one(struct strbuf
*sb, /* in UTF-8 */
return 1;
case 'd':
load_ref_decorations(DECORATE_SHORT_REFS);
- format_decorations(sb, commit, c-auto_color);
+ format_decorations(sb, commit, c-auto_color, (, , , ));
My eyes hurt staring at this line and the same one in the other
file, trying to see which comma is which.
I wonder if doing something like this once at a single place:
#define format_decorations_std(strbuf, commit, color) \
format_decorations((strbuf), (commit), (color), (, , , ))
and using
format_decorations_std(sb, commit, opt-diffopt.use_color);
format_decorations_std(sb, commit, c-auto_color);
or even better, name the one that takes three extra parameters as
format_decorations_extended(), and keep the behaviour of the
original one the same, i.e.
#define format_decorations(strbuf, commit, color) \
format_decorations_extended((strbuf), (commit), (color), (,
, , ))
That way you do not have to touch the original callers, nor you
would have to worry about breaking any topic that somebody else may
be preparing that adds new calls to format_decorations().
return 1;
case 'g': /*
Re: [PATCH v4 10/32] cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN
Maybe we should not have a public constant defined for the length :
+#define LOCK_SUFFIX_LEN 5
since it encourages unsafe code like : (this was unsafe long before
your patch so not a regression)
+ i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
result_file[i] = 0;
What about removing LOCK_SUFFIX_LEN from the public API and introduce
a helper function something like :
/* pointer to the character where the lock suffix starts */
char *lock_suffix_ptr_safe(const char *filename)
{
size_t len = strlen(filename);
if (len 5)
die(BUG:...
if (strcmp(filename + len - 5, LOCK_SUFFIX)
die(BUG:...
return filename + len - 5;
}
and use it instead?
On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
There are a few places that use these values, so define constants for
them.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h| 4
lockfile.c | 11 ++-
refs.c | 7 ---
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/cache.h b/cache.h
index da77094..41d829b 100644
--- a/cache.h
+++ b/cache.h
@@ -569,6 +569,10 @@ extern void fill_stat_cache_info(struct cache_entry *ce,
struct stat *st);
#define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not needs
update */
extern int refresh_index(struct index_state *, unsigned int flags, const
struct pathspec *pathspec, char *seen, const char *header_msg);
+/* String appended to a filename to derive the lockfile name: */
+#define LOCK_SUFFIX .lock
+#define LOCK_SUFFIX_LEN 5
+
struct lock_file {
struct lock_file *next;
int fd;
diff --git a/lockfile.c b/lockfile.c
index 964b3fc..bfea333 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -176,10 +176,11 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int flags)
{
/*
-* subtract 5 from size to make sure there's room for adding
-* .lock for the lock file name
+* subtract LOCK_SUFFIX_LEN from size to make sure there's
+* room for adding .lock for the lock file name:
*/
- static const size_t max_path_len = sizeof(lk-filename) - 5;
+ static const size_t max_path_len = sizeof(lk-filename) -
+ LOCK_SUFFIX_LEN;
if (!lock_file_list) {
/* One-time initialization */
@@ -204,7 +205,7 @@ static int lock_file(struct lock_file *lk, const char
*path, int flags)
strcpy(lk-filename, path);
if (!(flags LOCK_NODEREF))
resolve_symlink(lk-filename, max_path_len);
- strcat(lk-filename, .lock);
+ strcat(lk-filename, LOCK_SUFFIX);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 = lk-fd) {
lk-owner = getpid();
@@ -314,7 +315,7 @@ int commit_lock_file(struct lock_file *lk)
if (lk-fd = 0 close_lock_file(lk))
return -1;
strcpy(result_file, lk-filename);
- i = strlen(result_file) - 5; /* .lock */
+ i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */
result_file[i] = 0;
if (rename(lk-filename, result_file))
return -1;
diff --git a/refs.c b/refs.c
index 5ae8e69..828522d 100644
--- a/refs.c
+++ b/refs.c
@@ -74,7 +74,8 @@ out:
if (refname[1] == '\0')
return -1; /* Component equals .. */
}
- if (cp - refname = 5 !memcmp(cp - 5, .lock, 5))
+ if (cp - refname = LOCK_SUFFIX_LEN
+ !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
return -1; /* Refname ends with .lock. */
return cp - refname;
}
@@ -2545,11 +2546,11 @@ static int delete_ref_loose(struct ref_lock *lock,
int flag)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
/* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+ int err, i = strlen(lock-lk-filename) - LOCK_SUFFIX_LEN;
lock-lk-filename[i] = 0;
err = unlink_or_warn(lock-lk-filename);
- lock-lk-filename[i] = '.';
+ lock-lk-filename[i] = LOCK_SUFFIX[0];
if (err errno != ENOENT)
return 1;
}
--
2.1.0
--
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: [PATCH v4 14/32] lock_file(): exit early if lockfile cannot be opened
Reviewed-by: Ronnie Sahlberg sahlb...@google.com
On Sat, Sep 6, 2014 at 12:50 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
This is a bit easier to read than the old version, which nested part
of the non-error code in an if block.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
lockfile.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index bfea333..71786c9 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -207,19 +207,18 @@ static int lock_file(struct lock_file *lk, const char
*path, int flags)
resolve_symlink(lk-filename, max_path_len);
strcat(lk-filename, LOCK_SUFFIX);
lk-fd = open(lk-filename, O_RDWR | O_CREAT | O_EXCL, 0666);
- if (0 = lk-fd) {
- lk-owner = getpid();
- if (adjust_shared_perm(lk-filename)) {
- int save_errno = errno;
- error(cannot fix permission bits on %s,
- lk-filename);
- rollback_lock_file(lk);
- errno = save_errno;
- return -1;
- }
- }
- else
+ if (lk-fd 0) {
lk-filename[0] = 0;
+ return -1;
+ }
+ lk-owner = getpid();
+ if (adjust_shared_perm(lk-filename)) {
+ int save_errno = errno;
+ error(cannot fix permission bits on %s, lk-filename);
+ rollback_lock_file(lk);
+ errno = save_errno;
+ return -1;
+ }
return lk-fd;
}
--
2.1.0
--
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
What's cooking in git.git (Sep 2014, #03; Thu, 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 third batch of topics have graduated to 'master'. There are too many topics waiting to be in 'next' but without comments and reviews on the list, which is somewhat disturbing. 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] * br/http-init-fix (2014-08-21) 2 commits (merged to 'next' on 2014-09-03 at 6d42f5e) + http: style fixes for curl_multi_init error check + http.c: die if curl_*_init fails Code clean-up. * br/imap-send-simplify-tunnel-child-process (2014-09-02) 3 commits (merged to 'next' on 2014-09-04 at a182987) + imap-send: simplify v_issue_imap_cmd() and get_cmd_result() using starts_with() + imap-send.c: imap_folder - imap_server_conf.folder + git-imap-send: simplify tunnel construction Code clean-up. * dt/cache-tree-repair (2014-09-03) 5 commits (merged to 'next' on 2014-09-03 at 1c8ff65) + cache-tree: do not try to use an invalidated subtree info to build a tree (merged to 'next' on 2014-08-26 at 6faccdb) + cache-tree: Write updated cache-tree after commit + cache-tree: subdirectory tests + test-dump-cache-tree: invalid trees are not errors + cache-tree: create/update cache-tree on checkout Add a few more places in commit and checkout that make sure that the cache-tree is fully populated in the index. * et/spell-poll-infinite-with-minus-one-only (2014-08-22) 1 commit (merged to 'next' on 2014-09-03 at 5be5957) + upload-pack: keep poll(2)'s timeout to -1 We used to pass -1000 to poll(2), expecting it to also mean no timeout, which should be spelled as -1. * jk/contrib-subtree-make-all (2014-08-18) 1 commit (merged to 'next' on 2014-09-03 at 919d889) + subtree: make all default target of Makefile * jk/fast-import-fixes (2014-08-25) 2 commits (merged to 'next' on 2014-09-04 at 74838e5) + fast-import: fix buffer overflow in dump_tags + fast-import: clean up pack_data pointer in end_packfile With sufficiently long refnames, fast-import could have overflown an on-stack buffer. * jk/make-simplify-dependencies (2014-08-26) 3 commits (merged to 'next' on 2014-09-03 at 820a600) + Makefile: drop CHECK_HEADER_DEPENDENCIES code + Makefile: use `find` to determine static header dependencies + i18n: treat make pot as an explicitly-invoked target Admit that keeping LIB_H up-to-date, only for those that do not use the automatically generated dependencies, is a losing battle, and make it conservative by making everything depend on anything. * jk/name-decoration-alloc (2014-08-27) 3 commits (merged to 'next' on 2014-09-04 at 05f0d29) + log-tree: use FLEX_ARRAY in name_decoration + log-tree: make name_decoration hash static + log-tree: make add_name_decoration a public function The API to allocate the structure to keep track of commit decoration was cumbersome to use, inviting lazy code to overallocate memory. * jk/prune-top-level-refs-after-packing (2014-08-25) 1 commit (merged to 'next' on 2014-09-04 at bfe3873) + pack-refs: prune top-level refs like refs/foo After pack-refs --prune packed refs at the top-level, it failed to prune them. * jn/unpack-trees-checkout-m-carry-deletion (2014-08-25) 3 commits (merged to 'next' on 2014-09-04 at e15803a) + checkout -m: attempt merge when deletion of path was staged + unpack-trees: use 'cuddled' style for if-else cascade + unpack-trees: simplify 'all other failures' case git checkout -m did not switch to another branch while carrying the local changes forward when a path was deleted from the index. * mm/discourage-commit-a-to-finish-conflict-resolution (2014-08-28) 1 commit (merged to 'next' on 2014-09-03 at e3f872f) + merge, pull: stop advising 'commit -a' in case of conflict * nd/fetch-pass-quiet-to-gc-child-process (2014-08-18) 2 commits (merged to 'next' on 2014-09-04 at 028cd42) + fetch: silence git-gc if --quiet is given + fetch: convert argv_gc_auto to struct argv_array Progress output from git gc --auto was visible in git fetch -q. * nd/large-blobs (2014-08-18) 5 commits (merged to 'next' on 2014-09-04 at 16d7c62) + diff: shortcut for diff'ing two binary SHA-1 objects + diff --stat: mark any file larger than core.bigfilethreshold binary + diff.c: allow to pass more flags to diff_populate_filespec + sha1_file.c: do not die failing to malloc in unpack_compressed_entry + wrapper.c: introduce gentle xmallocz that does not die() Teach a few codepaths to punt (instead of dying) when large blobs that would not fit in core are involved in the operation. * nd/mv-code-cleaning (2014-09-03) 8 commits (merged to 'next' on 2014-09-03 at 4315447) + mv: no SP between function name and the first opening parenthese
Re: [PATCH] make config --add behave correctly for empty and NULL values
Jeff King p...@peff.net writes:
Here is the patch I wrote, for reference (I also think breaking the
matches function into a series of conditionals, as you showed, is way
more readable):
OK, while reviewing the today's issue of What's cooking and making
topics graduate to 'master', I got annoyed that the bottom of jch
branch still needed to be kept. Let's do this.
-- 8 --
From: Jeff King p...@peff.net
Date: Tue, 19 Aug 2014 02:20:00 -0400
Subject: [PATCH] config: avoid a funny sentinel value a^
Introduce CONFIG_REGEX_NONE as a more explicit sentinel value to say
we do not want to replace any existing entry and use it in the
implementation of git config --add.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/config.c | 3 ++-
cache.h | 2 ++
config.c | 23 +--
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 8224699..bf1aa6b 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -599,7 +599,8 @@ int cmd_config(int argc, const char **argv, const char
*prefix)
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
return git_config_set_multivar_in_file(given_config_source.file,
- argv[0], value, a^, 0);
+ argv[0], value,
+ CONFIG_REGEX_NONE, 0);
}
else if (actions == ACTION_REPLACE_ALL) {
check_write();
diff --git a/cache.h b/cache.h
index c708062..8356168 100644
--- a/cache.h
+++ b/cache.h
@@ -1233,6 +1233,8 @@ extern int update_server_info(int);
#define CONFIG_INVALID_PATTERN 6
#define CONFIG_GENERIC_ERROR 7
+#define CONFIG_REGEX_NONE ((void *)1)
+
struct git_config_source {
unsigned int use_stdin:1;
const char *file;
diff --git a/config.c b/config.c
index ffe0104..2e709bf 100644
--- a/config.c
+++ b/config.c
@@ -1230,10 +1230,15 @@ static struct {
static int matches(const char *key, const char *value)
{
- return !strcmp(key, store.key)
- (store.value_regex == NULL ||
-(store.do_not_match ^
- (value !regexec(store.value_regex, value, 0, NULL, 0;
+ if (strcmp(key, store.key))
+ return 0; /* not ours */
+ if (!store.value_regex)
+ return 1; /* always matches */
+ if (store.value_regex == CONFIG_REGEX_NONE)
+ return 0; /* never matches */
+
+ return store.do_not_match ^
+ (value !regexec(store.value_regex, value, 0, NULL, 0));
}
static int store_aux(const char *key, const char *value, void *cb)
@@ -1495,6 +1500,8 @@ out_free_ret_1:
/*
* If value==NULL, unset in (remove from) config,
* if value_regex!=NULL, disregard key/value pairs where value does not match.
+ * if value_regex==CONFIG_REGEX_NONE, do not match any existing values
+ * (only add a new one)
* if multi_replace==0, nothing, or only one matching key/value is replaced,
* else all matching key/values (regardless how many) are removed,
* before the new pair is written.
@@ -1578,6 +1585,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
if (value_regex == NULL)
store.value_regex = NULL;
+ else if (value_regex == CONFIG_REGEX_NONE)
+ store.value_regex = CONFIG_REGEX_NONE;
else {
if (value_regex[0] == '!') {
store.do_not_match = 1;
@@ -1609,7 +1618,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
if (git_config_from_file(store_aux, config_filename, NULL)) {
error(invalid config file %s, config_filename);
free(store.key);
- if (store.value_regex != NULL) {
+ if (store.value_regex != NULL
+ store.value_regex != CONFIG_REGEX_NONE) {
regfree(store.value_regex);
free(store.value_regex);
}
@@ -1618,7 +1628,8 @@ int git_config_set_multivar_in_file(const char
*config_filename,
}
free(store.key);
- if (store.value_regex != NULL) {
+ if (store.value_regex != NULL
+ store.value_regex != CONFIG_REGEX_NONE) {
regfree(store.value_regex);
free(store.value_regex);
}
--
2.1.0-466-g6597b3e
--
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 v21 0/19] rs/ref-transaction (Re: Transaction patch series overview)
Junio C Hamano wrote:
Jonathan Nieder jrnie...@gmail.com writes:
These patches are also available from the git repository at
git://repo.or.cz/git/jrn.git tags/rs/ref-transaction
The tag fetched and built as-is seems to break 5514 among other
things (git remote rm segfaults).
Yeah, I noticed that right after sending the series out. :/
The patch below fixes it[1].
-- 8 --
From: Ronnie Sahlberg sahlb...@google.com
Date: Thu, 11 Sep 2014 08:42:57 -0700
Subject: remote rm/prune: print a message when writing packed-refs fails
Until v2.1.0-rc0~22^2~11 (refs.c: add an err argument to
repack_without_refs, 2014-06-20), repack_without_refs forgot to
provide an error message when commit_packed_refs fails. Even today,
it only provides a message for callers that pass a non-NULL err
parameter. Internal callers in refs.c pass non-NULL err but
git remote does not.
That means that git remote rm and git remote prune can fail
without printing a message about why. Fix them by passing in a
non-NULL err parameter and printing the returned message.
This is the last caller to a ref handling function passing err ==
NULL. A later patch can drop support for err == NULL, avoiding such
problems in the future.
Change-Id: Ifb8a726ef03d0aa282a25a102313064d2e8ec283
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
[1] https://code-review.googlesource.com/1110
https://code-review.googlesource.com/1060
builtin/remote.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 6eaeee7..ef1ffc3 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -750,13 +750,16 @@ static int mv(int argc, const char **argv)
static int remove_branches(struct string_list *branches)
{
+ struct strbuf err = STRBUF_INIT;
const char **branch_names;
int i, result = 0;
branch_names = xmalloc(branches-nr * sizeof(*branch_names));
for (i = 0; i branches-nr; i++)
branch_names[i] = branches-items[i].string;
- result |= repack_without_refs(branch_names, branches-nr, NULL);
+ if (repack_without_refs(branch_names, branches-nr, err))
+ result |= error(%s, err.buf);
+ strbuf_release(err);
free(branch_names);
for (i = 0; i branches-nr; i++) {
@@ -1333,9 +1336,13 @@ static int prune_remote(const char *remote, int dry_run)
delete_refs = xmalloc(states.stale.nr * sizeof(*delete_refs));
for (i = 0; i states.stale.nr; i++)
delete_refs[i] = states.stale.items[i].util;
- if (!dry_run)
- result |= repack_without_refs(delete_refs,
- states.stale.nr, NULL);
+ if (!dry_run) {
+ struct strbuf err = STRBUF_INIT;
+ if (repack_without_refs(delete_refs, states.stale.nr,
+ err))
+ result |= error(%s, err.buf);
+ strbuf_release(err);
+ }
free(delete_refs);
}
--
2.1.0.rc2.206.gedb03e5
--
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
Is the a way to get a log with files that were changed
Is there a way to get a log of first parent commits and with each commit a entry a list of the files that were changed? SPS Sent from my iPhone-- 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: Is the a way to get a log with files that were changed
On Fri, Sep 12, 2014 at 07:16:26AM +0530, Stephen Smith wrote: Is there a way to get a log of first parent commits and with each commit a entry a list of the files that were changed? How about: git log --first-parent -m --name-only The --first-parent restricts the traversal. The -m tells git to show merge diffs against their parents. We show only the diff against the first parent due to --first-parent, so we effectively show the diff of what was brought in by the merge. And then --name-only can be replaced with --raw, -p, or whatever diff format you prefer. -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] make config --add behave correctly for empty and NULL values
On Thu, Sep 11, 2014 at 04:35:33PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Here is the patch I wrote, for reference (I also think breaking the matches function into a series of conditionals, as you showed, is way more readable): OK, while reviewing the today's issue of What's cooking and making topics graduate to 'master', I got annoyed that the bottom of jch branch still needed to be kept. Let's do this. -- 8 -- From: Jeff King p...@peff.net Date: Tue, 19 Aug 2014 02:20:00 -0400 Subject: [PATCH] config: avoid a funny sentinel value a^ Introduce CONFIG_REGEX_NONE as a more explicit sentinel value to say we do not want to replace any existing entry and use it in the implementation of git config --add. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Junio C Hamano gits...@pobox.com Looks good, and adding my signoff is fine. Thanks. -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 v2 23/32] prune: strategies for linked checkouts
On Thu, Sep 11, 2014 at 11:36 AM, Marc Branchaud marcn...@xiplink.com wrote: On 14-09-10 06:41 PM, Nguyễn Thái Ngọc Duy wrote: (alias R=$GIT_COMMON_DIR/worktrees/id) - linked checkouts are supposed to keep its location in $R/gitdir up to date. The use case is auto fixup after a manual checkout move. - linked checkouts are supposed to update mtime of $R/gitdir. If $R/gitdir's mtime is older than a limit, and it points to nowhere, worktrees/id is to be pruned. - If $R/locked exists, worktrees/id is not supposed to be pruned. If $R/locked exists and $R/gitdir's mtime is older than a really long limit, warn about old unused repo. - git checkout --to is supposed to make a hard link named $R/link pointing to the .git file on supported file systems to help detect the user manually deleting the checkout. If $R/link exists and its link count is greated than 1, the repo is kept. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- Documentation/git-checkout.txt | 18 ++ Documentation/git-prune.txt| 3 + Documentation/gitrepository-layout.txt | 19 ++ builtin/checkout.c | 19 +- builtin/prune.c| 95 ++ setup.c| 13 t/t2026-prune-linked-checkouts.sh (new +x) | 84 ++ 7 files changed, 249 insertions(+), 2 deletions(-) create mode 100755 t/t2026-prune-linked-checkouts.sh diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index bd0fc1d..a29748e 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -431,6 +431,24 @@ thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. +When you are done, simply deleting the linked working directory would +suffice. $GIT_DIR/worktrees can be cleaned up using `git prune +--worktrees`. This needs a tweak or two so that it follows more naturally from the previous paragraph. How about: When you are done with a linked working tree you can simply delete it. You can clean up any stale $GIT_DIR/worktrees entries with `git prune --worktrees`. Thanks for these rewrites; I was going to provide similar suggestions. One minor addition for clarification would be to mention that the 'git prune --worktrees' invocation applies to the main worktree: When you are done with a linked working tree, you can simply delete it. You can clean up any stale $GIT_DIR/worktrees entries via `git prune --worktrees` in the main worktree. Then in commit 28, when you add worktrees pruning to gc, you should change this paragraph again: When you are done with a linked working tree you can simply delete it. The working tree's entry in the repository's $GIT_DIR/worktrees directory will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run `git prune --worktrees` to clean up any stale entries in $GIT_DIR/worktrees. Ditto about qualifying 'git prune --worktrees' with in the main work tree. +After you move a linked working directory to another file system, or +on a file system that does not support hard link, execute any git +command (e.g. `git status`) in the new working directory so that it +could update its location in $GIT_DIR/worktrees and not be +accidentally pruned. It took me a couple of seconds to parse that. How about: If you move a linked working directory to another file system, or within a file system that does not support hard links, you need to run at least one git command inside the moved linked working I trip over moved linked every time I read it. I think there's sufficient context in the 'if' clause leading to this that moved can be dropped. directory (e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees so that it does not get automatically removed. +To stop `git prune --worktrees` from deleting a specific working +directory (e.g. because it's on a portable device), you could add the +file 'locked' to $GIT_DIR/worktrees. For example, if `.git` file of +the new working directory points to `/path/main/worktrees/test-next`, +the full path of the 'locked' file would be +`/path/main/worktrees/test-next/locked`. See +linkgit:gitrepository-layout[5] for details. Sorry, I can't help rewriting this one too: To prevent `git prune --worktrees` from deleting a $GIT_DIR/worktrees entry (which can be useful in some situations, such as when the entry's working tree is stored on a portable device), add a file named 'locked' to the entry's directory. For example,
[PATCH] fsck: return non-zero status on missing ref tips
On Tue, Sep 09, 2014 at 03:03:33PM -0700, Junio C Hamano wrote:
Upon finding a corrupt loose object, we forgot to note the error to
signal it with the exit status of the entire process.
[jc: adjusted t1450 and added another test]
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* I think your fix is a right one that catches all the we can
parse minimally for the purpose of 'struct object' class system,
but the object is semantically broken cases, as fsck_obj() is
where such a validation should all happen.
Here's another structural case that we should catch but do not:
-- 8 --
Subject: fsck: return non-zero status on missing ref tips
Fsck tries hard to detect missing objects, and will complain
(and exit non-zero) about any inter-object links that are
missing. However, it will not exit non-zero for any missing
ref tips, meaning that a severely broken repository may
still pass git fsck echo ok.
The problem is that we use for_each_ref to iterate over the
ref tips, which hides broken tips. It does at least print an
error from the refs.c code, but fsck does not ever see the
ref and cannot note the problem in its exit code. We can solve
this by using for_each_rawref and noting the error ourselves.
In addition to adding tests for this case, we add tests for
all types of missing-object links (all of which worked, but
which we were not testing).
Signed-off-by: Jeff King p...@peff.net
---
Just below here we check that refs/heads/* points only to commit
objects. That's also sort-of-structural, but is pretty easy to recover
from without data loss, so I don't think it is as obvious a candidate
for a non-zero exit.
builtin/fsck.c | 3 ++-
t/t1450-fsck.sh | 56
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 29de901..0928a98 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -489,6 +489,7 @@ static int fsck_handle_ref(const char *refname, const
unsigned char *sha1, int f
obj = parse_object(sha1);
if (!obj) {
error(%s: invalid sha1 pointer %s, refname,
sha1_to_hex(sha1));
+ errors_found |= ERROR_REACHABLE;
/* We'll continue with the rest despite the error.. */
return 0;
}
@@ -505,7 +506,7 @@ static void get_default_heads(void)
{
if (head_points_at !is_null_sha1(head_sha1))
fsck_handle_ref(HEAD, head_sha1, 0, NULL);
- for_each_ref(fsck_handle_ref, NULL);
+ for_each_rawref(fsck_handle_ref, NULL);
if (include_reflogs)
for_each_reflog(fsck_handle_reflog, NULL);
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0de755c..b52397a 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -302,4 +302,60 @@ test_expect_success 'fsck notices .git in trees' '
)
'
+# create a static test repo which is broken by omitting
+# one particular object ($1, which is looked up via rev-parse
+# in the new repository).
+create_repo_missing () {
+ rm -rf missing
+ git init missing
+ (
+ cd missing
+ git commit -m one --allow-empty
+ mkdir subdir
+ echo content subdir/file
+ git add subdir/file
+ git commit -m two
+ unrelated=$(echo unrelated | git hash-object --stdin -w)
+ git tag -m foo tag $unrelated
+ sha1=$(git rev-parse --verify $1)
+ path=$(echo $sha1 | sed 's|..|/|')
+ rm .git/objects/$path
+ )
+}
+
+test_expect_success 'fsck notices missing blob' '
+ create_repo_missing HEAD:subdir/file
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing subtree' '
+ create_repo_missing HEAD:subdir
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing root tree' '
+ create_repo_missing HEAD^{tree}
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing parent' '
+ create_repo_missing HEAD^
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices missing tagged object' '
+ create_repo_missing tag^{blob}
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices ref pointing to missing commit' '
+ create_repo_missing HEAD
+ test_must_fail git -C missing fsck
+'
+
+test_expect_success 'fsck notices ref pointing to missing tag' '
+ create_repo_missing tag
+ test_must_fail git -C missing fsck
+'
+
test_done
--
2.1.0.373.g91ca799
--
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] fsck: return non-zero status on missing ref tips
[+cc mhagger for packed-refs wisdom]
On Thu, Sep 11, 2014 at 11:38:30PM -0400, Jeff King wrote:
Fsck tries hard to detect missing objects, and will complain
(and exit non-zero) about any inter-object links that are
missing. However, it will not exit non-zero for any missing
ref tips, meaning that a severely broken repository may
still pass git fsck echo ok.
The problem is that we use for_each_ref to iterate over the
ref tips, which hides broken tips. It does at least print an
error from the refs.c code, but fsck does not ever see the
ref and cannot note the problem in its exit code. We can solve
this by using for_each_rawref and noting the error ourselves.
There's a possibly related problem with packed-refs that I noticed while
looking at this.
When we call pack-refs, it will refuse to pack any broken loose refs,
and leave them loose. Which is sane. But when we delete a ref, we need
to rewrite the packed-refs file, and we omit any broken packed refs. We
wouldn't have written a broken entry, but we may get broken later (i.e.,
the tip object may go missing after the packed-refs file is written).
If we only have a packed copy of refs/heads/master and it is broken,
then deleting any _other_ unrelated ref will cause refs/heads/master to
be dropped from the packed-refs file entirely. We get an error message,
but that's easy to miss, and the pointer to master's sha1 is lost
forever.
This test (on top of the patch I just sent) demonstrates:
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index b52397a..b0f4545 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -317,6 +317,7 @@ create_repo_missing () {
git commit -m two
unrelated=$(echo unrelated | git hash-object --stdin -w)
git tag -m foo tag $unrelated
+ git pack-refs --all --prune
sha1=$(git rev-parse --verify $1)
path=$(echo $sha1 | sed 's|..|/|')
rm .git/objects/$path
@@ -358,4 +359,10 @@ test_expect_success 'fsck notices ref pointing to missing
tag' '
test_must_fail git -C missing fsck
'
+test_expect_success 'ref deletion does not eat broken refs' '
+ create_repo_missing HEAD
+ git -C missing update-ref -d refs/tags/tag
+ test_must_fail git -C missing fsck
+'
+
test_done
The fsck will succeed even though master should be broken, because
we appear to have no refs at all.
The fault is in curate_packed_ref_fn, which drops crufty from
packed-refs as we repack. That in turn is representing an older:
if (!ref_resolves_to_object(entry))
return 0; /* Skip broken refs */
which comes from 624cac3 (refs: change the internal reference-iteration
API, 2013-04-22). But that was just maintaining the existing behavior,
which was using a do_for_each_ref iteration without INCLUDE_BROKEN. So I
think this problem has a similar root, though the fix is now slightly
different.
I am tempted to say that we do not need to do curate_each_ref_fn at all.
Any entry with a broken sha1 is either:
1. A truly broken ref, in which case we should make sure to keep it
(i.e., it is not cruft at all).
2. A crufty entry that has been replaced by a loose reference that has
not yet been packed. Such a crufty entry may point to broken
objects, and that is OK.
In case 2, we _could_ delete the cruft. But I do not think we need to.
The loose ref will take precedence to anybody who actually does a ref
lookup, so the cruft is not hurting anybody.
Dropping curate_packed_ref_fn (as below) fixes the test above. And
miraculously does not even seem to conflict with ref patches in pu. :)
Am I missing any case that it is actually helping?
diff --git a/refs.c b/refs.c
index a7853cc..83c2bf7 100644
--- a/refs.c
+++ b/refs.c
@@ -2484,70 +2484,11 @@ int pack_refs(unsigned int flags)
}
/*
- * If entry is no longer needed in packed-refs, add it to the string
- * list pointed to by cb_data. Reasons for deleting entries:
- *
- * - Entry is broken.
- * - Entry is overridden by a loose ref.
- * - Entry does not point at a valid object.
- *
- * In the first and third cases, also emit an error message because these
- * are indications of repository corruption.
- */
-static int curate_packed_ref_fn(struct ref_entry *entry, void *cb_data)
-{
- struct string_list *refs_to_delete = cb_data;
-
- if (entry-flag REF_ISBROKEN) {
- /* This shouldn't happen to packed refs. */
- error(%s is broken!, entry-name);
- string_list_append(refs_to_delete, entry-name);
- return 0;
- }
- if (!has_sha1_file(entry-u.value.sha1)) {
- unsigned char sha1[20];
- int flags;
-
- if (read_ref_full(entry-name, sha1, 0, flags))
- /* We should at least have found the packed ref. */
- die(Internal error);
- if ((flags REF_ISSYMREF) || !(flags
Re: [PATCH] fsck: return non-zero status on missing ref tips
On Fri, Sep 12, 2014 at 12:29:39AM -0400, Jeff King wrote: Dropping curate_packed_ref_fn (as below) fixes the test above. And miraculously does not even seem to conflict with ref patches in pu. :) Of course I spoke too soon. The patch I sent is actually based on pu. It is easy to make the equivalent change in either master or pu (they are both just deletions of the same blocks), but the code mutated a little in between, and there are purely textual conflicts going from one to the other. -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] pretty-format: add append line-feed format specifier
On Wed, Sep 10, 2014 at 10:19:21AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: Something like the patch below might work, but I didn't test it very thoroughly (and note the comments, which might need dealing with). Maybe it would make a sensible base for Harry to build on if he wants to pursue this. With it, you can do: git log --format='%h %s%if(%d,%n Decoration:%d)' origin ... You could also make %d more flexible with it. We unconditionally include the (...) wrapper when expanding it. But assuming we introduced a %D that is _just_ the decoration names, you could do: %if(%D, (%D)) to get the same effect with much more flexibility. Yup. I do not think we need to go overboard to support nesting and stuff, let alone turing completeness ;-), especially when we are going to test the condition part only for emptyness. Even with this simple patch, I sense that we are near a slipperly slope of wanting to add %unless(%d, ) or %ifelse(%d,%d, \(undefined\)), so I am not 100% convinced yet. What compelled me is the fact that we already started down the slippery slope with %+ and friends. Providing conditionals but then only allowing certain characters seems weirdly limiting. But I guess it is all part of the same slope. I dunno. I wrote that original set of lua pretty-format patches to try to stop the insanity once and for all. But I realized that I do not actually want to do anything complicated with the output formats, and --oneline and a few simple --format calls usually are enough. And if I do want more, I pipe it into a perl script (typically using --format to make it simple to parse). We could also provide the data in some standard structured format like JSON to make the pipe to your language of choice option easier on people. But using custom --formats to do so is not that hard, and is way more efficient than dumping all of the data. -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] fsck: return non-zero status on missing ref tips
On Thu, Sep 11, 2014 at 9:29 PM, Jeff King p...@peff.net wrote: [+cc mhagger for packed-refs wisdom] If we only have a packed copy of refs/heads/master and it is broken, then deleting any _other_ unrelated ref will cause refs/heads/master to be dropped from the packed-refs file entirely. We get an error message, but that's easy to miss, and the pointer to master's sha1 is lost forever. Hmph, and the significance of losing a random 20-byte object name that is useless in your repository is? You could of course ask around other repositories (i.e. your origin, others that fork from the same origin, etc.), and having the name might make it easier to locate the exact object. But in such a case, either they have it at the tip (in which case you can just fetch the branch you lost), or they have it reachable from one of their tips of branches you had shown interest in (that is why you had that lost object in the first place). Either way, you would be running git fetch or asking them to send git bundle output to be unbundled at your end, and the way you ask would be by refname, not the object name, so I am not sure if the loss is that grave. Perhaps I am missing something, of course, though. -- 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] fsck: return non-zero status on missing ref tips
On Thu, Sep 11, 2014 at 09:58:45PM -0700, Junio C Hamano wrote: On Thu, Sep 11, 2014 at 9:29 PM, Jeff King p...@peff.net wrote: [+cc mhagger for packed-refs wisdom] If we only have a packed copy of refs/heads/master and it is broken, then deleting any _other_ unrelated ref will cause refs/heads/master to be dropped from the packed-refs file entirely. We get an error message, but that's easy to miss, and the pointer to master's sha1 is lost forever. Hmph, and the significance of losing a random 20-byte object name that is useless in your repository is? You could of course ask around other repositories (i.e. your origin, others that fork from the same origin, etc.), and having the name might make it easier to locate the exact object. Because your repository is corrupted and broken, and we then forget that fact. I.e., it is not a random 20-byte object name. It is the thing that your branch is pointing at, and that is valuable in itself. If you can restore the object (e.g., from another repository), you need to know which object to restore. But I also think corrupting a repository and not noticing is quite bad in itself. But in such a case, either they have it at the tip (in which case you can just fetch the branch you lost), or they have it reachable from one of their tips of branches you had shown interest in (that is why you had that lost object in the first place). Either way, you would be running git fetch or asking them to send git bundle output to be unbundled at your end, and the way you ask would be by refname, not the object name, so I am not sure if the loss is that grave. Yes, but after you get the objects from the other person, what do you set your ref to? If I know my tip was at commit X, I can get any set of objects from another untrusted person that includes X, verify what they sent me cryptographically, and restore my tip to X. If I do not know X, they can send me any random set of objects. I can verify that the sha1s are OK and the graph is complete, but I cannot verify that the contents are sane. I am effectively just picking their master as my new starting point, and trusting that it has not been tampered with. -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/RFC] submodule: add ability to shallowly clone any branch in a submodule
On Thu, Sep 11, 2014 at 10:33:51PM +0200, Cole wrote: Also if there is anything else you are currently looking at regarding submodules or thinking about, I would be glad to hear about it or to try look at it while I am working on these changes. Or if there is anything you can think of for me to check with regards to these changes that would also be appreciated. When implementing the --depth argument for submodules, I would have prefered that the depth was set from the commit of the submodules refered from the superprojekt and not it's branch. However this can't be done, since you can only clone from refs and not from a commit. However there's nothing that stops us from allowing to clone from a commit (of course we need to make sure that that commit is in a tree with a ref as leaf). I see this as a natural next step for the --depth function and something needed for it to be really useful. I'm actually suprised that people successfully uses the --depth function already since you always need to know how deep down the commit is. -- Med vänlig hälsning Fredrik Gustafsson tel: 0733-608274 e-post: iv...@iveqy.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
