I updated Emscripten recently, to version 3.1.54. The result was that
none of the WASM puzzles run any more, because they produce a stack
trace at startup with the error message "to do setValue(i64) use
WASM_BIGINT".
I don't fully understand this. The stack trace comes via a JS wrapper
around a WASM-compiled function called __main_argc_argv, which sounds
like something in the Emscripten library startup. Somewhere in there
it goes via _js_get_date_64(), which tries to write an i64 into the
WASM memory array, which hits this abort in setValue().
Web searching for the error message turned up
https://github.com/godotengine/godot/pull/88594 which gave me the clue
that '-s WASM_BIGINT' is a command-line setting for the Emscripten
linker. And indeed, setting that makes the startup-time error go away
and the puzzles run again. But it also causes older versions of all
browsers to be unsupported, presumably on the grounds that they don't
have whatever WASM bigint feature this flag causes the code to use.
I don't really understand what's going on here. I assume
_js_get_date_64 is being called to handle a 64-bit time_t. But the
Puzzles code doesn't work with time_t at all, so this is entirely in
the Emscripten standard library. And if the pre-main() startup code
needs a 64-bit integer write, which won't work without this flag, then
surely _nothing_ would work without this flag, and surely someone
would have noticed that, and made that flag the default? This all
seems confusing and I wonder if I've misunderstood something.
However, if I don't fix it in _some_ way, the web puzzles will stay
out of action for days and I'll get lots of email complaints. So
here's something that makes them basically work again, and maybe we
can figure out the rest of the story later.
This includes the textual version number in its existing
form (yyyymmdd followed by an abbreviated git hash). The four-part
binary version is set to 1 followed by year, month and day; if I ever
want to change that, I can increment the initial 1.
FileDescription is taken from the existing DESCRIPTION string provided
to each puzzle() statement in CMakeLists.txt.
This means that puzzles.rc now always defines at least one resource,
so we can remove the workaround for MinGW's windres not being able to
cope with an empty .rc file, which added a dummy resource in the
absence of an icon.
There's no reason to put the .rc file into developer tools like
galaxieseditor at all. Its current job is to add an icon, and those
tools don't have any. I'm about to add version information, and they
won't have that either (in particular, no description string like the
games do).
The CLI developer tools already don't include puzzles.rc, and GUI dev
tools are more like those than they are like puzzles.
puzzles.rc was being added to an aux GUI tool's source file list by
get_platform_puzzle_extra_source_files(), which is called for aux GUI
tools as well as for puzzles proper. However, it's not as simple as
just eliminating that call, because on Unix, we _do_ need to add the
same extra source files to GUI dev tools that we do for puzzles,
because gtk.c contains external references to either an array of the
puzzle's icons or an empty array indicating that there aren't any, so
_something_ has to provide that.
So instead, get_platform_puzzle_extra_source_files now takes an extra
argument saying whether the program is a real puzzle or an aux tool;
windows.cmake leaves out puzzles.rc in the latter case, but unix.cmake
puts the icon array in unconditionally.
That way, any application displaying the .desktop with its icon will
pick the icon size it deems the best one for the current rendering.
See the Icon Theme Specification:
https://specifications.freedesktop.org/icon-theme-spec/latest/ar01s07.html
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
Now using the browser's "copy" operation while the focus is in the
puzzle will copy the puzzle state to the clipboard. Browsers seem to
have odd ideas about whate element to target with the "copy" event:
Firefox targets the parent of the <canvas> while Chromium targets the
<body>. To cope with these and possible future weirdness I attach the
event handler to the document and then look to see if it's plausibly
related to the canvas.
Arguably we might want to handle a wider range of "copy" events, maybe
any where the selection isn't empty. I'm not sure, though, so we'll
start with the minimal change.
The C stack used by Emscripten is quite small, so passing more than a
few klilobytes of data on it tends to cause an overflow. Current
versions of puzzles will only generate tiny preferences files, but this
might change in future and in any case Puzzles shouldn't crash just
because the preferences in local storage have got corrupted.
To fix this, we now have JavaScript allocate a suitable amount of C heap
memory using malloc() and stick the preferences file in there.
This could plausibly fail if the preferences file were really big, but
that's unlikely since browsers generally limit an origin to about 5 MB
of local storage. In any case, if malloc() does fail, we'll just ignore
the preferences file, which is probably the right thing to do.
Here, user preferences are stored in localStorage, so that they can
persist when you come back to the same puzzle page later.
localStorage is global across a whole web server, which means we need
to take care to put our uses of it in a namespace reasonably unlikely
to collide with unrelated web pages on the same server. Ben suggested
that a good way to do this would be to store things in localStorage
under keys derived from location.pathname. In this case I've appended
a fragment id "#preferences" to that, so that space alongside it
remains for storing other things we might want in future (such as
serialised saved-game files used as quick-save slots).
When loading preferences, I've chosen to pass the whole serialised
preferences buffer from Javascript to C as a single C string argument
to a callback, rather than reusing the existing system for C to read
the save file a chunk at a time. Partly that's because preferences
data is bounded in size whereas saved games can keep growing; also
it's because the way I'm storing preferences data means it will be a
UTF-8 string, and I didn't fancy trying to figure out byte offsets in
the data on the JS side.
I think at this point I should stop keeping a list in the docs of
which frontends support preferences. Most of the in-tree ones do now,
and that means the remaining interesting frontends are ones I don't
have a full list of. At this moment I guess no out-of-tree frontends
support preferences (unless someone is _very_ quick off the mark), but
as and when that changes, I won't necessarily know, and don't want to
have to keep updating the docs when I find out.
Emscripten has settings indicating which browser versions it should
build code for. These are now by default slightly newer than I'd been
targeting with my hand-written JavaScript. They also don't include
Firefox 48, which KaiOS 2.5 is based on.
This commit adds CMake variables to set the minimum versions that we
pass to Emscripten. They default to the earliest versions with
WebAssembly support, except that Firefox 48 is also supported.
I think the main consequence of this change is to stop Emscripten using
sign-extension and mutable-globals in WebAssembly, which it's done by
default since version 3.1.26.
Puzzles only runs in Web browsers, so there's no need to include
support for Node or (for now at least) running in a Web worker. This
removes about 5 kiB of code from the boilerplate JavaScript.
The KaiOS build includes compiled versions of various Emscripten library
files. These are generally under the MIT licence like Puzzles itself.
The MIT licence requires that the licence, and the copyright notice, be
"included in all copies or substantial portions of the Software."
Since each KaiOS package includes the full manual, which already
contains the licence for Puzzles itself, adding the copyright notices
there seems like the best approach. I've done this by providing an
additional input file that contains the licences for source files used
by a current Emscripten build. More automation might be nice, but the
set of copyright notices is unlikely to change very much. There are
basically one for Emscripten, one for musl, and a few for odd bits of
third-party code embedded in musl.
After Ben fixed all the unwanted global functions by using gcc's
-Wmissing-declarations to spot any that were not predeclared, I
remembered that clang has -Wmissing-variable-declarations, which does
the same job for global objects. Enabled it in -DSTRICT=ON, and made
the code clean under it.
Mostly this was just a matter of sticking 'static' on the front of
things. One variable was outright removed ('verbose' in signpost.c)
because after I made it static clang was then able to spot that it was
also unused.
The more interesting cases were the ones where declarations had to be
_added_ to header files. In particular, in COMBINED builds, puzzles.h
now arranges to have predeclared each 'game' structure defined by a
puzzle backend. Also there's a new tiny header file gtk.h, containing
the declarations of xpm_icons and n_xpm_icons which are exported by
each puzzle's autogenerated icon source file and by no-icon.c. Happily
even the real XPM icon files were generated by our own Perl script
rather than being raw xpm output from ImageMagick, so there was no
difficulty adding the corresponding #include in there.
Ben added -Wmissing-declarations in commit 3a3e491a8bc624e for gcc
builds, and observed that clang's option of the same name doesn't do
the same job. But clang does _have_ an option to do the same job: it's
just spelled differently. Added -Wmissing-prototypes in clang builds,
so that those will check the same thing.
Clang's -Wmissing-declarations means something quite different, so we
only enable it on GCC. This is slightly silly since Clang's
-Wmissing-declarations is enabled by default, but it makes it clear that
we know they're different.
Rather than installing the documentation only when Halibut is
available, install the documentation whenever it exists. This allows
for the way that Buildscr injects the documentation into official
KaiOS builds.
The Kai Store makes display of advertisements provided by the KaiAds API
mandatory. I don't want such adverts to be inconvenient for the users,
so I've just gone for adding a menu item that will display one. This is
probably a little too crude, but it's good for testing things.
The actual KaiAds API code is not free software, so it's not included
here. My intention is to add it by hand to the Zip files for Kai Store
uploads. Without it, the advertising code does nothing.
Previously, we initialised all of the JavaScript event handlers as soon
at the DOM was loaded, and then called main() ourselves once the
Emscripten runtime was ready. This was slightly dangerous since it
depended on none of those event handlers' being called before main().
In practice this was difficult because most of the elements the event
handlers were attached to were invisible, but it did limit what event
handlers could safely be used.
Now, the event handlers are initialised from main(). This makes things
work in a sufficiently conventional way that we can just let the
Emscripten run-time call main() in its usual way, rather than involving
ourselves in the minutiae of Emscripten's startup.
KaiOS (which is based on Firefox OS, formerly Boot to Gecko) runs its
"native" apps in a Web browser, so this is essentially a rather
specialised version of the JavaScript front-end. Indeed, the JavaScript
and C parts are the same as the Web version.
There are three major parts that are specific to the KaiOS build.
First, there's manifest.pl, which generates a KaiOS-specific JSON
manifest describing each puzzle.
Second, there's a new HTML page generator, apppage.pl, that generates an
HTML page that is much less like a Web page, and much more like an
application, than the one generated by jspage.pl. It expects to build a
single HTML page at a time and gets all its limited knowledge of the
environment from its command line. This makes it gratuitously different
from jspage.pl and javapage.pl, but makes it easier to run from the
build system.
And finally, there's the CMake glue that assembles the necessary parts
for each application in a directory. This includes the manifest, the
HTML, the JavaScript, the KaiOS-specific icons (generated as part of the
GTK build) and a copy of the HTML documentation. The directory is
assembled using CMake's install() function, and can be installed on a
KaiOS device using the developer tools.
This adds a new callback, rescale_puzzle(), that's called when the
device pixel ratio changes. This means that resize_puzzle() can safely
set the nominal canvas size, which means that manual resizing of the
puzzle now sticks.
Still missing: paying attention to the device pixel ratio when choosing
the initial (or reset) size.
- Generate HTML pages from the manual, and install them
- Add "Contents" and "Help on <name>" menu items that will open the
appropriate page in a web browser
I've had a lot of Puzzles nightly builds fail recently in the NestedVM
stage, with a 'jar' command producing a message along these lines:
java.util.zip.ZipException: attempt to write past end of STORED entry
at java.base/java.util.zip.ZipOutputStream.write(ZipOutputStream.java:337)
at jdk.jartool/sun.tools.jar.Main.copy(Main.java:1250)
at jdk.jartool/sun.tools.jar.Main.copy(Main.java:1263)
at jdk.jartool/sun.tools.jar.Main.addFile(Main.java:1211)
at jdk.jartool/sun.tools.jar.Main.create(Main.java:879)
at jdk.jartool/sun.tools.jar.Main.run(Main.java:319)
at jdk.jartool/sun.tools.jar.Main.main(Main.java:1681)
Suppressed: java.util.zip.ZipException: invalid entry size (expected 0 but got 786 bytes)
at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:288)
at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:361)
at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:238)
at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:378)
at jdk.jartool/sun.tools.jar.Main.create(Main.java:854)
... 2 more
It's hard to work out exactly what this error dump means, and
web-searching for the error message isn't much help because the same
exception can occur in application code using java.util.zip, and most
mentions on the web are about that, and not about what I want to know,
which is why it might happen in the 'jar' program in particular.
However, the clues visible in that message suggest that 'jar' had
somehow got confused about the size of one of the files it was adding
to the jar archive, in that it initially decided it was 0 bytes long
and later found it was longer. That suggests a problem of excessive
parallelism between the build steps, perhaps due to a missing
dependency in the makefile, which might plausibly cause the 'jar' step
to be running already while some file it needs to read is still being
written. (Which would also explain why it doesn't happen every time.)
An eyeball review of cmake/platforms/nestedvm.cmake didn't find any
obvious missing dependencies. But I vaguely remembered that in some
other context I'd had trouble with cmake 'add_custom_command'. So in
this commit I replace all those custom commands with custom _targets_,
listing the previous OUTPUT files as BYPRODUCTS. And then the
dependencies are written using the target names, instead of the file
names.
I don't fully understand why this should make a difference. But it
seems more reliable in a soak test, and still builds the right things,
so I'll commit it and see if it makes the flakiness in the actual
nightly builds stop happening.
These look like puzzles, in that they link against a frontend and
provide the usual 'struct game', but they don't count as a puzzle for
purposes of shipping, or even having to have descriptions and icons.
There's one of these buried in the code already under an ifdef, which
I'll re-enable in the next commit.
In this platform's set_platform_puzzle_target_properties, I referred
to ${EXENAME} twice, which is not one of the function parameters. It
worked anyway, because CMake has dynamic scope, and that variable was
defined - to the right value - within the local-variable scope of the
calling function. But that wasn't at all what I meant to do!
Renamed it to ${TARGET}, which is the actual parameter name we get
passed.
Using a stunt webserver which artificially introduces a 3s delay just
before the last line of the HTML output, I have reproduced a
uwer-reported loading/startup race bug:
Previously the wasm loading was started by the <script> element,
synchronously. If the wasm loading is fast, and finishes before the
HTML loading, the onRuntimeInitialized event may occur before
initPuzzles. But initPuzzles sets up the event handler.
Fix this bug, and introduce a new comment containing an argument for
the correctness of the new approach.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
How embarrassing. When I updated the Emscripten build to use WASM, a
major reason I bothered to do it at all was that I'd heard that WASM
was capable of reallocating its memory arena larger on the fly. Turns
out that it _can_, but only if you specifically set the option in
Emscripten to allow it.
With this option set, I can finish a 25x25 Galaxies, where previously
the game would crash part way through (and not even a very large part)
with errors about memory growth in the Javascript console.
Various cmake variables that I was informally expecting users to set
on the cmake command line (e.g. cmake -DSTRICT=ON, or cmake
-DPUZZLES_GTK_VERSION=2) are now labelled explicitly with the CACHE
tag, and provided with a documentation string indicating what they're
for.
One effect of this is that GUI-like interfaces to your cmake build
directory, such as ccmake or cmake-gui, will show those variables
explicitly to give you a hint that you might want to change them.
Another is that when you do change them, cmake will recognise that it
needs to redo the rest of its configuration. Previously, if you sat in
an existing cmake build directory and did 'cmake -DSTRICT=ON .'
followed by 'cmake -DSTRICT=OFF .', nothing would happen, even though
you obviously meant it to.
I presume this will improve performance. Also, if I've understood
correctly, WASM-based compiled web code is capable of automatically
growing its memory, which the previous asm.js build of the puzzles
could not do, and occasionally caused people to complain that if they
tried to play a _really big_ game in their browser, the JS would
eventually freeze because the emulated memory ran out.
I've been putting off doing this for ages because my previous
Emscripten build setup was so finicky that I didn't like to meddle
with it. But now that the new cmake system in this source tree makes
things generally easier, and particularly since I've just found out
that the up-to-date Emscripten is available as a Docker image (namely
"emscripten/emsdk"), this seemed like a good moment to give it a try.
The source and build changes required for this update weren't too
onerous. I was half expecting a huge API upheaval, and indeed there
was _some_ change, but very little:
- in the JS initPuzzle function, move the call to Module.callMain()
into Module.onRuntimeInitialized instead of doing it at the top
level, because New Emscripten's .js output likes to load the
accompanying .wasm file asynchronously, so you can't call the WASM
main() until it actually exists.
- in the JS-side library code, replace all uses of Emscripten's
Pointer_stringify() function with the new name UTF8ToString(). (The
new version also has an ASCIIToString(), so I guess the reason for
the name change is that now you get to choose which character set
you meant. I need to use UTF-8, so that the × and ÷ signs in Keen
will work.)
- set EXTRA_EXPORTED_RUNTIME_METHODS=[cwrap,callMain] on the emcc
link command line, otherwise they aren't available for my JS setup
code to call.
- (removed -s ASM_JS=1 from the link options, though I'm not actually
sure it made any difference one way or the other in the new WASM
world)
- be prepared for a set of .wasm files to show up as build products
alongside the .js ones.
- stop building with -DCMAKE_BUILD_TYPE=Release! I'm not sure why
that was needed, but if I leave that flag on my cmake command line,
the output .js file fails to embed my emccpre.js, so the initial
call to initPuzzle() fails from the HTML wrapper page, meaning
nothing at all happens.
At least, for the Unix build, so as to support Debian stable and a
couple of prior Ubuntu LTSes.
Not much needed to change in the cmake scripts; the only noticeable
difference was that the 'install' command needs an explicit RUNTIME
DESTINATION.
The puzzle icons are built by compiling and running a preliminary
set of puzzle binaries. We can't do that if the binaries won't run
on the build host.
A distro maintainer reminds me that downstreams often want to rename
my quite generic executable names to avoid clashes in bin directories.
Added a cmake option -DOUTPUT_NAME to make that easy.
It's better to be lax for normal users trying to build the puzzles
from source to actually run them. That way, warning changes in some
particular compiler I haven't seen yet won't break the build.
Instead, I've invented a cmake setting -DSTRICT=ON which turns on all
those flags. So I can build with them myself, to ensure the code is as
portable as possible. And that flag is set in Buildscr, so that my
official builds won't complete until that warning mode is satisfied.
This reinstates the feature of the previous build system, that the C
icon files for the GTK puzzles were included in the source tarball, so
that users building from that instead of from the raw git repo would
not need to run the fiddly piece of build that regenerates them.
Running that fiddly piece of build is much easier in the CMake world
(because it's integrated with the main makefile), but it has a build
dependency on ImageMagick which is easily avoided.
The makefile will still build the icons if it _can_. But in the case
where it can't, it will use pre-built icon source files if they're
available, and only fall back to no-icon.c if it can't even do that.
(So a user checking out from git and building without ImageMagick
present will still be able to build _something_ playable.)
This completely removes the old system of mkfiles.pl + Recipe + .R
files that I used to manage the various per-platform makefiles and
other build scripts in this code base. In its place is a
CMakeLists.txt setup, which is still able to compile for Linux,
Windows, MacOS, NestedVM and Emscripten.
The main reason for doing this is because mkfiles.pl was a horrible
pile of unmaintainable cruft. It was hard to keep up to date (e.g.
didn't reliably support the latest Visual Studio project files); it
was so specific to me that nobody else could maintain it (or was even
interested in trying, and who can blame them?), and it wasn't even
easy to _use_ if you weren't me. And it didn't even produce very good
makefiles.
In fact I've been wanting to hurl mkfiles.pl in the bin for years, but
was blocked by CMake not quite being able to support my clang-cl based
system for cross-compiling for Windows on Linux. But CMake 3.20 was
released this month and fixes the last bug in that area (it had to do
with preprocessing of .rc files), so now I'm unblocked!
CMake is not perfect, but it's better at mkfiles.pl's job than
mkfiles.pl was, and it has the great advantage that lots of other
people already know about it.
Other advantages of the CMake system:
- Easier to build with. At least for the big three platforms, it's
possible to write down a list of build commands that's actually the
same everywhere ("cmake ." followed by "cmake --build ."). There's
endless scope for making your end-user cmake commands more fancy
than that, for various advantages, but very few people _have_ to.
- Less effort required to add a new puzzle. You just add a puzzle()
statement to the top-level CMakeLists.txt, instead of needing to
remember eight separate fiddly things to put in the .R file. (Look
at the reduction in CHECKLST.txt!)
- The 'unfinished' subdirectory is now _built_ unconditionally, even
if the things in it don't go into the 'make install' target. So
they won't bit-rot in future.
- Unix build: unified the old icons makefile with the main build, so
that each puzzle builds without an icon, runs to build its icon,
then relinks with it.
- Windows build: far easier to switch back and forth between debug
and release than with the old makefiles.
- MacOS build: CMake has its own .dmg generator, which is surely
better thought out than my ten-line bodge.
- net reduction in the number of lines of code in the code base. In
fact, that's still true _even_ if you don't count the deletion of
mkfiles.pl itself - that script didn't even have the virtue of
allowing everything else to be done exceptionally concisely.
