Skip to content

Conversation

@huangxiaopingRD
Copy link
Contributor

What changes were proposed in this pull request?

Consistently invoke bash with /usr/bin/env bash in scripts to make code more portable

Why are the changes needed?

some bash still use #!/bin/bash

Does this PR introduce any user-facing change?

No

How was this patch tested?

no need test

@huangxiaopingRD huangxiaopingRD changed the title [SPARK-40735] Consistently invoke bash with /usr/bin/env bash in scri… [SPARK-40735] Consistently invoke bash with /usr/bin/env bash in scripts to make code more portable Oct 10, 2022
@srowen
Copy link
Member

srowen commented Oct 10, 2022

Sounds good. Is there any situation in which this wouldn't work? I can't think of one

@huangxiaopingRD
Copy link
Contributor Author

Sounds good. Is there any situation in which this wouldn't work? I can't think of one

"#!/bin/bash" specifies that the path of bash is under the bin directory. Once there is no bash command in the bin directory, the execution will fail, but usually this is unlikely to happen unless the user changes the bash command, such as , with multiple versions of the bash command installed. "#!/usr/bin/env bash" will look for the bash command according to the path set in the environment variable.

@srowen
Copy link
Member

srowen commented Oct 10, 2022

Right I know that, just wondering if the new version ever doesnt work but I can't think of a scenario

@huangxiaopingRD
Copy link
Contributor Author

Right I know that, just wondering if the new version ever doesnt work but I can't think of a scenario

Hahahaha, you're right to be worried, but I can't think of any scenario that could go wrong either. I have tested it on mac and linux and it is normal

@srowen
Copy link
Member

srowen commented Oct 10, 2022

Can you try retriggering tests? I think that's an unrelated failure

@huangxiaopingRD
Copy link
Contributor Author

Can you try retriggering tests? I think that's an unrelated failure

Check passed. Please help to review, thanks~

@srowen
Copy link
Member

srowen commented Oct 11, 2022

Merged to master

@srowen srowen closed this in efd9ef9 Oct 11, 2022
@huangxiaopingRD huangxiaopingRD deleted the script branch October 11, 2022 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants