@matheusbsbs / matheus-bagliano / commits / c70115b4b1

Fork 0

Teach gitlinks to ie_modified() and ce_modified_check_fs()

The ie_modified() function is the workhorse for refresh_cache_entry(), i.e. checking if an index entry that is stat-dirty actually has changes. After running quicker check to compare cached stat information with results from the latest lstat(2) to answer "has modification" early, the code goes on to check if there really is a change by comparing the staged data with what is on the filesystem by asking ce_modified_check_fs(). However, this function always said "no change" for any gitlinks that has a directory at the corresponding path. This made ie_modified() to miss actual changes in the subproject. The patch fixes this first by modifying an existing short-circuit logic before calling the ce_modified_check_fs() function. It knows that for any filesystem entity to which ie_match_stat() says its data has changed, if its cached size is nonzero then the contents cannot match, which is a correct optimization only for blob objects. We teach gitlink objects to this special case, as we already know that any gitlink that ie_match_stat() says is modified is indeed modified at this point in the codepath. With the above change, we could leave ce_modified_check_fs() broken, but it also futureproofs the code by teaching it to use ce_compare_gitlink(), instead of assuming (incorrectly) that any directory is unchanged. Originally noticed by Alex Riesen on Cygwin. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jul 29, 2008 at 01:13 UTC c70115b4b1a8c197b9b829b1fd15d578a3ffcdc3

1 file changed +22 -5

read-cache.c

+22 -5

@@ -147,7 +147,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)

147 break;

148 case S_IFDIR:

149 if (S_ISGITLINK(ce->ce_mode))

150 - return 0;

150 + return ce_compare_gitlink(ce) ? DATA_CHANGED : 0;

151 default:

152 return TYPE_CHANGED;

153 }

@@ -187,6 +187,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)

187 changed |= TYPE_CHANGED;

188 break;

189 case S_IFGITLINK:

190 + /* We ignore most of the st_xxx fields for gitlinks */

191 if (!S_ISDIR(st->st_mode))

192 changed |= TYPE_CHANGED;

193 else if (ce_compare_gitlink(ce))

@@ -293,11 +294,22 @@ int ie_modified(const struct index_state *istate,

294 if (changed & (MODE_CHANGED | TYPE_CHANGED))

295 return changed;

296

296 - /* Immediately after read-tree or update-index --cacheinfo,

297 - * the length field is zero. For other cases the ce_size

298 - * should match the SHA1 recorded in the index entry.

297 + /*

298 + * Immediately after read-tree or update-index --cacheinfo,

299 + * the length field is zero, as we have never even read the

300 + * lstat(2) information once, and we cannot trust DATA_CHANGED

301 + * returned by ie_match_stat() which in turn was returned by

302 + * ce_match_stat_basic() to signal that the filesize of the

303 + * blob changed. We have to actually go to the filesystem to

304 + * see if the contents match, and if so, should answer "unchanged".

305 + *

306 + * The logic does not apply to gitlinks, as ce_match_stat_basic()

307 + * already has checked the actual HEAD from the filesystem in the

308 + * subproject. If ie_match_stat() already said it is different,

309 + * then we know it is.

310 */

300 - if ((changed & DATA_CHANGED) && ce->ce_size != 0)

311 + if ((changed & DATA_CHANGED) &&

312 + (S_ISGITLINK(ce->ce_mode) || ce->ce_size != 0))

313 return changed;

314

315 changed_fs = ce_modified_check_fs(ce, st);

@@ -1326,6 +1338,11 @@ static void ce_smudge_racily_clean_entry(struct cache_entry *ce)

1338 * falsely clean entry due to touch-update-touch race, so we leave

1339 * everything else as they are. We are called for entries whose

1340 * ce_mtime match the index file mtime.

1341 + *

1342 + * Note that this actually does not do much for gitlinks, for

1343 + * which ce_match_stat_basic() always goes to the actual

1344 + * contents. The caller checks with is_racy_timestamp() which

1345 + * always says "no" for gitlinks, so we are not called for them ;-)

1346 */

1347 struct stat st;

1348