[FLINK-22555][build][python] Exclude leftover jboss files #15827

zentol · 2021-05-04T10:01:58Z

Removes auto-generated maven metadata from all jars as this may accidentally bundle poms under incompatible licenses, and adds another exclusions for jboss schema files contained in flink-python.

flinkbot · 2021-05-04T10:08:55Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit 2632d0b (Sat Aug 28 11:18:00 UTC 2021)

Warnings:

15 pom.xml files were touched: Check for build and licensing issues.
No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❓ 3. Needs [attention] from.
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2021-05-04T10:27:10Z

CI report:

6250916 Azure: FAILURE

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build
@flinkbot run azure re-run the last Azure build

pom.xml

tillrohrmann

Thanks for creating this PR @zentol. The change looks in general good to me. It seems that the module flink-connector-cassandra_2.11 does not pick up the exclusion. Moreover, the e2e test Quickstarts Java nightly end-to-end test fails on AZP. It would be good to verify that this is not caused by your changes.

tillrohrmann

flink-dist_2.11-1.14-SNAPSHOT.jar

META-INF/versions/11/META-INF/maven/
META-INF/versions/11/META-INF/maven/javax.activation/
META-INF/versions/11/META-INF/maven/javax.activation/javax.activation-api/
META-INF/versions/11/META-INF/maven/javax.activation/javax.activation-api/pom.xml
META-INF/versions/11/META-INF/maven/javax.activation/javax.activation-api/pom.properties
META-INF/versions/11/META-INF/maven/javax.xml.bind/
META-INF/versions/11/META-INF/maven/javax.xml.bind/jaxb-api/
META-INF/versions/11/META-INF/maven/javax.xml.bind/jaxb-api/pom.xml
META-INF/versions/11/META-INF/maven/javax.xml.bind/jaxb-api/pom.properties
META-INF/maven/
META-INF/maven/org.apache.flink/
META-INF/maven/org.apache.flink/flink-dist_2.11/
META-INF/maven/org.apache.flink/flink-dist_2.11/pom.xml
META-INF/maven/org.apache.flink/flink-dist_2.11/pom.properties
META-INF/maven/org.apache.commons/
META-INF/maven/org.apache.commons/commons-lang3/
META-INF/maven/org.apache.commons/commons-lang3/pom.xml
META-INF/maven/org.apache.commons/commons-lang3/pom.properties
META-INF/maven/com.esotericsoftware.kryo/
META-INF/maven/com.esotericsoftware.kryo/kryo/
META-INF/maven/com.esotericsoftware.kryo/kryo/pom.properties
META-INF/maven/com.esotericsoftware.kryo/kryo/pom.xml
META-INF/maven/com.esotericsoftware.reflectasm/
META-INF/maven/com.esotericsoftware.reflectasm/reflectasm/
META-INF/maven/com.esotericsoftware.reflectasm/reflectasm/pom.xml
META-INF/maven/com.esotericsoftware.reflectasm/reflectasm/pom.properties
META-INF/maven/com.esotericsoftware.minlog/
META-INF/maven/com.esotericsoftware.minlog/minlog/
META-INF/maven/com.esotericsoftware.minlog/minlog/pom.xml
META-INF/maven/com.esotericsoftware.minlog/minlog/pom.properties
META-INF/maven/commons-collections/
META-INF/maven/commons-collections/commons-collections/
META-INF/maven/commons-collections/commons-collections/pom.xml
META-INF/maven/commons-collections/commons-collections/pom.properties
META-INF/maven/org.apache.commons/commons-compress/
META-INF/maven/org.apache.commons/commons-compress/pom.xml
META-INF/maven/org.apache.commons/commons-compress/pom.properties
META-INF/maven/org.apache.commons/commons-math3/
META-INF/maven/org.apache.commons/commons-math3/pom.xml
META-INF/maven/org.apache.commons/commons-math3/pom.properties
META-INF/maven/jline/
META-INF/maven/jline/jline/
META-INF/maven/jline/jline/pom.properties
META-INF/maven/jline/jline/pom.xml
META-INF/maven/commons-io/
META-INF/maven/commons-io/commons-io/
META-INF/maven/commons-io/commons-io/pom.xml
META-INF/maven/commons-io/commons-io/pom.properties
META-INF/maven/commons-cli/
META-INF/maven/commons-cli/commons-cli/
META-INF/maven/commons-cli/commons-cli/pom.xml
META-INF/maven/commons-cli/commons-cli/pom.properties
META-INF/maven/org.javassist/
META-INF/maven/org.javassist/javassist/
META-INF/maven/org.javassist/javassist/pom.properties
META-INF/maven/org.javassist/javassist/pom.xml
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/pom.xml
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/pom.properties
META-INF/maven/io.fabric8/
META-INF/maven/io.fabric8/kubernetes-client/
META-INF/maven/io.fabric8/kubernetes-client/pom.xml
META-INF/maven/io.fabric8/kubernetes-client/pom.properties
META-INF/maven/io.fabric8/kubernetes-model/
META-INF/maven/io.fabric8/kubernetes-model/pom.properties
META-INF/maven/io.fabric8/kubernetes-model/pom.xml
META-INF/maven/io.fabric8/kubernetes-model-common/
META-INF/maven/io.fabric8/kubernetes-model-common/pom.xml
META-INF/maven/io.fabric8/kubernetes-model-common/pom.properties
META-INF/maven/com.squareup.okhttp3/
META-INF/maven/com.squareup.okhttp3/okhttp/
META-INF/maven/com.squareup.okhttp3/okhttp/pom.properties
META-INF/maven/com.squareup.okhttp3/okhttp/pom.xml
META-INF/maven/com.squareup.okio/
META-INF/maven/com.squareup.okio/okio/
META-INF/maven/com.squareup.okio/okio/pom.xml
META-INF/maven/com.squareup.okio/okio/pom.properties
META-INF/maven/com.squareup.okhttp3/logging-interceptor/
META-INF/maven/com.squareup.okhttp3/logging-interceptor/pom.xml
META-INF/maven/com.squareup.okhttp3/logging-interceptor/pom.properties
META-INF/maven/com.fasterxml.jackson.dataformat/
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/pom.properties
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/pom.xml
META-INF/maven/org.yaml/
META-INF/maven/org.yaml/snakeyaml/
META-INF/maven/org.yaml/snakeyaml/pom.properties
META-INF/maven/org.yaml/snakeyaml/pom.xml
META-INF/maven/com.fasterxml.jackson.datatype/
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/pom.properties
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/pom.xml
META-INF/maven/com.fasterxml.jackson.core/
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.xml
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/pom.xml
META-INF/maven/com.fasterxml.jackson.core/jackson-core/
META-INF/maven/com.fasterxml.jackson.core/jackson-core/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-core/pom.xml
META-INF/maven/io.fabric8/zjsonpatch/
META-INF/maven/io.fabric8/zjsonpatch/pom.properties
META-INF/maven/io.fabric8/zjsonpatch/pom.xml
META-INF/maven/org.apache.flink/force-shading/
META-INF/maven/org.apache.flink/force-shading/pom.xml
META-INF/maven/org.apache.flink/force-shading/pom.properties
META-INF/maven/org.slf4j/
META-INF/maven/org.slf4j/slf4j-api/
META-INF/maven/org.slf4j/slf4j-api/pom.xml
META-INF/maven/org.slf4j/slf4j-api/pom.properties

./flink-kubernetes/target/flink-kubernetes_2.11-1.14-SNAPSHOT.jar

META-INF/maven/
META-INF/maven/org.apache.flink/
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/pom.xml
META-INF/maven/org.apache.flink/flink-kubernetes_2.11/pom.properties
META-INF/maven/io.fabric8/
META-INF/maven/io.fabric8/kubernetes-client/
META-INF/maven/io.fabric8/kubernetes-client/pom.xml
META-INF/maven/io.fabric8/kubernetes-client/pom.properties
META-INF/maven/io.fabric8/kubernetes-model/
META-INF/maven/io.fabric8/kubernetes-model/pom.properties
META-INF/maven/io.fabric8/kubernetes-model/pom.xml
META-INF/maven/io.fabric8/kubernetes-model-common/
META-INF/maven/io.fabric8/kubernetes-model-common/pom.xml
META-INF/maven/io.fabric8/kubernetes-model-common/pom.properties
META-INF/maven/com.squareup.okhttp3/
META-INF/maven/com.squareup.okhttp3/okhttp/
META-INF/maven/com.squareup.okhttp3/okhttp/pom.properties
META-INF/maven/com.squareup.okhttp3/okhttp/pom.xml
META-INF/maven/com.squareup.okio/
META-INF/maven/com.squareup.okio/okio/
META-INF/maven/com.squareup.okio/okio/pom.xml
META-INF/maven/com.squareup.okio/okio/pom.properties
META-INF/maven/com.squareup.okhttp3/logging-interceptor/
META-INF/maven/com.squareup.okhttp3/logging-interceptor/pom.xml
META-INF/maven/com.squareup.okhttp3/logging-interceptor/pom.properties
META-INF/maven/com.fasterxml.jackson.dataformat/
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/pom.properties
META-INF/maven/com.fasterxml.jackson.dataformat/jackson-dataformat-yaml/pom.xml
META-INF/maven/org.yaml/
META-INF/maven/org.yaml/snakeyaml/
META-INF/maven/org.yaml/snakeyaml/pom.properties
META-INF/maven/org.yaml/snakeyaml/pom.xml
META-INF/maven/com.fasterxml.jackson.datatype/
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/pom.properties
META-INF/maven/com.fasterxml.jackson.datatype/jackson-datatype-jsr310/pom.xml
META-INF/maven/com.fasterxml.jackson.core/
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-annotations/pom.xml
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-databind/pom.xml
META-INF/maven/com.fasterxml.jackson.core/jackson-core/
META-INF/maven/com.fasterxml.jackson.core/jackson-core/pom.properties
META-INF/maven/com.fasterxml.jackson.core/jackson-core/pom.xml
META-INF/maven/io.fabric8/zjsonpatch/
META-INF/maven/io.fabric8/zjsonpatch/pom.properties
META-INF/maven/io.fabric8/zjsonpatch/pom.xml

And in general the jars of the connectors also seem to contain a non empty META-INF/maven directory.

zentol · 2021-05-05T14:22:17Z

Well this was quite an ordeal.

The quickstart test failure was related; archetype jars contain an archetype-metadata.xml file under META-INF/maven, that was now accidentally removed. The filter pattern was adjusted to ignore this files.

We then had various files still containing poms under META-INF/maven:

Once cause was jars being built with the maven-(jar|assembly|source)-plugin, which needed to be configured separately to not add these files.

The second cause was various modules using ever so slightly shade-plugin setups; some were flat-out overriding the configuration of the parent so they didn't inherit things (like cassandra), others like flink-dist have special shade-plugin executions for reasons. This was resolved by moving the filters (and common transformers, while I was at it) into the general configuration of the shade-plugin (i.e., independent of any execution) and making small adjustments here and there as to how configurations are inherited/merged.

Finally, the java11 exclusive dependencies still contained poms, because the pattern assumed this directory to always be present under META-INF/maven, but for multi-release jars it can be present under META-INF/versions/....

zentol · 2021-05-06T10:35:42Z

flink-dist/pom.xml

 						<goals>
 							<goal>shade</goal>
 						</goals>
-						<configuration combine.self="override">


this removal is safe because it is a distinct execution, and hence never had any effect.

zentol · 2021-05-06T10:35:47Z

flink-dist/pom.xml

 						<goals>
 							<goal>shade</goal>
 						</goals>
-						<configuration combine.self="override">


this removal is safe because it is a distinct execution, and hence never had any effect.

zentol · 2021-05-06T10:36:28Z

flink-connectors/flink-connector-cassandra/pom.xml

 						<goals>
 							<goal>shade</goal>
 						</goals>
-						<configuration combine.self="override">


this removal is safe because it the override was plain unnecessary, and we get rid of an odd special case.

zentol · 2021-05-06T10:38:00Z

flink-kubernetes/pom.xml

 						<goals>
 							<goal>shade</goal>
 						</goals>
-						<configuration combine.children="append">


afaik this doesn't make sense and just results in any parent configurations being ignored. Similar to cassandra it is unnecessary because we aren't doing anything special, and we get rid of an odd special case.

zentol · 2021-05-06T10:39:04Z

flink-connectors/flink-connector-kafka/pom.xml

 						<configuration>
+							<archive>
+								<!-- Globally exclude maven metadata, because it may accidentally bundle files we don't intend to -->
+								<addMavenDescriptor>false</addMavenDescriptor>


ideally we move this into the pluginManagement section in the root pom, but I already spent too much time on this ticket (and actually pin the plugin version...)

zentol · 2021-05-06T10:39:34Z

flink-dist/pom.xml

 				<artifactId>maven-shade-plugin</artifactId>
 				<executions>
 					<execution>
+						<id>shade-dist</id>


This is essentially just a safeguard against accidentally inheriting something from somewhere.

tillrohrmann

Thanks for updating this PR @zentol. As far as I can tell, the changes look good to me. I built Flink and checked the flink-dist contents. I could not find any problematic contents.

Nit: The quickstarts still contain some logic to deactivate the maven-shade plugin. I guess they can be removed now.

tillrohrmann · 2021-05-06T09:51:21Z

pom.xml

+									The initial '**' allows the pattern to also work for multi-release jars that may contain such entries under
+									'META-INF/versions/11/META-INF/maven/'.
+								 	-->
+								<exclude>**/META-INF/maven/?*/?*/**</exclude>


Would it work to explicitly add the archetype-metadata.xml in the quickstart as an inclusion?

I had the same idea, but don't think it would work because exclusions are used to narrow down inclusions.
The default inclusion is *, matching all files and we exclude some of those..
If set an inclusion for archetype-metadata.xml, then all other files are implicitly excluded. We'd need include for everything we truly want in the jar, but that seems like more overhead than the current approach.

Ah ok. Then let's keep the current solution.

…hade-plugin configuration

…ecution

rmetzger added review=description? component=API/Python labels May 4, 2021

zentol commented May 4, 2021

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

tillrohrmann reviewed May 5, 2021

View reviewed changes

tillrohrmann requested changes May 5, 2021

View reviewed changes

zentol force-pushed the 22555 branch 2 times, most recently from 8764643 to 7a82a27 Compare May 5, 2021 14:22

zentol commented May 6, 2021

View reviewed changes

tillrohrmann approved these changes May 6, 2021

View reviewed changes

[FLINK-22560][build] Move generic filters/transformers into general s…

ee87f7c

…hade-plugin configuration

zentol force-pushed the 22555 branch from 6250916 to b8cc0d7 Compare May 7, 2021 08:25

zentol added 3 commits May 7, 2021 10:25

[FLINK-22560][build] Filter maven metadata directory

c161969

[FLINK-22560][build] Add dedicated name to flink-dist shade-plugin ex…

f90acde

…ecution

[FLINK-22555][build][python] Exclude leftover jboss files

2632d0b

zentol force-pushed the 22555 branch from b8cc0d7 to 2632d0b Compare May 7, 2021 08:26

zentol merged commit 8f3d483 into apache:master May 7, 2021

zentol deleted the 22555 branch May 11, 2021 22:07

[FLINK-22555][build][python] Exclude leftover jboss files #15827

[FLINK-22555][build][python] Exclude leftover jboss files #15827

Uh oh!

Conversation

zentol commented May 4, 2021

Uh oh!

flinkbot commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automated Checks

Review Progress

Uh oh!

flinkbot commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

Uh oh!

tillrohrmann left a comment

Choose a reason for hiding this comment

Uh oh!

tillrohrmann left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zentol commented May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tillrohrmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

flinkbot commented May 4, 2021 •

edited

Loading

flinkbot commented May 4, 2021 •

edited

Loading

tillrohrmann left a comment •

edited

Loading

zentol commented May 5, 2021 •

edited

Loading