Re: guile-lib - devel branch - patch 4 of 11
Hello Eli, > > Don't you understand? How could I possibly answer that quiz, since it's > > been 4 > > years I use 2.69? You can't tell for sure just because there has been no > > complain: you can only tell for sure if someone you 'trust' check with what > > ever > > version you'd like to use. > Problems in past versions of development tools are described in the > documentation, and if that's not enough, the maintainers of the tools > can be asked about them. There should be no need to learn about that > from personal experience alone. I totally disagree with your last sentence, as a consequence of both what I've been teached and what my experience 'told' me. So, if you have a well isolated environment using an earlier version of autoconf and automake 1.14, I'll trust you. Besides, should I agree with you, but I don't, I still have absolutely no time and no interest to read about a 5y doc and ask maintainers quiz about 5y old version of these tools. > > Besides, 'users' who locally manually install and compile GDB probably know > > a lot > > more then I on the subject :) > I don't think the fact that I build my own GDB means my time is cheap > and should be disregarded. I never implied that, this is a very unfair statement: you are the one who ask me to spend my time to read and talk to maintainers about a more then 4y tool and doc. At the very most, all you have to do is to locally edit the guile-lib configure.ac and change AC_PREREQ(2.69) to what ever version you use and 'pass': that does take any time. > AC_PREREQ(2.69) > > even though an older version would do, is IMO not a good idea. This has nothing to do with an 'idea', it's about what we can guarantee: as I said above, it's not ok for me to 'just' read, and if you can test 'make distcheck' and tells me it's ok, using an earlier version of autoconf and automake 1.14, I'll trust you. > And if you still disagree, let's leave it at that. I'm not speaking > for the Guile project, so my opinion can be easily overridden. I don't speak in the name of Guile either, I am contributor. Note that this is Guile-Lib, not Guile (not that it would change my position wrt this, but...). Thanks, David pgpf4xZLlYSgZ.pgp Description: OpenPGP digital signature
Re: Update 'uname' emulation on MS-Windows
On Sun 24 Jul 2016 16:32, Eli Zaretskii writes: >> Does the uname MinGW patch introduce a regression in any case that we >> care about? > > No. On the contrary: where before Guile would only report correct OS > version up to and including XP, it now reports correct values also for > Vista, Windows 7, and Windows 8. Versions beyond that require the > manifest, and otherwise will be reported as 8.0. > > Also, the modern CPU types are now supported, including 64-bit ones, > which were not reported at all before. > >> Is it a user-visible change? If so an update to NEWS might be >> needed. > > From my POV, this is a bugfix: APIs that were reporting inaccurate > values now report more accurate ones. Thanks for the info, and the patches of course :) Andy
Re: Avoid warning about alloca in read.c
> From: Andy Wingo > Cc: guile-devel@gnu.org > Date: Sun, 24 Jul 2016 15:35:08 +0200 > > >> > +#ifdef __MINGW32__ > >> > +#include > >> > +#endif > >> > + > >> > >> OK to commit but please remove the ifdef -- just include in all > >> cases. > > > > Is that header available on all supported platforms? > > Yes, we use a Gnulib module to ensure that some workable version is > present. Thanks, pushed.
Re: MinGW-related patches that were reported in 2014 but not applied
> From: Andy Wingo
> Cc: guile-devel@gnu.org
> Date: Sat, 23 Jul 2016 22:36:17 +0200
>
> >> > * libguile/stime.c (scm_strftime) [__MINGW32__]: Don't use the
> >> > trick of appending "0" to the time-zone string, Windows runtime
> >> > doesn't support that.
> >> > +#ifndef __MINGW32__
> >> > +/* Don't do this for MinGW: it only supports fixed-format
> >> > + TTTnnnDDD TZ specifications, and gets confused if a zero is
> >> > + appended. */
> >>
> >> This patch disables the setzone() call entirely; seems to be the wrong
> >> thing, given that MinGW doesn't appear to have struct tm->tm_zone. What
> >> if we just disable appending the 0 to the time zone?
> >
> > I'm not sure I understand what you have in mind, but if you show a
> > patch and a simple test, I can run it here.
>
> The current code:
>
> #if !defined (HAVE_STRUCT_TM_TM_ZONE)
> /* it seems the only way to tell non-GNU versions of strftime what
>zone to use (for the %Z format) is to set TZ in the
>environment. interrupts and thread switching must be deferred
>until TZ is restored. */
> char **oldenv = NULL;
> SCM zone_spec = SCM_SIMPLE_VECTOR_REF (stime, 10);
> int have_zone = 0;
>
> if (scm_is_true (zone_spec) && scm_c_string_length (zone_spec) > 0)
> {
> /* it's not required that the TZ setting be correct, just that
>it has the right name. so try something like TZ=EST0.
>using only TZ=EST would be simpler but it doesn't work on
>some OSs, e.g., Solaris. */
> SCM zone =
> scm_string_append (scm_list_2 (zone_spec,
> scm_from_locale_string ("0")));
>
> have_zone = 1;
> SCM_CRITICAL_SECTION_START;
> oldenv = setzone (zone, SCM_ARG2, FUNC_NAME);
> }
> #endif
>
> You said that appending the 0 confuses MinGW, so you skip this section
> and the corresponding one after the nstrftime. But you're skipping the
> setzone too, so I don't see how the strftime tests would work.
Appending 0 confuses MinGW because time-zone strings on Windows can be
something like "Jerusalem Standard Time", in which case appending 0
will not DTRT. If the TZ string is a 3-letter name like EST, then
EST0 will indeed work on Windows.
But what setzone does causes much more trouble: it replaces the
environ array with one that has a single variable TZ, and that
confuses the heck out of the Windows C runtime. Windows programs
cannot run with an empty environ.
Frankly, I don't understand why all this stuff with setzone is needed,
because on Windows all that's required to do what I think this code is
trying to do is this:
putenv ("TZ=whatever");
tzset ();
tm->tm_isdst = 0;
The last bit is required for when the "whatever" zone doesn't have a
DST name (like "EDT"), in which case %Z will produce an empty string.
The code already does call tzset, so it should simply call putenv and
set the isdst member, instead of using this strange method implemented
in setzone. Am I missing something?
> According to POSIX, our TZ value is valid:
>
> http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
>
> What is the MinGW restriction? You mention it in the comment but I
> don't understand the comment.
If you are asking about the forms of the TZ variable, they are
documented here:
https://msdn.microsoft.com/en-us/library/90s5c885(v=vs.71).aspx
But that's not the important problem with setzone.
Of course, the same problems exist with localtime and mktime: they
should also call putenv instead of setzone.
Re: Avoid warnings in sockets.c when HAVE_SIN6_SCOPE_ID is unavailable
> From: Andy Wingo > Cc: guile-devel@gnu.org > Date: Sat, 23 Jul 2016 22:55:50 +0200 > > > CC libguile_2.0_la-socket.lo > > socket.c: In function 'scm_fill_sockaddr': > > socket.c:747:16: warning: variable 'scope_id' set but not used > > [-Wunused-but-set-variable] > > unsigned long scope_id = 0; > > ^ > > > > The patch to avoid this warning is below. OK to commit? > > I don't think so -- whether the underlying system has scope_id or no, we > need this assert to happen: > > > +#ifdef HAVE_SIN6_SCOPE_ID > > SCM_VALIDATE_ULONG_COPY (which_arg + 3, SCM_CAR (*args), > >scope_id); > > +#endif > > Not sure what the right solution is. How about something this instead: #ifdef HAVE_SIN6_SCOPE_ID SCM_VALIDATE_ULONG_COPY (which_arg + 3, SCM_CAR (*args), scope_id); #else (void)SCM_NUM2ULONG (which_arg + 3, SCM_CAR (*args)); #endif
Re: Update 'uname' emulation on MS-Windows
> From: Andy Wingo > Cc: andrewjmore...@gmail.com, guile-devel@gnu.org > Date: Sat, 23 Jul 2016 23:03:46 +0200 > > >> > Does the Mingw toolchain supply a suitable manifest automatically ? > >> > >> No. The manifest should be provided with Guile. > > > > Of course, singe Guile is mainly a library, any application that is > > linked against libguile also needs such a manifest. > > Does the uname MinGW patch introduce a regression in any case that we > care about? No. On the contrary: where before Guile would only report correct OS version up to and including XP, it now reports correct values also for Vista, Windows 7, and Windows 8. Versions beyond that require the manifest, and otherwise will be reported as 8.0. Also, the modern CPU types are now supported, including 64-bit ones, which were not reported at all before. > Is it a user-visible change? If so an update to NEWS might be > needed. >From my POV, this is a bugfix: APIs that were reporting inaccurate values now report more accurate ones.
Re: c-api.test fails on MS-Windows due to non-portable quoting
> From: Andy Wingo
> Cc: guile-devel@gnu.org
> Date: Sat, 23 Jul 2016 23:11:08 +0200
>
> > It fails like this:
> >
> > Running c-api.test
> > 'CUR' is not recognized as an internal or external command,
> > operable program or batch file.
> > egrep: Unmatched ( or \('CUR' is not recognized as an internal or
> > external command, operable program or batch file.
> >
> > This is because it quotes shell commands /bin/sh '..' style:
>
> Of course, because that's how `system' is specified.
On Posix hosts, yes. But the ANSI C standard only says that the
argument will be passed to the host environment's command processor.
> > --- test-suite/tests/c-api.test~0 2016-01-02 13:32:40.0 +0200
> > +++ test-suite/tests/c-api.test 2016-07-23 14:12:57.257375000 +0300
> > @@ -22,7 +22,7 @@
> > (define srcdir (cdr (assq 'srcdir %guile-build-info)))
> >
> > (define (egrep string filename)
> > - (zero? (system (string-append "egrep '" string "' " filename
> > + (zero? (system (string-append "egrep \"" string "\" " filename
> > " >" %null-device
> >
> > (define (seek-offset-test dirname)
> >
> > OK to push such a change?
>
> I think instead to get this to work on MinGW we should switch to use
> system* instead of praying that we get quoting right ;) Something like:
>
> (zero? (system* "egrep" "-q" string filename))
For this to work, the Windows implementation of system* will need to
be augmented to quote characters special for the shell, because
(unlike execvp on Posix hosts) the arguments of spawnvp are eventually
concatenated into a single string that gets passed to the system API
which invokes programs. If this is the way you are willing to solve
this, I will submit a patch to that effect.
Re: Guile test-ffi uses an unportable assumption
> From: Andy Wingo > Cc: guile-devel@gnu.org > Date: Sat, 23 Jul 2016 23:17:58 +0200 > > > It assumes that libltdl can only produce a handle for a symbol in the > > the program itself, as opposed to those loaded from shared libraries. > > It tries 'strerror'. This cannot work on MS-Windows, unless the > > program was linked with -export-dynamic, which is not true for > > the test program. > > Interesting. The test program is a Scheme script which looks up > strerror in the dlopen(NULL). Libguile is compiled with -export-dynamic > but the guile binary is not. Which part needs to be compiled with > -export-dynamic, do you think? The guile binary. Here's what the libltdl documentation has to say about this: -- Function: lt_dlhandle lt_dlopen (const char *FILENAME) [...] If FILENAME is `NULL' and the program was linked with `-export-dynamic' or `-dlopen self', `lt_dlopen' will return a handle for the program itself, which can be used to access its symbols. [...] If you use `lt_dlopen (NULL)' to get a HANDLE for the running binary, that handle will always be marked as resident, and consequently cannot be successfully `lt_dlclose'd. However, in the case in point even compiling guile with -export-dynamic won't help, because the function being used by the test, strerror, comes from the C library, and is not statically linked into the guile binary. So I suggest to modify the test to use a function that is part of the guile binary's own sources.
Re: Avoid warning about alloca in read.c
On Sun 24 Jul 2016 04:39, Eli Zaretskii writes: >> From: Andy Wingo >> Cc: guile-devel@gnu.org >> Date: Sat, 23 Jul 2016 22:57:02 +0200 >> >> > +#ifdef __MINGW32__ >> > +#include >> > +#endif >> > + >> >> OK to commit but please remove the ifdef -- just include in all >> cases. > > Is that header available on all supported platforms? Yes, we use a Gnulib module to ensure that some workable version is present. Andy
Re: Avoid warnings in threads.c when building without threads
On Sun 24 Jul 2016 04:38, Eli Zaretskii writes:
>> > --- libguile/null-threads.h~0 2016-01-02 13:32:40.0 +0200
>> > +++ libguile/null-threads.h2016-07-15 17:47:37.101375000 +0300
>> > @@ -43,7 +43,7 @@
>> > #define scm_i_pthread_create(t,a,f,d) (*(t)=0, (void)(f), ENOSYS)
>> > #define scm_i_pthread_detach(t) do { } while (0)
>> > #define scm_i_pthread_exit(v) exit (EXIT_SUCCESS)
>> > -#define scm_i_pthread_cancel(t) 0
>> > +#define scm_i_pthread_cancel(t) (void)0
>> > #define scm_i_pthread_cleanup_push(t,v) 0
>> > #define scm_i_pthread_cleanup_pop(e)0
>> > #define scm_i_sched_yield() 0
>>
>> I think not, sorry :/ pthread_cancel returns an int, so
>> scm_i_pthread_cancel should always return an int.
>
> Then code which ignores the value should cast to void.
That way leads to madness :) There are many function call sites in Guile
that ignore the function return values. GCC does not warn about them,
and rightly so. The problem here is that the "null" definition of
scm_i_pthread_cancel is a bare expression and for some reason GCC is
treating this case differently. The solution AFAIU is to make the null
definition of scm_i_pthread_cancel into a function so that it is more
like the proper definition. That way not only will GCC not warn, but it
will also do type checking for us.
Applied a fix along these lines; a --without-threads configuration
builds without warnings and passes all tests for me now.
Andy
