Skip to content

Fix Windows and 32-bit support #146

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

Merged
merged 6 commits into from
Jul 4, 2022
Merged

Conversation

kleisauke
Copy link
Member

@kleisauke kleisauke commented Jun 27, 2022

Change summary:

  • Move FFI functions to a separate class. Reverted with commit f3086e4.
  • Split the GLib and GObject symbols into a separate string.
  • Ensure g_* functions are called via libglib-2.0-0.dll or libgobject-2.0-0.dll on Windows.
  • Fix 32-bit support, use PHP_INT_SIZE to determine GType.
  • Remove debug code in GValue::toEnum.
  • Prefer to call cast on the static \FFI class.
  • Remove Config::error() in favor of throw new Exception(), move error buffer logic to Vips\Exception.
  • Remove GsfOutputCsvQuotingMode class.
  • Remove Config::printAll, already exists as VipsObject::printAll.
  • Move Config::filenameGetFilename and Config::filenameGetOptions to Utils class.
  • General cleanups.

This was successfully tested on my Windows PC using the 64-bit libvips binaries. I also fixed 32-bit support, but for some reason this results in OOM errors (I was able to get the test suite to pass when I disabled the NewTest and WriteTest, fwiw).

(This PR got a bit bigger than I initially thought, I could split the changes if necessary)

Resolves: #144.

- Move FFI functions to a separate class.
- Split the GLib and GObject symbols into a separate string.
- Ensure `g_*` functions are called via `libglib-2.0-0.dll` or `libgobject-2.0-0.dll` on Windows.
- Fix 32-bit support, use `PHP_INT_SIZE` to determine `GType`.
- Remove debug code in `GValue::toEnum`.
- Prefer to call `cast` on the static `\FFI` class.
- Remove `Config::error()` in favor of `throw new Exception()`, move error buffer logic to `Vips\Exception`.
- Remove `GsfOutputCsvQuotingMode` class.
- Remove `Config::printAll`, already exists as `VipsObject::printAll`.
- Move `Config::filenameGetFilename` and `Config::filenameGetOptions` to Utils class.
@kleisauke
Copy link
Member Author

I've moved the FFI functions back to Config.php to make reviewing easier.

@jcupitt
Copy link
Member

jcupitt commented Jul 3, 2022

Sorry for sitting on this, I've been horribly busy this week. I'll read it now.

@jcupitt
Copy link
Member

jcupitt commented Jul 3, 2022

Oh I forgot to say, we should update the CHANGELOG and bump the version number.

@jcupitt
Copy link
Member

jcupitt commented Jul 3, 2022

This all looks great -- fixes the issue, and includes lots of useful cleanups.

I was groaning and rolling my eyes and thinking I'd need to spend half a day sorting out this linking problem, but you've done it all, and very well! Nice job.

@jcupitt jcupitt merged commit d8d0d63 into libvips:master Jul 4, 2022
@jcupitt
Copy link
Member

jcupitt commented Jul 4, 2022

... I added a short CHANGELOG note and tagged it as 2.0.3.

@kleisauke
Copy link
Member Author

Great!

BTW, v2.0.3 was tagged on commit e2874ce, but I don't see this commit present in the master branch. Is this intentional?

@kleisauke kleisauke deleted the windows-32-bit branch July 4, 2022 15:18
@jcupitt
Copy link
Member

jcupitt commented Jul 4, 2022

Oop, I always forget that git push --tags only pushes tags sigh. Thanks!

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.

php-ffi on windows fails to link to g_malloc and g_free
2 participants