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

Fix missing merge parent #628

Closed
wants to merge 2 commits into from

Conversation

daveh551
Copy link
Contributor

This pull request fixes issue #627 (and also #590,of which it is a duplicate).

In some cases (probably involving a badly formed TFS changeset, or a merge changeset in which the merge parent has been deleted), the VersionControl.QueryMerges call in FindMergeChangesetParent can return an empty array. In such cases, the subsequent merginfo.Max(x => x.SourceVersion) call will throw a "Sequence contains no objects" InvalidOperationException.

This patch avoids that error by checking for a zero-length array, and returning -1 to the caller, which then checks for that and handles it with a warning message that git-tfs can't locate the parent.

@coridrew
Copy link

Awesome!
On Jun 30, 2014 7:27 AM, "Dave Hanna" notifications@github.com wrote:

This pull request fixes issue #627
#627 (and also #590
#590 which it is a
duplicate).

In some cases (probably involving a badly formed TFS changeset, or a merge
changeset in which the merge parent has been deleted), the
VersionControl.QueryMerges call in FindMergeChangesetParent can return an
empty array. In such cases, the subsequent merginfo.Max(x =>
x.SourceVersion) call will throw a "Sequence contains no objects"
InvalidOperationException.

This patch avoids that error by checking for a zero-length array, and
returning -1 to the caller, which then checks for that and handles it with

a warning message that git-tfs can't locate the parent.

You can merge this Pull Request by running

git pull https://github.com/daveh551/git-tfs fix-missing-merge-parent

Or view, comment on, or merge it at:

#628
Commit Summary

  • Handle error case where no merge info is returned by QueryMerges
  • Change error return from FindMergeChangesetParent from 0 to -1

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#628.

@pmiossec
Copy link
Member

@daveh551 I have merged #480 into master that change a lot of things... Could you verify that pull request is still needed. If it is, could you rebase your work on top of master. This way, I will be able to merge it...

@daveh551
Copy link
Contributor Author

Philippe,
I will do that, hopefully within the next day or two. I’ll let you know.
Thanks for your efforts.

@@ -133,6 +133,7 @@ public virtual int FindMergeChangesetParent(string path, long targetChangeset, G
{
var targetVersion = new ChangesetVersionSpec((int)targetChangeset);
var mergeInfo = VersionControl.QueryMerges(null, null, path, targetVersion, targetVersion, targetVersion, RecursionType.Full);
if (mergeInfo.Length == 0) return -1;
Copy link

Choose a reason for hiding this comment

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

technically you should just return targetChangeset. the subsequent FindCommitByChangesetId will not find any and fall through accordingly. the real fix is avoiding the empty mergeInfo.Max(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I wasn’t sure how things worked and what should be returned, but returning an error indication and then testing for it seemed like a sure cure. But I like your method better.

@gfody
Copy link

gfody commented Jul 13, 2014

just built from master and this fix is still necessary to clone my repo. I think it could be reproduced by creating a changeset with a single file in it and then using tf destroy to erase that file.

@ghost
Copy link

ghost commented Oct 31, 2014

@gfody do you have a version of this fix that will work with current master?

@pmiossec
Copy link
Member

@voltagex I have rebased the branch onto master. You could find it here. Try it and tell me if it solve your problem...

@ghost
Copy link

ghost commented Nov 2, 2014

@pmiossec I've tried your patch and unfortunately I still get the "Sequence contains no elements" error at line 273 of Rcheckin.cs

@pmiossec
Copy link
Member

pmiossec commented Nov 3, 2014

@voltagex "Sequence contains no elements" error is a "generic" error that could occurs at different places. The one you've got is in no mean related to this issue!

I think I have fixed it with #671. Could you try it?

PS: But I think that if you encounter this crash, you are doing something at best a "strange" thing, but at least a wrong one :( I would like to warn you before go ahead... You are trying to rcheckin a commit that is a merge commit. You've got the crash because one of the parent branch is a "pure" git branch (with no "link" with tfs).
Perhaps, it's normal and what you wanted to do... but be aware that this branch won't be checked in tfs and that you will lost its history, if you do it again with my fix! Perhaps you would have wanted to check the branch in tfs before.
If it's not what you want to do, then, you're git workflow is perhaps not the best one....you perhaps should have rebased and/or squashed your work.

@ghost
Copy link

ghost commented Nov 3, 2014

I think I will have to open a new issue. What I'm doing works with git-tf
but not git-tfs

Check out old project from TFS server one -> check out new project from TFS
server two (empty folder) -> git pull first repo -> git tfs rcheckin

@pmiossec
Copy link
Member

pmiossec commented Nov 3, 2014

Git-tfs and git-tf are not working the same way, so it's possible that the
way to achieve what you want is not the same...

You are right, open a new issue and explain the more possible what you want
to do because I understand nothing to your last message :-(

@pmiossec
Copy link
Member

If someone is here because he want to test this fix, see #678 where the commit has been rebased (because this pull request seems not maintained :( )

@pmiossec
Copy link
Member

Similiar fix #678 merged in master.

@pmiossec pmiossec closed this Nov 21, 2014
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.

None yet

4 participants