<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">With some digging I worked out at least part of the history. It's not that we care about PATH per se; the test is making sure that even if you don't have a
linker in the package, there's still a file with the right name for clang to find in a place that it would (eventually) look. If you *do* have a linker in the package, clang should find it and still construct the command line correctly.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">An upstream lit test can't arrange a co-located linker (without extraordinary measures such as you suggest, which I think we don't want); we should have an
internal regression test for that case, but apparently do not.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The *actual bug* is the missing .exe extension. This appears to be a regression, looking at the original (internal) bug. You were correct to remove the directory
part of the path check, but not to remove the .exe part, as that masked the regression.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">You should be able to drop 'orbis-ld.exe' in the directory with clang and have clang find it correctly. The same is not true if you drop 'orbis-ld' into that
directory.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">--paulr<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></a></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Sean Silva [mailto:chisophugis@gmail.com]
<br>
<b>Sent:</b> Friday, July 01, 2016 12:09 AM<br>
<b>To:</b> Robinson, Paul<br>
<b>Cc:</b> cfe-commits (cfe-commits@lists.llvm.org)<br>
<b>Subject:</b> Re: r274084 - Revert "[PS4] Tighten up a test (noticed in passing)"<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Thu, Jun 30, 2016 at 5:30 PM, Robinson, Paul <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>> wrote:<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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).</span><o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Yeah, as I described in the commit message of r262285 it is due to an extra-high-priority lookup done in <span style="font-size:9.5pt">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).</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:9.5pt">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).</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:9.5pt">(actually, copying the clang binary is not ideal since that binary may be quite large)</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><a name="m_-6325035471274442508__MailEndCompose"></a><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I do think a bug report would have been appropriate, rather than just munging the test… </span><o:p></o:p></p>
</div>
</div>
</blockquote>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thanks. To be completely honest I had not noticed that due to the prefix property it wouldn't quite check the right thing.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">--paulr</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Sean Silva [mailto:<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>]
<br>
<b>Sent:</b> Thursday, June 30, 2016 4:03 PM<br>
<b>To:</b> Robinson, Paul<br>
<b>Cc:</b> cfe-commits (<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>)<br>
<b>Subject:</b> Re: r274084 - Revert "[PS4] Tighten up a test (noticed in passing)"</span><o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">On Thu, Jun 30, 2016 at 3:52 PM, Robinson, Paul <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><br>
<br>
> -----Original Message-----<br>
> From: cfe-commits [mailto:<a href="mailto:cfe-commits-bounces@lists.llvm.org" target="_blank">cfe-commits-bounces@lists.llvm.org</a>] On Behalf Of<br>
> Sean Silva via cfe-commits<br>
> Sent: Tuesday, June 28, 2016 5:29 PM<br>
> To: <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> Subject: r274084 - Revert "[PS4] Tighten up a test (noticed in passing)"<br>
><br>
> Author: silvas<br>
> Date: Tue Jun 28 19:29:23 2016<br>
> New Revision: 274084<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=274084&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=274084&view=rev</a><br>
> Log:<br>
> Revert "[PS4] Tighten up a test (noticed in passing)"<br>
><br>
> This reverts commit r269709.<br>
><br>
> r262285 changed this deliberately so that the test would not be<br>
> sensitive to which binaries are in the same directory as clang.<br>
> See the commit message of that commit for more background.<br>
<br>
Okay, but the point of the test is to match a "file.exe" instead<br>
of just "file". See commentary at the top of the test.<br>
Also "orbis-ld" is a prefix of "orbis-ld.gold" and so matching<br>
just the former doesn't verify we're looking for the right one.<br>
<br>
I understand taking out the path part of the check in r262285 but<br>
if you named your test linker "orbis-ld.exe" instead of "orbis-ld"<br>
then the test would pass with r269709, right?<o:p></o:p></p>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">Unfortunately not.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto">-- Sean Silva<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> If that's all this<br>
is about, please undo this revert and use the standard Windows<br>
file extension for your test linkers.<br>
--paulr<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><br>
><br>
> Modified:<br>
> cfe/trunk/test/Driver/ps4-linker-win.c<br>
><br>
> Modified: cfe/trunk/test/Driver/ps4-linker-win.c<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ps4-linker-" target="_blank">
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ps4-linker-</a><br>
> win.c?rev=274084&r1=274083&r2=274084&view=diff<br>
> ==========================================================================<br>
> ====<br>
> --- cfe/trunk/test/Driver/ps4-linker-win.c (original)<br>
> +++ cfe/trunk/test/Driver/ps4-linker-win.c Tue Jun 28 19:29:23 2016<br>
> @@ -22,5 +22,5 @@<br>
> // RUN: env "PATH=%T;%PATH%;" %clang -target x86_64-scei-ps4 %s -shared<br>
> \<br>
> // RUN: -fuse-ld=ps4 -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-<br>
> LINKER %s<br>
><br>
> -// CHECK-PS4-GOLD: <a href="file:///\\orbis-ld.gold.exe">\\orbis-ld.gold.exe</a>"<br>
> -// CHECK-PS4-LINKER: <a href="file:///\\orbis-ld.exe">\\orbis-ld.exe</a>"<br>
> +// CHECK-PS4-GOLD: <a href="file:///\\orbis-ld.gold">\\orbis-ld.gold</a><br>
> +// CHECK-PS4-LINKER: <a href="file:///\\orbis-ld">\\orbis-ld</a><br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><o:p></o:p></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"> <o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>