Migrate rules_rust for https://github.com/bazelbuild/bazel/issues/7153 and https://github.com/bazelbuild/bazel/issues/7152

[hlopko]

This PR migrates rules_rust for following Bazel incompatible changes:

• incompatible_disable_legacy_proto_provider: Remove legacy dep.proto. access to the proto provider bazel#7152 (removal of legacy dep.proto provider)
• incompatible_disable_proto_source_root: remove the attribute proto_library.proto_source_root bazel#7153 (removal of proto_source_root)

7152 is trivial, just not compatible with Bazel <= 0.21 which at this point is fine I guess?

7153 part is very controversial. I think we're missing Bazel APIs in ProtoInfo to implement fully correct code. The PR passes the test suite, but that says nothing about the actual quality of my code :) It introduces many bugs:

• I created Add more test targets for proto support #234 to demonstrate edge cases that are missed before this PR, and well, they are still missed after this PR.
• I honestly didn't understand the current implementation, especially why was _compute_proto_source_path introduced (I know very little about protos), it may be that this PR broke smth I don't understand yet.

So pls bear with me and be careful with your review :)

Thanks!

[damienmg]

This PR migrates rules_rust for following Bazel incompatible changes:

• bazelbuild/bazel#7152 (removal of legacy dep.proto provider)
• bazelbuild/bazel#7153 (removal of proto_source_root)

7152 is trivial, just not compatible with Bazel <= 0.21 which at this point is fine I guess?

This is fine by me, but the compatibility check need to be updated.

7153 part is very controversial. I think we're missing Bazel APIs in ProtoInfo to implement fully correct code. The PR passes the test suite, but that says nothing about the actual quality of my code :) It introduces many bugs:

• I created Add more test targets for proto support #234 to demonstrate edge cases that are missed before this PR, and well, they are still missed after this PR.
• I honestly didn't understand the current implementation, especially why was _compute_proto_source_path introduced (I know very little about protos), it may be that this PR broke smth I don't understand yet.

The problem is the way path is relative to the root of the repository for proto. E.g. if you include a proto @repo1//my/path:to.proto in your proto //my:own.proto then the path of the first proto in the second one will be my/path/to.proto in the proto descriptor (without the repo definition) whereas short_path will return external/repo1/my/path/to.proto. The proto compiler needs the list of proto descriptor and the path of the file they represent.

I would need to test on my personal computer to see if I it still works fine with external repositories.

So pls bear with me and be careful with your review :)

No worries :)

Thanks!

[hlopko]

@damienmg Did you get to test this PR?

[damienmg]

Not yet, I am travelling at the moment :/

…

On Tue, Jul 2, 2019, 6:51 AM Marcel Hlopko ***@***.***> wrote: @damienmg <https://github.com/damienmg> Did you get to test this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#235?email_source=notifications&email_token=AA4MO72GWJNYTMRQFWSEIV3P5NMOXA5CNFSM4HZVOWBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZBK3PY#issuecomment-507686335>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA4MO7YZEMHIPXFY3YGBDQLP5NMOXANCNFSM4HZVOWBA> .

[aiuto]

friendly ping

[hlopko]

Rules rust are already migrated for both incompatible flags. This PR is obsolete.