Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Boost.python requires numpy. Fix function _run_python_script(). #5643

Merged
merged 2 commits into from
Jun 7, 2021

Conversation

wdobbe
Copy link
Contributor

@wdobbe wdobbe commented May 26, 2021

Specify library name and version: boost/all_versions

First fix:
Function _run_python_script was broken when provided script returns empty output.
And on Windows the script that _python_abiflags(self) sends to _run_python_script does return no output. As a result the _run_python_script returned something like

 ----Running------
> "python" -c "from __future__ import print_function; import sys; print(getattr(sys, 'abiflags', ''))"
-----------------

instead of an empty string. Result was the following error message when building the boost recipe with option without_python=False even though python libraries were present:

ERROR: boost/1.76.0: Invalid configuration: couldn't locate python libraries - make sure you have installed python development files

Second fix:
Build of Boost.python library will fail if python package numpy is not installed. Check for presence of numpy in validate().


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@CLAassistant
Copy link

CLAassistant commented May 26, 2021

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@Nekto89
Copy link
Contributor

Nekto89 commented May 27, 2021

Is numpy dependency new for Boost.Python? I'm currently using 1.74.0 that was built without conan and without numpy.

@wdobbe
Copy link
Contributor Author

wdobbe commented May 27, 2021

Is numpy dependency new for Boost.Python? I'm currently using 1.74.0 that was built without conan and without numpy.

From this vcpkg issue it seems that Boost should check during build if numpy is installed or not and decides whether to build boost numpy library based on that result. If it builds boost numpy this will result in an extra library (libboost_numpy39.lib on Windows).
So the question is why Boost starts to build the numpy library on my systems even when numpy package is not installed.

In general: if the check for python package numpy in this PR is a problem, then better to remove that part and think about how we can fix it properly later.

The other patch is more important, because it completely messes up the boost python build on Windows and it takes quite a bit of time and debugging to find out what is going wrong.

@wdobbe
Copy link
Contributor Author

wdobbe commented May 27, 2021

ps: I don't understand why the license/cla is shown as 'not signed yet'. I have already signed it multiple times and have submitted PRs before. I signed the CLA again yesterday, but no result yet.

@prince-chrismc
Copy link
Contributor

The commits is @winfriedd ... you need to sign the CLA with that account

@wdobbe
Copy link
Contributor Author

wdobbe commented May 28, 2021

Thanks, I didn't even know I had that other Github account. I will close that asap.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SSE4 SSE4 requested a review from uilianries May 29, 2021 12:02
@SSE4
Copy link
Contributor

SSE4 commented May 29, 2021

the following code could be used to detect numpy (better in another PR):

    @property
    def _has_numpy(self):
        return self._run_python_script("from __future__ import print_function; "
                                       "import numpy; "
                                       "print(numpy.__version__)")

@wdobbe
Copy link
Contributor Author

wdobbe commented May 29, 2021

Thanks. I noticed that when building the test_package with VS2019 linking the numpy.cpp executable fails with unresolved synbol errors to boost numpy. So I removed numpy stuff from this PR, needs more investigation.

@conan-center-bot
Copy link
Collaborator

All green in build 6 (4401448c02269adb572f88a52da804d707e1c1e2):

  • boost/1.69.0@:
    All packages built successfully! (All logs)

  • boost/1.70.0@:
    All packages built successfully! (All logs)

  • boost/1.71.0@:
    All packages built successfully! (All logs)

  • boost/1.72.0@:
    All packages built successfully! (All logs)

  • boost/1.74.0@:
    All packages built successfully! (All logs)

  • boost/1.73.0@:
    All packages built successfully! (All logs)

  • boost/1.76.0@:
    All packages built successfully! (All logs)

  • boost/1.75.0@:
    All packages built successfully! (All logs)

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@conan-center-bot conan-center-bot merged commit 63a4784 into conan-io:master Jun 7, 2021
madebr pushed a commit to madebr/conan-center-index that referenced this pull request Jun 21, 2021
…_script().

* Boost.python requires numpy. Fix function _run_python_script(), was broken when script returns empty output.

* Boost: implement PR conan-io#5643 feedback from @SSE4. Remove numpy check, needs more investigation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants