buildtools exists to be shared between the various chromium-related projects. See buildtools/README.md.

I don't think we want to get rid of it. In fact, I think we should actually be moving most stuff from depot_tools into it so they can be versioned.

I think that at the very least we should merge buildtools/third_party into build/third_party.  Here's the rationale:

1. The only thing that depends on buildtools/third_party is build.  Projects that don't use build (like skia) don't need buildtools/third_party, but it always downloads buildtools/third_party unnecessarily.  By moving to build/third_party, projects will automatically get exactly what they need, depending on if they use build or not.
2. buildtools/third_party is the only part of buildtools that has BUILD.gn files.  The rest seems to be just binaries.  It makes sense to keep build files separate.
3. The reason in the original post, and the motivation for this change: we would be able to test buildtools/third_party/*.gn changes on the CQ.
4. Projects that roll buildtools almost always roll build at the same time, since the buildfiles are so tightly coupled.  This would remove the coupling.

I initially considered moving buildtools/third_party in third_party, but I think it makes more sense to put it in build/third_party because:
1. There would be no duplicate BUILD.gn files across projects.
2. It should mostly "just work": we won't need to make changes in other chromium projects (besides making build a recursive dep instead of a regular one).

+brettw ptal at c#2

The third_party powers that be really don't like us having multiple third_party locations, so I don't think we should create a //build/third_party, even if this does mean that we have to have dependencies from //build to //third_party and that creates more work for others.

> buildtools exists to be shared between the various chromium-related projects.
> See buildtools/README.md.

I know why it exists :). I also don't think it's been worth the hassle, and given that there's too many toolchain-y things that aren't part of buildtools (like clang, xcode, msvs), I think it's misleading.

As to moving stuff from depot_tools to in-repo, we also currently version depot_tools (in //third_party/depot_tools), so that's kind of a misleading analogy. There are places where we probably aren't calling the appropriate version, though, but that's a different change.

> The third_party powers that be really don't like us having multiple third_party locations, so I don't think we should create a //build/third_party, even if this does mean that we have to have dependencies from //build to //third_party and that creates more work for others.

I feel like that's an insufficient reason given the benefits listed in c#2.  Besides, we're not adding a third_party, we're just moving one.

Maybe we could have the build files in //build, but pull the libraries into //third_party anyway?

> I feel like that's an insufficient reason given the benefits listed in c#2.

Actually, they tend to get absolute veto for things like this :).

> Maybe we could have the build files in //build, but pull the libraries
> into //third_party anyway?

We can do //build/secondary/third_party, but I'm not sure if that's much better in this case. Maybe it is?

> We can do //build/secondary/third_party, but I'm not sure if that's much better in this case. Maybe it is?

If that loophole will allow getting around the third_party restriction, then it's ok with me 👌

I should note that nothing in //build/secondary/third_party is DEPS'ed in, but we would be adding repos that would be.

I'm confused. //build/secondary/third_party should *only* have files that map onto things being DEPS'ed in, that seems to mostly be the case, at least for android, catapult, and libjpeg_turbo? 

thomasanderson@thomasanderson:~/dev/chromium_official/src/build/secondary/third_party$ ls
android_platform  android_tools  catapult  libjpeg_turbo  nss
thomasanderson@thomasanderson:~/dev/chromium_official/src/build/secondary/third_party$ find . -name "\.git"
# nothing

You're misunderstanding how //build/secondary works; "//build/secondary/X/BUILD.gn" is where to find the build file for "//X/BUILD.gn" if //X/BUILD.gn doesn't exist. 

Ah I see.  Yeah in that case I think it makes sense to put it in //third_party and use //build/secondary, so long as that sg to everyone.  Projects would still have to manually DEPS in lib{c++,c++abi,unwind}, but I think I can live with that.

If they still need the other files in //third_party/* (like README.chromium, OWNERS, etc.) then I'm not sure if there's a win to using //build/secondary, though.

Good point.  Then I still think //build/third_party is the way to go.  I just want to make sure the change won't get filibustered before starting.  Do you know who needs to give approval for //build/third_party?

I am telling you it won't be approved :).

In addition, we already have other dependencies from //build to //third_party, so we shouldn't try to put things in two places (and I don't think we want to move //third_party/android_{platform,tools} into //build).

dpranke@ how about we just move //buildtools/third_party into //third_party and then put the build files somewhere in //build ?

Move //buildtools/third_party/* to //third_party/*, build files and all.

And duplicate the BUILD.gn files for any projects that require libc++?  I guess I'm ok with this, but just double-checking that this is what we want.

I'm a bit confused: what specifically would be duplicated? Aren't the build files in the directory?

//buildtools/third_party/libc++/BUILD.gn would need to be duplicated.  The libc++ repo is in //buildtools/third_party/libc++/trunk/

The same is true for libc++abi and libunwind.

Okay, yeah, then I guess we'd require people to duplicate the build files (or we'd create subtreed mirrors of our //third_party/libc++ directory that they could deps in).

> or we'd create subtreed mirrors of our //third_party/libc++ directory that they could deps in

I would prefer this.  Since the libc++ revision is tightly coupled with the build file, would it be ok if we used a recursive DEPS to achieve this?  eg. the submodule would be //third_party/libc++ which would contain //third_party/libc++/DEPS that pulls in the real libc++ repo at //third_party/libc++/trunk ?

Any chance we can upstream the build files to LLVM?

Side note: I think it is actually possible to test changes to buildtools against chromium now (we've been adding this functionality for ANGLE). +ehmaldonaldo for more. So, maybe that solves the original problem (though it doesn't get rid of buildtools).

I don't think we should create 2*N repos. The whole point of //build/secondary is for build files for repos that aren't willing to take the build files upstream, and I think the creating a bunch of near-empty subtreed modules would probably be a decent amount of overhead for little gain.

Given that, the options seem to be:

1) use //build/secondary and deps in //third_party/libc++ et al. Other people need to deps in //build and //third_party/libc++.

2) upstream the build files.

3) keep //buildtools the way it is, at least as far as the llvm-based parts of it go. Possibly be able to test libc++ changes via ehmaldonaldo's work.

Yes, it should be possible to test the changes to buildtools against chromium now.
For example in [1] you can check the logs and see that libc++/trunk got synced to the revision specified on the patch:

[1] https://chromium-review.googlesource.com/c/chromium/buildtools/+/1079904

I like options 1 and 3.  With regard to option 1, last time you said "If they still need the other files in //third_party/* (like README.chromium, OWNERS, etc.) then I'm not sure if there's a win to using //build/secondary, though."  I guess we won't be able to just stuff those files in //build/secondary?  If not, I'm fine with closing this issue out and relying on option 3.

Still think that build/third_party is the best way to go, but that's obviously not going to happen.

Hm, true re: README.chromium et al.

That's unfortunate.

In that case, the best option would be to make use of ehmaldonaldo's work to test buildtools changes in Chromium, so I'm closing this issue.