search

TEOS-10 / GSW-C

C implementation of the Thermodynamic Equation Of Seawater - 2010 (TEOS-10)
Other
18 stars 17 forks source link

rename *_data.c to *_data.h #50

Closed bluesquall closed 2 years ago

bluesquall commented 2 years ago

If you're not going to #include a header with the macro, you need to #define it in the source file.

I noticed this when make check failed in my autotools branch after merging recent changes from upstream. I suspect you don't experience it with your manual Makefile because of the specific way you call gcc. But if you want to be able to compile gsw_saar_data.c -> gsw_saar_data.o without implicitly depending on additional files, you need to define the macro within.

You would also use a #ifndef ... #define ... #endif block, if you'd prefer.

efiring commented 2 years ago

It doesn't work with the Windows compiler; see how the definition is handled in gsw_internal_const.h. But beyond that, we are simply including gsw_saar_data in gsw_saar, which works fine. We don't need or use or make gsw_saar_data.o.

bluesquall commented 2 years ago

Interesting … I will look again later and see if I can come up with a solution that works with autotools (or Cmake, or another build system) and with your windows test.

Using a build system is critical for my sanity when deploying as a dependency on various systems. I went with autotools first because one of the systems I have needs to be cross-compiled and I had other examples to use for reference, but I’m open to alternatives in general.

efiring commented 2 years ago

Would the problem be solved by renaming gsw_saar_data.c to gsw_saar_data.h, and continuing to #include it as we do now? Is the problem that autotools assumes any .c file will be compiled to .o rather than inserted via #include?

bluesquall commented 2 years ago

That’s a good, simple approach. I will try it later today when I get back into code.

On Thu, Jul 21, 2022, at 19:52, Eric Firing wrote:

Would the problem be solved by renaming gsw_saar_data.c to gsw_saar_data.h, and continuing to #include it as we do now? Is the problem that autotools assumes any .c file will be compiled to .o rather than inserted via #include?

— Reply to this email directly, view it on GitHub https://github.com/TEOS-10/GSW-C/pull/50#issuecomment-1192126179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PZWJYBXP7SFOOOZWWH3VVIEIVANCNFSM54JNM7OQ. You are receiving this because you authored the thread.Message ID: @.***>

bluesquall commented 2 years ago

Tested with my autotools branch and it seems to have worked.

It looks like the windows test only runs on a PR with approval from a maintainer.

bluesquall commented 2 years ago

Huh. I thought I got that before I pushed the rebased changes. I'll check again.

On Fri, Jul 22, 2022, at 16:23, Eric Firing wrote:

@.**** requested changes on this pull request.

I think this will be fine once you remove the #define statement.

In gsw_saar_data.h https://github.com/TEOS-10/GSW-C/pull/50#discussion_r928041151:

@@ -2,6 +2,7 @@ $Id$ Extracted from ../../gsw_matlab_v3_06_11/library/gsw_data_v3_0.mat */ +#define UNUSED attribute ((unused))

This line should be removed now.

— Reply to this email directly, view it on GitHub https://github.com/TEOS-10/GSW-C/pull/50#pullrequestreview-1048464421, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5PZXZ6VF5PQKTG2UWZ3TVVMUP7ANCNFSM54JNM7OQ. You are receiving this because you authored the thread.Message ID: @.***>

efiring commented 2 years ago

Thank you!