Hi Corinna,
Corinna Vinschen wrote:
Hi Mark,
On May 22 02:32, Mark Geisert wrote:
On May 25 14:06, Corinna Vinschen wrote:
Modifies winsup/cygwin/Makefile.in to build localtime.o from items in
new winsup/cygwin/tzcode subdirectory. Compiler option "-fpermissive"
is used to accept warnings about missing casts on the return values of
malloc() calls. This patch also removes existing localtime.cc and
tz_posixrules.h from winsup/cygwin as they are superseded by the
subsequent patches in this set.
[...]
@@ -246,6 +246,15 @@ MATH_OFILES:= \
tgammal.o \
truncl.o
+TZCODE_OFILES:=localtime.o
+
+localtime.o: $(srcdir)/tzcode/localtime.cc $(srcdir)/tzcode/localtime.c.patch
+ (cd $(srcdir)/tzcode && \
+ patch -u -o localtime.c.patched localtime.c localtime.c.patch)
+ $(CXX) ${CXXFLAGS} ${localtime_CFLAGS} \
+ -I$(target_builddir)/winsup/cygwin \
+ -I$(srcdir) -I$(srcdir)/tzcode -c -o $@ $<
+
This doesn't work well for me. That rule is the top rule in Makefile.in
now, so just calling `make' doesn't build the DLL anymore, only
localtime.o. The rule should get moved way down Makefile.in.
Oops. My workflow didn't make this apparent to me. Thanks for the fix.
What still bugs me is that we get these -fpermissive warnings (albeit
non-fatal) and the fact that we don't get a dependencies file. On
second thought, there's no good reason to keep localtime.cc a C++ file.
Converting this file to a plain C wrapper drops the C++-specific warning
and thus allows to revert the localtime.o build rule to use ${COMMON_CFLAGS}.
So I took the liberty to tweak your patch a bit. I created a followup
patchset, which I'd like you to take a look at.
I attached the followup patches to this mail. Please scrutinize it and
don't hesitate to discuss the changes. For a start:
- I do not exactly like the name "localtime_wrapper.c" but I don't
have a better idea.
localtime_cygwin.c? cyglocaltime.c? Not much nicer IMO.
- muto's are C++-only, so I changed rwlock_wrlock/rwlock_unlock to use
Windows SRWLocks. I think this is a good thing and I'm inclined
to drop the muto datatype entirely in favor of using SRWLocks since
they are cleaner and langauge-agnostic.
Two changes in my patchset:
- I didn't initialize the SRWLOCK following the books. Fixed that.
- Rather than creating the patched file in the source dir, I changed
the Makefile.in rule so that the patched file is created in the build
dir. This drops the requirement to tweak .gitignore. It's also
cleaner.
- Splitting the build rule for localtime.c.patched from the build rule
for localtime.o makes sure that the patched file is not regenerated
every time we build localtime.o.
I attached my patchset again, but only patch 3 and 4 actually changed.
All the above are great improvements. But I would now remove the "// Get ready
to wrap NetBSD's localtime.c" line and blank line following it. Good to go!
Thank you,
..mark
