Re: [FFmpeg-devel] [PATCH] configure: Make MSVC version grabbing more robust

[Alexander Strasser via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

[-- Attachment #1: Type: message/rfc822, Size: 10727 bytes --]

[-- Attachment #1.1.1: Type: text/plain, Size: 3053 bytes --]

On 2025-06-21 00:03 +0200, Kacper Michajlow wrote:
> On Fri, 20 Jun 2025 at 22:26, Hendrik Leppkes
> <h.leppkes-at-gmail.com@ffmpeg.org> wrote:
> >
> > On Fri, Jun 20, 2025 at 9:25 PM Timo Rothenpieler <timo@rothenpieler.org> wrote:
> > >
> > > On 19.06.2025 22:21, Martin Storsjö wrote:
> > > > On Fri, 13 Jun 2025, Martin Storsjö wrote:
> > > >
> > > >> When running plain "cl", to get the MSVC version, it prints the
> > > >> version header on stderr, while the usage instructions are printed
> > > >> on stdout. Usually, the version on stderr gets flushed first,
> > > >> so "head -n1" gets the line it expects, but some times (in particular
> > > >> when running MSVC wrapped in wine), it can get the usage line
> > > >> first.
> > > >>
> > > >> Redirect stdout to /dev/null, so we only grab the version among
> > > >> the lines printed to stderr. This should make the version number
> > > >> grabbing more robust.
> > > >>
> > > >> At least all relevant versions of MSVC seem to print this specifically
> > > >> to stderr, not stdout (so we don't risk to miss it); checked down
> > > >> to MSVC 2010.
> > > >> ---
> > > >> This should avoid the occasionally misdetected version number lines
> > > >> as seen at https://fate.ffmpeg.org/history.cgi?slot=x86_64-msvc2022-wine.
> > > >> ---
> > > >> configure | 5 ++++-
> > > >> 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > Will push.
> > >
> > > Likely this patch broke multiple fate runners in a silent way.
> > > On mine, configure simply never returns, and just sits there
> > > indefinitely, with no CPU usage or any activity whatsoever.
> > >
> > > nevcairiel confirmed seeing the same behaviour on IRC.
> > >
> > > The msys+clang builds from within the same environment work fine.
> > >
> > >
> > > Didn't verify completely if it's caused by this patch, but nothing else
> > > happened with configure since the last successful run.
> >
> > I did some digging, and it happens when probe_cc probes link.exe
> >
> > link.exe has an interactive help output (its paginated) - previously
> > piping stdout disabled the pagination automatically - but redirecting
> > it to devnull does not, and it gets stuck waiting for input.
> > Additionally, link.exe outputs the ident on stdout, so there is no
> > result on stderr (not super bad, as LD_IDENT is never used - yet)
> 
> Instead of redirecting to devnull, we could use the same condition as
> in if. We already look for specific ident line, so no need to head.
> _ident=$($_cc -nologo- 2>&1 | grep ^Microsoft | tr -d '\r')
> should work, no? I would be happy to see a better solution, though.

I think making sure to only use the first line that starts with
Microsoft is more robust; there could be (in the future) more lines
that start with Microsoft.

> btw. running cl.exe 3 times to just get its name is interesting :)

Yes, so that as well. Could be optimized, but is probably not so
relevant in total.

I propose the attached patch.


  Alexander

[-- Attachment #1.1.2: 0001-configure-Fix-a-regression-when-probing-link.exe.patch --]
[-- Type: text/plain, Size: 1405 bytes --]

From f97df3657ab73573659d7738ac55d688c5744bf9 Mon Sep 17 00:00:00 2001
From: Alexander Strasser <eclipse7@gmx.net>
Date: Sat, 21 Jun 2025 11:13:22 +0200
Subject: [PATCH] configure: Fix a regression when probing link.exe

The version ident is printed on stdout for link.exe and redirecting
stdout to /dev/null will cause the output of link.exe to be paged.
---
 configure | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 708ecd9025..ce027d3845 100755
--- a/configure
+++ b/configure
@@ -5130,10 +5130,9 @@ probe_cc(){
     elif $_cc -nologo- 2>&1 | grep -q ^Microsoft || { $_cc -v 2>&1 | grep -q clang && $_cc -? > /dev/null 2>&1; }; then
         _type=msvc
         if $_cc -nologo- 2>&1 | grep -q ^Microsoft; then
-            # The version number is printed on the first line on stderr, stdout
-            # gets the usage instructions. Only include stderr, to avoid
-            # potential ordering race conditions.
-            _ident=$($_cc 2>&1 >/dev/null | head -n1 | tr -d '\r')
+            # Depending on the tool (cl.exe or link.exe), the version number
+            # is printed on the first line of stderr or stdout
+            _ident=$($_cc 2>&1 | grep ^Microsoft | head -n1 | tr -d '\r')
         else
             _ident=$($_cc --version 2>/dev/null | head -n1 | tr -d '\r')
         fi
-- 
2.49.0


[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".