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

fix: pypi parse_simpleapi_html.bzl is robust to metadata containing ">" sign #2031

Merged

Conversation

mihaidusmanu
Copy link
Contributor

@mihaidusmanu mihaidusmanu commented Jul 3, 2024

This PR modifies the logic for finding the end of the of the
tag metadata attributes in the pypi parse_simpleapi_html function.

This was discovered after investigation of the following error:

Error in repository_rule: invalid user-provided repo name 'pypi_311_=2_7,!=3_0_*,!=3_1_*,!=3_2_*">six_py2_none_any_8abb2f1d': valid names may contain only A-Z, a-z, 0-9, '-', '_', '.', and must start with a letter

which was traced back to the to a data-requires-python attribute containing a > sign (instead of >) in the Azure Artifacts pypi feed, e.g.:
<a href="https://microsoft.pkgs.visualstudio.com/REDACTED_URL/six-1.16.0-py2.py3-none-any.whl#sha256=8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254" data-requires-python=">=2.7,!=3.0.*,!=3.1.*,!=3.2.*">six-1.16.0-py2.py3-none-any.whl</a><br/>

@mihaidusmanu mihaidusmanu requested a review from groodt as a code owner July 3, 2024 14:17
Copy link

google-cla bot commented Jul 3, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@aignas aignas self-requested a review July 4, 2024 08:42
@aignas
Copy link
Collaborator

aignas commented Jul 4, 2024

@mihaidusmanu, could you please rebase on latest main, add a line in CHANGELOG.md and also accept the Google's CLA please?

The PR description would become a commit message, so feel free to clean it up a little, so that it looks better when reading git log. :)

Thanks for the contribution!

@mihaidusmanu
Copy link
Contributor Author

Hello @aignas - I just signed the CLA, updated the changelog and also tried to update the PR description.

Regarding rebasing on latest main, I believe it is already up to date (?)

@aignas aignas force-pushed the user/mihaidusmanu/pip-azure-artifacts branch from 2f39744 to af2a5d8 Compare July 5, 2024 00:34
@aignas
Copy link
Collaborator

aignas commented Jul 5, 2024

I simplified the code using rpartition instead of using for loop and finding quotes. It is still passing the tests you have added. Thanks a lot by adding more tests!

@aignas aignas enabled auto-merge July 5, 2024 06:31
@aignas aignas added this pull request to the merge queue Jul 5, 2024
Merged via the queue into bazelbuild:main with commit 25a0a47 Jul 5, 2024
4 checks passed
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.

2 participants