bug#36828: 27.0.50; Uninstalled emacs shows installed documentation

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Cc: stepnem@gmail.com,  36828@debbugs.gnu.org
> Date: Mon, 28 Oct 2019 22:45:44 +0100
> 
> Indeed, this simple patch does the trick:
> 
> modified   src/callproc.c
> @@ -1567,7 +1567,7 @@ init_callproc (void)
>  
>        tem = Fexpand_file_name (build_string ("NEWS"), Vdata_directory);
>        if (!NILP (Fequal (srcdir, Vinvocation_directory))
> -       || NILP (Ffile_exists_p (tem)))
> +       || NILP (Ffile_exists_p (tem)) || !NILP (Vinstallation_directory))
>       {
>         Lisp_Object newdir;
>         newdir = Fexpand_file_name (build_string ("../etc/"), lispdir);
> 
> But then we can also get rid of
> 
> !NILP (Fequal (srcdir, Vinvocation_directory))
> 
> because that condition should always be true when Vinvocation_directory
> is non-nil, right?

We could, but I'd like to leave it for now, with a FIXME comment
saying whether this condition is necessary.  The reason is that I'm
not sure whether there aren't any additional use cases which this
condition allows.

> Actually, we can remove the `if' altogether because if we enter
> 
>   if (data_dir == 0)
>     {
> 
> then we know that this is a non-installed emacs and even if we end
> assigning a non-existent directory to Vdata_directory within that `if'
> (for reasons I can't imagine) it is not a regression: Vdata_directory
> would contain a wrong directory anyways.

Sorry, I don't follow: data_dir being NULL just means EMACSDATA is not
defined in the environment.  If EMACSDATA is defined, its value tramps
everything else, and we still want to support that.  How is EMACSDATA
related to the issue at hand? it's a completely separate use case.

> All that function looks unnecessarily complex to me. It comes from a
> long time ago and it smells like it didn't adapt to the availability of
> new variables. I'm a bit reluctant to adding yet another condition and
> cause future hackers to scratch their heads trying to figure out whas is
> about with the redundant stuff.

I don't think I agree.  That function calculates values of several
variables:

  exec-path
  exec-directory
  doc-directory
  data-directory
  shell-file-name
  shared-game-score-directory

It also emits warnings if exec-directory or data-directory are
inaccessible.  Each of the above variables takes just a few lines of
code to compute, so I don't think I see the complexity, let alone an
unnecessary one.  Can you elaborate on what looks redundant there, and
how would you suggest to use the available new variables?

> Said that, if you don't want me to touch other parts of the function and
> just add the new condition, ok. Later I'll try to adapt C-h i to show
> the non-installed `dir' file.

Yes, let's for now make a simple and safe change.

Thanks.