[
https://issues.apache.org/jira/browse/THRIFT-6270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116258#comment-18116258
]
Jens Geyer commented on THRIFT-6270:
------------------------------------
Sweep done against master (75af40976). The build lists of every binding and of
the compiler were compared with the tracked source files, and each file that
turned up was then read. The PHP finding was fixed under this ticket (PR
#3870); the remaining findings are filed as THRIFT-6284 (Lua, D {{idlgen}}, and
the unlisted Python test) and THRIFT-6285 (the Android Gradle build).
h3. Files a build list leaves out
* *PHP:* five files under {{lib/php/lib}} are in no autotools install list:
{{StoredMessageProtocol.php}},
{{Factory/TBinaryProtocolAcceleratedFactory.php}},
{{Server/TSSLServerSocket.php}}, {{Transport/TPsrHttpClient.php}} and
{{Transport/TSSLSocket.php}}.
** {{make install}} leaves them out, although an installed
{{TMultiplexedProcessor}} needs {{StoredMessageProtocol}}.
** Composer and the release tarball have them.
** PR #3870 adds them. It also adds a {{check-install-lists}} target and runs
it in the lib-php CI job.
* *Lua:* none of the twelve {{lib/lua/*.lua}} modules is installed. {{make
install}} delivers only the C libraries, and the cross test loads the modules
from the source tree. → THRIFT-6284.
* *D:* {{lib/d/src/thrift/codegen/idlgen.d}} was taken out of the autotools
module list in 2014 (THRIFT-2781, "temporarily") and never put back. The dub
build still compiles it. → THRIFT-6284.
* *Java/Android:* {{lib/java/android/build.gradle}} points at {{../src}}. It
excludes three files by paths that no longer exist since THRIFT-5545 moved the
sources to {{src/main/java}}. → THRIFT-6285 (filed for a decision —
unmaintained, no CI).
h3. Nothing else missing
* *File lists compared:* cpp, c_glib, the compiler, d, lua, php, perl, ocaml,
st, delphi, dart, rs, rb and nodejs. Apart from the files above, the only files
outside a list are expected ones:
** The Windows-only sources in {{lib/cpp}}, which the CMake Windows branch
builds.
** {{compiler/cpp/src/thrift/logging.cc}}, which is left out on purpose.
* *Directory-based builds, nothing to compare:* go, java, netstd, erl, haxe, py
and js.
** py lists its packages rather than its files, so a new subpackage would be
dropped silently.
* *Could not be checked:*
** javame has no build.
** The cl sources are fetched at build time.
** kotlin and nodets have no library source of their own.
** json and xml hold schemas only.
A single check for all bindings is not practical, because each binding chooses
its files differently. Where a list is explicit, a check next to that list is
cheap; the PHP one is the first.
h3. Outside this ticket
* {{lib/py/test/test_sasl_transport.py}} is in no test list, so CI never runs
it. → THRIFT-6284.
* {{lib/py/test/test_immutable_exception.py}} runs under CMake only.
* {{lib/d/test/Makefile.am}} names {{async_test_client}}, but the target is
{{async_test}}.
_Drafted with AI assistance (Claude Opus 4.8); reviewed and posted by Jens
Geyer._
> Sweep for source files that no build list mentions
> --------------------------------------------------
>
> Key: THRIFT-6270
> URL: https://issues.apache.org/jira/browse/THRIFT-6270
> Project: Thrift
> Issue Type: Task
> Components: Build Process, Test Suite
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> {{lib/d/src/thrift/transport/websocket.d}} was in the tree for five releases
> without appearing in any module list in {{lib/d/Makefile.am}}. It was
> therefore never built, never tested and never installed, and the omission
> only surfaced when someone tried to add a test for it -- the file had to be
> wired into the build before it could be compiled at all.
> Nothing checks for this. A source file that no build list names produces no
> warning from either build system; it simply is not there.
> The related case on the *test* side is already filed as THRIFT-6217 (test
> programs in an autotools {{check_PROGRAMS}} but absent from
> {{CMakeLists.txt}}). This issue is about library sources rather than test
> programs, and about a file being in no list at all rather than in one of two.
> h2. Suggested
> # One pass across the bindings comparing the source files on disk against the
> file lists in {{Makefile.am}} / {{CMakeLists.txt}} / the binding's own
> manifest, and wire up or delete whatever is unreferenced.
> # If that turns up more than the one known case, a small check in the build
> worth keeping -- the comparison is mechanical.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
