velox icon indicating copy to clipboard operation
velox copied to clipboard

Add gcc11 to Ubuntu20.04 setup and add PkgConfig install

Open czentgr opened this issue 1 year ago • 5 comments

Modernize the compiler used on Ubuntu 20.04. Once Velox moves to using the C++20 standard the system gcc9 compiler cannot be used anymore.

In addition, recent upgrades added the usage of PkgConfig which was not installed into the system and is not by default pre-installed into ubuntu 20.04.

Resolves: https://github.com/facebookincubator/velox/issues/10953

czentgr avatar Oct 22 '24 21:10 czentgr

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 9de06d2afc2b11903deacc8f7e32ed243466c0c7
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/672a77365f375400087bffa4

netlify[bot] avatar Oct 22 '24 21:10 netlify[bot]

@PHILO-HE FYI.

czentgr avatar Oct 22 '24 21:10 czentgr

@czentgr, the recently upgraded folly requires GCC >= 10. Otherwise, the below compile error will be reported. So we should land this pr now to avoid this error, right?

/velox/deps-download/folly/folly/Portability.h:38:24: error: static assertion failed: __GNUC__ >= 10
   38 | static_assert(__GNUC__ >= 10, "__GNUC__ >= 10");

PHILO-HE avatar Oct 23 '24 05:10 PHILO-HE

@PHILO-HE Yes, basically, Ubuntu 20.04 is currently broken without these changes - the user would do all of this manually. We already state Velox requires a compiler at the minimum GCC 11.0 or Clang 15.0. in the README.

czentgr avatar Oct 24 '24 01:10 czentgr

@majetideepak @assignUser Please take a look.

czentgr avatar Oct 24 '24 02:10 czentgr

The pr looks good to me. Thanks!

PHILO-HE avatar Oct 28 '24 02:10 PHILO-HE

@xiaoxmeng, if no comment, could you merge this pr? I just found this pr is also required for ubuntu 22.04 to fix missing pkg-config.

PHILO-HE avatar Nov 04 '24 06:11 PHILO-HE

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Nov 04 '24 13:11 facebook-github-bot

@czentgr Would you rebase to allow merging?

mbasmanova avatar Nov 04 '24 14:11 mbasmanova

@mbasmanova I rebased. Please take another look.

czentgr avatar Nov 05 '24 20:11 czentgr

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Nov 05 '24 20:11 facebook-github-bot

@mbasmanova merged this pull request in facebookincubator/velox@bfc199fc65ced0a4bf7644ea5314e6ec5fa69eea.

facebook-github-bot avatar Nov 05 '24 21:11 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit bfc199fc.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Nov 05 '24 22:11 conbench-facebook[bot]