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

[CI][C++] Force CMake to find Python2 #9690

Merged
merged 5 commits into from
Feb 25, 2021

Conversation

BewareMyPower
Copy link
Contributor

@BewareMyPower BewareMyPower commented Feb 24, 2021

Fixes #9682

Motivation

Currently, ci-cpp-tests uses pulsar-build image that is from ubuntu:16.04 to build C++/Python client. The image uses libboost-all-dev for CMake to find boost dependencies. However, the Boost.Python library from Ubuntu 16.04's apt source only supports Python 2.

Modifications

  • Specifying PYTHON_INCLUDE_DIR and PYTHON_LIBRARY could indicate the installation of Python to use. Since the pulsar-build image only contains Python binary but not the Python2 library (libpython2.7so), this PR installs libpython-dev to setup the Python2 library. Otherwise, CMake would still find the Python3 library(libpython3.5.so).
  • Remove redundant C++ client dependencies like libjsoncpp-dev and replace libboost-all-dev with the specific libboost-xxx-dev.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

@BewareMyPower
Copy link
Contributor Author

After discussing with @eolivelli in #9684 , instead of removing Python2 which may need a lot of work, this PR just keeps the original CI and lets CMake to find Python2 only in CI.

Since there's no way to verify pulsar-build image yet, I still use the image that was built by myself first. After the CI passed, I think committers that have permissions to apachepulsar's dockerhub could upload the new image and run CI again.

@BewareMyPower
Copy link
Contributor Author

It looks like pip install setuptools will fail

Collecting setuptools>=34.0.0 (from apache-bookkeeper-client>=4.9.2->pulsar-client==2.8.0)
  Downloading https://files.pythonhosted.org/packages/12/68/95515eaff788370246dac534830ea9ccb0758e921ac9e9041996026ecaf2/setuptools-53.0.0.tar.gz (2.1MB)
    100% |################################| 2.1MB 502kB/s 
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "setuptools/__init__.py", line 16, in <module>
        import setuptools.version
      File "setuptools/version.py", line 1, in <module>
        import pkg_resources
      File "pkg_resources/__init__.py", line 1367
        raise SyntaxError(e) from e
                                ^

It tried to download setuptools 53.0.0 but it doesn't support Python2. I saw the successfully CI log is

Requirement already satisfied: setuptools>=34.0.0 in /usr/local/lib/python2.7/dist-packages (from apache-bookkeeper-client>=4.9.2->pulsar-client==2.8.0) (44.1.1)

I found the cause is the pip version of my image is 8.1.1 but the pip version of apachepulsar/pulsar-build is 20.3.4. I'll solve this problem.

@eolivelli
Copy link
Contributor

eolivelli commented Feb 24, 2021

what about installing pip 2.7 with this command ?

curl https://bootstrap.pypa.io/2.7/get-pip.py | python

@BewareMyPower
Copy link
Contributor Author

@eolivelli Yeah, I've already tested successfully in my local environment, just waiting for docker image uploaded.

@BewareMyPower
Copy link
Contributor Author

@zymap The CPP, Python Tests have passed. I think you can upload a new image now.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM
thank you!

build/docker/Dockerfile Outdated Show resolved Hide resolved
@merlimat
Copy link
Contributor

@BewareMyPower This should also work with the current image on DockerHub, right?

Just saying that because we could merge this PR, wait few days and then push the image, so that we minimize the chances of having PRs blocked on CI because they were created on top of a version of master that didn't include this PR yet.

@merlimat
Copy link
Contributor

Uhm, actually, we should just call this image with a different tag... I'll push the image

pulsar-client-cpp/docker-build.sh Outdated Show resolved Hide resolved
pulsar-client-cpp/docker-tests.sh Outdated Show resolved Hide resolved
@BewareMyPower
Copy link
Contributor Author

Thanks @merlimat

@BewareMyPower
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@eolivelli
Copy link
Contributor

/pulsarbot run-failure-checks

@eolivelli
Copy link
Contributor

@aahmed-se @BewareMyPower @merlimat
QA tests passed.
Can we merge this patch and push the new image ?
Thank you very much !

@BewareMyPower
Copy link
Contributor Author

@eolivelli It looks like @merlimat has already pushed a new image apachepulsar/pulsar-build:ubuntu-16.04-py2 which has a new tag ubuntu-16.04-py2. See commit 0726016. The current CI is based on the new image.

@eolivelli
Copy link
Contributor

@BewareMyPower thanks for your information.

and thanks to @merlimat for pushing the new image.

I will start to run CI jobs in JDK11 on my private fork and see what happens.

I hope we can merge this patch soon and keep the master branch up to date

@zymap zymap merged commit 1419d28 into apache:master Feb 25, 2021
@BewareMyPower BewareMyPower deleted the bewaremypower/fix-issue-9682 branch February 25, 2021 09:08
@merlimat
Copy link
Contributor

I just did the basic versioning with -py2 here, we can do -py3 for the other changes. In future we should prob have a better convention to tag these images.

codelipenghui pushed a commit that referenced this pull request May 21, 2021
### Motivation

The CI - Pulsar Website Build has been broken for a long time. Here's an example run: https://github.com/apache/pulsar/runs/2635024657?check_suite_focus=true

```
CMake Error at python/CMakeLists.txt:85 (MESSAGE):
-- Using Boost Python libs: 
  Could not find Boost Python library
```

It looks like to be the same issue with #9682. #9690 fixed the broken CI for cpp client but the `ci-pulsar-website-build.yaml` wasn't modified.

Another issue is after I fixed the cpp build for website build, the `python-doc-gen.sh` still failed because the default `pdoc` is a Python3 tool

```
+ pdoc pulsar --html --html-dir /pulsar/generated-site/api/python/2.8.0-SNAPSHOT
Traceback (most recent call last):
  File "/usr/local/bin/pdoc", line 7, in <module>
    from pdoc.__main__ import cli
  File "/usr/local/lib/python3.5/dist-packages/pdoc/__init__.py", line 328
    ) -> str:
    ^
```

So we need to install a Python2 version `pdoc` as well.

### Modifications

- Use `ubuntu-16.04-py2` tag instead of old `ubuntu-16.04` tag in `docker-build-site.sh` and force CMake to find Python2 in `python-doc-gen.sh`. The the Pulsar Python client library could be built successfully.
- Install a Python2 version `pdoc` in `python-doc-gen.sh`.
yangl pushed a commit to yangl/pulsar that referenced this pull request Jun 23, 2021
### Motivation

The CI - Pulsar Website Build has been broken for a long time. Here's an example run: https://github.com/apache/pulsar/runs/2635024657?check_suite_focus=true

```
CMake Error at python/CMakeLists.txt:85 (MESSAGE):
-- Using Boost Python libs: 
  Could not find Boost Python library
```

It looks like to be the same issue with apache#9682. apache#9690 fixed the broken CI for cpp client but the `ci-pulsar-website-build.yaml` wasn't modified.

Another issue is after I fixed the cpp build for website build, the `python-doc-gen.sh` still failed because the default `pdoc` is a Python3 tool

```
+ pdoc pulsar --html --html-dir /pulsar/generated-site/api/python/2.8.0-SNAPSHOT
Traceback (most recent call last):
  File "/usr/local/bin/pdoc", line 7, in <module>
    from pdoc.__main__ import cli
  File "/usr/local/lib/python3.5/dist-packages/pdoc/__init__.py", line 328
    ) -> str:
    ^
```

So we need to install a Python2 version `pdoc` as well.

### Modifications

- Use `ubuntu-16.04-py2` tag instead of old `ubuntu-16.04` tag in `docker-build-site.sh` and force CMake to find Python2 in `python-doc-gen.sh`. The the Pulsar Python client library could be built successfully.
- Install a Python2 version `pdoc` in `python-doc-gen.sh`.
codelipenghui pushed a commit that referenced this pull request Dec 11, 2021
Fixes #9682

Currently, ci-cpp-tests uses `pulsar-build` image that is from `ubuntu:16.04` to build C++/Python client. The image uses `libboost-all-dev` for CMake to find boost dependencies. However, the Boost.Python library from Ubuntu 16.04's apt source only supports Python 2.

- Specifying `PYTHON_INCLUDE_DIR` and `PYTHON_LIBRARY` could indicate the installation of Python to use. Since the `pulsar-build` image only contains Python binary but not the Python2 library (`libpython2.7so`), this PR installs `libpython-dev` to setup the Python2 library. Otherwise, CMake would still find the Python3 library(`libpython3.5.so`).
- Remove redundant C++ client dependencies like `libjsoncpp-dev` and replace `libboost-all-dev` with the specific `libboost-xxx-dev`.

- [ ] Make sure that the change passes the CI checks.

*(Please pick either of the following options)*

This change is a trivial rework / code cleanup without any test coverage.

(cherry picked from commit 1419d28)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Dec 11, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation

The CI - Pulsar Website Build has been broken for a long time. Here's an example run: https://github.com/apache/pulsar/runs/2635024657?check_suite_focus=true

```
CMake Error at python/CMakeLists.txt:85 (MESSAGE):
-- Using Boost Python libs: 
  Could not find Boost Python library
```

It looks like to be the same issue with apache#9682. apache#9690 fixed the broken CI for cpp client but the `ci-pulsar-website-build.yaml` wasn't modified.

Another issue is after I fixed the cpp build for website build, the `python-doc-gen.sh` still failed because the default `pdoc` is a Python3 tool

```
+ pdoc pulsar --html --html-dir /pulsar/generated-site/api/python/2.8.0-SNAPSHOT
Traceback (most recent call last):
  File "/usr/local/bin/pdoc", line 7, in <module>
    from pdoc.__main__ import cli
  File "/usr/local/lib/python3.5/dist-packages/pdoc/__init__.py", line 328
    ) -> str:
    ^
```

So we need to install a Python2 version `pdoc` as well.

### Modifications

- Use `ubuntu-16.04-py2` tag instead of old `ubuntu-16.04` tag in `docker-build-site.sh` and force CMake to find Python2 in `python-doc-gen.sh`. The the Pulsar Python client library could be built successfully.
- Install a Python2 version `pdoc` in `python-doc-gen.sh`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI cpp-tests is broken now for the latest pulsar-build image
6 participants