Analysis: why I think PR #586 is not good #617
Closed
pzhlkj6612
started this conversation in
Show and tell
Replies: 1 comment 2 replies
-
The cqtdeployer tool will move to cmake build system. So I think it is no actuality article |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Summary
This is an analysis of why PR #586 not only did not solve all the problems, but also introduced new problems, but our test did not fail at that time.
I didn't figure out why the commands in the Makefile can be successfully executed through "mingw32-make.exe" before. Although it passed the test, I thought it didn't look good, so I said "unconditionally adding quotes may break the program", and didn't provide any proof. After analysis, I know that adding quotes unconditionally in qmake code like #586 will not break anything, because doing so has no actual effect. Moreover, there are indeed some problems that are hid by those passed tests, so we should think more about it.
At the same time, I also provide a patch to fix the problem introduced in #586 , although it is useless since we are no longer specifying the full path to the installed CQtDeployer. Here is the patch:
useless patch
Analysis
Here are some analyses that are not directly related to the code in our program, which will help us to understand the behavior of our code.
qmake
We could know the following rules of qmake through experiments (yes, I tried to read the source code of qmake before, but... it's toooooo complicated):
foo = bar
means that the value of the "foo" variable is a single-element list, and its first and only element is the string literal "bar". And the statementfoo = bar baz
means that there are two elements in the "foo" variable.''
or""
) represents neither an empty string nor an empty string enclosed in quotes, but an ignorable empty statement (i.e., it's nothing). In function calls, the statementsfunc(,)
,func('',)
,func(, "")
andfunc("", '')
have the same meaning, which is calling the functionfunc
with two empty parameters.' '
," "
,'foo'
or"bar"
) or a function that returns a string ('$$absolute_path(foo, C:\bar)'
or"$$clean_path(/foo/bar/..)"
) will not change the value of that string (or the returned string).joined = '$$var'
orjoined = "$$files(C:/Program Files/Windows M*)"
) or calling the$$quote()
function (joined = $$quote($$var)
orjoined = $$quote($$files(C:/Program Files/Windows M*))
). If the RHS variable only contains one string, the value of that string will be directly copied to the LHS variable without any changes. You can "expand" an arbitrary variable in this way. In addition, you can also directly print out the "expanded" list viamessage($$var)
.\'
or\"
), enclose it in another type of quotes ("'"
or'"'
), or use the builtin functions (shell_quote()
orsystem_quote()
).'foo'bar"baz"
,'"'foo"'"
or\""'\''"'"\"\'"'"\"foo\''\""'"\"'"'"\'
(hey, don't play with quotes and backslashes).$$cat(/etc/hosts, lines)$$cat(/etc/networks, lines)$$cat(/etc/shells, lines)
.foo$$escape_expand(bar\\n\\tbaz)
,$$find(CONFIG, re)'foo'
or$$upper(cm)$$lower(AKE, iS, "")$$lower(tHE, '')$$upper(best)
.$$()
expansion syntax and the$$getenv()
replace function will return the value of the specific environment variable as a string without any changes. That is, regardless of whether the value contains spaces, it will not add quotes at the beginning and end. Don't forget that the returned value is a complete string, so the spaces in it will not divide its content into multiple parts like we assign a list to a qmake variable. For example, thefoo = C:\Program Files (x86)
statement will produce a variable named "foo" which contains three elementsC:\Program
,Files
and(x86)
, while thebar = $$getenv(ProgramFiles(x86))
statement will produce a variable named "bar" which contains only one elementC:\Program Files (x86)
.Practice is the sole criterion of truth, so let's read and run the following code:
too long qmake code
I think the syntax of qmake is a bit casual... maybe CMake can save us, I guess.
qmake and Make
We have the following qmake code:
It will generate the following Makefile rule:
Let's copy the command into CMD and execute it:
This is the expected behavior. We do need quotes when a path contains spaces. However, if we build this target with "mingw32-make.exe", CQtDeployer will be launched!
Uh, why does it work?
Make and Windows API
Before reading the source code of "mingw32-make.exe", we could use Process Monitor to see what operations "mingw32-make.exe" did before launching CQtDeployer:
Events from Process Monitor (simplified)
According to the source code of "mingw32-make.exe", I divided these events into two parts and only kept the "Path" column.
The first part, calling SearchPath (source code):
The other part, calling CreateProcess (source code):
SearchPath
I've created a simple program to emulate (I think it's not a "simulation") the process of file search (source code).
CPP code (SearchPath)
Since the
*lpFileName
variable holds an absolute path, the parameterlpPath
will not have any effect on the search process.As long as the five files "C:\Program.exe", "C:\Program.cmd", "C:\Program.bat", "C:\Program" and "C:\Program.com" do not exist, all ten calls to
SearchPath
will fail, and we could get the same events in Process Monitor.Why
C:\\Program*
?The command
C:\Program Files (x86)\CQtDeployer\1.x\cqtdeployer.bat version
in Makefile is divided into several parts by spaces. The first part isC:\Program
, which is regarded as a program that needs to be found and executed (akaargv[0]
).CreateProcess
After enabling outputting the debug information of "jobs", we could take a look at the process of "mingw32-make.exe" for process creation, and know the two important parameters of the
CreateProcess
function:As you can see, the second parameter is the full command from our Makefile.
I've also created a simple program to emulate the process of process creation (source code).
CPP code (CreateProcess)
As long as the two files "C:\Program" and "C:\Program.exe" do not exist, CQtDeployer will be called. This behavior is already documented, but I still think it looks like magic.
Another reference
Further exploring
You may remember that "CMD.EXE" can be launched by typing "cmd" in CMD, PowerShell or the "Run" dialog of Windows Explorer without specifying its extension. Do you know how it is achieved?
Go back to CQtDeployer
After reading my lengthy analysis, we can start analyzing CQtDeployer.
Before the fixes
Both "/test.pri" and "/QIFData/InstallerBase.pri" have the same qmake instructions, they will get the value of the environment variable
cqtdeployer
in the same way:CQtDeployer/test.pri
Line 11 in 7fda866
CQtDeployer/QIFData/InstallerBase.pri
Line 7 in 7fda866
And we have the following qmake instructions:
In "/test.pri":
CQtDeployer/test.pri
Line 14 in 7fda866
In "/QIFData/installerCQtDeployer.pri":
CQtDeployer/QIFData/installerCQtDeployer.pri
Line 20 in 7fda866
In our system, there is an environment varialbe named
cqtdeployer
and its value is"C:\Program Files (x86)\CQtDeployer\1.x\cqtdeployer.bat"
. The above instructions will generate the following Makefile rules to call the installed CQtDeployer:It works fine. However, we know that those quotes in the value of the environment variable will cause trouble in PowerShell ( #581 ), so we should remove them.
After the "arbitrary" fixes...
diff of #586
7fda866...9dd5da5
The changes in the file "/QIFData/packages/cqtdeployer.1_5/meta/installscript.js" is fine, but enclosing the
$$(cqtdeployer)
statement in quotes in the files "/test.pri" and "/QIFData/InstallerBase.pri" has no real effect.Now the value of the environment variable is
C:\Program Files (x86)\CQtDeployer\1.x\cqtdeployer.bat
, the qmake instructions will generate the following rules:Note that the paths to the bat file are not enclosed in quotes.
If we "build" these two targets, "mingw32-make.exe" will not complain anything since CQtDeployer will be found and launched successfully. Yes, it works, but in my opinion, the changed code does not seem to be a best practice, and may confuse people who read the code in the future.
Afterword
I want to share my knowledge and discoveries with the world, so I wrote this analysis. Any discussion is welcomed.
Thank you for reading.
Beta Was this translation helpful? Give feedback.
All reactions