Skip to content

kaldi: fix build#236313

Merged
jtojnar merged 4 commits intomasterfrom
kaldi-fix
Jul 11, 2023
Merged

kaldi: fix build#236313
jtojnar merged 4 commits intomasterfrom
kaldi-fix

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 jtojnar requested a review from Mic92 June 6, 2023 17:33
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Jun 6, 2023
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

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants