[ARVADOS] updated: d4deb0590e54eece238c1c5ad120e0daffd7806a

git at public.curoverse.com git at public.curoverse.com
Thu Apr 16 21:02:31 EDT 2015


Summary of changes:
 apps/workbench/app/models/repository.rb            |  8 ++---
 .../pipeline_instances/_running_component.html.erb |  5 ++-
 .../repositories/_repository_breadcrumbs.html.erb  |  2 +-
 .../arvados/v1/repositories_controller_test.rb     | 41 ++++++++++++++++------
 4 files changed, 38 insertions(+), 18 deletions(-)

  discards  feb6833d90f5208503cb04bf4c10233847796b8d (commit)
  discards  faabe2f83241f6d1015ac1a346e299675d519787 (commit)
       via  d4deb0590e54eece238c1c5ad120e0daffd7806a (commit)
       via  34abff5723aa938d03a1709e3c28899b2949b4d6 (commit)
       via  0929ea3535871ac7f3d4a2d2f8c19d785565fef3 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (feb6833d90f5208503cb04bf4c10233847796b8d)
            \
             N -- N -- N (d4deb0590e54eece238c1c5ad120e0daffd7806a)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit d4deb0590e54eece238c1c5ad120e0daffd7806a
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Apr 16 16:44:38 2015 -0400

    5416: Remove second trailing slash in breadcrumbs link.

diff --git a/apps/workbench/app/views/repositories/_repository_breadcrumbs.html.erb b/apps/workbench/app/views/repositories/_repository_breadcrumbs.html.erb
index 736c187..14f9ba7 100644
--- a/apps/workbench/app/views/repositories/_repository_breadcrumbs.html.erb
+++ b/apps/workbench/app/views/repositories/_repository_breadcrumbs.html.erb
@@ -3,7 +3,7 @@
   <%= link_to(@commit, show_repository_commit_path(id: @object.uuid, commit: @commit), title: 'show commit message') %>
 </div>
 <p>
-  <%= link_to(@object.name, show_repository_tree_path(id: @object.uuid, commit: @commit, path: '/'), title: 'show root directory of source tree') %>
+  <%= link_to(@object.name, show_repository_tree_path(id: @object.uuid, commit: @commit, path: ''), title: 'show root directory of source tree') %>
   <% parents = ''
      (@path || '').split('/').each do |pathpart|
      parents = parents + pathpart + '/'

commit 34abff5723aa938d03a1709e3c28899b2949b4d6
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Apr 16 16:31:22 2015 -0400

    5416: Disable repository browsing (and skip tests) if git version is suspected unreliable.

diff --git a/apps/workbench/app/controllers/repositories_controller.rb b/apps/workbench/app/controllers/repositories_controller.rb
index c5b3501..89dd96b 100644
--- a/apps/workbench/app/controllers/repositories_controller.rb
+++ b/apps/workbench/app/controllers/repositories_controller.rb
@@ -1,5 +1,8 @@
 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)
@@ -32,4 +35,10 @@ 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 aa77913..28f8f0c 100644
--- a/apps/workbench/app/models/repository.rb
+++ b/apps/workbench/app/models/repository.rb
@@ -48,6 +48,15 @@ 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
+
   protected
 
   # refresh fetches the latest repository content into the local
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 ce63b2e..473a7c1 100644
--- a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
@@ -99,7 +99,8 @@
               <% # link to repo tree/file only if the repo is readable
                  # and the commit is a sha1...
                  repo =
-                 (/^[0-9a-f]{40}$/ =~ current_component[:script_version] and
+                 (not Repository.disable_repository_browsing? 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 4061ee8..e28f76a 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -211,3 +211,9 @@ 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 25bf557..852a602 100644
--- a/apps/workbench/test/controllers/repositories_controller_test.rb
+++ b/apps/workbench/test/controllers/repositories_controller_test.rb
@@ -69,6 +69,7 @@ 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, {
@@ -85,6 +86,7 @@ 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, {
@@ -96,6 +98,7 @@ 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, {
@@ -110,6 +113,7 @@ 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 a6a85b5..d936877 100644
--- a/apps/workbench/test/integration/repositories_browse_test.rb
+++ b/apps/workbench/test/integration/repositories_browse_test.rb
@@ -13,6 +13,7 @@ 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'
@@ -36,6 +37,7 @@ 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 0929ea3535871ac7f3d4a2d2f8c19d785565fef3
Author: Tom Clegg <tom at curoverse.com>
Date:   Tue Apr 14 16:56:04 2015 -0400

    5416: Add read-only clone_urls attribute to Repository resources, deprecate push_url and fetch_url, tidy up config settings.

diff --git a/apps/workbench/app/models/repository.rb b/apps/workbench/app/models/repository.rb
index 48c7f9e..aa77913 100644
--- a/apps/workbench/app/models/repository.rb
+++ b/apps/workbench/app/models/repository.rb
@@ -59,12 +59,10 @@ class Repository < ArvadosBase
     @fresh = true
   end
 
-  # http_fetch_url returns an http:// or https:// fetch-url which can
-  # accept arvados API token authentication. The API server currently
-  # advertises SSH fetch-urls, which work for users with SSH keys but
-  # are useless for fetching repository content into workbench itself.
+  # http_fetch_url returns the first http:// or https:// url (if any)
+  # in the api response's clone_urls attribute.
   def http_fetch_url
-    "https://git.#{uuid[0,5]}.arvadosapi.com/#{name}.git"
+    clone_urls.andand.select { |u| /^http/ =~ u }.first
   end
 
   # run_git sets up the ARVADOS_API_TOKEN environment variable,
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 cc3b4c8..ce63b2e 100644
--- a/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_running_component.html.erb
@@ -97,10 +97,13 @@
           <div class="col-md-6">
             <table>
               <% # link to repo tree/file only if the repo is readable
-                 # and the commit is a sha1
+                 # and the commit is a sha1...
                  repo =
                  (/^[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
+                 repo = nil unless repo.http_fetch_url
                  %>
               <% [:script, :repository, :script_version, :supplied_script_version, :nondeterministic].each do |k| %>
                 <tr>
diff --git a/doc/api/schema/Repository.html.textile.liquid b/doc/api/schema/Repository.html.textile.liquid
index 27cc711..0f9b25e 100644
--- a/doc/api/schema/Repository.html.textile.liquid
+++ b/doc/api/schema/Repository.html.textile.liquid
@@ -19,5 +19,7 @@ Each Repository has, in addition to the usual "attributes of Arvados resources":
 table(table table-bordered table-condensed).
 |_. Attribute|_. Type|_. Description|_. Example|
 |name|string|The name of the repository on disk.  Repository names must begin with a letter and contain only alphanumerics.  Unless the repository is owned by the system user, the name must begin with the owner's username, then be separated from the base repository name with @/@.  You may not create a repository that is owned by a user without a username.|@username/project1@|
-|fetch_url|string|The git remote's fetch URL for the repository.  Read-only.||
-|push_url|string|The git remote's push URL for the repository.  Read-only.||
+|clone_urls|array|URLs from which the repository can be cloned. Read-only.|@["git at git.zzzzz.arvadosapi.com:foo/bar.git",
+ "https://git.zzzzz.arvadosapi.com/foo/bar.git"]@|
+|fetch_url|string|URL suggested as a fetch-url in git config. Deprecated. Read-only.||
+|push_url|string|URL suggested as a push-url in git config. Deprecated. Read-only.||
diff --git a/docker/api/application.yml.in b/docker/api/application.yml.in
index 3cfb5a9..627e775 100644
--- a/docker/api/application.yml.in
+++ b/docker/api/application.yml.in
@@ -20,7 +20,11 @@ development:
 
 production:
   host: api.dev.arvados
-  git_host: api.dev.arvados
+
+  git_repo_ssh_base: "git at api.dev.arvados:"
+
+  # Docker setup doesn't include arv-git-httpd yet.
+  git_repo_https_base: false
 
   # At minimum, you need a nice long randomly generated secret_token here.
   # Use a long string of alphanumeric characters (at least 36).
diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index 20e9690..dcc9c63 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -28,7 +28,6 @@ class Arvados::V1::SchemaController < ApplicationController
         description: "The API to interact with Arvados.",
         documentationLink: "http://doc.arvados.org/api/index.html",
         defaultCollectionReplication: Rails.configuration.default_collection_replication,
-        gitHttpBase: Rails.configuration.git_http_base,
         protocol: "rest",
         baseUrl: root_url + "arvados/v1/",
         basePath: "/arvados/v1/",
diff --git a/services/api/app/models/repository.rb b/services/api/app/models/repository.rb
index e83ac41..f361a49 100644
--- a/services/api/app/models/repository.rb
+++ b/services/api/app/models/repository.rb
@@ -13,23 +13,27 @@ class Repository < ArvadosModel
     t.add :name
     t.add :fetch_url
     t.add :push_url
+    t.add :clone_urls
   end
 
   def self.attributes_required_columns
-    super.merge({"push_url" => ["name"], "fetch_url" => ["name"]})
+    super.merge("clone_urls" => ["name"],
+                "fetch_url" => ["name"],
+                "push_url" => ["name"])
   end
 
+  # Deprecated. Use clone_urls instead.
   def push_url
-    prefix = new_record? ? Rails.configuration.uuid_prefix : uuid[0,5]
-    if prefix == Rails.configuration.uuid_prefix
-      host = Rails.configuration.git_host
-    end
-    host ||= "git.%s.arvadosapi.com" % prefix
-    "git@%s:%s.git" % [host, name]
+    ssh_clone_url
   end
 
+  # Deprecated. Use clone_urls instead.
   def fetch_url
-    push_url
+    ssh_clone_url
+  end
+
+  def clone_urls
+    [ssh_clone_url, https_clone_url].compact
   end
 
   def server_path
@@ -88,4 +92,24 @@ class Repository < ArvadosModel
       false
     end
   end
+
+  def ssh_clone_url
+    _clone_url :git_repo_ssh_base, 'git at git.%s.arvadosapi.com:'
+  end
+
+  def https_clone_url
+    _clone_url :git_repo_https_base, 'https://git.%s.arvadosapi.com/'
+  end
+
+  def _clone_url config_var, default_base_fmt
+    configured_base = Rails.configuration.send config_var
+    return nil if configured_base == false
+    prefix = new_record? ? Rails.configuration.uuid_prefix : uuid[0,5]
+    if prefix == Rails.configuration.uuid_prefix and configured_base != true
+      base = configured_base
+    else
+      base = default_base_fmt % prefix
+    end
+    '%s%s.git' % [base, name]
+  end
 end
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index e54fa65..afba2a9 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -58,10 +58,19 @@ common:
   # logic for deciding on a hostname.
   host: false
 
-  # If not false, this is the hostname that will be used to generate
-  # fetch_url and push_url for locally hosted git repositories.  By
-  # default, this is git.(uuid_prefix).arvadosapi.com
-  git_host: false
+  # Base part of SSH git clone url given with repository resources. If
+  # true, the default "git at git.(uuid_prefix).arvadosapi.com:" is
+  # used. If false, SSH clone URLs are not advertised. Include a
+  # trailing ":" or "/" if needed: it will not be added automatically.
+  git_repo_ssh_base: true
+
+  # Base part of HTTPS git clone urls given with repository
+  # resources. This is expected to be an arv-git-httpd service which
+  # accepts API tokens as HTTP-auth passwords. If true, the default
+  # "https://git.(uuid_prefix).arvadosapi.com/" is used. If false,
+  # HTTPS clone URLs are not advertised. Include a trailing ":" or "/"
+  # if needed: it will not be added automatically.
+  git_repo_https_base: true
 
   # If this is not false, HTML requests at the API server's root URL
   # are redirected to this location, and it is provided in the text of
@@ -75,11 +84,6 @@ common:
   # {git_repositories_dir}/arvados/.git
   git_repositories_dir: /var/lib/arvados/git
 
-  # If an arv-git-httpd service is running, advertise it in the
-  # discovery document by adding its public URI base here. Example:
-  # https://git.xxxxx.arvadosapi.com
-  git_http_base: false
-
   # This is a (bare) repository that stores commits used in jobs.  When a job
   # runs, the source commits are first fetched into this repository, then this
   # repository is used to deploy to compute nodes.  This should NOT be a
@@ -235,7 +239,8 @@ common:
   # should be at least 50 characters.
   secret_token: ~
 
-  # email address to which mail should be sent when the user creates profile for the first time
+  # Email address to notify whenever a user creates a profile for the
+  # first time
   user_profile_notification_address: false
 
   default_openid_prefix: https://www.google.com/accounts/o8/id
diff --git a/services/api/test/functional/arvados/v1/repositories_controller_test.rb b/services/api/test/functional/arvados/v1/repositories_controller_test.rb
index fe5bb1c..7f4ed8e 100644
--- a/services/api/test/functional/arvados/v1/repositories_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/repositories_controller_test.rb
@@ -97,26 +97,45 @@ class Arvados::V1::RepositoriesControllerTest < ActionController::TestCase
   end
 
   [
-    {config: "example.com", host: "example.com"},
-    {config: false, host: "git.zzzzz.arvadosapi.com"}
-  ].each do |set_git_host|
-    test "setting git_host to #{set_git_host[:host]} changes fetch/push_url to #{set_git_host[:config]}" do
-      Rails.configuration.git_host = set_git_host[:config]
+    {cfg: :git_repo_ssh_base, cfgval: "git at example.com:", match: %r"^git at example.com:/"},
+    {cfg: :git_repo_ssh_base, cfgval: true, match: %r"^git at git.zzzzz.arvadosapi.com:/"},
+    {cfg: :git_repo_ssh_base, cfgval: false, refute: /^git@/ },
+    {cfg: :git_repo_https_base, cfgval: "https://example.com/", match: %r"https://example.com/"},
+    {cfg: :git_repo_https_base, cfgval: true, match: %r"^https://git.zzzzz.arvadosapi.com/"},
+    {cfg: :git_repo_https_base, cfgval: false, refute: /^http/ },
+  ].each do |expect|
+    test "set #{expect[:cfg]} to #{expect[:cfgval]}" do
+      Rails.configuration.send expect[:cfg].to_s+"=", expect[:cfgval]
       authorize_with :active
-      get(:index)
+      get :index
       assert_response :success
-      assert_includes(json_response["items"].map { |r| r["fetch_url"] },
-                      "git@#{set_git_host[:host]}:active/foo.git")
-      assert_includes(json_response["items"].map { |r| r["push_url"] },
-                      "git@#{set_git_host[:host]}:active/foo.git")
+      json_response['items'].each do |r|
+        if expect[:refute]
+          r['clone_urls'].each do |u|
+            refute_match expect[:refute], u
+          end
+        else
+          assert r['clone_urls'].any? do |u|
+            expect[:prefix].match u
+          end
+        end
+      end
     end
   end
 
-  test "can select push_url in index" do
+  test "select push_url in index" do
     authorize_with :active
     get(:index, {select: ["uuid", "push_url"]})
     assert_response :success
     assert_includes(json_response["items"].map { |r| r["push_url"] },
                     "git at git.zzzzz.arvadosapi.com:active/foo.git")
   end
+
+  test "select clone_urls in index" do
+    authorize_with :active
+    get(:index, {select: ["uuid", "clone_urls"]})
+    assert_response :success
+    assert_includes(json_response["items"].map { |r| r["clone_urls"] }.flatten,
+                    "git at git.zzzzz.arvadosapi.com:active/foo.git")
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list