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

support ocaml 4.10. remove usage of -unsafe-string #316

Closed
olafhering opened this issue Feb 15, 2020 · 10 comments
Closed

support ocaml 4.10. remove usage of -unsafe-string #316

olafhering opened this issue Feb 15, 2020 · 10 comments
Labels
effort-low issue is likely resolvable with <= 5h of effort ocaml Issue is about building on different ocaml versions (not interop of those)

Comments

@olafhering
Copy link
Contributor

[   57s] + /usr/bin/mkdir /home/abuild/rpmbuild/BUILDROOT/unison-2.51.2-60.opt_410.1.x86_64
[   57s] + cd unison-2.51.2
[   57s] + export SUSE_ASNEEDED=0
[   57s] + SUSE_ASNEEDED=0
[   57s] + NATIVE=true
[   57s] + make UISTYLE=gtk2 NATIVE=true THREADS=true
[   57s] UISTYLE = gtk2
[   57s] Building for Unix
[   57s] NATIVE = true
[   57s] THREADS = true
[   57s] STATIC = false
[   57s] OSTYPE = linux
[   57s] OSARCH = Linux
[   57s] ocamlopt: ubase/rx.mli ---> ubase/rx.cmi
[   57s] ocamlopt -g -unsafe-string -I lwt -I ubase -I system -I fsmonitor -I fsmonitor/linux -I fsmonitor/windows -I system/generic -I lwt/generic -I +lablgtk2 -c /home/abuild/rpmbuild/BUILD/unison-2.51.2/ubase/rx.mli
[   58s] ocamlopt: OCaml has been configured with -force-safe-string: -unsafe-string is not available.
```
@glondu
Copy link
Collaborator

glondu commented Feb 15, 2020

Note that I am working on a bin_prot-based fork that builds without -unsafe-string:

https://github.com/glondu/unison/tree/bin_prot

The specific commit removing -unsafe-string is:

glondu@100916a

I am waiting for the pull request #315 to be merged to propose a pull request with this commit.

@olafhering
Copy link
Contributor Author

This branch compiles in native mode, but fails in bytecode (ocaml 4.08+4.09):

[   75s] + /usr/bin/mkdir /home/abuild/rpmbuild/BUILDROOT/unison-2.51.2.87922db-63.obc_408.1.x86_64
[   75s] + cd unison-2.51.2.87922db
[   75s] + export SUSE_ASNEEDED=0
[   75s] + SUSE_ASNEEDED=0
[   75s] + NATIVE=false
[   75s] + make UISTYLE=gtk2 NATIVE=false THREADS=true
[   75s] UISTYLE = gtk2
[   75s] Building for Unix
[   75s] NATIVE = false
[   75s] THREADS = true
[   75s] STATIC = false
[   75s] OSTYPE = linux
[   75s] OSARCH = Linux
[   75s] /usr/bin/ocamlfind ocamlc: ubase/rx.mli ---> ubase/rx.cmi
[   75s] /usr/bin/ocamlfind ocamlc -g -package ppx_bin_prot -I lwt -I ubase -I system -I fsmonitor -I fsmonitor/linux -I fsmonitor/windows -I system/generic -I lwt/generic -I /usr/lib64/ocaml/lablgtk2 -custom -g -c /home/abuild/rpmbuild/BUILD/unison-2.51.2.87922db/ubase/rx.mli
[   76s] File "_none_", line 1:
[   76s] Error: Cannot load ppx_bin_prot: this object file uses unsafe features
[   76s] make: *** [Makefile.OCaml:402: ubase/rx.cmi] Error 2
[   76s] error: Bad exit status from /var/tmp/rpm-tmp.iLlRbM (%build)

@glondu
Copy link
Collaborator

glondu commented Feb 16, 2020

Error: Cannot load ppx_bin_prot: this object file uses unsafe features

Oh. But Unison itself already uses unsafe features (the Obj module is used in Proplist). Maybe an option is missing.

Nevertheless, the specific commit I was pointing at (100916a) does not depend on ppx_bin_prot... did you try it?

@olafhering
Copy link
Contributor Author

I used the entire branch. Will try to use selected commits.

@glondu
Copy link
Collaborator

glondu commented Feb 17, 2020

Will try to use selected commits.

I've pushed a branch for your convenience:

https://github.com/glondu/unison/tree/safe-string

@olafhering
Copy link
Contributor Author

https://github.com/glondu/unison/tree/safe-string

Thanks. This worked for ocaml 4.07 to 4.10. With 4.05 this branch fails:

[   76s] ocamlopt -g -I lwt -I ubase -I system -I fsmonitor -I fsmonitor/linux -I fsmonitor/windows -I system/generic -I lwt/generic -I /usr/lib64/ocaml/lablgtk2 -c /home/abuild/rpmbuild/BUILD/unison-2.51.2.100916a/lwt/generic/lwt_unix_impl.ml
[   76s] File "/home/abuild/rpmbuild/BUILD/unison-2.51.2.100916a/lwt/generic/lwt_unix_impl.ml", line 356, characters 16-33:
[   76s] Error: Unbound module Stdlib
[   76s] make: *** [Makefile.OCaml:412: lwt/generic/lwt_unix_impl.cmx] Error 2

Does it needs its own copy of lwt?

@glondu
Copy link
Collaborator

glondu commented Feb 18, 2020

With 4.05 this branch fails:

[   76s] ocamlopt -g -I lwt -I ubase -I system -I fsmonitor -I fsmonitor/linux -I fsmonitor/windows -I system/generic -I lwt/generic -I /usr/lib64/ocaml/lablgtk2 -c /home/abuild/rpmbuild/BUILD/unison-2.51.2.100916a/lwt/generic/lwt_unix_impl.ml
[   76s] File "/home/abuild/rpmbuild/BUILD/unison-2.51.2.100916a/lwt/generic/lwt_unix_impl.ml", line 356, characters 16-33:
[   76s] Error: Unbound module Stdlib
[   76s] make: *** [Makefile.OCaml:412: lwt/generic/lwt_unix_impl.cmx] Error 2

Stdlib was introduced only recently. I guess this could be fixed with stdlib-shims (or by replacing occurrences of Stdlib by Pervasives)... but why bother? I would just declare the minimal supported OCaml version to be 4.08.

Does it needs its own copy of lwt?

Unison predates lwt as a standalone project. lwt has evolved a lot since the epoch it was part of Unison, so I expect some work to adapt Unison to a modern lwt. But who knows, maybe it is trivial... Anyway, this is not my top priority for now (which is to make Unison independent of OCaml's marshalling).

@olafhering
Copy link
Contributor Author

Well, either way would probably work for me.
Once I upgrade Tumbleweed to 4.10 I will use this branch, or whatever will be available at that time. Thank you for your support.

@glondu
Copy link
Collaborator

glondu commented Jul 6, 2020

Note that I've made a pull request for this: #351

@gdt gdt added effort-low issue is likely resolvable with <= 5h of effort ocaml Issue is about building on different ocaml versions (not interop of those) labels Sep 14, 2020
@gdt
Copy link
Collaborator

gdt commented Oct 23, 2020

I believe fixes are merged and this is resolved in 2.51.3. If not, please open a new PR with rationale for what's wrong in 2.51.3/master, assuming as little reader knowledge as possible.

@gdt gdt closed this as completed Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-low issue is likely resolvable with <= 5h of effort ocaml Issue is about building on different ocaml versions (not interop of those)
Projects
None yet
Development

No branches or pull requests

3 participants