search

RDTK / generator

A tool for creating Jenkins jobs and other things from recipes describing software projects
GNU General Public License v3.0
21 stars 3 forks source link

Drop version-specific xpath for minimum-severity of issues-recorder #54

Closed rhaschke closed 2 years ago

rhaschke commented 2 years ago

Fixes #51 I'm not sure why the xpath was version-specific before. Making it version-agnostic fixes issue #51 for me. However, the issue recorder might not be configured properly anymore. Before:

<io.jenkins.plugins.analysis.core.steps.IssuesRecorder plugin="warnings-ng@8.10.1">
  ...
  <minimumSeverity plugin="analysis-model-api@9.8.1">
    <name>HIGH</name>
  </minimumSeverity>
...

After:

<io.jenkins.plugins.analysis.core.steps.IssuesRecorder plugin="warnings-ng@8.10.1">
  ...
  <minimumSeverity>
    <name>HIGH</name>
  </minimumSeverity>
...
scymtym commented 2 years ago

I'm not sure why the xpath was version-specific before.

I think difference in generated XML illustrates the reason I included the plugin version (and not just one fixed plugin version either, as the "inner" version depends on the "outer" version).

At least in the past, the "After" fragment was not sufficient to configure the plugin.

rhaschke commented 2 years ago

I believe that these plugin attributes are not relevant for jenkins to consider the config. However, I guess they are used when performing a plugin update to adapt an existing config file. Looking deeper, I noticed that compiler warnings do not yield an unstable build for ages:

Looks like the Warnings NG plugin replaced the fixed thresholds config with more fine-grained qualityGates in version 3.0.0:

<thresholds>
    <unstableTotalAll>0</unstableTotalAll>
    ...
</thresholds>

became:

<qualityGates>
    <io.jenkins.plugins.analysis.core.util.QualityGate>
        <threshold>1</threshold>
        <type>TOTAL</type>
        <status>WARNING</status>
    </io.jenkins.plugins.analysis.core.util.QualityGate>
</qualityGates>

Adding this to my config, compiler errors are reported as expected. I don't need the plugin version attributes at all for this to work!

In any case, you probably should configure the whole IssuesRecorder block differently depending on the actual warnings-ng version (instead of the minimumSeverity block, which doesn't care about the exact version). However, currently, the version is hard-coded to 8.10.1: https://github.com/RDTK/generator/blob/49720b1af386e3d9e059f580137c2a10e7b02e90/lib/jenkins.api/src/api/model/job-publisher.lisp#L244-L245

scymtym commented 2 years ago

In any case, you probably should configure the whole IssuesRecorder block differently depending on the actual warnings-ng version (instead of the minimumSeverity block, which doesn't care about the exact version). However, currently, the version is hard-coded to 8.10.1

That version is only used when generating the configuration from scratch. The plugin will then migrate the configuration as a result of certain actions such as editing the configuration manually.

It would be good to configure the plugin differently, but I'm not sure what the canonical configuration for recent plugin versions is. If I manually configure the plugin, I get the following

  <io.jenkins.plugins.analysis.core.steps.IssuesRecorder plugin="warnings-ng@9.5.0">
      <analysisTools>
        <io.jenkins.plugins.analysis.warnings.Gcc4>
          <id>gcc4</id>
          <name></name>
          <pattern></pattern>
          <reportEncoding></reportEncoding>
          <skipSymbolicLinks>false</skipSymbolicLinks>
        </io.jenkins.plugins.analysis.warnings.Gcc4>
        <io.jenkins.plugins.analysis.warnings.tasks.OpenTasks>
          <id>open-tasks</id>
          <name>Open Tasks</name>
          <highTags>HACK,FIXME</highTags>
          <normalTags>TODO,KLUDGE</normalTags>
          <lowTags></lowTags>
          <ignoreCase>true</ignoreCase>
          <isRegularExpression>false</isRegularExpression>
          <includePattern>**/*.c,**/*.cpp,**/*.cc,**/*.h,**/*.hpp,**/*.hh,**/CMakeLists.txt,**/*.cmake</includePattern>
          <excludePattern>upstream/**/*.*,build/**/*.*,.dir-locals.el,.git/**/*.*,.gitignore/**/*.*,.gitattributes/**/*.*,.svn/**/*.*,.bzr/**/*.*,.project/**/*.*,.idea/**/*.*</excludePattern>
        </io.jenkins.plugins.analysis.warnings.tasks.OpenTasks>
      </analysisTools>
      <sourceCodeEncoding></sourceCodeEncoding>
      <sourceDirectory></sourceDirectory>
      <ignoreQualityGate>false</ignoreQualityGate>
      <ignoreFailedBuilds>false</ignoreFailedBuilds>
      <failOnError>false</failOnError>
      <healthy>2</healthy>
      <unhealthy>10</unhealthy>
      <minimumSeverity plugin="analysis-model-api@10.3.0">
        <name>HIGH</name>
      </minimumSeverity>
      <filters/>
      <isEnabledForFailure>true</isEnabledForFailure>
      <isAggregatingResults>false</isAggregatingResults>
      <isBlameDisabled>false</isBlameDisabled>
      <skipPublishingChecks>false</skipPublishingChecks>
      <publishAllIssues>false</publishAllIssues>
      <qualityGates>
        <io.jenkins.plugins.analysis.core.util.QualityGate>
          <threshold>1</threshold>
          <type>TOTAL</type>
          <status>FAILED</status>
        </io.jenkins.plugins.analysis.core.util.QualityGate>
      </qualityGates>
      <trendChartType>AGGREGATION_TOOLS</trendChartType>
      <scm></scm>
    </io.jenkins.plugins.analysis.core.steps.IssuesRecorder>

which has

For the jobs in your links, I cannot see the configuration. The second one seems to have detected warnings but I guess the "medium" severity of the warnings prevented the build health from being affected. But that (still) depends on the value of minimumSeverity, I think. But without knowing the configuration, I cannot draw a conclusion either way.

rhaschke commented 2 years ago

That version is only used when generating the configuration from scratch. The plugin will then migrate the configuration as a result of certain actions such as editing the configuration manually.

I've got the impression that this hard-coded version (8.10.1) is written on updates from build-gen as well.

which has

  • thresholds (although without the surrounding <thresholds> element that you saw)

To be honest, I cannot see any threshold element - except the one within the quality gate. What do you refer to, healthy and unhealthy? These existed before as well and you configure them as zero: https://github.com/RDTK/generator/blob/49720b1af386e3d9e059f580137c2a10e7b02e90/lib/jenkins.api/src/api/model/job-publisher.lisp#L268-L275

My key point is that the old thresholds got replaced with quality gates.

I suggest having a phone call in case of more questions :wink:

rhaschke commented 2 years ago

I can confirm that the recent jenkins / warnings-ng plugin gracefully handle unknown plugin versions in the config.xml. I removed all of them and the plugin was still able to correctly interpret the different values I entered. When manually editing (and saving) the config, all plugin versions are added back in. Thus, the proposed PR works as expected.

It would be nice to learn how to set the key parameters from a recipe:

scymtym commented 2 years ago

I can confirm that the recent jenkins / warnings-ng plugin gracefully handle unknown plugin versions in the config.xml. I removed all of them and the plugin was still able to correctly interpret the different values I entered. When manually editing (and saving) the config, all plugin versions are added back in. Thus, the proposed PR works as expected.

Thank you for doing the experiments. Looks good to me.

It would be nice to learn how to set the key parameters from a recipe: …

Please file a separate issue.