diesel icon indicating copy to clipboard operation
diesel copied to clipboard

fix: fix glob resolution issue

Open bvanjoi opened this issue 1 year ago • 3 comments

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: https://github.com/rust-lang/rust/pull/114682

bvanjoi avatar Jan 09 '24 07:01 bvanjoi

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?

weiznich avatar Jan 10 '24 13:01 weiznich

What is the impact of not issuing a patch release for this issue for diesel 1.x.

There are approximately over 200 regressions as a result of this case. For details, refer to migrations_internals-1.4.x at the following link: https://crater-reports.s3.amazonaws.com/pr-114682-1/index.html

Would it break existing applications on newer rust version

If the https://github.com/rust-lang/rust/pull/114682 gets merged directly, it will lead to a build error.

or would it only result in a new warning

Initially, I intended to issue a warning regarding this matter. However, I've discovered a more complex case: max has been used as a function in __diesel_schema_migrations.select(max(version)), when it is actually intended to refer to a type max = xxx. For more details, please refer to https://github.com/rust-lang/rust/pull/114682#issuecomment-1881630225.

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 1.4.9

bvanjoi avatar Jan 10 '24 14:01 bvanjoi

Once this PR is merged, we can advise users who encounter this regression to use 1.4.9

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.

weiznich avatar Jan 10 '24 14:01 weiznich