Viewing Issue Simple Details Jump to Notes ] Wiki ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006024 [DCSS] Bug Report minor have not tried 2012-08-03 06:21 2012-08-04 20:30
Reporter KiloByte View Status public  
Assigned To sgrunt
Priority normal Resolution done  
Status resolved   Product Branch 0.11 ancient branch
Summary 0006024: rltiles get rebuilt at wrong time
Description The Makefile keeps rebuilding (possibly parts of) the rltiles subdirectory when obviously not needed. For example, repeatedly rebuilding Crawl with minor changes to a file unrelated to rltiles, you'll see "GEN" lines over and over, that seem to randomly go away after multiple builds. Even worse, targets like "clean" will build/regenerate rltiles as well.
Additional Information
Tags No tags attached.
Attached Files

- Relationships

-  Notes
(0019375)
sgrunt (administrator)
2012-08-03 17:15

This is apparently being caused by the tilegen tool refusing to overwrite identical files, leading to the timestamps not being updated and thereby confusing the Makefile.

I can think of two immediate approaches to fix this:

1) disable checking to see if the two files are identical - remove rltiles/tool/tile-gen-processor.cc:899-908), causing the file to get overwritten in all cases; or

2) update the timestamp of the old file by utimes() or equivalent call at tile_gen_processor.cc:905) - this would require some work to ensure that a utime() or utimes()-alike is available across platforms, though even Windows seems to have a _utime().

I would lean towards 2), subject to a clean way to implement the relevant checking for an appropriate function.
(0019379)
KiloByte (manager)
2012-08-03 18:24

How exactly would 2) help? The prerequisites are known to be newer than the target file (as otherwise make wouldn't do a rebuild), so if the target's timestamp is not updated, unnecessary rebuilds will continue.

It is possible to give every generated header a separate stamp file and use those for dependencies, but I doubt this would be worth the hassle. The only spurious rebuilds would be if you replaced a .png file (as opposed to adding/removing/etc one), and that's no different from editing some C++ sources. In other words: rebuilds would happen at most once per edit, while currently they happen every single unrelated compile.
(0019380)
sgrunt (administrator)
2012-08-03 18:29

By "the old file", I am referring to the target file with the old timestamp, as opposed to the new temporary file that is generated when tilegen is called.

Because tilegen refuses to overwrite the target file when the target file and the new temporary file are identical, the target's timestamp isn't updated. Inserting the call as specified in 2) updates the target file's timestamp as is needed to prevent rebuilds.
(0019384)
KiloByte (manager)
2012-08-03 20:59

... which is the same as just overwriting it unconditionally, except slower.
(0019410)
sgrunt (administrator)
2012-08-04 20:30

I did some further investigation of this, and came up with the following:

- Commit 5d812b3 introduced the "refuse to overwrite if unchanged" behaviour. The cited reason was to "prevent needless rebuilds when merely editing tile files" (this doesn't make sense to me; the code as written regenerates the image file before everything else, and the image file obviously needs regeneration if tiles are edited).

- With this change (partly) reverted (i.e. always unconditionally writing the files), tiles builds behaved correctly, but non-tiles builds would regenerate apparently unrelated files (perhaps this is the above-described behaviour). Further investigation showed that the dependency paths of some image files was incorrect for non-tiles builds; this was because the code for finding tiles is meant to check several paths for the relevant images and only add the correct path, but when tiles was disabled the code short-circuited at the first path.

- Making both changes above solves the issue for both tiles and non-tiles builds.

The above changes are reflected in 0.11-a0-3124-g6162a48, and I consider this issue to be addressed.

- Issue History
Date Modified Username Field Change
2012-08-03 06:21 KiloByte New Issue
2012-08-03 17:15 sgrunt Note Added: 0019375
2012-08-03 17:15 sgrunt Status new => confirmed
2012-08-03 18:24 KiloByte Note Added: 0019379
2012-08-03 18:29 sgrunt Note Added: 0019380
2012-08-03 20:59 KiloByte Note Added: 0019384
2012-08-04 20:30 sgrunt Note Added: 0019410
2012-08-04 20:30 sgrunt Status confirmed => resolved
2012-08-04 20:30 sgrunt Fixed in Branch => 0.11 development branch
2012-08-04 20:30 sgrunt Resolution open => done
2012-08-04 20:30 sgrunt Assigned To => sgrunt


Mantis 1.1.8[^]
Copyright © 2000 - 2009 Mantis Group
Powered by Mantis Bugtracker