Wrong data for PR merges provided from GitHub

[GustavoVale]

I am trying to get the commits of pull-request 4 (https://github.com/GustavoVale/Merge_Conflict_Scenarios/pull/4/commits), but instead of get two commits (46b7d34 and a40dbf9) I am getting a list with thousands of commits (6817 to be precise, but most of them are repeated). I guess that it is happening because this is the first closed pull-request of the project and it is getting a wrong "base" commit. The correct base commit is (4076cc11574f99ab067470b02f0d6cda24a0cb30) the third commit of the returned list.

[GustavoVale]

@fheck Do you have some idea?

[GustavoVale]

With the following line I can get the commits I want "wget -O - -q https://api.github.com/repos/_:user_/_:repository_/pulls/_:numberPR_/commits | grep sha"

[GustavoVale]

Hi @fheck your changes did not solve the problem.

In line 176 (} while (c != null && ! c.equals(base));) I had to add "!" because I got a loop.

I am interested in get the commits on GitHub. Using the GitHub API (command in a previous comment) I am able to get the right commits. Could you insert a command to get it? I think this is the easiest way.

[xai]

Hi guys!

I remember something about the commits returned by the API need to be filtered.
Like, there is a kind of hierarchy and @GustavoVale needs only the top-level ones.

The grep from #6 (comment) prints all commits instead. Can you verify this, @GustavoVale?

[GustavoVale]

Hi,

To be honest, I did not get what exactly that method should get because in some cases it got commits that are not part of the merge. I guess getMergeBase() is not getting the right base. For instance, in the project https://github.com/GustavoVale/Merge_Conflict_Scenarios almost always the merge base is the commit a40dbf9b9c5b65c32074d3b0d131dd811c619c5a.

[fheck]

I looked into it some more, and I get consistent behavior with the current version 3bffcb9 (and since I did not touch any related code since 05027d1 this still seems as the likely culprit for the original bug to me) .

For example, if I run

List<Optional<List<Commit>>> prcommits = repo.getPullRequests(State.ANY).get().stream().map(PullRequest::getCommits).collect(Collectors.toList())

on your example repo, I get

prcommits = {java.util.ArrayList@2149}  size = 6
0 = {java.util.Optional@2153} "Optional[[1601adc1d57e24641b032311e39e2009ba09fde1]]"
1 = {java.util.Optional@2154} "Optional[[c03ac19b42f2d6054c1de53ee3b4c78fb60f7425]]"
2 = {java.util.Optional@2155} "Optional[[2511c7816fa183b44f6bf506e7bac936000f43cf]]"
3 = {java.util.Optional@2156} "Optional[[0b68a2c0719dbb4ea235adc1e52804d8cd051351]]"
4 = {java.util.Optional@2157} "Optional[[fec4a7878e4c33a90a0462c0662151b7eae38c5d]]"
5 = {java.util.Optional@2158} "Optional[[a40dbf9b9c5b65c32074d3b0d131dd811c619c5a, 46b7d342cc4c18d679cce16703064ee02acbfd0d]]"

which is, I believe after looking at the PR page for the repo, exactly what is supposed to be returned.

If this still does not work for you, I can support an additional method providing the data directly from GitHub. but I would prefer the local version.

[GustavoVale]

I had made many tests in this commit you mentioned (05027d1) and it did not work, but now it is working on that project.

Just to be sure, could you test with cotonet project (https://github.com/riselabs-ufba/cotonet). I am getting a different result than the expected. The expected result is:

0 = {java.util.Optional@2153} "Optional[[1d8d85617ea0657b13f98719c6275c94e1f497ed, 468deabd7bb45b2dd56d22104e1980d1d04cc173, ca30c02586827ea3d5131ffb96e083dc49d7f1ca, e49696e6d784e8038d74ee97db3761f7f2e365ed, 3df81622365ddf36666400b0d2a7ce7723ef7095, 6df8b64fe5e81bdebe5cf06745a4787cb605d601, 8558b7079bbcb3ef5e6889b1a9febc3db95cd110, 13d75e2c6fb2df1bf8e4fc9da602ec6d2f59a418, 690402f32feaa410371f616ba83dc1f8372f9411]]"
1 = {java.util.Optional@2154} "Optional[[7aef670ff851cc6c6e5ce81272837ead3f2a221c, 419b2b7a66c741226538d6194d5d4b60520d15c0]]"
2 = {java.util.Optional@2155} "Optional[[40507ea0897d58917efa4dd7575f0a659a0143a0, e2ac51a5ff62855177125b7701af8859f1fa78a2]]"
3 = {java.util.Optional@2156} "Optional[[e69bf7d8dec6aa70fde02a23b209d3edb6ab1375, 833347733957783dc546a9ddf61548c42905b9d8, 1a7c4e2bffac9b6e368943463943104e0b1b5f8c, 57d309e3898238845861a8341bdfc8eca910e9a5, b593ebe5b142bd807164d1b49d130e49599ab1d0, 5fb00c797f9e1851e05b642b47406a43afe89170, adfc4177ce60ed38bdaa6234ecc0b550a2347610, 995656243931fce8ca46ade383063c79601c9357, 174dcba5998b53cf50a5891d487b3207c2669c92, 22f6ce834f41df2b396e69e100cb0391f70f0a86, 0b1be907f12f89ca450b7cf2fa98218abc92a316]]"
4 = {java.util.Optional@2157} "Optional[[7b12b635468852f77e63063a667d473127a7cad6, 1c683de937971810f997bfbdc870bf25a14f33b3, d5541784fe05756f32b572a70d132e20335dc123, 61f59c1a36aaec245951c0fd3093469018d7d040]]"
5 = {java.util.Optional@2158} "Optional[[ e69bf7d8dec6aa70fde02a23b209d3edb6ab1375, 170049a52192f7b7216894b5c2787e9aa06c9355, 4b314c4897a5e551aebcaa081c17321aa201e960, 5f1acc1c2cbf1514c02321fac6262114fa795eda, 6e6e2fa728b9a945ee6ac8505d8db28954993c35, 9b3e79343ce67547911427eac280d5ba5f48bbd3, d778f66adc3de9d3c57079ef83563f7aa3cd679b, 5e35611c3ed55a6d2e18fff56b29eada6e67e497]]"

The differences are in the position 3 and 4. The result I got miss commits in position 3 (I am getting just 1 commit) and has much more commits in position 4 (to be exactly 1026 commits, it should be just 4 commits). I mean position as the position in the array presented, but these positions refer to pull-requests 2 and 3.

[GustavoVale]

@fheck Did you see my email?

[fheck]

Ok, @GustavoVale, your example did reveal another problem, which is - I think - the actual root of the bug.
(Side note: I have implement the alternative data source for the commit list, which I will publish, once I have tested it fully.)

From a git graph perspective, the data provided by GitHub is wrong.

So I would like additional input, as - from my point of view - this bug may introduce latent faulty data. (@xai)

I'm using cotonet issue 3 as my example.

Looking at the data provided by GitHub, the head of the PR is 0b1be90, the merge commit (merge_commit_sha, which should hold the actual merge commits sha1 after the merge) is also at 0b1be90, but should be at 148e43a according to my local copy of the repo.

Now, basing my assumptions about anything relating to the internal git graph structure on this wrong merge, obviously leads to faulty data. In this case, the discrepancy can be mitigated, by only using the commit list provided by GitHub, but this might - even in this case (deleted branches come to mind and I have already seen other inconsistent behavior) - be not enough and will ultimately provide wrong data to the user of the library (PullRequest#getMerge()).

I can try to reconstruct the correct merge commit id from looking at the graph, but this is at least partially guessing. On the other hand, I do not understand, why GitHub behaves this way, did I misunderstand something or is this a special case, which I have to account for?

[xai]

Thanks for summarizing this situation in detail!
This is ... interesting indeed. I'll look into this tomorrow. (Copying @GSeibt here as well)