Align git-sync-deps and CMake to use external/spirv-headers by default

[kpet]

Align git-sync-deps and CMake to use external/SPIRV-Headers by default

This should help with avoiding mistakes such as the one that happened under #4958.

Signed-off-by: Kevin Petit kevin.petit@arm.com
Change-Id: I922f02e25c507f3412e0e7a99f525fb617b2d426

[Keenuts]

Not against changing the order, but what about just not looking for SPIRV-Headers? the README way is to use the DEPS, meaning path should be in lowercase. If people want to have their tree in another path, they can define SPIRV-Headers_SOURCE_DIR no? (maybe renaming it to something easier to use would be nice)

[kpet]

Not against changing the order, but what about just not looking for SPIRV-Headers?

That works for me and is cleaner. I kept it as I assumed it was used by someone (the CI at least :)).

they can define SPIRV-Headers_SOURCE_DIR no? (maybe renaming it to something easier to use would be nice)

I would advise against renaming that variable. I know of several projects that make use of it.

[Keenuts]

Not against changing the order, but what about just not looking for SPIRV-Headers?

That works for me and is cleaner. I kept it as I assumed it was used by someone (the CI at least :)).

they can define SPIRV-Headers_SOURCE_DIR no? (maybe renaming it to something easier to use would be nice)

I would advise against renaming that variable. I know of several projects that make use of it.

@dneto0 do you know if that's still used?

[kpet]

Taking a step back I'm wondering whether it would make sense to align everything on SPIRV-Headers since that's what the folder will be named after a manual clone on OSes that have case-sensitive file names. Is the name used by git-sync-deps different by design?

[Keenuts]

Taking a step back I'm wondering whether it would make sense to align everything on SPIRV-Headers since that's what the folder will be named after a manual clone on OSes that have case-sensitive file names. Is the name used by git-sync-deps different by design?

If that's not by design, I would also be in favor of keeping SPIRV-Headers

[kpet]

Alright, I'll do that then.

[Keenuts]

Thanks! You'll have to fix the CI scripts too (some don't use the DEPS file), like kokoro/scripts/linux/build-docker.sh

[dneto0]

Not against changing the order, but what about just not looking for SPIRV-Headers?

That works for me and is cleaner. I kept it as I assumed it was used by someone (the CI at least :)).

they can define SPIRV-Headers_SOURCE_DIR no? (maybe renaming it to something easier to use would be nice)

I would advise against renaming that variable. I know of several projects that make use of it.

@dneto0 do you know if that's still used?

Sorry for the very late reply. That's used by shaderc and amber. I think vkb does it too (but vkb is dormant).

All this was made when CMake was at version 2.8.12. There may be better mechanisms now.

But also people have strong preferences for naming directories lower-case. That's the case for Chromium IIRC.

[s-perron]

This is not the direction we want to go. We discussed internally, and we should change the CMAKE file to only look for spirv-header, to match the existing git-sync-deps.

The review is stale.

[s-perron]

The smoke test failures are not related to this change.