[ARVADOS] updated: 1d73ebb1fb2f2f50144de4f74b3aa77616a5d4f2

git at public.curoverse.com git at public.curoverse.com
Thu Oct 8 15:18:28 EDT 2015


Summary of changes:
 apps/workbench/app/helpers/version_helper.rb       | 28 ++----------
 .../views/application/_report_issue_popup.html.erb | 15 +++---
 apps/workbench/config/application.default.yml      | 48 ++------------------
 apps/workbench/config/application.rb               |  1 +
 apps/workbench/lib/app_version.rb                  | 53 ++++++++++++++++++++++
 .../controllers/arvados/v1/schema_controller.rb    |  2 +-
 services/api/config/application.default.yml        | 48 ++------------------
 services/api/config/initializers/app_version.rb    |  1 +
 services/api/lib/app_version.rb                    | 52 +++++++++++++++++++++
 services/api/lib/tasks/config_check.rake           |  3 +-
 .../arvados/v1/schema_controller_test.rb           | 22 +++++++++
 services/api/test/unit/app_version_test.rb         | 43 ++++++++++++++++++
 12 files changed, 198 insertions(+), 118 deletions(-)
 create mode 100644 apps/workbench/lib/app_version.rb
 create mode 100644 services/api/config/initializers/app_version.rb
 create mode 100644 services/api/lib/app_version.rb
 create mode 100644 services/api/test/unit/app_version_test.rb

       via  1d73ebb1fb2f2f50144de4f74b3aa77616a5d4f2 (commit)
       via  15e0b0671b04a81a8e8fa289002655d28d7efb50 (commit)
       via  b598910bb715d9f4803a2ca1c0f84492c5761ed7 (commit)
       via  966b17d66eafb2d1707c5b5119ba0c87fcdae44a (commit)
       via  8a3fb7961809da1a2aeca4bfd26c012f794379e3 (commit)
       via  e88cbab376ec5e26266ed021193b9d387b14dfa7 (commit)
      from  7c97dd88e541a0245272b8e93a33e4d2fe4e32cd (commit)

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 1d73ebb1fb2f2f50144de4f74b3aa77616a5d4f2
Merge: 7c97dd8 15e0b06
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 15:24:19 2015 -0400

    Merge branch '6967-yaml-format' closes #6967


commit 15e0b0671b04a81a8e8fa289002655d28d7efb50
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 14:46:22 2015 -0400

    6967: More helpful comment & assertion failure message

diff --git a/apps/workbench/lib/app_version.rb b/apps/workbench/lib/app_version.rb
index bf50ee4..48cb8f6 100644
--- a/apps/workbench/lib/app_version.rb
+++ b/apps/workbench/lib/app_version.rb
@@ -1,5 +1,5 @@
 # If you change this file, you'll probably also want to make the same
-# changes in apps/workbench/lib/app_version.rb.
+# changes in services/api/lib/app_version.rb.
 
 class AppVersion
   def self.git(*args, &block)
diff --git a/services/api/test/unit/app_version_test.rb b/services/api/test/unit/app_version_test.rb
index eaf9cbb..e8c8374 100644
--- a/services/api/test/unit/app_version_test.rb
+++ b/services/api/test/unit/app_version_test.rb
@@ -29,7 +29,8 @@ class AppVersionTest < ActiveSupport::TestCase
 
   test 'override with file' do
     path = Rails.root.join 'git-commit.version'
-    assert !File.exists?(path)
+    assert(!File.exists?(path),
+           "Packaged version file found in source tree: #{path}")
     begin
       File.open(path, 'w') do |f|
         f.write "0.1.abc123\n"

commit b598910bb715d9f4803a2ca1c0f84492c5761ed7
Author: Tom Clegg <tom at curoverse.com>
Date:   Thu Oct 8 14:45:33 2015 -0400

    6967: Use git status --porcelain to isolate from user config

diff --git a/apps/workbench/lib/app_version.rb b/apps/workbench/lib/app_version.rb
index 05b9f26..bf50ee4 100644
--- a/apps/workbench/lib/app_version.rb
+++ b/apps/workbench/lib/app_version.rb
@@ -30,7 +30,7 @@ class AppVersion
     if @hash.nil? or @hash.empty?
       begin
         local_modified = false
-        git("status", "-s") do |git_pipe|
+        git("status", "--porcelain") do |git_pipe|
           git_pipe.each_line do |_|
             STDERR.puts _
             local_modified = true
diff --git a/services/api/lib/app_version.rb b/services/api/lib/app_version.rb
index 2f235d1..769f4e5 100644
--- a/services/api/lib/app_version.rb
+++ b/services/api/lib/app_version.rb
@@ -30,7 +30,7 @@ class AppVersion
     if @hash.nil? or @hash.empty?
       begin
         local_modified = false
-        git("status", "-s") do |git_pipe|
+        git("status", "--porcelain") do |git_pipe|
           git_pipe.each_line do |_|
             local_modified = true
             # Continue reading the pipe so git doesn't get SIGPIPE.

commit 966b17d66eafb2d1707c5b5119ba0c87fcdae44a
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Oct 7 11:11:14 2015 -0400

    6967: Move source_version detection code from config yaml to lib/app_version.rb.

diff --git a/apps/workbench/app/helpers/version_helper.rb b/apps/workbench/app/helpers/version_helper.rb
index 6cae78f..5c15986 100644
--- a/apps/workbench/app/helpers/version_helper.rb
+++ b/apps/workbench/app/helpers/version_helper.rb
@@ -1,30 +1,12 @@
 module VersionHelper
-  # api_version returns the git commit hash for the API server's
-  # current version.  It is extracted from api_version_text, which
-  # returns the source_version provided by the discovery document and
-  # may have the word "-modified" appended to it (if the API server is
-  # running from a locally modified repository).
-
-  def api_version
-    api_version_text.sub(/[^[:xdigit:]].*/, '')
-  end
-
-  def api_version_text
+  # Get the source_version given in the API server's discovery
+  # document.
+  def api_source_version
     arvados_api_client.discovery[:source_version]
   end
 
-  # wb_version and wb_version_text provide the same strings for the
-  # code version that this Workbench is currently running.
-
-  def wb_version
-    Rails.configuration.source_version
-  end
-
-  def wb_version_text
-    wb_version + (Rails.configuration.local_modified or '')
-  end
-
+  # URL for browsing source code for the given version.
   def version_link_target version
-    "https://arvados.org/projects/arvados/repository/changes?rev=#{version}"
+    "https://arvados.org/projects/arvados/repository/changes?rev=#{version.sub(/-.*/, "")}"
   end
 end
diff --git a/apps/workbench/app/views/application/_report_issue_popup.html.erb b/apps/workbench/app/views/application/_report_issue_popup.html.erb
index 1c964ab..1f66146 100644
--- a/apps/workbench/app/views/application/_report_issue_popup.html.erb
+++ b/apps/workbench/app/views/application/_report_issue_popup.html.erb
@@ -3,18 +3,15 @@
   arvados_base = Rails.configuration.arvados_v1_base
   support_email = Rails.configuration.support_email_address
 
-  api_version_link = link_to api_version_text, version_link_target(api_version)
-  wb_version_link = link_to wb_version_text, version_link_target(wb_version)
-
   additional_info = {}
   additional_info['Current location'] = params[:current_location]
   additional_info['User UUID'] = current_user.uuid if current_user
 
   additional_info_str = additional_info.map {|k,v| "#{k}=#{v}"}.join("\n")
 
-  additional_info['api_version'] = api_version_text
+  additional_info['api_source_version'] = api_source_version
   additional_info['generated_at'] = generated_at
-  additional_info['workbench_version'] = wb_version_text
+  additional_info['workbench_version'] = AppVersion.hash
   additional_info['arvados_base'] = arvados_base
   additional_info['support_email'] = support_email
   additional_info['error_message'] = params[:error_message] if params[:error_message]
@@ -71,14 +68,18 @@
         <div class="form-group">
           <label for="wb_version" class="col-sm-4 control-label"> Workbench version </label>
           <div class="col-sm-8">
-            <p class="form-control-static" name="wb_version"><%= wb_version_link %></p>
+            <p class="form-control-static" name="wb_version">
+              <%= link_to AppVersion.hash, version_link_target(AppVersion.hash) %>
+            </p>
           </div>
         </div>
 
         <div class="form-group">
           <label for="server_version" class="col-sm-4 control-label"> API version </label>
           <div class="col-sm-8">
-            <p class="form-control-static" name="server_version"><%= api_version_link %></p>
+            <p class="form-control-static" name="server_version">
+              <%= link_to api_source_version, version_link_target(api_source_version) %>
+            </p>
           </div>
         </div>
 
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 1650e12..00959bb 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -1,45 +1,6 @@
 # Do not use this file for site configuration. Create application.yml
 # instead (see application.yml.example).
 
-<%
-# If you change any of the code in this block, you'll probably also want
-# to update it in API server's application.default.yml.
-def info_cmd(*args, &block)
-  IO.popen(args, "r", chdir: Rails.root, err: "/dev/null", &block)
-end
-
-source_version = ""
-local_modified = false
-if Rails.env == "production"
-  # Read the version from our package's git-commit.version file, if available.
-  begin
-    source_version = IO.read(Rails.root.join("git-commit.version")).strip
-  rescue Errno::ENOENT
-  end
-end
-
-if source_version.empty?
-  begin
-    status_output = false
-    info_cmd("git", "status", "-s") do |git_pipe|
-      git_pipe.each_line do |_|
-        status_output = true
-        # Continue reading the pipe so git doesn't get SIGPIPE.
-      end
-    end
-    if $?.success?
-      info_cmd("git", "log", "-n1", "--format=%H") do |git_pipe|
-        git_pipe.each_line do |line|
-          source_version = line.chomp
-        end
-      end
-      local_modified = status_output
-    end
-  rescue SystemCallError
-  end
-end
-%>
-
 # Below is a sample setting for diagnostics testing.
 # Configure workbench URL as "arvados_workbench_url"
 # Configure test user tokens as "user_tokens".
@@ -84,7 +45,6 @@ development:
   assets.debug: true
   profiling_enabled: true
   site_name: Arvados Workbench (dev)
-  local_modified: "<%= local_modified ? '-modified' : '' %>"
 
   # API server configuration
   arvados_login_base: ~
@@ -222,9 +182,11 @@ common:
   # the profile page.
   user_profile_form_message: Welcome to Arvados. All <span style="color:red">required fields</span> must be completed before you can proceed.
 
-  # source_version
-  source_version: "<%= source_version[0...8] %>"
-  local_modified: false
+  # Override the automatic version string. With the default value of
+  # false, the version string is read from git-commit.version in
+  # Rails.root (included in vendor packages) or determined by invoking
+  # "git log".
+  source_version: false
 
   # report notification to and from addresses
   issue_reporter_email_from: arvados at example.com
diff --git a/apps/workbench/config/application.rb b/apps/workbench/config/application.rb
index 4ac6819..d1c7934 100644
--- a/apps/workbench/config/application.rb
+++ b/apps/workbench/config/application.rb
@@ -12,6 +12,7 @@ module ArvadosWorkbench
 
     # Custom directories with classes and modules you want to be autoloadable.
     # config.autoload_paths += %W(#{config.root}/extras)
+    config.autoload_paths += %W(#{config.root}/lib)
 
     # Only load the plugins named here, in the order given (default is alphabetical).
     # :all can be used as a placeholder for all plugins not explicitly named.
diff --git a/apps/workbench/lib/app_version.rb b/apps/workbench/lib/app_version.rb
new file mode 100644
index 0000000..05b9f26
--- /dev/null
+++ b/apps/workbench/lib/app_version.rb
@@ -0,0 +1,53 @@
+# If you change this file, you'll probably also want to make the same
+# changes in apps/workbench/lib/app_version.rb.
+
+class AppVersion
+  def self.git(*args, &block)
+    IO.popen(["git", "--git-dir", ".git"] + args, "r",
+             chdir: Rails.root.join('../..'),
+             err: "/dev/null",
+             &block)
+  end
+
+  def self.forget
+    @hash = nil
+  end
+
+  # Return abbrev commit hash for current code version: "abc1234", or
+  # "abc1234-modified" if there are uncommitted changes. If present,
+  # return contents of {root}/git-commit.version instead.
+  def self.hash
+    if (cached = Rails.configuration.source_version || @hash)
+      return cached
+    end
+
+    # Read the version from our package's git-commit.version file, if available.
+    begin
+      @hash = IO.read(Rails.root.join("git-commit.version")).strip
+    rescue Errno::ENOENT
+    end
+
+    if @hash.nil? or @hash.empty?
+      begin
+        local_modified = false
+        git("status", "-s") do |git_pipe|
+          git_pipe.each_line do |_|
+            STDERR.puts _
+            local_modified = true
+            # Continue reading the pipe so git doesn't get SIGPIPE.
+          end
+        end
+        if $?.success?
+          git("log", "-n1", "--format=%H") do |git_pipe|
+            git_pipe.each_line do |line|
+              @hash = line.chomp[0...8] + (local_modified ? '-modified' : '')
+            end
+          end
+        end
+      rescue SystemCallError
+      end
+    end
+
+    @hash || "unknown"
+  end
+end

commit 8a3fb7961809da1a2aeca4bfd26c012f794379e3
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Oct 7 10:00:16 2015 -0400

    6967: Move source_version detection code from config yaml to lib/app_version.rb.

diff --git a/services/api/app/controllers/arvados/v1/schema_controller.rb b/services/api/app/controllers/arvados/v1/schema_controller.rb
index 62d5e59..ba0f90f 100644
--- a/services/api/app/controllers/arvados/v1/schema_controller.rb
+++ b/services/api/app/controllers/arvados/v1/schema_controller.rb
@@ -22,7 +22,7 @@ class Arvados::V1::SchemaController < ApplicationController
         name: "arvados",
         version: "v1",
         revision: "20131114",
-        source_version: (Rails.application.config.source_version ? Rails.application.config.source_version : "No version information available") + (Rails.application.config.local_modified ? Rails.application.config.local_modified.to_s : ''),
+        source_version: AppVersion.hash,
         generatedAt: db_current_time.iso8601,
         title: "Arvados API",
         description: "The API to interact with Arvados.",
diff --git a/services/api/config/application.default.yml b/services/api/config/application.default.yml
index a61420c..8777f28 100644
--- a/services/api/config/application.default.yml
+++ b/services/api/config/application.default.yml
@@ -8,45 +8,6 @@
 # 4. Section in application.default.yml corresponding to RAILS_ENV
 # 5. Section in application.default.yml called "common"
 
-<%
-# If you change any of the code in this block, you'll probably also want
-# to update it in Workbench's application.default.yml.
-def info_cmd(*args, &block)
-  IO.popen(args, "r", chdir: Rails.root, err: "/dev/null", &block)
-end
-
-source_version = ""
-local_modified = false
-if Rails.env == "production"
-  # Read the version from our package's git-commit.version file, if available.
-  begin
-    source_version = IO.read(Rails.root.join("git-commit.version")).strip
-  rescue Errno::ENOENT
-  end
-end
-
-if source_version.empty?
-  begin
-    status_output = false
-    info_cmd("git", "status", "-s") do |git_pipe|
-      git_pipe.each_line do |_|
-        status_output = true
-        # Continue reading the pipe so git doesn't get SIGPIPE.
-      end
-    end
-    if $?.success?
-      info_cmd("git", "log", "-n1", "--format=%H") do |git_pipe|
-        git_pipe.each_line do |line|
-          source_version = line.chomp
-        end
-      end
-      local_modified = status_output
-    end
-  rescue SystemCallError
-  end
-end
-%>
-
 common:
   ###
   ### Essential site configuration
@@ -365,9 +326,11 @@ common:
 
   default_openid_prefix: https://www.google.com/accounts/o8/id
 
-  # source_version
-  source_version: "<%= source_version[0...8] %>"
-  local_modified: false
+  # Override the automatic version string. With the default value of
+  # false, the version string is read from git-commit.version in
+  # Rails.root (included in vendor packages) or determined by invoking
+  # "git log".
+  source_version: false
 
 
 development:
@@ -384,7 +347,6 @@ development:
   active_record.auto_explain_threshold_in_seconds: 0.5
   assets.compress: false
   assets.debug: true
-  local_modified: "<%= local_modified ? '-modified' : '' %>"
 
 production:
   force_ssl: true
diff --git a/services/api/config/initializers/app_version.rb b/services/api/config/initializers/app_version.rb
new file mode 100644
index 0000000..c904856
--- /dev/null
+++ b/services/api/config/initializers/app_version.rb
@@ -0,0 +1 @@
+require 'app_version'
diff --git a/services/api/lib/app_version.rb b/services/api/lib/app_version.rb
new file mode 100644
index 0000000..2f235d1
--- /dev/null
+++ b/services/api/lib/app_version.rb
@@ -0,0 +1,52 @@
+# If you change this file, you'll probably also want to make the same
+# changes in apps/workbench/lib/app_version.rb.
+
+class AppVersion
+  def self.git(*args, &block)
+    IO.popen(["git", "--git-dir", ".git"] + args, "r",
+             chdir: Rails.root.join('../..'),
+             err: "/dev/null",
+             &block)
+  end
+
+  def self.forget
+    @hash = nil
+  end
+
+  # Return abbrev commit hash for current code version: "abc1234", or
+  # "abc1234-modified" if there are uncommitted changes. If present,
+  # return contents of {root}/git-commit.version instead.
+  def self.hash
+    if (cached = Rails.configuration.source_version || @hash)
+      return cached
+    end
+
+    # Read the version from our package's git-commit.version file, if available.
+    begin
+      @hash = IO.read(Rails.root.join("git-commit.version")).strip
+    rescue Errno::ENOENT
+    end
+
+    if @hash.nil? or @hash.empty?
+      begin
+        local_modified = false
+        git("status", "-s") do |git_pipe|
+          git_pipe.each_line do |_|
+            local_modified = true
+            # Continue reading the pipe so git doesn't get SIGPIPE.
+          end
+        end
+        if $?.success?
+          git("log", "-n1", "--format=%H") do |git_pipe|
+            git_pipe.each_line do |line|
+              @hash = line.chomp[0...8] + (local_modified ? '-modified' : '')
+            end
+          end
+        end
+      rescue SystemCallError
+      end
+    end
+
+    @hash || "unknown"
+  end
+end
diff --git a/services/api/lib/tasks/config_check.rake b/services/api/lib/tasks/config_check.rake
index a01e1ea..b7316a5 100644
--- a/services/api/lib/tasks/config_check.rake
+++ b/services/api/lib/tasks/config_check.rake
@@ -1,6 +1,7 @@
 namespace :config do
   desc 'Ensure site configuration has all required settings'
   task check: :environment do
+    $stderr.puts "%-32s %s" % ["AppVersion (discovered)", AppVersion.hash]
     $application_config.sort.each do |k, v|
       if ENV.has_key?('QUIET') then
         # Make sure we still check for the variable to exist
diff --git a/services/api/test/functional/arvados/v1/schema_controller_test.rb b/services/api/test/functional/arvados/v1/schema_controller_test.rb
index 520e36e..f651d81 100644
--- a/services/api/test/functional/arvados/v1/schema_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/schema_controller_test.rb
@@ -2,6 +2,13 @@ require 'test_helper'
 
 class Arvados::V1::SchemaControllerTest < ActionController::TestCase
 
+  setup do forget end
+  teardown do forget end
+  def forget
+    Rails.cache.delete 'arvados_v1_rest_discovery'
+    AppVersion.forget
+  end
+
   test "should get fresh discovery document" do
     MAX_SCHEMA_AGE = 60
     get :index
@@ -20,4 +27,19 @@ class Arvados::V1::SchemaControllerTest < ActionController::TestCase
     assert_includes discovery_doc, 'defaultTrashLifetime'
     assert_equal discovery_doc['defaultTrashLifetime'], Rails.application.config.default_trash_lifetime
   end
+
+  test "discovery document has source_version" do
+    get :index
+    assert_response :success
+    discovery_doc = JSON.parse(@response.body)
+    assert_match /^[0-9a-f]+(-modified)?$/, discovery_doc['source_version']
+  end
+
+  test "discovery document overrides source_version with config" do
+    Rails.configuration.source_version = 'aaa888fff'
+    get :index
+    assert_response :success
+    discovery_doc = JSON.parse(@response.body)
+    assert_equal 'aaa888fff', discovery_doc['source_version']
+  end
 end
diff --git a/services/api/test/unit/app_version_test.rb b/services/api/test/unit/app_version_test.rb
new file mode 100644
index 0000000..eaf9cbb
--- /dev/null
+++ b/services/api/test/unit/app_version_test.rb
@@ -0,0 +1,42 @@
+require 'test_helper'
+
+class AppVersionTest < ActiveSupport::TestCase
+
+  setup do AppVersion.forget end
+
+  teardown do AppVersion.forget end
+
+  test 'invoke git processes only on first call' do
+    AppVersion.expects(:git).
+      with("status", "-s").once.
+      yields " M services/api/README\n"
+    AppVersion.expects(:git).
+      with("log", "-n1", "--format=%H").once.
+      yields "da39a3ee5e6b4b0d3255bfef95601890afd80709\n"
+
+    (0..4).each do
+      v = AppVersion.hash
+      assert_equal 'da39a3ee-modified', v
+    end
+  end
+
+  test 'override with configuration' do
+    Rails.configuration.source_version = 'foobar'
+    assert_equal 'foobar', AppVersion.hash
+    Rails.configuration.source_version = false
+    assert_not_equal 'foobar', AppVersion.hash
+  end
+
+  test 'override with file' do
+    path = Rails.root.join 'git-commit.version'
+    assert !File.exists?(path)
+    begin
+      File.open(path, 'w') do |f|
+        f.write "0.1.abc123\n"
+      end
+      assert_equal "0.1.abc123", AppVersion.hash
+    ensure
+      File.unlink path
+    end
+  end
+end

commit e88cbab376ec5e26266ed021193b9d387b14dfa7
Author: Tom Clegg <tom at curoverse.com>
Date:   Wed Oct 7 10:01:22 2015 -0400

    6967: Treat blob_signing_key like a secret in `rake config:check`.

diff --git a/services/api/lib/tasks/config_check.rake b/services/api/lib/tasks/config_check.rake
index 1b38655..a01e1ea 100644
--- a/services/api/lib/tasks/config_check.rake
+++ b/services/api/lib/tasks/config_check.rake
@@ -6,7 +6,7 @@ namespace :config do
         # Make sure we still check for the variable to exist
         eval("Rails.configuration.#{k}")
       else
-        if /(password|secret)/.match(k) then
+        if /(password|secret|signing_key)/.match(k) then
           # Make sure we still check for the variable to exist, but don't print the value
           eval("Rails.configuration.#{k}")
           $stderr.puts "%-32s %s" % [k, '*********']

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list