Skip to contentSkip to conversation

package/tealdeer: exclude unsupported targets

PATCH In review Danilo Bargen 4 participants 7 messages Dec 8, 2022 – Dec 21, 2022

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 package/tealdeer: exclude unsupported targets

Not all target architectures are supported by the "ring" dependency.

---
 package/tealdeer/Config.in | 8 ++++++++
 1 file changed, 8 insertions(+)
package/tealdeer/Config.in +1 -0
index 96ed81614b..a61a59f3ee 100644
@@ -1,6 +1,14 @@
 config BR2_PACKAGE_TEALDEER
 	bool "tealdeer"
 	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
+    # Crypto dependency (ring) not available for mips:

https://github.com/briansmith/ring/issues/562
+ depends on !(BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el)
+ # Crypto dependency (ring) not available for PowerPC:
https://github.com/briansmith/ring/issues/389
+ depends on !(BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le)
+ # Crypto dependency (ring) not available for Sparc:
https://github.com/briansmith/ring/issues/1512
+ depends on !(BR2_sparc || BR2_sparc64)
+ # Crypto dependency (ring) not available for s390x:
https://github.com/briansmith/ring/commit/4d2e1a8fb80398f7c3c0137ea8fdd512f82d37a0
+ depends on !BR2_s390x
select BR2_PACKAGE_HOST_RUSTC
help
A fast and full-featured tldr client. tldr pages are
--
2.38.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#1 lore raw

Hello Danilo,

Quoted 3 lines · On Thu, 8 Dec 2022 11:02:47 +0100 Danilo Bargen <[email protected]> wrote:

Not all target architectures are supported by the "ring" dependency.

Thanks for working on this. Your patch is badly line-wrapped by your e-mail client. You can disable wrapping in Claws Mail, or better you can use "git send-email". See below for another comment.

Quoted 18 lines
package/tealdeer/Config.in +1 -0
index 96ed81614b..a61a59f3ee 100644
@@ -1,6 +1,14 @@
 config BR2_PACKAGE_TEALDEER
 	bool "tealdeer"
 	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
+    # Crypto dependency (ring) not available for mips:

https://github.com/briansmith/ring/issues/562

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el)
+    # Crypto dependency (ring) not available for PowerPC:

https://github.com/briansmith/ring/issues/389

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le)
+    # Crypto dependency (ring) not available for Sparc:

https://github.com/briansmith/ring/issues/1512

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_sparc || BR2_sparc64)
+    # Crypto dependency (ring) not available for s390x:
Quoted 2 lines

https://github.com/briansmith/ring/commit/4d2e1a8fb80398f7c3c0137ea8fdd512f82d37a0
+ depends on !BR2_s390x

We normally handle this kind of situation using a BR2_PACKAGE_TEALDEER_ARCH_SUPPORTS variable, like this:

config BR2_PACKAGE_TEALDEER_ARCH_SUPPORTS
bool
default y if ...
depends on ...

config BR2_PACKAGE_TEALDEER
bool "tealdeer"
depends on BR2_PACKAGE_TEALDEER_ARCH_SUPPORTS

Also, in this kind of case, we prefer to use "positive" logic: instead of excluding the architectures that are not supported, it is better to list the ones that are supported.

Side note: overall, if you take a step back, it is absolutely mind-blowing that a simple application showing tldr pages has a dependency on architecture-specific things. There is really no sane reason to have such dependencies, but apparently the Rust ecosystem is not completely sane... :-/

Thomas

Signature · 7 lines
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#2 lore raw

Hello Thomas

Quoted 3 lines

Thanks for working on this. Your patch is badly line-wrapped by your e-mail client. You can disable wrapping in Claws Mail, or better you can use "git send-email". See below for another comment.

Oops, I do use Claws Mail, and used "Edit with external editor" for pasting the patch. I didn't notice that this doesn't disable auto-wrapping.

Quoted 5 lines

Side note: overall, if you take a step back, it is absolutely mind-blowing that a simple application showing tldr pages has a dependency on architecture-specific things. There is really no sane reason to have such dependencies, but apparently the Rust ecosystem is not completely sane... :-/

The reason is that Tealdeer uses https://github.com/rustls/rustls for TLS, which depends on the "ring" library https://github.com/briansmith/ring/. To quote from its README:

Quoted 3 lines

ring is focused on the implementation, testing, and optimization of a core set of cryptographic operations exposed via an easy-to-use (and hard-to-misuse) API. ring exposes a Rust API and is written in a hybrid of Rust, C, and assembly language.
...
Most of the C and assembly language code in ring comes from BoringSSL, and BoringSSL is derived from OpenSSL. ring merges changes from BoringSSL regularly. Also, several changes that were developed for ring have been contributed to and integrated into BoringSSL.

Due to the reliance on BoringSSL and assembly code for some parts of the project, it is not as portable as pure-Rust code. Support for MIPS, PowerPC and other architectures is in the works, but not yet finished.

Previously tealdeer used OpenSSL, but quite a few of the users had trouble with broken OpenSSL installations. Plus, it made cross-compilation and static builds much harder. Using rustls makes this much easier on the common platforms, but of course, this has certain limitations with regards to more obscure target architectures.

If you would rather remove tealdeer from Buildroot (because the config is getting too complex), that would be OK for me. Otherwise, I'd update my patch based on your suggestions.

Danilo
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#3 lore raw

Danilo, All,

On 2022-12-08 16:48 +0100, Danilo Bargen spake thusly:

Quoted 8 lines
Quoted 5 lines

Side note: overall, if you take a step back, it is absolutely mind-blowing that a simple application showing tldr pages has a dependency on architecture-specific things. There is really no sane reason to have such dependencies, but apparently the Rust ecosystem is not completely sane... :-/

The reason is that Tealdeer uses https://github.com/rustls/rustls for TLS, which depends on the "ring" library

[--SNIP--]

Quoted 5 lines

Previously tealdeer used OpenSSL, but quite a few of the users had trouble with broken OpenSSL installations. Plus, it made cross-compilation and static builds much harder. Using rustls makes this much easier on the common platforms, but of course, this has certain limitations with regards to more obscure target architectures.

cross-compilation with openssl is very easu. It just works.

That static build is harder I consider that a benefit rather than a deficiency. ;-]

Quoted 3 lines

If you would rather remove tealdeer from Buildroot (because the config is getting too complex), that would be OK for me. Otherwise, I'd update my patch based on your suggestions.

I think what Thomas meant, was that it is a pity that we need a TLS implementation just to read the equivalent of man-pages, which by virtue of being local oalways work, and that that TLS implementation has limitations on architectures, which man-pages do not...

Regards,
Yann E. MORIN.

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#4 lore raw

Hello Yann

Thanks for adapting and merging the patch!

Quoted 4 lines

I think what Thomas meant, was that it is a pity that we need a TLS implementation just to read the equivalent of man-pages, which by virtue of being local oalways work, and that that TLS implementation has limitations on architectures, which man-pages do not...

Yes, I understand that :) The only reason is that the current tldr pages are fetched from the domain "tldr.sh" via HTTPS. An alternative to this would be to add a build flag to Tealdeer to not require/support network connectivity at all, and to bundle all current tldr pages with the buildroot package. This way, no network connectivity and no TLS would be needed. This feature was requested by a user a few years ago, and I already implemented it, but never got feedback on the implementation and nobody ever requested that feature again. So in the interest of keeping the codebase simple, I never merged the branch.

Anyways, I brought back a build flag to allow linking against OpenSSL instead: https://github.com/dbrgn/tealdeer/pull/303 Initially I had quite some trouble getting it to find OpenSSL, until I discovered that I need to add a dependency on host-pkgconf. Once the next release is out, I can submit a patch to build against OpenSSL instead of Rustls.

Cheers,
Danilo
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#5 lore raw
In reply to Danilo Bargen

Danilo, All,

On 2022-12-08 11:02 +0100, Danilo Bargen spake thusly:

Quoted 25 lines

Not all target architectures are supported by the "ring" dependency.

---
 package/tealdeer/Config.in | 8 ++++++++
 1 file changed, 8 insertions(+)
package/tealdeer/Config.in +1 -0
index 96ed81614b..a61a59f3ee 100644
@@ -1,6 +1,14 @@
 config BR2_PACKAGE_TEALDEER
 	bool "tealdeer"
 	depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
+    # Crypto dependency (ring) not available for mips:

https://github.com/briansmith/ring/issues/562

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el)
+    # Crypto dependency (ring) not available for PowerPC:

https://github.com/briansmith/ring/issues/389

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le)
+    # Crypto dependency (ring) not available for Sparc:

https://github.com/briansmith/ring/issues/1512

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_sparc || BR2_sparc64)
+    # Crypto dependency (ring) not available for s390x:
Quoted 2 lines

https://github.com/briansmith/ring/commit/4d2e1a8fb80398f7c3c0137ea8fdd512f82d37a0
+ depends on !BR2_s390x

As Thomas already noted, this patch is badly broken.

I've massaged into back into something that looks nicer, and applied to master, after simplifying the comment and moving the references to the commit log.

Regards,
Yann E. MORIN.

Quoted 10 lines
package/tealdeer/Config.in +0 -1
 	select BR2_PACKAGE_HOST_RUSTC
 	help
 	  A fast and full-featured tldr client. tldr pages are
--

2.38.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#6 lore raw
Quoted 34 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Danilo, All,
On 2022-12-08 11:02 +0100, Danilo Bargen spake thusly:

Quoted 25 lines

Not all target architectures are supported by the "ring" dependency.

---
package/tealdeer/Config.in | 8 ++++++++

1 file changed, 8 insertions(+)
package/tealdeer/Config.in +0 -0
index 96ed81614b..a61a59f3ee 100644
@@ -1,6 +1,14 @@

config BR2_PACKAGE_TEALDEER
bool "tealdeer"
depends on BR2_PACKAGE_HOST_RUSTC_TARGET_ARCH_SUPPORTS
+ # Crypto dependency (ring) not available for mips:
https://github.com/briansmith/ring/issues/562

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_mips || BR2_mipsel || BR2_mips64 || BR2_mips64el)
+    # Crypto dependency (ring) not available for PowerPC:

https://github.com/briansmith/ring/issues/389

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le)
+    # Crypto dependency (ring) not available for Sparc:

https://github.com/briansmith/ring/issues/1512

package/tealdeer/Config.in +2 -0
+    depends on !(BR2_sparc || BR2_sparc64)
+    # Crypto dependency (ring) not available for s390x:
Quoted 2 lines

https://github.com/briansmith/ring/commit/4d2e1a8fb80398f7c3c0137ea8fdd512f82d37a0
+ depends on !BR2_s390x

As Thomas already noted, this patch is badly broken.

Quoted 3 lines

I've massaged into back into something that looks nicer, and applied to master, after simplifying the comment and moving the references to the commit log.

Committed to 2022.11.x, thanks.

Signature · 5 lines
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot