New Wrap: cfitsio-4.5.0 #1767
New Wrap: cfitsio-4.5.0 #1767
Conversation
You might want to just require a newer version of Meson instead. |
kjmeagher
force-pushed
the
cfitsio-4.5.0
branch
4 times, most recently
from
796bc80
to
16a96b3
Compare
| cc = meson.get_compiler('c') | ||
| libm = cc.find_library('m') | ||
|
|
||
| if target_machine.system() != 'windows' |
There was a problem hiding this comment.
target_machine is used when the thing you are creating is a cross compiler that will, when built and installed and run, emit platform specific outputs for a platform that isn't the one it is installed for.
Probably you mean host_machine instead, as that is the machine that libcfitsio is being compiled to run on.
| curl = dependency( | ||
| 'libcurl', | ||
| required: get_option('curl'), | ||
| fallback: ['curl', 'libcurl'], |
There was a problem hiding this comment.
Fallback is probably not necessary since curl wraps can just [provide] curl = curl_dep.
| zlib = dependency('zlib', fallback: ['zlib', 'zlib_dep']) | ||
|
|
||
| if target_machine.system() != 'windows' | ||
| bzip2 = dependency( |
There was a problem hiding this comment.
bzip2 doesn't provide pkg-config files, please detect it with cc.find_library instead.
There was a problem hiding this comment.
both the platforms i developed this for (arch linux and macos) have pkg-config files for bzip2. but since they are not provided by up stream i will switch to find_library()
|
|
||
| if get_option('sse2') | ||
| if cc.has_argument('-msse2') | ||
| add_project_arguments('-msse2', language: 'c') |
There was a problem hiding this comment.
This seems like an odd thing to have as a project option. Can't users just compile with -msse2 themselves, especially given its often the default for a given -march?
There was a problem hiding this comment.
I was copying from the autotools/cmake file, but you are right this should be removed
|
|
||
| install_headers('fitsio.h', 'fitsio2.h', 'longnam.h') | ||
|
|
||
| libcfitsio_la_SOURCES = [ |
There was a problem hiding this comment.
The _la refers to "libtool archive" so I wonder how useful it is to have that in the name.
| ] | ||
| endif | ||
|
|
||
| libcfitsio = shared_library( |
There was a problem hiding this comment.
Does upstream really not support installing this as a static library e.g. using --enable-static --disable-shared? Usually libtool provides these options for free.
There was a problem hiding this comment.
this library is often used as a static library, switched to library()
| 'fitscopy', | ||
| 'utilities/fitscopy.c', | ||
| dependencies: cfitsio_dep, | ||
| install: get_option('utils'), |
There was a problem hiding this comment.
This option should be used in an "if" statement to control whether the executable() is defined -- not whether the install parameter is set.
| option( | ||
| 'utils', | ||
| type: 'boolean', | ||
| description: 'Build fpack, funpack, fitscopy, and fitsverify executables', |
There was a problem hiding this comment.
... and as we see here the option does indeed claim it will control building, not control installing.
kjmeagher
force-pushed
the
cfitsio-4.5.0
branch
9 times, most recently
from
795c840
to
219ab0b
Compare
This is a file wrap with a patch. It is mostly based on the CMakeLists.txt provided in the source. But there were a few defines in the autotolls version that werent in the cmake that I also added.
kjmeagher
force-pushed
the
cfitsio-4.5.0
branch
from
1b0c510
to
aeb55f0
Compare
This is a file wrap with a patch. It is mostly based on the
CMakeLists.txt provided in the source. But there were a few defines
in the autotolls version that werent in the cmake that I also added.