Fix the build with gcc prior to 5.0

[kantan2015]

Got import error with gcc below 5.0 ImportError: python-crfsuite-0.8.4/pycrfsuite/_pycrfsuite.cpython-35m-x86_64-linux-gnu.so: undefined symbol: isfinite

Build warning crfsuite/lib/crf/src/train_l2sgd.c:197: warning: implicit declaration of function 'isfinite'

"isfinite" can be used after c99 but, with gcc below 5.0, c89 is used by default. Added compiler option to -std=c99 to use c99 in setup.py.

[kmike]

Do you know if it is safe to enable it, i.e. that msvc and clang also support this flag?

[kantan2015]

clang supports the option. http://clang.llvm.org/docs/UsersManual.html#id45 msvc does not but it is safe to use(just shows warning "ignoring unknown option")

[kmike]

Thanks for the clarification!

[kmike]

Hey @kantan2015,

Sorry for bothering you, but I just tried to build it today with clang, and it failed with this error message on OS X:

clang -fno-strict-aliasing -fno-common -dynamic -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Icrfsuite/include/ -Icrfsuite/lib/cqdb/include -Iliblbfgs/include -Ipycrfsuite -I/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/include/python2.7 -c pycrfsuite/_pycrfsuite.cpp -o build/temp.macosx-10.11-x86_64-2.7/pycrfsuite/_pycrfsuite.o -std=c99 error: invalid argument '-std=c99' not allowed with 'C++/ObjC++' error: command 'clang' failed with exit status 1

---

Failed building wheel for python-crfsuite Running setup.py clean for python-crfsuite Failed to build python-crfsuite Installing collected packages: python-crfsuite Running setup.py install for python-crfsuite: started Running setup.py install for python-crfsuite: finished with status 'error' Complete output from command /Users/kmike/svn/python-crfsuite/.tox/py27/bin/python2.7 -u -c "import setuptools, tokenize;file='/private/var/folders/_5/cbsg50991szfp1r9nwxpx8580000gn/T/pip-FH3PxN-build/setup.py';f=getattr(tokenize, 'open', open)(file);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, file, 'exec'))" install --record /var/folders/_5/cbsg50991szfp1r9nwxpx8580000gn/T/pip-P_OU9P-record/install-record.txt --single-version-externally-managed --compile --install-headers /Users/kmike/svn/python-crfsuite/.tox/py27/bin/../include/site/python2.7/python-crfsuite: running install running build running build_py creating build creating build/lib.macosx-10.11-x86_64-2.7 creating build/lib.macosx-10.11-x86_64-2.7/pycrfsuite copying pycrfsuite/init.py -> build/lib.macosx-10.11-x86_64-2.7/pycrfsuite copying pycrfsuite/_dumpparser.py -> build/lib.macosx-10.11-x86_64-2.7/pycrfsuite copying pycrfsuite/_logparser.py -> build/lib.macosx-10.11-x86_64-2.7/pycrfsuite running build_ext building 'pycrfsuite._pycrfsuite' extension creating build/temp.macosx-10.11-x86_64-2.7 creating build/temp.macosx-10.11-x86_64-2.7/pycrfsuite creating build/temp.macosx-10.11-x86_64-2.7/crfsuite creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/lib creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/lib/crf creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/lib/crf/src creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/swig creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/lib/cqdb creating build/temp.macosx-10.11-x86_64-2.7/crfsuite/lib/cqdb/src creating build/temp.macosx-10.11-x86_64-2.7/liblbfgs creating build/temp.macosx-10.11-x86_64-2.7/liblbfgs/lib clang -fno-strict-aliasing -fno-common -dynamic -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -Icrfsuite/include/ -Icrfsuite/lib/cqdb/include -Iliblbfgs/include -Ipycrfsuite -I/usr/local/Cellar/python/2.7.12/Frameworks/Python.framework/Versions/2.7/include/python2.7 -c pycrfsuite/_pycrfsuite.cpp -o build/temp.macosx-10.11-x86_64-2.7/pycrfsuite/_pycrfsuite.o -std=c99 error: invalid argument '-std=c99' not allowed with 'C++/ObjC++' error: command 'clang' failed with exit status 1

Any ideas?

[kantan2015]

Ah, clang treats the "-std=c99" flag on cpp files as error. gcc treats it as warning. I think there are following choices. i) Pass "-std=c99" only for the compilation for c files. It seems distutils doesn't have the option for that other separating Extensions with cpp files and c files. ii) Not to pass "-std=c99" for Mac That is not perfect, clang can be used in the other system. iii) User to set environment variable CFLAGS=-std=c99 if necessary and update the Readme file.

What do you think?

[kmike]

Thanks for looking at it! I like the environment variable approach. Do you know if we can add -std=c99 flag to CFLAGS in setup.py script, some like that - does it work for you?

cflags = os.environ.get('CFLAGS', '')
if '-std=c99' not in cflags:
    cflags += ' -std=c99'
os.environ['CFLAGS'] = cflags

[kantan2015]

Hi kmike, I meant ,if user uses old gcc, he needs to specify the environment variable before running pip or setup.py. CFLAGS=-std=c99 pip install python-crfsuite or CFLAGS=-std=c99 python setup.py

But it seems disutils use sysconfig.get_config_var('CC') as compiler in unix system including linux and MacOS. So we can use it as a check.

from distutils import sysconfig
extra_compile_args=[]
if sysconfig.get_config_var('CC').startswith('gcc'):
     extra_compile_args=['-std=c99']

Probably that is better.I will test it.

[kmike]

@kantan2015 It may be unnecessary to require users to set CFLAGS if we can set in setup.py via os.environ (well, I assume that CFLAGS is harmless and we can set in unconditionally, which may not be the case).

[kmike]

Setting flags as env variables outside setup.py doesn't work well e.g. with requirements.txt files, or with install_requires (e.g. when another package requires python-crfsuite), or with other automated deploy methods, so it'd be great to solve it without requiring users to change flags in their environment.

[kmike]

I like environment variable approach more than sysconfig.get_config_var('CC') because this 'CC' doesn't seem to be documented anywhere. Of course, only if setting env variable in setup.py works :D

[kantan2015]

Hi kmike, Unfortunately, setting CFLAGS also affects the compilation of .cpp files. As I checked, distutils uses same command for .cpp and .c files in compile phase in linux or mac(both handled by distutils/unixccompiler.py). So just setting CFLAGS=-std=c99 and extra_compile_args=['-std=c99'] would result in same.

[kantan2015]

Another way to check compiler is to create extended class of build_ext. This is a bit longer than to use sysconfig.get_config_var but is more accurate. sysconfig.get_config_var('CC') is default compiler but this can check the one to be used.

class build_ext_subclass( build_ext ):
    def build_extensions(self):
        c = self.compiler
        if c.compiler_type == 'unix' and 'gcc' in c.compiler:
            for e in self.extensions:
                e.extra_compile_args=['-std=c99']
        build_ext.build_extensions(self)

setup(
    name='python-crfsuite',
    ...
    cmdclass={ 'build_ext': build_ext_subclass}
)

[kmike]

Thanks for debugging this @kantan2015! Both of your proposed solutions look good to me.

[kantan2015]

Thank you for reviewing it. I tested extended build_ext approach in linux and Mac OS and both look ok. I will send another pr.