Re: [FFmpeg-devel] [PATCH] configure: Fix Microsoft tools detection

["Martin Storsjö" <martin@martin.st>]

On Thu, 3 Feb 2022, Marvin Scholz wrote:

>
>
> On 3 Feb 2022, at 12:55, Hendrik Leppkes wrote:
>
>> On Thu, Feb 3, 2022 at 12:34 PM Martin Storsjö <martin@martin.st> 
>> wrote:
>>>
>>> On Thu, 3 Feb 2022, Kacper Michajlow wrote:
>>>
>>>> On Wed, 26 Jan 2022 at 15:00, Martin Storsjö <martin@martin.st> 
>>>> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On Sat, 22 Jan 2022, Kacper Michajłow wrote:
>>>>>
>>>>>> LLVM tools print installation path upon execution. If one uses 
>>>>>> LLVM
>>>>>> tools bundled with Microsoft Visual Studio installation, they 
>>>>>> would be
>>>>>> incorrectly detected as Microsoft's ones.
>>>>>>
>>>>>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
>>>>>> ---
>>>>>> configure | 6 +++---
>>>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> While the patch description seems to make sense, I wanted to try it 
>>>>> out to
>>>>> see the practical effect for myself, and I fail to observe any 
>>>>> difference.
>>>>>
>>>>> Can you provide your exact configure command line you use, where it 
>>>>> makes
>>>>> a difference? I tried with "--cc=clang-cl --ld=lld-link 
>>>>> --toolchain=msvc"
>>>>> and that works just as fine before this patch.
>>>>>
>>>>> In particular, the commands that you adjust run "$_cc -nologo-" and 
>>>>> grep
>>>>> for "Microsoft" in the output of that. When I run that with 
>>>>> clang-cl, it
>>>>> doesn't print a string containing "Microsoft".
>>>>>
>>>>> // Martin
>>>>
>>>> Hi,
>>>>
>>>> Yes you are right. In case of CC it doesn't change anything. 
>>>> clang-cl
>>>> prints installation dir only with `-v`. The main thing this patch
>>>> fixes is `--ar=llvm-ar` where it is mistaken for lib.exe and used 
>>>> with
>>>> wrong parameters. While fixing this I figured to make CC check also
>>>> more strict, because at some point it could be a problem. Sync all 
>>>> of
>>>> them to have same style as one that was already there
>>>
>>> Oh, ok, with the reference to llvm-ar, I see what it fixes. Thanks! 
>>> The
>>> reference to llvm-ar absolutely needs to be in the patch description 
>>> then.
>>>
>>> I remember that there has been some variance throughout the versions 
>>> for
>>> exactly what MSVC prints as the identification thoughout the 
>>> versions, but
>>> I think 'Microsoft.*Optimizing.*Compiler' should be safe.
>>>
>>
>> I was wondering if non-english locale would translate that string, but
>> I can't easily test that, I don't think.
>>
>
> Sorry, need to correct myself. It is indeed localized I was just lacking
> the language pack…
>
> For example in german it is:
>
> Microsoft (R) C/C++-Optimierungscompiler Version 19.30.30709 für x64
> Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.

So, should we scale back on this patch to only extend the regex for 
lib.exe (to 'Microsoft.*Library.*Manager') but leave the one for cl.exe as 
it is? Marvin verified that lib.exe doesn't seem to print a localized 
message, only cl.exe seems to do that. Otherwise there's a risk we'd break 
the currently working detection of cl.exe for users of localized MSVC.

// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".