Skip to content

Commit

Permalink
td: Track deleted files. (#724)
Browse files Browse the repository at this point in the history
This commit updates the target determination to track deleted files.
Mats pointed out that someone could delete a manifest and get a passsing
build. The original thought was that we wouldn't need to build an app if
it was deleted, which is still true. This change returns all changes
including deletes, and validates the directory still exists before
returning it as a target.
  • Loading branch information
betterengineering authored Apr 17, 2023
1 parent 18292b5 commit 718ea0a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
28 changes: 25 additions & 3 deletions cmd/community/targetdeterminator.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ func determineTargets(cmd *cobra.Command, args []string) error {

changedApps := map[string]bool{}
for _, f := range changedFiles {
dir := filepath.Dir(f) + string(os.PathSeparator)

// We only care about things in apps/{{ app package }}/ changing and
// nothing else. Skip any file changes that are not in that structure.
parts := strings.Split(f, string(os.PathSeparator))
Expand All @@ -56,10 +58,17 @@ func determineTargets(cmd *cobra.Command, args []string) error {

// If the filepath does not start with apps, we also don't care about
// it.
dir := filepath.Dir(f) + string(os.PathSeparator)
if strings.HasPrefix(dir, "apps") {
changedApps[dir] = true
if !strings.HasPrefix(dir, "apps") {
continue
}

// If the directory no longer exists, we don't care about it. This would
// happen if someone deleted an app in a PR.
if !dirExists(dir) {
continue
}

changedApps[dir] = true
}

for app := range changedApps {
Expand All @@ -68,3 +77,16 @@ func determineTargets(cmd *cobra.Command, args []string) error {

return nil
}

func dirExists(dir string) bool {
_, err := os.Stat(dir)
if os.IsNotExist(err) {
return false
}

if err != nil {
return false
}

return true
}
15 changes: 2 additions & 13 deletions tools/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/go-git/go-git/v5/plumbing/object"
"github.com/go-git/go-git/v5/utils/merkletrie"
)

// IsInRepo determines if the provided directory is in the provided git
Expand Down Expand Up @@ -106,16 +105,6 @@ func DetermineChanges(dir string, oldCommit string, newCommit string) ([]string,
// Create a unique list of changed files.
changedFiles := []string{}
for _, change := range changes {
action, err := change.Action()
if err != nil {
return nil, fmt.Errorf("couldn't determine action for commit: %w", err)
}

// Skip deleted files.
if action == merkletrie.Delete {
continue
}

changedFiles = append(changedFiles, getChangeName(change))
}

Expand All @@ -125,11 +114,11 @@ func DetermineChanges(dir string, oldCommit string, newCommit string) ([]string,
func getChangeName(change *object.Change) string {
var empty = object.ChangeEntry{}

// Given we skip deletes, this should never be empty. To should be populated
// on inserts and modifications.
// Use the To field for Inserts and Modifications.
if change.To != empty {
return change.To.Name
}

// The From field for Deletes.
return change.From.Name
}

0 comments on commit 718ea0a

Please sign in to comment.