soegaard / racket-poppler

Racket bindings for Poppler (library for reading and generating pdfs)
24 stars 6 forks source link

Fix windows dependencies #13

Closed formalizm closed 1 year ago

soegaard commented 1 year ago

Thanks for the PR. I fixed the problem slightly different in the hope that racket-poppler also works on older systems.

a0b6402

soegaard commented 1 year ago

Note: I did not test this on Windows, so alert me if it doesn't work.

formalizm commented 1 year ago

Welcome.

Commits https://github.com/soegaard/racket-poppler/commit/815f19c5c51677bf29f0234b04d31673e2a52ab4 and https://github.com/soegaard/racket-poppler/commit/a0b64023790ca8306599e50c25a72c1b0504c6a1 did not work (bad syntax in ffi.rkt). The latest commit https://github.com/soegaard/racket-poppler/commit/12fe23e6f8072766e5e7926ebef8eaf6097cd26e works. It is identical to my PR, however.

The issue with empty PDFs rendered https://github.com/soegaard/racket-poppler/issues/4#issuecomment-53155274 is still there, but that's a different story.

In Racket base this library was renamed 5 years ago : https://github.com/racket/racket/blame/e8b957c556d71660c1f2c5e96897e74a8877dbaa/racket/src/native-libs/install.rkt#L37

This was included first in Racket v7.0, so to support the older distributions, it's probably the best to change dependency in info.rkt like "base" -> ["base" #:version "7.0"]. Should I make a new PR with this fix?

soegaard commented 1 year ago

Commits https://github.com/soegaard/racket-poppler/commit/815f19c5c51677bf29f0234b04d31673e2a52ab4 and https://github.com/soegaard/racket-poppler/commit/a0b64023790ca8306599e50c25a72c1b0504c6a1 did not work (bad syntax in ffi.rkt). The latest commit https://github.com/soegaard/racket-poppler/commit/12fe23e6f8072766e5e7926ebef8eaf6097cd26e works. It is identical to my PR, however.

Yes, but it's only temporary in order to fix the problem introduced in a0b6402.

Older systems might still use "libintl.8", so I prefer to find a way to supply a default rather than just changing the version number to "libintl.9".

In Racket base this library was renamed 5 years ago : https://github.com/racket/racket/blame/e8b957c556d71660c1f2c5e96897e74a8877dbaa/racket/src/native-libs/install.rkt#L37

Thanks for digging this up.

formalizm commented 1 year ago

Welcome. I think the way I suggested should work. Racket installs before 7.0 wouldn't upgrade with dependency ["base" #:version "7.0"], I guess.