DEVPROD-10420: do not link patch_optional dependencies for alias vers…

…ions (#8348)

docs/Project-Configuration/Project-Configuration-Files.md

@@ -1545,7 +1545,9 @@ parameters are available:
 -   `patch_optional` - boolean (default: false). If true the dependency
     will only exist when the depended on task is present in the version
     at the time the dependent task is created. The depended on task will
-    not be automatically pulled in to the version.
+    not be automatically pulled in to the version. This means that, despite the
+    name of the field, `patch_optional` makes the dependency optional for _all
+    versions, not just patches_.
 -   `omit_generated_tasks` - boolean (default: false). If true and the
     dependency is a generator task (i.e. it generates tasks via the
     [`generate.tasks`](Project-Commands#generatetasks) command), then generated tasks will not be included

model/lifecycle.go

@@ -928,8 +928,9 @@ func makeDeps(deps []TaskUnitDependency, thisTask *task.Task, taskIds TaskIdTabl
 		} else if dep.Name == AllDependencies {
 			depIDs = taskIds.GetIdsForAllTasksInVariant(dep.Variant)
 		} else {
-			// don't add missing dependencies
-			// patch_optional tasks aren't in the patch and will be missing from the table
+			// Don't add missing dependencies - patch_optional dependencies may
+			// not be created and therefore might be missing from the task ID
+			// table.
 			if id := taskIds.GetId(dep.Variant, dep.Name); id != "" {
 				depIDs = []string{id}
 			}
@@ -1123,6 +1124,10 @@ func getTaskCreateTime(creationInfo TaskCreationInfo) (time.Time, error) {
 
 // createOneTask is a helper to create a single task.
 func createOneTask(ctx context.Context, id string, creationInfo TaskCreationInfo, buildVarTask BuildVariantTaskUnit) (*task.Task, error) {
+	if id == "" {
+		return nil, errors.Errorf("cannot create task  '%s' in build variant '%s' for project '%s' with an empty task ID", creationInfo.ProjectRef.Id, buildVarTask.Name, creationInfo.Build.BuildVariant)
+	}
+
 	activateTask := creationInfo.Build.Activated && !creationInfo.ActivationInfo.taskHasSpecificActivation(creationInfo.Build.BuildVariant, buildVarTask.Name)
 
 	// If stepback is enabled, check if the task should be activated via stepback.

model/lifecycle_test.go

@@ -1020,7 +1020,7 @@ func TestCreateBuildFromVersion(t *testing.T) {
 		project, err := TranslateProject(parserProject)
 		So(err, ShouldBeNil)
 		So(project, ShouldNotBeNil)
-		table := NewTaskIdConfigForRepotrackerVersion(project, v, "", "")
+		table := NewTaskIdConfigForRepotrackerVersion(project, v, TVPairSet{}, "", "")
 		tt := table.ExecutionTasks
 		dt := table.DisplayTasks
 
@@ -1724,7 +1724,7 @@ func TestCreateTaskGroup(t *testing.T) {
 		Id:         "projectId",
 		Identifier: projectIdentifier,
 	}
-	table := NewTaskIdConfigForRepotrackerVersion(proj, v, "", "")
+	table := NewTaskIdConfigForRepotrackerVersion(proj, v, TVPairSet{}, "", "")
 
 	creationInfo := TaskCreationInfo{
 		Project:          proj,

model/project.go

@@ -846,19 +846,25 @@ func (t TaskIdConfig) Length() int {
 }
 
 // NewTaskIdConfigForRepotrackerVersion creates a special TaskIdTable for a
-// repotracker version.
-func NewTaskIdConfigForRepotrackerVersion(p *Project, v *Version, sourceRev, defID string) TaskIdConfig {
+// repotracker version. If pairsToCreate is not empty, that means only some of
+// the tasks will be created for this version so only create task IDs for those
+// tasks that actually will be created; otherwise, it will create task IDs for
+// all possible tasks in the version.
+func NewTaskIdConfigForRepotrackerVersion(p *Project, v *Version, pairsToCreate TVPairSet, sourceRev, defID string) TaskIdConfig {
 	// init the variant map
 	execTable := TaskIdTable{}
 	displayTable := TaskIdTable{}
 
+	isCreatingSubsetOfTasks := len(pairsToCreate) > 0
+
 	sort.Stable(p.BuildVariants)
 
 	projectIdentifier, err := GetIdentifierForProject(p.Identifier)
 	if err != nil { // default to ID
 		projectIdentifier = p.Identifier
 	}
 	for _, bv := range p.BuildVariants {
+		taskNamesInBV := pairsToCreate.TaskNames(bv.Name)
 		rev := v.Revision
 		if evergreen.IsPatchRequester(v.Requester) {
 			rev = fmt.Sprintf("patch_%s_%s", v.Revision, v.Id)
@@ -876,10 +882,16 @@ func NewTaskIdConfigForRepotrackerVersion(p *Project, v *Version, sourceRev, def
 			}
 			if tg := p.FindTaskGroup(t.Name); tg != nil {
 				for _, groupTask := range tg.Tasks {
+					if isCreatingSubsetOfTasks && !utility.StringSliceContains(taskNamesInBV, groupTask) {
+						continue
+					}
 					taskId := generateId(groupTask, projectIdentifier, &bv, rev, v)
 					execTable[TVPair{bv.Name, groupTask}] = util.CleanName(taskId)
 				}
 			} else {
+				if isCreatingSubsetOfTasks && !utility.StringSliceContains(taskNamesInBV, t.Name) {
+					continue
+				}
 				// create a unique Id for each task
 				taskId := generateId(t.Name, projectIdentifier, &bv, rev, v)
 				execTable[TVPair{bv.Name, t.Name}] = util.CleanName(taskId)

model/task_creation.go

@@ -22,8 +22,8 @@ type TaskCreationInfo struct {
 	ActivateBuild       bool                    // True if the build should be scheduled
 	ActivationInfo      specificActivationInfo  // Indicates if the task has a specific activation or is a stepback task
 	TasksInBuild        []task.Task             // The set of task names that already exist for the given build, including display tasks
-	TaskNames           []string                // Names of tasks to create (used in patches). Will create all if nil
-	DisplayNames        []string                // Names of display tasks to create (used in patches). Will create all if nil
+	TaskNames           []string                // Names of tasks to create (used in patches). Will create all if empty
+	DisplayNames        []string                // Names of display tasks to create (used in patches). Will create all if empty
 	GeneratedBy         string                  // ID of the task that generated this build
 	SourceRev           string                  // Githash of the revision that triggered this build
 	DefinitionID        string                  // Definition ID of the trigger used to create this build

repotracker/repotracker.go

@@ -654,6 +654,7 @@ func CreateVersionFromConfig(ctx context.Context, projectInfo *model.ProjectInfo
 
 		}
 	}
+
 	var aliases model.ProjectAliases
 	if metadata.Alias == evergreen.GitTagAlias {
 		aliases, err = model.FindMatchingGitTagAliasesInProject(projectInfo.Ref.Id, metadata.GitTag.Tag)
@@ -808,7 +809,6 @@ func createVersionItems(ctx context.Context, v *model.Version, metadata model.Ve
 	if metadata.SourceVersion != nil {
 		sourceRev = metadata.SourceVersion.Revision
 	}
-	taskIds := model.NewTaskIdConfigForRepotrackerVersion(projectInfo.Project, v, sourceRev, metadata.TriggerDefinitionID)
 
 	// create all builds for the version
 	buildsToCreate := []interface{}{}
@@ -869,6 +869,7 @@ func createVersionItems(ctx context.Context, v *model.Version, metadata model.Ve
 	}))
 	batchTimeCatcher := grip.NewBasicCatcher()
 	debuggingData := map[string]string{}
+
 	var githubCheckAliases model.ProjectAliases
 	if v.Requester == evergreen.RepotrackerVersionRequester && projectInfo.Ref.IsGithubChecksEnabled() {
 		githubCheckAliases, err = model.FindAliasInProjectRepoOrConfig(v.Identifier, evergreen.GithubChecksAlias)
@@ -878,6 +879,9 @@ func createVersionItems(ctx context.Context, v *model.Version, metadata model.Ve
 			"version": v.Id,
 		}))
 	}
+
+	taskIds := model.NewTaskIdConfigForRepotrackerVersion(projectInfo.Project, v, pairsToCreate, sourceRev, metadata.TriggerDefinitionID)
+
 	for _, buildvariant := range projectInfo.Project.BuildVariants {
 		taskNames := pairsToCreate.TaskNames(buildvariant.Name)
 		var aliasesMatchingVariant model.ProjectAliases

repotracker/repotracker_test.go

@@ -280,7 +280,7 @@ func TestBatchTimeForTasks(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	assert.NoError(t, db.ClearCollections(model.VersionCollection, distro.Collection, model.ParserProjectCollection,
-		build.Collection, task.Collection, model.ProjectConfigCollection, model.ProjectRefCollection), ShouldBeNil)
+		build.Collection, task.Collection, model.ProjectConfigCollection, model.ProjectRefCollection))
 
 	simpleYml := `
 buildvariants:
@@ -387,10 +387,12 @@ tasks:
 
 	build1, err := build.FindOneId(bv.BuildId)
 	assert.NoError(t, err)
+	require.NotZero(t, build1)
 
 	// neither batchtime task nor disabled task should be activated
 	tasks, err := task.Find(task.ByBuildId(build1.Id))
 	assert.NoError(t, err)
+	require.NotEmpty(t, tasks)
 	assert.Len(t, tasks, 3)
 	for _, tsk := range tasks {
 		if tsk.DisplayName == "t1" {
@@ -1245,6 +1247,194 @@ tasks:
 	s.Len(dbTasks, 2)
 }
 
+func (s *CreateVersionFromConfigSuite) TestWithAliasAndPatchOptionalDependencyDoesNotCreateDependentTaskAutomatically() {
+	configYml := `
+buildvariants:
+- name: bv1
+  display_name: bv_display
+  run_on: d
+  tasks:
+  - name: task1
+  - name: task2
+tasks:
+- name: task1
+  depends_on:
+  - name: task2
+    patch_optional: true
+- name: task2
+`
+	p := &model.Project{}
+	pp, err := model.LoadProjectInto(s.ctx, []byte(configYml), nil, s.ref.Id, p)
+	s.NoError(err)
+
+	alias := model.ProjectAlias{
+		Alias:     "task1_alias",
+		ProjectID: s.ref.Id,
+		Task:      "task1",
+		Variant:   ".*",
+	}
+	s.NoError(alias.Upsert())
+
+	projectInfo := &model.ProjectInfo{
+		Ref:                 s.ref,
+		IntermediateProject: pp,
+		Project:             p,
+	}
+	metadata := model.VersionMetadata{
+		Revision: *s.rev,
+		Alias:    alias.Alias,
+	}
+	v, err := CreateVersionFromConfig(s.ctx, projectInfo, metadata, false, nil)
+	s.NoError(err)
+	s.Require().NotNil(v)
+	s.Len(v.Errors, 0)
+
+	tasks, err := task.Find(task.ByVersion(v.Id))
+	s.NoError(err)
+	s.Require().Len(tasks, 1)
+	s.Equal("task1", tasks[0].DisplayName, "should create task matching alias")
+	s.Equal("bv1", tasks[0].BuildVariant, "should create task matching alias in build variant")
+	s.Empty(tasks[0].DependsOn, "should not automatically create patch_optional dependency since the task is not created")
+}
+
+func (s *CreateVersionFromConfigSuite) TestWithAliasMatchingTaskGroup() {
+	configYml := `
+buildvariants:
+- name: bv1
+  display_name: bv_display
+  run_on: d
+  tasks:
+  - name: tg1
+  - name: task3
+tasks:
+- name: task1
+- name: task2
+- name: task3
+task_groups:
+- name: tg1
+  max_hosts: 2
+  tasks:
+  - task1
+  - task2
+`
+	p := &model.Project{}
+	pp, err := model.LoadProjectInto(s.ctx, []byte(configYml), nil, s.ref.Id, p)
+	s.NoError(err)
+
+	alias := model.ProjectAlias{
+		Alias:     "tg_alias",
+		ProjectID: s.ref.Id,
+		Task:      "tg1",
+		Variant:   ".*",
+	}
+	s.NoError(alias.Upsert())
+
+	projectInfo := &model.ProjectInfo{
+		Ref:                 s.ref,
+		IntermediateProject: pp,
+		Project:             p,
+	}
+	metadata := model.VersionMetadata{
+		Revision: *s.rev,
+		Alias:    alias.Alias,
+	}
+	v, err := CreateVersionFromConfig(s.ctx, projectInfo, metadata, false, nil)
+	s.NoError(err)
+	s.Require().NotNil(v)
+	s.Len(v.Errors, 0)
+
+	tasks, err := task.Find(task.ByVersion(v.Id))
+	s.NoError(err)
+	s.Require().Len(tasks, 2)
+	var foundTask1, foundTask2 bool
+	for _, tsk := range tasks {
+		switch tsk.DisplayName {
+		case "task1":
+			s.Equal("bv1", tsk.BuildVariant)
+			foundTask1 = true
+		case "task2":
+			s.Equal("bv1", tsk.BuildVariant)
+			foundTask2 = true
+		default:
+			s.FailNow("unexpected task created", tsk.DisplayName)
+		}
+	}
+
+	s.True(foundTask1, "should create task1 because it matches alias")
+	s.True(foundTask2, "should create task2 because it matches alias")
+}
+
+func (s *CreateVersionFromConfigSuite) TestWithAliasAndPatchOptionalDependencyCreatesDependencyIfDependentTaskIsCreated() {
+	configYml := `
+buildvariants:
+- name: bv1
+  display_name: bv_display
+  run_on: d
+  tasks:
+  - name: task1
+  - name: task2
+  - name: task3
+tasks:
+- name: task1
+  depends_on:
+  - name: task2
+    patch_optional: true
+- name: task2
+- name: task3
+`
+	p := &model.Project{}
+	pp, err := model.LoadProjectInto(s.ctx, []byte(configYml), nil, s.ref.Id, p)
+	s.NoError(err)
+
+	alias := model.ProjectAlias{
+		Alias:     "task_alias",
+		ProjectID: s.ref.Id,
+		Task:      "(task1)|(task2)",
+		Variant:   ".*",
+	}
+	s.NoError(alias.Upsert())
+
+	projectInfo := &model.ProjectInfo{
+		Ref:                 s.ref,
+		IntermediateProject: pp,
+		Project:             p,
+	}
+	metadata := model.VersionMetadata{
+		Revision: *s.rev,
+		Alias:    alias.Alias,
+	}
+	v, err := CreateVersionFromConfig(s.ctx, projectInfo, metadata, false, nil)
+	s.NoError(err)
+	s.Require().NotNil(v)
+	s.Len(v.Errors, 0)
+
+	tasks, err := task.Find(task.ByVersion(v.Id))
+	s.NoError(err)
+	s.Len(tasks, 2)
+
+	var foundTask1, foundTask2 bool
+	var task1DepID, task2ID string
+	for _, tsk := range tasks {
+		switch tsk.DisplayName {
+		case "task1":
+			s.Equal("bv1", tsk.BuildVariant)
+			s.Require().Len(tsk.DependsOn, 1, "should create patch_optional dependency since the task is created")
+			task1DepID = tsk.DependsOn[0].TaskId
+			foundTask1 = true
+		case "task2":
+			s.Equal("bv1", tsk.BuildVariant)
+			task2ID = tsk.Id
+			foundTask2 = true
+		default:
+			s.FailNow("unexpected task created", tsk.DisplayName)
+		}
+	}
+
+	s.True(foundTask1, "should create task1 because it matches alias")
+	s.True(foundTask2, "should create task2 because it matches alias")
+	s.Equal(task1DepID, task2ID, "task1 should depend on task2 even with patch_optional dependency because task2 was created")
+}
+
 func TestCreateManifest(t *testing.T) {
 	assert := assert.New(t)
 	settings := testutil.TestConfig()

units/check_blocked_tasks.go

@@ -127,12 +127,12 @@ func checkUnmarkedBlockingTasks(ctx context.Context, t *task.Task, dependencyCac
 
 	dependenciesMet, err := t.DependenciesMet(dependencyCaches)
 	if err != nil {
-		grip.Debug(message.Fields{
-			"message":      "checking if dependencies met for task",
+		grip.Debug(message.WrapError(err, message.Fields{
+			"message":      "could not check if dependencies met for task",
 			"task_id":      t.Id,
 			"activated_by": t.ActivatedBy,
 			"depends_on":   t.DependsOn,
-		})
+		}))
 		return errors.Wrapf(err, "checking if dependencies met for task '%s'", t.Id)
 	}
 	if dependenciesMet {