bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrew Whatson]

Hi,

The change to the treatment of the load parameter of
native-compile-async in commit 6781cd67 has exposed some problems with
how native compilation has been handled by the `straight.el` package
manager.

Straight calls `(native-compile-async path-to-pkg 'recursively 'late)`
to trigger compilation of packages, and relies on
`comp-deferred-compilation-black-list` to prevent compilation of
packages when requested by the user.

The reasoning behind using the late is two-fold:

1) When compilation is requested, the package has not been loaded.
Once async compilation has finished, it may have been loaded.  We want
to load the native elisp if the package has been loaded, but otherwise
not force the package to load.

It seems that `late` loading provided this behaviour thanks to
`comp--late-register-subr` checking `comp-deferred-pending-h` before
loading the native definition.  The hash entry will be missing for
packages where we requested compilation but were never loaded.

Relying on undocumented behaviour of the `late` flag like is obviously
fragile and naughty, but I believe this is a valid use-case.  What do
you think about adding official support for this kind of "lazy"
loading?

2) The `comp-deferred-compilation-black-list` is the only way that a
user can prevent compilation of an elisp file, and this blacklist is
only checked when compilation is requested with `late` loading.

While what straight.el (and now package.el) are doing is not
technically "deferred" compilation, I think it's useful to have a
single blacklist to prevent compilation of a file regardless of how
that compilation is requested.

Thanks & Cheers,
Andrew

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew Whatson <whatson@gmail.com> writes:

> Hi,
>
> The change to the treatment of the load parameter of
> native-compile-async in commit 6781cd67 has exposed some problems with
> how native compilation has been handled by the `straight.el` package
> manager.
>
> Straight calls `(native-compile-async path-to-pkg 'recursively 'late)`
> to trigger compilation of packages, and relies on
> `comp-deferred-compilation-black-list` to prevent compilation of
> packages when requested by the user.
>
> The reasoning behind using the late is two-fold:
>
> 1) When compilation is requested, the package has not been loaded.
> Once async compilation has finished, it may have been loaded.  We want
> to load the native elisp if the package has been loaded, but otherwise
> not force the package to load.
>
> It seems that `late` loading provided this behaviour thanks to
> `comp--late-register-subr` checking `comp-deferred-pending-h` before
> loading the native definition.  The hash entry will be missing for
> packages where we requested compilation but were never loaded.
>
> Relying on undocumented behaviour of the `late` flag like is obviously
> fragile and naughty, but I believe this is a valid use-case.  What do
> you think about adding official support for this kind of "lazy"
> loading?

Hi Andrew,

I think straight should call `async-native-compile' with the load
parameter set to nil and perform the conventional byte compilation.

In case the late load will prove to be necessary (bytecode is loaded
before native compilation has finished) should be set automatically by
the existent machinery.

> 2) The `comp-deferred-compilation-black-list` is the only way that a
> user can prevent compilation of an elisp file, and this blacklist is
> only checked when compilation is requested with `late` loading.
>
> While what straight.el (and now package.el) are doing is not
> technically "deferred" compilation, I think it's useful to have a
> single blacklist to prevent compilation of a file regardless of how
> that compilation is requested.

I think would be nicer to add like a selector parameter to
`async-native-compile', this being a regexp to be satisfied for
compilation.

The reason why we rely on `comp-deferred-compilation-black-list' for
deferred compilation is that the user indeed does not invoke a function
as this happen automatically.

So the proposal would be to have like:

`(defun native-compile-async (paths &optional recursively load selector)`

WDYT?

Thanks

  Andrea

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrew Whatson]

Andrea Corallo <akrl@sdf.org> wrote:
>
> I think straight should call `async-native-compile' with the load
> parameter set to nil and perform the conventional byte compilation.
>
> In case the late load will prove to be necessary (bytecode is loaded
> before native compilation has finished) should be set automatically by
> the existent machinery.

Ok great, that is much easier.

> I think would be nicer to add like a selector parameter to
> `async-native-compile', this being a regexp to be satisfied for
> compilation.
>
> The reason why we rely on `comp-deferred-compilation-black-list' for
> deferred compilation is that the user indeed does not invoke a function
> as this happen automatically.
>
> So the proposal would be to have like:
>
> `(defun native-compile-async (paths &optional recursively load selector)`
>
> WDYT?

Most users will be running with `comp-deferred-compilation` enabled,
even when using a package manager that pre-compiles their packages.
Using a selector to exclude files from the pre-compilation step won't
prevent them from being compiled later with deferred loading.
Straight has a `no-native-compile` option for package recipes to
explicitly prevent their compilation, and the deferred compilation
blacklist is the only way we can ensure this behaviour.  It's also
possible that a user has added their own entries to the blacklist and
would expect straight to respect those entries when compiling
packages.

I agree that the selector is an improvement to the recursive
compilation API, but in straight's case we would just use a selector
which checked against `comp-deferred-compilation-black-list`.  This
solution would still allow people to call `native-compile-async`
directly to bypass the blacklist, which might be desirable.  If you're
happy with this outcome, I think it's acceptable.

Thanks!

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew Whatson <whatson@gmail.com> writes:

> Andrea Corallo <akrl@sdf.org> wrote:
>>
>> I think straight should call `async-native-compile' with the load
>> parameter set to nil and perform the conventional byte compilation.
>>
>> In case the late load will prove to be necessary (bytecode is loaded
>> before native compilation has finished) should be set automatically by
>> the existent machinery.
>
> Ok great, that is much easier.
>
>> I think would be nicer to add like a selector parameter to
>> `async-native-compile', this being a regexp to be satisfied for
>> compilation.
>>
>> The reason why we rely on `comp-deferred-compilation-black-list' for
>> deferred compilation is that the user indeed does not invoke a function
>> as this happen automatically.
>>
>> So the proposal would be to have like:
>>
>> `(defun native-compile-async (paths &optional recursively load selector)`
>>
>> WDYT?
>
> Most users will be running with `comp-deferred-compilation` enabled,
> even when using a package manager that pre-compiles their packages.
> Using a selector to exclude files from the pre-compilation step won't
> prevent them from being compiled later with deferred loading.
> Straight has a `no-native-compile` option for package recipes to
> explicitly prevent their compilation, and the deferred compilation
> blacklist is the only way we can ensure this behaviour.  It's also
> possible that a user has added their own entries to the blacklist and
> would expect straight to respect those entries when compiling
> packages.
>
> I agree that the selector is an improvement to the recursive
> compilation API, but in straight's case we would just use a selector
> which checked against `comp-deferred-compilation-black-list`.  This
> solution would still allow people to call `native-compile-async`
> directly to bypass the blacklist, which might be desirable.  If you're
> happy with this outcome, I think it's acceptable.

Sounds like a plane, only two details:

- I'd prefer to have this selector parameter as a single regexp (not a
  list of regexps), as an interface for a function looks cleaner to me.

- I'd like to rename `comp-deferred-compilation-black-list' into
  `comp-deferred-compilation-deny-list'.  I'm not the ugest fan of these
  renames but OTOH IMO not doing that before merging wouldn't feel
  correct.

Sounds good?

Thanks

  Andrea

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrew Whatson]

Andrea Corallo <akrl@sdf.org> wrote:

> - I'd prefer to have this selector parameter as a single regexp (not a
>   list of regexps), as an interface for a function looks cleaner to me.

Could you take regexp *or* a predicate function on the filename?  That
would make straight's use-case a bit easier to implement.

> - I'd like to rename `comp-deferred-compilation-black-list' into
>   `comp-deferred-compilation-deny-list'.  I'm not the ugest fan of these
>   renames but OTOH IMO not doing that before merging wouldn't feel
>   correct.

No complaints here, renaming seems sensible and better sooner than later.

Cheers

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew Whatson <whatson@gmail.com> writes:

> Andrea Corallo <akrl@sdf.org> wrote:
>
>> - I'd prefer to have this selector parameter as a single regexp (not a
>>   list of regexps), as an interface for a function looks cleaner to me.
>
> Could you take regexp *or* a predicate function on the filename?  That
> would make straight's use-case a bit easier to implement.

Sounds good to me, will do.

Thanks

  Andrea

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew Whatson <whatson@gmail.com> writes:

> Andrea Corallo <akrl@sdf.org> wrote:
>
>> - I'd prefer to have this selector parameter as a single regexp (not a
>>   list of regexps), as an interface for a function looks cleaner to me.
>
> Could you take regexp *or* a predicate function on the filename?  That
> would make straight's use-case a bit easier to implement.
>
>> - I'd like to rename `comp-deferred-compilation-black-list' into
>>   `comp-deferred-compilation-deny-list'.  I'm not the ugest fan of these
>>   renames but OTOH IMO not doing that before merging wouldn't feel
>>   correct.
>
> No complaints here, renaming seems sensible and better sooner than later.

Hi Andrew,

Should be inplemented as discussed as of 7a8370ed0f.

Please have a try and see if works for you.

Thanks!

  Andrea

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrew Whatson]

Andrea Corallo <akrl@sdf.org> wrote:

> Should be inplemented as discussed as of 7a8370ed0f.
>
> Please have a try and see if works for you.

Hi Andrea,

Everything is working as expected.  Thanks for the fix and for all
your efforts on this awesome feature!

Cheers,
Andrew

bug#44813: 28.0.50; [feature/native-comp] Recent change to native-compile-async

[Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Andrew Whatson <whatson@gmail.com> writes:

> Andrea Corallo <akrl@sdf.org> wrote:
>
>> Should be inplemented as discussed as of 7a8370ed0f.
>>
>> Please have a try and see if works for you.
>
> Hi Andrea,
>
> Everything is working as expected.  Thanks for the fix and for all
> your efforts on this awesome feature!

Wonderful, closing then.

Thanks for the kind words :)

  Andrea