[PATCH] configure: resolve real path to bash

[PATCH] configure: resolve real path to bash

[Đoàn Trần Công Danh]

The old code somehow resolves to `bin/sh' on Arch Linux/Void Linux
auto build systems.

cf:
https://travis-ci.org/void-linux/void-packages/jobs/470139880#L1691

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index b2200be0..84a74b17 100755
--- a/configure
+++ b/configure
@@ -563,6 +563,7 @@ printf "Checking for bash... "
 if command -v ${BASH} > /dev/null; then
     have_bash=1
     bash_absolute=$(command -v ${BASH})
+    bash_absolute=$(readlink -f "$bash_absolute")
     printf "Yes (%s).\n" "$bash_absolute"
 else
     have_bash=0
-- 
2.20.1.352.g40155ab247

Re: [PATCH] configure: resolve real path to bash

[David Bremner]

Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:

> The old code somehow resolves to `bin/sh' on Arch Linux/Void Linux
> auto build systems.
>

I'm not sure if this is better or worse than

    https://nmbug.notmuchmail.org/nmweb/show/20190117021132.28327-1-david%40tethera.net

I welcome input on this.

One issue is that readlink(1) is not in POSIX, so we can expect some
portability pains.

d

Re: [PATCH] configure: resolve real path to bash

[Tomi Ollila]

On Wed, Jan 30 2019, David Bremner wrote:

> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
>
>> The old code somehow resolves to `bin/sh' on Arch Linux/Void Linux
>> auto build systems.
>>
>
> I'm not sure if this is better or worse than
>
>     https://nmbug.notmuchmail.org/nmweb/show/20190117021132.28327-1-david%40tethera.net
>
> I welcome input on this.

I'd choose your version. readlink(1) smells somewhat unportable (namual
page does mention anything else than it is part of gnu coreutils...

Tomi

>
> One issue is that readlink(1) is not in POSIX, so we can expect some
> portability pains.
>
> d
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] configure: resolve real path to bash

[Danh Doan]

David Bremner <david@tethera.net> writes:

> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
>
>> The old code somehow resolves to `bin/sh' on Arch Linux/Void Linux
>> auto build systems.
>>
>
> I'm not sure if this is better or worse than
>
>     https://nmbug.notmuchmail.org/nmweb/show/20190117021132.28327-1-david%40tethera.net

Sorry for the noise, I somehow couldn't find your patch at that time.
And I only recognized readlink(1) is not POSIX after sending the patch.

I wonder if it's better to keep `/usr/bin/env bash` instead of resolving
bash to specific file. Something like this:

diff --git a/emacs/Makefile.local b/emacs/Makefile.local
index 04913a06..2252e818 100644
--- a/emacs/Makefile.local
+++ b/emacs/Makefile.local
@@ -117,7 +117,6 @@ endif
 	install -m0644 $(emacs_images) "$(DESTDIR)$(emacsetcdir)"
 	mkdir -p "$(DESTDIR)$(prefix)/bin/"
 ifeq ($(HAVE_BASH),1)
-	sed "1s|^#!.*|#! $(BASH_ABSOLUTE)|" < $(emacs_mua) > $(DESTDIR)$(prefix)/bin/notmuch-emacs-mua
 	chmod 755 $(DESTDIR)$(prefix)/bin/notmuch-emacs-mua
 endif
 ifeq ($(WITH_DESKTOP),1)


>
> I welcome input on this.
>
> One issue is that readlink(1) is not in POSIX, so we can expect some
> portability pains.
>
> d

-- 
Danh

Re: [PATCH] configure: resolve real path to bash

[David Bremner]

Danh Doan <congdanhqx@gmail.com> writes:

> David Bremner <david@tethera.net> writes:
>
>> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
>>
>>> The old code somehow resolves to `bin/sh' on Arch Linux/Void Linux
>>> auto build systems.
>>>
>>
>> I'm not sure if this is better or worse than
>>
>>     https://nmbug.notmuchmail.org/nmweb/show/20190117021132.28327-1-david%40tethera.net
>
> Sorry for the noise, I somehow couldn't find your patch at that time.
> And I only recognized readlink(1) is not POSIX after sending the patch.
>
> I wonder if it's better to keep `/usr/bin/env bash` instead of resolving
> bash to specific file. Something like this:
>

This will require Debian (and I think Fedora) to do the resolution
themselves, because they don't support "#! /usr/bin/env interpreter"
Of course the distro specific patches are slightly less complicated,
since they can hard code the location of bash.

d

Re: [PATCH] configure: resolve real path to bash

[Danh Doan]

David Bremner <david@tethera.net> writes:

> Danh Doan <congdanhqx@gmail.com> writes:
>
>>
>> I wonder if it's better to keep `/usr/bin/env bash` instead of resolving
>> bash to specific file. Something like this:
>>
>
> This will require Debian (and I think Fedora) to do the resolution
> themselves, because they don't support "#! /usr/bin/env interpreter"
> Of course the distro specific patches are slightly less complicated,
> since they can hard code the location of bash.

I have nothing against your approach,
but I still slightly prefer to be able to run `configure` script without
setting environment variable.

Is it acceptable to emulate `realpath` in `configure` script?
If yes, we could borrow this script (MIT licensed).

https://github.com/chriskempson/base16-shell/blob/master/realpath/realpath.sh

-- 
Danh

Re: [PATCH] configure: resolve real path to bash

[David Bremner]

Danh Doan <congdanhqx@gmail.com> writes:

> David Bremner <david@tethera.net> writes:
>
>> Danh Doan <congdanhqx@gmail.com> writes:
>>
>>>
>>> I wonder if it's better to keep `/usr/bin/env bash` instead of resolving
>>> bash to specific file. Something like this:
>>>
>>
>> This will require Debian (and I think Fedora) to do the resolution
>> themselves, because they don't support "#! /usr/bin/env interpreter"
>> Of course the distro specific patches are slightly less complicated,
>> since they can hard code the location of bash.
>
> I have nothing against your approach,
> but I still slightly prefer to be able to run `configure` script without
> setting environment variable.
>

Have you verified that you need to set an evironment variable? If there
is only one bash in $PATH, command -v should work fine. The previous bug
was using the variable BASH, which bash itself sets to /bin/sh (which I
find odd, but nobody asked me).  I only have the environment variable
setting in the Debian packaging to work around a (since resolved)
misconfiguration [1] of the debian autobuilders.

> Is it acceptable to emulate `realpath` in `configure` script?
> If yes, we could borrow this script (MIT licensed).
>
> https://github.com/chriskempson/base16-shell/blob/master/realpath/realpath.sh
>

It starts to seem a bit overcomplicated at that point.

[1]: The autobuilders had /bin as a link to /usr/bin, but the resulting
binary packages were installed on systems without that link. Hilarity
ensued.

Re: [PATCH] configure: resolve real path to bash

[Danh Doan]

David Bremner <david@tethera.net> writes:

> Have you verified that you need to set an evironment variable? If there
> is only one bash in $PATH, command -v should work fine. The previous bug
> was using the variable BASH, which bash itself sets to /bin/sh (which I
> find odd, but nobody asked me).

I hadn't tested at that time since I had taken a quick skim over your
second patch.
I've just tested your patch, I confirm your patch is good with Void XBPS
autobuild system.

-- 
Danh