-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Support out of tree builds #23987
Support out of tree builds #23987
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! (docs)
git checkout master and a local doc build returns "build succeeded, 29 warnings.", which are all "Duplicate label" or "Duplicate function description" warnings.
Pull this PR's branch and run a new local doc build results in the same set of warnings.
@@ -40,12 +40,12 @@ | |||
<artifactId>drift-maven-plugin</artifactId> | |||
<executions> | |||
<execution> | |||
<phase>validate</phase> | |||
<phase>generate-sources</phase> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain the changes in this file (why did the phases change, and why did "thrift" get added to the directory path?) also, is there any noticeable difference after this change for a regular build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support out of tree build the <module>/target
directory has to be linked at the very beginning. The drift-maven-plugin
is generating .thrift
file definition into /target
directory. To make sure the target
directory is linked before the drift
plugin invocation It has to be moved to a later build phase. Also generate-sources
seems to be a more appropriate phase to run a plugin generating code.
Since PrestoThriftService.thrift
been moved to a later phase I had to move the processing as well (to process-sources
phase).
I had to move run-sphinx
plugging to compile phase as keeping it in package
phase introduces non determinism. Plugins in the same phase are invoked in order. But order of plugins in Maven is somehow obscure (for example what if a child module has plugins in different order than the parent module).
The reason why /thrift
is needed is funny. Basically the thrift
generator plugin tries to create parent directories automatically. And if a parent directory is a symbolic link - it fails.
Description
Allow building Presto out of tree by automatically linking target directories.
Motivation and Context
Out of tree builds are useful when the source tree is immutable or is available via an NFS.
Impact
N/A
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.