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

kaldi: fix build #236313

Merged
merged 4 commits into from
Jul 11, 2023
Merged

kaldi: fix build #236313

merged 4 commits into from
Jul 11, 2023

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Jun 6, 2023

Description of changes

It looks like after a bump, CMake started passing --git-dir=.git as the first argument of git rev-parse. But we do not really need to spoof git program now that Kaldi uses CMake’s FetchContent module. We can just pass the path using configure flag directly:

https://cmake.org/cmake/help/latest/module/FetchContent.html#variable:FETCHCONTENT_SOURCE_DIR_%3CuppercaseName%3E

Git was also inadvertently used for generating KALDI_PATCH_NUMBER in VersionHelper, to be potentially appended to KALDI_VERSION but it was not actually being done by default. We can set the variable directly to skip VersionHelper.

Also remove redundant enableParallelBuild since CMake enables it.

And drop separate dev output since Kaldi currently hardcodes the include/ directory so consumers would not be able to find them. Splitting it out only removes 7 out of 303 MB.

Things done

jtojnar added 4 commits June 6, 2023 22:02
It looks like after a bump, CMake started passing `--git-dir=.git` as the first argument of `git rev-parse`.
But we do not really need to spoof `git` program now that Kaldi uses CMake’s `FetchContent` module.
We can just pass the path using configure flag directly:

https://cmake.org/cmake/help/latest/module/FetchContent.html#variable:FETCHCONTENT_SOURCE_DIR_%3CuppercaseName%3E

Git was also inadvertently used for generating `KALDI_PATCH_NUMBER`
in `VersionHelper`, to be potentially appended to `KALDI_VERSION`
but it was not actually being done by default.
We can set the variable directly to skip `VersionHelper`.

Also remove redundant `enableParallelBuild` since CMake enables it.

And drop separate `dev` output since Kaldi currently hardcodes
the `include/` directory so consumers would not be able to find them.
Splitting it out only removes 7 out of 303 MB.
It is shadowed by fetchgit source and can only be used vendored.
Also switch to finalAttrs so that the values can be easily overridden.
@jtojnar jtojnar merged commit 65dbea5 into master Jul 11, 2023
@jtojnar jtojnar deleted the kaldi-fix branch July 11, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants