After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 726604 - #!/bin/bash in src/box_drawing_generate.sh
#!/bin/bash in src/box_drawing_generate.sh
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other FreeBSD
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
[fixed-0-36][needed-next][commit:cbdb...
Depends on:
Blocks:
 
 
Reported: 2014-03-18 02:42 UTC by Ting-Wei Lan
Modified: 2014-03-27 18:15 UTC
See Also:
GNOME target: ---
GNOME version: 3.11/3.12


Attachments
Use /usr/bin/env (1.25 KB, patch)
2014-03-18 11:29 UTC, Egmont Koblinger
committed Details | Review

Description Ting-Wei Lan 2014-03-18 02:42:48 UTC
Some systems do not have bash installed in /bin. Change it to #!/usr/bin/env
bash is more portable.
Comment 1 Egmont Koblinger 2014-03-18 11:29:44 UTC
Created attachment 272264 [details] [review]
Use /usr/bin/env

I guess we should change all the scripts then. Is this patch okay?

Shouldn't we need a configure check to make sure we have a recent enough bash? Or can we just relax here and silently assume everyone has, and wait until someone actually complains?
Comment 2 Christian Persch 2014-03-18 11:42:46 UTC
-#!/bin/bash -i
+#!/usr/bin/env bash

That loses the -i.
Comment 3 Ting-Wei Lan 2014-03-18 11:52:27 UTC
Instead of using /usr/bin/env, we can check the path of bash in the configure script. I didn't find the -i problem when I reported this bug because src/box_drawing_generate.sh already caused build failure.
Comment 4 Christian Persch 2014-03-18 12:04:13 UTC
Why don't you just create a /bin/bash symlink?
Comment 5 Ting-Wei Lan 2014-03-18 12:19:49 UTC
I think we should not require users to modify the system in order to build and install a package.
Comment 6 Christian Persch 2014-03-18 12:21:57 UTC
I meant that freebsd should do that out of the box, not that each user should do it himself. As it stands, this is just a gratuitous difference.
Comment 7 Ting-Wei Lan 2014-03-18 12:54:17 UTC
I think a package (bash) should not add things to the system (/bin).
Comment 8 Egmont Koblinger 2014-03-18 12:57:56 UTC
(In reply to comment #2)
> -#!/bin/bash -i
> +#!/usr/bin/env bash
> 
> That loses the -i.

But I added it later on in that script. The reason is that shebang only splits at the first space, the rest is one single argument, as if you typed /usr/bin/env "bash -i" in the shell.

(What's the point of that -i, by the way?)

(In reply to comment #3)
> Instead of using /usr/bin/env, we can check the path of bash in the configure
> script. I didn't find the -i problem when I reported this bug because
> src/box_drawing_generate.sh already caused build failure.

Gosh, I so much wish I created box_drawing.h manually at the first place. Now it's generated by a script which in turn should be generated from a .in script - please noooooooooooo! :)

Original proposal of using "/usr/bin/env bash" sounds good to me.
Comment 9 Ting-Wei Lan 2014-03-18 13:05:36 UTC
(In reply to comment #1)
> Created an attachment (id=272264) [details] [review]
> Use /usr/bin/env
> 
This patch works.
Comment 10 Egmont Koblinger 2014-03-18 13:05:48 UTC
Ting-Wei, just to be absolutely sure, could you please confirm that your proposed change works correctly? I mean: please fix the script, run "make" followed by "./src/vte2_90", and insde that terminal run "cat doc/boxes.txt". Do the line drawing characters show up correctly? I'm almost sure they do, but if there's some unexpected problem with freebsd, let's catch it now.
Comment 11 Ting-Wei Lan 2014-03-18 13:13:57 UTC
Yes, "cat doc/boxes.txt" shows correctly in "./src/vte2_90".
Comment 12 Christian Persch 2014-03-18 13:32:47 UTC
Comment on attachment 272264 [details] [review]
Use /usr/bin/env

Did you run 'make check' too? If it passes, ok to commit.
Comment 13 Ting-Wei Lan 2014-03-18 13:35:44 UTC
Yes, it passes all 4 tests.
Comment 14 Egmont Koblinger 2014-03-18 13:45:48 UTC
Thanks Ting-Wei!

Fixed in vte-0-36.
Comment 15 Christian Persch 2014-03-27 18:15:16 UTC
Fixed on master.