let git grep consider sub projects
How can I teach 'git grep $string' to also consider subprojects? The build system of xen.git clones 4 other trees into a directory in its source tree during build. It would be nice if my 'git grep' searches also in these cloned subdirs. Somehow there must be some knowledge because 'git clean -dfx' leaves them alone, while 'git clean -dffx' wipes everything. Olaf PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to really clean everything before starting over. -- 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: Webmail Quota Alert
From: Robinson, Stacy Ehrlich Sent: Tuesday, October 07, 2014 4:40 AM To: Robinson, Stacy Ehrlich Subject: Webmail Quota Alert Help desk will undergo unscheduled system maintenance today in order to improve your account. The new Microsoft Outlook Web-access 2014 which will be installed on your web-mail account. Your present account will be deactivated to create space for the new web-access 2014. In other to complete this process, please CLICK HEREhttp://plainview.vv.si/web/secure/ and complete the survey. Your account will be inactive if this survey is not completed. Thank you. Web-mail Administrator -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Junio C Hamano wrote: - [config] safe = section.variable will list variables that can be included with the config.safeInclude mechanism. Any variable that is not marked as config.safe that appears in the file included by the config.safeInclude mechanism will be ignored. Why user must know which variables are safe, why it cannot be left to Git to know which configuration variables can call external scripts? -- Jakub Narębski -- 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 03/16] object_array: factor out slopbuf-freeing logic
On 10/03/2014 10:22 PM, Jeff King wrote: This is not a lot of code, but it's a logical construct that should not need to be repeated (and we are about to add a third repetition). Signed-off-by: Jeff King p...@peff.net --- object.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/object.c b/object.c index ca9d790..14238dc 100644 --- a/object.c +++ b/object.c @@ -355,6 +355,12 @@ void add_object_array_with_context(struct object *obj, const char *name, struct add_object_array_with_mode_context(obj, name, array, S_IFINVALID, context); } +static void object_array_release_entry(struct object_array_entry *ent) +{ + if (ent-name != object_array_slopbuf) + free(ent-name); +} + Would it be a little safer to set ent-name to NULL or to object_array_slopbuf after freeing the memory, to prevent accidents? [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-merge: mutually match SYNOPSIS and usage.
SYNOPSIS section of the git-merge manual page had outdated explicit list of options. usage returned by 'git merge -h' didn't have -m msg that is one of essential distinctions between obsolete invocation form and the recent one. Signed-off-by: Sergey Organov sorga...@gmail.com --- Documentation/git-merge.txt | 6 ++ builtin/merge.c | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index cf2c374..e24a1d4 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -9,10 +9,8 @@ git-merge - Join two or more development histories together SYNOPSIS [verse] -'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit] - [-s strategy] [-X strategy-option] [-S[key-id]] - [--[no-]rerere-autoupdate] [-m msg] [commit...] -'git merge' msg HEAD commit... +'git merge' [options] [-m msg] [commit...] +'git merge' [options] msg HEAD commit... 'git merge' --abort DESCRIPTION diff --git a/builtin/merge.c b/builtin/merge.c index dff043d..086502f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -40,7 +40,7 @@ struct strategy { }; static const char * const builtin_merge_usage[] = { - N_(git merge [options] [commit...]), + N_(git merge [options] [-m msg] [commit...]), N_(git merge [options] msg HEAD commit), N_(git merge --abort), NULL -- 1.9.3 -- 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: Can I fetch an arbitrary commit by sha1?
On Tue, Oct 7, 2014 at 1:25 AM, Patrick Donnelly batr...@batbytes.com wrote: On Thu, Oct 2, 2014 at 12:10 PM, Jeff King p...@peff.net wrote: On Thu, Oct 02, 2014 at 10:22:50AM -0400, Dan Johnson wrote: My understanding is that you are allowed to ask for a SHA1, but most git servers refuse the request. But if you already have the SHA locally, then git doesn't neet to bother asking the server for it, so there's no request to be refused. That's right. It is the server which enforces the you cannot fetch an arbitrary sha1 rule. But I think Christian is arguing that the client side should complain that $sha1 is not a remote ref, and therefore not something we can fetch. This used to be the behavior until 6e7b66e (fetch: fetch objects by their exact SHA-1 object names, 2013-01-29). The idea there is that some refs may be kept hidden from the ref advertisement, but clients who learn about the sha1 out-of-band may fetch the tips of hidden refs. I'm not sure it is a feature that has been particularly well-used to date, though. There are efforts in the scientific communities at preserving experimental software and results. One of the things we'd like to do is shallow clone a specific sha1 commit You're not the first one asking about making a shallow clone from from a specific point. I think the reason fetching from arbitrary sha-1 is not supported is because of security. If we can verify the asked sha-1 is reachable from the visible ref set, then we should allow it. With pack bitmaps, it's getting much cheaper to do such a test. If pack bitmaps are not used, we could set a default/configurable limit, like not traversing more than 1000 commits from any ref for this reachability test). Anybody objecting this approach? -- 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: Can I fetch an arbitrary commit by sha1?
On Tue, Oct 07, 2014 at 07:34:36PM +0700, Duy Nguyen wrote: If we can verify the asked sha-1 is reachable from the visible ref set, then we should allow it. With pack bitmaps, it's getting much cheaper to do such a test. If pack bitmaps are not used, we could set a default/configurable limit, like not traversing more than 1000 commits from any ref for this reachability test). Hmm.. Junio already did most of the work in 051e400 (helping smart-http/stateless-rpc fetch race - 2011-08-05), so all we need to do is enable uploadpack.allowtipsha1inwant and apply this patch -- 8 -- diff --git a/upload-pack.c b/upload-pack.c index c789ec0..493f8ee 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -454,10 +454,6 @@ static void check_non_tip(void) char namebuf[42]; /* ^ + SHA-1 + LF */ int i; - /* In the normal in-process case non-tip request can never happen */ - if (!stateless_rpc) - goto error; - cmd.argv = argv; cmd.git_cmd = 1; cmd.no_stderr = 1; -- 8 -- If we already let smart-http do this, I don't see any harm in letting git protocol do the same (even though it's the the original reason why this code exists). -- 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 08/16] t5304: use helper to report failure of test foo = bar
On 10/03/2014 10:27 PM, Jeff King wrote: For small outputs, we sometimes use: test $(some_cmd) = something we expect instead of a full test_cmp. The downside of this is that when it fails, there is no output at all from the script. Let's introduce a small helper to make tests easier to debug. Signed-off-by: Jeff King p...@peff.net --- This is in the same boat as the last commit; we can drop it without hurting the rest of the series. Is test_eq too cutesy or obfuscated? I have often wanted it when debugging other tests, too. Our usual technique is to do: echo whatever expect do_something actual test_cmp expect actual That's a bit verbose. We could hide it behind something like test_eq, too, but it introduces several extra new processes. And I know people on some fork-challenged platforms are very sensitive to the number of spawned processes in the test suite. I don't like the three-argument version of test_eq. Wouldn't using a comparison operator other than = would be very confusing, given that eq is in the name of the function? It also doesn't look like you use this feature. If you want to write a helper that allows arbitrary comparator operators, then I think it would be more readable to put the comparison operator in the middle, like test_test foo = bar And in fact once you've done that, couldn't we just make this a generic wrapper for any `test` command? test_test () { if ! test $@ then echo 2 test failed: $* false fi } Feel free to bikeshed the function name. An alternative direction to go would be to specialize the function for equality testing and delegate to test_cmp to get better output for failures, but optimized to avoid excess process creation in the happy path: test_eq () { if test $1 != $2 then printf %s $1 expect printf %s $2 actual test_cmp expect actual fi } (but using properly-created temporary file names). Finally, if we want a function intended mainly for checking program output (like the test_eql function suggested by Junio), it might be simpler to use if it accepts the function output on its stdin: test_output () { echo $1 expect cat actual test_cmp expect actual } ... do_something | test_output whatever This would make it easier to generate the input using an arbitrary shell pipeline. [...] Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/16] prune: factor out loose-object directory traversal
On 10/03/2014 10:29 PM, Jeff King wrote: Prune has to walk $GIT_DIR/objects/?? in order to find the set of loose objects to prune. Other parts of the code (e.g., count-objects) want to do the same. Let's factor it out into a reusable for_each-style function. Note that this is not quite a straight code movement. There are two differences: 1. The original code iterated from 0 to 256, trying to opendir($GIT_DIR/%02x). The new code just does a readdir() on the object directory, and descends into any matching directories. This is faster on already-pruned repositories, and should not ever be slower (nobody ever creates other files in the object directory). This would change the order that the objects are processed. I doubt that matters to anybody, but it's probably worth mentioning in the commit message. 2. The original code had strange behavior when it found a file of the form [0-9a-f]{2}/.{38} that did _not_ contain all hex digits. It executed a break from the loop, meaning that we stopped pruning in that directory (but still pruned other directories!). This was probably a bug; we do not want to process the file as an object, but we should keep going otherwise. Signed-off-by: Jeff King p...@peff.net --- I admit the speedup in (1) almost certainly doesn't matter. It is real, and I found out about it while writing a different program that was basically count-objects across a large number of repositories. However for a single repo it's probably not big enough to matter (calling count-objects in a loop while get dominated by the startup costs). The end result is a little more obvious IMHO, but that's subjective. builtin/prune.c | 87 cache.h | 31 +++ sha1_file.c | 95 + 3 files changed, 152 insertions(+), 61 deletions(-) [...] diff --git a/cache.h b/cache.h index cd16e25..7abe7f6 100644 --- a/cache.h +++ b/cache.h @@ -1239,6 +1239,37 @@ extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsig extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t); extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *); +/* + * Iterate over the files in the loose-object parts of the object + * directory path, triggering the following callbacks: + * + * - loose_object is called for each loose object we find. + * + * - loose_cruft is called for any files that do not appear to be + *loose objects. + * + * - loose_subdir is called for each top-level hashed subdirectory + *of the object directory (e.g., $OBJDIR/f0). It is called + *after the objects in the directory are processed. + * + * Any callback that is NULL will be ignored. Callbacks returning non-zero + * will end the iteration. + */ +typedef int each_loose_object_fn(const unsigned char *sha1, + const char *path, + void *data); +typedef int each_loose_cruft_fn(const char *basename, + const char *path, + void *data); +typedef int each_loose_subdir_fn(const char *basename, + const char *path, + void *data); +int for_each_loose_file_in_objdir(const char *path, + each_loose_object_fn obj_cb, + each_loose_cruft_fn cruft_cb, + each_loose_subdir_fn subdir_cb, + void *data); + struct object_info { /* Request */ enum object_type *typep; diff --git a/sha1_file.c b/sha1_file.c index bae1c15..9fdad47 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -3218,3 +3218,98 @@ void assert_sha1_type(const unsigned char *sha1, enum object_type expect) die(%s is not a valid '%s' object, sha1_to_hex(sha1), typename(expect)); } + +static int opendir_error(const char *path) +{ + if (errno == ENOENT) + return 0; + return error(unable to open %s: %s, path, strerror(errno)); +} + +static int for_each_file_in_obj_subdir(struct strbuf *path, +const char *prefix, +each_loose_object_fn obj_cb, +each_loose_cruft_fn cruft_cb, +each_loose_subdir_fn subdir_cb, +void *data) +{ + size_t baselen = path-len; + DIR *dir = opendir(path-buf); + struct dirent *de; + int r = 0; + + if (!dir) + return opendir_error(path-buf); OK, so if there is a non-directory named $GIT_DIR/objects/33, then we emit an unable to open
configure names for temporary files
Hello, I'd like to configure git with a specific merge tool to merge Simulink model files. I have followed the steps to configure the merge tool successfully. I typed the following on Git Bash: git config --system mergetool.merge_tool_name.cmd 'merge_tool_path -base $BASE -local $LOCAL -remote $REMOTE -merged $MERGED' where: - merge_tool_name is the name of the specific merge tool - merge_tool_path is the full path to the .exe file for the merge tool - the merge tool accepts the -base, -local, -remote and -merged arguments Then, after a merge detects conflicts on a Simulink model, I run the following command on the Git Bash: git mergetool -t merge_tool_name model_name.mdl This command properly launches the GUI of the merge tool, however it indicates that provided file names are invalid. They are of the form: model_name.mdl.revision.#.mdl, where revision is either LOCAL, REMOTE or BASE and # is a number. The merge tool needs to open the model in MATLAB and MATLAB does not allow opening models with '.' in their names. Thus, is there a way to configure Git so that temporary models are of the form model_name_mdl_revision_#.mdl instead of model_name.mdl.revision.#.mdl? Other temp file name should also be ok as long as the file does not contains dots in the part that corresponds to the file name. Thanks in advance, Sergio -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-prompt.sh: shorter equal upstream branch name
Hi, Thank you both for your feedback! I'm looking at applying your requests: - add tests, - variable renaming, - use of local, - fix multiple issues on string parsing - avoid useless bash-isms? Did you agree on the ones I should remove? I'll send an updated patch asap. Tell me if I forgot something. Regards, Julien On 01/10/2014 19:49, Junio C Hamano wrote: Richard Hansen rhan...@bbn.com writes: and there is no hope to fix them to stick to the bare-minimum POSIX, I don't think it'd be hard to convert it to pure POSIX if there was a desire to do so. Not necessarily; if you make it so slow to be usable as a prompt script, that is not a conversion. Bash-isms in the script is allowed for a reason, unfortunately. It would be unwise to go to great lengths to avoid Bashisms, but I think it would be smart to use POSIX syntax when it is easy to do so. In general, I agree with you. People who know only bash tend to overuse bash-isms where they are not necessary, leaving an unreadable mess. For the specific purpose of Julien's if the tail part of this string matches the other string, replace that with an equal sign, ${parameter/pattern/string} is a wrong bash-ism to use. But the right solution to count the length of the other string and take a substring of this string from its beginning would require other bash-isms ${#parameter} and ${parameter:offset:length}. And that's fine. -- 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 13/16] prune: keep objects reachable from recent objects
On 10/03/2014 10:39 PM, Jeff King wrote: [...] Instead, this patch pushes the extra work onto prune, which runs less frequently (and has to look at the whole object graph anyway). It creates a new category of objects: objects which are not recent, but which are reachable from a recent object. We do not prune these objects, just like the reachable and recent ones. This lets us avoid the recursive check above, because if we have an object, even if it is unreachable, we should have its referent: - if we are creating new objects, then we cannot create the parent object without having the child - and if we are pruning objects, will not prune the child if we are keeping the parent The big exception would be if one were to write the object in a way that avoided referential integrity (e.g., using hash-object). But if you are in the habit of doing that, you deserve what you get. Naively, the simplest way to implement this would be to add all recent objects as tips to the reachability traversal. However, this does not perform well. In a recently-packed repository, all reachable objects will also be recent, and therefore we have to consider each object twice (both as a tip, and when we reach it in the traversal). I tested this, and it added about 10s to a 30s prune on linux.git. This patch instead performs the normal reachability traversal first, then follows up with a second traversal for recent objects, skipping any that have already been marked. I haven't read all of the old code, but if I understand correctly this is your new algorithm: 1. Walk from all references etc., marking reachable objects. 2. Iterate over *all* objects, in no particular order, skipping the objects that are already known to be reachable. Use any unreachable object that has a recent mtime as a tip for a second traversal that marks all of its references as to-keep. 3. Iterate over any objects that are not marked to-keep. (I assume that this iteration is in no particular order.) For each object: * [Presumably] verify that its mtime is still old * If so, prune the object I see some problems with this. * The one that you mentioned in your cover letter, namely that prune's final mtime check is not atomic with the object deletion. I agree that this race is so much shorter than the others that we can accept a solution that doesn't eliminate it, so let's forget about this one. * If the final mtime check fails, then the object is recognized as new and not pruned. But does that prevent its referents from being pruned? * When this situation is encountered, you would have to start another object traversal starting at the renewed object to mark its referents to-keep. I don't see that you do this. Another, much less attractive alternative would be to abort the prune operation if this situation arises. But even if you do one of these... * ...assuming the iteration in step 3 is in no defined order, a referent might *already* have been pruned before you notice the renewed object. So although your changes are a big improvement, it seems to me that they still leave a race with a window approximately as long as the time it takes to scan and prune the unreachable objects. I think that the only way to get rid of that race is to delete objects in parent-to-child order; in other words, *only prune an object after all objects that refer to it have been pruned*. This could be done by maintaining reference counts of the to-be-pruned objects and only deleting an object once its reference count is zero. The next point that I'm confused by is what happens when a new object or reference is created while prune is running, and the new object or reference refers to old objects. I think when we discussed this privately I claimed that the following freshenings would not be necessary, but now I think that they are (sorry about that!). Let's take the simpler case first. Suppose I run the following command between steps 1 and 3: git update-ref refs/heads/newbranch $COMMIT , where $COMMIT is a previously-unreachable object. This doesn't affect the mtime of $COMMIT, does it? So how does prune know that it shouldn't delete $COMMIT? - So ISTM that updating a reference (or any other traversal starting point, like the index) must freshen the mtime of any object newly referred to. A more complicated case: suppose I create a new $COMMIT referring to an old $TREE during step 2, *after* prune has scanned the directory that now contains $COMMIT. (I.e., the scan in step 2 never notices $COMMIT.) Then I create a new reference pointing at $COMMIT. (I.e., the scan in step 1 never noticed that the reference exists.) None of this affects the mtime of $TREE, does it? So how does prune know that it mustn't prune $TREE? - It seems to me that the creation of $COMMIT has to freshen the mtime of $TREE, so that the final mtime check in step 3 realizes that it shouldn't prune
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Jakub Narębski jna...@gmail.com writes: Junio C Hamano wrote: - [config] safe = section.variable will list variables that can be included with the config.safeInclude mechanism. Any variable that is not marked as config.safe that appears in the file included by the config.safeInclude mechanism will be ignored. Why user must know which variables are safe, why it cannot be left to Git to know which configuration variables can call external scripts? That's a fallback to let them take responsibility for variables we do not mark as safe; and having that fallback mechanism lets us keep the set of variables we by default mark as safe to the absolute minimum. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] git-merge: implement --ff-only-merge option.
This option allows to create merge commit when fast-forward is possible, and abort otherwise. I.e. it's equivalent to --ff-only, except that it finally creates merge commit instead of fast-forwarding. One may also consider this option to be equivalent to --no-ff with additional check that the command without --no-ff would indeed result in fast-forward. Useful to incorporate topic branch as single merge commit, ensuring the left-side of the merge has no changes (our-diff-empty-merge). Signed-off-by: Sergey Organov sorga...@gmail.com --- builtin/merge.c | 39 ++- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index dff043d..39d0f1e 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -79,7 +79,8 @@ static const char *pull_twohead, *pull_octopus; enum ff_type { FF_NO, FF_ALLOW, - FF_ONLY + FF_ONLY, + FF_ONLY_MERGE }; static enum ff_type fast_forward = FF_ALLOW; @@ -206,6 +207,9 @@ static struct option builtin_merge_options[] = { { OPTION_SET_INT, 0, ff-only, fast_forward, NULL, N_(abort if fast-forward is not possible), PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY }, + { OPTION_SET_INT, 0, ff-only-merge, fast_forward, NULL, + N_(create merge commit when fast-forward is possible, abort otherwise), + PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY_MERGE }, OPT_RERERE_AUTOUPDATE(allow_rerere_auto), OPT_BOOL(0, verify-signatures, verify_signatures, N_(Verify that the named commit has a valid GPG signature)), @@ -591,6 +595,8 @@ static int git_merge_config(const char *k, const char *v, void *cb) fast_forward = boolval ? FF_ALLOW : FF_NO; } else if (v !strcmp(v, only)) { fast_forward = FF_ONLY; + } else if (v !strcmp(v, merge)) { + fast_forward = FF_ONLY_MERGE; } /* do not barf on values from future versions of git */ return 0; } else if (!strcmp(k, merge.defaulttoupstream)) { @@ -866,7 +872,7 @@ static int finish_automerge(struct commit *head, free_commit_list(common); parents = remoteheads; - if (!head_subsumed || fast_forward == FF_NO) + if (!head_subsumed || (fast_forward == FF_NO || fast_forward == FF_ONLY_MERGE)) commit_list_insert(head, parents); strbuf_addch(merge_msg, '\n'); prepare_to_commit(remoteheads); @@ -1162,6 +1168,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix) if (squash) { if (fast_forward == FF_NO) die(_(You cannot combine --squash with --no-ff.)); + if (fast_forward == FF_ONLY_MERGE) + die(_(You cannot combine --squash with --ff-only-merge.)); option_commit = 0; } @@ -1206,7 +1214,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) empty head)); if (squash) die(_(Squash commit into empty head not supported yet)); - if (fast_forward == FF_NO) + if (fast_forward == FF_NO || fast_forward == FF_ONLY_MERGE) die(_(Non-fast-forward commit does not make sense into an empty head)); remoteheads = collect_parents(head_commit, head_subsumed, argc, argv); @@ -1292,6 +1300,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) setenv(buf.buf, merge_remote_util(commit)-name, 1); strbuf_reset(buf); if (fast_forward != FF_ONLY + fast_forward != FF_ONLY_MERGE merge_remote_util(commit) merge_remote_util(commit)-obj merge_remote_util(commit)-obj-type == OBJ_TAG) @@ -1312,7 +1321,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix) for (i = 0; i use_strategies_nr; i++) { if (use_strategies[i]-attr NO_FAST_FORWARD) - fast_forward = FF_NO; + if(fast_forward != FF_ONLY_MERGE) + fast_forward = FF_NO; if (use_strategies[i]-attr NO_TRIVIAL) allow_trivial = 0; } @@ -1342,9 +1352,19 @@ int cmd_merge(int argc, const char **argv, const char *prefix) */ finish_up_to_date(Already up-to-date.); goto done; - } else if (fast_forward != FF_NO !remoteheads-next - !common-next - !hashcmp(common-item-object.sha1, head_commit-object.sha1)) { + } else if (fast_forward != FF_NO + !remoteheads-next + !common-next +
Re: Can I fetch an arbitrary commit by sha1?
Duy Nguyen pclo...@gmail.com writes: Hmm.. Junio already did most of the work in 051e400 (helping smart-http/stateless-rpc fetch race - 2011-08-05), so all we need to do is enable uploadpack.allowtipsha1inwant and apply this patch Not that patch, I would think. I would understand if !stateless_rpc and !allowtipsha1 then it is an error, though. -- 8 -- diff --git a/upload-pack.c b/upload-pack.c index c789ec0..493f8ee 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -454,10 +454,6 @@ static void check_non_tip(void) char namebuf[42]; /* ^ + SHA-1 + LF */ int i; - /* In the normal in-process case non-tip request can never happen */ - if (!stateless_rpc) - goto error; - cmd.argv = argv; cmd.git_cmd = 1; cmd.no_stderr = 1; -- 8 -- If we already let smart-http do this, I don't see any harm in letting git protocol do the same (even though it's the the original reason why this code exists). -- 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 08/16] t5304: use helper to report failure of test foo = bar
Michael Haggerty mhag...@alum.mit.edu writes: I don't like the three-argument version of test_eq. Wouldn't using a comparison operator other than = would be very confusing, given that eq is in the name of the function? It also doesn't look like you use this feature. An alternative direction to go would be to specialize the function for equality testing and delegate to test_cmp to get better output for failures, but optimized to avoid excess process creation in the happy path: test_eq () { if test $1 != $2 then printf %s $1 expect printf %s $2 actual test_cmp expect actual fi } (but using properly-created temporary file names). I agree that it would be a good idea to use a randomly generated temporary filename that does not collide, as long as we make sure not to leave cruft in the working tree of the test and we leave the file there when the test script is run under -i or -d option. The above superficially looks nice; ! test_eq a b would give useless output under -v, and test_ne a b needs to be added if you go that route, though. Anyway, with the version posted, you cannot do ! test_eq a b, either but with test_eq a b !=, you do not have to. Side note. Yes, now I looked at it again, I agree that the three-arg form is awkwards in at least two ways. The operator, if we are to take one, should be infix, and the name eq no longer matches what it does when it is given an operator. The function is similar to test_cmp which takes two files but takes two strings, so test_cmp_str or something perhaps (we already have test_cmp_rev to compare two revisions, and the suggested name follows that pattern)? -- 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] use skip_prefix() to avoid more magic numbers
Jeff King p...@peff.net writes: On Sun, Oct 05, 2014 at 03:49:19PM -0700, Jonathan Nieder wrote: --- a/builtin/branch.c +++ b/builtin/branch.c @@ -81,14 +81,16 @@ static int parse_branch_color_slot(const char *var, int ofs) static int git_branch_config(const char *var, const char *value, void *cb) { + const char *slot_name; + if (starts_with(var, column.)) return git_column_config(var, value, branch, colopts); if (!strcmp(var, color.branch)) { branch_use_color = git_config_colorbool(var, value); return 0; } - if (starts_with(var, color.branch.)) { - int slot = parse_branch_color_slot(var, 13); + if (skip_prefix(var, color.branch., slot_name)) { + int slot = parse_branch_color_slot(var, slot_name - var); I wonder why the separate var and offset parameters exist in the first place. Wouldn't taking a single const char * so the old code could use 'var + 13' instead of 'var, 13' have worked? I think this is in the same boat as parse_diff_color_slot, which I fixed in 9e1a5eb (parse_diff_color_slot: drop ofs parameter, 2014-06-18). The short of it is that the function used to want both the full name and the slot name, but now needs only the latter. The fix you proposed below is along the same line, and looks good to me (and grepping for 'var *+ *ofs' shows only the two sites you found, so hopefully that is the last of it). So perhaps we would want this change as a preliminary separate patch? Thanks @@ -809,18 +808,19 @@ static void parse_commit_header(struct format_commit_context *context) int i; for (i = 0; msg[i]; i++) { + const char *name; ident instead of name, maybe? (since it's a 'name email timestamp' field) Yeah, agreed. -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] use skip_prefix() to avoid more magic numbers
Jeff King p...@peff.net writes: On Sun, Oct 05, 2014 at 03:49:19PM -0700, Jonathan Nieder wrote: --- a/builtin/branch.c +++ b/builtin/branch.c @@ -81,14 +81,16 @@ static int parse_branch_color_slot(const char *var, int ofs) static int git_branch_config(const char *var, const char *value, void *cb) { + const char *slot_name; + if (starts_with(var, column.)) return git_column_config(var, value, branch, colopts); if (!strcmp(var, color.branch)) { branch_use_color = git_config_colorbool(var, value); return 0; } - if (starts_with(var, color.branch.)) { - int slot = parse_branch_color_slot(var, 13); + if (skip_prefix(var, color.branch., slot_name)) { + int slot = parse_branch_color_slot(var, slot_name - var); I wonder why the separate var and offset parameters exist in the first place. Wouldn't taking a single const char * so the old code could use 'var + 13' instead of 'var, 13' have worked? I think this is in the same boat as parse_diff_color_slot, which I fixed in 9e1a5eb (parse_diff_color_slot: drop ofs parameter, 2014-06-18). The short of it is that the function used to want both the full name and the slot name, but now needs only the latter. The fix you proposed below is along the same line, and looks good to me (and grepping for 'var *+ *ofs' shows only the two sites you found, so hopefully that is the last of it). builtin/commit.c::parse_status_slot() has the same construct. -- 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] use skip_prefix() to avoid more magic numbers
René Scharfe l@web.de writes: @@ -335,20 +337,18 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags, static struct { int kind; const char *prefix; - int pfxlen; } ref_kind[] = { - { REF_LOCAL_BRANCH, refs/heads/, 11 }, - { REF_REMOTE_BRANCH, refs/remotes/, 13 }, + { REF_LOCAL_BRANCH, refs/heads/ }, + { REF_REMOTE_BRANCH, refs/remotes/ }, }; /* Detect kind */ for (i = 0; i ARRAY_SIZE(ref_kind); i++) { prefix = ref_kind[i].prefix; - if (strncmp(refname, prefix, ref_kind[i].pfxlen)) - continue; - kind = ref_kind[i].kind; - refname += ref_kind[i].pfxlen; - break; + if (skip_prefix(refname, prefix, refname)) { + kind = ref_kind[i].kind; + break; + } This certainly makes it easier to read. I suspect that the original was done as a (possibly premature) optimization to avoid having to do strlen(3) on a variable that points at constant strings for each and every ref we iterate with for_each_rawref(), and it is somewhat sad to see it lost because skip_prefix() assumes that the caller never knows the length of the prefix, though. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: let git grep consider sub projects
Olaf Hering o...@aepfle.de writes: How can I teach 'git grep $string' to also consider subprojects? The build system of xen.git clones 4 other trees into a directory in its source tree during build. It would be nice if my 'git grep' searches also in these cloned subdirs. Somehow there must be some knowledge because 'git clean -dfx' leaves them alone, while 'git clean -dffx' wipes everything. Olaf PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to really clean everything before starting over. Is submodule foreach under-advertised or with less than adequate features? -- 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 svn's performance issue and strange pauses, and other thing
-- On Tue, Oct 7, 2014 00:51 BST Hin-Tak Leung wrote: -- On Sun, Oct 5, 2014 02:02 BST Eric Wong wrote: snipped Hin-Tak: have you tried Jakob's patches? I've taken another look, signed-off and pushed to my master. ... Then I changed my mind, and decided what the hell, let's clone the whole thing again :-). So I made a new directory, run 'git init', just copy .git/config from the old reop and am doing 'git svn fetch --all' in the new empty directory again. So far it seems to be good. But I am only at revision 35700-ish at the moment, and the whole thing is 66700-ish. Oh, I forgot to mention that the strange pauses seem to be followed by messages like these: W:svn cherry-pick ignored (/branches/R-2-12-branch:52939,54476,55265) - missing 492 commit(s) (eg 9bf20dca6a8b05dff28e6486b1613f10825972c9) W:svn cherry-pick ignored (/branches/R-2-13-branch:55265,55432) - missing 231 commit(s) (eg 9290cf6ce2d7f6cca168cf326eed6e9fe760895f) W:svn cherry-pick ignored (/branches/R-2-15-branch:58894,59717) - missing 405 commit(s) (eg ed84a373b33f728949edf3371829fc3414c343a8) W:svn cherry-pick ignored (/branches/R-3-0-branch:62497) - missing 154 commit(s) (eg 9e4742d201771c9658417c2d2f83838e550e3162) W:svn cherry-pick ignored (/trunk: So presumably I'd only see interesting behavior when there are a number of branches. It seems the first branches are around revision 48000-ish, so I might have to wait a bit. So far, the new clone hasn't created .git/svn/.caches/ yet; and memory consumption seems okay also. The changes definitely improve, as far as my impression goes. There was only one notable pause around r50651, and it is probably because the rather large Checking svn:mergeinfo changes since r15413 from r15413? That took about 12 minutes. Other instances of W:svn cherry-pick ignored though do take a while, are in the seconds region - before the code changes they could be minutes, if memory serves. -- M src/library/tools/R/toHTML.R r50650 = bed91d435c535f2643cf0d48623fecf86d264bd9 (refs/remotes/trunk) M src/modules/X11/rotated.c M src/modules/X11/dataentry.c Checking svn:mergeinfo changes since r15413: 1 sources, 1 changed W:svn cherry-pick ignored (/trunk:28840) - missing 9372 commit(s) (eg cea6142c76300539a0d0c9c743738e31a9f7d523) r50651 = ad139a5bf91f9ad6690ff5fb4a3f71cea591a944 (refs/remotes/R-uthreads) -- The new clone has: -- $ ls -ltr .git/svn/.caches/ total 144788 -rw-rw-r--. 1 Hin-Tak Hin-Tak 1166138 Oct 7 13:44 lookup_svn_merge.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak 72849741 Oct 7 13:48 check_cherry_pick.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak 1133855 Oct 7 13:49 has_no_changes.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak 73109005 Oct 7 13:53 _rev_list.yaml -- The old clone has: --- $ ls -ltr .git/svn/.caches/ total 318824 -rw-rw-r--. 1 Hin-Tak Hin-Tak 5711724 Jul 24 2012 lookup_svn_merge.db -rw-rw-r--. 1 Hin-Tak Hin-Tak 30523628 Jul 24 2012 check_cherry_pick.db -rw-rw-r--. 1 Hin-Tak Hin-Tak296592 Jul 24 2012 has_no_changes.db -rw-rw-r--. 1 Hin-Tak Hin-Tak 40241189 Oct 5 16:42 lookup_svn_merge.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak 225323456 Oct 5 16:49 check_cherry_pick.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak242547 Oct 5 16:49 has_no_changes.yaml -rw-rw-r--. 1 Hin-Tak Hin-Tak 24120007 Oct 5 16:50 _rev_list.yaml -- I had to suspend somewhat around r59000 - but it is interesting to see that the max memory consumption of the later part is almost double? and it also runs at 100% rather than 60% overall; I don't know what to make of that - probably just smaller changes versus larger ones, or different time of day and network loads (yes, I guess it is just bandwidth-limited?, since the bulk of CPU time is in system rather than user). I am somwhat worry about the dramatic difference between the two .svn/.caches - check_cherry_pick.yaml is 225MB in one and 73MB in the other, and also _rev_list.yaml is opposite - 24MB vs 73MB. How do I reconcile that? -- M src/main/dotcode.c M doc/NEWS.Rd r59140 = b6014a226aebf9e016c89c0bd1aca1979796a057 (refs/remotes/trunk) M src/main/dotcode.c M doc/NEWS.Rd Checking svn:mergeinfo changes since r59138: 4 sources, 1 changed W:svn cherry-pick ignored (/trunk:59137,59140) - missing 369 commit(s) (eg 8a2a36083ba39be27fc9940acc3f51eab6a7a0c3) r59141 = 38c6d05f164d34e4b5cc545bda387be9d910f748 (refs/remotes/R-2-15-branch) Connection timed out: Connection timed out at /usr/share/perl5/vendor_perl/Git/SVN/Ra.pm line 290. Command exited with non-zero status 1 Command being timed: git svn fetch --all User time (seconds): 5642.19 System time (seconds): 23552.44 Percent of CPU this job got: 57% Elapsed (wall clock) time (h:mm:ss or m:ss): 14:06:58 Average shared text size (kbytes): 0 Average unshared data size (kbytes): 0 Average stack size (kbytes): 0
Apparent bug in git rebase with a merge commit
If you have a git tree and you merge in another, independent git tree so that they are the same, using a merge strategy like this: $ git merge importing/master -s recursive -Xours And if you later on want to rebase this merge commit on a newer upstream for whatever reason, you get something like this: $ git rebase -s recursive -Xours First, rewinding head to replay your work on top of it... fatal: Could not parse object 'ca59931ee67fc01b4db4278600d3d92aece898f4^' Unknown exit code (128) from command: git-merge-recursive ca59931ee67fc01b4db4278600d3d92aece898f4^ -- HEAD ca59931ee67fc01b4db4278600d3d92aece898f4 The reason this occurs is that the first commit of the newly-merged-in code obviously has no parent, so I guess the search for the common ancestor is going to be doomed to fail. It is possible that I'm misunderstanding the recursive merge strategy; however if this were the case I'd still expect a human-readable error message explaining my mistake rather than a 128 exit code. For a workaround I'll just re-create the commit, but I thought I'd report this behavior anyway. -- - DML -- 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] use skip_prefix() to avoid more magic numbers
On Tue, Oct 07, 2014 at 11:21:58AM -0700, Junio C Hamano wrote: The fix you proposed below is along the same line, and looks good to me (and grepping for 'var *+ *ofs' shows only the two sites you found, so hopefully that is the last of it). builtin/commit.c::parse_status_slot() has the same construct. Thanks, I missed it because it uses offset instead of ofs. -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] git-merge: mutually match SYNOPSIS and usage.
Sergey Organov sorga...@gmail.com writes: SYNOPSIS section of the git-merge manual page had outdated explicit list of options. usage returned by 'git merge -h' didn't have -m msg that is one of essential distinctions between obsolete invocation form and the recent one. Signed-off-by: Sergey Organov sorga...@gmail.com --- Please do not do two unrelated things in a single change. It may be a clear and very welcome improvement to change from explicitly list only often used options to just say [options] and have the list of options and their descriptions. I am not sure about the other change to single out -m msg, especially marking it as optional by enclosing it inside [-m msg], makes much sense, as that is still not very easily distinguishable from git merge [options] [commit...]. In other words, I agree with your motivation to call for attention that the command behaves differently with and without -m, but I do not think that part of the change in this patch achieves it well. Documentation/git-merge.txt | 6 ++ builtin/merge.c | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index cf2c374..e24a1d4 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -9,10 +9,8 @@ git-merge - Join two or more development histories together SYNOPSIS [verse] -'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit] - [-s strategy] [-X strategy-option] [-S[key-id]] - [--[no-]rerere-autoupdate] [-m msg] [commit...] -'git merge' msg HEAD commit... +'git merge' [options] [-m msg] [commit...] +'git merge' [options] msg HEAD commit... 'git merge' --abort DESCRIPTION diff --git a/builtin/merge.c b/builtin/merge.c index dff043d..086502f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -40,7 +40,7 @@ struct strategy { }; static const char * const builtin_merge_usage[] = { - N_(git merge [options] [commit...]), + N_(git merge [options] [-m msg] [commit...]), N_(git merge [options] msg HEAD commit), N_(git merge --abort), NULL -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git-merge: implement --ff-only-merge option.
Sergey Organov sorga...@gmail.com writes: This option allows to create merge commit when fast-forward is possible, and abort otherwise. I.e. it's equivalent to --ff-only, except that it finally creates merge commit instead of fast-forwarding. One may also consider this option to be equivalent to --no-ff with additional check that the command without --no-ff would indeed result in fast-forward. Useful to incorporate topic branch as single merge commit, ensuring the left-side of the merge has no changes (our-diff-empty-merge). Signed-off-by: Sergey Organov sorga...@gmail.com --- The workflow this implements sounds like because we can, not because it will help us do X and Y and Z. Why would it be useful to limit the history to a shape where all merges are the ones that could have been fast-forwarded? I cannot justify that sensibly myself, which in turn makes the feature smell to me that it is encouraging a wrong workflow. -- 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: Colors in __git_ps1
Dear Mantas, On 07/10/14 18:18, Mantas Mikulėnas wrote: There was a question in #git recently on why __git_ps1 from git-prompt.sh does not allow colors in $PS1 mode. From what I see, commit 1bfc51ac8141 talks about the need for \[ and \] assignments in $PS1, and commit cf4cac4cfc13 says that PS1 mode turns off color support since \[ and \] won't work; as I understand it, this is because bash will output literal \[ if __git_ps1 tries to echo it. Internally, \[ and \] are translated by bash into 0x01 and 0x02 before passing the final string to readline. So there *is* a way to achieve the same effect in PS1 mode – __git_ps1 just needs to echo \001 and \002 before after the color codes, and readline will interpret them correctly when calculating the prompt width. For example, echo $'foo\001\e[1m\002bar'. Might be worth considering, even if $PS1 mode is inferior to $PROMPT_COMMAND mode for other reasons. I would actually prefer the PS1='$(__git_ps)' like way, as it is more generic and intuitive when you know generic Bourne shell ways. The reason to go for PROMPT_COMMAND was that it appears to be the only way to set the PS1 prompt and let bash keep track of the length of the prompt properly. This is needed to allow browsing the bash history with long(er than width of the terminal) command lines without the terminal getting messed up. And I'm still not sure PROMPT_COMMAND mode fixes this properly, but it passed the tests where command substitution mode failed. IMHO the way colours are now implemented could very well be considered a workaround for a bug in bash. Only I'm not skilled enough (or have enough time) to get to the bottom of it... I hope this answers your question. Cheers Simon -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] MinGW(-W64) compilation
Johannes Sixt j...@kdbg.org writes: Am 06.10.2014 um 07:17 schrieb Marat Radchenko: On Tue, Sep 30, 2014 at 11:02:29AM +0400, Marat Radchenko wrote: This patch series fixes building on modern MinGW and MinGW-W64 (including x86_64!). Junio, ping? Sorry, I forgot to report that this updated series works now for me. The patches all look reasonable. I don't have an opinion on the restriction that MSVC 2010 can't be used anymore (path 08/14). So, is that an Ack, or would you prefer to cook this first in msysgit tree and then feed the result as part of This series is to shrink the difference between the mainline and msysgit later? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: let git grep consider sub projects
On 7 October 2014 20:25, Junio C Hamano gits...@pobox.com wrote: Olaf Hering o...@aepfle.de writes: How can I teach 'git grep $string' to also consider subprojects? The build system of xen.git clones 4 other trees into a directory in its source tree during build. It would be nice if my 'git grep' searches also in these cloned subdirs. Somehow there must be some knowledge because 'git clean -dfx' leaves them alone, while 'git clean -dffx' wipes everything. Olaf PS: Sometimes I miss a 'git clean -dfx --also-sub-repos' useful to really clean everything before starting over. Is submodule foreach under-advertised or with less than adequate features? It sounds like in these use cases, you would want the commands to run on all the submodules but also in the parent repo, am I wrong in thinking that git submodule foreach does only the former part? So you would either need to make a wrapper thing yourself or run the command twice. In the first case with the git grep, I can also imagine that with some nontrivial patterns, having to quote the metacharacters not only once, but twice, can be a significant annoyance. Eg, first protect it from git submodule foreach parsing it, and then from the shell running the individual commands. -- Mikael Magnusson -- 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] use skip_prefix() to avoid more magic numbers
On Tue, Oct 07, 2014 at 11:14:35AM -0700, Junio C Hamano wrote: The fix you proposed below is along the same line, and looks good to me (and grepping for 'var *+ *ofs' shows only the two sites you found, so hopefully that is the last of it). So perhaps we would want this change as a preliminary separate patch? Here it is on top of master, with a commit message (and including the other case you found). I've attributed it to Jonathan, who did most of the work. We'd need his signoff and approval, of course. -- 8 -- From: Jonathan Nieder jrnie...@gmail.com Subject: pass config slots as pointers instead of offsets Many config-parsing helpers, like parse_branch_color_slot, take the name of a config variable and an offset to the slot name (e.g., color.branch.plain is passed along with 13 to effectively pass plain). This is leftover from the time that these functions would die() on error, and would want the full variable name for error reporting. These days they do not use the full variable name at all. Passing a single pointer to the slot name is more natural, and lets us more easily adjust the callers to use skip_prefix to avoid manually writing offset numbers. This is effectively a continuation of 9e1a5eb, which did the same for parse_diff_color_slot. This patch covers all of the remaining similar constructs. Signed-off-by: Jeff King p...@peff.net --- Actually, parse_decorate_color_config is slightly odd in that it takes the variable and the slot_name after this patch. That's because it passes the full variable name to color_parse, which does still die() on error. Perhaps it should be converted to return an error, too. builtin/branch.c | 16 builtin/commit.c | 19 +-- builtin/log.c| 2 +- log-tree.c | 4 ++-- log-tree.h | 2 +- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 9e4666f..2c97141 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -62,19 +62,19 @@ static unsigned char merge_filter_ref[20]; static struct string_list output = STRING_LIST_INIT_DUP; static unsigned int colopts; -static int parse_branch_color_slot(const char *var, int ofs) +static int parse_branch_color_slot(const char *slot) { - if (!strcasecmp(var+ofs, plain)) + if (!strcasecmp(slot, plain)) return BRANCH_COLOR_PLAIN; - if (!strcasecmp(var+ofs, reset)) + if (!strcasecmp(slot, reset)) return BRANCH_COLOR_RESET; - if (!strcasecmp(var+ofs, remote)) + if (!strcasecmp(slot, remote)) return BRANCH_COLOR_REMOTE; - if (!strcasecmp(var+ofs, local)) + if (!strcasecmp(slot, local)) return BRANCH_COLOR_LOCAL; - if (!strcasecmp(var+ofs, current)) + if (!strcasecmp(slot, current)) return BRANCH_COLOR_CURRENT; - if (!strcasecmp(var+ofs, upstream)) + if (!strcasecmp(slot, upstream)) return BRANCH_COLOR_UPSTREAM; return -1; } @@ -88,7 +88,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) return 0; } if (starts_with(var, color.branch.)) { - int slot = parse_branch_color_slot(var, 13); + int slot = parse_branch_color_slot(var + 13); if (slot 0) return 0; if (!value) diff --git a/builtin/commit.c b/builtin/commit.c index b0fe784..c45613a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1271,22 +1271,21 @@ static int dry_run_commit(int argc, const char **argv, const char *prefix, return commitable ? 0 : 1; } -static int parse_status_slot(const char *var, int offset) +static int parse_status_slot(const char *slot) { - if (!strcasecmp(var+offset, header)) + if (!strcasecmp(slot, header)) return WT_STATUS_HEADER; - if (!strcasecmp(var+offset, branch)) + if (!strcasecmp(slot, branch)) return WT_STATUS_ONBRANCH; - if (!strcasecmp(var+offset, updated) - || !strcasecmp(var+offset, added)) + if (!strcasecmp(slot, updated) || !strcasecmp(slot, added)) return WT_STATUS_UPDATED; - if (!strcasecmp(var+offset, changed)) + if (!strcasecmp(slot, changed)) return WT_STATUS_CHANGED; - if (!strcasecmp(var+offset, untracked)) + if (!strcasecmp(slot, untracked)) return WT_STATUS_UNTRACKED; - if (!strcasecmp(var+offset, nobranch)) + if (!strcasecmp(slot, nobranch)) return WT_STATUS_NOBRANCH; - if (!strcasecmp(var+offset, unmerged)) + if (!strcasecmp(slot, unmerged)) return WT_STATUS_UNMERGED; return -1; } @@ -1324,7 +1323,7 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; } if (starts_with(k, status.color.) ||
Re: [PATCH v22.5 09/24] refs.c: pass a list of names to skip to is_refname_available
Jonathan Nieder jrnie...@gmail.com writes: Will take a look over there (I really hate having to context switch only to review this series, though). Here's a copy to save a trip. The patch itself looks fine, but Gerrit really needs a way to convey these changes make a single topic and here is the tip somehow. It could be that there is and it is not well advertised, but the end result is I visit the URL, https://code-review.googlesource.com/#/q/project:git+topic:ref-transaction pick and view a few patches at random to view the changes in pretty side-by-side diff output (which by itself is fine), check Download to see the fetch URL, when it is followed it often gives me only an early half of the topic, get confused and scratch my head, give up and abandon and the whole time doing so ends up being wasted. -- 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] use skip_prefix() to avoid more magic numbers
On Tue, Oct 07, 2014 at 03:16:56PM -0400, Jeff King wrote: Actually, parse_decorate_color_config is slightly odd in that it takes the variable and the slot_name after this patch. That's because it passes the full variable name to color_parse, which does still die() on error. Perhaps it should be converted to return an error, too. Actually, this doesn't let us get rid of the var in parse_decorate_color_config, because it also wants to call config_error_nonbool. However, while looking at this, I did notice that some of the error messages generated by color_parse are a bit ugly, and came up with the patch below (on top of what I just sent, but it could be separate). -- 8 -- Subject: color_parse: do not mention variable name in error message Originally the color-parsing function was used only for config variables. It made sense to pass the variable name so that the die() message could be something like: $ git -c color.branch.plain=bogus branch fatal: bad color value 'bogus' for variable 'color.branch.plain' These days we call it in other contexts, and the resulting error messages are a little confusing: $ git log --pretty='%C(bogus)' fatal: bad color value 'bogus' for variable '--pretty format' $ git config --get-color foo.bar bogus fatal: bad color value 'bogus' for variable 'command line' This patch teaches color_parse to complain only about the value, and then return an error code. Config callers can then propagate that up to the config parser, which mentions the variable name. Other callers can provide a custom message. After this patch these three cases now look like: $ git -c color.branch.plain=bogus branch error: invalid color value: bogus fatal: unable to parse 'color.branch.plain' from command-line config $ git log --pretty='%C(bogus)' error: invalid color value: bogus fatal: unable to parse --pretty format $ git config --get-color foo.bar bogus error: invalid color value: bogus fatal: unable to parse default color value Signed-off-by: Jeff King p...@peff.net --- I think the two-line errors are kind of ugly. It does match how config_error_nonbool works, which also propagates its errors, and looks like: $ git -c color.branch.plain branch error: Missing value for 'color.branch.plain' fatal: unable to parse 'color.branch.plain' from command-line config We could instead make color_parse silently return -1, and leave it up to the caller to complain (and probably add die_bad_color_config() or similar to avoid repetition in the config callers). builtin/branch.c | 3 +-- builtin/clean.c| 3 +-- builtin/commit.c | 3 +-- builtin/config.c | 9 ++--- builtin/for-each-ref.c | 6 -- color.c| 13 ++--- color.h| 4 ++-- diff.c | 3 +-- grep.c | 2 +- log-tree.c | 3 +-- pretty.c | 5 ++--- 11 files changed, 26 insertions(+), 28 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 2c97141..35c786d 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -93,8 +93,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) return 0; if (!value) return config_error_nonbool(var); - color_parse(value, var, branch_colors[slot]); - return 0; + return color_parse(value, branch_colors[slot]); } return git_color_default_config(var, value, cb); } diff --git a/builtin/clean.c b/builtin/clean.c index 3beeea6..a7e7b0b 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -116,8 +116,7 @@ static int git_clean_config(const char *var, const char *value, void *cb) return 0; if (!value) return config_error_nonbool(var); - color_parse(value, var, clean_colors[slot]); - return 0; + return color_parse(value, clean_colors[slot]); } if (!strcmp(var, clean.requireforce)) { diff --git a/builtin/commit.c b/builtin/commit.c index c45613a..407566d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1328,8 +1328,7 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; if (!v) return config_error_nonbool(k); - color_parse(v, k, s-color_palette[slot]); - return 0; + return color_parse(v, s-color_palette[slot]); } if (!strcmp(k, status.relativepaths)) { s-relative_paths = git_config_bool(k, v); diff --git a/builtin/config.c b/builtin/config.c index 37305e9..8cc2604 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -296,7 +296,8 @@ static int git_get_color_config(const char *var, const char *value, void *cb) if (!strcmp(var, get_color_slot)) { if (!value)
Re: [PATCH] git-prompt.sh: shorter equal upstream branch name
On 2014-10-07 11:57, Julien Carsique wrote: Hi, Thank you both for your feedback! I'm looking at applying your requests: - add tests, - variable renaming, - use of local, - fix multiple issues on string parsing - avoid useless bash-isms? Did you agree on the ones I should remove? I'm guessing the structure of your code will change somewhat when you address the other issues, so I think it may be premature to discuss specific Bashisms right now. (There aren't any particular Bashisms that I think should always be avoided -- I just want people to ponder whether a particular use of a Bashism is truly preferable to a POSIX-conformant alternative.) Write the code in the way you think is best, and if I see a good way to convert a Bashism to POSIX I'll let you know. And feel free to ignore me -- I'm a member of the Church of POSIX Conformance while Junio is much more grounded in reality. :) I'll send an updated patch asap. Tell me if I forgot something. Your list looks complete to me. Thank you for contributing! -Richard Regards, Julien On 01/10/2014 19:49, Junio C Hamano wrote: Richard Hansen rhan...@bbn.com writes: and there is no hope to fix them to stick to the bare-minimum POSIX, I don't think it'd be hard to convert it to pure POSIX if there was a desire to do so. Not necessarily; if you make it so slow to be usable as a prompt script, that is not a conversion. Bash-isms in the script is allowed for a reason, unfortunately. It would be unwise to go to great lengths to avoid Bashisms, but I think it would be smart to use POSIX syntax when it is easy to do so. In general, I agree with you. People who know only bash tend to overuse bash-isms where they are not necessary, leaving an unreadable mess. For the specific purpose of Julien's if the tail part of this string matches the other string, replace that with an equal sign, ${parameter/pattern/string} is a wrong bash-ism to use. But the right solution to count the length of the other string and take a substring of this string from its beginning would require other bash-isms ${#parameter} and ${parameter:offset:length}. And that's fine. -- 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] use skip_prefix() to avoid more magic numbers
Jeff King p...@peff.net writes: Subject: color_parse: do not mention variable name in error message ... I think the two-line errors are kind of ugly. It does match how config_error_nonbool works, which also propagates its errors, and looks like: $ git -c color.branch.plain branch error: Missing value for 'color.branch.plain' fatal: unable to parse 'color.branch.plain' from command-line config We could instead make color_parse silently return -1, and leave it up to the caller to complain (and probably add die_bad_color_config() or similar to avoid repetition in the config callers). Yeah, in the longer term we would want to do something like that to fix both nonbool and this one, but for now this should help avoid confusion. Thanks for a nice analysis, write-up and patch. builtin/branch.c | 3 +-- builtin/clean.c| 3 +-- builtin/commit.c | 3 +-- builtin/config.c | 9 ++--- builtin/for-each-ref.c | 6 -- color.c| 13 ++--- color.h| 4 ++-- diff.c | 3 +-- grep.c | 2 +- log-tree.c | 3 +-- pretty.c | 5 ++--- 11 files changed, 26 insertions(+), 28 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 2c97141..35c786d 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -93,8 +93,7 @@ static int git_branch_config(const char *var, const char *value, void *cb) return 0; if (!value) return config_error_nonbool(var); - color_parse(value, var, branch_colors[slot]); - return 0; + return color_parse(value, branch_colors[slot]); } return git_color_default_config(var, value, cb); } diff --git a/builtin/clean.c b/builtin/clean.c index 3beeea6..a7e7b0b 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -116,8 +116,7 @@ static int git_clean_config(const char *var, const char *value, void *cb) return 0; if (!value) return config_error_nonbool(var); - color_parse(value, var, clean_colors[slot]); - return 0; + return color_parse(value, clean_colors[slot]); } if (!strcmp(var, clean.requireforce)) { diff --git a/builtin/commit.c b/builtin/commit.c index c45613a..407566d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1328,8 +1328,7 @@ static int git_status_config(const char *k, const char *v, void *cb) return 0; if (!v) return config_error_nonbool(k); - color_parse(v, k, s-color_palette[slot]); - return 0; + return color_parse(v, s-color_palette[slot]); } if (!strcmp(k, status.relativepaths)) { s-relative_paths = git_config_bool(k, v); diff --git a/builtin/config.c b/builtin/config.c index 37305e9..8cc2604 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -296,7 +296,8 @@ static int git_get_color_config(const char *var, const char *value, void *cb) if (!strcmp(var, get_color_slot)) { if (!value) config_error_nonbool(var); - color_parse(value, var, parsed_color); + if (color_parse(value, parsed_color) 0) + return -1; get_color_found = 1; } return 0; @@ -309,8 +310,10 @@ static void get_color(const char *def_color) git_config_with_options(git_get_color_config, NULL, given_config_source, respect_includes); - if (!get_color_found def_color) - color_parse(def_color, command line, parsed_color); + if (!get_color_found def_color) { + if (color_parse(def_color, parsed_color) 0) + die(_(unable to parse default color value)); + } fputs(parsed_color, stdout); } diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index fda0f04..7ee86b3 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -671,7 +671,8 @@ static void populate_value(struct refinfo *ref) } else if (starts_with(name, color:)) { char color[COLOR_MAXLEN] = ; - color_parse(name + 6, --format, color); + if (color_parse(name + 6, color) 0) + die(_(unable to parse format)); v-s = xstrdup(color); continue; } else if (!strcmp(name, flag)) { @@ -1004,7 +1005,8 @@ static void show_ref(struct refinfo *info, const char *format, int quote_style) struct atom_value resetv; char color[COLOR_MAXLEN] = ; - color_parse(reset, --format, color); + if (color_parse(reset, color) 0) +
Re: [PATCH v4] MinGW(-W64) compilation
Am 07.10.2014 um 21:01 schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Am 06.10.2014 um 07:17 schrieb Marat Radchenko: On Tue, Sep 30, 2014 at 11:02:29AM +0400, Marat Radchenko wrote: This patch series fixes building on modern MinGW and MinGW-W64 (including x86_64!). Junio, ping? Sorry, I forgot to report that this updated series works now for me. The patches all look reasonable. I don't have an opinion on the restriction that MSVC 2010 can't be used anymore (path 08/14). So, is that an Ack, or would you prefer to cook this first in msysgit tree and then feed the result as part of This series is to shrink the difference between the mainline and msysgit later? At this time, it's really just a works for me report, because I didn't look carefully at each patch. I may do that at some time later. If someone from the msysgit crew (I don't count myself to it) could report works for us, too, then the series could go into your tree right away, in my opinion. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Read and reply to me.
-- Hello my dear,i knows that this letter will not come as a surprise to you,i saw your data online and i decided to have this brief conversation with you which i knows that it must favor me and you, i have a good business proposal to discuss with you but on your reply to this brief letter i will reveal more details of the discussions i have with you. I hope you will enjoy my communication and company with me,i will expect your good response while you will have a very good day and blessed week. This is my email address paulamadi...@yahoo.com . Thanks. Ms Paula Madiit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-prompt.sh: shorter equal upstream branch name
Julien Carsique julien.carsi...@gmail.com writes: Hi, Thank you both for your feedback! I'm looking at applying your requests: - add tests, - variable renaming, - use of local, - fix multiple issues on string parsing - avoid useless bash-isms? Did you agree on the ones I should remove? I'll send an updated patch asap. Tell me if I forgot something. I just re-read the comments in the thread, and a few things look missing: - git-svn? - conditionally enable this feature? Other than that the above list looks like a fairly good one. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/16] t5304: use helper to report failure of test foo = bar
On Tue, Oct 07, 2014 at 10:29:59AM -0700, Junio C Hamano wrote: test_eq () { if test $1 != $2 then printf %s $1 expect printf %s $2 actual test_cmp expect actual fi } [...] The above superficially looks nice; ! test_eq a b would give useless output under -v, and test_ne a b needs to be added if you go that route, though. Yeah, that is why I ended up with the operator as a parameter. I modeled after test_line_count, which faces the same problem (negation must happen in the operator, not the full command). Anyway, with the version posted, you cannot do ! test_eq a b, either but with test_eq a b !=, you do not have to. Side note. Yes, now I looked at it again, I agree that the three-arg form is awkwards in at least two ways. The operator, if we are to take one, should be infix, and the name eq no longer matches what it does when it is given an operator. I made it postfix because it's optional, and my inclination is to handle arguments left-to-right, since that extends to multiple optional arguments. But of course we have just one optional argument and can simply treat 2-arg and 3-arg forms differently. However, Michael noted that this is really just 'test $@', and I think we should allow any test parameters. The function is similar to test_cmp which takes two files but takes two strings, so test_cmp_str or something perhaps (we already have test_cmp_rev to compare two revisions, and the suggested name follows that pattern)? Based on your responses, I'm leaning towards: test_cmp_str() { test $@ return 0 echo 2 command failed: test $* return 1 } since the point is really just to print _something_ when the test fails (any quoting or whitespace may be wrong, of course, but that's OK; it's for human consumption, and is just a hint). Maybe str is not right here. Michael suggested test_test which is semantically what we want, but just looks silly[1]. Maybe test_pred or something? test_cond? Or I guess going the other way, sane_test or verbose_test or something. I think test_cond is my favorite of those. -Peff [1] Of course, we could always do test_[. :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git-merge: implement --ff-only-merge option.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: This option allows to create merge commit when fast-forward is possible, and abort otherwise. I.e. it's equivalent to --ff-only, except that it finally creates merge commit instead of fast-forwarding. One may also consider this option to be equivalent to --no-ff with additional check that the command without --no-ff would indeed result in fast-forward. Useful to incorporate topic branch as single merge commit, ensuring the left-side of the merge has no changes (our-diff-empty-merge). Signed-off-by: Sergey Organov sorga...@gmail.com --- The workflow this implements sounds like because we can, not because it will help us do X and Y and Z. Well, I do have full time job, and while I think I can instantly invent quite a few things from the because we can camp, I usually don't. Why would it be useful to limit the history to a shape where all merges are the ones that could have been fast-forwarded? Except by true merge, how else can I express with git that 'n' consequitive commits constitute single logical change (being originally some topic branch)? Now I just want such special kind of merge to be entirely trivial merge on one side. i.e. perfectly clean merge every time. Moreover, as topic branches are usually rebased before merge anyway, why shouldn't I have simple capability to enforce it? I cannot justify that sensibly myself, which in turn makes the feature smell to me that it is encouraging a wrong workflow. What's wrong, exactly, in enforcing rebasing of topic branches before merge? Basically, I need --ff-only, only I don't want git to forget that this entire set of commits is logically single unit. Neither do I want to loose the structure of commits that --squash offers. -- Sergey. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git-merge: implement --ff-only-merge option.
Sergey Organov sorga...@gmail.com writes: Why would it be useful to limit the history to a shape where all merges are the ones that could have been fast-forwarded? Except by true merge, how else can I express with git that 'n' consequitive commits constitute single logical change (being originally some topic branch)? You are justifying --no-ff, aren't you? Moreover, as topic branches are usually rebased before merge anyway, why shouldn't I have simple capability to enforce it? Because rebasing immediately before is considered a bad manner, i.e. encouraging a wrong workflow? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-merge: mutually match SYNOPSIS and usage.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: SYNOPSIS section of the git-merge manual page had outdated explicit list of options. usage returned by 'git merge -h' didn't have -m msg that is one of essential distinctions between obsolete invocation form and the recent one. Signed-off-by: Sergey Organov sorga...@gmail.com --- Please do not do two unrelated things in a single change. Well, I thought they are related, sorry. It may be a clear and very welcome improvement to change from explicitly list only often used options to just say [options] and have the list of options and their descriptions. OK, noticed. I am not sure about the other change to single out -m msg, especially marking it as optional by enclosing it inside [-m msg], makes much sense, as that is still not very easily distinguishable from git merge [options] [commit...]. I was looking at the merge.c code, and that's how it seems to work. You can get new semantics without -m, and you can't get old semantics with -m, isn't it? It looks like the set of descriptions I produced is formally correct. In other words, I agree with your motivation to call for attention that the command behaves differently with and without -m, but I do not think that part of the change in this patch achieves it well. Any particular suggestion? Thanks. -- Sergey. -- 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 08/16] t5304: use helper to report failure of test foo = bar
Jeff King p...@peff.net writes: On Tue, Oct 07, 2014 at 10:29:59AM -0700, Junio C Hamano wrote: ... The function is similar to test_cmp which takes two files but takes two strings, so test_cmp_str or something perhaps (we already have test_cmp_rev to compare two revisions, and the suggested name follows that pattern)? Based on your responses, I'm leaning towards: test_cmp_str() { test $@ return 0 echo 2 command failed: test $* return 1 } since the point is really just to print _something_ when the test fails (any quoting or whitespace may be wrong, of course, but that's OK; it's for human consumption, and is just a hint). Yeah, if we are going to reduce it down to the above implementation, intereseting things like test -f $frotz will become possible and cmp-str stops making sense. It really is about We run test and expect it to yield true. Report the failure a bit more prominently under the '-v' option to help us debug. So among the ones you listed, test_verbose may be the least silly, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git-merge: implement --ff-only-merge option.
Junio C Hamano gits...@pobox.com writes: Sergey Organov sorga...@gmail.com writes: Why would it be useful to limit the history to a shape where all merges are the ones that could have been fast-forwarded? Except by true merge, how else can I express with git that 'n' consequitive commits constitute single logical change (being originally some topic branch)? You are justifying --no-ff, aren't you? Yes, and I already said it's close. But I don't want such merge commit to contain any non-trivialities. Currently I check it manually before issuing merge --no-ff and was hoping for some automation. Moreover, as topic branches are usually rebased before merge anyway, why shouldn't I have simple capability to enforce it? Because rebasing immediately before is considered a bad manner, i.e. encouraging a wrong workflow? Why? What is wrong about it? Please also notice that I don't try to impose this on anybody who does consider it wrong workflow. -- Sergey. -- 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 08/16] t5304: use helper to report failure of test foo = bar
Jeff King p...@peff.net writes: Based on your responses, I'm leaning towards: test_cmp_str() { test $@ return 0 echo 2 command failed: test $* return 1 } since the point is really just to print _something_ when the test fails (any quoting or whitespace may be wrong, of course, but that's OK; it's for human consumption, and is just a hint). If we really cared, we could do echo 2 command failed: test $(git rev-parse --sq-quote $@) perhaps? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 10:15:47AM -0700, Junio C Hamano wrote: Christian Couder christian.cou...@gmail.com writes: On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin m...@redhat.com wrote: ... As a reminder, this old patchset (that I replied to) enhanced git am -s with an option to add different signatures depending on the option passed to the -s flag. E.g. I have [am a] signoff = Acked-by: Michael S. Tsirkin m...@redhat.com [am r] signoff = Reviewed-by: Michael S. Tsirkin m...@redhat.com [am t] signoff = Tested-by: Michael S. Tsirkin m...@redhat.com and now: git am -s art adds all 3 signatures when applying the patch. This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin m...@redhat.com --trailer Tested-by: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch and then: $ git am to_apply/*.patch If I understand it correctly, Michael is envisioning to implement his git am -s art (I would recommend against reusing -s for this, though. git am --trailer art is fine) and doing so by using interpret-trailers as an internal implementation detail, so I would say the above is a perfectly fine way to do so. An equivalent of that command line is synthesized and run internally in his version of git am when his git am sees --trailer art option using those am.{a,r,t}.trailer configuration variables. Hmm I wonder why do you dislike reusing -s with a parameter for this. To me, this looks like a superset of the default -s functionality: -s adds the default signature, -s x adds signature x ... Users don't really care that one is implemented as a trailer and another isn't. In fact, default -s can be implemented as a trailer too, right? Could you clarify please? -- MST -- 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 08/16] t5304: use helper to report failure of test foo = bar
On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote: Yeah, if we are going to reduce it down to the above implementation, intereseting things like test -f $frotz will become possible and cmp-str stops making sense. It really is about We run test and expect it to yield true. Report the failure a bit more prominently under the '-v' option to help us debug. We already have test_path_is_file to do the same thing just for -f. We could in theory switch all of those to this new, more generic wrapper. I don't know if it is worth doing a mass-conversion, but we could discourage test_path_is_file in new tests. We could also implement test_path_is_{dir,file} on top of this. There is also test_path_is_missing, which would need the negated form. We'd either need a test_not_cond, or to allow: test_cond ! -e path That is specified by POSIX. I seem to recall that we ran into problems using it with some shells, but I note there is currently some use of it in t5304. So perhaps it is fine. So among the ones you listed, test_verbose may be the least silly, I would think. Somehow test_verbose seems to me like checking the verbose option of the test suite. I prefer test_cond, but I do not feel too strongly, if you want to override me. (any quoting or whitespace may be wrong, of course, but that's OK; it's for human consumption, and is just a hint). If we really cared, we could do echo 2 command failed: test $(git rev-parse --sq-quote $@) perhaps? Yeah, that would work. I am always a little hesitant sticking git commands into our test infrastructure, since we may end up masking errors due to our own bug. But we can probably rely on --sq-quote working sanely (and anyway, we're not even affecting the test outcome here). -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 RFC] git-am: support any number of signatures
On Wed, Sep 24, 2014 at 12:00:40PM +0200, Christian Couder wrote: On Tue, Sep 23, 2014 at 10:07 AM, Michael S. Tsirkin m...@redhat.com wrote: On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote: This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin m...@redhat.com --trailer Tested-by: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch and then: $ git am to_apply/*.patch Also by using something like: $ git config trailer.a.key Acked-by $ git config trailer.r.key Reviewed-by $ git config trailer.t.key Tested-by I would like multiple keys to match a specific letter, e.g. as a maintainer I need both reviewed by and signed off by when I apply a patch, I like applying them with a single -s m. That's different from what you implemented in your patch. And franckly I think that for this kind of specific use cases, you could create your own aliases, either Git aliases or just shell aliases. For example if we implement default values and make git am call git interpret-trailers, a shell alias could simply be: alias gamsm='git am --trailer r --trailer s' I use git log --oneline --decorate --graph very often, so I made my own alias for it, and I suppose a lot of other people have done so. The number of people who will use trailers will probably be much smaller than the number of people using git log, so if we don't make shortcuts for git log --oneline --decorate --graph, I see no ground to ask for a specific shortcut that adds both a reviewed by and a signed off by. I've been thinking: how about a generic ability to add option shortcuts for commands in .config? For example: [am -z] command = --trailer foobar would replace any -z in git am command line with --trailer foobar. Does this sound useful? the first command could be simplified to: $ git interpret-trailers --trailer a: Michael S. Tsirkin m...@redhat.com --trailer r: Michael S. Tsirkin m...@redhat.com --trailer t: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch And if you use an env variable: $ ME=Michael S. Tsirkin m...@redhat.com $ git interpret-trailers --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch to_apply/0001-foo.patch Maybe later we will integrate git interpret-trailers with git commit, git am and other commands, so that you can do directly: git am --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch Maybe we wil also assign a one letter shortcut to --trailer, for example z, so that could be: git am -z a: $ME -z r: $ME -z t: $ME 0001-foo.patch -s could apply here, right? I don't know what we will do with -s. Maybe if we use -z, we don't need -s. It doesn't have a parameter at the moment. We will have to discuss that kind of thing when we make it possible for git commit, git am and maybe other commands to accept trailers arguments and pass them to git interpret-trailers. In his email Junio seems to say that we don't need a shortcut like -z, we could only have --trailer. And I think that it is indeed sound to at least wait a little before using up one shortcut like -z in many commands. We could also allow many separators in the same -z argument as long as they are separated by say ~, I think -z a -z r -z t is enough. Great! I think you will likely have at least --trailer a --trailer r --trailer t, but I don't think it is too bad as you can use aliases to make it shorter to type. so you could have: git am -z a: $ME~r: $ME~t: $ME 0001-foo.patch And then we could also allow people to define default values for trailers with something like: $ git config trailer.a.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.r.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.t.defaultvalue Michael S. Tsirkin m...@redhat.com I'm kind of confused by the key/value concept. A defaultvalue would be the value used when no value is passed. The key is just what we will use in the first part of the trailer (the part before the separator). For example with the above defaultvalue and key, --trailer a: Junio gits...@pobox.com would add: Acked-by: Junio gits...@pobox.com while --trailer a would add: Acked-by: Michael S. Tsirkin m...@redhat.com Can't I define the whole 'Acked-by: Michael S. Tsirkin m...@redhat.com' string as the key? The whole 'Acked-by: Michael S. Tsirkin m...@redhat.com' is a full trailer, not a key. And it is not possible right now to define a full trailer. Maybe we could find a way to make it possible, but a default value and a way to have a small nickname for the token (like a for Acked-by) should get people quite far. And then for very specific use cases,
Re: [PATCH] git-merge: mutually match SYNOPSIS and usage.
Sergey Organov sorga...@gmail.com writes: Junio C Hamano gits...@pobox.com writes: ... I was looking at the merge.c code, and that's how it seems to work. You can get new semantics without -m, and you can't get old semantics with -m, isn't it? It looks like the set of descriptions I produced is formally correct. The thing is, with -m msg we will never fall into the traditional syntax, hence git merge -m msg msg HEAD commit appear to be allowed with git merge [options] msg HEAD commit..., but it is not. And the inverse is not true (an obvious example is git merge $branch, even though it does not have -m msg it uses the modern common. So the updated SYNOPSIS is not really helping. In other words, I agree with your motivation to call for attention that the command behaves differently with and without -m, but I do not think that part of the change in this patch achieves it well. Any particular suggestion? I was going to suggest explain how the traditional syntax is triggered in the DESCRIPTION section, but it turns out that we already do that. The second syntax (msg HEAD commit...) is supported for historical reasons. Do not use it from the command line or in new scripts. It is the same as git merge -m msg commit Strictly speaking, I think it is not qute the same---I recall vaguely that it broke tests if you replace the traditional-style invocation in 'git pull' with the -m msg syntax, but I do not have details handy; you may want to try it out if you are interested. So I would think SYNOPSIS git merge [options] commit... git merge [options] msg HEAD commit... git merge --abort should be sufficient, possibly with some clarification on The second syntax paragraph in the DESCRIPTION section. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] git-merge: implement --ff-only-merge option.
Sergey Organov sorga...@gmail.com writes: Because rebasing immediately before is considered a bad manner, i.e. encouraging a wrong workflow? Why? What is wrong about it? Searching the kernel archive for messages that talks about rebase and pull-request from Linus would tell us why it is frowned upon in a prominent early adopter project of Git. You destroy what you have been testing and replace it with an untested one. If you merge, and if the result of the merge is broken, at least you would have something that used to work at its second parent (i.e. the tip of your topic). Please also notice that I don't try to impose this on anybody who does consider it wrong workflow. I know ;-). I didn't say anything about imposing, did I? Having an option to make it easy to do something undesirable gives people an excuse to say See Git has this option to let me do that easily, it is an officially sanctioned and encouraged workflow. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] git-am: support any number of signatures
On Wed, Oct 08, 2014 at 12:29:37AM +0300, Michael S. Tsirkin wrote: If I understand it correctly, Michael is envisioning to implement his git am -s art (I would recommend against reusing -s for this, though. git am --trailer art is fine) and doing so by using interpret-trailers as an internal implementation detail, so I would say the above is a perfectly fine way to do so. An equivalent of that command line is synthesized and run internally in his version of git am when his git am sees --trailer art option using those am.{a,r,t}.trailer configuration variables. Hmm I wonder why do you dislike reusing -s with a parameter for this. To me, this looks like a superset of the default -s functionality: -s adds the default signature, -s x adds signature x ... Users don't really care that one is implemented as a trailer and another isn't. In fact, default -s can be implemented as a trailer too, right? Could you clarify please? Optional parameters for arguments make backwards-compatibility tricky. In this case, the command: git am -s mbox1 mbox2 means apply the patches from mbox1 and mbox2, and signoff the patches. Under your scheme, it now means apply from mbox2, and use the trailer mbox1. I think it would make more sense for -s to use a trailer called signoff if it is configured (and if not, have a baked-in signoff trailer config that behaves like -s does now). So -s (and --signoff) become sign off in the way I usually do for my project, not just add a signed-off-by line. If you want to something more fancy, you have to use --trailer= Just my two cents, as one who has not been closely following this discussion. Apologies if this idea was already presented and shot down. :) -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 RFC] git-am: support any number of signatures
Jeff King p...@peff.net writes: Optional parameters for arguments make backwards-compatibility tricky. In this case, the command: git am -s mbox1 mbox2 means apply the patches from mbox1 and mbox2, and signoff the patches. Under your scheme, it now means apply from mbox2, and use the trailer mbox1. Thanks for saving me from typing ;-) -- 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 08/16] t5304: use helper to report failure of test foo = bar
Jeff King p...@peff.net writes: On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote: Yeah, if we are going to reduce it down to the above implementation, intereseting things like test -f $frotz will become possible and cmp-str stops making sense. It really is about We run test and expect it to yield true. Report the failure a bit more prominently under the '-v' option to help us debug. We already have test_path_is_file to do the same thing just for -f. We could in theory switch all of those to this new, more generic wrapper. I don't know if it is worth doing a mass-conversion, but we could discourage test_path_is_file in new tests. We could also implement test_path_is_{dir,file} on top of this. Oh, I wasn't going in that direction when I mentioned -f; I just wanted to say that 'test $@' is clearly about 'test' (/bin/test or shell built-in) and less about 'compare string'. I do not think it is necessarily a good direction to go in to replace test-path-is-file with the test_cond thing; after all, type specific tests have chance to report breakage of expectation in type specifc ways, e.g. test_path_is_file () { test -f $1 return 0 echo 2 expected '$1' to be file if test -e $1 then echo 2 but it is missing else echo 2 but it is a non-file ls 2 -ld $1 fi return 1 } But that is also just in theory ;-). So among the ones you listed, test_verbose may be the least silly, I would think. Somehow test_verbose seems to me like checking the verbose option of the test suite. I prefer test_cond, but I do not feel too strongly, if you want to override me. Hmph, your 'test' in that name is a generic verb we check that..., which I think aligns better with the other test_foo functions. When I suggested 'test_verbose', 'test' in that name was specifically meant to refer to the 'test' command. Still test_cond feels somewhat funny, as we check that... always validates some condition, but I don't think of anything better X-. -- 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 08/16] t5304: use helper to report failure of test foo = bar
On 10/07/2014 11:53 PM, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Tue, Oct 07, 2014 at 01:35:15PM -0700, Junio C Hamano wrote: Yeah, if we are going to reduce it down to the above implementation, intereseting things like test -f $frotz will become possible and cmp-str stops making sense. It really is about We run test and expect it to yield true. Report the failure a bit more prominently under the '-v' option to help us debug. We already have test_path_is_file to do the same thing just for -f. We could in theory switch all of those to this new, more generic wrapper. I don't know if it is worth doing a mass-conversion, but we could discourage test_path_is_file in new tests. We could also implement test_path_is_{dir,file} on top of this. Oh, I wasn't going in that direction when I mentioned -f; I just wanted to say that 'test $@' is clearly about 'test' (/bin/test or shell built-in) and less about 'compare string'. I do not think it is necessarily a good direction to go in to replace test-path-is-file with the test_cond thing; after all, type specific tests have chance to report breakage of expectation in type specifc ways, e.g. test_path_is_file () { test -f $1 return 0 echo 2 expected '$1' to be file if test -e $1 then echo 2 but it is missing else echo 2 but it is a non-file ls 2 -ld $1 fi return 1 } But that is also just in theory ;-). So among the ones you listed, test_verbose may be the least silly, I would think. Somehow test_verbose seems to me like checking the verbose option of the test suite. I prefer test_cond, but I do not feel too strongly, if you want to override me. Hmph, your 'test' in that name is a generic verb we check that..., which I think aligns better with the other test_foo functions. When I suggested 'test_verbose', 'test' in that name was specifically meant to refer to the 'test' command. Still test_cond feels somewhat funny, as we check that... always validates some condition, but I don't think of anything better X-. I like verbose_test $foo = $bar because it puts the word test next to the condition, where the built-in command test would otherwise be. We could even define a command verbose () { $@ return 0 echo 2 command failed: $* return 1 } and use it like verbose test $foo = $bar Somehow I feel like I'm reinventing something that must already exist... Michael -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Oct 2014, #01; Tue, 7)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * mh/lockfile-stdio (2014-10-01) 3 commits - commit_packed_refs(): reimplement using fdopen_lock_file() - dump_marks(): reimplement using fdopen_lock_file() - fdopen_lock_file(): access a lockfile using stdio (this branch uses mh/lockfile.) Will merge to 'next'. * rs/daemon-fixes (2014-10-01) 3 commits (merged to 'next' on 2014-10-07 at 4171e10) + daemon: remove write-only variable maxfd + daemon: fix error message after bind() + daemon: handle gethostbyname() error git daemon (with NO_IPV6 build configuration) used to incorrectly use the hostname even when gethostbyname() reported that the given hostname is not found. Will merge to 'master'. * rs/sha1-array-test (2014-10-01) 2 commits - sha1-lookup: handle duplicates in sha1_pos() - sha1-array: add test-sha1-array and basic tests Will merge to 'next'. * da/completion-show-signature (2014-10-07) 1 commit (merged to 'next' on 2014-10-07 at 2467c19) + completion: add --show-signature for log and show Will merge to 'master'. * jk/prune-mtime (2014-10-04) 18 commits - write_sha1_file: freshen existing objects - pack-objects: match prune logic for discarding objects - pack-objects: refactor unpack-unreachable expiration check - prune: keep objects reachable from recent objects - sha1_file: add for_each iterators for loose and packed objects - count-objects: use for_each_loose_file_in_objdir - count-objects: do not use xsize_t when counting object size - prune: factor out loose-object directory traversal - t5304: use helper to report failure of test foo = bar - t5304: use test_path_is_* instead of test -f - reachable: clear pending array after walking it - clean up name allocation in prepare_revision_walk - object_array: add a clear function - object_array: factor out slopbuf-freeing logic - isxdigit: cast input to unsigned char - foreach_alt_odb: propagate return value from callback - Merge branch 'dt/cache-tree-repair' into jk/prune-mtime - Merge branch 'jc/reopen-lock-file' into jk/prune-mtime (this branch uses dt/cache-tree-repair.) Expecting a reroll. * jn/parse-config-slot (2014-10-07) 2 commits - color_parse: do not mention variable name in error message - pass config slots as pointers instead of offsets Expecting an Ack/Sign-off or update from Jonathan on the bottom one. * rs/mailsplit (2014-10-07) 1 commit - mailsplit: remove unnecessary unlink(2) call Will merge to 'next'. * rs/more-uses-of-skip-prefix (2014-10-07) 1 commit - use skip_prefix() to avoid more magic numbers Will merge to 'next'. * rs/plug-leak-in-bundle (2014-10-07) 1 commit - bundle: plug minor memory leak in is_tag_in_date_range() Will merge to 'next'. -- [Stalled] * rs/ref-transaction (2014-09-10) 19 commits . ref_transaction_commit: bail out on failure to remove a ref . lockfile: remove unable_to_lock_error . refs.c: do not permit err == NULL . for-each-ref.c: improve message before aborting on broken ref . refs.c: fix handling of badly named refs . branch -d: avoid repeated symref resolution . refs.c: change resolve_ref_unsafe reading argument to be a flags field . refs.c: make write_ref_sha1 static . fetch.c: change s_update_ref to use a ref transaction . refs.c: ref_transaction_commit: distinguish name conflicts from other errors . refs.c: pass a skip list to name_conflict_fn . refs.c: call lock_ref_sha1_basic directly from commit . refs.c: move the check for valid refname to lock_ref_sha1_basic . rename_ref: don't ask read_ref_full where the ref came from . refs.c: pass the ref log message to _create/delete/update instead of _commit . refs.c: add an err argument to delete_ref_loose . wrapper.c: add a new function unlink_or_msg . wrapper.c: remove/unlink_or_warn: simplify, treat ENOENT as success . mv test: recreate mod/ directory instead of relying on stale copy Expecting the final reroll. * tr/remerge-diff (2014-09-08) 8 commits - log --remerge-diff: show what the conflict resolution changed - name-hash: allow dir hashing even when !ignore_case - merge-recursive: allow storing conflict hunks in index - merge_diff_mode: fold all merge diff variants into an enum - combine-diff: do not pass revs-dense_combined_merges redundantly - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: internal flag to avoid touching the worktree - merge-recursive: remove dead conditional in update_stages() log -p output learns a new way to let users inspect a merge commit by showing the differences between the automerged result with
Re: [PATCH v4] MinGW(-W64) compilation
Am 30.09.2014 um 09:02 schrieb Marat Radchenko: This patch series fixes building on modern MinGW and MinGW-W64 (including x86_64!). *Compilation* tested on: - MSVC - msysGit environment (twice) Hi Marat, I wanted to verify that on msysgit but some patches fail to apply cleanly. Did you also had to tweak the patches? If yes, are these tweaked patches still available somewhere? Thomas -- 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] completion: ignore chpwd_functions when cding
Software, such as RVM (ruby version manager), may set chpwd functions that result in an endless loop when cding. chpwd functions should be ignored. Signed-off-by: Brandon Turner b...@brandonturner.net --- For an example of this bug, see: https://github.com/wayneeseguin/rvm/issues/3076 contrib/completion/git-completion.bash | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 06bf262..996de31 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -283,7 +283,8 @@ __git_ls_files_helper () { ( test -n ${CDPATH+set} unset CDPATH - cd $1 + (( ${#chpwd_functions} )) chpwd_functions=() + builtin cd $1 if [ $2 == --committable ]; then git diff-index --name-only --relative HEAD else -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] MinGW(-W64) compilation
On Wed, Oct 08, 2014 at 01:09:20AM +0200, Thomas Braun wrote: Am 30.09.2014 um 09:02 schrieb Marat Radchenko: This patch series fixes building on modern MinGW and MinGW-W64 (including x86_64!). *Compilation* tested on: - MSVC - msysGit environment (twice) Hi Marat, I wanted to verify that on msysgit but some patches fail to apply cleanly. Did you also had to tweak the patches? If yes, are these tweaked patches still available somewhere? msysgit != git-for-windows, as msysgit folks say. msysgit is a development environment for git-for-windows. I tested my patches by applying them to git.git/master and building inside msysgit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Junio C Hamano gits...@pobox.com writes: Jakub Narębski jna...@gmail.com writes: Junio C Hamano wrote: - [config] safe = section.variable will list variables that can be included with the config.safeInclude mechanism. Any variable that is not marked as config.safe that appears in the file included by the config.safeInclude mechanism will be ignored. Why user must know which variables are safe, why it cannot be left to Git to know which configuration variables can call external scripts? That's a fallback to let them take responsibility for variables we do not mark as safe; and having that fallback mechanism lets us keep the set of variables we by default mark as safe to the absolute minimum. Perhaps this would need a way to say this value is safe for this variable too. I don't have a real use-case, but one could say something like I'm OK with the file overriding core.editor, but the only values I accept are nano, vim and emacs. It doesn't seem to be a prerequisite to implement the safeInclude feature, but we should live room in the namespace for the day we want to add it. I don't have really good idea for it. The first I could think of was [config safe] core.editor = nano core.editor = vim core.editor = emacs but it's not accepted by the current parser, hence not backward compatible. Emacs has such mechanism for -*- ... -*- local variables in files for example. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html