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

Implement setting environment variables #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mtolly
Copy link
Contributor

@mtolly mtolly commented Oct 20, 2014

Currently using System.Environment.setEnv crashes with this:

uncaught exception in Haskell main thread: ReferenceError: h$putenv is not defined
ReferenceError: h$putenv is not defined
    at h$$mi (/Users/mtolly/git/setenv/dist/build/setenv/setenv.jsexe/all.js:38868:11)
    at h$mainLoop (/Users/mtolly/git/setenv/dist/build/setenv/setenv.jsexe/all.js:10661:25)
    at /Users/mtolly/git/setenv/dist/build/setenv/setenv.jsexe/all.js:11127:13
    at /Users/mtolly/git/setenv/dist/build/setenv/setenv.jsexe/all.js:13454:17
    at Object.wrapper [as oncomplete] (fs.js:521:5)

So I implemented that apparently missing function :)

mtolly added a commit to mtolly/hs-cordova that referenced this pull request Oct 20, 2014
@luite
Copy link
Member

luite commented Oct 21, 2014

Thanks for implementing this. I've added some source comments.

Additionally, the return type of putenv is int though and like getenv this is using the C calling convention. int size in GHCJS is 32 bit (long long is 64), so it must return a number (0 on success) and not touch the secondary return value (h$ret1). Can you change it to do that?

@mtolly
Copy link
Contributor Author

mtolly commented Oct 21, 2014

Sure, I'll make those edits shortly. The part about setting to empty string removing the variable is from http://hackage.haskell.org/package/base-4.7.0.1/docs/System-Environment.html#v:setEnv which says that Windows does it; thus for compatibility setEnv does it on all platforms.

@mtolly mtolly force-pushed the master branch 2 times, most recently from 9984bdb to 2d29116 Compare October 21, 2014 04:18
@mtolly
Copy link
Contributor Author

mtolly commented Oct 21, 2014

I amended the commit with all the edits (assuming .split('=', 1) was supposed to be .split('=', 2)).

@mtolly
Copy link
Contributor Author

mtolly commented Oct 21, 2014

Now that you mention this is supposed to be a translation of the C function, it looks like setEnv in base is supposed to already have logic for handling the empty value case and unsetting the variable, before calling putenv at all. But as it stands the GHCJS code for setEnv does end up passing "key=" to h$putenv in the case of an empty value. Perhaps the logic should go in setEnv instead.

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.

2 participants