Greg Hurrell [Wed, 9 Jun 2021 19:28:06 +0000 (21:28 +0200)]
gitweb: use HEAD as secondary sort key in git_get_heads_list()
The "heads" section on the gitweb summary page shows heads in
`-committerdate` order (ie. the most recently-modified ones at the
top), tie-breaking equal-dated refs using the implicit `refname` sort
fallback. This recency-based ordering appears in multiple places in the
UI, such as the project listing, the tags list, and even the
shortlog and log views.
Given two equal-dated refs, however, sorting the `HEAD` ref before
the non-`HEAD` ref provides more useful signal than merely sorting by
refname. For example, say we had "master" and "trunk" both pointing at
the same commit but "trunk" was `HEAD`, sorting "trunk" first helps
communicate its special status as the default branch that you'll check
out if you clone the repo.
Add `-HEAD` as a secondary sort key to the `git for-each-ref` call
in `git_get_heads_list()` to provide the desired behavior. The most
recently committed refs will appear first, but `HEAD`-ness will be used
as a tie-breaker. Note that `refname` is the implicit fallback sort key,
which means that two same-dated non-`HEAD` refs will continue to be
sorted in lexicographical order, as they are today.
Signed-off-by: Greg Hurrell <greg@hurrell.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 8 Jun 2021 16:11:42 +0000 (16:11 +0000)]
merge-ort: miscellaneous touch-ups
Add some notes in the code about invariants with match_mask when adding
pairs. Also add a comment that seems to have been left out in my work
of pushing these changes upstream.
Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 8 Jun 2021 16:11:41 +0000 (16:11 +0000)]
Fix various issues found in comments
A random hodge-podge of incorrect or out-of-date comments that I found:
* t6423 had a comment that has referred to the wrong test for years;
fix it to refer to the right one.
* diffcore-rename had a FIXME comment meant to remind myself to
investigate if I could make another code change. I later
investigated and removed the FIXME, but while cherry-picking the
patch to submit upstream I missed the later update. Remove the
comment now.
* merge-ort had the early part of a comment for a function; I had
meant to include the more involved description when I updated the
function. Update the comment now.
Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 8 Jun 2021 16:11:40 +0000 (16:11 +0000)]
diffcore-rename: avoid unnecessary strdup'ing in break_idx
The keys of break_idx are strings from the diff_filepairs of
diff_queued_diff. break_idx is only used in location_rename_dst(), and
that usage is always before any free'ing of the pairs (and thus the
strings in the pairs). As such, there is no need to strdup these keys;
we can just reuse the existing strings as-is.
The merge logic doesn't make use of break detection, so this does not
affect the performance of any of my testcases. It was just a minor
unrelated optimization noted in passing while looking at the code.
Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Tue, 8 Jun 2021 16:11:39 +0000 (16:11 +0000)]
merge-ort: replace string_list_df_name_compare with faster alternative
Gathering accumulated times from trace2 output on the mega-renames
testcase, I saw the following timings (where I'm only showing a few
lines to highlight the portions of interest):
In the above output, note that the 4.462 seconds for process_entries was
split as 3.143 seconds for "process_entries setup" and 1.305 seconds for
"processing" (and a little time for other stuff removed from the
highlight). Most of the "process_entries setup" time was spent on
"plist special sort" which corresponds to the following code:
trace2_region_enter("merge", "plist special sort", opt->repo);
plist.cmp = string_list_df_name_compare;
string_list_sort(&plist);
trace2_region_leave("merge", "plist special sort", opt->repo);
In other words, in a merge strategy that would be invoked by passing
"-sort" to either rebase or merge, sorting an array takes more time than
anything else. Serves me right for naming my merge strategy this way.
Rewrite the comparison function in a way that does not require finding
out the lengths of the strings when comparing them. While at it, tweak
the code for our specific case -- no need to handle a variety of modes,
for example. The combination of these changes reduced the time spent in
"plist special sort" by ~25% in the mega-renames case.
For the testcases mentioned in commit 557ac0350d ("merge-ort: begin
performance work; instrument with trace2_region_* calls", 2020-10-28),
this change improves the performance as follows:
Before After
no-renames: 5.622 s ± 0.059 s 5.235 s ± 0.042 s
mega-renames: 10.127 s ± 0.073 s 9.419 s ± 0.107 s
just-one-mega: 500.3 ms ± 3.8 ms 480.1 ms ± 3.9 ms
Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Andrei Rybak [Tue, 8 Jun 2021 20:48:41 +0000 (22:48 +0200)]
mailinfo: use starts_with() when checking scissors
Existing checks for scissors characters using memcmp(3) never read past
the end of the line, because all substrings we are interested in are two
characters long, and the outer loop guarantees we have at least one
character. So at most we will look at the NUL.
However, this is too subtle and may lead to bugs in code which copies
this behavior without realizing substring length requirement. So use
starts_with() instead, which will stop at NUL regardless of the length
of the prefix. Remove extra pair of parentheses while we are here.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Atharva Raykar [Tue, 8 Jun 2021 19:06:12 +0000 (00:36 +0530)]
MyFirstContribution: link #git-devel to Libera Chat
Many of the regulars on #git-devel are now on Libera Chat, to the extent
that the community page now lists it as the IRC Channel[1]. This will
help new contributors find the right place, if they choose to ask
questions on `#git-devel`.
Relevant discussion on the IRC transition:
https://lore.kernel.org/git/CAJoAoZ=e62sceNpcR5L5zjsj177uczTnXjcAg+BbOoOkeH8vXQ@mail.gmail.com/
[1] https://git-scm.com/community
Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtins + test helpers: use return instead of exit() in cmd_*
Change various cmd_* functions that claim to return an "int" to use
"return" instead of exit() to indicate an exit code. These were not
marked with NORETURN, and by directly exit()-ing we'll skip the
cleanup git.c would otherwise do (e.g. closing fd's, erroring if we
can't). See run_builtin() in git.c.
In the case of shell.c and sh-i18n--envsubst.c this was the result of
an incomplete migration to using a cmd_main() in 3f2e2297b9 (add an
extra level of indirection to main(), 2016-07-01).
This was spotted by SunCC 12.5 on Solaris 10 (gcc210 on the gccfarm).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 8 Jun 2021 07:24:48 +0000 (03:24 -0400)]
doc: warn people against --max-pack-size
This option is almost never a good idea, as the resulting repository is
larger and slower (see the new explanations in the docs).
I outlined the potential problems. We could go further and make the
option harder to find (or at least, make the command-line option
descriptions a much more terse "you probably don't want this; see
pack.packsizeLimit for details"). But this seems like a minimal change
that may prevent people from thinking it's more useful than it is.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t: use user-specified utf-8 locale for testing svn
In some test-cases, UTF-8 locale is required. To find such locale,
we're using the first available UTF-8 locale that returned by
"locale -a".
However, the locale(1) utility is unavailable on some systems,
e.g. Linux with musl libc.
However, without "locale -a", we can't guess provided UTF-8 locale.
Add a Makefile knob GIT_TEST_UTF8_LOCALE and activate it for
linux-musl in our CI system.
Rename t/lib-git-svn.sh:prepare_a_utf8_locale to prepare_utf8_locale,
since we no longer prepare the variable named "a_utf8_locale",
but set up a fallback value for GIT_TEST_UTF8_LOCALE instead.
The fallback will be LC_ALL, LANG environment variable,
or the first UTF-8 locale from output of "locale -a", in that order.
Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There're two different default options for log --decorate:
* Should `--decorate` be given without any arguments, it's default to
`short`
* Should neither `--decorate` nor `--no-decorate` be given, it's default
to the `log.decorate` or `auto`.
We documented the former, but not the latter.
Let's document them, too.
Reported-by: Andy AO <zen96285@gmail.com> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The xdl_bug() function was introduced in e8adf23d1e (xdl_change_compact(): introduce the concept of a change
group, 2016-08-22), let's use our usual BUG() function instead.
We'll now have meaningful line numbers if we encounter bugs in xdiff,
and less code duplication.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't guard the calls to the progress.c API with "if (progress)". The
API itself will check this. This doesn't change any behavior, but
makes this code consistent with the rest of the codebase.
See ae9af12287b (status: show progress bar if refreshing the index
takes too long, 2018-09-15) for the commit that added the pattern
we're changing here.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a trailing newline to the protocol-caps.h file added in the recent a2ba162cda (object-info: support for retrieving object info,
2021-04-20). Various editors add this implicitly, and some compilers
warn about the lack of a \n here.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sun, 6 Jun 2021 01:01:57 +0000 (03:01 +0200)]
parallel-checkout: avoid dash local bug in tests
Dash bug https://bugs.launchpad.net/ubuntu/+source/dash/+bug/139097
lets the shell erroneously perform field splitting on the expansion of a
command substitution during declaration of a local variable. It causes
the parallel-checkout tests to fail e.g. when running them with
/bin/dash on MacOS 11.4, where they error out like this:
./t2080-parallel-checkout-basics.sh: 33: local: 0: bad variable name
That's because the output of wc -l contains leading spaces and the
returned number of lines is treated as another variable to declare, i.e.
as in "local workers= 0".
Work around it by enclosing the command substitution in quotes.
Helped-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 4 Jun 2021 01:36:11 +0000 (10:36 +0900)]
fsync(): be prepared to see EINTR
Some platforms, like NonStop do not automatically restart fsync()
when interrupted by a signal, even when that signal is setup with
SA_RESTART.
This can lead to test breakage, e.g., where "--progress" is used,
thus SIGALRM is sent often, and can interrupt an fsync() syscall.
Make sure we deal with such a case by retrying the syscall
ourselves. Luckily, we call fsync() fron a single wrapper,
fsync_or_die(), so the fix is fairly isolated.
Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> Helped-by: Jeff King <peff@peff.net> Helped-by: Taylor Blau <me@ttaylorr.com>
[jc: the above two did most of the work---I just tied the loose end] Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Josh Steadmon [Fri, 4 Jun 2021 02:41:30 +0000 (19:41 -0700)]
docs: fix api-trace2 doc for "too_many_files" event
In 87db61a (trace2: write discard message to sentinel files,
2019-10-04), we added a new "too_many_files" event for when trace2
logging would create too many files in an output directory.
Unfortunately, the api-trace2 doc described a "discard" event instead.
Fix the doc to use the correct event name.
Signed-off-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tao Klerks [Wed, 2 Jun 2021 11:47:26 +0000 (11:47 +0000)]
Remove warning that repack only works on non-promisor packfiles
The git-repack doc clearly states that it *does* operate on promisor
packfiles (in a separate partition), with "-a" specified. Presumably
the statements here are outdated, as they feature from the first doc
in 2017 (and the repack support was added in 2018)
Signed-off-by: Tao Klerks <tao@klerks.biz> Reviewed-by: Taylor Blau <me@ttaylorr.com> Acked-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ZheNing Hu [Thu, 3 Jun 2021 16:29:26 +0000 (16:29 +0000)]
cat-file: merge two block into one
There are two "if (opt->all_objects)" blocks next
to each other, merge them into one to provide better
readability.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: ZheNing Hu <adlternative@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ZheNing Hu [Thu, 3 Jun 2021 16:29:25 +0000 (16:29 +0000)]
cat-file: handle trivial --batch format with --batch-all-objects
The --batch code to print an object assumes we found out the type of
the object from calling oid_object_info_extended(). This is true for
the default format, but even in a custom format, we manually modify
the object_info struct to ask for the type.
This assumption was broken by 845de33a5b (cat-file: avoid noop calls
to sha1_object_info_extended, 2016-05-18). That commit skips the call
to oid_object_info_extended() entirely when --batch-all-objects is in
use, and the custom format does not include any placeholders that
require calling it.
Or when the custom format only include placeholders like %(objectname) or
%(rest), oid_object_info_extended() will not get the type of the object.
This results in an error when we try to confirm that the type didn't
change:
and also has other subtle effects (e.g., we'd fail to stream a blob,
since we don't realize it's a blob in the first place).
We can fix this by flipping the order of the setup. The check for "do
we need to get the object info" must come _after_ we've decided
whether we need to look up the type.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: ZheNing Hu <adlternative@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
David Aguilar [Tue, 1 Jun 2021 20:52:29 +0000 (13:52 -0700)]
contrib/completion: fix zsh completion regression from 59d85a2a05
A recent change to make git-completion.bash use $__git_cmd_idx
in more places broke a number of completions on zsh because it
modified __git_main but did not update __git_zsh_main.
Notably, completions for "add", "branch", "mv" and "push" were
broken as a result of this change.
In addition to the undefined variable usage, "git mv <tab>" also
prints the following error:
__git_count_arguments:7: bad math expression:
operand expected at `"1"'
_git_mv:[:7: unknown condition: -gt
Remove the quotes around $__git_cmd_idx in __git_count_arguments
and set __git_cmd_idx=1 early in __git_zsh_main to fix the
regressions from 59d85a2a05.
This was tested on zsh 5.7.1 (x86_64-apple-darwin19.0).
Suggested-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: David Aguilar <davvid@gmail.com> Acked-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:51:14 +0000 (14:51 -0500)]
push: split switch cases
We want all the cases that don't do anything with a branch first, and
then the rest. That way we will be able to get the branch and die if
there's a problem in the parent function, instead of inside the function
of each mode.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:32:35 +0000 (14:32 -0500)]
push: simplify setup_push_simple()
There's a safety check to make sure branch->refname isn't different
from branch->merge[0]->src, otherwise we die().
Therefore we always push to branch->refname.
Suggestions-by: Elijah Newren <newren@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:32:34 +0000 (14:32 -0500)]
push: reorganize setup_push_simple()
Simply move the code around and remove dead code. In particular the
'!same_remote' conditional is a no-op since that part of the code is the
same_remote leg of the conditional beforehand.
No functional changes.
Suggestions-by: Elijah Newren <newren@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:32:33 +0000 (14:32 -0500)]
push: copy code to setup_push_simple()
In order to avoid doing unnecessary things and simplify it in further
patches. In particular moving the additional name safety out of
setup_push_upstream() and into setup_push_simple() and thus making both
more straightforward.
The code is copied exactly as-is; no functional changes.
Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:32:32 +0000 (14:32 -0500)]
push: hedge code of default=simple
`simple` is the most important mode so move the relevant code to its own
function to make it easier to see what it's doing.
Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Felipe Contreras [Mon, 31 May 2021 19:32:31 +0000 (14:32 -0500)]
push: rename !triangular to same_remote
The typical case is what git was designed for: distributed remotes.
It's only the atypical case--fetching and pushing to the same
remote--that we need to keep an eye on.
No functional changes.
Liked-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:37 +0000 (16:56 +0000)]
t1415: set REFFILES for test specific to storage format
Packing refs (and therefore checking that certain refs are not packed)
is a property of the packed/loose ref storage. Add a comment to explain
what the test checks.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:34 +0000 (16:56 +0000)]
t7900: stop checking for loose refs
Given that git-maintenance simply calls out git-pack-refs, it seems superfluous
to test the functionality of pack-refs itself, as that is covered by
t3210-pack-refs.sh.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:33 +0000 (16:56 +0000)]
t1404: mark tests that muck with .git directly as REFFILES.
The packed/loose ref storage is an overlay combination of packed-refs (refs and
tags in a single file) and one-file-per-ref. This creates all kinds of edge
cases related to directory/file conflicts, (non-)empty directories, and the
locking scheme, none of which applies to reftable.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:32 +0000 (16:56 +0000)]
t2017: mark --orphan/logAllRefUpdates=false test as REFFILES
In reftable, there is no notion of a per-ref 'existence' of a reflog. Each
reflog entry has its own key, so it is not possible to distinguish between
{reflog doesn't exist,reflog exists but is empty}. This makes the logic
in log_ref_setup() (file refs/files-backend.c), which depends on the existence
of the reflog file infeasible.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:31 +0000 (16:56 +0000)]
t1414: mark corruption test with REFFILES
The test checks what happens if reflog and ref database disagree on the state of
the latest commit. This seems to require accessing reflog storage directly.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:29 +0000 (16:56 +0000)]
test-lib: provide test prereq REFFILES
REFFILES can be used to mark tests that are specific to the packed/loose ref
storage format and its limitations. Marking such tests is a preparation for
introducing the reftable storage backend.
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:28 +0000 (16:56 +0000)]
t5304: use "reflog expire --all" to clear the reflog
This test checks that unreachable objects are really removed. For the test to
work, it has to ensure that no reflog retain any reachable objects.
Previously, it did this by manipulating the file system to remove reflog in the
first test, and relying on git not updating the reflog if the relevant logfile
doesn't exist in follow-up tests.
Now, explicitly clear the reflog using 'reflog expire'. This reduces the
dependency between test functions. It also is more amenable to use with
reftable, which has no concept of (non)-existence of a reflog
Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han-Wen Nienhuys [Mon, 31 May 2021 16:56:17 +0000 (16:56 +0000)]
t/helper/ref-store: initialize oid in resolve-ref
This will print $ZERO_OID when asking for a non-existent ref from the
test-helper.
Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
resolution to refs/heads/REFNAME.
Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/fsck.c: don't conflate "int" and "enum" in callback
Fix a warning on AIX's xlc compiler that's been emitted since my a1aad71601a (fsck.h: use "enum object_type" instead of "int",
2021-03-28):
"builtin/fsck.c", line 805.32: 1506-068 (W) Operation between
types "int(*)(struct object*,enum object_type,void*,struct
fsck_options*)" and "int(*)(struct object*,int,void*,struct
fsck_options*)" is not allowed.
I.e. it complains about us assigning a function with a prototype "int"
where we're expecting "enum object_type".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Thomas Braun [Sun, 30 May 2021 11:31:35 +0000 (13:31 +0200)]
completion: add --anchored to diff's options
This flag was introduced in 2477ab2e (diff: support anchoring line(s),
2017-11-27) but back then, the bash completion script did not learn
about the new flag. Add it.
Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Optimize the startup time of git-send-email by using an amended
config_regexp() function to retrieve the list of config keys and
values we're interested in.
For boolean keys we can handle the [true|false] case ourselves, and
the "--get" case didn't need any parsing. Let's leave "--path" and
other "--bool" cases to "git config". I'm not bothering with the
"undef" or "" case (true and false, respectively), let's just punt on
those and others and have "git config --type=bool" handle it.
The "grep { defined } @values" here covers a rather subtle case. For
list values such as sendemail.to it is possible as with any other
config key to provide a plain "-c sendemail.to", i.e. to set the key
as a boolean true. In that case the Git::config() API will return an
empty string, but this new parser will correctly return "undef".
However, that means we can end up with "undef" in the middle of a
list. E.g. for sendemail.smtpserveroption in conjuction with
sendemail.smtpserver as a path this would have produce a warning. For
most of the other keys we'd behave the same despite the subtle change
in the value, e.g. sendemail.to would behave the same because
Mail::Address->parse() happens to return an empty list if fed
"undef". For the boolean values we were already prepared to handle
these variables being initialized as undef anyway.
This brings the runtime of "git send-email" from ~60-~70ms to a very
steady ~40ms on my test box. We now run just one "git config"
invocation on startup instead of 8, the exact number will differ based
on the local sendemail.* config. I happen to have 8 of those set.
This brings the runtime of t9001-send-email.sh from ~13s down to ~12s
for me. The change there is less impressive as many of those tests set
various config values, and we're also getting to the point of
diminishing returns for optimizing "git send-email" itself.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
perl: nano-optimize by replacing Cwd::cwd() with Cwd::getcwd()
It has been pointed out[1] that cwd() invokes "pwd(1)" while getcwd()
is a Perl-native XS function. For what we're using these for we can
use getcwd().
The performance difference is miniscule, we're saving on the order of
a millisecond or so, see [2] below for the benchmark. I don't think
this matters in practice for optimizing git-send-email or perl
execution (unlike the patches leading up to this one).
But let's do it regardless of that, if only so we don't have to think
about this as a low-hanging fruit anymore.
1. https://lore.kernel.org/git/20210512180517.GA11354@dcvr/
2.
$ perl -MBenchmark=:all -MCwd -wE 'cmpthese(10000, { getcwd => sub { getcwd }, cwd => sub { cwd }, pwd => sub { system "pwd >/dev/null" }})'
(warning: too few iterations for a reliable count)
Rate pwd cwd getcwd
pwd 982/s -- -48% -100%
cwd 1890/s 92% -- -100%
getcwd 10000000000000000000/s 1018000000000000000% 529000000000000064% -
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of unconditionally requiring modules such as File::Spec, let's
only load them when needed. This speeds up code that only needs a
subset of the features Git.pm provides.
This brings a plain invocation of "git send-email" down from 52/37
loaded modules under NO_GETTEXT=[|Y] to 39/18, and it now takes
~60-~70ms instead of ~80-~90ms. The runtime of t9001-send-email.sh
test is down to ~13s from ~15s.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Optimize the time git-send-email takes to do even the simplest of
things (such as serving up "-h") from around ~150ms to ~80ms-~90ms by
lazily loading the modules it requires.
Before this change Devel::TraceUse would report 99/97 used modules
under NO_GETTEXT=[|Y], respectively. Now it's 52/37. It now takes ~15s
to run t9001-send-email.sh, down from ~20s.
Changing File::Spec::Functions::{catdir,catfile} to invoking class
methods on File::Spec itself is idiomatic. See [1] for a more
elaborate explanation, the resulting code behaves the same way, just
without the now-pointless function wrapper.
Change indirect object syntax such as "new X ARGS" to
"X->new(ARGS)". This allows perl to see what "new" is at compile-time
without having loaded Term::ReadLine. This doesn't matter now, but
will in a subsequent commit when we start lazily loading it.
Let's do the same for the adjacent "FakeTerm" package for consistency,
even though we're not going to conditionally load it.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Optimize git-send-email by only shelling out to "git var" if we need
to. This is easily done by re-inventing our own small version of
perl's Memoize module.
I suppose I could just use Memoize itself, but in a subsequent patch
I'll be micro-optimizing send-email's use of dependencies. Using
Memoize is a measly extra 5-10 milliseconds, but as we'll see that'll
end up mattering for us in the end.
This brings the runtime of a plain "send-email" from around ~160-170ms
to ~140m-150ms. The runtime of the tests is around the same, or around
~20s.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reduce the time it takes git-send-email to get to even the most
trivial of tasks (such as serving up its "-h" output) by first listing
config keys that exist, and only then only call e.g. "git config
--bool" on them if they do.
Over a lot of runs this speeds the time to "-h" up for me from ~250ms
to ~150ms, and the runtime of t9001-send-email.sh goes from ~25s to
~20s.
This introduces a race condition where we'll do the "wrong" thing if a
config key were to be inserted between us discovering the list and
calling read_config(), i.e. we won't know about the racily added
key. In theory this is a change in behavior, in practice it doesn't
matter.
The config_regexp() function being changed here was added in dd84e528a34 (git-send-email: die if sendmail.* config is set,
2020-07-23) for use by git-send-email. So we can change its odd return
value in the case where no values are found by "git config". The
difference in the *.pm code would matter if it was invoked in scalar
context, but now it no longer is.
Arguably this caching belongs in Git.pm itself, but in lieu of
modifying it for all its callers let's only do this for "git
send-email". The other big potential win would be "git svn", but
unlike "git send-email" it doesn't check tens of config variables one
at a time at startup (in my brief testing it doesn't check any).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
send-email: copy "config_regxp" into git-send-email.perl
The config_regexp() function was added in dd84e528a3 (git-send-email:
die if sendmail.* config is set, 2020-07-23) for use in
git-send-email, and it's the only in-tree user of it.
However, the consensus is that Git.pm is a public interface, so even
though it's a recently added function we can't change it. So let's
copy over a minimal version of it to git-send-email.perl itself. In a
subsequent commit it'll be changed further for our own use.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
With the removal of the support for sendemail.smtpssl in the preceding
commit the parsing of sendemail.smtpencryption is no longer special,
and can by moved to %config_settings.
This gets us rid of an unconditional call to Git::config(), which as
we'll see in subsequent commits matters for startup performance.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / rit / log