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

Allow users to circumvent PowerShell stuff #53

Closed
wants to merge 1 commit into from

Conversation

alexarchambault
Copy link
Contributor

@alexarchambault alexarchambault commented Sep 28, 2021

These changes allow users to provide a custom way of getting Windows system folders from UUIDs, rather than using the default PowerShell-based way, which has been a source of issues for quite some time.

I've been using this fine from coursier (and some other not yet publicly released projects), where I provide a JNI-based class, that directly uses Windows system calls to get those directories (based on this library).

@soc
Copy link
Collaborator

soc commented Sep 28, 2021

Thanks! I will read it in more detail later, just so you are aware there is some work happening in https://github.com/dirs-dev/directories-net, I was hoping to get this polished, released and investigate replacing the PowerShell approach with using that library.

My concern is that the PR pretty much encodes a technical workaround into the public API. I believe that users should never have to worry about this – things should just work.

@alexarchambault
Copy link
Contributor Author

But https://github.com/dirs-dev/directories-net requires the dotnet runtime to run, right? I'm not sure this can work.

Also, I'm interested in the flexible approach if JNI is involved. I'm "shading" the directories-jvm classes in some of my libraries, which messes with JNI (if you rename a class, JNI will look for different native method names, so the JNI native code needs to be changed too, and shading libraries can't really do that).

@soc
Copy link
Collaborator

soc commented Sep 28, 2021

The current approach also needs a .NET runtime, so we would not introduce a new dependency but remove the PowerShell one.

@alexarchambault
Copy link
Contributor Author

The JNI approach is even better. No need of a runtime at all.

@alexarchambault
Copy link
Contributor Author

The JNI DLL of coursier/jni-utils could even be vendored here. It'd work for me, as long as one can change the internals like this PR proposes.

@alexarchambault
Copy link
Contributor Author

alexarchambault commented Sep 28, 2021

(That DLL could be slimmed down… It contains duplicated code, as a workaround for the various shading stuff I mentioned above.)

@soc
Copy link
Collaborator

soc commented Sep 28, 2021

The problem is that I would need to ship and maintain native x86, amd64, arm32, arm64 dlls. The devices I own cover 0% of that.

@alexarchambault
Copy link
Contributor Author

The Windows amd64 at least (and probably the x86 too…) can be built on GitHub actions. That's what coursier/jni-utils does.

For ARM, until a solution can be found, the PowerShell fallback can be kept…

@soc
Copy link
Collaborator

soc commented Oct 3, 2021

My main concern is the ARM situation, because I think even more approaches and fallbacks on increasingly obscure platforms puts us into a situation where it gets really hard to reproduce and fix issues.

Perhaps there is some build service that can also generate ARM .dll files, but this is nothing I'd ask anyone to do because it's a thankless job, and I also lack the capacity to look into this in the coming months myself.

In the long-term Panama could solve this for good, but I fear we are at least a decade away from that. :-/

@brcolow
Copy link

brcolow commented Mar 30, 2024

@soc I made a proof-of-concept using Java 22 Foreign Function & Memory API of how to extract a LocalAppData (as an example) known folder id, in case it is helpful for you.

https://gist.github.com/brcolow/e6c2e59a3aa29d32d3332bcf10313031

@alexarchambault alexarchambault deleted the custom-get-win-dirs branch May 28, 2024 12:08
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.

None yet

3 participants