package/rpm: add gettext dependency

PATCH New Romain Naour 2 participants 8 messages Jan 18, 2015

lore

Share & tools

lore

Romain Naour

author

2015-01-18 11:34 UTC #0 lore raw

PATCH 1/3 package/rpm: add gettext dependency

rpm check for libintl library if BR2_NEEDS_GETTEXT_IF_LOCALE is set.

checking for GNU gettext in libintl... yes
checking whether to use NLS... yes
checking where the gettext function comes from... external libintl checking how to link with libintl... -lintl

But the dependency on gettext package is missing to ensures reproducible builds.

Also, sort selected packages alphabetically in Config.in

Signed-off-by Romain Naour <[email protected]>

---
 package/rpm/Config.in | 7 ++++---
 package/rpm/rpm.mk    | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

package/rpm/Config.in +4 -3

index 620eb1a..5ec4167 100644
@@ -7,12 +7,13 @@ config BR2_PACKAGE_RPM
 	depends on BR2_TOOLCHAIN_HAS_THREADS # beecrypt
 	depends on BR2_USE_MMU # fork()
 	select BR2_PACKAGE_BEECRYPT
-	select BR2_PACKAGE_POPT
-	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT_IF_LOCALE
 	select BR2_PACKAGE_NEON
+	select BR2_PACKAGE_NEON_SSL
 	select BR2_PACKAGE_NEON_XML
 	select BR2_PACKAGE_NEON_ZLIB
-	select BR2_PACKAGE_NEON_SSL
+	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_POPT
 	select BR2_PACKAGE_ZLIB
 	help
 	  The RPM package management system.

package/rpm/rpm.mk +4 -0

index c4215b5..74f5b88 100644
@@ -26,6 +26,10 @@ RPM_CONF_OPTS = \
 	--with-libbeecrypt=$(STAGING_DIR) \
 	--with-popt=$(STAGING_DIR)
 
+ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
+RPM_DEPENDENCIES += gettext
+endif
+
 ifeq ($(BR2_PACKAGE_PCRE),y)
 RPM_DEPENDENCIES += pcre
 RPM_CONF_OPTS += --with-pcre=external

Signature · 1 line

1.9.3

Romain Naour

author

2015-01-18 11:34 UTC #1 lore raw

PATCH 2/3 package/rpm: use external xz library

Fixes:
http://autobuild.buildroot.net/results/2c6/2c61562008418ff58fa3085a3342b0e304923073/

Signed-off-by Romain Naour <[email protected]>

---
 package/rpm/Config.in | 1 +
 package/rpm/rpm.mk    | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

package/rpm/Config.in +1 -0

index 5ec4167..fe7a38a 100644
@@ -31,6 +31,7 @@ config BR2_PACKAGE_RPM_BZIP2_PAYLOADS
 config BR2_PACKAGE_RPM_XZ_PAYLOADS
 	bool "support for xz payloads"
 	depends on BR2_INSTALL_LIBSTDCPP
+	select BR2_PACKAGE_XZ
 	help
 	  Support for xz payloads in RPM.

package/rpm/rpm.mk +4 -1

index 74f5b88..ce7189e 100644
@@ -45,7 +45,10 @@ RPM_CONF_OPTS += --with-file=no
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_XZ_PAYLOADS),y)
-RPM_CONF_OPTS += --with-xz
+RPM_CONF_OPTS += --with-xz=external
+RPM_DEPENDENCIES += xz
+else
+RPM_CONF_OPTS += --with-xz=none
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_BZIP2_PAYLOADS),y)

Signature · 1 line

1.9.3

Yann E. MORIN

2015-01-18 14:59 UTC #2 lore raw

PATCH 2/3 package/rpm: use external xz library

Romain, All,

On 2015-01-18 12:34 +0100, Romain Naour spake thusly:

Quoted 2 lines

Fixes:
http://autobuild.buildroot.net/results/2c6/2c61562008418ff58fa3085a3342b0e304923073/

Weird, this build error is about being unable to link with libintl.so (indeed while building the xz backend, but still about libintl).

Igues I could use that autobuild failure to test your patch 1/3, though...

Otherwise, I have some comments, see below...

Quoted 11 lines

Signed-off-by Romain Naour <[email protected]>
---
 package/rpm/Config.in | 1 +
 package/rpm/rpm.mk    | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)
package/rpm/Config.in +0 -0
index 5ec4167..fe7a38a 100644

package/rpm/Config.in config BR2_PACKAGE_RPM_BZIP2_PAYLOADS · line 34

Quoted 4 lines

package/rpm/Config.in +1 -0

@@ -31,6 +31,7 @@ config BR2_PACKAGE_RPM_BZIP2_PAYLOADS
 config BR2_PACKAGE_RPM_XZ_PAYLOADS
 	bool "support for xz payloads"
 	depends on BR2_INSTALL_LIBSTDCPP
+	select BR2_PACKAGE_XZ

I know you are fixing existing code, but we usually do not have such sub-options, and we instead rely entirely on the dependant package in the .mk, without adding such sub-options...

Quoted 8 lines

package/rpm/Config.in +0 -0
 	help
 	  Support for xz payloads in RPM.
 
package/rpm/rpm.mk +0 -0
index 74f5b88..ce7189e 100644

package/rpm/rpm.mk RPM_CONF_OPTS += --with-file=no · line 51

Quoted 8 lines

package/rpm/rpm.mk +4 -1

@@ -45,7 +45,10 @@ RPM_CONF_OPTS += --with-file=no
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_XZ_PAYLOADS),y)
-RPM_CONF_OPTS += --with-xz
+RPM_CONF_OPTS += --with-xz=external
+RPM_DEPENDENCIES += xz
+else
+RPM_CONF_OPTS += --with-xz=none

Reviewed-by "Yann E. MORIN" <[email protected]>

Regards,
Yann E. MORIN.

Quoted 10 lines

package/rpm/rpm.mk +0 -0

 endif
 
 ifeq ($(BR2_PACKAGE_RPM_BZIP2_PAYLOADS),y)

Signature · 6 lines

1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Romain Naour

author

2015-01-18 18:57 UTC #3 lore raw

PATCH 2/3 package/rpm: use external xz library

Hi Yann, all

Le 18/01/2015 15:59, Yann E. MORIN a ?crit :

Quoted 8 lines

Romain, All,
On 2015-01-18 12:34 +0100, Romain Naour spake thusly:
Quoted 2 lines
Fixes:
http://autobuild.buildroot.net/results/2c6/2c61562008418ff58fa3085a3342b0e304923073/
Weird, this build error is about being unable to link with libintl.so (indeed while building the xz backend, but still about libintl).

I seems that the internal xz backend is statically linked and needs libintl library when BR2_NEEDS_GETTEXT_IF_LOCALE is set. But since it's a shared only build, there is no static libintl available in the sysroot.

Quoted 3 lines

Igues I could use that autobuild failure to test your patch 1/3, though...

Yes.

Quoted 9 lines

Otherwise, I have some comments, see below...
Quoted 6 lines
Signed-off-by Romain Naour <[email protected]>
---
 package/rpm/Config.in | 1 +
 package/rpm/rpm.mk    | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

Quoted 13 lines

package/rpm/Config.in config BR2_PACKAGE_RPM_BZIP2_PAYLOADS · line 34
Quoted 9 lines
package/rpm/Config.in +1 -0
index 5ec4167..fe7a38a 100644
@@ -31,6 +31,7 @@ config BR2_PACKAGE_RPM_BZIP2_PAYLOADS
 config BR2_PACKAGE_RPM_XZ_PAYLOADS
 	bool "support for xz payloads"
 	depends on BR2_INSTALL_LIBSTDCPP
+	select BR2_PACKAGE_XZ
I know you are fixing existing code, but we usually do not have such sub-options, and we instead rely entirely on the dependant package in the .mk, without adding such sub-options...

OK, I will update this series and remove BR2_PACKAGE_RPM_BZIP2_PAYLOADS and BR2_PACKAGE_RPM_XZ_PAYLOADS in Config.in, then modify BR2_PACKAGE_RPM_XZ_PAYLOADS by BR2_PACKAGE_XZ (the same for bzip2) in the .mk

By the way, what's the use case for rpm and opkg for a rootfs build with Buildroot ?

Best regards,
Romain

Quoted 34 lines

Quoted 9 lines
package/rpm/Config.in +0 -0
 	help
 	  Support for xz payloads in RPM.
 
package/rpm/rpm.mk +0 -0
index 74f5b88..ce7189e 100644
package/rpm/rpm.mk RPM_CONF_OPTS += --with-file=no · line 51
Quoted 8 lines
package/rpm/rpm.mk +4 -1
@@ -45,7 +45,10 @@ RPM_CONF_OPTS += --with-file=no
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_XZ_PAYLOADS),y)
-RPM_CONF_OPTS += --with-xz
+RPM_CONF_OPTS += --with-xz=external
+RPM_DEPENDENCIES += xz
+else
+RPM_CONF_OPTS += --with-xz=none
Reviewed-by "Yann E. MORIN" <[email protected]>
Regards,
Yann E. MORIN.
Quoted 10 lines
package/rpm/rpm.mk +0 -0
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_BZIP2_PAYLOADS),y)
Signature · 6 lines
1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Romain Naour

author

2015-01-18 11:35 UTC #4 lore raw

PATCH 3/3 package/rpm: use 'none' for location path

In reply to Romain Naour

The recognized keywords are:
(location path: "external:internal:none")

Signed-off-by Romain Naour <[email protected]>

---
 package/rpm/rpm.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

package/rpm/rpm.mk +2 -2

index ce7189e..6725ca4 100644
@@ -34,14 +34,14 @@ ifeq ($(BR2_PACKAGE_PCRE),y)
 RPM_DEPENDENCIES += pcre
 RPM_CONF_OPTS += --with-pcre=external
 else
-RPM_CONF_OPTS += --with-pcre=no
+RPM_CONF_OPTS += --with-pcre=none
 endif
 
 ifeq ($(BR2_PACKAGE_FILE),y)
 RPM_DEPENDENCIES += file
 RPM_CONF_OPTS += --with-file=external
 else
-RPM_CONF_OPTS += --with-file=no
+RPM_CONF_OPTS += --with-file=none
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_XZ_PAYLOADS),y)

Signature · 1 line

1.9.3

Yann E. MORIN

2015-01-18 15:01 UTC #5 lore raw

PATCH 3/3 package/rpm: use 'none' for location path

Romain, All,

On 2015-01-18 12:35 +0100, Romain Naour spake thusly:

Quoted 4 lines

The recognized keywords are:
(location path: "external:internal:none")
Signed-off-by Romain Naour <[email protected]>

Reviewed-by "Yann E. MORIN" <[email protected]>

However, some comments below...

package/rpm/rpm.mk ifeq ($(BR2_PACKAGE_PCRE),y)

Quoted 9 lines

---
 package/rpm/rpm.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

package/rpm/rpm.mk +0 -0

index ce7189e..6725ca4 100644
@@ -34,14 +34,14 @@ ifeq ($(BR2_PACKAGE_PCRE),y)

... here ...

package/rpm/rpm.mk ifeq ($(BR2_PACKAGE_PCRE),y) · line 40

Quoted 8 lines

package/rpm/rpm.mk +1 -1

 RPM_DEPENDENCIES += pcre
 RPM_CONF_OPTS += --with-pcre=external
 else
-RPM_CONF_OPTS += --with-pcre=no
+RPM_CONF_OPTS += --with-pcre=none
 endif
 
 ifeq ($(BR2_PACKAGE_FILE),y)

... and here, we do rely on the dependant packages, without adding sub-options to rpm to select them. I think we should do the same for bzip2 and xz.

Regards,
Yann E. MORIN.

Quoted 15 lines

package/rpm/rpm.mk +1 -1

 RPM_DEPENDENCIES += file
 RPM_CONF_OPTS += --with-file=external
 else
-RPM_CONF_OPTS += --with-file=no
+RPM_CONF_OPTS += --with-file=none
 endif
 
 ifeq ($(BR2_PACKAGE_RPM_XZ_PAYLOADS),y)

Signature · 6 lines

1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

2015-01-18 14:21 UTC #6 lore raw

PATCH 1/3 package/rpm: add gettext dependency

In reply to Romain Naour

Romain, All,

On 2015-01-18 12:34 +0100, Romain Naour spake thusly:

Quoted 6 lines

rpm check for libintl library if BR2_NEEDS_GETTEXT_IF_LOCALE is set.
checking for GNU gettext in libintl... yes
checking whether to use NLS... yes
checking where the gettext function comes from... external libintl checking how to link with libintl... -lintl

Do you ave a defconfig with which I could test, please?

Quoted 4 lines

But the dependency on gettext package is missing to ensures reproducible builds.
Also, sort selected packages alphabetically in Config.in

I would prefer we do not mix feature changes with eye-candy, it makes reviewing a bit more complicated than it should be. Next time, could you just do that in a different patch, please?

Regards,
Yann E. MORIN.

Quoted 49 lines

Signed-off-by Romain Naour <[email protected]>

---
 package/rpm/Config.in | 7 ++++---
 package/rpm/rpm.mk    | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

package/rpm/Config.in +4 -3

index 620eb1a..5ec4167 100644
@@ -7,12 +7,13 @@ config BR2_PACKAGE_RPM
 	depends on BR2_TOOLCHAIN_HAS_THREADS # beecrypt
 	depends on BR2_USE_MMU # fork()
 	select BR2_PACKAGE_BEECRYPT
-	select BR2_PACKAGE_POPT
-	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT_IF_LOCALE
 	select BR2_PACKAGE_NEON
+	select BR2_PACKAGE_NEON_SSL
 	select BR2_PACKAGE_NEON_XML
 	select BR2_PACKAGE_NEON_ZLIB
-	select BR2_PACKAGE_NEON_SSL
+	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_POPT
 	select BR2_PACKAGE_ZLIB
 	help
 	  The RPM package management system.

package/rpm/rpm.mk +4 -0

index c4215b5..74f5b88 100644
@@ -26,6 +26,10 @@ RPM_CONF_OPTS = \
 	--with-libbeecrypt=$(STAGING_DIR) \
 	--with-popt=$(STAGING_DIR)
 
+ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
+RPM_DEPENDENCIES += gettext
+endif
+
 ifeq ($(BR2_PACKAGE_PCRE),y)
 RPM_DEPENDENCIES += pcre
 RPM_CONF_OPTS += --with-pcre=external

Signature · 6 lines

1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Romain Naour

author

2015-01-18 18:57 UTC #7 lore raw

PATCH 1/3 package/rpm: add gettext dependency

Hi Yann, all

Le 18/01/2015 15:21, Yann E. MORIN a ?crit :

Quoted 11 lines

Romain, All,
On 2015-01-18 12:34 +0100, Romain Naour spake thusly:
Quoted 6 lines
rpm check for libintl library if BR2_NEEDS_GETTEXT_IF_LOCALE is set.
checking for GNU gettext in libintl... yes
checking whether to use NLS... yes
checking where the gettext function comes from... external libintl checking how to link with libintl... -lintl
Do you ave a defconfig with which I could test, please?

Yes, see:
http://autobuild.buildroot.net/results/2c6/2c61562008418ff58fa3085a3342b0e304923073/config

I'm fixing a autobuild failure, but while doing 'make rpm' the failure didn't happened. So I make a diff with my build.log and the build-end.log provided by the autobuilder. That is how I found the missing dependency on gettext.

Quoted 9 lines

Quoted 5 lines
But the dependency on gettext package is missing to ensures reproducible builds.
Also, sort selected packages alphabetically in Config.in
I would prefer we do not mix feature changes with eye-candy, it makes reviewing a bit more complicated than it should be. Next time, could you just do that in a different patch, please?

Yes, sorry.
While rebasing to clean up this series, I wasn't happy that the select BR2_PACKAGE_* wasn't alphabetically sorted.
I'll do that in a other patch.

Thanks for your review :)

Best regards,
Romain

Quoted 54 lines

Regards,
Yann E. MORIN.

Quoted 49 lines

Signed-off-by Romain Naour <[email protected]>

---
 package/rpm/Config.in | 7 ++++---
 package/rpm/rpm.mk    | 4 ++++
 2 files changed, 8 insertions(+), 3 deletions(-)

package/rpm/Config.in +4 -3

index 620eb1a..5ec4167 100644
@@ -7,12 +7,13 @@ config BR2_PACKAGE_RPM
 	depends on BR2_TOOLCHAIN_HAS_THREADS # beecrypt
 	depends on BR2_USE_MMU # fork()
 	select BR2_PACKAGE_BEECRYPT
-	select BR2_PACKAGE_POPT
-	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT_IF_LOCALE
 	select BR2_PACKAGE_NEON
+	select BR2_PACKAGE_NEON_SSL
 	select BR2_PACKAGE_NEON_XML
 	select BR2_PACKAGE_NEON_ZLIB
-	select BR2_PACKAGE_NEON_SSL
+	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_POPT
 	select BR2_PACKAGE_ZLIB
 	help
 	  The RPM package management system.

package/rpm/rpm.mk +4 -0

index c4215b5..74f5b88 100644
@@ -26,6 +26,10 @@ RPM_CONF_OPTS = \
 	--with-libbeecrypt=$(STAGING_DIR) \
 	--with-popt=$(STAGING_DIR)
 
+ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
+RPM_DEPENDENCIES += gettext
+endif
+
 ifeq ($(BR2_PACKAGE_PCRE),y)
 RPM_DEPENDENCIES += pcre
 RPM_CONF_OPTS += --with-pcre=external

Signature · 6 lines

1.9.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot