Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rc: Fix calls to mktemp #1429

Merged
merged 2 commits into from
Jun 14, 2017
Merged

rc: Fix calls to mktemp #1429

merged 2 commits into from
Jun 14, 2017

Conversation

lenormf
Copy link
Contributor

@lenormf lenormf commented Jun 9, 2017

Allow mktemp to make use of the TMPDIR environment variable when
calling it with a template.

Don't use the deprecated -t flag.

lenormf added 2 commits June 9, 2017 14:30
Allow `mktemp` to make use of the `TMPDIR` environment variable when
calling it with a template.

Don't use the deprecated `-t` flag.
@lenormf lenormf force-pushed the fix-tmpdir-modules branch from 2e4863e to d113d52 Compare June 9, 2017 11:31
@mawww mawww merged commit d113d52 into mawww:master Jun 14, 2017
@casimir
Copy link
Contributor

casimir commented Jun 14, 2017

This PR breaks all the modified scripts on macOS.

Example for lint.kak:

*** This is the debug buffer, where debug info will be written ***
shell stderr: <<<
mktemp: illegal option -- -
usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-q] [-u] -t prefix 
mkfifo: /fifo: Permission denied
>>>
Error: 1:1: 'lint' 1:1: 'eval' 1:1: 'write' /buf: Permission denied

@lenormf
Copy link
Contributor Author

lenormf commented Jun 14, 2017

So the mac implementation excepts a -t flag to know what format to use, but the GNU implementation considers this flag deprecated (and will potentially remove it in newer releases) and doesn't seem to take $TMPDIR into account all the time unless the mac-unsupported -p flag is passed…

@mawww
Copy link
Owner

mawww commented Jun 15, 2017

Can we just use mktemp -d ${TMPDIR:-/tmp}/blah-XXXX in that case ? or will that break on OSX ?

@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

I think this will work, I'll adapt my changes (although GNU/mktemp expects at least 6 X in the template).

@casimir
Copy link
Contributor

casimir commented Jun 16, 2017

It's working with just -d on OSX.

@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

It doesn't on all versions, i.e. on OS X 10.10.5.

@mawww
Copy link
Owner

mawww commented Jun 16, 2017

Whats the problem there ?

@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

It simply shows a usage message apparently, the template was probably made optional in recent versions.

@lenormf
Copy link
Contributor Author

lenormf commented Jun 16, 2017

Follow up in #1449.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants