Commit 6e5cd8e0810c63e8744c23cdb887fea78ea37fb4
Exists in
master
and in
4 other branches
Merge pull request #1375 from tsigo/discover_default_branch
Default branch auto-discovery
Showing
8 changed files
with
118 additions
and
28 deletions
Show diff stats
app/controllers/application_controller.rb
@@ -120,16 +120,6 @@ class ApplicationController < ActionController::Base | @@ -120,16 +120,6 @@ class ApplicationController < ActionController::Base | ||
120 | end | 120 | end |
121 | end | 121 | end |
122 | 122 | ||
123 | - def load_refs | ||
124 | - if params[:ref].blank? | ||
125 | - @branch = params[:branch].blank? ? nil : params[:branch] | ||
126 | - @tag = params[:tag].blank? ? nil : params[:tag] | ||
127 | - @ref = @branch || @tag || @project.try(:default_branch) || Repository.default_ref | ||
128 | - else | ||
129 | - @ref = params[:ref] | ||
130 | - end | ||
131 | - end | ||
132 | - | ||
133 | def render_404 | 123 | def render_404 |
134 | render file: File.join(Rails.root, "public", "404"), layout: false, status: "404" | 124 | render file: File.join(Rails.root, "public", "404"), layout: false, status: "404" |
135 | end | 125 | end |
app/controllers/commits_controller.rb
@@ -59,7 +59,7 @@ class CommitsController < ApplicationController | @@ -59,7 +59,7 @@ class CommitsController < ApplicationController | ||
59 | 59 | ||
60 | def patch | 60 | def patch |
61 | @commit = project.commit(params[:id]) | 61 | @commit = project.commit(params[:id]) |
62 | - | 62 | + |
63 | send_data( | 63 | send_data( |
64 | @commit.to_patch, | 64 | @commit.to_patch, |
65 | type: "text/plain", | 65 | type: "text/plain", |
@@ -67,4 +67,16 @@ class CommitsController < ApplicationController | @@ -67,4 +67,16 @@ class CommitsController < ApplicationController | ||
67 | filename: (@commit.id.to_s + ".patch") | 67 | filename: (@commit.id.to_s + ".patch") |
68 | ) | 68 | ) |
69 | end | 69 | end |
70 | + | ||
71 | + protected | ||
72 | + | ||
73 | + def load_refs | ||
74 | + if params[:ref].blank? | ||
75 | + @branch = params[:branch].blank? ? nil : params[:branch] | ||
76 | + @tag = params[:tag].blank? ? nil : params[:tag] | ||
77 | + @ref = @branch || @tag || @project.try(:default_branch) || 'master' | ||
78 | + else | ||
79 | + @ref = params[:ref] | ||
80 | + end | ||
81 | + end | ||
70 | end | 82 | end |
app/models/project.rb
@@ -187,7 +187,7 @@ end | @@ -187,7 +187,7 @@ end | ||
187 | # private_flag :boolean(1) default(TRUE), not null | 187 | # private_flag :boolean(1) default(TRUE), not null |
188 | # code :string(255) | 188 | # code :string(255) |
189 | # owner_id :integer(4) | 189 | # owner_id :integer(4) |
190 | -# default_branch :string(255) default("master"), not null | 190 | +# default_branch :string(255) |
191 | # issues_enabled :boolean(1) default(TRUE), not null | 191 | # issues_enabled :boolean(1) default(TRUE), not null |
192 | # wall_enabled :boolean(1) default(TRUE), not null | 192 | # wall_enabled :boolean(1) default(TRUE), not null |
193 | # merge_requests_enabled :boolean(1) default(TRUE), not null | 193 | # merge_requests_enabled :boolean(1) default(TRUE), not null |
app/roles/push_observer.rb
1 | +# Includes methods for handling Git Push events | ||
2 | +# | ||
3 | +# Triggered by PostReceive job | ||
1 | module PushObserver | 4 | module PushObserver |
2 | def observe_push(oldrev, newrev, ref, user) | 5 | def observe_push(oldrev, newrev, ref, user) |
3 | data = post_receive_data(oldrev, newrev, ref, user) | 6 | data = post_receive_data(oldrev, newrev, ref, user) |
@@ -84,11 +87,10 @@ module PushObserver | @@ -84,11 +87,10 @@ module PushObserver | ||
84 | data | 87 | data |
85 | end | 88 | end |
86 | 89 | ||
87 | - | ||
88 | - # This method will be called after each post receive | ||
89 | - # and only if user present in gitlab. | ||
90 | - # All callbacks for post receive should be placed here | 90 | + # This method will be called after each post receive and only if the provided |
91 | + # user is present in GitLab. | ||
91 | # | 92 | # |
93 | + # All callbacks for post receive should be placed here. | ||
92 | def trigger_post_receive(oldrev, newrev, ref, user) | 94 | def trigger_post_receive(oldrev, newrev, ref, user) |
93 | # Create push event | 95 | # Create push event |
94 | self.observe_push(oldrev, newrev, ref, user) | 96 | self.observe_push(oldrev, newrev, ref, user) |
@@ -101,5 +103,11 @@ module PushObserver | @@ -101,5 +103,11 @@ module PushObserver | ||
101 | 103 | ||
102 | # Create satellite | 104 | # Create satellite |
103 | self.satellite.create unless self.satellite.exists? | 105 | self.satellite.create unless self.satellite.exists? |
106 | + | ||
107 | + # Discover the default branch, but only if it hasn't already been set to | ||
108 | + # something else | ||
109 | + if default_branch.nil? | ||
110 | + update_attributes(default_branch: discover_default_branch) | ||
111 | + end | ||
104 | end | 112 | end |
105 | end | 113 | end |
app/roles/repository.rb
@@ -94,6 +94,24 @@ module Repository | @@ -94,6 +94,24 @@ module Repository | ||
94 | end.sort_by(&:name) | 94 | end.sort_by(&:name) |
95 | end | 95 | end |
96 | 96 | ||
97 | + # Discovers the default branch based on the repository's available branches | ||
98 | + # | ||
99 | + # - If no branches are present, returns nil | ||
100 | + # - If one branch is present, returns its name | ||
101 | + # - If two or more branches are present, returns the one that has a name | ||
102 | + # matching root_ref (default_branch or 'master' if default_branch is nil) | ||
103 | + def discover_default_branch | ||
104 | + branches = heads.collect(&:name) | ||
105 | + | ||
106 | + if branches.length == 0 | ||
107 | + nil | ||
108 | + elsif branches.length == 1 | ||
109 | + branches.first | ||
110 | + else | ||
111 | + branches.select { |v| v == root_ref }.first | ||
112 | + end | ||
113 | + end | ||
114 | + | ||
97 | def has_commits? | 115 | def has_commits? |
98 | !!commit | 116 | !!commit |
99 | end | 117 | end |
@@ -102,7 +120,7 @@ module Repository | @@ -102,7 +120,7 @@ module Repository | ||
102 | default_branch || "master" | 120 | default_branch || "master" |
103 | end | 121 | end |
104 | 122 | ||
105 | - def root_ref? branch | 123 | + def root_ref?(branch) |
106 | root_ref == branch | 124 | root_ref == branch |
107 | end | 125 | end |
108 | 126 | ||
@@ -111,7 +129,7 @@ module Repository | @@ -111,7 +129,7 @@ module Repository | ||
111 | # Already packed repo archives stored at | 129 | # Already packed repo archives stored at |
112 | # app_root/tmp/repositories/project_name/project_name-commit-id.tag.gz | 130 | # app_root/tmp/repositories/project_name/project_name-commit-id.tag.gz |
113 | # | 131 | # |
114 | - def archive_repo ref | 132 | + def archive_repo(ref) |
115 | ref = ref || self.root_ref | 133 | ref = ref || self.root_ref |
116 | commit = self.commit(ref) | 134 | commit = self.commit(ref) |
117 | return nil unless commit | 135 | return nil unless commit |
@@ -138,6 +156,6 @@ module Repository | @@ -138,6 +156,6 @@ module Repository | ||
138 | end | 156 | end |
139 | 157 | ||
140 | def http_url_to_repo | 158 | def http_url_to_repo |
141 | - http_url = [Gitlab.config.url, "/", path, ".git"].join() | 159 | + http_url = [Gitlab.config.url, "/", path, ".git"].join('') |
142 | end | 160 | end |
143 | end | 161 | end |
db/migrate/20120905043334_set_default_branch_default_to_nil.rb
0 → 100644
@@ -0,0 +1,12 @@ | @@ -0,0 +1,12 @@ | ||
1 | +class SetDefaultBranchDefaultToNil < ActiveRecord::Migration | ||
2 | + def up | ||
3 | + # Set the default_branch to allow nil, and default it to nil | ||
4 | + change_column_null(:projects, :default_branch, true) | ||
5 | + change_column_default(:projects, :default_branch, nil) | ||
6 | + end | ||
7 | + | ||
8 | + def down | ||
9 | + change_column_null(:projects, :default_branch, false) | ||
10 | + change_column_default(:projects, :default_branch, 'master') | ||
11 | + end | ||
12 | +end |
db/schema.rb
@@ -11,7 +11,7 @@ | @@ -11,7 +11,7 @@ | ||
11 | # | 11 | # |
12 | # It's strongly recommended to check this file into your version control system. | 12 | # It's strongly recommended to check this file into your version control system. |
13 | 13 | ||
14 | -ActiveRecord::Schema.define(:version => 20120729131232) do | 14 | +ActiveRecord::Schema.define(:version => 20120905043334) do |
15 | 15 | ||
16 | create_table "events", :force => true do |t| | 16 | create_table "events", :force => true do |t| |
17 | t.string "target_type" | 17 | t.string "target_type" |
@@ -98,16 +98,16 @@ ActiveRecord::Schema.define(:version => 20120729131232) do | @@ -98,16 +98,16 @@ ActiveRecord::Schema.define(:version => 20120729131232) do | ||
98 | t.string "name" | 98 | t.string "name" |
99 | t.string "path" | 99 | t.string "path" |
100 | t.text "description" | 100 | t.text "description" |
101 | - t.datetime "created_at", :null => false | ||
102 | - t.datetime "updated_at", :null => false | ||
103 | - t.boolean "private_flag", :default => true, :null => false | 101 | + t.datetime "created_at", :null => false |
102 | + t.datetime "updated_at", :null => false | ||
103 | + t.boolean "private_flag", :default => true, :null => false | ||
104 | t.string "code" | 104 | t.string "code" |
105 | t.integer "owner_id" | 105 | t.integer "owner_id" |
106 | - t.string "default_branch", :default => "master", :null => false | ||
107 | - t.boolean "issues_enabled", :default => true, :null => false | ||
108 | - t.boolean "wall_enabled", :default => true, :null => false | ||
109 | - t.boolean "merge_requests_enabled", :default => true, :null => false | ||
110 | - t.boolean "wiki_enabled", :default => true, :null => false | 106 | + t.string "default_branch" |
107 | + t.boolean "issues_enabled", :default => true, :null => false | ||
108 | + t.boolean "wall_enabled", :default => true, :null => false | ||
109 | + t.boolean "merge_requests_enabled", :default => true, :null => false | ||
110 | + t.boolean "wiki_enabled", :default => true, :null => false | ||
111 | end | 111 | end |
112 | 112 | ||
113 | create_table "protected_branches", :force => true do |t| | 113 | create_table "protected_branches", :force => true do |t| |
spec/roles/repository_spec.rb
@@ -19,4 +19,54 @@ describe Project, "Repository" do | @@ -19,4 +19,54 @@ describe Project, "Repository" do | ||
19 | project.should_not be_empty_repo | 19 | project.should_not be_empty_repo |
20 | end | 20 | end |
21 | end | 21 | end |
22 | + | ||
23 | + describe "#discover_default_branch" do | ||
24 | + let(:master) { double(name: 'master') } | ||
25 | + let(:stable) { double(name: 'stable') } | ||
26 | + | ||
27 | + it "returns 'master' when master exists" do | ||
28 | + project.should_receive(:heads).and_return([stable, master]) | ||
29 | + project.discover_default_branch.should == 'master' | ||
30 | + end | ||
31 | + | ||
32 | + it "returns non-master when master exists but default branch is set to something else" do | ||
33 | + project.default_branch = 'stable' | ||
34 | + project.should_receive(:heads).and_return([stable, master]) | ||
35 | + project.discover_default_branch.should == 'stable' | ||
36 | + end | ||
37 | + | ||
38 | + it "returns a non-master branch when only one exists" do | ||
39 | + project.should_receive(:heads).and_return([stable]) | ||
40 | + project.discover_default_branch.should == 'stable' | ||
41 | + end | ||
42 | + | ||
43 | + it "returns nil when no branch exists" do | ||
44 | + project.should_receive(:heads).and_return([]) | ||
45 | + project.discover_default_branch.should be_nil | ||
46 | + end | ||
47 | + end | ||
48 | + | ||
49 | + describe "#root_ref" do | ||
50 | + it "returns default_branch when set" do | ||
51 | + project.default_branch = 'stable' | ||
52 | + project.root_ref.should == 'stable' | ||
53 | + end | ||
54 | + | ||
55 | + it "returns 'master' when default_branch is nil" do | ||
56 | + project.default_branch = nil | ||
57 | + project.root_ref.should == 'master' | ||
58 | + end | ||
59 | + end | ||
60 | + | ||
61 | + describe "#root_ref?" do | ||
62 | + it "returns true when branch is root_ref" do | ||
63 | + project.default_branch = 'stable' | ||
64 | + project.root_ref?('stable').should be_true | ||
65 | + end | ||
66 | + | ||
67 | + it "returns false when branch is not root_ref" do | ||
68 | + project.default_branch = nil | ||
69 | + project.root_ref?('stable').should be_false | ||
70 | + end | ||
71 | + end | ||
22 | end | 72 | end |