Move bnd-maven-plugin to a profile.

[motlin]

Moved bnd-maven-plugin configuration to the release-artifacts profile so that it will affect released jars but will otherwise not affect local development.

cc @fipro78

[fipro78]

But that also means that you are not able to test the OSGi stuff with a local build, because the MANIFEST.MF files are not generated with the necessary OSGi meta-data. You need to do a release build, which you can not do locally IIUC. So how would you verify that the changes work in an OSGi environment with a local build after that change?

IMHO that change is unfortunate.

[motlin]

But that also means that you are not able to test the OSGi stuff with a local build. You need to do a release build, which you can not do locally IIUC. So how would you verify that the changes work in an OSGi environment with a local build after that change?

I created this PR somewhat as an experiment, not sure if the builds will pass. I think the fact that the builds pass shows that there are no automated tests of the OSGi functionality. What is your expectation for testing - manual or automated tests? I'm not aware of anyone doing manual testing either - am I missing anything?

[fipro78]

I suppose there are no automatic OSGi tests, as before the OSGi bundle was created in a separate step that created the p2 repository. Now the artifacts are itself OSGi bundles. But I have not added OSGi specific tests, would need to see how that would look like. So I was referring to manual tests, which I did a lot while I was implementing the changes. If you ever have an issue related to OSGi, nobody will be able to work in that area, because you will only get the OSGi meta-data on a release build. So it can only be fixed after a release.

It actually generates the meta-data. Why do you want it to be done only for a release and not already at development time? This way the release artifacts would be different in terms of the content from development artefacts.

[motlin]

If folks do want to do OSGi testing manually, I could arrange the maven profiles slightly differently to make that happen. Rather than reuse the release-artifacts profile, I'd create a new profile called bnd-maven-plugin that also is activated when performRelease is set to true. This way, anyone could test with --activate-profiles bnd-maven-plugin if they want, and we could even activate that profile in GitHub workflows if that gives peace of mind.

However, I suspect there are no manual tests of OSGi functionality of any sort. You know I was running into trouble getting the bnd annotations to be recognized by my IDE. After staring at the problem for a while I realized that it was running the maven build which broke the IDE, and running a clean build from the IDE which fixed it. That means whenever I'm running tests or debugging in the IDE, I'm doing so after clearing out everything related to OSGi.

There is probably another way forward here, which is to configure the IDE with annotation processors or whatever it needs in order to understand the OSGi metadata, but I don't know how.

[fipro78]

What setup do you have? Which IDE? What steps do you perform to raise the issue you have?

Maybe I can reproduce it somehow and find a better solution.

[motlin]

What setup do you have? Which IDE? What steps do you perform to raise the issue you have?

Maybe I can reproduce it somehow and find a better solution.

To reproduce:

git checkout upstream/master
mvn clean install

Open IntelliJ. Navigate to any test. As an example, I was just working on UnmodifiableMutableOrderedMapTest. Click the gutter icon that looks like a play button to run the test. The IDE will error out with many lines of the form:

/Users/Craig/projects/eclipse-collections/eclipse-collections-api/target/generated-sources/java/org/eclipse/collections/api/factory/primitive/IntStacks.java:24:22
java: package aQute.bnd.annotation.spi is not visible
  (package aQute.bnd.annotation.spi is declared in the unnamed module, but module org.eclipse.collections.api does not read it)

To get around this error, navigate to the Build menu, then Rebuild Project. This is the step that's like a clean and gets rid of all OSGi metadata. It takes a few minutes because it has to recompile everything.

Afterward, the tests will run. If we switch back to maven and run mvn install without clean then we can get maven to error with a sort of mirrored set of error messages.

10:22:12:654 [ERROR] /Users/Craig/projects/eclipse-collections/eclipse-collections-api/target/generated-sources/java/org/eclipse/collections/api/factory/primitive/LongLists.java:[24,21] error: package aQute.bnd.annotation.spi is not visible

[fipro78]

And why do you call mvn clean install and not just mvn clean verify?

[motlin]

And why do you call mvn clean install and not just mvn clean verify?

No reason, I think either one will reproduce the issue.

[fipro78]

Have you tried the verify approach?

Craig P. Motlin @.***> schrieb am Do., 8. Aug. 2024, 16:36:

And why do you call mvn clean install and not just mvn clean verify?

No reason, I think either one will reproduce the issue.

— Reply to this email directly, view it on GitHub https://github.com/eclipse/eclipse-collections/pull/1665#issuecomment-2275992409, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXGVI7WPLUPRKRFX2HZJKLZQN67ZAVCNFSM6AAAAABMGORN66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZVHE4TENBQHE . You are receiving this because you were mentioned.Message ID: @.***>

[motlin]

Have you tried the verify approach?

Just tried it now, same thing happens.

While scrolling through the error messages to make sure they are identical, I realized there's another pattern of error mixed in.

Same as before:

/Users/Craig/projects/eclipse-collections/eclipse-collections-api/target/generated-sources/java/org/eclipse/collections/api/factory/primitive/IntStacks.java:24:22
java: package aQute.bnd.annotation.spi is not visible
  (package aQute.bnd.annotation.spi is declared in the unnamed module, but module org.eclipse.collections.api does not read it)

The one I didn't notice/mention before:

/Users/Craig/projects/eclipse-collections/eclipse-collections-api/target/generated-sources/java/org/eclipse/collections/api/factory/primitive/IntStacks.java:25:26
java: package aQute.bnd.annotation.spi does not exist

[fipro78]

Hm, it looks like some issue with the generated jpms stuff. Maybe intellij uses the module.info to determine the test build module path.

Need to investigate in that area. There are posts related to similar issues in the internet. Maybe the spi packages need to be specified somehow although not necessary at runtime?

[fipro78]

I can find several entries related to issues with intellij

https://stackoverflow.com/questions/20137020/why-does-intellij-give-me-package-doesnt-exist-error https://intellij-support.jetbrains.com/hc/en-us/community/posts/12200628643346-Recompiled-failed-java-package-does-not-exist https://youtrack.jetbrains.com/issue/IDEA-291860/Package-xxx-is-declared-in-the-unnamed-module-but-module-yyy-does-not-read-it

To narrow down the issue, could you try to change the scope of the bnd annotations from provided to compile? Just to see if this changes anything.

[motlin]

To narrow down the issue, could you try to change the scope of the bnd annotations from provided to compile? Just to see if this changes anything.

I tried changing the scope of the dependency to compile in both places, reran a clean maven build, and clicked run on some tests in IntelliJ. I got the error messages about "java: package aQute.bnd.annotation.spi is not visible", just the one error kind this time.

[fipro78]

@motlin

I tried different IDEs to see if it is a general issue:

1. Eclipse IDE

• Fresh checkout from GitHub git clone https://github.com/eclipse/eclipse-collections.git
• Perform a build as explained in the docs mvn clean install -DskipTests=true
• Import projects

After the import I get several errors about missing classes. The generated sources in the target folder are not added as source folders!

• Fix the project setups

  • manually add the target/generated-sources/java folder to the build path of eclipse-collections-api and eclipse-collections
  • create empty src/main/java and src/main/resources folders in the unit-tests project
  • add the target/generated-test-sources/java folder to the build path of the unit-tests project and configure it as test resource via properties

• Run the UnmodifiableMutableOrderedMapTest via Run As - JUnit Test

Execution succeeds!

• Rerun the Maven build mvn clean install -DskipTests=true
• Run the UnmodifiableMutableOrderedMapTest via Run As - JUnit Test

Execution succeeds!

2. VS Code

• Fresh checkout from GitHub git clone https://github.com/eclipse/eclipse-collections.git
• Perform a build as explained in the docs mvn clean install -DskipTests=true
• Import projects

After the import I get several errors about missing classes. The generated sources in the target folder are not added as source folders!

• Fix the project setups To add the generated sources as source folders you need to modify the pom.xml files and add the following to the plugins section of eclipse-collections-api and eclipse-collections after the eclipse-collections-code-generator-maven-plugin

<plugin>
	<groupId>org.codehaus.mojo</groupId>
	<artifactId>build-helper-maven-plugin</artifactId>
	<version>3.2.0</version>
	<executions>
		<execution>
			<phase>generate-sources</phase>
			<goals>
				<goal>add-source</goal>
			</goals>
			<configuration>
				<sources>
					<source>target/generated-sources/java</source>
				</sources>
			</configuration>
		</execution>
	</executions>
</plugin>

In the pom.xml file of the unit-tests project add the following plugin:

<plugin>
	<groupId>org.codehaus.mojo</groupId>
	<artifactId>build-helper-maven-plugin</artifactId>
	<version>3.2.0</version>
	<executions>
		<execution>
			<phase>generate-test-sources</phase>
			<goals>
				<goal>add-test-source</goal>
			</goals>
			<configuration>
				<sources>
					<source>target/generated-test-sources/java</source>
				</sources>
			</configuration>
		</execution>
	</executions>
</plugin>

• Run the UnmodifiableMutableOrderedMapTest via Test Runner for the Java extension

Execution succeeds!

• Rerun the Maven build mvn clean install -DskipTests=true
• Run the UnmodifiableMutableOrderedMapTest via Test Runner for the Java extension

Execution succeeds!

3. IntelliJ

• Fresh checkout from GitHub git clone https://github.com/eclipse/eclipse-collections.git
• Perform a build as explained in the docs mvn clean install -DskipTests=true
• Import projects

No errors at this point. Probably because of the .idea folder or any other configuration.

• Run the UnmodifiableMutableOrderedMapTest

Execution succeeds!

• Rerun the Maven build mvn clean install -DskipTests=true
• Run the UnmodifiableMutableOrderedMapTest

Execution fails with the java: package aQute.bnd.annotation.spi is not visible error!

IMHO it is a bug in IntelliJ.

• The Maven build succeeds and other IDEs do not show this error.
• The error itself is strange, because it says that the package is not visible, but the compilation succeeds. I can only assume that it is related to JPMS and that IntelliJ uses the module system for the compilation internally. The annotation library is not a module and therefore does not export its packages via module-info. So yes, the annotations are not visible for JPMS. But they don't have to, because they are not needed at runtime!

My best guess is that when the projects are initially opened, IntelliJ does not recognize the JPMS nature. After the build there is some hook or other mechanism that spots the module-info.class in the resulting JAR and makes the projects "module" projects, which then bring up the error. Which I don't really understand, because the project itself is not setup as a Java module project. The module nature is added at build time and only in the resulting JAR. Maybe the mvn install triggers some update in IntelliJ which then does a Maven Update and gets the information from the local Maven repository instead of the project. But that is just a guess, not sure if that is really the case. You could try to do the mvn install only for the code generator plugin, which is needed to be installed. Then only call mvn clean verify to avoid that the API and IMPL modules get installed into the local repository, and see if that changes anything. That of course means you first need to clear your local Maven repository to avoid that existing artefacts are loaded from there.

I am not an IntelliJ user, and therefore I have absolutely no idea what is going on here and why. Sorry to say that, but at this point I am not able to help out anymore. The code is correct, the project setup could be corrected to work with all IDEs (see above, the pom.xml changes should be sufficient for both, Eclipse and VS Code). Why IntelliJ behaves this way, no idea.

[motlin]

@fipro78 thank you for the thorough investigation. I looked around in YouTrack for an IntelliJ bug like what we're seeing and couldn't find anything that matches exactly.

The build-helper-maven-plugin config shouldn't be necessary because the plugin has code to add generated source dirs to the compile classpath for main and test. If it is necessary, it would imply that the IDEs parse the xml but don't execute/instrument the maven build.

At this point we don't have any great answers, but I'd like to land this PR, or the similar idea discussed above with a new profile name.

[nikhilnanivadekar]

Does this mean that the release steps need to be updated to trigger the profile?

On Fri, Aug 9, 2024 at 4:54 PM Craig P. Motlin @.***> wrote:

@fipro78 https://github.com/fipro78 thank you for the thorough investigation. I looked around in YouTrack for an IntelliJ bug like what we're seeing and couldn't find anything that matches exactly.

The build-helper-maven-plugin config shouldn't be necessary because the plugin has code to add generated source dirs to the compile classpath for main https://github.com/eclipse/eclipse-collections/blob/master/eclipse-collections-code-generator-maven-plugin/src/main/java/org/eclipse/collections/codegenerator/maven/GenerateSourcesMojo.java#L51-L57 and test https://github.com/eclipse/eclipse-collections/blob/master/eclipse-collections-code-generator-maven-plugin/src/main/java/org/eclipse/collections/codegenerator/maven/GenerateTestSourcesMojo.java#L51-L56. If it is necessary, it would imply that the IDEs parse the xml but don't execute/instrument the maven build.

At this point we don't have any great answers, but I'd like to land this PR, or the similar idea discussed above with a new profile name.

— Reply to this email directly, view it on GitHub https://github.com/eclipse/eclipse-collections/pull/1665#issuecomment-2278890197, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACLY4XIWBLHKXKDBQB3TES3ZQVJFHAVCNFSM6AAAAABMGORN66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZYHA4TAMJZG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[fipro78]

@motlin Which XML file should be parsed by the IDEs?

At the end it is your project. I just gave my comment and advice.

I really think it is an issue related to mvn install which has always been a struggle for development. Have you tried the approach I described?

[motlin]

Does this mean that the release steps need to be updated to trigger the profile?

@nikhilnanivadekar,

The way I structured this PR, I moved the plugin configuration of bnd-maven-plugin into the existing maven profile called release-artifacts. I did this because I assume that this profile is activated by the release infrastructure and so no changes will be needed to the release steps.

Could you confirm whether the release steps run --activate-profiles release-artifacts or -DperformRelease=true or both?

[motlin]

You could try to do the mvn install only for the code generator plugin, which is needed to be installed. Then only call mvn clean verify to avoid that the API and IMPL modules get installed into the local repository, and see if that changes anything. That of course means you first need to clear your local Maven repository to avoid that existing artefacts are loaded from there.

I ran:

❯ rm -rf ~/.m2/repository/org/eclipse/collections/
❯ git fetch --all
❯ git checkout upstream/master

I clicked Play on a test or two and got the usual error messages.

java: package aQute.bnd.annotation.spi is not visible
  (package aQute.bnd.annotation.spi is declared in the unnamed module, but module org.eclipse.collections.api does not read it)

I think this is what you suggested to try, is that right? I think it IntelliJ only reads the target/ directories, not the .m2/ directory.

Which XML file should be parsed by the IDEs?

This is kind of an unimportant tangent, but the reason IntelliJ detects the generated sources directories is because it runs maven with instrumentation, and so it can detect when eclipse-collections-code-generator-maven-plugin adds more directories to the compile source roots.

Adding the build-helper-maven-plugin configuration is redundant for the maven build obviously, and it's also not necessary for IDEs as long as they run and instrument the maven build like IntelliJ does. If the build-helper-maven-plugin does help, then it implies the IDE parses the pom.xml files but doesn't execute them. That would be surprising to me, but I suppose it's possible.

[nikhilnanivadekar]

@motlin our release is broken up in three steps:

1. Create the release: This is for github only, checkout, tag, etc.
2. Deploy the jar: Build the jar and push to Sonatype
3. Deploy the p2: Build the p2 and push it to Eclipse

None of them activate the profile. Here are the command line parameters

Deploy :

-pl
'!jcstress-tests,!junit-trait-runner,!serialization-tests,!test-coverage,!unit-tests-java8,!unit-tests,!p2-repository,!p2-repository/org.eclipse.collections'
-DperformRelease=true
-Dsurefire.useFile=false
clean
install
javadoc:jar
eclipse-jarsigner:sign
deploy
-B

p2 :

-pl '!jmh-scala-tests,!jmh-tests,!junit-trait-runner,!scala-unit-tests,!serialization-tests,!test-coverage,!unit-tests-java8,!unit-tests'
clean
install

Please let me know which one needs to change

[motlin]

@nikhilnanivadekar

Thank you, I see the way profiles are activated. The command you pasted includes -DperformRelease=true and the release-artifacts profile is activated using this system property. This is great, it means I have flexibility to organize the profiles however we'd like without you needing to change anything on the release side. Don't merge this PR yet - I'm going to make the changes I suggested in https://github.com/eclipse/eclipse-collections/pull/1665#issuecomment-2275929371 where there's a second profile activated with the same system property.

        <profile>
            <id>release-artifacts</id>
            <activation>
                <property>
                    <name>performRelease</name>
                    <value>true</value>
                </property>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <groupId>org.eclipse.cbi.maven.plugins</groupId>
                        <artifactId>eclipse-jarsigner-plugin</artifactId>

[motlin]

To use this locally:

mvn clean verify -DperformRelease doesn't work, because it activates both profiles and will fail with an error about MAVEN_GPG_PASSPHRASE not being available.

mvn clean verify --activate-profiles bnd-maven-plugin works and activates the plugin. I can tell that it works, because it brings back the familiar errors inside IntelliJ that I was seeing before.

I think this is ready to merge!

[motlin]

Could we land this? I rebase all my commits on it while working locally, in order to not have to constantly perform clean builds.

@donraab @nikhilnanivadekar

[motlin]

LGTM. @motlin I am approving this in the interest of getting it closed out, with the understanding that if this accidentally breaks the release process, then we may need your assistance fixing it. Thanks! @nikhilnanivadekar

Surely. It should also be pretty easy to revert if I'm way off here.