search

di / pip-api

An unofficial, importable pip API
https://pypi.org/p/pip-api
Apache License 2.0
110 stars 15 forks source link

test_parse_requirements_editable_file: Incorrectly fails on paths with symbols (#, @) #207

Closed hseg closed 7 months ago

hseg commented 7 months ago

When building under a path containing characters that end up being url-encoded, test_parse_requirements_editable_file incorrectly fails. Log attached error.log (have seen such errors elsewhere, and thought replacing # by @ in the path would work. In this case, it didn't. Ended up working around this by replacing the character with -)

woodruffw commented 7 months ago

Thanks for the report! To make sure I understand: the error here is not in the test itself, but in the fact that the absolute path to the test asset (a.txt) contains a @ in the path, right?

hseg commented 7 months ago

On Wed, Feb 14, 2024 at 10:27:37AM -0800, William Woodruff wrote:

Thanks for the report! To make sure I understand: the error here is not in the test itself, but in the fact that the absolute path to the test asset (a.txt) contains a @ in the path, right?

If by that you mean that the test fails when the test asset's absolute path contains a @, yes -- I'm suspecting the path gets url-encoded in one codepath but not in another. Note the occurence of @ here is due to the git repo being cloned to a subdirectory containing @ in its path -- this isn't due to your build harness mangling the location where to copy the test asset or something, as far as I can tell.

woodruffw commented 7 months ago

Yep, that's what I meant, thanks. I'll attempt to fix this tomorrow.

woodruffw commented 7 months ago

Triaging: what's happening here is that we're calling _parse_local_package_name with a path that's been normalized via _path_to_url, which includes a call to urllib.pathname2url. That stdlib function, in turn, performs the entity escaping.

I need to think about this a bit more, but I think we want to call _parse_local_package_name with the pre-normalized path instead, since normalization isn't guaranteed to preserve the actual path's existence.

woodruffw commented 7 months ago

Thanks again for the report @hseg -- #208 should fix this.

hseg commented 6 months ago

On Thu, Feb 15, 2024 at 10:31:53AM -0800, William Woodruff wrote:

Closed #207 as completed via #208.

Checked, and indeed build succeeds on such paths now (v0.0.33). Thank you!