Skip to content

Commit

Permalink
Centralize perl linters
Browse files Browse the repository at this point in the history
- Mixed all linter configurations from os-autoinst, openQA,
  os-autoinst-distri-opensuse.

  Perlcritic RC & Perltidy RC files are centralized now here. For
  convenience, the most laxed rules were picked, to minimize the
  changeset size on downstream repositories.

- Improved perltidy & perlcritic wrappers

  Downstream repositories have a divergent copy of a tidyall
  wrapper that validates before that perltidy is in the correct
  version before running.

  The wrapper does *quite a lot* of text processing to assert the
  correct version of perltidy.

  The new and improved version will get the detected perltidy
  version directly from perl in a clean manner, and will invoke
  tidyall with all arguments passed to it.

  Additionally, perlcritic has a wrapper in Makefiles to inject
  a custom Perl critic policy that it's now provided in this
  repository.

  Both scripts are intended to be symlinked by downstream
  repositories that receive this repository via the subrepo flow.

  So rules can be enforced uniformly across repositories, even if
  repositories are not using the default settings provided here.

- `Perl::Critic::Policy::HashKeyQuotes` is now enforced automatically
  via .perlcriticrc.
  Added `|` symbol as an exception to
  `Perl::Critic::Policy::HashKeyQuotes`.

- All CI Perl-related checks are driven by GitHub Actions. This repo offers a
  sample for Prove, Perlcritic & Tidyall.

Co-authored-by: Tina Müller (tinita) <[email protected]>
Co-authored-by: Oliver Kurz <[email protected]>
  • Loading branch information
3 people committed Nov 8, 2023
1 parent 0cfcdb3 commit 36b4039
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 28 deletions.
26 changes: 26 additions & 0 deletions .github/workflows/perl-lint-checks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
name: 'Perl Lint Checks'

on:
pull_request:
push:
branches:
- 'master'

jobs:
perl-lint-checks:
runs-on: ubuntu-latest
name: "Perltidy"
container:
image: perldocker/perl-tester:5.26
steps:
- uses: actions/checkout@v4
- run: GITHUB_ACTIONS=1 ./tools/tidyall --check-only --all --quiet
perl-critic-checks:
runs-on: ubuntu-latest
name: "Perlcritic"
container:
image: perldocker/perl-tester:5.26
steps:
- uses: actions/checkout@v4
- run: ./tools/perlcritic --quiet .
18 changes: 18 additions & 0 deletions .github/workflows/perl-prove.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
name: 'Perl Testing Checks'

on:
pull_request:
push:
branches:
- 'master'

jobs:
perl-prove:
runs-on: ubuntu-latest
name: "Prove"
container:
image: perldocker/perl-tester:5.26
steps:
- uses: actions/checkout@v4
- run: prove .
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.perltidyrc
*.tdy
.*.swp
*~
__pycache__/
.tidyall.d/
22 changes: 22 additions & 0 deletions .perlcriticrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
theme = community
severity = 4
include = strict Perl::Critic::Policy::HashKeyQuotes
verbose = ::warning file=%f,line=%l,col=%c,title=%m - severity %s::[%p] %e\n

# Policies built-in in perlcritic
[ControlStructures::ProhibitDeepNests]
severity = 4
add_themes = community
max_nests = 4

# Policies from Community
[Community::DiscouragedModules]
severity = 3

# Test modules have no package declaration
[Community::PackageMatchesFilename]
severity = 1

# Custom Policies
[Perl::Critic::Policy::HashKeyQuotes]
severity = 5
14 changes: 14 additions & 0 deletions .perltidyrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Workaround needed for handling non-ASCII in files.
# # See <https://github.com/houseabsolute/perl-code-tidyall/issues/84>.
--character-encoding=none
--no-valign
-l=160
-fbl # don't change blank lines
-fnl # don't remove new lines
-nsfs # no spaces before semicolons
-baao # space after operators
-bbao # space before operators
-pt=2 # no spaces around ()
-bt=2 # no spaces around []
-sbt=2 # no spaces around {}
-sct # stack closing tokens )}
3 changes: 3 additions & 0 deletions .proverc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
--formatter TAP::Formatter::File
--lib
xt/
3 changes: 3 additions & 0 deletions .tidyallrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[PerlTidy]
select = **/*.{pl,pm,t} tools/tidyall tools/perlcritic tools/update-deps
argv = --profile=$ROOT/.perltidyrc
81 changes: 65 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,61 @@
# Common files for os-autoinst/os-autoinst and os-autoinst/openQA

This repository is to be used as a
[git-subrepo](https://github.com/ingydotnet/git-subrepo).
This repository is to be used as a[git-subrepo]
(https://github.com/ingydotnet/git-subrepo). See the instructions below in the
[Git Subrepo Usage](#git-subrepo-usage) section

## Tooling offered

This repo offers:

* Linter configuration for perl projects: `.perltidyrc`, `.perlcriticrc`,
`.proverc` & `.tidyallrc`.
* Perlcritic policies that `os-autoinst/os-autoinst` and `os-autoinst/openQA`
share.
* Useful tools for linting & testing:
* A `perlcritic` wrapper that will automatically add Perlcritic rules
defined under `lib/perlcritic` and
`external/os-autoinst-common/lib/perlcritic`.
* A `tidyall` wrapper that will validate the Perltidy version against the
`cpanfile` definition.
Because `Perl::Tidy` defaults may vary between versions this tool ensures
the version of perltidy matches the required version specified in the
`cpanfile` for non-`cpanm` based installs.

* Example Github Actions for linting and unit testing for Perl.

All files can be either copied or symlinked for any downstream repository
consuming these tools.

## Running tools and tests from scratch

```bash
# Run a container with the project mounted in it.
podman run -it -v $(pwd):/host/project --workdir /host/project opensuse/leap:latest bash

# Inside the container
zypper in -y perl-App-cpanminus make gcc
cpanm --installdeps . --with-develop

prove .
./tools/tidyall --check-only --all --quiet
./tools/perlcritic --quiet .
```

## Git Subrepo Usage

`git-subrepo` is available in the following repositories:

[![Packaging status](https://repology.org/badge/vertical-allrepos/git-subrepo.svg)](https://repology.org/project/git-subrepo/versions)

## Usage

### Clone

To use it in your repository, you would usually do something like this:

% cd your-repo
% git subrepo clone [email protected]:os-autoinst/os-autoinst-common.git ext/os-autoinst-common
```bash
cd your-repo
git subrepo clone [email protected]:os-autoinst/os-autoinst-common.git external/os-autoinst-common
```

This will automatically create a commit with information on what command
was used.
Expand All @@ -27,42 +67,51 @@ The cloned repository files will be part of your actual repository, so anyone
cloning this repo will have the files automatically without needing to use
`git-subrepo` themselves.

`ext` is just a convention, you can clone it into any directory.
`external` is just a convention, you can clone it into any directory.

It's also possible to clone a branch (or a specific tag or sha):

% git subrepo clone [email protected]:os-autoinst/os-autoinst-common.git \
-b branchname ext/os-autoinst-common
```bash
git subrepo clone [email protected]:os-autoinst/os-autoinst-common.git \
-b branchname external/os-autoinst-common
```

After cloning, you should see a file `ext/os-autoinst-common/.gitrepo` with
After cloning, you should see a file `external/os-autoinst-common/.gitrepo` with
information about the cloned commit.

### Pull

To get the latest changes, you can pull:

% git subrepo pull ext/os-autoinst-common
```bash
git subrepo pull external/os-autoinst-common
```

If that doesn't work for whatever reason, you can also simply reclone it like
that:

% git subrepo clone --force [email protected]:os-autoinst/os-autoinst-common.git ext/os-autoinst-common
```bash
git subrepo clone --force [email protected]:os-autoinst/os-autoinst-common.git \
external/os-autoinst-common
```

### Making changes

If you make changes in the subrepo inside of your top repo, you can simply commit
them and then do:
If you make changes in the subrepo inside of your top repo, you can simply
commit them and then do:

% git subrepo push ext/os-autoinst-common
```bash
git subrepo push external/os-autoinst-common
```

## git-subrepo

You can find more information here:

* [Repository and usage](https://github.com/ingydotnet/git-subrepo)
* [A good comparison between subrepo, submodule and
subtree](https://github.com/ingydotnet/git-subrepo/blob/master/Intro.pod)


## License

This project is licensed under the MIT license, see LICENSE file for details.
10 changes: 10 additions & 0 deletions cpanfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Force an up-to-date version so TidyAll dependencies can be installed and
# tested. Older versions of Storable do not handle Regexes.
requires 'Storable', '>= 3.06';

on 'develop' => sub {
requires 'Perl::Tidy', '== 20230912';
requires 'Code::TidyAll';
requires 'Perl::Critic';
requires 'Perl::Critic::Community';
};
9 changes: 5 additions & 4 deletions lib/OpenQA/Test/PatchDeparse.pm
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
package OpenQA::Test::PatchDeparse;
use strict;
use warnings 'all';
use Test::Most;

# Monkeypatch B::Deparse
Expand All @@ -15,7 +17,8 @@ if (
)
{

#<<< do not let perltidy touch this
#<<< do not let perltidy nor perlcritic touch this
## no critic (TestingAndDebugging::ProhibitNoStrict)
# This is not our code, and formatting should stay the same for
# better comparison with new versions of B::Deparse
# <---- PATCH
Expand Down Expand Up @@ -60,7 +63,5 @@ elsif ($B::Deparse::VERSION) {
diag
"Using B::Deparse v$B::Deparse::VERSION. If you see 'uninitialized' warnings, update patch in t/lib/OpenQA/Test/PatchDeparse.pm";
}

## use critic
1;


8 changes: 5 additions & 3 deletions lib/OpenQA/Test/TimeLimit.pm
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Copyright 2020-2021 SUSE LLC
# Copyright SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

package OpenQA::Test::TimeLimit;
use strict;
use warnings 'all';
use Test::Most;

my $SCALE_FACTOR = $ENV{OPENQA_TEST_TIMEOUT_SCALE_FACTOR} // 1;
Expand All @@ -12,8 +14,8 @@ sub import {
# disable timeout if requested by ENV variable or running within debugger
return if ($ENV{OPENQA_TEST_TIMEOUT_DISABLE} or $INC{'perl5db.pl'});
$SCALE_FACTOR *= $ENV{OPENQA_TEST_TIMEOUT_SCALE_COVER} // 3 if Devel::Cover->can('report');
$SCALE_FACTOR *= $ENV{OPENQA_TEST_TIMEOUT_SCALE_CI} // 2 if $ENV{CI};
$limit *= $SCALE_FACTOR;
$SCALE_FACTOR *= $ENV{OPENQA_TEST_TIMEOUT_SCALE_CI} // 2 if $ENV{CI};
$limit *= $SCALE_FACTOR;
$SIG{ALRM} = sub { BAIL_OUT "test '$0' exceeds runtime limit of '$limit' seconds\n" };
alarm $limit;
}
Expand Down
35 changes: 35 additions & 0 deletions lib/perlcritic/Perl/Critic/Policy/HashKeyQuotes.pm
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Copyright SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later

package Perl::Critic::Policy::HashKeyQuotes;

use strict;
use warnings;
use base 'Perl::Critic::Policy';

use Perl::Critic::Utils qw( :severities :classification :ppi );

our $VERSION = '0.0.1';

sub default_severity { return $SEVERITY_HIGH }
sub default_themes { return qw(openqa) }
sub applies_to { return qw(PPI::Token::Quote::Single PPI::Token::Quote::Double) }

# check that hashes are not overly using quotes
# (os-autoinst coding style)
sub violates {
my ($self, $elem) = @_;

#we only want the check hash keys
return if !is_hash_key($elem);

my $c = $elem->content;
# special characters
return if $c =~ m/[- \/<>.=_:\\\$\|]/;

my $desc = q{Hash key with quotes};
my $expl = q{Avoid useless quotes};
return $self->violation($desc, $expl, $elem);
}

1;
31 changes: 31 additions & 0 deletions tools/perlcritic
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env perl
# Copyright SUSE LLC
# SPDX-License-Identifier: GPL-2.0-or-later
#
# perlcritic with auto-injection of custom perlcritic rules.
use strict;
use warnings 'all';
use Cwd qw(abs_path getcwd);
use File::Basename 'dirname';
use File::Spec::Functions 'catfile';

sub extra_include_paths {
my @extra_paths = @_;
# using abs_path without arguments so symlinks are not resolved.
# dirname twice to go from `$prj_root/tools/tidyall` to: `$proj_root`
my $prj_root = dirname dirname catfile(abs_path(), __FILE__);

my @paths = ();

foreach my $path (@extra_paths) {
push @paths, catfile($prj_root, $path);
push @paths, catfile($prj_root, "external/os-autoinst-common", $path);
}

# Remove non existing paths
return grep { -e $_ } @paths;
}

$ENV{PERL5LIB} = join(':', (extra_include_paths('lib/perlcritic'), $ENV{PERL5LIB} // ''));

exec 'perlcritic', @ARGV;
Loading

0 comments on commit 36b4039

Please sign in to comment.