Update packages testing

[lukaszstolarczuk]

// Preview of nightly build: https://github.com/lukaszstolarczuk/pmdk/actions/runs/4927772024

---

This change is 

[grom72]

Reviewed 1 of 1 files at r1, 1 of 2 files at r2, 1 of 3 files at r6, 1 of 4 files at r7, all commit messages.
Reviewable status: 4 of 13 files reviewed, all discussions resolved

[codecov]

Codecov Report

Merging #5622 (61fb7b5) into stable-1.13 (5bfd7d9) will increase coverage by 0.01%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           stable-1.13    #5622      +/-   ##
===============================================
+ Coverage        74.25%   74.27%   +0.01%     
===============================================
  Files              145      145              
  Lines            22131    22131              
  Branches          3704     3705       +1     
===============================================
+ Hits             16433    16437       +4     
+ Misses            5698     5694       -4     

[grom72]

Reviewed 1 of 1 files at r3, 3 of 4 files at r5, 2 of 3 files at r6, 1 of 5 files at r8, 4 of 4 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @lukaszstolarczuk)

[grom72]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @lukaszstolarczuk)

[grom72]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

---

.github/workflows/nightly.yml line 33 at r9 (raw file):

    strategy:
      matrix:
        CONFIG: ["OS=fedora OS_VER=37 MAKE_PKG=1 EXPERIMENTAL=y VALGRIND=0",

Perhaps we can skip it as we test MAKE_PKG with rockylinux

Code quote:

"OS=fedora OS_VER=37 MAKE_PKG=1 EXPERIMENTAL=y VALGRIND=0"

[osalyk]

Reviewed 1 of 1 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, 3 of 4 files at r5, 3 of 3 files at r6, 1 of 5 files at r8, 4 of 4 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

[lukaszstolarczuk]

Reviewable status: 1 of 14 files reviewed, 1 unresolved discussion (waiting on @grom72 and @osalyk)

---

.github/workflows/nightly.yml line 33 at r9 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

Perhaps we can skip it as we test MAKE_PKG with rockylinux

Done.

[janekmi]

Reviewed 1 of 1 files at r3, 1 of 4 files at r9, 1 of 1 files at r12, 2 of 4 files at r15, 2 of 4 files at r16, all commit messages.
Reviewable status: 6 of 14 files reviewed, 4 unresolved discussions (waiting on @grom72, @lukaszstolarczuk, and @osalyk)

a discussion (no related file):
@lukaszstolarczuk would you summarize what builds exactly moved where and if decommissioned for what reason? I'm so sorry, but I struggle tremendously to follow this change.

---

---

.github/workflows/gha.yml line 33 at r16 (raw file):

                 "N=3 OS=ubuntu OS_VER=22.04 PMDK_CC=clang PMDK_CXX=clang++ TEST_BUILD=debug SRC_CHECKERS=1",
                 "N=4 OS=ubuntu OS_VER=22.04 PMDK_CC=clang PMDK_CXX=clang++ TEST_BUILD=nondebug",
                 "N=5 OS=fedora OS_VER=37    PMDK_CC=clang PMDK_CXX=clang++ TEST_BUILD=debug PUSH_IMAGE=1",

Does it make any difference that previously the Fedora image was pushed from the build using GCC instead of Clang?

---

.github/workflows/nightly.yml line 45 at r16 (raw file):

                 "OS=rockylinux OS_VER=9 TEST_BUILD=debug",
                 "OS=rockylinux OS_VER=9 TEST_BUILD=nondebug",
                 "OS=rockylinux OS_VER=9 MAKE_PKG=1 EXPERIMENTAL=y VALGRIND=0 PUSH_IMAGE=1"

I feel like this file grows in an uncontrolled manner.

@grom72

---

utils/docker/packages/cleanup-packages.py line 112 at r9 (raw file):

        uninstall_rpm_packages(args.pmdk_path)
    else:
        uninstall_dpkg_packages(args.pmdk_path)

I understand that we do not test packages on bare metal self-hosted runners, hence uninstalling packages is not needed now, right?

@grom72 @osalyk

---

utils/docker/test_package/test-built-packages.py line 256 at r16 (raw file):

            skip_daxio = True
            index = sys.argv.index(daxio_build_argument)
            sys.argv.pop(index)

Argument parsing in this script is utterly ugly. I would recommend dropping this argument popping but it does not make it any better. On the other hand, this PR is not about reworking this script to make it parse arguments Python-like.

I guess it is just a heads-up. Sorry, you even had to touch this file.

[janekmi]

Reviewed 1 of 1 files at r1, 1 of 2 files at r11, 3 of 4 files at r14, 1 of 4 files at r16.
Reviewable status: 11 of 14 files reviewed, 4 unresolved discussions (waiting on @grom72, @lukaszstolarczuk, and @osalyk)

---

utils/docker/run-build-package.sh line 69 at r16 (raw file):

# (rather than in the git tree) - for testing packages installed in the system.
$SCRIPTSDIR/configure-tests.sh PKG
./RUNTESTS -t check

@grom72 we have to consider whether it is a cost-effective approach. These files ought to be binary identical to ones built when other workflow tests PMDK. If they are, and they are located properly, and we can build against them, I think we can conclude, to a high level of certainty, that they just work. Or, am I missing something?

[janekmi]

Reviewed 2 of 4 files at r15, 1 of 4 files at r16.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72 and @lukaszstolarczuk)

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

@lukaszstolarczuk would you summarize what builds exactly moved where and if decommissioned for what reason? I'm so sorry, but I struggle tremendously to follow this change.

Ok. I think I get it now. Steep curve.

---

[lukaszstolarczuk]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @janekmi)

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

Ok. I think I get it now. Steep curve.

I believe each commit says what's happening - if you want any commit msg to be updated, please mark in the "Commits" section below.

As you wrote - it looks complicated, but I believe all these changes go in the one direction, which is making "building and verifying packages" a little simpler and, hopefully, better.

---

---

.github/workflows/gha.yml line 33 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Does it make any difference that previously the Fedora image was pushed from the build using GCC instead of Clang?

no difference - we use only one Fedora image recipe

---

.github/workflows/nightly.yml line 45 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I feel like this file grows in an uncontrolled manner.

@grom72

I'd say, in general, more jobs are better. The execution is at no cost to you (unless everything starts to fail and you have to start debug it, of course 😄 )

---

utils/docker/run-build-package.sh line 69 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

@grom72 we have to consider whether it is a cost-effective approach. These files ought to be binary identical to ones built when other workflow tests PMDK. If they are, and they are located properly, and we can build against them, I think we can conclude, to a high level of certainty, that they just work. Or, am I missing something?

hmm I'd say - you may be right.

On the other hand - this is cost-effective - you don't have to invest your time to make this potentially a little simpler/faster. This script is executed in a nightly build, which is a non-blocker, I believe 😉

---

utils/docker/packages/cleanup-packages.py line 112 at r9 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I understand that we do not test packages on bare metal self-hosted runners, hence uninstalling packages is not needed now, right?

@grom72 @osalyk

That was the original conclusion from some meeting - we don't have to do this on bare metal, since it should deliver the same results as, e.g. docker.

---

utils/docker/test_package/test-built-packages.py line 256 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Argument parsing in this script is utterly ugly. I would recommend dropping this argument popping but it does not make it any better. On the other hand, this PR is not about reworking this script to make it parse arguments Python-like.

I guess it is just a heads-up. Sorry, you even had to touch this file.

yeah, I didn't like it either, python has a good parsing module - to bad it wasn't used from the beginning. Anyway, as you wrote - this PR is not about the re-work of the script.

[janekmi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

a discussion (no related file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

I believe each commit says what's happening - if you want any commit msg to be updated, please mark in the "Commits" section below.

As you wrote - it looks complicated, but I believe all these changes go in the one direction, which is making "building and verifying packages" a little simpler and, hopefully, better.

I believe so. But since builds have so many parameters it is really hard to track where a certain build (or a free-flowing parameter) went. Anyways. I think I got it now. Thanks.

---

[janekmi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

---

.github/workflows/nightly.yml line 45 at r16 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

I'd say, in general, more jobs are better. The execution is at no cost to you (unless everything starts to fail and you have to start debug it, of course 😄 )

:D yeah... but having so many of them makes finding what you looking for harder. Hence, grouping is sometimes helpful.

---

utils/docker/run-build-package.sh line 69 at r16 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

hmm I'd say - you may be right.

On the other hand - this is cost-effective - you don't have to invest your time to make this potentially a little simpler/faster. This script is executed in a nightly build, which is a non-blocker, I believe 😉

I think the same at this point. But considering the mid-term future, having more build logs to take care of makes managing them more time-consuming. Hence at some point, I think we might consider putting in a little work once to alleviate everyday burdens. Just an idea.

[janekmi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72 and @lukaszstolarczuk)

---

.github/workflows/gha.yml line 33 at r16 (raw file):

Previously, lukaszstolarczuk (Łukasz Stolarczuk) wrote…

no difference - we use only one Fedora image recipe

What goes into an image? If build artefacts go there, I guess we should be at least aware that we decided to use a different compiler to provide them.

[grom72]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukaszstolarczuk)

[janekmi]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @lukaszstolarczuk)

---

.github/workflows/gha.yml line 33 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

What goes into an image? If build artefacts go there, I guess we should be at least aware that we decided to use a different compiler to provide them.

FYI @grom72

[janekmi]

I submitted a follow-up issue to investigate the build failures: #5646

[lukaszstolarczuk]

Reviewable status: complete! all files reviewed, all discussions resolved

---

.github/workflows/gha.yml line 33 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

FYI @grom72

the only thigs going into docker image (sent to the Github's artifactory) are defined in recipes, e.g.: https://github.com/pmem/pmdk/blob/master/utils/docker/images/Dockerfile.fedora-37

---

.github/workflows/nightly.yml line 45 at r16 (raw file):

Previously, janekmi (Jan Michalski) wrote…

:D yeah... but having so many of them makes finding what you looking for harder. Hence, grouping is sometimes helpful.

I'll leave it here to consider - there's always an option to leave some comments between the lines here (to "virtually" group the builds)