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 "configure" operation #2753

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

add "configure" operation #2753

wants to merge 6 commits into from

Conversation

jcupitt
Copy link
Member

@jcupitt jcupitt commented Apr 11, 2022

This operation can be used to set various library config options, such
as vips_block_untrusted_set().

Having these options available as an operation makes like simpler for
bindings: they can expose them all via the usual introspection system.

Example:

pyvips.Image.configure(untrusted_block=True)

This operation can be used to set various library config options, such
as vips_block_untrusted_set().

Having these options available as an operation makes like simpler for
bindings: they can expose them all via the usual introspection system.

Example:

```python
pyvips.Image.configure(untrusted_block=True)
```
@jcupitt jcupitt marked this pull request as draft April 11, 2022 11:44
@jcupitt
Copy link
Member Author

jcupitt commented Apr 11, 2022

@kleisauke I thought this might be handy for bindings like NetVips. Rather than having to add API for every libvips configure option, we can just have a single vips operation which lets you set the config options in one place.

With this PR (for example), pyvips will instantly let you do:

pyvips.Image.configure(untrusted_block=True)

Same for lua-vips, ruby-vips, NetVips etc. And we can use the libvips arg system to safely deprecate, rename, add, extend, etc. options in future.

What do you think?

We should all the other config options too, eg concurrency, leak, etc. etc.

@jcupitt
Copy link
Member Author

jcupitt commented Apr 12, 2022

Notes:

  • Maybe this should be called options? The name configure sounds too much like autotools and compile time configuration.
  • We could allow read of settings as well as write.
  • We could deprecate all the annoying vips_concurrency_set() etc. API.
  • This adds GINT64 args, ie. signed int64s. I'm not sure all bindings support these, we should check.

kleisauke added a commit to kleisauke/net-vips that referenced this pull request Apr 12, 2022
@kleisauke
Copy link
Member

This is interesting. It could prevent a lot of DllImport's in NetVips. For example, the Cache class might be rewritten to call the single Configure operation instead.

I had a quick attempt to fix the build and add this to NetVips, see:
kleisauke@43d2daf
kleisauke/net-vips@04b0aed

Reading settings would also be nice to have. In C# this would look like this:

NetVips.Image.Configure(cacheMax: 1000, cacheMaxFiles: 1000);
NetVips.Image.Configure(out var cacheMax, out var cacheMaxFiles);
Console.WriteLine(cacheMax); // 1000
Console.WriteLine(cacheMaxFiles); // 1000

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

2 participants