* Finding a tag that introduced a submodule change
From: Robert Dailey @ 2017-03-03 15:40 UTC (permalink / raw)
To: Git
I have a repository with a single submodule in it. Since the parent
repository represents the code base for an actual product, I tag
release versions in the parent repository. I do not put tags in the
submodule since multiple other products may be using it there and I
wanted to avoid ambiguous tags.
Sometimes I run into a situation where I need to find out which
release of the product a submodule change was introduced in. This is
nontrivial, since there are no tags in the submodule itself. This is
one thing I tried:
1. Do a `git log` in the submodule to find the SHA1 representing the
change I want to check for
2. In the parent repository, do a git log with pickaxe to determine
when the submodule itself changed to the value of that SHA1.
3. Based on the result of #2, do a `git tag --contains` to see the
lowest-version tag that contains the SHA1, which will identify the
first release that introduced that change
However, I was not able to get past #2 because apparently there are
cases where when we move the submodule "forward", we skip over
commits, so the value of the submodule itself never was set to that
SHA1.
I'm at a loss here on how to easily do this. Can someone recommend a
way to do this? Obviously the easier the better, as I have to somehow
train my team how to do this on their own.
Thanks in advance.
^ permalink raw reply
* Re: [PATCH v2] Travis: also test on 32-bit Linux
From: Johannes Schindelin @ 2017-03-03 15:42 UTC (permalink / raw)
To: Lars Schneider; +Cc: git, gitster, ramsay, christian.couder
In-Reply-To: <20170303110125.43332-1-larsxschneider@gmail.com>
Hi Lars,
On Fri, 3 Mar 2017, Lars Schneider wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When Git v2.9.1 was released, it had a bug that showed only on Windows
> and on 32-bit systems: our assumption that `unsigned long` can hold
> 64-bit values turned out to be wrong.
>
> This could have been caught earlier if we had a Continuous Testing
> set up that includes a build and test run on 32-bit Linux.
>
> Let's do this (and take care of the Windows build later). This patch
> asks Travis CI to install a Docker image with 32-bit libraries and then
> goes on to build and test Git using this 32-bit setup.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
Thanks for keeping the ball rolling! I am totally fine with v2.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Do not require Python for the git-remote-{bzr,hg} placeholder scripts
From: Marc Branchaud @ 2017-03-03 15:32 UTC (permalink / raw)
To: Sebastian Schuberth, git
In-Reply-To: <0102015a93d18443-c574aaf8-5656-49e8-8848-810e1fadc87a-000000@eu-west-1.amazonses.com>
On 2017-03-03 05:57 AM, Sebastian Schuberth wrote:
> It does not make sense for these placeholder scripts to depend on Python
> just because the real scripts do. At the example of Git for Windows, we
> would not even be able to see those warnings as it does not ship with
> Python. So just use plain shell scripts instead.
Just a niggle: This change moves the warning message from stderr to stdout.
M.
> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
> contrib/remote-helpers/git-remote-bzr | 16 +++++++---------
> contrib/remote-helpers/git-remote-hg | 16 +++++++---------
> 2 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
> index 712a137..ccc4aea 100755
> --- a/contrib/remote-helpers/git-remote-bzr
> +++ b/contrib/remote-helpers/git-remote-bzr
> @@ -1,13 +1,11 @@
> -#!/usr/bin/env python
> +#!/bin/sh
>
> -import sys
> -
> -sys.stderr.write('WARNING: git-remote-bzr is now maintained independently.\n')
> -sys.stderr.write('WARNING: For more information visit https://github.com/felipec/git-remote-bzr\n')
> -
> -sys.stderr.write('''WARNING:
> +cat <<'EOT'
> +WARNING: git-remote-bzr is now maintained independently.
> +WARNING: For more information visit https://github.com/felipec/git-remote-bzr
> +WARNING:
> WARNING: You can pick a directory on your $PATH and download it, e.g.:
> -WARNING: $ wget -O $HOME/bin/git-remote-bzr \\
> +WARNING: $ wget -O $HOME/bin/git-remote-bzr \
> WARNING: https://raw.github.com/felipec/git-remote-bzr/master/git-remote-bzr
> WARNING: $ chmod +x $HOME/bin/git-remote-bzr
> -''')
> +EOT
> diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
> index 4255ad6..dfda44f 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -1,13 +1,11 @@
> -#!/usr/bin/env python
> +#!/bin/sh
>
> -import sys
> -
> -sys.stderr.write('WARNING: git-remote-hg is now maintained independently.\n')
> -sys.stderr.write('WARNING: For more information visit https://github.com/felipec/git-remote-hg\n')
> -
> -sys.stderr.write('''WARNING:
> +cat <<'EOT'
> +WARNING: git-remote-hg is now maintained independently.
> +WARNING: For more information visit https://github.com/felipec/git-remote-hg
> +WARNING:
> WARNING: You can pick a directory on your $PATH and download it, e.g.:
> -WARNING: $ wget -O $HOME/bin/git-remote-hg \\
> +WARNING: $ wget -O $HOME/bin/git-remote-hg \
> WARNING: https://raw.github.com/felipec/git-remote-hg/master/git-remote-hg
> WARNING: $ chmod +x $HOME/bin/git-remote-hg
> -''')
> +EOT
>
> --
> https://github.com/git/git/pull/333
>
^ permalink raw reply
* Re: [PATCH v5 19/24] refs: new transaction related ref-store api
From: Michael Haggerty @ 2017-03-03 15:48 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Johannes Schindelin, Ramsay Jones, Stefan Beller,
novalis
In-Reply-To: <20170222140450.30886-20-pclouds@gmail.com>
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> The transaction struct now takes a ref store at creation and will
> operate on that ref store alone.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs.c | 54 ++++++++++++++++++++++++++++++++++++++++------------
> refs.h | 8 ++++++++
> refs/refs-internal.h | 1 +
> 3 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 9137ac283..851b5e125 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -618,16 +618,19 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1
> return 0;
> }
>
> -int delete_ref(const char *refname, const unsigned char *old_sha1,
> - unsigned int flags)
> +int refs_delete_ref(struct ref_store *refs, const char *refname,
> + const unsigned char *old_sha1,
> + unsigned int flags)
> {
> struct ref_transaction *transaction;
> struct strbuf err = STRBUF_INIT;
>
> - if (ref_type(refname) == REF_TYPE_PSEUDOREF)
> + if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
> + assert(refs == get_main_ref_store());
Hmmm, I would have expected this to be checked via `(refs.flags &
REF_STORE_MAIN)`. I guess this is temporary; once we have compound ref
stores either version will need to be changed again. So I don't see a
strong argument for one vs. the other.
It might be more natural to make `delete_pseudoref()` take a `refs`
argument and do the check internally.
(Same comments below where `write_pseudoref()` is called.)
Michael
> return delete_pseudoref(refname, old_sha1);
> + }
>
> - transaction = ref_transaction_begin(&err);
> + transaction = ref_store_transaction_begin(refs, &err);
> if (!transaction ||
> ref_transaction_delete(transaction, refname, old_sha1,
> flags, NULL, &err) ||
> @@ -642,6 +645,13 @@ int delete_ref(const char *refname, const unsigned char *old_sha1,
> return 0;
> }
>
> +int delete_ref(const char *refname, const unsigned char *old_sha1,
> + unsigned int flags)
> +{
> + return refs_delete_ref(get_main_ref_store(), refname,
> + old_sha1, flags);
> +}
> +
> int copy_reflog_msg(char *buf, const char *msg)
> {
> char *cp = buf;
> @@ -801,11 +811,20 @@ int read_ref_at(const char *refname, unsigned int flags, unsigned long at_time,
> return 1;
> }
>
> -struct ref_transaction *ref_transaction_begin(struct strbuf *err)
> +struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
> + struct strbuf *err)
> {
> + struct ref_transaction *tr;
> assert(err);
>
> - return xcalloc(1, sizeof(struct ref_transaction));
> + tr = xcalloc(1, sizeof(struct ref_transaction));
> + tr->ref_store = refs;
> + return tr;
> +}
> +
> +struct ref_transaction *ref_transaction_begin(struct strbuf *err)
> +{
> + return ref_store_transaction_begin(get_main_ref_store(), err);
> }
>
> void ref_transaction_free(struct ref_transaction *transaction)
> @@ -922,18 +941,20 @@ int update_ref_oid(const char *msg, const char *refname,
> old_oid ? old_oid->hash : NULL, flags, onerr);
> }
>
> -int update_ref(const char *msg, const char *refname,
> - const unsigned char *new_sha1, const unsigned char *old_sha1,
> - unsigned int flags, enum action_on_err onerr)
> +int refs_update_ref(struct ref_store *refs, const char *msg,
> + const char *refname, const unsigned char *new_sha1,
> + const unsigned char *old_sha1, unsigned int flags,
> + enum action_on_err onerr)
> {
> struct ref_transaction *t = NULL;
> struct strbuf err = STRBUF_INIT;
> int ret = 0;
>
> if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
> + assert(refs == get_main_ref_store());
> ret = write_pseudoref(refname, new_sha1, old_sha1, &err);
> } else {
> - t = ref_transaction_begin(&err);
> + t = ref_store_transaction_begin(refs, &err);
> if (!t ||
> ref_transaction_update(t, refname, new_sha1, old_sha1,
> flags, msg, &err) ||
> @@ -964,6 +985,15 @@ int update_ref(const char *msg, const char *refname,
> return 0;
> }
>
> +int update_ref(const char *msg, const char *refname,
> + const unsigned char *new_sha1,
> + const unsigned char *old_sha1,
> + unsigned int flags, enum action_on_err onerr)
> +{
> + return refs_update_ref(get_main_ref_store(), msg, refname, new_sha1,
> + old_sha1, flags, onerr);
> +}
> +
> char *shorten_unambiguous_ref(const char *refname, int strict)
> {
> int i;
> @@ -1600,7 +1630,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
> int ref_transaction_commit(struct ref_transaction *transaction,
> struct strbuf *err)
> {
> - struct ref_store *refs = get_main_ref_store();
> + struct ref_store *refs = transaction->ref_store;
>
> return refs->be->transaction_commit(refs, transaction, err);
> }
> @@ -1719,7 +1749,7 @@ int reflog_expire(const char *refname, const unsigned char *sha1,
> int initial_ref_transaction_commit(struct ref_transaction *transaction,
> struct strbuf *err)
> {
> - struct ref_store *refs = get_main_ref_store();
> + struct ref_store *refs = transaction->ref_store;
>
> return refs->be->initial_transaction_commit(refs, transaction, err);
> }
> diff --git a/refs.h b/refs.h
> index 70d4eb87c..342cecd23 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -330,6 +330,9 @@ int reflog_exists(const char *refname);
> * exists, regardless of its old value. It is an error for old_sha1 to
> * be NULL_SHA1. flags is passed through to ref_transaction_delete().
> */
> +int refs_delete_ref(struct ref_store *refs, const char *refname,
> + const unsigned char *old_sha1,
> + unsigned int flags);
> int delete_ref(const char *refname, const unsigned char *old_sha1,
> unsigned int flags);
>
> @@ -414,6 +417,8 @@ enum action_on_err {
> * Begin a reference transaction. The reference transaction must
> * be freed by calling ref_transaction_free().
> */
> +struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs,
> + struct strbuf *err);
> struct ref_transaction *ref_transaction_begin(struct strbuf *err);
>
> /*
> @@ -548,6 +553,9 @@ void ref_transaction_free(struct ref_transaction *transaction);
> * ref_transaction_update(). Handle errors as requested by the `onerr`
> * argument.
> */
> +int refs_update_ref(struct ref_store *refs, const char *msg, const char *refname,
> + const unsigned char *new_sha1, const unsigned char *old_sha1,
> + unsigned int flags, enum action_on_err onerr);
> int update_ref(const char *msg, const char *refname,
> const unsigned char *new_sha1, const unsigned char *old_sha1,
> unsigned int flags, enum action_on_err onerr);
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 5f26208c2..690498698 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -200,6 +200,7 @@ enum ref_transaction_state {
> * as atomically as possible. This structure is opaque to callers.
> */
> struct ref_transaction {
> + struct ref_store *ref_store;
> struct ref_update **updates;
> size_t alloc;
> size_t nr;
>
^ permalink raw reply
* Re: [PATCH 3/3] Remove outdated info in difftool manpage
From: Johannes Schindelin @ 2017-03-03 15:46 UTC (permalink / raw)
To: Denton Liu; +Cc: git, davvid
In-Reply-To: <20170303115751.GA13225@arch-attack.localdomain>
Hi Denton (or should I address you as Liu?),
On Fri, 3 Mar 2017, Denton Liu wrote:
> When difftool was rewritten in C, it removed the capability to read
> fallback configs from mergetool. This changes the documentation to
> reflect this.
Thanks for pointing that out. But that is probably an oversight on my
part, not an intentional change...
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v5 20/24] files-backend: avoid ref api targetting main ref store
From: Michael Haggerty @ 2017-03-03 16:03 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Johannes Schindelin, Ramsay Jones, Stefan Beller,
novalis
In-Reply-To: <20170222140450.30886-21-pclouds@gmail.com>
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> A small step towards making files-backend works as a non-main ref store
> using the newly added store-aware API.
>
> For the record, `join` and `nm` on refs.o and files-backend.o tell me
> that files-backend no longer uses functions that defaults to
> get_main_ref_store().
Nice!
> I'm not yet comfortable at the idea of removing
> files_assert_main_repository() (or converting REF_STORE_MAIN to
> REF_STORE_WRITE). More staring and testing is required before that can
> happen. Well, except peel_ref(). I'm pretty sure that function is safe.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs/files-backend.c | 85 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 49 insertions(+), 36 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index dafddefd3..09c280fd3 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> [...]
> @@ -3873,8 +3883,10 @@ static int files_transaction_commit(struct ref_store *ref_store,
> * head_ref within the transaction, then split_head_update()
> * arranges for the reflog of HEAD to be updated, too.
> */
> - head_ref = resolve_refdup("HEAD", RESOLVE_REF_NO_RECURSE,
> - head_oid.hash, &head_type);
> + head_ref = (char *)refs_resolve_ref_unsafe(ref_store, "HEAD",
> + RESOLVE_REF_NO_RECURSE,
> + head_oid.hash, &head_type);
> + head_ref = xstrdup_or_null(head_ref);
If you combine the last two statements, you can avoid the ugly cast.
> [...]
Michael
^ permalink raw reply
* Re: log -S/-G (aka pickaxe) searches binary files by default
From: Junio C Hamano @ 2017-03-03 16:07 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Braun, GIT Mailing-list
In-Reply-To: <20170303051721.r6pahs4vjtqqoevc@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Mar 02, 2017 at 05:36:17PM -0800, Junio C Hamano wrote:
> ...
>> > Is that on purpose?
>>
>> No, it's a mere oversight (as I do not think I never even thought
>> about special casing binary
>> files from day one, it is unlikely that you would find _any_ old
>> version of Git that behaves
>> differently).
>
> The email focuses on "-G", and I think it is wrong to look in binary
> files there, as "grep in diff" does not make sense for a binary file
> that we would refuse to diff.
Yeah, I agree.
> But the subject also mentions "-S". I always assumed it was intentional
> to look in binary files there, as it is searching for a pure byte
> sequence. I would not mind an option to disable that, but I think the
> default should remain on.
As the feature was built to be one of the core ingredients necessary
towards the 'ideal SCM' envisioned in
<http://public-inbox.org/git/Pine.LNX.4.58.0504150753440.7211@ppc970.osdl.org>
"-S" is about finding "a block of text". It was merely an oversight
that we didn't add explicit code to ignore binary when we introduced
the concept of "is this text? is it worth finding things in and
diffing binary files?".
I do agree that it may be too late and/or disruptive to change its
behaviour now, as people may grew expectations different from the
original motivation and design, though.
^ permalink raw reply
* bisect-helper: we do not bisect --objects
From: Junio C Hamano @ 2017-03-03 16:16 UTC (permalink / raw)
To: git, Christian Couder
Ever since "bisect--helper" was introduced in 1bf072e366
("bisect--helper: implement "git bisect--helper"", 2009-03-26),
after setting up the "rev-list $bad --not $good_ones" machinery, the
code somehow prepared to mark the trees and blobs at the good boundary
as uninteresting, only when --objects option was given. This was kept
across a bit of refactoring done by 2ace9727be ("bisect: move common
bisect functionality to "bisect_common"", 2009-04-19) and survives
to this day.
However, "git bisect" does not care about tree/blob object
reachability at all---it purely works at the commit DAG level and
nobody passes (and nobody should pass) "--objects" option to the
underlying rev-list machinery. Remove the dead code.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Christian, do you recall what we were thinking when we added this
mark_edges_uninteresting() call in this program? If you don't,
don't worry--this was done more than 8 years ago. I am just
being curious and also a bit being cautious in case I am missing
something.
Thanks.
bisect.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/bisect.c b/bisect.c
index 30808cadf7..86c5929a23 100644
--- a/bisect.c
+++ b/bisect.c
@@ -634,8 +634,6 @@ static void bisect_common(struct rev_info *revs)
{
if (prepare_revision_walk(revs))
die("revision walk setup failed");
- if (revs->tree_objects)
- mark_edges_uninteresting(revs, NULL);
}
static void exit_if_skipped_commits(struct commit_list *tried,
^ permalink raw reply related
* rebase: confusing behaviour since --fork-point
From: Laszlo Kiss @ 2017-03-03 16:37 UTC (permalink / raw)
To: git; +Cc: ldubox-coding101
Hi all,
This will be a bit long, sorry in advance.
I've hit a problem with how rebase works since the introduction of the
--fork-point option. I initially thought it was a bug until the kind
folks over at git-for-windows patiently told me otherwise.
Consider the following:
-------8<-------
# On branch master which is at origin/master
# hack hack hack
git commit -am'First topic commit'
# optionally followed by more commits
# realize I want all this on a different branch instead
# (maybe I just forgot to create one; would be typical)
git checkout -b topic --track
git branch -f master origin/master
# optionally add more commits to 'topic' and/or update 'master'
# with changes to 'origin/master', then finally:
git rebase # or git pull --rebase
------->8-------
What happens is that 'First topic commit' is discarded because rebase,
with this syntax, defaults to --fork-point, which then works out from
master's reflog that the commit in question was previously part of the
upstream branch. The rebase is then carried out under the assumption
that, even though that specific commit is no longer in master,
something equivalent to it or superseding it is, or else it is meant to
be dropped - which would likely be the case if a complicated history
rewrite happened, but is not the case here.
There are two reasons why I think this is confusing and should be
changed:
(1) My mental model of git after some 5 years of using it is such that
I view the reflog as auxiliary information for purposes of mining
history or recovering from mistakes, and I rely only on the
contents and parent-child relationships of commits to work out what
a command I'm about to run will do. I would expect the vast
majority of users to have a similar mental model, in which case the
fact that the same commits can be rebased differently depending on
the data in the reflog will be equally confusing to those users.
(2) Most people don't like to rebase too often, consequently the time
elapsed from a user creating the topic branch and resetting its
upstream until actually rebasing it can be days or weeks, by the
end of which they are unlikely to remember the circumstances of
creating the branch. Even more alarmingly, they may have committed
dozens of further changes, and therefore may not even notice that
the first few of those silently disappeared. (I'm not making this
up: see discussion links below in PS.)
I believe the correct design would be to always make --no-fork-point
the default for rebase, and only use --fork-point when explicitly
specified. This would potentially be inconvenient for those who rebase
on top of complicated history rewrites, but said inconvenience would be
mitigated by several factors:
- Anyone doing rebases like that will already know that they can go
wrong in a million ways.
- Perhaps more crucially, they will have a way of noticing if it went
wrong, with all relevant information in short-term memory, so would
recover easily.
- If the rebase turns out to be really complex, they are likely to
resort to rebase -i, which shows full details of what is about to
happen, moreover it seems relatively simple to enhance the contents
of the rebase-todo file so that full information about the merge-base
and fork-point is readily available.
If changing the default is not an option, e.g. because of backwards
compatibility concerns, then some configurability could still be
helpful, e.g. rebase.useForkPoint = never / auto / always (default
auto, to keep the current behaviour). Although I suspect this would
just lead to everyone suggesting setting this to 'never'. In any case
this would provide a way to ensure that any git newbies in my
organization don't spend days trying to figure this out like I've just
done.
Assuming there is agreement to do one of the above (I don't even know
whose agreement is required), what's the process for getting it
implemented? Sorry, that probably counts as a dumb question, but I've
never been around open-source projects much & need someone to show me
the ropes.
Many thanks
Laszlo
PS. Further reading about the same topic if anyone is interested:
- http://marc.info/?l=git&m=140991293402880&w=2 (from this same mailing
list 2+ years ago, but I can see no clear conclusion)
- https://github.com/git-for-windows/git/issues/1076 (my bug report
where contacting this list was suggested)
- http://stackoverflow.com/questions/22790765 and
http://stackoverflow.com/questions/35320740 (various SO users being
confused and asking about / discussing the same thing)
^ permalink raw reply
* Re: [PATCH v5 23/24] t1405: some basic tests on main ref store
From: Michael Haggerty @ 2017-03-03 16:43 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Johannes Schindelin, Ramsay Jones, Stefan Beller,
novalis
In-Reply-To: <20170222140450.30886-24-pclouds@gmail.com>
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> t/t1405-main-ref-store.sh (new +x) | 123 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 123 insertions(+)
> create mode 100755 t/t1405-main-ref-store.sh
>
> diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
> new file mode 100755
> index 000000000..0999829f1
> --- /dev/null
> +++ b/t/t1405-main-ref-store.sh
> @@ -0,0 +1,123 @@
> +#!/bin/sh
> +
> +test_description='test main ref store api'
> +
> +. ./test-lib.sh
> +
> +RUN="test-ref-store main"
> +
> +test_expect_success 'pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE)' '
> + test_commit one &&
> + N=`find .git/refs -type f | wc -l` &&
I think we prefer $() to ``. (Same throughout this file and also t1406
in the next commit.)
It's notable that these tests grep around the filesystem, so they won't
be applicable to future refs backends. Of course, "pack-refs" is
intrinsically only applicable to the files backend, so for this test
it's not surprising. But some tests could conceivably be written in a
generic way, so that they should pass for any refs backend.
Just food for thought; no need to change anything now.
> + test "$N" != 0 &&
> + $RUN pack-refs 3 &&
> + N=`find .git/refs -type f | wc -l`
Didn't you mean to test the final `$N`?
> +'
> +
> +test_expect_success 'peel_ref(new-tag)' '
> + git rev-parse HEAD >expected &&
> + git tag -a -m new-tag new-tag HEAD &&
> + $RUN peel-ref refs/tags/new-tag >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'create_symref(FOO, refs/heads/master)' '
> + $RUN create-symref FOO refs/heads/master nothing &&
> + echo refs/heads/master >expected &&
> + git symbolic-ref FOO >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '
> + git rev-parse FOO -- &&
> + git rev-parse refs/tags/new-tag -- &&
> + $RUN delete-refs 0 FOO refs/tags/new-tag &&
> + test_must_fail git rev-parse FOO -- &&
> + test_must_fail git rev-parse refs/tags/new-tag --
> +'
> +
> +test_expect_success 'rename_refs(master, new-master)' '
> + git rev-parse master >expected &&
> + $RUN rename-ref refs/heads/master refs/heads/new-master &&
> + git rev-parse new-master >actual &&
> + test_cmp expected actual &&
> + test_commit recreate-master
> +'
Isn't HEAD set to `refs/heads/master` prior to this test? If so, then I
think it should become detached when you rename `refs/heads/master`. Or
maybe it should be changed to point at `refs/heads/new-master`, I can't
remember. In either case, it might be useful for the test to check that
the behavior matches the status quo, so we notice if the behavior ever
changes inadvertently.
> +
> +test_expect_success 'for_each_ref(refs/heads/)' '
> + $RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
> + cat >expected <<-\EOF &&
> + master 0x0
> + new-master 0x0
> + EOF
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'resolve_ref(new-master)' '
> + SHA1=`git rev-parse new-master` &&
> + echo "$SHA1 refs/heads/new-master 0x0" >expected &&
> + $RUN resolve-ref refs/heads/new-master 0 >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'verify_ref(new-master)' '
> + $RUN verify-ref refs/heads/new-master
> +'
> +
> +test_expect_success 'for_each_reflog()' '
> + $RUN for-each-reflog | cut -c 42- >actual &&
> + cat >expected <<-\EOF &&
> + refs/heads/master 0x0
> + refs/heads/new-master 0x0
> + HEAD 0x1
> + EOF
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'for_each_reflog_ent()' '
> + $RUN for-each-reflog-ent HEAD >actual &&
> + head -n1 actual | grep one &&
> + tail -n2 actual | head -n1 | grep recreate-master
> +'
> +
> +test_expect_success 'for_each_reflog_ent_reverse()' '
> + $RUN for-each-reflog-ent-reverse HEAD >actual &&
> + head -n1 actual | grep recreate-master &&
> + tail -n2 actual | head -n1 | grep one
> +'
> +
> +test_expect_success 'reflog_exists(HEAD)' '
> + $RUN reflog-exists HEAD
> +'
> +
> +test_expect_success 'delete_reflog(HEAD)' '
> + $RUN delete-reflog HEAD &&
> + ! test -f .git/logs/HEAD
Maybe also test the final state using `reflog-exists` to make sure that
function can also return false?
> +'
> +
> +test_expect_success 'create-reflog(HEAD)' '
> + $RUN create-reflog HEAD 1 &&
> + test -f .git/logs/HEAD
> +'
> +
> +test_expect_success 'delete_ref(refs/heads/foo)' '
> + git checkout -b foo &&
> + FOO_SHA1=`git rev-parse foo` &&
> + git checkout --detach &&
> + test_commit bar-commit &&
> + git checkout -b bar &&
> + BAR_SHA1=`git rev-parse bar` &&
> + $RUN update-ref updating refs/heads/foo $BAR_SHA1 $FOO_SHA1 0 &&
> + echo $BAR_SHA1 >expected &&
> + git rev-parse refs/heads/foo >actual &&
> + test_cmp expected actual
> +'
> +
> +test_expect_success 'delete_ref(refs/heads/foo)' '
> + SHA1=`git rev-parse foo` &&
> + git checkout --detach &&
> + $RUN delete-ref refs/heads/foo $SHA1 0 &&
> + test_must_fail git rev-parse refs/heads/foo --
> +'
The last two tests have the same name.
You might also want to test these functions when the old oid is
incorrect or when the reference is missing.
> +test_done
Thanks for all the new tests!
Michael
^ permalink raw reply
* Re: [PATCH v5 00/24] Remove submodule from files-backend.c
From: Michael Haggerty @ 2017-03-03 16:54 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Johannes Schindelin, Ramsay Jones, Stefan Beller,
novalis
In-Reply-To: <20170222140450.30886-1-pclouds@gmail.com>
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> v5 goes a bit longer than v4, basically:
I've reviewed all of the patches and left a bunch of comments, mostly
superficial. I'm very happy about the way it's going, and especially
want to say how much I appreciate that you've done so much legwork and
added so many tests.
Michael
^ permalink raw reply
* Re: [PATCH v2 8/9] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2017-03-03 15:26 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170303050630.mw6asla65prku3vq@sigill.intra.peff.net>
Hi Peff,
On Fri, 3 Mar 2017, Jeff King wrote:
> I think the "dirty hack..." comment in read_early_config() can be
> condensed (or go away) now.
Yes, I made that change in response to a comment you made about an earlier
patch in this series.
> I think we _could_ do away with read_early_config() entirely, and just
> have the regular config code do this lookup when we're not already in a
> repo. But then we'd really need to depend on the "creating_repository"
> flag being managed correctly.
Well, that would be a major design change. I'm not really all that
comfortable with that...
> I think I prefer the idea that a few "early" spots like pager and alias
> config need to use this special function to access the config. That's
> less likely to cause surprises when some config option is picked up
> before we have run setup_git_directory().
Exactly. There is semantic meaning in calling read_early_config() vs
git_config().
> There is one surprising case that I think we need to deal with even now,
> though. If I do:
>
> git init repo
> git -C repo config pager.upload-pack 'echo whoops'
> git upload-pack repo
> cd repo && git upload-pack .
>
> the first one is OK, but the second reads and executes the pager config
> from the repo, even though we usually consider upload-pack to be OK to
> run in an untrusted repo. This _isn't_ a new thing in your patch, but
> just something I noticed while we are here.
>
> And maybe it is a non-issue. The early-config bits all happen via the
> git wrapper, and normally we use the direct dashed "git-upload-pack" to
> access the other repositories. Certainly I have been known to use "git
> -c ... upload-pack" while debugging various things.
>
> So I dunno. You really have to jump through some hoops for it to matter,
> but I just wonder if the git wrapper should be special-casing
> upload-pack, too.
While I agree that it may sound surprising, I would like to point out that
there *is* a semantic difference between `git upload-pack repo` and `git
-C repo upload-pack .`: the working directory is different. And given that
so much in Git's operations depend on the working directory, it makes
sense that those two invocations have slightly different semantics, too.
I also agree, obviously, that this is something that would need a separate
patch series to tackle ;-)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Mike Crowe @ 2017-03-03 17:01 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <5d92d3b8-f438-9be5-9742-22f8cd8fe03d@web.de>
Hi Torsten,
Your patch has been superseded, but I thought I ought to answer your
questions rather than leave them hanging.
On Thursday 02 March 2017 at 19:17:00 +0100, Torsten Bögershausen wrote:
> On 2017-03-01 22:25, Mike Crowe wrote:
> > On Wednesday 01 March 2017 at 18:04:44 +0100, tboegi@web.de wrote:
> >> From: Junio C Hamano <gitster@pobox.com>
> >>
> >> git diff --quiet may take a short-cut to see if a file is changed
> >> in the working tree:
> >> Whenever the file size differs from what is recorded in the index,
> >> the file is assumed to be changed and git diff --quiet returns
> >> exit with code 1
> >>
> >> This shortcut must be suppressed whenever the line endings are converted
> >> or a filter is in use.
> >> The attributes say "* text=auto" and a file has
> >> "Hello\nWorld\n" in the index with a length of 12.
> >> The file in the working tree has "Hello\r\nWorld\r\n" with a length of 14.
> >> (Or even "Hello\r\nWorld\n").
> >> In this case "git add" will not do any changes to the index, and
> >> "git diff -quiet" should exit 0.
> >>
> >> Add calls to would_convert_to_git() before blindly saying that a different
> >> size means different content.
> >>
> >> Reported-By: Mike Crowe <mac@mcrowe.com>
> >> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> >> ---
> >> This is what I can come up with, collecting all the loose ends.
> >> I'm not sure if Mike wan't to have the Reported-By with a
> >> Signed-off-by ?
> >> The other question is, if the commit message summarizes the discussion
> >> well enough ?
> >>
> >> diff.c | 18 ++++++++++++++----
> >> t/t0028-diff-converted.sh | 27 +++++++++++++++++++++++++++
> >> 2 files changed, 41 insertions(+), 4 deletions(-)
> >> create mode 100755 t/t0028-diff-converted.sh
> >>
> >> diff --git a/diff.c b/diff.c
> >> index 051761b..c264758 100644
> >> --- a/diff.c
> >> +++ b/diff.c
> >> @@ -4921,9 +4921,10 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
> >> * differences.
> >> *
> >> * 2. At this point, the file is known to be modified,
> >> - * with the same mode and size, and the object
> >> - * name of one side is unknown. Need to inspect
> >> - * the identical contents.
> >> + * with the same mode and size, the object
> >> + * name of one side is unknown, or size comparison
> >> + * cannot be depended upon. Need to inspect the
> >> + * contents.
> >> */
> >> if (!DIFF_FILE_VALID(p->one) || /* (1) */
> >> !DIFF_FILE_VALID(p->two) ||
> >> @@ -4931,7 +4932,16 @@ static int diff_filespec_check_stat_unmatch(struct diff_filepair *p)
> >> (p->one->mode != p->two->mode) ||
> >> diff_populate_filespec(p->one, CHECK_SIZE_ONLY) ||
> >> diff_populate_filespec(p->two, CHECK_SIZE_ONLY) ||
> >> - (p->one->size != p->two->size) ||
> >> +
> >> + /*
> >> + * only if eol and other conversions are not involved,
> >> + * we can say that two contents of different sizes
> >> + * cannot be the same without checking their contents.
> >> + */
> >> + (!would_convert_to_git(p->one->path) &&
> >> + !would_convert_to_git(p->two->path) &&
> >> + (p->one->size != p->two->size)) ||
> >> +
> >> !diff_filespec_is_identical(p->one, p->two)) /* (2) */
> >> p->skip_stat_unmatch_result = 1;
> >> return p->skip_stat_unmatch_result;
> >> diff --git a/t/t0028-diff-converted.sh b/t/t0028-diff-converted.sh
> >> new file mode 100755
> >> index 0000000..3d5ab95
> >> --- /dev/null
> >> +++ b/t/t0028-diff-converted.sh
> >> @@ -0,0 +1,27 @@
> >> +#!/bin/sh
> >> +#
> >> +# Copyright (c) 2017 Mike Crowe
> >> +#
> >> +# These tests ensure that files changing line endings in the presence
> >> +# of .gitattributes to indicate that line endings should be ignored
> >> +# don't cause 'git diff' or 'git diff --quiet' to think that they have
> >> +# been changed.
> >> +
> >> +test_description='git diff with files that require CRLF conversion'
> >> +
> >> +. ./test-lib.sh
> >> +
> >> +test_expect_success setup '
> >> + echo "* text=auto" >.gitattributes &&
> >> + printf "Hello\r\nWorld\r\n" >crlf.txt &&
> >> + git add .gitattributes crlf.txt &&
> >> + git commit -m "initial"
> >> +'
> >> +
> >> +test_expect_success 'quiet diff works on file with line-ending change that has no effect on repository' '
> >> + printf "Hello\r\nWorld\n" >crlf.txt &&
> >> + git status &&
> >> + git diff --quiet
> >> +'
> >> +
> >> +test_done
> >
[snip]
> > Also, I think I've found a behaviour change with this fix. Consider:
> >
> > echo "* text=auto" >.gitattributes
> > printf "Hello\r\nWorld\r\n" >crlf.txt
> That should give
> "Hello\nWorld\n" in the index:
>
> git add .gitattributes crlf.txt
> warning: CRLF will be replaced by LF in ttt/crlf.txt.
> The file will have its original line endings in your working directory.
> tb@mac:/tmp/ttt> git commit -m "initial"
> [master (root-commit) 354f657] initial
> 2 files changed, 3 insertions(+)
> create mode 100644 ttt/.gitattributes
> create mode 100644 ttt/crlf.txt
> tb@mac:/tmp/ttt> git ls-files --eol
> i/lf w/lf attr/text=auto .gitattributes
> i/lf w/crlf attr/text=auto crlf.txt
> tb@mac:/tmp/ttt>
>
> > git add .gitattributes crlf.txt
> > git commit -m "initial"
> >
> > printf "\r\n" >>crlf.txt
> >
> > With the above patch, both "git diff" and "git diff --quiet" report that
> > there are no changes. Previously Git would report the extra newline
> > correctly.
> Wait a second.
> Which extra newline "correctly" ?
The extra newline I appended with the printf "\r\n" >> crlf.txt
> The "git diff" command is about the changes which will be done to the index.
> Regardless if you have any of these in the working tree on disk:
>
> "Hello\nWorld\n"
> "Hello\nWorld\r\n"
> "Hello\r\nWorld\n"
> "Hello\r\nWorld\r\n"
>
> "git status" and "git diff --quiet"
> should not report any changes.
But I didn't have any of those. I ended up with:
"Hello\nWorld\n"
in the index, and
"Hello\r\nWorld\r\n\r\n"
in the working tree, but the extra newline was not reported by git diff.
> So I don't know if there is a mis-understanding about "git diff" on your side,
> or if I miss something.
I don't think it matters any more since Junio's patch didn't suffer from
this problem.
Thanks.
Mike.
^ permalink raw reply
* Re: [PATCH v5 24/24] t1406: new tests for submodule ref store
From: Michael Haggerty @ 2017-03-03 16:51 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, git
Cc: Junio C Hamano, Johannes Schindelin, Ramsay Jones, Stefan Beller,
novalis
In-Reply-To: <20170222140450.30886-25-pclouds@gmail.com>
On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> t/t1406-submodule-ref-store.sh (new +x) | 95 +++++++++++++++++++++++++++++++++
> 1 file changed, 95 insertions(+)
> create mode 100755 t/t1406-submodule-ref-store.sh
I wonder if you could reduce some of the repetition between this test
file and t1405, at least for the functions that should work for both the
main repository and for submodules? For example, if you were to define
GIT='git'
in one case and
GIT='git -C sub'
in the other, then maybe some of the setup code could be made to look
alike? Then the division could be one test file for the functions that
work for both main and submodules, and a second for the functions that
only work for main. Just a thought...
Michael
^ permalink raw reply
* [PATCH v3 0/9] Fix the early config
From: Johannes Schindelin @ 2017-03-03 17:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488506615.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, or 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 dirty little secret is that I need to discover the Git directory
early, without changing global state, for usage statistics gathering in
the GVFS Git project, so I actually do not care all that much about the
early config, although it is a welcome fallout (and a good reason for
accepting these patches and thereby releasing me of more maintenance
burden :-)).
Notable notes:
- In contrast to earlier versions, I no longer special-case init and
clone. Peff pointed out that this adds technical debt, and that we can
actually argue (for consistency's sake) that early config reads the
current repository config (if any) even for init and clone.
- The read_early_config() function does not cache Git directory
discovery nor read values. If needed, this can be implemented later,
in a separate patch series.
- 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 v2:
- replaced `test -e` by `test_path_is_file`
- fixed premature "cwd -> dir" in 2/9
- the setup_git_directory_gently_1() function is no longer renamed
because it is not exported directly, anyway
- fixed the way setup_discovered_git_dir() expected the offset parameter
to exclude the trailing slash (which is not true for root
directories); also verified that setup_bare_git_dir() does not require
a corresponding patch
- switched to using size_t instead of int to save the length of the
strbuf in discover_git_directory()
- ensured that discover_git_directory() turns a relative gitdir into an
absolute one even if there is already some text in the strbuf
- clarified under which circumstances we turn a relative gitdir into an
absolute one
- avoided absolute gitdir with trailing "/." to be returned
- the commit that fixes the "really dirty hack" now rewords that comment
to reflect that it is no longer a really dirty hack
- dropped the special-casing of init and clone
- the discover_git_directory() function now correctly checks the
repository version, warning (and returning NULL) in case of a problem
Johannes Schindelin (9):
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
Export 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()
cache.h | 3 +
config.c | 27 ++++++
pager.c | 31 -------
setup.c | 237 ++++++++++++++++++++++++++++++++----------------
t/helper/test-config.c | 15 +++
t/t1309-early-config.sh | 50 ++++++++++
t/t7006-pager.sh | 18 +++-
7 files changed, 269 insertions(+), 112 deletions(-)
create mode 100755 t/t1309-early-config.sh
base-commit: 3bc53220cb2dcf709f7a027a3f526befd021d858
Published-As: https://github.com/dscho/git/releases/tag/early-config-v3
Fetch-It-Via: git fetch https://github.com/dscho/git early-config-v3
Interdiff vs v2:
diff --git a/cache.h b/cache.h
index 0af7141242f..8a4580f921d 100644
--- a/cache.h
+++ b/cache.h
@@ -518,6 +518,7 @@ 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 */
extern const char *discover_git_directory(struct strbuf *gitdir);
extern const char *setup_git_directory_gently(int *);
extern const char *setup_git_directory(void);
@@ -2070,7 +2071,7 @@ const char *split_cmdline_strerror(int cmdline_errno);
/* setup.c */
struct startup_info {
- int have_repository, creating_repository;
+ int have_repository;
const char *prefix;
};
extern struct startup_info *startup_info;
diff --git a/config.c b/config.c
index bcda397d42e..749623a9649 100644
--- a/config.c
+++ b/config.c
@@ -1419,25 +1419,16 @@ 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).
+ * 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)) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
diff --git a/git.c b/git.c
index 9fb9bb90a21..33f52acbcc8 100644
--- a/git.c
+++ b/git.c
@@ -337,9 +337,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
struct stat st;
const char *prefix;
- if (p->fn == cmd_init_db || p->fn == cmd_clone)
- startup_info->creating_repository = 1;
-
prefix = NULL;
help = argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
diff --git a/setup.c b/setup.c
index 7ceca6cc6ef..5320ae37314 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;
}
@@ -839,8 +841,8 @@ enum discovery_result {
* the discovered .git/ directory, if any. This path may be relative against
* `dir` (i.e. *not* necessarily the cwd).
*/
-static enum discovery_result discover_git_directory_1(struct strbuf *dir,
- struct strbuf *gitdir)
+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;
@@ -923,24 +925,44 @@ static enum discovery_result discover_git_directory_1(struct strbuf *dir,
const char *discover_git_directory(struct strbuf *gitdir)
{
- struct strbuf dir = STRBUF_INIT;
- int len;
+ 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;
- len = dir.len;
- if (discover_git_directory_1(&dir, gitdir) < 0) {
+ cwd_len = dir.len;
+ if (setup_git_directory_gently_1(&dir, gitdir) < 0) {
strbuf_release(&dir);
return NULL;
}
- if (dir.len < len && !is_absolute_path(gitdir->buf)) {
- strbuf_addch(&dir, '/');
- strbuf_insert(gitdir, 0, dir.buf, dir.len);
+ /*
+ * 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;
}
@@ -970,7 +992,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
die_errno(_("Unable to read current working directory"));
strbuf_addbuf(&dir, &cwd);
- switch (discover_git_directory_1(&dir, &gitdir)) {
+ switch (setup_git_directory_gently_1(&dir, &gitdir)) {
case GIT_DIR_NONE:
prefix = NULL;
break;
@@ -1000,7 +1022,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
"Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),
dir.buf);
default:
- die("BUG: unhandled discover_git_directory() result");
+ die("BUG: unhandled setup_git_directory_1() result");
}
if (prefix)
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index bf89340988b..4f3794d415e 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -387,7 +387,7 @@ test_expect_success TTY 'core.pager in repo config works and retains cwd' '
cd sub &&
rm -f cwd-retained &&
test_terminal git -p rev-parse HEAD &&
- test -e cwd-retained
+ test_path_is_file cwd-retained
)
'
--
2.12.0.windows.1.7.g94dafc3b124
^ permalink raw reply
* [PATCH v3 1/9] t7006: replace dubious test
From: Johannes Schindelin @ 2017-03-03 17:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 v3 2/9] setup_git_directory(): use is_dir_sep() helper
From: Johannes Schindelin @ 2017-03-03 17:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index 967f289f1ef..c47619605fb 100644
--- a/setup.c
+++ b/setup.c
@@ -910,7 +910,8 @@ 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]));
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 v3 3/9] Prepare setup_discovered_git_directory() the root directory
From: Johannes Schindelin @ 2017-03-03 17:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 c47619605fb..91d884b6746 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 v3 4/9] setup_git_directory_1(): avoid changing global state
From: Johannes Schindelin @ 2017-03-03 17:32 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 | 189 ++++++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 113 insertions(+), 76 deletions(-)
diff --git a/setup.c b/setup.c
index 91d884b6746..9a09bb41ab5 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,62 +888,100 @@ 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) {
- ret = setup_discovered_git_dir(gitdirenv,
- &cwd, offset,
- nongit_ok);
- free(gitfile);
- return ret;
+ strbuf_addstr(gitdir, gitdirenv);
+ return GIT_DIR_DISCOVERED;
}
- free(gitfile);
- if (is_git_directory("."))
- return setup_bare_git_dir(&cwd, offset, nongit_ok);
-
- offset_parent = offset;
- while (--offset_parent > ceil_offset &&
- !is_dir_sep(cwd.buf[offset_parent]));
- 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);
+ if (is_git_directory(dir->buf)) {
+ strbuf_addstr(gitdir, ".");
+ return GIT_DIR_BARE;
}
- offset = offset_parent;
+
+ if (offset <= min_offset)
+ return GIT_DIR_HIT_CEILING;
+
+ while (--offset > ceil_offset && !is_dir_sep(dir->buf[offset]));
+ 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 v3 7/9] read_early_config(): avoid .git/config hack when unneeded
From: Johannes Schindelin @ 2017-03-03 17:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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
* [PATCH v3 5/9] Export the discover_git_directory() function
From: Johannes Schindelin @ 2017-03-03 17:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.git.johannes.schindelin@gmx.de>
The setup_git_directory_gently_1() function we modified earlier still
needs to return both the path to the .git/ directory as well as the
"cd-up" path to allow setup_git_directory() to retain its previous
behavior as if it changed the current working directory on its quest for
the .git/ directory.
This is 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 support early config reading
better.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 2 ++
setup.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/cache.h b/cache.h
index 80b6372cf76..4d8eb38de74 100644
--- a/cache.h
+++ b/cache.h
@@ -518,6 +518,8 @@ 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 */
+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 9a09bb41ab5..32ce023638a 100644
--- a/setup.c
+++ b/setup.c
@@ -923,6 +923,37 @@ 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;
+ size_t gitdir_offset = gitdir->len, cwd_len;
+
+ 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_release(&dir);
+
+ return gitdir->buf;
+}
+
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 v3 6/9] Make read_early_config() reusable
From: Johannes Schindelin @ 2017-03-03 17:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 4d8eb38de74..8a4580f921d 100644
--- a/cache.h
+++ b/cache.h
@@ -1799,6 +1799,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 v3 9/9] Test read_early_config()
From: Johannes Schindelin @ 2017-03-03 17:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 v3 8/9] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2017-03-03 17:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Duy Nguyen
In-Reply-To: <cover.1488562287.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 | 33 ++++++++++++++-------------------
setup.c | 14 +++++++++++++-
| 8 ++++----
3 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/config.c b/config.c
index 068fa4dcfa6..749623a9649 100644
--- a/config.c
+++ b/config.c
@@ -1414,34 +1414,29 @@ 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 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 (!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);
diff --git a/setup.c b/setup.c
index 32ce023638a..5320ae37314 100644
--- a/setup.c
+++ b/setup.c
@@ -925,8 +925,9 @@ 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;
+ 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;
@@ -949,8 +950,19 @@ const char *discover_git_directory(struct strbuf *gitdir)
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;
}
--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
* Re: [PATCH v1 1/1] git diff --quiet exits with 1 on clean tree with CRLF conversions
From: Torsten Bögershausen @ 2017-03-03 17:02 UTC (permalink / raw)
To: Mike Crowe, Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <20170302200356.GA31318@mcrowe.com>
Understood, thanks for the explanation.
quiet is not quite any more..
Does the following fix help ?
--- a/diff.c
+++ b/diff.c
@@ -2826,6 +2826,8 @@ int diff_populate_filespec(struct diff_filespec *s,
unsigned int flags)
enum safe_crlf crlf_warn = (safe_crlf == SAFE_CRLF_FAIL
? SAFE_CRLF_WARN
: safe_crlf);
+ if (size_only)
+ crlf_warn = SAFE_CRLF_FALSE;
^ permalink raw reply
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