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

4.06.0 breaks compatibility for users of $(ocamlc -config) in scripts to get C compilation flags #7650

Closed
vicuna opened this issue Oct 3, 2017 · 7 comments
Assignees
Milestone

Comments

@vicuna
Copy link

vicuna commented Oct 3, 2017

Original bug ID: 7650
Reporter: @gasche
Assigned to: @gasche
Status: resolved (set by @gasche on 2017-10-04T15:47:28Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.06.0 +dev/beta1/beta2/rc1
Target version: 4.06.0 +dev/beta1/beta2/rc1
Fixed in version: 4.06.0 +dev/beta1/beta2/rc1
Category: configure and build/install
Monitored by: @dbuenzli

Bug description

I'm creating a mantis issue to discuss a compatibility problem that occurs in 4.06.0+dev due to the removal of the variables bytecomp_c_compiler and native_c_compiler from the ocamlc -config output.

The PR that made this change, by Sébastien Hinderer, is #1114

#1114

The issue was raised by Andreas Hauptmann and Jeremy Yallop in

https://github.com/ocaml/ocaml/pull/1114/files#r142261040

Additional information

David Allsopp suggested to pass C files to ocamlc directly, instead of relying on this configuration variable.

Finally, please notes that various improvements were made (mostly by Sébastien) over the 4.05 and 4.06 development cycles to the configuration system, with in particular the intention to separate the C variables used by the compiler distribution and the C variables that third-party OCaml users should use. (If in the future we add more errors/warnings to the internal C compiler flags, we don't want user code to break). Whatever solution is chosen must ensure that it cleanly respects this separation. See the relevant documentation of configuration variables at

https://github.com/ocaml/ocaml/pull/911/files#diff-e2d5a00791bce9a01f99bc6fd613a39dR331

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: @gasche

Meta: I think that one reason why we forgot to consider the compatibility impact of this particular change is that there is a difference between the (ocamlc -config) variables {bytecomp,native}_c_compiler and the ./configure variable {BYTECOMP,NATIVE}_C_COMPILER. The latter are very new (they only appeared in 4.05.0), but the former have been available for a long time and correspond to the {BYTECOMP,NATIVE}CC ./configure variables.

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: @yallop

A few details on my use case for the 'native_c_compiler' configuration variable:

In some libraries the types of functions and the layout of objects can vary according to the compiler flags used. For example, the layout of 'struct stat' objects may depend on whether the macro _FILE_OFFSET_BITS is defined.

The ctypes tests involve bindings that use 'struct stat'. There are two types of C code that accesses 'struct stat' in the tests.

  1. Plain C code that forms part of a shared library. This code is compiled and linked using a plain C compiler; it doesn't involve any OCaml values.

  2. Bindings stubs generated by ctypes. These are compiled and linked using ocamlc, since they also use macros and functions that manipulate OCaml values.

Since the layout of 'struct stat' depends on the compiler flags used, it's essential that both pieces of C code are compiled with the same flags. The tests determine which C compiler+flags to use by examining the 'native_c_compiler' config variable.

There's more information in the following issue:

Test test-lwt-jobs fails on 32-bit architectures
yallop/ocaml-ctypes#402

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: aha

It's not always feasible to use ocamlopt/ocamlc as driver as suggested by dra27. You sometimes also need to build pure c libraries/executables or you have to integrate third party build instructions (e.g. autotools).
If the output of ocamlc -config isn't stable, it will further encourage the usage of hardcoded cc/gcc. It often works and will make opam's CI happy, but it is wrong under many circumstances (e.g. gcc instead of clang, cross compiling, missing flags,...).

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: @dra27

I prefix this by explicitly acknowledging that a breaking change should have been noted in the Changes file, of course.

@yallop - I haven't checked the complete detail, but looking at the use of CC in https://github.com/ocamllabs/ocaml-ctypes/blob/8e9fe43ecc4257cfc71a5f744f8708feeeb03b39/Makefile.tests#L7, it looked as though in 13/14 cases you should be able to call that via ocamlopt (given the addition of -c) and the link possibly via ocamlmklib?

I think for these others, additional fields now provided allow these things to be determined (ocamlopt_cflags, ocamlopt_cppflags)?

Also, for these kind of things when you're in a Makefile, it is better to attempt to read the original config/Makefile for the compiler, which is installed (i.e. include $(shell ocamlopt -where | tr -d '\r')/Makefile.config? That's part of the reason its installed (FlexDLL's build system does this, for example).

@aha - yes, indeed, although in many respects the newer flags should be better for that (where c_compiler is just gcc/cc/clang, etc. and the flags are in variables of their own).

I think we should update the documentation to reflect the breaking nature of the change, but I don't think we should restore the old variables - most build systems already need to know what version of OCaml they're looking at (not as long ago as one may think, you needed to know which version of OCaml it was simply in order to know if -config existed!)

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: @yallop

I don't think that ocamlopt and ocamlmklib are appropriate tools for building pure C libraries, which is what the ctypes tests need to do.

[Aside: -config has been available since 2005: https://github.com/ocaml/ocaml/commit/6c9bac39d4]

My impression is that the original PR was intended to simplify the OCaml build system, and that breaking the interface in this way was a mistake. Is that not the case? I don't see any discussion of the interface-breaking changes before the PR was merged.

@vicuna
Copy link
Author

vicuna commented Oct 3, 2017

Comment author: @yallop

I've submitted a little PR to restore the missing variables: #1393

@vicuna
Copy link
Author

vicuna commented Oct 4, 2017

Comment author: @gasche

Solved by merging Jeremy's Pull Request.

@vicuna vicuna closed this as completed Oct 4, 2017
@vicuna vicuna added this to the 4.06.0 milestone Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants