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.

Sign up for GitHub

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

Jump to bottom

Add execvpe support #682

Closed
Susurrus opened this issue Jul 17, 2017 · 5 comments · Fixed by #975
Closed

Add execvpe support #682

Susurrus opened this issue Jul 17, 2017 · 5 comments · Fixed by #975
Labels
A-features E-good-first-bug E-mentor

Comments

@Susurrus
Copy link
Contributor

Susurrus commented Jul 17, 2017 •
edited

This was removed in #681 because it was broken. Re-adding it is basically undoing fcc22f2 (without the feature support and instead conditionally including it on supported platforms). libc has execvpe included for haiku, linux, and openbsd platforms. If you are a new contributor, go ahead and read CONTRIBUTING.md, assign this to yourself, and file a PR when you've gotten somewhere (start the title with WIP if it's not ready to merge, and include "closes #682" in the text of the PR).
@panzertime
Copy link

I'd like to be assigned this issue

@Susurrus
Copy link
Contributor Author

Awesome! Thanks for offering to contribute to nix!

I wasn't expecting someone to pick this up so quickly, so this is actually missing a crucial detail that you'll want to make a first commit that changes the libc dependency back to git (making that one change as a single commit means it can be easily removed later). And since you got here so quick I also wanted to ask how did you find out about this issue or nix?

Just post here if you have any issues and I'll be sure to address them ASAP!

@panzertime
Copy link

Basically, I stumbled across nix while searching for a way to practice my Rust, and when I looked at the issues I was lucky enough to find a fresh beginner issue

@Susurrus
Copy link
Contributor Author

Awesome, well I'm glad I keep a few of these around! There's plenty of easy work to do around here (see #254 and #264 for more).

@panzertime panzertime mentioned this issue Jul 17, 2017
Closed
@F1rst-Unicorn F1rst-Unicorn mentioned this issue Nov 18, 2018
Merged
@F1rst-Unicorn
Copy link
Contributor

Hi guys,
I have ported the code from the old pull request and adapted the missing parts. Will be glad to see this
merged soon.

bors bot added a commit that referenced this issue Nov 23, 2018
@bors @F1rst-Unicorn
Merge #975
6920394 
975: Add execvpe support, conditional on platform r=asomers a=F1rst-Unicorn

This closes #682 

Co-authored-by: F1rst-Unicorn <f1rst_unicorn@njsm.de>
@bors bors bot closed this as completed in #975 Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-features E-good-first-bug E-mentor
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add execvpe support, conditional on platform
3 participants
@Susurrus @panzertime @F1rst-Unicorn