Re: [PATCH v4 23/24] POC for partial writing

2013-11-30 Thread Duy Nguyen
On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
 This makes update-index use both partial reading and partial writing.
 Partial reading is only used no option other than the paths is passed to
 the command.

 This passes the test suite,

Just checking, the test suite was run with TEST_GIT_INDEX_VERSION=5, right?

 but doesn't behave correctly when a write
 fails.  A log should be written to the lock file, in order to be able to
 recover if a write fails.

From the API point of view this looks nice (you should have hidden
needs_write = 1 in cache_invalidate_path and change_cache_version
though)  We could support partial file removal too by marking removed
files removed, but that impacts the reading code and may have bad
interaction with cache_invalidate_path/needs_rewrite. Probably not
worth the effort until someone shows us they remove stuff often.

 ---
  builtin/update-index.c |  43 +++---
  cache-tree.c   |  13 +
  cache-tree.h   |   1 +
  cache.h|  27 -
  lockfile.c |   2 +-
  read-cache-v2.c|   2 +
  read-cache-v5.c| 154 
 -
  read-cache.c   |  30 ++
  read-cache.h   |   1 +
  resolve-undo.c |   1 +
  10 files changed, 237 insertions(+), 37 deletions(-)

 diff --git a/builtin/update-index.c b/builtin/update-index.c
 index 8b3f7a0..69f0949 100644
 --- a/builtin/update-index.c
 +++ b/builtin/update-index.c
 @@ -56,6 +56,7 @@ static int mark_ce_flags(const char *path, int flag, int 
 mark)
 else
 active_cache[pos]-ce_flags = ~flag;
 cache_tree_invalidate_path(active_cache_tree, path);
 +   the_index.needs_rewrite = 1;
 active_cache_changed = 1;
 return 0;
 }
 @@ -99,6 +100,8 @@ static int add_one_path(const struct cache_entry *old, 
 const char *path, int len
 memcpy(ce-name, path, len);
 ce-ce_flags = create_ce_flags(0);
 ce-ce_namelen = len;
 +   if (old)
 +   ce-entry_pos = old-entry_pos;
 fill_stat_cache_info(ce, st);
 ce-ce_mode = ce_mode_from_stat(old, st-st_mode);

 @@ -268,6 +271,7 @@ static void chmod_path(int flip, const char *path)
 goto fail;
 }
 cache_tree_invalidate_path(active_cache_tree, path);
 +   the_index.needs_rewrite = 1;
 active_cache_changed = 1;
 report(chmod %cx '%s', flip, path);
 return;
 @@ -706,15 +710,18 @@ static int reupdate_callback(struct parse_opt_ctx_t 
 *ctx,

  int cmd_update_index(int argc, const char **argv, const char *prefix)
  {
 -   int newfd, entries, has_errors = 0, line_termination = '\n';
 +   int newfd, has_errors = 0, line_termination = '\n';
 int read_from_stdin = 0;
 int prefix_length = prefix ? strlen(prefix) : 0;
 int preferred_index_format = 0;
 char set_executable_bit = 0;
 struct refresh_params refresh_args = {0, has_errors};
 int lock_error = 0;
 +   struct filter_opts opts;
 +   struct pathspec pathspec;
 struct lock_file *lock_file;
 struct parse_opt_ctx_t ctx;
 +   int i, needs_full_read = 0;
 int parseopt_state = PARSE_OPT_UNKNOWN;
 struct option options[] = {
 OPT_BIT('q', NULL, refresh_args.flags,
 @@ -810,9 +817,23 @@ int cmd_update_index(int argc, const char **argv, const 
 char *prefix)
 if (newfd  0)
 lock_error = errno;

 -   entries = read_cache();
 -   if (entries  0)
 -   die(cache corrupted);
 +   for (i = 0; i  argc; i++) {
 +   if (!prefixcmp(argv[i], --))
 +   needs_full_read = 1;
 +   }
 +   if (!needs_full_read) {
 +   memset(opts, 0, sizeof(struct filter_opts));
 +   parse_pathspec(pathspec, 0,
 +  PATHSPEC_PREFER_CWD |
 +  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 +  prefix, argv + 1);
 +   opts.pathspec = pathspec;
 +   if (read_cache_filtered(opts)  0)
 +   die(cache corrupted);
 +   } else {
 +   if (read_cache()  0)
 +   die(cache corrupted);
 +   }

 /*
  * Custom copy of parse_options() because we want to handle
 @@ -862,6 +883,7 @@ int cmd_update_index(int argc, const char **argv, const 
 char *prefix)
 preferred_index_format,
 INDEX_FORMAT_LB, INDEX_FORMAT_UB);

 +   the_index.needs_rewrite = 1;
 active_cache_changed = 1;
 change_cache_version(preferred_index_format);
 }
 @@ -890,17 +912,22 @@ int cmd_update_index(int argc, const char **argv, const 
 char *prefix)
 }

 if 

Re: [PATCH v4 23/24] POC for partial writing

2013-11-30 Thread Thomas Gummerer
Duy Nguyen pclo...@gmail.com writes:

 On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer t.gumme...@gmail.com wrote:
 This makes update-index use both partial reading and partial writing.
 Partial reading is only used no option other than the paths is passed to
 the command.

 This passes the test suite,

 Just checking, the test suite was run with TEST_GIT_INDEX_VERSION=5, right?

Yes, sure, I've been using that for finding problems in my approach.

 but doesn't behave correctly when a write
 fails.  A log should be written to the lock file, in order to be able to
 recover if a write fails.

 From the API point of view this looks nice (you should have hidden
 needs_write = 1 in cache_invalidate_path and change_cache_version
 though).

Ok, thanks, I'll look into that, once I have time to make more than a
POC for this.

 We could support partial file removal too by marking removed
 files removed, but that impacts the reading code and may have bad
 interaction with cache_invalidate_path/needs_rewrite. Probably not
 worth the effort until someone shows us they remove stuff often.

Yes, right, I think it could be done, but I didn't take the time to look
into that for now.  The reading code should actually be fine as it is,
as it already takes care of entries with the removed flag, but I'm not
sure about cache_tree_invalidate_path/needs_rewrite.

 ---
  builtin/update-index.c |  43 +++---
  cache-tree.c   |  13 +
  cache-tree.h   |   1 +
  cache.h|  27 -
  lockfile.c |   2 +-
  read-cache-v2.c|   2 +
  read-cache-v5.c| 154 
 -
  read-cache.c   |  30 ++
  read-cache.h   |   1 +
  resolve-undo.c |   1 +
  10 files changed, 237 insertions(+), 37 deletions(-)

 diff --git a/builtin/update-index.c b/builtin/update-index.c
 index 8b3f7a0..69f0949 100644
 --- a/builtin/update-index.c
 +++ b/builtin/update-index.c
 @@ -56,6 +56,7 @@ static int mark_ce_flags(const char *path, int flag, int 
 mark)
 else
 active_cache[pos]-ce_flags = ~flag;
 cache_tree_invalidate_path(active_cache_tree, path);
 +   the_index.needs_rewrite = 1;
 active_cache_changed = 1;
 return 0;
 }
 @@ -99,6 +100,8 @@ static int add_one_path(const struct cache_entry *old, 
 const char *path, int len
 memcpy(ce-name, path, len);
 ce-ce_flags = create_ce_flags(0);
 ce-ce_namelen = len;
 +   if (old)
 +   ce-entry_pos = old-entry_pos;
 fill_stat_cache_info(ce, st);
 ce-ce_mode = ce_mode_from_stat(old, st-st_mode);

 @@ -268,6 +271,7 @@ static void chmod_path(int flip, const char *path)
 goto fail;
 }
 cache_tree_invalidate_path(active_cache_tree, path);
 +   the_index.needs_rewrite = 1;
 active_cache_changed = 1;
 report(chmod %cx '%s', flip, path);
 return;
 @@ -706,15 +710,18 @@ static int reupdate_callback(struct parse_opt_ctx_t 
 *ctx,

  int cmd_update_index(int argc, const char **argv, const char *prefix)
  {
 -   int newfd, entries, has_errors = 0, line_termination = '\n';
 +   int newfd, has_errors = 0, line_termination = '\n';
 int read_from_stdin = 0;
 int prefix_length = prefix ? strlen(prefix) : 0;
 int preferred_index_format = 0;
 char set_executable_bit = 0;
 struct refresh_params refresh_args = {0, has_errors};
 int lock_error = 0;
 +   struct filter_opts opts;
 +   struct pathspec pathspec;
 struct lock_file *lock_file;
 struct parse_opt_ctx_t ctx;
 +   int i, needs_full_read = 0;
 int parseopt_state = PARSE_OPT_UNKNOWN;
 struct option options[] = {
 OPT_BIT('q', NULL, refresh_args.flags,
 @@ -810,9 +817,23 @@ int cmd_update_index(int argc, const char **argv, const 
 char *prefix)
 if (newfd  0)
 lock_error = errno;

 -   entries = read_cache();
 -   if (entries  0)
 -   die(cache corrupted);
 +   for (i = 0; i  argc; i++) {
 +   if (!prefixcmp(argv[i], --))
 +   needs_full_read = 1;
 +   }
 +   if (!needs_full_read) {
 +   memset(opts, 0, sizeof(struct filter_opts));
 +   parse_pathspec(pathspec, 0,
 +  PATHSPEC_PREFER_CWD |
 +  PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 +  prefix, argv + 1);
 +   opts.pathspec = pathspec;
 +   if (read_cache_filtered(opts)  0)
 +   die(cache corrupted);
 +   } else {
 +   if (read_cache()  0)
 +   die(cache corrupted);
 +   }

 /*
  * Custom copy of parse_options() because we want to handle
 @@ -862,6 +883,7 @@ int