ejoffe / spr Goto Github PK

It's common to configuration GH authentication with a PAT that you place into your .netrc file. It would be great if spr could source from there instead of requiring an env variable

When merging an spr stack, only the top PR is merged will the others are closed. This causes anything that relies on the "merged" status to fail, for example, GitHub's automatic changelog generator will only pick up the top PR when making a new release.

Gerrit has a neat feature where you can specify reviewers when pushing:

git push origin HEAD:refs/for/main%r=leo

(with fuzzy matching for the username)

spr could emulate this and allow specifying a spr update -r leo parameter to set the default reviewer for all commits in the stack.

If a branch name has . character in it, SPR has weird behavior:

• Running git spr update first time, results in:

  $ git spr update
  > git rev-parse --show-toplevel
  > git status --porcelain
  > git remote -v
  pull request stack is empty

  The PR is created properly:
  

• Running git spr status results in:

  git spr status
  > git rev-parse --show-toplevel
  > git status --porcelain
  > git remote -v
  pull request stack is empty

• Running git spr update second time results in:

  git spr update
  > git rev-parse --show-toplevel
  > git status --porcelain
  > git remote -v
  panic: A pull request already exists for sbienkow-ninja:pr/sbienkow-ninja/branch.with.dots/677369d5.
  
  goroutine 1 [running]:
  github.com/ejoffe/spr/github/githubclient.check({0x8720a0, 0xc0000b4060})
      /Users/runner/work/spr/spr/github/githubclient/client.go:542 +0xfa
  github.com/ejoffe/spr/github/githubclient.(*client).CreatePullRequest(0xc000312e30, {0x879e70, 0xc0000aa000}, 0xc0005520a0, {{0xc00021c0e1, 0x8}, {0xc00021c007, 0x28}, {0xc00021c098, 0x1c}, ...}, ...)
      /Users/runner/work/spr/spr/github/githubclient/client.go:307 +0x5d5
  github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc000069080, {0x879e70, 0xc0000aa000})
      /Users/runner/work/spr/spr/spr/spr.go:161 +0xa98
  main.main.func3(0xc0000bf5e0)
      /Users/runner/work/spr/spr/cmd/spr/main.go:139 +0x25
  github.com/urfave/cli/v2.(*Command).Run(0xc0003145a0, 0xc00002ea00)
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x64a
  github.com/urfave/cli/v2.(*App).RunContext(0xc000001d40, {0x879e70, 0xc0000aa000}, {0xc0000be000, 0x2, 0x2})
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x81e
  github.com/urfave/cli/v2.(*App).Run(...)
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
  main.main()
      /Users/runner/work/spr/spr/cmd/spr/main.go:174 +0xde5

• Running git spr merge results in:

  git spr merge
  > git rev-parse --show-toplevel
  > git status --porcelain
  > git remote -v
  panic: A pull request already exists for sbienkow-ninja:pr/sbienkow-ninja/branch.with.dots/677369d5.
  
  goroutine 1 [running]:
  github.com/ejoffe/spr/github/githubclient.check({0x8720a0, 0xc000454150})
      /Users/runner/work/spr/spr/github/githubclient/client.go:542 +0xfa
  github.com/ejoffe/spr/github/githubclient.(*client).CreatePullRequest(0xc0002c0010, {0x879e70, 0xc000118000}, 0xc000100f00, {{0xc0002b80e1, 0x8}, {0xc0002b8007, 0x28}, {0xc0002b8098, 0x1c}, ...}, ...)
      /Users/runner/work/spr/spr/github/githubclient/client.go:307 +0x5d5
  github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc0000680c0, {0x879e70, 0xc000118000})
      /Users/runner/work/spr/spr/spr/spr.go:161 +0xa98
  main.main.func4(0xc00012e4a0)
      /Users/runner/work/spr/spr/cmd/spr/main.go:151 +0x48
  github.com/urfave/cli/v2.(*Command).Run(0xc0003ec240, 0xc0002c84c0)
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x64a
  github.com/urfave/cli/v2.(*App).RunContext(0xc000082d00, {0x879e70, 0xc000118000}, {0xc00012e000, 0x2, 0x2})
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x81e
  github.com/urfave/cli/v2.(*App).Run(...)
      /Users/runner/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
  main.main()
      /Users/runner/work/spr/spr/cmd/spr/main.go:174 +0xde5

It'd be nice to be able to override the upstream to use per-branch (extract value of https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote if present) as well as per-repository (for example use value from https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault if present), falling back to origin.

Possibly, there needs to also be a split of remote to which refs are pushed and remote on which PRs are opened. Currently it isn't possible to create PRs to open-source repos like this one.

When I had panics for previous git amend attempts, I was left with fixup commits that opened PRs and I had no way of consolidating them and close the PR automatically. After doing a manual fixup, I just had to go close the PRs. Maybe there could be a way to do like a git spr clean which will look at all the current commits and commit IDs and close any PRs that don't have a matching commit ID?

I have installed SPR with use of deb package. This is the installed version:
spr version : 0.2.0 : 2021-05-27T05:05:34Z : c49a3038.

When trying to check the PRs status I get:

$ git spr --status
panic: Could not resolve to a Repository with the name '/'.

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
        /Users/eitan/Code/spr/spr/spr.go:788
github.com/ejoffe/spr/spr.(*stackediff).getGitHubInfo(0xc0000c0480, 0x7d5ad0, 0xc0000a8000, 0xc0000bc050, 0x747280)
        /Users/eitan/Code/spr/spr/spr.go:489 +0xa15
github.com/ejoffe/spr/spr.(*stackediff).StatusPullRequests(0xc0000c0480, 0x7d5ad0, 0xc0000a8000)
        /Users/eitan/Code/spr/spr/spr.go:248 +0x92
main.main()
        /Users/eitan/Code/spr/cmd/spr/main.go:74 +0x3ab

This is worrying for two reasons:

1. Hardcoded(?) paths
2. Inability to find the repo name.

For 2. I checked the config file:

$ cat .spr.yml 
githubRepoOwner: ""
githubRepoName: ""
requireChecks: true
requireApproval: true
showPRLink: true

After putting repo owner and repo name manually, the panic is not occurring.

When you run git spr outside of workspace (for example /tmp), it results in a panic:

$ git spr
panic: exit status 128

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
	/Users/eitan/Code/spr/spr/spr.go:896
github.com/ejoffe/spr/spr.ConfigFilePath(0x7235e0, 0xc00000e060)
	/Users/eitan/Code/spr/spr/config.go:26 +0x10b
main.main()
	/Users/eitan/Code/spr/cmd/spr/main.go:52 +0x199

Should probably put a meaningful message instead.

Even though the key is unlocked with ssh-agent, the tool is asking multiple times for the passphrase.
This makes the tool unusable.

Example:

$ git spr -u                   
Enter passphrase for key '/home/example/.ssh/id_rsa': 
Enter passphrase for key '/home/example/.ssh/id_rsa': 
Enter passphrase for key '/home/example/.ssh/id_rsa': 
Enter passphrase for key '/home/example/.ssh/id_rsa':

git error: /usr/lib/git-core/git-rebase: 1: eval: /usr/bin/true: not found
Could not execute editor
panic: exit status 1

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
        /Users/eitan/Code/spr/spr/spr.go:426
github.com/ejoffe/spr/spr.(*stackediff).mustgit(...)
        /Users/eitan/Code/spr/spr/spr.go:421
github.com/ejoffe/spr/spr.(*stackediff).AmendCommit(0xc0000d3f28, 0x5efa78, 0xc000116000)
        /Users/eitan/Code/spr/spr/spr.go:73 +0x6b0
main.main()
        /Users/eitan/Code/spr/cmd/amend/main.go:57 +0x32e

It looks like the path to true is hardcoded for /usr/bin/true as per this line:

I have /bin/true which I just copied to /usr/bin/true which fixed this problem but maybe spr should just look for something in the PATH?

We use main as our primary branch for tracking releases and develop for merging in all PRs / commits to be released. I noticed a few files where master is hardcoded which you might want to make configurable.

Furthermore, even though I use develop as our development branch spr still rebases my branch on master.

Here are the lines:

When landing a stack, when changing the top PR's base, reviews get dismissed automatically when "Dismiss stale pull request approvals when new commits are pushed" is enabled.

This breaks the workflow in repos with that option enabled. Honestly not sure what to do about it, though...

I was wondering if using flags (-s, -m, -u) was the idiomatic way of handling different actions.

It seems to me that commands (git spr update, git spr merge, git spr status) may be more idiomatic, as commands exclude one another by their nature (git spr update merge is plain wrong, just as you would not use git diff status), whereas flags seem like they can all be passed in at the same time git spr -u -s -m is as idiomatic as git diff -a -p)

Then, we could also provide command-specific help, e.g. only git spr status -h would include information from #54.

This is a minor issue, but I thought that from UNIX/CLI standpoint, commands make more sense to me in this case and I wanted to bring it up 🙂

When running git spr -u, it seems to be ignoring custom settings in gitconfig:

$ git spr -u
git error: ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
panic: exit status 128

goroutine 18 [running]:
github.com/ejoffe/spr/spr.check(...)
	/Users/eitan/Code/spr/spr/spr.go:896
github.com/ejoffe/spr/spr.mustgit(...)
	/Users/eitan/Code/spr/spr/spr.go:747
github.com/ejoffe/spr/spr.(*stackediff).fetchAndGetGitHubInfo.func1()
	/Users/eitan/Code/spr/spr/spr.go:513 +0xe9
created by github.com/ejoffe/spr/spr.(*stackediff).fetchAndGetGitHubInfo
	/Users/eitan/Code/spr/spr/spr.go:518 +0x8f

According to stacktrace, it seems to run git fetch and fails. Running git fetch manually works just fine.

When setting LogLevel DEBUG in ssh config, I can see it attempting to use wrong ssh keys (uses ~/.ssh/id_rsa).

My global .gitconfig has following entry:

[includeIf "gitdir:~/Work/"]
  path = .gitconfig-work

And then .gitconfig-work has:

# vim: ft=gitconfig
[core]
  sshCommand = ssh -i ~/.ssh/id_rsa_work -F /dev/null
[user]
  name = Szymon Bieńkowski
  email = [email protected]

I suspect this is caused when running from a freshly-added directory and SPR tries to rebase the branch.

git spr -u
panic: exit status 128

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
	/Users/eitan/Code/spr/spr/spr.go:908
github.com/ejoffe/spr/spr.mustgit(...)
	/Users/eitan/Code/spr/spr/spr.go:754
github.com/ejoffe/spr/spr.(*stackediff).getLocalCommitStack(0xc0001b2180, 0xc0002d5af0, 0x40de58, 0x48)
	/Users/eitan/Code/spr/spr/spr.go:418 +0xc5
github.com/ejoffe/spr/spr.(*stackediff).syncCommitStackToGitHub(0xc0001b2180, 0x7d7a90, 0xc00001a180, 0xc0002f6960, 0x0, 0x0, 0x0)
	/Users/eitan/Code/spr/spr/spr.go:632 +0x85
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc0001b2180, 0x7d7a90, 0xc00001a180)
	/Users/eitan/Code/spr/spr/spr.go:97 +0x10a
main.main()
	/Users/eitan/Code/spr/cmd/spr/main.go:92 +0x7d6

Version:

git spr --version
spr version : 0.4.4 : 2021-06-10T18:17:56Z : da3a051c

Even though bash completion is enabled in the cli here it is not working.
In the cli package docs we can see that adding /etc/bash_completion.d/git-spr should help, but it didn't work as well.
It only works when invoking SPR with git-spr, but not by calling git subcommand git spr.

My request is to:

1. Add proper file to /etc/bash_completion.d
2. Figure out a way to work with git spr
3. Probably have some support for non bash shells as well

Using git checkout causes issues, for example when you make a few commits, then cd to a new directory and commit it too. If you were to run git spr update, it'll checkout first commit, leaving the shell in a non-existent directory and causing many shell hooks to scream.

It looks like most of those can be avoided - for example it appears that checkout is being done to both create/reset a branch as well as push it. These can be done without checking anything out. What is more - a local branch is not needed at all so creating it in order to delete it later can be avoided.

For example:

• Create a branch: git branch my_new_branch <COMMIT_HASH>
• Since you might be moving existing branch: git branch --force my_new_branch <COMMIT_HASH> (Seems to no longer be the case as local branches are deleted after push?)
• To push a local branch that you don't have checked out: git push origin <LOCAL_REF>:<REMOTE_BRANCH>. This allows pushing arbitrary refs, without the need to create local branches at all (and pollute reflog).

Referenced code which could be simplified (maybe there's more):
https://github.com/ejoffe/spr/blob/master/spr/spr.go#L402-L417
https://github.com/ejoffe/spr/blob/master/spr/spr.go#L421-L428

git error: There is no tracking information for the current branch.
Please specify which branch you want to rebase against.
See git-rebase(1) for details.

    git rebase <branch>

If you wish to set tracking information for this branch you can do so with:

    git branch --set-upstream-to=origin/<branch> abatilo/testing-spr

panic: exit status 1

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
        /Users/eitan/Code/spr/spr/spr.go:426
github.com/ejoffe/spr/spr.(*stackediff).mustgit(...)
        /Users/eitan/Code/spr/spr/spr.go:421
github.com/ejoffe/spr/spr.(*stackediff).AmendCommit(0xc000139f28, 0x5efa78, 0xc00001c110)
        /Users/eitan/Code/spr/spr/spr.go:73 +0x6b0
main.main()
        /Users/eitan/Code/spr/cmd/amend/main.go:57 +0x32e

When I tried to amend a commit, it panicked when the branch I created was not yet pushed to the remote.

I marked the top commit on this stack WIP and inserted another commit beneath it:

... which somehow lead to this PRs body changing:

#155

... to the old message of #147. Not sure what happened here?

Removing the "WIP" title and re-running spr undid the damage.

Hi, currently updating the spr is a manual process. Is there a chance for adding a package to ubuntu repository? Or providing a simple installation script?

The current logic for installing the commit-msg hook is to symlink the spr_commit_hook bin directly into .git/hooks/commit-msg. However, if a user already has a commit hook of this name, it just fails silently, and you don't find out it was a problem until you try to spr update and it complaints about the commit-id trailer being missing.

Instead, we should check to see if a hook already exists, and if so, append spr_commit_hook "$@" to the bottom of it (if we want to be REALLY safe, we can check that it's a bash plaintext file first). If one doesn't exist, we can continue with the existing behavior.

PR to fix this incoming shortly.

One of the PR requirements in our repo is that a CODEOWNER needs to approve a PR changing code they are responsible for. I had a bunch of PRs which only modified files with MyOrg/devops codeowner, and in between them there was a PR with MyOrg/backend codeowner.

Running git spr merge changes "top" PR target branch to master, merges it and the ncloases all remaining ones. The issue is that this PR now has a commit modifying files owned the MyOrg/backend, and thus GH rejects such merge:

$ git spr merge
2:45PM FTL ../../../../../../../../Users/eitan/Code/spr/github/githubclient/client.go:292 > pull request merge failed error="Waiting on code owner review from MyOrg/backend." id=oa428PUSP2Y3SuXgHgKdIReqvfrUuaGL number=123 title="My last PR in stack"

Hey 🙂

I wanted to use another person's commit (not merged to master yet) as a base. The commits were as so:

• master
• commit-from-colleague (has an open PR using git spr)
• my-commit <- HEAD (no PR yet)

After executing git spr update, spr created 2 PRs - one for my commit, but another one for commit-from-colleague even though that commit already had a PR.

Can we detect such situations and then use an existing branch/PR as a base instead of recreating the PR as-if it was my own?

VERSION: 0.7.6 : 2021-09-26T20:22:29Z : ac0b650

git amend
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1124c73]

goroutine 1 [running]:
github.com/ejoffe/spr/spr.(*stackediff).getLocalCommitStack(0xc00015df28, 0x482, 0x1400f18, 0x500)
/Users/eitan/Code/spr/spr/spr.go:253 +0x53
github.com/ejoffe/spr/spr.(*stackediff).AmendCommit(0xc00015df28, 0x11b0d78, 0xc00011c000)
/Users/eitan/Code/spr/spr/spr.go:45 +0x45
main.main()
/Users/eitan/Code/spr/cmd/amend/main.go:55 +0x366

This consistenly happens whenever I run git spr -m. The PRs are getting merged properly regardless of this error. This was happening since my first SPR, which I believe was v0.4.3. For sure happened in v0.4.4 and continues to happen in v0.5.0.

$ git spr -m
git error: error: unable to delete 'pr/sbienkow-ninja/mybranch/1e2f4484': remote ref does not exist
error: failed to push some refs to 'github.com:organisation/repo.git'
error deleting branch: exit status 1
MERGED github.com/organisation/repo/pull/392 : My First Commit
MERGED github.com/organisation/repo/pull/387 : My Second Commit

Sadly I didn't have new logging enabled, will do that next time I merge something.

Hey! I'm using spr and I have encountered a problem with reordering commits that were mergeable. I had a stack of 4 commits, the 2nd and the 4th one were approved, but the 1st and 3rd were not.

$ git spr -s
[✔✔✔✗] commit D
[·✗✔✗] commit C
[✔✔✔✗] commit B
[·✗✔✗] commit A

I ran git rebase -i and reordered them, so they were in the following order:

1. B
2. D
3. A
4. C

and then I wanted to update the PRs to then merge commits B and D. Howeever, after running git spr -u, I got the following error:

$ git spr -u
panic: Cannot change the base branch of a closed pull request.

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
        /Users/eitan/Code/spr/spr/spr.go:896
github.com/ejoffe/spr/spr.updateGithubPullRequest(0x7d7a30, 0xc00001a0c8, 0xc00000e0f8, 0xc0004e40a0, 0xc0003a1e40, 0xc00030cd1c, 0x8, 0xc00030cc70, 0x28, 0xc00030ccea, ...)
        /Users/eitan/Code/spr/spr/spr.go:738 +0x3ee
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc000026680, 0x7d7a30, 0xc00001a0c8)
        /Users/eitan/Code/spr/spr/spr.go:105 +0x2e5
main.main()
        /Users/eitan/Code/spr/cmd/spr/main.go:86 +0x747

The result was that PRs for commits B and D were merged, but the commits were not reordered, so I ended up with 2 open PRs, one of which had 3 commits, and another one had 1 commit:

1. PR with B, D, A
2. PR with C

Ideally, git spr -u would only rebase those PRs so the order was correct, and not merge anything.

If you end up not going through with any of the work you had reviewed, it would be really nice if there were a way to close all the PRs that you opened

Currently, running git spr update in either a worktree or a submodule, ends with:

2021/08/16 15:45:18 symlink /home/sbienkow/.local/bin/spr_commit_hook .git/hooks/commit-msg: not a directory

That is because in worktrees and submodules, .git is not a directory but a file which contains path to its ".git" folder.

It'd be to at least receive a more friendly message :)

Our team makes extensive use of environment variables and one of our hooks is a ruby-based git hook manager overcommit which is installed in a specific ruby environment managed by the ruby version manager rbenv which uses environment variables for setting the current version.

When I used git spr u it couldn't find overcommit, and I also couldn't disable overcommit using OVERCOMMIT_DISABLED=1 because spr does not forward all the environment variables to the underlying git process.

Often when looking at the output of git spr -s I need to go back to the readme and read https://github.com/ejoffe/spr#managing-pull-requests.

It would be great if that information about specific status checks was included in git spr -h (or maybe git spr --help for extended help, or git spr -s -h) 🙂

A Git commit with this message:

This is a test:
    A B C

...results in a PR with this body:

This is a test:
A B C

ghstack has a nice feature visualizing the stack:

Large stacks are usually reviewed in order, so this would be a useful feature to have (or perhaps it already does and I just didn't know how to turn it on?)

One other option that might make this a bit easier is to add a flag: git spr merge --upto 123 which will make git spr only merge part of the stack up to that pr. This way one can merge a batch of changes with the same reviewer in one call, then wait for actions to run, and merge the next batch.

Originally posted by @ejoffe in #98 (comment)

This would be awesome to have - many times, you want to merge the "lower" parts of the stack while the "top" parts are still under review. Right now, this doesn't seem to be possible (or is it?)

Hi, I tried .deb installation as well as in brew.
After running "git spr update" there was a problem with commit hook:
`git spr update
...
Add ticket from branch to commit message.................................Passed
Run StackedPR message hook...............................................Failed

• hook id: spr
• exit code: 1

Executable spr_commit_hook not found`

It was not linked to .git/hooks/commit_msg unfortunately.
I had to find your older commit with instruction and link it manually.

It'd be nice to be able to use subcommand shortcuts like git spr u. Makes it faster to use, without having to add custom git aliases.

Wha'ts more, current behavior that invalid subcommands cause git spr status to be triggered is super confusing. When I first switched to new version and did git spr u, status was shown without any error information. Took me a while to realize it didn't actually update my PR stack. IMO, running git spr hell should show an error that such subcommand doesn't exist and (maybe) also show help with list of all subcommands.

Hey!

Would it be possible to add a warning/error message when running git spr update on a branch that looks like a remote auto-generated spr branch (when the current branch has the format pr/username/branchname/commitid)?

One of my team members raised a PR with spr and when they wanted to update it, they checked out the remote branch that was used in the PR (pr/username/branchname/commitid). Then, they amended the commit and ran git spr update again, which created a branch pr/username/pr/username/branchname/commitid/commitid (it correctly used pr/username/branchname/commitid as the local branch name).

I reckon this may be a common mistake newcomers may make, so having a warning when the local branch name matches the format for remote branch names would be great.

I tried to land the current stack, and this happened:

(minikube) (develop) spr $ git spr status 
> git rev-parse --show-toplevel
> git status --porcelain
> git remote -v

 ┌─ github checks pass
 │ ┌── pull request approved
 │ │ ┌─── no merge conflicts
 │ │ │ ┌──── stack check
 │ │ │ │
[❌➖✅❌] https://github.com/ejoffe/spr/pull/155 : Use emojis for "spr status" output <-- I dropped this one from my local branch
[✅➖✅✅] https://github.com/ejoffe/spr/pull/156 : Support the "gh" CLI tool in addition to "hub"
[✅➖✅✅] https://github.com/ejoffe/spr/pull/154 : Only update run count when user is not already a stargazer
[✅➖✅✅] https://github.com/ejoffe/spr/pull/153 : Do not panic on runtime errors
[✅➖✅✅] https://github.com/ejoffe/spr/pull/152 : Manual merge notice
[✅➖✅✅] https://github.com/ejoffe/spr/pull/151 : README.md: document how to install from source
[✅➖✅✅] https://github.com/ejoffe/spr/pull/150 : Wrap commit body in code area in PR message
[✅➖✅✅] https://github.com/ejoffe/spr/pull/149 : Mark current position in stack
[✅➖✅✅] https://github.com/ejoffe/spr/pull/148 : Include https:// in PR link
[✅➖✅✅] https://github.com/ejoffe/spr/pull/147 : Reformat PR comment messages

(minikube) (develop) spr $ git spr merge 
> git rev-parse --show-toplevel
> git status --porcelain
> git remote -v
MERGED https://github.com/ejoffe/spr/pull/147 : Reformat PR comment messages
MERGED https://github.com/ejoffe/spr/pull/148 : Include https:// in PR link
MERGED https://github.com/ejoffe/spr/pull/149 : Mark current position in stack
MERGED https://github.com/ejoffe/spr/pull/150 : Wrap commit body in code area in PR message
MERGED https://github.com/ejoffe/spr/pull/151 : README.md: document how to install from source
MERGED https://github.com/ejoffe/spr/pull/152 : Manual merge notice
MERGED https://github.com/ejoffe/spr/pull/153 : Do not panic on runtime errors
MERGED https://github.com/ejoffe/spr/pull/154 : Only update run count when user is not already a stargazer
MERGED https://github.com/ejoffe/spr/pull/156 : Support the "gh" CLI tool in addition to "hub"
panic: was submitted too quickly

goroutine 1 [running]:
github.com/ejoffe/spr/github/githubclient.check({0x8774c0, 0xc00012aae0})
	/home/leopold/projects/spr/github/githubclient/client.go:542 +0xfa
github.com/ejoffe/spr/github/githubclient.(*client).CreatePullRequest(0xc0002b1f90, {0x87f370, 0xc00001c128}, 0xc00008e050, {{0xc00013a5ed, 0x8}, {0xc00013a4a0, 0x28}, {0xc00013a51f, 0x13}, ...}, ...)
	/home/leopold/projects/spr/github/githubclient/client.go:307 +0x5d5
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc00068f200, {0x87f370, 0xc00001c128})
	/home/leopold/projects/spr/spr/spr.go:161 +0xa98
main.main.func4(0xc00031aa20)
	/home/leopold/projects/spr/cmd/spr/main.go:151 +0x48
github.com/urfave/cli/v2.(*Command).Run(0xc0003187e0, 0xc0002b9200)
	/home/leopold/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x64a
github.com/urfave/cli/v2.(*App).RunContext(0xc0001036c0, {0x87f370, 0xc00001c128}, {0xc000012040, 0x2, 0x2})
	/home/leopold/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x81e
github.com/urfave/cli/v2.(*App).Run(...)
	/home/leopold/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
main.main()
	/home/leopold/projects/spr/cmd/spr/main.go:174 +0xde5

Even though it worked, it got confused and created a new set of PRs for stuff already landed:

• #173 : README.md: document how to install from source
• #172 : Wrap commit body in code area in PR message
• #171 : Mark current position in stack
• #170 : Include https:// in PR link
• #169 : Reformat PR comment messages

It also closed this one because the local commit went away (fair enough, what is the proper way to land only parts of a stack?):

• #155

This happened after #168, when I switched back to my local branch develop.

This happened when I typed git spr u and I didn't have any stashed changes. The branch on remote was updated, but the pull request wasn't created. Detailed logs:

git error: No stash entries found.
panic: exit status 1

goroutine 1 [running]:
github.com/ejoffe/spr/spr.check(...)
	/Users/eitan/Code/spr/spr/spr.go:425
github.com/ejoffe/spr/spr.(*stackediff).mustgit(0xc0001a4550, 0x842ad0, 0x9, 0x0)
	/Users/eitan/Code/spr/spr/spr.go:420 +0x86
github.com/ejoffe/spr/spr.(*stackediff).syncCommitStackToGitHub(0xc0001a4550, 0x8c25b0, 0xc00001a140, 0xc00057e000, 0x3, 0x4, 0xc0003da5a0)
	/Users/eitan/Code/spr/spr/spr.go:412 +0x5a5
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0xc0001a4550, 0x8c25b0, 0xc00001a140)
	/Users/eitan/Code/spr/spr/spr.go:102 +0x3c6
main.main.func3(0xc000188d80, 0x2, 0x2)
	/Users/eitan/Code/spr/cmd/spr/main.go:139 +0x3c
github.com/urfave/cli/v2.(*Command).Run(0xc000246120, 0xc000188c80, 0x0, 0x0)
	/Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x4dd
github.com/urfave/cli/v2.(*App).RunContext(0xc000194d00, 0x8c25b0, 0xc00001a140, 0xc00018c000, 0x2, 0x2, 0x0, 0x0)
	/Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x810
github.com/urfave/cli/v2.(*App).Run(...)
	/Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
main.main()
	/Users/eitan/Code/spr/cmd/spr/main.go:174 +0xc77

The workaround is to create a dummy stash entry, but after the git spr u it's popped from the stack, which is inconvenient.

The message says:

A commit is missing a commit-id.                                                                                                                                                                                   
This most likely means the commit-msg hook isn't installed.                                 
To install the hook run the following cmd in the repo root dir:                                                                                                                                                    
 > ln -s spr-commithook .git/hooks/commit-msg

The last line should be:

 > ln -s $(which spr_commit_hook) .git/hooks/commit-msg

With CleanupRemoteBranch option set to true, when spr deletes the branch of a pr that just merged, the next pr up the stack gets automatically closed by github. This appears to be a race between the base branch of the next pr changing, and the branch getting cleanup up by spr.

`

git rev-parse --show-toplevel
github fetch pull requests
git branch
github update 14 - feature 5
github merge 14 - feature 5
github merge 14: feature 5
git push -d origin pr/ejoffe/master/4ea09813
MERGED github.com/ejoffe/spr-demo/pull/14 : feature 5
github fetch pull requests
git fetch
git branch
git rebase origin/master --autostash
f8
git log origin/master..HEAD
git status --porcelain --untracked-files=no
git push --force --atomic origin b769870e29007005fdacb2dd5618866bf474d70a:refs/heads/pr/ejoffe/master/d33767df cfab98ee4c6e2131d6f89e510a33c199c8179dfd:refs/heads/pr/ejoffe/master/40addd96
github update 16 - feature 6
3:55PM FTL ../spr/github/githubclient/client.go:240 > pull request update failed error="Cannot change the base branch of a closed pull request." id=PR_kwDOFqhinc4tG-69 number=16 title="feature 6"
`

Case in point, this mess:

• #167
• #166
• #165
• #164
• #163
• #162
• #161
• #160
• #159

None of the commit-id tags changed.

When merging a stack the following error occurs:

git error: error: unable to delete 'pr/kwapik/branch_name/f9d7904b': remote ref does not exist
error: failed to push some refs to '[email protected]:owner/repo'            
panic: exit status 1                                                                                                                                                                                               
                                                                                                         
goroutine 1 [running]:                                                                                                                                                                                             
github.com/ejoffe/spr/spr.check(...)                                                                                                                                                                               
        /Users/eitan/Code/spr/spr/spr.go:867                                                             
github.com/ejoffe/spr/spr.mustgit(...)                                                                                                                                                                             
        /Users/eitan/Code/spr/spr/spr.go:737                                                             
github.com/ejoffe/spr/spr.(*stackediff).MergePullRequests(0xc0000c0640, 0x7d7a10, 0xc0000a8000)                                                                                                                    
        /Users/eitan/Code/spr/spr/spr.go:213 +0x10ec 
main.main()
        /Users/eitan/Code/spr/cmd/spr/main.go:89 +0x705

Expected behavior:

1. The top mergeable PR is merged
2. All the PRs "below" are closed

Observed behavior:

1. The top mergeable PR is merged
2. All the PRs "below" are not closed

I replicates the readme example and I got this error, I generated two tokens just in case with repo scope, but the same error arises.

panic: unable to fetch local commits

goroutine 1 [running]:
github.com/ejoffe/spr/spr.(*stackediff).getLocalCommitStack(0x90a6780, 0x83d0104, 0x29, 0x95c60f0)
        /Users/eitan/Code/spr/spr/spr.go:263 +0x213
github.com/ejoffe/spr/spr.(*stackediff).UpdatePullRequests(0x90a6780, 0x8437fac, 0x901a0e0)
        /Users/eitan/Code/spr/spr/spr.go:89 +0xaa
main.main.func3(0x9097260, 0x2, 0x2)
        /Users/eitan/Code/spr/cmd/spr/main.go:139 +0x2e
github.com/urfave/cli/v2.(*Command).Run(0x90d1b00, 0x90971c0, 0x0, 0x0)
        /Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/command.go:163 +0x3c4
github.com/urfave/cli/v2.(*App).RunContext(0x9098680, 0x8437fac, 0x901a0e0, 0x910a000, 0x2, 0x2, 0x0, 0x0)
        /Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:313 +0x651
github.com/urfave/cli/v2.(*App).Run(...)
        /Users/eitan/go/pkg/mod/github.com/urfave/cli/[email protected]/app.go:224
main.main()
        /Users/eitan/Code/spr/cmd/spr/main.go:174 +0xa26

Distributor ID: Ubuntu
Description: Ubuntu 18.04.4 LTS
Release: 18.04
Codename: bionic
git version 2.33.0

Currently, the conf file looks like this:

# repo settings
githubRepoOwner: asdf
githubRepoName: ijkl
requireChecks: true
requireApproval: true
cleanupRemoteBranch: true  # not sure about this one

# user settings
showPRLink: true
logGitCommands: false
logGitHubCalls: false
statusBitsHeader: true

This mixes per-repo settings and user settings. This is not perfect as:

• as an spr user, I'd like to tweak user settings globally,
• we keep the .spr.yml in the repo and having user settings there seems really weird (and causes a PR on each spr update).

Alternatively – merge spr yamls from different locations?

cc @kwapik @GregoRCodi

When installing from deb the following warning is showed:

$ sudo dpkg -i ~/Downloads/spr_amd64.deb 
dpkg: warning: parsing file '/var/lib/dpkg/status' near line 62848 package 'spr':
 missing 'Maintainer' field
dpkg: warning: parsing file '/var/lib/dpkg/tmp.ci/control' near line 11 package 'spr':
 missing 'Maintainer' field
(Reading database ... 329317 files and directories currently installed.)
Preparing to unpack .../Downloads/spr_amd64.deb ...
Unpacking spr (0.4.3) over (0.4.2) ...
Setting up spr (0.4.3) ...

This is really minor issue, not blocking any flow.

Hey @ejoffe, I'd appreciate if you could add me as a member to the repository - that way, I could use spr for my contributions to spr :)

With branch protection enabled, there shouldn't be a security risk.

The description of cleanupRemoteBranch in the readme (https://github.com/ejoffe/spr#configuration) seems to have the same content as for showPRLink

When switching to a different local branch, all upstream PRs will be recreated with a different branch name.

At the very least, we should display an error and refuse.

This is log created when both logGitCommands and logGitHubCalls is set to true. I don't know why it wouldn't merge the PR when all checks passed (confirmed in the WebUI).

$ git spr -m
> git rev-parse --show-toplevel
> github fetch pull requests
> git branch --show-current
> github fetch pull requests
> git fetch
> git branch --show-current
> git rebase origin/master --autostash
> git log origin/master..HEAD
> git status --porcelain --untracked-files=no
> git stash
> git stash pop
> github fetch pull requests
> git branch --show-current
[✔✗✔✗] github.com/organization/repo/pull/388 : Update SPR to v0.5.0

NOTE: This branch originally had 3 commits and this was the last one. The remaining 2 were merged earlier, this one wasn't and refuses to be merged now.