Skip to content

Conversation

@luszczewskakasia1
Copy link
Contributor

@luszczewskakasia1 luszczewskakasia1 commented Oct 21, 2025

The change allows to use a L0 commit instead of a tag which in some cases is needed by developers.

@luszczewskakasia1 luszczewskakasia1 requested review from a team as code owners October 21, 2025 08:35
@luszczewskakasia1 luszczewskakasia1 marked this pull request as draft October 21, 2025 08:41
@luszczewskakasia1 luszczewskakasia1 marked this pull request as ready for review October 22, 2025 09:28
@luszczewskakasia1 luszczewskakasia1 changed the title Modify files to enable installing L0 from .sh file [CI] Modify files to enable installing L0 from .sh file Oct 22, 2025
@luszczewskakasia1 luszczewskakasia1 changed the title [CI] Modify files to enable installing L0 from .sh file [CI][L0] Modify files to enable installing L0 from .sh file Oct 22, 2025
Copy link
Contributor

@mateuszpn mateuszpn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't fetch and build should be unconditional if commit hash is given? Now it is possible to set ZE_LOADER_LIBRARIES to /usr/lib/x86_64-linux-gnu/libze_loader.so

string(REGEX MATCH "^[0-9a-fA-F]+$" IS_HEX "${UR_LEVEL_ZERO_LOADER_TAG}")

if(PkgConfig_FOUND AND NOT (TAG_LENGTH EQUAL 40 AND IS_HEX))
pkg_check_modules(level-zero level-zero>1.24.3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably >=, as it was above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# just try to search for the path.
if(PkgConfig_FOUND)
pkg_check_modules(level-zero level-zero>=1.24.3)
string(LENGTH "${UR_LEVEL_ZERO_LOADER_TAG}" TAG_LENGTH)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why UR_LEVEL_ZERO_LOADER_TAG is checked, and not set here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik this line of code checks the length of a given string and this length is checked in if-statement one line below. I think this is expected behaviour

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of further if conditions may be bad because if the variable is not set, its LENGTH is 0

@luszczewskakasia1 luszczewskakasia1 force-pushed the scripts-change branch 3 times, most recently from bcf6515 to 0f571ed Compare October 24, 2025 10:42
Copy link
Contributor

@mateuszpn mateuszpn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cmake script lgtm.


set(UR_LEVEL_ZERO_LOADER_REPO "https://github.com/oneapi-src/level-zero.git")
# Remember to update the pkg_check_modules minimum version above when updating the
# clone tag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Make the whole comment coherent, as you mention some "minimum version above" (if fact it is below now), and in l. 20 you mention the change again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants