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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

win32|cmake: Fix FileVersion and ProductVersion in DLL version info #785

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

hartwork
Copy link
Member

@hartwork hartwork commented Nov 3, 2023

Reported by @sgarske at #555 (comment)
Follow-up to pull request #570

Before: {'FileVersion': 'VER_FILEVERSION', 'ProductVersion': 'VER_FILEVERSION'}
After: {'FileVersion': '2.5.0.0', 'ProductVersion': '2.5.0.0'}

Thanks to @spookyahell for helpful gist exe2version_info.py! 馃檹

@sgarske
Copy link

sgarske commented Nov 3, 2023

Thanks for looking at a fix

Unfortunately, I'm not seeing the macro work as intended with the MSVC compiler (I believe locally using v14.34 or 1934 but not sure if that'll matter). It looks to not handle tokenizing the string into the macro arguments correctly like GCC/clang might allow and you just get the version appended with some .'s
image

We found we can get the macro to work with the compiler if we specify the /Zc:preprocessor argument MSDN. However, that does not seem to be available for the .rc/resource compiler.

As an alternative solution, we found generating the .rc file with the version string via something like an .rc.in does seem to work. Something like
image

If that's hard to parse, I can try to fork and push a branch. My cmake experience is limited so there may be cleaner ways to do something like that

@hartwork
Copy link
Member Author

hartwork commented Nov 3, 2023

@sgarske thanks for testing and the detailed feedback! 馃憤 I get the idea, did see something similar elsewhere. Let me apply a variation of it, confirm that it's working locally, and update the pull request. With luck give me 15 minutes, will report back here.

@hartwork
Copy link
Member Author

hartwork commented Nov 3, 2023

@sgarske done, pushed. Could you try again?

Before: {'FileVersion': 'VER_FILEVERSION', 'ProductVersion': 'VER_FILEVERSION'}
After: {'FileVersion': '2.5.0.0', 'ProductVersion': '2.5.0.0'}

Thanks to @spookyahell for helpful gist "exe2version_info.py":
https://gist.github.com/spookyahell/b317bdf0712aac5fd37dd79f70bfbe69
@hartwork
Copy link
Member Author

hartwork commented Nov 3, 2023

@sgarske updated once more, previous version gave "2,5,0,0", now at "2.5.0.0" with dots again.

@sgarske
Copy link

sgarske commented Nov 3, 2023

The latest version seems to work for me 馃憤 . Thanks for your help with this and quick turnaround.

@hartwork
Copy link
Member Author

hartwork commented Nov 3, 2023

@sgarske that is great news, thanks for testing once more! 馃憤 I'll merge when/if the CI turns all green. Thanks for contributing to Expat! 馃檹

@hartwork hartwork mentioned this pull request Nov 3, 2023
35 tasks
@hartwork hartwork merged commit d285faf into master Nov 3, 2023
62 checks passed
@hartwork hartwork deleted the fix-dll-version-info branch November 3, 2023 23:23
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.

None yet

3 participants