Linker flags for common libraries

[lukehoffmann]

Add support for libraries in the Linker class. As per issue #7:

In order to better support different paths on different sites:

• [ ] The linker base class should provide a dictionary that maps strings (which are 'standard' library names) to a list of linker flags. E.g.

  
  'xios' -> ["-L", "/bla/bla", "-l", "morebla"] 
  'netcdf' -> ['$(nf-config --flibs)', '($nc-config --libs)`]`,

• [ ] An application than only specifies which libraries it needs to link with (and in what order), and the linker then adds the correct flags during the linking stage.
• [ ] If a library is specified that is unknown, raise an error.
• [ ] There must be function to add and remove libraries (and Fab as default would likely provide none? Or Maybe just say netcdf as an example, since this is reasonable portable). Site-specific scripts will want to modify the settings (e.g.

[lukehoffmann]

@hiker please let me know if I've misunderstood what you had in mind, or if not can you review please?

[hiker]

Looks perfect. Bring it up to the link_exe (and link_shared_object), and this is ready to go.

[hiker]

I've merged bom_masters into this and resolved the conflicts. I detected one test failure only triggered when the tests were executed in a certain order (in steps/archive) - I switched this test to use mock_c_compiler (which is always available), instead of the default in the tool box (which somehow changed when things were executed in a certain oder).

I also noticed that linesn66 and 147 are not covered by a test in test_linker:

~/work/fab/tests/unit_tests/tools$ pytest --cov-report term-missing --cov fab.tools.linker ./test_linker.py 
/home/joerg/.local/lib/python3.10/site-packages/pep8.py:110: FutureWarning: Possible nested set at position 1
  EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')
============================================================================== test session starts ===============================================================================
platform linux -- Python 3.10.12, pytest-7.4.4, pluggy-1.0.0
rootdir: /home/joerg/work/fab
configfile: pyproject.toml
plugins: pylint-0.21.0, pep8-1.0.6, flakes-4.0.5, xdist-3.2.1, anyio-3.6.2, cov-4.0.0
collected 15 items                                                                                                                                                               

test_linker.py ...............                                                                                                                                             [100%]

---------- coverage: platform linux, python 3.10.12-final-0 ----------
Name                                              Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------
/home/joerg/work/fab/source/fab/tools/linker.py      59      2    97%   66, 147
-------------------------------------------------------------------------------
TOTAL                                                59      2    97%

I don't think this is an official requirement (line 66 is covered if we run all tests in unit_tests/tools, and line 147 somewhere from unit_tests/steps), but imho this is quite useful.

Now, line 66 is actually my fault I think (unless it was a side-effect of conflicts when merging??), and looking at it, this code is actually wrong (it relies on linker._compiler to exist, which atm does not have to be the case ... though de-facto it is). With #20 this will actually be fixed, so ... just ignore line 66 for now, but covering 147 looks useful

[hiker]

I think we need two extensions:

1. We need to have a list of flags that are always added before the library names (use case: all my libraries come from spack, but for whatever reasons I need to add their path using -L ... the only option to do this atm is to add this to every library, since I can't know in which order the future applications will add the library names). For this case, I can just add the paths once, and it will always be added.
2. Similarly, it might be good to have a list of flags to be added after the libraries. Use case: when linking xios with Fortran, you need to link explicitly with the stdc++ libraries. Since there could be more than one library that is based on c++, and a linker will link a library only once, being able to always specify this at the end is very useful

[lukehoffmann]

@hiker

1. We need to have a list of flags that are always added before the library names
2. Similarly, it might be good to have a list of flags to be added after the libraries

Currently the libs part of the link() function call.

Do you want to also put these pre/post flags into the link() function call, e.g.

linker.link(..., prelib_flags=[..], libs=[..], postlib_flags[..], ...)

Or should the be properties of the linker,

linker.prelib_flags = [ ... ]
linker.postlib_flags = [ ... ]
...
linker.link(..., libs=[..], ...)

Or do you want both options?

Currently we can technically do both with

• linker.flags - always goes before libs (but also before the source files)
• the add_flags argument - these flags always go after the libs flags

But the first is configured on the linker object, while the second is passed into link().

[lukehoffmann]

I decided to go with the option of adding both arguments to the link function. So these will be in addition to linker.flags. See what you think.

[hiker]

I decided to go with the option of adding both arguments to the link function. So these will be in addition to linker.flags. See what you think.

I admit I can't decide, and am happy with either (as long as these flags are all optional, since typically you won't need them). The only thing I did notice: flags that go before the source code (linker.flags) seems pretty useless. Any 'generic' linker flag can go anywhere (e.g. to ask for a symbol map file or so, or adding a search path), but any library must come after the .o files.

[lukehoffmann]

Yeah the linker.flags is just built-in behaviour of the Tool baseclass, which adds them in the run method. We will probably not use them, but I thought I'd better add a test for them since they exist

[lukehoffmann]

No problem, as I said I was weighing up both options, and just picked one option to implement for you to review. I've switched to configure the additional flags on the Linker itself now.

[hiker]

That looks good now, thanks. I'll port our lfric build to see if any other issue pops up :)