[ARVADOS] created: 00121a241b60d5aca578e98c379e35b92e759c2b

git at public.curoverse.com git at public.curoverse.com
Sat May 2 02:37:28 EDT 2015


        at  00121a241b60d5aca578e98c379e35b92e759c2b (commit)


commit 00121a241b60d5aca578e98c379e35b92e759c2b
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat May 2 02:34:03 2015 -0400

    5416: 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 89dd96b..c5b3501 100644
--- 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 6aa6bb7..1caab89 100644
--- 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 80210c4..2ab8da1 100644
--- 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 e28f76a..4061ee8 100644
--- 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 852a602..25bf557 100644
--- 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 d936877..a6a85b5 100644
--- 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__)
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index a86de69..e3d574e 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -3,6 +3,7 @@
 --
 
 SET statement_timeout = 0;
+SET lock_timeout = 0;
 SET client_encoding = 'UTF8';
 SET standard_conforming_strings = on;
 SET check_function_bodies = false;

commit 0f1d3aca63546ecbb4ad1d05ec7955258ea3bd44
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat May 2 02:16:19 2015 -0400

    5416: Use git credential helpers for arv-git-httpd tests. Fix workbench helper.
    
    The sporadic "git exited 128" errors -- very common in git2 and rare
    in git1 -- seem to have been caused by git getting SIGPIPE when the
    credential helper exited without consuming stdin. The solution is for
    the credential helper to discard its standard input before exiting.

diff --git a/apps/workbench/app/models/repository.rb b/apps/workbench/app/models/repository.rb
index b9b9ce3..6aa6bb7 100644
--- a/apps/workbench/app/models/repository.rb
+++ b/apps/workbench/app/models/repository.rb
@@ -96,7 +96,7 @@ class Repository < ArvadosBase
       "credential.#{http_fetch_url}.username", 'none'],
      ['git', '--git-dir', @workdir, 'config', '--local',
       "credential.#{http_fetch_url}.helper",
-      '!token(){ echo password="$ARVADOS_API_TOKEN"; }; token'],
+      '!cred(){ cat >/dev/null; if [ "$1" = get ]; then echo password=$ARVADOS_API_TOKEN; fi; };cred'],
      ['git', '--git-dir', @workdir, 'config', '--local',
            'http.sslVerify',
            Rails.configuration.arvados_insecure_https ? 'false' : 'true'],
diff --git a/services/arv-git-httpd/server_test.go b/services/arv-git-httpd/server_test.go
index 5bf861b..e5ddc29 100644
--- a/services/arv-git-httpd/server_test.go
+++ b/services/arv-git-httpd/server_test.go
@@ -91,6 +91,17 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) {
 	_, err = exec.Command("sh", "-c", "cd "+s.tmpWorkdir+" && echo work >work && git add work && git -c user.name=Foo -c user.email=Foo commit -am 'workdir: test'").CombinedOutput()
 	c.Assert(err, check.Equals, nil)
 
+	_, err = exec.Command("git", "config",
+		"--file", s.tmpWorkdir+"/.git/config",
+		"credential.http://"+s.testServer.Addr+"/.helper",
+		"!cred(){ cat >/dev/null; if [ \"$1\" = get ]; then echo password=$ARVADOS_API_TOKEN; fi; };cred").Output()
+	c.Assert(err, check.Equals, nil)
+	_, err = exec.Command("git", "config",
+		"--file", s.tmpWorkdir+"/.git/config",
+		"credential.http://"+s.testServer.Addr+"/.username",
+		"none").Output()
+	c.Assert(err, check.Equals, nil)
+
 	theConfig = &config{
 		Addr:       ":",
 		GitCommand: "/usr/bin/git",
@@ -127,9 +138,10 @@ func (s *IntegrationSuite) runGit(c *check.C, token, gitCmd, repo string, args .
 	os.Chdir(s.tmpWorkdir)
 
 	gitargs := append([]string{
-		gitCmd, "http://none:" + token + "@" + s.testServer.Addr + "/" + repo,
+		gitCmd, "http://" + s.testServer.Addr + "/" + repo,
 	}, args...)
 	cmd := exec.Command("git", gitargs...)
+	cmd.Env = append(os.Environ(), "ARVADOS_API_TOKEN="+token)
 	w, err := cmd.StdinPipe()
 	c.Assert(err, check.Equals, nil)
 	w.Close()

commit c0a80b0a49ab007440cb314d36055b5a23c606ab
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat May 2 02:09:54 2015 -0400

    5416: gofmt fixes. Fix some logs to print as strings, not byte arrays.

diff --git a/services/arv-git-httpd/basic_auth_test.go b/services/arv-git-httpd/basic_auth_test.go
index 6bc88cb..2bd84dc 100644
--- a/services/arv-git-httpd/basic_auth_test.go
+++ b/services/arv-git-httpd/basic_auth_test.go
@@ -14,15 +14,15 @@ type basicAuthTestCase struct {
 
 func TestBasicAuth(t *testing.T) {
 	tests := []basicAuthTestCase{
-		basicAuthTestCase{"Basic Zm9vOmJhcg==", "foo", "bar", true},
-		basicAuthTestCase{"Bogus Zm9vOmJhcg==", "", "", false},
-		basicAuthTestCase{"Zm9vOmJhcg==", "", "", false},
-		basicAuthTestCase{"Basic", "", "", false},
-		basicAuthTestCase{"", "", "", false},
+		{"Basic Zm9vOmJhcg==", "foo", "bar", true},
+		{"Bogus Zm9vOmJhcg==", "", "", false},
+		{"Zm9vOmJhcg==", "", "", false},
+		{"Basic", "", "", false},
+		{"", "", "", false},
 	}
 	for _, test := range tests {
 		if u, p, ok := BasicAuth(&http.Request{Header: map[string][]string{
-			"Authorization": []string{test.hdr},
+			"Authorization": {test.hdr},
 		}}); u != test.user || p != test.pass || ok != test.ok {
 			t.Error("got:", u, p, ok, "expected:", test.user, test.pass, test.ok, "from:", test.hdr)
 		}
diff --git a/services/arv-git-httpd/server_test.go b/services/arv-git-httpd/server_test.go
index ca5c5f4..5bf861b 100644
--- a/services/arv-git-httpd/server_test.go
+++ b/services/arv-git-httpd/server_test.go
@@ -149,10 +149,10 @@ func (s *IntegrationSuite) runGit(c *check.C, token, gitCmd, repo string, args .
 // Make a bare arvados repo at {tmpRepoRoot}/arvados.git
 func (s *IntegrationSuite) makeArvadosRepo(c *check.C) {
 	msg, err := exec.Command("git", "init", "--bare", s.tmpRepoRoot+"/zzzzz-s0uqq-arvadosrepo0123.git").CombinedOutput()
-	c.Log(msg)
+	c.Log(string(msg))
 	c.Assert(err, check.Equals, nil)
 	msg, err = exec.Command("git", "--git-dir", s.tmpRepoRoot+"/zzzzz-s0uqq-arvadosrepo0123.git", "fetch", "../../.git", "HEAD:master").CombinedOutput()
-	c.Log(msg)
+	c.Log(string(msg))
 	c.Assert(err, check.Equals, nil)
 }
 

commit 9019ae81f586fe10e6687710070c34e6ba00aecd
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri May 1 19:47:09 2015 -0400

    5416: Add realm="git" to WWW-Authenticate header, cf. rfc2617

diff --git a/services/arv-git-httpd/auth_handler.go b/services/arv-git-httpd/auth_handler.go
index df2d8d6..6313d50 100644
--- a/services/arv-git-httpd/auth_handler.go
+++ b/services/arv-git-httpd/auth_handler.go
@@ -60,7 +60,7 @@ func (h *authHandler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
 	username, password, ok := BasicAuth(r)
 	if !ok || username == "" || password == "" {
 		statusCode, statusText = http.StatusUnauthorized, "no credentials provided"
-		w.Header().Add("WWW-Authenticate", "basic")
+		w.Header().Add("WWW-Authenticate", "Basic realm=\"git\"")
 		return
 	}
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list