Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/dist: add support for broken ports #56679

Closed
ianlancetaylor opened this issue Nov 9, 2022 · 5 comments
Closed

cmd/dist: add support for broken ports #56679

ianlancetaylor opened this issue Nov 9, 2022 · 5 comments
Assignees
@dmitshur
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.21

Comments

@ianlancetaylor
Copy link
Contributor

In #53383 we said the following:

  • We will introduce a new concept: the broken port.
    • If a port stops working, including the case where a builder stops working, we can decide to mark the port as broken.
      • Or in some cases we can roll back the change that broke it; this is a judgement call.
    • In general, a port can be considered broken if its builder has failed multiple times in a development cycle with a failure mode that does not occur for first class ports, and that failure mode is not believed to have been fixed or suppressed by a change in either a Go repository or the builder's configuration, and maintainers are not actively working on a solution.
    • Any approver can declare that a port that meets these criteria is broken.
    • The list of broken ports will be maintained in cmd/dist and will appear in the release notes if broken at release time.
    • Attempting to build a broken port will fail unless make.bash or go tool dist is invoked with a new option -force.
    • If a port is broken in release 1.N, then the core Go team can choose to remove the port from release 1.N+1.
      • This is not obligatory and will depend on whether anybody is willing and able to maintain the port going forward.
    • We currently have a list of incomplete ports in cmd/dist/build.go; these should probably be treated as broken ports.
      • Currently the only entry on that list is linux/sparc64.
    • The goal here is not to get ports out of the tree; if people are actively working on the port they should have as much as latitude as possible to fix it.

This issue is about adding that support to cmd/dist:

  • A list of broken ports.
  • An error by default when trying to build a broken port.
  • A new -force option for both cmd/dist and make.bash to build a port that is marked as broken.
@ianlancetaylor
Copy link
Contributor Author

CC @toothrot

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 9, 2022
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Nov 9, 2022
@ianlancetaylor ianlancetaylor mentioned this issue Nov 9, 2022
Closed
@dmitshur dmitshur assigned dmitshur and unassigned toothrot Nov 29, 2022
@dmitshur dmitshur moved this to Planned in Go Release Nov 29, 2022
@dmitshur dmitshur moved this from Planned to In Progress in Go Release Dec 16, 2022
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Dec 16, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/458516 mentions this issue: cmd/dist: mark linux/sparc64 as a broken port, remove incomplete map

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/458515 mentions this issue: cmd/dist: add map of broken ports and -force flag

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/458517 mentions this issue: dashboard: add makeScriptArgs for fine control over make-bash args

gopherbot pushed a commit to golang/build that referenced this issue Jan 10, 2023
@dmitshur @gopherbot
dashboard: add makeScriptArgs for fine control over make script args
15934aa 
When a port is restored after being marked as broken, its builder won't
be able to test the port unless the -force flag is passed to make.bash.

This change makes it possible to configure such builders as so:

	makeScriptArgs: []string{
		// TODO(go.dev/issue/nnn): Temporarily force build
		// until the port is no longer marked as broken.
		"-force",
	},

To see test results before the port is no longer marked as broken.

This change stops reusing the value of allScriptArgs for make.bash args.
That doesn't break anything since only the misc-compile builders have a
non-empty allScriptArgs set, and that value is needed for buildall.bash,
not for make.bash.

For golang/go#56679.

Change-Id: I6b7a51a7dc1a00674bae4bddf4fca5c96c06fa15
Reviewed-on: https://go-review.googlesource.com/c/build/+/458517
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Jan 20, 2023
@dmitshur @gopherbot
cmd/dist: add map of broken ports and -force flag
399ad79 
It's empty so far. The next CL adds linux/sparc64.

Also add -force flag to the bootstrap.bash script
so that it's possible to use it with broken ports.

For #56679.

Change-Id: I09c733d0df0a68df34fb808eae29be010a6da461
Reviewed-on: https://go-review.googlesource.com/c/go/+/458515
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
gopherbot pushed a commit that referenced this issue Jan 20, 2023
@dmitshur @gopherbot
cmd/dist: mark linux/sparc64 as a broken port, remove incomplete map
1011cca 
The linux/sparc64 port is incomplete—it doesn't work, and it doesn't
have a builder. Now that dist supports broken ports, mark it as such.

The incomplete map was created to hide ports that aren't functional
from dist list output. Now that we have the broken port concept, it
seems largely redundant, so remove it for now.

For #56679.
Updates #28944.

Change-Id: I34bd23e913ed6d786a4d0aa8d2852f2b926fe4b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/458516
Auto-Submit: Dmitri Shuralyov <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
@dmitshur
Copy link
Contributor

dmitshur commented Jan 20, 2023 •
edited
Loading

With the changes above, I believe this is complete. Closing. Please comment if I missed something.

(For now, only the incomplete linux/sparc64 port was marked as broken. If some approvers believe another port should be marked, that should be handled by filing a new issue.)

@github-project-automation github-project-automation bot moved this from In Progress to Done in Go Release Jan 20, 2023
@cristaloleg cristaloleg mentioned this issue Apr 30, 2023
Open
@golang golang locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dmitshur dmitshur

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Release
Archived in project
Milestone
Go1.21
Development

No branches or pull requests
4 participants
@toothrot @dmitshur @ianlancetaylor @gopherbot