Add new repository archive routejramsay-38830-tarball

Repository archives are always named `<project>-<ref>-<sha>` even if
the ref is a commit. A consequence of always including the sha even
for tags is that packaging a release is more difficult because both
the ref and sha must be known by the packager.

- add `<project>/-/archive/<ref>/<filename>.<format>` route using the
`-` separator to prevent namespace collisions. If the filename is
`<project>-<ref>` the sha will be omitted, otherwise the default
filename will be used.
- deprecate previous archive route `repository/<ref>/archive`

7 files changed, 60 insertions, 17 deletions

diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb
index d5af0341d18..1d5f8131c02 100644
--- a/app/controllers/projects/repositories_controller.rb
+++ b/app/controllers/projects/repositories_controller.rb
@@ -1,6 +1,9 @@
 class Projects::RepositoriesController < Projects::ApplicationController
+  include ExtractsPath
+
   # Authorize
   before_action :require_non_empty_project, except: :create
+  before_action :assign_archive_vars, only: :archive
   before_action :authorize_download_code!
   before_action :authorize_admin_project!, only: :create
 
@@ -11,9 +14,17 @@ class Projects::RepositoriesController < Projects::ApplicationController
   end
 
   def archive
-    send_git_archive @repository, ref: params[:ref], format: params[:format]
+    is_shortname = @filename == "#{params[:project_id]}-#{@ref}"
+    send_git_archive @repository, ref: @ref, format: params[:format], append_sha: !is_shortname
   rescue => ex
     logger.error("#{self.class.name}: #{ex}")
     return git_not_found!
   end
+
+  def assign_archive_vars
+    @id = params[:id]
+    @ref, @filename = extract_ref(@id)
+  rescue InvalidPathError
+    render_404
+  end
 end
diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml
index fa9a9bfc8f7..225f90be9af 100644
--- a/app/views/projects/buttons/_download.html.haml
+++ b/app/views/projects/buttons/_download.html.haml
@@ -1,4 +1,5 @@
 - pipeline = local_assigns.fetch(:pipeline) { project.latest_successful_pipeline_for(ref) }
+- archive_prefix = "#{project.path}-#{ref}"
 
 - if !project.empty_repo? && can?(current_user, :download_code, project)
   .project-action-button.dropdown.inline>
@@ -10,16 +11,16 @@
       %li.dropdown-header
         #{ _('Source code') }
       %li
-        = link_to archive_project_repository_path(project, ref: ref, format: 'zip'), rel: 'nofollow', download: '' do
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'zip'), rel: 'nofollow', download: '' do
           %span=  _('Download zip')
       %li
-        = link_to archive_project_repository_path(project, ref: ref, format: 'tar.gz'), rel: 'nofollow', download: '' do
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.gz'), rel: 'nofollow', download: '' do
           %span= _('Download tar.gz')
       %li
-        = link_to archive_project_repository_path(project, ref: ref, format: 'tar.bz2'), rel: 'nofollow', download: '' do
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.bz2'), rel: 'nofollow', download: '' do
           %span= _('Download tar.bz2')
       %li
-        = link_to archive_project_repository_path(project, ref: ref, format: 'tar'), rel: 'nofollow', download: '' do
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar'), rel: 'nofollow', download: '' do
           %span= _('Download tar')
 
       - if pipeline && pipeline.latest_builds_with_artifacts.any?
diff --git a/changelogs/unreleased/jramsay-38830-tarball.yml b/changelogs/unreleased/jramsay-38830-tarball.yml
new file mode 100644
index 00000000000..6d40c305614
--- /dev/null
+++ b/changelogs/unreleased/jramsay-38830-tarball.yml
@@ -0,0 +1,5 @@
+---
+title: Add alternate archive route for simplified packaging
+merge_request: 17225
+author:
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index c803737d40b..eeaabc0f8b4 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -249,6 +249,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       end
 
       scope '-' do
+        get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive'
+
         resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do
           collection do
             post :cancel_all
diff --git a/config/routes/repository.rb b/config/routes/repository.rb
index eace3a615b4..d24a4856dd8 100644
--- a/config/routes/repository.rb
+++ b/config/routes/repository.rb
@@ -2,10 +2,11 @@
 
 resource :repository, only: [:create] do
   member do
-    get ':ref/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, ref: /.+/ }, action: 'archive', as: 'archive'
-
     # deprecated since GitLab 9.5
     get 'archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex }, as: 'archive_alternative'
+
+    # deprecated since GitLab 10.6
+    get ':id/archive', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+/ }, action: 'archive', as: 'archive_deprecated'
   end
 end
 
diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb
index 04d16e98913..31b1b52fdd1 100644
--- a/spec/controllers/projects/repositories_controller_spec.rb
+++ b/spec/controllers/projects/repositories_controller_spec.rb
@@ -6,7 +6,7 @@ describe Projects::RepositoriesController do
   describe "GET archive" do
     context 'as a guest' do
       it 'responds with redirect in correct format' do
-        get :archive, namespace_id: project.namespace, project_id: project, format: "zip", ref: 'master'
+        get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
 
         expect(response.header["Content-Type"]).to start_with('text/html')
         expect(response).to be_redirect
@@ -22,18 +22,25 @@ describe Projects::RepositoriesController do
       end
 
       it "uses Gitlab::Workhorse" do
-        get :archive, namespace_id: project.namespace, project_id: project, ref: "master", format: "zip"
+        get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
 
         expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
       end
 
+      it 'responds with redirect to the short name archive if fully qualified' do
+        get :archive, namespace_id: project.namespace, project_id: project, id: "master/#{project.path}-master", format: "zip"
+
+        expect(assigns(:ref)).to eq("master")
+        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
+      end
+
       context "when the service raises an error" do
         before do
           allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed")
         end
 
         it "renders Not Found" do
-          get :archive, namespace_id: project.namespace, project_id: project, ref: "master", format: "zip"
+          get :archive, namespace_id: project.namespace, project_id: project, id: "master", format: "zip"
 
           expect(response).to have_gitlab_http_status(404)
         end
diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb
index fb1281a6b42..896c6f990b2 100644
--- a/spec/routing/project_routing_spec.rb
+++ b/spec/routing/project_routing_spec.rb
@@ -164,20 +164,36 @@ describe 'project routing' do
   #  archive_project_repository GET    /:project_id/repository/archive(.:format)  projects/repositories#archive
   #     edit_project_repository GET    /:project_id/repository/edit(.:format)     projects/repositories#edit
   describe Projects::RepositoriesController, 'routing' do
-    it 'to #archive' do
-      expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'master')
-    end
-
     it 'to #archive format:zip' do
-      expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', ref: 'master')
+      expect(get('/gitlab/gitlabhq/-/archive/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master/archive')
     end
 
     it 'to #archive format:tar.bz2' do
-      expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', ref: 'master')
+      expect(get('/gitlab/gitlabhq/-/archive/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master/archive')
     end
 
     it 'to #archive with "/" in route' do
-      expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', ref: 'improve/awesome')
+      expect(get('/gitlab/gitlabhq/-/archive/improve/awesome/gitlabhq-improve-awesome.tar.gz')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.gz', id: 'improve/awesome/gitlabhq-improve-awesome')
+    end
+
+    it 'to #archive_alternative' do
+      expect(get('/gitlab/gitlabhq/repository/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq')
+    end
+
+    it 'to #archive_deprecated' do
+      expect(get('/gitlab/gitlabhq/repository/master/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'master')
+    end
+
+    it 'to #archive_deprecated format:zip' do
+      expect(get('/gitlab/gitlabhq/repository/master/archive.zip')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'zip', id: 'master')
+    end
+
+    it 'to #archive_deprecated format:tar.bz2' do
+      expect(get('/gitlab/gitlabhq/repository/master/archive.tar.bz2')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', format: 'tar.bz2', id: 'master')
+    end
+
+    it 'to #archive_deprecated with "/" in route' do
+      expect(get('/gitlab/gitlabhq/repository/improve/awesome/archive')).to route_to('projects/repositories#archive', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'improve/awesome')
     end
   end