Skip to content
This repository has been archived by the owner. It is now read-only.

openfst: add python option and a description #3010

Closed
wants to merge 2 commits into from

Conversation

@Moisan
Copy link
Member

commented Nov 13, 2015

Following issue #2920, this PR adds the option to include the python bindings. However I couldn't get audit to pass:

* python modules have explicit framework links
These python extension modules were linked directly to a Python
framework binary. They should be linked with -undefined dynamic_lookup
instead of -lpython or -framework Python.
  /usr/local/Cellar/openfst/1.5.0/lib/python2.7/site-packages/fst.so

How can I fix this?

@Moisan

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2015

Also, to get the python module to work I had to run

export PYTHONPATH=/usr/local/lib/python2.7/site-packages:$PYTHONPATH

Maybe it should be added to the formula caveat.

openfst.rb Outdated
@@ -13,21 +12,31 @@ class Openfst < Formula
end

needs :cxx11
depends_on :python => :optional

This comment has been minimized.

Copy link
@dpo

dpo Dec 17, 2015

Contributor

unless OS.mac? && MacOS.version > :snow_leopard

@dpo

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2015

It seems your branch has conflicts. Would you mind rebasing against master?

@dpo

This comment has been minimized.

Copy link
Contributor

commented Dec 30, 2015

@BrewTestBot test this please

@Moisan

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2016

Jenkins seems to have issues unrelated with this formula (Failed to read test report file).

@dpo

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2016

Sorry for the long delay. That's an intermittent Jenkins hiccup. But your formula needs a revision number.

openfst.rb Outdated
@@ -11,20 +13,30 @@ class Openfst < Formula
end

needs :cxx11
depends_on :python => :optional unless OS.mac? && MacOS.version > :snow_leopard

This comment has been minimized.

Copy link
@dpo

dpo Feb 21, 2016

Contributor

Please convert this to an if ... <= :snow_leopard and squash your commits together. Thanks!

@dpo

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2016

@BrewTestBot test this please

2 similar comments
@Moisan

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2016

@BrewTestBot test this please

@dpo

This comment has been minimized.

Copy link
Contributor

commented May 16, 2016

@BrewTestBot test this please

@dpo

This comment has been minimized.

Copy link
Contributor

commented May 17, 2016

Sorry (again) for the long delay. It seems your branch now has conflicts. Would you mind rebasing?

@dpo dpo added the needs response label May 19, 2016

@ghost ghost removed the needs response label Jun 7, 2016

@iMichka

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

@Moisan Could you please rebase this? We deliver Openfst 1.6.1 now. I can help with the audit failure, let's see what the bots are saying first.

@iMichka

This comment has been minimized.

Copy link
Member

commented Feb 23, 2017

I gave this a try, and the problem is still there with the latest version. We need to open an issue upstream, I will do this during the weekend.

These python extension modules were linked directly to a Python
framework binary. They should be linked with -undefined dynamic_lookup
instead of -lpython or -framework Python.
/usr/local/opt/openfst/lib/python2.7/site-packages/pywrapfst.so

@iMichka

This comment has been minimized.

@belambert belambert referenced this pull request Sep 11, 2017
0 of 1 task complete
@stale

This comment has been minimized.

Copy link

commented Oct 31, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Oct 31, 2017

@stale stale bot closed this Nov 7, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.