[PATCH 1/3 v3] Cygwin: tzcode resync: basics
Corinna Vinschen
corinna-cygwin@cygwin.com
Tue May 26 08:27:36 GMT 2020
Hi Mark,
On May 26 00:09, Mark Geisert wrote:
> 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 [...etc...]
> > > 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.
(Belatedly) done.
> Good to go!
Great! I added two more tweaks:
- I renamed the generated file from localtime.c.patched to
localtime.patched.c so the .c suffix remains intact. Seems
a bit cleaner to me. I also added it to the 'clean' rule,
so that it gets removed at `make clean' time.
- I simplified the #includes in the wrapper file. The paths to these
headers are searched anyway, so they don't have to be written out
explicitely.
> Thank you,
Good job, thank you!
Corinna
--
Corinna Vinschen
Cygwin Maintainer
More information about the Cygwin-patches
mailing list