Taylor Blau [Tue, 16 May 2023 21:33:43 +0000 (17:33 -0400)]
run-command.c: fix missing include under `NO_PTHREADS`
Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined,
i.e.
$ make NO_PTHREADS=1 run-command.o
as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the
macro definition is not available.
This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from
cache.h, 2023-02-24), which replaced includes of <cache.h> with
<alloc.h>, which is the new home of `ALLOC_GROW()` (and
`ALLOC_GROW_BY()`).
We can see that run-command.c is the only one that try to use these
macros without including <alloc.h>.
The "compat/" file only talks about the macro in the comment,
and is not broken.
We could fix this by conditionally including "alloc.h" when
`NO_PTHREADS` is defined. But we have relatively few examples of
conditional includes throughout the tree[^1].
Instead, include "alloc.h" unconditionally in run-command.c to allow it
to successfully compile even when NO_PTHREADS is defined.
Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> Co-authored-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 17 Mar 2023 21:03:08 +0000 (14:03 -0700)]
Merge branch 'en/header-cleanup'
Code clean-up to clarify the rule that "git-compat-util.h" must be
the first to be included.
* en/header-cleanup:
diff.h: remove unnecessary include of object.h
Remove unnecessary includes of builtin.h
treewide: replace cache.h with more direct headers, where possible
replace-object.h: move read_replace_refs declaration from cache.h to here
object-store.h: move struct object_info from cache.h
dir.h: refactor to no longer need to include cache.h
object.h: stop depending on cache.h; make cache.h depend on object.h
ident.h: move ident-related declarations out of cache.h
pretty.h: move has_non_ascii() declaration from commit.h
cache.h: remove dependence on hex.h; make other files include it explicitly
hex.h: move some hex-related declarations from cache.h
hash.h: move some oid-related declarations from cache.h
alloc.h: move ALLOC_GROW() functions from cache.h
treewide: remove unnecessary cache.h includes in source files
treewide: remove unnecessary cache.h includes
treewide: remove unnecessary git-compat-util.h includes in headers
treewide: ensure one of the appropriate headers is sourced first
Junio C Hamano [Fri, 17 Mar 2023 21:03:08 +0000 (14:03 -0700)]
Merge branch 'en/dir-api-cleanup'
Code clean-up to clarify directory traversal API.
* en/dir-api-cleanup:
unpack-trees: add usage notices around df_conflict_entry
unpack-trees: special case read-tree debugging as internal usage
unpack-trees: rewrap a few overlong lines from previous patch
unpack-trees: mark fields only used internally as internal
unpack_trees: start splitting internal fields from public API
sparse-checkout: avoid using internal API of unpack-trees, take 2
sparse-checkout: avoid using internal API of unpack-trees
unpack-trees: clean up some flow control
dir: mark output only fields of dir_struct as such
dir: add a usage note to exclude_per_dir
dir: separate public from internal portion of dir_struct
unpack-trees: heed requests to overwrite ignored files
t2021: fix platform-specific leftover cruft
Junio C Hamano [Fri, 17 Mar 2023 21:03:08 +0000 (14:03 -0700)]
Merge branch 'jk/fsck-indices-in-worktrees'
"git fsck" learned to check the index files in other worktrees,
just like "git gc" honors them as anchoring points.
* jk/fsck-indices-in-worktrees:
fsck: check even zero-entry index files
fsck: mention file path for index errors
fsck: check index files in all worktrees
fsck: factor out index fsck
Junio C Hamano [Wed, 1 Mar 2023 00:38:47 +0000 (16:38 -0800)]
Merge branch 'jk/http-test-fixes'
Various fix-ups on HTTP tests.
* jk/http-test-fixes:
t5559: make SSL/TLS the default
t5559: fix test failures with LIB_HTTPD_SSL
t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c
t/lib-httpd: respect $HTTPD_PROTO in expect_askpass()
t5551: drop curl trace lines without headers
t5551: handle v2 protocol in cookie test
t5551: simplify expected cookie file
t5551: handle v2 protocol in upload-pack service test
t5551: handle v2 protocol when checking curl trace
t5551: stop forcing clone to run with v0 protocol
t5551: handle HTTP/2 when checking curl trace
t5551: lower-case headers in expected curl trace
t5551: drop redundant grep for Accept-Language
t5541: simplify and move "no empty path components" test
t5541: stop marking "used receive-pack service" test as v0 only
t5541: run "used receive-pack service" test earlier
René Scharfe [Tue, 28 Feb 2023 16:13:27 +0000 (17:13 +0100)]
range-diff: avoid compiler warning when char is unsigned
Since 2b15969f61 (range-diff: let '--abbrev' option takes effect,
2023-02-20), GCC 11.3 on Ubuntu 22.04 on aarch64 warns (and errors
out if the make variable DEVELOPER is set):
range-diff.c: In function ‘output_pair_header’:
range-diff.c:388:20: error: comparison is always false due to limited range of data type [-Werror=type-limits]
388 | if (abbrev < 0)
| ^
cc1: all warnings being treated as errors
That's because char is unsigned on that platform. Use int instead, just
like in struct diff_options, to copy the value faithfully.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Gwyneth Morgan [Mon, 27 Feb 2023 20:27:18 +0000 (20:27 +0000)]
signature-format.txt: note SSH and X.509 signature delimiters
This document only explains PGP signatures, but Git now supports X.509
signatures as of 1e7adb9756 (gpg-interface: introduce new signature
format "x509" using gpgsm, 2018-07-17), and SSH signatures as of 29b315778e (ssh signing: add ssh key format and signing code,
2021-09-10).
Additionally, explain that these signature formats are controlled
`gpg.format`, linking to its documentation, and explain in said
`gpg.format` documentation that the underlying signature format is
documented in signature-format.txt.
Signed-off-by: Gwyneth Morgan <gwymor@tilde.club> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 27 Feb 2023 18:08:57 +0000 (10:08 -0800)]
Merge branch 'jc/countermand-format-attach'
The format.attach configuration variable lacked a way to override a
value defined in a lower-priority configuration file (e.g. the
system one) by redefining it in a higher-priority configuration
file. Now, setting format.attach to an empty string means show the
patch inline in the e-mail message, without using MIME attachment.
This is a backward incompatible change.
* jc/countermand-format-attach:
format.attach: allow empty value to disable multi-part messages
sscanf(3) used in "git symbolic-ref --short" implementation found
to be not working reliably on macOS in UTF-8 locales. Rewrite the
code to avoid sscanf() altogether to work it around.
Junio C Hamano [Mon, 27 Feb 2023 18:08:57 +0000 (10:08 -0800)]
Merge branch 'rs/archive-mtime'
"git archive HEAD^{tree}" records the paths with the current
timestamp in the archive, making it harder to obtain a stable
output. The command learned the --mtime option to specify an
arbitrary timestamp (e.g. --mtime="@0 +0000" for the epoch).
Andrei Rybak [Sun, 26 Feb 2023 10:53:03 +0000 (11:53 +0100)]
test-lib: drop comment about test_description
When a comment describing how each test file should start was added in
commit [1], it was the second comment of t/test-lib.sh. The comment
describes how variable "test_description" is supposed to be assigned at
the top of each test file and how "test-lib.sh" should be used by
sourcing it. However, even in [1], the comment was ten lines away from
the usage of the variable by test-lib.sh. Since then, the comment has
drifted away both from the top of the file and from the usage of the
variable. The comment just sits in the middle of the initialization of
the test library, surrounded by unrelated code, almost one hundred lines
away from the usage of "test_description".
Nobody has noticed this drift during evolution of test-lib.sh, which
suggests that this comment has outlived its usefulness. The assignment
of "test_description", sourcing of "test-lib.sh" by tests, and the
process of writing tests in general are described in detail in
"t/README". So drop the obsolete comment.
An alternative solution could be to move the comment either to the top
of the file, or down to the usage of variable "test_description".
[1] e1970ce43a ("[PATCH 1/2] Test framework take two.", 2005-05-13)
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:20 +0000 (15:28 +0000)]
unpack-trees: add usage notices around df_conflict_entry
Avoid making users believe they need to initialize df_conflict_entry
to something (as happened with other output only fields before) with
a quick comment and a small sanity check.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:19 +0000 (15:28 +0000)]
unpack-trees: special case read-tree debugging as internal usage
builtin/read-tree.c has some special functionality explicitly designed
for debugging unpack-trees.[ch]. Associated with that is two fields
that no other external caller would or should use. Mark these as
internal to unpack-trees, but allow builtin/read-tree to read or write
them for this special case.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:18 +0000 (15:28 +0000)]
unpack-trees: rewrap a few overlong lines from previous patch
The previous patch made many lines a little longer, resulting in four
becoming a bit too long. They were left as-is for the previous patch
to facilitate reviewers verifying that we were just adding "internal."
in a bunch of places, but rewrap them now.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:17 +0000 (15:28 +0000)]
unpack-trees: mark fields only used internally as internal
Continue the work from the previous patch by finding additional fields
which are only used internally but not yet explicitly marked as such,
and include them in the internal fields struct.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:16 +0000 (15:28 +0000)]
unpack_trees: start splitting internal fields from public API
This just splits the two fields already marked as internal-only into a
separate internal struct. Future commits will add more fields that
were meant to be internal-only but were not explicitly marked as such
to the same struct.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:15 +0000 (15:28 +0000)]
sparse-checkout: avoid using internal API of unpack-trees, take 2
Commit 2f6b1eb794 ("cache API: add a "INDEX_STATE_INIT" macro/function,
add release_index()", 2023-01-12) mistakenly added some initialization
of a member of unpack_trees_options that was intended to be
internal-only. This initialization should be done within
update_sparsity() instead.
Note that while o->result is mostly meant for unpack_trees() and
update_sparsity() mostly operates without o->result,
check_ok_to_remove() does consult it so we need to ensure it is properly
initialized.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:14 +0000 (15:28 +0000)]
sparse-checkout: avoid using internal API of unpack-trees
struct unpack_trees_options has the following field and comment:
struct pattern_list *pl; /* for internal use */
Despite the internal-use comment, commit e091228e17 ("sparse-checkout:
update working directory in-process", 2019-11-21) starting setting this
field from an external caller. At the time, the only way around that
would have been to modify unpack_trees() to take an extra pattern_list
argument, and there's a lot of callers of that function. However, when
we split update_sparsity() off as a separate function, with
sparse-checkout being the sole caller, the need to update other callers
went away. Fix this API problem by adding a pattern_list argument to
update_sparsity() and stop setting the internal o.pl field directly.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:13 +0000 (15:28 +0000)]
unpack-trees: clean up some flow control
The update_sparsity() function was introduced in commit 7af7a25853
("unpack-trees: add a new update_sparsity() function", 2020-03-27).
Prior to that, unpack_trees() was used, but that had a few bugs because
the needs of the caller were different, and different enough that
unpack_trees() could not easily be modified to handle both usecases.
The implementation detail that update_sparsity() was written by copying
unpack_trees() and then streamlining it, and then modifying it in the
needed ways still shows through in that there are leftover vestiges in
both functions that are no longer needed. Clean them up. In
particular:
* update_sparsity() allows a pattern list to be passed in, but
unpack_trees() never should use a different pattern list. Add a
check and a BUG() if this gets violated.
* update_sparsity() has a check early on that will BUG() if
o->skip_sparse_checkout is set; as such, there's no need to check
for that condition again later in the code. We can simply remove
the check and its corresponding goto label.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:11 +0000 (15:28 +0000)]
dir: add a usage note to exclude_per_dir
As evidenced by the fix a couple commits ago, places in the code using
exclude_per_dir are likely buggy and should be adapted to call
setup_standard_excludes() instead. Unfortunately, the usage of
exclude_per_dir has been hardcoded into the arguments ls-files accepts,
so we cannot actually remove it. Add a note that it is deprecated and
no other callers should use it directly.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:10 +0000 (15:28 +0000)]
dir: separate public from internal portion of dir_struct
In order to make it clearer to callers what portions of dir_struct are
public API, and avoid errors from them setting fields that are meant as
internal API, split the fields used for internal implementation reasons
into a separate embedded struct.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:09 +0000 (15:28 +0000)]
unpack-trees: heed requests to overwrite ignored files
When a directory exists but has only ignored files within it and we are
trying to switch to a branch that has a file where that directory is,
the behavior depends upon --[no]-overwrite-ignore. If the user wants to
--overwrite-ignore (the default), then we should delete the ignored file
and directory and switch to the new branch.
The code to handle this in verify_clean_subdirectory() in unpack-trees
tried to handle this via paying attention to the exclude_per_dir setting
of the internal dir field. This came from commit c81935348b ("Fix
switching to a branch with D/F when current branch has file D.",
2007-03-15), which pre-dated 039bc64e88 ("core.excludesfile clean-up",
2007-11-14), and thus did not pay attention to ignore patterns from
other relevant files. Change it to use setup_standard_excludes() so
that it is also aware of excludes specified in other locations.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Mon, 27 Feb 2023 15:28:08 +0000 (15:28 +0000)]
t2021: fix platform-specific leftover cruft
t2021.6 existed to test the status of a symlink that was left around by
previous tests. It tried to also clean up the symlink after it was done
so that subsequent tests wouldn't be tripped up by it. Unfortunately,
since this test had a SYMLINK prerequisite, that made the cleanup
platform dependent...and made a testcase I was trying to add to this
testsuite fail (that testcase will be included in the next patch).
Before we go and add new testcases, fix this cleanup by moving it into a
separate test.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sun, 26 Feb 2023 22:29:43 +0000 (17:29 -0500)]
fsck: check even zero-entry index files
In fb64ca526a (fsck: check index files in all worktrees, 2023-02-24), we
swapped out a call to vanilla repo_read_index() for a series of
read_index_from() calls, one per worktree. The code for the latter was
copied from add_index_objects_to_pending(), which checks for a positive
return value from the index reading function, and we do the same here in
fsck now.
But this is probably the wrong thing. I had interpreted the check as
"don't operate on the index struct if there was an error". But in
reality, if there is an error then the index-reading code will simply
die (which admittedly is not great for fsck, but that is not a new
problem).
The return value here is actually the number of entries read. So it
makes sense for add_index_objects_to_pending() to ignore a zero-entry
index (there is nothing to add). But for fsck, we would still want to
check any extensions, etc (though presumably it is unlikely to have them
in an empty index, I don't think it's impossible).
So we should ignore the return value from read_index_from() entirely.
This matches the behavior before fb64ca526a, when we ignored the return
value from repo_read_index().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Zhang Yi [Sat, 25 Feb 2023 01:30:03 +0000 (09:30 +0800)]
t9700: modernize test scripts
The style of t9700-perl-git.sh is old. There are 3 problems:
* A title is not on the same line with test_expect_success command.
* A test body is indented by whitespaces.
* There are whitespaces after redirect operators.
Modernize test scripts by:
* Combine the title with test_expect_success command.
* Replace whitespace indents with TAB.
* Delete whitespaces after redirect operators.
Signed-off-by: Zhang Yi <18994118902@163.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 24 Feb 2023 19:32:30 +0000 (11:32 -0800)]
Merge branch 'jc/genzeros-avoid-raw-write'
A test helper had a single write(2) of 256kB, which was too big for
some platforms (e.g. NonStop), which has been corrected by using
xwrite() wrapper appropriately.
* jc/genzeros-avoid-raw-write:
test-genzeros: avoid raw write(2)
Jeff King [Fri, 24 Feb 2023 08:12:11 +0000 (03:12 -0500)]
fsck: mention file path for index errors
If we encounter an error in an index file, we may say something like:
error: 1234abcd: invalid sha1 pointer in resolve-undo
But if you have multiple worktrees, each with its own index, it can be
very helpful to know which file had the problem. So let's pass that path
down through the various index-fsck functions and use it where
appropriate. After this patch you should get something like:
error: 1234abcd: invalid sha1 pointer in resolve-undo of .git/worktrees/wt/index
That's a bit verbose, but since the point is that you shouldn't see this
normally, we're better to err on the side of more details.
I've also added the index filename to the name used by "fsck
--name-objects", which will show up if we find the object to be missing,
etc. This is bending the rules a little there, as the option claims to
write names that can be fed to rev-parse. But there is no revision
syntax to access the index of another worktree, so the best we can do is
make up something that a human will probably understand.
I did take care to retain the existing ":file" syntax for the current
worktree. So the uglier output should kick in only when it's actually
necessary. See the included tests for examples of both forms.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 08:09:57 +0000 (03:09 -0500)]
fsck: check index files in all worktrees
We check the index file for the main worktree, but completely ignore the
index files in other worktrees. These should be checked, too, as they
are part of the repository state (and in particular, errors in those
index files may cause repo-wide operations like "git gc" to complain).
Reported-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Fri, 24 Feb 2023 08:07:32 +0000 (03:07 -0500)]
fsck: factor out index fsck
The code to fsck an index operates directly on the_index. Let's move it
into its own function in preparation for handling the index files from
other worktrees.
Since we now have only a single reference to the_index, let's drop
our USE_THE_INDEX_VARIABLE definition and just use the_repository.index
directly. That's a minor cleanup, but also ensures that we didn't miss
any references when moving the code into fsck_index().
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:32 +0000 (00:09 +0000)]
object-store.h: move struct object_info from cache.h
Move struct object_info, and a few related #define's from cache.h to
object-store.h.
A surprising effect of this change is that replace-object.h, which
includes object-store.h, now needs to directly include cache.h since
that is where read_replace_refs is declared and that variable is used
in one of its inline functions. The next commit will move that
declaration and fix that unfortunate new direct inclusion of cache.h.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:30 +0000 (00:09 +0000)]
object.h: stop depending on cache.h; make cache.h depend on object.h
Things should be able to depend on object.h without pulling in all of
cache.h. Move an enum to allow this.
Note that a couple files previously depended on things brought in
through cache.h indirectly (revision.h -> commit.h -> object.h ->
cache.h). As such, this change requires making existing dependencies
more explicit in half a dozen files. The inclusion of strbuf.h in
some headers if of particular note: these headers directly embedded a
strbuf in some new structs, meaning they should have been including
strbuf.h all along but were indirectly getting the necessary
definitions.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:26 +0000 (00:09 +0000)]
hex.h: move some hex-related declarations from cache.h
hex.c contains code for hex-related functions, but for some reason these
functions were declared in the catch-all cache.h. Move the function
declarations into a hex.h header instead.
This also allows us to remove includes of cache.h from a few C files.
For now, we make cache.h include hex.h, so that it is easier to review
the direct changes being made by this patch. In the next patch, we will
remove that, and add the necessary direct '#include "hex.h"' in the
hundreds of C files that need it.
Note that reviewing the header changes in this commit might be
simplified via
git log --no-walk -p --color-moved $COMMIT -- '*.h'`
In particular, it highlights the simple movement of code in .h files
rather nicely.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:25 +0000 (00:09 +0000)]
hash.h: move some oid-related declarations from cache.h
These defines and enum are all oid-related and as such seem to make
more sense being included in hash.h. Further, moving them there
allows us to remove some includes of cache.h in other files.
The change to line-log.h might look unrelated, but line-log.h includes
diffcore.h, which previously included cache.h, which included the
kitchen sink. Since this patch makes diffcore.h no longer include
cache.h, the compiler complains about the 'struct string_list *'
function parameter. Add a forward declaration for struct string_list to
address this.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:24 +0000 (00:09 +0000)]
alloc.h: move ALLOC_GROW() functions from cache.h
This allows us to replace includes of cache.h with includes of the much
smaller alloc.h in many places. It does mean that we also need to add
includes of alloc.h in a number of C files.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:23 +0000 (00:09 +0000)]
treewide: remove unnecessary cache.h includes in source files
We had several C files include cache.h unnecessarily. Replace those
with an include of "git-compat-util.h" instead. Much like the previous
commit, these have all been verified via both ensuring that
gcc -E $SOURCE_FILE | grep '"cache.h"'
found no hits and that
make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE}
successfully compiles without warnings.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:22 +0000 (00:09 +0000)]
treewide: remove unnecessary cache.h includes
We had several header files include cache.h unnecessarily. Remove
those. These have all been verified via both ensuring that
gcc -E $HEADER | grep '"cache.h"'
found no hits and that
cat >temp.c <<EOF &&
#include "git-compat-util.h"
#include "$HEADER"
int main() {}
EOF
gcc -c temp.c
successfully compiles without warnings.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Fri, 24 Feb 2023 00:09:21 +0000 (00:09 +0000)]
treewide: remove unnecessary git-compat-util.h includes in headers
For sanity, we should probably do one of the following:
(a) make C and header files both depend upon everything they need
(b) consistently exclude git-compat-util.h from headers and require it
be the first include in C files
Currently, we have some of the headers following (a) and others
following (b), which makes things messy. In the past I was pushed
towards (b), as per [1] and [2]. Further, during this series I
discovered that this mixture empirically will mean that we end up with C
files that do not directly include git-compat-util.h, and do include
headers that don't include git-compat-util.h, with the result that we
likely have headers included before an indirect inclusion of
git-compat-util.h. Since git-compat-util.h has tricky platform-specific
stuff that is meant to be included before everything else, this state of
affairs is risky and may lead to things breaking in subtle ways (and
only on some platforms) as per [1] and [2].
Since including git-compat-util.h in existing header files makes it
harder for us to catch C files that are missing that include, let's
switch to (b) to make the enforcement of this rule easier. Remove the
inclusion of git-compat-util.h from header files other than the ones
that have been approved as alternate first includes.
Elijah Newren [Fri, 24 Feb 2023 00:09:20 +0000 (00:09 +0000)]
treewide: ensure one of the appropriate headers is sourced first
We had several C files ignoring the rule to include one of the
appropriate headers first; fix that.
While at it, the rule in Documentation/CodingGuidelines about which
header to include has also fallen out of sync, so update the wording to
mention other allowed headers.
Unfortunately, C files in reftable/ don't actually follow the previous
or updated rule. If you follow the #include chain in its C files,
reftable/system.h _tends_ to be first (i.e. record.c first includes
record.h, which first includes basics.h, which first includees
system.h), but not always (e.g. publicbasics.c includes another header
first that does not include system.h). However, I'm going to punt on
making actual changes to the C files in reftable/ since I do not want to
risk bringing it out-of-sync with any version being used externally.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood [Thu, 23 Feb 2023 20:55:01 +0000 (20:55 +0000)]
rebase -i: fix parsing of "fixup -C<commit>"
If the user omits the space between "-C" and the commit in a fixup
command then it is parsed as an ordinary fixup and the commit message is
not updated as it should be. Fix this by making the space between "-C"
and "<commit>" optional as it is for the "merge" command.
Note that set_replace_editor() is changed to set $GIT_SEQUENCE_EDITOR
instead of $EDITOR in order to be able to replace the todo list and
reword commits with $FAKE_COMMIT_MESSAGE. This is safe as all the
existing users are using set_replace_editor() to replace the todo list.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Phillip Wood [Thu, 23 Feb 2023 20:55:00 +0000 (20:55 +0000)]
rebase -i: match whole word in is_command()
When matching an unabbreviated command is_command() only does a prefix
match which means it parses "pickled" as TODO_PICK. parse_insn_line()
does error out because is_command() only advances as far as the end of
"pick" so it looks like the command name is not followed by a space but
the error message is "missing arguments for pick" rather than telling
the user that the "pickled" is not a valid command.
Fix this by ensuring the match is follow by whitespace or the end of the
string as we already do for abbreviated commands. The (*bol = p) at the
end of the condition is a bit cute for my taste but I decided to leave
it be for now. Rather than add new tests the existing tests for bad
commands are adapted to use a bad command name that triggers the prefix
matching bug.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:08:41 +0000 (06:08 -0500)]
t5559: make SSL/TLS the default
The point of t5559 is run the regular t5551 tests with HTTP/2. But it
does so with the "h2c" protocol, which uses cleartext upgrades from
HTTP/1.1 to HTTP/2 (rather than learning about HTTP/2 support during the
TLS negotiation).
This has a few problems:
- it's not very indicative of the real world. In practice, most servers
that support HTTP/2 will also support TLS.
- support for upgrading does not seem as robust. In particular, we've
run into bugs in some versions of Apache's mod_http2 that trigger
only with the upgrade mode. See:
So the upside is that this change makes our HTTP/2 tests more robust and
more realistic. The downside is that if we can't set up SSL for any
reason, we'll skip the tests (even though you _might_ have been able to
run the HTTP/2 tests the old way). We could probably have a conditional
fallback, but it would be complicated for little gain, and it's not even
clear it would help (i.e., would any test environment even have HTTP/2
but not SSL support?).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:07:29 +0000 (06:07 -0500)]
t5559: fix test failures with LIB_HTTPD_SSL
One test needs to be tweaked in order for t5559 to pass with SSL/TLS set
up. When we make our initial clone, we check that the curl trace of
requests is what we expected. But we need to fix two things:
- along with ignoring "data" lines from the trace, we need to ignore
"SSL data" lines
- when TLS is used, the server is able to tell the client (via ALPN)
that it supports HTTP/2 before the first HTTP request is made. So
rather than request an upgrade using an HTTP header, it can just
speak HTTP/2 immediately
With this patch, running:
LIB_HTTPD_SSL=1 ./t5559-http-fetch-smart-http2.sh
works, whereas it did not before.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:06:44 +0000 (06:06 -0500)]
t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c
Commit 73c49a4474 (t: run t5551 tests with both HTTP and HTTP/2,
2022-11-11) added Apache config to enable HTTP/2. However, it only
enabled the "h2c" protocol, which allows cleartext HTTP/2 (generally
based on an upgrade header during an HTTP/1.1 request). This is what
t5559 is generally testing, since by default we don't set up SSL/TLS.
However, it should be possible to run t5559 with LIB_HTTPD_SSL set. In
that case, Apache will advertise support for HTTP/2 via ALPN during the
TLS handshake. But we need to tell it support "h2" (the non-cleartext
version) to do so. Without that, then curl does not even try to do the
HTTP/1.1 upgrade (presumably because after seeing that we did TLS but
didn't get the ALPN indicator, it assumes it would be fruitless).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:05:55 +0000 (06:05 -0500)]
t/lib-httpd: respect $HTTPD_PROTO in expect_askpass()
When the HTTP tests are run with LIB_HTTPD_SSL in the environment, then
we access the test server as https://. This causes expect_askpass to
complain, because it tries to blindly match "http://" in the prompt
shown to the user. We can adjust this to use $HTTPD_PROTO, which is set
during the setup phase.
Note that this is enough for t5551 and t5559 to pass when run with
https, but there are similar problems in other scripts that will need to
be fixed before the whole suite can run with LIB_HTTPD_SSL.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:05:07 +0000 (06:05 -0500)]
t5551: drop curl trace lines without headers
We pick apart a curl trace, looking for "=> Send header:" and so on, and
matching against an expected set of requests and responses. We remove
"== Info" lines entirely. However, our parser is fooled when running the
test with LIB_HTTPD_SSL on Ubuntu 20.04 (as found in our linux-gcc CI
job), as curl hands us an "Info" buffer with a newline, and we get:
which results in the "CApath" line ending up in the cleaned-up output,
causing the test to fail.
Arguably the tracing code should detect this and put it on two separate
"== Info" lines. But this is actually a curl bug, fixed by their 80d73bcca (tls: provide the CApath verbose log on its own line,
2020-08-18). It's simpler to just work around it here.
Since we are using GIT_TRACE_CURL, every line should just start with one
of "<=", "==", or "=>", and we can throw away anything else. In fact, we
can just replace the pattern for deleting "*" lines. Those were from the
old GIT_CURL_VERBOSE output, but we switched over in 14e24114d9
(t5551-http-fetch-smart.sh: use the GIT_TRACE_CURL environment var,
2016-09-05).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:02:31 +0000 (06:02 -0500)]
t5551: handle v2 protocol in cookie test
After making a request, we check that it stored the expected cookies.
This depends on the protocol version, because the cookies we store
depend on the exact requests we made (and for ls-remote, v2 will always
hit /git-upload-pack to get the refs, whereas v0 is happy with the
initial ref advertisement).
As a result, hardly anybody runs this test, as you'd have to manually
set GIT_TEST_PROTOCOL_VERSION=0 to do so.
Let's teach it to handle both protocol versions. One way to do this
would be to make the expectation conditional on the protocol used. But
there's a simpler solution. The reason that v0 doesn't hit
/git-upload-pack is that ls-remote doesn't fetch any objects. If we
instead do a fetch (making sure there's an actual object to grab), then
both v0 and v2 will hit the same endpoints and set the same cookies.
Note that we do have to clean up our new tag here; otherwise it confuses
the later "clone 2,000 tags" test.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Thu, 23 Feb 2023 11:01:31 +0000 (06:01 -0500)]
t5551: simplify expected cookie file
After making an HTTP request that should store cookies, we check that
the expected values are in the cookie file. We don't want to look at the
whole file, because it has noisy comments at the top that we shouldn't
depend on. But we strip out the interesting bits using "tail -3", which
is brittle. It requires us to put an extra blank line in our expected
output, and it would fail to notice any reordering or extra content in
the cookie file.
Instead, let's just grep for non-blank lines that are not comments,
which more directly describes what we're interested in.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
