[Bug] git gc with alternates tries accessing non-existing directory
I have a non-bare repository /home/a set up with an alternate to the bare repository /b. Running git gc on /home/a produces below's error error: unable to open /b/objects/56/b969ffdf64343777a069260f41761dc0551bfa/00: Not a directory The referenced file /b/objects/56/b969ffdf64343777a069260f41761dc0551bfa exists, but is not a directory. Thanks, - Eph git --version git version 2.3.0 cat /home/a/.git/objects/info/alternates /b/objects cd /home/a git gc Counting objects: 4046, done. Delta compression using up to 32 threads. Compressing objects: 100% (970/970), done. Writing objects: 100% (4046/4046), done. Total 4046 (delta 3074), reused 4046 (delta 3074) error: unable to open /b/objects/56/b969ffdf64343777a069260f41761dc0551bfa/00: Not a directory fatal: unable to mark recent objects error: failed to run prune ls -al /b/objects/56 -r--r--r-- ## 199 Mar 11 11:21 0ae39e1f65160f0256aa1411d6c3c6d36cb79a -r--r--r-- ## 1717 Mar 10 17:15 0c22f578c0f47c54bcd9de899701fceb08607a -r--r--r-- ## 477 Mar 5 11:34 0dc6b1dac39e4366c739ede698232ce1a02d1a -r--r--r-- ##51 Mar 5 09:04 14ee4b890b61b9e8ac07a05b878d44ed2138da -r--r- ## 431 Mar 5 15:58 161adcb8a02868940f2fab2e7eb084de9a106a -r--r--r-- ## 264 Mar 11 14:35 30af07890c31e54bc92da16ee6557d9ffba21a -r--r- ## 795 Mar 13 09:35 4ab2cb69f2f2e22215772f2ae604d24863ab9a -r--r--r-- ## 181 Mar 9 12:38 ad631112e5971e872a88946cbae8176b4563ba -r--r- ## 14574 Mar 2 16:25 b969ffdf64343777a069260f41761dc0551bfa -r--r--r-- ## 821 Mar 6 11:16 c55b01a68613c0ea1946869bb9b72370b8738a -r--r--r-- ## 8199 Mar 4 12:14 cb732d06ae2adf04150766a70ca079e11801aa -r--r- ## 261 Mar 10 09:47 fadac9ce2f87392986438737b329fc6cab18ca -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] nd/multiple-work-trees updates
On Wed, Mar 18, 2015 at 3:04 PM, Ephrim Khong dr.kh...@gmail.com wrote: Without having looked into this and nd/multiple-work-trees, but with make multiple checkouts aware of each other in mind: Could this mechanism be re-used to make alternates aware of each other, to mitigate the dangers of having git gc on an alternate remove objects that are used by a referencing repository? If we can turn on ref namespace and make $GIT_DIR/config and hooks per worktree, I think it may have a chance of replacing alternate object mechanism entirely: one object database, one ref database (but refs of each worktree is namespaced so no conflicts), multiple worktrees, multiple config files, multiple hooks. Because some config keys affect object database, having multiple/conflicting config keys imply that this worktree may change object database in a way trhat impacts performance (not correctness) of another worktree. Later on when we have multiple ref backends, if config keys can change ref backend behavior (or even choose the backend), we may run into other problems. This problem might go away if we define that those global config keys can't be per-worktree.. In short, I am good at confusing people. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC/GSOC] make git-pull a builtin
Paul Tan writes: I would like to share this very rough prototype with everyone. ... I started this as a just-for-fun exercise to learn about the git internal API I started to rewrite git-pull for similar reasons a couple of months ago, but I haven't had time to complete it. It looks like my version has less work remaining than yours, would you like me to share it? Finally, there is the possibility that in the process of conversion bugs or incompatible behavioral changes may be introduced which are not caught by the test suite. Ideally, I think the solution is to improve the test suite and make it as comprehensive as possible, but writing a comprehensive test suite may be too time consuming. This is why I haven't had time to finish and submit my patch. While the c code is pretty much complete, I felt the test suite needed some extension before I could be confident the new code is correct. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 2/8] Move reset_tree from builtin/checkout.c to unpack-trees.c
---
builtin/checkout.c | 40 +++-
builtin/merge.c| 29 +++--
unpack-trees.c | 33 +
unpack-trees.h | 4
4 files changed, 47 insertions(+), 59 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..93b18ad 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -362,40 +362,6 @@ static void describe_detached_head(const char *msg, struct
commit *commit)
strbuf_release(sb);
}
-static int reset_tree(struct tree *tree, const struct checkout_opts *o,
- int worktree, int *writeout_error)
-{
- struct unpack_trees_options opts;
- struct tree_desc tree_desc;
-
- memset(opts, 0, sizeof(opts));
- opts.head_idx = -1;
- opts.update = worktree;
- opts.skip_unmerged = !worktree;
- opts.reset = 1;
- opts.merge = 1;
- opts.fn = oneway_merge;
- opts.verbose_update = !o-quiet isatty(2);
- opts.src_index = the_index;
- opts.dst_index = the_index;
- parse_tree(tree);
- init_tree_desc(tree_desc, tree-buffer, tree-size);
- switch (unpack_trees(1, tree_desc, opts)) {
- case -2:
- *writeout_error = 1;
- /*
-* We return 0 nevertheless, as the index is all right
-* and more importantly we have made best efforts to
-* update paths in the work tree, and we cannot revert
-* them.
-*/
- case 0:
- return 0;
- default:
- return 128;
- }
-}
-
struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
@@ -427,7 +393,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
resolve_undo_clear();
if (opts-force) {
- ret = reset_tree(new-commit-tree, opts, 1, writeout_error);
+ ret = reset_tree(new-commit-tree, opts-quiet, 1,
writeout_error);
if (ret)
return ret;
} else {
@@ -513,7 +479,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
o.verbosity = 0;
work = write_tree_from_memory(o);
- ret = reset_tree(new-commit-tree, opts, 1,
+ ret = reset_tree(new-commit-tree, opts-quiet, 1,
writeout_error);
if (ret)
return ret;
@@ -522,7 +488,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
o.branch2 = local;
merge_trees(o, new-commit-tree, work,
old-commit-tree, result);
- ret = reset_tree(new-commit-tree, opts, 0,
+ ret = reset_tree(new-commit-tree, opts-quiet, 0,
writeout_error);
if (ret)
return ret;
diff --git a/builtin/merge.c b/builtin/merge.c
index 6babf39..c51e4f5 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -269,33 +269,18 @@ static void read_empty(unsigned const char *sha1, int
verbose)
die(_(read-tree failed));
}
-static void reset_hard(unsigned const char *sha1, int verbose)
-{
- int i = 0;
- const char *args[6];
-
- args[i++] = read-tree;
- if (verbose)
- args[i++] = -v;
- args[i++] = --reset;
- args[i++] = -u;
- args[i++] = sha1_to_hex(sha1);
- args[i] = NULL;
-
- if (run_command_v_opt(args, RUN_GIT_CMD))
- die(_(read-tree failed));
-}
-
-static void restore_state(const unsigned char *head,
+static void restore_state(struct commit *head_commit,
const unsigned char *stash)
{
struct strbuf sb = STRBUF_INIT;
const char *args[] = { stash, apply, NULL, NULL };
+ int error = 0;
if (is_null_sha1(stash))
return;
- reset_hard(head, 1);
+ if (reset_tree(head_commit-tree, 0, 1, error) || error)
+ die(_(read-tree failed));
args[2] = sha1_to_hex(stash);
@@ -1409,7 +1394,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
int ret;
if (i) {
printf(_(Rewinding the tree to pristine...\n));
- restore_state(head_commit-object.sha1, stash);
+ restore_state(head_commit, stash);
}
if (use_strategies_nr != 1)
printf(_(Trying merge strategy %s...\n),
@@ -1475,7 +1460,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
* it up.
*/
if (!best_strategy) {
-
[PATCH/WIP 4/8] rebase: turn rebase--merge into a separate program
---
Makefile | 2 +-
git-rebase--merge.sh (mode +x) | 34 ++
git-rebase.sh | 2 ++
3 files changed, 37 insertions(+), 1 deletion(-)
mode change 100644 = 100755 git-rebase--merge.sh
diff --git a/Makefile b/Makefile
index 93151f4..7ee8df7 100644
--- a/Makefile
+++ b/Makefile
@@ -446,6 +446,7 @@ SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
+SCRIPT_SH += git-rebase--merge.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
@@ -455,7 +456,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
SCRIPT_LIB += git-rebase--interactive
-SCRIPT_LIB += git-rebase--merge
SCRIPT_LIB += git-sh-setup
SCRIPT_LIB += git-sh-i18n
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
old mode 100644
new mode 100755
index b10f2cf..baaef41
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -3,8 +3,42 @@
# Copyright (c) 2010 Junio C Hamano.
#
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action rebase
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
prec=4
+write_basic_state () {
+ echo $head_name $state_dir/head-name
+ echo $onto $state_dir/onto
+ echo $orig_head $state_dir/orig-head
+ echo $GIT_QUIET $state_dir/quiet
+ test t = $verbose : $state_dir/verbose
+ test -n $strategy echo $strategy $state_dir/strategy
+ test -n $strategy_opts echo $strategy_opts \
+ $state_dir/strategy_opts
+ test -n $allow_rerere_autoupdate echo $allow_rerere_autoupdate
\
+ $state_dir/allow_rerere_autoupdate
+}
+
+move_to_original_branch () {
+ case $head_name in
+ refs/*)
+ message=rebase finished: $head_name onto $onto
+ git update-ref -m $message \
+ $head_name $(git rev-parse HEAD) $orig_head
+ git symbolic-ref \
+ -m rebase finished: returning to $head_name \
+ HEAD $head_name ||
+ die $(gettext Could not move back to $head_name)
+ ;;
+ esac
+}
+
read_state () {
onto_name=$(cat $state_dir/onto_name)
end=$(cat $state_dir/end)
diff --git a/git-rebase.sh b/git-rebase.sh
index 42e868d..ff2c2ae 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -156,6 +156,8 @@ run_specific_rebase () {
export state_dir verbose strategy strategy_opts
if [ $type = am ]; then
exec git-rebase--am
+ elif [ $type = merge ]; then
+ exec git-rebase--merge
else
. git-rebase--$type
fi
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 6/8] rebase: remove unused function
git-rebase.sh is no longer a dependency for rebase subscripts. This
function is only used by subscripts only, which now becomes useless.
---
git-rebase.sh | 13 -
1 file changed, 13 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 86119e7..d941239 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -102,19 +102,6 @@ read_basic_state () {
allow_rerere_autoupdate=$(cat
$state_dir/allow_rerere_autoupdate)
}
-write_basic_state () {
- echo $head_name $state_dir/head-name
- echo $onto $state_dir/onto
- echo $orig_head $state_dir/orig-head
- echo $GIT_QUIET $state_dir/quiet
- test t = $verbose : $state_dir/verbose
- test -n $strategy echo $strategy $state_dir/strategy
- test -n $strategy_opts echo $strategy_opts \
- $state_dir/strategy_opts
- test -n $allow_rerere_autoupdate echo $allow_rerere_autoupdate
\
- $state_dir/allow_rerere_autoupdate
-}
-
output () {
case $verbose in
'')
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 8/8] Build in rebase
---
Makefile| 2 +-
builtin.h | 1 +
builtin/rebase.c (new) | 752
commit.c| 4 +-
commit.h| 4 +-
contrib/examples/git-rebase.sh (new +x) | 532 ++
git-rebase.sh (gone)| 532 --
git.c | 1 +
8 files changed, 1291 insertions(+), 537 deletions(-)
create mode 100644 builtin/rebase.c
create mode 100755 contrib/examples/git-rebase.sh
delete mode 100755 git-rebase.sh
diff --git a/Makefile b/Makefile
index 83972a2..223d50b 100644
--- a/Makefile
+++ b/Makefile
@@ -444,7 +444,6 @@ SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
-SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
SCRIPT_SH += git-rebase--interactive.sh
SCRIPT_SH += git-rebase--merge.sh
@@ -903,6 +902,7 @@ BUILTIN_OBJS += builtin/prune-packed.o
BUILTIN_OBJS += builtin/prune.o
BUILTIN_OBJS += builtin/push.o
BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase.o
BUILTIN_OBJS += builtin/receive-pack.o
BUILTIN_OBJS += builtin/reflog.o
BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index 7e7bbd6..431bbbf 100644
--- a/builtin.h
+++ b/builtin.h
@@ -108,6 +108,7 @@ extern int cmd_prune(int argc, const char **argv, const
char *prefix);
extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
extern int cmd_push(int argc, const char **argv, const char *prefix);
extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase(int argc, const char **argv, const char *prefix);
extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
extern int cmd_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/builtin/rebase.c b/builtin/rebase.c
new file mode 100644
index 000..29eff0e
--- /dev/null
+++ b/builtin/rebase.c
@@ -0,0 +1,752 @@
+#include cache.h
+#include parse-options.h
+#include argv-array.h
+#include run-command.h
+#include tree-walk.h
+#include unpack-trees.h
+#include diff.h
+#include commit.h
+#include revision.h
+#include submodule.h
+#include commit.h
+#include dir.h
+
+#define REBASE_AM 1
+#define REBASE_MERGE 2
+#define REBASE_INTERACTIVE 3
+
+static const char * const builtin_rebase_usage[] = {
+ N_(git rebase [-i] [options] [--exec cmd] [--onto newbase]
[upstream] [branch]),
+ N_(git rebase [-i] [options] [--exec cmd] [--onto newbase] --root
[branch]),
+ N_(git rebase --continue | --abort | --skip | --edit-todo),
+ NULL
+};
+
+/* These are exported as environment variables for git-rebase--*.sh */
+static int action_abort;
+static int action_continue;
+static int action_skip;
+static int autosquash;
+static int force_rebase;
+static int keep_empty;
+static int preserve_merges;
+static int quiet;
+static int rerere_autoupdate;
+static int root;
+static int verbose;
+static const char *exec_cmd;
+static const char *head_name;
+static const char *onto;
+static const char *orig_head;
+static struct strbuf revisions = STRBUF_INIT;
+static const char *state_basedir;
+static const char *strategy;
+static const char *strategy_opts;
+static const char *switch_to;
+static const char *squash_onto;
+
+static int do_merge;
+static int edit_todo;
+static int interactive_rebase;
+static int rebase_type;
+static int show_stat;
+
+static char *read_file(const char *name)
+{
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_read_file(sb,
+git_path(%s/%s, state_basedir, name),
+0) = 0)
+ return strbuf_detach(sb, NULL);
+ else
+ return NULL;
+}
+
+static char *read_file_or_die(const char *name)
+{
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_read_file_or_die(sb,
+ git_path(%s/%s, state_basedir, name),
+ 0);
+ return strbuf_detach(sb, NULL);
+}
+
+static void read_bool(const char *name, int *var)
+{
+ char *buf = read_file(name);
+ if (buf) {
+ *var = buf[0] !isspace(buf[0]);
+ free(buf);
+ }
+}
+
+static int read_bool_or_die(const char *name)
+{
+ char *buf = read_file_or_die(name);
+ int ret = buf[0] !isspace(buf[0]);
+ free(buf);
+ return ret;
+}
+
+/*
+ * Note:
+ *
+ * After git-rebase--*.sh are integrated, we should probably adopt
+ * git-config format and store everything in one file instead of so
+ * many like this. state_dir will be something different to avoid
+ * misuse by old rebase versions. This code will stay for a few major
+ * releases before being phased out.
+ */
+static void
[PATCH/WIP 7/8] rebase: move resolvemsg to rebase--* scripts
---
git-rebase--am.sh | 5 +
git-rebase--interactive.sh | 5 +
git-rebase--merge.sh | 5 +
git-rebase.sh | 7 +--
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index ab84330..399956b 100755
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -9,6 +9,11 @@ set_reflog_action rebase
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg=
+$(gettext 'When you have resolved this problem, run git rebase --continue.
+If you prefer to skip this patch, run git rebase --skip instead.
+To check out the original branch and stop rebasing, run git rebase --abort.')
+
write_basic_state () {
echo $head_name $state_dir/head-name
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b1c71a9..337dec3 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -16,6 +16,11 @@ set_reflog_action rebase -i ($action)
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg=
+$(gettext 'When you have resolved this problem, run git rebase --continue.
+If you prefer to skip this patch, run git rebase --skip instead.
+To check out the original branch and stop rebasing, run git rebase --abort.')
+
# The file containing rebase commands, comments, and empty lines.
# This file is created by git rebase -i then edited by the user. As
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index baaef41..7e240be 100755
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -9,6 +9,11 @@ set_reflog_action rebase
require_work_tree_exists
GIT_QUIET=$git_quiet
+resolvemsg=
+$(gettext 'When you have resolved this problem, run git rebase --continue.
+If you prefer to skip this patch, run git rebase --skip instead.
+To check out the original branch and stop rebasing, run git rebase --abort.')
+
prec=4
diff --git a/git-rebase.sh b/git-rebase.sh
index d941239..38530e8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -49,11 +49,6 @@ cd_to_toplevel
LF='
'
ok_to_skip_pre_rebase=
-resolvemsg=
-$(gettext 'When you have resolved this problem, run git rebase --continue.
-If you prefer to skip this patch, run git rebase --skip instead.
-To check out the original branch and stop rebasing, run git rebase --abort.')
-
unset onto
cmd=
strategy=
@@ -139,7 +134,7 @@ run_specific_rebase () {
git_quiet=$GIT_QUIET
export GIT_PAGER
export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
- export onto orig_head rebase_root resolvemsg revisions
+ export onto orig_head rebase_root revisions
export state_dir verbose strategy strategy_opts
export autosquash cmd force_rebase preserve_merges squash_onto
switch_to upstream
exec git-rebase--$type
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 1/8] strbuf: add and use strbuf_read_file_or_die()
---
builtin/blame.c | 4 ++--
builtin/commit.c | 16 +---
builtin/merge.c | 3 +--
builtin/notes.c | 4 ++--
builtin/tag.c| 7 ++-
strbuf.c | 8
strbuf.h | 1 +
7 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index bc6c899..503595c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2193,8 +2193,8 @@ static struct commit *fake_working_tree_commit(struct
diff_options *opt,
if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV)
textconv_object(read_from, mode, null_sha1, 0,
buf_ptr, buf_len))
strbuf_attach(buf, buf_ptr, buf_len, buf_len +
1);
- else if (strbuf_read_file(buf, read_from, st.st_size)
!= st.st_size)
- die_errno(cannot open or read '%s',
read_from);
+ else
+ strbuf_read_file_or_die(buf, read_from,
st.st_size);
break;
case S_IFLNK:
if (strbuf_readlink(buf, read_from, st.st_size) 0)
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..dad9acf 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -612,9 +612,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
die_errno(_(could not read log from standard input));
hook_arg1 = message;
} else if (logfile) {
- if (strbuf_read_file(sb, logfile, 0) 0)
- die_errno(_(could not read log file '%s'),
- logfile);
+ strbuf_read_file_or_die(sb, logfile, 0);
hook_arg1 = message;
} else if (use_message) {
buffer = strstr(use_message_buffer, \n\n);
@@ -634,16 +632,13 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
sb, ctx);
hook_arg1 = message;
} else if (!stat(git_path(MERGE_MSG), statbuf)) {
- if (strbuf_read_file(sb, git_path(MERGE_MSG), 0) 0)
- die_errno(_(could not read MERGE_MSG));
+ strbuf_read_file_or_die(sb, git_path(MERGE_MSG), 0);
hook_arg1 = merge;
} else if (!stat(git_path(SQUASH_MSG), statbuf)) {
- if (strbuf_read_file(sb, git_path(SQUASH_MSG), 0) 0)
- die_errno(_(could not read SQUASH_MSG));
+ strbuf_read_file_or_die(sb, git_path(SQUASH_MSG), 0);
hook_arg1 = squash;
} else if (template_file) {
- if (strbuf_read_file(sb, template_file, 0) 0)
- die_errno(_(could not read '%s'), template_file);
+ strbuf_read_file_or_die(sb, template_file, 0);
hook_arg1 = template;
clean_message_contents = 0;
}
@@ -1497,8 +1492,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
fclose(fp);
strbuf_release(m);
if (!stat(git_path(MERGE_MODE), statbuf)) {
- if (strbuf_read_file(sb, git_path(MERGE_MODE), 0)
0)
- die_errno(_(could not read MERGE_MODE));
+ strbuf_read_file_or_die(sb, git_path(MERGE_MODE), 0);
if (!strcmp(sb.buf, no-ff))
allow_fast_forward = 0;
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 9307e9c..6babf39 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -769,8 +769,7 @@ static void read_merge_msg(struct strbuf *msg)
{
const char *filename = git_path(MERGE_MSG);
strbuf_reset(msg);
- if (strbuf_read_file(msg, filename, 0) 0)
- die_errno(_(Could not read from '%s'), filename);
+ strbuf_read_file_or_die(msg, filename, 0);
}
static void write_merge_state(struct commit_list *);
diff --git a/builtin/notes.c b/builtin/notes.c
index 453457a..3210c7f 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -252,8 +252,8 @@ static int parse_file_arg(const struct option *opt, const
char *arg, int unset)
if (!strcmp(arg, -)) {
if (strbuf_read((msg-buf), 0, 1024) 0)
die_errno(_(cannot read '%s'), arg);
- } else if (strbuf_read_file((msg-buf), arg, 1024) 0)
- die_errno(_(could not open or read '%s'), arg);
+ } else
+ strbuf_read_file_or_die((msg-buf), arg, 0);
stripspace((msg-buf), 0);
msg-given = 1;
diff --git a/builtin/tag.c b/builtin/tag.c
index 9c3e067..69f4ea3 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -540,11 +540,8 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (!strcmp(msgfile, -)) {
if (strbuf_read(buf, 0, 1024) 0)
[PATCH/WIP 5/8] rebase: turn rebase--interactive into a separate program
---
Makefile | 2 +-
git-rebase--interactive.sh (mode +x) | 36 +++-
git-rebase.sh| 9 ++---
3 files changed, 38 insertions(+), 9 deletions(-)
mode change 100644 = 100755 git-rebase--interactive.sh
diff --git a/Makefile b/Makefile
index 7ee8df7..83972a2 100644
--- a/Makefile
+++ b/Makefile
@@ -446,6 +446,7 @@ SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-rebase--am.sh
+SCRIPT_SH += git-rebase--interactive.sh
SCRIPT_SH += git-rebase--merge.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
@@ -455,7 +456,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--interactive
SCRIPT_LIB += git-sh-setup
SCRIPT_LIB += git-sh-i18n
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
old mode 100644
new mode 100755
index 44901d5..b1c71a9
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -9,7 +9,14 @@
#
# The original idea comes from Eric W. Biederman, in
# http://article.gmane.org/gmane.comp.version-control.git/22407
-#
+
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action rebase -i ($action)
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
# The file containing rebase commands, comments, and empty lines.
# This file is created by git rebase -i then edited by the user. As
# the lines are processed, they are removed from the front of this
@@ -80,6 +87,33 @@ rewritten_pending=$state_dir/rewritten-pending
GIT_CHERRY_PICK_HELP=$resolvemsg
export GIT_CHERRY_PICK_HELP
+write_basic_state () {
+ echo $head_name $state_dir/head-name
+ echo $onto $state_dir/onto
+ echo $orig_head $state_dir/orig-head
+ echo $GIT_QUIET $state_dir/quiet
+ test t = $verbose : $state_dir/verbose
+ test -n $strategy echo $strategy $state_dir/strategy
+ test -n $strategy_opts echo $strategy_opts \
+ $state_dir/strategy_opts
+ test -n $allow_rerere_autoupdate echo $allow_rerere_autoupdate
\
+ $state_dir/allow_rerere_autoupdate
+}
+
+output () {
+ case $verbose in
+ '')
+ output=$($@ 21 )
+ status=$?
+ test $status != 0 printf %s\n $output
+ return $status
+ ;;
+ *)
+ $@
+ ;;
+ esac
+}
+
warn () {
printf '%s\n' $* 2
}
diff --git a/git-rebase.sh b/git-rebase.sh
index ff2c2ae..86119e7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -154,13 +154,8 @@ run_specific_rebase () {
export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
export onto orig_head rebase_root resolvemsg revisions
export state_dir verbose strategy strategy_opts
- if [ $type = am ]; then
- exec git-rebase--am
- elif [ $type = merge ]; then
- exec git-rebase--merge
- else
- . git-rebase--$type
- fi
+ export autosquash cmd force_rebase preserve_merges squash_onto
switch_to upstream
+ exec git-rebase--$type
}
run_pre_rebase_hook () {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 3/8] rebase: turn rebase--am into a separate program
---
Makefile| 2 +-
git-rebase--am.sh (mode +x) | 34 ++
git-rebase.sh | 11 ++-
3 files changed, 45 insertions(+), 2 deletions(-)
mode change 100644 = 100755 git-rebase--am.sh
diff --git a/Makefile b/Makefile
index 1b30d7b..93151f4 100644
--- a/Makefile
+++ b/Makefile
@@ -445,6 +445,7 @@ SCRIPT_SH += git-mergetool.sh
SCRIPT_SH += git-pull.sh
SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
+SCRIPT_SH += git-rebase--am.sh
SCRIPT_SH += git-repack.sh
SCRIPT_SH += git-request-pull.sh
SCRIPT_SH += git-stash.sh
@@ -453,7 +454,6 @@ SCRIPT_SH += git-web--browse.sh
SCRIPT_LIB += git-mergetool--lib
SCRIPT_LIB += git-parse-remote
-SCRIPT_LIB += git-rebase--am
SCRIPT_LIB += git-rebase--interactive
SCRIPT_LIB += git-rebase--merge
SCRIPT_LIB += git-sh-setup
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
old mode 100644
new mode 100755
index 97f31dc..ab84330
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -3,6 +3,40 @@
# Copyright (c) 2010 Junio C Hamano.
#
+. git-sh-setup
+. git-sh-i18n
+set_reflog_action rebase
+require_work_tree_exists
+
+GIT_QUIET=$git_quiet
+
+write_basic_state () {
+ echo $head_name $state_dir/head-name
+ echo $onto $state_dir/onto
+ echo $orig_head $state_dir/orig-head
+ echo $GIT_QUIET $state_dir/quiet
+ test t = $verbose : $state_dir/verbose
+ test -n $strategy echo $strategy $state_dir/strategy
+ test -n $strategy_opts echo $strategy_opts \
+ $state_dir/strategy_opts
+ test -n $allow_rerere_autoupdate echo $allow_rerere_autoupdate
\
+ $state_dir/allow_rerere_autoupdate
+}
+
+move_to_original_branch () {
+ case $head_name in
+ refs/*)
+ message=rebase finished: $head_name onto $onto
+ git update-ref -m $message \
+ $head_name $(git rev-parse HEAD) $orig_head
+ git symbolic-ref \
+ -m rebase finished: returning to $head_name \
+ HEAD $head_name ||
+ die $(gettext Could not move back to $head_name)
+ ;;
+ esac
+}
+
case $action in
continue)
git am --resolved --resolvemsg=$resolvemsg
diff --git a/git-rebase.sh b/git-rebase.sh
index b2f1c76..42e868d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -149,7 +149,16 @@ run_specific_rebase () {
export GIT_EDITOR
autosquash=
fi
- . git-rebase--$type
+ git_quiet=$GIT_QUIET
+ export GIT_PAGER
+ export action allow_rerere_autoupdate git_am_opt git_quiet head_name
keep_empty
+ export onto orig_head rebase_root resolvemsg revisions
+ export state_dir verbose strategy strategy_opts
+ if [ $type = am ]; then
+ exec git-rebase--am
+ else
+ . git-rebase--$type
+ fi
}
run_pre_rebase_hook () {
--
2.3.0.rc1.137.g477eb31
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
It would help if you pasted the push output. For example, does it stop at 20% at the compressing objects line or writing objects. How many total objects does it say? It rattles through compressing objects, and the first 20% of writing objects, then slows to a crawl. Writing objects: 33% (3647/10804), 80.00 MiB | 112.00 KiB/s Another question is how big are these binary files on average? Git considers a file is big if its size is 512MB or more (see core.bigFileThreshold). If your binary files are are mostly under this limit, but still big enough, then git may still try to compare new objects with these to find the smallest diff to send. If it's the case, you could set core.bigFileThreshold to cover these binary files. None of the files are very big (KB rather than MB), but there's a lot of them. I'll try setting the threshold to something lower, thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Need help deciding between subtree and submodule
My $0.02 based on $dayjob (disclaimer I've never used subtree) On Wed, Mar 18, 2015 at 11:14 AM, Robert Dailey rcdailey.li...@gmail.com wrote: At my workplace, the team is using Atlassian Stash + git We have a Core library that is our common code between various projects. To avoid a single monolithic repository and to allow our apps and tools to be modularized into their own repos, I have considered moving Core to a subtree or submodule. Our environment is slightly different. Our projects are made up entirely of submodules, we don't embed submodules within a repo with actual code (side note: I know syslog-ng does so it might be worth having a look around there). Day to day development is done at the submodule level. A developer working on a particular feature is generally only touching one repo notwithstanding a little bit of to-and-fro as they work on the UI aspects. When changes do touch multiple submodules the pushes can generally be ordered in a sane manner. Things get a little complicated when there are interdependent changes, then those pushes require co-operation between submodule owners. The key to making this work is our build system. It is the thing that updates the project repo. After a successful build for all targets (we hope to add unit/regression tests one day) the submodules sha1s are updated and a new baseline (to borrow a clearcase term) is published. Developers can do git pull git submodule update to get the latest stable baseline, but they can also run git pull in a submodule if they want to be on the bleeding edge. I tried subtree and this is definitely far more transparent and simple to the team (simplicity is very important), however I notice it has problems with unnecessary conflicts when you do not do `git subtree push` for each `git subtree pull`. This is unnecessary overhead and complicates the log graph which I don't like. Submodule functionally works but it is complicated. We make heavy use of pull requests for code reviews (they are required due to company policy). Instead of a pull request being atomic and containing any app changes + accompanying Core changes, we now need to create two pull requests and manage them in proper order. Things also become more difficult when branching. All around it just feels like submodule would interfere and add more administration overhead on a day to day basis, affecting productivity. We do have policies around review etc. With submodules it does sometimes require engaging owners/reviewers from multiple repositories. Tools like Gerrit can help, particularly where multiple changes and reviewers are involved. Is there a third option here I'm missing? If only that issue with subtree could be addressed (the conflicts), it would be perfect enough for us I think. I have done all the stackoverflow reading and research I can manage at this point. I would really love some feedback from the actual git community on what would be a practical solution and structure here from a company perspective. There's the thing google use for android, I think it's called repo. There's a few googlers around here so mybe one of them will chime in. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
Teaching reset the - shorthand involves checking if any file named '-' exists.
check_filename() is used to perform this check.
When the @{-1} branch does not exist then it can be safely assumed that the
user is referring to the file '-',if any. If this file exists then it is reset
or else
a bad flag error is shown.
But if the @{-1} branch exists then it becomes ambiguous without the explicit
'--' disambiguation as to whether the user wants to reset the file '-' or if
he wants to reset the working tree to the previous branch. Hence the program
dies
with a message about the ambiguous argument.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sundararajan R dyou...@gmail.com
---
Thank you Eric and Junio for your patient feedback.
As verify_filename() and verify_non_filename() die and return,respectively when
passed the argument '-' without actually checking if such a file exists,
check_filename() has been used to perform this check. I hope it is okay.
builtin/reset.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..a126b38 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = HEAD;
unsigned char unused[20];
+ int file_named_minus = 0;
+ int shorthand = 0;
/*
* Possible arguments are:
*
@@ -205,6 +207,12 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -) !argv[1]) {
+ argv[0] = @{-1};
+ shorthand = 1;
+ if(check_filename(prefix, -))
+ file_named_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,11 +230,20 @@ static void parse_args(struct pathspec *pathspec,
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[0]);
+ if (file_named_minus) {
+ die(_(ambiguous argument '-': both revision
and filename\n
+ Use '--' to separate paths from revisions,
like this:\n
+ 'git command [revision...] --
[file...]'));
+ }
+ else if (!shorthand)
+ verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[0], 1);
+ if (shorthand)
+ argv[0] = -;
+ if (!file_named_minus)
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Wed, Mar 18, 2015 at 4:47 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Thank you for doing it. I was about to write another number parser and you did it :D Maybe you can add another patch to convert the only strtol in upload-pack.c to parse_ui. This place should accept positive number in base 10, plus sign is not accepted. If the general direction of this patch series is accepted, I'll gradually try to go through the codebase, replacing *all* integer parsing with these functions. So there's no need to request particular callers of strtol()/strtoul() to be converted; I'll get to them all sooner or later (I hope). Good to know. No there's no hurry in converting this particular place. It's just tightening things up, not bug fixing or anything. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 5:55 PM, Graham Hay grahamr...@gmail.com wrote: We have a fairly large repo (~2.4GB), mainly due to binary resources (for an ios app). I know this can generally be a problem, but I have a specific question. If I cut a branch, and edit a few (non-binary) files, and push, what should be uploaded? I assumed it was just the diff (I know whole compressed files are used, I mean the differences between my branch and where I cut it from). Is that correct? Because when I push, it grinds to a halt at the 20% mark, and feels like it's trying to push the entire repo. If I run git diff --stat --cached origin/foo I see the files I would expect (i.e. just those that have changed). If I run git format-patch origin/foo..foo the patch files total 1.7MB, which should upload in just a few seconds, but I've had pushes take over an hour. I'm using git 2.2.2 on Mac OS X (Mavericks), and ssh (g...@github.com). Am I doing it wrong? Is this the expected behaviour? If not, is there anything I can do to debug it? It would help if you pasted the push output. For example, does it stop at 20% at the compressing objects line or writing objects. How many total objects does it say? Another question is how big are these binary files on average? Git considers a file is big if its size is 512MB or more (see core.bigFileThreshold). If your binary files are are mostly under this limit, but still big enough, then git may still try to compare new objects with these to find the smallest diff to send. If it's the case, you could set core.bigFileThreshold to cover these binary files. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git with Lader logic
On Tue, Mar 17, 2015 at 11:33:54PM +, Bharat Suvarna wrote: Hi I am trying to find a way of using version control on PLC programmers like Allen Bradley PLC. I can't find a way of this. Could you please give me an idea if it will work with Plc programs. Which are basically Ladder logic. I'm not familiar with these programs, so I can't give you specific advice about this. Although git is not very picky about the contents, it is optimized to track text files. Things like showing diffs and merging files only works on text files. Git can track binary files, but there are some disadvantages: - Diff / merge doesn't work well - Compression is often difficult, so the repository size may grow depending on the size of the things stored These disadvantages are not limited to only git, other SCM systems also have to deal with these problems. So if the Ladder logic is represented as text source, there is no problem with it. If it'sbinary, there might be other sollutions better suited. Kevin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC/GSOC] make git-pull a builtin
Hi Stephen, On 2015-03-18 09:38, Stephen Robin wrote: Paul Tan writes: I would like to share this very rough prototype with everyone. ... I started this as a just-for-fun exercise to learn about the git internal API I started to rewrite git-pull for similar reasons a couple of months ago, but I haven't had time to complete it. It looks like my version has less work remaining than yours, would you like me to share it? Hmm. It always results in an unnecessary round trip if you ask people to ask you to share it. At this point, because Paul already made his work public, I would say that we should continue with his version. Please understand this as an encouragement for the future to share your code pro-actively, just like Git's own source code has been shared with you without the need for you to request it explicitly. For the record, Duy also already shared his C code to implement `git pull` in C publicly. I think that Paul decided to start again in order to understand every detail of the process of converting a shell script into a builtin; If that was the rationale, I would agree that it is a wise one. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP 0/8] Convert git-rebase.sh to C
In the spirit of sharing code proactively [1], despite my embarrassment, this is what I got for converting git-rebase.sh to C. Note that this is only about git-rebase.sh, not git-rebase--*.sh. Some changes in git-rebase.sh are pushed back to git-rebase--*.sh. The idea is we convert git-rebase.sh first, then the rest later. Anybody who wants to base their work on this code, feel free to --reset-author (and take all the bugs with you, I'm sure there are lots of them). This work is from 2013. git-rebase.sh has received lots of updates since then, so there's still lots of work to do. [1] http://article.gmane.org/gmane.comp.version-control.git/265699 Nguyễn Thái Ngọc Duy (8): strbuf: add and use strbuf_read_file_or_die() Move reset_tree from builtin/checkout.c to unpack-trees.c rebase: turn rebase--am into a separate program rebase: turn rebase--merge into a separate program rebase: turn rebase--interactive into a separate program rebase: remove unused function rebase: move resolvemsg to rebase--* scripts Build in rebase Makefile| 8 +- builtin.h | 1 + builtin/blame.c | 4 +- builtin/checkout.c | 40 +- builtin/commit.c| 16 +- builtin/merge.c | 32 +- builtin/notes.c | 4 +- builtin/rebase.c (new) | 752 builtin/tag.c | 7 +- commit.c| 4 +- commit.h| 4 +- contrib/examples/git-rebase.sh (new +x) | 532 ++ git-rebase--am.sh (mode +x) | 39 ++ git-rebase--interactive.sh (mode +x)| 41 +- git-rebase--merge.sh (mode +x) | 39 ++ git-rebase.sh (gone)| 544 --- git.c | 1 + strbuf.c| 8 + strbuf.h| 1 + unpack-trees.c | 33 ++ unpack-trees.h | 4 + 21 files changed, 1480 insertions(+), 634 deletions(-) create mode 100644 builtin/rebase.c create mode 100755 contrib/examples/git-rebase.sh mode change 100644 = 100755 git-rebase--am.sh mode change 100644 = 100755 git-rebase--interactive.sh mode change 100644 = 100755 git-rebase--merge.sh delete mode 100755 git-rebase.sh -- 2.3.0.rc1.137.g477eb31 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On 03/18/2015 11:03 AM, Jeff King wrote:
On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote:
But in case you have some reason that you want upload-pack.c to be
converted right away, I just pushed that change (plus some related
cleanups) to my GitHub repo [1]. The branch depends only on the first
patch of the numparse patch series.
By the way, some other packet line parsing code in that file doesn't
verify that there are no trailing characters on the lines that they
process. That might be another thing that should be tightened up.
Do you mean that upload-pack gets a pkt-line of length N that contains a
line of length M, and then doesn't check that M==N? We use the space
between M and N for passing capabilities and other metadata around.
Or do you mean that we see lines like:
want [0-9a-f]{40} ...\n
and do not bother looking at the ... that comes after the data we
expect? That I can believe, and I don't think it would hurt to tighten
up (we shouldn't need it for extensibility, as anybody trying to stick
extra data there should do so only after using a capability flag earlier
in the protocol).
The latter. For example here [1], the have command and its SHA-1 are
read from the line, but I don't see a check that there are no characters
after the SHA-1. The same here [2].
Michael
[1]
https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L404-L429
[2]
https://github.com/gitster/git/blob/9ab698f4000a736864c41f57fbae1e021ac27799/upload-pack.c#L550-L565
--
Michael Haggerty
mhag...@alum.mit.edu
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] not making corruption worse
On Tue, Mar 17, 2015 at 03:54:02PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: But it strikes me as weird that we consider the _tips_ of history to be special for ignoring breakage. If the tip of bar is broken, we omit it. But if the tip is fine, and there's breakage three commits down in the history, then doing a clone is going to fail horribly, as pack-objects realizes it can't generate the pack. So in practice, I'm not sure how much you're buying with the don't mention broken refs code. I think this is a trade-off between strictness and convenience. Is it preferrable that every time you try to clone a repository you get reminded that one of its refs point at a bogus object and you instead have to do git fetch $there with a refspec that excludes the broken one, or is it OK to allow clones and fetches silently succeed as if nothing is broken? I think the real issue is that we do not know on the server side what the client wants. Is it tell me the refs, so I can grab just the one I need, and I don't care about the broken ones? Or is it I want everything you have, and tell me if you can't serve it? You want strictness in the latter case, but not in the former. But if we were to err on the side of strictness, you could not do the former at all (because upload-pack would barf before the client even has a chance to say anything). I'm not sure if anyone will actually find GIT_REF_PARANOIA useful for something like that or not. As an environment variable, it may impact a filesystem-local clone, but it would not travel across a TCP connection. And doing so is tough, because the ref advertisement happens before the client speaks. If we ever have a client-speaks-first protocol, one extension could allow the client to flip the paranoia switch on the server. But my main goal here was really just making prune safer, so I'm happy enough with what this series does, for now. In some parts of the system there is a movement to make this trade off tweakable (hint: what happened to the knobs to fsck that allow certain kinds of broken objects in the object store? did the topic go anywhere?). This one so far lacked such a knob to tweak, and I view your paranoia bit as such a knob. I think I promised several times to review that topic and never got around to it. Which makes me a bad person. It is still on my todo list. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On 03/18/2015 12:05 AM, Duy Nguyen wrote: On Tue, Mar 17, 2015 at 11:00 PM, Michael Haggerty mhag...@alum.mit.edu wrote: Michael Haggerty (14): numparse: new module for parsing integral numbers cacheinfo_callback(): use convert_ui() when handling --cacheinfo write_subdirectory(): use convert_ui() for parsing mode handle_revision_opt(): use skip_prefix() in many places handle_revision_opt(): use convert_i() when handling -digit strtoul_ui(), strtol_i(): remove functions handle_revision_opt(): use convert_ui() when handling --abbrev= builtin_diff(): detect errors when parsing --unified argument opt_arg(): val is always non-NULL opt_arg(): use convert_i() in implementation opt_arg(): report errors parsing option values opt_arg(): simplify pointer handling diff_opt_parse(): use convert_i() when handling -lnum diff_opt_parse(): use convert_i() when handling --abbrev=num Thank you for doing it. I was about to write another number parser and you did it :D Maybe you can add another patch to convert the only strtol in upload-pack.c to parse_ui. This place should accept positive number in base 10, plus sign is not accepted. If the general direction of this patch series is accepted, I'll gradually try to go through the codebase, replacing *all* integer parsing with these functions. So there's no need to request particular callers of strtol()/strtoul() to be converted; I'll get to them all sooner or later (I hope). But in case you have some reason that you want upload-pack.c to be converted right away, I just pushed that change (plus some related cleanups) to my GitHub repo [1]. The branch depends only on the first patch of the numparse patch series. By the way, some other packet line parsing code in that file doesn't verify that there are no trailing characters on the lines that they process. That might be another thing that should be tightened up. Michael [1] https://github.com/mhagger/git.git, branch upload-pack-numparse -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Wed, Mar 18, 2015 at 10:47:40AM +0100, Michael Haggerty wrote:
But in case you have some reason that you want upload-pack.c to be
converted right away, I just pushed that change (plus some related
cleanups) to my GitHub repo [1]. The branch depends only on the first
patch of the numparse patch series.
By the way, some other packet line parsing code in that file doesn't
verify that there are no trailing characters on the lines that they
process. That might be another thing that should be tightened up.
Do you mean that upload-pack gets a pkt-line of length N that contains a
line of length M, and then doesn't check that M==N? We use the space
between M and N for passing capabilities and other metadata around.
Or do you mean that we see lines like:
want [0-9a-f]{40} ...\n
and do not bother looking at the ... that comes after the data we
expect? That I can believe, and I don't think it would hurt to tighten
up (we shouldn't need it for extensibility, as anybody trying to stick
extra data there should do so only after using a capability flag earlier
in the protocol).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[v3 PATCH 2/2] reset: add tests for git reset -
The failure case which occurs on teaching git is taught the '-' shorthand
is when there exists no branch pointed to by '@{-1}'. But, if there is a file
named - in the working tree, the user can be unambiguously assumed to be
referring to it while issuing this command.
The ambiguous case occurs when the @{-1} branch exists and file named '-' also
exists in the working tree. This are also treated as a failure case but here
the user is given advice as to how he can proceed.
Another potentially tricky case is when the file '@{-1}' exists. In this case,
the command should succeed as the user doesn't mention the file '@{-1}' and can
be safely assumed to be referring to the @{-1} branch.
Add tests to check the handling of these cases.
Also add a test to verify that reset - behaves like reset @{-1} when none
of the above cases are true.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sundararajan R dyou...@gmail.com
---
Thank you for your feedback Torsten and Eric. I have made modifications
suggested
by you. I have also acted on Matthieu's suggestions on the archive.
Please let me know if there is something else I should add.
I have also removed one irrelevant test from which we come to know of nothing
new.
t/t7102-reset.sh | 75
1 file changed, 75 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..f5a8e76 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,79 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no @{-1} branch should fail' '
+ test_when_finished rm -rf new
+ git init new
+ (
+ cd new
+ test_must_fail git reset - 2../actual
+ )
+ test_i18ngrep bad flag actual
+'
+
+test_expect_success 'reset - with no @{-1} branch and file named - should
succeed' '
+ test_when_finished rm -rf new
+ expected
+ git init new
+ (
+ cd new
+ echo Hello -
+ git add -
+ git reset - ../actual
+ )
+ test_cmp expected actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named - should fail' '
+ test_when_finished rm -rf new
+ git init new
+ (
+ cd new
+ echo Hello -
+ git add -
+ git commit -m first_commit
+ git checkout -b new_branch
+ -
+ git add -
+ test_must_fail git reset - 2../actual
+ )
+ test_i18ngrep ambiguous argument actual
+'
+
+test_expect_success 'reset - with @{-1} branch and file named @{-1} should
succeed' '
+ test_when_finished rm -rf new
+ git init new
+ (
+ cd new
+ echo Hello @{-1}
+ git add @{-1}
+ git commit -m first_commit
+ git checkout -b new_branch
+ @{-1}
+ git add @{-1}
+ git reset - ../actual
+ )
+ test_i18ngrep Unstaged actual
+'
+
+test_expect_success 'reset - with @{-1} branch and no file named - should
succeed' '
+ test_when_finished rm -rf new
+ git init new
+ (
+ cd new
+ echo Hey new_file
+ git add new_file
+ git commit -m first_commit
+ git checkout -b new_branch
+ new_file
+ git add new_file
+ git reset -
+ git status -uno actual
+ git add new_file
+ git reset @{-1}
+ git status -uno expected
+ test_cmp actual expected
+ )
+'
+
test_done
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] git gc with alternates tries accessing non-existing directory
On Wed, Mar 18, 2015 at 09:11:48AM +0100, Ephrim Khong wrote: I have a non-bare repository /home/a set up with an alternate to the bare repository /b. Running git gc on /home/a produces below's error [...] git --version git version 2.3.0 Try v2.3.2. It has b0a4264 (sha1_file: fix iterating loose alternate objects, 2015-02-08), which is almost certainly the problem. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] rev-list: refuse --first-parent combined with --bisect
On Tue, Mar 17, 2015 at 9:46 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: On Tue, Mar 17, 2015 at 7:33 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: Yes, but the user is supposed to not change the bad pointer for no good reason. That is irrelevant, no? Nobody is questioning that the user is supposed to judge if a commit is good or bad correctly. So if there is already a bad commit and the user gives another bad commit, that means that the user knows that it will replace the existing bad commit with the new one and that it's done for this purpose. ECANNOTQUITEPARSE. The user may say git bisect bad $that and we do not question $that is bad. Git does not know better than the user. But that does not mean Git does not know better than the user how the current bad commit and $that commit are related. The user is not interested in replacing at all. The user is telling just one single fact, that is, $that is bad. The user may make mistakes and try to fix them, like for example: $ git checkout master $ git bisect bad $ git log --oneline --decorate --graph --all # Ooops I was not on the right branch $ git checkout dev $ git bisect bad $ git log --oneline --decorate --graph --all # Everything looks ok now; the bad commit is what I expect # I can properly continue bisecting using git bisect good... In this case the user, who knows how git bisect works, expected that the second git bisect bad would fix the previous mistake made using the first git bisect bad. If we make git bisect bad behave in another way we may break an advanced user's expectation. I am not quite sure if I am correctly getting what you meant to say, but if you meant only when --alternate is given, we should do the merge-base thing; we should keep losing the current 'bad' and replace it with the new one without the --alternate option, I would see that as an exercise of a bad taste. What I wanted to say is that if we change git bisect bad commitish, so that now it means add a new bad commit instead of the previous replace the current bad commit, if any, with this one, then experienced users might see that change as a regression in the user interface and it might even break scripts. Huh? Step back a bit. The place you need to start from is to admit the fact that what git bisect bad committish currently does is broken. Try creating this history yourself a---b---c---d---e---f and start bisection this way: $ git bisect start f c $ git bisect bad a Immediately after the second command, git bisect moans Some good revs are not ancestor of the bad rev. git bisect cannot work properly in this case. Maybe you mistake good and bad revs? when it notices that the good rev (i.e. 'c') is no longer an ancestor of the 'bad', which now points at 'a'. But that is because git bisect bad _blindly_ moved 'bad' that used to point at 'f' to 'a', making a good rev (i.e. 'c') an ancestor of the bad rev, without even bothering to check. Yeah, git bisect bad currently does what it is asked and then complains when it looks like a mistake has been made. You might see that as a bug. I am seeing that more as git bisect expecting users to know what they are doing. For example an advanced user might have realized that the first git bisect start f c was completely rubish for some reason, and the git bisect bad a might be a first step to fix that. (The next step might then be deleting the good pointer...) Now, if we fixed this bug and made the bisect_state function more careful (namely, when accepting bad, make sure it is not beyond any existing good, or barf like the above, _without_ moving the bad pointer), the user interface and behaviour would be changed. Is that a regression? No, it is a usability fix and a progress. Yeah, you might see that as a usability fix and a progress. Simply put, bisect_state function can become more careful and intelligent to help users. Yeah, we can try to help users more, but doing that we might annoy some advanced users, who are used to the current way git bisect works, and perhaps break some scripts. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: Seems to be pushing more than necessary
We have a fairly large repo (~2.4GB), mainly due to binary resources (for an ios app). I know this can generally be a problem, but I have a specific question. If I cut a branch, and edit a few (non-binary) files, and push, what should be uploaded? I assumed it was just the diff (I know whole compressed files are used, I mean the differences between my branch and where I cut it from). Is that correct? Because when I push, it grinds to a halt at the 20% mark, and feels like it's trying to push the entire repo. If I run git diff --stat --cached origin/foo I see the files I would expect (i.e. just those that have changed). If I run git format-patch origin/foo..foo the patch files total 1.7MB, which should upload in just a few seconds, but I've had pushes take over an hour. I'm using git 2.2.2 on Mac OS X (Mavericks), and ssh (g...@github.com). Am I doing it wrong? Is this the expected behaviour? If not, is there anything I can do to debug it? Any help gratefully received. Thanks, Graham -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] nd/multiple-work-trees updates
Without having looked into this and nd/multiple-work-trees, but with make multiple checkouts aware of each other in mind: Could this mechanism be re-used to make alternates aware of each other, to mitigate the dangers of having git gc on an alternate remove objects that are used by a referencing repository? Thanks - Eph On 03.01.2015 10:41, Nguyễn Thái Ngọc Duy wrote: These patches are on top of what's in 'pu'. They add --ignore-other-worktrees and make a note about current submodule support status. I don't think submodule support is ready yet even with Max Kirillov's series [1]. His 03/03 is already fixed in 'pu' though, so only 01/03 and 02/03 are new. [1] http://thread.gmane.org/gmane.comp.version-control.git/261107 Nguyễn Thái Ngọc Duy (3): checkout: pass whole struct to parse_branchname_arg instead of individual flags checkout: add --ignore-other-wortrees git-checkout.txt: a note about multiple checkout support for submodules Documentation/git-checkout.txt | 9 + builtin/checkout.c | 19 +++ t/t2025-checkout-to.sh | 7 +++ 3 files changed, 27 insertions(+), 8 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC/GSOC] make git-pull a builtin
Hi Paul,
thank you for this very detailed mail. It was a real pleasure to read this
well-researched document.
In the following, I will pick out only parts from the mail, in the interest of
both of our time. Please assume that I agree with everything that I do not
quote below (and even the with quoted parts I agree mostly ;-)).
On 2015-03-17 14:57, Paul Tan wrote:
on Windows the runtime fell from 8m 25s to 1m 3s.
This is *exactly* the type of benefit that makes this project so important!
Nice one.
A simpler, but less perfect strategy might be to just convert the shell
scripts directly statement by statement to C, using the run_command*()
functions as Duy Nguyen[2] suggested, before changing the code to use
the internal API.
Yeah, the idea is to have a straight-forward strategy to convert the scripts in
as efficient manner as possible. It also makes reviewing easier if the first
step is an almost one-to-one translation to `run_command*()`-based builtins.
Plus, it is rewarding to have concise steps that can be completed in a timely
manner.
+/* NOTE: git-pull.sh only supports --log and --no-log, as opposed to what
+ * man git-pull says. */
+static int opt_shortlog_len;
This comment might want to live in the commit message instead... It is prone to
get stale in the code while it will always be correct (and easier to spot) in
the commit message.
+/**
+ * Returns default rebase option value
+ */
+static int rebase_config_default(void)
+{
+ struct strbuf name = STRBUF_INIT;
+ const char *value = NULL;
+ int boolval;
+
+ strbuf_addf(name, branch.%s.rebase, head_name_short);
+ if (git_config_get_value(name.buf, value))
+ git_config_get_value(pull.rebase, value);
+ strbuf_release(name);
+ if (!value)
+ return REBASE_FALSE;
+ boolval = git_config_maybe_bool(pull.rebase, value);
+ if (boolval = 0)
+ return boolval ? REBASE_TRUE : REBASE_FALSE;
+ else if (value !strcmp(value, preserve))
+ return REBASE_PRESERVE;
+ die(_(invalid value for branch.%s.rebase \%s\), head_name_short,
value);
+}
Personally, I would use a couple of empty lines for enhanced structure.
Conceptually, there are four parts: the variable declarations, querying the
config, parsing the value, and `die()`ing in case of error. There is already an
empty line between the first two parts, and I would imagine that readability
would be improved further by having an empty line after the `strbuf_release()`
and the `return REBASE_PRESERVE` statement, respectively.
+static int parse_opt_rebase(const struct option *opt, const char
*arg, int unset)
+{
+ if (arg)
+ *(int *)opt-value = parse_rebase(arg);
+ else
+ *(int *)opt-value = unset ? REBASE_FALSE : REBASE_TRUE;
+ return (*(int *)opt-value) = 0 ? 0 : -1;
+}
In this function (and also in other places below), there is this pattern that a
`struct option` pointer is passed to the function, but then only `*(int
*)opt-value` is written to. Therefore, I would suggest to change the signature
of the function and pass `(int *)opt-value` as function parameter.
+static int has_unstaged_changes(void)
Yeah, this function, as well as the ones below it, look as if they are so
common that they *should* be already somewhere in libgit.a. But I did not find
them, either...
Of course it *would* be nice to identify places where essentially the same code
is needed, and refactor accordingly. But I think that is outside the scope of
this project.
The rest looks pretty good (and you realize that my comments above are really
more nit picks than anything else).
The FIXMEs should be relatively easy to address. It would be my pleasure to
work with you.
Ciao,
Johannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git pull git gc
Hello, I have a local folder with the git-repository (so that its .git/config contains ([remote origin]\n url = git://github.com/git/git.git\nfetch = +refs/heads/*:refs/remotes/origin/* ) I do there git pull. Usually the output is Already up to date but since today it prints Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. Already up-to-date. and starts in the background a git gc --auto process. This is all fine, however, when the git gc process finishes, and I do again git pull I get the same message, as above (git gc is again started). My understanding is, that git gc has to be occasionally run and then the garbage collection is done for a while. In the concrete case, if git pull starts git gc in the background and prints a message on this, it is all fine, but running git pull after a while, when the garbage collection was recently done, where shall be neither message nor an action about git gc. My system-wide gitconfig contains [pack] threads = 1. I have tar xJf'ed my local git repository and have put it under http://mail.aegee.org/dpa/v/git-repository.tar.xz The question is: Why does git pull every time when it is invoked today print information about git gc? I have git 2.3.3 adjusted with ./configure --with-openssl --with-libpcre --with-curl --with-expat. Thanks in advance for your answer Dilian -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 6:26 PM, Graham Hay grahamr...@gmail.com wrote: It would help if you pasted the push output. For example, does it stop at 20% at the compressing objects line or writing objects. How many total objects does it say? It rattles through compressing objects, and the first 20% of writing objects, then slows to a crawl. Writing objects: 33% (3647/10804), 80.00 MiB | 112.00 KiB/s This 10804 looks wrong (i.e. sending that many compressed objects). Also 80 MiB sent at that point. If you modify just a couple files, something is really wrong because the number of new objects may be hundreds at most, not thousands. v2.2.2 supports git fast-export --anonymize [1] to create an anonymized clone of your repo that you can share, which might help us understand the problem. There's also the environment variable GIT_TRACE_PACKET that can help see what's going on at the protocol level, but I think you're on your own because without access to this repo, SHA-1s from that trace may not make much sense. [1] https://github.com/git/git/commit/a8722750985a53cc502a66ae3d68a9e42c7fdb98 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 7:26 PM, Duy Nguyen pclo...@gmail.com wrote: It's quite a lot of work :) I created this script named git and put it in $PATH to capture input for pack-objects. You'll need to update /path/to/real/git to point to the real binary then you'll get /tmp/stdin Forgot one important sentence: You need to push again using this fake git program to save data in /tmp/stdin. Also you can stop the push when it goes to compressing objects phase. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
Are there any commands that I can use to show exactly what it is trying to push? I'll see if I can create a (public) repo that has the same problem. Thanks for your help. This 10804 looks wrong (i.e. sending that many compressed objects). Also 80 MiB sent at that point. If you modify just a couple files, something is really wrong because the number of new objects may be hundreds at most, not thousands. v2.2.2 supports git fast-export --anonymize [1] to create an anonymized clone of your repo that you can share, which might help us understand the problem. There's also the environment variable GIT_TRACE_PACKET that can help see what's going on at the protocol level, but I think you're on your own because without access to this repo, SHA-1s from that trace may not make much sense. [1] https://github.com/git/git/commit/a8722750985a53cc502a66ae3d68a9e42c7fdb98 -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH, RFC] checkout: Attempt to checkout submodules
If a user does git checkout HEAD -- path/to/submodule they'd expect the
submodule to be checked out to the commit that submodule is at in HEAD.
This is the most brute force possible way of try to do that, and so its
probably broken in some cases. However I'm not terribly familiar with
git's internals and I'm not sure if this is even wanted so I'm starting
simple. If people want this to work I can try and do something better.
Signed-off-by: Trevor Saunders tbsau...@tbsaunde.org
---
entry.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/entry.c b/entry.c
index 1eda8e9..2dbf5b9 100644
--- a/entry.c
+++ b/entry.c
@@ -1,6 +1,8 @@
#include cache.h
+#include argv-array.h
#include blob.h
#include dir.h
+#include run-command.h
#include streaming.h
static void create_directories(const char *path, int path_len,
@@ -277,9 +279,25 @@ int checkout_entry(struct cache_entry *ce,
* just do the right thing)
*/
if (S_ISDIR(st.st_mode)) {
- /* If it is a gitlink, leave it alone! */
- if (S_ISGITLINK(ce-ce_mode))
+ if (S_ISGITLINK(ce-ce_mode)) {
+ struct argv_array args = ARGV_ARRAY_INIT;
+ char sha1[41];
+
+ argv_array_push(args, checkout);
+
+ if (state-force)
+ argv_array_push(args, -f);
+
+ memcpy(sha1, sha1_to_hex(ce-sha1), 41);
+ argv_array_push(args, sha1);
+
+ run_command_v_opt_cd_env(args.argv,
+RUN_GIT_CMD, ce-name,
+NULL);
+ argv_array_clear(args);
+
return 0;
+ }
if (!state-force)
return error(%s is a directory, path.buf);
remove_subtree(path);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 7:03 PM, Graham Hay grahamr...@gmail.com wrote: Are there any commands that I can use to show exactly what it is trying to push? It's a bit more than a command. If you push when GIT_TRACE is set to 2, you'll see it executes git pack-objects command with all its arguments. This command expects some input from stdin. If you can capture that, you can run it by yourself to create the exact pack that is transferred over network. Run that pack through git index-pack --verify-stat will show you SHA-1 of all sent objects. It's quite a lot of work :) I created this script named git and put it in $PATH to capture input for pack-objects. You'll need to update /path/to/real/git to point to the real binary then you'll get /tmp/stdin -- 8 -- #!/bin/sh if [ $1 = pack-objects ]; then exec tee /tmp/stdin | /path/to/real/git $@ else exec /path/to/real/git $@ fi -- 8 -- The remaining steps may be this (may need tweaking) git pack-objects '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' /tmp/stdin | git index-pack --fix-thin --stdin pack708538afeda8eb331858680e227f7713228ce782 -- new pack git verify-pack --verbose .git/objects/pack/pack-708538afeda8eb331858680e227f7713228ce782.pack d75631bd83ebdf03d4b0d925ff6734380f801fc6 commit 567 377 12 dd44100a7cdad113b23d31876e469b74fbe21e1b tree 15069 10492 389 8f4bbccea759d7a47616e29bd55b3f205b3615c2 tree 3869 2831 10881 3db0460935bc843a2a70a0e087222eec61a0ff0d blob 12379 3529 13712 Here we can see this push of mine sends four objects, 1 commit, 2 trees and 1 blob. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
On Wed, Mar 18, 2015 at 12:57 AM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Mon, Mar 16, 2015 at 09:05:45AM -0700, Junio C Hamano wrote: The offending one came from eec3e7e4 (cache-tree: invalidate i-t-a paths after generating trees, 2012-12-16), which was a fix to an earlier bug where a cache-tree written out of an index with i-t-a entries had incorrect information and still claimed it is fully valid after write-tree rebuilt it. The test probably should add another path without i-t-a bit, run the same diff --cached with updated expectation before write-tre, and run the diff --cached again to make sure it produces a result that match the updated expectation. Would adding another non-i-t-a entry help? Before this patch diff --cached after write-tree shows the i-t-a entry only when eec3e7e4 is applied. But with this patch we don't show i-t-a entry any more, before or after write-tree, eec3e7e4 makes no visible difference. We could even revert eec3e7e4 and the outcome of diff --cached would be the same because we just sort of move the invalidation part from cache-tree to do_oneway_diff(). Not invalidating would speed up diff --cached when i-t-a entries are present. Still it may be a good idea to invalidate i-t-a paths to be on the safe side. Perhaps a patch like this to resurrect the test? My unerstanding of what eec3e7e4 (cache-tree: invalidate i-t-a paths after generating trees, 2012-12-16) fixed was that in this sequence: ... So reverting the fix obviously is not the right thing to do. If the tests show different results from two invocations of diff --cached with your patch applied, there is something that is broken by your patch, because the index and the HEAD does not change across write-tree in that test. Right. I missed this but I think this is a less important test because I added a new test to make sure diff --cached (git status to be exact) outputs the right thing when i-t-a entries are present. If on the other hand the tests show the same result from these two diff --cached and the result is different from what the test expects, that means your patch changed the world order, i.e. an i-t-a entry used to be treated as if it were adding an empty blob to the index but it is now treated as non-existent, then that is a good thing and the only thing we need to update is what the test expects. I am guessing that instead of expecting dir/bar to be shown, it now should expect no output? Yes, no output. Does adding an non-i-t-a entry help? It does not hurt, and it makes the test uses a non-empty output, making its effect more visible, which may or may not count as helping. But still, if I revert the change in cache-tree.c and force write-tree to produce valid cache-tree when i-t-a entries are present, this test still passes incorrectly. This is why I changed the test. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
On Wed, Mar 18, 2015 at 8:16 PM, Graham Hay grahamr...@gmail.com wrote: I created a repo with over 1GB of images, but it works as expected (only pushed 3 objects). Sorry, I must have done something wrong. I put that script in ~/Applications, and checked it worked. Then I ran this: $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin git-wtf I think I encountered the same problem. Inserting --exec-path=$HOME/Applications between git and push was probably what made it work for me. Haven't investigated the reason yet. We really should have an easier way to get this info without jumping through hoops like this. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug in git-verify-pack or in its documentation?
On Mon, Mar 16, 2015 at 8:18 PM, Mike Hommey m...@glandium.org wrote: On Mon, Mar 16, 2015 at 05:13:25PM +0700, Duy Nguyen wrote: On Mon, Mar 16, 2015 at 3:05 PM, Mike Hommey m...@glandium.org wrote: Hi, git-verify-pack's man page says the following about --stat-only: Do not verify the pack contents; only show the histogram of delta chain length. With --verbose, list of objects is also shown. However, there is no difference of output between --verbose and --verbose --stat-only (and in fact, looking at the code, only one of them has an effect, --stat-only having precedence). There is. very-pack --verbose would show a lot of sha-1 type random numbers lines before the histogram while --stat-only (with or without --verbose) would only show the histogram. Err, I meant between --stat-only and --verbose --stat-only. Yeah --verbose is always ignored when --stat-only is set. This command is funny. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] git gc with alternates tries accessing non-existing directory
On 18.03.2015 10:42, Jeff King wrote: On Wed, Mar 18, 2015 at 09:11:48AM +0100, Ephrim Khong wrote: I have a non-bare repository /home/a set up with an alternate to the bare repository /b. Running git gc on /home/a produces below's error [...] git --version git version 2.3.0 Try v2.3.2. It has b0a4264 (sha1_file: fix iterating loose alternate objects, 2015-02-08), which is almost certainly the problem. Indeed, the message is gone in v2.3.3. Thanks! - Eph -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
I created a repo with over 1GB of images, but it works as expected (only pushed 3 objects). Sorry, I must have done something wrong. I put that script in ~/Applications, and checked it worked. Then I ran this: $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin git-wtf 12:48:28.839026 git.c:349 trace: built-in: git 'push' '--set-upstream' 'origin' 'git-wtf' 12:48:28.907605 run-command.c:351 trace: run_command: 'ssh' 'g...@github.com' 'git-receive-pack '\''grahamrhay/bornlucky-ios.git'\''' 12:48:30.137410 run-command.c:351 trace: run_command: 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' 12:48:30.138246 exec_cmd.c:130 trace: exec: 'git' 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' 12:48:30.144783 git.c:349 trace: built-in: git 'pack-objects' '--all-progress-implied' '--revs' '--stdout' '--thin' '--delta-base-offset' '--progress' Counting objects: 10837, done. Delta compression using up to 4 threads. Compressing objects: 100% (9301/9301), done. Writing objects: 21% (2276/10837) but there was nothing in /tmp/stdin. Have I missed a step? I tried changing the tee to point to ~ in case it was permissions related. I fear this is some Mac nonsense. I added an echo in the script, but it only gets called for the first git incantation. On 18 March 2015 at 12:34, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 18, 2015 at 7:26 PM, Duy Nguyen pclo...@gmail.com wrote: It's quite a lot of work :) I created this script named git and put it in $PATH to capture input for pack-objects. You'll need to update /path/to/real/git to point to the real binary then you'll get /tmp/stdin Forgot one important sentence: You need to push again using this fake git program to save data in /tmp/stdin. Also you can stop the push when it goes to compressing objects phase. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
Hello, # git gc --auto Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. and calls in the background: 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire 2.weeks.ago # git count-objects -v count: 6039 size: 65464 in-pack: 185432 packs: 1 size-pack: 46687 prune-packable: 0 garbage: 0 size-garbage: 0 Regards Dilian On 18.03.2015 15:16, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 8:53 PM, Дилян Палаузов dilyan.palau...@aegee.org wrote: Hello, I have a local folder with the git-repository (so that its .git/config contains ([remote origin]\nurl = git://github.com/git/git.git\nfetch = +refs/heads/*:refs/remotes/origin/* ) I do there git pull. Usually the output is Already up to date but since today it prints Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. Already up-to-date. and starts in the background a git gc --auto process. This is all fine, however, when the git gc process finishes, and I do again git pull I get the same message, as above (git gc is again started). So if you do git gc --auto now, does it exit immediately or go through the garbage collection process again (it'll print something)? What does git count-objects -v show? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen pclo...@gmail.com wrote: If not, I made some mistake in analyzing this and we'll start again. I did make one mistake, the first gc should have reduced the number of loose objects to zero. Why didn't it.? I'll come back to this tomorrow if nobody finds out first :) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
Hello Duy, #ls .git/objects/17/* | wc -l 30 30 * 256 = 7 680 6 700 And now? Do I have to run git gc --aggressive ? Kind regards Dilian On 18.03.2015 15:33, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 9:23 PM, Дилян Палаузов dilyan.palau...@aegee.org wrote: Hello, # git gc --auto Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. and calls in the background: 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire 2.weeks.ago # git count-objects -v count: 6039 loose number threshold is 6700, unless you tweaked something. But there's a tweak, we'll come back to this. size: 65464 in-pack: 185432 packs: 1 Pack threshold is 50, You only have one pack, good OK back to the count 6039 above. You have that many loose objects. But 'git gc' is lazier than 'git count-objects'. It assume a flat distribution, and only counts the number of objects in .git/objects/17 directory only, then extrapolate for the total number. So can you see how many files you have in this directory .git/objects/17? That number, multiplied by 256, should be greater than 6700. If that's the case git gc laziness is the problem. If not, I made some mistake in analyzing this and we'll start again. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 8:53 PM, Дилян Палаузов dilyan.palau...@aegee.org wrote: Hello, I have a local folder with the git-repository (so that its .git/config contains ([remote origin]\nurl = git://github.com/git/git.git\nfetch = +refs/heads/*:refs/remotes/origin/* ) I do there git pull. Usually the output is Already up to date but since today it prints Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. Already up-to-date. and starts in the background a git gc --auto process. This is all fine, however, when the git gc process finishes, and I do again git pull I get the same message, as above (git gc is again started). So if you do git gc --auto now, does it exit immediately or go through the garbage collection process again (it'll print something)? What does git count-objects -v show? -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 9:23 PM, Дилян Палаузов dilyan.palau...@aegee.org wrote: Hello, # git gc --auto Auto packing the repository in background for optimum performance. See git help gc for manual housekeeping. and calls in the background: 25618 1 0 32451 884 1 14:20 ?00:00:00 git gc --auto 25639 25618 51 49076 49428 0 14:20 ?00:00:07 git prune --expire 2.weeks.ago # git count-objects -v count: 6039 loose number threshold is 6700, unless you tweaked something. But there's a tweak, we'll come back to this. size: 65464 in-pack: 185432 packs: 1 Pack threshold is 50, You only have one pack, good OK back to the count 6039 above. You have that many loose objects. But 'git gc' is lazier than 'git count-objects'. It assume a flat distribution, and only counts the number of objects in .git/objects/17 directory only, then extrapolate for the total number. So can you see how many files you have in this directory .git/objects/17? That number, multiplied by 256, should be greater than 6700. If that's the case git gc laziness is the problem. If not, I made some mistake in analyzing this and we'll start again. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen pclo...@gmail.com wrote: If not, I made some mistake in analyzing this and we'll start again. I did make one mistake, the first gc should have reduced the number of loose objects to zero. Why didn't it.? I'll come back to this tomorrow if nobody finds out first :) Most likely they are not referenced by anything but are younger than 2 weeks. I saw a similar issue with automatic gc triggering after every operation when I did something equivalent to: git add lots of files git commit git reset --hard HEAD^ which creates a log of unreachable objects which are not old enough to be pruned. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Seems to be pushing more than necessary
Got there eventually! $ git verify-pack --verbose bar.pack e13e21a1f49704ed35ddc3b15b6111a5f9b34702 commit 220 152 12 03691863451ef9db6c69493da1fa556f9338a01d commit 334 227 164 ... snip ... chain length = 50: 2 objects bar.pack: ok Now what do I do with it :) On 18 March 2015 at 13:33, Duy Nguyen pclo...@gmail.com wrote: On Wed, Mar 18, 2015 at 8:16 PM, Graham Hay grahamr...@gmail.com wrote: I created a repo with over 1GB of images, but it works as expected (only pushed 3 objects). Sorry, I must have done something wrong. I put that script in ~/Applications, and checked it worked. Then I ran this: $ GIT_TRACE=2 PATH=~/Applications:$PATH git push --set-upstream origin git-wtf I think I encountered the same problem. Inserting --exec-path=$HOME/Applications between git and push was probably what made it work for me. Haven't investigated the reason yet. We really should have an easier way to get this info without jumping through hoops like this. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/4] docs/git-credential-store: document XDG file and precedence
I would personnally prefer to see this squashed with PATCH 2/4: pushing the bisectable history principle a bit, the state between patches 2 and 3 could be considered broken because the code does not do what the documentation says. And as a reviewer, I like having pieces of docs linked to the patch they document. Paul Tan pyoka...@gmail.com writes: +Credential storage will per default Not a native, but per default sounds weird and by default seems far more common. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
Similarly to the merge doc and code, I personally prefer seeing code and tests in the same patch. Actually, my preference goes for a first patch that introduces the tests with test_expect_failure for things that are not yet implemented (and you can check that tests do not pass yet before you code), and then the patch introducing the feature doing -test_expect_failure +test_expect_success which documents quite clearly and concisely that you just made the feature work. Paul Tan pyoka...@gmail.com writes: +test_expect_success 'if custom XDG_CONFIG_HOME credentials file + ~/xdg/git/credentials exists, it will only be written to; + ~/.config/git/credentials and ~/.git-credentials will not be + created' ' + test_when_finished rm -f $HOME/xdg/git/credentials + test -s $HOME/xdg/git/credentials + test_path_is_missing $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' Broken -chain. That is a real issue that must be fixed. +test_expect_success 'get: return credentials from home file if xdg files are unreadable' ' files are - file is, no? +' + + +test_expect_success 'erase: erase matching credentials from both xdg and home files' ' Double blank line. On overall, except the broken -chain above, the whole series looks good. Feel free to ignore my other nitpicks if you prefer. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] git-credential-store: support multiple credential files
Paul Tan pyoka...@gmail.com writes: + /* Write credential to the filename specified by fns-items[0], thus + * creating it */ Just to show I'm following: misformatted multi-line comment. Other than that, good job! -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3 PATCH 2/2] reset: add tests for git reset -
Sundararajan R dyou...@gmail.com writes:
Subject: [v3 PATCH 2/2] reset: add tests for git reset -
This should be [PATCH v3 2/2].
git send-email -v2 can do this for you.
Sundararajan R dyou...@gmail.com writes:
+test_expect_success 'reset - with no @{-1} branch and file named - should
succeed' '
+ test_when_finished rm -rf new
+ expected
+ git init new
+ (
+ cd new
+ echo Hello -
+ git add -
+ git reset - ../actual
+ )
+ test_cmp expected actual
+'
test_must_be_empty actual would be easier to read than expected ...
test_cmp expected IMHO.
+test_expect_success 'reset - with @{-1} branch and no file named - should
succeed' '
+ test_when_finished rm -rf new
+ git init new
+ (
+ cd new
+ echo Hey new_file
+ git add new_file
+ git commit -m first_commit
+ git checkout -b new_branch
+ new_file
+ git add new_file
+ git reset -
+ git status -uno actual
+ git add new_file
+ git reset @{-1}
+ git status -uno expected
+ test_cmp actual expected
+ )
+'
Better use git status --porcelain here as its format is meant to be
stable and unambiguous. The non-porcelain should work two because you're
comparing the output on two identical states, but who knows.
With or without my suggested change, the series looks good to me.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Submodule not working with specified work-tree
It seems like submodule isn't picking up on the work tree that I'm specifying. In the scenario that I'm working with, I'd prefer to not have to cd into a directory to update the submodules. All the other git subcommands that I'm executing work fine with specifying the git dir and work tree via envvars or parameters. I'm not sure if this is a bug or not. $ git --git-dir $HOME/testrepo/.git --work-tree $HOME/testrepo submodule update --init --recursive fatal: /usr/local/Cellar/git/2.3.0/libexec/git-core/git-submodule cannot be used without a working tree. $ GIT_WORK_TREE=$HOME/testrepo GIT_DIR=$HOME/testrepo/.git git submodule update --init --recursive fatal: /usr/local/Cellar/git/2.3.0/libexec/git-core/git-submodule cannot be used without a working tree. $ git version git version 2.3.0 $ uname -a Darwin hanazawa.local 14.1.0 Darwin Kernel Version 14.1.0: Mon Dec 22 23:10:38 PST 2014; root:xnu-2782.10.72~2/RELEASE_X86_64 x86_64 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC/GSOC] make git-pull a builtin
Hi,
First of all, thanks a lot for working on this. I'm rather impressed to
see a working proof of concept so soon! And impressed by the quality for
a first draft.
A few minor remaks below after a very quick look.
Paul Tan pyoka...@gmail.com writes:
Ideally, I think the solution is to
improve the test suite and make it as comprehensive as possible, but
writing a comprehensive test suite may be too time consuming.
time-consuming, but also very beneficial since the code would end up
being better tested. For sure, a rewrite is a good way to break stuff,
but anything untested can also be broken by mistake rather easily at any
time.
I'd suggest doing a bit of manual mutation testing: take your C code,
comment-out a few lines of code, see if the tests still pass, and if
they do, add a failing test that passes again once you uncomment the
code.
diff --git a/Makefile b/Makefile
index 44f1dd1..50a6a16 100644
--- a/Makefile
+++ b/Makefile
@@ -470,7 +470,6 @@ SCRIPT_SH += git-merge-octopus.sh
SCRIPT_SH += git-merge-one-file.sh
SCRIPT_SH += git-merge-resolve.sh
SCRIPT_SH += git-mergetool.sh
-SCRIPT_SH += git-pull.sh
When converting a script into a builtin, we usually move the old script
to contrib/examples/.
+static const char * const builtin_pull_usage[] = {
+ N_(git pull [-n | --no-stat] [--[no-]commit] [--[no-]squash]
[--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s strategy]...
[fetch-options] repo head...),
I know we have many instances of very long lines for usage string, but
it would be better IMHO to wrap it both in the code and in the output of
git pull -h.
+/* NOTE: git-pull.sh only supports --log and --no-log, as opposed to what
+ * man git-pull says. */
We usually write multi-line comments
/*
* like
* this
*/
+/* Global vars since they are used often */
Being use often does not count as an excuse for being global IMHO.
Having global variables means you share the same instance in several
functions, and you have to be careful with things like
void g()
{
glob = bar;
}
void f()
{
glob = foo;
g();
bar = glob;
}
As a reader, I'd rather not have to be careful about this to keep my
neurons free for other things.
+static char *head_name;
Actually, this one is used only in one function, so often is not even
true ;-).
+static struct option builtin_pull_options[] = {
You may also declare this as local in cmd_pull().
+/**
+ * Returns remote for branch
Here and elsewhere: use imperative (return, not return_s_). The comment
asks the function to return a value.
+ strbuf_addf(key, branch.%s.remote, branch);
+ git_config_get_value(key.buf, remote);
+ strbuf_release(key);
This config API is beautiful :-).
(Before last year's GSoC, you'd have needed ~10 lines of code to do the
same thing)
+ return error(Ambiguous refname: '%s', ref);
Here and elsewhere: don't forget to mark strings for translation.
+/**
+ * Appends FETCH_HEAD's merge heads into arr. Returns number of merge heads,
+ * or -1 on error.
+ */
+static int sha1_array_append_fetch_merge_heads(struct sha1_array *arr)
+{
+ int num_heads = 0;
+ char *filename = git_path(FETCH_HEAD);
+ FILE *fp = fopen(filename, r);
I guess this is one instance where we could avoid writting (fetch) and
then parsing (here) if we had a better internal API.
But that can come after, of course.
+}
+
+
+static void argv_array_push_merge_args(struct argv_array *arr,
Bikeshed: you sometimes have two blank lines between functions,
sometimes one. Not sure it's intended.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository
Am 17.03.2015 um 19:55 schrieb Jeff King: + echo $bogus .git/refs/heads/bogus..name ... I assumed the final . in your example wasn't significant (it is not to git), but let me know if I've run afoul of another weird restriction. :) It was actually deliberate (with intents too complicated to explain), but it turns out not to be required. Your updated test case is good. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 1:33 PM, Alessandro Zanardi pensierinmus...@gmail.com wrote: Here are other sources describing the issue: http://stackoverflow.com/questions/21109672/git-ignoring-icon-files-because-of-icon-rule http://blog.bitfluent.com/post/173740409/ignoring-icon-in-gitignore Sorry to bother the Git development team with such a minor issue, I just wanted to know if it's already been fixed. I do not ship your ~/.gitignore_global file as part of my software, so the problem is not mine to fix in the first place ;-) Where did you get that file from? We need to find whoever is responsible and notify them so that these users who are having the issue will be helped. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan pyoka...@gmail.com wrote: t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: --- The previous version can be found at [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 * Merge related, but previously separate, tests together in order to make the test suite easier to understand. * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set it, and unset it immediately before and after helper_test store is called in order to make it localized to only the command that it should affect. * Add test, previously missing, to check that only the home credentials file is written to if both the xdg and home files exist. * Correct mislabelling of home-user/home-pass to the proper xdg-user/xdg-pass. * Use rm -f instead of test_might_fail rm. This round looks much better. Thanks. Most of the comments below are just nit-picky, with one or two genuine (minor) issues. Thanks Eric for the code review. diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index f61b40c..5b0a666 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,115 @@ test_description='credential-store tests' helper_test store +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' ' These test descriptions are quite long, often mirroring in prose what the test body already says more concisely. I generally try to keep descriptions short while still being descriptive enough to give a general idea about what is being tested. I've rewritten a few test descriptions (below) to be very short, in fact probably too terse; but perhaps better descriptions would lie somewhere between the two extremes. First example, for this test: !xdg: .git-credentials !xdg Key: Space-separated items. Items before : are the pre-conditions, and items after are the post-state. ! file does not exist; output goes here. + test -s $HOME/.git-credentials + test_path_is_missing $HOME/.config/git/credentials +' + +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' It's customary to call this setup rather than create. Terse version: setup: -.git-redentials +xdg Key: - file removed; + file created. + rm -f $HOME/.git-credentials + mkdir -p $HOME/.config/git + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'when xdg credentials file exists, only write to xdg file; do not create ~/.git-credentials' ' Terse version: !.git-credentials xdg: !.git-credentials xdg + test -s $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'set up custom XDG_CONFIG_HOME credential file at ~/xdg/git/credentials' ' s/set up/setup/ Terse: setup custom-xdg + mkdir -p $HOME/xdg/git + rm -f $HOME/.config/git/credentials + $HOME/xdg/git/credentials It would be easier to read this if you placed the two lines together which refer to the custom xdg file. Also, for completeness and to be self-contained, don't you want to remove ~/.git-credentials? rm -f $HOME/.git-credentials rm -f $HOME/.config/git/credentials mkdir -p $HOME/xdg/git $HOME/xdg/git/credentials +' + +XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store +unset XDG_CONFIG_HOME It's hard to spot the helper_test store at the end of line. I'd place it on a line by itself so that it is easy to see that it is wrapped by the setting and unsetting of the environment variable. +test_expect_success 'if custom XDG_CONFIG_HOME credentials file ~/xdg/git/credentials exists, it will only be written to; ~/.config/git/credentials and ~/.git-credentials will not be created' ' Terse: !.git-credentials !xdg custom-xdg: !.git-credentials !xdg custom-xdg + test_when_finished rm -f $HOME/xdg/git/credentials + test -s $HOME/xdg/git/credentials + test_path_is_missing $HOME/.config/git/credentials Matthieu already pointed out the broken -chain. + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'get: return credentials from home file if matches are found in both home and xdg files' ' Terse: .git-credentials xdg: .git-credentials Key: taken from here. + mkdir -p $HOME/.config/git + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + echo https://home-user:home-p...@example.com; $HOME/.git-credentials + check fill store -\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com + username=home-user + password=home-pass + -- + EOF +' + +test_expect_success 'get: return credentials from xdg file
[PATCH v4 1/4] git-credential-store: support multiple credential files
Previously, git-credential-store only supported storing credentials in a
single file: ~/.git-credentials. In order to support the XDG base
directory specification[1], git-credential-store needs to be able to
lookup and erase credentials from multiple files, as well as to pick the
appropriate file to write to so that the credentials can be found on
subsequent lookups.
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
Note that some credential storage files may not be owned, readable or
writable by the user, as they may be system-wide files that are meant to
apply to every user.
Instead of a single file path, lookup_credential(), remove_credential()
and store_credential() now take a precedence-ordered string_list of
file paths. lookup_credential() expects both user-specific and
system-wide credential files to be provided to support the use case of
system administrators setting default credentials for users.
remove_credential() and store_credential() expect only the user-specific
credential files to be provided as usually the only config files that
users are allowed to edit are their own user-specific ones.
lookup_credential() will read these (user-specific and system-wide) file
paths in order until it finds the 1st matching credential and print it.
As some files may be private and thus unreadable, any file which cannot
be read will be ignored silently.
remove_credential() will erase credentials from all (user-specific)
files in the list. This is because if credentials are only erased from
the file with the highest precedence, a matching credential may still be
found in a file further down the list. (Note that due to the lockfile
code, this requires the directory to be writable, which should be so for
user-specific config files)
store_credential() will write the credentials to the first existing
(user-specific) file in the list. If none of the files in the list
exist, store_credential() will write to the filename specified by the
first item of the filename list. For backwards compatibility, this
filename should be ~/.git-credentials.
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Paul Tan pyoka...@gmail.com
---
The previous version can be found at [1].
[1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265309
The changes are as follows:
* Remove default_fn argument from store_credential(). It now uses the
first item of the filenames string_list. Thanks Jeff, Matthieu and
Junio for your input in this discussion.
* Change filename string_list fns to duplicate strings by default
(STRING_LIST_INIT_DUP). This is to make memory ownership explicit --
when the string_list receives the file paths from xdg_config_home()
and expand_user_path(), it is responsible for freeing those strings.
Thanks Jeff for pointing this out.
credential-store.c | 79 +-
1 file changed, 54 insertions(+), 25 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index 925d3f4..8dad479 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -6,7 +6,7 @@
static struct lock_file credential_lock;
-static void parse_credential_file(const char *fn,
+static int parse_credential_file(const char *fn,
struct credential *c,
void (*match_cb)(struct credential *),
void (*other_cb)(struct strbuf *))
@@ -14,18 +14,20 @@ static void parse_credential_file(const char *fn,
FILE *fh;
struct strbuf line = STRBUF_INIT;
struct credential entry = CREDENTIAL_INIT;
+ int found_credential = 0;
fh = fopen(fn, r);
if (!fh) {
- if (errno != ENOENT)
+ if (errno != ENOENT errno != EACCES)
die_errno(unable to open %s, fn);
- return;
+ return found_credential;
}
while (strbuf_getline(line, fh, '\n') != EOF) {
credential_from_url(entry, line.buf);
if (entry.username entry.password
credential_match(c, entry)) {
+ found_credential = 1;
if (match_cb) {
match_cb(entry);
break;
@@ -38,6 +40,7 @@ static void parse_credential_file(const char *fn,
credential_clear(entry);
strbuf_release(line);
fclose(fh);
+ return found_credential;
}
static void print_entry(struct credential *c)
@@ -64,21 +67,10 @@ static void rewrite_credential_file(const char *fn, struct
credential *c,
die_errno(unable to commit credential store);
}
-static void store_credential(const char *fn, struct credential *c)
+static void store_credential_file(const char *fn, struct credential *c)
{
struct strbuf buf =
[PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: * Ensure that the XDG file is strictly opt-in. It should not be created by git at all times if it does not exist. * Conversely, if the XDG file exists, ~/.git-credentials should not be created at all times. * If both the XDG file and ~/.git-credentials exists, then both files should be used for credential lookups. However, credentials should only be written to ~/.git-credentials. * Credentials must be erased from both files. * $XDG_CONFIG_HOME can be a custom directory set by the user as per the XDG base directory specification. Test that git-credential-store respects that, but defaults to ~/.config/git/credentials if it does not exist or is empty. Helped-by: Matthieu Moy matthieu@grenoble-inp.fr Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Paul Tan pyoka...@gmail.com --- The previous version can be found at [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 * Merge related, but previously separate, tests together in order to make the test suite easier to understand. * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set it, and unset it immediately before and after helper_test store is called in order to make it localized to only the command that it should affect. * Add test, previously missing, to check that only the home credentials file is written to if both the xdg and home files exist. * Correct mislabelling of home-user/home-pass to the proper xdg-user/xdg-pass. * Use rm -f instead of test_might_fail rm. Thanks Eric for the code review. t/t0302-credential-store.sh | 111 1 file changed, 111 insertions(+) diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index f61b40c..5b0a666 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,115 @@ test_description='credential-store tests' helper_test store +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' ' + test -s $HOME/.git-credentials + test_path_is_missing $HOME/.config/git/credentials +' + +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' + rm -f $HOME/.git-credentials + mkdir -p $HOME/.config/git + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'when xdg credentials file exists, only write to xdg file; do not create ~/.git-credentials' ' + test -s $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'set up custom XDG_CONFIG_HOME credential file at ~/xdg/git/credentials' ' + mkdir -p $HOME/xdg/git + rm -f $HOME/.config/git/credentials + $HOME/xdg/git/credentials +' + +XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store +unset XDG_CONFIG_HOME + +test_expect_success 'if custom XDG_CONFIG_HOME credentials file ~/xdg/git/credentials exists, it will only be written to; ~/.config/git/credentials and ~/.git-credentials will not be created' ' + test_when_finished rm -f $HOME/xdg/git/credentials + test -s $HOME/xdg/git/credentials + test_path_is_missing $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'get: return credentials from home file if matches are found in both home and xdg files' ' + mkdir -p $HOME/.config/git + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + echo https://home-user:home-p...@example.com; $HOME/.git-credentials + check fill store -\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com + username=home-user + password=home-pass + -- + EOF +' + +test_expect_success 'get: return credentials from xdg file if the home files do not have them' ' + mkdir -p $HOME/.config/git + $HOME/.git-credentials + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + check fill store -\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com + username=xdg-user + password=xdg-pass + -- + EOF +' + +test_expect_success 'get: return credentials from home file if xdg files are unreadable' ' + mkdir -p $HOME/.config/git + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + echo https://home-user:home-p...@example.com; $HOME/.git-credentials + chmod -r $HOME/.config/git/credentials + check fill store -\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com +
[PATCH v4 2/4] git-credential-store: support XDG_CONFIG_HOME
Add $XDG_CONFIG_HOME/git/credentials to the default credential search
path of git-credential-store. This allows git-credential-store to
support user-specific configuration files in accordance with the XDG
base directory specification[1].
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
~/.git-credentials has a higher precedence than
$XDG_CONFIG_HOME/git/credentials when looking up credentials. This
means that if any duplicate matching credentials are found in the xdg
file (due to ~/.git-credentials being updated by old versions of git or
outdated tools), they will not be used at all. This is to give the user
some leeway in switching to old versions of git while keeping the xdg
directory. This is consistent with the behavior of git-config.
However, the higher precedence of ~/.git-credentials means that as long
as ~/.git-credentials exist, all credentials will be written to the
~/.git-credentials file even if the user has an xdg file as having a
~/.git-credentials file indicates that the user wants to preserve
backwards-compatibility. This is also consistent with the behavior of
git-config.
Since the xdg file will not be used unless it actually exists, to
prevent the situation where some credentials are present in the xdg file
while some are present in the home file, users are recommended to not
create the xdg file if they require compatibility with old versions of
git or outdated tools. Note, though, that erase can be used to
explicitly erase matching credentials from all files.
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Paul Tan pyoka...@gmail.com
---
The previous version can be found at [1].
[1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308
There are no changes in this version.
credential-store.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index 8dad479..d805df7 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -163,11 +163,16 @@ int main(int argc, char **argv)
usage_with_options(usage, options);
op = argv[0];
- if (!file)
- file = expand_user_path(~/.git-credentials);
- if (file)
+ if (file) {
string_list_append(fns, file);
- else
+ } else {
+ if ((file = expand_user_path(~/.git-credentials)))
+ string_list_append_nodup(fns, file);
+ home_config_paths(NULL, file, credentials);
+ if (file)
+ string_list_append_nodup(fns, file);
+ }
+ if (!fns.nr)
die(unable to set up default path; use --file);
if (credential_read(c, stdin) 0)
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/4] docs/git-credential-store: document XDG file and precedence
git-credential-store now supports an additional default credential file at $XDG_CONFIG_HOME/git/credentials. However, ~/.git-credentials takes precedence over it for backwards compatibility. To make the precedence ordering explicit, add a new section FILES that lists out the credential file paths in their order of precedence, and explains how the ordering affects the lookup, storage and erase operations. Also update documentation for --store to briefly explain the operations on multiple files if the --store option is not provided. Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Paul Tan pyoka...@gmail.com --- The previous version can be found at [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 The changes are as follows: * Remove support for this file was added fairly recently statement, as it will become out-dated with time. Thanks Eric for pointing this out. Documentation/git-credential-store.txt | 35 -- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt index bc97071..e16afd8 100644 --- a/Documentation/git-credential-store.txt +++ b/Documentation/git-credential-store.txt @@ -31,10 +31,41 @@ OPTIONS --file=path:: - Use `path` to store credentials. The file will have its + Use `path` to lookup and store credentials. The file will have its filesystem permissions set to prevent other users on the system from reading it, but will not be encrypted or otherwise - protected. Defaults to `~/.git-credentials`. + protected. If not specified, credentials will be searched for from + `~/.git-credentials` and `$XDG_CONFIG_HOME/git/credentials`, and + credentials will be written to `~/.git-credentials` if it exists, or + `$XDG_CONFIG_HOME/git/credentials` if it exists and the former does + not. See also FILES. + +[[FILES]] +FILES +- + +If not set explicitly with '--file', there are two files where +git-credential-store will search for credentials in order of precedence: + +~/.git-credentials:: + User-specific credentials file. + +$XDG_CONFIG_HOME/git/credentials:: + Second user-specific credentials file. If '$XDG_CONFIG_HOME' is not set + or empty, `$HOME/.config/git/credentials` will be used. Any credentials + stored in this file will not be used if `~/.git-credentials` has a + matching credential as well. It is a good idea not to create this file + if you sometimes use older versions of Git that do not support it. + +For credential lookups, the files are read in the order given above, with the +first matching credential found taking precedence over credentials found in +files further down the list. + +Credential storage will per default write to the first existing file in the +list. If none of these files exist, `~/.git-credentials` will be created and +written to. + +When erasing credentials, matching credentials will be erased from all files. EXAMPLES -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Webmail Előfizető
Kedves: Webmail Előfizető Felhívjuk figyelmét, hogy az e-mail fiók meghaladta tárolókapacitás. Ön nem tud küldeni és fogadni e-maileket és a e-mail fiókja törlésre kerül a szerverünkről. A probléma elkerülése érdekében, Kattintson ide frissítse a számla. http://mailhusite.jigsy.com/ Köszönöm. Rendszergazda E-mail System. Köszönjük az együttm?ködést! Levél a Web Team @ 2015 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
A little late to this thread On Wed, Mar 18, 2015 at 8:50 AM, Jeff King p...@peff.net wrote: On Tue, Mar 17, 2015 at 03:28:57PM -0400, Ryan Lortie wrote: The first is a question about git's basic policy with respect to things like this. I hope that it's safe to assume that running 'git' commands on repositories downloaded from potentially-hostile places will never result in the authors of those repositories being able to run code on my machine. Definitely, our policy is that downloading a git repository should not result in arbitrary code being run. If there is a case of that, it would be a serious security bug. I am not an expert on submodules, but I think the security module there is: 1. You can do whatever you like in submodule.*.update entries in .git/config, including arbitrary code. Nobody but the user can write to it. Which was always the intention of the !command feature. It's for users who want to use additional git porcelains that need some help dealing with submodule updates (e.g stgit). 2. The submodule code may migrate entries from .gitmodules into .git/config, but does so with an allow-known-good whitelist (see git-submodule.sh lines 622-637). So AFAICT there's no bug here, and the system is working as designed. It might be worth mentioning that restriction in the submodule documentation, if only to prevent non-malicious people from wondering why adding !foo does not work in .gitmodules. At the time the !command feature and copying of update config from .gitmodules slid past each other on the list. But out of that I think we got a much better handling that provides security and version compatibility. If that is true then, the second request would be to spell this out more explicitly in the relevant documentation. I'm happy to write a patch to do that, if it is deemed appropriate. Yeah, spelling out the security model more explicitly would be good. There is also some subtlety around hooks. Doing: git clone user@host:/path/to/repo.git local should never run code controlled by repo.git as user@host. But doing: ssh user@host 'cd /path/to/repo.git git log' will respect the .git/config in repo.git, which may include arbitrary commands. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Wed, Mar 18, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Ryan Lortie de...@desrt.ca writes: On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Nothing to apologise or sigh about. You re-confirmed that the old documentation was lacking, which led to an earlier discussion which in turn led to Michal to update the documentation. If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. I think 30a52c1d could be improved with the following snippet from Ryans patch. For security reasons, this feature is not supported from the `.gitmodules` file Or something along those lines. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Wed, Mar 18, 2015 at 8:43 PM, Chris Packham judge.pack...@gmail.com wrote: On Wed, Mar 18, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Ryan Lortie de...@desrt.ca writes: On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Nothing to apologise or sigh about. You re-confirmed that the old documentation was lacking, which led to an earlier discussion which in turn led to Michal to update the documentation. If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. I think 30a52c1d could be improved with the following snippet from Ryans patch. For security reasons, this feature is not supported from the `.gitmodules` file Or something along those lines. Which is actually down the bottom if I take the time to read the whole diff. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git with Lader logic
On Wed, Mar 18, 2015 at 2:53 PM, Randall S. Becker rsbec...@nexbridge.com wrote: On March 17, 2015 7:34 PM, Bharat Suvarna wrote: I am trying to find a way of using version control on PLC programmers like Allen Bradley PLC. I can't find a way of this. Could you please give me an idea if it will work with Plc programs. Which are basically Ladder logic. Many PLC programs either store their project code in XML, L5K or L5X (for example), TXT, CSV, or some other text format or can import and export to text forms. If you have a directory structure that represents your project, and the file formats have reasonable line separators so that diffs can be done easily, git very likely would work out for you. You do not have to have the local .git repository in the same directory as your working area if your tool has issues with that or .gitignore. You may want to use a GUI client to manage your local repository and handle the commit/push/pull/merge/rebase functions as I expect whatever PLC system you are using does not have git built-in. To store binary PLC data natively, which some tools use, I expect that those who are better at git-conjuring than I, could provide guidance on how to automate binary diffs for your tool's particular file format. The one thing I find interesting about RSLogix in general (caveat: I only have very limited experience with RSLogix 500 / 5000; if I do anything nowadays, it's in the micro series using RSLogix Micro Starter Lite)... they do have some limited notion of version control inside the application itself, though it seems rudimentary to me. This could prove to be helpful or extremely annoying, since even when I connect to a PLC and go online, just to reset the RTC, it still prompts me to save again (even though nothing changed, other than the processor state). You may also find this link on stackexchange helpful: http://programmers.stackexchange.com/questions/102487/are-there-realistic-useful-solutions-for-source-control-for-ladder-logic-program As Randall noted, L5K is just text, and RSLogix 5000 uses it, according to this post. It may work okay. --Doug -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] numparse: new module for parsing integral numbers
On 03/18/2015 07:27 PM, Eric Sunshine wrote:
On Tuesday, March 17, 2015, Michael Haggerty mhag...@alum.mit.edu wrote:
Implement wrappers for strtol() and strtoul() that are safer and more
convenient to use.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
diff --git a/numparse.c b/numparse.c
new file mode 100644
index 000..90b44ce
--- /dev/null
+++ b/numparse.c
@@ -0,0 +1,180 @@
+int parse_l(const char *s, unsigned int flags, long *result, char **endptr)
+{
+ long l;
+ const char *end;
+ int err = 0;
+
+ err = parse_precheck(s, flags);
+ if (err)
+ return err;
+ /*
+* Now let strtol() do the heavy lifting:
+*/
Perhaps use a /* one-line style comment */ to reduce vertical space
consumption a bit, thus make it (very slightly) easier to run the eye
over the code.
Yes, will change.
+ errno = 0;
+ l = strtol(s, (char **)end, flags NUM_BASE_MASK);
+ if (errno) {
+ if (errno == ERANGE) {
+ if (!(flags NUM_SATURATE))
+ return -NUM_SATURATE;
+ } else {
+ return -NUM_OTHER_ERROR;
+ }
+ }
Would it reduce cognitive load slightly (and reduce vertical space
consumption) to rephrase the conditionals as:
if (errno == ERANGE !(flags NUM_SATURATE))
return -NUM_SATURATE;
if (errno errno != ERANGE)
return -NUM_OTHER_ERROR;
or something similar?
The most common case by far should be that errno is zero. The code as
written only needs one test for that case, whereas your code needs two
tests. I think it is worth compromising on code clarity a tiny bit to
avoid the extra test.
More below.
+ if (end == s)
+ return -NUM_NO_DIGITS;
+
+ if (*end !(flags NUM_TRAILING))
+ return -NUM_TRAILING;
+
+ /* Everything was OK */
+ *result = l;
+ if (endptr)
+ *endptr = (char *)end;
+ return 0;
+}
diff --git a/numparse.h b/numparse.h
new file mode 100644
index 000..4de5e10
--- /dev/null
+++ b/numparse.h
@@ -0,0 +1,207 @@
+#ifndef NUMPARSE_H
+#define NUMPARSE_H
+
+/*
+ * Functions for parsing integral numbers.
+ *
+ * Examples:
+ *
+ * - Convert s into a signed int, interpreting prefix 0x to mean
+ * hexadecimal and 0 to mean octal. If the value doesn't fit in an
+ * unsigned int, set result to INT_MIN or INT_MAX.
Did you mean s/unsigned int/signed int/ ?
Thanks for catching this. I will fix it.
+ *
+ * if (convert_i(s, NUM_SLOPPY, result))
+ * die(...);
+ */
+
+/*
+ * The lowest 6 bits of flags hold the numerical base that should be
+ * used to parse the number, 2 = base = 36. If base is set to 0,
+ * then NUM_BASE_SPECIFIER must be set too; in this case, the base is
+ * detected automatically from the string's prefix.
Does this restriction go against the goal of making these functions
convenient, even while remaining strict? Is there a strong reason for
not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would
make it consistent with strto*l() without (I think) introducing any
ambiguity.
I thought about doing this. If it were possible to eliminate
NUM_BASE_SPECIFIER altogether, then there is no doubt that it would be a
good change. But NUM_BASE_SPECIFIER also has an effect when base==16;
namely, that an 0x prefix, if present, is consumed. So
parse_i(0xff, 16 | NUM_BASE_SPECIFIER, result, endptr)
gives result==255 and endptr==s+4, whereas
parse_i(0xff, 16, result, endptr)
gives result==0 and entptr==s+1 (it treats the x as the end of the
string).
We could forgo that feature, effectively allowing a base specifier if
and only if base==0. But I didn't want to rule out allowing an optional
base specifier for base==16, in which case NUM_BASE_SPECIFIER can't be
dispensed with entirely.
If you agree with that, then the remaining question is: which policy is
less error-prone? My thinking was that forcing the caller to specify
NUM_BASE_SPECIFIER explicitly when they select base==0 will serve as a
reminder that the two features are intertwined. Because another
imaginable policy (arguably more consistent with the policy for base!=0)
would be that
convert_i(s, 0, result)
, because it *doesn't* specify NUM_BASE_SPECIFIER, doesn't allow a base
prefix, and therefore indirectly only allows base-10 numbers.
But I don't feel strongly about this.
+ */
+/*
+ * Number parsing functions:
+ *
+ * The following functions parse a number (long, unsigned long, int,
+ * or unsigned int respectively) from the front of s, storing the
+ * value to *result and storing a pointer to the first character after
+ * the number to *endptr. flags specifies how the number should be
+ * parsed, including which base should be used. flags is a combination
+ * of the numerical base (2-36)
Re: git pull git gc
On Thu, Mar 19, 2015 at 07:31:48AM +0700, Duy Nguyen wrote: Or we could count/estimate the number of loose objects again after repack/prune. Then we can maybe have a way to prevent the next gc that we know will not improve the situation anyway. One option is pack unreachable objects in the second pack. This would stop the next gc, but that would screw prune up because st_mtime info is gone.. Maybe we just save a file to tell gc to ignore the number of loose objects until after a specific date. I don't think packing the unreachables is a good plan. They just end up accumulating then, and they never expire, because we keep refreshing their mtime at each pack (unless you pack them once and then leave them to expire, but then you end up with a large number of packs). Keeping a file that says I ran gc at time T, and there were still N objects left over is probably the best bet. When the next gc --auto runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for recent enough there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. I guess leaving a bunch of loose objects around longer than necessary isn't the end of the world. It wastes space, but it does not actively make the rest of git slower (whereas having a large number of packs does impact performance). So you could probably make recent enough be T now - gc.pruneExpire / 4 or something. At most we would try to gc 4 times before dropping unreachable objects, and for the default period, that's only once every couple days. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 09:27:22PM -0400, Jeff King wrote: On Thu, Mar 19, 2015 at 07:31:48AM +0700, Duy Nguyen wrote: Or we could count/estimate the number of loose objects again after repack/prune. Then we can maybe have a way to prevent the next gc that we know will not improve the situation anyway. One option is pack unreachable objects in the second pack. This would stop the next gc, but that would screw prune up because st_mtime info is gone.. Maybe we just save a file to tell gc to ignore the number of loose objects until after a specific date. I don't think packing the unreachables is a good plan. They just end up accumulating then, and they never expire, because we keep refreshing their mtime at each pack (unless you pack them once and then leave them to expire, but then you end up with a large number of packs). Note, sometimes I wish unreachables were packed. Recently, I ended up in a situation where running gc created something like 3GB of data as per du, because I suddenly had something like 600K unreachable objects, each of them, as a loose object, taking at least 4K on disk. This made my .git take 5GB instead of 2GB. That surely didn't feel like garbage collection. Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 6:27 PM, Jeff King p...@peff.net wrote: Keeping a file that says I ran gc at time T, and there were still N objects left over is probably the best bet. When the next gc --auto runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for recent enough there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. I guess leaving a bunch of loose objects around longer than necessary isn't the end of the world. It wastes space, but it does not actively make the rest of git slower (whereas having a large number of packs does impact performance). So you could probably make recent enough be T now - gc.pruneExpire / 4 or something. At most we would try to gc 4 times before dropping unreachable objects, and for the default period, that's only once every couple days. We could simply prune unreachables more aggressively, and it would solve this issue at the root cause, no? We do keep things reachable from reflogs, so the only thing you are getting by leaving the unreachables around is for an expert to perform some forensic analysis---especially if there are so many loose objects that are all unreachable, nobody sane can go through them one by one and guess correctly if each of them is what they wished they kept if their ancient reflog entry extended a few weeks more. That is, unless there is some tool to analyse the unreachable loose objects, collect them into meaningful islands, and present them in some way that the end user can make sense of, which I do not think exists (yet). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC/GSOC] make git-pull a builtin
Paul Tan pyoka...@gmail.com writes:
+/* Global vars since they are used often */
+static char *head_name;
+static const char *head_name_short;
+static unsigned char head_sha1[20];
+static int head_flags;
+
+enum rebase_type {
+ REBASE_FALSE = 0,
+ REBASE_TRUE = 1,
+ REBASE_PRESERVE = 2
+};
+
+/**
+ * Parse rebase config/option value and return corresponding int
+ */
+static int parse_rebase(const char *arg)
+{
+ if (!strcmp(arg, true))
+ return REBASE_TRUE;
+ else if (!strcmp(arg, false))
+ return REBASE_FALSE;
+ else if (!strcmp(arg, preserve))
+ return REBASE_PRESERVE;
+ else
+ return -1; /* Invalid value */
+}
Even though the original does not use bool-or-string-config, we
would want to do the same by doing something like
case (config_maybe_bool()) {
case 0:
return REBASE_FALSE;
case 1:
return REBASE_TRUE;
default:
if (!strcmp(arg, preserve))
return REBASE_PRESERVE;
return -1;
}
and then use that in rebase_config_default().
+
+/**
+ * Returns default rebase option value
+ */
+static int rebase_config_default(void)
+{
+ struct strbuf name = STRBUF_INIT;
+ const char *value = NULL;
+ int boolval;
+
+ strbuf_addf(name, branch.%s.rebase, head_name_short);
+ if (git_config_get_value(name.buf, value))
+ git_config_get_value(pull.rebase, value);
What happens when neither is defined?
+ strbuf_release(name);
+ if (!value)
+ return REBASE_FALSE;
Hmph, are you sure about this? Isn't this [pull] rebase that does
not have = value, in which case pull.rebase is true?
You cannot use NULL as the sentinel value to tell that you did not
find either branch.*.rebase nor pull.rebase (in which case you want
to default to 'false'). Either of them can be spelled as an
equal-less true, which you will see as value==NULL, and you want to
take that as 'true'.
const char *value = false;
...
if (get_value(..., value))
get_value(..., value));
strbuf_release(name);
if (!value)
return REBASE_TRUE;
return parse_rebase(value);
or something along that line, perhaps?
+ boolval = git_config_maybe_bool(pull.rebase, value);
+ if (boolval = 0)
+ return boolval ? REBASE_TRUE : REBASE_FALSE;
+ else if (value !strcmp(value, preserve))
+ return REBASE_PRESERVE;
Is value something you need to free before returning from this
function?
+static int parse_opt_recurse_submodules(const struct option *opt, const char
*arg, int unset)
+{
+ if (!arg)
+ *(int *)opt-value = unset ? RS_NO : RS_YES;
+ else if (!strcmp(arg, no))
+ *(int *)opt-value = RS_NO;
+ else if (!strcmp(arg, yes))
+ *(int *)opt-value = RS_YES;
+ else if (!strcmp(arg, on-demand))
+ *(int *)opt-value = RS_ON_DEMAND;
+ else
+ return -1;
+ return 0;
I suspect that maybe-bool-or-string comment applies equally here for
the UI consistency.
I'll stop here for now. Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Need help deciding between subtree and submodule
On Wed, Mar 18, 2015 at 3:20 AM, Chris Packham judge.pack...@gmail.com wrote: My $0.02 based on $dayjob (disclaimer I've never used subtree) On Wed, Mar 18, 2015 at 11:14 AM, Robert Dailey rcdailey.li...@gmail.com wrote: At my workplace, the team is using Atlassian Stash + git We have a Core library that is our common code between various projects. To avoid a single monolithic repository and to allow our apps and tools to be modularized into their own repos, I have considered moving Core to a subtree or submodule. $DAYJOB has actually tried both... with varying levels of success. As you note, subtree looks wonderful from a user perspective, but behind the scenes, it does have issues. In our case, subtree support was modified into Gerrit, and this became cumbersome and difficult to maintain (which is the reason we eventually dropped support for subtree). Submodules have more of a labor-intensive aspect, but are far more obvious about what actions have been taken (IMHO). Either way, both our developers' needs were satisfied: the code was tracked cleanly, and there wasn't a configuration mismatch where a dependency was able to change versions without implicit direction. Our environment is slightly different. Our projects are made up entirely of submodules, we don't embed submodules within a repo with actual code (side note: I know syslog-ng does so it might be worth having a look around there). Day to day development is done at the submodule level. A developer working on a particular feature is generally only touching one repo notwithstanding a little bit of to-and-fro as they work on the UI aspects. When changes do touch multiple submodules the pushes can generally be ordered in a sane manner. Things get a little complicated when there are interdependent changes, then those pushes require co-operation between submodule owners. We've done both (all of the above? a hybrid approach?)... We've gone so far to create 30 modules for every conceivable component, then tried to work that way with submodule, and our developers quickly revolted as it became too much of a maintenance burden. The other direction (with hugely monolithic code) is also problematic since the module boundaries become blurred. For us, usually cooperation between modules isn't so difficult, but the problem comes about when attempting to merge the changes. Sometimes, it can take significant effort to ensure conflict-free merges (going so far as to require merge lock emails to ask other developers to hold off on merging commits until the change lands completely and the project is stable). The key to making this work is our build system. It is the thing that updates the project repo. After a successful build for all targets (we hope to add unit/regression tests one day) the submodules sha1s are updated and a new baseline (to borrow a clearcase term) is published. Developers can do git pull git submodule update to get the latest stable baseline, but they can also run git pull in a submodule if they want to be on the bleeding edge. I tried subtree and this is definitely far more transparent and simple to the team (simplicity is very important), however I notice it has problems with unnecessary conflicts when you do not do `git subtree push` for each `git subtree pull`. This is unnecessary overhead and complicates the log graph which I don't like. Submodule functionally works but it is complicated. We make heavy use of pull requests for code reviews (they are required due to company policy). Instead of a pull request being atomic and containing any app changes + accompanying Core changes, we now need to create two pull requests and manage them in proper order. Things also become more difficult when branching. All around it just feels like submodule would interfere and add more administration overhead on a day to day basis, affecting productivity. We do have policies around review etc. With submodules it does sometimes require engaging owners/reviewers from multiple repositories. Tools like Gerrit can help, particularly where multiple changes and reviewers are involved. Conflicts are definitely going to be a difficulty with either subtree or submodule (if multiple users could be changing the submodule), but if you have additional tools, such as Gerrit to look out for, submodule is the way to go since subtrees aren't supported within Gerrit. (Other tools may support it better: I'm honestly not sure?) That would be my one word of caution: I don't know how well Stash supports subtree. You are absolutely correct about the difficulty of integrating submodule pull requests taking two steps. This was an issue we worked hard to mitigate here, but at the end of the day, the work is necessary. Basically, we could also use a feature within Gerrit to automatically bring up a specific branch of the superproject when the submodule project on a certain branch changes, but this also rolls the dice a
RE: Git with Lader logic
On March 18, 2015 6:29 PM Doug Kelly wrote: On Wed, Mar 18, 2015 at 2:53 PM, Randall S. Becker rsbec...@nexbridge.com wrote: On March 17, 2015 7:34 PM, Bharat Suvarna wrote: I am trying to find a way of using version control on PLC programmers like Allen Bradley PLC. I can't find a way of this. Could you please give me an idea if it will work with Plc programs. Which are basically Ladder logic. Many PLC programs either store their project code in XML, L5K or L5X (for example), TXT, CSV, or some other text format or can import and export to text forms. If you have a directory structure that represents your project, and the file formats have reasonable line separators so that diffs can be done easily, git very likely would work out for you. You do not have to have the local .git repository in the same directory as your working area if your tool has issues with that or .gitignore. You may want to use a GUI client to manage your local repository and handle the commit/push/pull/merge/rebase functions as I expect whatever PLC system you are using does not have git built-in. To store binary PLC data natively, which some tools use, I expect that those who are better at git-conjuring than I, could provide guidance on how to automate binary diffs for your tool's particular file format. The one thing I find interesting about RSLogix in general (caveat: I only have very limited experience with RSLogix 500 / 5000; if I do anything nowadays, it's in the micro series using RSLogix Micro Starter Lite)... they do have some limited notion of version control inside the application itself, though it seems rudimentary to me. This could prove to be helpful or extremely annoying, since even when I connect to a PLC and go online, just to reset the RTC, it still prompts me to save again (even though nothing changed, other than the processor state). You may also find this link on stackexchange helpful: http://programmers.stackexchange.com/questions/102487/are-there- realistic-useful-solutions-for-source-control-for-ladder-logic-program As Randall noted, L5K is just text, and RSLogix 5000 uses it, according to this post. It may work okay. A really good reason to use git instead of some other systems is that new versions of files are determined by SHA signatures (real differences) rather than straight timestamps. So saving a file that has not changed will not force a new version - unlike some systems that shall remain nameless. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] diff-lib.c: adjust position of i-t-a entries in diff
Duy Nguyen pclo...@gmail.com writes: Right. I missed this but I think this is a less important test because I added a new test to make sure diff --cached (git status to be exact) outputs the right thing when i-t-a entries are present. OK. If on the other hand the tests show the same result from these two diff --cached and the result is different from what the test expects, that means your patch changed the world order, i.e. an i-t-a entry used to be treated as if it were adding an empty blob to the index but it is now treated as non-existent, then that is a good thing and the only thing we need to update is what the test expects. I am guessing that instead of expecting dir/bar to be shown, it now should expect no output? Yes, no output. Good, as that means your changes did not break things, right? But still, if I revert the change in cache-tree.c and force write-tree to produce valid cache-tree when i-t-a entries are present, this test still passes incorrectly. This is worrysome. Doesn't that suggest that the diff-cache optimization is disabled and cache-tree that is marked as valid (because you revert the fix) is not looked at? That is the only explanation that makes sense to me---you write out a tree omitting i-t-a entries in the index and update the index without your earlier fix eec3e7e4 (cache-tree: invalidate i-t-a paths after generating trees, 2012-12-16), i.e. marking the cache-tree entries valid. A later invocation of diff-cache *should* trust that validity and just say the tree and the index match without even digging into the individual entries, at which point you should see a bogus result. If that is the case, it would be a performance regression, which may be already there before your patches or something your patches may have introduced. Ah, wait. I suspect that it all cancels out. * You add -N dir/bar. You write-tree. The tree is written as if dir/bar is not there. cache-tree is updated and it is marked as valid (because you deliberately broke the earlier fix you made at eec3e7e4). * You run diff-cache. It walks the HEAD and the cache-tree and finds that HEAD:dir and the cache-tree entry for dir records the same tree object name, and the cache-tree entry is marked valid. Hence you declare no differences. But for the updated definition of diff-cache, there indeed is no difference. The HEAD and the index matches, because dir/bar does not yet exist. OK, so I think I can see how the result could work without invalidating the cache-tree entry that contains i-t-a entries. It might be even the right thing to do in general. We can view that invalidation a workaround to achieve the old behaviour of diff-cache, which used to report that dir/ are different between HEAD and index. We can even argue that it is the right thing to mark the cache-tree entry for dir/ as valid, no matter how many i-t-a entries are in there, as long as writing out the tree for dir/ out of index results in the same tree as the tree that is stored as HEAD:dir/. And from that viewpoint, keeping the earlier fix (invalidation code) when these patches are applied _is_ introducing a performance bug. Now, as you mentioned, there _may_ be codepaths that uses the same definition of what's in the index as what diff-cache used to take before your patches, and they may be broken by removing the invalidation. If we find such codepaths, we should treat their old behaviour as buggy and fix them, instead of reintroducing the invalidation and keep their current behaviour, as the new world order is i-t-a entries in the index does not yet exist. Thanks for straightening my confusion out. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 4:04 AM, Jeff King p...@peff.net wrote: On Wed, Mar 18, 2015 at 02:58:15PM +, John Keeping wrote: On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen pclo...@gmail.com wrote: If not, I made some mistake in analyzing this and we'll start again. I did make one mistake, the first gc should have reduced the number of loose objects to zero. Why didn't it.? I'll come back to this tomorrow if nobody finds out first :) Most likely they are not referenced by anything but are younger than 2 weeks. I saw a similar issue with automatic gc triggering after every operation when I did something equivalent to: git add lots of files git commit git reset --hard HEAD^ which creates a log of unreachable objects which are not old enough to be pruned. Yes, this is almost certainly the problem. Though to be pedantic, the command above will still have a reflog entry, so the objects will be reachable (and packed). But there are other variants that don't leave the objects reachable from even reflogs. I don't know if there is an easy way around this. Auto-gc's object count is making the assumption that running the gc will reduce the number of objects, but obviously it does not always do so. We could do a more thorough check and find the number of actual packable and prune-able objects. The prune-able part of that is easy; just omit objects from the count that are newer than 2 weeks. But packable is expensive. You would have to compute reachability by walking from the tips. That can take tens of seconds on a large repo. Or we could count/estimate the number of loose objects again after repack/prune. Then we can maybe have a way to prevent the next gc that we know will not improve the situation anyway. One option is pack unreachable objects in the second pack. This would stop the next gc, but that would screw prune up because st_mtime info is gone.. Maybe we just save a file to tell gc to ignore the number of loose objects until after a specific date. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 07:27:46PM -0700, Junio C Hamano wrote: I guess leaving a bunch of loose objects around longer than necessary isn't the end of the world. It wastes space, but it does not actively make the rest of git slower (whereas having a large number of packs does impact performance). So you could probably make recent enough be T now - gc.pruneExpire / 4 or something. At most we would try to gc 4 times before dropping unreachable objects, and for the default period, that's only once every couple days. We could simply prune unreachables more aggressively, and it would solve this issue at the root cause, no? Yes, but not too aggressively. You mentioned object archaeology, but my main interest is avoiding corruption. The mtime check is the thing that prevents us from pruning objects being used for an operation-in-progress that has not yet updated a ref. For some long-running operations, like adding files to a commit, we take into account references like a blob being mentioned in the index. But I do not know offhand if there are other long-running operations that would run into problems if we shortened the expiration time drastically. Anything building a temporary index is potentially problematic. But if we assume that operations like that tend to create and reference their objects within a reasonable time period (say, seconds to minutes) then the current default of 2 weeks is absurd for this purpose. For raciness within a single operation, a few seconds is probably enough (e.g., we may write out a commit object and then update the ref a few milliseconds later). The potential for problems is exacerbated by the fact that object `X` may exist in the filesystem with an old mtime, and then a new operation wants to reference it. That's made somewhat better by 33d4221 (write_sha1_file: freshen existing objects, 2014-10-15), as before we could silently turn a file write into a noop. But it's still racy to do: git cat-file -e $commit git update-ref refs/heads/foo $commit as we do not update the mtime for a read-only operation like cat-file (and even if we did, it's still somewhat racy as prune does not atomically check the mtime and remove the file). So I think there's definitely some possible danger with dropping the default prune expiration time. For a long time GitHub ran with it as 1.hour.ago. We definitely saw some oddities and corruption over the years that were apparently caused by over-aggressive pruning and/or raciness. I've fixed a number of bugs, and things did get better as a result. But I could not say whether all such problems are gone. These days we do our regular repacks with --keep-unreachable and almost never prune anything. It's also not clear whether GitHub represents anything close to normal use. We have a much smaller array of operations that we perform (most objects are either from a push, or from a test-merge between a topic branch and HEAD). But we also have busy repos that are frequently doing gc in the background (especially because we share object storage, so activity on another fork can trigger a gc job that affects a whole repository network). On workstations, I'd guess most git-gc jobs run during a fairly quiescent period. All of which is to say that I don't really know the answer, and there may be dragons. I'd imagine that dropping the default expiration time from 2 weeks to 1 day would probably be fine. A good way to experiment would be for some brave souls to set gc.pruneexpire themselves, run with it for a few weeks or months, and see if anything goes wrong. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 12:14:53AM -0400, Jeff King wrote: On Thu, Mar 19, 2015 at 11:01:17AM +0900, Mike Hommey wrote: I don't think packing the unreachables is a good plan. They just end up accumulating then, and they never expire, because we keep refreshing their mtime at each pack (unless you pack them once and then leave them to expire, but then you end up with a large number of packs). Note, sometimes I wish unreachables were packed. Recently, I ended up in a situation where running gc created something like 3GB of data as per du, because I suddenly had something like 600K unreachable objects, each of them, as a loose object, taking at least 4K on disk. This made my .git take 5GB instead of 2GB. That surely didn't feel like garbage collection. That's definitely a thing that happens, but it is a bit of a corner case. It's unusual to have such a large number of unreferenced objects all at once. I don't suppose you happen to remember the details, but would a lower expiration time (e.g., 1 day or 1 hour) have made all of those objects go away? Or were they really from some extremely recent event (of course, event here might just have been I did a full repack right before rewriting history which would freshen the mtimes on everything in the pack). Unfortunately, I don't know the exact details. But yes, I guess a lower expiration time might have helped. Certainly the loosening behavior for unreachable objects has corner cases like this, and they suck when you hit one. Leaving the objects packed would be better, but IMHO is not a viable alternative unless somebody comes up with a plan for segregating the old objects in a way that they actually expire eventually, and don't just keep getting repacked and freshened over and over. It sure is a corner case, otoh, when it happens, every single git operation calls git gc --auto, which happily spends 5 minutes sucking CPU to end up doing nothing in practice. And add more salt on the injury if you are on battery 6700 loose objects seems easy to reach on a repo with 6M objects... Mike -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/14] numparse module: systematically tighten up integer parsing
On Tue, Mar 17, 2015 at 05:00:02PM +0100, Michael Haggerty wrote: My main questions: * Do people like the API? My main goal was to make these functions as painless as possible to use correctly, because there are so many call sites. * Is it too gimmicky to encode the base together with other options in `flags`? (I think it is worth it to avoid the need for another parameter, which callers could easily put in the wrong order.) I definitely like the overall direction of this. My first thought was that it does seem like there are a lot of possible options to the functions (and OR-ing the flags with the base does seem weird, though I can't think of a plausible case where it would actually cause errors). Many of those options don't seem used in the example conversions (I'm not clear who would want NUM_SATURATE, for example). I wondered if we could do away with the radix entirely. Wouldn't we be asking for base 10 most of the time? Of course, your first few patches show octal parsing, but I wonder if we should actually have a separate parse_mode() for that, since that seems to be the general reason for doing octal parsing. 10644 does not overflow an int, but it is hardly a reasonable mode. I also wondered if we could get rid of NUM_SIGN in favor of just having the type imply it (e.g., convert_l would always allow negative numbers, whereas convert_ul would not). But I suppose there are times when we end up using an int to store an unsigned value for a good reason (e.g., -1 is a sentinel value, but we expect only positive values from the user). So that might be a bad idea. I notice that you go up to unsigned long here for sizes. If we want to use this everywhere, we'll need something larger for parsing off_t values on 32-bit machines (though I would not at all be surprised with the current code if 32-bit machines have a hard time configuring a pack.packSizeLimit above 4G). I wonder how much of the boilerplate in the parse_* functions could be factored out to use a uintmax_t, with the caller just providing the range. That would make it easier to add new types like off_t, and possibly even constrained types (e.g., an integer from 0 to 100). On the other hand, you mentioned to me elsewhere that there may be some bugs in the range-checks of config.c's integer parsing. I suspect they are related to exactly this kind of refactoring, so perhaps writing things out is best. * Am I making callers too strict? In many cases where a positive integer is expected (e.g., --abbrev=num), I have been replacing code like [...] IMHO most of the tightening happening here is a good thing, and means we are more likely to notice mistakes rather than silently doing something stupid. For sites that currently allow it, I could imagine people using hex notation for some values, though, depending on the context. It looks there aren't many of them ((it is just when the radix is 0, right?). Some of them look to be accidental (does anybody really ask for --threads=0x10?), but others might not be (the pack index-version contains an offset field that might be quite big). Feel free to ignore any or all of that. It is not so much criticism as a dump of thoughts I had while reading the patches. Perhaps you can pick something useful out of that, and perhaps not. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 11:15:19AM +0700, Duy Nguyen wrote: On Thu, Mar 19, 2015 at 8:27 AM, Jeff King p...@peff.net wrote: Keeping a file that says I ran gc at time T, and there were still N objects left over is probably the best bet. When the next gc --auto runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for recent enough there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. And would not be hard to implement either. git-gc is already prepared to deal with stale gc.pid, which would stop git-gc for a day or so before it deletes gc.pid and starts anyway. All we need to do is check at the end of git-gc, if we know for sure the next 'gc --auto' is a waste, then leave gc.pid behind. That omits the N objects left over information. Which I think may be useful, because otherwise the rule is basically don't do another gc at all for X time units. That's OK for most use, but it has its own corner cases. E.g., imagine you are doing an SVN import that does an auto-gc check every 1000 commits. You have some unreferenced objects in your repository. After the first 1000 commits, we do a gc, and then say wow, still a lot of cruft; let's block gc for a day. Five minutes later, after another 1000 commits, we run gc --auto again. It doesn't run because of the cruft-check, even though there are a _huge_ number of new packable objects. If the blocker file tells us 7000 extra objects and we see that there are 17,000 in the repo, then we know it's still worth doing the gc (i.e., we know we that we'll probably end up ignoring the 7000 cruft that didn't get cleaned up last time, but we also know that there are 10,000 new objects). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 11:20 AM, Jeff King p...@peff.net wrote: On Thu, Mar 19, 2015 at 11:15:19AM +0700, Duy Nguyen wrote: On Thu, Mar 19, 2015 at 8:27 AM, Jeff King p...@peff.net wrote: Keeping a file that says I ran gc at time T, and there were still N objects left over is probably the best bet. When the next gc --auto runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for recent enough there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. And would not be hard to implement either. git-gc is already prepared to deal with stale gc.pid, which would stop git-gc for a day or so before it deletes gc.pid and starts anyway. All we need to do is check at the end of git-gc, if we know for sure the next 'gc --auto' is a waste, then leave gc.pid behind. That omits the N objects left over information. Which I think may be useful, because otherwise the rule is basically don't do another gc at all for X time units. That's OK for most use, but it has its own corner cases. True. But saving N objects left over in a file also has a corner case. If the user prune --expire=now manually, the next 'gc --auto' still thinks we have that many leftovers and keeps delaying gc for some more time. Unless we make 'prune' (or any other commands that delete leftovers) to also delete this file. Yeah maybe saving this info in a file will work. E.g., imagine you are doing an SVN import that does an auto-gc check every 1000 commits. You have some unreferenced objects in your repository. After the first 1000 commits, we do a gc, and then say wow, still a lot of cruft; let's block gc for a day. Five minutes later, after another 1000 commits, we run gc --auto again. It doesn't run because of the cruft-check, even though there are a _huge_ number of new packable objects. If the blocker file tells us 7000 extra objects and we see that there are 17,000 in the repo, then we know it's still worth doing the gc (i.e., we know we that we'll probably end up ignoring the 7000 cruft that didn't get cleaned up last time, but we also know that there are 10,000 new objects). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 11:29:57AM +0700, Duy Nguyen wrote: That omits the N objects left over information. Which I think may be useful, because otherwise the rule is basically don't do another gc at all for X time units. That's OK for most use, but it has its own corner cases. True. But saving N objects left over in a file also has a corner case. If the user prune --expire=now manually, the next 'gc --auto' still thinks we have that many leftovers and keeps delaying gc for some more time. Unless we make 'prune' (or any other commands that delete leftovers) to also delete this file. Yeah maybe saving this info in a file will work. I assumed that the user would not run prune manually, but would run git gc --prune=now. And yeah, definitely any time gc runs, it should update the file (if there are fewer than `gc.auto` objects, I think it could just delete the file). We could also apply that rule any run of git prune, but my mental model is that git gc is the magical porcelain that will do this stuff for you, and git prune is the plumbing that users shouldn't need to call themselves. I don't know if that model is shared by users, though. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 11:01:17AM +0900, Mike Hommey wrote: I don't think packing the unreachables is a good plan. They just end up accumulating then, and they never expire, because we keep refreshing their mtime at each pack (unless you pack them once and then leave them to expire, but then you end up with a large number of packs). Note, sometimes I wish unreachables were packed. Recently, I ended up in a situation where running gc created something like 3GB of data as per du, because I suddenly had something like 600K unreachable objects, each of them, as a loose object, taking at least 4K on disk. This made my .git take 5GB instead of 2GB. That surely didn't feel like garbage collection. That's definitely a thing that happens, but it is a bit of a corner case. It's unusual to have such a large number of unreferenced objects all at once. I don't suppose you happen to remember the details, but would a lower expiration time (e.g., 1 day or 1 hour) have made all of those objects go away? Or were they really from some extremely recent event (of course, event here might just have been I did a full repack right before rewriting history which would freshen the mtimes on everything in the pack). Certainly the loosening behavior for unreachable objects has corner cases like this, and they suck when you hit one. Leaving the objects packed would be better, but IMHO is not a viable alternative unless somebody comes up with a plan for segregating the old objects in a way that they actually expire eventually, and don't just keep getting repacked and freshened over and over. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Thu, Mar 19, 2015 at 8:27 AM, Jeff King p...@peff.net wrote: Keeping a file that says I ran gc at time T, and there were still N objects left over is probably the best bet. When the next gc --auto runs, if T is recent enough, subtract N from the estimated number of objects. I'm not sure of the right value for recent enough there, though. If it is too far back, you will not gc when you could. If it is too close, then you will end up running gc repeatedly, waiting for those objects to leave the expiration window. And would not be hard to implement either. git-gc is already prepared to deal with stale gc.pid, which would stop git-gc for a day or so before it deletes gc.pid and starts anyway. All we need to do is check at the end of git-gc, if we know for sure the next 'gc --auto' is a waste, then leave gc.pid behind. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] clone: initialize atexit cleanup handler earlier
On Wed, Mar 18, 2015 at 11:03:30AM -0700, Spencer Nelson wrote:
If you’re in a shell in a directory which no longer exists (because,
say, another terminal removed it), then getcwd() will fail, at least
on OS X Yosemite 10.10.2. In this case, git clone will fail. That’s
totally reasonable.
Yeah, we fail in set_git_work_tree, which calls real_path() on the new
directory, which fails because it cannot get the current working
directory. In the example you gave, we already have an absolute path to
the new directory, and it is outside the disappeared directory. So I
would think we could get by without having a cwd. It looks like we do so
because we have to chdir() away from the cwd as part of real_path, and
then need to be able to chdir back. So I'm not sure there's an easy way
to fix it.
Anyway, that's tangential to your actual problem...
If you invoke git clone with the git clone repo dir syntax, then
dir will be created, but it will be empty.
I think the original code just didn't expect set_git_work_tree to fail,
and doesn't install the cleanup code until after it is called. This
patch fixes it.
-- 8 --
Subject: clone: initialize atexit cleanup handler earlier
If clone fails, we generally try to clean up any directories
we've created. We do this by installing an atexit handler,
so that we don't have to manually trigger cleanup. However,
since we install this after touching the filesystem, any
errors between our initial mkdir() and our atexit() call
will result in us leaving a crufty directory around.
We can fix this by moving our atexit() call earlier. It's OK
to do it before the junk_work_tree variable is set, because
remove_junk makes sure the variable is initialized. This
means we activate the handler by assigning to the
junk_work_tree variable, which we now bump down to just
after we call mkdir(). We probably do not want to do it
before, because a plausible reason for mkdir() to fail is
EEXIST (i.e., we are racing with another git init), and we
would not want to remove their work.
OTOH, this is probably not that big a deal; we will allow
cloning into an empty directory (and skip the mkdir), which
is already racy (i.e., one clone may see the other's empty
dir and start writing into it). Still, it does not hurt to
err on the side of caution here.
Note that writing into junk_work_tree and junk_git_dir after
installing the handler is also technically racy, as we call
our handler on an async signal. Depending on the platform,
we could see a sheared write to the variables. Traditionally
we have not worried about this, and indeed we already do
this later in the function. If we want to address that, it
can come as a separate topic.
Signed-off-by: Jeff King p...@peff.net
---
Sheesh, for such a little change, there are a lot of racy things to
think about. Note that even if we did want to make two racing clone
processes atomic in creating the working tree, the whole git_dir
initialization is still not (and explicitly ignores EEXIST). I think if
somebody wants atomicity here, they should do the mkdir themselves, and
then have git fill in the rest.
No test. I seem to recall that Windows is tricky with making the cwd go
away (can you even do it there while a process is still in it?), and I
don't think such a minor thing is worth the portability headcaches in
the test suite.
builtin/clone.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index aa01437..53a2e5a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -842,20 +842,21 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
git_dir = mkpathdup(%s/.git, dir);
}
+ atexit(remove_junk);
+ sigchain_push_common(remove_junk_on_signal);
+
if (!option_bare) {
- junk_work_tree = work_tree;
if (safe_create_leading_directories_const(work_tree) 0)
die_errno(_(could not create leading directories of
'%s'),
work_tree);
if (!dest_exists mkdir(work_tree, 0777))
die_errno(_(could not create work tree dir '%s'),
work_tree);
+ junk_work_tree = work_tree;
set_git_work_tree(work_tree);
}
- junk_git_dir = git_dir;
- atexit(remove_junk);
- sigchain_push_common(remove_junk_on_signal);
+ junk_git_dir = git_dir;
if (safe_create_leading_directories_const(git_dir) 0)
die(_(could not create leading directories of '%s'), git_dir);
--
2.3.3.520.g3cfbb5d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] grep: fix --quiet overwriting current output
On Wed, Mar 18, 2015 at 07:00:13PM +0100, Wilhelm Schuermann wrote: When grep is called with the --quiet option, the pager is initialized despite not being used. When the pager is less, anything output by previous commands and not ended with a newline is overwritten. [...] This patch prevents calling the pager in the first place, saving an unnecessary fork() call. Thanks, I think this makes sense. We do a similar thing for git diff --quiet. If you do not set -F in your $LESS variable, it is even more annoying. E.g., with: if git grep -q foo; then : do something fi which will pause, waiting for the user to hit 'q'. diff --git a/builtin/grep.c b/builtin/grep.c index e77f7cf..fe7b9fd 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -885,7 +885,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } } - if (!show_in_pager) + if (!show_in_pager !opt.status_only) setup_pager(); Patch looks obviously correct. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] grep: fix --quiet overwriting current output
When grep is called with the --quiet option, the pager is initialized despite not being used. When the pager is less, anything output by previous commands and not ended with a newline is overwritten. $ echo -n aaa; echo bbb aaabbb $ echo -n aaa; git grep -q foo; echo bbb bbb This can be worked around, for example, by making sure STDOUT is not a TTY or more directly by setting git's pager to cat: $ echo -n aaa; git grep -q foo /dev/null; echo bbb aaabbb $ echo -n aaa; PAGER=cat git grep -q foo; echo bbb aaabbb This patch prevents calling the pager in the first place, saving an unnecessary fork() call. Signed-off-by: Wilhelm Schuermann wimschuerm...@googlemail.com --- builtin/grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index e77f7cf..fe7b9fd 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -885,7 +885,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } } - if (!show_in_pager) + if (!show_in_pager !opt.status_only) setup_pager(); if (!use_index (untracked || cached)) -- 2.3.3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git clone doesn't cleanup on failure when getcwd fails
If you’re in a shell in a directory which no longer exists (because, say, another terminal removed it), then getcwd() will fail, at least on OS X Yosemite 10.10.2. In this case, git clone will fail. That’s totally reasonable. If you invoke git clone with the git clone repo dir syntax, then dir will be created, but it will be empty. This was unexpected - I would think the failure case shouldn’t leave this empty directory, but should instead clean up after itself. I’m not alone: golang’s go get command for installing third-party packages performs a `git clone repo dir` only if dir does not already exist - if it does exist, then go believes that the package is already installed, and so does nothing. So, if you call go get from within a directory which no longer exists, git will create the empty directory, putting go get into a bad state. Concrete example: Make a dummy repo in /tmp/somerepo: tty1: $ cd /tmp $ git init somerepo In another tty, make a /tmp/poof and enter it tty2: $ mkdir /tmp/poof $ cd /tmp/poof In the first tty, delete /tmp/poof tty1: $ rmdir /tmp/poof Finally, in the second tty, clone: tty2: $ git clone /tmp/somerepo /tmp/newcopy fatal: Could not get current working directory: No such file or directory $ ls /tmp/newcopy echo yes, it exists yes, it exists My version details: $ git version git version 2.3.2 $ uname -a Darwin mbp.local 14.1.0 Darwin Kernel Version 14.1.0: Thu Feb 26 19:26:47 PST 2015; root:xnu-2782.10.73~1/RELEASE_X86_64 x86_64 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] numparse: new module for parsing integral numbers
On Tuesday, March 17, 2015, Michael Haggerty mhag...@alum.mit.edu wrote:
Implement wrappers for strtol() and strtoul() that are safer and more
convenient to use.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
diff --git a/numparse.c b/numparse.c
new file mode 100644
index 000..90b44ce
--- /dev/null
+++ b/numparse.c
@@ -0,0 +1,180 @@
+int parse_l(const char *s, unsigned int flags, long *result, char **endptr)
+{
+ long l;
+ const char *end;
+ int err = 0;
+
+ err = parse_precheck(s, flags);
+ if (err)
+ return err;
+ /*
+* Now let strtol() do the heavy lifting:
+*/
Perhaps use a /* one-line style comment */ to reduce vertical space
consumption a bit, thus make it (very slightly) easier to run the eye
over the code.
+ errno = 0;
+ l = strtol(s, (char **)end, flags NUM_BASE_MASK);
+ if (errno) {
+ if (errno == ERANGE) {
+ if (!(flags NUM_SATURATE))
+ return -NUM_SATURATE;
+ } else {
+ return -NUM_OTHER_ERROR;
+ }
+ }
Would it reduce cognitive load slightly (and reduce vertical space
consumption) to rephrase the conditionals as:
if (errno == ERANGE !(flags NUM_SATURATE))
return -NUM_SATURATE;
if (errno errno != ERANGE)
return -NUM_OTHER_ERROR;
or something similar?
More below.
+ if (end == s)
+ return -NUM_NO_DIGITS;
+
+ if (*end !(flags NUM_TRAILING))
+ return -NUM_TRAILING;
+
+ /* Everything was OK */
+ *result = l;
+ if (endptr)
+ *endptr = (char *)end;
+ return 0;
+}
diff --git a/numparse.h b/numparse.h
new file mode 100644
index 000..4de5e10
--- /dev/null
+++ b/numparse.h
@@ -0,0 +1,207 @@
+#ifndef NUMPARSE_H
+#define NUMPARSE_H
+
+/*
+ * Functions for parsing integral numbers.
+ *
+ * Examples:
+ *
+ * - Convert s into a signed int, interpreting prefix 0x to mean
+ * hexadecimal and 0 to mean octal. If the value doesn't fit in an
+ * unsigned int, set result to INT_MIN or INT_MAX.
Did you mean s/unsigned int/signed int/ ?
+ *
+ * if (convert_i(s, NUM_SLOPPY, result))
+ * die(...);
+ */
+
+/*
+ * The lowest 6 bits of flags hold the numerical base that should be
+ * used to parse the number, 2 = base = 36. If base is set to 0,
+ * then NUM_BASE_SPECIFIER must be set too; in this case, the base is
+ * detected automatically from the string's prefix.
Does this restriction go against the goal of making these functions
convenient, even while remaining strict? Is there a strong reason for
not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would
make it consistent with strto*l() without (I think) introducing any
ambiguity.
+ */
+/*
+ * Number parsing functions:
+ *
+ * The following functions parse a number (long, unsigned long, int,
+ * or unsigned int respectively) from the front of s, storing the
+ * value to *result and storing a pointer to the first character after
+ * the number to *endptr. flags specifies how the number should be
+ * parsed, including which base should be used. flags is a combination
+ * of the numerical base (2-36) and the NUM_* constants above (see).
(see) what?
+ * Return 0 on success or a negative value if there was an error. On
+ * failure, *result and *entptr are left unchanged.
+ *
+ * Please note that if NUM_TRAILING is not set, then it is
+ * nevertheless an error if there are any characters between the end
+ * of the number and the end of the string.
Again, on the subject of convenience, why this restriction? The stated
purpose of the parse_*() functions is to parse the number from the
front of the string and return a pointer to the first non-numeric
character following. As a reader of this API, I interpret that as
meaning that NUM_TRAILING is implied. Is there a strong reason for not
inferring NUM_TRAILING for the parse_*() functions at the API level?
(I realize that the convert_*() functions are built atop parse_*(),
but that's an implementation detail.)
+ */
+
+int parse_l(const char *s, unsigned int flags,
+ long *result, char **endptr);
Do we want to perpetuate the ugly (char **) convention for 'endptr'
from strto*l()? Considering that the incoming string is const, it
seems undesirable to return a non-const pointer to some place inside
that string.
+/*
+ * Number conversion functions:
+ *
+ * The following functions parse a string into a number. They are
+ * identical to the parse_*() functions above, except that the endptr
+ * is not returned. These are most useful when parsing a whole string
+ * into a number; i.e., when (flags NUM_TRAILING) is unset.
I can formulate arguments for allowing or disallowing NUM_TRAILING
with convert_*(), however, given their purpose of parsing the
[PATCH] clone: drop period from end of die_errno message
We do not usually end our errors with a full stop, but it looks especially bad when you use die_errno, which adds a colon, like: fatal: could not create work tree dir 'foo'.: No such file or directory Signed-off-by: Jeff King p...@peff.net --- Not strictly related to the other patch, but I noticed it while playing around. builtin/clone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/clone.c b/builtin/clone.c index 9572467..aa01437 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -848,7 +848,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) die_errno(_(could not create leading directories of '%s'), work_tree); if (!dest_exists mkdir(work_tree, 0777)) - die_errno(_(could not create work tree dir '%s'.), + die_errno(_(could not create work tree dir '%s'), work_tree); set_git_work_tree(work_tree); } -- 2.3.3.520.g3cfbb5d -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
On Wed, Mar 18 2015 at 04:29:44 AM, Sundararajan R dyou...@gmail.com wrote:
Teaching reset the - shorthand involves checking if any file named '-' exists.
check_filename() is used to perform this check.
When the @{-1} branch does not exist then it can be safely assumed that the
user is referring to the file '-',if any. If this file exists then it is
reset or else
a bad flag error is shown.
But if the @{-1} branch exists then it becomes ambiguous without the explicit
'--' disambiguation as to whether the user wants to reset the file '-' or if
he wants to reset the working tree to the previous branch. Hence the program
dies
with a message about the ambiguous argument.
I might be wrong but I think any pathspec that begins with - needs to
be preceded by either a -- marker or be specified as ./-filename,
else verify_filename just die. Therefore you would need to do something
like git reset ./- if you wanted to reset a file. I don't know if given
simply - as filename is desired since options starts with -.
I don't know if you saw but Junio posted a while ago about about
allowing - as a stand-in everywhere a revision was allowed. He updated
a version on pu : d40f108d
On Tue, Mar 17 2015 at 02:49:48 AM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
if (try to see if it is a revision or regvision range) {
/* if failed ... */
if (starts with '-') {
do the option thing;
continue;
}
/* args must be pathspecs from here on */
check the '--' disambiguation;
add pathspec to prune-data;
} else {
got_rev_arg = 1;
}
See $gmane/265672
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] sha1_file: refactor sha1_file.c to support 'cat-file --literally'
On 03/18/2015 01:40 AM, Junio C Hamano wrote:
Karthik Nayak karthik@gmail.com writes:
Subject: Re: [PATCH v4 2/2] sha1_file: refactor sha1_file.c to support
'cat-file --literally'
Modify sha1_loose_object_info() to support 'cat-file --literally'
by accepting flags and also make changes to copy the type to
object_info::typename.
It would be more descriptive to mention the central point on the
title regarding what it takes to support cat-file --literally.
For example:
sha1_file.c: support reading from a loose object of a bogus type
Update sha1_loose_object_info() to optionally allow it to read
from a loose object file of unknown/bogus type; as the function
usually returns the type of the object it read in the form of enum
for known types, add an optional typename field to receive the
name of the type in textual form.
By the way, I think your 1/2 would not even compile unless you have
this change; the patches in these two patch series must be swapped,
no?
Noted. Yes it wouldn't. I thought both would be applied together and
didn't give it much thought, but yeah, I should pay more attention to that.
diff --git a/sha1_file.c b/sha1_file.c
index 69a60ec..e31e9e2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned
char *map, unsigned long ma
return git_inflate(stream, 0);
}
+static int unpack_sha1_header_literally(git_zstream *stream, unsigned char
*map,
+ unsigned long mapsize,
+ struct strbuf *header)
+{
+ unsigned char buffer[32], *cp;
+ unsigned long bufsiz = sizeof(buffer);
+ int status;
+
+ /* Get the data stream */
+ memset(stream, 0, sizeof(*stream));
+ stream-next_in = map;
+ stream-avail_in = mapsize;
+ stream-next_out = buffer;
+ stream-avail_out = bufsiz;
+
+ git_inflate_init(stream);
+
+ do {
+ status = git_inflate(stream, 0);
+ strbuf_add(header, buffer, stream-next_out - buffer);
+ for (cp = buffer; cp stream-next_out; cp++)
+ if (!*cp)
+ /* Found the NUL at the end of the header */
+ return 0;
+ stream-next_out = buffer;
+ stream-avail_out = bufsiz;
+ } while (status == Z_OK);
+ return -1;
+}
+
There is nothing literal about this function.
The only difference between the original and this one is that this
uses a strbuf, instead of a buffer of a fixed size allocated by the
caller, to return the early part of the inflated data.
I wonder if it incurs too much overhead to the existing callers if
you reimplementated unpack_sha1_header() as a thin wrapper around
this function, something like
int unpack_sha1_header(git_zstream *stream,
unsigned char *map, unsigned long mapsize,
void *buffer, unsigned long bufsiz)
{
int status;
struct strbuf header = STRBUF_INIT;
status = unpack_sha1_header_to_strbuf(stream, map, mapsize,
header);
if (bufsiz header.len)
die(object header too long);
memcpy(buffer, header.buf, header.len);
return status;
}
Note that I am *not* suggesting to do this blindly. If there is no
downside from performance point of view, however, the above would be
a way to avoid code duplication.
Another way to avoid code duplication may be to implement
unpack_sha1_header_to_strbuf() in terms of unpack_sha1_header(),
perhaps like this:
static int unpack_sha1_header_to_strbuf(...)
{
unsigned char buffer[32], *cp;
unsigned long bufsiz = sizeof(buffer);
int status = unpack_sha1_header(stream, map, mapsize, buffer,
bufsiz);
if (status)
return status;
do {
strbuf_add(header, buffer, stream-next_out - buffer);
for (cp = buffer; cp stream-next_out; cp++)
if (!*cp)
/* Found the NUL at the end of the
header */
return 0;
stream-next_out = buffer;
stream-avail_out = bufsiz;
} while (status == Z_OK);
return -1;
}
which may be more in line with how we read data from loose objects.
Agreed there is code duplication with unpack_sha1_header() and
unpack_sha1_header_extended(). But I thought there would be a
performance trade off, Any way I could test this?
Also the second suggestion you have given seems to be more practical, As
there is no performance overhead, if called by
RE: Git with Lader logic
On March 17, 2015 7:34 PM, Bharat Suvarna wrote: I am trying to find a way of using version control on PLC programmers like Allen Bradley PLC. I can't find a way of this. Could you please give me an idea if it will work with Plc programs. Which are basically Ladder logic. Many PLC programs either store their project code in XML, L5K or L5X (for example), TXT, CSV, or some other text format or can import and export to text forms. If you have a directory structure that represents your project, and the file formats have reasonable line separators so that diffs can be done easily, git very likely would work out for you. You do not have to have the local .git repository in the same directory as your working area if your tool has issues with that or .gitignore. You may want to use a GUI client to manage your local repository and handle the commit/push/pull/merge/rebase functions as I expect whatever PLC system you are using does not have git built-in. To store binary PLC data natively, which some tools use, I expect that those who are better at git-conjuring than I, could provide guidance on how to automate binary diffs for your tool's particular file format. Cheers, Randall -- Brief whoami: NonStopUNIX developer since approximately UNIX(421664400)/NonStop(2112884442) -- In my real life, I talk too much. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 12:41 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Similarly to the merge doc and code, I personally prefer seeing code and tests in the same patch. In this case, the patch introducing the tests is already quite long and intricate, almost to the point of being a burden to review. Combining the patches would likely push it over the edge. I'd elect to keep them separate. Actually, my preference goes for a first patch that introduces the tests with test_expect_failure for things that are not yet implemented (and you can check that tests do not pass yet before you code), and then the patch introducing the feature doing -test_expect_failure +test_expect_success which documents quite clearly and concisely that you just made the feature work. I also tend to favor adding failure tests which are flipped to success when appropriate, however, as this is an entirely new feature, this approach may be unsuitable (and perhaps overkill). Generally speaking, these new tests don't really make sense without the feature; and, more specifically, due to their nature, several of them will pass even without the feature implemented. As such, the current patch organization seems reasonable. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/2] path: implement common_dir handling in git_path_submodule()
This allows making submodules a linked workdirs.
Same as for .git, but ignores the GIT_COMMON_DIR environment variable,
because it would mean common directory for the parent repository and
does not make sense for submodule.
Also add test for functionality which uses this call.
Signed-off-by: Max Kirillov m...@max630.net
---
cache.h | 1 +
path.c | 24
setup.c | 17 -
t/t7410-submodule-checkout-to.sh | 10 ++
4 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index 670e861..4cd03f0 100644
--- a/cache.h
+++ b/cache.h
@@ -437,6 +437,7 @@ extern char *get_object_directory(void);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
extern int get_common_dir(struct strbuf *sb, const char *gitdir);
extern const char *get_git_namespace(void);
extern const char *strip_namespace(const char *namespaced_ref);
diff --git a/path.c b/path.c
index a5c51a3..78f718f 100644
--- a/path.c
+++ b/path.c
@@ -98,7 +98,7 @@ static const char *common_list[] = {
NULL
};
-static void update_common_dir(struct strbuf *buf, int git_dir_len)
+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char*
common_dir)
{
char *base = buf-buf + git_dir_len;
const char **p;
@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int
git_dir_len)
path++;
is_dir = 1;
}
+
+ if (!common_dir) {
+ common_dir = get_git_common_dir();
+ }
+
if (is_dir dir_prefix(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
+ replace_dir(buf, git_dir_len, common_dir);
return;
}
if (!is_dir !strcmp(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
+ replace_dir(buf, git_dir_len, common_dir);
return;
}
}
@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int
git_dir_len)
else if (git_db_env dir_prefix(base, objects))
replace_dir(buf, git_dir_len + 7, get_object_directory());
else if (git_common_dir_env)
- update_common_dir(buf, git_dir_len);
+ update_common_dir(buf, git_dir_len, NULL);
}
static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
@@ -256,6 +261,8 @@ const char *git_path_submodule(const char *path, const char
*fmt, ...)
{
struct strbuf *buf = get_pathname();
const char *git_dir;
+ struct strbuf git_submodule_common_dir = STRBUF_INIT;
+ struct strbuf git_submodule_dir = STRBUF_INIT;
va_list args;
strbuf_addstr(buf, path);
@@ -269,11 +276,20 @@ const char *git_path_submodule(const char *path, const
char *fmt, ...)
strbuf_addstr(buf, git_dir);
}
strbuf_addch(buf, '/');
+ strbuf_addstr(git_submodule_dir, buf-buf);
va_start(args, fmt);
strbuf_vaddf(buf, fmt, args);
va_end(args);
+
+ if (get_common_dir_noenv(git_submodule_common_dir,
git_submodule_dir.buf)) {
+ update_common_dir(buf, git_submodule_dir.len,
git_submodule_common_dir.buf);
+ }
+
strbuf_cleanup_path(buf);
+
+ strbuf_release(git_submodule_dir);
+ strbuf_release(git_submodule_common_dir);
return buf-buf;
}
diff --git a/setup.c b/setup.c
index fb61860..ffda622 100644
--- a/setup.c
+++ b/setup.c
@@ -226,14 +226,21 @@ void verify_non_filename(const char *prefix, const char
*arg)
int get_common_dir(struct strbuf *sb, const char *gitdir)
{
+ const char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+ if (git_env_common_dir) {
+ strbuf_addstr(sb, git_env_common_dir);
+ return 1;
+ } else {
+ return get_common_dir_noenv(sb, gitdir);
+ }
+}
+
+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
+{
struct strbuf data = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
- const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
int ret = 0;
- if (git_common_dir) {
- strbuf_addstr(sb, git_common_dir);
- return 1;
- }
+
strbuf_addf(path, %s/commondir, gitdir);
if (file_exists(path.buf)) {
if (strbuf_read_file(data, path.buf, 0) = 0)
diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh
index 8f30aed..b43391a 100755
--- a/t/t7410-submodule-checkout-to.sh
+++ b/t/t7410-submodule-checkout-to.sh
@@ -47,4 +47,14
Re:
On Wed, Mar 18, 2015 at 05:17:16PM -0400, Jeff King wrote: [1] The double-CR fix works because we strip a single CR from the end of the line (as a convenience for CRLF systems), and then the remaining CR is syntactically significant. But I am surprised that quoting like: printf 'Icon\r' .gitignore does not seem to work. Answering myself: we don't do quoting like this in .gitignore. We allow backslashing to escape particular characters, like trailing whitespace. So in theory: Icon\\r (where \r is a literal CR) would work. But it doesn't, because the CRLF chomping happens separately, and CR is therefore a special case. I suspect you could not .gitignore a file with a literal LF in it at all (and I equally suspect that nobody cares in practice). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 2:28 PM, Jeff King p...@peff.net wrote: On Wed, Mar 18, 2015 at 05:17:16PM -0400, Jeff King wrote: [1] The double-CR fix works because we strip a single CR from the end of the line (as a convenience for CRLF systems), and then the remaining CR is syntactically significant. But I am surprised that quoting like: printf 'Icon\r' .gitignore does not seem to work. Answering myself: we don't do quoting like this in .gitignore. We allow backslashing to escape particular characters, like trailing whitespace. So in theory: Icon\\r (where \r is a literal CR) would work. But it doesn't, because the CRLF chomping happens separately, and CR is therefore a special case. I suspect you could not .gitignore a file with a literal LF in it at all (and I equally suspect that nobody cares in practice). What does the Icon^M try to catch, exactly? Is it a file? Is it a directory? Is it anything that begins with Icon^M? I am wondering if we need an opposite of '/' prefix in the .gitignore file to say the pattern does not match a directory, only a file. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 2:33 PM, Junio C Hamano gits...@pobox.com wrote: What does the Icon^M try to catch, exactly? Is it a file? Is it a directory? Is it anything that begins with Icon^M? It seems to be a special hidden file on Macs for UI convenience. On Apr 25, 2005, at 6:21 AM, Peter N. Lundblad wrote: The Icon^M file in a directory gives that directory a custom icon in the Finder. They are a holdover from MacOS 9 but there are still a lot of them out there. The new OS X format for icons are .icns files but I'm not sure if you can do custom file directory icons with them (you probably can, I just haven't found the docs yet). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 02:58:15PM +, John Keeping wrote: On Wed, Mar 18, 2015 at 09:41:59PM +0700, Duy Nguyen wrote: On Wed, Mar 18, 2015 at 9:33 PM, Duy Nguyen pclo...@gmail.com wrote: If not, I made some mistake in analyzing this and we'll start again. I did make one mistake, the first gc should have reduced the number of loose objects to zero. Why didn't it.? I'll come back to this tomorrow if nobody finds out first :) Most likely they are not referenced by anything but are younger than 2 weeks. I saw a similar issue with automatic gc triggering after every operation when I did something equivalent to: git add lots of files git commit git reset --hard HEAD^ which creates a log of unreachable objects which are not old enough to be pruned. Yes, this is almost certainly the problem. Though to be pedantic, the command above will still have a reflog entry, so the objects will be reachable (and packed). But there are other variants that don't leave the objects reachable from even reflogs. I don't know if there is an easy way around this. Auto-gc's object count is making the assumption that running the gc will reduce the number of objects, but obviously it does not always do so. We could do a more thorough check and find the number of actual packable and prune-able objects. The prune-able part of that is easy; just omit objects from the count that are newer than 2 weeks. But packable is expensive. You would have to compute reachability by walking from the tips. That can take tens of seconds on a large repo. You could perhaps cut off the walk early when you hit a packed commit (this does not strictly imply that all of the related objects are packed, but it would be good enough for a heuristic). But even that is probably too expensive for gc --auto. -Peff PS Note that in git v2.2.0 and up, prune will leave not only recent unreachable objects, but older objects which are reachable from those recent ones (so that we keep or prune whole chunks of history, rather than dropping part and leaving the rest broken). Technically this exacerbates the problem (we keep more objects), though I doubt it makes much difference in practice (most chunks of history were created at similar times, so the mtimes of the whole chunk will be close together). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git pull git gc
On Wed, Mar 18, 2015 at 03:48:42PM +0100, Дилян Палаузов wrote: #ls .git/objects/17/* | wc -l 30 30 * 256 = 7 680 6 700 And now? Do I have to run git gc --aggressive ? No, aggressive just controls the time we spend on repacking. If the guess is correct that the objects are kept because they are unreachable but recent, then shortening the prune expiration time would get rid of them. E.g., git gc --prune=1.hour.ago. That does not solve the underlying problem discussed elsewhere in the thread, but it would make this particular instance of it go away. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 1:45 PM, Junio C Hamano gits...@pobox.com wrote: On Wed, Mar 18, 2015 at 1:33 PM, Alessandro Zanardi pensierinmus...@gmail.com wrote: Here are other sources describing the issue: http://stackoverflow.com/questions/21109672/git-ignoring-icon-files-because-of-icon-rule http://blog.bitfluent.com/post/173740409/ignoring-icon-in-gitignore Sorry to bother the Git development team with such a minor issue, I just wanted to know if it's already been fixed. I do not ship your ~/.gitignore_global file as part of my software, so the problem is not mine to fix in the first place ;-) Maybe this can be understood as a critique on the .gitignore format specifier for paths. (Maybe not, I dunno) So the `gitignore` script/executable which would generate your .gitignore file for you introduced a bug to also ignore files in Icons/ when all you wanted to have is ignoring the file Icon\r\r (Mind that \r is an escape character to explain the meaning, gitignore cannot understand it. Sometimes it also shows up as ^M^M depending on operating system/editor used.) But as you can see, there have been several attempts at fixing it right and https://github.com/github/gitignore/pull/334 eventually got the right fix. (it was merged 2012, which has been a while now), maybe you'd want to use a new version of this gitignore script to regenerate your gitignore? Where did you get that file from? We need to find whoever is responsible and notify them so that these users who are having the issue will be helped. Given that this is part of https://github.com/github/gitignore which is the official collection of .gitignore files from Github, the company, we could ask Jeff or Michael if it is urgent. The actual fix being merged 3 years ago makes me belief it is not urgent though. Thanks, Stefan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:
On Wed, Mar 18, 2015 at 02:06:22PM -0700, Stefan Beller wrote: Where did you get that file from? We need to find whoever is responsible and notify them so that these users who are having the issue will be helped. Given that this is part of https://github.com/github/gitignore which is the official collection of .gitignore files from Github, the company, we could ask Jeff or Michael if it is urgent. The actual fix being merged 3 years ago makes me belief it is not urgent though. It looks like the fix they have in that repo does the right thing[1], but for reference, you are much more likely to get results by creating an issue or PR on that repository, rather than asking me. -Peff [1] The double-CR fix works because we strip a single CR from the end of the line (as a convenience for CRLF systems), and then the remaining CR is syntactically significant. But I am surprised that quoting like: printf 'Icon\r' .gitignore does not seem to work. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
