Protect windows file with #ifdef

[eddelbuettel]

When not using cmake (as I do in the R integration of ccztz in package RcppCCTZ) the source file src/time_zone_name_win.cc may be included which triggers an error on non-Windows systems. The workaround is a simple preprocessor test which I added here. I understand that this facility may not be a primary concern for you but as the fix is simple and free of side-effects it would be nice if you could accommodate it.

[devbww]

Hi Dirk,

I'm not diametrically opposed to this.

However, I was undoubtedly the one who requested that platform-specific code be preferably treated using the build system and/or file system rather than by conditional compilation, and it would be nice if we continued, and even extended that trend (* see below) instead of reversing it.

I took a quick look at your repository, but couldn't really see how things are built. It appears that R supports Windows though, so my question to you is what your build systems can do to support platform-specific source files (similar to what CCTZ does with bazel and cmake), rather than just (apparently) trying to build everything. I proffer that would be a more desirable path forward.

(*) I probably didn't go far enough in my reviews of this platform-specific code. It would be better if there was a platform-independent GetLocalTimeZoneName() interface that was implemented in platform-specific files, rather than the mismatch of files and #ifdefs that we currently have. It would also be nicer (methinks) if we used a "<platform>/" prefix for such implementations instead of a "_<platform>" suffix.

[eddelbuettel]

Hi there,

Yes the key difference is that cmake allows you to modify the set of files. R, by default, uses one fixed tar.gz and then two different Makefiles snippets called src/Makevars and src/Makevars.win: one for Unix systems including macOS, and one for Windows.

So we could play games here and have a (shell) script configure copy the two files in.

Or we could play a different games and explicitly list the files built -- and have those differ between the Windows and non-Windows builds. But ... why? Adding a #define is clean, simple and well understood. R has a (by default) build "simply": it can work without a Makevars, and implicit make rules work. All files found are compiled and linked into the dynamically loadable extension a package brings. So my preference would be to keep it simple, following best and time-honoured practices of 'least surprises'.

Regarding your last paragraph, we could go that route and always have the stub function present and only on Windows call out to that platform specific code. I have written and published R packages at CRAN that do that, see for example RcppAPT which really only can work where libapt-dev exists and is empty even on, say, Fedora. See https://github.com/eddelbuettel/rcppapt/tree/master/src, the #define is set from configure.

Anyway, most importantly I would prefer for us to have this be 'low touch' -- I have happily provided your very nice cctz library here for a decade here and mostly got by without crazy acrobatics (and as I recall on occassional reported a minor need for a specific compiler tweak back to you even though I apparently never forked this).

[eddelbuettel]

I guess I could enumerate the files as in

SOURCES = \
   civil_time_detail.cc \
   time_zone_fixed.cc \
   time_zone_format.cc \
   time_zone_if.cc \
   time_zone_impl.cc \
   time_zone_info.cc \
   time_zone_libc.cc \
   time_zone_lookup.cc \
   time_zone_posix.cc \
   zone_info_source.cc 

OBJECTS = $(SOURCES:.cc=.o)

$(SHLIB): $(OBJECTS)

and that satisfy the R standards ("no GNU make extension or else declare a GNU make dependency") and for Windows I could add the one file. A bit tedious as I know need to maintain a master list but you could probably (and rightly) argue that I already did that by excluding the test and benchmark files.

(For context, R takes the 'snippet' that is src/Makevars and creates / forms a full Makefile around it. That Makefile is ephemeral and disappears.)

[eddelbuettel]

Ok, I have the alternate approach working (as validated by uploading to the (hosted-by-the-R-Project) win-builder along with local testing under Linux.

I still find the two-line (#if defined, and #endif) change simpler but your project, your rules. I reserve myself the same rights for my projects so naturally am playing along here too.

[eddelbuettel]

Closing in favor of now merged eddelbuettel/rcppcctz#47