Re: [PATCH v1] config.c: fix msvc compile error
On 7/24/2018 3:56 PM, Taylor Blau wrote: On Tue, Jul 24, 2018 at 03:30:10PM +, g...@jeffhostetler.com wrote: From: Jeff Hostetler In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added to builtin/config.c to define a new function and a forward declaration for an array of unknown size. This causes a compile error under MSVC. Thanks for spending time fixing that. fb0dc3bac1 (builtin/config.c: support `--type=` as preferred alias for `--`, 2018-04-18) is from me, so I owe you an extra thanks for patching up mistakes :-). As others have noted in this thread, another patch was sent into similar effect, which has already been queued, and I agree that we should prefer that, since it's further along. Thanks, Taylor Yes, the other version is further along. Let's take it. Jeff
Re: [PATCH v1] config.c: fix msvc compile error
On Tue, Jul 24, 2018 at 03:30:10PM +, g...@jeffhostetler.com wrote: > From: Jeff Hostetler > > In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added > to builtin/config.c to define a new function and a forward declaration > for an array of unknown size. This causes a compile error under MSVC. Thanks for spending time fixing that. fb0dc3bac1 (builtin/config.c: support `--type=` as preferred alias for `--`, 2018-04-18) is from me, so I owe you an extra thanks for patching up mistakes :-). As others have noted in this thread, another patch was sent into similar effect, which has already been queued, and I agree that we should prefer that, since it's further along. Thanks, Taylor
Re: [PATCH v1] config.c: fix msvc compile error
On 24.07.18 20:50, Junio C Hamano wrote: > Beat Bolli writes: > >> On 24.07.18 20:22, Junio C Hamano wrote: >> This was already fixed (differently) in <20180705183445.30901-1-dev+...@drbeat.li>. >>> >>> Thanks for saving me from having to dig the list archive myself. >>> Yes, it is already applied to the tip of the topic that originally >>> caused the breakage. >>> >> Just a general question: >> >> Is it OK to refer to patches on pu with the Message-ID, or would you >> prefer the commit hash? The hash changes whenever you recreate pu, >> doesn't it? > > Either is fine in practice. The commits themselves on a topic > branch that is not yet in 'next' usually stay the same once the tip > of 'pu' that contains them gets published. Even though I often use > "git rebase -i", "git commit --amend", etc. to fix up posted patches > while turning them into commits on topic branches, I usually stop > doing so once I push out day's integration result. > > Until a new version of the series is posted to replace them on the > topic branch, that is. But at that point we are talking about new > patches with different message-ids that got turned into different > commit objects, so either commit object name or message id that > refer to older iteration would still name the same old version, and > new names would refer to the same new version. > Ok, thanks!
Re: [PATCH v1] config.c: fix msvc compile error
Beat Bolli writes: > On 24.07.18 20:22, Junio C Hamano wrote: > >>> This was already fixed (differently) in >>> <20180705183445.30901-1-dev+...@drbeat.li>. >> >> Thanks for saving me from having to dig the list archive myself. >> Yes, it is already applied to the tip of the topic that originally >> caused the breakage. >> > Just a general question: > > Is it OK to refer to patches on pu with the Message-ID, or would you > prefer the commit hash? The hash changes whenever you recreate pu, > doesn't it? Either is fine in practice. The commits themselves on a topic branch that is not yet in 'next' usually stay the same once the tip of 'pu' that contains them gets published. Even though I often use "git rebase -i", "git commit --amend", etc. to fix up posted patches while turning them into commits on topic branches, I usually stop doing so once I push out day's integration result. Until a new version of the series is posted to replace them on the topic branch, that is. But at that point we are talking about new patches with different message-ids that got turned into different commit objects, so either commit object name or message id that refer to older iteration would still name the same old version, and new names would refer to the same new version.
Re: [PATCH v1] config.c: fix msvc compile error
On 24.07.18 20:22, Junio C Hamano wrote: > Beat Bolli writes: > >> Hi Jeff >> >> On 24.07.18 17:30, g...@jeffhostetler.com wrote: >>> From: Jeff Hostetler >>> >>> In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added >>> to builtin/config.c to define a new function and a forward declaration >>> for an array of unknown size. This causes a compile error under MSVC. >>> >>> Reorder the code to forward declare the function instead of the array. >> >> This was already fixed (differently) in >> <20180705183445.30901-1-dev+...@drbeat.li>. > > Thanks for saving me from having to dig the list archive myself. > Yes, it is already applied to the tip of the topic that originally > caused the breakage. > Just a general question: Is it OK to refer to patches on pu with the Message-ID, or would you prefer the commit hash? The hash changes whenever you recreate pu, doesn't it? Beat
Re: [PATCH v1] config.c: fix msvc compile error
Beat Bolli writes: > Hi Jeff > > On 24.07.18 17:30, g...@jeffhostetler.com wrote: >> From: Jeff Hostetler >> >> In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added >> to builtin/config.c to define a new function and a forward declaration >> for an array of unknown size. This causes a compile error under MSVC. >> >> Reorder the code to forward declare the function instead of the array. > > This was already fixed (differently) in > <20180705183445.30901-1-dev+...@drbeat.li>. Thanks for saving me from having to dig the list archive myself. Yes, it is already applied to the tip of the topic that originally caused the breakage.
Re: [PATCH v1] config.c: fix msvc compile error
Hi Jeff
On 24.07.18 17:30, g...@jeffhostetler.com wrote:
> From: Jeff Hostetler
>
> In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added
> to builtin/config.c to define a new function and a forward declaration
> for an array of unknown size. This causes a compile error under MSVC.
>
> Reorder the code to forward declare the function instead of the array.
This was already fixed (differently) in
<20180705183445.30901-1-dev+...@drbeat.li>.
Cheers,
Beat
> Signed-off-by: Jeff Hostetler
> ---
> builtin/config.c | 79
>
> 1 file changed, 40 insertions(+), 39 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index b29d26d..564f18f 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -67,7 +67,46 @@ static int show_origin;
> { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
> PARSE_OPT_NONEG, option_parse_type, (i) }
>
> -static struct option builtin_config_options[];
> +static int option_parse_type(const struct option *opt, const char *arg,
> + int unset);
> +
> +static struct option builtin_config_options[] = {
> + OPT_GROUP(N_("Config file location")),
> + OPT_BOOL(0, "global", _global_config, N_("use global config file")),
> + OPT_BOOL(0, "system", _system_config, N_("use system config file")),
> + OPT_BOOL(0, "local", _local_config, N_("use repository config
> file")),
> + OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
> given config file")),
> + OPT_STRING(0, "blob", _config_source.blob, N_("blob-id"),
> N_("read config from given blob object")),
> + OPT_GROUP(N_("Action")),
> + OPT_BIT(0, "get", , N_("get value: name [value-regex]"),
> ACTION_GET),
> + OPT_BIT(0, "get-all", , N_("get all values: key
> [value-regex]"), ACTION_GET_ALL),
> + OPT_BIT(0, "get-regexp", , N_("get values for regexp:
> name-regex [value-regex]"), ACTION_GET_REGEXP),
> + OPT_BIT(0, "get-urlmatch", , N_("get value specific for the
> URL: section[.var] URL"), ACTION_GET_URLMATCH),
> + OPT_BIT(0, "replace-all", , N_("replace all matching variables:
> name value [value_regex]"), ACTION_REPLACE_ALL),
> + OPT_BIT(0, "add", , N_("add a new variable: name value"),
> ACTION_ADD),
> + OPT_BIT(0, "unset", , N_("remove a variable: name
> [value-regex]"), ACTION_UNSET),
> + OPT_BIT(0, "unset-all", , N_("remove all matches: name
> [value-regex]"), ACTION_UNSET_ALL),
> + OPT_BIT(0, "rename-section", , N_("rename section: old-name
> new-name"), ACTION_RENAME_SECTION),
> + OPT_BIT(0, "remove-section", , N_("remove a section: name"),
> ACTION_REMOVE_SECTION),
> + OPT_BIT('l', "list", , N_("list all"), ACTION_LIST),
> + OPT_BIT('e', "edit", , N_("open an editor"), ACTION_EDIT),
> + OPT_BIT(0, "get-color", , N_("find the color configured: slot
> [default]"), ACTION_GET_COLOR),
> + OPT_BIT(0, "get-colorbool", , N_("find the color setting: slot
> [stdout-is-tty]"), ACTION_GET_COLORBOOL),
> + OPT_GROUP(N_("Type")),
> + OPT_CALLBACK('t', "type", , "", N_("value is given this type"),
> option_parse_type),
> + OPT_CALLBACK_VALUE(0, "bool", , N_("value is \"true\" or
> \"false\""), TYPE_BOOL),
> + OPT_CALLBACK_VALUE(0, "int", , N_("value is decimal number"),
> TYPE_INT),
> + OPT_CALLBACK_VALUE(0, "bool-or-int", , N_("value is --bool or
> --int"), TYPE_BOOL_OR_INT),
> + OPT_CALLBACK_VALUE(0, "path", , N_("value is a path (file or
> directory name)"), TYPE_PATH),
> + OPT_CALLBACK_VALUE(0, "expiry-date", , N_("value is an expiry
> date"), TYPE_EXPIRY_DATE),
> + OPT_GROUP(N_("Other")),
> + OPT_BOOL('z', "null", _null, N_("terminate values with NUL byte")),
> + OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
> + OPT_BOOL(0, "includes", _includes_opt, N_("respect include
> directives on lookup")),
> + OPT_BOOL(0, "show-origin", _origin, N_("show origin of config
> (file, standard input, blob, command line)")),
> + OPT_STRING(0, "default", _value, N_("value"), N_("with --get,
> use default value when missing entry")),
> + OPT_END(),
> +};
>
> static int option_parse_type(const struct option *opt, const char *arg,
>int unset)
> @@ -119,44 +158,6 @@ static int option_parse_type(const struct option *opt,
> const char *arg,
> return 0;
> }
>
> -static struct option builtin_config_options[] = {
> - OPT_GROUP(N_("Config file location")),
> - OPT_BOOL(0, "global", _global_config, N_("use global config file")),
> - OPT_BOOL(0, "system", _system_config, N_("use system config file")),
> - OPT_BOOL(0, "local", _local_config, N_("use repository config
> file")),
> - OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
> given config file")),
> - OPT_STRING(0, "blob", _config_source.blob, N_("blob-id"),
> N_("read config from given blob
Re: [PATCH v1] config.c: fix msvc compile error
g...@jeffhostetler.com writes:
> From: Jeff Hostetler
>
> In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added
> to builtin/config.c to define a new function and a forward declaration
> for an array of unknown size. This causes a compile error under MSVC.
>
> Reorder the code to forward declare the function instead of the array.
>
> Signed-off-by: Jeff Hostetler
> ---
Somebody please tell me why I have a feeling that I've seen this
patch, even though it is labeled as v1... The problem and the
solution seem pretty obvious and I recall somebody saying that a
single pass compiler wouldn't be able to grok the forward decl.
> builtin/config.c | 79
>
> 1 file changed, 40 insertions(+), 39 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index b29d26d..564f18f 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -67,7 +67,46 @@ static int show_origin;
> { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
> PARSE_OPT_NONEG, option_parse_type, (i) }
>
> -static struct option builtin_config_options[];
> +static int option_parse_type(const struct option *opt, const char *arg,
> + int unset);
> +
> +static struct option builtin_config_options[] = {
> + OPT_GROUP(N_("Config file location")),
> + OPT_BOOL(0, "global", _global_config, N_("use global config file")),
> + OPT_BOOL(0, "system", _system_config, N_("use system config file")),
> + OPT_BOOL(0, "local", _local_config, N_("use repository config
> file")),
> + OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
> given config file")),
> + OPT_STRING(0, "blob", _config_source.blob, N_("blob-id"),
> N_("read config from given blob object")),
> + OPT_GROUP(N_("Action")),
> + OPT_BIT(0, "get", , N_("get value: name [value-regex]"),
> ACTION_GET),
> + OPT_BIT(0, "get-all", , N_("get all values: key
> [value-regex]"), ACTION_GET_ALL),
> + OPT_BIT(0, "get-regexp", , N_("get values for regexp:
> name-regex [value-regex]"), ACTION_GET_REGEXP),
> + OPT_BIT(0, "get-urlmatch", , N_("get value specific for the
> URL: section[.var] URL"), ACTION_GET_URLMATCH),
> + OPT_BIT(0, "replace-all", , N_("replace all matching variables:
> name value [value_regex]"), ACTION_REPLACE_ALL),
> + OPT_BIT(0, "add", , N_("add a new variable: name value"),
> ACTION_ADD),
> + OPT_BIT(0, "unset", , N_("remove a variable: name
> [value-regex]"), ACTION_UNSET),
> + OPT_BIT(0, "unset-all", , N_("remove all matches: name
> [value-regex]"), ACTION_UNSET_ALL),
> + OPT_BIT(0, "rename-section", , N_("rename section: old-name
> new-name"), ACTION_RENAME_SECTION),
> + OPT_BIT(0, "remove-section", , N_("remove a section: name"),
> ACTION_REMOVE_SECTION),
> + OPT_BIT('l', "list", , N_("list all"), ACTION_LIST),
> + OPT_BIT('e', "edit", , N_("open an editor"), ACTION_EDIT),
> + OPT_BIT(0, "get-color", , N_("find the color configured: slot
> [default]"), ACTION_GET_COLOR),
> + OPT_BIT(0, "get-colorbool", , N_("find the color setting: slot
> [stdout-is-tty]"), ACTION_GET_COLORBOOL),
> + OPT_GROUP(N_("Type")),
> + OPT_CALLBACK('t', "type", , "", N_("value is given this type"),
> option_parse_type),
> + OPT_CALLBACK_VALUE(0, "bool", , N_("value is \"true\" or
> \"false\""), TYPE_BOOL),
> + OPT_CALLBACK_VALUE(0, "int", , N_("value is decimal number"),
> TYPE_INT),
> + OPT_CALLBACK_VALUE(0, "bool-or-int", , N_("value is --bool or
> --int"), TYPE_BOOL_OR_INT),
> + OPT_CALLBACK_VALUE(0, "path", , N_("value is a path (file or
> directory name)"), TYPE_PATH),
> + OPT_CALLBACK_VALUE(0, "expiry-date", , N_("value is an expiry
> date"), TYPE_EXPIRY_DATE),
> + OPT_GROUP(N_("Other")),
> + OPT_BOOL('z', "null", _null, N_("terminate values with NUL byte")),
> + OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
> + OPT_BOOL(0, "includes", _includes_opt, N_("respect include
> directives on lookup")),
> + OPT_BOOL(0, "show-origin", _origin, N_("show origin of config
> (file, standard input, blob, command line)")),
> + OPT_STRING(0, "default", _value, N_("value"), N_("with --get,
> use default value when missing entry")),
> + OPT_END(),
> +};
>
> static int option_parse_type(const struct option *opt, const char *arg,
>int unset)
> @@ -119,44 +158,6 @@ static int option_parse_type(const struct option *opt,
> const char *arg,
> return 0;
> }
>
> -static struct option builtin_config_options[] = {
> - OPT_GROUP(N_("Config file location")),
> - OPT_BOOL(0, "global", _global_config, N_("use global config file")),
> - OPT_BOOL(0, "system", _system_config, N_("use system config file")),
> - OPT_BOOL(0, "local", _local_config, N_("use repository config
> file")),
> - OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
[PATCH v1] config.c: fix msvc compile error
From: Jeff Hostetler
In commit fb0dc3bac135e9f6243bd6d293e8c9293c73b9cd code was added
to builtin/config.c to define a new function and a forward declaration
for an array of unknown size. This causes a compile error under MSVC.
Reorder the code to forward declare the function instead of the array.
Signed-off-by: Jeff Hostetler
---
builtin/config.c | 79
1 file changed, 40 insertions(+), 39 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index b29d26d..564f18f 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -67,7 +67,46 @@ static int show_origin;
{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
PARSE_OPT_NONEG, option_parse_type, (i) }
-static struct option builtin_config_options[];
+static int option_parse_type(const struct option *opt, const char *arg,
+int unset);
+
+static struct option builtin_config_options[] = {
+ OPT_GROUP(N_("Config file location")),
+ OPT_BOOL(0, "global", _global_config, N_("use global config file")),
+ OPT_BOOL(0, "system", _system_config, N_("use system config file")),
+ OPT_BOOL(0, "local", _local_config, N_("use repository config
file")),
+ OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
given config file")),
+ OPT_STRING(0, "blob", _config_source.blob, N_("blob-id"),
N_("read config from given blob object")),
+ OPT_GROUP(N_("Action")),
+ OPT_BIT(0, "get", , N_("get value: name [value-regex]"),
ACTION_GET),
+ OPT_BIT(0, "get-all", , N_("get all values: key
[value-regex]"), ACTION_GET_ALL),
+ OPT_BIT(0, "get-regexp", , N_("get values for regexp:
name-regex [value-regex]"), ACTION_GET_REGEXP),
+ OPT_BIT(0, "get-urlmatch", , N_("get value specific for the
URL: section[.var] URL"), ACTION_GET_URLMATCH),
+ OPT_BIT(0, "replace-all", , N_("replace all matching variables:
name value [value_regex]"), ACTION_REPLACE_ALL),
+ OPT_BIT(0, "add", , N_("add a new variable: name value"),
ACTION_ADD),
+ OPT_BIT(0, "unset", , N_("remove a variable: name
[value-regex]"), ACTION_UNSET),
+ OPT_BIT(0, "unset-all", , N_("remove all matches: name
[value-regex]"), ACTION_UNSET_ALL),
+ OPT_BIT(0, "rename-section", , N_("rename section: old-name
new-name"), ACTION_RENAME_SECTION),
+ OPT_BIT(0, "remove-section", , N_("remove a section: name"),
ACTION_REMOVE_SECTION),
+ OPT_BIT('l', "list", , N_("list all"), ACTION_LIST),
+ OPT_BIT('e', "edit", , N_("open an editor"), ACTION_EDIT),
+ OPT_BIT(0, "get-color", , N_("find the color configured: slot
[default]"), ACTION_GET_COLOR),
+ OPT_BIT(0, "get-colorbool", , N_("find the color setting: slot
[stdout-is-tty]"), ACTION_GET_COLORBOOL),
+ OPT_GROUP(N_("Type")),
+ OPT_CALLBACK('t', "type", , "", N_("value is given this type"),
option_parse_type),
+ OPT_CALLBACK_VALUE(0, "bool", , N_("value is \"true\" or
\"false\""), TYPE_BOOL),
+ OPT_CALLBACK_VALUE(0, "int", , N_("value is decimal number"),
TYPE_INT),
+ OPT_CALLBACK_VALUE(0, "bool-or-int", , N_("value is --bool or
--int"), TYPE_BOOL_OR_INT),
+ OPT_CALLBACK_VALUE(0, "path", , N_("value is a path (file or
directory name)"), TYPE_PATH),
+ OPT_CALLBACK_VALUE(0, "expiry-date", , N_("value is an expiry
date"), TYPE_EXPIRY_DATE),
+ OPT_GROUP(N_("Other")),
+ OPT_BOOL('z', "null", _null, N_("terminate values with NUL byte")),
+ OPT_BOOL(0, "name-only", _values, N_("show variable names only")),
+ OPT_BOOL(0, "includes", _includes_opt, N_("respect include
directives on lookup")),
+ OPT_BOOL(0, "show-origin", _origin, N_("show origin of config
(file, standard input, blob, command line)")),
+ OPT_STRING(0, "default", _value, N_("value"), N_("with --get,
use default value when missing entry")),
+ OPT_END(),
+};
static int option_parse_type(const struct option *opt, const char *arg,
int unset)
@@ -119,44 +158,6 @@ static int option_parse_type(const struct option *opt,
const char *arg,
return 0;
}
-static struct option builtin_config_options[] = {
- OPT_GROUP(N_("Config file location")),
- OPT_BOOL(0, "global", _global_config, N_("use global config file")),
- OPT_BOOL(0, "system", _system_config, N_("use system config file")),
- OPT_BOOL(0, "local", _local_config, N_("use repository config
file")),
- OPT_STRING('f', "file", _config_source.file, N_("file"), N_("use
given config file")),
- OPT_STRING(0, "blob", _config_source.blob, N_("blob-id"),
N_("read config from given blob object")),
- OPT_GROUP(N_("Action")),
- OPT_BIT(0, "get", , N_("get value: name [value-regex]"),
ACTION_GET),
- OPT_BIT(0, "get-all", , N_("get all values: key
[value-regex]"), ACTION_GET_ALL),
- OPT_BIT(0, "get-regexp", , N_("get values for regexp:
