Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix mktemp usage for OS X #344

Merged
merged 1 commit into from
Mar 21, 2015
Merged

fix mktemp usage for OS X #344

merged 1 commit into from
Mar 21, 2015

Conversation

r1chardj0n3s
Copy link
Contributor

This fixes the invocation of mktemp so it works on OS X also.

tito added a commit that referenced this pull request Mar 21, 2015
@tito tito merged commit cb5ffae into kivy:master Mar 21, 2015
@kived
Copy link
Contributor

kived commented Mar 26, 2015

I've reverted this as it breaks mktemp on Linux. Please find and test a solution which works on both OSX and Linux, or make the script check for which platform is in use. I don't know what problem this was supposed to fix.

@r1chardj0n3s
Copy link
Contributor Author

It was my understanding that the mktemp -t option is supported under Linux. I guess it's only supported under some flavours of Linux :/

How does it fail for you - what is the error? I see that on some mktemps the -t option must take a specifically-formatted string with a number of X in it (ie. ensaveXXXXX). Is that the case for you?

The problem this is solving is that the mktemp command does not work on OS X.

@kived
Copy link
Contributor

kived commented Mar 26, 2015

Yes, the -t option works on Linux and it requires XXXXXX in the template. But plain mktemp works just fine, and it seems odd that it wouldn't on OSX. So I didn't just make the change to fix, because I can't test on OSX and don't know why plain mktemp wouldn't work. :)

If you fix this to work on both Linux and OSX, I will gladly merge it. :)

@r1chardj0n3s
Copy link
Contributor Author

Unfortunately the mktemp version on OS X requires a template. Hilariously
the -t option does different things on the different versions.

On Fri, 27 Mar 2015 at 08:53 Ryan Pessa notifications@github.com wrote:

Yes, the -t option works on Linux and it requires XXXXXX in the template.
But plain mktemp works just fine, and it seems odd that it wouldn't on
OSX. So I didn't just make the change to fix, because I can't test on OSX
and don't know why plain mktemp wouldn't work. :)

If you fix this to work on both Linux and OSX, I will gladly merge it. :)


Reply to this email directly or view it on GitHub
#344 (comment)
.

@denys-duchier
Copy link
Contributor

on linux you need XXXXXX at the end of envsave, i.e. envsaveXXXXXX.

@kived
Copy link
Contributor

kived commented Mar 26, 2015

I don't know a ton about OSX, but from what I understand it's not horribly uncommon for OSX to have standard commands which work different from their Linux counterparts. Apparently they take that "Think Differently" very seriously. :P

Feel free to add an OS check (maybe via uname, that works on Linux anyway :P) if necessary. You could also do something like the following:

envsave=$(mktemp)
if [ $? -ne 0 ]; then
    envsave=$(mktemp -t envsave)
fi

But only if you can't specify the XXXXXX part on OSX. Though I'd think that the worst case scenario would be that the filename would end up being /tmp/envsaveXXXXXXdHGlsh or something.

@kived kived mentioned this pull request Jun 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants