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

Update pom files for preparing Apache release #3772

Merged
merged 1 commit into from
Feb 1, 2019
Merged

Conversation

snleee
Copy link
Contributor

@snleee snleee commented Jan 31, 2019

  1. Changed the version form "0.016" to "0.1.0-SNAPSHOT"
  2. Added two profiles (-Psrc-dist, -Pbin-dist) for creating
    binary, source distribution tarbell. For bin-dist profile,
    LICENSE-binary, NOTICE-binary, and licenses-binary are
    included to a distribution tarbell after removing "-binary"
    from their names.
  3. Added "maven-remote-resources-plugin" to include NOTICE,
    LICENSE, DISCLAIMER files in META-INF/ for jar bundle.

From now on, "mvn install -DskipTest" will only package the
projects into jars. In order to create a binary distribution
or source distribution, we will need to run the following
commands:

mvn install -Psrc-dist -DskipTests
mvn install -Pbin-dist -DskipTests
mvn install -Psrc-dist,bin-dist -DskipTests

@snleee
Copy link
Contributor Author

snleee commented Jan 31, 2019

@felixcheung This addresses the issue of including LICENSE-binary, NOTICE-binary, /licenses-binary for binary distribution tarbell. #3722

@codecov-io
Copy link

codecov-io commented Jan 31, 2019

Codecov Report

Merging #3772 into master will increase coverage by 0.24%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3772      +/-   ##
============================================
+ Coverage     67.02%   67.26%   +0.24%     
  Complexity        4        4              
============================================
  Files          1027     1027              
  Lines         50798    50798              
  Branches       7091     7091              
============================================
+ Hits          34045    34169     +124     
+ Misses        14424    14295     -129     
- Partials       2329     2334       +5
Impacted Files Coverage Δ Complexity Δ
...he/pinot/core/query/pruner/ValidSegmentPruner.java 57.14% <0%> (-28.58%) 0% <0%> (ø)
...impl/dictionary/DoubleOnHeapMutableDictionary.java 68.88% <0%> (-6.67%) 0% <0%> (ø)
.../impl/dictionary/FloatOnHeapMutableDictionary.java 86.66% <0%> (-6.67%) 0% <0%> (ø)
...e/operator/dociditerators/BitmapDocIdIterator.java 60.71% <0%> (-3.58%) 0% <0%> (ø)
...ot/core/query/pruner/ColumnValueSegmentPruner.java 85.91% <0%> (-1.41%) 0% <0%> (ø)
.../pinot/core/data/manager/BaseTableDataManager.java 92.59% <0%> (-1.24%) 0% <0%> (ø)
...a/org/apache/pinot/core/common/DataBlockCache.java 79.38% <0%> (-0.77%) 0% <0%> (ø)
...ot/common/protocols/SegmentCompletionProtocol.java 93.56% <0%> (-0.59%) 0% <0%> (ø)
...impl/dictionary/FloatOffHeapMutableDictionary.java 87.27% <0%> (ø) 0% <0%> (ø) ⬇️
...ntroller/helix/core/PinotHelixResourceManager.java 60.16% <0%> (+0.3%) 0% <0%> (ø) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67c6507...2f6b1f1. Read the comment docs.

1. Changed the version from "0.016" to "0.1.0-SNAPSHOT"
2. Added two profiles (-Psrc-dist, -Pbin-dist) for creating
   binary, source distribution tarbell. For bin-dist profile,
   LICENSE-binary, NOTICE-binary, and licenses-binary are
   included to a distribution tarbell after removing "-binary"
   from their names.
3. Added "maven-remote-resources-plugin" to include NOTICE,
   LICENSE, DISCLAIMER files in META-INF/ for jar bundle.

From now on, "mvn install -DskipTest" will only package the
projects into jars. In order to create a binary distribution
or source distribution, we will need to run the following
commands:

mvn install -Psrc-dist -DskipTests
mvn install -Pbin-dist -DskipTests
mvn install -Psrc-dist,bin-dist -DskipTests
Copy link
Contributor

@mcvsubbu mcvsubbu left a comment

Choose a reason for hiding this comment

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

Is there a way to specify the version in the top leve pom and use ${pinot-version} in others? Making sure that we change all versions correctly all the time seems to be painful

@snleee
Copy link
Contributor Author

snleee commented Jan 31, 2019

@mcvsubbu I don't think there's a way to do that. I have checked at least 5 apache projects and they all have version for each sub module. However, maven-release-plugin will handle updating versions in the pom files. I added the plugin along with this commit (I still need to do some study on how to use it correctly). Once I have a good understanding on end-to-end release process. I will write up the doc.

@snleee snleee merged commit f48fdd3 into master Feb 1, 2019
@snleee snleee deleted the apache-build branch February 1, 2019 02:39
<fileSets>
<!-- Rename liscenses-binary directory to licenses and include it to a distribution tarbell -->
Copy link
Member

Choose a reason for hiding this comment

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

liscenses-binary -> licenses-binary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch. I will resolve it with a follow-up pr.

<id>source-release</id>
<formats>
<format>tar.gz</format>
</formats>
Copy link
Member

Choose a reason for hiding this comment

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

you might want to remove -binary from the source release: apache/spark@30aa37f#diff-01ca42240614718522afde4d4885b40d

Copy link
Contributor Author

@snleee snleee Feb 2, 2019

Choose a reason for hiding this comment

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

I actually considered that. However, that will cause the issue when people try to build the binary tarbell from the released source code since the source release won't contain -binary files. Spark seemed to have the same issue and their decision is to remove -binary files from the source release. Side effect is that people cannot build a completely valid binary release from the released source code. apache/spark#22840

On the other hand, I checked that Apache Flink is using similar approach as Spark (dividing LICENSE vs LICENSE-binary) and they include -binary files to their source code. You can check it from https://www.apache.org/dyn/closer.lua/flink/flink-1.7.1/flink-1.7.1-src.tgz

I would prefer the latter since it allows our users to build a valid binary tarbell (with correct LICENSE, NOTICE).

@felixcheung How do you think?

@@ -1058,8 +1067,9 @@
<!-- Top level files -->
<exclude>.codecov*</exclude>
<exclude>HEADER</exclude>
<exclude>LICENSE</exclude>
Copy link
Member

Choose a reason for hiding this comment

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

why is <exclude>thirdeye/**</exclude>?

Copy link
Contributor Author

@snleee snleee Feb 2, 2019

Choose a reason for hiding this comment

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

Thirdeye (Anomaly Detection Platform) is a separate project and this won't be a part of Apache Pinot distribution. We are sharing the same repository because Thirdeye is interacting closely with Pinot as its primary backend storage layer and it's using many parts of Pinot code. In my understanding, Thirdeye will move out of this repository in near future.

@kishoreg @apucher @xiaohui-sun @raviarin Can you guys also comment on the plan for Thirdeye?

@felixcheung
Copy link
Member

sorry, late comment.

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.

4 participants