Removing qt4 capability from faust2{faustvst,jaqt,lv2} #349

dvzrv · 2019-09-28T10:16:41Z

This removes qt4 functionality from faust2{faustvst,jaqt,lv2} as discussed in #300.
Additionally this fixes many (but not all!) general problems (e.g. unquoted variables, etc.), found using shellcheck.

There's still plenty of tab poisoning going on in these files, too, but that can be done in a separate pull request.

It would be awesome, if these scripts could be integration tested during each pull request.

sletz · 2019-09-28T14:15:08Z

Thanks.

faust2jaqt does not work anymore here on OS X. Does it works for you?
why is "Quoting all variables" needed ?

sletz · 2019-09-28T14:21:25Z

faust2jaqt good.dsp
ERROR : cannot open file '' : No such file or directory

dvzrv · 2019-09-28T15:00:03Z

Well, a lot of variables are not quoted and yet they might contain e.g. paths with whitespaces in them.
This is very dangerous, when e.g. removing files recursively, as this can lead to a lot of files being removed, that shouldn't be removed.
The easiest way to circumvent this is to quote variables. This also has the upside of being lintable using shellcheck :-)

dvzrv · 2019-09-28T15:01:12Z

Do you have a good example to build using faust2jaqt? I have no macOS system available, so I can't really test that.
It's fairly possible, that I broke something.

dvzrv · 2019-09-28T16:29:52Z

tools/faust2appls/faust2jaqt

- $QMAKE -project "QT += widgets printsupport network" "CONFIG+=warn_off" "$CLANGOPT" "INCLUDEPATH+=$CUR" "INCLUDEPATH+=$FAUSTINC /opt/local/include" "QMAKE_CXXFLAGS=$CXXFLAGS -Wno-unused-parameter $FAUSTTOOLSFLAGS" "LIBS+=$ARCHLIB $SOUNDFILELIBS $SAMPLERATELIBS $OSCLIBS $HTTPLIBS" "HEADERS+=$FAUSTINC/faust/gui/QTUI.h" "RESOURCES+= $FAUSTINC/faust/gui/Styles/Grey.qrc" "$OSCDEFS" "$HTTPDEFS" "$QRDEFS" "$POLYDEFS" "$MIDIDEFS" "$SOUNDFILEDEFS" "$SAMPLERATEDEFS"
- $QMAKE $SPEC QMAKE_CFLAGS_ISYSTEM=-I
+ "$QMAKE" -project "QT += widgets printsupport network" "CONFIG+=warn_off" "$CLANGOPT" "INCLUDEPATH+=$CUR" "INCLUDEPATH+=$FAUSTINC /opt/local/include" "QMAKE_CXXFLAGS=$CXXFLAGS -Wno-unused-parameter $FAUSTTOOLSFLAGS" "LIBS+=$ARCHLIB $SOUNDFILELIBS $SAMPLERATELIBS $OSCLIBS $HTTPLIBS" "HEADERS+=$FAUSTINC/faust/gui/QTUI.h" "RESOURCES+= $FAUSTINC/faust/gui/Styles/Grey.qrc" "$OSCDEFS" "$HTTPDEFS" "$QRDEFS" "$POLYDEFS" "$MIDIDEFS" "$SOUNDFILEDEFS" "$SAMPLERATEDEFS"
+ "$QMAKE" "$SPEC" QMAKE_CFLAGS_ISYSTEM=-I


It seems, that "$SPEC" is empty on all OS, but Darwin, which means, that it can not be quoted here easily.
Relying on variables in such a way is not robust and should be changed.

dvzrv · 2019-09-28T16:31:03Z

tools/faust2appls/faust2jaqt

 faust -i -cn effect -a minimal-effect.cpp "$SRCDIR/$EFFECT" -o "$TMP/effect.h" || exit
 fi
 else
- faust -i -json -a $ARCHFILE $OPTIONS "$SRCDIR/$f" -o "$TMP/${f%.dsp}_tmp.cpp" || exit
+ faust -i -json -a "$ARCHFILE" "$OPTIONS" "$SRCDIR/$f" -o "$TMP/${f%.dsp}_tmp.cpp" || exit


Same as with $SPEC, $OPTIONS might be empty and therefore can not be quoted easily.
This is another case of unrobust variable use and should be changed.

dvzrv · 2019-09-28T17:13:40Z

@sletz Please try again with faust2ajqt. this works for me now!

dvzrv · 2019-09-28T17:49:06Z

There were similar issues with how CXXFLAGS and CPPFLAGS were set in faust2lv2.

josmithiii · 2019-09-28T19:16:25Z

I am getting normal results with faust2caqt and faust2jaqt on Mac OS X running Qt 5.12.4_1 installed by MacPorts. There is an error message, however: Project WARNING: Qt has only been tested with version 10.14 of the platform SDK, you're using 10.15. Project WARNING: This is an unsupported configuration. You may experience build issues, and by using Project WARNING: the 10.15 SDK you are opting in to new features that Qt has not been prepared for. Project WARNING: Please downgrade the SDK you use to build your app to version 10.14, or configure Project WARNING: with CONFIG+=sdk_no_version_check when running qmake to silence this warning.

…

On Sat, Sep 28, 2019 at 10:49 AM David Runge ***@***.***> wrote: There were similar issues with how CXXFLAGS and CPPFLAGS were set in faust2lv2. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/

dvzrv · 2019-09-28T20:50:52Z

@josmithiii thanks for the feedback.
The warning message (should) be unrelated to my pull request though.

@sletz do you have an idea what FAUSTTOOLSFLAGS is used for and where it is defined? It's being used in the scripts, but it's not defined anywhere.

josmithiii · 2019-09-28T21:29:16Z

It is not defined in my shell, so I think it is empty when used by the script. It looks to me like a hook for finding faust tools not installed in /usr/local/bin, etc., if any.

…

On Sat, Sep 28, 2019 at 1:51 PM David Runge ***@***.***> wrote: @josmithiii <https://github.com/josmithiii> thanks for the feedback. The warning message (should) be unrelated to my pull request though. @sletz <https://github.com/sletz> do you have an idea what FAUSTTOOLSFLAGS is used for and where it is defined? It's being used in the scripts, but it's not defined anywhere. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#349?email_source=notifications&email_token=AAQZKFOJMIVAQVN7UAIVBFDQL67S5A5CNFSM4I3NS3T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73CFGI#issuecomment-536224409>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAQZKFK5VJHNACHZ5RVE5NTQL67S5ANCNFSM4I3NS3TQ> .

-- Julius O. Smith III <[email protected]> Professor of Music and, by courtesy, Electrical Engineering CCRMA, Stanford University http://ccrma.stanford.edu/~jos/ <http://ccrma.stanford.edu/>

dvzrv · 2019-09-28T21:46:29Z

@josmithiii judging from the locations, it's supposed to carry flags. I just don't know why there's no mention of these anywhere else.
Probably a legacy artifact :)

…qt4 functionality and searching for qmake-qt5/qmake using the shell builtin 'command'.

…issues in this script.

…. Quoting some variables.

…roperly. Removing remaining non-qt5 parts.

…ting -e specifically.

… when converting from dsp to cpp, if they are actually defined. Making sure, that SPEC is only used with qmake/qmake-qt5, if it is non-empty.

…s to be able to quote them properly. Only optionally using OPTIONS or FAUSTTOOLSFLAGS when compiling, if they are non-empty.

…o proper arrays, to which additions can just be added with the += operator. Using the arrays as separate elements with the [@] operator in e.g. faust compiler calls. Expanding to single string with [*] operator in qmake call.

…emoving quoted versions of CPPFLAGS. Adding to CPPFLAGS with += operator. Using CPPFLAGS, and OPTIONS by expanding them as several strings with the [@] operator (e.g. in faust or CXX calls). Expanding CPPFLAGS with [*] to single string in calls to QMAKE (because they are being assigned to a variable).

…XXFLAGS and CPPFLAGS into arrays, so they can be more easily handled and safely added to. Only using OPTIONS, if they are actually defined. Only using FAUSTTOOLSFLAGS, if they are actually defined. Dropping qt5 related extra check for x11extras and adding it directly to qmake call.

…strict handling of undefined variables and return code handling. Guarding all potentially undefined variables.

…rding all potentially undefined variables and defining OSCLIBS (formerly wrongly defined as OSCLIB), OSCDEFS, HTTPLIBS, HTTPDEFS, QRDEFS, POLYDEFS and MIDIDEFS. These definitions should go into a separate LIBS/DEFS array and plainly be added to on demand.

…e stricter handling of return codes and potentially undefined variables. Guarding all potentially undefined variables. Removing debug echo for dspname.

dvzrv · 2019-12-13T00:05:53Z

@sletz I've rebased against current master-dev. Please check and feel free to merge at any time.

dvzrv commented Sep 28, 2019

View reviewed changes

dvzrv mentioned this pull request Sep 29, 2019

Avoid -march=native as default in CXXFLAGS #304

Open

dvzrv added 14 commits December 13, 2019 00:28

tools/faust2appls/faust2faustvst: Setting shebang properly. Removing …

1b8e6c9

…qt4 functionality and searching for qmake-qt5/qmake using the shell builtin 'command'.

tools/faust2appls/faust2faustvst: Fixing most missing-quotes related …

6d92882

…issues in this script.

tools/faust2appls/faust2jaqt: Removing qt4 capability from the script…

06135bc

…. Quoting some variables.

tools/faust2appls/faust2jaqt: Quoting all variables.

debcefd

tools/faust2appls/faust2lv2: Quoting all variables. Setting shebang p…

8c8a71a

…roperly. Removing remaining non-qt5 parts.

tools/faust2appls/faust2jaqt: Fixing shebang to use /usr/bin/env. Set…

4b928e3

…ting -e specifically.

tools/faust2appls/faust2jaqt: Making sure, that OPTIONS are only used…

9b80cd7

… when converting from dsp to cpp, if they are actually defined. Making sure, that SPEC is only used with qmake/qmake-qt5, if it is non-empty.

tools/faust2appls/faust2lv2: Providing CXXFLAGS and CPPFLAGS in array…

b8c267e

…s to be able to quote them properly. Only optionally using OPTIONS or FAUSTTOOLSFLAGS when compiling, if they are non-empty.

tools/faust2appls/faust2faustvst: Setting -eou bash options for more …

6a8c287

…strict handling of undefined variables and return code handling. Guarding all potentially undefined variables.

tools/faust2appls/faust2lv2: Adding -eou pipefail bash options to hav…

e34135a

…e stricter handling of return codes and potentially undefined variables. Guarding all potentially undefined variables. Removing debug echo for dspname.

dvzrv force-pushed the issue/300 branch from 49a6db9 to e34135a Compare December 13, 2019 00:04

dvzrv mentioned this pull request Dec 20, 2022

Please remove Qt4 references, now Qt versions are 5 and 6 #810

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing qt4 capability from faust2{faustvst,jaqt,lv2} #349

Removing qt4 capability from faust2{faustvst,jaqt,lv2} #349

dvzrv commented Sep 28, 2019

sletz commented Sep 28, 2019

sletz commented Sep 28, 2019 •

edited

dvzrv commented Sep 28, 2019

dvzrv commented Sep 28, 2019

dvzrv Sep 28, 2019

dvzrv Sep 28, 2019

dvzrv commented Sep 28, 2019

dvzrv commented Sep 28, 2019

josmithiii commented Sep 28, 2019 via email

dvzrv commented Sep 28, 2019

josmithiii commented Sep 28, 2019 via email

dvzrv commented Sep 28, 2019

dvzrv commented Dec 13, 2019

Removing qt4 capability from faust2{faustvst,jaqt,lv2} #349

Are you sure you want to change the base?

Removing qt4 capability from faust2{faustvst,jaqt,lv2} #349

Conversation

dvzrv commented Sep 28, 2019

sletz commented Sep 28, 2019

sletz commented Sep 28, 2019 • edited

dvzrv commented Sep 28, 2019

dvzrv commented Sep 28, 2019

dvzrv Sep 28, 2019

Choose a reason for hiding this comment

dvzrv Sep 28, 2019

Choose a reason for hiding this comment

dvzrv commented Sep 28, 2019

dvzrv commented Sep 28, 2019

josmithiii commented Sep 28, 2019 via email

dvzrv commented Sep 28, 2019

josmithiii commented Sep 28, 2019 via email

dvzrv commented Sep 28, 2019

dvzrv commented Dec 13, 2019

sletz commented Sep 28, 2019 •

edited