Skip to content

Conversation

@bruchar1
Copy link
Member

@bruchar1 bruchar1 commented Nov 5, 2025

Add test for #15206

@bonzini
Copy link
Collaborator

bonzini commented Nov 19, 2025

This should fix the issue, if either the creator or someone else wants to turn this into a non-draft PR.

@bruchar1 bruchar1 force-pushed the fix-windows-rc-compiler-subdir branch 3 times, most recently from e0da676 to 205a0a7 Compare November 19, 2025 13:05
@bonzini
Copy link
Collaborator

bonzini commented Nov 19, 2025

I did put together something that should work, but I don't have time to finish it. I think it's best to revert 632115e for 1.10 and try again in 1.11

@bruchar1 bruchar1 force-pushed the fix-windows-rc-compiler-subdir branch 2 times, most recently from 34b7582 to 83ccd0f Compare November 19, 2025 14:22
@bruchar1
Copy link
Member Author

I'm confused. The initial problem seems to be that current directory is not added to include paths when compiling resources. I tried to add it, and now the "15 resource scripts with duplicate filenames" test is failing with a gcc internal compiler error. But if I don't add the current dir to include path for "windres" resource compiler, my test fail because it doesn't find an included file in the .rc file.

The gcc bug has nothing to do with the preprocessor command. It is caused by the added include dir. I think that the added preprocessor command just showed a bug that was already present.

@bruchar1 bruchar1 force-pushed the fix-windows-rc-compiler-subdir branch from 83ccd0f to b7cc53b Compare November 19, 2025 14:46
@bruchar1 bruchar1 force-pushed the fix-windows-rc-compiler-subdir branch from b7cc53b to d0e59a7 Compare November 19, 2025 15:07
@bruchar1 bruchar1 marked this pull request as ready for review November 19, 2025 17:04
@bruchar1 bruchar1 requested a review from jpakkane as a code owner November 19, 2025 17:04
@bonzini
Copy link
Collaborator

bonzini commented Nov 19, 2025

Removing from milestone (#15206 is still there) because I don't think this is the right fix. The problem stems from adding project and global arguments for C to the preprocessor invocation, the include path is a red herring.

@bonzini bonzini removed this from the 1.10 milestone Nov 19, 2025
@bruchar1
Copy link
Member Author

I don't agree. The first commit shows a bug that is present even if 632115e is reverted. The second commit fixes that.

The remaining question is whether it fixes #15206 or not. @lb90 should confirm it.

@bonzini
Copy link
Collaborator

bonzini commented Nov 19, 2025

This might be considered a bug for sure, but it's not the root cause of #15206. And given the windres issue with spaces in file names, it's dangerous to add to the include directories unconditionally.

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.

2 participants