Jens Geyer created THRIFT-6188:
----------------------------------
Summary: Drop the redundant distdir overrides from Makefile.am (75
of 85 are no-ops)
Key: THRIFT-6188
URL: https://issues.apache.org/jira/browse/THRIFT-6188
Project: Thrift
Issue Type: Improvement
Components: Build Process
Reporter: Jens Geyer
85 Makefile.am files carry a hand-written override of automake's {{distdir}}
target, all with the same body:
{code}
distdir:
$(MAKE) $(AM_MAKEFLAGS) distdir-am
{code}
{{automake -Wall}} reports 84 of these as overrides:
{code}
test/rb/Makefile.am:36: warning: user target 'distdir' defined here ...
/usr/share/automake-1.16/am/distdir.am: ... overrides Automake target 'distdir'
defined here
{code}
h2. First, what this is not
It is *not* a {{make dist}} bug, and it is *not* the source of the CI
annotations described in THRIFT-6187. Plain {{automake}} -- which is what
./bootstrap.sh runs -- reports *zero* of these. They appear only under
{{automake -Wall}} or {{autoreconf -Wall}}, which nothing in the build or in CI
uses.
{{make dist}} also works. Running the make-dist workflow's own steps on a clean
tree at 569e4df2a produced thrift-0.25.0.tar.gz with 3516 entries, including
the generated parser sources compiler/cpp/src/thrift/thrifty.cc and thriftl.cc
-- those arrive through {{distdir-am: $(DISTFILES)}} regardless of the override.
h2. What the override actually changes
automake's own rule in am/distdir.am is byte-identical to the override apart
from one prerequisite:
{code}
if %?DIST_BUILT_SOURCES%
distdir:
$(MAKE) $(AM_MAKEFLAGS) distdir-am
else !%?DIST_BUILT_SOURCES%
distdir: $(BUILT_SOURCES)
$(MAKE) $(AM_MAKEFLAGS) distdir-am
endif !%?DIST_BUILT_SOURCES%
{code}
So dropping {{$(BUILT_SOURCES)}} is the whole of it -- which is exactly what it
was for. The overrides come from 63f04583e (2024-03-21), whose commit message
is the entire rationale: _"Manually remove $(BUILT_SOURCES) since
no-dist-built-sources doesnt work"_.
h2. 75 of the 85 do nothing at all
Only ten of the 85 overriding files define BUILT_SOURCES:
{code}
compiler/cpp/src/Makefile.am lib/c_glib/test/Makefile.am
test/py/Makefile.am
lib/cpp/test/Makefile.am lib/perl/Makefile.am
tutorial/c_glib/Makefile.am
test/cpp/Makefile.am test/go/Makefile.am
tutorial/cpp/Makefile.am
tutorial/dart/Makefile.am
{code}
In the other 75 the override is a pure no-op. Checked on automake 1.16.5: a
Makefile.am with no BUILT_SOURCES still gets {{distdir: $(BUILT_SOURCES)}}
generated, and BUILT_SOURCES is never defined in the resulting Makefile.in, so
the prerequisite list expands to nothing. Deleting the override in those 75
files produces an identical rule.
So: *delete the 75, keep the 10.* The ten still carry the intent of 63f04583e
and each deserves a one-line comment saying so, since it is not obvious from
the code why the same three lines matter in ten places and are noise in
seventy-five.
h2. The option is still broken, so the workaround has to stay
Worth recording, because it is the obvious thing a reviewer will suggest.
{{no-dist-built-sources}} cannot replace these overrides on any image the
project builds:
* *automake 1.16.5* (ubuntu-jammy, ubuntu-noble): the option is accepted and
has *no effect*. Tested in {{AM_INIT_AUTOMAKE}}, in {{AUTOMAKE_OPTIONS}}, and
in both at once -- all four still generate {{distdir: $(BUILT_SOURCES)}}. The
2024 commit message is accurate.
* *automake 1.16.1* (ubuntu-focal): hard error, {{configure.ac:2: error: option
'no-dist-built-sources' not recognized}}. The option only arrived in automake
1.16.3, and configure.ac declares {{AM_INIT_AUTOMAKE(\[1.13 subdir-objects
tar-ustar foreign])}}.
h2. Loose ends
* 85 files carry the override but automake warns about 84. The missing one is
contrib/fb303/py/Makefile.am, which belongs to contrib/fb303's own configure.ac
and is not part of the top-level automake run.
* The same {{-Wall}} run reports three {{install}}, two {{clean}} and one
{{uninstall}} user-target overrides (tutorial/d, test/erl, contrib/fb303/py
among them). Same family, not touched here.
----
Investigated and written with AI assistance (Claude Opus 5). Every count, the
automake rule comparison, the no-dist-built-sources behaviour on 1.16.1 and
1.16.5, and the make dist run were all reproduced by hand in clean containers.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
