search

jyknight / llvm-git-migration

7 stars 6 forks source link

len(candidate_upstreams) != 1 #4

Open bjope opened 5 years ago

bjope commented 5 years ago

https://github.com/jyknight/llvm-git-migration/blob/e6a6e0a4818a0116d3cd6cfa72f6bdad4100cc1e/migrate-downstream-fork.py#L130

When testing this script on our out-of-tree llvm clone I ended up triggering this exception.

Maybe this should be a warning instead, and then the heuristic could be to just continue (as the code below is using "candidate_upstreams[0]"? Or do you think that I am in deep trouble if doing it like that?

jyknight commented 5 years ago

I doubt that'd be a correct assumption.

As the comment above this code says, you likely want to pull in the more recent content, but going for candidate_upstreams[0], I think, is not usually going to give you that.

A way to input a manual mapping ("given a merge between commit X and Y, use the tree only from X.") would be the most general fix I think.

bjope commented 5 years ago

I only end up at the TODO 16 times (if changing the exception to a warning), so maybe I can hard code something for those situations in a local copy of the script. It seems to involve some ancient commits so I just need to figure out how they are related to each other.

I'm not sure how many users that will hit this exception. Maybe it is a waste of time adding some kind of general support if this is a very rare problem.

bigboze commented 5 years ago

I've come across the same issue, for really old merge commits that merge from different release branches. My current workaround is to pick the candidate with the largest svn revision, but not sure if this is sufficient?