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

layers: Allow SPIRV-Tools commit to be a tag #8463

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spencer-lunarg
Copy link
Contributor

For people working on private extension (and possibly if SPIR-V Tools adds tags in near future) we will want to go

diff --git a/scripts/known_good.json b/scripts/known_good.json
index 4237ef3ee..66cc20a39 100755
--- a/scripts/known_good.json
+++ b/scripts/known_good.json
@@ -43,7 +43,7 @@
                 "-DSPIRV_SKIP_TESTS=ON",
                 "-DSPIRV_SKIP_EXECUTABLES=OFF"
             ],
-            "commit": "363486479d4c8dfbfdf2e2dc397cd10aa15f80b0"
+            "commit": "some tag name"
         },

so this removes the (honestly over complex) logic to get the git sha into the UUID for shader caching and just takes the string and uses the same internal hashing we use for everything else

@spencer-lunarg spencer-lunarg requested a review from a team as a code owner August 27, 2024 15:58
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 244397.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 17327 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 17327 passed.

@jeremyg-lunarg
Copy link
Contributor

Are you ok if the tag moves? Eg if it points at a dev branch that gets force pushed.

@spencer-lunarg
Copy link
Contributor Author

Are you ok if the tag moves? Eg if it points at a dev branch that gets force pushed.

ugh, why you have to come in with the hard hitting questions... ya, I guess that might effect things, let me digest this more

@jeremyg-lunarg
Copy link
Contributor

https://stackoverflow.com/questions/1862423/how-to-tell-which-commit-a-tag-points-to-in-git/1862542#1862542 says you do:

 git rev-list -n 1 origin/sdk-1.1.101
6471d151f6882a0f93a6919a33255615a5d1aa59

Ideally this would be handled by cmake somehow and remove the need for generate_spirv.py after each update of SPIRV-Tools. But I don't have a magic recipe for that...

@spencer-lunarg
Copy link
Contributor Author

I spent way to much time on this, my CMAKE foo is not good enough, but wanted to do something like

# We need the hash of the SPIRV-Tools to know when to invalidate caches that rely on it.
# Because the known_good.json might use a development tag name, the sha might change underneath it, so we need to call git to get the real sha.
# We do it in the cmake, as it is easy to forget to re-run a python script and it will not change correctly.
# We have the check the code in so that BUILD.gn build works, but luckily no one is developing in that path so doesn't need to be update each time there.
get_target_property(SPIRV_TOOLS_OPT_LOCATION SPIRV-Tools-opt LOCATION)
get_filename_component(SPIRV_TOOLS_DIR "${SPIRV_TOOLS_OPT_LOCATION}" DIRECTORY)
set(SPIRV_TOOLS_COMMIT_FILE "${CMAKE_CURRENT_SOURCE_DIR}/${API_TYPE}/generated/spirv_tools_commit_id.h")

set(SHA_COMMAND "${CMAKE_COMMAND} -E echo \"#define SPIRV_TOOLS_COMMIT_SHA \\\"\$(git -C ${SPIRV_TOOLS_DIR} rev-parse HEAD)\\\"\" > ${SPIRV_TOOLS_COMMIT_FILE}")

add_custom_command(
    OUTPUT ${SPIRV_TOOLS_COMMIT_FILE}
    COMMAND ${SHA_COMMAND}
)

add_custom_target(update_spirv_tools_commit ALL DEPENDS ${SPIRV_TOOLS_COMMIT_FILE})
add_dependencies(vvl update_spirv_tools_commit)

and have it just write the file out

The issue is we need the sha checked in so things like BUILD.gn sees it... but it is hard to get cmake to re-run based off the sha from another repo

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.

4 participants