-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-91048: Refactor _testexternalinspection and add Windows support #132852
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
Conversation
66b7014
to
b04fbc5
Compare
46bf7d7
to
e13c5a6
Compare
5ab86ad
to
4ffe070
Compare
Man, getting windows to work was NOT easy |
47efe9c
to
fe42a7f
Compare
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>
This change broke FreeBSD buildbot:
|
I wrote #132945 to fix the build on FreeBSD. |
Seems like this change broke JIT CI on |
How? This should not affect the JIT in any way |
All look like that |
Yeah I'm confused as well. See what Steve said above about the tests failing though. I wonder if the JIT build lays out stuff differently in memory? No clue. |
No, that looks like whatever the JIT build is doing is breaking this feature, which is different. This PR makes that test run on windows and is failing, ergo is not that this broke the JIT but the other way around. |
I am looking into it |
Yeah I believe the JIT broke the test internal inspection, but the JIT CI was passing before this, so if I'm being pedantic, the PR did add tests that broke the JIT CI :). Pedantics aside, @chris-eibl is looking into it, and I'm looking too. |
Hold on I am looking into this I think I know what's going on |
But JIT must do something different on Windows, because Linux is green? |
Windows is also green in the normal CI. My bet is that it has to be with release/vs not release and not related to the JIT |
I think you're right. On the commit, Azure pipelines (which uses release builds for Windows) fails, but not the normal debug builds. |
Can confirm on a local Windows release 64bit build, that the tests fail:
|
I know what the problem is, I am working on a fix |
As a takeaway: it would be good to get at least one release CI run on all PRs :) |
Yeah,
dumpbin shows no |
Signed-off-by: Pablo Galindo pablogsal@gmail.com