Skip to content

TOOL-11892 Bulk modify of shebang lines to use /usr/bin/env#179

Merged
1 commit merged intomasterfrom
unknown repository
Jul 19, 2021
Merged

TOOL-11892 Bulk modify of shebang lines to use /usr/bin/env#179
1 commit merged intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jul 15, 2021

Problem

We hardcode the shebang of our bash scripts to use /bin/bash. There
are two problems with this.

  • Using that as the shebang line makes these bash scripts
    non-portable in the cases where /bin/bash does not exist.
  • By using that as the shebang line we do not allow users to change
    what version of bash they use by modifying their PATH.

Solution

Use whichever version of bash is first on their PATH by using
/usr/bin/env bash instead of /bin/bash.

Implementation

I greped for #!/bin/bash and replaced that with #!/usr/bin/env bash. We never passed extra flags such as -e to the shebang so no
futher changes are needed.

Testing

None

@prakashsurya
Copy link
Copy Markdown
Contributor

Out of curiosity, what is the motivation for doing this?

Using that as the shebang line makes these bash scripts non-portable in the cases where /bin/bash does not exist.

What about when /usr/bin/env doesn't exit?

By using that as the shebang line we do not allow users to change what version of bash they use by modifying their PATH.

Is this something we actually do?

I'm not opposed to this type of change, I'm just curious why we're choosing to do this.. I don't see any additional information in the JIRA ticket.

@prakashsurya
Copy link
Copy Markdown
Contributor

Additionally, is there plans to try and enforce the use of /usr/bin/env over /bin/bash moving forward? Just curious if/how we'd prevent regressions.

@ghost
Copy link
Copy Markdown
Author

ghost commented Jul 15, 2021

Out of curiosity, what is the motivation for doing this?

I guess my motivation was selfish.. On my machine:

└→ ls -lah /bin/
total 8.1k
drwxr-xr-x   2 root           root     4.0k 2021-07-14 17:04 .
drwxr-xr-x  18 root           root     4.0k 2021-05-27 08:52 ..
lrwxrwxrwx   1 root           root       75 2021-07-14 17:04 sh -> ...

└→ ls -lah /usr/bin/
total 8.1k
drwxr-xr-x   2 root           root     4.0k 2021-07-14 17:04 .
drwxr-xr-x   3 root           root     4.0k 2021-05-22 13:16 ..
lrwxrwxrwx   1 root           root       66 2021-07-14 17:04 env -> ...

I do not have /bin/bash and couldn't run ./verify-query-packages.sh because of it.

What about when /usr/bin/env doesn't exit?

I haven't seen any system without /usr/bin/env, though I also haven't seen many systems without /bin/bash. I do think that /usr/bin/env is always available.

By using that as the shebang line we do not allow users to change what version of bash they use by modifying their PATH.

Is this something we actually do?

It's definitely something that one could do. I can imagine a future where OSx no longer comes with bash since the default shell is already ZSH.

Additionally, is there plans to try and enforce the use of /usr/bin/env over /bin/bash moving forward? Just curious if/how we'd prevent regressions.

This hasn't been planned yet but I imagine it could be done.

@prakashsurya
Copy link
Copy Markdown
Contributor

I do not have /bin/bash and couldn't run ./verify-query-packages.sh because of it.

👍 ok, that sounds reasonable.

This hasn't been planned yet but I imagine it could be done.

sure, I was just wondering if this was going to be a requirement moving forward (e.g. some build automation will not work correctly with out it), in which case enforcement would be more necessary.. but sounds like it's more of a best-effort type of thing, where some manual workflows may not work due to how a developer customizes their own development environment, but the product (and automation) should continue to work regardless.

@ghost ghost merged commit dbd833c into delphix:master Jul 19, 2021
pzakha added a commit to pzakha/linux-pkg that referenced this pull request Aug 2, 2021
Backports the following changes:
- Add comment to explain use of "--allow-downgrades" (delphix#150)
- change drgn upstream repo to main (delphix#161)
- TOOL-11734 linux-pkg should use install_build_deps_from_control_file whenever possible (delphix#177)
- TOOL-11757 linux-pkg: use build dependencies list stored in virtualization package repository (delphix#175)
- TOOL-11815 linux-pkg: kernel sync-with-upstream logic sometimes fails to find expected tags upstream (delphix#178)
- TOOL-11892 Bulk modify of shebang lines to use /usr/bin/env (delphix#179)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants