Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge SWT native binaries into this repository stored in Git LFS #956

Merged

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Jan 2, 2024

Configure this Git-repository to store the SWT native binaries, i.e. all *.dll, *.so and *jnilib files, using Git's Large File Storage (LFS) and move the platform-specific SWT fragments from the
eclipse.platform.swt.binaries repository into the binaries folder in this repository.

The platform specific projects are (minimally adjusted) copies of the corresponding projects from
https://github.com/eclipse-platform/eclipse.platform.swt.binaries/tree/9cdcee51a79a3f72e10e074b87326f80d3d6ada2

Only leave out the 'forceQualifier.txt' since the main bundle only drives the qualifier.

Fixes eclipse-platform/eclipse.platform.swt.binaries#2

Pending tasks before this can be submitted:

Copy link
Contributor

github-actions bot commented Jan 2, 2024

Test Results

   99 files   -   200     99 suites   - 200   1m 59s ⏱️ - 3m 30s
4 079 tests  -    19  4 029 ✅  -    61  49 💤 +41  1 ❌ +1 
4 079 runs   - 8 127  4 029 ✅  - 8 104  49 💤  - 24  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 1e83a48. ± Comparison against base commit 1c2e820.

This pull request removes 19 tests.
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letters
…
This pull request skips 42 tests.
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CCombo ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CLabel ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_StyledText ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Button ‑ test_setForegroundAlphaCheckButton
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Button ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Button ‑ test_setForegroundAlphaRadiokButton
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Canvas ‑ test_setForegroundAlphaLorg_eclipse_swt_graphics_Color
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_widgets_Combo ‑ test_setForegroundAlphaDropDownCombo
…

♻️ This comment has been updated with latest results.

@HannesWell HannesWell force-pushed the store-binaries-in-lfs branch 5 times, most recently from de9d63c to 7ebf16a Compare January 3, 2024 12:38
@Phillipus
Copy link
Contributor

Phillipus commented Jan 3, 2024

Hi, I'm trying to test this by running a child Eclipse instance but something is missing.

Usually I import the project at eclipse.platform.swt/bundles/org.eclipse.swt, set the right .classpath file according to my OS and it finds the binaries in the eclipse.platform.swt.binaries project.

With this change it's not finding the binaries folder:

Is there something else I need to do?

EDIT: sorry for the noise, I needed to also import the binary project. At least I can report that this works!

@HannesWell
Copy link
Member Author

EDIT: sorry for the noise, I needed to also import the binary project. At least I can report that this works!

No problem and thanks for trying this out. I probably have to further adjust the Oomph setup as well and have to check again the move of binary content in the workspace.
One important part is also to install the jgit.lfs plugin (which the setup should do already).

In general there is a lot to adjust, but first I'm focusing on the build.

@HannesWell HannesWell force-pushed the store-binaries-in-lfs branch 5 times, most recently from 7019c30 to a863170 Compare January 3, 2024 20:14
@HannesWell
Copy link
Member Author

After a few more adjustments of the Jenkins pipeline the verification build is now passing.
I'm about to test with a separate test-branch if pushing also works as expected.

I have also looked into the test failure in the GH workflows and it looks like only org.eclipse.swt.tests.junit.Test_org_eclipse_swt_SWT.test_isLocal is failing although the signature actually looks ok. But I'll investigate that further.

Furthermore I have now adjusted the Oomph setup to work well with the binaries in this repository. My tests with an (almost) cleared workspace and git repo worked well.

I have now also requested the EF-IT team to install git-lfs on the affected Jenkins instance so that my current work-around to download and install it each time is not necessary anymore:
https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/4119

Furthermore I created a list of task that have to or at least should be prepared before this is submitted in my opening comment.

@HannesWell
Copy link
Member Author

I have also looked into the test failure in the GH workflows and it looks like only org.eclipse.swt.tests.junit.Test_org_eclipse_swt_SWT.test_isLocal is failing although the signature actually looks ok. But I'll investigate that further.

These failures now magically vanished.

After a few more adjustments of the Jenkins pipeline the verification build is now passing. I'm about to test with a separate test-branch if pushing also works as expected.

Pushing worked like a charm and thanks to the EF-Infra team we even already have git-lfs installed on all centOS images, so technically all prerequisites are completed and I will continue to work off the TO-DO list in my initial comment.

@HannesWell HannesWell force-pushed the store-binaries-in-lfs branch 3 times, most recently from de2540f to da53083 Compare January 6, 2024 11:41
HannesWell added a commit to HannesWell/eclipse.platform.releng.aggregator that referenced this pull request Jan 6, 2024
With eclipse-platform/eclipse.platform.swt#956
the swt native binaries are stored in the eclipse.platform.swt
repository using git-lfs. The eclipse.platform.swt.binaries repository
is therefore obsolete.

Part of eclipse-platform/eclipse.platform.swt.binaries#2
@Phillipus
Copy link
Contributor

Phillipus commented Jan 6, 2024

Some user feedback (please let me know if not needed)...

I tested the test-lfs-pushing branch:

  1. Initialised LFS on my clone of eclipse.platform.swt repo with git lfs install --local
  2. Checked out the test-lfs-pushing branch
  3. All binary files in the eclipse.platform.swt/binaries folder were correct (i.e not SHA pointers)
  4. I removed the old eclipse.platform.swt.binaries/bundles/org.eclipse.swt.cocoa.macosx.x86_64 project from my workspace
  5. I imported the new eclipse.platform.swt/binaries/org.eclipse.swt.cocoa.macosx.x86_64 project into my workspace
  6. I successfully ran a child Eclipse instance, and my RCP app

The only minor inconvenience is having to do stages 4 and 5 each time when switching between the old and new binaries. This is necessary because the binary project names are the same, and you cannot import the new one without first removing the old one from the workspace.

@Phillipus
Copy link
Contributor

Phillipus commented Jan 6, 2024

The only minor inconvenience is having to do stages 4 and 5 each time when switching between the old and new binaries. This is necessary because the binary project names are the same, and you cannot import the new one without first removing the old one from the workspace.

A workaround is to change the project name in the .project file for the old binary project in eclipse.platform.swt.binaries.

For my testing on Mac I renamed the project to org.eclipse.swt.cocoa.macosx.x86_64_Old. This way I could have both the old and new binary projects present in my workspace, and I only had to close one or the other, not actually remove/add it from the workspace each time.

@HannesWell HannesWell force-pushed the store-binaries-in-lfs branch 2 times, most recently from 9843760 to fc6fe52 Compare January 7, 2024 10:36
@HannesWell
Copy link
Member Author

Some user feedback (please let me know if not needed)...

Constructive feedback is always welcome and I'm very glad that another person also verifies everything keeps working. :)

I tested the test-lfs-pushing branch:

1. Initialised LFS on my clone of `eclipse.platform.swt` repo with `git lfs install --local`

It is good to verify that it also works with git-cli, do you know if many SWT devs work with git from the CLI? If so I think it should be mentioned in the CONTRIBUTING guide that git-lfs is required (altough git for windows has it built in, but it still needs to be installed).

Where I personally put more effort into verifying that LFS also works with EGit/JGit from inside Eclipse.
If EGit is used, all that needs to be done is to install org.eclipse.jgit.lfs and to enable Git LFS support globally or only for the swt repos as described in https://github.com/eclipse-egit/egit/wiki/User-Guide#git-lfs-support
For the maximum convenience one can configure Eclipse to automatically make sure the LFS support is enabled whenever Eclipse starts up. by enabling in the Preferences under Version Control (Team) -> Git -> Confirmations and Warnings -> Automatically configure LFS...". This is what I have done in SWT's Oomph setup in this commit.

So everybody that uses the Oomph just has to run the setup after changing to the latest master once this is merged and restart and then the next fetch should also do an lfs pull.

The only minor inconvenience is having to do stages 4 and 5 each time when switching between the old and new binaries. This is necessary because the binary project names are the same, and you cannot import the new one without first removing the old one from the workspace.

A workaround is to change the project name in the .project file for the old binary project in eclipse.platform.swt.binaries.

For my testing on Mac I renamed the project to org.eclipse.swt.cocoa.macosx.x86_64_Old. This way I could have both the old and new binary projects present in my workspace, and I only had to close one or the other, not actually remove/add it from the workspace each time.

That's indeed an inconvenience. Renaming the project's sounds like a good workaround that those that cannot simply rebase their current work on the latest master can apply.
But since I will also prepare a PR for swt.binaries to clear the repo on the master, like it was for example done for https://github.com/eclipse-platform/eclipse.platform.runtime, I think it is not suitable to push that workaround to swt.binaries.
My suggestion would simply be to rebase pending work on the latest master once this is merged. So the transition only needs to be done once. And once the mentioned clear of swt.binaries happend the imported native project will close itself any way automatically and one can just delete it. Unfortunately, in this case, the project is only closed if removed from the originating SCM.

@Phillipus
Copy link
Contributor

@HannesWell Thanks for your feedback on my feedback. I'm continuing to test, but now on your PR branch rather than test-lfs-pushing

It is good to verify that it also works with git-cli, do you know if many SWT devs work with git from the CLI?

I actually use the commercial SmartGit app for Eclipse repo git management instead of EGit, but I used the CLI to initialise LFS in this case so as to document how to do it in case other people need to know. I don't use Oomph either, preferring the basic vanilla installation of Eclipse (old habits die hard, Eclipse dev since 2003).

Renaming the project's sounds like a good workaround that those that cannot simply rebase their current work on the latest master can apply.

I think the main use case for this is when you want to test an older SWT commit pre-LFS.

@HannesWell
Copy link
Member Author

There are about 20 existing PRs in this repo. Should owners be informed that should rebase them on the post-LFS commit when this is done? Otherwise checking out an old PR will require the old binaries project.

I have already prepared a mail to the platform-dev mailing list informing about this change that also includes the necessary steps to get things working again (with EGit and CLI-Git).
So my suggestion would be to get this landed and fully working as soon as possible and then send out the mail. PRs that are not based on the current master should still work assuming the Jenkins pipeline is executed in the state it was at the PR's commit and since the swt.binaries repo still exists this should still work.
Alternatively we could probably update most of the PRs through the GH web-interface.

@HannesWell
Copy link
Member Author

HannesWell commented Jan 8, 2024

The last build of eclipse-platform/eclipse.platform.releng.aggregator#1700 confirmed again with the latest state of the testing branch of this PR that everything works as expected and since then I just rebased this branch.

This PR is now in the state that can be submitted (i.e. all testing changes are removed).

I would prefer that all SDK tests on all platforms would complete before we switch the SWT build, to have clear baseline we can compare with. Unfortunately we don't have one because the SDK build before was not stable, and we've missed two previous builds too due outage.

The macos-arm build machine will complete its currently running I-build test-job in about 15/20min. So for the I-builds we would have a clear baseline.

There are currently just one (or maybe two, I'm not sure) waiting P/Y build jobs in https://ci.eclipse.org/releng/:
grafik

I would just cancel them now and restart once this PR's build has passed. I assume that P/Y builds are not that urgent are they?

@iloveeclipse
Copy link
Member

I would just cancel them now and restart once this PR's build has passed. I assume that P/Y builds are not that urgent are they?

I think so, so please cancel.

@HannesWell
Copy link
Member Author

I would just cancel them now and restart once this PR's build has passed. I assume that P/Y builds are not that urgent are they?

I think so, so please cancel.

Just canceled both waiting jobs YPBuilds/job/ep431Y-unit-mac64-java17 and YPBuilds/job/ep431Y-unit-macM1-java17 after verifying that they both run on the needed nc1ht-macos11-arm64 machine.

Unfortunately I cannot re-trigger them since the jobs did not even start and Jenkins (probably therefore) didn't record them. But YPBuilds/job/Y-build-4.31/ will run again tomorrow 16:00UTC and trigger that again. If it is really urgent I can also manually trigger these builds with parameter buildID=Y20240108-1000.

@HannesWell
Copy link
Member Author

OK, the I-build mac-arm tests completed.
I'm going to submit this now, will then submit the platform.releng.aggr changes once the native binaries are re-build and will then trigger another I-build (I would rather get the failures at 8/9 in the evening than at midnight).
I'll keep you updated here.

Configure this Git-repository to store the SWT native binaries, i.e. all
*.so, *.dll and *jnilib files, using Git's Large File Storage (LFS) and
move the platform-specific SWT fragments from the
eclipse.platform.swt.binaries repository into the 'binaries' folder in
this repository.

The platform specific projects are copies (with only the adjustments
necessary due to the move) of the corresponding projects from
https://github.com/eclipse-platform/eclipse.platform.swt.binaries/tree/e58be4cbe6e638e5a190d019643bb11db49c3ade

The native binary files are not copied because the build triggered by
this commit will re-build and add them anyways.
Additionally the 'forceQualifier.txt' from the native fragments are also
left out since the main bundle only drives the qualifier.

The commit procedure when the binaries are re-build is changed to commit
the binary and pom changes in one commit.

Fixes eclipse-platform/eclipse.platform.swt.binaries#2
@HannesWell HannesWell merged commit d86d39b into eclipse-platform:master Jan 8, 2024
3 of 8 checks passed
@HannesWell HannesWell changed the title Move SWT native binaries in this repository stored in Git LFS Move SWT native binaries into this repository stored in Git LFS Jan 8, 2024
@HannesWell HannesWell changed the title Move SWT native binaries into this repository stored in Git LFS Merge SWT native binaries into this repository stored in Git LFS Jan 8, 2024
@HannesWell HannesWell deleted the store-binaries-in-lfs branch January 8, 2024 17:58
@HannesWell
Copy link
Member Author

The master branch build has completed successfully and the binaries were (re-)build and pushed to the master branch with commit bb3af98.
All GH-workflows are now passing again as well ✔️.

The PR eclipse-platform/eclipse.platform.releng.aggregator#1700 is now ready and I will submit it as soon as the build has succeeded.

HannesWell added a commit to eclipse-platform/eclipse.platform.releng.aggregator that referenced this pull request Jan 8, 2024
With eclipse-platform/eclipse.platform.swt#956
the swt native binaries are stored in the eclipse.platform.swt
repository using git-lfs. The eclipse.platform.swt.binaries repository
is therefore obsolete.

Part of eclipse-platform/eclipse.platform.swt.binaries#2
@HannesWell
Copy link
Member Author

eclipse-platform/eclipse.platform.releng.aggregator#1700 has been merged and I just triggered I-build-4.31#77.

@Phillipus
Copy link
Contributor

I'm trying to check out the latest master and I'm getting an error:

git-lfs filter-process
Error downloading object: binaries/org.eclipse.swt.cocoa.macosx.aarch64/libswt-awt-cocoa-4964r5.jnilib (6f0b5d7): Smudge error: Error downloading binaries/org.eclipse.swt.cocoa.macosx.aarch64/libswt-awt-cocoa-4964r5.jnilib (6f0b5d7ece96b0a8dc73cc7770b28937ff4c5093b562893d01f553c981125c4c): batch request: missing protocol: ""

I've installed LFS already. Are the binaries actually available in LFS?

@HannesWell
Copy link
Member Author

I've installed LFS already. Are the binaries actually available in LFS?

That's strange.
Yes the binaries are actually available. They have been pulled successfully by the eclipse.platform.releng.aggregator build and I-build. Fetching them in my Eclipse worked for this repository and the eclipse.platform.releng.aggregator repository where the lfs objects were pulled for the swt sub-module worked and a fresh clone using CLI-Git (git clone https://github.com/eclipse-platform/eclipse.platform.swt.git) worked as well.
Does a fresh clone works for you?

@iloveeclipse
Copy link
Member

@HannesWell : assuming everything works (SDK build is done soon), could you share steps needed for existing local repositories to "properly" update from master.

If you mean, egit doesn't work, what command is needed, which (if any) config need to be changed etc.

@Phillipus
Copy link
Contributor

Does a fresh clone works for you?

It was one of those weird things where it was working on one machine but not this particular one. The error occurred when switching commits between pre and post LFS. Then, after lots of re-cloning, re-pulling, etc etc. the problem vanished. Who knows?

But let me say this @HannesWell - you've done a lot of hard work on this, and it works. Thank-you!

@HannesWell
Copy link
Member Author

HannesWell commented Jan 8, 2024

@HannesWell : assuming everything works (SDK build is done soon), could you share steps needed for existing local repositories to "properly" update from master.

Yes, the I-build just succeeded and I checked the result and the native fragments all look good, all changes are expected (version names, etc.). 🎉 I also updated a few open PRs and they pass or fail the verification as they did before.

Furthermore I have just send an announcement to the platform-dev mailing in order to provide the necessary steps to other develpers.

If you mean, egit doesn't work, what command is needed, which (if any) config need to be changed etc.

Only pushing LFS-objects seems not to work from EGit to Github (see eclipse-jgit/jgit#11, but it might also be GH specific). Fetching works like charm, which is probably the relevant use-case 99% of the time.

In order to conclude this task I have created eclipse-platform/eclipse.platform.swt.binaries#70.
With that I think this is completely done and everything looks good.

Does a fresh clone works for you?

It was one of those weird things where it was working on one machine but not this particular one. The error occurred when switching commits between pre and post LFS. Then, after lots of re-cloning, re-pulling, etc etc. the problem vanished. Who knows?

Strange, but I'm glad it is now working for you. 😃

But let me say this @HannesWell - you've done a lot of hard work on this, and it works. Thank-you!

It was indeed a lot of work. And thank you for your help in testing this!
A special tanks also to @fredg02 and @heurtematte from the EF-Infra team for their fast help to prepare the CI infrastructure for this.

HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
eclipse-platform#956
but instead just should have been to the changed check-out location.
HannesWell added a commit that referenced this pull request Jan 12, 2024
The working directory was incorrectly removed in
#956
but instead just should have been to the changed check-out location.
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.

Merge swt.binaries repo in the swt one
3 participants