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 build param to skip creating hardlinks #23782

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

Conversation

singcha
Copy link
Contributor

@singcha singcha commented Oct 7, 2024

Description

Presto build fails when a filesystem with no support for hardlinks is used. Add new param to specify hardlink include jars.

Motivation and Context

Fixes build issue when underlying filesystem doesn't support hardlinks

Test Plan

Built on devserver

== NO RELEASE NOTE ==

Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thanks for the doc!

README.md Outdated Show resolved Hide resolved
steveburnett
steveburnett previously approved these changes Oct 8, 2024
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

README.md Outdated
Comment on lines 36 to 39
Presto uses the [Provisio](https://github.com/jvanzyl/provisio) plugin for packaging using hard links. If your filesystem doesn't support hard links, you can bypass hard links using:

./mvnw clean install -presto.hard-link-includes=''

Copy link
Contributor

Choose a reason for hiding this comment

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

My sense is that the vast majority of filesystems folks use do support hardlinks. If that's the case, this is a very prominent callout in the README for relatively esoteric information. I would remove it, and perhaps add a comment in the POM instead.

Copy link
Contributor Author

@singcha singcha Oct 8, 2024

Choose a reason for hiding this comment

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

IMO - Anyone setting up project & building it, is going to look in README file for build instructions. Please let me know if you still think we should remove it and I can update the PR

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's create a section toward the bottom for common issues with the build. For now, we can just put this there, but add other stuff as needed (incorrect JDK, Maven timeouts, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Presto build fails when a filesystem with no support for hardlinks is used. Add new param to specify hardlink include jars.
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.

3 participants