Set correct subsystem value for Windows Autotools build

[Vort]

Fixes #912

[felix-salfelder]

On Tue, Apr 07, 2020 at 09:29:27PM -0700, Vort wrote:

Fixes #912

thanks.

what is LDFLAGS="$LDFLAGS" supposed to do?

the logic is a bit obscure. e.g.

test "x$use_CLANG" != "xyes" -a "x$GCC" = "xyes"

can it be both? (this is not part of this commit, and I am not suggesting that it is your fault.).

what does -mwindows have to do with GCC? I understand that it is needed in case x$WIN32 = xyes; iow: can the nested test move out?

whichever way, could you please add an explaining sentence ideally with reference into the commit message?

[Vort]

what is LDFLAGS="$LDFLAGS" supposed to do?

I'm not sure. Most likely it is the place, where additional options can be added. My change is just replacing LDDFLAGS, which looks like typo.

can it be both?

Theoretically, compiler may report itself as GCC for compatibility reasons.

this is not part of this commit, and I am not suggesting that it is your fault

My goal here is to get rid of console window. Since broken clang check was preventing me from doing this, I needed to fix it somehow. Of course, there may be lots of other problems in that script, I'm not going to fix them all.

what does -mwindows have to do with GCC?

This option change subsystem value in PE (.exe) file. It is described here: https://gcc.gnu.org/onlinedocs/gcc/x86-Windows-Options.html It may be that other compilers have this option as well, but I did not tested any other compiler except for GCC.

[Vort]

whichever way, could you please add an explaining sentence ideally with reference into the commit message?

I have tried, but since English is not my native language, result may be not good enough.

[felix-salfelder]

On Wed, Apr 08, 2020 at 02:31:17AM -0700, Vort wrote:

what is LDFLAGS="$LDFLAGS" supposed to do?

I'm not sure.

please don't change it then.

Theoretically, compiler may report itself as GCC for compatibility reasons.

okay, i see.

Of course, there may be lots of other problems in that script, I'm not going to fix them all.

I am not asking to fix them all -- however in order to review, I need to understand the lines that you have edited, including their context.

what does -mwindows have to do with GCC?

This option change subsystem value in PE (.exe) file. It is described here: https://gcc.gnu.org/onlinedocs/gcc/x86-Windows-Options.html It may be that other compilers have this option as well, but I did not tested any other compiler except for GCC.

Then why exclude clang, if it reports itself as GCC compatible?

thanks

[felix-salfelder]

On Wed, Apr 08, 2020 at 02:48:00AM -0700, Vort wrote:

whichever way, could you please add an explaining sentence ideally with reference into the commit message?

I have tried, but since English is not my native language, result may be not good enough.

Thanks, We are getting there. I do not care about language.

As you found, the current line

if test "$CXX -dM -E - < /dev/null | grep clang" ; then

does not do seem to do anything useful. Now it's

if test "$CXX -dM -E - < /dev/null | grep __clang__" ; then

what do the backticks do? And what is "test" for?

have you tried

if $CXX -dM -E - < /dev/null | grep clang > /dev/null ; then

instead?

[Vort]

Then why exclude clang, if it reports itself as GCC compatible?

Because I have even less experience with Clang than GCC. But people tell that this option should also work with Clang: https://stackoverflow.com/a/58948885/8680450 And my quick test confirms this.

Let's test it with AppVeyor then.

please don't change it then.

I am not asking to fix them all -- however in order to review, I need to understand the lines that you have edited, including their context.

Ok. One bug -> one fix. If GCC build will succeed, then Clang check changes can be omitted.

[Vort]

I have checked https://ci.appveyor.com/api/buildjobs/tljf0hw88jsfj647/artifacts/qucs-win64.zip and it works good (=no console window).

[thomaslepoix]

As you found, the current line

if test "$CXX -dM -E - < /dev/null | grep __clang__" ; then

does not do seem to do anything useful. Now it's

if test "`$CXX -dM -E - < /dev/null | grep __clang__`" ; then

what do the backticks do? And what is "test" for?

Backticks, just as $( ), execute the command inside and become its result. Then test checks if the string between double quotes is empty (exit status 0) or not (exit status 1). A more common syntax would have been :

if [ "$( $CXX -dM -E - < /dev/null | grep __clang__ )" ] ; then

have you tried

if $CXX -dM -E - < /dev/null | grep __clang__ > /dev/null ; then

instead?

Both are valid

[thomaslepoix]

what does -mwindows have to do with GCC?

Then why exclude clang, if it reports itself as GCC compatible?

I am not sure that clang reports itself as GCC but it is mostly compatible with it (command line interface, pragma, etc). MinGW on the other hand does because it is a Windows port of GCC.

[felix-salfelder]

On Wed, Apr 08, 2020 at 05:47:11AM -0700, Thomas Lepoix wrote:

have you tried

if $CXX -dM -E - < /dev/null | grep __clang__ > /dev/null ; then

instead?

Both are valid

Validity is not the real question, but simplicity. We have just witnessed a line overloaded with syntax going south.

We neither need test nor do we need a subshell nor double quotes for this. This is sufficient reason not to use it.

[felix-salfelder]

On Wed, Apr 08, 2020 at 05:56:15AM -0700, Thomas Lepoix wrote:

what does -mwindows have to do with GCC?

Then why exclude clang, if it reports itself as GCC compatible?

I am not sure that clang reports itself as GCC but it is mostly compatible with it (command line interface, pragma, etc). MinGW on the other hand does because it is a Windows port of GCC.

84ae6e4190 looks good. We should take a note about the broken test, when we merge this.

[Vort]

We should take a note about the broken test, when we merge this.

I have created #1001 instead.

[felix-salfelder]

should be merged into develop