Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 9169105: don't always run LASTCHANGE (Closed)

Created:
6 years, 5 months ago by scottmg
Modified:
6 years, 5 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : move LASTCHANGE to build/util, don't always run it, and force regen during runhooks #

Patch Set 3 : test for existence before unlink #

Patch Set 4 : just update LASTCHANGE in deps, rather than only removing it #

Total comments: 1

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -10 lines) Patch
M .gitignore View 1 1 chunk +1 line, -0 lines 0 comments Download
M DEPS View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M build/util/build_util.gyp View 1 2 chunks +1 line, -2 lines 0 comments Download
M chrome/chrome.gyp View 1 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/chrome_common.gypi View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_installer.gypi View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/installer_tools.gyp View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/mini_installer.gyp View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/mini_installer_syzygy.gyp View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/installer/upgrade_test.gyp View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 18 (0 generated)
scottmg
I know this is ugly, but do you think acceptable for local usage?
6 years, 5 months ago (2012-01-27 00:52:36 UTC) #1
Evan Martin
I think we should just remove lastchange as part of the build, and move it ...
6 years, 5 months ago (2012-01-27 01:03:15 UTC) #2
scottmg
OK, I can do that for lastchange. I guess I could have cygwin touch a ...
6 years, 5 months ago (2012-01-27 01:10:31 UTC) #3
Evan Martin
On Thu, Jan 26, 2012 at 5:10 PM, Scott Graham <scottmg@chromium.org> wrote: > OK, I ...
6 years, 5 months ago (2012-01-27 02:46:45 UTC) #4
scottmg
On 2012/01/27 01:03:15, Evan Martin wrote: > I think we should just remove lastchange as ...
6 years, 5 months ago (2012-01-27 21:15:25 UTC) #5
Evan Martin
On 2012/01/27 21:15:25, scottmg wrote: > Looking at this a bit more, as there's quite ...
6 years, 5 months ago (2012-01-27 21:23:30 UTC) #6
scottmg
On 2012/01/27 21:23:30, Evan Martin wrote: > On 2012/01/27 21:15:25, scottmg wrote: > > Looking ...
6 years, 5 months ago (2012-01-30 19:15:01 UTC) #7
Evan Martin
On 2012/01/30 19:15:01, scottmg wrote: > Hmm, unfortunately LASTCHANGE is stored in SHARED_INTERMEDIATE_DIR so it's ...
6 years, 5 months ago (2012-01-30 19:40:01 UTC) #8
scottmg
(re-using this issue so as not to lose the conversation here) On 2012/01/30 19:40:01, Evan ...
6 years, 5 months ago (2012-01-31 00:37:41 UTC) #9
M-A Ruel
I'm not excited to generate stuff from a build outside of the build directory. :/ ...
6 years, 5 months ago (2012-01-31 01:12:03 UTC) #10
Evan Martin
Wow, awesome! This code doesn't have good test coverage, so if you could build and ...
6 years, 5 months ago (2012-01-31 03:59:10 UTC) #11
scottmg
OK. I'll see if I can split the path out into a .gypi. The only ...
6 years, 5 months ago (2012-01-31 15:16:41 UTC) #12
M-A Ruel
On 2012/01/31 15:16:41, scottmg wrote: > OK. I'll see if I can split the path ...
6 years, 5 months ago (2012-01-31 15:23:42 UTC) #13
scottmg
> > Well, there's another option; > > > Have the rm */*/LASTCHANGE hook to ...
6 years, 5 months ago (2012-01-31 16:23:00 UTC) #14
Marc-Antoine Ruel (Google)
The hooks are run in order. It's a list of dicts. So the implementation would ...
6 years, 5 months ago (2012-01-31 16:32:21 UTC) #15
scottmg
You just mean delete as the first step and then regenerate as the last? How ...
6 years, 5 months ago (2012-01-31 16:38:53 UTC) #16
Marc-Antoine Ruel (Google)
You're right, it doesn't help. M-A Le 31 janvier 2012 11:38, Scott Graham <scottmg@chromium.org> a ...
6 years, 5 months ago (2012-01-31 16:41:57 UTC) #17
scottmg
6 years, 5 months ago (2012-01-31 17:47:34 UTC) #18
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.

Powered by Google App Engine
This is Rietveld 408576698