Build example/model check BOUT++ version

[bendudson]

Is there a way to enable us to specify in the makefile which version(s) of BOUT++ are known to work? It would be nice for users to get a useful error message rather than a compiler error, when they try to compile a model with a different version of BOUT++.

[d7919]

Something like the following might do it

ifeq ($(shell if [  ! $(BOUT_VERSION) -lt 4.0] ; then echo "FAIL" ; fi), "FAIL"))
$(warning "Expect compilation to fail as the target requires BOUT++ prior to 4.0")
endif

We could probably wrap it in a function like

define version_warning_if_not_less_than
ifeq ($(shell if [  ! $(BOUT_VERSION) -lt $(1)] ; then echo "FAIL" ; fi), "FAIL"))
$(warning "Expect compilation to fail as $(2) requires BOUT++ prior to $(1)")
endif

$(call version_warning_if_not_less_than(4.0, "The name of the target"))

(not tested)

[dschwoerer]

This does not work as test expects integers. Similarly preprocessing macros do not work, as they expect integers ...

I am not sure what version we started setting DBOUT_VERSION but you could explicitly white list some versions with [ "x$$BOUT_VERSION" = "x3.1" -o "x$$BOUT_VERSION" = "x3.0" ] || (echo "This does not work" ; exit 1) And set that as a requirement for all.

Maybe we should provide an integer version (e.g. 300 for 3.0 / 401 for 4.0.1)?

I think errors should be preferred over warnings, that might be ignored :-)

Finally, configure is what should be used to ensure appropriate versions are available, but that might be overkill ...

(not tested)

[d7919]

Thanks, yes this was just to give a flavour of an approach.

How about BOUT_MAJOR, BOUT_MINOR, BOUT_PATCH or similar?

I opted for warning in the example as it's just to help explain to the user that a subsequent failure to build is not unexpected but to still allow us to attempt to build the model. This could make it slightly less painful for someone that's trying to fix the model as they don't have to remove the check + error from the makefile. We could also have an error version if we're able to say it is wrong to use this with a particular version.

[ZedThree]

The PETSC_VERSION_GE family of macros works by checking major/minor/patch. Could do something similar:

#define BOUT_VERSION_GE(MAJOR, MINOR, PATCH) \
  ((BOUT_VERSION_MAJOR > MAJOR)              \
   || ((BOUT_VERSION_MAJOR == MAJOR)         \
       && ((BOUT_VERSION_MINOR > MINOR)      \
           || ((BOUT_VERSION_MINOR == MINOR) && (BOUT_VERSION_PATCH >= PATCH)))))

#1920 removes the version macros entirely, but maybe version.hxx.in should actually look like:

...
#define BOUT_VERSION_MAJOR @BOUT_VERSION_MAJOR@
...
namespace bout {
namespace version {
...
constexpr int major = BOUT_VERSION_MAJOR;
...
}
}

CMake's find_package takes a version argument, and does major/minor/patch version compatibility, for example find_package(bout++ 4) to work with version >= 4.0, and find_package(bout++ 4.2.1 EXACT) to need exactly 4.2.1

[dschwoerer]

Thanks, yes this was just to give a flavour of an approach.

How about BOUT_MAJOR, BOUT_MINOR, BOUT_PATCH or similar?

Checking 3 variables is more complicated than just one. - especially if you need to check a custom range like between 4.0.1 and 4.1.2 This limits us to 9 releases/patchlevel before we need to change that.

I opted for warning in the example as it's just to help explain to the user that a subsequent failure to build is not unexpected but to still allow us to attempt to build the model. This could make it slightly less painful for someone that's trying to fix the model as they don't have to remove the check + error from the makefile. We could also have an error version if we're able to say it is wrong to use this with a particular version.

I'd assume it is easy enough to disable them. If you don't know how to disable them, you probably shouldn't ignore the warning, as the result may not fail, but be wrong in less obvious ways ...