There are several files used only by test, and these files only build if tests enabled. So it's redundant to check DBUS_ENABLE_EMBEDDED_TESTS in source code. These patches removed these checks in bus/ and dbus/, no such checks in test/, it's clean.
Created attachment 82250 [details] [review] [PATCH 1/3] TEST: remove redundant test build detection in bus daemon
Created attachment 82251 [details] [review] [PATCH 2/3] TEST: do not build test only files into libdbus
Created attachment 82252 [details] [review] [PATCH 3/3] TEST: remove redundant test build detection in libdbus
these patches looks very trivial, not make much sense, I think I need explain more to convince myself first. current status. quite several files which used only by tests build into libdbus, although they did nothing since they detect DBUS_ENABLE_EMBEDDED_TESTS in the file begin. This looks like a double confirm to prevent these files be built by mistake, so I'm now OK to keep these detect in source file. Meanwhile, I'd like to fix the others which detect DBUS_ENABLE_EMBEDDED_TESTS in the middle of file although they're only used in tests, like that in [PATCH 3/3]. And move these test only files into test build in Makefile.am rather than keep them in libdbus build.
Created attachment 82311 [details] [review] [PATCH v2 1/3] TEST: do not build test only files into internal libdbus
Created attachment 82312 [details] [review] [PATCH v2 2/3] TEST: polish test build detection in bus daemon test files
Created attachment 82313 [details] [review] [PATCH v2 3/3] TEST: polish test build detection in libdbus test files
Comment on attachment 82313 [details] [review] [PATCH v2 3/3] TEST: polish test build detection in libdbus test files Review of attachment 82313 [details] [review]: ----------------------------------------------------------------- This seems a lot of diffstat, for what benefit? ::: dbus/dbus-test.c @@ -98,4 @@ > void > dbus_internal_do_not_use_run_tests (const char *test_data_dir, const char *specific_test) > { > -#ifdef DBUS_ENABLE_EMBEDDED_TESTS This is technically an ABI break: it removes symbols that were previously exported. (Nobody should have been calling those functions, but, still.)
Given the choice between conditionally including files in the build system, and always including all files but having some of them conditionally empty, I usually prefer the "conditionally empty" route unless there are technical reasons to avoid it.
OK.
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.