build fails if no X server is present for xpce

[matko]

Starting with commit 769595e, builds fail if xpce is enabled in the build, but no X server is found. Relevant bit of log from the failing (Nix) build:

[...]
[39/41] Build home/library/ext/libedit/INDEX.pl
[40/41] -- Building manual index
/homeless-shelter: No such file or directory
[PCE fatal: @display/display: Failed to connect to X-server at `': no DISPLAY environment variable
*********************************************************************
* You MUST be running the X11 Windowing environment.  If you are,   *
* check the setting of your DISPLAY environment variable as well    *
* the access rights to your X11 server.  See xauth(1) and xhost(1). *
*********************************************************************
    in:     <No exception goal>
]
Host stack:
     [13] pce_principal:send(@2200448/chain, merge(@image_class/class?send_methods))
     [12] man_word_index:make_class_index(@image_class/class)
     [11] '$c_call_prolog'
     [10] pce_principal:send(@2198907/chain, for_all(message(@prolog/host, make_class_index, @arg1(= @image_class/class))))
      [9] man_word_index:pce_make_manual_index('/build/source/build/home/xpce/man/reference/index.obj')
[40/41] Install the project...
-- Install configuration: "Release"
-- Set non-toolchain portion of runtime path of "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/bin/x86_64-linux/swipl" to "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/lib/x86_64-linux"
-- Set non-toolchain portion of runtime path of "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/libswipl.so.9.3.9-769595e" to "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/lib/x86_64-linux"
-- Set non-toolchain portion of runtime path of "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/bin/x86_64-linux/swipl-ld" to "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/lib/x86_64-linux"
-- Set non-toolchain portion of runtime path of "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/lib/x86_64-linux/libjpl.so" to "/nix/store/8k5dja7djiryrnjqr3f03j5wyfd2pag2-swi-prolog-9.3.9-769595e/lib/swipl/lib/x86_64-linux:/nix/store/k9snfnb9i9yc1ww9r0bb0vah9mgwjw7c-openjdk-21.0.3+9/include:/nix/store/k9snfnb9i9yc1ww9r0bb0vah9mgwjw7c-openjdk-21.0.3+9/lib/openjdk/lib/server"
CMake Error at packages/xpce/cmake_install.cmake:70 (file):
  file INSTALL cannot find
  "/build/source/build/home/xpce/man/reference/index.obj": No such file or
  directory.
Call Stack (most recent call first):
  cmake_install.cmake:107 (include)

FAILED: CMakeFiles/install.util 
cd /build/source/build && /nix/store/ll2b6cslly4v30im7qk34f9y21ziiz1i-cmake-3.29.6/bin/cmake -P cmake_install.cmake
ninja: build stopped: subcommand failed.

The build still works when the X dependencies are not present.

So it appears to be faling on a missing DISPLAY variable, which subsequently causes the install to fail due to a missing file. I think SWI-Prolog should still be buildable if all dependencies are present, even if it's being built on a machine with no X server available (or, as is the case here, available but inaccessible to the sandboxed build).

Besides the error about the missing DISPLAY variable it also complains about not being able to find /homeless-shelter. There is a Nix sandboxing trick in play here, where it sets the home dir of the build users to the non-existent path /homeless-shelter, to ensure nothing in the build looks at arbitrary things in the home dir (which would make the build nondeterministic). It showing up here indicates that something in the build is trying to do so which is a bit suspicious, but I think ultimately the error here is not that, but the lacking DISPLAY variable.

[matko]

Unfortunately, this is still not building with latest swipl-devel (9624767), which has this patch. Error remains the same.

[JanWielemaker]

Hmm. That is odd. I unset DISPLAY and did a clean build without any problems. Could it be that DISPLAY is set to the empty string? As is, I just check getenv("DISPLAY"), I do not check whether the value is empty or in any way sensible.

[matko]

I patched the nix build to report on the DISPLAY variable early on in the script using echo "display is ${DISPLAY:-unset}", and this reports it is unset. It is really not there, unless something later on in the build sets it, which I very much doubt.

[JanWielemaker]

Hmm. Tried again, but I cannot reproduce this. All I can suggest is to put some print statements in getDPIDisplay(). That seems to be what opens the display and what has been changed not to try this if $DISPLAY is unset on non-Windows.

[matko]

doing a bisect points at 2bbad8c2cbcf5c9ac83353d9df3574dcab191b17 as the culprit. Indeed, if I revert this patch, things build again. The problem appears to be that generating documentation wants the default values for these properties to be retrievable, which they were before cause they were just constants, but now that there's an expression involving @display, this doesn't work anymore. Maybe you're not seeing this due to not building the documentation?

[JanWielemaker]

Maybe you're not seeing this due to not building the documentation?

I build the HTML docs. I did not test building the PDF docs. I.e., I ran a completely default build with all dependencies in place. Yes, it refers to @display, but AFAIK, that should not be a problem as long as we do not open the display.

This commit indeed forces the display to be opened to try and fetch the DPI from the X server. The subsequent 4564011c4579a8b98614c8b8ab6f1bc23635bf9a restricts this to only open the display if getenv("DISPLAY") is non-NULL. For me, this does the trick. Without this patch the build crashes as indicated and after this patch it builds cleanly.

I know it is a silly question, but you did update the submodule, no? Otherwise I'd add some print statements near the patches in getDPIDisplay() to see what is happening.

[matko]

Yeah, I've been running with the latest xpce (fd35abb8), and I've been liberally adding printf statements to see what is going on. getDpiDisplay is not even called. GetSizeDisplay however is and is what is failing.

[JanWielemaker]

Ah, now I see. My Defaults file overruled the image.scale resource. But, that one is wrong. Should have been dpi rather than size. Just pushed a fix for that after testing on my big monitor.

That probably also refers to using files from $HOME: it reads the xpce/Defaults file. It probably should not :cry:

[matko]

Yep, now it builds! sandboxed builds sure are fun for spotting such issues :)