Skip to content

Don't generate a source bundle with only an about.html#1772

Merged
merks merged 1 commit intoeclipse-pde:masterfrom
merks:pr-avoid-empty-source-bundle
May 16, 2025
Merged

Don't generate a source bundle with only an about.html#1772
merks merged 1 commit intoeclipse-pde:masterfrom
merks:pr-avoid-empty-source-bundle

Conversation

@merks
Copy link
Copy Markdown
Contributor

@merks merks commented May 9, 2025

No description provided.

@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 9, 2025

The source bundle contains only an about.html so effectively empty/useless.

image

@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 9, 2025

Actually I'm not really sure this is the right way.

Other places seem to set the property to not generate, but even that seems insufficient and so there also appears to be a need to configure the pom.xml to exclude it:

image

I.e., it appears to be necessary to add org.eclipse.core.filesystem.linux.aarch64 to the pom configuration.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2025

Test Results

   285 files  ±0     285 suites  ±0   51m 34s ⏱️ - 1m 46s
 3 611 tests ±0   3 535 ✅ ±0   76 💤 ±0  0 ❌ ±0 
11 025 runs  ±0  10 794 ✅ ±0  231 💤 ±0  0 ❌ ±0 

Results for commit 6bd6975. ± Comparison against base commit 1121eb8.

♻️ This comment has been updated with latest results.

@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 9, 2025

This is related:

eclipse-platform/eclipse.platform.releng.aggregator#3038

For the record, these are all bundles that I noticed have source bundle that don't actually contain any useful sources:

pde/org.eclipse.pde/3.13.3100/org.eclipse.pde-3.13.3100.jar
platform/org.eclipse.core.filesystem.linux.aarch64/1.4.200/org.eclipse.core.filesystem.linux.aarch64-1.4.200.jar
platform/org.eclipse.equinox.console.jaas.fragment/1.2.0/org.eclipse.equinox.console.jaas.fragment-1.2.0.jar
platform/org.eclipse.osgi.util/3.7.300/org.eclipse.osgi.util-3.7.300.jar
platform/org.eclipse.swt.tools.base/3.107.700/org.eclipse.swt.tools.base-3.107.700.jar
platform/org.eclipse.ui.themes/1.2.2700/org.eclipse.ui.themes-1.2.2700.jar

@mickaelistria
Copy link
Copy Markdown
Contributor

Could Tycho be made smarter and not generate source bundles if it identifies no .class file was generated?

@laeubi
Copy link
Copy Markdown
Contributor

laeubi commented May 9, 2025

Could Tycho be made smarter and not generate source bundles if it identifies no .class file was generated?

A bundles does not need to contain classes to have sources, e.g. html, images, ... so its a bit a matter of definition and in the past some checks in platform even complained when there is no source bundle at all.

I think instead of trying to be smart, it would be easier to add a skip option to https://tycho.eclipseprojects.io/doc/latest/tycho-source-plugin/plugin-source-mojo.html that the can be set in the build.properties with pomless options.

The second part is about source features what will exclude it from the feature, but as we use includeAllSources since a while it is mostly irrelevant for inclusion of such things.

@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 9, 2025

Please advise on the correct/best approach. I'd simply like to eliminate pointless noise...

(Of course a source bundle can have an arbitrary content, but the general purpose is to provide sources of corresponding .class files for which there is no corresponding content in the binary bundle, not to duplicate the binary content or simply to exist with no useful information at all.)

@laeubi
Copy link
Copy Markdown
Contributor

laeubi commented May 9, 2025

If i where to solve it I would go to OsgiSourceMojo and add a new @Parameter with name skip and property tycho-plugin-source.skip, then one can add pom.model.property.tycho-plugin-source.skip=true to any build.properties file and the generation would be skipped.

Of course one would add an integration-test to demonstrate use-case and probably find then that other places are not happy when we simply not generate it in the first place :-)

@merks merks marked this pull request as draft May 10, 2025 05:34
@HannesWell
Copy link
Copy Markdown
Member

If i where to solve it I would go to OsgiSourceMojo and add a new @Parameter with name skip and property tycho-plugin-source.skip

Created a corresponding PR at

@HannesWell
Copy link
Copy Markdown
Member

HannesWell commented May 14, 2025

If i where to solve it I would go to OsgiSourceMojo and add a new @Parameter with name skip and property tycho-plugin-source.skip

Created a corresponding PR at

* [Add flag to skip execution of tycho-source-plugin eclipse-tycho/tycho#4976](https://github.com/eclipse-tycho/tycho/pull/4976)

@merks the required change in Tycho is now available, also as backport to Tycho 4.0.13-SNAPSHOT. Do you want to continue this?

@merks merks force-pushed the pr-avoid-empty-source-bundle branch from 8834098 to 431b274 Compare May 15, 2025 05:15
@merks merks marked this pull request as ready for review May 15, 2025 05:15
@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 15, 2025

The build seems to hang with this failure which looks unrelated to this change.

 Results:
Failures: 
  UpdateUnitVersionsCommandTests.testVersionSort:55->confirmVersionUpdates:93 ID: org.eclipse.fake.3 has the incorrect version. updatedText=<target>
<locations>
<location>
<unit id="org.eclipse.fake.1" version="0.0.0"/>
<unit id="org.eclipse.fake.2" version="0.0.0"/>
<unit id="org.eclipse.fake.3" version="0.0.0"/>
<unit id="org.eclipse.fake.4" version="0.0.0"/>
<unit id="org.eclipse.fake.5" version="0.0.0"/>
<unit id="org.eclipse.fake.6" version="0.0.0"/>
<unit id="org.eclipse.fake.7" version="0.0.0"/>
<repository location="bundleentry://358.fwk1077351082/testing-files/testing-sites/MultipleUnitsConfirmSorting/"/>
</location>
</locations>
</target> expected:<[1.1.2]> but was:<[0.0.0]>
Tests run: 29, Failures: 1, Errors: 0, Skipped: 0

@HannesWell @laeubi

Is this the change you are expecting?

I see this in the log

07:16:03  [INFO] --- tycho-source:4.0.13-SNAPSHOT:plugin-source (plugin-source) @ eclipse.pde ---
07:16:03  [INFO] 
07:16:03  [INFO] --- javadoc:3.11.2:jar (attach-javadocs) @ eclipse.pde ---
07:16:03  [INFO] Not executing Javadoc as the project is not a Java classpath-capable package
07:16:03  [INFO] 

Does that tell us anything useful?

I think not because in https://ci.eclipse.org/pde/job/eclipse.pde/job/PR-1772/lastSuccessfulBuild/artifact/repository/target/repository/plugins/ I see this

image

What might I be doing incorrectly?

Comment thread ui/org.eclipse.pde/build.properties Outdated
@merks merks force-pushed the pr-avoid-empty-source-bundle branch from 431b274 to 2a9061d Compare May 15, 2025 09:12
@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 15, 2025

It's still there: 😱

image

@laeubi
Copy link
Copy Markdown
Contributor

laeubi commented May 15, 2025

You should see that the source mojo prints a message that execution was skipped. But if you don't increment the version it might maybe restored from baseline.

@merks merks force-pushed the pr-avoid-empty-source-bundle branch 2 times, most recently from 15f425d to bd1b3a6 Compare May 15, 2025 14:23
@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 15, 2025

Still there:

image

But now I realize this thing has a pom and I bet the property only works on a pomless bundle and here I need to configure this in the pom.xml. Or? How best to do that?

@laeubi
Copy link
Copy Markdown
Contributor

laeubi commented May 15, 2025

Yes if there is a pom (didn't realized that) you need to configure it in the pom in the build section.

@HannesWell
Copy link
Copy Markdown
Member

Yes if there is a pom (didn't realized that) you need to configure it in the pom in the build section.

Wouldn't it be sufficient to define it as property in the properties section? I.e.

  <properties>
    <tycho.source.skip>true</tycho.source.skip>
  </properties>

@merks merks force-pushed the pr-avoid-empty-source-bundle branch from bd1b3a6 to 6bd6975 Compare May 16, 2025 05:42
@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 16, 2025

It's really gone this time:

image

@merks merks merged commit 1eacb11 into eclipse-pde:master May 16, 2025
19 checks passed
@merks merks deleted the pr-avoid-empty-source-bundle branch May 16, 2025 06:43
@merks
Copy link
Copy Markdown
Contributor Author

merks commented May 16, 2025

To make sure this doesn't cause downstream problems and correctly produces the expect result in the final p2 repository I kicked off this I-build:

https://ci.eclipse.org/releng/job/Builds/job/I-build-4.36/60/

Then I can apply this solution to the other bundle mentioned in #1772 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants