[MacPorts] #26664: epubcheck @1.0.5_0 problems with shell wrapper
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: | Port: epubcheck -----------------------------------+---------------------------------------- There are number of problems with the epubcheck wrapper script provided with this port: * creates temporary files without using/checking TMPDIR or TEMP * merges output of STDOUT and STDERR (2>&1) * looses the exit code returned by the epubcheck application itself, and instead parses the combined output * looses the ability to pass files with spaces in the name (uses $@ instead of "$@") I'm attaching a patch containing a simplified script that corrects these issues. -- Ticket URL: <https://trac.macports.org/ticket/26664> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Changes (by jmr@…): * keywords: => haspatch -- Ticket URL: <https://trac.macports.org/ticket/26664#comment:1> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Changes (by ryandesign@…): * cc: oksmith@… (added) Comment: Cc'ing the author of that wrapper script for comment. -- Ticket URL: <https://trac.macports.org/ticket/26664#comment:2> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Comment(by oksmith@…): Well I guess anybody who uses epubcheck in batch process such as a Makefile will have to do everything I tried to provide in the wrapper. Is there any way to make better what there was instead of clobbering it? I am happy to try to fix it. Please advise. You may want to review the script associated with the port relames. It doesn't use exec or have double quotes around the $@ and is what I used as the starting point for this script. Thanks. Replying to [ticket:26664 jpenney@…]:
There are number of problems with the epubcheck wrapper script provided with this port:
* creates temporary files without using/checking TMPDIR or TEMP * merges output of STDOUT and STDERR (2>&1) * looses the exit code returned by the epubcheck application itself, and instead parses the combined output * looses the ability to pass files with spaces in the name (uses $@ instead of "$@")
I'm attaching a patch containing a simplified script that corrects these issues.
-- Ticket URL: <https://trac.macports.org/ticket/26664#comment:3> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Comment(by jpenney@…): Replying to [comment:3 oksmith@…]:
Is there any way to make better what there was instead of clobbering it? I am happy to try to fix it. Please advise.
You may want to review the script associated with the port relames. It doesn't use exec or have double quotes around the $@ and is what I used as the starting point for this script.
Unless I'm missing something my wrapper and your wrapper achieve the same end result (they run epubcheck, and return an error to the shell when it fails). The only difference being the four points I've called out above don't occur in my wrapper. Without the double quotes around `$@` `epubcheck "file one.epub" "filetwo.epub" "file three.epub"` Passes the following arguments: * `file` * `one.epub` * `filetwo.epub` * `file` * `three.epub` With the double quotes around `$@`: * `file one.epub` * `filetwo.epub` * `file three.epub` By using `exec` the script gracefully steps aside, letting the epubcheck application handle the output streams and the error code (epubcheck already returns exit codes). I often write scripts where I want to silence STDOUT but not STDERR, or capture STDERR only. It's also future proof in that if they change the output or what error codes they return, it'll just keep working. -- Ticket URL: <https://trac.macports.org/ticket/26664#comment:4> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Comment(by oksmith@…): You are correct. I just tested it. The use of exec does simply what I was trying to do by parsing the output. It is a much better solution. I'm sure that other java programs, such as relames which I mentioned above and used as a template, would similarly benefit from the use of exec and double quotes around the $@. You may wish to investigate. Thanks for making Macports better. Replying to [comment:4 jpenney@…]:
Replying to [comment:3 oksmith@…]:
Is there any way to make better what there was instead of clobbering it? I am happy to try to fix it. Please advise.
You may want to review the script associated with the port relames. It doesn't use exec or have double quotes around the $@ and is what I used as the starting point for this script.
Unless I'm missing something my wrapper and your wrapper achieve the same end result (they run epubcheck, and return an error to the shell when it fails). The only difference being the four points I've called out above don't occur in my wrapper.
Without the double quotes around `$@`
`epubcheck "file one.epub" "filetwo.epub" "file three.epub"`
Passes the following arguments:
* `file` * `one.epub` * `filetwo.epub` * `file` * `three.epub`
With the double quotes around `$@`:
* `file one.epub` * `filetwo.epub` * `file three.epub`
By using `exec` the script gracefully steps aside, letting the epubcheck application handle the output streams and the error code (epubcheck already returns exit codes). I often write scripts where I want to silence STDOUT but not STDERR, or capture STDERR only. It's also future proof in that if they change the output or what error codes they return, it'll just keep working.
-- Ticket URL: <https://trac.macports.org/ticket/26664#comment:5> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper -----------------------------------+---------------------------------------- Reporter: jpenney@… | Owner: ryandesign@… Type: defect | Status: assigned Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Keywords: haspatch | Port: epubcheck -----------------------------------+---------------------------------------- Changes (by ryandesign@…): * owner: macports-tickets@… => ryandesign@… * status: new => assigned -- Ticket URL: <https://trac.macports.org/ticket/26664#comment:6> MacPorts <http://www.macports.org/> Ports system for Mac OS
#26664: epubcheck @1.0.5_0 problems with shell wrapper ------------------------------------+--------------------------------------- Reporter: jpenney@… | Owner: ryandesign@… Type: defect | Status: closed Priority: Normal | Milestone: Component: ports | Version: 1.9.1 Resolution: fixed | Keywords: haspatch Port: epubcheck | ------------------------------------+--------------------------------------- Changes (by ryandesign@…): * status: assigned => closed * resolution: => fixed Comment: Committed the change in r71990. -- Ticket URL: <https://trac.macports.org/ticket/26664#comment:7> MacPorts <http://www.macports.org/> Ports system for Mac OS
participants (1)
-
MacPorts