search

osandov / drgn

Programmable debugger
Other
1.78k stars 165 forks source link

contrib: bpf_inspect: adjust syntax for python < 3.12 #412

Closed brenns10 closed 4 months ago

brenns10 commented 4 months ago

Hi, this is a totally optional PR, but I noticed that contrib/bpf_inspect.py uses nested f-string quotes that are only available in Python 3.12 or later. It would be nice to be able to run this on older Python versions. If @Asphaltt doesn't mind it's a simple tweak to make these work on the older versions.

Asphaltt commented 4 months ago

May you fix it after merging #408 ? Because it uses nested f-string quotes too.

brenns10 commented 4 months ago

That makes perfect sense! I'm happy to wait, I just wanted to post the PR since it was a patch I needed to include in my RPM builds for drgn on python 3.6-based Oracle Linux versions.

osandov commented 4 months ago

Ok, #408 is merged. Even though we don't want to enforce style guidelines on contrib, I wonder if we should at least check the syntax with vermin.

brenns10 commented 4 months ago

Even though we don't want to enforce style guidelines on contrib, I wonder if we should at least check the syntax with vermin.

Maybe, yeah. But I don't necessarily think that the contrib directory needs to have the same Python compatibility guarantees (3.6+) as the rest of drgn -- part of the goal is to lower the barrier to entry. Expecting compatibility with old (EOL...) python versions is exactly one of those barriers that we would probably like to avoid here?

I wasn't trying to require that this file or any other, e.g., be 3.6-compatible, but rather helpfully come around after the fact to make easy changes to broaden compatibility, since it wasn't intrusive.

As a distributor I'm happy to either downstream patch, or flat-out remove files from the contrib directory if they're not compatible with the Python I'm shipping on. It's kind of my job to make sure these things work :)

osandov commented 4 months ago

Yeah, I was hoping to avoid any requirements beyond the bare minimum for contrib, so I'm fine keeping it that way and accepting fixes after the fact.