Do not run gclient hooks in fetch

Do not run gclient hooks in fetch

They can fail if the system doesn't have build dependencies installed,
especially on Linux.

See https://groups.google.com/a/chromium.org/d/msg/chromium-dev/7dzfBvZuNG8/omnJTA5Vjl4J for more info.

BUG=none

[Dirk Pranke, 2013-12-06 17:22:00]

I have some reservations about this change, in that it changes the current
behavior for fetch ...

it is true that, for the first fetch on a machine, it is broken. It's probably
better to fix this than to not fix it and leave things the way they are.

However, for subsequent fetches, you will have to remember to type 'gclient
runhooks' afterwards. Requiring a second step that people might forget seems
like a step backwards, especially given that it's such a slow command. For me,
at least, I run fetch multiple times per machine, but perhaps this is not the
common case.

Perhaps there should be a --runhooks argument to fetch, or a --no-runhooks
argument, so that you can still get a single command?

Even better (IMO), would be to run install-build-deps as part of the initial
fetch, but that would probably require fetch to capture the user's password up
front so that it would not halt for the prompt after the initial git clone
completed. This would probably complicate things a fair amount. Chase and I (and
probably Robbie and Aaron, I don't remember) debated this quite a bit when we
first rolled out fetch, and the conclusion at the time was that we didn't want
to do this because we didn't want to be installing things on the bots, but this
was pre-wyk and mostly-pre-recipes, so I'm not sure how we'd approach this
today.

At any rate, don't consider this a blocking comment by itself. I'm not really
happy with any solution here except for the one like that in the previous
paragraph, and that would be a much larger change.

(The change itself lgtm, if we decide that is the behavior we want).

[agable, 2013-12-09 19:45:25]

On 2013/12/06 17:22:00, Dirk Pranke wrote:
> I have some reservations about this change, in that it changes the current
> behavior for fetch ...
> 
> it is true that, for the first fetch on a machine, it is broken. It's probably
> better to fix this than to not fix it and leave things the way they are.
> 
> However, for subsequent fetches, you will have to remember to type 'gclient
> runhooks' afterwards. Requiring a second step that people might forget seems
> like a step backwards, especially given that it's such a slow command. For me,
> at least, I run fetch multiple times per machine, but perhaps this is not the
> common case.
> 
> Perhaps there should be a --runhooks argument to fetch, or a --no-runhooks
> argument, so that you can still get a single command?
> 
> Even better (IMO), would be to run install-build-deps as part of the initial
> fetch, but that would probably require fetch to capture the user's password up
> front so that it would not halt for the prompt after the initial git clone
> completed. This would probably complicate things a fair amount. Chase and I
(and
> probably Robbie and Aaron, I don't remember) debated this quite a bit when we
> first rolled out fetch, and the conclusion at the time was that we didn't want
> to do this because we didn't want to be installing things on the bots, but
this
> was pre-wyk and mostly-pre-recipes, so I'm not sure how we'd approach this
> today.
> 
> At any rate, don't consider this a blocking comment by itself. I'm not really
> happy with any solution here except for the one like that in the previous
> paragraph, and that would be a much larger change.
> 
> (The change itself lgtm, if we decide that is the behavior we want).

The whole point of 'fetch' is to be a one-line way to get everything you need.
Removing runhooks from it is, I think, the wrong way to go about fixing it. I
think more people use 'fetch' to quickly and easily get second or third
checkouts than new users use it for the first time ever.

I think the correct thing to do here is:
a) gclient sync --nohooks
b) check for the existence of a package we think is likely to only be installed
by install-build-deps
c) if we don't find it, ask the user if they'd like to run install-build-deps
d) if so, run it
e) gclient runhooks

I think this change will create more problems than it will solve, so until we do
something more like the above or someone really convinces me, not lgtm.