feat(mr): respect --push=false with --fill

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/cli:jmrp-glab-mr-fill-respects-push-false into main

Description

glab mr create --fill always pushes the source branch, and an explicit --push=false is ignored: the autofill branch of run() sets ShouldPush = true after the flags are parsed, so the value given on the command line never reaches the push. That leaves no way to use --fill when the push itself fails, which is the case in the issue: a Jujutsu bookmark pushed by an earlier step, where pushing HEAD again conflicts with the remote and aborts the command.

This MR follows the plan in the issue, in four commits:

  1. test(mr): pin that mr create --fill pushes the source branch adds TestNewCmdCreate_FillPush. It drives the command through cmdtest.SetupCmdForTest with --fill and with --fill --push, and asserts on the git commands the command stubber records: one git push origin HEAD:feat-new-mr each. Nothing ran --fill through to the push before. Both cases pass on main.
  2. feat(mr): respect --push=false with --fill is the fix. complete() records whether --push was given, with cmd.Flags().Changed("push"), after the handling of --allow-collaboration, --remove-source-branch and --squash-before-merge, and the autofill branch turns the push on only when it was not. Bare --fill and --fill --push push as before, and --fill --push=false creates the merge request without pushing. The commit adds that third case to the test, which fails on the first commit and passes on this one.
  3. docs(mr): describe --push=false with --fill in mr create help. The --fill description wrapped push in backticks, which pflag reads as the flag's value name, so the help and docs/source/mr/create.md showed -f, --fill push. It now says the branch is pushed unless --push=false is set. The --push description says it defaults to true with --fill and that --push=false skips the push, and there is an example for --fill --push=false.
  4. docs(mr): say that mr create --push pushes before creating. The --push description said the push happens after the merge request is created, while run() pushes first and creates the merge request afterwards.

Commits 3 and 4 carry docs/source/mr/create.md exactly as make gen-docs writes it.

Step 5 of the plan, the error when the branch is not on the remote: GitLab refuses the create with 400 {"message":{"source_branch":["does not exist"]}} (the spec "returns 400 when source_branch does not exist in the repository" in spec/requests/api/merge_requests_spec.rb pins that answer), and against the stub described below, glab prints:

Post http://127.0.0.1:8099/api/v4/projects/OWNER%2FREPO/merge_requests: 400 {message: {source_branch: [does not exist]}}.

It names the field and what is wrong with it, and it is the error glab mr create already gives today without --fill for a branch that was never pushed, so I left it as it is. If you would rather have a hint that points at --push there, I can add it to this MR.

Related Issues

Closes #8224 (closed)

How has this been tested?

With Go 1.27.2 and golangci-lint 2.14.0, on each of the four commits:

  • TestNewCmdCreate_FillPush passes. On the first commit, the --push=false case added by the second fails with expected: [][]string(nil), actual: [][]string{[]string{"git", "push", "origin", "HEAD:feat-new-mr"}}, and the other two pass.
  • go test passes on the packages make test-changed selects for this change, the changed package and every package that imports it: cmd/gen-docs, cmd/glab, internal/commands, internal/commands/mr, internal/commands/mr/create, internal/commands/stack and internal/commands/stack/sync.
  • golangci-lint run (make lint): 0 issues. golangci-lint fmt --diff, go vet ./internal/commands/mr/..., make check-embed check-args and go run ./scripts/comment-overlap -threshold 1.0 -max-findings 0 ./cmd/... ./internal/... are clean.
  • go run ./cmd/gen-docs/docs.go, which is what make gen-docs runs, leaves docs/ and README.md unchanged.
  • On commits 3 and 4, in the docs lint image CI uses, vale --minAlertLevel error, markdownlint-cli2 and lychee --offline --include-fragments report nothing on docs/source/mr/create.md.
  • Every commit message and the title pass commitlint with scripts/commit-lint/.commitlintrc.js.

End to end, I built glab from main and from this branch and ran both in a scratch repository whose origin is a small stub of the GitLab API. The stub logs every request, git's smart HTTP requests included, so a push attempt shows up as GET /OWNER/REPO.git/info/refs?service=git-receive-pack. feat-on-remote is a branch the stub reports as present, feat-local-only one it does not.

Runs and the stub's request log 
### main on feat-on-remote: glab mr create --fill --push=false --yes
fatal: repository 'http://127.0.0.1:8099/OWNER/REPO.git/' not found
Failed to create merge request. Created recovery file: .../recover/OWNER/REPO/mr.json
Run the command again with the '--recover' option to retry.

   ERROR

  Exit status 128.

### this branch on feat-on-remote: glab mr create --fill --push=false --yes

Creating merge request for feat-on-remote into main in OWNER/REPO

http://127.0.0.1:8099/OWNER/REPO/-/merge_requests/1

### this branch on feat-local-only: glab mr create --fill --push=false --yes

Creating merge request for feat-local-only into main in OWNER/REPO

Failed to create merge request. Created recovery file: .../recover/OWNER/REPO/mr.json
Run the command again with the '--recover' option to retry.

   ERROR

  Post http://127.0.0.1:8099/api/v4/projects/OWNER%2FREPO/merge_requests: 400 {message: {source_branch: [does not
  exist]}}.

### this branch on feat-local-only: glab mr create --fill --yes
fatal: repository 'http://127.0.0.1:8099/OWNER/REPO.git/' not found
Failed to create merge request. Created recovery file: .../recover/OWNER/REPO/mr.json
Run the command again with the '--recover' option to retry.

   ERROR

  Exit status 128.

### main on feat-local-only, without --fill: glab mr create --title Title --description Body --yes

Creating merge request for feat-local-only into main in OWNER/REPO

Failed to create merge request. Created recovery file: .../recover/OWNER/REPO/mr.json
Run the command again with the '--recover' option to retry.

   ERROR

  Post http://127.0.0.1:8099/api/v4/projects/OWNER%2FREPO/merge_requests: 400 {message: {source_branch: [does not
  exist]}}.
---- main on feat-on-remote: glab mr create --fill --push=false --yes
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
POST /api/graphql
GET /api/v4/projects/OWNER%2FREPO/repository/commits/main
GET /OWNER/REPO.git/info/refs?service=git-receive-pack
---- this branch on feat-on-remote: glab mr create --fill --push=false --yes
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
POST /api/graphql
GET /api/v4/projects/OWNER%2FREPO/repository/commits/main
POST /api/v4/projects/OWNER%2FREPO/merge_requests
  source_branch="feat-on-remote" target_branch="main" title="feat: add the feature"
---- this branch on feat-local-only: glab mr create --fill --push=false --yes
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
POST /api/graphql
GET /api/v4/projects/OWNER%2FREPO/repository/commits/main
POST /api/v4/projects/OWNER%2FREPO/merge_requests
  source_branch="feat-local-only" target_branch="main" title="fix: a change that was never pushed"
---- this branch on feat-local-only: glab mr create --fill --yes
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
POST /api/graphql
GET /api/v4/projects/OWNER%2FREPO/repository/commits/main
GET /OWNER/REPO.git/info/refs?service=git-receive-pack
---- main on feat-local-only, without --fill: glab mr create --title Title --description Body --yes
GET /api/v4/projects/OWNER%2FREPO?license=true&with_custom_attributes=true
POST /api/graphql
POST /api/v4/projects/OWNER%2FREPO/merge_requests
  source_branch="feat-local-only" target_branch="main" title="Title"

Merge request reports