Finalizing 'inhibit-automatic-native-compilation'

Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

The variable 'inhibit-automatic-native-compilation' was introduced in
last October, as result of various discussions on this list regarding
the need to disable async native-compilation in some situations.
Since its introduction was met with some opposition, in particular
from Andrea, the final decision about whether this variable should
stay in Emacs was deferred, with the purpose of collecting more data
points and user experience.

With the pretest of Emacs 29 just around the corner, I think now is
the time to make that final decision.  With that in mind, I will first
summarize the changes which this variable introduced into Emacs, and
then ask for opinions regarding some of its aspects.

This variable was introduced (under the name
'inhibit-native-compilation') in commit 5fec9182db.  In that commit:

  . comp-trampoline-compile was changed to avoid writing the
    trampolines to the eln-cache if this variable is non-nil (instead,
    it writes the trampolines to a temporary-file directory, and
    attempts to delete them after that, which on Posix platforms will
    cause their deletion when Emacs which produced them exits, and on
    Windows currently fails).
  . In normal-top-level, we set this variable if the environment
    variable EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION is set.
  . In several places this variable either replaces
    native-comp-deferred-compilation or has the same effect as the
    latter (modulo the opposite meaning of nil/t value), therefore
    disabling async compilation of *.el files that Emacs loads for
    which there are no corresponding *.eln files.

Here are the questions I think we want to be answered to finalize the
implementation and effects of 'inhibit-automatic-native-compilation':

  . Do people actually use 'inhibit-automatic-native-compilation' or
    the corresponding environment variable?  If so, for what reasons,
    and why tweaking 'native-comp-eln-load-path' to direct the *.eln
    files to some other place, including the temporary-file directory,
    was not enough?

  . What do we want to do about compiling trampolines when
    native-compilation is disabled?

    Currently, 'inhibit-automatic-native-compilation' doesn't really
    disable compilation of trampolines, it just causes them to be
    written to a temporary location, and hopefully deleted when the
    session ends.  This means that, for example, if the user has a
    broken installation of GCC and Binutils, loading Lisp code that
    uses advices will signal errors when Emacs compiles the
    trampolines (because the compilation fails).
    The alternative is to disable compilation of trampolines, but that
    has a downside that advices for primitives will not have effect.
    It is not clear to me which alternative is better, as they both
    have failure modes.  Note that the build process was augmented so
    you can say, after building Emacs as usual

       make trampolines

    and have all the trampolines for the built-in functions
    (a.k.a. "primitives") compiled and written to the build tree, from
    where they will be installed by "make install", thus minimizing
    potential problems with the need to build trampolines when running
    the installed Emacs.

    If we leave the current build-trampolines-then-delete-them
    machinery intact, is it a problem to have those trampolines not
    deleted on MS-Windows?  They will then be left in the temporary
    directory, and are supposed to be removed by system cleanup
    processes, or maybe remain there forever.  Or do we have to add a
    mechanism for deleting them at exit?

  . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
    environment variable?

    I dislike having environment variables that alter Emacs behavior,
    because environment variables are inherited by sub-processes.  So
    having this variable set runs the risk of affecting all the
    sub-processes, something that could be unexpected and is not easy
    to prevent.  We had similar problems with EMACSLOADPATH, for
    example, which is especially painful when building another version
    of Emacs from a shell buffer inside Emacs.  This causes some
    hard-to-debug problems.
    So if this environment variable is largely unused, maybe we should
    remove it, even if we keep 'inhibit-automatic-native-compilation'.

Relevant discussions, for those who want to refresh their memories:

  https://lists.gnu.org/archive/html/emacs-devel/2022-09/msg01911.html
  https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-10/msg01126.html

Please respond soon, so as not to delay the pretest of Emacs 29.

TIA

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

> The variable 'inhibit-automatic-native-compilation' was introduced in
> last October, as result of various discussions on this list regarding
> the need to disable async native-compilation in some situations.
> Since its introduction was met with some opposition, in particular
> from Andrea, the final decision about whether this variable should
> stay in Emacs was deferred, with the purpose of collecting more data
> points and user experience.
>
> With the pretest of Emacs 29 just around the corner, I think now is
> the time to make that final decision.  With that in mind, I will first
> summarize the changes which this variable introduced into Emacs, and
> then ask for opinions regarding some of its aspects.
>
> This variable was introduced (under the name
> 'inhibit-native-compilation') in commit 5fec9182db.  In that commit:
>
>   . comp-trampoline-compile was changed to avoid writing the
>     trampolines to the eln-cache if this variable is non-nil (instead,
>     it writes the trampolines to a temporary-file directory, and
>     attempts to delete them after that, which on Posix platforms will
>     cause their deletion when Emacs which produced them exits, and on
>     Windows currently fails).
>   . In normal-top-level, we set this variable if the environment
>     variable EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION is set.
>   . In several places this variable either replaces
>     native-comp-deferred-compilation or has the same effect as the
>     latter (modulo the opposite meaning of nil/t value), therefore
>     disabling async compilation of *.el files that Emacs loads for
>     which there are no corresponding *.eln files.
>
> Here are the questions I think we want to be answered to finalize the
> implementation and effects of 'inhibit-automatic-native-compilation':
>
>   . Do people actually use 'inhibit-automatic-native-compilation' or
>     the corresponding environment variable?  If so, for what reasons,
>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>     files to some other place, including the temporary-file directory,
>     was not enough?
>
>   . What do we want to do about compiling trampolines when
>     native-compilation is disabled?
>
>     Currently, 'inhibit-automatic-native-compilation' doesn't really
>     disable compilation of trampolines, it just causes them to be
>     written to a temporary location, and hopefully deleted when the
>     session ends.  This means that, for example, if the user has a
>     broken installation of GCC and Binutils, loading Lisp code that
>     uses advices will signal errors when Emacs compiles the
>     trampolines (because the compilation fails).

I believe that's the biggest issue here.  As you have very well
explained 'inhibit-automatic-native-compilation' simply doesn't really
inhibit native compilation.  This is source of user confusion and
issues.

As we can't provide a good command that really disables native
compilation it's better for me to have the two original knobs, one for
jit compilation and one for trampolines.  I personally strongly prefer
no abstraction over a broken one.

I'm very curious about reading the anwers about if
'inhibit-automatic-native-compilation' is used and particularly why.

The only functonality that 'inhibit-automatic-native-compilation'
introduced is to leave the eln cache clean by trampoline files.  IMO if
targetting a tmp directory is not a sufficiently good option we probably
should have controlled this clean-up mechanism with a new value in
`comp-enable-subr-trampolines'.

Best Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

> Here are the questions I think we want to be answered to finalize the
> implementation and effects of 'inhibit-automatic-native-compilation':
>
>   . Do people actually use 'inhibit-automatic-native-compilation' or
>     the corresponding environment variable?  If so, for what reasons,
>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>     files to some other place, including the temporary-file directory,
>     was not enough?

I do.

When running Emacs ada-mode tests, I spawn lots of Emacsen. If each one
tries to native compile stuff, it just slows down the tests.

I could try to ensure the stuff is native compiled first, but that's a
maintenance burden.

>   . What do we want to do about compiling trampolines when
>     native-compilation is disabled?

What is the name of the "temporary directory" that is not deleted on
Windows? I can check it see if is created when I run my tests.

>   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
>     environment variable?

I don't need this for my use case.

-- 
-- Stephe

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Please respond soon, so as not to delay the pretest of Emacs 29.

AFAICT the main issue is how to handle the case where the home directory
is not writable (or doesn't exist).  I think the Debian packaging needs
mostly stem from this.

For "normal" compilation, we should just make sure that in the absence
of a writable eln cache Emacs still behaves correctly, i.e. it should
just load the `.elc` file and move on (without native-compiling it).

For trampolines, in the short term we should probably add a fallback to
use an eln-cache in the `temporary-file-directory` (i.e. use the code
we currently use when `inhibit-automatic-native-compilation` is non-nil).

I think together this should let Debian get the behavior they want, by
setting HOME to /doesntexist.  And that should make both
`inhibit-automatic-native-compilation` and
`EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION` unnecessary.

There are still some problems after that:

- HOME is ignored when the user is root (Emacs uses something like
  ~root/ or ~$LOGNAME/ instead).  I think we should throw away this
  (mis)feature, but I understand some people like it when they use `su`.
  Maybe a workaround is to tweak that (mis)feature so HOME is only
  ignored if it points a directory that belongs to another user, or so
  HOME is not ignored when it points nowhere.

- There's of course the problem of needing to compile trampolines when
  GCC is absent.  The current `make trampolines` kind of solves that,
  but it's ridiculously inefficient (the code for the trampolines is
  trivially small, compared to the size of the generated `.eln`
  trampoline files).  Maybe a better solution is to precompile the
  trampoline when we generate a `.eln` that may need a trampoline, so
  that the (small) trampoline code is directly included in the normal
  `.eln` file.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> Here are the questions I think we want to be answered to finalize the
>> implementation and effects of 'inhibit-automatic-native-compilation':
>>
>>   . Do people actually use 'inhibit-automatic-native-compilation' or
>>     the corresponding environment variable?  If so, for what reasons,
>>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>>     files to some other place, including the temporary-file directory,
>>     was not enough?
>
> I do.
>
> When running Emacs ada-mode tests, I spawn lots of Emacsen. If each one
> tries to native compile stuff, it just slows down the tests.

Any reason you use `inhibit-automatic-native-compilation` rather than
`native-comp-deferred-compilation`?


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Here are the questions I think we want to be answered to finalize the
>>> implementation and effects of 'inhibit-automatic-native-compilation':
>>>
>>>   . Do people actually use 'inhibit-automatic-native-compilation' or
>>>     the corresponding environment variable?  If so, for what reasons,
>>>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>>>     files to some other place, including the temporary-file directory,
>>>     was not enough?
>>
>> I do.
>>
>> When running Emacs ada-mode tests, I spawn lots of Emacsen. If each one
>> tries to native compile stuff, it just slows down the tests.
>
> Any reason you use `inhibit-automatic-native-compilation` rather than
> `native-comp-deferred-compilation`?

Hmm. That is the variable I'm currently using, but I thought it was
deprecated. In addition, it doesn't seem to be declared:

M-x describe-variable only finds
native-comp-deferred-compilation-deny-list.

Maybe native-comp-deferred-compilation is only declared in a let-binding
somewhere?

So if native-comp-deferred-compilation is fully supported in it's
current form, I'm happy.

-- 
-- Stephe

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Cc: emacs-devel@gnu.org,  Andrea Corallo <akrl@sdf.org>,  Lars Ingebrigtsen
>  <larsi@gnus.org>,  Stefan Monnier <monnier@iro.umontreal.ca>,  Rob
>  Browning <rlb@defaultvalue.org>
> Date: Fri, 27 Jan 2023 15:11:18 -0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Here are the questions I think we want to be answered to finalize the
> > implementation and effects of 'inhibit-automatic-native-compilation':
> >
> >   . Do people actually use 'inhibit-automatic-native-compilation' or
> >     the corresponding environment variable?  If so, for what reasons,
> >     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
> >     files to some other place, including the temporary-file directory,
> >     was not enough?
> 
> I do.
> 
> When running Emacs ada-mode tests, I spawn lots of Emacsen. If each one
> tries to native compile stuff, it just slows down the tests.
> 
> I could try to ensure the stuff is native compiled first, but that's a
> maintenance burden.

Disabling async native-compilation was supported before we introduced
this variable (via native-comp-deferred-compilation) and will continue
being supported, for several valid reasons.  Removing that capability
is not on the table.

I understand that if inhibiting native-compilation would still produce
trampolines in the rare cases where they are needed, it will not do
any significant harm to your use case, is that right?

> >   . What do we want to do about compiling trampolines when
> >     native-compilation is disabled?
> 
> What is the name of the "temporary directory" that is not deleted on
> Windows? I can check it see if is created when I run my tests.

It's the system temporary directory, the one that's the value of
temporary-file-directory.  And it isn't the directory that is not
deleted, it's the comp-lambda-*.eln files (and maybe also *.eln files
under different names, like *--trampoline-*.eln) in that directory.

> >   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
> >     environment variable?
> 
> I don't need this for my use case.

Thanks for the feedback.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org,  Andrea Corallo
>  <akrl@sdf.org>,  Lars Ingebrigtsen <larsi@gnus.org>,  Rob Browning
>  <rlb@defaultvalue.org>
> Date: Fri, 27 Jan 2023 16:32:46 -0800
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Any reason you use `inhibit-automatic-native-compilation` rather than
> > `native-comp-deferred-compilation`?
> 
> Hmm. That is the variable I'm currently using, but I thought it was
> deprecated. In addition, it doesn't seem to be declared:
> 
> M-x describe-variable only finds
> native-comp-deferred-compilation-deny-list.

describe-variable only knows about user options, not about all the
variables.  Try "M-: native-comp-def TAB RET".

> Maybe native-comp-deferred-compilation is only declared in a let-binding
> somewhere?

It's a variable defined in comp.c, but it is not a defcustom.

> So if native-comp-deferred-compilation is fully supported in it's
> current form, I'm happy.

I don't think we will remove inhibit-automatic-native-compilation.  It
was in the source based for long enough to stay.  So I think we will
keep it, but I hope we can change its semantics to not generate the
problems it currently causes.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  Andrea Corallo <akrl@sdf.org>,  Lars Ingebrigtsen
>  <larsi@gnus.org>,  Rob Browning <rlb@defaultvalue.org>
> Date: Fri, 27 Jan 2023 18:57:20 -0500
> 
> AFAICT the main issue is how to handle the case where the home directory
> is not writable (or doesn't exist).  I think the Debian packaging needs
> mostly stem from this.

The Debian use case has other solutions, which don't require disabling
native-compilation.  I never understood why they insisted on
disabling, instead of, say, redirecting the eln-cache to some suitable
place, whether a throwaway directory like /tmp or some other
directory.  I hope maybe Rob will respond now and we could finally
understand why they want to disable the compilation, or they will
understand why disabling is not needed.  Or maybe they do use the
redirect-eln-cache solution after all, and we just don't know?

In any case, inhibit-automatic-native-compilation, as implemented now,
doesn't actually disable compiling the trampolines, it just writes
them to /tmp.  So if that is okay with Debian, why isn't it okay to
tweak native-comp-eln-load-path to do the same to begin with, I
wonder?

> For "normal" compilation, we should just make sure that in the absence
> of a writable eln cache Emacs still behaves correctly, i.e. it should
> just load the `.elc` file and move on (without native-compiling it).

That is already happening, and either native-comp-deferred-compilation
or inhibit-automatic-native-compilation will disable the following
async compilation.  We had this through the entire Emacs 28 release
cycle, and the only new issue that came up was with the trampolines
that are still being compiled and written to the eln-cache.

> For trampolines, in the short term we should probably add a fallback to
> use an eln-cache in the `temporary-file-directory` (i.e. use the code
> we currently use when `inhibit-automatic-native-compilation` is non-nil).

This have two problems:

  . it cleans up only on Posix platforms
  . it is fundamentally broken, since it doesn't actually prevent
    native-compilation, so if the system cannot natively compile at
    all (e.g., if Binutils are not available), this will still signal
    errors

The only solution we could come up with for the latter part is not to
compile trampolines at all, and it has another adverse effect: advices
for primitives will not work.  I don't see any way around these two
problems, we could only make comp-enable-subr-trampolines a variable
that has more than just 2 boolean values, so that people could tweak
the behavior to whatever problems they have with trampolines.

> I think together this should let Debian get the behavior they want, by
> setting HOME to /doesntexist.  And that should make both
> `inhibit-automatic-native-compilation` and
> `EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION` unnecessary.

Such a solution was proposed, but for some reason Debian didn't like
it, or at least that was my impression.

And again, why should we only consider the Debian use case?

> - There's of course the problem of needing to compile trampolines when
>   GCC is absent.  The current `make trampolines` kind of solves that,
>   but it's ridiculously inefficient (the code for the trampolines is
>   trivially small, compared to the size of the generated `.eln`
>   trampoline files).

Why is that a problem? if we leave the compilation of trampolines to
run time, we will produce those same small files, right?  And if you
think about having a single .eln file with all the trampolines, that
is inefficient in another sense: we'd be wasting memory by loading
trampolines which are not actually needed.  So I'm not sure I see a
significant problem here or a significantly better solution for it.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> The Debian use case has other solutions, which don't require disabling
> native-compilation.  I never understood why they insisted on
> disabling, instead of, say, redirecting the eln-cache to some suitable
> place, whether a throwaway directory like /tmp or some other
> directory.  I hope maybe Rob will respond now and we could finally
> understand why they want to disable the compilation, or they will
> understand why disabling is not needed.  Or maybe they do use the
> redirect-eln-cache solution after all, and we just don't know?

IIUC the problem is that they need to control this "higher up in the
scripts" and have it take effect in many Emacs invocations underneath,
so using an env-var (like $HOME) is an easier solution.  Otherwise they
may need to tweak many build scripts in many different packages.

> In any case, inhibit-automatic-native-compilation, as implemented now,
> doesn't actually disable compiling the trampolines, it just writes
> them to /tmp.  So if that is okay with Debian, why isn't it okay to
> tweak native-comp-eln-load-path to do the same to begin with, I
> wonder?

Because one can be controlled via an env-var?

>> For "normal" compilation, we should just make sure that in the absence
>> of a writable eln cache Emacs still behaves correctly, i.e. it should
>> just load the `.elc` file and move on (without native-compiling it).
> That is already happening, and either native-comp-deferred-compilation
> or inhibit-automatic-native-compilation will disable the following
> async compilation.  We had this through the entire Emacs 28 release

AFAICT somewhere along the emacs-28 line, if your $HOME doesn't exist or
the eln-cache is not writable, Emacs failed with an error at startup.
I can't remember if we fixed this before or after Emacs-28.1.

> cycle, and the only new issue that came up was with the trampolines
> that are still being compiled and written to the eln-cache.

Indeed, and at that point if the eln-cache is not writable then we fail,
which is what still need(s|ed) to be fixed.

>> For trampolines, in the short term we should probably add a fallback to
>> use an eln-cache in the `temporary-file-directory` (i.e. use the code
>> we currently use when `inhibit-automatic-native-compilation` is non-nil).
>
> This have two problems:
>
>   . it cleans up only on Posix platforms

Yup.  I hope it's "good enough" for our immediate needs, tho.

>   . it is fundamentally broken, since it doesn't actually prevent
>     native-compilation, so if the system cannot natively compile at
>     all (e.g., if Binutils are not available), this will still signal
>     errors

Right, that's the more general issue of not being able to generate the
trampolines, for which the only solution is to find a way to pregenerate
the trampolines.  We have not tried that hard enough yet, but I think
it's too late to do that for Emacs-29.

> The only solution we could come up with for the latter part is not to
> compile trampolines at all, and it has another adverse effect: advices
> for primitives will not work.

Indeed, it's not a solution.

>> I think together this should let Debian get the behavior they want, by
>> setting HOME to /doesntexist.  And that should make both
>> `inhibit-automatic-native-compilation` and
>> `EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION` unnecessary.
> Such a solution was proposed, but for some reason Debian didn't like
> it, or at least that was my impression.

My understanding is that we didn't get a clear answer.  Until we have
such a clear answer I'd assume that it *does* solve their problem.

> And again, why should we only consider the Debian use case?

AFAIK it's the only request we've had.  All others were satisfied with
`native-comp-deferred-compilation` (except for the fact that it's not
sufficiently documented/discoverable, maybe).

>> - There's of course the problem of needing to compile trampolines when
>>   GCC is absent.  The current `make trampolines` kind of solves that,
>>   but it's ridiculously inefficient (the code for the trampolines is
>>   trivially small, compared to the size of the generated `.eln`
>>   trampoline files).
>
> If we leave the compilation of trampolines to run time, we will
> produce those same small files, right?

No: what I'm suggesting is to pregenerate the trampolines before we know
if we'll need them (a bit like `make trampolines`), but to place them
alongside the code that might need it, so there's no additional "small"
files containing nothing but a trampoline.  Instead we add tiny chunks
of code (the trampolines) to other `.eln` files and we do it at a time
where we know we have GCC at hand.

> And if you think about having a single .eln file with all the
> trampolines, that is inefficient in another sense: we'd be wasting
> memory by loading trampolines which are not actually needed.

Actually, I don't know if that would be a problem: while the sum of the
trampoline files is large, the concatenation of all the trampolines into
a single `.eln` file should be *much* smaller.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sat, 28 Jan 2023 12:00:57 -0500
> 
> > The Debian use case has other solutions, which don't require disabling
> > native-compilation.  I never understood why they insisted on
> > disabling, instead of, say, redirecting the eln-cache to some suitable
> > place, whether a throwaway directory like /tmp or some other
> > directory.  I hope maybe Rob will respond now and we could finally
> > understand why they want to disable the compilation, or they will
> > understand why disabling is not needed.  Or maybe they do use the
> > redirect-eln-cache solution after all, and we just don't know?
> 
> IIUC the problem is that they need to control this "higher up in the
> scripts" and have it take effect in many Emacs invocations underneath,
> so using an env-var (like $HOME) is an easier solution.  Otherwise they
> may need to tweak many build scripts in many different packages.
> 
> > In any case, inhibit-automatic-native-compilation, as implemented now,
> > doesn't actually disable compiling the trampolines, it just writes
> > them to /tmp.  So if that is okay with Debian, why isn't it okay to
> > tweak native-comp-eln-load-path to do the same to begin with, I
> > wonder?
> 
> Because one can be controlled via an env-var?

Sorry, I don't buy the "easier" argument.  Distros are not users, they
can do things even if they aren't "easy".

> >> For trampolines, in the short term we should probably add a fallback to
> >> use an eln-cache in the `temporary-file-directory` (i.e. use the code
> >> we currently use when `inhibit-automatic-native-compilation` is non-nil).
> >
> > This have two problems:
> >
> >   . it cleans up only on Posix platforms
> 
> Yup.  I hope it's "good enough" for our immediate needs, tho.

Not from my POV, no.

> No: what I'm suggesting is to pregenerate the trampolines before we know
> if we'll need them (a bit like `make trampolines`), but to place them
> alongside the code that might need it, so there's no additional "small"
> files containing nothing but a trampoline.  Instead we add tiny chunks
> of code (the trampolines) to other `.eln` files and we do it at a time
> where we know we have GCC at hand.

What do you mean by "alongside"?  The code which needs trampolines is
the code which advises primitives, and that is written in C.  Where
would "alongside" be in that case?

> > And if you think about having a single .eln file with all the
> > trampolines, that is inefficient in another sense: we'd be wasting
> > memory by loading trampolines which are not actually needed.
> 
> Actually, I don't know if that would be a problem: while the sum of the
> trampoline files is large, the concatenation of all the trampolines into
> a single `.eln` file should be *much* smaller.

Smaller or not, most of it will be wasted in most sessions.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> >> For trampolines, in the short term we should probably add a fallback to
>> >> use an eln-cache in the `temporary-file-directory` (i.e. use the code
>> >> we currently use when `inhibit-automatic-native-compilation` is non-nil).
>> >
>> > This have two problems:
>> >
>> >   . it cleans up only on Posix platforms
>> 
>> Yup.  I hope it's "good enough" for our immediate needs, tho.
>
> Not from my POV, no.

I assume using Emacs with a non-writable HOME/eln-cache directory is
quite rare, and if (in addition to the that) the
`temporary-file-directory` is always the same, then those trampoline
files that we fail to delete won't keep accumulating ad-infinitum.
So the problem is hopefully rare.

And we do have a solution: pregenerate the trampolines.

>> No: what I'm suggesting is to pregenerate the trampolines before we know
>> if we'll need them (a bit like `make trampolines`), but to place them
>> alongside the code that might need it, so there's no additional "small"
>> files containing nothing but a trampoline.  Instead we add tiny chunks
>> of code (the trampolines) to other `.eln` files and we do it at a time
>> where we know we have GCC at hand.
> What do you mean by "alongside"?  The code which needs trampolines is
> the code which advises primitives, and that is written in C.  Where
> would "alongside" be in that case?

We need trampolines because we call (from .eln files) some functions via

    <functionvar> (...)

instead of looking at the function cell of a symbol, checking what kind
of Lisp_Object it holds, etc...

When an advice is installed (i.e. when the function cell of the symbol
is modified), we need to update the `functionvar` so it points to
a trampoline which does the dance of "looking at the function cell of
a symbol, checking what kind of Lisp_Object it holds, etc...".

So, we could pregenerate the trampoline and store it directly inside the
subr that's originally stored in the symbol's function cell.
So instead of (comp-subr-trampoline-install SUBR) having to generate the
trampoline, it would find it in one of the fields of the subr struct.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sat, 28 Jan 2023 12:42:08 -0500
> 
> >> >> For trampolines, in the short term we should probably add a fallback to
> >> >> use an eln-cache in the `temporary-file-directory` (i.e. use the code
> >> >> we currently use when `inhibit-automatic-native-compilation` is non-nil).
> >> >
> >> > This have two problems:
> >> >
> >> >   . it cleans up only on Posix platforms
> >> 
> >> Yup.  I hope it's "good enough" for our immediate needs, tho.
> >
> > Not from my POV, no.
> 
> I assume using Emacs with a non-writable HOME/eln-cache directory is
> quite rare, and if (in addition to the that) the
> `temporary-file-directory` is always the same, then those trampoline
> files that we fail to delete won't keep accumulating ad-infinitum.
> So the problem is hopefully rare.

People will use inhibit-automatic-native-compilation even if their
problem is not non-writable HOME directory.  So the fact that
non-writable HOME is rare tells us nothing about the frequency of
these situations.

> And we do have a solution: pregenerate the trampolines.

So you are saying that the "prevent writing trampolines" part of
inhibit-automatic-native-compilation is not needed?

> >> No: what I'm suggesting is to pregenerate the trampolines before we know
> >> if we'll need them (a bit like `make trampolines`), but to place them
> >> alongside the code that might need it, so there's no additional "small"
> >> files containing nothing but a trampoline.  Instead we add tiny chunks
> >> of code (the trampolines) to other `.eln` files and we do it at a time
> >> where we know we have GCC at hand.
> > What do you mean by "alongside"?  The code which needs trampolines is
> > the code which advises primitives, and that is written in C.  Where
> > would "alongside" be in that case?
> 
> We need trampolines because we call (from .eln files) some functions via
> 
>     <functionvar> (...)
> 
> instead of looking at the function cell of a symbol, checking what kind
> of Lisp_Object it holds, etc...
> 
> When an advice is installed (i.e. when the function cell of the symbol
> is modified), we need to update the `functionvar` so it points to
> a trampoline which does the dance of "looking at the function cell of
> a symbol, checking what kind of Lisp_Object it holds, etc...".
> 
> So, we could pregenerate the trampoline and store it directly inside the
> subr that's originally stored in the symbol's function cell.
> So instead of (comp-subr-trampoline-install SUBR) having to generate the
> trampoline, it would find it in one of the fields of the subr struct.

Thanks, but I don't see how that answers my question.  Unless by
"inside the subr" you mean we should change the implementation of
defsubr?  If so, this is definitely not for Emacs 29, and we should
discuss it separately.  My question about what to do with trampolines
was about Emacs 29.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> And we do have a solution: pregenerate the trampolines.
> So you are saying that the "prevent writing trampolines" part of
> inhibit-automatic-native-compilation is not needed?

I'm saying it's only needed temporarily, until we fix our compilation
system so trampolines are pregenerated.

>> >> No: what I'm suggesting is to pregenerate the trampolines before we know
>> >> if we'll need them (a bit like `make trampolines`), but to place them
>> >> alongside the code that might need it, so there's no additional "small"
>> >> files containing nothing but a trampoline.  Instead we add tiny chunks
>> >> of code (the trampolines) to other `.eln` files and we do it at a time
>> >> where we know we have GCC at hand.
>> > What do you mean by "alongside"?  The code which needs trampolines is
>> > the code which advises primitives, and that is written in C.  Where
>> > would "alongside" be in that case?
>> 
>> We need trampolines because we call (from .eln files) some functions via
>> 
>>     <functionvar> (...)
>> 
>> instead of looking at the function cell of a symbol, checking what kind
>> of Lisp_Object it holds, etc...
>> 
>> When an advice is installed (i.e. when the function cell of the symbol
>> is modified), we need to update the `functionvar` so it points to
>> a trampoline which does the dance of "looking at the function cell of
>> a symbol, checking what kind of Lisp_Object it holds, etc...".
>> 
>> So, we could pregenerate the trampoline and store it directly inside the
>> subr that's originally stored in the symbol's function cell.
>> So instead of (comp-subr-trampoline-install SUBR) having to generate the
>> trampoline, it would find it in one of the fields of the subr struct.
>
> Thanks, but I don't see how that answers my question.  Unless by
> "inside the subr" you mean we should change the implementation of
> defsubr?  If so, this is definitely not for Emacs 29, and we should
> discuss it separately.  My question about what to do with trampolines
> was about Emacs 29.

I'm definitely talking about a post-Emacs-29 solution here, yes.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sat, 28 Jan 2023 13:00:48 -0500
> 
> >> And we do have a solution: pregenerate the trampolines.
> > So you are saying that the "prevent writing trampolines" part of
> > inhibit-automatic-native-compilation is not needed?
> 
> I'm saying it's only needed temporarily, until we fix our compilation
> system so trampolines are pregenerated.

Can we decide that "temporarily" we will write trampolines even if
native-compilation was disabled, i.e. go back to what we had in Emacs
28?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andy Moreton]

On Sat 28 Jan 2023, Eli Zaretskii wrote:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
>> Date: Sat, 28 Jan 2023 13:00:48 -0500
>> 
>> >> And we do have a solution: pregenerate the trampolines.
>> > So you are saying that the "prevent writing trampolines" part of
>> > inhibit-automatic-native-compilation is not needed?
>> 
>> I'm saying it's only needed temporarily, until we fix our compilation
>> system so trampolines are pregenerated.
>
> Can we decide that "temporarily" we will write trampolines even if
> native-compilation was disabled, i.e. go back to what we had in Emacs
> 28?

`comp-enable-subr-trampolines' is also relevant.

In bug #60996 Windows builds currently error on delete-file for a
trampoline .eln file if inhibit-automatic-native-compilation is non-nil
(because Windows does not allow the .eln file to be deleted while it
is loaded as a DLL).

If inhibit-automatic-native-compilation is non-nil and
comp-enable-subr-trampolines is nil then the delete-file error is
avoided (at a cost of losing correct handling of advised primitives).

    AndyM

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> >> And we do have a solution: pregenerate the trampolines.
>> > So you are saying that the "prevent writing trampolines" part of
>> > inhibit-automatic-native-compilation is not needed?
>> 
>> I'm saying it's only needed temporarily, until we fix our compilation
>> system so trampolines are pregenerated.
>
> Can we decide that "temporarily" we will write trampolines even if
> native-compilation was disabled, i.e. go back to what we had in Emacs
> 28?

I don't understand the question: the context of what I'm proposing is
when there is no writable eln-cache in $HOME, and I'm proposing to solve
the problem by writing them to `temporary-file-directory`.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sat, 28 Jan 2023 17:24:55 -0500
> 
> >> >> And we do have a solution: pregenerate the trampolines.
> >> > So you are saying that the "prevent writing trampolines" part of
> >> > inhibit-automatic-native-compilation is not needed?
> >> 
> >> I'm saying it's only needed temporarily, until we fix our compilation
> >> system so trampolines are pregenerated.
> >
> > Can we decide that "temporarily" we will write trampolines even if
> > native-compilation was disabled, i.e. go back to what we had in Emacs
> > 28?
> 
> I don't understand the question: the context of what I'm proposing is
> when there is no writable eln-cache in $HOME, and I'm proposing to solve
> the problem by writing them to `temporary-file-directory`.

IMO, the "no writable eln-cache" scenario should be solved by tweaking
native-comp-eln-load-path to include a writable directory.  That
writable directory could be temporary-file-directory, or it could be
anything else, but it should be specified by the caller/user, not
pulled out of our hat behind the scenes.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Sat, 28 Jan 2023 21:41:06 +0000
> 
> On Sat 28 Jan 2023, Eli Zaretskii wrote:
> 
> > Can we decide that "temporarily" we will write trampolines even if
> > native-compilation was disabled, i.e. go back to what we had in Emacs
> > 28?
> 
> `comp-enable-subr-trampolines' is also relevant.

Yes, of course.

> In bug #60996 Windows builds currently error on delete-file for a
> trampoline .eln file if inhibit-automatic-native-compilation is non-nil
> (because Windows does not allow the .eln file to be deleted while it
> is loaded as a DLL).
> 
> If inhibit-automatic-native-compilation is non-nil and
> comp-enable-subr-trampolines is nil then the delete-file error is
> avoided (at a cost of losing correct handling of advised primitives).

Removing comp-enable-subr-trampolines is not on the table.  We need
that to allow users to start Emacs built with native-compilation when
libgccjit and/or Binutils are not available on the system where Emacs
is run, something that happens mostly on MS-Windows, but can also
happen elsewhere.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andy Moreton]

On Sun 29 Jan 2023, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Sat, 28 Jan 2023 21:41:06 +0000
>> 
>> On Sat 28 Jan 2023, Eli Zaretskii wrote:
>> 
>> > Can we decide that "temporarily" we will write trampolines even if
>> > native-compilation was disabled, i.e. go back to what we had in Emacs
>> > 28?
>> 
>> `comp-enable-subr-trampolines' is also relevant.
>
> Yes, of course.
>
>> In bug #60996 Windows builds currently error on delete-file for a
>> trampoline .eln file if inhibit-automatic-native-compilation is non-nil
>> (because Windows does not allow the .eln file to be deleted while it
>> is loaded as a DLL).
>> 
>> If inhibit-automatic-native-compilation is non-nil and
>> comp-enable-subr-trampolines is nil then the delete-file error is
>> avoided (at a cost of losing correct handling of advised primitives).
>
> Removing comp-enable-subr-trampolines is not on the table.  We need
> that to allow users to start Emacs built with native-compilation when
> libgccjit and/or Binutils are not available on the system where Emacs
> is run, something that happens mostly on MS-Windows, but can also
> happen elsewhere.

I was not suggesting removal of comp-enable-subr-trampolines, only its
use as a workaround for bug #60996, which is now fixed.

    AndyM

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> IMO, the "no writable eln-cache" scenario should be solved by tweaking
> native-comp-eln-load-path to include a writable directory.  That
> writable directory could be temporary-file-directory, or it could be
> anything else, but it should be specified by the caller/user, not
> pulled out of our hat behind the scenes.

That's where I disagree: when it comes to trampolines I think users
would be better served if we silently used an eln-cache in
`temporary-file-directory` rather than ignoring the subr's redefinition
(usually due to an advice).


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sun, 29 Jan 2023 09:58:26 -0500
> 
> > IMO, the "no writable eln-cache" scenario should be solved by tweaking
> > native-comp-eln-load-path to include a writable directory.  That
> > writable directory could be temporary-file-directory, or it could be
> > anything else, but it should be specified by the caller/user, not
> > pulled out of our hat behind the scenes.
> 
> That's where I disagree: when it comes to trampolines I think users
> would be better served if we silently used an eln-cache in
> `temporary-file-directory` rather than ignoring the subr's redefinition
> (usually due to an advice).

Why? in the Debian use case all they care about is that the file is
not written to HOME, and my proposal doesn't break that.  The
trampoline isn't needed in their scenario, so there's no reason to
generate it.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Leake <stephen_leake@stephe-leake.org>
>> Cc: emacs-devel@gnu.org,  Andrea Corallo <akrl@sdf.org>,  Lars Ingebrigtsen
>>  <larsi@gnus.org>,  Stefan Monnier <monnier@iro.umontreal.ca>,  Rob
>>  Browning <rlb@defaultvalue.org>
>> Date: Fri, 27 Jan 2023 15:11:18 -0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Here are the questions I think we want to be answered to finalize the
>> > implementation and effects of 'inhibit-automatic-native-compilation':
>> >
>> >   . Do people actually use 'inhibit-automatic-native-compilation' or
>> >     the corresponding environment variable?  If so, for what reasons,
>> >     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>> >     files to some other place, including the temporary-file directory,
>> >     was not enough?
>> 
>> I do.
>> 
>> When running Emacs ada-mode tests, I spawn lots of Emacsen. If each one
>> tries to native compile stuff, it just slows down the tests.
>> 
>> I could try to ensure the stuff is native compiled first, but that's a
>> maintenance burden.
>
> Disabling async native-compilation was supported before we introduced
> this variable (via native-comp-deferred-compilation) and will continue
> being supported, for several valid reasons.  Removing that capability
> is not on the table.

Ok, good.

> I understand that if inhibiting native-compilation would still produce
> trampolines in the rare cases where they are needed, it will not do
> any significant harm to your use case, is that right?

Right.

>> >   . What do we want to do about compiling trampolines when
>> >     native-compilation is disabled?
>> 
>> What is the name of the "temporary directory" that is not deleted on
>> Windows? I can check it see if is created when I run my tests.
>
> It's the system temporary directory, the one that's the value of
> temporary-file-directory.  And it isn't the directory that is not
> deleted, it's the comp-lambda-*.eln files (and maybe also *.eln files
> under different names, like *--trampoline-*.eln) in that directory.

On Windows: c:/Users/Stephe/AppData/Local/Temp

No *.eln files there, so apparently I'm not doing anything that requires
trampolines.

-- 
-- Stephe

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> > IMO, the "no writable eln-cache" scenario should be solved by tweaking
>> > native-comp-eln-load-path to include a writable directory.  That
>> > writable directory could be temporary-file-directory, or it could be
>> > anything else, but it should be specified by the caller/user, not
>> > pulled out of our hat behind the scenes.
>> 
>> That's where I disagree: when it comes to trampolines I think users
>> would be better served if we silently used an eln-cache in
>> `temporary-file-directory` rather than ignoring the subr's redefinition
>> (usually due to an advice).
>
> Why? in the Debian use case all they care about is that the file is
> not written to HOME, and my proposal doesn't break that.

AFAIK your proposal breaks some uses of advice.

> The trampoline isn't needed in their scenario, so there's no reason to
> generate it.

I don't know if they need trampolines, but I wouldn't be surprised if
they do on occasion.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Sun, 29 Jan 2023 21:30:41 -0500
> 
> >> > IMO, the "no writable eln-cache" scenario should be solved by tweaking
> >> > native-comp-eln-load-path to include a writable directory.  That
> >> > writable directory could be temporary-file-directory, or it could be
> >> > anything else, but it should be specified by the caller/user, not
> >> > pulled out of our hat behind the scenes.
> >> 
> >> That's where I disagree: when it comes to trampolines I think users
> >> would be better served if we silently used an eln-cache in
> >> `temporary-file-directory` rather than ignoring the subr's redefinition
> >> (usually due to an advice).
> >
> > Why? in the Debian use case all they care about is that the file is
> > not written to HOME, and my proposal doesn't break that.
> 
> AFAIK your proposal breaks some uses of advice.

Are you alluding to some uses we've heard about, or is this just a
theoretical possibility?  AFAIU, for this to be a real issue, we'd
need a use where the user (a) disables native-compilation for some
reason, but (b) still wants advices of natively-compiled code to
work.  Why would someone do something like that?

> > The trampoline isn't needed in their scenario, so there's no reason to
> > generate it.
> 
> I don't know if they need trampolines, but I wouldn't be surprised if
> they do on occasion.

We'd need them to answer that and explain.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Are you alluding to some uses we've heard about, or is this just a
> theoretical possibility?  AFAIU, for this to be a real issue, we'd
> need a use where the user (a) disables native-compilation for some
> reason, but (b) still wants advices of natively-compiled code to
> work.  Why would someone do something like that?

(a) definitely happens.
(b) is completely outside the control of the user, it's just a matter of
the user using a package which relies on such an advice.  I'd be very
surprised if this never happens.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,  rlb@defaultvalue.org
> Date: Mon, 30 Jan 2023 09:57:18 -0500
> 
> > Are you alluding to some uses we've heard about, or is this just a
> > theoretical possibility?  AFAIU, for this to be a real issue, we'd
> > need a use where the user (a) disables native-compilation for some
> > reason, but (b) still wants advices of natively-compiled code to
> > work.  Why would someone do something like that?
> 
> (a) definitely happens.
> (b) is completely outside the control of the user, it's just a matter of
> the user using a package which relies on such an advice.  I'd be very
> surprised if this never happens.

I asked why would advice be needed while compiling/loading files with
natively-compilation disabled?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> > Are you alluding to some uses we've heard about, or is this just a
>> > theoretical possibility?  AFAIU, for this to be a real issue, we'd
>> > need a use where the user (a) disables native-compilation for some
>> > reason, but (b) still wants advices of natively-compiled code to
>> > work.  Why would someone do something like that?
>> 
>> (a) definitely happens.
>> (b) is completely outside the control of the user, it's just a matter of
>> the user using a package which relies on such an advice.  I'd be very
>> surprised if this never happens.
>
> I asked why would advice be needed while compiling/loading files with
> natively-compilation disabled?

Since this is only relevant for Emacsen compiled with native-compilation
support, the natively compiled code would presumably come from the
pre-native-compiled files that are bundled/dumped with Emacs.

And then the rest of the code (the one that installs the advice, for
example) may or may not be native-compiled: that doesn't make
a difference to the need for a trampoline.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > (b) is completely outside the control of the user, it's just a matter of
  > the user using a package which relies on such an advice.  I'd be very
  > surprised if this never happens.

What sort of scenario would this be?  Should we treat this as proper
usage, or as "don't complain if it breaks"?

Native compilation or no, I think that releasing a package A that puts
advice on functions outside of A is asking to lose.  I wouldn't
suggest we try to make it impossible to do that, but we should try to
keep the code in Emacs and ELPA free of such practices.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Native compilation or no, I think that releasing a package A that puts
> advice on functions outside of A is asking to lose.

??

That's the whole point of advice.  Obviously, uses of advice should be
limited as much as possible, but we do have this feature for a reason.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Native compilation or no, I think that releasing a package A that puts
  > > advice on functions outside of A is asking to lose.

  > ??

  > That's the whole point of advice.

Not at all.  The point of advice is for _users_ to alter functions
in Emacs.  But code in Emacs should never do that.

If you want file A to modify the way file B operates, and these
are both parts of Emacs, the proper way to do it is for B to define a
hook variable, and document it.  Then A can put something on that
hook.

The reason for this is to avoid confusion that hampers debugging.
If B's behavior is altered through a hook, you will see in the code
for B that it calls a hook at a certain place.  That wil remind you
to check the hook variable's value to see if that is affecting behavior.

But if B's behavior has been altered through advice, nothing you
see will remind you to check whether B is advised.

The node Advising Named Functions in the Emacs Lisp Reference says:

      For these reasons, advice should be reserved for the cases where you
    cannot modify a function's behavior in any other way.  If it is
    possible to do the same thing via a hook, that is preferable
    (@pxref{Hooks}).  If you simply want to change what a particular key
    does, it may be better to write a new command, and remap the old
    command's key bindings to the new one (@pxref{Remapping Commands}).

      If you are writing code for release, for others to use, try to avoid
    including advice in it.  If the function you want to advise has no
    hook to do the job, please talk with the Emacs developers about adding
    a suitable hook.  Especially, Emacs's own source files should not put
    advice on functions in Emacs.  (There are currently a few exceptions
    to this convention, but we aim to correct them.)  It is generally
    cleaner to create a new hook in @code{foo}, and make @code{bar} use
    the hook, than to have @code{bar} put advice in @code{foo}.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Sun 29 Jan 2023 at 09:30PM -05, Stefan Monnier wrote:

>>> > IMO, the "no writable eln-cache" scenario should be solved by tweaking
>>> > native-comp-eln-load-path to include a writable directory.  That
>>> > writable directory could be temporary-file-directory, or it could be
>>> > anything else, but it should be specified by the caller/user, not
>>> > pulled out of our hat behind the scenes.
>>>
>>> That's where I disagree: when it comes to trampolines I think users
>>> would be better served if we silently used an eln-cache in
>>> `temporary-file-directory` rather than ignoring the subr's redefinition
>>> (usually due to an advice).
>>
>> Why? in the Debian use case all they care about is that the file is
>> not written to HOME, and my proposal doesn't break that.
>
> AFAIK your proposal breaks some uses of advice.
>
>> The trampoline isn't needed in their scenario, so there's no reason to
>> generate it.
>
> I don't know if they need trampolines, but I wouldn't be surprised if
> they do on occasion.

Some of our addon package test suites use advice in ways that mean we
need the trampolines.

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

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

Hello,

[quoting whole message for the benefit of debian-emacsen]

On Fri 27 Jan 2023 at 02:57PM +02, Eli Zaretskii wrote:

> The variable 'inhibit-automatic-native-compilation' was introduced in
> last October, as result of various discussions on this list regarding
> the need to disable async native-compilation in some situations.
> Since its introduction was met with some opposition, in particular
> from Andrea, the final decision about whether this variable should
> stay in Emacs was deferred, with the purpose of collecting more data
> points and user experience.
>
> With the pretest of Emacs 29 just around the corner, I think now is
> the time to make that final decision.  With that in mind, I will first
> summarize the changes which this variable introduced into Emacs, and
> then ask for opinions regarding some of its aspects.
>
> This variable was introduced (under the name
> 'inhibit-native-compilation') in commit 5fec9182db.  In that commit:
>
>   . comp-trampoline-compile was changed to avoid writing the
>     trampolines to the eln-cache if this variable is non-nil (instead,
>     it writes the trampolines to a temporary-file directory, and
>     attempts to delete them after that, which on Posix platforms will
>     cause their deletion when Emacs which produced them exits, and on
>     Windows currently fails).
>   . In normal-top-level, we set this variable if the environment
>     variable EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION is set.
>   . In several places this variable either replaces
>     native-comp-deferred-compilation or has the same effect as the
>     latter (modulo the opposite meaning of nil/t value), therefore
>     disabling async compilation of *.el files that Emacs loads for
>     which there are no corresponding *.eln files.
>
> Here are the questions I think we want to be answered to finalize the
> implementation and effects of 'inhibit-automatic-native-compilation':
>
>   . Do people actually use 'inhibit-automatic-native-compilation' or
>     the corresponding environment variable?  If so, for what reasons,
>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>     files to some other place, including the temporary-file directory,
>     was not enough?
>
>   . What do we want to do about compiling trampolines when
>     native-compilation is disabled?
>
>     Currently, 'inhibit-automatic-native-compilation' doesn't really
>     disable compilation of trampolines, it just causes them to be
>     written to a temporary location, and hopefully deleted when the
>     session ends.  This means that, for example, if the user has a
>     broken installation of GCC and Binutils, loading Lisp code that
>     uses advices will signal errors when Emacs compiles the
>     trampolines (because the compilation fails).
>     The alternative is to disable compilation of trampolines, but that
>     has a downside that advices for primitives will not have effect.
>     It is not clear to me which alternative is better, as they both
>     have failure modes.  Note that the build process was augmented so
>     you can say, after building Emacs as usual
>
>        make trampolines
>
>     and have all the trampolines for the built-in functions
>     (a.k.a. "primitives") compiled and written to the build tree, from
>     where they will be installed by "make install", thus minimizing
>     potential problems with the need to build trampolines when running
>     the installed Emacs.
>
>     If we leave the current build-trampolines-then-delete-them
>     machinery intact, is it a problem to have those trampolines not
>     deleted on MS-Windows?  They will then be left in the temporary
>     directory, and are supposed to be removed by system cleanup
>     processes, or maybe remain there forever.  Or do we have to add a
>     mechanism for deleting them at exit?
>
>   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
>     environment variable?
>
>     I dislike having environment variables that alter Emacs behavior,
>     because environment variables are inherited by sub-processes.  So
>     having this variable set runs the risk of affecting all the
>     sub-processes, something that could be unexpected and is not easy
>     to prevent.  We had similar problems with EMACSLOADPATH, for
>     example, which is especially painful when building another version
>     of Emacs from a shell buffer inside Emacs.  This causes some
>     hard-to-debug problems.
>     So if this environment variable is largely unused, maybe we should
>     remove it, even if we keep 'inhibit-automatic-native-compilation'.

Debian is now relying on this environment variable.  It is going into
our next stable release, which is already partially frozen, and it has
already propagated to many (probably most) of our derivatives.
We took the decision to start using the variable after Lars said it was
going to stick around and, specifically, that it was therefore okay to
backport it to Emacs 28.

Over in our own channels, we had actually been planning to add our own
version of this environment variable, before it got added upstream.
In other words, we independently came to the conclusion that a mechanism
of this nature was what we needed; as Stefan says, one reason is that we
need to handle invocations of Emacs deep within third party Makefiles.

In the shorter term, were it to be removed, we'd probably patch it back
in with a DEBIAN_something name, rather than break all our packages
again, or, rather than delaying uploading Emacs 29 to Debian.
But in the longer term, I'm sure that we would be open to alternatives,
if you really do want to remove the variable.

(Just fyi, I am now co-maintaining Emacs in Debian alongside Rob.)

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 869 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org,  akrl@sdf.org,
>   larsi@gnus.org,  rlb@defaultvalue.org
> Date: Wed, 01 Feb 2023 22:18:32 -0700
> 
> On Sun 29 Jan 2023 at 09:30PM -05, Stefan Monnier wrote:
> 
> >>> > IMO, the "no writable eln-cache" scenario should be solved by tweaking
> >>> > native-comp-eln-load-path to include a writable directory.  That
> >>> > writable directory could be temporary-file-directory, or it could be
> >>> > anything else, but it should be specified by the caller/user, not
> >>> > pulled out of our hat behind the scenes.
> >>>
> >>> That's where I disagree: when it comes to trampolines I think users
> >>> would be better served if we silently used an eln-cache in
> >>> `temporary-file-directory` rather than ignoring the subr's redefinition
> >>> (usually due to an advice).
> >>
> >> Why? in the Debian use case all they care about is that the file is
> >> not written to HOME, and my proposal doesn't break that.
> >
> > AFAIK your proposal breaks some uses of advice.
> >
> >> The trampoline isn't needed in their scenario, so there's no reason to
> >> generate it.
> >
> > I don't know if they need trampolines, but I wouldn't be surprised if
> > they do on occasion.
> 
> Some of our addon package test suites use advice in ways that mean we
> need the trampolines.

Thanks, but if this should be taken into consideration for the
purposes of this discussion and the decision to be made as result, we
need all the details you can provide about these uses.  So:

  . what kind of advice are those and which functions they advise?
  . where and under what circumstances will those advised functions be
    called, as part of your preparation of the packages?  (if the
    advised functions are only called when the end-user uses the
    package, that is not relevant to the present discussion)
  . what bad things will happen if the advice will not work (details, please)?
  . if we provide a way to specify, via comp-enable-subr-trampolines,
    the directory to which the trampolines will be written, will that
    satisfy your uses? if not, why not (details, please)?
  . why cannot you use native-comp-eln-load-path to force the
    trampolines go to a directory of your choice?
  . any other pertinent details you can provide

TIA

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: emacs-devel@gnu.org,  Andrea Corallo <akrl@sdf.org>,  Lars Ingebrigtsen
>  <larsi@gnus.org>,  Stefan Monnier <monnier@iro.umontreal.ca>,  Rob
>  Browning <rlb@defaultvalue.org>, debian-emacsen@lists.debian.org
> Date: Wed, 01 Feb 2023 22:40:48 -0700
> 
> Debian is now relying on this environment variable.  It is going into
> our next stable release, which is already partially frozen, and it has
> already propagated to many (probably most) of our derivatives.
> We took the decision to start using the variable after Lars said it was
> going to stick around and, specifically, that it was therefore okay to
> backport it to Emacs 28.
> 
> Over in our own channels, we had actually been planning to add our own
> version of this environment variable, before it got added upstream.
> In other words, we independently came to the conclusion that a mechanism
> of this nature was what we needed; as Stefan says, one reason is that we
> need to handle invocations of Emacs deep within third party Makefiles.
> 
> In the shorter term, were it to be removed, we'd probably patch it back
> in with a DEBIAN_something name, rather than break all our packages
> again, or, rather than delaying uploading Emacs 29 to Debian.
> But in the longer term, I'm sure that we would be open to alternatives,
> if you really do want to remove the variable.

It is fine by me if Debian decides to patch Emacs to solve whatever
problems you have in your work on the distribution.  That's the
prerogative of downstream packages, and that's why we have Free
Software to begin with.

What I'm interested in is hearing the _reasons_ why you decided to go
with the environment variable, because those reasons might be
important in other uses.  Was it just the convenience, or was it
something else?

> (Just fyi, I am now co-maintaining Emacs in Debian alongside Rob.)

Congrats.

Re: Finalizing 'inhibit-automatic-native-compilation'

[tomas]

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

On Thu, Feb 02, 2023 at 10:02:28AM +0200, Eli Zaretskii wrote:

[...]

> What I'm interested in is hearing the _reasons_ why you decided to go
> with the environment variable, because those reasons might be
> important in other uses.  Was it just the convenience, or was it
> something else?

I think one was put forward in the original:

  "one reason is that we need to handle invocations of Emacs deep
   within third party Makefiles."

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> Date: Thu, 2 Feb 2023 09:41:46 +0100
> From: <tomas@tuxteam.de>
> 
> > What I'm interested in is hearing the _reasons_ why you decided to go
> > with the environment variable, because those reasons might be
> > important in other uses.  Was it just the convenience, or was it
> > something else?
> 
> I think one was put forward in the original:
> 
>   "one reason is that we need to handle invocations of Emacs deep
>    within third party Makefiles."

Which is exactly why using environment variables is a bad idea, IME.
Who says the third-party packages are ready for this?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Aymeric, you investigated one of these cases in details.  Would you be
able to summarise the situation we had in Debian for Eli, please?

I don't myself have time for the next week or so.

On Thu 02 Feb 2023 at 09:55AM +02, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> 
>> On Sun 29 Jan 2023 at 09:30PM -05, Stefan Monnier wrote:
>> 
>> > I don't know if they need trampolines, but I wouldn't be surprised if
>> > they do on occasion.
>> 
>> Some of our addon package test suites use advice in ways that mean we
>> need the trampolines.
>
> Thanks, but if this should be taken into consideration for the
> purposes of this discussion and the decision to be made as result, we
> need all the details you can provide about these uses.  So:
>
>   . what kind of advice are those and which functions they advise?
>   . where and under what circumstances will those advised functions be
>     called, as part of your preparation of the packages?  (if the
>     advised functions are only called when the end-user uses the
>     package, that is not relevant to the present discussion)
>   . what bad things will happen if the advice will not work (details, please)?
>   . if we provide a way to specify, via comp-enable-subr-trampolines,
>     the directory to which the trampolines will be written, will that
>     satisfy your uses? if not, why not (details, please)?
>   . why cannot you use native-comp-eln-load-path to force the
>     trampolines go to a directory of your choice?
>   . any other pertinent details you can provide
>
> TIA

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

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

Hello,

On Thu 02 Feb 2023 at 10:02AM +02, Eli Zaretskii wrote:

> It is fine by me if Debian decides to patch Emacs to solve whatever
> problems you have in your work on the distribution.  That's the
> prerogative of downstream packages, and that's why we have Free
> Software to begin with.

Of course.  But we don't want to be maintaining a fork of Emacs.
We are all on the same side when it comes to wanting to preserve
manpower for more important tasks, right?

> What I'm interested in is hearing the _reasons_ why you decided to go
> with the environment variable, because those reasons might be
> important in other uses.  Was it just the convenience, or was it
> something else?

Without excluding the possibility of an improved solution, we did not
think that we needed an env var out of convenience, or in the manner of
a temporary fix.

I am not able to answer myself, right now, whether what has been
proposed in this thread about redirecting trampoline writes would be
sufficient for us.  Hopefully someone else can chime in.

One thing that I can say for sure is that we need Emacs not to crash
when HOME is non-existent or otherwise not writeable.
I am not completely sure whether that is the whole of the issue, but it
may well be.

>> (Just fyi, I am now co-maintaining Emacs in Debian alongside Rob.)
>
> Congrats.

Dubious :P  I just wanted to ask, please CC both me and Rob in the
future, else I might have missed this thread.

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 869 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,
>   monnier@iro.umontreal.ca,  rlb@defaultvalue.org,
>   debian-emacsen@lists.debian.org
> Date: Thu, 02 Feb 2023 09:28:39 -0700
> 
> I am not able to answer myself, right now, whether what has been
> proposed in this thread about redirecting trampoline writes would be
> sufficient for us.  Hopefully someone else can chime in.

Let's hope.  The changes I'm talking about were largely motivated by
Debian's reports, so I'd very much prefer not to overturn some of
those changes without understanding well enough what it does to Debian
and in particular making sure that Debian can still do what they need
even if some of those changes are reverted.

> One thing that I can say for sure is that we need Emacs not to crash
> when HOME is non-existent or otherwise not writeable.
> I am not completely sure whether that is the whole of the issue, but it
> may well be.

I don't think it crashes in that case.  It definitely shouldn't.  So
if you have a recipe where it does, please report it with the details.

However, that isn't directly related to the issue at hand, since I
don't expect anyone to point HOME to an invalid place as a means for
disabling native-compilation: we have much more direct means for that.

> >> (Just fyi, I am now co-maintaining Emacs in Debian alongside Rob.)
> >
> > Congrats.
> 
> Dubious :P  I just wanted to ask, please CC both me and Rob in the
> future, else I might have missed this thread.

Will do.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Liliana Marie Prikler]

Hi Eli,

Am Freitag, dem 27.01.2023 um 14:57 +0200 schrieb Eli Zaretskii:
> The variable 'inhibit-automatic-native-compilation' was introduced in
> last October, as result of various discussions on this list regarding
> the need to disable async native-compilation in some situations.
> Since its introduction was met with some opposition, in particular
> from Andrea, the final decision about whether this variable should
> stay in Emacs was deferred, with the purpose of collecting more data
> points and user experience.
> 
> With the pretest of Emacs 29 just around the corner, I think now is
> the time to make that final decision.  With that in mind, I will
> first summarize the changes which this variable introduced into
> Emacs, and then ask for opinions regarding some of its aspects.
Since I am on the Guix team for Emacs packaging, I'll try to lay out
some concerns both from the perspective of a user and a downstream
packager.  I hope I'm not too late to the party.

> This variable was introduced (under the name
> 'inhibit-native-compilation') in commit 5fec9182db.  In that commit:
> 
>   . comp-trampoline-compile was changed to avoid writing the
>     trampolines to the eln-cache if this variable is non-nil
> (instead,
>     it writes the trampolines to a temporary-file directory, and
>     attempts to delete them after that, which on Posix platforms will
>     cause their deletion when Emacs which produced them exits, and on
>     Windows currently fails).
>   . In normal-top-level, we set this variable if the environment
>     variable EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION is set.
>   . In several places this variable either replaces
>     native-comp-deferred-compilation or has the same effect as the
>     latter (modulo the opposite meaning of nil/t value), therefore
>     disabling async compilation of *.el files that Emacs loads for
>     which there are no corresponding *.eln files.
> 
> Here are the questions I think we want to be answered to finalize the
> implementation and effects of 'inhibit-automatic-native-compilation':
> 
>   . Do people actually use 'inhibit-automatic-native-compilation' or
>     the corresponding environment variable?  If so, for what reasons,
>     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>     files to some other place, including the temporary-file
> directory,
>     was not enough?
On Guix, I want my Emacs packages to either (1) already have been
natively compiled when using `guix install some-emacs-package' or (2)
to not natively compile anything and clutter my user-emacs-directory. 
Now the concern about clutter is somewhat secondary to Emacs spending
time that some other Emacs could have spent on CI.  (We don't do native
compilation on CI *yet*, but imho it would be worth doing for some
packages)

>   . What do we want to do about compiling trampolines when
>     native-compilation is disabled?
In my opinion, there should be a way to generate these trampolines
ahead of time in a known location (e.g. $package-trampolines.so) for
the emacs package $package.  If no such trampoline is found and native-
compilation is disabled, no compilation should take place.

>     Currently, 'inhibit-automatic-native-compilation' doesn't really
>     disable compilation of trampolines, it just causes them to be
>     written to a temporary location, and hopefully deleted when the
>     session ends.  This means that, for example, if the user has a
>     broken installation of GCC and Binutils, loading Lisp code that
>     uses advices will signal errors when Emacs compiles the
>     trampolines (because the compilation fails).
>     The alternative is to disable compilation of trampolines, but
> that
>     has a downside that advices for primitives will not have effect.
>     It is not clear to me which alternative is better, as they both
>     have failure modes.  Note that the build process was augmented so
>     you can say, after building Emacs as usual
> 
>        make trampolines
> 
>     and have all the trampolines for the built-in functions
>     (a.k.a. "primitives") compiled and written to the build tree,
> from
>     where they will be installed by "make install", thus minimizing
>     potential problems with the need to build trampolines when
> running
>     the installed Emacs.
> 
>     If we leave the current build-trampolines-then-delete-them
>     machinery intact, is it a problem to have those trampolines not
>     deleted on MS-Windows?  They will then be left in the temporary
>     directory, and are supposed to be removed by system cleanup
>     processes, or maybe remain there forever.  Or do we have to add a
>     mechanism for deleting them at exit?
In my opinion, the failure mode of not being able to advise native code
is an acceptable one.  If possible, I'd like to extend the "make
trampolines" approach into one that can also be applied to packages
installed via package.el/straight/...

>   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
>     environment variable?
> 
>     I dislike having environment variables that alter Emacs behavior,
>     because environment variables are inherited by sub-processes.  So
>     having this variable set runs the risk of affecting all the
>     sub-processes, something that could be unexpected and is not easy
>     to prevent.  We had similar problems with EMACSLOADPATH, for
>     example, which is especially painful when building another
> version
>     of Emacs from a shell buffer inside Emacs.  This causes some
>     hard-to-debug problems.
>     So if this environment variable is largely unused, maybe we
> should
>     remove it, even if we keep 'inhibit-automatic-native-
> compilation'.
I don't think a variable is needed necessarily.  What is needed is a
method of enabling it reliably on the command line (i.e. a switch like
--no-native-compilation would work, but so would --eval '(setq inhibit-
automatic-native-compilation t)'), and reliably as a user editing just
Emacs configuration files (in particular, early-init.el seems like the
place I would naturally place it in).


Cheers

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Liliana Marie Prikler <liliana.prikler@gmail.com>
> Cc: Andrea Corallo <akrl@sdf.org>, Lars Ingebrigtsen <larsi@gnus.org>, 
>  Stefan Monnier <monnier@iro.umontreal.ca>, Rob Browning
>  <rlb@defaultvalue.org>
> Date: Sat, 04 Feb 2023 18:48:34 +0100
> 
> Since I am on the Guix team for Emacs packaging, I'll try to lay out
> some concerns both from the perspective of a user and a downstream
> packager.  I hope I'm not too late to the party.

Thank you for your feedback.

> >   . Do people actually use 'inhibit-automatic-native-compilation' or
> >     the corresponding environment variable?  If so, for what reasons,
> >     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
> >     files to some other place, including the temporary-file
> > directory,
> >     was not enough?
> On Guix, I want my Emacs packages to either (1) already have been
> natively compiled when using `guix install some-emacs-package' or (2)
> to not natively compile anything and clutter my user-emacs-directory. 
> Now the concern about clutter is somewhat secondary to Emacs spending
> time that some other Emacs could have spent on CI.  (We don't do native
> compilation on CI *yet*, but imho it would be worth doing for some
> packages)

That's fine.  Disabling native compilation will continue be supported,
of course, for any number of reasons.

> >   . What do we want to do about compiling trampolines when
> >     native-compilation is disabled?
> In my opinion, there should be a way to generate these trampolines
> ahead of time in a known location (e.g. $package-trampolines.so) for
> the emacs package $package.  If no such trampoline is found and native-
> compilation is disabled, no compilation should take place.

Andrea, I think comp-compile-all-trampolines fits this bill?  That is,
if I load a package, then invoke that function, it will compile all
the trampolines that are not already compiled?

> >   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
> >     environment variable?
> > 
> >     I dislike having environment variables that alter Emacs behavior,
> >     because environment variables are inherited by sub-processes.  So
> >     having this variable set runs the risk of affecting all the
> >     sub-processes, something that could be unexpected and is not easy
> >     to prevent.  We had similar problems with EMACSLOADPATH, for
> >     example, which is especially painful when building another
> > version
> >     of Emacs from a shell buffer inside Emacs.  This causes some
> >     hard-to-debug problems.
> >     So if this environment variable is largely unused, maybe we
> > should
> >     remove it, even if we keep 'inhibit-automatic-native-
> > compilation'.
> I don't think a variable is needed necessarily.  What is needed is a
> method of enabling it reliably on the command line (i.e. a switch like
> --no-native-compilation would work, but so would --eval '(setq inhibit-
> automatic-native-compilation t)'), and reliably as a user editing just
> Emacs configuration files (in particular, early-init.el seems like the
> place I would naturally place it in).

We already have (and will keep supporting) a way of disabling native
compilation, and/or the compilation of trampolines, via --eval.

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Wed, Feb 1, 2023 at 12:05 AM Richard Stallman <rms@gnu.org> wrote:
> Not at all.  The point of advice is for _users_ to alter functions
> in Emacs.  But code in Emacs should never do that.
> [...]
> The node Advising Named Functions in the Emacs Lisp Reference says:
> [...]  Especially, Emacs's own source files should not put
>     advice on functions in Emacs.  (There are currently a few exceptions
>     to this convention, but we aim to correct them.)

Are those exceptions considered bugs?
Something in emacs is putting advice on call-interactively.  When I
extended the baseline dump to include essentially all libraries
included with emacs with native-compilation, it caused an infinite
(asynchronous) regress while compiling the trampoline for
call-interactively.  I wasn't aware of this until /tmp filled up.
That could also be cured by ensuring the dump file used for
asynchronous compiling is a version that excludes site-specific
libraries.  But as you noted, it's harder to debug advice than hooks.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Lynn Winebarger <owinebar@gmail.com>
> Date: Sat, 4 Feb 2023 14:55:27 -0500
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, eliz@gnu.org, emacs-devel@gnu.org
> 
> > The node Advising Named Functions in the Emacs Lisp Reference says:
> > [...]  Especially, Emacs's own source files should not put
> >     advice on functions in Emacs.  (There are currently a few exceptions
> >     to this convention, but we aim to correct them.)
> 
> Are those exceptions considered bugs?

Not necessarily.  But this should be used rarely and only if there's
no better way.

> Something in emacs is putting advice on call-interactively.  When I
> extended the baseline dump to include essentially all libraries
> included with emacs with native-compilation, it caused an infinite
> (asynchronous) regress while compiling the trampoline for
> call-interactively.  I wasn't aware of this until /tmp filled up.

Is this with Emacs 28 or Emacs 29?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Lynn Winebarger <owinebar@gmail.com>
> > Date: Sat, 4 Feb 2023 14:55:27 -0500
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, eliz@gnu.org, emacs-devel@gnu.org
> >
> > > The node Advising Named Functions in the Emacs Lisp Reference says:
> > > [...]  Especially, Emacs's own source files should not put
> > >     advice on functions in Emacs.  (There are currently a few exceptions
> > >     to this convention, but we aim to correct them.)
> >
> > Are those exceptions considered bugs?
>
> Not necessarily.  But this should be used rarely and only if there's
> no better way.
>
> > Something in emacs is putting advice on call-interactively.  When I
> > extended the baseline dump to include essentially all libraries
> > included with emacs with native-compilation, it caused an infinite
> > (asynchronous) regress while compiling the trampoline for
> > call-interactively.  I wasn't aware of this until /tmp filled up.
>
> Is this with Emacs 28 or Emacs 29?

This was with 28.1 modified to support dumping with many pre-loaded
native-compiled files.
I disposed of that experiment (at least, of all copies I had access
to), which was created on my employer's systems.
I'm working toward implementing the functionality for the dev branch,
but my first step has been to create the infrastructure for testing my
work on build.opensuse.org against different versions of libgccjit.

As far as I know, emacs still doesn't support dumping arbitrary
native-compiled libraries at compile-time.  Which is too bad, because
the dumped version is a LOT faster to startup.  As per the other
recent thread (3x increase in startup time), loading eln files seems
to be considerably slower than loading elc files at startup, but in
dumped images the difference is probably the other way around.  I've
since approximated that dump with a byte-compiled version and the user
function dump-emacs-portable, but then I have to install before-init
hooks to systematically replace occurences of the user name and home
directory in variables and customizations to make the dump usable.
It's a kludge no distro would want to use.  Even then there are a
handful of libraries that have to be kept out of the dump (using bogus
"(provide <x>)" expressions) and then explicitly loaded in an
after-init hook after removing the bogus features. And they aren't
consistent with the ones that were problems for native-compiled
dumping.  But my point is, loading that byte-compiled dump takes
somewhat longer than loading the native-compiled dump that had twice
as many pre-loaded libraries.

Emacs needs a lot more work to really support compile-time dumping of
a larger set of libraries.  There are too many instances of code
evaluated at load time that should be evaluated in a before-init or
after-init hook, that depend on variables set in C code after the dump
file is loaded.  Or run-time services like dbus.  To really support
site-specific dumping, a robust analysis identifying that code and
transforming it appropriately would have to be performed either prior
to or as part of dump-mode.  It's non-trivial.  The changes to the C
code to deal with additional objects like text properties while
purifying are (or were) trivial by comparison.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Lynn Winebarger <owinebar@gmail.com>
> Date: Sat, 4 Feb 2023 17:05:19 -0500
> Cc: rms@gnu.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > Something in emacs is putting advice on call-interactively.  When I
> > > extended the baseline dump to include essentially all libraries
> > > included with emacs with native-compilation, it caused an infinite
> > > (asynchronous) regress while compiling the trampoline for
> > > call-interactively.  I wasn't aware of this until /tmp filled up.
> >
> > Is this with Emacs 28 or Emacs 29?
> 
> This was with 28.1 modified to support dumping with many pre-loaded
> native-compiled files.

I asked because Emacs 29 adds some changes specifically intended to
solve the problem of infinite recursion in trampoline compilation.

> As far as I know, emacs still doesn't support dumping arbitrary
> native-compiled libraries at compile-time.

What problems do you see if you try?  AFAIR, if you tweak
native-comp-eln-load-path so that the *.eln files you want to dump are
directed to the ../native-lisp directory relative to where the Emacs
binary lives, then dump Emacs, loading such a dumped Emacs should
work.

Andrea, am I missing something?

> Which is too bad, because the dumped version is a LOT faster to
> startup.

Are you sure? is this with native-compilation or without it?

> As per the other recent thread (3x increase in startup time),
> loading eln files seems to be considerably slower than loading elc
> files at startup, but in dumped images the difference is probably
> the other way around.

I don't see why it would be faster when loading a dumped Emacs: the
pdumper file records only the names of the *.eln files, not their
contents, so Emacs still has to load all of the *.eln files recorded
in the pdumper file, and that takes time and doesn't change much when
the names are recorded in emacs.pdmp.

> I've
> since approximated that dump with a byte-compiled version and the user
> function dump-emacs-portable, but then I have to install before-init
> hooks to systematically replace occurences of the user name and home
> directory in variables and customizations to make the dump usable.
> It's a kludge no distro would want to use.  Even then there are a
> handful of libraries that have to be kept out of the dump (using bogus
> "(provide <x>)" expressions) and then explicitly loaded in an
> after-init hook after removing the bogus features. And they aren't
> consistent with the ones that were problems for native-compiled
> dumping.  But my point is, loading that byte-compiled dump takes
> somewhat longer than loading the native-compiled dump that had twice
> as many pre-loaded libraries.

If you want to dump more libraries than Emacs does by default, why not
modify loadup.el to load those libraries at build time?  Then they
will be dumped together with all the rest, and the loading won't need
any changes on your side.

> Emacs needs a lot more work to really support compile-time dumping of
> a larger set of libraries.  There are too many instances of code
> evaluated at load time that should be evaluated in a before-init or
> after-init hook, that depend on variables set in C code after the dump
> file is loaded.  Or run-time services like dbus.  To really support
> site-specific dumping, a robust analysis identifying that code and
> transforming it appropriately would have to be performed either prior
> to or as part of dump-mode.  It's non-trivial.  The changes to the C
> code to deal with additional objects like text properties while
> purifying are (or were) trivial by comparison.

I guess this is true, at least to some degree.  Patches to this
effect are welcome.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Sun, Feb 5, 2023 at 2:40 AM Eli Zaretskii <eliz@gnu.org> wrote:
> I asked because Emacs 29 adds some changes specifically intended to
> solve the problem of infinite recursion in trampoline compilation.
I'll test again after I reimplement the dumping support for text
properties, but it might be a while.
> > As far as I know, emacs still doesn't support dumping arbitrary
> > native-compiled libraries at compile-time.
>
> What problems do you see if you try?
   I filed a number of bug reports for tracking purposes at the time.
1.  Lack of support for purifying certain object types, including but
not limited to text properties
2.  Keymap definitions that were incompatible with purification
3.  The strict requirements around autoload and load order in
compile-time dumping - plenty of libraries just aren't factored to be
loaded without autoloads, or weren't at the time of the experiment
4.   Issues with variable initialization and code evaluated at
load-time rather than at init, as noted at the end of my previous
reply
5   There were some issues with docstrings, but those should have been
resolved the move to always load those from .elc files.
6.  Spawning the asynchronous compile process with whatever dump file
the session was loaded with is asking for trouble.  It would be better
to specify a "known good" dump file, like one limited to loadup with
no site files, that has been tested.  The compiler process dump file
could be a customization variable if tweaks are required.

Presumably 1 & 2 will be resolved by removing purification, whenever
that happens.  Number 3 is fairly easy to resolve by introducing a
variable to control when that strict ordering is required, so it can
be turned off after the explicit loading is done in "loadup".  As
Stefan Monnier suggested, the site-load phase is probably best done as
a third build phase, following the bootstrap and loadup-only dumps.

> > Which is too bad, because the dumped version is a LOT faster to
> > startup.
>
> Are you sure? is this with native-compilation or without it?

Very sure.  I had dumped emacs with 4000-5000 loaded libraries, all
precompiled .eln files, and it would pop up in 5 seconds or less the
first time it loaded. Subsequent starts (while the first was still
running) appeared to load even faster,  I assumed because the eln
files are loaded as shared objects and shared between processes by the
kernel.

> I don't see why it would be faster when loading a dumped Emacs: the
> pdumper file records only the names of the *.eln files, not their
> contents, so Emacs still has to load all of the *.eln files recorded
> in the pdumper file, and that takes time and doesn't change much when
> the names are recorded in emacs.pdmp.

I need to do proper testing, but part of the speedup is probably due
to being able to skip checking whether the source file has the right
checksum.  I recently submitted a bug report showing that when I had
900+ packages installed and native-compilation enabled for 30.0.50,
that startup time is dominated by locate-file, presumably because the
eln loader has to find the right source file to check.  Otherwise, the
length of the load-path would have minimal effect, due to the eln
caching.

> If you want to dump more libraries than Emacs does by default, why not
> modify loadup.el to load those libraries at build time?  Then they
> will be dumped together with all the rest, and the loading won't need
> any changes on your side.

See above.  Also, I would use site-load to avoid polluting loadup.el.
The biggest advantage of dump-emacs-portable is that it doesn't have
the prohibition against autoloads and circular requires that dump-mode
does, or the problems with purification.
I'm happy to submit patches once I've been able to set up the testing
infrastructure necessary to be sure they're sound.  And actually done
the work, which is non-trivial.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Lynn Winebarger <owinebar@gmail.com>
>> Date: Sat, 4 Feb 2023 17:05:19 -0500
>> Cc: rms@gnu.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>> 
>> On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
>> >
>> > > Something in emacs is putting advice on call-interactively.  When I
>> > > extended the baseline dump to include essentially all libraries
>> > > included with emacs with native-compilation, it caused an infinite
>> > > (asynchronous) regress while compiling the trampoline for
>> > > call-interactively.  I wasn't aware of this until /tmp filled up.
>> >
>> > Is this with Emacs 28 or Emacs 29?
>> 
>> This was with 28.1 modified to support dumping with many pre-loaded
>> native-compiled files.
>
> I asked because Emacs 29 adds some changes specifically intended to
> solve the problem of infinite recursion in trampoline compilation.
>
>> As far as I know, emacs still doesn't support dumping arbitrary
>> native-compiled libraries at compile-time.
>
> What problems do you see if you try?  AFAIR, if you tweak
> native-comp-eln-load-path so that the *.eln files you want to dump are
> directed to the ../native-lisp directory relative to where the Emacs
> binary lives, then dump Emacs, loading such a dumped Emacs should
> work.
>
> Andrea, am I missing something?

I think he might be talking about re-dumping Emacs?

Otherwise no, I'm not aware of anything you are missing.  We can dump
any library we want just loading it before the dump (we modified the
list of dumped libraries just doing this many times in the last two
years).

Bests

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Liliana Marie Prikler <liliana.prikler@gmail.com>
>> Cc: Andrea Corallo <akrl@sdf.org>, Lars Ingebrigtsen <larsi@gnus.org>, 
>>  Stefan Monnier <monnier@iro.umontreal.ca>, Rob Browning
>>  <rlb@defaultvalue.org>
>> Date: Sat, 04 Feb 2023 18:48:34 +0100
>> 
>> Since I am on the Guix team for Emacs packaging, I'll try to lay out
>> some concerns both from the perspective of a user and a downstream
>> packager.  I hope I'm not too late to the party.
>
> Thank you for your feedback.
>
>> >   . Do people actually use 'inhibit-automatic-native-compilation' or
>> >     the corresponding environment variable?  If so, for what reasons,
>> >     and why tweaking 'native-comp-eln-load-path' to direct the *.eln
>> >     files to some other place, including the temporary-file
>> > directory,
>> >     was not enough?
>> On Guix, I want my Emacs packages to either (1) already have been
>> natively compiled when using `guix install some-emacs-package' or (2)
>> to not natively compile anything and clutter my user-emacs-directory. 
>> Now the concern about clutter is somewhat secondary to Emacs spending
>> time that some other Emacs could have spent on CI.  (We don't do native
>> compilation on CI *yet*, but imho it would be worth doing for some
>> packages)
>
> That's fine.  Disabling native compilation will continue be supported,
> of course, for any number of reasons.
>
>> >   . What do we want to do about compiling trampolines when
>> >     native-compilation is disabled?
>> In my opinion, there should be a way to generate these trampolines
>> ahead of time in a known location (e.g. $package-trampolines.so) for
>> the emacs package $package.  If no such trampoline is found and native-
>> compilation is disabled, no compilation should take place.
>
> Andrea, I think comp-compile-all-trampolines fits this bill?

Yes I believe so.

> That is,
> if I load a package, then invoke that function, it will compile all
> the trampolines that are not already compiled?

Yes, it will compile all trampolines that are needed *and* are not found
to be already compiled.  If comp-compile-all-trampolines was used before
all trampolines are already available and no trampoline compilation will
be performed.

>> >   . Do we want to keep the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION
>> >     environment variable?
>> > 
>> >     I dislike having environment variables that alter Emacs behavior,
>> >     because environment variables are inherited by sub-processes.  So
>> >     having this variable set runs the risk of affecting all the
>> >     sub-processes, something that could be unexpected and is not easy
>> >     to prevent.  We had similar problems with EMACSLOADPATH, for
>> >     example, which is especially painful when building another
>> > version
>> >     of Emacs from a shell buffer inside Emacs.  This causes some
>> >     hard-to-debug problems.
>> >     So if this environment variable is largely unused, maybe we
>> > should
>> >     remove it, even if we keep 'inhibit-automatic-native-
>> > compilation'.
>> I don't think a variable is needed necessarily.  What is needed is a
>> method of enabling it reliably on the command line (i.e. a switch like
>> --no-native-compilation would work, but so would --eval '(setq inhibit-
>> automatic-native-compilation t)'), and reliably as a user editing just
>> Emacs configuration files (in particular, early-init.el seems like the
>> place I would naturally place it in).
>
> We already have (and will keep supporting) a way of disabling native
> compilation, and/or the compilation of trampolines, via --eval.
>
> Thanks.
>

Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Andrea Corallo <akrl@sdf.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Lynn Winebarger <owinebar@gmail.com>
>>> Date: Sat, 4 Feb 2023 17:05:19 -0500
>>> Cc: rms@gnu.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
>>> 
>>> On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
>>> >
>>> > > Something in emacs is putting advice on call-interactively.  When I
>>> > > extended the baseline dump to include essentially all libraries
>>> > > included with emacs with native-compilation, it caused an infinite
>>> > > (asynchronous) regress while compiling the trampoline for
>>> > > call-interactively.  I wasn't aware of this until /tmp filled up.
>>> >
>>> > Is this with Emacs 28 or Emacs 29?
>>> 
>>> This was with 28.1 modified to support dumping with many pre-loaded
>>> native-compiled files.
>>
>> I asked because Emacs 29 adds some changes specifically intended to
>> solve the problem of infinite recursion in trampoline compilation.
>>
>>> As far as I know, emacs still doesn't support dumping arbitrary
>>> native-compiled libraries at compile-time.
>>
>> What problems do you see if you try?  AFAIR, if you tweak
>> native-comp-eln-load-path so that the *.eln files you want to dump are
>> directed to the ../native-lisp directory relative to where the Emacs
>> binary lives, then dump Emacs, loading such a dumped Emacs should
>> work.
>>
>> Andrea, am I missing something?
>
> I think he might be talking about re-dumping Emacs?
>
> Otherwise no, I'm not aware of anything you are missing.  We can dump
> any library we want just loading it before the dump (we modified the
> list of dumped libraries just doing this many times in the last two
> years).

To be more precise: with native-comp we should be able to dump any
library we are capable of dumping with a non native comp Emacs.  There
are for instance lisp objects we still don't know how to dump (at least
it was the case till a while ago), there there are probably other edges
to be rounded, but they should be non native comp specific.

Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Hello everyone, and sorry for my being late.

Le jeudi 2 février 2023 à 09:17, Sean Whitton 
<spwhitton@spwhitton.name> a écrit :

> Aymeric, you investigated one of these cases in details.  Would 
> you be
> able to summarise the situation we had in Debian for Eli, 
> please?

The case Sean refers to is probably the packaging of projectile 
(https://github.com/bbatsov/projectile).

In the testbed of this package, the buttercup library 
(https://github.com/jorgenschaefer/emacs-buttercup) is used to 
advise some primitives, like `file-exists-p', 
`insert-file-contents', `file-directory-p' and probably 
others. These primitives can be advised so as to be replaced by a 
function that does nothing, or that always returns a specific 
value, etc...

The following test was interesting :

(describe "projectile-parse-dirconfig-file"
  (it "parses dirconfig and returns directories to ignore and 
  keep"
    (spy-on 'file-exists-p :and-return-value t)
    (spy-on 'file-truename :and-call-fake (lambda (filename) 
    filename))
    (spy-on 'insert-file-contents :and-call-fake
            (lambda (filename)
              (save-excursion (insert 
              "\n-exclude\n+include\n#may-be-a-comment\nno-prefix\n 
              left-wspace\nright-wspace\t\n"))))
    (expect (projectile-parse-dirconfig-file) :to-equal 
    '(("include/")
                                                          ("exclude"
							   "#may-be-a-comment"
							   "no-prefix"
							   "left-wspace"
							   "right-wspace")
                                                          nil))
    ;; same test - but with comment lines enabled using prefix '#'
    (let ((projectile-dirconfig-comment-prefix ?#))
      (expect (projectile-parse-dirconfig-file) :to-equal 
      '(("include/")
							    ("exclude"
							     "no-prefix"
							     "left-wspace"
							     "right-wspace")
							    nil)))
    ))

The primitive `file-exists-p' is advised as to always return t, 
whether the argument corresponds to an existing file or not. If we 
assume the trampoline is not already there (and that is the case 
in our build environment), a trampoline compilation is triggered, 
and exits without error. The advice is effective only after that.

Then `file-truename' is advised, but it is not a primitive.

Then `insert-file-contents' is advised as well, it is a 
primitive. We enter the function `comp-subr-trampoline-install', 
and we begin by checking whether the corresponding trampoline 
already exists by entering the function 
`comp-trampoline-search'. This function relies on `file-exists-p', 
which answers that the file corresponding to the compiled 
trampoline exists, even if it doesn't. Hence we enter 
`native-elisp-load' with a filename that doesn't exist as 
argument, and we error out :

comp-subr-trampoline-install(insert-file-contents)
comp-trampoline-search(insert-file-contents)
native-elisp-load("/tmp/buttercupKuLmvD/28.2-4001e2a9/subr--trampoline-696...
error: (native-lisp-load-failed "file does not exists" 
"/tmp/buttercupKuLmvD/28.2-4001e2a9/subr--trampoline-696e736572742d66696c652d636f6e74656e7473_insert_file_contents_0.eln")

We always get the error because the trampoline is never there 
beforehand, and upstream did not get it precisely because it was 
lucky to always have it before running the test. If we have to 
load *any* trampoline after the advice of `file-exists-p' is 
effective, and we're not lucky enough to already have the 
trampoline, then we reach the error.

In that case, we worked around the problem by adding the primitive 
`insert-file-contents' to the variable 
`native-comp-never-optimize-functions'.

>>   . where and under what circumstances will those advised 
>>   functions be
>>     called, as part of your preparation of the packages?  (if 
>>     the
>>     advised functions are only called when the end-user uses 
>>     the
>>     package, that is not relevant to the present discussion)

As far as we can tell, this advising of primitives happens mostly 
in tests, that is in our build environment on our build machines.

>>   . if we provide a way to specify, via 
>>   comp-enable-subr-trampolines,
>>     the directory to which the trampolines will be written, 
>>     will that
>>     satisfy your uses? if not, why not (details, please)?
>>   . why cannot you use native-comp-eln-load-path to force the
>>     trampolines go to a directory of your choice?

In this case, this is not pertinent. We need to :
- either not to have to install a trampoline
- or, if we have to, be certain to find it in the *first* 
  directory returned by `comp-eln-load-path-eff'.

If neither condition are met, we reach the error. The only 
variables that are of help here are 
`native-comp-never-optimize-functions' or 
`comp-enable-subr-trampolines'.

Since EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION acts on neither 
of these, we have to manually add

(with-eval-after-load 'comp (push 'insert-file-contents 
native-comp-never-optimize-functions))

At the beginning of the test file.

For this reason, it would be nice, if possible, that 
EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION also disables 
trampoline compilation, for instance by setting 
`comp-enable-subr-trampolines'. The name of the environment 
variable would be changed accordingly, of course.

The performance penalty wouldn't be a problem, since this would 
only be for the duration of the test. If Andrea prefers, we could 
also have two different environment variables, one for trampoline 
and the other for deferred async compilation. But I don't see why 
only one knob doesn't work : if we had an environment variable 
that acted on both `inhibit-automatic-native-compilation' and 
`comp-enable-subr-trampolines', like what happens when 
`native-comp-available-p' evaluates to nil, wouldn't that exactly 
"really disable native compilation", or am I missing something ?

>> I dislike having environment variables that alter Emacs 
>> behavior,
>> because environment variables are inherited by sub-processes.

This is precisely why I like it. Our build mechanism can have 
emacs instances nested deep in layers of wrapper scripts. Because 
of this, we can simply export the variable in the environment of 
the ancestor process, and not worry about adding --eval arguments 
in various places in the middle of our scripts. But only the 
descendants of this ancestor process are concerned by this 
environment variable, and they live only as long as the building 
mechanism runs, so I don't see the reason for worry. Then, if 
users want to use this variable in normal use, and they decide on 
top of that to have multiple nested emacsen, then I agree that 
they have to be careful, but that situation seems a bit 
far-fetched.

I hope everything was clear.

Best,

Aymeric Agon-Rambosson

P.S. We have a couple of packages for which we have deactivated 
some trampoline compilation in order to pass tests or avoid an 
error. I haven't been able to find out why like I did with 
projectile, but I guess these cases can be of interest to you :

https://github.com/joaotavora/yasnippet/pull/1158

https://github.com/DamienCassou/beginend/pull/75

P.P.S. The new mechanism consisting in having all the trampolines 
compiled as part of the building process would probably solve 
these two problems, but not the one from projectile. I assume 
these trampolines would be put in the system eln cache, which is 
not returned as the first directory by 
`comp-eln-load-path-eff'. We would therefore reach the error 
anyway. But this is very good in my opinion, as are all efforts to 
increase the amount of stuff native compiled eagerly (like 
NATIVE_FULL_AOT).

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Lynn Winebarger <owinebar@gmail.com>, monnier@iro.umontreal.ca,
>         emacs-devel@gnu.org
> Date: Mon, 06 Feb 2023 10:15:28 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Lynn Winebarger <owinebar@gmail.com>
> >> Date: Sat, 4 Feb 2023 17:05:19 -0500
> >> Cc: rms@gnu.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >> 
> >> On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >> >
> >> > > Something in emacs is putting advice on call-interactively.  When I
> >> > > extended the baseline dump to include essentially all libraries
> >> > > included with emacs with native-compilation, it caused an infinite
> >> > > (asynchronous) regress while compiling the trampoline for
> >> > > call-interactively.  I wasn't aware of this until /tmp filled up.
> >> >
> >> > Is this with Emacs 28 or Emacs 29?
> >> 
> >> This was with 28.1 modified to support dumping with many pre-loaded
> >> native-compiled files.
> >
> > I asked because Emacs 29 adds some changes specifically intended to
> > solve the problem of infinite recursion in trampoline compilation.
> >
> >> As far as I know, emacs still doesn't support dumping arbitrary
> >> native-compiled libraries at compile-time.
> >
> > What problems do you see if you try?  AFAIR, if you tweak
> > native-comp-eln-load-path so that the *.eln files you want to dump are
> > directed to the ../native-lisp directory relative to where the Emacs
> > binary lives, then dump Emacs, loading such a dumped Emacs should
> > work.
> >
> > Andrea, am I missing something?
> 
> I think he might be talking about re-dumping Emacs?

Yes.  I'm saying that for re-dumping one should make sure that the
additional dumped *.eln files are in the ../native-lisp directory
relative to where the Emacs binary lives, and then when Emacs is
restarted, it will find those *.eln files at startup time.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

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

On Mon, Feb 6, 2023, 8:05 AM Eli Zaretskii <eliz@gnu.org> wrote:

> Yes.  I'm saying that for re-dumping one should make sure that the
> additional dumped *.eln files are in the ../native-lisp directory
> relative to where the Emacs binary lives, and then when Emacs is
> restarted, it will find those *.eln files at startup time.
>

Are you saying dump-emacs-portable supports preloaded native-compiled
libraries now?

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

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Lynn Winebarger <owinebar@gmail.com>
> Date: Mon, 6 Feb 2023 08:37:46 -0500
> Cc: Andrea Corallo <akrl@sdf.org>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	emacs-devel <emacs-devel@gnu.org>
> 
> On Mon, Feb 6, 2023, 8:05 AM Eli Zaretskii <eliz@gnu.org> wrote:
> 
>  Yes.  I'm saying that for re-dumping one should make sure that the
>  additional dumped *.eln files are in the ../native-lisp directory
>  relative to where the Emacs binary lives, and then when Emacs is
>  restarted, it will find those *.eln files at startup time.
> 
> Are you saying dump-emacs-portable supports preloaded native-compiled libraries now?

No, I'm saying that starting Emacs assumes the dumped *.eln files live
in one of two possible locations, and you must make sure they are in
one of those two locations, or else Emacs will fail to start.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

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

On Mon, Feb 6, 2023, 9:07 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Lynn Winebarger <owinebar@gmail.com>
> > Date: Mon, 6 Feb 2023 08:37:46 -0500
> > Cc: Andrea Corallo <akrl@sdf.org>, Stefan Monnier <
> monnier@iro.umontreal.ca>,
> >       emacs-devel <emacs-devel@gnu.org>
> >
> > On Mon, Feb 6, 2023, 8:05 AM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> >  Yes.  I'm saying that for re-dumping one should make sure that the
> >  additional dumped *.eln files are in the ../native-lisp directory
> >  relative to where the Emacs binary lives, and then when Emacs is
> >  restarted, it will find those *.eln files at startup time.
> >
> > Are you saying dump-emacs-portable supports preloaded native-compiled
> libraries now?
>
> No, I'm saying that starting Emacs assumes the dumped *.eln files live
> in one of two possible locations, and you must make sure they are in
> one of those two locations, or else Emacs will fail to start.
>

Then I'm confused by what you mean by "re-dumping" above.  I'm only
referencing the result of starting temacs in dump-mode.

Andreas may well be correct that there are no additional challenges for
native-compiled libraries than there would be for byte-compiled, aside from
the part of the build process ensuring those eln files exist in the right
location that you note.  But since dump-portable-emacs is available for the
byte-compiled case, these limitations are most noticable for someone
attempting to dump native-compiled libraries.

As soon as a library requiring cl-lib is included, the prohibition (in
dump-mode) against autoloading and circular requires is a problem.

Lynn

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

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, akrl@sdf.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Mon, 06 Feb 2023 11:57:41 +0100
> 
> Hello everyone, and sorry for my being late.

Thanks for chiming in.

> >>   . where and under what circumstances will those advised 
> >>   functions be
> >>     called, as part of your preparation of the packages?  (if 
> >>     the
> >>     advised functions are only called when the end-user uses 
> >>     the
> >>     package, that is not relevant to the present discussion)
> 
> As far as we can tell, this advising of primitives happens mostly 
> in tests, that is in our build environment on our build machines.

Test harnesses frequently need to jump through various hoops,
certainly so when they need to mock-out some APIs to do their job.

> >>   . if we provide a way to specify, via 
> >>   comp-enable-subr-trampolines,
> >>     the directory to which the trampolines will be written, 
> >>     will that
> >>     satisfy your uses? if not, why not (details, please)?
> >>   . why cannot you use native-comp-eln-load-path to force the
> >>     trampolines go to a directory of your choice?
> 
> In this case, this is not pertinent. We need to :
> - either not to have to install a trampoline
> - or, if we have to, be certain to find it in the *first* 
>   directory returned by `comp-eln-load-path-eff'.

I don't think I understand: the output of that function depends on
native-comp-eln-load-path, so why tweaking that is not enough?

> If neither condition are met, we reach the error. The only 
> variables that are of help here are 
> `native-comp-never-optimize-functions' or 
> `comp-enable-subr-trampolines'.

If those can help you solve your problem, I think that's "good enough"
for something that needs to be done in a test suite.  My primary
target audience, in contrast, is Emacs users.

> Since EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION acts on neither 
> of these, we have to manually add
> 
> (with-eval-after-load 'comp (push 'insert-file-contents 
> native-comp-never-optimize-functions))
> 
> At the beginning of the test file.

A similar solution could have solved your problem even if
inhibit-automatic-native-compilation didn't exist at all,and we only
had comp-enable-subr-trampolines and native-comp-deferred-compilation,
right?

> For this reason, it would be nice, if possible, that 
> EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION also disables 
> trampoline compilation, for instance by setting 
> `comp-enable-subr-trampolines'. The name of the environment 
> variable would be changed accordingly, of course.

Why do you need this to be done together with disabling native
compilation? why not do it with two separate settings?

> >> I dislike having environment variables that alter Emacs 
> >> behavior,
> >> because environment variables are inherited by sub-processes.
> 
> This is precisely why I like it. Our build mechanism can have 
> emacs instances nested deep in layers of wrapper scripts. Because 
> of this, we can simply export the variable in the environment of 
> the ancestor process, and not worry about adding --eval arguments 
> in various places in the middle of our scripts. But only the 
> descendants of this ancestor process are concerned by this 
> environment variable, and they live only as long as the building 
> mechanism runs, so I don't see the reason for worry. Then, if 
> users want to use this variable in normal use, and they decide on 
> top of that to have multiple nested emacsen, then I agree that 
> they have to be careful, but that situation seems a bit 
> far-fetched.

I understand the convenience, but once again, my primary audience is
the Emacs users, and they rarely need to handle such convoluted
situations.  Some relative inconvenience aside, I see no reason why
you couldn't make do without the environment variable, but via some
automatically-loaded init files or somesuch.

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Mon, Feb 6, 2023 at 5:15 AM Andrea Corallo <akrl@sdf.org> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Lynn Winebarger <owinebar@gmail.com>
> >> Date: Sat, 4 Feb 2023 17:05:19 -0500
> >> Cc: rms@gnu.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> >>
> >> On Sat, Feb 4, 2023 at 3:08 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >> >
> >> > > Something in emacs is putting advice on call-interactively.  When I
> >> > > extended the baseline dump to include essentially all libraries
> >> > > included with emacs with native-compilation, it caused an infinite
> >> > > (asynchronous) regress while compiling the trampoline for
> >> > > call-interactively.  I wasn't aware of this until /tmp filled up.
> >> >
> >> > Is this with Emacs 28 or Emacs 29?
> >>
> >> This was with 28.1 modified to support dumping with many pre-loaded
> >> native-compiled files.
> >
> > I asked because Emacs 29 adds some changes specifically intended to
> > solve the problem of infinite recursion in trampoline compilation.
> >
> >> As far as I know, emacs still doesn't support dumping arbitrary
> >> native-compiled libraries at compile-time.
> >
> > What problems do you see if you try?  AFAIR, if you tweak
> > native-comp-eln-load-path so that the *.eln files you want to dump are
> > directed to the ../native-lisp directory relative to where the Emacs
> > binary lives, then dump Emacs, loading such a dumped Emacs should
> > work.
> >
> > Andrea, am I missing something?
>
> I think he might be talking about re-dumping Emacs?
>
> Otherwise no, I'm not aware of anything you are missing.  We can dump
> any library we want just loading it before the dump (we modified the
> list of dumped libraries just doing this many times in the last two
> years).
>

I'm wondering if we are talking about the same thing.  It's not enough
that the eln files exist after the dump.  For the benefits I'm
discussing, the eln files have to be generated and placed in
native-lisp (or preferably native-lisp/preloaded) *before* dumping,
not just at run-time.  The eln files must actually be loaded into
memory before the dump so that all the associated work is skipped at
start-up.

If I had meant that the byte-compiled dump happened with eln files
slated to be loaded at startup time as though they had just been
asynchronously compiled, then I would also expect there to be minimal
benefit.  Getting those eln files generated and in the right place
*before* the dump takes some effort.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Lynn Winebarger <owinebar@gmail.com>
> Date: Mon, 6 Feb 2023 09:29:10 -0500
> Cc: Andrea Corallo <akrl@sdf.org>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	emacs-devel <emacs-devel@gnu.org>
> 
>  No, I'm saying that starting Emacs assumes the dumped *.eln files live
>  in one of two possible locations, and you must make sure they are in
>  one of those two locations, or else Emacs will fail to start.
> 
> Then I'm confused by what you mean by "re-dumping" above.  I'm only referencing the result of starting
> temacs in dump-mode.

You started the re-dumping subject (although the current discussion is
about something completely different).

> Andreas may well be correct that there are no additional challenges for native-compiled libraries than there
> would be for byte-compiled, aside from the part of the build process ensuring those eln files exist in the right
> location that you note.  But since dump-portable-emacs is available for the byte-compiled case, these
> limitations are most noticable for someone attempting to dump native-compiled libraries. 
> 
> As soon as a library requiring cl-lib is included, the prohibition (in dump-mode) against autoloading and
> circular requires is a problem.  

Sorry, this is unrelated to the issue at hand, and was already
discussed in the past, so if you want still to discuss it, please
start a new thread.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le lundi 6 février 2023 à 16:29, Eli Zaretskii <eliz@gnu.org> a 
écrit :

> I don't think I understand: the output of that function depends 
> on
> native-comp-eln-load-path, so why tweaking that is not enough?

For this specific problem, if the trampoline has not already been 
compiled, and can therefore be found in none of the directories in 
the `native-comp-eln-load-path' list, being able to tweak the 
variable does us no good.

If we assume that all trampolines have been compiled beforehand by 
a mechanism similar to "make trampolines", then we can find the 
trampolines in the system eln cache directory, but we still need 
to ensure that this system eln cache directory is the first in the 
variable `native-comp-eln-load-path'. However, the docstring says 
"The last directory of this list is assumed to be the system 
one.".

So for the variable `native-comp-eln-load-path', in order to 
respect that assumption and solve our projectile problem, we can 
either have :

("/usr/lib/emacs/<version>/native-lisp")

or

("/usr/lib/emacs/<version>/native-lisp" <arbitrary number of user 
eln cache directories> "/usr/lib/emacs/<version>/native-lisp")

The first solution works only if we are sure not to have to load 
any native-compiled elisp from user cache directories, and we 
don't have to native-compile any elisp (if we assume emacs running 
as unpriviledged user).

The second solution could work, I'm not sure whether the same 
value being present twice would create problems.

This projectile case is very specific, I admit, but just to give 
you an idea of why we might want to avoid all of this and simply 
disable trampoline compilation, either globally (which we did not 
have to do until now) or a specific number of primitives (see the 
list of my last message).

> If those can help you solve your problem, I think that's "good 
> enough"
> for something that needs to be done in a test suite.  My primary
> target audience, in contrast, is Emacs users.

I agree. For this specific case, disabling trampoline compilation 
on the relevant primitives by modifying 
`native-comp-never-optimize-functions' works (see 
https://github.com/bbatsov/projectile/commit/e18ad4d6111eb9e975ccce028baf5e4bb786bfcf), 
and is also the least intrusive way. And since it is not specific 
to our build system, we can reproduce with the upstream sources 
under specific circumstances, those can eventually be patched.

> A similar solution could have solved your problem even if
> inhibit-automatic-native-compilation didn't exist at all,and we 
> only
> had comp-enable-subr-trampolines and 
> native-comp-deferred-compilation,
> right?

`inhibit-automatic-native-compilation' is not exactly the negation 
of 
`native-comp-deferred-compilation'. `inhibit-automatic-native-compilation' 
also disables the outputting of the result of trampoline 
compilation to the file system, which solves the other problems 
related to unwritable $HOME mentioned earlier. This new variable 
allows for three behaviours :
- Not compiling trampolines (`comp-enable-subr-trampolines' set to 
  nil)
- Compiling them, but not outputting them to the filesystem 
  (`comp-enable-subr-trampolines' set to non-nil, and 
  `inhibit-automatic-native-compilation' set to non-nil)
- Compiling them and outputting them to the filesystem 
  (`comp-enable-subr-trampolines' set to non-nil, and 
  `inhibit-automatic-native-compilation' set to nil)

By contrast, `native-comp-deferred-compilation' and 
`comp-enable-subr-trampolines' only allowed for the first and the 
third behaviour. This second behaviour is the one we use by 
default now. The trampolines are used in the test (except in the 
pathological cases where they make emacs error or make the test 
fail), but we do not worry about where to output them, because we 
don't.

> Why do you need this to be done together with disabling native
> compilation? why not do it with two separate settings?

We could do it with two separate settings, but if it is decided 
not to get rid of environment variables, that would mean two of 
them.

> I understand the convenience, but once again, my primary 
> audience is
> the Emacs users, and they rarely need to handle such convoluted
> situations.  Some relative inconvenience aside, I see no reason 
> why
> you couldn't make do without the environment variable, but via 
> some
> automatically-loaded init files or somesuch.

I think there was some worry about whether an extra eval would 
arrive "soon enough", that is before any native compilation would 
have been attempted. In contrast, the value of the environment 
variable is obtained at very beginning of `normal-top-level', that 
is before any native compilation could possibly happen.

Other than that, I agree that this is mainly a question of 
convenience for us. So much so that, as Sean said, we would 
probably patch it back should it be removed. On top of that, the 
delta corresponding to the environment variable specifically is 
negligible (1 line in normal-top-level) when compared to the delta 
needed to implement the underlying 
`inhibit-automatic-native-compilation'.

Thanks !

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Mon, Feb 6, 2023 at 10:28 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Lynn Winebarger <owinebar@gmail.com>
> > Date: Mon, 6 Feb 2023 09:29:10 -0500
> > Cc: Andrea Corallo <akrl@sdf.org>, Stefan Monnier <monnier@iro.umontreal.ca>,
> >       emacs-devel <emacs-devel@gnu.org>
> >
> >  No, I'm saying that starting Emacs assumes the dumped *.eln files live
> >  in one of two possible locations, and you must make sure they are in
> >  one of those two locations, or else Emacs will fail to start.
> >
> > Then I'm confused by what you mean by "re-dumping" above.  I'm only referencing the result of starting
> > temacs in dump-mode.
>
> You started the re-dumping subject (although the current discussion is
> about something completely different).
Not with regard to native-compiled libraries.  But to circle back to
your question about whether emacs 29 exhibited this behavior, this
discussion has clarified the disconnect in our understanding.

To answer your question, I need to construct some test cases with the
following property:
Library A containing function f which is  always invoked in the native
compiler process
Library B containing function g is used to advise function f
under 2 variants of the dump file
1) Library A is included in the dump used in the native compiler
process, but the ELN is not loaded prior to the dump
2) Library A is included in the dump used in the native compiler
process, and the ELN was loaded prior to the dump
and 3 variants in the way the advice is applied
a) In the ordinary eval phase of loading the library
b) In a compile-when-eval or compile-and-eval context
c) During macro expansion

A variant of these test cases would be when function f is only
conditionally invoked in the native compiler process.

I observed the behavior in question when "f" was "call-interactively"
with variant (2) of the dump file.  I don't know what library B or
function g was, or which of variants (a) - (c) was the case.  But it
should be possible to identify some other function f involved in the
native compiler process and a library B and function g under each of
those variants without having to be able to dump (e.g.) a
native-compiled cl-lib.

Another set of test cases would be those that are unlikely to arise by
accident, for example in which each B/g deliberately creates a novel
B'/g' that it native-compiles and requires during the trampoline
compiling process, so it couldn't be caught by simply tracking which
functions are being processed.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: spwhitton@spwhitton.name, monnier@iro.umontreal.ca, emacs-devel@gnu.org,
>  akrl@sdf.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Tue, 07 Feb 2023 04:39:09 +0100
> 
> > I don't think I understand: the output of that function depends on
> > native-comp-eln-load-path, so why tweaking that is not enough?
> 
> For this specific problem, if the trampoline has not already been 
> compiled, and can therefore be found in none of the directories in 
> the `native-comp-eln-load-path' list, being able to tweak the 
> variable does us no good.

I don't think I follow: why not?

But maybe this is again because of some special requirements of
mocking-out primitives for testing purposes, in which case stranger
issues could happen.

> If we assume that all trampolines have been compiled beforehand by 
> a mechanism similar to "make trampolines", then we can find the 
> trampolines in the system eln cache directory, but we still need 
> to ensure that this system eln cache directory is the first in the 
> variable `native-comp-eln-load-path'.

Why do you need it to be the first directory?

> However, the docstring says "The last directory of this list is
> assumed to be the system one.".

Are you sure you interpret this correctly? what do you think being a
"system one" means for the purposes of your situation, and why is that
important in the first place?

> This projectile case is very specific, I admit, but just to give 
> you an idea of why we might want to avoid all of this and simply 
> disable trampoline compilation, either globally (which we did not 
> have to do until now) or a specific number of primitives (see the 
> list of my last message).

It is fine with me to disable trampoline compilation; the ability to
do so existed (albeit for different reasons) before
inhibit-automatic-native-compilation was introduced.

> `inhibit-automatic-native-compilation' is not exactly the negation 
> of 
> `native-comp-deferred-compilation'. `inhibit-automatic-native-compilation' 
> also disables the outputting of the result of trampoline 
> compilation to the file system, which solves the other problems 
> related to unwritable $HOME mentioned earlier. This new variable 
> allows for three behaviours :
> - Not compiling trampolines (`comp-enable-subr-trampolines' set to 
>   nil)
> - Compiling them, but not outputting them to the filesystem 
>   (`comp-enable-subr-trampolines' set to non-nil, and 
>   `inhibit-automatic-native-compilation' set to non-nil)
> - Compiling them and outputting them to the filesystem 
>   (`comp-enable-subr-trampolines' set to non-nil, and 
>   `inhibit-automatic-native-compilation' set to nil)
> 
> By contrast, `native-comp-deferred-compilation' and 
> `comp-enable-subr-trampolines' only allowed for the first and the 
> third behaviour.

We intend to modify comp-enable-subr-trampolines so that it could
accept a string value, which would be interpreted as the directory to
place compiled trampolines.  Would that be good enough?

The problem I'm trying to solve is that a boolean variable,
inhibit-automatic-native-compilation, has a two effects that cannot be
separated.  I'd like to have two variables instead (which is basically
what we had in Emacs 28), and add to one of them the ability to accept
a string value.

> > I understand the convenience, but once again, my primary audience
> > is the Emacs users, and they rarely need to handle such convoluted
> > situations.  Some relative inconvenience aside, I see no reason
> > why you couldn't make do without the environment variable, but via
> > some automatically-loaded init files or somesuch.
> 
> I think there was some worry about whether an extra eval would 
> arrive "soon enough", that is before any native compilation would 
> have been attempted. In contrast, the value of the environment 
> variable is obtained at very beginning of `normal-top-level', that 
> is before any native compilation could possibly happen.

You could do this in the early-init file, which is processed
approximately at the same place in normal-top-level.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Aymeric Agon-Rambosson <aymeric.agon@yandex.com> writes:

> Hello everyone, and sorry for my being late.
>
> Le jeudi 2 février 2023 à 09:17, Sean Whitton
> <spwhitton@spwhitton.name> a écrit :
>
>> Aymeric, you investigated one of these cases in details.  Would you
>> be
>> able to summarise the situation we had in Debian for Eli, please?
>
> The case Sean refers to is probably the packaging of projectile
> (https://github.com/bbatsov/projectile).
>
> In the testbed of this package, the buttercup library
> (https://github.com/jorgenschaefer/emacs-buttercup) is used to advise
> some primitives, like `file-exists-p', `insert-file-contents',
> `file-directory-p' and probably others. These primitives can be
> advised so as to be replaced by a function that does nothing, or that
> always returns a specific value, etc...
>
> The following test was interesting :
>
> (describe "projectile-parse-dirconfig-file"
>  (it "parses dirconfig and returns directories to ignore and   keep"
>    (spy-on 'file-exists-p :and-return-value t)
>    (spy-on 'file-truename :and-call-fake (lambda (filename)
>    filename))
>    (spy-on 'insert-file-contents :and-call-fake
>            (lambda (filename)
>              (save-excursion (insert
>              "\n-exclude\n+include\n#may-be-a-comment\nno-prefix\n
>              left-wspace\nright-wspace\t\n"))))
>    (expect (projectile-parse-dirconfig-file) :to-equal
>    '(("include/")
>                                                          ("exclude"
> 							   "#may-be-a-comment"
> 							   "no-prefix"
> 							   "left-wspace"
> 							   "right-wspace")
>                                                          nil))
>    ;; same test - but with comment lines enabled using prefix '#'
>    (let ((projectile-dirconfig-comment-prefix ?#))
>      (expect (projectile-parse-dirconfig-file) :to-equal
>      '(("include/")
> 							    ("exclude"
> 							     "no-prefix"
> 							     "left-wspace"
> 							     "right-wspace")
> 							    nil)))
>    ))
>
> The primitive `file-exists-p' is advised as to always return t,
> whether the argument corresponds to an existing file or not. If we
> assume the trampoline is not already there (and that is the case in
> our build environment), a trampoline compilation is triggered, and
> exits without error. The advice is effective only after that.
>
> Then `file-truename' is advised, but it is not a primitive.
>
> Then `insert-file-contents' is advised as well, it is a primitive. We
> enter the function `comp-subr-trampoline-install', and we begin by
> checking whether the corresponding trampoline already exists by
> entering the function `comp-trampoline-search'. This function relies
> on `file-exists-p', which answers that the file corresponding to the
> compiled trampoline exists, even if it doesn't. Hence we enter
> `native-elisp-load' with a filename that doesn't exist as argument,
> and we error out :
>
> comp-subr-trampoline-install(insert-file-contents)
> comp-trampoline-search(insert-file-contents)
> native-elisp-load("/tmp/buttercupKuLmvD/28.2-4001e2a9/subr--trampoline-696...
> error: (native-lisp-load-failed "file does not exists"
> "/tmp/buttercupKuLmvD/28.2-4001e2a9/subr--trampoline-696e736572742d66696c652d636f6e74656e7473_insert_file_contents_0.eln")
>
> We always get the error because the trampoline is never there
> beforehand, and upstream did not get it precisely because it was lucky
> to always have it before running the test. If we have to load *any*
> trampoline after the advice of `file-exists-p' is effective, and we're
> not lucky enough to already have the trampoline, then we reach the
> error.

Advising primitives is a dangerous action, it can lead the Emacs
machinery to malfunction anytime, these misbehaviors are not documented
and can change from version to version as from configuration to
configuration.  The programmer who advises or redefines a primitive
should be ready to understand the underlying machinery and work out
potential issues (as it was done in this case).

I don't think this is a native comp specific problem, I'm sure one can
find similar examples also on non native comp enabled Emacses.

Best Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Advising primitives is a dangerous action, it can lead the Emacs
> machinery to malfunction anytime, these misbehaviors are not documented
> and can change from version to version as from configuration to
> configuration.

But this is not the problem here.  The problem here is that the advice
is not seen by some Lisp callers because the trampoline installation fails.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Advising primitives is a dangerous action, it can lead the Emacs
>> machinery to malfunction anytime, these misbehaviors are not documented
>> and can change from version to version as from configuration to
>> configuration.
>
> But this is not the problem here.  The problem here is that the advice
> is not seen by some Lisp callers because the trampoline installation fails.

Agreed, that's what I tried to say.  If the Emacs machinery fails we
enter into UB territory, the exposed issue (affecting our execution
engine in this case) is only one of the many possible consequences.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le mardi 7 février 2023 à 13:56, Andrea Corallo <akrl@sdf.org> a 
écrit :

> Advising primitives is a dangerous action, it can lead the Emacs
> machinery to malfunction anytime, these misbehaviors are not 
> documented
> and can change from version to version as from configuration to
> configuration.  The programmer who advises or redefines a 
> primitive
> should be ready to understand the underlying machinery and work 
> out
> potential issues (as it was done in this case).

Agreed. Advising primitives was already frowned upon even before 
native compilation, it always was dangerous. However, tests have 
been doing this for quite some time.

> I don't think this is a native comp specific problem, I'm sure 
> one can
> find similar examples also on non native comp enabled Emacses.

This is very possible, but in the cases I mentioned (projectile, 
yasnippet and beginend), the test failures or errors disappeared 
when we excluded the primitives from trampoline 
compilation. Sometimes, like in the projectile case, we were able 
to track exactly what happened, and offer a convincing explanation 
of the fix along with it. But that is not always the case : I have 
offered patches for yasnippet and beginend that solve test 
failures, and I can not completely justify them with anything 
other than "it solves the issue I'm presenting". On top of that, 
said issue has probably not been encountered by the upstream 
maintainer (maybe because they had the trampolines already 
available beforehand), we uncovered it with our specific build 
setting. Those patches may or may not be accepted by the upstream 
maintainers. My point being, the issues caused by widespread 
primitive advice in the test suites of addon packages, that we 
have just uncovered with trampoline compilation and our peculiar 
build environment, will probably be there for a long time. And I 
agree that this is the job of upstream maintainers to patch them, 
but they will be probably be very slow to fix them.

These examples were meant as an illustration of why we might want 
to exclude primitives from trampoline compilation in the 
(potentially quite long) meantime.

Best,

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: Sean Whitton <spwhitton@spwhitton.name>, Eli Zaretskii <eliz@gnu.org>,
>  monnier@iro.umontreal.ca, emacs-devel@gnu.org, larsi@gnus.org,
>  rlb@defaultvalue.org
> Date: Thu, 09 Feb 2023 08:26:09 +0100
> 
> > I don't think this is a native comp specific problem, I'm sure one
> > can find similar examples also on non native comp enabled Emacses.
> 
> This is very possible, but in the cases I mentioned (projectile, 
> yasnippet and beginend), the test failures or errors disappeared 
> when we excluded the primitives from trampoline 
> compilation.

We already have the mechanism for disabling trampoline compilations,
and will continue supporting that: the comp-enable-subr-trampolines
variable.  Are you saying that it is not selective enough, and that in
some cases you needed to disable only trampolines for specific subrs?
IOW, is anything else needed in addition to
comp-enable-subr-trampolines to support those cases you mention?  If
so, what else is needed?

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le mardi 7 février 2023 à 14:49, Eli Zaretskii <eliz@gnu.org> a 
écrit :

> I don't think I follow: why not?

> Why do you need it to be the first directory?

What I am going to describe is true only in the specific setting I 
am presenting, projectile.

After the advice of `file-exists-p' so as to make it always return 
t, implemented by this line :

(spy-on 'file-exists-p :and-return-value t)

Whenever we enter the function `comp-subr-trampoline-install', for 
instance when we advise another primitive, we will enter the 
function `comp-trampoline-search' if :
- we have trampoline compilation enabled
- AND the primitive has not been excluded from trampoline 
  compilation
- AND the trampoline is not already installed

we will try to install the trampoline, by either finding it on the 
filesystem, or if we don't find it, by compiling it.

If `file-exists-p' has been advised as to always return t, 
`comp-trampoline-search' will *always* return a filename of the 
form :

<first directory returned by 
comp-eln-load-path-eff>/subr--trampoline-<hash>_<primitive 
name>.eln (or something like this)

*Regardless* of whether said file actually exists, and in 
 particular even when it should really return nil because the file 
 does not exist.

So if this file that `comp-trampoline-search' assures us to exist 
does not, we reach the error.

So the point was, in order not to reach the error, we need :
- the file subr--trampoline-<hash>_<primitive name>.eln to already 
  exist.
- AND for it to be in the first directory returned by 
  `comp-eln-load-path-eff'.

This was just to show that avoiding the problem without resorting 
to deactivating trampoline compilation for this specific primitive 
is too complicated to bother with.

I hope this answers your first two questions.

> Are you sure you interpret this correctly? what do you think 
> being a
> "system one" means for the purposes of your situation, and why 
> is that
> important in the first place?

I just meant that maybe this assumption of the last directory in 
the list being /usr/lib/emacs/<version>/native-lisp was used 
somewhere else, and that we should not violate it.

In fact, I seem to remember that a function to prune eln cache 
directories from stale eln files was discussed on emacs-devel and 
even implemented. This function was later patched as to exclude 
the system eln cache from such pruning, for some reason I do not 
remember. The "system" eln cache was identified by its being the 
last in the list. So there is in fact at least one place in emacs 
where this assumption is used, and maybe Andrea or Lars or Stefan 
could find others.

But again, this was just part of the hypothetical thinking of 
"what should we do if we want to avoid this specific problem 
without resorting to disabling primitive trampoline 
compilation". That was confusing rather than clarifying, my 
mistake.

All the more since we agree on the necessity to be able to disable 
trampoline compilation.

> We intend to modify comp-enable-subr-trampolines so that it 
> could
> accept a string value, which would be interpreted as the 
> directory to
> place compiled trampolines.  Would that be good enough?

So if I follow, we would have these different possible values for 
`comp-enable-subr-trampolines' :
- nil means not compiling trampolines (no change with now)
- t means compiling them, and outputting them in the location 
  either pointed by `native-compile-target-directory' if it 
  exists, the first valid directory returned by 
  `comp-eln-load-path-eff' if not (no changes with now)
- a string value means compiling them, and outputting them in the 
  location pointed by the string (error if unwritable, or fallback 
  to `native-compile-target-directory' or `comp-eln-load-path-eff' 
  possibly ?) (this is new)

Would it possible to add a constant value, like 
'compile-but-no-output, that would have exactly the second 
behaviour I was mentioning before, that is compiling trampolines 
but not even trying to output them anywhere, by passing nil as the 
last argument of `comp--native-compile' ?

> The problem I'm trying to solve is that a boolean variable,
> inhibit-automatic-native-compilation, has a two effects that 
> cannot be
> separated. I'd like to have two variables instead

That's perfectly reasonable. We would just like to be able to 
replicate with these two variables exactly what 
`inhibit-automatic-native-compilation' did.

> You could do this in the early-init file, which is processed
> approximately at the same place in normal-top-level.

If I am not mistaken, the early init file's name is fixed, and is 
looked for in various places in the user's home. We do not have an 
existing home in our build environment.

Best,

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: spwhitton@spwhitton.name, monnier@iro.umontreal.ca, emacs-devel@gnu.org,
>  akrl@sdf.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Thu, 09 Feb 2023 09:40:35 +0100
> 
> After the advice of `file-exists-p' so as to make it always return 
> t, implemented by this line :
> 
> (spy-on 'file-exists-p :and-return-value t)
> 
> Whenever we enter the function `comp-subr-trampoline-install', for 
> instance when we advise another primitive, we will enter the 
> function `comp-trampoline-search' if :
> - we have trampoline compilation enabled
> - AND the primitive has not been excluded from trampoline 
>   compilation
> - AND the trampoline is not already installed
> 
> we will try to install the trampoline, by either finding it on the 
> filesystem, or if we don't find it, by compiling it.
> 
> If `file-exists-p' has been advised as to always return t, 
> `comp-trampoline-search' will *always* return a filename of the 
> form :
> 
> <first directory returned by 
> comp-eln-load-path-eff>/subr--trampoline-<hash>_<primitive 
> name>.eln (or something like this)
> 
> *Regardless* of whether said file actually exists, and in 
>  particular even when it should really return nil because the file 
>  does not exist.
> 
> So if this file that `comp-trampoline-search' assures us to exist 
> does not, we reach the error.
> 
> So the point was, in order not to reach the error, we need :
> - the file subr--trampoline-<hash>_<primitive name>.eln to already 
>   exist.
> - AND for it to be in the first directory returned by 
>   `comp-eln-load-path-eff'.

So the problem happens because the advice uses the first directory
returned by comp-eln-load-path-eff, for the value it returns, is that
right?  If so, could this then be fixed by changing the advice?

> I hope this answers your first two questions.

It does, thanks.

> > Are you sure you interpret this correctly? what do you think being
> > a "system one" means for the purposes of your situation, and why
> > is that important in the first place?
> 
> I just meant that maybe this assumption of the last directory in 
> the list being /usr/lib/emacs/<version>/native-lisp was used 
> somewhere else, and that we should not violate it.

The main reason of having the system directory last is so that
packages could override *.eln files installed in the 'system" eln
directory, which came with Emacs.

> > We intend to modify comp-enable-subr-trampolines so that it could
> > accept a string value, which would be interpreted as the directory
> > to place compiled trampolines.  Would that be good enough?
> 
> So if I follow, we would have these different possible values for 
> `comp-enable-subr-trampolines' :
> - nil means not compiling trampolines (no change with now)
> - t means compiling them, and outputting them in the location 
>   either pointed by `native-compile-target-directory' if it 
>   exists, the first valid directory returned by 
>   `comp-eln-load-path-eff' if not (no changes with now)
> - a string value means compiling them, and outputting them in the 
>   location pointed by the string (error if unwritable, or fallback 
>   to `native-compile-target-directory' or `comp-eln-load-path-eff' 
>   possibly ?) (this is new)

Yes.  If the directory pointed to by the value of
comp-enable-subr-trampolines is not writable, I think Emacs should
signal an error.  Andrea, WDYT?

> Would it possible to add a constant value, like 
> 'compile-but-no-output, that would have exactly the second 
> behaviour I was mentioning before, that is compiling trampolines 
> but not even trying to output them anywhere, by passing nil as the 
> last argument of `comp--native-compile' ?

Argh, the last argument to comp--native-compile is not documented.
AFAIR, setting it to nil means the trampoline will be written to
temporary-file-directory, right?  Because I don't think we can compile
a trampoline without writing it to some file, from which we will
thereafter load it.  (Andrea, please correct me if I'm wrong.)  If so,
we could assume Lisp programs will use temporary-file-directory if
they need this, would that be good enough?

> > You could do this in the early-init file, which is processed
> > approximately at the same place in normal-top-level.
> 
> If I am not mistaken, the early init file's name is fixed, and is 
> looked for in various places in the user's home. We do not have an 
> existing home in our build environment.

Emacs 29 has the --init-directory command-line argument, which you
could use to tell it to look for early-init file in a non-default
place.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Tue 07 Feb 2023 at 04:39AM +01, Aymeric Agon-Rambosson wrote:

> Le lundi 6 février 2023 à 16:29, Eli Zaretskii <eliz@gnu.org> a écrit :
>
>> I understand the convenience, but once again, my primary audience is
>> the Emacs users, and they rarely need to handle such convoluted
>> situations.  Some relative inconvenience aside, I see no reason why
>> you couldn't make do without the environment variable, but via some
>> automatically-loaded init files or somesuch.
>
> I think there was some worry about whether an extra eval would arrive "soon
> enough", that is before any native compilation would have been attempted. In
> contrast, the value of the environment variable is obtained at very beginning
> of `normal-top-level', that is before any native compilation could possibly
> happen.
>
> Other than that, I agree that this is mainly a question of convenience for
> us. So much so that, as Sean said, we would probably patch it back should it
> be removed. On top of that, the delta corresponding to the environment
> variable specifically is negligible (1 line in normal-top-level) when compared
> to the delta needed to implement the underlying
> `inhibit-automatic-native-compilation'.

I think that the interpretation of this as a matter of convenience is
wrong.  The *correct* solution for Debian, we think, is one that doesn't
involve patching invocations deep inside third party Makefiles.
Carrying all those patches is, I think, not technically correct design.

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Thu 09 Feb 2023 at 12:11PM +02, Eli Zaretskii wrote:

>> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
>> Cc: spwhitton@spwhitton.name, monnier@iro.umontreal.ca, emacs-devel@gnu.org,
>>  akrl@sdf.org, larsi@gnus.org, rlb@defaultvalue.org
>> Date: Thu, 09 Feb 2023 09:40:35 +0100
>>
>> > You could do this in the early-init file, which is processed
>> > approximately at the same place in normal-top-level.
>>
>> If I am not mistaken, the early init file's name is fixed, and is
>> looked for in various places in the user's home. We do not have an
>> existing home in our build environment.
>
> Emacs 29 has the --init-directory command-line argument, which you
> could use to tell it to look for early-init file in a non-default
> place.

This is another case where patching all the third party Makefiles would
not be a technically correct solution, speaking as distribution packagers.

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Thu 02 Feb 2023 at 07:21PM +02, Eli Zaretskii wrote:

>> From: Sean Whitton <spwhitton@spwhitton.name>
>> Cc: emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,
>>   monnier@iro.umontreal.ca,  rlb@defaultvalue.org,
>>   debian-emacsen@lists.debian.org
>> Date: Thu, 02 Feb 2023 09:28:39 -0700
>>
>> One thing that I can say for sure is that we need Emacs not to crash
>> when HOME is non-existent or otherwise not writeable.
>> I am not completely sure whether that is the whole of the issue, but it
>> may well be.
>
> I don't think it crashes in that case.  It definitely shouldn't.  So
> if you have a recipe where it does, please report it with the details.

https://bugs.debian.org/1021842

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le jeudi 9 février 2023 à 09:52, Eli Zaretskii <eliz@gnu.org> a 
écrit :

> Are you saying that it is not selective enough, and that in
> some cases you needed to disable only trampolines for specific 
> subrs?

This is already possible with the variable 
`native-comp-never-optimize-functions'.

This is what we've used for the cases I've presented. For 
projectile for instance :

(with-eval-after-load 'comp
  (push 'insert-file-contents 
  native-comp-never-optimize-functions))

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Thu, 09 Feb 2023 14:05:35 -0700
> 
> > Other than that, I agree that this is mainly a question of convenience for
> > us. So much so that, as Sean said, we would probably patch it back should it
> > be removed. On top of that, the delta corresponding to the environment
> > variable specifically is negligible (1 line in normal-top-level) when compared
> > to the delta needed to implement the underlying
> > `inhibit-automatic-native-compilation'.
> 
> I think that the interpretation of this as a matter of convenience is
> wrong.  The *correct* solution for Debian, we think, is one that doesn't
> involve patching invocations deep inside third party Makefiles.
> Carrying all those patches is, I think, not technically correct design.

Design of what software that is whose responsibility?

The only Makefile's that the upstream Emacs project is responsible for
are our own Makefile's.  The design of how those work when building
Emacs or when running its own test suite is indeed our responsibility.
When, several months ago, we had issues with running tests using Emacs
with native-compilation, we modified our Makefile's to handle that;
the amount of changes to Emacs itself for that purpose was almost nil
(AFAIR, it involved some code to deal with unwritable HOME directory,
something that is also needed in other situations).

But here we are talking about test suites provided by 3rd-party
packages (not even Debian's code, AFAIU), and Makefile's they use.
How is the correctness and/or elegance of that design is of any
concern to us?  Or for Debian, for that matter?  I understand very
well that it is convenient for Debian to be able to run those test
suites without changing them too much, but I see no design issues
here, nothing except for the convenience.  I definitely see no design
issues that should bother us, the upstream project.  And I object to
complicating Emacs when the only valid reason is that some test suite
of some 3rd-party package causes problems because its test harness was
evidently not adapted to Emacs with native-compilation.  If Debian
wants to solve these problems by patching Emacs, that is fine by me;
here we discuss what should and shouldn't be in the upstream Emacs
sources, available and exposed to more than just Debian and more than
just running those particular 3rd-party test suites.

Once again, please try looking at this from my POV, the POV of an
Emacs maintainer.  From my POV, adding features that cause non-trivial
complexity and processing on behalf of a few corner use cases makes
little sense.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Date: Thu, 09 Feb 2023 14:07:23 -0700
> 
> >> > You could do this in the early-init file, which is processed
> >> > approximately at the same place in normal-top-level.
> >>
> >> If I am not mistaken, the early init file's name is fixed, and is
> >> looked for in various places in the user's home. We do not have an
> >> existing home in our build environment.
> >
> > Emacs 29 has the --init-directory command-line argument, which you
> > could use to tell it to look for early-init file in a non-default
> > place.
> 
> This is another case where patching all the third party Makefiles would
> not be a technically correct solution, speaking as distribution packagers.

I presume that those Makefiles run Emacs through some Makefile
variable or macro, like EMACS or RUN_EMACS or suchlike.  If so, just
setting that variable/macro to include --init-directory before running
the top-level Make should be quite easy, and should not require
patching all the Makefiles.

But if that is for some reason inconvenient or impossible, then feel
free to come up with a different solution.  I just don't see why that
should be a problem for the Emacs project to solve.  We cannot be
responsible for test harnesses of 3rd-party packages.  If we can help
you by providing features useful in enough use cases, fine; but this
is not one of those.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le jeudi 9 février 2023 à 12:11, Eli Zaretskii <eliz@gnu.org> a 
écrit :

>> Would it possible to add a constant value, like 
>> 'compile-but-no-output, that would have exactly the second 
>> behaviour I was mentioning before, that is compiling 
>> trampolines 
>> but not even trying to output them anywhere, by passing nil as 
>> the 
>> last argument of `comp--native-compile' ?
>
> Argh, the last argument to comp--native-compile is not 
> documented.
> AFAIR, setting it to nil means the trampoline will be written to
> temporary-file-directory, right?  Because I don't think we can 
> compile
> a trampoline without writing it to some file, from which we will
> thereafter load it.  (Andrea, please correct me if I'm wrong.) 
> If so,
> we could assume Lisp programs will use temporary-file-directory 
> if
> they need this, would that be good enough?

I'm asking this because this is already what 
`inhibit-automatic-native-compilation' does :

(defun comp-trampoline-compile (subr-name)
  "Synthesize compile and return a trampoline for SUBR-NAME."
  (let* ((lambda-list (comp-make-lambda-list-from-subr
                       (symbol-function subr-name)))
         ;; The synthesized trampoline must expose the exact same 
         ABI of
         ;; the primitive we are replacing in the function reloc 
         table.
         (form `(lambda ,lambda-list
                  (let ((f #',subr-name))
                    (,(if (memq '&rest lambda-list) #'apply 
                    'funcall)
                     f
                     ,@(cl-loop
                        for arg in lambda-list
                        unless (memq arg '(&optional &rest))
                        collect arg)))))
         ;; Use speed 1 for compilation speed and not to optimize 
         away
         ;; funcall calls!
         (byte-optimize nil)
         (native-comp-speed 1)
         (lexical-binding t))
    (comp--native-compile
     form nil
     ;; If we've disabled nativecomp, don't write the trampolines 
     to
     ;; the eln cache (but create them).
     (unless inhibit-automatic-native-compilation
       (cl-loop
        for dir in (if native-compile-target-directory
                       (list (expand-file-name 
                       comp-native-version-dir
                                               native-compile-target-directory))
                     (comp-eln-load-path-eff))
        for f = (expand-file-name
                 (comp-trampoline-filename subr-name)
                 dir)
        unless (file-exists-p dir)
          do (ignore-errors
               (make-directory dir t)
               (cl-return f))
        when (file-writable-p f)
          do (cl-return f)
        finally (error "Cannot find suitable directory for output 
        in \
`native-comp-eln-load-path'"))))))

The line :

(unless inhibit-automatic-native-compilation

Could be replaced with :

(unless (equal comp-enable-subr-trampolines 
'compile-but-no-output)

To the same effect (the current effect of 
`inhibit-automatic-native-compilation', with which we haven't had 
issues so far).

As a side note, Lars' comment seems to say that it is completely 
possible to attach a trampoline to a primitive without saving said 
trampoline anywhere (not even in `temporary-file-directory').

This is supported by the fact the *optional* outputting of the 
trampoline to the file system is done by the function 
`comp--native-compile', with last argument set to a filename, 
while the association of this trampoline with the primitive is 
done by the (primitive) function `comp--trampoline-install'. So 
these two actions (outputting and attaching) are independent.

I completely agree that the last argument of 
`comp--native-compile' could be documented, particularly what 
exactly happens when it is set to nil.

Best,

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Fri, 10 Feb 2023 09:04:36 +0100
> 
> 
> Le jeudi 9 février 2023 à 09:52, Eli Zaretskii <eliz@gnu.org> a 
> écrit :
> 
> > Are you saying that it is not selective enough, and that in
> > some cases you needed to disable only trampolines for specific 
> > subrs?
> 
> This is already possible with the variable 
> `native-comp-never-optimize-functions'.
> 
> This is what we've used for the cases I've presented. For 
> projectile for instance :
> 
> (with-eval-after-load 'comp
>   (push 'insert-file-contents 
>   native-comp-never-optimize-functions))

Thanks, I'm aware of the variable, but didn't think its effects
included trampolines.  I guess we need to update the doc string.

Andrea, WDYT?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Aymeric Agon-Rambosson <aymeric.agon@yandex.com> writes:

> Le jeudi 9 février 2023 à 12:11, Eli Zaretskii <eliz@gnu.org> a écrit
> :
>
>>> Would it possible to add a constant value, like
>>> 'compile-but-no-output, that would have exactly the second
>>> behaviour I was mentioning before, that is compiling trampolines
>>> but not even trying to output them anywhere, by passing nil as the
>>> last argument of `comp--native-compile' ?
>>
>> Argh, the last argument to comp--native-compile is not documented.
>> AFAIR, setting it to nil means the trampoline will be written to
>> temporary-file-directory, right?  Because I don't think we can
>> compile
>> a trampoline without writing it to some file, from which we will
>> thereafter load it.  (Andrea, please correct me if I'm wrong.) If
>> so,
>> we could assume Lisp programs will use temporary-file-directory if
>> they need this, would that be good enough?
>
> I'm asking this because this is already what
> `inhibit-automatic-native-compilation' does :
>
> (defun comp-trampoline-compile (subr-name)
>  "Synthesize compile and return a trampoline for SUBR-NAME."
>  (let* ((lambda-list (comp-make-lambda-list-from-subr
>                       (symbol-function subr-name)))
>         ;; The synthesized trampoline must expose the exact same
>            ABI of
>         ;; the primitive we are replacing in the function reloc
>            table.
>         (form `(lambda ,lambda-list
>                  (let ((f #',subr-name))
>                    (,(if (memq '&rest lambda-list) #'apply
>                    'funcall)
>                     f
>                     ,@(cl-loop
>                        for arg in lambda-list
>                        unless (memq arg '(&optional &rest))
>                        collect arg)))))
>         ;; Use speed 1 for compilation speed and not to optimize
>            away
>         ;; funcall calls!
>         (byte-optimize nil)
>         (native-comp-speed 1)
>         (lexical-binding t))
>    (comp--native-compile
>     form nil
>     ;; If we've disabled nativecomp, don't write the trampolines
>        to
>     ;; the eln cache (but create them).
>     (unless inhibit-automatic-native-compilation
>       (cl-loop
>        for dir in (if native-compile-target-directory
>                       (list (expand-file-name
>                       comp-native-version-dir
>                                               native-compile-target-directory))
>                     (comp-eln-load-path-eff))
>        for f = (expand-file-name
>                 (comp-trampoline-filename subr-name)
>                 dir)
>        unless (file-exists-p dir)
>          do (ignore-errors
>               (make-directory dir t)
>               (cl-return f))
>        when (file-writable-p f)
>          do (cl-return f)
>        finally (error "Cannot find suitable directory for output
>        in \
> `native-comp-eln-load-path'"))))))
>
> The line :
>
> (unless inhibit-automatic-native-compilation
>
> Could be replaced with :
>
> (unless (equal comp-enable-subr-trampolines 'compile-but-no-output)
>
> To the same effect (the current effect of
> `inhibit-automatic-native-compilation', with which we haven't had
> issues so far).
>
> As a side note, Lars' comment seems to say that it is completely
> possible to attach a trampoline to a primitive without saving said
> trampoline anywhere (not even in `temporary-file-directory').

Unfortunatelly it is not, in posix to dynamically load code dlopen is
used and a file is necessary.  See DLOPEN(3)

Best Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
>> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
>> Date: Fri, 10 Feb 2023 09:04:36 +0100
>> 
>> 
>> Le jeudi 9 février 2023 à 09:52, Eli Zaretskii <eliz@gnu.org> a 
>> écrit :
>> 
>> > Are you saying that it is not selective enough, and that in
>> > some cases you needed to disable only trampolines for specific 
>> > subrs?
>> 
>> This is already possible with the variable 
>> `native-comp-never-optimize-functions'.
>> 
>> This is what we've used for the cases I've presented. For 
>> projectile for instance :
>> 
>> (with-eval-after-load 'comp
>>   (push 'insert-file-contents 
>>   native-comp-never-optimize-functions))
>
> Thanks, I'm aware of the variable, but didn't think its effects
> included trampolines.  I guess we need to update the doc string.
>
> Andrea, WDYT?

When a primitive is listed in that variable the generated code will not
call it directly, instead will go through the funcall machinery making
the trampoline unnecessary.  IIRC this will not apply to primitives for
which a dedicated op code exists already in bytecode (those are already
optimized by the byte compiler).

So yeah maybe we should be more verbose on that, but we must be careful
on the wording, primitives like + will still be not re-definable
reliably.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le vendredi 10 février 2023 à 16:53, Andrea Corallo <akrl@sdf.org> 
a écrit :

> Unfortunatelly it is not, in posix to dynamically load code 
> dlopen is
> used and a file is necessary.  See DLOPEN(3)

So what happens exactly when we pass nil as the last argument of 
`comp--native-compile' ? Is a temporary file created ?

>> The line :
>>
>> (unless inhibit-automatic-native-compilation
>>
>> Could be replaced with :
>>
>> (unless (equal comp-enable-subr-trampolines 
>> 'compile-but-no-output)

I'd say this is still possible anyway.

Best,

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Fri 10 Feb 2023 at 10:08AM +02, Eli Zaretskii wrote:

> But here we are talking about test suites provided by 3rd-party
> packages (not even Debian's code, AFAIU), and Makefile's they use.
> How is the correctness and/or elegance of that design is of any
> concern to us?  Or for Debian, for that matter?  I understand very
> well that it is convenient for Debian to be able to run those test
> suites without changing them too much, but I see no design issues
> here, nothing except for the convenience.  I definitely see no design
> issues that should bother us, the upstream project.

Well, let me try to put it in more general terms.

Using Emacs as part of Debian's package build toolchain is, I think, as
valid a use of Emacs as using it to write this e-mail message.  And for
that use case, what we need, in the most general terms, is some
mechanism which (i) doesn't require patching lots of third party
Makefiles to activate, and which (ii) avoids Emacs crashing in ways that
it didn't crash before we turned on native compilation.

Lars accepted these criteria and so he added the environment variable.

(i) is about design, not a matter of convenience, because a tool that
works as part of Debian's package build toolchain must not require piles
of patching of third party Makefiles.  Otherwise, it's a bug in that
element of our toolchain.  This is a point of view developed from our
years of experience of working on Debian, and I'm sure developers of
other distributions would agree.  Distribution toolchains are
fundamentally different to things like, say, GNU ELPA.  We have a
different relationship with the third party code than you do.

Currently, the only thing that satisfies both (i) and (ii) is the
environment variable.  We would be very happy to use something else, and
can help with testing it etc..

I understand that you don't want features in upstream Emacs for corner
cases.  I share this design goal with you.  I think, though, that there
are good reasons to think this is not a corner case, with Lars.
The majority of users of Emacs on GNU systems are probably using our
packages, and that requires a feature satisfying (i) and (ii).

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Aymeric Agon-Rambosson <aymeric.agon@yandex.com> writes:

> Le vendredi 10 février 2023 à 16:53, Andrea Corallo <akrl@sdf.org> a
> écrit :
>
>> Unfortunatelly it is not, in posix to dynamically load code dlopen
>> is
>> used and a file is necessary.  See DLOPEN(3)
>
> So what happens exactly when we pass nil as the last argument of
> `comp--native-compile' ? Is a temporary file created ?

Yes, and it is deleted just after the trampoline is loaded.  This is the
mechanism we are trying to get rid of.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: aymeric.agon@yandex.com,  monnier@iro.umontreal.ca,
>   emacs-devel@gnu.org,  akrl@sdf.org,  larsi@gnus.org,
>   rlb@defaultvalue.org
> Date: Fri, 10 Feb 2023 15:13:14 -0700
> 
> On Fri 10 Feb 2023 at 10:08AM +02, Eli Zaretskii wrote:
> 
> > But here we are talking about test suites provided by 3rd-party
> > packages (not even Debian's code, AFAIU), and Makefile's they use.
> > How is the correctness and/or elegance of that design is of any
> > concern to us?  Or for Debian, for that matter?  I understand very
> > well that it is convenient for Debian to be able to run those test
> > suites without changing them too much, but I see no design issues
> > here, nothing except for the convenience.  I definitely see no design
> > issues that should bother us, the upstream project.
> 
> Well, let me try to put it in more general terms.
> 
> Using Emacs as part of Debian's package build toolchain is, I think, as
> valid a use of Emacs as using it to write this e-mail message.  And for
> that use case, what we need, in the most general terms, is some
> mechanism which (i) doesn't require patching lots of third party
> Makefiles to activate, and which (ii) avoids Emacs crashing in ways that
> it didn't crash before we turned on native compilation.

I understand all that, and I'm nowhere near saying this is not a
legitimate use of Emacs: of course it is!  But Emacs cannot possibly
solve every legitimate problem OOTB which it could solve; solving some
problems does require some coding on the part of whoever must solve
the problem, and it is also legitimate for the upstream project to
expect that someone to write such code, instead of requesting Emacs to
solve the problem for them.

The bug report you posted, https://bugs.debian.org/1021842, is not a
crash, it is a failure which happens when a 3rd-party package is used.
I see no detailed analysis of the error in the bug report, so I can
only speculate what could be the reasons for the error:

  . buttercup was not updated to work with native-compilation (was
    this failure reported to the buttercup developers?)
  . Debian test harness uses buttercup incorrectly when
    native-compilation is enabled (e.g., it doesn't set up
    native-comp-eln-load-path to allow the trampolines to be produced)
  . the test that failed should only be run if Emacs was built without
    native-compilation

> Lars accepted these criteria and so he added the environment variable.

With all due respect to Lars, I disagree with that.  The record shows
that I disagreed from the start, back when Lars added the variable,
based on bitter experience we had with similar environment variables
in the past.  Since the variable was introduced and until now,
including this discussion, I've seen no new evidence indicating that
the variable is important enough to have, and nothing to make me
change my mind.  IMO, the dangers from using an environment variable
for these purposes still outweigh its usefulness.

> (i) is about design, not a matter of convenience, because a tool that
> works as part of Debian's package build toolchain must not require piles
> of patching of third party Makefiles.  Otherwise, it's a bug in that
> element of our toolchain.  This is a point of view developed from our
> years of experience of working on Debian, and I'm sure developers of
> other distributions would agree.  Distribution toolchains are
> fundamentally different to things like, say, GNU ELPA.  We have a
> different relationship with the third party code than you do.

That is understood and agreed upon, but then it is the job of Debian
to develop better tools.

> I understand that you don't want features in upstream Emacs for corner
> cases.  I share this design goal with you.  I think, though, that there
> are good reasons to think this is not a corner case, with Lars.
> The majority of users of Emacs on GNU systems are probably using our
> packages, and that requires a feature satisfying (i) and (ii).

We will have to agree to disagree on that, and this is final.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le samedi 11 février 2023 à 08:11, Andrea Corallo <akrl@sdf.org> a 
écrit :

>> So what happens exactly when we pass nil as the last argument 
>> of
>> `comp--native-compile' ? Is a temporary file created ?

> Yes, and it is deleted just after the trampoline is loaded. 
> This is the
> mechanism we are trying to get rid of.

What part exactly ? The ability to pass nil as last argument of 
`comp--native-compile', the deletion after the loading of the 
trampoline, or both ?

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, spwhitton@spwhitton.name,
>  monnier@iro.umontreal.ca, emacs-devel@gnu.org, larsi@gnus.org,
>  rlb@defaultvalue.org
> Date: Sat, 11 Feb 2023 11:06:45 +0100
> 
> 
> Le samedi 11 février 2023 à 08:11, Andrea Corallo <akrl@sdf.org> a 
> écrit :
> 
> >> So what happens exactly when we pass nil as the last argument 
> >> of
> >> `comp--native-compile' ? Is a temporary file created ?
> 
> > Yes, and it is deleted just after the trampoline is loaded. 
> > This is the
> > mechanism we are trying to get rid of.
> 
> What part exactly ? The ability to pass nil as last argument of 
> `comp--native-compile', the deletion after the loading of the 
> trampoline, or both ?

The deletion after loading part.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le samedi 11 février 2023 à 12:44, Eli Zaretskii <eliz@gnu.org> a 
écrit :

>> What part exactly ? The ability to pass nil as last argument of 
>> `comp--native-compile', the deletion after the loading of the 
>> trampoline, or both ?
>
> The deletion after loading part.

Well, if that's the case, passing nil as last argument of 
`comp--native-compile' would still work.

Being able to do (setq comp-enable-subr-trampolines 
'compile-but-no-output) would be quite practical. The temporary 
file would be created by `make-temp-file', according to the value 
of `temporary-file-directory', and we wouldn't have to worry about 
specifying some exact location ourselves (but being still able to 
so with the string value, as you've proposed, could prove useful 
in some cases as well).

Best,

Aymeric

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Sun, 12 Feb 2023 17:47:13 +0100
> 
> Being able to do (setq comp-enable-subr-trampolines 
> 'compile-but-no-output) would be quite practical. The temporary 
> file would be created by `make-temp-file', according to the value 
> of `temporary-file-directory', and we wouldn't have to worry about 
> specifying some exact location ourselves

Why is it a problem that you provide the location?
"/tmp/SOME-DIRECTORY" is always a possibility, and you can delete the
entire directory when you are done would then get rid of all the
leftovers, something that test runs and installations routinely do.
Why isn't this good enough?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le dimanche 12 février 2023 à 18:55, Eli Zaretskii <eliz@gnu.org> 
a écrit :

>> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
>> Cc: akrl@sdf.org, spwhitton@spwhitton.name, 
>> monnier@iro.umontreal.ca,
>>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
>> Date: Sun, 12 Feb 2023 17:47:13 +0100
>> 
>> Being able to do (setq comp-enable-subr-trampolines 
>> 'compile-but-no-output) would be quite practical. The temporary 
>> file would be created by `make-temp-file', according to the 
>> value 
>> of `temporary-file-directory', and we wouldn't have to worry 
>> about 
>> specifying some exact location ourselves
>
> Why is it a problem that you provide the location?
> "/tmp/SOME-DIRECTORY" is always a possibility, and you can 
> delete the
> entire directory when you are done would then get rid of all the
> leftovers, something that test runs and installations routinely 
> do.
> Why isn't this good enough?

If we have to provide the location, we have to ensure the 
directory exists. I guess it is not necessarily an issue, but I 
figured that if the semantic of nil as last argument of 
`comp--native-compile' and the underlying mechanism of temporary 
files is already there, and is supposed to stay, we might as well 
use it.

The current meaning of `inhibit-automatic-native-compilation' is a 
current "good" state that allowed a large number of build failures 
to disappear, and it would be reassuring to be able to reproduce 
that state exactly, before eventually migrating to providing the 
location ourselves, if we see that we can do so without errors.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Sun, 12 Feb 2023 20:58:15 +0100
> 
> If we have to provide the location, we have to ensure the 
> directory exists.

That has never been a problem.  There's any number of ways to ensure
that, whether from Emacs or outside it.

> The current meaning of `inhibit-automatic-native-compilation' is a 
> current "good" state that allowed a large number of build failures 
> to disappear

But it caused trouble in other situations, and is very hard to support
portably.  Any code that removes a .eln file that is being used by the
very Emacs session which removes it is building on non-portable
assumptions.

So I very much want to remove this part from Emacs.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Hello all,

at feature/inhibit-native-comp-cleanup I've pushed the branch I'm
testing that:

  . removes the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION environment
    variable
  . removes `inhibit-automatic-native-compilation' variable and uses
    `native-comp-deferred-compilation', with or without
    `comp-enable-subr-trampolines' (as needed), instead
  . adds a third possible value to `comp-enable-subr-trampolines', a
    string that specifies a directory in which to deposit the JIT
    compiled trampolines
  . updates the doc string of `native-comp-never-optimize-functions' to
    mention its effect on trampolines
  . updates some do doc here and there accordingly 


Few notes:

- I think renaming `native-comp-deferred-compilation' to
  `native-comp-jit-compilation' might be more descriptive to the user
  but I guess we are late now? :/

- If `comp-enable-subr-trampolines' is meant to be used by the user we
  should probably rename it into `native-comp-enable-subr-trampolines'?
  I guess we are late as well?

- I left the piece of code that, when removing an eln file on windows,
  guards for errors.  This to be extra cautious, I'm not aware of any
  exection path were we should remove an eln while loaded now, but maybe
  we could remove that piece of code only on the emacs-30 branch.

Please have a look, seems to work here.

Last question for my knowledge: what's the exact reason why we want a
third possible value for `comp-enable-subr-trampolines' and we can use
`native-comp-eln-load-path' to redirect the trampoline compilation?  Is
this to support packagers wanting to produce eln files only for packages
but not for trampolines?

Bests Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: emacs-devel@gnu.org,  Lars Ingebrigtsen <larsi@gnus.org>,  Stefan
>  Monnier <monnier@iro.umontreal.ca>,  Rob Browning <rlb@defaultvalue.org>
> Date: Mon, 13 Feb 2023 12:05:09 +0000
> 
> Hello all,
> 
> at feature/inhibit-native-comp-cleanup I've pushed the branch I'm
> testing that:
> 
>   . removes the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION environment
>     variable
>   . removes `inhibit-automatic-native-compilation' variable and uses
>     `native-comp-deferred-compilation', with or without
>     `comp-enable-subr-trampolines' (as needed), instead
>   . adds a third possible value to `comp-enable-subr-trampolines', a
>     string that specifies a directory in which to deposit the JIT
>     compiled trampolines
>   . updates the doc string of `native-comp-never-optimize-functions' to
>     mention its effect on trampolines
>   . updates some do doc here and there accordingly 

Thanks, I will take a look shortly.

> - I think renaming `native-comp-deferred-compilation' to
>   `native-comp-jit-compilation' might be more descriptive to the user
>   but I guess we are late now? :/
> 
> - If `comp-enable-subr-trampolines' is meant to be used by the user we
>   should probably rename it into `native-comp-enable-subr-trampolines'?
>   I guess we are late as well?

We can add new names, and make the old ones obsolete aliases.  Then we
could remove the obsolete aliases in some future release.

> - I left the piece of code that, when removing an eln file on windows,
>   guards for errors.  This to be extra cautious, I'm not aware of any
>   exection path were we should remove an eln while loaded now, but maybe
>   we could remove that piece of code only on the emacs-30 branch.

Agreed.

> Last question for my knowledge: what's the exact reason why we want a
> third possible value for `comp-enable-subr-trampolines' and we can use
> `native-comp-eln-load-path' to redirect the trampoline compilation?  Is
> this to support packagers wanting to produce eln files only for packages
> but not for trampolines?

That's one treason, AFAIR.  Another is that a single scalar variable
is easier to modify without screwing up than a list of directories,
especially since one of those directories must be the last one...

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: emacs-devel@gnu.org,  Lars Ingebrigtsen <larsi@gnus.org>,  Stefan
>>  Monnier <monnier@iro.umontreal.ca>,  Rob Browning <rlb@defaultvalue.org>
>> Date: Mon, 13 Feb 2023 12:05:09 +0000
>> 
>> Hello all,
>> 
>> at feature/inhibit-native-comp-cleanup I've pushed the branch I'm
>> testing that:
>> 
>>   . removes the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION environment
>>     variable
>>   . removes `inhibit-automatic-native-compilation' variable and uses
>>     `native-comp-deferred-compilation', with or without
>>     `comp-enable-subr-trampolines' (as needed), instead
>>   . adds a third possible value to `comp-enable-subr-trampolines', a
>>     string that specifies a directory in which to deposit the JIT
>>     compiled trampolines
>>   . updates the doc string of `native-comp-never-optimize-functions' to
>>     mention its effect on trampolines
>>   . updates some do doc here and there accordingly 
>
> Thanks, I will take a look shortly.

Thanks, feel free to suggest improvements or implement them directly if
you feel.

>> - I think renaming `native-comp-deferred-compilation' to
>>   `native-comp-jit-compilation' might be more descriptive to the user
>>   but I guess we are late now? :/
>> 
>> - If `comp-enable-subr-trampolines' is meant to be used by the user we
>>   should probably rename it into `native-comp-enable-subr-trampolines'?
>>   I guess we are late as well?
>
> We can add new names, and make the old ones obsolete aliases.  Then we
> could remove the obsolete aliases in some future release.

Cool, can we obsolete the old names already in emacs 29 or shall we do
it in 30?

Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: emacs-devel@gnu.org,  larsi@gnus.org,  monnier@iro.umontreal.ca,
>   rlb@defaultvalue.org
> Date: Mon, 13 Feb 2023 15:21:12 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> - I think renaming `native-comp-deferred-compilation' to
> >>   `native-comp-jit-compilation' might be more descriptive to the user
> >>   but I guess we are late now? :/
> >> 
> >> - If `comp-enable-subr-trampolines' is meant to be used by the user we
> >>   should probably rename it into `native-comp-enable-subr-trampolines'?
> >>   I guess we are late as well?
> >
> > We can add new names, and make the old ones obsolete aliases.  Then we
> > could remove the obsolete aliases in some future release.
> 
> Cool, can we obsolete the old names already in emacs 29 or shall we do
> it in 30?

We can do this in Emacs 29.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: emacs-devel@gnu.org,  larsi@gnus.org,  monnier@iro.umontreal.ca,
>>   rlb@defaultvalue.org
>> Date: Mon, 13 Feb 2023 15:21:12 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> - I think renaming `native-comp-deferred-compilation' to
>> >>   `native-comp-jit-compilation' might be more descriptive to the user
>> >>   but I guess we are late now? :/
>> >> 
>> >> - If `comp-enable-subr-trampolines' is meant to be used by the user we
>> >>   should probably rename it into `native-comp-enable-subr-trampolines'?
>> >>   I guess we are late as well?
>> >
>> > We can add new names, and make the old ones obsolete aliases.  Then we
>> > could remove the obsolete aliases in some future release.
>> 
>> Cool, can we obsolete the old names already in emacs 29 or shall we do
>> it in 30?
>
> We can do this in Emacs 29.

Great, pushed some more commits to the branch.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> at feature/inhibit-native-comp-cleanup I've pushed the branch I'm
> testing that:
>
>   . removes the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION environment
>     variable
>   . removes `inhibit-automatic-native-compilation' variable and uses
>     `native-comp-deferred-compilation', with or without
>     `comp-enable-subr-trampolines' (as needed), instead
>   . adds a third possible value to `comp-enable-subr-trampolines', a
>     string that specifies a directory in which to deposit the JIT
>     compiled trampolines
>   . updates the doc string of `native-comp-never-optimize-functions' to
>     mention its effect on trampolines
>   . updates some do doc here and there accordingly

The one thing which I think would help avoid surprises is:

when `comp-enable-subr-trampolines` is non-nil (and not a string) and
~/.emacs.d/eln-cache is not writable (e.g. because $HOME doesn't exist)
we should produce the trampolines somewhere in
`temporary-file-directory` (maybe by setting
`comp-enable-subr-trampolines` to an appropriate string)


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> at feature/inhibit-native-comp-cleanup I've pushed the branch I'm
>> testing that:
>>
>>   . removes the EMACS_INHIBIT_AUTOMATIC_NATIVE_COMPILATION environment
>>     variable
>>   . removes `inhibit-automatic-native-compilation' variable and uses
>>     `native-comp-deferred-compilation', with or without
>>     `comp-enable-subr-trampolines' (as needed), instead
>>   . adds a third possible value to `comp-enable-subr-trampolines', a
>>     string that specifies a directory in which to deposit the JIT
>>     compiled trampolines
>>   . updates the doc string of `native-comp-never-optimize-functions' to
>>     mention its effect on trampolines
>>   . updates some do doc here and there accordingly
>
> The one thing which I think would help avoid surprises is:
>
> when `comp-enable-subr-trampolines` is non-nil (and not a string) and
> ~/.emacs.d/eln-cache is not writable (e.g. because $HOME doesn't exist)
> we should produce the trampolines somewhere in
> `temporary-file-directory` (maybe by setting
> `comp-enable-subr-trampolines` to an appropriate string)
>
>
>         Stefan

Hi Stefan,

I'm positive about the concept of staying on the safe side on this, I'm
just not a big fan of leaving `comp-enable-subr-trampolines' value
mutated.  Dunno maybe we should just trying `temporary-file-directory'
as last resource in our search for a suitable place for a trampoline?

BR

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> I'm positive about the concept of staying on the safe side on this, I'm
> just not a big fan of leaving `comp-enable-subr-trampolines' value
> mutated.

I can relate to that :-)

> Dunno maybe we should just trying `temporary-file-directory' as last
> resource in our search for a suitable place for a trampoline?

Fine by me.  The main issue is that this dir can be "dangerous" so we
need to use `make-temp-file` or something equivalent.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> I'm positive about the concept of staying on the safe side on this, I'm
>> just not a big fan of leaving `comp-enable-subr-trampolines' value
>> mutated.
>
> I can relate to that :-)
>
>> Dunno maybe we should just trying `temporary-file-directory' as last
>> resource in our search for a suitable place for a trampoline?
>
> Fine by me.  The main issue is that this dir can be "dangerous" so we
> need to use `make-temp-file` or something equivalent.

Mmmh, we already use the `make-temp-file' machinery anyway for every
compilation, but to understand if this is sufficient or doesn't help at
all, I should understand more on why using `temporary-file-directory' is
dangerous and in which context.

Thanks

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Sat 11 Feb 2023 at 11:16AM +02, Eli Zaretskii wrote:

> The bug report you posted, https://bugs.debian.org/1021842, is not a
> crash, it is a failure which happens when a 3rd-party package is used.
> I see no detailed analysis of the error in the bug report, so I can
> only speculate what could be the reasons for the error:
>
>   . buttercup was not updated to work with native-compilation (was
>     this failure reported to the buttercup developers?)
>   . Debian test harness uses buttercup incorrectly when
>     native-compilation is enabled (e.g., it doesn't set up
>     native-comp-eln-load-path to allow the trampolines to be produced)
>   . the test that failed should only be run if Emacs was built without
>     native-compilation

I posted that particular bug because it's one that occurs precisely when
HOME is not writeable, and not otherwise, and the error output says that
the problem is not being able to write to HOME.  But it sounds like you
think someone who enables native comp is also responsible for setting up
trampoline redirection in the event that HOME is not writeable; i.e.,
the idea that Emacs should be able to do everything not explicitly
involving HOME without a writeable HOME doesn't hold once native-comp is
enabled.

>> I understand that you don't want features in upstream Emacs for corner
>> cases.  I share this design goal with you.  I think, though, that there
>> are good reasons to think this is not a corner case, with Lars.
>> The majority of users of Emacs on GNU systems are probably using our
>> packages, and that requires a feature satisfying (i) and (ii).
>
> We will have to agree to disagree on that, and this is final.

Yes, we'll have to agree to disagree.  We'll rename the variable to have
a DEBIAN_ prefix once Emacs 29 is out and our freeze is over, so that
people can assume it's not a feature they will find elsewhere, hopefully
to avoid any spurious bug reports upstream.

Thanks.

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Mmmh, we already use the `make-temp-file' machinery anyway for every
> compilation, but to understand if this is sufficient or doesn't help at
> all, I should understand more on why using `temporary-file-directory' is
> dangerous and in which context.

`temporary-file-directory' may point to a world-writable directory, so
it's vulnerable to the usual race condition where someone manages to
predict the name of the file you're going to write and places there
a symlink to some "interesting" place, so you end up overwriting some
other file unwittingly.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org,  Lars Ingebrigtsen
>  <larsi@gnus.org>,  Rob Browning <rlb@defaultvalue.org>
> Date: Mon, 13 Feb 2023 19:34:43 +0000
> 
> > when `comp-enable-subr-trampolines` is non-nil (and not a string) and
> > ~/.emacs.d/eln-cache is not writable (e.g. because $HOME doesn't exist)
> > we should produce the trampolines somewhere in
> > `temporary-file-directory` (maybe by setting
> > `comp-enable-subr-trampolines` to an appropriate string)
> 
> I'm positive about the concept of staying on the safe side on this, I'm
> just not a big fan of leaving `comp-enable-subr-trampolines' value
> mutated.  Dunno maybe we should just trying `temporary-file-directory'
> as last resource in our search for a suitable place for a trampoline?

If we do this, any reasons not to do that also for any other .eln
files, not just trampolines?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> If we do this, any reasons not to do that also for any other .eln
> files, not just trampolines?

Generating&using trampolines is necessary for correct execution.
In contrast other `.eln` affect only performance.

Generating files in `temporary-file-directory` is definitely not ideal,
so I'd rather limit it to the cases where it's necessary for correctness.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[tomas]

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

On Mon, Feb 13, 2023 at 03:57:03PM -0700, Sean Whitton wrote:
> Hello,
> 
> On Sat 11 Feb 2023 at 11:16AM +02, Eli Zaretskii wrote:

[...]

> > We will have to agree to disagree on that, and this is final.
> 
> Yes, we'll have to agree to disagree. [...]

This is sad.

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> If we do this, any reasons not to do that also for any other .eln
>> files, not just trampolines?
>
> Generating&using trampolines is necessary for correct execution.
> In contrast other `.eln` affect only performance.
>
> Generating files in `temporary-file-directory` is definitely not ideal,
> so I'd rather limit it to the cases where it's necessary for correctness.

+1

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Mmmh, we already use the `make-temp-file' machinery anyway for every
>> compilation, but to understand if this is sufficient or doesn't help at
>> all, I should understand more on why using `temporary-file-directory' is
>> dangerous and in which context.
>
> `temporary-file-directory' may point to a world-writable directory, so
> it's vulnerable to the usual race condition where someone manages to
> predict the name of the file you're going to write and places there
> a symlink to some "interesting" place, so you end up overwriting some
> other file unwittingly.

Okay thanks for the explaination, should be easy to handle then, I'll
try to implement it as soon as I've a bit of time.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le dimanche 12 février 2023 à 22:09, Eli Zaretskii <eliz@gnu.org> 
a écrit :

> But it caused trouble in other situations, and is very hard to 
> support
> portably.  Any code that removes a .eln file that is being used 
> by the
> very Emacs session which removes it is building on non-portable
> assumptions.
>
> So I very much want to remove this part from Emacs.

I probably wasn't very clear.

From what I understand, passing nil as last argument to 
`comp--native-compile' only means that a temporary file will be 
created in `temporary-file-directory' to store the output of the 
native compilation process.

On top of that, in the current state of emacs, when the result of 
this native compilation is a trampoline, this temporary .eln file 
is deleted after the trampoline is loaded in memory to be 
associated with the primitive.

This last part (the deletion after loading) is what you want to 
get rid of, and I agree this is the sensible thing to do. To be 
clear, I was not asking that you keep that part.

But the first part (the creating of a temporary file in 
`temporary-file-directory') is completely independent from the 
last one, or am I missing something ? It should still be possible 
to output the result of a native compilation to a temporary file, 
without having to remove that file afterwards ?

So something like (setq comp-enable-subr-trampolines 
'compile-but-no-output) would *only* mean : output to a temporary 
file in `temporary-file-directory'. It wouldn't also mean "and 
remove it afterwards". This meaning is perfectly compatible with 
the removal of the deletion after loading logic.

This is approximately what (setq comp-enable-subr-trampolines 
temporary-file-directory) would mean, except the two are not 
exactly equivalent. In the first case, `temporary-file-directory' 
would be evaluated when the temporary file needs to be created, 
which would be a guarantee that the file creation always works. In 
the second case, since `temporary-file-directory' is evaluated at 
the time of the setq, we would need its value at the time of the 
eval to remain valid for the whole duration of the emacs 
session. The first one is preferable to us because it always works 
and respects any modification we might make to 
`temporary-file-directory' for whatever reason.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,
>
> On Sat 11 Feb 2023 at 11:16AM +02, Eli Zaretskii wrote:
>
>> The bug report you posted, https://bugs.debian.org/1021842, is not a
>> crash, it is a failure which happens when a 3rd-party package is used.
>> I see no detailed analysis of the error in the bug report, so I can
>> only speculate what could be the reasons for the error:
>>
>>   . buttercup was not updated to work with native-compilation (was
>>     this failure reported to the buttercup developers?)
>>   . Debian test harness uses buttercup incorrectly when
>>     native-compilation is enabled (e.g., it doesn't set up
>>     native-comp-eln-load-path to allow the trampolines to be produced)
>>   . the test that failed should only be run if Emacs was built without
>>     native-compilation
>
> I posted that particular bug because it's one that occurs precisely when
> HOME is not writeable, and not otherwise, and the error output says that
> the problem is not being able to write to HOME.  But it sounds like you
> think someone who enables native comp is also responsible for setting up
> trampoline redirection in the event that HOME is not writeable; i.e.,
> the idea that Emacs should be able to do everything not explicitly
> involving HOME without a writeable HOME doesn't hold once native-comp is
> enabled.

Hi Sean,

this should be fixed by the last commit in
feature/inhibit-native-comp-cleanup (ce4a066ed1e).

Please let us know if this solution works for you.

Thanks

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Mmmh, we already use the `make-temp-file' machinery anyway for every
>> compilation, but to understand if this is sufficient or doesn't help at
>> all, I should understand more on why using `temporary-file-directory' is
>> dangerous and in which context.
>
> `temporary-file-directory' may point to a world-writable directory, so
> it's vulnerable to the usual race condition where someone manages to
> predict the name of the file you're going to write and places there
> a symlink to some "interesting" place, so you end up overwriting some
> other file unwittingly.

Okay, ce4a066ed1e generates trampolines in a temporary directory if no
other option is viable (using the make-temp-file machinery to generate
the unpredictable name).

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Andrea Corallo <akrl@sdf.org>,  emacs-devel@gnu.org,  larsi@gnus.org,
>   rlb@defaultvalue.org
> Date: Mon, 13 Feb 2023 22:31:08 -0500
> 
> > If we do this, any reasons not to do that also for any other .eln
> > files, not just trampolines?
> 
> Generating&using trampolines is necessary for correct execution.
> In contrast other `.eln` affect only performance.
> 
> Generating files in `temporary-file-directory` is definitely not ideal,
> so I'd rather limit it to the cases where it's necessary for correctness.

If by this you mean that we should not allow
comp-enable-subr-trampolines to have a string value, meaning the
directory to place the trampolines, then I think we should, since that
can support use cases other than unwritable home directory.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> Date: Tue, 14 Feb 2023 06:17:19 +0100
> From: <tomas@tuxteam.de>
> 
> > On Sat 11 Feb 2023 at 11:16AM +02, Eli Zaretskii wrote:
> 
> [...]
> 
> > > We will have to agree to disagree on that, and this is final.
> > 
> > Yes, we'll have to agree to disagree. [...]
> 
> This is sad.

Not as sad as if we had to disagree to disagree.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Tue, 14 Feb 2023 11:36:15 +0100
> 
> >From what I understand, passing nil as last argument to 
> `comp--native-compile' only means that a temporary file will be 
> created in `temporary-file-directory' to store the output of the 
> native compilation process.

When will this be useful, and why the existing features don't already
support whatever use case you had in mind?

As people here pointed out just recently, if Emacs cannot write a .eln
file, it will still work correctly by using the corresponding .elc
file.  So why do we need a subtle feature of depositing *.eln files in
a temporary directory (which on many systems is ephemeral, or is
regularly purged)?

> So something like (setq comp-enable-subr-trampolines 
> 'compile-but-no-output) would *only* mean : output to a temporary 
> file in `temporary-file-directory'. It wouldn't also mean "and 
> remove it afterwards". This meaning is perfectly compatible with 
> the removal of the deletion after loading logic.

By the very nature of temporary files and directories created by
Emacs, you cannot know their exact name, and so putting a file there
would be almost completely useless.  So I'm asking when will this be
useful.  I'd like to avoid adding features whose usability is not well
understood and whose justification is not solid enough.  We have too
many subtle knobs in native-compilation already.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> > If we do this, any reasons not to do that also for any other .eln
>> > files, not just trampolines?
>> 
>> Generating&using trampolines is necessary for correct execution.
>> In contrast other `.eln` affect only performance.
>> 
>> Generating files in `temporary-file-directory` is definitely not ideal,
>> so I'd rather limit it to the cases where it's necessary for correctness.
>
> If by this you mean that we should not allow
> comp-enable-subr-trampolines to have a string value,

No, that's not what I mean (and I don't understand the connection you
see between what I wrote and what you wrote).

> meaning the directory to place the trampolines, then I think we
> should, since that can support use cases other than unwritable
> home directory.

I have no objection to the string value of
comp-enable-subr-trampolines`.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Tue 14 Feb 2023 at 11:29AM GMT, Andrea Corallo wrote:

> Hi Sean,
>
> this should be fixed by the last commit in
> feature/inhibit-native-comp-cleanup (ce4a066ed1e).
>
> Please let us know if this solution works for you.

Cool, thanks for the notification, will test and report back.

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Aymeric Agon-Rambosson]

Le mardi 14 février 2023 à 15:51, Eli Zaretskii <eliz@gnu.org> a 
écrit :

> When will this be useful, and why the existing features don't 
> already
> support whatever use case you had in mind?

The use case we want to have supported is for instance when there 
is no writable home directory, like in the bug report Sean posted 
in the other branch of this conversation.

> As people here pointed out just recently, if Emacs cannot write 
> a .eln
> file, it will still work correctly by using the corresponding 
> .elc
> file.

I could be wrong, but it does not seem to be the case for 
trampolines. In the current state of master, if no writable 
directory can be found for the output of the .eln file containing 
the trampoline, emacs errors out.

> So why do we need a subtle feature of depositing *.eln files in
> a temporary directory

The temporary directory ("/tmp/" on POSIX systems) can potentially 
be the only user-writable directory. So being able to write there 
as a last resort can guarantee that a trampoline compilation will 
not produce an error.

> (which on many systems is ephemeral, or is
> regularly purged)?

(In the case of POSIX systems at least, a program can still expect 
the content of /tmp to be preserved for the duration of its 
current invocation, according to 
https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s18.html). I 
admit I do not know the semantics of the equivalent for Windows.

> By the very nature of temporary files and directories created by
> Emacs, you cannot know their exact name, and so putting a file 
> there
> would be almost completely useless.  So I'm asking when will 
> this be
> useful.  I'd like to avoid adding features whose usability is 
> not well
> understood and whose justification is not solid enough.  We have 
> too
> many subtle knobs in native-compilation already.

The commit added by Andrea (ce4a066ed1e) on the 
inhibit-native-comp-cleanup branch implements the following 
behaviour : if no writable directory is found in either 
`native-comp-target-directory' or in the output of 
`comp-eln-load-path-eff', a filename in the directory 
`temporary-file-directory' is returned. This file is guaranteed to 
be creatable, which is what we wanted to ensure, and we know its 
exact name (you had a good point there). This is I guess 
equivalent to what I wanted to ensure with 'compile-but-no-output, 
so personnally I'm good. But what actually matters are the results 
of Sean's tests, of course.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Aymeric Agon-Rambosson <aymeric.agon@yandex.com>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> Date: Wed, 15 Feb 2023 23:39:19 +0100
> 
> 
> Le mardi 14 février 2023 à 15:51, Eli Zaretskii <eliz@gnu.org> a 
> écrit :
> 
> > When will this be useful, and why the existing features don't 
> > already
> > support whatever use case you had in mind?
> 
> The use case we want to have supported is for instance when there 
> is no writable home directory, like in the bug report Sean posted 
> in the other branch of this conversation.

That use case should be solved now by recent Andrea's work.

> The commit added by Andrea (ce4a066ed1e) on the 
> inhibit-native-comp-cleanup branch implements the following 
> behaviour : if no writable directory is found in either 
> `native-comp-target-directory' or in the output of 
> `comp-eln-load-path-eff', a filename in the directory 
> `temporary-file-directory' is returned. This file is guaranteed to 
> be creatable, which is what we wanted to ensure, and we know its 
> exact name (you had a good point there). This is I guess 
> equivalent to what I wanted to ensure with 'compile-but-no-output, 
> so personnally I'm good. But what actually matters are the results 
> of Sean's tests, of course.

OK, so let's consider this issue resolved, until and unless Sean
reports some problems with the solution.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

Hello,

On Tue 14 Feb 2023 at 11:29AM GMT, Andrea Corallo wrote:

> this should be fixed by the last commit in
> feature/inhibit-native-comp-cleanup (ce4a066ed1e).
>
> Please let us know if this solution works for you.

This patch does apply to Emacs 28.2, which is what Debian has, so
unfortunately I can't test it at present.

If you would like me to help test, can you let me know what to revert
and what to cherry-pick, please?

We currently have these nativecomp commits on top of 28.2 (hashes are
Debian's, so mostly meaningless to you):

* 2f28496d038..: Lars Ingebrigtsen 2022-10-03 Rename to 'inhibit-automatic-native-compilation'
* 654428b65ae..: Lars Ingebrigtsen 2022-10-03 Add 'inhibit-native-compilation'
* d91aca70c4f..: Eli Zaretskii 2022-11-10 Fix large core dumps from background processes
* 376678555b3..: Andrea Corallo 2022-10-19 Fix eln files not being generated when native-comp-async runs
* 65845dea956..: Andrea Corallo 2022-10-18 Avoid fork bomb caused by native compilation trampolines
* d2f60a22a18..: Andrea Corallo 2022-10-15 Avoid fork bomb caused by native compilation

-- 
Sean Whitton

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> Date: Thu, 16 Feb 2023 10:04:46 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
> 
> OK, so let's consider this issue resolved, until and unless Sean
> reports some problems with the solution.

Andrea, I think you can now merge this branch to emacs-29 when you
have time, and let's take it from there.

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Sean Whitton <spwhitton@spwhitton.name> writes:

> Hello,
>
> On Tue 14 Feb 2023 at 11:29AM GMT, Andrea Corallo wrote:
>
>> this should be fixed by the last commit in
>> feature/inhibit-native-comp-cleanup (ce4a066ed1e).
>>
>> Please let us know if this solution works for you.
>
> This patch does apply to Emacs 28.2, which is what Debian has, so
> unfortunately I can't test it at present.
>
> If you would like me to help test, can you let me know what to revert
> and what to cherry-pick, please?
>
> We currently have these nativecomp commits on top of 28.2 (hashes are
> Debian's, so mostly meaningless to you):
>
> * 2f28496d038..: Lars Ingebrigtsen 2022-10-03 Rename to 'inhibit-automatic-native-compilation'
> * 654428b65ae..: Lars Ingebrigtsen 2022-10-03 Add 'inhibit-native-compilation'
> * d91aca70c4f..: Eli Zaretskii 2022-11-10 Fix large core dumps from background processes
> * 376678555b3..: Andrea Corallo 2022-10-19 Fix eln files not being generated when native-comp-async runs
> * 65845dea956..: Andrea Corallo 2022-10-18 Avoid fork bomb caused by native compilation trampolines
> * d2f60a22a18..: Andrea Corallo 2022-10-15 Avoid fork bomb caused by native compilation

Hi Sean,

can't you just test feature/inhibit-native-comp-cleanup directy?  That
would be the safest and simpler option IMO.

Otherwise you have to revert probably 654428b65ae and 2f28496d038 and
cherry pick in order 1795839babc 5d0912f1445 and ce4a066ed1e from
feature/inhibit-native-comp-cleanup.  But this is a blind guess, I can't
guarantee it to work without trying it.

BR

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 16 Feb 2023 10:04:46 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: akrl@sdf.org, spwhitton@spwhitton.name, monnier@iro.umontreal.ca,
>>  emacs-devel@gnu.org, larsi@gnus.org, rlb@defaultvalue.org
>> 
>> OK, so let's consider this issue resolved, until and unless Sean
>> reports some problems with the solution.
>
> Andrea, I think you can now merge this branch to emacs-29 when you
> have time, and let's take it from there.
>
> Thanks.

Done now with d6e4f243720.

Best Regards

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: aymeric.agon@yandex.com,  spwhitton@spwhitton.name,
>   monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org,
>   rlb@defaultvalue.org
> Date: Fri, 17 Feb 2023 10:16:38 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Andrea, I think you can now merge this branch to emacs-29 when you
> > have time, and let's take it from there.
> >
> > Thanks.
> 
> Done now with d6e4f243720.

Thanks.  I've installed a few updates to the docs, please take a look
and tell me if I made any mistakes.

I have a question about this code from comp.el:

  (defun comp--trampoline-abs-filename (subr-name)
    "Return the absolute filename for a trampoline for SUBR-NAME."
    (cl-loop
     with dirs = (if (stringp native-comp-enable-subr-trampolines)
		     (list native-comp-enable-subr-trampolines)
		   (if native-compile-target-directory
		       (list (expand-file-name comp-native-version-dir
					       native-compile-target-directory))
		     (comp-eln-load-path-eff)))
     with rel-filename = (comp-trampoline-filename subr-name)
     for dir in dirs
     for abs-filename = (expand-file-name rel-filename dir)
     unless (file-exists-p dir)
       do (ignore-errors
	    (make-directory dir t)
	    (cl-return abs-filename))
     when (file-writable-p abs-filename)
       do (cl-return abs-filename)
     ;; Default to some temporary directory if no better option was
     ;; found.
     finally (cl-return
	      (expand-file-name
	       (make-temp-file-internal (file-name-sans-extension rel-filename)
					0 ".eln" nil)
	       temporary-file-directory))))

This seems to use native-comp-enable-subr-trampolines, if it is a
string, without expanding it relative to anything, which AFAIU means
it will be interpreted relative to the current buffer's
default-directory.  Is that indeed so, and if so, should we perhaps
interpret it relative to invocation-directory, like we do with
native-comp-eln-load-path?  Because which buffer is current when
trampoline compilation happens is anyone's guess, especially if that
happens in a sub-process.

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Sean Whitton]

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

Hello,

On Fri 17 Feb 2023 at 09:00AM GMT, Andrea Corallo wrote:

> can't you just test feature/inhibit-native-comp-cleanup directy?  That
> would be the safest and simpler option IMO.

Not really -- Debian filters out a lot of the source tree due to
disagreements with the FSF regarding software freedom, and we have our
other patches.  So trying it directly it wouldn't give much confidence
regarding the original bug filing.

> Otherwise you have to revert probably 654428b65ae and 2f28496d038 and
> cherry pick in order 1795839babc 5d0912f1445 and ce4a066ed1e from
> feature/inhibit-native-comp-cleanup.  But this is a blind guess, I can't
> guarantee it to work without trying it.

Those are the right ones!

So: commit ce4a066ed1e fixes Debian bug #1021842 without the env var.

Thanks.

-- 
Sean Whitton

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 869 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Sean Whitton <spwhitton@spwhitton.name>
> Cc: Eli Zaretskii <eliz@gnu.org>,  aymeric.agon@yandex.com,
>   monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org,
>   rlb@defaultvalue.org
> Date: Fri, 17 Feb 2023 09:42:29 -0700
> 
> > Otherwise you have to revert probably 654428b65ae and 2f28496d038 and
> > cherry pick in order 1795839babc 5d0912f1445 and ce4a066ed1e from
> > feature/inhibit-native-comp-cleanup.  But this is a blind guess, I can't
> > guarantee it to work without trying it.
> 
> Those are the right ones!
> 
> So: commit ce4a066ed1e fixes Debian bug #1021842 without the env var.

Thanks for testing.

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Tatsuya Kinoshita]

On 2023-02-17 at 09:42 -0700, Sean Whitton wrote:
> So: commit ce4a066ed1e fixes Debian bug #1021842 without the env var.

On 2023-02-14 at 11:32 +0000, Andrea Corallo wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > `temporary-file-directory' may point to a world-writable directory, so
> > it's vulnerable to the usual race condition where someone manages to
> > predict the name of the file you're going to write and places there
> > a symlink to some "interesting" place, so you end up overwriting some
> > other file unwittingly.
> 
> Okay, ce4a066ed1e generates trampolines in a temporary directory if no
> other option is viable (using the make-temp-file machinery to generate
> the unpredictable name).

> +   finally (cl-return
> +            (expand-file-name
> +             (make-temp-file-internal (file-name-sans-extension rel-filename)
> +                                      0 ".eln" nil)
> +             temporary-file-directory))))

Hmm, it seems using make-temp-file-internal with DIR-FLAG=0 which just
constructs a name and do not create the file like make-temp-name, so
there is a race condition as Stefan mentioned.  Is that really OK?

Thanks,

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: aymeric.agon@yandex.com,  spwhitton@spwhitton.name,
>>   monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org,
>>   rlb@defaultvalue.org
>> Date: Fri, 17 Feb 2023 10:16:38 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Andrea, I think you can now merge this branch to emacs-29 when you
>> > have time, and let's take it from there.
>> >
>> > Thanks.
>> 
>> Done now with d6e4f243720.
>
> Thanks.  I've installed a few updates to the docs, please take a look
> and tell me if I made any mistakes.
>
> I have a question about this code from comp.el:
>
>   (defun comp--trampoline-abs-filename (subr-name)
>     "Return the absolute filename for a trampoline for SUBR-NAME."
>     (cl-loop
>      with dirs = (if (stringp native-comp-enable-subr-trampolines)
> 		     (list native-comp-enable-subr-trampolines)
> 		   (if native-compile-target-directory
> 		       (list (expand-file-name comp-native-version-dir
> 					       native-compile-target-directory))
> 		     (comp-eln-load-path-eff)))
>      with rel-filename = (comp-trampoline-filename subr-name)
>      for dir in dirs
>      for abs-filename = (expand-file-name rel-filename dir)
>      unless (file-exists-p dir)
>        do (ignore-errors
> 	    (make-directory dir t)
> 	    (cl-return abs-filename))
>      when (file-writable-p abs-filename)
>        do (cl-return abs-filename)
>      ;; Default to some temporary directory if no better option was
>      ;; found.
>      finally (cl-return
> 	      (expand-file-name
> 	       (make-temp-file-internal (file-name-sans-extension rel-filename)
> 					0 ".eln" nil)
> 	       temporary-file-directory))))
>
> This seems to use native-comp-enable-subr-trampolines, if it is a
> string, without expanding it relative to anything, which AFAIU means
> it will be interpreted relative to the current buffer's
> default-directory.  Is that indeed so, and if so, should we perhaps
> interpret it relative to invocation-directory, like we do with
> native-comp-eln-load-path?  Because which buffer is current when
> trampoline compilation happens is anyone's guess, especially if that
> happens in a sub-process.
>
> Thanks.

Hi Eli,

yes your is a very good point.  I pushed c15bc91e1bf to address that,
please have a look.

Thanks

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Tatsuya Kinoshita <tats@debian.org> writes:

> On 2023-02-17 at 09:42 -0700, Sean Whitton wrote:
>> So: commit ce4a066ed1e fixes Debian bug #1021842 without the env var.
>
> On 2023-02-14 at 11:32 +0000, Andrea Corallo wrote:
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> > `temporary-file-directory' may point to a world-writable directory, so
>> > it's vulnerable to the usual race condition where someone manages to
>> > predict the name of the file you're going to write and places there
>> > a symlink to some "interesting" place, so you end up overwriting some
>> > other file unwittingly.
>> 
>> Okay, ce4a066ed1e generates trampolines in a temporary directory if no
>> other option is viable (using the make-temp-file machinery to generate
>> the unpredictable name).
>
>> +   finally (cl-return
>> +            (expand-file-name
>> +             (make-temp-file-internal (file-name-sans-extension rel-filename)
>> +                                      0 ".eln" nil)
>> +             temporary-file-directory))))
>
> Hmm, it seems using make-temp-file-internal with DIR-FLAG=0 which just
> constructs a name and do not create the file like make-temp-name, so
> there is a race condition as Stefan mentioned.  Is that really OK?

Mmhh, Stefan mentioned the case where the tmp file name is predicted.

Shouldn't make-temp-file-internal return a non predictable file name?
Otherwise what's the point of using make-temp-file in the first place if
the temporary name is predictable?

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Shouldn't make-temp-file-internal return a non predictable file name?

Nope.  It's less predictable but it's still predictable.

> Otherwise what's the point of using make-temp-file in the first place if
> the temporary name is predictable?

`make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
condition: if someone predicated the filename, we detect it atomically
and we try again.

You might like to check

    https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories


-- Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org
> Date: Sat, 18 Feb 2023 21:48:55 +0000
> 
> yes your is a very good point.  I pushed c15bc91e1bf to address that,
> please have a look.

Thanks.

Any comments to the documentation changes I did yesterday?

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Tatsuya Kinoshita]

On 2023-02-18 at 21:56 +0000, Andrea Corallo wrote:
> >> +            (expand-file-name
> >> +             (make-temp-file-internal (file-name-sans-extension rel-filename)
> >> +                                      0 ".eln" nil)
> >> +             temporary-file-directory))))
> >
> > Hmm, it seems using make-temp-file-internal with DIR-FLAG=0 which just
> > constructs a name and do not create the file like make-temp-name, so
> > there is a race condition as Stefan mentioned.  Is that really OK?
> 
> Mmhh, Stefan mentioned the case where the tmp file name is predicted.
> 
> Shouldn't make-temp-file-internal return a non predictable file name?
> Otherwise what's the point of using make-temp-file in the first place if
> the temporary name is predictable?

Imagine if a local attacker creates symlinks as the candidate names
before creating the file, though less predictable.

make-temp-name describes as follows:

> There is a race condition between calling `make-temp-name' and
> later creating the file, which opens all kinds of security holes.
> For that reason, you should normally use `make-temp-file' instead.

To create a temporary file in a secure fashion, use make-temp-file
to create a file, or use make-temp-file with DIR-FLAG to create a
subdirectory and then create a file in it.

Thanks,

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Shouldn't make-temp-file-internal return a non predictable file name?
>
> Nope.  It's less predictable but it's still predictable.
>
>> Otherwise what's the point of using make-temp-file in the first place if
>> the temporary name is predictable?
>
> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
> condition: if someone predicated the filename, we detect it atomically
> and we try again.
>
> You might like to check
>
>     https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories

Thanks for the pointer.

I'm still not really convinced we have a problem here with trampolines.
With `make-temp-file' we are really only choosing the filename and
suggesting it to libgccjit, this last one will perform the file
creation.  I'd be surprised if GCC does not handle this correctly, and
in case shouldn't this be a GCC bug?

OTOH on a slightly differnt subject and in light of this, I think we
should probably backport e6043641d30 into emacs-30, so that eln files
are created onace and only by libgccjit.  Eli WDYT?

Thanks

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org
>> Date: Sat, 18 Feb 2023 21:48:55 +0000
>> 
>> yes your is a very good point.  I pushed c15bc91e1bf to address that,
>> please have a look.
>
> Thanks.
>
> Any comments to the documentation changes I did yesterday?

Yes, reads clear and complete to me.

Thanks

  Andrea

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Andrea Corallo <akrl@sdf.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> Shouldn't make-temp-file-internal return a non predictable file name?
>>
>> Nope.  It's less predictable but it's still predictable.
>>
>>> Otherwise what's the point of using make-temp-file in the first place if
>>> the temporary name is predictable?
>>
>> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
>> condition: if someone predicated the filename, we detect it atomically
>> and we try again.
>>
>> You might like to check
>>
>>     https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories
>
> Thanks for the pointer.
>
> I'm still not really convinced we have a problem here with trampolines.
> With `make-temp-file' we are really only choosing the filename and
> suggesting it to libgccjit, this last one will perform the file
> creation.  I'd be surprised if GCC does not handle this correctly, and
> in case shouldn't this be a GCC bug?
>
> OTOH on a slightly differnt subject and in light of this, I think we
> should probably backport e6043641d30 into emacs-30, so that eln files
> are created onace and only by libgccjit.  Eli WDYT?

And BTW, probably also in emacs-28 if we are doing another release.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Tatsuya Kinoshita <tats@debian.org>,  emacs-devel@gnu.org,
>   spwhitton@spwhitton.name,  1021842@bugs.debian.org, Eli Zaretskii
>  <eliz@gnu.org>
> Date: Mon, 20 Feb 2023 09:03:34 +0000
> 
> OTOH on a slightly differnt subject and in light of this, I think we
> should probably backport e6043641d30 into emacs-30, so that eln files
> are created onace and only by libgccjit.  Eli WDYT?

You mean, from master to emacs-29, I guess?

What was the motivation for that change?  The log message doesn't
explain the problem it meant to solve in enough detail, it just says
something about creating the file twice?  How can that happen? who
creates the file the second time?

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org
> Date: Mon, 20 Feb 2023 09:14:13 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Andrea Corallo <akrl@sdf.org>
> >> Cc: monnier@iro.umontreal.ca,  emacs-devel@gnu.org,  larsi@gnus.org
> >> Date: Sat, 18 Feb 2023 21:48:55 +0000
> >> 
> >> yes your is a very good point.  I pushed c15bc91e1bf to address that,
> >> please have a look.
> >
> > Thanks.
> >
> > Any comments to the documentation changes I did yesterday?
> 
> Yes, reads clear and complete to me.

Thanks for reviewing.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: Tatsuya Kinoshita <tats@debian.org>,  emacs-devel@gnu.org,
>   spwhitton@spwhitton.name,  1021842@bugs.debian.org,  Eli Zaretskii
>  <eliz@gnu.org>
> Date: Mon, 20 Feb 2023 09:18:02 +0000
> 
> Andrea Corallo <akrl@sdf.org> writes:
> 
> > Stefan Monnier <monnier@iro.umontreal.ca> writes:
> >
> >>> Shouldn't make-temp-file-internal return a non predictable file name?
> >>
> >> Nope.  It's less predictable but it's still predictable.
> >>
> >>> Otherwise what's the point of using make-temp-file in the first place if
> >>> the temporary name is predictable?
> >>
> >> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
> >> condition: if someone predicated the filename, we detect it atomically
> >> and we try again.
> >>
> >> You might like to check
> >>
> >>     https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories
> >
> > Thanks for the pointer.
> >
> > I'm still not really convinced we have a problem here with trampolines.
> > With `make-temp-file' we are really only choosing the filename and
> > suggesting it to libgccjit, this last one will perform the file
> > creation.  I'd be surprised if GCC does not handle this correctly, and
> > in case shouldn't this be a GCC bug?
> >
> > OTOH on a slightly differnt subject and in light of this, I think we
> > should probably backport e6043641d30 into emacs-30, so that eln files
> > are created onace and only by libgccjit.  Eli WDYT?
> 
> And BTW, probably also in emacs-28 if we are doing another release.

We don't want to make any enhancements in Emacs 28.3, just to plug the
security hole.  People who want enhancements will need to wait for
Emacs 29.1, hopefully not too long.

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

>> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
>> condition: if someone predicated the filename, we detect it atomically
>> and we try again.
>>
>> You might like to check
>>
>>     https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories
>
> Thanks for the pointer.
>
> I'm still not really convinced we have a problem here with trampolines.
> With `make-temp-file' we are really only choosing the filename and
> suggesting it to libgccjit, this last one will perform the file
> creation.

The important part is the use of `O_EXCL | O_CREAT` when creating
the file.
*BUT* `O_EXCL | O_CREAT` will fail if the file already exists.  Which is
why `make-temp-file` needs `make-temp-name` to generate new names until
we find one that really doesn't exist (not just at the time
`make-temp-name` is called but the fraction of a millisecond later when
we do try to create it).

> I'd be surprised if GCC does not handle this correctly, and
> in case shouldn't this be a GCC bug?

I'd be surprised.  If you tell it to write to a pre-existing file, does
it fail with an error?  If not, then I think it can't be used safely unless
*you* pre-create the file (e.g. with `make-temp-file`).


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: Tatsuya Kinoshita <tats@debian.org>,  emacs-devel@gnu.org,
>>   spwhitton@spwhitton.name,  1021842@bugs.debian.org, Eli Zaretskii
>>  <eliz@gnu.org>
>> Date: Mon, 20 Feb 2023 09:03:34 +0000
>> 
>> OTOH on a slightly differnt subject and in light of this, I think we
>> should probably backport e6043641d30 into emacs-30, so that eln files
>> are created onace and only by libgccjit.  Eli WDYT?
>
> You mean, from master to emacs-29, I guess?

Yes

> What was the motivation for that change?  The log message doesn't
> explain the problem it meant to solve in enough detail, it just says
> something about creating the file twice?  How can that happen? who
> creates the file the second time?

Before e6043641d30 the file was created by Fmake_temp_file_internal and
afterwards overwritten by libgccjit.  So I guess one could remove the
file after the first creation and make it a link pointing to some other
file waiting for libgccjit to do its write.

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Before e6043641d30 the file was created by Fmake_temp_file_internal and
> afterwards overwritten by libgccjit.

Yes, that was good.

> So I guess one could remove the file after the first creation and make
> it a link pointing to some other file waiting for libgccjit to do
> its write.

"One" as in "an attacker"?  In `/tmp` an attacker should not be able to
do that because it's supposed to be using the sticky bit so that only
the owner of a file can remove it.


        Stefan

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race
>>> condition: if someone predicated the filename, we detect it atomically
>>> and we try again.
>>>
>>> You might like to check
>>>
>>>     https://wiki.sei.cmu.edu/confluence/display/c/FIO21-C.+Do+not+create+temporary+files+in+shared+directories
>>
>> Thanks for the pointer.
>>
>> I'm still not really convinced we have a problem here with trampolines.
>> With `make-temp-file' we are really only choosing the filename and
>> suggesting it to libgccjit, this last one will perform the file
>> creation.
>
> The important part is the use of `O_EXCL | O_CREAT` when creating
> the file.
> *BUT* `O_EXCL | O_CREAT` will fail if the file already exists.  Which is
> why `make-temp-file` needs `make-temp-name` to generate new names until
> we find one that really doesn't exist (not just at the time
> `make-temp-name` is called but the fraction of a millisecond later when
> we do try to create it).

We can't use this loop, we tipically pass a filename to be used to
libgccjit and we have no control after (also see my last comment).

>> I'd be surprised if GCC does not handle this correctly, and
>> in case shouldn't this be a GCC bug?
>
> I'd be surprised.

Surprised if it does or does not?

> If you tell it to write to a pre-existing file, does
> it fail with an error?

I believe it does not.

> If not, then I think it can't be used safely unless
> *you* pre-create the file (e.g. with `make-temp-file`).

Are we sure?

Also if I pre-create the file with make-temp-file can't someone just
replace it even more easily with the infamous link before libgccjit
comes in?

Thanks

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Eli Zaretskii]

> From: Andrea Corallo <akrl@sdf.org>
> Cc: monnier@iro.umontreal.ca,  tats@debian.org,  emacs-devel@gnu.org,
>   spwhitton@spwhitton.name,  1021842@bugs.debian.org
> Date: Mon, 20 Feb 2023 15:42:08 +0000
> 
> > You mean, from master to emacs-29, I guess?
> 
> Yes
> 
> > What was the motivation for that change?  The log message doesn't
> > explain the problem it meant to solve in enough detail, it just says
> > something about creating the file twice?  How can that happen? who
> > creates the file the second time?
> 
> Before e6043641d30 the file was created by Fmake_temp_file_internal and
> afterwards overwritten by libgccjit.  So I guess one could remove the
> file after the first creation and make it a link pointing to some other
> file waiting for libgccjit to do its write.

Then it's okay to cherry-pick it to emacs-29.  (I actually am not sure
why we didn't install it on emacs-29 to begin with, but never mind.)

Thanks.

Re: Finalizing 'inhibit-automatic-native-compilation'

[tomas]

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

On Mon, Feb 20, 2023 at 04:07:59PM +0000, Andrea Corallo wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> >>> `make-temp-name` uses `O_EXCL | O_CREAT` so as to close the race

[...]

> > *BUT* `O_EXCL | O_CREAT` will fail if the file already exists.  Which is
> > why `make-temp-file` needs `make-temp-name` to generate new names until
> > we find one that really doesn't exist (not just at the time
> > `make-temp-name` is called but the fraction of a millisecond later when
> > we do try to create it).
> 
> We can't use this loop, we tipically pass a filename to be used to
> libgccjit and we have no control after (also see my last comment).

I think the idea is to run the loop before passing the name to
libgccjit: i.e. make name and try to create an empty file until
success, after that, the file is ours (and due to the special
permissions in /tmp can't be taken away) -- then pass to libgccjit.

At least this is the "standard pattern" for this. Ligccjit should
just not be surprised that there is a file and simply overwrite
it.

> > If not, then I think it can't be used safely unless
> > *you* pre-create the file (e.g. with `make-temp-file`).
> 
> Are we sure?
> 
> Also if I pre-create the file with make-temp-file can't someone just
> replace it even more easily with the infamous link before libgccjit
> comes in?

Not if they are another user, and /tmp (or whatever temporary dir
it is) has the right permission bits set (that "sticky bit" on the
directory only allows the file owner to change the directory entry).

Cheers
-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Before e6043641d30 the file was created by Fmake_temp_file_internal and
>> afterwards overwritten by libgccjit.
>
> Yes, that was good.
>
>> So I guess one could remove the file after the first creation and make
>> it a link pointing to some other file waiting for libgccjit to do
>> its write.
>
> "One" as in "an attacker"?  In `/tmp` an attacker should not be able to
> do that because it's supposed to be using the sticky bit so that only
> the owner of a file can remove it.

Finally understood thanks!

I've installed 5d0b45cd67b on emacs-29 in order to use always
`make-temp-file'.

I think at the time I preferred Fmake_temp_file_internal not to call
into Lisp for some reason I can't remember now, anyway seems to work now
for me (just bootstrapped successfully).

  Andrea

Re: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrea Corallo <akrl@sdf.org>
>> Cc: monnier@iro.umontreal.ca,  tats@debian.org,  emacs-devel@gnu.org,
>>   spwhitton@spwhitton.name,  1021842@bugs.debian.org
>> Date: Mon, 20 Feb 2023 15:42:08 +0000
>> 
>> > You mean, from master to emacs-29, I guess?
>> 
>> Yes
>> 
>> > What was the motivation for that change?  The log message doesn't
>> > explain the problem it meant to solve in enough detail, it just says
>> > something about creating the file twice?  How can that happen? who
>> > creates the file the second time?
>> 
>> Before e6043641d30 the file was created by Fmake_temp_file_internal and
>> afterwards overwritten by libgccjit.  So I guess one could remove the
>> file after the first creation and make it a link pointing to some other
>> file waiting for libgccjit to do its write.
>
> Then it's okay to cherry-pick it to emacs-29.  (I actually am not sure
> why we didn't install it on emacs-29 to begin with, but never mind.)

I didn't install it at the time in emacs-29 as I thought this had
nothing to do with security.  Anyway this turned out not to be the best
solution, so after today's discussion in this thread I've installed
5d0b45cd67b into emacs-29 as should be the optimal fix.

Note there will be conflict in mergin 29 into 30 and we'll have to
accept the change in 29 (sorry never managed to get gitmerge.el working
here).

Best Regards

  Andrea

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

On Mon, Feb 20, 2023 at 11:02 AM Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> > So I guess one could remove the file after the first creation and make
> > it a link pointing to some other file waiting for libgccjit to do
> > its write.
>
> "One" as in "an attacker"?  In `/tmp` an attacker should not be able to
> do that because it's supposed to be using the sticky bit so that only
> the owner of a file can remove it.

Just to be clear, this condition should be checked before emacs is
willing to use the temporary directory in question.  No unprivileged
user should be able to overwrite a directory entry the uid of the
emacs process creates at any point in the path to the temporary file.

Lynn

Re: Finalizing 'inhibit-automatic-native-compilation'

[Stefan Monnier]

> Just to be clear, this condition should be checked before emacs is
> willing to use the temporary directory in question.  No unprivileged
> user should be able to overwrite a directory entry the uid of the
> emacs process creates at any point in the path to the temporary file.

AFAIK we usually consider it's up to the user/system to make sure this
is the case.


        Stefan

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Tatsuya Kinoshita]

On 2023-02-20 at 20:22 +0000, Andrea Corallo wrote:
> I've installed 5d0b45cd67b on emacs-29 in order to use always
> `make-temp-file'.

Please be careful with the difference between make-temp-file-internal
and make-temp-file.

> +++ b/lisp/emacs-lisp/comp.el
>              (expand-file-name
> -             (make-temp-file-internal (file-name-sans-extension rel-filename)
> -                                      0 ".eln" nil)
> +             (make-temp-file (file-name-sans-extension rel-filename) 0 ".eln"
> +                             nil)
>               temporary-file-directory))))

This second argument of make-temp-file should be nil, and expanding
against temporary-file-directory is already done by make-temp-file.

Thanks,

Re: Finalizing 'inhibit-automatic-native-compilation'

[Lynn Winebarger]

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

On Mon, Feb 20, 2023, 4:34 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > Just to be clear, this condition should be checked before emacs is
> > willing to use the temporary directory in question.  No unprivileged
> > user should be able to overwrite a directory entry the uid of the
> > emacs process creates at any point in the path to the temporary file.
>
> AFAIK we usually consider it's up to the user/system to make sure this
> is the case.


That seems like a pretty aggressive assumption for this functionality.

Lynn

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

Bug#1021842: Finalizing 'inhibit-automatic-native-compilation'

[Andrea Corallo]

Tatsuya Kinoshita <tats@debian.org> writes:

> On 2023-02-20 at 20:22 +0000, Andrea Corallo wrote:
>> I've installed 5d0b45cd67b on emacs-29 in order to use always
>> `make-temp-file'.
>
> Please be careful with the difference between make-temp-file-internal
> and make-temp-file.
>
>> +++ b/lisp/emacs-lisp/comp.el
>>              (expand-file-name
>> -             (make-temp-file-internal (file-name-sans-extension rel-filename)
>> -                                      0 ".eln" nil)
>> +             (make-temp-file (file-name-sans-extension rel-filename) 0 ".eln"
>> +                             nil)
>>               temporary-file-directory))))
>
> This second argument of make-temp-file should be nil, and expanding
> against temporary-file-directory is already done by make-temp-file.
>
> Thanks,

Hi Tatsuya,

thanks for checking, 68df9e5953c fix that.

Bests

  Andrea