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

Type setuptools/msvc.py dir methods and properties #4755

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all 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
43 changes: 22 additions & 21 deletions setuptools/msvc.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

from more_itertools import unique_everseen

from ._path import StrPath

import distutils.errors

if TYPE_CHECKING:
Expand Down Expand Up @@ -135,7 +137,7 @@ def target_dir(self, hidex86=False, x64=False) -> str:
else r'\%s' % self.target_cpu
)

def cross_dir(self, forcex86=False):
def cross_dir(self, forcex86=False) -> str:
r"""
Cross platform specific subfolder.

Expand Down Expand Up @@ -306,7 +308,7 @@ def microsoft(self, key, x86=False):
node64 = '' if self.pi.current_is_x86() or x86 else 'Wow6432Node'
return os.path.join('Software', node64, 'Microsoft', key)

def lookup(self, key, name):
def lookup(self, key: str, name: str) -> str | None:
"""
Look for values in registry in Microsoft software registry.

Expand All @@ -319,7 +321,7 @@ def lookup(self, key, name):

Return
------
str
str | None
value
"""
key_read = winreg.KEY_READ
Expand Down Expand Up @@ -486,7 +488,7 @@ def _as_float_version(version):
return float('.'.join(version.split('.')[:2]))

@property
def VSInstallDir(self):
def VSInstallDir(self) -> str:
"""
Microsoft Visual Studio directory.

Expand All @@ -504,7 +506,7 @@ def VSInstallDir(self):
return self.ri.lookup(self.ri.vs, '%0.1f' % self.vs_ver) or default

@property
def VCInstallDir(self):
def VCInstallDir(self) -> str:
"""
Microsoft Visual C++ directory.

Expand Down Expand Up @@ -607,7 +609,7 @@ def WindowsSdkLastVersion(self):
return self._use_last_dir_name(os.path.join(self.WindowsSdkDir, 'lib'))

@property
def WindowsSdkDir(self) -> str | None: # noqa: C901 # is too complex (12) # FIXME
def WindowsSdkDir(self) -> str: # noqa: C901 # is too complex (12) # FIXME
"""
Microsoft Windows SDK directory.

Expand Down Expand Up @@ -650,13 +652,13 @@ def WindowsSdkDir(self) -> str | None: # noqa: C901 # is too complex (12) # F
return sdkdir

@property
def WindowsSDKExecutablePath(self):
def WindowsSDKExecutablePath(self) -> str | None:
"""
Microsoft Windows SDK executable directory.

Return
------
str
str | None
path
"""
# Find WinSDK NetFx Tools registry dir name
Expand Down Expand Up @@ -687,7 +689,7 @@ def WindowsSDKExecutablePath(self):
return None

@property
def FSharpInstallDir(self):
def FSharpInstallDir(self) -> str:
"""
Microsoft Visual F# directory.

Expand All @@ -700,7 +702,7 @@ def FSharpInstallDir(self):
return self.ri.lookup(path, 'productdir') or ''

@property
def UniversalCRTSdkDir(self):
def UniversalCRTSdkDir(self) -> str:
"""
Microsoft Universal CRT SDK directory.

Expand All @@ -716,9 +718,9 @@ def UniversalCRTSdkDir(self):
for ver in vers:
sdkdir = self.ri.lookup(self.ri.windows_kits_roots, 'kitsroot%s' % ver)
if sdkdir:
return sdkdir or ''
return sdkdir

return None
return ''
Comment on lines -719 to +723
Copy link
Contributor Author

@Avasam Avasam Nov 20, 2024

Choose a reason for hiding this comment

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

This one I'm not entirely certain of, and does change the possible return types.

dir-related properties of this class are a mixed bag on whether they return None or '' as a fallthrough case. And are almost never documented as returning None (which is fixed in this PR).

However all other properties that can return None are checked for truthiness when used, this one isn't. (hence it was causing issues for #4744)

The redundant or '' makes me think this may have been intended to not return None. But the true intention is unclear to me.


After further research, it was auto-added here as an explicit return: https://github.com/pypa/setuptools/pull/4169/files#diff-2f10e0b183f00587feb575c7716d38f4951d4f7ded5b714396e96066ae7254bdR953

Now I'm pretty sure that the original intention was to return a '' by default. But that means maybe other methods as well ?

Anyway, if you think this should return None by default, I'll add checks to the methods that use this property instead.


@property
def UniversalCRTSdkLastVersion(self):
Expand Down Expand Up @@ -750,16 +752,15 @@ def NetFxSdkVersion(self):
)

@property
def NetFxSdkDir(self):
def NetFxSdkDir(self) -> str | None:
"""
Microsoft .NET Framework SDK directory.

Return
------
str
str | None
path
"""
sdkdir = ''
for ver in self.NetFxSdkVersion:
loc = os.path.join(self.ri.netfx_sdk, ver)
sdkdir = self.ri.lookup(loc, 'kitsinstallationfolder')
Expand All @@ -768,7 +769,7 @@ def NetFxSdkDir(self):
return sdkdir

@property
def FrameworkDir32(self):
def FrameworkDir32(self) -> str:
"""
Microsoft .NET Framework 32bit directory.

Expand All @@ -784,7 +785,7 @@ def FrameworkDir32(self):
return self.ri.lookup(self.ri.vc, 'frameworkdir32') or guess_fw

@property
def FrameworkDir64(self):
def FrameworkDir64(self) -> str:
"""
Microsoft .NET Framework 64bit directory.

Expand Down Expand Up @@ -854,7 +855,7 @@ def _find_dot_net_versions(self, bits) -> tuple[str, ...]:
return ()

@staticmethod
def _use_last_dir_name(path, prefix=''):
def _use_last_dir_name(path: StrPath, prefix: str = '') -> str:
"""
Return name of the last dir in path or '' if no dir found.

Expand All @@ -876,7 +877,7 @@ def _use_last_dir_name(path, prefix=''):
if os.path.isdir(os.path.join(path, dir_name))
and dir_name.startswith(prefix)
)
return next(matching_dirs, None) or ''
return next(matching_dirs, '')


class _EnvironmentDict(TypedDict):
Expand Down Expand Up @@ -1190,7 +1191,7 @@ def _sdk_tools(self):
yield self.si.WindowsSDKExecutablePath

@property
def _sdk_subdir(self):
def _sdk_subdir(self) -> str:
"""
Microsoft Windows SDK version subdir.

Expand Down Expand Up @@ -1369,7 +1370,7 @@ def UCRTIncludes(self):
return [os.path.join(include, '%sucrt' % self._ucrt_subdir)]

@property
def _ucrt_subdir(self):
def _ucrt_subdir(self) -> str:
"""
Microsoft Universal C Runtime SDK version subdir.

Expand Down
Loading