Update Vulkan, SPIR-V, glslang and shaderc #94049
Conversation
|
closes #94247 |
|
the shaderc bump is causing some regressions, I now get: |
|
I created an upstream issue https://code.videolan.org/videolan/libplacebo/-/issues/108 |
|
I see two solutions:
|
|
I have also noticed that the shaderc-dev derivation has bundled the glslang headers, shouldn't they be in a separate derivation? |
|
@CajuM shaderc is re-exporting all of the glslang headers it was built with. Easiest solution I found is just make $ git diff pkgs/
diff --git a/pkgs/development/libraries/libplacebo/default.nix b/pkgs/development/libraries/libplacebo/default.nix
index dc3e5e30c74..15b52c400df 100644
--- a/pkgs/development/libraries/libplacebo/default.nix
+++ b/pkgs/development/libraries/libplacebo/default.nix
@@ -47,10 +47,10 @@ stdenv.mkDerivation rec {
buildInputs = [
vulkan-headers
vulkan-loader
- shaderc
glslang
lcms2
epoxy
+ shaderc
];
mesonFlags = [ |
|
Not sure if this is intended behavior, and probably exists in the current package: |
|
The situation here's a bit weird because various vulkan tooling packages all have different hardcoded revision dependencies on glslang, none of which are stable releases. If a package depends on both shaderc and glslang (why?) then it should probably try to use the same version of glslang that shaderc does. Maybe the explicit glslang dep should be removed from libplacebo entirely? |
|
I made one more patch, on top of this PR, but I'm not sure if it's ok because I removed the custom versions for glslang, everything works now. I also updated vulkan tooling to newer versions, except shaderc which requires an unreleased version of glslang. I also had some problems with vkd3d and vkquake, but managed to fix them. Also about glslang, there are several workarounds available made by openSUSE, Fedora and Gentoo. Gentoo patched shaderc to support the older version of glslang. Fedora and openSUSE used unreleased versions in rawhide and factory |
pkgs/top-level/all-packages.nix
Outdated
| @@ -15824,24 +15824,24 @@ in | |||
| src = fetchFromGitHub { | |||
There was a problem hiding this comment.
Again, I know you are merely attempting to update a few packages, but I think it ought to be taken care of while at it:
I understand why Google needs overrides as explained here. But could you explain what's with the overrides here? Ideally, these overrides should be in a separate file, and automatically generated via an passthru.updateScript. If that's not possible, it would be nice to get these details out of all-packages.nix.
There was a problem hiding this comment.
there's an easier way to do it, the patch I posted on gist removes the custom buildInputs versions all-together. So we'd be using one stable version for each of the updated packages in the entire repo. I didn't push it to this branch because I wasn't sure if it would be accepted. One problem I see is that we'd have to wait a bit to update the whole set of packages due to incompatible versions. Another might be that we'd be introducing regressions by not using the provided dependency versions for glslang and shaderc.
There was a problem hiding this comment.
The general issue is that upstream only advertises support for specific revisions, and every package that depends on glslang/spirv-tools seems to have its own unique vetted revision. Using a different revision than the one supported by upstream seems like asking for trouble. No opinion on the proper place/mechanism to encode the information, though; I can't keep up with the latest nixpkgs tricks.
There was a problem hiding this comment.
This is what I was thinking of:
diff --git i/pkgs/development/compilers/glslang/default.nix w/pkgs/development/compilers/glslang/default.nix
index dee3f488ecc..135eb94dd70 100644
--- i/pkgs/development/compilers/glslang/default.nix
+++ w/pkgs/development/compilers/glslang/default.nix
@@ -3,9 +3,27 @@
, cmake
, jq
, python3
-, spirv-headers
-, spirv-tools
+, spirv-headers'
+, spirv-tools'
}:
+let
+ spirv-tools = spirv-tools'.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Tools";
+ rev = "fd8e130510a6b002b28eee5885a9505040a9bdc9";
+ sha256 = "00b7xgyrcb2qq63pp3cnw5q1xqx2d9rfn65lai6n6r89s1vh3vg6";
+ };
+ });
+ spirv-headers = spirv-headers'.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Headers";
+ rev = "f8bf11a0253a32375c32cad92c841237b96696c0";
+ sha256 = "1znwjy02dl9rshqzl87rqsv9mfczw7gvwfhcirbl81idahgp4p6l";
+ };
+ });
+in
stdenv.mkDerivation rec {
pname = "glslang";
diff --git i/pkgs/development/tools/vulkan-validation-layers/default.nix w/pkgs/development/tools/vulkan-validation-layers/default.nix
index 0d3c227291f..0bd9dd0a662 100644
--- i/pkgs/development/tools/vulkan-validation-layers/default.nix
+++ w/pkgs/development/tools/vulkan-validation-layers/default.nix
@@ -1,6 +1,46 @@
-{ stdenv, fetchFromGitHub, cmake, writeText, python3
-, vulkan-headers, vulkan-loader, glslang, spirv-headers
-, pkgconfig, xlibsWrapper, libxcb, libXrandr, wayland }:
+{ stdenv
+, fetchFromGitHub
+, cmake
+, writeText
+, python3
+, spirv-headers
+, spirv-tools
+, vulkan-headers
+, vulkan-loader
+, glslang'
+, pkgconfig
+, xlibsWrapper
+, libxcb
+, libXrandr
+, wayland
+}:
+let
+ glslang = (glslang'.override {
+ spirv-tools' = spirv-tools.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Tools";
+ rev = "e128ab0d624ce7beb08eb9656bb260c597a46d0a";
+ sha256 = "0jj8zrl3dh9fq71jc8msx3f3ifb2vjcb37nl0w4sa8sdhfff74pv";
+ };
+ });
+ spirv-headers' = spirv-headers.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Headers";
+ rev = "ac638f1815425403e946d0ab78bac71d2bdbf3be";
+ sha256 = "1lkhs7pxcrfkmiizcxl0w5ajx6swwjv7w3iq586ipgh571fc75gx";
+ };
+ });
+ }).overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "glslang";
+ rev = "e00d27c6d65b7d3e72506a311d7f053da4051295";
+ sha256 = "00lzvzk613gpm1vsdxffmx52z3c52ijwvzk4sfhh95p71kdydhgv";
+ };
+ });
+in
stdenv.mkDerivation rec {
pname = "vulkan-validation-layers";
@@ -21,7 +61,7 @@ stdenv.mkDerivation rec {
buildInputs = [
glslang
- spirv-headers
+ glslang.spirv-headers
vulkan-headers
vulkan-loader
libxcb
diff --git i/pkgs/top-level/all-packages.nix w/pkgs/top-level/all-packages.nix
index f4b0ae7dc31..fbf6f625e0c 100644
--- i/pkgs/top-level/all-packages.nix
+++ w/pkgs/top-level/all-packages.nix
@@ -9095,22 +9095,8 @@ in
dotnetPackages = recurseIntoAttrs (callPackage ./dotnet-packages.nix {});
glslang = callPackage ../development/compilers/glslang {
- spirv-tools = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Tools";
- rev = "fd8e130510a6b002b28eee5885a9505040a9bdc9";
- sha256 = "00b7xgyrcb2qq63pp3cnw5q1xqx2d9rfn65lai6n6r89s1vh3vg6";
- };
- });
- spirv-headers = spirv-headers.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Headers";
- rev = "f8bf11a0253a32375c32cad92c841237b96696c0";
- sha256 = "1znwjy02dl9rshqzl87rqsv9mfczw7gvwfhcirbl81idahgp4p6l";
- };
- });
+ spirv-headers' = spirv-headers;
+ spirv-tools' = spirv-tools;
};
go_bootstrap = if stdenv.isAarch64 then
@@ -15819,31 +15805,7 @@ in
vulkan-loader = callPackage ../development/libraries/vulkan-loader { };
vulkan-tools = callPackage ../tools/graphics/vulkan-tools { };
vulkan-validation-layers = callPackage ../development/tools/vulkan-validation-layers {
- glslang = (glslang.override {
- spirv-tools = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Tools";
- rev = "e128ab0d624ce7beb08eb9656bb260c597a46d0a";
- sha256 = "0jj8zrl3dh9fq71jc8msx3f3ifb2vjcb37nl0w4sa8sdhfff74pv";
- };
- });
- spirv-headers = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Headers";
- rev = "ac638f1815425403e946d0ab78bac71d2bdbf3be";
- sha256 = "1lkhs7pxcrfkmiizcxl0w5ajx6swwjv7w3iq586ipgh571fc75gx";
- };
- });
- }).overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "glslang";
- rev = "e00d27c6d65b7d3e72506a311d7f053da4051295";
- sha256 = "00lzvzk613gpm1vsdxffmx52z3c52ijwvzk4sfhh95p71kdydhgv";
- };
- });
+ glslang' = glslang;
};
vtkWithQt5 = vtk.override { qtLib = qt514; };The diff doesn't change the inputs of glslang at all, from the version of the current PR, but it does change them for vulkan-validation-layers. I think it's unavoidable if the typo is to be fixed.
There was a problem hiding this comment.
how about?
diff --git a/pkgs/development/compilers/glslang/default.nix b/pkgs/development/compilers/glslang/default.nix
index dee3f488ecc..183d0f4df02 100644
--- a/pkgs/development/compilers/glslang/default.nix
+++ b/pkgs/development/compilers/glslang/default.nix
@@ -5,7 +5,28 @@
, python3
, spirv-headers
, spirv-tools
+, argSpirv-tools ? null
+, argSpirv-headers ? null
}:
+let
+ localSpirv-tools = ({ argSpirv-tools ? spirv-tools.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Tools";
+ rev = "fd8e130510a6b002b28eee5885a9505040a9bdc9";
+ sha256 = "00b7xgyrcb2qq63pp3cnw5q1xqx2d9rfn65lai6n6r89s1vh3vg6";
+ };
+ }) }: argSpirv-tools) { argSpirv-tools = argSpirv-tools; };
+
+ localSpirv-headers = ({ argSpirv-headers ? spirv-headers.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Headers";
+ rev = "f8bf11a0253a32375c32cad92c841237b96696c0";
+ sha256 = "1znwjy02dl9rshqzl87rqsv9mfczw7gvwfhcirbl81idahgp4p6l";
+ };
+ }) }: argSpirv-headers) { argSpirv-headers = argSpirv-headers; };
+in
stdenv.mkDerivation rec {
pname = "glslang";
@@ -20,22 +41,22 @@ stdenv.mkDerivation rec {
# These get set at all-packages, keep onto them for child drvs
passthru = {
- inherit spirv-tools spirv-headers;
+ inherit localSpirv-tools localSpirv-headers;
};
nativeBuildInputs = [ cmake python3 bison jq ];
enableParallelBuilding = true;
postPatch = ''
- cp --no-preserve=mode -r "${spirv-tools.src}" External/spirv-tools
- ln -s "${spirv-headers.src}" External/spirv-tools/external/spirv-headers
+ cp --no-preserve=mode -r "${localSpirv-tools.src}" External/spirv-tools
+ ln -s "${localSpirv-headers.src}" External/spirv-tools/external/spirv-headers
'';
# Ensure spirv-headers and spirv-tools match exactly to what is expected
preConfigure = ''
HEADERS_COMMIT=$(jq -r < known_good.json '.commits|map(select(.name=="spirv-tools/external/spirv-headers"))[0].commit')
TOOLS_COMMIT=$(jq -r < known_good.json '.commits|map(select(.name=="spirv-tools"))[0].commit')
- if [ "$HEADERS_COMMIT" != "${spirv-headers.src.rev}" ] || [ "$TOOLS_COMMIT" != "${spirv-tools.src.rev}" ]; then
+ if [ "$HEADERS_COMMIT" != "${localSpirv-headers.src.rev}" ] || [ "$TOOLS_COMMIT" != "${localSpirv-tools.src.rev}" ]; then
echo "ERROR: spirv-tools commits do not match expected versions: expected tools at $TOOLS_COMMIT, headers at $HEADERS_COMMIT";
exit 1;
fi
diff --git a/pkgs/development/tools/vulkan-validation-layers/default.nix b/pkgs/development/tools/vulkan-validation-layers/default.nix
index 0d3c227291f..9c8490f7277 100644
--- a/pkgs/development/tools/vulkan-validation-layers/default.nix
+++ b/pkgs/development/tools/vulkan-validation-layers/default.nix
@@ -1,6 +1,46 @@
-{ stdenv, fetchFromGitHub, cmake, writeText, python3
-, vulkan-headers, vulkan-loader, glslang, spirv-headers
-, pkgconfig, xlibsWrapper, libxcb, libXrandr, wayland }:
+{ stdenv
+, fetchFromGitHub
+, cmake
+, writeText
+, python3
+, spirv-headers
+, spirv-tools
+, vulkan-headers
+, vulkan-loader
+, glslang
+, pkgconfig
+, xlibsWrapper
+, libxcb
+, libXrandr
+, wayland
+}:
+let
+ localGlslang = (glslang.override {
+ argSpirv-tools = spirv-tools.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Tools";
+ rev = "e128ab0d624ce7beb08eb9656bb260c597a46d0a";
+ sha256 = "0jj8zrl3dh9fq71jc8msx3f3ifb2vjcb37nl0w4sa8sdhfff74pv";
+ };
+ });
+ argSpirv-headers = spirv-headers.overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "SPIRV-Headers";
+ rev = "ac638f1815425403e946d0ab78bac71d2bdbf3be";
+ sha256 = "1lkhs7pxcrfkmiizcxl0w5ajx6swwjv7w3iq586ipgh571fc75gx";
+ };
+ });
+ }).overrideAttrs (_: {
+ src = fetchFromGitHub {
+ owner = "KhronosGroup";
+ repo = "glslang";
+ rev = "e00d27c6d65b7d3e72506a311d7f053da4051295";
+ sha256 = "00lzvzk613gpm1vsdxffmx52z3c52ijwvzk4sfhh95p71kdydhgv";
+ };
+ });
+in
stdenv.mkDerivation rec {
pname = "vulkan-validation-layers";
@@ -20,8 +60,8 @@ stdenv.mkDerivation rec {
];
buildInputs = [
- glslang
- spirv-headers
+ localGlslang
+ localGlslang.localSpirv-headers
vulkan-headers
vulkan-loader
libxcb
@@ -32,7 +72,7 @@ stdenv.mkDerivation rec {
enableParallelBuilding = true;
cmakeFlags = [
- "-DGLSLANG_INSTALL_DIR=${glslang}"
+ "-DGLSLANG_INSTALL_DIR=${localGlslang}"
];
# Help vulkan-loader find the validation layers
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index f4b0ae7dc31..7ee745abe9f 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -9094,24 +9094,7 @@ in
dotnetPackages = recurseIntoAttrs (callPackage ./dotnet-packages.nix {});
- glslang = callPackage ../development/compilers/glslang {
- spirv-tools = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Tools";
- rev = "fd8e130510a6b002b28eee5885a9505040a9bdc9";
- sha256 = "00b7xgyrcb2qq63pp3cnw5q1xqx2d9rfn65lai6n6r89s1vh3vg6";
- };
- });
- spirv-headers = spirv-headers.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Headers";
- rev = "f8bf11a0253a32375c32cad92c841237b96696c0";
- sha256 = "1znwjy02dl9rshqzl87rqsv9mfczw7gvwfhcirbl81idahgp4p6l";
- };
- });
- };
+ glslang = callPackage ../development/compilers/glslang { };
go_bootstrap = if stdenv.isAarch64 then
srcOnly {
@@ -15818,33 +15801,7 @@ in
vulkan-headers = callPackage ../development/libraries/vulkan-headers { };
vulkan-loader = callPackage ../development/libraries/vulkan-loader { };
vulkan-tools = callPackage ../tools/graphics/vulkan-tools { };
- vulkan-validation-layers = callPackage ../development/tools/vulkan-validation-layers {
- glslang = (glslang.override {
- spirv-tools = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Tools";
- rev = "e128ab0d624ce7beb08eb9656bb260c597a46d0a";
- sha256 = "0jj8zrl3dh9fq71jc8msx3f3ifb2vjcb37nl0w4sa8sdhfff74pv";
- };
- });
- spirv-headers = spirv-tools.overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "SPIRV-Headers";
- rev = "ac638f1815425403e946d0ab78bac71d2bdbf3be";
- sha256 = "1lkhs7pxcrfkmiizcxl0w5ajx6swwjv7w3iq586ipgh571fc75gx";
- };
- });
- }).overrideAttrs (_: {
- src = fetchFromGitHub {
- owner = "KhronosGroup";
- repo = "glslang";
- rev = "e00d27c6d65b7d3e72506a311d7f053da4051295";
- sha256 = "00lzvzk613gpm1vsdxffmx52z3c52ijwvzk4sfhh95p71kdydhgv";
- };
- });
- };
+ vulkan-validation-layers = callPackage ../development/tools/vulkan-validation-layers { };
vtkWithQt5 = vtk.override { qtLib = qt514; };
There was a problem hiding this comment.
@CajuM sorry for the long wait. This diff is just as good. Please commit it, and please mention near every override why it is needed.
|
Please follow CONTRIBUTING.md and manual#submitting-changes-making-patches and squash the fix-up commits.
|
| @@ -20,22 +49,22 @@ stdenv.mkDerivation rec { | |||
|
|
|||
| # These get set at all-packages, keep onto them for child drvs | |||
| passthru = { | |||
| inherit spirv-tools spirv-headers; | |||
| inherit localSpirv-tools localSpirv-headers; | |||
There was a problem hiding this comment.
Small nitpick - this is a change in the "API" of it - use spirv-tools = localSpirv-tools, etc.
| # vulkan-validation-layers requires a custom glslang version, while glslang requires | ||
| # custom versions for spirv-tools and spirv-headers | ||
| # The vulkan-validation-layers dependency is taken from: | ||
| # https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/master/scripts/known_good.json | ||
| # While those for glslang are taken from: | ||
| # https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/master/scripts/known_good.json |
There was a problem hiding this comment.
| # vulkan-validation-layers requires a custom glslang version, while glslang requires | |
| # custom versions for spirv-tools and spirv-headers | |
| # The vulkan-validation-layers dependency is taken from: | |
| # https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/master/scripts/known_good.json | |
| # While those for glslang are taken from: | |
| # https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/master/scripts/known_good.json | |
| # vulkan-validation-layers requires a custom glslang version, while glslang requires | |
| # custom versions for spirv-tools and spirv-headers. The git hashes required for all | |
| # of these deps is documented upstream here: | |
| # https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/master/scripts/known_good.json |
| glslang | ||
| spirv-headers | ||
| localGlslang | ||
| localGlslang.localSpirv-headers |
There was a problem hiding this comment.
If you change back the API, as I think you should, don't forget this place. Note it shouldn't trigger rebuilds, but please check it all.
I still get this error. Reverting the shaderc bump fixes it for me. |
|
😬 Then this means |
It would seem that removing the https://code.videolan.org/videolan/libplacebo/-/commit/523056828ab86c2f17ea65f432424d48b6fdd389.patch patch fixes things |
|
All of these packages are a mess (I'm not sure who is to blame). How come Arch Linux doesn't need such overrides? Perhaps they do need and use
Maybe we shouldn't give to much attention to the Since there are no shaderc maintainers, I'm reluctant into doing this. |
I removed https://code.videolan.org/videolan/libplacebo/-/commit/82e3be1839379791b58e98eb049415b42888d5b0.patch and it builds for me. Reverting shaderc also works #100818. Not sure what the right way is. Open for suggestions. |
I don't know what you did, but this sounds like a reasonable idea in comparison to #100818. I get: With this change: diff --git i/pkgs/development/libraries/libplacebo/default.nix w/pkgs/development/libraries/libplacebo/default.nix
index dc3e5e30c74..91de1f70fa7 100644
--- i/pkgs/development/libraries/libplacebo/default.nix
+++ w/pkgs/development/libraries/libplacebo/default.nix
@@ -27,10 +27,6 @@ stdenv.mkDerivation rec {
patches = [
# to work with latest glslang, remove on release >2.72.0
- (fetchpatch {
- url = "https://code.videolan.org/videolan/libplacebo/-/commit/523056828ab86c2f17ea65f432424d48b6fdd389.patch";
- sha256 = "051vhd0l3yad1fzn5zayi08kqs9an9j8p7m63kgqyfv1ksnydpcs";
- })
(fetchpatch {
url = "https://code.videolan.org/videolan/libplacebo/-/commit/82e3be1839379791b58e98eb049415b42888d5b0.patch";
sha256 = "0nklj9gfiwkbbj6wfm1ck33hphaxrvzb84c4h2nfj88bapnlm90l"; |
|
My bad, it was 82e3be1839379791b58e98eb049415b42888d5b0.patch |
From some reason, if we take this patch out, libplacebo builds fine, and mpv is again usable.
|
Out of scope for the issue with libplacebo: All projects of KhronosGroup should not have local overrides, as according to my tests they are compatible with each other, as you'd expect since it's the same group that develops all of these tools - (glslang has local overrides). To improve the situation of all ugly overrides, I think it'd be best to introduce an attribute set such as It should be also worth nagging upstream that their build process is not ideal for downstream package maintainers, considering the fact |
(cherry picked from commit 0826f2e)
This reverts commit b2c8af3.
|
Does anyone have any new thoughts on how to handle these upgrades? I packaged the |
|
We can either add multiple versions of glslang (maybe 11.1 or 8.X?) or you add a local overwrite in your package if only the version changes and nothing else. |
That sounds great to me - to remove all overrides inside packages' expressions, and have as many variants as needed of the same packages all declared in |
|
Prominent consumers use distinct, specific, non-release revisions that must be updated in lockstep with the consumers. Moving that into a separate file sounds painful. |
Janik-Haag
added
the
12. first-time contribution
Motivation for this change
Keeping master up to date with recent developments in upstream projects
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)