Renamed workspace name from io_bazel_rule_rust to rules_rust

[UebelAndre]

Generated by running

LANG=C sed -i '' -e 's/io_bazel_rules_rust/rules_rust/' $(find . -type f -not -path "./.git/*")

There's some small amount of sorting in here as well (based on the new name)

Closes #499

Blocked by google/cargo-raze#298 and google/cargo-raze#304

[UebelAndre]

@mfarrugi with the release of cargo-raze 0.8.0 this PR is ready to go.

[UebelAndre]

@mfarrugi Just a heads up, It'd be great to get this in in a timely manner given the messaging in cargo-raze:

WARNING: The default of `[*.raze.rust_rules_workspace_name]` will soon be set to `"rules_rust"`. Please explicitly set this flag to prevent a change in behavior or upgrade your code to use the latest version of `rules_rust` and change references of `io_bazel_rules_rust` to `rules_rust` in your project.

I understand this is has larger implication for users but I feel confident that with the snippet above in this PR (or just a find and replace in common editors), users can easily update the their projects to allow for this switch.

[UebelAndre]

@illicitonion Hey, @mfarrugi seems to have been away for some time, would you be able to review this PR?

[acmcarther]

This should probably not be merged. This ruleset is named "io_bazel" to be consistent with the other rules defined in the bazelbuild org.

See rules go: https://github.com/bazelbuild/rules_go. Their workspace is named "io_bazel_rules_go". I don't know the history behind this naming scheme, but I do know that rules_go has "high-tier" support internally, so we should follow their lead on things.

I'm going to mirror this into the issue.

[UebelAndre]

#500 (comment) this conversation should happen in one place. I responded on #499

[UebelAndre]

@mfarrugi re google/cargo-raze#323 (comment)

I'd like to talk about potential backwards compatibility shims here. I don't know of many (if any) options. To me this change is something users would have to deal with if they wanted to continue to accept changes from master. My stance is still that the changes here have a low impact on users since they can run the command in the description to update. Happy to make a shim if someone can explain what that looks like though. But I feel the sooner these changes are in the better for everyone. Otherwise things are likely to go the way of rules_go where it's acquired so many users that it becomes much more difficult to fix the name.

[mfarrugi]

The only option I know of is a bind, but I have never used it and am not confident that it actually works.

…

On Tue, Dec 22, 2020, 09:51 UebelAndre ***@***.***> wrote: @mfarrugi <https://github.com/mfarrugi> re google/cargo-raze#323 (comment) <google/cargo-raze#323 (comment)> I'd like to talk about potential backwards compatibility shims here. I don't know of many options. To me this change is something users would have to deal with if they wanted to continue to accept changes from master. My stance is still that the changes here have a low impact on users since they can run the command in the description to update. Happy to make a shim if someone can explain what that looks like though. But I feel the sooner these changes are in the better for everyone. Otherwise things are likely to go the way of rules_go <https://github.com/bazelbuild/rules_go/blob/master/WORKSPACE#L1> where it's acquired so many users that it becomes much more difficult to fix the name. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#500 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAROG6GPMQGVYOKAKQMCLFLSWCW6XANCNFSM4T6XEVDA> .

[UebelAndre]

@mfarrugi bind only appears to work on targets. Not entire repositories.

edit: Or do I misunderstand that? I'm otherwise not able to get bind to work for me when importing it:

When running bazel build //... in ./examples I get this error

ERROR: error loading package 'ffi/rust_calling_c/simple': Unable to find package for @rules_rust//bindgen:bindgen.bzl: The repository '@rules_rust' could not be resolved.

With this diff applied to this PR.

diff --git a/examples/examples_repositories.bzl b/examples/examples_repositories.bzl
index f11f380..9843b81 100644
--- a/examples/examples_repositories.bzl
+++ b/examples/examples_repositories.bzl
@@ -7,10 +7,15 @@ def repositories():
     """Define repository dependencies for `rules_rust` examples"""
     maybe(
         native.local_repository,
-        name = "rules_rust",
+        name = "io_bazel_rules_rust",
         path = "..",
     )

+    native.bind(
+        name = "rules_rust",
+        actual = "@io_bazel_rules_rust",
+    )
+
     maybe(
         http_archive,
         name = "build_bazel_rules_nodejs",
diff --git a/rust/repositories.bzl b/rust/repositories.bzl
index 29aeeea..bdb6349 100644
--- a/rust/repositories.bzl
+++ b/rust/repositories.bzl
@@ -745,19 +745,19 @@ def rust_repository_set(
     native.register_toolchains("@rules_rust//rust/private/dummy_cc_toolchain:dummy_cc_wasm32_toolchain")

     # Inform users that they should be using the canonical name if it's not detected
-    if "rules_rust" not in native.existing_rules():
-        message = "\n" + ("=" * 79) + "\n"
-        message += (
-            "It appears that you are trying to import rules_rust without using its\n" +
-            "canonical name, \"@rules_rust\". This does not work. Please change your\n" +
-            "WORKSPACE file to import this repo with `name = \"rules_rust\"` instead."
-        )
-
-        if "io_bazel_rules_rust" in native.existing_rules():
-            message += "\n\n" + (
-                "Note that the previous name of \"@io_bazel_rules_rust\" is no longer used.\n" +
-                "See https://github.com/bazelbuild/rules_rust/issues/499 for context."
-            )
-
-        message += "\n" + ("=" * 79)
-        fail(message)
+    # if "rules_rust" not in native.existing_rules():
+    #     message = "\n" + ("=" * 79) + "\n"
+    #     message += (
+    #         "It appears that you are trying to import rules_rust without using its\n" +
+    #         "canonical name, \"@rules_rust\". This does not work. Please change your\n" +
+    #         "WORKSPACE file to import this repo with `name = \"rules_rust\"` instead."
+    #     )
+
+    #     if "io_bazel_rules_rust" in native.existing_rules():
+    #         message += "\n\n" + (
+    #             "Note that the previous name of \"@io_bazel_rules_rust\" is no longer used.\n" +
+    #             "See https://github.com/bazelbuild/rules_rust/issues/499 for context."
+    #         )
+
+    #     message += "\n" + ("=" * 79)
+    #     fail(message)

[UebelAndre]

@mfarrugi Yeah, I'm not able to get this to work with bind. Can you think of any other options?

[acmcarther]

In the other link you referenced rules python which already made this change, so I'm not concerned about this being an issue anymore. Discussions about impact on users might still be prudent though.

[UebelAndre]

@mfarrugi I tried asking in the slack channel but also wasn't able to find great solutions for backward compatibility. I think the only thing to do would be to pull the trigger. I discussed this with @illicitonion a bit and he suggests that this be coupled a new release model (#415) and official support for Bazel 4.0 where the min supported version follows the new LTS model introduced in that version of Bazel. Since very little changes to add 4.0 as a min version (since the rules, according to bazelisk, is already compatible). So I feel this could be merged and announcements can follow about changes to compatibility.

[UebelAndre]

I spent quite a lot of time looking into ways ways to support both workspace names. The two blockers are both related to the use of generated repositories and how they refer to rules_rust.

1. Renamed workspace name from io_bazel_rule_rust to rules_rust #500 (comment)
2. Renamed workspace name from io_bazel_rule_rust to rules_rust #500 (comment)

I created #543 and #542 as a result of my attempt to solve this. I do not know of any way to support multiple workspace names and believe the only path forward is a hard cut-over.

[UebelAndre]

@mfarrugi @damienmg @illicitonion This PR is ready to go. I'm not sure what else should be done for this. @illicitonion talked about doing the rename at the same time as the 4.0 migration (which I think is a no-op since the rules are already compatible). Maybe #445 can be done at the same time? And that would conclude "breaking" changes for the near future?

[damienmg]

I can take a look at what is needed for #445 next week but I don't think it should block this PR.

[UebelAndre]

@damienmg I'm happy to have this merged 😃 is there anything else I should do here?

[UebelAndre]

@damienmg hey, just pinging this again 😅

[damienmg]

Sorry I cannot address #445 at the moment, I am fighting fires at work and in my personal life :(

Related to #445, I received a bug that we have whitelist somewhere in our codebase, suggesting we replaced it with allowlist (definitely a good idea IMO).

[UebelAndre]

Thanks @damienmg! I've really enjoyed having you as a reviewer and would love to keep working with you. I hope everything bounces back for you and am wishing you the best 😄