1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-05-18 22:56:07 +02:00
Commit Graph

360 Commits

Author SHA1 Message Date
Junio C Hamano 38bbb9e990 Merge branch 'ab/string-list-count-in-size-t'
Count string_list items in size_t, not "unsigned int".

* ab/string-list-count-in-size-t:
  string-list API: change "nr" and "alloc" to "size_t"
  gettext API users: don't explicitly cast ngettext()'s "n"
2022-03-16 17:53:09 -07:00
Junio C Hamano 7431379a9c Merge branch 'ab/racy-hooks'
Code clean-up to allow callers of run_commit_hook() to learn if it
got "success" because the hook succeeded or because there wasn't
any hook.

* ab/racy-hooks:
  hooks: fix an obscure TOCTOU "did we just run a hook?" race
  merge: don't run post-hook logic on --no-verify
2022-03-16 17:53:09 -07:00
Junio C Hamano 430883a70c Merge branch 'ab/object-file-api-updates'
Object-file API shuffling.

* ab/object-file-api-updates:
  object-file API: pass an enum to read_object_with_reference()
  object-file.c: add a literal version of write_object_file_prepare()
  object-file API: have hash_object_file() take "enum object_type"
  object API: rename hash_object_file_literally() to write_*()
  object-file API: split up and simplify check_object_signature()
  object API users + docs: check <0, not !0 with check_object_signature()
  object API docs: move check_object_signature() docs to cache.h
  object API: correct "buf" v.s. "map" mismatch in *.c and *.h
  object-file API: have write_object_file() take "enum object_type"
  object-file API: add a format_object_header() function
  object-file API: return "void", not "int" from hash_object_file()
  object-file.c: split up declaration of unrelated variables
2022-03-16 17:53:08 -07:00
Ævar Arnfjörð Bjarmason a8cc594333 hooks: fix an obscure TOCTOU "did we just run a hook?" race
Fix a Time-of-check to time-of-use (TOCTOU) race in code added in
680ee550d7 (commit: skip discarding the index if there is no
pre-commit hook, 2017-08-14).

This obscure race condition can occur if we e.g. ran the "pre-commit"
hook and it modified the index, but hook_exists() returns false later
on (e.g., because the hook itself went away, the directory became
unreadable, etc.). Then we won't call discard_cache() when we should
have.

The race condition itself probably doesn't matter, and users would
have been unlikely to run into it in practice. This problem has been
noted on-list when 680ee550d7 was discussed[1], but had not been
fixed.

This change is mainly intended to improve the readability of the code
involved, and to make reasoning about it more straightforward. It
wasn't as obvious what we were trying to do here, but by having an
"invoked_hook" it's clearer that e.g. our discard_cache() is happening
because of the earlier hook execution.

Let's also change this for the push-to-checkout hook. Now instead of
checking if the hook exists and either doing a push to checkout or a
push to deploy we'll always attempt a push to checkout. If the hook
doesn't exist we'll fall back on push to deploy. The same behavior as
before, without the TOCTOU race. See 0855331941 (receive-pack:
support push-to-checkout hook, 2014-12-01) for the introduction of the
previous behavior.

This leaves uses of hook_exists() in two places that matter. The
"reference-transaction" check in refs.c, see 6754159767 (refs:
implement reference transaction hook, 2020-06-19), and the
"prepare-commit-msg" hook, see 66618a50f9 (sequencer: run
'prepare-commit-msg' hook, 2018-01-24).

In both of those cases we're saving ourselves CPU time by not
preparing data for the hook that we'll then do nothing with if we
don't have the hook. So using this "invoked_hook" pattern doesn't make
sense in those cases.

The "reference-transaction" and "prepare-commit-msg" hook also aren't
racy. In those cases we'll skip the hook runs if we race with a new
hook being added, whereas in the TOCTOU races being fixed here we were
incorrectly skipping the required post-hook logic.

1. https://lore.kernel.org/git/20170810191613.kpmhzg4seyxy3cpq@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07 13:00:53 -08:00
Ævar Arnfjörð Bjarmason 99d60545f8 string-list API: change "nr" and "alloc" to "size_t"
Change the "nr" and "alloc" members of "struct string_list" to use
"size_t" instead of "nr". On some platforms the size of an "unsigned
int" will be smaller than a "size_t", e.g. a 32 bit unsigned v.s. 64
bit unsigned. As "struct string_list" is a generic API we use in a lot
of places this might cause overflows.

As one example: code in "refs.c" keeps track of the number of refs
with a "size_t", and auxiliary code in builtin/remote.c in
get_ref_states() appends those to a "struct string_list".

While we're at it split the "nr" and "alloc" in string-list.h across
two lines, which is the case for most such struct member
declarations (e.g. in "strbuf.h" and "strvec.h").

Changing e.g. "int i" to "size_t i" in run_and_feed_hook() isn't
strictly necessary, and there are a lot more cases where we'll use a
local "int", "unsigned int" etc. variable derived from the "nr" in the
"struct string_list". But in that case as well as
add_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the
printf format referring to "nr" anyway, so let's also change the other
variables referring to it.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07 12:02:04 -08:00
Ævar Arnfjörð Bjarmason c80d226a04 object-file API: have write_object_file() take "enum object_type"
Change the write_object_file() function to take an "enum object_type"
instead of a "const char *type". Its callers either passed
{commit,tree,blob,tag}_type and can pass the corresponding OBJ_* type
instead, or were hardcoding strings like "blob".

This avoids the back & forth fragility where the callers of
write_object_file() would have the enum type, and convert it
themselves via type_name(). We do have to now do that conversion
ourselves before calling write_object_file_prepare(), but those
codepaths will be similarly adjusted in subsequent commits.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-25 17:16:31 -08:00
Junio C Hamano 867b520301 Merge branch 'cb/clear-quarantine-early-on-all-ref-update-errors'
"receive-pack" checks if it will do any ref updates (various
conditions could reject a push) before received objects are taken
out of the temporary directory used for quarantine purposes, so
that a push that is known-to-fail will not leave crufts that a
future "gc" needs to clean up.

* cb/clear-quarantine-early-on-all-ref-update-errors:
  receive-pack: purge temporary data if no command is ready to run
2022-02-18 13:53:27 -08:00
Junio C Hamano c70bc338e9 Merge branch 'ab/config-based-hooks-2'
More "config-based hooks".

* ab/config-based-hooks-2:
  run-command: remove old run_hook_{le,ve}() hook API
  receive-pack: convert push-to-checkout hook to hook.h
  read-cache: convert post-index-change to use hook.h
  commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
  git-p4: use 'git hook' to run hooks
  send-email: use 'git hook run' for 'sendemail-validate'
  git hook run: add an --ignore-missing flag
  hooks: convert worktree 'post-checkout' hook to hook library
  hooks: convert non-worktree 'post-checkout' hook to hook library
  merge: convert post-merge to use hook.h
  am: convert applypatch-msg to use hook.h
  rebase: convert pre-rebase to use hook.h
  hook API: add a run_hooks_l() wrapper
  am: convert {pre,post}-applypatch to use hook.h
  gc: use hook library for pre-auto-gc hook
  hook API: add a run_hooks() wrapper
  hook: add 'run' subcommand
2022-02-09 14:21:00 -08:00
Junio C Hamano 492261a6de Merge branch 'jc/find-header'
Code clean-up.

* jc/find-header:
  receive-pack.c: consolidate find header logic
2022-02-05 09:42:29 -08:00
Chen Bojun 5407764069 receive-pack: purge temporary data if no command is ready to run
When pushing a hidden ref, e.g.:

    $ git push origin HEAD:refs/hidden/foo

"receive-pack" will reject our request with an error message like this:

    ! [remote rejected] HEAD -> refs/hidden/foo (deny updating a hidden ref)

The remote side ("git-receive-pack") will not create the hidden ref as
expected, but the pack file sent by "git-send-pack" is left inside the
remote repository. I.e. the quarantine directory is not purged as it
should be.

Add a checkpoint before calling "tmp_objdir_migrate()" and after calling
the "pre-receive" hook to purge that temporary data in the quarantine
area when there is no command ready to run.

The reason we do not add the checkpoint before the "pre-receive" hook,
but after it, is that the "pre-receive" hook is called with a switch-off
"skip_broken" flag, and all commands, even broken ones, should be fed
by calling "feed_receive_hook()".

Add a new test case in t5516 as well.

Helped-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Helped-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Chen Bojun <bojun.cbj@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-01 14:51:36 -08:00
Emily Shaffer 306f445ed1 receive-pack: convert push-to-checkout hook to hook.h
Move the push-to-checkout hook away from run-command.h to and over to
the new hook.h library.

This removes the last direct user of run_hook_le(), so we could remove
that function now, but let's leave that to a follow-up cleanup commit.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Acked-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-07 15:19:35 -08:00
John Cai cfc5cf428b receive-pack.c: consolidate find header logic
There are two functions that have very similar logic of finding a header
value. find_commit_header, and find_header. We can conslidate the logic
by introducing a new function find_header_mem, which is equivalent to
find_commit_header except it takes a len parameter that determines how
many bytes will be read. find_commit_header and find_header can then both
call find_header_mem.

This reduces duplicate logic, as the logic for finding header values
can now all live in one place.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-06 13:17:20 -08:00
Junio C Hamano 0dc90d954d Merge branch 'ns/tmp-objdir'
New interface into the tmp-objdir API to help in-core use of the
quarantine feature.

* ns/tmp-objdir:
  tmp-objdir: disable ref updates when replacing the primary odb
  tmp-objdir: new API for creating temporary writable databases
2022-01-03 16:24:15 -08:00
Junio C Hamano 13fa77b689 Merge branch 'ak/protect-any-current-branch'
"git fetch" without the "--update-head-ok" option ought to protect
a checked out branch from getting updated, to prevent the working
tree that checks it out to go out of sync.  The code was written
before the use of "git worktree" got widespread, and only checked
the branch that was checked out in the current worktree, which has
been updated.
(originally called ak/fetch-not-overwrite-any-current-branch)

* ak/protect-any-current-branch:
  branch: protect branches checked out in all worktrees
  receive-pack: protect current branch for bare repository worktree
  receive-pack: clean dead code from update_worktree()
  fetch: protect branches checked out in all worktrees
  worktree: simplify find_shared_symref() memory ownership model
  branch: lowercase error messages
  receive-pack: lowercase error messages
  fetch: lowercase error messages
2021-12-21 15:03:16 -08:00
Junio C Hamano d2f0b72759 Merge branch 'fs/ssh-signing-key-lifetime'
Extend the signing of objects with SSH keys and learn to pay
attention to the key validity time range when verifying.

* fs/ssh-signing-key-lifetime:
  ssh signing: verify ssh-keygen in test prereq
  ssh signing: make fmt-merge-msg consider key lifetime
  ssh signing: make verify-tag consider key lifetime
  ssh signing: make git log verify key lifetime
  ssh signing: make verify-commit consider key lifetime
  ssh signing: add key lifetime test prereqs
  ssh signing: use sigc struct to pass payload
  t/fmt-merge-msg: make gpgssh tests more specific
  t/fmt-merge-msg: do not redirect stderr
2021-12-21 15:03:15 -08:00
Junio C Hamano 4f7e2f0b21 Merge branch 'rj/receive-pack-avoid-sigpipe-during-status-reporting'
When the "git push" command is killed while the receiving end is
trying to report what happened to the ref update proposals, the
latter used to die, due to SIGPIPE.  The code now ignores SIGPIPE
to increase our chances to run the post-receive hook after it
happens.

* rj/receive-pack-avoid-sigpipe-during-status-reporting:
  receive-pack: ignore SIGPIPE while reporting status to client
2021-12-15 09:39:48 -08:00
Fabian Stelzer 02769437e1 ssh signing: use sigc struct to pass payload
To be able to extend the payload metadata with things like its creation
timestamp or the creators ident we remove the payload parameters to
check_signature() and use the already existing sigc->payload field
instead, only adding the length field to the struct. This also allows
us to get rid of the xmemdupz() calls in the verify functions. Since
sigc is now used to input data as well as output the result move it to
the front of the function list.

 - Add payload_length to struct signature_check
 - Populate sigc.payload/payload_len on all call sites
 - Remove payload parameters to check_signature()
 - Remove payload parameters to internal verify_* functions and use sigc
   instead
 - Remove xmemdupz() used for verbose output since payload is now already
   populated.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 13:38:04 -08:00
Neeraj Singh b3cecf49ea tmp-objdir: new API for creating temporary writable databases
The tmp_objdir API provides the ability to create temporary object
directories, but was designed with the goal of having subprocesses
access these object stores, followed by the main process migrating
objects from it to the main object store or just deleting it.  The
subprocesses would view it as their primary datastore and write to it.

Here we add the tmp_objdir_replace_primary_odb function that replaces
the current process's writable "main" object directory with the
specified one. The previous main object directory is restored in either
tmp_objdir_migrate or tmp_objdir_destroy.

For the --remerge-diff usecase, add a new `will_destroy` flag in `struct
object_database` to mark ephemeral object databases that do not require
fsync durability.

Add 'git prune' support for removing temporary object databases, and
make sure that they have a name starting with tmp_ and containing an
operation-specific name.

Based-on-patch-by: Elijah Newren <newren@gmail.com>

Signed-off-by: Neeraj Singh <neerajsi@microsoft.com>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-08 14:06:36 -08:00
Anders Kaseorg 9fdf4f1db4 receive-pack: protect current branch for bare repository worktree
A bare repository won’t have a working tree at "..", but it may still
have separate working trees created with git worktree. We should protect
the current branch of such working trees from being updated or deleted,
according to receive.denyCurrentBranch.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01 22:18:25 -08:00
Anders Kaseorg 38baae6cfe receive-pack: clean dead code from update_worktree()
update_worktree() can only be called with a non-NULL worktree parameter,
because that’s the only case where we set do_update_worktree = 1.
worktree->path is always initialized to non-NULL.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01 22:18:25 -08:00
Anders Kaseorg c8dd491fa5 worktree: simplify find_shared_symref() memory ownership model
Storing the worktrees list in a static variable meant that
find_shared_symref() had to rebuild the list on each call (which is
inefficient when the call site is in a loop), and also that each call
invalidated the pointer returned by the previous call (which is
confusing).

Instead, make it the caller’s responsibility to pass in the worktrees
list and manage its lifetime.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01 22:18:25 -08:00
Anders Kaseorg c25edee9a5 receive-pack: lowercase error messages
Documentation/CodingGuidelines says “do not end error messages with a
full stop” and “do not capitalize the first word”.  Clean up existing
messages, some of which we will be touching in later steps in the
series, that deviate from these rules in this file, as a preparation for
the main part of the topic.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-01 22:18:24 -08:00
Ævar Arnfjörð Bjarmason c7c4bdeccf run-command API: remove "env" member, always use "env_array"
Remove the "env" member from "struct child_process" in favor of always
using the "env_array". As with the preceding removal of "argv" in
favor of "args" this gets rid of current and future oddities around
memory management at the API boundary (see the amended API docs).

For some of the conversions we can replace patterns like:

    child.env = env->v;

With:

    strvec_pushv(&child.env_array, env->v);

But for others we need to guard the strvec_pushv() with a NULL check,
since we're not passing in the "v" member of a "struct strvec",
e.g. in the case of tmp_objdir_env()'s return value.

Ideally we'd rename the "env_array" member to simply "env" as a
follow-up, since it and "args" are now inconsistent in not having an
"_array" suffix, and seemingly without any good reason, unless we look
at the history of how they came to be.

But as we've currently got 122 in-tree hits for a "git grep env_array"
let's leave that for now (and possibly forever). Doing that rename
would be too disruptive.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25 22:15:08 -08:00
Ævar Arnfjörð Bjarmason 7f14609e29 run-command API users: use strvec_push(), not argv construction
Change a pattern of hardcoding an "argv" array size, populating it and
assigning to the "argv" member of "struct child_process" to instead
use "strvec_push()" to add data to the "args" member.

As noted in the preceding commit this moves us further towards being
able to remove the "argv" member in a subsequent commit

These callers could have used strvec_pushl(), but moving to
strvec_push() makes the diff easier to read, and keeps the arguments
aligned as before.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25 22:15:07 -08:00
Ævar Arnfjörð Bjarmason 2b7098936c run-command API users: use strvec_pushl(), not argv construction
Change a pattern of hardcoding an "argv" array size, populating it and
assigning to the "argv" member of "struct child_process" to instead
use "strvec_pushl()" to add data to the "args" member.

This implements the same behavior as before in fewer lines of code,
and moves us further towards being able to remove the "argv" member in
a subsequent commit.

Since we've entirely removed the "argv" variable(s) we can be sure
that no potential logic errors of the type discussed in a preceding
commit are being introduced here, i.e. ones where the local "argv" was
being modified after the assignment to "struct child_process"'s
"argv".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25 22:15:07 -08:00
Robin Jarry d34182b9e3 receive-pack: ignore SIGPIPE while reporting status to client
Before running the post-receive hook, status info is reported back to
the client. If a remote client exits before or during the status report,
receive-pack is killed by SIGPIPE and post-receive is never executed.

The post-receive hook is often used to send email notifications (see
contrib/hooks/post-receive-email), update bug trackers, start automatic
builds, etc. Not executing it after an interrupted yet "successful" push
can lead to inconsistencies.

Ignore SIGPIPE before reporting status to the client to increase the
chances of post-receive running if pre-receive was successful. This does
not guarantee 100% consistency but it should resist early disconnection
by the client.

Signed-off-by: Robin Jarry <robin@jarry.cc>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-10 13:43:04 -08:00
Junio C Hamano 18c6653da0 Merge branch 'fs/ssh-signing'
Use ssh public crypto for object and push-cert signing.

* fs/ssh-signing:
  ssh signing: test that gpg fails for unknown keys
  ssh signing: tests for logs, tags & push certs
  ssh signing: duplicate t7510 tests for commits
  ssh signing: verify signatures using ssh-keygen
  ssh signing: provide a textual signing_key_id
  ssh signing: retrieve a default key from ssh-agent
  ssh signing: add ssh key format and signing code
  ssh signing: add test prereqs
  ssh signing: preliminary refactoring and clean-up
2021-10-25 16:06:58 -07:00
Ævar Arnfjörð Bjarmason 07a348e746 hook.c users: use "hook_exists()" instead of "find_hook()"
Use the new hook_exists() function instead of find_hook() where the
latter was called in boolean contexts. This make subsequent changes in
a series where we further refactor the hook API clearer, as we won't
conflate wanting to get the path of the hook with checking for its
existence.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 09:44:54 -07:00
Ævar Arnfjörð Bjarmason 5e3aba33da hook.[ch]: move find_hook() from run-command.c to hook.c
Move the find_hook() function from run-command.c to a new hook.c
library. This change establishes a stub library that's pretty
pointless right now, but will see much wider use with Emily Shaffer's
upcoming "configuration-based hooks" series.

Eventually all the hook related code will live in hook.[ch]. Let's
start that process by moving the simple find_hook() function over
as-is.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27 09:44:54 -07:00
Junio C Hamano c042ad5ad5 Merge branch 'js/run-command-close-packs'
The run-command API has been updated so that the callers can easily
ask the file descriptors open for packfiles to be closed immediately
before spawning commands that may trigger auto-gc.

* js/run-command-close-packs:
  Close object store closer to spawning child processes
  run_auto_maintenance(): implicitly close the object store
  run-command: offer to close the object store before running
  run-command: prettify the `RUN_COMMAND_*` flags
  pull: release packs before fetching
  commit-graph: when closing the graph, also release the slab
2021-09-20 15:20:45 -07:00
Junio C Hamano 5331af2352 Merge branch 'ab/serve-cleanup'
Code clean-up around "git serve".

* ab/serve-cleanup:
  upload-pack: document and rename --advertise-refs
  serve.[ch]: remove "serve_options", split up --advertise-refs code
  {upload,receive}-pack tests: add --advertise-refs tests
  serve.c: move version line to advertise_capabilities()
  serve: move transfer.advertiseSID check into session_id_advertise()
  serve.[ch]: don't pass "struct strvec *keys" to commands
  serve: use designated initializers
  transport: use designated initializers
  transport: rename "fetch" in transport_vtable to "fetch_refs"
  serve: mark has_capability() as static
2021-09-20 15:20:43 -07:00
Fabian Stelzer facca53ac3 ssh signing: verify signatures using ssh-keygen
To verify a ssh signature we first call ssh-keygen -Y find-principal to
look up the signing principal by their public key from the
allowedSignersFile. If the key is found then we do a verify. Otherwise
we only validate the signature but can not verify the signers identity.

Verification uses the gpg.ssh.allowedSignersFile (see ssh-keygen(1) "ALLOWED
SIGNERS") which contains valid public keys and a principal (usually
user@domain). Depending on the environment this file can be managed by
the individual developer or for example generated by the central
repository server from known ssh keys with push access. This file is usually
stored outside the repository, but if the repository only allows signed
commits/pushes, the user might choose to store it in the repository.

To revoke a key put the public key without the principal prefix into
gpg.ssh.revocationKeyring or generate a KRL (see ssh-keygen(1)
"KEY REVOCATION LISTS"). The same considerations about who to trust for
verification as with the allowedSignersFile apply.

Using SSH CA Keys with these files is also possible. Add
"cert-authority" as key option between the principal and the key to mark
it as a CA and all keys signed by it as valid for this CA.
See "CERTIFICATES" in ssh-keygen(1).

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-10 14:15:52 -07:00
Johannes Schindelin c4dee2c085 Close object store closer to spawning child processes
In many cases where we spawned child processes that _may_ trigger a
repack, we explicitly closed the object store first (so that the
`repack` process can delete the `.pack` files, which would otherwise not
be possible on Windows since files cannot be deleted as long as they as
still in use).

Wherever possible, we now use the new `close_object_store` bit of the
`run_command()` API, to delay closing the object store even further.
This makes the code easier to maintain because it is now more obvious
that we only release those file handles because of those child
processes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-09 12:56:11 -07:00
Patrick Steinhardt 9fec7b2130 connected: refactor iterator to return next object ID directly
The object ID iterator used by the connectivity checks returns the next
object ID via an out-parameter and then uses a return code to indicate
whether an item was found. This is a bit roundabout: instead of a
separate error code, we can just return the next object ID directly and
use `NULL` pointers as indicator that the iterator got no items left.
Furthermore, this avoids a copy of the object ID.

Refactor the iterator and all its implementations to return object IDs
directly. This brings a tiny performance improvement when doing a mirror-fetch of a repository with about 2.3M refs:

    Benchmark #1: 328dc58b49919c43897240f2eabfa30be2ce32a4~: git-fetch
      Time (mean ± σ):     30.110 s ±  0.148 s    [User: 27.161 s, System: 5.075 s]
      Range (min … max):   29.934 s … 30.406 s    10 runs

    Benchmark #2: 328dc58b49919c43897240f2eabfa30be2ce32a4: git-fetch
      Time (mean ± σ):     29.899 s ±  0.109 s    [User: 26.916 s, System: 5.104 s]
      Range (min … max):   29.696 s … 29.996 s    10 runs

    Summary
      '328dc58b49919c43897240f2eabfa30be2ce32a4: git-fetch' ran
        1.01 ± 0.01 times faster than '328dc58b49919c43897240f2eabfa30be2ce32a4~: git-fetch'

While this 1% speedup could be labelled as statistically insignificant,
the speedup is consistent on my machine. Furthermore, this is an end to
end test, so it is expected that the improvement in the connectivity
check itself is more significant.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-01 12:43:56 -07:00
Ævar Arnfjörð Bjarmason 98e2d9d6f7 upload-pack: document and rename --advertise-refs
The --advertise-refs documentation in git-upload-pack added in
9812f2136b (upload-pack.c: use parse-options API, 2016-05-31) hasn't
been entirely true ever since v2 support was implemented in
e52449b672 (connect: request remote refs using v2, 2018-03-15). Under
v2 we don't advertise the refs at all, but rather dump the
capabilities header.

This option has always been an obscure internal implementation detail,
it wasn't even documented for git-receive-pack. Since it has exactly
one user let's rename it to --http-backend-info-refs, which is more
accurate and points the reader in the right direction. Let's also
cross-link this from the protocol v1 and v2 documentation.

I'm retaining a hidden --advertise-refs alias in case there's any
external users of this, and making both options hidden to the bash
completion (as with most other internal-only options).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-05 08:59:37 -07:00
Ævar Arnfjörð Bjarmason 103e02c700 *.c static functions: don't forward-declare __attribute__
9cf6d3357a (Add git-index-pack utility, 2005-10-12) and
466dbc42f5 (receive-pack: Send internal errors over side-band #2,
2010-02-10) we added these static functions and forward-declared their
__attribute__((printf)).

I think this may have been to work around some compiler limitation at
the time, but in any case we have a lot of code that uses the briefer
way of declaring these that I'm using here, so if we had any such
issues with compilers we'd have seen them already.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-12 12:09:53 -07:00
brian m. carlson 14228447c9 hash: provide per-algorithm null OIDs
Up until recently, object IDs did not have an algorithm member, only a
hash.  Consequently, it was possible to share one null (all-zeros)
object ID among all hash algorithms.  Now that we're going to be
handling objects from multiple hash algorithms, it's important to make
sure that all object IDs have a correct algorithm field.

Introduce a per-algorithm null OID, and add it to struct hash_algo.
Introduce a wrapper function as well, and use it everywhere we used to
use the null_oid constant.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-27 16:31:39 +09:00
Junio C Hamano 92ccd7b752 Merge branch 'rs/calloc-array'
CALLOC_ARRAY() macro replaces many uses of xcalloc().

* rs/calloc-array:
  cocci: allow xcalloc(1, size)
  use CALLOC_ARRAY
  git-compat-util.h: drop trailing semicolon from macro definition
2021-03-19 15:25:38 -07:00
Junio C Hamano eabacfd9cb Merge branch 'jc/calloc-fix'
Code clean-up.

* jc/calloc-fix:
  xcalloc: use CALLOC_ARRAY() when applicable
2021-03-19 15:25:37 -07:00
Junio C Hamano 486f4bd183 xcalloc: use CALLOC_ARRAY() when applicable
These are for codebase before Git 2.31

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-15 17:51:10 -07:00
René Scharfe ca56dadb4b use CALLOC_ARRAY
Add and apply a semantic patch for converting code that open-codes
CALLOC_ARRAY to use it instead.  It shortens the code and infers the
element size automatically.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13 16:00:09 -08:00
Junio C Hamano 6ee353d42f Merge branch 'jt/transfer-fsck-across-packs'
The approach to "fsck" the incoming objects in "index-pack" is
attractive for performance reasons (we have them already in core,
inflated and ready to be inspected), but fundamentally cannot be
applied fully when we receive more than one pack stream, as a tree
object in one pack may refer to a blob object in another pack as
".gitmodules", when we want to inspect blobs that are used as
".gitmodules" file, for example.  Teach "index-pack" to emit
objects that must be inspected later and check them in the calling
"fetch-pack" process.

* jt/transfer-fsck-across-packs:
  fetch-pack: print and use dangling .gitmodules
  fetch-pack: with packfile URIs, use index-pack arg
  http-fetch: allow custom index-pack args
  http: allow custom index-pack args
2021-03-01 14:02:57 -08:00
Jonathan Tan 5476e1efde fetch-pack: print and use dangling .gitmodules
Teach index-pack to print dangling .gitmodules links after its "keep" or
"pack" line instead of declaring an error, and teach fetch-pack to check
such lines printed.

This allows the tree side of the .gitmodules link to be in one packfile
and the blob side to be in another without failing the fsck check,
because it is now fetch-pack which checks such objects after all
packfiles have been downloaded and indexed (and not index-pack on an
individual packfile, as it is before this commit).

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-22 12:07:40 -08:00
brian m. carlson 482c119186 gpg-interface: improve interface for parsing tags
We have a function which parses a buffer with a signature at the end,
parse_signature, and this function is used for signed tags.  However,
we'll need to store values for multiple algorithms, and we'll do this by
using a header for the non-default algorithm.

Adjust the parse_signature interface to store the parsed data in two
strbufs and turn the existing function into parse_signed_buffer.  The
latter is still used in places where we know we always have a signed
buffer, such as push certs.

Adjust all the callers to deal with this new interface.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10 23:35:42 -08:00
Junio C Hamano 01b8886a62 Merge branch 'js/trace2-session-id'
The transport layer was taught to optionally exchange the session
ID assigned by the trace2 subsystem during fetch/push transactions.

* js/trace2-session-id:
  receive-pack: log received client session ID
  send-pack: advertise session ID in capabilities
  upload-pack, serve: log received client session ID
  fetch-pack: advertise session ID in capabilities
  transport: log received server session ID
  serve: advertise session ID in v2 capabilities
  receive-pack: advertise session ID in v0 capabilities
  upload-pack: advertise session ID in v0 capabilities
  trace2: add a public function for getting the SID
  docs: new transfer.advertiseSID option
  docs: new capability to advertise session IDs
2020-12-08 15:11:20 -08:00
Josh Steadmon a2a066d96a receive-pack: log received client session ID
When receive-pack receives a session-id capability from the client, log
the received session ID via a trace2 data event.

Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11 18:26:53 -08:00
Josh Steadmon 8073d75bbf receive-pack: advertise session ID in v0 capabilities
When transfer.advertiseSID is true, advertise receive-pack's session ID
via the new session-id capability.

Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11 18:26:53 -08:00
Jiang Xin 80ffeb94f4 receive-pack: use default version 0 for proc-receive
In the verison negotiation phase between "receive-pack" and
"proc-receive", "proc-receive" can send an empty flush-pkt to end the
negotiation and use default version 0. Capabilities (such as
"push-options") are not supported in version 0.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11 12:46:56 -08:00
Jiang Xin f65003b4c4 receive-pack: gently write messages to proc-receive
Johannes found a flaky hang in `t5411/test-0013-bad-protocol.sh` in the
osx-clang job of the CI/PR builds, and ran into an issue when using
the `--stress` option with the following error messages:

    fatal: unable to write flush packet: Broken pipe
    send-pack: unexpected disconnect while reading sideband packet
    fatal: the remote end hung up unexpectedly

In this test case, the "proc-receive" hook sends an error message and
dies earlier. While "receive-pack" on the other side of the pipe
should forward the error message of the "proc-receive" hook to the
client side, but it fails to do so. This is because "receive-pack"
uses `packet_write_fmt()` and `packet_flush()` to write pkt-line
message to "proc-receive" hook, and these functions die immediately
when pipe is broken. Using "gently" forms for these functions will get
more predicable output.

Add more "--die-*" options to test helper to test different stages of
the protocol between "receive-pack" and "proc-receive" hook.

Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-11 12:46:56 -08:00
Junio C Hamano 6c430a647c Merge branch 'jx/proc-receive-hook'
"git receive-pack" that accepts requests by "git push" learned to
outsource most of the ref updates to the new "proc-receive" hook.

* jx/proc-receive-hook:
  doc: add documentation for the proc-receive hook
  transport: parse report options for tracking refs
  t5411: test updates of remote-tracking branches
  receive-pack: new config receive.procReceiveRefs
  doc: add document for capability report-status-v2
  New capability "report-status-v2" for git-push
  receive-pack: feed report options to post-receive
  receive-pack: add new proc-receive hook
  t5411: add basic test cases for proc-receive hook
  transport: not report a non-head push as a branch
2020-09-25 15:25:39 -07:00