bpo-30912: Don't check ffi.h for specific contents by shlomif · Pull Request #2687 · python/cpython (original) (raw)

On Fri, 14 Jul 2017 17:10:06 +0000 (UTC) Nir Soffer ***@***.***> wrote: nirs requested changes on this pull request. > @@ -2003,16 +2003,9 @@ def detect_ctypes(self, inc_dirs, lib_dirs): ffi_inc = find_file('ffi.h', [], inc_dirs) if ffi_inc is not None: ffi_h = ffi_inc[0] + '/ffi.h' - # Just check that the file is not empty - with open(ffi_h) as f: - for line in f: - line = line.strip() - if len(line) > 0: - break - else: - ffi_inc = None - print('Header file {} does not define LIBFFI_H or ' - 'ffi_wrapper_h'.format(ffi_h)) + if not os.path.exists(ffi_h): + ffi_inc = None + print('Header file {} does not exist'.format(ffi_h)) Looks good. But we need a proper commit message describing this change. The previous patch commit message is not relevant any more, and this patch commit message is not useful. People looking at python git history should not have to search github pull requests comments. I think the proper way to do this would be to fixup this change into the previous patch, and upgrade the commit message to describe the actual change.

…