Fix so that args are actually passed to scripts. #245
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Two bugs fixed: * the "-c" argument to "sh" causes subsequent args to "sh" to *not* be passed to the script given in the "-c" value * the "logrotate_script" arg ends up as arg1 of the script, which isn't documented anywhere, and was probably thought to end up as arg0 of the script, so delete
|
Nonsense. This would only work if you passed shell script's file name to I think it is obvious from the CI results -- 19 test-cases are failing with the proposed change applied. |
|
How, then, pray tell, is the documented behaviour supposed to occur? Quoting from e.g. the Currently that is, as far as I'm able to see, not happening. |
|
Yes, it works exactly as documented. You can have a look at examples in logrotate's test-suite. This is really about understanding how shell works. The underlying problem is not specific to logortate. You need to pass the arguments to the external shell script yourself. For example, you need to write: ... instead of: |
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Mar 20, 2019
Pkgsrc changes: * Track rename of logrotate-default to logrotate.conf * Add a fix so that the log file name is actually passed to the various script hooks(!) logrotate/logrotate#245 Upstream changes: 3.15.0 ====== * timer unit: change trigger fuzz from 12h to 1h (#230) * service unit: only run if /var/log is mounted (#230) * preserve fractional part of timestamps when compressing (#226) * re-indent source code using spaces only (#188) * minage: avoid rounding issue while comparing the amount of seconds (#36) * never remove old log files if rotate -1 is specified (#202) * return non-zero exit status if a config file contains an error (#199) * make copytruncate work with rotate 0 (#191) * warn user if both size and the time interval options are used (#192) * pass rotated log file name as the 2nd argument of the postrotate script when sharedscript is not enabled (#193) * rename logrotate-default to logrotate.conf (#187) 3.14.0 ====== * make configure show support status for SELinux and ACL at the end (#179) * make logrotate build again on FreeBSD (#178) * move wtmp and btmp definitions from logrotate.conf to * separate configuration files in logrotate.d (#168) * print a warning about logrotate doing nothing when -d is used (#165) * do not reject executable config files (#166) * add hardening options to logrotate.service in examples (#143) * fix spurious compressor failure when using su and compress (#169) * keep logrotate version in .tarball-version in release tarballs (#156) * introduce the hourago configuration directive (#159) * ignore empty patterns in tabooext to avoid exclusion of everything (#160) * properly report skipped test cases instead of pretending success 3.13.0 ====== * make distribution tarballs report logrotate version properly (RHBZ#1500264) * make (un)compress work even if stdin and/or stdout are closed (#154) * remove -s from DEFAULT_MAIL_COMMAND and improve its documenation (#152) * uncompress logs before mailing them even if delaycompress is enabled (#151) * handle unlink of a non-existing log file as a warning only (#144) * include compile-time options in the output of logrotate --version (#145) * make logrotate --version print to stdout instead of stderr (#145) * flush write buffers before syncing state file (#148) * specify (un)compress utility explicitly in tests (#137) * enable running tests in parallel (#132) * explicitly map root UID/GID to 0 on Cygwin (#133) * add .dpkg-bak and .dpkg-del to default tabooext list (#134) 3.12.3 ====== * copy and copytruncate directives now work together again * unlink() is no longer preceded by open() unless shred is enabled (#124) * compress and uncompress now take commands from $PATH, too (#122) 3.12.2 ====== * build fixes related to -Werror (#119) and -Werror=format= (#108) * configure --enable-werror now controls use of the -Werror flag (#123) 3.12.1 ====== * Included forgotten build-aux directory in release tarballs. 3.12.0 ====== * Fixed accident removal of rotated files with dateext. (#118) * Line comments inside globs in config files are now skipped. (#109) * logrotate now recovers from a corrupted state file. (#45) * Makefile.legacy has been removed. (#103) * config.h is now generated by autotools. (#102 and #103) * createolddir now creates old directory as unprivileged user. (#114) * weekly rotations are now predictable and configurable. (#93) * Errors in config files are no longer treated as fatal errors. (#81) * configure --with-default-mail-command specifies default mail command. (#100) * Fixed heap buffer overflow when parsing crafted config file. (#33)
|
Hm, I see... As evidenced, I wasn't alone in mis-understanding this part based on the documentation. I wonder if there is some way I can suggest an update to the documentation to make it more evident what is meant when "the script" is used as a term. In theory, each line in the script could be executed by /bin/sh and given the args, or (as is apparently the case), all the commands could be collected and put in a script file which is then run by /bin/sh. Perhaps something along the lines of and correspondingly for the other script hooks. |
|
On Wednesday, March 20, 2019 3:12:54 PM CET he32 wrote:
Hm, I see... As evidenced, I wasn't alone in mis-understanding this part
based on the documentation. I wonder if there is some way I can suggest an
update to the documentation to make it more evident what is meant when "the
script" is used as a term.
Yes, improvements of the documentation are welcome.
Perhaps something along the lines of
```postrotate/endscript
The lines between postrotate and endscript (both of which must
appear on lines by themselves) are collected in a file, and the resulting
script is executed (using /bin/sh) after the log file is rotated. ```
This is, however, _not_ how it works. logrotate neither creates nor executes
any script files on its own. It just passes the text between `postrotate` and
`endscript` directives as a command-line argument (operand of the `-c` option)
to the /bin/sh shell interpreter.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two bugs fixed:
to not be passed to the script given in the "-c" value
which isn't documented anywhere, and was probably thought
to end up as arg0 of the script, so delete
This fixes issue #244 (and issue #7).