Migrate equinox.security.win32 to JNA to enhance CPU-arch compatibility

[HannesWell]

Migrate equinox.security.win32 to JNA to enhance CPI-arch compatibility, just like it is done for equinox.security.linux.
This simplifies the build and allows to use equinox.security.win32 not only on x86_64 but also ARM CPUs, as it is required in #559.
Use Crypt32Util class from jna.platform to get high-level java access to Crypt32.dll respectively the methods defined in dpapi.h.

Rename the fragment from org.eclipse.equinox.security.win32.x86_64 to org.eclipse.equinox.security.win32 to reflect that it's not architecture specific anymore. This should be a save change since fragments can only be referenced from products and features and not from other bundles (at least not by their symbolic name).
Because features lock the bundle version one has to re-build the feature anyways to pick-up a new version and I think it is similar for plugin-based Products. One has to re-build them to pick-up a new version and then also also change the name.

The org.eclipse.equinox.p2.core.feature feature has to be adapted to the new name too.

[HannesWell]

Use Crypt32Util class from jna.platform to get high-level java access to Crypt32.dll respectively the methods defined in dpapi.h.

If the with that implied dependency to com.sun.jna.platform is too much everything done in the utility can the replicated to only require com.sun.jna. But com.sun.jna.platform is required for the Eclipse SDK anyways so it would only matter for consumers with much lower requirements.

Rename the fragment from org.eclipse.equinox.security.win32.x86_64 to org.eclipse.equinox.security.win32 to reflect that it's not architecture specific anymore.

As it was done in fdebce6, we could also retain the old fragment and require the new one using a p2.inf, but I hope that this is not necessary.

[github-actions]

Test Results

   28 files  ±0     28 suites  ±0   11m 57s ⏱️ -6s
2 170 tests ±0  2 124 ✅ ±0  46 💤 ±0  0 ❌ ±0 
2 242 runs  ±0  2 196 ✅ ±0  46 💤 ±0  0 ❌ ±0 

Results for commit 1f2c66f. ± Comparison against base commit a90c944.

♻️ This comment has been updated with latest results.

[chirontt]

Great work in using JNA for the org.eclipse.equinox.security.win32 bundle. I'm building the Eclipse SDK for WoA with this PR to see how it goes, which brings up questions about testing.

Are there any (existing) unit/integration tests for this security bundle? I see the WinPreferencesTest.java there, but can it be executed on the command line (with mvn test..., etc.)? At the Eclipse SDK/IDE level, how do we test this specific bundle? Which area of the Eclipse SDK/IDE is affected by this bundle?

Rename the fragment from org.eclipse.equinox.security.win32.x86_64 to org.eclipse.equinox.security.win32 to reflect that it's not architecture specific anymore.

As it was done in fdebce6, we could also retain the old fragment and require the new one using a p2.inf, but I hope that this is not necessary.

I really hope not, for sanity's sake. Seeing empty fragments lying around and contributing nothing is the most confusing aspect when working with Eclipse repositories.

[HannesWell]

Thanks.

Are there any (existing) unit/integration tests for this security bundle? I see the WinPreferencesTest.java there, but can it be executed on the command line (with mvn test..., etc.)?

Does mvn verify not work?

At the Eclipse SDK/IDE level, how do we test this specific bundle? Which area of the Eclipse SDK/IDE is affected by this bundle?

The PasswordProvider is used by the General -> Security -> Secure Storage. For exact details I suggest you check the callers of the interesting methods of PasswordProvider.

Rename the fragment from org.eclipse.equinox.security.win32.x86_64 to org.eclipse.equinox.security.win32 to reflect that it's not architecture specific anymore.

As it was done in fdebce6, we could also retain the old fragment and require the new one using a p2.inf, but I hope that this is not necessary.

I really hope not, for sanity's sake. Seeing empty fragments lying around and contributing nothing is the most confusing aspect when working with Eclipse repositories.

Personally that's also my favorite solution, but we have to keep in mind existing users and should be gentle to them, but as written in my initial comment and if I'm not mistaken, there is probably not a case where the renaming of a fragment should break existing users since one has to re-build everything that can reference it any ways.
IIRC one cannot update single plugins/fragments of plugin-based products.

[HannesWell]

Rename the fragment from org.eclipse.equinox.security.win32.x86_64 to org.eclipse.equinox.security.win32 to reflect that it's not architecture specific anymore. This should be a save change since fragments can only be referenced from products and features and not from other bundles (at least not by their symbolic name).
Because features lock the bundle version one has to re-build the feature anyways to pick-up a new version and I think it is similar for plugin-based Products. One has to re-build them to pick-up a new version and then also also change the name.

I also checked if that fragment could be referenced as dependency in a feature and at least the PDE Feature editor does not permit that.

I also analyzed the SimRel and nobody except the following three features in the Eclipse SDK seem to require that fragment:

• org.eclipse.equinox.core.sdk
• org.eclipse.equinox.p2.core.feature
• org.eclipse.platform

The feature org.eclipse.equinox.core.sdk is adapted with this PR, org.eclipse.equinox.p2.core.feature is adapted with eclipse-equinox/p2#492 and for org.eclipse.platform I'll have to create a PR.

Considering all that I think it is ok, to just rename the fragment.

[HannesWell]

@chirontt btw. according to Wikipedia, JNA is in average ten times slower than JNI. This probably only refers to the invocation itself, but for invocations of short-running methods this is probably a show-stopper if it is performance critical.
That being said, I don't think that's the case here.

[laeubi]

@HannesWell we might improve this in the future by using FFM API is crypt32.dll a standard Windows API or does it needs to be installed/included in the bundle?

[HannesWell]

@HannesWell we might improve this in the future by using FFM API

Absolutely.

is crypt32.dll a standard Windows API or does it needs to be installed/included in the bundle?

As far as I can tell it is a standard Windows API and available by default on all Windows computers.
It's that API, which seems to be part of win32:
https://learn.microsoft.com/en-us/windows/win32/api/dpapi/nf-dpapi-cryptunprotectdata

[tjwatson]

@HannesWell we might improve this in the future by using FFM API

I don't want to discount the potential for FFM, but it is in preview for Java 21 so I don't think we can consider using it by default for a while yet until it is finalized.

[HannesWell]

@HannesWell we might improve this in the future by using FFM API

I don't want to discount the potential for FFM, but it is in preview for Java 21 so I don't think we can consider using it by default for a while yet until it is finalized.

Yes, unfortunately FFM has been finalized only in Java 22. Given the FFM-API code necessary for this didn't change between 21 and 22, in general we could consider to use it with 21 already. Given we would agree that running Eclipse with --enable-preview is acceptable. But I think that's a point that would have to be discussed in a broader audience.

But to come back to the topic of this PR: I have now tested this extensively running the a debugged secondary Eclipse and everything worked well. It wasn't that simple since there is no simple user of ISecurePreferences in the SDK but with some modification I could verify that existing an secure-storage on Windows can be read and new entries can be added.

I also tried to rename the WindowsPasswordProvider64bit org.eclipse.equinox.security.secureStorage extension to WindowsPasswordProvider. But then we would have to keep the old one as legacy provider in order to be able to read existing entries. And that would also show up in the UI. For now I don't think it's worth the effort.

With all that, I think this is ready for submission.