dev-cafe / autocmake

CMake plugin composer.
http://autocmake.org
BSD 3-Clause "New" or "Revised" License
41 stars 18 forks source link

[Discussion, Enhancement] Code duplication in language selection #210

Open robertodr opened 6 years ago

robertodr commented 6 years ago

I introduced the language section in autocmake.yml to obviate to the fact that enable_language and project(<name> <languages>) do not exactly generate the same build system. The consensus we reached at the time was that enable_language is better left for optionally enabling one or more languages, see for example the fc_optional.cmake file. I think, however, that I did a sloppy job, since users of Autocmake now have to have the following lines in their autocmake.yml:

language:
  - Fortran
  - C
  - CXX

modules:
- compilers:
  - source:
    - '%(url_root)modules/fc.cmake'
    - '%(url_root)modules/cc.cmake'
    - '%(url_root)modules/cxx.cmake'

The explicit inclusion of fc.cmake, cc.cmake and cxx.cmake is, in my opinion, completely redundant. It should be possible to:

bast commented 6 years ago

I agree about the redundancy but do not agree that you did a sloppy job :-) Let us automatically fetch these. Should these files still reside under modules/ and be fetched to downloaded/? In other words should the only API change be a simplification of the yaml recipe? I think so but wanted to get your view first before starting hacking.

robertodr commented 6 years ago

Yes, I agree with you. Only YAML simplification. The checks on CMAKE_<LANG>_COMPILER_WORKS should go in <lang>.cmake should go away too.

robertodr commented 6 years ago

@bast I'll try to submit a PR today.

bast commented 6 years ago

I am unsure about one thing: I had this fc_optional. This was for libraries that are mostly used out of C/C++ but sometimes used from Fortran. I have learned that Fortran is not a standard thing on Mac so I wanted to keep it optional to not force Mac people to install Fortran if the library does not need it. What is your view on this? Should we allow optional languages?

robertodr commented 6 years ago

My view is that fc_optional is fine as is. I don't think we need equivalent cc_optional and cxx_optional though.

bast commented 6 years ago

OK I agree. So if you work on this, please either make sure that fc_optional still works. But if it gets too clunky and weird, then I am not too attached to it and we can also retire it.

robertodr commented 6 years ago

I'm hitting a problem and I don't know what's the best way to solve it. So once the language(s) for the project have been read in the YAML, I generate the list of modules to automatically include. This list has to be injected into the config read from the YAML file. This means:

  1. I have to hardcode the location of these modules in the update.py script. This means that testing local changes of cc.cmake, cxx.cmake and fc.cmake will no longer be possible.
  2. I have to tamper with the contents of autocmake.yml after they've been read in, meaning that what the user provides is not exactly what will be used to generate CMakeLists.txt.

How should I/we proceed?

bast commented 6 years ago

Hmm ... difficult. I need to think more about this. Tricky. One problem of hardcoding this is that it would make it hard for people to adapt these modules (perhaps they would like to change the flags that come with these modules). Another thought is https://github.com/coderefinery/autocmake/issues/214.

Already now we have some components that are always injected: https://github.com/coderefinery/autocmake/blob/master/autocmake/generate.py#L79-L86 Perhaps we should consider the compiler setup flags as fixed.

If we consider them fixed, perhaps we should remove the lang enabling modules from modules/ and hide it inside autocmake?

Perhaps the current solution of forcing people to specify project() langs in combination of giving optional modules which handle some useful flags is not too bad?

I am not sure, just throwing here some thoughts that I need to digest myself.

bast commented 6 years ago

I always feel a bit uncomfortable with hidden actions but perhaps it makes sense here.

bast commented 6 years ago

Thinking more about this, a setup script for a C/CXX project without --cc/--cxx flags is weird so it is not completely optional. Also these are pretty standard names and we can consider them fixed I think. Let us perhaps first think what we want to be fixed and what we want to be customizable. Once we have that defined we can think about how we accommodate it.

bast commented 6 years ago

To add some confusion :-) yet another alternative could be to set LANGUAGE to NONE in project() and enable them one by one explicitly, see https://cmake.org/cmake/help/v3.0/command/project.html, quoting: "By default C and CXX are enabled if no language options are given. Specify language NONE, or use the LANGUAGES keyword and list no languages, to skip enabling any languages."

robertodr commented 6 years ago

Yes, just saw that. And now I am wondering if we should have NONE as default and do enable_language. Let's sit on this for a little longer.

bast commented 6 years ago

Agree, let's sleep on it few times to get it right.

bast commented 6 years ago

The code duplication is still a bit of a sore point and I think we agree that we don't want to keep the current state. I think both strategies are possible and I will try to summarize advantages/disadvantages:

Using NONE and enable_language

Advantages:

Disadvantages:

Listing languages in autocmake.yml which then generates the necessary CMake code

Advantages:

Disadvantages: