Add tags/make releases for this repo

christgau commented 9 months ago

It would very helpful to know which API or loader version these tests require. The prerequisites in the BUILD file appear to be outdates as the file has not been updated for about two years. At the moment one can only guess which version of the loader/API these tests are working with (see #33). Therefore, I propose to introduce tags or make releases which may follow the same numbering scheme as the loader or the API (I don't care in the end, but it should be documented). At least the BUILD file should be updated.

lisanna-dettwyler commented 5 months ago

We'll soon start adding conditional guards around test content that requires newer header versions which should solve this issue. I'll take a look at the BUILD file.

eero-t commented 5 months ago

We'll soon start adding conditional guards around test content that requires newer header versions which should solve this issue. I'll take a look at the BUILD file.

@lisanna-dettwyler, you should really re-open this as:

Few header guards are not the same thing as git tags and projects releases
Bug should really be closed only when it has been actually fixed in this repo (not in some random internal one), and reporter has verified it to be fixed (or several weeks have passed without reporter response)

lisanna-dettwyler commented 4 months ago

This was closed inadvertently because an internal PR linked against it. GitHub closed it automatically when it was merged. I intend to keep this open until the BUILD.md file is updated publicly. We're not going to do releases for these tests, always use HEAD.

eero-t commented 4 months ago

We're not going to do releases for these tests, always use HEAD.

While releases would be nice, this bug is not requesting them, project tags would be enough.

Documentation can state what can be expected from the tagged commits; did they go through some additional testing, or do they indicate some change within the project, such as change in the required L0 spec version.

Minimal quality statement could be something like following:

Tagged commits do not have any guarantee of functionality. They have just been tested to successfully build with the L0 spec version indicated by the tag name. Tags follow the "builds-with-L0-spec-VERSION" format.

Tags are trivial to add, also for historical commits. And if tagged version eventually turns out to be "bad", tag can be removed.

PS. It would be better to avoid moving tags to different commits. If e.g. builds-with-L0-spec-v1.5 tag is found to point to a bad commit, e.g. new builds-with-L0-spec-v1.5-update1 tag can be added for a better one.

christgau commented 4 months ago

We're not going to do releases for these tests, always use HEAD.

While releases would be nice, this bug is not requesting them, project tags would be enough.

It's not about releases for the tests, but pointing at HEAD alone does not help a user - as I'm such one. The scenario I faced was like this: Suppose you have a installation of the L0 runtime made by system administrators. How do I know which commit of the tests in this repo I can use to successfully compile. That's my main point.

Documentation can state what can be expected from the tagged commits; did they go through some additional testing, or do they indicate some change within the project, such as change in the required L0 spec version.

That would be fine as well.

Tags are trivial to add, also for historical commits.

2x Exactly. The ecosystem would benefit quite bit from this.

christgau commented 4 months ago

Thanks for updating the BUILD file. However, it still does not solve the issue:

https://github.com/oneapi-src/level-zero-tests/blob/e94215d06ead13ecbe8901600eb5bafd4863993b/BUILD.md?plain=1#L34-L36

The first sentence is still rather imprecise. Is it guaranteed that HEAD of this repo and HEAD of the loader repo are always compatible? Or put another way: If I go back in time, because I need a specific version... uhm... commit, can I just pick the commit from the loader repo with the same date and it works?

And: What's the proposed way to identify the commit of this repo that works with a specific version of the loader?

Further, the second sentence from the BUILD file does not apply. Here's what I did following the steps fro the BUILD file:

...
$ cmake -D CMAKE_INSTALL_PREFIX=$PWD/../out ..
-- The C compiler identification is GNU 13.2.1
-- The CXX compiler identification is GNU 13.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Warning at cmake/clang_tools.cmake:20 (message):
  clang-format-7 not found.  clang-format and clang-format-check targets will
  be disabled.
Call Stack (most recent call first):
  CMakeLists.txt:12 (include)

-- Found ZLIB: /usr/lib/libz.so (found version "1.3.1")
-- Found PNG: /usr/lib/libpng.so (found version "1.6.43")
-- Found Git: /usr/bin/git (found version "2.44.0")
CMake Warning at CMakeLists.txt:26 (message):
  Unable to locate third_party/googletest submodule in checkout - Trying to
  fetch with /usr/bin/git submodule update --init --recursive

Submodule 'third_party/googletest' (https://github.com/google/googletest.git) registered for path 'third_party/googletest'
Cloning into '/tmp/level-zero-tests/third_party/googletest'...
Submodule path 'third_party/googletest': checked out '15460959cbbfa20e66ef0b5ab497367e47fc0a04'
-- Found Python: /usr/bin/python3.11 (found version "3.11.8") found components: Interpreter
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found Boost: /usr/lib/cmake/Boost-1.83.0/BoostConfig.cmake (found suitable version "1.83.0", minimum required is "1.65") found components: log program_options timer chrono system serialization
-- Boost_FOUND: TRUE
-- Found LevelZero: /usr/include
-- LevelZero_LIBRARIES: /usr/lib/libze_loader.sodl
-- LevelZero_INCLUDE_DIRS: /usr/include
-- Found PAPI library: papi
[...]
-- Configuring done (3.6s)
-- Generating done (0.3s)
-- Build files have been written to: /tmp/level-zero-tests/build

Note that the cmake step finds my local installation which is great. Following up, I try to compile..

/tmp/level-zero-tests/build $ cmake --build . --config Release --target install
[ 93%] Built target test_api_ltracing
[ 93%] Building CXX object layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/src/test_api_ltracing_compat.cpp.o
[ 93%] Building CXX object layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/src/test_api_ltracing.cpp.o
[ 93%] Building CXX object layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/src/test_api_ltracing_threading.cpp.o
[ 93%] Building CXX object layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/src/main.cpp.o
/tmp/level-zero-tests/layer_tests/tracing/src/main.cpp: In function ‘int main(int, char**)’:
/tmp/level-zero-tests/layer_tests/tracing/src/main.cpp:29:3: error: ‘zelEnableTracingLayer’ was not declared in this scope
   29 |   zelEnableTracingLayer();
      |   ^~~~~~~~~~~~~~~~~~~~~
/tmp/level-zero-tests/layer_tests/tracing/src/main.cpp:35:3: error: ‘zelDisableTracingLayer’ was not declared in this scope
   35 |   zelDisableTracingLayer();
      |   ^~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/build.make:118: layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/src/main.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:5512: layer_tests/tracing/CMakeFiles/test_api_ltracing_dynamic.dir/all] Error 2

Ok, error missing declaration. Is that an error inside HEAD (would be bad) or are the header files installed on my maschine not compatible with the test repo (contradicts the sentence from the BUILD file)?

$ pacman -Ss level-zero
extra/intel-compute-runtime 23.48.27912.11-2 [installed]
    Intel(R) Graphics Compute Runtime for oneAPI Level Zero and OpenCL(TM) Driver
extra/level-zero-headers 1.15.1-1 [installed]
    API for accessing low level interfaces in oneAPI platform devices (headers)
extra/level-zero-loader 1.15.1-1 [installed]
    API for accessing low level interfaces in oneAPI platform devices (loader)

My proposal for the tags would be that I can do git checkout l0-loader-1.15.1-1 on a clone of this repo and be sure that it works/compiles with the version of loader/headers installed on my machine.

lisanna-dettwyler commented 4 months ago

And: What's the proposed way to identify the commit of this repo that works with a specific version of the loader?

It's HEAD of master, the tests are intended to remain backwards-compatible with older releases. I'll make this clearer in the documentation, and add a section at the top talking about header compatibility. Historically this hasn't been the case, but we're going to start enforcing it.

Ok, error missing declaration. Is that an error inside HEAD (would be bad) or are the header files installed on my maschine not compatible with the test repo (contradicts the sentence from the BUILD file)?

The most recent version of the BUILD file addresses this, it appears you read the version just prior: https://github.com/oneapi-src/level-zero-tests/blob/master/BUILD.md?plain=1#L36

lisanna-dettwyler commented 3 months ago

Version guards will only work for 1.9 onwards, since a required macro in the headers is set to be released in 1.10. In other words, loaders with at least 1.9 of the headers will be able to use HEAD of master indefinitely. I have created https://github.com/oneapi-src/level-zero-tests/releases/tag/spec-1.8 and https://github.com/oneapi-src/level-zero-tests/releases/tag/spec-1.7 for older loaders.

oneapi-src / level-zero-tests

Add tags/make releases for this repo #34