[PATCH] dtdiff: change to POSIX shell

[PATCH] dtdiff: change to POSIX shell

[Mike Frysinger]

From: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

This changes from the bash-specific process substitution feature to
reading with pipes.  It relies on /dev/fd or /proc/self/fd existing.

URL: https://crbug.com/756559
Signed-off-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 dtdiff | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/dtdiff b/dtdiff
index 5fa772b0ab62..4d1b71756c2f 100644
--- a/dtdiff
+++ b/dtdiff
@@ -1,8 +1,4 @@
-#! /bin/bash
-
-# This script uses the bash <(...) extension.
-# If you want to change this to work with a generic /bin/sh, make sure
-# you fix that.
+#! /bin/sh
 
 
 DTC=dtc
@@ -35,4 +31,15 @@ if [ $# != 2 ]; then
     exit 1
 fi
 
-diff -u <(source_and_sort "$1") <(source_and_sort "$2")
+for dir in /dev/fd /proc/self/fd; do
+    if [ -d "${dir}" ]; then
+        break
+    fi
+done
+
+source_and_sort "$1" | (
+    # Duplicate current stdin from the first file to fd 3 so we can change fd 0
+    # to the second file.
+    exec 3<&0
+    source_and_sort "$2" | diff -u "${dir}/3" "${dir}/0"
+)
-- 
2.15.1

Re: [PATCH] dtdiff: change to POSIX shell

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

On Wed, Jan 10, 2018 at 12:47:56AM -0500, Mike Frysinger wrote:
> From: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> This changes from the bash-specific process substitution feature to
> reading with pipes.  It relies on /dev/fd or /proc/self/fd existing.
> 
> URL: https://crbug.com/756559
> Signed-off-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Yeah, sorry, replacing a dependency on a complex but widely available
shell with a much more cryptic dependency on a Linux specific feature
does not seem like a win to me.

> ---
>  dtdiff | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/dtdiff b/dtdiff
> index 5fa772b0ab62..4d1b71756c2f 100644
> --- a/dtdiff
> +++ b/dtdiff
> @@ -1,8 +1,4 @@
> -#! /bin/bash
> -
> -# This script uses the bash <(...) extension.
> -# If you want to change this to work with a generic /bin/sh, make sure
> -# you fix that.
> +#! /bin/sh
>  
>  
>  DTC=dtc
> @@ -35,4 +31,15 @@ if [ $# != 2 ]; then
>      exit 1
>  fi
>  
> -diff -u <(source_and_sort "$1") <(source_and_sort "$2")
> +for dir in /dev/fd /proc/self/fd; do
> +    if [ -d "${dir}" ]; then
> +        break
> +    fi
> +done
> +
> +source_and_sort "$1" | (
> +    # Duplicate current stdin from the first file to fd 3 so we can change fd 0
> +    # to the second file.
> +    exec 3<&0
> +    source_and_sort "$2" | diff -u "${dir}/3" "${dir}/0"
> +)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] dtdiff: change to POSIX shell

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On 10 Jan 2018 16:57, David Gibson wrote:
> On Wed, Jan 10, 2018 at 12:47:56AM -0500, Mike Frysinger wrote:
> > This changes from the bash-specific process substitution feature to
> > reading with pipes.  It relies on /dev/fd or /proc/self/fd existing.
> > 
> > URL: https://crbug.com/756559
> > Signed-off-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Yeah, sorry, replacing a dependency on a complex but widely available
> shell with a much more cryptic dependency on a Linux specific feature
> does not seem like a win to me.

bash's <(...) only works when the host supports either /dev/fd or /proc/self/fd,
so i don't think this is less portable.  /dev/fd also isn't Linux-specific.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] dtdiff: change to POSIX shell

[Kyle Evans]

On Thu, Feb 1, 2018 at 10:14 PM, Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org> wrote:
> On 10 Jan 2018 16:57, David Gibson wrote:
>> On Wed, Jan 10, 2018 at 12:47:56AM -0500, Mike Frysinger wrote:
>> > This changes from the bash-specific process substitution feature to
>> > reading with pipes.  It relies on /dev/fd or /proc/self/fd existing.
>> >
>> > URL: https://crbug.com/756559
>> > Signed-off-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> Yeah, sorry, replacing a dependency on a complex but widely available
>> shell with a much more cryptic dependency on a Linux specific feature
>> does not seem like a win to me.
>
> bash's <(...) only works when the host supports either /dev/fd or /proc/self/fd,
> so i don't think this is less portable.  /dev/fd also isn't Linux-specific.
> -mike

For what it's worth, this isn't completely true. Bash can also use
FIFOs for process substitution [1]. FreeBSD does not use fdescfs by
default, but it can be mounted, and using fdescfs for process
substitution is provided as an optional feature in shells/bash. It
would be nice to not grow a dependency on fdescfs for dtc. =)

[1] https://www.gnu.org/software/bash/manual/html_node/Process-Substitution.html

Re: [PATCH] dtdiff: change to POSIX shell

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

On 01 Feb 2018 22:35, Kyle Evans wrote:
> On Thu, Feb 1, 2018 at 10:14 PM, Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org> wrote:
> > On 10 Jan 2018 16:57, David Gibson wrote:
> >> On Wed, Jan 10, 2018 at 12:47:56AM -0500, Mike Frysinger wrote:
> >> > This changes from the bash-specific process substitution feature to
> >> > reading with pipes.  It relies on /dev/fd or /proc/self/fd existing.
> >> >
> >> > URL: https://crbug.com/756559
> >> > Signed-off-by: Mike Frysinger <vapier-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >>
> >> Yeah, sorry, replacing a dependency on a complex but widely available
> >> shell with a much more cryptic dependency on a Linux specific feature
> >> does not seem like a win to me.
> >
> > bash's <(...) only works when the host supports either /dev/fd or /proc/self/fd,
> > so i don't think this is less portable.  /dev/fd also isn't Linux-specific.
> 
> For what it's worth, this isn't completely true. Bash can also use
> FIFOs for process substitution [1]. FreeBSD does not use fdescfs by
> default, but it can be mounted, and using fdescfs for process
> substitution is provided as an optional feature in shells/bash. It
> would be nice to not grow a dependency on fdescfs for dtc. =)
> 
> [1] https://www.gnu.org/software/bash/manual/html_node/Process-Substitution.html

sorry, you're right.  i didn't fully trace the bash source.  that man page
matches the current sources in that either named fifos or /dev/fd are needed.

Simon suggested i just use tempfiles myself.  i had avoided them because the
current method makes it easy to avoid leaks (for whatever reason), but if it
isn't a big deal, that'd be an easy fix.
-mike

TMP1= TMP2=
trap 'rm -f "${TMP1}" "${TMP2}"' EXIT
TMP1=$(mktemp)
TMP2=$(mktemp)

source_and_sort "$1" "${TMP1}"
source_and_sort "$2" "${TMP2}"
diff -u "${TMP1}" "${TMP2}"

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]