search

GitoxideLabs / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
8.91k stars 303 forks source link

audit uses of `as_ref()` and remove those that are ambiguous #1466

Closed BurntSushi closed 2 months ago

BurntSushi commented 2 months ago

Adding a trait impl in bstr 1.9.2 (which is yanked for now) caused gix-credentials to fail to compile.

The issue looks to be the result of using ambiguous as_ref(). That is, an as_ref() whose inference relies on the fact that there is only one trait implementation to choose from. If new trait implementations are added, then inference fails and a compile error occurs.

I think this has happened before with gix, although I don't remember the precise details. What do you think about doing an audit for as_ref() usage? In general, I'd like to be able to add trait implementations to bstr as they come up, but I also don't want to be breaking folks downstream of me. :-)

Byron commented 2 months ago

Thanks for the heads-up! Indeed, it's not the first time and I can't promise it will be the last one after this one is fixed.

As for bstr, adding new impls for AsRef isn't considered a breaking change (despite the risk), and since I seem to be making that mistake (and so will others), maybe embracing a reactive response is the only way here.

Yanking temporarily gives some time to respond and like in this case, publish a patched release which will be picked up by the most recent gix and the one before.

I will let you know once the fix is available, so the latest version of bstr can be unyanked again.

BurntSushi commented 2 months ago

Thanks sounds good!

Byron commented 2 months ago

It looks like gitoxide-core is also broken (my 'audit' was made by using the latest unreleased local clone so I could compile with what v1.9.2 would be), which means that only the most recent version of gitoxide will install correctly. That fortunately should be fine as this is what most people do and do by default with cargo install gitoxide.

So with git-credentials released as patch and gitoxide-core released as patch, all current users should be safe.

Once CI passes here I will publish the new versions and share them in this issue.

Byron commented 2 months ago

Both gitoxide-core and gix-credentials have been published as patch releases. They should prevent damage downstream as they don't use as_ref() anymore.

Byron commented 2 months ago

Please feel free to close the issue once bstr 1.9.2 is unyanked, and I suppose I will know if it worked by the absence of a flood of new issues 😅,

BurntSushi commented 2 months ago

All righty, bstr 1.10.0 is out. (I did a patch release before by mistake. Should have been a minor bump due to adding a new API. ¯\_(ツ)_/¯)