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

JENKINS-49218 Using new PostBuildScript configuration format of v2.3.0 #1098

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@dheid
Copy link

commented Jan 28, 2018

Hi!

thanks for un-deprecating the PostBuildScript Jenkins plugin!

The PostBuildScript plugin configuration has changed since version 0.17. Currently the Job DSL creates a configuration, that needs to be merged to new format. Users complain about very long generation times: https://issues.jenkins-ci.org/browse/JENKINS-49180

I'm trying to implement the new features of the plugin into the Job DSL. This pull request is the first step towards the new configuration format. It is fully compatible with existing scripts, because nothing on the syntax itself changed. First step is to simply write the new format.

Tested manually with the build-in Jenkins server and the following job DSL:

<?xml version='1.0' encoding='UTF-8'?>
<project>
  <actions/>
  <description></description>
  <keepDependencies>false</keepDependencies>
  <properties/>
  <scm class="hudson.scm.NullSCM"/>
  <canRoam>true</canRoam>
  <disabled>false</disabled>
  <blockBuildWhenDownstreamBuilding>false</blockBuildWhenDownstreamBuilding>
  <blockBuildWhenUpstreamBuilding>false</blockBuildWhenUpstreamBuilding>
  <triggers/>
  <concurrentBuild>false</concurrentBuild>
  <builders>
    <javaposse.jobdsl.plugin.ExecuteDslScripts plugin="job-dsl@1.68-SNAPSHOT">
      <scriptText>job(&apos;postbuildscript-project&apos;) {
  publishers {
    postBuildScripts {
            steps {
                shell(&apos;echo TEST&apos;)
            }
            onlyIfBuildSucceeds(true)
            onlyIfBuildFails(true)
            markBuildUnstable(true)
        }
  }
}</scriptText>
      <usingScriptText>true</usingScriptText>
      <sandbox>false</sandbox>
      <ignoreExisting>false</ignoreExisting>
      <ignoreMissingFiles>false</ignoreMissingFiles>
      <failOnMissingPlugin>false</failOnMissingPlugin>
      <unstableOnDeprecation>false</unstableOnDeprecation>
      <removedJobAction>IGNORE</removedJobAction>
      <removedViewAction>IGNORE</removedViewAction>
      <removedConfigFilesAction>IGNORE</removedConfigFilesAction>
      <lookupStrategy>JENKINS_ROOT</lookupStrategy>
    </javaposse.jobdsl.plugin.ExecuteDslScripts>
  </builders>
  <publishers/>
  <buildWrappers/>
</project>

It would be very kind, if you could merge this pull request. Thanks in advance!

Best wishes

Daniel

@rdsubhas

This comment has been minimized.

Copy link

commented Jan 29, 2018

Thank you so much @dheid !! Our Job DSL seed jobs are running for >1 hour (each postBuildStep taking 1s), would love to have this please 👍

@daspilker

This comment has been minimized.

Copy link
Member

commented Jan 30, 2018

This will break compatibility for user's running an older version of the plugin.

I'll rather remove built-in support for the Post Build Scripts plugin as it has been deprecated anyway and rely on the Automatically Generated DSL instead, which always supports the currently installed version of any plugin.

In this case the DSL syntax will be

postBuildScript {
    genericScriptFiles {
        scriptFile {
            results(Iterable<String> value)
            filePath(String value)
            role(String value)
            scriptType(String value)
        }
    }
    groovyScriptFiles {
        scriptFile {
            results(Iterable<String> value)
            filePath(String value)
            role(String value)
            scriptType(String value)
        }
    }
    groovyScripts {
        script {
            results(Iterable<String> value)
            content(String value)
            role(String value)
        }
    }
    buildSteps {
        postBuildStep {
            results(Iterable<String> value)
            buildSteps {}
            role(String value)
        }
    }
    markBuildUnstable(boolean value)
} 
@dheid

This comment has been minimized.

Copy link
Author

commented Jan 30, 2018

Thanks for your reply!

Automatically generated DSL is a great and idea and I welcome it, but...

Yes, the users need to update to a more recent version of the plugin. But this is recommended anyway. If we remove the built-in support, every user has to update their Job DSL scripts to the new format that they first need to learn.

Is it possible to provide both variants? Built-in and automatically generated? If not, I would prefer to keep the built-in format to be compatible to the existing Job DSL scripts. In my projects I have lots of PostBuildScript Job DSL script blocks and it would be a hassle to update them all.

@daspilker

This comment has been minimized.

Copy link
Member

commented Jan 31, 2018

Yes we can keep both variants since there is no name clash, postBuildScripts for the build-in DSL and postBuildScript for the generated DSL.

But this PR just changes the generated config. The PostBuildScript plugin should be able to handle the config migration, see https://wiki.jenkins.io/display/JENKINS/Hint+on+retaining+backward+compatibility. So we can keep compatibility for old and new versions. I tested version 2.3.0 and it seems to handle the migration as expected. I see no need for this PR as it is.

@dheid

This comment has been minimized.

Copy link
Author

commented Feb 2, 2018

Good point! Then let's deprecate the old functions in favor of the automatic generated DSL.

@dheid dheid closed this Feb 2, 2018

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.