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.

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

feat: cleanup change detection and add Git commit hash detection #603

Merged
merged 9 commits into from
Mar 17, 2025
1 change: 1 addition & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
"logdir",
"mainboard",
"mainboarddir",
"mapfile",
"markdownlint",
"megalinter",
"menuconfig",
Expand Down
4 changes: 3 additions & 1 deletion cmd/firmware-action/filesystem/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func LoadLastRunTime(pathLastRun string) (time.Time, error) {
// Return zero time if file doesn't exist
err := CheckFileExists(pathLastRun)
if errors.Is(err, os.ErrNotExist) {
return time.Time{}, nil
return time.Time{}, fmt.Errorf("%w: %s", os.ErrNotExist, pathLastRun)
}

content, err := os.ReadFile(pathLastRun)
Expand Down Expand Up @@ -290,6 +290,8 @@ func sanitizeAndTruncate(input string, length int) string {
// https://www.compart.com/en/unicode/category/Cc
result = strings.ToValidUTF8(result, "_")

// slicing works with bytes, not runes
// range works with runes
if len(result) > length {
return string(result[:length])
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/firmware-action/filesystem/filesystem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestLastSaveRunTime(t *testing.T) {

// Load - should fallback because no file exists, but no error
loadTime, err := LoadLastRunTime(pathTimeFile)
assert.NoError(t, err)
assert.ErrorIs(t, err, os.ErrNotExist)
assert.Equal(t, time.Time{}, loadTime)
assert.ErrorIs(t, CheckFileExists(pathTimeFile), os.ErrNotExist)

Expand Down
75 changes: 63 additions & 12 deletions cmd/firmware-action/filesystem/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package filesystem

import (
"errors"
"fmt"
"log/slog"
"os"
Expand All @@ -12,6 +13,9 @@ import (
"strings"
)

// ErrNotGitRepository is returned when a git command is executed outside of git repository
var ErrNotGitRepository = errors.New("not a git repository")

// gitRun is generic function to execute any git command in sub-directory
func gitRun(subdir string, command []string) (string, error) {
// subdir is relative to current working directory
Expand All @@ -35,36 +39,83 @@ func gitRun(subdir string, command []string) (string, error) {

// Run git describe
cmd := exec.Command(command[0], command[1:]...)
describe, err := cmd.CombinedOutput()
stdout, err := cmd.CombinedOutput()
stdoutStr := string(stdout)
if err != nil {
pattern := regexp.MustCompile(`^fatal: not a git repository.*`)
match := pattern.MatchString(stdoutStr)
if match {
err = ErrNotGitRepository
}

slog.Error(
fmt.Sprintf("Failed to run git command in '%s'", subdir),
slog.String("stdout and stderr", stdoutStr),
slog.Any("error", err),
)
return "", err
return stdoutStr, err
}

return string(describe), nil
return stdoutStr, nil
}

// GitDescribeCoreboot is coreboot-specific git describe command
type describe struct {
abbrev int
dirty bool
always bool
}

// GitDescribeCoreboot is coreboot-specific git describe command (do not touch)
func GitDescribeCoreboot(repoPath string) (string, error) {
describe, err := gitRun(repoPath, []string{"git", "describe", "--abbrev=12", "--dirty", "--always"})
if err != nil {
return "", err
cfg := describe{
abbrev: 12,
dirty: true,
always: true,
}

// Remove trailing newline
result := strings.TrimSpace(describe)

// Check validity of the returned string
hash, err := gitDescribe(repoPath, &cfg)

pattern := regexp.MustCompile(`[\d\w]{13}(\-dirty)?`)
valid := pattern.MatchString(result)
valid := pattern.MatchString(hash)
if !valid {
slog.Warn(
fmt.Sprintf("Output of 'git describe' for '%s' seems to be invalid, output is: '%s'", repoPath, result),
fmt.Sprintf("Output of 'git describe' for '%s' seems to be invalid, output is: '%s'", repoPath, hash),
)
}

return hash, err
}

// GitDescribe is a generic git describe command, with some sane defaults
func GitDescribe(repoPath string) (string, error) {
cfg := describe{
abbrev: 8,
dirty: true,
always: true,
}
return gitDescribe(repoPath, &cfg)
}

func gitDescribe(repoPath string, cfg *describe) (string, error) {
cmd := []string{"git", "describe"}
if cfg.abbrev > 0 {
cmd = append(cmd, fmt.Sprintf("--abbrev=%d", cfg.abbrev))
}
if cfg.dirty {
cmd = append(cmd, "--dirty")
}
if cfg.always {
cmd = append(cmd, "--always")
}

describe, err := gitRun(repoPath, cmd)
if err != nil {
return "", err
}

// Remove trailing newline
result := strings.TrimSpace(describe)

return result, err
}
4 changes: 4 additions & 0 deletions cmd/firmware-action/filesystem/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func TestGitRun(t *testing.T) {
err = os.Chdir(tmpDir)
assert.NoError(t, err)

// Test git status ordinary directory (not git repo)
_, err = gitRun("./", []string{"git", "status"})
assert.ErrorIs(t, err, ErrNotGitRepository)

gitRepoPrepare(t, tmpDir)

// Test git status
Expand Down
6 changes: 6 additions & 0 deletions cmd/firmware-action/recipes/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ func (opts CommonOpts) GetSources() []string {

// ANCHOR_END: CommonOptsGetSources

// GetRepoPath returns Repository path
func (opts CommonOpts) GetRepoPath() string {
return opts.RepoPath
}

// Config is for storing parsed configuration file
type Config struct {
// defined in coreboot.go
Expand Down Expand Up @@ -296,6 +301,7 @@ type FirmwareModule interface {
GetOutputDir() string
GetSources() []string
buildFirmware(ctx context.Context, client *dagger.Client) error
GetRepoPath() string
}

// ======================
Expand Down
Loading
Loading