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

ffmpeg: Add option to build without GPL. #18909

Merged
merged 3 commits into from Oct 4, 2017
Merged

Conversation

techdragon
Copy link
Contributor

@techdragon techdragon commented Oct 3, 2017

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • [] Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • [] Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Small change, it should help when using homebrew to build shared libraries as part of CI toolchains where GPL licensed FFMPEG shared libraries are unsuitable. With this change the existing behaviour of GPL builds should remain the default and continue to work unchanged. Its possible the build flags may need more sophistication regarding without-gpl and conflicting flags/options to do with GPL licensed components of FFMPEG, but exhaustively installing and uninstalling and reinstalling the combinations of options and flags to be sure is not something I really want to run by hand. (which is why I haven't done a local install with this option)

Edit: checklist syntax

Should help when using homebrew to build shared libraries as part of CI toolchains where GPL licensed FFMPEG shared libraries are unsuitable.
--enable-version3
--enable-hardcoded-tables
--enable-avresample
--cc=#{ENV.cc}
--host-cflags=#{ENV.cflags}
--host-ldflags=#{ENV.ldflags}
]


args << "--enable-gpl" if not build.without? "gpl"
Copy link
Member

Choose a reason for hiding this comment

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

args << "--enable-gpl" if build.with? "gpl"

Is the preferred syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I was watching the Jenkins log output and saw the style check complaint. I've issued an update to the file.

Copy link
Contributor Author

@techdragon techdragon left a comment

Choose a reason for hiding this comment

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

Fix mistake, I don't normally write ruby.

@ilovezfs
Copy link
Contributor

ilovezfs commented Oct 3, 2017

==> brew style ffmpeg
==> FAILED
== /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/ffmpeg.rb ==
C:122:  1: Trailing whitespace detected.

1 file inspected, 1 offense detected

@@ -57,6 +57,7 @@ class Ffmpeg < Formula
option "without-securetransport", "Disable use of SecureTransport"
option "without-x264", "Disable H.264 encoder"
option "without-xvid", "Disable Xvid MPEG-4 video encoder"
option "without-gpl", "Disable building GPL Licensed parts of FFMPEG"
Copy link
Contributor

Choose a reason for hiding this comment

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

Licensed shouldn't be capitalized.

It's FFmpeg not FFMPEG.

@techdragon
Copy link
Contributor Author

Looks like it passes the CI test now. I checked the console output and noticed the brew audit in Jenkins isn't run with the --strict flag and Jenkins also only runs brew install not brew install --build-from-source. Will I need to brew install --build-from-source <formula> and brew audit --strict <formula> before this can be merged?

@ilovezfs ilovezfs merged commit 53c199e into Homebrew:master Oct 4, 2017
@ilovezfs
Copy link
Contributor

ilovezfs commented Oct 4, 2017

Thanks for your first contribution to Homebrew, @techdragon! Without people like you submitting PRs we couldn't run this project. You rock!

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants