Target is incorrect when using `--remote-name` and `--fork-name`

[aaron-skydio]

Describe the bug When creating a PR by pushing to a fork, the target branch is shown as the fork target branch. Here origin is the fork and upstream is the repo where the PR should be made:

$ revup --remote-name=upstream --fork-name=origin upload
W: Couldn't find an existing label named main                                                                                                                 

Topic: copybara-null-baseline → origin/main
Commits:
  Forward `null` when parent cannot be found
Github URL:
   (new) (pushed)
Press <Enter> to continue or <Ctrl-C> to quit

Topic: copybara-null-baseline → origin/main
Commits:
  Forward `null` when parent cannot be found
Github URL:
  https://github.com/google/copybara/pull/228 (new) (pushed)

Expected behavior The Topic line should read Topic: copybara-null-baseline -> upstream/main

Other info

$ git remote -v
origin  https://github.com/skydio/copybara (fetch)
origin  https://github.com/skydio/copybara (push)
upstream        https://github.com/google/copybara (fetch)
upstream        https://github.com/google/copybara (push)

[jerry-skydio]

this may be a bit odd but its highly built into how forks work

Notably the git.py class doesn't maintain knowledge of 2 different remotes, it only knows about the one that it is pushing to / pulling from / comparing content in. When using a fork, this is always the fork remote. Up until the actual github requests, its perhaps accurate to say that revup "thinks" it is making a PR against the fork, we just stick the upstream remote into the github api at the last moment.

another way to think about it is that the arrow describes the "remote base" commit used for the branches. because of the above, revup will only ever use fork/main as the base for branches, it won't ever use anything from the upstream remote. thus putting upstream/main would be more right in some ways but less right in others

maybe the best thing to do is just remove the remote prefix from that line altogether?

[aaron-skydio]

Oh I see, yeah that totally makes sense. Yeah it's probably clearer to just remove the remote prefix? Since it's probably clear without it for the non-fork case, and for the fork case just has potential to cause this sort of confusion?