Skip to content
This repository was archived by the owner on Jun 12, 2020. It is now read-only.

Don't try to chmod or chown files under /dev #43

Merged
merged 1 commit into from
Dec 28, 2018

Conversation

madddi
Copy link
Collaborator

@madddi madddi commented Dec 27, 2018

#38 introduced a bug: when using a file as a redirect target for Cron jobs as described in the documentation, the install task will change owner and permissions of this file.

The docs recommend to use /dev/null when output should be discarded. The task will then change permissions on these files as well, leading to unexpected results.

Copy link
Collaborator

@TheLastProject TheLastProject left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched condition?

@madddi madddi force-pushed the fix-cron-output-chmod branch from c1ab6f6 to b4b85db Compare December 27, 2018 20:20
@madddi
Copy link
Collaborator Author

madddi commented Dec 27, 2018

I've pushed a fix and tested it on my machines.

@TheLastProject
Copy link
Collaborator

Looks good to me, thanks for the pull request and noticing this bug, let's see what @paulfantom says :)

@paulfantom
Copy link
Owner

LGTM 👍

@paulfantom paulfantom merged commit cfb48ba into paulfantom:master Dec 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants