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

[BUG] distutils.ccompiler.CCompiler.has_function does not quote provided includes anymore #3820

Closed
rokm opened this issue Feb 12, 2023 · 6 comments · Fixed by pypa/distutils#205
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@rokm
Copy link

rokm commented Feb 12, 2023

setuptools version

67.2.0

Python version

3.x

OS

Linux (but presumably applicable to all)

Additional environment information

No response

Description

As of 56a5b33 (setuptools 67.2.0), the distutils.ccompiler.CCompiler.has_function in the setuptools-provided distutils ceased to quote the provided includes in the generated test C file (here).

Therefore, the compilation of the generated test C file always fails.

Expected behavior

The provided includes should be quoted (as before the change) to retain compatibility with older code using this function.

How to Reproduce

  1. Run the following test code on Linux:
# program.py
import distutils.ccompiler

# Check for `clock` function in the standard library
cc = distutils.ccompiler.new_compiler()
has_clock = cc.has_function('clock', includes=['time.h'])
print("Has clock:" , has_clock)

Output

With setuptools 67.2.0:

$ python program.py 
/tmp/clocku2l593gf.c:1:10: error: #include expects "FILENAME" or <FILENAME>
    1 | #include time.h
      |          ^~~~
/tmp/clocku2l593gf.c: In function ‘main’:
/tmp/clocku2l593gf.c:3:5: warning: implicit declaration of function ‘clock’ [-Wimplicit-function-declaration]
    3 |     clock();
      |     ^~~~~
/tmp/clocku2l593gf.c:1:1: note: ‘clock’ is defined in header ‘<time.h>’; did you forget to ‘#include <time.h>’?
  +++ |+#include <time.h>
    1 | #include time.h
Has clock: False

With setuptools 67.1.0:

$ python program.py 
Has clock: True
@rokm rokm added bug Needs Triage Issues that need to be evaluated for severity and status. labels Feb 12, 2023
@lazka
Copy link
Contributor

lazka commented Feb 15, 2023

@fweimer-rh

@fweimer-rh
Copy link
Contributor

Yeah, this was clearly an error on my part. I view "" and <> as part of header specification (and they are not interchangeable), but it's too late to change this now, and I had not noticed that I was altering behavior in this way.

@fweimer-rh
Copy link
Contributor

Should I send a PR to fix this?

@abravalheri
Copy link
Contributor

abravalheri commented Feb 15, 2023

@fweimer-rh, would it be possible to add quotes if the argument does not start with " either <?

@abravalheri
Copy link
Contributor

Should I send a PR to fix this?

I believe that a PR to pypa/distutils mentioning this bug in setuptools should be the way to go.

@fweimer-rh
Copy link
Contributor

@fweimer-rh, would it be possible to add quotes if the argument does not start with " either <?

I would prefer to fix just this bug, it's an obsolete feature anyway because the headers won't work for function that take any parameters (and most functions do). Sorry for messing this up.

fweimer-rh added a commit to fweimer-rh/distutils that referenced this issue Feb 15, 2023
Arguably, this is a historic wart in the interface, and subconsciously
fixed it in commit 56a5b33 ("distutils.ccompiler:
Make has_function work with more C99 compilers").  But it's clearly
not a backwards-compatible change, so it's wrong and has to be
reverted.

Fixes pypa/setuptools#3820.
fweimer-rh added a commit to fweimer-rh/distutils that referenced this issue Feb 15, 2023
Arguably, this is a historic wart in the interface, which is why I
subconsciously fixed it in commit 56a5b33 ("distutils.ccompiler:
Make has_function work with more C99 compilers").  But it's clearly
not a backwards-compatible change, so it's wrong and has to be
reverted.

Fixes pypa/setuptools#3820.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants