Skip to content

fix(golang): override snapshot to pick up 1.25.8 cert fix#16274

Open
WithEnoughCoffee wants to merge 1 commit intotomls/base/mainfrom
fix/golang-expired-cert
Open

fix(golang): override snapshot to pick up 1.25.8 cert fix#16274
WithEnoughCoffee wants to merge 1 commit intotomls/base/mainfrom
fix/golang-expired-cert

Conversation

@WithEnoughCoffee
Copy link
Copy Markdown

@WithEnoughCoffee WithEnoughCoffee commented Mar 24, 2026

Summary

The net/smtp test certificate (localhostCert) expired on 2026-03-18, causing %check failures in golang-1.25.7 builds. The fix landed in Go 1.25.8 (upstream, backport) and Fedora f43 updated on ~2026-03-06.

The default snapshot (2026-02-24) predates this fix.

Changes

  1. Snapshot override — Create golang/golang.comp.toml with a Mar 10 snapshot to pick up golang-1.25.8-1.fc43.
  2. Source-files origin — The internal lookaside cache has not synced the go1.25.8.src.tar.gz tarball yet, so a source-files entry with an upstream download origin (go.dev) is added to bypass the cache. This follows the same pattern as kernel.comp.toml.

Merge Checklist

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted
  • LICENSE-MAP files are up-to-date
  • All source files have up-to-date hashes in the signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge

Does this affect the toolchain?

YES — golang is a toolchain component. Dependent packages with static linkage will need Release bumps.

Test Methodology

Addressed comments from copilot review , build passed : https://controltower-dev-jwisitgpr74k6-gjb0fchvgkbnamgp.b01.azurefd.net/workflow/package/02ec0073-68a5-4ee5-41bd-08de8a9d163b

build to test after latest comments addressed. https://controltower-dev-jwisitgpr74k6-gjb0fchvgkbnamgp.b01.azurefd.net/workflow/61a6e605-6760-4651-cda6-08de8c31d520

@WithEnoughCoffee WithEnoughCoffee marked this pull request as ready for review March 25, 2026 19:12
Copilot AI review requested due to automatic review settings March 25, 2026 19:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses golang %check failures caused by an expired net/smtp test certificate by pinning the Fedora 43 snapshot to one that includes the Go 1.25.8 fix, and by adding a direct upstream source download entry for the new tarball while internal lookaside is not yet synced.

Changes:

  • Add a dedicated golang.comp.toml to override the Fedora 43 snapshot to 2026-03-10 and pick up golang-1.25.8-1.fc43.
  • Add a source-files entry to download go1.25.8.src.tar.gz directly from go.dev.
  • Remove the inline golang entry from components-full.toml (now defined via the dedicated component file).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
base/comps/golang/golang.comp.toml Pins Fedora snapshot for golang and adds direct upstream tarball download metadata.
base/comps/components-full.toml Removes the inline golang component entry in favor of the dedicated comp file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tobiasb-ms
tobiasb-ms previously approved these changes Mar 25, 2026
# TODO: Drop this explicit Fedora 43 snapshot override once the default Fedora 43 snapshot
# in distro/azurelinux.distro.toml advances to a build that includes golang-1.25.8-1.fc43
# (net/smtp TLS test certificate fix).
spec = { type = "upstream", upstream-distro = { name = "fedora", version = "43", snapshot = "2026-03-10T00:00:00-08:00" } }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question (non-blocking): Out of curiosity, how did you choose 03/10? Nothing wrong with it, but I think that change went in on 03/06 -- https://src.fedoraproject.org/rpms/golang/c/e0faaabdb215feb6be2b3232f480a03ce76ca132?branch=f43.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I picked 03/10 to match the existing rust snapshot override (rust.comp.toml also uses 2026-03-10),
which gives a few days of buffer after the Fedora f43 commit on 03/06 to ensure the built RPM was available in the repo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@WithEnoughCoffee -- Can we please use a specific commit instead? We need to use the timestamp for the global default, but we should prefer a specific commit hash for individual components.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed

@tobiasb-ms tobiasb-ms self-requested a review March 26, 2026 17:18
@tobiasb-ms tobiasb-ms dismissed their stale review March 26, 2026 17:21

Other feedback makes it clear that I approved too soon.

@WithEnoughCoffee WithEnoughCoffee force-pushed the fix/golang-expired-cert branch from 0c27f58 to 69b426f Compare March 26, 2026 20:55
Copy link
Copy Markdown
Collaborator

@christopherco christopherco left a comment

Choose a reason for hiding this comment

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

Other than the one comment I left, rest generally LGTM. Once fixed, I can re-review.
Also I see there are currently 2 commits in this PR, but since this is one functional change unit, this should really be just one commit in the end after all the feedback is applied in. Please do a rebase squash so your 2 commits are combined into just 1 commit. We're moving to a "rebase-merge" policy instead of a "squash-merge" policy so we will be looking for clean commit messages and commit history in branch (and I am authoring the Contributing.md to clarify this for new inbound contributions)


## Commit Messages

Do NOT add `Co-authored-by: Copilot` trailers to any commit messages.
Copy link
Copy Markdown
Collaborator

@christopherco christopherco Mar 27, 2026

Choose a reason for hiding this comment

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

issue: This diff to the copilot-instructions is not relevant to this PR. Please remove and make copilot-instruction updates in a separate dedicated PR.

This is separate to whether we should or should not attribute changes to Copilot, which personally I think we should give attribution to Copilot when we use Copilot to generate parts of the changes

@WithEnoughCoffee WithEnoughCoffee force-pushed the fix/golang-expired-cert branch from 69b426f to 1a6ae7c Compare March 27, 2026 19:32
Go 1.25.8 fixes an expired net/smtp TLS test certificate that causes
%check failures after 2026-03-18 (golang/go#77504, golang/go#77531).

- Add dedicated golang.comp.toml with upstream-commit pinned to Fedora
  commit e0faaabdb2 (golang-1.25.8-1.fc43)
- Remove golang entry from components-full.toml (now in dedicated file)
@WithEnoughCoffee WithEnoughCoffee force-pushed the fix/golang-expired-cert branch from 1a6ae7c to 1cffcfb Compare March 27, 2026 19:36
@WithEnoughCoffee
Copy link
Copy Markdown
Author

All commits have been squashed and comments have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants