Sorry I missed this...thanks for the in-depth analysis! On Tue, Oct 16, 2018 at 3:16 AM Lam, Tiago <[email protected]> wrote: > > On 15/10/2018 17:44, Ilya Maximets wrote: > > This reverts commit a7be68a4d77791bbe02c37f7ad8ae60b02e5679e > > and a subsequent commit 4617d1f6bd24c543f533f6485b42ebca6b0a8371. > > There are too many issues with these patches. It's better to revert > > them for now and make a separate fixed versions later if needed. > > > > List of issues (maybe not full): > > > > 1. 'make clean' removes entire 'python' directory.
Doh. Using a build dir instead of building in-tree hid this from me. just changing it to delete $builddir/python/ovs/*.so fixes > > 2. Fully broken Travis-CI testsuite build: > > building 'ovs._json' extension > > creating build/temp.linux-x86_64-2.7 > > error: could not create 'build/temp.linux-x86_64-2.7': \ > > Permission denied > > https://travis-ci.org/openvswitch/ovs/jobs/440693765 Is this a problem with the patch? It looks more like an issue with the testing infrastructure. Why wouldn't we have permission to write to a builddir? If not, does anyone have any idea what needs to be done to work around this restriction? It's handy being able to use python setup.py to build the extensions as opposed to having to maintain build instructions that match what it would do. Using autoconf/automake with python projects is such a pita...especially in this chicken-and-egg-prone situation where we are building C extensions that require libovs to link to. I get how it is really handy to be able to test that the Python IDL code does exactly what the C does, but not being able to use the Python-style testing and build processes in a Python project makes it really hard to do things "right". As an aside, the autoconf based tests for the json parser are kind of brittle. They end up requiring sorted output of keys, etc. since they do blind string-matching on the outputs even though other output would be perfectly acceptable JSON. There's no need to match on whitespace, key order, etc. If both implementations can take in json text and compare json, we could test that parsed(test_correct_output) == parsed(input) maybe? > > 3. Broken local testsuite build on Ubuntu 18.04: > > running build_ext > > building 'ovs._json' extension > > creating build/temp.linux-x86_64-3.6 > > creating build/temp.linux-x86_64-3.6/ovs > > <...> > > /usr/bin/ld: .libs/libopenvswitch.a(util.o): \ > > relocation R_X86_64_TPOFF32 against `var.7749' can not be \ > > used when making a shared object; recompile with -fPIC > > <...> > > collect2: error: ld returned 1 exit status This just looks scary as python setup.py build_ext is configured to try to build the extension and if it fails, falls back to building the package w/o speedups and exits 0, so the build actually completes. I'll see if I can only do this if --enable-shared to avoid the scary output. > > 4. Fedora build failure because of 'setuptools' ('distutils') > > hard dependency on 'redhat-rpm-config' package: > > building 'ovs._json' extension > > <...> > > gcc: error: <...>/redhat-hardened-cc1: No such file or directory This is like above in that the extension just isn't built, but the build completes. This sounds like a packaging issue if redhat-rpm-config is required, but isn't listed as a dependency of python-setuptools? How would you recommend proceeding on this? > > 5. Looks like 'setuptools' also could download and install > > unwanted python modules during package build. It's a Python project. Setuptools is a pretty standard requirement for those. I fail to see the issue with this one. > (I'm CC'ing the author of the patch since I don't see him in the address > list for this patch). > > Thanks, Ilya. I hadn't seen this patch before submitting mine. > > The problem I was seeing with 3. is that beginning with commit a7be68a > the build now tries to build the Python JSON C extension by default > (shared), and tries to link to libopenvswitch, which most likely hasn't > been built with a "-fPIC" flag, producing several `ld` errors and "***" > warnings. So, instead of trying to build the C extension and using the > Python lib as an alternative if the former fails, which ends up > polluting the build system, I think we could try to detect which JSON > extension to use before proceeding. Perhaps detecting for > "--enable-shared" could be sufficient here, and only try to build the > extension if one builds a shared version of libopenvswitch. > > But I agree that this needs to be rethought. > > Tiago. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
