* [PATCH 0/4] Deprecate "not allow as-is commit with i-t-a entries"
From: Nguyễn Thái Ngọc Duy @ 2012-02-06 10:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
By the end of this series "git commit -m foo" will proceed regardless
intent-to-add (aka "git add -N") entries. commit.ignoreIntentToAdd is
used as transitiono config key.
The plan is in 2/4. I set 1.8.0 as the first deprecation date, but of
course it's just a suggestion. The second deprecation date might be
1.8.5, long enough for users to adapt to the new behavior.
..or we switch back half way because we find current behavior does
make more sense.
Nguyễn Thái Ngọc Duy (4):
cache-tree: update API to take abitrary flags
commit: introduce a config key to allow as-is commit with i-t-a
entries
commit: turn commit.ignoreIntentToAdd to true by default
commit: remove commit.ignoreIntentToAdd, assume it's always true
Documentation/git-add.txt | 12 ++++++++++--
builtin/commit.c | 9 ++++++---
cache-tree.c | 35 +++++++++++++++++------------------
cache-tree.h | 5 ++++-
merge-recursive.c | 2 +-
t/t2203-add-intent.sh | 21 ++++++++++++++++++++-
test-dump-cache-tree.c | 2 +-
7 files changed, 59 insertions(+), 27 deletions(-)
--
1.7.8.36.g69ee2
^ permalink raw reply
* [PATCH 1/4] cache-tree: update API to take abitrary flags
From: Nguyễn Thái Ngọc Duy @ 2012-02-06 10:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328525855-2547-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.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.8.36.g69ee2
^ permalink raw reply related
* [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Nguyễn Thái Ngọc Duy @ 2012-02-06 10:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328525855-2547-1-git-send-email-pclouds@gmail.com>
This is the start of deprecating current commit behavior (do not allow
commit as-is when there are i-t-a entries in index). Users will be
annoyed by a warning saying the behavior will change in 1.8.0 and asked
to set commit.ignoreIntentToAdd properly.
The plan after that is:
- Keep it this way until 1.8.0
- In 1.8.0, we consider the lack of commit.ignoreIntentToAdd means
"true". A deprecation date is set. The warning message is updated
stating that from that day, this config key will be ignored. The
behavior will be as if this config key is always "true".
- In the chosen release, remove support for this config key.
WRITE_TREE_IGNORE_INTENT_TO_ADD is set unconditionally.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 10 ++++++++++
Documentation/git-add.txt | 12 ++++++++++--
builtin/commit.c | 28 +++++++++++++++++++++++++---
cache-tree.c | 8 +++++---
cache-tree.h | 1 +
t/t2203-add-intent.sh | 21 ++++++++++++++++++++-
6 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index abeb82b..6ec81a8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,6 +831,16 @@ 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. Set to `false` to disallow commit in this acase, or `true`
+ to allow it.
++
+By default, `git commit` refuses to commit as-is when you have intent-to-add
+entries. This will change in 1.8.0, where `git commit` allows it. If you
+prefer current behavior, please set it to `false`.
+
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/builtin/commit.c b/builtin/commit.c
index bf42bb3..af3250c 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;
@@ -117,6 +118,8 @@ static enum {
} status_format = STATUS_FORMAT_LONG;
static int status_show_branch;
+static int set_commit_ignoreintenttoadd;
+
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt->value;
@@ -400,7 +403,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"));
@@ -420,8 +423,20 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
+ if (!set_commit_ignoreintenttoadd) {
+ int i;
+ for (i = 0; i < active_nr; i++)
+ if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
+ break;
+ if (i < active_nr)
+ warning(_("You are committing as-is with intent-to-add entries as the result of\n"
+ "\"git add -N\". Git currently forbids this case. This will change in\n"
+ "1.8.0 where intent-to-add entries are simply ignored when committing\n"
+ "as-is. Please look up document and set commit.ignoreIntentToAdd\n"
+ "properly to stop this warning."));
+ }
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 +885,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 +1353,13 @@ 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")) {
+ set_commit_ignoreintenttoadd = 1;
+ 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/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..88a508e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,26 @@ 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
+ test_must_fail git commit -minitial
+'
+
+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' '
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 3/4] commit: turn commit.ignoreIntentToAdd to true by default
From: Nguyễn Thái Ngọc Duy @ 2012-02-06 10:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328525855-2547-1-git-send-email-pclouds@gmail.com>
From now on, those who has not set commit.ignoreIntentToAdd can commit
as-is even if there are intent-to-add entries. Users are advised/annoyed
to switch to new behavior.
Support for "commit.ignoreIntentToAdd = true" will be dropped in future.
The placeholder "FIXME" needs to be replaced when it's decided what
release will drop config support.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 5 ++---
builtin/commit.c | 13 ++++++++-----
t/t2203-add-intent.sh | 4 ++--
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6ec81a8..f9a05ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -837,9 +837,8 @@ commit.ignoreIntentToAdd::
in index. Set to `false` to disallow commit in this acase, or `true`
to allow it.
+
-By default, `git commit` refuses to commit as-is when you have intent-to-add
-entries. This will change in 1.8.0, where `git commit` allows it. If you
-prefer current behavior, please set it to `false`.
+By default, `git commit` allows to commit as-is when you have intent-to-add
+entries. Support for this configuration variable will be dropped in FIXME.
credential.helper::
Specify an external helper to be called when a username or
diff --git a/builtin/commit.c b/builtin/commit.c
index af3250c..eb0ca49 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -423,17 +423,17 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (!set_commit_ignoreintenttoadd) {
+ if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
int i;
for (i = 0; i < active_nr; i++)
if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
break;
if (i < active_nr)
warning(_("You are committing as-is with intent-to-add entries as the result of\n"
- "\"git add -N\". Git currently forbids this case. This will change in\n"
- "1.8.0 where intent-to-add entries are simply ignored when committing\n"
- "as-is. Please look up document and set commit.ignoreIntentToAdd\n"
- "properly to stop this warning."));
+ "\"git add -N\". Git currently forbids this case. But this is deprecated\n"
+ "support for this behavior will be dropped in FIXME.\n"
+ "Please look up document and set commit.ignoreIntentToAdd to true\n"
+ "or remove it."));
}
if (active_cache_changed) {
update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
@@ -1423,6 +1423,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, &s);
determine_whence(&s);
+ if (!set_commit_ignoreintenttoadd)
+ cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+
if (get_sha1("HEAD", sha1))
current_head = NULL;
else {
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 88a508e..09b8bbf 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,11 +41,11 @@ 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 -minitial
+ git commit -minitial
'
test_expect_success 'can commit tree with i-t-a entry' '
- git reset --hard &&
+ git reset --hard HEAD^ &&
echo xyzzy >rezrov &&
echo frotz >nitfol &&
git add rezrov &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 4/4] commit: remove commit.ignoreIntentToAdd, assume it's always true
From: Nguyễn Thái Ngọc Duy @ 2012-02-06 10:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328525855-2547-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 9 ---------
builtin/commit.c | 24 +-----------------------
t/t2203-add-intent.sh | 2 +-
3 files changed, 2 insertions(+), 33 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f9a05ac..abeb82b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,15 +831,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. Set to `false` to disallow commit in this acase, or `true`
- to allow it.
-+
-By default, `git commit` allows to commit as-is when you have intent-to-add
-entries. Support for this configuration variable will be dropped in FIXME.
-
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/builtin/commit.c b/builtin/commit.c
index eb0ca49..491cae1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,8 +118,6 @@ static enum {
} status_format = STATUS_FORMAT_LONG;
static int status_show_branch;
-static int set_commit_ignoreintenttoadd;
-
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt->value;
@@ -423,18 +421,6 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
- int i;
- for (i = 0; i < active_nr; i++)
- if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
- break;
- if (i < active_nr)
- warning(_("You are committing as-is with intent-to-add entries as the result of\n"
- "\"git add -N\". Git currently forbids this case. But this is deprecated\n"
- "support for this behavior will be dropped in FIXME.\n"
- "Please look up document and set commit.ignoreIntentToAdd to true\n"
- "or remove it."));
- }
if (active_cache_changed) {
update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1353,13 +1339,6 @@ 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")) {
- set_commit_ignoreintenttoadd = 1;
- 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)
@@ -1423,8 +1402,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, &s);
determine_whence(&s);
- if (!set_commit_ignoreintenttoadd)
- cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+ cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
if (get_sha1("HEAD", sha1))
current_head = NULL;
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 09b8bbf..7c7ab54 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -50,7 +50,7 @@ test_expect_success 'can commit tree with i-t-a entry' '
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- git config commit.ignoreIntentToAdd true &&
+ git config commit.ignoreIntentToAdd false &&
git commit -m initial &&
git ls-tree -r HEAD >actual &&
cat >expected <<EOF &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Specifying revisions in the future
From: Matthieu Moy @ 2012-02-06 11:43 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Philip Oakley, Jakub Narebski, jpaugh, git
In-Reply-To: <m2obtcx4i2.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> writes:
> The rule should be to follow the leftmost parent as far as possible.
But then, if --first-parent doesn't reach the commit you want, there may
be several paths not following --first-parent that reach it. And you'll
have to invent some more rules to order them.
Sure, that's not impossible, but is the complexity really worth it?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Felipe Contreras @ 2012-02-06 12:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <7vy5sgaby1.fsf@alter.siamese.dyndns.org>
On Mon, Feb 6, 2012 at 5:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> Ugh, yeah. I was thinking about how it would improve this call site, but
>> I don't want to get into auditing the others. Let's drop it and go with
>> your patch.
>
> In any case, here is what I queued for tonight.
>
> -- >8 --
> Subject: [PATCH] mailmap: do not leave '>' in the output when answering "we did something"
This subject doesn't explain the *purpose* of the patch: always return
a plain mail address from map_user()
That would be enough for most readers.
I think the immediate problem should be here:
Currently 'git blame -e' would add an extra '>' if map_user() returns
true, which would end up as '<foo@bar.com>>'. This is because
map_user() sometimes modifies, the mail string, but sometimes not. So
let's always modify it.
At this point a lot of readers can skip the rest of the explanation.
> The callers of map_user() give email and name to it, and expect to get an
> up-to-date versions of email and/or name to be used in their output. The
> function rewrites the given buffers in place. To optimize the majority of
> cases, the function returns 0 when it did not do anything, and it returns
> 1 when the caller should use the updated contents.
>
> The 'email' input to the function is terminated by '>' or a NUL (whichever
> comes first) for historical reasons, but when a rewrite happens, the value
> is replaced with the mailbox inside the <> pair. However, it failed to
> meet this expectation when it only rewrote the name part without rewriting
> the email part, and the email in the input was terminated by '>'.
With the above explanation I suggested, I think this can be summarized as:
As of right now most of the callers of map_user() give a plan address,
and expect a plain address, however, 'git blame -e' passes along a
mail address that ends with >, and uses the return value to determine
if a transformation was made, and adds the missing '>'. This is
because when map_user() does the transformation, it returns a plan
address.
This might have worked in previous versions of map_user(), but now not
only does it transforms mail addresses, but also the name (phrase). So
now callers can't use the return value to know if they need to add an
extra '>', or not. So lets always remove the '>', so they can.
> This causes an extra '>' to appear in the output of "blame -e", because the
> caller does send in '>'-terminated email, and when the function returned 1
> to tell it that rewriting happened, it appends '>' that is necessary when
> the email part was rewritten.
It took too long to reach the problem. As a reader, that's the first
thing I would expect.
> The patch looks bigger than it actually is, because this change makes a
> variable that points at the end of the email part in the input 'p' live
> much longer than it used to, deserving a more descriptive name.
This is a *separate* logically independent patch. And you yourself
mention, makes the review of the patch more difficult, as of right now
it's difficult to spot the functional change, because it's mixed with
non-functional changes.
I find it peculiar how patches in the Linux kernel are truly logically
independent, but Git has a tendency to squash many things: cleanups,
fixes, documentation, tests, extra tests, remove unused code, etc.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Specifying revisions in the future
From: Andreas Schwab @ 2012-02-06 12:27 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Philip Oakley, Jakub Narebski, jpaugh, git
In-Reply-To: <vpq62fk89x1.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> The rule should be to follow the leftmost parent as far as possible.
>
> But then, if --first-parent doesn't reach the commit you want, there may
> be several paths not following --first-parent that reach it. And you'll
> have to invent some more rules to order them.
The leftmost parent is not necessarily the first parent, but the
leftmost parent that still reaches the commit. It's a depth-first
search: if following the first parent doesn't reach the commit any more,
try again with the second parent.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-06 13:45 UTC (permalink / raw)
To: Steven Michalske; +Cc: git
In-Reply-To: <EAF9D593-4E0C-4C95-A048-3F6AC8ADD866@gmail.com>
On Mon, 6 Feb 2012, Steven Michalske wrote:
> See inlined responses below.
Is this comment necessary at all?
> On Feb 4, 2012, at 11:45 AM, Jakub Narebski wrote:
>
> > So people would like for git to warn them about rewriting history before
> > they attempt a push and it turns out to not fast-forward.
> >
>
> I like this idea and I encounter this issue with my co-workers new to git.
> It scares them thinking they broke the repository.
It is true that while this feature would be useful also for "power
users", it would be most helpful for newbies (users new to git).
So I am afraid that implementing it with example hooks that must be
turned on explicitly might be not enough...
> > In Mercurial 2.1 there are three available phases: 'public' for
> > published commits, 'draft' for local un-published commits and
> > 'secret' for local un-published commits which are not meant to
> > be published.
> >
> > The phase of a changeset is always equal to or higher than the phase
> > of it's descendants, according to the following order:
> >
> > public < draft < secret
>
> Let's not limit ourselves to just three levels. They are a great start
> but I propose the following.
As we don't have any implementation, I'd rather we don't multiply entities.
I was even thinking about limiting to just 'public' and 'draft' "phases".
> published - The commits that are on a public repository that if are
> rewritten will invoke uprisings. general rule here would be
> to revert or patch, no rewrites.
> based - The commits that the core developers have work based upon.
> (not just the commits in their repo.)
> general rule is notify your fellow developers before a rewrite.
> shared - The commits that are known to your fellow core developers.
> These commits are known, but have not had work based off of them.
> Minimal risk to rewrite.
All these are very fairly nuanced, with minuscule differences between
them. I'd rather not multiply entities, especially not introduce such
hard to guess what it about from their name.
In Mercurial phases share hierarchy of traits:
http://mercurial.selenic.com/wiki/Phases
| traits |
.......................
| immutable | shared |
----------+-----------+---------+
public | x | x | ^
draft | | x | ^
secret | | | ^
The names of those traits probably should be changed in Git.
Those traits are boolean in Mercurial, but I think we can implement
what you would like to have to change them to tristate: 'deny' (unless
forced, i.e. the same as true), 'warn', 'ignore' (i.e. the same as false).
I think that it would be nice to be able to tune "severity" of trait
on per-remote and/or per-branch basis. This way you would get warned
before rewriting commits that were pushed to your group repository,
and prevented from rewriting commits that are present in projects public
repository.
Nevertheless I think it is something better left for later, and added
only if it turns out to be really needed.
> local - The commits that are local only, no one else has a copy.
> Commits your willing to share, but have not been yet shared,
> either from actions of you, or a fetch from others.
That's Mercurial's 'draft' phase.
> restricted or private - The commits that you do not want shared.
> Manually added, think of a branch tip marked as restricted
> automatically promotes commits to the branch as restricted.
That's Mercurial 'secret' phase.
> Maybe make these like nice levels, but as two components,
> publicity 0-100 and rewritability 0-100
> Published is publicity 100 and rewritability 0
> Restricted is publicity 0 and rewritability 100
> Based publicity 75 and rewritability 25
> Shared publicity 50 and rewritability 50
> Local publicity 25 and rewritability 75
> Restricted publicity 0 and rewritability 100
Continuous traits are IMHO a bad idea. You would have to quantize them
and turn them on into specific behavior: ignore, warn, deny.
For example WTF does 25 "publicity" (bad name) or "rewritability" actually
means in term of git behavior, eh?
> Other option are flags stating if the commit is published, based,
> shared, or restricted. You could have a published and based commit
> that is more opposed to rewrite than a public commit.
>
> Call security on a published restricted commit ;-)
Please note that while "phases" look like they are trait of individual
commits, they are in fact artifact of revision walking. The idea is
that ancestors of 'private' commit can be 'private', 'draft' or 'public',
that ancestors of 'draft' commit are 'draft' or 'public', and that _all_
ancestors of 'public' commit are 'public'.
> Commits are by default local.
This 'by default' needs to be specified further, because for example
all commits in freshly cloned repository should be in 'public' phase
by default.
Also, don't say 'commits are local', 'commits are published'; use "phases"
nomenclature (at least until we invent something so much better that it
is worth breaking consistency with Mercurial terminology).
>
> Commits are published when they are pushed or fetched and merged to
> a publishing branch of a repository.
BTW. I am not sure if pushing to remote repository updates (or can update)
any remote-tracking branches...
> On fetch/merge a post merge hook should send back a note to
> the remote repository that the commits were published.
I think this is unnecessary in the "best practices" scenario, where each
user has separate private repository where he/she does his/her work, and
one's own public repository, where people fetch from. He/she can push
to some shared repository, and that has to be supported too.
Though there is mothership/ sattellite situation, where you can pull and
push only from one direction. There we might want for some way to notify
that some commits were fetched and should now be considered 'public'.
Though I am not sure if it is really necessary.
> Restricted commits/branches/tags should not be made public, error out and
> require clearing of the attribute or a --force-restricted option that
> automatically removes the restricted attribute. They are at least promoted
> to shared, if not published.
Or just skip them (silently or not) if we push using globbing refspec, and
glob matches some refs marked as 'private'.
> Based is only used in situations where you have developers sharing amongst
> their repositories, and you want a rule that is less restrictive than
> no rewrites.
Multiplying entities.
> Shared is what we have now when a commit is in a remote repository without
> the no rewrite options. e.g. receive.denyNonFastForwards.
Multiplying entities.
[...]
> > Using the nomenclature from Mercurial
> > public < draft < secret
>
> public -> publicity 100, rewritability 0
> draft -> publicity ?, rewritability 50
> secret -> publicity 0, rewritability 100
That doesn't really help, at all.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-06 14:44 UTC (permalink / raw)
To: Johan Herland; +Cc: Philip Oakley, git
In-Reply-To: <CALKQrgcAsPXziQCTReZkCKnnXTX=rwPFrzp0wJ3ZYwn0b_M5Tw@mail.gmail.com>
On Sun, 5 Feb 2012, Johan Herland wrote:
> On Sun, Feb 5, 2012 at 21:46, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Sun, 5 Feb 2012, Johan Herland wrote:
>>> 2012/2/5 Jakub Narebski <jnareb@gmail.com>:
[...]
>>> I agree that the 'public' state should (by default) be automatically
>>> inferred from remote-tracking branches. As it stands, we can do this
>>> with current git, by writing a pre-rebase hook that checks if any of
>>> the commits to-be-rebased are reachable from any remote-tracking
>>> branch.
>>
>> It is nice that we can achieve a large part of this feature with existing
>> infrastructure. It would be nice if we ship such pre-rebase hook with
>> git, so people can just enable it if they want to use this functionality,
>> like the default pre-commit hook that checks for whitespace errors.
>
> Yeah. As it is, the pre-rebase hook shipped with v1.7.9 (when
> activated) does something similar (i.e. prevent rewriting 'public'
> commits). However, it's highly workflow-specific, since it determines
> whether the branch being rebased has been merged into "next" or
> "master". IMHO, a hook that tested for reachability from
> remote-tracking refs would be more generally useful. Obviously, the
> two can be combined, and even further combinations may be desirable
> (e.g. also checking for reachability from commits annotated in
> refs/notes/public).
Relying on (default) hooks to implement this feature has the disadvantage
that it wouldn't be turned on by default... while this feature would be
most helpful for users new to git (scared by refuse to push).
I am not sure either if everything (wrt. safety net) can be implemented
via hooks. One thing that I forgot about is preventing rewinding of
branch past the published commit using e.g. "git reset --hard <commit>".
Unless `pre-rewrite` hook could be used for that safety too...
[...]
>> Note however that the safety net, i.e. refusing or warning against attempted
>> rewrite of published history is only part of issue. Another important part
>> is querying and showing "phase" of a commit. What I'd like to see is
>> ability to show among others in "git log" and "git show" output if commit
>> was already published or not (and if it is marked 'secret').
>
> Today, you can use --decorate to display remote-tracking refs in the
> log/show output. However, only the tip commits are decorated, so if
> the commits shown are not at the tip, you're out of luck. I believe
> teaching log/show to decorate _all_ commits that are reachable from
> some given ref(s) should be fairly straightforward.
That would be nice.
> If you use 'git notes' to annotate 'public' and 'secret' states, then
> you can also use the --show-notes=<ref> option to let show/log display
> the annotations on 'public'/'secret' commits.
First, in my opinion annotating _all_ commits with their phase is I think
out of question, especially annotating 'public' commits. I don't think
git-notes mechanism would scale well to annotating every commit; but
perhaps this was tested to work, and I am mistaken.
Second, I have doubts if "phase" is really state of an individual commit,
and not the feature of revision walking.
Take for example the situation where given commit is reference by
remote-tracking branch 'public/foo', and also by two local branches:
'foo' with upstream 'public/foo', and local branch 'bar' with no upstream.
Now it is quite obvious that this feature should prevent rewriting 'foo'
branch, for which commits are published upstream. But what about branch
'bar'? Should we prevent rewriting (e.g. rebase) here too? What about
rewinding 'bar' to point somewhere else. What if 'bar' is really detached
HEAD?
These questions need to be answered...
[...]
>>> Also, if you want to record where 'public' commits have been sent
>>> (other than what can be inferred from the remote-tracking branches),
>>> you could write this into the refs/notes/public annotation.
>>
>> I wonder if this too can be done by hook...
>
> You're looking for someting like a post-push hook that runs on the
> _client_ after a successful push. AFAIK, that doesn't exist yet. (Not
> to be confused with the receive/update hooks that run on the
> _server_.)
And such hook could react to what was successfully pushed. Without
such hook we would have to write wrapper around git-push and parse
its output...
Nb. such hook could create "fake" remote-tracking branches if given
push-only remote doesn't have them set up.
>>> As for 'secret' commits, you could annotate these on a
>>> refs/notes/secret notes ref, and then teach 'git push' (or whatever
>>> other method for publishing commits you use) to refuse to publish
>>> commits annotated on this notes ref. Possibly we would want to add a
>>> "pre-push" or "pre-publish" hook.
>>
>> Well, addition of pre-push / pre-publish was resisted on the grounds
>> that all it does is something that can be as easy done by hand before
>> push. Perhaps this new use case would help bring it forward, don't
>> you think?
>
> Maybe. I didn't follow the original discussion. From my POV, you could
> argue that instead of another hook, you could always write a script
> that does the 'secret' check before invoking 'git push', and then
> you'd use that script instead of 'git push'. But you could argue the
> same point for pretty much all of the other existing hooks (e.g.
> instead of a pre-commit hook you could have your own commit wrapper
> script). So I don't think that's a sufficient argument to refuse the
> existence of a pre-push/publish hook.
Right, This would be for this feature very much like pre-commit hook.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH/RFC v4] grep: Add the option '--exclude'
From: Nguyen Thai Ngoc Duy @ 2012-02-06 15:16 UTC (permalink / raw)
To: Albert Yale; +Cc: git, gitster
In-Reply-To: <1328192753-29162-1-git-send-email-surfingalbert@gmail.com>
On Thu, Feb 2, 2012 at 9:25 PM, Albert Yale <surfingalbert@gmail.com> wrote:
> I added a "struct pathspec_set" as you suggested
> in your previous review. It had the side effect
> of forcing me to update a few more files than was
> previously necessary.
Please make it a separate patch, it's hard to follow an all-in-one
patch. Although those changes might be unnecessary (see below).
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -566,6 +566,10 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> while (tree_entry(tree, &entry)) {
> int te_len = tree_entry_len(&entry);
>
> + if (!match_pathspec_depth(pathspec,
> + entry.path, strlen(entry.path),
> + 0, NULL))
> + continue;
> if (match != all_entries_interesting) {
> match = tree_entry_interesting(&entry, base, tn_len, pathspec);
> if (match == all_entries_not_interesting)
tree_entry_interesting() is equivalent to match_pathspec_depth(). The
only difference is that the former is designed to match on trees why
the latter a list. And tree_entry_interesting() is more efficient than
match_pathspec_depth(). As you can see there's
tree_entry_interesting() call above already. You should make the
tree_entry_interesting() understand exclude pathspec instead of adding
match_pathspec_depth() in.
> @@ -295,6 +298,25 @@ int match_pathspec_depth(const struct pathspec *ps,
> return retval;
> }
>
> +int match_pathspec_depth(const struct pathspec *ps,
> + const char *name, int namelen,
> + int prefix, char *seen)
> +{
> + int retval = match_pathspec_set_depth(&ps->include,
> + ps->recursive, ps->max_depth,
> + name, namelen, prefix, seen);
> +
> + if (retval && ps->exclude.nr)
> + {
> + if (match_pathspec_set_depth(&ps->exclude,
> + ps->recursive, ps->max_depth,
> + name, namelen, prefix, seen))
> + return 0;
> + }
> +
> + return retval;
> +}
> +
> static int no_wildcard(const char *string)
> {
> return string[strcspn(string, "*?[{\\")] == '\0';
It makes me wonder, why not add match_pathspec_with_exclusion(const
struct pathspec *include_ps, const struct pathspec *exclude_ps,...),
use the new function in grep.c and revert struct pathspec back to
original? The same can be applied to tree_entry_interesting() (i.e.
add a new one that takes two pathspec sets, which supports exclusion)
I think you may make less changes that way. I'm bad at naming. It's up
to you to rename match_pathspec_with_exclusion to something meaningful
and short enough.
--
Duy
^ permalink raw reply
* Re: Git performance results on a large repository
From: Joey Hess @ 2012-02-06 15:40 UTC (permalink / raw)
To: git@vger.kernel.org
In-Reply-To: <CACsJy8Bf95JMp1qOiruR7+Tdi7JN42KNeMqGLud+z3O26DREnw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]
Nguyen Thai Ngoc Duy wrote:
> The "interface to report which files have changed" is exactly "git
> update-index --[no-]assume-unchanged" is for. Have a look at the man
> page. Basically you can mark every file "unchanged" in the beginning
> and git won't bother lstat() them. What files you change, you have to
> explicitly run "git update-index --no-assume-unchanged" to tell git.
>
> Someone on HN suggested making assume-unchanged files read-only to
> avoid 90% accidentally changing a file without telling git. When
> assume-unchanged bit is cleared, the file is made read-write again.
That made me think about using assume-unchanged with git-annex since it
already has read-only files.
But, here's what seems a misfeature... If an assume-unstaged file has
modifications and I git add it, nothing happens. To stage a change, I
have to explicitly git update-index --no-assume-unchanged and only then
git add, and then I need to remember to reset the assume-unstaged bit
when I'm done working on that file for now. Compare with running git mv
on the same file, which does stage the move despite assume-unstaged. (So
does git rm.)
--
see shy jo
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] On compresing large index
From: Joshua Redstone @ 2012-02-06 15:54 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy, Thomas Rast; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8AnGg11PeCGFs_BxOM3wAjwzs2tOCWJV31_2_KMFTxhDA@mail.gmail.com>
Fwiw, specifically related to 'git ls-files', since it is a relatively
rare operation, it's probably ok if it's a bit slow. I know you chose it
as a good benchmark of index reading performance. I just mention it
because, in some hypothetical wild-and-crazy world in which we had a
git-aware file system layer, one could imagine doing away with most of the
index file and querying the file system for info on what's changed, SHA1
of subtrees, etc.
Do you have a sense of which operations on the index are high-value pain
points for large repositories? I can imagine things like 'git-add' and
'git-commit', but I'm not super familiar with other common operations it
has a role in.
Josh
On 2/5/12 8:35 PM, "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> wrote:
>2012/2/6 Thomas Rast <trast@inf.ethz.ch>:
>>> We need to figure out what git uses 4s user time for.
>>
>> When I worked on the cache-tree stuff, my observation (based on
>> profiling, so I had actual data :-) was that computing SHA1s absolutely
>> dominates everything in such operations. It does that when writing the
>> index to write the trailing checksum, and also when loading it to verify
>> that the index is valid.
>
>You're right. This is on another machine but with same index (2M
>files), without SHA1 checksum:
>
>$ time ~/w/git/git ls-files --stage|head > /dev/null
>real 0m1.533s
>user 0m1.228s
>sys 0m0.306s
>
>and with SHA-1 checksum:
>
>$ time git ls-files --stage|head > /dev/null
>real 0m7.525s
>user 0m7.257s
>sys 0m0.268s
>
>I guess we could fall back to cheaper digests for such a large index.
>Still more than one second for doing nothing but reading index is too
>slow to me.
>
>> ls-files shouldn't be so slow though. A quick run with callgrind in a
>> linux-2.6.git tells me it spends about 45% of its time on SHA1s and a
>> whopping 25% in quote_c_style(). I wonder what's so hard about
>> quoting...
>
>That's why I put "| head" there, to cut output processing overhead
>(hopefully).
>--
>Duy
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Johan Herland @ 2012-02-06 15:59 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Oakley, git
In-Reply-To: <201202061544.14417.jnareb@gmail.com>
On Mon, Feb 6, 2012 at 15:44, Jakub Narebski <jnareb@gmail.com> wrote:
> On Sun, 5 Feb 2012, Johan Herland wrote:
> Relying on (default) hooks to implement this feature has the disadvantage
> that it wouldn't be turned on by default... while this feature would be
> most helpful for users new to git (scared by refuse to push).
True. I too believe that this will be most helpful if it is enabled by
default. That said, the easiest way to get there might be through
first demonstrating that it works in practice when implemented as
hooks.
> I am not sure either if everything (wrt. safety net) can be implemented
> via hooks. One thing that I forgot about is preventing rewinding of
> branch past the published commit using e.g. "git reset --hard <commit>".
> Unless `pre-rewrite` hook could be used for that safety too...
Hmm. I don't think we'll be able to "plug" all the holes that might
leave the user in a rewritten state (e.g. what if the user (possibly
with the help of some tool) does an "echo $SHA1 >
.git/refs/head/master"?). And trying to plug too many holes might end
up annoying more experienced users who "know what they're doing".
Instead we might want to add a client-side check at push time. I
realize that this check is already done by the remote end, but the
client-side might be able to give a more helpful response along the
lines of:
You are trying to push branch X to remote Y, but remote Y already
has a branch X that is N commits in front of you. You may want to
rebase your work on top of the remote branch (see 'git pull
--rebase'), If you instead force this push (with --force), you will
remove those N commits, and replace them with the M last commits from
your branch X.
(followed by a list of the remote N and local M commits, respectively)
[...]
>> If you use 'git notes' to annotate 'public' and 'secret' states, then
>> you can also use the --show-notes=<ref> option to let show/log display
>> the annotations on 'public'/'secret' commits.
>
> First, in my opinion annotating _all_ commits with their phase is I think
> out of question, especially annotating 'public' commits. I don't think
> git-notes mechanism would scale well to annotating every commit; but
> perhaps this was tested to work, and I am mistaken.
First, we don't need to annotate _all_ commits. For the 'public'
state, we only annotate the last/tip commit that was pushed/published.
From there, we can defer that all ancestor commits are also 'public'.
For the 'secret' state, we do indeed annotate _all_ secret commits,
but I believe this will be a somewhat limited number of commits. If
your workflow forces you to annotate millions of commits as 'secret',
I claim there is something wrong with your workflow.
Second, git-notes were indeed designed scale well to handle a large
number of notes, up to the same order of magnitude as the number of
commits in your repo. (When git-notes was originally written, I
successfully tested it on versions of a linux-kernel repo where every
single commit was annotated). In this case, the number of 'public'
annotations in your repo would be equal to the number of pushes you
do, and the number of 'secret' annotations would be equal to the
number of 'secret' commits in your repo. I'd expect both of these
numbers to be orders of magnitude smaller than the total number of
commits in your repo (given a fairly typical workflow in a fairly
typical repo).
> Second, I have doubts if "phase" is really state of an individual commit,
> and not the feature of revision walking.
I believe the 'public' state is a "feature of revision walking" (i.e.
one annotated 'public' commit implies that all its ancestors are also
'public'). However, the 'secret' state should be bound to the
individual commit, IMHO.
> Take for example the situation where given commit is reference by
> remote-tracking branch 'public/foo', and also by two local branches:
> 'foo' with upstream 'public/foo', and local branch 'bar' with no upstream.
>
> Now it is quite obvious that this feature should prevent rewriting 'foo'
> branch, for which commits are published upstream. But what about branch
> 'bar'? Should we prevent rewriting (e.g. rebase) here too? What about
> rewinding 'bar' to point somewhere else. What if 'bar' is really detached
> HEAD?
>
> These questions need to be answered...
Good point. There are two questions we may need to answer: "Has commit
X ever been published?", and "Has commit X ever been published in the
context of branch Y?". In the latter case, we do indeed need to take
the upstream branch into account.
Basically, there are three different "levels" for this rewrite/publish
protection to run at:
1. Do not meddle at all. This is the current behavior, and assumes
that if the user rewrites and pushes something, the user knows what
he/she is doing, and Git should not meddle (obviously unless the
server refuses the push).
2. Warn/refuse rewriting commits in your upstream. This would only
check branch X against its registered upstream. Only if there is a
registered upstream, and you're about to rewrite commits that are
reachable from the upstream remote-tracking branch, should Git
intervene and warn/refuse the rewrite. This level would IMHO provide
most of the benefit, and little or no trouble (i.e. false positives).
3. Warn/refuse rewriting _any_ 'public' commit. Refuse to rewrite any
commit that is reachable from any remote-tracking branch. Some would
say that this is a Good Thing(tm), since it prevents a commit from
being _copied_ (i.e. rebased or cherry-picked) between branches (you'd
be in this camp if you run a tightly-controlled workflow, where you
e.g. mandate upmerging patches from the oldest applicable branch
instead of cherry-picking patches from a newer branch). However, other
people would say that this is too limiting, and imposes unnecessary
rules on the workflow of the project (where e.g. copying (by way of
git-rebase) a topic branch from one place to another would cause an
annoying false positive).
[...]
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: Git performance results on a large repository
From: Matt Graham @ 2012-02-06 16:23 UTC (permalink / raw)
To: Joshua Redstone; +Cc: Nguyen Thai Ngoc Duy, git@vger.kernel.org
In-Reply-To: <243C23AF01622E49BEA3F28617DBF0AD5912CA85@SC-MBX02-5.TheFacebook.com>
On Sat, Feb 4, 2012 at 18:05, Joshua Redstone <joshua.redstone@fb.com> wrote:
> [ wanted to reply to my initial msg, but wasn't subscribed to the list at time of mailing, so replying to most recent post instead ]
>
> Matt Graham: I don't have file stats at the moment. It's mostly code files, with a few larger data files here and there. We also don't do sparse checkouts, primarily because most people use git (whether on top of SVN or not), which doesn't support it.
This doesn't help your original goal, but while you're still working
with git-svn, you can do sparse checkouts. Use --ignore-paths when you
do the original clone and it will filter out directories that are not
of interest.
We used this at Etsy to keep git svn checkouts manageable when we
still had a gigantic svn repo. You've repeatedly said you don't want
to reorganize your repos but you may find this writeup informative
about how Etsy migrated to git (which included a health amount of repo
manipuation).
http://codeascraft.etsy.com/2011/12/02/moving-from-svn-to-git-in-1000-easy-steps/
^ permalink raw reply
* [PATCH] fsck: give accurate error message on empty loose object files
From: Matthieu Moy @ 2012-02-06 16:24 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpqfwf6en6e.fsf@bauges.imag.fr>
Since 3ba7a065527a (A loose object is not corrupt if it
cannot be read due to EMFILE), "git fsck" on a repository with an empty
loose object file complains with the error message
fatal: failed to read object <sha1>: Invalid argument
This comes from a failure of mmap on this empty file, which sets errno to
EINVAL. Instead of calling xmmap on empty file, we display a clean error
message ourselves, and return a NULL pointer. The new message is
error: object file .git/objects/09/<rest-of-sha1> is empty
fatal: loose object <sha1> (stored in .git/objects/09/<rest-of-sha1>) is corrupt
The second line was already there before the regression in 3ba7a065527a,
and the first is an additional message, that should help diagnosing the
problem for the user.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
sha1_file.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 88f2151..fafc187 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1202,6 +1202,11 @@ void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
if (!fstat(fd, &st)) {
*size = xsize_t(st.st_size);
+ if (*size == 0) {
+ /* mmap() is forbidden on empty files */
+ error("object file %s is empty", sha1_file_name(sha1));
+ return NULL;
+ }
map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
}
close(fd);
--
1.7.9.111.gf3fb0.dirty
^ permalink raw reply related
* Re: git-gui Ctrl-U (unstage) broken
From: Stefan Haller @ 2012-02-06 16:48 UTC (permalink / raw)
To: Pat Thoyts, Victor Engmark; +Cc: git
In-Reply-To: <877h0at7ua.fsf@fox.patthoyts.tk>
Pat Thoyts <patthoyts@users.sourceforge.net> wrote:
> Victor Engmark <victor.engmark@gmail.com> writes:
>
> >Using the git-gui available with the default Ubuntu 10.10 repos, I'm
> >not able to unstage files with the default keyboard shortcut. To
> >reproduce:
> >1. Change a file in the repository
> >2. Run `git gui`
> >3. Stage the changed file
> >4. Select the changed file in the "Staged Changes (Will Commit)" list
> >5. Click Ctrl-U
> >
> >Expected outcome: The selected file should be unstaged.
> >
> >Actual outcome: Nothing at all changes in the GUI.
>
> I checked this with the current version (gitgui-0.16.0) and it works ok
> for me (on windows) - ie: ctrl-u unstaged a selected file.
Pat, it depends on where the focus is when you press ctrl-u. If you
click in the diff pane, and then select the file to unstage, and then
press ctrl-u, then nothing happens. If you click in the commit message
field, then ctrl-u works fine.
The same problem exists with ctrl-j for "Revert Changes". In that case
it is caused by the vi bindings that were introduced by 60aa065f69. It
seems that a binding for <Key-j> is also triggered by ctrl-j (if you
have a long diff you can see the diff pane scroll down by one line).
I'm not sure how to explain why ctrl-u doesn't work though, as I can't
see any binding for <Key-u>, but maybe this gives a clue to someone who
knows more about TCL than I do.
(It's not Windows-specific, btw: the same problem exists on Mac with
Command-U and Command-J.)
--
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
^ permalink raw reply
* [PATCH] bash-completion: add --edit-description to choices for branch
From: Paul Gortmaker @ 2012-02-06 17:03 UTC (permalink / raw)
To: git; +Cc: Paul Gortmaker
Support was recently added to allow storing a branch description,
so teach bash completion about it.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 78be195..a2965f7 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1152,7 +1152,7 @@ _git_branch ()
__gitcomp "
--color --no-color --verbose --abbrev= --no-abbrev
--track --no-track --contains --merged --no-merged
- --set-upstream
+ --set-upstream --edit-description
"
;;
*)
--
1.7.9
^ permalink raw reply related
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-06 17:14 UTC (permalink / raw)
To: Johan Herland; +Cc: Philip Oakley, git
In-Reply-To: <CALKQrgcN9Miq7SN_ETvjboRkHRFzcZejrQfy36BQFxYDnVPP6w@mail.gmail.com>
On Mon, 6 Feb 2012, Johan Herland wrote:
> On Mon, Feb 6, 2012 at 15:44, Jakub Narebski <jnareb@gmail.com> wrote:
> > On Sun, 5 Feb 2012, Johan Herland wrote:
> > Relying on (default) hooks to implement this feature has the disadvantage
> > that it wouldn't be turned on by default... while this feature would be
> > most helpful for users new to git (scared by refuse to push).
>
> True. I too believe that this will be most helpful if it is enabled by
> default. That said, the easiest way to get there might be through
> first demonstrating that it works in practice when implemented as
> hooks.
Yes, starting with prototype implementation using existing infrastructure
(hooks) would be a very good idea. (That's how first versions of what
became submodules were implemented.)
OTOH we should be aware of limitations of said prototype due to the fact
that it is a prototype...
> > I am not sure either if everything (wrt. safety net) can be implemented
> > via hooks. One thing that I forgot about is preventing rewinding of
> > branch past the published commit using e.g. "git reset --hard <commit>".
> > Unless `pre-rewrite` hook could be used for that safety too...
>
> Hmm. I don't think we'll be able to "plug" all the holes that might
> leave the user in a rewritten state (e.g. what if the user (possibly
> with the help of some tool) does an "echo $SHA1 >
> .git/refs/head/master"?).
First, I was thinking about having safety net against rewriting published
commits being present only in porcelain. Plumbing would be not affected
(perhaps there would be need to extend or add new plumbing to query "phase"
state, though).
> And trying to plug too many holes might end
> up annoying more experienced users who "know what they're doing".
Second, forcing via command line parameter should always be an option.
> Instead we might want to add a client-side check at push time. I
> realize that this check is already done by the remote end, but the
> client-side might be able to give a more helpful response along the
> lines of:
[...]
Explanation is good, but the whole idea of rewriting safety is that you
are informed (warned or denied) _before_ attempting rewrite and doing much
work.
> > > If you use 'git notes' to annotate 'public' and 'secret' states, then
> > > you can also use the --show-notes=<ref> option to let show/log display
> > > the annotations on 'public'/'secret' commits.
> >
> > First, in my opinion annotating _all_ commits with their phase is I think
> > out of question, especially annotating 'public' commits. I don't think
> > git-notes mechanism would scale well to annotating every commit; but
> > perhaps this was tested to work, and I am mistaken.
>
> First, we don't need to annotate _all_ commits. For the 'public'
> state, we only annotate the last/tip commit that was pushed/published.
> From there, we can defer that all ancestor commits are also 'public'.
Right.
> For the 'secret' state, we do indeed annotate _all_ secret commits,
> but I believe this will be a somewhat limited number of commits. If
> your workflow forces you to annotate millions of commits as 'secret',
> I claim there is something wrong with your workflow.
Well, for the 'secret' we can rely on the fact that child of 'secret'
commit must also be 'secret' (non-publishable) if secret is to stay
secret. Still marking all 'secret' commits might be better idea from
UI and from performance point of view.
> Second, git-notes were indeed designed scale well to handle a large
> number of notes, up to the same order of magnitude as the number of
> commits in your repo. (When git-notes was originally written, I
> successfully tested it on versions of a linux-kernel repo where every
> single commit was annotated).
Ah. That is very nice!
> In this case, the number of 'public'
> annotations in your repo would be equal to the number of pushes you
> do, and the number of 'secret' annotations would be equal to the
> number of 'secret' commits in your repo. I'd expect both of these
> numbers to be orders of magnitude smaller than the total number of
> commits in your repo (given a fairly typical workflow in a fairly
> typical repo).
Right.
> > Second, I have doubts if "phase" is really state of an individual commit,
> > and not the feature of revision walking.
It matters to presentation: can commit be simultaneously 'public' because
of one branch, and 'draft' because of other.
> I believe the 'public' state is a "feature of revision walking" (i.e.
> one annotated 'public' commit implies that all its ancestors are also
> 'public'). However, the 'secret' state should be bound to the
> individual commit, IMHO.
Good call, otherwise 'secret' commit could have been "side-leaked"
by other refs being pushed.
This means though that 'public' / 'draft' while looking similar to 'secret'
are in fact a bit different things. In other words 'immutable' and
'impushable' traits are quite a bit different in behavior...
Especially that one acts at pre-rewrite time, and second pre-push time.
> > Take for example the situation where given commit is reference by
> > remote-tracking branch 'public/foo', and also by two local branches:
> > 'foo' with upstream 'public/foo', and local branch 'bar' with no upstream.
> >
> > Now it is quite obvious that this feature should prevent rewriting 'foo'
> > branch, for which commits are published upstream. But what about branch
> > 'bar'? Should we prevent rewriting (e.g. rebase) here too? What about
> > rewinding 'bar' to point somewhere else. What if 'bar' is really detached
> > HEAD?
> >
> > These questions need to be answered...
>
> Good point. There are two questions we may need to answer: "Has commit
> X ever been published?", and "Has commit X ever been published in the
> context of branch Y?". In the latter case, we do indeed need to take
> the upstream branch into account.
I think the second one is more interested for rewrite safeties.
> Basically, there are three different "levels" for this rewrite/publish
> protection to run at:
>
> 1. Do not meddle at all. This is the current behavior, and assumes
> that if the user rewrites and pushes something, the user knows what
> he/she is doing, and Git should not meddle (obviously unless the
> server refuses the push).
I think that there should be some easy way to force such behavior,
i.e. to discard rewrite safeties.
> 2. Warn/refuse rewriting commits in your upstream. This would only
> check branch X against its registered upstream. Only if there is a
> registered upstream, and you're about to rewrite commits that are
> reachable from the upstream remote-tracking branch, should Git
> intervene and warn/refuse the rewrite. This level would IMHO provide
> most of the benefit, and little or no trouble (i.e. false positives).
Right. I wonder if we can get usage statistics from Mercurial users
about usage of their "phases" feature... though mapping terminology
for example 'upstream' from Git to Mercurial and vice versa can be
a pain, I guess.
> 3. Warn/refuse rewriting _any_ 'public' commit. Refuse to rewrite any
> commit that is reachable from any remote-tracking branch. Some would
> say that this is a Good Thing(tm), since it prevents a commit from
> being _copied_ (i.e. rebased or cherry-picked) between branches (you'd
> be in this camp if you run a tightly-controlled workflow, where you
> e.g. mandate upmerging patches from the oldest applicable branch
> instead of cherry-picking patches from a newer branch). However, other
> people would say that this is too limiting, and imposes unnecessary
> rules on the workflow of the project (where e.g. copying (by way of
> git-rebase) a topic branch from one place to another would cause an
> annoying false positive).
Well, we could always 'deny' on 2nd, and just 'warn' on 3rd...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v5 09/12] branch: add --column
From: Junio C Hamano @ 2012-02-06 17:31 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328371156-4009-10-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> +test_expect_success 'git branch -v with column.ui ignored' '
> + git config column.ui column &&
> + COLUMNS=80 git branch -v | cut -c -10 >actual &&
> + git config --unset column.ui &&
> + cat >expected <<\EOF &&
> + a/b/c
> + abc
> + bam
> + bar
> + foo
> + j/k
> + l
> + m/m
> +* master
> + master2
> + n
> + o/o
> + o/p
> + q
> + r
> +EOF
For a test like these where you expect whitespaces at the end, please make
sure that these whitespaces are visible to people who read test scripts,
by doing something like this instead:
sed -e 's/#$//' >expected <<\EOF
a/b/c #
abc #
bam #
bar #
foo #
EOF
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Phil Hord @ 2012-02-06 17:31 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <4F29BEB7.1080901@web.de>
On Wed, Feb 1, 2012 at 5:37 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 31.01.2012 23:50, schrieb Phil Hord:
>> What I mean is that a developer may be completely focused on one
>> particular submodule (his domain). He does his work in this module,
>> and when it's ready he commits and pushes to the server. 'git status'
>> shows him that his directory is clean. But this is only because he
>> doesn't really know where the submodules top-directories are, so he
>> doesn't realize that he has changes in another submodule that he has
>> not committed. He has to know to run 'git status' from somewhere in
>> the superproject (ostensibly in the root directory of that
>> superproject). But he may forget since 'git status' already assured
>> him he was done.
> <snip>
>> I guess what would help here is something like the opposite of 'git
>> status' showing the status of descendant submodules; it would help if
>> it showed the status of sibling submodules and the superproject as
>> well.
>
> Hmm, I really think the fact that submodules are unaware that they
> are part of a superproject is a feature. I'd prefer seeing that kind
> of problem being tackled by the CI server and/or user education. Or
> maybe a pre-commit hook which issues a warning in that case?
I agree that submodule isolation is a feature essential to the
architecture of git and the submodules implementation. But it is also
a limitation, not just of this example. A pre-commit hook is a nice
idea, but it doesn't help 'git status' (which is the standard go-to
answer point for "where am I").
This has me thinking more about recursing siblings now, though. I find
myself typing something like this quite a lot:
git submodule foreach 'git grep "someFunction" || :'
Or worse (in that the UI is more unwieldy):
git submodule foreach 'git log --oneline "-SsomeFunction" || :'
But what I want is this:
git --git-dir=${TOP}/../.git grep --recurse-submodules "someFunction"
But not really, because I am lazy and that is too much typing.
git grep --include-siblings "someFunction"
Maybe I can add a "sib" macro to get this:
git sib grep "someFunction"
But now I've really wandered off-topic.
Phil
^ permalink raw reply
* Re: [PATCH v5 00/12] Column display
From: Junio C Hamano @ 2012-02-06 17:58 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1328371156-4009-1-git-send-email-pclouds@gmail.com>
I've added two patches on top of this series and queued on 'pu'.
The first one is a fix for style issues; this is limited to absolute
minimum and I wouldn't be surprised if I fixed one instance of the same
class of style violations just as an example while ignoring bunch of
similar ones.
The second was to see how "git branch" behaves when it sees one absurdly
long entry (longer than $COLUMN width) among the normal ones, primarily
done just for fun.
You may want to tease the first one apart and squash them in to where the
problem originates. The second one is optional, but as you hinted an
interest on coming up with a heuristic to cram more info by making some
oddball entries span multi-column, it might serve as a good starting point
to think about the possible issues.
Thanks for an amusing read.
^ permalink raw reply
* Re: [PATCH 2/3] tag: die when listing missing or corrupt objects
From: Junio C Hamano @ 2012-02-06 18:04 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Tom Grennan, git, jasampler
In-Reply-To: <20120206083832.GA9425@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> OK, that's easy enough to do. Should we show lightweight tags to commits
> for backwards compatibility (and just drop the parse_signature junk in
> that case)? The showing of blobs or trees is the really bad thing, I
> think.
I think that is a sensible thing to do. I see many end-user documents on
the Interweb that uses lightweight "git tag", and I do not think they are
shooting for brevity of their illustration. The authors of these pages do
primarily use lightweight tags because they do not have anything more to
add in the message more than the log message commit objects they point at.
And it is a huge regression if we stop showing them if they are used to
use "tag -n".
^ permalink raw reply
* Re: [PATCH 2/3] tag: die when listing missing or corrupt objects
From: Junio C Hamano @ 2012-02-06 18:13 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Tom Grennan, git, jasampler
In-Reply-To: <20120206083832.GA9425@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> OK, that's easy enough to do. Should we show lightweight tags to commits
> for backwards compatibility (and just drop the parse_signature junk in
> that case)? The showing of blobs or trees is the really bad thing, I
> think.
For now, dropping 3/3 and queuing this instead...
---
Subject: tag: do not show non-tag contents with "-n"
"git tag -n" did not check the type of the object it is reading the top n
lines from. At least, avoid showing the beginning of trees and blobs when
dealing with lightweight tags that point at them.
As the payload of a tag and a commit look similar in that they both start
with a header block, which is skipped for the purpose of "-n" output,
followed by human readable text, allow the message of commit objects to be
shown just like the contents of tag objects. This avoids regression for
people who have been using "tag -n" to show the log messages of commits
that are pointed at by lightweight tags.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/tag.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 1e27f5c..6d6ae88 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -95,19 +95,20 @@ static void show_tag_lines(const unsigned char *sha1, int lines)
buf = read_sha1_file(sha1, &type, &size);
if (!buf)
die_errno("unable to read object %s", sha1_to_hex(sha1));
- if (!size) {
- free(buf);
- return;
- }
+ if (type != OBJ_COMMIT || type != OBJ_TAG)
+ goto free_return;
+ if (!size)
+ die("an empty %s object %s?",
+ typename(type), sha1_to_hex(sha1));
/* skip header */
sp = strstr(buf, "\n\n");
- if (!sp) {
- free(buf);
- return;
- }
- /* only take up to "lines" lines, and strip the signature */
- size = parse_signature(buf, size);
+ if (!sp)
+ goto free_return;
+
+ /* only take up to "lines" lines, and strip the signature from a tag */
+ if (type == OBJ_TAG)
+ size = parse_signature(buf, size);
for (i = 0, sp += 2; i < lines && sp < buf + size; i++) {
if (i)
printf("\n ");
@@ -118,6 +119,7 @@ static void show_tag_lines(const unsigned char *sha1, int lines)
break;
sp = eol + 1;
}
+free_return:
free(buf);
}
^ permalink raw reply related
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-02-06 18:17 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Haggerty, Andrew Ardill, git
In-Reply-To: <20120206085707.GA24149@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> PS I probably would have done it as:
>
> git init vendor
> cd vendor
> import import import
> cd ..
>
> git init project
> cd project
> git fetch ../vendor master:vendor
>
> but I don't think there's anything wrong with your approach (in fact,
> it's slightly more efficient).
Probably I am slower than my usual slow self this morning. Does Michael's
approach go like this:
git init project
cd project
import import import
git branch -m vendor
git checkout -b master
to fork from third-party codebase?
Or Michael had his own 'master' already and wanted an independent orphaned
history from vendor, perhaps like this?
git init project
cd project
work work work
git checkout --orphan vendor
: perhaps "git clean -f" here???
import import import
git checkout master
: rootless merge???
git merge vendor
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox