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

Enable removal of virtual filesystems #8772

Merged
merged 4 commits into from
Dec 3, 2023
Merged

Conversation

Maxxen
Copy link
Contributor

@Maxxen Maxxen commented Nov 21, 2023

What does this PR do?

This PR enables the removal of installed virtual filesystems.

What are related issues/pull requests?

Not in GDAL, but over in DuckDB Spatial I've ran into multiple problems as a result of trying to integrate DuckDB's own filesystem abstractions into GDAL since we don't assume that there is a "global" filesystem. New filesystems can be attached or removed at runtime and are tied to the lifetime of a connection (of which there can be multiple simultaneously in different threads). My solution is to give every new connection its own filesystem handler with a random and unique prefix, which keeps connections from interfering with each other, but ideally I'd like to be able to remove the attached filesystem as well once a connection is closed. Hence, this patch.

Tasklist

  • Add documentation
  • Review
  • Adjust for comments
  • All CI builds and checks have passed

Environment

Provide environment details, if relevant:

  • OS: MacOS 13.4
  • Compiler: Homebrew Clang 16.0.6

@rouault
Copy link
Member

rouault commented Nov 21, 2023

Can you add a test for that in autotest/cpp/test_cpl.cpp at the place where VSIInstallPluginHandler() is tested ?

@rouault
Copy link
Member

rouault commented Nov 21, 2023

to get formatting automatically fixed, use pre-commit. Cf https://gdal.org/development/dev_practices.html#commit-hooks

@rouault rouault added this to the 3.9.0 milestone Nov 21, 2023
@coveralls
Copy link
Collaborator

coveralls commented Nov 21, 2023

Coverage Status

coverage: 67.996% (+0.1%) from 67.869%
when pulling 0a0e701 on Maxxen:fs-remove-handler
into 7d5725c on OSGeo:master.

port/cpl_vsi.h Outdated Show resolved Hide resolved
@Maxxen
Copy link
Contributor Author

Maxxen commented Nov 22, 2023

Thanks! Will update later tonight.

@Maxxen Maxxen requested a review from rouault December 3, 2023 22:06
autotest/cpp/test_cpl.cpp Outdated Show resolved Hide resolved
@rouault rouault merged commit fba7ec4 into OSGeo:master Dec 3, 2023
31 checks passed
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