See the description of the problem - https://gitlab.com/koha-community/koha-testing-docker/-/issues/483
Created attachment 179381 [details] [review] Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing: From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. => The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?)
I don't think this fix is ideal and it would be better to fix the origin of the problem (either a potential prettier bug, and/or the possibility to have a single node_modules) NSO/In discussion.
*** Bug 39352 has been marked as a duplicate of this bug. ***
Created attachment 179386 [details] [review] Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing: From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. => The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes
Created attachment 179387 [details] [review] Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing: From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. => The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! :) Added test plan. Tested misc/devel/tidy.pl with a .inc file to see that it's handled as the .tt
Comment on attachment 179387 [details] [review] Bug 39353: Prevent tidy.pl to empty template files Review of attachment 179387 [details] [review]: ----------------------------------------------------------------- Should severity be raised to critical at least? ::: misc/devel/tidy.pl @@ +203,5 @@ > $content =~ s#\n*( *)</script>\n*#\n$1</script>\n#g; > $content =~ s#(\[%\s*SWITCH[^\]]*\]\n)\n#$1#g; > > + unless ($content) { > + return ( 0, "Something went wrong, Prettier generated an empty file.", [], [], [] ); Maybe add a bit to the message: "Something went wrong, Prettier generated an empty file. Original file was kept." So the output after a commit doesn't cast doubt about corruption.
Note that https://gitlab.com/koha-community/koha-testing-docker/-/issues/485 is about removing one of the node_modules directory.
Created attachment 179480 [details] [review] Bug 39353: Prevent tidy.pl to empty template files In ktd context: if host's node_modules ktd has been generated prior to bug 38644, the template-toolkit's prettier plugin is not there and prettier will empty the file. It seems to be a prettier bug, and the following behaviour is confusing: From the host: % yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier koha-tmpl/intranet-tmpl/prog/en/modules/test.tt [error] Cannot find module '@koha-community/prettier-plugin-template-toolkit' [error] Require stack: [error] - /home/jonathan/workspace/koha/node_modules/prettier/index.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/cli.js [error] - /home/jonathan/workspace/koha/node_modules/prettier/bin-prettier.js error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. => The file is not modified Within ktd: $ yarn run prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt yarn run v1.22.22 $ prettier --write koha-tmpl/intranet-tmpl/prog/en/modules/test.tt koha-tmpl/intranet-tmpl/prog/en/modules/test.tt 6ms Done in 0.39s. => No error, and the file is emptied! It seems that the plugin is found by prettier but not used (?) Test plan: 1. Follow this from step 1 to 14 https://gitlab.com/koha-community/koha-testing-docker/-/issues/483 2. Apply patch 3. misc/devel/tidy.pl --no-write koha-tmpl/intranet-tmpl/prog/en/modules/intranet-main.tt It should print the new error message 4. Make a change to a .tt file, commit 5. There will be a message about and issue with prettier. But it will still commit. It's expect. 6. Inspect and see that the commit has the changes Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Created attachment 179481 [details] [review] Bug 39353: (QA follow-up) Clarify error message Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
I agree it's better to fix the origin of the problem, but even once we do, it's still nice to have this error handling in place in case a similar problem comes up again in the future. This is a straightforward change and it works to prevent data loss until we're able to prevent the underlying problem. Passing QA, and a big thank you to everyone who collaborated on this one!
Pushed for 25.05! Well done everyone, thank you!
Depends on Bug 38664 not in 24.11.x
No updates to the manual required.