Link the sources in the build properties instead of copy by ant

[laeubi]

Lets see how this works out...

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   6m 54s ⏱️ +14s
 4 100 tests ±0   4 092 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 212 runs  ±0  12 137 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit 623c8ce. ± Comparison against base commit 8dc46be.

♻️ This comment has been updated with latest results.

[merks]

What will happen when you do File -> Import... -> Plug-in Development -> Plug-ins and Fragments?

It's kind of disrupting to me to see paths that assume something about the underling filesystem structure of these things. That doesn't feel right to me. Even the structured editor would never allow one to create such a result...

[laeubi]

What will happen when you do File -> Import... -> Plug-in Development -> Plug-ins and Fragments?

I'm not sure I understand? build.properties is something read by build tools and currently the same is archived by running an ant task that copies all sources from the "common" swt to the current fragment.

It's kind of disrupting to me to see paths that assume something about the underling filesystem structure of these things.

SWT is/was always special in this regards and we try to transform it into something more usable/standard so I don't see the real issue compared to the old state that didn't worked quite standard anyways...

Even the structured editor would never allow one to create such a result...

??? I don't understand we don't have Ui for everything...

[laeubi]

By the way just to make sure if its unclear, this does not add anything "new" but accounts for the fact that these folders are already linked in the .project file, so this jsut replaces the build-time copy from ant by "native" maven/tycho.

[merks]

I mean this by the structured editor:

The structured editor suggests that only local content can be specified. In the workspace the things projects look "flat", so paths that navigate between projects based on their physical (and invisible) location in the file system seems strange...

[laeubi]

The structured editor suggests that only local content can be specified.

Well actually you can't specify anything in the UI as this is flagged as a "custom build" :-)

In the workspace the things projects look "flat",

Well yes it looks flat but as you can see in the tiny little blue icon on the folders they are linked resources

so paths that navigate between projects based on their physical (and invisible) location in the file system seems strange...

that actually link to "their physical (and invisible) location in the file system" :-)

[HannesWell]

I'm actually not convinced this is really beneficial.

Of course copying the sources is not the most elegant way to solve this, but it is working for the current master and even the API-checks about to be implemented for SWT in #1011 work since the sources are available in the src folder mentioned in the build.properties.
And I would rather keep this solution then having to maintain the source folders in seven more places.

And as mentioned in #1031 (comment) I see no real problems with the current approach.

Further I'm about to rework the fragment projects to be mainly based on maven and the bnd-maven-plugin in order to generate the fragments manifest.
This will also eventually allow to add an OSGi requirement from the host to a fragment.
Then the build.properties are probably obsolete any way.

[laeubi]

Then the build.properties are probably obsolete any way.

Then it won't be a problem to delete them but until then I want to get rid of as much ant / script /whatever magic when there is a standard solution and these pathes are not really "maintained" for years now so I don't see any issues with that.

Copy stuff around an making the build work different than in the IDE will just create surprise and making analyze things more complex.

[laeubi]

and even the API-checks about to be implemented for SWT in #1011 work since the sources are available in the src folder

It obviously does not discover the sources in all cases, I'm about to analyze that but API Checks are very sensitive to the project layout of Eclipse + Build matches and I don't want to get surprised by some magic (and avoidable) copy thing that make things break.

[laeubi]

Just to maybe make it more clear, currently you see this in the logs:

[WARNING] [API WARNING] File ImageLoaderListener.java at line 34: ImageLoaderListener extends non-API type SWTEventListener (location: /opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/Eclipse SWT/common/org/eclipse/swt/graphics/ImageLoaderListener.java)
[WARNING] [API WARNING] File ShellListener.java at line 35: ShellListener extends non-API type SWTEventListener (location: /opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/Eclipse SWT/common/org/eclipse/swt/events/ShellListener.java)
[WARNING] [API WARNING] File PaintListener.java at line 35: PaintListener extends non-API type SWTEventListener (location: /opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/Eclipse SWT/common/org/eclipse/swt/events/PaintListener.java)
[W...
[WARNING] 60 API problems can't be mapped to the compiler log!

so if you look more closeley you will see that API tools uses the "linked" path here:

location: /opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/Eclipse SWT/common/org/eclipse/swt/events/PaintListener.java

in the complie logs you will find that the copied path is used:

<argument value="/opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/src/org/eclipse/swt/events/PaintListener.java"/>

<source output="/opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/target/classes" package="org/eclipse/swt/events" path="/opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/src/org/eclipse/swt/events/PaintListener.java">
			<classfile path="/opt/eclipse/platform-sdk/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/target/classes/org/eclipse/swt/events/PaintListener.class"/>
</source>

so these two path do not match (from the API / compiler POV), so I would like them to match so I then probabbly can just use the full resolved location in the report and it then should fully work to match them!

[HannesWell]

so these two path do not match (from the API / compiler POV), so I would like them to match so I then probabbly can just use the full resolved location in the report and it then should fully work to match them!

OK, that and the potential abandoned src folder in the workspace after a local build are an advantage for me.
When I continue to rework the fragment projects to be Maven-first projects then the src-locations can be defined in a unified way in the binaries/pom.xml using profiles and the maven-build-helper-plugin:add-source goal to assemble the lists per platform only once.

I further removed now unused code in the CollectSources java script and removed the now unused ant-task. Furthermore I reviewed the source.. lists in the build.properties in detail and they all match the corresponding .classpath.

In general I think it would be nice to have some kind of macro/reference/variables ${project_classpath_sourceFolders} or alike that injects the paths of all src-entries in the .classpath file as list. This would especially be convenient for plugin-projects with multiple source folders.

Regarding Ed's concern that this setup is nothing one can create through the Editor:
That's right, but I agree with Christoph that this the SWT configuration is special already anyways. And at the same time there are no warnings, so nothing a developer should disturb.