Skip to content

Implement execvpe_impl#16322

Merged
straight-shoota merged 15 commits intocrystal-lang:masterfrom
straight-shoota:feat/process.execvpe
Nov 7, 2025
Merged

Implement execvpe_impl#16322
straight-shoota merged 15 commits intocrystal-lang:masterfrom
straight-shoota:feat/process.execvpe

Conversation

@straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Nov 4, 2025

Fills the stub execvpe_impl fallback for platforms without LibC.execvpe with a proper implementation.

It's possible to choose our own implementation with the flag -Dexecvpe_impl. This has proven to be useful for testing purposes, and we might keep it around for a while (there will be more follow-up changes where it could be useful), maybe indefinitely.

Some of the design decisions for edge case behaviour were not exactly straightforward because system libraries handle things very differently sometimes. See #6464 (comment) for more details.

I usually opted for the most common and simple variant. We can refine details in the future if the need arises.

  • default path is /usr/bin:/bin (like darwin and similar to mosts BSDs).
  • too long pathnames are skipped without warning
  • check executability by trying execve
  • NOEXEC error does not try to execute a shell script.

The following system implementations served as role models:

@straight-shoota straight-shoota marked this pull request as draft November 4, 2025 15:39
@straight-shoota straight-shoota marked this pull request as ready for review November 4, 2025 16:21
Copy link
Collaborator

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could make the implementation feel more like Crystal and less like C. I mean it basically does the following which is much more readable (but allocates lots of String on each attempt just to throw them away):

ENV["PATH"].split(':') do |path|
  next if path.bytesize + file.bytesize + 2 > LibC::PATH_MAX

  path = "." if path.empty?
  LibC.execve(File.join(path, file), argv, envp)

  # execve failed: handle errno
end

Co-authored-by: Julien Portalier <julien@portalier.com>
@straight-shoota
Copy link
Member Author

If we could make that idiomatic Crystal code work without allocations, I'm all ears =)

This is one of the raw parts of the standard library which just needs to make a lot of concessions.
The version with ReferenceStorage(IO::Memory) is already much cleaner: af547b6
We should be able to use that eventually.

FWIW the C-style code has the benefit of making it easier to compare with implementations in C libraries... 🤷

@straight-shoota straight-shoota added this to the 1.19.0 milestone Nov 4, 2025
@ysbaddaden
Copy link
Collaborator

Yes, ReferenceStorage(IO::Memory) is much nicer.
I wonder if a Slice(T)#split(value, &) would make sense 🤔

Anyway, that's out of scope :)

@straight-shoota straight-shoota merged commit 9472be2 into crystal-lang:master Nov 7, 2025
40 checks passed
@straight-shoota straight-shoota deleted the feat/process.execvpe branch November 7, 2025 11:20
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