[ 
https://issues.apache.org/jira/browse/THRIFT-6187?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jens Geyer updated THRIFT-6187:
-------------------------------
    Description: 
Every run of ./bootstrap.sh emits five autoconf obsolescence warnings. On a 
clean tree at 569e4df2a, with autoconf 2.71 / automake 1.16.5 / libtool 2.4.7 
as shipped by ubuntu-24.04:

{code}
configure.ac:87:  warning: The macro `AC_PROG_LIBTOOL' is obsolete.
configure.ac:91:  warning: AC_PROG_LEX without either yywrap or noyywrap is 
obsolete
configure.ac:603: warning: The macro `AC_HEADER_STDC' is obsolete.
configure.ac:604: warning: The macro `AC_HEADER_TIME' is obsolete.
configure.ac:606: warning: The macro `AC_TYPE_SIGNAL' is obsolete.
{code}

h2. Why this is worth a ticket rather than ignoring

They are not just log noise. The lib-java-kotlin job runs 
{{actions/setup-java}}, which registers a javac problem matcher, and later runs 
{{./bootstrap.sh}}. Autoconf's {{file:line: warning: text}} output happens to 
match that matcher, so GitHub turns the warnings into *file annotations* and 
paints an alert marker next to configure.ac in the "Files changed" view of any 
PR that touches it. Nobody wrote a check for this; it is an accidental 
cross-hit between a Java matcher and autoconf's output, and it makes an 
untouched, unrelated file look like it has findings. It confused a reviewer on 
PR #3800.

h2. The changes

|| line || now || replacement || note ||
| 87 | {{AC_PROG_LIBTOOL}} | {{LT_INIT}} | aclocal/libtool.m4:99 already 
carries {{AU_ALIAS(@<!---->[AC_PROG_LIBTOOL], @<!---->[LT_INIT])}}, so this is 
a rename with no behaviour change. configure.ac:87 is the only call site. |
| 91 | {{AC_PROG_LEX}} | {{AC_PROG_LEX(@<!---->[noyywrap])}} | {{noyywrap}} is 
not a guess: compiler/cpp/src/thrift/thriftl.ll:105 already declares {{%option 
noyywrap}}. |
| 603 | {{AC_HEADER_STDC}} | delete | |
| 604 | {{AC_HEADER_TIME}} | delete | |
| 606 | {{AC_TYPE_SIGNAL}} | delete | |

h2. Three things to check before deleting anything

# *The three deletions are safe because nothing consumes their output.* 
{{STDC_HEADERS}}, {{TIME_WITH_SYS_TIME}} and {{RETSIGTYPE}} appear only inside 
checked-in generated config.h files (lib/cpp/src/thrift/config.h, 
lib/c_glib/src/thrift/config.h, lib/php/src/ext/thrift_protocol/config.h). No 
hand-written source guards on any of them.
# *Do not confuse AC_HEADER_TIME with HAVE_SYS_TIME_H.* That one _is_ used in 
real code -- lib/cpp/src/thrift/server/TServer.cpp:22 and 
lib/cpp/src/thrift/transport/TFDTransport.h:24 both {{#ifdef}} on it. It comes 
from {{AC_CHECK_HEADERS(@<!---->[sys/time.h])}} at configure.ac:642, which 
stays. Deleting AC_HEADER_TIME does not touch it.
# *No AC_PREREQ bump is needed.* The argument form of AC_PROG_LEX was 
introduced in autoconf 2.70, and configure.ac declares {{AC_PREREQ(2.65)}} 
while build/docker/ubuntu-focal still ships autoconf 2.69. Checked: 2.69 
accepts {{AC_PROG_LEX(@<!---->[noyywrap])}}, ignores the argument and generates 
configure normally (exit 0). So the fix is effective on 2.70+ and harmless on 
2.69, and AC_PREREQ can stay where it is.

h2. Out of scope

The same bootstrap run also produces roughly forty automake warnings of the 
form {{test/rb/Makefile.am:36: warning: user target 'distdir' defined here 
...}}, across test/ and tutorial/. Different tool, different cause, much larger 
blast radius -- that belongs in its own ticket if anyone wants it.

h2. Verification

autoconf reports zero warnings for configure.ac after the change, and 
{{./bootstrap.sh && ./configure && make}} still succeeds. Worth confirming on 
ubuntu-focal (autoconf 2.69) as well as noble, since those are the two ends of 
the supported range.

----
Investigated and written with AI assistance (Claude Opus 5). The warning list, 
the three safety checks and the autoconf 2.69 behaviour were all reproduced by 
hand in clean containers.

  was:
Every run of ./bootstrap.sh emits five autoconf obsolescence warnings. On a 
clean tree at 569e4df2a, with autoconf 2.71 / automake 1.16.5 / libtool 2.4.7 
as shipped by ubuntu-24.04:

{code}
configure.ac:87:  warning: The macro `AC_PROG_LIBTOOL' is obsolete.
configure.ac:91:  warning: AC_PROG_LEX without either yywrap or noyywrap is 
obsolete
configure.ac:603: warning: The macro `AC_HEADER_STDC' is obsolete.
configure.ac:604: warning: The macro `AC_HEADER_TIME' is obsolete.
configure.ac:606: warning: The macro `AC_TYPE_SIGNAL' is obsolete.
{code}

h2. Why this is worth a ticket rather than ignoring

They are not just log noise. The lib-java-kotlin job runs 
{{actions/setup-java}}, which registers a javac problem matcher, and later runs 
{{./bootstrap.sh}}. Autoconf's {{file:line: warning: text}} output happens to 
match that matcher, so GitHub turns the warnings into *file annotations* and 
paints an alert marker next to configure.ac in the "Files changed" view of any 
PR that touches it. Nobody wrote a check for this; it is an accidental 
cross-hit between a Java matcher and autoconf's output, and it makes an 
untouched, unrelated file look like it has findings. It confused a reviewer on 
PR #3800.

h2. The changes

|| line || now || replacement || note ||
| 87 | {{AC_PROG_LIBTOOL}} | {{LT_INIT}} | aclocal/libtool.m4:99 already 
carries {{AU_ALIAS(@<!---->[AC_PROG_LIBTOOL], @<!---->[LT_INIT])}}, so this is 
a rename with no behaviour change. configure.ac:87 is the only call site. |
| 91 | {{AC_PROG_LEX}} | {{AC_PROG_LEX(@<!---->[noyywrap])}} | {{noyywrap}} is 
not a guess: compiler/cpp/src/thrift/thriftl.ll:105 already declares {{%option 
noyywrap}}. |
| 603 | {{AC_HEADER_STDC}} | delete | |
| 604 | {{AC_HEADER_TIME}} | delete | |
| 606 | {{AC_TYPE_SIGNAL}} | delete | |

h2. Three things to check before deleting anything

# *The three deletions are safe because nothing consumes their output.* 
{{STDC_HEADERS}}, {{TIME_WITH_SYS_TIME}} and {{RETSIGTYPE}} appear only inside 
checked-in generated config.h files (lib/cpp/src/thrift/config.h, 
lib/c_glib/src/thrift/config.h, lib/php/src/ext/thrift_protocol/config.h). No 
hand-written source guards on any of them.
# *Do not confuse AC_HEADER_TIME with HAVE_SYS_TIME_H.* That one _is_ used in 
real code -- lib/cpp/src/thrift/server/TServer.cpp:22 and 
lib/cpp/src/thrift/transport/TFDTransport.h:24 both {{#ifdef}} on it. It comes 
from {{AC_CHECK_HEADERS(@<!---->[sys/time.h])}} at configure.ac:642, which 
stays. Deleting AC_HEADER_TIME does not touch it.
# *No AC_PREREQ bump is needed.* The argument form of AC_PROG_LEX was 
introduced in autoconf 2.70, and configure.ac declares {{AC_PREREQ(2.65)}} 
while build/docker/ubuntu-focal still ships autoconf 2.69. Checked: 2.69 
accepts {{AC_PROG_LEX(@<!---->[noyywrap])}}, ignores the argument and generates 
configure normally (exit 0). So the fix is effective on 2.70+ and harmless on 
2.69, and AC_PREREQ can stay where it is.

h2. Out of scope

The same bootstrap run also produces roughly forty automake warnings of the 
form {{test/rb/Makefile.am:36: warning: user target 'distdir' defined here 
...}}, across test/ and tutorial/. Different tool, different cause, much larger 
blast radius -- that belongs in its own ticket if anyone wants it.

h2. Verification

autoconf reports zero warnings for configure.ac after the change, and 
{{./bootstrap.sh && ./configure && make}} still succeeds. Worth confirming on 
ubuntu-focal (autoconf 2.69) as well as noble, since those are the two ends of 
the supported range.


> Replace the five obsolete autoconf macros in configure.ac
> ---------------------------------------------------------
>
>                 Key: THRIFT-6187
>                 URL: https://issues.apache.org/jira/browse/THRIFT-6187
>             Project: Thrift
>          Issue Type: Improvement
>          Components: Build Process
>            Reporter: Jens Geyer
>            Priority: Minor
>
> Every run of ./bootstrap.sh emits five autoconf obsolescence warnings. On a 
> clean tree at 569e4df2a, with autoconf 2.71 / automake 1.16.5 / libtool 2.4.7 
> as shipped by ubuntu-24.04:
> {code}
> configure.ac:87:  warning: The macro `AC_PROG_LIBTOOL' is obsolete.
> configure.ac:91:  warning: AC_PROG_LEX without either yywrap or noyywrap is 
> obsolete
> configure.ac:603: warning: The macro `AC_HEADER_STDC' is obsolete.
> configure.ac:604: warning: The macro `AC_HEADER_TIME' is obsolete.
> configure.ac:606: warning: The macro `AC_TYPE_SIGNAL' is obsolete.
> {code}
> h2. Why this is worth a ticket rather than ignoring
> They are not just log noise. The lib-java-kotlin job runs 
> {{actions/setup-java}}, which registers a javac problem matcher, and later 
> runs {{./bootstrap.sh}}. Autoconf's {{file:line: warning: text}} output 
> happens to match that matcher, so GitHub turns the warnings into *file 
> annotations* and paints an alert marker next to configure.ac in the "Files 
> changed" view of any PR that touches it. Nobody wrote a check for this; it is 
> an accidental cross-hit between a Java matcher and autoconf's output, and it 
> makes an untouched, unrelated file look like it has findings. It confused a 
> reviewer on PR #3800.
> h2. The changes
> || line || now || replacement || note ||
> | 87 | {{AC_PROG_LIBTOOL}} | {{LT_INIT}} | aclocal/libtool.m4:99 already 
> carries {{AU_ALIAS(@<!---->[AC_PROG_LIBTOOL], @<!---->[LT_INIT])}}, so this 
> is a rename with no behaviour change. configure.ac:87 is the only call site. |
> | 91 | {{AC_PROG_LEX}} | {{AC_PROG_LEX(@<!---->[noyywrap])}} | {{noyywrap}} 
> is not a guess: compiler/cpp/src/thrift/thriftl.ll:105 already declares 
> {{%option noyywrap}}. |
> | 603 | {{AC_HEADER_STDC}} | delete | |
> | 604 | {{AC_HEADER_TIME}} | delete | |
> | 606 | {{AC_TYPE_SIGNAL}} | delete | |
> h2. Three things to check before deleting anything
> # *The three deletions are safe because nothing consumes their output.* 
> {{STDC_HEADERS}}, {{TIME_WITH_SYS_TIME}} and {{RETSIGTYPE}} appear only 
> inside checked-in generated config.h files (lib/cpp/src/thrift/config.h, 
> lib/c_glib/src/thrift/config.h, lib/php/src/ext/thrift_protocol/config.h). No 
> hand-written source guards on any of them.
> # *Do not confuse AC_HEADER_TIME with HAVE_SYS_TIME_H.* That one _is_ used in 
> real code -- lib/cpp/src/thrift/server/TServer.cpp:22 and 
> lib/cpp/src/thrift/transport/TFDTransport.h:24 both {{#ifdef}} on it. It 
> comes from {{AC_CHECK_HEADERS(@<!---->[sys/time.h])}} at configure.ac:642, 
> which stays. Deleting AC_HEADER_TIME does not touch it.
> # *No AC_PREREQ bump is needed.* The argument form of AC_PROG_LEX was 
> introduced in autoconf 2.70, and configure.ac declares {{AC_PREREQ(2.65)}} 
> while build/docker/ubuntu-focal still ships autoconf 2.69. Checked: 2.69 
> accepts {{AC_PROG_LEX(@<!---->[noyywrap])}}, ignores the argument and 
> generates configure normally (exit 0). So the fix is effective on 2.70+ and 
> harmless on 2.69, and AC_PREREQ can stay where it is.
> h2. Out of scope
> The same bootstrap run also produces roughly forty automake warnings of the 
> form {{test/rb/Makefile.am:36: warning: user target 'distdir' defined here 
> ...}}, across test/ and tutorial/. Different tool, different cause, much 
> larger blast radius -- that belongs in its own ticket if anyone wants it.
> h2. Verification
> autoconf reports zero warnings for configure.ac after the change, and 
> {{./bootstrap.sh && ./configure && make}} still succeeds. Worth confirming on 
> ubuntu-focal (autoconf 2.69) as well as noble, since those are the two ends 
> of the supported range.
> ----
> Investigated and written with AI assistance (Claude Opus 5). The warning 
> list, the three safety checks and the autoconf 2.69 behaviour were all 
> reproduced by hand in clean containers.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to