GitTools / GitVersion

From git log to SemVer in no time
https://gitversion.net/docs/
MIT License
2.84k stars 650 forks source link

Error with /exec: System.ArgumentException: Item has already been added. Key in dictionary: 'CONFIGURATION' Key being added: 'Configuration' #1134

Closed sergey-s-betke closed 7 years ago

sergey-s-betke commented 7 years ago

I have error with /exec key. Build session log - https://ci.appveyor.com/project/sergey-s-betke/itg-metrcontrol-2-4/build/0.1.0-bootstrapper-vnext.128.build.62.

log:

INFO [01/11/17 7:26:09:71] Launching cmd.exe /C update_version.h.cmd version.h
ERROR [01/11/17 7:26:09:72] An unexpected error occurred:
  System.ArgumentException: Item has already been added. Key in dictionary: 'CONFIGURATION'  Key being added: 'Configuration'
  at System.Collections.Hashtable.Insert(Object key, Object nvalue, Boolean add)
  at System.Diagnostics.ProcessStartInfo.get_EnvironmentVariables()
  at GitTools.ProcessHelper.Run(Action`1 output, Action`1 errorOutput, TextReader input, String exe, String args, String workingDirectory, KeyValuePair`2[] environmentalVariables)
  at GitVersion.SpecifiedArgumentRunner.RunExecCommandIfNeeded(Arguments args, String workingDirectory, VersionVariables variables)
  at GitVersion.SpecifiedArgumentRunner.Run(Arguments arguments, IFileSystem fileSystem)
  at GitVersion.Program.VerifyArgumentsAndRun()
INFO [01/11/17 7:26:09:72] 
INFO [01/11/17 7:26:09:72] Attempting to show the current git graph (please include in issue): 
INFO [01/11/17 7:26:09:72] Showing max of 100 commits

In GitVersion environment exists CONFIGURATION variable (from AppVeyor...), and, project has Configuration variable. I think, collection (for environment variables) key must be case independent!!!

Excuse for my English...

Please, help.

asbjornu commented 7 years ago

@sergey-s-betke: What are the values of the CONFIGURATION and Configuration environment variables, respectively?

sergey-s-betke commented 7 years ago

Release in my scenario. (Release | Debug, but in my AppVeyor scenario - just Release).

asbjornu commented 7 years ago

If both variables have the same value, what's the value in having both available in a case-sensitive way? Perhaps what we can do in GitVersion is to do a case-insensitive search for both the key and value and if both match the key/value being added, we can drop it (perhaps with a warning).

sergey-s-betke commented 7 years ago

Yes! Now AppVeyor generates upper case variable CONFIGURATION, and MSBuild generates mixed case variable Configuration. But environment variables must be case-insensitive. I think, if exists a few variables with same key (case-insensitive comparison), we must drop second and other variables with same key, and no matter identical values or not. May be with warning, but with ZERO (ERROR_SUCCESS) EXIT CODE!

sergey-s-betke commented 7 years ago

@asbjornu , I think, is a bag, not a enhancement :-) Now I can not build my project on AppVeyor build servers!

asbjornu commented 7 years ago

@sergey-s-betke: I'm not sure this is actually a bug in GitVersion, seeing how it's the System.Diagnostics.ProcessStartInfo.get_EnvironmentVariables() method that fails. It might be seen as a bug, but I'm not certain there's anything we can do about it in GitVersion, to be honest.

sergey-s-betke commented 7 years ago

https://github.com/dotnet/corefx/issues/13146

sergey-s-betke commented 7 years ago

http://stackoverflow.com/questions/8732816/c-sharp-system-diagnostics-processstartinfo-environmentvariables-being-case-inse

@asbjornu, please, switch to .Net 4.0! It is a solution!

asbjornu commented 7 years ago

@sergey-s-betke: Thanks for the reference. From what I understand of that issue, it is fixed in .NET Core, not in any other version of .NET. GitVersion already targets .NET Framework v4.0.

JakeGinnivan commented 7 years ago

I am unsure how we can solve this, it is an environmental issue which .NET just does not support. The resolution here is to ensure that you standardise the casing of environmental variables.

If you would like to try and submit a pull request with a fix for this then please go ahead @sergey-s-betke. Am going to close this issue as an upstream issue.