Skip to content

Added pkgconfig file for GLFW #17100

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

Merged
merged 9 commits into from
Jun 3, 2022
Merged

Added pkgconfig file for GLFW #17100

merged 9 commits into from
Jun 3, 2022

Conversation

nthirtyone
Copy link
Contributor

Addresses #2387 for GLFW.

Configuration misses prefix and other usual variables to stay in line with existing files. Moreover for the same reason it does not define Cflags with include directory path. If it is desirable I can look into ways of defining these variables for all of the libraries that are part of the core distribution (but this requires way more research into Emscripten on my part to get it done in a solid way). For now, I just personally needed GLFW, made a config for it, and decided to share.

Name: GLFW
Description: A multi-platform library for OpenGL, window and input
Version: 3.2.1
Libs: -s USE_GLFW=3
Copy link
Collaborator

Choose a reason for hiding this comment

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

We tend to recommend no space after -s these days.

@sbc100
Copy link
Collaborator

sbc100 commented May 29, 2022

Could we add some kind of test for this? Would it be best to test directly by running pkg-config or indirectly via autoconf/cmake?

If you want to test directly you could run emmake /path/to/pkg-config which has the effect to setting PKG_CONFIG_LIBDIR in the environment

@sbc100 sbc100 enabled auto-merge (squash) May 31, 2022 21:56
@nthirtyone
Copy link
Contributor Author

I missed enabling the test in test-windows, but that should be it from my side afterwards if the tests will pass.

auto-merge was automatically disabled May 31, 2022 22:00

Head branch was pushed to by a user without write access


@wraps(func)
def decorated(self, *args, **kwargs):
if not utils.which('pkg-config'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a EMTEST_SKIP_PKG_CONFIG setting (see the existing EMTEST_SKIP_V8) so that we can disable these tests in certain environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done in 211f806

@@ -439,7 +439,7 @@ jobs:
- pip-install:
python: "$EMSDK_PYTHON"
- run-tests:
test_targets: "other.test_es5_transpile other.test_emcc_cflags other.test_stdin other.test_bad_triple core2.test_sse1 core2.test_ccall other.test_closure_externs other.test_binaryen_debug other.test_js_optimizer_parse_error other.test_output_to_nowhere other.test_emcc_dev_null other.test_cmake* other.test_system_include_paths other.test_emar_response_file core2.test_utf16 other.test_special_chars_in_arguments other.test_toolchain_profiler other.test_realpath_nodefs other.test_response_file_encoding other.test_libc_progname other.test_realpath other.test_embed_file_dup other.test_dot_a_all_contents_invalid"
test_targets: "other.test_es5_transpile other.test_emcc_cflags other.test_stdin other.test_bad_triple core2.test_sse1 core2.test_ccall other.test_closure_externs other.test_binaryen_debug other.test_js_optimizer_parse_error other.test_output_to_nowhere other.test_emcc_dev_null other.test_cmake* other.test_system_include_paths other.test_emar_response_file core2.test_utf16 other.test_special_chars_in_arguments other.test_toolchain_profiler other.test_realpath_nodefs other.test_response_file_encoding other.test_libc_progname other.test_realpath other.test_embed_file_dup other.test_dot_a_all_contents_invalid other.test_pkg_config*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just landed a change to reformat this list into multiple lines, so I'm afraid you will need to rebase this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, no worries.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 2, 2022

Looks like a rebase/merge is needed

@nthirtyone
Copy link
Contributor Author

Failure in test-browser-chrome appears to be unrelated. I don't have permissions to rerun it. I guess I could push something to trigger it, but that would invalidate the review, so no matter what it needs maintainer attention.

@kripken kripken merged commit 0138013 into emscripten-core:main Jun 3, 2022
@kripken
Copy link
Member

kripken commented Jun 3, 2022

Would it be worth mentioning stuff like this in the changelog perhaps?

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.

3 participants