[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

MartinKosicky · 2022-08-02T11:20:12Z

I added in CpsScmFlowDefinition (create) to send to lightweight checkout the build value into SCMFileSystem.of which is changed in jenkinsci/scm-api-plugin#160. The goal is to be able to expand branch value by build params if branch name contains for example ${BRANCH}

jglick

Ought to include an integration test picking up jenkinsci/git-plugin#1305 and demonstrating effectiveness in solving the stated bug.

pom.xml

MartinKosicky · 2022-08-02T18:49:09Z

Ought to include an integration test picking up jenkinsci/git-plugin#1305 and demonstrating effectiveness in solving the stated bug.

Im planning to add integration test here and unit test in Git plugin

MartinKosicky · 2022-08-04T09:31:22Z

Ought to include an integration test picking up jenkinsci/git-plugin#1305 and demonstrating effectiveness in solving the stated bug.

Done

src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinitionTest.java

MartinKosicky · 2022-08-04T13:16:30Z

hmm, are these check fails related to this PR ?

jglick · 2022-08-04T14:21:26Z

are these check fails related to this PR ?

From a quick glance, looks like infrastructure problems with Windows agents; retriggered build.

MartinKosicky · 2022-08-05T05:38:16Z

are these check fails related to this PR ?

From a quick glance, looks like infrastructure problems with Windows agents; retriggered build.

Now there is a bunch of
No test report files were found. Configuration error?
and
Caused: java.io.IOException: Remote call on JNLP4-connect connection from 20.102.0.182/20.102.0.182:11009 failed

:(

jglick · 2022-08-05T12:28:46Z

https://ci.jenkins.io/job/Plugins/job/workflow-cps-plugin/job/PR-577/10/execution/node/53/ is https://issues.jenkins.io/browse/JENKINS-69229

jglick · 2022-08-05T12:54:14Z

OK, that much seems to be fixed, sorry for the interruption. @dduportal FYI

MartinKosicky · 2022-08-05T13:22:26Z

Ok this is testCase I was working on but not that test, hmm. Locally they were all sucesful. I will assume that I broke something?

pom.xml

jglick · 2022-08-05T14:22:39Z

src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java

@@ -115,7 +115,7 @@ public boolean isLightweight() {
        Run<?,?> build = (Run<?,?>) _build;
        String expandedScriptPath = build.getEnvironment(listener).expand(scriptPath);
        if (isLightweight()) {
-            try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm)) {
+            try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm,null, build)) {


Suggested change

try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm,null, build)) {

try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm, null, build)) {

src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinitionTest.java

jglick · 2022-08-05T14:27:34Z

Edit docs: #329 (comment)

MartinKosicky · 2022-08-11T08:48:43Z

hmm, ill try to bump the versions?

MartinKosicky · 2022-08-11T10:19:12Z

@jglick do you have any suggestion if I should bump the versions in git plugin? or what should we do here ?

MartinKosicky · 2022-08-11T13:11:02Z

When I import the incremental build of git plugin to my Idea, I see different versions than those that were specified in the incremental .pom file. Also the test seems to look at the pom definitions? Or I dont know where it knows the versions, but I see that maven loaded a different version than was generated in the .pom

jglick · 2022-08-11T14:21:20Z

When I import the incremental build of git plugin to my Idea, I see different versions

You may need to configure your IDE to honor

workflow-cps-plugin/.mvn/maven.config

Line 1 in 9101f1b

-Pconsume-incrementals

NetBeans does this automatically; I have no idea about IDEA (sorry for the pun). If in doubt, command-line mvn is authoritative.

Failed to load: Git plugin (git 4.12.0-rc4821.0fa_d6b_a_a_0928)
 - Update required: Jenkins Mailer Plugin (mailer 414.vcc4c33714601) to be updated to 435.v79ef3972b_5c7 or higher
 - Update required: Token Macro Plugin (token-macro 293.v283932a_0a_b_49) to be updated to 308.v4f2b_ed62b_b_16 or higher
 - Update required: Structs Plugin (structs 318.va_f3ccb_729b_71) to be updated to 324.va_f5d6774f3a_d or higher

possibly just means that this should subsume #574.

MartinKosicky · 2022-08-11T14:26:47Z

Yes I am taking the incrementals, i saw this behaviour also in mvn dependency tree to confirm.

MartinKosicky · 2022-12-16T19:44:29Z

hmm the build seemed to completely change , now i have to figure out how to build this thing in idea :(

ok i seemed to broke the history by some dump rebase , eh

pom.xml

fix newline Co-authored-by: Devin Nusbaum <[email protected]>

MartinKosicky · 2022-12-17T07:32:02Z

I think it shouls be ok now

MartinKosicky · 2022-12-20T09:56:43Z

@MarkEWaite hello, will you have time so we can finalize these 3 PRs. This one is the last and I think you also tested this already when we were working with the git-scm

MarkEWaite · 2022-12-20T20:56:31Z

I've added it to the plugins.txt file that I'm using for my testing of git plugin 5.0.0 and git client plugin 4.0.0. I'll include the details of JENKINS-42971 in my testing.

MarkEWaite · 2022-12-20T23:12:30Z

I've confirmed that the incremental release of this pull request resolves JENKINS-42971. I tested with the git plugin 5.0.0 pre-release and the git client plugin 4.0.0 pre-release.

MarkEWaite

Thanks! Approved

MartinKosicky · 2022-12-21T10:33:18Z

Thanks! Approved

Thank you :). Hmm just a quick question, how does the merging process work here? Who will merge this or we need more approvals?

MarkEWaite · 2022-12-21T13:04:41Z

Thanks! Approved

Thank you :). Hmm just a quick question, how does the merging process work here? Who will merge this or we need more approvals?

One of the maintainers needs to merge it. Since two of them have approved the pull request, I believe it is ready for merge. Once it is merged, the continuous delivery process will release a new version of the plugin.

basil · 2022-12-21T17:08:53Z

Note that in this project, being ready for merge is no guarantee that a pull request will be merged in a timely fashion. I routinely file PRs that are approved by maintainers with the green checkmark yet remain unmerged (and therefore unreleased) for an extended period of time.

jglick

Seems OK though I have not followed details recently. @dwnusbaum was most recently reviewing seriously; not sure if he is around to check latest changes.

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinitionTest.java

jglick · 2022-12-21T17:29:53Z

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinitionTest.java

+        def.setLightweight(true);
+        p.setDefinition(def);
+
+        r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("BRANCH", "master2"))));


Do you mean to also assert that it actually loaded the specified revision of flow.groovy, for example by checking the altered message?

FTR with main reverted, the failure is as expected

java.lang.AssertionError: unexpected build status; build log was: ------ Started hudson.plugins.git.GitException: Command "git fetch --tags --force --progress --prune -- origin +refs/heads/${BRANCH}:refs/remotes/origin/${BRANCH}" returned status code 128: stdout: stderr: fatal: couldn't find remote ref refs/heads/${BRANCH} at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:2734) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandWithCredentials(CliGitAPIImpl.java:2111) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.access$500(CliGitAPIImpl.java:87) at org.jenkinsci.plugins.gitclient.CliGitAPIImpl$1.execute(CliGitAPIImpl.java:623) at jenkins.plugins.git.GitSCMFileSystem$BuilderImpl.build(GitSCMFileSystem.java:406) at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:219) at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:191) at jenkins.scm.api.SCMFileSystem.of(SCMFileSystem.java:174) at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:118) at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition.create(CpsScmFlowDefinition.java:70) at org.jenkinsci.plugins.workflow.job.WorkflowRun.run(WorkflowRun.java:312) at hudson.model.ResourceController.execute(ResourceController.java:107) at hudson.model.Executor.run(Executor.java:449) Finished: FAILURE ------ Expected: is <SUCCESS> but: was <FAILURE> at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20) at org.jvnet.hudson.test.JenkinsRule.assertBuildStatus(JenkinsRule.java:1438) at org.jvnet.hudson.test.JenkinsRule.assertBuildStatus(JenkinsRule.java:1444) at org.jvnet.hudson.test.JenkinsRule.assertBuildStatusSuccess(JenkinsRule.java:1472) at org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinitionTest.lightweight_brach_parametrised(CpsScmFlowDefinitionTest.java:222)

Would just be nice to strengthen the test to prove that it was really running from the correct branch, and not merely that the build passed.

Hmm but on 4th august you wrote that :

jglick on Aug 4
Probably a more straightforward way to assert this. Or just delete entirely—the fact that the build ran at all (when the repo only contains the master2 branch) is proof enough that the fix worked, right?

I was parsing the log output before, or did i misunderstood the conversation before...

Sigh, this is what happens when months go by between review comments. Fine enough as it is.

dwnusbaum · 2022-12-21T17:38:08Z

@dwnusbaum was most recently reviewing seriously

No, my recent review was just to try to help resolve #577 (comment) and a bad merge related to #612, which is something I worked on. I have not looked at this PR beyond that.

MarkEWaite · 2022-12-30T15:42:33Z

I found no issues related to this change in my testing of git plugin 5.0.0 and git client plugin 4.0.0. I would like to see it merged and released. Thanks for your patience @MartinKosicky !

Most recent build from pull request 577 jenkinsci/workflow-cps-plugin#577

Does not include the expansion of environment variables in lightweight checkout. jenkinsci/workflow-cps-plugin#577 is the lightweight checkout pull request.

MartinKosicky · 2023-01-04T07:02:50Z

Thank you :)

jglick · 2023-01-04T13:24:27Z

Thank you for driving this forward and for your patience.

MartinKosicky marked this pull request as draft August 2, 2022 11:20

MartinKosicky mentioned this pull request Aug 2, 2022

[JENKINS-42971] New API for SCMFileSystem to allow for lightweight checkout with build params jenkinsci/scm-api-plugin#160

Merged

jglick reviewed Aug 2, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

pom.xml Outdated Show resolved Hide resolved

jglick reviewed Aug 4, 2022

View reviewed changes

MartinKosicky force-pushed the use_new_scp_api_lw_checkout branch 2 times, most recently from 1f065b7 to ea74d85 Compare August 5, 2022 05:13

MartinKosicky force-pushed the use_new_scp_api_lw_checkout branch from ea74d85 to cc23daa Compare August 5, 2022 10:36

MartinKosicky force-pushed the use_new_scp_api_lw_checkout branch 2 times, most recently from 5e5aea3 to 7239bbe Compare August 5, 2022 14:01

jglick added the enhancement label Aug 5, 2022

jglick approved these changes Aug 5, 2022

View reviewed changes

jglick mentioned this pull request Aug 5, 2022

JENKINS-48431 Support both lightweight checkout AND build parameters #329

Closed

jglick requested review from MarkEWaite, car-roll and dwnusbaum August 5, 2022 15:15

MartinKosicky marked this pull request as ready for review August 5, 2022 17:04

MartinKosicky force-pushed the use_new_scp_api_lw_checkout branch from 4ee0b63 to 54d0770 Compare December 16, 2022 19:43

dwnusbaum reviewed Dec 16, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

Fix of bad merge

1073bfa

dwnusbaum reviewed Dec 16, 2022

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

Update pom.xml

24ad926

fix newline Co-authored-by: Devin Nusbaum <[email protected]>

MartinKosicky marked this pull request as ready for review December 16, 2022 20:26

MartinKosicky requested a review from a team as a code owner December 16, 2022 20:26

MarkEWaite approved these changes Dec 20, 2022

View reviewed changes

olamy approved these changes Dec 21, 2022

View reviewed changes

jglick requested a review from dwnusbaum December 21, 2022 17:27

jglick approved these changes Dec 21, 2022

View reviewed changes

MarkEWaite added a commit to MarkEWaite/docker-lfs that referenced this pull request Dec 30, 2022

Use workflow cps plugin 3599.v6c6b_68cd1d60

b2765d3

Most recent build from pull request 577 jenkinsci/workflow-cps-plugin#577

jglick added 2 commits January 3, 2023 17:50

Merge branch 'master' into use_new_scp_api_lw_checkout

2731d65

Typo in test name

eb24fb7

jglick approved these changes Jan 3, 2023

View reviewed changes

jglick enabled auto-merge January 3, 2023 22:51

jglick merged commit 9b36ad9 into jenkinsci:master Jan 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

MartinKosicky commented Aug 2, 2022

jglick left a comment

MartinKosicky commented Aug 2, 2022 •

edited

Loading

MartinKosicky commented Aug 4, 2022

MartinKosicky commented Aug 4, 2022

jglick commented Aug 4, 2022

MartinKosicky commented Aug 5, 2022

jglick commented Aug 5, 2022

jglick commented Aug 5, 2022

MartinKosicky commented Aug 5, 2022

jglick Aug 5, 2022

jglick commented Aug 5, 2022

MartinKosicky commented Aug 11, 2022 •

edited

Loading

MartinKosicky commented Aug 11, 2022

MartinKosicky commented Aug 11, 2022 •

edited

Loading

jglick commented Aug 11, 2022

MartinKosicky commented Aug 11, 2022

MartinKosicky commented Dec 16, 2022 •

edited

Loading

MartinKosicky commented Dec 17, 2022

MartinKosicky commented Dec 20, 2022

MarkEWaite commented Dec 20, 2022

MarkEWaite commented Dec 20, 2022

MarkEWaite left a comment

MartinKosicky commented Dec 21, 2022

MarkEWaite commented Dec 21, 2022

basil commented Dec 21, 2022

jglick left a comment

jglick Dec 21, 2022

jglick Dec 21, 2022

MartinKosicky Dec 28, 2022 •

edited

Loading

jglick Jan 3, 2023

dwnusbaum commented Dec 21, 2022

MarkEWaite commented Dec 30, 2022

MartinKosicky commented Jan 4, 2023

jglick commented Jan 4, 2023

	try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm,null, build)) {
	try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm, null, build)) {

[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

[JENKINS-42971] CpsScmFlowDefinition sends build to SCMFileSystem in … #577

Conversation

MartinKosicky commented Aug 2, 2022

jglick left a comment

Choose a reason for hiding this comment

MartinKosicky commented Aug 2, 2022 • edited Loading

MartinKosicky commented Aug 4, 2022

MartinKosicky commented Aug 4, 2022

jglick commented Aug 4, 2022

MartinKosicky commented Aug 5, 2022

jglick commented Aug 5, 2022

jglick commented Aug 5, 2022

MartinKosicky commented Aug 5, 2022

jglick Aug 5, 2022

Choose a reason for hiding this comment

jglick commented Aug 5, 2022

MartinKosicky commented Aug 11, 2022 • edited Loading

MartinKosicky commented Aug 11, 2022

MartinKosicky commented Aug 11, 2022 • edited Loading

jglick commented Aug 11, 2022

MartinKosicky commented Aug 11, 2022

MartinKosicky commented Dec 16, 2022 • edited Loading

MartinKosicky commented Dec 17, 2022

MartinKosicky commented Dec 20, 2022

MarkEWaite commented Dec 20, 2022

MarkEWaite commented Dec 20, 2022

MarkEWaite left a comment

Choose a reason for hiding this comment

MartinKosicky commented Dec 21, 2022

MarkEWaite commented Dec 21, 2022

basil commented Dec 21, 2022

jglick left a comment

Choose a reason for hiding this comment

jglick Dec 21, 2022

Choose a reason for hiding this comment

jglick Dec 21, 2022

Choose a reason for hiding this comment

MartinKosicky Dec 28, 2022 • edited Loading

Choose a reason for hiding this comment

jglick Jan 3, 2023

Choose a reason for hiding this comment

dwnusbaum commented Dec 21, 2022

MarkEWaite commented Dec 30, 2022

MartinKosicky commented Jan 4, 2023

jglick commented Jan 4, 2023

MartinKosicky commented Aug 2, 2022 •

edited

Loading

MartinKosicky commented Aug 11, 2022 •

edited

Loading

MartinKosicky commented Aug 11, 2022 •

edited

Loading

MartinKosicky commented Dec 16, 2022 •

edited

Loading

MartinKosicky Dec 28, 2022 •

edited

Loading