Re: [Opengrads-devel] the build environment

[Arlindo da S. <da...@al...>]

On Wed, May 14, 2008 at 11:47 AM, Jennifer Adams <jm...@co...> wrote:

>
> On May 12, 2008, at 11:29 PM, Arlindo da Silva wrote:
>
> On Mon, May 12, 2008 at 6:26 PM, Jennifer Adams <jm...@co...> wrote:
>
>>
>> for v1.9 and more recently adapted it for v2. We don't claim that what is
>> on the OpenGrADS repo is perfect but it is a pretty good start.  Some of the
>> small changes we made to the v2 codebase were made to support the new build,
>> and to fix some potentially hard to find bugs (for example, "grads.h" no
>> longer includes "netcdf.h").
>>
>> Why is that a bug?
>>
>
> The problem is that both "grads" and "gradsdap" link against the same
> "grads.o". So, you end up linking "gradsdap" with something that was
> compiled with the wrong "netcdf.h" (the one from the NetCDF library, rather
> than the one from nc-dap). Since both versions of "netcdf.h" currently in
> use are very similar, I have no evidence that your v2 build has a problem.
> However, we've been bitten by this netcdf.h mismatch issue several times in
> the past that it pays to play it save.
>
>
> Why should grads.o be different for grads and gradsdap? There are no netcdf
> dependent calls in there. All the netcdf library calls are in gasdf.c,
> gaio.c and gauser.c -- these three files have:
> #include netcdf.h
> and have separate .o files when linking grads and gradsdap. The other .o
> files that are separated for the two executables are gaddes and gacfg
> because there are some netcdf dependencies there but no calls to the
> library.
>
>
Having a separate grads.o would solve this problem, but if we don't watch
soon enough we would having 2 versions of a lot of files (since grads.h is a
very popular header file). My solution was to move all the sdf prototypes
from grads.h to sdf.h, and not include sdf.h at all in grads.h (it is not
needed).

>From an architectural point of view the current approach of adding all
possible prototypes flat in grads.h is somewhat problematic as it violates
"information hiding" and get's in the way of encapsulation.  (It would also
creates a configuration management nightmare in an environment with multiple
developers all making changes to grads.h).  In principle, only the "public"
functions of a given subpackage (e.g., gasdf) would need its prototype
broadcast through "grads.h". Internal (read: private) functions and
datatypes should be kept in a separate header file and only included by the
subpackage itself (or at the top of the file as it is done in many places
throughout grads).  Even in this case, "grads.h" should *include*  the
"public" header files rather than have them all flat in the same file.

Another suggestion.  Right now every time a given feature is not compiled in
you get an error message trying to use it:

ga-> printim
Unknown command: printim

something which can be very confusing for a new user. A message like:

  This version of GrADS has not been built with "printim" support.

would be more user friendly. One way to achieve this, which is also more
modular and would eliminate all those USE??? scattered through the code,
would be to have each subpackage compiling in a stub when it is not being
USEd. Of course the package API would be unchanged: if the subpackage is not
being used only the subpackage needs to know about it.
The same goes for gasdf.c. If the NetCDF and nc-dap versions have the exact
same  API and public header, you only need 2 versions of gasdf.o, all the
rest of the GrADS code (except for gacfg.c) would stay exactly the same.
This kind of encapsulation would simplify the build tremendously, and
ultimately is much easier to maintain.

  I hope you don't mind my unsolicited suggestions...

  Regards,

       Arlindo


-- 
Arlindo da Silva
da...@al...