From c752b24063dc8d7742bc31adf30eed657f5d7c27 Mon Sep 17 00:00:00 2001 From: "Jose D. Gomez R" Date: Thu, 14 Dec 2023 13:07:58 +0100 Subject: [PATCH] Unify Perl Tidyall - Introducing `tools/tidyall` an improved wrapper over perl's `tidyall`. It does version detection directly in perl and exposes the underlying `tidyall` CLI. - Adds a complementary GitHub Action to run `tools/tidyall` automatically on Pull Requests & Master. - Brings bare minimum dependencies.yaml for running `tools/tidyall`. Adjusted `tools/update-deps` to produce a `cpanfile` alone (without needing an `.spec` file first). - Applied tidy rules. Branched off #30. --- .github/workflows/perl-lint-checks.yml | 18 +++++++ .gitignore | 1 + .perltidyrc | 14 +++++ .tidyallrc | 3 ++ cpanfile | 26 ++++++++++ dependencies.yaml | 32 ++++++++++++ lib/OpenQA/Test/TimeLimit.pm | 4 +- tools/tidyall | 71 ++++++++++++++++++++++++++ tools/update-deps | 35 +++++++------ 9 files changed, 186 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/perl-lint-checks.yml create mode 100644 .gitignore create mode 100644 .perltidyrc create mode 100644 .tidyallrc create mode 100644 cpanfile create mode 100644 dependencies.yaml create mode 100755 tools/tidyall diff --git a/.github/workflows/perl-lint-checks.yml b/.github/workflows/perl-lint-checks.yml new file mode 100644 index 0000000..f6e3452 --- /dev/null +++ b/.github/workflows/perl-lint-checks.yml @@ -0,0 +1,18 @@ +--- +name: 'Perl static checks' + +on: + pull_request: + push: + branches: + - 'master' + +jobs: + perl-lint-checks: + runs-on: ubuntu-latest + name: "Perltidy" + container: + image: perldocker/perl-tester + steps: + - uses: actions/checkout@v4 + - run: GITHUB_ACTIONS=1 ./tools/tidyall --check-only --all --quiet diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..80ff5bf --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +.tidyall.d/ \ No newline at end of file diff --git a/.perltidyrc b/.perltidyrc new file mode 100644 index 0000000..523db5c --- /dev/null +++ b/.perltidyrc @@ -0,0 +1,14 @@ +# Workaround needed for handling non-ASCII in files. +# # See . +--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 )} \ No newline at end of file diff --git a/.tidyallrc b/.tidyallrc new file mode 100644 index 0000000..d4c7a45 --- /dev/null +++ b/.tidyallrc @@ -0,0 +1,3 @@ +[PerlTidy] +select = **/*.{pl,pm,t} tools/tidyall tools/perlcritic tools/update-deps +argv = --profile=$ROOT/.perltidyrc diff --git a/cpanfile b/cpanfile new file mode 100644 index 0000000..bd53e7e --- /dev/null +++ b/cpanfile @@ -0,0 +1,26 @@ +################################################## +# WARNING +# This file is autogenerated by tools/update-deps +# from dependencies.yaml +################################################## + +requires 'Module::CPANfile'; +requires 'Storable', '>= 3.06'; + +on 'test' => sub { + requires 'Test::Most'; + requires 'Test::Warnings'; + +}; + +on 'develop' => sub { + requires 'Code::TidyAll'; + requires 'Perl::Critic'; + requires 'Perl::Critic::Community'; + requires 'Perl::Tidy', '== 20230912'; + +}; + +feature 'coverage', 'coverage for CI' => sub { + +}; diff --git a/dependencies.yaml b/dependencies.yaml new file mode 100644 index 0000000..2d6cee0 --- /dev/null +++ b/dependencies.yaml @@ -0,0 +1,32 @@ +--- +# % is placeholder for section. +# e.g.: +# % => develop +# %_requires => develop_requires +targets: + # List all %_requires into a cpanfile + cpanfile: [main, develop, test] + cpanfile-targets: + # save %_require into cpanfile section + develop: develop + test: test + +main_requires: + # Needed until preaction/Log-Any#105 is solved. + perl(Storable): '>= 3.06' + perl(Module::CPANfile): + perl(version): + +develop_requires: + perl(Perl::Tidy): '== 20230912' + perl(Code::TidyAll): + perl(Perl::Critic): + perl(Perl::Critic::Community): + +cover_requires: + perl(Devel::Cover): + perl(Devel::Cover::Report::Codecov): + +test_requires: + perl(Test::Most): + perl(Test::Warnings): \ No newline at end of file diff --git a/lib/OpenQA/Test/TimeLimit.pm b/lib/OpenQA/Test/TimeLimit.pm index 4aa3c5e..2a2dc70 100644 --- a/lib/OpenQA/Test/TimeLimit.pm +++ b/lib/OpenQA/Test/TimeLimit.pm @@ -12,8 +12,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; } diff --git a/tools/tidyall b/tools/tidyall new file mode 100755 index 0000000..00f202b --- /dev/null +++ b/tools/tidyall @@ -0,0 +1,71 @@ +#!/usr/bin/env perl +# Copyright SUSE LLC +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Tidyall command with perltidy version constraint. +use strict; +use warnings; +use version; +use Perl::Tidy; +use Module::CPANfile; +use FindBin '$Bin'; + +=item perltidy_version() + +Grabs the perltidy version from cpanfile using Module::CPANfile. + +=cut + +sub perltidy_version() { + my $cpanfile_location; + # Try searching for a cpanfile in: + # - the current working directory + # - a directory above this command + # - the catch-all location in external/os-autoinst-common + my @locations = ('.', "$Bin/..", "$Bin/../external/os-autoinst-common"); + + foreach my $path (@locations) { + next unless -e "$path/cpanfile"; + $cpanfile_location = "$path/cpanfile" and last; + } + + my $version = Module::CPANfile->load($cpanfile_location) + ->prereq_for_module('Perl::Tidy') + ->requirement + ->version; + # Version requirements may contain qualifiers >=, ==, <, etc. The convention + # is to separate the qualifier from the actual version with a space. + # + # It's safe enough to assume that the last item is really the version. + # + # Additionally version will take care of 1.2.0 being equal to 1.2 + return version->new((split ' ', $version)[-1]); +} + +sub is_force_flag() { $_ eq '--force' } + +my $required_version = perltidy_version(); +my $detected_version = version->new($Perl::Tidy::VERSION); +my @tidyall_argv = @ARGV; + +unless ($detected_version eq $required_version) { + print STDERR "Incorrect version of perltidy.\n"; + printf STDERR "- Detected: %s\n+ Required: %s\n\n", $detected_version, $required_version; + + my $force_run = grep { is_force_flag } @ARGV; + + unless ($force_run) { + printf STDERR "Please install the appropriate version of perltidy.\n"; + printf STDERR "If you want to proceed anyways, re run with --force flag.\n"; + exit 1; + } + + # tidyall does not know about the --force flag. + @tidyall_argv = grep { !is_force_flag } @tidyall_argv; + + print STDERR 'Proceeding to run with incorrect version of perltidy. '; + print STDERR "Results might not be consistent.\n"; + print STDERR "==================\n"; +} + +exec 'tidyall', @tidyall_argv; diff --git a/tools/update-deps b/tools/update-deps index 85b5465..4d217d6 100755 --- a/tools/update-deps +++ b/tools/update-deps @@ -13,20 +13,22 @@ use FindBin qw($Bin); GetOptions( "help|h" => \my $help, + "cpanfile" => \my $cpanfile, "specfile=s" => \my $specfile, "dockerfile=s" => \my $dockerfile, ); usage(0) if $help; -usage(1) unless $specfile; +usage(1) unless ($cpanfile || $specfile || $dockerfile); -my $scriptname = path(__FILE__)->to_rel("$Bin/.."); -my $yamlfile = "dependencies.yaml"; -my $file = "$Bin/../$yamlfile"; -my $cpanfile = "$Bin/../cpanfile"; +my $proj_root = "$Bin/.."; + +my $scriptname = path(__FILE__)->to_rel($proj_root); +my $dependencies_yaml_location = "dependencies.yaml"; +my $file = "$proj_root/$dependencies_yaml_location"; +my $cpanfile_location = "$proj_root/cpanfile"; my $data = YAML::PP->new->load_file($file); -my $spec = path($specfile)->slurp; my $spectargets = $data->{targets}->{spec}; my $cpantargets = $data->{targets}->{cpanfile}; @@ -35,8 +37,8 @@ my $cpantarget_mapping = $data->{targets}->{'cpanfile-targets'}; my ($modules_by_target) = get_modules($data, $cpantargets, $cpantarget_mapping); -update_spec(); -update_cpanfile($modules_by_target); +update_spec() if $specfile; +update_cpanfile($modules_by_target) if $cpanfile; update_dockerfile($dockerfile) if $dockerfile; sub update_dockerfile { @@ -71,7 +73,7 @@ sub update_dockerfile { my $begin = '# AUTODEPS START'; my $end = '# AUTODEPS END'; my $run = <<"EOM"; -# This part is autogenerated by $scriptname from $yamlfile +# This part is autogenerated by $scriptname from $dependencies_yaml_location # hadolint ignore=DL3034,DL3037 RUN zypper in -y -C \\ $dep && zypper clean @@ -82,6 +84,7 @@ EOM } sub update_spec { + my $spec = path($specfile)->slurp if $specfile; for my $target (@$spectargets) { my $name = $target . '_requires'; @@ -101,7 +104,7 @@ sub update_spec { $specline .= " $version"; } } - my $comment = "# The following line is generated from $yamlfile"; + my $comment = "# The following line is generated from $dependencies_yaml_location"; if ($spec =~ s/^# .*generated.*\n^$prefix.*/$comment\n$specline/m) { next; } @@ -152,7 +155,7 @@ sub update_cpanfile { ################################################## # WARNING # This file is autogenerated by $scriptname -# from $yamlfile +# from $dependencies_yaml_location ################################################## EOM @@ -168,8 +171,8 @@ EOM $cover_requires .= ' ' . _requires_line($modules_by_target->{cover}, $module); } my $devel_requires = ''; - for my $module (sort keys %{$modules_by_target->{devel}}) { - $devel_requires .= ' ' . _requires_line($modules_by_target->{devel}, $module); + for my $module (sort keys %{$modules_by_target->{develop}}) { + $devel_requires .= ' ' . _requires_line($modules_by_target->{develop}, $module); } $cpan .= <<"EOM"; @@ -177,7 +180,7 @@ on 'test' => sub { $test_requires }; -on 'devel' => sub { +on 'develop' => sub { $devel_requires }; @@ -186,8 +189,8 @@ $cover_requires }; EOM - path($cpanfile)->spew($cpan); - say "Updated $cpanfile"; + path($cpanfile_location)->spew($cpan); + say "Updated $cpanfile_location"; } sub usage {