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

Add a new setup script for Fedora Linux (40) #11352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ghelmling
Copy link

This adds a new setup script for Fedora Linux. It is based on a combination of the setup-centos9.sh and setup-ubuntu.sh scripts.

I haven't tested all permutations, but it works for my Fedora 40 setup with:
USE_CLANG=true

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 25, 2024
Copy link

netlify bot commented Oct 25, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit b1d00d0
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67210ac35eb79000083b6171

@majetideepak
Copy link
Collaborator

CC: @czentgr

Copy link
Collaborator

@czentgr czentgr left a comment

Choose a reason for hiding this comment

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

I had the same change at some point but closed it. See #8374

One of the issues was that it wasn't the dependencies but Velox that would not compile with the system GCC version (it being pretty new).
Did you get Velox to build with Clang 15 (which we hadn't as an option) or did we fix all the gcc issues that it now builds on the much newer one?

BUILD_DUCKDB="${BUILD_DUCKDB:-true}"
USE_CLANG="${USE_CLANG:-false}"

FB_OS_VERSION="v2024.05.20.00"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was updated not that long ago. The current version is v2024.07.01.00. Please see the other setup files.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it looks like there were a number of changes to the setup scripts that I missed in my rebase. I just updated to account for the changes.

I've only tested with Clang 15, ie.
USE_CLANG=true ./setup-fedora.sh

After running the setup script, building with clang required once additional import fix (exec/QueryTraceConfig.h). I'm not sure why that issue isn't showing up elsewhere. But with that in place, I am able to build Velox (make debug && make unittest).

I have not tested with GCC 13 to see if that works with this. If there are known build issue with it, then I would guess not.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One of the issues was that it wasn't the dependencies but Velox that would not compile with the system GCC version (it being pretty new).

That was probably #5898

@facebook-github-bot
Copy link
Contributor

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

Copy link
Collaborator

@assignUser assignUser left a comment

Choose a reason for hiding this comment

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

I haven't run this in docker but as it is mostly the same as our other scripts I assume it's fine. @czentgr as you are afaik also a fedora user and have been elbow deep in the setup scripts recently, I'll defer to you for any required changes.

@czentgr
Copy link
Collaborator

czentgr commented Oct 30, 2024

I haven't run this in docker but as it is mostly the same as our other scripts I assume it's fine. @czentgr as you are afaik also a fedora user and have been elbow deep in the setup scripts recently, I'll defer to you for any required changes.

Yes, I was using Fedora for a while but are now macOS user mainly and don't have a Fedora setup anymore. But others I have talked to use Fedora.
This should be ok, I will have quite the work to do on the refactoring of the scripts given all the recent changes for my refactor PR. I hope we get some calmer time to push this through eventually.

@czentgr
Copy link
Collaborator

czentgr commented Oct 30, 2024

@ghelmling You have a conflict please resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants