[BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Lin Jian]

Remember to cover the basics, that is, what you expected to happen and
what in fact did happen.  You don't know how to make a good report?  See

     https://orgmode.org/manual/Feedback.html#Feedback

Your bug report will be posted to the Org mailing list.
------------------------------------------------------------------------

Dear Org maintainers,

Currently, the .dir-locals.el file is included in the GNU ELPA
tarball[1], which causes this compilation error[2] when doing AOT native
compilation for Emacs lisp packages in NixOS.

Error: wrong-type-argument ("/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el" proper-list-p (org-edit-src-content-indentation . 0))
  mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode -0xf42c55d2510e41>))
  debug-early-backtrace()
  debug-early(error (wrong-type-argument "/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el" proper-list-p (org-edit-src-content-indentation . 0)))
  signal(wrong-type-argument ("/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el" proper-list-p (org-edit-src-content-indentation . 0)))
  comp--native-compile("/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el")
  batch-native-compile()
  command-line-1(("--eval" "(setq large-file-warning-threshold nil)" "--eval" "(setq byte-compile-error-on-warn nil)" "-f" "batch-native-compile" "/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el"))
  command-line()
  normal-top-level()
Wrong type argument: "/nix/store/<hash>-emacs-org-9.7.10/share/emacs/site-lisp/elpa/org-9.7.10/.dir-locals.el", proper-list-p, (#<symbol org-edit-src-content-indentation at 304> . 0)

We can workaround this by skipping native compilation for
.dir-locals.el.  However, I do not think .dir-locals.el has to be
included in the GNU ELPA tarball.  In addition, MELPA ignores[3] that
file by default.

Could you exclude .dir-locals.el from GNU ELPA tarball?  FYI,
:ignored-files[4] of GNU ELPA specification can be used to do this.

[1]: https://elpa.gnu.org/packages/org-9.7.11.tar
[2]: https://hydra.nixos.org/build/271406205/nixlog/1
[3]: https://github.com/melpa/melpa/blob/0c608bf895a3b5230b781662510e1326af17ea13/README.md?plain=1#L169-L170
[4]: https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/README?id=51936a29b693bfc5f4f92e365b485a7c547b2ac1#n175

Best wishes,
Lin Jian

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Ihor Radchenko]

Lin Jian <me@linj.tech> writes:

> Currently, the .dir-locals.el file is included in the GNU ELPA
> tarball[1], which causes this compilation error[2] when doing AOT native
> compilation for Emacs lisp packages in NixOS.

I do not see why anything is wrong on Org side.
.dir-locals.el is a perfectly valid file that may be created by users as
well. The fact that they are native-compiled is probably a bug in NixOS
or a missing feature in Emacs's native compilation (maybe it should skip
compiling dir locals automatically).

> We can workaround this by skipping native compilation for
> .dir-locals.el.  However, I do not think .dir-locals.el has to be
> included in the GNU ELPA tarball.  In addition, MELPA ignores[3] that
> file by default.
>
> Could you exclude .dir-locals.el from GNU ELPA tarball?  FYI,
> :ignored-files[4] of GNU ELPA specification can be used to do this.

Having dir locals file in the tarball can be useful for the users who
wish to edit Org mode's source code. We set a number of editing defaults
there that are employed across Org codebase. These defaults make our
life easier when users create patches by directly modifying Org mode code
they got via ELPA.

Not a bug.
Canceled.

I suggest you to report a bug or feature request to Emacs upstream about
native compiling .dir-locals.el.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Morgan Willcock]

Ihor Radchenko <yantar92@posteo.net> writes:

> Lin Jian <me@linj.tech> writes:
>
>> Currently, the .dir-locals.el file is included in the GNU ELPA
>> tarball[1], which causes this compilation error[2] when doing AOT native
>> compilation for Emacs lisp packages in NixOS.
>
> I do not see why anything is wrong on Org side.
> .dir-locals.el is a perfectly valid file that may be created by users as
> well. The fact that they are native-compiled is probably a bug in NixOS
> or a missing feature in Emacs's native compilation (maybe it should skip
> compiling dir locals automatically).

I believe it is up to the file to opt-out of compilation.

If the file is created with add-dir-local-variable there is a boiler
plate header inserted which opts out:

  ;;; Directory Local Variables            -*- no-byte-compile: t -*-
  ;;; For more information see (info "(emacs) Directory Variables")

>> We can workaround this by skipping native compilation for
>> .dir-locals.el.  However, I do not think .dir-locals.el has to be
>> included in the GNU ELPA tarball.  In addition, MELPA ignores[3] that
>> file by default.
>>
>> Could you exclude .dir-locals.el from GNU ELPA tarball?  FYI,
>> :ignored-files[4] of GNU ELPA specification can be used to do this.
>
> Having dir locals file in the tarball can be useful for the users who
> wish to edit Org mode's source code. We set a number of editing defaults
> there that are employed across Org codebase. These defaults make our
> life easier when users create patches by directly modifying Org mode code
> they got via ELPA.
>
> Not a bug.
> Canceled.
>
> I suggest you to report a bug or feature request to Emacs upstream about
> native compiling .dir-locals.el.

I don't think there is any upstream bug.  The solution would be to add
the missing header to the .dir-locals.el file.

-- 
Morgan Willcock

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Lin Jian]

Ihor Radchenko <yantar92@posteo.net> writes:

> Having dir locals file in the tarball can be useful for the users who
> wish to edit Org mode's source code. We set a number of editing defaults
> there that are employed across Org codebase. These defaults make our
> life easier when users create patches by directly modifying Org mode code
> they got via ELPA.

I thought including .dir-locals.el may be a mistake because there are
only 4 packages (auctex, el-get, helm and org) among all packages in
GNU(-devel) ELPA and MELPA generating native compilation error about
.dir-locals.el in NixOS.

Now I know that including .dir-locals.el in org is not a mistake.
Instead, it is intended.  I will change our code in NixOS to not compile
.dir-locals.el.

The no-byte-compile cookie Morgan mentions is very interesting.  There
may be more packages including a .dir-locals.el file in their release
tarballs but not generating error in NixOS because their .dir-locals.el
has this cookie set.  add-dir-local-variable adds this cookie only since
Emacs 29[1] so older .dir-locals.el does not have this cookie.  I think
adding it to .dir-locals.el of org is a good idea.

[1]: 6539eb05889c783d782f114d9c072208d3080561

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Ihor Radchenko]

Morgan Willcock <morgan@ice9.digital> writes:

> I believe it is up to the file to opt-out of compilation.

That's true, but .dir-locals is a special file. So, I expected Emacs to
treat it specially during compilation as well. To not break old,
no-longer-maintained packages, if nothing.

> If the file is created with add-dir-local-variable there is a boiler
> plate header inserted which opts out:
>
>   ;;; Directory Local Variables            -*- no-byte-compile: t -*-
>   ;;; For more information see (info "(emacs) Directory Variables")
> ...
> ...  The solution would be to add
> the missing header to the .dir-locals.el file.

I do not see any downside of adding this to .dir-locals.el file.
Would you mind submitting a patch?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Morgan Willcock]

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

Ihor Radchenko <yantar92@posteo.net> writes:

>> I believe it is up to the file to opt-out of compilation.
>
> That's true, but .dir-locals is a special file. So, I expected Emacs to
> treat it specially during compilation as well. To not break old,
> no-longer-maintained packages, if nothing.

I guess the counter argument would be that, if this file doesn't need to
compiled, and the error is that compilation of this file fails, then it
isn't a critical error.

>> If the file is created with add-dir-local-variable there is a boiler
>> plate header inserted which opts out:
>>
>>   ;;; Directory Local Variables            -*- no-byte-compile: t -*-
>>   ;;; For more information see (info "(emacs) Directory Variables")
>> ...
>> ...  The solution would be to add
>> the missing header to the .dir-locals.el file.
>
> I do not see any downside of adding this to .dir-locals.el file.
> Would you mind submitting a patch?

I've attached a patch.

(To be fully transparent, I have not verified that applying the patch
will prevent errors during native compilation of the ELPA package, but I
am pretty confident that the change is a correct one.)

-- 
Morgan Willcock

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-.dir-locals.el-Disable-compilation-of-.dir-locals.el.patch --]
[-- Type: text/x-diff, Size: 677 bytes --]

From 18e55680191e2c803b731750643d3e854134f1a5 Mon Sep 17 00:00:00 2001
From: Morgan Willcock <morgan@ice9.digital>
Date: Tue, 17 Sep 2024 18:09:08 +0100
Subject: [PATCH] .dir-locals.el: Disable compilation of .dir-locals.el

* .dir-locals.el (org-mode): Add 'no-byte-compile'.
---
 .dir-locals.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 9df10dfed..551a831f7 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -1,4 +1,4 @@
-;;; Directory Local Variables
+;;; Directory Local Variables            -*- no-byte-compile: t -*-
 ;;; For more information see (info "(emacs) Directory Variables")
 
 ((nil
-- 
2.39.5

Re: [BUG] What about excluding .dir-locals.el from GNU ELPA tarball?

[Ihor Radchenko]

Morgan Willcock <morgan@ice9.digital> writes:

>> I do not see any downside of adding this to .dir-locals.el file.
>> Would you mind submitting a patch?
>
> I've attached a patch.

Thanks!
Applied, onto bugfix, with amendments.
I changed the commit message adding TINYCHANGE cookie (I do not see your
FSF copyright assignment in our records), and linking to this thread.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c766ea34ec

I also added you to the list of contributors.
https://git.sr.ht/~bzg/worg/commit/209bcd0a
(You seem to have one more commit a few years ago, but you were not
listed on Worg before now)

Fixed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>