Move plugin assembly into a separate profile to fix apache release

[Jackie-Jiang]

Problem to solve:
Currently when running mvn install -Papache-release, it throws the following exception:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-assembly-plugin:3.7.1:single (make-assembly) on project pinot-plugins: Failed to create assembly: Error adding file to archive: /Users/jackie/Projects/pinot/pinot-plugins/target/classes/pinot-plugin.properties -> [Help 1]

[Jackie-Jiang]

@rfscholte @gortiz Please take a look and see if this is correct

[Jackie-Jiang]

cc @jadami10

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.77%. Comparing base (59551e4) to head (c143662).
Report is 1293 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14379      +/-   ##
============================================
+ Coverage     61.75%   63.77%   +2.02%     
- Complexity      207     1554    +1347     
============================================
  Files          2436     2660     +224     
  Lines        133233   145958   +12725     
  Branches      20636    22347    +1711     
============================================
+ Hits          82274    93090   +10816     
- Misses        44911    45990    +1079     
- Partials       6048     6878     +830     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (+99.99%)	⬆️
integration	100.00% <ø> (+99.99%)	⬆️
integration1	100.00% <ø> (+99.99%)	⬆️
integration2	0.00% <ø> (ø)
java-11	63.72% <ø> (+2.01%)	⬆️
java-21	63.67% <ø> (+2.05%)	⬆️
skip-bytebuffers-false	63.76% <ø> (+2.02%)	⬆️
skip-bytebuffers-true	63.64% <ø> (+35.91%)	⬆️
temurin	63.77% <ø> (+2.02%)	⬆️
unittests	63.77% <ø> (+2.02%)	⬆️
unittests1	55.49% <ø> (+8.60%)	⬆️
unittests2	34.14% <ø> (+6.41%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rfscholte]

I wouldn't do this, because both assemblies have a different purpose: the make-assembly should always be executed for plugins as it behaves like some packaging-plugin, creating the main deliverable. The source-release-assembly is only executed during release to generated the sources-jar (jar containing all the sourcefiles as required by deployments to maven central).

I do the see issue you're trying to solve. The -Papache-release triggers the maven-assembly-plugin (and with it all its execution-blocks), however you don't want it to be executed on pinot-plugins itself nor at the assembly-descriptor. So there are two options: only enable it on plugins or always enable it, but explicitly disabled on non-plugins. The original request was to activate it as easy as possible for plugins (like https://github.com/apache/pinot/blob/master/pinot-plugins/pinot-metrics/pinot-dropwizard/pom.xml#L40-L43), but we missed something.

Maybe the simplest solution is to move the assembly-plugin in the pinot-plugins to a separate profile, like this:

	<profile>
		<id>plugin-assembly</id>
		<activation>
			<file>
				<exists>src/main/resources/pinot-plugin.properties</exists>
			</file>
		</activation>
		<build>
			<plugins>
				<plugin>
					<groupId>org.apache.maven.plugins</groupId>
					<artifactId>maven-assembly-plugin</artifactId>
					<dependencies>
						<dependency>
							<groupId>org.apache.pinot</groupId>
							<artifactId>assembly-descriptor</artifactId>
							<version>${project.version}</version>
						</dependency>
					</dependencies>
					<executions>
						<execution>
							<id>make-assembly</id>
							<phase>package</phase>
							<goals>
								<goal>single</goal>
							</goals>
							<configuration>
								<descriptorRefs>
									<descriptorRef>pinot-plugin</descriptorRef>
								</descriptorRefs>
								<archiverConfig>
									<compress>${archiver.compress}</compress>
								</archiverConfig>
								<recompressZippedFiles>
									${archiver.recompressZippedFiles}</recompressZippedFiles>
							</configuration>
						</execution>
					</executions>
				</plugin>
			</plugins>
		</build>
	</profile>

[Jackie-Jiang]

@rfscholte Thanks for the suggestion! Made the change accordingly.
Do you see other changes required? IIUC, this PR will change the behavior to not run the assembly plugin by default. Do you see side effect of that?

[Jackie-Jiang]

@xiangfu0 Can you also help take a look?

[rfscholte]

@rfscholte Thanks for the suggestion! Made the change accordingly. Do you see other changes required? IIUC, this PR will change the behavior to not run the assembly plugin by default. Do you see side effect of that?

If it now works with both with or without -Papache-release I would expect it to be fine. The only downside I can think of, is that it is less clear how/why the zip is created, i.e. the existence of pinot-plugin.properties as defined in the parent.
For third parties that want to build a pinot-plugin, I would suggest to simply use the complete maven-assembly-plugin in their build.

[gortiz]

I like this approach. Anyway, we are not using the new packaging right now, so this should be safe anything that disables that packaging right now. We are planning to work on this in the following months and part of the work to be done is to test a release using the new plugging packaging.