rust-lang
Update `log` crate and create shim method(s) between `git2`'s tracing and `log`
This change came out of some work I'm doing on https://github.com/mitre/hipcheck, and it seemed like it would be useful upstream as well.
I'm a little uncertain about adding this, because it makes
logbecome a public dependency of git2. Updating public dependencies can cause problems, and I generally would like to avoid them if at all possible. It's also not clear to me why we would choose "log" over say "tracing" or any other option. I'm not yet sure if I would like to add that.Generally, all changes should have a test.
Also, I'm a little uneasy with moving forward with this since it looks like the existing code has undefined behavior. Is that something you can maybe fix in a separate PR?
I think you're right in your assessment of public dependencies and their issues. The best solution to this in my mind is to just make any function that interacts with a type from log private, so that we can still consume the crate's facade (much in the same way that src/cred.rs did before this PR) without publicly using log types. I'll make those changes and update this PR.
In regards to the existing undefined behavior, I'll have to take a look, but I would be happy to open another PR if I can find what you're referring to.
https://github.com/rust-lang/git2-rs/pull/1071 fixes the undefined behavior you mentioned.
Just saw ehuss has left comments and noticed the public dependency issue I wasn't aware of. I totally agree with that being a way bigger problem than other issues I've pointed out. Given that, I don't think this is a thing we're going to merge. Thanks for being interested in upstreaming your changes anyway!
Of course! I completely understand -- it may be worth upgrading log regardless -- I leave that to you.