Code review comment for lp:~aaron-whitehouse/duplicity/filelist_combine

Sorry about the conflict. I thought I had, but was updating against a branch instead of the dev branch - I'm still getting the hang of bzr. Next time I'll know to check the information on the merge page. Hopefully I've rebased it in the correct way, but please let me know if not.

Kenneth is right, I folded the functionality of globbing filelists into non-globbing filelists. What is called --include-filelist is what used to be --include-globbing-filelist, but I went for the shorter name. So it is the old --in/exclude-filelist functionality that has been removed.

I'm not sure that I follow your point that /folder is different from /folder/** now - do you mean that they are prior to the merge, or will be after the merge? Would you mind explaining a little further, please?

I was basing my view that there would be a functionality change on the manual, but in testing the current version (prior to this merge), I'm not sure that it works as I thought it did. The code below works on the current dev version, suggesting to me that at least in the case I've tested they are including the same things (which is all files and subfolders).

class TestOldInclude
    def test_old_style_include(self):
        """This compares a backup using --include-filelist with one using --include-globbing-filelist."""
        with open("testfiles/filelist.txt", 'w') as f:
            f.write("+ testfiles/select2\n"
                    "- testfiles/select")
        self.backup("full", "testfiles/", options=["--include-filelist=testfiles/filelist.txt"])
        self.restore()
        restore_dir = 'testfiles/restore_out'
        restored = self.directory_tree_to_list_of_lists(restore_dir + "/select2")
        self.backup("full", "testfiles/", options=["--include-globbing-filelist=testfiles/filelist.txt"])
        self.restore()
        restore_dir = 'testfiles/restore_out'
        restored2 = self.directory_tree_to_list_of_lists(restore_dir + "/select2")
        self.assertEqual(restored, restored2)
        print(restored)

I also just had a play around on my system version (0.6.21) to investigate the point from the manual:
"2. Include patterns do not match files in a directory that is included. So /usr/local in an include file will not match /usr/local/doc"

I created a test tree of:
===
usr:
clown local

usr/clown:

usr/local:
doc test1 test.txt

usr/local/doc:

usr/local/test1:
===

and a filelist of:
usr/local
- usr/clown

and ran:
duplicity --include-filelist=filelist.txt usr file://des/

On restoring it, my restore filetree is:
===
restore/:
local

restore/local:
doc test1 test.txt

restore/local/doc:

restore/local/test1:
===

So usr/local in my include file _does_ in fact include the file usr/local/doc in my backup.

Maybe I'm missing the point of what it was supposed to do differently, but obviously if it wasn't actually doing it, we don't need to worry about a functionality change. I would appreciate any thoughts that you have from your much greater knowledge of the code/project!