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

Hi Michael,

Michael Albinus <[email protected]> writes:

> Btw, I found it hard to scan a bug report with so many different
> problems. Usually, it is more simple to discuss every problem in a
> separate bug report, in order not to lose control. Except, all of them
> are strongly dependent.

Thanks for the thorough remarks, and apologies for the "omnibus" bug
report. I have others pending as well that are actually subsets of this
one, and I really should have done likewise for all this auth-source
business (filed a new, "interdependent" report, that is). Next time,
let's hope I'll be more thoughtful about it.

> You have said somewhere there is an archive at
> <https://jpneverwas.gitlab.io/erc-tools/archive/>. I cannot access
> this URL. Is there another URL to be used?

Sorry for the confusion. That's a package.el-only endpoint without any
browsable HTML, i.e.,

  (push '("erc-tools" . "https://jpneverwas.gitlab.io/erc-tools/archive/")
        package-archives)

For future reference, the full patch set is available for browsing here:

  https://git.neverwas.me/repos/erc-tools/tree/bugs/48598/patches/wip

And is downloadable here:

  https://jpneverwas.gitlab.io/erc-tools/48598/patches.tar.gz

>> While it may be tempting to single out pass, this part from the doc
>> string for `auth-source-search' says that ignoring :max and returning at
>> most one result is totally acceptable:
>>
>>   :max N means to try to return at most N items (defaults to 1).
>>   More than N items may be returned, depending on the search and
>>   the backend.
>
> Indeed. In Tramp there are two calls of auth-source-search: One call in
> order to retrieve the password, and there I use ':max 1' explicitly (see
> `tramp-read-passwd'). And there is another function used for user/host
> name completion, not looking for the password, and there I use ':max
> most-positive-fixnum' (see `tramp-parse-auth-sources'). But the former
> case could refrain from specifying :max.

Wow, so my lack of communications skills strikes again! What I meant to
say was that from my reading of that doc string (basically the de facto
compliance spec), a *back end* ignoring :max is fair game. But I think
the way I wrote it gave the misleading impression I was saying fair game
from the querying client's perspective. But regardless, the tramp
examples are indeed helpful. So, thanks for those.

> Making auth-source-pass conform to the auth-source API would always be a
> good thing™. I don't know whether there exist already such a bug report,
> otherwise I recommend you to write a new report.

Agreed. Unless Ted or Damien have anything to add, I'm going to remove
support for pass from this patch series (at least for now, even though
my terrible hacks seem to make it gel well enough). I'll later open a
new bug report asking for clarification on the interface and possibly
include a patch for making auth-source-pass :max aware.

> Just follow the usual recommendation for ERT tests: make a temporary
> environment for the test (for example, create a temporary
> auth-source-pass store), run your test, remove test data (for example,
> remove the temporary auth-source-pass store). That's how the tests in
> auth-source-tests.el and auth-source-pass-tests.el are designed, IIRC.
>
> Since you don't want to test auth-source-pass functionality explicitly,
> you can of course use any other auth-source backend. The scenario is the
> same, and you could even provide several test functions like
> erc-tests--auth-source-netrc, erc-tests--auth-source-json, and alike.

Makes total sense. I believe I've ticked most of those boxes already
(having used auth-source-tests.el as a reference, as you wisely
recommend). I should have excerpted that work for highlighting instead
of dropping another biggish patch in that last email. Anyway, here is
the relevant file (in case you're curious), which I believe reflects the
approach you describe:

  https://git.neverwas.me/repos/erc-v3/tree/test/erc-services-tests.el?id=00ad7115#n468

>>       Currently, results are narrowed to the best candidate, and its
>>       secret is returned as a string for (relatively) immediate
>>       transmission. IOW, I don't think any obvious references to the
>>       discarded ones remain, if that matters.
>
> That sounds OK. Even better would be to use functions for the :secret
> token, instead of the secret strings themselves. And call that function
> when you need it.

Right, currently, outgoing messages are fully baked out before being
enqueued, and the queue itself is fully accessible. Keeping those quasi
thunks around would make more sense to prevent accidental leakage when
printing debug info and dumping to I/O logs. But that would require some
real redesign, which will become easier after a few ponderous
deprecations mature, so perhaps by Emacs 30.

Anyway, I can't thank you enough for taking the time to look at this
stuff. Definitely obliged.

J.P.