After execution of koha-elasticsearch command a file names 0 remains in the current directory.
Created attachment 167487 [details] [review] Bug 37038: koha-elasticsearch creates a file named 0 After execution of koha-elasticsearch command a file named 0 remains in the current directory. Inside the single parentheses, the character '>' is treated as a file redirection, not as a comparison operator. Test plan: ========== 1. Have a test installation with Elasticsearch. As root, perform: ./debian/scripts/koha-elasticsearch --rebuild kohadev When the command finishes, check the contents of the current directory (ls -ltr | tail). You sould notice a fresh file named '0' 2. Apply the patch. Delete the file named 0. 3. Repeat p. 1. There should be no file named 0 now.
Created attachment 167491 [details] [review] Bug 37038: koha-elasticsearch creates a file named 0 After execution of koha-elasticsearch command a file named 0 remains in the current directory. Inside the single parentheses, the character '>' is treated as a file redirection, not as a comparison operator. Test plan: ========== 1. Have a test installation with Elasticsearch. As root, perform: ./debian/scripts/koha-elasticsearch --rebuild kohadev When the command finishes, check the contents of the current directory (ls -ltr | tail). You sould notice a fresh file named '0' 2. Apply the patch. Delete the file named 0. 3. Repeat p. 1. There should be no file named 0 now. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Shouldn't it be if [ "${clo_commit_size}" -gt "0" ]; then ?
(In reply to Tomás Cohen Arazi from comment #3) > Shouldn't it be > > if [ "${clo_commit_size}" -gt "0" ]; then > > ? Thanks, Tomás, for having a look. Well, yes, and it was my first shot. It would be OK, provided that clo_commit_size is defined and is a integer number. Otherwise we get: ./debian/scripts/koha-elasticsearch: 93: [: Illegal number: I have tried several options (including nested ifs) -- the proposed solution seems to be a reasonable, working trade-off. But I'm not a bash expert ;)
Created attachment 168220 [details] [review] Bug 37038: koha-elasticsearch creates a file named 0 After execution of koha-elasticsearch command a file named 0 remains in the current directory. Inside the single parentheses, the character '>' is treated as a file redirection, not as a comparison operator. Test plan: ========== 1. Have a test installation with Elasticsearch. As root, perform: ./debian/scripts/koha-elasticsearch --rebuild kohadev When the command finishes, check the contents of the current directory (ls -ltr | tail). You sould notice a fresh file named '0' 2. Apply the patch. Delete the file named 0. 3. Repeat p. 1. There should be no file named 0 now. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 168221 [details] [review] Bug 37038: (QA follow-up) Replace \> by -gt Note that -gt is the bash integer comparison and > is the lexical one: $ if [ '1' \> '09' ]; then echo true; fi true $ if [ '1' -gt '09' ]; then echo true; fi Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Janusz Kaczmarek from comment #4) > (In reply to Tomás Cohen Arazi from comment #3) > > Shouldn't it be > > > > if [ "${clo_commit_size}" -gt "0" ]; then > > > > ? > > Thanks, Tomás, for having a look. > > Well, yes, and it was my first shot. It would be OK, provided that > clo_commit_size is defined and is a integer number. > > Otherwise we get: > > ./debian/scripts/koha-elasticsearch: 93: [: Illegal number: > > I have tried several options (including nested ifs) -- the proposed solution > seems to be a reasonable, working trade-off. But I'm not a bash expert ;) Ah just seeing this now.. Hang on.
A simple workaround might be: clo_processes=aa; if [ "${clo_processes}" -gt 0 ] 2>/dev/null; then echo true; fi for bin/sh We just oppress the warn. The condition is false as expected. And for bin/bash this would just work: clo_processes=aa; if [[ "${clo_processes}" -gt 0 ]]; then echo true; fi But this script is marked bin/sh Which actually is confusing when you git grep bash in that folder.. But not for here.
Created attachment 168222 [details] [review] Bug 37038: (QA follow-up) Replace \> by -gt Note that -gt is the bash integer comparison and > is the lexical one: $ if [ '1' \> '09' ]; then echo true; fi true $ if [ '1' -gt '09' ]; then echo true; fi Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Added the dev/null redirection to suppress warn in bin/sh.
O_O Good catch folks
Pushed for 24.11! Well done everyone, thank you!
Backported to 24.05.x for upcoming 24.05.06