Adding svn provider support for versioning of individual files

[squarebracket]

I extended the SVN provider to include some particular functionality I needed, but I'm not sure if it fits with the "goal" of the module. If it does, I'll add tests and make it more robust; otherwise I'll fork it off into my own module.

Two features have been added to the provider.

1. Export support
   Added :export parameter/feature to type/provider -- accepts :true or :false
   Provider runs svn export instead of svn checkout if :export is true, which creates an non-versioned copy of the working tree in path. Of course, since it is non-versioned, there is no way of tracking revision through svn, so each subsequent puppet run requires :force to be true.
2. Cherry-picking individual files from the repo
   Added :files parameter/feature to type/provider -- accepts array (Should probably be renamed to :paths)
   Implements functionality of subversion sparse directories. Only useful when using a non-full depth. It calls svn update on the paths that are passed through to it.
   In other words, if you want to check out only certain parts of the repo to the local working tree, you could, for example, do:

vcsrepo { '/some/local/directory':
  ensure => present,
  provider => svn,
  depth => 'immediates',
  source => 'http://svn.example.com/',
  files => ['/some/path', '/some/file.txt', '/some/otherfile.txt']
}

[hunner]

Of note, svn export is similar to git archive --remote=git://... <branch> | tar xf - which outputs just the work tree with no repo attached. (see https://www.kernel.org/pub/software/scm/git/docs/git-archive.html)

It's nice to have portable options between providers with similar functionality. I think the name export is appropriate for this feature, as the word archive could be used for creating an actual tarball of the contents (say, svn export into tar in the svn provider).

[hunner]

I don't really understand what files does from your description. It only updates SOME files from the repo, instead of all of them? Why is this useful?

[sodabrew]

How will the provider know when it needs to update the output without talking to the network every time? I think you'll end up rewriting the targets on every puppet execution.

[squarebracket]

I can definitely add in the export functionality to the git provider as well.

Regarding files, I'll let you know how I'm using it since that will probably provide a clearer picture of its use case. I use it to pull just the jmeter test plan files from our tests repository. I don't pull down the entire repo because it also includes documentation, example properties files, and example data files, and I don't want to clutter the working directory where I launch jmeter tests.

In a more general sense, I see it as a (poorly-named) implementation of subversion's sparse directories functionality.

[squarebracket]

@sodabrew if you're talking about the export functionality, that is correct.

[squarebracket]

Updated first comment to make the files point clearer in its use.

[jonnytdevops]

@hunner Do you think this is something we'd like inside a puppet module? As @sodabrew says, the module would have to communicate over the network on each run. Maybe that's ok, as it would only do this when using this new 'files' property, but it may not be a "moduley" things...

[hunner]

This mentions SVN explicitly, but it may be usable from other providers.

[hunner]

After community review in the weekly PR triage, we are 👍 for functionality, 👍 for bandwidth concerns, and 👍 for the property names files and export

[HelenCampbell]

Hey @squarebracket , thank you for your contribution! We would be keen to get this merged, however there are a few things we need to do so. Would you be willing to add documentation to the README and some tests for your added functionality?

[squarebracket]

Hi! Yes, I saw the comments from a few days ago. I've been a bit busy working on the puppet-graphite module so I haven't had a chance to get around to this. I'll push some tests/docs in the next couple weeks and bump the thread when I'm done.

[squarebracket]

Ok, I reworked things quite a bit since the last update.

Functionality changes

I renamed the files parameter to be includes, since I think that makes way more sense. It is now a property that can be properly tracked and changed. Providing paths to includes auto-activates sparse mode (i.e. it checks out the root repo with --depth empty), and if a depth is given, it will check out the includes paths at that depth. See the docs for a better explanation.

I've decided to drop support for export since it cluttered up the code too much with if/else and a boat load of warnings. However, after looking at the magic inside the type's ensure, I'm thinking that, if export is implemented, it should be implemented as an ensure type. I think it would be much clearer that way that you're getting an unversioned copy. I'd like to hear your feedback, but regardless, it won't be part of this PR.

Tests

Added unit tests for all the includes stuff, as well as some acceptance tests, cuz why not.

Docs

Updated documentation with a verbose explanation of how sparse stuff works.

Outstanding issues

I purposely checked in an acceptance test that is failing. It is that switching sources does not work. The type/provider will not know that the source has changed, since source is a param and not a property. I'd like to know what should be done about this. Should I change source to a property, and implement the logic in all of the providers? I see you're doing some fancy stuff with multiple remotes in the git provider, but I didn't look over it too much.

Also, there's somewhat of a name clash between excludes, which is more like "local ignore paths" and includes, which is paths to check out from the repo. I want to know if you have any reservations about this.

[squarebracket]

Woops, accidentally checked in my libvirt puppet vagrant box, just undid that.

[squarebracket]

The only test that's failing is the one I noted before -- the acceptance test for switching sources. This is intentional because it's something that needs to be fixed. If you'd like me to file a bug in JIRA to have the discussion there, let me know.

[squarebracket]

Did this come up at your meeting yesterday by any chance?

[hunner]

The ability to change the source as a property should probably be submitted in a separate PR that this depends on. For the git provider, the update_remotes method is essentially the source=(value) property setter, and the exists? method would be updated to use the getter (called source) instead of working_copy_exists?