* Re: Google Doc about the Contributors' Summit
From: Johannes Schindelin @ 2017-02-10 15:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7f4zqiyj.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 9 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > I just started typing stuff up in a Google Doc, and made it editable to
> > everyone, feel free to help and add things:
> >
> > https://docs.google.com/document/d/1KDoSn4btbK5VJCVld32he29U0pUeFGhpFxyx9ZJDDB0/edit?usp=sharing
>
> Thanks for a write-up, Dscho.
Technically, it is not a write-up, and I never meant it to be that. I
intended this document to help me remember what had been discussed, and I
doubt it is useful at all to anybody who has not been there.
I abused the Git mailing list to share that link, what I really should
have done is to use an URL shortener and jot the result down on the
whiteboard.
Very sorry for that,
Johannes
^ permalink raw reply
* Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]
From: Johannes Schindelin @ 2017-02-10 16:10 UTC (permalink / raw)
To: Arif Khokar
Cc: Jakub Narębski, Jeff King, Stefan Beller,
meta@public-inbox.org, git@vger.kernel.org, Eric Wong
In-Reply-To: <alpine.DEB.2.20.1608241509200.4924@virtualbox>
[-- Attachment #1: Type: text/plain, Size: 2311 bytes --]
Hi Arif,
On Wed, 24 Aug 2016, Johannes Schindelin wrote:
> On Tue, 23 Aug 2016, Arif Khokar wrote:
>
> > On 08/20/2016 03:57 PM, Jakub Narębski wrote:
> >
> > > But perhaps the problem is current lack of tooling in the opposite
> > > direction, namely getting patches from mailing list and applying
> > > them to GitHub repo, or Bitbucket, or GitLab. Though with working
> > > Git, it is something easier than sending patches via email; it is
> > > enough that email client can save email to a file (or better, whole
> > > sub-thread to file or files).
> >
> > Given that public-inbox provides an NNTP interface, couldn't the
> > ARTICLE <message-id> NNTP command be used to easily retrieve the
> > messages in a given patch series (at least compared to POP or IMAP).
> > Perhaps git-send-email could be modified to include the message-id
> > value of each patch in the series that it sends to the mailing list
> > and include it in the cover letter.
>
> I am no expert in the NNTP protocol (I abandoned News long ago), but if
> you go from HTML, you can automate the process without requiring changes
> in format-patch.
>
> > Then a script could be written (i.e., git-download-patch) which could
> > parse the cover letter message (specified using its message-id), and
> > download all the patches in series, which can then be applied using
> > git-am. This would in fact take the email client out of the equation
> > in terms of saving patches.
>
> I recently adapted an old script I had to apply an entire patch series
> given the GMane link to its cover letter:
>
> https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh
>
> Maybe you find it in you to adapt that to work with public-inbox.org?
Oh well. That would have been too easy a task, right?
As it happens, I needed this functionality myself (when reworking my
git-path-in-subdir patch to include Mike Rappazzo's previous patch series
that tried to fix the same bug).
I copy-edited the script to work with public-inbox.org, it accepts a
Message-ID or URL or GMane URL and will try to apply the patch (or patch
series) on top of the current revision:
https://github.com/git-for-windows/build-extra/blob/2268850552c7/apply-from-public-inbox.sh
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
From: Pranit Bauva @ 2017-02-10 15:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git List, Junio C Hamano
In-Reply-To: <a1b9143bb29a8a5979dd733ed20161e6769b2b83.1486736391.git.johannes.schindelin@gmx.de>
Hey Johannes,
On Fri, Feb 10, 2017 at 7:50 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
>
> It is curious that only MacOSX builds trigger an error about this, both
> GCC and Clang, but not Linux GCC nor Clang (see
> https://travis-ci.org/git/git/jobs/200182819#L1152 for details):
>
> builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used
> uninitialized whenever 'if' condition is true
> [-Werror,-Wsometimes-uninitialized]
> if (missing_good && !missing_bad && current_term &&
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> builtin/bisect--helper.c:350:7: note: uninitialized use occurs here
> if (!good_syn)
> ^~~~~~~~
>
> If you "re-roll" (or, as pointed out at the Contributors' Summit, better
> put: if you send another iteration of the patch series), please squash
> this fix in.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Thanks for making this fix! :) I will squash it in.
Regards,
Pranit Bauva
^ permalink raw reply
* Re: [PATCH v2 0/9] Store submodules in a hash, not a linked list
From: Jeff King @ 2017-02-10 15:56 UTC (permalink / raw)
To: Michael Haggerty
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Stefan Beller, Johannes Schindelin, David Turner, git
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
On Fri, Feb 10, 2017 at 12:16:10PM +0100, Michael Haggerty wrote:
> This is v2 of the patch series, considerably reorganized but not that
> different codewise. Thanks to Stefan, Junio, and Peff for their
> feedback about v1 [1]. I think I have addressed all of your comments.
>
> Changes since v1:
I read through these and didn't didn't see any problems.
The reordering and new commits made sense to me.
-Peff
^ permalink raw reply
* Re: Bug with fixup and autosquash
From: Johannes Schindelin @ 2017-02-10 15:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ashutosh Bapat, git, Michael Haggerty, Michael J Gruber,
Matthieu Moy
In-Reply-To: <xmqqd1ernh7g.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 9 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Almost. While I fixed the performance issues as well as the design
> > allowed, I happened to "fix" the problem where an incomplete prefix
> > match could be favored over an exact match.
>
> Hmph. Would it require too much further work to do what you said the
> code does:
I was just being overly precise. I *did* fix the problem. But since it was
not my intention, I quoted the verb "fix".
> > The rebase--helper code (specifically, the patch moving autosquash
> > logic into it: https://github.com/dscho/git/commit/7d0831637f) tries
> > to match exact onelines first, and falls back to prefix matching only
> > after that.
>
> If the code matches exact onlines and then falls back to prefix, I do
> not think incomplete prefix would be mistakenly chosen over an exact
> one, so perhaps your code already does the right thing?
The code does exactly that. It does even more: as `fixup! <SHA-1>` is
allowed (for SHA-1s that have been mentioned in previous `pick` lines), it
tries to match that before falling back to the incomplete prefix match.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v2 1/2] rev-parse tests: add tests executed from a subdirectory
From: Johannes Schindelin @ 2017-02-10 15:33 UTC (permalink / raw)
To: git; +Cc: Michael Rappazzo, Junio C Hamano,
Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1486740772.git.johannes.schindelin@gmx.de>
From: Michael Rappazzo <rappazzo@gmail.com>
t2027-worktree-list has an incorrect expectation for --git-common-dir
which has been adjusted and marked to expect failure.
Some of the tests added have been marked to expect failure. These
demonstrate a problem with the way that some options to git rev-parse
behave when executed from a subdirectory of the main worktree.
[jes: fixed incorrect assumption that objects/ lives in the
worktree-specific git-dir (it lives in the common dir instead).]
Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t1500-rev-parse.sh | 28 ++++++++++++++++++++++++++++
t/t1700-split-index.sh | 17 +++++++++++++++++
t/t2027-worktree-list.sh | 12 ++++++++++--
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c4014..f39f783f2db 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -87,4 +87,32 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru
test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
+test_expect_success 'git-common-dir from worktree root' '
+ echo .git >expect &&
+ git rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-common-dir inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+ git -C path/to/child rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+ echo .git/objects >expect &&
+ git rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'git-path inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
+ git -C path/to/child rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 292a0720fcc..1d6e27a09d8 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,21 @@ EOF
test_cmp expect actual
'
+test_expect_failure 'rev-parse --shared-index-path' '
+ rm -rf .git &&
+ test_create_repo . &&
+ git update-index --split-index &&
+ ls -t .git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual &&
+ mkdir work &&
+ test_when_finished "rm -rf work" &&
+ (
+ cd work &&
+ ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 465eeeacd3d..c1a072348e7 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,16 +8,24 @@ test_expect_success 'setup' '
test_commit init
'
-test_expect_success 'rev-parse --git-common-dir on main worktree' '
+test_expect_failure 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
mkdir sub &&
git -C sub rev-parse --git-common-dir >actual2 &&
- echo sub/.git >expected2 &&
+ echo ../.git >expected2 &&
test_cmp expected2 actual2
'
+test_expect_failure 'rev-parse --git-path objects linked worktree' '
+ echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&
+ test_when_finished "rm -rf linked-tree && git worktree prune" &&
+ git worktree add --detach linked-tree master &&
+ git -C linked-tree rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '"list" all worktrees from main' '
echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
test_when_finished "rm -rf here && git worktree prune" &&
--
2.11.1.windows.1
^ permalink raw reply related
* Re: [PATCH] mingw: use OpenSSL's SHA-1 routines
From: Johannes Schindelin @ 2017-02-10 15:49 UTC (permalink / raw)
To: Jeff King; +Cc: git, Jeff Hostetler, Junio C Hamano
In-Reply-To: <20170210050237.gajicliueuvk6s5d@sigill.intra.peff.net>
Hi Peff,
On Fri, 10 Feb 2017, Jeff King wrote:
> On Thu, Feb 09, 2017 at 11:27:49PM +0100, Johannes Schindelin wrote:
>
> > From: Jeff Hostetler <jeffhost@microsoft.com>
> >
> > Use OpenSSL's SHA-1 routines rather than builtin block-sha1 routines.
> > This improves performance on SHA1 operations on Intel processors.
> >
> > OpenSSL 1.0.2 has made considerable performance improvements and
> > support the Intel hardware acceleration features. See:
> > https://software.intel.com/en-us/articles/improving-openssl-performance
> > https://software.intel.com/en-us/articles/intel-sha-extensions
> >
> > To test this I added/staged a single file in a gigantic repository
> > having a 450MB index file. The code in read-cache.c verifies the
> > header SHA as it reads the index and computes a new header SHA as it
> > writes out the new index. Therefore, in this test the SHA code must
> > process 900MB of data. Testing was done on an Intel I7-4770 CPU @
> > 3.40GHz (Intel64, Family 6, Model 60) CPU.
>
> I think this is only half the story. A heavy-sha1 workload is faster,
> which is good. But one of the original reasons to prefer blk-sha1 (at
> least on Linux) is that resolving libcrypto.so symbols takes a
> non-trivial amount of time. I just timed it again, and it seems to be
> consistently 1ms slower to run "git rev-parse --git-dir" on my machine
> (from the top-level of a repo).
>
> 1ms is mostly irrelevant, but it adds up on scripted workloads that
> start a lot of git processes.
You know my answer to that. If scripting slows things down, we should
avoid it in production code. As it is, scripting slows us down. Therefore
I work slowly but steadily to get rid of scripting where it hurts most.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Johannes Schindelin @ 2017-02-10 15:44 UTC (permalink / raw)
To: Mike Rappazzo; +Cc: Junio C Hamano, Duy Nguyen, Git Mailing List
In-Reply-To: <CANoM8SV7oJ6YmKM-n63620EkODxD562BZnLZB6OvX8O6BmDT1A@mail.gmail.com>
Hi Mike,
On Thu, 9 Feb 2017, Mike Rappazzo wrote:
> On Thu, Feb 9, 2017 at 5:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > That leaves what the right single-step behaviour change should be. As
> > I recall Duy said something about --common-dir and other things Mike's
> > earlier change also covered, I'd prefer to leave it to three of you to
> > figure out what the final patch should be.
> >
>
> The tests which I covered in my previous patch [1] addressed the places
> where we identified similar problems. We should try to include some
> form of those tests. As far as implementation goes in rev-parse, the
> version in this thread is probably better that what I had, but it would
> need to also be applied to --git-common-dir and --shared-index-path.
Thank you so much for pointing out that git-common-dir and
shared-index-path have the same problem.
I looked a little further, and it seems that the show_file() function may
have the exact same problem... but then, it only prefixes filenames if the
--prefix=<prefix> option has been passed, and it could be argued that
those prefixed filenames are *not* meant to be relative to the cwd but to
the top-level directory.
Anways, v2 was just sent out, and with Peff's acknowledgement that this
fixes a real bug and that hypothetical scripts relying on the buggy
behavior were broken beyond repair even without worktrees anyway, I am
hopeful that we'll get somewhere.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v2 2/2] rev-parse: fix several options when running in a subdirectory
From: Johannes Schindelin @ 2017-02-10 15:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Michael Rappazzo
In-Reply-To: <cover.1486740772.git.johannes.schindelin@gmx.de>
In addition to making git_path() aware of certain file names that need
to be handled differently e.g. when running in worktrees, the commit
557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
2014-11-30) also snuck in a new option for `git rev-parse`:
`--git-path`.
On the face of it, there is no obvious bug in that commit's diff: it
faithfully calls git_path() on the argument and prints it out, i.e. `git
rev-parse --git-path <filename>` has the same precise behavior as
calling `git_path("<filename>")` in C.
The problem lies deeper, much deeper. In hindsight (which is always
unfair), implementing the .git/ directory discovery in
`setup_git_directory()` by changing the working directory may have
allowed us to avoid passing around a struct that contains information
about the current repository, but it bought us many, many problems.
In this case, when being called in a subdirectory, `git rev-parse`
changes the working directory to the top-level directory before calling
`git_path()`. In the new working directory, the result is correct. But
in the working directory of the calling script, it is incorrect.
Example: when calling `git rev-parse --git-path HEAD` in, say, the
Documentation/ subdirectory of Git's own source code, the string
`.git/HEAD` is printed.
Side note: that bug is hidden when running in a subdirectory of a
worktree that was added by the `git worktree` command: in that case, the
(correct) absolute path of the `HEAD` file is printed.
In the interest of time, this patch does not go the "correct" route to
introduce a struct with repository information (and removing global
state in the process), instead this patch chooses to detect when the
command was called in a subdirectory and forces the result to be an
absolute path.
While at it, we are also fixing the output of --git-common-dir and
--shared-index-path.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rev-parse.c | 15 +++++++++++----
t/t1500-rev-parse.sh | 4 ++--
t/t1700-split-index.sh | 2 +-
t/t2027-worktree-list.sh | 4 ++--
4 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1db..84af2802f6f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -545,6 +545,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
unsigned int flags = 0;
const char *name = NULL;
struct object_context unused;
+ struct strbuf buf = STRBUF_INIT;
if (argc > 1 && !strcmp("--parseopt", argv[1]))
return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -599,7 +600,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
if (!strcmp(arg, "--git-path")) {
if (!argv[i + 1])
die("--git-path requires an argument");
- puts(git_path("%s", argv[i + 1]));
+ strbuf_reset(&buf);
+ puts(relative_path(git_path("%s", argv[i + 1]),
+ prefix, &buf));
i++;
continue;
}
@@ -821,8 +824,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
- const char *pfx = prefix ? prefix : "";
- puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
+ strbuf_reset(&buf);
+ puts(relative_path(get_git_common_dir(),
+ prefix, &buf));
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -845,7 +849,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die(_("Could not read the index"));
if (the_index.split_index) {
const unsigned char *sha1 = the_index.split_index->base_sha1;
- puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));
+ const char *path = git_path("sharedindex.%s", sha1_to_hex(sha1));
+ strbuf_reset(&buf);
+ puts(relative_path(path, prefix, &buf));
}
continue;
}
@@ -906,5 +912,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die_no_single_rev(quiet);
} else
show_default();
+ strbuf_release(&buf);
return 0;
}
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index f39f783f2db..d74f09ad93e 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -93,7 +93,7 @@ test_expect_success 'git-common-dir from worktree root' '
test_cmp expect actual
'
-test_expect_failure 'git-common-dir inside sub-dir' '
+test_expect_success 'git-common-dir inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
@@ -107,7 +107,7 @@ test_expect_success 'git-path from worktree root' '
test_cmp expect actual
'
-test_expect_failure 'git-path inside sub-dir' '
+test_expect_success 'git-path inside sub-dir' '
mkdir -p path/to/child &&
test_when_finished "rm -rf path" &&
echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 1d6e27a09d8..446ff34f966 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,7 +200,7 @@ EOF
test_cmp expect actual
'
-test_expect_failure 'rev-parse --shared-index-path' '
+test_expect_success 'rev-parse --shared-index-path' '
rm -rf .git &&
test_create_repo . &&
git update-index --split-index &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index c1a072348e7..848da5f3684 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -8,7 +8,7 @@ test_expect_success 'setup' '
test_commit init
'
-test_expect_failure 'rev-parse --git-common-dir on main worktree' '
+test_expect_success 'rev-parse --git-common-dir on main worktree' '
git rev-parse --git-common-dir >actual &&
echo .git >expected &&
test_cmp expected actual &&
@@ -18,7 +18,7 @@ test_expect_failure 'rev-parse --git-common-dir on main worktree' '
test_cmp expected2 actual2
'
-test_expect_failure 'rev-parse --git-path objects linked worktree' '
+test_expect_success 'rev-parse --git-path objects linked worktree' '
echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&
test_when_finished "rm -rf linked-tree && git worktree prune" &&
git worktree add --detach linked-tree master &&
--
2.11.1.windows.1
^ permalink raw reply related
* [PATCH v2 0/2] Fix bugs in rev-parse's output when run in a subdirectory
From: Johannes Schindelin @ 2017-02-10 15:33 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Michael Rappazzo
In-Reply-To: <50fe3ea3302c40f4c96eaa5a568837e3334f9dc4.1486555851.git.johannes.schindelin@gmx.de>
The bug that bit me (hard!) and that triggered not only a long series of
curses but also my writing a patch and sending it to the list was that
`git rev-parse --git-path HEAD` would give *incorrect* output when run
in a subdirectory of a regular checkout, but *correct* output when run
in a subdirectory of an associated *worktree*.
I had tested the script in question quite a bit, but in a worktree. And
in production, it quietly did exactly the wrong thing.
Relative to v1 of the then-single patch, this changed:
- the patch now covers also --git-common-dir and --shared-index-path
- the output is now a relative path when git_dir is relative
- I plucked the tests from Mike's patch series and dropped my ad-hoc
test from t0060.
- While at it, I fixed Mike's test that assumed that the objects/
directory lives in .git/worktrees/<name>/objects/.
Johannes Schindelin (1):
rev-parse: fix several options when running in a subdirectory
Michael Rappazzo (1):
rev-parse tests: add tests executed from a subdirectory
builtin/rev-parse.c | 15 +++++++++++----
t/t1500-rev-parse.sh | 28 ++++++++++++++++++++++++++++
t/t1700-split-index.sh | 17 +++++++++++++++++
t/t2027-worktree-list.sh | 10 +++++++++-
4 files changed, 65 insertions(+), 5 deletions(-)
base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
Published-As: https://github.com/dscho/git/releases/tag/git-path-in-subdir-v2
Fetch-It-Via: git fetch https://github.com/dscho/git git-path-in-subdir-v2
Interdiff vs v1:
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f9d5762bf2b..84af2802f6f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -545,6 +545,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
unsigned int flags = 0;
const char *name = NULL;
struct object_context unused;
+ struct strbuf buf = STRBUF_INIT;
if (argc > 1 && !strcmp("--parseopt", argv[1]))
return cmd_parseopt(argc - 1, argv + 1, prefix);
@@ -597,13 +598,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
}
if (!strcmp(arg, "--git-path")) {
- const char *path;
if (!argv[i + 1])
die("--git-path requires an argument");
- path = git_path("%s", argv[i + 1]);
- if (prefix && !is_absolute_path(path))
- path = real_path(path);
- puts(path);
+ strbuf_reset(&buf);
+ puts(relative_path(git_path("%s", argv[i + 1]),
+ prefix, &buf));
i++;
continue;
}
@@ -825,8 +824,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
- const char *pfx = prefix ? prefix : "";
- puts(prefix_filename(pfx, strlen(pfx), get_git_common_dir()));
+ strbuf_reset(&buf);
+ puts(relative_path(get_git_common_dir(),
+ prefix, &buf));
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -849,7 +849,9 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die(_("Could not read the index"));
if (the_index.split_index) {
const unsigned char *sha1 = the_index.split_index->base_sha1;
- puts(git_path("sharedindex.%s", sha1_to_hex(sha1)));
+ const char *path = git_path("sharedindex.%s", sha1_to_hex(sha1));
+ strbuf_reset(&buf);
+ puts(relative_path(path, prefix, &buf));
}
continue;
}
@@ -910,5 +912,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
die_no_single_rev(quiet);
} else
show_default();
+ strbuf_release(&buf);
return 0;
}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 790584fcc54..444b5a4df80 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -271,8 +271,6 @@ relative_path "<null>" "<empty>" ./
relative_path "<null>" "<null>" ./
relative_path "<null>" /foo/a/b ./
-test_git_path "mkdir sub && cd sub && test_when_finished cd .. &&" \
- foo "$(pwd)/.git/foo"
test_git_path A=B info/grafts .git/info/grafts
test_git_path GIT_GRAFT_FILE=foo info/grafts foo
test_git_path GIT_GRAFT_FILE=foo info/////grafts foo
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 038e24c4014..d74f09ad93e 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -87,4 +87,32 @@ test_rev_parse -C work -g ../repo.git -b t 'GIT_DIR=../repo.git, core.bare = tru
test_rev_parse -C work -g ../repo.git -b u 'GIT_DIR=../repo.git, core.bare undefined' false false true ''
+test_expect_success 'git-common-dir from worktree root' '
+ echo .git >expect &&
+ git rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-common-dir inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git" >expect &&
+ git -C path/to/child rev-parse --git-common-dir >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-path from worktree root' '
+ echo .git/objects >expect &&
+ git rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git-path inside sub-dir' '
+ mkdir -p path/to/child &&
+ test_when_finished "rm -rf path" &&
+ echo "$(git -C path/to/child rev-parse --show-cdup).git/objects" >expect &&
+ git -C path/to/child rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 292a0720fcc..446ff34f966 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -200,4 +200,21 @@ EOF
test_cmp expect actual
'
+test_expect_success 'rev-parse --shared-index-path' '
+ rm -rf .git &&
+ test_create_repo . &&
+ git update-index --split-index &&
+ ls -t .git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual &&
+ mkdir work &&
+ test_when_finished "rm -rf work" &&
+ (
+ cd work &&
+ ls -t ../.git/sharedindex* | tail -n 1 >expect &&
+ git rev-parse --shared-index-path >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 465eeeacd3d..848da5f3684 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -14,10 +14,18 @@ test_expect_success 'rev-parse --git-common-dir on main worktree' '
test_cmp expected actual &&
mkdir sub &&
git -C sub rev-parse --git-common-dir >actual2 &&
- echo sub/.git >expected2 &&
+ echo ../.git >expected2 &&
test_cmp expected2 actual2
'
+test_expect_success 'rev-parse --git-path objects linked worktree' '
+ echo "$(git rev-parse --show-toplevel)/.git/objects" >expect &&
+ test_when_finished "rm -rf linked-tree && git worktree prune" &&
+ git worktree add --detach linked-tree master &&
+ git -C linked-tree rev-parse --git-path objects >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '"list" all worktrees from main' '
echo "$(git rev-parse --show-toplevel) $(git rev-parse --short HEAD) [$(git symbolic-ref --short HEAD)]" >expect &&
test_when_finished "rm -rf here && git worktree prune" &&
--
2.11.1.windows.1
^ permalink raw reply
* [PATCH] preload-index: avoid lstat for skip-worktree items
From: Johannes Schindelin @ 2017-02-10 15:10 UTC (permalink / raw)
To: git; +Cc: Jeff Hostetler, Junio C Hamano
From: Jeff Hostetler <jeffhost@microsoft.com>
Teach preload-index to avoid lstat() calls for index-entries
with skip-worktree bit set. This is a performance optimization.
During a sparse-checkout, the skip-worktree bit is set on items
that were not populated and therefore are not present in the
worktree. The per-thread preload-index loop performs a series
of tests on each index-entry as it attempts to compare the
worktree version with the index and mark them up-to-date.
This patch short-cuts that work.
On a Windows 10 system with a very large repo (450MB index)
and various levels of sparseness, performance was improved
in the {preloadindex=true, fscache=false} case by 80% and
in the {preloadindex=true, fscache=true} case by 20% for various
commands.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/preload-index-sparse-v1
Fetch-It-Via: git fetch https://github.com/dscho/git preload-index-sparse-v1
preload-index.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/preload-index.c b/preload-index.c
index c1fe3a3ef9c..70a4c808783 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -53,6 +53,8 @@ static void *preload_thread(void *_data)
continue;
if (ce_uptodate(ce))
continue;
+ if (ce_skip_worktree(ce))
+ continue;
if (!ce_path_match(ce, &p->pathspec, NULL))
continue;
if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce)))
base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
--
2.11.1.windows.1
^ permalink raw reply related
* [PATCH] fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
From: Johannes Schindelin @ 2017-02-10 14:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Pranit Bauva
It is curious that only MacOSX builds trigger an error about this, both
GCC and Clang, but not Linux GCC nor Clang (see
https://travis-ci.org/git/git/jobs/200182819#L1152 for details):
builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used
uninitialized whenever 'if' condition is true
[-Werror,-Wsometimes-uninitialized]
if (missing_good && !missing_bad && current_term &&
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
builtin/bisect--helper.c:350:7: note: uninitialized use occurs here
if (!good_syn)
^~~~~~~~
If you "re-roll" (or, as pointed out at the Contributors' Summit, better
put: if you send another iteration of the patch series), please squash
this fix in.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Based-On: pu at https://github.com/dscho/git
Fetch-Base-Via: git fetch https://github.com/dscho/git pu
Published-As: https://github.com/dscho/git/releases/tag/bisect--helper-fixup-v1
Fetch-It-Via: git fetch https://github.com/dscho/git bisect--helper-fixup-v1
builtin/bisect--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8cd6527bd1..614a85ffb5 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -280,7 +280,7 @@ static int bisect_next_check(const struct bisect_terms *terms,
int missing_good = 1, missing_bad = 1, retval = 0;
char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
char *good_glob = xstrfmt("%s-*", terms->term_good);
- char *bad_syn, *good_syn;
+ char *bad_syn = NULL, *good_syn = NULL;
if (ref_exists(bad_ref))
missing_bad = 0;
base-commit: 6fa4b393c01a84c9adf2e2435fba6de13227eabf
--
2.11.1.windows.1
^ permalink raw reply related
* Re: Bug with fixup and autosquash
From: Johannes Schindelin @ 2017-02-10 14:02 UTC (permalink / raw)
To: Philip Oakley
Cc: Junio C Hamano, Ashutosh Bapat, git, Michael Haggerty,
Michael J Gruber, Matthieu Moy
In-Reply-To: <454E7D934160418EB4C4871ED209CBCA@PhilipOakley>
Hi Philip,
On Thu, 9 Feb 2017, Philip Oakley wrote:
> From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> Sent: Thursday, February 09, 2017 8:55 PM
>
> > The rebase--helper code (specifically, the patch moving autosquash
> > logic into it: https://github.com/dscho/git/commit/7d0831637f) tries
> > to match exact onelines first,
>
> While I think this is an improvement, and will strongly support the `git
> commit --fixup=<commit>` option which will, if the sha1/oid is given,
> create the exact commit subject line.
That is already the case (with the exception that it is not the "exact
commit subject line" but the oneline, i.e. unwrapped first paragraph).
> However it would also be useful if the actual commit subject line could
> have a similar format option, so that those who use say the git gui
> (rather than the cli) for the commit message, could easily create the
> `!fixup <commit>` message which would allow a broader range of ways of
> spelling the commit (e.g. giving a sha1(min length) that is within the
> rebase todo list).
It is already the case that `fixup! <sha1>` is accepted, see the code
replaced by above-mentioned commit:
https://github.com/dscho/git/commit/7d0831637f#diff-0f15aff45d5dd346465c35597a5f274eL780
... and its replacement code in C:
https://github.com/dscho/git/commit/7d0831637f#diff-79231f0693f84f3951daeea17065aad9R2800
Note that both preimage and postimage code try to match onelines first,
with the new code changing behavior ever so slightly: it tries to match
the exact oneline first, then a commit SHA-1 of an already-seen `pick`
line, and only then falls back to the (expensive) prefix match.
Ciao,
Dscho
^ permalink raw reply
* Continuous Testing of Git on Windows
From: Johannes Schindelin @ 2017-02-10 12:24 UTC (permalink / raw)
To: git-for-windows; +Cc: git
Hi team,
at the GitMerge conference, I heard a couple of times "I do not bother
reporting bugs in `pu` or `next` on MacOSX and Windows anymore, only Linux
seems to be truly supported" or some variation thereof.
This is a strong indicator that we have some room for improvement here.
Active readers of the Git mailing list will not be surprised to read that
I think we have to react to build/test failures quicker, that it is not
enough to declare it okay for those integration branches to fail the test
suite or even to fail to build[*1*].
In that vein, I worked quite a bit on "Continuous Integration", or more
appropriately: Continuous Testing. That is, I created an ensemble of jobs
that build & test the four integration tests of upstream Git (`maint`,
`master`, `next` and `pu`) to verify the *basic* validity of their
respective current revisions.
Most CI integrations these days require custom configuration files to be
committed, and certain knobs to be twisted on GitHub (which I cannot turn
because I have no special privileges on git/git). After struggling with
making it work *somehow* anyway (even trying to get in touch with Travis,
but they have not bothered to reply to my requests in over a year...), I
decided to go with the Visual Studio Team Services (or short, VSTS; it
does come in handy that it is developed by distant colleagues of mine, so
they *have* to reply to my requests) where the CI configuration can be
separated from the source code.
The entire setup is a little bit more complex than your grandfather's CI
setup: it has to orchestrate five separate Git repositories, two of them
generated and updated from live 32/64-bit Git for Windows SDKs, using a
custom pool of build agents due to high resource demands, using three
separate Git for Windows installations to support 32/64-bit as well as
updating git.exe via `git pull`[*2*].
There is currently only one downside to that setup: the ability to have
publicly accessible build logs on VSTS is still in development.
This is not *such* a big downside: if the MacOSX/Linux CI based on
Travis[*3*] is any indicator, few people, if any, give a flying,
fornicating fly about public build logs.
However, we should strive to improve our software development practices,
and one such well-known Best Practice is to use Continuous Testing more
effectively, i.e. *not* to ignore it.
That is why I taught the Git for Windows CI job that tests the four
upstream Git integration branches to *also* bisect test breakages and then
upload comments to the identified commit on GitHub. See an example here
(the identified breakage seems to have disappeared in the meantime):
https://github.com/git/git/commit/5a12b3d76973#commitcomment-20802488
The code that generates this comment can be seen here:
https://github.com/git-for-windows/build-extra/blob/50c392c7d107/please.sh#L1648-L1665
So here is hoping to a quicker turnaround from breakage to fix in the
future!
Ciao,
Johannes
P.S.: I realize that these commit comments may *still* be ignored, but I
simply was not yet ready to annoy everybody by having automated mails sent
out.
Footnote *1*: It would be kind of okay if, say, `pu` would simply pick up
*all* patches so that they would not be forgotten. But that is not the
case. Even worse: it was stated recently that the expectation is that the
*submitters* of patches find bugs in their code, that the patches should
essentially be bug-free by the time they were submitted. This reasoning
falls flat on its face considering the very real failures, of course,
demonstrating our dear need for Continuous Testing.
Footnote *2*: I will describe the entire setup, including links to the
involved repositories, in a separate mail at a later stage.
Footnote *3*: Look at https://travis-ci.org/git/git/builds, and be happy
if you have a red/green deficiency so you cannot see all that red.
^ permalink raw reply
* [PATCH v2 5/9] refs: store submodule ref stores in a hashmap
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
Aside from scaling better, this means that the submodule name needn't be
stored in the ref_store instance anymore (which will be changed in a
moment). This, in turn, will help loosen the strict 1:1 relationship
between ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 58 ++++++++++++++++++++++++++++++++++++++++------------
refs/refs-internal.h | 6 ------
2 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/refs.c b/refs.c
index d7158b6..5121c57 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
*/
#include "cache.h"
+#include "hashmap.h"
#include "lockfile.h"
#include "refs.h"
#include "refs/refs-internal.h"
@@ -1352,11 +1353,41 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
return 0;
}
+struct submodule_hash_entry
+{
+ struct hashmap_entry ent; /* must be the first member! */
+
+ struct ref_store *refs;
+
+ /* NUL-terminated name of submodule: */
+ char submodule[FLEX_ARRAY];
+};
+
+static int submodule_hash_cmp(const void *entry, const void *entry_or_key,
+ const void *keydata)
+{
+ const struct submodule_hash_entry *e1 = entry, *e2 = entry_or_key;
+ const char *submodule = keydata ? keydata : e2->submodule;
+
+ return strcmp(e1->submodule, submodule);
+}
+
+static struct submodule_hash_entry *alloc_submodule_hash_entry(
+ const char *submodule, struct ref_store *refs)
+{
+ struct submodule_hash_entry *entry;
+
+ FLEX_ALLOC_STR(entry, submodule, submodule);
+ hashmap_entry_init(entry, strhash(submodule));
+ entry->refs = refs;
+ return entry;
+}
+
/* A pointer to the ref_store for the main repository: */
static struct ref_store *main_ref_store;
-/* A linked list of ref_stores for submodules: */
-static struct ref_store *submodule_ref_stores;
+/* A hashmap of ref_stores, stored by submodule name: */
+static struct hashmap submodule_ref_stores;
/*
* Return the ref_store instance for the specified submodule (or the
@@ -1365,17 +1396,18 @@ static struct ref_store *submodule_ref_stores;
*/
static struct ref_store *lookup_ref_store(const char *submodule)
{
- struct ref_store *refs;
+ struct submodule_hash_entry *entry;
if (!submodule)
return main_ref_store;
- for (refs = submodule_ref_stores; refs; refs = refs->next) {
- if (!strcmp(submodule, refs->submodule))
- return refs;
- }
+ if (!submodule_ref_stores.tablesize)
+ /* It's initialized on demand in register_ref_store(). */
+ return NULL;
- return NULL;
+ entry = hashmap_get_from_hash(&submodule_ref_stores,
+ strhash(submodule), submodule);
+ return entry ? entry->refs : NULL;
}
/*
@@ -1389,15 +1421,15 @@ static void register_ref_store(struct ref_store *refs, const char *submodule)
if (main_ref_store)
die("BUG: main_ref_store initialized twice");
- refs->next = NULL;
main_ref_store = refs;
} else {
- if (lookup_ref_store(submodule))
+ if (!submodule_ref_stores.tablesize)
+ hashmap_init(&submodule_ref_stores, submodule_hash_cmp, 0);
+
+ if (hashmap_put(&submodule_ref_stores,
+ alloc_submodule_hash_entry(submodule, refs)))
die("BUG: ref_store for submodule '%s' initialized twice",
submodule);
-
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
}
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d8a7eb1..07fd208 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -636,12 +636,6 @@ struct ref_store {
* reference store:
*/
const char *submodule;
-
- /*
- * Submodule reference store instances are stored in a linked
- * list using this pointer.
- */
- struct ref_store *next;
};
/*
--
2.9.3
^ permalink raw reply related
* [PATCH v2 1/9] refs: reorder some function definitions
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
This avoids the need to add forward declarations in the next step.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/refs.c b/refs.c
index 9bd0bc1..707092f 100644
--- a/refs.c
+++ b/refs.c
@@ -1358,27 +1358,19 @@ static struct ref_store *main_ref_store;
/* A linked list of ref_stores for submodules: */
static struct ref_store *submodule_ref_stores;
-void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule)
+struct ref_store *lookup_ref_store(const char *submodule)
{
- refs->be = be;
- if (!submodule) {
- if (main_ref_store)
- die("BUG: main_ref_store initialized twice");
+ struct ref_store *refs;
- refs->submodule = "";
- refs->next = NULL;
- main_ref_store = refs;
- } else {
- if (lookup_ref_store(submodule))
- die("BUG: ref_store for submodule '%s' initialized twice",
- submodule);
+ if (!submodule || !*submodule)
+ return main_ref_store;
- refs->submodule = xstrdup(submodule);
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
+ for (refs = submodule_ref_stores; refs; refs = refs->next) {
+ if (!strcmp(submodule, refs->submodule))
+ return refs;
}
+
+ return NULL;
}
struct ref_store *ref_store_init(const char *submodule)
@@ -1395,21 +1387,6 @@ struct ref_store *ref_store_init(const char *submodule)
return be->init(submodule);
}
-struct ref_store *lookup_ref_store(const char *submodule)
-{
- struct ref_store *refs;
-
- if (!submodule || !*submodule)
- return main_ref_store;
-
- for (refs = submodule_ref_stores; refs; refs = refs->next) {
- if (!strcmp(submodule, refs->submodule))
- return refs;
- }
-
- return NULL;
-}
-
struct ref_store *get_ref_store(const char *submodule)
{
struct ref_store *refs;
@@ -1435,6 +1412,29 @@ struct ref_store *get_ref_store(const char *submodule)
return refs;
}
+void base_ref_store_init(struct ref_store *refs,
+ const struct ref_storage_be *be,
+ const char *submodule)
+{
+ refs->be = be;
+ if (!submodule) {
+ if (main_ref_store)
+ die("BUG: main_ref_store initialized twice");
+
+ refs->submodule = "";
+ refs->next = NULL;
+ main_ref_store = refs;
+ } else {
+ if (lookup_ref_store(submodule))
+ die("BUG: ref_store for submodule '%s' initialized twice",
+ submodule);
+
+ refs->submodule = xstrdup(submodule);
+ refs->next = submodule_ref_stores;
+ submodule_ref_stores = refs;
+ }
+}
+
void assert_main_repository(struct ref_store *refs, const char *caller)
{
if (*refs->submodule)
--
2.9.3
^ permalink raw reply related
* [PATCH v2 3/9] refs: remove some unnecessary handling of submodule == ""
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
The only external entry point to the ref_store lookup functions is
get_ref_store(), which ensures that submodule == "" is passed along as
NULL. So ref_store_init() and lookup_ref_store() don't have to handle
submodule being specified as the empty string.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index d7265cc..6348414 100644
--- a/refs.c
+++ b/refs.c
@@ -1362,15 +1362,12 @@ static struct ref_store *submodule_ref_stores;
* Return the ref_store instance for the specified submodule (or the
* main repository if submodule is NULL). If that ref_store hasn't
* been initialized yet, return NULL.
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
*/
static struct ref_store *lookup_ref_store(const char *submodule)
{
struct ref_store *refs;
- if (!submodule || !*submodule)
+ if (!submodule)
return main_ref_store;
for (refs = submodule_ref_stores; refs; refs = refs->next) {
@@ -1384,9 +1381,6 @@ static struct ref_store *lookup_ref_store(const char *submodule)
/*
* Create, record, and return a ref_store instance for the specified
* submodule (or the main repository if submodule is NULL).
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
*/
static struct ref_store *ref_store_init(const char *submodule)
{
@@ -1396,10 +1390,7 @@ static struct ref_store *ref_store_init(const char *submodule)
if (!be)
die("BUG: reference backend %s is unknown", be_name);
- if (!submodule || !*submodule)
- return be->init(NULL);
- else
- return be->init(submodule);
+ return be->init(submodule);
}
struct ref_store *get_ref_store(const char *submodule)
--
2.9.3
^ permalink raw reply related
* [PATCH v2 8/9] files_ref_store::submodule: use NULL for the main repository
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
The old practice of storing the empty string in this member for the main
repository was a holdover from before 00eebe3 (refs: create a base class
"ref_store" for files_ref_store, 2016-09-04), when the submodule was
stored in a flex array at the end of `struct files_ref_store`. Storing
NULL for this case is more idiomatic and a tiny bit less code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs/files-backend.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c9d2d28..4fe92f0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -915,8 +915,8 @@ struct files_ref_store {
/*
* The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
+ * NULL if it represents the main repository's reference
+ * store:
*/
const char *submodule;
@@ -982,7 +982,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files);
- refs->submodule = submodule ? xstrdup(submodule) : "";
+ refs->submodule = xstrdup_or_null(submodule);
return ref_store;
}
@@ -994,7 +994,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
static void files_assert_main_repository(struct files_ref_store *refs,
const char *caller)
{
- if (*refs->submodule)
+ if (refs->submodule)
die("BUG: %s called for a submodule", caller);
}
@@ -1158,7 +1158,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->submodule)
+ if (refs->submodule)
packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
@@ -1228,7 +1228,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->submodule)
+ if (refs->submodule)
err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
@@ -1269,7 +1269,7 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->submodule) {
+ if (refs->submodule) {
hashclr(sha1);
flag = 0;
read_ok = !resolve_gitlink_ref(refs->submodule,
@@ -1383,7 +1383,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->submodule)
+ if (refs->submodule)
strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
--
2.9.3
^ permalink raw reply related
* [PATCH v2 4/9] register_ref_store(): new function
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
Move the responsibility for registering the ref_store for a submodule
from base_ref_store_init() to a new function, register_ref_store(). Call
the latter from ref_store_init().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/refs.c b/refs.c
index 6348414..d7158b6 100644
--- a/refs.c
+++ b/refs.c
@@ -1379,6 +1379,29 @@ static struct ref_store *lookup_ref_store(const char *submodule)
}
/*
+ * Register the specified ref_store to be the one that should be used
+ * for submodule (or the main repository if submodule is NULL). It is
+ * a fatal error to call this function twice for the same submodule.
+ */
+static void register_ref_store(struct ref_store *refs, const char *submodule)
+{
+ if (!submodule) {
+ if (main_ref_store)
+ die("BUG: main_ref_store initialized twice");
+
+ refs->next = NULL;
+ main_ref_store = refs;
+ } else {
+ if (lookup_ref_store(submodule))
+ die("BUG: ref_store for submodule '%s' initialized twice",
+ submodule);
+
+ refs->next = submodule_ref_stores;
+ submodule_ref_stores = refs;
+ }
+}
+
+/*
* Create, record, and return a ref_store instance for the specified
* submodule (or the main repository if submodule is NULL).
*/
@@ -1386,11 +1409,14 @@ static struct ref_store *ref_store_init(const char *submodule)
{
const char *be_name = "files";
struct ref_storage_be *be = find_ref_storage_backend(be_name);
+ struct ref_store *refs;
if (!be)
die("BUG: reference backend %s is unknown", be_name);
- return be->init(submodule);
+ refs = be->init(submodule);
+ register_ref_store(refs, submodule);
+ return refs;
}
struct ref_store *get_ref_store(const char *submodule)
@@ -1423,22 +1449,11 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule)
{
refs->be = be;
- if (!submodule) {
- if (main_ref_store)
- die("BUG: main_ref_store initialized twice");
+ if (!submodule)
refs->submodule = "";
- refs->next = NULL;
- main_ref_store = refs;
- } else {
- if (lookup_ref_store(submodule))
- die("BUG: ref_store for submodule '%s' initialized twice",
- submodule);
-
+ else
refs->submodule = xstrdup(submodule);
- refs->next = submodule_ref_stores;
- submodule_ref_stores = refs;
- }
}
void assert_main_repository(struct ref_store *refs, const char *caller)
--
2.9.3
^ permalink raw reply related
* [PATCH v2 6/9] refs: push the submodule attribute down
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
Push the submodule attribute down from ref_store to files_ref_store.
This is another step towards loosening the 1:1 connection between
ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 11 ----------
refs/files-backend.c | 61 ++++++++++++++++++++++++++++++++++++----------------
refs/refs-internal.h | 13 -----------
3 files changed, 43 insertions(+), 42 deletions(-)
diff --git a/refs.c b/refs.c
index 5121c57..07959ff 100644
--- a/refs.c
+++ b/refs.c
@@ -1481,17 +1481,6 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule)
{
refs->be = be;
-
- if (!submodule)
- refs->submodule = "";
- else
- refs->submodule = xstrdup(submodule);
-}
-
-void assert_main_repository(struct ref_store *refs, const char *caller)
-{
- if (*refs->submodule)
- die("BUG: %s called for a submodule", caller);
}
/* backend functions */
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f902393..5e135a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -912,6 +912,14 @@ struct packed_ref_cache {
*/
struct files_ref_store {
struct ref_store base;
+
+ /*
+ * The name of the submodule represented by this object, or
+ * the empty string if it represents the main repository's
+ * reference store:
+ */
+ const char *submodule;
+
struct ref_entry *loose;
struct packed_ref_cache *packed;
};
@@ -974,10 +982,23 @@ static struct ref_store *files_ref_store_create(const char *submodule)
base_ref_store_init(ref_store, &refs_be_files, submodule);
+ refs->submodule = submodule ? xstrdup(submodule) : "";
+
return ref_store;
}
/*
+ * Die if refs is for a submodule (i.e., not for the main repository).
+ * caller is used in any necessary error messages.
+ */
+static void files_assert_main_repository(struct files_ref_store *refs,
+ const char *caller)
+{
+ if (*refs->submodule)
+ die("BUG: %s called for a submodule", caller);
+}
+
+/*
* Downcast ref_store to files_ref_store. Die if ref_store is not a
* files_ref_store. If submodule_allowed is not true, then also die if
* files_ref_store is for a submodule (i.e., not for the main
@@ -987,14 +1008,18 @@ static struct files_ref_store *files_downcast(
struct ref_store *ref_store, int submodule_allowed,
const char *caller)
{
+ struct files_ref_store *refs;
+
if (ref_store->be != &refs_be_files)
die("BUG: ref_store is type \"%s\" not \"files\" in %s",
ref_store->be->name, caller);
+ refs = (struct files_ref_store *)ref_store;
+
if (!submodule_allowed)
- assert_main_repository(ref_store, caller);
+ files_assert_main_repository(refs, caller);
- return (struct files_ref_store *)ref_store;
+ return refs;
}
/* The length of a peeled reference line in packed-refs, including EOL: */
@@ -1133,8 +1158,8 @@ static struct packed_ref_cache *get_packed_ref_cache(struct files_ref_store *ref
{
char *packed_refs_file;
- if (*refs->base.submodule)
- packed_refs_file = git_pathdup_submodule(refs->base.submodule,
+ if (*refs->submodule)
+ packed_refs_file = git_pathdup_submodule(refs->submodule,
"packed-refs");
else
packed_refs_file = git_pathdup("packed-refs");
@@ -1203,8 +1228,8 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
size_t path_baselen;
int err = 0;
- if (*refs->base.submodule)
- err = strbuf_git_path_submodule(&path, refs->base.submodule, "%s", dirname);
+ if (*refs->submodule)
+ err = strbuf_git_path_submodule(&path, refs->submodule, "%s", dirname);
else
strbuf_git_path(&path, "%s", dirname);
path_baselen = path.len;
@@ -1244,10 +1269,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
} else {
int read_ok;
- if (*refs->base.submodule) {
+ if (*refs->submodule) {
hashclr(sha1);
flag = 0;
- read_ok = !resolve_gitlink_ref(refs->base.submodule,
+ read_ok = !resolve_gitlink_ref(refs->submodule,
refname.buf, sha1);
} else {
read_ok = !read_ref_full(refname.buf,
@@ -1358,8 +1383,8 @@ static int files_read_raw_ref(struct ref_store *ref_store,
*type = 0;
strbuf_reset(&sb_path);
- if (*refs->base.submodule)
- strbuf_git_path_submodule(&sb_path, refs->base.submodule, "%s", refname);
+ if (*refs->submodule)
+ strbuf_git_path_submodule(&sb_path, refs->submodule, "%s", refname);
else
strbuf_git_path(&sb_path, "%s", refname);
@@ -1540,7 +1565,7 @@ static int lock_raw_ref(struct files_ref_store *refs,
int ret = TRANSACTION_GENERIC_ERROR;
assert(err);
- assert_main_repository(&refs->base, "lock_raw_ref");
+ files_assert_main_repository(refs, "lock_raw_ref");
*type = 0;
@@ -2006,7 +2031,7 @@ static struct ref_lock *lock_ref_sha1_basic(struct files_ref_store *refs,
int attempts_remaining = 3;
int resolved;
- assert_main_repository(&refs->base, "lock_ref_sha1_basic");
+ files_assert_main_repository(refs, "lock_ref_sha1_basic");
assert(err);
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -2152,7 +2177,7 @@ static int lock_packed_refs(struct files_ref_store *refs, int flags)
static int timeout_value = 1000;
struct packed_ref_cache *packed_ref_cache;
- assert_main_repository(&refs->base, "lock_packed_refs");
+ files_assert_main_repository(refs, "lock_packed_refs");
if (!timeout_configured) {
git_config_get_int("core.packedrefstimeout", &timeout_value);
@@ -2190,7 +2215,7 @@ static int commit_packed_refs(struct files_ref_store *refs)
int save_errno = 0;
FILE *out;
- assert_main_repository(&refs->base, "commit_packed_refs");
+ files_assert_main_repository(refs, "commit_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2223,7 +2248,7 @@ static void rollback_packed_refs(struct files_ref_store *refs)
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(refs);
- assert_main_repository(&refs->base, "rollback_packed_refs");
+ files_assert_main_repository(refs, "rollback_packed_refs");
if (!packed_ref_cache->lock)
die("internal error: packed-refs not locked");
@@ -2397,7 +2422,7 @@ static int repack_without_refs(struct files_ref_store *refs,
struct string_list_item *refname;
int ret, needs_repacking = 0, removed = 0;
- assert_main_repository(&refs->base, "repack_without_refs");
+ files_assert_main_repository(refs, "repack_without_refs");
assert(err);
/* Look for a packed ref */
@@ -2930,7 +2955,7 @@ static int commit_ref_update(struct files_ref_store *refs,
const unsigned char *sha1, const char *logmsg,
struct strbuf *err)
{
- assert_main_repository(&refs->base, "commit_ref_update");
+ files_assert_main_repository(refs, "commit_ref_update");
clear_loose_ref_cache(refs);
if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg, 0, err)) {
@@ -3560,7 +3585,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
int ret;
struct ref_lock *lock;
- assert_main_repository(&refs->base, "lock_ref_for_update");
+ files_assert_main_repository(refs, "lock_ref_for_update");
if ((update->flags & REF_HAVE_NEW) && is_null_sha1(update->new_sha1))
update->flags |= REF_DELETING;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 07fd208..008822d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -629,13 +629,6 @@ extern struct ref_storage_be refs_be_files;
struct ref_store {
/* The backend describing this ref_store's storage scheme: */
const struct ref_storage_be *be;
-
- /*
- * The name of the submodule represented by this object, or
- * the empty string if it represents the main repository's
- * reference store:
- */
- const char *submodule;
};
/*
@@ -658,10 +651,4 @@ void base_ref_store_init(struct ref_store *refs,
*/
struct ref_store *get_ref_store(const char *submodule);
-/*
- * Die if refs is for a submodule (i.e., not for the main repository).
- * caller is used in any necessary error messages.
- */
-void assert_main_repository(struct ref_store *refs, const char *caller);
-
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related
* [PATCH v2 9/9] read_loose_refs(): read refs using resolve_ref_recursively()
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
There is no need to call read_ref_full() or resolve_gitlink_ref() from
read_loose_refs(), because we already have a ref_store object in hand.
So we can call resolve_ref_recursively() ourselves. Happily, this
unifies the code for the submodule vs. non-submodule cases.
This requires resolve_ref_recursively() to be exposed to the refs
subsystem, though not to non-refs code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 50 +++++++++++++++++++++++++-------------------------
refs/files-backend.c | 18 ++++--------------
refs/refs-internal.h | 5 +++++
3 files changed, 34 insertions(+), 39 deletions(-)
diff --git a/refs.c b/refs.c
index 05af56b..f03dcf5 100644
--- a/refs.c
+++ b/refs.c
@@ -1230,10 +1230,10 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
}
/* This function needs to return a meaningful errno on failure */
-static const char *resolve_ref_recursively(struct ref_store *refs,
- const char *refname,
- int resolve_flags,
- unsigned char *sha1, int *flags)
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags)
{
static struct strbuf sb_refname = STRBUF_INIT;
int unused_flags;
@@ -1390,27 +1390,6 @@ static struct ref_store *main_ref_store;
static struct hashmap submodule_ref_stores;
/*
- * Return the ref_store instance for the specified submodule (or the
- * main repository if submodule is NULL). If that ref_store hasn't
- * been initialized yet, return NULL.
- */
-static struct ref_store *lookup_ref_store(const char *submodule)
-{
- struct submodule_hash_entry *entry;
-
- if (!submodule)
- return main_ref_store;
-
- if (!submodule_ref_stores.tablesize)
- /* It's initialized on demand in register_ref_store(). */
- return NULL;
-
- entry = hashmap_get_from_hash(&submodule_ref_stores,
- strhash(submodule), submodule);
- return entry ? entry->refs : NULL;
-}
-
-/*
* Register the specified ref_store to be the one that should be used
* for submodule (or the main repository if submodule is NULL). It is
* a fatal error to call this function twice for the same submodule.
@@ -1451,6 +1430,27 @@ static struct ref_store *ref_store_init(const char *submodule)
return refs;
}
+/*
+ * Return the ref_store instance for the specified submodule (or the
+ * main repository if submodule is NULL). If that ref_store hasn't
+ * been initialized yet, return NULL.
+ */
+static struct ref_store *lookup_ref_store(const char *submodule)
+{
+ struct submodule_hash_entry *entry;
+
+ if (!submodule)
+ return main_ref_store;
+
+ if (!submodule_ref_stores.tablesize)
+ /* It's initialized on demand in register_ref_store(). */
+ return NULL;
+
+ entry = hashmap_get_from_hash(&submodule_ref_stores,
+ strhash(submodule), submodule);
+ return entry ? entry->refs : NULL;
+}
+
struct ref_store *get_ref_store(const char *submodule)
{
struct ref_store *refs;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4fe92f0..cdb6b8f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1267,20 +1267,10 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
create_dir_entry(refs, refname.buf,
refname.len, 1));
} else {
- int read_ok;
-
- if (refs->submodule) {
- hashclr(sha1);
- flag = 0;
- read_ok = !resolve_gitlink_ref(refs->submodule,
- refname.buf, sha1);
- } else {
- read_ok = !read_ref_full(refname.buf,
- RESOLVE_REF_READING,
- sha1, &flag);
- }
-
- if (!read_ok) {
+ if (!resolve_ref_recursively(&refs->base,
+ refname.buf,
+ RESOLVE_REF_READING,
+ sha1, &flag)) {
hashclr(sha1);
flag |= REF_ISBROKEN;
} else if (is_null_sha1(sha1)) {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 793c850..33adbf9 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -650,4 +650,9 @@ void base_ref_store_init(struct ref_store *refs,
*/
struct ref_store *get_ref_store(const char *submodule);
+const char *resolve_ref_recursively(struct ref_store *refs,
+ const char *refname,
+ int resolve_flags,
+ unsigned char *sha1, int *flags);
+
#endif /* REFS_REFS_INTERNAL_H */
--
2.9.3
^ permalink raw reply related
* [PATCH v2 7/9] base_ref_store_init(): remove submodule argument
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
This is another step towards weakening the 1:1 relationship between
ref_stores and submodules.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +--
refs/files-backend.c | 2 +-
refs/refs-internal.h | 7 +++----
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 07959ff..05af56b 100644
--- a/refs.c
+++ b/refs.c
@@ -1477,8 +1477,7 @@ struct ref_store *get_ref_store(const char *submodule)
}
void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule)
+ const struct ref_storage_be *be)
{
refs->be = be;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 5e135a4..c9d2d28 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -980,7 +980,7 @@ static struct ref_store *files_ref_store_create(const char *submodule)
struct files_ref_store *refs = xcalloc(1, sizeof(*refs));
struct ref_store *ref_store = (struct ref_store *)refs;
- base_ref_store_init(ref_store, &refs_be_files, submodule);
+ base_ref_store_init(ref_store, &refs_be_files);
refs->submodule = submodule ? xstrdup(submodule) : "";
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 008822d..793c850 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -632,12 +632,11 @@ struct ref_store {
};
/*
- * Fill in the generic part of refs for the specified submodule and
- * add it to our collection of reference stores.
+ * Fill in the generic part of refs and add it to our collection of
+ * reference stores.
*/
void base_ref_store_init(struct ref_store *refs,
- const struct ref_storage_be *be,
- const char *submodule);
+ const struct ref_storage_be *be);
/*
* Return the ref_store instance for the specified submodule. For the
--
2.9.3
^ permalink raw reply related
* [PATCH v2 0/9] Store submodules in a hash, not a linked list
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
This is v2 of the patch series, considerably reorganized but not that
different codewise. Thanks to Stefan, Junio, and Peff for their
feedback about v1 [1]. I think I have addressed all of your comments.
Changes since v1:
* Rebase from `master` onto `maint` (to match Junio).
* Reorder some commits to make the presentation more logical.
* Added an explicit preparatory commit that just reorders some
function definitions, because it makes the diff for the subsequent
commit a lot easier to read.
* Make some preexisting functions private:
* lookup_ref_store()
* ref_store_init()
* Remove some unnecessary handling of `submodule == ""` when it is
already known to have been converted to `NULL`. (Some of the
purported handling also happened to be broken.)
* Introduce function `register_ref_store()` in a separate step, before
switching to hashmaps.
* Don't initialize hashmap in `lookup_ref_store()`. (Just return
`NULL`; the hashmap will be initialized in `register_ref_store()` a
moment later.)
* Make code in `submodule_hash_cmp()` clearer.
* Use `FLEX_ALLOC_STR()` in `alloc_submodule_hash_entry()`.
* Don't specify an initial size for the submodule hashmap (the default
is OK).
This patch series is also available from my fork on GitHub [2] as
branch "submodule-hash".
Michael
[1] http://public-inbox.org/git/cover.1486629195.git.mhagger@alum.mit.edu/T/#u
[2] https://github.com/mhagger/git
Michael Haggerty (9):
refs: reorder some function definitions
refs: make some ref_store lookup functions private
refs: remove some unnecessary handling of submodule == ""
register_ref_store(): new function
refs: store submodule ref stores in a hashmap
refs: push the submodule attribute down
base_ref_store_init(): remove submodule argument
files_ref_store::submodule: use NULL for the main repository
read_loose_refs(): read refs using resolve_ref_recursively()
refs.c | 107 +++++++++++++++++++++++++++++++++++----------------
refs/files-backend.c | 77 +++++++++++++++++++++---------------
refs/refs-internal.h | 48 ++++-------------------
3 files changed, 127 insertions(+), 105 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH v2 2/9] refs: make some ref_store lookup functions private
From: Michael Haggerty @ 2017-02-10 11:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, Stefan Beller,
Johannes Schindelin, David Turner, Jeff King, git,
Michael Haggerty
In-Reply-To: <cover.1486724698.git.mhagger@alum.mit.edu>
The following functions currently don't need to be exposed:
* ref_store_init()
* lookup_ref_store()
That might change in the future, but for now make them private.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 19 +++++++++++++++++--
refs/refs-internal.h | 19 -------------------
2 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index 707092f..d7265cc 100644
--- a/refs.c
+++ b/refs.c
@@ -1358,7 +1358,15 @@ static struct ref_store *main_ref_store;
/* A linked list of ref_stores for submodules: */
static struct ref_store *submodule_ref_stores;
-struct ref_store *lookup_ref_store(const char *submodule)
+/*
+ * Return the ref_store instance for the specified submodule (or the
+ * main repository if submodule is NULL). If that ref_store hasn't
+ * been initialized yet, return NULL.
+ *
+ * For backwards compatibility, submodule=="" is treated the same as
+ * submodule==NULL.
+ */
+static struct ref_store *lookup_ref_store(const char *submodule)
{
struct ref_store *refs;
@@ -1373,7 +1381,14 @@ struct ref_store *lookup_ref_store(const char *submodule)
return NULL;
}
-struct ref_store *ref_store_init(const char *submodule)
+/*
+ * Create, record, and return a ref_store instance for the specified
+ * submodule (or the main repository if submodule is NULL).
+ *
+ * For backwards compatibility, submodule=="" is treated the same as
+ * submodule==NULL.
+ */
+static struct ref_store *ref_store_init(const char *submodule)
{
const char *be_name = "files";
struct ref_storage_be *be = find_ref_storage_backend(be_name);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 708b260..d8a7eb1 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -653,25 +653,6 @@ void base_ref_store_init(struct ref_store *refs,
const char *submodule);
/*
- * Create, record, and return a ref_store instance for the specified
- * submodule (or the main repository if submodule is NULL).
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
- */
-struct ref_store *ref_store_init(const char *submodule);
-
-/*
- * Return the ref_store instance for the specified submodule (or the
- * main repository if submodule is NULL). If that ref_store hasn't
- * been initialized yet, return NULL.
- *
- * For backwards compatibility, submodule=="" is treated the same as
- * submodule==NULL.
- */
-struct ref_store *lookup_ref_store(const char *submodule);
-
-/*
* Return the ref_store instance for the specified submodule. For the
* main repository, use submodule==NULL; such a call cannot fail. For
* a submodule, the submodule must exist and be a nonbare repository,
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 0/5] Store submodules in a hash, not a linked list
From: Michael Haggerty @ 2017-02-10 10:27 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Stefan Beller, Johannes Schindelin, David Turner, git
In-Reply-To: <20170210004033.cgqmovhvoylad5cf@sigill.intra.peff.net>
On 02/10/2017 01:40 AM, Jeff King wrote:
> On Thu, Feb 09, 2017 at 10:23:35PM +0100, Michael Haggerty wrote:
>
>>>> So push the submodule attribute down to the `files_ref_store` class
>>>> (but continue to let the `ref_store`s be looked up by submodule).
>>>
>>> I'm not sure I understand all of the ramifications here. It _sounds_ like
>>> pushing this down into the files-backend code would make it harder to
>>> have mixed ref-backends for different submodules. Or is this just
>>> pushing down an implementation detail of the files backend, and future
>>> code is free to have as many different ref_stores as it likes?
>>
>> I don't understand how this would make it harder, aside from the fact
>> that a new backend class might also need a path member and have to
>> maintain its own copy rather than using one that the base class provides.
>
> Probably the answer is "I'm really confused".
>
> But here's how my line of reasoning went:
>
> Right now we have a main ref-store that points to the submodule
> ref-stores. I don't know the current state of it, but in theory those
> could all use different backends.
>
> This seems like it's pushing that submodule linkage down into the
> backend.
>
> But I think from your response that the answer is no, the thing that is
> being pushed down is not the right way for the main ref store and the
> submodules to be linked. In fact, there is no reason at all for the
> main ref store to know or care about submodules. Anybody who wants to
> know about a submodule's refs should ask the hashmap.
That's correct; the main ref store and submodule ref stores know nothing
of each other.
Michael
^ 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