* RE: [PATCH] http: http.emptyauth should allow empty (not just NULL) usernames
From: David Turner @ 2016-10-03 21:54 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git@vger.kernel.org, sandals@crustytoothpaste.net
In-Reply-To: <20161003210100.t5nqknwfotag3lmj@sigill.intra.peff.net>
> -----Original Message-----
> From: Jeff King [mailto:peff@peff.net]
> Sent: Monday, October 03, 2016 5:01 PM
> To: David Turner
> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net
> Subject: Re: [PATCH] http: http.emptyauth should allow empty (not just
> NULL) usernames
>
> On Mon, Oct 03, 2016 at 01:19:28PM -0400, David Turner wrote:
>
> > When using kerberos authentication, one URL pattern which is allowed
> > is http://@gitserver.example.com. This leads to a username of
> > zero-length, rather than a NULL username. But the two cases should be
> > treated the same by http.emptyauth.
> >
> > Signed-off-by: David Turner <dturner@twosigma.com>
> > ---
> > http.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/http.c b/http.c
> > index 82ed542..bd0dba2 100644
> > --- a/http.c
> > +++ b/http.c
> > @@ -351,7 +351,7 @@ static int http_options(const char *var, const
> > char *value, void *cb)
> >
> > static void init_curl_http_auth(CURL *result) {
> > - if (!http_auth.username) {
> > + if (!http_auth.username || !*http_auth.username) {
>
> Hmm. This fixes this caller, but what about other users of the credential
> struct? I wonder if the correct fix is in credential_from_url(), which
> should avoid writing an empty field.
>
> OTOH, I can imagine that "http://user:@example.com" would be a way to say
> "I have a username and the password is blank" without getting prompted.
> Which makes me wonder if it is useful to say "my username is blank" in the
> same way.
Yes, that was my thought process.
> I dunno. The code path you are changing _only_ affects anything if the
> http.emptyauth config is set. But I guess I just don't understand why you
> would say "http://@gitserver" in the first place. Is that a common thing?
>
> -Peff
I have no idea if it is common. I know that we do it.
It used to be that git 2.8/libcurl would handle @gitserver as if the username were blank, but then we upgraded our company's libcurl and it broke (git started prompting for a password). I do not know what the previous version of libcurl was.
The reason we have a required-to-be-blank username/password is apparently Kerberos (or something about our particular Kerberos configuration), which I treat as inscrutable black magic.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #08; Tue, 27)
From: Stefan Beller @ 2016-10-03 21:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <xmqqwphp5f9q.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 3, 2016 at 1:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> I am looking at the tip of jc/attr-more and for a minimum
>> thread safety we'd want to change the call sites to be aware of the
>> threads, i.e. instead of doing
>>
> static struct git_attr_check *check;
>> if (!check)
>> check = git_attr_check_initl("crlf", "ident",
>> "filter", "eol", "text",
>> NULL);
>>
>> We'd rather call
>>
>> struct git_attr_check *check;
>> check = git_attr_check_lookup_or_initl_threadsafe(
>> "crlf", "ident", "filter", "eol", "text", NULL);
>> if (!git_check_attr(path, check)) {
>> ...
>
> I actually am hoping that the "static" can be kept so that we can
> minimize the cost of interning these symbols into struct git_attr.
>
> The initialization would thus become something like:
>
> static struct git_attr_check *check;
> git_attr_check_initl(&check, "crlf", "ident", ..., NULL);
>
> The structure will have an array of git_attr[], once interned will
> be shared and used by everybody. _initl() will need to make sure
> that the "check" pointer is updated atomically so that multiple
> people racing to initialize this variable will not step on each
> others' toes.
I see and that mutex to protect the first argument of git_attr_check_initl
is unrelated to the actual pointer; we can use a global mutex for that,
or rather a static scoped mutex in attr.c, such that all parallel
racing git_attr_check_initl try to acquire that init_lock and only
one succeeds and that one makes sure to first initialize a git_attr_check
completely and then atomically storing the pointer to the &check location.
>
> Then the use site would do something like
>
> const char *result[... some size ...];
... some size ... depends on the git_attr_check->check_nr
and not on the path as I first assumed. So when we still
want to go fast here:
static struct git_attr_check *check;
/* hard code 2 as it has to hold results for crlf and ident only */
const char *results[2];
if (!check)
git_attr_check_initl(&check, "crlf", "ident", NULL);
if (nr != check->check_nr)
ALLOC_GROW(results, check->check_nr, alloc);
git_check_attr(path, check, &result);
// result[i] contains the result for the i-th element of check
// Note: git_attr_check_elem seems to be useless now, as the
// results are not stored in there, we only make use of the `attr` key.
^ permalink raw reply
* Re: [PATCH v2 0/5] receive-pack: quarantine pushed objects
From: Jeff King @ 2016-10-03 21:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Turner
In-Reply-To: <xmqqlgy55dmk.fsf@gitster.mtv.corp.google.com>
On Mon, Oct 03, 2016 at 02:25:23PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Here's a v2; the original was at:
> >
> > http://public-inbox.org/git/20160930193533.ynbepaago6oycg5t@sigill.intra.peff.net/
> >
> > which contains the rationale. The required alternate-objects patches
> > (both the "allow recursive relative" one, and the helper to add an
> > internal alt-odb) have been pushed into their own series, that I posted
> > here:
> >
> > http://public-inbox.org/git/20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net/
> >
> > This needs to be applied on top.
>
> Sorry, you lost me. So this 5-patch series comes on top of a brand
> new 18-patch clean-up series?
Yes, sorry to be unclear. The final sentence should have been:
This v2 quarantine series need to be applied on top of that cleanup
series.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 0/2] place cherry pick line below commit title
From: Jonathan Tan @ 2016-10-03 21:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Christian Couder
In-Reply-To: <xmqqbmz16y42.fsf@gitster.mtv.corp.google.com>
On 10/03/2016 12:17 PM, Junio C Hamano wrote:
> It may be necessary for the code to analize the lines in a block
> identified as "likely to be a trailing block" more carefully,
> though. The example 59f0aa94 in the message you are responding to
> has "Link 1:" that consists of 3 physical lines. An instruction to
> interpret-trailers to add a new one _after_ "Link-$n:" may have to
> treat these as a single logical line and do the addition after them,
> i.e. before "Link 2:" line, for example.
>
> I also saw
>
> Signed-off-by: Some body <some@body.xz> (some comment
> that extends to the next line without being indented)
> Sined-off-by: Some body Else <some.body@else.xz>
>
> where the only clue that the second line is logically a part of the
> first one was the balancing of parentheses (or [brakets]). To
> accomodate real-world use cases, you may have to take into account a
> lot more than the strict rfc-822 style "line folding".
If we define a logical trailer line as the set of physical lines until
the next trailer line...it is true that this has some precedence in that
such lines can be written (although this might not be intentional):
$ git interpret-trailers --trailer "a=foo
bar" </dev/null
a: foo
bar
This solves the "Line 1:" case above, but raises other issues:
1. Checking for existence (trailer.ifexists) is now conceptually more
difficult. For example, handling of inner whitespace in between lines
might be confusing (currently, there is only one line, and whitespace
handling is clearly described).
2. Replacement (trailer.ifexists=replace) might replace more than expected.
3. There is a corner case - the first line might not be a trailer line.
Even if interpret-trailers knew about "(cherry picked from", a user
might enter something else there. (We could just declare such blocks as
non-trailers, but if we are already loosening the definition of a
trailer, this might be something that we should allow.)
My initial thought was to think of a trailer as a block of trailer lines
possibly interspersed with other lines. This leads to interpret-trailers
placing the trailer in the wrong place if trailer.where=after, as you
describe, but this might not be a big problem if trailer.where=after is
not widely used (and it is not the default). (The other options behave
as expected.)
There are other options like checking for indentation or checking for
balanced parentheses/brackets, but I think that these would lead to
surprising behavior for the user (this would mean that whitespace or
certain characters could turn a valid trailer into an invalid one or
vice versa, or change the behavior of trailer.ifexists, especially
"replace").
^ permalink raw reply
* Re: [PATCH v2 0/5] receive-pack: quarantine pushed objects
From: Junio C Hamano @ 2016-10-03 21:25 UTC (permalink / raw)
To: Jeff King; +Cc: git, David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Here's a v2; the original was at:
>
> http://public-inbox.org/git/20160930193533.ynbepaago6oycg5t@sigill.intra.peff.net/
>
> which contains the rationale. The required alternate-objects patches
> (both the "allow recursive relative" one, and the helper to add an
> internal alt-odb) have been pushed into their own series, that I posted
> here:
>
> http://public-inbox.org/git/20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net/
>
> This needs to be applied on top.
Sorry, you lost me. So this 5-patch series comes on top of a brand
new 18-patch clean-up series?
^ permalink raw reply
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Stefan Beller @ 2016-10-03 21:23 UTC (permalink / raw)
To: Linus Torvalds, Heiko Voigt; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CAGZ79kYXx+a+U3o6FPBy2mHOy4BxxuF97t6iyedPLk0Qw1Jx1A@mail.gmail.com>
+cc Heiko
On Mon, Oct 3, 2016 at 2:17 PM, Stefan Beller <sbeller@google.com> wrote:
>
> * sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
> - push: change submodule default to check
>
> Turn the default of "push.recurseSubmodules" to "check".
>
> Will hold to wait for hv/submodule-not-yet-pushed-fix
>
> This reveals that the "check" mode is too inefficient to use in
> real projects, even in ones as small as git itself.
> cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
So maybe we should eject this series from pu as long as
hv/submodule-not-yet-pushed-fix is ejected to enable you
running pu happily.
Thanks,
Stefan
^ permalink raw reply
* Re: Slow pushes on 'pu' - even when up-to-date..
From: Stefan Beller @ 2016-10-03 21:17 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CA+55aFyos78qODyw57V=w13Ux5-8SvBqObJFAq22K+XKPWVbAA@mail.gmail.com>
On Mon, Oct 3, 2016 at 2:11 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> This seems to be because I'm now on 'pu' as of a day or two ago in
> order to test the abbrev logic, but lookie here:
>
> time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
> .. shows all the branches and tags ..
> real 0m0.655s
> user 0m0.011s
> sys 0m0.004s
>
> so the remote is fast to connect to, and with network connection
> overhead and everything, it's just over half a second. But then:
>
> time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
>
> and it just sits there, and it's at 100% CPU the whole time, until it says:
>
> Everything up-to-date
>
> real 1m7.307s
> user 1m2.761s
> sys 0m0.475s
>
> Whaa? It took a *minute* of CPU time to decide that everything was up-to-date?
>
> That's just not right. The branch is entirely up-to-date:
>
> git rev-parse HEAD
> af79ad2b1f337a00aa150b993635b10bc68dc842
>
> git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux master
> af79ad2b1f337a00aa150b993635b10bc68dc842 refs/heads/master
>
> so there should be no need for any history walking. But it sure is
> doing *something*. A minute of CPU time on my machine is actually a
> pretty damn big deal.
>
> Looking at the trace, there's no IO - there's no back-and-forth about
> "I have this, do you have it?" or anything like that. The system call
> trace is just a lot of allocations, which I think means that "git
> push" is walking a lot of objects but not doing anything useful.
>
> I bisected it to commit 60cd66f "push: change submodule default to
> check", which makes little sense since I have no submodules, but there
> you go.. Apparently RECURSE_SUBMODULES_CHECK is just terminally
> broken.
Sorry for breaking you, too.
Junio complained about that when I was proposing the topic; but now
the strategy seems to just wait until Heiko fixed the
RECURSE_SUBMODULES_CHECK to be less broken.
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Will hold to wait for hv/submodule-not-yet-pushed-fix
This reveals that the "check" mode is too inefficient to use in
real projects, even in ones as small as git itself.
cf. <xmqqh9aaot49.fsf@gitster.mtv.corp.google.com>
Which itself is
* hv/submodule-not-yet-pushed-fix (2016-09-14) 2 commits
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
The last two in the original series seem to break a few tests when
queued to 'pu', and dropped for now.
Waiting for a reroll.
^ permalink raw reply
* Slow pushes on 'pu' - even when up-to-date..
From: Linus Torvalds @ 2016-10-03 21:11 UTC (permalink / raw)
To: Junio C Hamano, Stefan Beller; +Cc: Git Mailing List
This seems to be because I'm now on 'pu' as of a day or two ago in
order to test the abbrev logic, but lookie here:
time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
.. shows all the branches and tags ..
real 0m0.655s
user 0m0.011s
sys 0m0.004s
so the remote is fast to connect to, and with network connection
overhead and everything, it's just over half a second. But then:
time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
and it just sits there, and it's at 100% CPU the whole time, until it says:
Everything up-to-date
real 1m7.307s
user 1m2.761s
sys 0m0.475s
Whaa? It took a *minute* of CPU time to decide that everything was up-to-date?
That's just not right. The branch is entirely up-to-date:
git rev-parse HEAD
af79ad2b1f337a00aa150b993635b10bc68dc842
git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux master
af79ad2b1f337a00aa150b993635b10bc68dc842 refs/heads/master
so there should be no need for any history walking. But it sure is
doing *something*. A minute of CPU time on my machine is actually a
pretty damn big deal.
Looking at the trace, there's no IO - there's no back-and-forth about
"I have this, do you have it?" or anything like that. The system call
trace is just a lot of allocations, which I think means that "git
push" is walking a lot of objects but not doing anything useful.
I bisected it to commit 60cd66f "push: change submodule default to
check", which makes little sense since I have no submodules, but there
you go.. Apparently RECURSE_SUBMODULES_CHECK is just terminally
broken.
Linus
^ permalink raw reply
* Re: [PATCH] http: http.emptyauth should allow empty (not just NULL) usernames
From: Jeff King @ 2016-10-03 21:01 UTC (permalink / raw)
To: David Turner; +Cc: git, sandals
In-Reply-To: <1475515168-29679-1-git-send-email-dturner@twosigma.com>
On Mon, Oct 03, 2016 at 01:19:28PM -0400, David Turner wrote:
> When using kerberos authentication, one URL pattern which is
> allowed is http://@gitserver.example.com. This leads to a username
> of zero-length, rather than a NULL username. But the two cases
> should be treated the same by http.emptyauth.
>
> Signed-off-by: David Turner <dturner@twosigma.com>
> ---
> http.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index 82ed542..bd0dba2 100644
> --- a/http.c
> +++ b/http.c
> @@ -351,7 +351,7 @@ static int http_options(const char *var, const char *value, void *cb)
>
> static void init_curl_http_auth(CURL *result)
> {
> - if (!http_auth.username) {
> + if (!http_auth.username || !*http_auth.username) {
Hmm. This fixes this caller, but what about other users of the
credential struct? I wonder if the correct fix is in
credential_from_url(), which should avoid writing an empty
field.
OTOH, I can imagine that "http://user:@example.com" would be a way to
say "I have a username and the password is blank" without getting
prompted. Which makes me wonder if it is useful to say "my username is
blank" in the same way.
I dunno. The code path you are changing _only_ affects anything
if the http.emptyauth config is set. But I guess I just don't understand
why you would say "http://@gitserver" in the first place. Is that a
common thing?
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/5] Add --format to tag verification
From: Junio C Hamano @ 2016-10-03 20:58 UTC (permalink / raw)
To: Santiago Torres; +Cc: git, peff, sunshine, walters
In-Reply-To: <20161003144901.4yainza537g5bujz@LykOS.localdomain>
Santiago Torres <santiago@nyu.edu> writes:
> Hi, Junio.
>> I however notice that there is no new tests to protect these two new
>> features from future breakages. Perhaps you want to add some in
>> [6/5]?
>
> I'll be working on this. I spent some time looking around for example
> tests for format. Are there any that I should pay special attention to?
> (I'm looking at t7004 mostly right now).
By the way, running t7030 seems to reveal a segfault introduced by
this 5-patch series.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #08; Tue, 27)
From: Junio C Hamano @ 2016-10-03 20:49 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79ka8dO1AHJftKAqD6LvxJSP+8yGGa7Citcdxxrnc5DMeYg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> I am looking at the tip of jc/attr-more and for a minimum
> thread safety we'd want to change the call sites to be aware of the
> threads, i.e. instead of doing
>
static struct git_attr_check *check;
> if (!check)
> check = git_attr_check_initl("crlf", "ident",
> "filter", "eol", "text",
> NULL);
>
> We'd rather call
>
> struct git_attr_check *check;
> check = git_attr_check_lookup_or_initl_threadsafe(
> "crlf", "ident", "filter", "eol", "text", NULL);
> if (!git_check_attr(path, check)) {
> ...
I actually am hoping that the "static" can be kept so that we can
minimize the cost of interning these symbols into struct git_attr.
The initialization would thus become something like:
static struct git_attr_check *check;
git_attr_check_initl(&check, "crlf", "ident", ..., NULL);
The structure will have an array of git_attr[], once interned will
be shared and used by everybody. _initl() will need to make sure
that the "check" pointer is updated atomically so that multiple
people racing to initialize this variable will not step on each
others' toes.
Then the use site would do something like
const char *result[... some size ...];
git_check_attr(path, check, result[]);
to receive result regardless of anybody else who is using the same
"check" structure to ask the same question (for different paths).
^ permalink raw reply
* [PATCH v2 4/5] tmp-objdir: put quarantine information in the environment
From: Jeff King @ 2016-10-03 20:49 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
The presence of the GIT_QUARANTINE_PATH variable lets any
called programs know that they're operating in a temporary
object directory (and where that directory is).
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 1 +
tmp-objdir.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/cache.h b/cache.h
index 9866e46..fcab658 100644
--- a/cache.h
+++ b/cache.h
@@ -433,6 +433,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_GLOB_PATHSPECS_ENVIRONMENT "GIT_GLOB_PATHSPECS"
#define GIT_NOGLOB_PATHSPECS_ENVIRONMENT "GIT_NOGLOB_PATHSPECS"
#define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
+#define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
/*
* This environment variable is expected to contain a boolean indicating
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 9443868..780af8e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -147,6 +147,8 @@ struct tmp_objdir *tmp_objdir_create(void)
env_append(&t->env, ALTERNATE_DB_ENVIRONMENT,
absolute_path(get_object_directory()));
env_replace(&t->env, DB_ENVIRONMENT, absolute_path(t->path.buf));
+ env_replace(&t->env, GIT_QUARANTINE_ENVIRONMENT,
+ absolute_path(t->path.buf));
return t;
}
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH v2 5/5] tmp-objdir: do not migrate files starting with '.'
From: Jeff King @ 2016-10-03 20:49 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
This avoids "." and "..", as we already do, but also leaves
room for index-pack to store extra data in the quarantine
area (e.g., for passing back any analysis to be read by the
pre-receive hook).
Signed-off-by: Jeff King <peff@peff.net>
---
tmp-objdir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 780af8e..64435f2 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -188,7 +188,7 @@ static int read_dir_paths(struct string_list *out, const char *path)
return -1;
while ((de = readdir(dh)))
- if (!is_dot_or_dotdot(de->d_name))
+ if (de->d_name[0] != '.')
string_list_append(out, de->d_name);
closedir(dh);
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH v2 3/5] receive-pack: quarantine objects until pre-receive accepts
From: Jeff King @ 2016-10-03 20:49 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
When a client pushes objects to us, index-pack checks the
objects themselves and then installs them into place. If we
then reject the push due to a pre-receive hook, we cannot
just delete the packfile; other processes may be depending
on it. We have to do a normal reachability check at this
point via `git gc`.
But such objects may hang around for weeks due to the
gc.pruneExpire grace period. And worse, during that time
they may be exploded from the pack into inefficient loose
objects.
Instead, this patch teaches receive-pack to put the new
objects into a "quarantine" temporary directory. We make
these objects available to the connectivity check and to the
pre-receive hook, and then install them into place only if
it is successful (and otherwise remove them as tempfiles).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/receive-pack.c | 41 ++++++++++++++++++++++++++++++++++++++++-
t/t5547-push-quarantine.sh | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 1 deletion(-)
create mode 100755 t/t5547-push-quarantine.sh
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 896b16f..267d320 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -20,6 +20,7 @@
#include "gpg-interface.h"
#include "sigchain.h"
#include "fsck.h"
+#include "tmp-objdir.h"
static const char * const receive_pack_usage[] = {
N_("git receive-pack <git-dir>"),
@@ -86,6 +87,8 @@ static enum {
} use_keepalive;
static int keepalive_in_sec = 5;
+static struct tmp_objdir *tmp_objdir;
+
static enum deny_action parse_deny_action(const char *var, const char *value)
{
if (value) {
@@ -663,6 +666,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed,
} else
argv_array_pushf(&proc.env_array, "GIT_PUSH_OPTION_COUNT");
+ if (tmp_objdir)
+ argv_array_pushv(&proc.env_array, tmp_objdir_env(tmp_objdir));
+
if (use_sideband) {
memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
@@ -762,6 +768,7 @@ static int run_update_hook(struct command *cmd)
proc.stdout_to_stderr = 1;
proc.err = use_sideband ? -1 : 0;
proc.argv = argv;
+ proc.env = tmp_objdir_env(tmp_objdir);
code = start_command(&proc);
if (code)
@@ -833,6 +840,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
!delayed_reachability_test(si, i))
sha1_array_append(&extra, si->shallow->sha1[i]);
+ opt.env = tmp_objdir_env(tmp_objdir);
setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
if (check_connected(command_singleton_iterator, cmd, &opt)) {
rollback_lock_file(&shallow_lock);
@@ -1240,12 +1248,17 @@ static void set_connectivity_errors(struct command *commands,
for (cmd = commands; cmd; cmd = cmd->next) {
struct command *singleton = cmd;
+ struct check_connected_options opt = CHECK_CONNECTED_INIT;
+
if (shallow_update && si->shallow_ref[cmd->index])
/* to be checked in update_shallow_ref() */
continue;
+
+ opt.env = tmp_objdir_env(tmp_objdir);
if (!check_connected(command_singleton_iterator, &singleton,
- NULL))
+ &opt))
continue;
+
cmd->error_string = "missing necessary objects";
}
}
@@ -1428,6 +1441,7 @@ static void execute_commands(struct command *commands,
data.si = si;
opt.err_fd = err_fd;
opt.progress = err_fd && !quiet;
+ opt.env = tmp_objdir_env(tmp_objdir);
if (check_connected(iterate_receive_command_list, &data, &opt))
set_connectivity_errors(commands, si);
@@ -1444,6 +1458,19 @@ static void execute_commands(struct command *commands,
return;
}
+ /*
+ * Now we'll start writing out refs, which means the objects need
+ * to be in their final positions so that other processes can see them.
+ */
+ if (tmp_objdir_migrate(tmp_objdir) < 0) {
+ for (cmd = commands; cmd; cmd = cmd->next) {
+ if (!cmd->error_string)
+ cmd->error_string = "unable to migrate objects to permanent storage";
+ }
+ return;
+ }
+ tmp_objdir = NULL;
+
check_aliased_updates(commands);
free(head_name_to_free);
@@ -1639,6 +1666,18 @@ static const char *unpack(int err_fd, struct shallow_info *si)
argv_array_push(&child.args, alt_shallow_file);
}
+ tmp_objdir = tmp_objdir_create();
+ if (!tmp_objdir)
+ return "unable to create temporary object directory";
+ child.env = tmp_objdir_env(tmp_objdir);
+
+ /*
+ * Normally we just pass the tmp_objdir environment to the child
+ * processes that do the heavy lifting, but we may need to see these
+ * objects ourselves to set up shallow information.
+ */
+ tmp_objdir_add_as_alternate(tmp_objdir);
+
if (ntohl(hdr.hdr_entries) < unpack_limit) {
argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
if (quiet)
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
new file mode 100755
index 0000000..1e5d32d
--- /dev/null
+++ b/t/t5547-push-quarantine.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='check quarantine of objects during push'
+. ./test-lib.sh
+
+test_expect_success 'create picky dest repo' '
+ git init --bare dest.git &&
+ write_script dest.git/hooks/pre-receive <<-\EOF
+ while read old new ref; do
+ test "$(git log -1 --format=%s $new)" = reject && exit 1
+ done
+ exit 0
+ EOF
+'
+
+test_expect_success 'accepted objects work' '
+ test_commit ok &&
+ git push dest.git HEAD &&
+ commit=$(git rev-parse HEAD) &&
+ git --git-dir=dest.git cat-file commit $commit
+'
+
+test_expect_success 'rejected objects are not installed' '
+ test_commit reject &&
+ commit=$(git rev-parse HEAD) &&
+ test_must_fail git push dest.git reject &&
+ test_must_fail git --git-dir=dest.git cat-file commit $commit
+'
+
+test_expect_success 'rejected objects are removed' '
+ echo "incoming-*" >expect &&
+ (cd dest.git/objects && echo incoming-*) >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH v2 2/5] tmp-objdir: introduce API for temporary object directories
From: Jeff King @ 2016-10-03 20:49 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
Once objects are added to the object database by a process,
they cannot easily be deleted, as we don't know what other
processes may have started referencing them. We have to
clean them up with git-gc, which will apply the usual
reachability and grace-period checks.
This patch provides an alternative: it helps callers create
a temporary directory inside the object directory, and a
temporary environment which can be passed to sub-programs to
ask them to write there (the original object directory
remains accessible as an alternate of the temporary one).
See tmp-objdir.h for details on the API.
Signed-off-by: Jeff King <peff@peff.net>
---
Makefile | 1 +
tmp-objdir.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tmp-objdir.h | 54 ++++++++++++
3 files changed, 328 insertions(+)
create mode 100644 tmp-objdir.c
create mode 100644 tmp-objdir.h
diff --git a/Makefile b/Makefile
index 1aad150..4e3becb 100644
--- a/Makefile
+++ b/Makefile
@@ -831,6 +831,7 @@ LIB_OBJS += submodule-config.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += tempfile.o
+LIB_OBJS += tmp-objdir.o
LIB_OBJS += trace.o
LIB_OBJS += trailer.o
LIB_OBJS += transport.o
diff --git a/tmp-objdir.c b/tmp-objdir.c
new file mode 100644
index 0000000..9443868
--- /dev/null
+++ b/tmp-objdir.c
@@ -0,0 +1,273 @@
+#include "cache.h"
+#include "tmp-objdir.h"
+#include "dir.h"
+#include "sigchain.h"
+#include "string-list.h"
+#include "strbuf.h"
+#include "argv-array.h"
+
+struct tmp_objdir {
+ struct strbuf path;
+ struct argv_array env;
+};
+
+/*
+ * Allow only one tmp_objdir at a time in a running process, which simplifies
+ * our signal/atexit cleanup routines. It's doubtful callers will ever need
+ * more than one, and we can expand later if so. You can have many such
+ * tmp_objdirs simultaneously in many processes, of course.
+ */
+static struct tmp_objdir *the_tmp_objdir;
+
+static void tmp_objdir_free(struct tmp_objdir *t)
+{
+ strbuf_release(&t->path);
+ argv_array_clear(&t->env);
+ free(t);
+}
+
+static int tmp_objdir_destroy_1(struct tmp_objdir *t, int on_signal)
+{
+ int err;
+
+ if (!t)
+ return 0;
+
+ if (t == the_tmp_objdir)
+ the_tmp_objdir = NULL;
+
+ /*
+ * This may use malloc via strbuf_grow(), but we should
+ * have pre-grown t->path sufficiently so that this
+ * doesn't happen in practice.
+ */
+ err = remove_dir_recursively(&t->path, 0);
+
+ /*
+ * When we are cleaning up due to a signal, we won't bother
+ * freeing memory; it may cause a deadlock if the signal
+ * arrived while libc's allocator lock is held.
+ */
+ if (!on_signal)
+ tmp_objdir_free(t);
+ return err;
+}
+
+int tmp_objdir_destroy(struct tmp_objdir *t)
+{
+ return tmp_objdir_destroy_1(t, 0);
+}
+
+static void remove_tmp_objdir(void)
+{
+ tmp_objdir_destroy(the_tmp_objdir);
+}
+
+static void remove_tmp_objdir_on_signal(int signo)
+{
+ tmp_objdir_destroy_1(the_tmp_objdir, 1);
+ sigchain_pop(signo);
+ raise(signo);
+}
+
+/*
+ * These env_* functions are for setting up the child environment; the
+ * "replace" variant overrides the value of any existing variable with that
+ * "key". The "append" variant puts our new value at the end of a list,
+ * separated by PATH_SEP (which is what separate values in
+ * GIT_ALTERNATE_OBJECT_DIRECTORIES).
+ */
+static void env_append(struct argv_array *env, const char *key, const char *val)
+{
+ const char *old = getenv(key);
+
+ if (!old)
+ argv_array_pushf(env, "%s=%s", key, val);
+ else
+ argv_array_pushf(env, "%s=%s%c%s", key, old, PATH_SEP, val);
+}
+
+static void env_replace(struct argv_array *env, const char *key, const char *val)
+{
+ argv_array_pushf(env, "%s=%s", key, val);
+}
+
+static int setup_tmp_objdir(const char *root)
+{
+ char *path;
+ int ret = 0;
+
+ path = xstrfmt("%s/pack", root);
+ ret = mkdir(path, 0777);
+ free(path);
+
+ return ret;
+}
+
+struct tmp_objdir *tmp_objdir_create(void)
+{
+ static int installed_handlers;
+ struct tmp_objdir *t;
+
+ if (the_tmp_objdir)
+ die("BUG: only one tmp_objdir can be used at a time");
+
+ t = xmalloc(sizeof(*t));
+ strbuf_init(&t->path, 0);
+ argv_array_init(&t->env);
+
+ strbuf_addf(&t->path, "%s/incoming-XXXXXX", get_object_directory());
+
+ /*
+ * Grow the strbuf beyond any filename we expect to be placed in it.
+ * If tmp_objdir_destroy() is called by a signal handler, then
+ * we should be able to use the strbuf to remove files without
+ * having to call malloc.
+ */
+ strbuf_grow(&t->path, 1024);
+
+ if (!mkdtemp(t->path.buf)) {
+ /* free, not destroy, as we never touched the filesystem */
+ tmp_objdir_free(t);
+ return NULL;
+ }
+
+ the_tmp_objdir = t;
+ if (!installed_handlers) {
+ atexit(remove_tmp_objdir);
+ sigchain_push_common(remove_tmp_objdir_on_signal);
+ installed_handlers++;
+ }
+
+ if (setup_tmp_objdir(t->path.buf)) {
+ tmp_objdir_destroy(t);
+ return NULL;
+ }
+
+ env_append(&t->env, ALTERNATE_DB_ENVIRONMENT,
+ absolute_path(get_object_directory()));
+ env_replace(&t->env, DB_ENVIRONMENT, absolute_path(t->path.buf));
+
+ return t;
+}
+
+/*
+ * Make sure we copy packfiles and their associated metafiles in the correct
+ * order. All of these ends_with checks are slightly expensive to do in
+ * the midst of a sorting routine, but in practice it shouldn't matter.
+ * We will have a relatively small number of packfiles to order, and loose
+ * objects exit early in the first line.
+ */
+static int pack_copy_priority(const char *name)
+{
+ if (!starts_with(name, "pack"))
+ return 0;
+ if (ends_with(name, ".keep"))
+ return 1;
+ if (ends_with(name, ".pack"))
+ return 2;
+ if (ends_with(name, ".idx"))
+ return 3;
+ return 4;
+}
+
+static int pack_copy_cmp(const char *a, const char *b)
+{
+ return pack_copy_priority(a) - pack_copy_priority(b);
+}
+
+static int read_dir_paths(struct string_list *out, const char *path)
+{
+ DIR *dh;
+ struct dirent *de;
+
+ dh = opendir(path);
+ if (!dh)
+ return -1;
+
+ while ((de = readdir(dh)))
+ if (!is_dot_or_dotdot(de->d_name))
+ string_list_append(out, de->d_name);
+
+ closedir(dh);
+ return 0;
+}
+
+static int migrate_paths(struct strbuf *src, struct strbuf *dst);
+
+static int migrate_one(struct strbuf *src, struct strbuf *dst)
+{
+ struct stat st;
+
+ if (stat(src->buf, &st) < 0)
+ return -1;
+ if (S_ISDIR(st.st_mode)) {
+ if (!mkdir(dst->buf, 0777)) {
+ if (adjust_shared_perm(dst->buf))
+ return -1;
+ } else if (errno != EEXIST)
+ return -1;
+ return migrate_paths(src, dst);
+ }
+ return finalize_object_file(src->buf, dst->buf);
+}
+
+static int migrate_paths(struct strbuf *src, struct strbuf *dst)
+{
+ size_t src_len = src->len, dst_len = dst->len;
+ struct string_list paths = STRING_LIST_INIT_DUP;
+ int i;
+ int ret = 0;
+
+ if (read_dir_paths(&paths, src->buf) < 0)
+ return -1;
+ paths.cmp = pack_copy_cmp;
+ string_list_sort(&paths);
+
+ for (i = 0; i < paths.nr; i++) {
+ const char *name = paths.items[i].string;
+
+ strbuf_addf(src, "/%s", name);
+ strbuf_addf(dst, "/%s", name);
+
+ ret |= migrate_one(src, dst);
+
+ strbuf_setlen(src, src_len);
+ strbuf_setlen(dst, dst_len);
+ }
+
+ string_list_clear(&paths, 0);
+ return ret;
+}
+
+int tmp_objdir_migrate(struct tmp_objdir *t)
+{
+ struct strbuf src = STRBUF_INIT, dst = STRBUF_INIT;
+ int ret;
+
+ if (!t)
+ return 0;
+
+ strbuf_addbuf(&src, &t->path);
+ strbuf_addstr(&dst, get_object_directory());
+
+ ret = migrate_paths(&src, &dst);
+
+ strbuf_release(&src);
+ strbuf_release(&dst);
+
+ tmp_objdir_destroy(t);
+ return ret;
+}
+
+const char **tmp_objdir_env(const struct tmp_objdir *t)
+{
+ if (!t)
+ return NULL;
+ return t->env.argv;
+}
+
+void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
+{
+ add_to_alternates_memory(t->path.buf);
+}
diff --git a/tmp-objdir.h b/tmp-objdir.h
new file mode 100644
index 0000000..b1e45b4
--- /dev/null
+++ b/tmp-objdir.h
@@ -0,0 +1,54 @@
+#ifndef TMP_OBJDIR_H
+#define TMP_OBJDIR_H
+
+/*
+ * This API allows you to create a temporary object directory, advertise it to
+ * sub-processes via GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES,
+ * and then either migrate its object into the main object directory, or remove
+ * it. The library handles unexpected signal/exit death by cleaning up the
+ * temporary directory.
+ *
+ * Example:
+ *
+ * struct tmp_objdir *t = tmp_objdir_create();
+ * if (!run_command_v_opt_cd_env(cmd, 0, NULL, tmp_objdir_env(t)) &&
+ * !tmp_objdir_migrate(t))
+ * printf("success!\n");
+ * else
+ * die("failed...tmp_objdir will clean up for us");
+ *
+ */
+
+struct tmp_objdir;
+
+/*
+ * Create a new temporary object directory; returns NULL on failure.
+ */
+struct tmp_objdir *tmp_objdir_create(void);
+
+/*
+ * Return a list of environment strings, suitable for use with
+ * child_process.env, that can be passed to child programs to make use of the
+ * temporary object directory.
+ */
+const char **tmp_objdir_env(const struct tmp_objdir *);
+
+/*
+ * Finalize a temporary object directory by migrating its objects into the main
+ * object database, removing the temporary directory, and freeing any
+ * associated resources.
+ */
+int tmp_objdir_migrate(struct tmp_objdir *);
+
+/*
+ * Destroy a temporary object directory, discarding any objects it contains.
+ */
+int tmp_objdir_destroy(struct tmp_objdir *);
+
+/*
+ * Add the temporary object directory as an alternate object store in the
+ * current process.
+ */
+void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
+
+#endif /* TMP_OBJDIR_H */
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH v2 1/5] check_connected: accept an env argument
From: Jeff King @ 2016-10-03 20:49 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20161003204851.klwspo6agykx6s3q@sigill.intra.peff.net>
This lets callers influence the environment seen by
rev-list, which will be useful when we start providing
quarantined objects.
Signed-off-by: Jeff King <peff@peff.net>
---
connected.c | 1 +
connected.h | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/connected.c b/connected.c
index 8e3e4b1..136c2ac 100644
--- a/connected.c
+++ b/connected.c
@@ -63,6 +63,7 @@ int check_connected(sha1_iterate_fn fn, void *cb_data,
_("Checking connectivity"));
rev_list.git_cmd = 1;
+ rev_list.env = opt->env;
rev_list.in = -1;
rev_list.no_stdout = 1;
if (opt->err_fd)
diff --git a/connected.h b/connected.h
index afa48cc..4ca325f 100644
--- a/connected.h
+++ b/connected.h
@@ -33,6 +33,11 @@ struct check_connected_options {
/* If non-zero, show progress as we traverse the objects. */
int progress;
+
+ /*
+ * Insert these variables into the environment of the child process.
+ */
+ const char **env;
};
#define CHECK_CONNECTED_INIT { 0 }
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH v2 0/5] receive-pack: quarantine pushed objects
From: Jeff King @ 2016-10-03 20:48 UTC (permalink / raw)
To: git; +Cc: David Turner
In-Reply-To: <20160930193533.ynbepaago6oycg5t@sigill.intra.peff.net>
Here's a v2; the original was at:
http://public-inbox.org/git/20160930193533.ynbepaago6oycg5t@sigill.intra.peff.net/
which contains the rationale. The required alternate-objects patches
(both the "allow recursive relative" one, and the helper to add an
internal alt-odb) have been pushed into their own series, that I posted
here:
http://public-inbox.org/git/20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net/
This needs to be applied on top.
Other than that, v2 just fixes the minor issues raised on the list
(missing "static", and some clarifying comments).
[1/5]: check_connected: accept an env argument
[2/5]: tmp-objdir: introduce API for temporary object directories
[3/5]: receive-pack: quarantine objects until pre-receive accepts
[4/5]: tmp-objdir: put quarantine information in the environment
[5/5]: tmp-objdir: do not migrate files starting with '.'
Makefile | 1 +
builtin/receive-pack.c | 41 ++++++-
cache.h | 1 +
connected.c | 1 +
connected.h | 5 +
t/t5547-push-quarantine.sh | 36 ++++++
tmp-objdir.c | 275 +++++++++++++++++++++++++++++++++++++++++++++
tmp-objdir.h | 54 +++++++++
8 files changed, 413 insertions(+), 1 deletion(-)
create mode 100755 t/t5547-push-quarantine.sh
create mode 100644 tmp-objdir.c
create mode 100644 tmp-objdir.h
^ permalink raw reply
* [PATCH 18/18] alternates: use fspathcmp to detect duplicates
From: Jeff King @ 2016-10-03 20:36 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
On a case-insensitive filesystem, we should realize that
"a/objects" and "A/objects" are the same path. We already
use fspathcmp() to check against the main object directory,
but until recently we couldn't use it for comparing against
other alternates (because their paths were not
NUL-terminated strings). But now we can, so let's do so.
Note that we also need to adjust count-objects to load the
config, so that it can see the setting of core.ignorecase
(this is required by the test, but is also a general bugfix
for users of count-objects).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/count-objects.c | 2 ++
sha1_file.c | 2 +-
t/t5613-info-alternate.sh | 17 +++++++++++++++++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a700409..a04b4f2 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -97,6 +97,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+ git_config(git_default_config, NULL);
+
argc = parse_options(argc, argv, prefix, opts, count_objects_usage, 0);
/* we do not take arguments other than flags for now */
if (argc)
diff --git a/sha1_file.c b/sha1_file.c
index b514167..b05ec9c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -260,7 +260,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
* thing twice, or object directory itself.
*/
for (alt = alt_odb_list; alt; alt = alt->next) {
- if (!strcmp(path->buf, alt->path))
+ if (!fspathcmp(path->buf, alt->path))
return 0;
}
if (!fspathcmp(path->buf, normalized_objdir))
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 76525a0..926fe14 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -116,4 +116,21 @@ test_expect_success 'relative duplicates are eliminated' '
test_cmp expect actual.alternates
'
+test_expect_success CASE_INSENSITIVE_FS 'dup finding can be case-insensitive' '
+ git init --bare insensitive.git &&
+ # the previous entry for "A" will have used uppercase
+ cat >insensitive.git/objects/info/alternates <<-\EOF &&
+ ../../C/.git/objects
+ ../../a/.git/objects
+ EOF
+ cat >expect <<-EOF &&
+ alternate: $(pwd)/C/.git/objects
+ alternate: $(pwd)/B/.git/objects
+ alternate: $(pwd)/A/.git/objects
+ EOF
+ git -C insensitive.git count-objects -v >actual &&
+ grep ^alternate: actual >actual.alternates &&
+ test_cmp expect actual.alternates
+'
+
test_done
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 17/18] sha1_file: always allow relative paths to alternates
From: Jeff King @ 2016-10-03 20:36 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
We recursively expand alternates repositories, so that if A
borrows from B which borrows from C, A can see all objects.
For the root object database, we allow relative paths, so A
can point to B as "../B/objects". However, we currently do
not allow relative paths when recursing, so B must use an
absolute path to reach C.
That is an ancient protection from c2f493a (Transitively
read alternatives, 2006-05-07) that tries to avoid adding
the same alternate through two different paths. Since
5bdf0a8 (sha1_file: normalize alt_odb path before comparing
and storing, 2011-09-07), we use a normalized absolute path
for each alt_odb entry.
This means that in most cases the protection is no longer
necessary; we will detect the duplicate no matter how we got
there (but see below). And it's a good idea to get rid of
it, as it creates an unnecessary complication when setting
up recursive alternates (B has to know that A is going to
borrow from it and make sure to use an absolute path).
Note that our normalization doesn't actually look at the
filesystem, so it can still be fooled by crossing symbolic
links. But that's also true of absolute paths, so it's not a
good reason to disallow only relative paths (it's
potentially a reason to switch to real_path(), but that's a
separate and non-trivial change).
We adjust the test script here to demonstrate that this now
works, and add new tests to show that the normalization does
indeed suppress duplicates.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 7 +------
t/t5613-info-alternate.sh | 24 ++++++++++++++++++++++--
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 80a3333..b514167 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -354,12 +354,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
const char *entry = entries.items[i].string;
if (entry[0] == '\0' || entry[0] == '#')
continue;
- if (!is_absolute_path(entry) && depth) {
- error("%s: ignoring relative alternate object store %s",
- relative_base, entry);
- } else {
- link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
- }
+ link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
}
string_list_clear(&entries, 0);
free(alt_copy);
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 74f6770..76525a0 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -92,8 +92,28 @@ test_expect_success 'that relative alternate is possible for current dir' '
git fsck
'
-test_expect_success 'that relative alternate is only possible for current dir' '
- test_must_fail git -C D fsck
+test_expect_success 'that relative alternate is recursive' '
+ git -C D fsck
+'
+
+# we can reach "A" from our new repo both directly, and via "C".
+# The deep/subdir is there to make sure we are not doing a stupid
+# pure-text comparison of the alternate names.
+test_expect_success 'relative duplicates are eliminated' '
+ mkdir -p deep/subdir &&
+ git init --bare deep/subdir/duplicate.git &&
+ cat >deep/subdir/duplicate.git/objects/info/alternates <<-\EOF &&
+ ../../../../C/.git/objects
+ ../../../../A/.git/objects
+ EOF
+ cat >expect <<-EOF &&
+ alternate: $(pwd)/C/.git/objects
+ alternate: $(pwd)/B/.git/objects
+ alternate: $(pwd)/A/.git/objects
+ EOF
+ git -C deep/subdir/duplicate.git count-objects -v >actual &&
+ grep ^alternate: actual >actual.alternates &&
+ test_cmp expect actual.alternates
'
test_done
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 16/18] count-objects: report alternates via verbose mode
From: Jeff King @ 2016-10-03 20:36 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
There's no way to get the list of alternates that git
computes internally; our tests only infer it based on which
objects are available. In addition to testing, knowing this
list may be helpful for somebody debugging their alternates
setup.
Let's add it to the "count-objects -v" output. We could give
it a separate flag, but there's not really any need.
"count-objects -v" is already a debugging catch-all for the
object database, its output is easily extensible to new data
items, and printing the alternates is not expensive (we
already had to find them to count the objects).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-count-objects.txt | 5 +++++
builtin/count-objects.c | 10 ++++++++++
t/t5613-info-alternate.sh | 10 ++++++++++
3 files changed, 25 insertions(+)
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 2ff3568..cb9b4d2 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -38,6 +38,11 @@ objects nor valid packs
+
size-garbage: disk space consumed by garbage files, in KiB (unless -H is
specified)
++
+alternate: absolute path of alternate object databases; may appear
+multiple times, one line per path. Note that if the path contains
+non-printable characters, it may be surrounded by double-quotes and
+contain C-style backslashed escape sequences.
-H::
--human-readable::
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index ba92919..a700409 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -8,6 +8,7 @@
#include "dir.h"
#include "builtin.h"
#include "parse-options.h"
+#include "quote.h"
static unsigned long garbage;
static off_t size_garbage;
@@ -73,6 +74,14 @@ static int count_cruft(const char *basename, const char *path, void *data)
return 0;
}
+static int print_alternate(struct alternate_object_database *alt, void *data)
+{
+ printf("alternate: ");
+ quote_c_style(alt->path, NULL, stdout, 0);
+ putchar('\n');
+ return 0;
+}
+
static char const * const count_objects_usage[] = {
N_("git count-objects [-v] [-H | --human-readable]"),
NULL
@@ -140,6 +149,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
printf("prune-packable: %lu\n", packed_loose);
printf("garbage: %lu\n", garbage);
printf("size-garbage: %s\n", garbage_buf.buf);
+ foreach_alt_odb(print_alternate, NULL);
strbuf_release(&loose_buf);
strbuf_release(&pack_buf);
strbuf_release(&garbage_buf);
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index b393613..74f6770 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -39,6 +39,16 @@ test_expect_success 'preparing third repository' '
)
'
+test_expect_success 'count-objects shows the alternates' '
+ cat >expect <<-EOF &&
+ alternate: $(pwd)/B/.git/objects
+ alternate: $(pwd)/A/.git/objects
+ EOF
+ git -C C count-objects -v >actual &&
+ grep ^alternate: actual >actual.alternates &&
+ test_cmp expect actual.alternates
+'
+
# Note: These tests depend on the hard-coded value of 5 as "too deep". We start
# the depth at 0 and count links, not repositories, so in a chain like:
#
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 15/18] fill_sha1_file: write into a strbuf
From: Jeff King @ 2016-10-03 20:36 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
It's currently the responsibility of the caller to give
fill_sha1_file() enough bytes to write into, leading them to
manually compute the required lengths. Instead, let's just
write into a strbuf so that it's impossible to get this
wrong.
The alt_odb caller already has a strbuf, so this makes
things strictly simpler. The other caller, sha1_file_name(),
uses a static PATH_MAX buffer and dies when it would
overflow. We can convert this to a static strbuf, which
means our allocation cost is amortized (and as a bonus, we
no longer have to worry about PATH_MAX being too short for
normal use).
This does introduce some small overhead in fill_sha1_file(),
as each strbuf_addchar() will check whether it needs to
grow. However, between the optimization in fec501d
(strbuf_addch: avoid calling strbuf_grow, 2015-04-16) and
the fact that this is not generally called in a tight loop
(after all, the next step is typically to access the file!)
this probably doesn't matter. And even if it did, the right
place to micro-optimize is inside fill_sha1_file(), by
calling a single strbuf_grow() there.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index efc8cee..80a3333 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -172,36 +172,28 @@ enum scld_error safe_create_leading_directories_const(const char *path)
return result;
}
-static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
+static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
{
int i;
for (i = 0; i < 20; i++) {
static char hex[] = "0123456789abcdef";
unsigned int val = sha1[i];
- *pathbuf++ = hex[val >> 4];
- *pathbuf++ = hex[val & 0xf];
+ strbuf_addch(buf, hex[val >> 4]);
+ strbuf_addch(buf, hex[val & 0xf]);
if (!i)
- *pathbuf++ = '/';
+ strbuf_addch(buf, '/');
}
- *pathbuf = '\0';
}
const char *sha1_file_name(const unsigned char *sha1)
{
- static char buf[PATH_MAX];
- const char *objdir;
- int len;
+ static struct strbuf buf = STRBUF_INIT;
- objdir = get_object_directory();
- len = strlen(objdir);
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "%s/", get_object_directory());
- /* '/' + sha1(2) + '/' + sha1(38) + '\0' */
- if (len + 43 > PATH_MAX)
- die("insanely long object directory %s", objdir);
- memcpy(buf, objdir, len);
- buf[len] = '/';
- fill_sha1_path(buf + len + 1, sha1);
- return buf;
+ fill_sha1_path(&buf, sha1);
+ return buf.buf;
}
struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
@@ -213,14 +205,8 @@ struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
static const char *alt_sha1_path(struct alternate_object_database *alt,
const unsigned char *sha1)
{
- /* hex sha1 plus internal "/" */
- size_t len = GIT_SHA1_HEXSZ + 1;
struct strbuf *buf = alt_scratch_buf(alt);
-
- strbuf_grow(buf, len);
- fill_sha1_path(buf->buf + buf->len, sha1);
- strbuf_setlen(buf, buf->len + len);
-
+ fill_sha1_path(buf, sha1);
return buf->buf;
}
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 14/18] alternates: store scratch buffer as strbuf
From: Jeff King @ 2016-10-03 20:36 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
We pre-size the scratch buffer to hold a loose object
filename of the form "xx/yyyy...", which leads to allocation
code that is hard to verify. We have to use some magic
numbers during the initial allocation, and then writers must
blindly assume that the buffer is big enough. Using a strbuf
makes it more clear that we cannot overflow.
Unfortunately, we do still need some magic numbers to grow
our strbuf before calling fill_sha1_path(), but the strbuf
growth is much closer to the point of use. This makes it
easier to see that it's correct, and opens the possibility
of pushing it even further down if fill_sha1_path() learns
to work on strbufs.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 13 +++++++++++--
sha1_file.c | 28 ++++++++++++++++++----------
sha1_name.c | 9 +++------
3 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/cache.h b/cache.h
index e1996c5..9866e46 100644
--- a/cache.h
+++ b/cache.h
@@ -1383,8 +1383,9 @@ extern void remove_scheduled_dirs(void);
extern struct alternate_object_database {
struct alternate_object_database *next;
- char *name;
- char *scratch;
+ /* see alt_scratch_buf() */
+ struct strbuf scratch;
+ size_t base_len;
char path[FLEX_ARRAY];
} *alt_odb_list;
@@ -1413,6 +1414,14 @@ extern void add_to_alternates_file(const char *dir);
*/
extern void add_to_alternates_memory(const char *dir);
+/*
+ * Returns a scratch strbuf pre-filled with the alternate object directory,
+ * including a trailing slash, which can be used to access paths in the
+ * alternate. Always use this over direct access to alt->scratch, as it
+ * cleans up any previous use of the scratch buffer.
+ */
+extern struct strbuf *alt_scratch_buf(struct alternate_object_database *alt);
+
struct pack_window {
struct pack_window *next;
unsigned char *base;
diff --git a/sha1_file.c b/sha1_file.c
index c6308c1..efc8cee 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -204,11 +204,24 @@ const char *sha1_file_name(const unsigned char *sha1)
return buf;
}
+struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
+{
+ strbuf_setlen(&alt->scratch, alt->base_len);
+ return &alt->scratch;
+}
+
static const char *alt_sha1_path(struct alternate_object_database *alt,
const unsigned char *sha1)
{
- fill_sha1_path(alt->name, sha1);
- return alt->scratch;
+ /* hex sha1 plus internal "/" */
+ size_t len = GIT_SHA1_HEXSZ + 1;
+ struct strbuf *buf = alt_scratch_buf(alt);
+
+ strbuf_grow(buf, len);
+ fill_sha1_path(buf->buf + buf->len, sha1);
+ strbuf_setlen(buf, buf->len + len);
+
+ return buf->buf;
}
/*
@@ -396,16 +409,11 @@ void read_info_alternates(const char * relative_base, int depth)
struct alternate_object_database *alloc_alt_odb(const char *dir)
{
struct alternate_object_database *ent;
- size_t dirlen = strlen(dir);
- size_t entlen;
- entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
FLEX_ALLOC_STR(ent, path, dir);
- ent->scratch = xmalloc(entlen);
- xsnprintf(ent->scratch, entlen, "%s/", dir);
-
- ent->name = ent->scratch + dirlen + 1;
- ent->scratch[dirlen] = '/';
+ strbuf_init(&ent->scratch, 0);
+ strbuf_addf(&ent->scratch, "%s/", dir);
+ ent->base_len = ent->scratch.len;
return ent;
}
diff --git a/sha1_name.c b/sha1_name.c
index 770ea4f..defbb3e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -92,15 +92,12 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
+ struct strbuf *buf = alt_scratch_buf(alt);
struct dirent *de;
DIR *dir;
- /*
- * every alt_odb struct has 42 extra bytes after the base
- * for exactly this purpose
- */
- xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
- dir = opendir(alt->scratch);
+ strbuf_addf(buf, "%.2s/", hex_pfx);
+ dir = opendir(buf->buf);
if (!dir)
continue;
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 13/18] fill_sha1_file: write "boring" characters
From: Jeff King @ 2016-10-03 20:35 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
This function forms a sha1 as "xx/yyyy...", but skips over
the slot for the slash rather than writing it, leaving it to
the caller to do so. It also does not bother to put in a
trailing NUL, even though every caller would want it (we're
forming a path which by definition is not a directory, so
the only thing to do with it is feed it to a system call).
Let's make the lives of our callers easier by just writing
out the internal "/" and the NUL.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 70c3e2f..c6308c1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -178,10 +178,12 @@ static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
for (i = 0; i < 20; i++) {
static char hex[] = "0123456789abcdef";
unsigned int val = sha1[i];
- char *pos = pathbuf + i*2 + (i > 0);
- *pos++ = hex[val >> 4];
- *pos = hex[val & 0xf];
+ *pathbuf++ = hex[val >> 4];
+ *pathbuf++ = hex[val & 0xf];
+ if (!i)
+ *pathbuf++ = '/';
}
+ *pathbuf = '\0';
}
const char *sha1_file_name(const unsigned char *sha1)
@@ -198,8 +200,6 @@ const char *sha1_file_name(const unsigned char *sha1)
die("insanely long object directory %s", objdir);
memcpy(buf, objdir, len);
buf[len] = '/';
- buf[len+3] = '/';
- buf[len+42] = '\0';
fill_sha1_path(buf + len + 1, sha1);
return buf;
}
@@ -406,8 +406,6 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
ent->name = ent->scratch + dirlen + 1;
ent->scratch[dirlen] = '/';
- ent->scratch[dirlen + 3] = '/';
- ent->scratch[entlen-1] = 0;
return ent;
}
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 12/18] alternates: use a separate scratch space
From: Jeff King @ 2016-10-03 20:35 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
The alternate_object_database struct uses a single buffer
both for storing the path to the alternate, and as a scratch
buffer for forming object names. This is efficient (since
otherwise we'd end up storing the path twice), but it makes
life hard for callers who just want to know the path to the
alternate. They have to remember to stop reading after
"alt->name - alt->base" bytes, and to subtract one for the
trailing '/'.
It would be much simpler if they could simply access a
NUL-terminated path string. We could encapsulate this in a
function which puts a NUL in the scratch buffer and returns
the string, but that opens up questions about the lifetime
of the result. The first time another caller uses the
alternate, the scratch buffer may get other data tacked onto
it.
Let's instead just store the root path separately from the
scratch buffer. There aren't enough alternates being stored
for the duplicated data to matter for performance, and this
keeps things simple and safe for the callers.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/fsck.c | 10 ++--------
builtin/submodule--helper.c | 11 +++--------
cache.h | 5 ++++-
sha1_file.c | 28 ++++++++++++----------------
sha1_name.c | 3 ++-
transport.c | 4 +---
6 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 055dfdc..f01b81e 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -644,14 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
fsck_object_dir(get_object_directory());
prepare_alt_odb();
- for (alt = alt_odb_list; alt; alt = alt->next) {
- /* directory name, minus trailing slash */
- size_t namelen = alt->name - alt->base - 1;
- struct strbuf name = STRBUF_INIT;
- strbuf_add(&name, alt->base, namelen);
- fsck_object_dir(name.buf);
- strbuf_release(&name);
- }
+ for (alt = alt_odb_list; alt; alt = alt->next)
+ fsck_object_dir(alt->path);
}
if (check_full) {
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e3fdc0a..fd72c90 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -492,20 +492,16 @@ static int add_possible_reference_from_superproject(
{
struct submodule_alternate_setup *sas = sas_cb;
- /* directory name, minus trailing slash */
- size_t namelen = alt->name - alt->base - 1;
- struct strbuf name = STRBUF_INIT;
- strbuf_add(&name, alt->base, namelen);
-
/*
* If the alternate object store is another repository, try the
* standard layout with .git/modules/<name>/objects
*/
- if (ends_with(name.buf, ".git/objects")) {
+ if (ends_with(alt->path, ".git/objects")) {
char *sm_alternate;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
- strbuf_add(&sb, name.buf, name.len - strlen("objects"));
+ strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects"));
+
/*
* We need to end the new path with '/' to mark it as a dir,
* otherwise a submodule name containing '/' will be broken
@@ -533,7 +529,6 @@ static int add_possible_reference_from_superproject(
strbuf_release(&sb);
}
- strbuf_release(&name);
return 0;
}
diff --git a/cache.h b/cache.h
index d36b2ad..e1996c5 100644
--- a/cache.h
+++ b/cache.h
@@ -1382,8 +1382,11 @@ extern void remove_scheduled_dirs(void);
extern struct alternate_object_database {
struct alternate_object_database *next;
+
char *name;
- char base[FLEX_ARRAY]; /* more */
+ char *scratch;
+
+ char path[FLEX_ARRAY];
} *alt_odb_list;
extern void prepare_alt_odb(void);
extern void read_info_alternates(const char * relative_base, int depth);
diff --git a/sha1_file.c b/sha1_file.c
index ccf59ba..70c3e2f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -208,7 +208,7 @@ static const char *alt_sha1_path(struct alternate_object_database *alt,
const unsigned char *sha1)
{
fill_sha1_path(alt->name, sha1);
- return alt->base;
+ return alt->scratch;
}
/*
@@ -261,8 +261,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
* thing twice, or object directory itself.
*/
for (alt = alt_odb_list; alt; alt = alt->next) {
- if (path->len == alt->name - alt->base - 1 &&
- !memcmp(path->buf, alt->base, path->len))
+ if (!strcmp(path->buf, alt->path))
return 0;
}
if (!fspathcmp(path->buf, normalized_objdir))
@@ -401,13 +400,14 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
size_t entlen;
entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
- ent = xmalloc(st_add(sizeof(*ent), entlen));
- memcpy(ent->base, dir, dirlen);
+ FLEX_ALLOC_STR(ent, path, dir);
+ ent->scratch = xmalloc(entlen);
+ xsnprintf(ent->scratch, entlen, "%s/", dir);
- ent->name = ent->base + dirlen + 1;
- ent->base[dirlen] = '/';
- ent->base[dirlen + 3] = '/';
- ent->base[entlen-1] = 0;
+ ent->name = ent->scratch + dirlen + 1;
+ ent->scratch[dirlen] = '/';
+ ent->scratch[dirlen + 3] = '/';
+ ent->scratch[entlen-1] = 0;
return ent;
}
@@ -1485,11 +1485,8 @@ void prepare_packed_git(void)
return;
prepare_packed_git_one(get_object_directory(), 1);
prepare_alt_odb();
- for (alt = alt_odb_list; alt; alt = alt->next) {
- alt->name[-1] = 0;
- prepare_packed_git_one(alt->base, 0);
- alt->name[-1] = '/';
- }
+ for (alt = alt_odb_list; alt; alt = alt->next)
+ prepare_packed_git_one(alt->path, 0);
rearrange_packed_git();
prepare_packed_git_mru();
prepare_packed_git_run_once = 1;
@@ -3670,8 +3667,7 @@ static int loose_from_alt_odb(struct alternate_object_database *alt,
struct strbuf buf = STRBUF_INIT;
int r;
- /* copy base not including trailing '/' */
- strbuf_add(&buf, alt->base, alt->name - alt->base - 1);
+ strbuf_addstr(&buf, alt->path);
r = for_each_loose_file_in_objdir_buf(&buf,
data->cb, NULL, NULL,
data->data);
diff --git a/sha1_name.c b/sha1_name.c
index 98152a6..770ea4f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -94,12 +94,13 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
struct dirent *de;
DIR *dir;
+
/*
* every alt_odb struct has 42 extra bytes after the base
* for exactly this purpose
*/
xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
- dir = opendir(alt->base);
+ dir = opendir(alt->scratch);
if (!dir)
continue;
diff --git a/transport.c b/transport.c
index 94d6dc3..4bc4eea 100644
--- a/transport.c
+++ b/transport.c
@@ -1084,9 +1084,7 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,
const struct ref *extra;
struct alternate_refs_data *cb = data;
- e->name[-1] = '\0';
- other = xstrdup(real_path(e->base));
- e->name[-1] = '/';
+ other = xstrdup(real_path(e->path));
len = strlen(other);
while (other[len-1] == '/')
--
2.10.0.618.g82cc264
^ permalink raw reply related
* [PATCH 11/18] alternates: encapsulate alt->base munging
From: Jeff King @ 2016-10-03 20:35 UTC (permalink / raw)
To: git; +Cc: René Scharfe
In-Reply-To: <20161003203321.rj5jepviwo57uhqw@sigill.intra.peff.net>
The alternate_object_database struct holds a path to the
alternate objects, but we also use that buffer as scratch
space for forming loose object filenames. Let's pull that
logic into a helper function so that we can more easily
modify it.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 549cf1e..ccf59ba 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -204,6 +204,13 @@ const char *sha1_file_name(const unsigned char *sha1)
return buf;
}
+static const char *alt_sha1_path(struct alternate_object_database *alt,
+ const unsigned char *sha1)
+{
+ fill_sha1_path(alt->name, sha1);
+ return alt->base;
+}
+
/*
* Return the name of the pack or index file with the specified sha1
* in its filename. *base and *name are scratch space that must be
@@ -601,8 +608,8 @@ static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
struct alternate_object_database *alt;
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt->next) {
- fill_sha1_path(alt->name, sha1);
- if (check_and_freshen_file(alt->base, freshen))
+ const char *path = alt_sha1_path(alt, sha1);
+ if (check_and_freshen_file(path, freshen))
return 1;
}
return 0;
@@ -1600,8 +1607,8 @@ static int stat_sha1_file(const unsigned char *sha1, struct stat *st)
prepare_alt_odb();
errno = ENOENT;
for (alt = alt_odb_list; alt; alt = alt->next) {
- fill_sha1_path(alt->name, sha1);
- if (!lstat(alt->base, st))
+ const char *path = alt_sha1_path(alt, sha1);
+ if (!lstat(path, st))
return 0;
}
@@ -1621,8 +1628,8 @@ static int open_sha1_file(const unsigned char *sha1)
prepare_alt_odb();
for (alt = alt_odb_list; alt; alt = alt->next) {
- fill_sha1_path(alt->name, sha1);
- fd = git_open_noatime(alt->base);
+ const char *path = alt_sha1_path(alt, sha1);
+ fd = git_open_noatime(path);
if (fd >= 0)
return fd;
if (most_interesting_errno == ENOENT)
--
2.10.0.618.g82cc264
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox