From ba885b70221bf9caa2664dec59ffc9f4ac10c964 Mon Sep 17 00:00:00 2001 From: Martin Kosicky Date: Thu, 28 Jul 2022 22:17:14 +0200 Subject: [PATCH 1/8] New API for SCMFileSystem to allow to pass Run to be able to make parametrised lightweight checkout --- .../java/jenkins/scm/api/SCMFileSystem.java | 72 ++++++++++++++++--- 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 286c7bb4..99ec3435 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -187,7 +187,28 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm) throws IOE * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. */ @CheckForNull - public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) + public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm,@CheckForNull SCMRevision rev) throws IOException, InterruptedException { + return of(owner, scm, rev,null); + } + + + /** + * Given a {@link SCM} this method will try to retrieve a corresponding {@link SCMFileSystem} instance that + * reflects the content at the specified {@link SCMRevision}. + * + * @param owner the owner of the {@link SCM} + * @param scm the {@link SCM}. + * @param rev the specified {@link SCMRevision}. + * @param build the specified {@link Run}. + * @return the corresponding {@link SCMFileSystem} or {@code null} if there is none. + * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error + * (such as the remote system being unavailable) + * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. + */ + @CheckForNull + public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, + @CheckForNull SCMRevision rev, + @CheckForNull Run build) throws IOException, InterruptedException { Objects.requireNonNull(scm); SCMFileSystem fallBack = null; @@ -195,7 +216,15 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, @CheckForN for (Builder b : ExtensionList.lookup(Builder.class)) { if (b.supports(scm)) { try { - SCMFileSystem inspector = b.build(owner, scm, rev); + SCMFileSystem inspector; + if (b instanceof Builder2) + { + inspector = ((Builder2)b).build(owner,scm,rev, build); + } else + { + inspector = b.build(owner, scm, rev); + } + if (inspector != null) { if (inspector.isFixedRevision()) { return inspector; @@ -231,15 +260,15 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, @CheckForN /** * Given a {@link SCM} this method will check if there is at least one {@link SCMFileSystem} provider capable - * of being instantiated. Returning {@code true} does not mean that {@link #of(Item, SCM, SCMRevision)} + * of being instantiated. Returning {@code true} does not mean that {@link #of(Item, SCM, SCMRevision, Run)} * will be able to instantiate a {@link SCMFileSystem} for any specific {@link SCMRevision}, * rather returning {@code false} indicates that there is absolutely no point in calling - * {@link #of(Item, SCM, SCMRevision)} as it will always return {@code null}. + * {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. * * @param scm the {@link SCMSource}. - * @return {@code true} if {@link SCMFileSystem#of(Item, SCM)} / {@link #of(Item, SCM, SCMRevision)} could return a + * @return {@code true} if {@link SCMFileSystem#of(Item, SCM)} / {@link #of(Item, SCM, SCMRevision, Run)} could return a * {@link SCMFileSystem} implementation, {@code false} if {@link SCMFileSystem#of(Item, SCM)} / - * {@link #of(Item, SCM, SCMRevision)} will always return {@code null} for the supplied {@link SCM}. + * {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. * @since 2.0 */ public static boolean supports(@NonNull SCM scm) { @@ -350,13 +379,13 @@ public static boolean supports(@NonNull SCMSource source) { /** * Given a {@link SCMDescriptor} this method will check if there is at least one {@link SCMFileSystem} provider * capable of being instantiated from the descriptor's {@link SCMSource}. Returning {@code true} does not mean that - * {@link #of(Item, SCM, SCMRevision)} will be able to instantiate a {@link SCMFileSystem} for any specific + * {@link #of(Item, SCM, SCMRevision, Run)} will be able to instantiate a {@link SCMFileSystem} for any specific * {@link Item} or {@link SCMRevision}, rather returning {@code false} indicates that there is absolutely no point - * in calling {@link #of(Item, SCM, SCMRevision)} as it will always return {@code null}. + * in calling {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. * * @param descriptor the {@link SCMDescriptor}. - * @return {@code true} if {@link #of(Item, SCM, SCMRevision)} could return a {@link SCMFileSystem} implementation, - * {@code false} if {@link #of(Item, SCM, SCMRevision)} will always return {@code null} for the supplied {@link SCM}. + * @return {@code true} if {@link #of(Item, SCM, SCMRevision, Run)} could return a {@link SCMFileSystem} implementation, + * {@code false} if {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. * @since 2.3.0 */ public static boolean supports(@NonNull SCMDescriptor descriptor) { @@ -533,4 +562,27 @@ private boolean isEnclosedByDescribable(Descriptor descriptor) { return enclosingClass != null && descriptor.clazz.isAssignableFrom(enclosingClass); } } + + public abstract static class Builder2 extends Builder { + + /** + * Given a {@link SCM} this should try to build a corresponding {@link SCMFileSystem} instance that + * reflects the content at the specified {@link SCMRevision}. If the {@link SCM} is supported but not + * for a fixed revision, best effort is acceptable as the most capable {@link SCMFileSystem} will be returned + * to the caller. + * + * @param owner the owner of the {@link SCM} + * @param scm the {@link SCM}. + * @param rev the specified {@link SCMRevision}. + * @return the corresponding {@link SCMFileSystem} or {@code null} if this builder cannot create a {@link + * SCMFileSystem} for the specified {@link SCM}. + * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error + * (such as the remote system being unavailable) + * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. + */ + @CheckForNull + public abstract SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev, + Run build) + throws IOException, InterruptedException; + } } From ef72c98ddf20059f7c5e289ed46956da84536202 Mon Sep 17 00:00:00 2001 From: Martin Kosicky Date: Sun, 31 Jul 2022 11:50:09 +0200 Subject: [PATCH 2/8] Fixed javadoc --- .../java/jenkins/scm/api/SCMFileSystem.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 99ec3435..54c210d0 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -199,7 +199,7 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm,@CheckForNu * @param owner the owner of the {@link SCM} * @param scm the {@link SCM}. * @param rev the specified {@link SCMRevision}. - * @param build the specified {@link Run}. + * @param build the specified {@link Run}. * @return the corresponding {@link SCMFileSystem} or {@code null} if there is none. * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error * (such as the remote system being unavailable) @@ -260,15 +260,15 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, /** * Given a {@link SCM} this method will check if there is at least one {@link SCMFileSystem} provider capable - * of being instantiated. Returning {@code true} does not mean that {@link #of(Item, SCM, SCMRevision, Run)} + * of being instantiated. Returning {@code true} does not mean that {@link #of(Item, SCM, SCMRevision, Run)} * will be able to instantiate a {@link SCMFileSystem} for any specific {@link SCMRevision}, * rather returning {@code false} indicates that there is absolutely no point in calling - * {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. + * {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. * * @param scm the {@link SCMSource}. - * @return {@code true} if {@link SCMFileSystem#of(Item, SCM)} / {@link #of(Item, SCM, SCMRevision, Run)} could return a + * @return {@code true} if {@link SCMFileSystem#of(Item, SCM)} / {@link #of(Item, SCM, SCMRevision, Run)} could return a * {@link SCMFileSystem} implementation, {@code false} if {@link SCMFileSystem#of(Item, SCM)} / - * {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. + * {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. * @since 2.0 */ public static boolean supports(@NonNull SCM scm) { @@ -379,13 +379,13 @@ public static boolean supports(@NonNull SCMSource source) { /** * Given a {@link SCMDescriptor} this method will check if there is at least one {@link SCMFileSystem} provider * capable of being instantiated from the descriptor's {@link SCMSource}. Returning {@code true} does not mean that - * {@link #of(Item, SCM, SCMRevision, Run)} will be able to instantiate a {@link SCMFileSystem} for any specific + * {@link #of(Item, SCM, SCMRevision, Run)} will be able to instantiate a {@link SCMFileSystem} for any specific * {@link Item} or {@link SCMRevision}, rather returning {@code false} indicates that there is absolutely no point - * in calling {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. + * in calling {@link #of(Item, SCM, SCMRevision, Run)} as it will always return {@code null}. * * @param descriptor the {@link SCMDescriptor}. - * @return {@code true} if {@link #of(Item, SCM, SCMRevision, Run)} could return a {@link SCMFileSystem} implementation, - * {@code false} if {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. + * @return {@code true} if {@link #of(Item, SCM, SCMRevision, Run)} could return a {@link SCMFileSystem} implementation, + * {@code false} if {@link #of(Item, SCM, SCMRevision, Run)} will always return {@code null} for the supplied {@link SCM}. * @since 2.3.0 */ public static boolean supports(@NonNull SCMDescriptor descriptor) { From c47605389ae989a283865ce69ce3b6ebd32bb392 Mon Sep 17 00:00:00 2001 From: Martin Kosicky Date: Sun, 31 Jul 2022 18:13:47 +0200 Subject: [PATCH 3/8] Fixed javadoc --- src/main/java/jenkins/scm/api/SCMFileSystem.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 54c210d0..040fe396 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -569,11 +569,13 @@ public abstract static class Builder2 extends Builder { * Given a {@link SCM} this should try to build a corresponding {@link SCMFileSystem} instance that * reflects the content at the specified {@link SCMRevision}. If the {@link SCM} is supported but not * for a fixed revision, best effort is acceptable as the most capable {@link SCMFileSystem} will be returned - * to the caller. + * to the caller. If the {@link Run} is provided, it can be used to alter the behaviour of buil based on the current Run + * for example, expand branch name based on current build properties, etc. * * @param owner the owner of the {@link SCM} * @param scm the {@link SCM}. * @param rev the specified {@link SCMRevision}. + * @param build the specified {@link Run}. * @return the corresponding {@link SCMFileSystem} or {@code null} if this builder cannot create a {@link * SCMFileSystem} for the specified {@link SCM}. * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error @@ -582,7 +584,7 @@ public abstract static class Builder2 extends Builder { */ @CheckForNull public abstract SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev, - Run build) + @CheckForNull Run build) throws IOException, InterruptedException; } } From e69eaa1846efcc3dfb9b05db414e56749a061c88 Mon Sep 17 00:00:00 2001 From: Martin Kosicky Date: Mon, 1 Aug 2022 21:00:48 +0200 Subject: [PATCH 4/8] Remove unneccesary new Builder2 class --- .../java/jenkins/scm/api/SCMFileSystem.java | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 040fe396..e9845fe1 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -216,14 +216,7 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, for (Builder b : ExtensionList.lookup(Builder.class)) { if (b.supports(scm)) { try { - SCMFileSystem inspector; - if (b instanceof Builder2) - { - inspector = ((Builder2)b).build(owner,scm,rev, build); - } else - { - inspector = b.build(owner, scm, rev); - } + SCMFileSystem inspector = b.build(owner, scm, rev, build); if (inspector != null) { if (inspector.isFixedRevision()) { @@ -561,9 +554,6 @@ private boolean isEnclosedByDescribable(Descriptor descriptor) { Class enclosingClass = getClass().getEnclosingClass(); return enclosingClass != null && descriptor.clazz.isAssignableFrom(enclosingClass); } - } - - public abstract static class Builder2 extends Builder { /** * Given a {@link SCM} this should try to build a corresponding {@link SCMFileSystem} instance that @@ -575,7 +565,7 @@ public abstract static class Builder2 extends Builder { * @param owner the owner of the {@link SCM} * @param scm the {@link SCM}. * @param rev the specified {@link SCMRevision}. - * @param build the specified {@link Run}. + * @param _build the specified {@link Run}. * @return the corresponding {@link SCMFileSystem} or {@code null} if this builder cannot create a {@link * SCMFileSystem} for the specified {@link SCM}. * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error @@ -583,8 +573,12 @@ public abstract static class Builder2 extends Builder { * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. */ @CheckForNull - public abstract SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev, - @CheckForNull Run build) - throws IOException, InterruptedException; + public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev, + @CheckForNull Run _build) + throws IOException, InterruptedException + { + // if this is not overriden, fallback to the previous implementation + return build(owner,scm, rev); + } } } From ac59c48ce36f184ab4f74e5a7f359046dc2eb458 Mon Sep 17 00:00:00 2001 From: Martin Kosicky Date: Tue, 2 Aug 2022 10:39:36 +0200 Subject: [PATCH 5/8] Changed original method to public, so we dont have to override both implementations --- src/main/java/jenkins/scm/api/SCMFileSystem.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index e9845fe1..3f69de78 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -522,8 +522,11 @@ public final boolean supports(SCMSourceDescriptor descriptor) { * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. */ @CheckForNull - public abstract SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) - throws IOException, InterruptedException; + public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) + throws IOException, InterruptedException + { + throw new AbstractMethodError("Implement build(Item, SCM, SCMRevision,Run)"); + } /** * Given a {@link SCMSource}, a {@link SCMHead} and a {@link SCMRevision} this method should try to build a @@ -547,7 +550,7 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, if (owner == null) { throw new IOException("Cannot instantiate a SCMFileSystem from an SCM without an owner"); } - return build(owner, source.build(head, rev), rev); + return build(owner, source.build(head, rev), rev, null); } private boolean isEnclosedByDescribable(Descriptor descriptor) { From 8e5440567de6b077097c66f63c9eb4a68bb7402f Mon Sep 17 00:00:00 2001 From: MartinKosicky Date: Fri, 5 Aug 2022 17:06:52 +0200 Subject: [PATCH 6/8] Apply suggestions from code review (code cleanup) Co-authored-by: Jesse Glick --- src/main/java/jenkins/scm/api/SCMFileSystem.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 3f69de78..2ab704d3 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -187,8 +187,8 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm) throws IOE * @throws InterruptedException if the attempt to create a {@link SCMFileSystem} was interrupted. */ @CheckForNull - public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm,@CheckForNull SCMRevision rev) throws IOException, InterruptedException { - return of(owner, scm, rev,null); + public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) throws IOException, InterruptedException { + return of(owner, scm, rev, null); } @@ -199,7 +199,7 @@ public static SCMFileSystem of(@NonNull Item owner, @NonNull SCM scm,@CheckForNu * @param owner the owner of the {@link SCM} * @param scm the {@link SCM}. * @param rev the specified {@link SCMRevision}. - * @param build the specified {@link Run}. + * @param build an associated build context, if any, that could be used for example to look up parameters * @return the corresponding {@link SCMFileSystem} or {@code null} if there is none. * @throws IOException if the attempt to create a {@link SCMFileSystem} failed due to an IO error * (such as the remote system being unavailable) @@ -580,8 +580,8 @@ public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull @CheckForNull Run _build) throws IOException, InterruptedException { - // if this is not overriden, fallback to the previous implementation - return build(owner,scm, rev); + // if this is not overridden, fall back to the previous implementation + return build(owner, scm, rev); } } } From b46d83594ce76e6487773e00a3179125cbca102d Mon Sep 17 00:00:00 2001 From: MartinKosicky Date: Fri, 5 Aug 2022 17:45:21 +0200 Subject: [PATCH 7/8] Update src/main/java/jenkins/scm/api/SCMFileSystem.java Fixed signature Co-authored-by: Jesse Glick --- src/main/java/jenkins/scm/api/SCMFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 2ab704d3..87d0fc49 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -525,7 +525,7 @@ public final boolean supports(SCMSourceDescriptor descriptor) { public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) throws IOException, InterruptedException { - throw new AbstractMethodError("Implement build(Item, SCM, SCMRevision,Run)"); + throw new AbstractMethodError("Implement build(Item, SCM, SCMRevision, Run)"); } /** From b75bb5003f5a6b9e4f7021e2e64cb851c6fe362a Mon Sep 17 00:00:00 2001 From: MartinKosicky Date: Tue, 9 Aug 2022 09:30:33 +0200 Subject: [PATCH 8/8] Apply suggestions from code review. Improved javadoc and more helpful exception if caller uses builder directly Co-authored-by: Devin Nusbaum --- src/main/java/jenkins/scm/api/SCMFileSystem.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/jenkins/scm/api/SCMFileSystem.java b/src/main/java/jenkins/scm/api/SCMFileSystem.java index 87d0fc49..69378cea 100644 --- a/src/main/java/jenkins/scm/api/SCMFileSystem.java +++ b/src/main/java/jenkins/scm/api/SCMFileSystem.java @@ -525,7 +525,7 @@ public final boolean supports(SCMSourceDescriptor descriptor) { public SCMFileSystem build(@NonNull Item owner, @NonNull SCM scm, @CheckForNull SCMRevision rev) throws IOException, InterruptedException { - throw new AbstractMethodError("Implement build(Item, SCM, SCMRevision, Run)"); + throw new AbstractMethodError("Implement build(Item, SCM, SCMRevision, Run) and override this method to delegate to it"); } /** @@ -562,8 +562,8 @@ private boolean isEnclosedByDescribable(Descriptor descriptor) { * Given a {@link SCM} this should try to build a corresponding {@link SCMFileSystem} instance that * reflects the content at the specified {@link SCMRevision}. If the {@link SCM} is supported but not * for a fixed revision, best effort is acceptable as the most capable {@link SCMFileSystem} will be returned - * to the caller. If the {@link Run} is provided, it can be used to alter the behaviour of buil based on the current Run - * for example, expand branch name based on current build properties, etc. + * to the caller. If the {@link Run} is provided, it can be used to alter the behavior. + * For example, variables in the branch name can be expanded based on current build properties, etc. * * @param owner the owner of the {@link SCM} * @param scm the {@link SCM}.