Skip to content

Report UpstreamGone. #20

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

Merged
merged 4 commits into from
Jan 17, 2017
Merged

Report UpstreamGone. #20

merged 4 commits into from
Jan 17, 2017

Conversation

cmarcusreid
Copy link
Owner

Report UpstreamGone. Resolves #19.

UpstreamGone is set when a branch has no remote tracking reference but still has an upstream configured. This is similar to the git warning:

Your branch is based on 'origin/master', but the upstream is gone.
(use "git branch --unset-upstream" to fixup)

UpstreamGone is set when a branch has no remote tracking reference but still has an upstream configured. This is similar to the git warning:

Your branch is based on 'origin/master', but the upstream is gone.
  (use "git branch --unset-upstream" to fixup)
@cmarcusreid
Copy link
Owner Author

Reading side for posh-git is cmarcusreid/posh-git@4fa6c8c. I'll submit a PR there once this is merged and the new executable is published.

@DoCode
Copy link

DoCode commented Jan 15, 2017

@cmarcusreid, can you please update libgit to the latest release?

@cmarcusreid
Copy link
Owner Author

@DoCode I've previously had some functionality issues when upgrading libgit2, so I'd like to do that separately from this change. If you could share additional details in how to setup for your scenario in #18 that'd make it easier to validate the LFS bits. Of course, please also feel free to submit a PR if you want to make the change.

Copy link

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up so quickly!

@@ -270,6 +271,20 @@ bool Git::GetRefStatus(Git::Status& status, UniqueGitRepository& repository)
Log("Git.GetRefStatus.NoUpstream", Severity::Spam)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could argue that this should only be logged if UpstreamGone condition isn't met.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. I think I like it logged with the current behavior since we don't populate Upstream in this case. If we change that behavior to report the old Upstream details, I think your suggested adjustment makes sense.

@@ -270,6 +271,20 @@ bool Git::GetRefStatus(Git::Status& status, UniqueGitRepository& repository)
Log("Git.GetRefStatus.NoUpstream", Severity::Spam)
<< R"(Branch does not have a remote tracking reference. { "repositoryPath": ")" << status.RepositoryPath
<< R"(", "localBranch": ")" << status.Branch << R"(" })";

auto upstreamName = git_buf{ 0 };
result = git_branch_upstream_remote(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading correctly, this only checks branch.{name}.remote. Git doesn't consider an upstream to be valid without branch.{name}.merge also being set.

image

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. I'll tweak to check both. At that point I think we'll have all the information to populate the Upstream even if it's gone, so I'll adjust the log statement and I think I can revert the inversion on the posh-git change.

<< R"(Branch does not have a remote tracking reference. { "repositoryPath": ")" << status.RepositoryPath
<< R"(", "localBranch": ")" << status.Branch << R"(" })";
auto upstreamRemoteName = git_buf{ 0 };
auto upstreamRemoteResult = git_branch_upstream_remote(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this now - git_branch_upstream_name includes a remote check.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cmarcusreid cmarcusreid merged commit e1ea015 into master Jan 17, 2017
@cmarcusreid cmarcusreid deleted the upstreamGone branch January 17, 2017 00:23
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.

Upstream Gone
3 participants