All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] qt: make installation of translation files optional
@ 2014-08-02  3:08 Danomi Manchego
  2014-08-03  8:30 ` Thomas Petazzoni
  2014-08-13 13:59 ` Luca Ceresoli
  0 siblings, 2 replies; 5+ messages in thread
From: Danomi Manchego @ 2014-08-02  3:08 UTC (permalink / raw)
  To: buildroot

Commit 93917b6980f7f2b51302e1a3fa451b07cf7d674e introduced the
installation of the binary .qm translation files, unconditionally.
This patch introduces an option to disable this behavior, saving
almost 8MB of space.

Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>

---

Note: the new option is defaulted to "y", to match the current qt.mk operation.
---
 package/qt/Config.in |    7 +++++++
 package/qt/qt.mk     |    2 ++
 2 files changed, 9 insertions(+)

diff --git a/package/qt/Config.in b/package/qt/Config.in
index 0a21e93..a84d082 100644
--- a/package/qt/Config.in
+++ b/package/qt/Config.in
@@ -416,6 +416,13 @@ config BR2_PACKAGE_QT_DECLARATIVE
 	  Build the Qt Declarative Module for qml support
 	  if unsure, say n.
 
+config BR2_PACKAGE_QT_INSTALL_TRANSLATION_FILES
+	bool "Install translation files"
+	default y
+	help
+	  Install binary .qm translation files.
+	  if unsure, say y.
+
 config BR2_PACKAGE_QT_TEST
 	bool "Test Module"
 	help
diff --git a/package/qt/qt.mk b/package/qt/qt.mk
index 82a5509..cbc4d35 100644
--- a/package/qt/qt.mk
+++ b/package/qt/qt.mk
@@ -670,12 +670,14 @@ define QT_INSTALL_TARGET_POWERVR
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_QT_INSTALL_TRANSLATION_FILES),y)
 define QT_INSTALL_TARGET_TRANSLATIONS
 	if [ -d $(STAGING_DIR)/usr/share/qt/translations/ ] ; then \
 		mkdir -p $(TARGET_DIR)/usr/share/qt/translations ; \
 		cp -dpfr $(STAGING_DIR)/usr/share/qt/translations/* $(TARGET_DIR)/usr/share/qt/translations ; \
 	fi
 endef
+endif
 
 define QT_INSTALL_TARGET_CMDS
 	$(QT_INSTALL_TARGET_LIBS)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [Buildroot] [PATCH 1/1] qt: make installation of translation files optional
  2014-08-02  3:08 [Buildroot] [PATCH 1/1] qt: make installation of translation files optional Danomi Manchego
@ 2014-08-03  8:30 ` Thomas Petazzoni
  2014-08-13 13:59 ` Luca Ceresoli
  1 sibling, 0 replies; 5+ messages in thread
From: Thomas Petazzoni @ 2014-08-03  8:30 UTC (permalink / raw)
  To: buildroot

Dear Danomi Manchego,

On Fri,  1 Aug 2014 23:08:56 -0400, Danomi Manchego wrote:
> Commit 93917b6980f7f2b51302e1a3fa451b07cf7d674e introduced the
> installation of the binary .qm translation files, unconditionally.
> This patch introduces an option to disable this behavior, saving
> almost 8MB of space.
> 
> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>

Thanks, I've applied your patch, after renaming the option and moving
it a bit in Config.in, so that it doesn't appear in the middle of the
Qt modules options.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Buildroot] [PATCH 1/1] qt: make installation of translation files optional
  2014-08-02  3:08 [Buildroot] [PATCH 1/1] qt: make installation of translation files optional Danomi Manchego
  2014-08-03  8:30 ` Thomas Petazzoni
@ 2014-08-13 13:59 ` Luca Ceresoli
  2014-08-13 16:27   ` Danomi Manchego
  2014-08-13 17:25   ` Thomas Petazzoni
  1 sibling, 2 replies; 5+ messages in thread
From: Luca Ceresoli @ 2014-08-13 13:59 UTC (permalink / raw)
  To: buildroot

Dear Danomi, Vivien,

[Copying Vivien as the author of commit 93917b6980f7, which originated
the issue.]

I have a comment on this patch, even though it has already been
committed.

Danomi Manchego wrote:
> Commit 93917b6980f7f2b51302e1a3fa451b07cf7d674e introduced the
> installation of the binary .qm translation files, unconditionally.
> This patch introduces an option to disable this behavior, saving
> almost 8MB of space.
>
> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
>
> ---
>
> Note: the new option is defaulted to "y", to match the current qt.mk operation.

I'm not sure this is a good default value.
Surely it preserves backward compatibility with Buildroot versions since
2013.11, when the installation got introduced. But for years the
translation files were never installed, and apparently it was not a
problem to anybody.

The size is very large for some embedded systems. I have Qt-based
systems working since years and never needed them. I have one that is
around 17 MB, becomes 25 MB with translations!

Danomi, Vivien, can you explain exactly what these translations are and
when they are needed?

Unless there is a good reason to have the translations on most targets
I would change the default to no in order to preserve new users from an
unnoticed extra size that might be unneeded.

If you agree, I'd send a patch to change this.

Thanks.
-- 
Luca

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Buildroot] [PATCH 1/1] qt: make installation of translation files optional
  2014-08-13 13:59 ` Luca Ceresoli
@ 2014-08-13 16:27   ` Danomi Manchego
  2014-08-13 17:25   ` Thomas Petazzoni
  1 sibling, 0 replies; 5+ messages in thread
From: Danomi Manchego @ 2014-08-13 16:27 UTC (permalink / raw)
  To: buildroot

Luca,

On Wed, Aug 13, 2014 at 9:59 AM, Luca Ceresoli <luca@lucaceresoli.net> wrote:
> Dear Danomi, Vivien,
>
> [Copying Vivien as the author of commit 93917b6980f7, which originated
> the issue.]
>
> I have a comment on this patch, even though it has already been
> committed.
>
>
> Danomi Manchego wrote:
>>
>> Commit 93917b6980f7f2b51302e1a3fa451b07cf7d674e introduced the
>> installation of the binary .qm translation files, unconditionally.
>> This patch introduces an option to disable this behavior, saving
>> almost 8MB of space.
>>
>> Signed-off-by: Danomi Manchego <danomimanchego123@gmail.com>
>>
>> ---
>>
>> Note: the new option is defaulted to "y", to match the current qt.mk
>> operation.
>
>
> I'm not sure this is a good default value.
> Surely it preserves backward compatibility with Buildroot versions since
> 2013.11, when the installation got introduced. But for years the
> translation files were never installed, and apparently it was not a
> problem to anybody.

I am in total agreement.  In fact, this default of 'y' is *never* good
for my projects, as we make our own translation files.  For our local
customized version of buildroot, I comment out the default of 'y', to
generally discourage its use, and to preserve the *old* behavior
(since we only recently updated to buildroot-2014.05, from a 2013.xx).
My main goal was to get the ability to remove the translation files
into the upstream buildroot, so that my local customization is less
significant (now just the default setting mod in Config.in, rather
than the switch itself and the .mk file mod).

The only reason I defaulted it to 'y' was to make the patch
uncontroversial, to increase the chances of patch acceptance.


> The size is very large for some embedded systems. I have Qt-based
> systems working since years and never needed them. I have one that is
> around 17 MB, becomes 25 MB with translations!
>
> Danomi, Vivien, can you explain exactly what these translations are and
> when they are needed?

Try reading about Qt Linuguist, at
http://qt-project.org/doc/qt-4.8/linguist-manual.html .
Basically, the Linquist tools extract text tokens, and provide a
framework for substituting language translations.


> Unless there is a good reason to have the translations on most targets
> I would change the default to no in order to preserve new users from an
> unnoticed extra size that might be unneeded.
>
> If you agree, I'd send a patch to change this.

Sounds good to me!

Danomi -


> Thanks.
> --
> Luca

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Buildroot] [PATCH 1/1] qt: make installation of translation files optional
  2014-08-13 13:59 ` Luca Ceresoli
  2014-08-13 16:27   ` Danomi Manchego
@ 2014-08-13 17:25   ` Thomas Petazzoni
  1 sibling, 0 replies; 5+ messages in thread
From: Thomas Petazzoni @ 2014-08-13 17:25 UTC (permalink / raw)
  To: buildroot

Dear Luca Ceresoli,

On Wed, 13 Aug 2014 15:59:24 +0200, Luca Ceresoli wrote:

> > Note: the new option is defaulted to "y", to match the current qt.mk operation.
> 
> I'm not sure this is a good default value.
> Surely it preserves backward compatibility with Buildroot versions since
> 2013.11, when the installation got introduced. But for years the
> translation files were never installed, and apparently it was not a
> problem to anybody.
> 
> The size is very large for some embedded systems. I have Qt-based
> systems working since years and never needed them. I have one that is
> around 17 MB, becomes 25 MB with translations!
> 
> Danomi, Vivien, can you explain exactly what these translations are and
> when they are needed?

When applying the patch, I also hesitated a bit on the default value,
and I was tempted to change Danomi's patch to *not* install the
translation files by default. But then, I saw the argument of keeping
the existing behavior, and decided to apply Danomi's patch as is, with
the "default y".

That being said, I am personally completely fine with having this
option default disabled. I think it makes sense to have a lightweight
system by default, so I will support a patch changing the default value
for this option.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-08-13 17:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-02  3:08 [Buildroot] [PATCH 1/1] qt: make installation of translation files optional Danomi Manchego
2014-08-03  8:30 ` Thomas Petazzoni
2014-08-13 13:59 ` Luca Ceresoli
2014-08-13 16:27   ` Danomi Manchego
2014-08-13 17:25   ` Thomas Petazzoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.