[MCLEAN-111] Fix fast delete feature for filesets

[wilx]

Pass GlobSelector instance into cleaner.delete() only if it matters. Passing GlobSelector always breaks the fast delete feature.

Following this checklist to help us incorporate your
contribution quickly and easily:

• Make sure there is a JIRA issue filed
  for the change (usually before you start working on it). Trivial changes like typos do not
  require a JIRA issue. Your pull request should address just this issue, without
  pulling in other changes.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [MCLEAN-XXX] - Fixes bug in ApproximateQuantiles,
  where you replace MCLEAN-XXX with the appropriate JIRA issue. Best practice
  is to use the JIRA issue title in the pull request title and in the first line of the
  commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify to make sure basic checks pass. A more thorough check will
  be performed on your pull request automatically.
• You have run the integration tests successfully (mvn -Prun-its clean verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• In any other case, please file an Apache Individual Contributor License Agreement.

[wilx]

JIRA: https://issues.apache.org/jira/browse/MCLEAN-111

[slawekjaranowski]

@wilx thanks
Is it possible to make an IT to be sure for regressions with next changes?

[wilx]

@slawekjaranowski I have added the test.

[chabala]

Is this a symptom of GlobSelector not working on Windows at all?

[slawekjaranowski]

Is this a symptom of GlobSelector not working on Windows at all?

Good point - it should be also checked.

[wilx]

Is this a symptom of GlobSelector not working on Windows at all?

The issue is not with GlobSelector. The issue is that the GlobSelector instance passed into cleaner.delete() stops the fast method from taking effect:

maven-clean-plugin/src/main/java/org/apache/maven/plugins/clean/Cleaner.java

Lines 122 to 127 in 06876c1

	if (selector == null && !followSymlinks && fastDir != null && session != null) {
	// If anything wrong happens, we'll just use the usual deletion mechanism
	if (fastDelete(file)) {
	return;
	}
	}

[slawekjaranowski]

So it looks like when we use includes/excludes fastDelete will not works - correct?

[wilx]

So it looks like when we use includes/excludes fastDelete will not works - correct?

Yes and it makes sense. The fast delete moves the entire directory. If you want to delete only some files, you cannot move the directory because the files that should stay there would not.

[slawekjaranowski]

So it looks like when we use includes/excludes fastDelete will not works - correct?

Yes and it makes sense. The fast delete moves the entire directory. If you want to delete only some files, you cannot move the directory because the files that should stay there would not.

Yes make sense.

Can you update documentation?
https://maven.apache.org/plugins/maven-clean-plugin/examples/delete_additional_files.html
(source in project src/site/apt/examples/delete_additional_files.apt.vm)

We should explain such behaviors.

[chabala]

So it looks like when we use includes/excludes fastDelete will not works - correct?

Yes and it makes sense. The fast delete moves the entire directory. If you want to delete only some files, you cannot move the directory because the files that should stay there would not.

I agree. Looks like fast delete has always required GlobSelector to be null, since it was added in #6. So, it has never worked for this Fileset parsing path, only the directory parsing path above it at:

maven-clean-plugin/src/main/java/org/apache/maven/plugins/clean/CleanMojo.java

Lines 250 to 254 in f9da326

	for (File directoryItem : getDirectories()) {
	if (directoryItem != null) {
	cleaner.delete(directoryItem, null, followSymLinks, failOnError, retryOnError);
	}
	}

[wilx]

So it looks like when we use includes/excludes fastDelete will not works - correct?

Yes and it makes sense. The fast delete moves the entire directory. If you want to delete only some files, you cannot move the directory because the files that should stay there would not.

Yes make sense.

Can you update documentation? https://maven.apache.org/plugins/maven-clean-plugin/examples/delete_additional_files.html (source in project src/site/apt/examples/delete_additional_files.apt.vm)

We should explain such behaviors.

I have added a note to the delete_additional_files.apt.vm.

[slawekjaranowski]

@wilx - thanks for fix