[llvm] r298513 - Make home_directory look in the password database in addition to $HOME.

[Zachary Turner via llvm-commits]

I wasn't aware people do this.  If that's the case, then perhaps I could
simply change the test to get the value from the password database inside
the test, then unset HOME, then confirm that it returns the same thing that
it says in the password database?

It sounds like you don't disagree with the functionality change in
home_directory(), but just the way the test is implemented, right?

On Wed, Mar 22, 2017 at 9:02 AM Stephan Bergmann <sbergman at redhat.com>
wrote:

> On 03/22/2017 04:24 PM, Zachary Turner via llvm-commits wrote:
> > Author: zturner
> > Date: Wed Mar 22 10:24:59 2017
> > New Revision: 298513
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=298513&view=rev
> > Log:
> > Make home_directory look in the password database in addition to $HOME.
> >
> > This is something of an edge case, but when the $HOME environment
> > variable is not set, we can still look in the password database
> > to get the current user's home directory.
> >
> > Added a test for this by getting the value of $HOME, then unsetting
> > it, then calling home_directory() and verifying that it succeeds
> > and that the value is the same as what we originally read from
> > the environment.
>
> Why should builds not be supported which are done in an environment
> where HOME deliberately points somewhere other than the user's home
> directory?
>
> > Modified:
> >     llvm/trunk/lib/Support/Unix/Path.inc
> >     llvm/trunk/unittests/Support/Path.cpp
> >
> > Modified: llvm/trunk/lib/Support/Unix/Path.inc
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Path.inc?rev=298513&r1=298512&r2=298513&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Support/Unix/Path.inc (original)
> > +++ llvm/trunk/lib/Support/Unix/Path.inc Wed Mar 22 10:24:59 2017
> > @@ -920,12 +920,18 @@ std::error_code real_path(const Twine &p
> >  namespace path {
> >
> >  bool home_directory(SmallVectorImpl<char> &result) {
> > -  if (char *RequestedDir = getenv("HOME")) {
> > -    result.clear();
> > -    result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
> > -    return true;
> > +  char *RequestedDir = getenv("HOME");
> > +  if (!RequestedDir) {
> > +    struct passwd *pw = getpwuid(getuid());
> > +    if (pw && pw->pw_dir)
> > +      RequestedDir = pw->pw_dir;
> >    }
> > -  return false;
> > +  if (!RequestedDir)
> > +    return false;
> > +
> > +  result.clear();
> > +  result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
> > +  return true;
> >  }
> >
> >  static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char>
> &Result) {
> >
> > Modified: llvm/trunk/unittests/Support/Path.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/Path.cpp?rev=298513&r1=298512&r2=298513&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/unittests/Support/Path.cpp (original)
> > +++ llvm/trunk/unittests/Support/Path.cpp Wed Mar 22 10:24:59 2017
> > @@ -328,6 +328,26 @@ TEST(Support, HomeDirectory) {
> >    }
> >  }
> >
> > +#ifndef LLVM_ON_WIN32
> > +TEST(Support, HomeDirectoryWithNoEnv) {
> > +  std::string Original;
> > +  char const *path = ::getenv("HOME");
> > +  // Don't try to test if we don't have something to compare against.
> > +  if (!path)
> > +    return;
> > +  Original = path;
> > +  ::unsetenv("HOME");
> > +
> > +  SmallString<128> HomeDir;
> > +  auto status = path::home_directory(HomeDir);
> > +  EXPECT_TRUE(status);
> > +  EXPECT_EQ(Original, HomeDir);
> > +
> > +  // Now put the original environment variable back
> > +  ::setenv("HOME", Original.c_str(), 1);
> > +}
> > +#endif
> > +
> >  TEST(Support, UserCacheDirectory) {
> >    SmallString<13> CacheDir;
> >    SmallString<20> CacheDir2;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170322/b408d9f7/attachment.html>