Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop deqp-vksc from vulkan-cts package and upgrade vulkan-cts: 1.3.10.0 -> 1.4.1.3 #391105

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jljusten
Copy link
Contributor

Vulkan SC (Safety Critical) CTS is separate from the Vulkan CTS. This change splits it out into a separate vulkansc-cts package, and uses the latest Vulkan SC CTS tag, vulkansc-cts-1.0.2.1 with this package.

Then, the Vulkan CTS is upgraded: 1.3.10.0 -> 1.4.1.2.

These changes were prompted by the Vulkan SC executable (deqp-vksc) failing to build with the newer Vulkan CTS version. This build failure is not a bug of the CTS as the vulkan-cts-* tags are for the Vulkan CTS and not the Vulkan SC CTS. In other words, vulkan-cts-* tags support the Vulkan CTS and vulcansc-cts-* tags support the Vulkan SC CTS.

I don't have access to a Vulkan SC setup, so I was only able to test this command (to dump the CTS test names) with the new vulkansc-cts package:

  • deqp-vksc --deqp-runmode=txt-caselist

Cc: @Flakebi

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Mar 18, 2025
@nix-owners nix-owners bot requested a review from Flakebi March 18, 2025 23:04
@Flakebi
Copy link
Member

Flakebi commented Mar 25, 2025

Sounds mostly good to me, thanks!
As the packages are rather similar, it would be good to share the code for them. The cts-sources.py script is the same and the default.nix is almost identical, so it would be a shame to have the code duplicated :)

@Flakebi
Copy link
Member

Flakebi commented Mar 25, 2025

It seems like deqp-vksc does not work correctly at the moment, so I’d also be fine with just dropping it.

$ deqp-vksc -n dEQP-VKSC.info.device
…
FATAL ERROR: Couldn't match tags from sub.qpa Subprocess failed with exit code 0(0) at vktTestPackage.cpp:1009

@jljusten
Copy link
Contributor Author

It seems like deqp-vksc does not work correctly at the moment, so I’d also be fine with just dropping it.

$ deqp-vksc -n dEQP-VKSC.info.device
…
FATAL ERROR: Couldn't match tags from sub.qpa Subprocess failed with exit code 0(0) at vktTestPackage.cpp:1009

Do you have a driver that supports Vulkan SC? I also see a similar error message, but I don't think my driver supports Vulkan SC.

I did as you suggested, and just dropped Vulkan SC support, rather than creating a new package for it. Perhaps we can add the separate package in the future if we can get a way to confirm that it is working.

I also updated the package from 1.4.1.2 to a new 1.4.1.3 tag.

@jljusten jljusten changed the title Split Vulkan SC into vulkansc-cts and upgrade vulkan-cts: 1.3.10.0 -> 1.4.1.2 Drop deqp-vksc from vulkan-cts package and upgrade vulkan-cts: 1.3.10.0 -> 1.4.1.3 Mar 26, 2025
@Flakebi
Copy link
Member

Flakebi commented Mar 26, 2025

Seems like we need to remove one of the two leading \n here to make nixfmt happy:

f.write("\n\n prePatch = ''\n");

Otherwise the change looks good to me (builds and passes tests nix-build . -A vulkan-cts.tests), thanks!

Signed-off-by: Jordan Justen <jljusten@gmail.com>
The Vulkan SC (Safety Critical) CTS has different release tags and
build dependencies from the Vulkan CTS.

When updating the vulkan-cts package version, I found that the newer
version would fail to build if we attempted to build the deqp-vksc
executable.

I suspect that building the Vulkan SC CTS executable, deqp-vksc, may
not be supported unless using a vulkansc-cts-* tag.

This change adds cmake build define SELECTED_BUILD_TARGETS=deqp-vk,
which was introduced in vulkan-cts-1.3.7.0, and reduces the amount of
code built. Notably, it causes the build to no longer attempt to build
deqp-vksc.

Ref: vulkan-cts b22a8f06d ("Allow selecting a subset of targets at configuration time")
Signed-off-by: Jordan Justen <jljusten@gmail.com>
Note that the Vulkan SC executable deqp-vksc had to be dropped
(previous patch) from the Vulkan CTS (vulkan-cts) package, or this
update would lead to a build failure.

Signed-off-by: Jordan Justen <jljusten@gmail.com>
Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants