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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 29, 2024
Merged

meson: install headers #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3abf8e
meson: install headers
natto1784 Jun 18, 2024
e147214
Merge branch 'trunk' into meson
almarklein Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ffi/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
inc_dirs += [include_directories('.', is_system: true)]

headers = files(
'wgpu.h',
'webgpu-headers/webgpu.h'
Copy link

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.

Copy link
Collaborator

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?

Copy link

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:

  1. Installing the webgpu.h header.
  2. 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.

Copy link
Collaborator

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?

Copy link

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!

Copy link
Contributor Author

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.

)

install_headers(headers, preserve_path: true)
7 changes: 4 additions & 3 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@

project('wgpu-native', 'c', version : '0.0.0', license : 'MIT OR Apache-2.0')

inc_dirs = [include_directories('ffi', is_system: true)]
inc_dirs = [ ]

subdir('ffi')

cargo = find_program('cargo')
#determine which dir contain platform library
Expand All @@ -12,7 +14,7 @@ else
endif

#build by cargo
run_command(cargo,'build','--release')
run_command(cargo,'build','--release', check: true)
#mark cargo artifact to static library
wgpu_native = static_library('wgpu_native', objects : lib_path, install : true)

Expand All @@ -27,4 +29,3 @@ pkg.generate(name : 'wgpu-native',
libraries : [wgpu_native],
version : meson.project_version(),
)
install_subdir('include', install_dir : '')
Loading