Skip to content

SPARK-1650: Correctly identify maven project version#572

Closed
rahulsinghaliitd wants to merge 1 commit intoapache:masterfrom
ThalesGroup:SPARK-1650
Closed

SPARK-1650: Correctly identify maven project version#572
rahulsinghaliitd wants to merge 1 commit intoapache:masterfrom
ThalesGroup:SPARK-1650

Conversation

@rahulsinghaliitd
Copy link

Better account for various side-effect outputs while executing
"mvn help:evaluate -Dexpression=project.version"

Better account for various side-effect outputs while executing
"mvn help:evaluate -Dexpression=project.version"
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm - would be it be less brittle to do this?

VERSION=$(mvn help:evaluate -Dexpression=project.version | grep -v "INFO" | tail -n 1)

That way we don't make any assumptions about the messages that can occur before the version is printed. Otherwise over time we'll have to keep writing rules to match various types of input.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also tighten this up a bit by only asking the version for core:

VERSION=$(mvn help:evaluate -Dexpression=project.version -pl core | grep -v "INFO" | tail -n 1)

Copy link
Author

Choose a reason for hiding this comment

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

  1. Using "tail -n 1" works for all my error cases, will make the change.
  2. Wouldn't using the umbrella project be more accurate (considering the fact that we use a fat jar)?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure, if you want it's okay to just do the parent. They all have the same version, so it's fine either way.

@pwendell
Copy link
Contributor

Good catch on this - made a small suggestion.

@rahulsinghaliitd
Copy link
Author

@pwendell Thanks, I had recently used a maven build and encountered this problem. I even had a local commit to add the maven build to make-distribution.sh but you beat me to it. :)

@rahulsinghaliitd
Copy link
Author

I am noob to GitHub and Spark upstream procedures, so confused about what happens when a pull request is accepted.

  1. Is the PR merged or rebased on the destination branch?
  2. And as in this cases, a second commit is required. Now should I just push a second commit and the two commits will be squashed when the pull request is accepted, or do I need to squash them? If yes how (create a new PR or force modify this one)
  3. Lastly, will I need to cherry-pick the commit to 1.0.0. branch or will the core developer who accepts this PR will do it?

@pwendell
Copy link
Contributor

You can just make another commit and then push it to your branch. When we merge the pull request we actually bundle up all of your commits and squash them into a single commit.

We will deal with cherry-picking at the time of commit, you don't need to do that yourself.

@pwendell
Copy link
Contributor

@rahulsinghaliitd I actually just merged this and made the suggested change when merging it. Thanks!

asfgit pushed a commit that referenced this pull request Apr 27, 2014
Better account for various side-effect outputs while executing
"mvn help:evaluate -Dexpression=project.version"

Author: Rahul Singhal <rahul.singhal@guavus.com>

Closes #572 from rahulsinghaliitd/SPARK-1650 and squashes the following commits:

fd6a611 [Rahul Singhal] SPARK-1650: Correctly identify maven project version
(cherry picked from commit 7b2527d)

Signed-off-by: Patrick Wendell <pwendell@gmail.com>
@asfgit asfgit closed this in 7b2527d Apr 27, 2014
@rahulsinghaliitd
Copy link
Author

Thanks @pwendell , I was waiting because I think there maybe more minor problems

  1. The current test to detect if "mvn" is installed or not, is not correct.
  2. If maven command fails (due to whatever reason), we would end up picking some random error string as the version.

@rahulsinghaliitd rahulsinghaliitd deleted the SPARK-1650 branch April 28, 2014 04:55
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
Better account for various side-effect outputs while executing
"mvn help:evaluate -Dexpression=project.version"

Author: Rahul Singhal <rahul.singhal@guavus.com>

Closes apache#572 from rahulsinghaliitd/SPARK-1650 and squashes the following commits:

fd6a611 [Rahul Singhal] SPARK-1650: Correctly identify maven project version
meisam pushed a commit to meisam/spark that referenced this pull request Sep 24, 2014
Adding spark.streaming.blockInterval property
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
arjunshroff pushed a commit to arjunshroff/spark that referenced this pull request Nov 24, 2020
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