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

Change Assembly Version to AssemblyVersion in win_pe #4116

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

alok1304
Copy link

Fixes #3790

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁
  • Updated documentation pages (if applicable)
  • Updated CHANGELOG.rst (if applicable)

Signed-off-by: Alok Kumar [email protected]

@alok1304 alok1304 force-pushed the fix/assembly-version-format branch from 9cc045f to 51c9005 Compare January 23, 2025 15:39
@alok1304
Copy link
Author

alok1304 commented Jan 23, 2025

@pombredanne two test cases are failing when I used AssemblyVersion for
test_win_pe_Moq_Silverlight_dll and
test_win_pe_microsoft_practices_enterpriselibrary_caching_dll in test_win_pe.py

it shows (I checked for test_win_pe_Moq_Silverlight_dll)

>       assert result == expected
E       AssertionError: assert {'AssemblyVer...y': None, ...} == {'AssemblyVer...y': None, ...}
E
E         Omitting 20 identical items, use -vv to show
E         Differing items:
E         {'extra_data': {'Assembly Version': '4.2.1507.118'}} != {'extra_data': {}}
E         Use -v to get more diff

it shows that the result dictionary contains an extra_data field with {'Assembly Version': '4.2.1507.118'} that means extra_data field contains Assembly Version.

I think no need to change this 'Assembly Version' to 'AssemblyVersion' because extra_data field already contains
Assembly Version.

I think we should not handle the correct Windows PE versions when we using AssemblyVersion in #3790.
@pombredanne Am I Right??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check that we handle correctly the Windows PE versions
1 participant