-
-
Notifications
You must be signed in to change notification settings - Fork 636
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
Handle from foo import *
wildcard imports in Rust dep inference parser
#19249
Conversation
You're going to get bit by #18961 if you haven't already. I was thinking of computing a hash of the files during the build step and using that. Otherwise we need to manually bump the (not-yet-added) version. Thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take a close look today or tomorrow.
Thanks for this!
I've pushed a few changes, including a defensive one to hopefully make problems like this more obvious in future: make sure there's always some sort of import inserted every time a
Thanks, I got bit by that for testing #19173 😅 (On that note, I have a suspicion that
Either seems fine to me, both have trade-offs:
On balance, I guess hashing is probably better for now: this runs fast enough that (for most users) rerunning when upgrading pants versions is probably unnoticeable compared to all the other work that happens? |
…ser (#19249) This fixes #19248 by implementing explicit handling for `*` wildcard imports in the Rust-based Python dependency inference parser. The commits are somewhat individually reviewable: 1. switching how the `insert_import` arguments are consumed from the syntax tree, for `from ... import ...` statements in particular: - before: `name` and `module_name` correspond to `import $name`, `"$name"`, `from $module_name import $name` - after: `base` and `imported` correspond to `import $base`, `"$base"`, `from $base import $imported` - This is in particular flipping the last one, and changes whether the `from ...` part is the optional arg (before) or not (after). - The motivation is that there's more functional similarities between the `from ...` part and a plain `import ...` statement, than between the `import ...` part and a plain `import ...` statement, despite the superficial similarities of it! (`from foo import bar` is a little like `import foo as __secret; bar = __secret.bar`.) 2. Add some tests that fail 3. Fix the bug 4. (and others) some tweaks, including defence-in-depth against similar problems happening in future
…ser (Cherry-pick of #19249) (#19255) This fixes #19248 by implementing explicit handling for `*` wildcard imports in the Rust-based Python dependency inference parser. The commits are somewhat individually reviewable: 1. switching how the `insert_import` arguments are consumed from the syntax tree, for `from ... import ...` statements in particular: - before: `name` and `module_name` correspond to `import $name`, `"$name"`, `from $module_name import $name` - after: `base` and `imported` correspond to `import $base`, `"$base"`, `from $base import $imported` - This is in particular flipping the last one, and changes whether the `from ...` part is the optional arg (before) or not (after). - The motivation is that there's more functional similarities between the `from ...` part and a plain `import ...` statement, than between the `import ...` part and a plain `import ...` statement, despite the superficial similarities of it! (`from foo import bar` is a little like `import foo as __secret; bar = __secret.bar`.) 2. Add some tests that fail 3. Fix the bug 4. (and others) some tweaks, including defence-in-depth against similar problems happening in future
@huonw For future reference, this should likely have been labeled |
I think there's a debate on that, right? Because it fixes a bug that was never released 🤔 |
Fair point. |
Oh, I missed that. Would be good to have that on the PR in case it closes a bug while being labeled internal to avoid future confusion. :) |
This updates `changelog.py` to write the (non-internal) changes to the relevant release notes file directly, rather than requiring a human to copy-paste it in. For now, the file is just mutated, without committing. The internal changes are still printed for the human to deal with. For instance, `pants run src/python/pants_release/changelog.py -- --prior 2.18.0.dev1 --new 2.18.0.dev2` adds a new section to `src/python/pants/notes/2.18.x.md`: ```diff diff --git a/src/python/pants/notes/2.18.x.md b/src/python/pants/notes/2.18.x.md index e648a4525c..d6668a24b1 100644 --- a/src/python/pants/notes/2.18.x.md +++ b/src/python/pants/notes/2.18.x.md @@ -1,5 +1,35 @@ # 2.18.x Release Series +## 2.18.0.dev2 (Jun 14, 2023) + +### New Features + +* Include complete platforms for FaaS environments for more reliable building ([#19253](#19253)) + +* Add experimental support for Rustfmt ([#18842](#18842)) + +* Helm deployment chart field ([#19234](#19234)) + +### Plugin API Changes + +* Replace `include_special_cased_deps` flag with `should_traverse_deps_predicate` ([#19272](#19272)) + +### Bug Fixes + +* Raise an error if isort can't read a config file ([#19294](#19294)) + +* Improve handling of additional files in Helm unit tests ([#19263](#19263)) + +* Add taplo to the release ([#19258](#19258)) + +* Handle `from foo import *` wildcard imports in Rust dep inference parser ([#19249](#19249)) + +* Support usage of `scala_artifact` addresses in `scalac_plugin` targets ([#19205](#19205)) + +### Performance + +* `scandir` returns `Stat`s relative to its directory. ([#19246](#19246)) + ## 2.18.0.dev1 (Jun 02, 2023) ### New Features ``` This also moves it into the new `pants_release` root, adds some basic tests, and has it fetch the relevant branch directly, rather than prompting the user to do so. It also pulls out a `git.py` support module with helpers for exec-ing `git` as an external process. The commits are individually reviewable. This is a step towards automating more of the "start release" process, per #19279. After this core refactoring of the functionality, I think the next step is to combine it with the CONTRIBUTORS update and version bumping, and then after that string it all together on CI so that starting a release is fully automated.
This fixes #19248 by implementing explicit handling for
*
wildcard imports in the Rust-based Python dependency inference parser.The commits are somewhat individually reviewable:
insert_import
arguments are consumed from the syntax tree, forfrom ... import ...
statements in particular:name
andmodule_name
correspond toimport $name
,"$name"
,from $module_name import $name
base
andimported
correspond toimport $base
,"$base"
,from $base import $imported
from ...
part is the optional arg (before) or not (after).from ...
part and a plainimport ...
statement, than between theimport ...
part and a plainimport ...
statement, despite the superficial similarities of it! (from foo import bar
is a little likeimport foo as __secret; bar = __secret.bar
.)