#14146: Iterate PATH when no /usr/bin/env

[JerwuQu]

Another go at #14146, following @andrewrk's latest comment.

Am still pretty fresh at Zig stdlib code so feedback appreciated (e.g. is FixedBufferAllocator the best way here?)

[Cloudef]

I will be adding similar kind of code to ElfDynLib to find system libraries as well. Maybe we could add utility like this to std.fs or std.os ?

const SearchPathIterator = struct {
    paths: std.mem.TokenIterator(u8, .scalar),

    pub fn initPath(path: []const u8) @This() {
        return .{ .paths = std.mem.tokenizeScalar(u8, path, ':') };
    }

    pub fn initEnv(env: []const u8) @This() {
        return initPath(std.os.getenv(env) orelse "");
    }

    pub fn next(self: *@This()) ?[]const u8 {
        return self.paths.next();
    }
};

(Could have nextWithComponent, to construct the relevant path for you too)

(e.g. is FixedBufferAllocator the best way here?)

I'm not sure about zig std's status quo, but I wouldn't rely on std.os.PATH_MAX as it isn't the truth. Perhaps best is to use std.heap.StackFallbackAllocator with std.os.PATH_MAX as stack size and std.heap.page_allocator(?) as a fallback.

[JerwuQu]

I can't say about adding utility methods. Seems a bit redundant when the code to iterate PATH is already so simple.

About std.os.PATH_MAX I understand that it can't be fully trusted in the case of different filesystems. With that said, I don't think we can reasonably expect anyone to have a real env binary that has a path longer than 4096, and this code safely skips any such path and continues.

[JerwuQu]

Sorry for the delay! Have been busy and had no access to my desktop for a bit.

I followed the suggestion to use cwd as base path, and instead rely on the openFile to recognize when paths are absolute.
I added error handling for cases where the file was found but something went wrong while opening it, as opposed to ignoring the culprit and continuing the search through PATH and lose that information.
Being able to print the path required also exposing it one block (which works here because getenv retains the memory).

Maybe there are some new issues due to the implemenation changing slightly, don't be afraid to nitpick.

[andrewrk]

I'm sorry, I didn't review this in time, and now it has bitrotted. Furthermore, so many pull requests have stacked up that I can't keep up and I am therefore declaring Pull Request Bankruptcy and closing old PRs that now have conflicts with master branch.

If you want to reroll, you are by all means welcome to revisit this changeset with respect to the current state of master branch, and there's a decent chance your patch will be reviewed the second time around.

Either way, I'm closing this now, otherwise the PR queue will continue to grow indefinitely.

[JerwuQu]

Understandable! Unfortunate though that it's the second time for the same issue.

Haven't been using Termux recently but might create a new PR if I do.

[andrewrk]

It is indeed unfortunate, however I think in this case it is because someone else made a related change, so your change might not be necessary anymore and needs to be re-evaluated, or at least reintegrated. Anyway my point is that progress has been made.