Re: Proposed changes to wesnoth-1.16

On 2024-01-05 at 01:24, Vincent Cheng wrote:
> On Thu, Jan 4, 2024 at 7:36 AM P. J. McDermott <[email protected]> wrote:
> > On 2023-12-31 at 01:09, Vincent Cheng wrote:  
> > > I took a quick glance at the diff. My personal threshold for repacking
> > > source tarballs is higher than that of most other maintainers. I'm
> > > inclined to not repack unless otherwise necessary (i.e. there's
> > > non-DFSG-compatible stuff that needs to be removed). When it comes to
> > > minified js, I'd rather just store a non-minified copy in
> > > debian/missing-sources, leave the source tarball untouched, suppress
> > > the lintian warning and move on.  
> >
> > That addresses the missing sources (assuming the source and minified
> > versions match) but doesn't actually solve the embedded code copies
> > issue (the reason lintian warns about "embedded-javascript-library").
> > The JS files that are installed should be replaced with symlinks to
> > system copies (and dependencies added), as my commit did.  
> 
> I agree with replacing embedded code copies for things like Lua and
> other system libraries (generally well known libraries with stable
> APIs and ABIs), but I also think it's futile to do so specifically for
> embedded JS (as a side tangent, I've also mostly given up on packaging
> web applications for this reason). Wesnoth does not have a
> particularly large JS footprint, but even then I don't want to be on
> the hook for sanity checking whether the addon manager is still
> functional each time jquery gets updated in Debian/Ubuntu. I'm
> perfectly happy to ignore lintian's nagging here.

Having never intentionally (only once as required in a course) written
JS code using jQuery (I can do in CSS or JS with few or no dependencies
what others do with jQuery) I wasn't aware its API was so unstable that
Debian's libjs-jquery and related packages are apparently functionally
useless.  I guess I take things like SONAMEs for granted.

> > And like I said, note that 1.18 will also have src/modules/lua/ without
> > local changes, so it can finally be replaced with the system copy of Lua
> > 5.4.  Repacking is generally recommended to make sure the system copy is
> > used and thus there's no chance the embedded copy is accidentally used,
> > hiding from efforts to fix a security vulnerability across all copies in
> > the Debian archive (or worse, in Ubuntu where it will more likely
> > never get fixed).  But I guess you prefer not to repack to remove ECCs
> > (whether jQuery+tablesorter or Lua)?  
> 
> If you don't trust the build system used by the package and want to
> ensure system build-deps are used, you can simply rm -rf the embedded
> code copies in the package's clean target (dpkg-source ignores removed
> files so that's generally safe to do). I think this is a lesser evil
> vs repacking the upstream source tarball, and I'm not convinced it's
> appropriate or necessary to do so for wesnoth, sorry.

dpkg-source will ignore but also complain in the build log about every
single removed file, but otherwise sure, that works.  To be clear,
you're not convinced repacking is necessary, but you'd be OK with
removing the Lua ECC in the clean target?  Or you're not convinced
either is necessary?

I certainly don't trust the build system, because there's currently
nothing *to* trust.  Wesnoth 1.17/1.18's build systems (CMake and SCons)
don't even have an option to use a system copy of Lua 5.4.

Upstream wants Lua compiled as C++ [1][2] so that it can recover from
C++ exceptions [3][4].  Lua's build system doesn't compile as C++,
but Debian does [5].  As does Arch [6] but not Fedora [7].  Wesnoth also
still wants some compile-time definitions [8], although the LUA_COMPAT_*
ones are from Lua 5.3 and are unused in 5.4, so that leaves the Windows
strcoll() redefinition and exception rethrowing.

So since Lua compiled as C++ isn't (yet) universal among distributions
or Lua itself, and Wesnoth still wants some compile-time changes,
upstream might not be willing to support using system Lua (even though
they did before 1.9.0), especially this late in their development cycle
while they're understandably focused on getting a release ready.  But
I've developed and submitted [9] a patch series to add support to both
build systems for using system Lua 5.4 C++ on non-Windows platforms.

In the meantime, actually the only way to use system Lua 5.4 C++ without
a large patch series is to remove the ECC, replace it with header
symlinks, and hack around the build system:

debian/clean:
+src/modules/lua/

debian/rules:
+include /usr/share/dpkg/buildtools.mk
 override_dh_auto_configure:
+	1>source_lists/lua
+	mkdir -p src/modules/lua/.git/
+	for f in /usr/include/lua5.4/*.h; do \
+		ln -sf "$${f}" "src/modules/lua/$${f##*/}"; \
+	done
 	dh_auto_configure -- $(CMAKE_SWITCHES)
+	sed -i "s|\$$| $$($(PKG_CONFIG) --libs lua5.4-c++)|" \
+		build/src/CMakeFiles/wesnoth.dir/link.txt

(I don't like having to modify CMake's link command with sed, but CMake
apparently doesn't have anything like Automake's LIBS, and of course
LDFLAGS would put -l flags in the wrong place.)

[1]: https://github.com/wesnoth/wesnoth/blob/9315079/src/CMakeLists.txt#L90
[2]: https://github.com/wesnoth/wesnoth/blob/ac259f0/src/SConscript#L124
[3]: https://github.com/wesnoth/wesnoth/commit/e4d0ae0
[4]: https://github.com/lua/lua/blob/9be74cc/ldo.c#L47
[5]: https://salsa.debian.org/lua-team/lua5.2/-/commit/fa2dc77c
[6]: https://gitlab.archlinux.org/archlinux/packaging/packages/lua/-/commit/4e97e19d
[7]: https://src.fedoraproject.org/rpms/lua/blob/rawhide/f/lua.spec
[8]: https://github.com/wesnoth/wesnoth/blob/c44cc27/src/wesnoth_lua_config.h
[9]: https://github.com/wesnoth/wesnoth/pull/8234

> We could also just drop src:wesnoth-1.16 entirely at this point (we
> can just file an RM request to have it be removed from sid, and if
> it's done before Debian Import Freeze then it'll automatically get
> removed from Ubuntu Noble as well).

I don't think we should RM src:wesnoth-1.16 until after noble's Debian
Import Freeze (and better yet, after the release of 1.16.12).  After
discussion with upstream [10], I personally think the best option
overall is to have both versions in noble.

As you know, whatever version is in Ubuntu at the Debian Import
Freeze remains through the release's lifetime (except for the PPA you
maintain), since I think no one on this team is an Ubuntu MOTU team
member who could maintain this team's packages in Ubuntu universe
(someone please correct me if I'm wrong).

So if severe bugs are found in 1.17.26, 1.16.11 could be a less buggy
fallback option for Ubuntu users for at least single-player games (the
multiplayer server for 1.16 will clear out pretty soon after the 1.18.0
release).

Either way, of course we shouldn't RM src:wesnoth-1.16 before
src:wesnoth-1.18 leaves NEW, because then we run the risk of the latter
taking too long in NEW and noble having no wesnoth packages at all.

[10]: https://github.com/wesnoth/wesnoth/issues/8187

> The plan sounds fine to me, thanks for volunteering to tackle this!
> I'm happy to sponsor/upload wesnoth when it's ready, feel free to ping
> me and I'll try to get back ASAP.

Thanks for sanity checking the plan and for the sponsorship offer!  When
I have things ready to upload, should I send mail To: you and Cc: the
list or just send To: the list?  I'm asking what's more convenient for
you, in case you filter and don't always read list mail but do read mail
sent To: you.

And, do you have any thoughts on the campaigns dependency/AppStream
bug [11]?  The issue is that wesnoth-1.16-core provides the executable
and therefore also the .desktop and .appdata.xml files, but since
the campaigns are split out into individual packages and only the
wesnoth-1.16 metapackage depends on them, users only get the executable
without any campaigns when installing through "software center" package
manager frontends that find application packages via AppStream data.

Rhonda suggested some kind of manual AppStream data adjustments, but I
can't find anything in the AppStream specification about adding package
dependencies or anything else relevant, so I don't know what she means.

Would you object to making wesnoth-*-core recommend all the campaign
packages?  I found a package in Debian with AppStream metadata and
Recommends and tested that gnome-software with default APT configuration
does indeed install the Recommends packages.  Or do you have any other
suggestions?

[11]: https://bugs.launchpad.net/ubuntu/+source/gnome-software/+bug/2033222

-- 
Patrick "P. J." McDermott:  http://www.pehjota.net/
Lead Developer, ProteanOS:  http://www.proteanos.com/
Founder and CEO, Libiquity: http://www.libiquity.com/