espeak-ng: Pick mbrola voices from XDG_DATA_DIRS #325124
Open
+108
−82
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
2bd3a22 to
7859e5a
Compare
github-actions
bot
added
6.topic: nixos
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
4a3b15c to
5dcbae4
Compare
chuangzhu
reviewed
Jul 7, 2024
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
5dcbae4 to
206f1a0
Compare
This comment was marked as resolved.
This comment was marked as resolved.
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
aa8f836 to
acaf129
Compare
Extracted from mbrola.
This is required for packages to depend on `mbrola` while keeping the dependency on `mbrola-voices` implicit, since it is huge. It will be eventually picked from the environment. The old `mbrola` package is exposed as `mbrola.combined`. Our `speechd` patch is only using the voices.
Upstream espeak-ng expects one of the following paths: - $data_dir/mbrola/$lang - $data_dir/mbrola/$lang/$lang - $data_dir/mbrola/voices/$lang The previously used `$data_dir/mbrola/voices/$lang/$lang` is not supported. Let’s change the installation path to the second one, so that espeak-ng can still find mbrola voices once we drop our patch. speechd also expects one of the the paths above.
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
acaf129 to
e406b9b
Compare
jtojnar
commented
Jul 7, 2024
nixos/modules/programs/mbrola.nix
Outdated
| in | ||
| { | ||
| options.programs.mbrola = { | ||
| enable = lib.mkEnableOption "linking mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)"; |
There was a problem hiding this comment.
I am not very happy with this option.
There was a problem hiding this comment.
Suggested change
| enable = lib.mkEnableOption "linking mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)"; | |
| enable = lib.mkEnableOption "" // { description = "Wether to link mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)."; }; |
nixos/modules/programs/mbrola.nix
Outdated
| in | ||
| { | ||
| options.programs.mbrola = { | ||
| enable = lib.mkEnableOption "linking mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)"; |
There was a problem hiding this comment.
Suggested change
| enable = lib.mkEnableOption "linking mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)"; | |
| enable = lib.mkEnableOption "" // { description = "Wether to link mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)."; }; |
nixos/modules/programs/mbrola.nix
Outdated
| options.programs.mbrola = { | ||
| enable = lib.mkEnableOption "linking mbrola speech synthesizer voices installed through [](#opt-environment.systemPackages)"; | ||
|
|
||
| installDefaultVoices = lib.mkOption { |
There was a problem hiding this comment.
I would have expected a package option instead of this one.
There was a problem hiding this comment.
The plan is to eventually split the mbrola-voices package: #226869
pkgs/by-name/mb/mbrola/package.nix
Outdated
| ''; | ||
|
|
||
| passthru = { | ||
| combined = runCommandLocal "mbrola-combined-${finalAttrs.version}" { inherit (finalAttrs) meta; } '' |
There was a problem hiding this comment.
We could make a nicer git history if we didn't include this intermediate step in the commits.
There was a problem hiding this comment.
If we do not include this step then some of the commits will be broken.
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
e406b9b to
471b9ed
Compare
These provide slightly nicer pronunciation. Since we plan to stop hardcoding the path to the voices in `espeak-ng`, we need to pull them through the NixOS module.
This will require that user installs e.g. `mbrola-voices` in such a way its `share/` directory ends up in `XDG_DATA_DIRS` environment variable. The NixOS module change in the parent commit will handle that automatically for GNOME.
jtojnar
force-pushed
the
espeak-mbrola-size
branch
from
471b9ed to
5f1ff0f
Compare
wegank
added
the
2.status: merge conflict
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Fixes: #207204, #261281
Needed for: #226869
Things done
env XDG_DATA_DIRS=(nix-build -A mbrola-voices --no-out-link)/share (nix-build -A espeak-ng)/bin/espeak "Hello world!" -v mb-en1nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.