Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ABI version mismatches are not shown as a table if non-ABI3 symbols are present #124

Open
arcctgx opened this issue Nov 20, 2024 · 5 comments
Assignees
Labels
bug Something isn't working C:cli CLI components

Comments

@arcctgx
Copy link

arcctgx commented Nov 20, 2024

Hi,

First of all, thank you for making abi3audit. I found it very useful, especially that the process of creating ABI3-compatible wheels is not very well-documented.

As I was experimenting with abi3audit-0.0.19, I deliberately introduced two ABI3 errors into my C extension by adding this code:

// not in stable ABI
PySignal_SetWakeupFd(-1);
// not in stable ABI 3.7 - added in 3.10
PyErr_SetInterruptEx(1);

(None of these calls make sense for my code, the intention was to only trigger auditing errors.)

Then I built a wheel targeted for cp37, and ran abi3audit on it. JSON output lists both errors:

$ abi3audit ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl -R | jq
[21:38:53] 💁 ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: 1 extensions scanned; 1 ABI version mismatches and 1 ABI violations found                                                                             
{
  "specs": {
    "ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl": {
      "kind": "wheel",
      "wheel": [
        {
          "name": "_audio.abi3.so",
          "result": {
            "is_abi3": false,
            "is_abi3_baseline_compatible": false,
            "baseline": "3.7",
            "computed": "3.10",
            "non_abi3_symbols": [
              "PySignal_SetWakeupFd"
            ],
            "future_abi3_objects": {
              "PyErr_SetInterruptEx": "3.10"
            }
          }
        }
      ]
    }
  }
}

i.e. there are non_abi3_symbols and future_abi3_objects listed, as expected. But verbose mode does not show both types of errors in the tabular form:

$ abi3audit -v ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
[21:33:26] 👎 ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: _audio.abi3.so has non-ABI3 symbol
           ┏━━━━━━━━━━━━━━━━━━━━━━┓
           ┃ Symbol               ┃
           ┡━━━━━━━━━━━━━━━━━━━━━━┩
           │ PySignal_SetWakeupFd │
           └──────────────────────┘
           💁 ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: 1 extensions scanned; 1 ABI version mismatches and 1 ABI violations found

It does list both errors in the summary, but only prints a table with non-ABI3 symbols. A table with ABI version mismatches is not shown.

To experiment further I built another wheel with ABI version mismatch only. This time the table was shown in verbose mode, as expected:

$ abi3audit -v ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
[21:53:13] 👎 ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: _audio.abi3.so uses the Python 3.10 ABI, but is tagged for the Python 3.7 ABI                                                                         
           ┏━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━┓
           ┃ Symbol               ┃ Version ┃
           ┡━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━┩
           │ PyErr_SetInterruptEx │ 3.10    │
           └──────────────────────┴─────────┘
           💁 ARver-1.4.0.dev3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl: 1 extensions scanned; 1 ABI version mismatches and 0 ABI violations found

I expected an auditing tool to show every problem it detects when used in its verbose mode. But currently it appears that ABI version mismatches are not shown as a table if non-ABI3 symbols are present.

Is this behavior intended? Or am I using abi3audit wrong?

@woodruffw
Copy link
Member

Thanks for the detailed report @arcctgx! I greatly appreciate it.

That looks like a bug to me -- if there's an ABI3 version mismatch and --summary is enabled, we should always emit a table entry for it. I can try and root-cause that in the coming days.

@woodruffw woodruffw added the bug Something isn't working label Nov 20, 2024
@woodruffw woodruffw self-assigned this Nov 20, 2024
@woodruffw
Copy link
Member

Ah yeah, this looks like a silly early design choice: AuditResult.__rich_console__ only renders or one the other with ABI3 violations having higher precedence, and not both.

The thinking behind this was that ABI3 violations are more serious than version violations, and that the --summary output was mostly for visual inspection anyways. However, that's more confusing than necessary, so I'll look at fixing it.

@woodruffw woodruffw added the C:cli CLI components label Nov 20, 2024
@woodruffw
Copy link
Member

Relevant function:

def __rich_console__(self, console: Console, options: ConsoleOptions) -> RenderResult:
# Violating abi3 entirely is more "serious" than having the wrong abi3
# version, so we check for it first when deciding the Rich representation.
if self.non_abi3_symbols:
yield f"[red]:thumbs_down: [green]{self.so}[/green] has non-ABI3 symbols"
table = Table()
table.add_column("Symbol")
for sym in self.non_abi3_symbols:
table.add_row(sym.name)
yield table
elif self.computed > self.baseline:
yield (
f"[yellow]:thumbs_down: [green]{self.so}[/green] uses the Python "
f"[blue]{self.computed}[/blue] ABI, but is tagged for the Python "
f"[red]{self.baseline}[/red] ABI"
)
table = Table()
table.add_column("Symbol")
table.add_column("Version")
for obj in self.future_abi3_objects:
table.add_row(obj.symbol.name, str(obj.added))
yield table
else:
yield f"[green]:thumbs_up: {self.so}"

@arcctgx
Copy link
Author

arcctgx commented Nov 20, 2024

Now that was a quick RCA. 🙂

I forgot to mention that I also tested with older versions back to 0.0.13 and also saw the same thing. So it matches your observation about an early design choice.

@woodruffw
Copy link
Member

Now that was a quick RCA. 🙂

Detailed issues get quick triage 😄

And thanks for checking the older versions! That makes sense; this should be pretty simple to fix and I can try some things tonight or tomorrow most likely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C:cli CLI components
Projects
None yet
Development

No branches or pull requests

2 participants