V8 4.9 and trace_event DEPS change

[ofrobots]

Hello Build WG Members:

V8 4.9 introduces a new source dependency that embedders are going to need to be able to build V8. Specifically: https://github.com/v8/v8/blob/5312bfb9f3ecd09ebabd848dbf661e7326c84ef9/DEPS#L16

Basically, the directory v8/base/trace_event/common is supposed to be fetched from the external repository (https://chromium.googlesource.com/chromium/src/base/trace_event/common.git) as a pre-requisite in order to be able to build V8.

Since we (Node.js) don't use depot_tools and instead have a copy of V8 checked into our repository, we'll need some changes to ensure we can build and update V8 smoothly going forward.

Here are some of the options I can think of:

1. Delete (or edit) deps/v8/.gitignore in our repo and include copy of base/trace_event/common into the copy of V8 in our repository. This is simplest for the short term but requires overhead going forward when V8 or trace_event is updated.
2. Track base/trace_event/common as a direct dependency of Node.js. I.e. create a deps/trace-event-common directory with a copy of https://chromium.googlesource.com/chromium/src/base/trace_event/common.git. At build time we will have to copy / symlink this into the deps/v8/base/trace_event/common directory to allow V8 to be built.
3. Start using the v8 DEPS file directly and fetching all the dependencies as a step in the build process. This might be a more future proof solution.

Ideas/thoughts/comments?

/cc @nodejs/v8 @jeisinger

[jbergstroem]

I would prefer going with 1 and creating a script for updating v8 in deps/. I recall someone writing something similar at one point but we didn't land it. My rationale being the option of least moving parts and the burden of making sure everything checks out will be at merge/PR time.

[rvagg]

I'd like to hear @bnoordhuis and @indutny on this as they have the most history wrangling deps/v8. I don't have strong opinions either way, just a touch of dependency-fatigue.

[ofrobots]

I am not quite sure that creating a script for updating V8 is going to be trivial. We float quite a few patches over upstream and it is quite subjective which backports are going to be in what stream.

the burden of making sure everything checks out will be at merge/PR time.

It would be better if this burden was not present on every update to deps/v8. Choices 2 and 3 avoid this completely.

[mhdawson]

1 does have the advantage of not introducing additional dependencies. Pulling some of the ones needed for the V8 testing requires svn. Do you know what additional dependencies like svn would be required for 3 ?

[jeisinger]

hum, all deps from v8 should be hosted on git. which one are you referring to?

[mhdawson]

when using the gclient tools it seems to use svn https://ci.nodejs.org/job/mdawson-RunV8TestsInNode/8/console

Related to nodejs/node#4704

[jeisinger]

that's odd - at least the deps/v8/DEPS file does not reference this

[ofrobots]

This morning I have been spending some time bisecting to figure out why buffer tests are broken on the latest V8. This is on a vee-eight-4.9 that I built using option 1. It was pretty painful. Any time V8 is updated (e.g. each step of the bisect) the gitignore edit and trace-event commits have to be cherry-picked back on.

This is going to be ongoing pain for anyone who updates or bisects V8 in node. I personally would like to avoid option 1. Apart from myself, @targos, @bnoordhuis and @indutny do the bulk of work updating and backporting fixes, so I would really like to hear their opinions on this.

I think we should be able write a simple script to implement option 3 that doesn't involve any additional dependencies (e.g. svn).

[jeisinger]

@ofrobots are you hunting for issue #4701?

[targos]

I think that whatever we do, it is crucial that it stays as simple as it is now for someone to build node from the git repository: ./configure && make
If we go for option 3, a step in the configure script should check wether the trace_event dep is there and clone or update it if needed.
Ideally, the source tarball that we put on the website should have it included so that git is not necessary for people that want to build from it.

[bnoordhuis]

Option 3 implies a dependency on depot_tools. I'm +1 on option 1, it has the fewest moving parts.

[ofrobots]

It sounds like option 1 is best for now. We can revisit if it causes too much overhead going forward.

For posterity, here's how I updated trace-event DEP for a V8 extract without using depot_tools:

git_url="https://chromium.googlesource.com"
dir="base/trace_event/common"
git_repo=$(grep trace_event/common.git DEPS | awk -F+ '{ print $2 }' | tr -d " \",")
commit=$(grep trace_event/common.git DEPS | awk -F+ '{ print $4 }' | tr -d " \",")

mkdir -p ${dir}
pushd ${dir}
git init
git remote add origin ${git_url}${git_repo}
git fetch origin ${commit}
git reset --hard FETCH_HEAD
rm -rf .git
popd

[jbergstroem]

How about we create a repo with update procedures, floating patches and above script and whatnot to make bumping v8 more public? I still believe there must be some kind of scripting that should apply for most bumps -- especially minors. Also, last time I bumped a minor I didn't reapply floating patches, rather diff v8 trees and land commits on top. Perhaps thats not standard procedure but it was trivial to do.

[ofrobots]

@jbergstroem Sure thing. Should these scripts in the tools/ directory in node, or in a separate repo? A few things should be easily scriptable: 1) cherry-picking upstream fixes for backporting, 2) pulling down V8 deps such as trace-event as above. However, I am not sure that a major V8 version update is easily scriptable because it usually involves dealing with possible API changes, deprecations and test updates.

[ofrobots]

also scriptable: 3) cherry-picking patch level updates from V8 stable branch.

I can help with writing scripts for these.

[jbergstroem]

@ofrobots I don't have a strong opinion but keeping them separate perhaps makes it easier to get going?