Skip to content

Conversation

@copybara-service
Copy link

@copybara-service copybara-service bot commented Jan 5, 2026

Update Go to 1.25.5

  • checkescape: improve error output
    The output now includes stderr on failure e.g.:

    checkescape: tools/checkescape/test2/test2.go:98:6: stack: possible split on function entry (2 omitted) → (possible, error running "external/rules_go++go_sdk+main___download_0/bin/go tool objdump bazel-out/k8-fastbuild-ST-04c87098fb12/bin/tools/checkescape/test2/test2.a": exit status 2 (go: no such tool "objdump")) (GOOARCH=amd64, GOOS=linux)
    

    Simplify various bits of the implementation while I'm here:

    • Use a set of strings rather than a list to avoid some O(n) operations.
    • Use CommandContext to ensure proper cleanup.
    • Remove stderr pipe and goroutine by setting Stderr to a
      bytes.Buffer.
    • Use bufio.Scanner instead of manually calling
      (*bufio.Reader).ReadString and handling errors.
    • Remove NextLine loop label.
  • go_stateify: plumb go binary to goimports
    This avoids go_stateify relying on go being available outside of the
    bazel sandbox.

  • Update Go to 1.25.5
    Reduce duplication of the version information in various places:

    • Use go_sdk.from_file instead of repeating the version encoded in
      go.mod in MODULE.bazel.

    Add a workaround to allow objdump to be lazily compiled in the manner
    introduced in https://go.dev/issue/71867 when invoked under bazel in
    checkescape.

FUTURE_COPYBARA_INTEGRATE_REVIEW=#12452 from tamird:update-go 6c7f640

@copybara-service copybara-service bot added the exported Issue was exported automatically label Jan 5, 2026
@copybara-service copybara-service bot force-pushed the test/cl852335920 branch 8 times, most recently from ab636fe to 0e9ff5c Compare January 6, 2026 21:59
@tamird
Copy link
Contributor

tamird commented Jan 6, 2026

@avagin looks like internal changes are causing 077098a to be discarded :(

@copybara-service copybara-service bot force-pushed the test/cl852335920 branch 3 times, most recently from 336dc29 to c9d2c57 Compare January 7, 2026 08:33
Copy link
Contributor

@tamird tamird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avagin how come images/gpu/cuda-tests-12-8/install_go.sh is restored relative to #12365 ?

Comment on lines 32 to 45
go_version=1.25.5
go_base_url=https://dl.google.com/go/
arch="$(uname -m)"
case "$arch" in
x86_64) go_arch="amd64" ;;
aarch64) go_arch="arm64" ;;
*) echo "Unsupported architecture: $arch" >&2; exit 1 ;;
esac
go_filename="go${go_version}.linux-${go_arch}.tar.gz"
go_url="${go_base_url}${go_filename}"
mkdir go-bootstrap
curl -sSL "${go_url}" | tar -xz -C go-bootstrap
GOROOT_BOOTSTRAP="$(pwd)/go-bootstrap/go"
export GOROOT_BOOTSTRAP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avagin how come all this is needed? shall I add a commit to my PR so that we have something in the history explaining it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr removes golang from the default image, but here a custom golang is compiled and this process requires a "bootstrap" compiler.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks for explaining. I pulled this into my PR, could we keep the diff between this and that small please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also moved it to the proper commit and updated the message.

@copybara-service copybara-service bot force-pushed the test/cl852335920 branch 2 times, most recently from 27adedb to 42fddb2 Compare January 7, 2026 18:22
@tamird
Copy link
Contributor

tamird commented Jan 7, 2026

@avagin how come images/gpu/cuda-tests-12-8/install_go.sh is restored relative to #12365 ?

This is still the case - is it necessary? I'd prefer to avoid removing it in my commit and then having it reappear in the merge commit.

@avagin
Copy link
Collaborator

avagin commented Jan 7, 2026

@avagin how come images/gpu/cuda-tests-12-8/install_go.sh is restored relative to #12365 ?

This is still the case - is it necessary? I'd prefer to avoid removing it in my commit and then having it reappear in the merge commit.

I reverted it. Let's do that in another pr, otherwise we will never merged this one.

@tamird
Copy link
Contributor

tamird commented Jan 7, 2026

@avagin how come images/gpu/cuda-tests-12-8/install_go.sh is restored relative to #12365 ?

This is still the case - is it necessary? I'd prefer to avoid removing it in my commit and then having it reappear in the merge commit.

I reverted it. Let's do that in another pr, otherwise we will never merged this one.

Since we're still hitting failures, I sent this as a separate change in #12458.

@copybara-service copybara-service bot force-pushed the test/cl852335920 branch 10 times, most recently from ee6d277 to 998b441 Compare January 8, 2026 06:16
@copybara-service copybara-service bot closed this Jan 8, 2026
@copybara-service copybara-service bot deleted the test/cl852335920 branch January 8, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exported Issue was exported automatically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants