Review Request 113402: Fix KI18n standalone build
Aurélien Gâteau
agateau at kde.org
Mon Oct 28 12:00:12 UTC 2013
> On Oct. 23, 2013, 4:40 p.m., Chusslove Illich wrote:
> > I can only say that whatever is the proper fix here, it is fine with me.
> > Since non-installed headers are being used, maybe ki18n is using KJS in a
> > way which became deprecated somewhere along the way? If so, I've nothing
> > against fixing that instead.
> >
>
> Aleix Pol Gonzalez wrote:
> Well the thing is that ki18n was using private API (since it was in the same module, kdelibs).
>
> Aurélien: what about installing these headers in a private/ directory?
>
> Chusslove Illich wrote:
> If I recall, when implementing that I mimicked what khtml was doing.
> And apparently is still doing.
>
>
> Aleix Pol Gonzalez wrote:
> khtml is in kdelibs as well, nobody has tried to build it standalone just yet, I guess.
>
> Chusslove Illich wrote:
> Right, but, how is then KJS supposed to be used in a different way? Are
> these private headers really private, or actually necessary to use KJS as a
> JS interpreter in random client code?
>
>
> Aurélien Gâteau wrote:
> The ideal solution would be for ktranscript to only use public headers, but I assume there is no way to do so (I haven't taken the time to study ktranscript enough to understand what it does), Chusslove, it would be awesome if you could have a look at it.
>
> If it is definitely not possible, then I am going to update my changes to install to a private/ directory, like Aleix suggests.
>
> Aurélien Gâteau wrote:
> Actually, the even more ideal solution would be for ktranscript to use QtScript instead of KJS. In the future we will have more and more applications developed with QtQuick, so they will already link to QtScript, loading another JavaScript interpreter sounds wasteful to me. Do you think this is doable? I would be quite interested in helping making it happen.
>
> Aurélien Gâteau wrote:
> Mmm, actually I am wrong, QtScript is provided for Qt 4.x compatibility, QtQuick uses the QJS* classes from QtQML, so this is what we should be using.
>
> Kevin Ottens wrote:
> I would definitely welcome such a port... I remember asking ktranscript to be ported away from KJS. Would also make ki18n tier 1.
>
> Aurélien Gâteau wrote:
> I started diving into this today. No change for now, but I wrote some unit tests for ktranscript, so that a qjs port can be evaluated. Patchset is here: http://agateau.com/tmp/ki18n-qjs.patch but it's missing loads of tests for now. Chusslove: would you be interested in extending the test coverage?
>
> Chusslove Illich wrote:
> Yes, I most certainly would be interested in extending the test coverage.
> But I intended to do it "full-range", that is starting from test PO files
> (which are already there), once I get to it. And first I need to document it
> better :) Currently all there is is this wiki page:
> http://techbase.kde.org/Localization/Concepts/Transcript
>
> As for the port away from KJS, I have to state again that I see no value in
> that. Having ki18n be tier 1 is of no use: if someone is so tight on
> dependencies that using a tier 2 framework is a problem, then ki18n is the
> first thing to dump in favor of built-in Qt translation system, tier 1 or
> not. Note that ki18n also brings into play all the Gettext tools, as opposed
> to self-contained Qt translation tools. The only possible advantage of ki18n
> in tier 1 is that it can be used by tier 2 frameworks, of which there is not
> much by my estimate (and between tier 1 and tier 3, I also fail to see the
> reason for existence of tier 2).
>
>
> Aurélien Gâteau wrote:
> There is value in both full-range tests and tests exercising smaller parts of the code as well IMO.
>
> Thanks for the link, I am going to look at it.
>
> Regarding the port to QJSEngine, I see the following advantages to it:
> - It solves the build issue I have been trying to fix with this request: no need for uninstalled headers anymore.
> - The code is going to be simpler: from what I read of QJSEngine and KJS, exposing an object using QJSEngine is simpler than with KJS: no need for lookup tables or things like that, just a QObject with slots.
> - I expect QJSEngine to be better maintained than KJS, given that it is the backbone of QtQuick.
> - Having ki18n in tier1 would make it more attractive to Qt developers since it brings superior translation support. Right now the cost of dragging in a separate JavaScript interpreter, can be perceived as a burden especially for QtQuick developers.
>
> Kevin Ottens wrote:
> Yes, it's not only about the tiers (it's just a nice side-effect). But it's clear that at that point ktranscript is built on top of something huge and unmaintained. It'd be safer built on top of something with proper maintenance if possible.
>
> Chusslove Illich wrote:
> KJS works for this use now, and it always did, without problems. And it will
> be there as a KF5 framework. As I mentioned elsewhere earlier, I am afraid
> that Qt solution is "over-maintained": I'd rather rely on an engine that
> will not scramble to web whims. At least until it stops working, or someone
> finds a use in which it is too slow, or it is simply not there any more.
>
> As for other points:
>
> > [: Aurélien Gâteau :]
> > - It solves the build issue I have been trying to fix with this request:
> > no need for uninstalled headers anymore.
>
> Is there any example of how KJS should be used otherwise? How will KHTML
> link to it differently? If there is noone to answer this, then, in my view,
> these headers should be installed. That is, I agree with the patch :)
> (Though I'd also have the create_hash_table script installed, say as
> kjs_create_hash_table.)
>
> > - The code is going to be simpler
>
> Well, it is simple as it is, for the given use. I don't expect any new
> development in ki18n that would benefit from this yet simpler usage.
>
> > - Having ki18n in tier1 would make it more attractive to Qt developers
> > since it brings superior translation support. Right now the cost of
> > dragging in a separate JavaScript interpreter, can be perceived as a
> > burden especially for QtQuick developers.
>
> Much more costly dependency is the Gettext itself (runtime part in GNU libc,
> and tools in Gettext distribution). The extra translation features are too
> specific: those developers who cannot afford dependencies such as these,
> will also not work with translators who can make use of the extra features.
>
I think it's a mistake to rely on KJS not changing for stability of your code. After all, the people improving KJS most likely do so with KHTML in mind, so it is also subject to web whims. The best solution IMO to ensure stability is unit tests. Regardless of the outcome of this discussion, I think the tests I started to write should be merged in. I am going to file a separate review request for them. They need to be expended though, because they are far from covering all of the Transcript API.
I do not know KJS enough to judget whether you used it correctly. You have a point regarding KHTML use of KJS. I haven't checked, but it's quite likely it is going to present the same issues. Mmm... I could suggest porting KHTML to QJS, but I am not sure this idea is going to be very popular :)
Regarding simplicity, I think it is always good to aim for more simplicity, if only because it makes it easier for developers less familiar with the code to fix issues in it.
- Aurélien
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113402/#review42222
-----------------------------------------------------------
On Oct. 23, 2013, 4:30 p.m., Aurélien Gâteau wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113402/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2013, 4:30 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kdelibs
>
>
> Description
> -------
>
> KI18n depends on KJS to build the ktranscript plugin. This plugin requires an internal KJS Perl script as well as headers which were not installed.
>
> This is a 3-commit patches, which does the following:
>
> 1. Install kjs headers
>
> 2. Install wtf headers, using wtf/ and kjs/ in include directives (because some kjs headers includes wtf headers)
>
> 3. Fix KI18n:
> - Format top-level CMakeLists.txt according to CMake template
> - Duplicate KJS create_hash_table script
> - Add call to find_package(KJS)
>
> Individual patches available from http://agateau.com/tmp/ki18n-standalone.patch
>
>
> Diffs
> -----
>
> superbuild/CMakeLists.txt 5cdec94
> tier1/kjs/src/CMakeLists.txt 8629716
> tier1/kjs/src/kjs/CMakeLists.txt 9523e89
> tier1/kjs/src/wtf/CMakeLists.txt 83b4417
> tier1/kjs/src/wtf/FastMalloc.h 29a72a5
> tier1/kjs/src/wtf/HashCountedSet.h be3c387
> tier1/kjs/src/wtf/HashFunctions.h f665447
> tier1/kjs/src/wtf/HashMap.h ba2971c
> tier1/kjs/src/wtf/HashSet.h e84b349
> tier1/kjs/src/wtf/HashTable.h 0b2c22c
> tier1/kjs/src/wtf/HashTable.cpp e08d09a
> tier1/kjs/src/wtf/HashTraits.h 4d01726
> tier1/kjs/src/wtf/ListRefPtr.h 0a704d8
> tier1/kjs/src/wtf/OwnArrayPtr.h 3b77871
> tier1/kjs/src/wtf/OwnPtr.h 188a1dc
> tier1/kjs/src/wtf/PassRefPtr.h 25b9906
> tier1/kjs/src/wtf/RefPtr.h 493ab05
> tier1/kjs/src/wtf/Vector.h 9b0f38a
> tier1/kjs/src/wtf/VectorTraits.h 31ae028
> tier2/ki18n/CMakeLists.txt 4cc8e30
> tier2/ki18n/src/CMakeLists.txt 7f8259b4
> tier2/ki18n/src/create_hash_table PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/113402/diff/
>
>
> Testing
> -------
>
> Builds within kdelibs and standalone.
>
>
> Thanks,
>
> Aurélien Gâteau
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131028/29708215/attachment.html>
More information about the Kde-frameworks-devel
mailing list