srushti / goldberg

Goldberg is a lightweight CI server written in Ruby which worries about Bundler & RVM so that you don't have to.
Other
243 stars 29 forks source link

allow periods (.) in project names #142

Closed ghost closed 11 years ago

ghost commented 11 years ago

It's not possible to use freely-defined project-names as they have to match the folder name of the build, at the moment (meaning only characters the file system allows can be used). Although it would be nice to be able to do so, my bigger concern was to use periods in project names, which should be possible with filename-compliant names... To be able to do so, I had to change the routing as follows:

--- orangenschale.orig/config/routes.rb 2012-07-12 05:14:44.000000000 -0500
+++ orangenschale/config/routes.rb  2012-07-15 11:15:42.071137700 -0500
@@ -10,10 +10,10 @@

   resources :projects, :only => 'index'

-  get '/projects/:project_name' => 'projects#show', :as => :project
-  post '/projects/:project_name/builds' => 'projects#force', :as => :project_force
-  put '/projects/:project_name/builds/:build_number/cancel' => 'builds#cancel', :as => :build_cancel
+  get '/projects/:project_name' => 'projects#show', :constraints => {:project_name => /[^\/]+/}, :format => false, :as => :project
+  post '/projects/:project_name/builds' => 'projects#force', :constraints => {:project_name => /[^\/]+/}, :as => :project_force
+  put '/projects/:project_name/builds/:build_number/cancel' => 'builds#cancel', :constraints => {:project_name => /[^\/]+/}, :as => :build_cancel

-  get '/projects/:project_name/builds/:build_number' => 'builds#show', :as => :project_build
-  get '/projects/:project_name/builds/:build_number/artefacts/:path' => 'builds#artefact', :constraints => {:path => /.*/}, :as => :project_build_artefact
+  get '/projects/:project_name/builds/:build_number' => 'builds#show', :constraints => {:project_name => /[^\/]+/}, :as => :project_build
+  get '/projects/:project_name/builds/:build_number/artefacts/:path' => 'builds#artefact', :constraints => {:path => /.*/,:project_name => /[^\/]+/}, :as => :project_build_artefact
 end
srushti commented 11 years ago

The problem is the substitution strategy. If we replace unusable characters with an underscore or something, then you can't later add another project which really has underscores there. So, for now I'm leaning towards leaving this alone. Let me think about it for a bit.

ghost commented 11 years ago

Well, I understand, however, the patch is not about allowing any kind of project name, it's simple deactivating rails' behaviour to 'format'. If right now you'd add a project with a period in it, rails will simply display an error page, as it tries to match the 'suffix' to a format.

In my example, I have projects running builds on different emulators to test for portability, so a project has a name like: my_project_on_OpenBSD-4.4 - which would fail.

srushti commented 11 years ago

Ah, ok. I understand now.

That, of course, causes another problem. If you have project called 'foo' your browser url would become '/projects/foo', and we also have '/projects/foo.png' which returns an image with the current status of the build to use in documentation pages. If we ignore dots in those urls that might stop the dot in '.png' also from working.

Let me think about this a bit more (unless you have a different suggestion).

ghost commented 11 years ago

Interesting, I didn't even know that... Well, the :constraints => {:project_name => /[^\/]+/} could be changed to match everything except for .png suffixes, etc..

srushti commented 11 years ago

Fixed. Look at the commit above if you're curious. Basically, I figured out what approach travis-ci took to solve the problem.

ghost commented 11 years ago

Thanks, that's definitely cleaner than my version!