Re: [PATCH v2 03/19] read-cache: move index v2 specific functions to their own file
Duy Nguyen writes: > On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer > wrote: >> @@ -489,8 +479,8 @@ extern void *read_blob_data_from_index(struct >> index_state *, const char *, unsig >> #define CE_MATCH_RACY_IS_DIRTY 02 >> /* do stat comparison even if CE_SKIP_WORKTREE is true */ >> #define CE_MATCH_IGNORE_SKIP_WORKTREE 04 >> -extern int ie_match_stat(const struct index_state *, const struct >> cache_entry *, struct stat *, unsigned int); >> -extern int ie_modified(const struct index_state *, const struct cache_entry >> *, struct stat *, unsigned int); >> +extern int ie_match_stat(struct index_state *, const struct cache_entry *, >> struct stat *, unsigned int); >> +extern int ie_modified(struct index_state *, const struct cache_entry *, >> struct stat *, unsigned int); >> > > I would rather we keep "const struct index_state*" if we could. I > tried putting "const" back and found that ce_match_stat_basic() calls > set_istate_ops(), which writes to "struct index_state". Putting > set_istate_ops() in ce_match_stat_basic() may seem convenient, but > does not make much sense (why would a match_stat function update > index_ops?). I think you could move it out and > > - read_index calls set_istate_ops > - (bonus) discard_index probably should reset "version" field to zero > and clear internal_ops > - the callers that use index without read_index must call > initialize_index() or something, which in turn calls set_istate_ops. > initialize_index() may take the preferred index version > - do not let update-index modifies version field directly when > --index-version is given. Wrap it with set_index_version() or > something, so we can do internal conversion from one version to > another if needed > - remove set_istate_ops in write_index(), we may need internal_ops > long before writing. When write_index is called, internal_ops should > be already initialized Ok, this makes sense. The only thing that I'm a little worried about is catching all code paths that need to initialize the index. I'll implement these suggestions in the re-roll. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/19] read-cache: move index v2 specific functions to their own file
On Sat, Jul 13, 2013 at 12:26 AM, Thomas Gummerer wrote: > @@ -489,8 +479,8 @@ extern void *read_blob_data_from_index(struct index_state > *, const char *, unsig > #define CE_MATCH_RACY_IS_DIRTY 02 > /* do stat comparison even if CE_SKIP_WORKTREE is true */ > #define CE_MATCH_IGNORE_SKIP_WORKTREE 04 > -extern int ie_match_stat(const struct index_state *, const struct > cache_entry *, struct stat *, unsigned int); > -extern int ie_modified(const struct index_state *, const struct cache_entry > *, struct stat *, unsigned int); > +extern int ie_match_stat(struct index_state *, const struct cache_entry *, > struct stat *, unsigned int); > +extern int ie_modified(struct index_state *, const struct cache_entry *, > struct stat *, unsigned int); > I would rather we keep "const struct index_state*" if we could. I tried putting "const" back and found that ce_match_stat_basic() calls set_istate_ops(), which writes to "struct index_state". Putting set_istate_ops() in ce_match_stat_basic() may seem convenient, but does not make much sense (why would a match_stat function update index_ops?). I think you could move it out and - read_index calls set_istate_ops - (bonus) discard_index probably should reset "version" field to zero and clear internal_ops - the callers that use index without read_index must call initialize_index() or something, which in turn calls set_istate_ops. initialize_index() may take the preferred index version - do not let update-index modifies version field directly when --index-version is given. Wrap it with set_index_version() or something, so we can do internal conversion from one version to another if needed - remove set_istate_ops in write_index(), we may need internal_ops long before writing. When write_index is called, internal_ops should be already initialized -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/19] read-cache: move index v2 specific functions to their own file
Move index version 2 specific functions to their own file. The non-index
specific functions will be in read-cache.c, while the index version 2
specific functions will be in read-cache-v2.c.
Helped-by: Nguyen Thai Ngoc Duy
Signed-off-by: Thomas Gummerer
---
Makefile | 2 +
cache.h | 16 +-
read-cache-v2.c | 556 +
read-cache.c | 575 ---
read-cache.h | 57 +
test-index-version.c | 5 +
6 files changed, 661 insertions(+), 550 deletions(-)
create mode 100644 read-cache-v2.c
create mode 100644 read-cache.h
diff --git a/Makefile b/Makefile
index 5a68fe5..73369ae 100644
--- a/Makefile
+++ b/Makefile
@@ -711,6 +711,7 @@ LIB_H += progress.h
LIB_H += prompt.h
LIB_H += quote.h
LIB_H += reachable.h
+LIB_H += read-cache.h
LIB_H += reflog-walk.h
LIB_H += refs.h
LIB_H += remote.h
@@ -854,6 +855,7 @@ LIB_OBJS += prompt.o
LIB_OBJS += quote.o
LIB_OBJS += reachable.o
LIB_OBJS += read-cache.o
+LIB_OBJS += read-cache-v2.o
LIB_OBJS += reflog-walk.o
LIB_OBJS += refs.o
LIB_OBJS += remote.o
diff --git a/cache.h b/cache.h
index 7af853b..5082b34 100644
--- a/cache.h
+++ b/cache.h
@@ -95,19 +95,8 @@ unsigned long git_deflate_bound(git_zstream *, unsigned
long);
*/
#define DEFAULT_GIT_PORT 9418
-/*
- * Basic data structures for the directory cache
- */
#define CACHE_SIGNATURE 0x44495243 /* "DIRC" */
-struct cache_version_header {
- unsigned int hdr_signature;
- unsigned int hdr_version;
-};
-
-struct cache_header {
- unsigned int hdr_entries;
-};
#define INDEX_FORMAT_LB 2
#define INDEX_FORMAT_UB 4
@@ -280,6 +269,7 @@ struct index_state {
initialized : 1;
struct hash_table name_hash;
struct hash_table dir_hash;
+ struct index_ops *ops;
};
extern struct index_state the_index;
@@ -489,8 +479,8 @@ extern void *read_blob_data_from_index(struct index_state
*, const char *, unsig
#define CE_MATCH_RACY_IS_DIRTY 02
/* do stat comparison even if CE_SKIP_WORKTREE is true */
#define CE_MATCH_IGNORE_SKIP_WORKTREE 04
-extern int ie_match_stat(const struct index_state *, const struct cache_entry
*, struct stat *, unsigned int);
-extern int ie_modified(const struct index_state *, const struct cache_entry *,
struct stat *, unsigned int);
+extern int ie_match_stat(struct index_state *, const struct cache_entry *,
struct stat *, unsigned int);
+extern int ie_modified(struct index_state *, const struct cache_entry *,
struct stat *, unsigned int);
#define PATHSPEC_ONESTAR 1 /* the pathspec pattern sastisfies GFNM_ONESTAR
*/
diff --git a/read-cache-v2.c b/read-cache-v2.c
new file mode 100644
index 000..a6883c3
--- /dev/null
+++ b/read-cache-v2.c
@@ -0,0 +1,556 @@
+#include "cache.h"
+#include "read-cache.h"
+#include "resolve-undo.h"
+#include "cache-tree.h"
+#include "varint.h"
+
+/* Mask for the name length in ce_flags in the on-disk index */
+#define CE_NAMEMASK (0x0fff)
+
+struct cache_header {
+ unsigned int hdr_entries;
+};
+
+/*
+ * Index File I/O
+ */
+
+/*
+ * dev/ino/uid/gid/size are also just tracked to the low 32 bits
+ * Again - this is just a (very strong in practice) heuristic that
+ * the inode hasn't changed.
+ *
+ * We save the fields in big-endian order to allow using the
+ * index file over NFS transparently.
+ */
+struct ondisk_cache_entry {
+ struct cache_time ctime;
+ struct cache_time mtime;
+ unsigned int dev;
+ unsigned int ino;
+ unsigned int mode;
+ unsigned int uid;
+ unsigned int gid;
+ unsigned int size;
+ unsigned char sha1[20];
+ unsigned short flags;
+ char name[FLEX_ARRAY]; /* more */
+};
+
+/*
+ * This struct is used when CE_EXTENDED bit is 1
+ * The struct must match ondisk_cache_entry exactly from
+ * ctime till flags
+ */
+struct ondisk_cache_entry_extended {
+ struct cache_time ctime;
+ struct cache_time mtime;
+ unsigned int dev;
+ unsigned int ino;
+ unsigned int mode;
+ unsigned int uid;
+ unsigned int gid;
+ unsigned int size;
+ unsigned char sha1[20];
+ unsigned short flags;
+ unsigned short flags2;
+ char name[FLEX_ARRAY]; /* more */
+};
+
+/* These are only used for v3 or lower */
+#define align_flex_name(STRUCT,len) ((offsetof(struct STRUCT,name) + (len) +
8) & ~7)
+#define ondisk_cache_entry_size(len) align_flex_name(ondisk_cache_entry,len)
+#define ondisk_cache_entry_extended_size(len)
align_flex_name(ondisk_cache_entry_extended,len)
+#define ondisk_ce_size(ce) (((ce)->ce_flags & CE_EXTENDED) ? \
+ ondisk_cache_entry_extended_size(ce_namelen(ce)) : \
+ ondisk_cache_entry_size(ce_namelen(ce)))
+
+static int veri
