| Summary: | koha-elasticsearch creates a file named 0 | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Janusz Kaczmarek <januszop> |
| Component: | Command-line Utilities | Assignee: | Janusz Kaczmarek <januszop> |
| Status: | Pushed to stable --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | trivial | ||
| Priority: | P5 - low | CC: | dcook, lucas, m.de.rooy, robin, tomascohen |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.11.00,24.05.06
|
|
| Circulation function: | |||
| Attachments: |
Bug 37038: koha-elasticsearch creates a file named 0
Bug 37038: koha-elasticsearch creates a file named 0 Bug 37038: koha-elasticsearch creates a file named 0 Bug 37038: (QA follow-up) Replace \> by -gt Bug 37038: (QA follow-up) Replace \> by -gt |
||
|
Description
Janusz Kaczmarek
2024-06-05 15:54:01 UTC
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 |