search

rstudio / httpuv

HTTP and WebSocket server package for R
Other
227 stars 86 forks source link

CRAN firedrill 2023-05 #364

Closed jcheng5 closed 1 year ago

jcheng5 commented 1 year ago

1.6.11 release checklist here: https://github.com/rstudio/httpuv/issues/371

jcheng5 commented 1 year ago

I looked into the -DNDEBUG thing, messing with the environment variables that are set for the ./configure call in src/Makevars.

  1. CPPFLAGS="$(CPPFLAGS)" doesn't fix it, as -DNDEBUG is not part of it.
  2. CPPFLAGS="$(ALL_CPPFLAGS)" sort of fixes it--it puts -DNDEBUG in the right place, but using the RDcsan copy of R, the value of ALL_CPPFLAGS contains double quotes around one of the include paths which messes everything up.
  3. CPPFLAGS="-DNDEBUG" fixes it. I think this might be the best option, because the rest of the stuff in ALL_CPPFLAGS is include dirs for R that don't pertain to libuv anyway. But I will defer to the experts.
nealrichardson commented 1 year ago

How about this? 

diff --git a/src/Makevars b/src/Makevars
index 0dd1e62..dfe865d 100644
--- a/src/Makevars
+++ b/src/Makevars
@@ -77,7 +77,7 @@ libuv/Makefile: libuv/m4/lt~obsolete.m4
                sh autogen.sh; \
        fi; \
        chmod +x configure; \
-       CC="$(CC)" CFLAGS="$(CFLAGS) $(CPICFLAGS) $(C_VISIBILITY)" AR="$(AR)" RANLIB="$(RANLIB)" LDFLAGS="$(LDFLAGS)" ./configure $(CONFIGURE_FLAGS)
+       CC="$(CC)" CFLAGS="$(CFLAGS) $(CPICFLAGS) $(C_VISIBILITY) -DNDEBUG" AR="$(AR)" RANLIB="$(RANLIB)" LDFLAGS="$(LDFLAGS)" ./configure $(CONFIGURE_FLAGS)

 libuv/.libs/libuv.a: libuv/Makefile
        $(MAKE) --directory=libuv \

How would I verify in CI that the problem is resolved?

jcheng5 commented 1 year ago

@nealrichardson That would definitely solve it--it was one of the attempts I tried yesterday

nealrichardson commented 1 year ago

Done in https://github.com/rstudio/httpuv/pull/369