-
Notifications
You must be signed in to change notification settings - Fork 723
Improve branch point changeset detection #1017
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
Improve branch point changeset detection #1017
Conversation
Not sure why the tests failed...as I didn't author the original solution, and PR #973 was passing all of its checks, I'm unsure why these checks should be failing now. |
Yes. Bad news. We need to figure it out why... Perhaps I will be able to
find some time tomorrow...
|
I can't get the tests to run locally; I'm going to try and figure that out and also see if I can reproduce the issue as well. |
I can't get the tests to run locally
Which ones? The unit tests or the functional/smoke ones?
The smoke ones, that the ones that fails, are a powershell script in the
"functionalTesting" folder.
You should not have difficulties to run it, except that you should call it
from a folder outside a git repository.
|
Ah, I was talking unit and integration tests; but you are right, I want to look into the functional tests. Thanks. |
OK, good news, I think: the functional tests passed when I ran them locally. Could this be something temporal on the AppVeyor CI box?? Is there a way to force this to run again up on AppVeyor? |
Also, should I have updated |
No. In fact the tests are not passing. I reproduce the problem. (Perhaps you forgot to pass the directory path toward the new build exe --or update the script if you found it easier-- and it used the one in your path?) In file TfsHelper.Common.cs, there is 2 call to
yes, do it. Update the line to point to the 2 pull request (the yours first) and the 2 user ids. |
rootChangesetMergeInfo.TargetChangeset : rootChangesetMergeInfo.SourceChangeset; | ||
|
||
var rootBranch = new RootBranch(rootChangesetInParentBranch, rootChangesetInChildBranch, tfsPathBranchToCreate); | ||
AddNewRootBranch(rootBranches, rootBranch); | ||
|
||
AddNewRootBranch(rootBranches, new RootBranch(rootChangesetInParentBranch, tfsPathBranchToCreate)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be removed...
Hmm, wonder why the tests passed locally for me? In any event, I'll update the PR as you indicated a bit later this evening and hopefully we'll be good to go. |
I have updated my message to imagine a reason but perhaps you didn't see
It's late for me, too... ;-) I will wait. No problem. |
@pmiossec Yes, I saw your revisions, and thank you for spotting that!! I totally missed it. What I was trying to communicate is that I ran the functional tests with the exact same code I pushed up and they ran to completion without any errors. So that's what's weird. But, I do see that the line you mentioned should in fact be removed. I'll be pushing up the changes right after I run the functional tests. Thanks again for the once-over!! 😄 |
HAHA, I figured out why my functional tests passed! I wasn't using the newly built git-tfs, but my locally installed copy. Oops!! That'll do it! |
@jeremy-sylvis-tmg is the original author of this commit. He originally issued Pull Request git-tfs#973. However, a code review was performed by myself and @pmiossec whereby changes were requested. Unfortunately, the author of the pull request declined to make the requested changes due to time constraints. Because the changes implemented in this commit could potentially solve a long outstanding problem in how git-tfs currently detects branch point changeset detection, I have recreated this commit with the requested changes. Basically, this commit allows gits-tfs to properly clone a TFS branch into a git repository when the branch was first created as a folder, deleted, and subsequently created as an actual TFS branch: *--C1--C2--C3--C4(X)--*--*--* \ C5(?)--*--* In the diagram above, a folder would be created at C3 called 'Branch'. At C4, it was determined that either C3 was a mistake, or simply that a true TFS branch was desired as opposed to a TFS folder. So at C4, the folder named 'Branch' is deleted. At C5, an actual TFS branch called 'Branch' is created, and C5 is the first changeset on TFS branch 'Branch'. Prior to this commit, Git-Tfs would get "confused" because branches and folder in TFS are similar, but not the same--and there's no analogue in Git whatsoever. The branch 'Branch' usually fails to clone properly, and IIRC, you'll get messages such as "Could not find parent changeset for branch 'Branch'". In my experience, providing a parent changeset ID does not always resolve the situation satisfactorily. The changes in this commit detect this situation. At C3, it detects that a folder was created, called 'Branch'. TFS folders are not branches, and so this commit causes git-tfs to keep searching through TFS history for the most recent branch point. So, when the folder named 'Branch' is deleted at C4, git-tfs will continue to search for merge history, which it finds at C5, where the TFS branch named 'Branch' is created. This is a true branch point off of the main line. Because C3 and C4 effectively "cancel" each other out, the resulting Git history after cloning this TFS repository history looks like the following (NOTE: this author is unsure how git-tfs represents commits C3 and C4 in the commit history of a Git repository since Git has no concept of "folders"): *--C1--C2--C3--C4(X)--*--* \ C5--*--* NOTE: This commit does add some methods to an interface. As such, the next version of Git-TFS should have it's minor version incremented; e.g. the next version of Git-Tfs released containing this commit should be in the 0.26.x series. Signed-off-by: Craig E. Shea <craig.e.shea@gmail.com>
e2549c2
to
334a040
Compare
thanks a lot @fourpastmidnight and @jeremy-sylvis-tmg |
This PR contains a single commit which re-implements the changes made in PR #973. The original author of #973 declined to make the requested changes due to time constraints.
Since the contribution of those changes could have a significant, positive impact on enabling Git-TFS to better clone TFS repositories into Git repositories, I have made the requested changes to PR #973 and am submitting this PR in its place.