Re: [PATCH] for-each-ref: Always check stat_tracking_info()'s return value.
On Fri, Jan 2, 2015 at 3:28 PM, Raphael Kubo da Costa raphael.kubo.da.co...@intel.com wrote: The code handling %(upstream:track) and %(upstream:trackshort) assumed it always had a valid branch that had been sanitized earlier in populate_value(), and thus did not check the return value of the call to stat_tracking_info(). While there is indeed some sanitization code that basically corresponds to stat_tracking_info() returning 0 (no base branch set), the function can also return -1 when the base branch did exist but has since then been deleted. In this case, num_ours and num_theirs had undefined values and a call to `git for-each-ref --format=%(upstream:track)` could print spurious values such as [behind -111794512] [ahead 38881640, behind 5103867] even for repositories with one single commit. We now properly verify stat_tracking_info()'s return value and do not print anything if it returns -1. This behavior also matches the documentation (has no effect if the ref does not have tracking information associated with it). Signed-off-by: Raphael Kubo da Costa raphael.kubo.da.co...@intel.com --- diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index bda354c..df9c3bd 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -335,6 +335,21 @@ test_expect_success 'Check that :track[short] cannot be used with other atoms' ' ' cat expected EOF + + +EOF + +test_expect_success 'Check that :track[short] works when upstream is gone' ' + git branch --track to_delete master + git branch --track parent_gone to_delete + git branch -D to_delete + git for-each-ref --format=%(upstream:track) refs/heads/parent_gone actual + git for-each-ref --format=%(upstream:trackshort) refs/heads/parent_gone actual + test_when_finished git branch -D parent_gone This still has the same problem. If the commands prior to test_when_finish() fail, the test_when_finished() will never be invoked. To fix, move test_when_finished() to just after the command which creates the parent_gone branch. + test_cmp expected actual +' + +cat expected EOF $(git rev-parse --short HEAD) EOF -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-add.txt: add `add.ginore-errors` configuration variable
On Mon, Jan 26, 2015 at 11:55 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: 'git add' supports not only `add.ignoreErrors`, but also `add.ignore-errors` configuration variable. See 6b3020a2 (add: introduce add.ignoreerrors synonym for add.ignore-errors, 2010-12-01) for why this patch is undesirable. Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- Documentation/git-add.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 1c74907..f68c2a2 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -155,8 +155,8 @@ for git add --no-all pathspec..., i.e. ignored removed files. If some files could not be added because of errors indexing them, do not abort the operation, but continue adding the others. The command shall still exit with non-zero status. - The configuration variable `add.ignoreErrors` can be set to - true to make this the default behaviour. + The configuration variable `add.ignoreErrors` or `add.ignore-errors` + can be set to true to make this the default behaviour. --ignore-missing:: This option can only be used together with --dry-run. By using -- 2.3.0.rc1.275.g028c360 -- To unsubscribe from this list: send the line 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-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Fri, Feb 6, 2015 at 3:43 PM, Kyle J. McKay mack...@gmail.com wrote: On Feb 6, 2015, at 12:05, Junio C Hamano wrote: Kyle J. McKay mack...@gmail.com writes: So I think it needs to stay #define'd to nothing to be safe in case anything later on ends up including stuff that uses it. Doesn't the fact that your test failed indicates that it is not jsut to be safe in case but is required for correctness? [...] I do not know what changes they make to openssl/*.h (which is included just after the above header is included, but I would imagine that is where the AVAILABLE_MAC_OS_X_VERSION_XXX_AND_LATER_BUT_DEPRECATED_IN_MAC_OS_VERSION_YYY macros are checked and annoying warnings that are being squelched by the previous change are given? Yes. Although Eric didn't specify exactly where when he suggested adding this: On Feb 6, 2015, at 02:00, Eric Sunshine wrote: #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif I took the suggestion to be after the openssl/*.h headers are included which would avoid the error of having DEPRECATED_ATTRIBUTE be #undef'd for them. But, even math.h can end up including AvailabilityMacros.h, so I think #undef'ing DEPRECATED_ATTRIBUTE after the openssl/*.h headers are included would be unsafe in general. While we might happen to get away with that today, if say compat/apple-common-crypto.h changes in the future (or for that matter any sequence of includes in other files or any headers in the Apple SDK) we could start seeing the error. TLDR; yeah, DEPRECATED_ATTRIBUTE needs to remain defined to nothing. I agree with this analysis. An alternative would be to revert b195aa00 (and not apply this patch), but then we risk having legitimate Git-related compilation warnings lost in the noise of the useless Apple deprecation warnings. Given the glacial pace at which Apple headers changes, and considering the rapid pace of change in Git, it still seems the lesser evil to suppress the useless warnings Apple thrust upon us when they deprecated OpenSSL in its entirety without providing replacements. It's unfortunate that the DEPRECATED_ATTRIBUTE #define will bleed beyond the OpenSSL #includes and potentially allow us to miss some future Apple deprecations, however, given the shortcomings of b195aa00, the proposed patch seems to be the best we can do. -- To unsubscribe from this list: send the line 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-compat-util: do not step on MAC_OS_X_VERSION_MIN_REQUIRED
On Fri, Feb 6, 2015 at 4:35 AM, Kyle J. McKay mack...@gmail.com wrote: MAC_OS_X_VERSION_MIN_REQUIRED may be defined by the builder to a specific version in order to produce compatible binaries for a particular system. Blindly defining it to MAC_OS_X_VERSION_10_6 is bad. Additionally MAC_OS_X_VERSION_10_6 will not be defined on older systems and should AvailabilityMacros.h be included on such as system an error will result. However, using the explicit value of 1060 (which is what MAC_OS_X_VERSION_10_6 is defined to) does not solve the problem. The changes that introduced stepping on MAC_OS_X_VERSION_MIN were made in b195aa00 (git-compat-util: suppress unavoidable Apple-specific deprecation warnings) to avoid deprecation warnings. Instead of blindly setting MAC_OS_X_VERSION_MIN to 1060 change the definition of DEPRECATED_ATTRIBUTE to empty to avoid the warnings. This preserves any MAC_OS_X_VERSION_MIN_REQUIRED setting while avoiding the warnings as intended by b195aa00. Signed-off-by: Kyle J. McKay mack...@gmail.com Tested on 10.10.2 (Yosemite) and 10.5.8 (Snow Leopard). Reviewed-by: Eric Sunshine sunsh...@sunshineco.com More below... --- git-compat-util.h | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index eb9b0ff3..0efd32c4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -212,12 +212,15 @@ extern char *gitbasename(char *); #endif #ifndef NO_OPENSSL +#ifdef __APPLE__ #define __AVAILABILITY_MACROS_USES_AVAILABILITY 0 -#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6 +#include AvailabilityMacros.h +#undef DEPRECATED_ATTRIBUTE +#define DEPRECATED_ATTRIBUTE +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY +#endif #include openssl/ssl.h #include openssl/err.h -#undef MAC_OS_X_VERSION_MIN_REQUIRED -#undef __AVAILABILITY_MACROS_USES_AVAILABILITY DEPRECATED_ATTRIBUTE is a fairly generic name. Do we want to be extra careful and #undef it here to avoid potential unintended interactions with other #includes (Apple or not)? #ifdef __APPLE__ #undef DEPRECATED_ATTRIBUTE #endif On the other hand, we've already mucked with it, so #undef may be superfluous. #ifdef NO_HMAC_CTX_CLEANUP #define HMAC_CTX_cleanup HMAC_cleanup #endif -- -- To unsubscribe from this list: send the line 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: read() MAX_IO_SIZE bytes, more than SSIZE_MAX?
On Sun, Feb 8, 2015 at 7:05 AM, Joachim Schmitz j...@schmitz-digital.de wrote: Junio C Hamano gitster at pobox.com writes: (1) if Makefile gives one, use it without second-guessing with SSIZE_MAX. (2) if SSIZE_MAX is defined, and if it is smaller than our internal default, use it. (3) all other cases, us our internal default. oops, yes, of course /* allow overwriting from e.g. Makefile */ #ifndef(MAX_IO_SIZE) # define MAX_IO_SIZE (8*1024*1024) /* for plattforms that have SSIZE and have it smaller */ # if defined(SSIZE_MAX) (SSIZE_MAX MAX_IO_SIZE) # undef MAX_IO_SIZE /* avoid warning */ # define MAX_IO_SIZE SSIZE_MAX # endif #endif A bit cleaner: #ifndef(MAX_IO_SIZE) # define MAX_IO_SIZE_DEFAULT (8*1024*1024) # if defined(SSIZE_MAX) (SSIZE_MAX MAX_IO_SIZE_DEFAULT) # define MAX_IO_SIZE SSIZE_MAX # else # define MAX_IO_SIZE MAX_IO_SIZE_DEFAULT # endif #endif -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/4] apply: do not touch a file beyond a symbolic link
On Tue, Feb 3, 2015 at 4:01 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: diff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh index 942c5cb..fbba8dd 100755 --- a/t/t4122-apply-symlink-inside.sh +++ b/t/t4122-apply-symlink-inside.sh @@ -112,6 +113,20 @@ test_expect_success SYMLINKS 'do not follow symbolic link (same input)' ' test_i18ngrep beyond a symbolic link error-ct test_must_fail git ls-files --error-unmatch arch/x86_64/dir test_must_fail git ls-files --error-unmatch arch/i386/dir Broken -chain. + + arch/i386/dir/file + git add arch/i386/dir/file At this point, the target of the patch application has: arch/i386/boot/Makefile arch/i386/dir/file arch/x86_64/boot/Makefile all of which are regular files. The index and the working tree match. -- To unsubscribe from this list: send the line 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: sporadic git failures on interactive rebase
On Wed, Jan 14, 2015 at 4:00 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jan 14, 2015 at 3:54 PM, Jeff King p...@peff.net wrote: On Wed, Jan 14, 2015 at 09:12:46AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: What happens if we rebase with it? $ git checkout 01319837 $ git rebase -i HEAD^ will yield a todo file with the 8-character unambiguous abbreviation. So I guess all is working as intended there. Perhaps you really were just very unlucky and an earlier step of the rebase created a conflicting sha1. That would mean 75c69766 (rebase -i: fix short SHA-1 collision, 2013-08-23) did not fix what it intended to fix, no? Is the symptom coming from pre-1.8.4.2 version of Git? Yeah, you're right. I didn't even remember that commit at all. On the off chance that the abbreviation code was different in that earlier version, I also checked rebasing 01319837 with an older version, but it does work fine. So yeah, the most plausible theory to me so far is unluckiness combined with pre-1.8.4.2. That should be easy to disprove if Henning tells us his git version. Henning mentioned it at the very top of his original problem report: (git version 2.2.0) For completeness: http://article.gmane.org/gmane.comp.version-control.git/262334 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/init-db.txt: minor style and synopsys fixes
On Wed, Jan 14, 2015 at 12:33 PM, Alexander Kuleshov kuleshovm...@gmail.com wrote: This patch constists of two minor changes: * line-wrap 'git init-db' synopsis * last possible argument '[directory]' was missed Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- diff --git a/Documentation/git-init-db.txt b/Documentation/git-init-db.txt index 648a6cd..1d94fe8 100644 --- a/Documentation/git-init-db.txt +++ b/Documentation/git-init-db.txt @@ -9,8 +9,9 @@ git-init-db - Creates an empty Git repository SYNOPSIS [verse] -'git init-db' [-q | --quiet] [--bare] [--template=template_directory] [--separate-git-dir git dir] [--shared[=permissions]] - +'git init-db' [-q | --quiet] [--bare] [--template=template_directory] + [--separate-git-dir git dir] + [--shared[=permissions]] [directory] I realize that you copied/pasted the text from git-init.txt, but this should really be [directory]. While you're at it, you could fix it in git-init.txt, as well, and adjust the formatting of... If you provide a 'directory', ... to say: If you provide directory, ... -- To unsubscribe from this list: send the line 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: sporadic git failures on interactive rebase
On Wed, Jan 14, 2015 at 3:54 PM, Jeff King p...@peff.net wrote: On Wed, Jan 14, 2015 at 09:12:46AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: What happens if we rebase with it? $ git checkout 01319837 $ git rebase -i HEAD^ will yield a todo file with the 8-character unambiguous abbreviation. So I guess all is working as intended there. Perhaps you really were just very unlucky and an earlier step of the rebase created a conflicting sha1. That would mean 75c69766 (rebase -i: fix short SHA-1 collision, 2013-08-23) did not fix what it intended to fix, no? Is the symptom coming from pre-1.8.4.2 version of Git? Yeah, you're right. I didn't even remember that commit at all. On the off chance that the abbreviation code was different in that earlier version, I also checked rebasing 01319837 with an older version, but it does work fine. So yeah, the most plausible theory to me so far is unluckiness combined with pre-1.8.4.2. That should be easy to disprove if Henning tells us his git version. Henning mentioned it at the very top of his original problem report: (git version 2.2.0) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sporadic git failures on interactive rebase
On Wed, Jan 14, 2015 at 4:02 PM, Jeff King p...@peff.net wrote: On Wed, Jan 14, 2015 at 04:00:37PM -0500, Eric Sunshine wrote: So yeah, the most plausible theory to me so far is unluckiness combined with pre-1.8.4.2. That should be easy to disprove if Henning tells us his git version. Henning mentioned it at the very top of his original problem report: (git version 2.2.0) Ah, reading comprehension. It pays off. I'm stumped, then. Perhaps some new code been added to git-rebase--interactive.sh since 75c69766 which neglects to invoke expand_todo_ids()? Or, possibly some older version of git is being invoked somehow during rebase despite his front end use of 2.2.0? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Documentation/init-db.txt: minor style and synopsys fixes
On Thu, Jan 15, 2015 at 5:31 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: Subject: Documentation/init-db.txt: minor style and synopsys fixes Subject is incorrect now that you're modifying git-init-db.txt and git-init.txt. This patch constists of two minor changes: * line-wrap 'git init-db' synopsis * last possible argument '[directory]' was missed Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- Documentation/git-init-db.txt | 5 +++-- Documentation/git-init.txt| 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Documentation/git-init-db.txt b/Documentation/git-init-db.txt index 648a6cd..2c77aaa 100644 --- a/Documentation/git-init-db.txt +++ b/Documentation/git-init-db.txt @@ -9,8 +9,9 @@ git-init-db - Creates an empty Git repository SYNOPSIS [verse] -'git init-db' [-q | --quiet] [--bare] [--template=template_directory] [--separate-git-dir git dir] [--shared[=permissions]] - +'git init-db' [-q | --quiet] [--bare] [--template=template_directory] +[--separate-git-dir git-dir] +[--shared[=permissions]] [directory] DESCRIPTION --- diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index 369f889..25412ac 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -10,8 +10,8 @@ SYNOPSIS [verse] 'git init' [-q | --quiet] [--bare] [--template=template_directory] - [--separate-git-dir git dir] - [--shared[=permissions]] [directory] + [--separate-git-dir git-dir] + [--shared[=permissions]] [directory] DESCRIPTION @@ -108,7 +108,7 @@ By default, the configuration flag `receive.denyNonFastForwards` is enabled in shared repositories, so that you cannot force a non fast-forwarding push into it. -If you provide a 'directory', the command is run inside it. If this directory +If you provide a directory, the command is run inside it. If this directory does not exist, it will be created. -- -- 2.3.0.rc0.314.g170a664.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Documentation/init-db.txt: minor style and synopsys fixes
On Thu, Jan 15, 2015 at 12:08 PM, Alexander Kuleshov kuleshovm...@gmail.com wrote: Yes, right, Etiquette on this list is to avoid top-posting [1]. how to do it better? Something like: Documentation, init-db/init:.? Or something else? Simplest would be to split it into two patches: one for git-init-db.txt and one for git-init.txt. Also... On Thu, Jan 15, 2015 at 5:31 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: Subject: Documentation/init-db.txt: minor style and synopsys fixes This patch constists of two minor changes: * line-wrap 'git init-db' synopsis * last possible argument '[directory]' was missed Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- +'git init-db' [-q | --quiet] [--bare] [--template=template_directory] +[--separate-git-dir git-dir] +[--shared[=permissions]] [directory] Taking Alex's review[2] into consideration: s/template_directory/template-directory/ [1]: https://lkml.org/lkml/2005/1/11/111 [2]: http://article.gmane.org/gmane.comp.version-control.git/262427 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 15/18] fsck: Document the new receive.fsck.* options.
On Mon, Jan 19, 2015 at 10:51 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- diff --git a/Documentation/config.txt b/Documentation/config.txt index ae6791d..7371a5f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2130,6 +2130,31 @@ receive.fsckObjects:: Defaults to false. If not set, the value of `transfer.fsckObjects` is used instead. +receive.fsck.*:: + When `receive.fsckObjects` is set to true, errors can be switched + to warnings and vice versa by configuring the `receive.fsck.*` + settings. These settings contain comma-separated lists of fsck + message IDs. For convenience, fsck prefixes the error/warning with + the message ID, e.g. missing-email: invalid author/committer line + - missing email means that setting `receive.fsck.ignore = + missing-email` will hide that issue. ++ +-- + error:: + a comma-separated list of fsck message IDs that should be + trigger fsck to error out. + warn:: + a comma-separated list of fsck message IDs that should be + displayed, but fsck should continue to error out. + ignore:: + a comma-separated list of fsck message IDs that should be + ignored completely. ++ +This feature is intended to support working with legacy repositories +which would not pass pushing when `receive.fsckObjects = true`, allowing +the host to accept repositories certain known issues but still catch s/certain/with / +other issues. + receive.unpackLimit:: If the number of objects received in a push is below this limit then the objects will be unpacked into loose object -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rebase -i: respect core.abbrev for real
On Mon, Jan 19, 2015 at 9:20 AM, Kirill A. Shutemov
kirill.shute...@linux.intel.com wrote:
I have tried to fix this before: see 568950388be2, but it doesn't
really work.
I don't know how it happend, but that commit makes interactive rebase to
respect core.abbrev only during --edit-todo, but not the initial todo
list edit.
For this time I've included a test-case to avoid this frustration again.
Signed-off-by: Kirill A. Shutemov kirill.shute...@linux.intel.com
---
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed29a9ec..a8ffc24ce46b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,11 @@ test_expect_success 'short SHA-1 collide' '
)
'
+test_expect_success 'respect core.abbrev' '
+ git config core.abbrev 12
+ set_cat_todo_editor
+ test_must_fail git rebase -i HEAD~4 todo-list 21
Broken -chain.
+ test 4 = $(grep -c pick [0-9a-f]\{12,\} todo-list)
+'
+
test_done
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/1] http: Add Accept-Language header if possible
On Sunday, January 18, 2015, Yi EungJun semtlen...@gmail.com wrote:
Add an Accept-Language header which indicates the user's preferred
languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
Examples:
LANGUAGE= -
LANGUAGE=ko:en - Accept-Language: ko, en;q=0.9, *;q=0.1
LANGUAGE=ko LANG=en_US.UTF-8 - Accept-Language: ko, *;q=0.1
LANGUAGE= LANG=en_US.UTF-8 - Accept-Language: en-US, *;q=0.1
This gives git servers a chance to display remote error messages in
the user's preferred language.
Limit the number of languages to 1,000 because q-value must not be
smaller than 0.001, and limit the length of Accept-Language header to
4,000 bytes for some HTTP servers which cannot accept such long header.
Signed-off-by: Yi EungJun eungjun...@navercorp.com
---
diff --git a/http.c b/http.c
index 040f362..349b033 100644
--- a/http.c
+++ b/http.c
@@ -986,6 +993,145 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
}
+static void write_accept_language(struct strbuf *buf)
+{
+ [...]
+ const int MAX_DECIMAL_PLACES = 3;
+ const int MAX_LANGUAGE_TAGS = 1000;
+ const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000;
+ struct strbuf *language_tags = NULL;
+ int num_langs;
Mental note: 'num_langs' is not initialized.
+ const char *s = get_preferred_languages();
+
+ /* Don't add Accept-Language header if no language is preferred. */
+ if (!s)
+ return;
+
+ /*
+* Split the colon-separated string of preferred languages into
+* language_tags array.
+*/
+ do {
+ /* increase language_tags array to add new language tag */
+ REALLOC_ARRAY(language_tags, num_langs + 1);
'num_langs' is used here uninitialized.
+ strbuf_init(language_tags[num_langs], 0);
+
+ /* collect language tag */
+ for (; *s (isalnum(*s) || *s == '_'); s++)
+ strbuf_addch(language_tags[num_langs], *s == '_' ?
'-' : *s);
+
+ /* skip .codeset, @modifier and any other unnecessary parts */
+ while (*s *s != ':')
+ s++;
+
+ if (language_tags[num_langs].len 0) {
Mental note: An empty () language tag is never allowed in language_tags[].
+ num_langs++;
This is a little bit ugly. At the top of the loop, you allocate space
in the array for a strbuf and initialize it. However, if the language
tag is empty (), then 'num_langs' is never incremented, so the next
time through the loop, strbuf_init() is invoked on the same block of
memory (assuming the realloc was a no-op since the allocation size did
not change), overwriting whatever was there and possibly leaking
memory. In this particular case, by examining the parser code closely,
we can see that nothing was added to the strbuf, so nothing is being
leaked the next time around, given the current implementation of
strbuf.
However, this is potentially fragile. A change to the implementation
of strbuf in the future (for instance, if strbuf_init() allocates
memory immediately) could result in a leak here. Moreover, this
no-leak situation only holds true if no text at all has been added to
the strbuf after strbuf_init(). If someone changes the parser in the
future to operate a bit differently so that some text is added and
then removed from the strbuf, even though the end result still has
length is 0, then it will start leaking.
One way to make this more robust would be to have a separate strbuf
for collecting the language tag. When you encounter a non-empty tag,
only then grow the array and initialize the new strbuf in the array.
Finally, use strbuf_swap() to swap the collected language tag into the
new array position. Something like this:
struct strbuf tag = STRBUF_INIT;
do {
for (; *s (isalnum(*s) || *s == '_'); s++)
strbuf_addch(tag, *s == '_' ? '-' : *s);
[...]
if (tag.len) {
num_langs++;
REALLOC_ARRAY(language_tags, num_langs);
strbuf_init(language_tags[num_langs], 0);
strbuf_swap(tag, language_tags[num_langs]);
if (num_langs = ...)
break;
}
while (...);
strbuf_release(tag);
+ if (num_langs = MAX_LANGUAGE_TAGS - 1) /* -1 for '*'
*/
+ break;
+ }
+ } while (*s++);
+
+ /* write Accept-Language header into buf */
+ if (num_langs = 1) {
+ int i;
+ int last_buf_len;
Mental note: 'last_buf_len' is not initialized.
+ int max_q;
+ int decimal_places;
+ char q_format[32];
+
+ /* add '*' */
+ REALLOC_ARRAY(language_tags, num_langs + 1);
+
Re: [PATCH] commit: reword --author error message
On Fri, Jan 16, 2015 at 1:35 PM, Junio C Hamano gits...@pobox.com wrote: Philip Oakley philipoak...@iee.org writes: die(_(--author '%s': not 'Name email', nor matches any existing author)); Sounds good. Thanks. To further bikeshed (particularly if nor is in the mix): neither 'Name email' nor a match for an existing author -- To unsubscribe from this list: send the line 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] contacts: introduce --since and --min-percent
On Fri, Jan 16, 2015 at 3:58 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
index dbe2abf..b06f2e1 100755
--- a/contrib/contacts/git-contacts
+++ b/contrib/contacts/git-contacts
@@ -8,12 +8,16 @@
use strict;
use warnings;
use IPC::Open2;
+use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
Most of the rest of the codebase uses qw(...) rather than qw/.../.
my $since = '5-years-ago';
my $min_percent = 10;
my $labels_rx = qr/Signed-off-by|Reviewed-by|Acked-by|Cc/i;
my %seen;
+my $rv = GetOptions('since=s' = \$since, 'min-percent=i' = \$min_percent);
+exit 1 if (!$rv);
This would make more sense if moved down to the point where the script
arguments are processed (just before the 'if (!@ARGV)' line, for
instance).
These new options should be documented in git-contacts.txt. Also, the
Limitations section of the documentation says that these values are
currently hard-coded, so it deserves an update as well.
sub format_contact {
my ($name, $email) = @_;
return $name $email;
--
2.2.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-push.txt: document the behavior of --repo
On Tue, Jan 27, 2015 at 7:35 AM, Michael J Gruber g...@drmicha.warpmail.net wrote: As per the code, the --repo repo option is equivalent to the repo argument to 'git push'. [It exists for historical reasons, back from the time when options had to come before arguments.] Say so. [But not that.] Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index ea97576..0ad31c4 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -219,22 +219,8 @@ origin +master` to force a push to the `master` branch). See the `refspec...` section above for details. --repo=repository:: - This option is only relevant if no repository argument is - passed in the invocation. In this case, 'git push' derives the - remote name from the current branch: If it tracks a remote - branch, then that remote repository is pushed to. Otherwise, - the name origin is used. For this latter case, this option - can be used to override the name origin. In other words, - the difference between these two commands -+ --- -git push public #1 -git push --repo=public #2 --- -+ -is that #1 always pushes to public whereas #2 pushes to public -only if the current branch does not track a remote branch. This is -useful if you write an alias or script around 'git push'. + This option is equivalent to the repository argument; the latter + wins if both are specified. To what does latter refer in this case? (I presume it means the standalone repository argument, though the text feels ambiguous.) Also, both the standalone argument and the right-hand-side of --repo= are spelled repository, so there may be potential for confusion when talking about repository (despite the subsequent argument). Perhaps qualifying it as _standalone_ repository argument might help. -u:: --set-upstream:: -- 2.3.0.rc1.222.gae238f2 -- To unsubscribe from this list: send the line 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-push.txt: document the behavior of --repo
On Wed, Jan 28, 2015 at 3:12 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: + This option is equivalent to the repository argument; the latter + wins if both are specified. To what does latter refer in this case? (I presume it means the standalone repository argument, though the text feels ambiguous.) Also, both the standalone argument and the right-hand-side of --repo= are spelled repository, so there may be potential for confusion when talking about repository (despite the subsequent argument). Perhaps qualifying it as _standalone_ repository argument might help. I didn't find that latter too hard to understand (I admit that my reading stuttered there, though). I do not think saying standalone repository argument there would help very much, because there is no mention of standalone around there. The earlier part of the sentence mentions option and argument, so the repository specified as an argument is used if both this option and an argument are given or something? Yes, that addresses the two (minor) ambiguities and sounds fine. Thinking about it afterward, I came up with this: This option is equivalent to the repository argument. If both are specified, the command-line argument takes precedence. -- To unsubscribe from this list: send the line 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 17/21] list-files: show directories as well as files
On Sun, Jan 25, 2015 at 7:37 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
The index does not store directories explicitly (except submodules) so
we have to figure them out from file list when output lis depth-limited.
The function show_as_directory() deliberately generates duplicate
directories and expects the previous patch to remove duplicates.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 1a1c9c8..29b5c2e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -179,6 +181,35 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+static int show_as_directory(const struct cache_entry *ce)
+{
+ struct strbuf sb = STRBUF_INIT;
+ const char *p;
+
+ strbuf_add(sb, ce-name, ce_namelen(ce));
+ while (sb.len (p = strrchr(sb.buf, '/')) != NULL) {
+ struct strbuf sb2 = STRBUF_INIT;
+ strbuf_setlen(sb, p - sb.buf);
+ if (!match_pathspec(pathspec, sb.buf, sb.len,
+ max_prefix_len, NULL, 1))
+ continue;
+ write_name(sb2, sb.buf);
+ if (want_color(use_color)) {
+ struct strbuf sb3 = STRBUF_INIT;
+ color_filename(sb3, ce-name, sb2.buf, S_IFDIR, 1);
+ strbuf_release(sb2);
+ sb2 = sb3;
Although more expensive, would it be a bit more idiomatic and obvious
to phrase this as
strbuf_swap(sb2, sb3);
strbuf_release(sb3);
or is it not worth it?
+ }
+ if (show_tag)
+ strbuf_insert(sb2, 0, tag_cached,
strlen(tag_cached));
+ strbuf_fputs(sb2, strbuf_detach(sb, NULL), NULL);
The detached strbuf content gets assigned to the 'util' field of the
'struct string_list output' item and is eventually leaked, however,
the program exits soon after. Okay.
+ strbuf_release(sb2);
+ return 1;
+ }
+ strbuf_release(sb);
+ return 0;
+}
+
static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce)
{
struct strbuf quoted = STRBUF_INIT;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 21/21] t3080: tests for git-list-files
On Sun, Jan 25, 2015 at 7:37 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/t/t3080-list-files.sh b/t/t3080-list-files.sh new file mode 100755 index 000..6313dd9 --- /dev/null +++ b/t/t3080-list-files.sh +test_expect_success 'no dups' ' + echo dirty file To leave a clean slate for subsequent tests, would it make sense to restore 'file' to a clean state via test_when_finished()? + git list-files -m file actual + echo file expected + test_cmp expected actual + git list-files -cm file actual + echo C file expected + test_cmp expected actual + git list-files -tcm file actual + test_cmp expected actual +' + +test_expect_success 'diff-cached' ' + echo dirty file + git add file Ditto here? + git list-files -M actual + echo file expected + test_cmp expected actual +' -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC
On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins reuben...@gmail.com wrote:
The checks will override and unset YesPlease settings for HAVE_CLOCK_GETTIME
and HAVE_CLOCK_MONOTONIC in config.mak.uname.
I find this hard to grok and would rewrite it as:
Set or clear Makefile variables HAVE_CLOCK_GETTIME and
HAVE_CLOCK_MONOTONIC based upon results of the checks (overriding
default values from config.mak.uname).
CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3 systems
being used in production.
Signed-off-by: Reuben Hawkins reuben...@gmail.com
With or without the minor rewrite above...
Reviewed-by: Eric Sunshine sunsh...@sunshineco.com
---
diff --git a/Makefile b/Makefile
index 7482a4d..57e33f2 100644
--- a/Makefile
+++ b/Makefile
@@ -339,6 +339,8 @@ all::
# return NULL when it receives a bogus time_t.
#
# Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt.
+#
+# Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt.
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1382,6 +1384,10 @@ ifdef HAVE_CLOCK_GETTIME
EXTLIBS += -lrt
endif
+ifdef HAVE_CLOCK_MONOTONIC
+ BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
diff --git a/config.mak.uname b/config.mak.uname
index a2f380f..926773e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
HAVE_CLOCK_GETTIME = YesPlease
+ HAVE_CLOCK_MONOTONIC = YesPlease
endif
ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
diff --git a/configure.ac b/configure.ac
index 210eb4e..c3293b9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -924,6 +924,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
[CHARSET_LIB=-lcharset])])
GIT_CONF_SUBST([CHARSET_LIB])
#
+# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
+GIT_CHECK_FUNC(clock_gettime,
+ [HAVE_CLOCK_GETTIME=YesPlease],
+ [HAVE_CLOCK_GETTIME=])
+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
+
+AC_DEFUN([CLOCK_MONOTONIC_SRC], [
+AC_LANG_PROGRAM([[
+#include time.h
+clockid_t id = CLOCK_MONOTONIC;
+]])])
+
+#
+# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
+AC_MSG_CHECKING([for CLOCK_MONOTONIC])
+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
+ [AC_MSG_RESULT([yes])
+ HAVE_CLOCK_MONOTONIC=YesPlease],
+ [AC_MSG_RESULT([no])
+ HAVE_CLOCK_MONOTONIC=])
+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
+#
# Define NO_SETITIMER if you don't have setitimer.
GIT_CHECK_FUNC(setitimer,
[NO_SETITIMER=],
diff --git a/trace.c b/trace.c
index 4778608..bfbd48f 100644
--- a/trace.c
+++ b/trace.c
@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)
return !!get_trace_fd(key);
}
-#ifdef HAVE_CLOCK_GETTIME
+#if defined(HAVE_CLOCK_GETTIME) defined(HAVE_CLOCK_MONOTONIC)
static inline uint64_t highres_nanos(void)
{
--
2.2.0.68.g8f72f0c.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] configure.ac: check 'tv_nsec' field in 'struct stat'
On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins reuben...@gmail.com wrote: Detect 'tv_nsec' field in 'struct stat' and set Makefile variable NO_NSEC appropriately. A side-effect of the above detection is that we also determine whether 'stat.st_mtimespec' is available, so, as a bonus, set the Makefile variable USE_ST_TIMESPEC, as well. Signed-off-by: Reuben Hawkins reuben...@gmail.com These patches may or may not deserve a Helped-by:. With or without the Helped-by: and the minor style fix-up below... Reviewed-by: Eric Sunshine sunsh...@sunshineco.com Thanks for the perseverance. For convenience of other reviewers: This is v3. v1 through v3 are threaded together here: http://thread.gmane.org/gmane.comp.version-control.git/261619 --- diff --git a/configure.ac b/configure.ac index 6af9647..210eb4e 100644 --- a/configure.ac +++ b/configure.ac @@ -754,6 +754,19 @@ AC_CHECK_TYPES([struct itimerval], [#include sys/time.h]) GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL]) # +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exists. +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor +# stat.st_mtimespec.tv_nsec exists. +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec]) +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec]) +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then Style: For consistency with all other 'if' conditionals in configure.ac, drop the space before the semicolon. + USE_ST_TIMESPEC=YesPlease + GIT_CONF_SUBST([USE_ST_TIMESPEC]) +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then Ditto. + NO_NSEC=YesPlease + GIT_CONF_SUBST([NO_NSEC]) +fi +# # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent. AC_CHECK_MEMBER(struct dirent.d_ino, [NO_D_INO_IN_DIRENT=], -- 2.2.0.68.g8f72f0c.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup
On Thu, Jan 8, 2015 at 3:00 PM, Reuben Hawkins reuben...@gmail.com wrote: OpenSSL version 0.9.6b and before defined the function HMAC_cleanup. Newer versions define HMAC_CTX_cleanup. Check for HMAC_CTX_cleanup and fall back to HMAC_cleanup when the newer function is missing. Signed-off-by: Reuben Hawkins reuben...@gmail.com Reviewed-by: Eric Sunshine sunsh...@sunshineco.com Note that this patch has a minor and simple-to-resolve conflict with b195aa00c1 (git-compat-util: suppress unavoidable Apple-specific deprecation warnings; 2014-12-16) which was just promoted to master. Junio may resolve it locally or ask you to re-send. --- diff --git a/Makefile b/Makefile index 57e33f2..2ce1f1f 100644 --- a/Makefile +++ b/Makefile @@ -341,6 +341,9 @@ all:: # Define HAVE_CLOCK_GETTIME if your platform has clock_gettime in librt. # # Define HAVE_CLOCK_MONOTONIC if your platform has CLOCK_MONOTONIC in librt. +# +# Define NO_HMAC_CTX_CLEANUP if your OpenSSL is version 0.9.6b or earlier to +# cleanup the HMAC context with the older HMAC_cleanup function. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1061,6 +1064,9 @@ ifndef NO_OPENSSL ifdef NEEDS_CRYPTO_WITH_SSL OPENSSL_LIBSSL += -lcrypto endif + ifdef NO_HMAC_CTX_CLEANUP + BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP + endif else BASIC_CFLAGS += -DNO_OPENSSL BLK_SHA1 = 1 diff --git a/configure.ac b/configure.ac index c3293b9..9c66c3e 100644 --- a/configure.ac +++ b/configure.ac @@ -924,6 +924,10 @@ AC_CHECK_LIB([iconv], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) # +# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing. +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup], + [], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])]) +# # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available. GIT_CHECK_FUNC(clock_gettime, [HAVE_CLOCK_GETTIME=YesPlease], diff --git a/git-compat-util.h b/git-compat-util.h index 400e921..2fdca2d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -213,6 +213,9 @@ extern char *gitbasename(char *); #ifndef NO_OPENSSL #include openssl/ssl.h #include openssl/err.h +#ifdef NO_HMAC_CTX_CLEANUP +#define HMAC_CTX_cleanup HMAC_cleanup +#endif #endif /* On most systems netdb.h would have given us this, but -- 2.2.0.68.g8f72f0c.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv12 06/10] receive-pack.c: negotiate atomic push support
On Wed, Jan 7, 2015 at 10:23 PM, Stefan Beller sbel...@google.com wrote:
From: Ronnie Sahlberg sahlb...@google.com
This adds the atomic protocol option to allow
receive-pack to inform the client that it has
atomic push capability.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
v9:
We can configure the remote if it wants to advertise
atomicity!
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 362d33f..4c069c5 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -159,6 +160,11 @@ static int receive_pack_config(const char *var, const
char *value, void *cb)
return 0;
}
+ if (strcmp(var, receive.advertiseatomic) == 0) {
+ advertise_atomic_push = git_config_bool(var, value);
+ return 0;
+ }
Is this needed only to support the tests you add in t5543, or do you
intend it to be useful to end users?
If it is for end users, then it deserves to be documented. Is also
deserves proper mention and justification in the commit message.
If it's only intended to assist automated testing, then perhaps
control it via an environment variable rather than a configuration
option. (See, for instance, GIT_TEST_SPLIT_INDEX or GIT_USE_LOOKUP as
precedent.)
+
return git_default_config(var, value, cb);
}
@@ -174,6 +180,8 @@ static void show_ref(const char *path, const unsigned
char *sha1)
strbuf_addstr(cap,
report-status delete-refs side-band-64k
quiet);
+ if (advertise_atomic_push)
+ strbuf_addstr(cap, atomic);
if (prefer_ofs_delta)
strbuf_addstr(cap, ofs-delta);
if (push_cert_nonce)
@@ -1263,6 +1271,9 @@ static struct command *read_head_info(struct sha1_array
*shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, quiet))
quiet = 1;
+ if (advertise_atomic_push
+parse_feature_request(feature_list, atomic))
+ use_atomic = 1;
}
if (!strcmp(line, push-cert)) {
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line 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: [PATCHv12 10/10] t5543-atomic-push.sh: add basic tests for atomic pushes
On Wed, Jan 7, 2015 at 10:23 PM, Stefan Beller sbel...@google.com wrote:
This adds tests for the atomic push option.
The first four tests check if the atomic option works in
good conditions and the last three patches check if the atomic
s/patches/tests/
option prevents any change to be pushed if just one ref cannot
be updated.
The commit message talks about 7 tests, but I count 8.
Signed-off-by: Stefan Beller sbel...@google.com
---
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
new file mode 100755
index 000..3480b33
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,194 @@
+test_expect_success 'atomic push is not advertised if configured' '
+ mk_repo_pair
+ (
+ cd upstream
Broken -chain.
+ git config receive.advertiseatomic 0
+ )
+ (
+ cd workbench
+ test_commit one
+ git push --mirror up
+ test_commit two
+ test_must_fail git push --atomic up master
+ )
+ test_refs master HEAD@{1}
+'
+
+test_done
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line 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 2.2.2 annotate crash (strbuf.c:32)
On Mon, Feb 09, 2015 at 11:33:39AM +0100, Dilyan Palauzov wrote:
the point is that with exactly the same configuration on one
computer there is crash and on another one things work just fine.
I found out that line builtin/blame.c:1675 makes the problems:
if (len) {
printf(blame.c:1676, subject: %s, len: %i\n, subject, len);
-- strbuf_add(ret-summary, subject, len); --
} else
strbuf_addf(ret-summary, (%s), sha1_to_hex(commit-object.sha1));
commenting it out and compiling does not lead to crashing git
anymore. You can find below the output of printf.
git clone git://git.cyrusimap.org/cyrus-imapd
git annotate timsieved/parser.c
*** Error in `git': double free or corruption (!prev):
0x022e4b40 ***
There is a bit of suspicious code in builtin/blame.c where it is
destroying the commit_info without ever initializing it, and this
happens many times when blaming 'timsieved/parser.c'. Does the
following patch fix the problem for you?
--- 8 ---
diff --git a/builtin/blame.c b/builtin/blame.c
index 303e217..a3cc972 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2085,7 +2085,6 @@ static void find_alignment(struct scoreboard *sb, int
*option)
for (e = sb-ent; e; e = e-next) {
struct origin *suspect = e-suspect;
- struct commit_info ci;
int num;
if (compute_auto_abbrev)
@@ -2096,6 +2095,7 @@ static void find_alignment(struct scoreboard *sb, int
*option)
if (longest_file num)
longest_file = num;
if (!(suspect-commit-object.flags METAINFO_SHOWN)) {
+ struct commit_info ci;
suspect-commit-object.flags |= METAINFO_SHOWN;
get_commit_info(suspect-commit, ci, 1);
if (*option OUTPUT_SHOW_EMAIL)
@@ -2104,6 +2104,7 @@ static void find_alignment(struct scoreboard *sb, int
*option)
num = utf8_strwidth(ci.author.buf);
if (longest_author num)
longest_author = num;
+ commit_info_destroy(ci);
}
num = e-s_lno + e-num_lines;
if (longest_src_lines num)
@@ -2113,8 +2114,6 @@ static void find_alignment(struct scoreboard *sb, int
*option)
longest_dst_lines = num;
if (largest_score ent_score(sb, e))
largest_score = ent_score(sb, e);
-
- commit_info_destroy(ci);
}
max_orig_digits = decimal_width(longest_src_lines);
max_digits = decimal_width(longest_dst_lines);
--
2.3.0.rc2.191.g303d43c
--- 8 ---
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] reflog_expire(): never update a reference to null_sha1
On Mon, Feb 9, 2015 at 4:12 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
Currently, if --updateref is specified and the very last reflog entry
is expired or deleted, the reference's value is set to 0{40}. This is
an invalid state of the repository, and breaks, for example, git
fsck and git for-each-ref.
The only place we use --updateref in our own code is when dropping
stash entries. In that code, the very next step is to check if the
reflog has been made empty, and if so, delete the refs/stash
reference entirely. Thus that code path ultimately leaves the
repository in a valid state.
But we don't want the repository in an invalid state even temporarily,
and we don't want leave an invalid state if other callers of git
s/want/want to/
reflog expire|delete --updateref don't think to do the extra cleanup
step.
So, if git reflog expire|delete leaves no more entries in the
reflog, just leave the reference un-updated.
Signed-off-by: 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] builtin/blame: destroy initialized commit_info only
Since ea02ffa3 (mailmap: simplify map_user() interface, 2013-01-05),
find_alignment() has been invoking commit_info_destroy() on an
uninitialized auto 'struct commit_info' (when METAINFO_SHOWN is not
set). commit_info_destroy() calls strbuf_release() for each of
'commit_info' strbuf member, which randomly invokes free() on whatever
random stack value happens to be reside in strbuf.buf, thus leading to
periodic crashes.
Reported-by: Dilyan Palauzov dilyan.palau...@aegee.org
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
No test accompanying this fix since I don't know how to formulate one.
Discussion: http://thread.gmane.org/gmane.comp.version-control.git/263534
builtin/blame.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 303e217..a3cc972 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2085,7 +2085,6 @@ static void find_alignment(struct scoreboard *sb, int
*option)
for (e = sb-ent; e; e = e-next) {
struct origin *suspect = e-suspect;
- struct commit_info ci;
int num;
if (compute_auto_abbrev)
@@ -2096,6 +2095,7 @@ static void find_alignment(struct scoreboard *sb, int
*option)
if (longest_file num)
longest_file = num;
if (!(suspect-commit-object.flags METAINFO_SHOWN)) {
+ struct commit_info ci;
suspect-commit-object.flags |= METAINFO_SHOWN;
get_commit_info(suspect-commit, ci, 1);
if (*option OUTPUT_SHOW_EMAIL)
@@ -2104,6 +2104,7 @@ static void find_alignment(struct scoreboard *sb, int
*option)
num = utf8_strwidth(ci.author.buf);
if (longest_author num)
longest_author = num;
+ commit_info_destroy(ci);
}
num = e-s_lno + e-num_lines;
if (longest_src_lines num)
@@ -2113,8 +2114,6 @@ static void find_alignment(struct scoreboard *sb, int
*option)
longest_dst_lines = num;
if (largest_score ent_score(sb, e))
largest_score = ent_score(sb, e);
-
- commit_info_destroy(ci);
}
max_orig_digits = decimal_width(longest_src_lines);
max_digits = decimal_width(longest_dst_lines);
--
2.3.0.rc2.191.g303d43c
--
To unsubscribe from this list: send the line 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 2.2.2 annotate crash (strbuf.c:32)
On Sun, Feb 8, 2015 at 8:28 PM, Jeff King p...@peff.net wrote: On Sun, Feb 08, 2015 at 10:33:40PM +0100, Dilyan Palauzov wrote: I use git 2.2.2 and on my system git annotate crashed with the following log. I couldn't reproduce it with a few simple examples. Is it possible for you to show us the repository and command that caused this? I also was unable to reproduce on either Mac OS X or Linux with git 2.2.2. Clues from the traceback suggest the cyrus-imapd project and annotation of timsieved/parser.c. I tried: git clone git://git.cyrusimap.org/cyrus-imapd/ cd cyrus-imapd git --no-pager annotate timsieved/parser.c -- To unsubscribe from this list: send the line 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] builtin/blame: destroy initialized commit_info only
On Mon, Feb 9, 2015 at 4:28 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Since ea02ffa3 (mailmap: simplify map_user() interface, 2013-01-05), find_alignment() has been invoking commit_info_destroy() on an uninitialized auto 'struct commit_info' (when METAINFO_SHOWN is not set). commit_info_destroy() calls strbuf_release() for each of s/each of/each/ ...despite several proof-reads (sigh). 'commit_info' strbuf member, which randomly invokes free() on whatever random stack value happens to be reside in strbuf.buf, thus leading to periodic crashes. Reported-by: Dilyan Palauzov dilyan.palau...@aegee.org Signed-off-by: Eric Sunshine sunsh...@sunshineco.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/blame: destroy initialized commit_info only
On Mon, Feb 9, 2015 at 4:33 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Mon, Feb 9, 2015 at 4:28 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Since ea02ffa3 (mailmap: simplify map_user() interface, 2013-01-05), find_alignment() has been invoking commit_info_destroy() on an uninitialized auto 'struct commit_info' (when METAINFO_SHOWN is not set). commit_info_destroy() calls strbuf_release() for each of s/each of/each/ ...despite several proof-reads (sigh). 'commit_info' strbuf member, which randomly invokes free() on whatever random stack value happens to be reside in strbuf.buf, thus leading to s/to be/to/ (sigh again) periodic crashes. Reported-by: Dilyan Palauzov dilyan.palau...@aegee.org Signed-off-by: Eric Sunshine sunsh...@sunshineco.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat
On Wed, Jan 7, 2015 at 5:19 PM, Reuben Hawkins reuben...@gmail.com wrote: On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: This check will automatically set the correct NO_NSEC setting. This commit message neglects to mention the important point that you're also now setting USE_ST_TIMESPEC when detected. You might revise the message like this: Detect 'tv_nsec' field in 'struct stat' and set Makefile variable NO_NSEC appropriately. A side-effect of the above detection is that we also determine whether 'stat.st_mtimespec' is available, so, as a bonus, set the Makefile variable USE_ST_TIMESPEC, as well. I see you're single quoted 'tv_nsec' and 'struct stat'. Should I also use single quotes in the first line of the commit msg like this... configure.ac: check 'tv_nsec' field in 'struct stat' Quoting them was just my personal taste, however, consistency of formatting between subject and the body of the message would be nice. Use whatever seems correct to you. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC
On Wed, Jan 7, 2015 at 5:31 PM, Reuben Hawkins reuben...@gmail.com wrote: On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: +GIT_CHECK_FUNC(clock_gettime, +[HAVE_CLOCK_GETTIME=YesPlease], +[HAVE_CLOCK_GETTIME=]) +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME]) You could simplify the above four lines to this one-liner: GIT_CHECK_FUNC(clock_gettime, GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease])) +AC_MSG_CHECKING([for CLOCK_MONOTONIC]) +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC], + [AC_MSG_RESULT([yes]) + HAVE_CLOCK_MONOTONIC=YesPlease], + [AC_MSG_RESULT([no]) + HAVE_CLOCK_MONOTONIC=]) +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC]) Ditto regarding simplification: AC_MSG_CHECKING([for CLOCK_MONOTONIC]) AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC], [AC_MSG_RESULT([yes]) GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])], [AC_MSG_RESULT([no])]) I *think* there's an issue with this simplification as used right here. In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by setting it equal to nothing HAVE_CLOCK_MONOTONIC= So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC = YesPlease' will be overridden. So this one needs to stay as is. Yes, you're right. That means that the HAVE_CLOCK_GETTIME simplification also suffers the same shortcoming. So, neither simplification is appropriate in this instance. Sorry for the noise. -- To unsubscribe from this list: send the line 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 v3 1/2] t4255: test am submodule with diff.submodule
On Wed, Jan 7, 2015 at 2:31 PM, Doug Kelly dougk@gmail.com wrote:
git am will break when using diff.submodule=log; add some test cases
to illustrate this breakage as simply as possible. There are
currently two ways this can fail:
* With errors (unrecognized input), if only change
* Silently (no submodule change), if other files change
Test for both conditions and ensure without diff.submodule this works.
Signed-off-by: Doug Kelly dougk@gmail.com
Thanks-to: Eric Sunshine sunsh...@sunshineco.com
Thanks-to: Junio C Hamano gits...@pobox.com
On this project, it's customary to say Helped-by: rather than
Thanks-to:. Also, place your sign-off last.
---
Updated with Eric Sunshine's comments and changes to reduce complexity,
and also changed to include Junio's suggestions for using test_config,
test_unconfig, and test_might_fail (since we don't know if a previous
am failed or not -- we always want to clean up first).
Looking much better. Thanks. A couple minor comments below...
diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 8bde7db..523accf 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -18,4 +18,76 @@ am_3way () {
KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
test_submodule_switch am_3way
+test_expect_success 'setup diff.submodule' '
+ test_commit one
+ INITIAL=$(git rev-parse HEAD)
+
+ git init submodule
+ (
+ cd submodule
+ test_commit two
+ git rev-parse HEAD ../initial-submodule
+ )
+ git submodule add ./submodule
+ git commit -m first
+
+ (
+ cd submodule
+ test_commit three
+ git rev-parse HEAD ../first-submodule
+ )
+ git add submodule
+ test_tick
You can drop this test_tick (as I did in my squash[1]).
+ git commit -m second
+ SECOND=$(git rev-parse HEAD)
+
+ (
+ cd submodule
+ git mv two.t four.t
+ test_tick
And this one (which I overlooked in [1]).
The reason I suggest dropping the test_tick invocations is that they
do not impact these tests at all, yet their presence misleads the
reader into thinking that they are somehow significant.
+ git commit -m second submodule
+ git rev-parse HEAD ../second-submodule
+ )
+ test_commit four
+ git add submodule
+ git commit --amend --no-edit
+ THIRD=$(git rev-parse HEAD)
+ git submodule update --init
+'
[1]: http://article.gmane.org/gmane.comp.version-control.git/261852
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat
On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: This check will automatically set the correct NO_NSEC setting. This commit message neglects to mention the important point that you're also now setting USE_ST_TIMESPEC when detected. You might revise the message like this: Detect 'tv_nsec' field in 'struct stat' and set Makefile variable NO_NSEC appropriately. A side-effect of the above detection is that we also determine whether 'stat.st_mtimespec' is available, so, as a bonus, set the Makefile variable USE_ST_TIMESPEC, as well. Also, your sign-off is missing (as mentioned in my previous review[1]). [1]: http://article.gmane.org/gmane.comp.version-control.git/261626 --- configure.ac | 12 1 file changed, 12 insertions(+) diff --git a/configure.ac b/configure.ac index 6af9647..dcc4bf0 100644 --- a/configure.ac +++ b/configure.ac @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval], [#include sys/time.h]) GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL]) # +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist It would be slightly more accurate to drop the .tv_nsec bit from this comment. Also: s/exist/exists./ +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist Perhaps wrap this long comment over two lines. Also: s/exist/exist./ +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec]) +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec]) +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then + USE_ST_TIMESPEC=YesPlease + GIT_CONF_SUBST([USE_ST_TIMESPEC]) +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then + NO_NSEC=YesPlease + GIT_CONF_SUBST([NO_NSEC]) +fi +# # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent. AC_CHECK_MEMBER(struct dirent.d_ino, [NO_D_INO_IN_DIRENT=], -- 2.2.0.68.g8f72f0c.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] configure.ac: check for HMAC_CTX_cleanup
On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: OpenSSL version 0.9.6b and before defined the function HMAC_cleanup. Newer versions define HMAC_CTX_cleanup. Check for HMAC_CTX_cleanup and fall back to HMAC_cleanup when the newer function is missing. Missing sign-off. Overall, these patches are nicely improved from the previous round. A few more comments below... --- Makefile | 3 +++ configure.ac | 7 +++ git-compat-util.h | 3 +++ 3 files changed, 13 insertions(+) diff --git a/Makefile b/Makefile index af551a0..d3c2b58 100644 --- a/Makefile +++ b/Makefile @@ -1059,6 +1059,9 @@ ifndef NO_OPENSSL ifdef NEEDS_CRYPTO_WITH_SSL OPENSSL_LIBSSL += -lcrypto endif + ifdef NO_HMAC_CTX_CLEANUP + BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP + endif You need to document this new Makefile variable (NO_HMAC_CTX_CLEANUP) at the top of Makefile (as mentioned in my previous review[1]). [1]: http://article.gmane.org/gmane.comp.version-control.git/261631 else BASIC_CFLAGS += -DNO_OPENSSL BLK_SHA1 = 1 diff --git a/configure.ac b/configure.ac index 424dec5..c282663 100644 --- a/configure.ac +++ b/configure.ac @@ -923,6 +923,13 @@ AC_CHECK_LIB([iconv], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) # +# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing. +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup], + [NO_HMAC_CTX_CLEANUP=], + [NO_HMAC_CTX_CLEANUP=YesPlease], + []) +GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP]) It is customary to drop empty trailing arguments in m4. Also, you can simplify this entire check to: AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup], [], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])]) # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available. GIT_CHECK_FUNC(clock_gettime, [HAVE_CLOCK_GETTIME=YesPlease], -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC
On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3 systems being used in production. This change makes compiling git less tedious on older platforms without CLOCK_MONOTONIC. The second sentence is implied by the very presence of this patch, thus adds no value. I, personally, would drop it. Also, your sign-off is missing (as mentioned in my previous review[1]). --- diff --git a/Makefile b/Makefile index 7482a4d..af551a0 100644 --- a/Makefile +++ b/Makefile @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME EXTLIBS += -lrt endif +ifdef HAVE_CLOCK_MONOTONIC + BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC +endif You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC) at the top of Makefile (as mentioned in my previous review[1]), so that people who build without running 'configure' will know that they may need to tweak it. ifeq ($(TCLTK_PATH),) NO_TCLTK = NoThanks endif diff --git a/configure.ac b/configure.ac index dcc4bf0..424dec5 100644 --- a/configure.ac +++ b/configure.ac @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset], [CHARSET_LIB=-lcharset])]) GIT_CONF_SUBST([CHARSET_LIB]) # +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available. +GIT_CHECK_FUNC(clock_gettime, +[HAVE_CLOCK_GETTIME=YesPlease], +[HAVE_CLOCK_GETTIME=]) +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME]) You could simplify the above four lines to this one-liner: GIT_CHECK_FUNC(clock_gettime, GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease])) + +AC_DEFUN([CLOCK_MONOTONIC_SRC], [ +AC_LANG_PROGRAM([[ +#include time.h +clockid_t id = CLOCK_MONOTONIC; +]], [])]) No need to pass empty trailing arguments in m4. It's customary to drop them altogether (since they are implied). +# +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available. +AC_MSG_CHECKING([for CLOCK_MONOTONIC]) +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC], + [AC_MSG_RESULT([yes]) + HAVE_CLOCK_MONOTONIC=YesPlease], + [AC_MSG_RESULT([no]) + HAVE_CLOCK_MONOTONIC=]) +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC]) Ditto regarding simplification: AC_MSG_CHECKING([for CLOCK_MONOTONIC]) AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC], [AC_MSG_RESULT([yes]) GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])], [AC_MSG_RESULT([no])]) +# # Define NO_SETITIMER if you don't have setitimer. GIT_CHECK_FUNC(setitimer, [NO_SETITIMER=], [1]: http://article.gmane.org/gmane.comp.version-control.git/261630 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat
On Wed, Jan 7, 2015 at 4:33 PM, Reuben Hawkins reuben...@gmail.com wrote: On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote: +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist It would be slightly more accurate to drop the .tv_nsec bit from this comment. The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec. If I drop tv_nsec from the comment should I also drop it in the check? No. My observation was just about the comment. I thought it was better to be very explicit because the code using the check is using that .tv_nsec field...I figured the check may as well do exactly what the code is doing... Indeed, the check and code should agree. However, from the perspective of the person reading comment, the .tv_nsec is just an implementation detail of the check itself. The final outcome (the setting of USE_ST_TIMESPEC) is independent of how that check was made: it matters only that 'stat.st_mtimespec' was detected _somehow_. Anyhow, it's just a minor observation, hence my qualification of it as _slightly_ more accurate. If you feel strongly that it should remain as is, then 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] --disassociate alias for --dissociate clone option
On Fri, Feb 20, 2015 at 2:10 PM, Matt Whiteley mattwhite...@gmail.com wrote: I find the new --dissociate option for clone very helpful but I have a hard time with the spelling. It seems reasonable to have an alias since one exists for --recursive. You may be undermining your own argument for inclusion of a new --dissociate option by citing the aliased recursive option. git-clone's --recursive-submodules was added (see ccdd3da6) for disambiguation long after --recursive; and not the other way around with --recursive being more convenient or easier to remember or spell than --recursive-submodules. The implication of ccdd3da6 is that --recursive-submodules is favored over --recursive (and perhaps the latter may some day be deprecated). -- To unsubscribe from this list: send the line 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: [RFH] GSoC 2015 application
On Fri, Feb 20, 2015 at 5:06 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Jeff King p...@peff.net writes: I think these might be getting a little larger than micro. The ~/.git-credential-cache may be a bit harder, but the case of ~/.git-credentials should follow the same pattern as files for which this is already done. So, doing it by mimicking existing code shouldn't be too hard. But maybe that's me being optimistic ;-). As a person who did a significant number of GSoC 2014 micro-project reviews (as well as many actual GSoC reviews), I probably ought to weigh in and say that this may be too optimistic. In fact, most of the GSoC 2015 micro-project suggestions[1] seem far too large and involved. For 2014, most of the GSoC micro-projects were extremely simple, of the form replace starts_with() with skip_prefix(), yet most applicants still required three or four (or more) attempts to get it right (and some never did), even with extremely detailed hand-holding reviews at each step. (And, such reviews are quite time-consuming. I was allocating six to eight hours each day to those reviews, yet I couldn't keep up with all the submissions.) Although quite simple, many of the 2014 micro-projects[2] (particularly from Michael Haggerty) had a bit of a twist (or trick question) thrown in, requiring a tad more thought and effort than mere mechanical search-and-replace. That was useful because it helped identify potentially acceptable candidates more easily, however, that added twist was probably a good upper limit on difficulty for micro-projects. Another important aspect of 2014's micro-projects was that they could be accomplished with only very localized knowledge: that is, a student could read the logic of the one function being touched and learn enough to be successful. The micro-projects did not require global knowledge of Git internals or hours of research. The attitude in 2014 was that it was important for students to get a taste of what it is like working on the Git project and what would be expected of them as submitters, and for GSoC administrators and mentors to get a feel for the applicants by how they interacted with reviewers. By going through the review process on a project with high engineering standards, it also was hope that students would learn and benefit from the experience even if not selected. Extremely simple micro-projects (possibly with a twist) in the style of 2014's were more than sufficient to satisfy these goals, and were more than enough to consume significant reviewer time. The suggested 2015's micro-projects seem far in excess. [1]: http://git.github.io/SoC-2015-Microprojects.html [2]: http://git.github.io/SoC-2014-Microprojects.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote: Subject: Add tests for git-log --merges=show|hide|only Drop capitalization, mention area you're touching, followed by colon, followed by short summary: t4202-log: add --merges= tests More below. Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..ab6f371 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +Merge branch 'side' +EOF Current custom is to place this sort of setup code in its own test rather than having it at top-level. So, the above and below 'cat's should all go in a test named setup --merges= (or something better). Prefixing EOF with '-' allows you to indent the here-doc content and EOF so that it reads nicely inside a test. Also prefix EOF with '\' to indicate that you do not expect interpolation within the here-doc. Torsten already mentioned to drop the space following ''. Finally, within the setup test, chain them all together with . So: cat only_merges -\EOF ... EOF More below. +cat only_commits EOF +seventh +octopus-b +octopus-a +reach +tangle-a +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF + +cat both_commits_merges EOF +Merge tag 'reach' +Merge tags 'octopus-a' and 'octopus-b' +seventh +octopus-b +octopus-a +reach +Merge branch 'tangle' +Merge branch 'side' (early part) into tangle +Merge branch 'master' (early part) into tangle +tangle-a +Merge branch 'side' +side-2 +side-1 +Second +sixth +fifth +fourth +third +second +initial +EOF t4202 already does this sort of fragile hard-coding of expected output, so this isn't a particularly strong objection, but it would be more robust if you could create these expected lists dynamically by issuing some git command other than the one you're testing. (That could also be considered fodder for a follow-on patch if you don't consider such a change worthwhile at this time.) + +test_expect_success 'log with config log.merges=show' ' + git config log.merges show +git log --pretty=tformat:%s actual Torsten already mentioned botched indentation. Use (8-character wide) tab for indentation everywhere. + test_cmp both_commits_merges actual +git config --unset log.merges If the test_cmp fails (because the expected and actual output differed), then the git-config --unset will never be invoked. To ensure that the config setting is undone when the test finishes (whether successful or not), use test_config(). The other option is to ensure that each test sets or clears log.merges as appropriate at the start of the test, and then not bother about resetting it. The shortcoming of this approach, however, is that any tests added following these new tests won't necessarily know that log.merges is set or that they may need to clear it. Consequently, test_config() at the start of each of these tests is probably the better approach. More below. +' + +test_expect_success 'log with config log.merges=only' ' + git config log.merges only +git log --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide' ' + git config log.merges hide +git log --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=show with git log --no-merges' ' + git config log.merges show +git log --no-merges --pretty=tformat:%s actual + test_cmp only_commits actual +git config --unset log.merges +' + +test_expect_success 'log with config log.merges=hide with git log ---merges' ' + git config log.merges hide +git log --merges --pretty=tformat:%s actual + test_cmp only_merges actual +git config --unset log.merges +' + +test_expect_success 'log --merges=show' ' For these tests which expect log.merges to be unset, it would be more robust, and the intent clearer, if you actually ensured that it was unset. test_unconfig() at the start of the test would be the appropriate way to unset the config. It would also make the tests more self-contained, in case they are ever re-ordered. More below. +git log --merges=show --pretty=tformat:%s actual + test_cmp both_commits_merges actual +' + +test_expect_success 'log --merges=only' ' +git log --merges=only --pretty=tformat:%s actual + test_cmp only_merges actual +' + +test_expect_success 'log --merges=hide' ' +git log --merges=hide --pretty=tformat:%s actual + test_cmp only_commits actual
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan pyoka...@gmail.com wrote: t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: --- The previous version can be found at [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 * Merge related, but previously separate, tests together in order to make the test suite easier to understand. * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set it, and unset it immediately before and after helper_test store is called in order to make it localized to only the command that it should affect. * Add test, previously missing, to check that only the home credentials file is written to if both the xdg and home files exist. * Correct mislabelling of home-user/home-pass to the proper xdg-user/xdg-pass. * Use rm -f instead of test_might_fail rm. This round looks much better. Thanks. Most of the comments below are just nit-picky, with one or two genuine (minor) issues. Thanks Eric for the code review. diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index f61b40c..5b0a666 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,115 @@ test_description='credential-store tests' helper_test store +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' ' These test descriptions are quite long, often mirroring in prose what the test body already says more concisely. I generally try to keep descriptions short while still being descriptive enough to give a general idea about what is being tested. I've rewritten a few test descriptions (below) to be very short, in fact probably too terse; but perhaps better descriptions would lie somewhere between the two extremes. First example, for this test: !xdg: .git-credentials !xdg Key: Space-separated items. Items before : are the pre-conditions, and items after are the post-state. ! file does not exist; output goes here. + test -s $HOME/.git-credentials + test_path_is_missing $HOME/.config/git/credentials +' + +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' It's customary to call this setup rather than create. Terse version: setup: -.git-redentials +xdg Key: - file removed; + file created. + rm -f $HOME/.git-credentials + mkdir -p $HOME/.config/git + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'when xdg credentials file exists, only write to xdg file; do not create ~/.git-credentials' ' Terse version: !.git-credentials xdg: !.git-credentials xdg + test -s $HOME/.config/git/credentials + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'set up custom XDG_CONFIG_HOME credential file at ~/xdg/git/credentials' ' s/set up/setup/ Terse: setup custom-xdg + mkdir -p $HOME/xdg/git + rm -f $HOME/.config/git/credentials + $HOME/xdg/git/credentials It would be easier to read this if you placed the two lines together which refer to the custom xdg file. Also, for completeness and to be self-contained, don't you want to remove ~/.git-credentials? rm -f $HOME/.git-credentials rm -f $HOME/.config/git/credentials mkdir -p $HOME/xdg/git $HOME/xdg/git/credentials +' + +XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store +unset XDG_CONFIG_HOME It's hard to spot the helper_test store at the end of line. I'd place it on a line by itself so that it is easy to see that it is wrapped by the setting and unsetting of the environment variable. +test_expect_success 'if custom XDG_CONFIG_HOME credentials file ~/xdg/git/credentials exists, it will only be written to; ~/.config/git/credentials and ~/.git-credentials will not be created' ' Terse: !.git-credentials !xdg custom-xdg: !.git-credentials !xdg custom-xdg + test_when_finished rm -f $HOME/xdg/git/credentials + test -s $HOME/xdg/git/credentials + test_path_is_missing $HOME/.config/git/credentials Matthieu already pointed out the broken -chain. + test_path_is_missing $HOME/.git-credentials +' + +test_expect_success 'get: return credentials from home file if matches are found in both home and xdg files' ' Terse: .git-credentials xdg: .git-credentials Key: taken from here. + mkdir -p $HOME/.config/git + echo https://xdg-user:xdg-p...@example.com; $HOME/.config/git/credentials + echo https://home-user:home-p...@example.com; $HOME/.git-credentials + check fill store -\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com + username=home-user + password=home-pass + -- + EOF +' + +test_expect_success 'get: return credentials from xdg file
Re: [PATCH 3/5] Update documentations for git-log to include the new --merges option and also its corresponding config option.
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote:
Subject: Update documentations for git-log to include the new
--merges option and also its corresponding config option.
The Subject: should be a short summary of the change; ideally less
than 70 or 72 characters. The rest of the commit message can flesh out
the description if necessary. The subject on this patch is far too
long. Also, drop capitalization, mention the area you're touching, and
drop the final period. You might say instead:
Documentation: mention git-log --merges= and log.merges
In this case, it's probably not necessary to write anything else in
the commit message. The summary says enough, despite its conciseness.
More below.
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 1f7bc67..506125a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -190,6 +190,9 @@ log.showroot::
`git log -p` output would be shown without a diff attached.
The default is `true`.
+log.merges::
+Default for `--merges` option. Defaults to `show`.
To disambiguate this from the --merges option, you should probably
spell it out explicitly as `--merges=` rather than just `--merges`.
mailmap.*::
See linkgit:git-shortlog[1].
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 4ed8587..398e657 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -99,6 +99,12 @@ if it is part of the log message.
--merges::
Print only merge commits. This is exactly the same as
`--min-parents=2`.
+--merges=show|hide|only::
+ If show is specified, merge commits will be shown in conjunction with
+ other commits. If hide is specified, it will work like `--no-merges`.
+ If only is specified, it will work like `--merges`. The default option
+ is show.
By The default option is show, do you mean when --merges= is not
specified? Perhaps it would be better to phrase it as:
Default is `show` if `--merges=` is not specified.
+
--no-merges::
Do not print commits with more than one parent. This is
exactly the same as `--max-parents=1`.
It might make more sense to rewrite the --merges and --no-merges
options in terms of the new --merges= option. For instance, something
like this:
--merges::
Shorthand for `--merges=only`.
--merges={show|hide|only}::
`show`: show merge and non-merge commits
`hide`: omit merge commits; same as `--max-parents=1`
`only`: show only merge commits; same as `--min-parents=2`
Default is `show` if `--merges=` is not specified.
--no-merges::
Shorthand for `--merges=hide`.
Alternately, --merges and --merges= could be combined like this
(taking inspiration from --decorate[=]):
--merges[=show|hide|only]::
But then you need additional explanation about what --merges means
without the '=' and following keyword.
--
2.3.3.263.g095251d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] strbuf: introduce strbuf_read_cmd helper
On Sun, Mar 22, 2015 at 6:07 AM, Jeff King p...@peff.net wrote: Something as simple as reading the stdout from a command turns out to be rather hard to do right. Doing: if (!run_command(cmd)) strbuf_read(buf, cmd.out, 0); can result in deadlock if the child process produces a large amount of output. [...] Let's introduce a strbuf helper that can make this a bit simpler for callers to do right. Signed-off-by: Jeff King p...@peff.net --- This is really at the intersection of the strbuf and run-command APIs, so you could argue for it being part of either It is logically quite like the strbuf_read_file() function, so I put it there. It does feel like a layering violation. If moved to the run-command API, it could given one of the following names or something better: run_command_capture() capture_command() command_capture() run_command_with_output() capture_output() diff --git a/strbuf.h b/strbuf.h index 1883494..93a50da 100644 --- a/strbuf.h +++ b/strbuf.h @@ -1,6 +1,8 @@ #ifndef STRBUF_H #define STRBUF_H +struct child_process; + /** * strbuf's are meant to be used with all the usual C string and memory * APIs. Given that the length of the buffer is known, it's often better to @@ -373,6 +375,14 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint); /** + * Execute the given command, capturing its stdout in the given strbuf. + * Returns -1 if starting the command fails or reading fails, and otherwise + * returns the exit code of the command. The output collected in the + * buffer is kept even if the command returns a non-zero exit. + */ +int strbuf_read_cmd(struct strbuf *sb, struct child_process *cmd, size_t hint); + +/** * Read a line from a FILE *, overwriting the existing contents * of the strbuf. The second argument specifies the line * terminator character, typically `'\n'`. -- 2.3.3.618.ga041503 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] Add a new option 'merges' to revision.c
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote:
Subject: Add a new option 'merges' to revision.c
For the subject, mention the area you're touching, followed by a
colon, followed by a short but meaningful summary of the change. Drop
capitalization.
revision: add --merges={show|only|hide} option
revision.c: add a new option 'merges' with
No need to mention revision.c here since the patch itself shows this clearly.
Considering that there is already a --merges option, it is somewhat
misleading and not terribly clear to say only that you're adding a new
option 'merges'. Better would be to spell out --merges= explicitly.
possible values of 'only', 'show' and 'hide'.
The option is used when showing the list of commits.
The value 'only' lists only merges. The value 'show'
is the default behavior which shows the commits as well
as merges and the value 'hide' makes it just list commit
items.
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
Considering that Junio actually wrote this patch[1], it would be more
correct and considerate to attribute it to him. You can do so by
ensuring that there is a From: header at the very top of the commit
message before mailing out the patch:
From: Junio C Hamano gits...@pobox.com
The customary way to indicate that you wrote the commit message and
made a few small adjustments to Junio's patch would be with a
bracketed comment (starting with your initials) just before your
sign-off. Something like this:
[kk: wrote commit message; added a couple missing
{min|max}_parents assignments]
[1]: http://article.gmane.org/gmane.comp.version-control.git/265597
---
diff --git a/revision.c b/revision.c
index 66520c6..edb7bed 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs,
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
}
+int parse_merges_opt(struct rev_info *revs, const char *param)
+{
+ if (!strcmp(param, show)) {
+ revs-min_parents = 0;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, only)) {
+ revs-min_parents = 2;
+ revs-max_parents = -1;
+ } else if (!strcmp(param, hide)) {
+ revs-min_parents = 0;
+ revs-max_parents = 1;
+ } else {
+ return -1;
+ }
+ return 0;
+}
+
static int handle_revision_opt(struct rev_info *revs, int argc, const char
**argv,
int *unkc, const char **unkv)
{
@@ -1800,9 +1817,14 @@ static int handle_revision_opt(struct rev_info *revs,
int argc, const char **arg
revs-show_all = 1;
} else if (!strcmp(arg, --remove-empty)) {
revs-remove_empty_trees = 1;
+ } else if (starts_with(arg, --merges=)) {
+ if (parse_merges_opt(revs, arg + 9))
+ die(unknown option: %s, arg);
} else if (!strcmp(arg, --merges)) {
+ revs-max_parents = -1;
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
+ revs-min_parents = 0;
revs-max_parents = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
diff --git a/revision.h b/revision.h
index 0ea8b4e..f9df58c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv,
struct rev_info *revs,
extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t
*ctx,
const struct option *options,
const char * const usagestr[]);
+extern int parse_merges_opt(struct rev_info *, const char *);
#define REVARG_CANNOT_BE_FILENAME 01
#define REVARG_COMMITTISH 02
extern int handle_revision_arg(const char *arg, struct rev_info *revs,
--
2.3.3.263.g095251d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Update Bash completion script to include git log --merges option
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote:
Subject: Update Bash completion script to include git log --merges option
Nice to see a patch covering this oft-overlooked corner of the project.
It's misleading to say that you're updating it to include the --merges
option, which it already knows about. Spell it out explicitly as
--merges=. Also, drop capitalization, mention area you're touching,
followed by colon, followed by short summary:
completion: teach about git-log --merges= and log.merges
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 731c289..b63bb95 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1406,7 +1406,7 @@ _git_ls_tree ()
__git_log_common_options=
--not --all
--branches --tags --remotes
- --first-parent --merges --no-merges
+ --first-parent --merges --merges= --no-merges
--max-count=
--max-age= --since= --after=
--min-age= --until= --before=
@@ -1451,6 +1451,10 @@ _git_log ()
__gitcomp long short ${cur##--decorate=}
return
;;
+--merges=*)
+__gitcomp show hide only ${cur##--merges=}
+return
+;;
--*)
__gitcomp
$__git_log_common_options
@@ -1861,6 +1865,10 @@ _git_config ()
__gitcomp $__git_log_date_formats
return
;;
+ log.merges)
+ __gitcomp show hide only
+ return
+ ;;
sendemail.aliasesfiletype)
__gitcomp mutt mailrc pine elm gnus
return
@@ -2150,6 +2158,7 @@ _git_config ()
interactive.singlekey
log.date
log.decorate
+ log.merges
log.showroot
mailmap.file
man.
--
2.3.3.263.g095251d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] Add tests for git-log --merges=show|hide|only
On Sun, Mar 22, 2015 at 3:57 PM, Torsten Bögershausen tbo...@web.de wrote: On 22.03.15 19:28, Koosha Khajehmoogahi wrote: Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de --- diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 5f2b290..ab6f371 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -428,6 +428,147 @@ cat expect \EOF * initial EOF +cat only_merges EOF - please no space after the - please add the at the end of the line: cat only_merges EOF This code is outside any test, so has no impact (and would be slightly confusing). Better would be to move this setup code into a setup --merges= test, in which case the -chain would be appropriate. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] Make git-log honor log.merges option
On Sun, Mar 22, 2015 at 2:28 PM, Koosha Khajehmoogahi koo...@posteo.de wrote:
Subject: Make git-log honor log.merges option
Drop capitalization, mention area you're touching, followed by colon,
followed by short summary:
log: honor log.merges option
Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
As discussed in my review of patch 1/5, since Junio actually wrote
this patch[1], it would be more correct and considerate to attribute
it to him by ensuring that a From: header is at the very top of the
commit message before mailing out the patch:
From: Junio C Hamano gits...@pobox.com
(If Junio had also signed-off on his patch, you would also include his
sign-off just above yours. In this case, he didn't sign off[1], so
your sign-off is all that is needed; and Junio will add his own if he
picks up this series.)
[1]: http://article.gmane.org/gmane.comp.version-control.git/265597
---
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..c7a7aad 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -36,6 +36,7 @@ static int decoration_given;
static int use_mailmap_config;
static const char *fmt_patch_subject_prefix = PATCH;
static const char *fmt_pretty;
+static const char *log_merges;
static const char * const builtin_log_usage[] = {
N_(git log [options] [revision range] [[--] path...]),
@@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char
*value, void *cb)
decoration_style = 0; /* maybe warn? */
return 0;
}
+ if (!strcmp(var, log.merges)) {
+ return git_config_string(log_merges, var, value);
+ }
if (!strcmp(var, log.showroot)) {
default_show_root = git_config_bool(var, value);
return 0;
@@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char
*prefix)
init_revisions(rev, prefix);
rev.always_show_header = 1;
+ if (log_merges parse_merges_opt(rev, log_merges))
+ die(unknown config value for log.merges: %s, log_merges);
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
opt.revarg_opt = REVARG_COMMITTISH;
--
2.3.3.263.g095251d.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2/GSoC/RFC] fetch: git fetch --deepen
On Sun, Mar 22, 2015 at 11:24 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote: This patch is just for discusstion. An option --deepen is added to 'git fetch'. When it comes to '--deepen', git should fetch N more commits ahead the local shallow commit, where N is indicated by '--depth=N'. [1] Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com --- diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index d78f320..3b960c8 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -708,4 +708,16 @@ test_expect_success 'fetching a one-level ref works' ' ) ' +test_expect_success 'fetching deepen' ' + git clone . deepen --depth=1 ( Quoting Junio[1]: ...make sure tests serve as good examples, tests should stick to 'options first and then arguments'... + cd deepen + git fetch .. foo --depth=1 See [1]. + git show foo + test_must_fail git show foo~ + git fetch .. foo --depth=1 --deepen See [1]. + git show foo~ + test_must_fail git show foo~2 Mentioned previously[2]: Broken -chain throughout this test. + ) +' + test_done [1]: http://article.gmane.org/gmane.comp.version-control.git/265248 [2]: http://article.gmane.org/gmane.comp.version-control.git/265419 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Documentation/git-add.txt: describe --exclude option
On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index f2eb907..4bc156a 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -9,7 +9,7 @@ SYNOPSIS [verse] 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p] - [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] + [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--exclude=pattern] [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--] [pathspec...] @@ -164,6 +164,10 @@ for git add --no-all pathspec..., i.e. ignored removed files. be ignored, no matter if they are already present in the work tree or not. +--exclude=pattern:: + Do not add files to the index in addition which are found in + the .gitignore. This is difficult to understand. Perhaps something like: Also ignore files matching pattern, a .gitignore-like pattern. This option can be specified multiple times, can't it? The documentation should say so. + \--:: This option can be used to separate command-line options from the list of files, (useful when filenames might be mistaken -- 2.3.3.472.g20ceeac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC][GSoC] make git diff --no-index $directory $file DWIM better.
In addition to the points raised by Matthieu and Thomas...
On Sun, Mar 15, 2015 at 11:35 AM, Yurii Shevtsov unge...@gmail.com wrote:
make git diff --no-index $directory $file DWIM better.
Specify the area affected by the change, followed by a colon, followed
by the change summary. Drop the period at the end. So, something like:
diff: improve '--no-index directory file' DWIMing
Changes 'git diff --no-index $directory $file' behaviour.
Now it is transformed to 'git diff --no-index $directory/file $file'
instead of throwing an error.
Write in imperative mood, so Change rather than Changes. By
itself, the first sentence isn't saying much; it would read better if
you combined the two sentences into one.
Signed-off-by: Yurii Shevtsov ungetch at gmail.com
---
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..4e71b36 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -97,8 +97,25 @@ static int queue_diff(struct diff_options *o,
if (get_mode(name1, mode1) || get_mode(name2, mode2))
return -1;
- if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
- return error(file/directory conflict: %s, %s, name1, name2);
+ if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+ struct strbuf dirnfile;
Is this name supposed to stand for dir'n'file, a shorthand for
dir-and-file? Perhaps a simpler more idiomatic name such as path
would suffice. Also, you can initialize the strbuf here at point of
declaration:
struct strbuf path = STRBUF_INIT;
+ const char *dir, *file;
+ char *filename;
+ int ret = 0;
+
+ dir = S_ISDIR(mode1) ? name1 : name2;
+ file = (dir == name1) ? name2 : name1;
+ strbuf_init(dirnfile, strlen(name1) + strlen(name2) + 2);
Unless this is a performance critical loop where you want to avoid
multiple re-allocations, it's customary to pass 0 for the second
argument. Doing so makes the code a bit easier to read and understand,
and avoids questions like this one: Why are you adding 2 to the
requested length? I presume that you're taking the / and NUL into
account, however, strbuf takes NUL into account on its own as part of
its contract, so you probably wanted to add only 1.
+ strbuf_addstr(dirnfile, dir);
+ if (dirnfile.buf[dirnfile.len - 1] != '/')
I don't know how others feel about it, but it makes me a bit
uncomfortable to see potential access of array item -1. I suppose, in
this case, neither name will be zero-length, however, I'd still feel
more comfortable seeing that documented formally, either via assert():
assert(dirnfile.len 0);
if (dirnfile.buf[dirnfile.len - 1] != '/')
or:
if (dirnfile.len 0 dirnfile.buf[dirnfile.len - 1] != '/')
+ strbuf_addch(dirnfile, '/');
+ filename = strrchr(file, '/');
+ strbuf_addstr(dirnfile, filename ? (filename + 1) : file);
+ ret = queue_diff(o, dirnfile.buf, file);
+ strbuf_release(dirnfile);
+
+ return ret;
+ }
if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
struct strbuf buffer1 = STRBUF_INIT;
--
I hope I understood task correct. I think this patch requires writing
additional tests, so that's what I'm going to do now.
--
--
To unsubscribe from this list: send the line 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 3/3] t3700-add: added test for --exclude option
On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov kuleshovm...@gmail.com wrote: t3700-add: added test for --exclude option Write in imperative mood: s/added/add/ Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- diff --git a/t/t3700-add.sh b/t/t3700-add.sh index f7ff1f5..c52a5d0 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -81,6 +81,13 @@ test_expect_success '.gitignore is honored' ' ! (git ls-files | grep \\.ig) ' +test_expect_success 'Test that git add --exclude works' ' + touch foo + touch bar Expanding slightly on what Torsten said: Use 'touch' when the timestamp of the file is significant to the test; otherwise, just use foo. + git add --exclude=bar . + ! (git ls-files | grep bar) For completeness, don't you also want to test that foo _does_ appear in the git-ls-files output? +' + test_expect_success 'error out when attempting to add ignored ones without -f' ' test_must_fail git add a.?? ! (git ls-files | grep \\.ig) -- 2.3.3.472.g20ceeac -- To unsubscribe from this list: send the line 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 1/3] add: add new --exclude option to git add
In addition to points raised by Philip and Torsten...
On Sun, Mar 15, 2015 at 9:49 AM, Alexander Kuleshov
kuleshovm...@gmail.com wrote:
add: add new --exclude option to git add
No need for redundant to git add, since you already have the add: prefix.
This patch introduces new --exclude option for the git add
command.
This line merely repeats the Subject: line, thus can be dropped.
We already have core.excludesfile configuration variable which indicates
a path to file which contains patterns to exclude. This patch provides
ability to pass --exclude option to the git add command to exclude paths
from command line in addition to which found in the ignore files.
The commit message is missing the important justification for why this
new option is desirable, and why only git-add needs it.
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
diff --git a/builtin/add.c b/builtin/add.c
index 3390933..5c602a6 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option *opt,
const char *arg, int unse
return 0;
}
+struct string_list exclude_list = STRING_LIST_INIT_NODUP;
Shouldn't this be declared static?
+struct exclude_list *el;
Why is this declared globally when it's only needed locally by cmd_add()?
+static int exclude_cb(const struct option *opt, const char *arg, int unset)
+{
+ struct string_list *exclude_list = opt-value;
+ string_list_append(exclude_list, arg);
+ return 0;
+}
+
static struct option builtin_add_options[] = {
OPT__DRY_RUN(show_only, N_(dry run)),
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL('u', update, take_worktree_changes, N_(update tracked
files)),
OPT_BOOL('N', intent-to-add, intent_to_add, N_(record only the
fact that the path will be added later)),
OPT_BOOL('A', all, addremove_explicit, N_(add changes from all
tracked and untracked files)),
+ { OPTION_CALLBACK, 0, exclude, exclude_list, N_(pattern),
+ N_(do no add files matching pattern to index),
+ 0, exclude_cb },
Can this just be OPT_STRING_LIST instead of OPTION_CALLBACK?
{ OPTION_CALLBACK, 0, ignore-removal, addremove_explicit,
NULL /* takes no arguments */,
N_(ignore paths removed in the working tree (same as --no-all)),
@@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int flags)
int cmd_add(int argc, const char **argv, const char *prefix)
{
+ int i;
int exit_status = 0;
struct pathspec pathspec;
struct dir_struct dir;
@@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
if (!ignored_too) {
dir.flags |= DIR_COLLECT_IGNORED;
setup_standard_excludes(dir);
+
+ el = add_exclude_list(dir, EXC_CMDL, --exclude
option);
+ for (i = 0; i exclude_list.nr; i++)
+ add_exclude(exclude_list.items[i].string, ,
0, el, -(i+1));
+
}
memset(empty_pathspec, 0, sizeof(empty_pathspec));
@@ -446,5 +465,6 @@ finish:
die(_(Unable to write new index file));
}
+ string_list_clear(exclude_list, 0);
return exit_status;
}
--
2.3.3.472.g20ceeac
--
To unsubscribe from this list: send the line 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 3/3 v2] t3700-add: added test for --exclude option
On Sun, Mar 15, 2015 at 3:07 PM, Alexander Kuleshov kuleshovm...@gmail.com wrote: t3700-add: added test for --exclude option s/added/add/ Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- diff --git a/t/t3700-add.sh b/t/t3700-add.sh index f7ff1f5..6f71c67 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -332,4 +332,22 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out test_i18ncmp expect.err actual.err ' +test_expect_success 'Test that git add --exclude works' ' Rather than inventing a new title style, try to match the style of the existing tests titles in this file. + foo + bar + git add --exclude=bar . + ! (git ls-files | grep bar) Broken -chain. + (git ls-files | grep foo) Unnecessary additional git-ls-files invocation. You could just save the output to a file and then process that file. +' + +test_expect_success 'Test multiply --exclude' ' s/multiply/multiple/ Ditto: Match existing title style. + foo + bar + b a z + git add --exclude=bar --exclude=b a z . + (git ls-files | grep foo) + ! (git ls-files | grep b a z) + ! (git ls-files | grep baz) Broken -chain throughout. Ditto: Unnecessary git-ls-files invocations. +' + test_done -- 2.3.3.472.g20ceeac.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v2] add: introduce new --exclude option
On Sun, Mar 15, 2015 at 3:06 PM, Alexander Kuleshov
kuleshovm...@gmail.com wrote:
We already have core.excludesfile configuration variable which indicates
a path to file which contains patterns to exclude. This patch provides
ability to pass --exclude option to the git add command to exclude paths
from command line in addition to which specified in the ignore files.
This option can be useful in a case when we have a directory with some *.ext
files which have changes and we want to commit all files besides one for now.
It can be too annoying to touch .gitignore for this.
Won't this lead to unintuitive behavior? The 'excludes' mechanism does
not unconditionally ignore; instead, it ignores _untracked_ files.
Consider file foo which is already tracked. Make a temporary change
to foo which you don't intend to commit. Since foo is tracked, the
command 'git add . --exclude=foo' will still add foo to the index,
despite the use of --exclude. Most people would probably find such
behavior surprising and undesirable.
The negative pathspec mentioned by Junio[1], on the other hand, does
not suffer this shortcoming.
More below.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/265493/focus=265518
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Torsten Bögershausen tbo...@web.de
Helped-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com
---
diff --git a/builtin/add.c b/builtin/add.c
index 3390933..e165fbc 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -244,6 +244,8 @@ static int ignore_removal_cb(const struct option *opt,
const char *arg, int unse
return 0;
}
+static struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+
static struct option builtin_add_options[] = {
OPT__DRY_RUN(show_only, N_(dry run)),
OPT__VERBOSE(verbose, N_(be verbose)),
@@ -255,6 +257,8 @@ static struct option builtin_add_options[] = {
OPT_BOOL('u', update, take_worktree_changes, N_(update tracked
files)),
OPT_BOOL('N', intent-to-add, intent_to_add, N_(record only the
fact that the path will be added later)),
OPT_BOOL('A', all, addremove_explicit, N_(add changes from all
tracked and untracked files)),
+ OPT_STRING_LIST(0, exclude, exclude_list, N_(pattern),
+ N_(do not add files matching pattern to index)),
{ OPTION_CALLBACK, 0, ignore-removal, addremove_explicit,
NULL /* takes no arguments */,
N_(ignore paths removed in the working tree (same as --no-all)),
@@ -305,6 +309,7 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
int add_new_files;
int require_pathspec;
char *seen = NULL;
+ struct exclude_list *el;
This variable is only used within the 'if (!ignored_too)' block below,
so its declaration should be moved there.
git_config(add_config, NULL);
@@ -379,8 +384,14 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
/* Set up the default git porcelain excludes */
memset(dir, 0, sizeof(dir));
if (!ignored_too) {
+ int i;
dir.flags |= DIR_COLLECT_IGNORED;
setup_standard_excludes(dir);
+
+ el = add_exclude_list(dir, EXC_CMDL, --exclude
option);
+ for (i = 0; i exclude_list.nr; i++)
+ add_exclude(exclude_list.items[i].string, ,
0, el, -(i+1));
+
}
memset(empty_pathspec, 0, sizeof(empty_pathspec));
@@ -446,5 +457,6 @@ finish:
die(_(Unable to write new index file));
}
+ string_list_clear(exclude_list, 0);
return exit_status;
}
--
2.3.3.472.g20ceeac.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC][GSoC] make git diff --no-index $directory $file DWIM better.
[re-adding cc:git]
On Sun, Mar 15, 2015 at 2:45 PM, Yurii Shevtsov unge...@gmail.com wrote:
On Sun, Mar 15, 2015 at 11:35 AM, Yurii Shevtsov unge...@gmail.com wrote:
make git diff --no-index $directory $file DWIM better.
Specify the area affected by the change, followed by a colon, followed
by the change summary. Drop the period at the end. So, something like:
diff: improve '--no-index directory file' DWIMing
Changes 'git diff --no-index $directory $file' behaviour.
Now it is transformed to 'git diff --no-index $directory/file $file'
instead of throwing an error.
Write in imperative mood, so Change rather than Changes. By
itself, the first sentence isn't saying much; it would read better if
you combined the two sentences into one.
Got it! My commit message requires improvements
---
- if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
- return error(file/directory conflict: %s, %s, name1,
name2);
+ if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+ struct strbuf dirnfile;
Is this name supposed to stand for dir'n'file, a shorthand for
dir-and-file? Perhaps a simpler more idiomatic name such as path
would suffice. Also, you can initialize the strbuf here at point of
declaration:
struct strbuf path = STRBUF_INIT;
Yes it is supposed to be dir-and-file I thought path isn't
descriptive enough because it could be path to dir. But if you insist,
no problems
The reason I asked was because it is not uncommon for variable names
with an 'n' suffix to mean length of something, so the 'n' in 'dirn'
was a bit confusing. I personally find the idiomatic name 'path'
easier to grok, however, Junio, of course, has final say-so. It's okay
to argue for your choice in naming if you feel strongly that it is
better.
+ const char *dir, *file;
+ char *filename;
+ int ret = 0;
+
+ dir = S_ISDIR(mode1) ? name1 : name2;
+ file = (dir == name1) ? name2 : name1;
+ strbuf_init(dirnfile, strlen(name1) + strlen(name2) + 2);
Unless this is a performance critical loop where you want to avoid
multiple re-allocations, it's customary to pass 0 for the second
argument. Doing so makes the code a bit easier to read and understand,
and avoids questions like this one: Why are you adding 2 to the
requested length? I presume that you're taking the / and NUL into
account, however, strbuf takes NUL into account on its own as part of
its contract, so you probably wanted to add only 1.
Yes I thought about performance. I thought it is reasonable since I
always know size of resulting string. Checked strbuf.c one more time,
yoг are right I should really add only 1
A few reasons I probably would just pass 0 in this case: (1) this
string construction is not performance critical; (2) a person reading
the code has to spend extra time double-checking the math; (3) the
math may become outdated if someone later alters the string
construction in some way, thus it's a potential maintenance burden;
(4) terser code tends to be easier to read and understand at a glance,
so the less verbose the code, the better.
However, as usual, it's entirely acceptable to argue for your version
if you feel strongly about it.
+ strbuf_addstr(dirnfile, dir);
+ if (dirnfile.buf[dirnfile.len - 1] != '/')
I don't know how others feel about it, but it makes me a bit
uncomfortable to see potential access of array item -1. I suppose, in
this case, neither name will be zero-length, however, I'd still feel
more comfortable seeing that documented formally, either via assert():
assert(dirnfile.len 0);
if (dirnfile.buf[dirnfile.len - 1] != '/')
or:
if (dirnfile.len 0 dirnfile.buf[dirnfile.len - 1] != '/')
My fault again
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] Documentation/git-add.txt: describe --exclude option
On Sun, Mar 15, 2015 at 3:06 PM, Alexander Kuleshov kuleshovm...@gmail.com wrote: Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Alexander Kuleshov kuleshovm...@gmail.com --- diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index f2eb907..fee97ed 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -9,7 +9,7 @@ SYNOPSIS [verse] 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p] - [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] + [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--exclude=pattern] [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--] [pathspec...] @@ -164,6 +164,10 @@ for git add --no-all pathspec..., i.e. ignored removed files. be ignored, no matter if they are already present in the work tree or not. +--exclude=pattern:: + Also ignore files matching pattern, a .gitignore-like + pattern. Option can be used multiply times. s/multiply/multiple/ + \--:: This option can be used to separate command-line options from the list of files, (useful when filenames might be mistaken -- 2.3.3.472.g20ceeac.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] cat-file: teach cat-file a '--literally' option
On Tue, Mar 17, 2015 at 1:16 AM, Karthik Nayak karthik@gmail.com wrote:
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index df99df4..1625246 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -323,8 +338,8 @@ static int batch_objects(struct batch_options *opt)
}
I don't presently have time for a proper read-through, however, this
popped out while quickly running my eyes over the changes.
static const char * const cat_file_usage[] = {
- N_(git cat-file (-t | -s | -e | -p | type | --textconv) object),
- N_(git cat-file (--batch | --batch-check) list-of-objects),
+ N_(git cat-file (-t [--literally]|-s
[--literally]|-e|-p|type|--textconv) object),
+ N_(git cat-file (--batch | --batch-check) list-of-objects),
The '' in the second line before list-of-objects is intentional. It
means that list-of-objects is provided via stdin. Therefore, it is
incorrect to remove it.
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: [PATCH v4/GSoC/MICRO] revision: forbid combining --graph and --no-walk
On Tue, Mar 17, 2015 at 7:18 PM, Junio C Hamano gits...@pobox.com wrote: Dongcan Jiang dongcan.ji...@gmail.com writes: diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index b68afef..a989e8f 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -99,7 +99,7 @@ do test_cmp $expect actual ' - test $cmd != diff || continue + test $cmd != diff -a $cmd != show || continue I think we avoid -a and -o used with test (don't we have a write-up on this somewhere?). The very last item in the shell script section of CodingGuidelines discusses it. Write it like this test $cmd != diff test $cmd != show || continue or perhaps like this case $cmd in diff|show) continue ;; esac -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository
On Thu, Mar 19, 2015 at 9:32 PM, Jeff King p...@peff.net wrote: On Thu, Mar 19, 2015 at 09:16:52PM -0400, Eric Sunshine wrote: --- /dev/null +++ b/t/t5312-prune-corruption.sh @@ -0,0 +1,104 @@ +# we do not want to count on running pack-refs to +# actually pack it, as it is perfectly reasonable to +# skip processing a broken ref +test_expect_success 'create packed-refs file with broken ref' ' + rm -f .git/refs/heads/master + cat .git/packed-refs -EOF Broken -chain. Thanks. I notice that a large number of broken -chains are on here-docs. I really wish you could put the on the EOF line at the end of the here-doc. I understand _why_ that this not the case, but mentally it is where I want to type it, and I obviously sometimes fail to go back and fix it. I don't think there's a better solution in POSIX sh, though. I wonder if test-lint could be enhanced to detect this sort of problem? -- To unsubscribe from this list: send the line 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: test -chain lint (was: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository)
On Fri, Mar 20, 2015 at 1:10 AM, Jeff King p...@peff.net wrote:
On Thu, Mar 19, 2015 at 10:25:32PM -0400, Jeff King wrote:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c096778..02a03d5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -524,6 +524,21 @@ test_eval_ () {
test_run_ () {
+ if test -n $GIT_TEST_CHAIN_LINT; then
+ # 117 is unlikely to match the exit code of
+ # another part of the chain
+ test_eval_ (exit 117) $1
+ if test $? != 117; then
+ # all bets are off for continuing with other tests;
+ # we expected none of the rest of the test commands to
+ # run, but at least some did. Who knows what weird
+ # state we're in? Just bail, and the user can diagnose
+ # by running in --verbose mode
+ error bug in the test script: broken -chain
+ fi
+ fi
Clever (Jonathan's too); much nicer than trying to special case only here-doc.
This turns up an appalling number of failures, but AFAICT they are all
real in the sense that the -chains are broken. In some cases these
are real, but in others the tests are of an older style where they did
not expect some early commands to fail (and we would catch their bogus
output if they did). E.g., in the patch below, I think the first one is
a real potential bug, and the other two are mostly noise. I do not mind
setting a rule and fixing all of them, though.
FWIW, I have spent about a few hours wading through the errors, and am
about 75% done. There are definitely some broken chains that were
causing test results to be ignored (as opposed to just minor setup steps
that we would not expect to fail). In most cases, the tests do passed. I
have a few that I still need to examine more closely, but there may be
some where there are actual test failures (but it's possible that I just
screwed it up while fixing the -chaining).
I hope to post something tonight, but I wanted to drop a note on the off
chance that you were actively looking at it at the same time.
Thanks for working on this. It looks like this technique should be a
valuable addition to test-lint. (I had intended, but haven't yet found
time to dig into it, so I'm happy to hear of your progress.)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] t5312: test object deletion code paths in a corrupted repository
On Tue, Mar 17, 2015 at 3:28 AM, Jeff King p...@peff.net wrote: When we are doing a destructive operation like git prune, we want to be extra careful that the set of reachable tips we compute is valid. If there is any corruption or oddity, we are better off aborting the operation and letting the user figure things out rather than plowing ahead and possibly deleting some data that cannot be recovered. Signed-off-by: Jeff King p...@peff.net --- diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh new file mode 100755 index 000..167031e --- /dev/null +++ b/t/t5312-prune-corruption.sh @@ -0,0 +1,104 @@ +# we do not want to count on running pack-refs to +# actually pack it, as it is perfectly reasonable to +# skip processing a broken ref +test_expect_success 'create packed-refs file with broken ref' ' + rm -f .git/refs/heads/master + cat .git/packed-refs -EOF Broken -chain. + $missing refs/heads/master + $recoverable refs/heads/other + EOF + echo $missing expect + git rev-parse refs/heads/master actual + test_cmp expect actual +' -- To unsubscribe from this list: send the line 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 5/5] refs.c: drop curate_packed_refs
On Tue, Mar 17, 2015 at 3:31 AM, Jeff King p...@peff.net wrote: When we delete a ref, we have to rewrite the entire packed-refs file. We take this opportunity to curate the packed-refs file and drop any entries that are crufty or broken. Dropping broken entries (e.g., with bogus names, or ones that point to missing objects) is actively a bad idea, as it means that we lose any notion that the data was there in the first place. Aside from the general hackiness that we might lose any information about ref foo while deleting an unrelated ref bar, this may seriously hamper any attempts by the user at recovering from the corruption in foo. They will lose the sha1 and name of foo; the exact pointer may still be useful even if they recover missing objects from a different copy of the repository. But worse, once the ref is gone, there is no trace of the corruption. A follow-up git prune may delete objects, even though it would otherwise bail when seeing corruption. We could just drop the broken bits from curate_packed_refs, and continue to drop the crufty bits: refs whose loose counterpart exists in the filesystem. This is not wrong to do, and it does have the advantage that we may write out a slightly smaller packed-refs file. But it has two disadvantages: 1. It is a potential source of races or mistakes with respect to these refs that are otherwise unrelated to the operation. To my knowledge, there aren't any active problems in this area, but it seems like an unnecessary risk. 2. We have to spend time looking up the matching loose refs for every item in the packed-refs file. If you have a large number of packed refs that do not change, that outweights the benefit from writing out a smaller s/outweights/outweighs/ packed-refs file (it doesn't get smaller, and you do a bunch of directory traversal to find that out). Signed-off-by: Jeff King p...@peff.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/25] detecting -chain breakage
On Fri, Mar 20, 2015 at 6:04 AM, Jeff King p...@peff.net wrote: [...] There were a number of false positives, though as a percentage of the test suite, probably not many (it's just that we have quite a lot of tests). Most of them were in rather old tests, and IMHO the fixes I did actually improved the readability of the result. So overall I think this is a very positive change; I doubt it will get in people's way very often, and I look forward to having one less thing to worry about handling manually in review. The biggest downside is that I may have automated Eric Sunshine out of a job. :) Heh. I won't mind. Thanks for doing a thorough job. Ironically, one of the broken here-doc -links you detected with --chain-lint and fixed in 4/25 was from a patch from me: 5a9830cb (t8001/t8002 (blame): add blame -L :funcname tests, 2013-07-17). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC][GSoC] diff-no-index: transform $directory $file args to $directory/$file $file
On Sat, Mar 21, 2015 at 8:50 AM, Yurii Shevtsov unge...@gmail.com wrote:
Signed-off-by: Yurii Shevtsov unge...@gmail.com
---
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..9a3439a 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,
if (get_mode(name1, mode1) || get_mode(name2, mode2))
return -1;
Somehow, you lost all the tabs in the patch, and everything is instead
indented with spaces (including context lines).
-if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
-return error(file/directory conflict: %s, %s, name1, name2);
+if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+struct strbuf path;
+const char *dir, *file;
+char *filename, *dirname = 0;
+int i, ret = 0;
+
+dir = S_ISDIR(mode1) ? name1 : name2;
+file = (dir == name1) ? name2 : name1;
+strbuf_init(path, strlen(name1) + strlen(name2) + 1);
+strbuf_addstr(path, dir);
+filename = strrchr(file, '/');
+if (path.len path.buf[path.len - 1] != '/')
+strbuf_addch(path, '/');
+for (i = path.len - 2; i = 0; i--)
+if (path.buf[i] == '/') {
+dirname = path.buf[i];
+break;
+}
+if (dirname == 0)
+dirname = path.buf;
+
+if (!strncmp(dirname, filename, strlen(filename)))
+return error(file/directory conflict: %s, %s, name1, name2);
Leaking 'path' strbuf.
+
+strbuf_addstr(path, filename ? (filename + 1) : file);
+if (file == name1)
+ret = queue_diff(o, file, path.buf);
+else
+ret = queue_diff(o, path.buf, file);
+strbuf_release(path);
+
+return ret;
+}
if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
struct strbuf buffer1 = STRBUF_INIT;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git ignore help
On Fri, Mar 20, 2015 at 6:36 AM, mdc...@seznam.cz wrote: I am trying to setup my git ignore (resp. .git/info/exclude) so that I exclude all directories and files except the content of directories that I specifically include (incl. anything within them recursively). I set the .git/info/exclude with the following content: # Exclude everything /* # Except the below that we include !/db/data/load/base/bootstraponly !/db/data/load/base/safetoload !/db/ddl !/labels !/reports/usrint !/scripts !/src/cmdsrc/usrint However it does not do what I anticipated. It indeed excludes everything but the include part does not work - it only works for !/labels and !/scripts directories (i.e. the first level directories). All other are still ignored - so when I create file /db/data/load/base/bootstraponly/somefile.txt git still ignores it... Any idea what I am doing wrong? The fourth bullet point of the Pattern Format section of the gitignore man page has this to say, which explains the behavior you're seeing: An optional prefix ! which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug in bash completion for git-branch --set-upstream-to on OSX
On Fri, Mar 20, 2015 at 11:15 AM, Jason Karns karns...@gmail.com wrote: There appears to be a bug in the bash completion for git-branch when attempting to complete the remote ref argument for --set-upstream-to= When: $ git branch --set-upstream-to=origin/mastTAB I would expect it to complete to: $ git branch --set-upstream-to=origin/master However, the completion for --set-upstream-to= completes the ref correctly, but completely wipes the --set-upstream option; resulting in: $ git branch origin/master I'm running on OS X 10.9.5 with git from homebrew: $ bash --version GNU bash, version 4.3.33(1)-release (x86_64-apple-darwin13.4.0) Presumably, your bash is also from homebrew? Stock OS X bash tends to be quite a bit older. $ git --version git version 2.3.3 I'm unable to reproduce this problem using git 2.3.3 and bash 4.3.33. The same behavior does *not* manifest (it works as expected) on CentOS 6.5, bash 4.1.2.1 (GNU bash, version 4.1.2(1)-release (x86_64-redhat-linux-gnu)). I'm running git 2.0.3 on CentOs but sourcing the shell completion script from latest source: 9ab698f I also cloned down latest git source on OS X and the bug still manifests when sourcing the completion script at 9ab698f. Perhaps something in your bash startup script(s) is causing a strange interaction. -- To unsubscribe from this list: send the line 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] align D/F handling of diff --no-index with that of normal Git
On Sun, Mar 22, 2015 at 1:11 AM, Junio C Hamano gits...@pobox.com wrote:
When a commit changes a path P that used to be a file to a directory
and create a new path P/X in it, git show would say that file P
s/create/creates/
More below...
was removed and file P/X was created for such a commit.
However, if we compare two directories, D1 and D2, where D1 has a
file D1/P in it and D2 has a directory D2/P under which there is a
file D2/P/X, and ask git diff --no-index D1 D2 to show their
differences, we simply get a refusal file/directory conflict.
The diff --no-index implementation has an underlying machinery
that can make it more in line with the normal Git if it wanted to,
but somehow it is not being exercised. The only thing we need to
do, when we see a file P and a directory P/ (or the other way
around) is to show the removal of a file P and then pretend as if we
are comparing nothing with a whole directory P/, as the code is
fully prepared to express a creation of everything in a directory
(and if the comparison is between a directory P/ and a file P, then
show the creation of the file and then let the existing code remove
everything in P/).
Signed-off-by: Junio C Hamano gits...@pobox.com
---
diff-no-index.c | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..52e9546 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -97,8 +97,27 @@ static int queue_diff(struct diff_options *o,
if (get_mode(name1, mode1) || get_mode(name2, mode2))
return -1;
- if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2))
- return error(file/directory conflict: %s, %s, name1, name2);
+ if (mode1 mode2 S_ISDIR(mode1) != S_ISDIR(mode2)) {
+ struct diff_filespec *d1, *d2;
+
+ if (S_ISDIR(mode1)) {
+ /* 2 is file that is created */
+ d1 = noindex_filespec(NULL, 0);
+ d2 = noindex_filespec(name2, mode2);
+ name2 = NULL;
+ mode2 = 0;
+ } else {
+ /* 1 is file that is deleted */
+ d1 = noindex_filespec(name1, mode2);
+ d2 = noindex_filespec(NULL, 0);
+ name1 = NULL;
+ mode1 = 0;
+ }
+ /* emit that file */
+ diff_queue(diff_queued_diff, d1, d2);
+
+ /* and then let the entire directory created or deleted */
s/created/be created/
+ }
if (S_ISDIR(mode1) || S_ISDIR(mode2)) {
struct strbuf buffer1 = STRBUF_INIT;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] numparse: new module for parsing integral numbers
On Tuesday, March 17, 2015, Michael Haggerty mhag...@alum.mit.edu wrote:
Implement wrappers for strtol() and strtoul() that are safer and more
convenient to use.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
diff --git a/numparse.c b/numparse.c
new file mode 100644
index 000..90b44ce
--- /dev/null
+++ b/numparse.c
@@ -0,0 +1,180 @@
+int parse_l(const char *s, unsigned int flags, long *result, char **endptr)
+{
+ long l;
+ const char *end;
+ int err = 0;
+
+ err = parse_precheck(s, flags);
+ if (err)
+ return err;
+ /*
+* Now let strtol() do the heavy lifting:
+*/
Perhaps use a /* one-line style comment */ to reduce vertical space
consumption a bit, thus make it (very slightly) easier to run the eye
over the code.
+ errno = 0;
+ l = strtol(s, (char **)end, flags NUM_BASE_MASK);
+ if (errno) {
+ if (errno == ERANGE) {
+ if (!(flags NUM_SATURATE))
+ return -NUM_SATURATE;
+ } else {
+ return -NUM_OTHER_ERROR;
+ }
+ }
Would it reduce cognitive load slightly (and reduce vertical space
consumption) to rephrase the conditionals as:
if (errno == ERANGE !(flags NUM_SATURATE))
return -NUM_SATURATE;
if (errno errno != ERANGE)
return -NUM_OTHER_ERROR;
or something similar?
More below.
+ if (end == s)
+ return -NUM_NO_DIGITS;
+
+ if (*end !(flags NUM_TRAILING))
+ return -NUM_TRAILING;
+
+ /* Everything was OK */
+ *result = l;
+ if (endptr)
+ *endptr = (char *)end;
+ return 0;
+}
diff --git a/numparse.h b/numparse.h
new file mode 100644
index 000..4de5e10
--- /dev/null
+++ b/numparse.h
@@ -0,0 +1,207 @@
+#ifndef NUMPARSE_H
+#define NUMPARSE_H
+
+/*
+ * Functions for parsing integral numbers.
+ *
+ * Examples:
+ *
+ * - Convert s into a signed int, interpreting prefix 0x to mean
+ * hexadecimal and 0 to mean octal. If the value doesn't fit in an
+ * unsigned int, set result to INT_MIN or INT_MAX.
Did you mean s/unsigned int/signed int/ ?
+ *
+ * if (convert_i(s, NUM_SLOPPY, result))
+ * die(...);
+ */
+
+/*
+ * The lowest 6 bits of flags hold the numerical base that should be
+ * used to parse the number, 2 = base = 36. If base is set to 0,
+ * then NUM_BASE_SPECIFIER must be set too; in this case, the base is
+ * detected automatically from the string's prefix.
Does this restriction go against the goal of making these functions
convenient, even while remaining strict? Is there a strong reason for
not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would
make it consistent with strto*l() without (I think) introducing any
ambiguity.
+ */
+/*
+ * Number parsing functions:
+ *
+ * The following functions parse a number (long, unsigned long, int,
+ * or unsigned int respectively) from the front of s, storing the
+ * value to *result and storing a pointer to the first character after
+ * the number to *endptr. flags specifies how the number should be
+ * parsed, including which base should be used. flags is a combination
+ * of the numerical base (2-36) and the NUM_* constants above (see).
(see) what?
+ * Return 0 on success or a negative value if there was an error. On
+ * failure, *result and *entptr are left unchanged.
+ *
+ * Please note that if NUM_TRAILING is not set, then it is
+ * nevertheless an error if there are any characters between the end
+ * of the number and the end of the string.
Again, on the subject of convenience, why this restriction? The stated
purpose of the parse_*() functions is to parse the number from the
front of the string and return a pointer to the first non-numeric
character following. As a reader of this API, I interpret that as
meaning that NUM_TRAILING is implied. Is there a strong reason for not
inferring NUM_TRAILING for the parse_*() functions at the API level?
(I realize that the convert_*() functions are built atop parse_*(),
but that's an implementation detail.)
+ */
+
+int parse_l(const char *s, unsigned int flags,
+ long *result, char **endptr);
Do we want to perpetuate the ugly (char **) convention for 'endptr'
from strto*l()? Considering that the incoming string is const, it
seems undesirable to return a non-const pointer to some place inside
that string.
+/*
+ * Number conversion functions:
+ *
+ * The following functions parse a string into a number. They are
+ * identical to the parse_*() functions above, except that the endptr
+ * is not returned. These are most useful when parsing a whole string
+ * into a number; i.e., when (flags NUM_TRAILING) is unset.
I can formulate arguments for allowing or disallowing NUM_TRAILING
with convert_*(), however, given their purpose of parsing the
Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 18, 2015 at 3:26 PM, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan pyoka...@gmail.com wrote: t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: --- The previous version can be found at [1]. [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308 * Merge related, but previously separate, tests together in order to make the test suite easier to understand. * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set it, and unset it immediately before and after helper_test store is called in order to make it localized to only the command that it should affect. * Add test, previously missing, to check that only the home credentials file is written to if both the xdg and home files exist. * Correct mislabelling of home-user/home-pass to the proper xdg-user/xdg-pass. * Use rm -f instead of test_might_fail rm. This round looks much better. Thanks. Most of the comments below are just nit-picky, with one or two genuine (minor) issues. I should add that the nit-picky items are not necessarily actionable. As the person doing the actual work, it's okay if you disagree and feel that they are not worth the effort of addressing. (The genuine issues, on the other hand, ought to be addressed.) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/14] numparse: new module for parsing integral numbers
On Wed, Mar 18, 2015 at 6:47 PM, Michael Haggerty mhag...@alum.mit.edu wrote: On 03/18/2015 07:27 PM, Eric Sunshine wrote: On Tuesday, March 17, 2015, Michael Haggerty mhag...@alum.mit.edu wrote: Implement wrappers for strtol() and strtoul() that are safer and more convenient to use. + * The lowest 6 bits of flags hold the numerical base that should be + * used to parse the number, 2 = base = 36. If base is set to 0, + * then NUM_BASE_SPECIFIER must be set too; in this case, the base is + * detected automatically from the string's prefix. Does this restriction go against the goal of making these functions convenient, even while remaining strict? Is there a strong reason for not merely inferring NUM_BASE_SPECIFIER when base is 0? Doing so would make it consistent with strto*l() without (I think) introducing any ambiguity. I thought about doing this. If it were possible to eliminate NUM_BASE_SPECIFIER altogether, then there is no doubt that it would be a good change. But NUM_BASE_SPECIFIER also has an effect when base==16; namely, that an 0x prefix, if present, is consumed. So parse_i(0xff, 16 | NUM_BASE_SPECIFIER, result, endptr) gives result==255 and endptr==s+4, whereas parse_i(0xff, 16, result, endptr) gives result==0 and entptr==s+1 (it treats the x as the end of the string). We could forgo that feature, effectively allowing a base specifier if and only if base==0. But I didn't want to rule out allowing an optional base specifier for base==16, in which case NUM_BASE_SPECIFIER can't be dispensed with entirely. Making base==0 a special case doesn't have to mean ruling out or eliminating NUM_BASE_SPECIFIER for the base==16 case. However, a special case base==0 would make the API a bit non-orthogonal and require extra documentation. But for those familiar with how strto*l() treats base==0 specially, the rule of least surprise may apply. If you agree with that, then the remaining question is: which policy is less error-prone? My thinking was that forcing the caller to specify NUM_BASE_SPECIFIER explicitly when they select base==0 will serve as a reminder that the two features are intertwined. Since base==0 would unambiguously imply NUM_BASE_SPECIFIER, being able to tersely say convert_i(s, 0, result) would be a win from a convenience perspective. However... Because another imaginable policy (arguably more consistent with the policy for base!=0) would be that convert_i(s, 0, result) , because it *doesn't* specify NUM_BASE_SPECIFIER, doesn't allow a base prefix, and therefore indirectly only allows base-10 numbers. But I don't feel strongly about this. I don't feel strongly about it either, and can formulate arguments either way. Assuming you stick with your current design, if the strictness of having to specify NUM_BASE_SPECIFIER for base==0 proves too burdensome, it can be loosened later by making base==0 a special case. On the other hand, if you go with Junio's suggestion of choosing names for these functions which more closely mirror strto*l() names, then the rule of least surprise might suggest that a special case for base==0 has merit. + * Return 0 on success or a negative value if there was an error. On + * failure, *result and *entptr are left unchanged. + * + * Please note that if NUM_TRAILING is not set, then it is + * nevertheless an error if there are any characters between the end + * of the number and the end of the string. Again, on the subject of convenience, why this restriction? The stated purpose of the parse_*() functions is to parse the number from the front of the string and return a pointer to the first non-numeric character following. As a reader of this API, I interpret that as meaning that NUM_TRAILING is implied. Is there a strong reason for not inferring NUM_TRAILING for the parse_*() functions at the API level? (I realize that the convert_*() functions are built atop parse_*(), but that's an implementation detail.) Yes, I'd also thought about that change: * Make NUM_TRAILING private. * Make the current parse_*() functions private. * Add new parse_*(s, flags, result, endptr) functions that imply NUM_TRAILING (and maybe they should *require* a non-null endptr argument). * Change the convert_*() to not allow the NUM_TRAILING flag. This would add a little bit of code, so I didn't do it originally. But since you seem to like the idea too, I guess I will make the change. The other option (as Junio also suggested) is to collapse this API into just the four parse_*() functions. All of the call-sites you converted in this series invoked convert_*(), but it's not clear that having two sets of functions which do almost the same thing is much of a win. If the default is for NUM_TRAILING to be off, and if NULL 'endptr' is allowed, then: parse_ul(s, 10, result, NULL); doesn't seem particularly burdensome compared with: convert_ul(s, 10, result); A more compact API, with only the parse_
Re: [PATCH v4] git: treat -C treat as a no-op when path is empty
On Fri, Mar 6, 2015 at 6:18 AM, Karthik Nayak karthik@gmail.com wrote: 'git -C ' unhelpfully dies with error Cannot change to '', whereas the shell treats `cd ' as a no-op. Taking the shell's behavior as a precedent, teach git to treat `-C ' as a no-op, as well. Test to check the no-op behaviour of -C path when path is empty, written by Junio C Hamano. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunchine sunsh...@sunshineco.com s/Sunchine/Sunshine/ Signed-off-by: Karthik Nayak karthik@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] list-files: delete redundant cached entries
On Monday, March 9, 2015, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
When both --cached and one of -amdAMD is used together we may have two
entries of the same path, e.g. foo and MM foo. In this case it's
pretty clear that foo must be tracked, no need to displayfoo.
The new function does that.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
diff --git a/builtin/list-files.c b/builtin/list-files.c
index 74836f6..49fb820 100644
--- a/builtin/list-files.c
+++ b/builtin/list-files.c
@@ -305,6 +308,34 @@ static void wt_status_populate(struct string_list
*result,
string_list_remove_duplicates(result, 0);
}
+static void delete_duplicate_cached_entries(struct string_list *result)
+{
+ struct string_list_item *src, *dst;
+
+ if (show_unmerged || !show_cached || !show_changed)
+ return;
+
+ src = dst = result-items;
+ while (src + 1 result-items + result-nr) {
+ const char *s0 = dst-string;
+ const char *s1 = src[1].string;
+
+ if (s0[0] == ' ' s0[1] == ' '
+ !strcmp(s0 + 3, s1 + 3)) {
+ src++;
+ } else {
+ dst++;
+ src++;
+ }
+ if (src != dst)
+ *dst = *src;
+ }
+ if (src != dst)
+ *dst = *src;
Do you want to take some action here (and just above) to ensure that
the item being overwritten gets deleted properly rather than leaked?
+ result-nr = dst - result-items;
+
+}
+
--
To unsubscribe from this list: send the line 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 v3 2/2] Added tests for reset -
On Tue, Mar 10, 2015 at 1:52 PM, Sudhanshu Shekhar sudshekha...@gmail.com wrote: On Tue, Mar 10, 2015 at 6:56 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Sudhanshu Shekhar sudshekha...@gmail.com writes: +test_expect_success 'reset - in the presence of file named - with previous branch' ' + echo Unstaged changes after reset: expect + echo M - expect + echo M 1 expect Here and elsewhere: why not cat expect -EOF Unstaged changes after reset: M - M 1 ? I was confused whether to use cat or echo. I thought using cat will disrupt the indentation as the EOF needs to be on a single line. This is why I chose echo. Please let me know your thoughts on this. Here-docs are easier to compose and read than individual 'echo' statements for multi-line content. The '-' in front of EOF allows you to indent the entire body. Even better, use -\EOF to signify that you don't expect any interpolation to occur within the body. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] *config.txt: stick to camelCase naming convention
On Wed, Mar 11, 2015 at 07:20:18PM +0700, Nguyễn Thái Ngọc Duy wrote: This should improve readability. Compare thislongname and thisLongName. The following keys are left in unchanged. We can decide what to do with them later. - am.keepcr - core.autocrlf .safecrlf .trustctime - diff.dirstat .noprefix - gitcvs.usecrlfattr - gui.blamehistoryctx .trustmtime - pull.twohead - receive.autogc - sendemail.signedoffbycc .smtpsslcertpath .suppresscc Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/config.txt b/Documentation/config.txt index 1530255..54ae0f6 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -237,7 +237,7 @@ The default is false, except linkgit:git-clone[1] or linkgit:git-init[1] will probe and set core.ignorecase true if appropriate when the repository is created. -core.precomposeunicode:: +core.precomposeUnicode:: There are numerous places in config.txt where a config variable is mentioned in the description blurbs as well, so camel-casing it at just the point of definition makes for an incomplete conversion. This option is only used by Mac OS implementation of Git. When core.precomposeunicode=true, Git reverts the unicode decomposition Here is one such instance of the variable mentioned inline in a blurb. These converted variables are also mentioned in a number of other documentation files. Below is a more thorough conversion, covering all instances. You may want to split it up for easier review. --- 8 --- From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= pclo...@gmail.com Subject: [PATCH] *config.txt: stick to camelCase naming convention This should improve readability. Compare thislongname and thisLongName. The following keys are left in unchanged. We can decide what to do with them later. - am.keepcr - core.autocrlf .safecrlf .trustctime - diff.dirstat .noprefix - gitcvs.usecrlfattr - gui.blamehistoryctx .trustmtime - pull.twohead - receive.autogc - sendemail.signedoffbycc .smtpsslcertpath .suppresscc Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/CodingGuidelines | 2 +- Documentation/blame-options.txt| 2 +- Documentation/config.txt | 242 ++--- Documentation/diff-config.txt | 10 +- Documentation/diff-options.txt | 4 +- Documentation/fetch-options.txt| 2 +- Documentation/git-add.txt | 4 +- Documentation/git-apply.txt| 2 +- Documentation/git-branch.txt | 6 +- Documentation/git-check-ignore.txt | 2 +- Documentation/git-checkout.txt | 4 +- Documentation/git-commit-tree.txt | 2 +- Documentation/git-commit.txt | 2 +- Documentation/git-config.txt | 2 +- Documentation/git-cvsserver.txt| 22 ++-- Documentation/git-fetch.txt| 2 +- Documentation/git-format-patch.txt | 4 +- Documentation/git-gc.txt | 12 +- Documentation/git-init.txt | 2 +- Documentation/git-instaweb.txt | 2 +- Documentation/git-log.txt | 2 +- Documentation/git-merge.txt| 4 +- Documentation/git-pull.txt | 2 +- Documentation/git-rebase.txt | 6 +- Documentation/git-receive-pack.txt | 2 +- Documentation/git-repack.txt | 4 +- Documentation/git-rerere.txt | 2 +- Documentation/git-send-email.txt | 50 Documentation/git-status.txt | 4 +- Documentation/git-tag.txt | 2 +- Documentation/git.txt | 2 +- Documentation/gitattributes.txt| 2 +- Documentation/gitcredentials.txt | 2 +- Documentation/gitignore.txt| 4 +- Documentation/gitweb.conf.txt | 2 +- Documentation/merge-config.txt | 2 +- Documentation/user-manual.txt | 2 +- 37 files changed, 212 insertions(+), 212 deletions(-) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 7636199..376d5ec 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -526,7 +526,7 @@ Writing Documentation: `backticks around word phrases`, do so. `--pretty=oneline` `git rev-list` - `remote.pushdefault` + `remote.pushDefault` Word phrases enclosed in `backtick characters` are rendered literally and will not be further expanded. The use of `backticks` to achieve the diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 0cebc4f..b299b59 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -4,7 +4,7 @@ --root:: Do not treat root commits as boundaries. This can also be - controlled via the `blame.showroot` config option. + controlled via the `blame.showRoot` config option. --show-stats:: Include additional statistics at the end of blame
Re: [PATCH] [GSoC][MICRO] Forbid log --graph --no-walk
On Fri, Mar 6, 2015 at 3:55 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
Forbid log --graph --no-walk
Style: drop capitalization in the Subject: line. Also prefix with the
command or module being modified, followed by a colon. So:
log: forbid combining --graph and --no-walk
or:
revision: forbid combining --graph and --no-walk
Because --graph is about connected history while --no-walk is about discrete
points.
Okay. You might also want to cite the wider discussion[1].
[1]: http://article.gmane.org/gmane.comp.version-control.git/216083
revision.c: Judge whether --graph and --no-walk come together when running
git-log.
buildin/log.c: Set git-log cmd flag.
Documentation/rev-list-options.txt: Add specification on the forbidden usage.
No need to repeat in prose what the patch itself states more clearly
and concisely.
Also, such a change should be accompanied by new test(s).
Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com
---
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 4ed8587..eea2c0a 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+ Cannot be combined with `--graph` when running git-log.
--do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output. This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+ Cannot be combined with `--no-walk` when running git-log.
Nice to see documentation updates. More below.
This enables parent rewriting, see 'History Simplification' below.
+
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..7bf5adb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -627,6 +627,7 @@ int cmd_log(int argc, const char **argv, const char
*prefix)
git_config(git_log_config, NULL);
init_revisions(rev, prefix);
+ rev.cmd_is_log = 1;
rev.always_show_header = 1;
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
diff --git a/revision.c b/revision.c
index 66520c6..5f62c89 100644
--- a/revision.c
+++ b/revision.c
@@ -1399,6 +1399,8 @@ void init_revisions(struct rev_info *revs, const char
*prefix)
revs-commit_format = CMIT_FMT_DEFAULT;
+ revs-cmd_is_log = 0;
+
init_grep_defaults();
grep_init(revs-grep_filter, prefix);
revs-grep_filter.status_only = 1;
@@ -2339,6 +2341,8 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
if (revs-reflog_info revs-graph)
die(cannot combine --walk-reflogs with --graph);
+ if (revs-no_walk revs-graph revs-cmd_is_log)
Placing 'revs-cmd_is_log' first would make it clear at a glance that
this restriction impacts 'log' only (but see question below):
if (revs-cmd_is_log revs-no_walk revs-graph)
+ die(cannot combine --no-walk with --graph when running
git-log);
if (!revs-reflog_info revs-grep_filter.use_reflog_filter)
die(cannot use --grep-reflog without --walk-reflogs);
diff --git a/revision.h b/revision.h
index 0ea8b4e..255982a 100644
--- a/revision.h
+++ b/revision.h
@@ -146,6 +146,9 @@ struct rev_info {
track_first_time:1,
linear:1;
+ /* cmd type */
+ unsigned int cmd_is_log:1;
Genuine question: Despite the GSoC micro-project mentioning only
'log', is it ever meaningful for these two options to be specified
together? I suspect not, but it would be nice to hear from someone
more familiar with the issue. If not specific to 'log', then the patch
can be simplified a good deal.
enum date_mode date_mode;
unsigned intabbrev;
--
--
To unsubscribe from this list: send the line 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] t7510: do not fail when gpg warns about insecure memory
On Sun, Mar 8, 2015 at 11:40 AM, Kyle J. McKay mack...@gmail.com wrote: Depending on how gpg was built, it may issue the following message to stderr when run: Warning: using insecure memory! Unfortunately when running the test, that message gets collected in the stdout result of git show -s --show-signature but is collected in the stderr result of git verify-commit -v I'm having trouble parsing this. Is there a word missing? causing both the stdout and stderr result comparisions to fail. s/comparisions/comparisons/ Since checking for secure memory use by gpg is not the point of this test, filter out such messages to allow the test to pass even when gpg is using insecure memory. Signed-off-by: Kyle J. McKay mack...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7510: do not fail when gpg warns about insecure memory
On Sun, Mar 8, 2015 at 6:04 PM, brian m. carlson sand...@crustytoothpaste.net wrote: On Sun, Mar 08, 2015 at 05:43:41PM -0400, Eric Sunshine wrote: On Sun, Mar 8, 2015 at 11:40 AM, Kyle J. McKay mack...@gmail.com wrote: Warning: using insecure memory! Unfortunately when running the test, that message gets collected in the stdout result of git show -s --show-signature but is collected in the stderr result of git verify-commit -v I'm having trouble parsing this. Is there a word missing? Perhaps this is better? Unfortunately when running the test, that message is found in the standard output of git show -s --show-signature, but in the standard error of git verify-commit -v causing the comparisons of both standard output and standard error to fail. That doesn't help me parse it any better. It's the but without a corresponding not which seems to be throwing me off. Typically, one would write this but not that or not this but that. I can't tell if there is a not missing or if the but is supposed to be an and or if something else was intended. -- To unsubscribe from this list: send the line 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 v3 1/3] cache: modify for cat-file --literally -t
On Thu, Mar 5, 2015 at 1:18 PM, Karthik Nayak karthik@gmail.com wrote:
cache: modify for cat-file --literally -t
It is desirable for the first line of the commit message to explain,
as well as possible, the intent of the patch. The bulk of the commit
message then elaborates. Unfortunately, this line says almost nothing.
All patches modify, so writing modify here is not helpful and merely
wastes precious horizontal real estate. A more informative summary
might say something like:
cache: add object_info::typename in support of 'cat-file --literally'
Add a struct strbuf *typename to object_info to hold the
typename when the literally option is used. Add a flag to
notify functions when literally is used.
It's good to split up changes such that each patch comprises one
logical step, however, this patch does not really do anything on its
own, so having it stand-alone doesn't make much sense. It would make
more sense to fold it into the patch which actually requires these
changes.
Signed-off-by: Karthik Nayak karthik@gmail.com
---
diff --git a/cache.h b/cache.h
index 4d02efc..949ef4c 100644
--- a/cache.h
+++ b/cache.h
@@ -830,6 +830,7 @@ extern int is_ntfs_dotgit(const char *name);
/* object replacement */
#define LOOKUP_REPLACE_OBJECT 1
+#define LOOKUP_LITERALLY 2
extern void *read_sha1_file_extended(const unsigned char *sha1, enum
object_type *type, unsigned long *size, unsigned flag);
static inline void *read_sha1_file(const unsigned char *sha1, enum
object_type *type, unsigned long *size)
{
@@ -1296,6 +1297,7 @@ struct object_info {
unsigned long *sizep;
unsigned long *disk_sizep;
unsigned char *delta_base_sha1;
+ struct strbuf *typename;
/* Response */
enum {
--
2.3.1.167.g7f4ba4b.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GSoC][PATCH 1/2] userdiff: add built-in patterns for CSS
On Sun, Mar 8, 2015 at 7:03 AM, Hiroyuki Sano sh19910...@gmail.com wrote:
Add regex patterns for CSS. The word regex maches selectors, properties,
and values. On the other hand, the funcname regex matches lines contains
the curly brace character.
Signed-off-by: Hiroyuki Sano sh19910...@gmail.com
---
diff --git a/t/t4034/css/post b/t/t4034/css/post
new file mode 100644
index 000..7e64463
--- /dev/null
+++ b/t/t4034/css/post
@@ -0,0 +1,32 @@
[...]
+.class, elm:hover, elm:first-child, elm:lang(en), #id, elm#id, .num123{
Mental note: colon and parentheses are present
diff --git a/userdiff.c b/userdiff.c
index 2ccbee5..8374a2a 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -37,6 +37,9 @@ IPATTERN(fortran,
|//|\\*\\*|::|[/=]=),
PATTERNS(html, ^[ \t]*([Hh][1-6][ \t].*.*)$,
[^= \t]+),
+PATTERNS(css,
+^.*[{].*$,
+[-_\\.,#a-zA-Z0-9]+),
Is the intention that this should match elm:lang(en) as an atom, or
separately match elm, lang, and en?
PATTERNS(java,
!^[
\t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n
^[ \t]*(([A-Za-z_][A-Za-z_0-9]*[ \t]+)+[A-Za-z_][A-Za-z_0-9]*[
\t]*\\([^;]*)$,
--
--
To unsubscribe from this list: send the line 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 v3] rev-list: refuse --first-parent combined with --bisect
On Sun, Mar 8, 2015 at 11:03 AM, Kevin Daudt m...@ikke.info wrote:
rev-list --bisect is used by git bisect, but never together with
--first-parent. Because rev-list --bisect together with --first-parent
is not handled currently, and even leads to segfaults, refuse to use
both options together.
Signed-off-by: Kevin Daudt m...@ikke.info
Suggested-by: Junio C. Hamano gits...@pobox.com
It's customary for your sign-off to be last.
---
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index 4ed8587..05c3f6d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -123,7 +123,8 @@ parents) and `--max-parents=-1` (negative numbers denote
no upper limit).
because merges into a topic branch tend to be only about
adjusting to updated upstream from time to time, and
this option allows you to ignore the individual commits
- brought in to your history by such a merge.
+ brought in to your history by such a merge. Cannot be
+ combined with --bisect.
A couple questions:
Should the documentation for ---bisect be updated to mention this
restriction also?
Should this change be protected by a ifndef::git-rev-list[] as are
all other mentions of bisect in rev-list-options.txt?
--not::
Reverses the meaning of the '{caret}' prefix (or lack thereof)
--
To unsubscribe from this list: send the line 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: [GSoC][PATCH 2/2] attrs: add css to the list of userdiff bulit-in patterns
On Sun, Mar 8, 2015 at 7:03 AM, Hiroyuki Sano sh19910...@gmail.com wrote: attrs: add css to the list of userdiff bulit-in patterns s/bulit/built/ Signed-off-by: Hiroyuki Sano sh19910...@gmail.com --- diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index c892ffa..8904a2a 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -525,6 +525,8 @@ patterns are available: - `csharp` suitable for source code in the C# language. +- `css` suitable for source code in the CSS documents. Taking a hint from the 'html' case, it might make more sense to say: `css` suitable for CSS documents. Considering how directly related this change is to those in patch 1, it's not clear why this change needs a separate patch, so folding them into a single patch might be sensible. + - `fortran` suitable for source code in the Fortran language. - `html` suitable for HTML/XHTML documents. -- 2.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] git: treat -C treat as a no-op when path is empty
On Sat, Mar 7, 2015 at 5:49 AM, karthik nayak karthik@gmail.com wrote: This iteration looks sensible, except that the Subject reads strange. Will queue with minor tweaks to the log message, and perhaps with a fix to unreadable *(*argv)[1] that was mentioned elsewhere. Hey could you tell me what seems strange, so I can improve on it the next time. Junio means that you somehow botched the Subject: line when you copied the commit message I suggested into your new version of the patch. Instead of path, you wrote treat. Also *(*argv)[1] seems more readable to me, maybe more of a perspective? I also had considered suggesting (*argv)[1][0] as more readable, but it is primarily personal taste, and I didn't want to bike-shed the issue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] reset: enable '-' short-hand for previous branch
On Fri, Mar 13, 2015 at 2:18 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
git reset '-' will reset to the previous branch. To reset a file named
- use either git reset ./- or git reset -- -.
Change error message to treat single - as an ambigous revision or
path rather than a bad flag.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
I have changed the logic to ensure argv[0] isn't changed at any point.
Creating a modified_argv0 variable let's me do that.
I couldn't think of any other way to achieve this, apart from changing things
directly in the sha1_name.c file (like Junio's changes). Please let me know
if some further changes are needed.
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..bc50e14 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = HEAD;
unsigned char unused[20];
+ const char *modified_argv0 = argv[0];
This variable is only needed inside the 'if (argv[0])' block below, so
its declaration should be moved there. Also the initialization to
argv[0] is wasted since the assignment is overwritten below.
The variable name itself could be better. Unlike a name such as
'orig_arg0', modified doesn't tell us much. Modified how? Modified
to be what? Consideration where and how the variable is used, we can
see that it will be holding a value that _might_ be a rev. This
suggests a better name such as 'maybe_rev' or something similar.
/*
* Possible arguments are:
*
@@ -205,10 +206,17 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ modified_argv0 = @{-1};
+ }
+ else {
Style: cuddle the 'else' and braces: } else {
+ modified_argv0 = argv[0];
+ }
The unnecessary braces make this harder to read than it could be since
it is so spread out vertically. Dropping the braces would help. The
ternary operator ?: might improve readability (though it might also
make it worse).
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
- rev = argv[0];
+ rev = modified_argv0;
argv += 2;
}
/*
@@ -216,14 +224,15 @@ static void parse_args(struct pathspec *pathspec,
* has to be unambiguous. If there is a single argument, it
* can not be a tree
*/
- else if ((!argv[1] !get_sha1_committish(argv[0], unused))
||
-(argv[1] !get_sha1_treeish(argv[0], unused))) {
+ else if ((!argv[1] !get_sha1_committish(modified_argv0,
unused)) ||
+(argv[1] !get_sha1_treeish(modified_argv0,
unused))) {
/*
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
- rev = *argv++;
+ rev = modified_argv0;
+ argv++;
Good. This is much better than the previous rounds, and is the sort of
solution I had hoped to see when prodding you in previous reviews to
avoid argv[] kludges. Unlike the previous band-aid approach, this
demonstrates that you took the time to understand the overall logic
flow rather than merely plopping in a quick fix.
} else {
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
diff --git a/setup.c b/setup.c
index 979b13f..b621b62 100644
--- a/setup.c
+++ b/setup.c
@@ -200,7 +200,7 @@ void verify_filename(const char *prefix,
int diagnose_misspelt_rev)
{
if (*arg == '-')
- die(bad flag '%s' used after filename, arg);
+ die(ambiguous argument '%s': unknown revision or path, arg);
The conditional is only checking if the first character of 'arg' is
hyphen; it's not checking if 'arg' is exactly -. It's purpose is to
recognize -flags or --flags, so it's inappropriate to change the error
message like this. I think this also doesn't help the case when there
really is a file named -, since this conditional will just claim
that it's ambiguous.
It might or might not be appropriate to add a special case here to
allow an exact - to fall through to the check_filename() call below,
however, it would be necessary to thoroughly check for possible
Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen
On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
This patch is just for discusstion. An option --deepen is added to
'git fetch'. When it comes to '--deepen', git should fetch N more
commits ahead the local shallow commit, where N is indicated by
'--depth=N'. [1]
[...]
Of course, as a patch for discussion, it remains a long way to go
before being complete.
[...]
Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com
---
diff --git a/fetch-pack.c b/fetch-pack.c
index 655ee64..6f4adca 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,
if (no_done)strbuf_addstr(c, no-done);
if (use_sideband == 2) strbuf_addstr(c,
side-band-64k);
if (use_sideband == 1) strbuf_addstr(c,
side-band);
+ if (args-depth_deepen) strbuf_addstr(c,
depth_deepen);
For consistency, should this be depth-deepen rather than depth_deepen?
if (args-use_thin_pack) strbuf_addstr(c,
thin-pack);
if (args-no_progress) strbuf_addstr(c,
no-progress);
if (args-include_tag) strbuf_addstr(c,
include-tag);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d78f320..6738006 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '
)
'
+test_expect_success 'fetching deepen' '
+ git clone . deepen --depth=1
+ cd deepen
When this tests ends, the working directory will still be 'deepen',
which will likely break tests added after this one. If you wrap the
'cd' and following statements in a subshell via '(' and ')', then the
'cd' will affect the subshell but leave the parent shell's working
directory alone, and thus not negatively impact subsequent tests.
+ git fetch .. foo --depth=1
+ git show foo
+ test_must_fail git show foo~
+ git fetch .. foo --depth=1 --deepen
+ git show foo~
+ test_must_fail git show foo~2
Broken -chain throughout.
+'
+
test_done
--
To unsubscribe from this list: send the line 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 v5 2/2] t7102: add 'reset -' tests
On Fri, Mar 13, 2015 at 2:18 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
Add following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric: Thank you for pointing out the mistake. The '' after the Here
Docs was causing the issue. I have removed the concatenation from
there, hope that's okay.
The needs to go on the first line, not the last line of the here-doc.
However, that was not my main concern in the previous review. What
disturbed me was that the new tests, which were supposed to be
checking if - behaved as @{-1}, were succeeding even without patch
1/2 applied which implemented the - alias for @{-1}. That seems
wrong. I don't think you particularly addressed that issue in this
version (even though the first couple tests will now fail without 1/2
due to the changed error message).
Regarding the @{-1} test case, I created it as a check for Junio's
comment on the error message generated by git reset - when a file
named @{-1} is there. Since, in this situation git reset @{-1} will
return an error (but reset - shouldn't).
Reminder: Wrap commentary to about column 72, as you would the commit
message. (I re-wrapped it manually to reply to it.)
I have renamed the folder to 'dash' as suggested by you, keeping the
old name only where it made sense.
Considering that the test titles already tell us the intent of the
tests, I don't find that the directory name no_previous adds much
value to tests checking the behavior of - with no previous branch. A
single, consistent name used throughout all these tests would be less
surprising and place smaller cognitive load on the reader.
More below.
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..18523c1 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,162 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+test_expect_success 'reset - while having file named - and no previous
branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+
Style: Unnecessary extra blank line.
+test_expect_success \
+ 'reset - in the presence of file named - with previous branch resets
commit' '
+ cat expect -EOF
Place the at the end of this line. Also, prefix EOF with a
backslash to indicate that you don't intend any interpolation to occur
within the here-doc. So:
cat expect -\EOF
Ditto for the remaining tests.
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - ../actual
+ )
+ test_cmp expect actual
+'
+test_expect_success \
+ 'reset - in the presence of file named - with -- option resets
commit' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - -- ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - in the presence of file named - with -- file
option resets file' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
Re: [v2 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
On Tue, Mar 10, 2015 at 11:38 AM, Sundararajan R dyou...@gmail.com wrote:
Teaching reset the - shorthand involves checking if any file named '-' exists
because it then becomes ambiguous as to whether the user wants to reset the
file '-' or if he wants to reset the working tree to the previous branch.
For clarity, I'd probably mention that the ambiguity arises only in
the absence of explicit '--' disambiguation.
check_filename() is used to perform this check. A similar ambiguity occurs
when the file @{-1} exits. Therefore, when the files '-' or '@{-1}' exist
then the program dies with a message about the ambiguous argument.
Why single out @{-1} as a potential file name? Has @{-1} ever been
considered a filename rather than a treeish? Is this patch changing
the treatment of @{-1} so that it might be interpreted as a filename?
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sundararajan R dyou...@gmail.com
---
Have made the modifications suggest by you, Eric.
Removed the part where the user is told that he can use ./- instead.
builtin/reset.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..88ce0c5 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = HEAD;
unsigned char unused[20];
+ int file_named_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -) !argv[1]) {
+ if (!check_filename(prefix, -))
+ argv[0] = @{-1};
+ else
+ file_named_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -226,7 +233,13 @@ static void parse_args(struct pathspec *pathspec,
rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[0], 1);
+ if (file_named_minus) {
+ die(_(ambiguous argument '-': both revision
and filename\n
+ Use '--' to separate paths from
revisions, like this:\n
+ 'git command [revision...] --
[file...]'));
+ }
+ else
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v2 PATCH 2/2] reset: add tests for git reset -
On Tue, Mar 10, 2015 at 11:38 AM, Sundararajan R dyou...@gmail.com wrote:
reset: add tests for git reset -
Since this patch is changing the tests rather than 'reset' itself,
you'd likely want to say:
t7102: add 'reset -' tests
The failure case which occurs on teaching git is taught the '-' shorthand
is when there exists no branch pointed to by '@{-1}'.
ECANNOTPARSE
The ambiguous cases occur when there exist files named '-' or '@{-1}' in
the work tree. These are also treated as failure cases but here the user
is given advice as to how he can proceed.
Add tests to check the handling of these cases.
Also add a test to verify that reset - behaves like reset @{-1} when none
of the above cases are true.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Torsten Bögershausen tbo...@web.de
Torsten already pointed out this botch.
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sundararajan R dyou...@gmail.com
---
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..c05dab0 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,94 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no @{-1} should fail' '
+ git init new
+ (
+ cd new
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep unknown revision new/actual
Broken -chain here and throughout the patch.
+ test_when_finished rm -rf new
If one of the statements in the test before this point fails, then
test_when_finished() will never be invoked, which means that the rm
-rf new cleanup action will never be run. Here, and throughout the
patch, you need to invoke test_when_finished() at the earliest point
possible so that the cleanup is effective even if some other part of
the test fails. In this case, register the cleanup either just before
or just after git-init.
+'
+
+test_expect_success 'reset - with @{-1} and no file named - or @{-1} should
succeed' '
+ git init new
+ (
+ cd new
+ echo Hey new_file
+ git add new_file
+ git commit -m first_commit
+ git checkout -b new_branch
+ new_file
+ git add new_file
+ git reset -
+ git status -uno file1
+ git add new_file
+ git reset @{-1}
+ git status -uno file2
+ )
+ test_cmp new/file1 new/file2
Broken -chain.
+ test_when_finished rm -rf new
+'
+
test_done
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
On Wed, Mar 11, 2015 at 2:49 AM, Paul Tan pyoka...@gmail.com wrote: t0302 now tests git-credential-store's support for the XDG user-specific configuration file $XDG_CONFIG_HOME/git/credentials. Specifically: * Ensure that the XDG file is strictly opt-in. It should not be created by git at all times if it does not exist. * On the flip side, if the XDG file exists, ~/.git-credentials should not be created at all times. * If both the XDG file and ~/.git-credentials exists, then both files should be used for credential lookups. However, credentials should only be written to ~/.git-credentials. Regarding the final sentence: I don't see a test corresponding to the restriction that only ~/.git-credentials will be modified if both files exist. Am I missing something? More below. * Credentials must be erased from both files. * $XDG_CONFIG_HOME can be a custom directory set by the user as per the XDG base directory specification. Test that git-credential-store respects that, but defaults to ~/.config/git/credentials if it does not exist or is empty. Helped-by: Matthieu Moy matthieu@grenoble-inp.fr Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: Paul Tan pyoka...@gmail.com --- diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh index f61b40c..34752ea 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -6,4 +6,111 @@ test_description='credential-store tests' helper_test store +test_expect_success '~/.git-credentials is written to when xdg credentials file does not exist' ' + test -s $HOME/.git-credentials +' + +test_expect_success 'xdg credentials file will not be created if it does not exist' ' + test_path_is_missing $HOME/.config/git/credentials Since these two tests are complementary, each checking a facet of the same behavioral rule, it seems like they ought to be combined. For people reading the tests, having them separate implies incorrectly that they are unrelated, making it difficult to understand the overall picture of how the pieces relate to one another. In prose, you would describe the behavior as: When XDG credentials does not exist, write only to ~/.git-credentials; do not create XDG credentials. It's one thought; easy to read and understand. The test should reflect the same intent: test_expect_success '...' ' test -s $HOME/.git-credentials test_path_is_missing $HOME/.config/git/credentials ' The same observation applies to several other tests below. +' + +test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' ' + test_might_fail rm $HOME/.git-credentials Can this just be rm -f $HOME/.git-credentials instead? + mkdir -p $HOME/.config/git + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'xdg credentials file will be written to if it exists' ' + test -s $HOME/.config/git/credentials +' + +test_expect_success '~/.git-credentials will not be created if xdg credentials file exists' ' + test_path_is_missing $HOME/.git-credentials Ditto: It seems like these two tests should be combined. +' + +test_expect_success 'set up custom XDG_CONFIG_HOME credential file' ' + XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME + mkdir -p $XDG_CONFIG_HOME/git + $XDG_CONFIG_HOME/git/credentials + $HOME/.config/git/credentials +' + +helper_test store + +test_expect_success 'custom XDG_CONFIG_HOME credentials file will be written to if it exists' ' + test_when_finished unset XDG_CONFIG_HOME + test -s $XDG_CONFIG_HOME/git/credentials +' It feels wrong to set global state (XDG_CONFIG_HOME) in one test and then clear it later in another test, and it's not obvious to the casual reader that global state is being modified. An alternative would be to set XDG_CONFIG_HOME outside of the first test to which it applies, clear it after the final test. In reality, however, it only needs to be defined for the helper_test store invocation, so it also could be highly localized: XDG_CONFIG_HOME=$HOME/xdg export XDG_CONFIG_HOME helper_test store unset XDG_CONFIG_HOME A final alternative would be to wrap the block of tests needing XDG_CONFIG_HOME within a subshell and set the variable only within the subshell. Then, there's no need to unset it, and it's clear to the reader that only the tests within the subshell are affected by it. + +test_expect_success '~/.config/git/credentials file will not be written to if a custom XDG_CONFIG_HOME is set' ' + test_must_be_empty $HOME/.config/git/credentials +' + +test_expect_success '~/.git-credentials will not be created if xdg credentials file exists' ' + test_path_is_missing $HOME/.git-credentials For clarity, the three above tests probably ought to be combined. +' + +test_expect_success 'get: return credentials from home file if matches are
Re: [PATCH] config.txt: stick to CamelCase naming convention
On Tue, Mar 10, 2015 at 6:39 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- diff --git a/Documentation/config.txt b/Documentation/config.txt index 1530255..b4cc577 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -257,20 +257,20 @@ core.protectNTFS:: 8.3 short names. Defaults to `true` on Windows, and `false` elsewhere. -core.trustctime:: +core.trustCTime:: I personally find this a bit odd. 'ctime' is always typed exactly like that; never 'cTime' or 'Ctime' or 'CTime'. If false, the ctime differences between the index and the working tree are ignored; useful when the inode change time is regularly modified by something outside Git (file system crawlers and some backup systems). See linkgit:git-update-index[1]. True by default. @@ -290,7 +290,7 @@ core.eol:: linkgit:gitattributes[5] for more information on end-of-line conversion. -core.safecrlf:: +core.safeCRLF:: Here, you uppercase the entire acronym, CRLF... If true, makes Git check if converting `CRLF` is reversible when end-of-line conversion is active. Git will verify if a command modifies a file in the work tree either directly or indirectly. @@ -1281,11 +1281,11 @@ gitcvs.enabled:: Path to a log file where the CVS server interface well... logs various stuff. See linkgit:git-cvsserver[1]. -gitcvs.usecrlfattr:: +gitcvs.useCrlfAttr:: But, here you only capitalized it: Crlf. If true, the server will look up the end-of-line conversion attributes for files to determine the '-k' modes to use. If the attributes force Git to treat a file as text, @@ -1403,39 +1403,39 @@ gui.encoding:: true if linkgit:git-gui[1] should prune remote-tracking branches when performing a fetch. The default value is false. -gui.trustmtime:: +gui.trustMTime:: Ditto comment regarding strangeness of seeing 'mtime' typed any way other than 'mtime'. Determines if linkgit:git-gui[1] should trust the file modification timestamp or not. By default the timestamps are not trusted. Specifies the threshold to use in 'git gui blame' original location detection, measured in alphanumeric characters. See the linkgit:git-blame[1] manual for more information on copy detection. -gui.blamehistoryctx:: +gui.blameHistoryCTX:: You've uppercased acronyms (such as CRLF, URL, SSL), however, ctx is merely an abbreviation of context, not an acronym. As such, Ctx seems more correct. Specifies the radius of history context in days to show in linkgit:gitk[1] for the selected commit, when the `Show History Context` menu item is invoked from 'git gui blame'. If this @@ -2308,14 +2308,14 @@ sendemail.identity:: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl (deprecated):: +sendemail.smtpSSL (deprecated):: Here, SSL... Deprecated alias for 'sendemail.smtpencryption = ssl'. -sendemail.smtpsslcertpath:: +sendemail.smtpSslCertPath:: But here, inconsistently, Ssl. Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. -- To unsubscribe from this list: send the line 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 v3 1/2] reset: enable '-' short-hand for previous branch
On Tue, Mar 10, 2015 at 6:52 AM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
'git reset -' will reset to the previous branch. It will behave similar
to @{-1} except when a file named '@{-1}' is present. To refer to a file
named '-', use ./- or the -- flag.
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..8abd300 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
It's somewhat ugly to modify an element of argv[] since you don't own
the array and the contract does not particularly suggest that is
modifiable by you. A more serious concern is that doing so creates a
tighter binding between parse_args() and its caller since parse_args()
doesn't know how the caller will be disposing of argv[] when finished
with it. Say, for instance, that the caller disposes of each argv[]
element by invoking free(argv[n]). The literal @{-1} you've assigned
here is not allocated on the heap, so free()ing it would be wrong.
However, some of the other commands which alias - to @{-1} also
modify argv[] in this way, so we'll let it slide for the moment.
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,15 +227,21 @@ static void parse_args(struct pathspec *pathspec,
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
+ if (substituted_minus)
+ argv[0] = -;
verify_non_filename(prefix, argv[0]);
+ if (substituted_minus)
+ argv[0] = @{-1};
Do you find it ugly that you have to undo and then redo your argv[]
change from a few lines above? Rather than jumping through such hoops,
can't you instead define a new variable which holds the appropriate
value (@{-1}), or rework the logic to avoid such kludges altogether?
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
Mentioned by Matthieu: Don't sneak in unrelated changes. This change
was probably unintentional, but serves as a good reminder that you
should review the patch yourself before sending it. (And, if you do
have a need to make cleanups, it's typically best to do so as a
separate preparatory patch.)
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.279.gd534259
--
To unsubscribe from this list: send the line 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 v3 2/2] Added tests for reset -
On Tue, Mar 10, 2015 at 6:52 AM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
Added tests for reset -
Mention the area of the project you are changing, followed by a colon,
followed by a short summary of the change. Drop capitalization. Write
in imperative mood.
t7102: add 'reset -' tests
Added the following test cases:
Imperative mood: Add test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..0faf241 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,143 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - in the presence of file named - with previous
branch' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ echo M 1 expect
Mentioned by Matthieu: Use cat -\EOF.
+ git init no_previous
+ (
+ cd no_previous
+ ./-
Unnecessarily complex ./- when - will suffice.
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - ../output
Since you will be comparing this output with expected output, it is
customary to name this file actual.
+ )
+ rm -rf no_previous
Mentioned by Matthieu: Use test_when_finished(). You want this cleanup
action invoked even if any part of the test fails, so register it as
early as possible for it to be effective; either just before or after
git-init.
+ test_cmp output expect
When test_cmp() detects a difference in the expected and actual
output, it dumps the difference (in 'diff' format) as debugging
output. It's easier to read 'diff' output, and matches our
expectations more closely, if 'expect' is mentioned before 'actual',
so:
test_cmp expect actual
+'
+test_expect_success 'reset - in the presence of file named - with -- file
option' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset -- - ../output
+ )
+ rm -rf no_previous
Broken -chain.
+ test_cmp output expect
+'
+test_expect_success 'reset - in the presence of file named - with both pre
and post -- option' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - -- - ../output
+ )
+ rm -rf no_previous
Broken -chain.
+ test_cmp output expect
+'
+
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous
+ (
+ cd no_previous
+ echo random random
+ git add random
+ git commit -m base commit
+ git checkout -b temp
+ echo new-file new-file
+ git add new-file
+ git commit -m added new-file
+ git reset -
+ git status --porcelain ../first
+ git add new-file
+ git commit -m added new-file
+ git reset @{-1}
+ git status --porcelain ../second
+ )
In other tests, you performed rm -rf no_previous cleanup at this
point, but it's missing here.
+ test_cmp first second
+'
+
+test_expect_success 'reset - with file named @{-1}' '
+ echo Unstaged changes after reset: expect
+ echo M @{-1} expect
+ git init no_previous
+ (
+ cd no_previous
+ echo random ./@{-1}
+ git add ./@{-1}
+ git commit -m base commit
+ git checkout -b new_branch
+ echo additional stuff ./@{-1}
+ git add ./@{-1}
+ git reset - ../output
+ )
+ rm -rf
Re: [PATCH v3 3/4] docs/git-credential-store: document XDG file and precedence
On Wed, Mar 11, 2015 at 2:49 AM, Paul Tan pyoka...@gmail.com wrote: git-credential-store now supports an additional default credential file at $XDG_CONFIG_HOME/git/credentials. However, ~/.git-credentials takes precedence over it for backwards compatibility. To make the precedence ordering explicit, add a new section FILES that lists out the credential file paths in their order of precedence, and explains how the ordering affects the lookup, storage and erase operations. Also update documentation for --store to briefly explain the operations on multiple files if the --store option is not provided. Signed-off-by: Paul Tan pyoka...@gmail.com --- diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt index bc97071..451c4fa 100644 --- a/Documentation/git-credential-store.txt +++ b/Documentation/git-credential-store.txt @@ -31,10 +31,43 @@ OPTIONS +[[FILES]] +FILES +- + +If not set explicitly with '--file', there are two files where +git-credential-store will search for credentials in order of precedence: + +~/.git-credentials:: + User-specific credentials file. + +$XDG_CONFIG_HOME/git/credentials:: + Second user-specific credentials file. If '$XDG_CONFIG_HOME' is not set + or empty, `$HOME/.config/git/credentials` will be used. Any credentials + stored in this file will not be used if `~/.git-credentials` has a + matching credential as well. It is a good idea not to create this file + if you sometimes use older versions of Git, as support for this file + was added fairly recently. The final sentence won't age well: fairly recently is too nebulous. It may be sufficient merely to advise the reader to avoid this file if she also uses an older version of Git which doesn't support XDG for credentials. Other than this minor point, the patch series seems well prepared and quite nicely done. Thanks. +For credential lookups, the files are read in the order given above, with the +first matching credential found taking precedence over credentials found in +files further down the list. + +Credential storage will per default write to the first existing file in the +list. If none of these files exist, `~/.git-credentials` will be created and +written to. + +When erasing credentials, matching credentials will be erased from all files. EXAMPLES -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/7] t5541: move run_with_cmdline_limit to test-lib.sh
On Fri, Mar 13, 2015 at 12:53 AM, Jeff King p...@peff.net wrote:
We use this to test http pushing with a restricted
commandline. Other scripts (like t5551, which does http
fetching) will want to use it, too.
Signed-off-by: Jeff King p...@peff.net
---
As we discussed a while ago, this is the exact same code that
run_with_limited_stack uses in t7004. However, I think they are
conceptually two different things, and I could imagine a platform where
they have distinct implementations. So I did not refactor t7004 to make
use of this.
Perhaps this commentary should be moved to the commit message so that
the next person who notices that run_with_limited_stack() is the same
will understand why it was left alone.
t/t5541-http-push-smart.sh | 6 --
t/test-lib.sh | 6 ++
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index d2c681e..1ecb588 100755
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -324,12 +324,6 @@ test_expect_success 'push into half-auth-complete
requires password' '
test_cmp expect actual
'
-run_with_limited_cmdline () {
- (ulimit -s 128 $@)
-}
-
-test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
-
test_expect_success CMDLINE_LIMIT 'push 2000 tags over http' '
sha1=$(git rev-parse HEAD)
test_seq 2000 |
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7dd4b4d..9914d3e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1062,3 +1062,9 @@ test_lazy_prereq UNZIP '
$GIT_UNZIP -v
test $? -ne 127
'
+
+run_with_limited_cmdline () {
+ (ulimit -s 128 $@)
+}
+
+test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
--
2.3.2.472.geadab3c
--
To unsubscribe from this list: send the line 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 v3 1/2] reset: enable '-' short-hand for previous branch
On Tue, Mar 10, 2015 at 6:03 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
[PATCH v3 1/2] reset: enable '-' short-hand for previous branch
This should be v4, I think, not v3.
git reset -' will reset to the previous branch. It will behave similar
to @{-1} except when a file named '@{-1}' is present.
The way this is phrased, the reader is left hanging: What happens
when a file named @{-1} is present?
To refer to a file named '-', use ./- or the -- flag.
A documentation update to reflect this change would be appropriate.
See, for instance, 696acf45 (checkout: implement - abbreviation, add
docs and tests; 2009-01-17).
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric, I have added a user_input variable to record the input entered
by the user. This way I can avoid the multiple 'if' clauses. Thank you
for the suggestion.
I have also removed the unrelated change that I had unintentionally
committed. I am sending this patch on the thread for further review.
Once both the patches are reviewed and accepted, I will create a new
mail for it. Hope that is okay.
Please wrap commentary to about 72 columns, just as you would the
commit message, rather than typing it all on a single line. (I wrapped
it manually here in order to reply to it.)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..b428241 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = HEAD;
unsigned char unused[20];
+ int substituted_minus = 0;
+ char *user_input = argv[0];
You're assigning a 'const char *' to a 'char *'. The compiler should
have warned about it.
This variable name is not as expressive as it could be. Try to name
the variable after what you expect it to represent, for instance
maybe_rev or rev_or_path or something more suitable. Even
orig_argv0 would be more informative than user_input.
/*
* Possible arguments are:
*
@@ -205,6 +207,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,9 +228,12 @@ static void parse_args(struct pathspec *pathspec,
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[0]);
+ verify_non_filename(prefix, user_input);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
In my review of the previous version[1], I mentioned that it was ugly
to muck with argv[0]; moreover that it was particularly ugly to have
to muck with it multiple times, undoing and redoing assignments.
Although you've eliminated some reassignments via 'user_input', my
intent, by asking if you could rework the logic, was to prompt you to
take a couple steps back and examine the overall logic of the function
more closely. The munging of argv[0] is effectively a fragile and
undesirable band-aid approach. It is possible to support '-' as an
alias for '@{-1}' without having to resort to such kludges at all.
One other concern: When there is no previous branch, and no file named
-, then 'git reset -' misleadingly complains bad flag '-' used
after filename, rather than the more appropriate ambiguous argument
'-': unknown revision or path.
[1]: http://article.gmane.org/gmane.comp.version-control.git/265255
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
--
2.3.1.278.ge5c7b1f.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] t7102: add 'reset -' tests
On Tue, Mar 10, 2015 at 6:10 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
Add following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option is
specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Does it concern you that all new tests pass except the last one even
when patch 1/2 is not applied? I would have expected most or all of
these tests to fail without patch 1/2.
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..d3a5874 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,163 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch fails' '
+ git init no_previous
I understand the intention of the name no_previous in tests for
which there is no @{-1}, however...
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep bad flag no_previous/actual
+'
+
+test_expect_success 'reset - while having file named - and no previous
branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep bad flag no_previous/actual
+'
+
+test_expect_success \
+ 'reset - in the presence of file named - with previous branch resets
commit' '
+ cat expect -\EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init no_previous
I don't understand the name no_previous in tests for which @{-1} can
resolve. It probably would be better to choose a more generic name and
use it for all these tests. For instance, resetdash or just dash
or something better.
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - with file named @{-1} succeeds' '
I may be missing something obvious, but why is it necessary to single
out a file named @{-1} at all, particuarly when there are so many
other ways to specify revisions, and there may be files mirroring
those spellings, as well?
+ cat expect EOF
+ Unstaged changes after reset:
+ M file
+ M @{-1}
+ EOF
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ echo random @{-1}
+ echo random file
+ git add @{-1} file
+ git commit -m base commit
+ git checkout -b new_branch
+ echo additional stuff file
+ echo additional stuff @{-1}
+ git add file @{-1}
+ git reset - ../actual
+ )
+ test_cmp expect actual
+'
+
test_done
--
2.3.1.278.ge5c7b1f.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Mar 2015, #04; Wed, 11)
On Wed, Mar 11, 2015 at 8:33 PM, Junio C Hamano gits...@pobox.com wrote: * nd/list-files (2015-02-09) 21 commits - t3080: tests for git-list-files - list-files: -M aka diff-cached - list-files -F: show submodules with the new indicator '' - list-files: add -F/--classify - list-files: show directories as well as files - list-files: do not show duplicate cached entries - list-files: sort output and remove duplicates - list-files: add -t back - list-files: add -1 short for --no-column - list-files: add -R/--recursive short for --max-depth=-1 - list-files: -u does not imply showing stages - list-files: make alias 'ls' default to 'list-files' - list-files: a user friendly version of ls-files and more - ls-files: support --max-depth - ls-files: add --column - ls-files: add --color to highlight file names - ls-files: buffer full item in strbuf before printing - ls_colors.c: highlight submodules like directories - ls_colors.c: add a function to color a file name - ls_colors.c: parse color.ls.* from config file - ls_colors.c: add $LS_COLORS parsing code A new git list-files Porcelain command, ls-files with bells and whistles. Concern was raised that this is piggybacking on ls-files codebase, rather than wt-status codebase ($gmane/264258). Waiting for further comments or a reroll. This series has been re-rolled[1]. [1]: http://thread.gmane.org/gmane.comp.version-control.git/265142 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Adding - shorthand for @{-1} in RESET command
On Mon, Mar 9, 2015 at 4:46 PM, Sundararajan R dyou...@gmail.com wrote:
Please give feedback and suggest things I may have missed out on.
I hope I have incorporated all the suggestions.
If you haven't already, read Documentation/SubmittingPatches. Pay
particular attention to section #2 which explains how to write a good
commit message, and to section #4 to learn where to place patch
commentary not intended as part of the permanent commit history. The
above lines are commentary, not meant as part of the commit message.
Place such commentary below the '---' line just before the diffstat.
Subject: Adding - shorthand for @{-1} in RESET command
Prefix the first line of the commit message with the module or command
you re changing. Drop capitalization. Write in imperative mood. For
instance:
reset: add '-' shorthand for '@{-1}'
Signed-off-by: Sundararajan R dyou...@gmail.com
Thanks-to: Junio C Hamano
Place your sign-off last. Use Helped-by: rather than Thanks-to: and
include the person's full name and email address.
---
Here, just below the '---' line is where you should place commentary
not intended for the permanent commit record.
I have attempted to resolve the ambiguity when there exists a file named -
by communicating to the user that he/she can use ./- when he/she wants to
refer
to the - file. I perform this check using the check_filename() function.
This is important information for the commit message itself above the
'---' line, though you would want to rephrase it just to state the
facts. No need to mention I did this or I did that since the patch
itself implies that you made the changes.
builtin/reset.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..2bdd5cd 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,
{
const char *rev = HEAD;
unsigned char unused[20];
+ int file_named_minus=0;
Style: Here and elsewhere, add a space around '='.
/*
* Possible arguments are:
*
@@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -) !argv[1]) {
+ if(!check_filename(prefix,-))
Style: Here and elsewhere, add space after 'if'.
Style: Add space after comma.
+ argv[0]=@{-1};
+ else
+ file_named_minus=1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -226,7 +233,14 @@ static void parse_args(struct pathspec *pathspec,
rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[0], 1);
+ if(file_named_minus) {
+ die(_(ambiguous argument '-': both revision
and filename\n
+ Use ./- for file named -\n
+ Use '--' to separate paths from
revisions, like this:\n
+ 'git command [revision...] --
[file...]'));
This seems odd. If arguments following '--' are unconditionally
treated as paths, why is it be necessary to tell the user to spell out
file '-' as './-'? Shouldn't git reset -- - be sufficient?
+ }
+ else
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: Add CCs from additional commit tags
On Fri, Mar 6, 2015 at 4:59 PM, Soren Brinkmann
soren.brinkm...@xilinx.com wrote:
Add email addresses from additional commonly used tags to the CC-list of
patches. Additional tags are:
- Acked-by
- Reviewed-by
- Tested-by
- Reported-by
- Reviewed-and-tested-by
--suppress-cc=ack suppresses these additional CCs.
This and similar suggestions have come up a number of times. Rather
than hard-coding an ever-growing list of tags, general consensus seems
to be that it would be better to provide some sort of mechanism for
people to customize the list for their needs. See, for instance, [1].
Such ability would also be a better fit for non-standard, potentially
contested tags, such as Reviewed-and-tested-by:.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/233003/focus=233739
Signed-off-by: Soren Brinkmann soren.brinkm...@xilinx.com
---
Documentation/git-send-email.txt | 3 +++
git-send-email.perl | 5 +++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-send-email.txt
b/Documentation/git-send-email.txt
index f248a8665e1f..1b521446ca11 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -293,6 +293,9 @@ Automating
patch body (commit message) except for self (use 'self' for that).
- 'sob' will avoid including anyone mentioned in Signed-off-by lines except
for self (use 'self' for that).
+- 'ack' will avoid including anyone who acked the patch (mentioned in
+ Acked-by, Reviewed-by, Tested-by, Reviewed-and-tested-by lines except for
+ self (use 'self' for that).
- 'cccmd' will avoid running the --cc-cmd.
- 'body' is equivalent to 'sob' + 'bodycc'
- 'all' will suppress all auto cc values.
diff --git a/git-send-email.perl b/git-send-email.perl
index 3092ab356c76..18eb8a5139a4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -407,7 +407,7 @@ my(%suppress_cc);
if (@suppress_cc) {
foreach my $entry (@suppress_cc) {
die Unknown --suppress-cc field: '$entry'\n
- unless $entry =~
/^(?:all|cccmd|cc|author|self|sob|body|bodycc)$/;
+ unless $entry =~
/^(?:all|cccmd|cc|author|self|sob|body|bodycc|ack)$/;
$suppress_cc{$entry} = 1;
}
}
@@ -1452,7 +1452,7 @@ foreach my $t (@files) {
# Now parse the message body
while($fh) {
$message .= $_;
- if (/^(Signed-off-by|Cc): (.*)$/i) {
+ if
(/^(Signed-off-by|Cc|Acked-by|Reviewed-by|Tested-by|Reported-by|Reviewed-and-tested-by):
(.*)$/i) {
chomp;
my ($what, $c) = ($1, $2);
chomp $c;
@@ -1462,6 +1462,7 @@ foreach my $t (@files) {
} else {
next if $suppress_cc{'sob'} and $what =~
/Signed-off-by/i;
next if $suppress_cc{'bodycc'} and $what =~
/Cc/i;
+ next if $suppress_cc{'ack'} and $what =~
/(Acked-by|Reviewed-by|Tested-by|Reported-by|Reviewed-and-tested-by)/i;
}
push @cc, $c;
printf((body) Adding cc: %s from line '%s'\n,
--
2.3.1.2.g90df61e.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
