-
Notifications
You must be signed in to change notification settings - Fork 905
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
ChocolateyLastPathUpdate environment variable stores date as locale-specific #1604
Comments
@fredrikaverpil So we should probably store it as non-locale specific it sounds. |
So we've hit this on a 300+ large dev team and it brought down parts of our core infra. We've worked around it for now, but it seems that this has been open for quite some time... Any plans on fixing this soon? If not, I'll spend some time fixing this up and submitting a PR if that's OK? My intended fix in that case would be to just write the date as File Time. But I have a hard time seeing where this env var is used later on? If it's not used after being written for anything useful we could just as well stop writing the value? |
* Make the DateTime value written to Env be of type Long (FileTime) * Since it's not used for anything this seems to be safe to change.
Any update on this? Would be quite useful to get this merged and pushed in a new release. |
…stPathUpdate Due to the DateTime format used this would cause all kinds of issues when a tool or environment uses the environment variables. For example, a python script that reads all environment variables on a system with a Swedish DateTime culture set and tries to serialize the key/values as JSON would/could error out when a ÅÄ or Ö is written by choco into ChocolateyLastPathUpdate, it's also a hard bug to track down since in the Swedish case it would only trigger on a Saturday, Sunday or Monday where mentioned chars would appear. This change changes the formatting used to force it to a long in the form of using ToFileTime() on the returned Date object instead of the formatting flags used before the change.
@SneWs this should go in with 0.10.12. |
Due to the DateTime format used this would cause all kinds of issues when a tool or environment uses the environment variables. For example, a python script that reads all environment variables on a system with a Swedish DateTime culture set and tries to serialize the key/values as JSON would/could error out when a ÅÄ or Ö is written by choco into ChocolateyLastPathUpdate, it's also a hard bug to track down since in the Swedish case it would only trigger on a Saturday, Sunday or Monday where mentioned chars would appear. This change changes the formatting used to force it to a long in the form of using ToFileTime() on the returned Date object instead of the formatting flags used before the change.
* pr1719: (GH-1604) Fix: unicode char issues w/Env:ChocolateyLastPathUpdate
* stable: (GH-1604) Fix: unicode char issues w/Env:ChocolateyLastPathUpdate
This has been merged into stable at 7e1143a and will be released with 0.10.12. |
Thank you. And sorry for not being active the last few weeks, we've been moving to a new house and well, it's been quite time consuming. Great news that it finally goes into a release 👍 |
@SneWs I fully understand the moving aspect. Did that this summer and it was very time consuming. |
What You Are Seeing?
On my Windows 10 box, the
ChocolateyLastPathUpdate
environment variable's value contains Swedish, non-Ascii characters (such aså
orö
) for certain timestamps because my date and currencies settings in Windows are set to Swedish. This has caused several third-party applications which read the environment to crash or malfunction.Indeed, this problem lies more with the third-party applications than with choco. However, I'd argue it's bad form to store the date like this if this isn't necessary.
Example value when date and currency format is set to English:
Tue Apr 24 11:17:15 2018
.Unfortunately, I don't have an existing example string in Swedish anymore, as I had to change this. But as an example, Saturday is "Lördag" in Swedish and abbreviated into
Lör
.The text was updated successfully, but these errors were encountered: