diff mbox series

[FFmpeg-devel] mswindres: Use '-' instead of '/' for rc.exe options

Message ID BYAPR12MB3238D209DB7BA3280BA1113DA4D49@BYAPR12MB3238.namprd12.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel] mswindres: Use '-' instead of '/' for rc.exe options | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Ziemowit Laski Feb. 4, 2023, 7:52 p.m. UTC
Hello Gentlefolk,

I've been bringing up FFMPEG using Visual Studio 2022 and the MINGW64 environment, and came across sundry things that absolutely needed fixes, so I thought I'd submit them as a series of small patches for you to consider.  Here is the first patch.

--Zem
===========================================================================
When building FFMPEG from the MINGW/MSYS shell under Windows, one must not use forward slashes ('/') for command-line options.  MINGW/MSYS interprets these as absolute paths and then automatically rewrites them into equivalent Windows paths.  For example, the '/logo' switch below gets rewritten to something like 'C:/Program Files/Git/logo', and this obviously breaks the build.  Thankfully, most M$ tools accept dashes ('-') as well.

Signed-off-by: Ziemowit Łąski <15880281+zlaski@users.noreply.github.com>
---
 compat/windows/mswindres | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.39.1.windows.1

Comments

Martin Storsjö Feb. 9, 2023, 11:08 a.m. UTC | #1
Hi,

On Sat, 4 Feb 2023, Ziemowit Laski wrote:

> I've been bringing up FFMPEG using Visual Studio 2022 and the MINGW64 
> environment, and came across sundry things that absolutely needed fixes, 
> so I thought I'd submit them as a series of small patches for you to 
> consider.  Here is the first patch.

FWIW, this setup is definitely being used by lots of others already - so 
whenever there's such an issue, the main question to ask is why others 
haven't run into the issue before. But improvements are definitely 
welcome!

For this patch, the answer to that question is that configure has a test, 
which checks if "$windres --version" works, and if not, it doesn't try to 
use the tool. So in many cases, the mswindres hasn't been used at all.

> When building FFMPEG from the MINGW/MSYS shell under Windows, one must 
> not use forward slashes ('/') for command-line options.  MINGW/MSYS 
> interprets these as absolute paths and then automatically rewrites them 
> into equivalent Windows paths.  For example, the '/logo' switch below 
> gets rewritten to something like 'C:/Program Files/Git/logo', and this

You should probably talk about the option '/nologo' here, there's no 
'/logo' option afaik.

> obviously breaks the build.  Thankfully, most M$ tools accept dashes 
> ('-') as well.
>
> Signed-off-by: Ziemowit Łąski <15880281+zlaski@users.noreply.github.com>
> ---
> compat/windows/mswindres | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/compat/windows/mswindres b/compat/windows/mswindres
> index 450525a33e..ed32796230 100755
> --- a/compat/windows/mswindres
> +++ b/compat/windows/mswindres
> @@ -10,12 +10,12 @@ if [ $# -lt 2 ]; then
>     exit 0
> fi
>
> -EXTRA_OPTS="/nologo"
> +EXTRA_OPTS="-nologo"

These changes seem fine, but you're apparently not touching the case at 
the top, used for --version, where it is calling 'rc.exe /?'. For me, this 
causes configure to decide to not use the tool at all. (It seems like 
current msys2 rewrites "rc.exe /?" so that it fails, while git bash 
doesn't rewrite it in that way, so that "rc.exe /?" works there.)

So I guess that's the explanation for the issue you're seeing - you're 
running in an environment where the "rc.exe /?" check succeeds (so the 
tool is taken into use, unlike it is for me), but then failed at actual 
use. While it for me, building in msys2, didn't try to use the tool at 
all. (And in cross-msvc builds, it enabled and used the tool just fine.)

Anyway, with the commit message fixed, and the case of /? changed into -?, 
this patch would seem fine to me - thanks for your contribution!

// Martin
Hendrik Leppkes Feb. 9, 2023, 9:59 p.m. UTC | #2
On Thu, Feb 9, 2023 at 10:02 PM Ziemowit Laski <zlaski@ziemas.net> wrote:
>
> > FWIW, this setup is definitely being used by lots of others already - so
> > whenever there's such an issue, the main question to ask is why others
> > haven't run into the issue before. But improvements are definitely
> > welcome!
>
> You have to have PATH set up so that rc.exe is found inside the Windows SDK.
> This is NOT the default behavior.  Usually, you would have to invoke the
> 'Developer Command Prompt' from the start menu to get the correct environment.
>
> But I have my system set up so that the Microsoft tools are ALWAYS found,
> regardless of which shell you are running.  That could explain the different
> behavior that I'm seeing.
>
> > You should probably talk about the option '/nologo' here, there's no
> > '/logo' option afaik.
>
> Yes, good catch, thanks.
>
> > These changes seem fine, but you're apparently not touching the case at
> > the top, used for --version, where it is calling 'rc.exe /?'. For me, this
>
> That's an interesting point.  I guess MinGW is "smart enough" not to rewrite
> "/?" because it doesn't represent a valid path to begin with.  I will change
> it to "-?" as you suggest.
>

You would think so, but attempting to run rc.exe /? straight from a
msys bash prompt over here does come up with an invalid file error, so
definitely better to fix it.

- Hendrik
Martin Storsjö Feb. 9, 2023, 10:14 p.m. UTC | #3
On Thu, 9 Feb 2023, Ziemowit Laski wrote:

>> These changes seem fine, but you're apparently not touching the case at
>> the top, used for --version, where it is calling 'rc.exe /?'. For me, this
>
> That's an interesting point.  I guess MinGW is "smart enough" not to rewrite
> "/?" because it doesn't represent a valid path to begin with.  I will change
> it to "-?" as you suggest.

Just to clarify some details here - mingw doesn't do any such path 
rewriting - that's all msys's doing. Mingw processes themselves are 
entirely regular win32 processes which know nothing about unix style 
paths; it's msys2 which does the whole unix-style paths and which 
automatically tries to rewrite command line arguments as if they were 
paths, with some level of heuristics.

Git bash also uses the msys2 layer, but apparently it uses a different 
version of the msys2 runtime, since it didn't seem to fail when executing 
"rc.exe /?", while the current msys2 version fails.

None of that has anything to do with mingw.

// Martin
Ziemowit Laski Feb. 10, 2023, 1:47 a.m. UTC | #4
> paths; it's msys2 which does the whole unix-style paths and which
> automatically tries to rewrite command line arguments as if they were
> paths, with some level of heuristics.

Yes, this makes sense.  The build uses /bin/bash, which is configured as 
x86_64-pc-msys, and that's where the rewriting happens.  I always assumed that
MinGW was basically built on top of MSYS.
 
--Zem
Martin Storsjö Feb. 10, 2023, 10:22 p.m. UTC | #5
On Fri, 10 Feb 2023, Ziemowit Laski wrote:

> The build uses /bin/bash, which is configured as x86_64-pc-msys, and 
> that's where the rewriting happens.  I always assumed that MinGW was 
> basically built on top of MSYS.

No, that's a rather incorrect understanding.

MinGW is an environment which gives you totally win32 native executables, 
just like MSVC does. The main feature is that the headers/libs are 
constructed manually so they're freely redistributable, in a GCC 
compatible form, and with only a very minimal set of extra portability 
helping functions (not providing POSIX compat, just the very minimum 
necessary to make GCC work).

MinGW toolchains can be cross compilers from unix systems, or they can be 
win32 native. They can work on their own (where you call the compiler from 
an IDE, with GNU make in windows mode with cmd.exe executing makefile 
statements, or from another build tool). They can also be packaged as part 
of a posix compat environment.

MSYS2 is a stripped down version of Cygwin, which both gives you a POSIX 
compatibility environment. MSYS2 (and Cygwin) gives you things that don't 
work in a pure win32 environment, such as shell scripts, bash etc. MSYS2 
thus allows you to run e.g. unix style configure shell scripts. MSYS2 
comes packaged with MinGW toolchains integrated, and the main purpose of 
it (contrary to Cygwin) is to provide the extra unix tools you need around 
a MinGW toolchain.

FFmpeg requires MSYS2 (or similar) for running the configure shell script 
and the makefile. But all the weirdness about unix vs windows path 
handling is an MSYS2 feature - the mingw executables are just like any 
windows executable.

// Martin
Ziemowit Laski Feb. 11, 2023, 12:13 a.m. UTC | #6
Thanks for the detailed explanation.  Now my head hurts.

I guess by "native" you mean those apps that do not depend on MSYS/Cygwin
DLL functionality?  All of the apps are native.

So I can definitely understand the appeal of MinGW.  But what about
MSYS?  Is it some sort of copyright/GPL avoidance scheme?

Thanks again for your wisdom.

--Zem
Martin Storsjö Feb. 12, 2023, 10:25 p.m. UTC | #7
On Sat, 11 Feb 2023, Ziemowit Laski wrote:

> Thanks for the detailed explanation.  Now my head hurts.
>
> I guess by "native" you mean those apps that do not depend on MSYS/Cygwin
> DLL functionality?  All of the apps are native.

Yeah, more or less. They use the regular win32 api just like MSVC based 
apps do.

> So I can definitely understand the appeal of MinGW.  But what about
> MSYS?  Is it some sort of copyright/GPL avoidance scheme?

Not at all - it's entirely a technical thing. MSYS, which is a fork of 
cygwin, is an environment to provide a full POSIX (i.e. unix) environment 
on top of Windows, which involves a lot of trickery to make system calls 
like fork() work on top of a OS that doesn't provide that.

MSYS apps generally don't use/see the win32 API at all (when building 
MSYS/Cygwin code, the '_WIN32' define isn't set), they believe they're 
running on top of any regular unix, more or less.

GCC can run on plain win32 thanks to mingw, it doesn't use any fancy POSIX 
APIs, but e.g. bash doesn't run on mingw/win32 since it uses so many unix 
specific concepts that don't exist on win32. That is - it's "easier" (or 
more wisely spent effort) to just try to emulate unix concepts in a full 
emulation layer, than to port bash (and other similar tools) to work 
directly on win32.

// Martin
Martin Storsjö Feb. 13, 2023, 7:36 a.m. UTC | #8
On Mon, 13 Feb 2023, Ziemowit Laski wrote:

>> Not at all - it's entirely a technical thing. MSYS, which is a fork of
>> cygwin, is an environment to provide a full POSIX (i.e. unix) environment
>> on top of Windows, which involves a lot of trickery to make system calls
>> like fork() work on top of a OS that doesn't provide that.
>
> But that's exactly what Cygwin provides, no?

Yes - MSYS is a fork of Cygwin, with just some tweaks and repackaging for 
a slightly different purpose/usecase. (Cygwin wants to be its own universe 
where all tools are built in Cygwin mode, while MSYS is intentionally 
meant to interoperate more with tools running in native win32 mode.)

// Martin
diff mbox series

Patch

diff --git a/compat/windows/mswindres b/compat/windows/mswindres
index 450525a33e..ed32796230 100755
--- a/compat/windows/mswindres
+++ b/compat/windows/mswindres
@@ -10,12 +10,12 @@  if [ $# -lt 2 ]; then
     exit 0
 fi
 
-EXTRA_OPTS="/nologo"
+EXTRA_OPTS="-nologo"
 
 while [ $# -gt 2 ]; do
     case $1 in
-    -D*) EXTRA_OPTS="$EXTRA_OPTS /d$(echo $1 | sed -e "s/^..//" -e "s/ /\\\\ /g")" ;;
-    -I*) EXTRA_OPTS="$EXTRA_OPTS /i$(echo $1 | sed -e "s/^..//" -e "s/ /\\\\ /g")" ;;
+    -D*) EXTRA_OPTS="$EXTRA_OPTS -d$(echo $1 | sed -e "s/^..//" -e "s/ /\\\\ /g")" ;;
+    -I*) EXTRA_OPTS="$EXTRA_OPTS -i$(echo $1 | sed -e "s/^..//" -e "s/ /\\\\ /g")" ;;
     -o)  OPT_OUT="$2"; shift ;;
     esac
     shift
@@ -29,4 +29,4 @@  else
 fi
 
 eval set -- $EXTRA_OPTS
-rc.exe "$@" /fo "$OUT" "$IN"
+rc.exe "$@" -fo "$OUT" "$IN"