Fix library_path import

[carlocab]

This is needed (at least) when one customises the Python bindings
installation directory using PYTHON_LIB_INSTALL_DIR.

Since the library_path module will also be installed into the same
directory as stp.py, this change should be safe to make even when one
uses the default PYTHON_LIB_INSTALL_DIR.

I moved this line up with the other imports because my linter was
complaining about its location, but I don't mind moving it back to where
it was originally.

See #353 and #400.

[aytey]

Hmm, I'm not sure that this is immediately mergeable. Basically, this no longer works:

export PYTHONPATH=$(readlink -f bindings/python/stp)
python3 ./bindings/python/stp/stp.py

This is then a problem if we have a user that does this:

from stp import Solver

Traceback (most recent call last):
  File "test.py", line 1, in <module>
    from stp import Solver
  File "/home/avj/clones/stp/master/build/bindings/python/stp/stp.py", line 35, in <module>
    from stp.library_path import PATHS
ModuleNotFoundError: No module named 'stp.library_path'; 'stp' is not a package

I need to have a think about the best way to solve this ...

[aytey]

I'm marking this as "request changes", because it doesn't currently work with how our current users would expect.

[carlocab]

Sure, let me have a closer look at this to see if I can fix that.

[aytey]

Don't get me wrong; people from should point to one level above, but I am not sure they do.

[carlocab]

Does it currently work when pointed to one level from above? Because my impression is that it doesn't (but I'd want to test that with different configurations first).

[aytey]

Yeah, because our __init__.py imports from .stp ... not my code, but code that exists nonetheless!

[carlocab]

Aha, I see. Actually, setting PYTHONPATH should also get things working without the change I suggest here. Except you do need to set it -- if you don't, you get the error I mention in #400.

Maybe the fix here might look something like applying the change if you install stp, but not if you just want to run it in-tree...

[aytey]

Personally, I'd remove what's in __init__.py and let the user get those when they do something like from stp.stp import Solver.

Interestingly, there does appear to be a library_path.py.in_install already.

Is Homebrew "blocked" on this, or can we take a bit of time to work out a longer-term solution?

[carlocab]

We've got the time to work out a proper solution.

[aytey]

Okay, let me have a think.

I might opt for something even more disruptive (== remove the content of __init__.py and your change) and (finally) release a new version of STP.

At least then it is really clear about how we expect the package to be imported.

[carlocab]

Sounds good! Though, I should probably mention that I'm currently unable to build off master, with the following error:

/Users/carlocab/github/stp/tools/stp/main_common.cpp:342:10: error: no member named 'quick_exit' in namespace 'std'
    std::quick_exit(0);
    ~~~~~^

CMake was invoked as

cmake -S. -Bbuild -DCMAKE_INSTALL_PREFIX=/tmp/stp -DCMAKE_BUILD_TYPE=Release
cmake --build build

Adding the flags

-DCMAKE_CXX_STANDARD=11 -DCMAKE_CXX_FLAGS='-stdlib=libc++'

doesn't help. (Though IIRC you already pass -stdlib=libc++ in your CMakeLists.txt.)

I can open a separate issue to track this if you like unless it's a matter of me missing some flags I should be passing to CMake here, or if this is something you're otherwise already aware of.

[aytey]

Yeah, release seems broken on macOS:

avj@armadillo /tmp/stp$ clang --version
Apple clang version 12.0.0 (clang-1200.0.32.29)
Target: arm64-apple-darwin20.3.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
avj@armadillo /tmp/stp$ uname -a
Darwin armadillo 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:06:51 PST 2021; root:xnu-7195.81.3~1/RELEASE_ARM64_T8101 arm64 arm64 MacBookPro17,1 Darwin

/tmp/stp/tools/stp/main_common.cpp:342:10: error: no member named 'quick_exit' in namespace 'std'
    std::quick_exit(0);
    ~~~~~^