Honor `$TMPDIR' in `guile-snarf'

Honor `$TMPDIR' in `guile-snarf'

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]

Hi,

FYI, I committed this tiny patch that makes `guile-snarf' honor `$TMPDIR' [0].

Thanks,
Ludovic.

[0] http://www.opengroup.org/onlinepubs/009695399/xrat/xbd_chap10.html


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: The patch --]
[-- Type: text/x-patch, Size: 518 bytes --]

--- guile-1.8.3/libguile/guile-snarf.in	2008-02-12 10:32:48.000000000 +0100
+++ guile-1.8.3/libguile/guile-snarf.in	2008-02-12 11:01:53.000000000 +0100
@@ -69,9 +69,12 @@ fi
 
 # set vars and handler -- handle CPP override
 cpp_ok_p=false
-tempdir="/tmp/snarf.$$"
+
+if [ x"$TMPDIR" = x ]; then TMPDIR="/tmp" ; else : ; fi
+tempdir="$TMPDIR/guile-snarf.$$"
 (umask 077 && mkdir $tempdir) || exit 1
 temp="$tempdir/tmp"
+
 if [ x"$CPP" = x ] ; then cpp="@CPP@" ; else cpp="$CPP" ; fi
 
 trap "rm -rf $tempdir" 0 1 2 15

Re: Honor `$TMPDIR' in `guile-snarf'

[Neil Jerram]

ludo@gnu.org (Ludovic Courtès) writes:

> -tempdir="/tmp/snarf.$$"
> +
> +if [ x"$TMPDIR" = x ]; then TMPDIR="/tmp" ; else : ; fi
> +tempdir="$TMPDIR/guile-snarf.$$"

I believe that's equivalent to

tempdir="${TMPDIR:-/tmp}/guile-snarf.$$"

Although install-sh says:

# Don't use :- since 4.3BSD and earlier shells don't like it.

So your version is safer.

Regards,
     Neil

Re: Honor `$TMPDIR' in `guile-snarf'

[Bruce Korb]

test X"$TMPDIR" = X && TMPDIR=/tmp

> > +if [ x"$TMPDIR" = x ]; then TMPDIR="/tmp" ; else : ; fi

much less clutter.

# Don't use :- since 4.3BSD and earlier shells don't like it.

Amazing.  Here I thought Solaris' /bin/sh was about as
decrepit as one could get nowadays....

Re: Honor `$TMPDIR' in `guile-snarf'

[Ludovic Courtès]

Neil Jerram <neil@ossau.uklinux.net> writes:

> I believe that's equivalent to
>
> tempdir="${TMPDIR:-/tmp}/guile-snarf.$$"
>
> Although install-sh says:
>
> # Don't use :- since 4.3BSD and earlier shells don't like it.
>
> So your version is safer.

Indeed, and the rest of the file avoids `:-'.

Thanks,
Ludo'.

Re: Honor `$TMPDIR' in `guile-snarf'

[Aleksej Saushev]

Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> -tempdir="/tmp/snarf.$$"
>> +
>> +if [ x"$TMPDIR" = x ]; then TMPDIR="/tmp" ; else : ; fi
>> +tempdir="$TMPDIR/guile-snarf.$$"
>
> I believe that's equivalent to
>
> tempdir="${TMPDIR:-/tmp}/guile-snarf.$$"
>
> Although install-sh says:
>
> # Don't use :- since 4.3BSD and earlier shells don't like it.
>
> So your version is safer.

4.3BSD is dead for a long time, what's the use to support it in the new software?

As for Solaris, its "/bin/sh" isn't supported too by community,
any sane developer uses "/usr/xpg4/bin/sh" at least.


-- 
CKOPO BECHA...
   CKOPO CE3OH...

Re: Honor `$TMPDIR' in `guile-snarf'

[Thien-Thi Nguyen]

() Aleksej Saushev <asau@inbox.ru>
() Wed, 13 Feb 2008 23:25:17 +0300

   > # Don't use :- since 4.3BSD and earlier shells don't like it.

   4.3BSD is dead for a long time, what's the use to support
   it in the new software?

In case it becomes .... UNDEAD .... !!!!  :-)

IMHO ${TMPDIR-/tmp} is the best solution
because it doesn't introduce another var.

thi

Re: Honor `$TMPDIR' in `guile-snarf'

[Aleksej Saushev]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> () Aleksej Saushev <asau@inbox.ru>
> () Wed, 13 Feb 2008 23:25:17 +0300
>
>    > # Don't use :- since 4.3BSD and earlier shells don't like it.
>
>    4.3BSD is dead for a long time, what's the use to support
>    it in the new software?
>
> In case it becomes .... UNDEAD .... !!!!  :-)
>
> IMHO ${TMPDIR-/tmp} is the best solution
> because it doesn't introduce another var.

In my opinion it should be ${TMPDIR:-/tmp},
this is the way it is done in all modern enough systems,
including all living BSD branches.

Also I want to point, that pkgsrc provides patches for Guile
and related packages, see here, for instance:

http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/lang/guile/patches/

It targets mainly NetBSD, DragonFlyBSD (two primary platforms),
and Solaris, though FreeBSD, MacOS X, HP-UX, IRIX, and AIX (and
even GNU/Linux) are taken into account too.  It would be nice, 
if patches found their way into the source itself.


-- 
CKOPO BECHA...
   CKOPO CE3OH...

Pkgsrc patches

[Ludovic Courtès]

Hi,

Aleksej Saushev <asau@inbox.ru> writes:

> Also I want to point, that pkgsrc provides patches for Guile
> and related packages, see here, for instance:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/lang/guile/patches/

It would be great if Greg or you could report a bug for these patches,
or point us to existing bug reports, or comment them somehow, so that we
can integrate them while being aware of what they solve and how.

Thanks,
Ludovic.

Re: Pkgsrc patches

[Greg Troxel]

ludo@gnu.org (Ludovic Courtès) writes:

> Hi,
>
> Aleksej Saushev <asau@inbox.ru> writes:
>
>> Also I want to point, that pkgsrc provides patches for Guile
>> and related packages, see here, for instance:
>>
>> http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/lang/guile/patches/
>
> It would be great if Greg or you could report a bug for these patches,
> or point us to existing bug reports, or comment them somehow, so that we
> can integrate them while being aware of what they solve and how.

I'll try to go through all the patches and send them in to this list,
one per message (ok?).  Basically there are 3 build fixes, one extension
of processor types for netbsd/amd64, and a fix for systems that use
-rpath for guile to find shlibs when it's installed not in /usr.

patches from pkgsrc - patch-aa

[Greg Troxel]

These patches (this and more to follow) are all against guile 1.8.3.


$NetBSD: patch-aa,v 1.11 2007/07/20 00:09:22 gdt Exp $

Use a pointer, instead of an array, to avoid an error on Darwin.
Patch from DarwinPorts.

--- libguile/guile.c.orig	2007-05-09 16:22:03.000000000 -0400
+++ libguile/guile.c
@@ -68,7 +68,7 @@ main (int argc, char **argv)
 {
 #if !defined (__MINGW32__)
   /* libtool automagically inserts this variable into your executable... */
-  extern const lt_dlsymlist lt_preloaded_symbols[];
+  extern const lt_dlsymlist *lt_preloaded_symbols;
   lt_dlpreload_default (lt_preloaded_symbols);
 #endif
   scm_boot_guile (argc, argv, inner_main, 0);

Re: Pkgsrc patches: patch-ae

[Greg Troxel]

This just adds defines for more platforms, plus fixes a comment typo.

$NetBSD: patch-ae,v 1.13 2007/07/20 00:09:22 gdt Exp $

support for NetBSD/alpha, NetBSD/sparc64, and NetBSD/x86_64

--- libguile/gc_os_dep.c.orig	2007-05-09 16:22:03.000000000 -0400
+++ libguile/gc_os_dep.c
@@ -110,6 +110,11 @@ typedef int GC_bool;
 #    define OPENBSD
 #    define mach_type_known
 # endif
+# if defined(__NetBSD__) && defined(__alpha__)
+#    define ALPHA
+#    define NETBSD
+#    define mach_type_known
+# endif
 # if defined(__NetBSD__) && defined(__powerpc__)
 #    define POWERPC
 #    define NETBSD
@@ -127,7 +132,7 @@ typedef int GC_bool;
 #    define NETBSD
 #    define mach_type_known
 # endif
-# if defined(__NetBSD__) && defined(__sparc__)
+# if defined(__NetBSD__) && (defined(__sparc__) || defined(__sparc_v9__))
 #    define SPARC
 #    define NETBSD
 #    define mach_type_known
@@ -246,7 +251,7 @@ typedef int GC_bool;
 # endif
 # if defined(__alpha) || defined(__alpha__)
 #   define ALPHA
-#   if !defined(LINUX)
+#   if !defined(LINUX) && !defined(NETBSD)
 #     define OSF1	/* a.k.a Digital Unix */
 #   endif
 #   define mach_type_known
@@ -299,6 +304,11 @@ typedef int GC_bool;
 #   define NETBSD
 #   define mach_type_known
 # endif
+# if defined(__NetBSD__) && defined(__x86_64__)
+#   define X86_64
+#   define NETBSD
+#   define mach_type_known
+# endif
 # if defined(bsdi) && defined(i386)
 #    define I386
 #    define BSDI
@@ -439,12 +449,12 @@ scm_get_stack_base ()
 /*
  * For each architecture and OS, the following need to be defined:
  *
- * CPP_WORD_SZ is a simple integer constant representing the word size.
+ * CPP_WORDSZ is a simple integer constant representing the word size.
  * in bits.  We assume byte addressibility, where a byte has 8 bits.
- * We also assume CPP_WORD_SZ is either 32 or 64.
+ * We also assume CPP_WORDSZ is either 32 or 64.
  * (We care about the length of pointers, not hardware
  * bus widths.  Thus a 64 bit processor with a C compiler that uses
- * 32 bit pointers should use CPP_WORD_SZ of 32, not 64. Default is 32.)
+ * 32 bit pointers should use CPP_WORDSZ of 32, not 64. Default is 32.)
  *
  * MACH_TYPE is a string representation of the machine type.
  * OS_TYPE is analogous for the OS.
@@ -989,6 +999,21 @@ scm_get_stack_base ()
 #    endif
 # endif
 
+# ifdef X86_64
+#   define MACH_TYPE "X86_64"
+#   define ALIGNMENT 8
+#   define ALIGN_DOUBLE
+#   define CPP_WORDSZ 64
+#   ifdef NETBSD
+#	define OS_TYPE "NETBSD"
+#   endif
+#   if defined(NETBSD)
+#	define HEURISTIC2
+	extern char etext;
+#	define DATASTART ((ptr_t)(&etext))
+#   endif
+#   endif
+
 # ifdef NS32K
 #   define MACH_TYPE "NS32K"
 #   define ALIGNMENT 4

Re: Pkgsrc patches: patch-ag

[Greg Troxel]

Fix for solaris, but I thinh preserves behavior on C99 machines.


$NetBSD: patch-ag,v 1.5 2007/07/20 00:09:22 gdt Exp $

solaris-2.9 does not have stdint.h but does have inttypes.h

--- libguile/c-tokenize.c.orig	2007-06-13 18:00:56.000000000 -0400
+++ libguile/c-tokenize.c
@@ -39,7 +39,12 @@
 #define __STDC_LIMIT_MACROS 1
 #endif
 
+#ifdef HAVE_STDINT_H
+#include <stdint.h> /* May break IA64 test-noansi-r */
+#else
 #include <inttypes.h>
+#endif
+
 typedef int8_t flex_int8_t;
 typedef uint8_t flex_uint8_t;
 typedef int16_t flex_int16_t;

Re: Pkgsrc patches: patch-ah

[Greg Troxel]

This one you probably don't want to apply, but I'm sending it to point
out a much larger problem.  Programs that muck with library paths really
need to figure out what the rules are on the local system and deal
appropriately.  On NetBSD one needs -Wl,-R (or just -R really) to point
to the libdir at runtime when linking, not just -L to find the library.
This is not an issue on linux because of the common use of /usr, I think.

There are more serious issues with dynlinking of .so modules, and on
Darwin there are some PRs open about guile and guile-using programs that
I don't yet fully understand.

I think the fix involves a configure test to decide the -R rules, and
then conditional code to behave differently.


$NetBSD: patch-ah,v 1.3 2007/07/20 00:09:23 gdt Exp $

--- guile-config/guile-config.in.orig	2007-05-09 16:22:03.000000000 -0400
+++ guile-config/guile-config.in
@@ -155,7 +155,7 @@
                (if (or (string=? libdir "/usr/lib")
                        (string=? libdir "/usr/lib/"))
 		   ""
-		   (string-append "-L" (get-build-info 'libdir)))
+		   (string-append "-Wl,-R" (get-build-info 'libdir) " -L" (get-build-info 'libdir)))
 	       (string-join other-flags)
 
 	       )))

Re: Pkgsrc patches: patch-aj

[Greg Troxel]

I am not 100% clear on this one, but I think the basic issue is that the
code was defining errno always, and that could lose depending on whether
it matched the system definition.  Perhaps we need HAVE_ERRNO_H, but in
general I'm in favor of checking for C99/etc. and trying that, and only
if not found trying to work around.




$NetBSD: patch-aj,v 1.3 2007/07/20 00:09:23 gdt Exp $

--- libguile/_scm.h.orig	2007-05-09 16:22:03.000000000 -0400
+++ libguile/_scm.h
@@ -119,7 +119,9 @@
 # define SCM_SYSCALL(line) line;
 #endif /* ndef SCM_SYSCALL */
 
-#if !defined (MSDOS) && !defined (__MINGW32__)
+#include <errno.h>
+
+#if !defined (MSDOS) && !defined (__MINGW32__) && !defined(errno)
 # ifdef ARM_ULIB
     extern volatile int errno;
 # else

Re: Pkgsrc patches

[Greg Troxel]

OK, I've sent all 5 pkgsrc patches, of which I think 4 should probably
be applied.  Please feel free to ask about any of them, although I'll
point out that I'm not the originator of some of these.

In addition to the patches, pkgsrc bashes the slib support, and avoids
the rpath addition on Darwin.  Below, note:

  post-patch: unconditional replacement of ice-9 slib file

  post-patch: target conditional on darwin
  



# $NetBSD: Makefile,v 1.80 2008/01/04 18:17:16 joerg Exp $

DISTNAME=		guile-1.8.3
CATEGORIES=		lang
MASTER_SITES=		${MASTER_SITE_GNU:=guile/}

MAINTAINER=		gdt@NetBSD.org
HOMEPAGE=		http://www.gnu.org/software/guile/guile.html
COMMENT=		GNU's Ubiquitous Intelligent Language for Extension

CONFLICTS=		guile14<1.4.1nb1 guile-oops-[0-9]*

PKG_DESTDIR_SUPPORT=	user-destdir

GNU_CONFIGURE=		YES
USE_TOOLS+=		gmake makeinfo
USE_LIBTOOL=		YES
USE_GNU_READLINE=	YES
PTHREAD_AUTO_VARS=	YES
INFO_FILES=		YES

# XXX Make this conditional on the problematic systems.
# couldn't figure out how to work around an error on solaris-2.9/sparc with gcc when
# compiling ${WRKSRC}/libguile/filesys.c.  The warning is that readdir64_r was missing
# a prototype.
CONFIGURE_ARGS+=	--disable-error-on-warning

TEST_DIRS=		.
TEST_TARGET=		check

OWN_DIRS=		${PREFIX}/share/guile/site

.include "../../mk/bsd.prefs.mk"

# scmconfig.h will be rebuilt first thing.  This hack is here because
# gen-scmconfig.c includes string.h (from /usr/include) which in turn
# tries to include /usr/include/strings.h but instead the compiler picks
# up ./strings.h which tries to include scmconfig.h which doesn't exist
# yet.  While not a clean fix, this seems to work around the problem.
#
#
# for info on the slib part, see
# http://lists.gnu.org/archive/html/guile-user/2006-11/msg00035.html
post-patch:
	${TOUCH} ${WRKSRC}/libguile/scmconfig.h
	${ECHO} "(define-module (ice-9 slib))" > ${WRKSRC}/ice-9/slib.scm
	${ECHO} "(load-from-path \"slib/guile.init\")" >> ${WRKSRC}/ice-9/slib.scm

.if ${OPSYS} == "DragonFly"
CONFIGURE_ARGS+=	ac_cv_type_complex_double=no
.endif

.if ${OPSYS} == "Darwin"
CPPFLAGS+=		-Dmacosx

# Darwin does not use -rpath, so back out the patch that adds it.  It
# is not clear that guile works properly if directories with modules
# are not in LTDL_LIBRARY_PATH or LD_LIBRARY_PATH.  See the following
# PRs:
#   http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=33665
#   http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=36975
post-patch:
	${MV} ${WRKSRC}/guile-config/guile-config.in.orig \
	    ${WRKSRC}/guile-config/guile-config.in
.endif

.include "../../devel/gmp/buildlink3.mk"
.include "../../devel/libltdl/buildlink3.mk"
.include "../../devel/ncurses/buildlink3.mk"
.include "../../devel/readline/buildlink3.mk"
.include "../../mk/pthread.buildlink3.mk"
.include "../../mk/bsd.pkg.mk"

Re: patches from pkgsrc - patch-aa

[Ludovic Courtès]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> $NetBSD: patch-aa,v 1.11 2007/07/20 00:09:22 gdt Exp $
>
> Use a pointer, instead of an array, to avoid an error on Darwin.
> Patch from DarwinPorts.
>
> --- libguile/guile.c.orig	2007-05-09 16:22:03.000000000 -0400
> +++ libguile/guile.c
> @@ -68,7 +68,7 @@ main (int argc, char **argv)
>  {
>  #if !defined (__MINGW32__)
>    /* libtool automagically inserts this variable into your executable... */
> -  extern const lt_dlsymlist lt_preloaded_symbols[];
> +  extern const lt_dlsymlist *lt_preloaded_symbols;
>    lt_dlpreload_default (lt_preloaded_symbols);
>  #endif
>    scm_boot_guile (argc, argv, inner_main, 0);

I was inclined to install this one since it's harmless.  I did a bit of
research to get more information and found this:

  http://permalink.gmane.org/gmane.os.netbsd.devel.pkgsrc.bulk/2337
  http://lists.macosforge.org/pipermail/macports-changes/2007-March/003715.html

The patch supposedly fixes this error:

  guile.c:91: error: array type has incomplete element type

However, if <ltdl.h> is included, then `lt_dlsymlist' is necessarily
typedef'd, so there's not reason such an error would occur.  I would
expect an ltdl version mismatch perhaps, a broken compiler, or something
like that.

Thus, I'd say "no", unless MacPorts/Pkgsrc folks can provide some more
explanations.  

(BTW, we should use `LTDL_SET_PRELOADED_SYMBOLS ()', which does exactly
the same as what we're doing.)

Thanks,
Ludovic.

Re: Pkgsrc patches: patch-ae

[Ludovic Courtès]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> This just adds defines for more platforms, plus fixes a comment typo.
>
> $NetBSD: patch-ae,v 1.13 2007/07/20 00:09:22 gdt Exp $
>
> support for NetBSD/alpha, NetBSD/sparc64, and NetBSD/x86_64
>
I'm unclear about some of the changes below.

> -# if defined(__NetBSD__) && defined(__sparc__)
> +# if defined(__NetBSD__) && (defined(__sparc__) || defined(__sparc_v9__))

Two questions:

  1. Isn't `__sparc__' defined on V9?  On GNU/Linux, it seems that it is
     defined on both V9 and V8, so no special treatment is required.

  2. Unless I'm mistaken, this is equivalent to

       #if (defined(__NetBSD__) && (defined(__sparc__))) || (defined(__sparc_v9__))

     while this should probably be:

       #if defined(__NetBSD__) && (defined(__sparc__) || (defined(__sparc_v9__)))

> +# if defined(__NetBSD__) && defined(__x86_64__)
> +#   define X86_64
> +#   define NETBSD
> +#   define mach_type_known
> +# endif

[...]

> +# ifdef X86_64
> +#   define MACH_TYPE "X86_64"
> +#   define ALIGNMENT 8
> +#   define ALIGN_DOUBLE
> +#   define CPP_WORDSZ 64
> +#   ifdef NETBSD
> +#	define OS_TYPE "NETBSD"
> +#   endif
> +#   if defined(NETBSD)
> +#	define HEURISTIC2
> +	extern char etext;
> +#	define DATASTART ((ptr_t)(&etext))
> +#   endif
> +#   endif

Strangely enough, we don't have `LINUX && __x86_64__' either.
Interestingly, of these constants, it seems that Guile uses only
`HEURISTIC2'.  For instance, Guile's GC gets the alignment constraint
through the definition of `SCM_STACKITEM'.

Hence my question: didn't the NetBSD/i386 support suffice on x86-64?
What exact problem did it solve?

The other bits are indeed applicable.

Thanks,
Ludovic.

Re: Pkgsrc patches: patch-ag

[Ludovic Courtès]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> Fix for solaris, but I thinh preserves behavior on C99 machines.
>
>
> $NetBSD: patch-ag,v 1.5 2007/07/20 00:09:22 gdt Exp $
>
> solaris-2.9 does not have stdint.h but does have inttypes.h
>
> --- libguile/c-tokenize.c.orig	2007-06-13 18:00:56.000000000 -0400
> +++ libguile/c-tokenize.c
> @@ -39,7 +39,12 @@
>  #define __STDC_LIMIT_MACROS 1
>  #endif
>  
> +#ifdef HAVE_STDINT_H
> +#include <stdint.h> /* May break IA64 test-noansi-r */
> +#else
>  #include <inttypes.h>
> +#endif

First, the comment and the patch are contradictory.  A bit of googling
seems to confirm the comment, in which case the patch isn't needed.

Second, C99 requires both (Sections 7.18 and 7.8).

Third, the `#include' is within a `#if __STDC_VERSION__ >= 199901L', so
only C99 systems get to include it.

Fourth, this is a Flex-generated file, so it'd be nice to avoid patching
it.

Just to say that I'm dubious about this patch.  :-)

Thanks,
Ludovic.

Re: Pkgsrc patches: patch-ag

[Greg Troxel]

ludo@gnu.org (Ludovic Courtès) writes:

> Greg Troxel <gdt@ir.bbn.com> writes:
>
>> Fix for solaris, but I thinh preserves behavior on C99 machines.
>>
>> $NetBSD: patch-ag,v 1.5 2007/07/20 00:09:22 gdt Exp $
>>
>> solaris-2.9 does not have stdint.h but does have inttypes.h
>>
>> --- libguile/c-tokenize.c.orig	2007-06-13 18:00:56.000000000 -0400
>> +++ libguile/c-tokenize.c
>> @@ -39,7 +39,12 @@
>>  #define __STDC_LIMIT_MACROS 1
>>  #endif
>>  
>> +#ifdef HAVE_STDINT_H
>> +#include <stdint.h> /* May break IA64 test-noansi-r */
>> +#else
>>  #include <inttypes.h>
>> +#endif
>
> First, the comment and the patch are contradictory.  A bit of googling
> seems to confirm the comment, in which case the patch isn't needed.
>
> Second, C99 requires both (Sections 7.18 and 7.8).
>
> Third, the `#include' is within a `#if __STDC_VERSION__ >= 199901L', so
> only C99 systems get to include it.
>
> Fourth, this is a Flex-generated file, so it'd be nice to avoid patching
> it.
>
> Just to say that I'm dubious about this patch.  :-)

I think the issue is that some versions of solaris do not have stdint.h,
but do have inttypes.h.  But, the patch may be wrong in dealing with it.

Does guile have a position on supporting systems that don't conform to
C99?  I'm wondering if I should try to figure this out and get a fix
that is reasonable (I agree this one is gross and likely wrong), or just
punt.

Re: Pkgsrc patches: patch-ag

[Ludovic Courtès]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Greg Troxel <gdt@ir.bbn.com> writes:
>>
>>> Fix for solaris, but I thinh preserves behavior on C99 machines.
>>>
>>> $NetBSD: patch-ag,v 1.5 2007/07/20 00:09:22 gdt Exp $
>>>
>>> solaris-2.9 does not have stdint.h but does have inttypes.h
>>>
>>> --- libguile/c-tokenize.c.orig	2007-06-13 18:00:56.000000000 -0400
>>> +++ libguile/c-tokenize.c
>>> @@ -39,7 +39,12 @@
>>>  #define __STDC_LIMIT_MACROS 1
>>>  #endif
>>>  
>>> +#ifdef HAVE_STDINT_H
>>> +#include <stdint.h> /* May break IA64 test-noansi-r */
>>> +#else
>>>  #include <inttypes.h>
>>> +#endif
>>
>> First, the comment and the patch are contradictory.  A bit of googling
>> seems to confirm the comment, in which case the patch isn't needed.
>>
>> Second, C99 requires both (Sections 7.18 and 7.8).
>>
>> Third, the `#include' is within a `#if __STDC_VERSION__ >= 199901L', so
>> only C99 systems get to include it.
>>
>> Fourth, this is a Flex-generated file, so it'd be nice to avoid patching
>> it.
>>
>> Just to say that I'm dubious about this patch.  :-)
>
> I think the issue is that some versions of solaris do not have stdint.h,
> but do have inttypes.h.  But, the patch may be wrong in dealing with it.

Without the patch, `c-tokenize.c' includes *only* <inttypes.h> (see my
first comment above.)

> Does guile have a position on supporting systems that don't conform to
> C99?  I'm wondering if I should try to figure this out and get a fix
> that is reasonable (I agree this one is gross and likely wrong), or just
> punt.

Guile is written using C89 code.  It does use C99 headers when
available, but does not usually rely on them I think.

I believe the "position" is roughly to be portable to any platform with
Guile users.  :-)

Thanks,
Ludovic.

Re: Pkgsrc patches: patch-ag

[Greg Troxel]

ludo@gnu.org (Ludovic Courtès) writes:

> Without the patch, `c-tokenize.c' includes *only* <inttypes.h> (see my
> first comment above.)

Right you are - it may be that this is just confusion from long ago.

>> Does guile have a position on supporting systems that don't conform to
>> C99?  I'm wondering if I should try to figure this out and get a fix
>> that is reasonable (I agree this one is gross and likely wrong), or just
>> punt.
>
> Guile is written using C89 code.  It does use C99 headers when
> available, but does not usually rely on them I think.
>
> I believe the "position" is roughly to be portable to any platform with
> Guile users.  :-)

Yes, that sounds great.  I have just been running into "claims C99 but
sort of broken" fairly often lately, and wanted guidance - it's
difficult trying to deal with pkgsrc patches for platforms I don't have.

Re: Pkgsrc patches: patch-ae

[Greg Troxel]

ludo@gnu.org (Ludovic Courtès) writes:

> Greg Troxel <gdt@ir.bbn.com> writes:
>
>> This just adds defines for more platforms, plus fixes a comment typo.
>>
>> $NetBSD: patch-ae,v 1.13 2007/07/20 00:09:22 gdt Exp $
>>
>> support for NetBSD/alpha, NetBSD/sparc64, and NetBSD/x86_64
>>
> I'm unclear about some of the changes below.
>
>> -# if defined(__NetBSD__) && defined(__sparc__)
>> +# if defined(__NetBSD__) && (defined(__sparc__) || defined(__sparc_v9__))
>
> Two questions:
>
>   1. Isn't `__sparc__' defined on V9?  On GNU/Linux, it seems that it is
>      defined on both V9 and V8, so no special treatment is required.

I tested on both NetBSD 4-stable on an ultra2, and on 3-stable on an
ultra5.  Both had __sparc__ defined.  On the ultra2, I built guile from
pkgsrc without this patch, and then did 'make check' and it passed.  So
I'd say the __sparc_v9__ can be safely omitted.

But, I'm nervous that 64-bit sparc and 32-bit sparc aren't the same
thing, and don't necessarily have the same alignment rules and sizes.

>   2. Unless I'm mistaken, this is equivalent to
>
>        #if (defined(__NetBSD__) && (defined(__sparc__))) || (defined(__sparc_v9__))
>
>      while this should probably be:
>
>        #if defined(__NetBSD__) && (defined(__sparc__) || (defined(__sparc_v9__)))

Sorry, I don't follow this at all - it looks fine to me.  But it's moot,
as the change can be skipped.

>> +# if defined(__NetBSD__) && defined(__x86_64__)
>> +#   define X86_64
>> +#   define NETBSD
>> +#   define mach_type_known
>> +# endif
>
> [...]
>
>> +# ifdef X86_64
>> +#   define MACH_TYPE "X86_64"
>> +#   define ALIGNMENT 8
>> +#   define ALIGN_DOUBLE
>> +#   define CPP_WORDSZ 64
>> +#   ifdef NETBSD
>> +#	define OS_TYPE "NETBSD"
>> +#   endif
>> +#   if defined(NETBSD)
>> +#	define HEURISTIC2
>> +	extern char etext;
>> +#	define DATASTART ((ptr_t)(&etext))
>> +#   endif
>> +#   endif
>
> Strangely enough, we don't have `LINUX && __x86_64__' either.
> Interestingly, of these constants, it seems that Guile uses only
> `HEURISTIC2'.  For instance, Guile's GC gets the alignment constraint
> through the definition of `SCM_STACKITEM'.

There's a comment in gc_os_dep.c that says ALIGNMENT needs to be
defined.  The other arches seem to define it. (Not arguing that it's
used.)

I suppose the challenge is to figure out how/why guile works on Linux
amd64 systems.

> Hence my question: didn't the NetBSD/i386 support suffice on x86-64?
> What exact problem did it solve?

I don't know - I can inquire.  But amd64 (as we call it) is a different
processor architecture, with a different pointer size, so I don't see
how the i386 support would apply.  I would not expect the i386
preprocessor symbols to be defined, but I don't have a netbsd/amd64
system (I really should set one up).

Not sure this is relevant, but: On NetBSD, an amd64 system is all 64-bit
native code.  If there's i386 compat code, it's in /emul/netbsd32.  The
"lib64" convention is not used at all.

> The other bits are indeed applicable.

Thanks - I'll adjust the patch when there's a new release, dropping
whatever you merged and also the sparc v9.

Re: Pkgsrc patches: patch-aj

[Neil Jerram]

Hi Greg,

Greg Troxel <gdt@ir.bbn.com> writes:

> I am not 100% clear on this one, but I think the basic issue is that the
> code was defining errno always, and that could lose depending on whether
> it matched the system definition.

Makes sense.

> Perhaps we need HAVE_ERRNO_H, but in
> general I'm in favor of checking for C99/etc. and trying that, and only
> if not found trying to work around.

_scm.h already includes errno.h unconditionally.  It appears that it's
one of a very few headers that we always assume.  And no one has
reported this failing, so the assumption must be fine.

> $NetBSD: patch-aj,v 1.3 2007/07/20 00:09:23 gdt Exp $
>
> --- libguile/_scm.h.orig	2007-05-09 16:22:03.000000000 -0400
> +++ libguile/_scm.h
> @@ -119,7 +119,9 @@
>  # define SCM_SYSCALL(line) line;
>  #endif /* ndef SCM_SYSCALL */
>  
> -#if !defined (MSDOS) && !defined (__MINGW32__)
> +#include <errno.h>

The include isn't needed here, because errno.h was already included
earlier on in the same file.

> +
> +#if !defined (MSDOS) && !defined (__MINGW32__) && !defined(errno)
>  # ifdef ARM_ULIB
>      extern volatile int errno;
>  # else

I think we can just delete these extra errno declarations.  They've
been there forever - or at least since rev 1.1 of _scm.h in CVS (1996)
- and I suspect they've just been left in from a time (pre-CVS) before
the #include <errno.h> was added.

Can you confirm that ripping this stanza out works for you, then we'll
go ahead with that for 1.8.5.

Thanks,
        Neil

Re: Pkgsrc patches: patch-aj

[Greg Troxel]

  I think we can just delete these extra errno declarations.  They've
  been there forever - or at least since rev 1.1 of _scm.h in CVS (1996)
  - and I suspect they've just been left in from a time (pre-CVS) before
  the #include <errno.h> was added.

  Can you confirm that ripping this stanza out works for you, then we'll
  go ahead with that for 1.8.5.

I'm sure it works on netbsd, which is all i can test on easily.  As for
the reswt of the pkgsrc platforms, well my take is that you as upstream
should clean things up according to your best judgement and if we need
to re-add a patch we'll do it.  Given that you're including errno.h
always, I think it's just plain wrong to redeclare errno, so rip it out
and we'll pick up the pieces if needed.

Re: Pkgsrc patches: patch-aj

[Greg Troxel]

Neil Jerram <neil@ossau.uklinux.net> writes:

I have looked at this in more detail, to understand more precisely what
pkgsrc changed, in order to make sure I'm not suggesting that we throw
out a fix to an actual problem.

>> $NetBSD: patch-aj,v 1.3 2007/07/20 00:09:23 gdt Exp $
>>
>> --- libguile/_scm.h.orig	2007-05-09 16:22:03.000000000 -0400
>> +++ libguile/_scm.h
>> @@ -119,7 +119,9 @@
>>  # define SCM_SYSCALL(line) line;
>>  #endif /* ndef SCM_SYSCALL */
>>  
>> -#if !defined (MSDOS) && !defined (__MINGW32__)
>> +#include <errno.h>
>
> The include isn't needed here, because errno.h was already included
> earlier on in the same file.

Agreed, so I will remove the include from patch-aj (which dates from 1.6).

>> +
>> +#if !defined (MSDOS) && !defined (__MINGW32__) && !defined(errno)
>>  # ifdef ARM_ULIB
>>      extern volatile int errno;
>>  # else
>
> I think we can just delete these extra errno declarations.  They've
> been there forever - or at least since rev 1.1 of _scm.h in CVS (1996)
> - and I suspect they've just been left in from a time (pre-CVS) before
> the #include <errno.h> was added.
>
> Can you confirm that ripping this stanza out works for you, then we'll
> go ahead with that for 1.8.5.

Now it has finally become clear to me (sorry for being dense): pkgsrc
was only ifdef'ing out these redundant errno declarations in an attempt
to avoid some platform that didn't need them blowing up.

I did a build with the following patch instead:

$NetBSD$

--- libguile/_scm.h.orig	2008-02-06 08:09:18.000000000 -0500
+++ libguile/_scm.h
@@ -95,14 +95,6 @@
 # define SCM_SYSCALL(line) line;
 #endif /* ndef SCM_SYSCALL */
 
-#if !defined (MSDOS) && !defined (__MINGW32__)
-# ifdef ARM_ULIB
-    extern volatile int errno;
-# else
-    extern int errno;
-# endif /* def ARM_ULIB */
-#endif /* ndef MSDOS && ndef __MINGW32__*/
-
 \f
 
 #ifndef min

and then 1.8.4 builds and 'make check's ok on NetBSD-current i386.

I have put the above patch into pkgsrc, and I think it should be applied
in guile.

Re: Pkgsrc patches: patch-aj

[Neil Jerram]

Greg Troxel <gdt@ir.bbn.com> writes:

> Neil Jerram <neil@ossau.uklinux.net> writes:
>
> I have looked at this in more detail, to understand more precisely what
> pkgsrc changed, in order to make sure I'm not suggesting that we throw
> out a fix to an actual problem.

Thanks for your time on this!

>> The include isn't needed here, because errno.h was already included
>> earlier on in the same file.
>
> Agreed, so I will remove the include from patch-aj (which dates from 1.6).

Cool.

> I did a build with the following patch instead:
>
> $NetBSD$
>
> --- libguile/_scm.h.orig	2008-02-06 08:09:18.000000000 -0500
> +++ libguile/_scm.h
> @@ -95,14 +95,6 @@
>  # define SCM_SYSCALL(line) line;
>  #endif /* ndef SCM_SYSCALL */
>  
> -#if !defined (MSDOS) && !defined (__MINGW32__)
> -# ifdef ARM_ULIB
> -    extern volatile int errno;
> -# else
> -    extern int errno;
> -# endif /* def ARM_ULIB */
> -#endif /* ndef MSDOS && ndef __MINGW32__*/
> -
>  \f
>  
>  #ifndef min
>
> and then 1.8.4 builds and 'make check's ok on NetBSD-current i386.
>
> I have put the above patch into pkgsrc, and I think it should be applied
> in guile.

Yes, that works fine for me too on GNU/Linux, and I can't imagine any
system where it won't - so I've committed this change now to CVS.

       Neil