[llvm] r295768 - [Support] Add a function to check if a file resides locally.

[Zachary Turner via llvm-commits]

Honestly i think it may just be better to delete the test. I knew this test
had the chance of breaking when I originally added it but didn't think it
would occur.

There are plenty of examples of untested functions for low level system
functions whose results can't be predicted. Imagine getting the system page
size or system temp directory for example.
On Fri, Mar 3, 2017 at 8:49 AM Simon Dardis <Simon.Dardis at imgtec.com> wrote:

> I was considering the option of using an environmental variable as well.
> In our case, half our
> buildbots are configured to use remote storage due to the nature of the
> machines themselves.
>
> I think the environment variable route is the only cross platform way to
> retain the test in a
> useful fashion.
>
> It would also help dealing with tests (in the future) which are by their
> nature sensitive to the
> environment they're running it. Given the majority of buildbots (and
> developers I presume)
> build and test in a 'regular' environment, we wouldn't lose test coverage
> for the small minority
> with more exotic environments.
>
> I do agree it's quite the sledgehammer approach.
>
> Thanks,
> Simon
>
> From: Hahnfeld, Jonas [mailto:Hahnfeld at itc.rwth-aachen.de]
> Sent: 03 March 2017 16:20
> To: zturner at google.com
> Cc: Simon Dardis; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r295768 - [Support] Add a function to check if a file
> resides locally.
>
> In the latter case, #1 doesn't work either. We could introduce an option
> or a variable in the environment to enable this specific test and set it on
> a few build bots, but that seems a bit overkill...
>
> Am Freitag, den 03.03.2017, 15:27 +0000 schrieb Zachary Turner:
> #2 would introduce enough additional file system code into the test that
> the code building up to the important part is more likely to fail than the
> important part.
>
> I could also imagine someone installing their entire os on an NFS so
> perhaps there is just nothing we can do
> On Thu, Mar 2, 2017 at 11:15 PM Hahnfeld, Jonas <
> Hahnfeld at itc.rwth-aachen.de> wrote:
> Two more ideas after thinking about the problem a bit more:
> 1)      We could test with a file in $TMP and hope that it is local.
> 2)      On *nix systems, we could parse all mounted paths and blacklist
> certain filesystems, namely NFS to begin with.
> I think both methods may be error prone in some cases. The only other
> solution is removing the test which I also quite don’t like that much.
>
> From: Zachary Turner [mailto:zturner at google.com]
> Sent: Friday, March 03, 2017 7:56 AM
> To: Hahnfeld, Jonas
> Cc: Simon Dardis; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r295768 - [Support] Add a function to check if a file
> resides locally.
>
> My only idea in that case is to just delete the test sadly :-/
>
> I can't think of a way to reliably test this if we can't assume we're on a
> local filesystem
> On Thu, Mar 2, 2017 at 2:04 AM Hahnfeld, Jonas <
> Hahnfeld at itc.rwth-aachen.de> wrote:
> Hi Zachary,
>
> This change essentially breaks tests when anyone is building on NFS as I
> do.
> Simon XFAIL'd the test for the mips buildbots in r295840 but that leaves
> the
> problem for anyone else.
>
> Do you have a clever solution for that?
>
> Regards,
> Jonas
>
> > -----Original Message-----
> > From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf
> > Of Zachary Turner via llvm-commits
> > Sent: Tuesday, February 21, 2017 9:56 PM
> > To: llvm-commits at lists.llvm.org
> > Subject: [llvm] r295768 - [Support] Add a function to check if a file
> > resides
> > locally.
> >
> > Author: zturner
> > Date: Tue Feb 21 14:55:47 2017
> > New Revision: 295768
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=295768&view=rev
> > Log:
> > [Support] Add a function to check if a file resides locally.
> >
> > Differential Revision: https://reviews.llvm.org/D30010
> >
> > Modified:
> >     llvm/trunk/include/llvm/Support/FileSystem.h
> >     llvm/trunk/include/llvm/Support/MemoryBuffer.h
> >     llvm/trunk/lib/Support/MemoryBuffer.cpp
> >     llvm/trunk/lib/Support/Unix/Path.inc
> >     llvm/trunk/lib/Support/Windows/Path.inc
> >     llvm/trunk/unittests/Support/Path.cpp
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170303/4574c756/attachment.html>