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!