subversion-plugin
subversion-plugin copied to clipboard
[JENKINS-35227] added support for AdditionalCredentials to SubversionSCMSource
This fixes issues with multibranch pipeline jobs and SVN externals. The main problem was that SubversionSCMSource did not allow to configure additional credentials, which caused "svn: E200015: ISVNAuthentication" errors. This patch fixes that. The second problem was that during "Scan Multibranch Pipeline" it was just checked whether the revision number of the SVN root directory changed. Thus it did not detect changes in externals. That was fixed by declaring revision number non-deterministic in SCMRevisionImpl, if additional credentials are configured. Then, Jenkins polls for changes in the whole repository.
This should fix JENKINS-35227 and JENKINS-32167. I am not able to post a comment in the bug reports, because I failed to create an account on jenkins.io.
@jglick thank you for your input. I've moved AdditionalCredentials back in place.
Regarding the non-deterministic revision, it was an easy solution (or workaround) for Jenkins not detecting changes in externals. Of course, the clean solution would be to extend SCMRevisionImpl. But to me it seems that it requires quite a lot of effort to get that done.
Would it be okay to have a checkbox in the config and let the user specify whether isDeterministic() should return true or false?
Would it help to remove the changes to SCMRevisionImpl and isDeterministic() from this PR to get it accepted? Then this PR would only cover AdditionalCredentials for SubversionSCMSource, i.e. for multibranch pipeline jobs, which would solve JENKINS-35227.
The reason why I changed SCMRevisionImpl is, because "Scan multibranch pipeline" is not working properly with externals. But as far as I see it, there is no JIRA issue for that problem yet. So maybe I should create an issue for that first.
So maybe I should create an issue for that first.
+1. An issue would be useful. The scope of this change is not that big, so probably we could have two fixes in a single PR
Separate pull requests would be better though
Okay, this PR is now only about additional credentials. Is it good enough to be accepted?
Yes, it looks much better now. I probably could live even without tests for it. @jglick WDYT?
Any news on this issue?
Any news on this issue? This fix would help me.
Still would benefit from tests. Probably ought to be migrated to traits in a newer scm-api
. Requires a maintainer to do this work I think.
When this will be released?