go_repository_config generates invalid labels in Windows

[atavakoliyext]

What version of gazelle are you using?

v0.24.0

What version of rules_go are you using?

v0.29.0

What version of Bazel are you using?

4.2.1

Does this issue reproduce with the latest releases of all the above?

Yes.

What operating system and processor architecture are you using?

Windows Server Core (docker image mcr.microsoft.com/windows/servercore:1809)

What did you do?

Setup the WORKSPACE with rules_go and gazelle at the versions above.

Create tools/go/repos.bzl and add some go_repositorys to it.

In WORKSPACE:

load("//tools/go:repos.bzl", "go_repositories")
# gazelle:repository_macro tools/go/repos.bzl%go_repositories

Create a go module in the repo that uses at least one of the modules in the above file.

Run bazel run //:gazelle && bazel build //some/golibrary/target

What did you expect to see?

The build succeeds.

What did you see instead?

  ERROR: An error occurred during the fetch of repository 'bazel_gazelle_go_repository_config':
     Traceback (most recent call last):
    File "C:/users/.../external/bazel_gazelle/internal/go_repository_config.bzl", line 45, column 40, in _go_repository_config_impl
      macro_label = Label("@" + ctx.attr.config.workspace_name + "//:" + f)
  Error in Label: Illegal absolute label syntax: @//:tools\go\repos.bzl
  ERROR: Error fetching repository: Traceback (most recent call last):
    File "C:/users/.../external/bazel_gazelle/internal/go_repository_config.bzl", line 45, column 40, in _go_repository_config_impl
      macro_label = Label("@" + ctx.attr.config.workspace_name + "//:" + f)
  Error in Label: Illegal absolute label syntax: @//:tools\go\repos.bzl
  ERROR: C:/.../some/golibrary/target/BUILD.bazel:3:11: no such package '...': no such package '@bazel_gazelle_go_repository_config//': Illegal absolute label syntax: @//:tools\go\repos.bzl and referenced by '//some/golibrary/target:target'
  ERROR: Analysis of target '//some/golibrary/target:target' failed; build aborted: Analysis failed

This is fixed by using a local_repository for gazelle with the following diff:

diff --git a/internal/go_repository_config.bzl b/internal/go_repository_config.bzl
index 834e684..b1882b5 100644
--- a/internal/go_repository_config.bzl
+++ b/internal/go_repository_config.bzl
@@ -40,7 +40,7 @@ def _go_repository_config_impl(ctx):
             fail("generate_repo_config: " + result.stderr)
         if result.stdout:
             for f in result.stdout.splitlines():
-                f = f.lstrip()
+                f = f.lstrip().replace('\\', '/')
                 if len(f) > 0:
                     macro_label = Label("@" + ctx.attr.config.workspace_name + "//:" + f)
                     ctx.path(macro_label)

[UebelAndre]

This blocks windows support in rules_python: https://github.com/bazelbuild/rules_python/blob/0.8.0/.bazelci/presubmit.yml#L35-L36

[achew22]

@UebelAndre seems like a pretty reasonable step forward. While it's possible to make the change suggested in here by @atavakoliyext, I personally think it would be better to fix it in the place where the filepath is generated.

Something like modifying:

https://github.com/bazelbuild/bazel-gazelle/blob/77e0f34c43a9f5e95f6af93485a4bd83942ed3c6/cmd/generate_repo_config/generate_repo_config.go#L143-L147

to end with

files = append(files, strings.Replace(f, `\\`, `/`)

and then either finding a test that's currently disabled, or writing a new test to confirm this behavior in Windows. If you have any questions about how to do that, please feel free to ping the thread.

[atavakoliyext]

Hi @fmeum, we recently re-ran into this issue after performing an internal upgrade that missed the patch above in our local fork. Would you have any thoughts on the fix described by me or @achew22? If either are acceptable, I'd be happy to put up the PR.

(apologies in advance if you're not the right person to tag on this)

[fmeum]

@atavakoliyext Either looks good, although I would prefer this if it works:

diff --git a/cmd/generate_repo_config/generate_repo_config.go b/cmd/generate_repo_config/generate_repo_config.go
index f573fa2..79d90cc 100644
--- a/cmd/generate_repo_config/generate_repo_config.go
+++ b/cmd/generate_repo_config/generate_repo_config.go
@@ -141,7 +141,7 @@ func generateRepoConfig(configDest, configSource string) ([]string, error) {
        if err != nil {
            return nil, err
        }
-       files = append(files, f)
+       files = append(files, filepath.ToSlash(f))
    }

    return files, nil

Having a test in generate_repo_config_test.go that catches such Windows-specific issues would be nice as well.

[zecke]

@atavakoliyext Either looks good, although I would prefer this if it works:

this has worked for me.

Having a test in generate_repo_config_test.go that catches such Windows-specific issues would be nice as well.

What about an integration test? That failure makes things break rather early in a build?