brendangregg / FlameGraph

Stack trace visualizer
http://www.brendangregg.com/flamegraphs.html
17.37k stars 1.96k forks source link

Make interpreter location more platform-independent #123

Open mavam opened 7 years ago

mavam commented 7 years ago

The shebang of stackcollapse.pl begins with #!/usr/bin/perl. On FreeBSD, Perl is located in /usr/local/bin, causing the script to fail.

Does anything speak against using the canonical form #!/usr/bin/env perl?

brendangregg commented 7 years ago

First I've heard of that being "canonical".

It starts with #!/usr/bin/perl -w. env doesn't pass in the -w, so it would need a "use warnings;". You sure env is in /usr/bin/env on all OSes, and not /bin/env? env also spawns a new process (won't matter much for this tool, since its execution time should dwarf it). There's probably more, but that's what I remember off-hand.

I'm not saying I'm opposed to the change, just it's not as simple as it might seem.

mavam commented 7 years ago

(Perhaps "canonical" was the wrong word, I meant to imply "portable.")

You sure env is in /usr/bin/env on all OSes, and not /bin/env?

It is my understanding that /usr/bin/env exists for portability reasons, and I haven't seen any UNIX flavor yet that doesn't ship with env in /usr/bin, precisely because so many interpreted scripts rely on this lookup mechanism. (By the way, another anchored binary is /bin/sh, for the same reason.)

I see the pain that this causes, in particular in combination with Perl. But if switching to use warnings to make env happy is not a deal-breaker, I think we'll end up with a more portable set of scripts. The indirection of spawning through env seems negligible to me.

brendangregg commented 7 years ago

Ubuntu Xenial:

$ ls -l /bin/env /usr/bin/env
ls: cannot access '/bin/env': No such file or directory
-rwxr-xr-x 1 root root 31408 Feb 18  2016 /usr/bin/env

Amazon Linux:

$ ls -l /bin/env /usr/bin/env
-rwxr-xr-x 1 root root 28128 Aug  6  2015 /bin/env
lrwxrwxrwx 1 root root    13 Nov 25  2015 /usr/bin/env -> ../../bin/env

Fedora 25:

$ ls -li /bin/env /usr/bin/env
121194 -rwxr-xr-x. 1 root root 32520 Jul 19  2016 /bin/env
121194 -rwxr-xr-x. 1 root root 32520 Jul 19  2016 /usr/bin/env

Weird, huh!

That's just the first three systems I've tried. /usr/bin/env exists on all three, but the way it exists isn't standard.

brendangregg commented 7 years ago

I don't have any other systems to try right now, so I'll accept that /usr/bin/env is everywhere, and changing it would be fine.

mavam commented 7 years ago

Yeah, a lot of systems have to do some path tweaking to make available /usr/bin/env. But it's always there!

I'm close to submitting a PR as of https://github.com/mavam/FlameGraph/commit/69f275e035a149b56881622b200320c2051f482a, except that I don't have a good idea how to migrate use of perl -s. (See the comments there for details.)

mavam commented 7 years ago

nudge

versable commented 6 years ago

I'd be all for it (else a PR[1] with a sed replace implementation might get proposed), while it isn't a standard, it's used enough to warrant a wiki section dedicated to it[2].

[1] https://github.com/mit-pdos/xv6-public/pull/50 [2] https://en.wikipedia.org/wiki/Shebang_(Unix)#Portability

brendangregg commented 5 years ago

I recently learned the hard way that env breaks other observability tools. flamegraph.pl can consume CPU resources, and if it does, it shows up as "flamegraph.pl" in top(1). With env, it would show up as "perl". This either slows down debugging (if the user is SSH'd to the instance they then need to inspect the arguments) or could break it (post mortem debugging where only process names are captured in monitoring tools).