ros-infrastructure / rosindex

The source code for generating index.ros.org
https://index.ros.org
GNU General Public License v3.0
12 stars 24 forks source link

Why does ros2/navigation show up as being in rolling? #327

Closed mikeferguson closed 7 months ago

mikeferguson commented 10 months ago

This repo - https://index.ros.org/r/navigation/github-ros2-navigation/ - is only released in the Ardent/Bouncy rosdistro, but still shows up in the index.ros.org as being part of rolling? That seems like a bug in the indexer...

tfoote commented 10 months ago

That does look like a bug. I think that there might be an assumption that rolling is always the latest. Or else there's some caching going on. I'm not sure where it's getting the branch name from.

 -- Adding repo navigation instance: github-ros2-navigation from uri: https://github.com/ros2/navigation.git with version: bouncy
Updating repo / instance navigation / github-ros2-navigation from uri: https://github.com/ros2/navigation.git
[47.86%] Scraping github-ros2-navigation...
 --- scraping version for navigation instance: github-ros2-navigation distro: rolling
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
 --- scraping version for navigation instance: github-ros2-navigation distro: ardent
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
 --- scraping version for navigation instance: github-ros2-navigation distro: bouncy
 --- scraping version for navigation instance: github-ros2-navigation distro: jade
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
 --- scraping version for navigation instance: github-ros2-navigation distro: indigo
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
 --- scraping version for navigation instance: github-ros2-navigation distro: hydro
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated
 --- scraping version for navigation instance: github-ros2-navigation distro: kinetic
/home/jenkins-agent/.bundle/ruby/2.7.0/gems/rugged-0.27.7/lib/rugged/repository.rb:42: warning: Using the last argument as keyword parameters is deprecated

I suspect that is something to do with the scraper finding rolling in the branch name or some other heuristic to decide that branch is associated.

The individual packages also are improperly tagged: https://index.ros.org/p/dwa_local_planner/github-ros2-navigation/#rolling

rkent commented 7 months ago

This happens because the repo https://github.com/ros2/navigation has a branch 'always_update_rolling'. rosindex falls back to implicitly look for valid branches if there exists a branch name containing the distro name. That is done in vcs.rb:

      # save the branch as the version if it matches either the explicit
      # version or the distro name
      if explicit_version
        if branch_name == explicit_version
          return branch, branch_name
        end
      elsif branch_name.include? distro
        return branch, branch_name
      end

There is no reason for this repo to be included in the index at all, as it has been archived and marked as obsolete since 2020.

rkent commented 7 months ago

The fix for this is fairly simple: stop searching any repo that ever existed in rosdistro for possible other valid distro, by looking for branch names containing that distro. I ran a full rosindex where I flagged where implicit versions are being created, and came up with 969 examples. I randomly investigated a few of these cases, here are my notes from that:

In none of these cases do I feel that a rosindex entry was appropriate. Should rosindex only reflect entries in rosdistro, or should it aggressively try to find possible versions based only on the existence of branch names? I think that clutter is a major problem in rosindex, so I think we should only show "official" entries from rosdistro. But my knowledge of ROS history and lore is somewhat limited, so I'd appreciate comments on this before I do a PR.

The PR could be fairly trivial (maybe only one line changed), but that would orphan a lot of code. I believe the orphaned code should be deleted with a longer PR.

mikeferguson commented 7 months ago

Should rosindex only reflect entries in rosdistro, or should it aggressively try to find possible versions based only on the existence of branch names

Honestly, if it isn't even in rosdistro, how well supported could it be? @tfoote might have more thoughts on this, but I think it's probably a disservice to ROS users to include things that are outside the rosdistro.

tfoote commented 7 months ago

Ahh, @rkent thanks for digging into that! The original vision of rosindex (https://github.com/rosindex/rosindex) was to index not only the mainline versions of packages but also all the branches and forks. It's a great idea, but we've been slowly moving back from that vision and simplifying it to be the one official version in the index. So with that stated I think that removing the implicit branch matching is the right choice for the new behavior. We should both turn that off as well as eliminating the orphaned code at the same time. As can be seen in these incorrect matches the heuristics are getting the results wrong and misleading people.

To that end a PR to disable the fuzzy matching and any extra logic which can be moderately easily confirmed as unused would be great.

rkent commented 7 months ago

So @tfoote what do I need to do to make progress on the PR that I opened? I don't seem to be able to request review. Do I just need to wait, or is there some step I have not done?

tfoote commented 7 months ago

Thanks for the ping. I missed the notification for that.

tfoote commented 7 months ago

This is resolved in #352

https://index.ros.org/p/navigation/#rolling

image