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.

Sign up for GitHub

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

Jump to bottom

AutotoolsToolchain: curate paths coming from tools.build:compiler_executables on Windows before defining CC, CXX etc #12194

Closed
wants to merge 38 commits into from
Closed

AutotoolsToolchain: curate paths coming from tools.build:compiler_executables on Windows before defining CC, CXX etc #12194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
43ceb98
set LD to link & add -nologo
SpaceIm Sep 24, 2022
44ee28c
honor CC, CXX, LD from profile
SpaceIm Sep 24, 2022
bbc8cf7
handle more cases on windows
SpaceIm Sep 24, 2022
73345fa
refactor
SpaceIm Sep 24, 2022
dddfa50
also handle NM
SpaceIm Sep 25, 2022
1eed250
refactor again
SpaceIm Sep 25, 2022
6b69fd2
formatting
SpaceIm Sep 25, 2022
6b7743c
also handle AR
SpaceIm Sep 25, 2022
9977d83
refactor again
SpaceIm Sep 25, 2022
73c2e19
small change
SpaceIm Sep 25, 2022
6e9295b
add more variables
SpaceIm Sep 25, 2022
3ad51c3
add comments
SpaceIm Sep 25, 2022
305783b
add two new attributes to prefix CC, CXX & ARLIB with compatibility w…
SpaceIm Sep 25, 2022
fc3c72c
add cc, cxx, ld, ar, nm, objdump, ranlib & trip attributes
SpaceIm Sep 25, 2022
40fdb20
add compiler_wrapper & arlib_wrapper to arguments of constructor
SpaceIm Sep 25, 2022
f03d98d
default extra_options to None
SpaceIm Sep 25, 2022
c160c7b
typo
SpaceIm Sep 25, 2022
a81f43a
rename to arlib_wrapper to ar_wrapper
SpaceIm Sep 25, 2022
6a415fd
use VirtualBuildEnv to get env vars from profile
SpaceIm Sep 25, 2022
746bff5
do not add -nologo to AR if msvc
SpaceIm Sep 25, 2022
ea62645
don't check existence of wrapper
SpaceIm Sep 25, 2022
a2a2a5d
do not allow spaces in env var paths for autotools
SpaceIm Sep 25, 2022
7d4adf1
minor change
SpaceIm Sep 25, 2022
b539767
improve windows support
SpaceIm Sep 30, 2022
47b8213
move import of win32api where it is used
SpaceIm Sep 30, 2022
a3f77e1
add 4 conf to define build executables
SpaceIm Oct 1, 2022
39001d3
Merge branch 'develop' into autotoolstoolchain-msvc
SpaceIm Dec 1, 2022
49d2cd7
some fixes following merge
SpaceIm Dec 1, 2022
c994932
more fixes
SpaceIm Dec 1, 2022
c902918
minor change
SpaceIm Dec 1, 2022
e393600
less babysitting, only curate path
SpaceIm Dec 1, 2022
49264d8
minor change
SpaceIm Dec 1, 2022
8ab97ab
simplify curated_path
SpaceIm Dec 1, 2022
22abcaa
only manage env vars related to tools which may be provided by tools.…
SpaceIm Dec 2, 2022
e885850
do not compute these paths on instance creation
SpaceIm Dec 2, 2022
3bb2dc9
typo
SpaceIm Dec 2, 2022
3172628
simplify even more
SpaceIm Dec 2, 2022
b878025
Merge branch 'develop' into autotoolstoolchain-msvc
SpaceIm Jan 18, 2023
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: 12 additions & 3 deletions conan/tools/gnu/autotoolstoolchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
from conan.tools.env import Environment
from conan.tools.files.files import save_toolchain_args
from conan.tools.gnu.get_gnu_triplet import _get_gnu_triplet
from conan.tools.microsoft import VCVars, msvc_runtime_flag
from conan.tools.microsoft import VCVars, msvc_runtime_flag, unix_path
from conans.errors import ConanException
from conans.tools import args_to_string
import os


class AutotoolsToolchain:
Expand Down Expand Up @@ -127,14 +128,22 @@ def defines(self):
ret = [self.ndebug, self.gcc_cxx11_abi] + conf_flags + self.extra_defines
return self._filter_list_empty_fields(ret)

def _curated_path(self, path):
if path and os.path.exists(path):
if " " in path and os.name == "nt":
from conan.tools.microsoft.win32api import win32api
path = win32api.get_short_path_name(path)
path = unix_path(self._conanfile, path)
return path

def environment(self):
env = Environment()
compilers_by_conf = self._conanfile.conf.get("tools.build:compiler_executables", default={}, check_type=dict)
if compilers_by_conf:
compilers_mapping = {"c": "CC", "cpp": "CXX", "cuda": "NVCC", "fortran": "FC"}
compilers_mapping = {"asm": "CCAS", "c": "CC", "cpp": "CXX", "cuda": "NVCC", "fortran": "FC"}
for comp, env_var in compilers_mapping.items():
if comp in compilers_by_conf:
env.define(env_var, compilers_by_conf[comp])
env.define(env_var, self._curated_path(compilers_by_conf[comp]))
env.append("CPPFLAGS", ["-D{}".format(d) for d in self.defines])
env.append("CXXFLAGS", self.cxxflags)
env.append("CFLAGS", self.cflags)
Expand Down
1 change: 1 addition & 0 deletions conan/tools/microsoft/win32api/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from conan.tools.microsoft.win32api import win32api
22 changes: 22 additions & 0 deletions conan/tools/microsoft/win32api/win32api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import os

if os.name == "nt":
import ctypes
from ctypes import wintypes


def get_short_path_name(path):
Copy link
Member

Choose a reason for hiding this comment

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

This needs explanation why it is needed, I am not sure how this is related to improving the AutotoolsToolchain

Copy link
Contributor Author

@SpaceIm SpaceIm Sep 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's explained in #12194 (comment):

If one of these variables is set with a path, but this path contains spaces, it falls back to a default value, because scripts generated by autotools can't handle spaces in paths of these variables (even if they are protected, path is quoted etc). A warning is displayed in this case.

As a workaround on Windows, users may use short file names to refer to their compiler path if the long name contains spaces (PROGRA~1 instead of Program Files for example). I guess it might be possible for conan, if build machine is windows, to find the short file name (if it exists) of directories containing spaces, and do this conversion on the fly, but it's not implemented here (win32api.GetShortPathName from pywin32?).

Copy link
Member

Choose a reason for hiding this comment

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

It is too complicated, difficult to maintain. I prefer to limit the functionality to paths without spaces, honestly, we cannot add this complexity to satisfy every corner case in the world. If autotools does not support paths with spaces, it does not support paths with spaces, so things should go in paths without spaces.

Copy link
Contributor Author

@SpaceIm SpaceIm Sep 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

But why not implementing this workaround if it works? It's not so complicated and allows to support absolute path to Visual Studio. For me underlying build system is an internal detail.
It shouldn't fail to build if someone specificy some usual profile (CC set to a specific compiler, CC not set so it takes default compiler).
It should honor CC, CXX, LD, etc from profile (overridding CC to just cl is the very last solution because you may not honor compiler version from profile when there are several Visual Studio installations).

try:
_GetShortPathNameW = ctypes.windll.kernel32.GetShortPathNameW
_GetShortPathNameW.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD]
_GetShortPathNameW.restype = wintypes.DWORD
output_buf_size = 0
while True:
output_buf = ctypes.create_unicode_buffer(output_buf_size)
needed = _GetShortPathNameW(path, output_buf, output_buf_size)
if output_buf_size >= needed:
return output_buf.value
else:
output_buf_size = needed
except:
return path