New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634087 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

v8 must not depend on base/

Project Member Reported by thakis@chromium.org, Aug 3 2016

Issue description

https://codereview.chromium.org/2190973003 made v8 include base/ headers.

base lives in src.git; having dependencies on it from other repos means we can't easily change base. (Especially since src.git depends on v8.git, leading to a cyle). So we have a policy that things not in src.git must not depend on base. Please fix.

See also  bug 472900 


rskang, please take a look. The bug tracker won't let me assign this to you, so fmeawad serves as placeholder :-)
 
Jochen points out that there are other places too (eg https://codereview.chromium.org/988893003/diff/480001/src/DEPS), so fmeawad is probably a pretty good owner.
(see eg comments 4 and 5 on  bug 427900  for motivation)

Comment 3 by l...@chromium.org, Aug 3 2016

Cc: l...@chromium.org
Deleted #2 as it had a wrong related bug number,

It should say:
(see eg comments 4 and 5 on  bug 472900  for motivation)
Cc: jochen@chromium.org
The main concern as stated in the other bug ( bug 472900 ):
"The point is we _never_ want to have to make changes to base and have to hunt other repos and update them. i.e. this is independent of how often base changes."

I would argue that this is not the case here, the common directory was specially designed such that it has no dependency on base (despite living in base), as we import a single header file (https://cs.chromium.org/chromium/src/base/trace_event/common/trace_event_common.h) in other repos to make sure that we can propagate new TRACE_EVENT_* macros quickly.
The functionality that depends on base lives in trace-event.h instead.

It lives in base as the rest of the tracing code does live there.

Possible solutions:
1- Copy that file in V8 (which we do not want to do as this will make the V8 copy fall out of sync quickly), as this file tends to be very active at times.
2- Move it out of base, but that does not solve the problem entirely as mentioned in the original thread that other directories in chrome might be as active.
3- Move it to a third party, and have chrome and V8 depend on it, maybe move it to catapult or something.

Jochen, WDYT?
2) and 3) sound both fine
Cc: oysteine@chromium.org
Oystein, can we move common out of base?
trace_event/common/ is specifically designed to not include any outside headers, and be exported as a separate Git repo. If v8 imports and uses the latter as a DEPS (which I believe it does for the main tracing implementation?), then there's no difference between that and jochen's point 3 from the viewpoint of v8. Essentially, where the Git repo is exported from is an implementation detail.

The reason from chrome's viewpoint of doing it like this, is so the rest of base/ can also easily use tracing.
Cc: -oysteine@chromium.org
Owner: oysteine@chromium.org

Sign in to add a comment