Home Original page

remove "-g" (include debug flags) compiler option by alaniwi · Pull Request #305 · NCAS-CMS/cf-python

Thanks for this PR Alan, and sorry for the delayed reply, myself and @davidhassell have been very busy lately. David is now on leave, so I'll review this now.

FYI, I set the CI jobs to run, the failure in the 'Run test suite / test-suite-job-0' which cancelled the other jobs is due to a flaky test which is obviously unrelated to your Makefile change comprising this PR. The important thing is that I can see from the logs that in those jobs (pre-cancellation for all but that one) the 'Make UMRead' step ran fine, e.g:

$ cd /Users/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib
$ make
gcc -Wall -fPIC -DLINUX -c umfile.c
gcc -Wall -fPIC -DLINUX -c error.c
gcc -Wall -fPIC -DLINUX -c filetype.c
gcc -Wall -fPIC -DLINUX -c malloc.c
gcc -Wall -fPIC -DLINUX -c linklist.c
gcc -Wall -fPIC -DLINUX -c new_structs.c
gcc -Wall -fPIC -DLINUX -c swap.c
make -C type-dep
make[1]: Entering directory '/home/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib/type-dep'
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o umfile_test_typedep_sgl.o umfile_test_typedep.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o interpret_header_sgl.o interpret_header.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o read_sgl.o read.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o process_vars_sgl.o process_vars.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o debug_dump_sgl.o debug_dump.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o date_and_time_sgl.o date_and_time.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o compare_sgl.o compare.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o levels_sgl.o levels.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o axes_sgl.o axes.c
gcc -Wall -fPIC -DLINUX -I.. -c -DSINGLE -o unwgdos_sgl.o unwgdos.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o umfile_test_typedep_dbl.o umfile_test_typedep.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o interpret_header_dbl.o interpret_header.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o read_dbl.o read.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o process_vars_dbl.o process_vars.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o debug_dump_dbl.o debug_dump.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o date_and_time_dbl.o date_and_time.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o compare_dbl.o compare.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o levels_dbl.o levels.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o axes_dbl.o axes.c
gcc -Wall -fPIC -DLINUX -I.. -c -DDOUBLE -o unwgdos_dbl.o unwgdos.c
rm -f umfile_typedep.a
ar r umfile_typedep.a  umfile_test_typedep_sgl.o  interpret_header_sgl.o  read_sgl.o  process_vars_sgl.o  debug_dump_sgl.o  date_and_time_sgl.o  compare_sgl.o  levels_sgl.o  axes_sgl.o  unwgdos_sgl.o  umfile_test_typedep_dbl.o  interpret_header_dbl.o  read_dbl.o  process_vars_dbl.o  debug_dump_dbl.o  date_and_time_dbl.o  compare_dbl.o  levels_dbl.o  axes_dbl.o  unwgdos_dbl.o
ar: creating umfile_typedep.a
make[1]: Leaving directory '/home/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib/type-dep'
ld -shared --build-id -o umfile.so umfile.o error.o filetype.o malloc.o linklist.o new_structs.o swap.o --whole-archive type-dep/umfile_typedep.a

so the removal of the -g flag seems to be fine across those cases, including on MacOS:

$ cd /Users/runner/work/cf-python/cf-python/main/cf/umread_lib/c-lib
$ make
clang -Wall -fPIC -DOSX -c umfile.c
clang -Wall -fPIC -DOSX -c error.c
clang -Wall -fPIC -DOSX -c filetype.c
clang -Wall -fPIC -DOSX -c malloc.c
clang -Wall -fPIC -DOSX -c linklist.c
clang -Wall -fPIC -DOSX -c new_structs.c
clang -Wall -fPIC -DOSX -c swap.c
/Applications/Xcode_13.2.1.app/Contents/Developer/usr/bin/make -C type-dep
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o umfile_test_typedep_sgl.o umfile_test_typedep.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o interpret_header_sgl.o interpret_header.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o read_sgl.o read.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o process_vars_sgl.o process_vars.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o debug_dump_sgl.o debug_dump.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o date_and_time_sgl.o date_and_time.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o compare_sgl.o compare.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o levels_sgl.o levels.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o axes_sgl.o axes.c
clang -Wall -fPIC -DOSX -I.. -c -DSINGLE -o unwgdos_sgl.o unwgdos.c
unwgdos.c:371:45: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
      *inum = (*icomp << (31-istart)) & (~0 << 31);
                                         ~~ ^
unwgdos.c:3[9](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:9)6:[27](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:27): warning: shifting a negative signed value is undefined [-Wshift-negative-value]
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o umfile_test_typedep_dbl.o umfile_test_typedep.c
  i = (ui >> ibit) & ~(~0 << 1);
                       ~~ ^
2 warnings generated.
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o interpret_header_dbl.o interpret_header.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o read_dbl.o read.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o process_vars_dbl.o process_vars.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o debug_dump_dbl.o debug_dump.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o date_and_time_dbl.o date_and_time.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o compare_dbl.o compare.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o levels_dbl.o levels.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o axes_dbl.o axes.c
clang -Wall -fPIC -DOSX -I.. -c -DDOUBLE -o unwgdos_dbl.o unwgdos.c
unwgdos.c:371:45: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
      *inum = (*icomp << ([31](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:31)-istart)) & (~0 << 31);
                                         ~~ ^
rm -f umfile_typedep.a
unwgdos.c:[39](https://github.com/NCAS-CMS/cf-python/runs/5130889501?check_suite_focus=true#step:10:39)6:27: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
ar r umfile_typedep.a  umfile_test_typedep_sgl.o  interpret_header_sgl.o  read_sgl.o  process_vars_sgl.o  debug_dump_sgl.o  date_and_time_sgl.o  compare_sgl.o  levels_sgl.o  axes_sgl.o  unwgdos_sgl.o  umfile_test_typedep_dbl.o  interpret_header_dbl.o  read_dbl.o  process_vars_dbl.o  debug_dump_dbl.o  date_and_time_dbl.o  compare_dbl.o  levels_dbl.o  axes_dbl.o  unwgdos_dbl.o
  i = (ui >> ibit) & ~(~0 << 1);
                       ~~ ^
2 warnings generated.
ar: creating archive umfile_typedep.a
clang -dynamiclib -o umfile.so umfile.o error.o filetype.o malloc.o linklist.o new_structs.o swap.o -force_load type-dep/umfile_typedep.a

(Though it works and builds fine, I see there is a warning raised in the above make output, which I will raise as a separate Issue and tag you to check if you wouldn't mind, since I know little C and know you wrote the C for cf-python.)

I'm testing your PR locally now and will submit a review in a moment.