[ARVADOS] created: 6fcd07063276bb57a751fa21bd6ab416b4092b3c

git at public.curoverse.com git at public.curoverse.com
Sat May 2 02:59:40 EDT 2015


        at  6fcd07063276bb57a751fa21bd6ab416b4092b3c (commit)


commit 6fcd07063276bb57a751fa21bd6ab416b4092b3c
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat May 2 02:49:44 2015 -0400

    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 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__)

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

    5893: 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 49a76facc1c9b64c0e80551942fa76bc8386c2f0
Author: Tom Clegg <tom at curoverse.com>
Date:   Sat May 2 02:09:54 2015 -0400

    5893: 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 32254a99680b3dbcff053d7c99a26988e1f839eb
Author: Tom Clegg <tom at curoverse.com>
Date:   Fri May 1 19:47:09 2015 -0400

    5893: 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