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

gh-112984: Fix link error on free-threaded Windows build #114455

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

colesbury
Copy link
Contributor

@colesbury colesbury commented Jan 22, 2024

The test_peg_generator test tried to link the python313_d.lib library, which failed because the library is now named python313t_d.lib. The underlying problem is that the cmd.compiler attribute was not set when we call get_libraries() from distutils.

The test_peg_generator test tried to link the python313_d.lib library,
which failed because the library is now named python313t_d.lib. The
underlying problem is that the "compiler" attribute was not set when
we call get_libraries() from distutils.
@colesbury
Copy link
Contributor Author

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit ccfc91b 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • AMD64 Ubuntu NoGIL Refleaks PR
  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Ubuntu NoGIL PR

@colesbury
Copy link
Contributor Author

cc @itamaro

@colesbury
Copy link
Contributor Author

The relevant piece of distutils is this code:

https://github.com/pypa/distutils/blob/fb5c5704962cd3f40c69955437da9a88f4b28567/distutils/command/build_ext.py#L745-L758

The compiler attribute is normally set by run(), but we're decomposing the distutils command in test_peg_generator so it was not set. Because it's still set to None, distutils thinks we are not using MSVC and sticks in a manual library name that's not correct for the free-threaded build.

@colesbury
Copy link
Contributor Author

We might also want to fix the library name upstream in distutils for the free-threaded build on non-MSVC compilers.

Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

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

thanks for the quick fix, Sam!

is this test not covered by the free-threaded CI? only buildbot?

@colesbury
Copy link
Contributor Author

Yes, test_peg_generator is slow so it's only run by the build bots, not any of the GitHub actions runners.

@itamaro
Copy link
Contributor

itamaro commented Jan 23, 2024

the buildot is green on this PR (https://buildbot.python.org/all/#/builders/1295/builds/26)

@zooba
Copy link
Member

zooba commented Jan 23, 2024

LGTM

@colesbury
Copy link
Contributor Author

Thanks @zooba - would you please merge this PR?

@zooba zooba merged commit 5f19978 into python:main Jan 23, 2024
44 checks passed
@colesbury colesbury deleted the gh-112984-peggen branch January 24, 2024 17:28
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
…nGH-114455)

The test_peg_generator test tried to link the python313_d.lib library,
which failed because the library is now named python313t_d.lib. The
underlying problem is that the "compiler" attribute was not set when
we call get_libraries() from distutils.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…nGH-114455)

The test_peg_generator test tried to link the python313_d.lib library,
which failed because the library is now named python313t_d.lib. The
underlying problem is that the "compiler" attribute was not set when
we call get_libraries() from distutils.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants