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

Add Compatibility to older Dokan Versions? #34

Open
infeo opened this issue May 1, 2020 · 6 comments
Open

Add Compatibility to older Dokan Versions? #34

infeo opened this issue May 1, 2020 · 6 comments
Labels
Milestone

Comments

@infeo
Copy link
Member

infeo commented May 1, 2020

Since version 1.3.0.1000 Dokany has added the function DokanReleaseMountPointListto dokan1.dll. This library adapted to this change by also adding it to the Native Methods class (see 0716547).

This change breaks compability with older Dokany versions, since the method won't be present and will throw an UnsatisfiedLinkError.

We need to think if we need to stay compatible with older older versions and how we should implement/document this.

@infeo infeo added the bug label May 1, 2020
@Liryna
Copy link
Member

Liryna commented May 4, 2020

Hi @infeo ,

Yes, that unfortunately a break change that needed to happen.
For dokan-dotnet, we are releasing a version of the wrapper for "each" dokan version or at least say compatible from X.X.X version.
There were and will be future breaking changes for userland API and probably not all of them are going to be easy to hide from the user.

That's only my point of view I am sharing and not a guidance, if you find the way and keep the compatibility, that would be awesome!

@JaniruTEC
Copy link
Member

Is there a good reason to keep the compatibility? Are future Dokan versions not suitable for certain Windows Versions?
If not, I would suggest just adding a table with the least required dokan version to the README and "be done with it". My proposal would be #35

@Liryna
Copy link
Member

Liryna commented May 5, 2020

@JaniruTEC The breaking changes in the api/driver are only for improvement for the moment. All futur dokan versions will always be compatible with Win7 and futur Win10.

@JaniruTEC
Copy link
Member

Then I see no reason to create multiple versions of dokan-java for different dokany releases or enforce compatibility with older versions.
Giving the user a useful error message and giving developers a guide for the minimum version seems the best approach IMO.

@infeo
Copy link
Member Author

infeo commented May 11, 2020

Maybe we can keep the compability.

The dokan library dll is loaded during initialization of the NativeMethods class. Maybe we can split this loading into several subclasses, each standing for certain functions/version and depending on the version number given by DokanVersion and DokanDriverVersion only certain functions are loaded.

In the above example the function DokanReleaseMountPointList will be put into a seperate class and only loaded if the version returned by DokanVersion is high enough.

Of course, additionally we need to combine all possible methods in a single class. If the currently installed dokan version does not support a specific function something like an NotImplementedException is thrown.

This would also benefit #24 , because third party access directly to the native methods should be prevented. The wrapping class can then be the "interface" to the native methods.

@infeo
Copy link
Member Author

infeo commented May 19, 2020

Okay, I gave it another thought and decided to also go with the breaking change.

The construct I suggested would get quite complicated, I think, and would be hard to maintain.

@infeo infeo added this to the 2.0 milestone May 19, 2020
@infeo infeo changed the title Add Compability to older Dokan Versions? Add Compatibility to older Dokan Versions? May 19, 2020
infeo pushed a commit that referenced this issue May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants