Gentoo Archives: gentoo-dev

From: Donnie Berkholz <dberkholz@g.o>
To: gentoo-dev@l.g.o
Subject: Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs.
Date: Mon, 12 Sep 2011 22:11:37
Message-Id: 20110912221049.GC31178@comet
In Reply to: Re: [gentoo-dev] [PATCH] autotools-utils.eclass: punt unnecessary .la files even w/ USE=static-libs. by "Michał Górny"
1 On 23:58 Mon 12 Sep , Michał Górny wrote:
2 > On Mon, 12 Sep 2011 16:00:20 -0500
3 > Donnie Berkholz <dberkholz@g.o> wrote:
4 > > > local f
5 > > > for f in $(find "${D}" -type f -name '*.la'); do
6 > > > # Keep only .la files with shouldnotlink=yes -
7 > > > likely plugins local shouldnotlink=$(sed -ne
8 > > > '/^shouldnotlink=yes$/p' "${f}") if [[ "$1" == 'all' || -z
9 > > > ${shouldnotlink} ]]; then
10 > > > + if [[ "$1" == 'only-not-required' ]]; then
11 > >
12 > > Is there a case where one of those arguments might be $2 but you'd
13 > > still want to run this?
14 >
15 > Er? What are you referring to?
16
17 Two things.
18
19 1. This is only reached if shouldnotlink is false. That means it's only
20 the things that you are already assuming are plugins, right? If so, why
21 is this even done?
22
23 2. What happens if I call it with `remove_libtool_files all
24 only-not-required`? Nobody ever does any checking of the # of args.
25
26 > > I feel like that shouldnotlink thing is really confusing the logic,
27 > > because there's multiple nested tests for different values of $1 in
28 > > here instead of just testing the args once at the top and setting
29 > > variables.
30 >
31 > As mentioned earlier, the code needs to be refactored. First things
32 > first, then we'll rewrite it to be nice and clean. I don't really want
33 > to waste time doing this if we would need to rewrite it for more logic
34 > in the future.
35
36 I'd rewrite it once as soon as you get all the reviews and before
37 committing. Rewrites tend to never happen, since it turns out that
38 people have other things to do with their time.
39
40 > > > + # remove .la files only when .pc
41 > > > files provide the libs
42 > > > + # already or they don't give any
43 > > > information
44 > > > + ! has $(basename "${f}")
45 > > > ${pc_libs} \
46 > > > + && [[ -n "$(sed -n
47 > > > \
48 > >
49 > > The comment says "or" but I see an "and" here.
50 >
51 > Because everything's negated here. Boolean magic :D.
52
53 OK, got it. Stop writing confusing logic. =P
54
55 --
56 Thanks,
57 Donnie
58
59 Donnie Berkholz
60 Council Member / Sr. Developer
61 Gentoo Linux
62 Blog: http://dberkholz.com

Replies