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

Blank environment variables aren't loaded (even in a POSIX environment) #48

Open
habibalamin opened this Issue May 8, 2017 · 20 comments

Comments

Projects
None yet
3 participants
@habibalamin
Contributor

habibalamin commented May 8, 2017

I think being able to have blank environment variables is useful, and I kind of resent the fact that Windows is acting as a lowest common denominator here (no offence to the team here, I can understand why you made the decision).

Some conversation occurred on #7 and #44.


For those coming to this issue after a lot of conversation, this bug is being tracked upstream, as it's a bug with GHC itself. I'll update this comment to keep track of the status of the bug.

@jsl

This comment has been minimized.

Show comment
Hide comment
@jsl

jsl May 10, 2017

Member

Thanks @habibalamin! I'm leaning towards having an option as well to allow blank values. Maybe the default is to do whatever works on Linux / Mac and Windows users need to specify an option? I wish we could find precedent in another dotenv lib that we could use for inspiration. Feel free to post here if you have observed, or have time to research behavior in other dotenv libs (Node, Python, Ruby etc) that you like or dislike.

Member

jsl commented May 10, 2017

Thanks @habibalamin! I'm leaning towards having an option as well to allow blank values. Maybe the default is to do whatever works on Linux / Mac and Windows users need to specify an option? I wish we could find precedent in another dotenv lib that we could use for inspiration. Feel free to post here if you have observed, or have time to research behavior in other dotenv libs (Node, Python, Ruby etc) that you like or dislike.

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 10, 2017

Contributor

As it happens, @CristhianMotoche did some tests with the major Ruby & JS implementations and they both set blank variables. I'm not sure how those implementations would behave on Windows.

I also noted on #7 that there is an open GHC bug ticket claiming that Windows does, in fact, support blank environment variables.

From what I can gather from a quick Google, most sources corroborate the original claim of Windows not supporting blank environment variables. This could be because:

  1. Repetition of the claim has made others believe it to be the truth and repeat it themselves.
  2. The official Windows documentation linked to that suggests — and it does only suggest — that blank environment variables are supported is misleading.
Contributor

habibalamin commented May 10, 2017

As it happens, @CristhianMotoche did some tests with the major Ruby & JS implementations and they both set blank variables. I'm not sure how those implementations would behave on Windows.

I also noted on #7 that there is an open GHC bug ticket claiming that Windows does, in fact, support blank environment variables.

From what I can gather from a quick Google, most sources corroborate the original claim of Windows not supporting blank environment variables. This could be because:

  1. Repetition of the claim has made others believe it to be the truth and repeat it themselves.
  2. The official Windows documentation linked to that suggests — and it does only suggest — that blank environment variables are supported is misleading.
@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 11, 2017

Contributor

I used this program to test the documentation:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  // searches PATH, inherits environment
  _execlp("cmd", "cmd", NULL);

  printf("exec failed!\n");
  return 0;
}

I tested it on a Windows 7 VM:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

Perhaps more interestingly:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

If the environment variable is set, it seems to be forgotten on the third — consistently the third — nested exec call, but it doesn't unset it the first time it's set to the empty string. The documentation suggests that it would be unset if it were given an argument of NULL, but it didn't say anything about giving it an argument of a blank string, so that's technically valid.

Incidentally, I did also test SetEnvironmentVariable with a null value and that doesn't forget either, so the documentation seems to be inapplicable for whatever reason (it's possibly referencing Windows 8).

The most interesting finding is that when calling GetEnvironmentVariable, everything behaved as expected; blank environment variables were actually printed as blank (well, not printed), and when set to NULL, printing the environment variable results in funny characters being printed.

It could be that Windows does support blank environment variables under the hood, but the cmd.exe shell itself simply treats those variables as unset. In that case, it seems perfectly fine to use in our own programs, but I'm speculating.

Contributor

habibalamin commented May 11, 2017

I used this program to test the documentation:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  // searches PATH, inherits environment
  _execlp("cmd", "cmd", NULL);

  printf("exec failed!\n");
  return 0;
}

I tested it on a Windows 7 VM:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

Perhaps more interestingly:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
test

C:\Users\IEUser\Documents>.\enver.exe
TEST: ''

C:\Users\IEUser\Documents>Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser\Documents>echo %TEST%
%TEST%

C:\Users\IEUser\Documents>

If the environment variable is set, it seems to be forgotten on the third — consistently the third — nested exec call, but it doesn't unset it the first time it's set to the empty string. The documentation suggests that it would be unset if it were given an argument of NULL, but it didn't say anything about giving it an argument of a blank string, so that's technically valid.

Incidentally, I did also test SetEnvironmentVariable with a null value and that doesn't forget either, so the documentation seems to be inapplicable for whatever reason (it's possibly referencing Windows 8).

The most interesting finding is that when calling GetEnvironmentVariable, everything behaved as expected; blank environment variables were actually printed as blank (well, not printed), and when set to NULL, printing the environment variable results in funny characters being printed.

It could be that Windows does support blank environment variables under the hood, but the cmd.exe shell itself simply treats those variables as unset. In that case, it seems perfectly fine to use in our own programs, but I'm speculating.

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 11, 2017

Contributor

I'm going to be removing the VM, by the way, so if there's any particular tests you want me to do, you have 4 hours.

Contributor

habibalamin commented May 11, 2017

I'm going to be removing the VM, by the way, so if there's any particular tests you want me to do, you have 4 hours.

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche May 11, 2017

Contributor

Thanks for the research @habibalamin. I think the problem we have is that we cannot set env vars in POSIX environments. That is because the way setEnv in System.Environment is defined. Right?

Could you please test other dotenv libraries from other languages (JS, Ruby, or Python) and see what is their behaviour? You could use the examples that I did

Contributor

CristhianMotoche commented May 11, 2017

Thanks for the research @habibalamin. I think the problem we have is that we cannot set env vars in POSIX environments. That is because the way setEnv in System.Environment is defined. Right?

Could you please test other dotenv libraries from other languages (JS, Ruby, or Python) and see what is their behaviour? You could use the examples that I did

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 11, 2017

Contributor

I shut down the machine, and it's now updating 🙄, but I'll test that once that's done.

Yes, the default setEnv is defined to not be able to set blank environment variables for consistency. The unix package (which contains System.Posix.setEnv), as expected, won't build on the Windows system without something like Cygwin; I haven't tried with Cygwin, but I'll try that too.

We could also define our own setEnv that allows blank environment variables on both Windows and POSIX, but that sounds like perhaps the effort/readability/maintainability burden isn't worth it, although I wouldn't know (we'd probably have to interact with the FFI).

Contributor

habibalamin commented May 11, 2017

I shut down the machine, and it's now updating 🙄, but I'll test that once that's done.

Yes, the default setEnv is defined to not be able to set blank environment variables for consistency. The unix package (which contains System.Posix.setEnv), as expected, won't build on the Windows system without something like Cygwin; I haven't tried with Cygwin, but I'll try that too.

We could also define our own setEnv that allows blank environment variables on both Windows and POSIX, but that sounds like perhaps the effort/readability/maintainability burden isn't worth it, although I wouldn't know (we'd probably have to interact with the FFI).

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche May 11, 2017

Contributor

Another solution could be using CPP like in #7. However, I'm not sure if we want to use CPP for that. Let me know what you think about it?

Contributor

CristhianMotoche commented May 11, 2017

Another solution could be using CPP like in #7. However, I'm not sure if we want to use CPP for that. Let me know what you think about it?

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 11, 2017

Contributor

Okay, Ruby 2.3.3's Kernel#exec inherits the environment:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST'] = 'test'
=> "test"
irb(main):002:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
test

I had the same result as in C here. Setting a blank environment variable is retrievable through the language, but when spawning a cmd.exe process, it treats blank environment variables as unset:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST']
=> nil
irb(main):002:0> ENV['TEST'] = ''
=> ""
irb(main):003:0> ENV['TEST']
=> ""
irb(main):004:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
%TEST%

Perhaps Ruby or Windows' C API just store blank environment variables in a special place outside of the actual environment and that's why the shell doesn't recognise it; I tested that by execing two programs I controlled.

First, I modified the C program to this:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  _execlp("ruby", "ruby",  "enver.rb", NULL);

  printf("exec failed!\n");
  return 0;
}

and created a Ruby program like this

puts "TEST: #{ENV['TEST'].inspect}"

The output is as you would expect:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: ''

C:\Users\IEUser\Documents>TEST: ""

Some timing issues, but it makes the point. Of course, when passing NULL to SetEnvironmentVariable, I get:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: 'íéh▀■   ┌ÿ►v·á◄v░♫''

C:\Users\IEUser\Documents>TEST: nil

Then I did it the other way around:

puts "TEST: #{ENV['TEST'].inspect}"
exec '.\enver'
#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  return 0;
}

Here's the output of that:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby enver.rb
TEST: nil
TEST: '╨d╡4■   ┌ÿ►v·á◄v└♫X'

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>ruby enver.rb
TEST: "test"
TEST: 'test'

C:\Users\IEUser\Documents>

I think this is good evidence that it's being stored in the actual environment, as it's being transferred cross-language as we would expect.

Here's what I got for the dotenv gem with your .env example in #44:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby dotenv.rb
Foo is set
FOO: ""
BAR: "1234"

C:\Users\IEUser\Documents>

Although I slightly modified the program (Rubyists use string interpolation, which automatically calls #to_s on the value; string1 + nil + string2 explodes, but "prefix#{nil}suffix" evaluates to "prefixsuffix"):

require 'dotenv'

Dotenv.load('.env')

if ENV['FOO'].nil?
  puts 'Foo is not set'
else
  puts 'Foo is set'
end

# fun fact: #inspect is Ruby's equivalent of Haskell's `show`;
# every object has it by default, except when inheriting from
# BasicObject.
puts "FOO: #{ENV['FOO'].inspect}"
puts "BAR: #{ENV['BAR'].inspect}"

I used your example for node and I actually was not expecting this:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>node dotenv.js
Foo is not set
Foo
Bar 1234

C:\Users\IEUser\Documents>

That looks like everything.

Contributor

habibalamin commented May 11, 2017

Okay, Ruby 2.3.3's Kernel#exec inherits the environment:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST'] = 'test'
=> "test"
irb(main):002:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
test

I had the same result as in C here. Setting a blank environment variable is retrievable through the language, but when spawning a cmd.exe process, it treats blank environment variables as unset:

Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>irb
irb(main):001:0> ENV['TEST']
=> nil
irb(main):002:0> ENV['TEST'] = ''
=> ""
irb(main):003:0> ENV['TEST']
=> ""
irb(main):004:0> exec 'cmd'
Microsoft Windows [Version 6.1.7601]
Copyright <c> 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>echo %TEST%
%TEST%

Perhaps Ruby or Windows' C API just store blank environment variables in a special place outside of the actual environment and that's why the shell doesn't recognise it; I tested that by execing two programs I controlled.

First, I modified the C program to this:

#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  SetEnvironmentVariable("TEST", "");
  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  _execlp("ruby", "ruby",  "enver.rb", NULL);

  printf("exec failed!\n");
  return 0;
}

and created a Ruby program like this

puts "TEST: #{ENV['TEST'].inspect}"

The output is as you would expect:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: ''

C:\Users\IEUser\Documents>TEST: ""

Some timing issues, but it makes the point. Of course, when passing NULL to SetEnvironmentVariable, I get:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>.\enver
TEST: 'íéh▀■   ┌ÿ►v·á◄v░♫''

C:\Users\IEUser\Documents>TEST: nil

Then I did it the other way around:

puts "TEST: #{ENV['TEST'].inspect}"
exec '.\enver'
#include <stdlib.h>
#include <Windows.h>
#include <stdio.h>
#include <process.h>

int main(int argc, char *argv[])
{
  char test[50];

  GetEnvironmentVariable("TEST", test, 51);
  printf("TEST: '%s'\n", test);

  return 0;
}

Here's the output of that:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby enver.rb
TEST: nil
TEST: '╨d╡4■   ┌ÿ►v·á◄v└♫X'

C:\Users\IEUser\Documents>set TEST=test

C:\Users\IEUser\Documents>ruby enver.rb
TEST: "test"
TEST: 'test'

C:\Users\IEUser\Documents>

I think this is good evidence that it's being stored in the actual environment, as it's being transferred cross-language as we would expect.

Here's what I got for the dotenv gem with your .env example in #44:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>ruby dotenv.rb
Foo is set
FOO: ""
BAR: "1234"

C:\Users\IEUser\Documents>

Although I slightly modified the program (Rubyists use string interpolation, which automatically calls #to_s on the value; string1 + nil + string2 explodes, but "prefix#{nil}suffix" evaluates to "prefixsuffix"):

require 'dotenv'

Dotenv.load('.env')

if ENV['FOO'].nil?
  puts 'Foo is not set'
else
  puts 'Foo is set'
end

# fun fact: #inspect is Ruby's equivalent of Haskell's `show`;
# every object has it by default, except when inheriting from
# BasicObject.
puts "FOO: #{ENV['FOO'].inspect}"
puts "BAR: #{ENV['BAR'].inspect}"

I used your example for node and I actually was not expecting this:

Microsoft Windows [Version 6.1.7601]
Copyright (c) 2009 Microsoft Corporation.  All rights reserved.

C:\Users\IEUser>cd Documents

C:\Users\IEUser\Documents>node dotenv.js
Foo is not set
Foo
Bar 1234

C:\Users\IEUser\Documents>

That looks like everything.

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche May 11, 2017

Contributor

@habibalamin Amazing job! Thanks a lot for your research. Now we understand in a better way the behaviour of Windows with empty env vars.

Well, now, for the matter of having blank env vars. We can make a list of the possible solutions and then choose one, or in the other hand not allow empty env vars.

Contributor

CristhianMotoche commented May 11, 2017

@habibalamin Amazing job! Thanks a lot for your research. Now we understand in a better way the behaviour of Windows with empty env vars.

Well, now, for the matter of having blank env vars. We can make a list of the possible solutions and then choose one, or in the other hand not allow empty env vars.

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche May 11, 2017

Contributor

Feel free to add your solution:

  • Create an FFI that works in both OS. (Proposed by @habibalamin)
  • Using CPP to load System.Posix.setEnv when the OS is not Windows.
Contributor

CristhianMotoche commented May 11, 2017

Feel free to add your solution:

  • Create an FFI that works in both OS. (Proposed by @habibalamin)
  • Using CPP to load System.Posix.setEnv when the OS is not Windows.
@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin May 11, 2017

Contributor

Quoting from my other comment:

I realise the danger of adding complexity through feature toggles, but allowing the user to pick their poison seems like the right option for situations where neither option is entirely without issue. Maybe a --unix/--allow-empty-values flag [would be useful].

In this case, this is another step we might want to take if we go for the blank environment variables only on POSIX option. This would allow the users to decide whether they want a consistent experience on all platforms (good for teams) or improved semantics for their preferred platform (good for homogenous teams/individuals).

The FFI option is probably going to take longer, but I will take a quick look at that and see what I can do.

Contributor

habibalamin commented May 11, 2017

Quoting from my other comment:

I realise the danger of adding complexity through feature toggles, but allowing the user to pick their poison seems like the right option for situations where neither option is entirely without issue. Maybe a --unix/--allow-empty-values flag [would be useful].

In this case, this is another step we might want to take if we go for the blank environment variables only on POSIX option. This would allow the users to decide whether they want a consistent experience on all platforms (good for teams) or improved semantics for their preferred platform (good for homogenous teams/individuals).

The FFI option is probably going to take longer, but I will take a quick look at that and see what I can do.

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin Jun 4, 2017

Contributor

@CristhianMotoche if you look at the PR, you can see I had trouble with setting blank environment variables across the FFI, despite the fact that I can do it directly in C and the Ruby API allows it.

I thought it might have something to do with sending CString values across the FFI, but I also created a dedicated function with no arguments that itself calls SetEnvironmentVariable with a blank string directly in C, and that didn't work either.

I'd like it if someone could take a look at the PR, see if I missed anything, but I don't have high hopes. This problem could be why the GHC folks thought Windows doesn't support blank variables too, and Node doesn't seem to allow it, so they either didn't want to or had issues, too.

I could take a look at the Ruby project C source and see how they do it, but our best option is looking like support for UNIX only (possibly with a flag, so that the default is consistency across platforms).

Contributor

habibalamin commented Jun 4, 2017

@CristhianMotoche if you look at the PR, you can see I had trouble with setting blank environment variables across the FFI, despite the fact that I can do it directly in C and the Ruby API allows it.

I thought it might have something to do with sending CString values across the FFI, but I also created a dedicated function with no arguments that itself calls SetEnvironmentVariable with a blank string directly in C, and that didn't work either.

I'd like it if someone could take a look at the PR, see if I missed anything, but I don't have high hopes. This problem could be why the GHC folks thought Windows doesn't support blank variables too, and Node doesn't seem to allow it, so they either didn't want to or had issues, too.

I could take a look at the Ruby project C source and see how they do it, but our best option is looking like support for UNIX only (possibly with a flag, so that the default is consistency across platforms).

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin Jun 4, 2017

Contributor

The way Ruby does it is very complex. Obviously, there's some extra work for platforms like Solaris there, but it seems unwieldy and like something we don't want to adopt for something that should be so basic.

I'm not sure how it works, and it would probably take me hours or days to figure it out (I can get by in C, but I don't really know it or use it).

So, I propose we just ignore Windows support for this. Ideally, we'd default to consistency on all platforms, and use a flag to enable the feature for teams who exclusively use Unix platforms.

Contributor

habibalamin commented Jun 4, 2017

The way Ruby does it is very complex. Obviously, there's some extra work for platforms like Solaris there, but it seems unwieldy and like something we don't want to adopt for something that should be so basic.

I'm not sure how it works, and it would probably take me hours or days to figure it out (I can get by in C, but I don't really know it or use it).

So, I propose we just ignore Windows support for this. Ideally, we'd default to consistency on all platforms, and use a flag to enable the feature for teams who exclusively use Unix platforms.

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche Jun 20, 2017

Contributor

Closing this PR because of #51 (comment)

Contributor

CristhianMotoche commented Jun 20, 2017

Closing this PR because of #51 (comment)

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche Jun 28, 2017

Contributor

I'll let this issue open, because this is a problem that we shouldn't fix here. However, if you want to have empty env vars you could use something like the code below in your configuration.

fromMaybe "" <$> lookupEnv "ENV_VAR"

For instance, if ENV_VAR is empty or not set it will be "" in your configuration, otherwise it will be any value you have in ENV_VAR.

Contributor

CristhianMotoche commented Jun 28, 2017

I'll let this issue open, because this is a problem that we shouldn't fix here. However, if you want to have empty env vars you could use something like the code below in your configuration.

fromMaybe "" <$> lookupEnv "ENV_VAR"

For instance, if ENV_VAR is empty or not set it will be "" in your configuration, otherwise it will be any value you have in ENV_VAR.

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin Jun 28, 2017

Contributor

You can track the progress of this bug upstream (it's technically a GHC bug).

Contributor

habibalamin commented Jun 28, 2017

You can track the progress of this bug upstream (it's technically a GHC bug).

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin
Contributor

habibalamin commented Jul 30, 2017

I've fixed this upstream, and it seems to be slated for the 8.4.1 release, which looks like it's going to be released on the 18th January 2018 12am (~6 months from now).

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche Jul 30, 2017

Contributor

😮 Great! Thanks for the news @habibalamin 😄

Contributor

CristhianMotoche commented Jul 30, 2017

😮 Great! Thanks for the news @habibalamin 😄

@habibalamin

This comment has been minimized.

Show comment
Hide comment
@habibalamin

habibalamin Mar 21, 2018

Contributor

So now that the latest version of base, 4.11.0.0, that was uploaded to Hackage on Monday, includes System.Environment.Blank, which works with Windows and POSIX, I think we ought to look back into this.

I know there are backward-compatibility (and forward-compatibility) concerns. Eventually, System.Environment.Blank will be folded into System.Environment in the next major GHC version (IIRC).

Contributor

habibalamin commented Mar 21, 2018

So now that the latest version of base, 4.11.0.0, that was uploaded to Hackage on Monday, includes System.Environment.Blank, which works with Windows and POSIX, I think we ought to look back into this.

I know there are backward-compatibility (and forward-compatibility) concerns. Eventually, System.Environment.Blank will be folded into System.Environment in the next major GHC version (IIRC).

@CristhianMotoche

This comment has been minimized.

Show comment
Hide comment
@CristhianMotoche

CristhianMotoche Mar 21, 2018

Contributor

Awesome. Thanks a lot @habibalamin

Contributor

CristhianMotoche commented Mar 21, 2018

Awesome. Thanks a lot @habibalamin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment