Bug 39353 - Tidy - Do not empty template files
Summary: Tidy - Do not empty template files
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major
Assignee: Jonathan Druart
QA Contact: Emily Lamancusa (emlam)
URL:
Keywords:
: 39352 (view as bug list)
Depends on: 38664
Blocks:
  Show dependency treegraph
 
Reported: 2025-03-17 10:12 UTC by Jonathan Druart
Modified: 2025-03-19 16:36 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39353: Prevent tidy.pl to empty template files (2.98 KB, patch)
2025-03-17 10:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39353: Prevent tidy.pl to empty template files (3.42 KB, patch)
2025-03-17 11:16 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 39353: Prevent tidy.pl to empty template files (3.47 KB, patch)
2025-03-17 11:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 39353: Prevent tidy.pl to empty template files (3.55 KB, patch)
2025-03-19 15:11 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review
Bug 39353: (QA follow-up) Clarify error message (1.03 KB, patch)
2025-03-19 15:11 UTC, Emily Lamancusa (emlam)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-03-17 10:12:21 UTC
See the description of the problem - https://gitlab.com/koha-community/koha-testing-docker/-/issues/483
Comment 1 Jonathan Druart 2025-03-17 10:18:07 UTC
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 (?)
Comment 2 Jonathan Druart 2025-03-17 10:19:10 UTC
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.
Comment 3 Victor Grousset/tuxayo 2025-03-17 10:43:11 UTC
*** Bug 39352 has been marked as a duplicate of this bug. ***
Comment 4 Victor Grousset/tuxayo 2025-03-17 11:16:42 UTC
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
Comment 5 Victor Grousset/tuxayo 2025-03-17 11:18:27 UTC
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>
Comment 6 Victor Grousset/tuxayo 2025-03-17 11:19:50 UTC
It works! :)

Added test plan.

Tested misc/devel/tidy.pl with a .inc file to see that it's handled as the .tt
Comment 7 Victor Grousset/tuxayo 2025-03-17 11:47:39 UTC
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.
Comment 8 Jonathan Druart 2025-03-19 14:40:37 UTC
Note that https://gitlab.com/koha-community/koha-testing-docker/-/issues/485 is about removing one of the node_modules directory.
Comment 9 Emily Lamancusa (emlam) 2025-03-19 15:11:14 UTC
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>
Comment 10 Emily Lamancusa (emlam) 2025-03-19 15:11:17 UTC
Created attachment 179481 [details] [review]
Bug 39353: (QA follow-up) Clarify error message

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 11 Emily Lamancusa (emlam) 2025-03-19 15:19:40 UTC
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!
Comment 12 Katrin Fischer 2025-03-19 16:36:35 UTC
Pushed for 25.05!

Well done everyone, thank you!