fix: fix glob resolution issue #3898
Conversation
| @@ -41,7 +41,7 @@ where | |||
| } | |||
|
|
|||
| fn latest_run_migration_version(&self) -> QueryResult<Option<String>> { | |||
| use diesel::dsl::max; | |||
There was a problem hiding this comment.
diesel::dsl has re-exported an ambiguous max, one exported from dsl::helper_types::* and another from dsl::expression::dsl, see: https://github.com/diesel-rs/diesel/blob/1.4.x/diesel/src/lib.rs#L220-L224
There was a problem hiding this comment.
This pattern continues to exist in diesel 2.0. See #3745 for some discussion around that. As it is hard to completely remove this bug without doing a breaking change (and therefore a major version bump) what's the impact on that?
There was a problem hiding this comment.
This pattern continues to exist in diesel 2.0
The regression test report doesn't indicate any regression in diesel@2.0, so I need to investigate further to determine if it's truly identical to 1.4.x.
|
Thanks for opening this PR. What is the impact of not issuing a patch release for this issue for diesel 1.x. Would it break existing applications on newer rust version or would it only result in a new warning? |
There are approximately over 200 regressions as a result of this case. For details, refer to
If the rust-lang/rust#114682 gets merged directly, it will lead to a build error.
Initially, I intended to issue a warning regarding this matter. However, I've discovered a more complex case: Therefore, resolving this issue simply as a warning won't truly address this regression. It requires additional work, such as ignoring the type namespace if there is ambiguity in the binding defined at the extern crate. However, implementing this change doesn't seem worthwhile as the problem can be resolved by submitting this PR. Once this PR is merged, we can advise users who encounter this regression to use |
The main reason why I asked is because I consider diesel 1.4.x as not actively supported anymore. I fear that issuing another update will raise expectations from users that did not update yet that we will continue to provide fixes and other stuff for this old version. That's nothing I'm able to do on the side. For that reason I would prefer not having to issue an update that fixes an breaking change done by rustc. |
Hi, we have recently made some changes related to glob name resolution in rustc and identified some regressions in this case. This PR aims to address these issues. You can find more background information at: rust-lang/rust#114682