[PATCH] D29840: test-release.sh: Remove workaround for test-suite build

[Tom Stellard via Phabricator via llvm-commits]

tstellar added a comment.

In https://reviews.llvm.org/D29840#681106, @rengolin wrote:

> In https://reviews.llvm.org/D29840#673847, @tstellar wrote:
>
> > The only change with this patch is that by default the test-suite will be checked out to ${tag}/llvm.src/projects rather than ${tag}/, so the test-suite will still be checked out.
>
>
> Right, I see. This will cause the test to be built by default as well, and not ran (CMake can't do that yet), so it'll only slow down the release for no reason.


It won't do this, because the cmake file in the project dir explicitly doesn't build the test-suite: https://github.com/llvm-mirror/llvm/blob/master/projects/CMakeLists.txt.

>> Would it be better to keep ${tag}/ as the default location ?
> 
> Only if `do_test_suite=export-only`.
> 
> I'd say make the default be `export-only` in addition to your current patch, so that it gets built if the user requests it (`do_test_suite=yes`), but by default only be checked out, for external LNT validation.
> 
>>   I wasn't sure if the test-suite location mattered given that the test-release.sh script doesn't do anything with it.
> 
> AFAIK, CMake will build whatever is in the right place. Regardless, having it in `${tag}` is easier for LNT integration, so I'd still keep `export-only` as the default under `${tag}` and allow `yes` to move it to `projects`.
> 
> Makes sense?

Yes, but I think I'll also update the patch so that do_test_suite=yes actually works.

> --renato




https://reviews.llvm.org/D29840