Why does the get_new_commits method of git.py always have no data returned?

[sunwuyan]

Hi，
I found the get_new_commits method of git.py always have no data returned! Why? Doesn't seem to work？Is there an error in the options provided to the git rev-list command？

[kofrezo]

Hi， I found the get_new_commits method of git.py always have no data returned! Why? Doesn't seem to work？Is there an error in the options provided to the git rev-list command？

Hi @sunwuyan,

Unfortunately I am not too used to the code as I took it over from a colleauge but at a first glance this looks like relic to me.

The get_new_commits method of the Commit class you mentioned does indeed always return an empty list because of the rev-list command in its current form.

def get_new_commits(self):
      """Get the list of parent new commits in order"""
      output = check_output([
          git_exe_path,
          'rev-list',
          self.commit_id,
          '--not',
          '--all',
          '--reverse',
      ]).decode('utf-8')
      for commit_id in output.splitlines():
          yield Commit(commit_id)

https://github.com/innogames/igcommit/blob/master/igcommit/git.py#L51-L62

However the commit_list is appended by the commit itself after calling this method:

if ref_path_split[1] == 'heads':
    commit_list = CommitList(commit.get_new_commits(), name)
elif ref_path_split[1] == 'tags':
    commit_list = CommitList([commit], name)

# Appending the actual commit on the list to the new ones makes
# testing easier.
if commit not in commit_list:
    commit_list.append(commit)

https://github.com/innogames/igcommit/blob/master/igcommit/prereceive.py#L90,L98

And since the input of the prereceive hook includes all new commits, igcommit processes them:

def expand_checks(checks):    
    checked_commit_ids = set()
    for line in input():
        for check in expand_checks_to_input(checks, line, checked_commit_ids):
            yield check

https://github.com/innogames/igcommit/blob/master/igcommit/prereceive.py#L72

See also Testing for an example input for igcommit.

To summarise: It works because we get each new commit from the input and create a list of commit_ids with nothing but the commit itself.

I can't say what the intention of the method is you mentioned (yet) but it looks indeed obsolete to me.

[sunwuyan]

Thank you for your reply, I just think this piece of code is weird, and it is actually useless, so I will ignore it for the time being! And I will also close this question!

[kofrezo]

It is indeed. I will double check when I find the time if I can find any use of it I did not see yet or otherwise remove it.

Thanks for pointing this out!

[hasegeli]

You are right that this code is useless when the script is tested using the command on the README. It is feeding in all commit ids to be tested.

However, when the script is used actually as a Git pre-receive hook, this code does necessary job to find the commits to be tested. Only one id for each branch pushed is feed by Git to the pre-receive hook. We need to find all of the new commits on these branches to test them one by one.

rev-list is one of those Git plumbing commands with confusing arguments. Running it with these arguments in the Git workspace at pre-receive time does exactly what we need. --not together with --all excludes all commits in the workspace. The new commits are not received yet, so we get them.

[kofrezo]

That makes sense. Thanks for explaining @hasegeli. I'll add an explanatory comment.

CC: @sunwuyan