10
\$\begingroup\$

As a rule, whenever I find myself doing something that nobody else is doing, I have to be quite suspicious about what I'm doing. This is why I want to get code review to verify I'm not doing something insane with the build events for what it's not usually intended.

Contributing to Rubberduck VBA project, I have a PR that creates a new Visual Studio project, named Rubberduck.Deployment. The main purpose of the project is to assist at the build time the extraction of the data that the installer will need to perform its thing. Therefore it will generate files at build time which can vary from build to build as the codebase is updated. Those generated files are then consumed by Inno Setup. My constraint is to be able to support both building locally and remotely via the AppVeyor.

The key code to review is this entry in the Post Build.

<PreBuildEvent>
  %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe 
    -ExecutionPolicy Bypass 
    -command "& '$(ProjectDir)PreInnoSetupConfiguration.ps1' 
      -WorkingDir '$(ProjectDir)'"
</PreBuildEvent>
<PostBuildEvent>
  %SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe 
    -ExecutionPolicy Bypass 
    -command "& '$(ProjectDir)BuildRegistryScript.ps1' 
      -config '$(ConfigurationName)' 
      -builderAssemblyPath '$(TargetPath)' 
      -netToolsDir '$(FrameworkSDKDir)bin\NETFX 4.6.1 Tools\' 
      -wixToolsDir '$(ProjectDir)WixToolset\' 
      -sourceDir '$(TargetDir)' 
      -targetDir '$(TargetDir)' 
      -projectDir '$(ProjectDir)' 
      -includeDir '$(ProjectDir)InnoSetup\Includes\' 
      -filesToExtract 'Rubberduck.dll|Rubberduck.API.dll'"
</PostBuildEvent>

As indicated, we invoke PowerShell scripts which is included in the Visual Studio project (and being a script, it doesn't directly participate in the building of the Visual Studio project). Here's complete code for the post build script.

# The parameters should be supplied by the Build event of the project
# in order to take macros from Visual Studio to avoid hard-coding
# the paths. To simplify the process, the project should have a 
# reference to the projects that needs to be registered, so that 
# their DLL files will be present in the $(TargetDir) macro. 
#
# Possible syntax for Post Build event of the project to invoke this:
# C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe 
#  -command "$(ProjectDir)BuildRegistryScript.ps1 
#  -config '$(ConfigurationName)' 
#  -builderAssemblyPath '$(TargetPath)' 
#  -netToolsDir '$(FrameworkSDKDir)bin\NETFX 4.6.1 Tools\' 
#  -wixToolsDir '$(SolutionDir)packages\WiX.Toolset.3.9.1208.0\tools\wix\' 
#  -sourceDir '$(TargetDir)' 
#  -targetDir '$(TargetDir)' 
#  -projectDir '$(ProjectDir)'
#  -includeDir '$(ProjectDir)InnoSetup\Includes\'
#  -filesToExtract 'Rubberduck.dll'"
param (
    [Parameter(Mandatory=$true)][string]$config,
    [Parameter(Mandatory=$true)][string]$builderAssemblyPath,
    [Parameter(Mandatory=$true)][string]$netToolsDir,
    [Parameter(Mandatory=$true)][string]$wixToolsDir,
    [Parameter(Mandatory=$true)][string]$sourceDir,
    [Parameter(Mandatory=$true)][string]$targetDir,
    [Parameter(Mandatory=$true)][string]$projectDir,
    [Parameter(Mandatory=$true)][string]$includeDir,
    [Parameter(Mandatory=$true)][string]$filesToExtract
)

function Get-ScriptDirectory
{
  $Invocation = (Get-Variable MyInvocation -Scope 1).Value;
  Split-Path $Invocation.MyCommand.Path;
}

# Invokes a Cmd.exe shell script and updates the environment.
function Invoke-CmdScript {
  param(
    [String] $scriptName
  )
  $cmdLine = """$scriptName"" $args & set"
  & $Env:SystemRoot\system32\cmd.exe /c $cmdLine |
  select-string '^([^=]*)=(.*)$' | foreach-object {
    $varName = $_.Matches[0].Groups[1].Value
    $varValue = $_.Matches[0].Groups[2].Value
    set-item Env:$varName $varValue
  }
}

# Returns the current environment.
function Get-Environment {
  get-childitem Env:
}

# Restores the environment to a previous state.
function Restore-Environment {
  param(
    [parameter(Mandatory=$TRUE)]
      [System.Collections.DictionaryEntry[]] $oldEnv
  )
  # Remove any added variables.
  compare-object $oldEnv $(Get-Environment) -property Key -passthru |
  where-object { $_.SideIndicator -eq "=>" } |
  foreach-object { remove-item Env:$($_.Name) }
  # Revert any changed variables to original values.
  compare-object $oldEnv $(Get-Environment) -property Value -passthru |
  where-object { $_.SideIndicator -eq "<=" } |
  foreach-object { set-item Env:$($_.Name) $_.Value }
}

# Remove older imported registry scripts for debug builds.
function Clean-OldImports
{
    param(
        [String] $dir
    )
    $i = 0;
    Get-ChildItem $dir -Filter DebugRegistryEntries.reg.imported_*.txt | 
    Sort-Object Name -Descending |
    Foreach-Object {
        if($i -ge 10) {
            $_.Delete();
        }
        $i++;
    }
}

Set-StrictMode -Version latest;
$ErrorActionPreference = "Stop";
$DebugUnregisterRun = $false;

try
{
    # Clean imports older than 10 builds
    Clean-OldImports ((Get-ScriptDirectory) + "\LocalRegistryEntries");;

    # Allow multiple DLL files to be registered if necessary
    $separator = "|";
    $option = [System.StringSplitOptions]::RemoveEmptyEntries;
    $files = $filesToExtract.Split($separator, $option);

    # Load the Deployment DLL
    [System.Reflection.Assembly]::LoadFrom($builderAssemblyPath);

    # Determine if MIDL is available for building
    $devPath = $Env:ProgramFiles + "*\Microsoft Visual Studio\*\*\Common*\Tools\VsDevCmd.bat";
    $devPath = Resolve-Path -Path $devPath;
    if($devPath)
    {
        # Additional verifications as some versions of VsDevCmd.bat might not initialize the environment for C++ build tools
        $result = Get-Module -ListAvailable -Name "VSSetup" -ErrorAction SilentlyContinue;
        if(!$result)
        {
            Write-Warning "VSSetup not installed; extracting...";
            Expand-Archive "$projectDir\OleWoo\VSSetup.zip" "$([Environment]::GetFolderPath("MyDocuments"))\WindowsPowerShell\Modules\VSSetup" -Force
        }

        try {
            Import-Module VSSetup -Force:$true;
            $result = Get-VSSetupInstance | Select-VSSetupInstance -Latest -Require Microsoft.VisualStudio.Component.VC.Tools.x86.x64;
        } catch {
            $result = $null;
            Write-Warning "Error occurred with using VSSetup module";
            Write-Error ($_);
        }

        if(!$result)
        {
            $devPath = $null;
            Write-Warning "Cannot locate the VS Setup instance capable of building with C++ build tools";
        }
    }

    if(!$devPath)
    {
        Write-Warning "Cannot locate the VsDevCmd.bat to initialize C++ build tools; falling back to tlbexp.exe....";
    }

    Write-Host "";

    foreach($file in $files)
    {
        Write-Host "Processing '$file'";
        Write-Host "";

        $dllFile = [System.String]$file;
        $idlFile = [System.String]($file -replace ".dll", ".idl");
        $tlb32File = [System.String]($file -replace ".dll", ".x32.tlb");
        $tlb64File = [System.String]($file -replace ".dll", ".x64.tlb");

        $sourceDll = $sourceDir + $file;
        $targetDll = $targetDir + $file;
        $sourceTlb32 = $sourceDir + $tlb32File;
        $targetTlb32 = $targetDir + $tlb32File;
        $sourceTlb64 = $sourceDir + $tlb64File;
        $targetTlb64 = $targetDir + $tlb64File;
        $dllXml = $targetDll + ".xml";
        $tlbXml = $targetTlb32 + ".xml";

        # Write-Host "Variable printout:"
        # Write-Host "dllFile = $dllFile";
        # Write-Host "idlFile = $idlFile";
        # Write-Host "tlb32File = $tlb32File";
        # Write-Host "tlb64File = $tlb64File";
        # Write-Host "sourceDll = $sourceDll";
        # Write-Host "targetDll = $targetDll";
        # Write-Host "sourceTlb32 = $sourceTlb32";
        # Write-Host "targetTlb32 = $targetTlb32";
        # Write-Host "sourceTlb64 = $sourceTlb64";
        # Write-Host "targetTlb64 = $targetTlb64";
        # Write-Host "dllXml = $dllXml";
        # Write-Host "tlbXml = $tlbXml";
        # Write-Host "targetDir = $targetDir";
        # Write-Host "";

        # Use for debugging issues with passing parameters to the external programs
        # Note that it is not legal to have syntax like `& $cmdIncludingArguments` or `& $cmd $args`
        # For simplicity, the arguments are pass in literally.
        # & "C:\GitHub\Rubberduck\Rubberduck\Rubberduck.Deployment\echoargs.exe" ""$sourceDll"" /win32 /out:""$sourceTlb"";

        # Compile TLB files using MIDL
        if($devPath)
        {
            $idlGenerator = New-Object Rubberduck.Deployment.IdlGeneration.IdlGenerator;

            $idl = $idlGenerator.GenerateIdl($sourceDll);
            $encoding = New-Object System.Text.UTF8Encoding $true;
            [System.IO.File]::WriteAllLines($idlFile, $idl, $encoding);

            $origEnv = Get-Environment;
            try {
                Invoke-CmdScript "$devPath";

                if($targetDir.EndsWith("\"))
                {
                    $targetDirWithoutSlash = $targetDir.Substring(0,$targetDir.Length-1);
                }
                else
                {
                    $targetDirWithoutSlash = $targetDir;
                }

                & midl.exe /win32 /tlb ""$tlb32File"" ""$idlFile"" /out ""$targetDirWithoutSlash"";
                & midl.exe /amd64 /tlb ""$tlb64File"" ""$idlFile"" /out ""$targetDirWithoutSlash"";
            } catch {
                throw;
            } finally {
                Restore-Environment $origEnv;
            }
        }

        # Compile TLB files using tlbexp.exe
        if(!$devPath)
        {
            $cmd = "{0}tlbexp.exe" -f $netToolsDir;
            & $cmd ""$sourceDll"" /win32 /out:""$sourceTlb32"";
            & $cmd ""$sourceDll"" /win64 /out:""$sourceTlb64"";
        }

        # Harvest both DLL and TLB files using WiX's heat.exe, generating XML files
        $cmd = "{0}heat.exe" -f $wixToolsDir;
        & $cmd file ""$sourceDll"" -out ""$dllXml"";
        & $cmd file ""$sourceTlb32"" -out ""$tlbXml"";

        # Initialize the registry builder with the provided XML files
        $builder = New-Object Rubberduck.Deployment.Builders.RegistryEntryBuilder;
        $entries = $builder.Parse($tlbXml, $dllXml);

        # For debugging
        # $entries | Format-Table | Out-String |% {Write-Host $_};

        $writer = New-Object Rubberduck.Deployment.Writers.InnoSetupRegistryWriter;
        $content = $writer.Write($entries, $dllFile, $tlb32File, $tlb64File);

        # The file must be encoded in UTF-8 BOM
        $regFile = ($includeDir + ($file -replace ".dll", ".reg.iss"));
        $encoding = New-Object System.Text.UTF8Encoding $true;
        [System.IO.File]::WriteAllLines($regFile, $content, $encoding);
        $content = $null;

        # Register the debug build on the local machine
        if($config -eq "Debug")
        {
            if(!$DebugUnregisterRun) 
            {
                # First see if there are registry script from the previous build
                # If so, execute them to delete previous build's keys (which may
                # no longer exist for the current build and thus won't be overwritten)
                $dir = ((Get-ScriptDirectory) + "\LocalRegistryEntries");
                $regFileDebug = $dir + "\DebugRegistryEntries.reg";

                if (Test-Path -Path $dir -PathType Container)
                {
                    if (Test-Path -Path $regFileDebug -PathType Leaf)
                    {
                        $datetime = Get-Date;
                        if ([Environment]::Is64BitOperatingSystem)
                        {
                            & reg.exe import $regFileDebug /reg:32;
                            & reg.exe import $regFileDebug /reg:64;
                        }
                        else 
                        {
                            & reg.exe import $regFileDebug;
                        }
                        & reg.exe import ($dir + "\RubberduckAddinRegistry.reg");
                        Move-Item -Path $regFileDebug -Destination ($regFileDebug + ".imported_" + $datetime.ToUniversalTime().ToString("yyyyMMddHHmmss") + ".txt" );
                    }
                }
                else
                {
                    New-Item $dir -ItemType Directory;
                }

                $DebugUnregisterRun = $true;
            }

            # NOTE: The local writer will perform the actual registry changes; the return
            # is a registry script with deletion instructions for the keys to be deleted
            # in the next build.
            $writer = New-Object Rubberduck.Deployment.Writers.LocalDebugRegistryWriter;
            $content = $writer.Write($entries, $dllFile, $tlb32File, $tlb64File);

            $encoding = New-Object System.Text.ASCIIEncoding;
            [System.IO.File]::AppendAllText($regFileDebug, $content, $encoding);
        }

        Write-Host "Finished processing '$file'";
        Write-Host "";
    }

    Write-Host "Finished processing all files";
}
catch
{
    Write-Error ($_);
    # Cause the build to fail
    throw;
}

In start of the script, we do this:

[System.Reflection.Assembly]::LoadFrom($builderAssemblyPath);

Which means we load the DLL that was just built by the project (it's literally the output of the Rubberduck.Deployment project) which the powershell script goes on to invoke methods and then eventually write out a file:

$builder = New-Object Rubberduck.Deployment.Builders.RegistryEntryBuilder;
$entries = $builder.Parse($tlbXml, $dllXml);
....
$writer = New-Object Rubberduck.Deployment.Writers.InnoSetupRegistryWriter;
$content = $writer.Write($entries, $dllFile, $tlb32File, $tlb64File);

$regFile = ($includeDir + ($file -replace ".dll", ".reg.iss"));
$encoding = New-Object System.Text.UTF8Encoding $true;
[System.IO.File]::WriteAllLines($regFile, $content, $encoding);

The autogenerated file is then used as a input to the Inno Setup compiler which is used during the AppVeyor build to build a complete installer for the Rubberduck addin. But for a local debug build where we don't use installer, we run this section:

# Register the debug build on the local machine
if($config -eq "Debug")
{
    if(!$DebugUnregisterRun) 
    {
        # First see if there are registry script from the previous build
        # If so, execute them to delete previous build's keys (which may
        # no longer exist for the current build and thus won't be overwritten)
        $dir = ((Get-ScriptDirectory) + "\LocalRegistryEntries");
        $regFileDebug = $dir + "\DebugRegistryEntries.reg";
        if (Test-Path -Path $dir -PathType Container)
        {
            if (Test-Path -Path $regFileDebug -PathType Leaf)
            {
                $datetime = Get-Date;
                if ([Environment]::Is64BitOperatingSystem)
                {
                    & reg.exe import $regFileDebug /reg:32;
                    & reg.exe import $regFileDebug /reg:64;
                }
                else 
                {
                    & reg.exe import $regFileDebug;
                }
                & reg.exe import ($dir + "\RubberduckAddinRegistry.reg");
                Move-Item -Path $regFileDebug -Destination ($regFileDebug + ".imported_" + $datetime.ToUniversalTime().ToString("yyyyMMddHHmmss") + ".txt" );
            }
        }
        else
        {
            New-Item $dir -ItemType Directory;
        }
        $DebugUnregisterRun = $true;
    }

    # NOTE: The local writer will perform the actual registry changes; the return
    # is a registry script with deletion instructions for the keys to be deleted
    # in the next build.
    $writer = New-Object Rubberduck.Deployment.Writers.LocalDebugRegistryWriter;
    $content = $writer.Write($entries, $dllFile, $tlb32File, $tlb64File);

    $encoding = New-Object System.Text.ASCIIEncoding;
    [System.IO.File]::AppendAllText($regFileDebug, $content, $encoding);
}

The writer will actually create the registry in the developer's HKCU registry to register the debug build to COM among other things. As it does that, it also generates a registry script which is then saved to the disk that enables deletions of all keys it created. That file is then used in subsequent build to delete the previous build's old keys, ensuring that developer don't end up with 1000s of stale keys as they make changes to the objects that may or may not change the registration.

This all works all nicely but as I said at start, this is quite unusual use of Post Build event and I'm wondering if that is a code smell in itself. I'm also interested in hearing whether we can do this better to make the process more seamless and less rube-goldsberg-esque.

Note that while I could have just invoked the powershell script and the DLL inside the AppVeyor, this would not achieve the goal of being able to build and generate the files when building locally which can't be then used for unit tests, debugging, or just simply inspecting. More importantly, I don't want to have one process for building locally and other process for building on AppVeyor, as that has potential to create bugs that we can't see due to missed differences in the build.

Questions to answer:

  1. Is this a reasonable method to customize a build process? Are there better ways?

  2. I don't like that the macro $(FrameworkSDKDir) is basically undocumented to a C# project. AFAICT, it's only used on a C++ project. Am I going to run into trouble for using that macro? In tests, it seems to work but...? What about a new version? All this is just to be able to run tlbexp.exe which AFAICT isn't in the PATH variable.

  3. I currently have no exact error handling strategy; any error will cause the build as whole to fail. That might be acceptable since no output means build can't be completed anyway but it might violate the principle of least astonishment. I'm also annoyed that currently any errors will just give out a generic the command exited with exit code -1 in the Error List without giving any actual information about the errors. Currently, the contributors must go to the Output for the Build stream to see the actual error from the powershell script. Can we improve on that?

  4. Can we improve the environment check? Because we use MIDL compiler which is technically a C++ build tool and thus not a part of a normal C# build process, we must configure the environment. However, I've found it quite hard to reliably detect whether the installed Visual Studio supports C++ build tools or not. I originally checked for existence of VsDevCmd.bat but this has had false positives which causes build process to attempt to compile using MIDL and then fail.

Note on #4: Since the original path, a contributor found a hole in the logic and the script was since tightened on its check, with a section dedicated to analyzing the presence of C++ build tools, starting on line 106

\$\endgroup\$
  • \$\begingroup\$ To be fair, building a VBIDE add-in that exposes a COM API isn't something everybody else does either ;-) \$\endgroup\$ – Mathieu Guindon Mar 22 '18 at 19:36
  • \$\begingroup\$ Touché, @MathieuGuindon Still it's C# project, so funky stuff needs to be checked. I added the complete PS script. \$\endgroup\$ – this Mar 22 '18 at 19:47
  • 1
    \$\begingroup\$ Why did you tag this with C# if there isn't any C# involved ? \$\endgroup\$ – Heslacher Sep 18 '18 at 5:47
  • \$\begingroup\$ Because the project builds a C# dll which itself gets called by the powershell. Note this line in the first code block $builder = New-Object Rubberduck.Deployment.Builders.RegistryEntryBuilder; \$\endgroup\$ – this Sep 18 '18 at 23:35
  • 1
    \$\begingroup\$ But the code in question is Powershell hence you should remove the C# tag. Basically it doesn't build a C# dll but a .NET dll but I would say that you shouldn't tag it .NET either. \$\endgroup\$ – Heslacher Sep 19 '18 at 4:43
2
\$\begingroup\$

After a few months of usage, I'm inclined to think that while this method does work and probably will be best investment for more common scenario where you want to set up an automated build process on a dedicated server, it is a poor fit for our specific scenario of making it easy to build anywhere on anyone's machine with minimum changes. Here's few reasons:

1) Visual Studio and PowerShell actually are not integrated.

At the first glance, it appears that there's a high level of integration -- Package Manager Console is in fact a PowerShell console, right? However, in practice, we have no control over the powershell's version. Some may be stuck due to corporate policy or other reasons with PowerShell 2.0, and therefore cannot leverage the package management feature that exists in PowerShell 3.0/4.0/5.0.

Then there's the execution policy. While that can be worked around, it is yet another hinderance.

2) Poor error reporting

Because the PowerShell is running the show, if there's been an error, all Visual Studio ends up showing is a " exited with a code -1" in the Error List with no details. One'd have to scour through the debug output to locate the actual error that caused the script to fail. That does not make it easy to diagnose and resolve errors from the user's POV.

3) Double-double quoting

Because the command to execute script goes through the cmd.exe, it necessitates invoking the PowerShell, then passing in the actual command. That makes for more complicated setup as you must quote/escape for both cmd.exe and PowerShell.

4) Why mix code?

We have the script invoking some C# code in an assembly that's just built. But if we're writing C#... why not do it all in C#? In fact creating a custom build task is not that hard to do. It boils down to the 3 general steps:

1) Create a C# class that derives from MSBuild's ITask interface

2) Add a UsingTask XML element to the csproj that points to the assembly containing the class.

3) Add the task (perhaps within a Target for example)

Additionally, this offers strong-typing, even in the csproj format as Visual Studio is able to see the properties and provide intellisense support for constructing the custom task node.

5) Temptation to write quick'n'dirty code

This is more reflective of the programmer than the language. It is possible to write clean code in PowerShell bue due to the nature of PowerShell, it's also too easy to write a single god function with global state, etc. etc. Thus, it takes more discipline to write clean code in PowerShell. This also kind of runs contrary to the premise that you should write in PowerShell because it's quick to do what you need with minimum effort. We have encountered and had to fix bugs resulting from global namespace pollution of re-used variables.

So in short - PowerShell scripting to support custom build actions can work but requires too much work and special-casing across different environments. Setting a custom build task ensures that it will run in same version of Visual Studio that the solution itself requires, so there is no additional dependencies that must be satisfied in order for the build task to run.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.