TEOS-10 / GSW-R

R implementation of the Thermodynamic Equation Of Seawater - 2010 (TEOS-10)
http://teos-10.github.io/GSW-R/
Other
8 stars 5 forks source link

CRAN release to fix a problem with renamed C macros #63

Closed dankelley closed 2 months ago

dankelley commented 2 months ago

At https://www.stats.ox.ac.uk/pub/bdr/Strict/gsw.out, the following errors are reported. That machine uses "strict" checking. I will look into fixing this. I learned of this following a submission of oce to CRAN. From the formatting of the webpage, I get the impression that fixing this is not mandatory, but I suppose CRAN might switch to "strict" checking as a standard, so I ought to address this now and make a new release.

I want to speak with @richardsc before making a release, though. There may be something else that ought to be done. (I don't think so, actually. I try to keep up with GSW-C changes, and these are the kernel of actions here.)

gcc-14 -I"/data/gannet/ripley/R/R-devel/include" -DNDEBUG   -I/usr/local/include  -DSTRICT_R_HEADERS=1  -fpic  -g -O2 -Wall -pedantic -mtune=native -Wp,-D_FORTIFY_SOURCE=3 -fexceptions -fstack-protector-strong -fstack-clash-protection -fcf-protection -Werror=implicit-function-declaration -Wstrict-prototypes  -c wrappers.c -o wrappers.o
wrappers.c: In function 'set_up_gsw_data':
wrappers.c:34:21: error: implicit declaration of function 'Calloc'; did you mean 'calloc'? [-Wimplicit-function-declaration]
   34 |         longs_ref = Calloc(gsw_nx, double);
      |                     ^~~~~~
      |                     calloc
wrappers.c:34:36: error: expected expression before 'double'
   34 |         longs_ref = Calloc(gsw_nx, double);
      |                                    ^~~~~~
wrappers.c:38:35: error: expected expression before 'double'
   38 |         lats_ref = Calloc(gsw_ny, double);
      |                                   ^~~~~~
wrappers.c:42:32: error: expected expression before 'double'
   42 |         p_ref = Calloc(gsw_nz, double);
      |                                ^~~~~~
wrappers.c:48:34: error: expected expression before 'double'
   48 |         ndepth_ref = Calloc(nxy, double);
      |                                  ^~~~~~
wrappers.c:53:33: error: expected expression before 'double'
   53 |         saar_ref = Calloc(nxyz, double);
      |                                 ^~~~~~
wrappers.c:57:37: error: expected expression before 'double'
   57 |         delta_sa_ref = Calloc(nxyz, double);
      |                                     ^~~~~~
wrappers.c: In function 'clear_gsw_data':
wrappers.c:74:5: error: implicit declaration of function 'Free'; did you mean 'free'? [-Wimplicit-function-declaration]
   74 |     Free(longs_ref);
      |     ^~~~
      |     free
make[1]: *** [/data/gannet/ripley/R/R-devel/etc/Makeconf:195: wrappers.o] Error 1
dankelley commented 2 months ago

Hm, at https://cran.r-project.org/doc/manuals/R-exts.html#index-R_005fCalloc I see that there is R_Calloc() etc. A few lines down from that spot in the manual, I see as follows. My guess is that the fix is merely to use those macros. But are they defined in R.h, or where?

Historically the macros Calloc, Free and Realloc were used, and these remain available unless STRICT_R_HEADERS was defined prior to the inclusion of the header.

dankelley commented 2 months ago

I can reproduce the message with

clang -arch x86_64 -DSTRICT_R_HEADERS=1 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG   -I/opt/R/x86_64/include    -fPIC  -falign-functions=64 -Wall -g -O2  -c wrappers.c -o wrappers.o
dankelley commented 2 months ago

Hm. Local tests work fine but a remote one fails with a not-helpful error message when I run it interactively, but works when I run my check script with Rscript. I think this is a hickup with how things are done interactively.

I'll look into this in more detail later today.

Also, the check_package.R script uses the old rhub. Switching to the new one is tricky in this way, but I think it's not hard with a github action. If I'm right in that, I'll switch to testing that way. (The rhub tests are always a pain. But at least gsw does not need to use the ncdf4 package, so I think the 6-month-old in ncdf4 building won't be a roadblock, as it is for oce.)

dankelley commented 2 months ago

All the remote checks worked fine, as the revdep checks did.

On the GSW-R page I see

CRAN checks: | gsw results [issues need fixing before 2024-09-20]

so we ought not to delay much.

I'll rebuild oce with this trial update to gsw, and if that works, I will likely submit gsw to CRAN later today, unless a co-developer objects.

dankelley commented 2 months ago

I also noticed some problems with the badges in README.md so I've fixed them as well.

I forgot that @richardsc is still out of contact, so I am not going to wait until we can discuss this in person. I am doing my normal suite of local and remote tests, and if they all pass, I'll submit to CRAN as early as later today. After all, the conversion from Calloc() to R_Calloc() is something I did a long time ago in the oce package, so I know for sure that the simple search-replace operation is sufficient, given that the package builds and checks.

dankelley commented 2 months ago

Ah, not a "future" problem, really ... below is an email I got today

Dear maintainer,

Please see the problems shown on https://cran.r-project.org/web/checks/check_results_gsw.html.

Specifically, please see the Strict additional issue.

Compilation fails with _R_USE_STRICT_RHEADERS=true, which defines STRICT_R_HEADERS to 1 which removes

The aim is to clean the namespace: in particular having a definition for Free has conflicted with some packages' C++ code.

It is planned that STRICT_R_HEADERS=1 will become the default for 4.5.0, which in particular makes it necesssary that all CRAN packages with many strong reverse dependencies compile/install ok with the new default.

Your package is among the ones with many strong reverse dependencies. We would thus really appreciate if you could provide a new version of your package as soon as possible which checks ok with STRICT_R_HEADERS=1.

You can verify that your package checks ok with STRICT_R_HEADERS=1 via R CMD check --as-cran using a current version of R-devel.

Please correct before 2024-09-20 to safely retain your package on CRAN.

Best, -k

dankelley commented 2 months ago

I just submitted a new version to CRAN (see Details below). It normally takes an hour or so before I hear back about problems, and a few days before all the test builds finish. I will. keep this issue open until all is okay and gsw has been updated on the CRAN servers.

CAUTION: The Sender of this email is not from within Dalhousie. [This was generated from [CRAN.R-project.org/submit.html](http://cran.r-project.org/submit.html)] The following package was uploaded to CRAN: =========================================== Package Information: Package: gsw Version: 1.2-0 Title: Gibbs Sea Water Functions Author(s): Dan Kelley [aut, cre, cph] (), Clark Richards [aut, cph] (), WG127 SCOR/IAPSO [aut, cph] (Original 'Matlab' and derived code) Maintainer: Dan Kelley <[dan.kelley@dal.ca](mailto:dan.kelley@dal.ca)> Depends: R (>= 3.5.0), Suggests: knitr, rmarkdown, testthat Description: Provides an interface to the Gibbs 'SeaWater' ('TEOS-10') C library, version 3.06-16-0 (commit '657216dd4f5ea079b5f0e021a4163e2d26893371', dated 2022-10-11, available at , which stems from 'Matlab' and other code written by members of Working Group 127 of 'SCOR'/'IAPSO' (Scientific Committee on Oceanic Research / International Association for the Physical Sciences of the Oceans). License: GPL (>= 2) | file LICENSE The maintainer confirms that he or she has read and agrees to the CRAN policies. Submitter's comment: # Submission of 1.2-0 This update was required by 2024-09-20, in order to accommodate the CRAN-requested change (on that date) relating to the need to replace the Calloc() and Free() calls with R_Calloc() and R_Free() calls. I wish to thank the CRAN team for sending that email, since I do not check the CRAN-results pages as frequently as I ought to, for this mainly-stable package. The only other change from the previous CRAN version is the addition of the function `gsw_infunnel()`. This is mainly for developers' use and, in any case, its provision does not affect the actions of the rest of the package, so it should not affect users adversely. # Tests ## Local Tests I saw no problems with R 4.4.1 on macOS Beta 15.0 Beta (24A5298h) revealed no ERRORs, no WARNINGs, or NOTEs (apart from the usual NOTE about the author name). ## Remote tests * I saw no problems on win-builder (devel and release). * I saw no problems on Github R-CMD-check action tests. ================================================= Original content of DESCRIPTION file: Package: gsw Version: 1.2-0 Title: Gibbs Sea Water Functions Authors@R: c( person(given="Dan", family="Kelley", email="[dan.kelley@dal.ca](mailto:dan.kelley@dal.ca)", role=c("aut","cre","cph"), comment=c(ORCID="https://orcid.org/0000-0001-7808-5911")), person(given="Clark", family="Richards", email="[clark.richards@gmail.com](mailto:clark.richards@gmail.com)", role=c("aut","cph"), comment=c(ORCID="https://orcid.org/0000-0002-7833-206X")), person(given="WG127", family="SCOR/IAPSO", role=c("aut","cph"), comment="Original 'Matlab' and derived code")) Copyright: Original algorithms and 'Matlab'/C library (c) 2015-2023 WG127 SCOR/IAPSO (Scientific Committee on Oceanic Research / International Association for the Physical Sciences of the Oceans, Working Group 127); C wrapper code and R code (c) 2015-2023 Dan Kelley and Clark Richards Maintainer: Dan Kelley <[dan.kelley@dal.ca](mailto:dan.kelley@dal.ca)> Depends: R (>= 3.5.0), Suggests: knitr, rmarkdown, testthat BugReports: https://github.com/TEOS-10/GSW-R/issues Description: Provides an interface to the Gibbs 'SeaWater' ('TEOS-10') C library, version 3.06-16-0 (commit '657216dd4f5ea079b5f0e021a4163e2d26893371', dated 2022-10-11, available at , which stems from 'Matlab' and other code written by members of Working Group 127 of 'SCOR'/'IAPSO' (Scientific Committee on Oceanic Research / International Association for the Physical Sciences of the Oceans). URL: http://teos-10.github.io/GSW-R/ License: GPL (>= 2) | file LICENSE LazyLoad: yes LazyData: no Packaged: 2024-08-19 16:29:53 UTC; kelley Encoding: UTF-8 RoxygenNote: 7.3.2 BuildVignettes: true VignetteBuilder: knitr NeedsCompilation: yes Author: Dan Kelley [aut, cre, cph] (), Clark Richards [aut, cph] (), WG127 SCOR/IAPSO [aut, cph] (Original 'Matlab' and derived code)
dankelley commented 2 months ago

The first hurdle has been passed (see Details for the email I received).

Dear maintainer, package gsw_1.2-0.tar.gz has been auto-processed and is pending an automated reverse dependency check. This service will typically respond to you within the next day. For technical reasons you may receive a second copy of this message when a team member triggers a new check. Log dir: The files will be removed after roughly 7 days. Installation time in seconds: 11 Check time in seconds: 107 R Under development (unstable) (2024-08-17 r87027 ucrt) Pretests results: Windows: Status: OK Debian: Status: OK Last released version's CRAN status: OK: 13 See: Last released version's additional issues: Strict CRAN Web: *** Strong rev. depends ***: oce seacarb vprr Best regards, CRAN teams' auto-check service Flavor: r-devel-linux-x86_64-debian-gcc, r-devel-windows-x86_64 Check: CRAN incoming feasibility, Result: Note_to_CRAN_maintainers Maintainer: 'Dan Kelley <[dan.kelley@dal.ca](mailto:dan.kelley@dal.ca)>'
dankelley commented 2 months ago

Second hurdle passed as I was making note of the first one :-)

Dear maintainer, thanks, package gsw_1.2-0.tar.gz is on its way to CRAN. Best regards, CRAN teams' auto-check service Package check result: OK No changes to worse in reverse depends.
dankelley commented 2 months ago

Just one test machine to go. I expect to close this issue in a day or two, when that completes. I check about once a day or 2 days.

dankelley commented 2 months ago

I'm closing this now, since all the test machines have now built gsw and checked it, with no reported NOTEs, WARNINGs or ERRORs.