[PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules
This allows us to replace the submodule path trailing slash removal in
builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to
parse_pathspec() without changing the behaviour with respect to multiple
trailing slashes.
Signed-off-by: John Keeping j...@keeping.me.uk
---
pathspec.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 7c6963b..11b031a 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -251,12 +251,16 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
item-len = strlen(item-match);
item-prefix = prefixlen;
- if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
- (item-len = 1 is_dir_sep(item-match[item-len - 1]))
- (i = cache_name_pos(item-match, item-len - 1)) = 0
- S_ISGITLINK(active_cache[i]-ce_mode)) {
- item-len--;
- match[item-len] = '\0';
+ if (flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) {
+ size_t pathlen = item-len;
+ while (pathlen 0 is_dir_sep(item-match[pathlen - 1]))
+ pathlen--;
+
+ if ((i = cache_name_pos(item-match, pathlen)) = 0
+ S_ISGITLINK(active_cache[i]-ce_mode)) {
+ item-len = pathlen;
+ match[item-len] = '\0';
+ }
}
if (flags PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
@@ -271,11 +275,14 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
!is_dir_sep(match[ce_len]) ||
memcmp(ce-name, match, ce_len))
continue;
- if (item-len == ce_len + 1) {
- /* strip trailing slash */
+
+ while (item-len 0 is_dir_sep(match[item-len -
1]))
item-len--;
- match[item-len] = '\0';
- } else
+
+ /* strip trailing slash */
+ match[item-len] = '\0';
+
+ if (item-len != ce_len)
die (_(Pathspec '%s' is in submodule '%.*s'),
elt, ce_len, ce-name);
}
--
1.8.4.277.gfbd6843.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
[PATCH v2 1/4] pathspec: use is_dir_sep() to check for trailing slashes
This allows us to correctly removing trailing backslashes on Windows
when checking for submodules.
Signed-off-by: John Keeping j...@keeping.me.uk
---
pathspec.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index ad1a9f5..7c6963b 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -252,7 +252,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
item-prefix = prefixlen;
if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
- (item-len = 1 item-match[item-len - 1] == '/')
+ (item-len = 1 is_dir_sep(item-match[item-len - 1]))
(i = cache_name_pos(item-match, item-len - 1)) = 0
S_ISGITLINK(active_cache[i]-ce_mode)) {
item-len--;
@@ -267,7 +267,8 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
if (!S_ISGITLINK(ce-ce_mode))
continue;
- if (item-len = ce_len || match[ce_len] != '/' ||
+ if (item-len = ce_len ||
+ !is_dir_sep(match[ce_len]) ||
memcmp(ce-name, match, ce_len))
continue;
if (item-len == ce_len + 1) {
--
1.8.4.277.gfbd6843.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
[PATCH v2 3/4] rm: re-use parse_pathspec's trailing-slash removal
Instead of re-implementing the remove trailing slashes loop in
builtin/rm.c just pass PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to
parse_pathspec.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 9b59ab3..3a0e0ea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -298,22 +298,10 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
if (read_cache() 0)
die(_(index file corrupt));
- /*
-* Drop trailing directory separators from directories so we'll find
-* submodules in the index.
-*/
- for (i = 0; i argc; i++) {
- size_t pathlen = strlen(argv[i]);
- if (pathlen is_dir_sep(argv[i][pathlen - 1])
- is_directory(argv[i])) {
- do {
- pathlen--;
- } while (pathlen is_dir_sep(argv[i][pathlen - 1]));
- argv[i] = xmemdupz(argv[i], pathlen);
- }
- }
-
- parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
seen = xcalloc(pathspec.nr, 1);
--
1.8.4.277.gfbd6843.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
Re: [PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules
On Thu, Sep 12, 2013 at 12:48:10PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
This allows us to replace the submodule path trailing slash removal in
builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to
parse_pathspec() without changing the behaviour with respect to multiple
trailing slashes.
Where does prefix_pathspec()'s input, which could have an unwanted
trailing slash, come from?
If it is read from some of our internal data structure and known to
have at most one, then this change makes me feel very uneasy to cope
with potentially sloppy end-user input and data generated by ourselves
with the same logic. It will allow our internal to be sloppy without
forcing us notice and fix that sloppiness.
If it is coming from an end-user input, then I would not object to
the change, though.
I added this in response to Duy's comment on v1 [1].
[1] http://article.gmane.org/gmane.comp.version-control.git/234548
Looking more closely, this does come from user input (via the argv
passed into parse_pathspec) but does (some of the time) go through
prefix_path_gently which calls normalize_path_copy_len.
It's not immediately clear to me when prefix_pathspec goes through this
particular code path, but I think we may be able to drop this (and the
previous patch) without affecting the user.
Signed-off-by: John Keeping j...@keeping.me.uk
---
pathspec.c | 27 +--
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/pathspec.c b/pathspec.c
index 7c6963b..11b031a 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -251,12 +251,16 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
item-len = strlen(item-match);
item-prefix = prefixlen;
- if ((flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP)
- (item-len = 1 is_dir_sep(item-match[item-len - 1]))
- (i = cache_name_pos(item-match, item-len - 1)) = 0
- S_ISGITLINK(active_cache[i]-ce_mode)) {
- item-len--;
- match[item-len] = '\0';
+ if (flags PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) {
+ size_t pathlen = item-len;
+ while (pathlen 0 is_dir_sep(item-match[pathlen - 1]))
+ pathlen--;
+
+ if ((i = cache_name_pos(item-match, pathlen)) = 0
+ S_ISGITLINK(active_cache[i]-ce_mode)) {
+ item-len = pathlen;
+ match[item-len] = '\0';
+ }
}
if (flags PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE)
@@ -271,11 +275,14 @@ static unsigned prefix_pathspec(struct pathspec_item
*item,
!is_dir_sep(match[ce_len]) ||
memcmp(ce-name, match, ce_len))
continue;
- if (item-len == ce_len + 1) {
- /* strip trailing slash */
+
+ while (item-len 0 is_dir_sep(match[item-len -
1]))
item-len--;
- match[item-len] = '\0';
- } else
+
+ /* strip trailing slash */
+ match[item-len] = '\0';
+
+ if (item-len != ce_len)
die (_(Pathspec '%s' is in submodule '%.*s'),
elt, ce_len, ce-name);
}
--
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/2] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 08:05:44AM +0200, Johannes Sixt wrote:
Am 10.09.2013 21:13, schrieb John Keeping:
When using tab-completion, a directory path will often end with a
trailing slash which currently confuses git rm when dealing with
submodules. Now that we have parse_pathspec we can easily handle this
by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/reset.c| 5 +
t/t7400-submodule-basic.sh | 6 --
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5e4c551..9efac0f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
+
+ if (read_cache() 0)
+ die(_(index file corrupt));
When the index is now read here, I would have expected hunk in this
patch that removes a read_cache() invocation.
I think that needs to look like this on top - there's also a
read_cache_unmerged() around line 68 but I don't think we can remove
that one.
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec *pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const
char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
--
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 2/2] rm: re-use parse_pathspec's trailing-slash removal
On Wed, Sep 11, 2013 at 02:48:51PM +0700, Duy Nguyen wrote:
On Wed, Sep 11, 2013 at 2:13 AM, John Keeping j...@keeping.me.uk wrote:
Instead of re-implementing the remove trailing slashes loop in
builtin/rm.c just pass PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to
parse_pathspec.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 9b59ab3..3a0e0ea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -298,22 +298,10 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
if (read_cache() 0)
die(_(index file corrupt));
- /*
-* Drop trailing directory separators from directories so we'll find
-* submodules in the index.
-*/
- for (i = 0; i argc; i++) {
- size_t pathlen = strlen(argv[i]);
- if (pathlen is_dir_sep(argv[i][pathlen - 1])
- is_directory(argv[i])) {
- do {
- pathlen--;
- } while (pathlen is_dir_sep(argv[i][pathlen -
1]));
- argv[i] = xmemdupz(argv[i], pathlen);
- }
- }
-
- parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
I notice that _CHEAP implementation and the removed code are not
exactly the same. But I think they have the same purpose so it's
probably ok even there are some subtle behavioral changes.
Providing that there's only one trailing slash, the user-visible effect
should be the same since the only case affected by that is submodules.
In fact _CHEAP does better in the case where the submodule does not
exist in the working tree.
You may want to improve _CHEAP to remove consecutive trailing slashes
(i.e. foo - foo) too. And maybe is is_dir_sep() instead of
explicit == '/' comparison in there.
Sounds good, I'll try to look at that tonight.
+ prefix, argv);
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
seen = xcalloc(pathspec.nr, 1);
--
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/2] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 05:54:48PM +0700, Duy Nguyen wrote:
On Wed, Sep 11, 2013 at 3:20 PM, John Keeping j...@keeping.me.uk wrote:
On Wed, Sep 11, 2013 at 08:05:44AM +0200, Johannes Sixt wrote:
Am 10.09.2013 21:13, schrieb John Keeping:
When using tab-completion, a directory path will often end with a
trailing slash which currently confuses git rm when dealing with
submodules. Now that we have parse_pathspec we can easily handle this
by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/reset.c| 5 +
t/t7400-submodule-basic.sh | 6 --
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 5e4c551..9efac0f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
+
+ if (read_cache() 0)
+ die(_(index file corrupt));
When the index is now read here, I would have expected hunk in this
patch that removes a read_cache() invocation.
I think that needs to look like this on top - there's also a
read_cache_unmerged() around line 68 but I don't think we can remove
that one.
-- 8 --
diff --git a/builtin/reset.c b/builtin/reset.c
index 9efac0f..800117f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -143,7 +143,6 @@ static int read_from_tree(const struct pathspec
*pathspec,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- read_cache();
if (do_diff_cache(tree_sha1, opt))
return 1;
diffcore_std(opt);
@@ -169,7 +168,7 @@ static void set_reflog_message(struct strbuf *sb, const
char *action,
static void die_if_unmerged_cache(int reset_type)
{
- if (is_merge() || read_cache() 0 || unmerged_cache())
+ if (is_merge() || unmerged_cache())
die(_(Cannot do a %s reset in the middle of a merge.),
_(reset_type_names[reset_type]));
reset --soft does not go through these code paths (i.e. it does not
need index at all). If we fail to load index index in reset --soft I
think it's ok to die(). Corrupt index is fatal anyway. But reset
--soft now has to pay the cost to load index, which could be slow
when the index is big. Assuming nobody does reset --soft that often
I think this is OK.
Alternatively we could load index lazily in _CHEAP code only when we
see trailing slashes, then replace these read_cache() with
read_cache_unless_its_already_loaded_earlier() or something.
read_cache() already has an early return if the index is already loaded
so I don't think we need to worry about a special function for that.
I'm not sure it's worth optimizing this case too heavily, but it might
be a nice change to make parse_pathspec() not rely on the index being
loaded before it is called with certain flags.
--
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/2] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? Duy's suggestion below is necessary to avoid this then I think - we'll die if the user has a corrupt index and gives a path with a trailing slash, but without that path we won't try to load the index. But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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/2] reset: handle submodule with trailing slash
On Wed, Sep 11, 2013 at 11:14:57AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: On Wed, Sep 11, 2013 at 10:08:18AM -0700, Junio C Hamano wrote: Duy Nguyen pclo...@gmail.com writes: reset --soft does not go through these code paths (i.e. it does not need index at all). If we fail to load index index in reset --soft I think it's ok to die(). Corrupt index is fatal anyway. Do I smell a breakage here? Isn't reset --soft HEAD (or some known good commit) a way to recover from a corrupt index? If that is the case, I do not think it is OK at all. What do we suggest as an alternative? rm .git/index read-tree? Duy's suggestion below is necessary to avoid this then I think - we'll die if the user has a corrupt index and gives a path with a trailing slash, but without that path we won't try to load the index. Sorry, but I don't quite follow. Isn't git reset --soft path a nonsense, with or without a trailing slash at the end of path? Yes, but my point was that providing the user doesn't give a path with a trailing slash we will get past the option parser if we take the approach below. However, I think we do do a read_cache when using reset --soft since we go through builtin/reset.c::die_if_unmerged_cache() which dies if read_cache fails. So I don't think we are losing anything by moving this check earlier. But reset --soft now has to pay the cost to load index, which could be slow when the index is big. Assuming nobody does reset --soft that often I think this is OK. Alternatively we could load index lazily in _CHEAP code only when we see trailing slashes, then replace these read_cache() with read_cache_unless_its_already_loaded_earlier() or something. -- 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] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 06:02:35PM -0500, Felipe Contreras wrote: On Mon, Sep 9, 2013 at 3:24 PM, John Keeping j...@keeping.me.uk wrote: On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. And you propose to show that every single time the user does a 'git pull'' that results in a non-fast-forward merge? Isn't that what 'git pull --help' is for? Only if the user has not given an explicit mode (either on the command line or in their config) and possibly if an advice.pullNonFF variable is not set to false. I think that matches what Git does elsewhere. git-pull(1) provides quite a lot more information that I think a new Git user would be comfortable with. There certainly is not a quick way to find out how to fix this error and I don't think it makes sense to add one because we'll still be presenting the user with all of the other content and they won't have any way to know what they can safely ignore and what they have to read and understand. -- 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 1/2] reset: handle submodule with trailing slash
When using tab-completion, a directory path will often end with a trailing slash which currently confuses git rm when dealing with submodules. Now that we have parse_pathspec we can easily handle this by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag. Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/reset.c| 5 + t/t7400-submodule-basic.sh | 6 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 5e4c551..9efac0f 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec, } } *rev_ret = rev; + + if (read_cache() 0) + die(_(index file corrupt)); + parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL | + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP | (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0), prefix, argv); } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 4192fe0..c268d3c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -481,7 +481,7 @@ test_expect_success 'do not add files from a submodule' ' ' -test_expect_success 'gracefully add submodule with a trailing slash' ' +test_expect_success 'gracefully add/reset submodule with a trailing slash' ' git reset --hard git commit -m commit subproject init @@ -495,7 +495,9 @@ test_expect_success 'gracefully add submodule with a trailing slash' ' git add init/ test_must_fail git diff --exit-code --cached init test $commit = $(git ls-files --stage | - sed -n s/^16 \([^ ]*\).*/\1/p) + sed -n s/^16 \([^ ]*\).*/\1/p) + git reset init/ + git diff --exit-code --cached init ' -- 1.8.2 -- 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 2/2] rm: re-use parse_pathspec's trailing-slash removal
Instead of re-implementing the remove trailing slashes loop in
builtin/rm.c just pass PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP to
parse_pathspec.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 9b59ab3..3a0e0ea 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -298,22 +298,10 @@ int cmd_rm(int argc, const char **argv, const char
*prefix)
if (read_cache() 0)
die(_(index file corrupt));
- /*
-* Drop trailing directory separators from directories so we'll find
-* submodules in the index.
-*/
- for (i = 0; i argc; i++) {
- size_t pathlen = strlen(argv[i]);
- if (pathlen is_dir_sep(argv[i][pathlen - 1])
- is_directory(argv[i])) {
- do {
- pathlen--;
- } while (pathlen is_dir_sep(argv[i][pathlen - 1]));
- argv[i] = xmemdupz(argv[i], pathlen);
- }
- }
-
- parse_pathspec(pathspec, 0, PATHSPEC_PREFER_CWD, prefix, argv);
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_CWD |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
refresh_index(the_index, REFRESH_QUIET, pathspec, NULL, NULL);
seen = xcalloc(pathspec.nr, 1);
--
1.8.2
--
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 0/2] reset: handle submodule with trailing slash
The first patch is the important one here, the second one I noticed while checking if any other commands fail to handle submodule paths with a trailing slash and is just a simplification. John Keeping (2): reset: handle submodule with trailing slash rm: re-use parse_pathspec's trailing-slash removal builtin/reset.c| 5 + builtin/rm.c | 20 t/t7400-submodule-basic.sh | 6 -- 3 files changed, 13 insertions(+), 18 deletions(-) -- 1.8.2 -- 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/2] reset: handle submodule with trailing slash
On Tue, Sep 10, 2013 at 09:37:45PM +0200, Jens Lehmann wrote: Am 10.09.2013 21:13, schrieb John Keeping: When using tab-completion, a directory path will often end with a trailing slash which currently confuses git rm when dealing with I think you meant to say git reset in the line above. Apart from that I'm all for it. Yeah, you're right - I obviously got confused between the two patches :-(. I'll wait for more feedback before submitting a re-roll. submodules. Now that we have parse_pathspec we can easily handle this by simply adding the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag. Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/reset.c| 5 + t/t7400-submodule-basic.sh | 6 -- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 5e4c551..9efac0f 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -220,8 +220,13 @@ static void parse_args(struct pathspec *pathspec, } } *rev_ret = rev; + + if (read_cache() 0) + die(_(index file corrupt)); + parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL | + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP | (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0), prefix, argv); } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 4192fe0..c268d3c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -481,7 +481,7 @@ test_expect_success 'do not add files from a submodule' ' ' -test_expect_success 'gracefully add submodule with a trailing slash' ' +test_expect_success 'gracefully add/reset submodule with a trailing slash' ' git reset --hard git commit -m commit subproject init @@ -495,7 +495,9 @@ test_expect_success 'gracefully add submodule with a trailing slash' ' git add init/ test_must_fail git diff --exit-code --cached init test $commit = $(git ls-files --stage | - sed -n s/^16 \([^ ]*\).*/\1/p) + sed -n s/^16 \([^ ]*\).*/\1/p) + git reset init/ + git diff --exit-code --cached init ' -- 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] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. ? -- 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] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 04:44:16PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote: I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Yes, that's a good point. I don't know if just git rebase is the right advice, though; it would depend on whether we were actually pulling from the upstream or not. I wonder if we have sufficient information at the time of the warning to print out the actual git rebase invocation that would rebase as if they had run pull --rebase. I think we may have to do a little refactoring around the base selection from the reflog (IIRC, git-pull does not even calculate it at all if you are not using --rebase). We can probably do something like: opts= if git merge-base --is-ancestor $orig_head $merge_head then opts=$merge_head else opts=$orig_head --onto $merge_head fi so that git rebase $opts is the right thing. Most users then get the simple git rebase $merge_head variant. It is also depending on git rebase throwing away the merge commit we just created. Which I think should happen always if you have not configured anything (though perhaps we will eventually support a pull mode that does rebase -p, you would not see this warning with that option anyway). But another option would be to simply tell them: git reset --keep HEAD^ git pull --rebase [X...] where [X...] is the arguments they gave to rebase in the first place. That looks a little less friendly, though. Yeah, I think we should keep it simple if possible. In my experience people are relatively happy to run a single make things right command but less so if there's a sequence of steps to be performed. -- 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] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 02:54:20AM -0400, Jeff King wrote:
I am genuinely curious what people in favor of this feature would want
to say in the documentation to a user encountering this choice for the
first time. In my experience, rebasing introduces more complications,
specifically:
1. the merge is backwards with respect to ours/theirs
2. you may end up with difficult conflict resolution due to repeated
changes over the same section of code. E.g., you write some buggy
code and then fix it, but upstream has changed the same area.
Rebasing involves first resolving your buggy version with the
upstream code, and then resolving the fix on top of the previous
resolution.
3. rewriting of commits found in other branches, which then need
rebased on top of the branch you just rebased
4. a previously bug-free commit can show a bug after the rebase if
other parts of the project changed (whereas with a merge, the bug
would be attributable to the merge)
I know those are all balanced by some advantages of rebasing, but I also
think they are things that can be troublesome for a user who does not
fully grok the rebase process. I'm just wondering if we should mention
both, but steer people towards merging as the safer alternative (you
might have ugly history, but you are less likely to create a mess with
duplicate commits or badly-resolved conflicts).
The really correct thing to do here is to encourage a feature branch
workflow, but in my experience people are happier to walk through a
rebase than to switch over to feature branches completely.
An alternative pull mode would be:
git reset --keep @{u}
git merge @{-1}
which gets a sensible history shape without any of your disadvantages
above. But that didn't go anywhere last time it came up [1] [2].
[1] http://article.gmane.org/gmane.comp.version-control.git/210246
[2] http://article.gmane.org/gmane.comp.version-control.git/210625
Fortunately there probably are very few of these users.
Maybe. I am not sure how one would measure.
If you are interested, I can ask the opinion of some of the GitHub
trainers. They see a lot of new users and have a sense of what kinds of
confusion come up most frequently, what kinds of workflows they tend to
see, etc. Their experience may be biased towards corporate-ish users,
though, because those are the people who pay for training.
I expect corporate environments are the ones in which this is relevant.
Open source projects that care about the shape of history can have one
person able to write to the central repository who can enforce the
policy they want. This tends to be more difficult in a corporate
environment, particularly one that was previously using a centralised
VCS.
--
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] Reject non-ff pulls by default
On Fri, Sep 06, 2013 at 03:14:25PM -0700, Junio C Hamano wrote:
Jonathan Nieder jrnie...@gmail.com writes:
John Keeping wrote:
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
upstream parameter to rebase.
You're right.
It makes me wonder why we did that one inside pull and not in
rebase, though.
I'd never realised pull --rebase does that - it's exactly what I want
sometimes and I normally do fetch followed by rebase to get more control
over the process.
Perhaps we should do something like this (with added tests and
documentation)?
-- 8 --
Subject: [PATCH] rebase: use reflog to find common base with upstream
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-rebase.sh | 8
1 file changed, 8 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 8d7659a..5e3013d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -428,6 +428,14 @@ then
error_on_missing_default_upstream rebase rebase \
against git rebase branch
fi
+ for reflog in $(git rev-list -g $upstream_name 2/dev/null)
+ do
+ if test $reflog = $(git merge-base $reflog HEAD)
+ then
+ upstream_name=$reflog
+ break
+ fi
+ done
;;
*) upstream_name=$1
shift
--
1.8.4.239.g2332621
--
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] Reject non-ff pulls by default
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this may be
better off fetching and then updating their local branch, particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
Side note: a knee-jerk response to a yes answer to the
last question from me has always been then why are you
running 'git pull' in the first place. The next paragraph is
my attempt to extend my imagination a bit, stepping outside
that reaction.
--
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] Reject non-ff pulls by default
On Thu, Sep 05, 2013 at 07:01:03AM -0400, John Szakmeister wrote:
On Wed, Sep 4, 2013 at 6:59 PM, Junio C Hamano gits...@pobox.com wrote:
[snip]
When git pull stops because what was fetched in FETCH_HEAD does
not fast-forward, then what did _you_ do (and with the knowledge you
currently have, what would you do)? In a single project, would you
choose to sometimes rebase and sometimes merge, and if so, what is
the choice depend on? When I am on these selected branches, I want
to merge, but on other branches I want to rebase?
Our team isn't quite proficient enough yet to have a completely rebase
workflow... though we might have less of a problem if we did. So,
several interesting points. Most of the time, `git pull` would be a
fast-forward merge. We typically perform the merges of topic branches
server-side--we have a build server who checks to make sure the result
would be successful--and we just hit the big green button on the Merge
button for the pull request (we use GitHub Enterprise at the moment).
However, nearly as often, we just merge the branch locally because
someone on the team is doing some manual testing, and it's just
convenient to finish the process on the command line. What
occasionally happens is that you merge the topic locally, but someone
else has introduced a new commit to master. We try to preserve the
mainline ordering of commits, so `git pull` doing a merge underneath
the hood is undesirable (it moves the newly introduced commit off to
the side). Rebasing your current master branch is not the answer
either, because it picks up the commits introduced by the topic branch
and rebases those to--at least with the -p option, and without it, the
results are just as bad). Instead, we want to unfold our work,
fast-forward merge the upstream, and the replay our actions--namely
remerge the topic branch. It often ends up translating to this:
$ git reset --hard HEAD~1
$ git merge --ff-only @{u}
$ git merge topic
$ git push
So what I really want isn't quite rebase. I'm not sure any of the
proposed solutions would work. It'd be really nice to replay only the
mainline commits, without affecting commits introduced from a topic
branch.
Does git rebase --preserve-merges do what you want here?
--
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] Reject non-ff pulls by default
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Sure, that would make sense.
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
--
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: Transfer notes when rebasing
On Wed, Sep 04, 2013 at 03:53:10AM -0400, Jeff King wrote: On Wed, Sep 04, 2013 at 09:51:26AM +0200, Francis Moreau wrote: When rebasing a branch which contains commits with notes onto another branch it happens that some commits are already presents in the target branch. In that case git-rebase correctly drops those (already present) commits but it also drops the notes associated with them. Can the notes be transfered somehow in the target branch on the already present commits ? Yes, see the notes.rewriteRef config option to enable this. Does that actually work for this case? It sounds like Francis has the notes copying correctly when commits are rewritten but the notes are not copied anywhere if the commit becomes empty. I suspect it is difficult to do that in general as there is no clear way to know which commit those notes should be copied to. -- 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] Reject non-ff pulls by default
On Tue, Sep 03, 2013 at 03:38:58PM -0700, Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. We know what you said earlier. That does not make it right or wrong, but I do not think it is in line with the original discussion (that is why John Keeping is kept on the Cc: line). I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. But for (2) a merge is almost always the correct thing to do (in fact it may even be correct to create a merge commit even when this fast forwards) because this most likely comes for a pull request workflow. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. In the series currently in next, we treat this as (2) above but that's primarily because it is difficult to differentiate these in git-pull.sh without adding code to understand all of the options to git-fetch (or at least those that can accept unstuck arguments). Changing this so that git pull $remote is treated as (1) would be better, but I think it is more important to avoid catching case (1) in the same net which is why jc/pull-training-wheel simply checks if $# is zero; the cost of getting this completely right outweighed the benefit of getting code in that will catch 99% of users. -- 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] Reject non-ff pulls by default
On Wed, Sep 04, 2013 at 05:25:27AM -0400, Jeff King wrote: On Wed, Sep 04, 2013 at 09:10:47AM +0100, John Keeping wrote: I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. Is it always wrong? You are assuming a topic-branch workflow where --first-parent is actually meaningful. What about a centralized workflow where everyone works on master? The correct thing to do on a non-ff push in that case is git pull git push. Some people would argue that the pull should rebase there, but I think there are valid arguments either way. We can discuss in that direction if you want. I'm one of the people who argues that it should rebase there ;-) The point of jc/pull-training-wheel is to help users think about that. I can perhaps buy the argument that it is better to help people who are using a topic branch workflow (which we generally want to encourage) to avoid making backwards merges, and the cost is that people with sloppy workflows will have to do more work / configuration. But we should be clear that this is a tradeoff we are making. The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. The scenario I have is a central repository where some developers use a topic branch workflow but others are less familiar with Git and don't really think about what they're doing. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. Imagine a workflow where each topic is in its own repository instead of in its own branch inside a repository. Or where each developer has his or her own repository, but everybody just works on the master branch of their repository (or perhaps uses branches, but keeps master as a stable base). Alice is the integration manager; Bob tells her that he has work ready to integrate. She runs git pull ~bob/project, which will merge Bob's HEAD. This is not very different from the kernel workflow, where Linus may do a git pull $remote to fetch a sub-system maintainer's work, except that these days people typically mark the to-be-integrated work in a for-linus branch or tag. However, you can find many Merge git:// entries even in recent kernel history. I think this kind of pull would fall into the same situation as your (2) above. OK - so I was missing this. Given this, the jc/pull-training-wheel series is doing the right thing here. -- 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/4] Re: [PATCH 3/4] t: rev-parse-parents: avoid yoda conditions
On Wed, Sep 04, 2013 at 09:47:12AM -0700, Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: test_cmp_rev follows the same order of arguments a diff -u and produces the same output as plain git diff. It's perfectly readable and normal. This is way off tangent, but I am somewhat sympathetic to Felipe's compare actual with expect, with reservations. This isn't an argument either way, but note that JUnit (and NUnit and PHPUnit) all have assertEquals methods that take the arguments in the order expect, actual. I've always assumed that Git's test framework was imitating that, although I have no idea why that particular order is chosen and is so common. -- 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] xread(): Fix read error when filtering = 2GB on Mac OS X
On Sat, Aug 17, 2013 at 02:40:05PM +0200, Steffen Prohaska wrote:
Previously, filtering more than 2GB through an external filter (see
test) failed on Mac OS X 10.8.4 (12E55) with:
error: read from external filter cat failed
error: cannot feed the input to external filter cat
error: cat died of signal 13
error: external filter cat failed 141
error: external filter cat failed
The reason is that read() immediately returns with EINVAL if len = 2GB.
I haven't found any information under which specific conditions this
occurs. My suspicion is that it happens when reading from a pipe, while
reading from a standard file should always be fine. I haven't tested
any other version of Mac OS X, though I'd expect that other versions are
affected as well.
The problem is fixed by always reading less than 2GB in xread().
xread() doesn't guarantee to read all the requested data at once, and
callers are expected to gracefully handle partial reads. Slicing large
reads into 2GB pieces should not hurt practical performance.
Signed-off-by: Steffen Prohaska proha...@zib.de
---
t/t0021-conversion.sh | 9 +
wrapper.c | 8
2 files changed, 17 insertions(+)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index e50f0f7..aec1253 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -190,4 +190,13 @@ test_expect_success 'required filter clean failure' '
test_must_fail git add test.fc
'
+test_expect_success 'filter large file' '
+ git config filter.largefile.smudge cat
+ git config filter.largefile.clean cat
+ dd if=/dev/zero of=2GB count=2097152 bs=1024
+ echo /2GB filter=largefile .gitattributes
+ git add 2GB 2err
+ ! grep -q error err
+'
+
test_done
diff --git a/wrapper.c b/wrapper.c
index 6a015de..2a2f496 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -139,6 +139,14 @@ ssize_t xread(int fd, void *buf, size_t len)
{
ssize_t nr;
while (1) {
+#ifdef __APPLE__
+ const size_t twoGB = (1l 31);
+ /* len = 2GB immediately fails on Mac OS X with EINVAL when
+ * reading from pipe. */
+ if (len = twoGB) {
+ len = twoGB - 1;
+ }
Please don't use unnecessary curly braces here (see
Documentation/CodingGuidelines).
+#endif
nr = read(fd, buf, len);
if ((nr 0) (errno == EAGAIN || errno == EINTR))
continue;
--
1.8.4.rc3.5.gfcb973a
--
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: Proper URI for svn clone on a network share (Win32)
On Wed, Aug 14, 2013 at 06:26:57PM -0500, Tim Chase wrote: On 2013-08-14 12:49, Tim Chase wrote: If it makes any difference, this is within a cmd.exe shell (with $PATH set appropriately so git is being found). Just a follow-up, I tried it within the bashish shell included in the git install and got the same error regarding /tmp/report.tmp. It seems that report.tmp is something that SVN creates and for some reason the svn on your system is trying to create it in a Unix style temporary directory. What happens if you export TMPDIR=C:/Windows/Temp before running git-svn? -- 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: Proper URI for svn clone on a network share (Win32)
On Thu, Aug 15, 2013 at 06:12:29AM -0500, Tim Chase wrote: On 2013-08-15 09:00, John Keeping wrote: On Wed, Aug 14, 2013 at 06:26:57PM -0500, Tim Chase wrote: On 2013-08-14 12:49, Tim Chase wrote: If it makes any difference, this is within a cmd.exe shell (with $PATH set appropriately so git is being found). Just a follow-up, I tried it within the bashish shell included in the git install and got the same error regarding /tmp/report.tmp. It seems that report.tmp is something that SVN creates and for some reason the svn on your system is trying to create it in a Unix style temporary directory. What happens if you export TMPDIR=C:/Windows/Temp before running git-svn? Still getting the same results. I tried: 1) cmd.exe with my local temp dir: c:\temp TEMPDIR=%TEMP% This should be TMPDIR - note the missing 'E'! You may also need to export TMPDIR but I don't know how cmd.exe decides what environment variables to export to subprocesses. c:\temp git svn clone file:///x:/path/to/repo/trunk/utils/project1 2) cmd.exe with the windows temp dir as you specify: c:\temp TEMPDIR=c:\windows\temp c:\temp git svn clone file:///x:/path/to/repo/trunk/utils/project1 3) git's bash.exe with inline variable definition: $ TEMPDIR=c:/Windows/Temp git svn clone file:///x:/path/to/repo/trunk/utils/project1 4) git's bash.exe with exported variable: $ export TEMPDIR=c:/Windows/Temp $ git svn clone file:///x:/path/to/repo/trunk/utils/project1 All of them died with the complaint about /tmp/report.tmp Thanks for the suggestion though. At least we've determined one thing that *isn't* the issue ;-) -- 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] whatchanged: document its historical nature
On Fri, Aug 09, 2013 at 01:57:19PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: +New users are encouraged to use linkgit:git-log[1] instead. The +`whatchanged` command is essentially the same as linkgit:git-log[1] +run with different defaults that shows a --raw diff outputat the s/outputat/output at/ Thanks. Although I wonder if it would be better to say New users are encouraged to use linkgit:git-log[1] instead. The `whatchanged` command is essentially the same as linkgit:git-log[1] with the `--raw` option specified. But that is different from the truth, no? git whatchanged -p will not behave as if git whatchanged -p with the --raw specified. The 'raw' kicks in only as a default. Hmm, I hadn't realised that specifying -p would disable the --raw. I still find the last sentence of the original patch slightly awkward though. How about New users are encouraged to use linkgit:git-log[1] instead. The `whatchanged` command is essentially the same as linkgit:git-log[1] but defaults to ``raw'' diff output and does not show merge commits. ? -- 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] whatchanged: document its historical nature
On Fri, Aug 09, 2013 at 01:01:48PM -0700, Junio C Hamano wrote: After doing a bit of archaeology, I now know why whatchanged with an unwieldy long name persisted in the user's mindset for so long. My conclusions are: - It is better to encourage new users to use `log` very early in the document; - It is not sensible to remove the command at this point yet. After having used to `log` that does not take diff options for close to a year, it is understandable why there are many people who are used to type `whatchanged`. It could be argued that deprecation and retraining of fingers are doing favors to the long-time users. But the presense of the command is not hurting anybody, other than the new people who may stumble upon both and wonder what their differences are. By clearly indicating that these two are essentially the same, we would help the new people without harming anybody. -- 8 -- Subject: [PATCH] whatchanged: document its historical nature Encourage new users to use 'log' instead. These days, these commands are unified and just have different defaults. 'git log' only allowed you to view the log messages and no diffs when it was added in early June 2005. It was only in early April 2006 that the command learned to take diff options. Because of this, power users tended to use 'whatchanged' that already existed since mid May 2005 and supported diff options. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-whatchanged.txt | 41 --- 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/Documentation/git-whatchanged.txt b/Documentation/git-whatchanged.txt index c600b61..6faa200 100644 --- a/Documentation/git-whatchanged.txt +++ b/Documentation/git-whatchanged.txt @@ -13,43 +13,18 @@ SYNOPSIS DESCRIPTION --- -Shows commit logs and diff output each commit introduces. The -command internally invokes 'git rev-list' piped to -'git diff-tree', and takes command line options for both of -these commands. -This manual page describes only the most frequently used options. +Shows commit logs and diff output each commit introduces. +New users are encouraged to use linkgit:git-log[1] instead. The +`whatchanged` command is essentially the same as linkgit:git-log[1] +run with different defaults that shows a --raw diff outputat the s/outputat/output at/ Although I wonder if it would be better to say New users are encouraged to use linkgit:git-log[1] instead. The `whatchanged` command is essentially the same as linkgit:git-log[1] with the `--raw` option specified. +end. -OPTIONS --p:: - Show textual diffs, instead of the Git internal diff - output format that is useful only to tell the changed - paths and their nature of changes. +The command is kept primarily for historical reasons; fingers of +many people who learned Git long before `git log` was invented by +reading Linux kernel mailing list are trained to type it. --n:: - Limit output to n commits. - -since..until:: - Limit output to between the two named commits (bottom - exclusive, top inclusive). - --r:: - Show Git internal diff output, but for the whole tree, - not just the top level. - --m:: - By default, differences for merge commits are not shown. - With this flag, show differences to that commit from all - of its parents. -+ -However, it is not very useful in general, although it -*is* useful on a file-by-file basis. - -include::pretty-options.txt[] - -include::pretty-formats.txt[] Examples -- 1.8.4-rc2-195-gb76a8e9 -- 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: Remove old forgotten command: whatchanged
On Thu, Aug 08, 2013 at 08:06:09PM +0200, Matthieu Moy wrote: --- a/Documentation/gitcore-tutorial.txt +++ b/Documentation/gitcore-tutorial.txt @@ -532,12 +532,7 @@ commit, and you can tell it to show a whole series of diffs. Alternatively, you can tell it to be silent, and not show the diffs at all, but just show the actual commit message. -In fact, together with the 'git rev-list' program (which generates a -list of revisions), 'git diff-tree' ends up being a veritable fount of -changes. A trivial (but very useful) script called 'git whatchanged' is -included with Git which does exactly this, and shows a log of recent -activities. - +'git log' can also be used to display changes introduced by some commits. To see the whole history of our pitiful little git-tutorial project, you can do @@ -550,7 +545,7 @@ with the associated patches use the more complex (and much more powerful) Isn't this paragraph slightly misleading now? We're not using a different command any more so this could say: which shows just the log messages, or if we want to see the log together with the associated patches use the `--patch` option -$ git whatchanged -p +$ git log --raw -p Then this can be git log --patch, since it seems that the surrounding text doesn't actually care that whatchanged prints the raw diffstat. -- 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: Remove old forgotten command: whatchanged
On Wed, Aug 07, 2013 at 11:01:57AM -0700, Kyle J. McKay wrote:
On Aug 7, 2013, at 09:00, Ramkumar Ramachandra wrote:
Hi,
This is the difference between whatchanged and log:
diff --git a/whatchanged b/log
index fa1b223..004d9aa 100644
--- a/tmp/whatchanged
+++ b/tmp/log
@@ -1,4 +1,4 @@
-int cmd_whatchanged(int argc, const char **argv, const char *prefix)
+int cmd_log(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
struct setup_revision_opt opt;
@@ -7,13 +7,10 @@ int cmd_whatchanged(int argc, const char **argv,
const char *prefix)
git_config(git_log_config, NULL);
init_revisions(rev, prefix);
- rev.diff = 1;
- rev.simplify_history = 0;
+ rev.always_show_header = 1;
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
opt.revarg_opt = REVARG_COMMITTISH;
cmd_log_init(argc, argv, prefix, rev, opt);
- if (!rev.diffopt.output_format)
- rev.diffopt.output_format = DIFF_FORMAT_RAW;
return cmd_log_walk(rev);
}
Should we remove it?
I use it all the time. Is there some log option to get exactly the
same output? It doesn't appear that there is. The closest looks like
log --name-status but it omits the modes and hash values.
Is it not identical to log --raw --no-merges?
A quick test says mostly, but whatchanged doesn't show empty commits
whereas log does seem to; e.g. in git.git:
diff -u (git whatchanged) (git log --raw --no-merges)
--
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: Fwd: Bug: SEGV in git when applying the patches
On Mon, Aug 05, 2013 at 12:01:44PM +0100, Rafal W. wrote: Hi, When applying patch via git, it is doing sometimes SEGV. Please find more details in the attached crash logs. This looks like the issue fixed by commit 212eb96 (apply: carefully strdup a possibly-NULL name, 2013-06-21), which is in Git 1.8.3.3 and later. -- 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: Making a patch: git format-patch does not produce the documented format
On Wed, Jul 31, 2013 at 05:31:47PM -0400, Dale R. Worley wrote: Notice that the whole commit message has been formatted as if it is part of the Subject line, and the line breaks in the commit message have been refilled. The file Documentation/SubmittingPatches says that git format-patch produces patches in the best format, but the manual page shows an example more like this: From 8f72bad1baf19a53459661343e21d6491c3908d3 Mon Sep 17 00:00:00 2001 From: Tony Luck tony.l...@intel.com Date: Tue, 13 Jul 2010 11:42:54 -0700 Subject: [PATCH] Put ia64 config files on the MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit arch/arm config files were slimmed down using a python script (See commit c2330e286f68f1c408b4aa6515ba49d57f05beae comment) [...] That is, the first line of the commit message is in the Subject and the remaining lines are in the message body. As far as I can tell, that's what SubmittingPatches prescribes. And that is what I see in the Git mailing list on vger. (This is with git 1.8.3.3.) Exactly how should the commit message be inserted into the patch e-mail? What needs to be updated so the code is consistent with the documentation? git-format-patch(1) says: By default, the subject of a single patch is [PATCH] followed by the concatenation of lines from the commit message up to the first blank line (see the DISCUSSION section of git-commit(1)). I think that accurately describes what you're seeing. The referenced DISCUSSION section describes how to write a commit message that is formatted in a suitable way, with a short first subject line and then a blank line before the body of the message. -- 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
ANNOUNCE: git-integration -- Easily manage integration branches
I wrote this script a few months ago and have been using it pretty much daily since then, so I figure it's time to see if anyone else finds it useful... git-integration [1] is a script to help manage integration branches in Git. By defining a base point and a set of branches to be merged to form the integration branch, git-integration lets you easily rebuild an integration branch when anything in it changes, as well as showing you the status of all of the branches in the integration branch. For example, the instruction sheet for git-integration's pu branch recently looked like this: base master merge make-clean Add a clean target to the makefile. merge version Support for --version option. N.B. this builds on make-clean. merge skip-option Needs more work to be able to handle branch not found. This tells git-integration to base the pu branch on master and merge the make-clean, version and skip-option branches in. The comments following the merge instructions are added to the commit message for the corresponding merge commit. When I want to rebuild the pu branch I simply do: $ git integration --rebuild pu To change the contents of the branch, I either edit the instruction sheet manually: $ git integration --edit pu or quickly add a new branch from the command line: $ git integration --add my-new-branch pu In fact, I can combine these to get the benefit of bash-completion on the branch name and the ability to edit the instruction sheet - when multiple commands are specified, git-integration performs each of them in a sensible order, described in the manpage [2]. [1] http://johnkeeping.github.io/git-integration/ [2] http://johnkeeping.github.io/git-integration/git-integration.html -- 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: ANNOUNCE: git-integration -- Easily manage integration branches
On Tue, Jul 30, 2013 at 09:45:49AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I wrote this script a few months ago and have been using it pretty much daily since then, so I figure it's time to see if anyone else finds it useful... git-integration [1] is a script to help manage integration branches in Git. By defining a base point and a set of branches to be merged to form the integration branch, git-integration lets you easily rebuild an integration branch when anything in it changes, as well as showing you the status of all of the branches in the integration branch. For example, the instruction sheet for git-integration's pu branch recently looked like this: base master merge make-clean Add a clean target to the makefile. merge version Support for --version option. N.B. this builds on make-clean. merge skip-option Needs more work to be able to handle branch not found. This tells git-integration to base the pu branch on master and merge the make-clean, version and skip-option branches in. The comments following the merge instructions are added to the commit message for the corresponding merge commit. When I want to rebuild the pu branch I simply do: $ git integration --rebuild pu To change the contents of the branch, I either edit the instruction sheet manually: $ git integration --edit pu or quickly add a new branch from the command line: $ git integration --add my-new-branch pu In fact, I can combine these to get the benefit of bash-completion on the branch name and the ability to edit the instruction sheet - when multiple commands are specified, git-integration performs each of them in a sensible order, described in the manpage [2]. [1] http://johnkeeping.github.io/git-integration/ [2] http://johnkeeping.github.io/git-integration/git-integration.html Interesting. Would it help me to replay evil merges I previously made and avoid necessity to write merge log messages repeatedly? Currently it does nothing beyond having the ability to continue automatically if rerere manages to resolve all conflicts (disabled by default). There is no equivalent of your refs/merge-fix/ feature, although I think I might add one soon ;-). Since the commit messages for the merge commits come from the instruction sheet, it does avoid the need to write them repeatedly - if you want to change the merge message you can simply update the instruction sheet and rebuild. In short, can I replace my Meta/Reintegrate and Meta/cook with this (see Documentation/howto/maintain-git.txt)? It performs the same basic function as those scripts, but it's quite a lot simpler and hasn't been designed for the git.git workflow, so I don't think it's suitable for replacing your existing scripts. If I were starting from scratch and attempting to implement the git.git workflow on top of git-integration, I think I would make whats-cooking.txt a build artifact generated from the instruction sheet for pu. This would require some new commands to be added to git-integration's instruction sheet to let it assign sections to branches, but ought to be possible. I expect there would be some subtleties though - certainly git-integration's --status output does not handle all of the cases the Meta/cook does, not least because it only compares against a single base branch. -- 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: getting git from kernel.org is failing
On Tue, Jul 23, 2013 at 01:40:05PM -0700, Jonathan Nieder wrote: Jeff King wrote: then smart HTTP works fine. I wonder if there is a problem in the cgit setup on kernel.org (or if it was even intended that you could fetch from the cgit URL at all; the cgit page shows the clone URLs in /pub/scm/git). I didn't think cgit URLs were meant to be clonable, but since ls-remote works on them, it seems I thought wrong. :) Odd. CGit has a config option enable-http-clone which causes it to act as an endpoint for the dumb HTTP protocol. That option defaults to on, so I suspect that kernel.org has just left it at the default. -- 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: What's cooking in git.git (Jul 2013, #07; Sun, 21)
On Sun, Jul 21, 2013 at 11:57:43PM -0700, Junio C Hamano wrote: * jk/fast-import-empty-ls (2013-06-23) 4 commits - fast-import: allow moving the root tree - fast-import: allow ls or filecopy of the root tree - fast-import: set valid mode on root tree in ls - t9300: document fast-import empty path issues Comments? This originated with a user bug report [1] so I'd like to see it merged. Any chance of some fast-import experts taking a look? [1] http://article.gmane.org/gmane.comp.version-control.git/228586 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] rev-parse: remove restrictions on some options
The --local-env-vars and --resolve-git-dir arguments to
git-rev-parse are currently only handled if they appear first on the
command line (in the case of --local-env-vars, only if it is the only
argument). While it may not make sense to use these options when any
others are specified, there is no reason for this restriction and it
might confuse users if these arguments appear to be ignored.
There is no need for any documentation change here as the restrictions
on these options are not documented.
Signed-off-by: John Keeping j...@keeping.me.uk
---
builtin/rev-parse.c | 28 +---
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..c9aa28f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -486,21 +486,6 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
if (argc 1 !strcmp(--sq-quote, argv[1]))
return cmd_sq_quote(argc - 2, argv + 2);
- if (argc == 2 !strcmp(--local-env-vars, argv[1])) {
- int i;
- for (i = 0; local_repo_env[i]; i++)
- printf(%s\n, local_repo_env[i]);
- return 0;
- }
-
- if (argc 2 !strcmp(argv[1], --resolve-git-dir)) {
- const char *gitdir = resolve_gitdir(argv[2]);
- if (!gitdir)
- die(not a gitdir '%s', argv[2]);
- puts(gitdir);
- return 0;
- }
-
if (argc 1 !strcmp(-h, argv[1]))
usage(builtin_rev_parse_usage);
@@ -661,6 +646,12 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
for_each_remote_ref(show_reference, NULL);
continue;
}
+ if (!strcmp(arg, --local-env-vars)) {
+ int i;
+ for (i = 0; local_repo_env[i]; i++)
+ printf(%s\n, local_repo_env[i]);
+ continue;
+ }
if (!strcmp(arg, --show-toplevel)) {
const char *work_tree = get_git_work_tree();
if (work_tree)
@@ -711,6 +702,13 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
printf(%s%s.git\n, cwd, len cwd[len-1] !=
'/' ? / : );
continue;
}
+ if (!strcmp(arg, --resolve-git-dir)) {
+ const char *gitdir = resolve_gitdir(argv[i+1]);
+ if (!gitdir)
+ die(not a gitdir '%s', argv[i+1]);
+ puts(gitdir);
+ continue;
+ }
if (!strcmp(arg, --is-inside-git-dir)) {
printf(%s\n, is_inside_git_dir() ? true
: false);
--
1.8.3.3.972.gc83849e.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
[PATCH v2 0/2] Improvements to rev-parse option handling
This adds a new patch to remove the restrictions on --local-env-var and --resolve-git-dir so that they do not need to appear first on the command line. The other patch is update to reflect this as well as to split up the catch-all Options for Input subsection a bit. John Keeping (2): rev-parse: remove restrictions on some options rev-parse(1): logically group options Documentation/git-rev-parse.txt | 104 builtin/rev-parse.c | 28 +-- 2 files changed, 77 insertions(+), 55 deletions(-) -- 1.8.3.3.972.gc83849e.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
[PATCH v2 2/2] rev-parse(1): logically group options
The options section of the git-rev-parse manual page has grown
organically so that there now does not seem to be much logic behind the
ordering of the options. It also does not make it clear that certain
options must appear first on the command line.
Address this by reorganising the options into groups with subheadings.
The text of option descriptions does not change.
Signed-off-by: John Keeping j...@keeping.me.uk
---
Documentation/git-rev-parse.txt | 104
1 file changed, 64 insertions(+), 40 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 993903c..34c55a7 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -24,9 +24,23 @@ distinguish between them.
OPTIONS
---
+
+Operation Modes
+~~~
+
+Each of these options must appear first on the command line.
+
--parseopt::
Use 'git rev-parse' in option parsing mode (see PARSEOPT section below).
+--sq-quote::
+ Use 'git rev-parse' in shell quoting mode (see SQ-QUOTE
+ section below). In contrast to the `--sq` option below, this
+ mode does only quoting. Nothing else is done to command input.
+
+Options for --parseopt
+~~
+
--keep-dashdash::
Only meaningful in `--parseopt` mode. Tells the option parser to echo
out the first `--` met instead of skipping it.
@@ -36,10 +50,8 @@ OPTIONS
the first non-option argument. This can be used to parse sub-commands
that take options themselves.
---sq-quote::
- Use 'git rev-parse' in shell quoting mode (see SQ-QUOTE
- section below). In contrast to the `--sq` option below, this
- mode does only quoting. Nothing else is done to command input.
+Options for Filtering
+~
--revs-only::
Do not output flags and parameters not meant for
@@ -55,6 +67,9 @@ OPTIONS
--no-flags::
Do not output flag parameters.
+Options for Output
+~~
+
--default arg::
If there is no parameter given by the user, use `arg`
instead.
@@ -110,6 +125,17 @@ can be used.
strip '{caret}' prefix from the object names that already have
one.
+--abbrev-ref[=(strict|loose)]::
+ A non-ambiguous short name of the objects name.
+ The option core.warnAmbiguousRefs is used to select the strict
+ abbreviation mode.
+
+--short::
+--short=number::
+ Instead of outputting the full SHA-1 values of object names try to
+ abbreviate them to a shorter unique name. When no length is specified
+ 7 is used. The minimum length is 4.
+
--symbolic::
Usually the object names are output in SHA-1 form (with
possible '{caret}' prefix); this option makes them output in a
@@ -123,16 +149,8 @@ can be used.
unfortunately named tag master), and show them as full
refnames (e.g. refs/heads/master).
---abbrev-ref[=(strict|loose)]::
- A non-ambiguous short name of the objects name.
- The option core.warnAmbiguousRefs is used to select the strict
- abbreviation mode.
-
---disambiguate=prefix::
- Show every object whose name begins with the given prefix.
- The prefix must be at least 4 hexadecimal digits long to
- avoid listing each and every object in the repository by
- mistake.
+Options for Objects
+~~~
--all::
Show all refs found in `refs/`.
@@ -155,18 +173,20 @@ shown. If the pattern does not contain a globbing
character (`?`,
character (`?`, `*`, or `[`), it is turned into a prefix
match by appending `/*`.
---show-toplevel::
- Show the absolute path of the top-level directory.
+--disambiguate=prefix::
+ Show every object whose name begins with the given prefix.
+ The prefix must be at least 4 hexadecimal digits long to
+ avoid listing each and every object in the repository by
+ mistake.
---show-prefix::
- When the command is invoked from a subdirectory, show the
- path of the current directory relative to the top-level
- directory.
+Options for Files
+~
---show-cdup::
- When the command is invoked from a subdirectory, show the
- path of the top-level directory relative to the current
- directory (typically a sequence of ../, or an empty string).
+--local-env-vars::
+ List the GIT_* environment variables that are local to the
+ repository (e.g. GIT_DIR or GIT_WORK_TREE, but not GIT_EDITOR).
+ Only the names of the variables are listed, not their value,
+ even if they are set.
--git-dir::
Show `$GIT_DIR` if defined. Otherwise show the path to
@@ -188,17 +208,27 @@ print a message to stderr and exit with nonzero status.
--is-bare-repository::
When the repository is bare print true, otherwise false.
---local-env-vars::
- List the GIT_* environment variables that are local
Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull
On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
diff --git a/git-pull.sh b/git-pull.sh
index 638aabb..4a6a863 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -264,6 +274,30 @@ case $merge_head in
die $(gettext Cannot rebase onto multiple branches)
fi
;;
+*)
+ # integrating with a single other history
+ merge_head=${merge_head% }
+ if test -z $rebase$no_ff$ff_only${squash#--no-squash}
+ test -n $orig_head
+ ! $(git merge-base --is-ancestor $orig_head $merge_head)
I think this needs to be:
! $(git merge-base --is-ancestor $orig_head $merge_head ||
git merge-base --is-ancestor $merge_head $orig_head)
in order to avoid printing the message when git pull does not fetch
any new changes and the user has some new commits.
--
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
[RFC/PATCH] rev-parse(1): logically group options
The options section of the git-rev-parse manual page has grown
organically so that there now does not seem to be much logic behind the
ordering of the options. It also does not make it clear that certain
options must appear first on the command line.
Address this by reorganising the options into groups with subheadings.
The text of option descriptions does not change.
Signed-off-by: John Keeping j...@keeping.me.uk
---
Documentation/git-rev-parse.txt | 104 +++-
1 file changed, 61 insertions(+), 43 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 993903c..fa284b0 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -24,9 +24,35 @@ distinguish between them.
OPTIONS
---
+
+Operation Modes
+~~~
+
+Each of these options must appear first on the command line.
+
+--local-env-vars::
+ List the GIT_* environment variables that are local to the
+ repository (e.g. GIT_DIR or GIT_WORK_TREE, but not GIT_EDITOR).
+ Only the names of the variables are listed, not their value,
+ even if they are set.
+
--parseopt::
Use 'git rev-parse' in option parsing mode (see PARSEOPT section below).
+--resolve-git-dir path::
+ Check if path is a valid repository or a gitfile that
+ points at a valid repository, and print the location of the
+ repository. If path is a gitfile then the resolved path
+ to the real repository is printed.
+
+--sq-quote::
+ Use 'git rev-parse' in shell quoting mode (see SQ-QUOTE
+ section below). In contrast to the `--sq` option below, this
+ mode does only quoting. Nothing else is done to command input.
+
+Options for --parseopt
+~~
+
--keep-dashdash::
Only meaningful in `--parseopt` mode. Tells the option parser to echo
out the first `--` met instead of skipping it.
@@ -36,10 +62,8 @@ OPTIONS
the first non-option argument. This can be used to parse sub-commands
that take options themselves.
---sq-quote::
- Use 'git rev-parse' in shell quoting mode (see SQ-QUOTE
- section below). In contrast to the `--sq` option below, this
- mode does only quoting. Nothing else is done to command input.
+Options for Filtering
+~
--revs-only::
Do not output flags and parameters not meant for
@@ -55,6 +79,9 @@ OPTIONS
--no-flags::
Do not output flag parameters.
+Options for Output
+~~
+
--default arg::
If there is no parameter given by the user, use `arg`
instead.
@@ -110,6 +137,17 @@ can be used.
strip '{caret}' prefix from the object names that already have
one.
+--abbrev-ref[=(strict|loose)]::
+ A non-ambiguous short name of the objects name.
+ The option core.warnAmbiguousRefs is used to select the strict
+ abbreviation mode.
+
+--short::
+--short=number::
+ Instead of outputting the full SHA-1 values of object names try to
+ abbreviate them to a shorter unique name. When no length is specified
+ 7 is used. The minimum length is 4.
+
--symbolic::
Usually the object names are output in SHA-1 form (with
possible '{caret}' prefix); this option makes them output in a
@@ -123,16 +161,8 @@ can be used.
unfortunately named tag master), and show them as full
refnames (e.g. refs/heads/master).
---abbrev-ref[=(strict|loose)]::
- A non-ambiguous short name of the objects name.
- The option core.warnAmbiguousRefs is used to select the strict
- abbreviation mode.
-
---disambiguate=prefix::
- Show every object whose name begins with the given prefix.
- The prefix must be at least 4 hexadecimal digits long to
- avoid listing each and every object in the repository by
- mistake.
+Options for Input
+~
--all::
Show all refs found in `refs/`.
@@ -155,18 +185,11 @@ shown. If the pattern does not contain a globbing
character (`?`,
character (`?`, `*`, or `[`), it is turned into a prefix
match by appending `/*`.
---show-toplevel::
- Show the absolute path of the top-level directory.
-
---show-prefix::
- When the command is invoked from a subdirectory, show the
- path of the current directory relative to the top-level
- directory.
-
---show-cdup::
- When the command is invoked from a subdirectory, show the
- path of the top-level directory relative to the current
- directory (typically a sequence of ../, or an empty string).
+--disambiguate=prefix::
+ Show every object whose name begins with the given prefix.
+ The prefix must be at least 4 hexadecimal digits long to
+ avoid listing each and every object in the repository by
+ mistake.
--git-dir::
Show `$GIT_DIR` if defined. Otherwise show the path to
@@ -177,6 +200,14
Re: Git counterpart to SVN bugtraq properties?
On Wed, Jul 17, 2013 at 03:03:14PM +0200, Marc Strapetz wrote: I'm looking for a specification or guidelines on how a Git client should integrate with bug tracking systems. For SVN, one can use bugtraq-properties [1] to specify e.g. the issue tracker URL or how to parse the bug ID from a commit message. AFAIU, there is nothing comparable for Git [2]? If that's actually the case, is someone interested in working out a similar specification for Git? [1] http://code.google.com/p/tortoisesvn/source/browse/tags/version_1.2.0/doc/issuetrackers.txt [2] http://stackoverflow.com/questions/17545548 The Git way to record the issue ID as a footer in the commit message. See for example [1]. Although I'm not aware of any standard for naming this footer. In terms of recording the URL and other data, I think you'd want a dotfile in the repository (perhaps .bugzilla). This shoudld probably be in the gitconfig format, like .gitmodules. I think all it needs is to draw up a spec for the names of keys and format of their values, along with the format of footer(s) identifying issues associated with a commit and to persuade UI developers to support it... ;-) [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4a88f73f14f6d6c94616538427e1235a6d0a5885 -- 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 4/7] remote.c: add command line option parser for --lockref
On Tue, Jul 09, 2013 at 12:53:27PM -0700, Junio C Hamano wrote:
diff --git a/remote.c b/remote.c
index 81bc876..e9b423a 100644
--- a/remote.c
+++ b/remote.c
@@ -1938,3 +1938,62 @@ struct ref *get_stale_heads(struct refspec *refs, int
ref_count, struct ref *fet
string_list_clear(ref_names, 0);
return stale_refs;
}
+
+/*
+ * Lockref aka CAS
+ */
+void clear_cas_option(struct push_cas_option *cas)
+{
+ int i;
+
+ for (i = 0; i cas-nr; i++)
+ free(cas-entry-refname);
Should this be
free(cas-entry[i]-refname);
?
+ free(cas-entry);
+ memset(cas, 0, sizeof(*cas));
+}
--
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 7/7] push: document --lockref
On Sat, Jul 13, 2013 at 01:08:09PM -0700, Junio C Hamano wrote:
Junio C Hamano gits...@pobox.com writes:
If --lockref automatically implies --allow-no-ff (the design in
the reposted patch), you cannot express that combination. But once
you use --lockref in such a situation , for the push to succeed,
you know that the push replaces not just _any_ ancestor of what you
are pushing, but replaces the exact current value. So I do not think
your implicit introduction of --allow-no-ff via redefining the
semantics of the plus prefix is not adding much value (if any),
while making the common case less easy to use.
No; --lockref only adds the check that the destination is at the
expected revision, but does *NOT* override the no-ff check.
You _could_ do it in that way, but that is less useful.
Another issue I have with the proposal is that we close the door to
force only this one convenience we have with +ref vs --force
ref. Assuming that it is useful to require lockref while still
making sure that the usual must fast-forward rule is followed (if
that is not the case, I do not see a reason why your proposal is any
useful---am I missing something?), I would prefer to allow users a
way to decorate this basic syntax to say:
git push --lockref master jch pu
things like
(1) pu may not fast-forward and please override that must
fast-forward check from it, while still keeping the lockref
safety (e.g. +pu that does not --force, which is your
proposal);
(2) any of them may not fast-forward and please override that must
fast-forward check from it, while still keeping the lockref
safety (without adding --allow-no-ff, I do not see how it is
possible with your proposal, short of forcing user to add +
everywhere);
(3) I know jch does not fast-forward so please override the must
fast-forward, but still apply the lockref safety, pu may not
even satisfy lockref safety so please force it (as the only
force this one semantics is removed from +, I do not see how
it is possible with your proposal).
I haven't been following this thread too closely, but I was assuming
that the interface would be something like this:
git push origin +master
git push --force origin master
mean the same thing and do what they do now.
git push origin *master
git push --lockref origin master
both mean the same thing: using the new compare-and-swap mode only
update master if the remote side corresponds to remotes/origin/master
[1].
git push origin *master:refs/heads/master:@{1}
means to push the local ref master to the remote ref refs/heads/master
if it currently points at @{1}.
In this scenario, giving both --lockref and --force should be an error
because the user is probably confused (the obvious interpretation is
that --force wins, but I don't think that's sensible).
I'm not sure what should happen with:
git push --force origin *master
where it appears that the user is asking for a compare-and-swap update
of master but the --force is overriding this. I think we have to let
--force win because when the refspec comes from remote.name.push we
have to let the command-line --force override the specified behaviour.
I don't particularly like the name --lockref, the original --cas feels
more descriptive to me.
[1] In fact, I suspect this would have to be the ref that
refs/heads/master maps to using remote.origin.fetch.
--
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] fixup! pull: require choice between rebase/merge on non-fast-forward pull
---
On Fri, Jun 28, 2013 at 03:41:34PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
I don't think git pull remote branch falls into the same category as
plain git pull so I'm not convinced that defaulting to merge there is
unreasonable. The original message about this [1] did talk about only
git pull with no arguments.
If you want to limit the scope to only git pull (without any
command line argument), I actually do not have strong preference for
or against it either way. Perhaps a follow-up patch to be squashed?
Here is that patch. The test changes here are all reverting changes in
ae2dab2 (pull: require choice between rebase/merge on non-fast-forward
pull, 2013-06-27) - with this change to git-pull.sh the only change
needed in the tests is in t5524-pull-msg:
$ git diff ae2dab2^ -- t
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..660714b 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
test_expect_success pull '
(
cd cloned
- git pull --log
+ git pull --log --merge
git log -2
git cat-file commit HEAD result
grep Dollar result
git-pull.sh| 1 +
t/annotate-tests.sh| 2 +-
t/t4013-diff-various.sh| 2 --
t/t4200-rerere.sh | 2 --
t/t5500-fetch-pack.sh | 6 +-
t/t5521-pull-options.sh| 2 --
t/t5700-clone-reference.sh | 4 ++--
t/t6022-merge-rename.sh| 2 --
t/t6026-merge-attr.sh | 2 +-
t/t6029-merge-subtree.sh | 1 -
t/t6037-merge-ours-theirs.sh | 2 --
t/t9114-git-svn-dcommit-merge.sh | 2 +-
t/t9400-git-cvsserver-server.sh| 2 +-
t/t9500-gitweb-standalone-no-errors.sh | 2 +-
14 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 5ce67f9..0ff4a98 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -279,6 +279,7 @@ case $merge_head in
merge_head=${merge_head% }
if test -z $rebase$no_ff$ff_only${squash#--no-squash}
test -n $orig_head
+ test $# = 0
! $(git merge-base --is-ancestor $orig_head $merge_head)
then
echo 2 orig-head was $orig_head
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index af02c6d..c56a77d 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -79,7 +79,7 @@ test_expect_success \
test_expect_success \
'merge-setup part 3' \
-'git pull --merge . branch1'
+'git pull . branch1'
test_expect_success \
'Two lines blamed on A, one on B, two on B1, one on B2' \
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1ee2198..e77c09c 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -12,8 +12,6 @@ LF='
test_expect_success setup '
- git config pull.rebase false
-
GIT_AUTHOR_DATE=2006-06-26 00:00:00 +
GIT_COMMITTER_DATE=2006-06-26 00:00:00 +
export GIT_AUTHOR_DATE GIT_COMMITTER_DATE
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 0563357..7ff 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -25,8 +25,6 @@ test_description='git rerere
. ./test-lib.sh
test_expect_success 'setup' '
- git config pull.rebase false
-
cat a1 -\EOF
Some title
==
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 4be8877..fd2598e 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -143,11 +143,7 @@ test_expect_success 'clone shallow depth 1 with fsck' '
'
test_expect_success 'clone shallow' '
- git clone --no-single-branch --depth 2 file://$(pwd)/. shallow
- (
- cd shallow
- git config pull.rebase false
- )
+ git clone --no-single-branch --depth 2 file://$(pwd)/. shallow
'
test_expect_success 'clone shallow depth count' '
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index d821fab..453aba5 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -91,8 +91,6 @@ test_expect_success 'git pull --force' '
[branch master]
remote = two
merge = refs/heads/master
- [pull]
- rebase = false
EOF
git pull two
test_commit A
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 306badf..6537911 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -94,7 +94,7 @@ cd $base_dir
test_expect_success 'pulling changes from origin' \
'cd C
-git pull --merge origin'
+git pull origin'
cd $base_dir
@@ -109,7 +109,7 @@ cd $base_dir
test_expect_success 'pulling changes from origin' \
'cd D
-git pull --merge origin'
+git pull origin'
cd $base_dir
diff --git a/t/t6022-merge
Re: [PATCH 1/2] push: avoid suggesting merging remote changes
On Mon, Jul 08, 2013 at 03:47:19PM +0200, Matthieu Moy wrote: John Keeping j...@keeping.me.uk writes: static const char message_advice_pull_before_push[] = N_(Updates were rejected because the tip of your current branch is behind\n - its remote counterpart. Merge the remote changes (e.g. 'git pull')\n - before pushing again.\n + its remote counterpart. Integrate the remote changes (e.g.\n + 'git pull ...') before pushing again.\n To me, merge includes rebase, so I'd say the merge - integrate change is not needed, but I have nothing against it either. Yes, I agree that in some sense merge does include rebase but I suspect some people read it to mean git merge and saying integrate removes that potential source of confusion. The ... added are a bit weird with the quotes around. Quotes may suggest that the content is to be taken literally, which is not the case anymore. Not a real objection anyway, just thinking aloud. I hadn't thought of it that way, but I wonder how else we can delimit the command. -- 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: [RFC/PATCH] fetch: make --prune configurable
On Mon, Jul 08, 2013 at 02:56:57PM +0200, Michael Schubert wrote: $gmane/201715 brought up the idea to fetch --prune by default. Since --prune is a potentially destructive operation (Git doesn't keep reflogs for deleted references yet), we don't want to prune without users consent. To accommodate users who want to either prune always or when fetching from a particular remote, add two new configuration variables fetch.prune and remote.name.prune: - fetch.prune allows to enable prune for all fetch operations. - remote.name.prune allows to change the behaviour per remote. Should this be remote.name.pruneFetch? I'd quite like to be able to configure --prune for git-push as well (I just haven't got around to actually doing anything about it yet...) and it might be better to be explicit in the remote.name section from the start. I'm not sure it's necessary since we already have remote and pushremote so we could have prune and pushprune but perhaps it's worth considering. -- 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 2/2] send-email: introduce sendemail.smtpsslcertpath
On Sat, Jul 06, 2013 at 09:12:31PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
@@ -1096,19 +1101,18 @@ sub smtp_auth_maybe {
# Helper to come up with SSL/TLS certification validation params
# and warn when doing no verification
sub ssl_verify_params {
- use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
-
- if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ if ($smtp_ssl_verify == 0) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_NONE);
I do not see any use IO::Socket::SSL anywhere after applying this
patch. Is this expected to work?
I don't get any errors about unknown variables when running it. Do we
get IO::Socket::SSL imported through Net::SMTP::SSL, which extends it?
--
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] git-config: update doc for --get with multiple values
On Wed, Jul 03, 2013 at 11:47:50AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Since commit 00b347d (git-config: do not complain about duplicate entries, 2012-10-23), git config --get does not exit with an error if there are multiple values for the specified key but instead returns the last value. Update the documentation to reflect this. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-config.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 19a7be0..fbad05e 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -82,7 +82,7 @@ OPTIONS --get:: Get the value for a given key (optionally filtered by a regex matching the value). Returns error code 1 if the key was not - found and error code 2 if multiple key values were found. + found and the last value if multiple key values were found. --get-all:: Like get, but does not fail if the number of values for the key Thanks. I wondered if we should explain the significance of last a bit more (like this results in the value from the most specific configuration file to be used, the ones in $GIT_DIR/config overriding what is in $HOME/.gitconfig), but I do not have a strong opinion either way. Let's queue this for 'maint' for now. I don't think that change belongs here. How about doing something like this in the FILES section (the first two hunks are just reordering the existing list, only the last hunk changes the content): -- 8 -- diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index fbad05e..99dc497 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -206,12 +206,8 @@ FILES If not set explicitly with '--file', there are four files where 'git config' will search for configuration options: -$GIT_DIR/config:: - Repository specific configuration file. - -~/.gitconfig:: - User-specific configuration file. Also called global - configuration file. +$(prefix)/etc/gitconfig:: + System-wide configuration file. $XDG_CONFIG_HOME/git/config:: Second user-specific configuration file. If $XDG_CONFIG_HOME is not set @@ -221,8 +217,12 @@ $XDG_CONFIG_HOME/git/config:: you sometimes use older versions of Git, as support for this file was added fairly recently. -$(prefix)/etc/gitconfig:: - System-wide configuration file. +~/.gitconfig:: + User-specific configuration file. Also called global + configuration file. + +$GIT_DIR/config:: + Repository specific configuration file. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration @@ -230,6 +230,10 @@ file are not available they will be ignored. If the repository configuration file is not available or readable, 'git config' will exit with a non-zero error code. However, in neither case will an error message be issued. +The files are read in the order given above, with last value found taking +precedence over values read earlier. When multiple values are taken then all +values of a key from all files will be used. + All writing options will per default write to the repository specific configuration file. Note that this also affects options like '--replace-all' and '--unset'. *'git config' will only ever change one file at a time*. -- 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 0/2] Avoid suggestions to merge remote changes
As another aspect of the change to make git-pull die when remote changes do not fast-forward, this series changes the advice messages in git-push to avoid implying that the user wants to merge remote changes. I've chosen the word integrate because it does not carry any special meaning in Git (in terms of being a command) and seems to cover the merge and rebase cases nicely. John Keeping (2): push: avoid suggesting merging remote changes pull: change the description to integrate changes Documentation/git-pull.txt | 2 +- builtin/push.c | 12 ++-- git-pull.sh| 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) -- 1.8.3.2.855.gbc9faed -- 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 1/2] push: avoid suggesting merging remote changes
With some workflows, it is more suitable to rebase on top of remote changes when a push does not fast-forward. Change the advice messages in git-push to suggest that a user integrate the remote changes instead of merge the remote changes to make this slightly clearer. Also change the suggested 'git pull' to 'git pull ...' to hint to users that they may want to add other parameters. Suggested-by: Philip Oakley philipoak...@iee.org Signed-off-by: John Keeping j...@keeping.me.uk --- builtin/push.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/push.c b/builtin/push.c index 2d84d10..44e53cd 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -211,8 +211,8 @@ static void setup_default_push_refspecs(struct remote *remote) static const char message_advice_pull_before_push[] = N_(Updates were rejected because the tip of your current branch is behind\n - its remote counterpart. Merge the remote changes (e.g. 'git pull')\n - before pushing again.\n + its remote counterpart. Integrate the remote changes (e.g.\n + 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_use_upstream[] = @@ -223,15 +223,15 @@ static const char message_advice_use_upstream[] = static const char message_advice_checkout_pull_push[] = N_(Updates were rejected because a pushed branch tip is behind its remote\n - counterpart. Check out this branch and merge the remote changes\n - (e.g. 'git pull') before pushing again.\n + counterpart. Check out this branch and integrate the remote changes\n + (e.g. 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_ref_fetch_first[] = N_(Updates were rejected because the remote contains work that you do\n not have locally. This is usually caused by another repository pushing\n - to the same ref. You may want to first merge the remote changes (e.g.,\n - 'git pull') before pushing again.\n + to the same ref. You may want to first integrate the remote changes\n + (e.g., 'git pull ...') before pushing again.\n See the 'Note about fast-forwards' in 'git push --help' for details.); static const char message_advice_ref_already_exists[] = -- 1.8.3.2.855.gbc9faed -- 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 2/2] pull: change the description to integrate changes
Since git-pull learned the --rebase option it has not just been about merging changes from a remote repository (where merge is in the sense of git merge). Change the description to use integrate instead of merge in order to reflect this. Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/git-pull.txt | 2 +- git-pull.sh| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 24ab07a..6ef8d59 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -3,7 +3,7 @@ git-pull(1) NAME -git-pull - Fetch from and merge with another repository or a local branch +git-pull - Fetch from and integrate with another repository or a local branch SYNOPSIS diff --git a/git-pull.sh b/git-pull.sh index 6828e2c..ecf0011 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -5,7 +5,7 @@ # Fetch one or more remote refs and merge it/them into the current HEAD. USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash] [--[no-]ff] [-s strategy]... [fetch-options] repo head...' -LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.' +LONG_USAGE='Fetch one or more remote refs and integrate it/them into the current HEAD.' SUBDIRECTORY_OK=Yes OPTIONS_SPEC= . git-sh-setup -- 1.8.3.2.855.gbc9faed -- 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] git-config(1): clarify precedence of multiple values
In order to clarify which value is used when there are multiple values defined for a key, re-order the list of file locations so that it runs from least specific to most specific. Then add a paragraph which simply says that the last value will be used. Signed-off-by: John Keeping j...@keeping.me.uk --- On Sun, Jul 07, 2013 at 10:31:38AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: I wondered if we should explain the significance of last a bit more (like this results in the value from the most specific configuration file to be used, the ones in $GIT_DIR/config overriding what is in $HOME/.gitconfig), but I do not have a strong opinion either way. Let's queue this for 'maint' for now. I don't think that change belongs here. How about doing something like this in the FILES section (the first two hunks are just reordering the existing list, only the last hunk changes the content): Sounds like a good change to me ;-). So here it is as a proper patch :-) Documentation/git-config.txt | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index fbad05e..99dc497 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -206,12 +206,8 @@ FILES If not set explicitly with '--file', there are four files where 'git config' will search for configuration options: -$GIT_DIR/config:: - Repository specific configuration file. - -~/.gitconfig:: - User-specific configuration file. Also called global - configuration file. +$(prefix)/etc/gitconfig:: + System-wide configuration file. $XDG_CONFIG_HOME/git/config:: Second user-specific configuration file. If $XDG_CONFIG_HOME is not set @@ -221,8 +217,12 @@ $XDG_CONFIG_HOME/git/config:: you sometimes use older versions of Git, as support for this file was added fairly recently. -$(prefix)/etc/gitconfig:: - System-wide configuration file. +~/.gitconfig:: + User-specific configuration file. Also called global + configuration file. + +$GIT_DIR/config:: + Repository specific configuration file. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration @@ -230,6 +230,10 @@ file are not available they will be ignored. If the repository configuration file is not available or readable, 'git config' will exit with a non-zero error code. However, in neither case will an error message be issued. +The files are read in the order given above, with last value found taking +precedence over values read earlier. When multiple values are taken then all +values of a key from all files will be used. + All writing options will per default write to the repository specific configuration file. Note that this also affects options like '--replace-all' and '--unset'. *'git config' will only ever change one file at a time*. -- 1.8.3.2.855.gbc9faed -- 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] Documentation: finding $(prefix)/etc/gitconfig when prefix = /usr
On Mon, Jul 08, 2013 at 12:00:02AM +0200, Robin Rosenberg wrote: Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com --- Documentation/git-config.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 9ae2508..3198d52 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -107,7 +107,8 @@ See also FILES. --system:: For writing options: write to system-wide $(prefix)/etc/gitconfig - rather than the repository .git/config. + rather than the repository .git/config. However, $(prefix) is /usr + then /etc/gitconfig is used. That's a build time condition, not something that's decided at runtime so I'm not sure that this logic belongs in the user facing documentation. The technically correct change would be to use $(sysconfdir)/gitconfig but I think that will just confuse users more. Since we have a build step for the documentation, I wonder if it's possible to replace these with the correct directory at build time. + For reading options: read only from system-wide $(prefix)/etc/gitconfig rather than from all available files. @@ -214,7 +215,8 @@ $XDG_CONFIG_HOME/git/config:: file was added fairly recently. $(prefix)/etc/gitconfig:: - System-wide configuration file. + System-wide configuration file, unless $(prefix) is /usr. In the + latter case /etc/gitconfig is used. If no further options are given, all reading options will read all of these files that are available. If the global or the system-wide configuration -- 1.8.3.rc0.19.g7e6a0cc -- 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 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 11:25:36PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
I'd rather have '$smtp_ssl_cert_path ne ' in the first if condition
(instead of the '-d $smtp_ssl_cert_path') ...
I agree. The signal for no certs should be an explicit nonsense
value like an empty string, not just a string that does not name an
expected filesystem object. Otherwise people can misspell paths and
disable the validation by accident.
Perhaps a complete solution could allow CA files as well.
Yes, that would be a good idea. Care to roll into a fixup! patch
against [2/2]?
Here's a patch that should do that. However, when testing this I
couldn't get the SSL_verify_mode warning to disappear and
git-send-email kept connecting to my untrusted server - this was using
the SSL code path not the TLS upgrade one.
I think this is caused by the SSL_verify_mode argument not getting all
the way down to the configure_SSL function in IO::Socket::SSL but I
can't see what the code in git-send-email is doing wrong. Can any Perl
experts point out what's going wrong?
Also, I tried Brian's IO::Socket::SSL-import(qw(SSL_VERIFY_PEER
SSL_VERIFY_NONE)); but that produced a warning message about the uses
of SSL_VERIFY_PEER and SSL_VERIFY_NONE following that statement, so I
ended up qualifying each use in the code below.
-- 8 --
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 605f263..b56c215 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -198,6 +198,10 @@ must be used for each option.
--smtp-ssl::
Legacy alias for '--smtp-encryption ssl'.
+--smtp-ssl-verify::
+--no-smtp-ssl-verify::
+ Enable SSL certificate verification. Defaults to on.
+
--smtp-ssl-cert-path::
Path to ca-certificates. Defaults to `/etc/ssl/certs`, or
'sendemail.smtpsslcertpath'.
diff --git a/git-send-email.perl b/git-send-email.perl
index 3b80340..cbe85aa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -69,8 +69,10 @@ git send-email [options] file | directory | rev-list
options
--smtp-pass str * Password for SMTP-AUTH; not necessary.
--smtp-encryption str * tls or ssl; anything else disables.
--smtp-ssl * Deprecated. Use '--smtp-encryption ssl'.
+--[no-]smtp-ssl-verify * Enable SSL certificate verification.
+ Default on.
--smtp-ssl-cert-pathstr * Path to ca-certificates. Defaults to
- /etc/ssl/certs.
+ a platform-specific value.
--smtp-domain str * The domain name sent to HELO/EHLO
handshake
--smtp-debug0|1 * Disable, enable Net::SMTP debug.
@@ -197,7 +199,7 @@ my ($thread, $chain_reply_to, $suppress_from,
$signed_off_by_cc);
my ($to_cmd, $cc_cmd);
my ($smtp_server, $smtp_server_port, @smtp_server_options);
my ($smtp_authuser, $smtp_encryption);
-my ($smtp_ssl_cert_path);
+my ($smtp_ssl_verify, $smtp_ssl_cert_path);
my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
my ($validate, $confirm);
my (@suppress_cc);
@@ -214,6 +216,7 @@ my %config_bool_settings = (
suppressfrom = [\$suppress_from, undef],
signedoffbycc = [\$signed_off_by_cc, undef],
signedoffcc = [\$signed_off_by_cc, undef], # Deprecated
+smtpsslverify = [\$smtp_ssl_verify, 1],
validate = [\$validate, 1],
multiedit = [\$multiedit, undef],
annotate = [\$annotate, undef]
@@ -306,6 +309,8 @@ my $rc = GetOptions(h = \$help,
smtp-pass:s = \$smtp_authpass,
smtp-ssl = sub { $smtp_encryption = 'ssl' },
smtp-encryption=s = \$smtp_encryption,
+ smtp-ssl-cert-path=s = \$smtp_ssl_cert_path,
+ smtp-ssl-verify! = \$smtp_ssl_verify,
smtp-debug:i = \$debug_net_smtp,
smtp-domain:s = \$smtp_domain,
identity=s = \$identity,
@@ -1096,19 +1101,18 @@ sub smtp_auth_maybe {
# Helper to come up with SSL/TLS certification validation params
# and warn when doing no verification
sub ssl_verify_params {
- use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
-
- if (!defined $smtp_ssl_cert_path) {
- $smtp_ssl_cert_path = /etc/ssl/certs;
+ if ($smtp_ssl_verify == 0) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_NONE);
}
- if (-d $smtp_ssl_cert_path) {
- return (SSL_verify_mode = SSL_VERIFY_PEER,
- SSL_ca_path = $smtp_ssl_cert_path);
+ if (! defined $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_PEER);
+ } elsif (-f $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = IO::Socket::SSL-SSL_VERIFY_PEER,
+ SSL_ca_file
Re: [PATCH] send-email: squelch warning from Net::SMTP::SSL
On Fri, Jul 05, 2013 at 03:48:31PM +0530, Ramkumar Ramachandra wrote: Due to a recent change in the Net::SMTP::SSL module, send-email emits the following ugly warning everytime a email is sent via SSL: *** Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client is deprecated! Please set SSL_verify_mode to SSL_VERIFY_PEER together with SSL_ca_file|SSL_ca_path for verification. If you really don't want to verify the certificate and keep the connection open to Man-In-The-Middle attacks please set SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application. *** Fix this by explicitly specifying SSL_verify_mode = SSL_VERIFY_NONE in Net::SMTP::SSL-start_SSL(). I don't think this is really fix, it's more plastering over the problem. As the message from OpenSSL says, specifying this means that we're explicitly saying that we don't want to check the server certificate which loses half of the security of SSL. I'd rather leave this as it is (complete with the big scary error message) and eventually fix it properly by letting the user specify the ca_file or ca_path. Perhaps we can even set a sensible default, although I expect this needs to be platform-specific. -- 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 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
You've covered the STARTTLS case, but not the SSL one right above it.
Someone using smtps on port 465 will still see the warning. You can
pass SSL_verify_mode to Net::SMTP::SSL-new just like you pass it to
start_SSL.
OK, will a fix-up look like this on top of 1/2 and 2/2?
According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
is specified then builtin defaults will be used, so I wonder if we
should pass SSL_VERIFY_PEER regardless (possibly with a switch for
SSL_VERIFY_NONE if people really need that).
[1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
git-send-email.perl | 39 +++
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 52028ba..3b80340 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1093,6 +1093,25 @@ sub smtp_auth_maybe {
return $auth;
}
+# Helper to come up with SSL/TLS certification validation params
+# and warn when doing no verification
+sub ssl_verify_params {
+ use IO::Socket::SSL qw(SSL_VERIFY_PEER SSL_VERIFY_NONE);
+
+ if (!defined $smtp_ssl_cert_path) {
+ $smtp_ssl_cert_path = /etc/ssl/certs;
+ }
+
+ if (-d $smtp_ssl_cert_path) {
+ return (SSL_verify_mode = SSL_VERIFY_PEER,
+ SSL_ca_path = $smtp_ssl_cert_path);
+ } else {
+ print STDERR warning: Using SSL_VERIFY_NONE. .
+ See sendemail.smtpsslcertpath.\n;
+ return (SSL_verify_mode = SSL_VERIFY_NONE);
+ }
+}
+
--
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 2/2] send-email: introduce sendemail.smtpsslcertpath
On Fri, Jul 05, 2013 at 11:30:11AM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
On Fri, Jul 05, 2013 at 10:20:11AM -0700, Junio C Hamano wrote:
brian m. carlson sand...@crustytoothpaste.net writes:
You've covered the STARTTLS case, but not the SSL one right above it.
Someone using smtps on port 465 will still see the warning. You can
pass SSL_verify_mode to Net::SMTP::SSL-new just like you pass it to
start_SSL.
OK, will a fix-up look like this on top of 1/2 and 2/2?
According to IO::Socket::SSL [1], if neither SSL_ca_file nor SSL_ca_path
is specified then builtin defaults will be used, so I wonder if we
should pass SSL_VERIFY_PEER regardless (possibly with a switch for
SSL_VERIFY_NONE if people really need that).
[1] http://search.cpan.org/~sullr/IO-Socket-SSL-1.951/lib/IO/Socket/SSL.pm
Interesting. That frees us from saying we assume /etc/ssl/cacerts
is the default location, and let the users override it.
To help those I do not want verification because I know my server
does not present valid certificate, I know my server is internal and
trustable, and I do not bother to fix it people, we can let them
specify an empty string (or any non-directory) as the CACertPath,
and structure the code like so?
if (defined $smtp_ssl_cert_path -d $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_path = $smtp_ssl_cert_path);
} elsif (defined $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_NONE);
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER);
}
I'd rather have '$smtp_ssl_cert_path ne ' in the first if condition
(instead of the '-d $smtp_ssl_cert_path') but that seems reasonable and
agrees with my reading of the documentation.
Perhaps a complete solution could allow CA files as well:
if (defined $smtp_ssl_cert_path) {
if ($smtp_ssl_cert_path eq ) {
return (SSL_verify_mode = SSL_VERIFY_NONE);
} elsif (-f $smtp_ssl_cert_path) {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_file = $smtp_ssl_cert_path);
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER,
SSL_ca_path = $smtp_ssl_cert_path);
}
} else {
return (SSL_verify_mode = SSL_VERIFY_PEER);
}
--
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 gui replaces amend message when prepare-commit-msg hook is used
On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote: Hi, If a prepare-commit-msg hook is used, git gui executes it for New Commit. If the New Commit is selected, and then immediately Amend (before the hook returns), when the hook returns the message is replaced with the one produced by the hook. I think this is a problem with the hook you are running. The hook is given arguments specifying the message file and optionally the source of whatever is already in the file (see githooks(5) for details). It sounds like your hook is blindly overwriting the file, rather than preserving its contents in the cases where you wish to do so. -- 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 gui replaces amend message when prepare-commit-msg hook is used
On Thu, Jul 04, 2013 at 01:59:10PM +0300, Orgad Shaneh wrote: On Thu, Jul 4, 2013 at 1:34 PM, John Keeping j...@keeping.me.uk wrote: On Thu, Jul 04, 2013 at 12:47:28PM +0300, Orgad Shaneh wrote: Hi, If a prepare-commit-msg hook is used, git gui executes it for New Commit. If the New Commit is selected, and then immediately Amend (before the hook returns), when the hook returns the message is replaced with the one produced by the hook. I think this is a problem with the hook you are running. The hook is given arguments specifying the message file and optionally the source of whatever is already in the file (see githooks(5) for details). It sounds like your hook is blindly overwriting the file, rather than preserving its contents in the cases where you wish to do so. Let me try to explain. When git gui is executed, it calls the prepare-commit-msg script with .git/PREPARE_COMMIT_MSG as an argument. When amend is selected, the hook is *not* called at all (what would it prepare? The message is already committed) Use the following hook to reproduce: --- snip --- #!/bin/sh sleep 5 echo $@ /tmp/hook.log echo 'Hello hook' $1 --- snip --- Now run git gui (or press F5 if it is already running), and before 5 seconds pass, click Amend last commit. You'll see the commit's message, but when the 5 seconds pass it is replaced with Hello hook. That's the bug. Yes, and that's a bug in the hook. The hook is called with a second argument commit but it is ignoring this and blindly overwriting the message. githooks(5) says: prepare-commit-msg This hook is invoked by git commit right after preparing the default log message, and before the editor is started. It takes one to three parameters. The first is the name of the file that contains the commit log message. The second is the source of the commit message, and can be: message (if a -m or -F option was given); template (if a -t option was given or the configuration option commit.template is set); merge (if the commit is a merge or a .git/MERGE_MSG file exists); squash (if a .git/SQUASH_MSG file exists); or commit, followed by a commit SHA1 (if a -c, -C or --amend option was given). If the exit status is non-zero, git commit will abort. The purpose of the hook is to edit the message file in place, and it is not suppressed by the --no-verify option. A non-zero exit means a failure of the hook and aborts the commit. It should not be used as replacement for pre-commit hook. Your problem is that your hook script is not checking $2 so it is overwriting the message even when you do not want to do so. -- 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: intend-to-edit flag
On Thu, Jul 04, 2013 at 08:10:07PM +0200, Ævar Arnfjörð Bjarmason wrote: On Thu, Jul 4, 2013 at 7:56 PM, Thomas Koch tho...@koch.ro wrote: we're evaluating Git to be used in our companies Tool. But a hard requirement is the possibility to set an intend-to-edit flag on a file (better path). Notice that I did not use the word lock! :-) One easy implementation might be a special branch XYZ-locks that contains an empty blob for every flagged file. So our tool just needs to check, whether a blob exists for the path that's intended to edit, tries to push a commit that touches the file and only allows editing if the push succeeds. In my experience everyone who thinks this is a hard requirement is wrong. I completely agree with this. Having said that, if you're looking at using Gitolite (which you should if you're hosing your own repositories and not using some other hosting solution), there is a lock command [1]. Note that this cannot stop you committing changes to locked files locally but it does stop you pushing changes to the central repository that touch locked files. [1] http://gitolite.com/gitolite/locking.html -- 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: Review of git multimail
On Wed, Jul 03, 2013 at 10:02:34AM +0200, Michael Haggerty wrote: On 07/03/2013 12:21 AM, Junio C Hamano wrote: Ramkumar Ramachandra artag...@gmail.com writes: def get(self, name, default=''): try: values = self._split(read_git_output( ['config', '--get', '--null', '%s.%s' % (self.section, name)], env=self.env, keepends=True, )) Wait, what is the point of using --null and then splitting by hand using a poorly-defined static method? Why not drop the --null and splitlines() as usual? You may actually have spotted a bug or misuse of --get here. With this sample configuration: $ cat sample \EOF [a] one = value one = another [b] one = value\nanother EOF A script cannot differentiate between them without using '--null'. $ git config -f sample --get-all a.one $ git config -f sample --get-all b.one But that matters only when you use --get-all, not --get. If this method wants to make sure that the user did not misuse a.one as a multi-valued configuration variable, use of --null --get-all followed by checking how many items the command gives you back would be a way to do so. No, the code in question was a simple sanity check (i.e., mostly a check of my own sanity and understanding of git config behavior) preceding the information-losing next line return values[0]. If it had been meant as a check that the user hadn't misconfigured the system, then I wouldn't have used assert but rather raised a ConfigurationException with an explanatory message. I would be happy to add the checking that you described, but I didn't have the impression that it is the usual convention. Does code that wants a single value from the config usually verify that there is one-and-only-one value, or does it typically just do the equivalent of git config --get and use the returned (effectively the last) value? Doesn't git config --get return an error if there are multiple values? The answer is apparently no - I wrote the text below from git-config(1) and then checked the behaviour. This seems to be a regression in git-config (bisect running now). I think the correct answer is what's below, but it doesn't work like this in current Git: If you want a single value then I think it's normal to just read the output of git config and let it handle the error cases, without needing to split the result at all. I think there is a different issue in the except block following the code quoted at the top though - you will return default if a key happens to be multi-valued. The script should check the return code and raise a ConfigurationException if it is 2. -- 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: Review of git multimail
On Wed, Jul 03, 2013 at 09:29:02AM +0100, John Keeping wrote: Doesn't git config --get return an error if there are multiple values? The answer is apparently no - I wrote the text below from git-config(1) and then checked the behaviour. This seems to be a regression in git-config (bisect running now). Ah, that was an intentional change in commit 00b347d (git-config: do not complain about duplicate entries, 2012-10-23). So the issue is that the documentation was not updated when the behaviour was changed. -- 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] t4205: don't rely on en_US.UTF-8 locale existing
My system doesn't have the en_US.UTF-8 locale (or plain en_US), which
causes t4205 to fail by counting bytes instead of UTF-8 codepoints.
Instead of using sed for this, use Perl which behaves predictably
whatever locale is in use.
Signed-off-by: John Keeping j...@keeping.me.uk
---
This patch is on top of 'as/log-output-encoding-in-user-format'.
t/t4205-log-pretty-formats.sh | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 3cfb744..5864f5b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -20,9 +20,7 @@ commit_msg () {
# cut string, replace cut part with two dots
# $2 - chars count from the beginning of the string
# $3 - trailing chars
- # LC_ALL is set to make `sed` interpret . as a UTF-8 char not
a byte
- # as it does with C locale
- msg=$(echo $msg | LC_ALL=en_US.UTF-8 sed -e
s/^\(.\{$2\}\)$3/\1../)
+ msg=$(echo $msg | $PERL_PATH -CIO -pe s/^(.{$2})$3/\1../)
fi
echo $msg
}
@@ -205,7 +203,7 @@ test_expect_success 'left alignment formatting with ltrunc'
..sage two
..sage one
add bar Z
-$(commit_msg 0 .\{11\})
+$(commit_msg 0 .{11})
EOF
test_cmp expected actual
@@ -218,7 +216,7 @@ test_expect_success 'left alignment formatting with mtrunc'
mess.. two
mess.. one
add bar Z
-$(commit_msg 4 .\{11\})
+$(commit_msg 4 .{11})
EOF
test_cmp expected actual
--
1.8.3.1.747.g77f7d3a
--
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] t4205: don't rely on en_US.UTF-8 locale existing
On Wed, Jul 03, 2013 at 02:41:06PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
My system doesn't have the en_US.UTF-8 locale (or plain en_US), which
causes t4205 to fail by counting bytes instead of UTF-8 codepoints.
Instead of using sed for this, use Perl which behaves predictably
whatever locale is in use.
Signed-off-by: John Keeping j...@keeping.me.uk
---
This patch is on top of 'as/log-output-encoding-in-user-format'.
Thanks. I think Alexey is going to send incremental updates to the
topic so I won't interfere by applying this patch on top of the
version I have in my tree.
But I do agree that using Perl may be a workable solution.
An alternative might be not to use this cryptic 3-arg form of
commit_msg at all. They are used only for these three:
$(commit_msg 8 ..*$)
$(commit_msg 0 .\{11\})
$(commit_msg 4 .\{11\})
I somehow find them simply not readable, in order to figure out what
is going on.
Just using three variables to hold what are expected would be far
more portable and readable.
# anfänglich whatever it means.
sample_utf8_part=$(printf anf\303\244ng)
commit_msg () {
msg=initial. ${sample_utf8_part}lich;
if test -n $1
then
echo $msg | iconv -f utf-8 -t $1
else
echo $msg
fi
}
And then instead of writing in the expected test output.
$(commit_msg 8 ..*$)
$(commit_msg 0 .\{11\})
$(commit_msg 4 .\{11\})
we can just say
initial...
..an${sample_utf8_part}lich
init..lich
It is no worse than those cryptic 0, 4, 8 and 11 magic numbers we
see in the test, no?
That's probably better since we don't need to rely on some other tool
getting it right.
Alexey, will you incorporate this change in your incremental updates?
--
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: --follow is ignored when used with --reverse
On Fri, May 24, 2013 at 01:23:24AM +0200, Alois Mahdal wrote: Hello! This [has been reported][1] to this list about half a year ago but with no response so I'm not even sure if it's been acknowledged as bug. [1]: http://marc.info/?l=gitm=135215709307126q=raw When I use `git log --follow file` all is OK, but once I add `--reverse` to it, it no longer follows the file beyond renames. This makes it hard to query for when the file was really added, which I was trying to achieve with $ git -1 --reverse --follow several_times_renamed_file In my testing it actually seems to be worse than that. In git.git: $ git log --oneline builtin/clone.c | wc -l 99 $ git log --oneline --reverse builtin/clone.c | wc -l 99 $ git log --oneline --follow builtin/clone.c | wc -l 125 $ git log --oneline --follow --reverse builtin/clone.c | wc -l 3 So the combination of --reverse and --follow appears to have lost the majority of the commits! -- 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: --follow is ignored when used with --reverse
On Tue, Jul 02, 2013 at 11:51:42AM +0200, Thomas Rast wrote: Lukas Fleischer g...@cryptocrack.de writes: On Tue, Jul 02, 2013 at 10:19:36AM +0100, John Keeping wrote: [...] $ git log --oneline --follow builtin/clone.c | wc -l 125 $ git log --oneline --follow --reverse builtin/clone.c | wc -l 3 I just wanted to point out that it works fine when specifying the *original* file name (which kind of makes sense given that everything is done in reverse order): [...] However, that also doesn't seem to work for builtin/clone.c: $ git log --oneline --follow --reverse -- builtin-clone.c | wc -l 65 I'm pretty sure that is simply because --follow iis a horrible hack, known to be broken in many ways. I have it on my longer-term todo list to unify it with -L -M, which already does the Right Thing (more generally, not in the --reverse interaction, which it never occurred to me I should check). Interesting... this tells me that --reverse doesn't work the way I thought it did (although without any real evidence). Given how --reverse interacts with other options (like --max-count), I assumed it would generate the commit list first and then simply reverse it before display but it seems that this isn't what happens with --follow. I guess that makes sense to avoid calculating the diff twice but I suspect we have to pay that price to get correct output. -- 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: Review of git multimail
On Wed, Jul 03, 2013 at 12:53:39AM +0530, Ramkumar Ramachandra wrote:
class CommandError(Exception):
def __init__(self, cmd, retcode):
self.cmd = cmd
self.retcode = retcode
Exception.__init__(
self,
'Command %s failed with retcode %s' % (' '.join(cmd),
retcode,)
So cmd is a list.
class ConfigurationException(Exception):
pass
Dead code?
Huh? ConfigurationException is used elsewhere.
def read_git_output(args, input=None, keepends=False, **kw):
Read the output of a Git command.
return read_output(
['git', '-c', 'i18n.logoutputencoding=%s' % (ENCODING,)] + args,
input=input, keepends=keepends, **kw
)
Okay, although I'm wondering what i18n.logoutputencoding has to do with
anything.
Making sure the output is what's expected and not influenced by
environment variables?
def read_output(cmd, input=None, keepends=False, **kw):
if input:
stdin = subprocess.PIPE
else:
stdin = None
p = subprocess.Popen(
cmd, stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
**kw
)
(out, err) = p.communicate(input)
retcode = p.wait()
if retcode:
raise CommandError(cmd, retcode)
if not keepends:
out = out.rstrip('\n\r')
return out
Helper function that serves a single caller, read_git_output().
def read_git_lines(args, keepends=False, **kw):
Return the lines output by Git command.
Return as single lines, with newlines stripped off.
return read_git_output(args, keepends=True, **kw).splitlines(keepends)
Okay.
class Config(object):
def __init__(self, section, git_config=None):
Represent a section of the git configuration.
If git_config is specified, it is passed to git config in
the GIT_CONFIG environment variable, meaning that git config
will read the specified path rather than the Git default
config paths.
self.section = section
if git_config:
self.env = os.environ.copy()
self.env['GIT_CONFIG'] = git_config
else:
self.env = None
Okay.
@staticmethod
def _split(s):
Split NUL-terminated values.
words = s.split('\0')
assert words[-1] == ''
return words[:-1]
Ugh. Two callers of this poorly-defined static method: I wonder if
we'd be better off inlining it.
In what way poorly defined? Personally I'd make it _split_null at the
top level but it seems sensible.
def get(self, name, default=''):
try:
values = self._split(read_git_output(
['config', '--get', '--null', '%s.%s' % (self.section,
name)],
env=self.env, keepends=True,
))
Wait, what is the point of using --null and then splitting by hand
using a poorly-defined static method? Why not drop the --null and
splitlines() as usual?
assert len(values) == 1
When does this assert fail?
In can't, which is presumably why it's an assert - it checks that we're
processing the Git output as expected.
return values[0]
except CommandError:
return default
If you're emulating the dictionary get method, default=None. This is
not C, where all codepaths of the function must return the same type.
def get_bool(self, name, default=None):
try:
value = read_git_output(
['config', '--get', '--bool', '%s.%s' % (self.section,
name)],
env=self.env,
)
except CommandError:
return default
return value == 'true'
Correct. On success, return bool. On failure, return None.
def get_all(self, name, default=None):
Read a (possibly multivalued) setting from the configuration.
Return the result as a list of values, or default if the name
is unset.
try:
return self._split(read_git_output(
['config', '--get-all', '--null', '%s.%s' % (self.section,
name)],
env=self.env, keepends=True,
))
except CommandError, e:
CommandError as e?
Not before Python 2.6.
if e.retcode == 1:
What does this cryptic retcode mean?
It mirrors subprocess.CalledProcessError, retcode is the return code of
the process.
return default
else:
raise
raise what?
The current exception - this is pretty idiomatic Python.
You've instantiated the Config class in two places: user and
multimailhook sections. Considering that you're going to read all the
keys in that section, why not --get-regexp, pre-load the configuration
into a dictionary and refer to that instead of
Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jun 28, 2013 at 03:41:34PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: Here, git pull . branch1 is merely saying I want to integrate the work on my current branch with that of branch1 without saying how that integration wants to happen. The change that I think is important is that the bring my branch up-to-date operation should force the user to choose what to do if the branch does not fast-forward to its upstream. If that was spelled git update then having git pull perform a merge would be fine, but we spell this operation as git pull so the change needs to happen there. I am not sure I quite get what you want to say with git update, and I am not sure if I necessarily want to know---I do not think we would want to add yet another command that DWIMs for certain _I_, that may not match newbie expectations. I wasn't proposing any new command, I was trying to express the operation that users coming from non-distributed VCSs want to perform (which is called update in svn). The problem is that a DVCS operates in a completely different way and a lot of users do not seem to want to learn the difference but simply try to map the existing commands that they know onto Git commands ([1] is the top result for svn commands to git on Google and maps svn update straight to git pull). [1] http://git.or.cz/course/svn.html I don't think git pull remote branch falls into the same category as plain git pull so I'm not convinced that defaulting to merge there is unreasonable. The original message about this [1] did talk about only git pull with no arguments. If you want to limit the scope to only git pull (without any command line argument), I actually do not have strong preference for or against it either way. Perhaps a follow-up patch to be squashed? I remember looking at this a few weeks ago and being concerned that it's impossible to tell what options you actually have in git-pull because it just invokes 'git fetch $@' and git-pull(1) does advertise a number of fetch options. It may be that test $# = 0 is good enough, but ideally I want to test for non-option arguments. I can't see a way of doing this without putting knowledge of all of the fetch options in git-pull so that we can handle options with arguments correctly. -- 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/2] completion: handle unstuck form of base git options
Hi Junio, I don't think you've picked this up. Are you expecting a
re-roll or did it just get lost in the noise?
On Sat, Jun 22, 2013 at 12:25:17PM +0100, John Keeping wrote:
git-completion.bash's parsing of the command name relies on everything
preceding it starting with '-' unless it is the -c option. This
allows users to use the stuck form of --work-tree=path and
--namespace=path but not the unstuck forms --work-tree path and
--namespace path. Fix this.
Similarly, the completion only handles the stuck form --git-dir=path
and not --git-dir path, so fix this as well.
Signed-off-by: John Keeping j...@keeping.me.uk
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c3bafe..8fbf941 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2492,9 +2492,10 @@ __git_main ()
i=${words[c]}
case $i in
--git-dir=*) __git_dir=${i#--git-dir=} ;;
+ --git-dir) ((c++)) ; __git_dir=${words[c]} ;;
--bare) __git_dir=. ;;
--help) command=help; break ;;
- -c) c=$((++c)) ;;
+ -c|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command=$i; break ;;
esac
--
1.8.3.1.676.gaae6535
--
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] pull: require choice between rebase/merge on non-fast-forward pull
On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote: Because letting a trivial merge automatically handled by Git is so easy with git pull, a person who is new to Git may not realize that the project s/he is interacting with may prefer rebase workflow. Add a safety valve to fail git pull that is not a fast-forward until/unless the user expressed her preference between the two. Those who want the existing behaviour could just do git config --global pull.rebase false once, and they'd not even notice. http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326 for a full discussion. The fallout from this change to test suite is not very pretty, though. Signed-off-by: Junio C Hamano gits...@pobox.com --- [snip] diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index c56a77d..af02c6d 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -79,7 +79,7 @@ test_expect_success \ test_expect_success \ 'merge-setup part 3' \ -'git pull . branch1' +'git pull --merge . branch1' I think the --merge should be implied here because the suer has specified an explicit remote and branch. Similarly, if --ff, --no-ff or --ff-only are given then we can infer --merge in the absence of any other configuration. However, when I looked at doing this I decided that it would be difficult to get that ideal behaviour without rewriting git-pull as a builtin. -- 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] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jun 28, 2013 at 10:22:57AM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: test_expect_success \ 'merge-setup part 3' \ -'git pull . branch1' +'git pull --merge . branch1' I think the --merge should be implied here because the suer has specified an explicit remote and branch. The whole point of the topic is It used to be that when you said 'git pull' and did not tell us your preferred way to integrate your work and work by the others', we default to merging, but we no longer do so---you have to choose. Here, git pull . branch1 is merely saying I want to integrate the work on my current branch with that of branch1 without saying how that integration wants to happen. The change that I think is important is that the bring my branch up-to-date operation should force the user to choose what to do if the branch does not fast-forward to its upstream. If that was spelled git update then having git pull perform a merge would be fine, but we spell this operation as git pull so the change needs to happen there. I don't think git pull remote branch falls into the same category as plain git pull so I'm not convinced that defaulting to merge there is unreasonable. The original message about this [1] did talk about only git pull with no arguments. Even though, as an old timer, I find it mildly irritating that we now have to be explicit in these tests, this change is in line with the spirit of the topic. If we didn't have to change this example and the pull silently succeeded without complaining, we achieved nothing. I disagree that we would have achieved nothing. New users will not be using explicit arguments to git-pull when just trying to bring a branch up-to-date. [1] http://article.gmane.org/gmane.comp.version-control.git/225240 -- 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] am: replace uses of --resolved with --continue
On Wed, Jun 26, 2013 at 11:06:41PM +0300, Kevin Bracey wrote:
git am was previously modified to provide --continue for consistency
with rebase, merge etc, and the documentation changed to showing
--continue as the primary form.
Complete the work by replacing remaining uses of --resolved by
--continue, most notably in suggested command reminders.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
Documentation/git-am.txt | 4 ++--
Documentation/user-manual.txt | 2 +-
git-am.sh | 8
t/t7512-status-help.sh| 4 ++--
wt-status.c | 2 +-
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 5bbe7b6..54d8461 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -132,7 +132,7 @@ default. You can use `--no-utf8` to override this.
--resolvemsg=msg::
When a patch failure occurs, msg will be printed
to the screen before exiting. This overrides the
- standard message informing you to use `--resolved`
+ standard message informing you to use `--continue`
or `--skip` to handle the failure. This is solely
for internal use between 'git rebase' and 'git am'.
@@ -176,7 +176,7 @@ aborts in the middle. You can recover from this in one
of two ways:
. hand resolve the conflict in the working directory, and update
the index file to bring it into a state that the patch should
- have produced. Then run the command with the '--resolved' option.
+ have produced. Then run the command with the '--continue' option.
It isn't new in this patch, but there is an inconsistency in the quoting
of the options here. In the previous hunk we use backticks but here it
uses SQs.
The documentation isn't at all consistent on this, but
backticks seem to be the preferred style (there are some false positives
in both counts but this gives a good indication):
$ git grep '-- -- Documentation/ | wc -l
186
$ git grep '`--' -- Documentation/ | wc -l
487
The command refuses to process new mailboxes until the current
operation is finished, so if you decide to start over from scratch,
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..8218cf9 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1835,7 +1835,7 @@ Once the index is updated with the results of the
conflict
resolution, instead of creating a new commit, just run
-
-$ git am --resolved
+$ git am --continue
-
and Git will create the commit for you and continue applying the
diff --git a/git-am.sh b/git-am.sh
index 9f44509..7ea40fe 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -6,7 +6,7 @@ SUBDIRECTORY_OK=Yes
OPTIONS_KEEPDASHDASH=
OPTIONS_SPEC=\
git am [options] [(mbox|Maildir)...]
-git am [options] (--resolved | --skip | --abort)
+git am [options] (--continue | --skip | --abort)
--
i,interactive run interactively
b,binary* (historical option -- no-op)
@@ -102,7 +102,7 @@ stop_here_user_resolve () {
printf '%s\n' $resolvemsg
stop_here $1
fi
-eval_gettextln When you have resolved this problem, run \\$cmdline
--resolved\.
+eval_gettextln When you have resolved this problem, run \\$cmdline
--continue\.
If you prefer to skip this patch, run \\$cmdline --skip\ instead.
To restore the original branch and stop patching, run \\$cmdline --abort\.
@@ -523,7 +523,7 @@ Use \git am --abort\ to remove it.)
esac
fi
- # Make sure we are not given --skip, --resolved, nor --abort
+ # Make sure we are not given --skip, --continue, nor --abort
test $skip$resolved$abort = ||
die $(gettext Resolve operation not in progress, we are not
resuming.)
@@ -670,7 +670,7 @@ do
# - patch is the patch body.
#
# When we are resuming, these files are either already prepared
- # by the user, or the user can tell us to do so by --resolved flag.
+ # by the user, or the user can tell us to do so by --continue flag.
case $resume in
'')
if test -f $dotest/rebasing
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 4f09bec..bd8aab0 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -510,7 +510,7 @@ test_expect_success 'status in an am session: file
already exists' '
cat expected -\EOF
# On branch am_already_exists
# You are in the middle of an am session.
- # (fix conflicts and then run git am --resolved)
+ # (fix conflicts and then run git am --continue)
# (use git am --skip to skip this patch)
# (use git am --abort to restore the original branch)
#
@@ -532,7 +532,7 @@ test_expect_success 'status in an am session: file
Re: [PATCH] Do not call built-in aliases from scripts
On Thu, Jun 27, 2013 at 11:05:09AM -0700, Junio C Hamano wrote: John Szakmeister j...@szakmeister.net writes: On Thu, Jun 27, 2013 at 1:27 PM, Junio C Hamano gits...@pobox.com wrote: [snip] diff --git a/git-am.sh b/git-am.sh index 9f44509..ad67194 100755 --- a/git-am.sh +++ b/git-am.sh @@ -16,8 +16,8 @@ s,signoff add a Signed-off-by line to the commit message u,utf8 recode into utf8 (default) k,keep pass -k flag to git-mailinfo keep-non-patch pass -b flag to git-mailinfo -keep-cr pass --keep-cr flag to git-mailsplit for mbox format -no-keep-cr do not pass --keep-cr flag to git-mailsplit independent of am.keepcr +keep-cr pass --keep-cr flag to git mailsplit for mbox format +no-keep-cr do not pass --keep-cr flag to git mailsplit independent of am.keepcr c,scissors strip everything before a scissors line whitespace= pass it through git-apply ignore-space-change pass it through git-apply As you were saying yourself, we tell users to prefer the git foo form, so we should also do so in the git am option help, IMHO. What does the above change to the options-help have anything to do with that theme? It does not seem to say anything about git foo vs git-foo? I initially missed it too, but `git-mailsplit` changed to `git mailsplit` in the help. Ahh, OK. That is rendered differently though, I don't think just having the plain text git command is as useful. It should either use the hyphenated form or enclose the text in quotes. -- 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: fast-import bug?
On Sat, Jun 22, 2013 at 07:16:48PM -0700, Dave Abrahams wrote:
on Sat Jun 22 2013, John Keeping john-AT-keeping.me.uk wrote:
On Fri, Jun 21, 2013 at 02:21:47AM -0700, Dave Abrahams wrote:
The docs for fast-import seem to imply that I can use ls to get the
SHA1 of a commit for which I have a mark:
Reading from a named tree
The dataref can be a mark reference (:idnum) or the full
40-byte
SHA-1 of a Git tag, commit, or tree object, preexisting or
waiting to
be written. The path is relative to the top level of the tree
named by
dataref.
'ls' SP dataref SP path LF
See filemodify above for a detailed description of path.
Output uses the same format as git ls-tree tree -- path:
mode SP ('blob' | 'tree' | 'commit') SP dataref HT path LF
The dataref represents the blob, tree, or commit object at path
and
^^
can be used in later cat-blob, filemodify, or ls commands.
but I can't get it to work. It's not entirely clear it's supposed to
work. What path would I pass? Passing an empty path simply causes git
to report missing .
Which version of Git are you using?
,[ git --version ]
| git version 1.8.3.1
`
I just tried this and get the error
fatal: Empty path component found in input,
I get that too.
which seems to be from commit 178e1de (fast-import: don't allow 'ls'
of path with empty components, 2012-03-09), which is included in Git
1.7.9.5.
Yes, that's at least part of the issue. I notice git-fast-import
rejects the root path for other commands, e.g. when used as the
source of a filecopy we get the same issue. I also note that the docs
don't make it clear that quoting the path is mandatory if it might turn
out to be empty.
Interesting. There are two places that can produce this error message,
tree_content_get and tree_content_set, but I wonder if this means that
tree_content_get should not be doing this check. The two places that
call it are:
1) parse_ls as discussed here
2) file_change_cr which deals with file copy and rename.
My patch in the previous message only changes the behaviour for the
parse_ls case, but it seems that you have a valid use case for removing
this check in the file_change_cr case as well.
I also note that the docs
don't make it clear that quoting the path is mandatory if it might turn
out to be empty.
That's not quite the case. It looks to me like quoting the path is
mandatory if no dataref is given, and indeed the documentation says:
Reading from the active commit
This form can only be used in the middle of a commit. The path
names a directory entry within fast-import’s active commit. The
path must be quoted in this case.
'ls' SP path LF
It seems to be slightly more complicated than that though, because after
allowing empty trees I get the missing message for the root tree.
Yeah, I've tried to patch Git to solve this but ran into that problem
and gave up.
This seems to be because its mode is 0 and not S_IFDIR.
Aha.
With the patch below, things are working as I expect
Awesome; works for me, too!
but I don't understand why the mode of the root is not set correctly
at this point. Perhaps someone more familiar with fast-import will
have some insight...
Yeah... there's no bug tracker for Git, right? So if nobody pays
attention to this thread, the problem will persist?
Yes, but I don't see that happening particularly often. In the worst
case issues are normally documented by a failing test case.
In this case, I think I do now understand why the mode is 0: in parse_ls
a new tree object is created and the SHA1 of the original is copied in
but the mode is left blank; clearly this should be set to S_IFDIR when
the SHA1 is non-null.
I think the patch I now have is correct (and addresses the copy from
root scenario), but I need to spend some time understanding t9300 so
that I can add suitable test cases.
-- 8 --
diff --git a/fast-import.c b/fast-import.c
index 23f625f..e2c9d50 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1629,7 +1629,8 @@ del_entry:
static int tree_content_get(
struct tree_entry *root,
const char *p,
- struct tree_entry *leaf)
+ struct tree_entry *leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1641,31 +1642,39 @@ static int tree_content_get(
n = slash1 - p;
else
n = strlen(p);
- if (!n)
+ if (!n !allow_root)
die(Empty path component found in input);
if (!root-tree)
load_tree(root);
+
+ if (!n) {
+ e = root;
+ goto found_entry;
+ }
+
t
Re: fast-import bug?
On Sun, Jun 23, 2013 at 07:19:25AM -0700, Dave Abrahams wrote: on Sun Jun 23 2013, John Keeping john-AT-keeping.me.uk wrote: In this case, I think I do now understand why the mode is 0: in parse_ls a new tree object is created and the SHA1 of the original is copied in but the mode is left blank; clearly this should be set to S_IFDIR when the SHA1 is non-null. I think the patch I now have is correct (and addresses the copy from root scenario), but I need to spend some time understanding t9300 so that I can add suitable test cases. t9300? t/t9300-fast-import.sh in Git's source tree - it's where the tests for fast-import live. Thanks; I'll try this one too. Thanks. I now have a patch series incorporating this which also adds a few tests for handling of empty paths. I'm sending it out in the next few minutes. -- 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 2/4] fast-import: set valid mode on root tree in ls
This prevents a failure later when we lift the restriction on ls with the empty path. Signed-off-by: John Keeping j...@keeping.me.uk --- fast-import.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fast-import.c b/fast-import.c index 23f625f..bdbadea 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3051,6 +3051,8 @@ static void parse_ls(struct branch *b) struct object_entry *e = parse_treeish_dataref(p); root = new_tree_entry(); hashcpy(root-versions[1].sha1, e-idx.sha1); + if (!is_null_sha1(root-versions[1].sha1)) + root-versions[1].mode = S_IFDIR; load_tree(root); if (*p++ != ' ') die(Missing space after tree-ish: %s, command_buf.buf); -- 1.8.3.1.676.gaae6535 -- 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 0/4] fast-import: handle empty paths better
This series addressed Dave Abraham's recent bug report [1] about using fast-import's ls command with an empty path. I also found a couple of other places that do not handle the empty path when it can reasonably be interpreted as meaning the root tree object, which are also fixed here. [1] http://article.gmane.org/gmane.comp.version-control.git/228586 John Keeping (4): t9300: document fast-import empty path issues fast-import: set valid mode on root tree in ls fast-import: allow ls or filecopy of the root tree fast-import: allow moving the root tree fast-import.c | 58 t/t9300-fast-import.sh | 65 ++ 2 files changed, 103 insertions(+), 20 deletions(-) -- 1.8.3.1.676.gaae6535 -- 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 1/4] t9300: document fast-import empty path issues
When given an empty path, fast-import sometimes reports missing
instead of using the root tree object. On top of this, for ls and
file copy (but not move) it dies with Empty path component found in
input.
Document this behaviour with failing test cases.
Reported-by: Dave Abrahams d...@boostpro.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
t/t9300-fast-import.sh | 65 ++
1 file changed, 65 insertions(+)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ac6f3b6..f4b9355 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1031,6 +1031,32 @@ test_expect_success \
git diff-tree -M -r M3^ M3 actual
compare_diff_raw expect actual'
+cat input INPUT_END
+commit refs/heads/M4
+committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+data COMMIT
+rename root
+COMMIT
+
+from refs/heads/M2^0
+R sub
+
+INPUT_END
+
+cat expect EOF
+:100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1
7123f7f44e39be127c5eb701e5968176ee9d78b1 R100 file2/oldf sub/file2/oldf
+:100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730
85df50785d62d3b05ab03d9cbf7e4a0b49449730 R100 file4 sub/file4
+:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc R100 i/am/new/to/you
sub/i/am/new/to/you
+:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 R100 newdir/exec.sh
sub/newdir/exec.sh
+:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 R100 newdir/interesting
sub/newdir/interesting
+EOF
+test_expect_failure \
+ 'M: rename root to subdirectory' \
+ 'git fast-import input
+git diff-tree -M -r M4^ M4 actual
+cat actual
+compare_diff_raw expect actual'
+
###
### series N
###
@@ -1227,6 +1253,29 @@ test_expect_success \
git diff-tree -C --find-copies-harder -r N4 N6 actual
compare_diff_raw expect actual'
+test_expect_failure \
+ 'N: copy root by path' \
+ 'cat expect -\EOF
+ :100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf
oldroot/file2/newf
+ :100644 100644 7123f7f44e39be127c5eb701e5968176ee9d78b1
7123f7f44e39be127c5eb701e5968176ee9d78b1 C100 file2/oldf
oldroot/file2/oldf
+ :100755 100755 85df50785d62d3b05ab03d9cbf7e4a0b49449730
85df50785d62d3b05ab03d9cbf7e4a0b49449730 C100 file4 oldroot/file4
+ :100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 C100 newdir/exec.sh
oldroot/newdir/exec.sh
+ :100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 C100 newdir/interesting
oldroot/newdir/interesting
+ EOF
+cat input -INPUT_END
+ commit refs/heads/N-copy-root-path
+ committer $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL $GIT_COMMITTER_DATE
+ data COMMIT
+ copy root directory by (empty) path
+ COMMIT
+
+ from refs/heads/branch^0
+ C oldroot
+ INPUT_END
+git fast-import input
+git diff-tree -C --find-copies-harder -r branch N-copy-root-path
actual
+compare_diff_raw expect actual'
+
test_expect_success \
'N: delete directory by copying' \
'cat expect -\EOF
@@ -2934,4 +2983,20 @@ test_expect_success 'S: ls with garbage after sha1 must
fail' '
test_i18ngrep space after tree-ish err
'
+###
+### series T (ls)
+###
+# Setup is carried over from series S.
+
+test_expect_failure 'T: ls root tree' '
+ sed -e s/Z\$// expect -EOF
+ 04 tree $(git rev-parse S^{tree}) Z
+ EOF
+ sha1=$(git rev-parse --verify S)
+ git fast-import --import-marks=marks -EOF actual
+ ls $sha1
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.8.3.1.676.gaae6535
--
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 3/4] fast-import: allow ls or filecopy of the root tree
Commit 178e1de (fast-import: don't allow 'ls' of path with empty
components, 2012-03-09) restricted paths which:
. contain an empty directory component (e.g. foo//bar is invalid),
. end with a directory separator (e.g. foo/ is invalid),
. start with a directory separator (e.g. /foo is invalid).
However, the implementation also caught the empty path, which should
represent the root tree. Relax this restriction so that the empty path
is explicitly allowed and refers to the root tree.
Reported-by: Dave Abrahams d...@boostpro.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
fast-import.c | 35 ++-
t/t9300-fast-import.sh | 4 ++--
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index bdbadea..e2c9d50 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1629,7 +1629,8 @@ del_entry:
static int tree_content_get(
struct tree_entry *root,
const char *p,
- struct tree_entry *leaf)
+ struct tree_entry *leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1641,31 +1642,39 @@ static int tree_content_get(
n = slash1 - p;
else
n = strlen(p);
- if (!n)
+ if (!n !allow_root)
die(Empty path component found in input);
if (!root-tree)
load_tree(root);
+
+ if (!n) {
+ e = root;
+ goto found_entry;
+ }
+
t = root-tree;
for (i = 0; i t-entry_count; i++) {
e = t-entries[i];
if (e-name-str_len == n !strncmp_icase(p,
e-name-str_dat, n)) {
- if (!slash1) {
- memcpy(leaf, e, sizeof(*leaf));
- if (e-tree
is_null_sha1(e-versions[1].sha1))
- leaf-tree = dup_tree_content(e-tree);
- else
- leaf-tree = NULL;
- return 1;
- }
+ if (!slash1)
+ goto found_entry;
if (!S_ISDIR(e-versions[1].mode))
return 0;
if (!e-tree)
load_tree(e);
- return tree_content_get(e, slash1 + 1, leaf);
+ return tree_content_get(e, slash1 + 1, leaf, 0);
}
}
return 0;
+
+found_entry:
+ memcpy(leaf, e, sizeof(*leaf));
+ if (e-tree is_null_sha1(e-versions[1].sha1))
+ leaf-tree = dup_tree_content(e-tree);
+ else
+ leaf-tree = NULL;
+ return 1;
}
static int update_branch(struct branch *b)
@@ -2415,7 +2424,7 @@ static void file_change_cr(struct branch *b, int rename)
if (rename)
tree_content_remove(b-branch_tree, s, leaf);
else
- tree_content_get(b-branch_tree, s, leaf);
+ tree_content_get(b-branch_tree, s, leaf, 1);
if (!leaf.versions[1].mode)
die(Path %s not in branch, s);
if (!*d) { /* C path/to/subdir */
@@ -3067,7 +3076,7 @@ static void parse_ls(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_get(root, p, leaf);
+ tree_content_get(root, p, leaf, 1);
/*
* A directory in preparation would have a sha1 of zero
* until it is saved. Save, for simplicity.
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f4b9355..04385a7 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1253,7 +1253,7 @@ test_expect_success \
git diff-tree -C --find-copies-harder -r N4 N6 actual
compare_diff_raw expect actual'
-test_expect_failure \
+test_expect_success \
'N: copy root by path' \
'cat expect -\EOF
:100755 100755 f1fb5da718392694d0076d677d6d0e364c79b0bc
f1fb5da718392694d0076d677d6d0e364c79b0bc C100 file2/newf
oldroot/file2/newf
@@ -2988,7 +2988,7 @@ test_expect_success 'S: ls with garbage after sha1 must
fail' '
###
# Setup is carried over from series S.
-test_expect_failure 'T: ls root tree' '
+test_expect_success 'T: ls root tree' '
sed -e s/Z\$// expect -EOF
04 tree $(git rev-parse S^{tree}) Z
EOF
--
1.8.3.1.676.gaae6535
--
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 4/4] fast-import: allow moving the root tree
Because fast-import.c::tree_content_remove does not check for the empty
path, it is not possible to move the root tree to a subdirectory.
Instead the error Path not in branch is produced (note the double
space where the empty path has been inserted).
Fix this by explicitly checking for the empty path and handling it.
Signed-off-by: John Keeping j...@keeping.me.uk
---
fast-import.c | 21 ++---
t/t9300-fast-import.sh | 2 +-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index e2c9d50..21db3fc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1568,7 +1568,8 @@ static int tree_content_set(
static int tree_content_remove(
struct tree_entry *root,
const char *p,
- struct tree_entry *backup_leaf)
+ struct tree_entry *backup_leaf,
+ int allow_root)
{
struct tree_content *t;
const char *slash1;
@@ -1583,6 +1584,12 @@ static int tree_content_remove(
if (!root-tree)
load_tree(root);
+
+ if (!*p allow_root) {
+ e = root;
+ goto del_entry;
+ }
+
t = root-tree;
for (i = 0; i t-entry_count; i++) {
e = t-entries[i];
@@ -1599,7 +1606,7 @@ static int tree_content_remove(
goto del_entry;
if (!e-tree)
load_tree(e);
- if (tree_content_remove(e, slash1 + 1, backup_leaf)) {
+ if (tree_content_remove(e, slash1 + 1, backup_leaf, 0))
{
for (n = 0; n e-tree-entry_count; n++) {
if
(e-tree-entries[n]-versions[1].mode) {
hashclr(root-versions[1].sha1);
@@ -2188,7 +2195,7 @@ static uintmax_t do_change_note_fanout(
}
/* Rename fullpath to realpath */
- if (!tree_content_remove(orig_root, fullpath, leaf))
+ if (!tree_content_remove(orig_root, fullpath, leaf, 0))
die(Failed to remove path %s, fullpath);
tree_content_set(orig_root, realpath,
leaf.versions[1].sha1,
@@ -2323,7 +2330,7 @@ static void file_change_m(struct branch *b)
/* Git does not track empty, non-toplevel directories. */
if (S_ISDIR(mode) !memcmp(sha1, EMPTY_TREE_SHA1_BIN, 20) *p) {
- tree_content_remove(b-branch_tree, p, NULL);
+ tree_content_remove(b-branch_tree, p, NULL, 0);
return;
}
@@ -2384,7 +2391,7 @@ static void file_change_d(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_remove(b-branch_tree, p, NULL);
+ tree_content_remove(b-branch_tree, p, NULL, 1);
}
static void file_change_cr(struct branch *b, int rename)
@@ -2422,7 +2429,7 @@ static void file_change_cr(struct branch *b, int rename)
memset(leaf, 0, sizeof(leaf));
if (rename)
- tree_content_remove(b-branch_tree, s, leaf);
+ tree_content_remove(b-branch_tree, s, leaf, 1);
else
tree_content_get(b-branch_tree, s, leaf, 1);
if (!leaf.versions[1].mode)
@@ -2530,7 +2537,7 @@ static void note_change_n(struct branch *b, unsigned char
*old_fanout)
}
construct_path_with_fanout(sha1_to_hex(commit_sha1), *old_fanout, path);
- if (tree_content_remove(b-branch_tree, path, NULL))
+ if (tree_content_remove(b-branch_tree, path, NULL, 0))
b-num_notes--;
if (is_null_sha1(sha1))
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 04385a7..31a770d 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1050,7 +1050,7 @@ cat expect EOF
:100755 100755 e74b7d465e52746be2b4bae983670711e6e66657
e74b7d465e52746be2b4bae983670711e6e66657 R100 newdir/exec.sh
sub/newdir/exec.sh
:100644 100644 fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791
fcf778cda181eaa1cbc9e9ce3a2e15ee9f9fe791 R100 newdir/interesting
sub/newdir/interesting
EOF
-test_expect_failure \
+test_expect_success \
'M: rename root to subdirectory' \
'git fast-import input
git diff-tree -M -r M4^ M4 actual
--
1.8.3.1.676.gaae6535
--
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] add--interactive: respect diff.algorithm
On Sun, Jun 23, 2013 at 12:19:05PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
+my $diff_algorithm = ($repo-config('diff.algorithm') or 'default');
+
my $use_readkey = 0;
my $use_termcap = 0;
my %term_escapes;
@@ -731,6 +733,9 @@ sub run_git_apply {
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
+if ($diff_algorithm ne default) {
+push @diff_cmd, --diff-algorithm=${diff_algorithm};
+}
This is not exactly sanitary for stash -p, whose DIFF element is
defined like so:
'stash' = {
DIFF = 'diff-index -p HEAD',
and you will end up appending an option after a non-option argument,
It may happen to be accepted by the command line parser which is
overly lax, but we would want to tighten it in the longer term.
As a band-aid, we could do something like the attached patch, but
for the longer term, we might need to rethink the way the tweaking
of the command line is done by $patch_mode_revision.
I originally used splice here then for some reason decided that just
appending the option was okay (I think I thought we were already
appending an option with $patch_mode_revision).
The patch below involves deeper Perl magic than I fully grok, but
wouldn't it be simpler to simply use the fact that the string is
command --options... and use:
splice @diff_cmd 1, 0, --diff-algorithm=${diff_algorithm};
-- 8 --
Subject: add -i: add extra options at the right place in diff command line
Appending --diff-algorithm=histogram at the end of canned command
line for various modes of diff is correct for most of them but not
for stash that has a non-option already wired in, like so:
'stash' = {
DIFF = 'diff-index -p HEAD',
Appending an extra option after non-option may happen to work due to
overly lax command line parser, but that is not something we should
rely on. Instead, splice in the extra argument immediately after a
'-p' option, which is an option to ask for textual diff output that
has to be in all variants.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-add--interactive.perl | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 5310959..b50551a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -730,11 +730,23 @@ sub run_git_apply {
return close $fh;
}
+# The command array must have a single -p to ask for output in the
+# patch form. Splice additional options immediately after it; we
+# should not be randomly appending them, as some of the canned command.
+# has non-option argument like HEAD already on it.
+
+sub splice_diff_options {
+ my $diff_cmd = shift;
+ @$diff_cmd = map {
+ ($_ eq '-p') ? ($_, @_) : $_;
+ } @$diff_cmd;
+}
+
sub parse_diff {
my ($path) = @_;
my @diff_cmd = split( , $patch_mode_flavour{DIFF});
if (defined $diff_algorithm) {
- push @diff_cmd, --diff-algorithm=${diff_algorithm};
+ splice_diff_options(\@diff_cmd,
--diff-algorithm=${diff_algorithm});
}
if (defined $patch_mode_revision) {
push @diff_cmd, $patch_mode_revision;
--
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: fast-import bug?
On Fri, Jun 21, 2013 at 02:21:47AM -0700, Dave Abrahams wrote:
The docs for fast-import seem to imply that I can use ls to get the
SHA1 of a commit for which I have a mark:
Reading from a named tree
The dataref can be a mark reference (:idnum) or the full
40-byte
SHA-1 of a Git tag, commit, or tree object, preexisting or waiting
to
be written. The path is relative to the top level of the tree
named by
dataref.
'ls' SP dataref SP path LF
See filemodify above for a detailed description of path.
Output uses the same format as git ls-tree tree -- path:
mode SP ('blob' | 'tree' | 'commit') SP dataref HT path LF
The dataref represents the blob, tree, or commit object at path and
^^
can be used in later cat-blob, filemodify, or ls commands.
but I can't get it to work. It's not entirely clear it's supposed to
work. What path would I pass? Passing an empty path simply causes git
to report missing .
Which version of Git are you using? I just tried this and get the error
fatal: Empty path component found in input, which seems to be from
commit 178e1de (fast-import: don't allow 'ls' of path with empty
components, 2012-03-09), which is included in Git 1.7.9.5.
It seems to be slightly more complicated than that though, because after
allowing empty trees I get the missing message for the root tree.
This seems to be because its mode is 0 and not S_IFDIR.
With the patch below, things are working as I expect but I don't
understand why the mode of the root is not set correctly at this point.
Perhaps someone more familiar with fast-import will have some insight...
-- 8 --
diff --git a/fast-import.c b/fast-import.c
index 23f625f..bcce651 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1626,6 +1626,15 @@ del_entry:
return 1;
}
+static void copy_tree_entry(struct tree_entry *dst, struct tree_entry *src)
+{
+ memcpy(dst, src, sizeof(*dst));
+ if (src-tree is_null_sha1(src-versions[1].sha1))
+ dst-tree = dup_tree_content(src-tree);
+ else
+ dst-tree = NULL;
+}
+
static int tree_content_get(
struct tree_entry *root,
const char *p,
@@ -1651,11 +1660,7 @@ static int tree_content_get(
e = t-entries[i];
if (e-name-str_len == n !strncmp_icase(p,
e-name-str_dat, n)) {
if (!slash1) {
- memcpy(leaf, e, sizeof(*leaf));
- if (e-tree
is_null_sha1(e-versions[1].sha1))
- leaf-tree = dup_tree_content(e-tree);
- else
- leaf-tree = NULL;
+ copy_tree_entry(leaf, e);
return 1;
}
if (!S_ISDIR(e-versions[1].mode))
@@ -3065,7 +3070,11 @@ static void parse_ls(struct branch *b)
die(Garbage after path in: %s, command_buf.buf);
p = uq.buf;
}
- tree_content_get(root, p, leaf);
+ if (!*p) {
+ copy_tree_entry(leaf, root);
+ leaf.versions[1].mode = S_IFDIR;
+ } else
+ tree_content_get(root, p, leaf);
/*
* A directory in preparation would have a sha1 of zero
* until it is saved. Save, for simplicity.
--
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 2/2] completion: learn about --man-path
Signed-off-by: John Keeping j...@keeping.me.uk --- contrib/completion/git-completion.bash | 2 ++ t/t9902-completion.sh | 1 + 2 files changed, 3 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8fbf941..c3290af 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2513,11 +2513,13 @@ __git_main () --exec-path --exec-path= --html-path + --man-path --info-path --work-tree= --namespace= --no-replace-objects --help + -c ;; *) __git_compute_porcelain_commands diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh index 81a1657..14d605a 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -231,6 +231,7 @@ test_expect_success 'double dash git itself' ' --exec-path Z --exec-path= --html-path Z + --man-path Z --info-path Z --work-tree= --namespace= -- 1.8.3.1.676.gaae6535 -- 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 1/2] completion: handle unstuck form of base git options
git-completion.bash's parsing of the command name relies on everything
preceding it starting with '-' unless it is the -c option. This
allows users to use the stuck form of --work-tree=path and
--namespace=path but not the unstuck forms --work-tree path and
--namespace path. Fix this.
Similarly, the completion only handles the stuck form --git-dir=path
and not --git-dir path, so fix this as well.
Signed-off-by: John Keeping j...@keeping.me.uk
---
contrib/completion/git-completion.bash | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6c3bafe..8fbf941 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2492,9 +2492,10 @@ __git_main ()
i=${words[c]}
case $i in
--git-dir=*) __git_dir=${i#--git-dir=} ;;
+ --git-dir) ((c++)) ; __git_dir=${words[c]} ;;
--bare) __git_dir=. ;;
--help) command=help; break ;;
- -c) c=$((++c)) ;;
+ -c|--work-tree|--namespace) ((c++)) ;;
-*) ;;
*) command=$i; break ;;
esac
--
1.8.3.1.676.gaae6535
--
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] help: introduce man.viewer = eman
On Sat, Jun 22, 2013 at 05:13:29PM +0530, Ramkumar Ramachandra wrote:
Corresponding to woman.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Documentation/git-help.txt | 3 +++
builtin/help.c | 11 ---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index b21e9d7..0cb4c46 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -104,6 +104,9 @@ The 'man.viewer' config variable will be checked if the
'man' format
is chosen. The following values are currently supported:
* man: use the 'man' program as usual,
+* eman: use 'emacsclient' to launch the man mode in emacs
+(this only works starting with emacsclient versions 22), on systems
+with man,
* woman: use 'emacsclient' to launch the woman mode in emacs
(this only works starting with emacsclient versions 22),
* konqueror: use 'kfmclient' to open the man page in a new konqueror
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..7cb44e0 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -120,7 +120,7 @@ static int check_emacsclient_version(void)
return 0;
}
-static void exec_woman_emacs(const char *path, const char *page)
+static void exec_woman_emacs(const char *path, const char *page, int eman)
{
if (!check_emacsclient_version()) {
/* This works only with emacsclient version = 22. */
@@ -128,7 +128,10 @@ static void exec_woman_emacs(const char *path, const
char *page)
if (!path)
path = emacsclient;
- strbuf_addf(man_page, (woman \%s\), page);
+ if (eman)
+ strbuf_addf(man_page, (man \%s\), page);
+ else
+ strbuf_addf(man_page, (woman \%s\), page);
Would it be nicer to pass a string in here instead of a flag? Then this
becomes:
strbuf_addf(man_page, (%s \%s\), command, page);
You should probably also rename this function to exec_emacsclient or
something as well now that it doesn't just launch woman.
execlp(path, emacsclient, -e, man_page.buf, (char *)NULL);
warning(_(failed to exec '%s': %s), path, strerror(errno));
}
@@ -341,8 +344,10 @@ static void exec_viewer(const char *name, const char
*page)
if (!strcasecmp(name, man))
exec_man_man(info, page);
+ else if (!strcasecmp(name, eman))
+ exec_woman_emacs(info, page, 1);
else if (!strcasecmp(name, woman))
- exec_woman_emacs(info, page);
+ exec_woman_emacs(info, page, 0);
else if (!strcasecmp(name, konqueror))
exec_man_konqueror(info, page);
else if (info)
--
1.8.3.1.487.g3e7a5b4.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
Re: [Request] Git reset should be able to ignore file permissions
On Tue, Jun 18, 2013 at 03:25:22PM +0200, Alexander Nestorov wrote: Recently I had to write some automation scripts and I found that git reset --hard actually restores each file's permissions. That is causing both the created and the last-modified dates of the file to get changed to the time of the git reset. This behavior is easy to demonstrate: echo test myfile chmod 777 myfile git add myfile git commit -m Test git push chmod 775 myfile git reset --hard origin/master After the git reset --hard command, the entire file was checkout-ed. Instead, git should be able to check if the content of the file changed and only if it did, check it out. Does git reset --keep behave in the same way? I would expect it to leave permissions as they were. -- 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 1/6] t7401: make indentation consistent
Only leading whitespace is changed in this patch. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7401-submodule-summary.sh | 80 ++-- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 30b429e..c328726 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -76,8 +76,8 @@ head3=$( ) test_expect_success 'modified submodule(backward)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head2...$head3 (2): Add foo3 Add foo2 @@ -89,8 +89,8 @@ EOF head4=$(add_file sm1 foo4 foo5) head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD) test_expect_success 'modified submodule(backward and forward)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head2...$head4 (4): Add foo5 Add foo4 @@ -102,15 +102,15 @@ EOF test_expect_success '--summary-limit' -git submodule summary -n 3 actual -cat expected -EOF + git submodule summary -n 3 actual + cat expected -EOF * sm1 $head2...$head4 (4): Add foo5 Add foo4 Add foo3 EOF -test_cmp expected actual + test_cmp expected actual commit_file sm1 @@ -122,8 +122,8 @@ rm -f sm1 mv sm1-bak sm1 test_expect_success 'typechanged submodule(submodule-blob), --cached' -git submodule summary --cached actual -cat expected -EOF + git submodule summary --cached actual + cat expected -EOF * sm1 $head4(submodule)-$head5(blob) (3): Add foo5 @@ -132,59 +132,59 @@ EOF test_expect_success 'typechanged submodule(submodule-blob), --files' -git submodule summary --files actual -cat expected -EOF + git submodule summary --files actual + cat expected -EOF * sm1 $head5(blob)-$head4(submodule) (3): Add foo5 EOF -test_i18ncmp actual expected + test_i18ncmp actual expected rm -rf sm1 git checkout-index sm1 test_expect_success 'typechanged submodule(submodule-blob)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head4(submodule)-$head5(blob): EOF -test_i18ncmp actual expected + test_i18ncmp actual expected rm -f sm1 test_create_repo sm1 head6=$(add_file sm1 foo6 foo7) test_expect_success 'nonexistent commit' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head4...$head6: Warn: sm1 doesn't contain commit $head4_full EOF -test_i18ncmp actual expected + test_i18ncmp actual expected commit_file test_expect_success 'typechanged submodule(blob-submodule)' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head5(blob)-$head6(submodule) (2): Add foo7 EOF -test_i18ncmp expected actual + test_i18ncmp expected actual commit_file sm1 rm -rf sm1 test_expect_success 'deleted submodule' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head6...000: EOF -test_cmp expected actual + test_cmp expected actual test_create_repo sm2 @@ -192,43 +192,43 @@ head7=$(add_file sm2 foo8 foo9) git add sm2 test_expect_success 'multiple submodules' -git submodule summary actual -cat expected -EOF + git submodule summary actual + cat expected -EOF * sm1 $head6...000: * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual test_expect_success 'path filter' -git submodule summary sm2 actual -cat expected -EOF + git submodule summary sm2 actual + cat expected -EOF * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual commit_file sm2 test_expect_success 'given commit' -git submodule summary HEAD^ actual -cat expected -EOF + git submodule summary HEAD^ actual + cat expected -EOF * sm1 $head6...000: * sm2 000...$head7 (2): Add foo9 EOF -test_cmp expected actual + test_cmp expected actual test_expect_success '--for-status' -git submodule summary --for-status HEAD^ actual -test_i18ncmp actual - EOF + git submodule summary --for-status HEAD^ actual + test_i18ncmp actual - EOF # Submodule changes to be committed: # # * sm1 $head6...000: @@ -240,14 +240,14 @@ EOF test_expect_success 'fail when using --files together with --cached' -test_must_fail git submodule summary --files --cached + test_must_fail git submodule summary --files --cached
[PATCH v4 0/6] submodule: drop the top-level requirement
Changes since v3: * There are four new patches, three of which are style fixes for existing tests and one fixes an existing error message to return a more accurate path when recursing. * You now cannot run git submodule add relative URL from a subdirectory. Because the interpretation of the URL changes depending on whether or not remote.origin.url is configured, I have decided to just ban this for now. If someone comes up with a sensible way to handle this then we can lift this restriction later. * The path variable exported in submodule foreach now uses the relative path and matches the sm_path variable. * I audited the code again and fixed a few more cases that weren't printing relative paths (notably submodule init and submodule foreach). * More tests. John Keeping (6): t7401: make indentation consistent t7403: modernize style t7403: add missing chaining submodule: show full path in error message rev-parse: add --prefix option submodule: drop the top-level requirement Documentation/git-rev-parse.txt | 16 ++ builtin/rev-parse.c | 24 ++- git-submodule.sh| 135 ++ t/t1513-rev-parse-prefix.sh | 96 ++ t/t7400-submodule-basic.sh | 80 + t/t7401-submodule-summary.sh| 116 +++- t/t7403-submodule-sync.sh | 388 ++-- t/t7406-submodule-update.sh | 15 ++ t/t7407-submodule-foreach.sh| 16 ++ 9 files changed, 673 insertions(+), 213 deletions(-) create mode 100755 t/t1513-rev-parse-prefix.sh -- 1.8.3.779.g691e267 -- 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/6] t7403: modernize style
Change the indentation to use tabs consistently and start content on the line after the paren opening a subshell. Also don't put a space in file and remove : from : file to be consistent with the majority of tests elsewhere. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7403-submodule-sync.sh | 316 +++--- 1 file changed, 183 insertions(+), 133 deletions(-) diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 94e26c4..38f6cc4 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -11,216 +11,266 @@ These tests exercise the git submodule sync subcommand. . ./test-lib.sh test_expect_success setup ' - echo file file + echo file file git add file test_tick git commit -m upstream git clone . super git clone super submodule - (cd submodule -git submodule add ../submodule sub-submodule -test_tick -git commit -m sub-submodule + ( + cd submodule + git submodule add ../submodule sub-submodule + test_tick + git commit -m sub-submodule ) - (cd super -git submodule add ../submodule submodule -test_tick -git commit -m submodule + ( + cd super + git submodule add ../submodule submodule + test_tick + git commit -m submodule ) git clone super super-clone - (cd super-clone git submodule update --init --recursive) + ( + cd super-clone + git submodule update --init --recursive + ) git clone super empty-clone - (cd empty-clone git submodule init) + ( + cd empty-clone + git submodule init + ) git clone super top-only-clone git clone super relative-clone - (cd relative-clone git submodule update --init --recursive) + ( + cd relative-clone + git submodule update --init --recursive + ) git clone super recursive-clone - (cd recursive-clone git submodule update --init --recursive) + ( + cd recursive-clone + git submodule update --init --recursive + ) ' test_expect_success 'change submodule' ' - (cd submodule -echo second line file -test_tick -git commit -a -m change submodule + ( + cd submodule + echo second line file + test_tick + git commit -a -m change submodule ) ' test_expect_success 'change submodule url' ' - (cd super -cd submodule -git checkout master -git pull + ( + cd super + cd submodule + git checkout master + git pull ) mv submodule moved-submodule - (cd moved-submodule -git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule -test_tick -git commit -a -m moved-sub-submodule + ( + cd moved-submodule + git config -f .gitmodules submodule.sub-submodule.url ../moved-submodule + test_tick + git commit -a -m moved-sub-submodule ) - (cd super -git config -f .gitmodules submodule.submodule.url ../moved-submodule -test_tick -git commit -a -m moved-submodule + ( + cd super + git config -f .gitmodules submodule.submodule.url ../moved-submodule + test_tick + git commit -a -m moved-submodule ) ' test_expect_success 'git submodule sync should update submodule URLs' ' - (cd super-clone -git pull --no-recurse-submodules -git submodule sync + ( + cd super-clone + git pull --no-recurse-submodules + git submodule sync ) - test -d $(cd super-clone/submodule -git config remote.origin.url + test -d $( + cd super-clone/submodule + git config remote.origin.url ) - test ! -d $(cd super-clone/submodule/sub-submodule -git config remote.origin.url + test ! -d $( + cd super-clone/submodule/sub-submodule + git config remote.origin.url ) - (cd super-clone/submodule -git checkout master -git pull + ( + cd super-clone/submodule + git checkout master + git pull ) - (cd super-clone -test -d $(git config submodule.submodule.url) + ( + cd super-clone + test -d $(git config submodule.submodule.url) ) ' test_expect_success 'git submodule sync --recursive
[PATCH v4 4/6] submodule: show full path in error message
When --recursive was added to submodule foreach in commit 15fc56a (git submodule foreach: Add --recursive to recurse into nested submodules, 2009-08-19), the error message when the script returns a non-zero status was not updated to contain $prefix to show the full path. Fix this. Signed-off-by: John Keeping j...@keeping.me.uk --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-submodule.sh b/git-submodule.sh index 79bfaac..bdb438b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -485,7 +485,7 @@ cmd_foreach() cmd_foreach --recursive $@ fi ) 3 3- || - die $(eval_gettext Stopping at '\$sm_path'; script returned non-zero status.) + die $(eval_gettext Stopping at '\$prefix\$sm_path'; script returned non-zero status.) fi done } -- 1.8.3.779.g691e267 -- 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 6/6] submodule: drop the top-level requirement
Use the new rev-parse --prefix option to process all paths given to the
submodule command, dropping the requirement that it be run from the
top-level of the repository.
Since the interpretation of a relative submodule URL depends on whether
or not remote.origin.url is configured, explicitly block relative URLs
in git submodule add when not at the top level of the working tree.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-submodule.sh | 135 ---
t/t7400-submodule-basic.sh | 80 +
t/t7401-submodule-summary.sh | 36
t/t7403-submodule-sync.sh| 72 +++
t/t7406-submodule-update.sh | 15 +
t/t7407-submodule-foreach.sh | 16 +
6 files changed, 319 insertions(+), 35 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index bdb438b..7756d81 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -14,10 +14,13 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name
name] [--reference re
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--recursive] [--] [path...]
OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
. git-sh-setup
. git-sh-i18n
. git-parse-remote
require_work_tree
+wt_prefix=$(git rev-parse --show-prefix)
+cd_to_toplevel
command=
branch=
@@ -106,12 +109,48 @@ resolve_relative_url ()
echo ${is_relative:+${up_path}}${remoteurl#./}
}
+# Resolve a path to be relative to another path. This is intended for
+# converting submodule paths when git-submodule is run in a subdirectory
+# and only handles paths where the directory separator is '/'.
+#
+# The output is the first argument as a path relative to the second argument,
+# which defaults to $wt_prefix if it is omitted.
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
+
+ result=${result}../
+ if test $curdir = ${curdir%/*}
+ then
+ curdir=
+ else
+ curdir=${curdir%/*}
+ fi
+ done
+
+ echo $result$target
+}
+
#
# Get submodule info for registered submodules
# $@ = path to limit submodule list
#
module_list()
{
+ eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
(
git ls-files --error-unmatch --stage -- $@ ||
echo unmatched pathspec exists
@@ -282,6 +321,7 @@ isnumber()
cmd_add()
{
# parse $args after submodule ... add.
+ reference_path=
while test $# -ne 0
do
case $1 in
@@ -298,11 +338,11 @@ cmd_add()
;;
--reference)
case $2 in '') usage ;; esac
- reference=--reference=$2
+ reference_path=$2
shift
;;
--reference=*)
- reference=$1
+ reference_path=${1#--reference=}
;;
--name)
case $2 in '') usage ;; esac
@@ -323,6 +363,14 @@ cmd_add()
shift
done
+ if test -n $reference_path
+ then
+ is_absolute_path $reference_path ||
+ reference_path=$wt_prefix$reference_path
+
+ reference=--reference=$reference_path
+ fi
+
repo=$1
sm_path=$2
@@ -335,9 +383,14 @@ cmd_add()
usage
fi
+ is_absolute_path $sm_path || sm_path=$wt_prefix$sm_path
+
# assure repo is absolute or relative to parent
case $repo in
./*|../*)
+ test -z $wt_prefix ||
+ die $(gettext Relative path can only be used from the
toplevel of the working tree)
+
# dereference source url relative to parent's url
realrepo=$(resolve_relative_url $repo) || exit
;;
@@ -471,21 +524,23 @@ cmd_foreach()
die_if_unmatched $mode
if test -e $sm_path/.git
then
- say $(eval_gettext Entering '\$prefix\$sm_path')
+ displaypath=$(relative_path $sm_path)
+ say $(eval_gettext Entering '\$prefix\$displaypath')
name=$(module_name $sm_path)
(
prefix=$prefix$sm_path/
clear_local_git_env
- # we make $path available to scripts ...
- path=$sm_path
cd $sm_path
[PATCH v4 3/6] t7403: add missing chaining
Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7403-submodule-sync.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 38f6cc4..bf90098 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -174,7 +174,7 @@ test_expect_success 'git submodule sync handles origin URL of the form foo/bar cd submodule #actual foo/submodule test $(git config remote.origin.url) = ../foo/submodule - ) + ) ( cd submodule/sub-submodule test $(git config remote.origin.url) != ../../foo/submodule @@ -191,7 +191,7 @@ test_expect_success 'git submodule sync --recursive propagates changes in orig cd submodule #actual foo/submodule test $(git config remote.origin.url) = ../foo/submodule - ) + ) ( cd submodule/sub-submodule test $(git config remote.origin.url) = ../../foo/submodule -- 1.8.3.779.g691e267 -- 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 5/6] rev-parse: add --prefix option
This makes 'git rev-parse' behave as if it were invoked from the
specified subdirectory of a repository, with the difference that any
file paths which it prints are prefixed with the full path from the top
of the working tree.
This is useful for shell scripts where we may want to cd to the top of
the working tree but need to handle relative paths given by the user on
the command line.
Signed-off-by: John Keeping j...@keeping.me.uk
---
Documentation/git-rev-parse.txt | 16 +++
builtin/rev-parse.c | 24 ---
t/t1513-rev-parse-prefix.sh | 96 +
3 files changed, 131 insertions(+), 5 deletions(-)
create mode 100755 t/t1513-rev-parse-prefix.sh
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 947d62f..993903c 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -59,6 +59,22 @@ OPTIONS
If there is no parameter given by the user, use `arg`
instead.
+--prefix arg::
+ Behave as if 'git rev-parse' was invoked from the `arg`
+ subdirectory of the working tree. Any relative filenames are
+ resolved as if they are prefixed by `arg` and will be printed
+ in that form.
++
+This can be used to convert arguments to a command run in a subdirectory
+so that they can still be used after moving to the top-level of the
+repository. For example:
++
+
+prefix=$(git rev-parse --show-prefix)
+cd $(git rev-parse --show-toplevel)
+eval set -- $(git rev-parse --sq --prefix $prefix $@)
+
+
--verify::
Verify that exactly one parameter is provided, and that it
can be turned into a raw 20-byte SHA-1 that can be used to
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f267a1d..de894c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char
*datestr)
show(buffer);
}
-static int show_file(const char *arg)
+static int show_file(const char *arg, int output_prefix)
{
show_default();
if ((filter (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
- show(arg);
+ if (output_prefix) {
+ const char *prefix = startup_info-prefix;
+ show(prefix_filename(prefix,
+prefix ? strlen(prefix) : 0,
+arg));
+ } else
+ show(arg);
return 1;
}
return 0;
@@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n
int cmd_rev_parse(int argc, const char **argv, const char *prefix)
{
int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
+ int output_prefix = 0;
unsigned char sha1[20];
const char *name = NULL;
@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
const char *arg = argv[i];
if (as_is) {
- if (show_file(arg) as_is 2)
+ if (show_file(arg, output_prefix) as_is 2)
verify_filename(prefix, arg, 0);
continue;
}
@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
as_is = 2;
/* Pass on the -- if we show anything but
files.. */
if (filter (DO_FLAGS | DO_REVS))
- show_file(arg);
+ show_file(arg, 0);
continue;
}
if (!strcmp(arg, --default)) {
@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
i++;
continue;
}
+ if (!strcmp(arg, --prefix)) {
+ prefix = argv[i+1];
+ startup_info-prefix = prefix;
+ output_prefix = 1;
+ i++;
+ continue;
+ }
if (!strcmp(arg, --revs-only)) {
filter = ~DO_NOREV;
continue;
@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
if (verify)
die_no_single_rev(quiet);
as_is = 1;
- if (!show_file(arg))
+ if (!show_file(arg, output_prefix))
continue;
verify_filename(prefix, arg, 1);
}
diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
new file mode 100755
index 000..87ec3ae
[PATCH 1/2] Documentation/Makefile: fix spaces around assignments
A simple style fix; no functional change. Signed-off-by: John Keeping j...@keeping.me.uk --- Nothing in maint..pu is touching this at the moment, so hopefully this is a good time to fix the whitespace here. Documentation/Makefile | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 62dbd9a..af3d8a4 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -31,11 +31,11 @@ MAN7_TXT += gittutorial.txt MAN7_TXT += gitworkflows.txt MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) -MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT)) -MAN_HTML=$(patsubst %.txt,%.html,$(MAN_TXT)) +MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT)) +MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT)) OBSOLETE_HTML = git-remote-helpers.html -DOC_HTML=$(MAN_HTML) $(OBSOLETE_HTML) +DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML) ARTICLES = howto-index ARTICLES += everyday @@ -74,35 +74,35 @@ SP_ARTICLES += technical/api-index DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES)) -DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT)) -DOC_MAN5=$(patsubst %.txt,%.5,$(MAN5_TXT)) -DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT)) +DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT)) +DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT)) +DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT)) -prefix?=$(HOME) -bindir?=$(prefix)/bin -htmldir?=$(prefix)/share/doc/git-doc -pdfdir?=$(prefix)/share/doc/git-doc -mandir?=$(prefix)/share/man -man1dir=$(mandir)/man1 -man5dir=$(mandir)/man5 -man7dir=$(mandir)/man7 -# DESTDIR= +prefix ?= $(HOME) +bindir ?= $(prefix)/bin +htmldir ?= $(prefix)/share/doc/git-doc +pdfdir ?= $(prefix)/share/doc/git-doc +mandir ?= $(prefix)/share/man +man1dir = $(mandir)/man1 +man5dir = $(mandir)/man5 +man7dir = $(mandir)/man7 +# DESTDIR = ASCIIDOC = asciidoc ASCIIDOC_EXTRA = MANPAGE_XSL = manpage-normal.xsl XMLTO = xmlto XMLTO_EXTRA = -INSTALL?=install +INSTALL ?= install RM ?= rm -f MAN_REPO = ../../git-manpages HTML_REPO = ../../git-htmldocs -infodir?=$(prefix)/share/info -MAKEINFO=makeinfo -INSTALL_INFO=install-info -DOCBOOK2X_TEXI=docbook2x-texi -DBLATEX=dblatex +infodir ?= $(prefix)/share/info +MAKEINFO = makeinfo +INSTALL_INFO = install-info +DOCBOOK2X_TEXI = docbook2x-texi +DBLATEX = dblatex ifndef PERL_PATH PERL_PATH = /usr/bin/perl endif -- 1.8.3.779.g691e267 -- 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 2/2] Documentation/Makefile: move infodir to be with other '*dir's
Signed-off-by: John Keeping j...@keeping.me.uk --- Documentation/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index af3d8a4..0cfdc36 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT)) prefix ?= $(HOME) bindir ?= $(prefix)/bin htmldir ?= $(prefix)/share/doc/git-doc +infodir ?= $(prefix)/share/info pdfdir ?= $(prefix)/share/doc/git-doc mandir ?= $(prefix)/share/man man1dir = $(mandir)/man1 @@ -98,7 +99,6 @@ RM ?= rm -f MAN_REPO = ../../git-manpages HTML_REPO = ../../git-htmldocs -infodir ?= $(prefix)/share/info MAKEINFO = makeinfo INSTALL_INFO = install-info DOCBOOK2X_TEXI = docbook2x-texi -- 1.8.3.779.g691e267 -- 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] mergetool--lib: refactor {diff,merge}_cmd logic
Instead of needing a wrapper to call the diff/merge command, simply
provide the diff_cmd and merge_cmd functions for user-specified tools in
the same way as we do for built-in tools.
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-mergetool--lib.sh | 82 ++-
1 file changed, 35 insertions(+), 47 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index e338be5..6a72106 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -114,6 +114,33 @@ valid_tool () {
test -n $cmd
}
+setup_user_tool () {
+ merge_tool_cmd=$(get_merge_tool_cmd $tool)
+ test -n $merge_tool_cmd || return 1
+
+ diff_cmd () {
+ ( eval $merge_tool_cmd )
+ status=$?
+ return $status
+ }
+
+ merge_cmd () {
+ trust_exit_code=$(git config --bool \
+ mergetool.$1.trustExitCode || echo false)
+ if test $trust_exit_code = false
+ then
+ touch $BACKUP
+ ( eval $merge_tool_cmd )
+ status=$?
+ check_unchanged
+ else
+ ( eval $merge_tool_cmd )
+ status=$?
+ fi
+ return $status
+ }
+}
+
setup_tool () {
tool=$1
@@ -142,15 +169,15 @@ setup_tool () {
if ! test -f $MERGE_TOOLS_DIR/$tool
then
- # Use a special return code for this case since we want to
- # source defaults even when an explicit tool path is
- # configured since the user can use that to override the
- # default path in the scriptlet.
- return 2
+ setup_user_tool
+ return $?
fi
# Load the redefined functions
. $MERGE_TOOLS_DIR/$tool
+ # Now let the user override the default command for the tool. If
+ # they have not done so then this will return 1 which we ignore.
+ setup_user_tool
if merge_mode ! can_merge
then
@@ -187,20 +214,7 @@ run_merge_tool () {
status=0
# Bring tool-specific functions into scope
- setup_tool $1
- exitcode=$?
- case $exitcode in
- 0)
- :
- ;;
- 2)
- # The configured tool is not a built-in tool.
- test -n $merge_tool_path || return 1
- ;;
- *)
- return $exitcode
- ;;
- esac
+ setup_tool $1 || return 1
if merge_mode
then
@@ -213,38 +227,12 @@ run_merge_tool () {
# Run a either a configured or built-in diff tool
run_diff_cmd () {
- merge_tool_cmd=$(get_merge_tool_cmd $1)
- if test -n $merge_tool_cmd
- then
- ( eval $merge_tool_cmd )
- status=$?
- return $status
- else
- diff_cmd $1
- fi
+ diff_cmd $1
}
# Run a either a configured or built-in merge tool
run_merge_cmd () {
- merge_tool_cmd=$(get_merge_tool_cmd $1)
- if test -n $merge_tool_cmd
- then
- trust_exit_code=$(git config --bool \
- mergetool.$1.trustExitCode || echo false)
- if test $trust_exit_code = false
- then
- touch $BACKUP
- ( eval $merge_tool_cmd )
- status=$?
- check_unchanged
- else
- ( eval $merge_tool_cmd )
- status=$?
- fi
- return $status
- else
- merge_cmd $1
- fi
+ merge_cmd $1
}
list_merge_tool_candidates () {
--
1.8.3.779.g691e267
--
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 ignore logic does not work as advertised
On Sat, Jun 15, 2013 at 06:18:46PM +0200, Thomas Koch wrote: I'm using vcsh[1] to have my dotfiles in GIT. With that I use a .gitignore file referenced by core.excludesfile that looks like this: # ignore everything by default * # but do not ignore emacs stuff !.emacs.d/ # but than again please ignore backup files inside the .emacs.d folder .emacs.d/backups Now I'd expect git status to show everything in .emacs.d but not to show .emacs.d/backups. However the .emacs.d/backups folder is still shown in git status. I'd say that this is not in line with the man page and might be considered a bug. Which version of Git are you using? You may be hitting a regression that was introduced in Git 1.8.3 and is fixed in Git 1.8.3.1. -- 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
