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

Fix parsing preview MSBuild versions #121

Closed

Conversation

GerardSmit
Copy link

Running scons while having a preview version of MSBuild installed (with Visual Studio Preview) will result in the following error:

$ scons
scons: Reading SConscript files ...
Automatically detected platform: windows
Auto-detected 24 CPU cores available for build parallelism. Using 23 cores by default. You can override it with the `-j` or `num_jobs` arguments.
Using SCons-detected MSVC version 14.3, arch x86_64
Building for platform "windows", architecture "x86_64", target "editor".
ValueError: invalid literal for int() with base 10: '0 Preview 2':
  File "C:\Sources\redot-engine\SConstruct", line 642:
    cc_version = methods.get_compiler_version(env)
  File "C:\Sources\redot-engine\methods.py", line 831:
    ret["patch"] = int(sem_ver[2])

The Visual Studio Preview that I have installed is: 17.12.0 Preview 2.1

After checking, in methods.py the catalog version is being split by the '.' character, which results in the following array:

["17", "12", "0 Preview 2", "1"]
 ^^^   ^^^^  ^^^^^^^^^^^^^  ^^^
  |     |         |          |
  |     |         |          +--- Unused/invalid
  |     |         +--- Patch
  |     +--- Minor
  +--- Major

Then the patch version is being converted to an integer, which fails because it contains non-numeric characters.

This PR checks if the version has a space in it and trims everything after the space; causing the conversion to succeed.

Screenshot of Visual Studio Installer

image

Copy link
Contributor

@Norrox Norrox left a comment

Choose a reason for hiding this comment

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

nice

@GerardSmit
Copy link
Author

GerardSmit commented Oct 2, 2024

I just found out that in upstream this is issue godotengine/godot#97619 and was fixed with PR godotengine/godot#97639.

After rebasing with upstream this issue is also fixed, so maybe this PR is not needed.

@filipworksdev
Copy link

The fix from upstream patch is simpler but I like the comment explanation for the change in this pr. I guess we don't really need this one, right? Esentially what they do is they use the patch "1 Preview 2".split()[0] which is 1. Simple solution but it doesn't explain why.

@GerardSmit
Copy link
Author

The upstream branch has been merged to this repository, so this issue no longer persists.

@GerardSmit GerardSmit closed this Oct 4, 2024
@GerardSmit GerardSmit deleted the fix/msbuild-preview-version branch October 4, 2024 12:11
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.

5 participants