Fix matching between Starlark and query labels in gopackagesdriver

[fmeum]

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Makes gopackagesdriver work under Bzlmod by relying on the query flag --consistent_labels, which is available in Bazel 6.4.0.

Which issues(s) does this PR fix?

Fixes #3604
Fixes #3659

Other notes for review

[fmeum]

@pziggo @alan910127 @ian-h-chamberlain Could you test this PR with Bazel 6.4.0rc1, which has just been released?

[fmeum]

CC @tyler-french

[pziggo]

👍 Seems to work for me with Bazel 6.4.0rc1 (tested as patch on top of 0.41.0 as I did not manage to convince bazel override transitive dependencies with git_override ... seems to be an issue on its own)

[ian-h-chamberlain]

Unfortunately I can't test this properly as my organization is still on Bazel 5.4 and a quick attempt showed there would probably be a bit of work needed for upgrade to 6. I believe we are planning to upgrade within the relatively near future, so I can post an update here when that happens, but if it's working for other people than I'm hopeful it would resolve the issue for us as well.

However, since we're not using Bzlmod I'm still a bit confused why we were affected by the original issue... perhaps there is a codepath that was still active even without using modules?

[fmeum]

Interesting, I hope we didn't regress WORKSPACE support. Bazel 5 is not something I regularly use. I will need to test this more.

@tyler-french Do you still have a WORKSPACE setup you could test with?

[ian-h-chamberlain]

Just to circle back on this, I realized I could still try testing these changes on Bazel 5 without upgrading, and I can still reproduce with this patch on Bazel 5.4.0 in our WORKSPACE setup:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x112ae6d]

goroutine 1 [running]:
main.(*PackageRegistry).walk(0xc0000be290, 0xc00011baa0?, {0xc00035ec30?, 0x25?})
    external/io_bazel_rules_go/go/tools/gopackagesdriver/packageregistry.go:79 +0x4d
main.(*PackageRegistry).Match(0xc0000be290, {0xc000206000, 0x1, 0xc000305db0?})
    external/io_bazel_rules_go/go/tools/gopackagesdriver/packageregistry.go:112 +0x306
main.(*JSONPackagesDriver).GetResponse(0xc000305000?, {0xc000206000?, 0xc0000bc0f0?, 0xc000206000?})
    external/io_bazel_rules_go/go/tools/gopackagesdriver/json_packages_driver.go:51 +0x1f
main.run()
    external/io_bazel_rules_go/go/tools/gopackagesdriver/main.go:114 +0x558
main.main()
    external/io_bazel_rules_go/go/tools/gopackagesdriver/main.go:118 +0x1f

[tyler-french]

Tested that GPD works as expected in an IDE at Uber on 6.3.2, not sure about the above mentioned issues. Let me know if there's anything else I should test specifically.

[fmeum]

I will merge this as it does improve the situation for Bazel 6 and Bzlmod and doesn't make the situation on Bazel 5 worse (it still doesn't work).

@ian-h-chamberlain Could you check whether this PR fixes the issue for you if you also enable --incompatible_unambiguous_label_stringification?

[ian-h-chamberlain]

@fmeum unfortunately it looks like that flag was not even added until 6.0.0 (which is also where it was turned on by default): bazelbuild/bazel@c12dd9b

Hopefully my team can upgrade to 6.x soon and work around all this, but I guess a fix to support 5.4 would also be nice if possible. Let me know if there's anything else I can do to help debug

[seanmorton-afs]

@ian-h-chamberlain I'm also currently on bazel 5.4.0 and can't upgrade bazel in the near term either. Do you know if there's already an issue for fixing this in 5.4.0? I can create one if not. Also, have you found any workarounds?

EDIT: Just found #3700

[ian-h-chamberlain]

FWIW, if using https://github.com/bazelbuild/rules_go/pull/3700.patch doesn't work for you, what I ended up using in the meantime for our repo was what I mentioned in #3604 (comment) effectively creating a patch to revert all changes from #3606 and #3524 (which originally introduced this issue, I think). It does seem like #3700 might be a better option but I actually haven't had a chance to test it yet. Just to provide another option (it might make upgrading rules_go harder in the future though).