-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow copy and delete of ant default exclude files #105
Conversation
5b95638
to
d23effc
Compare
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. |
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? |
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. |
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 |
d23effc
to
dc2b240
Compare
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. |
The code looks good to me. I will try to perform some interactive tests in the next few days. Thanks |
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 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflict to resolve
dc2b240
to
0ce774c
Compare
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 allows disabling the default exclude list when copying transforming and deleting files. To allow e.g. deleting a .gitignore file. The Ant default exclude files are listed here: https://ant.apache.org/manual/dirtasks.html#defaultexcludes
0ce774c
to
b5c3e8f
Compare
Year you were right, I added tests for the XStream serialization and indeed the readResolve was needed. It should be fixed now! |
Thanks for your PR, will be released in some minutes |
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