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.

Sign up for GitHub

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

Jump to bottom

Fix build with latest version of openfst #4096

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix build with latest version of openfst #4096

wants to merge 8 commits into from
+158 −117

Conversation

sandsmark
Copy link

What it says on the tin

@sandsmark
Copy link
Author

OK, openfst is apparently not backwards compatible (as in they deprecate things at the same time they introduce them).

Should I leave this open for when you update the bundled openfst?

@kkm000
Copy link
Contributor

kkm000 commented Jun 25, 2020

Yes, please. I'm looking into making Kaldi compatible with either, your changeset should be helpful!

@kkm000 kkm000 self-assigned this Jun 25, 2020
@kkm000 kkm000 added duplicate in progress Issue has been taken and is being worked on stale-exclude Stale bot ignore this issue labels Jun 25, 2020
@kkm000
Copy link
Contributor

kkm000 commented Jun 25, 2020

Xref #4131

@kkm000 kkm000 removed the duplicate label Jun 25, 2020
@sandsmark
Copy link
Author

if they hadn't changed FstPrinter it would be fairly trivial to add a couple of ifdefs to support both, but because of that change it would end up a complete mess...

@kdorichev
Copy link

kdorichev commented Dec 21, 2020 •
edited

Is it still a requirement to use OpenFST 1.6.7 and the latest 1.8.0 is not yet supported?

rkjaran added a commit to tiro-is/kaldi that referenced this pull request May 4, 2021
@rkjaran
Merge PR kaldi-asr#4096
d0a040c 
Fix build with latest version (>=1.7.7) of openfst.

This is not yet merged upstream because of a breaking change in OpenFST
@kkm000 kkm000 mentioned this pull request May 21, 2021
Open
@danpovey
Copy link
Contributor

I think the only way we can make it compatible with the new version of Openfst without forcing an instant upgrade is to add, e.g. in fstext/fstext-utils.h, some kind of wrapper for Openfst's Print that uses an #ifdef of some kind to test the Openfst version.

This was referenced Mar 26, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@kkm000 kkm000

Labels
in progress Issue has been taken and is being worked on stale-exclude Stale bot ignore this issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sandsmark @kkm000 @kdorichev @danpovey