unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] configure: only install bash completion if supported
@ 2014-02-02 16:47 Jani Nikula
  2014-02-02 17:40 ` Mark Walters
  2014-02-03 20:28 ` David Bremner
  0 siblings, 2 replies; 6+ messages in thread
From: Jani Nikula @ 2014-02-02 16:47 UTC (permalink / raw)
  To: notmuch

Our bash completion depends on bash-completion 1.90 or later. Only
install where available.
---
 configure | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/configure b/configure
index 13b6062..66aaedb 100755
--- a/configure
+++ b/configure
@@ -360,6 +360,14 @@ else
     have_valgrind=0
 fi
 
+printf "Checking for bash-completion (>= 1.90)... "
+if pkg-config --atleast-version=1.90 bash-completion; then
+    printf "Yes.\n"
+else
+    printf "No (will not install bash completion).\n"
+    WITH_BASH=0
+fi
+
 if [ -z "${EMACSLISPDIR}" ]; then
     if pkg-config --exists emacs; then
 	EMACSLISPDIR=$(pkg-config emacs --variable sitepkglispdir)
-- 
1.8.5.2

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

* Re: [PATCH] configure: only install bash completion if supported
  2014-02-02 16:47 [PATCH] configure: only install bash completion if supported Jani Nikula
@ 2014-02-02 17:40 ` Mark Walters
  2014-02-02 20:59   ` David Bremner
  2014-02-03 20:28 ` David Bremner
  1 sibling, 1 reply; 6+ messages in thread
From: Mark Walters @ 2014-02-02 17:40 UTC (permalink / raw)
  To: Jani Nikula, notmuch


This LGTM (untested)

I think this is the right way to do this: I don't think we should
spam a user with error messages for attempting tab completion with
an older version of bash-completion (which I think you said was the case
with the other version)

Best wishes

Mark


On Sun, 02 Feb 2014, Jani Nikula <jani@nikula.org> wrote:
> Our bash completion depends on bash-completion 1.90 or later. Only
> install where available.
> ---
>  configure | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/configure b/configure
> index 13b6062..66aaedb 100755
> --- a/configure
> +++ b/configure
> @@ -360,6 +360,14 @@ else
>      have_valgrind=0
>  fi
>  
> +printf "Checking for bash-completion (>= 1.90)... "
> +if pkg-config --atleast-version=1.90 bash-completion; then
> +    printf "Yes.\n"
> +else
> +    printf "No (will not install bash completion).\n"
> +    WITH_BASH=0
> +fi
> +
>  if [ -z "${EMACSLISPDIR}" ]; then
>      if pkg-config --exists emacs; then
>  	EMACSLISPDIR=$(pkg-config emacs --variable sitepkglispdir)
> -- 
> 1.8.5.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH] configure: only install bash completion if supported
  2014-02-02 17:40 ` Mark Walters
@ 2014-02-02 20:59   ` David Bremner
  2014-02-02 21:19     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2014-02-02 20:59 UTC (permalink / raw)
  To: Mark Walters, Jani Nikula, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> This LGTM (untested)

I did test it, at least completely removing bash completion works as
expected.  

Unfortunately --with-bash-completion does not override this test because
the order things are processed. Do you think this is a bug?  I wondered
if users that "know what they are doing" (TM) might want to force
installation even though the pkg-config test fails.

d

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

* Re: [PATCH] configure: only install bash completion if supported
  2014-02-02 20:59   ` David Bremner
@ 2014-02-02 21:19     ` Jani Nikula
  2014-02-02 23:18       ` David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2014-02-02 21:19 UTC (permalink / raw)
  To: David Bremner, Mark Walters, notmuch

On Sun, 02 Feb 2014, David Bremner <david@tethera.net> wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> This LGTM (untested)
>
> I did test it, at least completely removing bash completion works as
> expected.  
>
> Unfortunately --with-bash-completion does not override this test because
> the order things are processed. Do you think this is a bug?  I wondered
> if users that "know what they are doing" (TM) might want to force
> installation even though the pkg-config test fails.

I thought it was a feature, not a bug, but I'm fine either way.

BR,
Jani.

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

* Re: [PATCH] configure: only install bash completion if supported
  2014-02-02 21:19     ` Jani Nikula
@ 2014-02-02 23:18       ` David Bremner
  0 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2014-02-02 23:18 UTC (permalink / raw)
  To: Jani Nikula, Mark Walters, notmuch

Jani Nikula <jani@nikula.org> writes:

> On Sun, 02 Feb 2014, David Bremner <david@tethera.net> wrote:
>> Mark Walters <markwalters1009@gmail.com> writes:
>>
>>> This LGTM (untested)
>>
>> I did test it, at least completely removing bash completion works as
>> expected.  
>>
>> Unfortunately --with-bash-completion does not override this test because
>> the order things are processed. Do you think this is a bug?  I wondered
>> if users that "know what they are doing" (TM) might want to force
>> installation even though the pkg-config test fails.
>
> I thought it was a feature, not a bug, but I'm fine either way.

I can live with the current patch. As you pointed out on IRC, this is
the usual way missing dependencies work.  And I think we should avoid
extra complications in the configure script when we can.

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

* Re: [PATCH] configure: only install bash completion if supported
  2014-02-02 16:47 [PATCH] configure: only install bash completion if supported Jani Nikula
  2014-02-02 17:40 ` Mark Walters
@ 2014-02-03 20:28 ` David Bremner
  1 sibling, 0 replies; 6+ messages in thread
From: David Bremner @ 2014-02-03 20:28 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> Our bash completion depends on bash-completion 1.90 or later. Only
> install where available.

pushed.

d

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

end of thread, other threads:[~2014-02-03 20:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-02 16:47 [PATCH] configure: only install bash completion if supported Jani Nikula
2014-02-02 17:40 ` Mark Walters
2014-02-02 20:59   ` David Bremner
2014-02-02 21:19     ` Jani Nikula
2014-02-02 23:18       ` David Bremner
2014-02-03 20:28 ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).