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

Add a test that reproduces the reported issue to #17399 #17777

Merged
merged 4 commits into from
Aug 3, 2016

Conversation

tkelman
Copy link
Contributor

@tkelman tkelman commented Aug 3, 2016

@wildart I got impatient, please review. The first few times I tried, LibGit2.add! did not seem to work, but now it's apparently okay.

The first 2 commits here are the same as #17399, just rebased. The third commit adds a test that reproduces the actual issue, meaning it fails if you run the new test without your changes to base here.

@tkelman tkelman added libgit2 The libgit2 library or the LibGit2 stdlib module kind:bugfix This change fixes an existing bug labels Aug 3, 2016
@kshyatt kshyatt added the test This change adds or pertains to unit tests label Aug 3, 2016
@wildart
Copy link
Member

wildart commented Aug 3, 2016

Commit is done into the checked out repo Example3, but it should be to the remote branch of Example.

But mine is better 😉, I just pushed #17399.

@tkelman
Copy link
Contributor Author

tkelman commented Aug 3, 2016

Commit is done into the checked out repo Example3, but it should be to the remote branch of Example.

No, the issue is a WARNING: Cannot perform fast-forward merge. because the local commits on the pre-checkout package conflict with the checked-out branch.

@tkelman
Copy link
Contributor Author

tkelman commented Aug 3, 2016

Let's just do both.

@wildart
Copy link
Member

wildart commented Aug 3, 2016

If it can't fast-forward merge, it's a trouble. The branch is going to be created but never positioned correctly.

@tkelman
Copy link
Contributor Author

tkelman commented Aug 3, 2016

It really shouldn't be doing a merge at all, we asked it to do a checkout.

@wildart
Copy link
Member

wildart commented Aug 3, 2016

Problem is that git checkout is not just checkout particular branch or commit. It's much more. And, when you checkout a remote branch, you want to FF merge changes on that branch.

@tkelman
Copy link
Contributor Author

tkelman commented Aug 3, 2016

I'm going to merge this one, since it tests more scenarios.

@tkelman tkelman merged commit 16ad3a4 into master Aug 3, 2016
@tkelman tkelman deleted the tk/addbranchtest branch August 3, 2016 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bugfix This change fixes an existing bug libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants