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

[SPARK-8781] Fix variables in published pom.xml are not resolved #7193

Closed
wants to merge 2 commits into from

Conversation

andrewor14
Copy link
Contributor

The issue is summarized in the JIRA and is caused by this commit: 984ad60.

This patch reverts that commit and fixes the maven build in a different way. We limit the dependencies of KinesisReceiverSuite to avoid having to deal with the complexities in how maven deals with transitive test dependencies.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36417 has started for PR 7193 at commit 5cb5f45.

// To avoid introducing a dependency on Spark core tests, simply use scalatest's FunSuite
// here instead of our own SparkFunSuite. Introducing the dependency has caused problems
// in the past (SPARK-8781) that are complicated by bugs in the maven shade plugin (MSHADE-148).
import org.scalatest.{BeforeAndAfter, Matchers, FunSuite}
Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: ordering

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I fixed

@vanzin
Copy link
Contributor

vanzin commented Jul 2, 2015

LGTM.

KinesisReceiverSuite really doesn't need to depend on
TestSuiteBase. The issue is that TestSuiteBase transitively
depends on Spark core tests (SparkFunSuite), but there is no simple
way in maven to express transitive test dependencies. To simplify
the build structure of this module, we should just have
KinesisReceiverSuite extend directly from FunSuite.
@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@andrewor14
Copy link
Contributor Author

retest this please

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36425 has started for PR 7193 at commit ca3d5d4.

@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36417 has finished for PR 7193 at commit 5cb5f45.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class ShiftLeft(left: Expression, right: Expression) extends BinaryExpression
    • case class ShiftRight(left: Expression, right: Expression) extends BinaryExpression

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@tdas
Copy link
Contributor

tdas commented Jul 2, 2015

LGTM.

@andrewor14
Copy link
Contributor Author

Merging into master, 1.4, and 1.3. Thanks for the reviews.

asfgit pushed a commit that referenced this pull request Jul 2, 2015
The issue is summarized in the JIRA and is caused by this commit: 984ad60.

This patch reverts that commit and fixes the maven build in a different way. We limit the dependencies of `KinesisReceiverSuite` to avoid having to deal with the complexities in how maven deals with transitive test dependencies.

Author: Andrew Or <[email protected]>

Closes #7193 from andrewor14/fix-kinesis-pom and squashes the following commits:

ca3d5d4 [Andrew Or] Limit kinesis test dependencies
f24e09c [Andrew Or] Revert "[BUILD] Fix Maven build for Kinesis"

(cherry picked from commit 82cf331)
Signed-off-by: Andrew Or <[email protected]>
@asfgit asfgit closed this in 82cf331 Jul 2, 2015
asfgit pushed a commit that referenced this pull request Jul 2, 2015
The issue is summarized in the JIRA and is caused by this commit: 984ad60.

This patch reverts that commit and fixes the maven build in a different way. We limit the dependencies of `KinesisReceiverSuite` to avoid having to deal with the complexities in how maven deals with transitive test dependencies.

Author: Andrew Or <[email protected]>

Closes #7193 from andrewor14/fix-kinesis-pom and squashes the following commits:

ca3d5d4 [Andrew Or] Limit kinesis test dependencies
f24e09c [Andrew Or] Revert "[BUILD] Fix Maven build for Kinesis"

(cherry picked from commit 82cf331)
Signed-off-by: Andrew Or <[email protected]>

Conflicts:
	extras/kinesis-asl/src/test/scala/org/apache/spark/streaming/kinesis/KinesisReceiverSuite.scala
@andrewor14 andrewor14 deleted the fix-kinesis-pom branch July 2, 2015 20:53
@SparkQA
Copy link

SparkQA commented Jul 2, 2015

Test build #36425 has finished for PR 7193 at commit ca3d5d4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

asfgit pushed a commit that referenced this pull request Jul 7, 2015
This is a workaround for MSHADE-148, which leads to an infinite loop when building Spark with maven 3.3.x. This was originally caused by #6441, which added a bunch of test dependencies on the spark-core test module. Recently, it was revealed by #7193.

This patch adds a `-Prelease` profile. If present, it will set `createDependencyReducedPom` to true. The consequences are:
- If you are releasing Spark with this profile, you are fine as long as you use maven 3.2.x or before.
- If you are releasing Spark without this profile, you will run into SPARK-8781.
- If you are not releasing Spark but you are using this profile, you may run into SPARK-8819.
- If you are not releasing Spark and you did not include this profile, you are fine.

This is all documented in `pom.xml` and tested locally with both versions of maven.

Author: Andrew Or <[email protected]>

Closes #7219 from andrewor14/fix-maven-build and squashes the following commits:

1d37e87 [Andrew Or] Merge branch 'master' of github.com:apache/spark into fix-maven-build
3574ae4 [Andrew Or] Review comments
f39199c [Andrew Or] Create a -Prelease profile that flags `createDependencyReducedPom`

(cherry picked from commit 9eae5fa)
Signed-off-by: Andrew Or <[email protected]>
asfgit pushed a commit that referenced this pull request Jul 7, 2015
This is a workaround for MSHADE-148, which leads to an infinite loop when building Spark with maven 3.3.x. This was originally caused by #6441, which added a bunch of test dependencies on the spark-core test module. Recently, it was revealed by #7193.

This patch adds a `-Prelease` profile. If present, it will set `createDependencyReducedPom` to true. The consequences are:
- If you are releasing Spark with this profile, you are fine as long as you use maven 3.2.x or before.
- If you are releasing Spark without this profile, you will run into SPARK-8781.
- If you are not releasing Spark but you are using this profile, you may run into SPARK-8819.
- If you are not releasing Spark and you did not include this profile, you are fine.

This is all documented in `pom.xml` and tested locally with both versions of maven.

Author: Andrew Or <[email protected]>

Closes #7219 from andrewor14/fix-maven-build and squashes the following commits:

1d37e87 [Andrew Or] Merge branch 'master' of github.com:apache/spark into fix-maven-build
3574ae4 [Andrew Or] Review comments
f39199c [Andrew Or] Create a -Prelease profile that flags `createDependencyReducedPom`
asfgit pushed a commit that referenced this pull request Jul 7, 2015
This is a workaround for MSHADE-148, which leads to an infinite loop when building Spark with maven 3.3.x. This was originally caused by #6441, which added a bunch of test dependencies on the spark-core test module. Recently, it was revealed by #7193.

This patch adds a `-Prelease` profile. If present, it will set `createDependencyReducedPom` to true. The consequences are:
- If you are releasing Spark with this profile, you are fine as long as you use maven 3.2.x or before.
- If you are releasing Spark without this profile, you will run into SPARK-8781.
- If you are not releasing Spark but you are using this profile, you may run into SPARK-8819.
- If you are not releasing Spark and you did not include this profile, you are fine.

This is all documented in `pom.xml` and tested locally with both versions of maven.

Author: Andrew Or <[email protected]>

Closes #7219 from andrewor14/fix-maven-build and squashes the following commits:

1d37e87 [Andrew Or] Merge branch 'master' of github.com:apache/spark into fix-maven-build
3574ae4 [Andrew Or] Review comments
f39199c [Andrew Or] Create a -Prelease profile that flags `createDependencyReducedPom`

Conflicts:
	dev/create-release/create-release.sh
	pom.xml
@andrewor14
Copy link
Contributor Author

Alright, I reverted this patch in master, 1.4 and 1.3 because #7219 proposes a nicer fix.

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.

5 participants