[Clang][CI] Extend clang version output #20520
Conversation
This patch adds "Nightly build" string to clang version output. Previously it was like: clang version 22.0.0git (https://github.com/intel/llvm.git 2de7d37) Now it looks like: clang version 22.0.0git (https://github.com/intel/llvm.git 2de7d37 Nightly build) I'm preserving the repo path, but extending the revision. Anyways, we need to define both CLANG_VC_REPOSITORY and CLANG_VC_REVISION CMake variables to redefine the output, see: llvm/llvm/cmake/modules/GenerateVersionFromVCS.cmake Moreover, we need to define LLVM_VC_REPOSITORY & LLVM_VC_REVISION, because if it's not the same as CLANG_VC_* variables, there will be two sections of brackets after clang version, like: clang version 22.0.0git (https://github.com/intel/llvm.git 2de7d37) (https://github.com/intel/llvm.git 2de7d37 Nightly build) This is kind of hacky, but it allows as not to change the source code.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
.github/workflows/sycl-nightly.yml
Outdated
| build_configure_extra_args: '--hip --cuda' | ||
| build_configure_extra_args: | | ||
| --hip --cuda \ | ||
| -DLLVM_VC_REPOSITORY="${{ github.repositoryUrl }}" -DLLVM_VC_REVISION="${{ github.sha }} Nightly build" \ |
There was a problem hiding this comment.
since we're only doing this for the nightly, is there some benefit we get to knowing if someone is using the nightly or not?
There was a problem hiding this comment.
I'll also add the same thing for nightly release builds like x.y.z pre-release build and x.y.z release build for official releases, so this is needed to distinguish them.
KornevNikita
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Why? How is the "nightly build" different from regular non-nightly build? |
What you mean by regular non-nightly build? There is a huge set of options on how to build the toolchain. Nightly builds are built in some specific way, tested properly enough and published. So there is a customer request to make the version info a bit more informative. |
For more details see: #20520 This PR adds branch:hash to nightly builds on release branches. It also allows to specify a custom version manually in case of official release builds, e.g. "6.3.0 release build".
I mean builds done by other workflows (e.g. pre-commit, post-commit, etc.).
What is missing in the current version info? How does you change make it more informative? |
Previous attempt: #20520 That one is kind of hacky, therefore proceeding with a more proper solution. There is a customer request to make clang version output more informative. This patch adds a string like this to the output: "Intel SYCL compiler X build based on:" - here goes regular clang output. Where X is "development" by default. "Nightly" for regular nightly builds. "X.Y.Z Nightly" for nightly release builds. "X.Y.Z release" for official releases.
@KornevNikita, why did you open another PR? Please, use one PR for all the discussions/review process. |
KornevNikita
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
KornevNikita
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
There is a customer request to make clang version output more informative. This patch adds a string like this to the output: "Intel SYCL compiler X build based on:" - here goes regular clang output. Where X is "development" by default. "Nightly" for regular nightly builds. "X.Y.Z Nightly" for nightly release builds. "X.Y.Z release" for official releases.
KornevNikita
changed the title
KornevNikita
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
@bader I've updated the PR, could you please check the description, as the approach was changed a bit.
The build info is missing. Some people are using our nightly builds, so they need this info to distinguish builds, I guess. If you're still against this change, then we can drop it, as I have nothing to say more here. Maybe @AlexeySachkov has some. Anyways, we still need this change to mark our official releases, as of now clang says nothing about it. Do you have any concerns regarding this point? |
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
@mdtoguchi can you confirm this? I thought we emitted release information ? |
@KornevNikita, I'm confused. I don't understand what exactly the problem you are trying to solve by adding "Nightly" string to the version reported by the driver.
I agree. I would expect our compiler to emit the version similar to the upstream compiler. |
There is additional release information emitted with the Intel compiler, but as I understand it the intel/llvm bits just emit the clang version. |
CMPLRLLVM-68631 is the internal tracker number. Hash does not exactly describe where a build is coming from: for example a local build and a build downloaded from nightly can have the same hash, but they could be built with different flags or options (like support for device image compression, or SYCL JIT). Another benefit is that we can instantly compare two nightly builds to understand which one is newer/older, without having to look up a commit hash in the repo. However, I do not see this part being implemented despite it being mentioned in the description - without it just "Nightly" string alone does not bring much benefit |
Yes, there is such mechanism in the internal compiler version, but intel/llvm clang currently reports the same info as llvm-project clang.
Added date. |
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
I want @AlexeySachkov to decide the readiness of this change as he is in context of the customer request mentioned in the description. @KornevNikita, please, get approval from @AlexeySachkov before merging this PR. |
Hmm, it's kind of tricky since we need to re-build clang to get the updated output, not sure how to implement it. Do you have any thoughts? At least we could add a test to Nightly to check if the output is right. |
There is a customer request to make clang version output more
informative. This patch adds a string like this to the output:
"Intel SYCL compiler X build based on:" - here goes regular clang output.
Where X is "development" by default.
"Nightly YYYY-MM-DD" for regular nightly builds.
"X.Y.Z Nightly YYYY-MM-DD" for nightly release builds.
"X.Y.Z release" for official releases.