Re: [PATCH] worktree add: add --lock option
Nguyễn Thái Ngọc Duy writes:
> - unlink_or_warn(sb.buf);
> + if (!ret && opts->keep_locked) {
> + /*
> + * Don't keep the confusing "initializing" message
> + * after it's already over.
> + */
> + truncate(sb.buf, 0);
> + } else {
> + unlink_or_warn(sb.buf);
> + }
builtin/worktree.c: In function 'add_worktree':
builtin/worktree.c:314:11: error: ignoring return value of 'truncate', declared
with attribute warn_unused_result [-Werror=unused-result]
truncate(sb.buf, 0);
^
cc1: all warnings being treated as errors
make: *** [builtin/worktree.o] Error 1
Re: Index files autocompletion too slow in big repositories (w / suggestion for improvement)
Jacob Keller writes:
> On Fri, Apr 14, 2017 at 3:33 PM, Ævar Arnfjörð Bjarmason
> wrote:
>> On Sat, Apr 15, 2017 at 12:08 AM, Carlos Pita
>> wrote:
>>> This is much faster (below 0.1s):
>>>
>>> __git_index_files ()
>>> {
>>> local dir="$(__gitdir)" root="${2-.}" file;
>>> if [ -d "$dir" ]; then
>>> __git_ls_files_helper "$root" "$1" | \
>>> sed -r 's@/.*@@' | uniq | sort | uniq
>>> fi
>>> }
>>>
>>> time __git_index_files
>>>
>>> real0m0.075s
>>> user0m0.083s
>>> sys0m0.010s
>>>
>>> Most of the improvement is due to the simpler, non-grouping, regex.
>>> Since I expect most of the common prefixes to arrive consecutively,
>>> running uniq before sort also improves things a bit. I'm not removing
>>> leading double quotes anymore (this isn't being done by the current
>>> version, anyway) but this doesn't seem to hurt.
>>>
>>> Despite the dependence on sed this is ten times faster than the
>>> original, maybe an option to enable fast index completion or something
>>> like that might be desirable.
>>>
>>> Best regards
>>
>> It's fine to depend on sed, these shell-scripts are POSIX compatible,
>> and so is sed, we use sed in a lot of the built-in shellscripts.
>>
>> I think you should submit this as a patch, see
>> Documentation/SubmittingPatches.
>
> Yea it should be fine to use sed.
As long as the use of "sed" is in line with POSIX.1; I do not think
you need the non-portable "-r" merely to strip out everything that
follow the first slash, so perhaps "s|-r|-e|" with the above (and do
not write backslash after pipe at the end of the line---shell knows
you haven't finished talking to it yet if you end a line with a
pipe, and there is no need for backslash), you'd be golden.
Re: [PATCH] worktree add: add --lock option
Junio C Hamano writes:
> Nguyễn Thái Ngọc Duy writes:
>
>> -unlink_or_warn(sb.buf);
>> +if (!ret && opts->keep_locked) {
>> +/*
>> + * Don't keep the confusing "initializing" message
>> + * after it's already over.
>> + */
>> +truncate(sb.buf, 0);
>> +} else {
>> +unlink_or_warn(sb.buf);
>> +}
>
> builtin/worktree.c: In function 'add_worktree':
> builtin/worktree.c:314:11: error: ignoring return value of 'truncate',
> declared with attribute warn_unused_result [-Werror=unused-result]
>truncate(sb.buf, 0);
>^
> cc1: all warnings being treated as errors
> make: *** [builtin/worktree.o] Error 1
I wonder why we need to have "initializing" and then remove the
string. Wouldn't it be simpler to do something like this instead?
Does an empty lockfile have some special meaning?
builtin/worktree.c | 16 +++-
t/t2025-worktree-add.sh | 3 +--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 3dab07c829..5ebdcce793 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -243,7 +243,10 @@ static int add_worktree(const char *path, const char
*refname,
* after the preparation is over.
*/
strbuf_addf(&sb, "%s/locked", sb_repo.buf);
- write_file(sb.buf, "initializing");
+ if (!opts->keep_locked)
+ write_file(sb.buf, "initializing");
+ else
+ write_file(sb.buf, "added with --lock");
strbuf_addf(&sb_git, "%s/.git", path);
if (safe_create_leading_directories_const(sb_git.buf))
@@ -306,15 +309,10 @@ static int add_worktree(const char *path, const char
*refname,
done:
strbuf_reset(&sb);
strbuf_addf(&sb, "%s/locked", sb_repo.buf);
- if (!ret && opts->keep_locked) {
- /*
-* Don't keep the confusing "initializing" message
-* after it's already over.
-*/
- truncate(sb.buf, 0);
- } else {
+ if (!ret && opts->keep_locked)
+ ;
+ else
unlink_or_warn(sb.buf);
- }
argv_array_clear(&child_env);
strbuf_release(&sb);
strbuf_release(&symref);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 6dce920c03..304f25fcd1 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -65,8 +65,7 @@ test_expect_success '"add" worktree' '
test_expect_success '"add" worktree with lock' '
git rev-parse HEAD >expect &&
- git worktree add --detach --lock here-with-lock master &&
- test_must_be_empty .git/worktrees/here-with-lock/locked
+ git worktree add --detach --lock here-with-lock master
'
test_expect_success '"add" worktree from a subdir' '
Re: [PATCH 00/12] PCREv2 & more
Jeff King writes: > On Sat, Apr 08, 2017 at 01:24:54PM +, Ævar Arnfjörð Bjarmason wrote: > >> This adds PCRE v2 support, but as I was adding that I kept noticing >> other related problems to fix. It's all bundled up into the same >> series because much of it conflicts because it modifies the same test >> or other code. Notes on each patch below. > > Overall, the series looks OK to me. > > I'm not sure if it is worth all the complexity to carry pcre1/pcre2 as > run-time options. That does make it easier to do back-to-back > comparisons, but it makes the code a lot more complicated. In particular > I'm worried about subtle cases where we pcre1 turns into pcre2 (or vice > versa) by use of the aliases. That shouldn't matter to a user for > correctness, but it would throw off the benchmarking. > > If we literally just added USE_LIBPCRE2 and built against one or the > other, then all the complexity would be limited to a few #ifdefs. The > big drawback AFAICT is that anybody doing timing tests would have to > recompile in between. Yeah, having to dl two libs at runtime, even when you would ever use just one in a single run, is less than ideal. A small downside inflicted on everybody will add up to million times more than a larger downside only suffered by developers, so I tend to agree with you that we probably should simplify to choose just one (or zero) at compile time. > So I dunno. I had hoped libpcre2 would be a hands-down win on timing, > but it sounds like there's a little back-and-forth depending on the > context. Is there a ticking clock on pcre1 going away? I suspect it will > be around for many years to come, but if they'll continue optimizing > pcre2 but not pcre1, then it may make sense to hitch our wagon to the > one that upstream is actually working on.
Re: [PATCHv3] rebase: pass --[no-]signoff option to git am
Giuseppe Bilotta writes: > This makes it easy to sign off a whole patchset before submission. > > To make things work, we also fix a design issue in git-am that made it > ignore the signoff option during rebase (specifically, signoff was > handled in parse_mail(), but not in parse_mail_rebasing()). I doubt that the above implementation detail in the code is "a design issue"; it is a logical consequence of a design whose "rebase" never passes "--signoff" down to underlying "am", so it is understandable that whoever wants to pass "--signoff" thru during the rebase needs to update the implementation, but I do not think it is fair to call that "an issue". > Documentation/git-rebase.txt | 5 + > builtin/am.c | 6 +++--- > git-rebase.sh| 3 ++- > 3 files changed, 10 insertions(+), 4 deletions(-) We need new tests for "git rebase --signoff" that makes sure this works as expected and only when it should. > diff --git a/builtin/am.c b/builtin/am.c > index f7a7a971fb..d072027b5a 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1321,9 +1321,6 @@ static int parse_mail(struct am_state *state, const > char *mail) > strbuf_addbuf(&msg, &mi.log_message); > strbuf_stripspace(&msg, 0); > > - if (state->signoff) > - am_signoff(&msg); > - > assert(!state->author_name); > state->author_name = strbuf_detach(&author_name, NULL); > > @@ -1848,6 +1845,9 @@ static void am_run(struct am_state *state, int resume) > if (skip) > goto next; /* mail should be skipped */ > > + if (state->signoff) > + am_append_signoff(state); > + > write_author_script(state); > write_commit_msg(state); > } This removes the last direct caller to am_signoff(). It may be worth considering to remove the function and move its body to its only internal caller am_append_signoff().
Re: [PATCHv3] rebase: pass --[no-]signoff option to git am
On Sat, Apr 15, 2017 at 11:17 AM, Junio C Hamano wrote: > Giuseppe Bilotta writes: > >> This makes it easy to sign off a whole patchset before submission. >> >> To make things work, we also fix a design issue in git-am that made it >> ignore the signoff option during rebase (specifically, signoff was >> handled in parse_mail(), but not in parse_mail_rebasing()). > > I doubt that the above implementation detail in the code is "a > design issue"; it is a logical consequence of a design whose > "rebase" never passes "--signoff" down to underlying "am", so it is > understandable that whoever wants to pass "--signoff" thru during > the rebase needs to update the implementation, but I do not think it > is fair to call that "an issue". Good point. It's an issue now that we want to be able to pass signoff, but when the split was introduced it most definitely wasn't 8-) >> Documentation/git-rebase.txt | 5 + >> builtin/am.c | 6 +++--- >> git-rebase.sh| 3 ++- >> 3 files changed, 10 insertions(+), 4 deletions(-) > > We need new tests for "git rebase --signoff" that makes sure this > works as expected and only when it should. Would the norm in this case be to introduce the test in the same commit, or in a previous commit (as in: this is the feature we want to implement, it obviously doesn't work now, but the next commit will fix that), or in a subsequent one? >> diff --git a/builtin/am.c b/builtin/am.c >> index f7a7a971fb..d072027b5a 100644 >> --- a/builtin/am.c >> +++ b/builtin/am.c >> @@ -1321,9 +1321,6 @@ static int parse_mail(struct am_state *state, const >> char *mail) >> strbuf_addbuf(&msg, &mi.log_message); >> strbuf_stripspace(&msg, 0); >> >> - if (state->signoff) >> - am_signoff(&msg); >> - >> assert(!state->author_name); >> state->author_name = strbuf_detach(&author_name, NULL); >> >> @@ -1848,6 +1845,9 @@ static void am_run(struct am_state *state, int resume) >> if (skip) >> goto next; /* mail should be skipped */ >> >> + if (state->signoff) >> + am_append_signoff(state); >> + >> write_author_script(state); >> write_commit_msg(state); >> } > > This removes the last direct caller to am_signoff(). It may be > worth considering to remove the function and move its body to its > only internal caller am_append_signoff(). Good point. It becomes a bit bigger change though, so I'll probably split it off in a separate commit now. -- Giuseppe "Oblomov" Bilotta
Re: [PATCH 00/12] PCREv2 & more
On Sat, Apr 15, 2017 at 10:11 AM, Junio C Hamano wrote: > Jeff King writes: > >> On Sat, Apr 08, 2017 at 01:24:54PM +, Ævar Arnfjörð Bjarmason wrote: >> >>> This adds PCRE v2 support, but as I was adding that I kept noticing >>> other related problems to fix. It's all bundled up into the same >>> series because much of it conflicts because it modifies the same test >>> or other code. Notes on each patch below. >> >> Overall, the series looks OK to me. >> >> I'm not sure if it is worth all the complexity to carry pcre1/pcre2 as >> run-time options. That does make it easier to do back-to-back >> comparisons, but it makes the code a lot more complicated. In particular >> I'm worried about subtle cases where we pcre1 turns into pcre2 (or vice >> versa) by use of the aliases. That shouldn't matter to a user for >> correctness, but it would throw off the benchmarking. >> >> If we literally just added USE_LIBPCRE2 and built against one or the >> other, then all the complexity would be limited to a few #ifdefs. The >> big drawback AFAICT is that anybody doing timing tests would have to >> recompile in between. > > Yeah, having to dl two libs at runtime, even when you would ever use > just one in a single run, is less than ideal. A small downside > inflicted on everybody will add up to million times more than a > larger downside only suffered by developers, so I tend to agree with > you that we probably should simplify to choose just one (or zero) at > compile time. I'll document & clarify this in v2, but I don't expect / want anyone who's distributing git to link to both v1 & v2, more details in my . It's just something we already have 95% of the code to support anyway, and doing the remaining 5% makes it easier to test & benchmark it for us devs without incurring any real maintenance or tech burden. But as noted elsewhere in that message I'll include a patch to only add the ability to use one PCRE version. So we can just review & discuss the tradeoffs of doing that then.
Re: [PATCHv3] rebase: pass --[no-]signoff option to git am
Giuseppe Bilotta writes: >> We need new tests for "git rebase --signoff" that makes sure this >> works as expected and only when it should. > > Would the norm in this case be to introduce the test in the same > commit, or in a previous commit (as in: this is the feature we want to > implement, it obviously doesn't work now, but the next commit will fix > that), or in a subsequent one? For a new feature (especially with this small implementation), it is best to have the test in the same commit. We often use the "start with expect_failure, update the code while flipping _failure to _success" pattern but that is primarily suitable for bugfixes.
What's cooking in git.git (Apr 2017, #02; draft as of Sat, 15)
This is not yet the second issue of this month, but is a draft. I
haven't caught up with the list traffic yet, but have skimmed most
of the discussion and even managed to pick up some new topics.
Two requests to topic owners:
- You'd notice that the topics in the New Topics section below do
not have any usual two/three-line one-paragraph summary. If you
can supply one for me, that would be helpful.
- Please check what is queued from you (listed below in New topics
section) to see if I picked up a stale one and you have a newer
version that I should replace them with, in which case let me
know the Message-Id of the latest.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--
[New Topics]
* ah/diff-files-ours-theirs-doc (2017-04-13) 1 commit
- diff-files: document --ours etc.
* dt/xgethostname-nul-termination (2017-04-13) 1 commit
- xgethostname: handle long hostnames
* jh/string-list-micro-optim (2017-04-15) 1 commit
- string-list: use ALLOC_GROW macro when reallocing string_list
* jh/unpack-trees-micro-optim (2017-04-15) 1 commit
- unpack-trees: avoid duplicate ODB lookups during checkout
* jh/verify-index-checksum-only-in-fsck (2017-04-15) 1 commit
- read-cache: force_verify_index_checksum
* jk/no-looking-at-dotgit-outside-repo (2017-04-13) 1 commit
- has_sha1_file: don't bother if we are not in a repository
* ls/travis-doc-asciidoctor (2017-04-13) 3 commits
- travis-ci: unset compiler for jobs that do not need one
- travis-ci: parallelize documentation build
- travis-ci: build documentation with AsciiDoc and Asciidoctor
* mg/status-in-progress-info (2017-04-14) 1 commit
- status: show in-progress info for short status
* nd/conditional-config-include (2017-04-14) 2 commits
- config: resolve symlinks in conditional include's patterns
- path.c: and an option to call real_path() in expand_user_path()
* nd/worktree-add-lock (2017-04-15) 2 commits
- SQUASH???
- worktree add: add --lock option
* ps/pathspec-empty-prefix-origin (2017-04-14) 1 commit
- pathspec: honor `PATHSPEC_PREFIX_ORIGIN` with empty prefix
* sb/submodule-rm-absorb (2017-04-13) 1 commit
- submodule.c: add missing ' in error messages
* sr/http-proxy-configuration-fix (2017-04-13) 2 commits
- http: fix the silent ignoring of proxy misconfiguraion
- http: honor empty http.proxy option to bypass proxy
* tb/doc-eol-normalization (2017-04-13) 1 commit
- gitattributes.txt: document how to normalize the line endings
* va/i18n-perl-scripts (2017-04-13) 1 commit
- git-add--interactive.perl: add missing dot in a message
* bw/submodule-is-active (2017-04-13) 1 commit
- submodule--helper: fix typo in is_active error message
* gp/rebase-signoff (2017-04-15) 1 commit
- rebase: pass --[no-]signoff option to git am
* jh/add-index-entry-optim (2017-04-15) 3 commits
- read-cache: speed up add_index_entry during checkout
- p0006-read-tree-checkout: perf test to time read-tree
- read-cache: add strcmp_offset function
--
[Stalled]
* sg/clone-refspec-from-command-line-config (2017-02-27) 1 commit
- clone: respect configured fetch respecs during initial fetch
Expecting a reroll.
cf. <20170227211217.73gydlxb2qu2s...@sigill.intra.peff.net>
cf.
Some nits but looks ok.
* sk/dash-is-previous (2017-03-01) 5 commits
- revert.c: delegate handling of "-" shorthand to setup_revisions
- sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"
- revision.c: args starting with "-" might be a revision
- revision.c: swap if/else blocks
- revision.c: do not update argv with unknown option
A dash "-" can be written to mean "the branch that was previously
checked out" in more places.
Needs review.
cf. <1488007487-12965-1-git-send-email-kannan.siddhart...@gmail.com>
* ls/filter-process-delayed (2017-03-06) 1 commit
- convert: add "status=delayed" to filter process protocol
What's the status of this one???
cf.
* nd/worktree-move (2017-01-27) 7 commits
. fixup! worktree move: new command
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
"git worktree" learned move and remove subcommands.
Tentatively ejected as it seems to break 'pu' when merged.
* pb/bisect (2017-02-18) 28 commits
- fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisec
[PATCH 0/3] git-p4: use symbolic-ref instead of name-rev
Followup to earlier discussion about use of name-rev in git-p4. http://marc.info/?l=git&m=148979063421355 Luke Diamand (3): git-p4: add failing test for name-rev rather than symbolic-ref git-p4: add read_pipe_text() internal function git-p4: don't use name-rev to get current branch git-p4.py| 38 +- t/t9807-git-p4-submit.sh | 16 2 files changed, 45 insertions(+), 9 deletions(-) -- 2.12.2.719.gcbd162c
[PATCH 3/3] git-p4: don't use name-rev to get current branch
git-p4 was using "git name-rev" to find out the current branch.
That is not safe, since if multiple branches or tags point at
the same revision, the result obtained might not be what is
expected.
Instead use "git symbolic-ref".
Signed-off-by: Luke Diamand
---
git-p4.py| 7 +--
t/t9807-git-p4-submit.sh | 2 +-
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 584b81775..8d151da91 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -602,12 +602,7 @@ def p4Where(depotPath):
return clientPath
def currentGitBranch():
-retcode = system(["git", "symbolic-ref", "-q", "HEAD"], ignore_error=True)
-if retcode != 0:
-# on a detached head
-return None
-else:
-return read_pipe(["git", "name-rev", "HEAD"]).split(" ")[1].strip()
+return read_pipe_text(["git", "symbolic-ref", "--short", "-q", "HEAD"])
def isValidGitDir(path):
return git_dir(path) != None
diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh
index ae05816e0..3457d5db6 100755
--- a/t/t9807-git-p4-submit.sh
+++ b/t/t9807-git-p4-submit.sh
@@ -139,7 +139,7 @@ test_expect_success 'submit with master branch name from
argv' '
)
'
-test_expect_failure 'allow submit from branch with same revision but different
name' '
+test_expect_success 'allow submit from branch with same revision but different
name' '
test_when_finished cleanup_git &&
git p4 clone --dest="$git" //depot &&
(
--
2.12.2.719.gcbd162c
[PATCH 1/3] git-p4: add failing test for name-rev rather than symbolic-ref
Using name-rev to find the current git branch means that git-p4 does not correctly get the current branch name if there are multiple branches pointing at HEAD, or a tag. This change adds a test case which demonstrates the problem. Configuring which branches are allowed to be submitted from goes wrong, as git-p4 gets confused about which branch is in use. This appears to be the only place that git-p4 actually cares about the current branch. Signed-off-by: Luke Diamand Signed-off-by: Junio C Hamano --- t/t9807-git-p4-submit.sh | 16 1 file changed, 16 insertions(+) diff --git a/t/t9807-git-p4-submit.sh b/t/t9807-git-p4-submit.sh index e37239e65..ae05816e0 100755 --- a/t/t9807-git-p4-submit.sh +++ b/t/t9807-git-p4-submit.sh @@ -139,6 +139,22 @@ test_expect_success 'submit with master branch name from argv' ' ) ' +test_expect_failure 'allow submit from branch with same revision but different name' ' + test_when_finished cleanup_git && + git p4 clone --dest="$git" //depot && + ( + cd "$git" && + test_commit "file8" && + git checkout -b branch1 && + git checkout -b branch2 && + git config git-p4.skipSubmitEdit true && + git config git-p4.allowSubmit "branch1" && + test_must_fail git p4 submit && + git checkout branch1 && + git p4 submit + ) +' + # # Basic submit tests, the five handled cases # -- 2.12.2.719.gcbd162c
[PATCH 2/3] git-p4: add read_pipe_text() internal function
The existing read_pipe() function returns an empty string on
error, but also returns an empty string if the command returns
an empty string.
This leads to ugly constructions trying to detect error cases.
Add read_pipe_text() which just returns None on error.
Signed-off-by: Luke Diamand
---
git-p4.py | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index eab319d76..584b81775 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -160,17 +160,42 @@ def p4_write_pipe(c, stdin):
real_cmd = p4_build_cmd(c)
return write_pipe(real_cmd, stdin)
-def read_pipe(c, ignore_error=False):
+def read_pipe_full(c):
+""" Read output from command. Returns a tuple
+of the return status, stdout text and stderr
+text.
+"""
if verbose:
sys.stderr.write('Reading pipe: %s\n' % str(c))
expand = isinstance(c,basestring)
p = subprocess.Popen(c, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
shell=expand)
(out, err) = p.communicate()
-if p.returncode != 0 and not ignore_error:
-die('Command failed: %s\nError: %s' % (str(c), err))
+return (p.returncode, out, err)
+
+def read_pipe(c, ignore_error=False):
+""" Read output from command. Returns the output text on
+success. On failure, terminates execution, unless
+ignore_error is True, when it returns an empty string.
+"""
+(retcode, out, err) = read_pipe_full(c)
+if retcode != 0:
+if ignore_error:
+out = ""
+else:
+die('Command failed: %s\nError: %s' % (str(c), err))
return out
+def read_pipe_text(c):
+""" Read output from a command with trailing whitespace stripped.
+On error, returns None.
+"""
+(retcode, out, err) = read_pipe_full(c)
+if retcode != 0:
+return None
+else:
+return out.rstrip()
+
def p4_read_pipe(c, ignore_error=False):
real_cmd = p4_build_cmd(c)
return read_pipe(real_cmd, ignore_error)
--
2.12.2.719.gcbd162c
Fwd: Errors running gitk on OS X
I have a similar one bash-4.4$ gitk 2017-04-15 13:09:52.514 Wish[3344:197352] *** Terminating app due to uncaught exception 'CALayerInvalidGeometry', reason: 'CALayer position contains NaN: [nan 0]' *** First throw call stack: ( 0 CoreFoundation 0x7fffd119048b __exceptionPreprocess + 171 1 libobjc.A.dylib 0x7fffe58f2cad objc_exception_throw + 48 2 CoreFoundation 0x7fffd120e96d +[NSException raise:format:] + 205 3 QuartzCore 0x7fffd6d35520 _ZN2CA5Layer12set_positionERKNS_4Vec2IdEEb + 152 4 QuartzCore 0x7fffd6d35695 -[CALayer setPosition:] + 44 5 QuartzCore 0x7fffd6d35ceb -[CALayer setFrame:] + 644 6 CoreUI 0x7fffdcafd4ce _ZN20CUICoreThemeRenderer26MakeOrUpdateScrollBarLayerEPK13CUIDescriptoraPP7CALayer + 1284 7 CoreUI 0x7fffdcaf94c3 _ZN20CUICoreThemeRenderer19CreateOrUpdateLayerEPK13CUIDescriptorPP7CALayer + 1595 8 CoreUI 0x7fffdca7a58d _ZN11CUIRenderer19CreateOrUpdateLayerEPK14__CFDictionaryPP7CALayer + 175 9 CoreUI 0x7fffdca7d241 CUICreateOrUpdateLayer + 221 10 AppKit 0x7fffcf7bc21f -[NSCompositeAppearance _callCoreUIWithBlock:options:] + 226 11 AppKit 0x7fffcee620b3 -[NSAppearance _createOrUpdateLayer:options:] + 76 12 AppKit 0x7fffcf0da71f -[NSScrollerImp _animateToRolloverState] + 274 13 AppKit 0x7fffcf09a0f9 __49-[NSScrollerImp _installDelayedRolloverAnimation]_block_invoke + 673 14 AppKit 0x7fffcef602c5 -[NSScrollerImp _doWork:] + 15 15 Foundation 0x7fffd2b6ec88 __NSFireDelayedPerform + 417 16 CoreFoundation 0x7fffd110fd74 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20 17 CoreFoundation 0x7fffd110f9ff __CFRunLoopDoTimer + 1071 18 CoreFoundation 0x7fffd110f55a __CFRunLoopDoTimers + 298 19 CoreFoundation 0x7fffd1106f81 __CFRunLoopRun + 2065 20 CoreFoundation 0x7fffd1106514 CFRunLoopRunSpecific + 420 21 Tcl 0x000106fe5b43 Tcl_WaitForEvent + 314 22 Tcl 0x000106fb55cd Tcl_DoOneEvent + 274 23 Tk 0x000106e1ef4f Tk_MainLoop + 33 24 Tk 0x000106e2aa5b Tk_MainEx + 1566 25 Wish0x000106dfe55a Wish + 9562 26 libdyld.dylib 0x7fffe61d1255 start + 1 ) libc++abi.dylib: terminating with uncaught exception of type NSException /usr/bin/wish: line 2: 3344 Abort trap: 6 "$(dirname $0)/../../System/Library/Frameworks/Tk.framework/Versions/8.5/Resources/Wish.app/Contents/MacOS/Wish" "$@" bash-4.4$ I think it is new - I do not really care as long as is working mint on Linux. Please pardon the 80th column limit exceed. > Begin forwarded message: > > From: Evan Aad > Subject: Errors running gitk on OS X > Date: 13 April 2017 at 19:03:45 GMT+2 > To: git@vger.kernel.org > > Running gitk from the command line, while at the top level of a Git > working directory, produces the following error message, and gitk > fails to open: > >> objc[1031]: Objective-C garbage collection is no longer supported. > >> /usr/local/bin/wish: line 2: 1031 Abort trap: 6 "$(dirname >> $0)/../../../Library/Frameworks/Tk.framework/Versions/8.5/Resources/Wish.app/Contents/MacOS/Wish" >> "$@" > > Additionally, the following error message pops up in a window: > >> Wish quit unexpectedly. > >> Click Reopen to open the application again. Click Report to see more >> detailed information and send a report to Apple. > >> Ignore | Report... | Reopen > > How can this be fixed? > > *** > > OS: macOS Sierra, Version 10.12.4 > Git version: 2.11.0 (Apple Git-81)
Re: [PATCH] worktree add: add --lock option
On Sat, Apr 15, 2017 at 3:07 PM, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>> Nguyễn Thái Ngọc Duy writes:
>>
>>> -unlink_or_warn(sb.buf);
>>> +if (!ret && opts->keep_locked) {
>>> +/*
>>> + * Don't keep the confusing "initializing" message
>>> + * after it's already over.
>>> + */
>>> +truncate(sb.buf, 0);
>>> +} else {
>>> +unlink_or_warn(sb.buf);
>>> +}
>>
>> builtin/worktree.c: In function 'add_worktree':
>> builtin/worktree.c:314:11: error: ignoring return value of 'truncate',
>> declared with attribute warn_unused_result [-Werror=unused-result]
>>truncate(sb.buf, 0);
>>^
Yes it's supposed to be safe to ignore the error in this case. I
thought of adding (void) last minute but didn't do it.
>> cc1: all warnings being treated as errors
>> make: *** [builtin/worktree.o] Error 1
>
> I wonder why we need to have "initializing" and then remove the
> string. Wouldn't it be simpler to do something like this instead?
> Does an empty lockfile have some special meaning?
It's mostly for troubleshooting. If "git worktree add" fails in a
later phase with a valid/locked worktree remains, this gives us a
clue.
> builtin/worktree.c | 16 +++-
> t/t2025-worktree-add.sh | 3 +--
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 3dab07c829..5ebdcce793 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -243,7 +243,10 @@ static int add_worktree(const char *path, const char
> *refname,
> * after the preparation is over.
> */
> strbuf_addf(&sb, "%s/locked", sb_repo.buf);
> - write_file(sb.buf, "initializing");
> + if (!opts->keep_locked)
> + write_file(sb.buf, "initializing");
> + else
> + write_file(sb.buf, "added with --lock");
>
> strbuf_addf(&sb_git, "%s/.git", path);
> if (safe_create_leading_directories_const(sb_git.buf))
> @@ -306,15 +309,10 @@ static int add_worktree(const char *path, const char
> *refname,
> done:
> strbuf_reset(&sb);
> strbuf_addf(&sb, "%s/locked", sb_repo.buf);
> - if (!ret && opts->keep_locked) {
> - /*
> -* Don't keep the confusing "initializing" message
> -* after it's already over.
> -*/
> - truncate(sb.buf, 0);
> - } else {
> + if (!ret && opts->keep_locked)
> + ;
> + else
> unlink_or_warn(sb.buf);
> - }
> argv_array_clear(&child_env);
> strbuf_release(&sb);
> strbuf_release(&symref);
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 6dce920c03..304f25fcd1 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -65,8 +65,7 @@ test_expect_success '"add" worktree' '
>
> test_expect_success '"add" worktree with lock' '
> git rev-parse HEAD >expect &&
> - git worktree add --detach --lock here-with-lock master &&
> - test_must_be_empty .git/worktrees/here-with-lock/locked
> + git worktree add --detach --lock here-with-lock master
I think you still need to check for the presence of "locked" file.
> '
>
> test_expect_success '"add" worktree from a subdir' '
--
Duy
Git allow to unconditionaly remove files on other developer host
Hi. That curious, but git allow to unconditionally delete files on other developer host when he do `git pull` How to reproduce: 1. File should be ignored: echo "somefile" >> .gitignore 2. Add this ignored file into repository git add -f somefile 3. Push changes to origin git push 4. When other developer has also 'somefile' on his host and when he does git pull Content of hist local `somefile` file will be replaced by content pushed by first developer EXPECTED: git should warn about that content will be replaced and do not pull/checkout until we force pull/checkout
Re: includeIf breaks calling dashed externals
On Fri, Apr 14, 2017 at 01:43:37PM -0400, Jeff King wrote: > On Fri, Apr 14, 2017 at 07:04:23PM +0200, Bert Wesarg wrote: > > > Dear Duy, > > > > heaving an includeIf in a git config file breaks calling external git > > commands, most prominently git-gui. > > > > $ git --version > > git version 2.12.2.599.gcf11a6797 > > $ git rev-parse --is-inside-work-tree > > true > > $ git echo > > git: 'echo' is not a git command. See 'git --help'. > > > > Did you mean this? > > fetch > > $ echo '[includeIf "gitdir:does-not-exists"]path = does-not-exists' > > >>.git/config > > $ git rev-parse --is-inside-work-tree > > true > > $ git echo > > fatal: BUG: setup_git_env called without repository > > Probably this fixes it: > > diff --git a/config.c b/config.c > index b6e4a57b9..8d66bdf56 100644 > --- a/config.c > +++ b/config.c > @@ -213,6 +213,9 @@ static int include_by_gitdir(const char *cond, size_t > cond_len, int icase) > struct strbuf pattern = STRBUF_INIT; > int ret = 0, prefix; > > + if (!have_git_dir()) > + return 0; > + > strbuf_add_absolute_path(&text, get_git_dir()); > strbuf_add(&pattern, cond, cond_len); > prefix = prepare_include_condition_pattern(&pattern); > > But it does raise a question of reading config before/after repository > setup, since those will give different answers. I guess they do anyway > because of $GIT_DIR/config. This happens in execv_dased_external() -> check_pager_config() -> read_early_config(). We probably could use the same discover_git_directory trick to get .git dir (because we should find it). Maybe something like this instead? diff --git a/config.c b/config.c index 1a4d85537b..4f540ae578 100644 --- a/config.c +++ b/config.c @@ -212,8 +212,14 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) struct strbuf text = STRBUF_INIT; struct strbuf pattern = STRBUF_INIT; int ret = 0, prefix; + struct strbuf gitdir = STRBUF_INIT; - strbuf_add_absolute_path(&text, get_git_dir()); + if (have_git_dir()) + strbuf_addstr(&gitdir, get_git_dir()); + else if (!discover_git_directory(&gitdir)) + goto done; + + strbuf_add_absolute_path(&text, gitdir.buf); strbuf_add(&pattern, cond, cond_len); prefix = prepare_include_condition_pattern(&pattern); @@ -237,6 +243,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) icase ? WM_CASEFOLD : 0, NULL); done: + strbuf_release(&gitdir); strbuf_release(&pattern); strbuf_release(&text); return ret; -- Duy
Re: Index files autocompletion too slow in big repositories (w / suggestion for improvement)
Cc Gábor.
Am 15.04.2017 um 00:33 schrieb Ævar Arnfjörð Bjarmason:
On Sat, Apr 15, 2017 at 12:08 AM, Carlos Pita wrote:
This is much faster (below 0.1s):
__git_index_files ()
{
local dir="$(__gitdir)" root="${2-.}" file;
if [ -d "$dir" ]; then
__git_ls_files_helper "$root" "$1" | \
sed -r 's@/.*@@' | uniq | sort | uniq
fi
}
time __git_index_files
real0m0.075s
user0m0.083s
sys0m0.010s
Most of the improvement is due to the simpler, non-grouping, regex.
Since I expect most of the common prefixes to arrive consecutively,
running uniq before sort also improves things a bit. I'm not removing
leading double quotes anymore (this isn't being done by the current
version, anyway) but this doesn't seem to hurt.
Despite the dependence on sed this is ten times faster than the
original, maybe an option to enable fast index completion or something
like that might be desirable.
It's fine to depend on sed, these shell-scripts are POSIX compatible,
and so is sed, we use sed in a lot of the built-in shellscripts.
This is about command line completion. We go a long way to avoid forking
processes there. What is 10x faster on Linux despite of forking a
process may not be so on Windows.
(I'm not using bash command line completion on Windows, so I can't tell
what the effect of your suggested change is on Windows. I hope Gábor can
comment on it.)
-- Hannes
Re: [PATCH 03/12] Makefile & configure: reword outdated comment about PCRE
On Tue, Apr 11, 2017 at 12:14 PM, Jeff King wrote: > On Sat, Apr 08, 2017 at 01:24:57PM +, Ævar Arnfjörð Bjarmason wrote: > >> Reword an outdated comment which suggests that only git-grep can use >> PCRE. > > Makes sense. > >> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be >> -# able to use Perl-compatible regular expressions. >> +# Define USE_LIBPCRE if you have and want to use libpcre. Various >> +# commands such as like log, grep offer runtime options to use >> +# Perl-compatible regular expressions instead of standard or extended >> +# POSIX regular expressions. > > s/such as like log, grep/such as log and grep/ ? Thanks. >> diff --git a/configure.ac b/configure.ac >> [...] >> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be >> -# able to use Perl-compatible regular expressions. >> +# Define USE_LIBPCRE if you have and want to use libpcre. Various >> +# commands such as like log, grep offer runtime options to use >> +# Perl-compatible regular expressions instead of standard or extended >> +# POSIX regular expressions. > > Ditto. > >> @@ -499,8 +501,10 @@ GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO]) >> GIT_CONF_SUBST([NO_OPENSSL]) >> >> # >> -# Define USE_LIBPCRE if you have and want to use libpcre. git-grep will be >> -# able to use Perl-compatible regular expressions. >> +# Define USE_LIBPCRE if you have and want to use libpcre. Various >> +# commands such as like log, grep offer runtime options to use >> +# Perl-compatible regular expressions instead of standard or extended >> +# POSIX regular expressions. > > And again. It's weird that the text appears twice in configure.ac. > Apparently you can use --with-libpcre or USE_LIBPCRE in the environment? > Configure is weird. You can't, I've added this to the commit message: Copy/pasting this so much in configure.ac is lame, these Makefile-like flags aren't even used by autoconf, just the corresponding --with[out]-* options. But copy/pasting the comments that make sense for the Makefile to configure.ac where they make less sence is the pattern everything else follows in that file. I'm not going to war against that as part of this change, just following the existing pattern.
Re: Git allow to unconditionaly remove files on other developer host
Am 15.04.2017 um 13:36 schrieb KES: That curious, but git allow to unconditionally delete files on other developer host when he do `git pull` How to reproduce: 1. File should be ignored: echo "somefile" >> .gitignore 2. Add this ignored file into repository git add -f somefile 3. Push changes to origin git push 4. When other developer has also 'somefile' on his host and when he does git pull Content of hist local `somefile` file will be replaced by content pushed by first developer This happens *only* if the other developers also have somefile mentioned in their .gitignore. EXPECTED: git should warn about that content will be replaced and do not pull/checkout until we force pull/checkout If somefile is *not* mentioned in their .gitignore, the file is not removed and there is a warning. Know that Git regards everything mentioned in .gitignore as dispensible; IOW, by mentioning a file in .gitignore you actually give permission to remove the file if necessary. Git does not have a feature to say "ignore this file, but it is precious". -- Hannes
Re: Index files autocompletion too slow in big repositories (w / suggestion for improvement)
Cc Gábor, resent with working email (hopefully); please follow-up on
this mail.
Am 15.04.2017 um 00:33 schrieb Ævar Arnfjörð Bjarmason:
On Sat, Apr 15, 2017 at 12:08 AM, Carlos Pita wrote:
This is much faster (below 0.1s):
__git_index_files ()
{
local dir="$(__gitdir)" root="${2-.}" file;
if [ -d "$dir" ]; then
__git_ls_files_helper "$root" "$1" | \
sed -r 's@/.*@@' | uniq | sort | uniq
fi
}
time __git_index_files
real0m0.075s
user0m0.083s
sys0m0.010s
Most of the improvement is due to the simpler, non-grouping, regex.
Since I expect most of the common prefixes to arrive consecutively,
running uniq before sort also improves things a bit. I'm not removing
leading double quotes anymore (this isn't being done by the current
version, anyway) but this doesn't seem to hurt.
Despite the dependence on sed this is ten times faster than the
original, maybe an option to enable fast index completion or something
like that might be desirable.
It's fine to depend on sed, these shell-scripts are POSIX compatible,
and so is sed, we use sed in a lot of the built-in shellscripts.
This is about command line completion. We go a long way to avoid forking
processes there. What is 10x faster on Linux despite of forking a
process may not be so on Windows.
(I'm not using bash command line completion on Windows, so I can't tell
what the effect of your suggested change is on Windows. I hope Gábor can
comment on it.)
-- Hannes
Re: Git allow to unconditionaly remove files on other developer host
On Sat, 15 Apr 2017 14:27:00 +0200 Johannes Sixt wrote: > > That curious, but git allow to unconditionally delete files on > > other developer host when he do `git pull` [...] > Know that Git regards everything mentioned in .gitignore as > dispensible; IOW, by mentioning a file in .gitignore you actually > give permission to remove the file if necessary. Git does not have a > feature to say "ignore this file, but it is precious". KES, you might also be interested in this recent thread [1]. 1. http://public-inbox.org/git/capuvn2u0uos2mt5+4ejj8m0oknk6xwerl6ce2mihfhtues-...@mail.gmail.com/
[PATCH 0/3] rebase --signoff
Allow signing off a whole patchset by rebasing it with the --signoff option, which is simply passed through to git am. Compared to previous incarnations, I've split the am massaging to separate commits (for cleanliness and easier reverts if needed), and introduced a test case for both --signoff and its negation. Giuseppe Bilotta (3): builtin/am: obey --signoff also when --rebasing builtin/am: fold am_signoff() into am_append_signoff() rebase: pass --[no-]signoff option to git am Documentation/git-rebase.txt | 5 + builtin/am.c | 39 + git-rebase.sh| 3 ++- t/t3428-rebase-signoff.sh| 46 4 files changed, 71 insertions(+), 22 deletions(-) create mode 100755 t/t3428-rebase-signoff.sh -- 2.12.2.765.g2bf946761b
[PATCH 2/3] builtin/am: fold am_signoff() into am_append_signoff()
There are no more direct calls to am_signoff(), so we can fold its
logic in am_append_signoff().
(This is done in a separate commit rather than in the previous one, to
make it easier to revert this specific change if additional calls are
ever introduced.)
Signed-off-by: Giuseppe Bilotta
---
builtin/am.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index d072027b5a..b29f885e41 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1181,42 +1181,39 @@ static void NORETURN die_user_resolve(const struct
am_state *state)
exit(128);
}
-static void am_signoff(struct strbuf *sb)
+/**
+ * Appends signoff to the "msg" field of the am_state.
+ */
+static void am_append_signoff(struct am_state *state)
{
char *cp;
struct strbuf mine = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
- /* Does it end with our own sign-off? */
+ strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
+
+ /* our sign-off */
strbuf_addf(&mine, "\n%s%s\n",
sign_off_header,
fmt_name(getenv("GIT_COMMITTER_NAME"),
getenv("GIT_COMMITTER_EMAIL")));
- if (mine.len < sb->len &&
- !strcmp(mine.buf, sb->buf + sb->len - mine.len))
+
+ /* Does sb end with it already? */
+ if (mine.len < sb.len &&
+ !strcmp(mine.buf, sb.buf + sb.len - mine.len))
goto exit; /* no need to duplicate */
/* Does it have any Signed-off-by: in the text */
- for (cp = sb->buf;
+ for (cp = sb.buf;
cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
cp = strchr(cp, '\n')) {
- if (sb->buf == cp || cp[-1] == '\n')
+ if (sb.buf == cp || cp[-1] == '\n')
break;
}
- strbuf_addstr(sb, mine.buf + !!cp);
+ strbuf_addstr(&sb, mine.buf + !!cp);
exit:
strbuf_release(&mine);
-}
-
-/**
- * Appends signoff to the "msg" field of the am_state.
- */
-static void am_append_signoff(struct am_state *state)
-{
- struct strbuf sb = STRBUF_INIT;
-
- strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
- am_signoff(&sb);
state->msg = strbuf_detach(&sb, &state->msg_len);
}
--
2.12.2.765.g2bf946761b
[PATCH 3/3] rebase: pass --[no-]signoff option to git am
This makes it easy to sign off a whole patchset before submission. Signed-off-by: Giuseppe Bilotta --- Documentation/git-rebase.txt | 5 + git-rebase.sh| 3 ++- t/t3428-rebase-signoff.sh| 46 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100755 t/t3428-rebase-signoff.sh diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 67d48e6883..e6f0b93337 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -385,6 +385,11 @@ have the long commit hash prepended to the format. Recreate merge commits instead of flattening the history by replaying commits a merge commit introduces. Merge conflict resolutions or manual amendments to merge commits are not preserved. + +--signoff:: + This flag is passed to 'git am' to sign off all the rebased + commits (see linkgit:git-am[1]). + + This uses the `--interactive` machinery internally, but combining it with the `--interactive` option explicitly is generally not a good diff --git a/git-rebase.sh b/git-rebase.sh index 48d7c5ded4..6889fd19f3 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -34,6 +34,7 @@ root! rebase all reachable commits up to the root(s) autosquash move commits that begin with squash!/fixup! under -i committer-date-is-author-date! passed to 'git am' ignore-date! passed to 'git am' +signoff! passed to 'git am' whitespace=! passed to 'git apply' ignore-whitespace! passed to 'git apply' C=!passed to 'git apply' @@ -321,7 +322,7 @@ do --ignore-whitespace) git_am_opt="$git_am_opt $1" ;; - --committer-date-is-author-date|--ignore-date) + --committer-date-is-author-date|--ignore-date|--signoff|--no-signoff) git_am_opt="$git_am_opt $1" force_rebase=t ;; diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh new file mode 100755 index 00..2afb564701 --- /dev/null +++ b/t/t3428-rebase-signoff.sh @@ -0,0 +1,46 @@ +#!/bin/sh + +test_description='git rebase --signoff + +This test runs git rebase --signoff and make sure that it works. +' + +. ./test-lib.sh + +# A simple file to commit +cat >file/") +EOF + +# Expected commit message after rebase without --signoff (or with --no-signoff) +cat >expected-unsigned < actual && + test_cmp expected-signed actual +' + +test_expect_success 'rebase --no-signoff does not add a sign-off line' ' + git commit --amend -m "first" && + git rbs --no-signoff HEAD^ && + git cat-file commit HEAD | sed -e "1,/^\$/d" > actual && + test_cmp expected-unsigned actual +' + +test_done -- 2.12.2.765.g2bf946761b
[PATCH 1/3] builtin/am: obey --signoff also when --rebasing
Signoff is handled in parse_mail(), but not in parse_mail_rebasing(), since the latter is only used when git-rebase calls git-am with the --rebasing option, and --signoff is never passed in this case. In order to introduce (in the upcoming commits) support for `git-rebase --signoff`, we must make gi-am obey it also in the rebase case. This is trivially fixed by moving the conditional addition of the signoff from parse_mail() to the caller am_run(), after either of the parse_mail*() functions were called. Signed-off-by: Giuseppe Bilotta --- builtin/am.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index f7a7a971fb..d072027b5a 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1321,9 +1321,6 @@ static int parse_mail(struct am_state *state, const char *mail) strbuf_addbuf(&msg, &mi.log_message); strbuf_stripspace(&msg, 0); - if (state->signoff) - am_signoff(&msg); - assert(!state->author_name); state->author_name = strbuf_detach(&author_name, NULL); @@ -1848,6 +1845,9 @@ static void am_run(struct am_state *state, int resume) if (skip) goto next; /* mail should be skipped */ + if (state->signoff) + am_append_signoff(state); + write_author_script(state); write_commit_msg(state); } -- 2.12.2.765.g2bf946761b
Re: [PATCH 3/3] rebase: pass --[no-]signoff option to git am
Damnit! I just realized that I forgot to amend before the format-patch: On Sat, Apr 15, 2017 at 4:41 PM, Giuseppe Bilotta wrote: > +signoff! passed to 'git am' This should be without the ! or --no-signoff is not accepted. Do I need to resend or ... ?
Re: [PATCH v10 3/3] read-cache: speed up add_index_entry during checkout
Am 14.04.2017 um 21:12 schrieb g...@jeffhostetler.com:
From: Jeff Hostetler
Teach add_index_entry_with_check() and has_dir_name()
to see if the path of the new item is greater than the
last path in the index array before attempting to search
for it.
During checkout, merge_working_tree() populates the new
index in sorted order, so this change will save at least 2
binary lookups per file. This preserves the original
behavior but simply checks the last element before starting
the search.
This helps performance on very large repositories.
This can be seen using p0006-read-tree-checkout.sh and the
artificial repository created by t/perf/repos/many-files.sh
with parameters (5, 10, 9). (1M files in index.)
TestHEAD^
HEAD
--
0006.2: read-tree br_base br_ballast (101) 4.15(2.72+1.41)
3.21(1.97+1.22) -22.7%
0006.3: switch between br_base br_ballast (101) 8.11(5.57+2.28)
6.77(4.36+2.14) -16.5%
0006.4: switch between br_ballast br_ballast_plus_1 (101)
13.52(8.68+4.35) 11.80(7.38+3.96) -12.7%
0006.5: switch between aliases (101)
13.59(8.75+4.43) 11.85(7.49+3.87) -12.8%
On linux.git, results are:
Test HEAD^
HEAD
--
0006.2: read-tree br_base br_ballast (57994) 0.24(0.22+0.01)
0.20(0.17+0.02) -16.7%
0006.3: switch between br_base br_ballast (57994) 9.91(5.82+2.79)
10.26(5.92+2.77) +3.5%
0006.4: switch between br_ballast br_ballast_plus_1 (57994) 0.59(0.44+0.16)
0.50(0.34+0.18) -15.3%
0006.5: switch between aliases (57994)0.62(0.48+0.16)
0.50(0.35+0.16) -19.4%
Signed-off-by: Jeff Hostetler
---
read-cache.c | 118 ++-
1 file changed, 116 insertions(+), 2 deletions(-)
Very nice, especially the perf test! But can we unbundle the different
optimizations into separate patches with their own performance numbers?
Candidates IMHO: The change in add_index_entry_with_check(), the first
hunk in has_dir_name(), the loop shortcuts. That might also help find
the reason for the slight slowdown of 0006.3 for the kernel repository.
diff --git a/read-cache.c b/read-cache.c
index 97f13a1..ba95fbb 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -910,6 +910,9 @@ int strcmp_offset(const char *s1, const char *s2, size_t
*first_change)
/*
* Do we have another file with a pathname that is a proper
* subset of the name we're trying to add?
+ *
+ * That is, is there another file in the index with a path
+ * that matches a sub-directory in the given entry?
*/
static int has_dir_name(struct index_state *istate,
const struct cache_entry *ce, int pos, int
ok_to_replace)
@@ -918,9 +921,51 @@ static int has_dir_name(struct index_state *istate,
int stage = ce_stage(ce);
const char *name = ce->name;
const char *slash = name + ce_namelen(ce);
+ size_t len_eq_last;
+ int cmp_last = 0;
+
+ /*
+* We are frequently called during an iteration on a sorted
+* list of pathnames and while building a new index. Therefore,
+* there is a high probability that this entry will eventually
+* be appended to the index, rather than inserted in the middle.
+* If we can confirm that, we can avoid binary searches on the
+* components of the pathname.
+*
+* Compare the entry's full path with the last path in the index.
+*/
+ if (istate->cache_nr > 0) {
+ cmp_last = strcmp_offset(name,
+ istate->cache[istate->cache_nr - 1]->name,
+ &len_eq_last);
+ if (cmp_last > 0) {
+ if (len_eq_last == 0) {
+ /*
+* The entry sorts AFTER the last one in the
+* index and their paths have no common prefix,
+* so there cannot be a F/D conflict.
+*/
+ return retval;
+ } else {
+ /*
+* The entry sorts AFTER the last one in the
+* index, but has a common prefix. Fall through
+* to the loop below to disect the entry's path
+* and see where the difference is.
+*/
+ }
+ } else if (cmp_last == 0) {
+ /*
+* The entry
Feature request: Configurable branch colors in git status --short --branch
It would be nice if the branch, remote tracking branch, and branch commit comparison count colors in git status --short --branch were configurable like the other git status colors. Example command output: $ git status --short --branch ## master...origin/master [ahead 1] M README.md M wrapper.c M ws.c M wt-status.c In the above example, "master", "origin/master", and "1" (in "ahead 1") use git's standard red and green colors. I have configured the staged and unstaged modification markers (the M's) to use brighter green and red, and I would like to configure the branch info to also use those brighter colors. However, there is currently no option (color slot for git status in git config) to change the branch colors in git status --short --branch. (See the attached screenshot or http://i.imgur.com/evqgaRd.png to see the colors I am talking about.) Stephen signature.asc Description: Digital signature
[PATCH] Documentation/git-checkout: make doc. of checkout clearer
While technically in the documentation, the fact that changes introduced by a checkout are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out. Signed-off-by: Christoph Michelbach --- Documentation/git-checkout.txt | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 8e2c066..cfd7b18 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -85,9 +85,10 @@ Omitting detaches HEAD at the tip of the current branch. from the index file or from a named (most often a commit). In this case, the `-b` and `--track` options are meaningless and giving either of them results in an error. The -argument can be used to specify a specific tree-ish - (i.e. commit, tag or tree) to update the index for the given - paths before updating the working tree. +argument can be used to specify the tree-ish (i.e. + commit, tag, or tree) to update the index to for the given paths + before updating the working tree accordingly. Note that this means + that the changes this command introduces are staged automatically. + 'git checkout' with or `--patch` is used to restore modified or deleted paths to their original contents from the index or replace paths -- 2.7.4
Re: [PATCH] Documentation/git-checkout: make doc. of checkout clearer
From: "Christoph Michelbach" While technically in the documentation, the fact that changes introduced by a checkout are staged automatically, was not obvious when reading its documentation. It is now specifically pointed out. Signed-off-by: Christoph Michelbach --- Documentation/git-checkout.txt | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 8e2c066..cfd7b18 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -85,9 +85,10 @@ Omitting detaches HEAD at the tip of the current branch. from the index file or from a named (most often a commit). In this case, the `-b` and `--track` options are meaningless and giving either of them results in an error. The - argument can be used to specify a specific tree-ish - (i.e. commit, tag or tree) to update the index for the given Do these lines above actually need reflowing? Their content hasn't changed making it more difficult to spot the significant changes below here. - paths before updating the working tree. + argument can be used to specify the tree-ish (i.e. + commit, tag, or tree) to update the index to for the given paths + before updating the working tree accordingly. + Note that this means + that the changes this command introduces are staged automatically. Does this actually capture the intent of the user confusion it's meant to cover? I may have missed the original discussions. For a clean commit checkout my mental model is not one of anything new being actively staged i.e. different from what was in the commit. I can see that if a particular tree is checkout then it's implicit staging could well be a surprise. I am in favour of improving the documentation to avoid user surprise + 'git checkout' with or `--patch` is used to restore modified or deleted paths to their original contents from the index or replace paths -- 2.7.4 -- Philip
Re: Index files autocompletion too slow in big repositories (w / suggestion for improvement)
On Sat, Apr 15, 2017 at 4:59 AM, Johannes Sixt wrote:
> Cc Gábor.
>
> Am 15.04.2017 um 00:33 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Sat, Apr 15, 2017 at 12:08 AM, Carlos Pita
>> wrote:
>>>
>>> This is much faster (below 0.1s):
>>>
>>> __git_index_files ()
>>> {
>>> local dir="$(__gitdir)" root="${2-.}" file;
>>> if [ -d "$dir" ]; then
>>> __git_ls_files_helper "$root" "$1" | \
>>> sed -r 's@/.*@@' | uniq | sort | uniq
>>> fi
>>> }
>>>
>>> time __git_index_files
>>>
>>> real0m0.075s
>>> user0m0.083s
>>> sys0m0.010s
>>>
>>> Most of the improvement is due to the simpler, non-grouping, regex.
>>> Since I expect most of the common prefixes to arrive consecutively,
>>> running uniq before sort also improves things a bit. I'm not removing
>>> leading double quotes anymore (this isn't being done by the current
>>> version, anyway) but this doesn't seem to hurt.
>>>
>>> Despite the dependence on sed this is ten times faster than the
>>> original, maybe an option to enable fast index completion or something
>>> like that might be desirable.
>>
>>
>> It's fine to depend on sed, these shell-scripts are POSIX compatible,
>> and so is sed, we use sed in a lot of the built-in shellscripts.
>
>
> This is about command line completion. We go a long way to avoid forking
> processes there. What is 10x faster on Linux despite of forking a process
> may not be so on Windows.
>
> (I'm not using bash command line completion on Windows, so I can't tell what
> the effect of your suggested change is on Windows. I hope Gábor can comment
> on it.)
>
> -- Hannes
>
In cases like this, might it be worth somehow splitting it so Linux
can use the best thing, and Windows can continue using what's best for
it, since it is a pretty significant advantage on Linux.
Thanks,
Jake
[PATCH] t1400: use consistent style for test_expect_success calls
Structure calls as test_expect_success 'description' ' body ' Use double quotes for the description if it requires parameter expansion or contains a single quote. Signed-off-by: Kyle Meyer --- This patch follows up on a recent t1400 series: https://public-inbox.org/git/xmqq8tnysekm@gitster.mtv.corp.google.com/ t/t1400-update-ref.sh | 335 +- 1 file changed, 167 insertions(+), 168 deletions(-) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index a23cd7b57..dc98b4bc6 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -35,14 +35,14 @@ test_expect_success setup ' cd - ' -test_expect_success \ - "create $m" \ - "git update-ref $m $A && -test $A"' = $(cat .git/'"$m"')' -test_expect_success \ - "create $m with oldvalue verification" \ - "git update-ref $m $B $A && -test $B"' = $(cat .git/'"$m"')' +test_expect_success "create $m" ' + git update-ref $m $A && + test $A = $(cat .git/$m) +' +test_expect_success "create $m with oldvalue verification" ' + git update-ref $m $B $A && + test $B = $(cat .git/$m) +' test_expect_success "fail to delete $m with stale ref" ' test_must_fail git update-ref -d $m $A && test $B = "$(cat .git/$m)" @@ -67,14 +67,14 @@ test_expect_success "fail to create $n" ' test_must_fail git update-ref $n $A ' -test_expect_success \ - "create $m (by HEAD)" \ - "git update-ref HEAD $A && -test $A"' = $(cat .git/'"$m"')' -test_expect_success \ - "create $m (by HEAD) with oldvalue verification" \ - "git update-ref HEAD $B $A && -test $B"' = $(cat .git/'"$m"')' +test_expect_success "create $m (by HEAD)" ' + git update-ref HEAD $A && + test $A = $(cat .git/$m) +' +test_expect_success "create $m (by HEAD) with oldvalue verification" ' + git update-ref HEAD $B $A && + test $B = $(cat .git/$m) +' test_expect_success "fail to delete $m (by HEAD) with stale ref" ' test_must_fail git update-ref -d HEAD $A && test $B = $(cat .git/$m) @@ -176,17 +176,17 @@ test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' test_must_fail git reflog exists $outside ' -test_expect_success \ - "create $m (by HEAD)" \ - "git update-ref HEAD $A && -test $A"' = $(cat .git/'"$m"')' -test_expect_success \ - "pack refs" \ - "git pack-refs --all" -test_expect_success \ - "move $m (by HEAD)" \ - "git update-ref HEAD $B $A && -test $B"' = $(cat .git/'"$m"')' +test_expect_success "create $m (by HEAD)" ' + git update-ref HEAD $A && + test $A = $(cat .git/$m) +' +test_expect_success 'pack refs' ' + git pack-refs --all +' +test_expect_success "move $m (by HEAD)" ' + git update-ref HEAD $B $A && + test $B = $(cat .git/$m) +' test_expect_success "delete $m (by HEAD) should remove both packed and loose $m" ' test_when_finished "rm -f .git/$m" && git update-ref -d HEAD $B && @@ -195,13 +195,13 @@ test_expect_success "delete $m (by HEAD) should remove both packed and loose $m" ' cp -f .git/HEAD .git/HEAD.orig -test_expect_success "delete symref without dereference" ' +test_expect_success 'delete symref without dereference' ' test_when_finished "cp -f .git/HEAD.orig .git/HEAD" && git update-ref --no-deref -d HEAD && test_path_is_missing .git/HEAD ' -test_expect_success "delete symref without dereference when the referred ref is packed" ' +test_expect_success 'delete symref without dereference when the referred ref is packed' ' test_when_finished "cp -f .git/HEAD.orig .git/HEAD" && echo foo >foo.c && git add foo.c && @@ -239,46 +239,46 @@ test_expect_success 'update-ref --no-deref -d can delete reference to bad ref' ' test_path_is_missing .git/refs/heads/ref-to-bad ' -test_expect_success '(not) create HEAD with old sha1' " +test_expect_success '(not) create HEAD with old sha1' ' test_must_fail git update-ref HEAD $A $B -" +' test_expect_success "(not) prior created .git/$m" ' test_when_finished "rm -f .git/$m" && test_path_is_missing .git/$m ' -test_expect_success \ - "create HEAD" \ - "git update-ref HEAD $A" -test_expect_success '(not) change HEAD with wrong SHA1' " +test_expect_success 'create HEAD' ' + git update-ref HEAD $A +' +test_expect_success '(not) change HEAD with wrong SHA1' ' test_must_fail git update-ref HEAD $B $Z -" +' test_expect_success "(not) changed .git/$m" ' test_when_finished "rm -f .git/$m" && ! test $B = $(cat .git/$m) ' rm -f .git/logs/refs/heads/master -test_expect_success \ - "create $m (logged by touch)" \ - 'test_config core.logAllRefUpdates false && -GIT_COMMITTER_DATE="2005-05-26 23:30" \ -git update-ref --create-reflog HEAD '"$
[REQ] Allow alternatives to gpg
I would like to try to make git signing pluggable, this would allow for using tools such as signify[1]. Now I'm wondering if this endeavor is worth taking and what would need to be changed besides gpg-interface? [1] http://man.openbsd.org/signify
[PATCH] doc/revisions: remove brackets from rev^-n shorthand
Given that other instances of "{...}" in the revision documentation
represent literal characters of revision specifications, describing
the rev^-n shorthand as "^-{}" incorrectly suggests that
something like "master^-{1}" is an acceptable form.
Signed-off-by: Kyle Meyer
---
Documentation/revisions.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 75d211f1a..61277469c 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -295,7 +295,7 @@ The 'r1{caret}@' notation means all parents of 'r1'.
The 'r1{caret}!' notation includes commit 'r1' but excludes all of its parents.
By itself, this notation denotes the single commit 'r1'.
-The '{caret}-{}' notation includes '' but excludes the th
+The '{caret}-' notation includes '' but excludes the th
parent (i.e. a shorthand for '{caret}..'), with '' = 1 if
not given. This is typically useful for merge commits where you
can just pass '{caret}-' to get all the commits in the branch
@@ -337,7 +337,7 @@ Revision Range Summary
as giving commit '' and then all its parents prefixed with
'{caret}' to exclude them (and their ancestors).
-'{caret}-{}', e.g. 'HEAD{caret}-, HEAD{caret}-2'::
+'{caret}-', e.g. 'HEAD{caret}-, HEAD{caret}-2'::
Equivalent to '{caret}..', with '' = 1 if not
given.
--
2.12.2
Re: [PATCH 3/3] reset.c: update files when using sparse to avoid data loss.
On Wed, Apr 12, 2017 at 10:37 PM, Kevin Willford wrote: > >> -Original Message- >> From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On >> Behalf Of Duy Nguyen >> Sent: Wednesday, April 12, 2017 7:21 AM >> To: Kevin Willford >> Cc: Kevin Willford ; git@vger.kernel.org; >> gits...@pobox.com; p...@peff.net >> Subject: Re: [PATCH 3/3] reset.c: update files when using sparse to avoid >> data loss. >> >> On Wed, Apr 12, 2017 at 5:30 AM, Kevin Willford >> wrote: >> > The loss of the skip-worktree bits is part of the problem if you are >> > talking about modified files. The other issue that I was having is >> > when running a reset and there were files added in the commit that is >> > being reset, there will not be an entry in the index and not a file on >> > disk so the data for that file is completely lost at that point. >> > "status" also doesn't include anything about this loss of data. On >> > modified files status will at least have the file as deleted since >> > there is still an index entry but again the previous version of the file >> > and it's >> data is lost. >> >> Well, we could have "deleted" index entries, those marked with >> CE_REMOVE. They are never written down to file though, so 'status' >> won't benefit from that. Hopefully we can restore the file before the index >> file is written down and we really lose skip-worktree bits? > > Because this is a reset --mixed it will never run through unpack_trees and > The entries are never marked with CE_REMOVE. I know. But in my view, it should. All updates from a tree object to the index should happen through unpack_trees(). >> > To me this is totally unexpected behavior, for example if I am on a >> > commit where there are only files that where added and run a reset >> > HEAD~1 and then a status, it will show a clean working directory. >> > Regardless of skip-worktree bits the user needs to have the data in >> > the working directory after the reset or data is lost which is always bad. >> >> I agree we no longer have a place to save the skip-worktree bit, we have to >> restore the state back as if skip-worktree bit does not exist. >> It would be good if we could keep the logic in unpack_trees() though. >> For example, if the file is present on disk even if skip-worktree bit is on, >> unpack_trees() would abort instead of silently overwriting it. >> This is a difference between skip-worktree and assume-unchanged bits. >> If you do explicit checkout_entry() you might have to add more checks to >> keep behavior consistent. >> -- >> Duy > > Because this is a reset --mixed it will follow the code path calling > read_from_tree > and ends up calling update_index_from_diff in the format_callback of the diff, > so unpack_trees() is never called in the --mixed case. This code change also > only applies > when the file does not exist and the skip-worktree bit is on and the updated > index entry either will be missing (covers the added scenario) or was not > missing > before (covers the modified scenario). If there is a better way to get the > previous > index entry to disk than what I am doing, I am happy to implement it > correctly. I think it's ok to just look at the diff (from update_index_from_diff) and restore the on-disk version for now. I'd like to make --mixed use unpack_trees() too but I haven't studied this code long enough to see why it went with "diff" instead of "read-tree" (which translates directly to unpack_trees). Maybe there is some subtle reason for that. Though it looks like it was more convenient to do "diff" in the git-reset.sh version, and that got translated literally to C when the command was rewritten. -- Duy
Re: [PATCH] t1400: use consistent style for test_expect_success calls
On Sat, Apr 15, 2017 at 10:31:02PM -0400, Kyle Meyer wrote: > Structure calls as > > test_expect_success 'description' ' > body > ' > > Use double quotes for the description if it requires parameter > expansion or contains a single quote. > > Signed-off-by: Kyle Meyer Looks good to me. > -test_expect_success \ > -'creating initial files' \ > -'test_when_finished rm -f M && > - echo TEST >F && > - git add F && > - GIT_AUTHOR_DATE="2005-05-26 23:30" \ > - GIT_COMMITTER_DATE="2005-05-26 23:30" git commit -m add -a && Not even sure what's going on with the indentation here in the original. I don't see any reason this "git add" should start an indented block. This and the other whitespace fixes all look improvements to me. -Peff
Re: [PATCH] doc/revisions: remove brackets from rev^-n shorthand
On Sun, Apr 16, 2017 at 12:07:57AM -0400, Kyle Meyer wrote:
> Given that other instances of "{...}" in the revision documentation
> represent literal characters of revision specifications, describing
> the rev^-n shorthand as "^-{}" incorrectly suggests that
> something like "master^-{1}" is an acceptable form.
I wondered at first if this was some weird asciidoc quoting thing. But
no, the curly braces make it through to the rendered version. I agree
they are confusing.
> -The '{caret}-{}' notation includes '' but excludes the th
> +The '{caret}-' notation includes '' but excludes the th
> parent (i.e. a shorthand for '{caret}..'), with '' = 1 if
This _could_ be:
^-[]
to show that the parameter is optional. I think the extra
punctuation in a situation like this just makes things harder to read,
though. The text already mentions the default for and gives an
example that omits it, so I think the paragraph is clear as-is (well,
after your patch removes the confusing "{}").
-Peff
Re: [PATCH v2 03/20] refs_ref_iterator_begin(): new function
On 04/07/2017 12:57 PM, Duy Nguyen wrote:
> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty
> wrote:
>> Extract a new function from `do_for_each_ref()`. It will be useful
>> elsewhere.
>>
>> Signed-off-by: Michael Haggerty
>> ---
>> refs.c | 15 +--
>> refs/refs-internal.h | 11 +++
>> 2 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/refs.c b/refs.c
>> index 0ed6c3c7a4..aeb704ab92 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)
>> return head_ref_submodule(NULL, fn, cb_data);
>> }
>>
>> +struct ref_iterator *refs_ref_iterator_begin(
>> + struct ref_store *refs,
>> + const char *prefix, int trim, int flags)
>> +{
>> + struct ref_iterator *iter;
>> +
>> + iter = refs->be->iterator_begin(refs, prefix, flags);
>> + iter = prefix_ref_iterator_begin(iter, prefix, trim);
>
> Off topic. This code made me wonder if we really need the prefix
> iterator if prefix is NULL. And in fact we don't since
> prefix_ref_iterator_begin() will return the old iter in that case. But
> it's probably better to move that optimization outside. I think it's
> easier to understand that way, calling prefix_ref_ will always give
> you a new iterator. Don't call it unless you want to have it.
I like this aspect of the design because it is more powerful. Consider
for example that some reference backend might (e.g., a future
`packed_ref_store`) need to use a `prefix_ref_iterator` to implement
`iterator_begin()`, while another (e.g., one based on `ref_cache`) might
not. It would be easy to make `prefix_ref_iterator_begin()` check
whether its argument is already a `prefix_ref_iterator`, and if so, swap
it out with a new one with different arguments (to avoid having to stack
them up). It would be inappropriate for the caller to make such an
optimization, because it (a) shouldn't have to care what type of
reference iterator it is dealing with, and (b) shouldn't have to know
enough about `prefix_ref_iterator`s to know that the optimization makes
sense.
>> +/*
>> + * Return an iterator that goes over each reference in `refs` for
>> + * which the refname begins with prefix. If trim is non-zero, then
>> + * trim that many characters off the beginning of each refname. flags
>> + * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in
>> + * the iteration.
>> + */
>
> Do we need a separate docstring here? I think we document more or less
> the same for ref_iterator_begin_fn (except the include-broken flag).
There is a subtle difference: currently, `ref_iterator_begin_fn()`
doesn't use its full `prefix` argument as prefix, but rather
`find_containing_dir(prefix)`. (This is basically an implementation
detail of `ref_cache` leaking out into the virtual function interface.)
`refs_ref_iterator_begin()`, on the other hand, treats the prefix
literally, using `starts_with()`.
I don't like this design and plan to improve it sometime, but for now
that's an important difference. The fix, incidentally, would motivate
the `prefix_ref_iterator_begin()` optimization mentioned above.
Michael
Re: [REQ] Allow alternatives to gpg
On Sat, Apr 15, 2017 at 08:10:41PM -0700, Nathan McSween wrote: > I would like to try to make git signing pluggable, this would allow for > using tools such as signify[1]. > Now I'm wondering if this endeavor is worth taking and what would need to be > changed besides > gpg-interface? > > [1] http://man.openbsd.org/signify I haven't used signify, but I have played around a bit with using gpgsm with git. You can actually get pretty far without writing any code by tweaking gpg.program, as long as: - your tool can generate and verify detached signatures - it follows the gpg command-line convention (or you wrap it in a script which converts the two) There are a few quirks around detecting the "BEGIN PGP MESSAGE" block. It's not necessary for tag signatures, but is for commit signatures (IIRC). There's some discussion in this thread: http://public-inbox.org/git/1459432304-35779-1-git-send-email-...@dwim.me/T/#u Which isn't to say we shouldn't teach Git natively to understand more encryption types. But it may be useful to prototype and get experience first by plugging the tool in via the config. (I don't have opinions on signify itself as a tool for general purpose signatures). -Peff
Re: [PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends
On 04/07/2017 01:20 PM, Duy Nguyen wrote:
> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty
> wrote:
>> It turns out that we can now implement
>> `refs_verify_refname_available()` based on the other virtual
>> functions, so there is no need for it to be defined at the backend
>> level. Instead, define it once in `refs.c` and remove the
>> `files_backend` definition.
>>
>> Signed-off-by: Michael Haggerty
>> ---
>> refs.c | 85
>> ++--
>> refs.h | 2 +-
>> refs/files-backend.c | 39 +---
>
> Much appreciated. This will make future backends simpler to implement as well.
>
>> + iter = refs_ref_iterator_begin(refs, dirname.buf, 0,
>> + DO_FOR_EACH_INCLUDE_BROKEN);
>> + while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
>> + if (skip &&
>> + string_list_has_string(skip, iter->refname))
>> + continue;
>> +
>> + strbuf_addf(err, "'%s' exists; cannot create '%s'",
>> + iter->refname, refname);
>> + ok = ref_iterator_abort(iter);
>
> Saving the return code in "ok" seems redundant because you don't use
> it. Or did you want to check that ok == ITER_DONE or die() too?
True, setting `ok` here is redundant. I don't think there's much point
worrying about whether `ref_iterator_abort()` fails here, since we've
already gotten the information that we require.
I'll remove it.
Thanks,
Michael
Re: includeIf breaks calling dashed externals
On Sat, Apr 15, 2017 at 06:49:01PM +0700, Duy Nguyen wrote: > > Probably this fixes it: > > > > diff --git a/config.c b/config.c > > index b6e4a57b9..8d66bdf56 100644 > > --- a/config.c > > +++ b/config.c > > @@ -213,6 +213,9 @@ static int include_by_gitdir(const char *cond, size_t > > cond_len, int icase) > > struct strbuf pattern = STRBUF_INIT; > > int ret = 0, prefix; > > > > + if (!have_git_dir()) > > + return 0; > > + > > strbuf_add_absolute_path(&text, get_git_dir()); > > strbuf_add(&pattern, cond, cond_len); > > prefix = prepare_include_condition_pattern(&pattern); > > > > But it does raise a question of reading config before/after repository > > setup, since those will give different answers. I guess they do anyway > > because of $GIT_DIR/config. > > This happens in execv_dased_external() -> check_pager_config() -> > read_early_config(). We probably could use the same discover_git_directory > trick to get .git dir (because we should find it). Maybe something > like this instead? I know that we only kick in discover_git_directory() in certain cases when we're reading config (for the "early config"). Would it makes sense to respect the same rules here? I.e., if we have a program that wants to look at config but explicitly _doesn't_ setup the git repository, is it right to say that we are inside that repository? So instead of lazily doing the discovery here: > diff --git a/config.c b/config.c > index 1a4d85537b..4f540ae578 100644 > --- a/config.c > +++ b/config.c > @@ -212,8 +212,14 @@ static int include_by_gitdir(const char *cond, size_t > cond_len, int icase) > struct strbuf text = STRBUF_INIT; > struct strbuf pattern = STRBUF_INIT; > int ret = 0, prefix; > + struct strbuf gitdir = STRBUF_INIT; > > - strbuf_add_absolute_path(&text, get_git_dir()); > + if (have_git_dir()) > + strbuf_addstr(&gitdir, get_git_dir()); > + else if (!discover_git_directory(&gitdir)) > + goto done; ..should we record the discovered git dir in read_early_config(), and use it only if available? And if not, then we know the program is explicitly trying not to be in a repo (or we know we tried to discover one already and it didn't exist). -Peff
Re: [PATCH v2 10/20] ref-cache: introduce a new type, ref_cache
On 04/07/2017 01:32 PM, Duy Nguyen wrote:
> On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty
> wrote:
>> +void free_ref_cache(struct ref_cache *cache)
>> +{
>> + free_ref_entry(cache->root);
>> + free(cache);
>> +}
>
> free(NULL) is no-op (and safe). Maybe we should follow the same
> pattern for free_ref_cache(). It's really up to you.
If I look at other `free_*()` functions in our codebase, it doesn't seem
that many of them handle NULL, and that feature wouldn't help the
callers of this function. So I think I'll leave it the way that it is.
Michael
Re: [PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir
On 04/07/2017 01:38 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty > wrote: >> Instead of keeping a pointer to the ref_store in every ref_dir entry, >> store it once in `struct ref_cache`, and change `struct ref_dir` to >> include a pointer to its containing `ref_cache` instead. This makes it >> easier to add to the information that is accessible from a `ref_dir` >> without increasing the size of every `ref_dir` instance. > ... >> @@ -526,7 +526,7 @@ static struct ref_dir *get_loose_refs(struct >> files_ref_store *refs) >> * lazily): >> */ >> add_entry_to_dir(get_ref_dir(refs->loose->root), >> -create_dir_entry(refs, "refs/", 5, 1)); >> +create_dir_entry(refs->loose, "refs/", 5, >> 1)); > > The commit message mentions nothing about this change. Is it intended? The old `create_dir_entry()` took a `files_ref_store` as its first parameter, because that is what needed to be stored into the old `dir_entry` struct. The new version takes a `ref_cache`, because that is what the new `dir_entry` struct requires. This hunk is a logical consequence of that change. I'll improve the commit message to explain this better. Thanks, Michael
Re: [PATCH v2 19/20] files_pack_refs(): use reference iteration
On 04/07/2017 01:51 PM, Duy Nguyen wrote: > On Fri, Mar 31, 2017 at 9:11 PM, Michael Haggerty > wrote: >> Use reference iteration rather than do_for_each_entry_in_dir() in the >> definition of files_pack_refs(). > > A "why" is missing here. My guess is readability/maintainability > because it's easier to follow the code with iterator design, than the > callback one? No maintaining data in struct pack_refs_cb_data is also > very nice. You're right. I'll improve the commit message to something like files_pack_refs(): use reference iteration Use reference iteration rather than `do_for_each_entry_in_dir()` in the definition of `files_pack_refs()`. This makes the code shorter and easier to follow, because the logic can be inline rather than spread between the main function and a callback function, and it removes the need to use `pack_refs_cb_data` to preserve intermediate state. This removes the last callers of `entry_resolves_to_object()` and `get_loose_ref_dir()`, so delete those functions. Thanks, Michael
Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module
On 04/07/2017 01:53 PM, Duy Nguyen wrote: > On Wed, Apr 5, 2017 at 9:03 PM, Duy Nguyen wrote: >> On Sat, Apr 1, 2017 at 12:16 PM, Michael Haggerty >> wrote: >>> Duy, have you looked over my patch series? Since you've been working in >>> the area, your feedback would be very welcome, if you have the time for it. >> >> You probably have guessed my answer based on my lack of response :) >> Tomorrow is holiday though, will have a look. But I doubt I would find >> anything given that both you and Jeff are involved in this. > > Overall, very nice. I made a comment here and there, but they're more > questions for my education than actual code review ) At least there's > another set of eyes on this series now. Thanks for your review! I'll submit v3 shortly to address your feedback. Michael
Business Proposal
Dear Friend, I would like to discuss a very important issue with you. I am writing to find out if this is your valid email. Please, let me know if this email is valid Kind regards Adrien Saif Attorney to Quatif Group of Companies
[PATCH v3 00/20] Separate `ref_cache` into a separate module
This is v3 of a patch series to separate the reference caching code into a separate module that interacts with `files_ref_cache` more at arm's length. Thanks to Stefan and Duy for their feedback about v2 [1]. This version has only minor changes since v2 (and indeed since v1 [2]): * Rebased onto the current nd/files-backend-git-dir (there were no conflicts). * Removed redundant assignment in refs_verify_refname_available(): implement once for all backends * Improved two commit messages. This patch series is also available from my GitHub fork [3] as branch "separate-ref-cache". These patches depend on Duy's nd/files-backend-git-dir branch. [1] http://public-inbox.org/git/cover.1490966385.git.mhag...@alum.mit.edu/ [2] http://public-inbox.org/git/cover.1490026594.git.mhag...@alum.mit.edu/ [3] https://github.com/mhagger/git Michael Haggerty (20): get_ref_dir(): don't call read_loose_refs() for "refs/bisect" refs_read_raw_ref(): new function refs_ref_iterator_begin(): new function refs_verify_refname_available(): implement once for all backends refs_verify_refname_available(): use function in more places ref-cache: rename `add_ref()` to `add_ref_entry()` ref-cache: rename `find_ref()` to `find_ref_entry()` ref-cache: rename `remove_entry()` to `remove_entry_from_dir()` refs: split `ref_cache` code into separate files ref-cache: introduce a new type, ref_cache refs: record the ref_store in ref_cache, not ref_dir ref-cache: use a callback function to fill the cache refs: handle "refs/bisect/" in `loose_fill_ref_dir()` do_for_each_entry_in_dir(): eliminate `offset` argument get_loose_ref_dir(): function renamed from get_loose_refs() get_loose_ref_cache(): new function cache_ref_iterator_begin(): make function smarter commit_packed_refs(): use reference iteration files_pack_refs(): use reference iteration do_for_each_entry_in_dir(): delete function Makefile |1 + refs.c | 111 - refs.h |2 +- refs/files-backend.c | 1229 +++--- refs/ref-cache.c | 523 + refs/ref-cache.h | 267 +++ refs/refs-internal.h | 22 +- 7 files changed, 1066 insertions(+), 1089 deletions(-) create mode 100644 refs/ref-cache.c create mode 100644 refs/ref-cache.h -- 2.11.0
[PATCH v3 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"
Since references under "refs/bisect/" are per-worktree, they have to
be sought in the worktree rather than in the main repository. But
since loose references are found by traversing directories, the
reference iterator won't even get the idea to look for a
"refs/bisect/" directory in the worktree if there is not a directory
with that name in the main repository. Thus `get_ref_dir()` manually
inserts a dir_entry for "refs/bisect/" whenever it reads the entry for
"refs/".
The current code then immediately calls `read_loose_refs()` on that
directory. But since the dir_entry is created with its `incomplete`
flag set, any traversal that gets to this point will read the
directory automatically. So there is no need to call
`read_loose_refs()` explicitly; the lazy mechanism suffices.
And in fact, the attempt to `read_loose_refs()` was broken anyway.
That function needs its `dirname` argument to have a trailing `/`
character, but the invocation here was passing it "refs/bisect"
without a trailing slash. So `read_loose_refs()` would read
`$GIT_DIR/refs/bisect" correctly, but if it found an entry "foo" in
that directory, it would try to read "$GIT_DIR/refs/bisectfoo".
Normally it wouldn't find anything at that path, but the failure was
canceled out because `get_ref_dir()` *also* forgot to reset the
`REF_INCOMPLETE` bit on the dir_entry. So the read was attempted again
when it was accessed, via the lazy mechanism, and this time the read
was done correctly.
This code has been broken since it was first introduced.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4d705b4037..2a0538dea7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -191,8 +191,6 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
"refs/bisect/",
12, 1);
add_entry_to_dir(dir, child_entry);
- read_loose_refs("refs/bisect",
- &child_entry->u.subdir);
}
}
entry->flag &= ~REF_INCOMPLETE;
--
2.11.0
[PATCH v3 19/20] files_pack_refs(): use reference iteration
Use reference iteration rather than `do_for_each_entry_in_dir()` in
the definition of `files_pack_refs()`. This makes the code shorter and
easier to follow, because the logic can be inline rather than spread
between the main function and a callback function, and it removes the
need to use `pack_refs_cb_data` to preserve intermediate state.
This removes the last callers of `entry_resolves_to_object()` and
`get_loose_ref_dir()`, so delete those functions.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 143 +--
1 file changed, 60 insertions(+), 83 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1419512d51..b81fb88fa3 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -32,17 +32,6 @@ static int ref_resolves_to_object(const char *refname,
return 1;
}
-/*
- * Return true if the reference described by entry can be resolved to
- * an object in the database; otherwise, emit a warning and return
- * false.
- */
-static int entry_resolves_to_object(struct ref_entry *entry)
-{
- return ref_resolves_to_object(entry->name,
- &entry->u.value.oid, entry->flag);
-}
-
struct packed_ref_cache {
struct ref_cache *cache;
@@ -547,11 +536,6 @@ static struct ref_cache *get_loose_ref_cache(struct
files_ref_store *refs)
return refs->loose;
}
-static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)
-{
- return get_ref_dir(get_loose_ref_cache(refs)->root);
-}
-
/*
* Return the ref_entry for the given refname from the packed
* references. If it does not exist, return NULL.
@@ -1408,65 +1392,6 @@ struct ref_to_prune {
char name[FLEX_ARRAY];
};
-struct pack_refs_cb_data {
- unsigned int flags;
- struct ref_dir *packed_refs;
- struct ref_to_prune *ref_to_prune;
-};
-
-/*
- * An each_ref_entry_fn that is run over loose references only. If
- * the loose reference can be packed, add an entry in the packed ref
- * cache. If the reference should be pruned, also add it to
- * ref_to_prune in the pack_refs_cb_data.
- */
-static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
-{
- struct pack_refs_cb_data *cb = cb_data;
- enum peel_status peel_status;
- struct ref_entry *packed_entry;
- int is_tag_ref = starts_with(entry->name, "refs/tags/");
-
- /* Do not pack per-worktree refs: */
- if (ref_type(entry->name) != REF_TYPE_NORMAL)
- return 0;
-
- /* ALWAYS pack tags */
- if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref)
- return 0;
-
- /* Do not pack symbolic or broken refs: */
- if ((entry->flag & REF_ISSYMREF) || !entry_resolves_to_object(entry))
- return 0;
-
- /* Add a packed ref cache entry equivalent to the loose entry. */
- peel_status = peel_entry(entry, 1);
- if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
- die("internal error peeling reference %s (%s)",
- entry->name, oid_to_hex(&entry->u.value.oid));
- packed_entry = find_ref_entry(cb->packed_refs, entry->name);
- if (packed_entry) {
- /* Overwrite existing packed entry with info from loose entry */
- packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
- oidcpy(&packed_entry->u.value.oid, &entry->u.value.oid);
- } else {
- packed_entry = create_ref_entry(entry->name,
entry->u.value.oid.hash,
- REF_ISPACKED |
REF_KNOWS_PEELED, 0);
- add_ref_entry(cb->packed_refs, packed_entry);
- }
- oidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);
-
- /* Schedule the loose reference for pruning if requested. */
- if ((cb->flags & PACK_REFS_PRUNE)) {
- struct ref_to_prune *n;
- FLEX_ALLOC_STR(n, name, entry->name);
- hashcpy(n->sha1, entry->u.value.oid.hash);
- n->next = cb->ref_to_prune;
- cb->ref_to_prune = n;
- }
- return 0;
-}
-
enum {
REMOVE_EMPTY_PARENTS_REF = 0x01,
REMOVE_EMPTY_PARENTS_REFLOG = 0x02
@@ -1556,21 +1481,73 @@ static int files_pack_refs(struct ref_store *ref_store,
unsigned int flags)
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
"pack_refs");
- struct pack_refs_cb_data cbdata;
-
- memset(&cbdata, 0, sizeof(cbdata));
- cbdata.flags = flags;
+ struct ref_iterator *iter;
+ struct ref_dir *packed_refs;
+ int ok;
+ struct ref_to_prune *refs_to_prune = NULL;
lock_packed_refs(refs, LOCK_DIE_ON_ERROR);
- cbdata.packed_refs = get_packed_refs(refs);
+ packed_refs = get_packed_refs(refs);
+
+ iter = cache_ref_iterator_begin(get_loose_ref_cach
[PATCH v3 13/20] refs: handle "refs/bisect/" in `loose_fill_ref_dir()`
That "refs/bisect/" has to be handled specially when filling the
ref_cache for loose references is a peculiarity of the files backend,
and the ref-cache code shouldn't need to know about it. So move this
code to the callback function, `loose_fill_ref_dir()`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 15 +++
refs/ref-cache.c | 16
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index ff9251b9cd..079ba941ef 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -507,6 +507,21 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
strbuf_release(&refname);
strbuf_release(&path);
closedir(d);
+
+ /*
+* Manually add refs/bisect, which, being per-worktree, might
+* not appear in the directory listing for refs/ in the main
+* repo.
+*/
+ if (!strcmp(dirname, "refs/")) {
+ int pos = search_ref_dir(dir, "refs/bisect/", 12);
+
+ if (pos < 0) {
+ struct ref_entry *child_entry = create_dir_entry(
+ dir->cache, "refs/bisect/", 12, 1);
+ add_entry_to_dir(dir, child_entry);
+ }
+ }
}
static struct ref_dir *get_loose_refs(struct files_ref_store *refs)
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 7f247b9170..0e0c13 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -26,22 +26,6 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)
die("BUG: incomplete ref_store without fill_ref_dir
function");
dir->cache->fill_ref_dir(dir->cache->ref_store, dir,
entry->name);
-
- /*
-* Manually add refs/bisect, which, being
-* per-worktree, might not appear in the directory
-* listing for refs/ in the main repo.
-*/
- if (!strcmp(entry->name, "refs/")) {
- int pos = search_ref_dir(dir, "refs/bisect/", 12);
- if (pos < 0) {
- struct ref_entry *child_entry;
- child_entry = create_dir_entry(dir->cache,
- "refs/bisect/",
- 12, 1);
- add_entry_to_dir(dir, child_entry);
- }
- }
entry->flag &= ~REF_INCOMPLETE;
}
return dir;
--
2.11.0
[PATCH v3 11/20] refs: record the ref_store in ref_cache, not ref_dir
Instead of keeping a pointer to the `ref_store` in every `ref_dir`
entry, store it once in `struct ref_cache`, and change `struct
ref_dir` to include a pointer to its containing `ref_cache` instead.
This makes it easier to add to the information that is accessible from
a `ref_dir` without increasing the size of every `ref_dir` instance.
Note that previously, every `ref_dir` pointed at the containing
`files_ref_store` regardless of whether it was a part of the loose or
packed reference cache. Now we have to be sure to initialize the
instances to point at the correct containing `ref_cache`. So change
`create_dir_entry()` to take a `ref_cache` parameter, and change its
callers to pass the correct `ref_cache` depending on the purpose of
the new `dir_entry`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 6 +++---
refs/ref-cache.c | 12 +++-
refs/ref-cache.h | 9 ++---
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 840a0869a0..1ed3a30d82 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -432,7 +432,7 @@ static void add_packed_ref(struct files_ref_store *refs,
*/
void read_loose_refs(const char *dirname, struct ref_dir *dir)
{
- struct files_ref_store *refs = dir->ref_store;
+ struct files_ref_store *refs = dir->cache->ref_store;
DIR *d;
struct dirent *de;
int dirnamelen = strlen(dirname);
@@ -468,7 +468,7 @@ void read_loose_refs(const char *dirname, struct ref_dir
*dir)
} else if (S_ISDIR(st.st_mode)) {
strbuf_addch(&refname, '/');
add_entry_to_dir(dir,
-create_dir_entry(refs, refname.buf,
+create_dir_entry(dir->cache,
refname.buf,
refname.len, 1));
} else {
if (!refs_resolve_ref_unsafe(&refs->base,
@@ -525,7 +525,7 @@ static struct ref_dir *get_loose_refs(struct
files_ref_store *refs)
* lazily):
*/
add_entry_to_dir(get_ref_dir(refs->loose->root),
-create_dir_entry(refs, "refs/", 5, 1));
+create_dir_entry(refs->loose, "refs/", 5, 1));
}
return get_ref_dir(refs->loose->root);
}
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index bf911028c8..96da094788 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -36,7 +36,7 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)
int pos = search_ref_dir(dir, "refs/bisect/", 12);
if (pos < 0) {
struct ref_entry *child_entry;
- child_entry = create_dir_entry(dir->ref_store,
+ child_entry = create_dir_entry(dir->cache,
"refs/bisect/",
12, 1);
add_entry_to_dir(dir, child_entry);
@@ -67,7 +67,8 @@ struct ref_cache *create_ref_cache(struct files_ref_store
*refs)
{
struct ref_cache *ret = xcalloc(1, sizeof(*ret));
- ret->root = create_dir_entry(refs, "", 0, 1);
+ ret->ref_store = refs;
+ ret->root = create_dir_entry(ret, "", 0, 1);
return ret;
}
@@ -104,13 +105,14 @@ static void clear_ref_dir(struct ref_dir *dir)
dir->entries = NULL;
}
-struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
+struct ref_entry *create_dir_entry(struct ref_cache *cache,
const char *dirname, size_t len,
int incomplete)
{
struct ref_entry *direntry;
+
FLEX_ALLOC_MEM(direntry, name, dirname, len);
- direntry->u.subdir.ref_store = ref_store;
+ direntry->u.subdir.cache = cache;
direntry->flag = REF_DIR | (incomplete ? REF_INCOMPLETE : 0);
return direntry;
}
@@ -181,7 +183,7 @@ static struct ref_dir *search_for_subdir(struct ref_dir
*dir,
* therefore, create an empty record for it but mark
* the record complete.
*/
- entry = create_dir_entry(dir->ref_store, subdirname, len, 0);
+ entry = create_dir_entry(dir->cache, subdirname, len, 0);
add_entry_to_dir(dir, entry);
} else {
entry = dir->entries[entry_index];
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index da5388c136..83051854ff 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -3,6 +3,9 @@
struct ref_cache {
struct ref_entry *root;
+
+ /* A pointer to the files_ref_store whose cache this is: */
+ struct files_ref_store *ref_store;
};
/*
@@ -66,8 +69,8 @@ struct ref_dir {
*/
i
[PATCH v3 10/20] ref-cache: introduce a new type, ref_cache
For now, it just wraps a `ref_entry *` that points at the root of the
tree. Soon it will hold more information.
Add two new functions, `create_ref_cache()` and `free_ref_cache()`.
Make `free_ref_entry()` private.
Change files-backend to use this type to hold its caches.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 28 +---
refs/ref-cache.c | 16 +++-
refs/ref-cache.h | 15 ++-
3 files changed, 46 insertions(+), 13 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index fcace124de..840a0869a0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -44,7 +44,7 @@ static int entry_resolves_to_object(struct ref_entry *entry)
}
struct packed_ref_cache {
- struct ref_entry *root;
+ struct ref_cache *cache;
/*
* Count of references to the data structure in this instance,
@@ -79,7 +79,7 @@ struct files_ref_store {
char *gitcommondir;
char *packed_refs_path;
- struct ref_entry *loose;
+ struct ref_cache *loose;
struct packed_ref_cache *packed;
};
@@ -101,7 +101,7 @@ static void acquire_packed_ref_cache(struct
packed_ref_cache *packed_refs)
static int release_packed_ref_cache(struct packed_ref_cache *packed_refs)
{
if (!--packed_refs->referrers) {
- free_ref_entry(packed_refs->root);
+ free_ref_cache(packed_refs->cache);
stat_validity_clear(&packed_refs->validity);
free(packed_refs);
return 1;
@@ -125,7 +125,7 @@ static void clear_packed_ref_cache(struct files_ref_store
*refs)
static void clear_loose_ref_cache(struct files_ref_store *refs)
{
if (refs->loose) {
- free_ref_entry(refs->loose);
+ free_ref_cache(refs->loose);
refs->loose = NULL;
}
}
@@ -386,11 +386,12 @@ static struct packed_ref_cache
*get_packed_ref_cache(struct files_ref_store *ref
refs->packed = xcalloc(1, sizeof(*refs->packed));
acquire_packed_ref_cache(refs->packed);
- refs->packed->root = create_dir_entry(refs, "", 0, 0);
+ refs->packed->cache = create_ref_cache(refs);
+ refs->packed->cache->root->flag &= ~REF_INCOMPLETE;
f = fopen(packed_refs_file, "r");
if (f) {
stat_validity_update(&refs->packed->validity,
fileno(f));
- read_packed_refs(f, get_ref_dir(refs->packed->root));
+ read_packed_refs(f,
get_ref_dir(refs->packed->cache->root));
fclose(f);
}
}
@@ -399,7 +400,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct
files_ref_store *ref
static struct ref_dir *get_packed_ref_dir(struct packed_ref_cache
*packed_ref_cache)
{
- return get_ref_dir(packed_ref_cache->root);
+ return get_ref_dir(packed_ref_cache->cache->root);
}
static struct ref_dir *get_packed_refs(struct files_ref_store *refs)
@@ -514,14 +515,19 @@ static struct ref_dir *get_loose_refs(struct
files_ref_store *refs)
* are about to read the only subdirectory that can
* hold references:
*/
- refs->loose = create_dir_entry(refs, "", 0, 0);
+ refs->loose = create_ref_cache(refs);
+
+ /* We're going to fill the top level ourselves: */
+ refs->loose->root->flag &= ~REF_INCOMPLETE;
+
/*
-* Create an incomplete entry for "refs/":
+* Add an incomplete entry for "refs/" (to be filled
+* lazily):
*/
- add_entry_to_dir(get_ref_dir(refs->loose),
+ add_entry_to_dir(get_ref_dir(refs->loose->root),
create_dir_entry(refs, "refs/", 5, 1));
}
- return get_ref_dir(refs->loose);
+ return get_ref_dir(refs->loose->root);
}
/*
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 4274a43a9b..bf911028c8 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -63,9 +63,17 @@ struct ref_entry *create_ref_entry(const char *refname,
return ref;
}
+struct ref_cache *create_ref_cache(struct files_ref_store *refs)
+{
+ struct ref_cache *ret = xcalloc(1, sizeof(*ret));
+
+ ret->root = create_dir_entry(refs, "", 0, 1);
+ return ret;
+}
+
static void clear_ref_dir(struct ref_dir *dir);
-void free_ref_entry(struct ref_entry *entry)
+static void free_ref_entry(struct ref_entry *entry)
{
if (entry->flag & REF_DIR) {
/*
@@ -77,6 +85,12 @@ void free_ref_entry(struct ref_entry *entry)
free(entry);
}
+void free_ref_cache(struct ref_cache *cache)
+{
+ free_ref_entry(cache->root);
+ free(cache);
+}
+
/*
* Clear and free all entries in dir, recursively.
*/
diff --git a/refs/ref-cache.h b/refs/re
[PATCH v3 18/20] commit_packed_refs(): use reference iteration
Use reference iteration rather than do_for_each_entry_in_dir() in the
definition of commit_packed_refs().
Note that an internal consistency check that was previously done in
`write_packed_entry_fn()` is not there anymore. This is actually an
improvement:
The old error message was emitted when there is an entry in the
packed-ref cache that is not `REF_KNOWS_PEELED`, and when we attempted
to peel the reference, the result was `PEEL_INVALID`,
`PEEL_IS_SYMREF`, or `PEEL_BROKEN`. Since a packed ref cannot be a
symref, `PEEL_IS_SYMREF` and `PEEL_BROKEN` can be ruled out. So we're
left with `PEEL_INVALID`.
An entry without `REF_KNOWS_PEELED` can get into the packed-refs cache
in the following two ways:
* The reference was read from a `packed-refs` file that didn't have
the `fully-peeled` attribute. In that case, we *don't want* to emit
an error, because the broken value is presumably a stale value of
the reference that is now masked by a loose version of the same
reference (which we just don't happen to be packing this time). This
is a perfectly legitimate situation and doesn't indicate that the
repository is corrupt. The old code incorrectly emits an error
message in this case. (It was probably never reported as a bug
because this scenario is rare.)
* The reference was a loose reference that was just added to the
packed ref cache by `files_packed_refs()` via
`pack_if_possible_fn()` in preparation for being packed. The latter
function refuses to pack a reference for which
`entry_resolves_to_object()` returns false, and otherwise calls
`peel_entry()` itself and checks the return value. So an entry added
this way should always have `REF_KNOWS_PEELED` and shouldn't trigger
the error message in either the old code or the new.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 38 +-
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8c360869c1..1419512d51 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1290,30 +1290,15 @@ static struct ref_lock *lock_ref_sha1_basic(struct
files_ref_store *refs,
* Write an entry to the packed-refs file for the specified refname.
* If peeled is non-NULL, write it as the entry's peeled value.
*/
-static void write_packed_entry(FILE *fh, char *refname, unsigned char *sha1,
- unsigned char *peeled)
+static void write_packed_entry(FILE *fh, const char *refname,
+ const unsigned char *sha1,
+ const unsigned char *peeled)
{
fprintf_or_die(fh, "%s %s\n", sha1_to_hex(sha1), refname);
if (peeled)
fprintf_or_die(fh, "^%s\n", sha1_to_hex(peeled));
}
-/*
- * An each_ref_entry_fn that writes the entry to a packed-refs file.
- */
-static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
-{
- enum peel_status peel_status = peel_entry(entry, 0);
-
- if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
- error("internal error: %s is not a valid packed reference!",
- entry->name);
- write_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,
- peel_status == PEEL_PEELED ?
- entry->u.value.peeled.hash : NULL);
- return 0;
-}
-
/*
* Lock the packed-refs file for writing. Flags is passed to
* hold_lock_file_for_update(). Return 0 on success. On errors, set
@@ -1359,9 +1344,10 @@ static int commit_packed_refs(struct files_ref_store
*refs)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(refs);
- int error = 0;
+ int ok, error = 0;
int save_errno = 0;
FILE *out;
+ struct ref_iterator *iter;
files_assert_main_repository(refs, "commit_packed_refs");
@@ -1373,8 +1359,18 @@ static int commit_packed_refs(struct files_ref_store
*refs)
die_errno("unable to fdopen packed-refs descriptor");
fprintf_or_die(out, "%s", PACKED_REFS_HEADER);
- do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
-write_packed_entry_fn, out);
+
+ iter = cache_ref_iterator_begin(packed_ref_cache->cache, NULL, 0);
+ while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
+ struct object_id peeled;
+ int peel_error = ref_iterator_peel(iter, &peeled);
+
+ write_packed_entry(out, iter->refname, iter->oid->hash,
+ peel_error ? NULL : peeled.hash);
+ }
+
+ if (ok != ITER_DONE)
+ die("error while iterating over references");
if (commit_lock_file(packed_ref_cache->lock)) {
save_errno = errno;
--
2.11.0
[PATCH v3 08/20] ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`
This function's visibility is about to be increased, so give it a more
distinctive name.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 6e08bc798c..f980af2420 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -413,7 +413,7 @@ static struct ref_entry *find_ref_entry(struct ref_dir
*dir, const char *refname
* empty by the removal. dir must represent the top-level directory
* and must already be complete.
*/
-static int remove_entry(struct ref_dir *dir, const char *refname)
+static int remove_entry_from_dir(struct ref_dir *dir, const char *refname)
{
int refname_len = strlen(refname);
int entry_index;
@@ -2335,7 +2335,7 @@ static int repack_without_refs(struct files_ref_store
*refs,
/* Remove refnames from the cache */
for_each_string_list_item(refname, refnames)
- if (remove_entry(packed, refname->string) != -1)
+ if (remove_entry_from_dir(packed, refname->string) != -1)
removed = 1;
if (!removed) {
/*
--
2.11.0
[PATCH v3 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()
The new name is more analogous to `get_packed_ref_dir()`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c0550ad9d6..3beab0b752 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -524,7 +524,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
}
}
-static struct ref_dir *get_loose_refs(struct files_ref_store *refs)
+static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)
{
if (!refs->loose) {
/*
@@ -1110,7 +1110,7 @@ static struct ref_iterator *files_ref_iterator_begin(
* date with what is on disk, and re-reads it if not.
*/
- loose_dir = get_loose_refs(refs);
+ loose_dir = get_loose_ref_dir(refs);
if (prefix && *prefix)
loose_dir = find_containing_dir(loose_dir, prefix, 0);
@@ -1581,7 +1581,7 @@ static int files_pack_refs(struct ref_store *ref_store,
unsigned int flags)
lock_packed_refs(refs, LOCK_DIE_ON_ERROR);
cbdata.packed_refs = get_packed_refs(refs);
- do_for_each_entry_in_dir(get_loose_refs(refs),
+ do_for_each_entry_in_dir(get_loose_ref_dir(refs),
pack_if_possible_fn, &cbdata);
if (commit_packed_refs(refs))
--
2.11.0
[PATCH v3 20/20] do_for_each_entry_in_dir(): delete function
Its only remaining caller was itself.
Signed-off-by: Michael Haggerty
---
refs/ref-cache.c | 21 -
refs/ref-cache.h | 11 ---
2 files changed, 32 deletions(-)
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index b3a30350d7..6059362f1d 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -316,27 +316,6 @@ static void sort_ref_dir(struct ref_dir *dir)
dir->sorted = dir->nr = i;
}
-int do_for_each_entry_in_dir(struct ref_dir *dir,
-each_ref_entry_fn fn, void *cb_data)
-{
- int i;
- assert(dir->sorted == dir->nr);
- for (i = 0; i < dir->nr; i++) {
- struct ref_entry *entry = dir->entries[i];
- int retval;
- if (entry->flag & REF_DIR) {
- struct ref_dir *subdir = get_ref_dir(entry);
- sort_ref_dir(subdir);
- retval = do_for_each_entry_in_dir(subdir, fn, cb_data);
- } else {
- retval = fn(entry, cb_data);
- }
- if (retval)
- return retval;
- }
- return 0;
-}
-
/*
* Load all of the refs from `dir` (recursively) into our in-memory
* cache.
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index 5e7a918ac0..ffdc54f3f0 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -251,17 +251,6 @@ struct ref_iterator *cache_ref_iterator_begin(struct
ref_cache *cache,
const char *prefix,
int prime_dir);
-typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);
-
-/*
- * Call `fn` for each reference in `dir`. Recurse into subdirectories,
- * sorting them before iterating. This function does not sort `dir`
- * itself; it should be sorted beforehand. `fn` is called for all
- * references, including broken ones.
- */
-int do_for_each_entry_in_dir(struct ref_dir *dir,
-each_ref_entry_fn fn, void *cb_data);
-
/*
* Peel the entry (if possible) and return its new peel_status. If
* repeel is true, re-peel the entry even if there is an old peeled
--
2.11.0
[PATCH v3 06/20] ref-cache: rename `add_ref()` to `add_ref_entry()`
This function's visibility is about to be increased, so give it a more
distinctive name.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index cf1c18cffb..05029d43b8 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -455,7 +455,7 @@ static int remove_entry(struct ref_dir *dir, const char
*refname)
* subdirectories as necessary. dir must represent the top-level
* directory. Return 0 on success.
*/
-static int add_ref(struct ref_dir *dir, struct ref_entry *ref)
+static int add_ref_entry(struct ref_dir *dir, struct ref_entry *ref)
{
dir = find_containing_dir(dir, ref->name, 1);
if (!dir)
@@ -993,7 +993,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
if (peeled == PEELED_FULLY ||
(peeled == PEELED_TAGS && starts_with(refname,
"refs/tags/")))
last->flag |= REF_KNOWS_PEELED;
- add_ref(dir, last);
+ add_ref_entry(dir, last);
continue;
}
if (last &&
@@ -1115,7 +1115,7 @@ static void add_packed_ref(struct files_ref_store *refs,
if (!packed_ref_cache->lock)
die("internal error: packed refs not locked");
- add_ref(get_packed_ref_dir(packed_ref_cache),
+ add_ref_entry(get_packed_ref_dir(packed_ref_cache),
create_ref_entry(refname, sha1, REF_ISPACKED, 1));
}
@@ -2176,7 +2176,7 @@ static int pack_if_possible_fn(struct ref_entry *entry,
void *cb_data)
} else {
packed_entry = create_ref_entry(entry->name,
entry->u.value.oid.hash,
REF_ISPACKED |
REF_KNOWS_PEELED, 0);
- add_ref(cb->packed_refs, packed_entry);
+ add_ref_entry(cb->packed_refs, packed_entry);
}
oidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);
--
2.11.0
[PATCH v3 05/20] refs_verify_refname_available(): use function in more places
Change `lock_raw_ref()` and `lock_ref_sha1_basic()` to use
`refs_verify_refname_available()` instead of
`verify_refname_available_dir()`. This means that those callsites now
check for conflicts with all references rather than just packed refs,
but the performance cost shouldn't be significant (and will be
regained later).
These were the last callers of `verify_refname_available_dir()`, so
also delete that (very complicated) function.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 171 ---
1 file changed, 11 insertions(+), 160 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 4185025efa..cf1c18cffb 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -738,152 +738,6 @@ static struct ref_iterator
*cache_ref_iterator_begin(struct ref_dir *dir)
return ref_iterator;
}
-struct nonmatching_ref_data {
- const struct string_list *skip;
- const char *conflicting_refname;
-};
-
-static int nonmatching_ref_fn(struct ref_entry *entry, void *vdata)
-{
- struct nonmatching_ref_data *data = vdata;
-
- if (data->skip && string_list_has_string(data->skip, entry->name))
- return 0;
-
- data->conflicting_refname = entry->name;
- return 1;
-}
-
-/*
- * Return 0 if a reference named refname could be created without
- * conflicting with the name of an existing reference in dir.
- * See verify_refname_available for more information.
- */
-static int verify_refname_available_dir(const char *refname,
- const struct string_list *extras,
- const struct string_list *skip,
- struct ref_dir *dir,
- struct strbuf *err)
-{
- const char *slash;
- const char *extra_refname;
- int pos;
- struct strbuf dirname = STRBUF_INIT;
- int ret = -1;
-
- /*
-* For the sake of comments in this function, suppose that
-* refname is "refs/foo/bar".
-*/
-
- assert(err);
-
- strbuf_grow(&dirname, strlen(refname) + 1);
- for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1,
'/')) {
- /* Expand dirname to the new prefix, not including the trailing
slash: */
- strbuf_add(&dirname, refname + dirname.len, slash - refname -
dirname.len);
-
- /*
-* We are still at a leading dir of the refname (e.g.,
-* "refs/foo"; if there is a reference with that name,
-* it is a conflict, *unless* it is in skip.
-*/
- if (dir) {
- pos = search_ref_dir(dir, dirname.buf, dirname.len);
- if (pos >= 0 &&
- (!skip || !string_list_has_string(skip,
dirname.buf))) {
- /*
-* We found a reference whose name is
-* a proper prefix of refname; e.g.,
-* "refs/foo", and is not in skip.
-*/
- strbuf_addf(err, "'%s' exists; cannot create
'%s'",
- dirname.buf, refname);
- goto cleanup;
- }
- }
-
- if (extras && string_list_has_string(extras, dirname.buf) &&
- (!skip || !string_list_has_string(skip, dirname.buf))) {
- strbuf_addf(err, "cannot process '%s' and '%s' at the
same time",
- refname, dirname.buf);
- goto cleanup;
- }
-
- /*
-* Otherwise, we can try to continue our search with
-* the next component. So try to look up the
-* directory, e.g., "refs/foo/". If we come up empty,
-* we know there is nothing under this whole prefix,
-* but even in that case we still have to continue the
-* search for conflicts with extras.
-*/
- strbuf_addch(&dirname, '/');
- if (dir) {
- pos = search_ref_dir(dir, dirname.buf, dirname.len);
- if (pos < 0) {
- /*
-* There was no directory "refs/foo/",
-* so there is nothing under this
-* whole prefix. So there is no need
-* to continue looking for conflicting
-* references. But we need to continue
-* looking for conflicting extras.
-*/
- dir = NULL;
- } else {
-
[PATCH v3 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument
It was never used.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 4 ++--
refs/ref-cache.c | 6 +++---
refs/ref-cache.h | 11 +--
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 079ba941ef..c0550ad9d6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1387,7 +1387,7 @@ static int commit_packed_refs(struct files_ref_store
*refs)
fprintf_or_die(out, "%s", PACKED_REFS_HEADER);
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
-0, write_packed_entry_fn, out);
+write_packed_entry_fn, out);
if (commit_lock_file(packed_ref_cache->lock)) {
save_errno = errno;
@@ -1581,7 +1581,7 @@ static int files_pack_refs(struct ref_store *ref_store,
unsigned int flags)
lock_packed_refs(refs, LOCK_DIE_ON_ERROR);
cbdata.packed_refs = get_packed_refs(refs);
- do_for_each_entry_in_dir(get_loose_refs(refs), 0,
+ do_for_each_entry_in_dir(get_loose_refs(refs),
pack_if_possible_fn, &cbdata);
if (commit_packed_refs(refs))
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 0e0c13..38d4c31985 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -307,18 +307,18 @@ static void sort_ref_dir(struct ref_dir *dir)
dir->sorted = dir->nr = i;
}
-int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
+int do_for_each_entry_in_dir(struct ref_dir *dir,
each_ref_entry_fn fn, void *cb_data)
{
int i;
assert(dir->sorted == dir->nr);
- for (i = offset; i < dir->nr; i++) {
+ for (i = 0; i < dir->nr; i++) {
struct ref_entry *entry = dir->entries[i];
int retval;
if (entry->flag & REF_DIR) {
struct ref_dir *subdir = get_ref_dir(entry);
sort_ref_dir(subdir);
- retval = do_for_each_entry_in_dir(subdir, 0, fn,
cb_data);
+ retval = do_for_each_entry_in_dir(subdir, fn, cb_data);
} else {
retval = fn(entry, cb_data);
}
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index ed51e80d88..6eecdf4276 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -258,13 +258,12 @@ struct ref_iterator *cache_ref_iterator_begin(struct
ref_dir *dir);
typedef int each_ref_entry_fn(struct ref_entry *entry, void *cb_data);
/*
- * Call fn for each reference in dir that has index in the range
- * offset <= index < dir->nr. Recurse into subdirectories that are in
- * that index range, sorting them before iterating. This function
- * does not sort dir itself; it should be sorted beforehand. fn is
- * called for all references, including broken ones.
+ * Call `fn` for each reference in `dir`. Recurse into subdirectories,
+ * sorting them before iterating. This function does not sort `dir`
+ * itself; it should be sorted beforehand. `fn` is called for all
+ * references, including broken ones.
*/
-int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
+int do_for_each_entry_in_dir(struct ref_dir *dir,
each_ref_entry_fn fn, void *cb_data);
/*
--
2.11.0
[PATCH v3 12/20] ref-cache: use a callback function to fill the cache
It is a leveling violation for `ref_cache` to know about
`files_ref_store` or that it should call `read_loose_refs()` to lazily
fill cache directories. So instead, have its constructor take as an
argument a callback function that it should use for lazy-filling, and
change `files_ref_store` to supply a pointer to function
`read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the
ref cache for its loose refs.
This means that we can generify the type of the back-pointer in
`struct ref_cache` from `files_ref_store` to `ref_store`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 10 ++
refs/ref-cache.c | 12 +++-
refs/ref-cache.h | 29 +
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1ed3a30d82..ff9251b9cd 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -386,7 +386,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct
files_ref_store *ref
refs->packed = xcalloc(1, sizeof(*refs->packed));
acquire_packed_ref_cache(refs->packed);
- refs->packed->cache = create_ref_cache(refs);
+ refs->packed->cache = create_ref_cache(&refs->base, NULL);
refs->packed->cache->root->flag &= ~REF_INCOMPLETE;
f = fopen(packed_refs_file, "r");
if (f) {
@@ -430,9 +430,11 @@ static void add_packed_ref(struct files_ref_store *refs,
* (without recursing). dirname must end with '/'. dir must be the
* directory entry corresponding to dirname.
*/
-void read_loose_refs(const char *dirname, struct ref_dir *dir)
+static void loose_fill_ref_dir(struct ref_store *ref_store,
+ struct ref_dir *dir, const char *dirname)
{
- struct files_ref_store *refs = dir->cache->ref_store;
+ struct files_ref_store *refs =
+ files_downcast(ref_store, REF_STORE_READ, "fill_ref_dir");
DIR *d;
struct dirent *de;
int dirnamelen = strlen(dirname);
@@ -515,7 +517,7 @@ static struct ref_dir *get_loose_refs(struct
files_ref_store *refs)
* are about to read the only subdirectory that can
* hold references:
*/
- refs->loose = create_ref_cache(refs);
+ refs->loose = create_ref_cache(&refs->base, loose_fill_ref_dir);
/* We're going to fill the top level ourselves: */
refs->loose->root->flag &= ~REF_INCOMPLETE;
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 96da094788..7f247b9170 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -4,9 +4,6 @@
#include "ref-cache.h"
#include "../iterator.h"
-/* FIXME: This declaration shouldn't be here */
-void read_loose_refs(const char *dirname, struct ref_dir *dir);
-
void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry)
{
ALLOC_GROW(dir->entries, dir->nr + 1, dir->alloc);
@@ -25,7 +22,10 @@ struct ref_dir *get_ref_dir(struct ref_entry *entry)
assert(entry->flag & REF_DIR);
dir = &entry->u.subdir;
if (entry->flag & REF_INCOMPLETE) {
- read_loose_refs(entry->name, dir);
+ if (!dir->cache->fill_ref_dir)
+ die("BUG: incomplete ref_store without fill_ref_dir
function");
+
+ dir->cache->fill_ref_dir(dir->cache->ref_store, dir,
entry->name);
/*
* Manually add refs/bisect, which, being
@@ -63,11 +63,13 @@ struct ref_entry *create_ref_entry(const char *refname,
return ref;
}
-struct ref_cache *create_ref_cache(struct files_ref_store *refs)
+struct ref_cache *create_ref_cache(struct ref_store *refs,
+ fill_ref_dir_fn *fill_ref_dir)
{
struct ref_cache *ret = xcalloc(1, sizeof(*ret));
ret->ref_store = refs;
+ ret->fill_ref_dir = fill_ref_dir;
ret->root = create_dir_entry(ret, "", 0, 1);
return ret;
}
diff --git a/refs/ref-cache.h b/refs/ref-cache.h
index 83051854ff..ed51e80d88 100644
--- a/refs/ref-cache.h
+++ b/refs/ref-cache.h
@@ -1,11 +1,27 @@
#ifndef REFS_REF_CACHE_H
#define REFS_REF_CACHE_H
+struct ref_dir;
+
+/*
+ * If this ref_cache is filled lazily, this function is used to load
+ * information into the specified ref_dir (shallow or deep, at the
+ * option of the ref_store). dirname includes a trailing slash.
+ */
+typedef void fill_ref_dir_fn(struct ref_store *ref_store,
+struct ref_dir *dir, const char *dirname);
+
struct ref_cache {
struct ref_entry *root;
- /* A pointer to the files_ref_store whose cache this is: */
- struct files_ref_store *ref_store;
+ /* A pointer to the ref_store whose cache this is: */
+ struct ref_store *ref_store;
+
+ /*
+* Function used (if necessary) to lazily-fill cache. May be
+* NULL.
+*/
+
[PATCH v3 16/20] get_loose_ref_cache(): new function
Extract a new function, `get_loose_ref_cache()`, from
get_loose_ref_dir(). The function returns the `ref_cache` for the
loose refs of a `files_ref_store`.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 3beab0b752..7ae7c6a1b7 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -524,7 +524,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
}
}
-static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)
+static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
{
if (!refs->loose) {
/*
@@ -544,7 +544,12 @@ static struct ref_dir *get_loose_ref_dir(struct
files_ref_store *refs)
add_entry_to_dir(get_ref_dir(refs->loose->root),
create_dir_entry(refs->loose, "refs/", 5, 1));
}
- return get_ref_dir(refs->loose->root);
+ return refs->loose;
+}
+
+static struct ref_dir *get_loose_ref_dir(struct files_ref_store *refs)
+{
+ return get_ref_dir(get_loose_ref_cache(refs)->root);
}
/*
--
2.11.0
[PATCH v3 02/20] refs_read_raw_ref(): new function
Extract a new function from `refs_resolve_ref_unsafe()`. It will be
useful elsewhere.
Signed-off-by: Michael Haggerty
---
refs.c | 11 +--
refs/refs-internal.h | 4
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index bad05ba861..aa461156c4 100644
--- a/refs.c
+++ b/refs.c
@@ -1326,6 +1326,13 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
return refs_for_each_rawref(get_main_ref_store(), fn, cb_data);
}
+int refs_read_raw_ref(struct ref_store *ref_store,
+ const char *refname, unsigned char *sha1,
+ struct strbuf *referent, unsigned int *type)
+{
+ return ref_store->be->read_raw_ref(ref_store, refname, sha1, referent,
type);
+}
+
/* This function needs to return a meaningful errno on failure */
const char *refs_resolve_ref_unsafe(struct ref_store *refs,
const char *refname,
@@ -1362,8 +1369,8 @@ const char *refs_resolve_ref_unsafe(struct ref_store
*refs,
for (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {
unsigned int read_flags = 0;
- if (refs->be->read_raw_ref(refs, refname,
- sha1, &sb_refname, &read_flags)) {
+ if (refs_read_raw_ref(refs, refname,
+ sha1, &sb_refname, &read_flags)) {
*flags |= read_flags;
if (errno != ENOENT || (resolve_flags &
RESOLVE_REF_READING))
return NULL;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 690498698e..6ee9f20dbc 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -165,6 +165,10 @@ struct ref_update {
const char refname[FLEX_ARRAY];
};
+int refs_read_raw_ref(struct ref_store *ref_store,
+ const char *refname, unsigned char *sha1,
+ struct strbuf *referent, unsigned int *type);
+
/*
* Add a ref_update with the specified properties to transaction, and
* return a pointer to the new object. This function does not verify
--
2.11.0
[PATCH v3 09/20] refs: split `ref_cache` code into separate files
The `ref_cache` code is currently too tightly coupled to
`files-backend`, making the code harder to understand and making it
awkward for new code to use `ref_cache` (as we indeed have planned).
Start loosening that coupling by splitting `ref_cache` into a separate
module.
This commit moves code, adds declarations, and changes the visibility
of some functions, but doesn't change any code.
The modules are still too tightly coupled, but the situation will be
improved in subsequent commits.
Signed-off-by: Michael Haggerty
---
Makefile | 1 +
refs/files-backend.c | 736 +--
refs/ref-cache.c | 512 +++
refs/ref-cache.h | 251 ++
4 files changed, 767 insertions(+), 733 deletions(-)
create mode 100644 refs/ref-cache.c
create mode 100644 refs/ref-cache.h
diff --git a/Makefile b/Makefile
index 5f3844e33e..2f30580cde 100644
--- a/Makefile
+++ b/Makefile
@@ -807,6 +807,7 @@ LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
LIB_OBJS += refs/files-backend.o
LIB_OBJS += refs/iterator.o
+LIB_OBJS += refs/ref-cache.o
LIB_OBJS += ref-filter.o
LIB_OBJS += remote.o
LIB_OBJS += replace_object.o
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f980af2420..fcace124de 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,6 +1,7 @@
#include "../cache.h"
#include "../refs.h"
#include "refs-internal.h"
+#include "ref-cache.h"
#include "../iterator.h"
#include "../dir-iterator.h"
#include "../lockfile.h"
@@ -13,509 +14,6 @@ struct ref_lock {
struct object_id old_oid;
};
-struct ref_entry;
-
-/*
- * Information used (along with the information in ref_entry) to
- * describe a single cached reference. This data structure only
- * occurs embedded in a union in struct ref_entry, and only when
- * (ref_entry->flag & REF_DIR) is zero.
- */
-struct ref_value {
- /*
-* The name of the object to which this reference resolves
-* (which may be a tag object). If REF_ISBROKEN, this is
-* null. If REF_ISSYMREF, then this is the name of the object
-* referred to by the last reference in the symlink chain.
-*/
- struct object_id oid;
-
- /*
-* If REF_KNOWS_PEELED, then this field holds the peeled value
-* of this reference, or null if the reference is known not to
-* be peelable. See the documentation for peel_ref() for an
-* exact definition of "peelable".
-*/
- struct object_id peeled;
-};
-
-struct files_ref_store;
-
-/*
- * Information used (along with the information in ref_entry) to
- * describe a level in the hierarchy of references. This data
- * structure only occurs embedded in a union in struct ref_entry, and
- * only when (ref_entry.flag & REF_DIR) is set. In that case,
- * (ref_entry.flag & REF_INCOMPLETE) determines whether the references
- * in the directory have already been read:
- *
- * (ref_entry.flag & REF_INCOMPLETE) unset -- a directory of loose
- * or packed references, already read.
- *
- * (ref_entry.flag & REF_INCOMPLETE) set -- a directory of loose
- * references that hasn't been read yet (nor has any of its
- * subdirectories).
- *
- * Entries within a directory are stored within a growable array of
- * pointers to ref_entries (entries, nr, alloc). Entries 0 <= i <
- * sorted are sorted by their component name in strcmp() order and the
- * remaining entries are unsorted.
- *
- * Loose references are read lazily, one directory at a time. When a
- * directory of loose references is read, then all of the references
- * in that directory are stored, and REF_INCOMPLETE stubs are created
- * for any subdirectories, but the subdirectories themselves are not
- * read. The reading is triggered by get_ref_dir().
- */
-struct ref_dir {
- int nr, alloc;
-
- /*
-* Entries with index 0 <= i < sorted are sorted by name. New
-* entries are appended to the list unsorted, and are sorted
-* only when required; thus we avoid the need to sort the list
-* after the addition of every reference.
-*/
- int sorted;
-
- /* A pointer to the files_ref_store that contains this ref_dir. */
- struct files_ref_store *ref_store;
-
- struct ref_entry **entries;
-};
-
-/*
- * Bit values for ref_entry::flag. REF_ISSYMREF=0x01,
- * REF_ISPACKED=0x02, REF_ISBROKEN=0x04 and REF_BAD_NAME=0x08 are
- * public values; see refs.h.
- */
-
-/*
- * The field ref_entry->u.value.peeled of this value entry contains
- * the correct peeled value for the reference, which might be
- * null_sha1 if the reference is not a tag or if it is broken.
- */
-#define REF_KNOWS_PEELED 0x10
-
-/* ref_entry represents a directory of references */
-#define REF_DIR 0x20
-
-/*
- * Entry has not yet been read from disk (used only for REF_DIR
- * entries representing loose references)
- */
-#define REF_INCOM
[PATCH v3 07/20] ref-cache: rename `find_ref()` to `find_ref_entry()`
This function's visibility is about to be increased, so give it a more
distinctive name.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 05029d43b8..6e08bc798c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -385,7 +385,7 @@ static struct ref_dir *find_containing_dir(struct ref_dir
*dir,
* and recursing into subdirectories as necessary. If the name is not
* found or it corresponds to a directory entry, return NULL.
*/
-static struct ref_entry *find_ref(struct ref_dir *dir, const char *refname)
+static struct ref_entry *find_ref_entry(struct ref_dir *dir, const char
*refname)
{
int entry_index;
struct ref_entry *entry;
@@ -1226,7 +1226,7 @@ static struct ref_dir *get_loose_refs(struct
files_ref_store *refs)
static struct ref_entry *get_packed_ref(struct files_ref_store *refs,
const char *refname)
{
- return find_ref(get_packed_refs(refs), refname);
+ return find_ref_entry(get_packed_refs(refs), refname);
}
/*
@@ -2168,7 +2168,7 @@ static int pack_if_possible_fn(struct ref_entry *entry,
void *cb_data)
if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
die("internal error peeling reference %s (%s)",
entry->name, oid_to_hex(&entry->u.value.oid));
- packed_entry = find_ref(cb->packed_refs, entry->name);
+ packed_entry = find_ref_entry(cb->packed_refs, entry->name);
if (packed_entry) {
/* Overwrite existing packed entry with info from loose entry */
packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
--
2.11.0
[PATCH v3 17/20] cache_ref_iterator_begin(): make function smarter
Change `cache_ref_iterator_begin()` to take two new arguments:
* `prefix` -- to iterate only over references with the specified
prefix.
* `prime_dir` -- to "prime" (i.e., pre-load) the cache before starting
the iteration.
The new functionality makes it possible for
`files_ref_iterator_begin()` to be made more ignorant of the internals
of `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to
be made private.
Signed-off-by: Michael Haggerty
---
refs/files-backend.c | 44 +---
refs/ref-cache.c | 38 ++
refs/ref-cache.h | 27 +--
3 files changed, 56 insertions(+), 53 deletions(-)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ae7c6a1b7..8c360869c1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1082,7 +1082,6 @@ static struct ref_iterator *files_ref_iterator_begin(
const char *prefix, unsigned int flags)
{
struct files_ref_store *refs;
- struct ref_dir *loose_dir, *packed_dir;
struct ref_iterator *loose_iter, *packed_iter;
struct files_ref_iterator *iter;
struct ref_iterator *ref_iterator;
@@ -1106,41 +1105,24 @@ static struct ref_iterator *files_ref_iterator_begin(
* condition if loose refs are migrated to the packed-refs
* file by a simultaneous process, but our in-memory view is
* from before the migration. We ensure this as follows:
-* First, we call prime_ref_dir(), which pre-reads the loose
-* references for the subtree into the cache. (If they've
-* already been read, that's OK; we only need to guarantee
-* that they're read before the packed refs, not *how much*
-* before.) After that, we call get_packed_ref_cache(), which
-* internally checks whether the packed-ref cache is up to
-* date with what is on disk, and re-reads it if not.
+* First, we call start the loose refs iteration with its
+* `prime_ref` argument set to true. This causes the loose
+* references in the subtree to be pre-read into the cache.
+* (If they've already been read, that's OK; we only need to
+* guarantee that they're read before the packed refs, not
+* *how much* before.) After that, we call
+* get_packed_ref_cache(), which internally checks whether the
+* packed-ref cache is up to date with what is on disk, and
+* re-reads it if not.
*/
- loose_dir = get_loose_ref_dir(refs);
-
- if (prefix && *prefix)
- loose_dir = find_containing_dir(loose_dir, prefix, 0);
-
- if (loose_dir) {
- prime_ref_dir(loose_dir);
- loose_iter = cache_ref_iterator_begin(loose_dir);
- } else {
- /* There's nothing to iterate over. */
- loose_iter = empty_ref_iterator_begin();
- }
+ loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),
+ prefix, 1);
iter->packed_ref_cache = get_packed_ref_cache(refs);
acquire_packed_ref_cache(iter->packed_ref_cache);
- packed_dir = get_packed_ref_dir(iter->packed_ref_cache);
-
- if (prefix && *prefix)
- packed_dir = find_containing_dir(packed_dir, prefix, 0);
-
- if (packed_dir) {
- packed_iter = cache_ref_iterator_begin(packed_dir);
- } else {
- /* There's nothing to iterate over. */
- packed_iter = empty_ref_iterator_begin();
- }
+ packed_iter = cache_ref_iterator_begin(iter->packed_ref_cache->cache,
+ prefix, 0);
iter->iter0 = overlay_ref_iterator_begin(loose_iter, packed_iter);
iter->flags = flags;
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 38d4c31985..b3a30350d7 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -177,8 +177,17 @@ static struct ref_dir *search_for_subdir(struct ref_dir
*dir,
return get_ref_dir(entry);
}
-struct ref_dir *find_containing_dir(struct ref_dir *dir,
- const char *refname, int mkdir)
+/*
+ * If refname is a reference name, find the ref_dir within the dir
+ * tree that should hold refname. If refname is a directory name
+ * (i.e., it ends in '/'), then return that ref_dir itself. dir must
+ * represent the top-level directory and must already be complete.
+ * Sort ref_dirs and recurse into subdirectories as necessary. If
+ * mkdir is set, then create any missing directories; otherwise,
+ * return NULL if the desired directory cannot be found.
+ */
+static struct ref_dir *find_containing_dir(struct ref_dir *dir,
+ const char *refname, int mkdir)
{
const char *slash;
for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1,
'/')) {
@@ -328,7 +337,11 @@ int
[PATCH v3 04/20] refs_verify_refname_available(): implement once for all backends
It turns out that we can now implement
`refs_verify_refname_available()` based on the other virtual
functions, so there is no need for it to be defined at the backend
level. Instead, define it once in `refs.c` and remove the
`files_backend` definition.
Signed-off-by: Michael Haggerty
---
refs.c | 85 ++--
refs.h | 2 +-
refs/files-backend.c | 39 +---
refs/refs-internal.h | 7 -
4 files changed, 92 insertions(+), 41 deletions(-)
diff --git a/refs.c b/refs.c
index 2ae18097d4..e841248b01 100644
--- a/refs.c
+++ b/refs.c
@@ -5,6 +5,7 @@
#include "cache.h"
#include "hashmap.h"
#include "lockfile.h"
+#include "iterator.h"
#include "refs.h"
#include "refs/refs-internal.h"
#include "object.h"
@@ -1658,11 +1659,91 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
int refs_verify_refname_available(struct ref_store *refs,
const char *refname,
- const struct string_list *extra,
+ const struct string_list *extras,
const struct string_list *skip,
struct strbuf *err)
{
- return refs->be->verify_refname_available(refs, refname, extra, skip,
err);
+ const char *slash;
+ const char *extra_refname;
+ struct strbuf dirname = STRBUF_INIT;
+ struct strbuf referent = STRBUF_INIT;
+ struct object_id oid;
+ unsigned int type;
+ struct ref_iterator *iter;
+ int ok;
+ int ret = -1;
+
+ /*
+* For the sake of comments in this function, suppose that
+* refname is "refs/foo/bar".
+*/
+
+ assert(err);
+
+ strbuf_grow(&dirname, strlen(refname) + 1);
+ for (slash = strchr(refname, '/'); slash; slash = strchr(slash + 1,
'/')) {
+ /* Expand dirname to the new prefix, not including the trailing
slash: */
+ strbuf_add(&dirname, refname + dirname.len, slash - refname -
dirname.len);
+
+ /*
+* We are still at a leading dir of the refname (e.g.,
+* "refs/foo"; if there is a reference with that name,
+* it is a conflict, *unless* it is in skip.
+*/
+ if (skip && string_list_has_string(skip, dirname.buf))
+ continue;
+
+ if (!refs_read_raw_ref(refs, dirname.buf, oid.hash, &referent,
&type)) {
+ strbuf_addf(err, "'%s' exists; cannot create '%s'",
+ dirname.buf, refname);
+ goto cleanup;
+ }
+
+ if (extras && string_list_has_string(extras, dirname.buf)) {
+ strbuf_addf(err, "cannot process '%s' and '%s' at the
same time",
+ refname, dirname.buf);
+ goto cleanup;
+ }
+ }
+
+ /*
+* We are at the leaf of our refname (e.g., "refs/foo/bar").
+* There is no point in searching for a reference with that
+* name, because a refname isn't considered to conflict with
+* itself. But we still need to check for references whose
+* names are in the "refs/foo/bar/" namespace, because they
+* *do* conflict.
+*/
+ strbuf_addstr(&dirname, refname + dirname.len);
+ strbuf_addch(&dirname, '/');
+
+ iter = refs_ref_iterator_begin(refs, dirname.buf, 0,
+ DO_FOR_EACH_INCLUDE_BROKEN);
+ while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
+ if (skip &&
+ string_list_has_string(skip, iter->refname))
+ continue;
+
+ strbuf_addf(err, "'%s' exists; cannot create '%s'",
+ iter->refname, refname);
+ ref_iterator_abort(iter);
+ goto cleanup;
+ }
+
+ if (ok != ITER_DONE)
+ die("BUG: error while iterating over references");
+
+ extra_refname = find_descendant_ref(dirname.buf, extras, skip);
+ if (extra_refname)
+ strbuf_addf(err, "cannot process '%s' and '%s' at the same
time",
+ refname, extra_refname);
+ else
+ ret = 0;
+
+cleanup:
+ strbuf_release(&referent);
+ strbuf_release(&dirname);
+ return ret;
}
int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 49e97d7d5f..07cf4cd41b 100644
--- a/refs.h
+++ b/refs.h
@@ -97,7 +97,7 @@ int read_ref(const char *refname, unsigned char *sha1);
int refs_verify_refname_available(struct ref_store *refs,
const char *refname,
- const struct string_list *extra,
+ const
[PATCH v3 03/20] refs_ref_iterator_begin(): new function
Extract a new function from `do_for_each_ref()`. It will be useful
elsewhere.
Signed-off-by: Michael Haggerty
---
refs.c | 15 +--
refs/refs-internal.h | 11 +++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index aa461156c4..2ae18097d4 100644
--- a/refs.c
+++ b/refs.c
@@ -1230,6 +1230,18 @@ int head_ref(each_ref_fn fn, void *cb_data)
return head_ref_submodule(NULL, fn, cb_data);
}
+struct ref_iterator *refs_ref_iterator_begin(
+ struct ref_store *refs,
+ const char *prefix, int trim, int flags)
+{
+ struct ref_iterator *iter;
+
+ iter = refs->be->iterator_begin(refs, prefix, flags);
+ iter = prefix_ref_iterator_begin(iter, prefix, trim);
+
+ return iter;
+}
+
/*
* Call fn for each reference in the specified submodule for which the
* refname begins with prefix. If trim is non-zero, then trim that
@@ -1247,8 +1259,7 @@ static int do_for_each_ref(struct ref_store *refs, const
char *prefix,
if (!refs)
return 0;
- iter = refs->be->iterator_begin(refs, prefix, flags);
- iter = prefix_ref_iterator_begin(iter, prefix, trim);
+ iter = refs_ref_iterator_begin(refs, prefix, trim, flags);
return do_for_each_ref_iterator(iter, fn, cb_data);
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 6ee9f20dbc..545989ae7f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -335,6 +335,17 @@ struct ref_iterator *empty_ref_iterator_begin(void);
*/
int is_empty_ref_iterator(struct ref_iterator *ref_iterator);
+/*
+ * Return an iterator that goes over each reference in `refs` for
+ * which the refname begins with prefix. If trim is non-zero, then
+ * trim that many characters off the beginning of each refname. flags
+ * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in
+ * the iteration.
+ */
+struct ref_iterator *refs_ref_iterator_begin(
+ struct ref_store *refs,
+ const char *prefix, int trim, int flags);
+
/*
* A callback function used to instruct merge_ref_iterator how to
* interleave the entries from iter0 and iter1. The function should
--
2.11.0
Re: Feature request: Configurable branch colors in git status --short --branch
On Sat, Apr 15, 2017 at 01:00:51PM -0700, Stephen Kent wrote:
> It would be nice if the branch, remote tracking branch, and branch commit
> comparison count colors in git status --short --branch were configurable
> like the other git status colors.
That seems like a reasonable thing to want.
I think the infrastructure is all there, and it would just need an
update to builtin/commit.c:parse_status_slot() to map the color.status.*
names into the {LOCAL,REMOTE}_BRANCH enum values.
Want to try your hand at a patch?
-Peff
