Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial support for lightweight checkout changelogs #185

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@stephenc
Copy link
Member

stephenc commented Oct 23, 2017

No description provided.

@stephenc stephenc requested review from abayer, jglick and svanoort Oct 23, 2017

@stephenc stephenc force-pushed the stephenc:lightweight-changelog branch from ea1ddb6 to 04fc6ac Oct 23, 2017

@stephenc

This comment has been minimized.

Copy link
Member Author

stephenc commented Oct 23, 2017

@jglick @abayer @svanoort Here is a Proof Of Concept WIP on getting the lightweight checkout changelog.

THIS IS NOT COMPLETE

I do not know how to wire up the changelog file to the WorkflowRun as we do not have access to the WorkflowRun.SCMCheckout class from here and hence there is no way to track.

@@ -96,10 +101,26 @@ public boolean isLightweight() {
}
Run<?,?> build = (Run<?,?>) _build;
if (isLightweight()) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm)) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm, SCMRevisionAction.getRevision(build))) {

This comment has been minimized.

Copy link
@stephenc

stephenc Oct 23, 2017

Author Member

Technically unrelated, but signals a bug waiting to happen... if there are commits concurrent with the build, we could end up checking out the wrong (newer) revision of the Jenkinsfile than the checkout will check out if we do not at least attempt to resolve the revision

This comment has been minimized.

Copy link
@jglick

jglick Oct 26, 2017

Member

CpsScmFlowDefintion does not support exact revision matching between Jenkinsfile and checkout scm. It could, if using scm-api (there is a filed RFE for it), but such a change does not belong here I think; should be done separately, with test coverage.

This comment has been minimized.

Copy link
@jglick

This comment has been minimized.

Copy link
@stephenc

stephenc Oct 26, 2017

Author Member

Well if you attach an SCMRevisionAction to the build in this step then you get JENKINS-36453 "for free"... but as this PR is mostly a PoC to show what should be done...

))
));
changeSets = b.getChangeSets();
assertEquals(1, changeSets.size());

This comment has been minimized.

Copy link
@stephenc

stephenc Oct 23, 2017

Author Member

So this is currently 0 not 1 because WorkflowRun is unaware of the additional changelog0.xml. What probably needs to happen is that there would be a higher level action that could be attached in a more generic fashion and then WorkflowRun would be aware of that action and use it in addition to WorkflowRun.SCMCheckout in order to build the changeSets

This comment has been minimized.

Copy link
@jglick

jglick Jan 8, 2018

Member

Cannot SCMRevisionAction be used for this purpose? WorkflowRun would need to use scm-api and compare to the last build, instead of that being done here.

@stephenc

This comment has been minimized.

Copy link
Member Author

stephenc commented Oct 23, 2017

FTR I am expecting one of @jglick @svanoort @abayer to pick this up. I do not intend on driving this further myself

@jglick
Copy link
Member

jglick left a comment

Is this solving some filed RFE/bug?

When this has come up in the past, my inclination has been to reject such a request as not worth fixing. If your build does a checkout scm, then the checkout step will already produce a changelog.

If it does not, then by definition the only thing you are using from the entire repository is a script plus anything readTrusted might load, so you would only care about changes to those files. Possible but probably not that common—would apply to cases where you have some release engineering repository containing just Pipeline scripts used by various jobs.

@@ -96,10 +101,26 @@ public boolean isLightweight() {
}
Run<?,?> build = (Run<?,?>) _build;
if (isLightweight()) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm)) {
try (SCMFileSystem fs = SCMFileSystem.of(build.getParent(), scm, SCMRevisionAction.getRevision(build))) {

This comment has been minimized.

Copy link
@jglick
if (fs != null) {
String script = fs.child(scriptPath).contentAsString();
listener.getLogger().println("Obtained " + scriptPath + " from " + scm.getKey());
// a heavyweight checkout will populate the changelog only if there is a previous build
// to compare with

This comment has been minimized.

Copy link
@jglick

jglick Oct 26, 2017

Member

That would be true of a lightweight checkout as well. A changelog is only meaningful when there is some previous build.

This comment has been minimized.

Copy link
@stephenc

stephenc Oct 26, 2017

Author Member

Which is why I added the comment... so that the if (prev != null) makes sense... we could choose to say this is the first build, so "oh here's the last 1000 changes"... but that would deviate from the behaviour of a heavyweight checkout

@stephenc

This comment has been minimized.

Copy link
Member Author

stephenc commented Oct 26, 2017

Is this solving some filed RFE/bug?

A number of issues in the issue tracker with users complaining about this, e.g.

https://issues.jenkins-ci.org/browse/JENKINS-46431

@jglick
Copy link
Member

jglick left a comment

I suspect some of this is being done in the wrong place; needs more thought.

))
));
changeSets = b.getChangeSets();
assertEquals(1, changeSets.size());

This comment has been minimized.

Copy link
@jglick

jglick Jan 8, 2018

Member

Cannot SCMRevisionAction be used for this purpose? WorkflowRun would need to use scm-api and compare to the last build, instead of that being done here.

@pvasilevich

This comment has been minimized.

Copy link

pvasilevich commented Sep 27, 2018

Can you please confirm if this pull request can help in the following scenario:
We have job that executed for every commit in a row. Build takes 15 minutes, and we need to disable concurrent builds, so there is a moment when job for commit A is still working, jobs for commits B and C are in the queue.
The issue is that if commit C modifies Jenkinsfile somehow, then this C-state of Jenkinsfile will be fetched during job for commit B, because Lightweight checkout doesn't understand exact revision which need to be checked out.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.