Skip to content

Commit

Permalink
Eliminate repetition by iterating over the problem space.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jaraco
jaraco committed Jul 31, 2022
1 parent 48213f8 commit 25dcca2
Showing 1 changed file with 21 additions and 27 deletions.
48 changes: 21 additions & 27 deletions distutils/unixccompiler.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,30 +361,24 @@ def _library_root(dir):
return os.path.join(sysroot, dir[1:])

def find_library_file(self, dirs, lib, debug=0):
shared_f = self.library_filename(lib, lib_type='shared')
dylib_f = self.library_filename(lib, lib_type='dylib')
xcode_stub_f = self.library_filename(lib, lib_type='xcode_stub')
static_f = self.library_filename(lib, lib_type='static')

for dir in dirs:
root = self._library_root(dir)
shared = os.path.join(root, shared_f)
dylib = os.path.join(root, dylib_f)
static = os.path.join(root, static_f)
xcode_stub = os.path.join(root, xcode_stub_f)

# Second-guess the linker with not much hard
# data to go on: GCC seems to prefer the shared library, so
# assume that *all* Unix C compilers do,
# ignoring even GCC's "-static" option.
if os.path.exists(dylib):
return dylib
elif os.path.exists(xcode_stub):
return xcode_stub
elif os.path.exists(shared):
return shared
elif os.path.exists(static):
return static

# Oops, didn't find it in *any* of 'dirs'
return None
"""

This comment has been minimized.

Sign in to view

Copy link
@TimotheusBachinger

TimotheusBachinger Aug 1, 2022

It seems this commit breaks Pillow builds on linux based systems, see python-pillow/Pillow#6471
Second-guess the linker with not much hard
data to go on: GCC seems to prefer the shared library, so
assume that *all* Unix C compilers do,
ignoring even GCC's "-static" option.
"""
lib_names = (
self.library_filename(lib, lib_type=type)
for type in 'dylib xcode_stub shared static'.split()
)

searched = (
os.path.join(root, lib_name)
for root in map(self._library_root, dirs)
for lib_name in lib_names
)

found = filter(os.path.exists, searched)

# Return None if it could not be found in any dir.
return next(found, None)

3 comments on commit 25dcca2

@fanselm
Copy link

@fanselm fanselm commented on 25dcca2 Aug 2, 2022

Choose a reason for hiding this comment

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

This breaks builds on Linux because you iterate over two generators in searched: after the first iteration over directories for the first lib_name, then generator will be empty as it's not cyclic. Fix it by converting generators to lists like so:

lib_names = list(
    self.library_filename(lib, lib_type=type)
    for type in 'dylib xcode_stub shared static'.split()
)

searched = list(
    os.path.join(root, lib_name)
    for root in map(self._library_root, dirs)
    for lib_name in lib_names
)

@jaraco
Copy link
Member Author

@jaraco jaraco commented on 25dcca2 Aug 3, 2022

Choose a reason for hiding this comment

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

Thanks for the report. I'd say too that distutils should grow a test for this function. I did test the iteration, but using 'abc':

[(x, y) for x in iter('abc') for y in iter('xyz')]
[('a', 'x'),
 ('a', 'y'),
 ('a', 'z'),
 ('b', 'x'),
 ('b', 'y'),
 ('b', 'z'),
 ('c', 'x'),
 ('c', 'y'),
 ('c', 'z')]

But now I realize that the reason that works is because iter('xyz') gets re-evaluated during each loop of 'abc':

>>> xyz = iter('xyz')
>>> [(x, y) for x in iter('abc') for y in xyz]
[('a', 'x'), ('a', 'y'), ('a', 'z')]

Yes. lib_names needs to be materialized somehow. I don't think searched does.

@jaraco
Copy link
Member Author

@jaraco jaraco commented on 25dcca2 Aug 3, 2022

Choose a reason for hiding this comment

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

Filed pypa/distutils#164 to track.

Please sign in to comment.