[JENKINS-38669] (1/3) `FlowDefinition.getSCMs` #111

AngryGami · 2019-11-27T08:59:12Z

Please see https://issues.jenkins-ci.org/browse/JENKINS-38669 for explanation

Downstream PRs:

AngryGami · 2019-11-28T11:27:29Z

Will anyone comment anything on that?

dwnusbaum

Thanks! I think something like this would be fine, but I'd like to see the rest of the PRs to make sure everything works together, and I'd like to see a bit of Javadoc for a new public API.

To create PRs that can use this code without it being merged, you can use the incremental build of this PR, which can be seen here. You can use this build in other plugins by depending on workflow-api:2.38-rc974.2eb15c40fbf9, and then file a PR that depends on that version, and so on, so that we can see everything linked together along with an integration test in workflow-job or git that shows that the issue has been fixed.

src/main/java/org/jenkinsci/plugins/workflow/flow/FlowDefinition.java

dwnusbaum · 2019-12-04T15:17:29Z

@AngryGami You only need to bump the versions in the PR where you got the upper bounds enforcer error, not in this PR or in workflow-cps.

AngryGami · 2019-12-04T15:26:16Z

Dependencies in workflow-job are and were up to date. You can check yourself:
https://github.com/AngryGami/workflow-job-plugin/blob/a6130a3d9511c697b9eacd3a677517e9a0ee75b8/pom.xml#L80
https://github.com/AngryGami/workflow-job-plugin/blob/a6130a3d9511c697b9eacd3a677517e9a0ee75b8/pom.xml#L95
From enforcer log it looks like that it see transient dependencies and bark on them.
And now this fails... wtf :)

AngryGami · 2019-12-04T15:29:22Z

Just for reference:
https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fworkflow-job-plugin/detail/PR-147/1/pipeline#step-39-log-37
https://ci.jenkins.io/blue/organizations/jenkins/Plugins%2Fworkflow-job-plugin/detail/PR-147/1/pipeline#step-39-log-85

dwnusbaum · 2019-12-04T15:32:42Z

@AngryGami No, the latest versions of workflow-step-api and script-security mentioned in the upper bounds errors are workflow-step-api:2.21 and script-security:1.63, and as of your PR, workflow-job is using 2.20 and 1.62, so they need to be updated.

Updating the dependencies here fails because workflow-step-api:2.20 depends on structs:1.18, but right now this plugin only depends on structs:1.17. (This might also affect workflow-job once you fix the first two errors)

Upper bounds dependencies errors just mean that the current artifact has a version requirement on a dependency that is older than a transitive version requirement on that same dependency, or conflicting transitive version requirements. In all cases, you just need to update the direct version requirement to be the most recent version mentioned in all of the conflicts, or newer.

AngryGami · 2019-12-04T15:33:51Z

Ah.. this is not obvious that enforcer need version bigger that it barks about. Ok.

dwnusbaum · 2019-12-04T15:36:09Z

Yes, for an error like this:

Require upper bound dependencies error for org.jenkins-ci.plugins:structs:1.17 paths to dependency are:
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins:structs:1.17
  and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins.workflow:workflow-step-api:2.20
    +-org.jenkins-ci.plugins:structs:1.18 **USE THIS ONE**
    and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins:scm-api:2.2.6
    +-org.jenkins-ci.plugins:structs:1.9
    and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins.workflow:workflow-cps:2.58
    +-org.jenkins-ci.plugins:structs:1.17
    and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins.workflow:workflow-basic-steps:2.3
    +-org.jenkins-ci.plugins:structs:1.4
    and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins.workflow:workflow-durable-task-step:2.22
    +-org.jenkins-ci.plugins:structs:1.10
    and
+-org.jenkins-ci.plugins.workflow:workflow-api:2.38-rc981.82f7aea37565
  +-org.jenkins-ci.plugins.workflow:workflow-support:2.21
    +-org.jenkins-ci.plugins.workflow:workflow-api:2.30
      +-org.jenkins-ci.plugins:structs:1.14

You have to look through and find the most recent version from all of the conflicts, it's not necessarily the first version listed. This all assumes that the most recent version is backwards-compatible and you can update successfully, and for Jenkins plugins that should be the case most of the time.

dwnusbaum · 2019-12-04T15:37:51Z

But again, there's not really any need to update workflow-step-api, script-security, or structs here, although assuming they don't require Jenkins core updates or anything, it should be harmless.

AngryGami · 2019-12-04T15:38:41Z

But build failed? How can I ignore that?

dwnusbaum · 2019-12-04T15:40:23Z

Yeah, the workflow-job PR build failed, so you need to go update dependencies in that PR. The build for this PR was fine as of eb77dfc, it only started failing when you updated the other dependencies, so you could revert those updates instead of updating them and fixing all of the resulting upper bounds issues.

AngryGami · 2019-12-04T15:55:08Z

I don't understand how and what I need to revert? And how this would help? As far as I understand changes from my PR get applied to latest state of master branch of main repository right before build and if I haven't touched anything related to versions how these become out of sync and fail build? Assuming that state of main repo is always buildable.

dwnusbaum · 2019-12-04T16:09:27Z

I don't understand how and what I need to revert? And how this would help?

Here is what I mean, looking at the commits in this PR in order:

2eb15c4 : PR is fine here.
eb77dfc: PR is fine here.
e290ec5: PR is not fine. You updated dependencies, thinking that was required to fix the workflow-job errors, but that was not the case. This PR was fine before this change, you only needed to change the workflow-job PR. This change introduced enforcer errors because the new version of workflow-step-api requires structs:1.18, but this plugin had a direct dependency on structs:1.17, so the enforcer fails the build and tells you to resolve the conflict, listing all of the paths to the conflicting dependency and what minimum version is required by each path.
4832389: PR is fine here. You resolved the conflict by updating to structs:1.18.

What I meant is that the dependency updates in e290ec5 and 4832389 are unrelated to this PR, and do not do anything to help the workflow-job PR build pass, so you could revert those commits. You don't necessarily have to, since they are harmless, but they are not related to the changes you are trying to make. You can tell that these commits are unrelated to helping the workflow-job PR pass, because you are still using the incremental build based on commit 2eb15c4 in that PR (the version is 2.38-rc974.2eb15c40fbf9), so that PR doesn't see commits eb77dfc, e290ec5, or 4832389 from this PR right now, it is only using the first commit.

if I haven't touched anything related to versions how these become out of sync and fail build

You did change something related to versions: you updated dependencies.

AngryGami · 2019-12-04T16:19:47Z

I understand that my hasty fixes to dependencies are irrelevant to meat of this and other two PRs. I would prefer to get rid of them if possible even that they are harmless but I don't know how.

You did change something related to versions: you updated dependencies.

Could you please show me where?
To be precise - I'm talking about initial PR for workflow-job plugin. How it manage to fail build even that only changes it contains are relevant to my goal and I didn't touch neither script-security or workflow-step-api dependencies?

dwnusbaum · 2019-12-04T17:09:29Z

Could you please show me where?
To be precise - I'm talking about initial PR for workflow-job plugin. How it manage to fail build even that only changes it contains are relevant to my goal and I didn't touch neither script-security or workflow-step-api dependencies?

Ah ok, sorry, I will try to clarify. Here are the paths to the relevant dependencies from workflow-job (you can use mvn dependency:tree to investigate this if you want):

master

workflow-step-api:2.20
script-security:1.62
workflow-api:2.36
- workflow-step-api:2.16 (older version than shortest path to this dependency, so no conflict)
- script-security:1.46 (older version than shortest path to this dependency, no conflict)
workflow-cps:2.74
- workflow-step-api:2.20 (same version as shortest path to this dependency, so no conflict)
- script-security:1.62 (same version as shortest path to this dependency, so no conflict)

So on master, the shortest paths to the dependencies require newer versions or the same version s as any other paths to the dependencies, the enforcer is happy, and there are no conflicts.

PR 147

workflow-step-api:2.20
script-security:1.62
workflow-api:2.38-rc974.2eb15c40fbf9 (includes updates from 2.37, but notably, that version did not include any dependency updates)
- workflow-step-api:2.16 (older version than shortest path to this dependency, so no conflict)
- script-security:1.46 (older version than shortest path to this dependency, no conflict)
workflow-cps:2.78-rc2461.89fa3a91a5e8 (includes updates from 2.75, 2.76, and 2.77! Those updates are the source of the problem):
- workflow-step-api:2.21 (newer than the shortest path to the dependency, causes an upper bounds error! This dependency was updated in version 2.77 of the plugin by Pick up DynamicContext as nonbeta workflow-cps-plugin#327)
- script-security:1.63 (newer than the shortest path to the dependency, causes an upper bounds error! This dependency was updated in version 2.75 of the plugin by Add regression tests for SECURITY-1538 workflow-cps-plugin#322)

So in your PR, you updated workflow-cps to an incremental release based on 2.77+your PR, but master was using workflow-cps 2.74 previously, so all of the updates in 2.75, 2.76, and 2.77 are new, and cause the conflict in your PR.

To fix it, all you have to do is update your workflow-job PR so that the shortest paths to workflow-step-api and script-security use either newer versions or the same versions out of all paths to the dependencies (which you already did).

In general, upper bounds errors are annoying, but pretty common in Jenkins where a lot of plugins have the same dependencies and/or depend on each other for testing purposes.

AngryGami · 2019-12-04T17:46:56Z

Ok, I see now. Though was there any way to avoid that? I mean, how on earth I would have known that workflow-cps master branch is not compatible (meaning from enforcer checker point of view) with workflow-job master branch? Should then I have used some other branch of workflow-cps to start my PR from?

dwnusbaum · 2019-12-04T18:04:23Z

Though was there any way to avoid that? I mean, how on earth I would have known that workflow-cps master branch is not compatible (meaning from enforcer checker point of view) with workflow-job master branch?

No, what you did was the right thing to do. I think the problem in this case is just that the upper bounds error message is very technical, and looks like something really bad has happened, when really it is a super common situation, and the error should probably say something more helpful like:

[ERROR] There is a transitive dependency on workflow-step-api:2.21 through workflow-cps:2.78-foobar, but this artifact depends on workflow-step-api:2.20.
[ERROR] Consider updating to workflow-step-api:2.21 to satisfy the transitive dependency.
[ERROR] Run mvn foobar for more details

AngryGami · 2019-12-04T19:35:17Z

Thank you. This is enough information :)

bitwiseman · 2021-06-10T23:08:33Z

@AngryGami Sorry for letting this fall through the cracks. Thanks for the PR, I'll be merging the downstream soon.

JENKINS-38669 first stage - adding getSCMs method to FlowDefinition

2eb15c4

dwnusbaum reviewed Dec 2, 2019

View reviewed changes

src/main/java/org/jenkinsci/plugins/workflow/flow/FlowDefinition.java Show resolved Hide resolved

dwnusbaum requested review from dwnusbaum and bitwiseman December 2, 2019 16:03

added javadoc to getSCMs method

eb77dfc

This was referenced Dec 4, 2019

[JENKINS-38669] (2/3) CpsScmFlowDefinition.getSCMs jenkinsci/workflow-cps-plugin#341

Merged

[JENKINS-38669] (3/3) Call FlowDefinition.getSCMs jenkinsci/workflow-job-plugin#147

Merged

bump workflow-step-api and script-security versions

e290ec5

bump structs versions

4832389

bitwiseman approved these changes Dec 18, 2019

View reviewed changes

dwnusbaum approved these changes Dec 19, 2019

View reviewed changes

Merge branch 'master' into master

0c00855

bitwiseman merged commit ed2467f into jenkinsci:master Jun 10, 2021

jglick changed the title ~~JENKINS-38669 first stage - adding getSCMs method to FlowDefinition~~ [JENKINS-38669] (1/3) FlowDefinition.getSCMs Dec 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-38669] (1/3) `FlowDefinition.getSCMs` #111

[JENKINS-38669] (1/3) `FlowDefinition.getSCMs` #111

AngryGami commented Nov 27, 2019 •

edited by dwnusbaum

Loading

AngryGami commented Nov 28, 2019

dwnusbaum left a comment

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 •

edited

Loading

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 •

edited

Loading

AngryGami commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019 •

edited

Loading

AngryGami commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

bitwiseman commented Jun 10, 2021

[JENKINS-38669] (1/3) FlowDefinition.getSCMs #111

[JENKINS-38669] (1/3) FlowDefinition.getSCMs #111

Conversation

AngryGami commented Nov 27, 2019 • edited by dwnusbaum Loading

AngryGami commented Nov 28, 2019

dwnusbaum left a comment

Choose a reason for hiding this comment

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 • edited Loading

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 • edited Loading

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

dwnusbaum commented Dec 4, 2019 • edited Loading

AngryGami commented Dec 4, 2019 • edited Loading

dwnusbaum commented Dec 4, 2019 • edited Loading

AngryGami commented Dec 4, 2019 • edited Loading

dwnusbaum commented Dec 4, 2019

AngryGami commented Dec 4, 2019

bitwiseman commented Jun 10, 2021

[JENKINS-38669] (1/3) `FlowDefinition.getSCMs` #111

[JENKINS-38669] (1/3) `FlowDefinition.getSCMs` #111

AngryGami commented Nov 27, 2019 •

edited by dwnusbaum

Loading

dwnusbaum commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019 •

edited

Loading

AngryGami commented Dec 4, 2019 •

edited

Loading

dwnusbaum commented Dec 4, 2019 •

edited

Loading

AngryGami commented Dec 4, 2019 •

edited

Loading