Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove PROJ_COMPILATION=1 definition #1425

Merged
merged 1 commit into from May 1, 2019
Merged

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Apr 16, 2019

These do not appear to be used, and I can't see what they are for. They were added from the d928db1 mega commit for the "GDAL SRS barn" work.

@rouault is it OK to remove these?

@rouault
Copy link
Member

rouault commented Apr 16, 2019

is it OK to remove these?

yes, can't remember why this was added and doesn't appear to be used anywhere

@cffk
Copy link
Contributor

cffk commented Apr 16, 2019

I can see a use for PROJ_COMPILATION...

I'd like to have geodesic.[ch] usable independently of PROJ (it's
currently a separate mini-project within GeographicLib). In that case
it would be nice if the compiler could know if geodesic.c were being
compiled as part of PROJ (in which case proj_math.h can be included) or
not. Currently this uses the PJ_LIB__ macro, but I don't think this
ever worked. However PROJ_COMPILATION would fill the bill.

@mwtoews
Copy link
Member Author

mwtoews commented Apr 17, 2019

@cffk sure, we can keep it.

Are there any other use-cases for this define? If not, then with the CMake setup, I may just reduce the scope of this define to lib_proj.cmake via:

# Use PROJ_COMPILATION to distinguish code compiled as part of PROJ 
set_source_files_properties(
  geodesic.c geodesic.h
  PROPERTIES COMPILE_DEFINITIONS PROJ_COMPILATION
)

And with the autotools setup, just set it in AM_CPPFLAGS, but it'd still be global.

@cffk
Copy link
Contributor

cffk commented Apr 17, 2019

Since it'll be a few months before I'm in a position to update the geodesic
routines, why don't you remove the definition for now. I'll add it back in
when I need it (narrowing the scope if I can).

@mwtoews
Copy link
Member Author

mwtoews commented Apr 18, 2019

@cffk great to know about new development with the C geodesic routines! And yup, it should be simple to re-apply this macro to autoconf/cmake when the time comes.

@kbevers kbevers merged commit 1a8068b into OSGeo:master May 1, 2019
cffk added a commit to cffk/proj.4 that referenced this pull request Sep 17, 2019
The supported C99 math functions provided by math.cpp are thus

   hypot
   log1p
   asinh
   atanh
   copysign
   cbrt
   remainder
   remquo
   round
   lround

Make compiler checks in CMakeLists.txt and configure.ac consistent with
this set.

Make geodesic.c use the math.cpp defined (instead of the internally
defined) versions of

   hypot
   atanh
   copysign
   cbrt

This is keyed off the presence of the PROJ_LIB macro.  I had at one
time

    OSGeo#1425

suggested supplying an additional macro PROJ_COMPILATION when compiling
geodesic.c.  However, PROJ_LIB seems to fill the bill OK.

The *next* version of geodesic.c (due out in a few weeks) will also use

   remainder
   remquo

All of this is only of concern for C compilers without C99 support.  So
this may become an historical footnote at some point.
@mwtoews mwtoews deleted the define branch March 4, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants