Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/build-pip.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest, ubuntu-24.04-arm, macos-13]
os: [ubuntu-latest, macos-latest, windows-latest, ubuntu-24.04-arm, macos-15-intel]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
fail-fast: false
steps:
Expand All @@ -61,7 +61,13 @@ jobs:
python3 setup.py bdist_wheel
ls -lh dist
unzip -l ./dist/*.whl

python3 -m pip install ./dist/kaldi*.whl

- name: Test Python
shell: bash
run: |
cd ../..

python3 -c "import kaldi_decoder; print(kaldi_decoder.__version__)"
python3 -c "import kaldi_decoder; print(kaldi_decoder.__file__)"
35 changes: 20 additions & 15 deletions .github/workflows/build-wheels-linux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: Install dependencies
run: |
python3 -m pip install --upgrade pip
python3 -m pip install wheel twine==5.0.0 setuptools

- name: Build sdist
if: matrix.python-version == 'cp38'
shell: bash
run: |
python3 setup.py sdist
ls -lh dist/*

- name: Publish sdist to PyPI
if: matrix.python-version == 'cp38'
env:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
twine upload --verbose dist/kaldi_decoder-*.tar.gz

# see https://cibuildwheel.readthedocs.io/en/stable/changelog/
# for a list of versions
- name: Build wheels
Expand Down Expand Up @@ -56,18 +76,3 @@ jobs:
python3 -m pip install wheel twine==5.0.0 setuptools

twine upload ./wheelhouse/*.whl

- name: Build sdist
if: matrix.python-version == 'cp38'
shell: bash
run: |
python3 setup.py sdist
ls -lh dist/*

- name: Publish sdist to PyPI
if: matrix.python-version == 'cp38'
env:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
twine upload dist/kaldi-decoder-*.tar.gz
6 changes: 3 additions & 3 deletions .github/workflows/build-wheels-macos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jobs:
shell: bash
run: |
ls -lh ./wheelhouse/
unzip -l ./wheelhouse/*.whl

- uses: actions/upload-artifact@v4
with:
Expand All @@ -60,8 +61,7 @@ jobs:
TWINE_USERNAME: ${{ secrets.PYPI_USERNAME }}
TWINE_PASSWORD: ${{ secrets.PYPI_PASSWORD }}
run: |
opts='--break-system-packages'
python3 -m pip install $opts --upgrade pip
python3 -m pip install $opts wheel twine==5.0.0 setuptools
# python3 -m pip install --break-system-packages --upgrade pip
python3 -m pip install --break-system-packages wheel twine==5.0.0 setuptools

twine upload ./wheelhouse/*.whl
1 change: 1 addition & 0 deletions .github/workflows/build-wheels-win32.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jobs:
shell: bash
run: |
ls -lh ./wheelhouse/
unzip -l wheelhouse/*.whl

- uses: actions/upload-artifact@v4
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/build-wheels-win64.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ jobs:
shell: bash
run: |
ls -lh ./wheelhouse/
unzip -l wheelhouse/*.whl

- uses: actions/upload-artifact@v4
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/linux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ permissions:

jobs:
linux:
name: ${{ matrix.python-version }}
name: ${{ matrix.os }} ${{ matrix.python-version }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/macos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ permissions:

jobs:
macos:
name: ${{ matrix.python-version }}
name: ${{ matrix.os }} ${{ matrix.python-version }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [macos-latest, macos-13]
os: [macos-latest, macos-15-intel]
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]

steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ permissions:

jobs:
windows:
name: ${{ matrix.python-version }}
name: ${{ matrix.os }} ${{ matrix.python-version }}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
Expand Down
17 changes: 7 additions & 10 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.24.0")
cmake_policy(SET CMP0135 NEW)
endif()

set(KALDI_DECODER_VERSION "0.2.11")
set(KALDI_DECODER_VERSION "0.3.0")

set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib")
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib")
Expand All @@ -35,21 +35,18 @@ if(NOT CMAKE_BUILD_TYPE)
endif()
message(STATUS "CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")

set(CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ version to be used.")
set(CMAKE_CXX_EXTENSIONS OFF)
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 17 CACHE STRING "The C++ version to be used.")
set(CMAKE_CXX_EXTENSIONS OFF)
endif()

list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)

option(BUILD_SHARED_LIBS "Whether to build shared libraries" ON)
option(KALDI_DECODER_ENABLE_TESTS "Whether to build tests" ON)
option(KALDI_DECODER_BUILD_PYTHON "Whether to build Python" ON)

if(BUILD_SHARED_LIBS AND MSVC)
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()

if(BUILD_SHARED_LIBS)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
if(WIN32)
add_definitions(-DNOMINMAX) # Otherwise, std::max() and std::min() won't work
endif()

if(KALDI_DECODER_BUILD_PYTHON)
Expand Down
19 changes: 7 additions & 12 deletions cmake/kaldifst.cmake
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
function(download_kaldifst)
include(FetchContent)

set(kaldifst_URL "https://github.com/k2-fsa/kaldifst/archive/refs/tags/v1.7.16.tar.gz")
set(kaldifst_HASH "SHA256=f338135a3d137b7a8c287e2ca2e0918e1394e1c08bad53b26e7be03a5d4a1066")
set(kaldifst_URL "https://github.com/k2-fsa/kaldifst/archive/refs/tags/v1.8.0.tar.gz")
set(kaldifst_HASH "SHA256=3f247b7e5a2409071202f5e2bc6200060f66728c0a3443c03923ad2723e040b3")

# If you don't have access to the Internet,
# please pre-download kaldifst
set(possible_file_locations
$ENV{HOME}/Downloads/kaldifst-1.7.16.tar.gz
${CMAKE_SOURCE_DIR}/kaldifst-1.7.16.tar.gz
${CMAKE_BINARY_DIR}/kaldifst-1.7.16.tar.gz
/tmp/kaldifst-1.7.16.tar.gz
/star-fj/fangjun/download/github/kaldifst-1.7.16.tar.gz
$ENV{HOME}/Downloads/kaldifst-1.8.0.tar.gz
${CMAKE_SOURCE_DIR}/kaldifst-1.8.0.tar.gz
${CMAKE_BINARY_DIR}/kaldifst-1.8.0.tar.gz
/tmp/kaldifst-1.8.0.tar.gz
/star-fj/fangjun/download/github/kaldifst-1.8.0.tar.gz
Comment on lines +10 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The list of possible file locations contains a hardcoded absolute path that is specific to a local development environment. This should be removed to ensure the build remains portable and does not leak local system information.

    $ENV{HOME}/Downloads/kaldifst-1.8.0.tar.gz
    ${CMAKE_SOURCE_DIR}/kaldifst-1.8.0.tar.gz
    ${CMAKE_BINARY_DIR}/kaldifst-1.8.0.tar.gz
    /tmp/kaldifst-1.8.0.tar.gz

)

foreach(f IN LISTS possible_file_locations)
Expand Down Expand Up @@ -64,11 +64,6 @@ function(download_kaldifst)
)

set_target_properties(kaldifst_core PROPERTIES OUTPUT_NAME "kaldi-decoder-kaldi-fst-core")

if(NOT BUILD_SHARED_LIBS)
install(TARGETS kaldifst_core DESTINATION lib)
endif()

endfunction()

download_kaldifst()
106 changes: 0 additions & 106 deletions cmake/openfst.cmake

This file was deleted.

13 changes: 9 additions & 4 deletions kaldi-decoder/csrc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ set(srcs
# Always static build
add_library(kaldi-decoder-core STATIC ${srcs})

if(NOT WIN32)
set_target_properties(kaldi-decoder-core
PROPERTIES
POSITION_INDEPENDENT_CODE ON
C_VISIBILITY_PRESET hidden
CXX_VISIBILITY_PRESET hidden
)
endif()

target_link_libraries(kaldi-decoder-core PUBLIC kaldifst_core)
target_link_libraries(kaldi-decoder-core PUBLIC Eigen3::Eigen)

Expand Down Expand Up @@ -42,7 +51,3 @@ if(KALDI_DECODER_ENABLE_TESTS)
kaldi_decoder_add_test(${source})
endforeach()
endif()

if(NOT BUILD_SHARED_LIBS)
install(TARGETS kaldi-decoder-core DESTINATION lib)
endif()
6 changes: 2 additions & 4 deletions kaldi-decoder/python/csrc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ if(APPLE)
if(PYTHON_SITE_PACKAGE_DIR STREQUAL "")
message(WARNING "PYTHON_SITE_PACKAGE_DIR is empty!")
else()
target_link_libraries(_kaldi_decoder PRIVATE "-Wl,-rpath,${PYTHON_SITE_PACKAGE_DIR}")
target_link_libraries(_kaldi_decoder PRIVATE "-Wl,-rpath,${PYTHON_SITE_PACKAGE_DIR}/kaldi_decoder/lib")
endif()
endif()

if(NOT WIN32)
target_link_libraries(_kaldi_decoder PRIVATE "-Wl,-rpath,${KALDI_DECODER_RPATH_ORIGIN}/kaldi_decoder/lib")
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The rpath construction appears to be incorrect. Since the library _kaldi_decoder is installed directly into the lib directory (i.e., kaldi_decoder/lib/ relative to the prefix), ${KALDI_DECODER_RPATH_ORIGIN} (which expands to $ORIGIN on Linux or @loader_path on macOS) already points to that directory. Appending /kaldi_decoder/lib creates a non-existent nested path like .../kaldi_decoder/lib/kaldi_decoder/lib.

  target_link_libraries(_kaldi_decoder PRIVATE "-Wl,-rpath,${KALDI_DECODER_RPATH_ORIGIN}")

endif()

install(TARGETS _kaldi_decoder
DESTINATION ../
)
install(TARGETS _kaldi_decoder DESTINATION lib)
2 changes: 1 addition & 1 deletion kaldi-decoder/python/kaldi_decoder/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from _kaldi_decoder import (
from kaldi_decoder.lib._kaldi_decoder import (
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a relative import is more idiomatic and robust here. It avoids hardcoding the package name, which makes the code more maintainable if the package is ever renamed or vendored.

Suggested change
from kaldi_decoder.lib._kaldi_decoder import (
from .lib._kaldi_decoder import (

DecodableCtc,
DecodableInterface,
FasterDecoder,
Expand Down
Empty file.
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def get_package_version():
return latest_version


package_name = "kaldi-decoder"
package_name = "kaldi_decoder"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Changing the distribution name from kaldi-decoder to kaldi_decoder is a breaking change for users who install the package via PyPI or depend on it in requirements.txt. It is also inconsistent with the repository name and the project URL. Unless this change is intentional for a specific branding reason, it is recommended to keep the hyphenated name for the distribution.

Suggested change
package_name = "kaldi_decoder"
package_name = "kaldi-decoder"


with open("kaldi-decoder/python/kaldi_decoder/__init__.py", "a") as f:
f.write(f"__version__ = '{get_package_version()}'\n")
Expand Down
Loading