atomist-attic / artifact-source

ArtifactSource abstraction for source code
GNU General Public License v3.0
2 stars 3 forks source link

Externalise ArtifactSource filtering #15

Closed kipz closed 7 years ago

kipz commented 7 years ago

Problem:

TS projects will always have a .gitignore containing node_modules. However, this currently means that rug-cli will ignore this too, and not make it available to the compiler and not publish it. The current solution is to add !node_modules to .atomist/ignore. Although this works, this cause huge confusion because the expectation is always that the typescript compiler (TSC) will have nothing to do with .gitignore and will decide how/where to load modules from.

Additionally, this only currently works for FileSystemArtifactSource's, which will make it operating on Github repos tricky.

It also feels like it shouldn't be the responsibility of the ArtifactSource to decide what is filter and when.

Proposal:

Rather than complicating the model in this project further, @cdupuis and I discussed the following solution so that users of this library can have finer grained control over filtering.

This is a breaking change to the behaviour of this library...

alankstewart commented 7 years ago

I agree that filtering is not the responsibility of this library. Will begin making the changes. ArtifactSource already does have a filter method which takes in a DirFilter and FileFilter.

johnsonr commented 7 years ago

Just discussed with Alan. We should add ability to pass two functions to anything that returns an ArtifactSource: One that checks a directory path to see whether it (and its descendants) should be included in the ArtifactSource; and one that checks a file path.

The present filtering mechanism once an ArtifactSource has been constructed should remain unchanged.

Our existing .gitignore support should migrate to the former approach.

alankstewart commented 7 years ago

Callers can send in zero or more filters implementing the ArtifactFilter trait which will apply in order. I have removed JGit from artifact-source and am now using Spring's AntPathMatcher from spring-core. JGit did not filter out the .git directory and so now I created a separate filter called GitDirFilter which can be passed in to the list of filters when creating an ArtifactSource. There is also a GitignoreFileFilter and a AtomistIgnoreFileFilter.

@cdupuis , @kipz I haven't released 0.14.0 of artifact-source yet and I added some sample (commented-out) code to rug-cli which uses the new filters. You can pull artifact-source and use locally. There is no negation of patterns anymore like putting in !node_modules in .atomist/ignore and hoping it will be included if it is specified in .gitignore. These filters work independently so if you want previous behaviour of having to negate !node_modules in .atomist/ignore for example, you will have to write a new filter which handles both ignore files.

alankstewart commented 7 years ago

Sorry, last commits were incorrectly assigned to this issue. Anyhow, this should be complete now