From d7c66c6f81227b58caa2a3f40dcc2318e04f9d33 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Tue, 6 Aug 2024 14:21:27 +1000 Subject: [PATCH 01/21] Add simple getter for linker library flags --- source/fab/tools/linker.py | 19 ++++++++++ tests/unit_tests/tools/test_linker.py | 50 +++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 02932a18..97c154ec 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -51,6 +51,11 @@ def __init__(self, name: Optional[str] = None, self._compiler = compiler self.flags.extend(os.getenv("LDFLAGS", "").split()) + # Maintain a set of flags for common libraries. + self.env_flags = { + 'netcdf': ['$(nf-config --flibs)', '($nc-config --libs)'] + } + def check_available(self) -> bool: ''' :returns: whether the linker is available or not. We do this @@ -61,6 +66,20 @@ def check_available(self) -> bool: return super().check_available() + def get_lib_flags(self, lib: str) -> List[str]: + '''Gets the standard flags for a standard library + + :param lib: the library name + + :returns: a list of flags + + :raises RuntimeError: if lib is not recognised + ''' + try: + return self.env_flags[lib] + except KeyError: + raise RuntimeError(f"Unknown library name: '{lib}'") + def link(self, input_files: List[Path], output_file: Path, openmp: bool, add_libs: Optional[List[str]] = None) -> str: diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 772cd7ec..bac9406a 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -136,3 +136,53 @@ def test_linker_add_compiler_flag(): tool_run.assert_called_with( ['no-compiler.exe', '-some-other-flag', 'a.o', '-o', 'a.out'], capture_output=True, env=None, cwd=None, check=False) + +# The linker base class should provide a dictionary that maps strings (which are +# 'standard' library names) to a list of linker flags. E.g. +# +# 'netcdf' -> ['$(nf-config --flibs)', '($nc-config --libs)'] +def test_linker_get_lib_flags(mock_c_compiler): + '''Linker should provide a map of 'standard' library names to a list of + linker flags + ''' + linker = Linker(compiler=mock_c_compiler) + assert linker.get_lib_flags('netcdf') == ['$(nf-config --flibs)', '($nc-config --libs)'] + + +# If a library is specified that is unknown, raise an error. +def test_linker_get_lib_flags_unknown(mock_c_compiler): + '''Linker should raise an error if flags are requested for a library that is + unknown + ''' + linker = Linker(compiler=mock_c_compiler) + with pytest.raises(RuntimeError) as err: + linker.get_lib_flags('unknown') + assert "Unknown library name" in str(err.value) + + +# # 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 +# def test_linker_add_library_flags(mock_c_compiler): +# linker = Linker(compiler=mock_c_compiler) +# linker.add_lib_flags('xios', ['-L', 'xios/lib', '-l', 'xios/lib']) + + +# # An application then 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. +# def test_linker_add_compiler_flags_by_lib(mock_c_compiler): +# '''Make sure linker flags are added for the required libraries: +# ''' +# linker = Linker(compiler=mock_c_compiler) +# linker.add_lib_flags('xios', ['-flag', '/xios/flag']) +# linker.add_lib_flags('netcdf', ['$(nf-config --flibs)', '($nc-config --libs)']) +# mock_result = mock.Mock(returncode=0) +# with mock.patch('fab.tools.tool.subprocess.run', +# return_value=mock_result) as tool_run: +# linker.link([Path("a.o")], Path("a.out"), libs=['xios'], openmp=False) +# tool_run.assert_called_with( +# ['mock_c_compiler.exe', '-flag', '/xios/flag', 'a.o', '-o', 'a.out'], +# capture_output=True, env=None, cwd=None, check=False) + + From 0340a27c44963fccdeb5bf8afbd911c58b9a5f45 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Wed, 21 Aug 2024 14:22:40 +1000 Subject: [PATCH 02/21] Add getter for linker flags by library --- source/fab/tools/linker.py | 3 +++ tests/unit_tests/tools/test_linker.py | 20 +++++++++++++------- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 97c154ec..af552218 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -80,6 +80,9 @@ def get_lib_flags(self, lib: str) -> List[str]: except KeyError: raise RuntimeError(f"Unknown library name: '{lib}'") + def add_lib_flags(self, lib: str, flags: List[str]): + self.env_flags[lib] = flags + def link(self, input_files: List[Path], output_file: Path, openmp: bool, add_libs: Optional[List[str]] = None) -> str: diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index bac9406a..d1b91812 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -146,7 +146,8 @@ def test_linker_get_lib_flags(mock_c_compiler): linker flags ''' linker = Linker(compiler=mock_c_compiler) - assert linker.get_lib_flags('netcdf') == ['$(nf-config --flibs)', '($nc-config --libs)'] + result = linker.get_lib_flags('netcdf') + assert result == ['$(nf-config --flibs)', '($nc-config --libs)'] # If a library is specified that is unknown, raise an error. @@ -160,12 +161,17 @@ def test_linker_get_lib_flags_unknown(mock_c_compiler): assert "Unknown library name" in str(err.value) -# # 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 -# def test_linker_add_library_flags(mock_c_compiler): -# linker = Linker(compiler=mock_c_compiler) -# linker.add_lib_flags('xios', ['-L', 'xios/lib', '-l', 'xios/lib']) +# 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 +def test_linker_add_library_flags(mock_c_compiler): + '''Linker should provide a way to add a new set of flags for a library ''' + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags('xios', ['-L', 'xios/lib', '-I', 'xios/inc']) + + # Make sure we can get it back. The order should be maintained. + result = linker.get_lib_flags('xios') + assert result == ['-L', 'xios/lib', '-I', 'xios/inc'] # # An application then only specifies which libraries it needs to link with (and From 4cc54854d6271b68ef6e37e6724cd12ea3777799 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Wed, 21 Aug 2024 14:48:12 +1000 Subject: [PATCH 03/21] Fix formatting --- source/fab/tools/linker.py | 14 +++++++++++ tests/unit_tests/tools/test_linker.py | 36 +++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index af552218..d21da5bb 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -81,8 +81,22 @@ def get_lib_flags(self, lib: str) -> List[str]: raise RuntimeError(f"Unknown library name: '{lib}'") def add_lib_flags(self, lib: str, flags: List[str]): + '''Add a set of flags for a standard library + + :param lib: the library name + :param flags: the flags to use with the library + + ''' self.env_flags[lib] = flags + def remove_lib_flags(self, lib: str): + '''Add a set of flags for a standard library + + :param lib: the library name + + ''' + del self.env_flags[lib] + def link(self, input_files: List[Path], output_file: Path, openmp: bool, add_libs: Optional[List[str]] = None) -> str: diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index d1b91812..4b2b36c6 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -137,10 +137,13 @@ def test_linker_add_compiler_flag(): ['no-compiler.exe', '-some-other-flag', 'a.o', '-o', 'a.out'], capture_output=True, env=None, cwd=None, check=False) + # The linker base class should provide a dictionary that maps strings (which are # 'standard' library names) to a list of linker flags. E.g. # # 'netcdf' -> ['$(nf-config --flibs)', '($nc-config --libs)'] +# (and Fab as default would likely provide none? Or Maybe just say netcdf as an +# example, since this is reasonable portable). def test_linker_get_lib_flags(mock_c_compiler): '''Linker should provide a map of 'standard' library names to a list of linker flags @@ -161,9 +164,7 @@ def test_linker_get_lib_flags_unknown(mock_c_compiler): assert "Unknown library name" in str(err.value) -# 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 +# There must be function to add and remove libraries def test_linker_add_library_flags(mock_c_compiler): '''Linker should provide a way to add a new set of flags for a library ''' linker = Linker(compiler=mock_c_compiler) @@ -174,6 +175,33 @@ def test_linker_add_library_flags(mock_c_compiler): assert result == ['-L', 'xios/lib', '-I', 'xios/inc'] +# There must be function to remove libraries +def test_linker_remove_library_flags(mock_c_compiler): + '''Linker should provide a way to add a new set of flags for a library ''' + linker = Linker(compiler=mock_c_compiler) + linker.remove_lib_flags('netcdf') + + with pytest.raises(RuntimeError) as err: + linker.get_lib_flags('netcdf') + assert "Unknown library name" in str(err.value) + + +# Site-specific scripts will want to modify the settings +def test_linker_replace_library_flags(mock_c_compiler): + '''Linker should provide a way to replace the default flags for a library ''' + linker = Linker(compiler=mock_c_compiler) + # Initially we have the default netcdf flags + result = linker.get_lib_flags('netcdf') + assert result == ['$(nf-config --flibs)', '($nc-config --libs)'] + + # Replace them with another set of flags. + linker.add_lib_flags('netcdf', ['-L', 'netcdf/lib', '-I', 'netcdf/inc']) + + # Test that we can get our custom flags back + result = linker.get_lib_flags('netcdf') + assert result == ['-L', 'netcdf/lib', '-I', 'netcdf/inc'] + + # # An application then 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. @@ -190,5 +218,3 @@ def test_linker_add_library_flags(mock_c_compiler): # tool_run.assert_called_with( # ['mock_c_compiler.exe', '-flag', '/xios/flag', 'a.o', '-o', 'a.out'], # capture_output=True, env=None, cwd=None, check=False) - - From f017eed7bf8853551b54987f257c12315f41abe7 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Wed, 21 Aug 2024 17:56:22 +1000 Subject: [PATCH 04/21] Add optional libs argument to link function --- source/fab/steps/link.py | 4 +-- source/fab/tools/linker.py | 23 +++++++++------ tests/unit_tests/tools/test_linker.py | 41 +++++++++++++++------------ 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index 78146ef6..c1381df5 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -65,7 +65,7 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None): target_objects = source_getter(config.artefact_store) for root, objects in target_objects.items(): exe_path = config.project_workspace / f'{root}' - linker.link(objects, exe_path, openmp=config.openmp, add_libs=flags) + linker.link(objects, exe_path, openmp=config.openmp, add_flags=flags) config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path) @@ -115,4 +115,4 @@ def link_shared_object(config, output_fpath: str, flags=None, objects = target_objects[None] out_name = Template(output_fpath).substitute(output=config.build_output) - linker.link(objects, out_name, openmp=config.openmp, add_libs=flags) + linker.link(objects, out_name, openmp=config.openmp, add_flags=flags) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index d21da5bb..525debc7 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -52,7 +52,7 @@ def __init__(self, name: Optional[str] = None, self.flags.extend(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. - self.env_flags = { + self.lib_flags = { 'netcdf': ['$(nf-config --flibs)', '($nc-config --libs)'] } @@ -76,7 +76,7 @@ def get_lib_flags(self, lib: str) -> List[str]: :raises RuntimeError: if lib is not recognised ''' try: - return self.env_flags[lib] + return self.lib_flags[lib] except KeyError: raise RuntimeError(f"Unknown library name: '{lib}'") @@ -87,7 +87,7 @@ def add_lib_flags(self, lib: str, flags: List[str]): :param flags: the flags to use with the library ''' - self.env_flags[lib] = flags + self.lib_flags[lib] = flags def remove_lib_flags(self, lib: str): '''Add a set of flags for a standard library @@ -95,18 +95,23 @@ def remove_lib_flags(self, lib: str): :param lib: the library name ''' - del self.env_flags[lib] + try: + del self.lib_flags[lib] + except KeyError: + pass def link(self, input_files: List[Path], output_file: Path, openmp: bool, - add_libs: Optional[List[str]] = None) -> str: + add_libs: Optional[List[str]] = None, + add_flags: Optional[List[str]] = None) -> str: '''Executes the linker with the specified input files, creating `output_file`. :param input_files: list of input files to link. :param output_file: output file. :param openm: whether OpenMP is requested or not. - :param add_libs: additional linker flags. + :param add_libs: additional libraries to link with. + :param add_flags: additional linker flags. :returns: the stdout of the link command ''' @@ -119,7 +124,9 @@ def link(self, input_files: List[Path], output_file: Path, params = [] # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) - if add_libs: - params += add_libs + for lib in add_libs or []: + params += self.lib_flags[lib] + if add_flags: + params += add_flags params.extend([self._output_flag, str(output_file)]) return self.run(params) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 4b2b36c6..791d517a 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -100,7 +100,7 @@ def test_linker_c_with_libraries(mock_c_compiler): '''Test the link command line when additional libraries are specified.''' linker = Linker(compiler=mock_c_compiler) with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), add_libs=["-L", "/tmp"], + linker.link([Path("a.o")], Path("a.out"), add_flags=["-L", "/tmp"], openmp=True) link_run.assert_called_with(['-fopenmp', 'a.o', '-L', '/tmp', '-o', 'a.out']) @@ -177,7 +177,7 @@ def test_linker_add_library_flags(mock_c_compiler): # There must be function to remove libraries def test_linker_remove_library_flags(mock_c_compiler): - '''Linker should provide a way to add a new set of flags for a library ''' + '''Linker should provide a way to remove the flags for a library ''' linker = Linker(compiler=mock_c_compiler) linker.remove_lib_flags('netcdf') @@ -186,6 +186,12 @@ def test_linker_remove_library_flags(mock_c_compiler): assert "Unknown library name" in str(err.value) +def test_linker_remove_unknown_library_flags(mock_c_compiler): + '''Linker should allow removing flags for unknown library without error''' + linker = Linker(compiler=mock_c_compiler) + linker.remove_lib_flags('unkown') + + # Site-specific scripts will want to modify the settings def test_linker_replace_library_flags(mock_c_compiler): '''Linker should provide a way to replace the default flags for a library ''' @@ -202,19 +208,18 @@ def test_linker_replace_library_flags(mock_c_compiler): assert result == ['-L', 'netcdf/lib', '-I', 'netcdf/inc'] -# # An application then 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. -# def test_linker_add_compiler_flags_by_lib(mock_c_compiler): -# '''Make sure linker flags are added for the required libraries: -# ''' -# linker = Linker(compiler=mock_c_compiler) -# linker.add_lib_flags('xios', ['-flag', '/xios/flag']) -# linker.add_lib_flags('netcdf', ['$(nf-config --flibs)', '($nc-config --libs)']) -# mock_result = mock.Mock(returncode=0) -# with mock.patch('fab.tools.tool.subprocess.run', -# return_value=mock_result) as tool_run: -# linker.link([Path("a.o")], Path("a.out"), libs=['xios'], openmp=False) -# tool_run.assert_called_with( -# ['mock_c_compiler.exe', '-flag', '/xios/flag', 'a.o', '-o', 'a.out'], -# capture_output=True, env=None, cwd=None, check=False) +# An application then 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. +def test_linker_add_compiler_flags_by_lib(mock_c_compiler): + '''Make sure linker flags are added for the required libraries: + ''' + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags('xios', ['-flag', '/xios/flag']) + mock_result = mock.Mock(returncode=0) + with mock.patch('fab.tools.tool.subprocess.run', + return_value=mock_result) as tool_run: + linker.link([Path("a.o")], Path("a.out"), add_libs=['xios'], openmp=False) + tool_run.assert_called_with( + ['mock_c_compiler.exe', 'a.o', '-flag', '/xios/flag', '-o', 'a.out'], + capture_output=True, env=None, cwd=None, check=False) From 4f62bfda5cde623e5c599070ea37df22dcb2f3c0 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 22 Aug 2024 15:52:05 +1000 Subject: [PATCH 05/21] Reorder and clean up linker tests --- source/fab/tools/linker.py | 19 +-- tests/unit_tests/tools/test_linker.py | 203 +++++++++++++------------- 2 files changed, 110 insertions(+), 112 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 525debc7..f385eb75 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -52,7 +52,7 @@ def __init__(self, name: Optional[str] = None, self.flags.extend(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. - self.lib_flags = { + self._lib_flags = { 'netcdf': ['$(nf-config --flibs)', '($nc-config --libs)'] } @@ -76,7 +76,7 @@ def get_lib_flags(self, lib: str) -> List[str]: :raises RuntimeError: if lib is not recognised ''' try: - return self.lib_flags[lib] + return self._lib_flags[lib] except KeyError: raise RuntimeError(f"Unknown library name: '{lib}'") @@ -87,7 +87,8 @@ def add_lib_flags(self, lib: str, flags: List[str]): :param flags: the flags to use with the library ''' - self.lib_flags[lib] = flags + # Make a copy to avoid modifying the caller's list + self._lib_flags[lib] = flags[:] def remove_lib_flags(self, lib: str): '''Add a set of flags for a standard library @@ -96,13 +97,13 @@ def remove_lib_flags(self, lib: str): ''' try: - del self.lib_flags[lib] + del self._lib_flags[lib] except KeyError: pass def link(self, input_files: List[Path], output_file: Path, openmp: bool, - add_libs: Optional[List[str]] = None, + libs: Optional[List[str]] = None, add_flags: Optional[List[str]] = None) -> str: '''Executes the linker with the specified input files, creating `output_file`. @@ -110,7 +111,7 @@ def link(self, input_files: List[Path], output_file: Path, :param input_files: list of input files to link. :param output_file: output file. :param openm: whether OpenMP is requested or not. - :param add_libs: additional libraries to link with. + :param libs: additional libraries to link with. :param add_flags: additional linker flags. :returns: the stdout of the link command @@ -124,9 +125,9 @@ def link(self, input_files: List[Path], output_file: Path, params = [] # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) - for lib in add_libs or []: - params += self.lib_flags[lib] + for lib in libs or []: + params.extend(self._lib_flags[lib]) if add_flags: - params += add_flags + params.extend(add_flags) params.extend([self._output_flag, str(output_file)]) return self.run(params) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 791d517a..ab1accf1 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -18,8 +18,7 @@ def test_linker(mock_c_compiler, mock_fortran_compiler): '''Test the linker constructor.''' - linker = Linker(name="my_linker", exec_name="my_linker.exe", - suite="suite") + linker = Linker(name="my_linker", exec_name="my_linker.exe", suite="suite") assert linker.category == Category.LINKER assert linker.name == "my_linker" assert linker.exec_name == "my_linker.exe" @@ -83,34 +82,119 @@ def test_linker_check_available(mock_c_compiler): assert linker.check_available() is False +# ==================== +# Managing lib flags: +# ==================== +def test_linker_get_lib_flags(mock_c_compiler): + """Linker should provide a map of library names, each leading to a list of + linker flags + """ + linker = Linker(compiler=mock_c_compiler) + # netcdf is built in to the linker, since it is reasonable portable + result = linker.get_lib_flags("netcdf") + assert result == ["$(nf-config --flibs)", "($nc-config --libs)"] + + +def test_linker_get_lib_flags_unknown(mock_c_compiler): + """Linker should raise an error if flags are requested for a library that is + unknown + """ + linker = Linker(compiler=mock_c_compiler) + with pytest.raises(RuntimeError) as err: + linker.get_lib_flags("unknown") + assert "Unknown library name" in str(err.value) + + +def test_linker_add_lib_flags(mock_c_compiler): + """Linker should provide a way to add a new set of flags for a library""" + linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("xios", ["-L", "xios/lib", "-I", "xios/inc"]) + + # Make sure we can get it back. The order should be maintained. + result = linker.get_lib_flags("xios") + assert result == ["-L", "xios/lib", "-I", "xios/inc"] + + +def test_linker_add_lib_flags_overwrite(mock_c_compiler): + """Linker should provide a way to replace the default flags for a library""" + linker = Linker(compiler=mock_c_compiler) + + # Initially we have the default netcdf flags + result = linker.get_lib_flags("netcdf") + assert result == ["$(nf-config --flibs)", "($nc-config --libs)"] + + # Replace them with another set of flags. + linker.add_lib_flags("netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) + + # Test that we can see our custom flags + result = linker.get_lib_flags("netcdf") + assert result == ["-L", "netcdf/lib", "-I", "netcdf/inc"] + + +def test_linker_remove_lib_flags(mock_c_compiler): + """Linker should provide a way to remove the flags for a library""" + linker = Linker(compiler=mock_c_compiler) + linker.remove_lib_flags("netcdf") + + with pytest.raises(RuntimeError) as err: + linker.get_lib_flags("netcdf") + assert "Unknown library name" in str(err.value) + + +def test_linker_remove_lib_flags_unknown(mock_c_compiler): + """Linker should silently allow removing flags for unknown library""" + linker = Linker(compiler=mock_c_compiler) + linker.remove_lib_flags("unkown") + + +# ==================== +# Linking: +# ==================== def test_linker_c(mock_c_compiler): - '''Test the link command line when no additional libraries are - specified.''' + '''Test the link command line when no additional libraries are specified.''' linker = Linker(compiler=mock_c_compiler) mock_result = mock.Mock(returncode=0) with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: linker.link([Path("a.o")], Path("a.out"), openmp=False) tool_run.assert_called_with( - ["mock_c_compiler.exe", 'a.o', '-o', 'a.out'], capture_output=True, - env=None, cwd=None, check=False) + ["mock_c_compiler.exe", "a.o", "-o", "a.out"], + capture_output=True, env=None, cwd=None, check=False) def test_linker_c_with_libraries(mock_c_compiler): - '''Test the link command line when additional libraries are specified.''' + """Test the link command line when additional libraries are specified.""" + linker = Linker(compiler=mock_c_compiler) + with mock.patch.object(linker, "run") as link_run: + linker.link([Path("a.o")], Path("a.out"), libs=["netcdf"], openmp=True) + link_run.assert_called_with( + ["-fopenmp", "a.o", + "$(nf-config --flibs)", "($nc-config --libs)", + "-o", "a.out"] + ) + + +def test_linker_c_with_custom_libraries(mock_c_compiler): + """Test the link command line when additional libraries are specified.""" linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), add_flags=["-L", "/tmp"], - openmp=True) - link_run.assert_called_with(['-fopenmp', 'a.o', '-L', '/tmp', - '-o', 'a.out']) + linker.link([Path("a.o")], Path("a.out"), + libs=["customlib", "netcdf"], openmp=True) + # The order of the 'libs' list should be maintained + link_run.assert_called_with( + ["-fopenmp", "a.o", + "-q", "/tmp", "-j", + "$(nf-config --flibs)", "($nc-config --libs)", + "-o", "a.out"] + ) def test_compiler_linker_add_compiler_flag(mock_c_compiler): '''Test that a flag added to the compiler will be automatically - added to the link line (even if the flags are modified after - creating the linker ... in case that the user specifies additional - flags after creating the linker).''' + added to the link line (even if the flags are modified after creating the + linker ... in case that the user specifies additional flags after creating + the linker).''' linker = Linker(compiler=mock_c_compiler) mock_c_compiler.flags.append("-my-flag") @@ -124,8 +208,8 @@ def test_compiler_linker_add_compiler_flag(mock_c_compiler): def test_linker_add_compiler_flag(): - '''Make sure linker flags work if a linker is created without - a compiler: + '''Make sure ad-hoc linker flags work if a linker is created without a + compiler: ''' linker = Linker("no-compiler", "no-compiler.exe", "suite") linker.flags.append("-some-other-flag") @@ -136,90 +220,3 @@ def test_linker_add_compiler_flag(): tool_run.assert_called_with( ['no-compiler.exe', '-some-other-flag', 'a.o', '-o', 'a.out'], capture_output=True, env=None, cwd=None, check=False) - - -# The linker base class should provide a dictionary that maps strings (which are -# 'standard' library names) to a list of linker flags. E.g. -# -# 'netcdf' -> ['$(nf-config --flibs)', '($nc-config --libs)'] -# (and Fab as default would likely provide none? Or Maybe just say netcdf as an -# example, since this is reasonable portable). -def test_linker_get_lib_flags(mock_c_compiler): - '''Linker should provide a map of 'standard' library names to a list of - linker flags - ''' - linker = Linker(compiler=mock_c_compiler) - result = linker.get_lib_flags('netcdf') - assert result == ['$(nf-config --flibs)', '($nc-config --libs)'] - - -# If a library is specified that is unknown, raise an error. -def test_linker_get_lib_flags_unknown(mock_c_compiler): - '''Linker should raise an error if flags are requested for a library that is - unknown - ''' - linker = Linker(compiler=mock_c_compiler) - with pytest.raises(RuntimeError) as err: - linker.get_lib_flags('unknown') - assert "Unknown library name" in str(err.value) - - -# There must be function to add and remove libraries -def test_linker_add_library_flags(mock_c_compiler): - '''Linker should provide a way to add a new set of flags for a library ''' - linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags('xios', ['-L', 'xios/lib', '-I', 'xios/inc']) - - # Make sure we can get it back. The order should be maintained. - result = linker.get_lib_flags('xios') - assert result == ['-L', 'xios/lib', '-I', 'xios/inc'] - - -# There must be function to remove libraries -def test_linker_remove_library_flags(mock_c_compiler): - '''Linker should provide a way to remove the flags for a library ''' - linker = Linker(compiler=mock_c_compiler) - linker.remove_lib_flags('netcdf') - - with pytest.raises(RuntimeError) as err: - linker.get_lib_flags('netcdf') - assert "Unknown library name" in str(err.value) - - -def test_linker_remove_unknown_library_flags(mock_c_compiler): - '''Linker should allow removing flags for unknown library without error''' - linker = Linker(compiler=mock_c_compiler) - linker.remove_lib_flags('unkown') - - -# Site-specific scripts will want to modify the settings -def test_linker_replace_library_flags(mock_c_compiler): - '''Linker should provide a way to replace the default flags for a library ''' - linker = Linker(compiler=mock_c_compiler) - # Initially we have the default netcdf flags - result = linker.get_lib_flags('netcdf') - assert result == ['$(nf-config --flibs)', '($nc-config --libs)'] - - # Replace them with another set of flags. - linker.add_lib_flags('netcdf', ['-L', 'netcdf/lib', '-I', 'netcdf/inc']) - - # Test that we can get our custom flags back - result = linker.get_lib_flags('netcdf') - assert result == ['-L', 'netcdf/lib', '-I', 'netcdf/inc'] - - -# An application then 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. -def test_linker_add_compiler_flags_by_lib(mock_c_compiler): - '''Make sure linker flags are added for the required libraries: - ''' - linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags('xios', ['-flag', '/xios/flag']) - mock_result = mock.Mock(returncode=0) - with mock.patch('fab.tools.tool.subprocess.run', - return_value=mock_result) as tool_run: - linker.link([Path("a.o")], Path("a.out"), add_libs=['xios'], openmp=False) - tool_run.assert_called_with( - ['mock_c_compiler.exe', 'a.o', '-flag', '/xios/flag', '-o', 'a.out'], - capture_output=True, env=None, cwd=None, check=False) From 1caf11089b13a11b36b073939b908f4df96dcecd Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 22 Aug 2024 16:17:38 +1000 Subject: [PATCH 06/21] Make sure `Linker.link()` raises for unknown lib --- source/fab/tools/linker.py | 16 +++++++++------- tests/unit_tests/tools/test_linker.py | 24 ++++++++++++++++++------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index f385eb75..0bb739f4 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -52,9 +52,12 @@ def __init__(self, name: Optional[str] = None, self.flags.extend(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. - self._lib_flags = { - 'netcdf': ['$(nf-config --flibs)', '($nc-config --libs)'] - } + self._lib_flags = {} + # Include netcdf as an example, since it is reasonable portable + self.add_lib_flags( + 'netcdf', + ['$(nf-config --flibs)', '($nc-config --libs)'] + ) def check_available(self) -> bool: ''' @@ -85,7 +88,6 @@ def add_lib_flags(self, lib: str, flags: List[str]): :param lib: the library name :param flags: the flags to use with the library - ''' # Make a copy to avoid modifying the caller's list self._lib_flags[lib] = flags[:] @@ -94,7 +96,6 @@ def remove_lib_flags(self, lib: str): '''Add a set of flags for a standard library :param lib: the library name - ''' try: del self._lib_flags[lib] @@ -125,8 +126,9 @@ def link(self, input_files: List[Path], output_file: Path, params = [] # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) - for lib in libs or []: - params.extend(self._lib_flags[lib]) + + for lib in (libs or []): + params.extend(self.get_lib_flags(lib)) if add_flags: params.extend(add_flags) params.extend([self._output_flag, str(output_file)]) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index ab1accf1..1e38d797 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -102,7 +102,7 @@ def test_linker_get_lib_flags_unknown(mock_c_compiler): linker = Linker(compiler=mock_c_compiler) with pytest.raises(RuntimeError) as err: linker.get_lib_flags("unknown") - assert "Unknown library name" in str(err.value) + assert "Unknown library name: 'unknown'" in str(err.value) def test_linker_add_lib_flags(mock_c_compiler): @@ -138,7 +138,7 @@ def test_linker_remove_lib_flags(mock_c_compiler): with pytest.raises(RuntimeError) as err: linker.get_lib_flags("netcdf") - assert "Unknown library name" in str(err.value) + assert "Unknown library name: 'netcdf'" in str(err.value) def test_linker_remove_lib_flags_unknown(mock_c_compiler): @@ -170,8 +170,7 @@ def test_linker_c_with_libraries(mock_c_compiler): link_run.assert_called_with( ["-fopenmp", "a.o", "$(nf-config --flibs)", "($nc-config --libs)", - "-o", "a.out"] - ) + "-o", "a.out"]) def test_linker_c_with_custom_libraries(mock_c_compiler): @@ -186,8 +185,21 @@ def test_linker_c_with_custom_libraries(mock_c_compiler): ["-fopenmp", "a.o", "-q", "/tmp", "-j", "$(nf-config --flibs)", "($nc-config --libs)", - "-o", "a.out"] - ) + "-o", "a.out"]) + + +def test_linker_c_with_unknown_library(mock_c_compiler): + """Test the link command raises an error when unknow libraries are + specified. + """ + linker = Linker(compiler=mock_c_compiler)\ + + with pytest.raises(RuntimeError) as err: + # Try to use "customlib" when we haven't added it to the linker + linker.link([Path("a.o")], Path("a.out"), + libs=["netcdf", "customlib"], openmp=True) + + assert "Unknown library name: 'customlib'" in str(err.value) def test_compiler_linker_add_compiler_flag(mock_c_compiler): From dd81dbec06bd4b8944a32702bbf3da0f8d356c19 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 22 Aug 2024 16:23:12 +1000 Subject: [PATCH 07/21] Add missing type --- source/fab/tools/linker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 0bb739f4..6b62fd45 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -52,7 +52,7 @@ def __init__(self, name: Optional[str] = None, self.flags.extend(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. - self._lib_flags = {} + self._lib_flags: dict[str, List[str]] = {} # Include netcdf as an example, since it is reasonable portable self.add_lib_flags( 'netcdf', From 8d72ccdf9b5bd684ed3314074679ceaf4bafbb1c Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 22 Aug 2024 16:26:03 +1000 Subject: [PATCH 08/21] Fix typing error --- source/fab/tools/linker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 6b62fd45..5b8b474b 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -9,7 +9,7 @@ import os from pathlib import Path -from typing import cast, List, Optional +from typing import cast, Dict, List, Optional from fab.tools.category import Category from fab.tools.compiler import Compiler @@ -52,7 +52,7 @@ def __init__(self, name: Optional[str] = None, self.flags.extend(os.getenv("LDFLAGS", "").split()) # Maintain a set of flags for common libraries. - self._lib_flags: dict[str, List[str]] = {} + self._lib_flags: Dict[str, List[str]] = {} # Include netcdf as an example, since it is reasonable portable self.add_lib_flags( 'netcdf', From b3021d605360cbce863d4716489e0242204ec3ee Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 29 Aug 2024 16:24:45 +1000 Subject: [PATCH 09/21] Add 'libs' argument to link_exe function --- source/fab/steps/link.py | 15 +++++++++++---- tests/unit_tests/steps/test_link.py | 8 ++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index c1381df5..710bac72 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -9,7 +9,7 @@ """ import logging from string import Template -from typing import Optional +from typing import Optional, Union from fab.artefacts import ArtefactSet from fab.steps import step @@ -33,7 +33,10 @@ def __call__(self, artefact_store): @step -def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None): +def link_exe(config, + libs: Union[list[str], None] = None, + flags: Union[list[str], None] = None, + source: Optional[ArtefactsGetter] = None): """ Link object files into an executable for every build target. @@ -49,8 +52,10 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None): The :class:`fab.build_config.BuildConfig` object where we can read settings such as the project workspace folder or the multiprocessing flag. + :param libs: + A list of required library names to pass to the linker. :param flags: - A list of flags to pass to the linker. + A list of additional flags to pass to the linker. :param source: An optional :class:`~fab.artefacts.ArtefactsGetter`. It defaults to the output from compiler steps, which typically is the expected behaviour. @@ -59,13 +64,15 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None): linker = config.tool_box.get_tool(Category.LINKER, config.mpi) logger.info(f'Linker is {linker.name}') + libs = libs or [] flags = flags or [] source_getter = source or DefaultLinkerSource() target_objects = source_getter(config.artefact_store) for root, objects in target_objects.items(): exe_path = config.project_workspace / f'{root}' - linker.link(objects, exe_path, openmp=config.openmp, add_flags=flags) + linker.link(objects, exe_path, openmp=config.openmp, libs=libs, + add_flags=flags) config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path) diff --git a/tests/unit_tests/steps/test_link.py b/tests/unit_tests/steps/test_link.py index a675f54c..a20c4ff4 100644 --- a/tests/unit_tests/steps/test_link.py +++ b/tests/unit_tests/steps/test_link.py @@ -34,6 +34,9 @@ def test_run(self, tool_box): linker = Linker("mock_link", "mock_link.exe", "mock-vendor") # Mark the linker as available to it can be added to the tool box linker._is_available = True + + # Add a custom library to the linker + linker.add_lib_flags('mylib', ['-L/my/lib', '-mylib']) tool_box.add_tool(linker, silent_replace=True) mock_result = mock.Mock(returncode=0, stdout="abc\ndef".encode()) with mock.patch('fab.tools.tool.subprocess.run', @@ -41,9 +44,10 @@ def test_run(self, tool_box): pytest.warns(UserWarning, match="_metric_send_conn not " "set, cannot send metrics"): - link_exe(config, flags=['-fooflag', '-barflag']) + link_exe(config, libs=['mylib'], flags=['-fooflag', '-barflag']) tool_run.assert_called_with( ['mock_link.exe', '-L/foo1/lib', '-L/foo2/lib', 'bar.o', 'foo.o', - '-fooflag', '-barflag', '-o', 'workspace/foo'], + '-L/my/lib', '-mylib', '-fooflag', '-barflag', + '-o', 'workspace/foo'], capture_output=True, env=None, cwd=None, check=False) From c9b4183d08182e92c4fd07ab7c979cb4abe63860 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Thu, 29 Aug 2024 17:54:35 +1000 Subject: [PATCH 10/21] Try to add documentation for the linker libs feature --- Documentation/source/advanced_config.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/source/advanced_config.rst b/Documentation/source/advanced_config.rst index faaf0b25..d2b54ebf 100644 --- a/Documentation/source/advanced_config.rst +++ b/Documentation/source/advanced_config.rst @@ -193,6 +193,14 @@ to specify 3rd party libraries at the link stage. link_exe(state, flags=['-lm', '-lnetcdf']) +Linkers will be pre-configured with flags for common libraries. Where possible, +a library name should be used to include the required flags for linking. + +.. code-block:: + :linenos: + + link_exe(state, libs=['netcdf']) + Path-specific flags ------------------- From 390a2fb824b669706f9d6e6b5cf4e3e1c92c5a21 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Fri, 30 Aug 2024 11:17:35 +1000 Subject: [PATCH 11/21] Use correct list type in link_exe hint --- source/fab/steps/link.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index 710bac72..7924d782 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -9,7 +9,7 @@ """ import logging from string import Template -from typing import Optional, Union +from typing import List, Optional, Union from fab.artefacts import ArtefactSet from fab.steps import step @@ -34,8 +34,8 @@ def __call__(self, artefact_store): @step def link_exe(config, - libs: Union[list[str], None] = None, - flags: Union[list[str], None] = None, + libs: Union[List[str], None] = None, + flags: Union[List[str], None] = None, source: Optional[ArtefactsGetter] = None): """ Link object files into an executable for every build target. From d8c9db19fb68f9b720c407afe8933c65f2ea23d6 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Fri, 30 Aug 2024 17:32:40 +1000 Subject: [PATCH 12/21] Add silent replace option to linker.add_lib_flags --- source/fab/tools/linker.py | 11 ++++++++++- tests/unit_tests/tools/test_linker.py | 27 +++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 5b8b474b..1e96b405 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -10,6 +10,7 @@ import os from pathlib import Path from typing import cast, Dict, List, Optional +import warnings from fab.tools.category import Category from fab.tools.compiler import Compiler @@ -83,12 +84,20 @@ def get_lib_flags(self, lib: str) -> List[str]: except KeyError: raise RuntimeError(f"Unknown library name: '{lib}'") - def add_lib_flags(self, lib: str, flags: List[str]): + def add_lib_flags(self, lib: str, flags: List[str], + silent_replace: bool = False): '''Add a set of flags for a standard library :param lib: the library name :param flags: the flags to use with the library + :param silent_replace: if set, no warning will be printed when an + existing lib is overwritten. ''' + if lib in self._lib_flags and not silent_replace: + warnings.warn(f"Replacing existing flags for library {lib}: " + f"'{self._lib_flags[lib]}' with " + f"'{flags}'.") + # Make a copy to avoid modifying the caller's list self._lib_flags[lib] = flags[:] diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 1e38d797..97c8a6a5 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -9,6 +9,7 @@ from pathlib import Path from unittest import mock +import warnings import pytest @@ -115,7 +116,7 @@ def test_linker_add_lib_flags(mock_c_compiler): assert result == ["-L", "xios/lib", "-I", "xios/inc"] -def test_linker_add_lib_flags_overwrite(mock_c_compiler): +def test_linker_add_lib_flags_overwrite_defaults(mock_c_compiler): """Linker should provide a way to replace the default flags for a library""" linker = Linker(compiler=mock_c_compiler) @@ -124,13 +125,35 @@ def test_linker_add_lib_flags_overwrite(mock_c_compiler): assert result == ["$(nf-config --flibs)", "($nc-config --libs)"] # Replace them with another set of flags. - linker.add_lib_flags("netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) + warn_message = 'Replacing existing flags for library netcdf' + with pytest.warns(UserWarning, match=warn_message): + linker.add_lib_flags("netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) # Test that we can see our custom flags result = linker.get_lib_flags("netcdf") assert result == ["-L", "netcdf/lib", "-I", "netcdf/inc"] +def test_linker_add_lib_flags_overwrite_silent(mock_c_compiler): + """Linker should provide the option to replace flags for a library without + generating a warning + """ + linker = Linker(compiler=mock_c_compiler) + + # Initially we have the default netcdf flags + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + assert linker.get_lib_flags("customlib") == ["-q", "/tmp", "-j"] + + # Replace them with another set of flags. + with warnings.catch_warnings(): + warnings.simplefilter("error") + linker.add_lib_flags("customlib", ["-t", "-b"], silent_replace=True) + + # Test that we can see our custom flags + result = linker.get_lib_flags("customlib") + assert result == ["-t", "-b"] + + def test_linker_remove_lib_flags(mock_c_compiler): """Linker should provide a way to remove the flags for a library""" linker = Linker(compiler=mock_c_compiler) From 649b9ffd5405f235102a220279f07fc8cf4abb35 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 6 Sep 2024 00:08:05 +1000 Subject: [PATCH 13/21] Fixed failing test triggered by executing them in specific order (tools then steps) --- tests/unit_tests/steps/test_archive_objects.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/unit_tests/steps/test_archive_objects.py b/tests/unit_tests/steps/test_archive_objects.py index 3c828ab9..b661799c 100644 --- a/tests/unit_tests/steps/test_archive_objects.py +++ b/tests/unit_tests/steps/test_archive_objects.py @@ -77,20 +77,21 @@ def test_for_library(self): assert config.artefact_store[ArtefactSet.OBJECT_ARCHIVES] == { None: set([str(config.build_output / 'mylib.a')])} - def test_incorrect_tool(self): + def test_incorrect_tool(self, mock_c_compiler): '''Test that an incorrect archive tool is detected ''' config = BuildConfig('proj', ToolBox(), mpi=False, openmp=False) tool_box = config.tool_box - cc = tool_box.get_tool(Category.C_COMPILER, config.mpi) - # And set its category to C_COMPILER + cc = mock_c_compiler + # And set its category to be AR cc._category = Category.AR - # So overwrite the C compiler with the re-categories Fortran compiler + # Now add this 'ar' tool to the tool box tool_box.add_tool(cc) with pytest.raises(RuntimeError) as err: archive_objects(config=config, output_fpath=config.build_output / 'mylib.a') - assert ("Unexpected tool 'gcc' of type '' instead of Ar" in str(err.value)) + assert ("Unexpected tool 'mock_c_compiler' of type '' instead of Ar" + in str(err.value)) From b0e5842e818d971c35b6cca0b327248740ff6f3e Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 6 Sep 2024 00:08:27 +1000 Subject: [PATCH 14/21] Fixed line lengths. --- tests/unit_tests/steps/test_archive_objects.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit_tests/steps/test_archive_objects.py b/tests/unit_tests/steps/test_archive_objects.py index b661799c..e6a4a957 100644 --- a/tests/unit_tests/steps/test_archive_objects.py +++ b/tests/unit_tests/steps/test_archive_objects.py @@ -51,7 +51,8 @@ def test_for_exes(self): # ensure the correct artefacts were created assert config.artefact_store[ArtefactSet.OBJECT_ARCHIVES] == { - target: set([str(config.build_output / f'{target}.a')]) for target in targets} + target: set([str(config.build_output / f'{target}.a')]) + for target in targets} def test_for_library(self): '''As used when building an object archive or archiving before linking @@ -65,12 +66,15 @@ def test_for_library(self): mock_result = mock.Mock(returncode=0, return_value=123) with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as mock_run_command, \ - pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"): - archive_objects(config=config, output_fpath=config.build_output / 'mylib.a') + pytest.warns(UserWarning, match="_metric_send_conn not set, " + "cannot send metrics"): + archive_objects(config=config, + output_fpath=config.build_output / 'mylib.a') # ensure the correct command line calls were made mock_run_command.assert_called_once_with([ - 'ar', 'cr', str(config.build_output / 'mylib.a'), 'util1.o', 'util2.o'], + 'ar', 'cr', str(config.build_output / 'mylib.a'), 'util1.o', + 'util2.o'], capture_output=True, env=None, cwd=None, check=False) # ensure the correct artefacts were created From 0645816699901a6188fcf6ad150219c89ca6b218 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Wed, 11 Sep 2024 16:35:00 +1000 Subject: [PATCH 15/21] Add tests for linker LDFLAG --- tests/unit_tests/tools/test_linker.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 97c8a6a5..6d12338d 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -52,6 +52,13 @@ def test_linker(mock_c_compiler, mock_fortran_compiler): "creating Linker." in str(err.value)) +def test_linker_gets_ldflags(mock_c_compiler): + """Tests that the linker retrieves env.LDFLAGS""" + with mock.patch.dict("os.environ", {"LDFLAGS": "-lm"}): + linker = Linker(compiler=mock_c_compiler) + assert "-lm" in linker.flags + + def test_linker_check_available(mock_c_compiler): '''Tests the is_available functionality.''' @@ -255,3 +262,22 @@ def test_linker_add_compiler_flag(): tool_run.assert_called_with( ['no-compiler.exe', '-some-other-flag', 'a.o', '-o', 'a.out'], capture_output=True, env=None, cwd=None, check=False) + + +def test_linker_all_flag_types(mock_c_compiler): + """Make sure all possible sources of linker flags are used in the right + order""" + with mock.patch.dict("os.environ", {"LDFLAGS": "-ldflag"}): + linker = Linker(compiler=mock_c_compiler) + + linker.flags.extend(["-linker-flag1", "-linker-flag2"]) + mock_result = mock.Mock(returncode=0) + with mock.patch("fab.tools.tool.subprocess.run", + return_value=mock_result) as tool_run: + linker.link([Path("a.o")], Path("a.out"), openmp=False) + tool_run.assert_called_with([ + "mock_c_compiler.exe", + "-ldflag", + "-linker-flag1", "-linker-flag2", + "a.o", "-o", "a.out"], + capture_output=True, env=None, cwd=None, check=False) From 728089e15759f4ff35226f4ce20e8f57542b23f7 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Wed, 11 Sep 2024 17:25:55 +1000 Subject: [PATCH 16/21] Add pre- and post- lib flags to link function --- source/fab/steps/link.py | 4 +- source/fab/tools/linker.py | 12 ++++-- tests/unit_tests/tools/test_linker.py | 59 +++++++++++++++++++++++++-- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index 7924d782..bb65a7dc 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -72,7 +72,7 @@ def link_exe(config, for root, objects in target_objects.items(): exe_path = config.project_workspace / f'{root}' linker.link(objects, exe_path, openmp=config.openmp, libs=libs, - add_flags=flags) + post_lib_flags=flags) config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path) @@ -122,4 +122,4 @@ def link_shared_object(config, output_fpath: str, flags=None, objects = target_objects[None] out_name = Template(output_fpath).substitute(output=config.build_output) - linker.link(objects, out_name, openmp=config.openmp, add_flags=flags) + linker.link(objects, out_name, openmp=config.openmp, post_lib_flags=flags) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index c2eea7f6..a36c73ca 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -118,16 +118,18 @@ def remove_lib_flags(self, lib: str): def link(self, input_files: List[Path], output_file: Path, openmp: bool, + pre_lib_flags: Optional[List[str]] = None, libs: Optional[List[str]] = None, - add_flags: Optional[List[str]] = None) -> str: + post_lib_flags: Optional[List[str]] = None) -> str: '''Executes the linker with the specified input files, creating `output_file`. :param input_files: list of input files to link. :param output_file: output file. :param openm: whether OpenMP is requested or not. + :param pre_lib_flags: additional linker flags to use before libs. :param libs: additional libraries to link with. - :param add_flags: additional linker flags. + :param post_lib_flags: additional linker flags to use after libs. :returns: the stdout of the link command ''' @@ -141,9 +143,11 @@ def link(self, input_files: List[Path], output_file: Path, # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) + if pre_lib_flags: + params.extend(pre_lib_flags) for lib in (libs or []): params.extend(self.get_lib_flags(lib)) - if add_flags: - params.extend(add_flags) + if post_lib_flags: + params.extend(post_lib_flags) params.extend([self._output_flag, str(output_file)]) return self.run(params) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 6d12338d..fd2827d9 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -203,6 +203,40 @@ def test_linker_c_with_libraries(mock_c_compiler): "-o", "a.out"]) +def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): + """Test the link command line when a library and additional flags are + specified.""" + linker = Linker(compiler=mock_c_compiler) + with mock.patch.object(linker, "run") as link_run: + linker.link( + [Path("a.o")], Path("a.out"), + libs=["netcdf"], post_lib_flags=["-extra-flag"], + openmp=False, + ) + link_run.assert_called_with([ + "a.o", + "$(nf-config --flibs)", "($nc-config --libs)", "-extra-flag", + "-o", "a.out", + ]) + + +def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): + """Test the link command line when a library and additional flags are + specified.""" + linker = Linker(compiler=mock_c_compiler) + with mock.patch.object(linker, "run") as link_run: + linker.link( + [Path("a.o")], Path("a.out"), + pre_lib_flags=["-extra-flag"], libs=["netcdf"], + openmp=False, + ) + link_run.assert_called_with([ + "a.o", + "-extra-flag", "$(nf-config --flibs)", "($nc-config --libs)", + "-o", "a.out", + ]) + + def test_linker_c_with_custom_libraries(mock_c_compiler): """Test the link command line when additional libraries are specified.""" linker = Linker(compiler=mock_c_compiler) @@ -270,14 +304,31 @@ def test_linker_all_flag_types(mock_c_compiler): with mock.patch.dict("os.environ", {"LDFLAGS": "-ldflag"}): linker = Linker(compiler=mock_c_compiler) + mock_c_compiler.flags.extend(["-compiler-flag1", "-compiler-flag2"]) linker.flags.extend(["-linker-flag1", "-linker-flag2"]) + linker.add_lib_flags("customlib", ["-libflag1", "libflag2"]) + mock_result = mock.Mock(returncode=0) with mock.patch("fab.tools.tool.subprocess.run", return_value=mock_result) as tool_run: - linker.link([Path("a.o")], Path("a.out"), openmp=False) + linker.link([ + Path("a.o")], Path("a.out"), + pre_lib_flags=["-prelibflag1", "-prelibflag2"], + libs=["customlib", "netcdf"], + post_lib_flags=["-postlibflag1", "-postlibflag2"], + openmp=True) + tool_run.assert_called_with([ "mock_c_compiler.exe", - "-ldflag", - "-linker-flag1", "-linker-flag2", - "a.o", "-o", "a.out"], + # Note: compiler flags and linker flags will be switched when the Linker + # becomes a CompilerWrapper in a following PR + "-ldflag", "-linker-flag1", "-linker-flag2", + "-compiler-flag1", "-compiler-flag2", + "-fopenmp", + "a.o", + "-prelibflag1", "-prelibflag2", + "-libflag1", "libflag2", + "$(nf-config --flibs)", "($nc-config --libs)", + "-postlibflag1", "-postlibflag2", + "-o", "a.out"], capture_output=True, env=None, cwd=None, check=False) From 76322a10f91cc72f002ebcb3f49716119d42cd90 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Mon, 23 Sep 2024 15:01:21 +1000 Subject: [PATCH 17/21] Fix syntax in built-in lib flags --- source/fab/tools/linker.py | 2 +- tests/unit_tests/tools/test_linker.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index a36c73ca..f7d56fcd 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -57,7 +57,7 @@ def __init__(self, name: Optional[str] = None, # Include netcdf as an example, since it is reasonable portable self.add_lib_flags( 'netcdf', - ['$(nf-config --flibs)', '($nc-config --libs)'] + ['$(nf-config --flibs)', '$(nc-config --libs)'] ) @property diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index fd2827d9..cce8a40e 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -100,7 +100,7 @@ def test_linker_get_lib_flags(mock_c_compiler): linker = Linker(compiler=mock_c_compiler) # netcdf is built in to the linker, since it is reasonable portable result = linker.get_lib_flags("netcdf") - assert result == ["$(nf-config --flibs)", "($nc-config --libs)"] + assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] def test_linker_get_lib_flags_unknown(mock_c_compiler): @@ -129,7 +129,7 @@ def test_linker_add_lib_flags_overwrite_defaults(mock_c_compiler): # Initially we have the default netcdf flags result = linker.get_lib_flags("netcdf") - assert result == ["$(nf-config --flibs)", "($nc-config --libs)"] + assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] # Replace them with another set of flags. warn_message = 'Replacing existing flags for library netcdf' @@ -199,7 +199,7 @@ def test_linker_c_with_libraries(mock_c_compiler): linker.link([Path("a.o")], Path("a.out"), libs=["netcdf"], openmp=True) link_run.assert_called_with( ["-fopenmp", "a.o", - "$(nf-config --flibs)", "($nc-config --libs)", + "$(nf-config --flibs)", "$(nc-config --libs)", "-o", "a.out"]) @@ -215,7 +215,7 @@ def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): ) link_run.assert_called_with([ "a.o", - "$(nf-config --flibs)", "($nc-config --libs)", "-extra-flag", + "$(nf-config --flibs)", "$(nc-config --libs)", "-extra-flag", "-o", "a.out", ]) @@ -232,7 +232,7 @@ def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): ) link_run.assert_called_with([ "a.o", - "-extra-flag", "$(nf-config --flibs)", "($nc-config --libs)", + "-extra-flag", "$(nf-config --flibs)", "$(nc-config --libs)", "-o", "a.out", ]) @@ -248,7 +248,7 @@ def test_linker_c_with_custom_libraries(mock_c_compiler): link_run.assert_called_with( ["-fopenmp", "a.o", "-q", "/tmp", "-j", - "$(nf-config --flibs)", "($nc-config --libs)", + "$(nf-config --flibs)", "$(nc-config --libs)", "-o", "a.out"]) @@ -328,7 +328,7 @@ def test_linker_all_flag_types(mock_c_compiler): "a.o", "-prelibflag1", "-prelibflag2", "-libflag1", "libflag2", - "$(nf-config --flibs)", "($nc-config --libs)", + "$(nf-config --flibs)", "$(nc-config --libs)", "-postlibflag1", "-postlibflag2", "-o", "a.out"], capture_output=True, env=None, cwd=None, check=False) From 58007e857739a29af4addcf03e53db40009dc256 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Mon, 23 Sep 2024 15:28:16 +1000 Subject: [PATCH 18/21] Remove netcdf as a built-in linker library Bash-style substitution is not currently handled --- source/fab/tools/linker.py | 5 -- tests/conftest.py | 2 + tests/unit_tests/tools/test_linker.py | 92 +++++++++++++-------------- 3 files changed, 45 insertions(+), 54 deletions(-) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index f7d56fcd..f8fb2068 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -54,11 +54,6 @@ def __init__(self, name: Optional[str] = None, # Maintain a set of flags for common libraries. self._lib_flags: Dict[str, List[str]] = {} - # Include netcdf as an example, since it is reasonable portable - self.add_lib_flags( - 'netcdf', - ['$(nf-config --flibs)', '$(nc-config --libs)'] - ) @property def mpi(self) -> bool: diff --git a/tests/conftest.py b/tests/conftest.py index 835cd294..8db11e9d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,6 +48,8 @@ def fixture_mock_linker(): Category.FORTRAN_COMPILER) mock_linker.run = mock.Mock() mock_linker._version = (1, 2, 3) + mock_linker.add_lib_flags('netcdf', + ['$(nf-config --flibs)', '$(nc-config --libs)']) return mock_linker diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index cce8a40e..79c6f1ce 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -93,13 +93,12 @@ def test_linker_check_available(mock_c_compiler): # ==================== # Managing lib flags: # ==================== -def test_linker_get_lib_flags(mock_c_compiler): +def test_linker_get_lib_flags(mock_linker): """Linker should provide a map of library names, each leading to a list of linker flags """ - linker = Linker(compiler=mock_c_compiler) - # netcdf is built in to the linker, since it is reasonable portable - result = linker.get_lib_flags("netcdf") + # netcdf flags are built in to the mock linker + result = mock_linker.get_lib_flags("netcdf") assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] @@ -123,58 +122,56 @@ def test_linker_add_lib_flags(mock_c_compiler): assert result == ["-L", "xios/lib", "-I", "xios/inc"] -def test_linker_add_lib_flags_overwrite_defaults(mock_c_compiler): +def test_linker_add_lib_flags_overwrite_defaults(mock_linker): """Linker should provide a way to replace the default flags for a library""" - linker = Linker(compiler=mock_c_compiler) # Initially we have the default netcdf flags - result = linker.get_lib_flags("netcdf") + result = mock_linker.get_lib_flags("netcdf") assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] # Replace them with another set of flags. warn_message = 'Replacing existing flags for library netcdf' with pytest.warns(UserWarning, match=warn_message): - linker.add_lib_flags("netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) + mock_linker.add_lib_flags( + "netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) # Test that we can see our custom flags - result = linker.get_lib_flags("netcdf") + result = mock_linker.get_lib_flags("netcdf") assert result == ["-L", "netcdf/lib", "-I", "netcdf/inc"] -def test_linker_add_lib_flags_overwrite_silent(mock_c_compiler): +def test_linker_add_lib_flags_overwrite_silent(mock_linker): """Linker should provide the option to replace flags for a library without generating a warning """ - linker = Linker(compiler=mock_c_compiler) # Initially we have the default netcdf flags - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) - assert linker.get_lib_flags("customlib") == ["-q", "/tmp", "-j"] + mock_linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + assert mock_linker.get_lib_flags("customlib") == ["-q", "/tmp", "-j"] # Replace them with another set of flags. with warnings.catch_warnings(): warnings.simplefilter("error") - linker.add_lib_flags("customlib", ["-t", "-b"], silent_replace=True) + mock_linker.add_lib_flags("customlib", ["-t", "-b"], + silent_replace=True) # Test that we can see our custom flags - result = linker.get_lib_flags("customlib") + result = mock_linker.get_lib_flags("customlib") assert result == ["-t", "-b"] -def test_linker_remove_lib_flags(mock_c_compiler): +def test_linker_remove_lib_flags(mock_linker): """Linker should provide a way to remove the flags for a library""" - linker = Linker(compiler=mock_c_compiler) - linker.remove_lib_flags("netcdf") + mock_linker.remove_lib_flags("netcdf") with pytest.raises(RuntimeError) as err: - linker.get_lib_flags("netcdf") + mock_linker.get_lib_flags("netcdf") assert "Unknown library name: 'netcdf'" in str(err.value) -def test_linker_remove_lib_flags_unknown(mock_c_compiler): +def test_linker_remove_lib_flags_unknown(mock_linker): """Linker should silently allow removing flags for unknown library""" - linker = Linker(compiler=mock_c_compiler) - linker.remove_lib_flags("unkown") + mock_linker.remove_lib_flags("unknown") # ==================== @@ -183,6 +180,9 @@ def test_linker_remove_lib_flags_unknown(mock_c_compiler): def test_linker_c(mock_c_compiler): '''Test the link command line when no additional libraries are specified.''' linker = Linker(compiler=mock_c_compiler) + # Add a library to the linker, but don't use it in the link step + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + mock_result = mock.Mock(returncode=0) with mock.patch('fab.tools.tool.subprocess.run', return_value=mock_result) as tool_run: @@ -195,11 +195,15 @@ def test_linker_c(mock_c_compiler): def test_linker_c_with_libraries(mock_c_compiler): """Test the link command line when additional libraries are specified.""" linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), libs=["netcdf"], openmp=True) + linker.link([Path("a.o")], Path("a.out"), + libs=["customlib"], openmp=True) + # The order of the 'libs' list should be maintained link_run.assert_called_with( ["-fopenmp", "a.o", - "$(nf-config --flibs)", "$(nc-config --libs)", + "-q", "/tmp", "-j", "-o", "a.out"]) @@ -207,15 +211,17 @@ def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): """Test the link command line when a library and additional flags are specified.""" linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + with mock.patch.object(linker, "run") as link_run: linker.link( [Path("a.o")], Path("a.out"), - libs=["netcdf"], post_lib_flags=["-extra-flag"], + libs=["customlib"], post_lib_flags=["-extra-flag"], openmp=False, ) link_run.assert_called_with([ "a.o", - "$(nf-config --flibs)", "$(nc-config --libs)", "-extra-flag", + "-q", "/tmp", "-j", "-extra-flag", "-o", "a.out", ]) @@ -224,34 +230,21 @@ def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): """Test the link command line when a library and additional flags are specified.""" linker = Linker(compiler=mock_c_compiler) + linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + with mock.patch.object(linker, "run") as link_run: linker.link( [Path("a.o")], Path("a.out"), - pre_lib_flags=["-extra-flag"], libs=["netcdf"], + pre_lib_flags=["-extra-flag"], libs=["customlib"], openmp=False, ) link_run.assert_called_with([ "a.o", - "-extra-flag", "$(nf-config --flibs)", "$(nc-config --libs)", + "-extra-flag", "-q", "/tmp", "-j", "-o", "a.out", ]) -def test_linker_c_with_custom_libraries(mock_c_compiler): - """Test the link command line when additional libraries are specified.""" - linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) - with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), - libs=["customlib", "netcdf"], openmp=True) - # The order of the 'libs' list should be maintained - link_run.assert_called_with( - ["-fopenmp", "a.o", - "-q", "/tmp", "-j", - "$(nf-config --flibs)", "$(nc-config --libs)", - "-o", "a.out"]) - - def test_linker_c_with_unknown_library(mock_c_compiler): """Test the link command raises an error when unknow libraries are specified. @@ -260,8 +253,8 @@ def test_linker_c_with_unknown_library(mock_c_compiler): with pytest.raises(RuntimeError) as err: # Try to use "customlib" when we haven't added it to the linker - linker.link([Path("a.o")], Path("a.out"), - libs=["netcdf", "customlib"], openmp=True) + linker.link([Path("a.o")], Path("a.out"), libs=["customlib"], + openmp=True) assert "Unknown library name: 'customlib'" in str(err.value) @@ -306,7 +299,8 @@ def test_linker_all_flag_types(mock_c_compiler): mock_c_compiler.flags.extend(["-compiler-flag1", "-compiler-flag2"]) linker.flags.extend(["-linker-flag1", "-linker-flag2"]) - linker.add_lib_flags("customlib", ["-libflag1", "libflag2"]) + linker.add_lib_flags("customlib1", ["-lib1flag1", "lib1flag2"]) + linker.add_lib_flags("customlib2", ["-lib2flag1", "lib2flag2"]) mock_result = mock.Mock(returncode=0) with mock.patch("fab.tools.tool.subprocess.run", @@ -314,7 +308,7 @@ def test_linker_all_flag_types(mock_c_compiler): linker.link([ Path("a.o")], Path("a.out"), pre_lib_flags=["-prelibflag1", "-prelibflag2"], - libs=["customlib", "netcdf"], + libs=["customlib2", "customlib1"], post_lib_flags=["-postlibflag1", "-postlibflag2"], openmp=True) @@ -327,8 +321,8 @@ def test_linker_all_flag_types(mock_c_compiler): "-fopenmp", "a.o", "-prelibflag1", "-prelibflag2", - "-libflag1", "libflag2", - "$(nf-config --flibs)", "$(nc-config --libs)", + "-lib2flag1", "lib2flag2", + "-lib1flag1", "lib1flag2", "-postlibflag1", "-postlibflag2", "-o", "a.out"], capture_output=True, env=None, cwd=None, check=False) From 72862006f3206785ed50db4b81c8886c346c48a3 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Mon, 23 Sep 2024 17:15:21 +1000 Subject: [PATCH 19/21] Configure pre- and post-lib flags on the Linker object Previously they were passed into the Linker.link() function --- source/fab/steps/link.py | 9 ++++---- source/fab/tools/linker.py | 33 +++++++++++++++++++-------- tests/unit_tests/tools/test_linker.py | 16 +++++-------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index bb65a7dc..d2146a83 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -65,14 +65,14 @@ def link_exe(config, logger.info(f'Linker is {linker.name}') libs = libs or [] - flags = flags or [] + if flags: + linker.add_post_lib_flags(flags) source_getter = source or DefaultLinkerSource() target_objects = source_getter(config.artefact_store) for root, objects in target_objects.items(): exe_path = config.project_workspace / f'{root}' - linker.link(objects, exe_path, openmp=config.openmp, libs=libs, - post_lib_flags=flags) + linker.link(objects, exe_path, openmp=config.openmp, libs=libs) config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path) @@ -122,4 +122,5 @@ def link_shared_object(config, output_fpath: str, flags=None, objects = target_objects[None] out_name = Template(output_fpath).substitute(output=config.build_output) - linker.link(objects, out_name, openmp=config.openmp, post_lib_flags=flags) + linker.add_post_lib_flags(flags) + linker.link(objects, out_name, openmp=config.openmp) diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index f8fb2068..8959b3de 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -54,6 +54,9 @@ def __init__(self, name: Optional[str] = None, # Maintain a set of flags for common libraries. self._lib_flags: Dict[str, List[str]] = {} + # Allow flags to include before or after any library-specific flags. + self._pre_lib_flags: List[str] = [] + self._post_lib_flags: List[str] = [] @property def mpi(self) -> bool: @@ -102,7 +105,7 @@ def add_lib_flags(self, lib: str, flags: List[str], self._lib_flags[lib] = flags[:] def remove_lib_flags(self, lib: str): - '''Add a set of flags for a standard library + '''Remove any flags configured for a standard library :param lib: the library name ''' @@ -111,20 +114,30 @@ def remove_lib_flags(self, lib: str): except KeyError: pass + def add_pre_lib_flags(self, flags: List[str]): + '''Add a set of flags to use before any library-specific flags + + :param flags: the flags to include + ''' + self._pre_lib_flags.extend(flags) + + def add_post_lib_flags(self, flags: List[str]): + '''Add a set of flags to use after any library-specific flags + + :param flags: the flags to include + ''' + self._post_lib_flags.extend(flags) + def link(self, input_files: List[Path], output_file: Path, openmp: bool, - pre_lib_flags: Optional[List[str]] = None, - libs: Optional[List[str]] = None, - post_lib_flags: Optional[List[str]] = None) -> str: + libs: Optional[List[str]] = None) -> str: '''Executes the linker with the specified input files, creating `output_file`. :param input_files: list of input files to link. :param output_file: output file. :param openm: whether OpenMP is requested or not. - :param pre_lib_flags: additional linker flags to use before libs. :param libs: additional libraries to link with. - :param post_lib_flags: additional linker flags to use after libs. :returns: the stdout of the link command ''' @@ -138,11 +151,11 @@ def link(self, input_files: List[Path], output_file: Path, # TODO: why are the .o files sorted? That shouldn't matter params.extend(sorted(map(str, input_files))) - if pre_lib_flags: - params.extend(pre_lib_flags) + if self._pre_lib_flags: + params.extend(self._pre_lib_flags) for lib in (libs or []): params.extend(self.get_lib_flags(lib)) - if post_lib_flags: - params.extend(post_lib_flags) + if self._post_lib_flags: + params.extend(self._post_lib_flags) params.extend([self._output_flag, str(output_file)]) return self.run(params) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index 79c6f1ce..ecf81dd6 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -212,13 +212,11 @@ def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): specified.""" linker = Linker(compiler=mock_c_compiler) linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + linker.add_post_lib_flags(["-extra-flag"]) with mock.patch.object(linker, "run") as link_run: linker.link( - [Path("a.o")], Path("a.out"), - libs=["customlib"], post_lib_flags=["-extra-flag"], - openmp=False, - ) + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) link_run.assert_called_with([ "a.o", "-q", "/tmp", "-j", "-extra-flag", @@ -231,13 +229,11 @@ def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): specified.""" linker = Linker(compiler=mock_c_compiler) linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + linker.add_pre_lib_flags(["-extra-flag"]) with mock.patch.object(linker, "run") as link_run: linker.link( - [Path("a.o")], Path("a.out"), - pre_lib_flags=["-extra-flag"], libs=["customlib"], - openmp=False, - ) + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) link_run.assert_called_with([ "a.o", "-extra-flag", "-q", "/tmp", "-j", @@ -299,17 +295,17 @@ def test_linker_all_flag_types(mock_c_compiler): mock_c_compiler.flags.extend(["-compiler-flag1", "-compiler-flag2"]) linker.flags.extend(["-linker-flag1", "-linker-flag2"]) + linker.add_pre_lib_flags(["-prelibflag1", "-prelibflag2"]) linker.add_lib_flags("customlib1", ["-lib1flag1", "lib1flag2"]) linker.add_lib_flags("customlib2", ["-lib2flag1", "lib2flag2"]) + linker.add_post_lib_flags(["-postlibflag1", "-postlibflag2"]) mock_result = mock.Mock(returncode=0) with mock.patch("fab.tools.tool.subprocess.run", return_value=mock_result) as tool_run: linker.link([ Path("a.o")], Path("a.out"), - pre_lib_flags=["-prelibflag1", "-prelibflag2"], libs=["customlib2", "customlib1"], - post_lib_flags=["-postlibflag1", "-postlibflag2"], openmp=True) tool_run.assert_called_with([ From c6e3b424d7609e8697564cce0018367a295fa027 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Mon, 23 Sep 2024 17:16:56 +1000 Subject: [PATCH 20/21] Use more realistic linker lib flags --- tests/conftest.py | 3 +- tests/unit_tests/tools/test_linker.py | 52 ++++++++++++--------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8db11e9d..559d4f3b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -48,8 +48,7 @@ def fixture_mock_linker(): Category.FORTRAN_COMPILER) mock_linker.run = mock.Mock() mock_linker._version = (1, 2, 3) - mock_linker.add_lib_flags('netcdf', - ['$(nf-config --flibs)', '$(nc-config --libs)']) + mock_linker.add_lib_flags("netcdf", ["-lnetcdff", "-lnetcdf"]) return mock_linker diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index ecf81dd6..ec5cd084 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -99,7 +99,7 @@ def test_linker_get_lib_flags(mock_linker): """ # netcdf flags are built in to the mock linker result = mock_linker.get_lib_flags("netcdf") - assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] + assert result == ["-lnetcdff", "-lnetcdf"] def test_linker_get_lib_flags_unknown(mock_c_compiler): @@ -115,11 +115,11 @@ def test_linker_get_lib_flags_unknown(mock_c_compiler): def test_linker_add_lib_flags(mock_c_compiler): """Linker should provide a way to add a new set of flags for a library""" linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags("xios", ["-L", "xios/lib", "-I", "xios/inc"]) + linker.add_lib_flags("xios", ["-L", "xios/lib", "-lxios"]) # Make sure we can get it back. The order should be maintained. result = linker.get_lib_flags("xios") - assert result == ["-L", "xios/lib", "-I", "xios/inc"] + assert result == ["-L", "xios/lib", "-lxios"] def test_linker_add_lib_flags_overwrite_defaults(mock_linker): @@ -127,17 +127,17 @@ def test_linker_add_lib_flags_overwrite_defaults(mock_linker): # Initially we have the default netcdf flags result = mock_linker.get_lib_flags("netcdf") - assert result == ["$(nf-config --flibs)", "$(nc-config --libs)"] + assert result == ["-lnetcdff", "-lnetcdf"] # Replace them with another set of flags. warn_message = 'Replacing existing flags for library netcdf' with pytest.warns(UserWarning, match=warn_message): mock_linker.add_lib_flags( - "netcdf", ["-L", "netcdf/lib", "-I", "netcdf/inc"]) + "netcdf", ["-L", "netcdf/lib", "-lnetcdf"]) # Test that we can see our custom flags result = mock_linker.get_lib_flags("netcdf") - assert result == ["-L", "netcdf/lib", "-I", "netcdf/inc"] + assert result == ["-L", "netcdf/lib", "-lnetcdf"] def test_linker_add_lib_flags_overwrite_silent(mock_linker): @@ -146,8 +146,8 @@ def test_linker_add_lib_flags_overwrite_silent(mock_linker): """ # Initially we have the default netcdf flags - mock_linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) - assert mock_linker.get_lib_flags("customlib") == ["-q", "/tmp", "-j"] + mock_linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + assert mock_linker.get_lib_flags("customlib") == ["-lcustom", "-jcustom"] # Replace them with another set of flags. with warnings.catch_warnings(): @@ -181,7 +181,7 @@ def test_linker_c(mock_c_compiler): '''Test the link command line when no additional libraries are specified.''' linker = Linker(compiler=mock_c_compiler) # Add a library to the linker, but don't use it in the link step - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) mock_result = mock.Mock(returncode=0) with mock.patch('fab.tools.tool.subprocess.run', @@ -195,50 +195,42 @@ def test_linker_c(mock_c_compiler): def test_linker_c_with_libraries(mock_c_compiler): """Test the link command line when additional libraries are specified.""" linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) with mock.patch.object(linker, "run") as link_run: - linker.link([Path("a.o")], Path("a.out"), - libs=["customlib"], openmp=True) + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=True) # The order of the 'libs' list should be maintained link_run.assert_called_with( - ["-fopenmp", "a.o", - "-q", "/tmp", "-j", - "-o", "a.out"]) + ["-fopenmp", "a.o", "-lcustom", "-jcustom", "-o", "a.out"]) def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): """Test the link command line when a library and additional flags are specified.""" linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) linker.add_post_lib_flags(["-extra-flag"]) with mock.patch.object(linker, "run") as link_run: linker.link( [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) - link_run.assert_called_with([ - "a.o", - "-q", "/tmp", "-j", "-extra-flag", - "-o", "a.out", - ]) + link_run.assert_called_with( + [ "a.o", "-lcustom", "-jcustom", "-extra-flag", "-o", "a.out"]) def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler): """Test the link command line when a library and additional flags are specified.""" linker = Linker(compiler=mock_c_compiler) - linker.add_lib_flags("customlib", ["-q", "/tmp", "-j"]) - linker.add_pre_lib_flags(["-extra-flag"]) + linker.add_lib_flags("customlib", ["-lcustom", "-jcustom"]) + linker.add_pre_lib_flags(["-L", "/common/path/"]) with mock.patch.object(linker, "run") as link_run: linker.link( [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) - link_run.assert_called_with([ - "a.o", - "-extra-flag", "-q", "/tmp", "-j", - "-o", "a.out", - ]) + link_run.assert_called_with( + ["a.o", "-L", "/common/path/", "-lcustom", "-jcustom", "-o", "a.out"]) def test_linker_c_with_unknown_library(mock_c_compiler): @@ -249,8 +241,8 @@ def test_linker_c_with_unknown_library(mock_c_compiler): with pytest.raises(RuntimeError) as err: # Try to use "customlib" when we haven't added it to the linker - linker.link([Path("a.o")], Path("a.out"), libs=["customlib"], - openmp=True) + linker.link( + [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=True) assert "Unknown library name: 'customlib'" in str(err.value) From f7e40f3482bedd7685464027f47a4eda2e2d2783 Mon Sep 17 00:00:00 2001 From: Luke Hoffmann Date: Mon, 23 Sep 2024 17:29:10 +1000 Subject: [PATCH 21/21] Formatting fix --- tests/unit_tests/tools/test_linker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/tools/test_linker.py b/tests/unit_tests/tools/test_linker.py index ec5cd084..6984c790 100644 --- a/tests/unit_tests/tools/test_linker.py +++ b/tests/unit_tests/tools/test_linker.py @@ -216,7 +216,7 @@ def test_linker_c_with_libraries_and_post_flags(mock_c_compiler): linker.link( [Path("a.o")], Path("a.out"), libs=["customlib"], openmp=False) link_run.assert_called_with( - [ "a.o", "-lcustom", "-jcustom", "-extra-flag", "-o", "a.out"]) + ["a.o", "-lcustom", "-jcustom", "-extra-flag", "-o", "a.out"]) def test_linker_c_with_libraries_and_pre_flags(mock_c_compiler):