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

Add VS2022 17.6 #64

Merged
merged 6 commits into from
Jun 23, 2023
Merged

Add VS2022 17.6 #64

merged 6 commits into from
Jun 23, 2023

Conversation

h-vetinari
Copy link
Member

@h-vetinari h-vetinari commented Jun 16, 2023

Recently released...

I'm pretty sure we need this because the windows images have been updated, and we're getting lots of

C:\Program Files\Microsoft Visual Studio\2022\Enterprise>CALL "VC\Auxiliary\Build\vcvars64.bat" -vcvars_ver=14.34 10.0.22621.0 
**********************************************************************
** Visual Studio 2022 Developer Command Prompt v17.6.2
** Copyright (c) 2022 Microsoft Corporation
**********************************************************************
[ERROR:vcvars.bat] Toolset directory for version '14.34' was not found.
[ERROR:VsDevCmd.bat] *** VsDevCmd.bat encountered errors. Environment may be incomplete and/or incorrect. ***
[ERROR:VsDevCmd.bat] In an uninitialized command prompt, please 'set VSCMD_DEBUG=[value]' and then re-run
[ERROR:VsDevCmd.bat] vsdevcmd.bat [args] for additional details.
[ERROR:VsDevCmd.bat] Where [value] is:
[ERROR:VsDevCmd.bat]    1 : basic debug logging
[ERROR:VsDevCmd.bat]    2 : detailed debug logging
[ERROR:VsDevCmd.bat]    3 : trace level logging. Redirection of output to a file when using this level is recommended.
[ERROR:VsDevCmd.bat] Example: set VSCMD_DEBUG=3
[ERROR:VsDevCmd.bat]          vsdevcmd.bat > vsdevcmd.trace.txt 2>&1

AFAICT this has broken VS2022 in a few places, but since that's not our default the blast radius is not so big.

Xref discussion in #57, where the conclusion was that we need to match the cl_version with what's in the image.

Closes #63

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • It looks like the 'vs2015_runtime' output doesn't have any tests.
  • It looks like the 'vc' output doesn't have any tests.

@h-vetinari
Copy link
Member Author

@conda-forge-admin, please rerender

@h-vetinari
Copy link
Member Author

Aside from smithy mis-rendering this recipe (I fixed it manually), it looks like MSFT changed the extension for some of the libraries from dll to dll_amd64 in 17.6:

 Directory of %PREFIX%\Library\bin

[...]
05/10/2023  07:01 AM           327,576 concrt140.dll_amd64
05/10/2023  07:01 AM            31,640 msvcp140_codecvt_ids.dll_amd64
05/10/2023  07:01 AM           578,384 msvcp140.dll_amd64
05/10/2023  07:01 AM            35,704 msvcp140_1.dll_amd64
05/10/2023  07:01 AM           267,160 msvcp140_2.dll_amd64
05/10/2023  07:01 AM            50,072 msvcp140_atomic_wait.dll_amd64
05/06/2022  03:22 PM         1,123,808 ucrtbase.dll
05/10/2023  07:01 AM           414,104 vcamp140.dll_amd64
05/10/2023  07:01 AM           346,008 vccorlib140.dll_amd64
05/10/2023  07:01 AM           191,864 vcomp140.dll_amd64
05/10/2023  07:01 AM           109,440 vcruntime140.dll_amd64
05/10/2023  07:01 AM            49,560 vcruntime140_1.dll_amd64

Would you like to just adapt the tests to reflect that, or rename them back to dll @isuruf? (I think the former is more sustainable)

@isuruf
Copy link
Member

isuruf commented Jun 18, 2023

Why do you think changing the DLL name to .dll_amd64 will work?

@h-vetinari
Copy link
Member Author

Why do you think changing the DLL name to .dll_amd64 will work?

Work in what way? Pass the tests here? That should be tautological if we adapt them to reflect the renaming.

Work more broadly? I guess it's up to Microsoft to ensure that, as they renamed the extensions.

@isuruf
Copy link
Member

isuruf commented Jun 19, 2023

Work more broadly?

Yes

I guess it's up to Microsoft to ensure that as they renamed the extensions.

No, they renamed the intermediate artifact in the executable. That does not mean that the file ends up as .dll_amd64 when it is installed by the installer vc_redist.x64.exe. If you look at C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Redist\MSVC\14.36.32532\x64 in a CI runner, you'll see that the DLL extension name is not changed at all.

Changing the name of a DLL is equivalent to changing the name of the shared object .so and it will simply not work at all.

@h-vetinari
Copy link
Member Author

No, they renamed the intermediate artifact in the executable. That does not mean that the file ends up as .dll_amd64 when it is installed by the installer vc_redist.x64.exe. If you look at C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Redist\MSVC\14.36.32532\x64 in a CI runner, you'll see that the DLL extension name is not changed at all.

OK, I had not gotten that deep or far, but that makes sense! So we don't have a choice about renaming those libs in any case.

@h-vetinari
Copy link
Member Author

I added a minimal fix to ensure the intermediate DLLs get properly renamed and tests are passing again. PTAL :)

recipe/vc_repack.py Outdated Show resolved Hide resolved
@isuruf
Copy link
Member

isuruf commented Jun 21, 2023

@conda-forge-admin, rerender

@h-vetinari
Copy link
Member Author

I don't know why, but the rerendering is broken here - it does not zip the values correctly.

Also, shouldn't we be able to drop some of the older builds? IIUC, it's only going to work with the latest cl_version (per series) that's present in the image?

@isuruf
Copy link
Member

isuruf commented Jun 23, 2023

Thanks. Yes, we can drop some of the older builds.

@isuruf isuruf merged commit 3816e17 into conda-forge:main Jun 23, 2023
@h-vetinari h-vetinari deleted the bump branch June 23, 2023 22:26
@h-vetinari
Copy link
Member Author

Thanks. Yes, we can drop some of the older builds.

#65

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.

Problems with VS2022 activation
2 participants