Chromium Code Reviews
Help | Chromium Project | Sign in
(7)

Issue 325483002: Change the order of include pathes to avoid including potential staled files. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by c.shu
Modified:
1 year, 1 month ago
CC:
blink-reviews
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Change the order of include pathes to avoid including potential staled files. BUG=380054 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175728

Patch Set 1 #

Messages

Total messages: 15 (0 generated)
c.shu
1 year, 1 month ago (2014-06-06 22:03:00 UTC) #1
dcheng
rslgtm if it doesn't break any builders and incremental build after applying FastMobileScrolling works. Sorry ...
1 year, 1 month ago (2014-06-06 22:11:56 UTC) #2
c.shu
On 2014/06/06 22:11:56, dcheng wrote: > rslgtm if it doesn't break any builders and incremental ...
1 year, 1 month ago (2014-06-06 22:23:04 UTC) #3
c.shu
The CQ bit was checked by c.shu@samsung.com
1 year, 1 month ago (2014-06-06 22:23:13 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/c.shu@samsung.com/325483002/1
1 year, 1 month ago (2014-06-06 22:24:50 UTC) #5
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink ...
1 year, 1 month ago (2014-06-06 23:38:08 UTC) #6
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
1 year, 1 month ago (2014-06-06 23:49:02 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/10699)
1 year, 1 month ago (2014-06-06 23:49:02 UTC) #8
c.shu
The CQ bit was checked by c.shu@samsung.com
1 year, 1 month ago (2014-06-07 00:16:23 UTC) #9
c.shu
On 2014/06/06 23:49:02, I haz the power (commit-bot) wrote: > Try jobs failed on following ...
1 year, 1 month ago (2014-06-07 00:16:57 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/c.shu@samsung.com/325483002/1
1 year, 1 month ago (2014-06-07 00:17:00 UTC) #11
haraken
> to avoid including potential staled files. Just help me understand: In what situation can ...
1 year, 1 month ago (2014-06-07 02:16:47 UTC) #12
commit-bot: I haz the power
Change committed as 175728
1 year, 1 month ago (2014-06-07 03:39:22 UTC) #13
c.shu
On 2014/06/07 02:16:47, haraken wrote: > > to avoid including potential staled files. > > ...
1 year, 1 month ago (2014-06-07 15:22:59 UTC) #14
haraken
1 year, 1 month ago (2014-06-07 16:32:35 UTC) #15
Message was sent while issue was closed.
On 2014/06/07 15:22:59, c.shu wrote:
> On 2014/06/07 02:16:47, haraken wrote:
> > > to avoid including potential staled files.
> > 
> > Just help me understand: In what situation can this happen, specifically?
> 
> Hi, haraken, here's the full story.
> 1. Originally, all generated files are located under gen/blink. E.g.,
> HTMLNames.h
> and cpp files include it using
> "#include "HTMLNames.h"
> since gen/blink is in the include search path.
> 2. The CL that moves the generated files to their component's directory
changed
> the location.
> E.g., HTMLNames.h is generated under gen/blink/core. The cpp file still uses
> "#include "HTMLNames.h"
> because gen/blink/core is added in the include search path.
> But if you don't do a clean build after this CL, the gen/blink/HTMLNames.h
still
> exists in your output dir.
> And gen/blink is also in the search path (for including v8 files).
> If "gen/blink" is found first, the staled gen/blink/HTMLNames.h is used.
> This will cause problem only if HTMLNames.h is regenerated with different
> content.
> Some people have reported the problem and it also happened to my own CL
related
> to fastmobilescrolling flag.
> 3. This CL changes the order of search paths, so gen/blink/core is found
before
> gen/blink and the staled files are ignore.
> Thanks.

ah, got it; thanks for the clarification. LGTM.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 5fa3ca5