Fix and document native build commands for all platforms#3200
Fix and document native build commands for all platforms#3200vogella merged 1 commit intoeclipse-platform:masterfrom
Conversation
Replace broken antrun:run@build-native-binaries command with the correct mvn clean install invocation, quote the -Dnative parameter, and add concise per-platform examples to both AGENTS.md and README.md.
153f48c to
db72074
Compare
HannesWell
left a comment
There was a problem hiding this comment.
I have to say that that this change is IMHO not a net improvement.
Please see my comments below.
Furthermore I'd appropriate if you would give others more time for a review or at leats tag those whose recent work you continue.
| ```bash | ||
| # Build the entire project | ||
| mvn clean verify | ||
| mvn clean install |
There was a problem hiding this comment.
| mvn clean install | |
| mvn clean verify |
Installing is IMO not the thing we should recommend by default, since it's modifing the user-wide Maven cache. And that's probably not always wanted.
Whoever wants to to that, can change the command accordingly.
For the AGENTS.md file this might not be that important, but we deficiently shouldn't do that in the README.
| Run from the repository root: | ||
|
|
||
| ```bash | ||
| # Windows (x86_64) | ||
| mvn clean install '-Dnative=win32.win32.x86_64' -DskipTests | ||
|
|
||
| # Linux (x86_64) | ||
| mvn clean install '-Dnative=gtk.linux.x86_64' -DskipTests | ||
|
|
||
| # macOS (x86_64 / aarch64) | ||
| mvn clean install '-Dnative=cocoa.macosx.x86_64' -DskipTests | ||
| mvn clean install '-Dnative=cocoa.macosx.aarch64' -DskipTests | ||
| ``` |
There was a problem hiding this comment.
The content should not be here, but instead this paragraph should reference https://github.com/eclipse-platform/eclipse.platform.swt/tree/master/bundles/org.eclipse.swt#building-and-testing-locally.
The commands are implemented in the launch configuration mentioned there.
Therefore I don't know if we should replicate this again, but if you think it should be replicated, there would be the best place.
| ``` | ||
| cd binaries/org.eclipse.swt.<ws>.<os>.<arch> | ||
| mvn clean antrun:run@build-native-binaries -Dnative=<ws>.<os>.<arch> | ||
| ```bash |
There was a problem hiding this comment.
The following commands are not bash specific, you can also run them in a Windows CMD or Powershell.
| Run from the repository root, specifying the target platform: | ||
|
|
||
| To build only the native binaries, run | ||
| ``` | ||
| cd binaries/org.eclipse.swt.<ws>.<os>.<arch> | ||
| mvn clean antrun:run@build-native-binaries -Dnative=<ws>.<os>.<arch> | ||
| ```bash | ||
| # Windows (x86_64) | ||
| mvn clean install '-Dnative=win32.win32.x86_64' -DskipTests | ||
|
|
||
| # Linux (x86_64) | ||
| mvn clean install '-Dnative=gtk.linux.x86_64' -DskipTests | ||
|
|
||
| # macOS (x86_64 / aarch64) | ||
| mvn clean install '-Dnative=cocoa.macosx.x86_64' -DskipTests | ||
| mvn clean install '-Dnative=cocoa.macosx.aarch64' -DskipTests | ||
| ``` |
There was a problem hiding this comment.
The commands now do something almost completely different and they don't match the section topic anymore.
Furthermore if the abstract syntax is to hard to understand, I suggest to add one specific example instead of just listing specificly only a subset of the supported/targeted platforms.
|
I think this is an example of standing in front of oncoming freeway traffic. There's just so much pushed through so rapidly there’s not enough capacity for adequate review. |
|
Absolutely ok for me if you revert that change. It took me a longer time to figure out how to compile the win natives but I have documented that for me and I'm fine if you restore the original content. |
It's totally reasonable to further enhance the documentation and especially to make it more discoverable. The native build instructions are currently indeed difficult to discover. But IMO the solution is not to duplicate the doc more, but instead to add more references to ideally one single source of truth. Please have a look and let me know what you think or what should be further improved. And regarding the process, I don't think we should record the discussion about changes in the history of the master branch in git and everybody should just submit his/her proposal and others revert it if they don't like it. |
Replace broken antrun:run@build-native-binaries command with the correct mvn clean install invocation, quote the -Dnative parameter, and add concise per-platform examples to both AGENTS.md and README.md.