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

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

Created:
7 years, 1 month ago by scottmg
Modified:
7 years, 1 month 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?
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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. :/ ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month ago (2012-01-31 15:23:42 UTC) #13
scottmg
> > Well, there's another option; > > > Have the rm */*/LASTCHANGE hook to ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month 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 ...
7 years, 1 month ago (2012-01-31 16:41:57 UTC) #17
scottmg
7 years, 1 month 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