Skip to content

Commit

Permalink
Merge branch 'ps/receive-use-only-advertised'
Browse files Browse the repository at this point in the history
"git receive-pack" used to use all the local refs as the boundary for
checking connectivity of the data "git push" sent, but now it uses
only the refs that it advertised to the pusher. In a repository with
the .hideRefs configuration, this reduces the resources needed to
perform the check.
cf. <221028.86bkpw805n.gmgdl@evledraar.gmail.com>
cf. <xmqqr0yrizqm.fsf@gitster.g>

* ps/receive-use-only-advertised:
  receive-pack: only use visible refs for connectivity check
  rev-parse: add `--exclude-hidden=` option
  revision: add new parameter to exclude hidden refs
  revision: introduce struct to handle exclusions
  revision: move together exclusion-related functions
  refs: get rid of global list of hidden refs
  refs: fix memory leak when parsing hideRefs config
  • Loading branch information
gitster committed Nov 23, 2022
2 parents 173fc54 + bcec678 commit f8828f9
Show file tree
Hide file tree
Showing 15 changed files with 410 additions and 82 deletions.
7 changes: 7 additions & 0 deletions Documentation/git-rev-parse.txt
Expand Up @@ -197,6 +197,13 @@ respectively, and they must begin with `refs/` when applied to `--glob`
or `--all`. If a trailing '/{asterisk}' is intended, it must be given
explicitly.

--exclude-hidden=[receive|uploadpack]::
Do not include refs that would be hidden by `git-receive-pack` or
`git-upload-pack` by consulting the appropriate `receive.hideRefs` or
`uploadpack.hideRefs` configuration along with `transfer.hideRefs` (see
linkgit:git-config[1]). This option affects the next pseudo-ref option
`--all` or `--glob` and is cleared after processing them.

--disambiguate=<prefix>::
Show every object whose name begins with the given prefix.
The <prefix> must be at least 4 hexadecimal digits long to
Expand Down
7 changes: 7 additions & 0 deletions Documentation/rev-list-options.txt
Expand Up @@ -195,6 +195,13 @@ respectively, and they must begin with `refs/` when applied to `--glob`
or `--all`. If a trailing '/{asterisk}' is intended, it must be given
explicitly.

--exclude-hidden=[receive|uploadpack]::
Do not include refs that would be hidden by `git-receive-pack` or
`git-upload-pack` by consulting the appropriate `receive.hideRefs` or
`uploadpack.hideRefs` configuration along with `transfer.hideRefs` (see
linkgit:git-config[1]). This option affects the next pseudo-ref option
`--all` or `--glob` and is cleared after processing them.

--reflog::
Pretend as if all objects mentioned by reflogs are listed on the
command line as `<commit>`.
Expand Down
10 changes: 7 additions & 3 deletions builtin/receive-pack.c
Expand Up @@ -80,6 +80,7 @@ static struct object_id push_cert_oid;
static struct signature_check sigcheck;
static const char *push_cert_nonce;
static const char *cert_nonce_seed;
static struct string_list hidden_refs = STRING_LIST_INIT_DUP;

static const char *NONCE_UNSOLICITED = "UNSOLICITED";
static const char *NONCE_BAD = "BAD";
Expand Down Expand Up @@ -130,7 +131,7 @@ static enum deny_action parse_deny_action(const char *var, const char *value)

static int receive_pack_config(const char *var, const char *value, void *cb)
{
int status = parse_hide_refs_config(var, value, "receive");
int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);

if (status)
return status;
Expand Down Expand Up @@ -296,7 +297,7 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
struct oidset *seen = data;
const char *path = strip_namespace(path_full);

if (ref_is_hidden(path, path_full))
if (ref_is_hidden(path, path_full, &hidden_refs))
return 0;

/*
Expand Down Expand Up @@ -1794,7 +1795,7 @@ static void reject_updates_to_hidden(struct command *commands)
strbuf_setlen(&refname_full, prefix_len);
strbuf_addstr(&refname_full, cmd->ref_name);

if (!ref_is_hidden(cmd->ref_name, refname_full.buf))
if (!ref_is_hidden(cmd->ref_name, refname_full.buf, &hidden_refs))
continue;
if (is_null_oid(&cmd->new_oid))
cmd->error_string = "deny deleting a hidden ref";
Expand Down Expand Up @@ -1928,6 +1929,8 @@ static void execute_commands(struct command *commands,
opt.err_fd = err_fd;
opt.progress = err_fd && !quiet;
opt.env = tmp_objdir_env(tmp_objdir);
opt.exclude_hidden_refs_section = "receive";

if (check_connected(iterate_receive_command_list, &data, &opt))
set_connectivity_errors(commands, si);

Expand Down Expand Up @@ -2591,6 +2594,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
packet_flush(1);
oid_array_clear(&shallow);
oid_array_clear(&ref);
string_list_clear(&hidden_refs, 0);
free((void *)push_cert_nonce);
return 0;
}
1 change: 1 addition & 0 deletions builtin/rev-list.c
Expand Up @@ -38,6 +38,7 @@ static const char rev_list_usage[] =
" --tags\n"
" --remotes\n"
" --stdin\n"
" --exclude-hidden=[receive|uploadpack]\n"
" --quiet\n"
" ordering output:\n"
" --topo-order\n"
Expand Down
18 changes: 14 additions & 4 deletions builtin/rev-parse.c
Expand Up @@ -39,7 +39,7 @@ static int abbrev_ref_strict;
static int output_sq;

static int stuck_long;
static struct string_list *ref_excludes;
static struct ref_exclusions ref_excludes = REF_EXCLUSIONS_INIT;

/*
* Some arguments are relevant "revision" arguments,
Expand Down Expand Up @@ -198,7 +198,7 @@ static int show_default(void)
static int show_reference(const char *refname, const struct object_id *oid,
int flag UNUSED, void *cb_data UNUSED)
{
if (ref_excluded(ref_excludes, refname))
if (ref_excluded(&ref_excludes, refname))
return 0;
show_rev(NORMAL, oid, refname);
return 0;
Expand Down Expand Up @@ -585,7 +585,7 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
for_each_glob_ref_in(show_reference, pattern, prefix, NULL);
else
for_each_ref_in(prefix, show_reference, NULL);
clear_ref_exclusion(&ref_excludes);
clear_ref_exclusions(&ref_excludes);
}

enum format_type {
Expand Down Expand Up @@ -863,7 +863,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
}
if (!strcmp(arg, "--all")) {
for_each_ref(show_reference, NULL);
clear_ref_exclusion(&ref_excludes);
clear_ref_exclusions(&ref_excludes);
continue;
}
if (skip_prefix(arg, "--disambiguate=", &arg)) {
Expand All @@ -876,10 +876,14 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (opt_with_value(arg, "--branches", &arg)) {
if (ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --branches"));
handle_ref_opt(arg, "refs/heads/");
continue;
}
if (opt_with_value(arg, "--tags", &arg)) {
if (ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --tags"));
handle_ref_opt(arg, "refs/tags/");
continue;
}
Expand All @@ -888,13 +892,19 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
continue;
}
if (opt_with_value(arg, "--remotes", &arg)) {
if (ref_excludes.hidden_refs_configured)
return error(_("--exclude-hidden cannot be used together with --remotes"));
handle_ref_opt(arg, "refs/remotes/");
continue;
}
if (skip_prefix(arg, "--exclude=", &arg)) {
add_ref_exclusion(&ref_excludes, arg);
continue;
}
if (skip_prefix(arg, "--exclude-hidden=", &arg)) {
exclude_hidden_refs(&ref_excludes, arg);
continue;
}
if (!strcmp(arg, "--show-toplevel")) {
const char *work_tree = get_git_work_tree();
if (work_tree)
Expand Down
3 changes: 3 additions & 0 deletions connected.c
Expand Up @@ -100,6 +100,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
strvec_push(&rev_list.args, "--exclude-promisor-objects");
if (!opt->is_deepening_fetch) {
strvec_push(&rev_list.args, "--not");
if (opt->exclude_hidden_refs_section)
strvec_pushf(&rev_list.args, "--exclude-hidden=%s",
opt->exclude_hidden_refs_section);
strvec_push(&rev_list.args, "--all");
}
strvec_push(&rev_list.args, "--quiet");
Expand Down
7 changes: 7 additions & 0 deletions connected.h
Expand Up @@ -46,6 +46,13 @@ struct check_connected_options {
* during a fetch.
*/
unsigned is_deepening_fetch : 1;

/*
* If not NULL, use `--exclude-hidden=$section` to exclude all refs
* hidden via the `$section.hideRefs` config from the set of
* already-reachable refs.
*/
const char *exclude_hidden_refs_section;
};

#define CHECK_CONNECTED_INIT { 0 }
Expand Down
13 changes: 9 additions & 4 deletions ls-refs.c
Expand Up @@ -6,6 +6,7 @@
#include "ls-refs.h"
#include "pkt-line.h"
#include "config.h"
#include "string-list.h"

static int config_read;
static int advertise_unborn;
Expand Down Expand Up @@ -73,6 +74,7 @@ struct ls_refs_data {
unsigned symrefs;
struct strvec prefixes;
struct strbuf buf;
struct string_list hidden_refs;
unsigned unborn : 1;
};

Expand All @@ -84,7 +86,7 @@ static int send_ref(const char *refname, const struct object_id *oid,

strbuf_reset(&data->buf);

if (ref_is_hidden(refname_nons, refname))
if (ref_is_hidden(refname_nons, refname, &data->hidden_refs))
return 0;

if (!ref_match(&data->prefixes, refname_nons))
Expand Down Expand Up @@ -137,14 +139,15 @@ static void send_possibly_unborn_head(struct ls_refs_data *data)
}

static int ls_refs_config(const char *var, const char *value,
void *data UNUSED)
void *cb_data)
{
struct ls_refs_data *data = cb_data;
/*
* We only serve fetches over v2 for now, so respect only "uploadpack"
* config. This may need to eventually be expanded to "receive", but we
* don't yet know how that information will be passed to ls-refs.
*/
return parse_hide_refs_config(var, value, "uploadpack");
return parse_hide_refs_config(var, value, "uploadpack", &data->hidden_refs);
}

int ls_refs(struct repository *r, struct packet_reader *request)
Expand All @@ -154,9 +157,10 @@ int ls_refs(struct repository *r, struct packet_reader *request)
memset(&data, 0, sizeof(data));
strvec_init(&data.prefixes);
strbuf_init(&data.buf, 0);
string_list_init_dup(&data.hidden_refs);

ensure_config_read();
git_config(ls_refs_config, NULL);
git_config(ls_refs_config, &data);

while (packet_reader_read(request) == PACKET_READ_NORMAL) {
const char *arg = request->line;
Expand Down Expand Up @@ -195,6 +199,7 @@ int ls_refs(struct repository *r, struct packet_reader *request)
packet_fflush(stdout);
strvec_clear(&data.prefixes);
strbuf_release(&data.buf);
string_list_clear(&data.hidden_refs, 0);
return 0;
}

Expand Down
16 changes: 5 additions & 11 deletions refs.c
Expand Up @@ -1414,9 +1414,8 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
refname, strict);
}

static struct string_list *hide_refs;

int parse_hide_refs_config(const char *var, const char *value, const char *section)
int parse_hide_refs_config(const char *var, const char *value, const char *section,
struct string_list *hide_refs)
{
const char *key;
if (!strcmp("transfer.hiderefs", var) ||
Expand All @@ -1431,21 +1430,16 @@ int parse_hide_refs_config(const char *var, const char *value, const char *secti
len = strlen(ref);
while (len && ref[len - 1] == '/')
ref[--len] = '\0';
if (!hide_refs) {
CALLOC_ARRAY(hide_refs, 1);
hide_refs->strdup_strings = 1;
}
string_list_append(hide_refs, ref);
string_list_append_nodup(hide_refs, ref);
}
return 0;
}

int ref_is_hidden(const char *refname, const char *refname_full)
int ref_is_hidden(const char *refname, const char *refname_full,
const struct string_list *hide_refs)
{
int i;

if (!hide_refs)
return 0;
for (i = hide_refs->nr - 1; i >= 0; i--) {
const char *match = hide_refs->items[i].string;
const char *subject;
Expand Down
5 changes: 3 additions & 2 deletions refs.h
Expand Up @@ -808,7 +808,8 @@ int update_ref(const char *msg, const char *refname,
const struct object_id *new_oid, const struct object_id *old_oid,
unsigned int flags, enum action_on_err onerr);

int parse_hide_refs_config(const char *var, const char *value, const char *);
int parse_hide_refs_config(const char *var, const char *value, const char *,
struct string_list *);

/*
* Check whether a ref is hidden. If no namespace is set, both the first and
Expand All @@ -818,7 +819,7 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
* the ref is outside that namespace, the first parameter is NULL. The second
* parameter always points to the full ref name.
*/
int ref_is_hidden(const char *, const char *);
int ref_is_hidden(const char *, const char *, const struct string_list *);

/* Is this a per-worktree ref living in the refs/ namespace? */
int is_per_worktree_ref(const char *refname);
Expand Down

0 comments on commit f8828f9

Please sign in to comment.