Commit Graph

70416 Commits

Author SHA1 Message Date
Junio C Hamano
a9ea4c23dc Merge branch 'ps/cat-file-null-output'
"git cat-file --batch" and friends learned "-Z" that uses NUL
delimiter for both input and output.

* ps/cat-file-null-output:
  cat-file: add option '-Z' that delimits input and output with NUL
  cat-file: simplify reading from standard input
  strbuf: provide CRLF-aware helper to read until a specified delimiter
  t1006: modernize test style to use `test_cmp`
  t1006: don't strip timestamps from expected results
2023-06-22 16:29:06 -07:00
Junio C Hamano
d9f9f6b358 Merge branch 'ds/disable-replace-refs'
Introduce a mechanism to disable replace refs globally and per
repository.

* ds/disable-replace-refs:
  repository: create read_replace_refs setting
  replace-objects: create wrapper around setting
  repository: create disable_replace_refs()
2023-06-22 16:29:06 -07:00
Junio C Hamano
f2ffc74186 Merge branch 'tb/pack-bitmap-traversal-with-boundary'
The object traversal using reachability bitmap done by
"pack-object" has been tweaked to take advantage of the fact that
using "boundary" commits as representative of all the uninteresting
ones can save quite a lot of object enumeration.

* tb/pack-bitmap-traversal-with-boundary:
  pack-bitmap.c: use commit boundary during bitmap traversal
  pack-bitmap.c: extract `fill_in_bitmap()`
  object: add object_array initializer helper function
2023-06-22 16:29:05 -07:00
Junio C Hamano
4dd0469328 Merge branch 'ja/worktree-orphan'
'git worktree add' learned how to create a worktree based on an
orphaned branch with `--orphan`.

* ja/worktree-orphan:
  worktree add: emit warn when there is a bad HEAD
  worktree add: extend DWIM to infer --orphan
  worktree add: introduce "try --orphan" hint
  worktree add: add --orphan flag
  t2400: add tests to verify --quiet
  t2400: refactor "worktree add" opt exclusion tests
  t2400: cleanup created worktree in test
  worktree add: include -B in usage docs
2023-06-22 16:29:05 -07:00
Junio C Hamano
6640c2d06d The second batch for 2.42
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-20 15:53:13 -07:00
Junio C Hamano
917d4c2569 Merge branch 'la/doc-interpret-trailers'
Doc update.

* la/doc-interpret-trailers:
  doc: trailer: add more examples in DESCRIPTION
  doc: trailer: mention 'key' in DESCRIPTION
  doc: trailer.<token>.command: emphasize deprecation
  doc: trailer: use angle brackets for <token> and <value>
  doc: trailer: remove redundant phrasing
  doc: trailer: examples: avoid the word "message" by itself
  doc: trailer: drop "commit message part" phrasing
  doc: trailer: swap verb order
  doc: trailer: fix grammar
2023-06-20 15:53:13 -07:00
Junio C Hamano
de00f4b7f3 Merge branch 'jk/log-follow-with-non-literal-pathspec'
"git [-c log.follow=true] log [--follow] ':(glob)f**'" used to barf.

* jk/log-follow-with-non-literal-pathspec:
  diff: detect pathspec magic not supported by --follow
  diff: factor out --follow pathspec check
  pathspec: factor out magic-to-name function
2023-06-20 15:53:13 -07:00
Junio C Hamano
7cb4274d26 Merge branch 'vd/worktree-config-is-per-repository'
The value of config.worktree is per-repository, but has been kept
in a singleton global variable per process. This has been OK as
most Git operations interacted with a single repository at a time,
but not right for operations like recursive "grep" that want to
access multiple repositories from a single process without forking.

The global variable has been eliminated and made into a member in
the per-repository data structure.

* vd/worktree-config-is-per-repository:
  repository: move 'repository_format_worktree_config' to repo scope
  config: pass 'repo' directly to 'config_with_options()'
  config: use gitdir to get worktree config
2023-06-20 15:53:13 -07:00
Junio C Hamano
9cd234e646 Merge branch 'tb/submodule-null-deref-fix'
"git submodule" code trusted the data coming from the config (and
the in-tree .gitmodules file) too much without validating, leading
to NULL dereference if the user mucks with a repository (e.g.
submodule.<name>.url is removed).  This has been corrected.

* tb/submodule-null-deref-fix:
  builtin/submodule--helper.c: handle missing submodule URLs
2023-06-20 15:53:13 -07:00
Junio C Hamano
098a191a97 Merge branch 'jc/test-modernization-2'
Test style updates.

* jc/test-modernization-2:
  t9400-git-cvsserver-server: modernize test format
  t9200-git-cvsexportcommit: modernize test format
  t9104-git-svn-follow-parent: modernize test format
  t9100-git-svn-basic: modernize test format
  t7700-repack: modernize test format
  t7600-merge: modernize test format
  t7508-status: modernize test format
  t7201-co: modernize test format
  t7111-reset-table: modernize test format
  t7110-reset-merge: modernize test format
2023-06-20 15:53:12 -07:00
Junio C Hamano
208a28ec08 Merge branch 'jc/test-modernization'
* jc/test-modernization:
  t7101-reset-empty-subdirs: modernize test format
  t6050-replace: modernize test format
  t5306-pack-nobase: modernize test format
  t5303-pack-corruption-resilience: modernize test format
  t5301-sliding-window: modernize test format
  t5300-pack-object: modernize test format
  t4206-log-follow-harder-copies: modernize test format
  t4202-log: modernize test format
  t4004-diff-rename-symlink: modernize test format
  t4003-diff-rename-1: modernize test format
  t4002-diff-basic: modernize test format
  t3903-stash: modernize test format
  t3700-add: modernize test format
  t3500-cherry: modernize test format
  t1006-cat-file: modernize test format
  t1002-read-tree-m-u-2way: modernize test format
  t1001-read-tree-m-2way: modernize test format
  t3210-pack-refs: modernize test format
  t0030-stripspace: modernize test format
  t0000-basic: modernize test format
2023-06-20 15:53:12 -07:00
Junio C Hamano
6069c1a5a7 Merge branch 'kh/use-default-notes-doc'
Doc update.

* kh/use-default-notes-doc:
  notes: move the documentation to the struct
  notes: update documentation for `use_default_notes`
2023-06-20 15:53:12 -07:00
Junio C Hamano
0899beb63c Merge branch 'pb/complete-and-document-auto-merge-and-friends'
Document more pseudo-refs and teach the command line completion
machinery to complete AUTO_MERGE.

* pb/complete-and-document-auto-merge-and-friends:
  completion: complete AUTO_MERGE
  Documentation: document AUTO_MERGE
  git-merge.txt: modernize word choice in "True merge" section
  completion: complete REVERT_HEAD and BISECT_HEAD
  revisions.txt: document more special refs
  revisions.txt: use description list for special refs
2023-06-20 15:53:12 -07:00
Junio C Hamano
693bde461c Merge branch 'mh/commit-reach-get-reachable-plug-leak'
Plug memory leak.

* mh/commit-reach-get-reachable-plug-leak:
  commit-reach: fix memory leak in get_reachable_subset()
2023-06-20 15:53:11 -07:00
Junio C Hamano
7f9b5ff41e Merge branch 'tz/test-fix-pthreads-prereq'
Test fix.

* tz/test-fix-pthreads-prereq:
  trace2 tests: fix PTHREADS prereq
2023-06-20 15:53:11 -07:00
Junio C Hamano
40693ae926 Merge branch 'tz/test-ssh-verifytime-fix'
Test fix.

* tz/test-ssh-verifytime-fix:
  t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0
2023-06-20 15:53:11 -07:00
Junio C Hamano
056d16406d Merge branch 'jk/ci-use-clang-for-sanitizer-jobs'
Clang's sanitizer implementation seems to work better than GCC's.

* jk/ci-use-clang-for-sanitizer-jobs:
  ci: drop linux-clang job
  ci: run ASan/UBSan in a single job
  ci: use clang for ASan/UBSan checks
2023-06-20 15:53:11 -07:00
Junio C Hamano
ae19633021 Merge branch 'tl/quote-problematic-arg-for-clarity'
Error message fix.

* tl/quote-problematic-arg-for-clarity:
  surround %s with quotes when failed to lookup commit
2023-06-20 15:53:10 -07:00
Junio C Hamano
06cff0c8d4 Merge branch 'ps/fetch-cleanups'
Code clean-up.

* ps/fetch-cleanups:
  fetch: use `fetch_config` to store "submodule.fetchJobs" value
  fetch: use `fetch_config` to store "fetch.parallel" value
  fetch: use `fetch_config` to store "fetch.recurseSubmodules" value
  fetch: use `fetch_config` to store "fetch.showForcedUpdates" value
  fetch: use `fetch_config` to store "fetch.pruneTags" value
  fetch: use `fetch_config` to store "fetch.prune" value
  fetch: pass through `fetch_config` directly
  fetch: drop unneeded NULL-check for `remote_ref`
  fetch: drop unused DISPLAY_FORMAT_UNKNOWN enum value
2023-06-20 15:53:10 -07:00
Linus Arver
d57fa7fc73 doc: trailer: add more examples in DESCRIPTION
Be more up-front about what trailers are in practice with examples, to
give the reader a visual cue while they go on to read the rest of the
description.

Also add an example for multiline values.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:20 -07:00
Linus Arver
eda2c44c8b doc: trailer: mention 'key' in DESCRIPTION
The 'key' option is used frequently in the examples at the bottom but
there is no mention of it in the description.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:20 -07:00
Linus Arver
dc8937fbb9 doc: trailer.<token>.command: emphasize deprecation
This puts the deprecation notice up front, instead of leaving it to the
next paragraph.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:20 -07:00
Linus Arver
8e80f2916b doc: trailer: use angle brackets for <token> and <value>
We already use angle brackets elsewhere, so this makes things more
consistent.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:20 -07:00
Linus Arver
74a50fbd7f doc: trailer: remove redundant phrasing
The phrase "many rules" gets essentially repeated again with "many other
rules", so remove this repetition.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:19 -07:00
Linus Arver
229d6ab6bf doc: trailer: examples: avoid the word "message" by itself
Previously, "message" could mean the input, output, commit message, or
"internal body text inside the commit message" (in the EXAMPLES
section). Avoid overloading this term by using the appropriate meanings
explicitly.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:19 -07:00
Linus Arver
94f15fe5d5 doc: trailer: drop "commit message part" phrasing
The command can take inputs that are either just a commit message, or
an email-like output such as git-format-patch which includes a commit
message, "---" divider, and patch part. The existing explanation blends
these two inputs together in the first sentence

    This command reads some patches or commit messages

which then necessitates using the "commit message part" phrasing (as
opposed to just "commit message") because the input is ambiguous per the
above definition.

This change separates the two input types and explains them separately,
and so there is no longer a need to use the "commit message part"
phrase.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:19 -07:00
Linus Arver
00432a36e2 doc: trailer: swap verb order
This matches the order already used in the NAME section.

Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:19 -07:00
Linus Arver
bfb5f57bb3 doc: trailer: fix grammar
Signed-off-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-14 21:42:19 -07:00
Junio C Hamano
d7d8841f67 Start the 2.42 cycle
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-13 12:29:46 -07:00
Junio C Hamano
32fe7fff0c Merge branch 'zh/ls-files-format-atoms'
Some atoms that can be used in "--format=<format>" for "git ls-tree"
were not supported by "git ls-files", even though they were relevant
in the context of the latter.

* zh/ls-files-format-atoms:
  ls-files: align format atoms with ls-tree
2023-06-13 12:29:46 -07:00
Junio C Hamano
ca9c063c18 Merge branch 'sl/diff-tree-sparse'
"git diff-tree" has been taught to take advantage of the
sparse-index feature.

* sl/diff-tree-sparse:
  diff-tree: integrate with sparse index
2023-06-13 12:29:46 -07:00
Junio C Hamano
e490bea8a6 Merge branch 'jk/format-patch-message-id-unleak'
Leakfix.

* jk/format-patch-message-id-unleak:
  format-patch: free elements of rev.ref_message_ids list
  format-patch: free rev.message_id when exiting
2023-06-13 12:29:46 -07:00
Junio C Hamano
cbc882ea38 Merge branch 'jc/pack-ref-exclude-include'
"git pack-refs" learns "--include" and "--exclude" to tweak the ref
hierarchy to be packed using pattern matching.

* jc/pack-ref-exclude-include:
  pack-refs: teach pack-refs --include option
  pack-refs: teach --exclude option to exclude refs from being packed
  docs: clarify git-pack-refs --all will pack all refs
2023-06-13 12:29:45 -07:00
Junio C Hamano
ebd07c9f7e Merge branch 'sa/doc-ls-remote'
Doc update.

* sa/doc-ls-remote:
  ls-remote doc: document the output format
  ls-remote doc: explain what each example does
  ls-remote doc: show peeled tags in examples
  ls-remote doc: remove redundant --tags example
  show-branch doc: say <ref>, not <reference>
  show-ref doc: update for internal consistency
2023-06-13 12:29:45 -07:00
Junio C Hamano
4c7d878df6 Merge branch 'gc/doc-cocci-updates'
Update documentation regarding Coccinelle patches.

* gc/doc-cocci-updates:
  cocci: codify authoring and reviewing practices
  cocci: add headings to and reword README
2023-06-13 12:29:45 -07:00
Junio C Hamano
6901ffe80c Merge branch 'jc/diff-s-with-other-options'
The "-s" (silent, squelch) option of the "diff" family of commands
did not interact with other options that specify the output format
well.  This has been cleaned up so that it will clear all the
formatting options given before.

* jc/diff-s-with-other-options:
  diff: fix interaction between the "-s" option and other options
2023-06-13 12:29:45 -07:00
Junio C Hamano
6d2a88c728 Merge branch 'kh/keep-tag-editmsg-upon-failure'
"git tag" learned to leave the "$GIT_DIR/TAG_EDITMSG" file when the
command failed, so that the user can salvage what they typed.

* kh/keep-tag-editmsg-upon-failure:
  tag: keep the message file in case ref transaction fails
  t/t7004-tag: add regression test for successful tag creation
  doc: tag: document `TAG_EDITMSG`
2023-06-13 12:29:44 -07:00
Derrick Stolee
9c7d1b057f repository: create read_replace_refs setting
The 'read_replace_refs' global specifies whether or not we should
respect the references of the form 'refs/replace/<oid>' to replace which
object we look up when asking for '<oid>'. This global has caused issues
when it is not initialized properly, such as in b6551feadf (merge-tree:
load default git config, 2023-05-10).

To make this more robust, move its config-based initialization out of
git_default_config and into prepare_repo_settings(). This provides a
repository-scoped version of the 'read_replace_refs' global.

The global still has its purpose: it is disabled process-wide by the
GIT_NO_REPLACE_OBJECTS environment variable or by a call to
disable_replace_refs() in some specific Git commands.

Since we already encapsulated the use of the constant inside
replace_refs_enabled(), we can perform the initialization inside that
method, if necessary. This solves the problem of forgetting to check the
config, as we will check it before returning this value.

Due to this encapsulation, the global can move to be static within
replace-object.c.

There is an interesting behavior change possible here: we now have a
repository-scoped understanding of this config value. Thus, if there was
a command that recurses into submodules and might follow replace refs,
then it would now respect the core.useReplaceRefs config value in each
repository.

'git grep --recurse-submodules' is such a command that recurses into
submodules in-process. We can demonstrate the granularity of this config
value via a test in t7814.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:34:55 -07:00
Derrick Stolee
f1178380ac replace-objects: create wrapper around setting
The 'read_replace_objects' constant is initialized by git_default_config
(if core.useReplaceRefs is disabled) and within setup_git_env (if
GIT_NO_REPLACE_OBJECTS) is set. To ensure that this variable cannot be
set accidentally in other places, wrap it in a replace_refs_enabled()
method.

Since we still assign this global in config.c, we are not able to remove
the global scope of this variable and make it a static within
replace-object.c. This will happen in a later change which will also
prevent the variable from being read before it is initialized.

Centralizing read access to the variable is an important first step.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:34:55 -07:00
Derrick Stolee
d24eda4e03 repository: create disable_replace_refs()
Several builtins depend on being able to disable the replace references
so we actually operate on each object individually. These currently do
so by directly mutating the 'read_replace_refs' global.

A future change will move this global into a different place, so it will
be necessary to change all of these lines. However, we can simplify that
transition by abstracting the purpose of these global assignments with a
method call.

We will need to keep this read_replace_refs global forever, as we want
to make sure that we never use replace refs throughout the life of the
process if this method is called. Future changes may present a
repository-scoped version of the variable to represent that repository's
core.useReplaceRefs config value, but a zero-valued read_replace_refs
will always override such a setting.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:34:55 -07:00
Patrick Steinhardt
f79e18849b cat-file: add option '-Z' that delimits input and output with NUL
In db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with
`-z`, 2022-07-22), we have introduced a new mode to read the input via
NUL-delimited records instead of newline-delimited records. This allows
the user to query for revisions that have newlines in their path
component. While unusual, such queries are perfectly valid and thus it
is clear that we should be able to support them properly.

Unfortunately, the commit only changed the input to be NUL-delimited,
but didn't change the output at the same time. While this is fine for
queries that are processed successfully, it is less so for queries that
aren't. In the case of missing commits for example the result can become
entirely unparsable:

```
$ printf "7ce4f05bae8120d9fa258e854a8669f6ea9cb7b1 blob 10\n1234567890\n\n\commit000" |
    git cat-file --batch -z
7ce4f05bae blob 10
1234567890

commit missing
```

This is of course a crafted query that is intentionally gaming the
deficiency, but more benign queries that contain newlines would have
similar problems.

Ideally, we should have also changed the output to be NUL-delimited when
`-z` is specified to avoid this problem. As the input is NUL-delimited,
it is clear that the output in this case cannot ever contain NUL
characters by itself. Furthermore, Git does not allow NUL characters in
revisions anyway, further stressing the point that using NUL-delimited
output is safe. The only exception is of course the object data itself,
but as git-cat-file(1) prints the size of the object data clients should
read until that specified size has been consumed.

But even though `-z` has only been introduced a few releases ago in Git
v2.38.0, changing the output format retroactively to also NUL-delimit
output would be a backwards incompatible change. And while one could
make the argument that the output is inherently broken already, we need
to assume that there are existing users out there that use it just fine
given that revisions containing newlines are quite exotic.

Instead, introduce a new option `-Z` that switches to NUL-delimited
input and output. While this new option could arguably only switch the
output format to be NUL-delimited, the consequence would be that users
have to always specify both `-z` and `-Z` when the input may contain
newlines. On the other hand, if the user knows that there never will be
newlines in the input, they don't have to use either of those options.
There is thus no usecase that would warrant treating input and output
format separately, which is why we instead opt to "do the right thing"
and have `-Z` mean to NUL-terminate both formats.

The old `-z` option is marked as deprecated with a hint that its output
may become unparsable. It is thus hidden both from the synopsis as well
as the command's help output.

Co-authored-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:23:46 -07:00
Patrick Steinhardt
3217f52a49 cat-file: simplify reading from standard input
The batch modes of git-cat-file(1) read queries from stantard input that
are either newline- or NUL-delimited. This code was introduced via
db9d67f2e9 (builtin/cat-file.c: support NUL-delimited input with `-z`,
2022-07-22), which notes that:

"""
The refactoring here is slightly unfortunate, since we turn loops like:

     while (strbuf_getline(&buf, stdin) != EOF)

 into:

     while (1) {
         int ret;
         if (opt->nul_terminated)
             ret = strbuf_getline_nul(&input, stdin);
         else
             ret = strbuf_getline(&input, stdin);

         if (ret == EOF)
             break;
     }
"""

The commit proposed introducing a helper function that is easier to use,
which is just what we have done in the preceding commit. Refactor the
code to use this new helper to simplify the loop.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:23:24 -07:00
Patrick Steinhardt
af35e56b0f strbuf: provide CRLF-aware helper to read until a specified delimiter
Many of our commands support reading input that is separated either via
newlines or via NUL characters. Furthermore, in order to be a better
cross platform citizen, these commands typically know to strip the CRLF
sequence so that we also support reading newline-separated inputs on
e.g. the Windows platform. This results in the following kind of awkward
pattern:

```
struct strbuf input = STRBUF_INIT;

while (1) {
	int ret;

	if (nul_terminated)
		ret = strbuf_getline_nul(&input, stdin);
	else
		ret = strbuf_getline(&input, stdin);
	if (ret)
		break;

	...
}
```

Introduce a new CRLF-aware helper function that can read up to a user
specified delimiter. If the delimiter is `\n` the function knows to also
strip CRLF, otherwise it will only strip the specified delimiter. This
results in the following, much more readable code pattern:

```
struct strbuf input = STRBUF_INIT;

while (strbuf_getdelim_strip_crlf(&input, stdin, delim) != EOF) {
	...
}
```

The new function will be used in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:23:24 -07:00
Patrick Steinhardt
b116c77307 t1006: modernize test style to use test_cmp
The tests for git-cat-file(1) are quite old and haven't ever been
updated since they were introduced. They thus tend to use old idioms
that have since grown outdated. Most importantly, many of the tests use
`test $A = $B` to compare expected and actual output. This has the
downside that it is impossible to tell what exactly is different between
both versions in case the test fails.

Refactor the tests to instead use `test_cmp`. While more verbose, it
both tends to be more readable and will result in a nice diff in case
states don't match.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:23:24 -07:00
Patrick Steinhardt
c7309f63c6 t1006: don't strip timestamps from expected results
In t1006 we have a bunch of tests that verify the output format of the
git-cat-file(1) command. But while part of the output for some tests
would include commit timestamps, we don't verify those but instead strip
them before comparing expected with actual results. This is done by the
function `maybe_remove_timestamp`, which goes all the way back to the
ancient commit b335d3f121 (Add tests for git cat-file, 2008-04-23).

Our tests had been in a different shape back then. Most importantly we
didn't yet have the infrastructure to create objects with deterministic
timestamps. Nowadays we do though, and thus there is no reason anymore
to strip the timestamps.

Refactor the tests to not strip the timestamp anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-12 13:23:24 -07:00
Kristoffer Haugsbakk
aeee1408ce notes: move the documentation to the struct
Its better to document the struct members directly instead of on a
function that takes a pointer to the struct. This will also make it
easier to update the documentation in the future.

Make adjustments for this new context. Also drop “may contain” since we
don’t need to emphasize that a list could be empty.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-06 09:35:05 +09:00
Kristoffer Haugsbakk
a2e9dbb884 notes: update documentation for use_default_notes
`suppress_default_notes` was renamed to `use_default_notes` in
3a03cf6b1d (notes: refactor display notes default handling,
2011-03-29).

The commit message says that “values less than one [indicates] “not
set” ”, but what was meant was probably “less than zero” (the author of
3a03cf6b1d agrees on this point).

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-06 09:35:03 +09:00
Mike Hommey
68b51172e3 commit-reach: fix memory leak in get_reachable_subset()
This is a leak that has existed since the method was first created
in fcb2c0769d (commit-reach: implement get_reachable_subset,
2018-11-02).

Signed-off-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-04 13:43:48 +09:00
Jeff King
d88d727143 ci: drop linux-clang job
Since the linux-asan-ubsan job runs using clang under Linux, there is
not much point in running a separate clang job. Any errors that a normal
clang compile-and-test cycle would find are likely to be a subset of
what the sanitizer job will find. Since this job takes ~14 minutes to
run in CI, this shaves off some of our CPU load (though it does not
affect end-to-end runtime, since it's typically run in parallel and is
not the longest job).

Technically this provides us with slightly less signal for a given run,
since you won't immediately know if a failure in the sanitizer job is
from using clang or from the sanitizers themselves. But it's generally
obvious from the logs, and anyway your next step would be to fix the
probvlem and re-run CI, since we expect all of these jobs to pass
normally.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-03 10:35:13 +09:00
Jeff King
ec6915265a ci: run ASan/UBSan in a single job
When we started running sanitizers in CI via 1c0962c0c4 (ci: add address
and undefined sanitizer tasks, 2022-10-20), we ran them as two separate
CI jobs, since as that commit notes, the combination "seems to take
forever".

And indeed, it does with gcc. However, since the previous commit
switched to using clang, the situation is different, and we can save
some CPU by using a single job for both. Comparing before/after CI runs,
this saved about 14 minutes (the single combined job took 54m, versus
44m plus 24m for ASan and UBSan jobs, respectively). That's wall-clock
and not CPU, but since our jobs are mostly CPU-bound, the two should be
closely proportional.

This does increase the end-to-end time of a CI run, though, since before
this patch the two jobs could run in parallel, and the sanitizer job is
our longest single job. It also means that we won't get a separate
result for "this passed with UBSan but not with ASan" or vice versa).
But as 1c0962c0c4 noted, that is not a very useful signal in practice.

Below are some more detailed timings of gcc vs clang that I measured by
running the test suite on my local workstation. Each measurement counts
only the time to run the test suite with each compiler (not the compile
time itself). We'll focus on the wall-clock times for simplicity, though
the CPU times follow roughly similar trends.

Here's a run with CC=gcc as a baseline:

  real	1m12.931s
  user	9m30.566s
  sys	8m9.538s

Running with SANITIZE=address increases the time by a factor of ~4.7x:

  real	5m40.352s
  user	49m37.044s
  sys	36m42.950s

Running with SANITIZE=undefined increases the time by a factor of ~1.7x:

  real	2m5.956s
  user	12m42.847s
  sys	19m27.067s

So let's call that 6.4 time units to run them separately (where a unit
is the time it takes to run the test suite with no sanitizers). As a
simplistic model, we might imagine that running them together would take
5.4 units (we save 1 unit because we are no longer running the test
suite twice, but just paying the sanitizer overhead on top of a single
run).

But that's not what happens. Running with SANITIZE=address,undefined
results in a factor of 9.3x:

  real	11m9.817s
  user	77m31.284s
  sys	96m40.454s

So not only did we not get faster when doing them together, we actually
spent 1.5x as much CPU as doing them separately! And while those
wall-clock numbers might not look too terrible, keep in mind that this
is on an unloaded 8-core machine. In the CI environment, wall-clock
times will be much closer to CPU times. So not only are we wasting CPU,
but we risk hitting timeouts.

Now let's try the same thing with clang. Here's our no-sanitizer
baseline run, which is almost identical to the gcc one (which is quite
convenient, because we can keep using the same "time units" to get an
apples-to-apples comparison):

  real	1m11.844s
  user	9m28.313s
  sys	8m8.240s

And now again with SANITIZE=address, we get a 5x factor (so slightly
worse than gcc's 4.7x, though I wouldn't read too much into it; there is
a fair bit of run-to-run noise):

  real	6m7.662s
  user	49m24.330s
  sys	44m13.846s

And with SANITIZE=undefined, we are at 1.5x, slightly outperforming gcc
(though again, that's probably mostly noise):

  real	1m50.028s
  user	11m0.973s
  sys	16m42.731s

So running them separately, our total cost is 6.5x. But if we combine
them in a single run (SANITIZE=address,undefined), we get:

  real	6m51.804s
  user	52m32.049s
  sys	51m46.711s

which is a factor of 5.7x. That's along the lines we'd hoped for!
Running them together saves us almost a whole time unit. And that's not
counting any time spent outside the test suite itself (starting the job,
setting up the environment, compiling) that we're no longer duplicating
by having two jobs.

So clang behaves like we'd hope: the overhead to run the sanitizers is
additive as you add more sanitizers. Whereas gcc's numbers seem very
close to multiplicative, almost as if the sanitizers were enforcing
their overheads on each other (though that is purely a guess on what is
going on; ultimately what matters to us is the amount of time it takes).

And that roughly matches the CI improvement I saw. A "time unit" there
is more like 12 minutes, and the observed time savings was 14 minutes
(with the extra presumably coming from avoiding duplicated setup, etc).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-03 10:35:13 +09:00