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

Break circular dependency between gstreamer and libnice #171902

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

carlocab
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

See Homebrew/discussions#3740, in particular, https://github.com/orgs/Homebrew/discussions/3740#discussioncomment-8566223.

  • gstreamer: enable libnice
  • libnice: disable gstreamer plugin
  • libnice-gstreamer 1.22 (new formula)

@carlocab carlocab added the CI-skip-dependents Pass --skip-dependents to brew test-bot. label May 16, 2024
@github-actions github-actions bot added new formula PR adds a new formula to Homebrew/homebrew-core long build Needs CI-long-timeout labels May 16, 2024
@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label May 16, 2024
Comment on lines 34 to 43
test do
# Based on https://github.com/libnice/libnice/blob/HEAD/examples/simple-example.c
(testpath/"test.c").write <<~EOS
#include <agent.h>
int main(int argc, char *argv[]) {
NiceAgent *agent;
GMainLoop *gloop;
gloop = g_main_loop_new(NULL, FALSE);
// Create the nice agent
agent = nice_agent_new(g_main_loop_get_context (gloop),
NICE_COMPATIBILITY_RFC5245);
if (agent == NULL)
g_error("Failed to create agent");

g_main_loop_unref(gloop);
g_object_unref(agent);
return 0;
}
EOS

pkg_config_cflags = shell_output("pkg-config --cflags --libs nice").chomp.split
system ENV.cc, "test.c", *pkg_config_cflags, "-o", "test"
system "./test"
end
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably need a new test.

Copy link

@nirbheek nirbheek May 16, 2024

Choose a reason for hiding this comment

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

Checking whether gst-inspect-1.0 --exists nicesrc returns 0 is probably a good test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably I would need to set some environment variables for this to work, since GStreamer wants to find all the plugins in its own prefix?

Choose a reason for hiding this comment

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

Hm, so this should actually be built as part of the gstreamer monorepo then. Let me cook up a patch to do that.

Choose a reason for hiding this comment

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

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/6871

So, instead of creating a new libnice-gstreamer.rb formula, you'd patch gstreamer.rb

@carlocab carlocab added CI-linux-self-hosted Build on Linux self-hosted runner and removed CI-skip-dependents Pass --skip-dependents to brew test-bot. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels May 17, 2024
@carlocab carlocab force-pushed the gstreamer-libnice branch 2 times, most recently from 2f8ee2a to d81b906 Compare May 17, 2024 17:30
@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label May 17, 2024
@carlocab carlocab removed the automerge-skip `brew pr-automerge` will skip this pull request label May 17, 2024
@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label May 18, 2024
@carlocab carlocab added CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. and removed automerge-skip `brew pr-automerge` will skip this pull request labels May 18, 2024
@github-actions github-actions bot added the automerge-skip `brew pr-automerge` will skip this pull request label May 18, 2024
Also, add dependencies that are linked indirectly.
This will allow us to build `gstreamer` with `libnice` as a dependency.

Also, add `gettext` dependency on macOS, since this has indirect linkage
to it.
This installs only the GStreamer plugin for libnice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip `brew pr-automerge` will skip this pull request CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Needs CI-long-timeout new formula PR adds a new formula to Homebrew/homebrew-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants