1. 05 May, 2022 5 commits
    • Ben Avison's avatar
      Check debug targets of submodules · b4ab212f
      Ben Avison authored
      This adds another job to the `softload` stage, which ensures that the `debug`
      target builds successfully.
      
      Since some makefiles don't define such a target, we filter based on the
      results of `make -n debug` first, to ensure that the only failures reported
      are those generated by tools invoked by `make`.
      b4ab212f
    • Ben Avison's avatar
      Test for files ending with blank lines · 82e19e82
      Ben Avison authored
      This test is implemented using sed. The list of files is echoed to the output,
      and the test is a (warning-class) failure if the list is non-empty. Files can
      be explicitly omitted from this test by adding them to `WHITESPACE_WHITELIST`
      in `.gitlab-ci.yml`.
      82e19e82
    • Ben Avison's avatar
      Allow additional flags to be passed to cppcheck · b4a3f5d9
      Ben Avison authored
      These can be specified in the `.gitlab-ci.yml` file (or if launching a
      pipeline manually, via `CI/CD` -> `Pipelines` -> `Run pipeline`) using the
      `CPPCHECK_EXTRA` variable. It is anticipated that this will be most useful
      for specifying additional `--suppress` arguments.
      
      Because library components routinely feature functions that are not called
      from elsewhere within the library itself, the `unusedFunction` warning is
      very commonly encountered in these components. To reduce the number of times
      this needs to be specified in `.gitlab-ci.yml` files, `CPPCHECK_EXTRA`
      defaults to `--suppress-unusedFunction` if it is unset for a library.
      
      Because libraries are often listed as type `C` in `ModuleDB`, we instead
      inspect the component name to see if it ends with `Lib` in order to activate
      this functionality.
      b4a3f5d9
    • Ben Avison's avatar
      Suppress cppcheck unusedFunction warnings for CMHG entry points · 1ffb6f7a
      Ben Avison authored
      Just about every C module in the source tree uses CMHG to interface with the
      kernel. However, cppcheck doesn't understand CMHG files, and so it will
      incorrectly identify any C entry points from the CMHG object file as unused
      functions.
      
      The main command-line suppression options for cppcheck are:
      * --suppress=unusedFunction : blanket suppression of the warning for all
        files. This is undesirable because it would mean we miss many examples of
        dead code, and ironically, since modules commonly feature in ROMs which
        are space-constrained, it's particularly valuable to identify dead code
        in these cases.
      * --suppress=unusedFunction:[filename] : better, but a pain to implement the
        CI job for (we'd need to pass it the relevant filename(s) somehow), a pain
        to maintain (for every module, we'd need to identify the relevant file(s))
        and could still miss some dead code.
      * --suppress=unusedFunction:[filename]:[line] : solves the dead code problem
        but at the cost of being even more hassle to maintain, due to having to
        keep line numbers up-to-date.
      
      Compared to these options, inline suppression markers look very attractive.
      However, objections have been raised to these also, so here we use a new
      feature of cppcheck 1.84 (available now that we have upgraded the GitLab
      runner machine to Ubuntu 20.04): the more verbose but more flexible option
      of passing a suppression specification to cppcheck in XML format. The XML
      file itself is generated during the "make standalone" command that is
      performed as part of the CI job. For non-module components, the XML file is
      not generated, and the option to cppcheck is silently removed.
      
      Required RiscOS/BuildSys!44
      1ffb6f7a
    • Ben Avison's avatar
      Adapt cppcheck output parser to cppcheck v1.90 · dcbcf360
      Ben Avison authored
      The GitLab runner machine previously ran Ubuntu 18.04, which featured
      cppcheck 1.82. Some planned enhancements to the CI scripts required a newer
      version of cppcheck, so we have upgraded it to Ubuntu 20.04, which has
      cppcheck 1.90. However, the format of the diagnostics printed by cppcheck
      has changed in 1.90, so our code that parsed them needs adapting to match.
      dcbcf360
  2. 16 Feb, 2022 1 commit
    • Ben Avison's avatar
      Fix exceptions to copyright job · 4ddfcd07
      Ben Avison authored
      The exceptions list is processed into a form that excludes the given files or
      directories from the `find` command that searches for files to scan. However,
      a missing `-print` option caused the files to be included anyway (due to the
      default action of the `find` command).
      4ddfcd07
  3. 07 Feb, 2022 1 commit
    • Ben Avison's avatar
      Add `copyright` job for ensuring source files have licence headers · 8b86fa48
      Ben Avison authored
      This job scans all the source files in a project, attempting to ensure they
      each have a header identifying the licence terms under which the file may be
      distributed. The job prints a list of the files that fail its check to the
      job log, and exits with failure status if the list is non-empty.
      
      There is no standard format for licence headers, however one thing they do
      have in common is that they all feature a copyright notice. Upon scanning
      all the source files hosted by ROOL, very few files failed this test, and
      those that did either genuinely lacked licence terms, or had unusual ones
      where the author had waived copyright and placed the code into the public
      domain.
      
      A few files used hard spaces around the word "Copyright" so the test uses
      `grep -w` which considers hard spaces to constitute a word boundary.
      
      Although not (yet?) in use by any of our sources, the ISO SPDX standard
      advocates using `SPDX-FileCopyrightText` in source files in place of a
      copyright declaration. Therefore, this patch pre-emptively permits this as
      an alternative.
      
      To cater for waived-copyright cases, a `.gitlab-ci.yml` file can define
      `COPYRIGHT_WHITELIST` to specify files for which this test should be skipped.
      This follows the same file naming pattern as `WHITESPACE_WHITELIST`.
      8b86fa48
  4. 06 Feb, 2022 3 commits
    • Ben Avison's avatar
      Introduce SUPPRESS_JOBS CI variable · e3b9a364
      Ben Avison authored
      Sometimes, a job isn't expected to pass (most often `softload_gnu`, because
      certain components don't support non-Norcroft toolchains). By defining this
      variable in a `.gitlab-ci.yml` file, we can force any of the jobs to be
      skipped. This only applies to submodule pipelines: it is not sensible to
      skip a build phase for a superproject pipeline.
      e3b9a364
    • Ben Avison's avatar
      Disable Disc and ROM pipeline stages for the time being · b37b8b02
      Ben Avison authored
      We're still some way out from having these jobs pass - estimated at least 70
      MRs until ROMs cross-compile. In the meantime, the main effect of these jobs
      will be to cause the overall pipeline status to be amber (i.e. passed with
      warnings) which undermines its usefulness as an at-a-glance indicator of
      whether the other jobs all passed.
      
      In theory, their job logs could be inspected by human reviewers to check
      that the MR doesn't make a cross-compile build any worse than it already is,
      but I'm not sure that this is going to be the case in practice, or whether
      this overrides the benefit of being able to quickly see that the whole
      pipeline passes.
      
      So, comment out the lines that generate that part of the YAML for the time
      being. Reinstate them once the respective cross-compile builds can complete.
      b37b8b02
    • Ben Avison's avatar
      50be6202
  5. 18 Jan, 2022 1 commit
    • Ben Avison's avatar
      Tighten up tests for `merge_whitesp` job · 87a0a578
      Ben Avison authored
      Now that we've had chance to see how this job functions with real world MRs,
      rather than just with artificial test cases, it's apparent that it is far too
      sensitive and generates far too many false positives to be useful as it
      currently stands. Problems include:
      
      * When two nearby groups of lines are swapped over, the underlying `-b` and
        non-`-b` `git diff` commands can make different decisions about which group
        is fixed and which was removed from one place and inserted into another.
        When our job script finds those lines unchanged in the `-b` diff and
        changed in the non-`-b` diff, it incorrectly assumes the presence of a
        whitespace change.
      
      * Even minor indentation changes get flagged up as an error. Even something
        as simple as changing
          bar();
        to
          if (foo)
            bar();
      
      * In order to pass `head_whitesp` on a component that hasn't been touched
        recently, it's necessary to strip the trailing whitespace from the
        `VersionNum` file even though once the MR is accepted, this will happen
        automatically. Yet doing this gets picked up as a whitespace error change
        by `merge_whitesp`, so it's impossible to simultaneously pass all jobs!
      
      We could just throw away this job, but there is still an important case that
      ideally we do want to catch: where a source file was originally imported from
      an upstream project which includes whitespace errors - meaning that we want
      to preserve them to keep tracking the upstream project easy - and when one of
      our contributors has accidentally stripped all trailing whitespace from the
      file. We want this to be caught at the review stage - better still by the
      contributor, before any reviewers have to get involved. And a CI job can
      still serve this purpose well.
      
      To distinguish these different cases, we can take into account the fact that
      the accidental space-stripping scenario will typically alter many lines a
      long way away from where the new contributor was working. Thus, the job
      script is tightened as follows:
      
      * Lines "nearby" a diff `-b` hunk are discounted, where "nearby" is
        arbitrarily defined as within +/- 20 lines.
      * At least one line in the "removed" half of the non-`-b` hunk must have
        contained a whitespace error. It's impossible to exactly match up the
        individual "removed" and "added" lines in the general case, because there
        are often different numbers of lines in each half of a hunk.
      * A threshold number of flagged lines (currently 10) must be detected across
        the whole project before the test is considered to have failed, although we
        still print any lines that we find.
      * The test for introduction of whitespace errors is removed, since this
        duplicates the functionality of the `head_whitesp` job.
      87a0a578
  6. 17 Dec, 2021 1 commit
    • Ben Avison's avatar
      Enable all jobs in detached pipelines · dc064d26
      Ben Avison authored
      When working on an unrelated issue, it became clear that the reason why
      previously only the `merge_log` and `merge_whitesp` jobs appeared in
      detached pipelines (the ones that relate to an open MR) was really because
      they include a `rules` section. It is as though in the absence of a `rules`
      section, a default one applies, which adds the job to the pipeline only if
      that pipeline was due to an update of a branch or tag ref. When `rules` was
      present for a job, it overrides the default, and is evaluated irrespective
      of the the pipeline trigger.
      
      Now, it's useful to have all the jobs present in detached pipelines. The
      latest detached pipeline state, and its associated artifactes, are displayed
      at the top of the "Overview" tab of each MR page, and a new one can be
      easily triggered from the "Run pipeline" button at the top of the MR page's
      "Pipelines" tab, without having to navigate to the contributor's fork project
      (which may not even be public). It's also a problem that th...
      dc064d26
  7. 01 Sep, 2021 1 commit
  8. 08 Jun, 2021 1 commit
  9. 03 Jun, 2021 2 commits
  10. 01 Jun, 2021 2 commits
    • Ben Avison's avatar
      Fix rom_IOMD32 jobs · 14297559
      Ben Avison authored
      These jobs were trying to fetch a build tree from a non-existent superproject
      `Products/IOMD32` when they should have referenced `Products/IOMDHAL`.
      14297559
    • Ben Avison's avatar
      Reduce disc space requirements for GitLab Runner server · 51bd4664
      Ben Avison authored
      On the Runner machine, each fork of each project gets its own directory,
      which is left in the state which the latest job for whatever pipeline was
      most recently run on it. This typically will include a large number of
      object and binary files, which are of no use to anyone (anything of interest
      will already have been packaged up into an artifact and uploaded to the
      main GitLab server). Address this by adding an additional job to the end of
      each pipeline, which does a `git clean` (it's worth leaving these in place
      to reduce the bandwidth requirement when doing a `git fetch` when a pipeline
      is next run for the fork).
      
      The Runner machine also stores cache files for each fork of each project, at
      least for jobs that complete fully successfully (and there are an increasing
      number of these). The way our pipelines use caches, these are tarballs of
      pre-built source trees for each target platform. These take up less space
      than the temporary files noted above, but will now become the dominant user
      of disc space. To address this, abandon use of GitLab Runner's own cache
      facility, and take advantage of the fact that shell executors actually have
      visibility of the gitlab-runner user's whole home directory to maintain a
      single, cached version of each tarball, shared across all forks of all
      projects. This is stored within ~/cache, but namespaced under ~/cache/common
      to avoid collisions with any users of GitLab Runner's cache facility.
      51bd4664
  11. 17 May, 2021 1 commit
  12. 14 May, 2021 1 commit
    • Ben Avison's avatar
      Work around bug in git 2.27.1 · db1f5999
      Ben Avison authored
      For these versions, "git submodule update --remote" will fail for any
      submodules that don't specify a branch in .gitmodules, and which are
      currently checked out on a remote tracking branch that tracks a remote
      other than "origin". For our pipelines, this means almost any submodule
      which has a CrossCompilationSupport branch which has not yet been merged.
      
      Work around it by first doing "git submodule update" without "--remote",
      which will put all submodules into detacehd HEAD state.
      db1f5999
  13. 23 Dec, 2020 1 commit
  14. 11 Dec, 2020 2 commits
    • Ben Avison's avatar
      Fix pipeline failures for established forks · 6a7e6b2b
      Ben Avison authored
      If the following happens:
      * a GitLab runner recursively clones a superproject
      * one or more submodules has changes upstream and the submodule references in
        the superproject have been updated to point to it
      * a new pipeline is launched for the new superproject revision
      
      then, while the runner would fetch changes to the superproject, it wasn't
      doing so for the submodules. Ironically, we do both `git submodule update`
      and `git submodule update --remote` in different pipeline stages and the
      latter includes an implicit submodule fetch - but that was the later stage
      which we don't get as far as.
      
      To fix this, while retaining the pipeline stage order, include an explicit
      fetch in the earlier stage (and remove the implicit fetch in the later one,
      since that now merely wastes time).
      6a7e6b2b
    • Ben Avison's avatar
      Fix generation of IOMDHAL.yml · 9c61e2f0
      Ben Avison authored
      This one is a special case where the superproject name mismatches the
      corresponding Env (and thus Components) file.
      
      Our own CI script also didn't previously support the *removal* of an
      autogenerated YAML file. This should be a rare occurrence, but it's best
      to automate this also. Also support force-pushing to the submodule; this
      will be required in most cases where the pipeline was triggered by a
      force-push to *this* project.
      9c61e2f0
  15. 25 Nov, 2020 1 commit