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

fix: maven-dependency-plugin configuration breaking downstream config #174

Merged
merged 1 commit into from Jul 1, 2020
Merged

fix: maven-dependency-plugin configuration breaking downstream config #174

merged 1 commit into from Jul 1, 2020

Conversation

BenWhitehead
Copy link
Contributor

When providing configuration to the maven-dependency-plugin,
ignoredUnusedDeclaredDependencies is a java.lang.String[] but mavens xml
handling allows multiple formats for providing a list of values (comma
separated string, or sub elements each specifying a single value). It appears
that when loading the config, the value of the configuration at a lower level
is blindly pre-pended as a string rather than first parsing and merging the
results. Below is the debug output of the resolved configuration using the
existing config added in cd635ad, the output after this change and a diff of
the two. Notice, in the left hand side of the diff the malformed xml.

The change introduced in this commit switches from using the comma separated
string representation, to using the sub element representation thereby allowing
the config resolution to create a valid config.

Resolved config as of cd635ad
[DEBUG] Goal:          org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze (default-cli)
[DEBUG] Style:         Regular
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <analyzer default-value="default">${analyzer}</analyzer>
  <baseDir default-value="${basedir}"/>
  <failOnWarning default-value="false">${failOnWarning}</failOnWarning>
  <ignoreNonCompile default-value="false">${ignoreNonCompile}</ignoreNonCompile>
  <ignoredUnusedDeclaredDependencies>
    <ignoredUnusedDeclaredDependency>com.google.http-client:google-http-client-jackson2</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>com.google.oauth-client:google-oauth-client</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependencies>
  <outputDirectory default-value="${project.build.directory}"/>
  <outputXML default-value="false">${outputXML}</outputXML>
  <project default-value="${project}"/>
  <scriptableFlag default-value="$$$%%%">${scriptableFlag}</scriptableFlag>
  <scriptableOutput default-value="false">${scriptableOutput}</scriptableOutput>
  <skip default-value="false">${mdep.analyze.skip}</skip>
  <verbose default-value="false">${verbose}</verbose>
</configuration>
Resolved config now
[DEBUG] Goal:          org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze (default-cli)
[DEBUG] Style:         Regular
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <analyzer default-value="default">${analyzer}</analyzer>
  <baseDir default-value="${basedir}"/>
  <failOnWarning default-value="false">${failOnWarning}</failOnWarning>
  <ignoreNonCompile default-value="false">${ignoreNonCompile}</ignoreNonCompile>
  <ignoredUnusedDeclaredDependencies>
    <ignoredUnusedDeclaredDependency>com.google.http-client:google-http-client-jackson2</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>com.google.oauth-client:google-oauth-client</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>
  </ignoredUnusedDeclaredDependencies>
  <outputDirectory default-value="${project.build.directory}"/>
  <outputXML default-value="false">${outputXML}</outputXML>
  <project default-value="${project}"/>
  <scriptableFlag default-value="$$$%%%">${scriptableFlag}</scriptableFlag>
  <scriptableOutput default-value="false">${scriptableOutput}</scriptableOutput>
  <skip default-value="false">${mdep.analyze.skip}</skip>
  <verbose default-value="false">${verbose}</verbose>
</configuration>

Diff between resolved configs
12c12,13
<     <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependencies>
---
>     <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>
>   </ignoredUnusedDeclaredDependencies>

When providing configuration to the maven-dependency-plugin,
`ignoredUnusedDeclaredDependencies` is a `java.lang.String[]` but mavens xml
handling allows multiple formats for providing a list of values (comma
separated string, or sub elements each specifying a single value). It appears
that when loading the config, the value of the configuration at a lower level
is blindly pre-pended as a string rather than first parsing and merging the
results. Below is the debug output of the resolved configuration using the
existing config added in cd635ad, the output after this change and a diff of
the two. Notice, in the left hand side of the diff the malformed xml.

The change introduced in this commit switches from using the comma separated
string representation, to using the sub element representation thereby allowing
the config resolution to create a valid config.

###### Resolved config as of cd635ad
```
[DEBUG] Goal:          org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze (default-cli)
[DEBUG] Style:         Regular
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <analyzer default-value="default">${analyzer}</analyzer>
  <baseDir default-value="${basedir}"/>
  <failOnWarning default-value="false">${failOnWarning}</failOnWarning>
  <ignoreNonCompile default-value="false">${ignoreNonCompile}</ignoreNonCompile>
  <ignoredUnusedDeclaredDependencies>
    <ignoredUnusedDeclaredDependency>com.google.http-client:google-http-client-jackson2</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>com.google.oauth-client:google-oauth-client</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependencies>
  <outputDirectory default-value="${project.build.directory}"/>
  <outputXML default-value="false">${outputXML}</outputXML>
  <project default-value="${project}"/>
  <scriptableFlag default-value="$$$%%%">${scriptableFlag}</scriptableFlag>
  <scriptableOutput default-value="false">${scriptableOutput}</scriptableOutput>
  <skip default-value="false">${mdep.analyze.skip}</skip>
  <verbose default-value="false">${verbose}</verbose>
</configuration>
```

###### Resolved config now
```
[DEBUG] Goal:          org.apache.maven.plugins:maven-dependency-plugin:3.1.2:analyze (default-cli)
[DEBUG] Style:         Regular
[DEBUG] Configuration: <?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <analyzer default-value="default">${analyzer}</analyzer>
  <baseDir default-value="${basedir}"/>
  <failOnWarning default-value="false">${failOnWarning}</failOnWarning>
  <ignoreNonCompile default-value="false">${ignoreNonCompile}</ignoreNonCompile>
  <ignoredUnusedDeclaredDependencies>
    <ignoredUnusedDeclaredDependency>com.google.http-client:google-http-client-jackson2</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>com.google.oauth-client:google-oauth-client</ignoredUnusedDeclaredDependency>
    <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>
  </ignoredUnusedDeclaredDependencies>
  <outputDirectory default-value="${project.build.directory}"/>
  <outputXML default-value="false">${outputXML}</outputXML>
  <project default-value="${project}"/>
  <scriptableFlag default-value="$$$%%%">${scriptableFlag}</scriptableFlag>
  <scriptableOutput default-value="false">${scriptableOutput}</scriptableOutput>
  <skip default-value="false">${mdep.analyze.skip}</skip>
  <verbose default-value="false">${verbose}</verbose>
</configuration>

```

##### Diff between resolved configs
```diff
12c12,13
<     <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependencies>
---
>     <ignoredUnusedDeclaredDependency>javax.annotation:javax.annotation-api</ignoredUnusedDeclaredDependency>
>   </ignoredUnusedDeclaredDependencies>
```
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 25, 2020
@chingor13 chingor13 merged commit 507217f into googleapis:master Jul 1, 2020
@BenWhitehead BenWhitehead deleted the fix-ignore-unused-javax-config branch July 1, 2020 16:16
gcf-merge-on-green bot pushed a commit that referenced this pull request Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants