diff mbox

[FFmpeg-devel] configure: fix clang-cl detection

Message ID 20180201105224.11216-1-bilyak.alexander@gmail.com
State New
Headers show

Commit Message

Alexander Bilyak Feb. 1, 2018, 10:52 a.m. UTC 
When using clang-cl it expects parameters passed in MSVC-style, so appropriate toolchain should be selected.
As soon as both clang and clang-cl report themselfs as "clang" with -v option the only chance to detect
clang-cl is passing -? option to both which is valid for clang-cl.exe and not for clang.exe.
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dale Curtis April 13, 2018, 9:51 p.m. UTC | #1 
lgtm, I was able to build ffmpeg for windows on Linux with this patch by
following my notes here:

https://bugs.chromium.org/p/chromium/issues/detail?id=783021#c6

- dale

On Thu, Feb 1, 2018 at 2:52 AM Alexander Bilyak <bilyak.alexander@gmail.com>
wrote:

> When using clang-cl it expects parameters passed in MSVC-style, so
> appropriate toolchain should be selected.
> As soon as both clang and clang-cl report themselfs as "clang" with -v
> option the only chance to detect
> clang-cl is passing -? option to both which is valid for clang-cl.exe and
> not for clang.exe.
> ---
>  configure | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index fcfa7aa442..f8c55876e5 100755
> --- a/configure
> +++ b/configure
> @@ -4216,7 +4216,7 @@ probe_cc(){
>          _depflags='-MMD'
>          _cflags_speed='-O3'
>          _cflags_size='-Os'
> -    elif $_cc -v 2>&1 | grep -q clang; then
> +    elif $_cc -v 2>&1 | grep -q clang && ! $_cc -? > /dev/null 2>&1; then
>          _type=clang
>          _ident=$($_cc --version 2>/dev/null | head -n1)
>          _depflags='-MMD -MF $(@:.o=.d) -MT $@'
> @@ -4287,7 +4287,7 @@ probe_cc(){
>          _flags_filter=msvc_flags
>          _ld_lib='lib%.a'
>          _ld_path='-libpath:'
> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q
> clang && $_cc -? > /dev/null 2>&1; then
>          _type=msvc
>          _ident=$($_cc 2>&1 | head -n1)
>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
> --
> 2.15.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer April 15, 2018, 3:37 p.m. UTC | #2 
On Fri, Apr 13, 2018 at 09:51:10PM +0000, Dale Curtis wrote:
> lgtm, I was able to build ffmpeg for windows on Linux with this patch by
> following my notes here:
> 
> https://bugs.chromium.org/p/chromium/issues/detail?id=783021#c6

applied

thanks

[...]
Wang Bin April 18, 2018, 8:05 a.m. UTC | #3 
>
>
> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q
> clang && $_cc -? > /dev/null 2>&1; then
>          _type=msvc
>          _ident=$($_cc 2>&1 | head -n1)
>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
>
>
This breaks msvc build because msys's link.exe is tested instead of mslink
script
Timo Rothenpieler April 18, 2018, 8:27 a.m. UTC | #4 
On 18.04.2018 10:05, Wang Bin wrote:
>>
>>
>> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
>> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q
>> clang && $_cc -? > /dev/null 2>&1; then
>>          _type=msvc
>>          _ident=$($_cc 2>&1 | head -n1)
>>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
>> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
>> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
>>
>>
> This breaks msvc build because msys's link.exe is tested instead of mslink
> script

Sounds more like an issue with your build environment to me? Make sure
the msvc build tools take precedence in your PATH.
Wang Bin April 18, 2018, 8:49 a.m. UTC | #5 
2018-04-18 16:27 GMT+08:00 Timo Rothenpieler <timo@rothenpieler.org>:

> On 18.04.2018 10:05, Wang Bin wrote:
> >>
> >>
> >> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
> >> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep
> -q
> >> clang && $_cc -? > /dev/null 2>&1; then
> >>          _type=msvc
> >>          _ident=$($_cc 2>&1 | head -n1)
> >>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
> >> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
> >> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
> >>
> >>
> > This breaks msvc build because msys's link.exe is tested instead of
> mslink
> > script
>
> Sounds more like an issue with your build environment to me? Make sure
> the msvc build tools take precedence in your PATH.
>
>
I build on windows. My environment is correct. $PATH always starts with
msys2 dirs. This is why mslink exists .
Hendrik Leppkes April 18, 2018, 9:08 a.m. UTC | #6 
On Thu, Feb 1, 2018 at 11:52 AM, Alexander Bilyak
<bilyak.alexander@gmail.com> wrote:
> When using clang-cl it expects parameters passed in MSVC-style, so appropriate toolchain should be selected.
> As soon as both clang and clang-cl report themselfs as "clang" with -v option the only chance to detect
> clang-cl is passing -? option to both which is valid for clang-cl.exe and not for clang.exe.
> ---
>  configure | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure b/configure
> index fcfa7aa442..f8c55876e5 100755
> --- a/configure
> +++ b/configure
> @@ -4216,7 +4216,7 @@ probe_cc(){
>          _depflags='-MMD'
>          _cflags_speed='-O3'
>          _cflags_size='-Os'
> -    elif $_cc -v 2>&1 | grep -q clang; then
> +    elif $_cc -v 2>&1 | grep -q clang && ! $_cc -? > /dev/null 2>&1; then
>          _type=clang
>          _ident=$($_cc --version 2>/dev/null | head -n1)
>          _depflags='-MMD -MF $(@:.o=.d) -MT $@'
> @@ -4287,7 +4287,7 @@ probe_cc(){
>          _flags_filter=msvc_flags
>          _ld_lib='lib%.a'
>          _ld_path='-libpath:'
> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; then
>          _type=msvc
>          _ident=$($_cc 2>&1 | head -n1)
>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 | awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'


This patch entirely broke building with MSVC on Windows. The reason
for that is that "cl.exe -?" outputs paginated help output that
requires user input to continue.
I'm going to revert this soon, unless a fix is submitted before that.

- Hendrik
Hendrik Leppkes April 19, 2018, 9:13 a.m. UTC | #7 
On Wed, Apr 18, 2018 at 11:08 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> On Thu, Feb 1, 2018 at 11:52 AM, Alexander Bilyak
> <bilyak.alexander@gmail.com> wrote:
>> When using clang-cl it expects parameters passed in MSVC-style, so appropriate toolchain should be selected.
>> As soon as both clang and clang-cl report themselfs as "clang" with -v option the only chance to detect
>> clang-cl is passing -? option to both which is valid for clang-cl.exe and not for clang.exe.
>> ---
>>  configure | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure b/configure
>> index fcfa7aa442..f8c55876e5 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4216,7 +4216,7 @@ probe_cc(){
>>          _depflags='-MMD'
>>          _cflags_speed='-O3'
>>          _cflags_size='-Os'
>> -    elif $_cc -v 2>&1 | grep -q clang; then
>> +    elif $_cc -v 2>&1 | grep -q clang && ! $_cc -? > /dev/null 2>&1; then
>>          _type=clang
>>          _ident=$($_cc --version 2>/dev/null | head -n1)
>>          _depflags='-MMD -MF $(@:.o=.d) -MT $@'
>> @@ -4287,7 +4287,7 @@ probe_cc(){
>>          _flags_filter=msvc_flags
>>          _ld_lib='lib%.a'
>>          _ld_path='-libpath:'
>> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
>> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; then
>>          _type=msvc
>>          _ident=$($_cc 2>&1 | head -n1)
>>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 | awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
>
>
> This patch entirely broke building with MSVC on Windows. The reason
> for that is that "cl.exe -?" outputs paginated help output that
> requires user input to continue.
> I'm going to revert this soon, unless a fix is submitted before that.
>

For the record, I pushed a patch to fix this instead, after I figured
out why this was happening.

- Hendrik
Derek Buitenhuis April 19, 2018, 2:21 p.m. UTC | #8 
On 4/18/2018 9:27 AM, Timo Rothenpieler wrote:
> On 18.04.2018 10:05, Wang Bin wrote:
>>>
>>>
>>> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
>>> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q
>>> clang && $_cc -? > /dev/null 2>&1; then
>>>          _type=msvc
>>>          _ident=$($_cc 2>&1 | head -n1)
>>>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
>>> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
>>> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
>>>
>>>
>> This breaks msvc build because msys's link.exe is tested instead of mslink
>> script
> 
> Sounds more like an issue with your build environment to me? Make sure
> the msvc build tools take precedence in your PATH.

I believe Wang is correct, it should be checking mslink, which is our own script
specifically to wrap link.exe.

See compat/windows/mslink and its history.

- Derek
Hendrik Leppkes April 19, 2018, 2:28 p.m. UTC | #9 
On Thu, Apr 19, 2018 at 4:21 PM, Derek Buitenhuis
<derek.buitenhuis@gmail.com> wrote:
> On 4/18/2018 9:27 AM, Timo Rothenpieler wrote:
>> On 18.04.2018 10:05, Wang Bin wrote:
>>>>
>>>>
>>>> -    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
>>>> +    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q
>>>> clang && $_cc -? > /dev/null 2>&1; then
>>>>          _type=msvc
>>>>          _ident=$($_cc 2>&1 | head -n1)
>>>>          _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 |
>>>> awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if
>>>> (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'
>>>>
>>>>
>>> This breaks msvc build because msys's link.exe is tested instead of mslink
>>> script
>>
>> Sounds more like an issue with your build environment to me? Make sure
>> the msvc build tools take precedence in your PATH.
>
> I believe Wang is correct, it should be checking mslink, which is our own script
> specifically to wrap link.exe.
>
> See compat/windows/mslink and its history.
>

--toolchain=msvc also sets mslink as ld_default, unless you override
that on the commandline or didn't use --toolchain at all, I'm not sure
how link.exe would otherwise ever factor into there.
There is no other magic to redirect link.exe to mslink, only
--toolchain sets that.

- Hendrik
diff mbox

Patch

diff --git a/configure b/configure
index fcfa7aa442..f8c55876e5 100755
--- a/configure
+++ b/configure
@@ -4216,7 +4216,7 @@  probe_cc(){
         _depflags='-MMD'
         _cflags_speed='-O3'
         _cflags_size='-Os'
-    elif $_cc -v 2>&1 | grep -q clang; then
+    elif $_cc -v 2>&1 | grep -q clang && ! $_cc -? > /dev/null 2>&1; then
         _type=clang
         _ident=$($_cc --version 2>/dev/null | head -n1)
         _depflags='-MMD -MF $(@:.o=.d) -MT $@'
@@ -4287,7 +4287,7 @@  probe_cc(){
         _flags_filter=msvc_flags
         _ld_lib='lib%.a'
         _ld_path='-libpath:'
-    elif $_cc -nologo- 2>&1 | grep -q Microsoft; then
+    elif $_cc -nologo- 2>&1 | grep -q Microsoft || $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; then
         _type=msvc
         _ident=$($_cc 2>&1 | head -n1)
         _DEPCMD='$(DEP$(1)) $(DEP$(1)FLAGS) $($(1)DEP_FLAGS) $< 2>&1 | awk '\''/including/ { sub(/^.*file: */, ""); gsub(/\\/, "/"); if (!match($$0, / /)) print "$@:", $$0 }'\'' > $(@:.o=.d)'