[Documentation] Update instructions to build SWT binaries locally

[fedejeanne]

Contributes to #1171

See #1167 (comment)

Move some sections of the documentation to the general Readme.md file and change some outdated/unclear documentation in Windows.

FYI I provided 2 commits for clarity but I will squash them before merging.

[fedejeanne]

cc @akurtakov , @HannesWell

[github-actions]

Test Results

   298 files   -     1    298 suites   - 1   1h 8m 30s ⏱️ + 1h 2m 17s
 4 101 tests ±    0    161 ✅  - 3 932   6 💤  -  2  0 ❌ ±0  3 934 🔥 +3 934 
15 953 runs  +3 738  8 220 ✅  - 3 920  61 💤  - 14  0 ❌ ±0  7 672 🔥 +7 672 

For more details on these errors, see this check.

Results for commit 74bda8a. ± Comparison against base commit 979d3f1.

♻️ This comment has been updated with latest results.

[HannesWell]

Thank you @fedejeanne for this.
I have added some comments below how this could be further enhanced.

[fedejeanne]

Thanks for the tips, @HannesWell , I changed some stuff based on them, WDYT?

I can squash all my commits before merging.

[HannesWell]

I just had another pass over the documentation and since there were many small points to further enhance (mainly in the already existing doc, but it IMHO fits into this change) I just applied the suggestions by myself and pushed them in a separate commit.
Please incorporate them as you see fit.
A short overview is

• Removed outdated information about Hudson builds.
• Remove links and mentioning of specific JDK vendors. From my impression there is not only one source for JDK binaries anymore and I assume that everyone that wants to contribute to SWT has already set up a JDK anyways.
  If we want to provide a link, I suggest to use a neutral place like:
  https://adoptium.net/marketplace/?version=17
• There is no help for Windows and Mac so I removed its mentioning.

Thank you for these updates. They are definitely due.

[HeikoKlare]

If I am not mistaken, the descriptions how to build the binaries by hand (directly executing the build.sh / build.bat scripts), do not work as described. I would propose to completely remove these descriptions, as there is a Maven configuration that does everything required and replication all the information in a documentation leads to unnecessary duplications that needs to be kept consistent.

[HeikoKlare]

In the requirements, I would update the reference to GTK:

• gtk3-devel is the Fedora package name, on Ubuntu/Debian it is libgtk-3-dev
• We could/should mention GTK 4 as well
• We could simply refer to the GTK project installation page for the required (an always up-to-date) information: https://www.gtk.org/docs/installations/linux

[HeikoKlare]

Does not work for the same reasons as described for Linux (except the OUTPUT_DIR problem).

[HannesWell]

Since it is very easy to create the binaries via Maven, I am not sure whether it is worth to describe all that manual setup (and keep it up to date), as that is already done by Maven (and thus documented in the Maven configuration).

Just as a quick-reply for now:
Yes, I agree. But to make it simpler to find the relevant part in the pom I suggest to also add a perma-link to the configuration of the corresponding antrun-mojo configuration.
Of course this might also change in the future, but if it is forgotten to update the link (which actually can only be done after the fact since it contains the commit Id :/), then it is at least easier to find to up-to-date info.

[fedejeanne]

@HannesWell , @HeikoKlare , thank you for the comments, I addressed almost all of them in d556545 and made some other minor changes. I am also in favor of simplifying these instructions as much as possible and let the "hackers" figure out the alternative ways (in case they don't want to run Maven) on their own.

I'm positive that the current state of these instructions is good enough to compile the binaries in the majority of the setups.

Once this PR is good to go, I will squash the commits.

@HannesWell We can add a permanent link to the desired commit after merging this PR if you want to. Just to be sure: you want to link this line (but not in master), right?

eclipse.platform.swt/binaries/pom.xml

Line 93 in 979d3f1

<id>build-native-binaries</id>

How would you phrase it in the documentation (in the follow-up PR)?

[fedejeanne]

The failing tests are unrelated

[SyntevoAlex]

Sorry, but inverse changes are actually needed.

You are removing steps for manual building, but that's exactly what I was always doing, and I was pretty much committer number one for a few years.

I was running steps manually because I'm not using Eclipse for developing SWT. And when I run maven from IDEA, it downloads unneeded stuff (tycho etc) for some 20 minutes before actually building.

[HeikoKlare]

You are removing steps for manual building, but that's exactly what I was always doing, and I was pretty much committer number one for a few years.

There seems to be some confusion here, as the proposed change removes nothing but redundant and outdated documentation. You can still do a "manual build" (i.e., do by hand what Maven does automatically). Having an explanation of that process in the documentation would be nothing else than writing down the according Maven step in natural language:

eclipse.platform.swt/binaries/pom.xml

Lines 92 to 134 in 979d3f1

	<execution>
	<id>build-native-binaries</id>
	<phase>process-resources</phase>
	<goals>
	<goal>run</goal>
	</goals>
	<configuration>
	<target>
	<!-- See https://stackoverflow.com/a/53227117 and http://ant-contrib.sourceforge.net/tasks/tasks/index.html -->
	<taskdef resource="net/sf/antcontrib/antlib.xml"/>
	<if><equals arg1="${native}" arg2="${ws}.${os}.${arch}"/>
	<then>
	<property name="build_dir" value="${project.build.directory}/natives-build-temp"/>
	<exec executable="${java.home}/bin/java" dir="${swtMainProject}" failonerror="true">
	<arg line="-Dws=${ws} -Darch=${arch} build-scripts/CollectSources.java -nativeSources '${build_dir}'"/>
	</exec>
	<property name="SWT_JAVA_HOME" value="${java.home}"/><!-- Not overwritten if already set, e.g. as CLI argument -->
	<echo>Compile SWT natives against headers and libraries from JDK: ${SWT_JAVA_HOME}</echo>
	<if><equals arg1="${ws}" arg2="win32" />
	<then>
	<exec dir="${build_dir}" executable="${build_dir}/build.bat" failonerror="true">
	<env key="SWT_JAVA_HOME" value="${SWT_JAVA_HOME}"/>
	<env key="OUTPUT_DIR" value="${project.basedir}"/>
	<arg line="install clean"/>
	</exec>
	</then>
	<else>
	<property name="gtk_version" value="3.0" />
	<exec dir="${build_dir}" executable="sh" failonerror="true">
	<arg line="build.sh"/>
	<env key="SWT_JAVA_HOME" value="${SWT_JAVA_HOME}"/>
	<env key="OUTPUT_DIR" value="${project.basedir}"/>
	<env key="GTK_VERSION" value="${gtk_version}"/>
	<env key="MODEL" value="${arch}"/>
	<arg line="install clean"/>
	</exec>
	</else>
	</if>
	</then>
	</if>
	</target>
	</configuration>
	</execution>

So as @HannesWell proposed (#1174 (comment)), adding a link to the Maven configuration in the documentation for those who want to perform the steps by hand would be good.

[SyntevoAlex]

I still think that instead of removing documentation, it should be fixed, so removing is half step in wrong direction. It is not redundant, because parsing maven files is hard for people who never dealt with maven before (such as me).

I'm merely commenting and do not intend to hold this PR.

[merks]

I think the point is that while the actual logic as spelled out in the pom.xml may evolve in the future (logic that is well tested by the builds and by the other developers) the documentation will forever need to evolve manually in lockstep. That's likely to be forgotten or not to happen. I image that most people will be happy if they can invoke Maven without further thought about the details, and even if that takes 20 minutes the first time to fill the cache, that's probably for most people better tradeoff than trying to read and to follow more detailed documentation instructions.

So I do understand your concerns, I think this is a little like with Oomph where it's must easier to maintain something that's regularly and automatically used, than it is to maintain documentation that's rarely read, rarely used, and error prone to follow exactly to the letter...

[fedejeanne]

@SyntevoAlex I agree with @HannesWell , @HeikoKlare and @merks here: pointing to the Maven goal that does the trick (in a follow-up PR) is enough here so I wouldn't hold this PR just to add the "manual" steps which are likely to be outdated in the near future anyway.

So as soon as @HeikoKlare and @HannesWell give me the green light (I hope I addressed every one of your comments), I'm squashing + merging :-)

[HeikoKlare]

I still think that instead of removing documentation, it should be fixed, so removing is half step in wrong direction. It is not redundant, because parsing maven files is hard for people who never dealt with maven before (such as me).

I understand your point that it is harder to parse Maven files than reading documentation that explains the Maven files in comparably simple language (even though that does not change anything w.r.t. it being redundant). However, maintaining the documentation is even harder, as only very few people will need that documentation (as stated by @merks). In my opinion, it even leads to the opposite: the people that may be interested in compiling the natives will run across all the information on how to execute the build.sh/build.bat and be tempted to follow that approach instead of simply executing the Maven build. So the documentation would have to be structured very well to lead the people in the proper direction, i.e., to just use the simple, Maven-based solution and understand that all the information about the manual build is irrelevant for them.

[fedejeanne]

Addressed #1174 (comment) and also added the perma-link from #1174 (comment) in 74bda8a

[HeikoKlare]

Looks good to me. Thank you for updating and cleaning up the documentation!

[HannesWell]

Hoping that it will be a good compromise I made more suggestions for improvements below.

[HannesWell]

Suggested change

	For more details see the co-located platform specific Readme files:
	- [Readme Linux](Readme.Linux.md)
	- [Readme macOS](Readme.macOS.md)
	- [Readme Windows](Readme.Win32.md)

This does not provide details anymore.

[fedejeanne]

Sealed and delivered. Thank you all for participating!