buildcontrol: avoid spurious changes to the 'waiting' state on disabl…

…ed resources

Before this change, the "waiting" state of disabled resources would be
continuously in flux. This is expensive and doesn't even make sense from a data
model perspective - a disabled resource isn't waiting or being held.

internal/engine/buildcontrol/build_control.go

@@ -14,11 +14,9 @@ import (
 	"github.com/tilt-dev/tilt/pkg/model"
 )
 
-// NOTE(maia): we eventually want to move the BuildController into its own package
-// (as we do with all subscribers), but for now, just move the underlying functions
-// so they can be used from elsewhere.
-
 // Algorithm to choose a manifest to build next.
+//
+// The HoldSet is used in the UI to display why a resource is waiting.
 func NextTargetToBuild(state store.EngineState) (*store.ManifestTarget, HoldSet) {
 	holds := HoldSet{}
 
@@ -80,7 +78,6 @@ func NextTargetToBuild(state store.EngineState) (*store.ManifestTarget, HoldSet)
 
 	HoldTargetsWithBuildingComponents(state, targets, holds)
 	HoldTargetsWaitingOnDependencies(state, targets, holds)
-	HoldDisabledTargets(targets, holds)
 	HoldTargetsWaitingOnCluster(state, targets, holds)
 
 	// If any of the manifest targets haven't been built yet, build them now.
@@ -279,14 +276,6 @@ func HoldTargetsWaitingOnDependencies(state store.EngineState, mts []*store.Mani
 	}
 }
 
-func HoldDisabledTargets(mts []*store.ManifestTarget, holds HoldSet) {
-	for _, mt := range mts {
-		if mt.State.DisableState == v1alpha1.DisableStateDisabled {
-			holds.AddHold(mt, store.Hold{Reason: store.HoldReasonDisabled})
-		}
-	}
-}
-
 // Helper function for ordering targets that have never been built before.
 func NextUnbuiltTargetToBuild(unbuilt []*store.ManifestTarget) *store.ManifestTarget {
 	// Local resources come before all cluster resources, because they
@@ -448,6 +437,11 @@ func FindTargetsNeedingAnyBuild(state store.EngineState) []*store.ManifestTarget
 
 	result := []*store.ManifestTarget{}
 	for _, target := range state.Targets() {
+		// Skip disabled targets.
+		if target.State.DisableState == v1alpha1.DisableStateDisabled {
+			continue
+		}
+
 		if !target.State.StartedFirstBuild() && target.Manifest.TriggerMode.AutoInitial() {
 			result = append(result, target)
 			continue

internal/engine/buildcontrol/build_control_test.go

@@ -524,8 +524,6 @@ func TestHoldDisabled(t *testing.T) {
 
 	f.upsertLocalManifest("local")
 	f.st.ManifestTargets["local"].State.DisableState = v1alpha1.DisableStateDisabled
-
-	f.assertHold("local", store.HoldReasonDisabled)
 	f.assertNoTargetNextToBuild()
 }
 

internal/engine/upper_test.go

@@ -3536,24 +3536,6 @@ func TestOverrideTriggerModeBadTriggerModeLogsError(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestDisablingResourcePreventsBuild(t *testing.T) {
-	f := newTestFixture(t)
-
-	m := manifestbuilder.New(f, "foo").WithLocalResource("foo", []string{f.Path()}).Build()
-
-	f.Start([]model.Manifest{m})
-
-	f.setDisableState(m.Name, true)
-
-	action := server.AppendToTriggerQueueAction{Name: "foo", Reason: 123}
-	f.store.Dispatch(action)
-
-	f.WaitUntil("is waiting+disabled", func(state store.EngineState) bool {
-		_, holds := buildcontrol.NextTargetToBuild(state)
-		return holds["foo"].Reason == store.HoldReasonDisabled
-	})
-}
-
 func TestDisableButtonIsCreated(t *testing.T) {
 	f := newTestFixture(t)
 	f.useRealTiltfileLoader()

internal/hud/webview/convert.go

@@ -429,7 +429,10 @@ func LogSegmentToEvent(seg *proto_webview.LogSegment, spans map[string]*proto_we
 }
 
 func holdToWaiting(hold store.Hold) *v1alpha1.UIResourceStateWaiting {
-	if hold.Reason == store.HoldReasonNone {
+	if hold.Reason == store.HoldReasonNone ||
+		// "Reconciling" just means the live update is handling the update (rather
+		// than the BuildController) and isn't indicative of a real waiting status.
+		hold.Reason == store.HoldReasonReconciling {
 		return nil
 	}
 	waiting := &v1alpha1.UIResourceStateWaiting{

internal/store/hold.go

@@ -32,7 +32,6 @@ const (
 	HoldReasonBuildingComponent                HoldReason = "building-component"
 	HoldReasonWaitingForDep                    HoldReason = "waiting-for-dep"
 	HoldReasonWaitingForDeploy                 HoldReason = "waiting-for-deploy"
-	HoldReasonDisabled                         HoldReason = "disabled"
 
 	// We're waiting for a reconciler to respond to the change,
 	// but don't know yet what it's waiting on.