RE: [PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

[STL] > I can't check in copies of `cl.exe` and `link.exe` (the former's version is > inspected, so it can't just be a dummy file). > I'm happy without upstream tests for this logic. In the long run, I hope that > we can eliminate this internal directory structure and make it identical to > the

Re: [PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

On Fri, Aug 18, 2017 at 6:59 PM Stephan T. Lavavej via Phabricator via cfe-commits wrote: > STL_MSFT added a comment. > > In https://reviews.llvm.org/D36860#846232, @thakis wrote: > > > Many driver tests check in a basic representative directory structure > (e.g.

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

STL_MSFT added a comment. In https://reviews.llvm.org/D36860#846232, @thakis wrote: > Many driver tests check in a basic representative directory structure (e.g. > test/Driver/Inputs/basic_freebsd_tree/ and its many siblings). > > But if you're happy with others breaking this code, I suppose

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Many driver tests check in a basic representative directory structure (e.g. test/Driver/Inputs/basic_freebsd_tree/ and its many siblings). But if you're happy with others breaking this code,

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

STL_MSFT marked an inline comment as done. STL_MSFT added a comment. In https://reviews.llvm.org/D36860#845469, @thakis wrote: > This approach looks good to me. Thanks! > Is there any chance we could have a test for this? Unfortunately, testing this involves having a VS toolset with the

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

STL_MSFT updated this revision to Diff 111776. STL_MSFT edited the summary of this revision. STL_MSFT added a comment. This addresses Don Hinton's feedback. It also unifies `IsVS2017OrNewer` and `IsDevDivInternal` into a 3-state `enum class ToolsetLayout`. https://reviews.llvm.org/D36860

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

thakis added a comment. This approach looks good to me. Is there any chance we could have a test for this? See `test/Driver` for examples; use `python bin\llvm-lit.py ../llvm/tools/clang/test/Driver/my_test.cpp` to run a single test, `ninja check-clang` to run all of them. You need some unix

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

hintonda added a comment. Looks good, but I'll defer to the owners. Minor nit... Comment at: lib/Driver/ToolChains/MSVC.cpp:142 return true; +} else if (llvm::sys::path::filename(ParentPath) == "x86ret" +|| llvm::sys::path::filename(ParentPath)

[PATCH] D36860: [Driver] Recognize DevDiv internal builds of MSVC, with a different directory structure

STL_MSFT created this revision. This is a minimally intrusive change, which I've verified works for both x86 and x64. The idea is to add another bool data member, `IsDevDivInternal`. (This could be unified with `IsVS2017OrNewer`, since it's a 3-way setting. Either a build is DevDiv-internal,