Tests won't pass on #MG5

[D10f]

There seems to be something wrong with the test cases for stage “The type builtin: executable files #MG5”. I can run the code fine in my local machine but when I push my changes (or use the CLI) I get a few errors.

1. As other threads have pointed out, it seems to be fixed with finding executables in /bin rather than /usr/bin. Since /bin is a symbolic link to /usr/bin I guess it’s just cannot follow it, and I’m not sure if it’s part of the challenge to work around that.

   Anyway, for now I just added a rule to ignore /usr/bin and this is solved:

   if ( strncmp( pathenv, "/usr/bin", strlen( "/usr/bin" ) ) == 0 )
      continue;
   

2. Cannot fine my_exe, as it seems to think that there should be something created in /tmp. I get this error both on my local machine and on the test suite.

   remote: [tester::#MG5] Output does not match expected value.
   remote: [tester::#MG5] Expected: "my_exe is /tmp/orange/blueberry/banana/my_exe"
   remote: [tester::#MG5] Received: "my_exe: not found"
   remote: [your-program] my_exe: not found
   

Here’s a link to the repo (yes, I’m using the evil goto to break out of the inner loop, I didn’t feel like writing any functions ):

GitHub - D10f/codecrafters-shell-c

[andy1li]

Thanks for sharing the details! I’ll take a look and get back to you by the end of the week.

[D10f]

Thank you!

I’ve updated the code to make it a tad more organized in case I’ve missed something, but still shows the same errors.

[noam-no]

The executable from the tester is created after the input, (it’s weird, I know) so you should source the commands inside PATH after.

[D10f]

Sorry, I’m a little lost on what you mean. Are you saying that the tester program is updating the PATH variable, but only after the program it’s testing runs? That seems strange… but anyway, how can I source it during program execution?

Thanks!

[rohitpaulk]

The value of PATH doesn’t change during program execution, but the executables available on the filesystem can change after the shell has started. This is exactly how shells like bash/zsh can pickup a new program soon after you’ve installed it using something like brew install <program>.

Your shell will need to check all entries in PATH every time it receives a command it doesn’t understand - it shouldn’t “preload” all entries on startup.

[D10f]

Right, that’s what I understood as well. I’m running a function separately on every loop iteration, not preloading anything.

When I print the contents of the PATH variable I get this:

/tmp/banana/banana/grape:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin

For readability:

/tmp/banana/banana/grape
/usr/local/sbin
/usr/local/bin
/usr/sbin
/usr/bin
/sbin
/bin

The temporary directory is created and placed at the beginning of the PATH. It should found the my_exe executable there.

I’ve just changed things up so that instead of printing “my_exe: not found” it prints the PATH variable, so we can see clearly whether it’s there or not, and it’s in fact loaded properly:

[tester::#MG5] Expected: "my_exe is /tmp/grape/strawberry/strawberry/my_exe"
[tester::#MG5] Received: "/tmp/grape/strawberry/strawberry:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"

If I return that path directly, it works fine. Which makes me think that the my_exe file is never actually there to be found.

char *path_var = getenv("PATH");
char *path_env = strndup(path_var, strlen(path_var));

path_env = strtok(path_env, ":");

if (strncmp(cmd, "my_exe", strlen(cmd)) == 0)
    return path_env;

Also note, regarding the other error, that it expects to find certain commands in /bin but /usr/bin comes first in the PATH:

[tester::#MG5] Expected: "cp is /bin/cp"
[tester::#MG5] Received: "cp is /usr/bin/cp"

[D10f]

I’ve found the issue with the second error regarding the my_exe command… it was completely my own fault, as I didn’t realize that I was calling strtok one extra time before actually starting going over the directories, effectively removing the first element in the PATH variable…

Sorry everyone

However, there’s still the issue of:

[tester::#MG5] Expected: "cp is /bin/cp"
[tester::#MG5] Received: "cp is /usr/bin/cp"

I still have no idea why this is a happening.

[noam-no]

Since /bin files are all symlink to /usr/bin binaries, you could exclude paths that start by /bin, maybe the tester doesn’t mind. You could also reverse iterate so that /usr/bin comes first.

[D10f]

That’s what I’ve been doing to get around this, yeah. In the code from my repository I removed it to keep it cleaner, but when testing and submitting the challenge I put it back in just for this purpose. Hope it can get fixed!

[andy1li]

@D10f here’s a reference implementation of the find_command() function FYI:

char *find_command(const char* cmd)
{
    char *path_var = getenv("PATH");
    if (path_var == NULL) {
        return NULL; // PATH not set
    }

    char *path_env = strdup(path_var);
    if (path_env == NULL) {
        return NULL; // Memory allocation failed
    }
    
    char *path_token = strtok(path_env, ":");

    while (path_token != NULL)
    {
        DIR *dir = opendir(path_token);

        if (dir != NULL)
        {
            struct dirent *directory;
            while ((directory = readdir(dir)) != NULL)
            {
                if (strcmp(directory->d_name, cmd) == 0)
                {
                    // Check if the file is executable
                    char full_path[PATH_MAX];
                    snprintf(full_path, sizeof(full_path), "%s", path_token);
                    
                    struct stat file_stat;
                    if (stat(full_path, &file_stat) == 0 && (file_stat.st_mode & S_IXUSR)) {

                      closedir(dir);
                      free(path_env);
                      return strdup(full_path); // Return full path
                    }
                   
                }
            }
            closedir(dir);
        }
        path_token = strtok(NULL, ":");
    }

    free(path_env);
    return NULL;
}

[D10f]

@andy1li

here’s a reference implementation of the find_command() function FYI:

Thanks! I’ll try to swap it out and see if there are any different results? I hope I’m not missing anything… but anyway, I’ll do that after the holidays.

Merry Christmas!

[camel-639]

Is the idea that we need to reload path in the loop? My code runs fine on my system, but I’m getting an error when I submit like ‘cat: not found’

[andy1li]

@camel-639 Could you clarify what you mean by “in the loop”?

As long as you reload PATH before attempting to resolve a command, your code should work as expected.

@rohitpaulk

Your shell will need to check all entries in PATH every time it receives a command it doesn’t understand - it shouldn’t “preload” all entries on startup.

[camel-639]

I tried solving this by filling a dynamic array with the commands in PATH on program start up. I assumed we would add a “dot” or “source” command later to update path, but it seems like I just need to redo this with a command that simply reads PATH for every single loop

[EricBrown56]

Is this doable on a windows computer?

[andy1li]

Hi @EricBrown56, Yep, you can either test remotely on our server (running Linux), or test locally after installing WSL.

[andy1li]

Closing this thread due to inactivity. If you still need assistance, feel free to reopen or start a new discussion!