diff mbox series

[FFmpeg-devel] configure: Fix Microsoft tools detection

Message ID 20220122201321.8368-1-kasper93@gmail.com
State New
Headers show
Series [FFmpeg-devel] configure: Fix Microsoft tools detection | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/make_aarch64_jetson success Make finished
andriy/make_fate_aarch64_jetson success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Kacper Michajlow Jan. 22, 2022, 8:13 p.m. UTC
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(-)

Comments

Martin Storsjö Jan. 26, 2022, 2 p.m. UTC | #1
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
Kacper Michajlow Feb. 3, 2022, 6:44 a.m. UTC | #2
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
> elif $_cc 2>&1 | grep -q 'Microsoft.*ARM.*Assembler'; then

just for consistency...

Also I noticed that latest MSVC (19.30.30709) complains about
> cl : Command line warning D9035 : option 'nologo-' has been deprecated and will be removed in a future release

But it doesn't affect anything, even if it were to be removed. Since
banner is shown always by default, unless `-nologo`. Just a side note
:)

-Kacper
Martin Storsjö Feb. 3, 2022, 11:34 a.m. UTC | #3
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.

So, the patch is ok if you equip it with a more detailed commit message.

Relatedly, I figured that it would be even more consistent to use 
--ar=llvm-lib instead of --ar=llvm-ar, when working in an MSVC style 
configuration, but we don't identify llvm-lib as the right kind of tool. 
Would you be interested in fixing that too? :-)

>> elif $_cc 2>&1 | grep -q 'Microsoft.*ARM.*Assembler'; then
>
> just for consistency...
>
> Also I noticed that latest MSVC (19.30.30709) complains about
>> cl : Command line warning D9035 : option 'nologo-' has been deprecated and will be removed in a future release
>
> But it doesn't affect anything, even if it were to be removed. Since
> banner is shown always by default, unless `-nologo`. Just a side note
> :)

Yep - I think this has been an attempt to make sure it does get printed, 
even if someone passes e.g. "--cc='cl -nologo'". As long as it doesn't 
break, I guess it's no rush to change this.

// Martin
Hendrik Leppkes Feb. 3, 2022, 11:55 a.m. UTC | #4
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.

- Hendrik
Marvin Scholz Feb. 3, 2022, 12:08 p.m. UTC | #5
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.
>

Just tested this and it does not seem to be translated for me, at least 
in the
x86 Native Tools Command Prompt.

> - Hendrik
> _______________________________________________
> 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".
Marvin Scholz Feb. 3, 2022, 12:26 p.m. UTC | #6
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.

> - Hendrik
> _______________________________________________
> 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".
Martin Storsjö Feb. 3, 2022, 12:33 p.m. UTC | #7
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:
>>>
>>> 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.

Oh, thanks for that!

I presume that this also could break the other existing check for armasm 
too? If you run e.g. "vsdevcmd -host_arch=x64 -arch=arm64" and then 
"armasm64", what does that print?

// Martin
Marvin Scholz Feb. 3, 2022, 12:43 p.m. UTC | #8
On 3 Feb 2022, at 13:33, Martin Storsjö wrote:

> 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:
>>>>
>>>> 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.
>
> Oh, thanks for that!
>
> I presume that this also could break the other existing check for 
> armasm too? If you run e.g. "vsdevcmd -host_arch=x64 -arch=arm64" and 
> then "armasm64", what does that print?
>

Interestingly that one does not seem localized:

C:\Program Files\Microsoft Visual Studio\2022\Community>vsdevcmd 
-host_arch=x64 -arch=arm64
**********************************************************************
** Visual Studio 2022 Developer Command Prompt v17.0.5
** Copyright (c) 2021 Microsoft Corporation
**********************************************************************

C:\Program Files\Microsoft Visual Studio\2022\Community>armasm64
Microsoft (R) ARM Macro Assembler Version 14.30.30709.0 for 64 bits
Copyright (C) Microsoft Corporation.  All rights reserved.

error A2033: missing input source file

  Usage:      armasm [<options>] sourcefile objectfile
              armasm [<options>] -o objectfile sourcefile
              armasm -h              for help

> // 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".
Martin Storsjö Feb. 3, 2022, 12:47 p.m. UTC | #9
On Thu, 3 Feb 2022, Marvin Scholz wrote:

> On 3 Feb 2022, at 13:33, Martin Storsjö wrote:
>
>> 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:
>>>>>
>>>>> 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.
>>
>> Oh, thanks for that!
>>
>> I presume that this also could break the other existing check for 
>> armasm too? If you run e.g. "vsdevcmd -host_arch=x64 -arch=arm64" and 
>> then "armasm64", what does that print?
>>
>
> Interestingly that one does not seem localized:
>
> C:\Program Files\Microsoft Visual Studio\2022\Community>armasm64
> Microsoft (R) ARM Macro Assembler Version 14.30.30709.0 for 64 bits
> Copyright (C) Microsoft Corporation.  All rights reserved.

That's interesting!

I tried to have a look at what e.g. meson does for detection. It doesn't 
look that much at the output, but first detects clang-cl specifically, 
then plain clang, then moves on to MSVC itself (matching only 
'Microsoft'). Meson does, however, check a longer regex for identifying 
and disambiguating the 'rc' tool from windres. Is 'rc' localized?

// Martin
Marvin Scholz Feb. 3, 2022, 4:02 p.m. UTC | #10
On 3 Feb 2022, at 13:47, Martin Storsjö wrote:

> On Thu, 3 Feb 2022, Marvin Scholz wrote:
>
>> On 3 Feb 2022, at 13:33, Martin Storsjö wrote:
>>
>>> 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:
>>>>>>
>>>>>> 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.
>>>
>>> Oh, thanks for that!
>>>
>>> I presume that this also could break the other existing check for 
>>> armasm too? If you run e.g. "vsdevcmd -host_arch=x64 -arch=arm64" 
>>> and then "armasm64", what does that print?
>>>
>>
>> Interestingly that one does not seem localized:
>>
>> C:\Program Files\Microsoft Visual Studio\2022\Community>armasm64
>> Microsoft (R) ARM Macro Assembler Version 14.30.30709.0 for 64 bits
>> Copyright (C) Microsoft Corporation.  All rights reserved.
>
> That's interesting!
>
> I tried to have a look at what e.g. meson does for detection. It 
> doesn't look that much at the output, but first detects clang-cl 
> specifically, then plain clang, then moves on to MSVC itself (matching 
> only 'Microsoft'). Meson does, however, check a longer regex for 
> identifying and disambiguating the 'rc' tool from windres. Is 'rc' 
> localized?
>

That one is not localized either for me. At least for the /? option.

> // 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".
Martin Storsjö Feb. 7, 2022, 9 p.m. UTC | #11
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
diff mbox series

Patch

diff --git a/configure b/configure
index 94f513288a..f27fd067eb 100755
--- a/configure
+++ b/configure
@@ -4820,9 +4820,9 @@  probe_cc(){
         _flags_filter=msvc_flags
         _ld_lib='lib%.a'
         _ld_path='-libpath:'
-    elif $_cc -nologo- 2>&1 | grep -q Microsoft || { $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; }; then
+    elif $_cc -nologo- 2>&1 | grep -q 'Microsoft.*Optimizing.*Compiler' || { $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; }; then
         _type=msvc
-        if $_cc -nologo- 2>&1 | grep -q Microsoft; then
+        if $_cc -nologo- 2>&1 | grep -q 'Microsoft.*Optimizing.*Compiler'; then
             _ident=$($_cc 2>&1 | head -n1 | tr -d '\r')
         else
             _ident=$($_cc --version 2>/dev/null | head -n1 | tr -d '\r')
@@ -4927,7 +4927,7 @@  if [ -z "$CC_DEPFLAGS" ] && [ "$dep_cc" != "$cc" ]; then
     DEPCCFLAGS=$_flags
 fi
 
-if $ar 2>&1 | grep -q Microsoft; then
+if $ar 2>&1 | grep -q 'Microsoft.*Library.*Manager'; then
     arflags="-nologo"
     ar_o='-out:$@'
 elif $ar 2>&1 | grep -q "\[D\] "; then