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

Use the integers package #515

Merged
merged 2 commits into from
May 16, 2017
Merged

Use the integers package #515

merged 2 commits into from
May 16, 2017

Conversation

bbc2
Copy link
Contributor

@bbc2 bbc2 commented May 10, 2017

This follows the discussion in #446 about moving Signed and Unsigned to a separate module. Until now, they had only been copied. This pull request removes them from the ctypes package and adds a dependency to the integers package.

@bbc2
Copy link
Contributor Author

bbc2 commented May 10, 2017

Some remaining points:

  • The ARM build fails. It looks like the Docker image is not provisioned with the integers package.
  • The Android build fails. It looks like ocamlfind can't find the integers package despite it being installed. I don't know why.

@yallop
Copy link
Owner

yallop commented May 10, 2017

I'm working on a fix for the ARM build now.

yallop added a commit to yallop/ocaml-ctypes-qemu-arm-base-dockerfile that referenced this pull request May 10, 2017
@yallop
Copy link
Owner

yallop commented May 10, 2017

The ARM build is fixed. I'm not sure yet what's wrong with the Android build. Perhaps the integers package needs some work to support cross-compilation.

@yallop
Copy link
Owner

yallop commented May 10, 2017

#517 should fix the problems with the Appveyor (Windows) builds.

@bbc2
Copy link
Contributor Author

bbc2 commented May 10, 2017

Cool! I tested a rebased version on a different branch and the Appveyor build succeeded.

@yallop
Copy link
Owner

yallop commented May 10, 2017

Could you rebase this branch, too, now that #517 is merged?

The Signed and Unsigned modules have been extracted to the integers
package for use without a dependency on ctypes but ctypes still hadn't
migrated to that new package.
@yallop
Copy link
Owner

yallop commented May 11, 2017

@whitequark: do you have any idea what might be needed to fix the Android build? The ctypes build logs have

# ocamlfind: Package `integers' not found

after integers has been installed successfuly. Does the integers package need some ocamlfind adjustments (e.g. with -toolchain android) to support cross-compilation?

@whitequark
Copy link
Contributor

One would think. Where's the repo of integers?

@yallop
Copy link
Owner

yallop commented May 11, 2017

@whitequark
Copy link
Contributor

@yallop Well yeah, someone would need to package integers for opam-cross-android.

@yallop
Copy link
Owner

yallop commented May 12, 2017

@whitequark
Copy link
Contributor

opam-cross-android PR merged

@yallop
Copy link
Owner

yallop commented May 12, 2017

Thanks, @whitequark.

@bbc2: I've pushed a commit (bc67d2f) to your branch which I hope fixes the Android build.

@bbc2
Copy link
Contributor Author

bbc2 commented May 13, 2017

Thank you for fixing the builds. It looks good to me now.

@yallop
Copy link
Owner

yallop commented May 16, 2017

Thank you for the contribution, @bbc2!

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