From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:54569) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1imgsm-0007r1-5d for guix-patches@gnu.org; Wed, 01 Jan 2020 11:30:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1imgsk-00007Y-OM for guix-patches@gnu.org; Wed, 01 Jan 2020 11:30:04 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:59718) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1imgsk-00005j-K3 for guix-patches@gnu.org; Wed, 01 Jan 2020 11:30:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1imgsk-00020o-GA for guix-patches@gnu.org; Wed, 01 Jan 2020 11:30:02 -0500 Subject: [bug#35866] [PATCH] gnu: Add qtwebengine. Resent-Message-ID: From: mike.rosset@gmail.com References: <20191218204537.24069-1-mike.rosset@gmail.com> Date: Wed, 01 Jan 2020 08:29:11 -0800 In-Reply-To: (Hartmut Goebel's message of "Tue, 31 Dec 2019 14:10:08 +0100") Message-ID: <87v9pvkunc.fsf@gmail.com> MIME-Version: 1.0 Content-Type: text/plain List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Hartmut Goebel Cc: 35866@debbugs.gnu.org Hartmut Goebel writes: > Hi, > in addtion to other remarks: > > Am 18.12.19 um 21:45 schrieb Mike Rosset > > > + (uri > + (string-append "https://download.qt.io/official_releases/qt/" > + (substring version 0 4) > + "/" version "/submodules/" > + (string-append name "-everywhere-src-" version) > + ".tar.xz")) > > Please use the same code and formatting as sued for qtsvg. This helps keeping the code consistent. > > + `(modify-phases ,phases > + (replace 'configure > + (lambda* (#:key inputs outputs #:allow-other-keys) > + ;; Avoids potential race conditions > > I suggest to put this into *two* new phases: `patch-source` (or so) containing the substitute stuff, and `setup-configure` for setting the environment variables. > > > + (substitute* "src/core/web_engine_library_info.cpp" > + (("QLibraryInfo::location\\(QLibraryInfo::TranslationsPath\\)") > + (format #f "QLatin1String(\"~a\")" (string-append (assoc-ref outputs "out") "/share/qt5/translations"))) > + (("QLibraryInfo::location\\(QLibraryInfo::DataPath\\)") > + (format #f "QLatin1String(\"~a\")" (string-append (assoc-ref outputs "out") "/share/qt5")))) > > I suggest *not* using `format`, as we rarely use it in substitutes and you are using `string-append` anyway. I also suggest to define a variable `out` to be used here (same for `nss`, `udev` below) to avoid having `assic-ref` here, see [0] as an example. Then `format` might even be beneficial: > > + (format #f "QLatin1String(\"~a/share/qt5/translations\")" out) > > + (string-append "QLatin1String(\"" out "/share/qt5/translations\")") > > [0] > I have made all these changes as requested they should be in 0004 of series. I've re emailed the complete series. I believe there was an issue mailing 003. > > + ;; Valid QT_BUILD_PARTS variables are: > + ;; libs tools tests examples demos docs translations > + (invoke "qmake" "QT_BUILD_PARTS = libs tools" "--" > + "--webengine-printing-and-pdf=no" > + "--webengine-ffmpeg=system" > + "--webengine-icu=system" > + "--webengine-pepper-plugins=no"))))) > > Would setting `#:configure-flags` with "-DBUILD_TESTS=off" (see e.g. [1]) work, too, instead of passing "QT_BUILD_PARTS"? > > [1] It's possible that this might work. But I don't think it's worth the effort. I kept this in line qtsvg hopefully I can switch the tests on at one point. I have documented why the tests can not be run as of now.