Skip to content

Replace usages of android::base::Realpath#2446

Open
jemoreira wants to merge 5 commits intogoogle:mainfrom
jemoreira:realpath
Open

Replace usages of android::base::Realpath#2446
jemoreira wants to merge 5 commits intogoogle:mainfrom
jemoreira:realpath

Conversation

@jemoreira
Copy link
Copy Markdown
Member

  • Adds our own implementation of RealPath, returning Result and handling EINTR errors.
  • Don't call realpath in SystemWideHome to break a circular dependency and because it's not correct in most situations.
  • Adds tilde expansion to cuttlefish::AbsolutePath instead of returning empty string (it still does if SystemWideHome fails, but at least that's less likely)

Bug: b/505481472
Assisted-by: Gemini:Next

@jemoreira jemoreira requested a review from Databean April 22, 2026 23:30
@jemoreira jemoreira enabled auto-merge April 23, 2026 00:46
Comment thread base/cvd/cuttlefish/host/libs/command_util/snapshot_utils.cc Outdated
Comment thread base/cvd/cuttlefish/host/libs/config/fetcher_configs.cc Outdated
Comment thread base/cvd/cuttlefish/host/libs/config/fetcher_configs.cc Outdated
Comment thread base/cvd/cuttlefish/host/libs/image_aggregator/sparse_image.cc Outdated
Comment thread base/cvd/cuttlefish/host/commands/cvd/instances/config_path.cpp Outdated
LOG(WARNING) << "Tilde expansion in path " << path <<" is not supported";
return {};
if (path == "~" || absl::StartsWith(path, "~/")) {
auto home = SystemWideUserHome();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not how ~ expansion works in the shell. For comparison, look at

HOME=$PWD bash -c 'echo ~'

This will print $PWD. So really ~ expansion should look at HOME. This also breaks the circular dependency, and SystemWideUserHome can still use AbsolutePath.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I mentioned it in the commit message, but forgot to copy to the PR description. I didn't want to use the HOME var because we frequently modify that value. We'd only get ~ in a path given by the user, the user is not expecting cvd to replace that with the group home instead of their home. I can drop this commit, since neither the variable or the system wide home are correct and it worked fine without it before. It just bothered me that it was returning and empty string, but that wasn't fixed by this either.

SystemWideHome doesn't need AbsolutePath. It's getting the value from getpwuid, which should be absolute or that user is going to have a different home directory every time they cd.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed the special handling of ~. This should be handled by the shell only, commands should allow it in paths since it's a valid file name.

The tilde character (~) is valid in path names. A file or directory
named just "~" is permitted in Linux/Unix, so AbsolutePath should allow
it instead of returning empty string.

Tilde expansion is performed by the shell and not typically expected
from commands.
and move variable declaration closer to where it's used
Calling realpath on the value obtained from getpwuid will cause it to
resolve symbolic links (that value should already be an absolute path).
That symbolic link resolution may cause problems when the returned value
is later compared with the HOME environment variable or when used in
cuttlefish::EmulateAbsolutePath with follow_symlinks set to false.
With our own implementation that calls C's realpath().

Bug: b/505481472
@jemoreira jemoreira requested a review from Databean April 23, 2026 18:49
@jemoreira jemoreira disabled auto-merge April 23, 2026 21:46
@jemoreira jemoreira added the kokoro:force-run Trigger a presubmit build unconditionally. label Apr 23, 2026
@GoogleCuttlefishTesterBot GoogleCuttlefishTesterBot removed the kokoro:force-run Trigger a presubmit build unconditionally. label Apr 23, 2026
@jemoreira jemoreira enabled auto-merge April 24, 2026 00:41
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

Successfully merging this pull request may close these issues.

3 participants