Re: [PATCH v2] fetch-pack: don't try to fetch peel values with --all
On Wed, Jun 13, 2018 at 05:13:02PM -0400, Jeff King wrote: > On Tue, Jun 12, 2018 at 06:54:17PM +, Kirill Smelkov wrote: > > > > If an extra connection isn't a problem, you might be better off with > > > "git ls-remote", and then picking through the results for refs of > > > interest, and then "git fetch-pack" to actually get the pack. That's how > > > git-fetch worked when it was a shell script (e.g., see c3a200120d, the > > > last shell version). > > > > Yes, this is what I ended up doing: > > > > https://lab.nexedi.com/kirr/git-backup/commit/899103bf > > > > but for another reason - to avoid repeating for every fetched repository > > slow (in case of my "big" destination backup repository) quickfetch() > > checking in every spawned `git fetch`: git-backup can build index of > > objects we already have ourselves only once at startup, and then in > > fetch, after checking lsremote output, consult that index, and if we see > > we already have everything for an advertised reference - just avoid > > giving it to fetch-pack to process. It turns out for many pulled > > repositories there is usually no references changed at all and this way > > fetch-pack can be skipped completely: > > > > https://lab.nexedi.com/kirr/git-backup/commit/3efed898 > > Thanks for sharing that, it's an interesting case. I'd hope that > git-fetch is smart enough not to even bother with quickfetch() if there > are no refs to update. But if we have even one change to fetch, then > yeah, in the general case it makes sense to me that you could do better > by amortizing the scan of local objects across many operations. Thanks for feedback. For the reference in case of git-backup `git fetch` or `git fetch-pack` would have to always do quickfetch scan or equivalent, because in case of backup repo there is only one reference in it - its master - and references of backed up repositories do not have anything representing them in backup.git/refs/ . Kirill
Re: [PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
On Wed, Jun 13, 2018 at 07:11:47PM -0400, Jeff King wrote:
> On Wed, Jun 13, 2018 at 05:05:09PM -0400, Jeff King wrote:
>
> > > In order to be sure fetching funky tags will never break, let's
> > > explicitly test all relevant cases with 4 tag objects pointing to 1) a
> > > blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> > > tag objects themselves are referenced from under regular refs/tags/*
> > > namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
> > >
> > > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git
> > > ls-remote ..
> > > 44085874...HEAD
> > > ...
> > > bc4e9e1f...refs/tags/tag-to-blob
> > > 038f48ad...refs/tags/tag-to-blob^{} # peeled
> > > 520db1f5...refs/tags/tag-to-tree
> > > 7395c100...refs/tags/tag-to-tree^{} # peeled
> > >
> > > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git
> > > fetch-pack --all ..
> > > fatal: A git upload-pack: not our ref 038f48ad...
> > > fatal: The remote end hung up unexpectedly
> >
> > TBH, I do not find this snippet all that compelling. We know that
> > e9502c0a7f already fixed the bug, and that it had nothing to do with
> > non-commits at all.
> >
> > The primary reason to add these tests is that in general we do not cover
> > fetch-pack over tags to non-commits. And I think the reason to use
> > otherwise unreferenced objects is that it they are more likely to have
> > detectable symptoms if they tickle a bug.
> >
> > So why don't we say that, instead of re-hashing output from the earlier
> > fix?
>
> Hmm, it looks like this already hit 'next', so it is too late to change
> the commit message (although 'next' will get rewound after the release,
> so we _could_ do it then).
>
> I also was going to suggest these style fixes, which could be applied on
> top (or squashed if we end up going that route). I actually wonder if
> the final tag one could just use two invocations of "git tag -m", but
> it's probably not worth spending too much time on polishing.
Jeff, thanks for corrections. I originally tried to look into invoking
"git tag" two times, but since git tag always creates a reference it
would not be semantically the same as having one referenced tag object
pointing to another tag object which does not have anything in refs/
pointing to it directly.
Maybe the commit text is not good but I tried to explain the reason you
are talking about with "In order to be sure fetching funky tags will
never break ..."
Kirill
> -- >8 --
> Subject: [PATCH] t5500: prettify non-commit tag tests
>
> We don't need to use backslash continuation, as the "&&"
> already provides continuation (and happily soaks up empty
> lines between commands).
>
> We can also expand the multi-line printf into a
> here-document, which lets us use line breaks more naturally
> (and avoids another continuation that required us to break
> the natural indentation).
>
> Signed-off-by: Jeff King
> ---
> t/t5500-fetch-pack.sh | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index ea6570e819..3d33ab3875 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -533,19 +533,26 @@ test_expect_success 'test --all wrt tag to non-commits'
> '
> # are reachable only via created tag references.
> blob=$(echo "hello blob" | git hash-object -t blob -w --stdin) &&
> git tag -a -m "tag -> blob" tag-to-blob $blob &&
> - \
> +
> tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> git tag -a -m "tag -> tree" tag-to-tree $tree &&
> - \
> +
> tree2=$(printf "100644 blob $blob\tfile2" | git mktree) &&
> commit=$(git commit-tree -m "hello commit" $tree) &&
> git tag -a -m "tag -> commit" tag-to-commit $commit &&
> - \
> +
> blob2=$(echo "hello blob2" | git hash-object -t blob -w --stdin) &&
> - tag=$(printf "object $blob2\ntype blob\ntag tag-to-blob2\n\
> -tagger author A U Thor 0 +\n\nhello tag" | git
> mktag) &&
> + tag=$(git mktag <<-EOF
> + object $blob2
> + type blob
> + tag tag-to-blob2
> + tagger author A U Thor 0 +
> +
> + hello tag
> + EOF
> + ) &&
> git tag -a -m "tag -> tag" tag-to-tag $tag &&
> - \
> +
> # `fetch-pack --all` should succeed fetching all those objects.
> mkdir fetchall &&
> (
> --
> 2.18.0.rc2.519.gb87ed92113
' Prize
USA Internet Command Protocol 2018 has selected your e-mail ID as a lucky winner of US $ 2.5 million out of US $ 350,000,000.00 shared worldwide for 1,600,000 e-mails users. Forward to this e-mail: m...@yahoo.com , your -Full name: Address: Mobile: Country: Winning code:USAL-q25M112233325 for certification of your prize amount by contacting the authorized agency via e-mail: m...@yahoo.com, Heather Durnell
Re: [PATCH v2 00/31] object-store: lookup_commit
On 6/13/2018 9:23 PM, Derrick Stolee wrote: On 6/13/2018 7:04 PM, Stefan Beller wrote * Once this is in good shape we can talk about converting parts of the revision walking code, This is another reason why I'll be waiting for this series of series. I plan to rework the revision walking code around sort_in_topological_order(), but I'll wait for your changes to land before submitting a patch. I'm itching to play with it, so I may start testing things out before then. In the meantime, I'll do my part by reviewing the current series. Looks good to me. Reviewed-by: Derrick Stolee
[PATCH v2] doc: update the order of the syntax `git merge --continue`
The syntax "git merge HEAD " has been removed. The
order of the syntax should also be updated.
Signed-off-by: Meng-Sung Wu
---
Documentation/git-merge.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index d5dfd8430..6a5c00e2c 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -57,7 +57,7 @@ reconstruct the original (pre-merge) changes. Therefore:
discouraged: while possible, it may leave you in a state that is hard to
back out of in the case of a conflict.
-The fourth syntax ("`git merge --continue`") can only be run after the
+The third syntax ("`git merge --continue`") can only be run after the
merge has resulted in conflicts.
OPTIONS
--
2.15.1 (Apple Git-101)
Re: [PATCH v2 00/31] object-store: lookup_commit
On 6/13/2018 7:04 PM, Stefan Beller wrote * Once this is in good shape we can talk about converting parts of the revision walking code, This is another reason why I'll be waiting for this series of series. I plan to rework the revision walking code around sort_in_topological_order(), but I'll wait for your changes to land before submitting a patch. I'm itching to play with it, so I may start testing things out before then. In the meantime, I'll do my part by reviewing the current series. Thanks, -Stolee
Re: [PATCH 03/35] object: add repository argument to lookup_unknown_object
On 6/13/2018 3:30 PM, Stefan Beller wrote: On Wed, Jun 6, 2018 at 12:38 PM Duy Nguyen wrote: On Wed, May 30, 2018 at 2:47 AM, Stefan Beller wrote: diff --git a/object.c b/object.c index 4de4fa58d59..def3c71cac2 100644 --- a/object.c +++ b/object.c @@ -177,7 +177,7 @@ void *object_as_type(struct object *obj, enum object_type type, int quiet) } } -struct object *lookup_unknown_object(const unsigned char *sha1) +struct object *lookup_unknown_object_the_repository(const unsigned char *sha1) I'm looking at your branch and this function (with the _the_repository suffix) is still there. Did you forget to send a patch to convert this function? This and parse_commit and parse_commit_gently have not been converted. I stopped with this series as soon as I hit the commit-graph code, which needs to be updated, too. I'll start redoing this series soon and will fix those conversions. Yes, we are colliding a bit at the moment. I'm planning to delay rerolling my patches until your last few series get merged. Thanks, -Stolee
Re: [PATCH 01/10] t: add tool to translate hash-related values
On Tue, Jun 12, 2018 at 03:29:47AM -0400, Eric Sunshine wrote:
> On Mon, Jun 11, 2018 at 9:05 PM, brian m. carlson
> wrote:
> > test_oid would be fine. One note is that this doesn't always produce
> > OIDs; sometimes it will produce other values, but as long as you don't
> > think that's too confusing, I'm fine with it.
>
> It was surprising to see it used for non-OID's (such as hash
> characteristics), but not hard to deal with.
>
> One could also view this as a generic key/value cache (not specific to
> OID's) with overriding super-key (the hash algorithm, in this case),
> which would allow for more generic name than test_oid(), but we don't
> have to go there presently.
It is essentially that. I'm happy with the test_oid name provided
others are. My only concern is that it would be confusing.
I opted to use the same mechanism for hash characteristics because it
seemed better than creating a lot of one-off functions that might have
duplicative implementations. But I'm open to arguments that having
test_oid_rawsz, test_oid_hexsz, etc. is better.
> > I agree perl would be expensive if it were invoked frequently, but
> > excepting SHA1-prerequisite tests, this function is invoked 32 times in
> > the entire testsuite.
> >
> > One of the reasons I chose perl was because we have a variety of cases
> > where we'll need spaces in values, and those tend to be complex in
> > shell.
>
> Can you give examples of cases in which values will contain spaces? It
> wasn't obvious from this patch series that such a need would arise.
>
> Are these values totally free-form? If not, some character (such as
> "_", "-", ".", etc.) could act as a stand-in for space. That shouldn't
> be too hard to handle.
t6030, which tests the bisect porcelain, is sensitive to the hash
algorithm because hash values are used as a secondary sort for the
closest commit. Without totally gutting the test and redoing it, some
solution to produce something resembling a sane commit message would be
helpful. We will also have cases where we need to provide strings to
printf(1), such as in some of the pack tests.
I have a minor modification to your code which handles that at the cost
of restricting us to one hash-key-value tuple on a line, which I think
is an acceptable tradeoff.
> > Using shell variables like this does have the downside that we're
> > restricted to only characters allowed in shell variables. That was
> > something I was trying to avoid, but it certainly isn't fatal.
>
> Is that just a general concern or do you have specific "weird" keys in mind?
I had originally thought of providing only the SHA-1 value instead of a
named key, which would have necessitated allowing arbitrary inputs, but
I ultimately decided that named keys were better. I also tend to prefer
dashes in inputs over underscores, since it's a bit easier to type, but
that's really a secondary concern.
I think the benefits of an implementation closer to your outweigh the
downsides.
> My original version of test_oid_cache() actually allowed for that by
> caching _all_ information from the tables rather than only values
> corresponding to $test_hash_algo. It looked like this:
>
> --- >8 ---
> test_oid_cache () {
> while read tag rest
> do
> case $tag in \#*) continue ;; esac
>
> for x in $rest
> do
> eval "test_oid_${tag}_${x%:*}=${x#*:}"
> done
> done
> }
> --- >8 ---
>
> The hash algorithm is incorporated into the cache variable name like
> this: "test_oid_hexsz_sha256"
Yeah, I basically reimplemented something similar to that based off your
code.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204
signature.asc
Description: PGP signature
Re: Hash algorithm analysis
On Tue, Jun 12, 2018 at 06:21:21PM +0200, Gilles Van Assche wrote: > Hi, > > On 10/06/18 00:49, brian m. carlson wrote: > > I imported the optimized 64-bit implementation of KangarooTwelve. The > > AVX2 implementation was not considered for licensing reasons (it's > > partially generated from external code, which falls foul of the GPL's > > "preferred form for modifications" rule). > > Indeed part of the AVX2 code in the Keccak code package is an extension > of the implementation in OpenSSL (written by Andy Polyakov). The > assembly code is generated by a Perl script, and we extended it to fit > in the KCP's internal API. > > Would it solve this licensing problem if we remap our extensions to the > Perl script, which would then become "the source"? The GPLv2 requires "the preferred form of the work for making modifications to it". If that form is the Perl script, then yes, that would be sufficient. If your code is dissimilar enough that editing it directly is better than editing the Perl script, then it might already meet the definition. I don't do assembly programming, so I don't know what forms one generally wants for editing assembly. Apparently OpenSSL wants a Perl script, but that is, I understand, less common. What would you use if you were going to improve it? > On 12/06/18 00:35, brian m. carlson wrote: > > While I think K12 is an interesting algorithm, I'm not sure we're > > going to get as good of performance out of it as we might want due to > > the lack of implementations. > > Implementation availability is indeed important. The effort to transform > an implementation of SHAKE128 into one of K12 is limited due to the > reuse of their main components (round function, sponge construction). So > the availability of SHA-3/Keccak implementations can benefit that of K12 > if there is sufficient interest. E.g., the SHA-3/Keccak instructions in > ARMv8.2 can speed up K12 as well. That's good to know. I wasn't aware that ARM was providing Keccak instructions, but it's good to see that new chips are providing them. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
On Wed, Jun 13, 2018 at 05:05:09PM -0400, Jeff King wrote:
> > In order to be sure fetching funky tags will never break, let's
> > explicitly test all relevant cases with 4 tag objects pointing to 1) a
> > blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> > tag objects themselves are referenced from under regular refs/tags/*
> > namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
> >
> > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote
> > ..
> > 44085874...HEAD
> > ...
> > bc4e9e1f...refs/tags/tag-to-blob
> > 038f48ad...refs/tags/tag-to-blob^{} # peeled
> > 520db1f5...refs/tags/tag-to-tree
> > 7395c100...refs/tags/tag-to-tree^{} # peeled
> >
> > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
> > --all ..
> > fatal: A git upload-pack: not our ref 038f48ad...
> > fatal: The remote end hung up unexpectedly
>
> TBH, I do not find this snippet all that compelling. We know that
> e9502c0a7f already fixed the bug, and that it had nothing to do with
> non-commits at all.
>
> The primary reason to add these tests is that in general we do not cover
> fetch-pack over tags to non-commits. And I think the reason to use
> otherwise unreferenced objects is that it they are more likely to have
> detectable symptoms if they tickle a bug.
>
> So why don't we say that, instead of re-hashing output from the earlier
> fix?
Hmm, it looks like this already hit 'next', so it is too late to change
the commit message (although 'next' will get rewound after the release,
so we _could_ do it then).
I also was going to suggest these style fixes, which could be applied on
top (or squashed if we end up going that route). I actually wonder if
the final tag one could just use two invocations of "git tag -m", but
it's probably not worth spending too much time on polishing.
-- >8 --
Subject: [PATCH] t5500: prettify non-commit tag tests
We don't need to use backslash continuation, as the "&&"
already provides continuation (and happily soaks up empty
lines between commands).
We can also expand the multi-line printf into a
here-document, which lets us use line breaks more naturally
(and avoids another continuation that required us to break
the natural indentation).
Signed-off-by: Jeff King
---
t/t5500-fetch-pack.sh | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index ea6570e819..3d33ab3875 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -533,19 +533,26 @@ test_expect_success 'test --all wrt tag to non-commits' '
# are reachable only via created tag references.
blob=$(echo "hello blob" | git hash-object -t blob -w --stdin) &&
git tag -a -m "tag -> blob" tag-to-blob $blob &&
- \
+
tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
git tag -a -m "tag -> tree" tag-to-tree $tree &&
- \
+
tree2=$(printf "100644 blob $blob\tfile2" | git mktree) &&
commit=$(git commit-tree -m "hello commit" $tree) &&
git tag -a -m "tag -> commit" tag-to-commit $commit &&
- \
+
blob2=$(echo "hello blob2" | git hash-object -t blob -w --stdin) &&
- tag=$(printf "object $blob2\ntype blob\ntag tag-to-blob2\n\
-tagger author A U Thor 0 +\n\nhello tag" | git mktag)
&&
+ tag=$(git mktag <<-EOF
+ object $blob2
+ type blob
+ tag tag-to-blob2
+ tagger author A U Thor 0 +
+
+ hello tag
+ EOF
+ ) &&
git tag -a -m "tag -> tag" tag-to-tag $tag &&
- \
+
# `fetch-pack --all` should succeed fetching all those objects.
mkdir fetchall &&
(
--
2.18.0.rc2.519.gb87ed92113
[PATCH v2 21/31] tag: allow parse_tag_buffer to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
tag.c | 10 +-
tag.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/tag.c b/tag.c
index 46b5882ee12..682e7793059 100644
--- a/tag.c
+++ b/tag.c
@@ -126,7 +126,7 @@ void release_tag_memory(struct tag *t)
t->date = 0;
}
-int parse_tag_buffer_the_repository(struct tag *item, const void *data,
unsigned long size)
+int parse_tag_buffer(struct repository *r, struct tag *item, const void *data,
unsigned long size)
{
struct object_id oid;
char type[20];
@@ -154,13 +154,13 @@ int parse_tag_buffer_the_repository(struct tag *item,
const void *data, unsigned
bufptr = nl + 1;
if (!strcmp(type, blob_type)) {
- item->tagged = (struct object *)lookup_blob(the_repository,
);
+ item->tagged = (struct object *)lookup_blob(r, );
} else if (!strcmp(type, tree_type)) {
- item->tagged = (struct object *)lookup_tree(the_repository,
);
+ item->tagged = (struct object *)lookup_tree(r, );
} else if (!strcmp(type, commit_type)) {
- item->tagged = (struct object *)lookup_commit(the_repository,
);
+ item->tagged = (struct object *)lookup_commit(r, );
} else if (!strcmp(type, tag_type)) {
- item->tagged = (struct object *)lookup_tag(the_repository,
);
+ item->tagged = (struct object *)lookup_tag(r, );
} else {
error("Unknown type %s", type);
item->tagged = NULL;
diff --git a/tag.h b/tag.h
index 6a160c91875..efd4c7da67c 100644
--- a/tag.h
+++ b/tag.h
@@ -12,8 +12,7 @@ struct tag {
timestamp_t date;
};
extern struct tag *lookup_tag(struct repository *r, const struct object_id
*oid);
-#define parse_tag_buffer(r, i, d, s) parse_tag_buffer_##r(i, d, s)
-extern int parse_tag_buffer_the_repository(struct tag *item, const void *data,
unsigned long size);
+extern int parse_tag_buffer(struct repository *r, struct tag *item, const void
*data, unsigned long size);
extern int parse_tag(struct tag *item);
extern void release_tag_memory(struct tag *t);
#define deref_tag(r, o, w, l) deref_tag_##r(o, w, l)
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 20/31] tag: allow lookup_tag to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
tag.c | 10 +-
tag.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/tag.c b/tag.c
index fbb4659325b..46b5882ee12 100644
--- a/tag.c
+++ b/tag.c
@@ -92,13 +92,13 @@ struct object *deref_tag_noverify(struct object *o)
return o;
}
-struct tag *lookup_tag_the_repository(const struct object_id *oid)
+struct tag *lookup_tag(struct repository *r, const struct object_id *oid)
{
- struct object *obj = lookup_object(the_repository, oid->hash);
+ struct object *obj = lookup_object(r, oid->hash);
if (!obj)
- return create_object(the_repository, oid->hash,
-alloc_tag_node(the_repository));
- return object_as_type(the_repository, obj, OBJ_TAG, 0);
+ return create_object(r, oid->hash,
+alloc_tag_node(r));
+ return object_as_type(r, obj, OBJ_TAG, 0);
}
static timestamp_t parse_tag_date(const char *buf, const char *tail)
diff --git a/tag.h b/tag.h
index 45b0b08b1f6..6a160c91875 100644
--- a/tag.h
+++ b/tag.h
@@ -11,8 +11,7 @@ struct tag {
char *tag;
timestamp_t date;
};
-#define lookup_tag(r, o) lookup_tag_##r(o)
-extern struct tag *lookup_tag_the_repository(const struct object_id *oid);
+extern struct tag *lookup_tag(struct repository *r, const struct object_id
*oid);
#define parse_tag_buffer(r, i, d, s) parse_tag_buffer_##r(i, d, s)
extern int parse_tag_buffer_the_repository(struct tag *item, const void *data,
unsigned long size);
extern int parse_tag(struct tag *item);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 28/31] object.c: allow parse_object to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
object.c | 14 +++---
object.h | 3 +--
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/object.c b/object.c
index b63a51febd4..62df8e11b63 100644
--- a/object.c
+++ b/object.c
@@ -245,28 +245,28 @@ struct object *parse_object_or_die(const struct object_id
*oid,
die(_("unable to parse object: %s"), name ? name : oid_to_hex(oid));
}
-struct object *parse_object_the_repository(const struct object_id *oid)
+struct object *parse_object(struct repository *r, const struct object_id *oid)
{
unsigned long size;
enum object_type type;
int eaten;
- const struct object_id *repl = lookup_replace_object(the_repository,
oid);
+ const struct object_id *repl = lookup_replace_object(r, oid);
void *buffer;
struct object *obj;
- obj = lookup_object(the_repository, oid->hash);
+ obj = lookup_object(r, oid->hash);
if (obj && obj->parsed)
return obj;
if ((obj && obj->type == OBJ_BLOB && has_object_file(oid)) ||
(!obj && has_object_file(oid) &&
-oid_object_info(the_repository, oid, NULL) == OBJ_BLOB)) {
+oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
if (check_object_signature(repl, NULL, 0, NULL) < 0) {
error("sha1 mismatch %s", oid_to_hex(oid));
return NULL;
}
- parse_blob_buffer(lookup_blob(the_repository, oid), NULL, 0);
- return lookup_object(the_repository, oid->hash);
+ parse_blob_buffer(lookup_blob(r, oid), NULL, 0);
+ return lookup_object(r, oid->hash);
}
buffer = read_object_file(oid, , );
@@ -277,7 +277,7 @@ struct object *parse_object_the_repository(const struct
object_id *oid)
return NULL;
}
- obj = parse_object_buffer(the_repository, oid, type, size,
+ obj = parse_object_buffer(r, oid, type, size,
buffer, );
if (!eaten)
free(buffer);
diff --git a/object.h b/object.h
index 9a667fbe795..a7892b04455 100644
--- a/object.h
+++ b/object.h
@@ -125,8 +125,7 @@ void *object_as_type(struct repository *r, struct object
*obj, enum object_type
*
* Returns NULL if the object is missing or corrupt.
*/
-#define parse_object(r, oid) parse_object_##r(oid)
-struct object *parse_object_the_repository(const struct object_id *oid);
+struct object *parse_object(struct repository *r, const struct object_id *oid);
/*
* Like parse_object, but will die() instead of returning NULL. If the
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 11/31] commit: add repository argument to get_cached_commit_buffer
Add a repository argument to allow callers of get_cached_commit_buffer to
be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
commit.c | 4 ++--
commit.h | 3 ++-
object.c | 2 +-
pretty.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/commit.c b/commit.c
index 667d4dc6cfd..ba06918ba41 100644
--- a/commit.c
+++ b/commit.c
@@ -256,7 +256,7 @@ void set_commit_buffer_the_repository(struct commit
*commit, void *buffer, unsig
v->size = size;
}
-const void *get_cached_commit_buffer(const struct commit *commit, unsigned
long *sizep)
+const void *get_cached_commit_buffer_the_repository(const struct commit
*commit, unsigned long *sizep)
{
struct commit_buffer *v = buffer_slab_peek(_slab, commit);
if (!v) {
@@ -271,7 +271,7 @@ const void *get_cached_commit_buffer(const struct commit
*commit, unsigned long
const void *get_commit_buffer(const struct commit *commit, unsigned long
*sizep)
{
- const void *ret = get_cached_commit_buffer(commit, sizep);
+ const void *ret = get_cached_commit_buffer(the_repository, commit,
sizep);
if (!ret) {
enum object_type type;
unsigned long size;
diff --git a/commit.h b/commit.h
index a1ecd067ccc..66eb576897f 100644
--- a/commit.h
+++ b/commit.h
@@ -92,7 +92,8 @@ void set_commit_buffer_the_repository(struct commit *, void
*buffer, unsigned lo
* Get any cached object buffer associated with the commit. Returns NULL
* if none. The resulting memory should not be freed.
*/
-const void *get_cached_commit_buffer(const struct commit *, unsigned long
*size);
+#define get_cached_commit_buffer(r, c, s) get_cached_commit_buffer_##r(c, s)
+const void *get_cached_commit_buffer_the_repository(const struct commit *,
unsigned long *size);
/*
* Get the commit's object contents, either from cache or by reading the object
diff --git a/object.c b/object.c
index ff0ba98675e..ecaac5c2990 100644
--- a/object.c
+++ b/object.c
@@ -216,7 +216,7 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
if (commit) {
if (parse_commit_buffer(the_repository, commit, buffer,
size))
return NULL;
- if (!get_cached_commit_buffer(commit, NULL)) {
+ if (!get_cached_commit_buffer(the_repository, commit,
NULL)) {
set_commit_buffer(the_repository, commit,
buffer, size);
*eaten_p = 1;
}
diff --git a/pretty.c b/pretty.c
index c99f8243faf..5b139775eac 100644
--- a/pretty.c
+++ b/pretty.c
@@ -630,7 +630,7 @@ const char *logmsg_reencode(const struct commit *commit,
* the cached copy from get_commit_buffer, we need to duplicate
it
* to avoid munging the cached copy.
*/
- if (msg == get_cached_commit_buffer(commit, NULL))
+ if (msg == get_cached_commit_buffer(the_repository, commit,
NULL))
out = xstrdup(msg);
else
out = (char *)msg;
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 19/31] commit: allow lookup_commit to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 10 +-
commit.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/commit.c b/commit.c
index ede31c46931..4b10e7173b5 100644
--- a/commit.c
+++ b/commit.c
@@ -52,13 +52,13 @@ struct commit *lookup_commit_or_die(const struct object_id
*oid, const char *ref
return c;
}
-struct commit *lookup_commit_the_repository(const struct object_id *oid)
+struct commit *lookup_commit(struct repository *r, const struct object_id *oid)
{
- struct object *obj = lookup_object(the_repository, oid->hash);
+ struct object *obj = lookup_object(r, oid->hash);
if (!obj)
- return create_object(the_repository, oid->hash,
-alloc_commit_node(the_repository));
- return object_as_type(the_repository, obj, OBJ_COMMIT, 0);
+ return create_object(r, oid->hash,
+alloc_commit_node(r));
+ return object_as_type(r, obj, OBJ_COMMIT, 0);
}
struct commit *lookup_commit_reference_by_name(const char *name)
diff --git a/commit.h b/commit.h
index 66eb576897f..d4561587851 100644
--- a/commit.h
+++ b/commit.h
@@ -53,8 +53,7 @@ enum decoration_type {
void add_name_decoration(enum decoration_type type, const char *name, struct
object *obj);
const struct name_decoration *get_name_decoration(const struct object *obj);
-#define lookup_commit(r, o) lookup_commit_##r(o)
-struct commit *lookup_commit_the_repository(const struct object_id *oid);
+struct commit *lookup_commit(struct repository *r, const struct object_id
*oid);
#define lookup_commit_reference(r, o) \
lookup_commit_reference_##r(o)
struct commit *lookup_commit_reference_the_repository(const struct object_id
*oid);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 30/31] commit.c: allow lookup_commit_reference_gently to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 8
commit.h | 4 +---
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/commit.c b/commit.c
index 7ee55f6b8f0..009e1d33e08 100644
--- a/commit.c
+++ b/commit.c
@@ -23,16 +23,16 @@ int save_commit_buffer = 1;
const char *commit_type = "commit";
-struct commit *lookup_commit_reference_gently_the_repository(
+struct commit *lookup_commit_reference_gently(struct repository *r,
const struct object_id *oid, int quiet)
{
- struct object *obj = deref_tag(the_repository,
- parse_object(the_repository, oid),
+ struct object *obj = deref_tag(r,
+ parse_object(r, oid),
NULL, 0);
if (!obj)
return NULL;
- return object_as_type(the_repository, obj, OBJ_COMMIT, quiet);
+ return object_as_type(r, obj, OBJ_COMMIT, quiet);
}
struct commit *lookup_commit_reference_the_repository(const struct object_id
*oid)
diff --git a/commit.h b/commit.h
index c7bb01ffe4b..717403d6431 100644
--- a/commit.h
+++ b/commit.h
@@ -57,9 +57,7 @@ struct commit *lookup_commit(struct repository *r, const
struct object_id *oid);
#define lookup_commit_reference(r, o) \
lookup_commit_reference_##r(o)
struct commit *lookup_commit_reference_the_repository(const struct object_id
*oid);
-#define lookup_commit_reference_gently(r, o, q) \
- lookup_commit_reference_gently_##r(o, q)
-struct commit *lookup_commit_reference_gently_the_repository(
+struct commit *lookup_commit_reference_gently(struct repository *r,
const struct object_id *oid,
int quiet);
struct commit *lookup_commit_reference_by_name(const char *name);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 23/31] commit-slabs: remove realloc counter outside of slab struct
The realloc counter is declared outside the struct for the given slabname,
which makes it harder for a follow up patch to move the declaration of the
struct around as then the counter variable would need special treatment.
As the reallocation counter is currently unused we can just remove it.
If we ever need to count the reallocations again, we can reintroduce
the counter as part of 'struct slabname' in commit-slab-decl.h.
Signed-off-by: Stefan Beller
---
commit-slab-impl.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/commit-slab-impl.h b/commit-slab-impl.h
index 87a9cadfcca..ac1e6d409ad 100644
--- a/commit-slab-impl.h
+++ b/commit-slab-impl.h
@@ -11,8 +11,6 @@
#define implement_commit_slab(slabname, elemtype, scope) \
\
-static int stat_ ##slabname## realloc; \
- \
scope void init_ ##slabname## _with_stride(struct slabname *s, \
unsigned stride) \
{ \
@@ -54,7 +52,6 @@ scope elemtype *slabname## _at_peek(struct slabname *s,
\
if (!add_if_missing)\
return NULL;\
REALLOC_ARRAY(s->slab, nth_slab + 1); \
- stat_ ##slabname## realloc++; \
for (i = s->slab_count; i <= nth_slab; i++) \
s->slab[i] = NULL; \
s->slab_count = nth_slab + 1; \
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 27/31] object.c: allow parse_object_buffer to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
object.c | 18 +-
object.h | 3 +--
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/object.c b/object.c
index 7a7b078e4d0..b63a51febd4 100644
--- a/object.c
+++ b/object.c
@@ -185,21 +185,21 @@ struct object *lookup_unknown_object(const unsigned char
*sha1)
return obj;
}
-struct object *parse_object_buffer_the_repository(const struct object_id *oid,
enum object_type type, unsigned long size, void *buffer, int *eaten_p)
+struct object *parse_object_buffer(struct repository *r, const struct
object_id *oid, enum object_type type, unsigned long size, void *buffer, int
*eaten_p)
{
struct object *obj;
*eaten_p = 0;
obj = NULL;
if (type == OBJ_BLOB) {
- struct blob *blob = lookup_blob(the_repository, oid);
+ struct blob *blob = lookup_blob(r, oid);
if (blob) {
if (parse_blob_buffer(blob, buffer, size))
return NULL;
obj = >object;
}
} else if (type == OBJ_TREE) {
- struct tree *tree = lookup_tree(the_repository, oid);
+ struct tree *tree = lookup_tree(r, oid);
if (tree) {
obj = >object;
if (!tree->buffer)
@@ -211,20 +211,20 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
}
}
} else if (type == OBJ_COMMIT) {
- struct commit *commit = lookup_commit(the_repository, oid);
+ struct commit *commit = lookup_commit(r, oid);
if (commit) {
- if (parse_commit_buffer(the_repository, commit, buffer,
size))
+ if (parse_commit_buffer(r, commit, buffer, size))
return NULL;
- if (!get_cached_commit_buffer(the_repository, commit,
NULL)) {
- set_commit_buffer(the_repository, commit,
buffer, size);
+ if (!get_cached_commit_buffer(r, commit, NULL)) {
+ set_commit_buffer(r, commit, buffer, size);
*eaten_p = 1;
}
obj = >object;
}
} else if (type == OBJ_TAG) {
- struct tag *tag = lookup_tag(the_repository, oid);
+ struct tag *tag = lookup_tag(r, oid);
if (tag) {
- if (parse_tag_buffer(the_repository, tag, buffer, size))
+ if (parse_tag_buffer(r, tag, buffer, size))
return NULL;
obj = >object;
}
diff --git a/object.h b/object.h
index 876480c933c..9a667fbe795 100644
--- a/object.h
+++ b/object.h
@@ -139,8 +139,7 @@ struct object *parse_object_or_die(const struct object_id
*oid, const char *name
* parsing it. eaten_p indicates if the object has a borrowed copy
* of buffer and the caller should not free() it.
*/
-#define parse_object_buffer(r, o, t, s, b, e) parse_object_buffer_##r(o, t, s,
b, e)
-struct object *parse_object_buffer_the_repository(const struct object_id *oid,
enum object_type type, unsigned long size, void *buffer, int *eaten_p);
+struct object *parse_object_buffer(struct repository *r, const struct
object_id *oid, enum object_type type, unsigned long size, void *buffer, int
*eaten_p);
/** Returns the object, with potentially excess memory allocated. **/
struct object *lookup_unknown_object(const unsigned char *sha1);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 22/31] commit.c: allow parse_commit_buffer to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 10 +-
commit.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/commit.c b/commit.c
index 4b10e7173b5..fe3b6ff216f 100644
--- a/commit.c
+++ b/commit.c
@@ -335,7 +335,7 @@ const void *detach_commit_buffer(struct commit *commit,
unsigned long *sizep)
return ret;
}
-int parse_commit_buffer_the_repository(struct commit *item, const void
*buffer, unsigned long size)
+int parse_commit_buffer(struct repository *r, struct commit *item, const void
*buffer, unsigned long size)
{
const char *tail = buffer;
const char *bufptr = buffer;
@@ -355,11 +355,11 @@ int parse_commit_buffer_the_repository(struct commit
*item, const void *buffer,
if (get_sha1_hex(bufptr + 5, parent.hash) < 0)
return error("bad tree pointer in commit %s",
oid_to_hex(>object.oid));
- item->tree = lookup_tree(the_repository, );
+ item->tree = lookup_tree(r, );
bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
pptr = >parents;
- graft = lookup_commit_graft(the_repository, >object.oid);
+ graft = lookup_commit_graft(r, >object.oid);
while (bufptr + parent_entry_len < tail && !memcmp(bufptr, "parent ",
7)) {
struct commit *new_parent;
@@ -374,7 +374,7 @@ int parse_commit_buffer_the_repository(struct commit *item,
const void *buffer,
*/
if (graft && (graft->nr_parent < 0 || grafts_replace_parents))
continue;
- new_parent = lookup_commit(the_repository, );
+ new_parent = lookup_commit(r, );
if (new_parent)
pptr = _list_insert(new_parent, pptr)->next;
}
@@ -382,7 +382,7 @@ int parse_commit_buffer_the_repository(struct commit *item,
const void *buffer,
int i;
struct commit *new_parent;
for (i = 0; i < graft->nr_parent; i++) {
- new_parent = lookup_commit(the_repository,
+ new_parent = lookup_commit(r,
>parent[i]);
if (!new_parent)
continue;
diff --git a/commit.h b/commit.h
index d4561587851..66aabb80068 100644
--- a/commit.h
+++ b/commit.h
@@ -71,8 +71,7 @@ struct commit *lookup_commit_reference_by_name(const char
*name);
*/
struct commit *lookup_commit_or_die(const struct object_id *oid, const char
*ref_name);
-#define parse_commit_buffer(r, i, b, s) parse_commit_buffer_##r(i, b, s)
-int parse_commit_buffer_the_repository(struct commit *item, const void
*buffer, unsigned long size);
+int parse_commit_buffer(struct repository *r, struct commit *item, const void
*buffer, unsigned long size);
int parse_commit_gently(struct commit *item, int quiet_on_missing);
static inline int parse_commit(struct commit *item)
{
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 24/31] commit.c: migrate the commit buffer to the parsed object store
Signed-off-by: Stefan Beller
---
commit.c | 29 +++--
commit.h | 4
object.c | 5 +
object.h | 4
4 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/commit.c b/commit.c
index fe3b6ff216f..e9b22268997 100644
--- a/commit.c
+++ b/commit.c
@@ -248,18 +248,32 @@ struct commit_buffer {
unsigned long size;
};
define_commit_slab(buffer_slab, struct commit_buffer);
-static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);
+
+struct buffer_slab *allocate_commit_buffer_slab(void)
+{
+ struct buffer_slab *bs = xmalloc(sizeof(*bs));
+ init_buffer_slab(bs);
+ return bs;
+}
+
+void free_commit_buffer_slab(struct buffer_slab *bs)
+{
+ clear_buffer_slab(bs);
+ free(bs);
+}
void set_commit_buffer_the_repository(struct commit *commit, void *buffer,
unsigned long size)
{
- struct commit_buffer *v = buffer_slab_at(_slab, commit);
+ struct commit_buffer *v = buffer_slab_at(
+ the_repository->parsed_objects->buffer_slab, commit);
v->buffer = buffer;
v->size = size;
}
const void *get_cached_commit_buffer_the_repository(const struct commit
*commit, unsigned long *sizep)
{
- struct commit_buffer *v = buffer_slab_peek(_slab, commit);
+ struct commit_buffer *v = buffer_slab_peek(
+ the_repository->parsed_objects->buffer_slab, commit);
if (!v) {
if (sizep)
*sizep = 0;
@@ -291,14 +305,16 @@ const void *get_commit_buffer(const struct commit
*commit, unsigned long *sizep)
void unuse_commit_buffer(const struct commit *commit, const void *buffer)
{
- struct commit_buffer *v = buffer_slab_peek(_slab, commit);
+ struct commit_buffer *v = buffer_slab_peek(
+ the_repository->parsed_objects->buffer_slab, commit);
if (!(v && v->buffer == buffer))
free((void *)buffer);
}
void free_commit_buffer(struct commit *commit)
{
- struct commit_buffer *v = buffer_slab_peek(_slab, commit);
+ struct commit_buffer *v = buffer_slab_peek(
+ the_repository->parsed_objects->buffer_slab, commit);
if (v) {
FREE_AND_NULL(v->buffer);
v->size = 0;
@@ -318,7 +334,8 @@ void release_commit_memory(struct commit *c)
const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
{
- struct commit_buffer *v = buffer_slab_peek(_slab, commit);
+ struct commit_buffer *v = buffer_slab_peek(
+ the_repository->parsed_objects->buffer_slab, commit);
void *ret;
if (!v) {
diff --git a/commit.h b/commit.h
index 66aabb80068..a417f99ad4f 100644
--- a/commit.h
+++ b/commit.h
@@ -79,6 +79,10 @@ static inline int parse_commit(struct commit *item)
}
void parse_commit_or_die(struct commit *item);
+struct buffer_slab;
+struct buffer_slab *allocate_commit_buffer_slab(void);
+void free_commit_buffer_slab(struct buffer_slab *bs);
+
/*
* Associate an object buffer with the commit. The ownership of the
* memory is handed over to the commit, and must be free()-able.
diff --git a/object.c b/object.c
index c1c1cbc1f53..7a7b078e4d0 100644
--- a/object.c
+++ b/object.c
@@ -467,6 +467,8 @@ struct parsed_object_pool *parsed_object_pool_new(void)
o->is_shallow = -1;
o->shallow_stat = xcalloc(1, sizeof(*o->shallow_stat));
+ o->buffer_slab = allocate_commit_buffer_slab();
+
return o;
}
@@ -538,6 +540,9 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)
FREE_AND_NULL(o->obj_hash);
o->obj_hash_size = 0;
+ free_commit_buffer_slab(o->buffer_slab);
+ o->buffer_slab = NULL;
+
clear_alloc_state(o->blob_state);
clear_alloc_state(o->tree_state);
clear_alloc_state(o->commit_state);
diff --git a/object.h b/object.h
index 66e0b7b93dc..876480c933c 100644
--- a/object.h
+++ b/object.h
@@ -1,6 +1,8 @@
#ifndef OBJECT_H
#define OBJECT_H
+struct buffer_slab;
+
struct parsed_object_pool {
struct object **obj_hash;
int nr_objs, obj_hash_size;
@@ -22,6 +24,8 @@ struct parsed_object_pool {
char *alternate_shallow_file;
int commit_graft_prepared;
+
+ struct buffer_slab *buffer_slab;
};
struct parsed_object_pool *parsed_object_pool_new(void);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 26/31] commit.c: allow get_cached_commit_buffer to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 4 ++--
commit.h | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/commit.c b/commit.c
index 44d1a38187a..7ee55f6b8f0 100644
--- a/commit.c
+++ b/commit.c
@@ -270,10 +270,10 @@ void set_commit_buffer(struct repository *r, struct
commit *commit, void *buffer
v->size = size;
}
-const void *get_cached_commit_buffer_the_repository(const struct commit
*commit, unsigned long *sizep)
+const void *get_cached_commit_buffer(struct repository *r, const struct commit
*commit, unsigned long *sizep)
{
struct commit_buffer *v = buffer_slab_peek(
- the_repository->parsed_objects->buffer_slab, commit);
+ r->parsed_objects->buffer_slab, commit);
if (!v) {
if (sizep)
*sizep = 0;
diff --git a/commit.h b/commit.h
index 3e883787855..c7bb01ffe4b 100644
--- a/commit.h
+++ b/commit.h
@@ -93,8 +93,7 @@ void set_commit_buffer(struct repository *r, struct commit *,
void *buffer, unsi
* Get any cached object buffer associated with the commit. Returns NULL
* if none. The resulting memory should not be freed.
*/
-#define get_cached_commit_buffer(r, c, s) get_cached_commit_buffer_##r(c, s)
-const void *get_cached_commit_buffer_the_repository(const struct commit *,
unsigned long *size);
+const void *get_cached_commit_buffer(struct repository *, const struct commit
*, unsigned long *size);
/*
* Get the commit's object contents, either from cache or by reading the object
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 31/31] commit.c: allow lookup_commit_reference to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 4 ++--
commit.h | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/commit.c b/commit.c
index 009e1d33e08..7b60e0cb030 100644
--- a/commit.c
+++ b/commit.c
@@ -35,9 +35,9 @@ struct commit *lookup_commit_reference_gently(struct
repository *r,
return object_as_type(r, obj, OBJ_COMMIT, quiet);
}
-struct commit *lookup_commit_reference_the_repository(const struct object_id
*oid)
+struct commit *lookup_commit_reference(struct repository *r, const struct
object_id *oid)
{
- return lookup_commit_reference_gently(the_repository, oid, 0);
+ return lookup_commit_reference_gently(r, oid, 0);
}
struct commit *lookup_commit_or_die(const struct object_id *oid, const char
*ref_name)
diff --git a/commit.h b/commit.h
index 717403d6431..e1eb48afde2 100644
--- a/commit.h
+++ b/commit.h
@@ -54,9 +54,8 @@ void add_name_decoration(enum decoration_type type, const
char *name, struct obj
const struct name_decoration *get_name_decoration(const struct object *obj);
struct commit *lookup_commit(struct repository *r, const struct object_id
*oid);
-#define lookup_commit_reference(r, o) \
- lookup_commit_reference_##r(o)
-struct commit *lookup_commit_reference_the_repository(const struct object_id
*oid);
+struct commit *lookup_commit_reference(struct repository *r,
+ const struct object_id *oid);
struct commit *lookup_commit_reference_gently(struct repository *r,
const struct object_id *oid,
int quiet);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 29/31] tag.c: allow deref_tag to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
tag.c | 5 ++---
tag.h | 3 +--
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/tag.c b/tag.c
index 682e7793059..94a89b21cb5 100644
--- a/tag.c
+++ b/tag.c
@@ -64,12 +64,11 @@ int gpg_verify_tag(const struct object_id *oid, const char
*name_to_report,
return ret;
}
-struct object *deref_tag_the_repository(struct object *o, const char *warn,
int warnlen)
+struct object *deref_tag(struct repository *r, struct object *o, const char
*warn, int warnlen)
{
while (o && o->type == OBJ_TAG)
if (((struct tag *)o)->tagged)
- o = parse_object(the_repository,
-&((struct tag *)o)->tagged->oid);
+ o = parse_object(r, &((struct tag *)o)->tagged->oid);
else
o = NULL;
if (!o && warn) {
diff --git a/tag.h b/tag.h
index efd4c7da67c..e669c3e497a 100644
--- a/tag.h
+++ b/tag.h
@@ -15,8 +15,7 @@ extern struct tag *lookup_tag(struct repository *r, const
struct object_id *oid)
extern int parse_tag_buffer(struct repository *r, struct tag *item, const void
*data, unsigned long size);
extern int parse_tag(struct tag *item);
extern void release_tag_memory(struct tag *t);
-#define deref_tag(r, o, w, l) deref_tag_##r(o, w, l)
-extern struct object *deref_tag_the_repository(struct object *, const char *,
int);
+extern struct object *deref_tag(struct repository *r, struct object *, const
char *, int);
extern struct object *deref_tag_noverify(struct object *);
extern int gpg_verify_tag(const struct object_id *oid,
const char *name_to_report, unsigned flags);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 25/31] commit.c: allow set_commit_buffer to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
commit.c | 4 ++--
commit.h | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/commit.c b/commit.c
index e9b22268997..44d1a38187a 100644
--- a/commit.c
+++ b/commit.c
@@ -262,10 +262,10 @@ void free_commit_buffer_slab(struct buffer_slab *bs)
free(bs);
}
-void set_commit_buffer_the_repository(struct commit *commit, void *buffer,
unsigned long size)
+void set_commit_buffer(struct repository *r, struct commit *commit, void
*buffer, unsigned long size)
{
struct commit_buffer *v = buffer_slab_at(
- the_repository->parsed_objects->buffer_slab, commit);
+ r->parsed_objects->buffer_slab, commit);
v->buffer = buffer;
v->size = size;
}
diff --git a/commit.h b/commit.h
index a417f99ad4f..3e883787855 100644
--- a/commit.h
+++ b/commit.h
@@ -87,8 +87,7 @@ void free_commit_buffer_slab(struct buffer_slab *bs);
* Associate an object buffer with the commit. The ownership of the
* memory is handed over to the commit, and must be free()-able.
*/
-#define set_commit_buffer(r, c, b, s) set_commit_buffer_##r(c, b, s)
-void set_commit_buffer_the_repository(struct commit *, void *buffer, unsigned
long size);
+void set_commit_buffer(struct repository *r, struct commit *, void *buffer,
unsigned long size);
/*
* Get any cached object buffer associated with the commit. Returns NULL
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 09/31] commit: add repository argument to parse_commit_buffer
Add a repository argument to allow the callers of parse_commit_buffer
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
commit.c| 4 ++--
commit.h| 3 ++-
object.c| 2 +-
sha1-file.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/commit.c b/commit.c
index 5e3f18801a1..dda585025ed 100644
--- a/commit.c
+++ b/commit.c
@@ -334,7 +334,7 @@ const void *detach_commit_buffer(struct commit *commit,
unsigned long *sizep)
return ret;
}
-int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long
size)
+int parse_commit_buffer_the_repository(struct commit *item, const void
*buffer, unsigned long size)
{
const char *tail = buffer;
const char *bufptr = buffer;
@@ -416,7 +416,7 @@ int parse_commit_gently(struct commit *item, int
quiet_on_missing)
return error("Object %s not a commit",
oid_to_hex(>object.oid));
}
- ret = parse_commit_buffer(item, buffer, size);
+ ret = parse_commit_buffer(the_repository, item, buffer, size);
if (save_commit_buffer && !ret) {
set_commit_buffer(item, buffer, size);
return 0;
diff --git a/commit.h b/commit.h
index 431a7d97a24..05b9eccaf66 100644
--- a/commit.h
+++ b/commit.h
@@ -72,7 +72,8 @@ struct commit *lookup_commit_reference_by_name(const char
*name);
*/
struct commit *lookup_commit_or_die(const struct object_id *oid, const char
*ref_name);
-int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long
size);
+#define parse_commit_buffer(r, i, b, s) parse_commit_buffer_##r(i, b, s)
+int parse_commit_buffer_the_repository(struct commit *item, const void
*buffer, unsigned long size);
int parse_commit_gently(struct commit *item, int quiet_on_missing);
static inline int parse_commit(struct commit *item)
{
diff --git a/object.c b/object.c
index 4eca16f4adb..fe1f84f7c6c 100644
--- a/object.c
+++ b/object.c
@@ -214,7 +214,7 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
} else if (type == OBJ_COMMIT) {
struct commit *commit = lookup_commit(the_repository, oid);
if (commit) {
- if (parse_commit_buffer(commit, buffer, size))
+ if (parse_commit_buffer(the_repository, commit, buffer,
size))
return NULL;
if (!get_cached_commit_buffer(commit, NULL)) {
set_commit_buffer(commit, buffer, size);
diff --git a/sha1-file.c b/sha1-file.c
index f66059ed7dd..00b1b2b8660 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1780,7 +1780,7 @@ static void check_commit(const void *buf, size_t size)
{
struct commit c;
memset(, 0, sizeof(c));
- if (parse_commit_buffer(, buf, size))
+ if (parse_commit_buffer(the_repository, , buf, size))
die("corrupt commit");
}
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 16/31] object: allow lookup_object to handle arbitrary repositories
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
object.c | 15 +++
object.h | 3 +--
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/object.c b/object.c
index 876ca4977e2..c1c1cbc1f53 100644
--- a/object.c
+++ b/object.c
@@ -84,21 +84,20 @@ static void insert_obj_hash(struct object *obj, struct
object **hash, unsigned i
* Look up the record for the given sha1 in the hash map stored in
* obj_hash. Return NULL if it was not found.
*/
-struct object *lookup_object_the_repository(const unsigned char *sha1)
+struct object *lookup_object(struct repository *r, const unsigned char *sha1)
{
unsigned int i, first;
struct object *obj;
- if (!the_repository->parsed_objects->obj_hash)
+ if (!r->parsed_objects->obj_hash)
return NULL;
- first = i = hash_obj(sha1,
-the_repository->parsed_objects->obj_hash_size);
- while ((obj = the_repository->parsed_objects->obj_hash[i]) != NULL) {
+ first = i = hash_obj(sha1, r->parsed_objects->obj_hash_size);
+ while ((obj = r->parsed_objects->obj_hash[i]) != NULL) {
if (!hashcmp(sha1, obj->oid.hash))
break;
i++;
- if (i == the_repository->parsed_objects->obj_hash_size)
+ if (i == r->parsed_objects->obj_hash_size)
i = 0;
}
if (obj && i != first) {
@@ -107,8 +106,8 @@ struct object *lookup_object_the_repository(const unsigned
char *sha1)
* that we do not need to walk the hash table the next
* time we look for it.
*/
- SWAP(the_repository->parsed_objects->obj_hash[i],
-the_repository->parsed_objects->obj_hash[first]);
+ SWAP(r->parsed_objects->obj_hash[i],
+r->parsed_objects->obj_hash[first]);
}
return obj;
}
diff --git a/object.h b/object.h
index bd25155480f..66e0b7b93dc 100644
--- a/object.h
+++ b/object.h
@@ -110,8 +110,7 @@ extern struct object *get_indexed_object(unsigned int);
* half-initialised objects, the caller is expected to initialize them
* by calling parse_object() on them.
*/
-#define lookup_object(r, s) lookup_object_##r(s)
-struct object *lookup_object_the_repository(const unsigned char *sha1);
+struct object *lookup_object(struct repository *r, const unsigned char *sha1);
extern void *create_object(struct repository *r, const unsigned char *sha1,
void *obj);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 13/31] tag: add repository argument to parse_tag_buffer
Add a repository argument to allow the callers of parse_tag_buffer
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
builtin/replace.c | 2 +-
log-tree.c| 2 +-
object.c | 2 +-
sha1-file.c | 2 +-
tag.c | 4 ++--
tag.h | 3 ++-
6 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 8c8cec4aae6..bff1c3df964 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -362,7 +362,7 @@ static void check_one_mergetag(struct commit *commit,
tag = lookup_tag(the_repository, _oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
- if (parse_tag_buffer(tag, extra->value, extra->len))
+ if (parse_tag_buffer(the_repository, tag, extra->value, extra->len))
die(_("malformed mergetag in commit '%s'"), ref);
/* iterate over new parents */
diff --git a/log-tree.c b/log-tree.c
index 727758eb442..ac42ad42acc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -511,7 +511,7 @@ static void show_one_mergetag(struct commit *commit,
return; /* error message already given */
strbuf_init(_message, 256);
- if (parse_tag_buffer(tag, extra->value, extra->len))
+ if (parse_tag_buffer(the_repository, tag, extra->value, extra->len))
strbuf_addstr(_message, "malformed mergetag\n");
else if (is_common_merge(commit) &&
!oidcmp(>tagged->oid,
diff --git a/object.c b/object.c
index 080e9b36eaf..c6779ee596f 100644
--- a/object.c
+++ b/object.c
@@ -225,7 +225,7 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
} else if (type == OBJ_TAG) {
struct tag *tag = lookup_tag(the_repository, oid);
if (tag) {
- if (parse_tag_buffer(tag, buffer, size))
+ if (parse_tag_buffer(the_repository, tag, buffer, size))
return NULL;
obj = >object;
}
diff --git a/sha1-file.c b/sha1-file.c
index 00b1b2b8660..3440b67639e 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1788,7 +1788,7 @@ static void check_tag(const void *buf, size_t size)
{
struct tag t;
memset(, 0, sizeof(t));
- if (parse_tag_buffer(, buf, size))
+ if (parse_tag_buffer(the_repository, , buf, size))
die("corrupt tag");
}
diff --git a/tag.c b/tag.c
index 5b41fc71fad..4971fd4dfc9 100644
--- a/tag.c
+++ b/tag.c
@@ -126,7 +126,7 @@ void release_tag_memory(struct tag *t)
t->date = 0;
}
-int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)
+int parse_tag_buffer_the_repository(struct tag *item, const void *data,
unsigned long size)
{
struct object_id oid;
char type[20];
@@ -203,7 +203,7 @@ int parse_tag(struct tag *item)
return error("Object %s not a tag",
oid_to_hex(>object.oid));
}
- ret = parse_tag_buffer(item, data, size);
+ ret = parse_tag_buffer(the_repository, item, data, size);
free(data);
return ret;
}
diff --git a/tag.h b/tag.h
index 276c448cd55..149959c81ba 100644
--- a/tag.h
+++ b/tag.h
@@ -13,7 +13,8 @@ struct tag {
};
#define lookup_tag(r, o) lookup_tag_##r(o)
extern struct tag *lookup_tag_the_repository(const struct object_id *oid);
-extern int parse_tag_buffer(struct tag *item, const void *data, unsigned long
size);
+#define parse_tag_buffer(r, i, d, s) parse_tag_buffer_##r(i, d, s)
+extern int parse_tag_buffer_the_repository(struct tag *item, const void *data,
unsigned long size);
extern int parse_tag(struct tag *item);
extern void release_tag_memory(struct tag *t);
extern struct object *deref_tag(struct object *, const char *, int);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 07/31] commit: add repository argument to lookup_commit_reference
Add a repository argument to allow callers of lookup_commit_reference
to be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
bisect.c | 2 +-
blame.c | 2 +-
branch.c | 2 +-
builtin/branch.c | 7 ---
builtin/clone.c | 3 ++-
builtin/describe.c| 2 +-
builtin/diff-tree.c | 2 +-
builtin/log.c | 7 ---
builtin/merge-base.c | 5 +++--
builtin/notes.c | 3 ++-
builtin/pull.c| 15 ++-
builtin/reset.c | 4 ++--
builtin/rev-parse.c | 6 +++---
builtin/show-branch.c | 2 +-
builtin/tag.c | 2 +-
bundle.c | 3 ++-
commit.c | 6 +++---
commit.h | 4 +++-
notes-merge.c | 5 +++--
parse-options-cb.c| 2 +-
remote.c | 4 ++--
revision.c| 4 ++--
sequencer.c | 6 +++---
sha1-name.c | 4 ++--
submodule.c | 10 +-
25 files changed, 63 insertions(+), 49 deletions(-)
diff --git a/bisect.c b/bisect.c
index 6de1abd407b..e1275ba79e8 100644
--- a/bisect.c
+++ b/bisect.c
@@ -724,7 +724,7 @@ static int bisect_checkout(const struct object_id
*bisect_rev, int no_checkout)
static struct commit *get_commit_reference(const struct object_id *oid)
{
- struct commit *r = lookup_commit_reference(oid);
+ struct commit *r = lookup_commit_reference(the_repository, oid);
if (!r)
die(_("Not a valid commit name %s"), oid_to_hex(oid));
return r;
diff --git a/blame.c b/blame.c
index 726a7a76f20..86d0dd73338 100644
--- a/blame.c
+++ b/blame.c
@@ -119,7 +119,7 @@ static struct commit_list **append_parent(struct
commit_list **tail, const struc
{
struct commit *parent;
- parent = lookup_commit_reference(oid);
+ parent = lookup_commit_reference(the_repository, oid);
if (!parent)
die("no such commit %s", oid_to_hex(oid));
return _list_insert(parent, tail)->next;
diff --git a/branch.c b/branch.c
index 9b2742de32a..08d4efc1be6 100644
--- a/branch.c
+++ b/branch.c
@@ -301,7 +301,7 @@ void create_branch(const char *name, const char *start_name,
break;
}
- if ((commit = lookup_commit_reference()) == NULL)
+ if ((commit = lookup_commit_reference(the_repository, )) == NULL)
die(_("Not a valid branch point: '%s'."), start_name);
oidcpy(, >object.oid);
diff --git a/builtin/branch.c b/builtin/branch.c
index efc9ac1922c..fbbe311e270 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -126,7 +126,8 @@ static int branch_merged(int kind, const char *name,
(reference_name = reference_name_to_free =
resolve_refdup(upstream, RESOLVE_REF_READING,
, NULL)) != NULL)
- reference_rev = lookup_commit_reference();
+ reference_rev = lookup_commit_reference(the_repository,
+ );
}
if (!reference_rev)
reference_rev = head_rev;
@@ -159,7 +160,7 @@ static int check_branch_commit(const char *branchname,
const char *refname,
const struct object_id *oid, struct commit
*head_rev,
int kinds, int force)
{
- struct commit *rev = lookup_commit_reference(oid);
+ struct commit *rev = lookup_commit_reference(the_repository, oid);
if (!rev) {
error(_("Couldn't look up commit object for '%s'"), refname);
return -1;
@@ -213,7 +214,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
}
if (!force) {
- head_rev = lookup_commit_reference(_oid);
+ head_rev = lookup_commit_reference(the_repository, _oid);
if (!head_rev)
die(_("Couldn't look up commit object for HEAD"));
}
diff --git a/builtin/clone.c b/builtin/clone.c
index 2f10c468300..5b10ddfc832 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -695,7 +695,8 @@ static void update_head(const struct ref *our, const struct
ref *remote,
install_branch_config(0, head, option_origin,
our->name);
}
} else if (our) {
- struct commit *c = lookup_commit_reference(>old_oid);
+ struct commit *c = lookup_commit_reference(the_repository,
+ >old_oid);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
update_ref(msg, "HEAD",
[PATCH v2 12/31] tag: add repository argument to lookup_tag
Add a repository argument to allow the callers of lookup_tag
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
builtin/describe.c | 6 +++---
builtin/pack-objects.c | 2 +-
builtin/replace.c | 2 +-
log-tree.c | 2 +-
object.c | 2 +-
sha1-name.c| 2 +-
tag.c | 4 ++--
tag.h | 4 ++--
8 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 080b5ce082c..6fb713b6be1 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -93,13 +93,13 @@ static int replace_name(struct commit_name *e,
struct tag *t;
if (!e->tag) {
- t = lookup_tag(>oid);
+ t = lookup_tag(the_repository, >oid);
if (!t || parse_tag(t))
return 1;
e->tag = t;
}
- t = lookup_tag(oid);
+ t = lookup_tag(the_repository, oid);
if (!t || parse_tag(t))
return 0;
*tag = t;
@@ -267,7 +267,7 @@ static unsigned long finish_depth_computation(
static void append_name(struct commit_name *n, struct strbuf *dst)
{
if (n->prio == 2 && !n->tag) {
- n->tag = lookup_tag(>oid);
+ n->tag = lookup_tag(the_repository, >oid);
if (!n->tag || parse_tag(n->tag))
die(_("annotated tag %s not available"), n->path);
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8c108109985..eb11ef4a5dd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2365,7 +2365,7 @@ static void add_tag_chain(const struct object_id *oid)
if (packlist_find(_pack, oid->hash, NULL))
return;
- tag = lookup_tag(oid);
+ tag = lookup_tag(the_repository, oid);
while (1) {
if (!tag || parse_tag(tag) || !tag->tagged)
die("unable to pack objects reachable from tag %s",
diff --git a/builtin/replace.c b/builtin/replace.c
index 14e142d5a80..8c8cec4aae6 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -359,7 +359,7 @@ static void check_one_mergetag(struct commit *commit,
int i;
hash_object_file(extra->value, extra->len, type_name(OBJ_TAG),
_oid);
- tag = lookup_tag(_oid);
+ tag = lookup_tag(the_repository, _oid);
if (!tag)
die(_("bad mergetag in commit '%s'"), ref);
if (parse_tag_buffer(tag, extra->value, extra->len))
diff --git a/log-tree.c b/log-tree.c
index a47283eca64..727758eb442 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -506,7 +506,7 @@ static void show_one_mergetag(struct commit *commit,
size_t payload_size, gpg_message_offset;
hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), );
- tag = lookup_tag();
+ tag = lookup_tag(the_repository, );
if (!tag)
return; /* error message already given */
diff --git a/object.c b/object.c
index ecaac5c2990..080e9b36eaf 100644
--- a/object.c
+++ b/object.c
@@ -223,7 +223,7 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
obj = >object;
}
} else if (type == OBJ_TAG) {
- struct tag *tag = lookup_tag(oid);
+ struct tag *tag = lookup_tag(the_repository, oid);
if (tag) {
if (parse_tag_buffer(tag, buffer, size))
return NULL;
diff --git a/sha1-name.c b/sha1-name.c
index 5eef8ddd6d6..98b66c4896c 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -359,7 +359,7 @@ static int show_ambiguous_object(const struct object_id
*oid, void *data)
format_commit_message(commit, " %ad - %s", , );
}
} else if (type == OBJ_TAG) {
- struct tag *tag = lookup_tag(oid);
+ struct tag *tag = lookup_tag(the_repository, oid);
if (!parse_tag(tag) && tag->tag)
strbuf_addf(, " %s", tag->tag);
}
diff --git a/tag.c b/tag.c
index 5dcdf7bf6f9..5b41fc71fad 100644
--- a/tag.c
+++ b/tag.c
@@ -92,7 +92,7 @@ struct object *deref_tag_noverify(struct object *o)
return o;
}
-struct tag *lookup_tag(const struct object_id *oid)
+struct tag *lookup_tag_the_repository(const struct object_id *oid)
{
struct object *obj = lookup_object(the_repository, oid->hash);
if (!obj)
@@ -160,7 +160,7 @@ int parse_tag_buffer(struct tag *item, const void *data,
[PATCH v2 08/31] commit: add repository argument to lookup_commit
Add a repository argument to allow callers of lookup_commit to be more
specific about which repository to handle. This is a small mechanical
change; it doesn't change the implementation to handle repositories
other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
builtin/am.c| 3 ++-
builtin/commit-tree.c | 4 +++-
builtin/diff-tree.c | 2 +-
builtin/fast-export.c | 2 +-
builtin/fmt-merge-msg.c | 2 +-
builtin/merge-base.c| 2 +-
builtin/verify-commit.c | 4 +++-
commit-graph.c | 10 +-
commit.c| 7 ---
commit.h| 3 ++-
fetch-pack.c| 5 +++--
log-tree.c | 2 +-
notes-utils.c | 4 +++-
object.c| 2 +-
sequencer.c | 4 ++--
sha1-name.c | 2 +-
shallow.c | 17 ++---
tag.c | 2 +-
tree.c | 2 +-
19 files changed, 46 insertions(+), 33 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index f4b510bcc5f..fb7c21f7103 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1633,7 +1633,8 @@ static void do_commit(const struct am_state *state)
if (!get_oid_commit("HEAD", )) {
old_oid =
- commit_list_insert(lookup_commit(), );
+ commit_list_insert(lookup_commit(the_repository, ),
+ );
} else {
old_oid = NULL;
say(state, stderr, _("applying to an empty history"));
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 9fbd3529fb1..9ec36a82b63 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -6,6 +6,7 @@
#include "cache.h"
#include "config.h"
#include "object-store.h"
+#include "repository.h"
#include "commit.h"
#include "tree.h"
#include "builtin.h"
@@ -60,7 +61,8 @@ int cmd_commit_tree(int argc, const char **argv, const char
*prefix)
if (get_oid_commit(argv[i], ))
die("Not a valid object name %s", argv[i]);
assert_oid_type(, OBJ_COMMIT);
- new_parent(lookup_commit(), );
+ new_parent(lookup_commit(the_repository, ),
+);
continue;
}
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index a5718d96ee2..91ba67070e2 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -25,7 +25,7 @@ static int stdin_diff_commit(struct commit *commit, const
char *p)
/* Graft the fake parents locally to the commit */
while (isspace(*p++) && !parse_oid_hex(p, , )) {
- struct commit *parent = lookup_commit();
+ struct commit *parent = lookup_commit(the_repository, );
if (!pptr) {
/* Free the real parent list */
free_commit_list(commit->parents);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 23ca46e6008..bc837d2593c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -964,7 +964,7 @@ static void import_marks(char *input_file)
/* only commits */
continue;
- commit = lookup_commit();
+ commit = lookup_commit(the_repository, );
if (!commit)
die("not a commit? can't happen: %s", oid_to_hex());
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 5e44589b545..36318ef46e7 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -572,7 +572,7 @@ static void find_merge_parents(struct merge_parents *result,
commit_list_insert(parent, );
add_merge_parent(result, >oid, >object.oid);
}
- head_commit = lookup_commit(head);
+ head_commit = lookup_commit(the_repository, head);
if (head_commit)
commit_list_insert(head_commit, );
reduce_heads_replace();
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index bbead6f33e5..08d91b1f0c0 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -124,7 +124,7 @@ static void add_one_commit(struct object_id *oid, struct
rev_collect *revs)
if (is_null_oid(oid))
return;
- commit = lookup_commit(oid);
+ commit = lookup_commit(the_repository, oid);
if (!commit ||
(commit->object.flags & TMP_MARK) ||
parse_commit(commit))
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index f6922da16d6..7772c07ed7a 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -9,6 +9,7 @@
#include "config.h"
#include "builtin.h"
#include "object-store.h"
+#include "repository.h"
#include "commit.h"
#include
[PATCH v2 10/31] commit: add repository argument to set_commit_buffer
Add a repository argument to allow callers of set_commit_buffer to
be more specific about which repository to handle. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
blame.c | 2 +-
commit.c | 4 ++--
commit.h | 3 ++-
object.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/blame.c b/blame.c
index 86d0dd73338..50808c0ff44 100644
--- a/blame.c
+++ b/blame.c
@@ -158,7 +158,7 @@ static void set_commit_buffer_from_strbuf(struct commit *c,
struct strbuf *sb)
{
size_t len;
void *buf = strbuf_detach(sb, );
- set_commit_buffer(c, buf, len);
+ set_commit_buffer(the_repository, c, buf, len);
}
/*
diff --git a/commit.c b/commit.c
index dda585025ed..667d4dc6cfd 100644
--- a/commit.c
+++ b/commit.c
@@ -249,7 +249,7 @@ struct commit_buffer {
define_commit_slab(buffer_slab, struct commit_buffer);
static struct buffer_slab buffer_slab = COMMIT_SLAB_INIT(1, buffer_slab);
-void set_commit_buffer(struct commit *commit, void *buffer, unsigned long size)
+void set_commit_buffer_the_repository(struct commit *commit, void *buffer,
unsigned long size)
{
struct commit_buffer *v = buffer_slab_at(_slab, commit);
v->buffer = buffer;
@@ -418,7 +418,7 @@ int parse_commit_gently(struct commit *item, int
quiet_on_missing)
}
ret = parse_commit_buffer(the_repository, item, buffer, size);
if (save_commit_buffer && !ret) {
- set_commit_buffer(item, buffer, size);
+ set_commit_buffer(the_repository, item, buffer, size);
return 0;
}
free(buffer);
diff --git a/commit.h b/commit.h
index 05b9eccaf66..a1ecd067ccc 100644
--- a/commit.h
+++ b/commit.h
@@ -85,7 +85,8 @@ void parse_commit_or_die(struct commit *item);
* Associate an object buffer with the commit. The ownership of the
* memory is handed over to the commit, and must be free()-able.
*/
-void set_commit_buffer(struct commit *, void *buffer, unsigned long size);
+#define set_commit_buffer(r, c, b, s) set_commit_buffer_##r(c, b, s)
+void set_commit_buffer_the_repository(struct commit *, void *buffer, unsigned
long size);
/*
* Get any cached object buffer associated with the commit. Returns NULL
diff --git a/object.c b/object.c
index fe1f84f7c6c..ff0ba98675e 100644
--- a/object.c
+++ b/object.c
@@ -217,7 +217,7 @@ struct object *parse_object_buffer_the_repository(const
struct object_id *oid, e
if (parse_commit_buffer(the_repository, commit, buffer,
size))
return NULL;
if (!get_cached_commit_buffer(commit, NULL)) {
- set_commit_buffer(commit, buffer, size);
+ set_commit_buffer(the_repository, commit,
buffer, size);
*eaten_p = 1;
}
obj = >object;
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 15/31] object: allow object_as_type to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
object.c | 4 ++--
object.h | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/object.c b/object.c
index c6779ee596f..876ca4977e2 100644
--- a/object.c
+++ b/object.c
@@ -158,13 +158,13 @@ void *create_object(struct repository *r, const unsigned
char *sha1, void *o)
return obj;
}
-void *object_as_type_the_repository(struct object *obj, enum object_type type,
int quiet)
+void *object_as_type(struct repository *r, struct object *obj, enum
object_type type, int quiet)
{
if (obj->type == type)
return obj;
else if (obj->type == OBJ_NONE) {
if (type == OBJ_COMMIT)
- ((struct commit *)obj)->index =
alloc_commit_index(the_repository);
+ ((struct commit *)obj)->index = alloc_commit_index(r);
obj->type = type;
return obj;
}
diff --git a/object.h b/object.h
index 5425d8e647c..bd25155480f 100644
--- a/object.h
+++ b/object.h
@@ -115,8 +115,7 @@ struct object *lookup_object_the_repository(const unsigned
char *sha1);
extern void *create_object(struct repository *r, const unsigned char *sha1,
void *obj);
-#define object_as_type(r, o, t, q) object_as_type_##r(o, t, q)
-void *object_as_type_the_repository(struct object *obj, enum object_type type,
int quiet);
+void *object_as_type(struct repository *r, struct object *obj, enum
object_type type, int quiet);
/*
* Returns the object, having parsed it to find out what it is.
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 17/31] blob: allow lookup_blob to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
blob.c | 10 +-
blob.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/blob.c b/blob.c
index 17b9314f0a0..342bdbb1bbe 100644
--- a/blob.c
+++ b/blob.c
@@ -5,13 +5,13 @@
const char *blob_type = "blob";
-struct blob *lookup_blob_the_repository(const struct object_id *oid)
+struct blob *lookup_blob(struct repository *r, const struct object_id *oid)
{
- struct object *obj = lookup_object(the_repository, oid->hash);
+ struct object *obj = lookup_object(r, oid->hash);
if (!obj)
- return create_object(the_repository, oid->hash,
-alloc_blob_node(the_repository));
- return object_as_type(the_repository, obj, OBJ_BLOB, 0);
+ return create_object(r, oid->hash,
+alloc_blob_node(r));
+ return object_as_type(r, obj, OBJ_BLOB, 0);
}
int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size)
diff --git a/blob.h b/blob.h
index 08bc34487a0..16648720557 100644
--- a/blob.h
+++ b/blob.h
@@ -9,8 +9,7 @@ struct blob {
struct object object;
};
-#define lookup_blob(r, o) lookup_blob_##r(o)
-struct blob *lookup_blob_the_repository(const struct object_id *oid);
+struct blob *lookup_blob(struct repository *r, const struct object_id *oid);
int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 14/31] tag: add repository argument to deref_tag
Add a repository argument to allow the callers of deref_tag
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
blame.c | 6 +++---
builtin/diff.c | 2 +-
builtin/fmt-merge-msg.c | 3 ++-
builtin/grep.c | 3 ++-
builtin/name-rev.c | 3 ++-
commit.c| 3 ++-
fetch-pack.c| 9 ++---
http-backend.c | 2 +-
http-push.c | 2 +-
line-log.c | 2 +-
merge-recursive.c | 3 ++-
remote.c| 6 --
server-info.c | 2 +-
sha1-name.c | 11 +++
shallow.c | 4 +++-
tag.c | 2 +-
tag.h | 3 ++-
upload-pack.c | 2 +-
18 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/blame.c b/blame.c
index 50808c0ff44..aad53c0f904 100644
--- a/blame.c
+++ b/blame.c
@@ -1674,7 +1674,7 @@ static struct commit *find_single_final(struct rev_info
*revs,
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
continue;
- obj = deref_tag(obj, NULL, 0);
+ obj = deref_tag(the_repository, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (found)
@@ -1705,7 +1705,7 @@ static struct commit *dwim_reverse_initial(struct
rev_info *revs,
/* Is that sole rev a committish? */
obj = revs->pending.objects[0].item;
- obj = deref_tag(obj, NULL, 0);
+ obj = deref_tag(the_repository, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
return NULL;
@@ -1741,7 +1741,7 @@ static struct commit *find_single_initial(struct rev_info
*revs,
struct object *obj = revs->pending.objects[i].item;
if (!(obj->flags & UNINTERESTING))
continue;
- obj = deref_tag(obj, NULL, 0);
+ obj = deref_tag(the_repository, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (found)
diff --git a/builtin/diff.c b/builtin/diff.c
index 0b7d0d612dd..1dd7dd4a267 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -395,7 +395,7 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
int flags = (obj->flags & UNINTERESTING);
if (!obj->parsed)
obj = parse_object(the_repository, >oid);
- obj = deref_tag(obj, NULL, 0);
+ obj = deref_tag(the_repository, obj, NULL, 0);
if (!obj)
die(_("invalid object '%s' given."), name);
if (obj->type == OBJ_COMMIT)
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 36318ef46e7..ff165c0fcd2 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -344,7 +344,8 @@ static void shortlog(const char *name,
const struct object_id *oid = _data->oid;
int limit = opts->shortlog_len;
- branch = deref_tag(parse_object(the_repository, oid), oid_to_hex(oid),
+ branch = deref_tag(the_repository, parse_object(the_repository, oid),
+ oid_to_hex(oid),
GIT_SHA1_HEXSZ);
if (!branch || branch->type != OBJ_COMMIT)
return;
diff --git a/builtin/grep.c b/builtin/grep.c
index 6e7bc76785a..c93c33cd28f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -646,7 +646,8 @@ static int grep_objects(struct grep_opt *opt, const struct
pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
- real_obj = deref_tag(list->objects[i].item, NULL, 0);
+ real_obj = deref_tag(the_repository, list->objects[i].item,
+NULL, 0);
/* load the gitmodules file for this rev */
if (recurse_submodules) {
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index f6eb419a029..f1cb45c2274 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -455,7 +455,8 @@ int cmd_name_rev(int argc, const char **argv, const char
*prefix)
commit = NULL;
object = parse_object(the_repository, );
if (object) {
- struct object *peeled = deref_tag(object, *argv, 0);
+ struct object *peeled = deref_tag(the_repository,
+ object, *argv, 0);
if
[PATCH v2 18/31] tree: allow lookup_tree to handle arbitrary repositories
Signed-off-by: Stefan Beller
---
tree.c | 10 +-
tree.h | 3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/tree.c b/tree.c
index 33063b8dde0..2bffc0714a8 100644
--- a/tree.c
+++ b/tree.c
@@ -195,13 +195,13 @@ int read_tree(struct tree *tree, int stage, struct
pathspec *match,
return 0;
}
-struct tree *lookup_tree_the_repository(const struct object_id *oid)
+struct tree *lookup_tree(struct repository *r, const struct object_id *oid)
{
- struct object *obj = lookup_object(the_repository, oid->hash);
+ struct object *obj = lookup_object(r, oid->hash);
if (!obj)
- return create_object(the_repository, oid->hash,
-alloc_tree_node(the_repository));
- return object_as_type(the_repository, obj, OBJ_TREE, 0);
+ return create_object(r, oid->hash,
+alloc_tree_node(r));
+ return object_as_type(r, obj, OBJ_TREE, 0);
}
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size)
diff --git a/tree.h b/tree.h
index 2ea21ed174b..d4807dc8058 100644
--- a/tree.h
+++ b/tree.h
@@ -12,8 +12,7 @@ struct tree {
unsigned long size;
};
-#define lookup_tree(r, oid) lookup_tree_##r(oid)
-struct tree *lookup_tree_the_repository(const struct object_id *oid);
+struct tree *lookup_tree(struct repository *r, const struct object_id *oid);
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size);
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 01/31] object: add repository argument to lookup_object
Add a repository argument to allow callers of lookup_object to be more
specific about which repository to handle. This is a small mechanical
change; it doesn't change the implementation to handle repositories
other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
blob.c | 2 +-
builtin/fast-export.c| 5 +++--
builtin/fsck.c | 5 +++--
builtin/name-rev.c | 3 ++-
builtin/prune.c | 2 +-
builtin/unpack-objects.c | 2 +-
commit.c | 2 +-
fetch-pack.c | 13 +++--
http-push.c | 2 +-
object.c | 8
object.h | 3 ++-
reachable.c | 4 ++--
tag.c| 2 +-
tree.c | 2 +-
upload-pack.c| 2 +-
15 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/blob.c b/blob.c
index 458dafa811e..75b737a761e 100644
--- a/blob.c
+++ b/blob.c
@@ -7,7 +7,7 @@ const char *blob_type = "blob";
struct blob *lookup_blob(const struct object_id *oid)
{
- struct object *obj = lookup_object(oid->hash);
+ struct object *obj = lookup_object(the_repository, oid->hash);
if (!obj)
return create_object(the_repository, oid->hash,
alloc_blob_node(the_repository));
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 0da875b58c9..24d42842f9d 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -231,7 +231,7 @@ static void export_blob(const struct object_id *oid)
if (is_null_oid(oid))
return;
- object = lookup_object(oid->hash);
+ object = lookup_object(the_repository, oid->hash);
if (object && object->flags & SHOWN)
return;
@@ -403,7 +403,8 @@ static void show_filemodify(struct diff_queue_struct *q,
anonymize_sha1(>oid) :
spec->oid.hash));
else {
- struct object *object =
lookup_object(spec->oid.hash);
+ struct object *object =
lookup_object(the_repository,
+
spec->oid.hash);
printf("M %06o :%d ", spec->mode,
get_object_mark(object));
}
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4b410cba54e..98fdeef5407 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -410,7 +410,7 @@ static void fsck_handle_reflog_oid(const char *refname,
struct object_id *oid,
struct object *obj;
if (!is_null_oid(oid)) {
- obj = lookup_object(oid->hash);
+ obj = lookup_object(the_repository, oid->hash);
if (obj && (obj->flags & HAS_OBJ)) {
if (timestamp && name_objects)
add_decoration(fsck_walk_options.object_names,
@@ -762,7 +762,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
const char *arg = argv[i];
struct object_id oid;
if (!get_oid(arg, )) {
- struct object *obj = lookup_object(oid.hash);
+ struct object *obj = lookup_object(the_repository,
+ oid.hash);
if (!obj || !(obj->flags & HAS_OBJ)) {
if (is_promisor_object())
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index de54fa93e4f..f6eb419a029 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -379,7 +379,8 @@ static void name_rev_line(char *p, struct name_ref_data
*data)
*(p+1) = 0;
if (!get_oid(p - (GIT_SHA1_HEXSZ - 1), )) {
struct object *o =
- lookup_object(oid.hash);
+ lookup_object(the_repository,
+ oid.hash);
if (o)
name = get_rev_name(o, );
}
diff --git a/builtin/prune.c b/builtin/prune.c
index 70ec35aa058..72b0621b768 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -40,7 +40,7 @@ static int prune_object(const struct object_id *oid, const
char *fullpath,
* Do we know about this object?
* It must have been reachable
*/
- if (lookup_object(oid->hash))
+ if (lookup_object(the_repository, oid->hash))
return 0;
if (lstat(fullpath, )) {
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index ded798b72ff..9a4d2708123
[PATCH v2 04/31] blob: add repository argument to lookup_blob
Add a repository argument to allow the callers of lookup_blob
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
blob.c | 2 +-
blob.h | 3 ++-
builtin/fast-export.c| 2 +-
builtin/fsck.c | 3 ++-
builtin/index-pack.c | 2 +-
builtin/merge-tree.c | 3 ++-
builtin/unpack-objects.c | 2 +-
fsck.c | 2 +-
http-push.c | 3 ++-
list-objects.c | 2 +-
object.c | 4 ++--
reachable.c | 2 +-
revision.c | 4 ++--
tag.c| 2 +-
walker.c | 3 ++-
15 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/blob.c b/blob.c
index dada295698c..17b9314f0a0 100644
--- a/blob.c
+++ b/blob.c
@@ -5,7 +5,7 @@
const char *blob_type = "blob";
-struct blob *lookup_blob(const struct object_id *oid)
+struct blob *lookup_blob_the_repository(const struct object_id *oid)
{
struct object *obj = lookup_object(the_repository, oid->hash);
if (!obj)
diff --git a/blob.h b/blob.h
index 44606168310..08bc34487a0 100644
--- a/blob.h
+++ b/blob.h
@@ -9,7 +9,8 @@ struct blob {
struct object object;
};
-struct blob *lookup_blob(const struct object_id *oid);
+#define lookup_blob(r, o) lookup_blob_##r(o)
+struct blob *lookup_blob_the_repository(const struct object_id *oid);
int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size);
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index a34ab9768f4..23ca46e6008 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -237,7 +237,7 @@ static void export_blob(const struct object_id *oid)
if (anonymize) {
buf = anonymize_blob();
- object = (struct object *)lookup_blob(oid);
+ object = (struct object *)lookup_blob(the_repository, oid);
eaten = 0;
} else {
buf = read_object_file(oid, , );
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 72d7a9cbd8c..0c61249282a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -808,7 +808,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
mode = active_cache[i]->ce_mode;
if (S_ISGITLINK(mode))
continue;
- blob = lookup_blob(_cache[i]->oid);
+ blob = lookup_blob(the_repository,
+ _cache[i]->oid);
if (!blob)
continue;
obj = >object;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0dd10693597..51c5244a15a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -832,7 +832,7 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
if (strict || do_fsck_object) {
read_lock();
if (type == OBJ_BLOB) {
- struct blob *blob = lookup_blob(oid);
+ struct blob *blob = lookup_blob(the_repository, oid);
if (blob)
blob->object.flags |= FLAG_CHECKED;
else
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 8a8d5797520..f8023bae1e2 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -2,6 +2,7 @@
#include "tree-walk.h"
#include "xdiff-interface.h"
#include "object-store.h"
+#include "repository.h"
#include "blob.h"
#include "exec-cmd.h"
#include "merge-blobs.h"
@@ -170,7 +171,7 @@ static struct merge_list *create_entry(unsigned stage,
unsigned mode, const stru
res->stage = stage;
res->path = path;
res->mode = mode;
- res->blob = lookup_blob(oid);
+ res->blob = lookup_blob(the_repository, oid);
return res;
}
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 8e454c48649..dfea7790fde 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -254,7 +254,7 @@ static void write_object(unsigned nr, enum object_type type,
added_object(nr, type, buf, size);
free(buf);
- blob = lookup_blob(_list[nr].oid);
+ blob = lookup_blob(the_repository, _list[nr].oid);
if (blob)
blob->object.flags |= FLAG_WRITTEN;
else
diff --git a/fsck.c b/fsck.c
index f9476f56e93..2d372f2a3f3 100644
--- a/fsck.c
+++ b/fsck.c
@@ -367,7 +367,7 @@ static int fsck_walk_tree(struct tree *tree, void *data,
struct fsck_options *op
result = options->walk(obj,
[PATCH v2 06/31] commit: add repository argument to lookup_commit_reference_gently
Add a repository argument to allow callers of
lookup_commit_reference_gently to be more specific about which
repository to handle. This is a small mechanical change; it doesn't
change the implementation to handle repositories other than
the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Stefan Beller
---
archive.c | 2 +-
blame.c | 3 ++-
builtin/checkout.c| 6 +++---
builtin/describe.c| 5 +++--
builtin/fetch.c | 9 ++---
builtin/reflog.c | 10 ++
builtin/show-branch.c | 3 ++-
bundle.c | 2 +-
commit-graph.c| 2 +-
commit.c | 6 +++---
commit.h | 5 -
fast-import.c | 6 --
notes-cache.c | 3 ++-
ref-filter.c | 6 --
remote.c | 9 +
sequencer.c | 2 +-
sha1-name.c | 4 ++--
shallow.c | 9 ++---
walker.c | 3 ++-
wt-status.c | 2 +-
20 files changed, 59 insertions(+), 38 deletions(-)
diff --git a/archive.c b/archive.c
index 9da1e3664a6..983a45ba3e7 100644
--- a/archive.c
+++ b/archive.c
@@ -380,7 +380,7 @@ static void parse_treeish_arg(const char **argv,
if (get_oid(name, ))
die("Not a valid object name");
- commit = lookup_commit_reference_gently(, 1);
+ commit = lookup_commit_reference_gently(the_repository, , 1);
if (commit) {
commit_sha1 = commit->object.oid.hash;
archive_time = commit->date;
diff --git a/blame.c b/blame.c
index 2904b0500d9..726a7a76f20 100644
--- a/blame.c
+++ b/blame.c
@@ -1712,7 +1712,8 @@ static struct commit *dwim_reverse_initial(struct
rev_info *revs,
/* Do we have HEAD? */
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, _oid, NULL))
return NULL;
- head_commit = lookup_commit_reference_gently(_oid, 1);
+ head_commit = lookup_commit_reference_gently(the_repository,
+_oid, 1);
if (!head_commit)
return NULL;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 105e07981ff..31a71f75625 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -379,7 +379,7 @@ static int checkout_paths(const struct checkout_opts *opts,
die(_("unable to write new index file"));
read_ref_full("HEAD", 0, , NULL);
- head = lookup_commit_reference_gently(, 1);
+ head = lookup_commit_reference_gently(the_repository, , 1);
errs |= post_checkout_hook(head, head, 0);
return errs;
@@ -823,7 +823,7 @@ static int switch_branches(const struct checkout_opts *opts,
memset(_branch_info, 0, sizeof(old_branch_info));
old_branch_info.path = path_to_free = resolve_refdup("HEAD", 0, ,
);
if (old_branch_info.path)
- old_branch_info.commit = lookup_commit_reference_gently(,
1);
+ old_branch_info.commit =
lookup_commit_reference_gently(the_repository, , 1);
if (!(flag & REF_ISSYMREF))
old_branch_info.path = NULL;
@@ -997,7 +997,7 @@ static int parse_branchname_arg(int argc, const char **argv,
else
new_branch_info->path = NULL; /* not an existing branch */
- new_branch_info->commit = lookup_commit_reference_gently(rev, 1);
+ new_branch_info->commit =
lookup_commit_reference_gently(the_repository, rev, 1);
if (!new_branch_info->commit) {
/* not a commit */
*source_tree = parse_tree_indirect(rev);
diff --git a/builtin/describe.c b/builtin/describe.c
index 58341c6f9ed..f3dfd0228be 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -331,7 +331,8 @@ static void describe_commit(struct object_id *oid, struct
strbuf *dst)
init_commit_names(_names);
n = hashmap_iter_first(, );
for (; n; n = hashmap_iter_next()) {
- c = lookup_commit_reference_gently(>peeled, 1);
+ c = lookup_commit_reference_gently(the_repository,
+ >peeled, 1);
if (c)
*commit_names_at(_names, c) = n;
}
@@ -509,7 +510,7 @@ static void describe(const char *arg, int last_one)
if (get_oid(arg, ))
die(_("Not a valid object name %s"), arg);
- cmit = lookup_commit_reference_gently(, 1);
+ cmit = lookup_commit_reference_gently(the_repository, , 1);
if (cmit)
describe_commit(, );
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6ce11bfb094..499cf6fcf12 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -690,8 +690,10 @@ static int update_local_ref(struct ref *ref,
return r;
}
-
[PATCH v2 00/31] object-store: lookup_commit
* removed mentions of cooci patches * added forward declaration of commit buffer slabs. * dropped 3 patches that add the repository to lookup_unkonwn_object, parse_commit and parse_commit_gently, but were not converting those functions. We'll convert these in the next series, as this series is growing big already. * This series can be found as branch 'object-store-lookup-commit' on github, it applies on top of nd/commit-util-to-slab merged with sb/object-store-grafts v1, https://public-inbox.org/git/20180530004810.30076-1-sbel...@google.com/ This applies on the merge of nd/commit-util-to-slab and sb/object-store-grafts, and is available at http://github.com/stefanbeller/ as branch object-store-lookup-commit as the merge has some merge conflicts as well as syntactical conflicts (upload-pack.c and fetch-pack.c introduce new calls of functions that would want to take a repository struct in the object-store-grafts series) As layed out in https://public-inbox.org/git/20180517225154.9200-1-sbel...@google.com/ this is getting close to finishing the set of object store series though the last unfinished part of this RFC hints at new work on the plate: * To give this series a nice polish, we'd want to convert parse_commit, too. But that requires the conversion of the new commit graph. Maybe we need to split this series into 2. * Once this is in good shape we can talk about converting parts of the revision walking code, * which then can be used by the submodule code as the end goal for the object store series. Thanks, Stefan Stefan Beller (31): object: add repository argument to lookup_object object: add repository argument to parse_object_buffer object: add repository argument to object_as_type blob: add repository argument to lookup_blob tree: add repository argument to lookup_tree commit: add repository argument to lookup_commit_reference_gently commit: add repository argument to lookup_commit_reference commit: add repository argument to lookup_commit commit: add repository argument to parse_commit_buffer commit: add repository argument to set_commit_buffer commit: add repository argument to get_cached_commit_buffer tag: add repository argument to lookup_tag tag: add repository argument to parse_tag_buffer tag: add repository argument to deref_tag object: allow object_as_type to handle arbitrary repositories object: allow lookup_object to handle arbitrary repositories blob: allow lookup_blob to handle arbitrary repositories tree: allow lookup_tree to handle arbitrary repositories commit: allow lookup_commit to handle arbitrary repositories tag: allow lookup_tag to handle arbitrary repositories tag: allow parse_tag_buffer to handle arbitrary repositories commit.c: allow parse_commit_buffer to handle arbitrary repositories commit-slabs: remove realloc counter outside of slab struct commit.c: migrate the commit buffer to the parsed object store commit.c: allow set_commit_buffer to handle arbitrary repositories commit.c: allow get_cached_commit_buffer to handle arbitrary repositories object.c: allow parse_object_buffer to handle arbitrary repositories object.c: allow parse_object to handle arbitrary repositories tag.c: allow deref_tag to handle arbitrary repositories commit.c: allow lookup_commit_reference_gently to handle arbitrary repositories commit.c: allow lookup_commit_reference to handle arbitrary repositories archive.c| 2 +- bisect.c | 2 +- blame.c | 13 +++ blob.c | 10 +++--- blob.h | 2 +- branch.c | 2 +- builtin/am.c | 9 +++-- builtin/branch.c | 7 ++-- builtin/checkout.c | 6 ++-- builtin/clone.c | 3 +- builtin/commit-tree.c| 4 ++- builtin/describe.c | 13 +++ builtin/diff-tree.c | 6 ++-- builtin/diff.c | 5 +-- builtin/fast-export.c| 12 --- builtin/fetch.c | 9 +++-- builtin/fmt-merge-msg.c | 5 +-- builtin/fsck.c | 16 + builtin/grep.c | 3 +- builtin/index-pack.c | 5 +-- builtin/log.c| 7 ++-- builtin/merge-base.c | 7 ++-- builtin/merge-tree.c | 3 +- builtin/name-rev.c | 6 ++-- builtin/notes.c | 3 +- builtin/pack-objects.c | 2 +- builtin/prune.c | 2 +- builtin/pull.c | 15 +--- builtin/reflog.c | 12 --- builtin/replace.c| 4 +-- builtin/reset.c | 4 +-- builtin/rev-parse.c | 6 ++-- builtin/show-branch.c| 5 +-- builtin/tag.c| 2 +- builtin/unpack-objects.c | 7 ++-- builtin/verify-commit.c | 4 ++- bundle.c | 5 +-- cache-tree.c | 3 +- commit-graph.c | 14 commit-slab-impl.h | 3 -- commit.c | 77 +--- commit.h
[PATCH v2 03/31] object: add repository argument to object_as_type
Signed-off-by: Stefan Beller
---
blob.c | 2 +-
builtin/fsck.c | 2 +-
commit.c | 4 ++--
object.c | 2 +-
object.h | 3 ++-
refs.c | 2 +-
tag.c | 2 +-
tree.c | 2 +-
8 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/blob.c b/blob.c
index 75b737a761e..dada295698c 100644
--- a/blob.c
+++ b/blob.c
@@ -11,7 +11,7 @@ struct blob *lookup_blob(const struct object_id *oid)
if (!obj)
return create_object(the_repository, oid->hash,
alloc_blob_node(the_repository));
- return object_as_type(obj, OBJ_BLOB, 0);
+ return object_as_type(the_repository, obj, OBJ_BLOB, 0);
}
int parse_blob_buffer(struct blob *item, void *buffer, unsigned long size)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6b8c9074920..72d7a9cbd8c 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -70,7 +70,7 @@ static const char *printable_type(struct object *obj)
enum object_type type = oid_object_info(the_repository,
>oid, NULL);
if (type > 0)
- object_as_type(obj, type, 0);
+ object_as_type(the_repository, obj, type, 0);
}
ret = type_name(obj->type);
diff --git a/commit.c b/commit.c
index 97b4ccde8f0..d76d64d4dfc 100644
--- a/commit.c
+++ b/commit.c
@@ -31,7 +31,7 @@ struct commit *lookup_commit_reference_gently(const struct
object_id *oid,
if (!obj)
return NULL;
- return object_as_type(obj, OBJ_COMMIT, quiet);
+ return object_as_type(the_repository, obj, OBJ_COMMIT, quiet);
}
struct commit *lookup_commit_reference(const struct object_id *oid)
@@ -57,7 +57,7 @@ struct commit *lookup_commit(const struct object_id *oid)
if (!obj)
return create_object(the_repository, oid->hash,
alloc_commit_node(the_repository));
- return object_as_type(obj, OBJ_COMMIT, 0);
+ return object_as_type(the_repository, obj, OBJ_COMMIT, 0);
}
struct commit *lookup_commit_reference_by_name(const char *name)
diff --git a/object.c b/object.c
index 0ffcf619cef..b221ba714a7 100644
--- a/object.c
+++ b/object.c
@@ -158,7 +158,7 @@ void *create_object(struct repository *r, const unsigned
char *sha1, void *o)
return obj;
}
-void *object_as_type(struct object *obj, enum object_type type, int quiet)
+void *object_as_type_the_repository(struct object *obj, enum object_type type,
int quiet)
{
if (obj->type == type)
return obj;
diff --git a/object.h b/object.h
index 7526ee151f5..5425d8e647c 100644
--- a/object.h
+++ b/object.h
@@ -115,7 +115,8 @@ struct object *lookup_object_the_repository(const unsigned
char *sha1);
extern void *create_object(struct repository *r, const unsigned char *sha1,
void *obj);
-void *object_as_type(struct object *obj, enum object_type type, int quiet);
+#define object_as_type(r, o, t, q) object_as_type_##r(o, t, q)
+void *object_as_type_the_repository(struct object *obj, enum object_type type,
int quiet);
/*
* Returns the object, having parsed it to find out what it is.
diff --git a/refs.c b/refs.c
index 23d53957deb..0304e2b866f 100644
--- a/refs.c
+++ b/refs.c
@@ -305,7 +305,7 @@ enum peel_status peel_object(const struct object_id *name,
struct object_id *oid
if (o->type == OBJ_NONE) {
int type = oid_object_info(the_repository, name, NULL);
- if (type < 0 || !object_as_type(o, type, 0))
+ if (type < 0 || !object_as_type(the_repository, o, type, 0))
return PEEL_INVALID;
}
diff --git a/tag.c b/tag.c
index 1b95eb9f07f..a14a4f23037 100644
--- a/tag.c
+++ b/tag.c
@@ -98,7 +98,7 @@ struct tag *lookup_tag(const struct object_id *oid)
if (!obj)
return create_object(the_repository, oid->hash,
alloc_tag_node(the_repository));
- return object_as_type(obj, OBJ_TAG, 0);
+ return object_as_type(the_repository, obj, OBJ_TAG, 0);
}
static timestamp_t parse_tag_date(const char *buf, const char *tail)
diff --git a/tree.c b/tree.c
index 47ecc85f90a..bd74ed66e23 100644
--- a/tree.c
+++ b/tree.c
@@ -201,7 +201,7 @@ struct tree *lookup_tree(const struct object_id *oid)
if (!obj)
return create_object(the_repository, oid->hash,
alloc_tree_node(the_repository));
- return object_as_type(obj, OBJ_TREE, 0);
+ return object_as_type(the_repository, obj, OBJ_TREE, 0);
}
int parse_tree_buffer(struct tree *item, void *buffer, unsigned long size)
--
2.18.0.rc1.244.gcf134e6275-goog
[PATCH v2 02/31] object: add repository argument to parse_object_buffer
Add a repository argument to allow the callers of parse_object_buffer
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
builtin/fast-export.c| 3 ++-
builtin/fsck.c | 6 --
builtin/index-pack.c | 3 ++-
builtin/unpack-objects.c | 3 ++-
object.c | 5 +++--
object.h | 3 ++-
ref-filter.c | 3 ++-
7 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 24d42842f9d..a34ab9768f4 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -245,7 +245,8 @@ static void export_blob(const struct object_id *oid)
die ("Could not read blob %s", oid_to_hex(oid));
if (check_object_signature(oid, buf, size, type_name(type)) < 0)
die("sha1 mismatch in blob %s", oid_to_hex(oid));
- object = parse_object_buffer(oid, type, size, buf, );
+ object = parse_object_buffer(the_repository, oid, type,
+size, buf, );
}
if (!object)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 98fdeef5407..6b8c9074920 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -392,7 +392,8 @@ static int fsck_obj_buffer(const struct object_id *oid,
enum object_type type,
* verify_packfile(), data_valid variable for details.
*/
struct object *obj;
- obj = parse_object_buffer(oid, type, size, buffer, eaten);
+ obj = parse_object_buffer(the_repository, oid, type, size, buffer,
+ eaten);
if (!obj) {
errors_found |= ERROR_OBJECT;
return error("%s: object corrupt or missing", oid_to_hex(oid));
@@ -522,7 +523,8 @@ static struct object *parse_loose_object(const struct
object_id *oid,
if (!contents && type != OBJ_BLOB)
die("BUG: read_loose_object streamed a non-blob");
- obj = parse_object_buffer(oid, type, size, contents, );
+ obj = parse_object_buffer(the_repository, oid, type, size,
+ contents, );
if (!eaten)
free(contents);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index e2f670bef9e..0dd10693597 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -848,7 +848,8 @@ static void sha1_object(const void *data, struct
object_entry *obj_entry,
* we do not need to free the memory here, as the
* buf is deleted by the caller.
*/
- obj = parse_object_buffer(oid, type, size, buf,
+ obj = parse_object_buffer(the_repository, oid, type,
+ size, buf,
);
if (!obj)
die(_("invalid %s"), type_name(type));
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 9a4d2708123..8e454c48649 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -265,7 +265,8 @@ static void write_object(unsigned nr, enum object_type type,
int eaten;
hash_object_file(buf, size, type_name(type), _list[nr].oid);
added_object(nr, type, buf, size);
- obj = parse_object_buffer(_list[nr].oid, type, size, buf,
+ obj = parse_object_buffer(the_repository, _list[nr].oid,
+ type, size, buf,
);
if (!obj)
die("invalid %s", type_name(type));
diff --git a/object.c b/object.c
index 4de4fa58d59..0ffcf619cef 100644
--- a/object.c
+++ b/object.c
@@ -186,7 +186,7 @@ struct object *lookup_unknown_object(const unsigned char
*sha1)
return obj;
}
-struct object *parse_object_buffer(const struct object_id *oid, enum
object_type type, unsigned long size, void *buffer, int *eaten_p)
+struct object *parse_object_buffer_the_repository(const struct object_id *oid,
enum object_type type, unsigned long size, void *buffer, int *eaten_p)
{
struct object *obj;
*eaten_p = 0;
@@ -278,7 +278,8 @@ struct object *parse_object_the_repository(const struct
object_id *oid)
return NULL;
}
- obj = parse_object_buffer(oid, type, size, buffer, );
+ obj = parse_object_buffer(the_repository, oid, type, size,
+ buffer, );
if (!eaten)
free(buffer);
[PATCH v2 05/31] tree: add repository argument to lookup_tree
Add a repository argument to allow the callers of lookup_tree
to be more specific about which repository to act on. This is a small
mechanical change; it doesn't change the implementation to handle
repositories other than the_repository yet.
As with the previous commits, use a macro to catch callers passing a
repository other than the_repository at compile time.
Signed-off-by: Jonathan Nieder
Signed-off-by: Stefan Beller
---
builtin/am.c| 6 --
builtin/diff-tree.c | 2 +-
builtin/diff.c | 3 ++-
builtin/reflog.c| 2 +-
cache-tree.c| 3 ++-
commit-graph.c | 2 +-
commit.c| 2 +-
fsck.c | 2 +-
http-push.c | 3 ++-
list-objects.c | 2 +-
merge-recursive.c | 6 +++---
object.c| 2 +-
reachable.c | 2 +-
revision.c | 4 ++--
sequencer.c | 2 +-
tag.c | 2 +-
tree.c | 4 ++--
tree.h | 3 ++-
walker.c| 3 ++-
19 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d834f9e62b6..f4b510bcc5f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -32,6 +32,7 @@
#include "apply.h"
#include "string-list.h"
#include "packfile.h"
+#include "repository.h"
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1400,9 +1401,10 @@ static void write_index_patch(const struct am_state
*state)
FILE *fp;
if (!get_oid_tree("HEAD", ))
- tree = lookup_tree();
+ tree = lookup_tree(the_repository, );
else
- tree = lookup_tree(the_hash_algo->empty_tree);
+ tree = lookup_tree(the_repository,
+ the_repository->hash_algo->empty_tree);
fp = xfopen(am_path(state, "patch"), "w");
init_revisions(_info, NULL);
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index d8db8f682f0..29901515a13 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -46,7 +46,7 @@ static int stdin_diff_trees(struct tree *tree1, const char *p)
struct tree *tree2;
if (!isspace(*p++) || parse_oid_hex(p, , ) || *p)
return error("Need exactly two trees, separated by a space");
- tree2 = lookup_tree();
+ tree2 = lookup_tree(the_repository, );
if (!tree2 || parse_tree(tree2))
return -1;
printf("%s %s\n", oid_to_hex(>object.oid),
diff --git a/builtin/diff.c b/builtin/diff.c
index ed6092ef1a3..0b7d0d612dd 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -379,7 +379,8 @@ int cmd_diff(int argc, const char **argv, const char
*prefix)
add_head_to_pending();
if (!rev.pending.nr) {
struct tree *tree;
- tree =
lookup_tree(the_hash_algo->empty_tree);
+ tree = lookup_tree(the_repository,
+
the_repository->hash_algo->empty_tree);
add_pending_object(, >object,
"HEAD");
}
break;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3a751fbaa60..93dabd7ce31 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -66,7 +66,7 @@ static int tree_is_complete(const struct object_id *oid)
int complete;
struct tree *tree;
- tree = lookup_tree(oid);
+ tree = lookup_tree(the_repository, oid);
if (!tree)
return 0;
if (tree->object.flags & SEEN)
diff --git a/cache-tree.c b/cache-tree.c
index 1c338c41f3a..69b214613dd 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -671,7 +671,8 @@ static void prime_cache_tree_rec(struct cache_tree *it,
struct tree *tree)
cnt++;
else {
struct cache_tree_sub *sub;
- struct tree *subtree = lookup_tree(entry.oid);
+ struct tree *subtree = lookup_tree(the_repository,
+ entry.oid);
if (!subtree->object.parsed)
parse_tree(subtree);
sub = cache_tree_sub(it, entry.path);
diff --git a/commit-graph.c b/commit-graph.c
index 71125d7cbb6..88a4b0d2a47 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -261,7 +261,7 @@ static int fill_commit_in_graph(struct commit *item, struct
commit_graph *g, uin
item->graph_pos = pos;
hashcpy(oid.hash, commit_data);
- item->tree = lookup_tree();
+ item->tree = lookup_tree(the_repository, );
date_high = get_be32(commit_data + g->hash_len + 8) & 0x3;
date_low = get_be32(commit_data + g->hash_len + 12);
diff --git a/commit.c b/commit.c
index d76d64d4dfc..755b8b9d94f 100644
--- a/commit.c
+++ b/commit.c
@@
Re: [PATCH 19/20] abbrev: support relative abbrev values
Ævar Arnfjörð Bjarmason writes: > E.g. here's a breakdown of my dotfiles repo: > > $ git -c core.abbrev=4 log --pretty=format:%h|perl -nE 'chomp;say > length'|sort|uniq -c|sort -nr > 784 4 > 59 5 > 7 6 > > I don't have a single commit that needs 7 characters, yet that's our > default. This is a sane trade-off for the kernel, but for something > that's just a toy or something you're playing around with something > shorter can make sense. > > SHA-1s aren't just written down, but also e.g. remembered in wetware > short-term memory. That's a fine example of what I called "supporting absurdly small absolute values like 4"; I still do not see why you want "negative relative values" from that example.
Re: [GSoC] GSoC with git, week 6
Hello, Nice job there! I also published a blog post regarding `git stash`. https://ungps.github.io/ Best, Paul
Re: [PATCH 19/20] abbrev: support relative abbrev values
On Tue, Jun 12 2018, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason writes:
>
>> Change the core.abbrev config variable and the corresponding --abbrev
>> command-line option to support relative values such as +1 or -1.
>>
>> Before Linus's e6c587c733 ("abbrev: auto size the default
>> abbreviation", 2016-09-30) git would default to abbreviating object
>> names to 7-hexdigits, and only picking longer SHA-1s as needed if that
>> was ambiguous.
>>
>> That change instead set the default length as a function of the
>> estimated current count of objects:
>>
>> Based on the expectation that we would see collision in a
>> repository with 2^(2N) objects when using object names shortened
>> to first N bits, use sufficient number of hexdigits to cover the
>> number of objects in the repository. Each hexdigit (4-bits) we
>> add to the shortened name allows us to have four times (2-bits) as
>> many objects in the repository.
>>
>> By supporting relative values for core.abbrev we can allow users to
>> consistently opt-in for either a higher or lower probability of
>> collision, without needing to hardcode a given numeric value like
>> "10", which would be overkill on some repositories, and far to small
>> on others.
>
> Nicely explained and calculated ;-)
>
>> test_expect_success 'describe core.abbrev=[-+]1 and --abbrev=[-+]1' '
>> -test_must_fail git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe
>> &&
>> -test_must_fail git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe
>> &&
>> +git -c core.abbrev=-1 describe | sed_g_tr_d_n >describe &&
>> +test_byte_count = 6 describe &&
>> +
>> +git -c core.abbrev=+1 describe | sed_g_tr_d_n >describe &&
>> +test_byte_count = 8 describe &&
>
> Even though I see the point of supporting absurdly small absolute
> values like 4, I do not quite see the point of supporting negative
> relative values here. What's the expected use case?
I'll add a better explanation for this to the commit message.
Initially I did this for consistency, since it was easy to implement,
and there's no reason to have that arbitrary limitation, but thinking
about it again I think I'll use this for some of my projects.
E.g. here's a breakdown of my dotfiles repo:
$ git -c core.abbrev=4 log --pretty=format:%h|perl -nE 'chomp;say
length'|sort|uniq -c|sort -nr
784 4
59 5
7 6
I don't have a single commit that needs 7 characters, yet that's our
default. This is a sane trade-off for the kernel, but for something
that's just a toy or something you're playing around with something
shorter can make sense.
SHA-1s aren't just written down, but also e.g. remembered in wetware
short-term memory.
>> git log --abbrev=+1 --pretty=format:%h -1 | tr_d_n >log &&
>> -test_byte_count = 4 log &&
>> +test_byte_count = 8 log &&
>
> This, together with many many others in the rest of the patch, is
> cute but confusing in that the diff shows change from 4 to 8 due to
> the redefinition of what abbrev=+1 means. I have a feeling that it
> may not be worth doing it "right", but if we were doing it "right",
> we would probably have done it in multiple steps:
>
> - the earlier patches in this series that demonstrates
> --abbrev=+1 is --abbrev=1 and core.abbrev=+1 is an error.
>
> - ensure --abbrev=+1 is rejected as syntax error just like
> core.abbrev=+1 was, without introducing relative values
>
> - introduce relative value.
>
> That way, the last step (which corresponds to this patch) would show
> change from "log --abbrev=+1" failing due to syntax error to showing
> abbreviated value that is slightly longer than the default.
>
> But a I said, it may not be worth doing so. "Is it worth supporting
> negative relative length?" still stands, though.
I'll see what I can do about this value churn.
fatal: could not reset submodule index
Hi, I would like to add submodules to existing projects. Some branches would have the submodules and some branches would not. Since we often switch from one branch to another, I would like the submodules to update automatically so I activate the option submodule.recurse. But I am experiencing a problem if I do the following: 1. Clone repo, with master containing no submodules 2. Checkout a branch that contains submodules. It results in this error: fatal: not a git repository: ../.git/modules/submodule fatal: could not reset submodule index I think git is trying to update the submodule but the submodule has not yet been initialized (runs "git submodule update" without "--init"). Is there a way to ask git to initialize the submodule automatically ? Here is the full reproduction instructions: # Create a repository mkdir main cd main git init touch main.txt git add main.txt git commit -a -m "Initial commit" cd .. # Create a second repository mkdir sub cd sub git init touch sub.txt git add sub.txt git commit -a -m "Initial commit of repo sub" cd .. # Add the second repository as submodule, on a separate branch cd main git branch with-submodule git checkout with-submodule git submodule add ../sub sub git commit -a -m "Add submodule" # Set main repo back to master branch (without the submodule) git checkout master cd .. # Make a clone and checkout the branch git clone main clone1 cd clone1 git checkout with-submodule # Submodule is not automatically updated (sub folder is empty) git submodule update --init --recursive # Now the submodule content is there # But I want to automatically update submodules when checking out a branch cd .. # Trying again, adding --recursive during clone git clone --recursive main clone2 cd clone2 git checkout with-submodule # Submodule is still not automatically updated (sub folder is empty) git submodule update --init --recursive cd .. # Trying again, adding --recurse-submodules during checkout git clone --recursive main clone3 cd clone3 git checkout --recurse-submodules with-submodule # Fails with these error messages : # fatal: not a git repository: ../.git/modules/sub # fatal: could not reset submodule index # It seems like Git tries to update the submodule but without having initialized the submodule cd .. # Trying again with submodule.recurse git config --global submodule.recurse true git clone main clone4 cd clone4 git checkout with-submodule # Submodule is still not automatically updated (sub folder is empty) # It seems like submodule.recurse does not affect git clone # Trying again with both submodule.recurse and --recursive git config --global submodule.recurse true git clone --recursive main clone5 cd clone5 git checkout with-submodule # Fails with these error messages : # fatal: not a git repository: ../.git/modules/sub # fatal: could not reset submodule index # Same issue as with "git checkout --recurse-submodules" # I tested this in git-bash on Windows 10 $ git --version git version 2.17.1.windows.2 # And in Ubuntu in WSL $ git --version git version 2.17.1
Re: [PATCH v2] packfile: Correct zlib buffer handling
Jeremy Linton writes: >> Here is what I tentatively came up with. >> >> -- >8 -- >> From: Jeremy Linton >> Date: Wed, 13 Jun 2018 09:22:07 -0500 >> Subject: [PATCH] packfile: correct zlib buffer handling >> >> The buffer being passed to zlib includes a NUL terminator that git >> ... >> + >> return buffer; >> } >> >> -- >> 2.18.0-rc1-1-g6f333ff2fb > > This is all fine with me, the original comment was an attempt to > indicate that the original null may not have been there anymore too.. > > Shall I resubmit it as above, or can it be picked up like this? If you are happy with what you saw above, I can just make it no longer "tentative" and use it as-is, which would save time for both of us ;-) Thanks.
Re: [PATCH v2] packfile: Correct zlib buffer handling
Hi, On Wed, Jun 13, 2018 at 1:38 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >>> + buffer[size] = 0; /* assure that the buffer is still terminated */ >> >> I think we normally use '\0' for NUL on this project rather than simply 0. >> >> The comment is also effectively pure noise since it merely repeats >> what the code already states clearly (especially when the code says >> "buffer[size] = '\0';"), so dropping the comment altogether would be >> reasonable. > > Actually, I'd prefer to have comment there, but not about "what this > line does" (which is useless, as you pointed out) but about "why do > we do this seemingly redundant clearing". > > Here is what I tentatively came up with. > > -- >8 -- > From: Jeremy Linton > Date: Wed, 13 Jun 2018 09:22:07 -0500 > Subject: [PATCH] packfile: correct zlib buffer handling > > The buffer being passed to zlib includes a NUL terminator that git > needs to keep in place. unpack_compressed_entry() attempts to detect > the case that the source buffer hasn't been fully consumed by > checking to see if the destination buffer has been over consumed. > > This causes a problem, that more recent zlib patches have been > poisoning the unconsumed portions of the buffer which overwrites > the NUL byte, while correctly returning length and status. > > Let's place the NUL at the end of the buffer after inflate returns > to assure that it doesn't result in problems for git even if its > been overwritten by zlib. > > Signed-off-by: Jeremy Linton > Signed-off-by: Junio C Hamano > --- > packfile.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/packfile.c b/packfile.c > index 4a5fe7ab18..d555699217 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -1422,6 +1422,9 @@ static void *unpack_compressed_entry(struct packed_git > *p, > return NULL; > } > > + /* versions of zlib can clobber unconsumed portion of outbuf */ > + buffer[size] = '\0'; > + > return buffer; > } > > -- > 2.18.0-rc1-1-g6f333ff2fb This is all fine with me, the original comment was an attempt to indicate that the original null may not have been there anymore too.. Shall I resubmit it as above, or can it be picked up like this?
[PATCH v2 6/8] fetch: refactor to make function args narrower
Refactor find_non_local_tags and get_ref_map to only take the
information they need instead of the entire transport struct. Besides
improving code clarity, this also improves their flexibility, allowing
for a different set of refs to be used instead of relying on the ones
stored in the transport struct.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 52 -
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ee8b87c78..b600e1f10 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -254,9 +254,9 @@ static int will_fetch(struct ref **head, const unsigned
char *sha1)
return 0;
}
-static void find_non_local_tags(struct transport *transport,
- struct ref **head,
- struct ref ***tail)
+static void find_non_local_tags(const struct ref *refs,
+ struct ref **head,
+ struct ref ***tail)
{
struct string_list existing_refs = STRING_LIST_INIT_DUP;
struct string_list remote_refs = STRING_LIST_INIT_NODUP;
@@ -264,7 +264,7 @@ static void find_non_local_tags(struct transport *transport,
struct string_list_item *item = NULL;
for_each_ref(add_existing, _refs);
- for (ref = transport_get_remote_refs(transport, NULL); ref; ref =
ref->next) {
+ for (ref = refs; ref; ref = ref->next) {
if (!starts_with(ref->name, "refs/tags/"))
continue;
@@ -338,7 +338,8 @@ static void find_non_local_tags(struct transport *transport,
string_list_clear(_refs, 0);
}
-static struct ref *get_ref_map(struct transport *transport,
+static struct ref *get_ref_map(struct remote *remote,
+ const struct ref *remote_refs,
struct refspec *rs,
int tags, int *autotags)
{
@@ -346,27 +347,11 @@ static struct ref *get_ref_map(struct transport
*transport,
struct ref *rm;
struct ref *ref_map = NULL;
struct ref **tail = _map;
- struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
/* opportunistically-updated references: */
struct ref *orefs = NULL, **oref_tail =
struct string_list existing_refs = STRING_LIST_INIT_DUP;
- const struct ref *remote_refs;
-
- if (rs->nr)
- refspec_ref_prefixes(rs, _prefixes);
- else if (transport->remote && transport->remote->fetch.nr)
- refspec_ref_prefixes(>remote->fetch, _prefixes);
-
- if (ref_prefixes.argc &&
- (tags == TAGS_SET || (tags == TAGS_DEFAULT && !rs->nr))) {
- argv_array_push(_prefixes, "refs/tags/");
- }
-
- remote_refs = transport_get_remote_refs(transport, _prefixes);
-
- argv_array_clear(_prefixes);
if (rs->nr) {
struct refspec *fetch_refspec;
@@ -403,7 +388,7 @@ static struct ref *get_ref_map(struct transport *transport,
if (refmap.nr)
fetch_refspec =
else
- fetch_refspec = >remote->fetch;
+ fetch_refspec = >fetch;
for (i = 0; i < fetch_refspec->nr; i++)
get_fetch_map(ref_map, _refspec->items[i],
_tail, 1);
@@ -411,7 +396,6 @@ static struct ref *get_ref_map(struct transport *transport,
die("--refmap option is only meaningful with command-line
refspec(s).");
} else {
/* Use the defaults */
- struct remote *remote = transport->remote;
struct branch *branch = branch_get(NULL);
int has_merge = branch_has_merge_config(branch);
if (remote &&
@@ -450,7 +434,7 @@ static struct ref *get_ref_map(struct transport *transport,
/* also fetch all tags */
get_fetch_map(remote_refs, tag_refspec, , 0);
else if (tags == TAGS_DEFAULT && *autotags)
- find_non_local_tags(transport, _map, );
+ find_non_local_tags(remote_refs, _map, );
/* Now append any refs to be updated opportunistically: */
*tail = orefs;
@@ -1137,6 +1121,8 @@ static int do_fetch(struct transport *transport,
struct ref *ref_map;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
+ const struct ref *remote_refs;
+ struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1152,7 +1138,21 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
- ref_map = get_ref_map(transport, rs, tags, );
+ if (rs->nr)
+ refspec_ref_prefixes(rs, _prefixes);
+ else if (transport->remote && transport->remote->fetch.nr)
+
[PATCH v2 2/8] upload-pack: implement ref-in-want
Currently, while performing packfile negotiation, clients are only
allowed to specify their desired objects using object ids. This causes
a vulnerability to failure when an object turns non-existent during
negotiation, which may happen if, for example, the desired repository is
provided by multiple Git servers in a load-balancing arrangement.
In order to eliminate this vulnerability, implement the ref-in-want
feature for the 'fetch' command in protocol version 2. This feature
enables the 'fetch' command to support requests in the form of ref names
through a new "want-ref " parameter. At the conclusion of
negotiation, the server will send a list of all of the wanted references
(as provided by "want-ref" lines) in addition to the generated packfile.
Signed-off-by: Brandon Williams
---
Documentation/config.txt| 7 ++
Documentation/technical/protocol-v2.txt | 29 -
t/t5703-upload-pack-ref-in-want.sh | 153
upload-pack.c | 64 ++
4 files changed, 252 insertions(+), 1 deletion(-)
create mode 100755 t/t5703-upload-pack-ref-in-want.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf5a..fb1dd7428 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3479,6 +3479,13 @@ Note that this configuration variable is ignored if it
is seen in the
repository-level config (this is a safety measure against fetching from
untrusted repositories).
+uploadpack.allowRefInWant::
+ If this option is set, `upload-pack` will support the `ref-in-want`
+ feature of the protocol version 2 `fetch` command. This feature
+ is intended for the benefit of load-balanced servers which may
+ not have the same view of what OIDs their refs point to due to
+ replication delay.
+
url..insteadOf::
Any URL that starts with this value will be rewritten to
start, instead, with . In cases where some site serves a
diff --git a/Documentation/technical/protocol-v2.txt
b/Documentation/technical/protocol-v2.txt
index 49bda76d2..6020632b4 100644
--- a/Documentation/technical/protocol-v2.txt
+++ b/Documentation/technical/protocol-v2.txt
@@ -299,12 +299,22 @@ included in the client's request:
for use with partial clone and partial fetch operations. See
`rev-list` for possible "filter-spec" values.
+If the 'ref-in-want' feature is advertised, the following argument can
+be included in the client's request as well as the potential addition of
+the 'wanted-refs' section in the server's response as explained below.
+
+want-ref
+ Indicates to the server that the client wants to retrieve a
+ particular ref, where is the full name of a ref on the
+ server. A server should ignore any "want-ref " lines where
+doesn't exist on the server.
+
The response of `fetch` is broken into a number of sections separated by
delimiter packets (0001), with each section beginning with its section
header.
output = *section
-section = (acknowledgments | shallow-info | packfile)
+section = (acknowledgments | shallow-info | wanted-refs | packfile)
(flush-pkt | delim-pkt)
acknowledgments = PKT-LINE("acknowledgments" LF)
@@ -319,6 +329,10 @@ header.
shallow = "shallow" SP obj-id
unshallow = "unshallow" SP obj-id
+wanted-refs = PKT-LINE("wanted-refs" LF)
+ *PKT-LINE(wanted-ref LF)
+wanted-ref = obj-id SP refname
+
packfile = PKT-LINE("packfile" LF)
*PKT-LINE(%x01-03 *%x00-ff)
@@ -379,6 +393,19 @@ header.
* This section is only included if a packfile section is also
included in the response.
+wanted-refs section
+ * This section is only included if the client has requested a
+ ref using a 'want-ref' line and if a packfile section is also
+ included in the response.
+
+ * Always begins with the section header "wanted-refs"
+
+ * The server will send a ref listing (" ") for
+ each reference requested using 'want-ref' lines.
+
+ * The server MUST NOT send any refs which were not requested
+ using 'want-ref' lines.
+
packfile section
* This section is only included if the client has sent 'want'
lines in its request and either requested that no more
diff --git a/t/t5703-upload-pack-ref-in-want.sh
b/t/t5703-upload-pack-ref-in-want.sh
new file mode 100755
index 0..0ef182970
--- /dev/null
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -0,0 +1,153 @@
+#!/bin/sh
+
+test_description='upload-pack ref-in-want'
+
+. ./test-lib.sh
+
+get_actual_refs() {
+ sed -n '/wanted-refs/,/0001/p' actual_refs
+}
+
+get_actual_commits() {
+ sed -n '/packfile/,//p' o.pack &&
+ git index-pack o.pack &&
+ git verify-pack -v o.idx | grep commit | cut -c-40 | sort
>actual_commits
+}
+
+check_output() {
+ get_actual_refs &&
+ test_cmp
[PATCH v2 7/8] fetch-pack: put shallow info in output parameter
Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched. Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.
This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.
Signed-off-by: Brandon Williams
---
builtin/clone.c | 4 ++--
builtin/fetch.c | 23 +++
fetch-object.c | 2 +-
fetch-pack.c | 17 +
transport-helper.c | 6 --
transport-internal.h | 9 -
transport.c | 34 --
transport.h | 3 ++-
8 files changed, 73 insertions(+), 25 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
}
if (!is_local && !complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs);
+ transport_fetch_refs(transport, mapped_refs, NULL);
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs);
+ transport_fetch_refs(transport, mapped_refs, NULL);
update_remote_refs(refs, mapped_refs, remote_head_points_at,
branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b600e1f10..ddf44ba1a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
}
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
{
int ret = quickfetch(ref_map);
if (ret)
- ret = transport_fetch_refs(transport, ref_map);
+ ret = transport_fetch_refs(transport, ref_map,
+ updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1106,7 +1108,7 @@ static void backfill_tags(struct transport *transport,
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- if (!fetch_refs(transport, ref_map))
+ if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
if (gsecondary) {
@@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+ struct ref *new_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
if (tags == TAGS_DEFAULT) {
@@ -1172,7 +1175,19 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map))
{
+
+ if (fetch_refs(transport, ref_map, _remote_refs)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
+ if (new_remote_refs) {
+ free_refs(ref_map);
+ ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
+ tags, );
+ free_refs(new_remote_refs);
+ }
+ if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref
*ref)
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
- transport_fetch_refs(transport, ref);
+ transport_fetch_refs(transport, ref, NULL);
fetch_if_missing = original_fetch_if_missing;
}
diff --git a/fetch-pack.c b/fetch-pack.c
index a320ce987..7799ee2cd 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1470,12 +1470,13 @@ static int
[PATCH v2 5/8] fetch: refactor fetch_refs into two functions
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 545635448..ee8b87c78 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -967,10 +967,16 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
int ret = quickfetch(ref_map);
if (ret)
ret = transport_fetch_refs(transport, ref_map);
- if (!ret)
- ret |= store_updated_refs(transport->url,
- transport->remote->name,
- ref_map);
+ if (ret)
+ transport_unlock_pack(transport);
+ return ret;
+}
+
+static int consume_refs(struct transport *transport, struct ref *ref_map)
+{
+ int ret = store_updated_refs(transport->url,
+transport->remote->name,
+ref_map);
transport_unlock_pack(transport);
return ret;
}
@@ -1116,7 +1122,8 @@ static void backfill_tags(struct transport *transport,
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- fetch_refs(transport, ref_map);
+ if (!fetch_refs(transport, ref_map))
+ consume_refs(transport, ref_map);
if (gsecondary) {
transport_disconnect(gsecondary);
@@ -1165,7 +1172,7 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map)) {
+ if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map))
{
free_refs(ref_map);
retcode = 1;
goto cleanup;
--
2.18.0.rc1.242.g61856ae69a-goog
[PATCH v2 3/8] upload-pack: test negotiation with changing repository
Add tests to check the behavior of fetching from a repository which
changes between rounds of negotiation (for example, when different
servers in a load-balancing agreement participate in the same stateless
RPC negotiation). This forms a baseline of comparison to the ref-in-want
functionality (which will be introduced to the client in subsequent
commits), and ensures that subsequent commits do not change existing
behavior.
As part of this effort, a mechanism to substitute strings in a single
HTTP response is added.
Signed-off-by: Brandon Williams
---
t/lib-httpd.sh | 1 +
t/lib-httpd/apache.conf| 8 +++
t/lib-httpd/one-time-sed.sh| 16 ++
t/t5703-upload-pack-ref-in-want.sh | 92 ++
4 files changed, 117 insertions(+)
create mode 100644 t/lib-httpd/one-time-sed.sh
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 435a37465..84f8efdd4 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -132,6 +132,7 @@ prepare_httpd() {
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
install_script broken-smart-http.sh
install_script error.sh
+ install_script one-time-sed.sh
ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules"
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 724d9ae46..fe68d37bb 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -111,9 +111,14 @@ Alias /auth/dumb/ www/auth/dumb/
SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
SetEnv GIT_HTTP_EXPORT_ALL
+
+ SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
+ SetEnv GIT_HTTP_EXPORT_ALL
+
ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
ScriptAlias /broken_smart/ broken-smart-http.sh/
ScriptAlias /error/ error.sh/
+ScriptAliasMatch /one_time_sed/(.*) one-time-sed.sh/$1
Options FollowSymlinks
@@ -123,6 +128,9 @@ ScriptAlias /error/ error.sh/
Options ExecCGI
+
+ Options ExecCGI
+
Options ExecCGI
diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
new file mode 100644
index 0..a9c4aa5f4
--- /dev/null
+++ b/t/lib-httpd/one-time-sed.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+if [ -e one-time-sed ]; then
+ "$GIT_EXEC_PATH/git-http-backend" >out
+
+ sed "$(cat one-time-sed)" out_modified
+
+ if diff out out_modified >/dev/null; then
+ cat out
+ else
+ cat out_modified
+ rm one-time-sed
+ fi
+else
+ "$GIT_EXEC_PATH/git-http-backend"
+fi
diff --git a/t/t5703-upload-pack-ref-in-want.sh
b/t/t5703-upload-pack-ref-in-want.sh
index 0ef182970..979ab6d03 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -150,4 +150,96 @@ test_expect_success 'want-ref with ref we already have
commit for' '
check_output
'
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo"
+LOCAL_PRISTINE="$(pwd)/local_pristine"
+
+test_expect_success 'setup repos for change-while-negotiating test' '
+ (
+ git init "$REPO" &&
+ cd "$REPO" &&
+ >.git/git-daemon-export-ok &&
+ test_commit m1 &&
+ git tag -d m1 &&
+
+ # Local repo with many commits (so that negotiation will take
+ # more than 1 request/response pair)
+ git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo;
"$LOCAL_PRISTINE" &&
+ cd "$LOCAL_PRISTINE" &&
+ git checkout -b side &&
+ for i in $(seq 1 33); do test_commit s$i; done &&
+
+ # Add novel commits to upstream
+ git checkout master &&
+ cd "$REPO" &&
+ test_commit m2 &&
+ test_commit m3 &&
+ git tag -d m2 m3
+ ) &&
+ git -C "$LOCAL_PRISTINE" remote set-url origin
"http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo; &&
+ git -C "$LOCAL_PRISTINE" config protocol.version 2
+'
+
+inconsistency() {
+ # Simulate that the server initially reports $2 as the ref
+ # corresponding to $1, and after that, $1 as the ref corresponding to
+ # $1. This corresponds to the real-life situation where the server's
+ # repository appears to change during negotiation, for example, when
+ # different servers in a load-balancing arrangement serve (stateless)
+ # RPCs during a single negotiation.
+ printf "s/%s/%s/" \
+ $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
+ $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
+ >"$HTTPD_ROOT_PATH/one-time-sed"
+}
+
+test_expect_success 'server is initially ahead - no ref in want' '
+ git -C "$REPO" config uploadpack.allowRefInWant false &&
+ rm -rf local &&
+ cp -r "$LOCAL_PRISTINE" local &&
+ inconsistency master 1234567890123456789012345678901234567890 &&
+ test_must_fail git -C local fetch 2>err &&
+
[PATCH v2 8/8] fetch-pack: implement ref-in-want
Implement ref-in-want on the client side so that when a server supports
the "ref-in-want" feature, a client will send "want-ref" lines for each
reference the client wants to fetch.
Signed-off-by: Brandon Williams
---
fetch-pack.c | 35 +++---
remote.c | 1 +
remote.h | 1 +
t/t5703-upload-pack-ref-in-want.sh | 4 ++--
4 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 7799ee2cd..51e8356ba 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1102,9 +1102,10 @@ static void add_shallow_requests(struct strbuf *req_buf,
static void add_wants(const struct ref *wants, struct strbuf *req_buf)
{
+ int use_ref_in_want = server_supports_feature("fetch", "ref-in-want",
0);
+
for ( ; wants ; wants = wants->next) {
const struct object_id *remote = >old_oid;
- const char *remote_hex;
struct object *o;
/*
@@ -1122,8 +1123,10 @@ static void add_wants(const struct ref *wants, struct
strbuf *req_buf)
continue;
}
- remote_hex = oid_to_hex(remote);
- packet_buf_write(req_buf, "want %s\n", remote_hex);
+ if (!use_ref_in_want || wants->exact_sha1)
+ packet_buf_write(req_buf, "want %s\n",
oid_to_hex(remote));
+ else
+ packet_buf_write(req_buf, "want-ref %s\n", wants->name);
}
}
@@ -1334,6 +1337,29 @@ static void receive_shallow_info(struct fetch_pack_args
*args,
args->deepen = 1;
}
+static void receive_wanted_refs(struct packet_reader *reader, struct ref *refs)
+{
+ process_section_header(reader, "wanted-refs", 0);
+ while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
+ struct object_id oid;
+ const char *end;
+ struct ref *r = NULL;
+
+ if (parse_oid_hex(reader->line, , ) || *end++ != ' ')
+ die("expected wanted-ref, got '%s'", reader->line);
+
+ for (r = refs; r; r = r->next) {
+ if (!strcmp(end, r->name)) {
+ oidcpy(>old_oid, );
+ break;
+ }
+ }
+ }
+
+ if (reader->status != PACKET_READ_DELIM)
+ die("error processing wanted refs: %d", reader->status);
+}
+
enum fetch_state {
FETCH_CHECK_LOCAL = 0,
FETCH_SEND_REQUEST,
@@ -1408,6 +1434,9 @@ static struct ref *do_fetch_pack_v2(struct
fetch_pack_args *args,
if (process_section_header(, "shallow-info", 1))
receive_shallow_info(args, );
+ if (process_section_header(, "wanted-refs", 1))
+ receive_wanted_refs(, ref);
+
/* get the pack */
process_section_header(, "packfile", 0);
if (get_pack(args, fd, pack_lockfile))
diff --git a/remote.c b/remote.c
index abe80c139..c9d452ac0 100644
--- a/remote.c
+++ b/remote.c
@@ -1735,6 +1735,7 @@ int get_fetch_map(const struct ref *remote_refs,
if (refspec->exact_sha1) {
ref_map = alloc_ref(name);
get_oid_hex(name, _map->old_oid);
+ ref_map->exact_sha1 = 1;
} else {
ref_map = get_remote_ref(remote_refs, name);
}
diff --git a/remote.h b/remote.h
index 45ecc6cef..e5338e368 100644
--- a/remote.h
+++ b/remote.h
@@ -73,6 +73,7 @@ struct ref {
force:1,
forced_update:1,
expect_old_sha1:1,
+ exact_sha1:1,
deletion:1;
enum {
diff --git a/t/t5703-upload-pack-ref-in-want.sh
b/t/t5703-upload-pack-ref-in-want.sh
index 979ab6d03..b94a51380 100755
--- a/t/t5703-upload-pack-ref-in-want.sh
+++ b/t/t5703-upload-pack-ref-in-want.sh
@@ -204,7 +204,7 @@ test_expect_success 'server is initially ahead - no ref in
want' '
grep "ERR upload-pack: not our ref" err
'
-test_expect_failure 'server is initially ahead - ref in want' '
+test_expect_success 'server is initially ahead - ref in want' '
git -C "$REPO" config uploadpack.allowRefInWant true &&
rm -rf local &&
cp -r "$LOCAL_PRISTINE" local &&
@@ -228,7 +228,7 @@ test_expect_success 'server is initially behind - no ref in
want' '
test_cmp expected actual
'
-test_expect_failure 'server is initially behind - ref in want' '
+test_expect_success 'server is initially behind - ref in want' '
git -C "$REPO" config uploadpack.allowRefInWant true &&
rm -rf local &&
cp -r "$LOCAL_PRISTINE" local &&
--
2.18.0.rc1.242.g61856ae69a-goog
[PATCH v2 1/8] test-pkt-line: add unpack-sideband subcommand
Add an 'unpack-sideband' subcommand to the test-pkt-line helper to
enable unpacking packet line data sent multiplexed using a sideband.
Signed-off-by: Brandon Williams
---
t/helper/test-pkt-line.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index 0f19e53c7..2a551 100644
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -1,3 +1,4 @@
+#include "cache.h"
#include "pkt-line.h"
static void pack_line(const char *line)
@@ -48,6 +49,40 @@ static void unpack(void)
}
}
+static void unpack_sideband(void)
+{
+ struct packet_reader reader;
+ packet_reader_init(, 0, NULL, 0,
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_CHOMP_NEWLINE);
+
+ while (packet_reader_read() != PACKET_READ_EOF) {
+ int band;
+ int fd;
+
+ switch (reader.status) {
+ case PACKET_READ_EOF:
+ break;
+ case PACKET_READ_NORMAL:
+ band = reader.line[0] & 0xff;
+ if (band == 1)
+ fd = 1;
+ else
+ fd = 2;
+
+ write_or_die(fd, reader.line+1, reader.pktlen-1);
+
+ if (band == 3)
+ die("sind-band error");
+ break;
+ case PACKET_READ_FLUSH:
+ return;
+ case PACKET_READ_DELIM:
+ break;
+ }
+ }
+}
+
int cmd_main(int argc, const char **argv)
{
if (argc < 2)
@@ -57,6 +92,8 @@ int cmd_main(int argc, const char **argv)
pack(argc - 2, argv + 2);
else if (!strcmp(argv[1], "unpack"))
unpack();
+ else if (!strcmp(argv[1], "unpack-sideband"))
+ unpack_sideband();
else
die("invalid argument '%s'", argv[1]);
--
2.18.0.rc1.242.g61856ae69a-goog
[PATCH v2 4/8] fetch: refactor the population of peer ref OIDs
Populate peer ref OIDs in get_ref_map instead of do_fetch. Besides
tightening scopes of variables in the code, this also prepares for
get_ref_map being able to be called multiple times within do_fetch.
Signed-off-by: Brandon Williams
---
builtin/fetch.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ea5b9669a..545635448 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -351,6 +351,7 @@ static struct ref *get_ref_map(struct transport *transport,
/* opportunistically-updated references: */
struct ref *orefs = NULL, **oref_tail =
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
const struct ref *remote_refs;
if (rs->nr)
@@ -458,7 +459,23 @@ static struct ref *get_ref_map(struct transport *transport,
tail = >next;
}
- return ref_remove_duplicates(ref_map);
+ ref_map = ref_remove_duplicates(ref_map);
+
+ for_each_ref(add_existing, _refs);
+ for (rm = ref_map; rm; rm = rm->next) {
+ if (rm->peer_ref) {
+ struct string_list_item *peer_item =
+ string_list_lookup(_refs,
+ rm->peer_ref->name);
+ if (peer_item) {
+ struct object_id *old_oid = peer_item->util;
+ oidcpy(>peer_ref->old_oid, old_oid);
+ }
+ }
+ }
+ string_list_clear(_refs, 1);
+
+ return ref_map;
}
#define STORE_REF_ERROR_OTHER 1
@@ -1110,14 +1127,10 @@ static void backfill_tags(struct transport *transport,
struct ref *ref_map)
static int do_fetch(struct transport *transport,
struct refspec *rs)
{
- struct string_list existing_refs = STRING_LIST_INIT_DUP;
struct ref *ref_map;
- struct ref *rm;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
- for_each_ref(add_existing, _refs);
-
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
tags = TAGS_SET;
@@ -1136,18 +1149,6 @@ static int do_fetch(struct transport *transport,
if (!update_head_ok)
check_not_current_branch(ref_map);
- for (rm = ref_map; rm; rm = rm->next) {
- if (rm->peer_ref) {
- struct string_list_item *peer_item =
- string_list_lookup(_refs,
- rm->peer_ref->name);
- if (peer_item) {
- struct object_id *old_oid = peer_item->util;
- oidcpy(>peer_ref->old_oid, old_oid);
- }
- }
- }
-
if (tags == TAGS_DEFAULT && autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
if (prune) {
@@ -1183,7 +1184,6 @@ static int do_fetch(struct transport *transport,
}
cleanup:
- string_list_clear(_refs, 1);
return retcode;
}
--
2.18.0.rc1.242.g61856ae69a-goog
[PATCH v2 0/8] ref-in-want
Changes in v2: * issuing a want-ref line to a ref which doesn't exist is just ignored. * fixed some typos Brandon Williams (8): test-pkt-line: add unpack-sideband subcommand upload-pack: implement ref-in-want upload-pack: test negotiation with changing repository fetch: refactor the population of peer ref OIDs fetch: refactor fetch_refs into two functions fetch: refactor to make function args narrower fetch-pack: put shallow info in output parameter fetch-pack: implement ref-in-want Documentation/config.txt| 7 + Documentation/technical/protocol-v2.txt | 29 ++- builtin/clone.c | 4 +- builtin/fetch.c | 126 +++- fetch-object.c | 2 +- fetch-pack.c| 52 +++-- remote.c| 1 + remote.h| 1 + t/helper/test-pkt-line.c| 37 t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf | 8 + t/lib-httpd/one-time-sed.sh | 16 ++ t/t5703-upload-pack-ref-in-want.sh | 245 transport-helper.c | 6 +- transport-internal.h| 9 +- transport.c | 34 +++- transport.h | 3 +- upload-pack.c | 64 +++ 18 files changed, 568 insertions(+), 77 deletions(-) create mode 100644 t/lib-httpd/one-time-sed.sh create mode 100755 t/t5703-upload-pack-ref-in-want.sh -- 2.18.0.rc1.242.g61856ae69a-goog
Re: [PATCH v2] fetch-pack: don't try to fetch peel values with --all
On Tue, Jun 12, 2018 at 06:54:17PM +, Kirill Smelkov wrote: > > If an extra connection isn't a problem, you might be better off with > > "git ls-remote", and then picking through the results for refs of > > interest, and then "git fetch-pack" to actually get the pack. That's how > > git-fetch worked when it was a shell script (e.g., see c3a200120d, the > > last shell version). > > Yes, this is what I ended up doing: > > https://lab.nexedi.com/kirr/git-backup/commit/899103bf > > but for another reason - to avoid repeating for every fetched repository > slow (in case of my "big" destination backup repository) quickfetch() > checking in every spawned `git fetch`: git-backup can build index of > objects we already have ourselves only once at startup, and then in > fetch, after checking lsremote output, consult that index, and if we see > we already have everything for an advertised reference - just avoid > giving it to fetch-pack to process. It turns out for many pulled > repositories there is usually no references changed at all and this way > fetch-pack can be skipped completely: > > https://lab.nexedi.com/kirr/git-backup/commit/3efed898 Thanks for sharing that, it's an interesting case. I'd hope that git-fetch is smart enough not to even bother with quickfetch() if there are no refs to update. But if we have even one change to fetch, then yeah, in the general case it makes sense to me that you could do better by amortizing the scan of local objects across many operations. -Peff
Re: [PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
On Wed, Jun 13, 2018 at 06:43:04PM +, Kirill Smelkov wrote:
> From: Kirill Smelkov
> Date: Wed, 13 Jun 2018 12:28:21 +0300
> Subject: [PATCH v2] fetch-pack: test explicitly that --all can fetch tag
> references pointing to non-commits
>
> Fetch-pack --all became broken with respect to unusual tags in
> 5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
> and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
> peel values with --all, 2018-06-11). However the test added in
> e9502c0a7f does not explicitly cover all funky cases.
>
> In order to be sure fetching funky tags will never break, let's
> explicitly test all relevant cases with 4 tag objects pointing to 1) a
> blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> tag objects themselves are referenced from under regular refs/tags/*
> namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
> 44085874...HEAD
> ...
> bc4e9e1f...refs/tags/tag-to-blob
> 038f48ad...refs/tags/tag-to-blob^{} # peeled
> 520db1f5...refs/tags/tag-to-tree
> 7395c100...refs/tags/tag-to-tree^{} # peeled
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
> --all ..
> fatal: A git upload-pack: not our ref 038f48ad...
> fatal: The remote end hung up unexpectedly
TBH, I do not find this snippet all that compelling. We know that
e9502c0a7f already fixed the bug, and that it had nothing to do with
non-commits at all.
The primary reason to add these tests is that in general we do not cover
fetch-pack over tags to non-commits. And I think the reason to use
otherwise unreferenced objects is that it they are more likely to have
detectable symptoms if they tickle a bug.
So why don't we say that, instead of re-hashing output from the earlier
fix?
-Peff
Re: [PATCH 28/35] commit.c: migrate the commit buffer to the parsed object store
On Wed, Jun 6, 2018 at 12:32 PM Duy Nguyen wrote: > > define_commit_slab(buffer_slab, struct commit_buffer); > > struct buffer_slab is defined locally here... > ... > > +struct buffer_slab *allocate_commit_buffer_slab(void); > > So you would need a forward declaration of struct buffer_slab in > commit.h before it's referenced here? Will do so in a resend; as well as in object.h
Re: [PATCH 03/35] object: add repository argument to lookup_unknown_object
On Wed, Jun 6, 2018 at 12:38 PM Duy Nguyen wrote: > > On Wed, May 30, 2018 at 2:47 AM, Stefan Beller wrote: > > diff --git a/object.c b/object.c > > index 4de4fa58d59..def3c71cac2 100644 > > --- a/object.c > > +++ b/object.c > > @@ -177,7 +177,7 @@ void *object_as_type(struct object *obj, enum > > object_type type, int quiet) > > } > > } > > > > -struct object *lookup_unknown_object(const unsigned char *sha1) > > +struct object *lookup_unknown_object_the_repository(const unsigned char > > *sha1) > > I'm looking at your branch and this function (with the _the_repository > suffix) is still there. Did you forget to send a patch to convert this > function? This and parse_commit and parse_commit_gently have not been converted. I stopped with this series as soon as I hit the commit-graph code, which needs to be updated, too. I'll start redoing this series soon and will fix those conversions.
Re: [PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
On Wed, Jun 13, 2018 at 10:42:33AM -0700, Junio C Hamano wrote:
> Kirill Smelkov writes:
>
> > Fetch-pack --all became broken with respect to unusual tags in
> > 5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
> > and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
> > peel values with --all, 2018-06-11). However the test added in
> > e9502c0a7f does not explicitly cover all funky cases.
>
> Thanks. Very much appreciated
Thanks.
> > In order to be sure fetching funky tags will never break, let's
> > explicitly test all relevant cases with 4 tag objects pointing to 1) a
> > blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> > tag objects themselves are referenced from under regular refs/tags/*
> > namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
> >
> > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote
> > ..
> > 440858748ae905d48259d4fb67a12a7aa1520cf7HEAD
> > ...
> > bc4e9e1fa80662b449805b1ac29fc9b1e4c49187
> > refs/tags/tag-to-blob # <-- NOTE
> > 038f48ad0beaffbea71d186a05084b79e3870cbf
> > refs/tags/tag-to-blob^{}
> > 520db1f5e1afeaa12b1a8d73ce82db72ca036ee1
> > refs/tags/tag-to-tree # <-- NOTE
> > 7395c100223b7cd760f58ccfa0d3f3d2dd539bb6
> > refs/tags/tag-to-tree^{}
> > .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
> > --all ..
> > fatal: A git upload-pack: not our ref
> > 038f48ad0beaffbea71d186a05084b79e3870cbf
> > fatal: The remote end hung up unexpectedly
>
> ... except for this bit. I am not sure readers understand what
> these two overlong lines want to say. Would it be easier to
> understand if you wrote the above like this?
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
> 44085874...HEAD
> ...
> bc4e9e1f...refs/tags/tag-to-blob
> 038f48ad...refs/tags/tag-to-blob^{} # peeled
> 520db1f5...refs/tags/tag-to-tree
> 7395c100...refs/tags/tag-to-tree^{} # peeled
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
> --all ..
> fatal: A git upload-pack: not our ref 038f48ad...
> fatal: The remote end hung up unexpectedly
>
> Instead of marking the tag-to-blob and tag-to-tree entries (which
> are not where the 'fatal' breakage comes from), I think it makes
> more sense to mark the entries that show peeled version (which also
> matches the object name in the error message), and by shortening the
> line, readers can see more easily which lines are highlighted.
Makes sense. I've amended the patch description with your suggestion.
> > +test_expect_success 'test --all wrt tag to non-commits' '
> > + blob=$(echo "hello blob" | git hash-object -t blob -w --stdin) &&
> > + git tag -a -m "tag -> blob" tag-to-blob $blob &&
> > + \
> > + tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> > + git tag -a -m "tag -> tree" tag-to-tree $tree &&
> > + \
> > + tree2=$(printf "100644 blob $blob\tfile2" | git mktree) &&
> > + commit=$(git commit-tree -m "hello commit" $tree) &&
> > + git tag -a -m "tag -> commit" tag-to-commit $commit &&
> > + \
> > + blob2=$(echo "hello blob2" | git hash-object -t blob -w --stdin) &&
> > + tag=$(printf "object $blob2\ntype blob\ntag tag-to-blob2\n\
> > +tagger author A U Thor 0 +\n\nhello tag" | git
> > mktag) &&
> > + git tag -a -m "tag -> tag" tag-to-tag $tag &&
>
> All of the above, while may not be techincallly wrong per-se, look
> unnecessarily complex.
>
> I guess the reason why you do the above is because you do not want
> to use any object that is reachable via other existing refs and
> instead want to ensure these objects are reachable only via the tags
> you are creating in this test. Otherwise using HEAD~4:test.txt and
> HEAD^{tree} instead of $blob and $tree constructed via hash-object
> and mktree would be sufficient and more readable. Oh well.
Yes, it is exactly the reason here. I've added corresponding comment
explaining this to the test case.
Kirill
8<
From: Kirill Smelkov
Date: Wed, 13 Jun 2018 12:28:21 +0300
Subject: [PATCH v2] fetch-pack: test explicitly that --all can fetch tag
references pointing to non-commits
Fetch-pack --all became broken with respect to unusual tags in
5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
peel values with --all, 2018-06-11). However the test added in
e9502c0a7f does not explicitly cover all funky cases.
In order to be sure fetching funky tags will never break, let's
explicitly test all relevant cases with 4 tag objects pointing to 1) a
blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
tag
Re: [RFC PATCH 0/3] submodules with no working tree shall unset core.worktree
On Wed, Jun 13, 2018 at 11:00 AM Junio C Hamano wrote:
>
> Stefan Beller writes:
>
> > The first patch teaches checkout/reset (with --recurse-submodules) to unset
> > the core.worktree config when the new state of the superprojects working
> > tree
> > doesn't contain the submodules working tree.
>
> Are there two cases of "doesn't contain working tree of a submodule"?
>
> The superproject's commit may not have a gitlink to a specific
> submodule in its tree (i.e. there is no way to even "submodule init"
> with such a commit in the superproject's history).
... for example git.git before 2.13 did not have a gitlink for its
sha1collision submodule ...
> Or there may be
> a gitlink but the user chose not to check it out in the working
> tree.
This is when the submodule is not active or de-initialized or not initialized
to begin with. However there is an empty directory as a place holder.
As long as the empty place holder is there, we do not run into trouble.
> Do they need to be treated differently, or can they be treated the
> same way?
I would think they are the same, as both cases do not have a working tree
for the submodule.
> Also, is the "submodule" feature supposed to play well with multiple
> worktree feature?
It is a long term goal to get this working.
> Let's imagine that you have two worktrees for a
> single superproject, and the branches of the superproject these two
> worktrees check out are different ones (which is the more sensible
> set-up than checking out the same branch twice).
(git-worktree even prevents you to checkout the same branch),
sounds all very sensible up to here.
> Further imagine
> that the superproject started using a single submodule sometime in
> the past and keeps using it throughout its life since then.
>
> 1. if both of these two branches have the submodule, and two
> worktrees both are interested in having the submodule checked
> out via "submodule init/update", where does core.worktree point
> at? Do we have two copies of the variable?
Currently in the real world (aka in origin/master), the submodules are
completely duplicated, and do not know about the worktree feature
themselves:
$ cd .git && find . |grep config |grep sha1co
./modules/sha1collisiondetection/config
./worktrees/git/modules/sha1collisiondetection/config
(Yes I do have a worktree called "git".)
So for the sake of this patch series this should be okay, as there
is only ever one working tree for a submodule currently.
The current state of affairs is really bad as the submodule is
there twice with all objects and the rest.
> 2. what if one branch predates the use of the submodule in the
> superproject while the other branch is newer and uses the
> submodule? Where does core.worktree point at?
As we have two copies of the repository, one of them points at the
correct place, and the other would be unset.
---
In an ideal world we would want the submodules to use the worktree
feature along-side the superproject, e.g. if the superproject has the
two worktrees as above, the submodule would also get these.
However I have not thought about this future too deeply, as there
are a couple bumps along the road:
Supposedly the submodules main (common) git dir would stay in the
common git dir of the superproject at /modules/.
However where would we put the submodules non-main worktree
git dirs? There are two places, as seen from the superprojects:
.git/modules//worktrees/
.git/worktrees//modules/
As worktrees can be removed pretty easily ("git worktree prune"),
I would not put the main parts of a submodule into a worktree part
of the superproject (The user could be in a non-main worktree
when they first ask for the submodule -- we have to make sure
the main git dir goes into the superprojects main git dir under
modules/).
An advantage for
.git/worktrees//modules/
would be that the worktree of the submodule is coupled to the
worktree of the superproject, i.e. if the user wipes the superprojects
worktree, the submodules accompanying worktree is gone automatically.
An advantage for
.git/modules//worktrees/
is to have any submodule related things in one place in
.git/modules/
Cleanup of the worktrees would be a bit harder as we'd have to
remember to prune the worktrees for all submodules as well
when the superproject removes one of its worktrees.
It is relatively rare to delete the submodule git directory compared to
removing a worktree, such that
.git/modules//worktrees/
sounds a bit more promising to me.
However the worktree command needs to figure out the worktrees:
/path/to/super/worktree/submodule $ git worktree list
This would look at its git dir, which is
.git/modules//worktrees/
discover the common dir at
.git/modules/
and then proceed in listing worktrees by exploring worktrees/.
If we had put it in the other form of
.git/worktrees//modules/
we'd discover the common git dir at
.git/modules/
and list it from there.
Re: [PATCH v2] packfile: Correct zlib buffer handling
On Wed, Jun 13, 2018 at 2:32 PM Junio C Hamano wrote: > Eric Sunshine writes: > > On this project, the character mnemonic "NUL" is typically used, not > > "null" or "NULL" (which is typically reserved for pointers), so: > > s/null/NUL/g > > Correct but I did not think it is a per-project preference; rather, > "NUL is the name of the byte" is universal ;-) Yes, the _mnemonic_ NUL is universal, but the character itself is sometimes named or described as the "null character". I was just being pedantic when "this project", by which I meant that we (on this project) prefer the mnemonic "NUL" over longhand "null character", whereas other projects may perhaps prefer "null character" or not care.
Re: [PATCH v2] packfile: Correct zlib buffer handling
Eric Sunshine writes: >> + buffer[size] = 0; /* assure that the buffer is still terminated */ > > I think we normally use '\0' for NUL on this project rather than simply 0. > > The comment is also effectively pure noise since it merely repeats > what the code already states clearly (especially when the code says > "buffer[size] = '\0';"), so dropping the comment altogether would be > reasonable. Actually, I'd prefer to have comment there, but not about "what this line does" (which is useless, as you pointed out) but about "why do we do this seemingly redundant clearing". Here is what I tentatively came up with. -- >8 -- From: Jeremy Linton Date: Wed, 13 Jun 2018 09:22:07 -0500 Subject: [PATCH] packfile: correct zlib buffer handling The buffer being passed to zlib includes a NUL terminator that git needs to keep in place. unpack_compressed_entry() attempts to detect the case that the source buffer hasn't been fully consumed by checking to see if the destination buffer has been over consumed. This causes a problem, that more recent zlib patches have been poisoning the unconsumed portions of the buffer which overwrites the NUL byte, while correctly returning length and status. Let's place the NUL at the end of the buffer after inflate returns to assure that it doesn't result in problems for git even if its been overwritten by zlib. Signed-off-by: Jeremy Linton Signed-off-by: Junio C Hamano --- packfile.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packfile.c b/packfile.c index 4a5fe7ab18..d555699217 100644 --- a/packfile.c +++ b/packfile.c @@ -1422,6 +1422,9 @@ static void *unpack_compressed_entry(struct packed_git *p, return NULL; } + /* versions of zlib can clobber unconsumed portion of outbuf */ + buffer[size] = '\0'; + return buffer; } -- 2.18.0-rc1-1-g6f333ff2fb
Re: [PATCH] fetch-pack: demonstrate --all failure when remote is empty
On Wed, Jun 13, 2018 at 10:13:07AM -0700, Junio C Hamano wrote: > Kirill Smelkov writes: > > > ( Junio, please pick up the patch provided in the end ) > > > > On Tue, Jun 12, 2018 at 06:54:17PM +, Kirill Smelkov wrote: > >> On Tue, Jun 12, 2018 at 05:48:49AM -0400, Jeff King wrote: > >> > On Mon, Jun 11, 2018 at 09:43:02AM +, Kirill Smelkov wrote: > > [...] > > > >> > > I'm not sure, but I would say that `fetch-pack --all` from an empty > >> > > repository should not fail and should just give empty output as fetch > >> > > does. > >> > > >> > Yeah, that seems reasonable to me. The die() that catches this dates > >> > back to 2005-era, and we later taught the "fetch" porcelain to handle > >> > this. I don't _think_ anybody would be upset that the plumbing learned > >> > to treat this as a noop. It's probably a one-liner change in > >> > fetch_pack() to return early instead of dying. > > I actually have a slight preference to the current "attempting to > fetch from a total emptiness is so rare that it is worth grabbing > attention of whoever does so" behaviour, to be honest. I see. > Oh, wait, is this specific to "fetch-pack" and the behaviour of > end-user-facing "git fetch" is kept same as before? If then, I'd be > somewhat sympathetic to the cause---it would be more convenient for > the calling Porcelain script if this turned into a silent noop (even > though it would probably make it harder to diagnose when such a > Porcelain is set up incorrectly e.g. pointing at an empty repository > that is not the one the Porcelain writer intended to fetch from). Yes, it is only for fetch-pack, and behaviour of porcelain fetch is kept as it was before. > > However with transport.c being there too, since I'm no longer using > > `fetch-pack --all`, now it is best for me to not delve into this story > > and just stop with attached patch. > > If we do not plan to change the behaviour later ourselves, I do not > think it makes sense, nor it is fair to those future developers who > inherit this project, to declare that the established behaviour is > wrong with an 'expect-failure' test like this, to be honest. I see. Let's please cancel this patch then. > > +test_expect_failure 'test --all wrt empty.git' ' > > + git init --bare empty.git && > > + ( > > + cd client && > > + git fetch-pack --all ../empty.git > > + ) > > +'
Re: [PATCH v2] packfile: Correct zlib buffer handling
Eric Sunshine writes: > A couple comments if you happen to re-roll... > > On Wed, Jun 13, 2018 at 10:22 AM Jeremy Linton > wrote: >> The buffer being passed to zlib includes a null terminator that > > On this project, the character mnemonic "NUL" is typically used, not > "null" or "NULL" (which is typically reserved for pointers), so: > s/null/NUL/g Correct but I did not think it is a per-project preference; rather, "NUL is the name of the byte" is universal ;-) >> diff --git a/packfile.c b/packfile.c >> @@ -1433,6 +1433,8 @@ static void *unpack_compressed_entry(struct packed_git >> *p, >> + buffer[size] = 0; /* assure that the buffer is still terminated */ > > I think we normally use '\0' for NUL on this project rather than simply 0. > > The comment is also effectively pure noise since it merely repeats > what the code already states clearly (especially when the code says > "buffer[size] = '\0';"), so dropping the comment altogether would be > reasonable. Both are sensible suggestions. Thanks for making them.
Re: [PATCH] doc: update the order of the syntax `git merge --continue`
Meng-Sung Wu writes:
> The syntax "git merge HEAD " has been removed. The
> order of the syntax should also be updated.
> ---
> Documentation/git-merge.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
An obviously good change. Could you please sign-off your patch?
cf. https://git.github.io/htmldocs/SubmittingPatches.html#sign-off
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index d5dfd8430..6a5c00e2c 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -57,7 +57,7 @@ reconstruct the original (pre-merge) changes. Therefore:
> discouraged: while possible, it may leave you in a state that is hard to
> back out of in the case of a conflict.
>
> -The fourth syntax ("`git merge --continue`") can only be run after the
> +The third syntax ("`git merge --continue`") can only be run after the
> merge has resulted in conflicts.
>
> OPTIONS
Re: [RFC PATCH 0/3] submodules with no working tree shall unset core.worktree
Stefan Beller writes: > The first patch teaches checkout/reset (with --recurse-submodules) to unset > the core.worktree config when the new state of the superprojects working tree > doesn't contain the submodules working tree. Are there two cases of "doesn't contain working tree of a submodule"? The superproject's commit may not have a gitlink to a specific submodule in its tree (i.e. there is no way to even "submodule init" with such a commit in the superproject's history). Or there may be a gitlink but the user chose not to check it out in the working tree. Do they need to be treated differently, or can they be treated the same way? Also, is the "submodule" feature supposed to play well with multiple worktree feature? Let's imagine that you have two worktrees for a single superproject, and the branches of the superproject these two worktrees check out are different ones (which is the more sensible set-up than checking out the same branch twice). Further imagine that the superproject started using a single submodule sometime in the past and keeps using it throughout its life since then. 1. if both of these two branches have the submodule, and two worktrees both are interested in having the submodule checked out via "submodule init/update", where does core.worktree point at? Do we have two copies of the variable? 2. what if one branch predates the use of the submodule in the superproject while the other branch is newer and uses the submodule? Where does core.worktree point at? Thanks. > The last patch is teaching "git submodule deinit" to unset the core.worktree > setting as well. It turned out this one is tricky, as for that we also > have to set it in the counter part, such as "submodule update". > > Thanks, > Stefan > > Stefan Beller (3): > submodule: unset core.worktree if no working tree is present > submodule: ensure core.worktree is set after update > submodule deinit: unset core.worktree > > builtin/submodule--helper.c | 26 ++ > git-submodule.sh| 5 + > submodule.c | 14 ++ > submodule.h | 2 ++ > t/lib-submodule-update.sh | 5 +++-- > t/t7400-submodule-basic.sh | 5 + > 6 files changed, 55 insertions(+), 2 deletions(-)
Re: [RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script
On Wed, Jun 13, 2018 at 1:21 PM Todd Zullinger wrote: > Eric Sunshine wrote: > > Since you're touching all the tests in this script anyhow, perhaps > > modernize them [...] > > (Not necessarily worth a re-roll.) > > These tests were based on similar test_external tests which > use perl. like t0202 & t9700. Both examples use the same > formatting (and use of 'set up'). Perhaps a later clean up > can adjust all three tests? Whichever course of action works for you and Junio is fine. In this case, it's such a minor bit of additional work to modernize the two tests in this script that it would make sense to do so in this patch if you happen to re-roll (and if you agree with me), but is itself probably not worth a re-roll (as mentioned above).
Re: [PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
Kirill Smelkov writes:
> Fetch-pack --all became broken with respect to unusual tags in
> 5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
> and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
> peel values with --all, 2018-06-11). However the test added in
> e9502c0a7f does not explicitly cover all funky cases.
Thanks. Very much appreciated
>
> In order to be sure fetching funky tags will never break, let's
> explicitly test all relevant cases with 4 tag objects pointing to 1) a
> blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
> tag objects themselves are referenced from under regular refs/tags/*
> namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
>
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
> 440858748ae905d48259d4fb67a12a7aa1520cf7HEAD
> ...
> bc4e9e1fa80662b449805b1ac29fc9b1e4c49187refs/tags/tag-to-blob
> # <-- NOTE
> 038f48ad0beaffbea71d186a05084b79e3870cbf
> refs/tags/tag-to-blob^{}
> 520db1f5e1afeaa12b1a8d73ce82db72ca036ee1refs/tags/tag-to-tree
> # <-- NOTE
> 7395c100223b7cd760f58ccfa0d3f3d2dd539bb6
> refs/tags/tag-to-tree^{}
> .../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
> --all ..
> fatal: A git upload-pack: not our ref
> 038f48ad0beaffbea71d186a05084b79e3870cbf
> fatal: The remote end hung up unexpectedly
... except for this bit. I am not sure readers understand what
these two overlong lines want to say. Would it be easier to
understand if you wrote the above like this?
.../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
44085874...HEAD
...
bc4e9e1f...refs/tags/tag-to-blob
038f48ad...refs/tags/tag-to-blob^{}# peeled
520db1f5...refs/tags/tag-to-tree
7395c100...refs/tags/tag-to-tree^{}# peeled
.../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
--all ..
fatal: A git upload-pack: not our ref 038f48ad...
fatal: The remote end hung up unexpectedly
Instead of marking the tag-to-blob and tag-to-tree entries (which
are not where the 'fatal' breakage comes from), I think it makes
more sense to mark the entries that show peeled version (which also
matches the object name in the error message), and by shortening the
line, readers can see more easily which lines are highlighted.
> +test_expect_success 'test --all wrt tag to non-commits' '
> + blob=$(echo "hello blob" | git hash-object -t blob -w --stdin) &&
> + git tag -a -m "tag -> blob" tag-to-blob $blob &&
> + \
> + tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
> + git tag -a -m "tag -> tree" tag-to-tree $tree &&
> + \
> + tree2=$(printf "100644 blob $blob\tfile2" | git mktree) &&
> + commit=$(git commit-tree -m "hello commit" $tree) &&
> + git tag -a -m "tag -> commit" tag-to-commit $commit &&
> + \
> + blob2=$(echo "hello blob2" | git hash-object -t blob -w --stdin) &&
> + tag=$(printf "object $blob2\ntype blob\ntag tag-to-blob2\n\
> +tagger author A U Thor 0 +\n\nhello tag" | git
> mktag) &&
> + git tag -a -m "tag -> tag" tag-to-tag $tag &&
All of the above, while may not be techincallly wrong per-se, look
unnecessarily complex.
I guess the reason why you do the above is because you do not want
to use any object that is reachable via other existing refs and
instead want to ensure these objects are reachable only via the tags
you are creating in this test. Otherwise using HEAD~4:test.txt and
HEAD^{tree} instead of $blob and $tree constructed via hash-object
and mktree would be sufficient and more readable. Oh well.
> + mkdir fetchall &&
> + (
> + cd fetchall &&
> + git init &&
> + git fetch-pack --all .. &&
> + git cat-file blob $blob >/dev/null &&
> + git cat-file tree $tree >/dev/null &&
> + git cat-file commit $commit >/dev/null &&
> + git cat-file tag $tag >/dev/null
> + )
> +'
> +
> test_expect_success 'shallow fetch with tags does not break the repository' '
> mkdir repo1 &&
> (
Re: [PATCH] git-credential-netrc: remove use of "autodie"
Hi, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> Per my reading of the file this was the only thing autodie was doing >> in this file (there was no other code it altered). So let's remove it, >> both to fix the logic error and to get rid of the dependency. >> >> 1. <87efhfvxzu@evledraar.gmail.com> >>(https://public-inbox.org/git/87efhfvxzu@evledraar.gmail.com/) >> 2. >> >> (https://public-inbox.org/git/cahqjxre8okskcck1aphahcclzhox+tzi8nnu2ra74rerx8s...@mail.gmail.com/) >> >> Signed-off-by: Ævar Arnfjörð Bjarmason >> --- >> contrib/credential/netrc/git-credential-netrc | 1 - >> 1 file changed, 1 deletion(-) > > Even though this may not be all that release-critical, let's queue > it so that we do not have to remember to dig it up later ;-) > > Thank you very much to all of you involved in the thread. Should I resend the RFC v2 series as well? The only change I'd make would be to add my signoff on the patches which are 'From: Luis' per suggestion. If you think that's worth adding, I can resend or you may add it while queueing. Whichever is easier for you works for me. Thanks, -- Todd ~~ Age doesn't always bring wisdom. Sometimes it comes alone.
Re: [RFC PATCH v2 2/4] git-credential-netrc: minor whitespace cleanup in test script
Eric Sunshine wrote: > On Tue, Jun 12, 2018 at 11:10 PM, Todd Zullinger wrote: >> Signed-off-by: Todd Zullinger >> --- >> diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh >> b/contrib/credential/netrc/t-git-credential-netrc.sh >> index 58191a62f8..c5661087fe 100755 >> --- a/contrib/credential/netrc/t-git-credential-netrc.sh >> +++ b/contrib/credential/netrc/t-git-credential-netrc.sh >> @@ -17,15 +17,15 @@ >> # set up test repository >> >> test_expect_success \ >> -'set up test repository' \ >> -'git config --add gpg.program test.git-config-gpg' >> + 'set up test repository' \ >> + 'git config --add gpg.program test.git-config-gpg' > > Since you're touching all the tests in this script anyhow, perhaps > modernize them so the title and opening quote of the test body are on > the same line as test_expect_success, and the closing body quote is on > a line of its own? > > test_expect_sucess 'setup test repository' ' > ...test body... > ' > > I also changed "set up" to "setup" to follow existing practice. > > (Not necessarily worth a re-roll.) These tests were based on similar test_external tests which use perl. like t0202 & t9700. Both examples use the same formatting (and use of 'set up'). Perhaps a later clean up can adjust all three tests? -- Todd ~~ How can I tell that the past isn't a fiction designed to account for the discrepancy between my immediate physical sensation and my state of mind? -- Douglas Adams
Re: [PATCH v2] packfile: Correct zlib buffer handling
A couple comments if you happen to re-roll... On Wed, Jun 13, 2018 at 10:22 AM Jeremy Linton wrote: > The buffer being passed to zlib includes a null terminator that On this project, the character mnemonic "NUL" is typically used, not "null" or "NULL" (which is typically reserved for pointers), so: s/null/NUL/g > git needs to keep in place. unpack_compressed_entry() attempts to > detect the case that the source buffer hasn't been fully consumed > by checking to see if the destination buffer has been over consumed. > > This causes a problem, that more recent zlib patches have been > poisoning the unconsumed portions of the buffer which overwrites > the null, while correctly returning length and status. > > Let's replace the null at the end of the buffer to assure that > if its been overwritten by zlib it doesn't result in problems for > git. > > Signed-off-by: Jeremy Linton > --- > diff --git a/packfile.c b/packfile.c > @@ -1433,6 +1433,8 @@ static void *unpack_compressed_entry(struct packed_git > *p, > + buffer[size] = 0; /* assure that the buffer is still terminated */ I think we normally use '\0' for NUL on this project rather than simply 0. The comment is also effectively pure noise since it merely repeats what the code already states clearly (especially when the code says "buffer[size] = '\0';"), so dropping the comment altogether would be reasonable.
Re: [PATCH] fetch-pack: demonstrate --all failure when remote is empty
Kirill Smelkov writes: > ( Junio, please pick up the patch provided in the end ) > > On Tue, Jun 12, 2018 at 06:54:17PM +, Kirill Smelkov wrote: >> On Tue, Jun 12, 2018 at 05:48:49AM -0400, Jeff King wrote: >> > On Mon, Jun 11, 2018 at 09:43:02AM +, Kirill Smelkov wrote: > [...] > >> > > I'm not sure, but I would say that `fetch-pack --all` from an empty >> > > repository should not fail and should just give empty output as fetch >> > > does. >> > >> > Yeah, that seems reasonable to me. The die() that catches this dates >> > back to 2005-era, and we later taught the "fetch" porcelain to handle >> > this. I don't _think_ anybody would be upset that the plumbing learned >> > to treat this as a noop. It's probably a one-liner change in >> > fetch_pack() to return early instead of dying. I actually have a slight preference to the current "attempting to fetch from a total emptiness is so rare that it is worth grabbing attention of whoever does so" behaviour, to be honest. Oh, wait, is this specific to "fetch-pack" and the behaviour of end-user-facing "git fetch" is kept same as before? If then, I'd be somewhat sympathetic to the cause---it would be more convenient for the calling Porcelain script if this turned into a silent noop (even though it would probably make it harder to diagnose when such a Porcelain is set up incorrectly e.g. pointing at an empty repository that is not the one the Porcelain writer intended to fetch from). > However with transport.c being there too, since I'm no longer using > `fetch-pack --all`, now it is best for me to not delve into this story > and just stop with attached patch. If we do not plan to change the behaviour later ourselves, I do not think it makes sense, nor it is fair to those future developers who inherit this project, to declare that the established behaviour is wrong with an 'expect-failure' test like this, to be honest. > +test_expect_failure 'test --all wrt empty.git' ' > + git init --bare empty.git && > + ( > + cd client && > + git fetch-pack --all ../empty.git > + ) > +'
Re: [PATCH] Use hyphenated "remote-tracking branch" (docs and comments)
"Robert P. J. Day" writes: > Use the obvious consensus of hyphenated "remote-tracking branch", and > fix an obvious typo, all in documentation and comments. > > Signed-off-by: Robert P. J. Day Thanks.
Re: [PATCH] git-credential-netrc: remove use of "autodie"
Ævar Arnfjörð Bjarmason writes: > Per my reading of the file this was the only thing autodie was doing > in this file (there was no other code it altered). So let's remove it, > both to fix the logic error and to get rid of the dependency. > > 1. <87efhfvxzu@evledraar.gmail.com> >(https://public-inbox.org/git/87efhfvxzu@evledraar.gmail.com/) > 2. > > (https://public-inbox.org/git/cahqjxre8okskcck1aphahcclzhox+tzi8nnu2ra74rerx8s...@mail.gmail.com/) > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > contrib/credential/netrc/git-credential-netrc | 1 - > 1 file changed, 1 deletion(-) Even though this may not be all that release-critical, let's queue it so that we do not have to remember to dig it up later ;-) Thank you very much to all of you involved in the thread. > diff --git a/contrib/credential/netrc/git-credential-netrc > b/contrib/credential/netrc/git-credential-netrc > index 0b9a94102e..ebfc123ec6 100755 > --- a/contrib/credential/netrc/git-credential-netrc > +++ b/contrib/credential/netrc/git-credential-netrc > @@ -2,7 +2,6 @@ > > use strict; > use warnings; > -use autodie; > > use Getopt::Long; > use File::Basename;
Re: What's cooking in git.git (Jun 2018, #03; Tue, 12)
On Wed, Jun 13, 2018 at 8:56 AM Junio C Hamano wrote: > At this point, only clear fixes to regressions between v2.17.0..master > would be eligible for the final. Is the one you are pointing at a > breakage during that timeframe? No, it was introduced in v2.15.0..v2.16.0. Thanks, Stefan
Re: [PATCH] RelNotes 2.18: clarify where directory rename detection applies
Elijah Newren writes: > Also, since the directory rename detection from this cycle was > specifically added in merge-recursive and not diffcore-rename, remove the > 'in "diff" family" phrase from the note. Thanks.
Re: What's cooking in git.git (Jun 2018, #03; Tue, 12)
Stefan Beller writes: >> A tl;dr summary for -rc2 that hopefully should happen in 24 hours: >> the following four topics >> >> jk/submodule-fsck-loose-fixup >> sb/submodule-merge-in-merge-recursive >> jk/index-pack-maint >> sg/completion-zsh-workaround >> >> are planned to be merged in -rc2. > > It would be nice to have > https://public-inbox.org/git/20180609110414.ga5...@duynguyen.home/ > included as well, but as it is not yet a proper patch, I can > understand why we err on the cautious side. At this point, only clear fixes to regressions between v2.17.0..master would be eligible for the final. Is the one you are pointing at a breakage during that timeframe? Thanks.
[GSoC][PATCH v2 0/2] rebase -i: rewrite the edit-todo functionality in C
This patch rewrites the edit-todo functionality from shell to C. This is part of the effort to rewrite interactive rebase in C. Changes since v1: - Add a new function to launch the sequence editor, as advised by Phillip Wood[0] [0] https://public-inbox.org/git/3bfd3470-4482-fe6a-2cd9-08311a0bb...@talktalk.net/ Alban Gruin (2): editor: add a function to launch the sequence editor rebase--interactive: rewrite the edit-todo functionality in C builtin/rebase--helper.c | 13 - cache.h| 1 + editor.c | 27 +-- git-rebase--interactive.sh | 11 +-- sequencer.c| 31 +++ sequencer.h| 1 + strbuf.h | 2 ++ 7 files changed, 69 insertions(+), 17 deletions(-) -- 2.16.4
[GSoC][PATCH v2 2/2] rebase--interactive: rewrite the edit-todo functionality in C
This rewrites the edit-todo functionality from shell to C.
To achieve that, a new command mode, `edit-todo`, is added, and the
`write-edit-todo` flag is removed, as the shell script does not need to
write the edit todo help message to the todo list anymore.
The shell version is then stripped in favour of a call to the helper.
Signed-off-by: Alban Gruin
---
builtin/rebase--helper.c | 13 -
git-rebase--interactive.sh | 11 +--
sequencer.c| 31 +++
sequencer.h| 1 +
4 files changed, 41 insertions(+), 15 deletions(-)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index ded5e291d..d2990b210 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -12,12 +12,12 @@ static const char * const builtin_rebase_helper_usage[] = {
int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts = REPLAY_OPTS_INIT;
- unsigned flags = 0, keep_empty = 0, rebase_merges = 0, write_edit_todo
= 0;
+ unsigned flags = 0, keep_empty = 0, rebase_merges = 0;
int abbreviate_commands = 0, rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
- ADD_EXEC, APPEND_TODO_HELP
+ ADD_EXEC, APPEND_TODO_HELP, EDIT_TODO
} command = 0;
struct option options[] = {
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
@@ -27,8 +27,6 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "rebase-merges", _merges, N_("rebase merge
commits")),
OPT_BOOL(0, "rebase-cousins", _cousins,
N_("keep original branch points of cousins")),
- OPT_BOOL(0, "write-edit-todo", _edit_todo,
-N_("append the edit-todo message to the todo-list")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -49,6 +47,9 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
N_("insert exec commands in todo list"), ADD_EXEC),
OPT_CMDMODE(0, "append-todo-help", ,
N_("insert the help in the todo list"),
APPEND_TODO_HELP),
+ OPT_CMDMODE(0, "edit-todo", ,
+ N_("edit the todo list during an interactive
rebase"),
+ EDIT_TODO),
OPT_END()
};
@@ -89,6 +90,8 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
if (command == ADD_EXEC && argc == 2)
return !!sequencer_add_exec_commands(argv[1]);
if (command == APPEND_TODO_HELP && argc == 1)
- return !!append_todo_help(write_edit_todo, keep_empty);
+ return !!append_todo_help(0, keep_empty);
+ if (command == EDIT_TODO && argc == 1)
+ return !!edit_todo_list(flags);
usage_with_options(builtin_rebase_helper_usage, options);
}
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c23a7af..2defe607f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -108,16 +108,7 @@ initiate_action () {
--continue
;;
edit-todo)
- git stripspace --strip-comments <"$todo" >"$todo".new
- mv -f "$todo".new "$todo"
- collapse_todo_ids
- git rebase--helper --append-todo-help --write-edit-todo
-
- git_sequence_editor "$todo" ||
- die "$(gettext "Could not execute editor")"
- expand_todo_ids
-
- exit
+ exec git rebase--helper --edit-todo
;;
show-current-patch)
exec git show REBASE_HEAD --
diff --git a/sequencer.c b/sequencer.c
index 1ffd990f7..7cc76332e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4386,6 +4386,37 @@ int append_todo_help(unsigned edit_todo, unsigned
keep_empty)
return ret;
}
+int edit_todo_list(unsigned flags)
+{
+ struct strbuf buf = STRBUF_INIT;
+ const char *todo_file = rebase_path_todo();
+ FILE *todo;
+
+ if (strbuf_read_file(, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ strbuf_stripspace(, 1);
+ todo = fopen_or_warn(todo_file, "w");
+ if (!todo) {
+ strbuf_release();
+ return 1;
+ }
+
+ strbuf_write(, todo);
+ fclose(todo);
+ strbuf_release();
+
+ transform_todos(flags | TODO_LIST_SHORTEN_IDS);
+ append_todo_help(1, 0);
+
+ if (launch_sequence_editor(todo_file, NULL, NULL))
+ return 1;
+
+
[GSoC][PATCH v2 1/2] editor: add a function to launch the sequence editor
As part of the rewrite of interactive rebase, the sequencer will need to
open the sequence editor to allow the user to edit the todo list.
Instead of duplicating the existing launch_editor() function, this
refactors it to a new function, launch_specified_editor(), which takes
the editor as a parameter, in addition to the path, the buffer and the
environment variables. launch_sequence_editor() is then added to launch
the sequence editor.
Signed-off-by: Alban Gruin
---
cache.h | 1 +
editor.c | 27 +--
strbuf.h | 2 ++
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 89a107a7f..c124849a1 100644
--- a/cache.h
+++ b/cache.h
@@ -1472,6 +1472,7 @@ extern const char *fmt_name(const char *name, const char
*email);
extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
+extern const char *git_sequence_editor(void);
extern const char *git_pager(int stdout_is_tty);
extern int is_terminal_dumb(void);
extern int git_ident_config(const char *, const char *, void *);
diff --git a/editor.c b/editor.c
index 9a9b4e12d..c985eee1f 100644
--- a/editor.c
+++ b/editor.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "config.h"
#include "strbuf.h"
#include "run-command.h"
#include "sigchain.h"
@@ -34,10 +35,21 @@ const char *git_editor(void)
return editor;
}
-int launch_editor(const char *path, struct strbuf *buffer, const char *const
*env)
+const char *git_sequence_editor(void)
{
- const char *editor = git_editor();
+ const char *editor = getenv("GIT_SEQUENCE_EDITOR");
+
+ if (!editor)
+ git_config_get_string_const("sequence.editor", );
+ if (!editor)
+ editor = git_editor();
+ return editor;
+}
+
+static int launch_specified_editor(const char *editor, const char *path,
+ struct strbuf *buffer, const char *const
*env)
+{
if (!editor)
return error("Terminal is dumb, but EDITOR unset");
@@ -95,3 +107,14 @@ int launch_editor(const char *path, struct strbuf *buffer,
const char *const *en
return error_errno("could not read file '%s'", path);
return 0;
}
+
+int launch_editor(const char *path, struct strbuf *buffer, const char *const
*env)
+{
+ return launch_specified_editor(git_editor(), path, buffer, env);
+}
+
+int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env)
+{
+ return launch_specified_editor(git_sequence_editor(), path, buffer,
env);
+}
diff --git a/strbuf.h b/strbuf.h
index 60a35aef1..66da9822f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -575,6 +575,8 @@ extern void strbuf_add_unique_abbrev(struct strbuf *sb,
* file's contents are not read into the buffer upon completion.
*/
extern int launch_editor(const char *path, struct strbuf *buffer, const char
*const *env);
+extern int launch_sequence_editor(const char *path, struct strbuf *buffer,
+ const char *const *env);
extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char
*buf, size_t size);
--
2.16.4
Re: GDPR compliance best practices?
On Wed, Jun 13, 2018 at 10:12:18AM -0400, Theodore Y. Ts'o wrote: > Sure, but given that you are the one trying to claim that people need > to do all sorts of extra development work (I don't see any patches No. I am not. I said it is desirable to have a convenient solution for the problem. I did not demand development work or patches from anyone, just kindly asked for a comment on a possible solution. > from you) and suffer performance degredation, the burden of proof is > on _you_ to show that this is a problem that github, et. al., are > likely run into. *You* claimed there was performance degradation, not me. That github et. al. will sooner or later receive such erasure requests is a practical certainty. Google receives them every day in large quantities. Just think about someone who committed smelly code on github and now wants to get a new job and wants to get rid of all associations with those smells. > In particular, keep in mind that distribution of open source code can > only be done under the terms of an open source license --- and a > license is a contract. Not that it would be relevant here, but, depending on jurisdication, it is highly controversial whether open source licenses really constitute contracts (or, for example, promissory estoppel). For the right to erasure, it does not matter whether a contract exists or not. The GDPR explicitly prohibits any use of contracts in a way that undermines the GDPR. Making it an irrevocable contractual obligation to publish the data is not going to be an excuse thus. And Free Software licenses have nothing whatsoever to do with repository metadata. Such software has existed long before version control became so popular. > So in particular, your claim that the data is > no longer necessary (point a) is at the very least going to be subject No, it is github's claim that it must no longer be necessary for being erased, not mine! I clearly stated that if ANY point (not: ALL points) is given, the data must be deleted. Thus, point b, c, d or any other are just as good as point a. > to dispute and is a legal question. I can think of any number of ways > that this could considered necessary in order to assure open source > license compliance, the public interest in terms of allowing forking, > etc. To claim that the data is necessary (which is, as I said, irrelevant) and then say it's not because you can as well use a dummy user string, is self-contradicting. > The bottom line is I'm sure the lawyers at github and Microsoft have > very carefully done their due diligence, and if they are concerned, > I'm sure we'll see patches from them, since after all, they would not Why should they be concerned? They can rewrite history if necessary. They have a solution, though an inconvenient one. As far as the lawyers are concerned, that solution is pefectly fine. Best wishes Peter -- Peter Backes, r...@helen.plasma.xg8.de
[PATCH v2] packfile: Correct zlib buffer handling
The buffer being passed to zlib includes a null terminator that git needs to keep in place. unpack_compressed_entry() attempts to detect the case that the source buffer hasn't been fully consumed by checking to see if the destination buffer has been over consumed. This causes a problem, that more recent zlib patches have been poisoning the unconsumed portions of the buffer which overwrites the null, while correctly returning length and status. Let's replace the null at the end of the buffer to assure that if its been overwritten by zlib it doesn't result in problems for git. Signed-off-by: Jeremy Linton --- packfile.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packfile.c b/packfile.c index 7c1a2519f..8db5d90ca 100644 --- a/packfile.c +++ b/packfile.c @@ -1433,6 +1433,8 @@ static void *unpack_compressed_entry(struct packed_git *p, return NULL; } + buffer[size] = 0; /* assure that the buffer is still terminated */ + return buffer; } -- 2.13.6
Re: GDPR compliance best practices?
On Tue, Jun 12, 2018 at 09:12:19PM +0200, Peter Backes wrote: > This incorrect claim is completely inverting the logic of Art. 17. > > The logic is clarly that if ANY of lit (a) to (f) is satisfied, the > data must be deleted. > > It is not necessary for ALL of them to be satisfied. > > In particular, if the data is no longer necessary for the purpose for > which it was collected, then THAT ALONE is grounds for erasure ((1) > lit. a). It does not matter at all whether processing was consent-based > or whether such consent was withdrawn. Sure, but given that you are the one trying to claim that people need to do all sorts of extra development work (I don't see any patches from you) and suffer performance degredation, the burden of proof is on _you_ to show that this is a problem that github, et. al., are likely run into. In particular, keep in mind that distribution of open source code can only be done under the terms of an open source license --- and a license is a contract. So in particular, your claim that the data is no longer necessary (point a) is at the very least going to be subject to dispute and is a legal question. I can think of any number of ways that this could considered necessary in order to assure open source license compliance, the public interest in terms of allowing forking, etc. The bottom line is I'm sure the lawyers at github and Microsoft have very carefully done their due diligence, and if they are concerned, I'm sure we'll see patches from them, since after all, they would not be interested in seeing the imperial European bureaucrats trying to assess 4% of Microsoft's world-wide revenues --- that's $3.6 billion dollars, by the way. I'm sure if they think it's a concern, their programmers will be right on it. - Ted
[PATCH v10] json_writer V10
From: Jeff Hostetler Here is V10 of my json-writer patch. I fixed the DEVELOPER=1 warnings that Eric pointed out and refactored the indent_pretty() code as Junio suggested. Jeff Hostetler (1): json_writer: new routines to create JSON data Makefile| 2 + json-writer.c | 414 json-writer.h | 105 t/helper/test-json-writer.c | 565 t/helper/test-tool.c| 1 + t/helper/test-tool.h| 1 + t/t0019-json-writer.sh | 331 ++ t/t0019/parse_json.perl | 52 8 files changed, 1471 insertions(+) create mode 100644 json-writer.c create mode 100644 json-writer.h create mode 100644 t/helper/test-json-writer.c create mode 100755 t/t0019-json-writer.sh create mode 100644 t/t0019/parse_json.perl -- 2.9.3
[PATCH v10] json_writer: new routines to create JSON data
From: Jeff Hostetler
Add "struct json_writer" and a series of jw_ routines to compose JSON
data into a string buffer. The resulting string may then be printed by
commands wanting to support a JSON-like output format.
The json_writer is limited to correctly formatting structured data for
output. It does not attempt to build an object model of the JSON data.
We say "JSON-like" because we do not enforce the Unicode (usually UTF-8)
requirement on string fields. Internally, Git does not necessarily have
Unicode/UTF-8 data for most fields, so it is currently unclear the best
way to enforce that requirement. For example, on Linux pathnames can
contain arbitrary 8-bit character data, so a command like "status" would
not know how to encode the reported pathnames. We may want to revisit
this (or double encode such strings) in the future.
Helped-by: Eric Sunshine
Helped-by: René Scharfe
Helped-by: Wink Saville
Helped-by: Ramsay Jones
Signed-off-by: Jeff Hostetler
---
Makefile| 2 +
json-writer.c | 414
json-writer.h | 105
t/helper/test-json-writer.c | 565
t/helper/test-tool.c| 1 +
t/helper/test-tool.h| 1 +
t/t0019-json-writer.sh | 331 ++
t/t0019/parse_json.perl | 52
8 files changed, 1471 insertions(+)
create mode 100644 json-writer.c
create mode 100644 json-writer.h
create mode 100644 t/helper/test-json-writer.c
create mode 100755 t/t0019-json-writer.sh
create mode 100644 t/t0019/parse_json.perl
diff --git a/Makefile b/Makefile
index 1d27f36..5a781e2 100644
--- a/Makefile
+++ b/Makefile
@@ -709,6 +709,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
TEST_BUILTINS_OBJS += test-genrandom.o
TEST_BUILTINS_OBJS += test-hashmap.o
TEST_BUILTINS_OBJS += test-index-version.o
+TEST_BUILTINS_OBJS += test-json-writer.o
TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
TEST_BUILTINS_OBJS += test-match-trees.o
TEST_BUILTINS_OBJS += test-mergesort.o
@@ -871,6 +872,7 @@ LIB_OBJS += hashmap.o
LIB_OBJS += help.o
LIB_OBJS += hex.o
LIB_OBJS += ident.o
+LIB_OBJS += json-writer.o
LIB_OBJS += kwset.o
LIB_OBJS += levenshtein.o
LIB_OBJS += line-log.o
diff --git a/json-writer.c b/json-writer.c
new file mode 100644
index 000..115de3e
--- /dev/null
+++ b/json-writer.c
@@ -0,0 +1,414 @@
+#include "cache.h"
+#include "json-writer.h"
+
+void jw_init(struct json_writer *jw)
+{
+ strbuf_reset(>json);
+ strbuf_reset(>open_stack);
+ jw->need_comma = 0;
+ jw->pretty = 0;
+}
+
+void jw_release(struct json_writer *jw)
+{
+ strbuf_release(>json);
+ strbuf_release(>open_stack);
+}
+
+/*
+ * Append JSON-quoted version of the given string to 'out'.
+ */
+static void append_quoted_string(struct strbuf *out, const char *in)
+{
+ unsigned char c;
+
+ strbuf_addch(out, '"');
+ while ((c = *in++) != '\0') {
+ if (c == '"')
+ strbuf_addstr(out, "\\\"");
+ else if (c == '\\')
+ strbuf_addstr(out, "");
+ else if (c == '\n')
+ strbuf_addstr(out, "\\n");
+ else if (c == '\r')
+ strbuf_addstr(out, "\\r");
+ else if (c == '\t')
+ strbuf_addstr(out, "\\t");
+ else if (c == '\f')
+ strbuf_addstr(out, "\\f");
+ else if (c == '\b')
+ strbuf_addstr(out, "\\b");
+ else if (c < 0x20)
+ strbuf_addf(out, "\\u%04x", c);
+ else
+ strbuf_addch(out, c);
+ }
+ strbuf_addch(out, '"');
+}
+
+static void indent_pretty(struct json_writer *jw)
+{
+ int k;
+
+ for (k = 0; k < jw->open_stack.len; k++)
+ strbuf_addstr(>json, " ");
+}
+
+/*
+ * Begin an object or array (either top-level or nested within the currently
+ * open object or array).
+ */
+static void begin(struct json_writer *jw, char ch_open, int pretty)
+{
+ jw->pretty = pretty;
+
+ strbuf_addch(>json, ch_open);
+
+ strbuf_addch(>open_stack, ch_open);
+ jw->need_comma = 0;
+}
+
+/*
+ * Assert that the top of the open-stack is an object.
+ */
+static void assert_in_object(const struct json_writer *jw, const char *key)
+{
+ if (!jw->open_stack.len)
+ BUG("json-writer: object: missing jw_object_begin(): '%s'",
key);
+ if (jw->open_stack.buf[jw->open_stack.len - 1] != '{')
+ BUG("json-writer: object: not in object: '%s'", key);
+}
+
+/*
+ * Assert that the top of the open-stack is an array.
+ */
+static void assert_in_array(const struct json_writer *jw)
+{
+ if (!jw->open_stack.len)
+ BUG("json-writer: array: missing jw_array_begin()");
+ if (jw->open_stack.buf[jw->open_stack.len - 1] != '[')
+
[PATCH] fetch-pack: demonstrate --all failure when remote is empty
( Junio, please pick up the patch provided in the end )
On Tue, Jun 12, 2018 at 06:54:17PM +, Kirill Smelkov wrote:
> On Tue, Jun 12, 2018 at 05:48:49AM -0400, Jeff King wrote:
> > On Mon, Jun 11, 2018 at 09:43:02AM +, Kirill Smelkov wrote:
[...]
> > > I'm not sure, but I would say that `fetch-pack --all` from an empty
> > > repository should not fail and should just give empty output as fetch
> > > does.
> >
> > Yeah, that seems reasonable to me. The die() that catches this dates
> > back to 2005-era, and we later taught the "fetch" porcelain to handle
> > this. I don't _think_ anybody would be upset that the plumbing learned
> > to treat this as a noop. It's probably a one-liner change in
> > fetch_pack() to return early instead of dying.
>
> Ok, I will try to send related testcase, and it is indeed easy to find
> - the fix itself.
I started doing it in full with the following
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1581,6 +1581,8 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
if (!ref) {
packet_flush(fd[1]);
+ if (nr_sought == 0) // XXX or better args->fetch_all
+ return NULL; /* nothing to fetch */
die(_("no matching remote head"));
}
prepare_shallow_info(, shallow);
but then came to the fact that !ref fetch_pack() return is analyzed in 2
places:
- in builtin/fetch-pack.c itself:
ref = fetch_pack(, fd, conn, ref, dest, sought, nr_sought,
, pack_lockfile_ptr, protocol_v0);
...
ret = !ref;
- and in transport.c in fetch_refs_via_pack():
case protocol_v1:
case protocol_v0:
refs = fetch_pack(, data->fd, data->conn,
refs_tmp ? refs_tmp : transport->remote_refs,
dest, to_fetch, nr_heads, >shallow,
>pack_lockfile, data->version);
break;
...
if (refs == NULL)
ret = -1;
As I don't know git codebase well-enough I don't see offhand how to
distinguish empty result from a real error when something was requested
and not fetched. If it would be only builtin/fetch-pack I could start to
play ugly games with analyzing too in the calling site args.fetch_all
and nr_though and if at that level we also know we requested nothing,
don't treat !refs as an error.
However with transport.c being there too, since I'm no longer using
`fetch-pack --all`, now it is best for me to not delve into this story
and just stop with attached patch.
Thanks,
Kirill
8<
>From 76d80ffcfd4574715545c62413d64d40af063d09 Mon Sep 17 00:00:00 2001
From: Kirill Smelkov
Date: Wed, 13 Jun 2018 15:46:00 +0300
Subject: [PATCH] fetch-pack: demonstrate --all failure when remote is empty
Currently `fetch-pack --all` from an empty repository gives:
fatal: no matching remote head
However it would be logical for this fetch operation to succeed with
empty result. Add test showing the failure.
Signed-off-by: Kirill Smelkov
---
t/t5500-fetch-pack.sh | 8
1 file changed, 8 insertions(+)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 82aee1c2d8..2234bad411 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -528,6 +528,14 @@ test_expect_success 'test --all with tag to non-tip' '
)
'
+test_expect_failure 'test --all wrt empty.git' '
+ git init --bare empty.git &&
+ (
+ cd client &&
+ git fetch-pack --all ../empty.git
+ )
+'
+
test_expect_success 'shallow fetch with tags does not break the repository' '
mkdir repo1 &&
(
--
2.18.0.rc1.253.gf85a566b11.dirty
Re: is there a canonical doc about how to deal with whitespace issues?
On Mon, 11 Jun 2018, Stefan Beller wrote:
> On Fri, Jun 8, 2018 at 10:15 AM Derrick Stolee wrote:
> >
> > On 6/8/2018 9:18 AM, Robert P. J. Day wrote:
> > >for one of my courses, i wanted to write a section about the
> > > various techniques for dealing with whitespace issues in git, so
> > > i started
>
> What do you mean by white space issues?
> That in itself is a complex topic:
i know ... it's not even clear that just dealing with EOL
standardization shouldn't be a topic all by itself.
> * There are 3 different modes to ignore white space changes:
> - trailing whitespaces,
> - conversion of tab to space and back
> These two are caught by the default in 'git diff --check'
> - any white space change
> This is interesting to ignore in git-blame[1], but sometimes
> it is actually interesting.
>
> [1] See also
> https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html
>
>
>
> > > making a list, things like:
> > >
> > >- running "git diff --check"
> > >- "git commit --cleanup=" possibilities
> > >- config options like core.{eol,safecrlf,autocrlf}
>
> This sounds more like line ending or cross platform issues
> than whitespaces (except .eol)
i just started a quick-and-dirty wiki page as a reference to things
that relate to whitespace:
http://crashcourse.ca/dokuwiki/doku.php?id=git_whitespace
it's not even *remotely* close to comprehensive, i just wanted to
start making a list. feel free to make other suggestions as i keep
adding to that page.
rday
--
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
[PATCH] fetch-pack: test explicitly that --all can fetch tag references pointing to non-commits
Fetch-pack --all became broken with respect to unusual tags in
5f0fc64513 (fetch-pack: eliminate spurious error messages, 2012-09-09),
and was fixed only recently in e9502c0a7f (fetch-pack: don't try to fetch
peel values with --all, 2018-06-11). However the test added in
e9502c0a7f does not explicitly cover all funky cases.
In order to be sure fetching funky tags will never break, let's
explicitly test all relevant cases with 4 tag objects pointing to 1) a
blob, 2) a tree, 3) a commit, and 4) another tag objects. The referenced
tag objects themselves are referenced from under regular refs/tags/*
namespace. Before e9502c0a7f `fetch-pack --all` was failing e.g. this way:
.../git/t/trash directory.t5500-fetch-pack/fetchall$ git ls-remote ..
440858748ae905d48259d4fb67a12a7aa1520cf7HEAD
...
bc4e9e1fa80662b449805b1ac29fc9b1e4c49187refs/tags/tag-to-blob
# <-- NOTE
038f48ad0beaffbea71d186a05084b79e3870cbfrefs/tags/tag-to-blob^{}
520db1f5e1afeaa12b1a8d73ce82db72ca036ee1refs/tags/tag-to-tree
# <-- NOTE
7395c100223b7cd760f58ccfa0d3f3d2dd539bb6refs/tags/tag-to-tree^{}
.../git/t/trash directory.t5500-fetch-pack/fetchall$ git fetch-pack
--all ..
fatal: A git upload-pack: not our ref
038f48ad0beaffbea71d186a05084b79e3870cbf
fatal: The remote end hung up unexpectedly
Signed-off-by: Kirill Smelkov
---
t/t5500-fetch-pack.sh | 28
1 file changed, 28 insertions(+)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index f20bb59d22..b560d90c7b 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -528,6 +528,34 @@ test_expect_success 'test --all with tag to non-tip' '
)
'
+test_expect_success 'test --all wrt tag to non-commits' '
+ blob=$(echo "hello blob" | git hash-object -t blob -w --stdin) &&
+ git tag -a -m "tag -> blob" tag-to-blob $blob &&
+ \
+ tree=$(printf "100644 blob $blob\tfile" | git mktree) &&
+ git tag -a -m "tag -> tree" tag-to-tree $tree &&
+ \
+ tree2=$(printf "100644 blob $blob\tfile2" | git mktree) &&
+ commit=$(git commit-tree -m "hello commit" $tree) &&
+ git tag -a -m "tag -> commit" tag-to-commit $commit &&
+ \
+ blob2=$(echo "hello blob2" | git hash-object -t blob -w --stdin) &&
+ tag=$(printf "object $blob2\ntype blob\ntag tag-to-blob2\n\
+tagger author A U Thor 0 +\n\nhello tag" | git mktag)
&&
+ git tag -a -m "tag -> tag" tag-to-tag $tag &&
+ \
+ mkdir fetchall &&
+ (
+ cd fetchall &&
+ git init &&
+ git fetch-pack --all .. &&
+ git cat-file blob $blob >/dev/null &&
+ git cat-file tree $tree >/dev/null &&
+ git cat-file commit $commit >/dev/null &&
+ git cat-file tag $tag >/dev/null
+ )
+'
+
test_expect_success 'shallow fetch with tags does not break the repository' '
mkdir repo1 &&
(
--
2.18.0.rc1.253.gf85a566b11.dirty
Hello My Dear Friend,
I have a business proposal in the tune of $10.2 Million USD for you to handle with me. I have opportunity to transfer this abandon fund to your bank account in your country which belongs to our client. I am inviting you in this transaction where this money can be shared between us at ratio of 60/40% and help the needy around us don’t be afraid of anything I am with you I will instruct you what you will do to maintain this fund. Please kindly contact me with your information's if you are interested in this transaction for more details. Your Name:.. Your Bank Name:. Your Account Number:... Your Telephone Number: Your Country And Address: Your Age And Sex:... Thanks Mrs.Zainab Ahmed,
[PATCH] git-credential-netrc: remove use of "autodie"
The "autodie" module was added in Perl 5.10.1, but our INSTALL
document says "version 5.8 or later is needed".
As discussed in <87efhfvxzu@evledraar.gmail.com> this script is in
contrib/, so we might not want to apply that policy, however in this
case "autodie" was recently added as a "gratuitous safeguard" in
786ef50a23 ("git-credential-netrc: accept gpg option",
2018-05-12) (see
).
Looking at it more carefully the addition of "autodie" inadvertently
introduced a logic error, since having it is equivalent to this patch:
@@ -245,10 +244,10 @@ sub load_netrc {
if ($gpgmode) {
my @cmd = ($options{'gpg'}, qw(--decrypt), $file);
log_verbose("Using GPG to open $file: [@cmd]");
- open $io, "-|", @cmd;
+ open $io, "-|", @cmd or die "@cmd: $!";
} else {
log_verbose("Opening $file...");
- open $io, '<', $file;
+ open $io, '<', $file or die "$file: $!$!;
}
# nothing to do if the open failed (we log the error later)
As shown in the context the intent of that code is not do die but to
log the error later.
Per my reading of the file this was the only thing autodie was doing
in this file (there was no other code it altered). So let's remove it,
both to fix the logic error and to get rid of the dependency.
1. <87efhfvxzu@evledraar.gmail.com>
(https://public-inbox.org/git/87efhfvxzu@evledraar.gmail.com/)
2.
(https://public-inbox.org/git/cahqjxre8okskcck1aphahcclzhox+tzi8nnu2ra74rerx8s...@mail.gmail.com/)
Signed-off-by: Ævar Arnfjörð Bjarmason
---
contrib/credential/netrc/git-credential-netrc | 1 -
1 file changed, 1 deletion(-)
diff --git a/contrib/credential/netrc/git-credential-netrc
b/contrib/credential/netrc/git-credential-netrc
index 0b9a94102e..ebfc123ec6 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -2,7 +2,6 @@
use strict;
use warnings;
-use autodie;
use Getopt::Long;
use File::Basename;
--
2.17.0.290.gded63e768a
Re: [PATCH] checkout files in-place
On Tue, Jun 12, 2018 at 11:51 AM Edward Thomson wrote: > > On Tue, Jun 12, 2018 at 09:13:54AM +0300, Orgad Shaneh wrote: > > Some of my colleagues use an ancient version of Source Insight, which also > > locks files for write. > > If that application is locking files for writing (that is to say, it did > not specify the `FILE_SHARE_WRITE` bit in the sharing modes during > `CreateFile`) then this patch would not help. > > Applications, generally speaking, should be locking files for write. > It's the default in Win32 and .NET's file open APIs because few > applications are prepared to detect and support a file changing out from > underneath them in the middle of a read. I agree. > > It's less important than it was before those fixes, but it is still needed > > for users of Qt Creator 4.6 (previous versions just avoided mmap, 4.7 uses > > mmap only for system headers). Other tools on Windows might as well > > misbehave. > > I don't understand what mmap'ing via `CreateFileMapping` has to do with > this. It takes an existing `HANDLE` that was opened with `CreateFile`, > which is where the sharing mode was supplied. I'm not completely sure. The file is opened using CreateFile[1] with FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE. Then this handle is passed to CreateFileMapping[2]. For a reason I don't understand, when mapping is used, the handle is never released (until the file is closed), but when it is not used, the file is being read, then the handle is released. Maybe Ivan or Nikolai can shed some light on this process. Anyway, with Qt Creator 4.7 this should be a non-issue, so I'm reluctant about this change here. > I would be surprised if there are other tools on Windows that have > specified `FILE_SHARE_WRITE` but not `FILE_SHARE_DELETE`. Generally > speaking, if you don't care about another process changing a file > underneath you then you should specify both. If you do then you should > specify neither. The problem is that even if you specify FILE_SHARE_WRITE and FILE_SHARE_DELETE, the file can be unlinked, but it cannot be created with the same name until its handle is closed, unless you rename it *before* unlinking. - Orgad [1] https://github.com/llvm-mirror/llvm/blob/371257e/lib/Support/Windows/Path.inc#L1045 [2] https://github.com/llvm-mirror/llvm/blob/371257e/lib/Support/Windows/Path.inc#L836
Re: [RFC PATCH v2 0/4] contrib/credential/netrc Makefile & test improvements
On Tue, Jun 12, 2018 at 11:10 PM Todd Zullinger wrote: > > This replaces my 2/2 "use in-tree Git.pm for tests" with > Luis's improved version. It also adds Luis's fix to ensure > the proper exit status on test failures and a minor > whitespace cleanup. > > Is it alright to forge your signoff Luis? Thanks, that's fine. You should sign-off on those, too: you identified the problems, told me the solutions, tested them, and deserve the credit. Moreover, Documentation/SubmittingPatches encourages it. When the patches are ready, I think we'll need the primary author (Ted) to acknowledge before the maintainer (Junio) can approve. > Luis Marsano (2): > git-credential-netrc: use in-tree Git.pm for tests > git-credential-netrc: fix exit status when tests fail > > Todd Zullinger (2): > git-credential-netrc: make "all" default target of Makefile > git-credential-netrc: minor whitespace cleanup in test script > > contrib/credential/netrc/Makefile | 3 +++ > contrib/credential/netrc/t-git-credential-netrc.sh | 9 + > contrib/credential/netrc/test.pl | 5 +++-- > 3 files changed, 11 insertions(+), 6 deletions(-)
Re: [RFC PATCH 0/2] contrib/credential/netrc Makefile & test improvements
On Tue, Jun 12, 2018 at 11:08 PM Todd Zullinger wrote: > As far as removing the autodie dep, is there anything more > to that than dropping the 'use autodie' line? It looks like > doing so leaves us no worse than we were before, but I > haven't written any perl in a long time. Erasing that line should remove the dependency. I added it as a gratuitous safeguard. I think it's fine to simply remove it. If anyone knows better, of course, I hope they'll say. > Thanks, > > -- > Todd > ~~ > Ocean, n. A body of water occupying about two-thirds of a world made > for man -- who has no gills. > -- Ambrose Bierce, "The Devil's Dictionary" >
