[JENKINS-70869] New option “clone” option to SCM-based retrievers to avoid workspace/xxx@libs/#55
Conversation
…-lib-plugin into SCMSourceRetriever.clone
src/main/java/org/jenkinsci/plugins/workflow/libs/SCMSourceRetriever.java
Outdated
Show resolved
Hide resolved
|
Could be merged independently but I did a bunch of follow-on changes relating to this in #57 and it seems more straightforward to just merge them together (pending review). |
…-lib-plugin into SCMSourceRetriever.clone
jglick
changed the title
SCMSourceRetriever.clone to avoid workspace/xxx@libs/workspace/xxx@libs/
jglick
changed the title
workspace/xxx@libs/workspace/xxx@libs/
|
|
||
| @DataBoundSetter public void setLibraryPath(String libraryPath) { | ||
| libraryPath = Util.fixEmptyAndTrim(libraryPath); | ||
| if (libraryPath != null && !libraryPath.endsWith("/")) { |
There was a problem hiding this comment.
NiT: For my own personal knowledge, why using a hardcoded String and / instead of File.separator or Path?
There was a problem hiding this comment.
Because this would be a relative path interpreted by FilePath and thus expected to use / regardless of the OS on which the controller runs.
| } | ||
|
|
||
| public String getLibraryPath() { | ||
| return libraryPath; |
There was a problem hiding this comment.
NiT: Can this eventually be part of an URL at some point of the usages? Like an API call somewhere in the execution stack?
There was a problem hiding this comment.
Not sure what you are asking. This is a getter matching setLibraryPath as required by databinding (so you can round-trip a form in which a library is already defined).
MarkEWaite
left a comment
MarkEWaite
left a comment
There was a problem hiding this comment.
Two hours of testing showed no issues with the cloning that I enabled with modern SCM and with legacy SCM. I've made an optional comment in one of the help files in case additional help might benefit users that enable caching.
I'd love to have an incremental build of this so that I can include it in my plugins.txt file.
src/main/resources/org/jenkinsci/plugins/workflow/libs/SCMBasedRetriever/help-clone.html
Show resolved
Hide resolved
| * | ||
| * <p>Used to prevent {@link #libraryPath} from being used for directory traversal. | ||
| */ | ||
| static final Pattern PROHIBITED_DOUBLE_DOT = Pattern.compile("(^|.*[\\\\/])\\.\\.($|[\\\\/].*)"); |
There was a problem hiding this comment.
I don't understand the left anchored | character in the regular expression. I'm accustomed to | as an OR expression combining two expressions where either can match.
In my interactive testing, all the patterns that I tried were correctly detected, so as far as I can tell, the regex is correct. However, I would like to understand the left anchored | character for my own benefit.
There was a problem hiding this comment.
Not sure offhand. Someone else wrote this for a security fix; I am merely moving it from
There was a problem hiding this comment.
Not part of a security fix, it was aded when libraryPath was added in jenkinsci/workflow-cps-global-lib-plugin#101.
(^|.*[\\\\/])- Either the start of the line, or any text followed by a directory separator
\\.\\.- Two dots
($|[\\\\/].*)- Either the end of the line, or a directory separator followed by any text
https://repo.jenkins-ci.org/incrementals/io/jenkins/plugins/pipeline-groovy-lib/649.v4a_8971b_1a_d8b_/ is getting published normally. I am not clear on what MarkEWaite/docker-lfs@a775455 was picking up, as jglick@c722f7acf8c2 does not exist. |
As far as I could tell from the jobs, 652 was in build 6 but not published as an incremental. I downloaded it from the build 6 artifacts and used it directly because it was not available as an incremental. I wanted to be sure that I was testing the most recent build of the pull request. Then I fell back to use 649 in plugins.txt because it was available as an incremental. That's what I'm running now, on the assumption that differences between 649 and 652 are not critical to the function of the pull request. |
Probably predated disabling of PR-merge mode. |
|
In case someone is looking for how to use this ( like me ) in jenkins system config, global shared lib config: config-as-code |
6 participants
JENKINS-70869
With this new option you can avoid locking and updating the
workspace/xxx@libs/subdirectory at the job level; instead, the SCM is checked out directly into the build directory. This is particularly practical in combination with Git shallow clones. Reduces disk usage somewhat; may avoid contention in heavily concurrent jobs (since no lock is needed during SCM operations); and is generally simpler, bypassing issues such as that solved by jenkinsci/workflow-cps-global-lib-plugin#115.Tested interactively with a large private library used by @cloudbees with >1600 commits and >200 files, using some combinations of both plain Git SCM source (ambient authn) and GitHub SCM source (PAT), with and without the new clone mode, with and without shallow clone option when using the new mode, and with and without caching. With clone mode on and using a shallow clone with GH PAT, could retrieve the lib for a build in a couple seconds in some cases (variable depending on GH response time), whereas a deep clone would typically take at least ten seconds.