* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Matthieu Moy @ 2017-03-07 7:16 UTC (permalink / raw)
To: Stefan Beller; +Cc: tom, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAGZ79kYxD9B_+3vBgO+Z-wh2GMg_REazA-xpTSAqe3_64VMV3w@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> What is the difference between signed commits and tags?
> (Not from a technical perspective, but for the end user)
In addition to what the others already said:
If you use GitHub, then in the web interface you get a "Verified" stamp
for each signed commits:
https://help.github.com/articles/signing-commits-using-gpg/
It's not a Git feature but a GitHub one, but given the popularity of
GitHub, this probably led some users to believe that signed commits are
more convenient than signed tags.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: regression: git push in non-shared repo stalls (v2.11.0+)
From: Horst Schirmeier @ 2017-03-07 11:14 UTC (permalink / raw)
To: git; +Cc: peff
In-Reply-To: <20170307110328.GE7566@quickstop.soohrt.org>
On Tue, 07 Mar 2017, Horst Schirmeier wrote:
> I observe a regression that seems to have been introduced between
> v2.10.0 and v2.11.0. When I try to push into a repository on the local
> filesystem that belongs to another user and has not explicitly been
> prepared for shared use, v2.11.0 shows some of the usual diagnostic
> output and then freezes instead of announcing why it failed to push.
Bisecting points to v2.10.1-373-g722ff7f:
722ff7f876c8a2ad99c42434f58af098e61b96e8 is the first bad commit
commit 722ff7f876c8a2ad99c42434f58af098e61b96e8
Author: Jeff King <peff@peff.net>
Date: Mon Oct 3 16:49:14 2016 -0400
receive-pack: quarantine objects until pre-receive accepts
--
PGP-Key 0xD40E0E7A
^ permalink raw reply
* Re: [PATCH v5 1/1] config: add conditional include
From: Jeff King @ 2017-03-07 8:47 UTC (permalink / raw)
To: Stefan Beller
Cc: Nguyễn Thái Ngọc Duy, git@vger.kernel.org,
Junio C Hamano, Sebastian Schuberth, Matthieu Moy
In-Reply-To: <CAGZ79kboaKfzMEyDjg-m2oK8CX6B56i52ZcWhCaq87ECE9x2Dw@mail.gmail.com>
On Mon, Mar 06, 2017 at 02:44:27PM -0800, Stefan Beller wrote:
> > +static int include_condition_is_true(const char *cond, size_t cond_len)
> > +{
> ...
> > +
> > + error(_("unrecognized include condition: %.*s"), (int)cond_len, cond);
> > + /* unknown conditionals are always false */
> > + return 0;
> > +}
>
> Thanks for putting an error message here. I was looking at what
> is currently queued as origin/nd/conditional-config-include,
> which doesn't have this error() (yet / not any more?)
It's "not any more". It was in the original and I asked for it to be
removed during the last review.
> I'd strongly suggest to keep the error message here as that way
> a user can diagnose e.g. a typo in the condition easily.
>
> If we plan to extend this list of conditions in the future, and a user
> switches between versions of git, then they may see this message
> on a regular basis (whenever they use the 'old' version).
That would make it unlike the rest of the config-include mechanism
(which quietly ignores things it doesn't understand, like include.foo,
or include.foo.path), as well as the config code in general (which
ignores misspelt keys).
Some of that "quiet when you don't understand it" is historical
necessity. Older versions _can't_ complain about not knowing
include.path, because they don't yet know it's worth complaining about.
Likewise here, if this ships in v2.13 and a new condition "foo:" ships
in v2.14, you get:
v2.12 - no complaint; we don't even understand includeIf at all
v2.13 - complain; we know includeIf, but not "foo:"
v2.14 - works as expected
Which is kind of weird and inconsistent. But maybe the typo-detection
case is more important to get right than consistency across historical
versions.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/9] Fix the early config
From: Johannes Schindelin @ 2017-03-07 11:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King, Duy Nguyen
In-Reply-To: <xmqqpohy6o2a.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 3 Mar 2017, Junio C Hamano wrote:
> 2/9 and corresponding 4/9 triggers "ERROR: trailing statements
> should be on next line" from ../linux/scripts/checkpatch.pl because
> of a line inherited from the original; I'll queue them with an
> obvious style fix to work it around.
Wow, it seems that script requires the entire Linux kernel repository to
be cloned, you cannot simply download just the script and run it.
In any case, as you pointed out, this style was inherited from the
original.
I squashed that style fix in, as you probably would have (your
js/early-config does not have anything beyond v2.12.0). But I have to
point out that it is conflating the purpose of this patch series (its goal
is *not* to fix any style). I am absolutely not a fan of that.
Ciao,
Johannes
^ permalink raw reply
* Re: [RFC 0/4] Shallow clones with on-demand fetch
From: Jeff King @ 2017-03-07 9:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Thomas, git
In-Reply-To: <xmqqr32anri1.fsf@junio-linux.mtv.corp.google.com>
On Mon, Mar 06, 2017 at 11:18:30AM -0800, Junio C Hamano wrote:
> Mark Thomas <markbt@efaref.net> writes:
>
> > This is a proof-of-concept, so it is in no way complete. It contains a
> > few hacks to make it work, but these can be ironed out with a bit more
> > work. What I have so far is sufficient to try out the idea.
>
> Two things that immediately come to mind (which may or may not be
> real issues) are
>
> (1) What (if any) security model you have in mind.
>
> From object-confidentiality's point of view, this needs to be
> enabled only on a host that allows
> uploadpack.allowAnySHA1InWant but even riskier.
>
> From DoS point of view, you can make a short 40-byte request to
> cause the other side emit megabytes of stuff. I do not think
> it is a new problem (anybody can repeatedly request a clone of
> large stuff), but there may be new ramifications.
>
> (2) If the interface to ask just one object kills the whole idea
> due to roundtrip latency.
>
> You may want to be able to say "I want all objects reachable
> from this tree; please give me a packfile of needed objects
> assuming that I have all objects reachable from this other tree
> (or these other trees)".
Not just latency, but you also lose all of the benefits of delta
compression. So if I asked for:
git log -p -- foo.c
and git is going to fault in all of the various versions of foo.c over
time, it's _much_ more efficient to batch them into a single request, so
that the server can reuse on-disk deltas between the various versions.
That makes the transmission smaller, and it also makes it more likely
for the server to be able to transmit the bits straight off the disk
(rather than assembling each delta itself then zlib-compressing the
result).
Similarly, there's a latency tension in just finding out whether an
object exists. When we call has_sha1_file() as part of a fetch, for
example, we really want to be able to answer it quickly. So you'd
probably want some mechanism to say "tell me the sha1, type, and size"
of each object I _could_ get via upload-file. The size of that data is
far from trivial for a large repository, but you're probably better off
getting it once than paying the latency cost to fetch it piecemeal.
-Peff
^ permalink raw reply
* [PATCH 1/6] receive-pack: fix deadlock when we cannot create tmpdir
From: Jeff King @ 2017-03-07 13:35 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
The err_fd descriptor passed to the unpack() function is
intended to be handed off to the child index-pack, and our
async muxer will read until it gets EOF. However, if we
encounter an error before handing off the descriptor, we
must manually close(err_fd). Otherwise we will be waiting
for our muxer to finish, while the muxer is waiting for EOF
on err_fd.
We fixed an identical deadlock already in 49ecfa13f
(receive-pack: close sideband fd on early pack errors,
2013-04-19). But since then, the function grew a new
early-return in 722ff7f87 (receive-pack: quarantine objects
until pre-receive accepts, 2016-10-03), when we fail to
create a temporary directory. This return needs the same
treatment.
Reported-by: Horst Schirmeier <horst@schirmeier.com>
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/receive-pack.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 9ed8fbbfa..f2c6953a3 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1667,8 +1667,11 @@ static const char *unpack(int err_fd, struct shallow_info *si)
}
tmp_objdir = tmp_objdir_create();
- if (!tmp_objdir)
+ if (!tmp_objdir) {
+ if (err_fd > 0)
+ close(err_fd);
return "unable to create temporary object directory";
+ }
child.env = tmp_objdir_env(tmp_objdir);
/*
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* [PATCH 2/6] send-pack: extract parsing of "unpack" response
From: Jeff King @ 2017-03-07 13:35 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
After sending the pack, we call receive_status() which gets
both the "unpack" line and the ref status. Let's break these
into two functions so we can call the first part
independently.
Signed-off-by: Jeff King <peff@peff.net>
---
send-pack.c | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 6195b43e9..12e229e44 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -130,22 +130,27 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru
return 0;
}
-static int receive_status(int in, struct ref *refs)
+static int receive_unpack_status(int in)
{
- struct ref *hint;
- int ret = 0;
- char *line = packet_read_line(in, NULL);
+ const char *line = packet_read_line(in, NULL);
if (!starts_with(line, "unpack "))
return error("did not receive remote status");
- if (strcmp(line, "unpack ok")) {
- error("unpack failed: %s", line + 7);
- ret = -1;
- }
+ if (strcmp(line, "unpack ok"))
+ return error("unpack failed: %s", line + 7);
+ return 0;
+}
+
+static int receive_status(int in, struct ref *refs)
+{
+ struct ref *hint;
+ int ret;
+
hint = NULL;
+ ret = receive_unpack_status(in);
while (1) {
char *refname;
char *msg;
- line = packet_read_line(in, NULL);
+ char *line = packet_read_line(in, NULL);
if (!line)
break;
if (!starts_with(line, "ok ") && !starts_with(line, "ng ")) {
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* regression: git push in non-shared repo stalls (v2.11.0+)
From: Horst Schirmeier @ 2017-03-07 11:03 UTC (permalink / raw)
To: git
Hi,
I observe a regression that seems to have been introduced between
v2.10.0 and v2.11.0. When I try to push into a repository on the local
filesystem that belongs to another user and has not explicitly been
prepared for shared use, v2.11.0 shows some of the usual diagnostic
output and then freezes instead of announcing why it failed to push.
Horst
Steps to reproduce (tested on Debian 8 "Jessie" amd64):
- User A creates a bare repository:
mkdir /tmp/gittest
git init --bare /tmp/gittest
- User B clones it, adds and commits a file:
git clone /tmp/gittest
cd gittest
echo 42 > x
git add x
git commit -m test
- User B tries to push to user A's bare repo:
git push
Expected result (git v2.10.0 and earlier):
test@ios:~/gittest$ git push
Counting objects: 3, done.
Writing objects: 100% (3/3), 230 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: error: insufficient permission for adding an object to repository database objects
remote: fatal: failed to write object
error: unpack failed: unpack-objects abnormal exit
To /tmp/gittest
! [remote rejected] master -> master (unpacker error)
error: failed to push some refs to '/tmp/gittest'
Actual result (git v2.11.0, v2.12.0, and 2.12.0.189.g3bc53220c):
test@ios:~/gittest$ git push
Counting objects: 3, done.
Writing objects: 100% (3/3), 230 bytes | 0 bytes/s, done.
Total 3 (delta 0), reused 0 (delta 0)
[... git freezes here ...]
--
PGP-Key 0xD40E0E7A
^ permalink raw reply
* [PATCH 6/6] send-pack: report signal death of pack-objects
From: Jeff King @ 2017-03-07 13:39 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
If our pack-objects sub-process dies of a signal, then it
likely didn't have a chance to write anything useful to
stderr. The user may be left scratching their head why the
push failed. Let's detect this situation and write something
to stderr.
Signed-off-by: Jeff King <peff@peff.net>
---
We could drop the SIGPIPE special-case, but I think it's just noise
after the unpack-status fix in the previous commit.
send-pack.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/send-pack.c b/send-pack.c
index e15232739..d2d2a49a0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -72,6 +72,7 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru
struct child_process po = CHILD_PROCESS_INIT;
FILE *po_in;
int i;
+ int rc;
i = 4;
if (args->use_thin_pack)
@@ -125,8 +126,20 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru
po.out = -1;
}
- if (finish_command(&po))
+ rc = finish_command(&po);
+ if (rc) {
+ /*
+ * For a normal non-zero exit, we assume pack-objects wrote
+ * something useful to stderr. For death by signal, though,
+ * we should mention it to the user. The exception is SIGPIPE
+ * (141), because that's a normal occurence if the remote end
+ * hangs up (and we'll report that by trying to read the unpack
+ * status).
+ */
+ if (rc > 128 && rc != 141)
+ error("pack-objects died of signal %d", rc - 128);
return -1;
+ }
return 0;
}
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* Re: [Request for Documentation] Differentiate signed (commits/tags/pushes)
From: Tom Jones @ 2017-03-07 9:45 UTC (permalink / raw)
To: Jeff King
Cc: Stefan Beller, Matthieu Moy, git@vger.kernel.org, Junio C Hamano
In-Reply-To: <20170307092353.ibirvitsxhzn3apz@sigill.intra.peff.net>
We use git to manage a config management repository for some
servers. We have tens of signed commits a day; all get deployed.
The logic on each host is roughly "is signed by sysadmin key and
is more recent than currently-deployed version".
Also, what is all this about "GPG"? The protocol is OpenPGP. A
particular implementation is GnuPG / gpg. It is completely mad
that this implementation detail is in the interface specs for git,
such as --gpg-sign for git-commit(1).
It is an indictment of a lack of appreciation of the relationship
between interfaces and implementations, and the importance of
proper treatment thereof.
If Bob creates Bob's git compatible program, and he happens to use
Bob's OpenPGP implementation, his compatible option for git-commit(1)
still has to be called "--gpg-sign". Madness.
Tom.
^ permalink raw reply
* [PATCH 5/6] send-pack: read "unpack" status even on pack-objects failure
From: Jeff King @ 2017-03-07 13:38 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
If the local pack-objects of a push fails, we'll tell the
user about it. But one likely cause is that the remote
index-pack stopped reading for some reason (because it
didn't like our input, or encountered another error). In
that case we'd expect the remote to report more details to
us via the "unpack ..." status line. However, the current
code just hangs up completely, and the user never sees it.
Instead, let's call receive_unpack_status(), which will
complain on stderr with whatever reason the remote told us.
Note that if our pack-objects fails because the connection
was severed or the remote just crashed entirely, then our
packet_read_line() call may fail with "the remote end hung
up unexpectedly". That's OK. It's a more accurate
description than what we get now (which is just "some refs
failed to push").
This should be safe from any deadlocks. At the point we make
this call we'll have closed the writing end of the
connection to the server (either by handing it off to
a pack-objects which exited, explicitly in the stateless_rpc
case, or by doing a half-duplex shutdown for a socket). So
there should be no chance that the other side is waiting
for the rest of our pack-objects input.
Signed-off-by: Jeff King <peff@peff.net>
---
send-pack.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/send-pack.c b/send-pack.c
index 83c23aef6..e15232739 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -562,6 +562,14 @@ int send_pack(struct send_pack_args *args,
close(out);
if (git_connection_is_socket(conn))
shutdown(fd[0], SHUT_WR);
+
+ /*
+ * Do not even bother with the return value; we know we
+ * are failing, and just want the error() side effects.
+ */
+ if (status_report)
+ receive_unpack_status(in);
+
if (use_sideband) {
close(demux.out);
finish_async(&demux);
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* [PATCH 4/6] send-pack: improve unpack-status error messages
From: Jeff King @ 2017-03-07 13:37 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
When the remote tells us that the "unpack" step failed, we
show an error message. However, unless you are familiar with
the internals of send-pack and receive-pack, it was not
clear that this represented an error on the remote side.
Let's re-word to make that more obvious.
Likewise, when we got an unexpected packet from the other
end, we complained with a vague message but did not actually
show the packet. Let's fix that.
And finally, neither message was marked for translation. The
message from the remote probably won't be translated, but
there's no reason we can't do better for the local half.
Signed-off-by: Jeff King <peff@peff.net>
---
send-pack.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 243633da1..83c23aef6 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -134,9 +134,9 @@ static int receive_unpack_status(int in)
{
const char *line = packet_read_line(in, NULL);
if (!skip_prefix(line, "unpack ", &line))
- return error("did not receive remote status");
+ return error(_("unable to parse remote unpack status: %s"), line);
if (strcmp(line, "ok"))
- return error("unpack failed: %s", line);
+ return error(_("remote unpack failed: %s"), line);
return 0;
}
--
2.12.0.429.gde83c8049
^ permalink raw reply related
* Re: [PATCH v3 0/9] Fix the early config
From: Johannes Schindelin @ 2017-03-07 14:31 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170304073952.drfgy7jacnlm6tum@sigill.intra.peff.net>
Hi Peff,
On Sat, 4 Mar 2017, Jeff King wrote:
> On Fri, Mar 03, 2017 at 06:31:55PM +0100, Johannes Schindelin wrote:
>
> > Interdiff vs v2:
> > [...]
> > + * When we are not about to create a repository ourselves (init or
> > + * clone) and when no .git/ directory was set up yet (in which case
> > + * git_config_with_options() would already have picked up the
> > + * repository config), we ask discover_git_directory() to figure out
> > + * whether there is any repository config we should use (but unlike
> > + * setup_git_directory_gently(), no global state is changed, most
> > + * notably, the current working directory is still the same after
> > + * the call).
> > */
> > - if (!startup_info->creating_repository && !have_git_dir() &&
> > - discover_git_directory(&buf)) {
> > + if (!have_git_dir() && discover_git_directory(&buf)) {
>
> I think this "when we are not about to..." part of the comment is no
> longer true, given the second part of the hunk.
Yep, that was a stale part of that patch. Thanks for noticing!
> > @@ -721,8 +721,10 @@ static const char *setup_discovered_git_dir(const char *gitdir,
> > if (offset == cwd->len)
> > return NULL;
> >
> > - /* Make "offset" point to past the '/', and add a '/' at the end */
> > - offset++;
> > + /* Make "offset" point past the '/' (already the case for root dirs) */
> > + if (offset != offset_1st_component(cwd->buf))
> > + offset++;
>
> Nice. I was worried we would have to have a hacky "well, sometimes we
> don't add one here..." code, but using offset_1st_component says
> exactly what we mean.
Right. I also wanted to avoid that very, very much. My initial version
actually tried to detect whether cwd already has a trailing slash, but
then I figured that we can be much, much more precise here (and I am
really pleased how offset_1st_component() is *semantically* precise, i.e.
it describes very well what the code is supposed to do here).
> > +/* Find GIT_DIR without changing the working directory or other global state */
> > extern const char *discover_git_directory(struct strbuf *gitdir);
>
> The parts that actually confused me were the parameters (mostly whether
> gitdir was a directory to start looking in, or an output parameter). So
> maybe:
>
> /*
> * Find GIT_DIR of the repository that contains the current working
> * directory, without changing the working directory or other global
> * state. The result is appended to gitdir. The return value is NULL
> * if no repository was found, or gitdir->buf otherwise.
> */
I changed it a little bit more. In particular, I changed the
discover_git_directory() function to return the pointer to the path
itself: it provides additional value, and if that is not what the caller
wants, they can use git_dir->buf just as well.
> This looks good to me aside from those few comment nits.
Thanks.
It is not obvious from the interdiff, but I had an incorrect fixup to 8/9
that actually wanted to go to 5/9: the code in
discover_git_repository() tests the repository version should be part of
the initial version of this function, of course.
There is one more thing I included in v4: when I (re-)implemented that
pre-command/post-command hook I was hinting at earlier, the test suite
identified a problem where an invalid .git file would prevent even `git
init` from working (it was actually much more complicated than that, but
the gist is that `git -p init` would fail, no matter how much sense it
may make to you to paginate an `init` run, it should still not fail,
right?). So I added a patch on top to fix that.
And another change: the GIT_DIR_NONE value was handled incorrectly in
discover_git_directory().
I am slightly disappointed that the these additional problems were not
spotted in any review but my own. And I had not even included a Duck.
> I'm still not sure I understand how ceil_offset works in
> setup_git_directory_gently_1(), but I don't think your patch actually
> changed it. I can live with my confusion.
Yes, that code is very confusing. It also does not help that the naming is
inconsistent in that it abbreviates "ceiling" but not "offset". What makes
it even worse is that the function name `longest_ancestor_length()` is
highly misleading: in Git's context, "ancestor" of sth pretty much always
refers to a commit reachable from sth, but in this context it refers to
the path of a directory containing sth.
So basically, we set ceil_offset to the offset of the last directory
separator in our path that corresponds to the most precise match in
GIT_CEILING_DIRECTORIES.
Example: given GIT_CEILING_DIRECTORIES /foo:/:/bar and a path of /foo/bar,
ceil_offset would be 4, pointing to the slash at the end of /foo/ because
that is the most precise match in GIT_CEILING_DIRECTORIES ("/" would also
match, but is less precise).
Later, setup_git_directory_gently_1() ensures that it does not go beyond
ceil_offset when looking for the parent directory as the next candidate to
test for .git/.
Hopefully that clears up the picture?
Ciao,
Dscho
^ permalink raw reply
* [PATCH v4 01/10] t7006: replace dubious test
From: Johannes Schindelin @ 2017-03-07 14:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
The idea of the test case "git -p - core.pager is not used from
subdirectory" was to verify that the setup_git_directory() function had
not been called just to obtain the core.pager setting.
However, we are about to fix the early config machinery so that it
*does* work, without messing up the global state.
Once that is done, the core.pager setting *will* be used, even when
running from a subdirectory, and that is a Good Thing.
The intention of that test case, however, was to verify that the
setup_git_directory() function has not run, because it changes global
state such as the current working directory.
To keep that spirit, but fix the incorrect assumption, this patch
replaces that test case by a new one that verifies that the pager is
run in the subdirectory, i.e. that the current working directory has
not been changed at the time the pager is configured and launched, even
if the `rev-parse` command requires a .git/ directory and *will* change
the working directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
| 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index c8dc665f2fd..304ae06c600 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -378,9 +378,19 @@ test_GIT_PAGER_overrides expect_success test_must_fail 'git -p request-pull'
test_default_pager expect_success test_must_fail 'git -p'
test_PAGER_overrides expect_success test_must_fail 'git -p'
test_local_config_ignored expect_failure test_must_fail 'git -p'
-test_no_local_config_subdir expect_success test_must_fail 'git -p'
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p'
+test_expect_failure TTY 'core.pager in repo config works and retains cwd' '
+ sane_unset GIT_PAGER &&
+ test_config core.pager "cat >cwd-retained" &&
+ (
+ cd sub &&
+ rm -f cwd-retained &&
+ test_terminal git -p rev-parse HEAD &&
+ test_path_is_file cwd-retained
+ )
+'
+
test_doesnt_paginate expect_failure test_must_fail 'git -p nonsense'
test_pager_choices 'git shortlog'
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 00/10] Fix the early config
From: Johannes Schindelin @ 2017-03-07 14:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.git.johannes.schindelin@gmx.de>
These patches are an attempt to make Git's startup sequence a bit less
surprising.
The idea here is to discover the .git/ directory gently (i.e. without
changing the current working directory, nor any global variables), and
to use it to read the .git/config file early, before we actually called
setup_git_directory() (if we ever do that).
This also allows us to fix the early config e.g. to determine the pager
or to resolve aliases in a non-surprising manner.
My own use case: in the GVFS Git fork, we need to execute pre-command
and post-command hooks before and after *every* Git command. A previous
version of the pre-command/post-command hook support was broken, as it
used run_hook() which implicitly called setup_git_directory() too early.
The discover_git_directory() function (and due to core.hooksPath also
the read_early_config() function) helped me fix this.
Notable notes:
- Even if it can cause surprising problems, `init` and `clone` are not
special-cased. Rationale: it would introduce technical debt and
violate the Principle Of Least Astonishment.
- The read_early_config() function does not cache Git directory
discovery nor read values. This is left for another patch series, if
it ever becomes necessary.
- The alias handling in git.c could possibly benefit from this work, but
again, this is a separate topic from the current patch series.
Changes since v3:
- instead of just fixing the `== '/'` comparison, 2/9 now *also* changes
the style of the original to something Linux' checkpatch.pl prefers.
- fixed the comment added in 8/9 which was stale from an earlier
iteration of this patch series.
- adjusted the commit above discover_git_directory()'s declaration, to
make it more understandable.
- touched up the commit message of 5/9 to make the flow more natural.
- moved a fault fixup: by mistake, the repository_format check was added
to 8/9 when it really needed to go to 5/9, where the
discover_git_directory() function was introduced (I noticed this while
rebasing).
- fixed discover_git_directory() when setup_git_directory_gently_1()
returned GIT_DIR_NONE (the previous iteration would *not* return NULL
in that case).
Johannes Schindelin (10):
t7006: replace dubious test
setup_git_directory(): use is_dir_sep() helper
Prepare setup_discovered_git_directory() the root directory
setup_git_directory_1(): avoid changing global state
Introduce the discover_git_directory() function
Make read_early_config() reusable
read_early_config(): avoid .git/config hack when unneeded
read_early_config(): really discover .git/
Test read_early_config()
setup_git_directory_gently_1(): avoid die()ing
cache.h | 8 ++
config.c | 25 +++++
pager.c | 31 ------
setup.c | 246 +++++++++++++++++++++++++++++++++---------------
t/helper/test-config.c | 15 +++
t/t1309-early-config.sh | 50 ++++++++++
t/t7006-pager.sh | 18 +++-
7 files changed, 282 insertions(+), 111 deletions(-)
create mode 100755 t/t1309-early-config.sh
base-commit: 3bc53220cb2dcf709f7a027a3f526befd021d858
Published-As: https://github.com/dscho/git/releases/tag/early-config-v4
Fetch-It-Via: git fetch https://github.com/dscho/git early-config-v4
Interdiff vs v3:
diff --git a/cache.h b/cache.h
index 8a4580f921d..e7b57457e73 100644
--- a/cache.h
+++ b/cache.h
@@ -518,7 +518,12 @@ extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
extern void setup_work_tree(void);
-/* Find GIT_DIR without changing the working directory or other global state */
+/*
+ * Find GIT_DIR of the repository that contains the current working directory,
+ * without changing the working directory or other global state. The result is
+ * appended to gitdir. The return value is either NULL if no repository was
+ * found, or pointing to the path inside gitdir's buffer.
+ */
extern const char *discover_git_directory(struct strbuf *gitdir);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
diff --git a/config.c b/config.c
index 749623a9649..a88df53fdbc 100644
--- a/config.c
+++ b/config.c
@@ -1419,14 +1419,12 @@ void read_early_config(config_fn_t cb, void *data)
git_config_with_options(cb, data, NULL, 1);
/*
- * When we are not about to create a repository ourselves (init or
- * clone) and when no .git/ directory was set up yet (in which case
- * git_config_with_options() would already have picked up the
- * repository config), we ask discover_git_directory() to figure out
- * whether there is any repository config we should use (but unlike
+ * When setup_git_directory() was not yet asked to discover the
+ * GIT_DIR, we ask discover_git_directory() to figure out whether there
+ * is any repository config we should use (but unlike
* setup_git_directory_gently(), no global state is changed, most
- * notably, the current working directory is still the same after
- * the call).
+ * notably, the current working directory is still the same after the
+ * call).
*/
if (!have_git_dir() && discover_git_directory(&buf)) {
struct git_config_source repo_config;
diff --git a/setup.c b/setup.c
index 5320ae37314..9118b48590a 100644
--- a/setup.c
+++ b/setup.c
@@ -825,7 +825,8 @@ enum discovery_result {
GIT_DIR_BARE,
/* these are errors */
GIT_DIR_HIT_CEILING = -1,
- GIT_DIR_HIT_MOUNT_POINT = -2
+ GIT_DIR_HIT_MOUNT_POINT = -2,
+ GIT_DIR_INVALID_GITFILE = -3
};
/*
@@ -842,7 +843,8 @@ enum discovery_result {
* `dir` (i.e. *not* necessarily the cwd).
*/
static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
- struct strbuf *gitdir)
+ struct strbuf *gitdir,
+ int die_on_error)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
@@ -890,14 +892,22 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (one_filesystem)
current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
- int offset = dir->len;
+ int offset = dir->len, error_code = 0;
if (offset > min_offset)
strbuf_addch(dir, '/');
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
- gitdirenv = read_gitfile(dir->buf);
- if (!gitdirenv && is_git_directory(dir->buf))
- gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
+ NULL : &error_code);
+ if (!gitdirenv) {
+ if (die_on_error ||
+ error_code == READ_GITFILE_ERR_NOT_A_FILE) {
+ if (is_git_directory(dir->buf))
+ gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ } else if (error_code &&
+ error_code != READ_GITFILE_ERR_STAT_FAILED)
+ return GIT_DIR_INVALID_GITFILE;
+ }
strbuf_setlen(dir, offset);
if (gitdirenv) {
strbuf_addstr(gitdir, gitdirenv);
@@ -912,7 +922,8 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (offset <= min_offset)
return GIT_DIR_HIT_CEILING;
- while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]));
+ while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]))
+ ; /* continue */
if (offset <= ceil_offset)
return GIT_DIR_HIT_CEILING;
@@ -933,7 +944,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
return NULL;
cwd_len = dir.len;
- if (setup_git_directory_gently_1(&dir, gitdir) < 0) {
+ if (setup_git_directory_gently_1(&dir, gitdir, 0) <= 0) {
strbuf_release(&dir);
return NULL;
}
@@ -963,7 +974,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
return NULL;
}
- return gitdir->buf;
+ return gitdir->buf + gitdir_offset;
}
const char *setup_git_directory_gently(int *nongit_ok)
@@ -992,7 +1003,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
die_errno(_("Unable to read current working directory"));
strbuf_addbuf(&dir, &cwd);
- switch (setup_git_directory_gently_1(&dir, &gitdir)) {
+ switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
case GIT_DIR_NONE:
prefix = NULL;
break;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply
* [PATCH v4 02/10] setup_git_directory(): use is_dir_sep() helper
From: Johannes Schindelin @ 2017-03-07 14:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
It is okay in practice to test for forward slashes in the output of
getcwd(), because we go out of our way to convert backslashes to forward
slashes in getcwd()'s output on Windows.
Still, the correct way to test for a dir separator is by using the
helper function we introduced for that very purpose. It also serves as a
good documentation what the code tries to do (not "how").
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index 967f289f1ef..4a15b105676 100644
--- a/setup.c
+++ b/setup.c
@@ -910,7 +910,9 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
return setup_bare_git_dir(&cwd, offset, nongit_ok);
offset_parent = offset;
- while (--offset_parent > ceil_offset && cwd.buf[offset_parent] != '/');
+ while (--offset_parent > ceil_offset &&
+ !is_dir_sep(cwd.buf[offset_parent]))
+ ; /* continue */
if (offset_parent <= ceil_offset)
return setup_nongit(cwd.buf, nongit_ok);
if (one_filesystem) {
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 03/10] Prepare setup_discovered_git_directory() the root directory
From: Johannes Schindelin @ 2017-03-07 14:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
Currently, the offset parameter (indicating what part of the cwd
parameter corresponds to the current directory after discovering the
.git/ directory) is set to 0 when we are running in the root directory.
However, in the next patches we will avoid changing the current working
directory while searching for the .git/ directory, meaning that the
offset corresponding to the root directory will have to be 1 to reflect
that this directory is characterized by the path "/" (and not "").
So let's make sure that setup_discovered_git_directory() only tries to
append the trailing slash to non-root directories.
Note: the setup_bare_git_directory() does not need a corresponding
change, as it does not want to return a prefix.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 4a15b105676..20a1f0f870e 100644
--- a/setup.c
+++ b/setup.c
@@ -721,8 +721,10 @@ static const char *setup_discovered_git_dir(const char *gitdir,
if (offset == cwd->len)
return NULL;
- /* Make "offset" point to past the '/', and add a '/' at the end */
- offset++;
+ /* Make "offset" point past the '/' (already the case for root dirs) */
+ if (offset != offset_1st_component(cwd->buf))
+ offset++;
+ /* Add a '/' at the end */
strbuf_addch(cwd, '/');
return cwd->buf + offset;
}
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 04/10] setup_git_directory_1(): avoid changing global state
From: Johannes Schindelin @ 2017-03-07 14:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
For historical reasons, Git searches for the .git/ directory (or the
.git file) by changing the working directory successively to the parent
directory of the current directory, until either anything was found or
until a ceiling or a mount point is hit.
Further global state may be changed in case a .git/ directory was found.
We do have a use case, though, where we would like to find the .git/
directory without having any global state touched, though: when we read
the early config e.g. for the pager or for alias expansion.
Let's just move all of code that changes any global state out of the
function `setup_git_directory_gently_1()` into
`setup_git_directory_gently()`.
In subsequent patches, we will use the _1() function in a new
`discover_git_directory()` function that we will then use for the early
config code.
Note: the new loop is a *little* tricky, as we have to handle the root
directory specially: we cannot simply strip away the last component
including the slash, as the root directory only has that slash. To remedy
that, we introduce the `min_offset` variable that holds the minimal length
of an absolute path, and using that to special-case the root directory,
including an early exit before trying to find the parent of the root
directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 187 ++++++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 112 insertions(+), 75 deletions(-)
diff --git a/setup.c b/setup.c
index 20a1f0f870e..d7af343d14e 100644
--- a/setup.c
+++ b/setup.c
@@ -818,50 +818,49 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
}
}
+enum discovery_result {
+ GIT_DIR_NONE = 0,
+ GIT_DIR_EXPLICIT,
+ GIT_DIR_DISCOVERED,
+ GIT_DIR_BARE,
+ /* these are errors */
+ GIT_DIR_HIT_CEILING = -1,
+ GIT_DIR_HIT_MOUNT_POINT = -2
+};
+
/*
* We cannot decide in this function whether we are in the work tree or
* not, since the config can only be read _after_ this function was called.
+ *
+ * Also, we avoid changing any global state (such as the current working
+ * directory) to allow early callers.
+ *
+ * The directory where the search should start needs to be passed in via the
+ * `dir` parameter; upon return, the `dir` buffer will contain the path of
+ * the directory where the search ended, and `gitdir` will contain the path of
+ * the discovered .git/ directory, if any. This path may be relative against
+ * `dir` (i.e. *not* necessarily the cwd).
*/
-static const char *setup_git_directory_gently_1(int *nongit_ok)
+static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
+ struct strbuf *gitdir)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
- static struct strbuf cwd = STRBUF_INIT;
- const char *gitdirenv, *ret;
- char *gitfile;
- int offset, offset_parent, ceil_offset = -1;
+ const char *gitdirenv;
+ int ceil_offset = -1, min_offset = has_dos_drive_prefix(dir->buf) ? 3 : 1;
dev_t current_device = 0;
int one_filesystem = 1;
/*
- * We may have read an incomplete configuration before
- * setting-up the git directory. If so, clear the cache so
- * that the next queries to the configuration reload complete
- * configuration (including the per-repo config file that we
- * ignored previously).
- */
- git_config_clear();
-
- /*
- * Let's assume that we are in a git repository.
- * If it turns out later that we are somewhere else, the value will be
- * updated accordingly.
- */
- if (nongit_ok)
- *nongit_ok = 0;
-
- if (strbuf_getcwd(&cwd))
- die_errno(_("Unable to read current working directory"));
- offset = cwd.len;
-
- /*
* If GIT_DIR is set explicitly, we're not going
* to do any discovery, but we still do repository
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
- if (gitdirenv)
- return setup_explicit_git_dir(gitdirenv, &cwd, nongit_ok);
+ if (gitdirenv) {
+ strbuf_addstr(gitdir, gitdirenv);
+ return GIT_DIR_EXPLICIT;
+ }
if (env_ceiling_dirs) {
int empty_entry_found = 0;
@@ -869,15 +868,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
filter_string_list(&ceiling_dirs, 0,
canonicalize_ceiling_entry, &empty_entry_found);
- ceil_offset = longest_ancestor_length(cwd.buf, &ceiling_dirs);
+ ceil_offset = longest_ancestor_length(dir->buf, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
}
- if (ceil_offset < 0 && has_dos_drive_prefix(cwd.buf))
- ceil_offset = 1;
+ if (ceil_offset < 0)
+ ceil_offset = min_offset - 2;
/*
- * Test in the following order (relative to the cwd):
+ * Test in the following order (relative to the dir):
* - .git (file containing "gitdir: <path>")
* - .git/
* - ./ (bare)
@@ -889,63 +888,101 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
*/
one_filesystem = !git_env_bool("GIT_DISCOVERY_ACROSS_FILESYSTEM", 0);
if (one_filesystem)
- current_device = get_device_or_die(".", NULL, 0);
+ current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
- gitfile = (char*)read_gitfile(DEFAULT_GIT_DIR_ENVIRONMENT);
- if (gitfile)
- gitdirenv = gitfile = xstrdup(gitfile);
- else {
- if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
- gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ int offset = dir->len;
+
+ if (offset > min_offset)
+ strbuf_addch(dir, '/');
+ strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
+ gitdirenv = read_gitfile(dir->buf);
+ if (!gitdirenv && is_git_directory(dir->buf))
+ gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ strbuf_setlen(dir, offset);
+ if (gitdirenv) {
+ strbuf_addstr(gitdir, gitdirenv);
+ return GIT_DIR_DISCOVERED;
}
- if (gitdirenv) {
- ret = setup_discovered_git_dir(gitdirenv,
- &cwd, offset,
- nongit_ok);
- free(gitfile);
- return ret;
+ if (is_git_directory(dir->buf)) {
+ strbuf_addstr(gitdir, ".");
+ return GIT_DIR_BARE;
}
- free(gitfile);
- if (is_git_directory("."))
- return setup_bare_git_dir(&cwd, offset, nongit_ok);
+ if (offset <= min_offset)
+ return GIT_DIR_HIT_CEILING;
- offset_parent = offset;
- while (--offset_parent > ceil_offset &&
- !is_dir_sep(cwd.buf[offset_parent]))
+ while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]))
; /* continue */
- if (offset_parent <= ceil_offset)
- return setup_nongit(cwd.buf, nongit_ok);
- if (one_filesystem) {
- dev_t parent_device = get_device_or_die("..", cwd.buf,
- offset);
- if (parent_device != current_device) {
- if (nongit_ok) {
- if (chdir(cwd.buf))
- die_errno(_("Cannot come back to cwd"));
- *nongit_ok = 1;
- return NULL;
- }
- strbuf_setlen(&cwd, offset);
- die(_("Not a git repository (or any parent up to mount point %s)\n"
- "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
- cwd.buf);
- }
- }
- if (chdir("..")) {
- strbuf_setlen(&cwd, offset);
- die_errno(_("Cannot change to '%s/..'"), cwd.buf);
- }
- offset = offset_parent;
+ if (offset <= ceil_offset)
+ return GIT_DIR_HIT_CEILING;
+
+ strbuf_setlen(dir, offset > min_offset ? offset : min_offset);
+ if (one_filesystem &&
+ current_device != get_device_or_die(dir->buf, NULL, offset))
+ return GIT_DIR_HIT_MOUNT_POINT;
}
}
const char *setup_git_directory_gently(int *nongit_ok)
{
+ struct strbuf cwd = STRBUF_INIT, dir = STRBUF_INIT, gitdir = STRBUF_INIT;
const char *prefix;
- prefix = setup_git_directory_gently_1(nongit_ok);
+ /*
+ * We may have read an incomplete configuration before
+ * setting-up the git directory. If so, clear the cache so
+ * that the next queries to the configuration reload complete
+ * configuration (including the per-repo config file that we
+ * ignored previously).
+ */
+ git_config_clear();
+
+ /*
+ * Let's assume that we are in a git repository.
+ * If it turns out later that we are somewhere else, the value will be
+ * updated accordingly.
+ */
+ if (nongit_ok)
+ *nongit_ok = 0;
+
+ if (strbuf_getcwd(&cwd))
+ die_errno(_("Unable to read current working directory"));
+ strbuf_addbuf(&dir, &cwd);
+
+ switch (setup_git_directory_gently_1(&dir, &gitdir)) {
+ case GIT_DIR_NONE:
+ prefix = NULL;
+ break;
+ case GIT_DIR_EXPLICIT:
+ prefix = setup_explicit_git_dir(gitdir.buf, &cwd, nongit_ok);
+ break;
+ case GIT_DIR_DISCOVERED:
+ if (dir.len < cwd.len && chdir(dir.buf))
+ die(_("Cannot change to '%s'"), dir.buf);
+ prefix = setup_discovered_git_dir(gitdir.buf, &cwd, dir.len,
+ nongit_ok);
+ break;
+ case GIT_DIR_BARE:
+ if (dir.len < cwd.len && chdir(dir.buf))
+ die(_("Cannot change to '%s'"), dir.buf);
+ prefix = setup_bare_git_dir(&cwd, dir.len, nongit_ok);
+ break;
+ case GIT_DIR_HIT_CEILING:
+ prefix = setup_nongit(cwd.buf, nongit_ok);
+ break;
+ case GIT_DIR_HIT_MOUNT_POINT:
+ if (nongit_ok) {
+ *nongit_ok = 1;
+ return NULL;
+ }
+ die(_("Not a git repository (or any parent up to mount point %s)\n"
+ "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
+ dir.buf);
+ default:
+ die("BUG: unhandled setup_git_directory_1() result");
+ }
+
if (prefix)
setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);
else
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 05/10] Introduce the discover_git_directory() function
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
We modified the setup_git_directory_gently_1() function earlier to make
it possible to discover the GIT_DIR without changing global state.
However, it is still a bit cumbersome to use if you only need to figure
out the (possibly absolute) path of the .git/ directory. Let's just
provide a convenient wrapper function with an easier signature that
*just* discovers the .git/ directory.
We will use it in a subsequent patch to fix the early config.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 7 +++++++
setup.c | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/cache.h b/cache.h
index 80b6372cf76..5218726cf88 100644
--- a/cache.h
+++ b/cache.h
@@ -518,6 +518,13 @@ extern void set_git_work_tree(const char *tree);
#define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
extern void setup_work_tree(void);
+/*
+ * Find GIT_DIR of the repository that contains the current working directory,
+ * without changing the working directory or other global state. The result is
+ * appended to gitdir. The return value is either NULL if no repository was
+ * found, or pointing to the path inside gitdir's buffer.
+ */
+extern const char *discover_git_directory(struct strbuf *gitdir);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
extern char *prefix_path(const char *prefix, int len, const char *path);
diff --git a/setup.c b/setup.c
index d7af343d14e..486acda2054 100644
--- a/setup.c
+++ b/setup.c
@@ -924,6 +924,49 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
}
}
+const char *discover_git_directory(struct strbuf *gitdir)
+{
+ struct strbuf dir = STRBUF_INIT, err = STRBUF_INIT;
+ size_t gitdir_offset = gitdir->len, cwd_len;
+ struct repository_format candidate;
+
+ if (strbuf_getcwd(&dir))
+ return NULL;
+
+ cwd_len = dir.len;
+ if (setup_git_directory_gently_1(&dir, gitdir) <= 0) {
+ strbuf_release(&dir);
+ return NULL;
+ }
+
+ /*
+ * The returned gitdir is relative to dir, and if dir does not reflect
+ * the current working directory, we simply make the gitdir absolute.
+ */
+ if (dir.len < cwd_len && !is_absolute_path(gitdir->buf + gitdir_offset)) {
+ /* Avoid a trailing "/." */
+ if (!strcmp(".", gitdir->buf + gitdir_offset))
+ strbuf_setlen(gitdir, gitdir_offset);
+ else
+ strbuf_addch(&dir, '/');
+ strbuf_insert(gitdir, gitdir_offset, dir.buf, dir.len);
+ }
+
+ strbuf_reset(&dir);
+ strbuf_addf(&dir, "%s/config", gitdir->buf + gitdir_offset);
+ read_repository_format(&candidate, dir.buf);
+ strbuf_release(&dir);
+
+ if (verify_repository_format(&candidate, &err) < 0) {
+ warning("ignoring git dir '%s': %s",
+ gitdir->buf + gitdir_offset, err.buf);
+ strbuf_release(&err);
+ return NULL;
+ }
+
+ return gitdir->buf + gitdir_offset;
+}
+
const char *setup_git_directory_gently(int *nongit_ok)
{
struct strbuf cwd = STRBUF_INIT, dir = STRBUF_INIT, gitdir = STRBUF_INIT;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 06/10] Make read_early_config() reusable
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
The pager configuration needs to be read early, possibly before
discovering any .git/ directory.
Let's not hide this function in pager.c, but make it available to other
callers.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 1 +
config.c | 31 +++++++++++++++++++++++++++++++
| 31 -------------------------------
3 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/cache.h b/cache.h
index 5218726cf88..e7b57457e73 100644
--- a/cache.h
+++ b/cache.h
@@ -1804,6 +1804,7 @@ extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
const unsigned char *sha1, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
+extern void read_early_config(config_fn_t cb, void *data);
extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
diff --git a/config.c b/config.c
index c6b874a7bf7..9cfbeafd04c 100644
--- a/config.c
+++ b/config.c
@@ -1412,6 +1412,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
}
+void read_early_config(config_fn_t cb, void *data)
+{
+ git_config_with_options(cb, data, NULL, 1);
+
+ /*
+ * Note that this is a really dirty hack that does the wrong thing in
+ * many cases. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have no idea if
+ * we are in a repository or not, and therefore are not sure whether
+ * and how to read repository-local config.
+ *
+ * So if we _aren't_ in a repository (or we are but we would reject its
+ * core.repositoryformatversion), we'll read whatever is in .git/config
+ * blindly. Similarly, if we _are_ in a repository, but not at the
+ * root, we'll fail to find .git/config (because it's really
+ * ../.git/config, etc). See t7006 for a complete set of failures.
+ *
+ * However, we have historically provided this hack because it does
+ * work some of the time (namely when you are at the top-level of a
+ * valid repository), and would rarely make things worse (i.e., you do
+ * not generally have a .git/config file sitting around).
+ */
+ if (!startup_info->have_repository) {
+ struct git_config_source repo_config;
+
+ memset(&repo_config, 0, sizeof(repo_config));
+ repo_config.file = ".git/config";
+ git_config_with_options(cb, data, &repo_config, 1);
+ }
+}
+
static void git_config_check_init(void);
void git_config(config_fn_t fn, void *data)
--git a/pager.c b/pager.c
index ae796433630..73ca8bc3b17 100644
--- a/pager.c
+++ b/pager.c
@@ -43,37 +43,6 @@ static int core_pager_config(const char *var, const char *value, void *data)
return 0;
}
-static void read_early_config(config_fn_t cb, void *data)
-{
- git_config_with_options(cb, data, NULL, 1);
-
- /*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
- */
- if (!startup_info->have_repository) {
- struct git_config_source repo_config;
-
- memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
- git_config_with_options(cb, data, &repo_config, 1);
- }
-}
-
const char *git_pager(int stdout_is_tty)
{
const char *pager;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 08/10] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
Earlier, we punted and simply assumed that we are in the top-level
directory of the project, and that there is no .git file but a .git/
directory so that we can read directly from .git/config.
However, that is not necessarily true. We may be in a subdirectory. Or
.git may be a gitfile. Or the environment variable GIT_DIR may be set.
To remedy this situation, we just refactored the way
setup_git_directory() discovers the .git/ directory, to make it
reusable, and more importantly, to leave all global variables and the
current working directory alone.
Let's discover the .git/ directory correctly in read_early_config() by
using that new function.
This fixes 4 known breakages in t7006.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 31 ++++++++++++-------------------
| 8 ++++----
2 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/config.c b/config.c
index 068fa4dcfa6..a88df53fdbc 100644
--- a/config.c
+++ b/config.c
@@ -1414,34 +1414,27 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
void read_early_config(config_fn_t cb, void *data)
{
+ struct strbuf buf = STRBUF_INIT;
+
git_config_with_options(cb, data, NULL, 1);
/*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc), unless setup_git_directory() was already called.
- * See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
+ * When setup_git_directory() was not yet asked to discover the
+ * GIT_DIR, we ask discover_git_directory() to figure out whether there
+ * is any repository config we should use (but unlike
+ * setup_git_directory_gently(), no global state is changed, most
+ * notably, the current working directory is still the same after the
+ * call).
*/
- if (!have_git_dir()) {
+ if (!have_git_dir() && discover_git_directory(&buf)) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
+ strbuf_addstr(&buf, "/config");
+ repo_config.file = buf.buf;
git_config_with_options(cb, data, &repo_config, 1);
}
+ strbuf_release(&buf);
}
static void git_config_check_init(void);
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 304ae06c600..4f3794d415e 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -360,19 +360,19 @@ test_pager_choices 'git aliasedlog'
test_default_pager expect_success 'git -p aliasedlog'
test_PAGER_overrides expect_success 'git -p aliasedlog'
test_core_pager_overrides expect_success 'git -p aliasedlog'
-test_core_pager_subdir expect_failure 'git -p aliasedlog'
+test_core_pager_subdir expect_success 'git -p aliasedlog'
test_GIT_PAGER_overrides expect_success 'git -p aliasedlog'
test_default_pager expect_success 'git -p true'
test_PAGER_overrides expect_success 'git -p true'
test_core_pager_overrides expect_success 'git -p true'
-test_core_pager_subdir expect_failure 'git -p true'
+test_core_pager_subdir expect_success 'git -p true'
test_GIT_PAGER_overrides expect_success 'git -p true'
test_default_pager expect_success test_must_fail 'git -p request-pull'
test_PAGER_overrides expect_success test_must_fail 'git -p request-pull'
test_core_pager_overrides expect_success test_must_fail 'git -p request-pull'
-test_core_pager_subdir expect_failure test_must_fail 'git -p request-pull'
+test_core_pager_subdir expect_success test_must_fail 'git -p request-pull'
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p request-pull'
test_default_pager expect_success test_must_fail 'git -p'
@@ -380,7 +380,7 @@ test_PAGER_overrides expect_success test_must_fail 'git -p'
test_local_config_ignored expect_failure test_must_fail 'git -p'
test_GIT_PAGER_overrides expect_success test_must_fail 'git -p'
-test_expect_failure TTY 'core.pager in repo config works and retains cwd' '
+test_expect_success TTY 'core.pager in repo config works and retains cwd' '
sane_unset GIT_PAGER &&
test_config core.pager "cat >cwd-retained" &&
(
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 10/10] setup_git_directory_gently_1(): avoid die()ing
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
This function now has a new caller in addition to setup_git_directory():
the newly introduced discover_git_directory(). That function wants to
discover the current .git/ directory, and in case of a corrupted one
simply pretend that there is none to be found.
Example: if a stale .git file exists in the parent directory, and the
user calls `git -p init`, we want Git to simply *not* read any
repository config for the pager (instead of aborting with a message that
the .git file is corrupt).
Let's actually pretend that there was no GIT_DIR to be found in that case
when being called from discover_git_directory(), but keep the previous
behavior (i.e. to die()) for the setup_git_directory() case.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
setup.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/setup.c b/setup.c
index 486acda2054..9118b48590a 100644
--- a/setup.c
+++ b/setup.c
@@ -825,7 +825,8 @@ enum discovery_result {
GIT_DIR_BARE,
/* these are errors */
GIT_DIR_HIT_CEILING = -1,
- GIT_DIR_HIT_MOUNT_POINT = -2
+ GIT_DIR_HIT_MOUNT_POINT = -2,
+ GIT_DIR_INVALID_GITFILE = -3
};
/*
@@ -842,7 +843,8 @@ enum discovery_result {
* `dir` (i.e. *not* necessarily the cwd).
*/
static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
- struct strbuf *gitdir)
+ struct strbuf *gitdir,
+ int die_on_error)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
@@ -890,14 +892,22 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
if (one_filesystem)
current_device = get_device_or_die(dir->buf, NULL, 0);
for (;;) {
- int offset = dir->len;
+ int offset = dir->len, error_code = 0;
if (offset > min_offset)
strbuf_addch(dir, '/');
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
- gitdirenv = read_gitfile(dir->buf);
- if (!gitdirenv && is_git_directory(dir->buf))
- gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
+ NULL : &error_code);
+ if (!gitdirenv) {
+ if (die_on_error ||
+ error_code == READ_GITFILE_ERR_NOT_A_FILE) {
+ if (is_git_directory(dir->buf))
+ gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
+ } else if (error_code &&
+ error_code != READ_GITFILE_ERR_STAT_FAILED)
+ return GIT_DIR_INVALID_GITFILE;
+ }
strbuf_setlen(dir, offset);
if (gitdirenv) {
strbuf_addstr(gitdir, gitdirenv);
@@ -934,7 +944,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
return NULL;
cwd_len = dir.len;
- if (setup_git_directory_gently_1(&dir, gitdir) <= 0) {
+ if (setup_git_directory_gently_1(&dir, gitdir, 0) <= 0) {
strbuf_release(&dir);
return NULL;
}
@@ -993,7 +1003,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
die_errno(_("Unable to read current working directory"));
strbuf_addbuf(&dir, &cwd);
- switch (setup_git_directory_gently_1(&dir, &gitdir)) {
+ switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {
case GIT_DIR_NONE:
prefix = NULL;
break;
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* Re: [PATCH 0/6] deadlock regression in v2.11.0 with failed mkdtemp
From: Jeff King @ 2017-03-07 13:50 UTC (permalink / raw)
To: Horst Schirmeier; +Cc: git
In-Reply-To: <20170307133437.qee2jtynbiwf6uzr@sigill.intra.peff.net>
On Tue, Mar 07, 2017 at 08:34:37AM -0500, Jeff King wrote:
> Yuck. In the original, the error was generated by the child index-pack,
> and we relayed it over the sideband. But we don't even get as far as
> running index-pack in the newer version; we fail trying to make the
> tmpdir. The error ends up in the "unpack" protocol field, but the client
> side does a bad job of showing that. With the rest of the patches, it
> looks like:
>
> $ git push ~/tmp/foo.git HEAD
> Counting objects: 210973, done.
> Delta compression using up to 8 threads.
> Compressing objects: 100% (52799/52799), done.
> error: remote unpack failed: unable to create temporary object directory
> error: failed to push some refs to '/home/peff/tmp/foo.git'
There are two other options here I should mention.
One is that when pack-objects dies, we suppress the ref-status table
entirely. Which is fair, because it doesn't have anything interesting to
say. But for a case like this where the other side stops reading our
pack early but still produces status reports, we could actually read
them all and have a status table like:
$ git push ~/tmp/foo.git HEAD
Counting objects: 209843, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52186/52186), done.
error: remote unpack failed: unable to create temporary object directory
To /home/peff/tmp/foo.git
! [remote rejected] HEAD -> jk/push-index-pack-deadlock (unpacker error)
error: failed to push some refs to '/home/peff/tmp/foo.git'
We'd have to take some care to make sure we handle the case when the
remote _doesn't_ manage to give us the status (e.g., when there's a
complete hangup). I don't know if it's worth the effort. There's no new
information there.
The second thing is that I think the design of the "unpack <reason>"
report is a bit dated. These days everybody supports the sideband
protocol, so it would probably make more sense to just issue the
"<reason>" report via the sideband (at which point it gets a nice
"remote: " prefix).
We could do something like this:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f2c6953a3..6204d3d00 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1670,6 +1670,8 @@ static const char *unpack(int err_fd, struct shallow_info *si)
if (!tmp_objdir) {
if (err_fd > 0)
close(err_fd);
+ rp_error("unable to create temporary object directory: %s",
+ strerror(errno));
return "unable to create temporary object directory";
}
child.env = tmp_objdir_env(tmp_objdir);
and drop the "try to show the unpack failure" parts of my series, and
you'd end up with:
$ git push ~/tmp/foo.git HEAD
Counting objects: 209843, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52186/52186), done.
remote: error: unable to create temporary object directory: Permission denied
error: failed to push some refs to '/home/peff/tmp/foo.git'
but in cases where pack-objects _doesn't_ fail, existing git versions
do show the "unpack" error. So you'd see it twice.
I don't know if it's worth trying to hack around.
-Peff
^ permalink raw reply related
* [PATCH v4 09/10] Test read_early_config()
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
So far, we had no explicit tests of that function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-config.c | 15 +++++++++++++++
t/t1309-early-config.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
create mode 100755 t/t1309-early-config.sh
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 83a4f2ab869..8e3ed6a76cb 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -66,6 +66,16 @@ static int iterate_cb(const char *var, const char *value, void *data)
return 0;
}
+static int early_config_cb(const char *var, const char *value, void *vdata)
+{
+ const char *key = vdata;
+
+ if (!strcmp(key, var))
+ printf("%s\n", value);
+
+ return 0;
+}
+
int cmd_main(int argc, const char **argv)
{
int i, val;
@@ -73,6 +83,11 @@ int cmd_main(int argc, const char **argv)
const struct string_list *strptr;
struct config_set cs;
+ if (argc == 3 && !strcmp(argv[1], "read_early_config")) {
+ read_early_config(early_config_cb, (void *)argv[2]);
+ return 0;
+ }
+
setup_git_directory();
git_configset_init(&cs);
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
new file mode 100755
index 00000000000..0c55dee514c
--- /dev/null
+++ b/t/t1309-early-config.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test read_early_config()'
+
+. ./test-lib.sh
+
+test_expect_success 'read early config' '
+ test_config early.config correct &&
+ test-config read_early_config early.config >output &&
+ test correct = "$(cat output)"
+'
+
+test_expect_success 'in a sub-directory' '
+ test_config early.config sub &&
+ mkdir -p sub &&
+ (
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test sub = "$(cat output)"
+'
+
+test_expect_success 'ceiling' '
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test -z "$(cat output)"
+'
+
+test_expect_success 'ceiling #2' '
+ mkdir -p xdg/git &&
+ git config -f xdg/git/config early.config xdg &&
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ XDG_CONFIG_HOME="$PWD"/xdg &&
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES XDG_CONFIG_HOME &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test xdg = "$(cat output)"
+'
+
+test_done
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply related
* [PATCH v4 07/10] read_early_config(): avoid .git/config hack when unneeded
From: Johannes Schindelin @ 2017-03-07 14:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488897111.git.johannes.schindelin@gmx.de>
So far, we only look whether the startup_info claims to have seen a
git_dir.
However, do_git_config_sequence() (and consequently the
git_config_with_options() call used by read_early_config() asks the
have_git_dir() function whether we have a .git/ directory, which in turn
also looks at git_dir and at the environment variable GIT_DIR. And when
this is the case, the repository config is handled already, so we do not
have to do that again explicitly.
Let's just use the same function, have_git_dir(), to determine whether we
have to handle .git/config explicitly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 9cfbeafd04c..068fa4dcfa6 100644
--- a/config.c
+++ b/config.c
@@ -1427,14 +1427,15 @@ void read_early_config(config_fn_t cb, void *data)
* core.repositoryformatversion), we'll read whatever is in .git/config
* blindly. Similarly, if we _are_ in a repository, but not at the
* root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
+ * ../.git/config, etc), unless setup_git_directory() was already called.
+ * See t7006 for a complete set of failures.
*
* However, we have historically provided this hack because it does
* work some of the time (namely when you are at the top-level of a
* valid repository), and would rarely make things worse (i.e., you do
* not generally have a .git/config file sitting around).
*/
- if (!startup_info->have_repository) {
+ if (!have_git_dir()) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
--
2.12.0.windows.1.7.g94dafc3b124
^ 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