Allow directories in cabal data-files (#713)

[hdgarrood]

This commit adds the capability to specify a whole directory which
contains data-files in a project's .cabal file.

If a data-files entry ends with a slash, like "static/", then it will
pull in all files under the directory named "static/".

Notes:

• This has only been tested by calling matchDirFileGlob in GHCi.
• I'm not sure about the name (data FileGlob = DirTrailingSlash --
  perhaps it should have Glob on the end like the others?) but I've been
  unable to come up with anything better.

[hdgarrood]

refs #713

[23Skidoo]

I think that the original idea with this feature was to add only a limited form of globbing and explicitly disallow including directories recursively because it makes it easy to include unwanted files.

[23Skidoo]

Alternatively, we could use zsh syntax (dir/**/*.js):

$ ls tests/**/*.hs
tests/data/executable/A.hs    tests/data/executable-lhs/B.hs     tests/data/executable-lhs-mutrec/B.hs     tests/data/executable/Main.hs      tests/data/executable-mutrec/C/C.hs
tests/data/executable/B.hs    tests/data/executable-lhs/C/C.hs   tests/data/executable-lhs-mutrec/C/C.hs   tests/data/executable-mutrec/A.hs  tests/data/executable-mutrec/Main.hs
tests/data/executable/C/C.hs  tests/data/executable-lhs/Main.hs  tests/data/executable-lhs-mutrec/Main.hs  tests/data/executable-mutrec/B.hs  tests/Tests.hs

[hdgarrood]

Ah yes, I think I prefer the zsh syntax; it seems to fit with the other types of globs better, since they refer to files, or groups of files, and not directories.

I'll wait until the spec is better defined ;)

[hdgarrood]

Another one to consider: foo/bar/**/*.{html,js} (should this be a separate package / added to System.Path.Glob?)

[tibbe]

What's the status of this? Are you waiting for a code review?

[hdgarrood]

I was waiting for the spec to become a bit clearer. Currently I'm not sure how this is feature is supposed to behave, or even if it's wanted at all.

[23Skidoo]

I vote for using the zsh syntax, but I think that we should still prohibit unlimited recursive globbing as it makes it easy to include unwanted files. We should force the user to specify either the full filename or extension. So foo/bar/**/* should be prohibited, but foo/bar/**/*.{html,js} and foo/bar/**/TEST_INPUT should work.

We could start by allowing ** only in the place of the final directory component in the pattern (example: foo/bar/**/*.ext) and later add support for placing it at any position (example: foo/bar/**/baz/*.ext). This should make this feature easier to implement.

@dcoutts, what do you think?

[tibbe]

Lets not get bogged down on syntax too much. Can we just pick the most commonly used one and stick with that? I'm not terribly worried that people will glob too much.

[hdgarrood]

What do you mean by most commonly used? The first thing that comes to mind when I hear 'globbing' is bash. With that in mind, I'd propose that:

• * can expand to a part of any file or directory but not descend into subdirectories
• ** is the same, except that it can descend into subdirectories.
• {a,b,c} can expand into a, b, or c.
• "Literal" and glob patterns may be mixed to an unlimited extent in a single directory level. That is, foo/**/* is allowed, and so is foo/**/*.hs, and foo/**/Test*.hs, and foo/**/*{Test,Spec}*.hs.

I think the the inconvenience of restricted globbing outweighs the benefit of reducing the risk of accidental file inclusions. It seems very unlikely that people would include sensitive data in public cabal project directories -- and so it also seems reasonable to claim that any accidentally included files would only be a minor inconvenience to users downloading a source distribution.

Here's a real use case: Currently I'm writing something which has to change its behaviour based on a file extension (https://github.com/hdgarrood/herringbone). Therefore, the tests have all kinds of data files with weird extensions, and the current behaviour makes including these in source distributions more difficult than it needs to be.

[tibbe]

I'm fine with bash globbing.

[dcoutts]

I didn't know bash had that feature. New in 4.0 with a thing to enable it.

Yes, we need to do something about the massive .cabal files people are creating because of the inability to concisely specify files to include. I'm happy with any reasonable & consistent syntax (so long as it is itself consistent with our existing syntax). Assuming bash style glob syntax fits that criteria then I'm happy with that.

Since it's new syntax we also have to check what older versions of Cabal do when they encounter it. We may have to only allow the new syntax with a cabal-version check.

[hdgarrood]

Ok, so the proposal for bash style globbing is separate to this, so shall I do some work on it and open another PR?

[hdgarrood]

Bash style globbing would also subsume #1343 I think

[hdgarrood]

Closing in favour of #2522.