tiltextension: fix a regression where we stopped pulling the default …

…repo (#5538)

also some minor usability fixes

internal/controllers/core/extensionrepo/reconciler.go

@@ -230,8 +230,11 @@ func (r *Reconciler) reconcileDownloaderRepo(ctx context.Context, state *repoSta
 	state.lastFetch = time.Now()
 
 	needsDownload := true
-	if exists && state.spec.Ref != "" {
-		// If an explicit ref is specified, there's no reason to pull a new version.
+	if exists && state.spec.Ref != "" && state.spec.Ref != "HEAD" {
+		// If an explicit ref is specified, we assume there's no reason to pull a new version.
+		//
+		// TODO(nick): Should we try to support cases where the ref can change server-side?
+		// e.g., a "stable" tag.
 		err := r.dlr.RefSync(importPath, state.spec.Ref)
 		if err == nil {
 			needsDownload = false

internal/controllers/core/extensionrepo/reconciler_test.go

@@ -153,6 +153,31 @@ func TestRepoSyncExisting(t *testing.T) {
 	f.assertSteadyState(&repo)
 }
 
+func TestRepoAlwaysSyncHead(t *testing.T) {
+	f := newFixture(t)
+
+	key := types.NamespacedName{Name: "default"}
+	repo := v1alpha1.ExtensionRepo{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: key.Name,
+		},
+		Spec: v1alpha1.ExtensionRepoSpec{
+			URL: "https://github.com/tilt-dev/tilt-extensions",
+			Ref: "HEAD",
+		},
+	}
+
+	f.dlr.Download("github.com/tilt-dev/tilt-extensions")
+	f.dlr.RefSync("github.com/tilt-dev/tilt-extensions", "HEAD")
+
+	f.Create(&repo)
+	f.MustGet(key, &repo)
+	require.Equal(t, repo.Status.Error, "")
+	assert.Equal(t, 2, f.dlr.downloadCount)
+	assert.Equal(t, "HEAD", f.dlr.lastRefSync)
+	f.assertSteadyState(&repo)
+}
+
 type fixture struct {
 	*fake.ControllerFixture
 	r    *Reconciler
@@ -217,7 +242,7 @@ func (d *fakeDownloader) Download(pkg string) (string, error) {
 
 	_, err = os.Stat(path)
 	exists := err == nil
-	if exists && d.lastRefSync != "" {
+	if exists && d.lastRefSync != "" && d.lastRefSync != "HEAD" {
 		// If the current disk state is checked out to a ref, then
 		// we expect Download() to fail.
 		// https://github.com/tilt-dev/tilt/issues/5508

internal/tiltfile/tiltextension/plugin.go

@@ -165,7 +165,6 @@ func (e *Plugin) ensureRepo(t *starlark.Thread, objSet apiset.ObjectSet, repoNam
 		},
 		Spec: v1alpha1.ExtensionRepoSpec{
 			URL: "https://github.com/tilt-dev/tilt-extensions",
-			Ref: "HEAD",
 		},
 	}
 

internal/tiltfile/v1alpha1/plugin.go

@@ -7,6 +7,7 @@ import (
 	"go.starlark.net/starlark"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 
+	"github.com/tilt-dev/tilt/internal/controllers/apicmp"
 	"github.com/tilt-dev/tilt/internal/controllers/apiset"
 	"github.com/tilt-dev/tilt/internal/tiltfile/starkit"
 )
@@ -45,8 +46,8 @@ func (p Plugin) register(t *starlark.Thread, obj apiset.Object) (starlark.Value,
 	err := starkit.SetState(t, func(set apiset.ObjectSet) (apiset.ObjectSet, error) {
 		typedSet := set.GetOrCreateTypedSet(obj)
 		name := obj.GetName()
-		_, exists := typedSet[name]
-		if exists {
+		existing, exists := typedSet[name]
+		if exists && !apicmp.DeepEqual(obj, existing) {
 			return set, fmt.Errorf("%s %q already registered", obj.GetGroupVersionResource().Resource, name)
 		}
 

internal/tiltfile/v1alpha1/plugin_test.go

@@ -35,11 +35,29 @@ v1alpha1.extension(name='cancel', repo_name='default', repo_path='cancel')
 	require.Equal(t, "default", ext.Spec.RepoName)
 }
 
+func TestExtensionRepoTwice(t *testing.T) {
+	f := newFixture(t)
+
+	// Some teams include a team extension repo in each tiltfile. It's OK as long as they match.
+	f.File("Tiltfile", `
+v1alpha1.extension_repo(name='default', url='https://github.com/tilt-dev/tilt-extensions')
+v1alpha1.extension_repo(name='default', url='https://github.com/tilt-dev/tilt-extensions')
+`)
+	result, err := f.ExecFile("Tiltfile")
+	require.NoError(t, err)
+
+	set := MustState(result)
+
+	repo := set.GetSetForType(&v1alpha1.ExtensionRepo{})["default"].(*v1alpha1.ExtensionRepo)
+	require.NotNil(t, repo)
+	require.Equal(t, "https://github.com/tilt-dev/tilt-extensions", repo.Spec.URL)
+}
+
 func TestExtensionArgs(t *testing.T) {
 	f := newFixture(t)
 
 	f.File("Tiltfile", `
-v1alpha1.extension_repo(name='default', url='https://github.com/tilt-dev/tilt-extensions', ref='HEAD')
+v1alpha1.extension_repo(name='default', url='https://github.com/tilt-dev/tilt-extensions')
 v1alpha1.extension(name='cancel', repo_name='default', repo_path='cancel', args=['--namespace=foo'])
 `)
 	result, err := f.ExecFile("Tiltfile")