Skip to content

Conversation

@deepika-u
Copy link
Contributor

@deepika-u deepika-u commented Mar 3, 2025

Configure SWT build scripts for 4.36

Fixes eclipse-platform/eclipse.platform.releng.aggregator#2855

@deepika-u
Copy link
Contributor Author

deepika-u commented Mar 3, 2025

I am getting below error on windows, linux platform
java.lang.UnsatisfiedLinkError:
Could not load SWT library. Reasons:
no swt-win32-4969 in java.library.path

@HeikoKlare @HannesWell
Do i have to do any additional changes for this task(or did anything changed recently which i am not aware of). Last time i have done only these changes for 4.35 -> #1618

@HeikoKlare
Copy link
Contributor

Something seems to have changed in the binaries build. The Library class loading the binaries does not append a revision suffix in case the revision is 0:

if (REVISION > 0) version += "r" + REVISION; //$NON-NLS-1$

But the native build append that revision. That's why the expected library name does not match the actual one.

Library names (Windows example):

  • Actual: swt-win32-4969r0.dll
  • Expected: swt-win32-4969.dll

@HeikoKlare
Copy link
Contributor

No matter what caused this, I would be in favor of using consistent library version naming always having a revision suffix starting with r0, thus removing the if-check in the posted line of the Library implementation.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do i have to do any additional changes for this task(or did anything changed recently which i am not aware of). Last time i have done only these changes for 4.35 -> #1618

The 'trick' is that a version zero is never published because as soon as this is build the revision is incremented by one, since this is a change in the native source-code. If you look at the build jars in Jenkins you can see that:
grafik

The increment just doesn't happen for a local build respectively in the GH workflows.
If you look at the commit of that previous PR (26abd14)
You can see that it immediately is succeeded by
a6b8fe7

So this change is perfectly fine.

That being said, I think Heiko's suggestion also make sense.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2025

Test Results

 96 files  ±0   96 suites  ±0   3s ⏱️ ±0s
 58 tests ±0   19 ✅  - 39  0 💤 ±0  0 ❌ ±0  39 🔥 +39 
191 runs  ±0  152 ✅  - 39  0 💤 ±0  0 ❌ ±0  39 🔥 +39 

For more details on these errors, see this check.

Results for commit baa0959. ± Comparison against base commit 6171056.

♻️ This comment has been updated with latest results.

@deepika-u deepika-u force-pushed the Configure_SWT_build_scripts_for_4.36 branch 2 times, most recently from f650587 to a6ca3dd Compare March 4, 2025 07:27
@deepika-u
Copy link
Contributor Author

No matter what caused this, I would be in favor of using consistent library version naming always having a revision suffix starting with r0, thus removing the if-check in the posted line of the Library implementation.

Updated Library.java as per the suggestion. Thanks alot.

Comment on lines 376 to 377
/* No "r" until first revision */
if (REVISION > 0) version += "r" + REVISION; //$NON-NLS-1$
//if (REVISION > 0) version += "r" + REVISION; //$NON-NLS-1$
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not correct:

  1. The revision must remain, it should only also be added for revision "0" (i.e., the if should be removed).
  2. The comment should be adapted to conform to the code.

For the sake of comprehensibility, I would be in favor of putting this change at least into a separate commit with a proper commit description as it is independent from configuring build scripts for the release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah will do it, had the same thought in my mind before doing this way, since discussed in this issue - did it in the same pr. Let me separate it out.

@MohananRahul
Copy link
Contributor

Please note that this is blocking Ist IBuild and master opening.

@HeikoKlare
Copy link
Contributor

As Hannes mentioned, we should probably just merge the original change as it was (since it will work fine) and potentially change the Library implementation independently.

@deepika-u deepika-u force-pushed the Configure_SWT_build_scripts_for_4.36 branch from c426f5f to baa0959 Compare March 4, 2025 10:17
@HeikoKlare
Copy link
Contributor

Since Hannes already approved that this change is correct in its current state, I would merge now. Any objections, @deepika-u?

@deepika-u
Copy link
Contributor Author

deepika-u commented Mar 4, 2025

Since Hannes already approved that this change is correct in its current state, I would merge now. Any objections, @deepika-u?

Please go ahead, indeed i was waiting for your reply...... and was away for lunch.

@HeikoKlare HeikoKlare merged commit 7228a5c into eclipse-platform:master Mar 4, 2025
3 of 10 checks passed
@deepika-u
Copy link
Contributor Author

Thanks alot.

@HeikoKlare
Copy link
Contributor

HeikoKlare commented Mar 4, 2025

Bundle versions have not been bumped yet, thus the build fails and updated binaries are not generated.
Created #1876

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.

Configure SWT build scripts for 4.36

4 participants