Skip to content

Fix regression in pip parse for finding implicit namespace packages.#504

Merged
hrfuller merged 5 commits intobazel-contrib:mainfrom
hrfuller:hrfuller/fix-499-normalize-path-names
Jul 10, 2021
Merged

Fix regression in pip parse for finding implicit namespace packages.#504
hrfuller merged 5 commits intobazel-contrib:mainfrom
hrfuller:hrfuller/fix-499-normalize-path-names

Conversation

@hrfuller
Copy link
Copy Markdown
Contributor

@hrfuller hrfuller commented Jul 7, 2021 •
edited
Loading

fixes #499
Pathlib.Path normalizes path names, which caused lookups of relative
paths using the special '.' directory path to fail to find parents of
standard packages.

Drive-by change fixes passing quiet argument to child repos.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

pip_parse fails to find implicit namespace packages.

Issue Number: #499

What is the new behavior?

pip_parse can find implicit namespace packages.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@hrfuller
Fix regression in pip parse for finding implicit namespace packages.
5a467ff 
Pathlib.Path normalizes path names, which caused lookups of relative
paths using the special '.' directory path to fail to find parents of
standard packages.
@hrfuller hrfuller requested a review from thundergolfer July 7, 2021 02:07
@hrfuller hrfuller requested review from brandjon and lberki as code owners July 7, 2021 02:07
@google-cla google-cla bot added the cla: yes label Jul 7, 2021
@thundergolfer
Copy link
Copy Markdown

thundergolfer commented Jul 7, 2021

Using the https://github.com/caseyduquettesc/rules_python_demo repro I still have the ModuleNotFoundError: No module named 'google.api_core' error.

Running PYTHONVERBOSE=2 bazel run //scripts/... I can see that Python isn't checking in deps_pypi__google_api_core/google/ for the api_core module.

@hrfuller
Copy link
Copy Markdown
Contributor Author

hrfuller commented Jul 7, 2021 •
edited
Loading

Mine definitely is

bytecode is stale for 'google.api_core'
# code object from /private/var/tmp/_bazel_hfuller/67a2eb7ad29da3877a82ffb129fac926/execroot/demo/bazel-out/darwin-fastbuild/bin/scripts/demo/demo.runfiles/deps_pypi__google_api_core/google/api_core/__init__.
py

What interpreter version do you have on the path?
I tried with both 36, 37 and 39 and they work for me.

@hrfuller
Copy link
Copy Markdown
Contributor Author

hrfuller commented Jul 7, 2021

Hey @caseyduquettesc can you test out this branch against your demo repository and see if it fixes the issue you were having?

@caseyduquettesc
Copy link
Copy Markdown

caseyduquettesc commented Jul 8, 2021

can you test out this branch against your demo repository and see if it fixes the issue you were having?

Sure thing. Will get back to you in a couple hours, just got back from some time off.

@caseyduquettesc
Copy link
Copy Markdown

caseyduquettesc commented Jul 8, 2021

@hrfuller I used your branch in the example at https://github.com/caseyduquettesc/rules_python_demo and in a private project that has its own python toolchain (3.8.10) and was unable to reproduce the issue in both. Functionally, the change LGTM.

@ssiano
Copy link
Copy Markdown

ssiano commented Jul 9, 2021

@hrfuller, I verified that this PR also fixes the ModuleNotFoundError: No module named 'azure.storage'.

I was using Python 3.6.

Cc: @thundergolfer

@thundergolfer
Copy link
Copy Markdown

thundergolfer commented Jul 10, 2021

Can also confirm this removes the ModuleNotFound errors in the repro I'm using. Tested in https://github.com/thundergolfer-forks/rules_python_demo

from google.api_core.exceptions import ResourceExhausted
import google.api_core
import google.auth

All that succeeds.

thundergolfer
thundergolfer approved these changes Jul 10, 2021
View reviewed changes
Copy link
Copy Markdown

@thundergolfer thundergolfer left a comment

Choose a reason for hiding this comment

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

Needs a fix to types, but otherwise good to merge 👍

@hrfuller hrfuller merged commit fbbecae into bazel-contrib:main Jul 10, 2021
@hrfuller hrfuller deleted the hrfuller/fix-499-normalize-path-names branch July 11, 2021 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@thundergolfer thundergolfer thundergolfer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pip_parse regression in 0.3.0

4 participants

@hrfuller @thundergolfer @caseyduquettesc @ssiano