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

$Env:ChocolateyInstall not set #68

Open
Jaykul opened this issue Mar 31, 2023 · 3 comments
Open

$Env:ChocolateyInstall not set #68

Jaykul opened this issue Mar 31, 2023 · 3 comments

Comments

@Jaykul
Copy link
Contributor

Jaykul commented Mar 31, 2023

I can't decide if I should file this as a bug here, or against Chocolatey or both.

I'll start here, because I think you need to fix it, even if they do.

It looks like you only set it if I hard-code it.

if ($InstallationDirectory)
{
[Environment]::SetEnvironmentVariable('ChocolateyInstall', $InstallationDirectory, 'Machine')
[Environment]::SetEnvironmentVariable('ChocolateyInstall', $InstallationDirectory, 'Process')
}

I don't need to hard code it, because you have code to recover, but your recovery code does not set the ChocolateyInstall environment variable:

$chocoPath = [Environment]::GetEnvironmentVariable('ChocolateyInstall')
if ([string]::IsNullOrEmpty($chocoPath))
{
$chocoPath = "$env:ALLUSERSPROFILE\Chocolatey"
}
if (-not (Test-Path -Path $chocoPath))
{
$chocoPath = "$env:SYSTEMDRIVE\ProgramData\Chocolatey"
}
$chocoExePath = Join-Path -Path $chocoPath -ChildPath 'bin'
if ($($env:Path).ToLower().Contains($($chocoExePath).ToLower()) -eq $false)
{
$env:Path = [Environment]::GetEnvironmentVariable('Path', [System.EnvironmentVariableTarget]::Machine)
}

However, you depend on this variable being set (in the process scope):

$CachePath = [io.path]::Combine($Env:ChocolateyInstall, 'cache', 'GetChocolateyPackageCache.xml')

I believe the chocolatey installer only sets this in the MACHINE scope, and then tells you to restart your shell. Perhaps you should explicitly read if from there on line 258? In fact, I think you should READ it from the MACHINE and then do your seftey check, and then SET it in MACHINE scope if it was missing or wrong, and PROCESS, regardless.

@gaelcolas
Copy link
Member

gaelcolas commented Apr 2, 2023

Thanks @Jaykul, I think I get what you're saying.
I'm changing the bit around the Environment variable $Env:ChocolateyInstall like so:

    Write-Verbose -Message 'Ensuring chocolatey commands are on the path.'
    [string] $chocoPath = ''

    if ($chocoPath = [Environment]::GetEnvironmentVariable('ChocolateyInstall','Machine'))
    {
        Write-Debug -Message ('The Machine Environment variable is already set to: ''{0}''.' -f $chocoPath)
    }
    else
    {
        # Checking if it was installed in AllUserProfile/Chocolatey (was default many years ago)
        $chocoPath = Join-Path -Path $env:ALLUSERSPROFILE -ChildPath 'Chocolatey'
        if (-not (Test-Path -Path $chocoPath))
        {
            # The AllUserProfile/Chocolatey folder does not exit, let's install in correct default location
            $chocoPath = Join-Path -Path $env:ProgramData -ChildPath 'Chocolatey'
        }

        # Set the Machine-scoped 'ChocolateyInstall' environement variable
        Write-Debug -Message ('Setting the Machine & Process Environment variable to: ''{0}''.' -f $chocoPath)
        [Environment]::SetEnvironmentVariable('ChocolateyInstall', $chocoPath, 'Machine')
        [Environment]::SetEnvironmentVariable('ChocolateyInstall', $chocoPath, 'Process')
    }

    $chocoExePath = Join-Path -Path $chocoPath -ChildPath 'bin'

    if (@($env:Path.ToLower() -split [io.path]::PathSeparator) -notcontains $chocoExePath.ToLower())
    {
        # we can't see the choco bin folder in $env:Path, trying to load from Machine.
        $env:Path = [Environment]::GetEnvironmentVariable('Path', 'Machine')
    }

The dependency on $env:ChocolateyInstall was to speed things up because running Choco every time for each resource was very slow, and since DSC imports the module in a new thread for each resource, we had to write to file.
I should improve that part too, but we'll wait for another time.

@gaelcolas gaelcolas mentioned this issue Apr 2, 2023
@Jaykul
Copy link
Contributor Author

Jaykul commented Apr 2, 2023

I think that after this:

if ($chocoPath = [Environment]::GetEnvironmentVariable('ChocolateyInstall','Machine'))

You still need to set it at the process scope, or not depend on it being set.

There's some weirdness in environment variables in Guest Configuration.

I suspect they are running each resource in a fresh process launched from the root process. If so, that means changes to environment variables DO NOT PERSIST at all, and there's no way to make them persist and have them visible within the same run. If that's true, you have to always explicitly get and set at machine scope to get persistence to follow-on resources.

@gaelcolas
Copy link
Member

This should be fixed in v0.2.0-preview0008 released earlier today.
When Choco errors (i.e. not installed), the error will also be brought up as a Reason.

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

No branches or pull requests

2 participants