Include version and checksum validation when updating packages #27
Conversation
|
Some tests always fail for me locally due to differences in errors from assert statements, but this is probably because my default shell is not |
|
I guess coverage isn't configured yet, it's the same for other PRs, too. |
There was a problem hiding this comment.
Thanks @agriyakhetarpal! We haven't been maintaining skeleton command that much, so it is nice you are working on it.
pyodide_build/mkpkg.py
Outdated
| if pypi_ver <= local_ver: | ||
| raise MkpkgFailedException( | ||
| f"Local version {local_ver} is newer than PyPI version {pypi_ver}, " | ||
| f"cannot update {package}. Please verify in case the version was " | ||
| "updated manually and is correct." | ||
| ) | ||
|
|
||
| # conditions to check if the package is up to date | ||
| is_sha256_up_to_date = sha256 == sha256_local | ||
| is_version_up_to_date = pypi_ver <= local_ver |
There was a problem hiding this comment.
When pypi_ver <= local_ver is true, it fails with exception, but is_version_up_to_date checks the same condition after the exception.
There was a problem hiding this comment.
Oops! Thanks for noticing. 143b729 fixes this – the version comparison should be made in equality (pypi_ver == local_ver), and the impossible update scenario should have a strict greater-than (pypi_ver < local_ver).
|
Thanks for the review! |
This PR closes #24. Here, we now check against two conditions when updating the package
Therefore, updating the package has three scenarios, and here is a summary of what happens:
Additionally, there's now a case where one manually updates the version (in error, for example), to a version that is not released or available on PyPI (yet), which means that the metadata for the version won't exist either. We raise an exception early here, asking the user so that they can check the version while updating. This is a rare situation, so it's more about raising a helpful error that aids the user in debugging the problem. I have skipped adding a test for such a case, but I can add one if needed. Please let me know your thoughts!