1 |
On Wed, 30 May 2012 17:19:49 -0400 |
2 |
Mike Frysinger <vapier@g.o> wrote: |
3 |
|
4 |
> On Monday 28 May 2012 03:58:56 Michał Górny wrote: |
5 |
> > +# @USAGE: [all] |
6 |
> |
7 |
> this is incorrect. the usage is: |
8 |
> <all | files to remove> |
9 |
|
10 |
No, it's perfectly valid. Moreover, it even explains what the function |
11 |
actually does rather than your imagination. |
12 |
|
13 |
But if we're not interested in keeping it compatible, I'd probably |
14 |
start with making it '--all'. |
15 |
|
16 |
> > + local arg |
17 |
> > + for arg in $(find "${D}" -name '*.pc' -exec \ |
18 |
> > + sed -n -e 's;^Libs:;;p' {} |
19 |
> > +); do |
20 |
> > + [[ ${arg} == -l* ]] && |
21 |
> > pc_libs+=(lib${arg#-l}.la) |
22 |
> > + done |
23 |
> |
24 |
> let sed do the processing: |
25 |
> pc_libs=( |
26 |
> $(find "${D}" -name '*.pc' \ |
27 |
> -exec sed -n -e '/^Libs:/{s|^[^:]*:||;s: :\n:g;p}' {} |
28 |
> + | \ sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}') |
29 |
> ) |
30 |
|
31 |
Nope. My poor eyes are too weak to disband all those magical characters. |
32 |
|
33 |
> however, i'd point out that parsing these files directly doesn't |
34 |
> actually work. some .pc files use variables in the filename which |
35 |
> isn't expanded by using sed. thus your only recourse is to let |
36 |
> pkg-config expand things for you. |
37 |
|
38 |
Right. I'll have to think about this a little. |
39 |
|
40 |
> although, since we don't call die or anything, we can pipeline it to |
41 |
> speed things up a bit: |
42 |
> pc_libs=( $( |
43 |
> tpc="${T}/.pc" |
44 |
> find "${D}" -name '*.pc' -type f | \ |
45 |
> while read pc ; do |
46 |
> sed -e '/^Requires:/d' "${pc}" > "${tpc}" |
47 |
> $(tc-getPKG_CONFIG) --libs "${tpc}" |
48 |
> done | tr ' ' '\n' | sort -u | \ |
49 |
> sed -n '/^-l/{s:^-l:lib:;s:$:.la:;p}' |
50 |
> rm -f "${tpc}" |
51 |
> ) ) |
52 |
|
53 |
Could you remind me, please, what performance-critical use of this |
54 |
function does justify making it so harsh? |
55 |
|
56 |
> > + local archivefile=${f/%.la/.a} |
57 |
> |
58 |
> remove the suffix and it'll be faster i think: |
59 |
> local archivefile="${f%.la}.a" |
60 |
|
61 |
Hmm, it's indeed inconsistent. |
62 |
|
63 |
> > + [[ "${f}" != "${archivefile}" ]] || die 'regex |
64 |
> > sanity check failed' |
65 |
> |
66 |
> no need to quote the ${f}, but eh |
67 |
|
68 |
Yep. |
69 |
|
70 |
> > + rm -f "${archivefile}" || die |
71 |
> |
72 |
> `rm -f` almost never fails. in the edge cases where it does, you've |
73 |
> got bigger problems. |
74 |
|
75 |
And that problem is good enough to die here. |
76 |
|
77 |
> > + elif has "$(basename "${f}")" "${pc_libs[@]}"; then |
78 |
> |
79 |
> use ${f##*/} rather than `basename` |
80 |
|
81 |
Hmm, yes. I'd split it out to a sep var as I see you used it already |
82 |
more than once. |
83 |
|
84 |
> > + removing='covered by .pc' |
85 |
> > + elif [[ ! $(sed -n -e \ |
86 |
> > + "s/^\(dependency_libs\|inherited_linker_flags\)='\(.*\)'$/\2/p" |
87 |
> > \ |
88 |
> > + "${f}") ]]; then removing='no libs & flags' |
89 |
> |
90 |
> unwrap that body, and use -r rather than escaping the (|) chars |
91 |
|
92 |
Will do. |
93 |
|
94 |
Expect a new version in next 12hrs. |
95 |
|
96 |
-- |
97 |
Best regards, |
98 |
Michał Górny |