[PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Aleajndro R Sedeño]

From: Alejandro R. Sedeño <asedeno@mit.edu>

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
---
 git-compat-util.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index b90b64718e..045b47f83a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -189,9 +189,12 @@ struct strbuf;
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
-#if defined(__GNUC__)
+#if GIT_GNUC_PREREQ(4, 5)
 #define UNUSED __attribute__((unused)) \
 	__attribute__((deprecated ("parameter declared as UNUSED")))
+#elif defined(__GNUC__)
+#define UNUSED __attribute__((unused)) \
+	__attribute__((deprecated))
 #else
 #define UNUSED
 #endif
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

On 2022-10-03 at 21:23:18, Aleajndro R Sedeño wrote:
> From: Alejandro R. Sedeño <asedeno@mit.edu>
> 
> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> Signed-off-by: Alejandro R Sedeño <asedeno@google.com>

It might be helpful to explain what system you're targeting when you see
this.  CentOS 7 has GCC 4.8, and I'm not aware of any systems with an
older compiler receiving publicly available updates still.  We've fairly
recently only been testing and targeting GCC 4.8 for that reason.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Alejandro R. Sedeño]

I'm targeting an old SunOS 5.10 with a GCC 3.4.3, for reasons that can
only be described as self-loathing. :-)
The other users of GIT_GNUC_PREREQ are used for 2.8, and 3.1, so I
figure distinguishing between 4.5+ and <4.5 should be well supported.

Regardless, there's no reason to break older compilers over something
that's this trivial to fix.

-Alejandro


On Mon, Oct 3, 2022 at 6:26 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2022-10-03 at 21:23:18, Aleajndro R Sedeño wrote:
> > From: Alejandro R. Sedeño <asedeno@mit.edu>
> >
> > Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> > Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
>
> It might be helpful to explain what system you're targeting when you see
> this.  CentOS 7 has GCC 4.8, and I'm not aware of any systems with an
> older compiler receiving publicly available updates still.  We've fairly
> recently only been testing and targeting GCC 4.8 for that reason.
> --
> brian m. carlson (he/him or they/them)
> Toronto, Ontario, CA

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Jeff King]

On Mon, Oct 03, 2022 at 07:45:44PM -0400, Alejandro R. Sedeño wrote:

> I'm targeting an old SunOS 5.10 with a GCC 3.4.3, for reasons that can
> only be described as self-loathing. :-)
> The other users of GIT_GNUC_PREREQ are used for 2.8, and 3.1, so I
> figure distinguishing between 4.5+ and <4.5 should be well supported.
> 
> Regardless, there's no reason to break older compilers over something
> that's this trivial to fix.

This will cause some mild hardships, as later patches will need to
#define UNUSED in other spots, as well, in order to get full coverage of
the code base (I have written those annotation patches, but they're not
applied upstream yet).

Still, I tend to agree with you that we should handle this case. It's
not too much work, and I should be able to work around the multiple
definitions by pulling it out into an unused.h or similar.

-Peff

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Ævar Arnfjörð Bjarmason]

On Wed, Oct 05 2022, Jeff King wrote:

> On Mon, Oct 03, 2022 at 07:45:44PM -0400, Alejandro R. Sedeño wrote:
>
>> I'm targeting an old SunOS 5.10 with a GCC 3.4.3, for reasons that can
>> only be described as self-loathing. :-)
>> The other users of GIT_GNUC_PREREQ are used for 2.8, and 3.1, so I
>> figure distinguishing between 4.5+ and <4.5 should be well supported.
>> 
>> Regardless, there's no reason to break older compilers over something
>> that's this trivial to fix.
>
> This will cause some mild hardships, as later patches will need to
> #define UNUSED in other spots, as well, in order to get full coverage of
> the code base (I have written those annotation patches, but they're not
> applied upstream yet).

Sorry about any trouble in having to rebase those on UNUSED.

If you're taking requests it would be really useful to prioritize
changes to shared headers and the like, e.g. DEVOPTS=extra-all on pretty
much any file will start with:
	
	git-compat-util.h: In function ‘precompose_argv_prefix’:
	git-compat-util.h:313:54: error: unused parameter ‘argc’ [-Werror=unused-parameter]
	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
	      |                                                  ~~~~^~~~
	git-compat-util.h:313:73: error: unused parameter ‘argv’ [-Werror=unused-parameter]
	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
	      |                                                            ~~~~~~~~~~~~~^~~~
	git-compat-util.h: In function ‘git_has_dos_drive_prefix’:
	git-compat-util.h:423:56: error: unused parameter ‘path’ [-Werror=unused-parameter]
	  423 | static inline int git_has_dos_drive_prefix(const char *path)
	      |                                            ~~~~~~~~~~~~^~~~
	git-compat-util.h: In function ‘git_skip_dos_drive_prefix’:
	git-compat-util.h:431:52: error: unused parameter ‘path’ [-Werror=unused-parameter]
	  431 | static inline int git_skip_dos_drive_prefix(char **path)

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Jeff King]

On Thu, Oct 06, 2022 at 09:29:11AM +0200, Ævar Arnfjörð Bjarmason wrote:

> > This will cause some mild hardships, as later patches will need to
> > #define UNUSED in other spots, as well, in order to get full coverage of
> > the code base (I have written those annotation patches, but they're not
> > applied upstream yet).
> 
> Sorry about any trouble in having to rebase those on UNUSED.

That part was not too bad, and is already done.

The trickiest part is that the headers get included in odd orders, and
if the macros don't match, the compiler will complain (this has to do
with compat/ headers which don't necessarily start by including
git-compat-util.h).

But if the definition gets much more complicated, then it's probably
worth pulling it out rather than repeating it.

> If you're taking requests it would be really useful to prioritize
> changes to shared headers and the like, e.g. DEVOPTS=extra-all on pretty
> much any file will start with:
> 	
> 	git-compat-util.h: In function ‘precompose_argv_prefix’:
> 	git-compat-util.h:313:54: error: unused parameter ‘argc’ [-Werror=unused-parameter]
> 	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
> 	      |                                                  ~~~~^~~~
> 	git-compat-util.h:313:73: error: unused parameter ‘argv’ [-Werror=unused-parameter]
> 	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
> 	      |                                                            ~~~~~~~~~~~~~^~~~
> 	git-compat-util.h: In function ‘git_has_dos_drive_prefix’:
> 	git-compat-util.h:423:56: error: unused parameter ‘path’ [-Werror=unused-parameter]
> 	  423 | static inline int git_has_dos_drive_prefix(const char *path)
> 	      |                                            ~~~~~~~~~~~~^~~~
> 	git-compat-util.h: In function ‘git_skip_dos_drive_prefix’:
> 	git-compat-util.h:431:52: error: unused parameter ‘path’ [-Werror=unused-parameter]
> 	  431 | static inline int git_skip_dos_drive_prefix(char **path)

Yeah, those are near the top of my list. I have a group classified as
"trivial": functions which are compat placeholders and have no body.
I'll be mostly offline for about a week, but I hope to send another
round of unused-mark patches when I get back. (Of course it is not
really useful until _all_ of the patches are there anyway).

-Peff

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Ævar Arnfjörð Bjarmason]

On Thu, Oct 06 2022, Jeff King wrote:

> On Thu, Oct 06, 2022 at 09:29:11AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> > This will cause some mild hardships, as later patches will need to
>> > #define UNUSED in other spots, as well, in order to get full coverage of
>> > the code base (I have written those annotation patches, but they're not
>> > applied upstream yet).
>> 
>> Sorry about any trouble in having to rebase those on UNUSED.
>
> That part was not too bad, and is already done.
>
> The trickiest part is that the headers get included in odd orders, and
> if the macros don't match, the compiler will complain (this has to do
> with compat/ headers which don't necessarily start by including
> git-compat-util.h).
>
> But if the definition gets much more complicated, then it's probably
> worth pulling it out rather than repeating it.

Yeah, I've dealt with that pain before in other contexts. It would be
great to have a git-compiler-compat.h with just the various
__attribute__ stuff split off from git-compat-util.h.

Maybe (compiles, but otherwise untested):

 git-compat-util.h   | 52 +-------------------------------------------------
 git-compiler-util.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 51 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index b90b64718eb..bfa44921c03 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1,18 +1,6 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
-
-#if __STDC_VERSION__ - 0 < 199901L
-/*
- * Git is in a testing period for mandatory C99 support in the compiler.  If
- * your compiler is reasonably recent, you can try to enable C99 support (or,
- * for MSVC, C11 support).  If you encounter a problem and can't enable C99
- * support with your compiler (such as with "-std=gnu99") and don't have access
- * to one with this support, such as GCC or Clang, you can remove this #if
- * directive, but please report the details of your system to
- * git@vger.kernel.org.
- */
-#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
-#endif
+#include "git-compiler-util.h"
 
 #ifdef USE_MSVC_CRTDBG
 /*
@@ -189,13 +177,6 @@ struct strbuf;
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
-#if defined(__GNUC__)
-#define UNUSED __attribute__((unused)) \
-	__attribute__((deprecated ("parameter declared as UNUSED")))
-#else
-#define UNUSED
-#endif
-
 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
 # if !defined(_WIN32_WINNT)
 #  define _WIN32_WINNT 0x0600
@@ -557,37 +538,6 @@ static inline int git_has_dir_sep(const char *path)
 #endif
 #endif
 
-#if defined(__HP_cc) && (__HP_cc >= 61000)
-#define NORETURN __attribute__((noreturn))
-#define NORETURN_PTR
-#elif defined(__GNUC__) && !defined(NO_NORETURN)
-#define NORETURN __attribute__((__noreturn__))
-#define NORETURN_PTR __attribute__((__noreturn__))
-#elif defined(_MSC_VER)
-#define NORETURN __declspec(noreturn)
-#define NORETURN_PTR
-#else
-#define NORETURN
-#define NORETURN_PTR
-#ifndef __GNUC__
-#ifndef __attribute__
-#define __attribute__(x)
-#endif
-#endif
-#endif
-
-/* The sentinel attribute is valid from gcc version 4.0 */
-#if defined(__GNUC__) && (__GNUC__ >= 4)
-#define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))
-/* warn_unused_result exists as of gcc 3.4.0, but be lazy and check 4.0 */
-#define RESULT_MUST_BE_USED __attribute__ ((warn_unused_result))
-#else
-#define LAST_ARG_MUST_BE_NULL
-#define RESULT_MUST_BE_USED
-#endif
-
-#define MAYBE_UNUSED __attribute__((__unused__))
-
 #include "compat/bswap.h"
 
 #include "wildmatch.h"
diff --git a/git-compiler-util.h b/git-compiler-util.h
new file mode 100644
index 00000000000..25fa0bc65d7
--- /dev/null
+++ b/git-compiler-util.h
@@ -0,0 +1,55 @@
+#ifndef GIT_COMPILER_UTIL_H
+#define GIT_COMPILER_UTIL_H
+
+#if __STDC_VERSION__ - 0 < 199901L
+/*
+ * Git is in a testing period for mandatory C99 support in the compiler.  If
+ * your compiler is reasonably recent, you can try to enable C99 support (or,
+ * for MSVC, C11 support).  If you encounter a problem and can't enable C99
+ * support with your compiler (such as with "-std=gnu99") and don't have access
+ * to one with this support, such as GCC or Clang, you can remove this #if
+ * directive, but please report the details of your system to
+ * git@vger.kernel.org.
+ */
+#error "Required C99 support is in a test phase.  Please see git-compiler-util.h for more details."
+#endif
+
+#if defined(__GNUC__)
+#define UNUSED __attribute__((unused)) \
+	__attribute__((deprecated ("parameter declared as UNUSED")))
+#else
+#define UNUSED
+#endif
+
+#endif
+
+#if defined(__HP_cc) && (__HP_cc >= 61000)
+#define NORETURN __attribute__((noreturn))
+#define NORETURN_PTR
+#elif defined(__GNUC__) && !defined(NO_NORETURN)
+#define NORETURN __attribute__((__noreturn__))
+#define NORETURN_PTR __attribute__((__noreturn__))
+#elif defined(_MSC_VER)
+#define NORETURN __declspec(noreturn)
+#define NORETURN_PTR
+#else
+#define NORETURN
+#define NORETURN_PTR
+#ifndef __GNUC__
+#ifndef __attribute__
+#define __attribute__(x)
+#endif
+#endif
+#endif
+
+/* The sentinel attribute is valid from gcc version 4.0 */
+#if defined(__GNUC__) && (__GNUC__ >= 4)
+#define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))
+/* warn_unused_result exists as of gcc 3.4.0, but be lazy and check 4.0 */
+#define RESULT_MUST_BE_USED __attribute__ ((warn_unused_result))
+#else
+#define LAST_ARG_MUST_BE_NULL
+#define RESULT_MUST_BE_USED
+#endif
+
+#define MAYBE_UNUSED __attribute__((__unused__))

>> If you're taking requests it would be really useful to prioritize
>> changes to shared headers and the like, e.g. DEVOPTS=extra-all on pretty
>> much any file will start with:
>> 	
>> 	git-compat-util.h: In function ‘precompose_argv_prefix’:
>> 	git-compat-util.h:313:54: error: unused parameter ‘argc’ [-Werror=unused-parameter]
>> 	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
>> 	      |                                                  ~~~~^~~~
>> 	git-compat-util.h:313:73: error: unused parameter ‘argv’ [-Werror=unused-parameter]
>> 	  313 | static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
>> 	      |                                                            ~~~~~~~~~~~~~^~~~
>> 	git-compat-util.h: In function ‘git_has_dos_drive_prefix’:
>> 	git-compat-util.h:423:56: error: unused parameter ‘path’ [-Werror=unused-parameter]
>> 	  423 | static inline int git_has_dos_drive_prefix(const char *path)
>> 	      |                                            ~~~~~~~~~~~~^~~~
>> 	git-compat-util.h: In function ‘git_skip_dos_drive_prefix’:
>> 	git-compat-util.h:431:52: error: unused parameter ‘path’ [-Werror=unused-parameter]
>> 	  431 | static inline int git_skip_dos_drive_prefix(char **path)
>
> Yeah, those are near the top of my list. I have a group classified as
> "trivial": functions which are compat placeholders and have no body.
> I'll be mostly offline for about a week, but I hope to send another
> round of unused-mark patches when I get back. (Of course it is not
> really useful until _all_ of the patches are there anyway).

I was ad-hoc testing this earlier and just dug this back out of my
stash, maybe going in something like this direction is useful:
	
	diff --git a/config.mak.dev b/config.mak.dev
	index 4fa19d361b7..60bc8c406cf 100644
	--- a/config.mak.dev
	+++ b/config.mak.dev
	@@ -54,6 +54,47 @@ DEVELOPER_CFLAGS += -Wno-empty-body
	 DEVELOPER_CFLAGS += -Wno-missing-field-initializers
	 DEVELOPER_CFLAGS += -Wno-sign-compare
	 DEVELOPER_CFLAGS += -Wno-unused-parameter
	+
	+define use-unused-parameter
	+$(1): DEVELOPER_CFLAGS += -Wunused-parameter
	+
	+endef
	+
	+TEST_BUILTINS_NO_UNUSED =
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-ctype.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-date.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-drop-caches.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-dump-cache-tree.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-dump-fsmonitor.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-dump-split-index.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-dump-untracked-cache.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-example-decorate.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-fsmonitor-client.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-index-version.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-match-trees.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-mergesort.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-oid-array.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-oidmap.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-oidtree.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-oidtree.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-online-cpus.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-parse-options.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-path-utils.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-prio-queue.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-read-graph.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-ref-store.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-run-command.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-scrap-cache-tree.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-sigchain.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-simple-ipc.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-strcmp-offset.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-submodule-config.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-trace2.o
	+TEST_BUILTINS_OBJS_NO_UNUSED += test-xml-encode.o
	+
	+TEST_BUILTINS_OBJS_CHECK_UNUSED = $(filter-out $(TEST_BUILTINS_OBJS_NO_UNUSED),$(TEST_BUILTINS_OBJS))
	+
	+$(eval $(foreach obj,$(TEST_BUILTINS_OBJS_CHECK_UNUSED:%=t/helper/%), $(call use-unused-parameter,$(obj))))
	 endif
	 endif
	 
It's probably too painful to maintain that on a per-file basis, but if
you can get to a point where e.g. t/helper/ is -Wunused-parameter clean
we can just append -Wunused-parameter to DEVELOPER_CFLAGS for those
paths.

That'll ensure that we don't have "regressions" in newly added
parameters for files we've already cleared.

Maybe not worth it, I don't know if we'd be re-adding these at a
sufficient rate to make it worth it, probably you'll send all these in
and we'll find there's maybe 1-5 easily tackled regressions before we
remove that "DEVELOPER_CFLAGS += -Wno-unused-parameter" line.

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Jeff King]

On Thu, Oct 06, 2022 at 11:15:16PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > But if the definition gets much more complicated, then it's probably
> > worth pulling it out rather than repeating it.
> 
> Yeah, I've dealt with that pain before in other contexts. It would be
> great to have a git-compiler-compat.h with just the various
> __attribute__ stuff split off from git-compat-util.h.

I was going to just have unused.h, since I'd worry that piling too much
stuff into it will eventually hit a place where some compat/ code is
unhappy. But I'd also be OK to _start_ with the UNUSED definition, but
call it git-compiler-compat or something, and then people can migrate
things as they choose to test them.

> 	+TEST_BUILTINS_OBJS_NO_UNUSED += test-ctype.o

I'd rather not go that route. I already have an UNUSED-clean code base,
and once that is all merged we shouldn't need any of that complexity. In
the meantime, yes, people introduce new cases, but I am fixing those as
they do (and have been for a few years now).

-Peff

[PATCH] git-compat-util.h: GCC deprecated message arg only in GCC 4.5+

[Aleajndro R Sedeño]

From: Alejandro R. Sedeño <asedeno@mit.edu>

From: https://gcc.gnu.org/gcc-4.5/changes.html

> The deprecated attribute now takes an optional string argument, for
> example, __attribute__((deprecated("text string"))), that will be
> printed together with the deprecation warning.

While GCC 4.5 is already 12 years old, git checks for even older
versions in places. Let's not needlessly break older compilers when
a small and simple fix is readily available.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
---
 git-compat-util.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index b90b64718e..045b47f83a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -189,9 +189,12 @@ struct strbuf;
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
-#if defined(__GNUC__)
+#if GIT_GNUC_PREREQ(4, 5)
 #define UNUSED __attribute__((unused)) \
 	__attribute__((deprecated ("parameter declared as UNUSED")))
+#elif defined(__GNUC__)
+#define UNUSED __attribute__((unused)) \
+	__attribute__((deprecated))
 #else
 #define UNUSED
 #endif
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [PATCH] git-compat-util.h: GCC deprecated message arg only in GCC 4.5+

[Ævar Arnfjörð Bjarmason]

On Wed, Oct 05 2022, Aleajndro R Sedeño wrote:

> From: Alejandro R. Sedeño <asedeno@mit.edu>
>
> From: https://gcc.gnu.org/gcc-4.5/changes.html
>
>> The deprecated attribute now takes an optional string argument, for
>> example, __attribute__((deprecated("text string"))), that will be
>> printed together with the deprecation warning.
>
> While GCC 4.5 is already 12 years old, git checks for even older
> versions in places. Let's not needlessly break older compilers when
> a small and simple fix is readily available.
>
> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
> ---
>  git-compat-util.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index b90b64718e..045b47f83a 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -189,9 +189,12 @@ struct strbuf;
>  #define _NETBSD_SOURCE 1
>  #define _SGI_SOURCE 1
>  
> -#if defined(__GNUC__)
> +#if GIT_GNUC_PREREQ(4, 5)
>  #define UNUSED __attribute__((unused)) \
>  	__attribute__((deprecated ("parameter declared as UNUSED")))
> +#elif defined(__GNUC__)
> +#define UNUSED __attribute__((unused)) \
> +	__attribute__((deprecated))
>  #else
>  #define UNUSED
>  #endif

This LGTM, thanks a lot for that fix & fixing this (minor) breakage of
mine.

I did test on an older GCC myself, and then (in lieu of logging into an
ancient system I have access to) scoured the release notes of gcc, and
discovered that "deprecated" was older than anything we cared about.

But I obviously missed that while the feature had been there for a
longer time, it didn't take this parameter until GCC 4.5, sorry!

Re: [PATCH] git-compat-util.h: GCC deprecated message arg only in GCC 4.5+

[Alejandro R. Sedeño]

Happy to do so! I build new versions of git on a few systems as they
roll out, and you'll find many of my (not that many) patches involve
breakage on this particularly old system. I'm sure one day a big
change will break git for good there, but in the meantime I'll do what
I can to keep it going. :-)

-Alejandro

On Thu, Oct 6, 2022 at 3:33 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
>
> On Wed, Oct 05 2022, Aleajndro R Sedeño wrote:
>
> > From: Alejandro R. Sedeño <asedeno@mit.edu>
> >
> > From: https://gcc.gnu.org/gcc-4.5/changes.html
> >
> >> The deprecated attribute now takes an optional string argument, for
> >> example, __attribute__((deprecated("text string"))), that will be
> >> printed together with the deprecation warning.
> >
> > While GCC 4.5 is already 12 years old, git checks for even older
> > versions in places. Let's not needlessly break older compilers when
> > a small and simple fix is readily available.
> >
> > Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> > Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
> > ---
> >  git-compat-util.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index b90b64718e..045b47f83a 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -189,9 +189,12 @@ struct strbuf;
> >  #define _NETBSD_SOURCE 1
> >  #define _SGI_SOURCE 1
> >
> > -#if defined(__GNUC__)
> > +#if GIT_GNUC_PREREQ(4, 5)
> >  #define UNUSED __attribute__((unused)) \
> >       __attribute__((deprecated ("parameter declared as UNUSED")))
> > +#elif defined(__GNUC__)
> > +#define UNUSED __attribute__((unused)) \
> > +     __attribute__((deprecated))
> >  #else
> >  #define UNUSED
> >  #endif
>
> This LGTM, thanks a lot for that fix & fixing this (minor) breakage of
> mine.
>
> I did test on an older GCC myself, and then (in lieu of logging into an
> ancient system I have access to) scoured the release notes of gcc, and
> discovered that "deprecated" was older than anything we cared about.
>
> But I obviously missed that while the feature had been there for a
> longer time, it didn't take this parameter until GCC 4.5, sorry!

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Alejandro R. Sedeño]

Sorry for the subject line change; I had hoped that --in-reply-to
would get it threaded properly and wanted to trim it down a bit.

-> "[PATCH] git-compat-util.h: GCC deprecated message arg only in GCC 4.5+"

-Alejandro

On Wed, Oct 5, 2022 at 5:59 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > On 2022-10-03 at 21:23:18, Aleajndro R Sedeño wrote:
> >> From: Alejandro R. Sedeño <asedeno@mit.edu>
> >>
> >> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
> >> Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
> >
> > It might be helpful to explain what system you're targeting when you see
> > this.  CentOS 7 has GCC 4.8, and I'm not aware of any systems with an
> > older compiler receiving publicly available updates still.  We've fairly
> > recently only been testing and targeting GCC 4.8 for that reason.
>
> I do agree with you that an update to give "why" in the proposed log
> message would be very welcome, e.g. "Even though such an ancient
> compiler may not matter much in real life, a workaround is simple
> enough." or something along that line.
>
> As to the change in the patch, I think it is good.
>
> Thanks, all.  Expecting an updated version which hopefully would
> become the final version ;-)
>
>

Re: [PATCH] git-compat-util.h: GCC deprecated only takes a message in GCC 4.5+

[Ævar Arnfjörð Bjarmason]

On Mon, Oct 03 2022, brian m. carlson wrote:

> [[PGP Signed Part:Undecided]]
> On 2022-10-03 at 21:23:18, Aleajndro R Sedeño wrote:
>> From: Alejandro R. Sedeño <asedeno@mit.edu>
>> 
>> Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
>> Signed-off-by: Alejandro R Sedeño <asedeno@google.com>
>
> It might be helpful to explain what system you're targeting when you see
> this.  CentOS 7 has GCC 4.8, and I'm not aware of any systems with an
> older compiler receiving publicly available updates still.  We've fairly
> recently only been testing and targeting GCC 4.8 for that reason.

We've had some friendly disagreements in the past about what our policy
should be for supporting such "EOL" software[1][2]. I'm not replying to
repeat any of that discussion, just noting it for context and that I'm
*not* raising that point here :)

What I do want to say to provide others context is that I think you're
conflating your own views & project policy here, e.g. if you run:

	git log --grep='gcc [0-9]' -i

You can see recent patches that have been accepted where we're adjusting
things for GCC's older than 4.8, so it's not the case that we're "only
[...] targeting GCC [versions >=]4.8".

I think you might be mixing up the oldest software we're taking patches
for with the oldest version we run in CI, although I didn't find if we
actually run GCC 4.8 in CI anymore. It appears to have been used with
ubuntu-18.04, but as of ef465848312 (ci: update 'static-analysis' to
Ubuntu 22.04, 2022-08-23) I don't think we pin that version anywhere.

Anyway, however us "in the know" work out what versions we support I
don't want someone to search the ML archive & come to the conclusion
that a patch for a compiler older than whetever IBM's bundling wouldn't
be welcome.

I think it's fair to say that we've taken e.g. major IBM/RedHat releases
into account in the past, e.g. for what libcurl version(s) to support,
per [2]. But we've never considered distro releases to be a hard cut-off
for support.

The actual "policy" has been some fuzzy combination of:

 * What OS's / distro's are used by anyone, for which e.g. RHEL releases
   are a very useful heuristic, and on some platforms (e.g. Windows?)
   vendor EOLs matter more than on others.

 * How much of a hassle older software or a platform is to support,
   e.g. this project has usually happily taken trivial patches such as
   this one, but in the case of PCRE we moved more aggressively from
   PCRE v1 to v2, as supporting both required duplicate code (see
   7599730b7e2 (Remove support for v1 of the PCRE library, 2021-01-24)).

 * That someone cares or sends in patches for. I know of various current
   breakages with software that's "supported" (bits of Solaris, AIX and
   HP/UX userland come to mind), but nobody's cared enough to fix it (in
   those cases, many/all of the "official packages" simply use GNU
   userland/libraries instead)

1. https://lore.kernel.org/git/YPimBp+TkaJ9ycuM@camp.crustytoothpaste.net/
2. https://lore.kernel.org/git/YPimBp+TkaJ9ycuM@camp.crustytoothpaste.net/