CQtDeployer icon indicating copy to clipboard operation
CQtDeployer copied to clipboard

[WIP] Process Qt Bin path correctly to find MSVC redistributable packages

Open pzhlkj6612 opened this issue 4 years ago • 12 comments

Fixed #495 .

Why it occurred

QtDir::setBins(const QString&) uses PathUtils::fixPath(const QString&) to convert all characters in the path to uppercase on Windows.

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/qtdir.cpp#L31-L34

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/pathutils.cpp#L101-L108


So, DeployCore::getVCredist(const QString&) receives the Qt bin path in all uppercase (like C:/QT/5.15.0/MSVC2019_64/BIN) from Extracter::deployMSVC().

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/extracter.cpp#L29-L40

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/qtdir.cpp#L27-L30


Then, DeployCore::getMSVC(const QString&) receives the Qt bin path in all uppercase from DeployCore::getVCredist(const QString&), but he has not processed msvcPath correctly ("msvc" vs. "MSVC" in C:/QT/5.15.0/MSVC2019_64).

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/deploycore.cpp#L513-L515

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/deploycore.cpp#L481-L489


And, neither DeployCore::getMSVCName(MSVCVersion) nor DeployCore::getMSVCVersion(MSVCVersion) uses the correct operator for matching, so DeployCore::getVCredist(const QString&) cannot receive the correct result and find the correct MSVC redist installer.

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/deploycore.cpp#L525-L527

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/Deploy/deploycore.cpp#L540-L563

Finally, it wants to copy the installer whose name contains "msvc2013" and "x86", but this is not what I expected.

So, it's broken.

Why tests passed

The test deploytest::testMSVC() has not covered all possible cases.

https://github.com/QuasarApp/CQtDeployer/blob/d7b48f3529f881f40f30ed1a7b8c21feaab97c22/UnitTests/tst_deploytest.cpp#L501-L520

In addition, I think more tests could be added:

  1. Test DeployCore::getVCredist(const QString&) by creating some dummy files as MSVC redist installer (For example, zero byte file "vcredist_msvc2019_x64.exe").
  2. Test both DeployCore::getMSVCName(MSVCVersion) and DeployCore::getMSVCVersion(MSVCVersion) with all possible values.

If changes applied

stdout:

...
Info: try deploy msvc
Info: copy :C:/QT/vcredist/vcredist_msvc2019_x64.exe
Info: deploy done!
...

Files:

.>tree /A /F
.
|   qt.conf
|   Qt5Core.dll
|   Qt5DBus.dll
|   Qt5Gui.dll
|   Qt5Svg.dll
|   Qt5Widgets.dll
|   test-CQD-1.bat
|   test-CQD-1.exe
|   vcredist_msvc2019_x64.exe <--- hey!
|
...

However, test deploytest::testMSVC() will fail, so I need your help to write some new tests.

pzhlkj6612 avatar Jan 13 '21 15:01 pzhlkj6612

I see you create this pull request to the master branch. The master is a planed 1.6 release - it is very very not stable release. If you want see this fix into release 1.5 reopen the PR to v1.5. Do not forget checkout to new branch with this changes from the v1.5 branch.

EndrII avatar Jan 13 '21 19:01 EndrII

I see you create this pull request to the master branch.

I forgot it. Sorry. I will change it now.

pzhlkj6612 avatar Jan 13 '21 23:01 pzhlkj6612

Bugs

Now cqtdeployer copy msvc pack whenever deploy windows app. It is wrong, cqdeployer must be deploy mvc only for msvc distributions. Bug in the getVCredist method. You need to add validation of MSVCVersion in begin of the getVCredist method.

if (msvc == MSVCVersion::MSVC_Unknown) {
    return "";
}

Deprecated method

The deployMSVC method is deprecated, because it copy msvc redist installer into root of target directory. Now cqtdeployer works with a packages. So there may be a situation where there should be no packages in the root directory. Copying mscv will break installer creation.

For fix this problem we need to add the mscvredis.exe to the separate package as a extra data.

How to it do:

  1. Remove the deployMSVC method from the Extracter class.
  2. Add new option "noDeployMsvc" (you can change name of the option) for disable deploy of the msvcredist installer. (see DeployCore::help() and helpKeys DeployCore::helpKeys())
  3. In the ConfigParser class create a new method "deployMsvc". in this method you can invoke getVCredist method. Check your noDeployMsvc option and invoke method parsePackagesPrivate Example:

check the noDeployMsvc option.
...

auto msvcInstaller = DeployCore::getVCredist(DeployCore::_config->qtDir.getBins());
if (msvcInstaller.isEmpty())
   return;
auto extraData = "NAME_OF_OIFPACKAGE_OF_MSVC_REDIST" + DeployCore::getSeparator(1) + msvcInstaller
if (!parsePackagesPrivate(mainDistro, , &DistroModule::addExtraData))
    error log;

  1. invoke the deployMsvc method in the end of initDistroStruct method.
  2. in The parseDeployMode method move this block :
    if (!initDistroStruct()) {
        return false;
    }

In to bottom of the initQmake block.

    if (!initQmake()) {

        if (DeployCore::isSnap()) {
            QuasarAppUtils::Params::log("If you are using qmake from the system repository,"
                                        " then you should use the classic version of the CQtDeployer instead of the snap version."
                                        " This is due to the fact that the snap version runs in an isolated container and has limited access"
                                        " to system utilities and the environment. "
                                        "For get the classic version of cqtdeployer use the cqtdeployer installer "
                                        "https://github.com/QuasarApp/CQtDeployer/releases");
        }

        return false;
    }
    Insert here.
  1. And goodluck ))

EndrII avatar Jan 14 '21 07:01 EndrII

The 32-bit MSVC redist missed

In my PC, the Qt installation folder is like this:

C:\Qt> tree /A /F
.
\---5.15.0
    +---mingw81_64
    +---msvc2019
    \---msvc2019_64

The directory "msvc2019" contains MSVC 2019 32-bit files. The code cannot handle this, so no 32 bit MSVC redist will be copied.

https://github.com/QuasarApp/CQtDeployer/blob/ea7439eebbb164017f39720b37f73c18850179a2/Deploy/deploycore.cpp#L503-L509

Has there ever been a naming change? I did some tests with the following downloadable Qt versions (Thanks to GitHub Actions, otherwise my PC will die):

  • Qt\5.9\msvc2015, Qt 5.9 with win32_msvc2015 (qt.59.win32_msvc2015)
  • Qt\5.9.1\msvc2015, Qt 5.9.1 with win32_msvc2015 (qt.591.win32_msvc2015)
  • Qt\5.10.1\msvc2015, Qt 5.10.1 with win32_msvc2015 (qt.qt5.5101.win32_msvc2015)
  • Qt\5.11.2\msvc2015, Qt 5.11.2 with win32_msvc2015 (qt.qt5.5112.win32_msvc2015)
  • Qt\5.12.0\msvc2017, Qt 5.12.0 with win32_msvc2017 (qt.qt5.5120.win32_msvc2017)
  • Qt\5.14.1\msvc2017, Qt 5.14.1 with win32_msvc2017 (qt.qt5.5141.win32_msvc2017)

As you can see, no "_32" in the directory name. This is a bug from e734a3701932af6c88f37f43138059c96d419bd1 and I fixed it in 2ab640ad28f5d0b343f32ec10ef66cd4429f565f .


If I deployed 32-bit MSVC program without any change

In "deploycore.cpp", both DeployCore::getMSVCName(MSVCVersion) and DeployCore::getMSVCVersion(MSVCVersion) will return an empty string when the input does not match any pattern. As we all known, an empty string is contained by any string, so the filename matching will not work as expected.

https://github.com/QuasarApp/CQtDeployer/blob/ea7439eebbb164017f39720b37f73c18850179a2/Deploy/deploycore.cpp#L528-L536

For example, when I deployed a MSVC 2019 32-bit program, the file C:/QT/vcredist/vcredist_msvc2019_x64.exe was copied. This is the process:

  1. In DeployCore::getMSVC(const QString&), as variable base was C:/QT/5.15.0/MSVC2019, type was 19 and it did not match any architecture, then the return value was a single MSVC_19, not MSVC_19 | MSVC_x32.

  2. In DeployCore::getVCredist(const QString&), version got an empty string from DeployCore::getMSVCVersion(MSVCVersion).

  3. Got the redist installer file list:

C:\Qt\vcredist> tree /A /F
    vcredist_msvc2017_x64.exe
    vcredist_msvc2017_x86.exe
    vcredist_msvc2019_x64.exe
    vcredist_msvc2019_x86.exe
    vcredist_x64.exe
    vcredist_x86.exe
  1. The variable name was 2019, then filtered out others:
C:\Qt\vcredist> tree /A /F
    vcredist_msvc2019_x64.exe
    vcredist_msvc2019_x86.exe
  1. The variable version was empty, then matched the first file vcredist_msvc2019_x64.exe and copied it. So I got the wrong installer.

pzhlkj6612 avatar Jan 14 '21 10:01 pzhlkj6612

Oh...no. I just noticed your reply. I will read it later.

pzhlkj6612 avatar Jan 14 '21 10:01 pzhlkj6612

@pzhlkj6612 thanks.

About 32-bit MSVC. Yes is a bug, solution is add validation of empty string in begin of deployMSVC method. About fixes. Сarefully read my instructions on what to do. If you have any question about this task just write your question on this poll request.

EndrII avatar Jan 14 '21 10:01 EndrII

@pzhlkj6612 hi When do you plan to sit down for this task?

EndrII avatar Jan 29 '21 13:01 EndrII

@EndrII Hi.

I'm so sorry for the delayed response. During this time, I was working on a project and encountered some complicated problems with the dependency in cross-platform scenarios. It was painful and I spent too much time on it, so I had to suspend work on this task (and any other projects).

I estimate that it will take another week on that project. What is your release plan of CQtDeplyer v1.5 ?

pzhlkj6612 avatar Jan 30 '21 00:01 pzhlkj6612

@EndrII Hi.

I'm so sorry for the delayed response. During this time, I was working on a project and encountered some complicated problems with the dependency in cross-platform scenarios. It was painful and I spent too much time on it, so I had to suspend work on this task (and any other projects).

I estimate that it will take another week on that project. What is your release plan of CQtDeplyer v1.5 ?

Ou, Sounds painful. Good luck with the cross-platform project 👍 As for CQtDeployer v1.5, I plan to complete and release the full release only by the summer, since there are a lot of tasks and there is no time. So you can take your time.

EndrII avatar Jan 30 '21 08:01 EndrII

@pzhlkj6612 do you have any plans of working of this issue ?

EndrII avatar Jul 01 '21 07:07 EndrII

Hi, @EndrII , I think I don't have enough time to complete this work within this year. What is the desired release date of 1.6?

pzhlkj6612 avatar Jul 01 '21 08:07 pzhlkj6612

Hi, @EndrII , I think I don't have enough time to complete this work within this year. What is the desired release date of 1.6?

the 1.6 release do not have deadline date, so you can continue this work in the 2022 year))

EndrII avatar Jul 03 '21 11:07 EndrII

This Pull Request is outdated and not valid anymore. So I close this pr.

EndrII avatar Oct 18 '22 21:10 EndrII