* Re: [PATCH 1/2] builtin/rebase.c: Emit warning when rebasing without a forkpoint
From: Junio C Hamano @ 2023-08-31 21:52 UTC (permalink / raw)
To: Wesley Schwengle; +Cc: git
In-Reply-To: <xmqqbkenszfa.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> I am not commenting on the tests, as the above code probably needs
> to be corrected first so that folks who want to squelch the message
> and want the "forkpoint behaviour by default when rebuilding on the
> usual upstream" behaviour can do so by setting the variable to true.
>
> And that obviously need to be tested, too.
Another worrysome thing about rebase.forkpoint is that it will be
inevitable for folks to start complaining that it does not work the
way other configuration variables do. Setting the variable to
'true' is not the same as passing '--fork-point=true' from the
command line.
I actually think it would be a lot larger behaviour change with a
huge potential to be received as a regression if we start making the
variable to mean the same thing as passing '--fork-point=true'.
People may like the current "if you are rebuilding your branch on
its usual upstream, pay attention to the rebase and rewind of the
upstream itself, but if you are giving an explicit upstream from the
command line, the tool does not second guess you with the fork-point
heuristics" behaviour and prefer to set it to true. We would be
breaking them big time if suddenly the rebase.forkpoint=true they
set previously starts triggering the fork-point heuristics when they
run "git rebase upstream". So that needs to be kept in mind when/if
we fix the "setting the variable, even to 'true', will squelch the
warning".
^ permalink raw reply
* Re: [PATCH v4] format-patch: --rfc honors what --subject-prefix sets
From: Jeff King @ 2023-08-31 21:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Drew DeVault, git
In-Reply-To: <xmqqsf808h4g.fsf@gitster.g>
On Wed, Aug 30, 2023 at 12:28:15PM -0700, Junio C Hamano wrote:
> Will queue. Let's wait to see if others find something fishy for a
> day or two and then merge it down to 'next'.
It looks good to me, and I'm much happier with where the refactoring
ended up compared to the earlier versions. I did have two nits, but I'm
content if neither is addressed.
One is that the commit message doesn't really describe the refactoring
of --subject-prefix. I'm OK with that rationale being in the list
archive, though.
> > static int subject_prefix_callback(const struct option *opt, const char *arg,
> > int unset)
> > {
> > + struct strbuf *sprefix;
> > +
> > BUG_ON_OPT_NEG(unset);
> > + sprefix = (struct strbuf *)opt->value;
> > subject_prefix = 1;
> > - ((struct rev_info *)opt->value)->subject_prefix = arg;
> > + strbuf_reset(sprefix);
> > + strbuf_addstr(sprefix, arg);
> > return 0;
> > }
>
> OK.
The cast is unnecessary here, since opt->value is a void pointer which
allows implicit casts. Just:
struct strbuf *sprefix = opt->value;
is IMHO a little more readable. But as we're just passing it along to
strbuf functions anyway, it would also work to do:
strbuf_reset(opt->value);
strbuf_addstr(opt->value, arg);
I think we're deep into questions of style / preference here, so I'm OK
with any of them. It's probably only that I've recently been refactoring
so many parseopt callbacks with the same pattern that I have opinions at
all. ;)
-Peff
^ permalink raw reply
* [PATCH v2 10/10] parse-options: mark unused parameters in noop callback
From: Jeff King @ 2023-08-31 21:22 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
Unsurprisingly, the noop options callback doesn't bother to look at any
of its parameters. Let's mark them so that -Wunused-parameter does not
complain.
Another option would be to drop the callback and have parse-options
itself recognize OPT_NOOP_NOARG. But that seems like extra work for no
real benefit.
Signed-off-by: Jeff King <peff@peff.net>
---
parse-options-cb.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/parse-options-cb.c b/parse-options-cb.c
index a24521dee0..bdc7fae497 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -227,7 +227,9 @@ int parse_opt_strvec(const struct option *opt, const char *arg, int unset)
return 0;
}
-int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
+int parse_opt_noop_cb(const struct option *opt UNUSED,
+ const char *arg UNUSED,
+ int unset UNUSED)
{
return 0;
}
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 09/10] interpret-trailers: mark unused "unset" parameters in option callbacks
From: Jeff King @ 2023-08-31 21:22 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
There are a few parse-option callbacks that do not look at their "unset"
parameters, but also do not set PARSE_OPT_NONEG. At first glance this
seems like a bug, as we'd ignore "--no-if-exists", etc.
But they do work fine, because when "unset" is true, then "arg" is NULL.
And all three functions pass "arg" on to helper functions which do the
right thing with the NULL.
Note that this shortcut would not be correct if any callback used
PARSE_OPT_NOARG (in which case "arg" would be NULL but "unset" would be
false). But none of these do.
So the code is fine as-is. But we'll want to mark the unused "unset"
parameters to quiet -Wunused-parameter. I've also added a comment to
make this rather subtle situation more explicit.
Signed-off-by: Jeff King <peff@peff.net>
---
I looked at another BUG_ON here, but it was just so ugly and one-off
that I preferred just adding the comments.
builtin/interpret-trailers.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 6aadce6a1e..a110e69f83 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -24,20 +24,23 @@ static enum trailer_if_exists if_exists;
static enum trailer_if_missing if_missing;
static int option_parse_where(const struct option *opt,
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
+ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_where(opt->value, arg);
}
static int option_parse_if_exists(const struct option *opt,
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
+ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_if_exists(opt->value, arg);
}
static int option_parse_if_missing(const struct option *opt,
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
+ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_if_missing(opt->value, arg);
}
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 08/10] parse-options: add more BUG_ON() annotations
From: Jeff King @ 2023-08-31 21:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
These callbacks are similar to the ones touched by 517fe807d6 (assert
NOARG/NONEG behavior of parse-options callbacks, 2018-11-05), but were
either missed in that commit (the one in add.c) or were added later (the
one in log.c).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/add.c | 2 ++
builtin/log.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/builtin/add.c b/builtin/add.c
index 4b0dd798df..cf59108523 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -232,6 +232,8 @@ static char *chmod_arg;
static int ignore_removal_cb(const struct option *opt, const char *arg, int unset)
{
+ BUG_ON_OPT_ARG(arg);
+
/* if we are told to ignore, we are not adding removals */
*(int *)opt->value = !unset ? 0 : 1;
return 0;
diff --git a/builtin/log.c b/builtin/log.c
index 3599063554..190e1952e9 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -121,6 +121,8 @@ static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
static int clear_decorations_callback(const struct option *opt UNUSED,
const char *arg, int unset)
{
+ BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
string_list_clear(&decorate_refs_include, 0);
string_list_clear(&decorate_refs_exclude, 0);
use_default_decoration_filter = 0;
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 07/10] merge: do not pass unused opt->value parameter
From: Jeff King @ 2023-08-31 21:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
The option_parse_strategy() callback does not look at opt->value;
instead it calls append_strategy(), which manipulates the global
use_strategies array directly. But the OPT_CALLBACK declaration assigns
"&use_strategies" to opt->value.
One could argue this is good, as it tells the reader what we generally
expect the callback to do. But it is also bad, because it can mislead
you into thinking that swapping out "&use_strategies" there might have
any effect. Let's switch it to pass NULL (which is what every other
"does not bother to look at opt->value" callback does). If you want to
know what the callback does, it's easy to read the function itself.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/merge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 0436986dab..545da0c8a1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -264,7 +264,7 @@ static struct option builtin_merge_options[] = {
OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
OPT_BOOL(0, "verify-signatures", &verify_signatures,
N_("verify that the named commit has a valid GPG signature")),
- OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
+ OPT_CALLBACK('s', "strategy", NULL, N_("strategy"),
N_("merge strategy to use"), option_parse_strategy),
OPT_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
N_("option for selected merge strategy")),
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 06/10] parse-options: mark unused "opt" parameter in callbacks
From: Jeff King @ 2023-08-31 21:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
The previous commit argued that parse-options callbacks should try to
use opt->value rather than touching globals directly. In some cases,
however, that's awkward to do. Some callbacks touch multiple variables,
or may even just call into an abstracted function that does so.
In some of these cases we _could_ convert them by stuffing the multiple
variables into a single struct and passing the struct pointer through
opt->value. But that may make other parts of the code less readable,
as the struct relationship has to be mentioned everywhere.
Let's just accept that these cases are special and leave them as-is. But
we do need to mark their "opt" parameters to satisfy -Wunused-parameter.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/gc.c | 2 +-
builtin/log.c | 10 ++++++----
builtin/merge.c | 2 +-
builtin/pack-objects.c | 6 +++---
builtin/read-tree.c | 2 +-
builtin/update-index.c | 4 ++--
6 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 369bd43fb2..b842349d86 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1403,7 +1403,7 @@ static void initialize_task_config(int schedule)
strbuf_release(&config_name);
}
-static int task_option_parse(const struct option *opt,
+static int task_option_parse(const struct option *opt UNUSED,
const char *arg, int unset)
{
int i, num_selected = 0;
diff --git a/builtin/log.c b/builtin/log.c
index fb90d43717..3599063554 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -118,16 +118,17 @@ static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
-static int clear_decorations_callback(const struct option *opt,
- const char *arg, int unset)
+static int clear_decorations_callback(const struct option *opt UNUSED,
+ const char *arg, int unset)
{
string_list_clear(&decorate_refs_include, 0);
string_list_clear(&decorate_refs_exclude, 0);
use_default_decoration_filter = 0;
return 0;
}
-static int decorate_callback(const struct option *opt, const char *arg, int unset)
+static int decorate_callback(const struct option *opt UNUSED, const char *arg,
+ int unset)
{
if (unset)
decoration_style = 0;
@@ -1555,7 +1556,8 @@ static int inline_callback(const struct option *opt, const char *arg, int unset)
return 0;
}
-static int header_callback(const struct option *opt, const char *arg, int unset)
+static int header_callback(const struct option *opt UNUSED, const char *arg,
+ int unset)
{
if (unset) {
string_list_clear(&extra_hdr, 0);
diff --git a/builtin/merge.c b/builtin/merge.c
index 21363b7985..0436986dab 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -231,7 +231,7 @@ static void append_strategy(struct strategy *s)
use_strategies[use_strategies_nr++] = s;
}
-static int option_parse_strategy(const struct option *opt,
+static int option_parse_strategy(const struct option *opt UNUSED,
const char *name, int unset)
{
if (unset)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 492372ee5d..91b4b7c177 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3739,7 +3739,7 @@ static void show_object__ma_allow_promisor(struct object *obj, const char *name,
show_object(obj, name, data);
}
-static int option_parse_missing_action(const struct option *opt,
+static int option_parse_missing_action(const struct option *opt UNUSED,
const char *arg, int unset)
{
assert(arg);
@@ -4150,7 +4150,7 @@ static int option_parse_index_version(const struct option *opt,
return 0;
}
-static int option_parse_unpack_unreachable(const struct option *opt,
+static int option_parse_unpack_unreachable(const struct option *opt UNUSED,
const char *arg, int unset)
{
if (unset) {
@@ -4165,7 +4165,7 @@ static int option_parse_unpack_unreachable(const struct option *opt,
return 0;
}
-static int option_parse_cruft_expiration(const struct option *opt,
+static int option_parse_cruft_expiration(const struct option *opt UNUSED,
const char *arg, int unset)
{
if (unset) {
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 1fec702a04..8196ca9dd8 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -49,7 +49,7 @@ static const char * const read_tree_usage[] = {
NULL
};
-static int index_output_cb(const struct option *opt, const char *arg,
+static int index_output_cb(const struct option *opt UNUSED, const char *arg,
int unset)
{
BUG_ON_OPT_NEG(unset);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index aee3cb8cbd..59acae3336 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -856,7 +856,7 @@ static int chmod_callback(const struct option *opt,
return 0;
}
-static int resolve_undo_clear_callback(const struct option *opt,
+static int resolve_undo_clear_callback(const struct option *opt UNUSED,
const char *arg, int unset)
{
BUG_ON_OPT_NEG(unset);
@@ -890,7 +890,7 @@ static int parse_new_style_cacheinfo(const char *arg,
}
static enum parse_opt_result cacheinfo_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt,
+ struct parse_opt_ctx_t *ctx, const struct option *opt UNUSED,
const char *arg, int unset)
{
struct object_id oid;
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 05/10] parse-options: prefer opt->value to globals in callbacks
From: Jeff King @ 2023-08-31 21:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
We have several parse-options callbacks that ignore their "opt"
parameters entirely. This is a little unusual, as we'd normally put the
result of the parsing into opt->value. In the case of these callbacks,
though, they directly manipulate global variables instead (and in
most cases the caller sets opt->value to NULL in the OPT_CALLBACK
declaration).
The immediate symptom we'd like to deal with is that the unused "opt"
variables trigger -Wunused-parameter. But how to fix that is debatable.
One option is to annotate them with UNUSED. But another is to have the
caller pass in the appropriate variable via opt->value, and use it. That
has the benefit of making the callbacks reusable (in theory at least),
and makes it clear from the OPT_CALLBACK declaration which variables
will be affected (doubly so for the cases in builtin/fast-export.c,
where we do set opt->value, but it is completely ignored!).
The slight downside is that we lose type safety, since they're now
passing through void pointers.
I went with the "just use them" approach here. The loss of type safety
is unfortunate, but that is already an issue with most of the other
callbacks. If we want to try to address that, we should do so more
consistently (and this patch would prepare these callbacks for whatever
we choose to do there).
Note that in the cases in builtin/fast-export.c, we are passing
anonymous enums. We'll have to give them names so that we can declare
the appropriate pointer type within the callbacks.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/checkout-index.c | 8 +++++---
builtin/describe.c | 6 ++++--
builtin/fast-export.c | 36 +++++++++++++++++++++---------------
builtin/fetch.c | 4 ++--
builtin/interpret-trailers.c | 12 ++++++------
builtin/pack-objects.c | 21 ++++++++++++---------
6 files changed, 50 insertions(+), 37 deletions(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 6687a495ff..6ef6ac4c2e 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -193,14 +193,16 @@ static const char * const builtin_checkout_index_usage[] = {
static int option_parse_stage(const struct option *opt,
const char *arg, int unset)
{
+ int *stage = opt->value;
+
BUG_ON_OPT_NEG(unset);
if (!strcmp(arg, "all")) {
- checkout_stage = CHECKOUT_ALL;
+ *stage = CHECKOUT_ALL;
} else {
int ch = arg[0];
if ('1' <= ch && ch <= '3')
- checkout_stage = arg[0] - '0';
+ *stage = arg[0] - '0';
else
die(_("stage should be between 1 and 3 or all"));
}
@@ -238,7 +240,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
N_("write the content to temporary files")),
OPT_STRING(0, "prefix", &state.base_dir, N_("string"),
N_("when creating files, prepend <string>")),
- OPT_CALLBACK_F(0, "stage", NULL, "(1|2|3|all)",
+ OPT_CALLBACK_F(0, "stage", &checkout_stage, "(1|2|3|all)",
N_("copy out the files from named stage"),
PARSE_OPT_NONEG, option_parse_stage),
OPT_END()
diff --git a/builtin/describe.c b/builtin/describe.c
index b28a4a1f82..718b5c3073 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -561,9 +561,11 @@ static void describe(const char *arg, int last_one)
static int option_parse_exact_match(const struct option *opt, const char *arg,
int unset)
{
+ int *val = opt->value;
+
BUG_ON_OPT_ARG(arg);
- max_candidates = unset ? DEFAULT_CANDIDATES : 0;
+ *val = unset ? DEFAULT_CANDIDATES : 0;
return 0;
}
@@ -578,7 +580,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "long", &longformat, N_("always use long format")),
OPT_BOOL(0, "first-parent", &first_parent, N_("only follow first parent")),
OPT__ABBREV(&abbrev),
- OPT_CALLBACK_F(0, "exact-match", NULL, NULL,
+ OPT_CALLBACK_F(0, "exact-match", &max_candidates, NULL,
N_("only output exact matches"),
PARSE_OPT_NOARG, option_parse_exact_match),
OPT_INTEGER(0, "candidates", &max_candidates,
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 56dc69fac1..70aff515ac 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -33,9 +33,9 @@ static const char *fast_export_usage[] = {
};
static int progress;
-static enum { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
-static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
-static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
+static enum signed_tag_mode { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;
+static enum tag_of_filtered_mode { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;
+static enum reencode_mode { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;
static int fake_missing_tagger;
static int use_done_feature;
static int no_data;
@@ -53,16 +53,18 @@ static struct revision_sources revision_sources;
static int parse_opt_signed_tag_mode(const struct option *opt,
const char *arg, int unset)
{
+ enum signed_tag_mode *val = opt->value;
+
if (unset || !strcmp(arg, "abort"))
- signed_tag_mode = SIGNED_TAG_ABORT;
+ *val = SIGNED_TAG_ABORT;
else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore"))
- signed_tag_mode = VERBATIM;
+ *val = VERBATIM;
else if (!strcmp(arg, "warn"))
- signed_tag_mode = WARN;
+ *val = WARN;
else if (!strcmp(arg, "warn-strip"))
- signed_tag_mode = WARN_STRIP;
+ *val = WARN_STRIP;
else if (!strcmp(arg, "strip"))
- signed_tag_mode = STRIP;
+ *val = STRIP;
else
return error("Unknown signed-tags mode: %s", arg);
return 0;
@@ -71,12 +73,14 @@ static int parse_opt_signed_tag_mode(const struct option *opt,
static int parse_opt_tag_of_filtered_mode(const struct option *opt,
const char *arg, int unset)
{
+ enum tag_of_filtered_mode *val = opt->value;
+
if (unset || !strcmp(arg, "abort"))
- tag_of_filtered_mode = TAG_FILTERING_ABORT;
+ *val = TAG_FILTERING_ABORT;
else if (!strcmp(arg, "drop"))
- tag_of_filtered_mode = DROP;
+ *val = DROP;
else if (!strcmp(arg, "rewrite"))
- tag_of_filtered_mode = REWRITE;
+ *val = REWRITE;
else
return error("Unknown tag-of-filtered mode: %s", arg);
return 0;
@@ -85,21 +89,23 @@ static int parse_opt_tag_of_filtered_mode(const struct option *opt,
static int parse_opt_reencode_mode(const struct option *opt,
const char *arg, int unset)
{
+ enum reencode_mode *val = opt->value;
+
if (unset) {
- reencode_mode = REENCODE_ABORT;
+ *val = REENCODE_ABORT;
return 0;
}
switch (git_parse_maybe_bool(arg)) {
case 0:
- reencode_mode = REENCODE_NO;
+ *val = REENCODE_NO;
break;
case 1:
- reencode_mode = REENCODE_YES;
+ *val = REENCODE_YES;
break;
default:
if (!strcasecmp(arg, "abort"))
- reencode_mode = REENCODE_ABORT;
+ *val = REENCODE_ABORT;
else
return error("Unknown reencoding mode: %s", arg);
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8f93529505..fd134ba74d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -176,7 +176,7 @@ static int parse_refmap_arg(const struct option *opt, const char *arg, int unset
* "git fetch --refmap='' origin foo"
* can be used to tell the command not to store anywhere
*/
- refspec_append(&refmap, arg);
+ refspec_append(opt->value, arg);
return 0;
}
@@ -2204,7 +2204,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
PARSE_OPT_HIDDEN, option_fetch_parse_recurse_submodules),
OPT_BOOL(0, "update-shallow", &update_shallow,
N_("accept refs that update .git/shallow")),
- OPT_CALLBACK_F(0, "refmap", NULL, N_("refmap"),
+ OPT_CALLBACK_F(0, "refmap", &refmap, N_("refmap"),
N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
OPT_IPVERSION(&family),
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index c5e8345265..6aadce6a1e 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -26,19 +26,19 @@ static enum trailer_if_missing if_missing;
static int option_parse_where(const struct option *opt,
const char *arg, int unset)
{
- return trailer_set_where(&where, arg);
+ return trailer_set_where(opt->value, arg);
}
static int option_parse_if_exists(const struct option *opt,
const char *arg, int unset)
{
- return trailer_set_if_exists(&if_exists, arg);
+ return trailer_set_if_exists(opt->value, arg);
}
static int option_parse_if_missing(const struct option *opt,
const char *arg, int unset)
{
- return trailer_set_if_missing(&if_missing, arg);
+ return trailer_set_if_missing(opt->value, arg);
}
static void new_trailers_clear(struct list_head *trailers)
@@ -97,11 +97,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")),
OPT_BOOL(0, "trim-empty", &opts.trim_empty, N_("trim empty trailers")),
- OPT_CALLBACK(0, "where", NULL, N_("action"),
+ OPT_CALLBACK(0, "where", &where, N_("action"),
N_("where to place the new trailer"), option_parse_where),
- OPT_CALLBACK(0, "if-exists", NULL, N_("action"),
+ OPT_CALLBACK(0, "if-exists", &if_exists, N_("action"),
N_("action if trailer already exists"), option_parse_if_exists),
- OPT_CALLBACK(0, "if-missing", NULL, N_("action"),
+ OPT_CALLBACK(0, "if-missing", &if_missing, N_("action"),
N_("action if trailer is missing"), option_parse_if_missing),
OPT_BOOL(0, "only-trailers", &opts.only_trailers, N_("output only the trailers")),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d2a162d528..492372ee5d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4120,29 +4120,32 @@ static void add_extra_kept_packs(const struct string_list *names)
static int option_parse_quiet(const struct option *opt, const char *arg,
int unset)
{
+ int *val = opt->value;
+
BUG_ON_OPT_ARG(arg);
if (!unset)
- progress = 0;
- else if (!progress)
- progress = 1;
+ *val = 0;
+ else if (!*val)
+ *val = 1;
return 0;
}
static int option_parse_index_version(const struct option *opt,
const char *arg, int unset)
{
+ struct pack_idx_option *popts = opt->value;
char *c;
const char *val = arg;
BUG_ON_OPT_NEG(unset);
- pack_idx_opts.version = strtoul(val, &c, 10);
- if (pack_idx_opts.version > 2)
+ popts->version = strtoul(val, &c, 10);
+ if (popts->version > 2)
die(_("unsupported index version %s"), val);
if (*c == ',' && c[1])
- pack_idx_opts.off32_limit = strtoul(c+1, &c, 0);
- if (*c || pack_idx_opts.off32_limit & 0x80000000)
+ popts->off32_limit = strtoul(c+1, &c, 0);
+ if (*c || popts->off32_limit & 0x80000000)
die(_("bad index version '%s'"), val);
return 0;
}
@@ -4190,7 +4193,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
LIST_OBJECTS_FILTER_INIT;
struct option pack_objects_options[] = {
- OPT_CALLBACK_F('q', "quiet", NULL, NULL,
+ OPT_CALLBACK_F('q', "quiet", &progress, NULL,
N_("do not show progress meter"),
PARSE_OPT_NOARG, option_parse_quiet),
OPT_SET_INT(0, "progress", &progress,
@@ -4200,7 +4203,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
OPT_BOOL(0, "all-progress-implied",
&all_progress_implied,
N_("similar to --all-progress when progress meter is shown")),
- OPT_CALLBACK_F(0, "index-version", NULL, N_("<version>[,<offset>]"),
+ OPT_CALLBACK_F(0, "index-version", &pack_idx_opts, N_("<version>[,<offset>]"),
N_("write the pack index file in the specified idx format version"),
PARSE_OPT_NONEG, option_parse_index_version),
OPT_MAGNITUDE(0, "max-pack-size", &pack_size_limit,
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 04/10] checkout-index: delay automatic setting of to_tempfile
From: Jeff King @ 2023-08-31 21:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
Using --stage=all requires writing to tempfiles, since we cannot put
multiple stages into a single file. So --stage=all implies --temp.
But we do this by setting to_tempfile in the options callback for
--stage, rather than after all options have been parsed. This leads to
two bugs:
1. If you run "checkout-index --stage=all --stage=2", this should not
imply --temp, but it currently does. The callback cannot just unset
to_tempfile when it sees the "2" value, because it no longer knows
if its value was from the earlier --stage call, or if the user
specified --temp explicitly.
2. If you run "checkout-index --stage=all --no-temp", the --no-temp
will overwrite the earlier implied --temp. But this mode of
operation cannot work, and the command will fail with "<path>
already exists" when trying to write the higher stages.
We can fix both by lazily setting to_tempfile. We'll make it a tristate,
with -1 as "not yet given", and have --stage=all enable it only after
all options are parsed. Likewise, after all options are parsed we can
detect and reject the bogus "--no-temp" case.
Note that this does technically change the behavior for "--stage=all
--no-temp" for paths which have only one stage present (which
accidentally worked before, but is now forbidden). But this behavior was
never intended, and you'd have to go out of your way to try to trigger
it.
The new tests cover both cases, as well the general "--stage=all implies
--temp", as most of the other tests explicitly say "--temp". Ironically,
the test "checkout --temp within subdir" is the only one that _doesn't_
use "--temp", and so was implicitly covering this case. But it seems
reasonable to have a more explicit test alongside the other related
ones.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
---
One other bug-let I noticed in this function: it only looks at the first
byte of the stage argument, so "--stage=13" is the same as "--stage=1".
We could tighten this, but I wonder if anybody does something weird like
"--stage=2ours" or something. That seems pretty unlikely, but I'd still
prefer to do it as a separate patch (if at all).
builtin/checkout-index.c | 8 ++++++--
t/t2004-checkout-cache-temp.sh | 20 ++++++++++++++++++++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index f62f13f2b5..6687a495ff 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -24,7 +24,7 @@
static int nul_term_line;
static int checkout_stage; /* default to checkout stage0 */
static int ignore_skip_worktree; /* default to 0 */
-static int to_tempfile;
+static int to_tempfile = -1;
static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
static struct checkout state = CHECKOUT_INIT;
@@ -196,7 +196,6 @@ static int option_parse_stage(const struct option *opt,
BUG_ON_OPT_NEG(unset);
if (!strcmp(arg, "all")) {
- to_tempfile = 1;
checkout_stage = CHECKOUT_ALL;
} else {
int ch = arg[0];
@@ -269,6 +268,11 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
state.base_dir = "";
state.base_dir_len = strlen(state.base_dir);
+ if (to_tempfile < 0)
+ to_tempfile = (checkout_stage == CHECKOUT_ALL);
+ if (!to_tempfile && checkout_stage == CHECKOUT_ALL)
+ die("--stage=all and --no-temp are incompatible");
+
/*
* when --prefix is specified we do not want to update cache.
*/
diff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh
index b16d69ca4a..8eb2ef44eb 100755
--- a/t/t2004-checkout-cache-temp.sh
+++ b/t/t2004-checkout-cache-temp.sh
@@ -117,6 +117,26 @@ test_expect_success 'checkout all stages/one file to temporary files' '
test $(cat $s3) = tree3path1)
'
+test_expect_success '--stage=all implies --temp' '
+ rm -f path* .merge_* actual &&
+ git checkout-index --stage=all -- path1 &&
+ test_path_is_missing path1
+'
+
+test_expect_success 'overriding --stage=all resets implied --temp' '
+ rm -f path* .merge_* actual &&
+ git checkout-index --stage=all --stage=2 -- path1 &&
+ echo tree2path1 >expect &&
+ test_cmp expect path1
+'
+
+test_expect_success '--stage=all --no-temp is rejected' '
+ rm -f path* .merge_* actual &&
+ test_must_fail git checkout-index --stage=all --no-temp -- path1 2>err &&
+ grep -v "already exists" err &&
+ grep "stage=all and --no-temp are incompatible" err
+'
+
test_expect_success 'checkout some stages/one file to temporary files' '
rm -f path* .merge_* actual &&
git checkout-index --stage=all --temp -- path2 >actual &&
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 03/10] format-patch: use OPT_STRING_LIST for to/cc options
From: Jeff King @ 2023-08-31 21:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
The to_callback() and cc_callback() functions are identical to the
generic parse_opt_string_list() function (except that they don't handle
optional arguments, but that's OK because their callers do not use the
OPTARG flag).
Let's simplify the code by using OPT_STRING_LIST.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/log.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index db3a88bfe9..fb90d43717 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1567,24 +1567,6 @@ static int header_callback(const struct option *opt, const char *arg, int unset)
return 0;
}
-static int to_callback(const struct option *opt, const char *arg, int unset)
-{
- if (unset)
- string_list_clear(&extra_to, 0);
- else
- string_list_append(&extra_to, arg);
- return 0;
-}
-
-static int cc_callback(const struct option *opt, const char *arg, int unset)
-{
- if (unset)
- string_list_clear(&extra_cc, 0);
- else
- string_list_append(&extra_cc, arg);
- return 0;
-}
-
static int from_callback(const struct option *opt, const char *arg, int unset)
{
char **from = opt->value;
@@ -1957,8 +1939,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
OPT_GROUP(N_("Messaging")),
OPT_CALLBACK(0, "add-header", NULL, N_("header"),
N_("add email header"), header_callback),
- OPT_CALLBACK(0, "to", NULL, N_("email"), N_("add To: header"), to_callback),
- OPT_CALLBACK(0, "cc", NULL, N_("email"), N_("add Cc: header"), cc_callback),
+ OPT_STRING_LIST(0, "to", &extra_to, N_("email"), N_("add To: header")),
+ OPT_STRING_LIST(0, "cc", &extra_cc, N_("email"), N_("add Cc: header")),
OPT_CALLBACK_F(0, "from", &from, N_("ident"),
N_("set From address to <ident> (or committer ident if absent)"),
PARSE_OPT_OPTARG, from_callback),
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 02/10] merge: simplify parsing of "-n" option
From: Jeff King @ 2023-08-31 21:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
The "-n" option is implemented by an option callback, as it is really a
"reverse bool". If given, it sets show_diffstat to 0. In theory, when
negated, it would set the same flag to 1. But it's not possible to
trigger that, since short options cannot be negated.
So in practice this is really just a SET_INT to 0. Let's use that
instead, which shortens the code.
Note that negation here would do the wrong thing (as with any SET_INT
with a value of "0"). We could specify PARSE_OPT_NONEG to future-proof
ourselves against somebody adding a long option name (which would make
it possible to negate). But there's not much point:
1. Nobody is going to do that, because the negated form already
exists, and is called "--stat" (which is defined separately so that
"--no-stat" works).
2. If they did, the BUG() check added by 3284b93862 (parse-options:
disallow negating OPTION_SET_INT 0, 2023-08-08) will catch it (and
that check is smart enough to realize that our short-only option is
OK).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/merge.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 53e9fe70d9..21363b7985 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -241,18 +241,9 @@ static int option_parse_strategy(const struct option *opt,
return 0;
}
-static int option_parse_n(const struct option *opt,
- const char *arg, int unset)
-{
- BUG_ON_OPT_ARG(arg);
- show_diffstat = unset;
- return 0;
-}
-
static struct option builtin_merge_options[] = {
- OPT_CALLBACK_F('n', NULL, NULL, NULL,
- N_("do not show a diffstat at the end of the merge"),
- PARSE_OPT_NOARG, option_parse_n),
+ OPT_SET_INT('n', NULL, &show_diffstat,
+ N_("do not show a diffstat at the end of the merge"), 0),
OPT_BOOL(0, "stat", &show_diffstat,
N_("show a diffstat at the end of the merge")),
OPT_BOOL(0, "summary", &show_diffstat, N_("(synonym to --stat)")),
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 01/10] merge: make xopts a strvec
From: Jeff King @ 2023-08-31 21:17 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831211637.GA949188@coredump.intra.peff.net>
The "xopts" variable uses a custom array with ALLOC_GROW(). Using a
strvec simplifies things a bit. We need fewer variables, and we can also
ditch our custom parseopt callback in favor of OPT_STRVEC().
As a bonus, this means that "--no-strategy-option", which was previously
a silent noop, now does something useful: like other list-like options,
it will clear the list of -X options seen so far. This matches the
behavior of revert/cherry-pick, which made the same change in fb60b9f37f
(sequencer: use struct strvec to store merge strategy options,
2023-04-10).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/merge.c | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index de68910177..53e9fe70d9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -79,8 +79,7 @@ static int overwrite_ignore = 1;
static struct strbuf merge_msg = STRBUF_INIT;
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
-static const char **xopts;
-static size_t xopts_nr, xopts_alloc;
+static struct strvec xopts = STRVEC_INIT;
static const char *branch;
static char *branch_mergeoptions;
static int verbosity;
@@ -242,17 +241,6 @@ static int option_parse_strategy(const struct option *opt,
return 0;
}
-static int option_parse_x(const struct option *opt,
- const char *arg, int unset)
-{
- if (unset)
- return 0;
-
- ALLOC_GROW(xopts, xopts_nr + 1, xopts_alloc);
- xopts[xopts_nr++] = xstrdup(arg);
- return 0;
-}
-
static int option_parse_n(const struct option *opt,
const char *arg, int unset)
{
@@ -287,8 +275,8 @@ static struct option builtin_merge_options[] = {
N_("verify that the named commit has a valid GPG signature")),
OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
N_("merge strategy to use"), option_parse_strategy),
- OPT_CALLBACK('X', "strategy-option", &xopts, N_("option=value"),
- N_("option for selected merge strategy"), option_parse_x),
+ OPT_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
+ N_("option for selected merge strategy")),
OPT_CALLBACK('m', "message", &merge_msg, N_("message"),
N_("merge commit message (for a non-fast-forward merge)"),
option_parse_message),
@@ -749,9 +737,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
o.show_rename_progress =
show_progress == -1 ? isatty(2) : show_progress;
- for (x = 0; x < xopts_nr; x++)
- if (parse_merge_opt(&o, xopts[x]))
- die(_("unknown strategy option: -X%s"), xopts[x]);
+ for (x = 0; x < xopts.nr; x++)
+ if (parse_merge_opt(&o, xopts.v[x]))
+ die(_("unknown strategy option: -X%s"), xopts.v[x]);
o.branch1 = head_arg;
o.branch2 = merge_remote_util(remoteheads->item)->name;
@@ -777,7 +765,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
return clean ? 0 : 1;
} else {
return try_merge_command(the_repository,
- strategy, xopts_nr, xopts,
+ strategy, xopts.nr, xopts.v,
common, head_arg, remoteheads);
}
}
--
2.42.0.561.gaa987ecc69
^ permalink raw reply related
* [PATCH v2 0/10] more unused parameters in parseopt callbacks
From: Jeff King @ 2023-08-31 21:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, René Scharfe
In-Reply-To: <20230831070935.GA3197495@coredump.intra.peff.net>
On Thu, Aug 31, 2023 at 03:09:35AM -0400, Jeff King wrote:
> Here are some more patches silencing -Wunused-parameter warnings. I've
> prepared them on top of the patches queued in jk/unused-post-2.42, but
> they should apply equally well directly on 'master'.
And here's a re-roll based on feedback from Junio:
[01/10]: merge: make xopts a strvec
[02/10]: merge: simplify parsing of "-n" option
[03/10]: format-patch: use OPT_STRING_LIST for to/cc options
[04/10]: checkout-index: delay automatic setting of to_tempfile
[05/10]: parse-options: prefer opt->value to globals in callbacks
[06/10]: parse-options: mark unused "opt" parameter in callbacks
[07/10]: merge: do not pass unused opt->value parameter
[08/10]: parse-options: add more BUG_ON() annotations
[09/10]: interpret-trailers: mark unused "unset" parameters in option callbacks
[10/10]: parse-options: mark unused parameters in noop callback
Range-diff is below, but the most interesting changes are in the two new
patches (3 and 4):
- patch 3 simplifies away a few string-list callbacks (which can then
be omitted from patch 6, the "mark unused opt" one).
- patch 4 fixes some minor bugs with "checkout-index --stage". In
doing so, it's now eligible for cleanup in patch 5 ("prefer
opt->value") and its annotation dropped from patch 6
- patch 5 ("prefer opt->value") converts the missed refmap callback,
and so its annotation is gone from patch 6 now
- a few extra clarifying comments
1: 8750d92016 ! 1: 82940b6936 merge: make xopts a strvec
@@ Commit message
As a bonus, this means that "--no-strategy-option", which was previously
a silent noop, now does something useful: like other list-like options,
- it will clear the list of -X options seen so far.
+ it will clear the list of -X options seen so far. This matches the
+ behavior of revert/cherry-pick, which made the same change in fb60b9f37f
+ (sequencer: use struct strvec to store merge strategy options,
+ 2023-04-10).
Signed-off-by: Jeff King <peff@peff.net>
2: ffdae69bd6 = 2: c98b87feb9 merge: simplify parsing of "-n" option
-: ---------- > 3: 615fef8f90 format-patch: use OPT_STRING_LIST for to/cc options
-: ---------- > 4: 66cf7bcf08 checkout-index: delay automatic setting of to_tempfile
3: f00c78cc49 ! 5: a7c2bfecba parse-options: prefer opt->value to globals in callbacks
@@ Commit message
Signed-off-by: Jeff King <peff@peff.net>
+ ## builtin/checkout-index.c ##
+@@ builtin/checkout-index.c: static const char * const builtin_checkout_index_usage[] = {
+ static int option_parse_stage(const struct option *opt,
+ const char *arg, int unset)
+ {
++ int *stage = opt->value;
++
+ BUG_ON_OPT_NEG(unset);
+
+ if (!strcmp(arg, "all")) {
+- checkout_stage = CHECKOUT_ALL;
++ *stage = CHECKOUT_ALL;
+ } else {
+ int ch = arg[0];
+ if ('1' <= ch && ch <= '3')
+- checkout_stage = arg[0] - '0';
++ *stage = arg[0] - '0';
+ else
+ die(_("stage should be between 1 and 3 or all"));
+ }
+@@ builtin/checkout-index.c: int cmd_checkout_index(int argc, const char **argv, const char *prefix)
+ N_("write the content to temporary files")),
+ OPT_STRING(0, "prefix", &state.base_dir, N_("string"),
+ N_("when creating files, prepend <string>")),
+- OPT_CALLBACK_F(0, "stage", NULL, "(1|2|3|all)",
++ OPT_CALLBACK_F(0, "stage", &checkout_stage, "(1|2|3|all)",
+ N_("copy out the files from named stage"),
+ PARSE_OPT_NONEG, option_parse_stage),
+ OPT_END()
+
## builtin/describe.c ##
@@ builtin/describe.c: static void describe(const char *arg, int last_one)
static int option_parse_exact_match(const struct option *opt, const char *arg,
@@ builtin/fast-export.c: static int parse_opt_tag_of_filtered_mode(const struct op
return error("Unknown reencoding mode: %s", arg);
}
+ ## builtin/fetch.c ##
+@@ builtin/fetch.c: static int parse_refmap_arg(const struct option *opt, const char *arg, int unset
+ * "git fetch --refmap='' origin foo"
+ * can be used to tell the command not to store anywhere
+ */
+- refspec_append(&refmap, arg);
++ refspec_append(opt->value, arg);
+
+ return 0;
+ }
+@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
+ PARSE_OPT_HIDDEN, option_fetch_parse_recurse_submodules),
+ OPT_BOOL(0, "update-shallow", &update_shallow,
+ N_("accept refs that update .git/shallow")),
+- OPT_CALLBACK_F(0, "refmap", NULL, N_("refmap"),
++ OPT_CALLBACK_F(0, "refmap", &refmap, N_("refmap"),
+ N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
+ OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
+ OPT_IPVERSION(&family),
+
## builtin/interpret-trailers.c ##
@@ builtin/interpret-trailers.c: static enum trailer_if_missing if_missing;
static int option_parse_where(const struct option *opt,
4: 19621fc01c ! 6: bd915fe4de parse-options: mark unused "opt" parameter in callbacks
@@ Commit message
Signed-off-by: Jeff King <peff@peff.net>
- ## builtin/checkout-index.c ##
-@@ builtin/checkout-index.c: static const char * const builtin_checkout_index_usage[] = {
- NULL
- };
-
--static int option_parse_stage(const struct option *opt,
-+static int option_parse_stage(const struct option *opt UNUSED,
- const char *arg, int unset)
- {
- BUG_ON_OPT_NEG(unset);
-
- ## builtin/fetch.c ##
-@@ builtin/fetch.c: static int git_fetch_config(const char *k, const char *v,
- return git_default_config(k, v, ctx, cb);
- }
-
--static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
-+static int parse_refmap_arg(const struct option *opt UNUSED,
-+ const char *arg, int unset)
- {
- BUG_ON_OPT_NEG(unset);
-
-
## builtin/gc.c ##
@@ builtin/gc.c: static void initialize_task_config(int schedule)
strbuf_release(&config_name);
@@ builtin/log.c: static int inline_callback(const struct option *opt, const char *
{
if (unset) {
string_list_clear(&extra_hdr, 0);
-@@ builtin/log.c: static int header_callback(const struct option *opt, const char *arg, int unset)
- return 0;
- }
-
--static int to_callback(const struct option *opt, const char *arg, int unset)
-+static int to_callback(const struct option *opt UNUSED, const char *arg,
-+ int unset)
- {
- if (unset)
- string_list_clear(&extra_to, 0);
-@@ builtin/log.c: static int to_callback(const struct option *opt, const char *arg, int unset)
- return 0;
- }
-
--static int cc_callback(const struct option *opt, const char *arg, int unset)
-+static int cc_callback(const struct option *opt UNUSED, const char *arg,
-+ int unset)
- {
- if (unset)
- string_list_clear(&extra_cc, 0);
## builtin/merge.c ##
@@ builtin/merge.c: static void append_strategy(struct strategy *s)
5: 2d6a3b9e1b = 7: 1720ddebc1 merge: do not pass unused opt->value parameter
6: 7669758cc7 = 8: a93f155f21 parse-options: add more BUG_ON() annotations
7: b80020daec ! 9: 529573e281 interpret-trailers: mark unused "unset" parameters in option callbacks
@@ Commit message
false). But none of these do.
So the code is fine as-is. But we'll want to mark the unused "unset"
- parameters to quiet -Wunused-parameter.
+ parameters to quiet -Wunused-parameter. I've also added a comment to
+ make this rather subtle situation more explicit.
Signed-off-by: Jeff King <peff@peff.net>
@@ builtin/interpret-trailers.c: static enum trailer_if_exists if_exists;
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
++ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_where(opt->value, arg);
}
static int option_parse_if_exists(const struct option *opt,
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
++ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_if_exists(opt->value, arg);
}
static int option_parse_if_missing(const struct option *opt,
- const char *arg, int unset)
+ const char *arg, int unset UNUSED)
{
++ /* unset implies NULL arg, which is handled in our helper */
return trailer_set_if_missing(opt->value, arg);
}
+
8: f21961ed23 = 10: 943161eaf2 parse-options: mark unused parameters in noop callback
^ permalink raw reply
* Re: [PATCH 10/10] lower core.maxTreeDepth default to 2048
From: Junio C Hamano @ 2023-08-31 21:06 UTC (permalink / raw)
To: Jeff King; +Cc: Oswald Buddenhagen, git
In-Reply-To: <20230831174215.GA3208283@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Aug 31, 2023 at 12:39:37PM +0200, Oswald Buddenhagen wrote:
>
>> On Thu, Aug 31, 2023 at 02:23:20AM -0400, Jeff King wrote:
>> > But I thought that
>> > following the sequence of logic (from "4096 is probably OK" to "whoops,
>> > it's not") had some value to share.
>> >
>> of course, but you can just integrate that into the squashed commit message.
>> having it all in one place makes it easier to follow.
>
> Yes, though I think having it as a separate patch makes it easier to
> revisit later (e.g., by reverting or by replacing the patch during a
> re-roll).
I am on the fence. Having it squashed into the same step as it was
introduced may reduce the patch count, but then it would not be easy
to explain why 2048 is a reasonable default at that step when no
code actually uses the variable, so the end result is not all that
easier to follow and read, as that earlier step would be handwaving
"2048 is good at the end of the series, trust me", unlike having it
at the end. When 4096 is introduced as a "random number that seems
larger than large enough" in the earlier step, it might be worth
mentioning that it is a tentative default and may turn out to be
larger than necessary in which case we may want to shrink it ;-)
^ permalink raw reply
* Re: [PATCH 1/2] builtin/rebase.c: Emit warning when rebasing without a forkpoint
From: Junio C Hamano @ 2023-08-31 20:57 UTC (permalink / raw)
To: Wesley Schwengle; +Cc: git
In-Reply-To: <20230819203528.562156-2-wesleys@opperschaap.net>
Wesley Schwengle <wesleys@opperschaap.net> writes:
> The behaviour of `git rebase' was that if you supply an upstream on the
> command line that it behaves as if `--no-forkpoint' was supplied and if
> you don't supply an upstream, it behaves as if `--forkpoint' was
> supplied.
I actually think it is a reasonable, if a bit too clever (for my
taste at least), default for those who do not want to type the
"--fork-point" option from the command line and still want to use
that option when they are pulling from or rebasing on the source
they usually interact with, while still allowing them to be precise
when they do want to specify exactly what commit they want to base
it on.
And the way how you tell if they are using the "usual" source is to
see if they used the lazy "git rebase" (without arguments) form. So
I do not think it is particularly a bad design to allow "git rebase
master" and "git rebase" to behave differently. The latter may use
the "fork point computed using 'master' branch" (when the current
branch is configured to rebuild on top of 'master') while the former
may use "exactly the commit pointed at by the 'master' branch".
> This can result in a loss of commits if you don't know that
> and if you don't know about `git reflog' or have other copies of your
> changes.
Surely, but you would lose commits if you don't know these things
and explicitly gave the --fork-point option the same way. So I am
not sure if switching of the default is warranted.
> - if (options.fork_point < 0)
> + if (options.fork_point < 0) {
> + warning(_(
> + "Rebasing without specifying a forkpoint is discouraged. You can squelch\n"
> + "this message by running one of the following commands something before your\n"
> + "next rebase:\n"
> + "\n"
> + " git config rebase.forkpoint = false # This will become the new default\n"
> + " git config rebase.forkpoint = true # This is the old default\n"
> + "\n"
The message "Rebasing without specifying a forkpoint" reads as if
you are encouraging the use of forkpoint mode (which you are not, I
know), but then what the message advertises as a future default
stops not make sense. "If we hate the forkpoint mode so much to
disable it by default, why so we discourage running the command
without specifying it?" would be the confused message the users will
read from it.
Your "git config" example command lines are not correct, are they?
There should be no '=' assignment operator.
I am also afraid that this is giving a way too broad an advice.
What you want to discourage is to rebase without specifying what to
rebase on and without saying if you want or you do not want the
forkpoint behaviour, which will opt the user into the more dangerous
forkpoint behaviour. The above makes it sound as if we will
discourage even the more precise "git rebase <newbase>" form, but I
do not think it is the case. We would and should not trigger the
folk-point behaviour if there is an explicit <upstream> and the user
does not say "--fork-point" from the command line.
Here is my attempt to rewrite the above:
When 'git rebase' is run without specifying <upstream> on the
command line, the current default is to use the fork-point
heuristics, but this is expected to change in a future version
of Git, and you will have to explicitly give "--fork-point" from
the command line if you keep using the fork-point mode. You can
run "git config rebase.forkpoint false" to adopt the new default
in advance and that will also squelch the message.
Note that the parsing of "rebase.forkpoint" is a bit peculiar in
that
- By leaving it unspecified, the .fork_point = -1 in
REBASE_OPTIONS_INIT takes effect (which is unsurprising);
- By setting it to false, .fork_point becomes 0; but
- If you set the configuration variable to true, .fork_point
becomes -1, not 1.
And this is very much deliberate if I understand it correctly [*1*].
By the time we get to this part of the code (i.e. .fork_point is
-1), the user may already have rebase.forkpoint set to true. IOW,
setting it to 'true' is not a valid way to squelch this message.
I am not commenting on the tests, as the above code probably needs
to be corrected first so that folks who want to squelch the message
and want the "forkpoint behaviour by default when rebuilding on the
usual upstream" behaviour can do so by setting the variable to true.
And that obviously need to be tested, too.
Thanks.
[References]
*1* https://lore.kernel.org/git/xmqqturbdxi2.fsf@gitster.c.googlers.com/
^ permalink raw reply
* Re: [PATCH 1/8] merge: make xopts a strvec
From: Taylor Blau @ 2023-08-31 20:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, René Scharfe
In-Reply-To: <xmqqcyz3z032.fsf@gitster.g>
On Thu, Aug 31, 2023 at 08:46:25AM -0700, Junio C Hamano wrote:
> > I guess you could argue this is a backwards-incompatible change, but the
> > existing behavior of --no-strategy-option is so dumb that I can't
> > believe somebody would prefer it (plus revert/cherry-pick already use
> > OPT_STRVEC for their matching "-X").
> >
> > I didn't bother adding a test since we're just re-using OPT_STRVEC code
> > that is used elsewhere.
>
> I do not think of any useful way to have "--no-strategy-option" on
> the command line (either as an early part of an alias or in a
> script) that does nothing (it's not like the command requires at
> least one -X option on the command line), either. Just like
> fb60b9f3 (sequencer: use struct strvec to store merge strategy
> options, 2023-04-10), which met no complaints about a possible
> fallout by the behaviour change, I do not think that this change
> even deserves an entry in the backward compatibility notes.
I concur with both of you. In a project like this one, we should be
rather generous with the set of things we expect users to do. But even
in a quite generous interpretation, I cannot imagine anybody relying on
this behavior, so I think skipping a mention of it in the backwards
compatibility section makes sense.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 4/8] parse-options: mark unused "opt" parameter in callbacks
From: Jeff King @ 2023-08-31 20:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, René Scharfe
In-Reply-To: <20230831175018.GB3208283@coredump.intra.peff.net>
On Thu, Aug 31, 2023 at 01:50:18PM -0400, Jeff King wrote:
> > > -static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
> > > +static int parse_refmap_arg(const struct option *opt UNUSED,
> > > + const char *arg, int unset)
> > > {
> > > BUG_ON_OPT_NEG(unset);
> >
> > Can't this just point opt->value at the global &refmap? Obviously
> > not a huge deal, as we could have taken the "annotate as UNUSED"
> > approach for all the functions in [3/8].
>
> Hmm, yeah. I think I looked at the abstract refspec_append() here and
> assumed that it might be touching other variables. But it's not. It's
> operating purely on the &refspec we pass it (and even though it uses
> ALLOC_GROW, the "nr" and "alloc" are both contained in the struct). So
> yeah, it really should have been converted in patch 3.
Oh, btw, there are two other cases in this patch that _could_ be
converted but I left alone. In log.c, we have a to_callback() and a
cc_callback(), both of which take a single string list. But there is
also header_callback(), which resets both of those when it sees
--no-add-header. So I left them alone as a set.
However, it occurs to me that to_callback() and cc_callback() can just
be OPT_STRING_LIST these days. And that is probably a worthwhile
cleanup to do.
-Peff
^ permalink raw reply
* Re: [PATCH 7/8] interpret-trailers: mark unused "unset" parameters in option callbacks
From: Jeff King @ 2023-08-31 17:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, René Scharfe
In-Reply-To: <xmqq4jkfxhx2.fsf@gitster.g>
On Thu, Aug 31, 2023 at 10:04:09AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > There are a few parse-option callbacks that do not look at their "unset"
> > parameters, but also do not set PARSE_OPT_NONEG. At first glance this
> > seems like a bug, as we'd ignore "--no-if-exists", etc.
> >
> > But they do work fine, because when "unset" is true, then "arg" is NULL.
> > And all three functions pass "arg" on to helper functions which do the
> > right thing with the NULL.
>
> Yuck. That is ugly.
Yep. I wondered about adding a comment here warning about the situation,
but it felt kind of content-less. Something like:
/* if unset, arg is NULL and handled below */
trailer_set_where(opt->value, arg);
> > Note that this shortcut would not be correct if any callback used
> > PARSE_OPT_NOARG (in which case "arg" would be NULL but "unset" would be
> > false). But none of these do.
>
> That is even uglier. Unlike the BUG_ON_OPT_NEG() and BUG_ON_OPT_ARG()
> that catch discrepancies between options[] flags and the expectation
> by the callback function, there is no way for us to protect against
> such mistakes?
I guess it would be something like:
if (!unset && !arg)
BUG("unexpected use of PARSE_OPT_OPTARG");
I think it is less important than those other ones, though, because the
mistake here is the OPT_CALLBACK declaration adding a flag that the
callback is not prepared to handle. Whereas in the other ones, the bug
is that the declaration _forgot_ to use a flag, which is a much more
likely bug.
So I dunno. If it were more than this one case (well, three, but they
are all of the same form) I'd be more worried.
-Peff
^ permalink raw reply
* Re: [PATCH 4/8] parse-options: mark unused "opt" parameter in callbacks
From: Jeff King @ 2023-08-31 17:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, René Scharfe
In-Reply-To: <xmqqil8vxjcd.fsf@gitster.g>
On Thu, Aug 31, 2023 at 09:33:22AM -0700, Junio C Hamano wrote:
> > -static int option_parse_stage(const struct option *opt,
> > +static int option_parse_stage(const struct option *opt UNUSED,
> > const char *arg, int unset)
> > {
> > BUG_ON_OPT_NEG(unset);
>
> I suspect that the original is buggy; when given
>
> $ git checkout-index --stage=all --stage=1 path
>
> the first option turns --temp on, but the second one does not turn
> it off. For now I think being bug-to-bug compatible and annotating
> the opt as UNUSED is good, but as a follow-up, we could make the
> caller:
>
> (1) point &checkout_stage with opt->value;
>
> (2) make to_tempfile to tristate <unspecified, false, true> by
> initializing it to -1;
>
> (3) adjust to_tempfile that is still <unspecified> after
> parse_options() returns, according to the value in
> checkout_stage.
>
> and then this can follow the "opt->value points at the variable that
> is affected" pattern.
Yeah, I think that would work, with one extra bit:
(4) complain when (!to_tempfile && checkout_stage == CHECKOUT_ALL)
I do think it would be better to fix separately, but maybe if I'm
re-rolling I can do it as an early patch in the series (it is not much
different than the "xopts" fix in scope).
> > -static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
> > +static int parse_refmap_arg(const struct option *opt UNUSED,
> > + const char *arg, int unset)
> > {
> > BUG_ON_OPT_NEG(unset);
>
> Can't this just point opt->value at the global &refmap? Obviously
> not a huge deal, as we could have taken the "annotate as UNUSED"
> approach for all the functions in [3/8].
Hmm, yeah. I think I looked at the abstract refspec_append() here and
assumed that it might be touching other variables. But it's not. It's
operating purely on the &refspec we pass it (and even though it uses
ALLOC_GROW, the "nr" and "alloc" are both contained in the struct). So
yeah, it really should have been converted in patch 3.
I think that is probably worth a re-roll.
-Peff
^ permalink raw reply
* Re: [PATCH 10/10] lower core.maxTreeDepth default to 2048
From: Jeff King @ 2023-08-31 17:42 UTC (permalink / raw)
To: Oswald Buddenhagen; +Cc: git
In-Reply-To: <ZPBt6VYAWHJhv3N2@ugly>
On Thu, Aug 31, 2023 at 12:39:37PM +0200, Oswald Buddenhagen wrote:
> On Thu, Aug 31, 2023 at 02:23:20AM -0400, Jeff King wrote:
> > But I thought that
> > following the sequence of logic (from "4096 is probably OK" to "whoops,
> > it's not") had some value to share.
> >
> of course, but you can just integrate that into the squashed commit message.
> having it all in one place makes it easier to follow.
Yes, though I think having it as a separate patch makes it easier to
revisit later (e.g., by reverting or by replacing the patch during a
re-roll).
-Peff
^ permalink raw reply
* Re: Problem: `fatal: too-short tree object` when executing hash-object on tree
From: Junio C Hamano @ 2023-08-31 17:25 UTC (permalink / raw)
To: Gareth Hayes; +Cc: git
In-Reply-To: <0de414f8-f409-467f-a504-06a78f088981@app.fastmail.com>
"Gareth Hayes" <gareth.hayes@bitcoin.org.hk> writes:
> Problem: I'm trying to reproduce the identifier of a tree object using `git cat-file -p <tree object identifier> | git hash-object -t tree --stdin`
>
> This results in an error:
> `fatal: too-short tree object`
>
> To replicate:
> `git cat-file -p HEAD`
> `git cat-file -p <tree object identifier from output of above> | git hash-object -t tree --stdin`
>
> This works for other object types but not trees. What am I doing wrong?
It would work *ONLY* for blob, and not commit or tag object types,
no?
The "-p" option is to present the object in a human-friendly format,
as opposed to giving the raw stream of bytes that is suitable for
machine consumption and required by hash-object. It so happens that
for "blob", the raw stream of bytes is just as human-friendly as the
tool can make, without having a deep knowledge of the content type
(e.g. it may be possible to make "cat-file -p <blob>" to somehow
apply an appropriate textconv filter if the path to <blob> is known
as a future enhancement, but such a code does not exist yet, and
when it happens, even "cat-file -p <blob> | hash-object --stdin"
would not round-trip).
So, that is what you are doing wrong, I think.
^ permalink raw reply
* Re: Is there a way to get the "format-patch" formatted file name?
From: Junio C Hamano @ 2023-08-31 17:19 UTC (permalink / raw)
To: Vít Ondruch; +Cc: git
In-Reply-To: <aa35fbdb-cca4-ae04-4124-9498d682ec06@redhat.com>
Vít Ondruch <vondruch@redhat.com> writes:
> My typical use case is to download patches from GH, e.g.:
>
> ~~~
>
> $ curl -OL https://github.com/rails/sprockets/pull/791.patch
...
> The problem with this is that I end up with the "791.patch" file,
> while I'd like have a file with similar name as if I have used the git
> command:
>
>
> ~~~
>
> $ git format-patch -1 6554b6d
> 0001-Fix-Minitest-constant-name-in-tests.patch
>
> ~~~
>
>
> So I wonder, is there a way to get such file name?
Do you mean: GitHub should let me run this command instead
$ curl -OL https://github.com/rails/sprockets/pull/0001-Fix-Minitest-constant-name-in-tests.patch
It may be nice for them to give a more meaningful name to their pull
request (not just the output file name) than just an integer. But
that is not a question/request we can answer here (this is not a
help forum for GitHub users).
Something along the lines of
sed -ne '/^Subject: /{
s/^Subject: *\[PATCH[^]]*\] *//;
s/[^a-zA-Z0-9]/-/g;
s/--*/-/g;
s/$/\.patch/;
p;
q;
}' 791.patch
should be doable, but I am not sure what the benefit is. Once you
get it in Git, you'd park it on a branch with a useful name and we
can forget about "791", so the "The files we get from GitHub are
named in a way that makes it hard to identify them" does not sound
like a Git issue, at least to me.
^ permalink raw reply
* Re: Problem: `fatal: too-short tree object` when executing hash-object on tree
From: Johannes Sixt @ 2023-08-31 17:10 UTC (permalink / raw)
To: Gareth Hayes; +Cc: git
In-Reply-To: <0de414f8-f409-467f-a504-06a78f088981@app.fastmail.com>
Am 31.08.23 um 12:58 schrieb Gareth Hayes:
> Problem: I'm trying to reproduce the identifier of a tree object using `git cat-file -p <tree object identifier> | git hash-object -t tree --stdin`
XY-problem alert!?
> This results in an error:
> `fatal: too-short tree object`
>
> To replicate:
> `git cat-file -p HEAD`
> `git cat-file -p <tree object identifier from output of above> | git hash-object -t tree --stdin`
>
> This works for other object types but not trees. What am I doing wrong?
Recall that the -p in cat-file means "pretty" (-print). The data that
makes up a tree object is far from pretty, hence, you can't feed it into
git hash-object directly.
You can either turn the pretty-printed input into a tree object
git cat-file -p $treeoid | git mktree
or hash the raw data
git cat-file tree $treeoid | git hash-object -t tree --stdin
-- Hannes
^ permalink raw reply
* Re: [PATCH 8/8] parse-options: mark unused parameters in noop callback
From: Junio C Hamano @ 2023-08-31 17:05 UTC (permalink / raw)
To: Jeff King; +Cc: git, René Scharfe
In-Reply-To: <20230831072027.GF3197751@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Unsurprisingly, the noop options callback doesn't bother to look at any
> of its parameters. Let's mark them so that -Wunused-parameter does not
> complain.
;-)
Thanks.
> Another option would be to drop the callback and have parse-options
> itself recognize OPT_NOOP_NOARG. But that seems like extra work for no
> real benefit.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> parse-options-cb.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index a24521dee0..bdc7fae497 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -227,7 +227,9 @@ int parse_opt_strvec(const struct option *opt, const char *arg, int unset)
> return 0;
> }
>
> -int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
> +int parse_opt_noop_cb(const struct option *opt UNUSED,
> + const char *arg UNUSED,
> + int unset UNUSED)
> {
> return 0;
> }
^ permalink raw reply
* Re: [PATCH 7/8] interpret-trailers: mark unused "unset" parameters in option callbacks
From: Junio C Hamano @ 2023-08-31 17:04 UTC (permalink / raw)
To: Jeff King; +Cc: git, René Scharfe
In-Reply-To: <20230831071945.GE3197751@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> There are a few parse-option callbacks that do not look at their "unset"
> parameters, but also do not set PARSE_OPT_NONEG. At first glance this
> seems like a bug, as we'd ignore "--no-if-exists", etc.
>
> But they do work fine, because when "unset" is true, then "arg" is NULL.
> And all three functions pass "arg" on to helper functions which do the
> right thing with the NULL.
Yuck. That is ugly.
> Note that this shortcut would not be correct if any callback used
> PARSE_OPT_NOARG (in which case "arg" would be NULL but "unset" would be
> false). But none of these do.
That is even uglier. Unlike the BUG_ON_OPT_NEG() and BUG_ON_OPT_ARG()
that catch discrepancies between options[] flags and the expectation
by the callback function, there is no way for us to protect against
such mistakes?
> So the code is fine as-is. But we'll want to mark the unused "unset"
> parameters to quiet -Wunused-parameter.
OK. Thanks.
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> builtin/interpret-trailers.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> index 6aadce6a1e..a5f9adf6b9 100644
> --- a/builtin/interpret-trailers.c
> +++ b/builtin/interpret-trailers.c
> @@ -24,19 +24,19 @@ static enum trailer_if_exists if_exists;
> static enum trailer_if_missing if_missing;
>
> static int option_parse_where(const struct option *opt,
> - const char *arg, int unset)
> + const char *arg, int unset UNUSED)
> {
> return trailer_set_where(opt->value, arg);
> }
>
> static int option_parse_if_exists(const struct option *opt,
> - const char *arg, int unset)
> + const char *arg, int unset UNUSED)
> {
> return trailer_set_if_exists(opt->value, arg);
> }
>
> static int option_parse_if_missing(const struct option *opt,
> - const char *arg, int unset)
> + const char *arg, int unset UNUSED)
> {
> return trailer_set_if_missing(opt->value, arg);
> }
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox