From: Michael W. <mic...@fo...> - 2007-10-24 12:33:09
|
* empty SBCL_HOME environment variable is treated the same as unset SBCL_HOME * (ensure-trailing-slash "") now returns "./", instead of throwing a type error Signed-off-by: Michael Weber <mic...@fo...> --- SBCL 1.0.10.55 does not like an empty (but set) SBCL_HOME: $ env SBCL_HOME= bin/sbcl --noinform --core lib/sbcl/sbcl.core debugger invoked on a TYPE-ERROR: The value -1 is not of type (MOD 1152921504606846975). Patch attached. src/code/filesys.lisp | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/code/filesys.lisp b/src/code/filesys.lisp index 53ff874..74d53ec 100644 --- a/src/code/filesys.lisp +++ b/src/code/filesys.lisp @@ -562,6 +562,8 @@ otherwise. An error of type FILE-ERROR is signaled if pathname is wild." t) (defun ensure-trailing-slash (string) + (when (equal string "") + (setq string ".")) (let ((last-char (char string (1- (length string))))) (if (or (eql last-char #\/) #!+win32 @@ -572,7 +574,8 @@ otherwise. An error of type FILE-ERROR is signaled if pathname is wild." (defun sbcl-homedir-pathname () (let ((sbcl-home (posix-getenv "SBCL_HOME"))) ;; SBCL_HOME isn't set for :EXECUTABLE T embedded cores - (when sbcl-home + (when (and sbcl-home + (not (equal sbcl-home ""))) (parse-native-namestring (ensure-trailing-slash sbcl-home))))) -- 1.5.3.1 |
From: Michael W. <mic...@fo...> - 2007-10-25 13:16:12
Attachments:
3ab512f9cec8cca4601af72227dc1cd93b2eda87.diff
|
* make code behave the same for empty and unset environment variables (in particular SBCL_HOME) * (ensure-trailing-slash "") now returns "./", instead of throwing a type error Signed-off-by: Michael Weber <mic...@fo...> --- [ This is an updated version of the patch. ] SBCL 1.0.10.55 does not like an empty (but set) SBCL_HOME: $ env SBCL_HOME= bin/sbcl --noinform --core lib/sbcl/sbcl.core debugger invoked on a TYPE-ERROR: The value -1 is not of type (MOD 1152921504606846975). After discovering that the behavior for empty HOME environment variable has been fixed before as well, I subsequently went through the sources and changed other uses of (sb-posix-)getenv accordingly. Patch below. sbcl builds and passes all tests. Note, that most of the checking of SBCL_HOME is not needed actually, due to the changes in runtime.c which ensure a non-empty SBCL_HOME (except for embedded cores). Their main purpose is now to guard against somebody doing (sb-posix:putenv "SBCL_HOME="), and still getting meaningful errors. I left a few call sites of getenv alone (i.e., changes are not included in this patch), see below: ** src/runtime/{ppc,x86}-darwin-langinfo.c:nl_langinfo This should probably be fixed separately. It seems like a workaround for something (what?), but, e.g., the "POSIX" locale (which is an alias for "C") is not handled correctly. Also, the duplicated code in both files should perhaps be factored out. ** contrib/asdf/asdf.lisp I find this quite ugly (it ends up in ASDF:*CENTRAL-REGISTRY*): (pushnew '(let ((home (sb-ext:posix-getenv "SBCL_HOME"))) (when (and home (not (equal home ""))) (merge-pathnames "site-systems/" (truename home)))) *central-registry*) A nicer way to write that would probably be (it matches the entry with USER-HOMEDIR-PATHNAME), except that it calls something from SB-INT: (pushnew '(merge-pathnames "site-systems/" (sb-int:sbcl-homedir-pathname)) *central-registry*) ** contrib/asdf-install/installer.lisp (defvar *sbcl-home* (directorify (posix-getenv "SBCL_HOME"))) Only problematic if somebody uses (sb-posix:putenv "SBCL_HOME=") before loading asdf-install. But given that *sbcl-home* is user-settable, and not recalculated if SBCL_HOME is modified, I think this is a case of "don't do that then". However, should this actually call SB-INT:SBCL-HOMEDIR-PATHNAME? ** contrib/sb-grovel/def-to-lisp.lisp (split-cflags (sb-ext:posix-getenv "EXTRA_CFLAGS")) This is safe use, because split-cflags returns the same for NIL and "". contrib/asdf-install/installer.lisp | 6 ++++-- contrib/asdf/asdf.lisp | 6 ++++-- contrib/sb-grovel/def-to-lisp.lisp | 12 +++++++----- src/code/filesys.lisp | 5 ++++- src/runtime/runtime.c | 6 ++++-- 5 files changed, 23 insertions(+), 12 deletions(-) |
From: Rudi S. <ru...@co...> - 2007-10-26 04:35:50
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Thanks, applied as 1.0.11.1, with some tweaks. On 25.10.2007, at 21:14, Michael Weber wrote: > (defvar *proxy* (posix-getenv "http_proxy")) > (defvar *cclan-mirror* > - (or (posix-getenv "CCLAN_MIRROR") > - "http://ftp.linux.org.uk/pub/lisp/cclan/")) > + (let ((mirror (posix-getenv "CCLAN_MIRROR"))) > + (or (and mirror > + (not (equal mirror ""))) > + "http://ftp.linux.org.uk/pub/lisp/cclan/"))) We don't want the value of mirror to be T; fixed in commit. > diff --git a/contrib/sb-grovel/def-to-lisp.lisp b/contrib/sb-grovel/=20= > def-to-lisp.lisp > index 30265a0..b4176ca 100644 > --- a/contrib/sb-grovel/def-to-lisp.lisp > +++ b/contrib/sb-grovel/def-to-lisp.lisp > @@ -187,11 +187,13 @@ code: > (funcall (intern "C-CONSTANTS-EXTRACT" (find-package "SB-=20 > GROVEL")) > filename tmp-c-source (constants-package component)) > (unless (do-not-grovel component) > - (let* ((cc (or (sb-ext:posix-getenv "CC") > - ;; It might be nice to include a CONTINUE or > - ;; USE-VALUE restart here, but ASDF seems to =20 > insist > - ;; on handling the errors itself. > - (error "The CC environment variable has not =20 > been set in SB-GROVEL. Since this variable should always be set =20 > during the SBCL build process, this might indicate an SBCL with a =20 > broken contrib installation."))) > + (let* ((cc (let ((cc (sb-ext:posix-getenv "CC"))) > + (if (and cc (not (equal cc ""))) > + cc > + ;; It might be nice to include a CONTINUE or > + ;; USE-VALUE restart here, but ASDF seems =20 > to insist > + ;; on handling the errors itself. > + (error "The CC environment variable has not =20= > been set in SB-GROVEL. Since this variable should always be set =20 > during the SBCL build process, this might indicate an SBCL with a =20 > broken contrib installation.")))) I decided it was slightly less fugly to call posix-getenv twice here, =20= once for the comparison and once for the value. > diff --git a/src/code/filesys.lisp b/src/code/filesys.lisp > index 53ff874..74d53ec 100644 > --- a/src/code/filesys.lisp > +++ b/src/code/filesys.lisp > @@ -562,6 +562,8 @@ otherwise. An error of type FILE-ERROR is =20 > signaled if pathname is wild." > t) > =0C > (defun ensure-trailing-slash (string) > + (when (equal string "") > + (setq string ".")) > (let ((last-char (char string (1- (length string))))) > (if (or (eql last-char #\/) > #!+win32 ensure-trailing-slash is only called from two places, both of them =20 already check for string emptiness. Please holler if I overlooked a =20 reason for this hunk... > diff --git a/src/runtime/runtime.c b/src/runtime/runtime.c > index 5db1453..4847b78 100644 > --- a/src/runtime/runtime.c > +++ b/src/runtime/runtime.c [...] > @@ -362,7 +362,9 @@ main(int argc, char *argv[], char *envp[]) > } > > /* Make sure that SBCL_HOME is set, unless loading an embedded =20= > core. */ > - if (!getenv("SBCL_HOME") && embedded_core_offset =3D=3D 0) { > + const char *sbcl_home =3D getenv("SBCL_HOME"); > + if (!(sbcl_home && *sbcl_home) > + && embedded_core_offset =3D=3D 0) { > char *envstring, *copied_core, *dir; > char *stem =3D "SBCL_HOME=3D"; > copied_core =3D copied_string(core); Moved the variable definition upwards to the start of the function; =20 you can call me old-fashioned now. :-) Cheers, Rudi -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.7 (Darwin) iD8DBQFHIW6S765FppppCGcRAsUgAKDAzJ25J6/a+zgIgjZmt4uuhpO3+gCfVaLo 7QUMZk/p5Hx44gLzzEonY0U=3D =3Dr4Yi -----END PGP SIGNATURE----- |