Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
97acdf2 to
fc62ff2
Compare
|
@UebelAndre Not sure who to ask about this, are there any changes required to get this merged? I'm just checking back in so we can hopefully delete our patch soon. |
UebelAndre
left a comment
There was a problem hiding this comment.
Sorry, about the delay! How does this fix the issue?
|
Previously the cargo config was stored as a |
|
@UebelAndre Sorry to ping you again, but just wanted to check in and see if you had any thoughts. |
UebelAndre
left a comment
There was a problem hiding this comment.
Thank you for the ping (I unfortunately need them)! Can you add a regression test for this? I suspect this is something that will change over time and with changes such as bazelbuild/bazel#27659 might make doing things in starlark more efficient.
313a91d to
e9780e0
Compare
|
That's cool! Native TOML parsing in Bazel would definitely make this much easier. I added a test that I confirmed fails to replicate the digest without my change. |
|
@UebelAndre Sorry forgot to tag you in my comment. I added a test to ensure Windows/Unix line endings don't change the digest. |
e9780e0 to
defcca5
Compare
1856d93 to
f7b40e4
Compare
Cargo configs are injected as a literal string which, on Windows, can end up with escaped CRLF line endings in cargo-bazel.json causing the digests to be different on mac/windows. This parses the inner string literal to prevent platform specific differences.
f7b40e4 to
b071768
Compare
illicitonion
left a comment
There was a problem hiding this comment.
Thanks for adding the test!
Cargo configs are injected as a literal string which, on Windows, can end up with escaped CRLF line endings in
cargo-bazel.jsoncausing the digests to be different on Linux/Windows. This PR parses the inner string literal to prevent platform specific differences in theconfig.toml. There might be a better way to do this, but adding a new top level command to cargo-bazel to parse TOML into JSON felt wrong, and adding a second binary seems even worse. I'm open to solving this another way if ya'll have a better idea.