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

use msbuild -version, not %VisualStudioVersion% #92

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions colcon_cmake/task/cmake/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import shutil
import subprocess
import sys
import functools

from colcon_core.environment_variable import EnvironmentVariable
from colcon_core.subprocess import check_output
Expand Down Expand Up @@ -206,6 +207,20 @@ def get_project_file(path, target):
return project_file


@functools.lru_cache(maxsize=1)
def get_msbuild_version():
"""
Get the version of msbuild or throw an exception.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docblock should contain more information what kind of exceptions can be raised. This should use the :raises ExceptionType: ... style.


:rtype: str
"""
completed_process = subprocess.run(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please invoke check_output instead of run since that is what it is used for here.

['msbuild', '-version', '-noLogo'],
capture_output=True
)
return completed_process.stdout.decode(encoding=sys.getdefaultencoding())


def get_visual_studio_version():
"""
Get the Visual Studio version.
Expand Down
25 changes: 11 additions & 14 deletions colcon_cmake/task/cmake/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
from colcon_cmake.task.cmake import get_buildfile
from colcon_cmake.task.cmake import get_cmake_version
from colcon_cmake.task.cmake import get_generator
from colcon_cmake.task.cmake import get_msbuild_version
from colcon_cmake.task.cmake import get_variable_from_cmake_cache
from colcon_cmake.task.cmake import get_visual_studio_version
from colcon_cmake.task.cmake import has_target
from colcon_cmake.task.cmake import is_multi_configuration_generator
from colcon_core.environment import create_environment_scripts
Expand Down Expand Up @@ -148,25 +148,22 @@ async def _reconfigure(self, args, env):
cmake_args += ['-DCMAKE_INSTALL_PREFIX=' + args.install_base]
generator = get_generator(args.build_base, args.cmake_args)
if os.name == 'nt' and generator is None:
vsv = get_visual_studio_version()
if vsv is None:
try:
msbuild_version = get_msbuild_version()
except FileNotFoundError as e:
raise RuntimeError(
'VisualStudioVersion is not set, '
'please run within a Visual Studio Command Prompt.')
supported_vsv = {
'16.0': 'Visual Studio 16 2019',
'15.0': 'Visual Studio 15 2017',
'14.0': 'Visual Studio 14 2015',
}
if vsv not in supported_vsv:
'msbuild is not found, '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: instead if "is" it should be "was".

'please run within a Visual Studio Command Prompt.') from e
major_version = int(msbuild_version.split('.')[0])
if major_version < 14:
raise RuntimeError(
"Unknown / unsupported VS version '{vsv}'"
"Unknown / unsupported VS version '{msbuild_version}'"
.format_map(locals()))
cmake_args += ['-G', supported_vsv[vsv]]
cmake_args += ['-G', 'Visual Studio ' + str(major_version)]
# choose 'x64' on VS 14 and 15 if not specified explicitly
# since otherwise 'Win32' is the default for those
# newer versions default to the host architecture
if '-A' not in cmake_args and vsv in ('14.0', '15.0'):
if '-A' not in cmake_args and major_version < 16:
cmake_args += ['-A', 'x64']
if CMAKE_EXECUTABLE is None:
raise RuntimeError("Could not find 'cmake' executable")
Expand Down
2 changes: 2 additions & 0 deletions test/spell_check.words
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ ctests
dcmake
etree
findtext
functools
getaffinity
github
https
Expand All @@ -23,6 +24,7 @@ lstrip
makefile
makefiles
makeflags
maxsize
modline
msbuild
mtime
Expand Down