Make CI enforce checkstyle clean code

[hartwork]

CC @henry2cox

[hartwork]

@henry2cox how do you feel about this pull request? Review on commit level is likely more fun than on whole-diff level, and it's perltidy that did most of this work. I am happy to adjust as needed 🍻

[hartwork]

@henry2cox rebased onto latest master now to resolve conflicts. What do you think?

[henry2cox]

Sorry for the long delay.
There are some perltidy differences that I see in my local environment which I haven't tried to diagnose yet.
Ideally: yes. We want this checker on proposed updates. I would just like it to be stable.
(Right now, I wouldn't be able to commit anything, as locally updated code won't pass... And I can't update locally to make it match.)

There is also a small backlog of updates I wanted to get in - still paused in review.
(Don't you love the summer season?)

[hartwork]

@henry2cox thanks for your reply!
For CI and your local pertidy to agree, either side can adapt, e.g. I installed latest perltidy through…

cpanm --notest https://github.com/perltidy/perltidy/archive/refs/tags/20260705.01.tar.gz

…on Gentoo and then call it through PERL5LIB="$HOME/perl5/lib/perl5:$PERL5LIB" ~/perl5/bin/perltidy. Alternatively, we can make the CI use the same version that you use locally. What do you think? Which version do you use?

I enjoy summer, mostly, yes 😃

[hartwork]

@henry2cox I have rebased this PR onto the latest master now to resolve conflicts. How do you feel about its future currently? Which version of perltidy are you using locally?

[henry2cox]

On one platform, v20220613. On another, v20240202. The code is fine under the former but wants to be reformatted under the latter. I did not find a set of configuration parameters which enabled cleanliness under both - and I haven't (yet) pinged the IT folks to install a more recent version - then freeze on that for a while. On my list...just not bubbled to the top, yet.

I do want to get this PR in place - but there are a few larger items I am finishing up first. (Sorry)
Will also be on vacation for the next about 10 days.

[hartwork]

Hi @henry2cox,

On one platform, v20220613. On another, v20240202. The code is fine under the former but wants to be reformatted under the latter. I did not find a set of configuration parameters which enabled cleanliness under both - and I haven't (yet) pinged the IT folks to install a more recent version - then freeze on that for a while. On my list...just not bubbled to the top, yet.

Is use of cpanm (or cpanminus) an option to you? It seems to be popular and allows installing other versions of perl-tidy without making changes to the system or getting sysadmins involved. E.g. Gentoo has perltidy 20250912.0.0 packaged for everyone but I can still use 20260705.01 via cpanminus.

I do want to get this PR in place

Cool!

• but there are a few larger items I am finishing up first. (Sorry) Will also be on vacation for the next about 10 days.

No worries. I'll be here when you're back. Have a good time! 🍻 🌴

[hartwork]

@henry2cox it's been about a month now since we last spoke, and the PR is open for over two months now. I have just resolved conflicts one more time. Let's merge it now or close it for good, please. I'll close it myself when master causes conflicts with it next time. Does that sound fair?

[henry2cox]

yeah...fair enough.
I keep getting sidetracked...

[hartwork]

@henry2cox thank you!

[henry2cox]

I'm in the process of pushing another large cr..
Would be good to know whether everything works in your environment or not.
You shouldn't have to disable xs... but would be good to know if you did.

[hartwork]

You shouldn't have to disable xs... but would be good to know if you did.

@henry2cox I'm not sure I understand. What is xs?

[henry2cox]

Perl's C extension language - a way to make certain operations go faster and/or to reduce memory footprint.

But - if you updated moderately recently and haven't seen issues - then XS is very likely simply working, silently.
Which is the intention.

[hartwork]

@henry2cox I'm afraid I'm still missing some context. Updated recently where? My system Perl is 5.44.0. I haven't run LCOV for a while outside of libexpat CI. I just notice that you released LCOV 2.5. Either I forgot or missed it altogether — interesting! I could see about bumping LCOV in Gentoo. Am I getting closer to what you're wondering about? Would be glad to understand.

[hartwork]

@henry2cox update: I now remembered #499 and that I didn't package 2.5 because of the failed tests. I'm running make check for master now…

[hartwork]

I now remembered #499 and that I didn't package 2.5 because of the failed tests.

@henry2cox is there a chance for a release 2.6 so that can package something with green tests for users of Gentoo?

[henry2cox]

A couple more in-flight things, and still haven't had time to look into the latest function end line/alias PR - so it is likely to be another few weeks.
I also want to give a bit of time for folks who use TOT to flush out issues I managed to overlook.

On the one hand: it is pretty easy to tag a release. On the other: take-up is historically very, very slow - so there is little point in making multiple releases within a few weeks of each other.

[hartwork]

On the one hand: it is pretty easy to tag a release. On the other: take-up is historically very, very slow - so there is little point in making multiple releases within a few weeks of each other.

@henry2cox I'm not sure if "take-up is slow" is the whole picture, at least some distros updated quickly — four of them the same day — with 2.5:

If making a release is not expensive, a clear vote from me for doing a release.

PS: https://en.wikipedia.org/wiki/Release_early,_release_often

[henry2cox]

FWIW: I pushed the last set of changes I wanted to upload before making a release (in my forked repo now) - but am seeing 2 spurious issues (or seem spurious):

• checkstyle is complaining about something I don't see locally. Not sure why.
  What would likely help, is to upload the directory tree after update - so we could download and see the differences compared to local.

• runtests is complaining on upload - because it doesn't like some directory names. What we actually want is to just upload a tarball (or zip) of the 'tests' directory, post-execution. Nobody should care what that directory looks like (until/unless they download it and need to inspect).

I don't know how to fix either of these, without doing some study.

Never mind...figured it out.

[hartwork]

FWIW: I pushed the last set of changes I wanted to upload before making a release (in my forked repo now) - but am seeing 2 spurious issues (or seem spurious):

• checkstyle is complaining about something I don't see locally. Not sure why.
  What would likely help, is to upload the directory tree after update - so we could download and see the differences compared to local.

@henry2cox I think I only used bin/checkstyle because it was there. Maybe we should delete this ball of blackmagic and just make it call perltidy -b "$@" and then follow by git diff --exit-code in CI so that we would see the diff in CI and everything would start explaining itself. What do you think? Should I make a PR for that?

• runtests is complaining on upload - because it doesn't like some directory names. What we actually want is to just upload a tarball (or zip) of the 'tests' directory, post-execution. Nobody should care what that directory looks like (until/unless they download it and need to inspect).

I found https://github.com/henry2cox/lcov/actions/runs/36463042115 for what you discribe. I understand that it rejects /jacoco2lcov/Z:\bin\jacoco2lcov for a filename. I think the fact that you have a file named Z:\bin\jacoco2lcov in Linux CI reveals that maybe some part of the the software stack mis-assumes it's on Windows or something? This probably deserves to be fixed at the core, and not at the upload layer. What do you think?

I don't know how to fix either of these, without doing some study.

Never mind...figured it out.

I guess I should have read that first 😄

[henry2cox]

maybe some part of the the software stack mis-assumes it's on Windows or something

Not quite. Some tests deliberately create and/or pass windows style paths and/or call widows-handling methods, to test some windows-only code. (Without those tests, some code isn't exercised by the regressions when run on linux.)

In any event: what I did was a hack, times two.
Upload the perltidy differences, and munge or delete the stuff that github upload doesn't like.
Then issues are at least partially visible and debug-able.

If you haven't done so already, please test that the TOT works on your systems. Assuming no complaints in the next few days, I think this can be a new release.

[hartwork]

maybe some part of the the software stack mis-assumes it's on Windows or something

Not quite. Some tests deliberately create and/or pass windows style paths and/or call widows-handling methods, to test some windows-only code. (Without those tests, some code isn't exercised by the regressions when run on linux.)

@henry2cox I think that means the project needs additional true-Windows CI.

If you haven't done so already, please test that the TOT works on your systems. Assuming no complaints in the next few days, I think this can be a new release.

I just ran make test and three tests failed:

• [master] Test lcov/misc/profile.sh fails (because of non-English LC_TIME) #542
• [master] Test genhtml/scriptargs/scriptargs.sh fails with error invalid tla 'C' in "select.pm --tla C" #543
• [master] Test genhtml/selectscript/selectscript.sh fails #544

[henry2cox]

Bummer.
I will try to get to this today. In the meantime: the tests do seem to run successfully on the github ubuntu machines (as well as on local machines) - and the test log and all the test shrapnel is uploaded by those jobs, so you may be able to look at that output to see what is different. (Log sort order is going to be different due to unpredictable parallel execution, but the sections themselves will be in the same order and should be extremely similar)