Gentoo Archives: gentoo-dev

From: Ulrich Mueller <ulm@g.o>
To: Ionen Wolkens <ionen@g.o>
Cc: gentoo-dev@l.g.o
Subject: Re: [gentoo-dev] [PATCH 1/2] esed.eclass: new eclass
Date: Fri, 03 Jun 2022 07:14:24
Message-Id: umteu8bia@gentoo.org
In Reply to: [gentoo-dev] [PATCH 1/2] esed.eclass: new eclass by Ionen Wolkens
1 >>>>> On Tue, 31 May 2022, Ionen Wolkens wrote:
2
3 > +# @FUNCTION: esed
4 > +# @USAGE: <sed-argument>...
5 > +# @DESCRIPTION:
6 > +# sed(1) wrapper that dies if the expression(s) did not modify any files.
7 > +# sed's -i/--in-place is forced, and so stdin/out cannot be used.
8
9 This sounds like a simple enough task ...
10
11 > +esed() {
12 > + local -i i
13 > +
14 > + if [[ ${esedexps@a} =~ a ]]; then
15 > + # expression must be before -- but after the rest for e.g. -E to work
16 > + local -i pos
17 > + for ((pos=1; pos<=${#}; pos++)); do
18 > + [[ ${!pos} == -- ]] && break
19 > + done
20 > +
21 > + for ((i=0; i<${#esedexps[@]}; i++)); do
22 > + [[ ${esedexps[i]} ]] &&
23 > + esedexps= esed "${@:1:pos-1}" -e "${esedexps[i]}" "${@:pos}"
24 > + done
25 > +
26 > + unset esedexps
27 > + return 0
28 > + fi
29 > +
30 > + # Roughly attempt to find files in arguments by checking if it's a
31 > + # readable file (aka s/// is not a file) and does not start with -
32 > + # (unless after --), then store contents for comparing after sed.
33 > + local contents=() endopts files=()
34 > + for ((i=1; i<=${#}; i++)); do
35 > + if [[ ${!i} == -- && ! -v endopts ]]; then
36 > + endopts=1
37 > + elif [[ ${!i} =~ ^(-i|--in-place)$ && ! -v endopts ]]; then
38 > + # detect rushed sed -i -> esed -i, -i also silently breaks enewsed
39 > + die "passing ${!i} to ${FUNCNAME[0]} is invalid"
40 > + elif [[ ${!i} =~ ^(-f|--file)$ && ! -v endopts ]]; then
41 > + i+=1 # ignore script files
42 > + elif [[ ( ${!i} != -* || -v endopts ) && -f ${!i} && -r ${!i} ]]; then
43 > + files+=( "${!i}" )
44 > +
45 > + # eval 2>/dev/null to silence \0 warnings if sed binary files
46 > + eval 'contents+=( "$(<"${!i}")" )' 2>/dev/null \
47 > + || die "failed to read: ${!i}"
48 > + fi
49 > + done
50 > + (( ${#files[@]} )) || die "no readable files found from '${*}' arguments"
51 > +
52 > + local verbose
53 > + [[ ${ESED_VERBOSE} ]] && type diff &>/dev/null && verbose=1
54 > +
55 > + local changed newcontents
56 > + if [[ -v _esed_output ]]; then
57 > + [[ -v verbose ]] &&
58 > + einfo "${FUNCNAME[0]}: sed ${*} > ${_esed_output} ..."
59 > +
60 > + sed "${@}" > "${_esed_output}" \
61 > + || die "failed to run: sed ${*} > ${_esed_output}"
62 > +
63 > + eval 'newcontents=$(<${_esed_output})' 2>/dev/null \
64 > + || die "failed to read: ${_esed_output}"
65 > +
66 > + local IFS=$'\n' # sed concats with newline even if none at EOF
67 > + contents=${contents[*]}
68 > + unset IFS
69 > +
70 > + [[ ${contents} != "${newcontents}" ]] && changed=1
71 > +
72 > + [[ -v verbose ]] &&
73 > + diff -u --color --label="${files[*]}" --label="${_esed_output}" \
74 > + <(echo "${contents}") <(echo "${newcontents}")
75 > + else
76 > + [[ -v verbose ]] && einfo "${FUNCNAME[0]}: sed -i ${*} ..."
77 > +
78 > + sed -i "${@}" || die "failed to run: sed -i ${*}"
79 > +
80 > + for ((i=0; i<${#files[@]}; i++)); do
81 > + eval 'newcontents=$(<"${files[i]}")' 2>/dev/null \
82 > + || die "failed to read: ${files[i]}"
83 > +
84 > + if [[ ${contents[i]} != "${newcontents}" ]]; then
85 > + changed=1
86 > + [[ -v verbose ]] || break
87 > + fi
88 > +
89 > + [[ -v verbose ]] &&
90 > + diff -u --color --label="${files[i]}"{,} \
91 > + <(echo "${contents[i]}") <(echo "${newcontents}")
92 > + done
93 > + fi
94 > +
95 > + [[ -v changed ]] \
96 > + || die "no-op: ${FUNCNAME[0]} ${*}${_esed_command:+ (from: ${_esed_command})}"
97 > +}
98
99 ... but then it's almost 100 lines of shell code, including very
100 convoluted parsing of parameters. The code for detection whether a
101 parameter is actually a file is a heuristic at best and looks rather
102 brittle. Also, don't use eval because it is evil.
103
104 So IMHO this isn't the right approach to the problem. If anything, make
105 it a simple function with well defined arguments, e.g. exactly one
106 expression and exactly one file, and call "sed -i" on it.
107
108 Then again, we had something similar in the past ("dosed" as a package
109 manager command) but later banned it for good reason.
110
111 Detection whether the file contents changed also seems complicated.
112 Why not compute a hash of the file before and after sed operated on it?
113 md5sum or even cksum should be good enough, since security isn't an
114 issue here.
115
116 Ulrich

Attachments

File name MIME type
signature.asc application/pgp-signature

Replies

Subject Author
Re: [gentoo-dev] [PATCH 1/2] esed.eclass: new eclass Ionen Wolkens <ionen@g.o>