go_test: I/O in package init function fails after Go 1.21 due to initialization order change

[jayconrod]

What version of rules_go are you using?

v0.41.0 but also reproduced at 5206498b4f67ff3e6a9222a923cf67ff5191754e (master as of 2023-09-01).

What version of gazelle are you using?

n/a

What version of Bazel are you using?

7.0.0-pre.20230530.3

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

yes

What operating system and processor architecture are you using?

darwin arm64

Any other potentially useful information about your toolchain?

n/a

What did you do?

We're in the process of upgrading to Go 1.21, running all our tests including those from vendored packages.

What did you expect to see?

All tests pass, as before.

What did you see instead?

A couple vendored tests are failing for us as part of that upgrade, for example github.com/klauspost/compress/huff0. This fails in compress_test.go because the test reads a testdata file in its init function.

I think we can all agree that doing I/O in an init function is bad and anti-social. Regrettably there are no test-I/O cops, so people do this somewhat frequently.

This is caused by the change documented in the Go 1.21 release notes:

Package initialization order is now specified more precisely. The new algorithm is:

• Sort all packages by import path.
• Repeat until the list of packages is empty:
  • Find the first package in the list for which all imports are already initialized.
  • Initialize that package and remove it from the list.

This may change the behavior of some programs that rely on a specific initialization ordering that was not expressed by explicit imports. The behavior of such programs was not well defined by the spec in past releases. The new rule provides an unambiguous definition.

go_test links tests with go/tools/bzltestutil. The init function in that package is responsible for changing into the test directory from the repository root directory where Bazel starts the test process. I added some print statements and confirmed that bzltestutil's init function does not run in the test above before it panics.

I see there's some discussion already in #3643. #3655 also adds comments documenting the change.

It would be great to fix the regression here. I'm happy to work on a fix, but this is definitely the kind of change that needs some discussion. Some ideas:

1. We could inject a source file into the internal and external test packages for each test that imports bzltestutil. That would add a dependency edge, forcing bzltestutil to be initialized first. It might still be initialized before other transitively imported packages though, so this may not be a sufficient fix.
2. We could set the package path for bzltestutil to something that sorts before anything else. Of course this is an egregious hack, but it's lightweight and would work reasonably well under the new semantics.
3. We could introduce a separate test wrapper executable that calls the main executable without linking in any user code. We already have a test wrapper process, but it runs code linked into the test executable (bzltestutil/wrap.go). Although it would be cleaner to separate that, I think it would mean an extra link action per go_test.

cc @fmeum @sluongng

[fmeum]

Thanks for the mini design doc!

I agree that it's worth fixing this at least if we can find a solution that isn't too complex. I personally prefer 2 as it's really simple if it works and should only really break if someone else is also hacking hard (and I won't be sorry for them).

According to https://go.dev/ref/spec#Packages, either - or a should be the first import path in the lexicographic order. Could you send a PR that gives that a try?

[sluongng]

Im also leaning toward 2. I don't think of it as a hack though, it's a solution that comply to current Go spec. And the spec was made intentionally simple so we should not try to make the fix complicated.

If there are cases that 2 would not work well for, we could revisit 3 in the future as well.

[jayconrod]

2 is also my preference. I'll send a fix early next week.