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.

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

cannot escape from "warning: PWD environment variable doesn't match current directory" #1892

Open
Thomas-Didymus opened this issue Jan 3, 2024 · 9 comments

Comments

@Thomas-Didymus
Copy link

I'm using CMake with Linux in an AWS account.
I always get this warning:
kj/filesystem-disk-unix.c++:1734: warning: PWD environment variable doesn't match current directory;

I've verified that I'm not launching from symbolic linked directory.
Even it if worked (presumably only warning about symbolic link) what value does it add? Seems quite legitimate to build in a symbolic linked directory.

It is an annoying little warning.
Anyone else seeing it? Any way to suppress it?

@kentonv
Copy link
Member

kentonv commented Jan 3, 2024

Normally, your shell would automatically set PWD. It's odd that it isn't. What shell are you using?

The PWD variable allows KJ to better-support the case where the current directory includes symbolic links. The getcwd() libc function will return the canonical path to the current directory -- i.e. the fully-resolved path, not using any symbolic links. PWD, however, normally communicates the logical path that the user believed they were in, with symlinks. The difference matters if you try to open ...

For example, say you typed cd /foo/bar in your shell, but path is actually a symlink to /baz/qux. Now in your KJ-based program you try to open ../corge. Does it open /foo/corge, or /baz/corge? Intuitively, one expects to open /foo/corge, but KJ can only accomplish this if it knows that your logical path was /foo/bar, which it can only know if the shell tells it so via PWD. Otherwise, KJ will fall back to getcwd() which says the path is /baz/qux, and so it will open /baz/corge.

With all that said, in most cases the difference doesn't actually matter. If your current directory in your shell doesn't include any symlinks, then it definitely doesn't matter. Also, the capnproto build does not attempt to reach outside the build directory and so whether it's a symlink or not doesn't matter. So, this warning is spurious in your case and can be ignored.

@Thomas-Didymus
Copy link
Author

Thank you for the reply! I'm using zsh and CMake 3.26.4.

I've verified that PWD is set by printing the $ENV(PWD) from within CMake. For the launch directory both "pwd -L" and "pwd -p" give the same directory, which match PWD printed from CMake. Perplexing. Could be some nuance with CMake. Seems like others would be seeing it though.

It does work, although frustrating. I try hard to enforce a "do not ignore warnings" policy.

@kentonv
Copy link
Member

kentonv commented Jan 3, 2024

So, looking at the actual code:

// If env var PWD is set and points to the current directory, use it. This captures the current
// path according to the user's shell, which may differ from the kernel's idea in the presence
// of symlinks.
const char* pwd = getenv("PWD");
if (pwd != nullptr) {
Path result = nullptr;
struct stat pwdStat, dotStat;
KJ_IF_SOME(e, kj::runCatchingExceptions([&]() {
KJ_ASSERT(pwd[0] == '/') { return; }
result = Path::parse(pwd + 1);
KJ_SYSCALL(lstat(result.toString(true).cStr(), &pwdStat), result) { return; }
KJ_SYSCALL(lstat(".", &dotStat)) { return; }
})) {
// failed, give up on PWD
KJ_LOG(WARNING, "PWD environment variable seems invalid", pwd, e);
} else {
if (pwdStat.st_ino == dotStat.st_ino &&
pwdStat.st_dev == dotStat.st_dev) {
return kj::mv(result);
} else {
KJ_LOG(WARNING, "PWD environment variable doesn't match current directory", pwd);
}
}
}

It looks like the error will occur when lstat($PWD) and lstat(".") return different inode numbers, indicating they point at different files.

(Actually... this seems like a bug... these should be using stat, not lstat, since the whole point is to check if they point at the same file after resolving links. However, you said there are no symlinks in your PWD so that doesn't seem to be the main issue.)

It looks like the error message should include the path that PWD reported. I guess you removed that part from the error in your bug report. But does the path appear to be correct?

Perhaps you could adjust the code slightly:

-          KJ_LOG(WARNING, "PWD environment variable doesn't match current directory", pwd);
+          char buf[512];
+          KJ_LOG(WARNING, "PWD environment variable doesn't match current directory", pwd, getcwd(buf, 512));

This way the error will report both the content of PWD and the actual current directory path. Are they the same?

@Thomas-Didymus
Copy link
Author

Great suggestion. That narrowed it down. I am using a specified build directory with CMake.
Apparently CMake internally "cd's" to the build directory, but PWD remains the same.

kj/filesystem-disk-unix.c++:1736: warning: PWD environment variable doesn't match current directory; pwd = /home/workplace/src/project; buf = /home/workplace/src/project/native-build
If I remove the -B option from CMake and build in "." there is no warning. (NATIVE_BUILD was set to 'native-build')

# ----- CMake configure -----
cmake -B$NATIVE_BUILD_DIR -DNATIVE_BUILD=true

# ----- CMake build -----
cmake --build $NATIVE_BUILD_DIR

Maybe just remove this check and the cwd stuff from capnp? If someone is using symbolic links, it could be on them to make sure they are not dot-dot'ing around to unknown parts.

@kentonv
Copy link
Member

kentonv commented Jan 3, 2024

I could be convinced that we should just remove the warning. If PWD doesn't match, we just ignore it and use the canonical path.

But I do think this code provides value when it works. Consider what would happen if cd .. in your shell actually moved you to the physical parent of the current directory, rather than chopping the last component off the logical path. It'd be really confusing!

Like imagine if your home directory contains a symlink foo -> /usr/local/foo, you could have a shell session like this:

~$ cd foo
~/foo$ cd ..
/usr/local$ # wtf how did I get here?

The above doesn't actually happen in practice, but only because the shell's implementation of cd is specifically designed to operate on PWD and not on physical directories. But only cd does this. Anything else runs into the same problem:

~$ touch file.txt
~$ cd foo
~/foo$ cat ../file.txt
cat: ../file.txt: No such file or directory

This second example is real: cat actually works this way. When I tested this, my shell actually auto-completed ../file.txt when I typed ../f<tab>, but then cat couldn't find it! What!

KJ's weird stuff with PWD avoids this problem: A KJ-based tool will do the intuitive thing and will actually find ../file.txt in the above example.

@Thomas-Didymus
Copy link
Author

I would definitely appreciate removing the warning!
Specifying a build directory for CMake other than "." is triggering it.

For removing the check altogether, I'm happy either way. I'm planning to paint inside the lines. If I don't get a warning doing that, I'm a all good.

For consideration though, sometimes when tools are 'automagical' it can be frustrating. "How in the world is this working? The Linux commands don't even do that.." Just my two cents, if someone cd's ".." to the point leaving a symbolic link, that's for them to debug. A friendly error message is always nice though.

All the best, Kenton. I appreciate that thought that goes into this. I was leery at first, but I'm a Capn'p convert. Love it.

@kentonv
Copy link
Member

kentonv commented Jan 12, 2024

Happy to accept a PR removing the KJ_LOG(WARNING line, or downgrading it to INFO (which isn't displayed by default).

@kentdlee
Copy link

I am all for downgrading it to INFO for my situation. In my case I was seeing this in the output.

[2024-03-19T19:20:45.014Z] python3 test_integration_shep_gs.py -v -f

[2024-03-19T19:20:45.385Z] kj/filesystem-disk-unix.c++:1703: warning: PWD environment variable doesn't match current directory; pwd = /home/****

[2024-03-19T19:20:45.386Z] test_api_create (main.GSLS) ... :914: ImportWarning: _Importer.find_spec() not found; falling back to find_module()

[2024-03-19T19:20:45.747Z] kj/filesystem-disk-unix.c++:1703: warning: PWD environment variable doesn't match current directory; pwd = /home/****

[2024-03-19T19:20:46.108Z] FAIL

[2024-03-19T19:20:46.108Z] FAIL

I am wondering if the ImportWarning is also coming from capnproto? I am going to try isolating. I am using pycapnp in my code along with a C++ version of capnp. I am not sure if the messages are coming from the pycapnp code or the C++ library I linked into our .so. Will spend some time isolating this.

@kentonv
Copy link
Member

kentonv commented Mar 21, 2024

@kentdlee The ImportWarning is not from C++.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants