Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gitweb link_files only works for top-level projects #123

Open
tfnico opened this issue Dec 29, 2011 · 17 comments
Open

gitweb link_files only works for top-level projects #123

tfnico opened this issue Dec 29, 2011 · 17 comments

Comments

@tfnico
Copy link

tfnico commented Dec 29, 2011

We sometimes like to organize our repositories into folders, but the notifier doesn't produce links for these correctly.

Example-wise, consider a directory that contain three git repos like this:

  • fizzle.git
  • goggle.git
  • bubble.git

The respective urls for these are:

The git-commit-notifier produces links to these repos just fine.

However, if we want to organize fizzle.git and goggle.git together in a directory "funky" like this:

  • funky/fizzle.git
  • funky/goggle.git
  • bubble.git

.. git-commit-notifier keeps on generating links the way they were before, instead of how they should be:

@tfnico
Copy link
Author

tfnico commented Dec 29, 2011

Note that we have a "global" configuration for all our repos:

gitweb:
  path: https://webserver.com/gitweb
  project: ${repo_name}

However, also if I "hard-code" the project to the sub-folder like this:

gitweb:
  path: https://webserver.com/gitweb
  project: funky/fizzle.git

.. the links still end up pointing to the top-level.

In any case, it would be nice if the repo_name variable would end up pointing to "funky/fizzle", as it also appears in the subject of the mail.

@akzhan
Copy link
Member

akzhan commented Feb 21, 2012

@tfnico, we have no installations like yours. pull requests are very apprepriated.

@dermoth
Copy link

dermoth commented Apr 2, 2012

I don't have a pull request just yet, but the fix is simple: for gitweb, diff_to_html.rb uses Git.repo_name instead of config['gitweb']['project'](which means the setting was totally ignored). See the diff below...

--- /usr/lib/ruby/gems/1.9.1/gems/git-commit-notifier-0.11.2/lib/git_commit_notifier/diff_to_html.rb.orig   2012-04-02 15:44:09.394652993 -0400
+++ /usr/lib/ruby/gems/1.9.1/gems/git-commit-notifier-0.11.2/lib/git_commit_notifier/diff_to_html.rb    2012-04-02 16:32:50.919423617 -0400
@@ -161,7 +161,7 @@
       # TODO: these filenames, etc, should likely be properly html escaped
       if config['link_files']
         file_name = if config["link_files"] == "gitweb" && config["gitweb"]
-          "<a href='#{config['gitweb']['path']}?p=#{Git.repo_name}.git;f=#{file_name};h=#{@current_sha};hb=#{@current_commit}'>#{file_name}</a>"
+          "<a href='#{config['gitweb']['path']}?p=#{config['gitweb']['project']};f=#{file_name};h=#{@current_sha};hb=#{@current_commit}'>#{file_name}</a>"
         elsif config["link_files"] == "gitorious" && config["gitorious"]
           "<a href='#{config['gitorious']['path']}/#{config['gitorious']['project']}/#{config['gitorious']['repository']}/blobs/#{branch_name}/#{file_name}'>#{file_name}</a>"
         elsif config["link_files"] == "cgit" && config["cgit"]
@@ -448,7 +448,7 @@
     def markup_commit_for_html(commit)
       commit = if config["link_files"]
         if config["link_files"] == "gitweb" && config["gitweb"]
-          "<a href='#{config['gitweb']['path']}?p=#{Git.repo_name}.git;a=commitdiff;h=#{commit}'>#{commit}</a>"
+          "<a href='#{config['gitweb']['path']}?p=#{config['gitweb']['project']};a=commitdiff;h=#{commit}'>#{commit}</a>"
         elsif config["link_files"] == "gitorious" && config["gitorious"]
           "<a href='#{config['gitorious']['path']}/#{config['gitorious']['project']}/#{config['gitorious']['repository']}/commit/#{commit}'>#{commit}</a>"
         elsif config["link_files"] == "trac" && config["trac"]

This has the potential to break projects that don't have the project setting set up properly, which would be a config error. Maybe a better fix would be to use Git.repo_name is the config param isn't set, but I'm not very Ruby-literate so don't expect this from me...

Thanks

@dermoth
Copy link

dermoth commented Apr 3, 2012

I see this is already fixed in Git. Thanks

@tfnico
Copy link
Author

tfnico commented Apr 3, 2012

@dermoth Thanks for suggesting the fix. Are you saying in your last comment that the issue has been since fixed in the git-commit-notifier itself?

@dermoth
Copy link

dermoth commented Apr 3, 2012

Yes, if you clone from git and build it you will have this fix, as well as proper email ordering when you push more than one commit at once.

@akzhan
Copy link
Member

akzhan commented Apr 24, 2012

Closed because already fixed.

@akzhan akzhan closed this as completed Apr 24, 2012
@tfnico
Copy link
Author

tfnico commented Apr 26, 2012

I didn't notice anything being fixed after upgrading to 0.11.4, but ended up just hacking it into lib/git_commit_notifier/git.rb like this:

  # TFN: Old line:
  # File.expand_path(git_dir).split("/").last.sub(/\.git$/, '')
  # TFN: Hack to fix subdir problem
  File.expand_path(git_dir).split("/var/git/").last.sub(/\.git$/, '')

Only works if gitweb is running in /var/git, of course.

@tfnico
Copy link
Author

tfnico commented Apr 26, 2012

Another hack for gitweb:

in lib/git_commit_notifier/diff_to_html.rb:491, I hardcoded in gitweb, because even though my config.gitweb.project is empty, I still want links on the commits:

  # TFN: Commented out the line below, because we don't want default, even though config.gitweb.project is empty (cause we use Git.repo_name instead)
  # COMMIT_LINK_MAP[mode].call(config, commit)
  COMMIT_LINK_MAP[:gitweb].call(config, commit)

This is all very hacky, of course, and I don't have the Ruby-fu to make something re-usable. But maybe someone else who comes along and has this configuration can make it work.

@akzhan akzhan reopened this Apr 26, 2012
@gjoseph
Copy link

gjoseph commented Jul 3, 2012

Yup, this issue is still happening in 0.11.4 - the fix as per @dermoth's patch indeed seems to have been applied before 0.11.4, but also only seems to be valid for configurations where one specific repository is configured for the notifier (in gitweb:project). Like @tfnico, we use the "global" configuration, and links are incorrect in such cases.

gitweb:
path: http://git.magnolia-cms.com/gitweb
#project: test.git

@akzhan
Copy link
Member

akzhan commented Jul 3, 2012

@gjoseph Any suggestions?

@gjoseph
Copy link

gjoseph commented Jul 3, 2012

I wish. I can't think of anything that would make sense for everyone, other than, perhaps, introduced a config variable such as "repos_root", i.e where all repos are. And even that might not work for everyone. How about looking at gitweb's projects.list file ?

@akzhan
Copy link
Member

akzhan commented Jul 3, 2012

Can you inform about its location and format? Gist is very apprepriated. This is because I have no Gitweb installations.

@gjoseph
Copy link

gjoseph commented Jul 4, 2012

The location of the file is specified in gitweb.conf ($project_list); or, as I've just realized not used. In any case, one specifies a $projectroot in gitweb.conf, which is the paths under which all repos are. If $project_list is specific, the format of the file is simply one repo per line, with the path (relative to $projectroot) to each repo/.git folder (in my case, these are bare repos). If $project_list is not specified, gitweb scans $projectroot to find all repos.

Example file:
foobar.git
modules/baz.git
modules/qux.git

So seeing this, my suggestion would be to

  1. allow projectroot to also be configured in git-commit-notifier (possibly in the gitweb section) - because you probably don't want to parse gitweb.conf (which is Perl, and also because its content might depend on env.variables)
  2. look at the absolute path of each repo, when building the url, and strip off $projectroot

@akzhan
Copy link
Member

akzhan commented Feb 25, 2013

Will be reviewed and feature planned.

@gjoseph
Copy link

gjoseph commented Feb 25, 2013

thanks !

@alin-simionoiuDE
Copy link

in my setup i see that ${repo_name} is not getting expanded as the project name for gitweb URL's. is this problem part of this issue or i should open a new issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants