Visual Studio builds can fail during installation due to incorrect environment variable parsing

[KazNX]

This is a downstream bug caused by https://github.com/colcon/colcon-core/issues/248

Building a package using Visual Studio generator can fail during installation due to the afforementioned bug. The failure message looks something like what follows:

Build started 18/10/2019 2:21:14 PM.
Project "<project-path>\INSTALL.vcxproj" on node 1 (default targets).

C:\Program Files (x86)\Windows Kits\10\DesignTime\CommonConfiguration\Neutral\UAP\10.0.18362.0\UAP.props(51,18): error MSB4086: A numeric comparison was attempted on "$(VisualStudioVersion)" that evaluates to "16 [<project-path>\INSTALL.vcxproj]
C:\Program Files (x86)\Windows Kits\10\DesignTime\CommonConfiguration\Neutral\UAP\10.0.18362.0\UAP.props(51,18): error MSB4086: ProgramFiles(x86)=C:\Program Files (x86)" instead of a number, in condition "'$(VisualStudioVersion)' != '' and '$(VisualStudioVersion)' <= '14.0'". [<project-path>\INSTALL.vcxproj]
Done Building Project "<project-path>\INSTALL.vcxproj" (default targets) -- FAILED.

Build FAILED.

"<project-path>\INSTALL.vcxproj" (default target) (1) ->
  C:\Program Files (x86)\Windows Kits\10\DesignTime\CommonConfiguration\Neutral\UAP\10.0.18362.0\UAP.props(51,18): error MSB4086: A numeric comparison was attempted on "$(VisualStudioVersion)" that evaluates to "16 [<project-path>\INSTALL.vcxproj]
C:\Program Files (x86)\Windows Kits\10\DesignTime\CommonConfiguration\Neutral\UAP\10.0.18362.0\UAP.props(51,18): error MSB4086: ProgramFiles(x86)=C:\Program Files (x86)" instead of a number, in condition "'$(VisualStudioVersion)' != '' and '$(VisualStudioVersion)' <= '14.0'". [<project-path>\INSTALL.vcxproj]

    0 Warning(s)
    1 Error(s)

Note the message "$(VisualStudioVersion)" that evaluates to.... The value should be simply 16 (for Visual Studio 2019). The message about is a little difficult to decode, because VisualStudioVersion contains a newline character, but the value is actually 16\nProgramFiles(x86)=C:\Program Files (x86). This has come about as described in https://github.com/colcon/colcon-core/issues/248 when decoding the environment and VisualStudioVersion is followed by ProgramFiles(x86).

It's highy dependent on the order environment variables, and can present itself somewhat sporadically by restarting the environment from which colcon is run.

[dirk-thomas]

Can you please include steps of a concrete problem to reproduce.

[KazNX]

The stochactic ordering of the problem makes that tricky, so it may take a while to setup an example.

[KazNX]

I've gone for a reproduction by injection because of the way the random variable ordering makes it hard to reproduce. Note that I am assuming use of Visual Studio 2019 on Windows 10 x64.

• First follow the steps to reproduce https://github.com/colcon/colcon-core/issues/248 up until modifying the code in get_environment_variables()
• Use the code below for get_environment_variables() instead of the given in the bug mentioned above.
• Run colcon build --event-handlers console_direct+
• Observe the failed installation step as described above

async def get_environment_variables(cmd, *, cwd=None, shell=True):
    """
    Get the environment variables from the output of the command.

    :param args: the sequence of program arguments
    :param cwd: the working directory for the subprocess
    :param shell: whether to use the shell as the program to execute
    :rtype: dict
    """
    output = await check_output(cmd, cwd=cwd, shell=shell)
    env = OrderedDict()
    # Inject the bug prone sequence.
    output += b'VisualStudioVersion=16\r\nProgramFiles(x86)=C:\\Program Files (x86)'
    # Make sure we skip the first occurrence of VisualStudioVersion. We want
    # to use the problematic sequence we just injected
    skip_vsver = True
    for line in output.splitlines():
        line = line.rstrip()
        if not line:
            continue
        encoding = locale.getpreferredencoding()
        try:
            line = line.decode(encoding)
        except UnicodeDecodeError:
            line_replaced = line.decode(encoding=encoding, errors='replace')
            logger.warning(
                'Failed to decode line from the environment using the '
                "encoding '{encoding}': {line_replaced}".format_map(locals()))
            continue
        # Skip first occurrence of VisualStudioVersion
        if skip_vsver and re.match('^VisualStudioVersion=.*', line):
            skip_vsver = False
            continue
        parts = line.split('=', 1)
        # if len(parts) == 2 and re.match('^[a-zA-Z0-9_\(\)%]+$', parts[0]):
        if len(parts) == 2 and re.match('^[a-zA-Z0-9_%]+$', parts[0]):
            # add new environment variable
            env[parts[0]] = parts[1]
        else:
            # assume a line without an equal sign or with a "key" which is not
            # a valid name is a continuation of the previous line
            if env:
                env[list(env.keys())[-1]] += '\n' + line
                print("combine '" + line + "' into '" + list(env.keys())[-1] + "'")
    assert len(env) > 0, "The environment shouldn't be empty"
    return env

[dirk-thomas]

@KazNX With colcon/colcon-core#248 merged can this ticket be closed, too?

[KazNX]

Yes, this should be closed. This was just the most critical symptom of https://github.com/colcon/colcon-core/issues/248