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

Change artifact folder structure #417

Merged
merged 7 commits into from
Sep 5, 2024

Conversation

almarklein
Copy link
Collaborator

Closes #410

This updates the package command in the makefile to introduce a folder structure instead of putting all files in a flat namespace. The idea is to prepare the contents of the archive in a directory and then zipping that dir as a whole.

Results can be seen here: https://github.com/almarklein/wgpu-native/actions/runs/10703101259 (can others access these?)

One example:

image

@almarklein
Copy link
Collaborator Author

cc @Korijn

I don't have a lot of experience with makefiles and shell scripts, so I would not be surprised if there are better ways to do things. E.g. I hate the cd ../.., but popd was not available on one of the builds (IIRC the android build).

Copy link
Collaborator

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Feel free to merge.

Though just noticed that ios archives (and the action name) have prefix iOS instead of ios making it inconsistent with other builds, would be good to have that fixed.

@almarklein
Copy link
Collaborator Author

Though just noticed that ios archives (and the action name) have prefix iOS instead of ios making it inconsistent with other builds, would be good to have that fixed.

Will do that in a new pr

@almarklein
Copy link
Collaborator Author

Oh, I misunderstood your comment. Naming the archive to be lowercase is something that makes sense doing in this pr :)

@almarklein
Copy link
Collaborator Author

Makefile Outdated Show resolved Hide resolved
@almarklein
Copy link
Collaborator Author

@almarklein almarklein merged commit 8c44ec1 into gfx-rs:trunk Sep 5, 2024
16 checks passed
@almarklein almarklein deleted the artifact-folder-structure branch September 5, 2024 14:57
@Sahnvour
Copy link

Hi, i'm sorry if I didn't make it clear in the issue, but an intermediate webgpu/ folder is missing in this case to be entirely useful.

In meta-build systems (cmake, meson, conan...) we'd want to do something like
add_include_path('path/to/wgpu-unzipped/include')
where include/ still contains a webgpu/ subfolder, so that other libraries depending on it can do #include <webgpu/webgpu.h>.

And if for any reason the project consuming wegpu wanted to use #include <webgpu.h> instead, they could do add_include_path('path/to/wgpu-unzipped/include/webgpu'): this way either option can be made to work.

@almarklein
Copy link
Collaborator Author

[...] where include/ still contains a webgpu/ subfolder, so that other libraries depending on it can do #include <webgpu/webgpu.h>.

For my understanding, why the need for the subfolder? The name "webgpu.h" seems unique enough by itself. I.e. can't these simply use #include <webgpu.h>?

@Sahnvour
Copy link

For my understanding, why the need for the subfolder? The name "webgpu.h" seems unique enough by itself. I.e. can't these simply use #include <webgpu.h>?

They can, but apparently the consensus is to use <webgpu/webgpu.h> in widely used libraries (that are often consumed via package managers and can't be manually edited by clients), see for example:
https://github.com/ocornut/imgui/blob/67cd4ead6505b3386d4ef9c713c98546e86e26ca/backends/imgui_impl_wgpu.h#L21
https://github.com/floooh/sokol/blob/1eb96dd0f96b9ea73065f9078244c2255c2b75d9/sokol_gfx.h#L4480

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.

Prebuilt releases header files location
4 participants