* request review: branch "wingo" @ 2009-03-27 23:29 Andy Wingo 2009-03-29 6:20 ` Andy Wingo 0 siblings, 1 reply; 17+ messages in thread From: Andy Wingo @ 2009-03-27 23:29 UTC (permalink / raw) To: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-27 23:29 request review: branch "wingo" Andy Wingo @ 2009-03-29 6:20 ` Andy Wingo 2009-03-29 21:16 ` Neil Jerram ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Andy Wingo @ 2009-03-29 6:20 UTC (permalink / raw) To: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-29 6:20 ` Andy Wingo @ 2009-03-29 21:16 ` Neil Jerram 2009-03-30 21:39 ` Neil Jerram 2009-03-31 16:38 ` Ludovic Courtès 2 siblings, 0 replies; 17+ messages in thread From: Neil Jerram @ 2009-03-29 21:16 UTC (permalink / raw) To: Andy Wingo; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-29 6:20 ` Andy Wingo 2009-03-29 21:16 ` Neil Jerram @ 2009-03-30 21:39 ` Neil Jerram 2009-03-31 3:31 ` Andy Wingo 2009-03-31 16:38 ` Ludovic Courtès 2 siblings, 1 reply; 17+ messages in thread From: Neil Jerram @ 2009-03-30 21:39 UTC (permalink / raw) To: Andy Wingo; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-30 21:39 ` Neil Jerram @ 2009-03-31 3:31 ` Andy Wingo 2009-03-31 23:25 ` Neil Jerram 0 siblings, 1 reply; 17+ messages in thread From: Andy Wingo @ 2009-03-31 3:31 UTC (permalink / raw) To: Neil Jerram; +Cc: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 3:31 ` Andy Wingo @ 2009-03-31 23:25 ` Neil Jerram 2009-04-01 7:49 ` Ludovic Courtès 2009-04-03 17:51 ` Andy Wingo 0 siblings, 2 replies; 17+ messages in thread From: Neil Jerram @ 2009-03-31 23:25 UTC (permalink / raw) To: Andy Wingo; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 23:25 ` Neil Jerram @ 2009-04-01 7:49 ` Ludovic Courtès 2009-04-01 13:46 ` Greg Troxel 2009-04-01 22:23 ` Neil Jerram 2009-04-03 17:51 ` Andy Wingo 1 sibling, 2 replies; 17+ messages in thread From: Ludovic Courtès @ 2009-04-01 7:49 UTC (permalink / raw) To: guile-devel 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'. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-04-01 7:49 ` Ludovic Courtès @ 2009-04-01 13:46 ` Greg Troxel 2009-04-01 22:23 ` Neil Jerram 1 sibling, 0 replies; 17+ messages in thread From: Greg Troxel @ 2009-04-01 13:46 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guile-devel [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-04-01 7:49 ` Ludovic Courtès 2009-04-01 13:46 ` Greg Troxel @ 2009-04-01 22:23 ` Neil Jerram 2009-04-03 17:24 ` Andy Wingo 1 sibling, 1 reply; 17+ messages in thread From: Neil Jerram @ 2009-04-01 22:23 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-04-01 22:23 ` Neil Jerram @ 2009-04-03 17:24 ` Andy Wingo 0 siblings, 0 replies; 17+ messages in thread From: Andy Wingo @ 2009-04-03 17:24 UTC (permalink / raw) To: Neil Jerram; +Cc: Ludovic Courtès, guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 23:25 ` Neil Jerram 2009-04-01 7:49 ` Ludovic Courtès @ 2009-04-03 17:51 ` Andy Wingo 2009-04-12 13:00 ` Neil Jerram 1 sibling, 1 reply; 17+ messages in thread From: Andy Wingo @ 2009-04-03 17:51 UTC (permalink / raw) To: Neil Jerram; +Cc: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-04-03 17:51 ` Andy Wingo @ 2009-04-12 13:00 ` Neil Jerram 0 siblings, 0 replies; 17+ messages in thread From: Neil Jerram @ 2009-04-12 13:00 UTC (permalink / raw) To: Andy Wingo; +Cc: guile-devel 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-29 6:20 ` Andy Wingo 2009-03-29 21:16 ` Neil Jerram 2009-03-30 21:39 ` Neil Jerram @ 2009-03-31 16:38 ` Ludovic Courtès 2009-03-31 19:11 ` Andy Wingo 2 siblings, 1 reply; 17+ messages in thread From: Ludovic Courtès @ 2009-03-31 16:38 UTC (permalink / raw) To: guile-devel 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'. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 16:38 ` Ludovic Courtès @ 2009-03-31 19:11 ` Andy Wingo 2009-03-31 21:31 ` Ludovic Courtès 0 siblings, 1 reply; 17+ messages in thread From: Andy Wingo @ 2009-03-31 19:11 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 19:11 ` Andy Wingo @ 2009-03-31 21:31 ` Ludovic Courtès 2009-03-31 22:47 ` Andy Wingo 0 siblings, 1 reply; 17+ messages in thread From: Ludovic Courtès @ 2009-03-31 21:31 UTC (permalink / raw) To: guile-devel 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'. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 21:31 ` Ludovic Courtès @ 2009-03-31 22:47 ` Andy Wingo 2009-04-01 7:51 ` Ludovic Courtès 0 siblings, 1 reply; 17+ messages in thread From: Andy Wingo @ 2009-03-31 22:47 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guile-devel 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/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: request review: branch "wingo" 2009-03-31 22:47 ` Andy Wingo @ 2009-04-01 7:51 ` Ludovic Courtès 0 siblings, 0 replies; 17+ messages in thread From: Ludovic Courtès @ 2009-04-01 7:51 UTC (permalink / raw) To: guile-devel 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'. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2009-04-12 13:00 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-27 23:29 request review: branch "wingo" Andy Wingo 2009-03-29 6:20 ` Andy Wingo 2009-03-29 21:16 ` Neil Jerram 2009-03-30 21:39 ` Neil Jerram 2009-03-31 3:31 ` Andy Wingo 2009-03-31 23:25 ` Neil Jerram 2009-04-01 7:49 ` Ludovic Courtès 2009-04-01 13:46 ` Greg Troxel 2009-04-01 22:23 ` Neil Jerram 2009-04-03 17:24 ` Andy Wingo 2009-04-03 17:51 ` Andy Wingo 2009-04-12 13:00 ` Neil Jerram 2009-03-31 16:38 ` Ludovic Courtès 2009-03-31 19:11 ` Andy Wingo 2009-03-31 21:31 ` Ludovic Courtès 2009-03-31 22:47 ` Andy Wingo 2009-04-01 7:51 ` Ludovic Courtès
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).