Opened 11 years ago
Closed 6 years ago
#11359 closed defect (fixed)
ECL does not fully recover from relocation
Reported by: | nbruin | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | packages: standard | Keywords: | ecl hardcoded paths |
Cc: | Merged in: | ||
Authors: | Reviewers: | Jeroen Demeyer | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
CPPFLAGS are recorded by ECL for future use as default values for c::*cc-flags*
. But we want Sage to be relocate-able, so no build paths should be hardcoded.
This ticket proposes various changes to the ECL spkg-install that may work.
See also #11348 and this sage-devel thread.
Attachments (1)
Change History (33)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
Confirmed! This seems to be indeed the only problem. I did the following in local/bin
$ mv ecl ecl.bin $ cat <<<EOF >ecl #!/bin/sh exec ecl.bin \ -eval '(setf c::*ecl-include-directory* (concatenate '"'"'string (SI:GETENV "SAGE_ROOT") "/local/include/"))'\ -eval '(setf c::*ecl-library-directory* (concatenate '"'"'string (SI:GETENV "SAGE_ROOT") "/local/lib/"))' \ "$@" EOF $ chmod a+x ecl
after which rebuilding maxima succeeds in a relocated position.
Note that this piece of code does not scrub c::*cc-flags* or c::*ld-flags* yet.
The definitions for the default values of these variables seem to live in spkg/build/ecl-*/src/src/cmp/cmpdefs.lsp so that is probably what we need to patch.
comment:3 Changed 11 years ago by
The dirty c::*cc-flags*
and c::*ld-flags
are our own fault. In the ECL spkg-install we have the lines:
CPPFLAGS="$CPPFLAGS -I$SAGE_LOCAL/include" LDFLAGS="$LDFLAGS -L$SAGE_LOCAL/lib"
but we are adding paths there, not flags! It needs the include path to find the boehm-gc headers, but our library search paths should already be set up. Can we instead just do
C_INCLUDE_PATH="$SAGE_LOCAL/include" export C_INCLUDE_PATH
If the library really needs to be mentioned here, perhaps also a
LIBRARY_PATH="$SAGE_LOCAL/lib" export LIBRARY_PATH
but I think we can leave that one out. With this I get an ECL where we obtain
> (require 'cmp) > c::*cc-flags* " -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -g -O2 -fPIC -Dlinux" > c::*ld-flags* " -lecl -lgmp -lgc -ldl -lm "
so no paths in the flags variables anymore! That just leaves figuring out where best to change the values of the *ecl-...-directory* variables.
comment:4 follow-up: ↓ 19 Changed 11 years ago by
The following two fixes resolve the issues
Cleaning paths from *..-flags* variables
In <ECL-PACKAGE>/spkg-install
replace the lines
CPPFLAGS="$CPPFLAGS -I$SAGE_LOCAL/include" LDFLAGS="$LDFLAGS -L$SAGE_LOCAL/lib"
by
C_INCLUDE_PATH="$SAGE_LOCAL/include" export C_INCLUDE_PATH
(web documentation indicates that Sun Studio and IBM compilers also support this variable, so although it's not POSIX (neither is CPPFLAGS), it doesn't seem to be GNU exclusive.)
Making initialization of *ecl-...-directory* variables runtime
Patch <ECL-PACKAGE>/src/src/cmp/cmpdefs.lsp
such that the lines
(defvar *ecl-include-directory* @includedir\@) (defvar *ecl-library-directory* @libdir\@)
are replaced by
;;; The following code sets the default values of the include and library directories used for ;;; invocation of the C-compiler during compilation to values relative to ECL's idea ;;; of the current "system" directory. The logical pathname "SYS:" normally points to ;;; ".../local/lib/ecl", where "..." would normally be "/usr" (if ECL is installed in ;;; "/usr/local"). So, we strip off the last 2 components of "SYS:" and append "include" or ;;; "lib". This results in the values ;;; ".../local/lib/" and ".../local/include/" in the above example. ;;; Since this substitution happens at runtime, these values get adjusted appropriately if ;;; the ".../local" tree gets relocated. (defvar *ecl-include-directory* (namestring (make-pathname :directory (append (butlast (pathname-directory (translate-logical-pathname "SYS:")) 2) '("include"))))) (defvar *ecl-library-directory* (namestring (make-pathname :directory (append (butlast (pathname-directory (translate-logical-pathname "SYS:")) 2) '("lib")))))
comment:5 Changed 11 years ago by
Avoiding patching ECL
The role of defvar
is 2-fold: It declares a variable to have dynamic scoping rather than lexical scoping and it sets the value only if the variable wasn't bound already. So, as long as we execute the two defvar statements
(defvar c::*ecl-include-directory* (namestring (make-pathname :directory (append (butlast (pathname-directory (translate-logical-pathname "SYS:")) 2) '("include"))))) (defvar c::*ecl-library-directory* (namestring (make-pathname :directory (append (butlast (pathname-directory (translate-logical-pathname "SYS:")) 2) '("lib")))))
before the compiler package is loaded, our values will persist. So, if one finds another file of lisp instructions that are guaranteed to be executed upon ECL startup (~/.eclrc
doesn't work because maxima is built by calling ecl -norc
) we're still good. I was not able to find such a file.
comment:6 follow-up: ↓ 7 Changed 11 years ago by
So do we have to do two things here? Reworking spkg-install and the relocation script or just one of them is enough? And then at what stage?
I am ready to cut a new ecl spkg for 4.7.1 if needed. I could take the opportunity to update maxima to 5.24.0 at the same time.
comment:7 in reply to: ↑ 6 Changed 11 years ago by
Replying to fbissey:
So do we have to do two things here? Reworking spkg-install
Yes, that and patch ECL.
and the relocation script
No changes to relocation script required. The patch to ECL means that the default values for the *ecl-...-library*
variables are derived from a quantity that is already aware of ECL's runtime location.
So both changes are necessary and both are to the ecl spkg and both take effect at ECL build-time (well, the patch of course also changes ECL's runtime behaviour. Since a large part of building ECL consists of ECL compiling itself, the two overlap)
(the comment about avoiding patching ECL is just recording possible avenues someone might pursue if they feel really reluctant to patch ECL)
comment:8 follow-up: ↓ 9 Changed 11 years ago by
I am for the less intrusive option but I am also pragmatic if it is far more practical to patch ecl itself. Your option for not patching ecl would involve calling ecl before compiling any lisp code right? Which means before building maxima and sage/libs/ecl.pyx in sage if I understand correctly. The second one especially bother me so patching may be the best avenue.
comment:9 in reply to: ↑ 8 Changed 11 years ago by
Actually, for sage/libs/ecl.pyx it's not so much of a problem, since our initialization of ecllib involves sending some instructions there anyway, so we could just add the defvar
s there. It would add a minuscule overhead for something that is unlikely to have any effect anyway: Who's going to use embedded ECL to compile something? That is one reason to prefer the patch to the compiler package already.
For fixing the behaviour of the ECL executable I haven't actually found a solution other than wrapping it with a shell script, which really does add considerable overhead.
Patching is definitely the cleaner solution, but will likely be with us in perpetuity, with all the extra maintenance of rebasing the patch with any ECL upgrade.
comment:10 Changed 11 years ago by
OK. I am putting cutting a patched ecl according to your instruction on my TODO list for the week starting tomorrow. Like I said I'll probably cut a new maxima as well only one non trivial doctest is being broken by 5.24.0 so it is easy work.
comment:11 follow-up: ↓ 12 Changed 11 years ago by
Avoiding patching ECL in a future release
The development version of ECL has two configuration options that will allow avoiding patching ECL in the future. We'll be able to make a file /path/to/sage.lsp
:
(in-package "SI") (defun customized-boot () (defparameter *ecl-include-directory* ...) (si::top-level t))
and then add the following options to ECL's configure:
--with-extra-files="/path/to/sage.lsp" --with-init-form='(si::customized-boot)'
This option is not available in 11.1.1 yet.
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 11 years ago by
Replying to nbruin:
Avoiding patching ECL in a future release
The development version of ECL has two configuration options that will allow avoiding patching ECL in the future. We'll be able to make a file
/path/to/sage.lsp
:(in-package "SI") (defun customized-boot () (defparameter *ecl-include-directory* ...) (si::top-level t))and then add the following options to ECL's configure:
--with-extra-files="/path/to/sage.lsp" --with-init-form='(si::customized-boot)'This option is not available in 11.1.1 yet.
Should we delay making a new spkg until it is released or do you think we should still do it now with a note to check ecl at the next rev-bump?
comment:13 in reply to: ↑ 12 Changed 11 years ago by
Replying to fbissey:
Should we delay making a new spkg until it is released or do you think we should still do it now with a note to check ecl at the next rev-bump?
That depends on how urgently people think this needs fixing. Given that in practice, it only affects people wanting to build maxima on a moved Sage install and that they have a reasonable workaround of rebuilding ecl first (which builds quickly), I'd say this can wait. But if you're going to make a new ECL package anyway, I'd say patch the flaw, or at least the CCFLAGS thing.
comment:14 Changed 11 years ago by
I am not in a particular hurry as I have other things I'd like to do. So unless there is pressure to do so. I'll wait for the next ecl.
comment:15 Changed 9 years ago by
- Component changed from build to packages: standard
- Description modified (diff)
- Priority changed from minor to blocker
- Status changed from new to needs_review
Changed 9 years ago by
comment:16 follow-up: ↓ 17 Changed 9 years ago by
Whats with the CFLAGS discussion on the ticket? Not necessary any more? The new spkg doesn't address that, it just implements a workaround for broken include paths.
comment:17 in reply to: ↑ 16 Changed 9 years ago by
Replying to vbraun:
The new spkg doesn't address that, it just implements a workaround for broken include paths.
True, but I guess this workaround is sufficient if it makes maxima build.
comment:18 Changed 9 years ago by
I think the c-flags are still a problem. Try this
sage -ecl > (defun f (x y) (+ x y)) > (compile 'f) ;; make sure the compiler package is loaded and initialized > c::*cc-flags* " -I/usr/local/sage/5.9b4/local/include -I/usr/local/sage/5.9b4/local/include -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -g -O2 -fPIC -Dlinux" > c::*ld-flags* " -L/usr/local/sage/5.9b4/local/lib -Wl,--rpath,/usr/local/sage/5.9b4/local/lib -L/usr/local/sage/5.9b4/local/lib -lecl -lgmp -lgc -ldl -lm "
I didn't check, but I assume these paths are still hardcoded and hence not adjusted by relocation. We're putting the "-I" and I think also the "-L" directives into those paths upon ecl config-and-build time. They are just not the right spot to put those. We'd be better off letting the environment be so that these things are found automatically, e.g. via setting the appropriate link paths (already done) and include search paths (for instance via C_INCLUDE_PATH
).
Failing that, we need to patch ECL so that these paths get set relative to $SAGE_ROOT
, evaluated at ecl invokation time rather than during ecl package build.
As explained above, we can now also avoid patching sage and instead configure sage to include our own customized ecl boot routine to set up the environment with the appropriate path information during ecl invokation.
comment:19 in reply to: ↑ 4 ; follow-up: ↓ 20 Changed 9 years ago by
Replying to nbruin:
Cleaning paths from *..-flags* variables
In
<ECL-PACKAGE>/spkg-install
replace the linesCPPFLAGS="$CPPFLAGS -I$SAGE_LOCAL/include" LDFLAGS="$LDFLAGS -L$SAGE_LOCAL/lib"by
C_INCLUDE_PATH="$SAGE_LOCAL/include" export C_INCLUDE_PATH(web documentation indicates that Sun Studio and IBM compilers also support this variable, so although it's not POSIX (neither is CPPFLAGS), it doesn't seem to be GNU exclusive.)
For the record: In contrast to C_INCLUDE_PATH
etc., CPPFLAGS
(and CFLAGS
etc.) are never read by the compiler / preprocessor; their names are just conventions used in Makefiles (and make
's default rules).
comment:20 in reply to: ↑ 19 Changed 9 years ago by
Replying to leif:
For the record: In contrast to
C_INCLUDE_PATH
etc.,CPPFLAGS
(andCFLAGS
etc.) are never read by the compiler / preprocessor; their names are just conventions used in Makefiles (andmake
's default rules).
Ah, OK. For ECL it does have a meaning: the value gets inherited as hard-wired default value for c::*cc-flags*
. There also is c::*ecl-include-directory*
, so including -I
directives in CPPFLAGS
leads to superfluous includes passed to ecl
compiler invocations. That would be one reason to try to get rid of it.
The other issue is that ecl by design hard-wires the default value of c::*ecl-include-directory*
, which is the real obstacle to relocating ecl. We can change that by patching ecl (but since the current behaviour is likely not considered a bug for ecl, we'd be doing that indefinitely) or we can work around it by configuring ecl to include a different setting (as outlined above, with the --with-extra-files
and --with-init-form
directives). [Note that this change is only required if sage builds a "private ecl" (as it does normally). If sage were to rely on an ecl supplied via other means, we wouldn't be responsible for relocating it.]
comment:21 follow-up: ↓ 22 Changed 9 years ago by
Nils or Leif: can you give an example of an actual compilation which fails because of this? The wrong paths might not matter that much if the proper include and library files are found anyway through other means (e.g. environment variables such as CPATH
and LIBRARY_PATH
set in sage-env
).
comment:22 in reply to: ↑ 21 ; follow-ups: ↓ 24 ↓ 25 Changed 9 years ago by
Replying to jdemeyer:
Nils or Leif: can you give an example of an actual compilation which fails because of this? The wrong paths might not matter that much if the proper include and library files are found anyway through other means (e.g. environment variables such as
CPATH
andLIBRARY_PATH
set insage-env
).
It's pretty clear how you can make it fail:
- build sage and ecl in location
<ORIGINAL>
- move sage to another location
- put a bogus file
<ORIGINAL>/local/include/ecl/config.h
(or whatever headers are required) in the old location
now trying to compile a file with ecl will use an invalid header. It's the usual risk of keeping references to uncontrolled locations in lookup tables. You may be exposed to arbitrary crap.
(I admit I haven't actually tried. It may be that with the symlink proposed on #14662 in place, the lookup happens at the right place prior to the place searched due to the -I
directive. But that would really just be a fluke).
I'm a little surprised this suddenly gets marked as blocker when the issue was known for 2 years.
comment:23 Changed 9 years ago by
FWIW, C*PATH
and LIBRARY_PATH
are searched last, after any dirs specified on the command line.
Especially for libs, searching "random" folders is pretty delicate. (Think e.g. of bdists; we of course still have almost the same problem with R[UN]PATH
s, although there's chrpath
, but that wouldn't be applicable in this case.)
And it's perhaps not too uncommon to copy a Sage installation for upgrading, i.e., keeping the files at the original location.
comment:24 in reply to: ↑ 22 Changed 9 years ago by
comment:25 in reply to: ↑ 22 Changed 9 years ago by
Replying to nbruin:
It's pretty clear how you can make it fail:
- build sage and ecl in location
<ORIGINAL>
- move sage to another location
- put a bogus file
<ORIGINAL>/local/include/ecl/config.h
(or whatever headers are required) in the old locationnow trying to compile a file with ecl will use an invalid header.
Fair enough, if you intentionally want to break it by providing a bogus <ORIGINAL>/local/include/ecl/config.h
, you can. But my spkg fixes the problem for people who don't intentionally break their setup.
Note: an old ecl/config.h
(as in the case for a copied install) is fine.
comment:26 Changed 9 years ago by
- Description modified (diff)
- Keywords hardcoded paths added; relocation removed
- Milestone changed from sage-5.10 to sage-5.11
- Priority changed from blocker to major
- Status changed from needs_review to needs_work
I will review the workaround as part of #14662.
Leaving this ticket open until you figure out how to set paths correctly ;-)
comment:27 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:28 Changed 8 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:29 Changed 8 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:30 Changed 8 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:31 Changed 6 years ago by
- Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
- Reviewers set to Jeroen Demeyer
- Status changed from needs_work to positive_review
Obsolete by the new binary packaging I guess.
comment:32 Changed 6 years ago by
- Resolution set to fixed
- Status changed from positive_review to closed
One problem (perhaps the only one?) is that ECL uses the C compiler for compiling lisp (much like cython) and gives it include paths and library paths that it found out upon configuration.
local/bin/sage-location
is obviously not updating those. Example in a relocated "sage -ecl":There is a slight problem that the paths also end up in other variables (perhaps because of the way that sage calls configure when configuring ECL?):
Since it is a bit unsanitary to have references to old locations lying around (one might link to wrong versions of libraries if the old location still exists!), we should probably strip those variables as well.
Possible solutions:
local/bin/sage-location
with the logic to update the relevant parts of ECL. That probably means recompiling (part of) ECL, because these values will be stored as a string in a ".fas"-file somewhere, which essentially a ".so".