Re: [pacman-dev] [PATCH] [makepkg] use double brackets

On 26/05/10 08:22, Andres P wrote:

On Tue, May 25, 2010 at 5:16 PM, Xavier Chantry wrote:

...

On Tue, May 25, 2010 at 10:06 PM, Andres P wrote:

...

I was about to do that, but first I think I should talk about another change.

`type -p foo` has a return val, so consider this: $ time for i in {1..1000}; do [[ $(type -p sh) ]]; done real 0m1.564s user 0m0.160s sys 0m0.337s

$ time for i in {1..1000}; do type -p sh&>/dev/null; done real 0m0.166s user 0m0.060s sys 0m0.103s

Even though there is a negligible perfomance difference in makepkg, this is the correct way to check for $?.

I don't understand this sentence. negligible = 10x ? And if your "correct" way is 10x faster, why do you say "even though" ? This makes it sound like we need to accept a compromise (like worse perf) for a correct behavior. But from my understanding of your mail, you are saying that using type -p directly is both more correct and faster.

I'm saying negligible *in* makepkg. I'm making it very clear that the justification for a change is not performance. Obviously, the same scale on that timediff could be applied to makepkg, but even then, the gains from implementing my suggestion are unperceivable.

...

...

If the redir to /dev/null becomes to much of a burden to type, it could become a function. Not to mention that the whole series of checking for vcs in PATH, which is where most of `type p` gets used, could be changed in to a more platable chunk.

Would there be interest in making this change?

I really think that throwing away the return val and making a string check is poor practice.

So the only downside of using type -p sh&>/dev/null directly without [/[[ and a subshell is that we need to type&>/dev/null ? It does not seem like a big deal to me. And yeah if it's used a lot, like more than 5 times, just make a function.

Just to be absolutely clear, I prefer&>/dev/null over the current way of handling this.

This is why I proposed this. ;)

I agree with the change. Feel free to change all the "[[ $(type -p foo) ]]" to "type -p foo > /dev/null". In fact this seems to be best in two separate patches for me. 1) [ $(type -p foo) ] -> type -p foo > /dev/null 2) [ "$(type -t foo)" = "bar" ] -> [[ $(type -t foo) == "bar" ]] Allan