<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Thanks, Paul. I figured the situation would be no worse on Windows than it currently is, but this is obviously better. I am wondering why you decided to use "git svn log" to find the revision number followed by "git svn info" to get the repository, since the GetSourceVersion script uses "git svn info" for the revision already. I also think it's worth not regressing what GetRepositoryPath supports for the pure git case—that's the configuration a lot of people use for internal branches (including Apple).</div><div class=""><br class=""></div><div class="">After this change, I would actually delete the GetSourceVersion and GetRepositoryPath scripts. They're not being used by LLVM itself for anything else, and they're not cross-platform.</div><div class=""><br class=""></div><div class="">I don't mind changing the macro names (in a separate patch). I didn't like them either. :-)</div><div class=""><br class=""></div><div class="">Jordan</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 1, 2014, at 7:01 , Robinson, Paul <<a href="mailto:Paul_Robinson@playstation.sony.com" class="">Paul_Robinson@playstation.sony.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">+Jordan as he's the last one to muck with GetSVN.cmake.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><a name="_MailEndCompose" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span></a></div><div style="border-style: none none none solid; border-left-color: blue; border-left-width: 1.5pt; padding: 0in 0in 0in 4pt;" class=""><div class=""><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class="">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class=""><span class="Apple-converted-space"> </span><a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits-bounces@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>[<a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">mailto:llvm-commits-bounces@cs.uiuc.edu</a>]<span class="Apple-converted-space"> </span><b class="">On Behalf Of<span class="Apple-converted-space"> </span></b>Robinson, Paul<br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Sunday, November 30, 2014 1:58 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Sean Silva<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>RE: [PATCH] Fix Windows build<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I have attached the output with a git-svn repo on Mac. Looks fine<span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Thanks for testing it! (But you didn't actually say LGTM so leaving this open.)<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt 0.5in; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">(except for the awkwardness of calling the macros SVN_REVISION and SVN_REPOSITORY; could you change the name of those in a follow-on patch?).<span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">The GetSVN.cmake script doesn't decide on those names; they're determined by the caller.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">In this case the generated header is private to clang/lib/Basic and IIRC is used by only one module within Basic; so I don’t see any value in changing the names.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">They refer to "our (i.e. clang's) SVN revision, and that other project's (i.e. LLVM's) SVN revision." Seems clear enough to me in context.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">--paulr<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""> </span></div><div style="border-style: none none none solid; border-left-color: blue; border-left-width: 1.5pt; padding: 0in 0in 0in 4pt;" class=""><div class=""><div style="border-style: solid none none; border-top-color: rgb(181, 196, 223); border-top-width: 1pt; padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><b class=""><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class="">From:</span></b><span style="font-size: 10pt; font-family: Tahoma, sans-serif;" class=""><span class="Apple-converted-space"> </span>Sean Silva [<a href="mailto:chisophugis@gmail.com" style="color: purple; text-decoration: underline;" class="">mailto:chisophugis@gmail.com</a>]<span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Wednesday, November 26, 2014 5:47 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Robinson, Paul<br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH] Fix Windows build<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">I have attached the output with a git-svn repo on Mac. Looks fine (except for the awkwardness of calling the macros SVN_REVISION and SVN_REPOSITORY; could you change the name of those in a follow-on patch?).<o:p class=""></o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">-- Sean Silva<o:p class=""></o:p></div></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">On Wed, Nov 26, 2014 at 5:09 PM, Robinson, Paul <<a href="mailto:Paul_Robinson@playstation.sony.com" target="_blank" style="color: purple; text-decoration: underline;" class="">Paul_Robinson@playstation.sony.com</a>> wrote:<o:p class=""></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class="">Revised patch that I'm pretty sure will handle straight git.<br class="">I'd appreciate somebody trying it with a git-svn clone, though.<br class="">Thanks,<br class="">--paulr<o:p class=""></o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;" class=""><br class="">> -----Original Message-----<br class="">> From: Robinson, Paul<br class="">> Sent: Wednesday, November 26, 2014 4:15 PM<br class="">> To: Robinson, Paul;<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class="">> Subject: RE: [PATCH] Fix Windows build<br class="">><br class="">> With patch...<br class="">><br class="">> > -----Original Message-----<br class="">> > From:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits-bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits-bounces@cs.uiuc.edu</a><span class="Apple-converted-space"> </span>[mailto:<a href="mailto:llvm-commits-" style="color: purple; text-decoration: underline;" class="">llvm-commits-</a><br class="">> ><span class="Apple-converted-space"> </span><a href="mailto:bounces@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">bounces@cs.uiuc.edu</a>] On Behalf Of Robinson, Paul<br class="">> > Sent: Wednesday, November 26, 2014 4:12 PM<br class="">> > To:<span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class="">> > Subject: [PATCH] Fix Windows build<br class="">> ><br class="">> > r222391 updated GetSVN.cmake to run the LLVM helper scripts<br class="">> > GetSourceVersion<br class="">> > and GetRepositoryPath. However, being shell scripts, they work only on<br class="">> > Unix-y hosts; on Windows they don't work, you end up with an<br class="">> > SVNVersion.inc<br class="">> > that looks like this:<br class="">> ><br class="">> > #define LLVM_REVISION ""<br class="">> > #define LLVM_REPOSITORY ""<br class="">> > #define SVN_REVISION ""<br class="">> > #define SVN_REPOSITORY ""<br class="">> ><br class="">> > I've rewritten the scripts as best I can into CMake so they will work on<br class="">> > Windows as well. Now I get a better-looking SVNVersion.inc:<br class="">> ><br class="">> > #define LLVM_REVISION "222770"<br class="">> > #define LLVM_REPOSITORY "<a href="http://llvm.org/svn/llvm-project/llvm/trunk" target="_blank" style="color: purple; text-decoration: underline;" class="">http://llvm.org/svn/llvm-project/llvm/trunk</a>"<br class="">> > #define SVN_REVISION "222770"<br class="">> > #define SVN_REPOSITORY "<a href="http://llvm.org/svn/llvm-project/cfe/trunk" target="_blank" style="color: purple; text-decoration: underline;" class="">http://llvm.org/svn/llvm-project/cfe/trunk</a>"<br class="">> ><br class="">> > I can only test it on SVN, but I was able to clone code from another<br class="">> > CMake script so I'm pretty hopeful that it works for Git as well.<br class="">> ><br class="">> > Thanks,<br class="">> > --paulr<br class="">> ><br class="">> ><br class="">> > _______________________________________________<br class="">> > llvm-commits mailing list<br class="">> ><span class="Apple-converted-space"> </span><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class="">> ><span class="Apple-converted-space"> </span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" style="color: purple; text-decoration: underline;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p class=""></o:p></div></div></div><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;" class="">llvm-commits@cs.uiuc.edu</a><br class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" style="color: purple; text-decoration: underline;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></p></div></div></div></div></div></div></blockquote></div><br class=""></body></html>