Self-review: jj-compat against latest master

.github/workflows/ci.yml

@@ -2,6 +2,11 @@ name: golang build
 
 on: [push, pull_request, workflow_dispatch]
 
+env:
+  # Pin the jj version used by integration tests. Bump intentionally;
+  # jj's revset/template syntax can change between minor versions.
+  JJ_VERSION: 0.40.0
+
 jobs:
 
   build:
@@ -15,22 +20,54 @@ jobs:
       with:
         go-version: 1.21
 
+    - name: Install jj
+      run: |
+        set -euo pipefail
+        mkdir -p "$HOME/.local/bin"
+        curl -fsSL \
+          "https://github.com/jj-vcs/jj/releases/download/v${JJ_VERSION}/jj-v${JJ_VERSION}-x86_64-unknown-linux-musl.tar.gz" \
+          | tar -xz -C "$HOME/.local/bin" jj
+        echo "$HOME/.local/bin" >> "$GITHUB_PATH"
+        "$HOME/.local/bin/jj" --version
+
     - name: Build
       working-directory: ./
       run: go build -v ./...
 
-    - name: Test
+    - name: Test (unit)
       working-directory: ./
-      run: go test -race -coverprofile=./coverage.txt -covermode=atomic ./...
+      run: go test -race -coverpkg=./... -coverprofile=cov-unit.out -covermode=atomic ./...
+
+    - name: Test (integration)
+      working-directory: ./
+      run: |
+        # Configure a default git identity so jj's colocated git backend
+        # has something to use (the per-repo overrides in jjtest also set
+        # this, but a global default avoids any first-call edge cases).
+        git config --global user.name "spr-ci"
+        git config --global user.email "spr-ci@example.com"
+        go test -race -tags=integration -coverpkg=./... -coverprofile=cov-integration.out -covermode=atomic ./...
+
+    - name: Merge coverage and report per-file attribution
+      working-directory: ./
+      run: go run ./scripts/coverage-merge cov-unit.out cov-integration.out cov-merged.out
+
+    - name: Upload coverage artifacts
+      uses: actions/upload-artifact@v4
+      with:
+        name: coverage
+        path: |
+          cov-unit.out
+          cov-integration.out
+          cov-merged.out
 
     - name: Check goreleaser file
       uses: goreleaser/goreleaser-action@v6.2.1
       with:
         version: latest
         args: check
 
-    #- name: Upload coverage
+    #- name: Upload coverage to codecov
     #  uses: codecov/codecov-action@v2
     #  with:
-    #    files: ./coverage.txt
-
+    #    files: ./cov-merged.out

Makefile

@@ -1,4 +1,30 @@
+.PHONY: bin test test-unit test-integration test-all coverage-html
 
 bin:
 	goreleaser build --snapshot --clean --single-target
 
+# Default test target: unit tests only.
+test: test-unit
+
+# Unit tests — no integration build tag, no jj binary required.
+# -coverpkg=./... so coverage of cross-package calls is counted (e.g.
+# spr_test.go calls into vcs.JjOps, which we want reflected in vcs/jj_ops.go
+# coverage).
+test-unit:
+	go test -race -coverpkg=./... -coverprofile=cov-unit.out -covermode=atomic ./...
+
+# Integration tests — runs against a real `jj` binary. Set
+# SPR_SKIP_JJ_INTEGRATION=1 to skip cleanly when jj is unavailable.
+test-integration:
+	go test -race -tags=integration -coverpkg=./... -coverprofile=cov-integration.out -covermode=atomic ./...
+
+# Both suites, then merge profiles and print per-file attribution.
+test-all: test-unit test-integration
+	@go run ./scripts/coverage-merge cov-unit.out cov-integration.out cov-merged.out
+
+# Generate HTML coverage reports for both profiles plus the merged view.
+coverage-html: test-all
+	go tool cover -html=cov-unit.out -o cov-unit.html
+	go tool cover -html=cov-integration.out -o cov-integration.html
+	go tool cover -html=cov-merged.out -o cov-merged.html
+	@echo "Open cov-unit.html, cov-integration.html, cov-merged.html"

cmd/amend/main.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ejoffe/spr/git/realgit"
 	"github.com/ejoffe/spr/github/githubclient"
 	"github.com/ejoffe/spr/spr"
+	"github.com/ejoffe/spr/vcs"
 	"github.com/jessevdk/go-flags"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
@@ -62,7 +63,8 @@ func main() {
 	client := githubclient.NewGitHubClient(ctx, cfg)
 	gitcmd = realgit.NewGitCmd(cfg)
 
-	sd := spr.NewStackedPR(cfg, client, gitcmd)
+	vcsOps := vcs.NewVCSOperations(cfg, gitcmd)
+	sd := spr.NewStackedPR(cfg, client, gitcmd, vcsOps)
 	sd.AmendCommit(ctx)
 
 	if opts.Update {

cmd/spr/main.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"os/exec"
+	"strconv"
 	"strings"
 
 	"github.com/ejoffe/rake"
@@ -13,6 +15,7 @@ import (
 	"github.com/ejoffe/spr/git/realgit"
 	"github.com/ejoffe/spr/github/githubclient"
 	"github.com/ejoffe/spr/spr"
+	"github.com/ejoffe/spr/vcs"
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 
@@ -95,9 +98,33 @@ func main() {
 	}
 	gitcmd = realgit.NewGitCmd(cfg)
 
+	// Check for --no-jj flag or SPR_NOJJ env var before creating VCS operations.
+	// This must happen before app.Run() since vcsOps is created here, so
+	// urfave/cli's later flag/env parsing won't be in time to affect jj
+	// detection. Accept all standard boolean forms: bare `--no-jj`,
+	// `--no-jj=true`, and any SPR_NOJJ value strconv.ParseBool understands
+	// (`1`, `t`, `T`, `TRUE`, `true`, ...).
+	for _, arg := range os.Args[1:] {
+		if arg == "--no-jj" {
+			cfg.User.NoJJ = true
+			continue
+		}
+		if v, ok := strings.CutPrefix(arg, "--no-jj="); ok {
+			if b, err := strconv.ParseBool(v); err == nil {
+				cfg.User.NoJJ = b
+			}
+		}
+	}
+	if v := os.Getenv("SPR_NOJJ"); v != "" {
+		if b, err := strconv.ParseBool(v); err == nil && b {
+			cfg.User.NoJJ = true
+		}
+	}
+
 	ctx := context.Background()
 	client := githubclient.NewGitHubClient(ctx, cfg)
-	stackedpr := spr.NewStackedPR(cfg, client, gitcmd)
+	vcsOps := vcs.NewVCSOperations(cfg, gitcmd)
+	stackedpr := spr.NewStackedPR(cfg, client, gitcmd, vcsOps)
 
 	detailFlag := &cli.BoolFlag{
 		Name:  "detail",
@@ -160,6 +187,12 @@ VERSION: fork of {{.Version}}
 				Value: false,
 				Usage: "Show runtime debug info",
 			},
+			&cli.BoolFlag{
+				Name:    "no-jj",
+				Value:   false,
+				Usage:   "Disable jj (Jujutsu) mode even in jj-colocated repos",
+				EnvVars: []string{"SPR_NOJJ"},
+			},
 		},
 		Before: func(c *cli.Context) error {
 			if c.IsSet("debug") {
@@ -346,6 +379,25 @@ VERSION: fork of {{.Version}}
 				return nil
 			},
 		},
+			{
+				Name:  "jj-setup",
+				Usage: "Register 'jj spr' alias for use in Jujutsu repos",
+				Action: func(c *cli.Context) error {
+					cmd := exec.Command("jj", "config", "set", "--user",
+						"aliases.spr", `["util", "exec", "--", "git-spr"]`)
+					cmd.Stdout = os.Stdout
+					cmd.Stderr = os.Stderr
+					err := cmd.Run()
+					if err != nil {
+						return cli.Exit(fmt.Sprintf("Failed to set jj alias: %s", err), 1)
+					}
+					fmt.Println("jj alias registered. You can now use:")
+					fmt.Println("  jj spr update")
+					fmt.Println("  jj spr status")
+					fmt.Println("  jj spr merge")
+					return nil
+				},
+			},
 			{
 				Name:  "version",
 				Usage: "Show version info",

config/config.go

@@ -41,6 +41,7 @@ type RepoConfig struct {
 	ForceFetchTags bool `default:"false" yaml:"forceFetchTags"`
 
 	ShowPrTitlesInStack    bool `default:"false" yaml:"showPrTitlesInStack"`
+	ShowStackNumberInTitle bool `default:"false" yaml:"showStackNumberInTitle"`
 	BranchPushIndividually bool `default:"false" yaml:"branchPushIndividually"`
 }
 
@@ -55,6 +56,16 @@ type UserConfig struct {
 	PreserveTitleAndBody bool `default:"false" yaml:"preserveTitleAndBody"`
 	NoRebase             bool `default:"false" yaml:"noRebase"`
 	NoFetch              bool `default:"false" yaml:"noFetch"`
+	NoJJ                 bool `default:"false" yaml:"noJJ"`
+	// NoPruneOrphans disables the jj-mode cascade-orphan recovery in
+	// spr update. When false (default), spr identifies local commits
+	// whose PR has been closed-without-merge on GitHub (typically by
+	// an upstream squash that absorbed their content) and `jj abandon`s
+	// them before rebase, avoiding the duplicate-content conflict
+	// cascade described in docs/jj-mode-design.md. Set true to skip
+	// this entirely if you don't trust the GitHub-side signal or want
+	// to investigate orphan commits manually before they're dropped.
+	NoPruneOrphans       bool `default:"false" yaml:"noPruneOrphans"`
 	DeleteMergedBranches bool `default:"false" yaml:"deleteMergedBranches"`
 	ShortPRLink          bool `default:"false" yaml:"shortPRLink"`
 	ShowCommitID         bool `default:"false" yaml:"showCommitID"`