Skip to content

Conversation

@ajantha-bhat
Copy link
Member

Importing
testFixturesImplementation project(':iceberg-core').sourceSets.test.runtimeClasspath
brings everything to class path and creates problem for shadow jar.

Need to depend only on required classes.

@github-actions github-actions bot added the build label Oct 29, 2024
testFixturesImplementation project(':iceberg-gcp')
testFixturesImplementation project(':iceberg-azure')
testFixturesImplementation(libs.hadoop3.common) {
exclude group: 'log4j'
Copy link
Member Author

Choose a reason for hiding this comment

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

exclusion copied from other modules.

Needed hadoop dependency for Configuration class in RCKUtils

exclude group: 'org.codehaus.woodstox'
exclude group: 'org.eclipse.jetty'
}
testFixturesImplementation project(path: ':iceberg-bundled-guava', configuration: 'shadow')
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed as we are using Maps from Guava.

exclude group: 'org.eclipse.jetty'
}
testFixturesImplementation project(path: ':iceberg-bundled-guava', configuration: 'shadow')
testFixturesImplementation libs.junit.jupiter
Copy link
Member Author

Choose a reason for hiding this comment

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

Junit dependency was previously coming from core runtime path.

testFixturesImplementation project(':iceberg-api')
testFixturesImplementation project(':iceberg-core')
testFixturesImplementation project(path: ':iceberg-core', configuration: 'testArtifacts')
testFixturesImplementation project(':iceberg-core').sourceSets.test.runtimeClasspath
Copy link
Member Author

Choose a reason for hiding this comment

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

This brings too many things from test runtime class path and creates a problem for shadow jar. For example it brings some csv files, GPL license file etc.

Copy link
Contributor

@danielcweeks danielcweeks left a comment

Choose a reason for hiding this comment

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

Thanks @ajantha-bhat

@Fokko
Copy link
Contributor

Fokko commented Oct 29, 2024

@ajantha-bhat Great catch, thank you!

@Fokko Fokko merged commit 740d4e7 into apache:main Oct 29, 2024
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants