Re: [PATCH] remote-helpers: point at their upstream repositories
On Sat, May 17, 2014 at 12:25:30AM -0500, Felipe Contreras wrote: I agree with the line of reasoning you laid out in your email, especially: What a shock. Please stop with these unproductive and rude comments. I hadn't thought of the rename idea, and it would address the concerns I brought up. I do think obsolete is the wrong word, as it sends the wrong message. The helpers are not obsolete; it is our _copy_ of them that is. Originally you said you would respect if I wanted the code out for v2.0, I said I would like it out at some point, not necessarily in v2.0. Junio said he was fine with that, but the proposals above don't do that. Now it seems you are changing your mind and you are OK with the code remaining in. My concerns were with people not noticing the README. Removing the code entirely is the way I thought of to address that. Junio suggested another way, which I would also be fine with. And it seems like a friendlier way than removal to handle it for v2.0, if we are going to remove the code entirely post-v2.0. As before, if your desire is to have the code out for v2.0, then say so. I think we should respect such a request. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/10] replace: refactor checking ref validity
This will be useful in a following commit when we will
want to check if the ref already exists before we let the
user edit an object.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 0751804..3d6edaf 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -124,6 +124,25 @@ static int delete_replace_ref(const char *name, const char
*ref,
return 0;
}
+static void check_ref_valid(unsigned char object[20],
+ unsigned char prev[20],
+ char *ref,
+ int ref_size,
+ int force)
+{
+ if (snprintf(ref, ref_size,
+refs/replace/%s,
+sha1_to_hex(object)) ref_size - 1)
+ die(replace ref name too long: %.*s..., 50, ref);
+ if (check_refname_format(ref, 0))
+ die('%s' is not a valid ref name., ref);
+
+ if (read_ref(ref, prev))
+ hashclr(prev);
+ else if (!force)
+ die(replace ref '%s' already exists, ref);
+}
+
static int replace_object_sha1(const char *object_ref,
unsigned char object[20],
const char *replace_ref,
@@ -135,13 +154,6 @@ static int replace_object_sha1(const char *object_ref,
char ref[PATH_MAX];
struct ref_lock *lock;
- if (snprintf(ref, sizeof(ref),
-refs/replace/%s,
-sha1_to_hex(object)) sizeof(ref) - 1)
- die(replace ref name too long: %.*s..., 50, ref);
- if (check_refname_format(ref, 0))
- die('%s' is not a valid ref name., ref);
-
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
if (!force obj_type != repl_type)
@@ -151,10 +163,7 @@ static int replace_object_sha1(const char *object_ref,
object_ref, typename(obj_type),
replace_ref, typename(repl_type));
- if (read_ref(ref, prev))
- hashclr(prev);
- else if (!force)
- die(replace ref '%s' already exists, ref);
+ check_ref_valid(object, prev, ref, sizeof(ref), force);
lock = lock_any_ref_for_update(ref, prev, 0, NULL);
if (!lock)
--
1.9.rc0.17.g651113e
--
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 00/10] replace: add option to edit a Git object
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1. I added 6 more small patches to add tests, documentation and a few small improvements. Christian Couder (6): replace: make sure --edit results in a different object replace: refactor checking ref validity replace: die early if replace ref already exists replace: add tests for --edit replace: add --edit to usage string Documentation: replace: describe new --edit option Jeff King (4): replace: refactor command-mode determination replace: use OPT_CMDMODE to handle modes replace: factor object resolution out of replace_object replace: add --edit option Documentation/git-replace.txt | 15 ++- builtin/replace.c | 225 +- t/t6050-replace.sh| 29 ++ 3 files changed, 223 insertions(+), 46 deletions(-) -- 1.9.rc0.17.g651113e -- 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 05/10] replace: make sure --edit results in a different object
It's a bad idea to create a replace ref for an object that points to the original object itself. That's why we have to check if the result from editing the original object is a different object and error out if it isn't. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/replace.c b/builtin/replace.c index 3da1bae..0751804 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -275,6 +275,9 @@ static int edit_and_replace(const char *object_ref, int force) free(tmpfile); + if (!hashcmp(old, new)) + return error(new object is the same as the old one: '%s', sha1_to_hex(old)); + return replace_object_sha1(object_ref, old, replacement, new, force); } -- 1.9.rc0.17.g651113e -- 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 03/10] replace: factor object resolution out of replace_object
From: Jeff King p...@peff.net
As we add new options that operate on objects before
replacing them, we'll want to be able to feed raw sha1s
straight into replace_object. Split replace_object into the
object-resolution part and the actual replacement.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 29cf699..af40129 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const
char *ref,
return 0;
}
-static int replace_object(const char *object_ref, const char *replace_ref,
- int force)
+static int replace_object_sha1(const char *object_ref,
+ unsigned char object[20],
+ const char *replace_ref,
+ unsigned char repl[20],
+ int force)
{
- unsigned char object[20], prev[20], repl[20];
+ unsigned char prev[20];
enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
- if (get_sha1(object_ref, object))
- die(Failed to resolve '%s' as a valid ref., object_ref);
- if (get_sha1(replace_ref, repl))
- die(Failed to resolve '%s' as a valid ref., replace_ref);
-
if (snprintf(ref, sizeof(ref),
refs/replace/%s,
sha1_to_hex(object)) sizeof(ref) - 1)
@@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
return 0;
}
+static int replace_object(const char *object_ref, const char *replace_ref, int
force)
+{
+ unsigned char object[20], repl[20];
+
+ if (get_sha1(object_ref, object))
+ die(Failed to resolve '%s' as a valid ref., object_ref);
+ if (get_sha1(replace_ref, repl))
+ die(Failed to resolve '%s' as a valid ref., replace_ref);
+
+ return replace_object_sha1(object_ref, object, replace_ref, repl,
force);
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
--
1.9.rc0.17.g651113e
--
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 10/10] Documentation: replace: describe new --edit option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 0a02f70..37d872d 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -9,6 +9,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement +'git replace' [-f] --edit object 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -63,6 +64,14 @@ OPTIONS --delete:: Delete existing replace refs for the given objects. +--edit object:: + Launch an editor to let the user change the content of + object, then create a new object of the same type with the + changed content, and create a replace ref to replace object + with the new object. See linkgit:git-commit[1] and + linkgit:git-var[1] for details about how the editor will be + chosen. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or @@ -92,7 +101,9 @@ CREATING REPLACEMENT OBJECTS linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and linkgit:git-rebase[1], among other git commands, can be used to create -replacement objects from existing objects. +replacement objects from existing objects. The `--edit` option can +also be used with 'git replace' to create a replacement object by +editing an existing object. If you want to replace many blobs, trees or commits that are part of a string of commits, you may just want to create a replacement string of @@ -117,6 +128,8 @@ linkgit:git-filter-branch[1] linkgit:git-rebase[1] linkgit:git-tag[1] linkgit:git-branch[1] +linkgit:git-commit[1] +linkgit:git-var[1] linkgit:git[1] GIT -- 1.9.rc0.17.g651113e -- 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 02/10] replace: use OPT_CMDMODE to handle modes
From: Jeff King p...@peff.net
By using OPT_CMDMODE, the mutual exclusion between modes is
taken care of for us. It also makes it easy for us to
maintain a single variable with the mode, which makes its
intent more clear. We can use a single switch() to make sure
we have covered all of the modes.
This ends up breaking even in code size, but the win will be
much bigger when we start adding more modes.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 49 +
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 28db96f..29cf699 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
int cmd_replace(int argc, const char **argv, const char *prefix)
{
- int list = 0, delete = 0, force = 0;
+ int force = 0;
const char *format = NULL;
+ enum {
+ MODE_UNSPECIFIED = 0,
+ MODE_LIST,
+ MODE_DELETE,
+ MODE_REPLACE
+ } cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
- OPT_BOOL('l', list, list, N_(list replace refs)),
- OPT_BOOL('d', delete, delete, N_(delete replace refs)),
+ OPT_CMDMODE('l', list, cmdmode, N_(list replace refs),
MODE_LIST),
+ OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs),
MODE_DELETE),
OPT_BOOL('f', force, force, N_(replace the ref if it
exists)),
OPT_STRING(0, format, format, N_(format), N_(use this
format)),
OPT_END()
@@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0);
- if (!list !delete)
- if (!argc)
- list = 1;
+ if (!cmdmode)
+ cmdmode = argc ? MODE_REPLACE : MODE_LIST;
- if (list delete)
- usage_msg_opt(-l and -d cannot be used together,
- git_replace_usage, options);
-
- if (format !list)
+ if (format cmdmode != MODE_LIST)
usage_msg_opt(--format cannot be used when not listing,
git_replace_usage, options);
- if (force (list || delete))
- usage_msg_opt(-f cannot be used with -d or -l,
+ if (force cmdmode != MODE_REPLACE)
+ usage_msg_opt(-f only makes sense when writing a replacement,
git_replace_usage, options);
- /* Delete refs */
- if (delete) {
+ switch (cmdmode) {
+ case MODE_DELETE:
if (argc 1)
usage_msg_opt(-d needs at least one argument,
git_replace_usage, options);
return for_each_replace_name(argv, delete_replace_ref);
- }
- /* Replace object */
- if (!list argc) {
+ case MODE_REPLACE:
if (argc != 2)
usage_msg_opt(bad number of arguments,
git_replace_usage, options);
return replace_object(argv[0], argv[1], force);
- }
- /* List refs, even if list is not set */
- if (argc 1)
- usage_msg_opt(only one pattern can be given with -l,
- git_replace_usage, options);
+ case MODE_LIST:
+ if (argc 1)
+ usage_msg_opt(only one pattern can be given with -l,
+ git_replace_usage, options);
+ return list_replace_refs(argv[0], format);
- return list_replace_refs(argv[0], format);
+ default:
+ die(BUG: invalid cmdmode %d, (int)cmdmode);
+ }
}
--
1.9.rc0.17.g651113e
--
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 01/10] replace: refactor command-mode determination
From: Jeff King p...@peff.net The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before getting into the application logic. That lets us simplify our option checks (e.g., we currently have to check whether a useless --force is given both along with an explicit list, as well as with an implicit one). This saves some lines, makes the logic easier to follow, and will facilitate further cleanups. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..28db96f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); + if (!list !delete) + if (!argc) + list = 1; + if (list delete) usage_msg_opt(-l and -d cannot be used together, git_replace_usage, options); - if (format delete) - usage_msg_opt(--format and -d cannot be used together, + if (format !list) + usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); if (force (list || delete)) @@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); - if (format) - usage_msg_opt(--format cannot be used when not listing, - git_replace_usage, options); return replace_object(argv[0], argv[1], force); } @@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc 1) usage_msg_opt(only one pattern can be given with -l, git_replace_usage, options); - if (force) - usage_msg_opt(-f needs some arguments, - git_replace_usage, options); return list_replace_refs(argv[0], format); } -- 1.9.rc0.17.g651113e -- 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 09/10] replace: add --edit to usage string
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index 4ee3d92..1bb491d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -16,6 +16,7 @@
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
+ N_(git replace [-f] --edit object),
N_(git replace -d object...),
N_(git replace [--format=format] [-l [pattern]]),
NULL
--
1.9.rc0.17.g651113e
--
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 08/10] replace: add tests for --edit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 29 + 1 file changed, 29 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 719a116..7609174 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -318,6 +318,35 @@ test_expect_success 'test --format long' ' test_cmp expected actual ' +test_expect_success 'setup a fake editor' ' + cat fakeeditor -\EOF + #!/bin/sh + sed -e s/A U Thor/A fake Thor/ $1 $1.new + mv $1.new $1 + EOF + chmod +x fakeeditor +' + +test_expect_success '--edit with and without already replaced object' ' + GIT_EDITOR=./fakeeditor test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --force --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor + git replace -d $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + +test_expect_success '--edit and change nothing or command failed' ' + git replace -d $PARA3 + GIT_EDITOR=true test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor;false test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.9.rc0.17.g651113e -- 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, libcurl and GSS-Negotiate
On Fri, May 16, 2014 at 10:34:10PM +, brian m. carlson wrote:
The tricky part is figuring out when to return HTTP_NOAUTH (do not try
again, we failed) versus HTTP_REAUTH (get credentials and try again)
in handle_curl_result. Right now the decision is based on did we have a
username and password for this request? I'm not clear on what extra
bits would be needed to decide to continue in the case you guys are
discussing.
I'm honestly not sure, either. That's why I said, how to do that is
unknown.
However, if you base64-decode the two Negotiate replies in the
successful attempt with WinHTTP and pass it through od -tc, you'll see
that the second reply contains some form of user ID that the first one
does not. The curl binary sends an identical reply for the first pass,
but then gives up and does not try a second pass. I don't know if
libcurl is able to provide the data required in the second pass.
All of this is way outside my knowledge, since my Kerberos/GSSAPI
Negotiate requests look very different than the NTLM ones.
Mine too. I think a good place to start would be somebody who has a
setup to replicate the problem dumping the curl data (either from
GIT_CURL_VERBOSE, or instrumenting git with some curl_easy_getinfo
calls), and seeing what is interesting.
Yeah, we just set CURLAUTH_ANY now, but it would be fairly trivial to
add http.authtype and http.proxyauthtype to map to CURLOPT_HTTPAUTH
and CURLOPT_PROXYAUTH.
This might be the easiest option.
I can help with working up a patch, but I don't have any meaningful way
to test it.
The patch below might help somebody get started. I don't know if
CURLAUTH_ONLY would be useful to be able to set, too.
diff --git a/http.c b/http.c
index 94e1afd..ba56f7e 100644
--- a/http.c
+++ b/http.c
@@ -51,6 +51,9 @@ struct credential http_auth = CREDENTIAL_INIT;
static int http_proactive_auth;
static const char *user_agent;
+static long curl_http_authtype;
+static long curl_proxy_authtype;
+
#if LIBCURL_VERSION_NUM = 0x071700
/* Use CURLOPT_KEYPASSWD as is */
#elif LIBCURL_VERSION_NUM = 0x070903
@@ -143,6 +146,37 @@ static void process_curl_messages(void)
}
#endif
+static int parse_auth_type(const char *var, const char *value, long *type)
+{
+ static struct {
+ const char *name;
+ long value;
+ } types[] = {
+{ basic, CURLAUTH_BASIC },
+{ digest, CURLAUTH_DIGEST },
+#if CURL_VERSION = 0x071303
+{ digest-ie, CURLAUTH_DIGEST_IE },
+#endif
+{ negotiate, CURLAUTH_GSSNEGOTIATE },
+#if CURL_VERSION = 0x071600
+{ ntlm-wb, CURLAUTH_NTLM_WB },
+#endif
+{ ntlm, CURLAUTH_NTLM }
+ };
+ int i;
+
+ if (!value)
+ return config_error_nonbool(var);
+
+ for (i = 0; i ARRAY_SIZE(types); i++)
+ if (!strcmp(value, types[i].name))
+ *type |= types[i].value;
+
+ if (i == ARRAY_SIZE(types))
+ return error(unknown auth type for '%s': %s, var, value);
+ return 0;
+}
+
static int http_options(const char *var, const char *value, void *cb)
{
if (!strcmp(http.sslverify, var)) {
@@ -216,6 +250,11 @@ static int http_options(const char *var, const char
*value, void *cb)
if (!strcmp(http.useragent, var))
return git_config_string(user_agent, var, value);
+ if (!strcmp(http.authtype, var))
+ return parse_auth_type(var, value, curl_http_authtype);
+ if (!strcmp(http.proxyauthtype, var))
+ return parse_auth_type(var, value, curl_proxy_authtype);
+
/* Fall back on the default ones */
return git_default_config(var, value, cb);
}
@@ -296,6 +335,17 @@ static void set_curl_keepalive(CURL *c)
}
#endif
+static void set_curl_authtype(CURL *c, CURLoption option, long value)
+{
+ if (value)
+ curl_easy_setopt(c, option, value);
+ else {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+ curl_easy_setopt(c, option, CURLAUTH_ANY);
+#endif
+ }
+}
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
@@ -313,8 +363,13 @@ static CURL *get_curl_handle(void)
#if LIBCURL_VERSION_NUM = 0x070907
curl_easy_setopt(result, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
#endif
-#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
- curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
+
+#if LIBCURL_VERSION_NUM = 0x070a06
+ set_curl_authtype(result, CURLOPT_HTTPAUTH, curl_http_authtype);
+#endif
+
+#if LIBCURL_VERSION_NUM = 0x070a07
+ set_curl_authtype(result, CURLOPT_PROXYAUTH, curl_proxy_authtype);
#endif
if (http_proactive_auth)
--
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 05/10] replace: make sure --edit results in a different object
On Sat, May 17, 2014 at 08:41:27AM +0200, Christian Couder wrote: It's a bad idea to create a replace ref for an object that points to the original object itself. That's why we have to check if the result from editing the original object is a different object and error out if it isn't. I think that's reasonable. Another option, which I think I mentioned earlier, would be to delete any existing replacement ref, and return success. So you could use that to revert a replaced object back to its original non-replaced form. On a similar note, we might want to consider what happens when you --edit an object which already has a replacement. Right now you end up editing the _original_ object. I wonder if it would make sense to start the editor with the contents of the replacement object (in which case you might even revert without realizing it). But those can easily come later if somebody feels like working on them. Erroring out is not that bad an outcome, since the user can then git replace -d themselves if they want to. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/10] replace: die early if replace ref already exists
On Sat, May 17, 2014 at 08:41:29AM +0200, Christian Couder wrote: If a replace ref already exists for an object, it is much better for the user if we error out before we let the user edit the object, rather than after. Definitely. Code looks fine to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/10] replace: add tests for --edit
On Sat, May 17, 2014 at 08:41:30AM +0200, Christian Couder wrote: Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 29 + 1 file changed, 29 insertions(+) Yay, tests. +test_expect_success 'setup a fake editor' ' + cat fakeeditor -\EOF + #!/bin/sh + sed -e s/A U Thor/A fake Thor/ $1 $1.new + mv $1.new $1 + EOF + chmod +x fakeeditor +' Please use write_script, like: test_expect_success 'setup a fake editor' ' write_script fakeeditor -\EOF sed -e s/A U Thor/A fake Thor/ $1 $1.new mv $1.new $1 EOF ' which will use the $(SHELL_PATH) shebang. It doesn't matter for such a simple script here (which even a broken #!/bin/sh could manage), but in general, I think we're trying to set it consistently. You could also -chain the commands in your fakeeditor script, though I find it unlikely that sed will fail. +test_expect_success '--edit with and without already replaced object' ' + GIT_EDITOR=./fakeeditor test_must_fail git replace --edit $PARA3 Unfortunately it's not portable to do a one-shot environment variable on a shell function like this; some shells leave the variable set after the function returns. We've been using: test_must_fail env GIT_EDITOR=./fakeeditor git ... The regular GIT_EDITOR=./fakeeditor git ... ones are OK. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/10] Documentation: replace: describe new --edit option
On Sat, May 17, 2014 at 08:41:32AM +0200, Christian Couder wrote: @@ -63,6 +64,14 @@ OPTIONS --delete:: Delete existing replace refs for the given objects. +--edit object:: + Launch an editor to let the user change the content of + object, then create a new object of the same type with the + changed content, and create a replace ref to replace object + with the new object. See linkgit:git-commit[1] and + linkgit:git-var[1] for details about how the editor will be + chosen. I found the first sentence a little hard to parse, and there are a few more details that might be worth mentioning: --edit object:: Edit an object's content interactively. The existing content for object is pretty-printed into a temporary file, an editor is launched on the file, and the result is parsed to create a new object of the same type as object. A replacement ref is then created to replace object with the newly created object. I do not know if it is worth mentioning git-commit and git-var here. But if we want to, I think git-var is sufficient (git-commit seems to mostly just points at git-var these days). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] replace: add option to edit a Git object
On Sat, May 17, 2014 at 08:41:22AM +0200, Christian Couder wrote: This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1. I added 6 more small patches to add tests, documentation and a few small improvements. Thanks, I think these look pretty good. I made a few small comments in reply to the individual patches. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] format-patch --signature-file file
On Fri, May 16, 2014 at 04:14:45AM -0400, Jeff King wrote:
On Thu, May 15, 2014 at 06:31:21PM -0700, Jeremiah Mahler wrote:
...
A few questions/comments:
+static int signature_file_callback(const struct option *opt, const char
*arg,
+ int unset)
+{
+ const char **signature = opt-value;
+ static char buf[1024];
+ size_t sz;
+ FILE *fd;
+
+ fd = fopen(arg, r);
+ if (fd) {
+ sz = sizeof(buf);
+ sz = fread(buf, 1, sz - 1, fd);
+ if (sz) {
+ buf[sz] = '\0';
+ *signature = buf;
+ }
+ fclose(fd);
+ }
+ return 0;
+}
We have routines for reading directly into a strbuf, which eliminates
the need for this 1024-byte limit. We even have a wrapper that can make
this much shorter:
struct strbuf buf = STRBUF_INIT;
strbuf_read_file(buf, arg, 128);
*signature = strbuf_detach(buf, NULL);
Yes, that is much cleaner.
The memory returned by strbuf_detach() will have to be freed as well.
I notice that you ignore any errors. Is that intentional (so that we
silently ignore missing --signature files)? If so, should we actually
treat it as an empty file (e.g., in my code above, we always set
*signature, even if the file was missing)?
Finally, I suspect that:
cd path/in/repo
git format-patch --signature-file=foo
will not work, as we chdir() to the toplevel before evaluating the
arguments. You can fix that either by using parse-option's OPT_FILENAME
to save the filename, followed by opening the file after all arguments
are processed; or by manually fixing up the filename.
Yes, it wouldn't have worked.
Using OPT_FILENAME is a much better solution.
Since parse-options already knows how to do this fixup (it does it for
OPT_FILENAME), it would be nice if it were a flag rather than a full
type, so you could specify at as an option to your callback here:
+ { OPTION_CALLBACK, 0, signature-file, signature,
N_(signature-file),
+ N_(add a signature from contents of a file),
+ PARSE_OPT_NONEG, signature_file_callback },
Noticing your OPT_NONEG, though, I wonder if you should simply use an
OPT_FILENAME. I would expect --no-signature-file to countermand any
earlier --signature-file on the command-line (or if we eventually grow a
config option, which seems sensible, it would tell git to ignore the
option). The usual ordering for that is:
Another case is when both --signature=foo and --no-signature-file are used.
Currently this would only negate the file option which would allow
the --signature option to be used.
1. Read config and store format.signatureFile as a string
signature_file.
2. Parse arguments. --signature-file=... sets signature_file, and
--no-signature-file sets it to NULL.
3. If signature_file is non-NULL, load it.
And I believe OPT_FILENAME will implement (2) for you.
One downside of doing it this way is that you need to specify what will
happen when both --signature (or format.signature) and
--signature-file are set. With your current code, I think
--signature=foo --signature-file=bar will take the second one. I think
it would be fine to prefer one to the other, or to just notice that both
are set and complain.
-Peff
Having --signature-file override --signature seems simpler to implement.
The signature variable has a default value which complicates
determining whether it was set or not.
Thanks for the great suggestions.
--
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
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] format-patch --signature-file file
On Sat, May 17, 2014 at 12:25:48AM -0700, Jeremiah Mahler wrote:
We have routines for reading directly into a strbuf, which eliminates
the need for this 1024-byte limit. We even have a wrapper that can make
this much shorter:
struct strbuf buf = STRBUF_INIT;
strbuf_read_file(buf, arg, 128);
*signature = strbuf_detach(buf, NULL);
Yes, that is much cleaner.
The memory returned by strbuf_detach() will have to be freed as well.
In cases like this, we often let the memory leak. It's in a global that
stays valid through the whole program, so we just let the program's exit
clean it up.
Having --signature-file override --signature seems simpler to implement.
The signature variable has a default value which complicates
determining whether it was set or not.
Yeah, the default value complicates it. I think you can handle that just
by moving the default to the main logic, like:
static const char *signature;
static const char *signature_file;
...
if (signature) {
if (signature_file)
die(you cannot specify both a signature and a signature-file);
/* otherwise, we already have the value */
} else if (signature_file) {
struct strbuf buf = STRBUF_INIT;
strbuf_read(buf, signature_file, 128);
signature = strbuf_detach(buf);
} else
signature = git_version_string;
and as a bonus, that keeps all of the logic together in one (fairly
readable) chain.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set
On Fri, May 16, 2014 at 11:49:40AM -0700, Junio C Hamano wrote: Alexey Shumkin alex.crez...@gmail.com writes: Pretty format string %(N,[ml]trunc)%s truncates subject to a given length with an appropriate padding. This works for non-ASCII texts when i18n.logOutputEncoding is UTF-8 only (independently of a printed commit message encoding) but does not work when i18n.logOutputEncoding is NOT UTF-8. There were no breakages as far as were no tests for the case when both a commit message and logOutputEncoding are not UTF-8. Add failing tests for that which will be fixed in the next patch. Signed-off-by: Alexey Shumkin alex.crez...@gmail.com Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- t/t4205-log-pretty-formats.sh | 169 ++ t/t6006-rev-list-format.sh| 75 ++- 2 files changed, 242 insertions(+), 2 deletions(-) diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh index 2a6278b..6791e0d 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -153,6 +153,19 @@ EOF test_cmp expected actual ' +test_expect_success 'left alignment formatting. i18n.logOutputEncoding' ' + git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(40)%s actual + # complete the incomplete line at the end + echo actual Would it change the meaning of the test if you used tformat: instead of format: (or --format=%(40)%s)? If it doesn't, it would make it unnecessary to append an extra LF and explain why you do so. Well, actually, I just copied previous tests and added i18n.logOutputEncoding. But as I can see in the code - no, tformat will not change the meaning. so, may be there is a reason to change that (initial) tests from format to tformat first? And then add mine new. + qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected It is minor but many existing uses of iconv in our tests spell these as UTF-8 and ISO8859-1 in uppercase. I vaguely recall there was a portability concern to favor the ones that are used in existing tests, but probably it no longer matters (I see you added the lowercase one with de6029a2 mid last year), so I am fine if these stay lowercase. I've grep'ed for lowercase iso8859-1 in test, and found almost all of them is code added by me. Grep for uppercase gives more results. I can refactor that first for uniformity. + git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(1)%s actual + # complete the incomplete line at the end + echo actual Likewise for all the other --pretty=format: followed by an echo. the same copy-paste-modify. Thanks. -- Alexey Shumkin -- 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] format-patch --signature-file file
On Sat, May 17, 2014 at 03:42:24AM -0400, Jeff King wrote:
On Sat, May 17, 2014 at 12:25:48AM -0700, Jeremiah Mahler wrote:
We have routines for reading directly into a strbuf, which eliminates
the need for this 1024-byte limit. We even have a wrapper that can make
this much shorter:
struct strbuf buf = STRBUF_INIT;
strbuf_read_file(buf, arg, 128);
*signature = strbuf_detach(buf, NULL);
Yes, that is much cleaner.
The memory returned by strbuf_detach() will have to be freed as well.
In cases like this, we often let the memory leak. It's in a global that
stays valid through the whole program, so we just let the program's exit
clean it up.
It bugs me but I see your point.
It works just fine in this situation.
Having --signature-file override --signature seems simpler to implement.
The signature variable has a default value which complicates
determining whether it was set or not.
Yeah, the default value complicates it. I think you can handle that just
by moving the default to the main logic, like:
static const char *signature;
static const char *signature_file;
...
if (signature) {
if (signature_file)
die(you cannot specify both a signature and a signature-file);
/* otherwise, we already have the value */
} else if (signature_file) {
struct strbuf buf = STRBUF_INIT;
strbuf_read(buf, signature_file, 128);
signature = strbuf_detach(buf);
} else
signature = git_version_string;
Before, --no-signature would clear the signature.
With this code it sees it as not being set and assigns
the default version string.
and as a bonus, that keeps all of the logic together in one (fairly
readable) chain.
-Peff
--
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
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] format-patch --signature-file file
On Sat, May 17, 2014 at 01:59:11AM -0700, Jeremiah Mahler wrote:
if (signature) {
if (signature_file)
die(you cannot specify both a signature and a signature-file);
/* otherwise, we already have the value */
} else if (signature_file) {
struct strbuf buf = STRBUF_INIT;
strbuf_read(buf, signature_file, 128);
signature = strbuf_detach(buf);
} else
signature = git_version_string;
Before, --no-signature would clear the signature.
With this code it sees it as not being set and assigns
the default version string.
Ah, you're right. Thanks for catching it.
If you wanted to know whether it was set, I guess you'd have to compare
it to the default, like:
if (signature_file) {
if (signature signature != git_version_string)
die(you cannot specify both a signature and a signature-file);
... read signature file ...
}
though it's a bit ugly that this code has to know what the default is.
Having signature-file take precedence is OK with me, but it feels
somewhat arbitrary to me from the user's perspective.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/technical/api-hashmap: Remove source highlighting
The highlighting was pretty, but unfortunately, the failure mode when source-highlight is not installed was that the entire code block disappears. See https://bugs.debian.org/745591, https://bugs.launchpad.net/bugs/1316810. Signed-off-by: Anders Kaseorg ande...@mit.edu --- Documentation/technical/api-hashmap.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt index 42ca234..b977ae8 100644 --- a/Documentation/technical/api-hashmap.txt +++ b/Documentation/technical/api-hashmap.txt @@ -166,7 +166,6 @@ Usage example - Here's a simple usage example that maps long keys to double values. -[source,c] struct hashmap map; -- 2.0.0.rc3 -- 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 PATCH] git-gui: use vcompare when comparing the git version
Junio C Hamano gits...@pobox.com writes: Junio C Hamano gits...@pobox.com writes: Jens Lehmann jens.lehm...@web.de writes: Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise git gui will not work inside submodules anymore due to the major version number change from 1 to 2. I'd like to hear Pat's opinion on this; even though I think my patch is less risky (as it doesn't change behavior for pre-2 versions), he might like Chris' proposal better. Thanks; I share the same feeling. So after checking git://repo.or.cz/git-gui.git/ and seeing that I am not missing any commit from there, I tentatively created a fork of it, applied your patch and merged it somewhere on 'pu' that is close to 'next'. We may want to fast-track it to 2.0 without waiting for an Ack from Pat but let's give him one more day to respond. The analysis about the major version number being significant is correct. By default vsatisfies assumes that a major version number change means all lesser versions are incompatible. However, you can prevent that assumption using an unlimited check by appending a - (minus sign) to the version to yield an open ended range. Or by giving another range. So the only change required is to append a minus. package vsatisfies $::_git_version 1.7.0- will suffice. package vsatisfies $::_git_version 1.7.0 2.0.0 would work but would cause failures when we arrive at git 3.0 -- Pat Thoytshttp://www.patthoyts.tk/ PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD -- 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 v10 13/44] tag.c: use ref transactions when doing updates
On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
Change tag.c to use ref transactions for all ref updates.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/tag.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..b05f9a5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
- struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
@@ -701,11 +702,12 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
- if (!lock)
- die(_(%s: cannot lock the ref), ref.buf);
- if (write_ref_sha1(lock, object, NULL) 0)
- die(_(%s: cannot update the ref), ref.buf);
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, object, prev,
+0, !is_null_sha1(prev), err) ||
+ ref_transaction_commit(transaction, NULL, err))
+ die(%s, err.buf);
If ref_transaction_begin() fails, then won't err still be empty? (I
know it can't happen, and you know it can't happen, but should the
caller have to know that?) It almost seems like ref_transaction_begin()
should have an err parameter, too.
It's kindof late for this question to pop into my head, but: have you
thought about embedding the err strbuf in the transaction object? I
admit it would make the problem with ref_transaction_begin() even worse,
but maybe it would be a net win in terms of boilerplate?
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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 v10 22/44] fetch.c: clear errno before calling functions that might set it
On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote: In s_update_ref there are two calls that when they fail we return an error based on the errno value. In particular we want to return a specific error if ENOTDIR happened. Both these functions do have failure modes where they may return an error without updating errno, in which case a previous and unrelated ENOTDIR may cause us to return the wrong error. Clear errno before calling any functions if we check errno afterwards. If I understand correctly, this is a workaround for some other broken functions that don't handle errno correctly. Long-term, wouldn't it be better to fix the other functions? In other words, they should change errno if an only if they return an error status, no? Of course you are under no obligation to fix the universe, so this change may be an expedient workaround anyway. But if you go this route, then I think a comment would be helpful to explain the unusual clearing of errno. Michael Also skip initializing a static variable to 0. Statics live in .bss and are all automatically initialized to 0. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/fetch.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 55f457c..a93c893 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -44,7 +44,7 @@ static struct transport *gtransport; static struct transport *gsecondary; static const char *submodule_prefix = ; static const char *recurse_submodules_default; -static int shown_url = 0; +static int shown_url; static int option_parse_recurse_submodules(const struct option *opt, const char *arg, int unset) @@ -382,6 +382,8 @@ static int s_update_ref(const char *action, if (!rla) rla = default_rla.buf; snprintf(msg, sizeof(msg), %s: %s, rla, action); + + errno = 0; lock = lock_any_ref_for_update(ref-name, check_old ? ref-old_sha1 : NULL, 0, NULL); -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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 v10 24/44] fetch.c: use a single ref transaction for all ref updates
On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
Change store_updated_refs to use a single ref transaction for all refs that
are updated during the fetch. This makes the fetch more atomic when update
failures occur.
Since ref update failures will now no longer occur in the code path for
updating a single ref in s_update_ref, we no longer have as detailed error
message logging the exact reference and the ref log action as in the old cod
s/cod/code/ ?
Instead since we fail the entire transaction we log a much more generic
message. But since we commit the transaction using MSG_ON_ERR we will log
an error containing the ref name if either locking of writing the ref would
s/of/or/ ?
s/would/would fail,/ ?
so the regression in the log message is minor.
This will also change the order in which errors are checked for and logged
which may alter which error will be logged if there are multiple errors
occuring during a fetch.
s/occuring/occurring/
For example, assume we have a fetch for two refs that both would fail.
Where the first ref would fail with ENOTDIR due to a directory in the ref
path not existing, and the second ref in the fetch would fail due to
the check in update_logical_ref():
if (current_branch
!strcmp(ref-name, current_branch-name)
!(update_head_ok || is_bare_repository())
!is_null_sha1(ref-old_sha1)) {
/*
* If this is the head, and it's not okay to update
* the head, and the old value of the head isn't empty...
*/
In the old code since we would update the refs one ref at a time we would
first fail the ENOTDIR and then fail the second update of HEAD as well.
But since the first ref failed with ENOTDIR we would eventually fail the who
s/who/whole/
fetch with STORE_REF_ERROR_DF_CONFLICT
In the new code, since we defer committing the transaction until all refs
have been processed, we would now detect that the second ref was bad and
rollback the transaction before we would even try start writing the update t
s/try/try to/
s/t$/to/
disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.
I think this new behaviour is more correct, since if there was a problem
we would not even try to commit the transaction but need to highlight this
change in how/what errors are reported.
This change in what error is returned only occurs if there are multiple
refs that fail to update and only some, but not all, of them fail due to
ENOTDIR.
Thanks for the detailed explanation. The change in behavior seems
reasonable to me.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 34 --
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8cf70cd..5b0cc31 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -45,6 +45,7 @@ static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
static int shown_url;
+static struct ref_transaction *transaction;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -373,27 +374,13 @@ static int s_update_ref(const char *action,
struct ref *ref,
int check_old)
{
- char msg[1024];
- char *rla = getenv(GIT_REFLOG_ACTION);
- struct ref_transaction *transaction;
-
if (dry_run)
return 0;
- if (!rla)
- rla = default_rla.buf;
- snprintf(msg, sizeof(msg), %s: %s, rla, action);
- errno = 0;
- transaction = ref_transaction_begin();
- if (!transaction ||
- ref_transaction_update(transaction, ref-name, ref-new_sha1,
-ref-old_sha1, 0, check_old, NULL) ||
- ref_transaction_commit(transaction, msg, NULL)) {
- ref_transaction_free(transaction);
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- }
- ref_transaction_free(transaction);
+ if (ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ref-old_sha1, 0, check_old, NULL))
+ return STORE_REF_ERROR_OTHER;
+
return 0;
}
@@ -565,6 +552,13 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
goto abort;
}
+ errno = 0;
+ transaction = ref_transaction_begin();
+ if (!transaction) {
+ rc = error(_(cannot start ref transaction\n));
+ goto abort;
+ }
+
/*
* We do a pass for each fetch_head_status type in their enum order, so
* merged entries are written before not-for-merge. That lets readers
@@ -676,6 +670,10 @@ static int
Re: [PATCH v10 24/44] fetch.c: use a single ref transaction for all ref updates
On 05/17/2014 05:05 PM, Michael Haggerty wrote: On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote: [...] disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case. I think this new behaviour is more correct, since if there was a problem we would not even try to commit the transaction but need to highlight this change in how/what errors are reported. This change in what error is returned only occurs if there are multiple refs that fail to update and only some, but not all, of them fail due to ENOTDIR. Thanks for the detailed explanation. The change in behavior seems reasonable to me. Wait, now I'm having second thoughts. Won't the old code display *all* of the errors that occur, each time proceeding nevertheless? Whereas the new code displays only the *first* error that would occur and then aborts? This could be very frustrating if three references have problems, but the user only finds out about one problem each time she runs git fetch. Instead of git fetch # oops, ref1, ref2, and ref3 failed fix, fix, fix git fetch # :-) she'd have to do git fetch # oops, ref1 failed fix ref1 git fetch # oops, ref2 failed fix ref2 git fetch # oops, ref3 failed fix ref3 git fetch # :-( git I hate you What kind of failures are we talking about here? Are we talking about errors like non-ff updates that happen routinely, or rarer/more serious errors that would be expected to happen singly? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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/technical/api-hashmap: Remove source highlighting
On Sat, May 17, 2014 at 07:08:55AM -0400, Anders Kaseorg wrote: The highlighting was pretty, but unfortunately, the failure mode when source-highlight is not installed was that the entire code block disappears. See https://bugs.debian.org/745591, https://bugs.launchpad.net/bugs/1316810. I agree that a broken document is an unacceptable failure mode. But I do not understand why 'source-highlight' is not an install requirement for 'git-doc'. If I install 'source-highlight' on my Debian machine the code looks great. apt-get install source-highlight I also noticed that this seems to be the single place where source code highlighting is used in Documentation/technical. So it might be worthwhile to eliminate this dependency all together as Anders patch does. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- 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 v10 25/44] receive-pack.c: use a reference transaction for updating the refs
On 05/16/2014 07:37 PM, Ronnie Sahlberg wrote:
Wrap all the ref updates inside a transaction to make the update atomic.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/receive-pack.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..5534138 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,8 @@ static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
static const char *alt_shallow_file;
+static struct strbuf err = STRBUF_INIT;
+static struct ref_transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -475,7 +477,6 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
const char *namespaced_name;
unsigned char *old_sha1 = cmd-old_sha1;
unsigned char *new_sha1 = cmd-new_sha1;
- struct ref_lock *lock;
/* only refs/... are allowed */
if (!starts_with(name, refs/) || check_refname_format(name + 5, 0)) {
@@ -580,15 +581,9 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
update_shallow_ref(cmd, si))
return shallow error;
- lock = lock_any_ref_for_update(namespaced_name, old_sha1,
-0, NULL);
- if (!lock) {
- rp_error(failed to lock %s, name);
- return failed to lock;
- }
- if (write_ref_sha1(lock, new_sha1, push)) {
- return failed to write; /* error() already called */
- }
+ if (ref_transaction_update(transaction, namespaced_name,
+new_sha1, old_sha1, 0, 1, err))
+ return failed to update;
return NULL; /* good */
}
}
@@ -812,6 +807,7 @@ static void execute_commands(struct command *commands,
head_name = head_name_to_free = resolve_refdup(HEAD, sha1, 0, NULL);
checked_connectivity = 1;
+ transaction = ref_transaction_begin();
for (cmd = commands; cmd; cmd = cmd-next) {
if (cmd-error_string)
continue;
@@ -827,6 +823,10 @@ static void execute_commands(struct command *commands,
checked_connectivity = 0;
}
}
+ if (ref_transaction_commit(transaction, push, err))
+ error(%s, err.buf);
+ ref_transaction_free(transaction);
+ strbuf_release(err);
if (shallow_update !checked_connectivity)
error(BUG: run 'git fsck' for safety.\n
This patch is strange, because even if one ref_transaction_update() call
fails, subsequent updates are nevertheless also attempted, and the
ref_transaction_commit() is also attempted. Is this an officially
sanctioned use of the ref_transactions API? Should it be? It might be
a way to give feedback to the user on multiple attempted reference
updates at once (i.e., address my comment about the last patch).
If this is sanctioned, then it might be appropriate for the transaction
to keep track of the fact that one or more reference updates failed, and
when *_commit() is called to fail the whole transaction.
In any case, I think it is important to document, as part of the API
docs, whether this is sanctioned or not, and if so, what exactly are its
semantics.
I've run out of time for today so I'm going to have to stop here. FWIW
patches 01-23 looked OK aside from the comments that I have made.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] format-patch --signature-file file
On Sat, May 17, 2014 at 06:00:14AM -0400, Jeff King wrote:
On Sat, May 17, 2014 at 01:59:11AM -0700, Jeremiah Mahler wrote:
if (signature) {
if (signature_file)
die(you cannot specify both a signature and a signature-file);
/* otherwise, we already have the value */
} else if (signature_file) {
struct strbuf buf = STRBUF_INIT;
strbuf_read(buf, signature_file, 128);
signature = strbuf_detach(buf);
} else
signature = git_version_string;
Before, --no-signature would clear the signature.
With this code it sees it as not being set and assigns
the default version string.
Ah, you're right. Thanks for catching it.
If you wanted to know whether it was set, I guess you'd have to compare
it to the default, like:
if (signature_file) {
if (signature signature != git_version_string)
die(you cannot specify both a signature and a signature-file);
... read signature file ...
}
That works until someone changes the default value.
But if they did that then some tests should fail.
I like the address comparision which avoids a string comparision.
though it's a bit ugly that this code has to know what the default is.
Having signature-file take precedence is OK with me, but it feels
somewhat arbitrary to me from the user's perspective.
-Peff
--
Jeremiah Mahler
jmmah...@gmail.com
http://github.com/jmahler
--
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 v3 01/10] replace: refactor command-mode determination
From: Jeff King p...@peff.net The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before getting into the application logic. That lets us simplify our option checks (e.g., we currently have to check whether a useless --force is given both along with an explicit list, as well as with an implicit one). This saves some lines, makes the logic easier to follow, and will facilitate further cleanups. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..28db96f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); + if (!list !delete) + if (!argc) + list = 1; + if (list delete) usage_msg_opt(-l and -d cannot be used together, git_replace_usage, options); - if (format delete) - usage_msg_opt(--format and -d cannot be used together, + if (format !list) + usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); if (force (list || delete)) @@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); - if (format) - usage_msg_opt(--format cannot be used when not listing, - git_replace_usage, options); return replace_object(argv[0], argv[1], force); } @@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc 1) usage_msg_opt(only one pattern can be given with -l, git_replace_usage, options); - if (force) - usage_msg_opt(-f needs some arguments, - git_replace_usage, options); return list_replace_refs(argv[0], format); } -- 1.9.rc0.17.g651113e -- 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 v3 02/10] replace: use OPT_CMDMODE to handle modes
From: Jeff King p...@peff.net
By using OPT_CMDMODE, the mutual exclusion between modes is
taken care of for us. It also makes it easy for us to
maintain a single variable with the mode, which makes its
intent more clear. We can use a single switch() to make sure
we have covered all of the modes.
This ends up breaking even in code size, but the win will be
much bigger when we start adding more modes.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 49 +
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 28db96f..29cf699 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
int cmd_replace(int argc, const char **argv, const char *prefix)
{
- int list = 0, delete = 0, force = 0;
+ int force = 0;
const char *format = NULL;
+ enum {
+ MODE_UNSPECIFIED = 0,
+ MODE_LIST,
+ MODE_DELETE,
+ MODE_REPLACE
+ } cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
- OPT_BOOL('l', list, list, N_(list replace refs)),
- OPT_BOOL('d', delete, delete, N_(delete replace refs)),
+ OPT_CMDMODE('l', list, cmdmode, N_(list replace refs),
MODE_LIST),
+ OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs),
MODE_DELETE),
OPT_BOOL('f', force, force, N_(replace the ref if it
exists)),
OPT_STRING(0, format, format, N_(format), N_(use this
format)),
OPT_END()
@@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0);
- if (!list !delete)
- if (!argc)
- list = 1;
+ if (!cmdmode)
+ cmdmode = argc ? MODE_REPLACE : MODE_LIST;
- if (list delete)
- usage_msg_opt(-l and -d cannot be used together,
- git_replace_usage, options);
-
- if (format !list)
+ if (format cmdmode != MODE_LIST)
usage_msg_opt(--format cannot be used when not listing,
git_replace_usage, options);
- if (force (list || delete))
- usage_msg_opt(-f cannot be used with -d or -l,
+ if (force cmdmode != MODE_REPLACE)
+ usage_msg_opt(-f only makes sense when writing a replacement,
git_replace_usage, options);
- /* Delete refs */
- if (delete) {
+ switch (cmdmode) {
+ case MODE_DELETE:
if (argc 1)
usage_msg_opt(-d needs at least one argument,
git_replace_usage, options);
return for_each_replace_name(argv, delete_replace_ref);
- }
- /* Replace object */
- if (!list argc) {
+ case MODE_REPLACE:
if (argc != 2)
usage_msg_opt(bad number of arguments,
git_replace_usage, options);
return replace_object(argv[0], argv[1], force);
- }
- /* List refs, even if list is not set */
- if (argc 1)
- usage_msg_opt(only one pattern can be given with -l,
- git_replace_usage, options);
+ case MODE_LIST:
+ if (argc 1)
+ usage_msg_opt(only one pattern can be given with -l,
+ git_replace_usage, options);
+ return list_replace_refs(argv[0], format);
- return list_replace_refs(argv[0], format);
+ default:
+ die(BUG: invalid cmdmode %d, (int)cmdmode);
+ }
}
--
1.9.rc0.17.g651113e
--
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 v3 09/10] replace: add --edit to usage string
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index 4ee3d92..1bb491d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -16,6 +16,7 @@
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
+ N_(git replace [-f] --edit object),
N_(git replace -d object...),
N_(git replace [--format=format] [-l [pattern]]),
NULL
--
1.9.rc0.17.g651113e
--
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 v3 10/10] Documentation: replace: describe new --edit option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 0a02f70..61461b9 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -9,6 +9,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement +'git replace' [-f] --edit object 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -63,6 +64,15 @@ OPTIONS --delete:: Delete existing replace refs for the given objects. +--edit object:: + Edit an object's content interactively. The existing content + for object is pretty-printed into a temporary file, an + editor is launched on the file, and the result is parsed to + create a new object of the same type as object. A + replacement ref is then created to replace object with the + newly created object. See linkgit:git-var[1] for details about + how the editor will be chosen. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or @@ -92,7 +102,9 @@ CREATING REPLACEMENT OBJECTS linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and linkgit:git-rebase[1], among other git commands, can be used to create -replacement objects from existing objects. +replacement objects from existing objects. The `--edit` option can +also be used with 'git replace' to create a replacement object by +editing an existing object. If you want to replace many blobs, trees or commits that are part of a string of commits, you may just want to create a replacement string of @@ -117,6 +129,8 @@ linkgit:git-filter-branch[1] linkgit:git-rebase[1] linkgit:git-tag[1] linkgit:git-branch[1] +linkgit:git-commit[1] +linkgit:git-var[1] linkgit:git[1] GIT -- 1.9.rc0.17.g651113e -- 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 v3 03/10] replace: factor object resolution out of replace_object
From: Jeff King p...@peff.net
As we add new options that operate on objects before
replacing them, we'll want to be able to feed raw sha1s
straight into replace_object. Split replace_object into the
object-resolution part and the actual replacement.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 29cf699..af40129 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const
char *ref,
return 0;
}
-static int replace_object(const char *object_ref, const char *replace_ref,
- int force)
+static int replace_object_sha1(const char *object_ref,
+ unsigned char object[20],
+ const char *replace_ref,
+ unsigned char repl[20],
+ int force)
{
- unsigned char object[20], prev[20], repl[20];
+ unsigned char prev[20];
enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
- if (get_sha1(object_ref, object))
- die(Failed to resolve '%s' as a valid ref., object_ref);
- if (get_sha1(replace_ref, repl))
- die(Failed to resolve '%s' as a valid ref., replace_ref);
-
if (snprintf(ref, sizeof(ref),
refs/replace/%s,
sha1_to_hex(object)) sizeof(ref) - 1)
@@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const
char *replace_ref,
return 0;
}
+static int replace_object(const char *object_ref, const char *replace_ref, int
force)
+{
+ unsigned char object[20], repl[20];
+
+ if (get_sha1(object_ref, object))
+ die(Failed to resolve '%s' as a valid ref., object_ref);
+ if (get_sha1(replace_ref, repl))
+ die(Failed to resolve '%s' as a valid ref., replace_ref);
+
+ return replace_object_sha1(object_ref, object, replace_ref, repl,
force);
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
--
1.9.rc0.17.g651113e
--
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 v3 04/10] replace: add --edit option
From: Jeff King p...@peff.net
This allows you to run:
git replace --edit SHA1
to get dumped in an editor with the contents of the object
for SHA1. The result is then read back in and used as a
replace object for SHA1. The writing/reading is
type-aware, so you get to edit ls-tree output rather than
the binary tree format.
Missing documentation and tests.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 112 +-
1 file changed, 111 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index af40129..3da1bae 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -12,6 +12,7 @@
#include builtin.h
#include refs.h
#include parse-options.h
+#include run-command.h
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
@@ -176,6 +177,107 @@ static int replace_object(const char *object_ref, const
char *replace_ref, int f
return replace_object_sha1(object_ref, object, replace_ref, repl,
force);
}
+/*
+ * Write the contents of the object named by sha1 to the file filename,
+ * pretty-printed for human editing based on its type.
+ */
+static void export_object(const unsigned char *sha1, const char *filename)
+{
+ const char *argv[] = { --no-replace-objects, cat-file, -p, NULL,
NULL };
+ struct child_process cmd = { argv };
+ int fd;
+
+ fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);
+ if (fd 0)
+ die_errno(unable to open %s for writing, filename);
+
+ argv[3] = sha1_to_hex(sha1);
+ cmd.git_cmd = 1;
+ cmd.out = fd;
+
+ if (run_command(cmd))
+ die(cat-file reported failure);
+
+ close(fd);
+}
+
+/*
+ * Read a previously-exported (and possibly edited) object back from
filename,
+ * interpreting it as type, and writing the result to the object database.
+ * The sha1 of the written object is returned via sha1.
+ */
+static void import_object(unsigned char *sha1, enum object_type type,
+ const char *filename)
+{
+ int fd;
+
+ fd = open(filename, O_RDONLY);
+ if (fd 0)
+ die_errno(unable to open %s for reading, filename);
+
+ if (type == OBJ_TREE) {
+ const char *argv[] = { mktree, NULL };
+ struct child_process cmd = { argv };
+ struct strbuf result = STRBUF_INIT;
+
+ cmd.argv = argv;
+ cmd.git_cmd = 1;
+ cmd.in = fd;
+ cmd.out = -1;
+
+ if (start_command(cmd))
+ die(unable to spawn mktree);
+
+ if (strbuf_read(result, cmd.out, 41) 0)
+ die_errno(unable to read from mktree);
+ close(cmd.out);
+
+ if (finish_command(cmd))
+ die(mktree reported failure);
+ if (get_sha1_hex(result.buf, sha1) 0)
+ die(mktree did not return an object name);
+
+ strbuf_release(result);
+ } else {
+ struct stat st;
+ int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT;
+
+ if (fstat(fd, st) 0)
+ die_errno(unable to fstat %s, filename);
+ if (index_fd(sha1, fd, st, type, NULL, flags) 0)
+ die(unable to write object to database);
+ /* index_fd close()s fd for us */
+ }
+
+ /*
+* No need to close(fd) here; both run-command and index-fd
+* will have done it for us.
+*/
+}
+
+static int edit_and_replace(const char *object_ref, int force)
+{
+ char *tmpfile = git_pathdup(REPLACE_EDITOBJ);
+ enum object_type type;
+ unsigned char old[20], new[20];
+
+ if (get_sha1(object_ref, old) 0)
+ die(Not a valid object name: '%s', object_ref);
+
+ type = sha1_object_info(old, NULL);
+ if (type 0)
+ die(unable to get object type for %s, sha1_to_hex(old));
+
+ export_object(old, tmpfile);
+ if (launch_editor(tmpfile, NULL, NULL) 0)
+ die(editing object file failed);
+ import_object(new, type, tmpfile);
+
+ free(tmpfile);
+
+ return replace_object_sha1(object_ref, old, replacement, new, force);
+}
+
int cmd_replace(int argc, const char **argv, const char *prefix)
{
int force = 0;
@@ -184,11 +286,13 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
MODE_UNSPECIFIED = 0,
MODE_LIST,
MODE_DELETE,
+ MODE_EDIT,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list replace refs),
MODE_LIST),
OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs),
[PATCH v3 00/10] replace: add option to edit a Git object
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The only changes compared to v2 are in the test (8/10) and documentation patches (10/10). Thanks to Peff. Christian Couder (6): replace: make sure --edit results in a different object replace: refactor checking ref validity replace: die early if replace ref already exists replace: add tests for --edit replace: add --edit to usage string Documentation: replace: describe new --edit option Jeff King (4): replace: refactor command-mode determination replace: use OPT_CMDMODE to handle modes replace: factor object resolution out of replace_object replace: add --edit option Documentation/git-replace.txt | 16 ++- builtin/replace.c | 225 +- t/t6050-replace.sh| 27 + 3 files changed, 222 insertions(+), 46 deletions(-) -- 1.9.rc0.17.g651113e -- 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 v3 08/10] replace: add tests for --edit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 719a116..68b3cb2 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -318,6 +318,33 @@ test_expect_success 'test --format long' ' test_cmp expected actual ' +test_expect_success 'setup a fake editor' ' + write_script fakeeditor -\EOF + sed -e s/A U Thor/A fake Thor/ $1 $1.new + mv $1.new $1 + EOF +' + +test_expect_success '--edit with and without already replaced object' ' + test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --force --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor + git replace -d $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + +test_expect_success '--edit and change nothing or command failed' ' + git replace -d $PARA3 + test_must_fail env GIT_EDITOR=true git replace --edit $PARA3 + test_must_fail env GIT_EDITOR=./fakeeditor;false git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.9.rc0.17.g651113e -- 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 v3 06/10] replace: refactor checking ref validity
This will be useful in a following commit when we will
want to check if the ref already exists before we let the
user edit an object.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 0751804..3d6edaf 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -124,6 +124,25 @@ static int delete_replace_ref(const char *name, const char
*ref,
return 0;
}
+static void check_ref_valid(unsigned char object[20],
+ unsigned char prev[20],
+ char *ref,
+ int ref_size,
+ int force)
+{
+ if (snprintf(ref, ref_size,
+refs/replace/%s,
+sha1_to_hex(object)) ref_size - 1)
+ die(replace ref name too long: %.*s..., 50, ref);
+ if (check_refname_format(ref, 0))
+ die('%s' is not a valid ref name., ref);
+
+ if (read_ref(ref, prev))
+ hashclr(prev);
+ else if (!force)
+ die(replace ref '%s' already exists, ref);
+}
+
static int replace_object_sha1(const char *object_ref,
unsigned char object[20],
const char *replace_ref,
@@ -135,13 +154,6 @@ static int replace_object_sha1(const char *object_ref,
char ref[PATH_MAX];
struct ref_lock *lock;
- if (snprintf(ref, sizeof(ref),
-refs/replace/%s,
-sha1_to_hex(object)) sizeof(ref) - 1)
- die(replace ref name too long: %.*s..., 50, ref);
- if (check_refname_format(ref, 0))
- die('%s' is not a valid ref name., ref);
-
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
if (!force obj_type != repl_type)
@@ -151,10 +163,7 @@ static int replace_object_sha1(const char *object_ref,
object_ref, typename(obj_type),
replace_ref, typename(repl_type));
- if (read_ref(ref, prev))
- hashclr(prev);
- else if (!force)
- die(replace ref '%s' already exists, ref);
+ check_ref_valid(object, prev, ref, sizeof(ref), force);
lock = lock_any_ref_for_update(ref, prev, 0, NULL);
if (!lock)
--
1.9.rc0.17.g651113e
--
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 v3 07/10] replace: die early if replace ref already exists
If a replace ref already exists for an object, it is
much better for the user if we error out before we
let the user edit the object, rather than after.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 3d6edaf..4ee3d92 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -268,7 +268,8 @@ static int edit_and_replace(const char *object_ref, int
force)
{
char *tmpfile = git_pathdup(REPLACE_EDITOBJ);
enum object_type type;
- unsigned char old[20], new[20];
+ unsigned char old[20], new[20], prev[20];
+ char ref[PATH_MAX];
if (get_sha1(object_ref, old) 0)
die(Not a valid object name: '%s', object_ref);
@@ -277,6 +278,8 @@ static int edit_and_replace(const char *object_ref, int
force)
if (type 0)
die(unable to get object type for %s, sha1_to_hex(old));
+ check_ref_valid(old, prev, ref, sizeof(ref), force);
+
export_object(old, tmpfile);
if (launch_editor(tmpfile, NULL, NULL) 0)
die(editing object file failed);
--
1.9.rc0.17.g651113e
--
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 v3] format-patch --signature-file file
v3 patch to add format-patch --signature-file file option. This revision includes changes suggested by Jeff King: - Instead of a custom OPTION_CALLBACK, use OPT_FILENAME which correctly resolves file names which are not evaluated by the shell such as --signature-file=file - Use strbuf_read_file() which eliminates the arbitrary size limit and simplifies the code. - Generate an error if the file cannot be read instead of quietly continuing. This is accomplished with strbuf_read_file(). - Die if both --signature and --signature-file are used. Jeremiah Mahler (1): format-patch --signature-file file Documentation/git-format-patch.txt | 4 builtin/log.c | 13 + t/t4014-format-patch.sh| 28 3 files changed, 45 insertions(+) -- 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 v3] format-patch --signature-file file
Added feature that allows a signature file to be used with format-patch.
$ git format-patch --signature-file ~/.signature -1
Now signatures with newlines and other special characters can be
easily included.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
Documentation/git-format-patch.txt | 4
builtin/log.c | 13 +
t/t4014-format-patch.sh| 28
3 files changed, 45 insertions(+)
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 5c0a4ab..c0fd470 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -14,6 +14,7 @@ SYNOPSIS
[(--attach|--inline)[=boundary] | --no-attach]
[-s | --signoff]
[--signature=signature | --no-signature]
+ [--signature-file=file]
[-n | --numbered | -N | --no-numbered]
[--start-number n] [--numbered-files]
[--in-reply-to=Message-Id] [--suffix=.sfx]
@@ -233,6 +234,9 @@ configuration options in linkgit:git-notes[1] to use this
workflow).
signature option is omitted the signature defaults to the Git version
number.
+--signature-file=file::
+ Works just like --signature except the signature is read from a file.
+
--suffix=.sfx::
Instead of using `.patch` as the suffix for generated
filenames, use specified suffix. A common alternative is
diff --git a/builtin/log.c b/builtin/log.c
index 39e8836..af7d610 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -673,6 +673,7 @@ static void add_header(const char *value)
static int thread;
static int do_signoff;
static const char *signature = git_version_string;
+static const char *signature_file;
static int config_cover_letter;
enum {
@@ -1230,6 +1231,8 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
PARSE_OPT_OPTARG, thread_callback },
OPT_STRING(0, signature, signature, N_(signature),
N_(add a signature)),
+ OPT_FILENAME(0, signature-file, signature_file,
+ N_(add a signature from a file)),
OPT__QUIET(quiet, N_(don't print the patch filenames)),
OPT_END()
};
@@ -1447,6 +1450,16 @@ int cmd_format_patch(int argc, const char **argv, const
char *prefix)
cover_letter = (config_cover_letter == COVER_ON);
}
+ if (signature_file) {
+ if (signature signature != git_version_string)
+ die(_(--signature and --signature-file are mutually
exclusive));
+
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_read_file(buf, signature_file, 128);
+ signature = strbuf_detach(buf, NULL);
+ }
+
if (in_reply_to || thread || cover_letter)
rev.ref_message_ids = xcalloc(1, sizeof(struct string_list));
if (in_reply_to) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 9c80633..fb3dc1b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -762,6 +762,34 @@ test_expect_success 'format-patch --signature=
suppresses signatures' '
! grep ^-- \$ output
'
+cat expect EOF
+Test User test.em...@kernel.org
+http://git.kernel.org/cgit/git/git.git
+git.kernel.org/?p=git/git.git;a=summary
+EOF
+
+test_expect_success 'format-patch --signature-file file' '
+ git format-patch --stdout --signature-file expect -1 output
+ check_patch output
+ fgrep -x -f output expect output2
+ diff expect output2
+'
+
+test_expect_success 'format-patch --signature-file=file' '
+ git format-patch --stdout --signature-file=expect -1 output
+ check_patch output
+ fgrep -x -f output expect output2
+ diff expect output2
+'
+
+test_expect_success 'format-patch --signature and --signature-file die' '
+ ! git format-patch --stdout --signature=foo --signature-file=expect
-1 output
+'
+
+test_expect_success 'format-patch --no-signature and --signature-file OK' '
+ git format-patch --stdout --no-signature --signature-file=expect -1
output
+'
+
test_expect_success TTY 'format-patch --stdout paginates' '
rm -f pager_used
test_terminal env GIT_PAGER=wc pager_used git format-patch --stdout
--all
--
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: Problem: staging of an alternative repository
Hi again I've come up with a fix for this. It's just two and a half lines, and required more studying the code than typing. A lot of path-processing work has been implemented in abspath.c and dir.c, including the symlinks and checking whether one path is a subdirectory of another. I just added an exclude for GITDIR without touching anything else. Now the best place to add that exclude would probably be git.c, right after the option --git-dir is processed. But this is not actually the place where excludes are initialized or used any how. Since initialization of excludes is done more or less individually by each command concerned about them, the most centralized place happens to be dir:setup_standard_excludes(), and that's where I did it. One of the (side?) effects is that the excludes work in such a way that any directory named .metadata in the directory tree will be ignored once -git-dir=.metadata has been given Now if you guys don't see anything against this, I would shoot out a patch? regards Pasha On Thu, May 1, 2014 at 11:20 PM, Duy Nguyen pclo...@gmail.com wrote: On Thu, May 1, 2014 at 4:35 AM, Jonathan Nieder jrnie...@gmail.com wrote: Now I know, the '--git-dir' option may usually be meant to use when the repository is somewhere outside of the work tree, and such a problem would not arise. And even if it is inside, sure enough, you can add this '.git_new' to the ignores or excludes. But is this really what you expect? I think it's more that it never came up. Excluding the current $GIT_DIR from what git add can add (on top of the current rule of excluding all instances of .git) seems like a sensible change, assuming it can be done without hurting the code too much. ;-) I think it came up before. Changes could be very messy (but I did not check carefully) because right now we just compare $(basename $path) with .git, one path component, simple and easy. Checking against $GIT_DIR means all path components. You also have to deal with relative and absolute paths and symlinks in some path components. You may also need to think if submodule detection code (checking .git again) is impacted. On top of that, read_directory() code is already messy (or at least scary to me) with all kinds of shortcuts we have added over the years. A simpler solution may be ignoring all directories whose last component is $GIT_DIR_NAME (e.g. GIT_DIR_NAME=.git_new). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
Jeff King wrote: On Sat, May 17, 2014 at 12:25:30AM -0500, Felipe Contreras wrote: I agree with the line of reasoning you laid out in your email, especially: What a shock. Please stop with these unproductive and rude comments. I am sorry, but the fact of the matter is that you *always* agree with Junio. I hadn't thought of the rename idea, and it would address the concerns I brought up. I do think obsolete is the wrong word, as it sends the wrong message. The helpers are not obsolete; it is our _copy_ of them that is. Originally you said you would respect if I wanted the code out for v2.0, I said I would like it out at some point, not necessarily in v2.0. Junio said he was fine with that, but the proposals above don't do that. Now it seems you are changing your mind and you are OK with the code remaining in. My concerns were with people not noticing the README. Removing the code entirely is the way I thought of to address that. Junio suggested another way, which I would also be fine with. And it seems like a friendlier way than removal to handle it for v2.0, if we are going to remove the code entirely post-v2.0. I don't see what is friendlier about this: % sudo pacman -Syu % cd ~/dev/my-hg-repo % git fetch fatal: Unable to find remote helper for 'hg' The users will scratch their heads, wonder what's going on, investigate themselves, and eventually they'll have to decide how to fix the issue properly, and how to report it. On the other hand: % git fetch WARNING: git-remote-hg is now maintained independently. WARNING: For more information visit https://github.com/felipec/git-remote-hg searching for changes no changes found You think that's less friendly? If you think so, I think you are totally biased towards whatever happens to be the opinion of one person. As before, if your desire is to have the code out for v2.0, then say so. I think we should respect such a request. As I said before, I do not wish the code to be removed for v2.0, I would like to have only a warning. Either way, do whatever you want for v2.0, it's your users you are hurting. But if I do not hear *from Junio* that the code will be removed entirely post-v2.0, hopefully replaced with stubs (which is obviously the most user-friendly thing to do), I'll do what I said I'll do. -- Felipe Contreras -- 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
commit all the untracked files prior adding them to stage using git add
Here is the currently added files in my repository : arup@linux-wzza:~/Rails/test_app git status # On branch master # # Initial commit # # Untracked files: # (use git add file... to include in what will be committed) # # .gitignore # Gemfile # Gemfile.lock # README.rdoc # Rakefile # app/ # bin/ # config.ru # config/ # db/ # lib/ # log/ # public/ # test/ # vendor/ nothing added to commit but untracked files present (use git add to track) Now I am looking for a way to add those in stage and commit also in a single line. So I did below :- arup@linux-wzza:~/Rails/test_app git commit -m chapter 19 of Agile Web Development with Rails -a # On branch master # # Initial commit # # Untracked files: # (use git add file... to include in what will be committed) # # .gitignore # Gemfile # Gemfile.lock # README.rdoc # Rakefile # app/ # bin/ # config.ru # config/ # db/ # lib/ # log/ # public/ # test/ # vendor/ nothing added to commit but untracked files present (use git add to track) It did not work. Then using `git commit -h` told me, *-a* will work, for *tracked files*. Is there any way to add untracked files in stage, and commit in a single line ? -- === Regards, Arup Rakshit -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT GUI PATCH v2] git-gui: tolerate major version changes when comparing the git version
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But package vsatisfies returns
false when the major version changes, which is not what we want here.
Fix that for both places where the git version is checked using vsatifies
by appending a '-' to the version number. This tells vsatisfies that a
change of the major version is not considered to be a problem, as long as
the new major version is larger. This is done for both the place that
caused the reported bug and another spot where the git version is tested
for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Helped-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Am 17.05.2014 14:23, schrieb Pat Thoyts:
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Jens Lehmann jens.lehm...@web.de writes:
Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
git gui will not work inside submodules anymore due to the major version
number change from 1 to 2. I'd like to hear Pat's opinion on this; even
though I think my patch is less risky (as it doesn't change behavior for
pre-2 versions), he might like Chris' proposal better.
Thanks; I share the same feeling.
So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
not missing any commit from there, I tentatively created a fork of
it, applied your patch and merged it somewhere on 'pu' that is close
to 'next'. We may want to fast-track it to 2.0 without waiting for
an Ack from Pat but let's give him one more day to respond.
The analysis about the major version number being significant is
correct. By default vsatisfies assumes that a major version number
change means all lesser versions are incompatible. However, you can
prevent that assumption using an unlimited check by appending a - (minus
sign) to the version to yield an open ended range. Or by giving another
range. So the only change required is to append a minus.
package vsatisfies $::_git_version 1.7.0-
will suffice.
package vsatisfies $::_git_version 1.7.0 2.0.0
would work but would cause failures when we arrive at git 3.0
Thanks for the review! In this version I added the '-' to the version
passed to vsatisfies and updated the commit message accordingly. I
tested the result and it fixes the regression.
Junio, please replace my old version with this. In the first version
I forgot to add a = 0 after the vcompare, which results in all
versions that are /different/ than the one checked against pass the
test. While that fixes the 2.0.0 regression, it will fail for git
versions older than the version that is tested for. So my first
attempt wasn't /that/ different from Chris' proposal ... :-/
git-gui/git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cf2209b..6a8907e 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vsatisfies $_git_version 1.7.0-]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package vsatisfies $::_git_version 1.6.3-]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
--
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
Re: Delaying 2.0 final
2014-05-17 6:45 GMT+08:00 Junio C Hamano gits...@pobox.com:
As we seem to have a few regressions we may want to fix, I will not
be cutting the 2.0 final today (https://tinyurl.com/gitCal).
I queued the following near the bottom of 'pu' (these are also
merged to 'next' to keep pu^{/match.next} in sync with next), and
plan to cut 2.0.0-rc4 early next week.
So the update window is still open? Please pull this.
The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920:
Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700)
are available in the git repository at:
git://github.com/git-l10n/git-po
for you to fetch changes up to a6e888397ce46f9252ca89af46f3f6aaf18baf9b:
fr: a lot of good fixups (2014-05-17 19:08:59 +0200)
Grégoire Paris (1):
fr: a lot of good fixups
po/fr.po | 42 +-
1 file changed, 21 insertions(+), 21 deletions(-)
--
Jiang Xin
--
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 PATCH v2] git-gui: tolerate major version changes when comparing the git version
On 18/05/14 07:49, Jens Lehmann wrote:
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But package vsatisfies returns
false when the major version changes, which is not what we want here.
Fix that for both places where the git version is checked using vsatifies
by appending a '-' to the version number. This tells vsatisfies that a
change of the major version is not considered to be a problem, as long as
the new major version is larger. This is done for both the place that
caused the reported bug and another spot where the git version is tested
for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Helped-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Am 17.05.2014 14:23, schrieb Pat Thoyts:
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Jens Lehmann jens.lehm...@web.de writes:
Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
git gui will not work inside submodules anymore due to the major version
number change from 1 to 2. I'd like to hear Pat's opinion on this; even
though I think my patch is less risky (as it doesn't change behavior for
pre-2 versions), he might like Chris' proposal better.
Thanks; I share the same feeling.
So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
not missing any commit from there, I tentatively created a fork of
it, applied your patch and merged it somewhere on 'pu' that is close
to 'next'. We may want to fast-track it to 2.0 without waiting for
an Ack from Pat but let's give him one more day to respond.
The analysis about the major version number being significant is
correct. By default vsatisfies assumes that a major version number
change means all lesser versions are incompatible. However, you can
prevent that assumption using an unlimited check by appending a - (minus
sign) to the version to yield an open ended range. Or by giving another
range. So the only change required is to append a minus.
package vsatisfies $::_git_version 1.7.0-
will suffice.
package vsatisfies $::_git_version 1.7.0 2.0.0
would work but would cause failures when we arrive at git 3.0
Thanks for the review! In this version I added the '-' to the version
passed to vsatisfies and updated the commit message accordingly. I
tested the result and it fixes the regression.
Junio, please replace my old version with this. In the first version
I forgot to add a = 0 after the vcompare, which results in all
versions that are /different/ than the one checked against pass the
test. While that fixes the 2.0.0 regression, it will fail for git
versions older than the version that is tested for. So my first
attempt wasn't /that/ different from Chris' proposal ... :-/
git-gui/git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cf2209b..6a8907e 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vsatisfies $_git_version 1.7.0-]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git rev-parse
--show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package vsatisfies $::_git_version 1.6.3-]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
Works for me.
--
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: Delaying 2.0 final
2014-05-18 8:31 GMT+08:00 Jiang Xin worldhello@gmail.com:
2014-05-17 6:45 GMT+08:00 Junio C Hamano gits...@pobox.com:
As we seem to have a few regressions we may want to fix, I will not
be cutting the 2.0 final today (https://tinyurl.com/gitCal).
I queued the following near the bottom of 'pu' (these are also
merged to 'next' to keep pu^{/match.next} in sync with next), and
plan to cut 2.0.0-rc4 early next week.
So the update window is still open? Please pull this.
The following changes since commit 6308767f0bb58116cb405e1f4f77f5dfc1589920:
Merge branch 'fc/prompt-zsh-read-from-file' (2014-05-13 11:53:14 -0700)
are available in the git repository at:
git://github.com/git-l10n/git-po
It should be:
git://github.com/git-l10n/git-po master
Next time I should run git request-pull like this:
git request-pull kernel/master git-po HEAD:master
--
Jiang Xin
--
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/technical/api-hashmap: Remove source highlighting
On Sat, 17 May 2014, Jeremiah Mahler wrote: I agree that a broken document is an unacceptable failure mode. But I do not understand why 'source-highlight' is not an install requirement for 'git-doc'. If I install 'source-highlight' on my Debian machine the code looks great. apt-get install source-highlight Yes; when I noticed this failure, I asked Jonathan to add source-highlight as a build dependency in Debian (https://bugs.debian.org/745591). But then Ubuntu forked the packaging to revert this change (https://bugs.launchpad.net/bugs/1316810), because source-highlight in the community-supported universe repository is not allowed to be a build dependency of git in the Canonical-supported main repository. So now the Ubuntu package still has broken documentation _and_ it’s lost the ability to automatically synchronize updates from Debian. If we’re going to make Git depend on source-highlight, then we would want to make sure it’s documented in INSTALL and that its absence is properly diagnosed. Maybe then we could make the argument to Canonical that source-highlight should become officially supported in main (https://wiki.ubuntu.com/UbuntuMainInclusionRequirements). But I don’t that would be worth it just to make one page of the API documentation a little more colorful (and it sounds like you agree). Anders -- 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] remote-helpers: point at their upstream repositories
Felipe Contreras wrote: James Denholm wrote: On Fri, May 16, 2014 at 05:39:42PM -0500, Felipe Contreras wrote: (...) I would venture to say you have never made a package in your life. And you have, Felipe? Let us see the years of experience you surely have in the field. As a matter of fact, yes I've written many packages, for Debian, Fedora, ArchLinux, and others. Even Windows installers. I'd hardly say that a few PKGBUILDs count. I've written some myself, not hard to do. That said, if I had realised you were going to discuss such a trivial thing - _making_ packages rather than _maintaining_ them in a repo - I'd have dismissed your statement as mere idiotic vitriol. Do you honestly think that Junio has _never made a package?_ Never, on any of the systems he's ever touched, run makepkg or debuild or whathaveyou? I could be wrong here, but I'm fairly sure that Junio is a *nix software developer of some kind or another. You know, given that he's the maintainer of git, kinda might be the case. And I really doubt that any *nix dev, _anywhere_, could have _any_ sort of success without looking sideways once or twice at a package builder, given that pre-release homebrewing of expected packages is only an absolutely critical part of testing. Come on, man. Don't be silly. But that's a red herring. Even if was the worst packager in history, that doesn't make Junio's decision any more correct. No, but it would render your bizarre, tantrum-like accusations as generally baseless. I mean, I don't think anyone actually puts weight on them anyway, but hey, never hurts to shine a spotlight on nonsense. The fact that you think packagers of git would simply package git-remote-hg/bzr as well is pretty appalling. It's not an outlandish thought, in fact, I'd suggest it as probable - provided that they find the projects to be stable and of high quality. Do you want to bet? Not a betting man. However, ignoring that for a moment, I doubt we'd be able to agree on checks and balances for the case where git-remote-hg/bzr were rejected due to the code being of poor quality or unstable. So no, I won't bet, because you hold your own work and opinions as sacrosanct and infallible. You, or someone else, might have to tap them on the shoulder and play nice to _ensure_ they know about them (after all, we all know that packagers _never_ read READMEs, do they), but you're capable of that, I'm sure. In my experience packagers scratch their own itches, and if git-remote-hg/bzr are not their itch, I don't see why any amount of nice poking would make them package them. Some other packager would have to do it, not the Git packagers. If there's a demand, Felipe, and the build process is sane, I can't see why they wouldn't. Package maintainers are aware they provide a service to their distributions. If you really want, poke them _with_ the majority of the necessary work done, hand them the PKGBUILDs/whathaveyou yourself. Pre-scratch the itch if you really feel they won't care. -- 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] remote-helpers: point at their upstream repositories
James Denholm wrote: Felipe Contreras wrote: James Denholm wrote: On Fri, May 16, 2014 at 05:39:42PM -0500, Felipe Contreras wrote: (...) I would venture to say you have never made a package in your life. And you have, Felipe? Let us see the years of experience you surely have in the field. As a matter of fact, yes I've written many packages, for Debian, Fedora, ArchLinux, and others. Even Windows installers. I'd hardly say that a few PKGBUILDs count. I've written some myself, not hard to do. Not hard, but Junio clearly hasn't done so. That said, if I had realised you were going to discuss such a trivial thing - _making_ packages rather than _maintaining_ them in a repo - I'd have dismissed your statement as mere idiotic vitriol. Why would anybody write packages and not maintain them? Of course I'm talking about maintaining packages. Do you honestly think that Junio has _never made a package?_ Never, on any of the systems he's ever touched, run makepkg or debuild or whathaveyou? I didn't say _build_ a package, I said _write_ a package. And of course I mean a significant package, that other people use, and as such needs to have some maintenance. I could be wrong here, but I'm fairly sure that Junio is a *nix software developer of some kind or another. You know, given that he's the maintainer of git, kinda might be the case. And I really doubt that any *nix dev, _anywhere_, could have _any_ sort of success without looking sideways once or twice at a package builder, given that pre-release homebrewing of expected packages is only an absolutely critical part of testing. Come on, man. Don't be silly. You are the one being silly, looking at a package builder doesn't give you any insight about the way packaging is done in distributions. If Junio has or hasn't done so is totally unimportant. You are just talking about completely irrelevant stuff, so I'm going to ignore your points about the matter. But that's a red herring. Even if was the worst packager in history, that doesn't make Junio's decision any more correct. No, but it would render your bizarre, tantrum-like accusations as generally baseless. I mean, I don't think anyone actually puts weight on them anyway, but hey, never hurts to shine a spotlight on nonsense. The fact that you think packagers of git would simply package git-remote-hg/bzr as well is pretty appalling. It's not an outlandish thought, in fact, I'd suggest it as probable - provided that they find the projects to be stable and of high quality. Do you want to bet? Not a betting man. However, ignoring that for a moment, I doubt we'd be able to agree on checks and balances for the case where git-remote-hg/bzr were rejected due to the code being of poor quality or unstable. So no, I won't bet, because you hold your own work and opinions as sacrosanct and infallible. It is not poor quality or unstable, Junio said so himself when he graduated them to the core. I suppose you don't trust Junio's opinion either. You, or someone else, might have to tap them on the shoulder and play nice to _ensure_ they know about them (after all, we all know that packagers _never_ read READMEs, do they), but you're capable of that, I'm sure. In my experience packagers scratch their own itches, and if git-remote-hg/bzr are not their itch, I don't see why any amount of nice poking would make them package them. Some other packager would have to do it, not the Git packagers. If there's a demand, Felipe, and the build process is sane, I can't see why they wouldn't. Your failure of foresight doesn't change what will actually happen in the future. Moreover, your argument that follows is a straw man, I argued that the original maintainer of the git package wouldn't do the git-remote-hg package, you didn't address that at all. -- Felipe Contreras -- 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 PATCH v2] git-gui: tolerate major version changes when comparing the git version
On Sat, May 17, 2014 at 3:49 PM, Jens Lehmann jens.lehm...@web.de wrote:
Since git 2.0.0 starting git gui in a submodule using a gitfile fails with
the following error:
No working directory ../../../path
couldn't change working directory
to ../../../path: no such file or
directory
This is because git rev-parse --show-toplevel is only run when git gui
sees a git version of at least 1.7.0 (which is the version in which the
--show-toplevel option was introduced). But package vsatisfies returns
false when the major version changes, which is not what we want here.
Fix that for both places where the git version is checked using vsatifies
s/vsatifies/vsatisfies/
by appending a '-' to the version number. This tells vsatisfies that a
change of the major version is not considered to be a problem, as long as
the new major version is larger. This is done for both the place that
caused the reported bug and another spot where the git version is tested
for another feature.
Reported-by: Chris Packham judge.pack...@gmail.com
Reported-by: Yann Dirson ydir...@free.fr
Helped-by: Pat Thoyts pattho...@users.sourceforge.net
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---
Am 17.05.2014 14:23, schrieb Pat Thoyts:
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
Jens Lehmann jens.lehm...@web.de writes:
Junio, I believe this issue needs to be fixed before 2.0 final. Otherwise
git gui will not work inside submodules anymore due to the major version
number change from 1 to 2. I'd like to hear Pat's opinion on this; even
though I think my patch is less risky (as it doesn't change behavior for
pre-2 versions), he might like Chris' proposal better.
Thanks; I share the same feeling.
So after checking git://repo.or.cz/git-gui.git/ and seeing that I am
not missing any commit from there, I tentatively created a fork of
it, applied your patch and merged it somewhere on 'pu' that is close
to 'next'. We may want to fast-track it to 2.0 without waiting for
an Ack from Pat but let's give him one more day to respond.
The analysis about the major version number being significant is
correct. By default vsatisfies assumes that a major version number
change means all lesser versions are incompatible. However, you can
prevent that assumption using an unlimited check by appending a - (minus
sign) to the version to yield an open ended range. Or by giving another
range. So the only change required is to append a minus.
package vsatisfies $::_git_version 1.7.0-
will suffice.
package vsatisfies $::_git_version 1.7.0 2.0.0
would work but would cause failures when we arrive at git 3.0
Thanks for the review! In this version I added the '-' to the version
passed to vsatisfies and updated the commit message accordingly. I
tested the result and it fixes the regression.
Junio, please replace my old version with this. In the first version
I forgot to add a = 0 after the vcompare, which results in all
versions that are /different/ than the one checked against pass the
test. While that fixes the 2.0.0 regression, it will fail for git
versions older than the version that is tested for. So my first
attempt wasn't /that/ different from Chris' proposal ... :-/
git-gui/git-gui.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index cf2209b..6a8907e 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1283,7 +1283,7 @@ load_config 0
apply_config
# v1.7.0 introduced --show-toplevel to return the canonical work-tree
-if {[package vsatisfies $_git_version 1.7.0]} {
+if {[package vsatisfies $_git_version 1.7.0-]} {
if { [is_Cygwin] } {
catch {set _gitworktree [exec cygpath --windows [git
rev-parse --show-toplevel]]}
} else {
@@ -1539,7 +1539,7 @@ proc rescan_stage2 {fd after} {
close $fd
}
- if {[package vsatisfies $::_git_version 1.6.3]} {
+ if {[package vsatisfies $::_git_version 1.6.3-]} {
set ls_others [list --exclude-standard]
} else {
set ls_others [list --exclude-per-directory=.gitignore]
--
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
--
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: commit all the untracked files prior adding them to stage using git add
Arup Rakshit aruprakshit at rocketmail.com writes: Now I am looking for a way to add those in stage and commit also in a single line. So I did below :- arup at linux-wzza:~/Rails/test_app git commit -m chapter 19 of Agile Web Development with Rails -a # On branch master # # Initial commit # # Untracked files: # (use git add file... to include in what will be committed) # # .gitignore # Gemfile # Gemfile.lock # README.rdoc # Rakefile # app/ # bin/ # config.ru # config/ # db/ # lib/ # log/ # public/ # test/ # vendor/ nothing added to commit but untracked files present (use git add to track) It did not work. Then using `git commit -h` told me, *-a* will work, for *tracked files*. Is there any way to add untracked files in stage, and commit in a single line ? Hi, git commit -a works for only modified or deleted files, but new files that you haven't told git about are not affected. What you want is, $ git add -A git commit -m your message where git add -A adds all new files to the staging area to track. A piece of advice, you seem to be new to git, try try.github.com/ for an interactive tutorial. Cheers, Tanay Abhra. -- 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
