configure.ac and CMakeLists.txt substitute several variables for test data (TEST_VALID_SERVICE_DIR, TEST_VALID_SERVICE_SYSTEM_DIR, etc.) and one for each test executable (TEST_SERVICE_BINARY, TEST_EXIT_BINARY etc.). I think they'd be clearer (and have less duplication between autotools and cmake) if they just substituted two directory names, DBUS_TEST_DATA and DBUS_TEST_EXEC, plus EXEEXT (which is a standard Autoconf feature - D-Bus' CMake build system calls it EXT, but I added EXEEXT as a synonym there). An additional motivation for this is that in a later branch, I want to make more tests installable, and allow the hard-coded paths to be overridden via environment variables $DBUS_TEST_DATA (which already exists) and $DBUS_TEST_EXEC (which will be new). (Ralf: is EXT a special CMake name with other side-effects, or something you made up for D-Bus? If it's something you made up, we could just rename it to EXEEXT and throw away EXT entirely.) Meanwhile, as mentioned on Bug #41033, the Autoconf and CMake build systems currently have parallel source files for some things. They shouldn't: they should share a source file, and perform the same substitutions (perhaps with different values - but in fact it makes more sense to vary the values for Windows vs Unix, not for CMake vs Autoconf). This branch fixes this for all the test data. Patches to follow later today (I'm tidying them up a bit).
Created attachment 51632 [details] [review] [1/5] Simplify generation of bus configuration files All we really need is one substituted variable, DBUS_TEST_DATA, rather than things like TEST_VALID_SERVICE_DIR. In theory we could even go further and use @abs_top_builddir@, but that'd break if we want the possibility to install more of the regression tests (to be used to test installed/packaged binaries), which is something I'm working on.
Created attachment 51633 [details] [review] [2/5] Simplify substitution of test executables to use fewer variables Also use EXEEXT in all the service files, even in the automake build system.
Created attachment 51634 [details] [review] [3/5] cmake: remove unused TEST_SERVICE_DIR variable `git grep TEST_SERVICE_DIR` says we don't need it.
Created attachment 51636 [details] [review] [4/5] Merge tests' cmake and autotools bus configuration In Unix, the tests listened on both debug-pipe (which is a socketpair, or a TCP emulation of socketpair on Windows) and a Unix socket. In the Windows port, the tests were hard-coded to listen on a particular port, which allowed the dispatch test to connect to that port, as long as no two tests ran simultaneously (which I don't think was ever guaranteed - make -j can violate this). That's valid out-of-process, and also fully-specified, so they only needed one <listen> directive, so the CMake input only had one. To make the tests work under CMake on Unix, there was a hack: the string substituted for the content of the <listen> directive contained </listen><listen> to get the other address in, which is pretty nasty. Instead of doing that, I've made both build systems, on both Unix and Windows, use both debug-pipe and a more normal transport (Unix or TCP). debug-pipe has a Windows implementation and it's used in dbus-spawn-win.c, so it'd better work. The use of debug-pipe is now hard-coded rather than being a configure parameter (there's no reason to vary it in different builds), and I used TEST_LISTEN as the name of the Unix/TCP address, because it's a "vague" address (no specific Unix path, no TCP port), that you can listen on but not connect to. This in turn means that we can merge the Autoconf .in and CMake .cmake files, similar to Bug #41033. You might wonder why I've kept debug-pipe. I did try to get rid of it, but it turns out that the tests in dispatch.c rely on dbus_connection_open_private() not blocking, and normal socket connections block on connect(). Until we fix that by adding an async version of dbus_connection_open_private(), it won't be safe to have a test like dispatch.c that "talks to itself", unless it uses a transport as trivial as debug-pipe in which neither end has to block on the other.
Created attachment 51637 [details] [review] [5/5] sysdeps: remove misleading comments The comment claims that _dbus_full_duplex_pipe() is only used for the debug-pipe server, but in fact the process-spawning code uses it now (on both Unix and Windows platforms). --- This one is trivial and can be applied before the others if desired.
The branch is against dbus-1.4, but perhaps this'd make more sense in master only.
(In reply to comment #0) > > (Ralf: is EXT a special CMake name with other side-effects no > or something you made up for D-Bus? If it's something you made up, we could just rename it to EXEEXT and throw away EXT entirely.) feel free to throw away EXT I'm going to apply and compile this stuff on windows to see if there still problems.
Created attachment 51655 [details] [review] [6] Remove EXT variable from CMake, just use Automake-compatible EXEEXT According to Ralf, there's no standard name for this in CMake, so we might as well use the standard Automake name.
(In reply to comment #7) > (In reply to comment #0) > > > > (Ralf: is EXT a special CMake name with other side-effects > no > > or something you made up for D-Bus? If it's something you made up, we could just rename it to EXEEXT and throw away EXT entirely.) > > feel free to throw away EXT > > I'm going to apply and compile this stuff on windows to see if there still > problems. no problems found on windows and patches are looking good.
(In reply to comment #9) > no problems found on windows and patches are looking good. Thanks, fixed in git for 1.5.10. On a second look, this wasn't suitable for dbus-1.4 (it's rearrangement/cleaning rather than strict bugfixing).
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.