Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hardcode path to dirname on macOS #10222

Closed
wants to merge 1 commit into from

Conversation

keith
Copy link
Member

@keith keith commented Nov 12, 2019

It seems that some repos have customized PATH so this isn't available.
This should fix that.

It seems that some repos have customized PATH so this isn't available.
This should fix that.
@keith
Copy link
Member Author

keith commented Nov 12, 2019

#10185 (comment)

@keith
Copy link
Member Author

keith commented Nov 12, 2019

cc @philwo can we run that failed test with this change? I don't really see how this is possible still since if users removed /usr/bin on macOS it seems like nothing would work.

@jmillikin-stripe
Copy link
Contributor

@keith It looks like the failures are specific to rules_rust, which is running commands under env -.

@keith
Copy link
Member Author

keith commented Nov 12, 2019

So they run with no PATH defined? I'm surprised that really works at all, but I guess this is a safe change then

@philwo
Copy link
Member

philwo commented Nov 13, 2019

I'm also surprised that this works at all. 🤷‍♂ Let's merge this.

@keith
Copy link
Member Author

keith commented Nov 13, 2019

@philwo should we cherry pick this to fix the downstream tests in the rc?

@philwo
Copy link
Member

philwo commented Nov 13, 2019

@keith Yes, I will cherry-pick this into 1.2.0rc2.

@kastiglione
Copy link
Contributor

Thanks @philwo

philwo pushed a commit that referenced this pull request Nov 14, 2019
It seems that some repos have customized PATH so this isn't available.
This should fix that.

Closes #10222.

PiperOrigin-RevId: 280180002
@thii
Copy link
Member

thii commented Nov 19, 2019

FWIW, rules_swift also runs commands under env -.

@keith keith deleted the ks/dirname-path branch November 19, 2019 20:39
@keith
Copy link
Member Author

keith commented Nov 19, 2019

Let me know if you see issues with that and this change, it should be ok now

philwo pushed a commit that referenced this pull request Nov 20, 2019
It seems that some repos have customized PATH so this isn't available.
This should fix that.

Closes #10222.

PiperOrigin-RevId: 280180002
philwo pushed a commit that referenced this pull request Nov 25, 2019
It seems that some repos have customized PATH so this isn't available.
This should fix that.

Closes #10222.

PiperOrigin-RevId: 280180002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants