unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Correct Path to Emacs C Sources after Installation
@ 2014-11-04 21:43 Alexander Shukaev
  2014-11-05  1:39 ` Stephen J. Turnbull
  2014-11-05 15:56 ` Eli Zaretskii
  0 siblings, 2 replies; 6+ messages in thread
From: Alexander Shukaev @ 2014-11-04 21:43 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1: Type: text/plain, Size: 1720 bytes --]

Hello everyone,

I've raised a discussion about the "source-directory" variable on another
mailing list and we've come to some intermediate conclusion which might be
satisfying for everyone, so I think it is time to consider implementation
details.

Motivation is as follows. Currently, there is the following code:

  DEFVAR_LISP ("source-directory", Vsource_directory,
       doc: /* Directory in which Emacs sources were found when Emacs was
built.
You cannot count on them to still be there!  */);
  Vsource_directory
    = Fexpand_file_name (build_string ("../"),
 Fcar (decode_env_path (0, PATH_DUMPLOADSEARCH, 0)));


and "Vsource_directory", i.e. this is the default. Notice that
"PATH_DUMPLOADSEARCH" is a hard coded path to the root directory containing
Emacs sources during build. It means that in overwhelming number of cases
this directory already does not exist when Emacs is installed and used.
Why? Consider the following cases:

   1. Emacs sources were relocated/deleted after build;
   2. Emacs is being built on some build machine and then distributed as a
   prebuilt package to end users (the most common and convenient way to obtain
   Emacs for users today on all platforms), i.e. that path does not even exist
   on users' machines.

To summarize:

You cannot count on them to still be there!


I find this default annoying and I believe it can be improved. Here is the
solution that I see.

For the installation phase we add the *optional* "--install-sources" flag
which would trigger copying of C sources under "${etcdir}/../src" (which in
fact is "<datadir>/emacs/VERSION/src"). We also apply the attached patch.

Looking forward to your feedback. Thanks in advance.

Kind regards,
Alexander

[-- Attachment #1.2: Type: text/html, Size: 2545 bytes --]

[-- Attachment #2: lread.c.diff --]
[-- Type: text/plain, Size: 506 bytes --]

--- src/lread.c.orig	2014-11-04 20:29:22.129549000 +0100
+++ src/lread.c	2014-11-04 22:33:07.346414100 +0100
@@ -4351,6 +4351,12 @@
             } /* Vinstallation_directory != Vsource_directory */
 
         } /* if Vinstallation_directory */
+      else
+        {
+          Vsource_directory
+            = Fexpand_file_name (build_string ("../"),
+                                 Fcar (decode_env_path (0, PATH_DATA, 0)));
+        }
     }
   else                          /* !initialized */
     {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Correct Path to Emacs C Sources after Installation
  2014-11-04 21:43 Correct Path to Emacs C Sources after Installation Alexander Shukaev
@ 2014-11-05  1:39 ` Stephen J. Turnbull
  2014-11-05 12:59   ` Alexander Shukaev
  2014-11-05 15:56 ` Eli Zaretskii
  1 sibling, 1 reply; 6+ messages in thread
From: Stephen J. Turnbull @ 2014-11-05  1:39 UTC (permalink / raw)
  To: Alexander Shukaev; +Cc: emacs-devel

Alexander Shukaev writes:

 > For the installation phase we add the *optional*
 > "--install-sources" flag which would trigger copying of C sources
 > under "${etcdir}/../src" (which in fact is
 > "<datadir>/emacs/VERSION/src"). We also apply the attached patch.

A technical comment:  Why not simply set source-directory in
site-start (or default.el if Emacs doesn't have site-start)?

Where do you add the optional flag?  In configure?  If so, you're
likely to overwrite the sources on future installations due to caching
of options in config.status, which may or may not be the right thing
(in particular, it could confuse make).  Or would it be a make target?

Even if you did install, the sources won't be guaranteed to be there
(although they're more likely to exist).  I see one usability problem
with the location you chose: for most users, it's likely to be
read-only, and if not, you have a race condition if different users
have access.

A suggestion: add an option to install not a copy of the C sources,
but rather a clone of the repo.  Note that if the complete version
string includes the revid, you can always recover the exact sources
used to build the executable.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Correct Path to Emacs C Sources after Installation
  2014-11-05  1:39 ` Stephen J. Turnbull
@ 2014-11-05 12:59   ` Alexander Shukaev
  0 siblings, 0 replies; 6+ messages in thread
From: Alexander Shukaev @ 2014-11-05 12:59 UTC (permalink / raw)
  To: emacs-devel, Stephen J. Turnbull

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

>
> A technical comment:  Why not simply set source-directory in
> site-start (or default.el if Emacs doesn't have site-start)?


Considering this a low-level default like "invocation-directory" or
"installation-directory" which are both set during this initialization
phase one the C side. Therefore, for consistency reasons this is a good
place to set the default for "source-directory". Furthermore, there is a
comment in this file like:

  /* Vsource_directory was initialized in init_lread.  */


which was there even before my patch, so I believe I'm on the right track.

Where do you add the optional flag?  In configure?  If so, you're
> likely to overwrite the sources on future installations due to caching
> of options in config.status, which may or may not be the right thing
> (in particular, it could confuse make).  Or would it be a make target?


I'm good at Make, but I'm not a huge expert with Autotools, so I was hoping
that somebody could suggest the best way of introducing such an option
whether it be a Make target or an Autoconf flag. Can anybody lend a hand
here?

Even if you did install, the sources won't be guaranteed to be there
> (although they're more likely to exist).  I see one usability problem
> with the location you chose: for most users, it's likely to be
> read-only, and if not, you have a race condition if different users
> have access.


Usually it is read-only. Well, but if not then it's the problem of users
and their systems, isn't it? We could say that other data files which are
written there during Emacs installation (such as help files) are subject to
race condition too. What now? I think this issue is a bit exaggerated.

A suggestion: add an option to install not a copy of the C sources,
> but rather a clone of the repo.  Note that if the complete version
> string includes the revid, you can always recover the exact sources
> used to build the executable.


Feels like an overkill to clone the whole repository with its huge history.
What if user has no internet connection?

[-- Attachment #2: Type: text/html, Size: 3299 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Correct Path to Emacs C Sources after Installation
  2014-11-04 21:43 Correct Path to Emacs C Sources after Installation Alexander Shukaev
  2014-11-05  1:39 ` Stephen J. Turnbull
@ 2014-11-05 15:56 ` Eli Zaretskii
  2014-11-05 16:23   ` Alexander Shukaev
  1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2014-11-05 15:56 UTC (permalink / raw)
  To: Alexander Shukaev; +Cc: emacs-devel

> Date: Tue, 4 Nov 2014 22:43:08 +0100
> From: Alexander Shukaev <haroogan@gmail.com>
> 
> For the installation phase we add the *optional* "--install-sources" flag which
> would trigger copying of C sources under "${etcdir}/../src" (which in fact is
> "<datadir>/emacs/VERSION/src"). We also apply the attached patch.
> 
> Looking forward to your feedback. Thanks in advance.
> 
> Kind regards,
> Alexander
> 
> --- src/lread.c.orig	2014-11-04 20:29:22.129549000 +0100
> +++ src/lread.c	2014-11-04 22:33:07.346414100 +0100
> @@ -4351,6 +4351,12 @@
>              } /* Vinstallation_directory != Vsource_directory */
>  
>          } /* if Vinstallation_directory */
> +      else
> +        {
> +          Vsource_directory
> +            = Fexpand_file_name (build_string ("../"),
> +                                 Fcar (decode_env_path (0, PATH_DATA, 0)));
> +        }
>      }
>    else                          /* !initialized */
>      {

Unless I'm missing something, this unconditionally sets the value of
source-directory to /usr/local/share/emacs/VERSION/etc/ in the
installed binary, is that right?  If so, I don't think this can be
acceptable, because it disallows the current practice of leaving the
sources where Emacs was built.

As I said earlier, I like Stefan's suggestion of changing the users of
this variable, so that they could look in alternative places if the C
sources in source-directory are not accessible.  After all, it might
well be that the sources are being removed while Emacs is running, so
a one-time computation might still cause failure.

There are only 2 users of source-directory now: find-func.el and
check-declare.el.  All you need is to teach them to look in
data-directory if the files cannot be found in source-directory.  I
think this will be much easier, and perhaps should also use some
defcustom that users could customize (e.g., Emacs could look in a list
of directories, not just one particular place).

Thanks.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Correct Path to Emacs C Sources after Installation
  2014-11-05 15:56 ` Eli Zaretskii
@ 2014-11-05 16:23   ` Alexander Shukaev
  2014-11-05 16:53     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Alexander Shukaev @ 2014-11-05 16:23 UTC (permalink / raw)
  To: emacs-devel, Eli Zaretskii

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

>
> Unless I'm missing something, this unconditionally sets the value of
> source-directory to /usr/local/share/emacs/VERSION/etc/ in the
> installed binary, is that right?  If so, I don't think this can be
> acceptable, because it disallows the current practice of leaving the
> sources where Emacs was built.
>

It sets the default to "/usr/local/share/emacs/VERSION/" only when Emacs is
installed. During the build stage the default of

Vsource_directory
    = Fexpand_file_name (build_string ("../"),
 Fcar (decode_env_path (0, PATH_DUMPLOADSEARCH, 0)));


remains. So that sources would end up under
"/usr/local/share/emacs/VERSION/src".

As I said earlier, I like Stefan's suggestion of changing the users of
> this variable, so that they could look in alternative places if the C
> sources in source-directory are not accessible.


They can tweak it by changing this variable manually to point to those
alternative places. Here we're considering only reasonable default. By the
way this default complies with how "load-path" is initialized.


> After all, it might
> well be that the sources are being removed while Emacs is running, so
> a one-time computation might still cause failure.
>

After all, it might be that "lisp" and "site-lisp" sources are removed too
and then Emacs would fail too. It's not about 1 time computation here. If
ones moves sources, then one is responsible to change the value of the
variable manually through Emacs Lisp. Once again the discussion is about
default. Anyway what's your point here?


> There are only 2 users of source-directory now: find-func.el and
> check-declare.el.  All you need is to teach them to look in
> data-directory if the files cannot be found in source-directory.  I
> think this will be much easier, and perhaps should also use some
> defcustom that users could customize (e.g., Emacs could look in a list
> of directories, not just one particular place).
>

This haven't been done before, i.e. there was only a hard coded path to
sources during build and nobody cared. Why would we need to do that now and
why is this easier than the proposed patch, could you elaborate? You don't
like C code changes?

Thanks.
>

Glad to help.

[-- Attachment #2: Type: text/html, Size: 3949 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Correct Path to Emacs C Sources after Installation
  2014-11-05 16:23   ` Alexander Shukaev
@ 2014-11-05 16:53     ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2014-11-05 16:53 UTC (permalink / raw)
  To: Alexander Shukaev; +Cc: emacs-devel

> Date: Wed, 5 Nov 2014 17:23:35 +0100
> From: Alexander Shukaev <haroogan@gmail.com>
> 
>     Unless I'm missing something, this unconditionally sets the value of
>     source-directory to /usr/local/share/emacs/VERSION/etc/ in the
>     installed binary, is that right? If so, I don't think this can be
>     acceptable, because it disallows the current practice of leaving the
>     sources where Emacs was built.
> 
> It sets the default to "/usr/local/share/emacs/VERSION/" only when Emacs is
> installed.

What happens in the installed Emacs _is_ the focus in this discussion.

Like I said, I don't think it's acceptable to break the current
behavior, because some people (like myself, for example) leave the
source tree of an installed Emacs around for a very long time, in the
same place where Emacs was compiled.  We must continue supporting this
use case.  There's no reason to stop supporting it.

> So that sources would end up under "/usr/local/share/emacs/VERSION/src".

They will not be found there, unless Emacs is configured with the
(proposed) option of installing the sources.  So by disabling the
current behavior, you break an existing feature, which I don't think
is acceptable, or even justified.

>     As I said earlier, I like Stefan's suggestion of changing the users of
>     this variable, so that they could look in alternative places if the C
>     sources in source-directory are not accessible.
> 
> They can tweak it by changing this variable manually to point to those
> alternative places.

Who are "they"?  By "users" I meant here the code that uses this
variable, I didn't mean people.

What I'm saying is that instead of changing the value of
source-directory, make the functions which use it look in other places
if the file they look for is not found under source-directory.  This
way, the existing behavior is preserved, and your optional behavior
becomes possible, _if_ the sources aren't in the place where Emacs was
built.  I think this gives you the best of both worlds.

> Here we're considering only reasonable default. By the way
> this default complies with how "load-path" is initialized.

The way load-path is used and its purpose are very different.

>     After all, it might
>     well be that the sources are being removed while Emacs is running, so
>     a one-time computation might still cause failure.
>     
> 
> After all, it might be that "lisp" and "site-lisp" sources are removed too and
> then Emacs would fail too. It's not about 1 time computation here. If ones
> moves sources, then one is responsible to change the value of the variable
> manually through Emacs Lisp. Once again the discussion is about default. Anyway
> what's your point here?

My point is that moving the solution to the functions that use this
variable solves your problem, and in addition (a) doesn't break
existing behavior, and (b) can support the use case where Someone(TM)
removes the source tree while SomeoneElse(TM) has an Emacs session
running on the same system.  The latter might not be a big deal, but
it's an advantage that you get for free.

>     There are only 2 users of source-directory now: find-func.el and
>     check-declare.el. All you need is to teach them to look in
>     data-directory if the files cannot be found in source-directory. I
>     think this will be much easier, and perhaps should also use some
>     defcustom that users could customize (e.g., Emacs could look in a list
>     of directories, not just one particular place).
>     
> 
> This haven't been done before, i.e. there was only a hard coded path to sources
> during build and nobody cared. Why would we need to do that now and why is this
> easier than the proposed patch, could you elaborate?

See above: it preserves the existing behavior, it doesn't change the
semantics of source-directory, and it solves your problem in a cleaner
way.

Put it another way: your original problem was that Emacs was unable to
show the implementation of Emacs primitives when the original source
directory was unavailable for some reason.  I say: solve that problem,
where it happens, i.e. in find-func.el, which looks for a C file to
show the function.

> You don't like C code changes?

It is true that we generally prefer to add features in Lisp rather
than in C.  But that's not the main issue in this case, at least not
IMO.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-11-05 16:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-04 21:43 Correct Path to Emacs C Sources after Installation Alexander Shukaev
2014-11-05  1:39 ` Stephen J. Turnbull
2014-11-05 12:59   ` Alexander Shukaev
2014-11-05 15:56 ` Eli Zaretskii
2014-11-05 16:23   ` Alexander Shukaev
2014-11-05 16:53     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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).