Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v8 1/1] mosh: new package
Date: Sun, 26 Jul 2015 19:35:17 +0200	[thread overview]
Message-ID: <20150726193517.40776cc8@free-electrons.com> (raw)
In-Reply-To: <1437928447-26907-1-git-send-email-christian@paral.in>

Christian,

On Sun, 26 Jul 2015 09:34:07 -0700, Christian Stewart wrote:
> Adding mosh, the mobile shell. Mosh uses ssh or dropbear as an initial transport to
> start mosh-server which uses UDP to communicate with the client.
> Supports a predictive model to enhance performance on weak connections
> and compensate for general lag. Also supports disconnections and
> reconnections seamlessly.
> 
> Signed-off-by: Christian Stewart <christian@paral.in>

I applied, with some more changes:

    [Thomas:
      - Add missing dependency on host-pkgconf, as noticed by Yann
        E. Morin.
      - Indicate that openssh/dropbear is a runtime dependency.
      - Pass some variables in the configure environment to tell that SSP
        support is not available when it isn't. Otherwise, it misdetects
        the SSP support as being available, causing a build failure.]

The last item clearly meant that the build was never tested with
uClibc, which is our default C library. Please do at least a test build
with uClibc before submitting new packages.

Also, one comment below.


> diff --git a/package/mosh/0001-remove-system-locale-calls.patch b/package/mosh/0001-remove-system-locale-calls.patch
> new file mode 100644
> index 0000000..114e6ed
> --- /dev/null
> +++ b/package/mosh/0001-remove-system-locale-calls.patch
> @@ -0,0 +1,46 @@
> +The locale command is not available on many systems. As this variable
> +is unused and appears to have been written with the intent of
> +displaying the locale settings to the user, it's not really necessary.
> +As this breaks Mosh on a lot of systems, it's best to remove the calls.
> +
> +Upstream status: refused, see: https://github.com/keithw/mosh/issues/650

I think upstream refused it, because the approach of your patch is not
acceptable upstream.

> +Signed-off-by: Christian Stewart <christian@paral.in>
> +---
> + src/frontend/mosh-server.cc | 4 +++-
> + src/frontend/stmclient.cc   | 4 +++-
> + 2 files changed, 6 insertions(+), 2 deletions(-)
> +
> +diff --git a/src/frontend/mosh-server.cc b/src/frontend/mosh-server.cc
> +index 76ed2ed..0d2f222 100644
> +--- a/src/frontend/mosh-server.cc
> ++++ b/src/frontend/mosh-server.cc
> +@@ -313,7 +313,9 @@ int main( int argc, char *argv[] )
> +       fprintf( stderr, "mosh-server needs a UTF-8 native locale to run.\n\n" );
> +       fprintf( stderr, "Unfortunately, the local environment (%s) specifies\nthe character set \"%s\",\n\n", native_ctype.str().c_str(), native_charset.c_str() );
> +       fprintf( stderr, "The client-supplied environment (%s) specifies\nthe character set \"%s\".\n\n", client_ctype.str().c_str(), client_charset.c_str() );
> +-      int unused __attribute((unused)) = system( "locale" );
> ++
> ++      fprintf( stderr, "This is a buildroot system, 'locale' debug output has been removed." );

Here is what your patch should do instead:

	if (locale command exists)
		int unused __attribute((unused)) = system( "locale" );
	else
		fprintf( stderr, "locale command not found, cannot show related debug output." );

I'm pretty sure this is going to make your patch much more acceptable
upstream. Can you retry with something like that?

Thanks!

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

  parent reply	other threads:[~2015-07-26 17:35 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-26 16:34 [Buildroot] [PATCH v8 1/1] mosh: new package Christian Stewart
2015-07-26 17:01 ` Yann E. MORIN
2015-07-26 17:35 ` Thomas Petazzoni [this message]
2015-07-26 18:02   ` Christian Stewart
2015-07-26 19:41     ` Thomas Petazzoni
2015-07-27 21:38       ` Christian Stewart
2015-07-26 22:20 ` Baruch Siach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150726193517.40776cc8@free-electrons.com \
    --to=thomas.petazzoni@free-electrons.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox