From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id CB0FB158091 for ; Fri, 3 Jun 2022 07:14:24 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 56184E08CD; Fri, 3 Jun 2022 07:14:18 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id CE80DE07AE for ; Fri, 3 Jun 2022 07:14:17 +0000 (UTC) From: Ulrich Mueller To: Ionen Wolkens Cc: gentoo-dev@lists.gentoo.org Subject: Re: [gentoo-dev] [PATCH 1/2] esed.eclass: new eclass In-Reply-To: <20220531112319.29168-2-ionen@gentoo.org> (Ionen Wolkens's message of "Tue, 31 May 2022 07:23:18 -0400") References: <20220531112319.29168-1-ionen@gentoo.org> <20220531112319.29168-2-ionen@gentoo.org> Date: Fri, 03 Jun 2022 09:14:05 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-dev@lists.gentoo.org Reply-to: gentoo-dev@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" X-Archives-Salt: 37b73a00-c02f-4a97-9e87-2b373901113d X-Archives-Hash: c4e0c278746101328b6889c27a89fb3c --=-=-= Content-Type: text/plain >>>>> On Tue, 31 May 2022, Ionen Wolkens wrote: > +# @FUNCTION: esed > +# @USAGE: ... > +# @DESCRIPTION: > +# sed(1) wrapper that dies if the expression(s) did not modify any files. > +# sed's -i/--in-place is forced, and so stdin/out cannot be used. This sounds like a simple enough task ... > +esed() { > + local -i i > + > + if [[ ${esedexps@a} =~ a ]]; then > + # expression must be before -- but after the rest for e.g. -E to work > + local -i pos > + for ((pos=1; pos<=${#}; pos++)); do > + [[ ${!pos} == -- ]] && break > + done > + > + for ((i=0; i<${#esedexps[@]}; i++)); do > + [[ ${esedexps[i]} ]] && > + esedexps= esed "${@:1:pos-1}" -e "${esedexps[i]}" "${@:pos}" > + done > + > + unset esedexps > + return 0 > + fi > + > + # 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}" ) > + > + # eval 2>/dev/null to silence \0 warnings if sed binary files > + eval '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}" > + > + eval '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 > + > + [[ ${contents} != "${newcontents}" ]] && changed=1 > + > + [[ -v verbose ]] && > + diff -u --color --label="${files[*]}" --label="${_esed_output}" \ > + <(echo "${contents}") <(echo "${newcontents}") > + else > + [[ -v verbose ]] && einfo "${FUNCNAME[0]}: sed -i ${*} ..." > + > + sed -i "${@}" || die "failed to run: sed -i ${*}" > + > + for ((i=0; i<${#files[@]}; i++)); do > + eval 'newcontents=$(<"${files[i]}")' 2>/dev/null \ > + || die "failed to read: ${files[i]}" > + > + if [[ ${contents[i]} != "${newcontents}" ]]; then > + changed=1 > + [[ -v verbose ]] || break > + fi > + > + [[ -v verbose ]] && > + diff -u --color --label="${files[i]}"{,} \ > + <(echo "${contents[i]}") <(echo "${newcontents}") > + done > + fi > + > + [[ -v changed ]] \ > + || die "no-op: ${FUNCNAME[0]} ${*}${_esed_command:+ (from: ${_esed_command})}" > +} ... but then it's almost 100 lines of shell code, including very convoluted parsing of parameters. The code for detection whether a parameter is actually a file is a heuristic at best and looks rather brittle. Also, don't use eval because it is evil. So IMHO this isn't the right approach to the problem. If anything, make it a simple function with well defined arguments, e.g. exactly one expression and exactly one file, and call "sed -i" on it. Then again, we had something similar in the past ("dosed" as a package manager command) but later banned it for good reason. Detection whether the file contents changed also seems complicated. Why not compute a hash of the file before and after sed operated on it? md5sum or even cksum should be good enough, since security isn't an issue here. Ulrich --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQFDBAEBCAAtFiEEtDnZ1O9xIP68rzDbUYgzUIhBXi4FAmKZtL0PHHVsbUBnZW50 b28ub3JnAAoJEFGIM1CIQV4uZLUH/RxJWupkXjpyw8DcdEE1o8mlKsYqmdrbDujF buzsNzAMvnVsWhikCUYbINnwjWkXGCKbClXgWb1hDkNpjjWIi546crtShurDHnc2 eGUFyNo6pF8GWnWS645liRNoUrdRtoU+HuUck9rpjI2qtfornmuIelL2YOJ+8Txn 5uX982btSrVzps6ttblFlkL4VxnQHBBdtfK199KDBBf0bOTIUqR+N0/7gkfTTcGI RQnB5hcfClyMMutn5U8+s5LjnY/r1h4zey3mW+l3l3RW3tn6sZp0VvInMs0emo8L 7h4O4vd+mf3Ts945Y3MuCOrbBgncIKGThDD99B79Cye2dzVi/hs= =6d5n -----END PGP SIGNATURE----- --=-=-=--