This is a follow-up of bug#1069210[1] and bug#1076964[2].  The
implementation of supporting sub-directories for elpa packages has
mostly been released in dh-elpa except for the `load-path' handling.
The original approach of adding `.nosearch' sentinels in
`/usr/share/emacs/site-lisp/elpa{,-src}' was discovered to be broken[2]:
though it works for normal mode, in batch mode it will disable loading
path for all sub-directories from these two path.  This was reverted in
2.1.5.

I am now proposing another approach: only adding `.nosearch' sentinels
in any sub-directories from the root directory of a package, e.g. for
package `foo' with a sub-directory `bar', only add `.nosearch' under
`foo/bar' but no `foo'.  This was proven to work well using an elpafied
auctex package, and doesn't break any tests using ERT or Buttercup.

I have prepared a patch at [3] and also attached.  Please help review
and comment.  I'll merge it once it's approved.

[1] https://bugs.debian.org/1069210
[2] https://bugs.debian.org/1076964
[3] https://salsa.debian.org/manphiz/dh-elpa/-/compare/master...no-recursive-handling?from_project_id=18920

Hello Xiyue,

Please don't merge this yourself; let me or David do it.  Thanks.

Sean Whitton <spwhitton@spwhitton.name> writes:

Ack. Will wait for your reviews.  Thanks.

Xiyue Deng <manphiz@gmail.com> writes:

Friendly ping for comments.

Xiyue Deng <manphiz@gmail.com> writes:

No review yet, but what have you done for testing?

Hi David,

David Bremner <david@tethera.net> writes:

I have tested running the previously failed tests, e.g. clojure-mode
which uses buttercup and flycheck which uses both buttercup and ERT, and
they are now passing using the new implementation.

Xiyue Deng <manphiz@gmail.com> writes:

Build failures are (relatively) fine. What I really want to avoid is
having to do a mass rebuild to fix maintainer scripts (or worse, to have
broken maintainer scripts shipped in stable). So we need to take some
extra care and attention with changes to dh-elpa.

Hi David,

David Bremner <david@tethera.net> writes:

Make sense.  AIUI rebuild is not necessary for enabling this because
currently there is no package depending on this feature, and before and
after this patch existing package will behave the same.  It is only
making a difference when a package has source code in a sub-directory
and this patch will disable loading that sub-directory into `load-path'.
Currently AFAIK only auctex has this issue, and it's not yet elpafied so
we are good.

Xiyue Deng <manphiz@gmail.com> writes:

Right, I'm just being extra paranoid (and not investingating very much
yet) about something breaking in the maintainer scripts via this change.

David Bremner <david@tethera.net> writes:

Understood.  The addition to the install and remove scripts is
independent and doesn't touch other parts and should not break any
existing logic.  But do let me know if I missed anything.

Xiyue Deng <manphiz@gmail.com> writes:

Friendly ping.

Xiyue Deng <manphiz@gmail.com> writes:

Friendly ping.  Would really like to get this in before Trixie so that
dh-elpa is compatible with upstream ELPA.

Xiyue Deng <manphiz@gmail.com> writes:

Friendly ping.  Would really like to make it in before Trixie freeze.

Xiyue Deng <manphiz@gmail.com> writes:

Is this an RC bug? because that's about all I have time for at the moment.

Hi David,

David Bremner <david@tethera.net> writes:
packages problematic - e.g. auctex which I have been trying to.

Also, this implementation has been around for a few months and I have
tested it using packages with ERT or Buttercup tests and they have been
working fine.  I would still suggest to merge this and I'm committed to
fix any corner issues.

Xiyue Deng <manphiz@gmail.com> writes:

I wonder about quoting here. I tend to overquote in shell scripts, but
what if some subdirectory has whitespace in the name?

I think it's better to use find -delete here?

In theory, I think an addon package could ship subdirs.el. In practice
this doesn't seem to happen much (at least I did not find any examples
cases). How should we handle this? Add a check? Ignore the possibility?
Does the emacs packaging documentation have anything to say about this?

d

Hi David,

David Bremner <david@tethera.net> writes:

Good point.  Now quoted all variables (including in helper/remove)

Doing "-exec rm {} \;" was just out of habit as it seems to be supported
since always.  Now changed to "-type f -delete" trying to be more safe.

Actually I think you bring up a good point that I also didn't get a
clear answer in my previous query to emacs-devel[1]: if a package want
to add a subdir to `load-path', how to do it?  Currently we don't see
any example, and the only package that don't want to load a subdir is
auctex, which I tried to ITS but the original maintainer just came back
and will take care of it himself[2], so I guess there is currently
nothing to do at dh-elpa's side.

So for now I think it's OK to let this bug wait until after Trixie, but
keep it open and observe its development (also pushed the changes to
no-recursive-handling branch for tracking).  Will follow up should any
use-case comes up.

(And it looks like waiting is not a bad strategy after all :)

[1] https://lists.gnu.org/archive/html/emacs-devel/2024-05/msg00658.html
[2] https://bugs.debian.org/1094224