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

.NET: Change NETCore.App version detection to use highest match #64922

Merged

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Aug 26, 2022

libnethost.a detection failed on my Linux system (Mageia 9, using Fedora 36
dotnet repos), because it used the first match which isn't the one matching
the rest of the SDK:

$ dotnet --list-runtimes
Microsoft.AspNetCore.App 3.1.28 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.28 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.5 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.8 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

No idea why I still have 6.0.5 installed but it should pick the highest I guess.

May fix #64765.

`libnethost.a` detection failed on my Linux system (Mageia 9, using Fedora 36
dotnet repos), because it used the first match which isn't the one matching
the rest of the SDK:
```
$ dotnet --list-runtimes
Microsoft.AspNetCore.App 3.1.28 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.28 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.5 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.8 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
```

No idea why I still have 6.0.5 installed but it should pick the highest I guess.
@akien-mga
Copy link
Member Author

No idea why I still have 6.0.5 installed but it should pick the highest I guess.

Turns out it's because I had an empty folder for 6.0.5:

$ ls /usr/share/dotnet/shared/Microsoft.NETCore.App/
3.1.28/  6.0.5/  6.0.8/

I'm using Microsoft's own Fedora packages from packages.microsoft.com so it means they are not / were not properly packaging this as RPM didn't know who owns the 6.0.5 folder and didn't remove it. I found plenty other stray empty folders throughout /usr/share/dotnet/ from several years of dnf updates.

If a Microsoft packager passes by, please make sure the relevant package owns its versioned folder with the %dir directive:

%files
%dir %{_datadir}/dotnet/shared/Microsoft.NETCore.App/%{version}

If the folder isn't listed, it won't be removed, and since dotnet's detection seems to only be folder based, it's a recipe for trouble.

Now to be fair, I just updated my microsoft-prod repo from Fedora 32 to Fedora 36, so it might have been packaging bugs in the Fedora 32 times which may have long been ironed out. Time will tell :)


Now, this PR might also fix #64765.

Could you test @derammo @mrbbbaixue? Seems like the Windows packaging had similar issues of not cleaning up its folders.

@derammo
Copy link
Contributor

derammo commented Aug 26, 2022

I will test tomorrow (Saturday) on Windows 10, using Visual Studio 2022.

@mrbbbaixue
Copy link
Contributor

Works on Windows 11, Visual Studio 2022 17.3.1.

@derammo
Copy link
Contributor

derammo commented Aug 26, 2022

Works on Windows 11, Visual Studio 2022 17.3.1.

With the 6.0.5 folders still present, just to confirm?

@mrbbbaixue
Copy link
Contributor

With the 6.0.5 folders still present, just to confirm?

Here on my computer is 6.0.7 and 6.0.8, it find 6.0.8 and builds successfully.

@derammo
Copy link
Contributor

derammo commented Aug 27, 2022

Here's my test report (blocked by the build being broken)

  • uninstalled all .NET and removed all from C:\Program Files\dotnet\shared\ (noticed later some trash in the ..\packs folders left from old versions)

  • installed just 6.0.5 SDK from standalone installer

  • verified folder C:\Program Files\dotnet\shared\Microsoft.NETCore.App\6.0.5

  • installed Visual Studio 2022 .NET workload

  • x64 Native Tools Command Prompt for VS 2022

  • (using code from before fix)

  • scons platform=windows target=debug module_mono_enabled=yes tests=yes verbose=yes modules\module_mono.windows.tools.64.lib

  • observed that include path includes /IC:\Program Files\dotnet\packs\Microsoft.NETCore.App.Host.win-x64\6.0.5\runtimes\win-x64\native

  • 6.0.5 was not uninstalled and the given path is populated

  • note C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Host.win-x64\6.0.8\runtimes\win-x64\native exists from Visual Studio Install and is populated also

  • -> problem confirmed

  • git pull

  • scons platform=windows target=debug module_mono_enabled=yes tests=yes verbose=yes modules\module_mono.windows.tools.64.lib

scons: Reading SConscript files ...
Auto-detected 16 CPU cores available for build parallelism. Using 15 cores by default. You can override it with the -j argument.

            Arch argument is not supported for MSVC manual configuration (VCINSTALLDIR configured).
            Architecture depends on the Native/Cross Compile Tools Prompt/Developer Console (or Visual Studio settings) that is being used to run SCons.
            As a consequence, the arch argument is disabled. Run scons again without arch argument (example: scons p=windows)
            and SCons will attempt to detect what MSVC compiler will be executed and inform you.

NameError: name 'SCons' is not defined:
  File "E:\godot_master\SConstruct", line 509:
    detect.configure(env)
  File "E:\godot_master\./platform/windows\detect.py", line 497:
    setup_msvc_manual(env)
  File "E:\godot_master\./platform/windows\detect.py", line 121:
    raise SCons.Errors.UserError("Arch argument should not be used when using VCINSTALLDIR")

Looks like someone broke the Visual Studio detection in manual (VCINSTALLDIR) mode?

  • scons
scons: Reading SConscript files ...
Automatically detected platform: windows
Auto-detected 16 CPU cores available for build parallelism. Using 15 cores by default. You can override it with the -j argument.

            Arch argument is not supported for MSVC manual configuration (VCINSTALLDIR configured).
            Architecture depends on the Native/Cross Compile Tools Prompt/Developer Console (or Visual Studio settings) that is being used to run SCons.
            As a consequence, the arch argument is disabled. Run scons again without arch argument (example: scons p=windows)
            and SCons will attempt to detect what MSVC compiler will be executed and inform you.

NameError: name 'SCons' is not defined:
  File "E:\godot_master\SConstruct", line 509:
    detect.configure(env)
  File "E:\godot_master\./platform/windows\detect.py", line 497:
    setup_msvc_manual(env)
  File "E:\godot_master\./platform/windows\detect.py", line 121:
    raise SCons.Errors.UserError("Arch argument should not be used when using VCINSTALLDIR")
    ```

@derammo
Copy link
Contributor

derammo commented Aug 27, 2022

ok trying again with the patch for #64921 (just FYI so nobody wastes time explaining to me why it broke :) )

@derammo
Copy link
Contributor

derammo commented Aug 27, 2022

Still failed I believe?

scons platform=windows target=debug module_mono_enabled=yes tests=yes verbose=yes | findstr dotnet

still includes

/IC:\Program Files\dotnet\packs\Microsoft.NETCore.App.Host.win-x64\6.0.5\runtimes\win-x64\native

@derammo
Copy link
Contributor

derammo commented Aug 27, 2022

E:\godot_master>dotnet --list-runtimes
Microsoft.AspNetCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@derammo
Copy link
Contributor

derammo commented Aug 27, 2022

oh boy... I just verified the patch and I didn't actually have the this PR applied at the last run, sorry for wasting your time. So let me go test again :)

confirmed fixed with this PR and 64921

/IC:\Program Files\dotnet\packs\Microsoft.NETCore.App.Host.win-x64\6.0.8\runtimes\win-x64\native

@akien-mga akien-mga merged commit b5294b5 into godotengine:master Aug 27, 2022
@akien-mga akien-mga deleted the dotnet-fix-app-host-version-detection branch August 27, 2022 16:58
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.

Build Mono module on Windows 11 throws RuntimeError: File Not found.
4 participants