-
Notifications
You must be signed in to change notification settings - Fork 378
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
idris_support: fix environ for macOS #3324
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Do you mind adding a really simple test to tests/base
that exercises getEnvironment
in Idris which in turn relies on the environ
variable? If you'd like some pointers on adding new tests, just let me know.
@mattpolzin This is a canonical definition: https://github.com/libuv/libuv/blob/ba24986f8deda3a0228e1e06e4a5ee87451ede92/src/unix/core.c#L59-L65 https://opensource.apple.com/source/Libc/Libc-1244.50.9/stdlib/FreeBSD/system.c.auto.html But I do not mind a test, of course, if you think it is needed. But yes, I will need help with it then. |
Idris uses golden tests that are automatically discovered (for the test directory we are concerned about) so the easiest thing to do is copy an existing test and modify. I'd recommend copying Next, delete the In the In Then, to generate a value for your test, run the following from the root of the repo:
You'll see "Golden value missing..." and it should suggest the expected success result to you. Type |
Depending on your familiarity with the Idris2 language itself, I may or may not have left you with some things to puzzle through. Feel free to ask if you're just getting started with programming in Idris and haven't yet worked out how to write IO do blocks. |
Your change here definitely looks good to go, so I hope you don't mind that I pushed up that new test I was suggesting myself. We can get this merged once CI finishes. |
@mattpolzin Thank you very much! And sorry that I did not return to test issue, too much stuff to handle, and I am not familiar with the language syntax here, so need to read documentation first to do anything. |
Description
Fix environ for Apple: this version works on every macOS, unlike the generic one.