threads / patch / 48934

v2Makefile: add a DEVOPTS flag to get pedantic compilation

Subject: [PATCH v2] Makefile: add a DEVOPTS flag to get pedantic compilation

## tl;dr

8 messages between Jul 21, 2018 and Jul 25, 2018. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Beat Bolli· Jul 21, 2018, 20:36 UTC · lore

In the interest of code hygiene, make it easier to compile Git with the flag -pedantic.

Pure pedantic compilation with GCC 7.3 results in one warning per use of the translation macro `N_`:

    warning: array initialized from parenthesized string constant [-Wpedantic]

Therefore also disable the parenthesising of i18n strings with -DUSE_PARENS_AROUND_GETTEXT_N=no.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---

This is the convenience knob for all developers that led to the series bb/pedantic[1]. It does not depend on this series, though.

[1] https://public-inbox.org/git/20180708144342.11922-1-dev+git@drbeat.li/T/#u
 Makefile       | 6 ++++++
 config.mak.dev | 5 +++++
 2 files changed, 11 insertions(+)
Show changes to 2 files +10 −0

Makefile, config.mak.dev

diff --git a/Makefile b/Makefile
index 0cb6590f24..2bfc051652 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,12 @@ all::
 #        The DEVELOPER mode enables -Wextra with a few exceptions. By
 #        setting this flag the exceptions are removed, and all of
 #        -Wextra is used.
+#
+#    pedantic:
+#
+#        Enable -pedantic compilation. This also disables
+#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
diff --git a/config.mak.dev b/config.mak.dev
index 2d244ca470..e11dd94741 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -1,6 +1,11 @@
 ifeq ($(filter no-error,$(DEVOPTS)),)
 CFLAGS += -Werror
 endif
+ifneq ($(filter pedantic,$(DEVOPTS)),)
+CFLAGS += -pedantic
+# don't warn for each N_ use
+CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=no
+endif
 CFLAGS += -Wdeclaration-after-statement
 CFLAGS += -Wno-format-zero-length
 CFLAGS += -Wold-style-definition

-- 
2.18.0.203.gfac676dfb9
Junio C Hamano· Jul 23, 2018, 18:53 UTC · re: Beat Bolli · lore

Re: [PATCH v2] Makefile: add a DEVOPTS flag to get pedantic compilation

Beat Bolli <dev+git@drbeat.li> writes:
Show 16 quoted lines
> In the interest of code hygiene, make it easier to compile Git with the
> flag -pedantic.
>
> Pure pedantic compilation with GCC 7.3 results in one warning per use of
> the translation macro `N_`:
>
>     warning: array initialized from parenthesized string constant [-Wpedantic]
>
> Therefore also disable the parenthesising of i18n strings with
> -DUSE_PARENS_AROUND_GETTEXT_N=no.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>
> This is the convenience knob for all developers that led to the series
> bb/pedantic[1]. It does not depend on this series, though.

Yup, but "make DEVELOPER=Yes" build won't pass unless this patch is queued after those clean-up ;-)

Remind me if I forget to tweak =no back to =0 before pushing the result out.

Thanks.
Show 38 quoted lines
> [1] https://public-inbox.org/git/20180708144342.11922-1-dev+git@drbeat.li/T/#u
>
>  Makefile       | 6 ++++++
>  config.mak.dev | 5 +++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 0cb6590f24..2bfc051652 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -484,6 +484,12 @@ all::
>  #        The DEVELOPER mode enables -Wextra with a few exceptions. By
>  #        setting this flag the exceptions are removed, and all of
>  #        -Wextra is used.
> +#
> +#    pedantic:
> +#
> +#        Enable -pedantic compilation. This also disables
> +#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
>  
>  GIT-VERSION-FILE: FORCE
>  	@$(SHELL_PATH) ./GIT-VERSION-GEN
> diff --git a/config.mak.dev b/config.mak.dev
> index 2d244ca470..e11dd94741 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -1,6 +1,11 @@
>  ifeq ($(filter no-error,$(DEVOPTS)),)
>  CFLAGS += -Werror
>  endif
> +ifneq ($(filter pedantic,$(DEVOPTS)),)
> +CFLAGS += -pedantic
> +# don't warn for each N_ use
> +CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=no
> +endif
>  CFLAGS += -Wdeclaration-after-statement
>  CFLAGS += -Wno-format-zero-length
>  CFLAGS += -Wold-style-definition
Beat Bolli· Jul 24, 2018, 06:04 UTC · re: Junio C Hamano · lore

Re: [PATCH v2] Makefile: add a DEVOPTS flag to get pedantic compilation

On 23.07.18 20:53, Junio C Hamano wrote:
Show 21 quoted lines
> Beat Bolli <dev+git@drbeat.li> writes:
> 
>> In the interest of code hygiene, make it easier to compile Git with the
>> flag -pedantic.
>>
>> Pure pedantic compilation with GCC 7.3 results in one warning per use of
>> the translation macro `N_`:
>>
>>     warning: array initialized from parenthesized string constant [-Wpedantic]
>>
>> Therefore also disable the parenthesising of i18n strings with
>> -DUSE_PARENS_AROUND_GETTEXT_N=no.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>
>> This is the convenience knob for all developers that led to the series
>> bb/pedantic[1]. It does not depend on this series, though.
> 
> Yup, but "make DEVELOPER=Yes" build won't pass unless this patch is
> queued after those clean-up ;-)

Then there's a bug in this patch. It should only have an effect if we "make DEVELOPER=Yes DEVOPTS=pedantic". Did you try this?

> Remind me if I forget to tweak =no back to =0 before pushing the
> result out.
No problem, I can send a v3 with this change reverted.
Beat
Show 41 quoted lines
> 
> Thanks.
> 
>> [1] https://public-inbox.org/git/20180708144342.11922-1-dev+git@drbeat.li/T/#u
>>
>>  Makefile       | 6 ++++++
>>  config.mak.dev | 5 +++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/Makefile b/Makefile
>> index 0cb6590f24..2bfc051652 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -484,6 +484,12 @@ all::
>>  #        The DEVELOPER mode enables -Wextra with a few exceptions. By
>>  #        setting this flag the exceptions are removed, and all of
>>  #        -Wextra is used.
>> +#
>> +#    pedantic:
>> +#
>> +#        Enable -pedantic compilation. This also disables
>> +#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
>>  
>>  GIT-VERSION-FILE: FORCE
>>  	@$(SHELL_PATH) ./GIT-VERSION-GEN
>> diff --git a/config.mak.dev b/config.mak.dev
>> index 2d244ca470..e11dd94741 100644
>> --- a/config.mak.dev
>> +++ b/config.mak.dev
>> @@ -1,6 +1,11 @@
>>  ifeq ($(filter no-error,$(DEVOPTS)),)
>>  CFLAGS += -Werror
>>  endif
>> +ifneq ($(filter pedantic,$(DEVOPTS)),)
>> +CFLAGS += -pedantic
>> +# don't warn for each N_ use
>> +CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=no
>> +endif
>>  CFLAGS += -Wdeclaration-after-statement
>>  CFLAGS += -Wno-format-zero-length
>>  CFLAGS += -Wold-style-definition
Junio C Hamano· Jul 24, 2018, 13:21 UTC · re: Beat Bolli · lore

Re: [PATCH v2] Makefile: add a DEVOPTS flag to get pedantic compilation

Beat Bolli <dev+git@drbeat.li> writes:
Show 10 quoted lines
> On 23.07.18 20:53, Junio C Hamano wrote:
>> 
>>> This is the convenience knob for all developers that led to the series
>>> bb/pedantic[1]. It does not depend on this series, though.
>> 
>> Yup, but "make DEVELOPER=Yes" build won't pass unless this patch is
>> queued after those clean-up ;-)
>
> Then there's a bug in this patch. It should only have an effect if we
> "make DEVELOPER=Yes DEVOPTS=pedantic". Did you try this?

Sorry, "exercising the feature" is what I meant to say with that "make" invocation, so I should have also spelled DEVOPTS bit, too.

Beat Bolli· Jul 24, 2018, 17:13 UTC · re: Junio C Hamano · lore

[PATCH v3] Makefile: add a DEVOPTS flag to get pedantic compilation

In the interest of code hygiene, make it easier to compile Git with the flag -pedantic.

Pure pedantic compilation with GCC 7.3 results in one warning per use of the translation macro `N_`:

    warning: array initialized from parenthesized string constant [-Wpedantic]

Therefore also disable the parenthesising of i18n strings with -DUSE_PARENS_AROUND_GETTEXT_N=no.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Now with -DUSE_PARENS_AROUND_GETTEXT_N=0 instead of =No.

This is the convenience knob for all developers that led to the series bb/pedantic[1]. It does not depend on this series, though.

[1] https://public-inbox.org/git/20180708144342.11922-1-dev+git@drbeat.li/T/#u
 Makefile       | 6 ++++++
 config.mak.dev | 5 +++++
 2 files changed, 11 insertions(+)
Show changes to 2 files +10 −0

Makefile, config.mak.dev

diff --git a/Makefile b/Makefile
index 0cb6590f24..2bfc051652 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,12 @@ all::
 #        The DEVELOPER mode enables -Wextra with a few exceptions. By
 #        setting this flag the exceptions are removed, and all of
 #        -Wextra is used.
+#
+#    pedantic:
+#
+#        Enable -pedantic compilation. This also disables
+#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
diff --git a/config.mak.dev b/config.mak.dev
index 2d244ca470..e11dd94741 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -1,6 +1,11 @@
 ifeq ($(filter no-error,$(DEVOPTS)),)
 CFLAGS += -Werror
 endif
+ifneq ($(filter pedantic,$(DEVOPTS)),)
+CFLAGS += -pedantic
+# don't warn for each N_ use
+CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0
+endif
 CFLAGS += -Wdeclaration-after-statement
 CFLAGS += -Wno-format-zero-length
 CFLAGS += -Wold-style-definition

-- 
2.18.0.203.gfac676dfb9
Beat Bolli· Jul 24, 2018, 19:26 UTC · re: Beat Bolli · lore

[PATCH v4] Makefile: add a DEVOPTS flag to get pedantic compilation

In the interest of code hygiene, make it easier to compile Git with the flag -pedantic.

Pure pedantic compilation with GCC 7.3 results in one warning per use of the translation macro `N_`:

    warning: array initialized from parenthesized string constant [-Wpedantic]

Therefore also disable the parenthesising of i18n strings with -DUSE_PARENS_AROUND_GETTEXT_N=0.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
Now with -DUSE_PARENS_AROUND_GETTEXT_N=0 instead of =No.

This is the convenience knob for all developers that led to the series bb/pedantic[1]. It does not depend on this series, though.

[1] https://public-inbox.org/git/20180708144342.11922-1-dev+git@drbeat.li/T/#u
 Makefile       | 6 ++++++
 config.mak.dev | 5 +++++
 2 files changed, 11 insertions(+)
Show changes to 2 files +10 −0

Makefile, config.mak.dev

diff --git a/Makefile b/Makefile
index 0cb6590f24..2bfc051652 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,12 @@ all::
 #        The DEVELOPER mode enables -Wextra with a few exceptions. By
 #        setting this flag the exceptions are removed, and all of
 #        -Wextra is used.
+#
+#    pedantic:
+#
+#        Enable -pedantic compilation. This also disables
+#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
diff --git a/config.mak.dev b/config.mak.dev
index 2d244ca470..e11dd94741 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -1,6 +1,11 @@
 ifeq ($(filter no-error,$(DEVOPTS)),)
 CFLAGS += -Werror
 endif
+ifneq ($(filter pedantic,$(DEVOPTS)),)
+CFLAGS += -pedantic
+# don't warn for each N_ use
+CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0
+endif
 CFLAGS += -Wdeclaration-after-statement
 CFLAGS += -Wno-format-zero-length
 CFLAGS += -Wold-style-definition

-- 
2.18.0.203.gfac676dfb9
Junio C Hamano· Jul 25, 2018, 16:57 UTC · re: Beat Bolli · lore

Re: [PATCH v4] Makefile: add a DEVOPTS flag to get pedantic compilation

Beat Bolli <dev+git@drbeat.li> writes:
Show 13 quoted lines
> In the interest of code hygiene, make it easier to compile Git with the
> flag -pedantic.
>
> Pure pedantic compilation with GCC 7.3 results in one warning per use of
> the translation macro `N_`:
>
>     warning: array initialized from parenthesized string constant [-Wpedantic]
>
> Therefore also disable the parenthesising of i18n strings with
> -DUSE_PARENS_AROUND_GETTEXT_N=0.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
Hmph, what did you change between v3 and v4?
Show 5 quoted lines
> diff --git a/Makefile b/Makefile
> index 0cb6590f24..2bfc051652 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -484,6 +484,12 @@ all::

The postimage of this hunk is supposed to be 11 lines long, as you have five additional line in the middle of 6 original context lines. Where did this 12 come from? I am only interested in finding out if our patch generation tool(s) have some bugs with this question.

If this is only because you hand-edit your patch, then we have no tool breakage to worry about, but please refrain from doing so in the future (instead always go back to the commit, amend it, and re-run format-patch).

Thanks.
Show 11 quoted lines
>  #        The DEVELOPER mode enables -Wextra with a few exceptions. By
>  #        setting this flag the exceptions are removed, and all of
>  #        -Wextra is used.
> +#
> +#    pedantic:
> +#
> +#        Enable -pedantic compilation. This also disables
> +#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
>  
>  GIT-VERSION-FILE: FORCE
>  	@$(SHELL_PATH) ./GIT-VERSION-GEN
Beat Bolli· Jul 25, 2018, 17:38 UTC · re: Junio C Hamano · lore

Re: [PATCH v4] Makefile: add a DEVOPTS flag to get pedantic compilation

On 25.07.18 18:57, Junio C Hamano wrote:
Show 17 quoted lines
> Beat Bolli <dev+git@drbeat.li> writes:
> 
>> In the interest of code hygiene, make it easier to compile Git with the
>> flag -pedantic.
>>
>> Pure pedantic compilation with GCC 7.3 results in one warning per use of
>> the translation macro `N_`:
>>
>>     warning: array initialized from parenthesized string constant [-Wpedantic]
>>
>> Therefore also disable the parenthesising of i18n strings with
>> -DUSE_PARENS_AROUND_GETTEXT_N=0.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
> 
> Hmph, what did you change between v3 and v4?
Just the commit text. In v3, it still said =No instead of =0.
Show 17 quoted lines
>> diff --git a/Makefile b/Makefile
>> index 0cb6590f24..2bfc051652 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -484,6 +484,12 @@ all::
> 
> The postimage of this hunk is supposed to be 11 lines long, as you
> have five additional line in the middle of 6 original context lines.
> Where did this 12 come from?  I am only interested in finding out if
> our patch generation tool(s) have some bugs with this question.
> 
> If this is only because you hand-edit your patch, then we have no
> tool breakage to worry about, but please refrain from doing so in
> the future (instead always go back to the commit, amend it, and
> re-run format-patch).
> 
> Thanks.
You got me there :-/
Won't happen again, sorry.
Show 12 quoted lines
>>  #        The DEVELOPER mode enables -Wextra with a few exceptions. By
>>  #        setting this flag the exceptions are removed, and all of
>>  #        -Wextra is used.
>> +#
>> +#    pedantic:
>> +#
>> +#        Enable -pedantic compilation. This also disables
>> +#        USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings.
>>  
>>  GIT-VERSION-FILE: FORCE
>>  	@$(SHELL_PATH) ./GIT-VERSION-GEN
>

← back to recent threads