Re: [gentoo-dev] [PATCH v2 1/1] esed.eclass: new eclass

[Oskari Pirhonen <xxc3ncoredxx@gmail.com>]

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

On Fri, Jun 03, 2022 at 07:36:46AM -0400, Ionen Wolkens wrote:
> ... snip ...
>
> +	# Roughly attempt to find files in arguments by checking if it's a
> +	# readable file (aka s/// is not a file) and does not start with -
> +	# (unless after --), then store contents for comparing after sed.
> +	local contents=() endopts files=()
> +	for ((i=1; i<=${#}; i++)); do
> +		if [[ ${!i} == -- && ! -v endopts ]]; then
> +			endopts=1
> +		elif [[ ${!i} =~ ^(-i|--in-place)$ && ! -v endopts ]]; then
> +			# detect rushed sed -i -> esed -i, -i also silently breaks enewsed
> +			die "passing ${!i} to ${FUNCNAME[0]} is invalid"
> +		elif [[ ${!i} =~ ^(-f|--file)$ && ! -v endopts ]]; then
> +			i+=1 # ignore script files
> +		elif [[ ( ${!i} != -* || -v endopts ) && -f ${!i} && -r ${!i} ]]; then
> +			files+=( "${!i}" )
> +
> +			# 2>/dev/null to silence null byte warnings if sed binary files
> +			{ contents+=( "$(<"${!i}")" ); } 2>/dev/null \
> +				|| die "failed to read: ${!i}"
> +		fi
> +	done
> +	(( ${#files[@]} )) || die "no readable files found from '${*}' arguments"
> +
> +	local verbose
> +	[[ ${ESED_VERBOSE} ]] && type diff &>/dev/null && verbose=1
> +
> +	local changed newcontents
> +	if [[ -v _esed_output ]]; then
> +		[[ -v verbose ]] &&
> +			einfo "${FUNCNAME[0]}: sed ${*} > ${_esed_output} ..."
> +
> +		sed "${@}" > "${_esed_output}" \
> +			|| die "failed to run: sed ${*} > ${_esed_output}"
> +
> +		{ newcontents=$(<"${_esed_output}"); } 2>/dev/null \
> +			|| die "failed to read: ${_esed_output}"
> +
> +		local IFS=$'\n' # sed concats with newline even if none at EOF
> +		contents=${contents[*]}
> +		unset IFS
> +
> +		if [[ ${contents} != "${newcontents}" ]]; then
> +			 changed=1
> +
> +			[[ -v verbose ]] &&
> +				diff -u --color --label="${files[*]}" --label="${_esed_output}" \
> +					<(echo "${contents}") <(echo "${newcontents}")
> +		fi
>
> ... snip ...

I'm not 100% convinced that it will give you anything meaningful. The
warning about ignoring NULL is not so much noise as it is bash warning
you that you're probably not doing something correctly. In this case,
you're not pulling _all_ the contents of the file:
    
    [ /tmp ]
    oskari@dj3ntoo λ printf "ab\0cd" >test.dat
    [ /tmp ]
    oskari@dj3ntoo λ hd test.dat
    00000000  61 62 00 63 64                                    |ab.cd|
    00000005
    [ /tmp ]
    oskari@dj3ntoo λ var=$(< test.dat)
    bash: warning: command substitution: ignored null byte in input
    [ /tmp ]
    oskari@dj3ntoo λ printf "$var" | hd
    00000000  61 62 63 64                                       |abcd|
    00000004

If it's a binary file, there's a decent chance the NULL's are
significant. Now, consider the following hypothetical example where we
want to remove the NULL's:
    
    [ /tmp ]
    oskari@dj3ntoo λ printf "ab\0cd" | sed -e 's/\x00//' | hd
    00000000  61 62 63 64                                       |abcd|
    00000004

Testing for (in)equality between pre- and post-sed contents is
reasonable enough in most cases. This time, though, it would fail to
detect anything has changed since the pre-sed contents have their NULL's
unintentionally stripped, whereas the post-sed contents have them
intentionally stripped.

While I personally don't think that running sed on binary files is a
good idea in the first place, it's still relevant since the end result
would be an incorrect answer to the question of "Did sed actually do
anything?".

On the other hand, saving a set of pre- and post-sed hashes like Ulrich
suggested would give the expected result.

- Oskari

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