bug#48598: 28.0.50; buffer-naming collisions involving bouncers in ERC

["J.P." <jp@neverwas.me>]

Hi Michael,

Michael Albinus <michael.albinus@gmx.de> writes:

> I'm now at erc 5.4.1.48598.0.20220425.270. Btw, it is a little bit
> tricky to decide which is the recent version, because you have two
> lines of patches: erc5.4.1.48598.* and erc 5.4.1.49860.*. The package
> manager always offers me to upgrade to the most recent 5.4.1.49860.*
> version, and I must pick then the most recent 5.4.1.48598.* version
> (hoping it is the proper decision). It might be more obvious to me if
> you could offer both erc-48598 and erc-49860 packages in parallel.

Thanks for the feedback on this. I've done as you suggest and am now
offering both in parallel. As a strange bonus, this has also helped
make the web UI usable:

  https://jpneverwas.gitlab.io/erc-tools/archive/

Not that you should care, but when the time comes to redo this setup for
actual use, I'll have to try and reassess whether the behavior we want
is really supported and sustainable. (Hopefully, the ELPA and/or
package.el people will be willing to advise.) At the moment, this
approach of using separate packages feels quite shaky, for example, with
how the shadowing of the built-in ERC happens or how a "main file"
matching the package name (such as erc-5xxxx.el) is expected at various
points. Such wrinkles will have to be ironed out because we'll be adding
packages for bugs in a mostly automated fashion, with the eventual goal
being to allow ordinary folks (some of whom may be new to Emacs) to try
these fixes and report back findings. It kills me that many people offer
to be guinea pigs, but as soon as you mention applying patches: poof!
They're gone.

> Indeed, test/Makefile fires an error then. I've pushed a fix to master,
> it shall work now with the file name erc-d-tests.el.
>
> [From your most recent email]
>
> I've poked around this problem. Looks like there is a better solution.
> If your test directory is not called test/lisp/erc/, but
> test/lisp/erc-tests/, you can locate there test packages with an
> arbitrary name, like erc-tests.el or erc-d-tests.el, which don't
> require a corresponding library under lisp/erc/. This is described in
> test/[file-organization.org], and it will work also with older
> Emacsen.

> I revert the aforementioned patch in test/Makefile therefore.

Thanks a lot for investigating. I was under the mistaken impression that
the -tests/ suffix was reserved for deeper libraries, like those under
lisp/emacs-lisp/. So, with this in mind, I guess it all comes down to
whether we want to

  a. keep the erc-scenarios/ subdir or change to a flat layout

  b. rename all scenarios using -tests.el, thus requiring a move to
     test/lisp/erc-tests/

For (a), staying with the status quo means that tests in erc-resources
are excluded from test-lisp-erc-inotify, though this seems irrelevant
because all scenarios-based tests are expensive anyway. At present, the
only inexpensive tests in that subdir reside in erc-d-self.el. But
having those wait around at most eight hours to run seems fine because
their natural place is alongside those expensive scenarios, I think. As
for actual potential downsides of keeping the subdir, one might be that
it makes ERC the odd man out. AFAICT, no other subdirs with ad hoc names
exist under test/lisp, and there's no mention in file-organization.org
(AFICT) of such names being supported.

For (b), keeping the status quo would mean existing references and
hyperlinks are preserved (a plus, IMO). Since, all tests are still
discovered without undergoing a move, I'm not sure if there's a clear
benefit to the latter other than (again) compliance with established
norms. Specifically, if people might be annoyed at having tests under a
plain test/lisp/erc/ live in files not suffixed by -tests.el, we should
probably appease them.

In the end, any combination is fine by me, really.

>> For now, I've moved it to test/lisp/erc/erc-scenarios/resources/erc-d/
>> and am artificially piggybacking on check-lisp-erc-erc-scenarios via
>> test/lisp/erc/erc-scenarios/erc-scenarios-meta.el, which does nothing
>> but load erc-d-self.el (as convoluted as that sounds).
>
> Where is this needed? I don't see any load of erc-scenarios-meta.el.
>
> And even if you need it somewhere, I believe it belongs into the
> resources/ subdirectory.

Right. The point was to ensure those tests in erc-d-self.el would be
discovered for the check-lisp-erc-erc-scenarios target and also during
the test-all-inotify job (check-expensive). But replacing that "meta"
file with the real thing (erc-d-self.el itself) works just as well and
avoids the indirection. If this is still problematic, I'm fine with
simply hiding erc-d-self.el under resources/ and never running it on
EMBA or just deleting it (along with all its resources).

>>   test/lisp/erc/
>>   ├── erc-tests.el
>>   ...
>>   └── erc-scenarios/
>>       ├── erc-scenarios-<foo>.el
>>       ├── erc-d.self.el                    <- formerly "meta"
>>       ...
>>       └── resources/
>>           ├── <foo>/...
>>           ...
>>           ├── erc-d/
>>           │   ├── erc-d.el
>>           │   ...
>>           │   ├── (erc-d-self.el)          <- gone
>>           │   └── resources/...            <- renamed
>>           └── erc-scenarios-common.el
>
> Looks OK to me except the location of erc-scenarios-meta.el.

For the sake of contrast, here's what the above would look like without
the subdir and with -tests.el everywhere (although these can be applied
independently):

   test/lisp/erc-tests/
   ├── erc-tests.el
   ...
   ├── erc-d-tests.el
   ├── erc-scenarios-<foo>-tests.el
   ...
   └── resources/
       ├── <foo>/...
       ...
       ├── erc-d/
       │   ├── erc-d.el
       │   ...
       │   └── resources/...
       └── erc-scenarios-common.el

> Well, there seems to be a cut'n'waste error in erc-services-tests.el. See
> this fix: [...]
>
> !       (erc-services-tests--auth-source-standard
>          #'erc--auth-source-search))))

Sadly typical on my part. Thanks. Is there any preference as to whether
these should stick around even though they're unused? (I had originally
planned on removing them.)

>> Sorry again for the packaging snafu. Despite all appearances, I really
>> do value your time, so please don't let this interfere (too much) with
>> whatever else is on your plate. You've already helped me so much, and I
>> owe you a ton!
>
> No need to sorry! That's what reviews are good for :-)

Definitely! (Always appreciated.)