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

Documentation: Implement rules for using the temporary folder #415

Closed
dhilgarth opened this issue Sep 20, 2015 · 14 comments
Closed

Documentation: Implement rules for using the temporary folder #415

dhilgarth opened this issue Sep 20, 2015 · 14 comments

Comments

@dhilgarth
Copy link

I now have had multiple occasions where an incorrect usage of $env:TEMP caused the package installation to fail.

There are two problems associated with directly using $env:TEMP:

  1. It might contain a space. Oftentimes, a path containing the value is passed without quotes as a command line argument
  2. It might be in 8.3 notation which is apparently not understood by all PowerShell commands and is also causing problems for some installers like the one of MS SQL Server 2014 Management Studio Express.

The documentation should insist on using (Get-Item $env:TEMP).FullName instead of simply $env:TEMP and it should also remember people to properly quote paths passed as command line arguments.

@aronovgj
Copy link

path might also have non-latin characters which would cause it to fail. someone reported this after one of my packages couldn't install. now i unpack my stuff in the package folder and not in temp.

@dhilgarth
Copy link
Author

This just emphasizes the need for documentation on what to use as the temporary path. I think some consistency is needed here.

@ferventcoder
Copy link
Member

Bug or enhancement?

@dhilgarth
Copy link
Author

Bug. Packages fail to install because of this

@ferventcoder ferventcoder added this to the 0.9.10.x milestone Sep 21, 2015
@ferventcoder ferventcoder modified the milestones: 0.9.10, 0.9.10.x Jan 11, 2016
@ferventcoder ferventcoder self-assigned this Jan 11, 2016
@ferventcoder
Copy link
Member

This may be fixed now.

@ferventcoder
Copy link
Member

About to find out.

@ferventcoder
Copy link
Member

It won't require a documentation change either. Chocolatey tells the executing scripts what the TEMP variable is, so we fix it to providing the full path and things are good. For spaces however, I'm not yet sure how that should be handled.

@ferventcoder
Copy link
Member

I'm going to create a new issue for not using 8.3 path for temp variable.

@ferventcoder
Copy link
Member

Also, the temp variable is controlled by the chocolatey cacheLocation setting - it defaults to TEMP when it is not set. You should have folks set that if they have issues with latin characters @aronovgj. I think that this fix I'm about to implement may fix both avenues.

@ferventcoder
Copy link
Member

This documentation may not be required.

@ferventcoder
Copy link
Member

I just pushed the latest beta - it should be available within about an hour or so after the verifier and the validator run. If you can give it a shot and see if this is still an issue - I will close this as not needed due to the fixes in #532.

https://chocolatey.org/packages/chocolatey/0.9.10-beta-20160111

If you find this is still an issue, we can reopen this for documentation purposes (or evaluate our options).

@dhilgarth
Copy link
Author

Not sure, I understand what happened here? Has some documentation been added to explain how to handle temporary paths? What's with the cacheLocation setting? First time I hear about it.

Maybe my issue was unclear? Many script internally use $env:TEMP to extract a downloaded zip file to a temporary location. Not sure how the cacheLocation command line switch plays into this.

@ferventcoder
Copy link
Member

Many script internally use $env:TEMP to extract a downloaded zip file to a temporary location. Not sure how the cacheLocation command line switch plays into this.

I guess a little explanation is in order. We set the TEMP environment variable before we let a package script run. We either set it to the actual TEMP location, or to cacheLocation as specified from the configuration. In #532, I fixed the issue of it using a short 8.3 path.

@ferventcoder
Copy link
Member

You can see the changes and possibly get a better understanding if you take a look at 76ae7e2

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

No branches or pull requests

3 participants