What is the problem with repositories cleanup?

[alexgarbarev]

Thanks for good plugin, I'm doing some minor fixies and I've noticed that in readme:

This plugin doesn't clean-up (delete) cloned repos from the file system when the record is deleted from Redmine.

And this in the code:

  # TODO: figure out how to do this safely (if at all)
  # before_deletion :rm_removed_repo
  # def rm_removed_repo
  #   if Repository.find_all_by_url(repo.url).length <= 1
  #     system "rm -Rf #{self.clone_path}"
  #   end
  # end

What was the reason for commenting this code out? I though if no Repository object with same URL is available, then it's safe to delete repo directory.. Is that right?

[dergachev]

Hi, thanks for using the plugin. Honestly I'm not sure anymore, it's been a while since I wrote it. Sorry! :)

I guess my main concern is the security side of things, eg. making sure we don't delete any path except within the right folder. Also what happens if two different redmine projects share the same repo path?

Although I haven't done anything with the plugin, but it's been working pretty well for us for the last year. What fixes do you have in mind? On my top priority list is better error reporting, in case of file permission or remote permission errors for example.

[alexgarbarev]

Hi, thanks for reply!

I guess my main concern is the security side of things, eg. making sure we don't delete any path except within the right folder

Yes, I see.. I thought about that too. My solution was to remove repository only if it's inside plugins folder. (if user chose directory outside plugin - then he responsible to delete that). Actually, I think ideal behaviour is to delete repositories created by plugin (we can store which repositories was created in db, but it's tricky when multiple repos references to same path)

Also what happens if two different redmine projects share the same repo path?

Then it's similar as retain count model. Removing latest repo will remove directory as well. That was in your original code:

   if Repository.find_all_by_url(repo.url).length <= 1 
  # ^-- checks if nobody else references to same path (since repo.url is path actually)

Regarding my minor fixes.. They are really small and available here: https://github.com/alexgarbarev/redmine_git_remote It's repo removing and using relative paths in database (I faced with problem after redmine was migrated to another server instance).

[dergachev]

OK cool! When you're done feel free to do a PR and I'll consider merging, if it's still simple enough to test. :)