Support `attachMap` and `attachSeed` independently of `attach`

[maloewe-ona]

Currently the parameters attachMap and attachSeed only have an effect if attach is set as well.
However, there might be cases where it would be useful to replace the final JAR file (i.e. attach=false), but still use attachMap=true to attach the mappings to make troubleshooting later easier.

Would it be possible to decouple these options so they are independent of attach?

Workaround

Set attach=true and appendClassifier=false, and then attachMap=true or attachSeed=true.

It appears this will then make the plugin use the same output name as the original JAR file, therefore replacing it and not additionally attaching the processed JAR.
But that feels rather hacky and I am not sure if that is officially supported usage of the plugin, since it defeats the purpose of attach=true.

[lasselindqvist]

So attach is related to the outJar/inJar parameters.
For outJar the docs say: "If attach=true the value ignored and name constructed base on classifier. If empty input jar would be overdriven."

Attaching is not really needed for anything but automatically deploying the produced artifacts to a repository. You can leave attach as false and manually use https://www.mojohaus.org/build-helper-maven-plugin/attach-artifact-mojo.html to attach whatever artifacts you wish.

So also if you want to obfuscate, but not deploy the obfuscated JAR, you could attach the seed and map files with the build-helper-maven-plugin.

In your case where you obfuscate and replace the original JAR and attach and deploy the JAR, seed an map, I would say the configuration you use is correct and fine.

[maloewe-ona]

Attaching is not really needed for anything but automatically deploying the produced artifacts to a repository. You can leave attach as false and manually use https://www.mojohaus.org/build-helper-maven-plugin/attach-artifact-mojo.html to attach whatever artifacts you wish.

Yes, I am trying to attach the files for deployment to a remote repository. Thanks for the link to the Build Helper Maven Plugin; but since attaching map and seed file is already part of the ProGuard Maven Plugin (with the limitation described in this issue here), I would like to avoid using the Build Helper Maven Plugin because it would make my pom.xml more verbose and slightly error-prone, because it requires setting up two plugins to properly work with each other.

In your case where you obfuscate and replace the original JAR and attach and deploy the JAR, seed an map, I would say the configuration you use is correct and fine.

Regarding this "and attach [...] the JAR" part in your message (emphasis mine), that is why this workaround mentioned in the issue description feels so hacky, because it does not attach the JAR (the case sameArtifact=true in the code below), the original JAR is simply replaced:

proguard-maven-plugin/src/main/java/com/github/wvengen/maven/proguard/ProGuardMojo.java

Lines 843 to 853 in 790b51e

	if (attach) {
	if (!sameArtifact) {
	final String classifier;
	if (useArtifactClassifier()) {
	classifier = attachArtifactClassifier;
	} else {
	classifier = null;
	}
	projectHelper.attachArtifact(mavenProject, attachArtifactType, classifier, outJarFile);
	}

And the workaround is also quite error-prone; you have to tweak other parameters (in this case appendClassifier=false) to get the ProGuard Maven Plugin to have sameArtifact=true to achieve the desired effect (i.e. only deploy small JAR and map). If you mess up these parameters, or if the behavior of the ProGuard Maven Plugin changes in a future version (either by accident or intentionally), then the main JAR will not be the small JAR anymore, but instead it is attached as separate JAR (which is undesired for my use case).

So to summarize, what I would like is to have the following deployed artifacts:

• small JAR
• map file

The situation that I would like to avoid:

• normal JAR
• small JAR (with classifier)
• map file

I think from a technical perspective there is nothing which would prevent making attachMap and attachSeed independent of attach; it might just involve moving the following lines outside the enclosing if (attach) { ... } statement (and adjusting documentation of the Mojo parameters):

proguard-maven-plugin/src/main/java/com/github/wvengen/maven/proguard/ProGuardMojo.java

Lines 855 to 862 in 790b51e

	final String mainClassifier = useArtifactClassifier() ? attachArtifactClassifier : null;
	final File buildOutput = new File(mavenProject.getBuild().getDirectory());
	if (attachMap) {
	attachTextFile(new File(buildOutput, mappingFileName), mainClassifier, "map");
	}
	if (attachSeed) {
	attachTextFile(new File(buildOutput, seedFileName), mainClassifier, "seed");
	}

[lasselindqvist]

Yes, I think we could just move the attachMap and attachSeed conditions to be fully separate from attach condition. It make make sense to rename attach to be attachJar then for consistency, but that would be a breaking change.

[maloewe-ona]

It make make sense to rename attach to be attachJar then for consistency, but that would be a breaking change.

Before doing a full rename, it might be possible to keep the existing attach parameter but mark it as @Deprecated (+ additionally with @deprecated Javadoc tag referring to new parameter), and add the new attachJar.
Then if attach=true, treat it as if attachJar was set to true (and maybe log a warning).
But that might require changing attach to Boolean and removing the default value (to detect when it had not been set by the user); not sure if that could be causing issues.

Though maybe this could still be considered a slight backward incompatible change since the behavior of attach changes.

[lasselindqvist]

I think that is a good strategy:

1. Deprecate attach.
2. Add attachJar parameter
3. Make both Big Booleans. If attachJar is not-null, use that value. If null, use attach if not-null. If both are null, default to the current default, which is false.
4. In addition to point 3, always log warning if attach is not-null.
5. In a later version, remove attach parameter.