Walker returns incorrect results after upgrade from 0.25.0b9 to 0.25.0b10

[adamniedzielski]

This is probably related to (#663 /cc @cfillion).

Scenario:

Given a repository:

* commit C (HEAD)
|
* commit B
|
* commit A

When we do:

walker.push sha_of_a
walker.hide sha_of_c

Walker returns 1 commit (commit A) instead of 0 commits. This happens only when using Rugged::SORT_DATE.

Script to reproduce

gem 'rugged', "= #{ENV['RUGGED_VERSION']}"
require 'rugged'
require 'tmpdir'

Class.new do
  def initialize
    path = Dir.mktmpdir 'test-repository'
    @repo = Rugged::Repository.init_at path
    @repo.config['user.name'] = 'John Doe'
    @repo.config['user.email'] = 'john@doe.com'

    @hash_a = create_commit
    sleep 1
    @hash_b = create_commit
    sleep 1
    @hash_c = create_commit

    puts Rugged::VERSION
    compare
  end

  def create_commit
    Rugged::Commit.create @repo,
      message: 'Hello World',
      parents: @repo.empty? ? [] : [@repo.head.target].compact,
      tree: @repo.index.write_tree(@repo),
      update_ref: 'HEAD'
  end

  def compare
    walker = Rugged::Walker.new @repo
    walker.sorting(Rugged::SORT_DATE)

    walker.push @hash_a
    walker.hide @hash_c

    commits = walker.to_a
    walker.reset

    puts commits.size
  end
end.new

> RUGGED_VERSION=0.25.0b9 ruby rugged.rb
0.25.0b9
0
> RUGGED_VERSION=0.25.0b10 ruby rugged.rb
0.25.0b10
1

[adamniedzielski]

Some remarks:

1. It is specific to Rugged::SORT_DATE, if we don't specify this option it will return 0 commits. This suggests that it's a regression not a change of behavior, since sort order should not influence the number of returned items.
2. When we first call Walker#hide and then Walker#push (as suggested in Crash when using Walker#hide after Walker#push and SORT_TOPO since v0.25.0 #663 (comment)) it will return 0 commits. This also suggests that it's a regression, because the order of these two calls should not matter.
3. There is one potential culprit between 0.25.0b9 and 0.25.0b10 - 2e9db5c. It includes significant changes to src/revwalk.c.

[vmg]

This sounds like a legit regression in libgit2. Could younplease open an issue on the actual repository (libgit2/libgit2) so they can fix it? We'll backport the fixed code. Thanks!

[vmg]

I believe this should be fixed in #677 !

Can you please verify for your use case? Thanks!

[adamniedzielski]

@vmg No, unfortunately this is still not fixed in rugged 0.26.0b1.

RUGGED_VERSION=0.26.0b1 ruby rugged.rb
0.26.0b1
1

I rebased my failing test (libgit2/libgit2#4100) against the current master of libgit2 and it is still failing.

(but thanks for libgit2/libgit2#4105 anyway 😄)