Conditionally disable libopusfile via a Go build tag #24
Conversation
|
|
||
| ```sh | ||
| go build -tags nolibopusfile ... | ||
| ``` |
There was a problem hiding this comment.
could you add a blurb here as to why anyone would ever want to do this? It's a pretty esoteric feature and it's good to know you can "safely ignore" it if you're not very experienced with the whole "dependencies, libraries, build systems" yet. Similarly, while the title is technically correct, it's quite generic. Could you reframe it as "Disabling libopusfile (for micro builds)" or something similar? Keep an audience in mind who might be overwhelmed by all the options, and who speaks poor English. It will help keep the issue count down :)
There was a problem hiding this comment.
in fact, the comment in your PR is a pretty good example of what I mean :)
There was a problem hiding this comment.
FWIW it works great on Alpine Linux, both static and dynamically linked to libopus. It even works on scratch with musl libc and libopus* copied from an Alpine builder image.
|
thank you for this patch and for testing it out on Alpine :) |
|
@hraban fixed! |
|
thank you @ydnar. I am running into some issues with travis configuration and I want to make sure the CI runs before merging this in, but that's the only blockade in the way of a merge. once I sort it out on my end it will be ready to go. Sorry for the delay. |
|
@hraban realized later that it’s probably reasonable to |
|
that's not a bad idea, and it would make this a lot easier to test. however I'm not sure about having to maintain two separate versions just for this. many people have v2 installed now, and bumping the major to solve a problem that almost nobody has seems like more trouble than it's worth :/ I'll have another think about it. |
|
Makes sense. Thanks for looking at it! |
|
Hi @ydnar I'm merging these changes in; do you accept me adding you to the AUTHORS file under the following conditions?
The LICENSE file contains:
i.e. MIT license. In other words: do you accept licensing your contributions under MIT? |
|
I'm sticking with the build tag because I don't want to upgrade the major version for this; there are too many installs and nobody would update for it, so I'd have to maintain two versions for a relatively |
|
I accept. Thanks! |
This patch introduces a new build tag `nolibopusfile` that conditionally compiles out the code that imports `libopusfile`. This enables a static binary build on Alpine Linux, which doesn’t have a static `libopusfile`. Tests still work: ```sh go test -tags nolibopusfile ./... go test ./... ``` We could also have moved all libopusfile related code (i.e. Stream) into a separate sub-package, but that would break backwards compatibility. This feature is too unpopular to justify introducing a new major version. See also: #24
|
Closing; superseded by #31 |
He explicitly allowed me to license his contributions under MIT. See #24 (comment)
He explicitly allowed me to license his contributions under MIT. See #24 (comment)
This patch introduces a new build tag `nolibopusfile` that conditionally compiles out the code that imports `libopusfile`. This enables a static binary build on Alpine Linux, which doesn’t have a static `libopusfile`. Tests still work: ```sh go test -tags nolibopusfile ./... go test ./... ``` We could also have moved all libopusfile related code (i.e. Stream) into a separate sub-package, but that would break backwards compatibility. This feature is too unpopular to justify introducing a new major version. See also: #24
He explicitly allowed me to license his contributions under MIT. See #24 (comment)
This PR adds a new build tag
nolibopusfilethat conditionally compiles out the code that importslibopusfile. This enables a static binary build on Alpine Linux, which doesn’t have a staticlibopusfile.Tests still work: