search

pypa / installer

A low-level library for installing from a Python wheel distribution.
https://installer.readthedocs.io/
MIT License
123 stars 51 forks source link

Cope with invalid hash algorithms in RECORD #179

Open dimbleby opened 1 year ago

dimbleby commented 1 year ago

Fixes #178

I said there that I didn't have a fix to offer; but perhaps even a wrong fix is a better starting point - or anyway has more momentum - than no fix at all

dimbleby commented 1 year ago

I can't tell what the test failure at https://github.com/pypa/installer/actions/runs/4646834563/jobs/8680555289?pr=179 was complaining about

I pushed again in the hope of kicking a retry but apparently that needs manual approval

dimbleby commented 1 year ago

oh this

FAIL Required test coverage of 100% not reached. Total coverage: 99.82%

I find it hard to believe that windows 3.9 uniquely fails to get to the line in question, so not sure what to do about that

dimbleby commented 1 year ago

I pushed a commit that adds # pragma: no cover to that line, and notes that it is a false positive reported only on windows and python < 3.10. Say if you have a better idea!

pradyunsg commented 1 year ago

Thanks for filing this PR @dimbleby, and appreicate your patience on this as I went away from OSS for a month.

Say if you have a better idea!

The only reason I can think of is that sha is a valid algorithm name on those (the only check being undertaken is in algorithms_available so it has to be in that list/set). Consider renaming the algorithm to notavailable= or invalid= instead?

dimbleby commented 1 year ago

No worries, take your time!

if sha was somehow a valid algorithm name then the new test wouldn't pass at all, right?

pradyunsg commented 1 year ago

It would, if it's conditionally available on only certain platforms. 😬

dimbleby commented 1 year ago

@pradyunsg if sha was conditionally available only on certain platforms, then the new test would fail on those certain platforms - yes?

The test verifies that we get an error complaining that the algorithm name is invalid: I don't see how that test can pass without hitting the line of code in question. I really don't see how this can be anything other than coverage weirdness.