[fix][build] Fix potential insufficient protostuff-related configs

[equanz]

Motivation

Currently, protobuf messages are built by lightproto. Lightproto uses protostuff in build. Protostuff can be configured using system property as follows:

pulsar/pulsar-broker/pom.xml

Lines 610 to 633 in e1d06b5

	<plugin>
	<groupId>org.codehaus.mojo</groupId>
	<artifactId>properties-maven-plugin</artifactId>
	<executions>
	<execution>
	<phase>initialize</phase>
	<goals>
	<goal>set-system-properties</goal>
	</goals>
	<configuration>
	<properties>
	<property>
	<name>proto_path</name>
	<value>${project.parent.basedir}</value>
	</property>
	<property>
	<name>proto_search_strategy</name>
	<value>2</value>
	</property>
	</properties>
	</configuration>
	</execution>
	</executions>
	</plugin>

pulsar/pulsar-transaction/coordinator/pom.xml

Lines 84 to 107 in e1d06b5

	<plugin>
	<groupId>org.codehaus.mojo</groupId>
	<artifactId>properties-maven-plugin</artifactId>
	<executions>
	<execution>
	<phase>initialize</phase>
	<goals>
	<goal>set-system-properties</goal>
	</goals>
	<configuration>
	<properties>
	<property>
	<name>proto_path</name>
	<value>${project.parent.parent.basedir}</value>
	</property>
	<property>
	<name>proto_search_strategy</name>
	<value>2</value>
	</property>
	</properties>
	</configuration>
	</execution>
	</executions>
	</plugin>

These configs are held as a static field, so it is impossible to separate each module's settings in a single build phase.
https://github.com/protostuff/protostuff/blob/protostuff-1.7.2/protostuff-parser/src/main/java/io/protostuff/parser/DefaultProtoLoader.java#L38-L39
https://github.com/protostuff/protostuff/blob/protostuff-1.7.2/protostuff-parser/src/main/java/io/protostuff/parser/DefaultProtoLoader.java#L49

For example, if we build as follows, pulsar-transaction/coordinator-specific configs are not used because maven builds pulsar-common first.

$ mvn package -pl pulsar-common,pulsar-transaction/coordinator -am -DskipTests=true

In the first place, there is no need to separate protostuff-related configs for each module, at least at present.

Modifications

• Add protostuff-related configs to the pulsar parent

Verifying this change

• Make sure that the change passes the CI checks.

This change is a trivial rework

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency)
• The public API
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: equanz#4

[tisonkun]

Thank you!

One of other approach is to add protostuff-related configs to the pulsar parent

Yes. I wonder if in this way we can remove other configs. Duplicate this config three times seems brittle ..

[equanz]

/pulsarbot rerun-failure-checks

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4198ed2) 73.61% compared to head (c5f5694) 73.61%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #21629      +/-   ##
============================================
- Coverage     73.61%   73.61%   -0.01%     
- Complexity    32424    32431       +7     
============================================
  Files          1861     1861              
  Lines        138678   138678              
  Branches      15184    15184              
============================================
- Hits         102089   102085       -4     
- Misses        28690    28698       +8     
+ Partials       7899     7895       -4     

Flag	Coverage Δ
inttests	24.09% <ø> (-0.01%)	⬇️
systests	23.74% <ø> (-0.02%)	⬇️
unittests	72.90% <ø> (+0.02%)	⬆️

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

see 76 files with indirect coverage changes

[equanz]

@tisonkun
sorry for the late reply.

Yes. I wonder if in this way we can remove other configs. Duplicate this config three times seems brittle ..

Ok. I've addressed your comment. PTAL

[tisonkun]

Thank you!