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

Improve error message when .so file without .abi3. in it is specified #118

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

vadz
Copy link
Contributor

@vadz vadz commented Nov 7, 2024

This PR is mostly done to fix #117, but also improves another error message which I noticed while making it.

Unfortunately I don't know how to add tests for the CLI behaviour and there doesn't seem to be any existing ones, so I've only tested this manually but at least it does seem to work as expected:

$ python -m abi3audit foo.so
[17:50:51] 👎 processing error: 'foo.so' must contain '.abi3.' to be recognized as a shared object

vadz and others added 4 commits November 7, 2024 17:29
Say that the file path must contain `.abi3.` and check that this is
indeed the case in the corresponding test.
spec._extractor() raises ExtractorError if and when an error occurs and
not an InvalidSpec, so handle this exception in the except clause.

This ensures that we give the expected error instead of a full
backtrace when specifying a non-existing file, for example.
Call make_specs() explicitly and handle exceptions from it in order to
show the appropriate error message instead of letting argparse call it
implicitly. As mentioned in that module documentation:

> In general, the type keyword is a convenience that should only be used
> for simple conversions that can only raise one of the three supported
> exceptions. Anything with more interesting error-handling or resource
> management should be done downstream after the arguments are parsed.

And this is exactly what this commit does.

Closes pypa#117.
Signed-off-by: William Woodruff <[email protected]>
@woodruffw
Copy link
Member

Unfortunately I don't know how to add tests for the CLI behaviour and there doesn't seem to be any existing ones, so I've only tested this manually but at least it does seem to work as expected:

Yeah, unfortunately we have no real CLI tests at the moment. Testing locally is helpful, thanks!

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @vadz!

@woodruffw woodruffw enabled auto-merge (squash) November 7, 2024 17:02
@woodruffw woodruffw merged commit c4290c6 into pypa:main Nov 7, 2024
10 checks passed
@vadz vadz deleted the improve-non-abi3-so branch November 7, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't require abi3 in shared file names or at least make it clear that it is required in the error message
2 participants