On Thu, Aug 26 2021, Taylor Blau wrote:
> On Thu, Aug 26, 2021 at 01:19:24AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> In 2f732bf15e6 (tr2: log parent process name, 2021-07-21) we started
>> logging parent process names, but only logged all parents on Windows.
>> on Linux only the name of the immediate parent process was logged.
>>
>> Extend the functionality added there to also log full parent chain on
>> Linux. In 2f732bf15e6 it was claimed that "further ancestry info can
>> be gathered with procfs, but it's unwieldy to do so.".
>>
>> I don't know what the author meant by that, but I think it probably
>> referred to needing to slurp this up from the FS, as opposed to having
>> an API.
>
> I don't think that this (specifically, "I don't know what the author
> meant by that") is necessary information to include in a patch message.
>
> If you're looking for a replacement (and you may not be, but just my
> $.02) I would suggest:
>
> "2f732bf15e6 does not log the full parent chain on Linux; implement
> that functionality here."
I hope Taylor doesn't mind me quoting it, but he sent me this follow-up
off-list:
For what it's worth, I really struggled to write this. What I was trying
to say was something along the lines of "let's give Emily a little more
credit for not doing this right off the bat", but I didn't want to write
that exactly, since I don't think it was your original intention.
At the very least, saying something to the effect of "I don't know what
the original author thought was so tough, here's a patch to implement
it" isn't helping anybody, so I tried to focus on that.
Anyway, just writing to you off-list to say that I do think you had good
intentions, but that what you wrote may be read differently by others in
a way that you didn't intend it to be.
First, thanks to Taylor for pointing this out, and second I'd like to
apologize for that comment.
More than some "sorry someone read it that way", this really does read
even to me, its author, shortly after having written it like something
that's way more on the side of sneakiness than a charitable comment.
I.e. like some snipe-y paraphrasing of "maybe they found this problem
too complex, but look how easy it is!" than something charitable that's
meant to barely pass under the radar.
Hopefully it helps that I'm honestly just being a bit of an
inconsiderate idiot here than actively malicious.
What I meant to accomplish here was to guide a reader of these patches
through the same mental states I went through when reading the original
patch.
I.e. I took it from its description that there were some unstated
special-cases in reading procfs that made it harder to deal with than
not. I think those comments were rather just a way to say that scraping
procfs was a bit of a pain, and the MVP in 2f732bf15e6 was good enough
for now, which is fair enough.
I rephrased those comments in v2 of this patch[1]. The summary starting
at the 4th paragraph ("It's possible given the semantics[...]") is an
edge case I hadn't considered when I wrote v1. I in turn copied that
"unweildy" comment from an even older version[2], where the difficulties
of parsing the "comm" field out of "/proc/*/stat" weren't apparent to
me.
I.e. I think if I had to describe that interface now I would describe it
as unwieldy, but wouldn't when I wrote v1[1] or that v0[2]. I.e. I think
there's no convenient way to get full atomic snapshot of the process
tree, so "give me the names of all my parents as an array" is definitely
somewhere between brittle and unwieldy, in particular considering the
hassle of parsing "/proc/*/stat" properly.
But I digress, which is also a problem I have with being overly verbose
sometimes and weaving enough rope to hang myself with.
The point isn't the difficulties of the procfs(5) interface, but that
particularly with a medium like the text-based communication we mainly
use in this project it behooves the sender to think not only about what
they're saying, but how what they're saying is going to be interpreted
by the recipient and bystanders alike.
I think this is a case where I clearly failed at that, sorry Emily, and
thanks Taylor for pointing this out to me.
1. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20210826T121820Z-avarab@xxxxxxxxx/
2. https://lore.kernel.org/git/87o8agp29o.fsf@xxxxxxxxxxxxxxxxxxx/
