request review: branch "wingo"

request review: branch "wingo"

[Andy Wingo]

Hi,

On the "wingo" branch in the main repository, you will find the
following patches:

    The following changes since commit 0fe95f9c4ce063781e79a15bc123c57c33ef9755:
      Ludovic Courtès (1):
            Improve wording in `libguile/Makefile.am' regarding stack calibration.

    are available in the git repository at:

      git.sv.gnu.org:/srv/git/guile.git wingo

    Andy Wingo (7):
          allow building against uninstalled guile; move some things to meta/
          add getrlimit and setrlimit wrappers
          getrlimit-based stack limits
          rely on getrlimit to DTRT, don't make stack calibration file
          fix check for guile-tools running uninstalled
          fix "linking" of guile-config
          fix distcheck hopefully, by cleaning the vm-i-*.i files

     .gitignore                                         |    1 -
     Makefile.am                                        |    9 +-
     README                                             |   20 ++-
     am/guilec                                          |    4 +-
     am/pre-inst-guile                                  |    2 +-
     check-guile.in                                     |    5 +-
     configure.in                                       |   16 +-
     doc/ref/Makefile.am                                |    4 +-
     gc-benchmarks/run-benchmark.scm                    |    2 +-
     libguile/Makefile.am                               |   25 +---
     libguile/debug.c                                   |   36 ++++
     libguile/measure-hwm.scm                           |  136 ---------------
     libguile/posix.c                                   |  174 ++++++++++++++++++++
     libguile/posix.h                                   |    2 +
     {guile-config => meta}/ChangeLog-2008              |    0 
     {guile-config => meta}/Makefile.am                 |   25 +--
     .../gdb-uninstalled-guile.in                       |   10 +-
     meta/guile-1.8-uninstalled.pc.in                   |    8 +
     guile-1.8.pc.in => meta/guile-1.8.pc.in            |    0 
     {guile-config => meta}/guile-config.in             |    7 +-
     guile-tools.in => meta/guile-tools.in              |    2 +-
     pre-inst-guile.in => meta/guile.in                 |    6 +-
     {guile-config => meta}/guile.m4                    |    0 
     pre-inst-guile-env.in => meta/uninstalled-env.in   |   17 ++-
     test-suite/standalone/Makefile.am                  |    2 +-
     test-suite/standalone/README                       |    2 +-
     test-suite/standalone/test-fast-slot-ref.in        |    2 +-
     test-suite/standalone/test-use-srfi.in             |    6 +-
     testsuite/Makefile.am                              |    2 +-
     29 files changed, 299 insertions(+), 226 deletions(-)
     delete mode 100644 libguile/measure-hwm.scm
     rename {guile-config => meta}/ChangeLog-2008 (100%)
     rename {guile-config => meta}/Makefile.am (59%)
     rename gdb-pre-inst-guile.in => meta/gdb-uninstalled-guile.in (79%)
     create mode 100644 meta/guile-1.8-uninstalled.pc.in
     rename guile-1.8.pc.in => meta/guile-1.8.pc.in (100%)
     rename {guile-config => meta}/guile-config.in (98%)
     rename guile-tools.in => meta/guile-tools.in (98%)
     rename pre-inst-guile.in => meta/guile.in (93%)
     rename {guile-config => meta}/guile.m4 (100%)
     rename pre-inst-guile-env.in => meta/uninstalled-env.in (87%)

The patches fix distcheck again, but the interesting one is
ec900eacb71bbf66b85a5605f67f83b43f2c6ca8, which does this in debug.c:

    @@ -513,11 +518,42 @@ SCM_DEFINE (scm_debug_hang, "debug-hang", 0, 1, 0,
     #undef FUNC_NAME
     #endif
     
    +static void
    +init_stack_limit (void)
    +{
    +#ifdef HAVE_GETRLIMIT
    +  struct rlimit lim;
    +  if (getrlimit (RLIMIT_STACK, &lim) == 0)
    +      {
    +        int bytes = lim.rlim_cur, words;
    +
    +        /* set our internal stack limit to 1 MB or 80% of the rlimit, whichever
    +           is lower. */
    +        if (bytes == RLIM_INFINITY)
    +          bytes = lim.rlim_max;
    +
    +        if (bytes == RLIM_INFINITY)
    +          words = 1024 * 1024 / sizeof (scm_t_bits);
    +        else
    +          {
    +            bytes = bytes * 8 / 10;
    +            if (bytes > 1024 * 1024)
    +              bytes = 1024 * 1024;
    +            words = bytes / sizeof (scm_t_bits);
    +          }
    +
    +        SCM_STACK_LIMIT = words;
    +      }
    +  errno = 0;
    +#endif
    +}
    +
     \f
     
     void
     scm_init_debug ()
     {
    +  init_stack_limit ();
       scm_init_opts (scm_debug_options, scm_debug_opts);
    [...]

Comments? OK to merge to master?

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Andy Wingo]

Hey all,

On Fri 27 Mar 2009 16:29, Andy Wingo <wingo@pobox.com> writes:

> On the "wingo" branch in the main repository, you will find the
> following patches:

So, I really intended to wait for review, but it's irritating having
`master' broken, so I went ahead and merged this in.

I think the stack calibration stuff is correct, but perhaps more jarring
in this commit is a move from ./pre-inst-guile to ./meta/guile, and
./pre-inst-guile-env to ./meta/uninstalled-env. I describe the rationale
in 0b6d8fdc28ed8af56e93157179c305fef037e0a0. But then again, given that
Neil invested so much time into the stack calibration stuff, that might
be jarring too.

Please let me know if yall have a concern with this merge -- we can fix
it tomorrow or Monday.

Cheers,

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Please let me know if yall have a concern with this merge -- we can fix
> it tomorrow or Monday.

I'll aim to take a look tomorrow evening.

     Neil

Re: request review: branch "wingo"

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hey all,
>
> On Fri 27 Mar 2009 16:29, Andy Wingo <wingo@pobox.com> writes:
>
>> On the "wingo" branch in the main repository, you will find the
>> following patches:
>
> So, I really intended to wait for review, but it's irritating having
> `master' broken, so I went ahead and merged this in.

Here are my comments on the merged commits.

- allow building against uninstalled guile; move some things to meta/

Looks like a good idea.  Could be some gotchas lurking, but I don't
immediately see any.

- add getrlimit and setrlimit wrappers

Should scm_getrlimit code be surrounded by #ifdef HAVE_GETRLIMIT ?

What's the idea of the numerical args?  Should RLIMIT_XXX be defined
somewhere as Scheme-level integers?

Does it make sense to want to set the soft limit only?  I thought some
hard limits were only settable by root, so I would suspect yes.  If
yes, does the setrlimit implementation support this?

- getrlimit-based stack limits
- rely on getrlimit to DTRT, don't make stack calibration file

My inclination is that we should revert these; but I'm not sure, so
could be persuaded that I'm wrong.

My feeling is that what we had before is a stronger statement (than
using getrlimit) about the expected behaviour of typical Scheme
programs (and in particular those used during the build), and is
therefore worth keeping.  The getrlimit implementation just says "if a
program is running away, we'll generate a Scheme exception a little
bit before you'd otherwise get a core".

But maybe this isn't important enough to justify the hassle around
stack-limit calibration.  What do others think?

- fix check for guile-tools running uninstalled
- fix distcheck hopefully, by cleaning the vm-i-*.i files

Fine.

- fix "linking" of guile-config

I don't understand the problem here.  In what way was @bindir@ not
fully expanded?

- bugfix: don't dynamic link if we found a registered extension

Would be great to have a test for this, if feasible.

> I think the stack calibration stuff is correct, but perhaps more jarring
> in this commit is a move from ./pre-inst-guile to ./meta/guile, and
> ./pre-inst-guile-env to ./meta/uninstalled-env. I describe the rationale
> in 0b6d8fdc28ed8af56e93157179c305fef037e0a0. But then again, given that
> Neil invested so much time into the stack calibration stuff, that might
> be jarring too.

As above - the meta/ stuff looks fine to me, but I'm not sure about
the stack limit change.

Regards,
        Neil

Re: request review: branch "wingo"

[Andy Wingo]

Hi Neil,

Thanks for the review.

On Mon 30 Mar 2009 14:39, Neil Jerram <neil@ossau.uklinux.net> writes:

> - add getrlimit and setrlimit wrappers
>
> Should scm_getrlimit code be surrounded by #ifdef HAVE_GETRLIMIT ?

I think it is -- the same block includes the RLIM_*, the helper
function, and the setrlimit definition. Not very clear though, I admit.

> What's the idea of the numerical args?  Should RLIMIT_XXX be defined
> somewhere as Scheme-level integers?

Uf, dunno. Now that I look at this again I'm not sure. We should
probably just have RLIM_* and not the symbols, right? Easier for tab
completion in any case...

> Does it make sense to want to set the soft limit only?  I thought some
> hard limits were only settable by root, so I would suspect yes.  If
> yes, does the setrlimit implementation support this?

You actually have to set both at the same time, in the system call
interface. So if you want to just modify the soft limit, you have to
first getrlimit to find the hard limit.

Hard limits can only be decreased, not increased, and can be done so by
the user process. This is good for e.g. before forking too.

> - getrlimit-based stack limits
> - rely on getrlimit to DTRT, don't make stack calibration file
>
> My inclination is that we should revert these; but I'm not sure, so
> could be persuaded that I'm wrong.
>
> My feeling is that what we had before is a stronger statement (than
> using getrlimit) about the expected behaviour of typical Scheme
> programs (and in particular those used during the build), and is
> therefore worth keeping.  The getrlimit implementation just says "if a
> program is running away, we'll generate a Scheme exception a little
> bit before you'd otherwise get a core".

I think that with the getrlimit code, we keep these guarantees, as much
as we had them before.

Before, you still didn't know that your program was not going to dump
core, because when you were at 35000 words (say) and a C program
recursed up to the limit without reentering Guile, you'd still dump
core. (A contrived example.)

What we have now is similar -- Guile's stack is set to 80% of the
rlimit, or 1 MB, whichever is less. So in the worst case we have 20% of
runway in which to prevent a segfault, and in the normal case (on my
x86-32 laptop) we have 9 MB. It's actually a stronger guarantee, for
example in the case in which your rlimit was set at 41000 words.

> - fix "linking" of guile-config
>
> I don't understand the problem here.  In what way was @bindir@ not
> fully expanded?

Because it was a make variable and not a shell variable, so it expanded
to ${exec_prefix}/bin. (There was code in the Makefile.am before to sed
in the variables at make-time instead of configure-time, but I had
removed it to simplify things.)

> - bugfix: don't dynamic link if we found a registered extension
>
> Would be great to have a test for this, if feasible.

Done.

>> I think the stack calibration stuff is correct, but perhaps more jarring
>> in this commit is a move from ./pre-inst-guile to ./meta/guile, and
>> ./pre-inst-guile-env to ./meta/uninstalled-env. I describe the rationale
>> in 0b6d8fdc28ed8af56e93157179c305fef037e0a0. But then again, given that
>> Neil invested so much time into the stack calibration stuff, that might
>> be jarring too.
>
> As above - the meta/ stuff looks fine to me, but I'm not sure about
> the stack limit change.

I think that it gives us stronger guarantees and less hassle. Did my
reasons sway your opinion at all?

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Ludovic Courtès]

Andy Wingo <wingo@pobox.com> writes:

> So, I really intended to wait for review, but it's irritating having
> `master' broken, so I went ahead and merged this in.

You waited for 31 hours and I still don't know how it was "broken",
which I find irritating as well.

> I think the stack calibration stuff is correct,

Again, this all boils down to an arbitrary choice: 1 MiB instead of
40 KiB.  Surely someday this won't be enough.

Besides, there's the thread about cross-compilation where we mention
building the compiler with an already installed Guile that may have an
inappropriate stack limit.

> but perhaps more jarring
> in this commit is a move from ./pre-inst-guile to ./meta/guile, and
> ./pre-inst-guile-env to ./meta/uninstalled-env. I describe the rationale
> in 0b6d8fdc28ed8af56e93157179c305fef037e0a0.

I think the rationale ("The proximate cause of its creation is that I
want to be able to build external packages against uninstalled Guile,
and to do that I need guile-tools in the PATH, but I don't want
$top_builddir/libtool in the path.") is questionable.  Things are not
meant to work this way (Libtool's `.la' and executable scripts, `.pc'
files, etc.), so it looks quite hackish to me.

I'm probably biased because I've got used to installing Guile when I
want to test apps against it (I have 1.8 and HEAD under a different
prefix so I can test against both).

Thanks,
Ludo'.

Re: request review: branch "wingo"

[Andy Wingo]

Hi,

On Tue 31 Mar 2009 09:38, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> writes:
>
>> So, I really intended to wait for review, but it's irritating having
>> `master' broken, so I went ahead and merged this in.
>
> You waited for 31 hours and I still don't know how it was "broken",
> which I find irritating as well.

    .scm.go:
    	$(MKDIR_P) `dirname $@`
    	$(top_builddir)/pre-inst-guile					\
    	  -l $(top_builddir)/libguile/stack-limit-calibration.scm	\
    	  $(top_srcdir)/scripts/compile -o "$@" "$<"

This will run the compile script, but since there is no entry point
(-e), you just load the compile script then exit. No compilation
happens.

>
>> I think the stack calibration stuff is correct,
>
> Again, this all boils down to an arbitrary choice: 1 MiB instead of
> 40 KiB.  Surely someday this won't be enough.

I can remove the 1 MiB limit -- perhaps that's the right thing to do,
then. Just use 80% of the rlimit.

> Besides, there's the thread about cross-compilation where we mention
> building the compiler with an already installed Guile that may have an
> inappropriate stack limit.

I don't think that is relevant. Since the Guile that is running would
choose a stack size appropriate for it, based on the host getrlimit,
there would be no problem.

>> but perhaps more jarring
>> in this commit is a move from ./pre-inst-guile to ./meta/guile, and
>> ./pre-inst-guile-env to ./meta/uninstalled-env. I describe the rationale
>> in 0b6d8fdc28ed8af56e93157179c305fef037e0a0.
>
> I think the rationale ("[...] I
> want to be able to build external packages against uninstalled Guile,
> [....]") is questionable.  Things are not
> meant to work this way (Libtool's `.la' and executable scripts, `.pc'
> files, etc.), so it looks quite hackish to me.

Linking against uninstalled libtool libraries works fine, as long as you
don't install. Pkg-config is designed for uninstalled operation, from
pkg-config(1):

  --uninstalled

         Normally if you request the package "foo" and the package
         "foo-uninstalled" exists, pkg-config will prefer the
         "-uninstalled" variant. This allows compilation/linking against
         uninstalled packages. If you specify the "--uninstalled"
         option, pkg-config will return successfully if any
         "-uninstalled" packages are being used, and return failure
         (false) otherwise. (The "PKG_CONFIG_DISABLE_UNINSTALLED"
         environment variable keeps pkg-config from implicitly choosing
         "-uninstalled" packages, so if that variable is set, they will
         only have been used if you pass a name like "foo-uninstalled"
         on the command line explicitly.)

> I'm probably biased because I've got used to installing Guile when I
> want to test apps against it (I have 1.8 and HEAD under a different
> prefix so I can test against both).

I have that too, but it adds a step to the debugging cycle. I don't
think there's any harm in supporting this additional mode of hacking,
which is only for hackers in any case.

Cheers,

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

>     .scm.go:
>     	$(MKDIR_P) `dirname $@`
>     	$(top_builddir)/pre-inst-guile					\
>     	  -l $(top_builddir)/libguile/stack-limit-calibration.scm	\
>     	  $(top_srcdir)/scripts/compile -o "$@" "$<"
>
> This will run the compile script, but since there is no entry point
> (-e), you just load the compile script then exit. No compilation
> happens.

Ouch, indeed.  Mea culpa!

>> Besides, there's the thread about cross-compilation where we mention
>> building the compiler with an already installed Guile that may have an
>> inappropriate stack limit.
>
> I don't think that is relevant. Since the Guile that is running would
> choose a stack size appropriate for it, based on the host getrlimit,
> there would be no problem.

The already-installed Guile wouldn't use getrlimit(2) since that would
be an old 1.8.

> Linking against uninstalled libtool libraries works fine, as long as you
> don't install.

That's right, but that seems awkward to me, except for tests.

> Pkg-config is designed for uninstalled operation, from
> pkg-config(1):

OK, I didn't know that.  Sorry for talking too fast.

> I have that too, but it adds a step to the debugging cycle. I don't
> think there's any harm in supporting this additional mode of hacking,
> which is only for hackers in any case.

Well, yes.

Thanks,
Ludo'.

Re: request review: branch "wingo"

[Andy Wingo]

Howdy howdy,

On Tue 31 Mar 2009 14:31, ludo@gnu.org (Ludovic Courtès) writes:

>>> Besides, there's the thread about cross-compilation where we mention
>>> building the compiler with an already installed Guile that may have an
>>> inappropriate stack limit.
>>
>> I don't think that is relevant. Since the Guile that is running would
>> choose a stack size appropriate for it, based on the host getrlimit,
>> there would be no problem.
>
> The already-installed Guile wouldn't use getrlimit(2) since that would
> be an old 1.8.

You probably saw the other response already, but I don't think that the
compiler will work with 1.8 as a "host". You'd have to install a 1.9 on
the host, and somehow tell it that it should compile .go files with the
endianness of the target.

>> Linking against uninstalled libtool libraries works fine, as long as you
>> don't install.
>
> That's right, but that seems awkward to me, except for tests.

I've used it in GStreamer for years now -- not a proof of correctness to
be sure, but it works well enough to hack.

Cheers,

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> Hi Neil,
>
> Thanks for the review.

Pleasure.  By the way, on the general point of pushing
possibly-contentious stuff to git.sv.gnu.org master...

- I certainly don't think it's the end of the world to do this.  We
  have complete history and can back things out if we need to.  It's
  also (IMO) a reasonable way of asking for review.  And if another
  developer ends up pulling something that they don't want, it's easy
  enough in Git to remove those things from a local tree.

- On the other hand, it does feel slightly incourteous, and the
  argument about master being broken sounds like nonsense - because
  everyone has their own local branches, and AFAIK there is never any
  need to push those apart from for publication purposes.  (Is there?)

On balance, I think it would be better to give an opportunity for
review first, either as a patch email or as commits on another branch.

> On Mon 30 Mar 2009 14:39, Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> - add getrlimit and setrlimit wrappers
>>
>> Should scm_getrlimit code be surrounded by #ifdef HAVE_GETRLIMIT ?
>
> I think it is -- the same block includes the RLIM_*, the helper
> function, and the setrlimit definition. Not very clear though, I admit.

No, it's fine as is then; my fault for missing that.

>> What's the idea of the numerical args?  Should RLIMIT_XXX be defined
>> somewhere as Scheme-level integers?
>
> Uf, dunno. Now that I look at this again I'm not sure. We should
> probably just have RLIM_* and not the symbols, right? Easier for tab
> completion in any case...

I personally like the symbols, but precedent (e.g. socket, bind etc.)
would favour just the integer constants.  If the constants are also
better for completion, I guess that clinches it. :-)

>> Does it make sense to want to set the soft limit only?  I thought some
>> hard limits were only settable by root, so I would suspect yes.  If
>> yes, does the setrlimit implementation support this?
>
> You actually have to set both at the same time, in the system call
> interface. So if you want to just modify the soft limit, you have to
> first getrlimit to find the hard limit.
>
> Hard limits can only be decreased, not increased, and can be done so by
> the user process. This is good for e.g. before forking too.

OK, fine, thanks for explaining that.

>> - getrlimit-based stack limits
>> - rely on getrlimit to DTRT, don't make stack calibration file
>>
>> My inclination is that we should revert these; but I'm not sure, so
>> could be persuaded that I'm wrong.
>>
>> My feeling is that what we had before is a stronger statement (than
>> using getrlimit) about the expected behaviour of typical Scheme
>> programs (and in particular those used during the build), and is
>> therefore worth keeping.  The getrlimit implementation just says "if a
>> program is running away, we'll generate a Scheme exception a little
>> bit before you'd otherwise get a core".
>
> I think that with the getrlimit code, we keep these guarantees, as much
> as we had them before.
>
> Before, you still didn't know that your program was not going to dump
> core, because when you were at 35000 words (say) and a C program
> recursed up to the limit without reentering Guile, you'd still dump
> core. (A contrived example.)
>
> What we have now is similar -- Guile's stack is set to 80% of the
> rlimit, or 1 MB, whichever is less. So in the worst case we have 20% of
> runway in which to prevent a segfault, and in the normal case (on my
> x86-32 laptop) we have 9 MB. It's actually a stronger guarantee, for
> example in the case in which your rlimit was set at 41000 words.

Yes, OK; after more thought I now agree with you.  There was some
discussion when I recently raised the question of stack overflow
(starting here:
http://www.mail-archive.com/guile-devel@gnu.org/msg02098.html), but
the only concern there was about how the stack depth tracking is
implemented - which there is no suggestion (now) of changing.

I'm favouring 80% getrlimit now because I don't think Guile (and I)
have really benefitted from the time spent investigating stack
overflows recently,  OK, so we are familiar in more detail with some
platforms using more stack than others, but that's not that
interesting and the time would probably have been better spent on
something else... so let's try not to have to do any more of this in
future.

So my concern now is are there platforms that don't provide getrlimit
(or equivalent)?  If not, I'm happy to rip out all the stack
calibration stuff; but if there are, don't we still need to keep it as
a fallback option?

>> - fix "linking" of guile-config
>>
>> I don't understand the problem here.  In what way was @bindir@ not
>> fully expanded?
>
> Because it was a make variable and not a shell variable, so it expanded
> to ${exec_prefix}/bin. (There was code in the Makefile.am before to sed
> in the variables at make-time instead of configure-time, but I had
> removed it to simplify things.)

Should we then put the Makefile.am code back?  Or does that break your
uninstalled usage?  Other things being equal, I think it's more
important for the generated guile-config to be simple, than for our
Makefile.am to be simple.

>> - bugfix: don't dynamic link if we found a registered extension
>>
>> Would be great to have a test for this, if feasible.
>
> Done.

Great, thanks!

Regards,
        Neil

Re: request review: branch "wingo"

[Ludovic Courtès]

Hi Neil,

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

> Should we then put the Makefile.am code back?  Or does that break your
> uninstalled usage?  Other things being equal, I think it's more
> important for the generated guile-config to be simple, than for our
> Makefile.am to be simple.

Speaking of which, should we mark `guile-config' as deprecated in favor
of `pkg-config' (in 1.9)?

Thanks,
Ludo'.

Re: request review: branch "wingo"

[Ludovic Courtès]

Hello!

Andy Wingo <wingo@pobox.com> writes:

> On Tue 31 Mar 2009 14:31, ludo@gnu.org (Ludovic Courtès) writes:
>
>>>> Besides, there's the thread about cross-compilation where we mention
>>>> building the compiler with an already installed Guile that may have an
>>>> inappropriate stack limit.
>>>
>>> I don't think that is relevant. Since the Guile that is running would
>>> choose a stack size appropriate for it, based on the host getrlimit,
>>> there would be no problem.
>>
>> The already-installed Guile wouldn't use getrlimit(2) since that would
>> be an old 1.8.
>
> You probably saw the other response already, but I don't think that the
> compiler will work with 1.8 as a "host".

Yes, I saw it right after, so forget my remark.

> You'd have to install a 1.9 on
> the host, and somehow tell it that it should compile .go files with the
> endianness of the target.

OK.

Thanks,
Ludo'.

Re: request review: branch "wingo"

[Greg Troxel]

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


  Speaking of which, should we mark `guile-config' as deprecated in favor
  of `pkg-config' (in 1.9)?

I think so.  An advantage of pkg-config, beyond the
not-rolling-your-own, is that pkgsrc has generic fixup support for -R,
and a separate fix for guile-config, and it would be nice to get rid of
the special case code eventually.


[-- Attachment #2: Type: application/pgp-signature, Size: 193 bytes --]

Re: request review: branch "wingo"

[Neil Jerram]

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

> Hi Neil,
>
> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Should we then put the Makefile.am code back?  Or does that break your
>> uninstalled usage?  Other things being equal, I think it's more
>> important for the generated guile-config to be simple, than for our
>> Makefile.am to be simple.
>
> Speaking of which, should we mark `guile-config' as deprecated in favor
> of `pkg-config' (in 1.9)?

I have no objection to that.  We still want to support existing
scripts, of course - but I assume that's why you said "mark as
deprecated" and not "remove". :-)

    Neil

Re: request review: branch "wingo"

[Andy Wingo]

Howdy,

On Wed 01 Apr 2009 15:23, Neil Jerram <neil@ossau.uklinux.net> writes:

> ludo@gnu.org (Ludovic Courtès) writes:
>
> I have no objection to that.  We still want to support existing
> scripts, of course - but I assume that's why you said "mark as
> deprecated" and not "remove". :-)

I agree :) We can make guile-config get its information from pkg-config
instead.

Cheers,

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Andy Wingo]

Howdy howdy,

On Tue 31 Mar 2009 16:25, Neil Jerram <neil@ossau.uklinux.net> writes:

> - On the other hand, it does feel slightly incourteous, and the
>   argument about master being broken sounds like nonsense - because
>   everyone has their own local branches, and AFAIK there is never any
>   need to push those apart from for publication purposes.  (Is there?)

Yeah, my apologies.

By way of explanation though perhaps not justification, I was getting to
the point at work where I wanted to tell folks to add Guile to the
autobuilding set of dependencies -- but I had to give them a pointer to
a branch that worked, and preferred that that branch be `master'.

Anyway. Apologies again, and hopefully we won't go through this again
any time soon.

>>> What's the idea of the numerical args?  Should RLIMIT_XXX be defined
>>> somewhere as Scheme-level integers?
>>
>> Uf, dunno. Now that I look at this again I'm not sure. We should
>> probably just have RLIM_* and not the symbols, right? Easier for tab
>> completion in any case...
>
> I personally like the symbols, but precedent (e.g. socket, bind etc.)
> would favour just the integer constants.  If the constants are also
> better for completion, I guess that clinches it. :-)

Yeah I guess so ;) I'll get to that soon, and document and add a NEWS
entry.

> I'm favouring 80% getrlimit now because I don't think Guile (and I)
> have really benefitted from the time spent investigating stack
> overflows recently,  OK, so we are familiar in more detail with some
> platforms using more stack than others, but that's not that
> interesting and the time would probably have been better spent on
> something else... so let's try not to have to do any more of this in
> future.
>
> So my concern now is are there platforms that don't provide getrlimit
> (or equivalent)?  If not, I'm happy to rip out all the stack
> calibration stuff; but if there are, don't we still need to keep it as
> a fallback option?

I went ahead and pushed an 80%-only patch, as you suggest.

All UNIX systems that I know of have getrlimit. Windows does not of
course, but it seems that their stack limit is hard-coded:

    http://www.mapleprimes.com/forum/stack_limit

In any case, it seems that when Windows people bump up against this
limit they'll let us know and we can send them looking for the right
#define.

>>> - fix "linking" of guile-config
>>>
>>> I don't understand the problem here.  In what way was @bindir@ not
>>> fully expanded?
>>
>> Because it was a make variable and not a shell variable, so it expanded
>> to ${exec_prefix}/bin. (There was code in the Makefile.am before to sed
>> in the variables at make-time instead of configure-time, but I had
>> removed it to simplify things.)
>
> Should we then put the Makefile.am code back?  Or does that break your
> uninstalled usage?  Other things being equal, I think it's more
> important for the generated guile-config to be simple, than for our
> Makefile.am to be simple.

Perhaps we can just change guile-config to use pkg-config instead, as
noted in the other thread. That will remove the need for the "linking"
as the installed vs uninstalled case should be handled by pkg-config.

Cheers,

Andy
-- 
http://wingolog.org/

Re: request review: branch "wingo"

[Neil Jerram]

Andy Wingo <wingo@pobox.com> writes:

> I went ahead and pushed an 80%-only patch, as you suggest.
>
> All UNIX systems that I know of have getrlimit. Windows does not of
> course, but it seems that their stack limit is hard-coded:
>
>     http://www.mapleprimes.com/forum/stack_limit
>
> In any case, it seems that when Windows people bump up against this
> limit they'll let us know and we can send them looking for the right
> #define.

OK, that sounds like a reasonable position.

Thanks,
        Neil