Skip to content

[3.6] Change order of find_vc_pdir arguments #101495

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

Closed

Conversation

Dadaskis
Copy link

Dadaskis@HOME-PC MINGW64 /c/GodotGit/godot (dadaskis_find_vc_pdir_vcproj_fix)
$ scons platform=windows vsproj=yes
scons: Reading SConscript files ...
Auto-detected 4 CPU cores available for build parallelism. Using 4 cores by default. You can override it with the -j argument.
Configuring for Windows: target=debug, bits=default
Found MSVC version 14.3, arch amd64, bits=64
Note: Building a debug binary (which will run slowly). Use `target=release_debug` to build an optimized release binary.
AttributeError: 'str' object has no attribute 'get':
  File "C:\GodotGit\godot\SConstruct", line 769:
    methods.generate_vs_project(env, GetOption("num_jobs"))
  File "C:\GodotGit\godot\methods.py", line 847:
    batch_file = find_visual_c_batch_file(env)
  File "C:\GodotGit\godot\methods.py", line 801:
    product_dir = find_vc_pdir(env, msvc_version)
  File "C:\Users\Dadaskis\AppData\Local\Programs\Python\Python312\Lib\site-packages\SCons\Tool\MSCommon\vc.py", line 1733:
    frozen_binary, _ = _VSWhereExecutable.vswhere_freeze_env(env)
  File "C:\Users\Dadaskis\AppData\Local\Programs\Python\Python312\Lib\site-packages\SCons\Tool\MSCommon\vc.py", line 1209:
    elif not env.get('VSWHERE'):

Dadaskis@HOME-PC MINGW64 /c/GodotGit/godot (dadaskis_find_vc_pdir_vcproj_fix)
$

I wanted to get Visual Studio solution file by using scons platform=windows vsproj=yes as it was said in documentation, however, i got an error here. After investigation, i found that one of the functions got its arguments swapped, so i decided to make a little fix in methods.py that would resolve this issue.

The only problem though, i'm not sure if this is a good fix, and if it is applicable to master branch, i was mostly interested to compile 3.6 only hence why i am making this pull request.

@Dadaskis Dadaskis requested a review from a team as a code owner January 13, 2025 14:48
@AThousandShips AThousandShips added this to the 4.4 milestone Jan 13, 2025
@akien-mga akien-mga changed the title Change order of find_vc_pdir arguments [3.6] Change order of find_vc_pdir arguments Jan 13, 2025
@akien-mga akien-mga modified the milestones: 4.4, 3.6 Jan 13, 2025
@akien-mga
Copy link
Member

Thanks for the contribution!

There are a few issues which would prevent from merging this:

  • For Godot 3, PRs should be made against the 3.x branch, and can then be cherry-picked to previous stable branches. Fixing the bug only for 3.6 (and not a future 3.7) would only hide the problem temporarily.
  • This fix would break compatibility with SCons versions before 4.8.0, as it seems in 4.8.0 they swapped the order of these parameters. The better fix would be to cherry-pick Fix VS project generation with SCons 4.8.0+ #94117. This seems needed for 3.x and earlier, as well as 4.2 and earlier. I'll have a look.

@akien-mga akien-mga closed this Jan 13, 2025
@akien-mga akien-mga removed this from the 3.6 milestone Jan 13, 2025
@akien-mga
Copy link
Member

The better fix would be to cherry-pick #94117. This seems needed for 3.x and earlier, as well as 4.2 and earlier. I'll have a look.

Done! The fix should be in all relevant branches.

@Dadaskis
Copy link
Author

Thank you! And sorry, i couldn't find the forementioned pull request earlier, i'm glad somebody noticed it before

@Dadaskis Dadaskis deleted the dadaskis_find_vc_pdir_vcproj_fix branch January 13, 2025 15:31
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.

3 participants