5893: Remove "disable repo browsing with git2" stuff.

git2 is no longer unreliable, now that we know the silent failures
meant "git credential helpers must consume their stdin".

diff --git a/apps/workbench/app/controllers/repositories_controller.rb b/apps/workbench/app/controllers/repositories_controller.rb
index 89dd96bc6e4264fe2b258583adb1c1ca1708d67a..c5b3501b328e1214cc00c292c19f61d0be07312f 100644 (file)
--- a/apps/workbench/app/controllers/repositories_controller.rb
+++ b/apps/workbench/app/controllers/repositories_controller.rb
@@ -1,8 +1,5 @@
 class RepositoriesController < ApplicationController
   before_filter :set_share_links, if: -> { defined? @object }
-  if Repository.disable_repository_browsing?
-    before_filter :render_browsing_disabled, only: [:show_tree, :show_blob, :show_commit]
-  end
 
   def index_pane_list
     %w(recent help)
@@ -35,10 +32,4 @@ class RepositoriesController < ApplicationController
   def show_commit
     @commit = params[:commit]
   end
-
-  protected
-
-  def render_browsing_disabled
-    render_not_found ActionController::RoutingError.new("Repository browsing features disabled")
-  end
 end

diff --git a/apps/workbench/app/models/repository.rb b/apps/workbench/app/models/repository.rb
index 6aa6bb718ee0d27f0f367d29863691fef5c35df3..1caab89cc70fe643dc37965fcde5561ce0096f70 100644 (file)
--- a/apps/workbench/app/models/repository.rb
+++ b/apps/workbench/app/models/repository.rb
@@ -48,15 +48,6 @@ class Repository < ArvadosBase
     subtree
   end
 
-  # git 2.1.4 does not use credential helpers reliably, see #5416
-  def self.disable_repository_browsing?
-    return false if Rails.configuration.use_git2_despite_bug_risk
-    if @buggy_git_version.nil?
-      @buggy_git_version = /git version 2/ =~ `git version`
-    end
-    @buggy_git_version
-  end
-
   # http_fetch_url returns the first http:// or https:// url (if any)
   # in the api response's clone_urls attribute.
   def http_fetch_url

diff --git a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
index 80210c43600bdccc1c2c3d9343bf998760f60e1b..2ab8da1f4ce2d258d8f098c5c78175d6fbcdf713 100644 (file)
--- a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
@@ -99,8 +99,7 @@
               <% # link to repo tree/file only if the repo is readable
                  # and the commit is a sha1...
                  repo =
-                 (not Repository.disable_repository_browsing? and
-                 /^[0-9a-f]{40}$/ =~ current_component[:script_version] and
+                 (/^[0-9a-f]{40}$/ =~ current_component[:script_version] and
                  Repository.where(name: current_component[:repository]).first)
 
                  # ...and the api server provides an http:// or https:// url

diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index e28f76ae29f26e5f51a655c2f747cd270c9673d8..4061ee83d89ae83a0ef363e1e43571326ca870e6 100644 (file)
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -211,9 +211,3 @@ common:
 
   # Enable response payload compression in Arvados API requests.
   include_accept_encoding_header_in_api_requests: true
-
-  # Enable repository browsing even if git2 is installed. Repository
-  # browsing requires credential helpers, which do not work reliably
-  # as of git version 2.1.4. If you have git version 2.* and you want
-  # to use it anyway, change this to true.
-  use_git2_despite_bug_risk: false

diff --git a/apps/workbench/test/controllers/repositories_controller_test.rb b/apps/workbench/test/controllers/repositories_controller_test.rb
index 852a602d526ee70120b68bde1af1caf4f48b113d..25bf55768529f58327e7f8861835516e0b0f5f06 100644 (file)
--- a/apps/workbench/test/controllers/repositories_controller_test.rb
+++ b/apps/workbench/test/controllers/repositories_controller_test.rb
@@ -69,7 +69,6 @@ class RepositoriesControllerTest < ActionController::TestCase
 
   [:active, :spectator].each do |user|
     test "show tree to #{user}" do
-      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, _ = stub_repo_content
       get :show_tree, {
@@ -86,7 +85,6 @@ class RepositoriesControllerTest < ActionController::TestCase
     end
 
     test "show commit to #{user}" do
-      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, commit, _ = stub_repo_content
       get :show_commit, {
@@ -98,7 +96,6 @@ class RepositoriesControllerTest < ActionController::TestCase
     end
 
     test "show blob to #{user}" do
-      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, filedata = stub_repo_content filename: 'COPYING'
       get :show_blob, {
@@ -113,7 +110,6 @@ class RepositoriesControllerTest < ActionController::TestCase
 
   ['', '/'].each do |path|
     test "show tree with path '#{path}'" do
-      skip "git2 is unreliable" if Repository.disable_repository_browsing?
       reset_api_fixtures_after_test false
       sha1, _, _ = stub_repo_content filename: 'COPYING'
       get :show_tree, {

diff --git a/apps/workbench/test/integration/repositories_browse_test.rb b/apps/workbench/test/integration/repositories_browse_test.rb
index d936877ae28a96ab530a9ad6df8e845e4480a2b7..a6a85b5e4db04705604df37c0a1ffd7e24c28e85 100644 (file)
--- a/apps/workbench/test/integration/repositories_browse_test.rb
+++ b/apps/workbench/test/integration/repositories_browse_test.rb
@@ -13,7 +13,6 @@ class RepositoriesTest < ActionDispatch::IntegrationTest
   end
 
   test "browse repository from jobs#show" do
-    skip "git2 is unreliable" if Repository.disable_repository_browsing?
     sha1 = api_fixture('jobs')['running']['script_version']
     _, fakecommit, fakefile =
       stub_repo_content sha1: sha1, filename: 'crunch_scripts/hash'
@@ -37,7 +36,6 @@ class RepositoriesTest < ActionDispatch::IntegrationTest
   end
 
   test "browse using arv-git-http" do
-    skip "git2 is unreliable" if Repository.disable_repository_browsing?
     repo = api_fixture('repositories')['foo']
     portfile =
       File.expand_path('../../../../../tmp/arv-git-httpd-ssl.port', __FILE__)