CFE-4244: Fix RlistEqual comparison on lists of different lengths

[amousset]

The original issue that leads to this is the fact that the caches for execresult and execresult_as_data were mixed (CFE-4244). This is caused by two separate issues:

• The function cache uses the args list as key and discards the function itself. This means different functions with the same args are considered identical, and cache is reused.

core/libpromises/eval_context.c

Line 2840 in 57745c7

Rval *rval = FuncCacheMapGet(ctx->function_cache, args);

• The function args are passed as an Rlist, and the comparison used ignores the additional items of the longest list when comparing two lists of different lengths, leading to treating execresult and execresult_as_data as identical when using the same command and shell args.

This PR only fixes the specific case of execresult, but leaves other function cache issues. For example, host2ip vs. ip2host could be confused:

bundle agent main {
vars:
  "res1" string => host2ip("cfengine.com");
  "res2" string => ip2host("cfengine.com");
  "res3" string => ip2host("northerntech.atlassian.net");

reports:
  "res1: ${res1}";
  "res2: ${res2}";
  "res3: ${res3}";
}

still returns:

R: res1: 34.107.174.45
R: res2: 34.107.174.45
R: res3: northerntech.atlassian.net

I'll try to add the function name to the Rlist used as cache key in a subsequent PR.

Note: Even if the function name "Equal" suggests lists of different lengths should be different, it might break other stuff using Rlists, I haven't tried to investigate other use cases.

[larsewi]

Very nice 🚀 I have only a few nitpicks.

Thank you for providing both an acceptance- and unit tests. I see that your acceptance test fails on Windows, and I'm not quite sure why. I will request a build from @cf-bottom in Jenkins, so that I can investigate this further. Feel free to address the suggested changes in the meanwhile.

[amousset]

I see that your acceptance test fails on Windows, and I'm not quite sure why.

I you think it would be acceptable I could skip this test on Windows and macOS.

[larsewi]

If you think it would be acceptable I could skip this test on Windows and macOS.

Let's not do that. BTW, the macOS failure is due to an unrelated known issue, so you can ignore it for now.

Started a build . I will investigate and then come back to you :)

[larsewi]

Hi @amousset, I just tested this manually on Windows, and you can fix it my using "powershell" instead of "useshell" as the second argument of execresult() and execresult_as_data() on Windows. You can achieve this by for example using the windows:: classguard.

[larsewi]

Awesome, thanks 🚀 Let's run this through CI again, just to make sure everything is good

[amousset]

Second part addressed in #5317

[larsewi]

Could you please squash the commits as described in https://github.com/cfengine/core/blob/master/CONTRIBUTING.md#interactive-rebase?

[larsewi]

Still failing on Windows 🤔 I will create a new build, investigate further, and then get back to you.

[larsewi]

🤞

[larsewi]

Thanks for your contribution 🚀

[larsewi]

Seems that the GitHub Action workflow for windows acceptance tests, tests with master nightly build, instead of testing it with the changes you did to core. Thus I merged this, and will see if it passes when we have a new master nightly build.