@souvlaki42 Yes, it’s best to delete or comment out the fake builtin function for now.
However, I can’t see any reasons not to accept both versions of the error as valid. Can you?
I agree. The tester could be more flexible to account for multiple valid behaviors. We’ll take a look at improving that once we have more data (when we have a reproduce error to look into).
Where does this come from? A crate. I was really trying to handle this project with as few of them as possible.
I should have waited for you be to able to see the problem, before adding the my own ls wrapper.
What should I now though? Closing this topic, just to open a similar one the next time your test runner decides to run all of the previous tests again, feels unneeded. Currently, without my wrapper my own version of ls produces the same incorrect error locally, so I know it could happen again.
@Souvlaki42 Okay, I tried digging through your older commits, and found this:
Will share an update shortly. In the meantime, you can try checking out 84e5e2421c9503219039715e27b810c1bc114d38 as well.
@Souvlaki42 There’re indeed multiple ls existing on the tester:
You can verify that the PATH logic was incorrect (as pointed out by @senevoldsen) by running the previous tests via our CLI:
codecrafters test --previous
Or you can verify it locally like this:
The function access is the underlying Linux/Posix/C function to test whether we can execute. Not as in if the bit is set, but whether we might execute or not (effective vs real permissions). I don’t think there are situations where the distinction matters for the tester, and I am not sure how to achieve equivalent in Rust.
About my earlier comments about 0o111 I thought mistakenly that, that was RWX and not XXX for each user, group, other. I recall another person had issue in C# when checking 0o111 and changed to only checking whether User Execute and User Read was set: Problem locating unix executables #MG5 - #10 by salmon-571 . So I think your check here is working.
I see @andy1li replicated the issue.
As for the exact conditions I cannot explain. But consider this: You check whether you find the executable (using whatever condition) and get the path: let Some(executable) = externals.get(cmd). Then shortly after you do let result = match Command::new(cmd).args(args).output() {. So cmd=ls and executable=/usr/bin/ls or something like that. But what you pass to Command::new is not the path but just ls. I can’t fathom why, but seems to me it finds a different executable than the expected one and runs.
Ok I think the fact that I later changed: results.insert(file_name, path);
To: results.entry(file_name).or_insert(path);
fixed the issue from before without me realizing it, because it now works without the wrapper.
This means it was in fact, a path ordering issue, like you said.
Sorry for wasting so much of your time, on this.
As for why, I chose Command.new(cmd) instead of executable, that’s because another earlier test case expected argv[0] to be the command name, not it’s path.
Can you share how you would fix this?
Good job. I recommend running the full suite occasionally because breaking an earlier test and not finding out until much later has bitten me too.
For argv[0], I’m not 100% sure, but looking at the docs I think something like this is possible: Command:new(executable).arg0(cmd).args(…); , where executable is an absolute path (so you decided the search logic) and cmd is what the user entered.
Thank you. You are correct!
This topic was automatically closed 5 days after the last reply. New replies are no longer allowed.
