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

Deal with Windows support being an ongoing shit show #49

Closed
soc opened this issue Mar 21, 2021 · 45 comments
Closed

Deal with Windows support being an ongoing shit show #49

soc opened this issue Mar 21, 2021 · 45 comments

Comments

@soc
Copy link
Collaborator

soc commented Mar 21, 2021

Many contributors have spent heroic efforts to keep Windows support working, for which I'm greatly thankful.

Though it appears to me that Windows support keeps breaking time and time again – perhaps it's time to think whether a different approach could be less painful and more respectful to the time & efforts of contributors?

I'm short on actual ideas though:

  • Alternative approaches that DON'T work:
    • Reading environment variables.
    • Reading registry values.
    • Shipping pre-compiled native code.
  • Alternative approaches that may work:
    • Shipping a .NET assembly that replaces the current Powershell + .NET part?

I'm open to suggestions, thoughts, ideas, etc. – what do people (@alexarchambault, @eatkins, @fthomas. @phongngtuan, ...) think?

@soc soc pinned this issue Mar 21, 2021
@phongngtuan
Copy link
Contributor

pardon my ignorance but what is the issue with System.getenv("APPDATA") and System.getenv("LOCALAPPDATA") ?

@alexarchambault
Copy link
Contributor

alexarchambault commented Mar 21, 2021

I think embedding a tiny JNI library could work too, as we're only calling simple system calls. I had started toying with that some time ago. IIRC, the main hurdle I ran into was charset conversions, from what SHGetKnownFolderPath gives us, to something JNI accepts as a string.

@alexarchambault
Copy link
Contributor

@phongngtuan It's discussed below #26 (comment), these environment variables are not always in sync with the system call, which returns the true values.

@alexarchambault
Copy link
Contributor

About the charset issue I mentioned above, maybe wcstombs and NewString would just work...

@eatkins
Copy link
Contributor

eatkins commented Mar 21, 2021

I feel strongly that precompiled native code with fallback to shelling out is the way to go but I also understand why @soc has skepticism. My experience is that while JNI is definitely a pain and does come with risks, it can be incorporated successfully. For sbt, I wrote a library that speeds up recursive directory listing by bundling pre-compiled code for some platforms. If the library is unable to load the native code for any reason it falls back to jvm built-ins. This code shipped with sbt 1.3.0 and there has only been one issue that came up in pre-release with the windows code: sbt/sbt#4690. Perhaps unsurprisingly given @alexarchambault's comment above, it was also related to Window's use of wide characters by default: swoval/swoval@e425e4c and was basically resolved by switching from NewStringUTF to NewString.

Loading a small dll and running it is generally much faster than shelling out to a process. There are certainly safety issues with using c but I think for this specific use case they are minimal because naively it seems as though the JNI code would just make a single system call and return a java value back to the jvm. This means that it wouldn't need to use the heap, mitigating one of the biggest worries with native code.

I am not personally interested in working on this but I would be happy to share my advice and review any changes. I have spent a lot of time incorporating native code into jvm libraries. In addition to swoval, I also added jni support for unix domain sockets and windows named pipes so that we could use them in a graalvm native image for the sbt thin client (JNA uses reflection in a way that the graalvm could not handle): sbt/ipcsocket#8. There are a few avoidable gotchas if the project decides to go down that road.

@alexarchambault
Copy link
Contributor

So I have this working. I also manually checked that it works with non-ASCII characters.

I think I'm going to try to have it built and published from GitHub actions, and have ProjectDirectories.fromPath accept a custom getWinDirs method as input, so that I can have it use this library, and see how it works.

@alexarchambault
Copy link
Contributor

For context, I think the JNI / Powershell approach is slightly better than just reading env vars, but more importantly, it should be useful in other places in coursier (terminal-related stuff, Windows env var stuff), which is why I'm trying to stick to it.

@soc
Copy link
Collaborator Author

soc commented Mar 22, 2021

@eatkins My main concern is that I'd really like to avoid having yet-another layer on top of the already existing layers that can fail (or not apply due to obscure reasons/on obscure platforms).
I'd like to have something that reduces the existing complexity while increasing the reliability.

I'll be migrating this library to the new Java FFI as soon as Project Panama ships, but in the meantime I think a stop-gap solution that is less brittle than what we currently have is direly needed.

unkarjedy added a commit to JetBrains/intellij-scala that referenced this issue Apr 19, 2021
… unit tests on windows to avoid hanging ProjectImportTest

This is required to workaround sbt/sbt#5128;
The bug is reproduced on Teamcity, on Windows agents.
ProjectImportingTest is stuck indefinitely when the test is run from sbt.
It's also reproduces locally when running the test from sbt.
But for some reason is not reproduced when running from IDEA test runners.

Environment variables which have to be mocked are inferred from
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeCacheDirectory
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeJvmCacheDirectory

see also dirs-dev/directories-jvm#49
see also https://github.com/ScoopInstaller/Main/pull/878/files
unkarjedy added a commit to JetBrains/intellij-scala that referenced this issue Apr 19, 2021
… unit tests on windows to avoid hanging ProjectImportTest

This is required to workaround sbt/sbt#5128;
The bug is reproduced on Teamcity, on Windows agents.
ProjectImportingTest is stuck indefinitely when the test is run from sbt.
It's also reproduces locally when running the test from sbt.
But for some reason is not reproduced when running from IDEA test runners.

Environment variables which have to be mocked are inferred from
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeCacheDirectory
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeJvmCacheDirectory

see also dirs-dev/directories-jvm#49
see also https://github.com/ScoopInstaller/Main/pull/878/files
unkarjedy added a commit to JetBrains/intellij-scala that referenced this issue Apr 22, 2021
… unit tests on windows to avoid hanging ProjectImportTest

This is required to workaround sbt/sbt#5128;
The bug is reproduced on Teamcity, on Windows agents.
ProjectImportingTest is stuck indefinitely when the test is run from sbt.
It's also reproduces locally when running the test from sbt.
But for some reason is not reproduced when running from IDEA test runners.

Environment variables which have to be mocked are inferred from
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeCacheDirectory
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeJvmCacheDirectory

see also dirs-dev/directories-jvm#49
see also https://github.com/ScoopInstaller/Main/pull/878/files
unkarjedy added a commit to JetBrains/intellij-scala that referenced this issue Apr 23, 2021
… unit tests on windows to avoid hanging ProjectImportTest

This is required to workaround sbt/sbt#5128;
The bug is reproduced on Teamcity, on Windows agents.
ProjectImportingTest is stuck indefinitely when the test is run from sbt.
It's also reproduces locally when running the test from sbt.
But for some reason is not reproduced when running from IDEA test runners.

Environment variables which have to be mocked are inferred from
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeCacheDirectory
  - lmcoursier.internal.shaded.coursier.paths.CoursierPaths.computeJvmCacheDirectory

see also dirs-dev/directories-jvm#49
see also https://github.com/ScoopInstaller/Main/pull/878/files
@matejsarlija
Copy link

matejsarlija commented Oct 22, 2021

https://i.imgflip.com/5refek.jpg (had to do this, this issue pops up on every Scala tool).

@soc
Copy link
Collaborator Author

soc commented Apr 26, 2022

@RayKoopa welcome!

@RayKoopa
Copy link

RayKoopa commented Apr 26, 2022

Hey there, crossposting my thoughts on this. The scenario here seems to be to remove the PowerShell invocation as it is causing trouble. These options seem viable, which you've mentioned in part above:

  • Push the SHGetKnownFolderPath call from your PS script into a .NET assembly and call the assembly instead.
  • Use JNI / C directly to do the SHGetKnownFolderPath call in there.
  • Find a PowerShell expert knowing how to solve the PS invocation issues, if it is generally possible at all (I'm afraid I'm not one).

I don't know security implications of JNI as I'm not a Java developer, but since you effectively call a C WinAPI method, the call itself wouldn't be more or less secure than doing it in .NET.

If you have considered using .NET's base library method System.Environment.GetFolderPath, I have to disappoint you:

  • It internally just calls SHGetKnownFolderPath too, but its signature is limited to the enum values defined in System.Environment.SpecialFolders.
  • Given this, it does not not support the "Downloads" and "Public" folder as the enum is based on folders that existed in Windows XP only (the enum internally maps to values intended for a deprecated WinAPI call which never supported these folders, despite .NET now internally calling the new Windows Vista+ API, but not having extended that enum due to those old values).
  • Support for the "new" (15+ years 😎) folders introduced in Vista is now mainly discussed in Environment.GetFolderPath() doesn't support all KNOWNFOLDERID's dotnet/runtime#554 and may ship in .NET 7 or 8. However, these .NET versions are not preinstalled by Windows (Update) and require to be installed manually. Additionally, if it doesn't make the cut for .NET 7 and only .NET 8, Windows 7 may not be supported as its extended support period ends before .NET 8's scheduled release (end of 2023). Also Windows Vista is not supported by the new .NET (Core) at all.

Given this, I'd personally recommend just using JNI / C for this. It seems to be the natural thing Java does for native OS-specific API calls anyway, I presume?

If you still want to P/Invoke in .NET, you may like my CodeProject article on that. As mentioned in the dotnet issue, your call in the PS script seems effectively correct, though you can / should use automatic string marshaling to not have to deal with manually converting the returned buffer to a .NET string, and freeing it in all cases afterwards.

Also, a side note on the "Public" folder: On Windows, this folder is per-system, not per-user. It also has subfolders for Documents, Downloads, Pictures, etc., but these subfolders can be redirected, even outside of the parent "Public" folder. If a user wants to store a "public document" and expects it to be in Public > Documents, they would have to explicitly query the "PublicDocuments" path, since appending "\Documents" to the public path is out of the question due to the redirection scenario.

@soc
Copy link
Collaborator Author

soc commented Apr 26, 2022

@RayKoopa Thank you! My concern with JNI/C is that I have to maintain/keep/compile a piece of code for every platform Windows runs on. I own a Windows license for exactly zero platforms, and cross-compiling with C seems to be painful as well.

Which brings us to using C (compilers), which I'd rather just not. "It can be written safely" is probably true for this use-case, but too many people making this verdict in too many situations is exactly what brought computing into disrepute.

I'm starting to wonder whether the registry values are generally reliable. So Fonts may not exist, but on the other hand even Downloads is in there. I wonder ...

  • ... what the registry keys look like on these Windows CI instances and other minimal installs
  • ... whether they are updated to reflect changes after calls to SHSetKnownFolderPath

@RayKoopa
Copy link

RayKoopa commented Apr 26, 2022

Hmm I see. I hoped there would be some kind of interoperability offered by JNI that would not require to compile raw C and introduce lots of complexity to your project; kinda like P/Invoke in .NET.
EDIT: I just found out about JNA, example usage. This looks like somebody already wrapped it for you quite nicely. /EDIT

Maybe you can find another way; effectively you "just" need to do two standard C calls made by something preinstalled playing together better with Java, and convert the returned memory buffer storing a 0-terminated UTF16 LE string to a Java String. 🤔

As you already know, the registry method is unsupported. In practice, to my experience, at very weird occassions the keys do not exist or can be outdated / wrong. Required reading would be https://devblogs.microsoft.com/oldnewthing/20031103-00/?p=41973 and https://devblogs.microsoft.com/oldnewthing/20110322-00/?p=11163 . It may however be "better" to risk "possibly" incorrect results rather than getting no results due to PS invocation failing (which I know nothing about sadly). Your mileage may vary. I just checked the registry key on my system: It does not list the "Public" folder you are seeking support for, nor for the subfolders included in it. So the registry solution is not only unsupported, it is also insufficient.

@eatkins
Copy link
Contributor

eatkins commented Apr 26, 2022

@soc your concerns about maintainability are quite well warranted and I am sympathetic. I can't remember if this came up on other threads but is there a reason why JNA is not considered an option here? I personally kind of hate JNA and would rather just write a JNI library but I think it may alleviate the cross compilation problem. Adding a dependency would be unfortunate but sbt, which I'm guessing is the biggest consumer of this library, already depends on the JNA anyway.

Either way, I will reiterate that I don't think the JNI solution is as bad as it may seen. Microsoft for all its many flaws religiously preserves backward compatibility so you just would need a system for compiling the binary once and then checking it in (though I will acknowledge that checking binaries in to a git repo is icky). I have had success cross-compiling jni libraries with mingw, which is available for mac and linux, for x86_64 though I have never tried it for an arm platform. It also would be possible to compile on a CI platform like appveyor (I have built and distributed binaries using appveyor) or github actions (I haven't actually tried this but would assume that github actions would have a mechanism for exporting artifacts). It would indeed be unreasonable for you to be expected to maintain code for platforms that you cannot easily test so you could reasonably draw a line in the sand that any supported windows platform must have freely available cloud vms to build any needed binaries.

I am very sympathetic to the desire to avoid JNI or any windows specific building. I don't have a windows license either. The way I handled it was to install windows on virtualbox, which does not require a paid license. It was never pleasant but it worked. I also agree with you ideologically that c is terrible but if the only api the OS provides is a c api, I'm not sure what option there is but to submit to working directly with that api.

@eatkins
Copy link
Contributor

eatkins commented Apr 27, 2022

I am going to unsubscribe from this thread. Every time there is a comment, it is like a mini ddos attack on my brain. It is frustrating that from my perspective the only viable solution is being rejected for ideological reasons. This is a disservice to the downstream users of the library who are affected. If someone needs help implementing a jni solution, I have experience and would be happy to offer assistance. Please email me directly and don’t @ me here.

@soc
Copy link
Collaborator Author

soc commented Apr 27, 2022

is there a reason why JNA is not considered an option here

Sadly JNA is rather big, it would turn this < 10kB library into a 3.7MB one. :-/ (see #16)

@alexarchambault
Copy link
Contributor

alexarchambault commented Jun 15, 2022

If ever people stumble upon this issue, this is addressed in coursier, via JNI, by:

coursier/directories-jvm isn't published on Maven Central. coursier uses it as a source dependency, then shades it. So if you depend on coursier, you can access it via coursier.cache.shaded.dirs.ProjectDirectories, and get the from method accepting a fourth parameter.

I didn't test it on Windows ARM64, so I have no idea how it works there (but coursier has a fallback to former powershell stuff in that case).

@soc
Copy link
Collaborator Author

soc commented Jun 21, 2022

@alexarchambault good work! Do you know how large the binaries turned out?

I'm experimenting with an approach in dirs-cli and end up with 30KiB (16KiB with upx).
Though I still need to replace the Unix-only File::from_raw_fd(1) with WriteConsole for Windows.

Another thought is building an x86 binary and using that on both x86-64 and x86. (Not sure what's the situation on ARM...)

@DavidGregory084
Copy link

DavidGregory084 commented Feb 6, 2023

@soc If it helps, you can implement JNI functions in Rust without too much trouble (see e.g. this code here) so you could call through to your directories-sys-rs crate rather than implementing something in C, and it would then support the same platforms as your Rust code does?
AFAIK the approach that folks usually use to deploy these is to bundle the dylib for every platform into the resources of the JAR file, then extract the one for the current platform into a temp directory and load it using System.load at static initialization time, although I have never implemented anything like this myself. I can't think of a library example off the top of my head but protoc-jar does something similar.

@soc
Copy link
Collaborator Author

soc commented Feb 6, 2023

@DavidGregory084 Interesting, thanks!

oyvindberg added a commit to oyvindberg/bleep that referenced this issue Feb 12, 2023
@soc
Copy link
Collaborator Author

soc commented Jul 31, 2024

Hey everyone, does the output on Windows here look sensible?

<snip>
UserDirectories (Windows Server 2022):
  homeDir     = 'C:\Users\runneradmin'
  audioDir    = 'C:\Users\runneradmin\Music'
  fontDir     = 'null'
  desktopDir  = 'C:\Users\runneradmin\Desktop'
  documentDir = 'C:\Users\runneradmin\Documents'
  downloadDir = 'C:\Users\runneradmin\Downloads'
  pictureDir  = 'C:\Users\runneradmin\Pictures'
  publicDir   = 'C:\Users\Public'
  templateDir = 'C:\Users\runneradmin\AppData\Roaming\Microsoft\Windows\Templates'
  videoDir    = 'C:\Users\runneradmin\Videos'

BaseDirectories (Windows Server 2022):
  homeDir       = 'C:\Users\runneradmin'
  cacheDir      = 'C:\Users\runneradmin\AppData\Local'
  configDir     = 'C:\Users\runneradmin\AppData\Roaming'
  dataDir       = 'C:\Users\runneradmin\AppData\Roaming'
  dataLocalDir  = 'C:\Users\runneradmin\AppData\Local'
  executableDir = 'null'
  preferenceDir = 'C:\Users\runneradmin\AppData\Roaming'
  runtimeDir    = 'null'

ProjectDirectories (Windows Server 2022):
  projectPath   = 'Baz Corp\Foo Bar-App'
  cacheDir      = 'C:\Users\runneradmin\AppData\Local\Baz Corp\Foo Bar-App\cache'
  configDir     = 'C:\Users\runneradmin\AppData\Roaming\Baz Corp\Foo Bar-App\config'
  dataDir       = 'C:\Users\runneradmin\AppData\Roaming\Baz Corp\Foo Bar-App\data'
  dataLocalDir  = 'C:\Users\runneradmin\AppData\Local\Baz Corp\Foo Bar-App\data'
  preferenceDir = 'C:\Users\runneradmin\AppData\Roaming\Baz Corp\Foo Bar-App\config'
  runtimeDir    = 'null'
<snip>

@brcolow
Copy link
Contributor

brcolow commented Jul 31, 2024

Looks good to me.

@soc
Copy link
Collaborator Author

soc commented Aug 1, 2024

Anyone interested in reviewing the code, it is here: #61

soc pushed a commit that referenced this issue Aug 2, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
@tliechti
Copy link

tliechti commented Aug 20, 2024

Anyone interested in reviewing the code, it is here: #61

Really appreciate your work. Sadly, I can not use Java 22. So I did a quick hack to omit powershell.exe and calling pwsh.exe only while omitting the -v -2 params. That worked for me with pwsh >7.4.3.

@soc
Copy link
Collaborator Author

soc commented Aug 20, 2024

That generally might or might not work depending on various Windows Defender settings.

@karianna
Copy link

karianna commented Oct 16, 2024

Hi folks, I run the Java Engineering and Golang Group at Microsoft, but I'll stress that I'm here in a personal capacity :-) (this isn't strictly related to the Microsoft Build of OpenJDK).

I suspect I'm missing a ton of nuance, but it sounds like that this project is unable to use System.getenv("APPDATA") and System.getenv("LOCALAPPDATA") because those environment variables are not consistently available, (in part due to changes of how you're allowed to read from the registry) across all versions of Windows, or if you invoke a Java program on Windows in a particular way (e.g., Run As)?

JNA and JNI aren't desirable because you're looking fro a pure Java solution?

Lastly, using FFI in Java 22+ does work but that requires users to have Java 22+ installed, which not all users can do. (Note with Java 22, you can jlink / jpackage the Java runtime modules with the application modules, but that would create a large bundle which I suspect is also not desirable).

Have I got that all correct?

@brcolow
Copy link
Contributor

brcolow commented Oct 16, 2024

@karianna That is a good summary in my opinion. I am the one who made the prototype for using FFI in Java 22 as I thought it was the best long-term solution. Of course it has drawbacks for backwards compatibility :).

@soloturn
Copy link

soloturn commented Oct 16, 2024

would JNA, to give some example: https://github.com/harawata/appdirs/blob/master/src/main/java/net/harawata/appdirs/impl/ShellFolderResolver.java , be an appropriate 2nd option for the time beeing, @karianna @brcolow , as the problem mentioned above with JNA was with graalvm - which anyway compiles it native - so no problem with ffi? is it not also a security challenge to have FFI code, as users do not know really what is inside, therefor the warning ?

WARNING: A restricted method in java.lang.foreign.AddressLayout has been called
WARNING: java.lang.foreign.AddressLayout::withTargetLayout has been called by dev.dirs.impl.Windows in an unnamed module
WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled

maybe some background why i bumped into this ticket. when packaging terasology for windows in chocolatey the app failed to start. as healing i made pull request MovingBlocks/Terasology#5284 using dir-env to get the correct folders on windows. the folders we could then overwrite with environment variable XDG* when testing with multiple clients on one box, and ideally properties (#62) so it is easier for unit tests. as since java-17 changing environment variables out of a running java process is not obvious. in the code review @BenjaminAmos pointed out that he prefers JNA as more robust than what dir-env currently does.

@soc
Copy link
Collaborator Author

soc commented Oct 16, 2024

Hi @karianna,

expanding on @brcolow a bit:

this project is unable to use System.getenv("APPDATA") and System.getenv("LOCALAPPDATA") because those environment variables are not consistently available

the problem is more that these env variables (as well as the registry keys) have no meaning.

Same for System.getenv("PROFILE") or System.getenv("HOME") that some people want to use.

Using those would mean inventing some proprietary handshake that would work if everyone used this library, but would be completely separate from the actual reality – that is solely defined by the KnownFolders API.

JNA and JNI aren't desirable because you're looking fro a pure Java solution?

  • JNA would be a 30x size increase of this library.
  • JNI suffers from needing Windows machines with the desired architecture to build things.
  • Shipping with my own CLI app and shelling out to it from Java also suffers from this, though picking an appropriate language would at least avoid having to build on Windows. (See dirs-cli-rs).

Lastly, using FFI in Java 22+ does work but that requires users to have Java 22+ installed, which not all users can do.

Yep, though that problem gets smaller by the minute. And those who don't upgrade their machine could also be fine with the unholy PowerShell mess that previously powered this library.

Hope that helped!

@karianna
Copy link

So I think the longer term solution is definitely FFI. @brcolow - I know the folks in OpenJDK would love to hear of any feedback on your experience there so far (openjdk.org has a project panama mailing list).

I can chat to my .NET colleagues at Microsoft and see what they've done in .NET core to resolve this type of thing, there might be an alternative way to approach this. However, I'm going to guess that they'll suggest going down the JNI path since getting the Windows team to add a new registry entry / way of accessing these folders in a consistent manner across all versions of Windows would be extremely challenging.

So shorter term, your bet best is JNI. I appreciate that causes some build platform pain, which brings me to asking how are the builds produced today, through GitHub Actions or something else?

@soloturn
Copy link

soloturn commented Oct 17, 2024

JNA and JNI aren't desirable because you're looking fro a pure Java solution?

* JNA would be a 30x size increase of this library.

to get windows support stable, that would be perfectly fine for a user of the library my guess would be. for plantuml, uml drawing software, we solved the size challenge by offering two binaries, one small, one big which included the pdf generation, if you search for pdf or pdfJar in the gradle build file. would such an approach be thinkable here as well @soc ?

@sideeffffect
Copy link

I, personally, would prefer something which is

  • correct
  • pure-Java (no other languages need to be involved)
  • platform independent -- not requiring multiple compilation steps for each architecture
  • works with GraalVM native-image

Does the JNA approach satisfy these criteria? If yes, I wouldn't mind the increase in size at all.

@soc
Copy link
Collaborator Author

soc commented Oct 17, 2024

Does the JNA approach satisfy these criteria?

No. It's native code, it's "prepackaged JNI".

@soc
Copy link
Collaborator Author

soc commented Oct 17, 2024

I can chat to my .NET colleagues at Microsoft and see what they've done in .NET core to resolve this type of thing, there might be an alternative way to approach this.

No need to ask: They use P/Invoke, and their implementation has been missing folders for years, which is the reason we did the PowerShell → .NET → P/Invoke dance in the first place.

@karianna
Copy link

I think JNI could be a middle ground here (should keep the package small). If it's just platform testing then I might be able to help secure some test platforms

@soc
Copy link
Collaborator Author

soc commented Oct 17, 2024

@karianna Thanks, though I don't think I want to consider JNI at this point.

It could have been useful 5 years ago, or before suffering the PowerShell mess; not now when we already suffered through another rewrite (the Java FFI one) a few weeks ago.

Not actually sure why this discussion popped up again – this issue is solved from my point of view. The only work items I thought I have left is merging #61 and publishing a new version on Maven Central.

@sideeffffect
Copy link

this issue is solved from my point of view. The only work items I thought I have left is merging #61 and publishing a new version on Maven Central.

The problem then is that it requires JVM >= 22, which may be problem for many of the current users of the library. Ideally the base JVM would 8, or at least 11, to cover wider user base. Other approaches (like JNA) might be better from this perspective. Or at least have multi-release JAR, which would have fallback for JVM < 22.

@soc
Copy link
Collaborator Author

soc commented Oct 17, 2024

@sideeffffect As mentioned using the version that fits your JVM version requirement is fine.

I'm not seeing how spending more time on this topic will lead to the discovery of a magic solution that has eluded us all for the last 5 years.

soc pushed a commit that referenced this issue Oct 17, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
soc pushed a commit that referenced this issue Oct 17, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
@soloturn
Copy link

soloturn commented Oct 19, 2024

Not actually sure why this discussion popped up again – this issue is solved from my point of view. The only work items I thought I have left is merging #61 and publishing a new version on Maven Central.

can i help doing the release as you are sure it is best approach? how do you propose changing such variables in unit tests @soc because changing environment variables is a little tricky?

soc pushed a commit that referenced this issue Oct 21, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
soc pushed a commit that referenced this issue Oct 21, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
soc pushed a commit that referenced this issue Oct 21, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
@soloturn
Copy link

soloturn commented Oct 27, 2024

@karianna would you be able to share a HelloWorld ? is "loadLibrary" broken or we tried to use it wrong?

public class HelloWorld {
    static {
        System.loadLibrary("combase");
    }

    public native void sayHello();

    public static void main(String[] args) {
        HelloWorld helloWorld = new HelloWorld();
        helloWorld.sayHello();
    }
}
$ java --version
openjdk 23.0.1 2024-10-15
OpenJDK Runtime Environment Zulu23.30+13-CA (build 23.0.1+11)
$ javac HelloWorld.java
$ java HelloWorld
Exception in thread "main" java.lang.UnsatisfiedLinkError: 'void HelloWorld.sayHello()'
        at HelloWorld.sayHello(Native Method)
        at HelloWorld.main(HelloWorld.java:12)

@Friendseeker tested for sbt sbt/sbt#7833 and got it to work with hard coding the path with getting an environment variable. the whole exercise in this ticket was to avoid getting an environment variable as it was not stable.

     String sysdir = System.getenv("WINDIR") + "/system32/";
     System.load(sysdir + "combase.dll");
     System.load(sysdir + "ole32.dll");
     System.load(sysdir + "shell32.dll");

soc pushed a commit that referenced this issue Nov 7, 2024
- Drop all existing mechanisms for retrieving this info on Windows
- This increases the required Java version of the library to 22

Fixes #49.
@soc soc closed this as completed in b76e360 Nov 7, 2024
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

No branches or pull requests