Allow copy and delete of ant default exclude files

[RandomInEqualities]

Hi,

I recently had to setup at build machine and noticed that it is not possible to copy or delete .gitignore files. We are using this plugin for file operations, so it is a bit annoying and I thought I would try a pull request.

Currently, it is not possible to copy or delete files with the following names when using FileCopyOperation and FileDeleteOperation:

CVS .cvsignore SCCS vssver.scc .svn .DS_Store .git .gitattributes .gitignore .gitmodules .hg .hgignore .hgsub .hgsubstate .hgtags .bzr .bzrignore

Since the glob Ant pattern that FilePath::list and FilePath::copyRecursiveTo uses cannot pick up the files. This change changes to not use the default exclude list, so e.g. it is possible to copy and delete .gitignore files.

The Ant default exclude files are listed here: https://ant.apache.org/manual/dirtasks.html#defaultexcludes

Testing done

I have added two automated tests that fail before this change, and pass after the change.

Submitter checklist

• [ x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• [ x] Ensure that the pull request title represents the desired changelog entry
• [x ] Please describe what you did
• [x ] Link to relevant issues in GitHub or Jira
• [x ] Link to relevant pull requests, esp. upstream and downstream changes
• [x ] Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jonesbusy]

Hi,

Thanks for your PR.

From what I saw it's a breaking change for users.

Could we add an optional option to those steps to not use default exclude if needed ?

Thanks.

[RandomInEqualities]

Hi,

Year sure. I am not too familiar with Jenkins plugins. Would adding overloaded functions to FileOperationsJobDslContext work? Something like this:

? In the same way would you prefer to have the input to the FileCreateOperation, FileCopyOperation and FileTransformOperation as constructor parameters or setter functions?

[RandomInEqualities]

Ideally I think a plugin wide boolean check would make sense, if that is possible? So it can be turned on an off for the whole plugin and not per function.

[jonesbusy]

Hi,

I think the best is to use https://javadoc.jenkins.io/component/stapler/org/kohsuke/stapler/DataBoundSetter.html

Generally mandatory argument are passed on the constructor via https://javadoc.jenkins.io/component/stapler/org/kohsuke/stapler/DataBoundConstructor.html and optional parameter via DataBoundSetter

[RandomInEqualities]

I have pushed a change that tries to use the DataBoundSetter, while also adding a test for FileTransformOperation. I have not tried to start a jenkins server and test it out. I have only run this project locally and run the tests, so I am not 100% sure if everything is ok with these changes.

[jonesbusy]

The code looks good to me. I will try to perform some interactive tests in the next few days. Thanks

[jonesbusy]

I think we need to be careful when adding new field

https://www.jenkins.io/doc/developer/persistence/backward-compatibility/#add-a-new-field

The default value must be true if not existent when the objects is resurected from storage (a job with legacy format loaded).

I think we can achieve it by

...
private Boolean useDefaultExcludes;
...

protected Object readResolve() {
  if (useDefaultExcludes== null) {
       useDefaultExcludes = true
  }
  return this;
}

The default value on the constructor can be kept when the object is created from code and not from storage.

I cannot confirm 100% of this, but I think it's the way to go

[RandomInEqualities]

Year you were right, I added tests for the XStream serialization and indeed the readResolve was needed. It should be fixed now!

[jonesbusy]

Thanks for your PR, will be released in some minutes