virtualenv links against wrong python #978

[themperek]

Addressing #978. It seems cleaner.

PS. In case this gets accepted please squash.

[imphil]

Thanks for this improvement @themperek, looks cleaner indeed.

However, I'm a bit worried about the performance impact of calling Python multiple times. Can we get away with calling PYTHON_BIN once, and then letting make split the result into the individual variables?

[themperek]

Can we get away with calling PYTHON_BIN once, and then letting make split the result into the individual variables?

We can but then we need to parse this with some Unix tools and this will be ugly (in my opinion).

[eric-wieser]

What if we invoked python once, and had it output makefile syntax? $(eval $(shell cocotb-config get-makefile-vars))

[themperek]

Seems like we still (how long @imphil ?) want to be able to use cocotb without cocotb-config?

[eric-wieser]

According to the implementation of python-config.py, the implementation should be something like:

    elif opt in ('--libs', '--ldflags'):
        libs = []
        if '--embed' in opt_flags:
            libs.append('-lpython' + pyver + sys.abiflags)
        else:
            libpython = getvar('LIBPYTHON')
            if libpython:
                libs.append(libpython)
        libs.extend(getvar('LIBS').split() + getvar('SYSLIBS').split())

        # add the prefix/lib/pythonX.Y/config dir, but only if there is no
        # shared library in prefix/lib/.
        if opt == '--ldflags':
            if not getvar('Py_ENABLE_SHARED'):
                libs.insert(0, '-L' + getvar('LIBPL'))
        print(' '.join(libs))

[themperek]

Maybe we should wait with all those changes until we can use cocotb-config.

[imphil]

Make can do word-splitting natively with $(word n,text) (https://www.gnu.org/software/make/manual/html_node/Text-Functions.html), with one important limitation: spaces spaces in paths are not supported. This is a general make limitation, not only for this function.

However, it feels to me that the "no spaces in paths" rule wasn't as important until now as most paths to source files are relative. With absolute paths and Windows added to the mix it's a whole different story.

So:

• I'd take this patch as-is, as it strictly improves things.
• Open a follow-up trying to find a sane way to avoid these multiple calls to an external tool, be it PYTHON_BIN or cocotb-config. (It's the same overhead.)

[imphil]

Seems like we still (how long @imphil ?) want to be able to use cocotb without cocotb-config?

We can require cocotb-config. We just cannot require it being installed into PATH. Instead, we need to look it up from the old COCOTB environment variable. (I will do that in #990 when I find time for it, hopefully in the next couple of days.)

[themperek]

Alternatively, we go into the direction to generate some makefile (with cocotb-config?) with all this configuration handled in python?

Edit: Can be done once during setup.

[imphil]

Approving this PR now, as it strictly improves things and fixes an actual bug. @themperek please open a new issue/PR to capture the further enhancements we discussed, there were some really nice ideas. However, I'd like to keep this PR focused on the actual fix. Is this PR ready to go in from your side @themperek?

[themperek]

It looks to me it is OK. And seems to work across diffrent build environments.
The objection I would have is that "LDLIBRARY"/"LIBRARY" variable is not really documented although a GitHub search suggests this is widely used.