[PATCH] D56419: [gn build] Move .gn file to the root of the monorepo

Tue Jan 8 10:14:04 PST 2019

thakis added a comment.

I'd be interested in feedback from more people if anyone has any (e.g. "don't care"). Given the current feedback, I'll put this on hold for now.

In D56419#1349128 <https://reviews.llvm.org/D56419#1349128>, @brzycki wrote:

> > Sorry, I don't see how a hidden file that says "this is unsupported" has much promotional value.
>
> Placing .gn at the top-level with a comment that it's unsupported contradicts users expectations. To the casual user outside the day-to-day of llvm-dev they have no idea if they should or should not use .gn. It's confusing at best: is the .gn file location stale or the comment stale or is something else going on? I've worked in this industry long enough to see too many unsupported tools quickly become the cornerstone of key infrastructure.
>
> I genuinely don't understand why making a single symlink satisfies all potential use cases for the intrepid users who decide to build a compiler with an unsupported build system.

- it's another setup step
- no symlinks on windows without opting in to devmode

> This statement:
> 
>> And why do I bother with that? So that more people hopefully see how needlessly slow the CMake build is, so that folks trying to make the CMake build get less pushback.
> 
> And this one:
> 
>> GN is 100% unsupported (as I've added as a comment to the file as I moved it over). I do not intend to push for changing this.
> 
> seem like contradictions to me. Why bother challenging the existing CMake build system as "needlessly slow" if you don't intend to push for changing the build system?

Because it's also possible to make the cmake build better, given that there's agreement that the current speed is a problem. So far there seems to be no appetite for that.

> This isn't just about gn: if this was a commit to put a scons file at the top level I'd be pushing back just as hard.
> 
> I've said my peace and I'll let other developers weigh in on this. I am still -1 on this commit.

Thanks, that sounds fair.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56419/new/

https://reviews.llvm.org/D56419