hiker / fab_new

BOM version of the flexible build system for scientific software
https://metoffice.github.io/fab/
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Properly handle compiler versions #8

Closed hiker closed 2 months ago

hiker commented 3 months ago

Atm compiler versions are only handled as strings. We need to properly convert them to integer-tuples to allow comparison, and also support compiler-specific compiler flags to be added.

This is the current way of how LFRic makefiles do compiler version comparison. I have converted this into Python:

compiler_version_comparison = ''.join(f"{int(version_component):02d}" for version_component in compiler.get_version().split('.')) 

if compiler_version_comparison >= '190000':
     runtime_flags = ...
else: ...

So current get_version() returns a string 1.20.0. It is converted to string 012000 and then compared with string 190000

lukehoffmann commented 2 months ago

I've implement this initial change of just making get_version return a tuple instead of a string (and updated other things that depend on it) https://github.com/hiker/fab_new/compare/bom_master...hiker:fab_new:version_tuples

It now returns an empty tuple if the compiler exists but doesn't have a valid version string. This is to support the hash function. It would be good if we could say that a compiler with no version is not valid and so it doesn't need to be hashed, but I don't know if that's safe.

lukehoffmann commented 2 months ago

Second possible component of this:

ATM, there is one method that handles both intel and gfortran. I think these should be moved into the corresponding derived classes (gcc/gfortran/icc/ifort further down in that file), and check that it is indeed the right compiler (e.g. atm we could say have an alias for icc that invokes gcc, and it would not be detected). So test for the right gnu/intel strings in the output. The background here is that I am adding compiler wrapper (like mpif90), so if a user requests intel-mpif90, the wrapper needs to make sure that mpif90 indeed calls the intel compiler. And atm the available check is based on the version number (and we want to minimise the number of times we call the compiler to identify it and make sure it works - so ideally we can deduce this all from one version call). I would hope (??) that the strings for the C and Fortran compiler of the same vendor are close enough to be handled by the same function?? So maybe a mixin?? Note that in general we try to avoid having global functions (which would of course be the other solution)

hiker commented 2 months ago

Looks good. Once you have done the second part, please do a PR - my compiler wrapper needs this (we need to make sure that e.g. mpif90 does indeed invoke the right compiler). A compiler without version is indeed not valid - well, more precisely it will be marked as not available (for a compiler having a version string makes it available), and a tool that is not available, cannot be added to the ToolBox, which means it cannot be used (and we therefore won't need a checksum).

hiker commented 2 months ago

Fixed in PR #9.