Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pushing to a specific git refspec #2098

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benbrittain
Copy link
Member

@benbrittain benbrittain commented Aug 16, 2023

Primarily for basic gerrit interop

Example:
```
jj git push --refspec refs/for/master
```

This is useful for code review tools like gerrit which expect the developer
to be able to push to arbitrary namespaces.
@martinvonz
Copy link
Member

I'm not sure how I feel about this. My main problem is how it would interact with the existing options we have for jj git push. For example, what does jj git push -c @- --refspec refs/for/master do? I think I know the answer; it just doesn't seem to make much sense.

When pushing multiple branches, I'd be surprised if anything other than Gerrit would accept multiple branches being pushed into a single other branch/ref.

I've previously proposed (on Discord only, I think) a separate command for pushing a single branch to another name on the remote. It might look something like jj git push-branch -r wty --to-branch main for pushing commit wty to branch main on the remote. That would avoid the problems with combining --to-branch with the other flags on jj git push (and, btw, the other flags play well with each other, I think). If we had such a command, it seems quite reasonable to add --to-ref to that in addition to --to-branch (but they're be mutually exclusive, of course). How does that sound?

@ilyagr
Copy link
Contributor

ilyagr commented Aug 17, 2023

It might look something like jj git push-branch -r wty --to-branch main for pushing commit wty to branch main on the remote.

I'm a little confused about how this would work with respect to remote-tracking branches for main. I guess it could be OK to not touch it, but only as long as the user never wants to fetch main from that remote or, ideally, fetch anything from that remote. There might be a solution short of fully supporting upstream branches, but I'm not sure.

This is less problematic with respect to jj git push-branch --to-ref, since the matching jj git fetch-branch --from-ref could just create the commit, inform the user what it is, but not put any branch there.

@martinvonz
Copy link
Member

I'm a little confused about how this would work with respect to remote-tracking branches for main. I guess it could be OK to not touch it, but only as long as the user never wants to fetch main from that remote or, ideally, fetch anything from that remote. There might be a solution short of fully supporting upstream branches, but I'm not sure.

I was thinking that this wouldn't do anything with remote-tracking branches. Do you see any problems with that?

@ilyagr
Copy link
Contributor

ilyagr commented Aug 17, 2023

I was thinking that this wouldn't do anything with remote-tracking branches. Do you see any problems with that?

The main think I'm worried about, I guess, is that if you jj git push-branch -r wty --to-branch main and then fetch from that remote, you'll have a "main" branch either created, or moved, or conflicted, at least unless some method like #2074 is used to prevent that (which might be doable). I'm not sure how bad this is, or if I'm looking at it wrong, but none of these options seem immediately great.

What should happen to remote-tracking branches depends on whether we want "moved" or "conflicted" in that situation, and on what we want to happen in other situations where the user does the pushing and/or fetching more than once.

On second thought, jj git fetch-ref is not so bad so long as it doesn't create any branches (even if the ref it fetches is a branch).

@martinvonz
Copy link
Member

The main think I'm worried about, I guess, is that if you jj git push-branch -r wty --to-branch main and then fetch from that remote, you'll have a "main" branch either created, or moved, or conflicted, at least unless some method like #2074 is used to prevent that (which might be doable). I'm not sure how bad this is, or if I'm looking at it wrong, but none of these options seem immediately great.

I think the main use case is for letting you create a few commits on top of main@origin without creating or moving any branches, and then jj git push-branch -r @- --to-branch main or something like that. If you then fetch from the remote, the main branch would typically simply have moved forward to the commit you pushed, which is what I would have wanted and expected. Oh, you're thinking that we should update our record of the remote when we push? Yeah, that seems better, both so the user sees an up to date main@origin and so we can deduce force-with-lease as we normally do.

@benbrittain
Copy link
Member Author

jj git push-branch does start to feel like an awkward name though since we're talking about jj git push-branch --to-ref working with this sub-command too.

@martinvonz
Copy link
Member

jj git push-branch does start to feel like an awkward name though since we're talking about jj git push-branch --to-ref working with this sub-command too.

True. jj git push-commit?

@benbrittain
Copy link
Member Author

I'll rewrite this patch to address what has been talked about here.

On second thought, jj git fetch-ref is not so bad so long as it doesn't create any branches (even if the ref it fetches is a branch).

Something like this would also be needed to get the basic gerrit support started

@martinvonz
Copy link
Member

On second thought, jj git fetch-ref is not so bad so long as it doesn't create any branches (even if the ref it fetches is a branch).

Something like this would also be needed to get the basic gerrit support started

Why is that needed? I'm not saying it's not, I probably just don't know enough about Gerrit.

@benbrittain
Copy link
Member Author

The way someone interacts with gerrit changes is through a ref:

git fetch ssh://HOST/REPO refs/changes/UniqueRefIdentifier && git checkout/cherry-pick/ FETCH_HEAD

So, being able to grab a ref and interact with it is important

@necauqua
Copy link
Contributor

Also github has those readonly PR refs I mentioned too

@ilyagr
Copy link
Contributor

ilyagr commented Aug 18, 2023

The main think I'm worried about, I guess, is that if you jj git push-branch -r wty --to-branch main and then fetch from that remote, you'll have a "main" branch either created, or moved, or conflicted, at least unless some method like #2074 is used to prevent that (which might be doable). I'm not sure how bad this is, or if I'm looking at it wrong, but none of these options seem immediately great.

I think the main use case is for letting you create a few commits on top of main@origin without creating or moving any branches, and then jj git push-branch -r @- --to-branch main or something like that. If you then fetch from the remote, the main branch would typically simply have moved forward to the commit you pushed, which is what I would have wanted and expected. Oh, you're thinking that we should update our record of the remote when we push? Yeah, that seems better, both so the user sees an up to date main@origin and so we can deduce force-with-lease as we normally do.

I was actually getting things backwards before. There's a mixup keeps happening when I think about fetching & remote-tracking branches.

On third thought, I now think that the only reasonable thing to do is to have jj git push-branch (or whatever it's called) not touch remote-tracking branches in any way, just as you suggested. I no longer think that this is likely to cause much trouble.

Why I think push-branch must not touch the remote-tracking branches.

Let's say jj git push-branch -r wty --to-branch main moved the remote-tracking branch for main. Then, a subsequent jj git fetch would see that the actual position of the branch on the remote is the same as the position of the remote-tracking branch. In this case, jj git fetch doesn't fetch anything, as it assumes the local repo is up to date. This is clearly bad. Also, jj git push would replace the branch on the remote for the same reason, which is also bad.

OTOH, if the remote-tracking branch was left where it was, jj git fetch would fetch the new commits properly and either fast-forward the branch or create a conflict (as appropriate). jj git push would do nothing since it will notice that the branch was updated on the remote (the remote-tracking branch does not match the actual position of the branch on the remote).

Why I was worried

This feature makes it more likely that people use remotes with branch names that don't correspond to the local branches (e.g. if the remote's main branch corresponds to, say, the local prod branch as opposed to the local main branch).

jj doesn't have great support for such remotes at the moment, but I now think that a push-branch operation won't create any extra bugs (on top of any bugs jj may already have when dealing with such remotes). If you are fetching from a weird remote with jj git fetch, the same thing would happen regardless of whether the reason for its weirdness is that you used jj push-branch to move a branch in it or whether the remote was that way when you added it.

Fetching like this is probably not a great idea (unless you're doing something tricky), but it should merely result in local branches becoming conflicted. Hopefully, these conflicts will be easy to understand and easy to resolve (using the remote-tracking branches, for example).

@yuja
Copy link
Contributor

yuja commented Aug 18, 2023

Let's say jj git push-branch -r wty --to-branch main moved the remote-tracking branch for main. Then, a subsequent jj git fetch would see that the actual position of the branch on the remote is the same as the position of the remote-tracking branch.

In that scenario, new position of the remote-tracking main can be observed by cmd_git_push*() and local main will move without issuing jj git fetch. It's not probably what we want, but that's how we handle remote-tracking branches right now. If we didn't move the tracking branch (which I don't know how to achieve with libgit2), jj git push-commit -r x --to-branch main; ...; jj git push-commit -r y --to-branch main might fail because the tracking branch is out of sync?

For Gerrit use case, I think this kind of stuff wouldn't matter since refs/for isn't tracked.

@thoughtpolice
Copy link
Contributor

The motivation for this change was Gerrit, but that will ideally be subsumbed by #2845, avoiding most of the nits and UX problems we've talked about here — though we could address them another time, I firmly believe this way forward is much better for Gerrit support.

@Zambito1
Copy link

Zambito1 commented Apr 3, 2024

I have recently been playing around with Radicle, which is a decentralized git source forge. It takes advantage of pushing to specific refspecs for features like creating patches. I haven't used Gerrit, from what I gather it probably does something similar.

The Radicle documentation for this can be found here: https://radicle.xyz/guides/user#working-with-patches

@yuja yuja mentioned this pull request Apr 29, 2024
@matts1
Copy link
Contributor

matts1 commented May 8, 2024

FWIW, my workflow involves no local branches. I simply have the following alias in my bashrc to do all my github PRs.

github_push () {
        git push "$1" "$(jj log -T commit_id --no-graph --no-pager -r "$2"):refs/heads/$3" --force
}

So I write something like github_push origin @ refs/heads/my_pr

I'd be a fan of anything that allows me to use jj to achieve this, since this is a little awkward. I'd like something along the lines of jj git push -c @ --destination refs/heads/main --force.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants