Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/browseros/tools/patch/cmd/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ func printAnnotateResult(ws workspace.Entry, result *engine.AnnotateResult) {
fmt.Printf(" %-24s %s\n", skipped.Name, skipped.Reason)
}
}
if len(result.Unclaimed) > 0 {
fmt.Println(ui.Warning("Unclaimed changes (no feature owns these; left uncommitted):"))
for _, rel := range result.Unclaimed {
fmt.Printf(" %s\n", rel)
}
fmt.Println(ui.Hint(`Claim them in bos_build/features.yaml, then re-run; "browseros-patch feature lint" checks coverage.`))
}
if result.CommitsCreated == 0 {
fmt.Println(ui.Hint("No commits created."))
}
Expand Down
15 changes: 13 additions & 2 deletions packages/browseros/tools/patch/cmd/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ func init() {
var reset bool
var changed string
var rangeEnd string
var noAnnotate bool
command := &cobra.Command{
Use: "apply [checkout] [-- files...]",
Annotations: map[string]string{"group": "Core:"},
Short: "Apply repo patches to a checkout",
Short: "Apply repo patches to a checkout and create feature commits",
Example: ` browseros-patch apply ch1
browseros-patch apply ch1 --no-annotate
browseros-patch apply ch1 -- chrome/browser/browser.cc
browseros-patch apply --src /path/to/chromium/src`,
Args: cobra.ArbitraryArgs,
Expand All @@ -41,7 +43,11 @@ func init() {
ChangedRef: changed,
RangeEnd: rangeEnd,
Filters: filters,
Progress: commandProgress(cmd),
// Filtered and --changed applies are surgical; a full
// annotate would sweep changes outside the requested scope
// into feature commits.
AutoAnnotate: !noAnnotate && len(filters) == 0 && changed == "",
Progress: commandProgress(cmd),
})
if err != nil {
return err
Expand All @@ -58,15 +64,20 @@ func init() {
}
fmt.Println(ui.Hint(`Run "browseros-patch continue" after fixing the current conflict.`))
}
printAnnotateOutcome(ws, result)
}); err != nil {
return err
}
if err := annotateFailureExit(result); err != nil {
return err
}
return conflictPauseError(len(result.Conflicts) > 0)
},
}
command.Flags().StringVar(&src, "src", "", srcFlagUsage)
command.Flags().BoolVar(&reset, "reset", false, "Reset patched files to BASE_COMMIT before applying")
command.Flags().StringVar(&changed, "changed", "", "Apply only patches changed in the given repo commit")
command.Flags().StringVar(&rangeEnd, "range-end", "", "End revision when using --changed as a range start")
command.Flags().BoolVar(&noAnnotate, "no-annotate", false, "Skip feature-commit annotation after apply (also skipped when -- files filters are given)")
rootCmd.AddCommand(command)
}
34 changes: 33 additions & 1 deletion packages/browseros/tools/patch/cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ Capture flow (turn checkout changes into patches):
3. browseros-patch publish -m "chore: sync patches" # commit + push chromium_patches

Feature commit flow (turn checkout changes into feature commits):
1. browseros-patch annotate ch1 # commit changed files for all bos_build/features.yaml features
1. browseros-patch apply ch1 # auto-annotates after a conflict-free apply (--no-annotate to skip)
2. browseros-patch annotate ch1 # standalone: commit changed files for all bos_build/features.yaml features
Annotation rules:
- Filtered (-- files...) and --changed applies never auto-annotate; continue/skip finish a paused apply's annotation, excluding skipped conflicts.
- apply, sync, and annotate refuse to start while a conflict resolution is pending; finish continue/skip/abort first.
- Changes no feature claims are reported as "unclaimed" and left uncommitted; claim them in bos_build/features.yaml.

Pool refresh flow (rebuild browseros branch before leasing a checkout):
1. browseros-patch status ch1 --json # check patches_freshness
Expand Down Expand Up @@ -130,6 +135,33 @@ func ensureRepoConfigured(override string) error {
return nil
}

// printAnnotateOutcome renders the auto-annotate section of an apply-family
// result: the feature-commit summary on success, a recovery hint on failure.
func printAnnotateOutcome(ws workspace.Entry, result *engine.ApplyResult) {
if result.Annotate != nil {
fmt.Println()
printAnnotateResult(ws, result.Annotate)
}
if result.AnnotateSkipped != "" {
fmt.Println(ui.Hint(fmt.Sprintf("Annotation skipped: %s.", result.AnnotateSkipped)))
}
if result.AnnotateError != "" {
fmt.Println(ui.Warning("Patches applied, but annotate failed"))
fmt.Printf(" %s\n", result.AnnotateError)
fmt.Println(ui.Hint(fmt.Sprintf(`Fix the cause, then run "browseros-patch annotate %s".`, ws.Name)))
}
}

// annotateFailureExit maps an auto-annotate failure to exit 1 after rendering,
// so scripts notice the missing feature commits while the apply outcome stays
// visible in the output.
func annotateFailureExit(result *engine.ApplyResult) error {
if result.AnnotateError != "" {
return &exitCodeError{code: 1}
}
return nil
}

// printStashOutcome reports what happened to a pending sync stash once the
// conflict loop completes.
func printStashOutcome(result *engine.ApplyResult) {
Expand Down
4 changes: 4 additions & 0 deletions packages/browseros/tools/patch/cmd/continue.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ func init() {
fmt.Printf(" %s\n", conflict.ChromiumPath)
}
}
printAnnotateOutcome(ws, result)
printStashOutcome(result)
}); err != nil {
return err
}
if err := annotateFailureExit(result); err != nil {
return err
}
return conflictPauseError(len(result.Conflicts) > 0 || result.StashConflict)
},
}
Expand Down
4 changes: 4 additions & 0 deletions packages/browseros/tools/patch/cmd/skip.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,14 @@ func init() {
fmt.Printf(" %s\n", conflict.ChromiumPath)
}
}
printAnnotateOutcome(ws, result)
printStashOutcome(result)
}); err != nil {
return err
}
if err := annotateFailureExit(result); err != nil {
return err
}
return conflictPauseError(len(result.Conflicts) > 0 || result.StashConflict)
},
}
Expand Down
99 changes: 78 additions & 21 deletions packages/browseros/tools/patch/internal/engine/annotate.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import (
type AnnotateOptions struct {
Workspace workspace.Entry
Repo *repo.Info
Progress Progress
// Exclude lists checkout paths deliberately left uncommitted — skipped
// conflicts whose files may hold partially applied hunks.
Exclude []string
Progress Progress
}

type AnnotateResult struct {
Expand All @@ -26,6 +29,9 @@ type AnnotateResult struct {
FeaturesSkipped int `json:"features_skipped"`
Committed []AnnotateCommittedFeature `json:"committed"`
Skipped []AnnotateSkippedFeature `json:"skipped"`
// Unclaimed lists changed files no feature owns; they stay uncommitted
// so the checkout remains dirty until features.yaml claims them.
Unclaimed []string `json:"unclaimed,omitempty"`
}

type AnnotateCommittedFeature struct {
Expand All @@ -48,17 +54,34 @@ type annotateFileSet struct {
}

// Annotate creates Chromium checkout commits grouped by the repo feature registry.
// It refuses to run mid-conflict-resolution: committing a half-applied tree
// would bake reject files and partial patches into feature history.
func Annotate(ctx context.Context, opts AnnotateOptions) (*AnnotateResult, error) {
if err := requireNoPendingResolution(ctx, opts.Workspace); err != nil {
return nil, err
}
features, featuresFile, err := LoadFeatures(opts.Repo)
if err != nil {
return nil, err
}
ignore, err := patch.LoadIgnoreSet(opts.Repo.Root, nil)
if err != nil {
return nil, err
}
result := &AnnotateResult{
Workspace: opts.Workspace.Name,
FeaturesFile: featuresFile,
Committed: []AnnotateCommittedFeature{},
Skipped: []AnnotateSkippedFeature{},
}
// One status snapshot serves the whole run; a full scan is seconds on a
// Chromium checkout. Each feature consumes the entries it matched —
// commitFeatureFiles settles them (committed, or staged clean) — so what
// remains at the end is exactly the unclaimed leftovers.
changes, err := annotateChanges(ctx, opts.Workspace.Path, ignore, opts.Exclude)
if err != nil {
return nil, err
}
for _, feature := range features {
result.Processed++
reportProgress(opts.Progress, "Annotating feature %s", feature.Name)
Expand All @@ -71,11 +94,8 @@ func Annotate(ctx context.Context, opts AnnotateOptions) (*AnnotateResult, error
result.FeaturesSkipped++
continue
}
changes, err := annotateChanges(ctx, opts.Workspace.Path)
if err != nil {
return nil, err
}
files := modifiedFeatureFiles(changes, feature)
matched, rest := partitionFeatureChanges(changes, feature)
files := annotateFileSetFrom(matched)
if len(files.report) == 0 {
result.Skipped = append(result.Skipped, AnnotateSkippedFeature{
Name: feature.Name,
Expand All @@ -89,6 +109,7 @@ func Annotate(ctx context.Context, opts AnnotateOptions) (*AnnotateResult, error
if err != nil {
return nil, fmt.Errorf("commit feature %s: %w", feature.Name, err)
}
changes = rest
if !committed {
result.Skipped = append(result.Skipped, AnnotateSkippedFeature{
Name: feature.Name,
Expand All @@ -106,6 +127,7 @@ func Annotate(ctx context.Context, opts AnnotateOptions) (*AnnotateResult, error
})
result.CommitsCreated++
}
result.Unclaimed = uniqueReportPaths(changes)
return result, nil
}

Expand Down Expand Up @@ -153,19 +175,62 @@ func stringSequence(node *yaml.Node, key string) []string {
return items
}

func annotateChanges(ctx context.Context, workspacePath string) ([]git.FileChange, error) {
return git.StatusPorcelain(ctx, workspacePath, nil)
// annotateChanges returns working-tree changes eligible for feature commits.
// Untracked junk (reject files, logs, .browseros-patchignore patterns) is
// filtered like extract does; tracked modifications always pass through
// unless explicitly excluded.
func annotateChanges(ctx context.Context, workspacePath string, ignore *patch.IgnoreSet, exclude []string) ([]git.FileChange, error) {
changes, err := git.StatusPorcelain(ctx, workspacePath, nil)
if err != nil {
return nil, err
}
kept := changes[:0]
for _, change := range changes {
if change.Status == "??" && ignore.Match(change.Path) {
continue
}
if len(exclude) > 0 && patch.PathMatches(patch.NormalizeChromiumPath(change.Path), exclude) {
continue
}
kept = append(kept, change)
}
return kept, nil
}

func modifiedFeatureFiles(changes []git.FileChange, feature FeatureSpec) annotateFileSet {
// uniqueReportPaths flattens changes into sorted, deduplicated checkout paths.
func uniqueReportPaths(changes []git.FileChange) []string {
seen := map[string]bool{}
var paths []string
for _, change := range changes {
for _, rel := range changeReportPaths(change) {
appendUniquePath(&paths, seen, rel)
}
}
slices.Sort(paths)
return paths
}

// partitionFeatureChanges splits the snapshot into the entries a feature
// claims and the rest, so the caller can settle the claimed ones and carry
// the remainder to the next feature without re-scanning git status.
func partitionFeatureChanges(changes []git.FileChange, feature FeatureSpec) ([]git.FileChange, []git.FileChange) {
var matched, rest []git.FileChange
for _, change := range changes {
if featureMatchesChange(feature, change) {
matched = append(matched, change)
continue
}
rest = append(rest, change)
}
return matched, rest
}

func annotateFileSetFrom(changes []git.FileChange) annotateFileSet {
set := annotateFileSet{}
reportSeen := map[string]bool{}
stageSeen := map[string]bool{}
commitSeen := map[string]bool{}
for _, change := range changes {
if !featureMatchesChange(feature, change) {
continue
}
for _, rel := range changeReportPaths(change) {
appendUniquePath(&set.report, reportSeen, rel)
}
Expand Down Expand Up @@ -266,13 +331,5 @@ func committedFeatureFiles(ctx context.Context, workspacePath string, commit str
if err != nil {
return nil, err
}
seen := map[string]bool{}
var paths []string
for _, change := range changes {
for _, rel := range changeReportPaths(change) {
appendUniquePath(&paths, seen, rel)
}
}
slices.Sort(paths)
return paths, nil
return uniqueReportPaths(changes), nil
}
Loading
Loading