Skip to content

Conversation

vincent-uden
Copy link
Contributor

This github action should match the configuration outlined in #165 by @messense

I've tested it on my own at https://github.com/vincent-uden/dummy-trusted which uses mupdf as a dependency to check that the compilation does indeed work.

The action would publish to crates.io on each tag starting with "v", as in "v0.6.0" which adheres to the current naming scheme of versions for release.

Copy link
Collaborator

@ginnyTheCat ginnyTheCat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. It sadly doesn't unblock #165 yet though, like you stated in the linked issue, since #149 (comment) is still unresolved :/

@vincent-uden
Copy link
Contributor Author

Well, I really really would love to have some of the 0.6.0 changes so I would love to help out on #149 too if I can.

Is there anything I can do to help out on that front? I'm not that knowledgeable on FFI but I am happy to help out if possible.

Otherwise I guess I'll just update my project to use mupdf-rs as a git dependency and pin it to the most recent commit hash. Of course I'd rather not do that. But it would probably work fine for the time being.

@ginnyTheCat
Copy link
Collaborator

Is there anything I can do to help out on that front?

It would be a simple change, the main issue is a decision about the following options to be made:

  • add allow(improper_ctypes_definitions)
  • be ok with rust versions below 1.89.0 generating false positive warnings (mupdf's current implicit MSRV is 1.82.0)
  • not opaque the type for now
  • put work in to create our own opaque type

since this only affects windows (where people are likely using rustup or choco) and 1.89.0 is only the second newest version now i think the second option would be fine, i would just like to have a second opinion on this.

Otherwise I guess I'll just update my project to use mupdf-rs as a git dependency

I'm currently doing that as well and it's not great. Makes me question life ever time it clones curl again hahahahhaha

@vincent-uden
Copy link
Contributor Author

I see! For what its worth, I think the second options sounds like a good idea.

  • Warnings aren't errors, it is fine to have them (especially if mentioned in the README)
  • Mupdf-rs isn't at 1.0.0 yet. I wouldn't expect a crate which isn't even marked stable yet (according to semver) to support old version of the compiler
  • Compiling on Windows is already non-trivial, so you kinda need to inspect your toolchain regardless when building on Windows, updating to 1.89 is wayyyyy simpler than managing your MSVC version for example, which you also need to consider

@messense messense merged commit cfa33c8 into messense:main Oct 20, 2025
12 checks passed
@messense
Copy link
Owner

since this only affects windows (where people are likely using rustup or choco) and 1.89.0 is only the second newest version now i think the second option would be fine, i would just like to have a second opinion on this.

I think this is fine, not a big deal. Feel free to release 0.6.0 when you think it's ready.

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.

3 participants