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

Get default fetch remote from configuration #2204

Merged

Conversation

cruessler
Copy link
Contributor

@cruessler cruessler commented Apr 24, 2024

This PR is in response to #1093. It changes how GitUI gets the remote to fetch from.

This is a follow-up PR to #2156 which added similar behaviour for pushing.

It changes the following:

  • It adds get_default_remote_for_fetch_in_repo which reads git configuration related to remote repositories. It first reads branch.<name>.remote. This is according to the docs.

I followed the checklist:

  • I added unittests
  • I ran make check without errors
  • I tested the overall application
  • I added an appropriate item to the changelog

Test fetch/pull behavior in status tab:

  • no upstream branch configured
    If no upstream branch is configured, you still can’t fetch from a remote if you have renamed origin to something else, e. g. non-origin. I think this is the right thing to do for the time being. As far as I’m aware, we want to provide a UI allowing users to choose a branch to fetch from in the future. man git-fetch says: “When no remote is specified, by default the origin remote will be used, unless there’s an upstream branch configured for the current branch.”
  • upstream branch configured, branch.<>.remote is set

@cruessler cruessler force-pushed the fetch-from-configured-default-remote branch from 9c06fa8 to 5f2ff11 Compare April 29, 2024 16:28
@cruessler cruessler force-pushed the fetch-from-configured-default-remote branch from 5f2ff11 to 9ebdddd Compare May 1, 2024 09:55
@cruessler cruessler marked this pull request as ready for review May 1, 2024 09:55
@cruessler
Copy link
Contributor Author

@extrawurst This PR is now ready for review. It is hopefully a small step towards making the error message git: inconclusive remotes disappear. 😄

Once it is merged, GitUI should respect configured remote branches for fetching and pushing. We could then start implementing a fallback for when no remote branch is configured. I’m also thinking about creating a couple good first issues in order to deduplicate/clean up some of the new code.

@extrawurst
Copy link
Owner

Awesome!! I am going to review it tomorrow !

@extrawurst
Copy link
Owner

does this fix #1093 now?

@extrawurst
Copy link
Owner

lgtm. just one thing:

I’m also thinking about creating a couple good first issues in order to deduplicate/clean up some of the new code.

can you mark the places with //TODO: <description> ?

@extrawurst
Copy link
Owner

@cruessler

does this fix #1093 now?

if so, lets mark that in the changelog

@cruessler
Copy link
Contributor Author

@cruessler

does this fix #1093 now?

if so, lets mark that in the changelog

I’ll have a look tomorrow on the train!

@cruessler
Copy link
Contributor Author

@cruessler

does this fix #1093 now?

if so, lets mark that in the changelog

As far as I can tell, this fixes #1093. I’ve updated the changelog entry. This is how I prepared the repository before I tested the branch:

❯ git remote rename origin non-origin

❯ git remote
non-origin
upstream

❯ git push non-origin
[…]
 * [new branch]      test-remotes -> test-remotes
branch 'test-remotes' set up to track 'non-origin/test-remotes'.

❯ git remote set-url non-origin git@github.com:cruessler/gitui.git

❯ git config --list | grep non-origin
remote.non-origin.url=git@github.com:cruessler/gitui.git
remote.non-origin.fetch=+refs/heads/*:refs/remotes/non-origin/*
branch.test-remotes.remote=non-origin

I was able to successfully pull in this setup. Since git automatically sets up branch.<>.remote, the only reason this does not fix the issue would be additional local configuration. Or something else I don’t know yet. 😄

@extrawurst extrawurst merged commit a92be3b into extrawurst:master May 16, 2024
18 checks passed
IndianBoy42 pushed a commit to IndianBoy42/gitui that referenced this pull request Jun 4, 2024
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.

2 participants