bug#10208: site-lisp directories in load-path after --no-site-lisp

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Juanma Barranquero]

Package: emacs

On non-w32 systems:

 - init_lread() removes directories matching "site-lisp" from
load-path, and then re-adds them unless --no-site-lisp was passed.
 - However, the site-lisp directories under the installation
(lread.c:4189) and source (lread.c:4229) directories are added
unconditionally.

On w32 systems, load-path gets its default from EMACSLOADPATH (defined
in w32.c:1592), which includes %emacs_dir%/site-lisp and
%emacs_dir%/../site-lisp unconditionally.

As an example, I build Emacs in

  C:\emacs\trunk

and have my site-lisp data in C:\emacs\site-lisp. After "emacs -Q",
load-path contains:

  c:/emacs/trunk/site-lisp
  C:/emacs/trunk/../site-lisp
  c:/emacs/site-lisp/bm
  c:/emacs/site-lisp/lua
  c:/emacs/site-lisp/slime
  c:/emacs/site-lisp/slime/contrib
  c:/emacs/site-lisp/slime/doc
  C:/emacs/trunk/lisp
  c:/emacs/trunk/lisp/calc
  [...]
  c:/emacs/trunk/lisp/cedet
  C:/emacs/trunk/leim

    Juanma

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Glenn Morris]

Juanma Barranquero wrote:

>  - init_lread() removes directories matching "site-lisp" from
> load-path, and then re-adds them unless --no-site-lisp was passed.
>  - However, the site-lisp directories under the installation
> (lread.c:4189) and source (lread.c:4229) directories are added
> unconditionally.

Fixed.

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Juanma Barranquero]

On Sun, Dec 4, 2011 at 00:29, Juanma Barranquero <lekktu@gmail.com> wrote:

> On w32 systems, load-path gets its default from EMACSLOADPATH (defined
> in w32.c:1592), which includes %emacs_dir%/site-lisp and
> %emacs_dir%/../site-lisp unconditionally.

Fixed in revno:106634.

    Juanma

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Achim Gratz]

Glenn Morris <rgm@gnu.org> writes:
>>  - init_lread() removes directories matching "site-lisp" from
>> load-path, and then re-adds them unless --no-site-lisp was passed.
>>  - However, the site-lisp directories under the installation
>> (lread.c:4189) and source (lread.c:4229) directories are added
>> unconditionally.
>
> Fixed.

I still get this with a fresh build from the git repo:

emacs/build> emacs -batch -Q --eval "(mapcar 'message load-path )"
/usr/local/share/emacs/24.0.92/site-lisp
/usr/local/share/emacs/site-lisp
/usr/local/share/emacs/site-lisp/org
/usr/local/share/emacs/24.0.92/lisp
/usr/local/share/emacs/24.0.92/lisp/…
/usr/local/share/emacs/24.0.92/leim

The two site-lisp directories are configured as $(locallisppath).  If I
remove this, I don't get any site-lisp directories when starting Emacs
normally.


Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

DIY Stuff:
http://Synth.Stromeko.net/DIY.html

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Achim Gratz]

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

Achim Gratz <Stromeko@nexgo.de> writes:
> I still get this with a fresh build from the git repo:
>
> emacs/build> emacs -batch -Q --eval "(mapcar 'message load-path )"
> /usr/local/share/emacs/24.0.92/site-lisp
> /usr/local/share/emacs/site-lisp
> /usr/local/share/emacs/site-lisp/org
> /usr/local/share/emacs/24.0.92/lisp
> /usr/local/share/emacs/24.0.92/lisp/…
> /usr/local/share/emacs/24.0.92/leim
>
> The two site-lisp directories are configured as $(locallisppath).  If I
> remove this, I don't get any site-lisp directories when starting Emacs
> normally.

If I put the equivalent (I think) of what lread.c should be doing into
no-site-lisp.el


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: no-site-lisp.el --]
[-- Type: text/x-emacs-lisp, Size: 238 bytes --]

(setq x-site-lisp nil)
(while (progn
	 (setq tem (car load-path))
	 (setq tem1 (string-match "site-lisp" tem))
	 (if tem1
	     (progn
	       (setq load-path (cdr load-path))
	       (setq x-site-lisp (cons tem x-site-lisp)))
	   nil)))

[-- Attachment #3: Type: text/plain, Size: 515 bytes --]


and load it like this, I get the expected result:

emacs/build> emacs --batch -Q -l no-site-lisp.el --eval "(mapcar 'message load-path )"
/usr/local/share/emacs/24.0.92/lisp
/usr/local/share/emacs/24.0.92/lisp/…
/usr/local/share/emacs/24.0.92/leim

I must be missing something or the option never propagates to lread.c?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Achim Gratz]

Achim Gratz <Stromeko@nexgo.de> writes:
> I must be missing something or the option never propagates to lread.c?

The whole incantation that removes the site-lisp from load-path is
safeguarded with "if (!NILP (Vinstallation_directory))", so for the
installed Emacs the --no-site-lisp option is never actually acted upon,
since installation-directory is nil.

The places to safeguard are actually further down, so the following
patch (I have not touched indentation to minimize the number of changed
lines) would allow --no-site-lisp to take effect as described in the man
page (the way I've read it, anyway).

--8<---------------cut here---------------start------------->8---
diff --git a/src/lread.c b/src/lread.c
index 8e6b6f6..6367370 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -4128,10 +4128,9 @@ init_lread (void)
     {
       if (! NILP (Fequal (dump_path, Vload_path)))
 	{
+	  int nilp_inst_dir = NILP (Vinstallation_directory);
+	  Lisp_Object tem, tem1, sitelisp;
 	  Vload_path = decode_env_path (0, normal);
-	  if (!NILP (Vinstallation_directory))
-	    {
-	      Lisp_Object tem, tem1, sitelisp;
 
 	      /* Remove site-lisp dirs from path temporarily and store
 		 them in sitelisp, then conc them on at the end so
@@ -4169,6 +4168,7 @@ init_lread (void)
 		   Lisp dirs instead.  */
 		Vload_path = nconc2 (Vload_path, dump_path);
 
+	      if (!nilp_inst_dir) {
 	      /* Add leim under the installation dir, if it exists.  */
 	      tem = Fexpand_file_name (build_string ("leim"),
 				       Vinstallation_directory);
@@ -4191,7 +4191,7 @@ init_lread (void)
 			Vload_path = Fcons (tem, Vload_path);
 		    }
 		}
-
+	      }
 	      /* If Emacs was not built in the source directory,
 		 and it is run from where it was built, add to load-path
 		 the lisp, leim and site-lisp dirs under that directory.  */
@@ -4237,7 +4237,7 @@ init_lread (void)
 		}
 	      if (!NILP (sitelisp) && !no_site_lisp)
 		Vload_path = nconc2 (Fnreverse (sitelisp), Vload_path);
-	    }
+	    
 	}
     }
   else
--8<---------------cut here---------------end--------------->8---


HTH,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Glenn Morris]

Achim Gratz wrote:

> The whole incantation that removes the site-lisp from load-path is
> safeguarded with "if (!NILP (Vinstallation_directory))", so for the
> installed Emacs the --no-site-lisp option is never actually acted upon,
> since installation-directory is nil.

Thanks for tracking this down, I installed something similar.
I had not appreciated half of what init_lread does, nor that
installation-directory is actually nil in installed Emacs (and would be
better named "build-directory" IMO).

AFAICS, there are still the following issues:

i) EMACSLOADPATH overrides --no-site-lisp; it probably should not.

ii) This presumably means --no-site-lisp does not work in --with-ns
builds, since they set load-path via EMACSLOADPATH (IMO, it's a bug that
one part of Emacs uses an env-var just to communicate with another, see eg
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6401#48 )

These could both be solved by simply removing any Vload_path element
matching "site-lisp" near the end of init_lread. (I guess this would
remove the need for the MS Windows build to implement this feature
separately as well.)

This would still leave:

iii) Any --enable-locallisppath element that does not happen to have
"site-lisp" in its name will not get removed (but such elements already
mess things up because they will break the sorting of load-path that
init_lread tries to do).


Perhaps it would be better to split PATH_LOADSEARCH in epaths.h into a
default and site-lisp component, and simply not add the latter with
--no-site-lisp.


Finally, I noticed that you have some changes installed (for Org) that
are not marked as "tiny changes", yet you don't seem to have a copyright
assignment. IF you have not completed one, are you willing to do so?
The process is straightforward.

bug#10208: site-lisp directories in load-path after --no-site-lisp

[ASSI]

Am 11.01.2012 02:08, schrieb Glenn Morris:
> Thanks for tracking this down, I installed something similar.

Appreciated, however I can't check right now.

> I had not appreciated half of what init_lread does, nor that
> installation-directory is actually nil in installed Emacs (and would be
> better named "build-directory" IMO).

Yes, this whole business of naming and using these variables might 
require something of an overhaul.

> AFAICS, there are still the following issues:
>
> i) EMACSLOADPATH overrides --no-site-lisp; it probably should not.

I haven't given this one much thought so far; but again, the general 
scheme might be in need of re-evaluation.

> ii) This presumably means --no-site-lisp does not work in --with-ns
> builds, since they set load-path via EMACSLOADPATH (IMO, it's a bug that
> one part of Emacs uses an env-var just to communicate with another, see eg
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=6401#48 )
>
> These could both be solved by simply removing any Vload_path element
> matching "site-lisp" near the end of init_lread. (I guess this would
> remove the need for the MS Windows build to implement this feature
> separately as well.)

I don't have a build environment set up on Windows, so I can't comment here.

> This would still leave:
>
> iii) Any --enable-locallisppath element that does not happen to have
> "site-lisp" in its name will not get removed (but such elements already
> mess things up because they will break the sorting of load-path that
> init_lread tries to do).
>
>
> Perhaps it would be better to split PATH_LOADSEARCH in epaths.h into a
> default and site-lisp component, and simply not add the latter with
> --no-site-lisp.

I'd be in favor of having separate path variables.  The two searchpath 
components are already seperately available in make during the build, so 
it may not be even that difficult.  However, such a change might need to 
be postponed to the next release of Emacs.

> Finally, I noticed that you have some changes installed (for Org) that
> are not marked as "tiny changes", yet you don't seem to have a copyright
> assignment. IF you have not completed one, are you willing to do so?
> The process is straightforward.

I had sent you a mail last year regarding this, I'll send it again. 
Please let me know if you don't get it.


-- 
Achim.

(on the road :-)

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Glenn Morris]

ASSI wrote:

> I'd be in favor of having separate path variables.  The two searchpath
> components are already seperately available in make during the build,
> so it may not be even that difficult.  However, such a change might
> need to be postponed to the next release of Emacs.

Yup.

>> Finally, I noticed that you have some changes installed (for Org) that
>> are not marked as "tiny changes", yet you don't seem to have a copyright
>> assignment. IF you have not completed one, are you willing to do so?
>> The process is straightforward.
>
> I had sent you a mail last year regarding this, I'll send it again.
> Please let me know if you don't get it.

Sorry, I had forgotten that we had discussed this before. Yes, I got
your mail; thanks.

bug#10208: site-lisp directories in load-path after --no-site-lisp

[Achim Gratz]

Glenn Morris <rgm@gnu.org> writes:
> Thanks for tracking this down, I installed something similar.

I confirm that "--no-site-lisp" now works correctly for my test case.


Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada