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

m4b-tool: init at 0.4.2 #87957

Open
wants to merge 1 commit into
base: master
from
Open

m4b-tool: init at 0.4.2 #87957

wants to merge 1 commit into from

Conversation

@lunik1
Copy link
Contributor

lunik1 commented May 16, 2020

Motivation for this change

Adds m4b-tool - cli for audiobook manipulation. Requires fdkaac added in #87952.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@lunik1 lunik1 requested a review from etu as a code owner May 16, 2020
@lunik1 lunik1 mentioned this pull request May 16, 2020
4 of 10 tasks complete
@ofborg ofborg bot requested review from Ma27, talyz, aanderse and globin May 16, 2020
@lunik1 lunik1 force-pushed the lunik1:add-m4b-tool branch from 3766a65 to 79eb877 May 16, 2020
@lunik1 lunik1 force-pushed the lunik1:add-m4b-tool branch 2 times, most recently from 0738c2f to 020f454 Jun 3, 2020
@etu
Copy link
Contributor

etu commented Jun 4, 2020

Should be rebased on master, and you should be able to drop the commit for the lib and for addition of the maintainer entry :)

@lunik1 lunik1 force-pushed the lunik1:add-m4b-tool branch 2 times, most recently from d320925 to 6a17b4f Jun 4, 2020
@lunik1
Copy link
Contributor Author

lunik1 commented Jun 4, 2020

All done :)

@etu
Copy link
Contributor

etu commented Jun 4, 2020

I just came to think... Is there a point of building three different packages of this package? One for each version of PHP.

I mean, since it's just a tool it should work fine to just use a single version. Some other tools it actually makes a difference for (such as composer, phpcs, phpcbf, phpstan etc). But this doesn't interact with actual PHP code.

So I'm thinking that it would fit better in like pkgs/tools/audio/m4b-tool and be registred in all-packages.nix as a normal package. And then just depend on the default php attribute. That way we only get one variant of the package.

@lunik1
Copy link
Contributor Author

lunik1 commented Jun 4, 2020

It's a fair point. I couldn't see many php packages using that appeoach so that's why I went with php-packages.nix. Looking at python packages, however, it does seem that your approach is taken in many cases - httpie, certbot, streamlink for example (but there are some exceptions, see youtube-dl).

Making this change would also allow for fdkaac fdk-aac-encoder and fdk-enabled ffmpeg to be optional, but default, dependencies. Most of the functionality of m4b-tool seems to require them but is is possible to perform some operations without. Saves users who know what they want a heavy footprint and possibly long build for ffmpeg-full. I'll start looking into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.