-
Notifications
You must be signed in to change notification settings - Fork 110
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
meson: install headers #394
Conversation
Signed-off-by: Amneesh Singh <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine by me ...
|
||
headers = files( | ||
'wgpu.h', | ||
'webgpu-headers/webgpu.h' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it your intention for this package to "take over" the dependency of webgpu.h
? Would it ever make sense to we webgpu.h
alone without wgpu-native
????
if so, installing webgpu.h
might be strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused as to what you're asking. But I can try to explain how the two header files relate.
The webgpu.h
is specified alongside webgpu.idl
(the JS spec), and can be targeted by any implementation (wgpu-native and Dawn are two examples). That's why its in a separate repo. The wgpu.h
contains additional functionality specific to desktop/wgpu. As far as I know, it is expected that both will stay separate for the foreseeable future.
Does this answer your question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was my understanding as well.
So to me, webgpu.h
is not "owned" by the wgpu-native
library and thus should not be installed by it.
Rather, some package integrator (this is REALLY annoying for C libraries since no single package manager has surfaced as a winner), is in charge of:
- Installing the
webgpu.h
header. - Then starting the compiation of
wgpu-native
.
wgpu-native
should be checking for compatibility with the installed header webgpu.h
prior to starting.
So having wgpu-native
copy over a vendoered version of webgpu.h
is somewhat problematic long term.
In terms of how this is done in practice, for example, at conda-forge, i declare the build time dependency on webgpu-headers
, but due to the strict version matching, I must also specify the version i want installed.
https://github.com/conda-forge/wgpu-native-feedstock/blob/main/recipe/meta.yaml#L27
Again, the large range of C package manager (mostly non-existant in a cross platform way without conda IMO) means that this is always done in an ad-hoc manner.
In either case, I think that the webgpu.h
should not be installed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure if I follow :) Might be my limited understanding of C build systems ...
I'm not sure what you mean by wgpu-native installing webgpu.h.
The webgpu-headers
repo is added as a git submodule, tagged to a specific commit. From time to time, the submodule is bumped to the latest version, and any necessary changes to the source of wgpu-native are made. I think you can consider (the specific version of) webgpu.h
to be part of this repo. Not owned, but definitely pinned.
So having wgpu-native copy over a vendoered version of webgpu.h is somewhat problematic long term.
How do you think this can become problematic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me try to see how this works in practice and followup with a larger issue if it is a problem.
thakns for your time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what @hmaarrfk is trying to say that webgpu-headers should be installed separately instead of installing its header with wgpu-native. I kind of agree with him but this is a relatively minor change I needed for my build system, feel free to change. Users will have to install webgpu-headers/webgpu.h separately though.
(I don't meant to backseat drive this project, just noticed build related PRs when trying to check what changed "recently") |
Installing as a standalone library works due to
install: true
instatic_library()
. However, headers are not automatically installed. This commit adds the two headers available underffi/
.Additionally, I have set
check: true
for the cargo command