search

NOAA-EMC / NCEPLIBS-bacio

This library performs binary I/O for the NCEP models.
Other
2 stars 6 forks source link

removed _4 from name of library and include directory #100

Closed edwardhartnett closed 2 years ago

edwardhartnett commented 2 years ago

Fixes #25

In this PR I remove the "_4" from the library name and the include directory. This is ugly because all users will have to change their build systems when upgrading to bacio-2.5.0, but as we intend to move away from the _4/_8/_d libraries, this is an inevitable annoyance for every one of the NCEPLIBS fortran libraries. So we might as well get started...

We will test out the new bacio library in out NCEPLIBS-g2 testing, before doing a bacio-2.5.0 release...

DusanJovic-NOAA commented 2 years ago

For backward compatibility you can create target alias named bacio_4.

edwardhartnett commented 2 years ago

@DusanJovic-NOAA when you say create a target alias, this would still have to be done in the project that is building with bacio.

For example, we could add such an alias to the NCEPLIBS-g2 build, because that includes bacio.

But since to do that, we have to modify every building project, it's just as easy to fix the name instead of adding an alias.

Or am I misunderstanding what you mean by alias?

DusanJovic-NOAA commented 2 years ago

@DusanJovic-NOAA when you say create a target alias, this would still have to be done in the project that is building with bacio.

No. My suggestion is to add a target alias in this project, in order to avoid the need to change every single library or program that currently depends on bacio::bacio_4. We can also inform users that this alias will be eventually removed, let's say in six months or a year, or in the next major release, but it will give them a transition period during which they can update their build system.

For example, we could add such an alias to the NCEPLIBS-g2 build, because that includes bacio.

But since to do that, we have to modify every building project, it's just as easy to fix the name instead of adding an alias.

Or am I misunderstanding what you mean by alias?

kgerheiser commented 2 years ago

@edwardhartnett there's a feature in CMake where you can create an alias target which references a target (library, exectuable)

@DusanJovic-NOAA how would you add an alias to the package install? It would be something like:

add_library(bacio::bacio_4 ALIAS bacio::bacio)

in the package config

edwardhartnett commented 2 years ago

OK, cool. Can we likewise alias the include_4 directory?

kgerheiser commented 2 years ago

I think the include directory change is fine. That's handled internally by the target for any project using CMake, so the name change shouldn't matter. But we'll have to adjust our modules (hpc-stack, Spack) to make sure they point to include instead of include_4.

edwardhartnett commented 2 years ago

OK, but since we have to make the break sometime, why not in the very next version?

That is, let's change the library and include directory name with the next release of bacio. Users who are not ready to change their build system can continue using the existing release. Those who are ready to change can upgrade to the new release...

kgerheiser commented 2 years ago

I don't really have a strong opinion. I think a clean break is fine, there's not much development going on in these libraries where that would be an issue.

edwardhartnett commented 2 years ago

OK, let's just go for it with the 2.5.0 release - we will eliminate the _4 from both the library and include directory name.