r274084 - Revert "[PS4] Tighten up a test (noticed in passing)"

[Sean Silva via cfe-commits]

On Thu, Jun 30, 2016 at 5:30 PM, Robinson, Paul <paul.robinson at sony.com>
wrote:

> Okay, that is peculiar.  But I can repro it.  If I put either orbis-ld or
> orbis-ld.exe co-located with clang.exe, it builds a command line without
> the .exe suffix (but using the directory where clang.exe lives).
>

Yeah, as I described in the commit message of r262285 it is due to an
extra-high-priority lookup done in Driver::GetProgramPath that overrides
the PATH lookup. In fact, in the production PS4 SDK the PATH lookup
purportedly tested in this test *never occurs* since these "look in the the
directory containing clang" lookups always find orbis-ld first (since it is
always(?) right beside clang).

So this test probably needs to be changed to copy the clang binary into a
directory, put an orbis-ld.exe there, and then verify that clang finds that
orbis-ld. There is little point to testing PATH lookups since AFAIK we
don't really advertise to licensees that they should mess with the contents
of the sdk dir (i.e. to cause clang to not be beside orbis-ld and instead
find orbis-ld through a PATH lookup).

(actually, copying the clang binary is not ideal since that binary may be
quite large)


>
> I do think a bug report would have been appropriate, rather than just
> munging the test…
>
As it is (with your mods) the test is not checking what we want it to
> check.  I'll write an internal bug for this.
>

Thanks. To be completely honest I had not noticed that due to the prefix
property it wouldn't quite check the right thing.

-- Sean Silva


> --paulr
>
>
>
> *From:* Sean Silva [mailto:chisophugis at gmail.com]
> *Sent:* Thursday, June 30, 2016 4:03 PM
> *To:* Robinson, Paul
> *Cc:* cfe-commits (cfe-commits at lists.llvm.org)
> *Subject:* Re: r274084 - Revert "[PS4] Tighten up a test (noticed in
> passing)"
>
>
>
>
>
>
>
> On Thu, Jun 30, 2016 at 3:52 PM, Robinson, Paul <paul.robinson at sony.com>
> wrote:
>
>
>
> > -----Original Message-----
> > From: cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org] On Behalf
> Of
> > Sean Silva via cfe-commits
> > Sent: Tuesday, June 28, 2016 5:29 PM
> > To: cfe-commits at lists.llvm.org
> > Subject: r274084 - Revert "[PS4] Tighten up a test (noticed in passing)"
> >
> > Author: silvas
> > Date: Tue Jun 28 19:29:23 2016
> > New Revision: 274084
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=274084&view=rev
> > Log:
> > Revert "[PS4] Tighten up a test (noticed in passing)"
> >
> > This reverts commit r269709.
> >
> > r262285 changed this deliberately so that the test would not be
> > sensitive to which binaries are in the same directory as clang.
> > See the commit message of that commit for more background.
>
> Okay, but the point of the test is to match a "file.exe" instead
> of just "file". See commentary at the top of the test.
> Also "orbis-ld" is a prefix of "orbis-ld.gold" and so matching
> just the former doesn't verify we're looking for the right one.
>
> I understand taking out the path part of the check in r262285 but
> if you named your test linker "orbis-ld.exe" instead of "orbis-ld"
> then the test would pass with r269709, right?
>
>
>
> Unfortunately not.
>
>
>
> -- Sean Silva
>
>
>
>   If that's all this
> is about, please undo this revert and use the standard Windows
> file extension for your test linkers.
> --paulr
>
>
> >
> > Modified:
> >     cfe/trunk/test/Driver/ps4-linker-win.c
> >
> > Modified: cfe/trunk/test/Driver/ps4-linker-win.c
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ps4-linker-
> > win.c?rev=274084&r1=274083&r2=274084&view=diff
> >
> ==========================================================================
> > ====
> > --- cfe/trunk/test/Driver/ps4-linker-win.c (original)
> > +++ cfe/trunk/test/Driver/ps4-linker-win.c Tue Jun 28 19:29:23 2016
> > @@ -22,5 +22,5 @@
> >  // RUN: env "PATH=%T;%PATH%;" %clang -target x86_64-scei-ps4  %s -shared
> > \
> >  // RUN:     -fuse-ld=ps4 -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-
> > LINKER %s
> >
> > -// CHECK-PS4-GOLD: \\orbis-ld.gold.exe"
> > -// CHECK-PS4-LINKER: \\orbis-ld.exe"
> > +// CHECK-PS4-GOLD: \\orbis-ld.gold
> > +// CHECK-PS4-LINKER: \\orbis-ld
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160701/93a14d71/attachment.html>