Bug 32012 - runreport.pl should use binmode UTF-8
Summary: runreport.pl should use binmode UTF-8
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Fridolin Somers
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 22323
Blocks:
  Show dependency treegraph
 
Reported: 2022-10-27 09:05 UTC by Fridolin Somers
Modified: 2023-12-28 20:44 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.08, 21.11.15


Attachments
Bug 32012: runreport.pl should use binmode UTF-8 (1.51 KB, patch)
2022-10-27 09:44 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 32012: runreport.pl should use binmode UTF-8 (1.45 KB, patch)
2022-10-27 09:48 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 32012: runreport.pl should use binmode UTF-8 (1.45 KB, patch)
2022-10-27 09:49 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 32012: runreport.pl should use binmode UTF-8 (1.50 KB, patch)
2022-10-27 10:26 UTC, David Nind
Details | Diff | Splinter Review
Bug 32012: runreport.pl should use binmode UTF-8 (1.56 KB, patch)
2022-11-02 14:25 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2022-10-27 09:05:34 UTC
Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.
Comment 1 Fridolin Somers 2022-10-27 09:06:07 UTC
Strange this was not done with Bug 22323
Comment 2 Fridolin Somers 2022-10-27 09:44:42 UTC
Created attachment 142701 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.

Test plan :
1) Do not apply patch
2) Create a SQL report with : S
   SELECT 'accentué',barcode FROM items limit 3
3) Note the id of this report, for example 1
4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
=> You see output with unknown char instead of é :
accentu<?>,1
accentu<?>,2
accentu<?>,3
5) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: iso-8859-1
6) Apply patch
7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
=> You see correct output :
accentuÃ,1
accentuÃ,2
accentuÃ,3
8) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: utf-8
Comment 3 Fridolin Somers 2022-10-27 09:46:37 UTC
Comment on attachment 142701 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

>From ef19187a6588da31d0706e73a6d8f45754b30c30 Mon Sep 17 00:00:00 2001
>From: Fridolin Somers <fridolin.somers@biblibre.com>
>Date: Wed, 26 Oct 2022 23:15:35 -1000
>Subject: [PATCH] Bug 32012: runreport.pl should use binmode UTF-8
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.
>
>Test plan :
>1) Do not apply patch
>2) Create a SQL report with : S
>   SELECT 'accentué',barcode FROM items limit 3
>3) Note the id of this report, for example 1
>4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
>=> You see output with unknown char instead of é :
>accentu<?>,1
>accentu<?>,2
>accentu<?>,3
>5) Run : file --mime-type /tmp/without.csv
>=> You see : /tmp/without.csv: iso-8859-1
>6) Apply patch
>7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
>=> You see correct output :
>accentué,1
>accentué,2
>accentué,3
>8) Run : file --mime-type /tmp/without.csv
>=> You see : /tmp/without.csv: utf-8
>---
> misc/cronjobs/runreport.pl | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl
>index d6fbb7fcbc..8b3c12b3b0 100755
>--- a/misc/cronjobs/runreport.pl
>+++ b/misc/cronjobs/runreport.pl
>@@ -164,6 +164,8 @@ Reports - Guided Reports
> 
> =cut
> 
>+binmode STDOUT, ":encoding(UTF-8)";
>+
> # These variables can be set by command line options,
> # initially set to default values.
> 
>-- 
>2.35.5
Comment 4 Fridolin Somers 2022-10-27 09:48:08 UTC
Created attachment 142702 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.

Test plan :
1) Do not apply patch
2) Create a SQL report with : S
   SELECT 'accentué',barcode FROM items limit 3
3) Note the id of this report, for example 1
4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
=> You see output with unknown character instead of é :
5) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: iso-8859-1
6) Apply patch
7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
=> You see correct output with é
8) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: utf-8
Comment 5 Fridolin Somers 2022-10-27 09:49:08 UTC
Created attachment 142703 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.

Test plan :
1) Do not apply patch
2) Create a SQL report with :
   SELECT 'accentué',barcode FROM items limit 3
3) Note the id of this report, for example 1
4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
=> You see output with unknown character instead of é :
5) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: iso-8859-1
6) Apply patch
7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
=> You see correct output with é
8) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: utf-8
Comment 6 David Nind 2022-10-27 10:26:20 UTC
Created attachment 142704 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.

Test plan :
1) Do not apply patch
2) Create a SQL report with :
   SELECT 'accentué',barcode FROM items limit 3
3) Note the id of this report, for example 1
4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
=> You see output with unknown character instead of é :
5) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: iso-8859-1
6) Apply patch
7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
=> You see correct output with é
8) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: utf-8

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-10-27 10:30:50 UTC
Testing notes using koha-testing-docker.

To display the mime type I ran the commands: 
- Step 5: file /tmp/without.csv -i
- Step 8: file /tmp/with.csv -i

Using file --mime-type didn't display the mime type for me.
Comment 8 Fridolin Somers 2022-10-27 20:23:05 UTC
(In reply to David Nind from comment #7)
> Testing notes using koha-testing-docker.
> 
> To display the mime type I ran the commands: 
> - Step 5: file /tmp/without.csv -i
> - Step 8: file /tmp/with.csv -i
> 
> Using file --mime-type didn't display the mime type for me.

Ah ok.
Thanks a lot for testing
Comment 9 Martin Renvoize 2022-11-02 14:25:11 UTC
Created attachment 142986 [details] [review]
Bug 32012: runreport.pl should use binmode UTF-8

Like Bug 26832 added binmode UTF-8 to script misc/search_tools/export_elasticsearch_mappings.pl, this should be added to misc/cronjobs/runreport.pl.

Test plan :
1) Do not apply patch
2) Create a SQL report with :
   SELECT 'accentué',barcode FROM items limit 3
3) Note the id of this report, for example 1
4) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/without.csv
=> You see output with unknown character instead of é :
5) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: iso-8859-1
6) Apply patch
7) Run : misc/cronjobs/runreport.pl 1 --format csv | tee /tmp/with.csv
=> You see correct output with é
8) Run : file --mime-type /tmp/without.csv
=> You see : /tmp/without.csv: utf-8

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Tomás Cohen Arazi 2022-11-02 22:27:57 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 11 Fridolin Somers 2022-11-03 21:47:43 UTC
This is great :D thanks everyone.
Comment 12 Lucas Gass 2022-12-05 23:51:23 UTC
Backported to 22.05.x for upcoming 22.05.08
Comment 13 Arthur Suzuki 2022-12-16 16:14:46 UTC
applied to 21.11 for 21.11.15
Comment 14 Fridolin Somers 2023-03-24 19:55:09 UTC
Ah strange, I don't see this commit in 21.11.x :

https://git.koha-community.org/Koha-community/Koha/commits/branch/21.11.x/misc/cronjobs/runreport.pl