Coderunner's Twig library needs updating

[AnupamaSarjoshi]

Hi Richard,

CodeRunner is currently using the Twig library version 3.1.1 (2020-10-27). We are using another moodle plugin auth/saml2 (https://moodle.org/plugins/auth_saml2), which is using the updated version of Twig libraries, we currently have this library updated to 3.14.2 (2024-11-07) on our system. Since we have two Twig versions, this is causing an issue for us, since both versions of the libraries cannot be loaded together.

And some of the functions in the CodeRunner Twig libraries are now deprecated. Could you please have a look and suggest if you plan to update this to the newer version? It would be very helpful.

Steps to reproduce:

1. We hacked the code in coderunner/vendor/composer/autoload_static.php to use the new Twig library from auth/saml2

   public static $prefixDirsPsr4 = [
   'Twig\' =>
   [
   // because this and auth_saml2 both include Twig, auth_saml2 has newer now.
   /*
   0 => DIR . '/..' . '/twig/twig/src',
   */
   0 => DIR . '/../../../../../auth/saml2/.extlib/simplesamlphp/vendor' . '/twig/twig/src',
   ],

2. With this change, we noticed there were two phpunit tests failing.
   test_randomised_sqr and test_randomised_sqr_with_seed.

Error:
qtype_coderunner\walkthrough_randomisation_test::test_randomised_sqr
Failed asserting that false is true.
/var/www/html/question/type/coderunner/tests/walkthrough_randomisation_test.php:69
/var/www/html/lib/phpunit/classes/advanced_testcase.php:72

3. Also we get the undefined function twig_test_iterable() error when previewing the question with Twig preprocessor ( as this function is deprecated since Twig 3.8.0)

Thanks,
Anupama

[AnupamaSarjoshi]

Hi @trampgeek, sorry not sure if you got any chance to look at this issue.

I tried looking at the code, some of the functions in coderunner/classes/twig.php are deprecated in the new twig library, and we could replace them with the newly provided ones. These are being called in the function qtype_coderunner_twig_random.

• twig_test_iterable (alternative new function: is_iterable)
• twig_to_array (alternative new function: toArray)

New code changes could be:

if (!is_iterable($values)) {
return $values;
}
$values = CoreExtension::toArray($values);
if (0 === \count($values)) {
throw new RuntimeError('The random function cannot pick from an empty array.');
}

I am not very sure on how to update the twig libraries, or the version you would like to upgrade it to? If you could please have a look and suggest, that would be very helpful.

Thanks in advance!

Best regards,
Anupama

[trampgeek]

Sorry about the delay in responding Anupama. I must confess to having rather skimmed over your first posting and hadn't realised that the issue was already significant for you. Although, if you have different plugins using Twig, it's going to be difficult to keep the versions compatible and in sync, I think.

Updating twig isn't quite as straightforward as you might think because we've added some extra functions to it. And we've also had to implement our own security policy because of a serious security flaw in Twig.

My preference is to install the latest version and then to retrofit our updates. I'll look into what's involved later this week.

[AnupamaSarjoshi]

Thank you very much Richard. Would really appreciate your help on this. It has become a major issue for us as loads of our coderunner questions using Twig libraries are failing, when we tried to point it to use the latest Twig library as part to handle with multiple versions.

[trampgeek]

Hi again Anupama. The update went gratifyingly smoothly. Suspiciously so, in fact.

I don't like pushing updates to master until they've been running on our production server for a while. But I've pushed an updated Twig library plus CodeRunner's Classes/twig.php to the development branch. You should be able to cherry-pick that last commit (4e3a951) and merge it to your production server if that's what you want. It runs the test suite without errors and also runs the 2000-odd questions in our COSC131 course. It should be OK but please check it out and report back.

FYI all I did was replace the vendor subtree with the latest Twig version and update the random function in classes/twig.php. The security policy didn't appear to need updating but I'd be grateful if you'd give it the once over. I'd prefer to get back to working on the Moodle 5.0 support (which seems to be working OK except the behat tests need updating because of the UI has changed).

[trampgeek]

Actually, I have an unrelated question for you or your team, Anupama. We've discovered that the bulk tester crashes with a memory limit error after several hundred question when the php.ini memory limit is lowish - say 128M. We haven't seen it before because we normally have a much high memory limit set.

We've pored over the code and tried calling gc_collect_cycles after testing each question. The memory usage grows by around 100k for each question tested . It looks like something somewhere is caching data in memory but even single stepping through lots of code we can't see why or where. The questiondata cache is a file cache, not an in-memory cache.

Tim Hunt once reported that you guys had unexplained crashes in the bulk tester. Could it have been the same problem and did you find a solution? We can resolve it by calling set_ini to raise the memory limit but it's annoying that there's an apparent memory leak we can't find.

[timhunt]

I think we would have noticed if the error message said memory limit. I agree that an un-trackable memory leek is annoying, and I wonder if we will start to see problems. Will let you know.

(Acutally, another thing that might cause issue for us on our test servrs, is that whenever anyone deploys another bug fix there, that can kill in-progress things. I wonder if that explains what we are seeing? That only just occurred to me.)

Thanks for doing the Twig update. I am sure Anu will keep you updated as we test it.

[AnupamaSarjoshi]

Thank you very much Richard for the quick twig upgrade. I will cherry-pick this and update on our test server.
Will let you know if we find any issues.

As Tim said, we have mostly seen the bulk tester crashing on our dev servers, whenever there is new deployment. Will let you know if we find something more.

[trampgeek]

Thanks for the responses guys.

But I see that the CI testing on github failed for all tests using PHP versions prior to 8.1. Composer is reporting that the dependencies require a PHP version ">=8.1.0". Digging down a bit, symfony/deprecation-contracts v3.5.1 required php >=8.1 and twig v3.18.0 requires php >=8.0.2.

Since Moodle 4.4 is currently the oldest supported Moodle, and it requires PHP8.1, I propose to remove CI tests prior to that and when updating the CodeRunner version in the plugin repository specifying it requires Moodle 4.4 or later. But I'd appreciate confirmation that this is an appropriate response - I'm not sure how other plugin developers behave with issues like this.

[timhunt]

That seems reasonable to me. If people are not updating their Moodle core, then they can live with a slightly older, but still very good, CodeRunner.

[trampgeek]

Thanks for the reassurance, Tim. I'll do that then.