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

Fix pkg_config sdl2 string #36

Merged
merged 2 commits into from
Nov 22, 2024
Merged
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion bundlex.exs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ defmodule Membrane.SDL.BundlexProject do
sdl2: [
{:precompiled, Membrane.PrecompiledDependencyProvider.get_dependency_url(:sdl2),
"SDL2"},
{:pkg_config, "SDL2"}
{:pkg_config, "sdl2"}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{:pkg_config, "SDL2"}
{:pkg_config, "sdl2"}
{:pkg_config, "SDL2"},
{:pkg_config, "sdl2"}

I guess the .pc may be named differently on different systems/vendors. Please try leaving them both, if one fails, bundlex should try another

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 have tried it now, but it doesn't seem to work.

==> membrane_sdl_plugin
Bundlex: Building natives: player
warning: Bundlex: couldn't load ["SDL2"] libraries with pkg-config due to:
	** (BundlexError) pkg-config error:
	Code: 1
	Package SDL2 was not found in the pkg-config search path.
	Perhaps you should add the directory containing `SDL2.pc'
	to the PKG_CONFIG_PATH environment variable
	Package 'SDL2', required by 'virtual:world', not found

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it works it just emits a warning now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is here.

Copy link
Member

Choose a reason for hiding this comment

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

Right, I guess we can't get away without it. You can switch the order so the warning will be emitted in the other case :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But it can be fixed in bundlex. I don't see a reason why to emit warning there, if all providers fail it will error anyway.

Copy link
Member

Choose a reason for hiding this comment

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

In this case yes, but when fetching a precompiled dep fails and we fall back to pkg-config, we should inform the user what's happening. Maybe we should only warn if the kind of dependency changes, like from pkg-config to precompiled and vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems reasonable, maybe I'll find some time today and try to make PR on bundlex repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

]
],
preprocessor: Unifex
Expand Down
Loading