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

[RFC] Clean up and extend existing support for C/C++ libraries #2644

Closed
snowleopard opened this issue Sep 16, 2019 · 9 comments
Closed

[RFC] Clean up and extend existing support for C/C++ libraries #2644

snowleopard opened this issue Sep 16, 2019 · 9 comments
Assignees
Labels
proposal RFC's that are awaiting discussion to be accepted or rejected

Comments

@snowleopard
Copy link
Collaborator

snowleopard commented Sep 16, 2019

@diml, @aalekseyev and I would like to clean up and extend existing support for C/C++ libraries in Dune, which requires a way to describe the following information:

  • The C/C++ library's name, i.e. the name of the resulting .a file. This name can be used as a reference in the self_build_stubs_archive field which could probably be aliased as c_archive for simplicity. A possible further extension is to also support public_name as in OCaml libraries.
  • The set of all C/C++ files.
  • The list of command-line flags: both common, as well as C and C++ specific ones.
  • The set of include directories: again both common, as well as C and C++ specific ones.

We can describe the above using the following top-level c_library stanza:

(c_library

  (archive_name my_lib)

  (c_names file1 file2 ...)
  (cxx_names file1 file2 ...)

  (flags common_flag1 common_flag2 ...)
  (c_flags c_flag1 c_flag2 ...)
  (cxx_flags cxx_flag1 cxx_flag2 ...) 

  (include_dirs common_dir1 %{lib:another_lib} ...)
  (c_include_dirs c_dir1 c_dir2 ...)
  (cxx_include_dirs cxx_dir1 cxx_dir2 ...)

  (extra_deps extra_file1 extra_file2 ...)
)

Here is the semantics:

  • The C/C++ compiler is called on each file matching *_names in a sandbox (duplicate name matches are disallowed as in the current implementation). All flags will be passed to every invocation of the compiler; c_flags will be passed when compiling C files and cxx_flags will be passed when compiling C++ files. All the resulting objects are placed into archive_name.a file.
  • The header directories from *include_dirs will be available and tracked as dependencies. They will also appear in the command line in -I flags. Note that you can refer to public headers of other (OCaml) libraries using the %{lib:another_lib} syntax.
  • Additionally, extra_deps will be available and tracked to support ad hoc include dependencies.

One can also use the same stanza as a part of an OCaml library definition via c_stubs:

(library 

...

  (c_stubs (c_names ... ) ...)

...
)

Any comments/suggestions are welcome!


See a generalisation of this proposal in #2650.

@snowleopard snowleopard added the proposal RFC's that are awaiting discussion to be accepted or rejected label Sep 16, 2019
@snowleopard snowleopard self-assigned this Sep 16, 2019
@nojb
Copy link
Collaborator

nojb commented Sep 16, 2019

A trivial remark is that if we are to be compatible with existing convention, then the names in c_sources, cxx_sources should not contain the extension (and should be called c_names, cxx_names).

@nojb
Copy link
Collaborator

nojb commented Sep 16, 2019

Suppose that you have a C file containing #include "foo/a.h". You would not want to pass -I foo on the command line, but you want to track the dependency on foo/a.h. How would that work?

@snowleopard
Copy link
Collaborator Author

snowleopard commented Sep 16, 2019

@nojb Thanks for the comments!

the names in c_sources, cxx_sources should not contain the extension

Yes, we should definitely list this as a possible alternative. I've added this to the top post to keep all suggestions in one place. Personally, I'm more inclined to go with a more explicit and flexible approach where the user is required to provide complete file paths but let's see what others think.

Regarding your foo/a.h example: I think extra_deps does exactly what you want. You'd simply add

  (extra_deps foo/a.h)

to the c_library stanza. Let me know if I'm missing something.

@nojb
Copy link
Collaborator

nojb commented Sep 16, 2019

@nojb Thanks for the comments!

the names in c_sources, cxx_sources should not contain the extension

Yes, we should definitely list this as a possible alternative. I've added this to the top post to keep all suggestions in one place. Personally, I'm more inclined to go with a more explicit and flexible approach where the user is required to provide complete file paths but let's see what others think.

I don't think it should be an alternative, but rather it should be the way to specify sources. This is the convention we follow when specifying C/C++ sources for C stubs in OCaml libraries already. See #1788 for some background on this issue.

Regarding your foo/a.h example: I think extra_deps does exactly what you want. You'd simply add

  (extra_deps foo/a.h)

to the c_library stanza. Let me know if I'm missing something.

It doesn't seem right to have to specify dependencies manually in such a common situation. By the way, you may want to check the behaviour of (include_subdirs) for C sources; it may already handle this issue.

@aalekseyev
Copy link
Collaborator

Looks like right now dune automatically makes C compilation depend on *.h in the current directory (but not descendants). (by code inspection, I think include_subdirs does not affect it, but I'm not too confident)

@aalekseyev
Copy link
Collaborator

I was wrong, it seems that the dependency on all .h files in subdirs is tracked with include_subdirs.

@snowleopard
Copy link
Collaborator Author

I've just made some edits to the proposal:

  • Switched to the current approach where we specify c_names and cxx_names instead of complete file paths. (This is something we might revisit in future.)
  • Renamed name to archive_name.

@nojb Yes, you can use include_subdirs, as before. The new extra_deps gives you more precise control over what you'd like to include in dependencies (e.g. just a single file instead of a directory).

@ghost
Copy link

ghost commented Sep 17, 2019

It's also for special cases where header files are accessed relative to the source file directory, for instance via #include "../headers/file.h". We could avoid it entirely by enforcing stronger conventions for the C code, but this is not very practical.

@snowleopard
Copy link
Collaborator Author

I'm tempted to abandon this proposal in favour of #2650.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal RFC's that are awaiting discussion to be accepted or rejected
Projects
None yet
Development

No branches or pull requests

3 participants