* [PATCH 0/2] nd/commit-ignore-i-t-a replacement
@ 2012-01-16 2:36 Nguyễn Thái Ngọc Duy
2012-01-16 2:36 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy
2012-01-16 2:36 ` [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees Nguyễn Thái Ngọc Duy
0 siblings, 2 replies; 10+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2012-01-16 2:36 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Junio C Hamano,
Nguyễn Thái Ngọc Duy
This replaces current topic branch in pu with a more sensible
approach using config key. write-tree also learns about this.
Nguyễn Thái Ngọc Duy (2):
cache-tree: update API to take abitrary flags
commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing
trees
Documentation/config.txt | 5 +++++
Documentation/git-add.txt | 12 ++++++++++--
Documentation/git-write-tree.txt | 8 +++++++-
builtin/commit.c | 13 ++++++++++---
builtin/write-tree.c | 2 ++
cache-tree.c | 35 +++++++++++++++++------------------
cache-tree.h | 5 ++++-
merge-recursive.c | 2 +-
t/t2203-add-intent.sh | 30 ++++++++++++++++++++++++++++++
test-dump-cache-tree.c | 2 +-
10 files changed, 87 insertions(+), 27 deletions(-)
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] cache-tree: update API to take abitrary flags 2012-01-16 2:36 [PATCH 0/2] nd/commit-ignore-i-t-a replacement Nguyễn Thái Ngọc Duy @ 2012-01-16 2:36 ` Nguyễn Thái Ngọc Duy 2012-01-16 2:36 ` [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees Nguyễn Thái Ngọc Duy 1 sibling, 0 replies; 10+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2012-01-16 2:36 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano, Nguyễn Thái Ngọc Duy Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/commit.c | 4 ++-- cache-tree.c | 27 ++++++++++++--------------- cache-tree.h | 4 +++- merge-recursive.c | 2 +- test-dump-cache-tree.c | 2 +- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index eba1377..bf42bb3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); add_files_to_cache(also ? prefix : NULL, pathspec, 0); refresh_cache_or_die(refresh_flags); - update_main_cache_tree(1); + update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || close_lock_file(&index_lock)) die(_("unable to write new_index file")); @@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); refresh_cache_or_die(refresh_flags); if (active_cache_changed) { - update_main_cache_tree(1); + update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || commit_locked_index(&index_lock)) die(_("unable to write new_index file")); diff --git a/cache-tree.c b/cache-tree.c index 8de3959..16355d6 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path) } static int verify_cache(struct cache_entry **cache, - int entries, int silent) + int entries, int flags) { int i, funny; + int silent = flags & WRITE_TREE_SILENT; /* Verify that the tree is merged */ funny = 0; @@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it, int entries, const char *base, int baselen, - int missing_ok, - int dryrun) + int flags) { struct strbuf buffer; + int missing_ok = flags & WRITE_TREE_MISSING_OK; + int dryrun = flags & WRITE_TREE_DRY_RUN; int i; if (0 <= it->entry_count && has_sha1_file(it->sha1)) @@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it, cache + i, entries - i, path, baselen + sublen + 1, - missing_ok, - dryrun); + flags); if (subcnt < 0) return subcnt; i += subcnt - 1; @@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it, int cache_tree_update(struct cache_tree *it, struct cache_entry **cache, int entries, - int missing_ok, - int dryrun, - int silent) + int flags) { int i; - i = verify_cache(cache, entries, silent); + i = verify_cache(cache, entries, flags); if (i) return i; - i = update_one(it, cache, entries, "", 0, missing_ok, dryrun); + i = update_one(it, cache, entries, "", 0, flags); if (i < 0) return i; return 0; @@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) was_valid = cache_tree_fully_valid(active_cache_tree); if (!was_valid) { - int missing_ok = flags & WRITE_TREE_MISSING_OK; - if (cache_tree_update(active_cache_tree, active_cache, active_nr, - missing_ok, 0, 0) < 0) + flags) < 0) return WRITE_TREE_UNMERGED_INDEX; if (0 <= newfd) { if (!write_cache(newfd, active_cache, active_nr) && @@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root, return 0; } -int update_main_cache_tree (int silent) +int update_main_cache_tree (int flags) { if (!the_index.cache_tree) the_index.cache_tree = cache_tree(); return cache_tree_update(the_index.cache_tree, - the_index.cache, the_index.cache_nr, 0, 0, silent); + the_index.cache, the_index.cache_nr, flags); } diff --git a/cache-tree.h b/cache-tree.h index 0ec0b2a..d8cb2e9 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root); struct cache_tree *cache_tree_read(const char *buffer, unsigned long size); int cache_tree_fully_valid(struct cache_tree *); -int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int); +int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int); int update_main_cache_tree(int); /* bitmasks to write_cache_as_tree flags */ #define WRITE_TREE_MISSING_OK 1 #define WRITE_TREE_IGNORE_CACHE_TREE 2 +#define WRITE_TREE_DRY_RUN 4 +#define WRITE_TREE_SILENT 8 /* error return codes */ #define WRITE_TREE_UNREADABLE_INDEX (-1) diff --git a/merge-recursive.c b/merge-recursive.c index d83cd6c..6479a60 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o) if (!cache_tree_fully_valid(active_cache_tree) && cache_tree_update(active_cache_tree, - active_cache, active_nr, 0, 0, 0) < 0) + active_cache, active_nr, 0) < 0) die("error building trees"); result = lookup_tree(active_cache_tree->sha1); diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c index e6c2923..a6ffdf3 100644 --- a/test-dump-cache-tree.c +++ b/test-dump-cache-tree.c @@ -59,6 +59,6 @@ int main(int ac, char **av) struct cache_tree *another = cache_tree(); if (read_cache() < 0) die("unable to read index file"); - cache_tree_update(another, active_cache, active_nr, 0, 1, 0); + cache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN); return dump_cache_tree(active_cache_tree, another, ""); } -- 1.7.3.1.256.g2539c.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees 2012-01-16 2:36 [PATCH 0/2] nd/commit-ignore-i-t-a replacement Nguyễn Thái Ngọc Duy 2012-01-16 2:36 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy @ 2012-01-16 2:36 ` Nguyễn Thái Ngọc Duy 2012-01-16 16:46 ` Pete Wyckoff 2012-01-16 23:21 ` Junio C Hamano 1 sibling, 2 replies; 10+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2012-01-16 2:36 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, Junio C Hamano, Nguyễn Thái Ngọc Duy Normally cache-tree will not produce trees from an index that has CE_INTENT_TO_ADD entries. This is a safe measure to avoid mis-interpreting user's intention regarding this flag. There are situations however where users want to create trees/commits regardless i-t-a entries. Allow such cases with commit.ignoreIntentToAdd for git-commit and --ignore-intent-to-add for git-write-tree. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Documentation/config.txt | 5 +++++ Documentation/git-add.txt | 12 ++++++++++-- Documentation/git-write-tree.txt | 8 +++++++- builtin/commit.c | 13 ++++++++++--- builtin/write-tree.c | 2 ++ cache-tree.c | 8 +++++--- cache-tree.h | 1 + t/t2203-add-intent.sh | 30 ++++++++++++++++++++++++++++++ 8 files changed, 70 insertions(+), 9 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index abeb82b..10c3767 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -831,6 +831,11 @@ commit.template:: "{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the specified user's home directory. +commit.ignoreIntentToAdd:: + Allow to commit the index as-is even if there are + intent-to-add entries (see option `-N` in linkgit:git-add[1]) + in index. + credential.helper:: Specify an external helper to be called when a username or password credential is needed; the helper may consult external diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 9c1d395..ec548ea 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -123,8 +123,16 @@ subdirectories. Record only the fact that the path will be added later. An entry for the path is placed in the index with no content. This is useful for, among other things, showing the unstaged content of - such files with `git diff` and committing them with `git commit - -a`. + such files with `git diff`. ++ +Paths added with this option have intent-to-add flag in index. The +flag is removed once real content is added or updated. By default you +cannot commit the index as-is from until this flag is removed from all +entries (i.e. all entries have real content). See commit.ignoreIntentToAdd +regardless the flag. ++ +Committing with `git commit -a` or with selected paths works +regardless the config key and the flag. --refresh:: Don't add the file(s), but only refresh their stat() diff --git a/Documentation/git-write-tree.txt b/Documentation/git-write-tree.txt index f22041a..d511d3a 100644 --- a/Documentation/git-write-tree.txt +++ b/Documentation/git-write-tree.txt @@ -9,7 +9,7 @@ git-write-tree - Create a tree object from the current index SYNOPSIS -------- [verse] -'git write-tree' [--missing-ok] [--prefix=<prefix>/] +'git write-tree' [--missing-ok] [--ignore-intent-to-add] [--prefix=<prefix>/] DESCRIPTION ----------- @@ -32,6 +32,12 @@ OPTIONS directory exist in the object database. This option disables this check. +--ignore-intent-to-add:: + Normally 'git write-tree' will refuse to proceed if there are any + intent-to-add entries (see `-N` option in linkgit:git-add[1]). + This option continues to create object tree as if there are no + intent-to-add entries in index. + --prefix=<prefix>/:: Writes a tree object that represents a subdirectory `<prefix>`. This can be used to write the tree object diff --git a/builtin/commit.c b/builtin/commit.c index bf42bb3..097699e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff; static int edit_flag = -1; /* unspecified */ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; static int no_post_rewrite, allow_empty_message; +static int cache_tree_flags; static char *untracked_files_arg, *force_date, *ignore_submodule_arg; static char *sign_commit; @@ -400,7 +401,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); add_files_to_cache(also ? prefix : NULL, pathspec, 0); refresh_cache_or_die(refresh_flags); - update_main_cache_tree(WRITE_TREE_SILENT); + update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || close_lock_file(&index_lock)) die(_("unable to write new_index file")); @@ -421,7 +422,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); refresh_cache_or_die(refresh_flags); if (active_cache_changed) { - update_main_cache_tree(WRITE_TREE_SILENT); + update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || commit_locked_index(&index_lock)) die(_("unable to write new_index file")); @@ -870,7 +871,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, */ discard_cache(); read_cache_from(index_file); - if (update_main_cache_tree(0)) { + if (update_main_cache_tree(cache_tree_flags)) { error(_("Error building trees")); return 0; } @@ -1338,6 +1339,12 @@ static int git_commit_config(const char *k, const char *v, void *cb) include_status = git_config_bool(k, v); return 0; } + if (!strcmp(k, "commit.ignoreintenttoadd")) { + if (git_config_bool(k, v)) + cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD; + else + cache_tree_flags &= ~WRITE_TREE_IGNORE_INTENT_TO_ADD; + } status = git_gpg_config(k, v, NULL); if (status) diff --git a/builtin/write-tree.c b/builtin/write-tree.c index b223af4..07797bb 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -23,6 +23,8 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix) struct option write_tree_options[] = { OPT_BIT(0, "missing-ok", &flags, "allow missing objects", WRITE_TREE_MISSING_OK), + OPT_BIT(0, "ignore-intent-to-add", &flags, "ignore i-t-a entries in index", + WRITE_TREE_IGNORE_INTENT_TO_ADD ), { OPTION_STRING, 0, "prefix", &prefix, "<prefix>/", "write tree object for a subdirectory <prefix>" , PARSE_OPT_LITERAL_ARGHELP }, diff --git a/cache-tree.c b/cache-tree.c index 16355d6..d0be159 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -159,7 +159,9 @@ static int verify_cache(struct cache_entry **cache, funny = 0; for (i = 0; i < entries; i++) { struct cache_entry *ce = cache[i]; - if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) { + if (ce_stage(ce) || + ((flags & WRITE_TREE_IGNORE_INTENT_TO_ADD) == 0 && + (ce->ce_flags & CE_INTENT_TO_ADD))) { if (silent) return -1; if (10 < ++funny) { @@ -339,8 +341,8 @@ static int update_one(struct cache_tree *it, mode, sha1_to_hex(sha1), entlen+baselen, path); } - if (ce->ce_flags & CE_REMOVE) - continue; /* entry being removed */ + if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD)) + continue; /* entry being removed or placeholder */ strbuf_grow(&buffer, entlen + 100); strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); diff --git a/cache-tree.h b/cache-tree.h index d8cb2e9..af3b917 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -38,6 +38,7 @@ int update_main_cache_tree(int); #define WRITE_TREE_IGNORE_CACHE_TREE 2 #define WRITE_TREE_DRY_RUN 4 #define WRITE_TREE_SILENT 8 +#define WRITE_TREE_IGNORE_INTENT_TO_ADD 16 /* error return codes */ #define WRITE_TREE_UNREADABLE_INDEX (-1) diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 58a3299..7b447d3 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -44,6 +44,36 @@ test_expect_success 'cannot commit with i-t-a entry' ' test_must_fail git commit ' +test_expect_success 'can write-tree with i-t-a entry' ' + git reset --hard && + echo xyzzy >rezrov && + echo frotz >nitfol && + git add rezrov && + git add -N nitfol && + git write-tree --ignore-intent-to-add >actual && + echo 150389afa1ccf46e4104667c741cd0e598269511 >expected && + test_cmp expected actual +' + +test_expect_success 'can commit tree with i-t-a entry' ' + git reset --hard && + echo xyzzy >rezrov && + echo frotz >nitfol && + git add rezrov && + git add -N nitfol && + git config commit.ignoreIntentToAdd true && + git commit -m initial && + git ls-tree -r HEAD >actual && + cat >expected <<EOF && +100644 blob ce013625030ba8dba906f756967f9e9ca394464a elif +100644 blob ce013625030ba8dba906f756967f9e9ca394464a file +100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4 rezrov +EOF + test_cmp expected actual && + git config commit.ignoreIntentToAdd false && + git reset HEAD^ +' + test_expect_success 'can commit with an unrelated i-t-a entry in index' ' git reset --hard && echo xyzzy >rezrov && -- 1.7.3.1.256.g2539c.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees 2012-01-16 2:36 ` [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees Nguyễn Thái Ngọc Duy @ 2012-01-16 16:46 ` Pete Wyckoff 2012-01-16 23:21 ` Junio C Hamano 1 sibling, 0 replies; 10+ messages in thread From: Pete Wyckoff @ 2012-01-16 16:46 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy Cc: git, Jonathan Nieder, Junio C Hamano pclouds@gmail.com wrote on Mon, 16 Jan 2012 09:36 +0700: > Normally cache-tree will not produce trees from an index that has > CE_INTENT_TO_ADD entries. This is a safe measure to avoid > mis-interpreting user's intention regarding this flag. > > There are situations however where users want to create trees/commits > regardless i-t-a entries. Allow such cases with commit.ignoreIntentToAdd > for git-commit and --ignore-intent-to-add for git-write-tree. Recently I tried to use "--intent-to-add" on a new file, but when committing was annoyed by the confusing error, and that I was forced to do something with that new file. With commit.ignoreIntentToAdd I can happily commit while leaving the new file for later. It stays in the index and is easy to see in "git status". I don't understand the need for an option in write-tree; just the configuration variable is required. Here's some changes to the docs you might squash in. It took me a while to figure out what this variable was about, and I tried to explain it more clearly for a non-developer audience. -- Pete --------8<------------ From 2471de7083ca3198f59a4734c0d11e9446874de1 Mon Sep 17 00:00:00 2001 From: Pete Wyckoff <pw@padd.com> Date: Mon, 16 Jan 2012 11:44:26 -0500 Subject: [PATCH] documentation for commit.ignoreIntentToAdd --- Documentation/config.txt | 13 ++++++++----- Documentation/git-add.txt | 15 ++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 7ba8777..a2cbb50 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -821,6 +821,14 @@ color.ui:: `never` if you prefer git commands not to use color unless enabled explicitly with some other configuration or the `--color` option. +commit.ignoreIntentToAdd:: + When 'git add' is invoked with `-N`, an "intent-to-add" entry is + made in the index. At commit time, these entries must be removed + from the index ("git reset ...") or added ("git add ..."). This + boolean variable makes it possible to commit while leaving the + "intent-to-add" entries still in the index. See the description + of the `-N` option in linkgit:git-add[1] for details. + commit.status:: A boolean to enable/disable inclusion of status information in the commit message template when using an editor to prepare the commit @@ -831,11 +839,6 @@ commit.template:: "{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the specified user's home directory. -commit.ignoreIntentToAdd:: - Allow to commit the index as-is even if there are - intent-to-add entries (see option `-N` in linkgit:git-add[1]) - in index. - credential.helper:: Specify an external helper to be called when a username or password credential is needed; the helper may consult external diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index ec548ea..1c2ac44 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -125,14 +125,15 @@ subdirectories. useful for, among other things, showing the unstaged content of such files with `git diff`. + -Paths added with this option have intent-to-add flag in index. The -flag is removed once real content is added or updated. By default you -cannot commit the index as-is from until this flag is removed from all -entries (i.e. all entries have real content). See commit.ignoreIntentToAdd -regardless the flag. +When committing, these paths must be either added to the index (without +the `-N` flag) or removed (with "git reset"). However, a configuration +variable `commit.ignoreIntentToAdd` can be set to allow commits to +proceed, while the intent-to-add paths remain in the index. + -Committing with `git commit -a` or with selected paths works -regardless the config key and the flag. +Regardless of the configuration variable, invoking `git commit -a` will +commit all files including the ones marked with intent-to-add. +Specifying a <filepattern> can be used to commit files other than the +ones marked with intent-to-add. --refresh:: Don't add the file(s), but only refresh their stat() -- 1.7.9.rc0.18.gdae96 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees 2012-01-16 2:36 ` [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees Nguyễn Thái Ngọc Duy 2012-01-16 16:46 ` Pete Wyckoff @ 2012-01-16 23:21 ` Junio C Hamano 2012-01-17 1:50 ` Nguyen Thai Ngoc Duy 1 sibling, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2012-01-16 23:21 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Nieder Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > Normally cache-tree will not produce trees from an index that has > CE_INTENT_TO_ADD entries. This is a safe measure to avoid > mis-interpreting user's intention regarding this flag. s/safe/safety/; > There are situations however where users want to create trees/commits > regardless i-t-a entries. A new command line option "--no-check-intent-to-add" to "commit" without any configuration bit may be a useful addition as a first cut, and in order to help users to which "there are situations" is more than 50% of the time, a configuration that can be overriden by "--check-intent-to-add" may be a usability improvement over that first cut, but if this is really about "there are situations", then a configuration that cannot be overriden by command line option feels a wrong way to go about it. Is this really about "there are situations" to begin with? I am suspecting that this patch is either: (1) making it easier to use a wrong workflow, by promoting a way to bypass a useful safety measure; (2) fixing an earlier UI mistake (iow, the interpretation #2 in the old discussion is always the right one and the existing safety measure is misguided) in such a way that allows you to work around an objection from a bonehead maintainer who refuses to admit that earlier mistake; and/or (3) splitting the Git userbase into two and making the resulting system harder to teach. If it is (2), and I suspect it may be the case, we might want to rather honestly describe that the future direction is to fix it, and describe the configuration option as "an early opt-in" switch, together with the usual three-step deprecation and migration schedule to make the new behaviour the default in a future version. From the timeline point of view, it probably can coincide with the change to always start an editor when recording a merge commit. In any case, for this change to help people who add more than one paths with "add -N" and want to include only a subset of them in the commit, we may want to explicitly teach them to add what they want to before committing with the new command line option in the documentation. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees 2012-01-16 23:21 ` Junio C Hamano @ 2012-01-17 1:50 ` Nguyen Thai Ngoc Duy 2012-01-17 2:47 ` Jonathan Nieder 0 siblings, 1 reply; 10+ messages in thread From: Nguyen Thai Ngoc Duy @ 2012-01-17 1:50 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jonathan Nieder 2012/1/17 Junio C Hamano <gitster@pobox.com>: > A new command line option "--no-check-intent-to-add" to "commit" without > any configuration bit may be a useful addition as a first cut, and in > order to help users to which "there are situations" is more than 50% of > the time, a configuration that can be overriden by "--check-intent-to-add" > may be a usability improvement over that first cut, but if this is really > about "there are situations", then a configuration that cannot be > overriden by command line option feels a wrong way to go about it. "git -c key=value commit" may help. > Is this really about "there are situations" to begin with? I am suspecting > that this patch is either: > > (1) making it easier to use a wrong workflow, by promoting a way to > bypass a useful safety measure; > > (2) fixing an earlier UI mistake (iow, the interpretation #2 in the old > discussion is always the right one and the existing safety measure is > misguided) in such a way that allows you to work around an objection > from a bonehead maintainer who refuses to admit that earlier mistake; > and/or > > (3) splitting the Git userbase into two and making the resulting system > harder to teach. This patch is towards (3). I agree that "add -N" can be confusing to new users because it does not actually add anything. Those who do not read manual carefully and commit without checking the result may fall into that trap. So a user is expected to "level up" a bit and hopefully by the time he/she feels the need to get around the safety check and discover this flag, they should be ready to go without the safety check. Although I would not oppose deprecating the old behavior if you go with (2) ;) > In any case, for this change to help people who add more than one paths > with "add -N" and want to include only a subset of them in the commit, we > may want to explicitly teach them to add what they want to before > committing with the new command line option in the documentation. yeah, keep telling people "this does not add any thing, you need to git-add again without -N" after running "add -N" using advice framework when this config is on? -- Duy ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees 2012-01-17 1:50 ` Nguyen Thai Ngoc Duy @ 2012-01-17 2:47 ` Jonathan Nieder 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Nieder @ 2012-01-17 2:47 UTC (permalink / raw) To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git Nguyen Thai Ngoc Duy wrote: > Although I would not oppose deprecating the old behavior if you go with (2) ;) My reaction on reading the patch and clarifying followup was similar to Junio's. I don't think a configuration item makes sense unless we are planning to flip the default in the future, and it would probably be clearer to document it that way. By the way, thanks very much for working on this. I wish I had more time to offer more than comments. Sincerely, Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH RFC] commit: allow to commit even if there are intent-to-add entries @ 2012-01-11 6:01 Nguyễn Thái Ngọc Duy 2012-01-11 9:59 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy 0 siblings, 1 reply; 10+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2012-01-11 6:01 UTC (permalink / raw) To: git; +Cc: Nguyễn Thái Ngọc Duy This patch replaces the approach in 331fcb5 (git add --intent-to-add: do not let an empty blob be committed by accident) regarding i-t-a entries: instead of forbidding i-t-a entries at commit time, we can simply ignore them. We already ignore CE_REMOVE entries while updating cache-tree. Putting CE_INTENT_TO_ADD ones in the same category should not cause any negative effects regarding cache-tree. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- On the few chances I have to use "git add -N" it does not fit well with "git add -p; git diff --cached; git commit -m foo" style. I think this may be a good thing to do. builtin/commit.c | 2 +- builtin/write-tree.c | 2 +- cache-tree.c | 14 +++++--------- t/t2203-add-intent.sh | 10 +++++++++- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index eba1377..767b78a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -871,7 +871,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, discard_cache(); read_cache_from(index_file); if (update_main_cache_tree(0)) { - error(_("Error building trees")); + error(_("Error building trees; the index is unmerged?")); return 0; } diff --git a/builtin/write-tree.c b/builtin/write-tree.c index b223af4..68baa24 100644 --- a/builtin/write-tree.c +++ b/builtin/write-tree.c @@ -46,7 +46,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix) die("%s: error reading the index", me); break; case WRITE_TREE_UNMERGED_INDEX: - die("%s: error building trees", me); + die("%s: error building trees; the index is unmerged?", me); break; case WRITE_TREE_PREFIX_ERROR: die("%s: prefix %s not found", me, prefix); diff --git a/cache-tree.c b/cache-tree.c index 8de3959..47defd1 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -158,19 +158,15 @@ static int verify_cache(struct cache_entry **cache, funny = 0; for (i = 0; i < entries; i++) { struct cache_entry *ce = cache[i]; - if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) { + if (ce_stage(ce)) { if (silent) return -1; if (10 < ++funny) { fprintf(stderr, "...\n"); break; } - if (ce_stage(ce)) - fprintf(stderr, "%s: unmerged (%s)\n", - ce->name, sha1_to_hex(ce->sha1)); - else - fprintf(stderr, "%s: not added yet\n", - ce->name); + fprintf(stderr, "%s: unmerged (%s)\n", + ce->name, sha1_to_hex(ce->sha1)); } } if (funny) @@ -338,8 +334,8 @@ static int update_one(struct cache_tree *it, mode, sha1_to_hex(sha1), entlen+baselen, path); } - if (ce->ce_flags & CE_REMOVE) - continue; /* entry being removed */ + if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD)) + continue; /* entry being removed or just placeholder */ strbuf_grow(&buffer, entlen + 100); strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh index 2543529..65430e4 100755 --- a/t/t2203-add-intent.sh +++ b/t/t2203-add-intent.sh @@ -41,7 +41,15 @@ test_expect_success 'cannot commit with i-t-a entry' ' echo frotz >nitfol && git add rezrov && git add -N nitfol && - test_must_fail git commit -m initial + git commit -m initial && + git ls-tree -r HEAD >actual && + cat >expected <<EOF && +100644 blob ce013625030ba8dba906f756967f9e9ca394464a elif +100644 blob ce013625030ba8dba906f756967f9e9ca394464a file +100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4 rezrov +EOF + test_cmp expected actual && + git reset HEAD^ ' test_expect_success 'can commit with an unrelated i-t-a entry in index' ' -- 1.7.3.1.256.g2539c.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/2] cache-tree: update API to take abitrary flags 2012-01-11 6:01 [PATCH RFC] commit: allow to commit even if there are intent-to-add entries Nguyễn Thái Ngọc Duy @ 2012-01-11 9:59 ` Nguyễn Thái Ngọc Duy 2012-01-11 23:48 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2012-01-11 9:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Nguyễn Thái Ngọc Duy --- builtin/commit.c | 4 ++-- cache-tree.c | 27 ++++++++++++--------------- cache-tree.h | 4 +++- merge-recursive.c | 2 +- test-dump-cache-tree.c | 2 +- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index eba1377..bf42bb3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); add_files_to_cache(also ? prefix : NULL, pathspec, 0); refresh_cache_or_die(refresh_flags); - update_main_cache_tree(1); + update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || close_lock_file(&index_lock)) die(_("unable to write new_index file")); @@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, fd = hold_locked_index(&index_lock, 1); refresh_cache_or_die(refresh_flags); if (active_cache_changed) { - update_main_cache_tree(1); + update_main_cache_tree(WRITE_TREE_SILENT); if (write_cache(fd, active_cache, active_nr) || commit_locked_index(&index_lock)) die(_("unable to write new_index file")); diff --git a/cache-tree.c b/cache-tree.c index 8de3959..16355d6 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path) } static int verify_cache(struct cache_entry **cache, - int entries, int silent) + int entries, int flags) { int i, funny; + int silent = flags & WRITE_TREE_SILENT; /* Verify that the tree is merged */ funny = 0; @@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it, int entries, const char *base, int baselen, - int missing_ok, - int dryrun) + int flags) { struct strbuf buffer; + int missing_ok = flags & WRITE_TREE_MISSING_OK; + int dryrun = flags & WRITE_TREE_DRY_RUN; int i; if (0 <= it->entry_count && has_sha1_file(it->sha1)) @@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it, cache + i, entries - i, path, baselen + sublen + 1, - missing_ok, - dryrun); + flags); if (subcnt < 0) return subcnt; i += subcnt - 1; @@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it, int cache_tree_update(struct cache_tree *it, struct cache_entry **cache, int entries, - int missing_ok, - int dryrun, - int silent) + int flags) { int i; - i = verify_cache(cache, entries, silent); + i = verify_cache(cache, entries, flags); if (i) return i; - i = update_one(it, cache, entries, "", 0, missing_ok, dryrun); + i = update_one(it, cache, entries, "", 0, flags); if (i < 0) return i; return 0; @@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix) was_valid = cache_tree_fully_valid(active_cache_tree); if (!was_valid) { - int missing_ok = flags & WRITE_TREE_MISSING_OK; - if (cache_tree_update(active_cache_tree, active_cache, active_nr, - missing_ok, 0, 0) < 0) + flags) < 0) return WRITE_TREE_UNMERGED_INDEX; if (0 <= newfd) { if (!write_cache(newfd, active_cache, active_nr) && @@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root, return 0; } -int update_main_cache_tree (int silent) +int update_main_cache_tree (int flags) { if (!the_index.cache_tree) the_index.cache_tree = cache_tree(); return cache_tree_update(the_index.cache_tree, - the_index.cache, the_index.cache_nr, 0, 0, silent); + the_index.cache, the_index.cache_nr, flags); } diff --git a/cache-tree.h b/cache-tree.h index 0ec0b2a..d8cb2e9 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root); struct cache_tree *cache_tree_read(const char *buffer, unsigned long size); int cache_tree_fully_valid(struct cache_tree *); -int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int); +int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int); int update_main_cache_tree(int); /* bitmasks to write_cache_as_tree flags */ #define WRITE_TREE_MISSING_OK 1 #define WRITE_TREE_IGNORE_CACHE_TREE 2 +#define WRITE_TREE_DRY_RUN 4 +#define WRITE_TREE_SILENT 8 /* error return codes */ #define WRITE_TREE_UNREADABLE_INDEX (-1) diff --git a/merge-recursive.c b/merge-recursive.c index d83cd6c..6479a60 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o) if (!cache_tree_fully_valid(active_cache_tree) && cache_tree_update(active_cache_tree, - active_cache, active_nr, 0, 0, 0) < 0) + active_cache, active_nr, 0) < 0) die("error building trees"); result = lookup_tree(active_cache_tree->sha1); diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c index e6c2923..a6ffdf3 100644 --- a/test-dump-cache-tree.c +++ b/test-dump-cache-tree.c @@ -59,6 +59,6 @@ int main(int ac, char **av) struct cache_tree *another = cache_tree(); if (read_cache() < 0) die("unable to read index file"); - cache_tree_update(another, active_cache, active_nr, 0, 1, 0); + cache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN); return dump_cache_tree(active_cache_tree, another, ""); } -- 1.7.3.1.256.g2539c.dirty ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cache-tree: update API to take abitrary flags 2012-01-11 9:59 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy @ 2012-01-11 23:48 ` Junio C Hamano 2012-01-12 1:20 ` Nguyen Thai Ngoc Duy 0 siblings, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2012-01-11 23:48 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > --- Thanks; this one looks very sensible regardless of what follows (or does not follow). Forgot to sign-off? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] cache-tree: update API to take abitrary flags 2012-01-11 23:48 ` Junio C Hamano @ 2012-01-12 1:20 ` Nguyen Thai Ngoc Duy 0 siblings, 0 replies; 10+ messages in thread From: Nguyen Thai Ngoc Duy @ 2012-01-12 1:20 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 2012/1/12 Junio C Hamano <gitster@pobox.com>: > Thanks; this one looks very sensible regardless of what follows (or does > not follow). Forgot to sign-off? Deliberately to stop you from using it because I did not test it carefully. It was created as material for the discussion only. Will resubmit later. -- Duy ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-01-17 2:48 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-16 2:36 [PATCH 0/2] nd/commit-ignore-i-t-a replacement Nguyễn Thái Ngọc Duy 2012-01-16 2:36 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy 2012-01-16 2:36 ` [PATCH 2/2] commit, write-tree: allow to ignore CE_INTENT_TO_ADD while writing trees Nguyễn Thái Ngọc Duy 2012-01-16 16:46 ` Pete Wyckoff 2012-01-16 23:21 ` Junio C Hamano 2012-01-17 1:50 ` Nguyen Thai Ngoc Duy 2012-01-17 2:47 ` Jonathan Nieder -- strict thread matches above, loose matches on Subject: below -- 2012-01-11 6:01 [PATCH RFC] commit: allow to commit even if there are intent-to-add entries Nguyễn Thái Ngọc Duy 2012-01-11 9:59 ` [PATCH 1/2] cache-tree: update API to take abitrary flags Nguyễn Thái Ngọc Duy 2012-01-11 23:48 ` Junio C Hamano 2012-01-12 1:20 ` Nguyen Thai Ngoc Duy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.