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

DRILL-6225: Add support for boost 1.68 with openSSL 1.1.0/1.1.1 support #1817

Closed
wants to merge 2 commits into from

Conversation

debraj92
Copy link

The change is to upgrade boost to the version 1.68. This ensures support for openssl 1.1.1 and 1.1.0.

Updating my drill branch
@arina-ielchiieva arina-ielchiieva changed the title Drill 6225 DRILL-6225: Add support for boost 1.68 with openSSL 1.1.0/1.1.1 support Jun 29, 2019
@arina-ielchiieva
Copy link
Member

@debraj92 please squash the commits and address protobuf job failures - https://travis-ci.org/apache/drill/jobs/552040788.

@debraj92 debraj92 force-pushed the DRILL-6225 branch 6 times, most recently from 3488a51 to 4814a98 Compare July 1, 2019 08:38
@arina-ielchiieva
Copy link
Member

@debraj92 there are some failures on protobuf job. Could you please check?
https://travis-ci.org/apache/drill/jobs/552669860

@dgrinchenko
Copy link
Contributor

@debraj92 for me the change is unclear:

  • the summary states about update to the libboost version 1.68, while PR updating it to 1.65
  • travis-ci currently using Ubuntu Xenial as an LTS distribution, and libboost 1.65 strictly require Ubuntu Bionic (https://packages.ubuntu.com/search?keywords=libboost1.65-all-dev). Also, Ubuntu Xenial EOL is roughly in 2022 and i don't see, why we should remove support of this version of the Linux. Requirements for CentOS version and RedHat linux need to be verified too.
  • why we changed expected function argument to be passed by reference?

@debraj92
Copy link
Author

debraj92 commented Jul 9, 2019

Hi @dgrinchenko ,

Actually the change is incomplete because I ran into a problem with the travis building. Before explaining the problem, let me answer your questions and make the idea clear:

  1. The purpose of this change is to enable support for openssl 1.1.0/1.1.1. This requires updating boost (to at least 1.64). So, I updated boost to the latest - 1.68. But, then I ran into problems with building using travis (as explained later) and I started experimenting with other versions of boost to see if the issue can be resolved. Thats why you see 1.65 there in the change. Ideally it should be 1.68.

  2. Agreed, the changes in travis.yml are only experiments to get the build pass. Will explain the problem shortly.

  3. Boost API has changed. The argument now needs to be passed by reference. This is the main purpose of the change. Without the change drill client code would not compile with boost 1.68.

Now, the problem. To get travis to build drill client code I would have to specify the location for downloading the boost 1.68 static libs. I could not find a repo from where I can do this and hence the change in the travis.yml file is not accurate. One option is to build boost from source but that would unnecessarily slow down the build. I also tried other boost versions (1.65, 1.67) but travis build did not succeed. Not sure how to proceed from here now.

@dgrinchenko
Copy link
Contributor

@debraj92 please read carefully point 2 of my comment, it describes why travis failing for you. Here is the max version of libboost available for the travis.

The another problem - if you decide to build own version of libboost, then it would need to be linked statically to the client, to not interference with already installed system version of libboost, otherwise your patch would change the minimal system version requirements for building and using drill client

@debraj92
Copy link
Author

Hi ,
I am closing this PR now because it looks like upgrading boost at this point is not feasible. I will leave the JIRA open and keep the patch locally so that we can apply it in the future.

@debraj92 debraj92 closed this Jul 16, 2019
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.

None yet

3 participants