diff --git a/gitea/prs.go b/gitea/prs.go index 3b127e4..f7fe901 100644 --- a/gitea/prs.go +++ b/gitea/prs.go @@ -13,6 +13,8 @@ type giteaPRService struct { client *gitea.Client } +func (*giteaPRService) SupportsQualifiedPRHeads() bool { return true } + func (f *giteaForge) PullRequests() forge.PullRequestService { return &giteaPRService{client: f.client} } diff --git a/github/prs.go b/github/prs.go index 70751dd..4fb1117 100644 --- a/github/prs.go +++ b/github/prs.go @@ -15,6 +15,8 @@ type gitHubPRService struct { client *github.Client } +func (*gitHubPRService) SupportsQualifiedPRHeads() bool { return true } + func (f *gitHubForge) PullRequests() forge.PullRequestService { return &gitHubPRService{client: f.client} } diff --git a/internal/cli/pr.go b/internal/cli/pr.go index 7873d69..70d4033 100644 --- a/internal/cli/pr.go +++ b/internal/cli/pr.go @@ -277,6 +277,7 @@ func prCreateCmd() *cobra.Command { flagHead string flagBase string flagDraft bool + flagPush bool flagReviewers []string flagAssignees []string flagLabels []string @@ -295,10 +296,31 @@ func prCreateCmd() *cobra.Command { return fmt.Errorf("--head is required") } - forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType) + forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType) if err != nil { return err } + prService := forge.PullRequests() + qualifiedHeadProvider, ok := prService.(forges.QualifiedPRHeadProvider) + supportsQualifiedHeads := ok && qualifiedHeadProvider.SupportsQualifiedPRHeads() + + localHead := flagHead + if flagPush { + headOwner, localBranch, err := splitPRHead(flagHead) + if err != nil { + return err + } + if headOwner != "" && !supportsQualifiedHeads { + return fmt.Errorf("qualified head %q is not supported by this forge; fork pushes require GitHub or Gitea", flagHead) + } + if err := validatePushRemote(cmd.Context(), domain, owner, repoName, headOwner, localBranch, supportsQualifiedHeads); err != nil { + return err + } + if err := git.PushBranch(cmd.Context(), "", resolve.RemoteName(), localBranch); err != nil { + return fmt.Errorf("pushing head branch: %w", err) + } + localHead = localBranch + } opts := forges.CreatePROpts{ Title: flagTitle, @@ -312,13 +334,13 @@ func prCreateCmd() *cobra.Command { Milestone: flagMilestone, } - pr, err := forge.PullRequests().Create(cmd.Context(), owner, repoName, opts) + pr, err := prService.Create(cmd.Context(), owner, repoName, opts) if err != nil { return fmt.Errorf("creating pull request: %w", err) } - if flagHead != "" && pr.Base.Ref != "" { - _ = git.SetBaseBranch(cmd.Context(), "", flagHead, pr.Base.Ref) + if localHead != "" && pr.Base.Ref != "" { + _ = git.SetBaseBranch(cmd.Context(), "", localHead, pr.Base.Ref) } p := printer() @@ -336,6 +358,7 @@ func prCreateCmd() *cobra.Command { cmd.Flags().StringVarP(&flagHead, "head", "H", "", "Head branch") cmd.Flags().StringVarP(&flagBase, "base", "B", "", "Base branch") cmd.Flags().BoolVarP(&flagDraft, "draft", "d", false, "Create as draft") + cmd.Flags().BoolVar(&flagPush, "push", false, "Push the head branch before creating the PR") cmd.Flags().StringSliceVarP(&flagReviewers, "reviewer", "r", nil, "Request a reviewer") cmd.Flags().StringSliceVarP(&flagAssignees, "assignee", "a", nil, "Assign to a user") cmd.Flags().StringSliceVarP(&flagLabels, "label", "l", nil, "Add a label") @@ -345,6 +368,47 @@ func prCreateCmd() *cobra.Command { return cmd } +func splitPRHead(head string) (owner, branch string, err error) { + owner, branch, qualified := strings.Cut(head, ":") + if !qualified { + return "", head, nil + } + if owner == "" || branch == "" { + return "", "", fmt.Errorf("invalid qualified head %q, expected OWNER:BRANCH", head) + } + return owner, branch, nil +} + +func validatePushRemote(ctx context.Context, domain, owner, repo, headOwner, branch string, supportsQualifiedHeads bool) error { + remote := resolve.RemoteName() + pushDomain, pushOwner, pushRepo, err := resolve.PushRemoteRepo(ctx, remote) + if err != nil { + // Local-path remotes cannot be mapped to a forge repository, but Git can + // still push to them. Let the push itself determine whether they work. + return nil + } + + if !strings.EqualFold(pushDomain, domain) { + return fmt.Errorf("push remote %q points to %s/%s/%s, not %s/%s/%s", remote, pushDomain, pushOwner, pushRepo, domain, owner, repo) + } + if headOwner != "" { + if !strings.EqualFold(headOwner, pushOwner) { + return fmt.Errorf("head owner %q does not match push remote %q owner %q", headOwner, remote, pushOwner) + } + return nil + } + if !strings.EqualFold(pushOwner, owner) { + if !supportsQualifiedHeads { + return fmt.Errorf("push remote %q belongs to %q; fork pushes are not supported by this forge", remote, pushOwner) + } + return fmt.Errorf("push remote %q belongs to %q; use --head %s:%s for a fork pull request", remote, pushOwner, pushOwner, branch) + } + if !strings.EqualFold(pushRepo, repo) { + return fmt.Errorf("push remote %q repository %q does not match target repository %q", remote, pushRepo, repo) + } + return nil +} + func prCloseCmd() *cobra.Command { return &cobra.Command{ Use: "close ", diff --git a/internal/cli/pr_checkout_test.go b/internal/cli/pr_checkout_test.go index a41f3d9..6c90862 100644 --- a/internal/cli/pr_checkout_test.go +++ b/internal/cli/pr_checkout_test.go @@ -15,12 +15,19 @@ import ( // mockPRService implements forges.PullRequestService for testing. type mockPRService struct { - pr *forges.PullRequest - err error - listResult []forges.PullRequest - listErr error + pr *forges.PullRequest + err error + listResult []forges.PullRequest + listErr error + createResult *forges.PullRequest + createErr error + createOpts forges.CreatePROpts + createCalls int + qualifiedHeads bool } +func (m *mockPRService) SupportsQualifiedPRHeads() bool { return m.qualifiedHeads } + func (m *mockPRService) Get(_ context.Context, _, _ string, _ int) (*forges.PullRequest, error) { return m.pr, m.err } @@ -29,8 +36,10 @@ func (m *mockPRService) List(_ context.Context, _, _ string, _ forges.ListPROpts return m.listResult, m.listErr } -func (m *mockPRService) Create(_ context.Context, _, _ string, _ forges.CreatePROpts) (*forges.PullRequest, error) { - return nil, nil +func (m *mockPRService) Create(_ context.Context, _, _ string, opts forges.CreatePROpts) (*forges.PullRequest, error) { + m.createCalls++ + m.createOpts = opts + return m.createResult, m.createErr } func (m *mockPRService) Update(_ context.Context, _, _ string, _ int, _ forges.UpdatePROpts) (*forges.PullRequest, error) { diff --git a/internal/cli/pr_test.go b/internal/cli/pr_test.go index 64877ce..9f33c35 100644 --- a/internal/cli/pr_test.go +++ b/internal/cli/pr_test.go @@ -112,6 +112,243 @@ func TestPRCreateRequiresHead(t *testing.T) { } } +func TestPRCreatePushesHeadBranch(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + remoteDir := filepath.Join(t.TempDir(), "remote.git") + t.Chdir(dir) + mustGit(t, "", "init", "--bare", "-q", remoteDir) + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "config", "user.email", "test@test.com") + mustGit(t, dir, "config", "user.name", "Test") + if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test\n"), 0o644); err != nil { + t.Fatal(err) + } + mustGit(t, dir, "add", "README") + mustGit(t, dir, "commit", "-q", "-m", "initial commit") + mustGit(t, dir, "checkout", "-q", "-b", "feature") + mustGit(t, dir, "remote", "add", "origin", remoteDir) + + prs := &mockPRService{ + qualifiedHeads: true, + createResult: &forges.PullRequest{ + Number: 1, + HTMLURL: "https://example.com/pulls/1", + Base: forges.PRBranch{Ref: "main"}, + }, + } + resolve.SetTestForge(&mockForge{prService: prs}, "owner", "repo", "example.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetRemote("origin") + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "forkowner:feature", "--push"}) + if err := cmd.Execute(); err != nil { + t.Fatalf("pr create: %v", err) + } + + if prs.createCalls != 1 { + t.Fatalf("Create calls = %d, want 1", prs.createCalls) + } + if prs.createOpts.Head != "forkowner:feature" { + t.Fatalf("Create head = %q, want %q", prs.createOpts.Head, "forkowner:feature") + } + localSHA := gitOutput(t, dir, "rev-parse", "refs/heads/feature") + remoteSHA := gitOutput(t, "", "--git-dir", remoteDir, "rev-parse", "refs/heads/feature") + if remoteSHA != localSHA { + t.Fatalf("remote branch SHA = %q, want %q", remoteSHA, localSHA) + } + base := gitOutput(t, dir, "config", "--local", "--get", "branch.feature.forge-merge-base") + if base != "main" { + t.Fatalf("cached base branch = %q, want %q", base, "main") + } +} + +func TestPRCreatePushRejectsQualifiedHeadForUnsupportedForge(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + + prs := &mockPRService{} + resolve.SetTestForge(&mockForge{prService: prs}, "upstream", "repo", "gitlab.com") + t.Cleanup(resolve.ResetTestForge) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "forkowner:feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "qualified head") { + t.Fatalf("pr create error = %v, want unsupported qualified-head error", err) + } + if prs.createCalls != 0 { + t.Fatalf("Create calls = %d, want 0", prs.createCalls) + } +} + +func TestPRCreatePushRejectsForkForUnsupportedForge(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "fork", "https://gitlab.com/forkowner/repo.git") + + prs := &mockPRService{} + resolve.SetTestForge(&mockForge{prService: prs}, "upstream", "repo", "gitlab.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetRemote("fork") + t.Cleanup(func() { resolve.SetRemote("origin") }) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "fork pushes are not supported") { + t.Fatalf("pr create error = %v, want unsupported fork-push error", err) + } + if prs.createCalls != 0 { + t.Fatalf("Create calls = %d, want 0", prs.createCalls) + } +} + +func TestPRCreatePushRejectsUnqualifiedForkHead(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "fork", "https://github.com/upstream/repo.git") + mustGit(t, dir, "remote", "set-url", "--push", "fork", "https://github.com/forkowner/repo.git") + + prs := &mockPRService{qualifiedHeads: true} + resolve.SetTestForge(&mockForge{prService: prs}, "upstream", "repo", "github.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetRemote("fork") + t.Cleanup(func() { resolve.SetRemote("origin") }) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "use --head forkowner:feature") { + t.Fatalf("pr create error = %v, want qualified-head guidance", err) + } + if prs.createCalls != 0 { + t.Fatalf("Create calls = %d, want 0", prs.createCalls) + } +} + +func TestValidatePushRemoteAllowsRenamedQualifiedFork(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "fork", "https://github.com/forkowner/renamed-fork.git") + resolve.SetRemote("fork") + t.Cleanup(func() { resolve.SetRemote("origin") }) + + err := validatePushRemote(context.Background(), "github.com", "upstream", "repo", "forkowner", "feature", true) + if err != nil { + t.Fatalf("validatePushRemote: %v", err) + } +} + +func TestPRCreatePushRejectsMismatchedQualifiedOwner(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "fork", "https://github.com/forkowner/repo.git") + + prs := &mockPRService{qualifiedHeads: true} + resolve.SetTestForge(&mockForge{prService: prs}, "upstream", "repo", "github.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetRemote("fork") + t.Cleanup(func() { resolve.SetRemote("origin") }) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "otherowner:feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "head owner \"otherowner\" does not match") { + t.Fatalf("pr create error = %v, want owner-mismatch error", err) + } + if prs.createCalls != 0 { + t.Fatalf("Create calls = %d, want 0", prs.createCalls) + } +} + +func TestSplitPRHead(t *testing.T) { + tests := []struct { + head string + wantOwner string + wantBranch string + wantErr bool + }{ + {head: "feature", wantBranch: "feature"}, + {head: "forkowner:feature", wantOwner: "forkowner", wantBranch: "feature"}, + {head: ":feature", wantErr: true}, + {head: "forkowner:", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.head, func(t *testing.T) { + owner, branch, err := splitPRHead(tt.head) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("splitPRHead: %v", err) + } + if owner != tt.wantOwner || branch != tt.wantBranch { + t.Fatalf("splitPRHead = %q, %q, want %q, %q", owner, branch, tt.wantOwner, tt.wantBranch) + } + }) + } +} + +func TestPRCreatePushFailurePreventsCreate(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, dir, "init", "-q") + + prs := &mockPRService{} + resolve.SetTestForge(&mockForge{prService: prs}, "owner", "repo", "example.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetRemote("missing") + t.Cleanup(func() { resolve.SetRemote("origin") }) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "pushing head branch") { + t.Fatalf("pr create error = %v, want push error", err) + } + if prs.createCalls != 0 { + t.Fatalf("Create calls = %d, want 0", prs.createCalls) + } +} + func TestPRMergeInvalidNumber(t *testing.T) { var buf bytes.Buffer rootCmd.SetOut(&buf) @@ -347,6 +584,21 @@ func mustGit(t *testing.T, dir string, args ...string) { } } +func gitOutput(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), + "GIT_CONFIG_GLOBAL=/dev/null", + "GIT_CONFIG_SYSTEM=/dev/null", + ) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + func TestPRViewJSONFlagNotSupported(t *testing.T) { tests := []struct { name string diff --git a/internal/git/git.go b/internal/git/git.go index 5c47bcf..3bac187 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -131,6 +131,23 @@ func CurrentBranch(ctx context.Context, dir string) (string, error) { return branch, nil } +// PushBranch pushes a local branch to the same branch name on a remote and +// configures the local branch to track it. +func PushBranch(ctx context.Context, dir, remote, branch string) error { + if remote == "" { + return fmt.Errorf("empty remote name") + } + if branch == "" { + return fmt.Errorf("empty branch name") + } + + ref := "refs/heads/" + branch + if _, err := runGit(ctx, dir, "push", "--set-upstream", remote, ref+":"+ref); err != nil { + return fmt.Errorf("failed to push branch %q to remote %q: %w", branch, remote, err) + } + return nil +} + func branchConfigKey(branch, name string) string { return fmt.Sprintf("branch.%s.%s", branch, name) } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 361ea3b..a465ae0 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -2,7 +2,9 @@ package git import ( "context" + "os" "os/exec" + "path/filepath" "testing" "github.com/git-pkgs/forge" @@ -173,6 +175,66 @@ func TestCurrentBranch(t *testing.T) { } } +func TestPushBranch(t *testing.T) { + dir := initGitRepo(t) + remoteDir := filepath.Join(t.TempDir(), "remote.git") + mustGit(t, "", "init", "--bare", "-q", remoteDir) + mustGit(t, dir, "config", "user.name", "Test") + mustGit(t, dir, "config", "user.email", "test@example.com") + if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test\n"), 0o644); err != nil { + t.Fatal(err) + } + mustGit(t, dir, "add", "README") + mustGit(t, dir, "commit", "-q", "-m", "initial commit") + mustGit(t, dir, "checkout", "-q", "-b", "feature") + mustGit(t, dir, "remote", "add", "origin", remoteDir) + + if err := PushBranch(context.Background(), dir, "origin", "feature"); err != nil { + t.Fatalf("PushBranch: %v", err) + } + + localSHA, err := runGit(context.Background(), dir, "rev-parse", "refs/heads/feature") + if err != nil { + t.Fatalf("reading local branch: %v", err) + } + remoteSHA, err := runGit(context.Background(), "", "--git-dir", remoteDir, "rev-parse", "refs/heads/feature") + if err != nil { + t.Fatalf("reading remote branch: %v", err) + } + if remoteSHA != localSHA { + t.Fatalf("remote branch SHA = %q, want %q", remoteSHA, localSHA) + } + + upstream, err := runGit(context.Background(), dir, "rev-parse", "--abbrev-ref", "feature@{upstream}") + if err != nil { + t.Fatalf("reading branch upstream: %v", err) + } + if upstream != "origin/feature" { + t.Fatalf("branch upstream = %q, want %q", upstream, "origin/feature") + } +} + +func TestPushBranchErrors(t *testing.T) { + tests := []struct { + name string + remote string + branch string + want string + }{ + {name: "empty remote", branch: "feature", want: "empty remote name"}, + {name: "empty branch", remote: "origin", want: "empty branch name"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := PushBranch(context.Background(), "", tt.remote, tt.branch) + if err == nil || err.Error() != tt.want { + t.Fatalf("PushBranch error = %v, want %q", err, tt.want) + } + }) + } +} + func TestSetAndGetPRNumber(t *testing.T) { dir := initGitRepo(t) ctx := context.Background() diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 3c4fea1..a96b5c4 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -7,6 +7,7 @@ import ( "net/url" "os" "os/exec" + "path/filepath" "strings" "github.com/git-pkgs/forge" @@ -246,6 +247,27 @@ func gitRemoteURL(name string) (string, error) { return strings.TrimSpace(string(out)), nil } +// PushRemoteRepo returns the forge domain, owner, and repository addressed by +// a remote's push URL. Git falls back to the fetch URL when no push URL is set. +func PushRemoteRepo(ctx context.Context, name string) (domain, owner, repo string, err error) { + out, err := exec.CommandContext(ctx, "git", "remote", "get-url", "--push", name).Output() + if err != nil { + return "", "", "", err + } + + rawURL := strings.TrimSpace(string(out)) + parsed, parseErr := url.Parse(rawURL) + if filepath.IsAbs(rawURL) || (parseErr == nil && parsed.Scheme == "file") { + return "", "", "", fmt.Errorf("push remote %q uses a local path", name) + } + + domain, owner, repo, err = forges.ParseRepoURL(rawURL) + if err != nil { + return "", "", "", err + } + return mapSSHHost(domain), owner, repo, nil +} + // OwnerForBranch returns the repository owner for the remote that the given // branch tracks. This is useful for determining which fork a branch was pushed // to when creating pull requests. diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index a9f2bd0..4e8911a 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -679,6 +679,41 @@ func TestOwnerForBranch(t *testing.T) { } } +func TestPushRemoteRepo(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, "init", "-q") + mustGit(t, "remote", "add", "origin", "https://github.com/mainowner/project.git") + mustGit(t, "remote", "set-url", "--push", "origin", "git@github.com:forkowner/project.git") + + domain, owner, repo, err := PushRemoteRepo(context.Background(), "origin") + if err != nil { + t.Fatalf("PushRemoteRepo: %v", err) + } + if domain != "github.com" || owner != "forkowner" || repo != "project" { + t.Fatalf("PushRemoteRepo = %q, %q, %q, want %q, %q, %q", domain, owner, repo, "github.com", "forkowner", "project") + } +} + +func TestPushRemoteRepoRejectsLocalPath(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + dir := t.TempDir() + t.Chdir(dir) + mustGit(t, "init", "-q") + mustGit(t, "remote", "add", "origin", filepath.Join(t.TempDir(), "remote.git")) + + if _, _, _, err := PushRemoteRepo(context.Background(), "origin"); err == nil { + t.Fatal("expected local-path remote to be rejected") + } +} + func mustGit(t *testing.T, args ...string) { t.Helper() cmd := exec.Command("git", args...) diff --git a/services.go b/services.go index 412bfb1..3006545 100644 --- a/services.go +++ b/services.go @@ -43,6 +43,12 @@ type PullRequestService interface { ListURL(repoHTMLURL string) string } +// QualifiedPRHeadProvider is implemented by pull request services that accept +// OWNER:BRANCH heads to identify a source repository in the same fork network. +type QualifiedPRHeadProvider interface { + SupportsQualifiedPRHeads() bool +} + // LabelService provides operations on repository labels. type LabelService interface { List(ctx context.Context, owner, repo string, opts ListLabelOpts) ([]Label, error)