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

Failure to follow config push settings #2537

Open
TypicalUsername-ai opened this issue Feb 25, 2025 · 8 comments · May be fixed by #2542
Open

Failure to follow config push settings #2537

TypicalUsername-ai opened this issue Feb 25, 2025 · 8 comments · May be fixed by #2542
Labels
bug Something isn't working
Milestone

Comments

@TypicalUsername-ai
Copy link

Describe the bug
I'm working on a repo with multiple remotes (origin and fork) and have a branch locally with different name which pushes to main branch on the fork

To Reproduce

  1. set upstream of a branch to a differently named one:
    .git/config
[branch "lints"]
	remote = workspace-lints
	merge = refs/heads/main
  1. set default for push to upstream (without it the cli asks you to specify to which branch)
    in .git/config
[push]
    default = upstream
  1. prepare a commit and push with gitui
  2. gitui creates a branch with the name of the local branch and pushes to it

Expected behavior
when executing git push --verbose on such branch

Pushing to gitlab.torproject.org:matt022/workspace-lints
To gitlab.torproject.org:matt022/workspace-lints
 = [up to date]          lints -> main
updating local tracking ref 'refs/remotes/workspace-lints/main'
Everything up-to-date

Screenshots
N/A

Context (please complete the following information):
Rust: cargo 1.84.0 (66221abde 2024-11-19) (stable)
gitui: 0.27.0
OS: NixOS 25.05 (Warbler) x86_64
Host: Windows Subsystem for Linux - NixOS (2.4.11)
Kernel: Linux 5.15.167.4-microsoft-standard-WSL2
Shell: bash 5.2.37

Additional context
As noted in the logs this is a gitlab instance if that changes things

@TypicalUsername-ai TypicalUsername-ai added the bug Something isn't working label Feb 25, 2025
@vlad-anger
Copy link

vlad-anger commented Feb 27, 2025

Same issue on gitui 0.27.0 2025-01-14 (99f6967)

Seems like repo config branch.<name>.merge did not affect GitUI, it will try to push to remote same named as local branch which you push from.

man git-config

branch.<name>.merge

It tells git fetch/git pull/git rebase which branch to merge and can also affect git push (see push.default).

I guess we should consider "can" as "should" for GitUI
Push remote same name as local branch should be default if branch.<name>.merge is not set in repo git config. (Current behavior)
Otherwise we push to remote branch which specified in config (We should check/add logic for)
That will allow to have local branch different name then remote
@extrawurst what do you think?

@vlad-anger vlad-anger linked a pull request Mar 1, 2025 that will close this issue
4 tasks
@naseschwarz
Copy link
Contributor

naseschwarz commented Mar 14, 2025

I don't want to discuss what's better or worse, but I've been looking at #2542 and was confused until I saw this:

It tells git fetch/git pull/git rebase which branch to merge and can also affect git push (see push.default).

I guess we should consider "can" as "should" for GitUI

While this is probably a configuration (push.default=upstream?) many use, this is not git's default configuration.

The current gitui behavior seems to be push.default=current, which is also not the default configuration.

Instead, git by default refuses such pushes in push.default=simple.

Would it be reasonable to check push.default before actually pushing?

@vlad-anger
Copy link

@naseschwarz Hey!

Would it be reasonable to check push.default before actually pushing?

From git-config(1):

push.default
Defines the action git push should take if no refspec is given

Key point is if no refspec is given

This issue & my PR fix is addressing the case, when refspec is configured,
but gitui fail to respect those configurations (branch.merge)

What you are referring to is a fallback scenario, when no refspec
configurations given, so we define which strategy we take - that's
what push.default stands for.

Arguing about best push.default default strategy [nothing, current,simple, ...]
should be outside of current issue

@naseschwarz
Copy link
Contributor

naseschwarz commented Mar 15, 2025

I don't think branch.*.merge configures a refspec for pushing. For example:

+ git init --bare remo
Initialized empty Git repository in /home/ja/scratch/remo/
+ git clone remo local
Cloning into 'local'...
warning: You appear to have cloned an empty repository.
done.
+ cd local
+ git branch -m b1
+ touch f1
+ git add f1
+ git commit f1 -m f1
[b1 (root-commit) 2e160d0] f1
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 f1
+ git push origin b1
Enumerating objects: 3, done.
Counting objects: 100% (3/3), done.
Writing objects: 100% (3/3), 199 bytes | 199.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
To /home/ja/scratch/remo
 * [new branch]      b1 -> b1
+ git checkout origin/b1 -b b2
branch 'b2' set up to track 'origin/b1'.
Switched to a new branch 'b2'
+ git config --get branch.b2.merge
refs/heads/b1
+ touch f2
+ git add f2
+ git commit f2 -m f2
[b2 5663ceb] f2
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 f2
+ git -c push.default=simple push origin b2
Enumerating objects: 3, done.
Counting objects: 100% (3/3), done.
Delta compression using up to 16 threads
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 223 bytes | 223.00 KiB/s, done.
Total 2 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
To /home/ja/scratch/remo
 * [new branch]      b2 -> b2

Even though b2 is set up to track origin/b1 via branch.b2.merge, git pushes to origin/b2 in the default configuration.

@vlad-anger
Copy link

vlad-anger commented Mar 15, 2025

Even though b2 is set up to track origin/b1 via branch.b2.merge, git pushes to origin/b2 in the default configuration.

Because you explicitly specified push.default=simple
But i see that it's a default option, so i can agree that checking push.default together with branch.*.merge
make sence

I don't think branch.*.merge configures a refspec for pushing

Do push with push.default=upstream, which is
used most of the time, then branch.*.merge config will
influence pushing to configured remote refspec

@vlad-anger
Copy link

vlad-anger commented Mar 15, 2025

To clarify:

I didn't checked that push.default strategy actually will
be prioritized over branch.*.merge refspec for pushing

Considering that i would say logic should be:
Check push.default config, if it's
[current,simple,matching?] - we don't consider branch.*.merge
[nothing,upstream] - we consider branch.*.merge refspec
@naseschwarz thoughts?

@naseschwarz
Copy link
Contributor

naseschwarz commented Mar 15, 2025

I mostly agree, except: As git defaults to push.default=simple if push.default is unset, only push.default=upstream should consider branch.*.merge if we want to keep gitui behavior consistent with git.

Ultimately, this is not up to me, though. I'm new here. ;)

@vlad-anger
Copy link

this is not up to me, though. I'm new here. ;)

You are totally right with push.default impact.
It was my lack of knowledge actually.
i'll be adjusting PR corresponding to that!

So thank you a lot for heads up :)

@extrawurst extrawurst added this to the v0.28 milestone Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants