Log the origin of manifest search paths by GloriousTacoo · Pull Request #1878 · KhronosGroup/Vulkan-Loader

GloriousTacoo · 2026-03-13T23:25:10Z

An attempt to fix Issue #1401.

I have a solid idea on how to log manifest path origins using a custom dynamic list, but I will need to setup infrastructure for this new list. Then I can replace the core function copy_data_file_info() with my own implementation.

[ ] Create an API for for this new dynamic list (init, destroy, append, etc.).
[ ] Write tests to valid expected dynamic list behavior.
[ ] Replace copy_data_file_info().
[ ] Write tests to validate expected function behavior.

ci-tester-lunarg · 2026-03-13T23:25:12Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-13T23:25:15Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

CLAassistant · 2026-03-13T23:25:57Z

All committers have signed the CLA.

Adds `loader_search_path_origin`, `loader_search_path`, and `loader_search_path_list`. These will be used to track the origin of a manifest path. For example, manifest files loaded from "etc/" will originate from `LOADER_SEARCH_PATH_ORIGIN_SYSCONFIG`. Signed-off-by: Ronald Caesar <github43132@proton.me>

ci-tester-lunarg · 2026-03-13T23:50:52Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-13T23:50:52Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T00:45:58Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T00:45:58Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T15:57:01Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T15:57:01Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T16:02:07Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-14T16:02:07Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

GloriousTacoo · 2026-03-15T19:43:20Z

I'm trying to add a new test file to the test_regression executable to test the functions in loader.h but got the following error:

[2/3] Building CXX object tests/CMakeFiles/test_regression.dir/loader_search_path_list_tests.cpp.o
FAILED: [code=1] tests/CMakeFiles/test_regression.dir/loader_search_path_list_tests.cpp.o 
/usr/lib/ccache/bin/c++ -DDEBUG -DEXTRASYSCONFDIR=\"/etc\" -DFALLBACK_CONFIG_DIRS=\"/etc/xdg\" -DFALLBACK_DATA_DIRS=\"/usr/local/share:/usr/share\" -DFRAMEWORK_CONFIG_HEADER=\"/home/apollo/dev/Vulkan-Loader/build/tests/framework/framework_config_Debug.h\" -DGIT_BRANCH_NAME=\"loader-search-path\" -DGIT_TAG_INFO=\"v1.4.345-6-g19fbea66d\" -DHAVE_REALPATH -DHAVE_SECURE_GETENV -DSYSCONFDIR=\"/usr/local/etc\" -DVK_ENABLE_BETA_EXTENSIONS -DVK_NO_PROTOTYPES -DVK_USE_PLATFORM_WAYLAND_KHR -DVK_USE_PLATFORM_XCB_KHR -DVK_USE_PLATFORM_XLIB_KHR -DVK_USE_PLATFORM_XLIB_XRANDR_EXT -I/home/apollo/dev/Vulkan-Loader/build/tests/framework -I/home/apollo/dev/Vulkan-Loader/tests/framework/util -I/home/apollo/dev/Vulkan-Loader/tests/framework/util/.. -I/home/apollo/dev/Vulkan-Loader/build/tests/framework/util -I/home/apollo/dev/Vulkan-Loader -I/home/apollo/dev/Vulkan-Loader/build/tests/framework/shim -I/home/apollo/dev/Vulkan-Loader/tests/framework/shim -isystem /home/apollo/dev/Vulkan-Loader/external/Debug/64/googletest/googletest/include -isystem /home/apollo/dev/Vulkan-Loader/external/Debug/64/googletest/googletest -isystem /home/apollo/dev/Vulkan-Headers/build/install/include -g -std=c++17 -fvisibility=hidden -fdiagnostics-color=always -fPIC -MD -MT tests/CMakeFiles/test_regression.dir/loader_search_path_list_tests.cpp.o -MF tests/CMakeFiles/test_regression.dir/loader_search_path_list_tests.cpp.o.d -o tests/CMakeFiles/test_regression.dir/loader_search_path_list_tests.cpp.o -c /home/apollo/dev/Vulkan-Loader/tests/loader_search_path_list_tests.cpp
In file included from /home/apollo/dev/Vulkan-Loader/loader/loader.h:32,
                 from /home/apollo/dev/Vulkan-Loader/tests/loader_search_path_list_tests.cpp:2:
/home/apollo/dev/Vulkan-Loader/loader/loader_common.h:39:10: fatal error: vk_layer_dispatch_table.h: No such file or directory
   39 | #include "vk_layer_dispatch_table.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
ninja: build stopped: subcommand failed.

Steps to reproduce:

Create a new test file in tests/.
Add the following to the file.

#include "test_environment.h"
#include "loader/loader.h"

TEST(Test, Test) {}

Add it to test_regression executable in tests/CMakeLists.txt
Compile tests.

ci-tester-lunarg · 2026-03-16T05:33:47Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-16T05:33:47Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

GloriousTacoo · 2026-03-16T16:02:29Z

Adding my tests to test_regression looks to be impossible, cause I have to link vulkan to to it which breaks a couple existing tests. Looks like I will have to create a new executable to test the new changes in loader.h

ci-tester-lunarg · 2026-03-16T16:47:01Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-16T16:47:01Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

The code structure is similar to create_string_list() and loader_init_generic_list() Signed-off-by: Ronald Caesar <github43132@proton.me>

This is your typical function for freeing memory Signed-off-by: Ronald Caesar <github43132@proton.me>

Signed-off-by: Ronald Caesar <github43132@proton.me>

Adds a new test executable `test_internal` which is meant to test private functions exported to tests with the TEST_FUNCTION_EXPORT macro. These tests will have the prefix "internal". Why create a new executable instead of adding new tests to `test_regression`? To do that we will need to link the vulkan library to test internal functions, but this breaks a few tests. This is the simplest solution. Signed-off-by: Ronald Caesar <github43132@proton.me>

charles-lunarg

I'm happy to help figure out why the tests do not compile, but github shows only whitespace changes for the new test file.

I can say up front that a vast majority of the tests are 'integration' tests. Testing log output should be done by adding a debug utils logger to the instance create info (other tests do it if you need reference) then looking to make sure the log has the right strings.

While it is conceivable to create a new loader/modify the binary to allow unit testing, this hasn't been done as it means the tests are able to execute on any random loader binary. (of course there are exceptions, the fuzz tests require exporting the handful of functions used in those tests, of which they are all about loading & parsing json files.)

GloriousTacoo · 2026-03-17T05:34:40Z

I'll explain the exact steps I took for the test to fail. Also I'm not testing log output per say. I'm testing every function I create in loader.h to make sure there are not any side effects.

GloriousTacoo · 2026-03-17T05:48:52Z

When this PR is finish logs could look something like this but the exact format can be decided by you.

# OLD
[Vulkan Loader] LAYER:          Searching for implicit layer manifest files
[Vulkan Loader] LAYER:             In following locations:
[Vulkan Loader] LAYER:                /home/fake_home/.config/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /etc/xdg/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /etc/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /home/fake_home/.local/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /usr/local/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /usr/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:             Found the following files:
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d/test_layer_0.json
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d/test_layer_1.json
[Vulkan Loader] INFO:           Found manifest file /usr/local/etc/vulkan/implicit_layer.d/test_layer_0.json (file version 1.0.0)
[Vulkan Loader] INFO:           Found manifest file /usr/local/etc/vulkan/implicit_layer.d/test_layer_1.json (file version 1.0.0)

# NEW
[Vulkan Loader] LAYER:          Searching for implicit layer manifest files
[Vulkan Loader] LAYER:             In XDG_DATA_DIRS:
[Vulkan Loader] LAYER:                /usr/local/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /usr/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /home/fake_home/.local/share/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:             Found no files
[Vulkan Loader] LAYER:             In SYSCONFIG:
[Vulkan Loader] LAYER:                /etc/xdg/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /etc/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d
[Vulkan Loader] LAYER:             Found the following files:
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d/test_layer_0.json
[Vulkan Loader] LAYER:                /usr/local/etc/vulkan/implicit_layer.d/test_layer_1.json
[Vulkan Loader] INFO:           Found manifest file /usr/local/etc/vulkan/implicit_layer.d/test_layer_0.json (file version 1.0.0)
[Vulkan Loader] INFO:           Found manifest file /usr/local/etc/vulkan/implicit_layer.d/test_layer_1.json (file version 1.0.0)

GloriousTacoo · 2026-03-17T06:16:26Z

To replicate the failed tests you need to link vulkan directly to test_regression in tests/CMakeLists.txt.

target_link_libraries(test_regression PUBLIC testing_dependencies vulkan)

5 tests fail. This is why I created a new test executable, link vulkan to it, and exposed the internal functions, but I see this is wrong now.

Tests if loader_init_search_path_list() is successful. Signed-off-by: Ronald Caesar <github43132@proton.me>

Tests if loader_destroy_search_path_list() is successfully. Signed-off-by: Ronald Caesar <github43132@proton.me>

Tests if loader_append_search_path() has correct behavior. Signed-off-by: Ronald Caesar <github43132@proton.me>

ci-tester-lunarg · 2026-03-17T06:25:52Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

ci-tester-lunarg · 2026-03-17T06:25:52Z

Author GloriousTacoo not on autobuild list. Waiting for curator authorization before starting CI build.

GloriousTacoo · 2026-03-17T06:49:49Z

Not gonna lie testing my changes without exposing internal functions is difficult. I'll probably drop the test commits once the PR is finished.

charles-lunarg

Yeah the way you have written the tests are in line with "unit tests" while the test suite is primarily composed of 'integration' tests.

Because the loader is a C library, there isn't a great way to test individual functions without statically linking the loader. Or exporting functions we want to test with, which definitely is hacky. Refactoring the loader into two libraries - a static one that contains all the functionality and a shared one that links to the static one then implements all the required exports & behavior the loader needs to have - is something I support but haven't had the impetus to do so. The original design of the tests was to not require modifying the loader source code itself in any way, as there was no significant tests of the loader and so we didn't want to break behavior by changing the loader in order to add tests. That was years ago now and the test suite is pretty extensive, so it is reasonable to explore modifying the loader source in order to write tests. But, as much as I agree that the current style of test writing is obnoxious (more setup, harder to validate behavior), there are non-trivial difficulties making individual functions unit-testable. Best thing to do is to create an issue describing how you would like to write tests, and see if we can't hash out details on how that might be achieved. I know how to write tests (cause I wrote the framework), I'd love to have another's opinions on it.

And to not just ramble, here are some actual code review comments.

charles-lunarg · 2026-03-19T17:12:08Z

loader/loader.c

+    search_paths->list[search_paths->count] = *path;
+    ++search_paths->count;


This would be a good place to do de-duplication.

charles-lunarg · 2026-03-19T17:16:02Z

loader/loader_common.h

+    LOADER_SEARCH_PATH_ORIGIN_XDG_CONFIG,
+    LOADER_SEARCH_PATH_ORIGIN_XDG_DATA,
+    LOADER_SEARCH_PATH_ORIGIN_SYSCONFIG,
+    LOADER_SEARCH_PATH_ORIGIN_HOME,


Quite a few different ways for manifests to be found are missing from this enum.

Add env-vars (VK_ADD_LAYER_PATH, VK_ADD_IMPLICIT_LAYER_PATH, VK_ADD_DRIVER_FILES)

Doesn't differentiate XDG_DATA_HOME & XDG_DATA_DIRS, as well as XDG_CONFIG_HOME & XDG_CONFIG_DIRS

macOS/iOS app bundles

The ever confusing EXTRASYSCONFDIR (which normally is /etc, but is build time configurable...)

windows "app package path" (special path microsoft added for compatibility layers found on the windows store)

There are enough different ways to find manifests that I wonder if the strategy is to just have a string that can contain 'anything' instead of locking it down into an ENUM. Plus, you'd have to create an enum->string helper for it anyway.

GloriousTacoo force-pushed the loader-search-path branch from de03c0c to a0a45d1 Compare March 13, 2026 23:50

GloriousTacoo force-pushed the loader-search-path branch from 7a93e51 to bd81bd4 Compare March 14, 2026 16:02

GloriousTacoo force-pushed the loader-search-path branch from bd81bd4 to 19fbea6 Compare March 16, 2026 05:33

GloriousTacoo added 4 commits March 16, 2026 14:03

Add search path list init function

3f44668

The code structure is similar to create_string_list() and loader_init_generic_list() Signed-off-by: Ronald Caesar <github43132@proton.me>

Add search path list destroy function

1bfdc40

This is your typical function for freeing memory Signed-off-by: Ronald Caesar <github43132@proton.me>

Add search path list append function

692560a

Signed-off-by: Ronald Caesar <github43132@proton.me>

charles-lunarg reviewed Mar 17, 2026

View reviewed changes

Add search path init test

8e1ae1a

Tests if loader_init_search_path_list() is successful. Signed-off-by: Ronald Caesar <github43132@proton.me>

GloriousTacoo added 2 commits March 17, 2026 02:23

Add search path destroy test

5c1910d

Tests if loader_destroy_search_path_list() is successfully. Signed-off-by: Ronald Caesar <github43132@proton.me>

Add search path append test

73254e7

Tests if loader_append_search_path() has correct behavior. Signed-off-by: Ronald Caesar <github43132@proton.me>

GloriousTacoo force-pushed the loader-search-path branch from a3ba566 to 73254e7 Compare March 17, 2026 06:25

charles-lunarg reviewed Mar 19, 2026

View reviewed changes

		search_paths->list[search_paths->count] = *path;
		++search_paths->count;

Conversation

GloriousTacoo commented Mar 13, 2026

Uh oh!

ci-tester-lunarg commented Mar 13, 2026

Uh oh!

ci-tester-lunarg commented Mar 13, 2026

Uh oh!

CLAassistant commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ci-tester-lunarg commented Mar 13, 2026

Uh oh!

ci-tester-lunarg commented Mar 13, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

ci-tester-lunarg commented Mar 14, 2026

Uh oh!

GloriousTacoo commented Mar 15, 2026

Uh oh!

ci-tester-lunarg commented Mar 16, 2026

Uh oh!

ci-tester-lunarg commented Mar 16, 2026

Uh oh!

GloriousTacoo commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ci-tester-lunarg commented Mar 16, 2026

Uh oh!

ci-tester-lunarg commented Mar 16, 2026

Uh oh!

charles-lunarg left a comment

Choose a reason for hiding this comment

Uh oh!

GloriousTacoo commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GloriousTacoo commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GloriousTacoo commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ci-tester-lunarg commented Mar 17, 2026

Uh oh!

ci-tester-lunarg commented Mar 17, 2026

Uh oh!

GloriousTacoo commented Mar 17, 2026

Uh oh!

charles-lunarg left a comment

Choose a reason for hiding this comment

Uh oh!

charles-lunarg Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

charles-lunarg Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Mar 13, 2026 •

edited

Loading

GloriousTacoo commented Mar 16, 2026 •

edited

Loading

GloriousTacoo commented Mar 17, 2026 •

edited

Loading

GloriousTacoo commented Mar 17, 2026 •

edited

Loading

GloriousTacoo commented Mar 17, 2026 •

edited

Loading