don't always run LASTCHANGE

Instead of always running the LASTCHANGE extractor, run it with regular file dependencies.

During gclient runhooks, update LASTCHANGE so that it will be updated at least as often as runhooks is run (i.e. for every sync).

LASTCHANGE moved to build/util rather than SHARED_INTERMEDIATE_DIR because that directory isn't known to runhooks.

BUG=111731
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=119950

[scottmg, 2012-01-27 00:52:36]

I know this is ugly, but do you think acceptable for local usage?

[Evan Martin, 2012-01-27 01:03:15]

I think we should just remove lastchange as part of the build, and move it
instead to a hook run on sync.

[scottmg, 2012-01-27 01:10:31]

OK, I can do that for lastchange.

I guess I could have cygwin touch a file in the build dir and check vs.
Does that seem OK, or too breakable?
On Jan 26, 2012 5:03 PM, <evan@chromium.org> wrote:

> I think we should just remove lastchange as part of the build, and move it
> instead to a hook run on sync.
>
>
https://chromiumcodereview.**appspot.com/9169105/<https://chromiumcodereview....
>

[Evan Martin, 2012-01-27 02:46:45]

On Thu, Jan 26, 2012 at 5:10 PM, Scott Graham <scottmg@chromium.org> wrote:
> OK, I can do that for lastchange.
>
> I guess I could have cygwin touch a file in the build dir and check vs. Does
> that seem OK, or too breakable?

Ah, I dunno about cygwin.  I'd ask John Abd El Malek, he always rants
to me about the cygwin setup.

[scottmg, 2012-01-27 21:15:25]

On 2012/01/27 01:03:15, Evan Martin wrote:
> I think we should just remove lastchange as part of the build, and move it
> instead to a hook run on sync.

Looking at this a bit more, as there's quite a few places that refer to that
target: A simpler solution would be to regenerate it (unconditionally?) on sync,
and modify the build rule to not always run (rather, only run when the file
isn't there). That way rm -rf out/ && build would still work too, but we
wouldn't have something running every build.

It would be incorrect locally when a dev pulls or changes branches without
gclient sync, but that seems unimportant (at least to me).

Seem OK?

[Evan Martin, 2012-01-27 21:23:30]

On 2012/01/27 21:15:25, scottmg wrote:
> Looking at this a bit more, as there's quite a few places that refer to that
> target: A simpler solution would be to regenerate it (unconditionally?) on
sync,
> and modify the build rule to not always run (rather, only run when the file
> isn't there). That way rm -rf out/ && build would still work too, but we
> wouldn't have something running every build.

Hm, that seems pretty good to me.

> It would be incorrect locally when a dev pulls or changes branches without
> gclient sync, but that seems unimportant (at least to me).

Agree totally.

[scottmg, 2012-01-30 19:15:01]

On 2012/01/27 21:23:30, Evan Martin wrote:
> On 2012/01/27 21:15:25, scottmg wrote:
> > Looking at this a bit more, as there's quite a few places that refer to that
> > target: A simpler solution would be to regenerate it (unconditionally?) on
> sync,
> > and modify the build rule to not always run (rather, only run when the file
> > isn't there). That way rm -rf out/ && build would still work too, but we
> > wouldn't have something running every build.
> 
> Hm, that seems pretty good to me.

Hmm, unfortunately LASTCHANGE is stored in SHARED_INTERMEDIATE_DIR so it's a bit
messy to delete or regenerate it when not run via gyp (and different per target
& platform). I guess find . -name LASTCHANGE -exec rm '{}' \; would work, but
that's kind of icky.

cygwin was solved in a separate CL; it happens in gclient runhooks now.

[Evan Martin, 2012-01-30 19:40:01]

On 2012/01/30 19:15:01, scottmg wrote:
> Hmm, unfortunately LASTCHANGE is stored in SHARED_INTERMEDIATE_DIR so it's a
bit
> messy to delete or regenerate it when not run via gyp (and different per
target
> & platform). I guess find . -name LASTCHANGE -exec rm '{}' \; would work, but
> that's kind of icky.

Yeah.  I think we'd need to generate it in the source directory, not the shared
intermediate one.  In some sense other hooks do the same thing -- gyp writes
makefiles into the source directory, gclient checks out stuff from svn -- so
it's not as horrible as it first seems.

[scottmg, 2012-01-31 00:37:41]

(re-using this issue so as not to lose the conversation here)

On 2012/01/30 19:40:01, Evan Martin wrote:
> Yeah.  I think we'd need to generate it in the source directory, not the
shared
> intermediate one.  In some sense other hooks do the same thing -- gyp writes
> makefiles into the source directory, gclient checks out stuff from svn -- so
> it's not as horrible as it first seems.

M-A, any objections?

I'm a little afraid of breaking something because we had hardcoded the
SHARED_INTERMEDIATE_DIR path in a bunch of places, but this updates the ones I
could find.

[M-A Ruel, 2012-01-31 01:12:03]

I'm not excited to generate stuff from a build outside of the build directory.
:/

I don't have much opinion beside that. Build steps based on the scm are bound to
be non-optimal.

https://chromiumcodereview.appspot.com/9169105/diff/9012/build/util/build_uti...
File build/util/build_util.gyp (right):

https://chromiumcodereview.appspot.com/9169105/diff/9012/build/util/build_uti...
build/util/build_util.gyp:11: 'lastchange_out_path':
'<(DEPTH)/build/util/LASTCHANGE',
"While at it". If this variable was moved out of the targets into a variable
block, you could include this gyp file to not have to duplicate the variable
everywhere. I think you'd have to split it into .gypi

[Evan Martin, 2012-01-31 03:59:10]

Wow, awesome!

This code doesn't have good test coverage, so if you could build and see how it
may fail (like if someone somehow doesn't run the hooks), then save that message
for mention for chromium-dev, this LGTM.

[scottmg, 2012-01-31 15:16:41]

OK. I'll see if I can split the path out into a .gypi.

The only other defense I can offer is that it's still generated by the
build system as well, just that the scm (runhooks) forces it to update. So,
if someone doesn't runhooks, then the worst that will happen is that the
version string will be out of date. But, at least the build won't break.

On Mon, Jan 30, 2012 at 5:12 PM, <maruel@chromium.org> wrote:

> I'm not excited to generate stuff from a build outside of the build
> directory.
> :/
>
> I don't have much opinion beside that. Build steps based on the scm are
> bound to
> be non-optimal.
>
>
> https://chromiumcodereview.**appspot.com/9169105/diff/9012/**
>
build/util/build_util.gyp<https://chromiumcodereview.appspot.com/9169105/diff/9012/build/util/build_util.gyp>
> File build/util/build_util.gyp (right):
>
> https://chromiumcodereview.**appspot.com/9169105/diff/9012/**
>
build/util/build_util.gyp#**newcode11<https://chromiumcodereview.appspot.com/9169105/diff/9012/build/util/build_util.gyp#newcode11>
> build/util/build_util.gyp:11: 'lastchange_out_path':
> '<(DEPTH)/build/util/**LASTCHANGE',
> "While at it". If this variable was moved out of the targets into a
> variable block, you could include this gyp file to not have to duplicate
> the variable everywhere. I think you'd have to split it into .gypi
>
>
https://chromiumcodereview.**appspot.com/9169105/<https://chromiumcodereview....
>

[M-A Ruel, 2012-01-31 15:23:42]

On 2012/01/31 15:16:41, scottmg wrote:
> OK. I'll see if I can split the path out into a .gypi.

Fine with follow up cleanup CL. Don't waste too much time with that.

> The only other defense I can offer is that it's still generated by the
> build system as well, just that the scm (runhooks) forces it to update. So,
> if someone doesn't runhooks, then the worst that will happen is that the
> version string will be out of date. But, at least the build won't break.

Well, there's another option;

Have the rm */*/LASTCHANGE hook to be run before gyp hook.
Have gyp to run lastchange.py _during gyp processing_.

Does that look insane?

[scottmg, 2012-01-31 16:23:00]

>
> Well, there's another option;
>
>
> Have the rm */*/LASTCHANGE hook to be run before gyp hook.
> Have gyp to run lastchange.py _during gyp processing_.
>
>
That would be fine, where can "before" happen though? I guess ideally it'd
be in a git or svn fetch hook. Is that possible?

[Marc-Antoine Ruel (Google), 2012-01-31 16:32:21]

The hooks are run in order. It's a list of dicts.

So the implementation would be to put one before the other in the list with
a comment explaining why the order is important.

M-A

Le 31 janvier 2012 11:22, Scott Graham <scottmg@chromium.org> a écrit :

>  Well, there's another option;
>>
>>
>> Have the rm */*/LASTCHANGE hook to be run before gyp hook.
>> Have gyp to run lastchange.py _during gyp processing_.
>>
>>
> That would be fine, where can "before" happen though? I guess ideally it'd
> be in a git or svn fetch hook. Is that possible?
>
>

[scottmg, 2012-01-31 16:38:53]

You just mean delete as the first step and then regenerate as the last? How
does that help?

If either the remove or the regenerate succeed then the build will be
correct so it doesn't seem useful to do the same thing 2x. Do I
misunderstand?

On Tue, Jan 31, 2012 at 8:31 AM, Marc-Antoine Ruel <maruel@google.com>wrote:

> The hooks are run in order. It's a list of dicts.
>
> So the implementation would be to put one before the other in the list
> with a comment explaining why the order is important.
>
> M-A
>
> Le 31 janvier 2012 11:22, Scott Graham <scottmg@chromium.org> a écrit :
>
>  Well, there's another option;
>>>
>>>
>>> Have the rm */*/LASTCHANGE hook to be run before gyp hook.
>>> Have gyp to run lastchange.py _during gyp processing_.
>>>
>>>
>> That would be fine, where can "before" happen though? I guess ideally
>> it'd be in a git or svn fetch hook. Is that possible?
>>
>>
>

[Marc-Antoine Ruel (Google), 2012-01-31 16:41:57]

You're right, it doesn't help.

M-A

Le 31 janvier 2012 11:38, Scott Graham <scottmg@chromium.org> a écrit :

> You just mean delete as the first step and then regenerate as the last?
> How does that help?
>
> If either the remove or the regenerate succeed then the build will be
> correct so it doesn't seem useful to do the same thing 2x. Do I
> misunderstand?
>
>
> On Tue, Jan 31, 2012 at 8:31 AM, Marc-Antoine Ruel <maruel@google.com>wrote:
>
>> The hooks are run in order. It's a list of dicts.
>>
>> So the implementation would be to put one before the other in the list
>> with a comment explaining why the order is important.
>>
>> M-A
>>
>> Le 31 janvier 2012 11:22, Scott Graham <scottmg@chromium.org> a écrit :
>>
>>  Well, there's another option;
>>>>
>>>>
>>>> Have the rm */*/LASTCHANGE hook to be run before gyp hook.
>>>> Have gyp to run lastchange.py _during gyp processing_.
>>>>
>>>>
>>> That would be fine, where can "before" happen though? I guess ideally
>>> it'd be in a git or svn fetch hook. Is that possible?
>>>
>>>
>>
>

[scottmg, 2012-01-31 17:47:34]

On 2012/01/31 03:59:10, Evan Martin wrote:
> This code doesn't have good test coverage, so if you could build and see how
it
> may fail (like if someone somehow doesn't run the hooks), then save that
message
> for mention for chromium-dev, this LGTM.

So, I can't think of any way to get it to fail with an error. If they don't
runhooks, they won't have the extracted LASTCAHNGE output file, but then build
will generate it.

The only failure condition that I can come up with is getting the out-of-date
version, but there won't be any error.