[PATCH v5 1/2] refs.c: optimize check_refname_component()
In a repository with many refs, check_refname_component can be a major
contributor to the runtime of some git commands. One such command is
git rev-parse HEAD
Timings for one particular repo, with about 60k refs, almost all
packed, are:
Old: 35 ms
New: 29 ms
Many other commands which read refs are also sped up.
Signed-off-by: David Turner dtur...@twitter.com
---
refs.c | 67 +++---
t/t5511-refspec.sh | 6 -
2 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/refs.c b/refs.c
index 28d5eca..dd28f2a 100644
--- a/refs.c
+++ b/refs.c
@@ -6,8 +6,29 @@
#include string-list.h
/*
- * Make sure ref is something reasonable to have under .git/refs/;
- * We do not like it if:
+ * How to handle various characters in refnames:
+ * 0: An acceptable character for refs
+ * 1: End-of-component
+ * 2: ., look for a preceding . to reject .. in refs
+ * 3: {, look for a preceding @ to reject @{ in refs
+ * 4: A bad character: ASCII control characters, ~, ^, : or SP
+ */
+static unsigned char refname_disposition[256] = {
+ 1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
+ 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 2, 1,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
+};
+
+/*
+ * Try to read one refname component from the front of refname.
+ * Return the length of the component found, or -1 if the component is
+ * not legal. It is legal if it is something reasonable to have under
+ * .git/refs/; We do not like it if:
*
* - any path component of it begins with ., or
* - it has double dots .., or
@@ -16,41 +37,31 @@
* - it ends with .lock
* - it contains a \ (backslash)
*/
-
-/* Return true iff ch is not allowed in reference names. */
-static inline int bad_ref_char(int ch)
-{
- if (((unsigned) ch) = ' ' || ch == 0x7f ||
- ch == '~' || ch == '^' || ch == ':' || ch == '\\')
- return 1;
- /* 2.13 Pattern Matching Notation */
- if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
- return 1;
- return 0;
-}
-
-/*
- * Try to read one refname component from the front of refname. Return
- * the length of the component found, or -1 if the component is not
- * legal.
- */
static int check_refname_component(const char *refname, int flags)
{
const char *cp;
char last = '\0';
for (cp = refname; ; cp++) {
- char ch = *cp;
- if (ch == '\0' || ch == '/')
+ unsigned char ch = (unsigned char) *cp;
+ unsigned char disp = refname_disposition[ch];
+ switch(disp) {
+ case 1:
+ goto out;
+ case 2:
+ if (last == '.')
+ return -1; /* Refname contains ... */
+ break;
+ case 3:
+ if (last == '@')
+ return -1; /* Refname contains @{. */
break;
- if (bad_ref_char(ch))
- return -1; /* Illegal character in refname. */
- if (last == '.' ch == '.')
- return -1; /* Refname contains ... */
- if (last == '@' ch == '{')
- return -1; /* Refname contains @{. */
+ case 4:
+ return -1;
+ }
last = ch;
}
+out:
if (cp == refname)
return 0; /* Component has zero length. */
if (refname[0] == '.') {
diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index c289322..1571176 100755
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -5,7 +5,6 @@ test_description='refspec parsing'
. ./test-lib.sh
test_refspec () {
-
kind=$1 refspec=$2 expect=$3
git config remote.frotz.url .
git config --remove-section remote.frotz
@@ -84,4 +83,9 @@ test_refspec push
'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*'
test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*'
+good=$(echo -n '\0377')
+test_refspec fetch refs/heads/${good}
+bad=$(echo -n '\011')
+test_refspec fetch refs/heads/${bad} invalid
+
test_done
--
2.0.0.rc1.18.gf763c0f
--
To unsubscribe from this list: send the line 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 v5 2/2] refs.c: SSE4.2 optimizations for check_refname_component
Optimize check_refname_component using SSE4.2, where available.
git rev-parse HEAD is a good test-case for this, since it does almost
nothing except parse refs. For one particular repo with about 60k
refs, almost all packed, the timings are:
Look up table: 29 ms
SSE4.2:25 ms
This is about a 15% improvement.
The configure.ac changes include code from the GNU C Library written
by Joseph S. Myers joseph at codesourcery dot com.
Signed-off-by: David Turner dtur...@twitter.com
---
Makefile | 6 +++
aclocal.m4 | 6 +++
configure.ac | 17
git-compat-util.h | 20 +
refs.c | 116 +
t/t5511-refspec.sh | 13 ++
6 files changed, 161 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index a53f3a8..dd2127a 100644
--- a/Makefile
+++ b/Makefile
@@ -1326,6 +1326,11 @@ else
COMPAT_OBJS += compat/win32mmap.o
endif
endif
+ifdef NO_SSE42
+ BASIC_CFLAGS += -DNO_SSE42
+else
+ BASIC_CFLAGS += -msse4.2
+endif
ifdef OBJECT_CREATION_USES_RENAMES
COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1
endif
@@ -2199,6 +2204,7 @@ GIT-BUILD-OPTIONS: FORCE
@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@
@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@
@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst
','\'',$(NO_UNIX_SOCKETS)))'\' $@
+ @echo NO_SSE42=\''$(subst ','\'',$(subst ','\'',$(NO_SSE42)))'\' $@
ifdef TEST_OUTPUT_DIRECTORY
@echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst
','\'',$(TEST_OUTPUT_DIRECTORY)))'\' $@
endif
diff --git a/aclocal.m4 b/aclocal.m4
index f11bc7e..d9f3f19 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -38,3 +38,9 @@ AC_DEFUN([TYPE_SOCKLEN_T],
[#include sys/types.h
#include sys/socket.h])
])
+
+dnl Test a compiler option or options with an empty input file.
+dnl LIBC_TRY_CC_OPTION([options], [action-if-true], [action-if-false])
+AC_DEFUN([LIBC_TRY_CC_OPTION],
+[AS_IF([AC_TRY_COMMAND([${CC-cc} $1 -xc /dev/null -S -o /dev/null])],
+ [$2], [$3])])
diff --git a/configure.ac b/configure.ac
index b711254..3a5bda9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -382,6 +382,23 @@ AS_HELP_STRING([],[Tcl/Tk interpreter will be found in a
system.]),
GIT_PARSE_WITH(tcltk))
#
+# Declare the with-sse42/without-sse42 options.
+AC_ARG_WITH(sse42,
+AS_HELP_STRING([--with-sse42],[use SSE4.2 instructions])
+AS_HELP_STRING([],[(default is YES if your compiler supports -msse4.2)]),
+GIT_PARSE_WITH(sse42))
+
+if test $NO_SSE42 != YesPlease; then
+ dnl Check if -msse4.2 works.
+ AC_CACHE_CHECK(for SSE4.2 support, cc_cv_sse42, [dnl
+ LIBC_TRY_CC_OPTION([-msse4.2], [cc_cv_sse42=yes], [cc_cv_sse42=no])
+ ])
+ if test $cc_cv_sse42 = no; then
+ NO_SSE42=1
+ fi
+fi
+
+GIT_CONF_SUBST([NO_SSE42])
## Checks for programs.
AC_MSG_NOTICE([CHECKS for programs])
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..254487a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -668,6 +668,26 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#endif
#endif
+#ifndef NO_SSE42
+#include nmmintrin.h
+/* Clang ships with a version of nmmintrin.h that's incomplete; if
+ * necessary, we define the constants that we're going to use. */
+#ifndef _SIDD_UBYTE_OPS
+#define _SIDD_UBYTE_OPS 0x00
+#define _SIDD_CMP_EQUAL_ANY 0x00
+#define _SIDD_CMP_RANGES0x04
+#define _SIDD_CMP_EQUAL_ORDERED 0x0c
+#define _SIDD_NEGATIVE_POLARITY 0x10
+#endif
+
+/* This is the system memory page size; it's used so that we can read
+ * outside the bounds of an allocation without segfaulting. It is
+ * assumed to be a power of 2. */
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+#endif
+
#ifdef UNRELIABLE_FSTAT
#define fstat_is_reliable() 0
#else
diff --git a/refs.c b/refs.c
index dd28f2a..22a2dae 100644
--- a/refs.c
+++ b/refs.c
@@ -24,6 +24,25 @@ static unsigned char refname_disposition[256] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
};
+static int check_refname_component_trailer(const char *cp, const char
*refname, int flags)
+{
+ if (cp == refname)
+ return 0; /* Component has zero length. */
+ if (refname[0] == '.') {
+ if (!(flags REFNAME_DOT_COMPONENT))
+ return -1; /* Component starts with '.'. */
+ /*
+* Even if leading dots are allowed, don't allow .
+* as a component (.. is prevented by a rule above).
+*/
+ if (refname[1] == '\0')
+ return -1; /* Component equals .. */
+ }
+ if (cp - refname = 5 !memcmp(cp - 5, .lock, 5))
+ return -1; /* Refname ends with .lock. */
+ return cp - refname;
+}
+
/*
* Try to read one refname component from the front of refname.
* Return the
Re: [PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell
On Fri, May 30, 2014 at 11:10:51PM +, Naumov, Michael (North Sydney) wrote: Some git tools such as GitExtensions for Windows use environment variable TERM=msys which causes the weird ANSI sequence shown for the messages returned from server-side hooks We add those ANSI sequences to help format sideband data on the user's terminal. However, GitExtensions is not using a terminal, and the ANSI sequences just confuses it. We can recognize this use by checking isatty(). See https://github.com/gitextensions/gitextensions/issues/1313 for more details NOTE: I considered to cover the case that a pager has already been started. But decided that is probably not worth worrying about here, though, as we shouldn't be using a pager for commands that do network communications (and if we do, omitting the magic line-clearing signal is probably a sane thing to do). Signed-off-by: Michael Naumov mnaou...@gmail.com Thanks-to: Erik Faye-Lund kusmab...@gmail.com Thanks-to: Jeff King p...@peff.net This version looks fine to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.0.0
On Sat, May 31, 2014 at 11:52:24AM +0200, David Kastrup wrote: Some mailing list filters and/or spam filters flag mails with too many recipients so that they need to pass through moderation first. The typical threads on this list are short and have few recipients while longer threads, due to the list policy of adding every participants to the Cc, will tend to have more recipients. AFAIK, vger does not do anything like this. They block HTML, messages lacking a message-id, messages over 100K, and certain taboo phrases: http://vger.kernel.org/majordomo-info.html#taboo And anyway, I do not think vger is responsible here. The messages were delivered through the list, and other archives have them. This looks like a gmane problem. According to gmane.org, their admins will look manually at messages flagged as spam, but I find it unlikely that they flagged several days worth of git traffic (and when they do, I think they cross-post them to a spam group in NNTP, and the messages do not seem to be marked as such). So I think this really is just a bug. And frankly, if I were a list moderator and software asked me through this sort of coincidence whether a mail should be delivered or not and a glance at it shows nothing but insults, wild accusations, threats and so on for the umpteenth time, I'd consider twice clicking Accept. Whether or not I ultimately did so, this would likely contribute to the delay. I do not disagree, but please let's not rehash all of that again. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Improve function dir.c:trim_trailing_spaces()
On Sat, May 31, 2014 at 08:21:31AM -0700, Pasha Bolokhov wrote:
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ if (*p == ' ') {
+ if (!last_space)
+ last_space = p;
+ } else {
+ if (*p == '\\')
+ p++;
+ last_space = NULL;
+ }
Your backslash-escape works here by incrementing p an extra time. So
we move past the backslash to the next character (which is escaped), and
then the for-loop increments it again to the character beyond that,
which is the next one worth considering.
What happens if we are parsing a string with an unmatched backslash at
the end of the string, like:
foo\
We consider the end-of-string NUL to be escaped, skip it, and then keep
reading whatever random bytes are in memory after the string.
The original version did not have a problem with this because it used
a length, which meant that i len caught this case.
I think you either need to insert an extra check for !p[1] when moving
past the escaped character, or move back to a length-based check.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.0.0
Jeff King wrote: On Sat, May 31, 2014 at 11:52:24AM +0200, David Kastrup wrote: And frankly, if I were a list moderator and software asked me through this sort of coincidence whether a mail should be delivered or not and a glance at it shows nothing but insults, wild accusations, threats and so on for the umpteenth time, I'd consider twice clicking Accept. Whether or not I ultimately did so, this would likely contribute to the delay. I do not disagree, but please let's not rehash all of that again. FTR. I haven't insulted anybody, I on the other hand have been insulted plenty of times, included by Junio. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: feature request - implement a GIT_AUTHOR_EMAIL equivalent, but processed BEFORE .gitconfig
On Fri, May 30, 2014 at 02:35:37PM -0700, Junio C Hamano wrote: Nathan's installation can set a GIT_MYSELF and then have something like this in the shared $HOME/.gitconfig [include] path = /usr/local/users/$GIT_MYSELF/ident we could even make the whole thing fail when GIT_MYSELF is not set but I haven't thought things through ;-) Yeah, that is something I considered[1] when writing the initial include.path implementation. Something like: [include] path = .gitconfig-$HOSTNAME could be potentially useful. But I punted at the time to wait for somebody to actually ask for it. If somebody wanted to implement it, I don't see a reason to avoid it. -Peff [1] http://article.gmane.org/gmane.comp.version-control.git/190196 -- To unsubscribe from this list: send the line unsubscribe 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] request-pull: resurrect for-linus - tags/for-linus DWIM
Am 5/16/2014 19:57, schrieb Junio C Hamano: --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -223,7 +223,13 @@ test_expect_success 'pull request format' ' git request-pull initial $downstream_url tags/full:refs/tags/full ) request sed -nf fuzz.sed request request.fuzzy - test_i18ncmp expect request.fuzzy + test_i18ncmp expect request.fuzzy + + ( + cd local + git request-pull initial $downstream_url full + ) request + grep ' tags/full$' ' What's this crap? Here's a fix. Feel free to tame down the subject line if you think it's too strong ;) --- 8 --- From: Johannes Sixt j...@kdbg.org Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh The recent addition to the test case 'pull request format' interrupted the single-quoted text, effectively adding a third argument to the test_expect_success command. Since we do not have a prerequisite named pull request format, the test is skipped, no matter what. Additionally, the file name argument to the grep command is missing. Fix both issues. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t5150-request-pull.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index 93e2c65..82c33b8 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -229,7 +229,7 @@ test_expect_success 'pull request format' ' cd local git request-pull initial $downstream_url full ) request - grep ' tags/full$' + grep tags/full\$ request ' test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' ' -- 2.0.0.1326.g81a507a -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
-- Kedves felhasználók e-mailben; Túllépte 23432 box set Web Service / Admin, és akkor nem lesz probléma a küldő és fogadhat e-maileket, amíg újra ellenőrizni. Kérjük, frissítse kattintva linkre, és töltse ki az adatokat, hogy ellenőrizze a számla Kérjük, kövesse az alábbi linkre, és majd másolja és illessze be a böngésző jelölőnégyzetet. http://mailupdatety.jigsy.com/ Figyelem! Ha nem, csak korlátozott hozzáférést az e-mail postafiókját. ha frissíteni? számla frissül három napon belül Értesítés a számla véglegesen be kell zárni. Tisztelettel, rendszergazda -- To unsubscribe from this list: send the line unsubscribe 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] string-list.h: Add a value to string_list initializer lists
Tanay Abhra tanay...@gmail.com writes: Add a NULL value in the end of STRING_LIST_INIT_NODUP and STRING_LIST_DUP to initialize `compare_strings_fn`. Signed-off-by: Tanay Abhra tanay...@gmail.com --- When I used a malloced string_list to play around with string-list API and used the default init_list, it caused a seg fault. After an hour of debugging I saw that comapre_strings_fn should be initialized to NULL. In C, even an incomplete initialzer initializes every value to NULl or 0, so in normal usage in the codebase this problem never occurs. Still it is better to be thorough. Part of this comment should be in the commit message itself. The I used ... part should not (it is your experience), but the last two sentences make sense IMHO. Other than that, the changes sounds good. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] replace signal() with sigaction()
Am 6/1/2014 20:10, schrieb Jeremiah Mahler: This is version 3 of the patch set to convert signal(2) to sigaction(2) (previous discussion [1]). [1]: http://marc.info/?l=gitm=140148352416926w=2 Changes in this revision include: - Using NULL pointers instead of 0 as per the Documentation/CodingGuidlines pointed out by Chris Packham. sigaction(SIGCHLD, sa, NULL); - Conversion of all remaining files which used signal(). - sigchain.c required the most changes. Both the old signal handler was used and the return value from signal() was being checked. signal() would return the previous error handler which would be SIG_ERR if an error occurred. sigaction() just returns -1 in this case. Jeremiah Mahler (9): compat/mingw.c: expand MinGW support for sigaction connect.c: replace signal() with sigaction() progress.c: replace signal() with sigaction() write_or_die.c: replace signal() with sigaction() daemon.c: replace signal() with sigaction() builtin/log.c: replace signal() with sigaction() builtin/merge-index.c: replace signal() with sigaction() builtin/verify-tag.c: replace signal() with sigaction() sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. -- Hannes builtin/log.c | 6 +- builtin/merge-index.c | 5 - builtin/verify-tag.c | 5 - compat/mingw.c| 9 + connect.c | 5 - daemon.c | 16 +--- progress.c| 6 +- sigchain.c| 14 +++--- write_or_die.c| 6 +- 9 files changed, 56 insertions(+), 16 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config: Add documentation for writing config files
Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..df08385 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data) Writing Config Files -TODO +Git gives multiple entry points in the Config API to write config values to +files namely `git_config_set_in_file` and `git_config_set`, which write to +a specific config file or to `.git/config` respectively. They both take a +key/value pair as parameter. +In the end they both all call `git_config_set_multivar_in_file` which takes +four parameters: + +- the name of the file, as a string, to which key/value pairs will be written. + +- the name of key, as a string. This is in canonical flat form: the section, + subsection, and variable segments will be separated by dots, and the section + and variable segments will be all lowercase. + E.g., `core.ignorecase`, `diff.SomeType.textconv`. + +- the value of the variable, as a string. If value is equal to NULL, it will + remove the matching key from the config file. + +- the value regex, as a string. It will disregard key/value pairs where value + does not match. + +- a multi_replace value, as an int. If value is equal to zero, nothing or only + one matching key/value is replaced, else all matching key/values (regardless + how many) are removed, before the new pair is written. + +It returns 0 on success. + +Also, there are functions `git_config_rename_section` and +`git_config_rename_section_in_file` with parameters `old_name` and `new_name` +for renaming or removing sections in the config files. If NULL is passed +through `new_name` parameter, the section will be removed from the config file. -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe 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 0/9] replace signal() with sigaction()
Hannes, On Mon, Jun 02, 2014 at 01:28:25PM +0200, Johannes Sixt wrote: Am 6/1/2014 20:10, schrieb Jeremiah Mahler: This is version 3 of the patch set to convert signal(2) to sigaction(2) (previous discussion [1]). ... sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. -- Hannes You are right, I missed the case where the old signal was used, as is done in sigchain.c. Sorry about that. Thanks again for looking at my patch. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 0/2] Git config cache special querying api utilizing the cache
Hi, [V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen. Added cache invalidation when config file is changed. I am using git_config_set_multivar_in_file() as an update hook. This is my first patch series for this year's GSoC. My project is Git Config API improvements. The link of my proposal is appended below [1]. The aim of this patch series is to generate a cache for querying values from the config files in a non-callback manner as the current method reads and parses the config files every time a value is queried for. The cache is generated from hooking the update_cache function to the current parsing and callback mechanism in config.c. It is implemented as an hashmap using the hashmap-api with variables and its corresponding values list as its members. The values in the list are sorted in order of increasing priority. The cache is initialised in git_config_early() as it is the first time a `git_config` function is called during program startup. setup_git_directory_gently() calls git_config_early() which in turn reads every config file (local, user and global config files). get_value() in config.c feeds variable and values into the callback function. Using this function as a hook, we update the cache. Also, we add two new functions to the config-api git_config_get_string() and git_config_get_string_multi() for querying in a non callback manner from the cache. I have run the tests and debug the code and it works, but I have to add a few things, 1. Metadata about the variables and values. I have added only the file from each variable value pair comes in an upcoming series. What else should I add or implement ;is my approach right? [1] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing Cheers, Tanay Abhra. Tanay Abhra (2): config: Add hashtable for config parsing retrieval config: Add new query functions to the api docs Documentation/technical/api-config.txt | 18 + cache.h| 2 + config.c | 118 + 3 files changed, 138 insertions(+) -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 2/2] config: Add new query functions to the api docs
Add explanations for `git_config_get_string_multi` and `git_config_get_string` which utilize the config hash table for querying in a non-callback manner for a specific variable. Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..8f1a37e 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -77,6 +77,24 @@ To read a specific file in git-config format, use `git_config_from_file`. This takes the same callback and data parameters as `git_config`. +Querying For Specific Variables +--- + +For programs wanting to query for specific variables in a non-callback +manner, the config API provides two functions `git_config_get_string` +and `git_config_get_string_multi`. They both take a single parameter, + +- a key string in canonical flat form for which the corresponding values + will be retrieved and returned. + +They both read values from an internal cache generated previously from +reading the config files. `git_config_get_string` returns the value with +the highest priority(i.e. value in the repo config will be preferred over +value in user wide config for the same variable). + +`git_config_get_string_multi` returns a `string_list` containing all the +values for that particular key, sorted in order of increasing priority. + Value Parsing Helpers - -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 1/2] config: Add hashtable for config parsing retrieval
Add a hash table to cache all key-value pairs read from config files
(repo specific .git/config, user wide ~/.gitconfig and the global
/etc/gitconfig). Add two external functions `git_config_get_string` and
`git_config_get_string_multi` for querying in a non-callback manner from the
hash table.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
cache.h | 2 ++
config.c | 118 +++
2 files changed, 120 insertions(+)
diff --git a/cache.h b/cache.h
index 107ac61..2038150 100644
--- a/cache.h
+++ b/cache.h
@@ -1271,6 +1271,8 @@ extern int check_repository_format_version(const char
*var, const char *value, v
extern int git_env_bool(const char *, int);
extern int git_config_system(void);
extern int config_error_nonbool(const char *);
+extern const char *git_config_get_string(const char *);
+extern const struct string_list *git_config_get_string_multi(const char *);
#if defined(__GNUC__) ! defined(__clang__)
#define config_error_nonbool(s) (config_error_nonbool(s), -1)
#endif
diff --git a/config.c b/config.c
index a30cb5c..23c9403 100644
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
#include exec_cmd.h
#include strbuf.h
#include quote.h
+#include hashmap.h
+#include string-list.h
struct config_source {
struct config_source *prev;
@@ -37,6 +39,112 @@ static struct config_source *cf;
static int zlib_compression_seen;
+static struct hashmap config_cache;
+
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list *value_list;
+};
+
+static int hashmap_is_init = 0;
+
+static int config_cache_entry_cmp_icase(const struct config_cache_entry *e1,
+const struct config_cache_entry *e2, const
char* key)
+{
+ return strcasecmp(e1-key, key ? key : e2-key);
+}
+
+static void config_cache_init(void)
+{
+ hashmap_init(config_cache,
(hashmap_cmp_fn)config_cache_entry_cmp_icase, 0);
+}
+
+static void config_cache_free(void)
+{
+ struct config_cache_entry* entry;
+ struct hashmap_iter iter;
+ hashmap_iter_init(config_cache, iter);
+ while (entry = hashmap_iter_next(iter)) {
+ free(entry-key);
+ string_list_clear(entry-value_list, 0);
+ }
+ hashmap_free(config_cache, 1);
+}
+
+static struct config_cache_entry *config_cache_find_entry(const char *key)
+{
+ struct hashmap_entry k;
+ hashmap_entry_init(k, strihash(key));
+ return hashmap_get(config_cache, k, key);
+}
+
+static struct string_list *config_cache_get_value(const char *key)
+{
+ struct config_cache_entry *e = config_cache_find_entry(key);
+ return e ? e-value_list : NULL;
+}
+
+
+static void config_cache_set_value(const char *key, const char *value)
+{
+ struct config_cache_entry *e;
+
+ e = config_cache_find_entry(key);
+ if (!e) {
+ e = malloc(sizeof(*e));
+ hashmap_entry_init(e, strihash(key));
+ e-key=xstrdup(key);
+ e-value_list = xcalloc(sizeof(struct string_list), 1);
+ e-value_list-strdup_strings = 1;
+ string_list_append(e-value_list, value);
+ hashmap_add(config_cache, e);
+ } else {
+ if (!unsorted_string_list_has_string(e-value_list, value))
+ string_list_append(e-value_list, value);
+ }
+}
+
+static void config_cache_remove_value(const char *key, const char *value)
+{
+ struct config_cache_entry *e;
+ int i;
+
+ e = config_cache_find_entry(key);
+ if(!e)
+ return;
+ /* If value == NULL then remove all the entries for the key */
+ if(!value) {
+ string_list_clear(e-value_list, 0);
+ free(e-value_list);
+ hashmap_remove(config_cache, e, NULL);
+ return;
+ }
+ /* All the occurances of value will be deleted */
+ for (i = 0; i e-value_list-nr; i++)
+ if (!strcmp(value, e-value_list-items[i].string))
+ unsorted_string_list_delete_item(e-value_list, i, 0);
+ if(e-value_list-nr == 0) {
+ string_list_clear(e-value_list, 0);
+ free(e-value_list);
+ hashmap_remove(config_cache, e, NULL);
+ }
+}
+
+extern const char *git_config_get_string(const char *name)
+{
+ struct string_list *values;
+ values = config_cache_get_value(name);
+ if (!values)
+ return NULL;
+ return values-items[values-nr - 1].string;
+}
+
+extern const struct string_list *git_config_get_string_multi(const char *key)
+{
+ return config_cache_get_value(key);
+}
+
static int config_file_fgetc(struct config_source *conf)
{
return fgetc(conf-u.file);
@@ -333,6 +441,7 @@ static int get_value(config_fn_t fn, void *data, struct
strbuf *name)
if (!value)
return -1;
Re: [PATCH nd/split-index] fixup! read-cache: new API write_locked_index instead of write_index/write_cache
On 01/06/14 01:47, Nguyễn Thái Ngọc Duy wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
I intended it resend the series after the comments I received, but it
looks like Junio has picked up all comments except this one, so
here's the fix.
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 377c877..4b709db 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -672,7 +672,7 @@ static void prepare_revs(struct replay_opts *opts)
static void read_and_refresh_cache(struct replay_opts *opts)
{
static struct lock_file index_lock;
- hold_locked_index(index_lock, 0);
+ int index_fd = hold_locked_index(index_lock, 0);
if (read_index_preload(the_index, NULL) 0)
die(_(git %s: failed to read the index), action_name(opts));
refresh_index(the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL,
NULL);
Yep, this silences sparse. I would have declared the variable to be
(say) fd (and changed the _use_ site as well, of course!), rather
than once again hiding the index_fd() global function. However, this
is perfectly fine as-is.
The actual culprit is the index_fd() global function, but renaming it
now is probably not worth the code churn (and I'm lazy). ;-)
Thanks!
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line 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] fetch doc: Move FETCH_HEAD material, and add an example.
Signed-off-by: Marc Branchaud marcn...@xiplink.com --- Documentation/git-fetch.txt | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) This patch applies on top of your 1/5. It: * De-emphasizes the FETCH_HEAD stuff by moving it to the end of the DESCRIPTION section, * States that remote-tracking branches are simply updated, and hints that playing with refspec can control this. * Includes the their histories rephrasing. * Adds your peek-at-a-remote-branch example. If you like this, feel free to sqush it into your 1/5. M. diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index d5f5b54..06106b9 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -18,14 +18,9 @@ SYNOPSIS DESCRIPTION --- Fetch branches and/or tags (collectively, refs) from one or more -other repositories, along with the objects necessary to complete the -histories of them. - -The names of refs that are fetched, together with the object names -they point at, are written to `.git/FETCH_HEAD`. This information -is used by a later merge operation done by 'git merge'. In addition, -the remote-tracking branches may be updated (see description on -refspec below for details). +other repositories, along with the objects necessary to complete their +histories. Remote-tracking branches are updated (see the description +of refspec below for ways to control this behavior). By default, any tag that points into the histories being fetched is also fetched; the effect is to fetch tags that @@ -35,7 +30,7 @@ configuring remote.name.tagopt. By using a refspec that fetches tags explicitly, you can fetch tags that do not point into branches you are interested in as well. -'git fetch' can fetch from either a single named repository, +'git fetch' can fetch from either a single named repository or URL, or from several repositories at once if group is given and there is a remotes.group entry in the configuration file. (See linkgit:git-config[1]). @@ -43,6 +38,10 @@ there is a remotes.group entry in the configuration file. When no remote is specified, by default the `origin` remote will be used, unless there's an upstream branch configured for the current branch. +The names of refs that are fetched, together with the object names +they point at, are written to `.git/FETCH_HEAD`. This information +may be used by scripts or other git commands, such as linkgit:git-pull[1]. + OPTIONS --- include::fetch-options.txt[] @@ -79,6 +78,19 @@ the local repository by fetching from the branches (respectively) The `pu` branch will be updated even if it is does not fast-forward, because it is prefixed with a plus sign; `tmp` will not be. +* Peek at a remote's branch, without configuring the remote in your local +repository: ++ + +$ git fetch git://git.kernel.org/pub/scm/git/git.git maint +$ git log FETCH_HEAD + ++ +The first command fetches the `maint` branch from the repository at +`git://git.kernel.org/pub/scm/git/git.git` and the second command uses +`FETCH_HEAD` to examine the branch with linkgit:git-log[1]. The fetched +objects will eventually be removed by git's built-in housekeeping (see +linkgit:git-gc[1]). BUGS -- 2.0.0.1.g335f86d.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 2/5] fetch doc: update note on '+' in front of the refspec
On 14-05-30 01:54 PM, Junio C Hamano wrote: Marc Branchaud marcn...@xiplink.com writes: Then start the next sentence with In this case, you would want I somehow find that in this case redundant, given that for such branches already limits the scope of the suggestion. I dunno. I shrug in indifference. Toh-may-toe, poh-tah-toe... M. -- To unsubscribe from this list: send the line 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] environment: enable core.preloadindex by default
There is consensus that the default should change because it will benefit nearly all users (some just a little, but some a lot). See [1] and replies. [1]: http://git.661346.n2.nabble.com/git-status-takes-30-seconds-on-Windows-7-Why-tp7580816p7580853.html Signed-off-by: Steve Hoelzer shoel...@gmail.com --- Documentation/config.txt | 4 ++-- environment.c| 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1932e9b..4b3d965 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -613,9 +613,9 @@ core.preloadindex:: + This can speed up operations like 'git diff' and 'git status' especially on filesystems like NFS that have weak caching semantics and thus -relatively high IO latencies. With this set to 'true', Git will do the +relatively high IO latencies. When enabled, Git will do the index comparison to the filesystem data in parallel, allowing -overlapping IO's. +overlapping IO's. Defaults to true. core.createObject:: You can set this to 'link', in which case a hardlink followed by diff --git a/environment.c b/environment.c index 5c4815d..1c686c9 100644 --- a/environment.c +++ b/environment.c @@ -71,7 +71,7 @@ unsigned long pack_size_limit_cfg; char comment_line_char = '#'; /* Parallel index stat data preload? */ -int core_preload_index = 0; +int core_preload_index = 1; /* This is set by setup_git_dir_gently() and/or git_default_config() */ char *git_work_tree_cfg; -- 1.9.0.msysgit.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
[PATCH alt-v2] Improve function dir.c:trim_trailing_spaces()
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
---
After correcting for the trailing backslash 'text\', a switch()
structure gives better readability than nested 'ifs', the way I see it.
Add a test to show that the trailing backslash 'text\' is handled
correctly
dir.c | 35 ---
t/t0008-ignores.sh | 23 +++
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index eb6f581..06483d4 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,26 @@ void clear_exclude_list(struct exclude_list *el)
static void trim_trailing_spaces(char *buf)
{
- int i, last_space = -1, nr_spaces, len = strlen(buf);
- for (i = 0; i len; i++)
- if (buf[i] == '\\')
- i++;
- else if (buf[i] == ' ') {
- if (last_space == -1) {
- last_space = i;
- nr_spaces = 1;
- } else
- nr_spaces++;
- } else
- last_space = -1;
-
- if (last_space != -1 last_space + nr_spaces == len)
- buf[last_space] = '\0';
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ switch (*p) {
+ case ' ':
+ if (!last_space)
+ last_space = p;
+ break;
+
+ case '\\':
+ p++;
+ if (!*p)
+ return;
+
+ default:
+ last_space = NULL;
+ }
+
+ if (last_space)
+ *last_space = '\0';
}
int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..4cea858 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing
whitespace' '
test_cmp err.expect err
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+ rm -rf whitespace
+ mkdir whitespace
+ whitespace/trailing 1
+ whitespace/trailing 2
+ whitespace/trailing 3
+ whitespace/trailing 4 \\
+ whitespace/trailing 5 \\ \\
+ whitespace/trailing 6 \\a\\
+ whitespace/untracked
+ echo whitespace/trailing 1 \\ ignore
+ echo whitespace/trailing 2 ignore
+ echo whitespace/trailing 3 ignore
+ echo whitespace/trailing 4 ignore
+ echo whitespace/trailing 5 ignore
+ echo whitespace/trailing 6 a ignore
+ echo whitespace/untracked expect
+ : err.expect
+ git ls-files -o -X ignore whitespace actual 2err
+ test_cmp expect actual
+ test_cmp err.expect err
+'
+
test_done
--
1.9.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] sideband.c: Get rid of ANSI sequences for non-terminal shell
Naumov, Michael (North Sydney) michael.nau...@fiserv.com writes: You either want to correct the From: header that appears in e-mails from you, or want to start your body of your patch message like this: From: Michael Naumov mnaou...@gmail.com Some git tools such as GitExtensions for Windows... if you want the author of the patch and the name you used to sign it off to match, which is almost always what you want. Some git tools such as GitExtensions for Windows use environment variable TERM=msys which causes the weird ANSI sequence shown for the messages returned from server-side hooks The above sentence, while it may be telling a truth, feels more or less irrelevant, especially the part that talks about TERM=msys. Even if GitExtensions used TERM=vt100, the end result would be the same, wouldn't it? If you are suggesting a fix to GitExtensions to make it export TERM=dumb like everybody else does, that would be a different story. Mentioning TERM=msys causes the problem would be a very relevant thing to do. But this patch is not that. We add those ANSI sequences to help format sideband data on the user's terminal. However, GitExtensions is not using a terminal, and the ANSI sequences just confuses it. We can recognize this use by checking isatty(). This on the other hand is very readable. How about rephrasing these two like so: Diagnostic messages received on the sideband #2 from the server side are sent to the standard error with ANSI terminal control sequence \033[K that erases to the end of line appended at the end of each line. However, some programs (e.g. GitExtensions for Windows) read and interpret and/or show the message without understanding the terminal control sequences, resulting them to be shown to their end users. To help these programs, squelch the control sequence when the standard error stream is not being sent to a tty. There are programs that drive other programs (not limited to Git) through pty (hence satisfying isatty(2)) without interpreting the ANSI terminal control sequences, and it is conventional for these programs to export TERM=dumb, so and your patch still checks for TERM=dumb to help them, which is very good. See https://github.com/gitextensions/gitextensions/issues/1313 for more details And if you explain it like the above, I do not think this external reference is very useful. NOTE: I considered to cover the case that a pager has already been started. But decided that is probably not worth worrying about here, though, as we shouldn't be using a pager for commands that do network communications (and if we do, omitting the magic line-clearing signal is probably a sane thing to do). Sensible. Signed-off-by: Michael Naumov mnaou...@gmail.com Thanks-to: Erik Faye-Lund kusmab...@gmail.com Thanks-to: Jeff King p...@peff.net --- sideband.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sideband.c b/sideband.c index d1125f5..7f9dc22 100644 --- a/sideband.c +++ b/sideband.c @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out) memcpy(buf, PREFIX, pf); term = getenv(TERM); - if (term strcmp(term, dumb)) + if (isatty(2) term strcmp(term, dumb)) suffix = ANSI_SUFFIX; else suffix = DUMB_SUFFIX; -- To unsubscribe from this list: send the line unsubscribe 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] request-pull: resurrect for-linus - tags/for-linus DWIM
Johannes Sixt j.s...@viscovery.net writes: Am 5/16/2014 19:57, schrieb Junio C Hamano: --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -223,7 +223,13 @@ test_expect_success 'pull request format' ' git request-pull initial $downstream_url tags/full:refs/tags/full ) request sed -nf fuzz.sed request request.fuzzy -test_i18ncmp expect request.fuzzy +test_i18ncmp expect request.fuzzy + +( +cd local +git request-pull initial $downstream_url full +) request +grep ' tags/full$' ' What's this crap? Here's a fix. Feel free to tame down the subject line if you think it's too strong ;) --- 8 --- From: Johannes Sixt j...@kdbg.org Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh Thanks for catching; I do not think the brown paper bag is too strong ;-) The recent addition to the test case 'pull request format' interrupted the single-quoted text, effectively adding a third argument to the test_expect_success command. Since we do not have a prerequisite named pull request format, the test is skipped, no matter what. Additionally, the file name argument to the grep command is missing. Fix both issues. Signed-off-by: Johannes Sixt j...@kdbg.org --- t/t5150-request-pull.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index 93e2c65..82c33b8 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -229,7 +229,7 @@ test_expect_success 'pull request format' ' cd local git request-pull initial $downstream_url full ) request - grep ' tags/full$' + grep tags/full\$ request ' test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' ' -- To unsubscribe from this list: send the line unsubscribe 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] refs.c: change read_ref_at to use the reflog iterators
On Fri, May 30, 2014 at 2:59 PM, Junio C Hamano gits...@pobox.com wrote: Ronnie Sahlberg sahlb...@google.com writes: read_ref_at has its own parsing of the reflog file for no really good reason so lets change this to use the existing reflog iterators. This removes one instance where we manually unmarshall the reflog file format. Log messages for errors are changed slightly. We no longer print the file name for the reflog, instead we refer to it as 'Log for ref refname'. This might be a minor useability regression, but I don't really think so, since experienced users would know where the log is anyway and inexperienced users would not know what to do about/how to repair 'Log ... has gap ...' anyway. Adapt the t1400 test to handle the cahnge in log messages. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c| 217 -- t/t1400-update-ref.sh | 4 +- 2 files changed, 122 insertions(+), 99 deletions(-) After reading the log message we are removing one redundant implementation, I would have expected that such a change would remove a lot more lines than it would add. Seeing the diffstat, I have to wonder if the fact that we need more lines to reuse the API is an indication that the reflog iterator API is overly cumbersome to use, perhaps requiring too much boilerplate or something. Yeah. We simplify the code and make it reuse existing unmarshallers and make it easier to read, and the line count goes up :-( Most of the extra code is the due to the structure to hold all the data we need in the callbacks and is a bit less compact. That said, I think the new code is a little easier to read. The biggest value is that we reduce the number of reflog unmarshallers by one which will save work when we start storing reflogs in a different type of backend. The update in the error message may be a side-effect, but I think it is a change in the good direction. The update in error message is also to prepare for the possibility that we might get a different type of ref and reflog store in the future. So that we only generate log messages that refer to filenames in those places where we are actually accessing files directly. Thanks. I will resend the patch later with Eric's suggestions. ronnie sahlberg -- To unsubscribe from this list: send the line 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] refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 202 ++
t/t1400-update-ref.sh | 4 +-
2 files changed, 107 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..b45bb2f 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
+ int tz;
+ unsigned long date;
+ char **msg;
+ unsigned long *cutoff_time;
+ int *cutoff_tz;
+ int *cutoff_cnt;
+};
+
+static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp, int tz,
+ const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ cb-reccnt++;
+ cb-tz = tz;
+ cb-date = timestamp;
+
+ if (timestamp = cb-at_time || cb-cnt == 0) {
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt - 1;
+ /*
+* we have not yet updated cb-[n|o]sha1 so they still
+* hold the values for the previous record.
+*/
+ if (!is_null_sha1(cb-osha1)) {
+ hashcpy(cb-sha1, nsha1);
+ if (hashcmp(cb-osha1, nsha1))
+ warning(Log for ref %s has gap after %s.,
+ cb-refname, show_date(cb-date,
cb-tz, DATE_RFC2822));
+ }
+ else if (cb-date == cb-at_time)
+ hashcpy(cb-sha1, nsha1);
+ else if (hashcmp(nsha1, cb-sha1))
+ warning(Log for ref %s unexpectedly ended on %s.,
+ cb-refname, show_date(cb-date, cb-tz,
+ DATE_RFC2822));
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ cb-found_it = 1;
+ return 1;
+ }
+ hashcpy(cb-osha1, osha1);
+ hashcpy(cb-nsha1, nsha1);
+ if (cb-cnt 0)
+ cb-cnt--;
+ return 0;
+}
+
+static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+ const char *email, unsigned long timestamp,
+ int tz, const char *message, void *cb_data)
+{
+ struct read_ref_at_cb *cb = cb_data;
+
+ if (cb-msg)
+ *cb-msg = xstrdup(message);
+ if (cb-cutoff_time)
+ *cb-cutoff_time = timestamp;
+ if (cb-cutoff_tz)
+ *cb-cutoff_tz = tz;
+ if (cb-cutoff_cnt)
+ *cb-cutoff_cnt = cb-reccnt;
+ hashcpy(cb-sha1, osha1);
+ if (is_null_sha1(cb-sha1))
+ hashcpy(cb-sha1, nsha1);
+ /* We just want the first entry */
+ return 1;
+}
+
int read_ref_at(const char *refname, unsigned long at_time, int cnt,
unsigned char *sha1, char **msg,
unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
{
- const char *logfile, *logdata, *logend, *rec, *lastgt, *lastrec;
- char *tz_c;
- int logfd, tz, reccnt = 0;
- struct stat st;
- unsigned long date;
- unsigned char logged_sha1[20];
- void *log_mapped;
- size_t mapsz;
+ struct read_ref_at_cb cb;
- logfile = git_path(logs/%s, refname);
- logfd = open(logfile, O_RDONLY, 0);
- if (logfd 0)
- die_errno(Unable to read log '%s', logfile);
- fstat(logfd, st);
- if (!st.st_size)
- die(Log %s is empty., logfile);
- mapsz = xsize_t(st.st_size);
- log_mapped = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, logfd, 0);
- logdata = log_mapped;
- close(logfd);
Re: [PATCH] fetch doc: Move FETCH_HEAD material, and add an example.
Marc Branchaud marcn...@xiplink.com writes: Signed-off-by: Marc Branchaud marcn...@xiplink.com --- Documentation/git-fetch.txt | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) This patch applies on top of your 1/5. It: * De-emphasizes the FETCH_HEAD stuff by moving it to the end of the DESCRIPTION section, This reads much better. Thanks. * States that remote-tracking branches are simply updated, and hints that playing with refspec can control this. * Includes the their histories rephrasing. * Adds your peek-at-a-remote-branch example. If you like this, feel free to sqush it into your 1/5. Will splice in as patch 1.5 instead ;-) -- To unsubscribe from this list: send the line unsubscribe 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] Makefile: don't hardcode HEAD in dist target
Dennis Kaarsemaker den...@kaarsemaker.net writes:
Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}.
This makes sure the archive name and contents are consistent, if HEAD
has moved, but GIT-VERSION-FILE hasn't been regenerated yet.
Signed-off-by: Dennis Kaarsemaker den...@kaarsemaker.net
---
I have a somewhat odd setup in which I share a .git between multiple
checkouts for automated builds. To minimize locking time for parallel
builds with different options, there's only a lock around checkout and
running git describe $commit version, the builds themselves run in
parallel.
This works just fine except during 'make dist', which is hardcoded to
package up HEAD, but that's not always the commit that is actually
checked out, another process may have checked out something else.
I realize this setup is somewhat strange, but the only change necessary
to make this work is this one-line patch, so I hope that's acceptable.
The dist target happens to do a clean checkout to a temporary
directory with git archive and then muck with its contents before
tarring up the result, so moving HEAD around may appear to work for
this particular target, but htmldocs/manpages targets use what is in
the current checkout of Documentation/ area. Turning the HEAD^{tree}
into $(GIT_VERSION)^{tree} would make the inconsistency between the
two worse, wouldn't it?
If we were to change the dist rules, we may want to go in the
opposite direction. Instead of using git archive to make a
temporary copy of a directory from a commit, make such a temporary
copy from the contents of the current working tree (or the index).
And then tar-up a result after adding configure, version etc. to it.
That would mean what will be in the tarball can be different from
even HEAD, which is more consistent with the way how documentation
tarballs are made.
Alternatively, you can update the dist-doc rules to make a temporary
copy of documentation area and generate the documentation tarballs
out of a pristine source of a known version---which would also make
these two consistent. I am not sure which one is more preferrable,
though.
Why are you sharing the .git/HEAD across multiple checkouts in the
first place? If they are to build all different versions, surely
these working trees are derived from different commits, no? Have
you considered using contrib/workdir/git-new-workdir, perhaps?
I dunno.
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a53f3a8..63d9bac 100644
--- a/Makefile
+++ b/Makefile
@@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE
GIT_TARNAME = git-$(GIT_VERSION)
dist: git.spec git-archive$(X) configure
./git-archive --format=tar \
- --prefix=$(GIT_TARNAME)/ HEAD^{tree} $(GIT_TARNAME).tar
+ --prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree}
$(GIT_TARNAME).tar
@mkdir -p $(GIT_TARNAME)
@cp git.spec configure $(GIT_TARNAME)
@echo $(GIT_VERSION) $(GIT_TARNAME)/version
--
To unsubscribe from this list: send the line unsubscribe 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 0/9] replace signal() with sigaction()
Johannes Sixt j.s...@viscovery.net writes: Jeremiah Mahler (9): compat/mingw.c: expand MinGW support for sigaction connect.c: replace signal() with sigaction() progress.c: replace signal() with sigaction() write_or_die.c: replace signal() with sigaction() daemon.c: replace signal() with sigaction() builtin/log.c: replace signal() with sigaction() builtin/merge-index.c: replace signal() with sigaction() builtin/verify-tag.c: replace signal() with sigaction() sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. Hmm, you sound a bit harsher than you usually do---although I sort of share with you the doubt on the practical merits. -- To unsubscribe from this list: send the line unsubscribe 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] contrib: add convert-grafts-to-replace-refs.sh
From: Eric Sunshine sunsh...@sunshineco.com
On Sun, Jun 1, 2014 at 11:10 AM, Christian Couder
chrisc...@tuxfamily.org wrote:
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Convertion failed for: $definition
s/Convertion/Conversion/ [1]
[1]:
http://git.661346.n2.nabble.com/Re-PATCH-contrib-add-convert-grafts-to-replace-refs-sh-tp7611822.html
Ooops, sorry I forgot this.
+ }
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
Could not rename... might be a bit more friendly to non-Unixy folk.
Ok, I will use rename.
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe 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 v2 1/2] config: Add hashtable for config parsing retrieval
On 2014-06-02 16.47, Tanay Abhra wrote:
[]
Please see 3 minor remarks inline.
--- a/config.c
+++ b/config.c
@@ -9,6 +9,8 @@
#include exec_cmd.h
#include strbuf.h
#include quote.h
+#include hashmap.h
+#include string-list.h
struct config_source {
struct config_source *prev;
@@ -37,6 +39,112 @@ static struct config_source *cf;
static int zlib_compression_seen;
+static struct hashmap config_cache;
+
+struct config_cache_entry {
+ struct hashmap_entry ent;
+ char *key;
+ struct string_list *value_list;
+};
+
+static int hashmap_is_init = 0;
we don't need the = 0, as all static data is initialized to 0 (or NULL)
+
+static int config_cache_entry_cmp_icase(const struct config_cache_entry *e1,
+ const struct config_cache_entry *e2, const
char* key)
the * should be aligned to the variable key:
const char *key
[]
+static void config_cache_set_value(const char *key, const char *value)
+{
+ struct config_cache_entry *e;
+
+ e = config_cache_find_entry(key);
+ if (!e) {
+ e = malloc(sizeof(*e));
I think we need xmalloc() here (from wrapper.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] Makefile: don't hardcode HEAD in dist target
On Mon, Jun 02, 2014 at 11:53:36AM -0700, Junio C Hamano wrote:
Dennis Kaarsemaker den...@kaarsemaker.net writes:
Instead of calling git-archive HEAD^{tree}, use $(GIT_VERSION)^{tree}.
This makes sure the archive name and contents are consistent, if HEAD
has moved, but GIT-VERSION-FILE hasn't been regenerated yet.
Signed-off-by: Dennis Kaarsemaker den...@kaarsemaker.net
---
I have a somewhat odd setup in which I share a .git between multiple
checkouts for automated builds. To minimize locking time for parallel
builds with different options, there's only a lock around checkout and
running git describe $commit version, the builds themselves run in
parallel.
This works just fine except during 'make dist', which is hardcoded to
package up HEAD, but that's not always the commit that is actually
checked out, another process may have checked out something else.
I realize this setup is somewhat strange, but the only change necessary
to make this work is this one-line patch, so I hope that's acceptable.
The dist target happens to do a clean checkout to a temporary
directory with git archive and then muck with its contents before
tarring up the result, so moving HEAD around may appear to work for
this particular target, but htmldocs/manpages targets use what is in
the current checkout of Documentation/ area. Turning the HEAD^{tree}
into $(GIT_VERSION)^{tree} would make the inconsistency between the
two worse, wouldn't it?
I'd say it would make the consistency better, because now both look at
what is checked out instead of at HEAD. I agree that it's a game of
whack-a-mole though and it's really easy to add another dependency on
HEAD and GIT-VERSION-FILE agreeing with each other.
If we were to change the dist rules, we may want to go in the
opposite direction. Instead of using git archive to make a
temporary copy of a directory from a commit, make such a temporary
copy from the contents of the current working tree (or the index).
And then tar-up a result after adding configure, version etc. to it.
That would mean what will be in the tarball can be different from
even HEAD, which is more consistent with the way how documentation
tarballs are made.
Alternatively, you can update the dist-doc rules to make a temporary
copy of documentation area and generate the documentation tarballs
out of a pristine source of a known version---which would also make
these two consistent. I am not sure which one is more preferrable,
though.
Why are you sharing the .git/HEAD across multiple checkouts in the
first place? If they are to build all different versions, surely
these working trees are derived from different commits, no?
I'm sharing all of .git using explicit GIT_DIR and GIT_WORK_TREE
environment variables, sharing .git/HEAD comes with that. What I'm
actually doing is a continuos integration setup that can run many
actions at once in freshly checked-out work trees, but sharing .git to
save space.
What it specifically doing is running 'make test' for master, next and
pu, and make dist for next. As long as I protect the 'git checkout
$sha1' with a lock, that all works just fine.
Have you considered using contrib/workdir/git-new-workdir, perhaps?
I have not, thanks for the pointer. That approach is definitely cleaner
than what I am currently doing.
I dunno.
With the hint above, I actually don't need this patch anymore. And if
you're not convinced it's a good idea, it's probably better to drop it :)
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a53f3a8..63d9bac 100644
--- a/Makefile
+++ b/Makefile
@@ -2433,7 +2433,7 @@ git.spec: git.spec.in GIT-VERSION-FILE
GIT_TARNAME = git-$(GIT_VERSION)
dist: git.spec git-archive$(X) configure
./git-archive --format=tar \
- --prefix=$(GIT_TARNAME)/ HEAD^{tree} $(GIT_TARNAME).tar
+ --prefix=$(GIT_TARNAME)/ $(GIT_VERSION)^{tree}
$(GIT_TARNAME).tar
@mkdir -p $(GIT_TARNAME)
@cp git.spec configure $(GIT_TARNAME)
@echo $(GIT_VERSION) $(GIT_TARNAME)/version
--
Dennis Kaarsemaker den...@kaarsemaker.net
http://twitter.com/seveas
--
To unsubscribe from this list: send the line unsubscribe 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] mailinfo: use strcmp() for string comparison
On Sun, Jun 01, 2014 at 11:00:40AM +0200, René Scharfe wrote:
The array header is defined as:
static const char *header[MAX_HDR_PARSED] = {
From,Subject,Date,
};
When looking for the index of a specfic string in that array, simply
use strcmp() instead of memcmp(). This avoids running over the end of
the string (e.g. with memcmp(Subject, From, 7)) and gets rid of
magic string length constants.
Signed-off-by: Rene Scharfe l@web.de
Looks correct to me.
---
This is a minimal fix. A good question, however, would be: Why do we
keep on looking up constant strings in a (short) constant string array
anyway?
Yeah, this code is quite confusing. I suspect it would be more readable
to unroll any loops over the header array into a series of function
calls or even just cascading if/else. Some of the sites (e.g.,
check_header) already have a mix, like:
for (i = 0; header[i]; i++)
if (cmp_header(line, header[i]))
... do something for this header ...
if (cmp_header(line, some-other-header))
... do something special for this header type ...
else if (cmp_header(line, another))
... and something else again ...
The looping is not really helping much there in the first place, since
it is not dealing with half of the headers. And then adding on top that
the loop has its own special cases found by comparing the string to
Subject, I think it would be simpler to just unroll it.
That's just from a quick 5-minute read of the code, though. This isn't
an area I'm very familiar with, so maybe the refactor would get ugly.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] config: Add documentation for writing config files
Tanay Abhra tanay...@gmail.com writes: Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/technical/api-config.txt | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) Even though the reason to replace a TODO with real stuff is rather straigthforward, the reader appriates a short commit message (ideally pointing to the commit introducing the TODO). My first thought reading the patch was wtf, is that a patch in the middle of a series, where does this TODO come from ;-). diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 230b3a0..df08385 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data) Writing Config Files -TODO +Git gives multiple entry points in the Config API to write config values to +files namely `git_config_set_in_file` and `git_config_set`, which write to +a specific config file or to `.git/config` respectively. They both take a +key/value pair as parameter. Sounds good. +In the end they both all call `git_config_set_multivar_in_file` which takes +four parameters: Do we really want to document this as part of the config API? i.e. does a normal user of the API want to know about this? My understanding is that git_config_set_multivar_in_file is essentially a private function, and then the best place to document is with comments near the function definition (it already has such comment). Your text is easier to understand and more detailed, but I would personnally prefer improving the in-code comment (possibly just leaving a mention of git_config_set_multivar_in_file and pointing the reader to the code for details). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
On Sun, Jun 01, 2014 at 01:07:21PM +0200, René Scharfe wrote: Whenever the hash table becomes too small then its size is increased, the original part (and the added space) is zerod out using memset(), and the table is rebuilt from scratch. Simplify this proceess by returning the old memory using free() and allocating the new buffer using xcalloc(), which already clears the buffer for us. That way we avoid copying the old hash table contents needlessly inside xrealloc(). While at it, use the first array member with sizeof instead of a specific type. The old code used uint32_t and int, while index is actually an array of int32_t. Their sizes are the same basically everywhere, so it's not actually a problem, but the new code is cleaner and doesn't have to be touched should the type be changed. Signed-off-by: Rene Scharfe l@web.de Looks good to me. BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked on), but actually originated in 7a979d9 (Thin pack - create packfile with missing delta base., 2006-02-19). Not that it matters, but I was just surprised since the code you are changing did not seem familiar to me. I guess there was just too much refactoring during the code movement for git-blame to pass along the blame in this case. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] string-list.h: Add a value to string_list initializer lists
Tanay Abhra tanay...@gmail.com writes:
Add a NULL value in the end of STRING_LIST_INIT_NODUP and
STRING_LIST_DUP to initialize `compare_strings_fn`.
Hmph. That is what change is proposed, which we can read from the
patch text itself below. We would want to see why is this change a
good thing to justify it.
As you mentioned later In C, ..., there is nothing wrong in what
we have (and we can not quite read what exactly you tried to do with
uninitialized memory with these macros), so we need an excuse other
than correctness to justify this change. Perhaps like this?
STRING_LIST_INIT_{NODUP,DUP} initializers list values only
for earlier structure members, relying on the usual
convention in C that the omitted members are initailized to
0, i.e. the former is expanded to the latter:
struct string_list l = STRING_LIST_INIT_DUP;
struct string_list l = { NULL, 0, 0, 1 };
and the last member that is not mentioned (i.e. 'cmp') is
initialized to NULL.
While there is nothing wrong in this construct, spelling out
all the values where the macros are defined will serve also
as a documentation, so let's do so.
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
When I used a malloced string_list to play around with string-list API and
used the default init_list, it caused a seg fault. After an hour of debugging
I saw that comapre_strings_fn should be initialized to NULL. In C, even an
incomplete initialzer initializes every value to NULl or 0, so in normal
usage in the codebase this problem never occurs. Still it is better to be
thorough.
string-list.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/string-list.h b/string-list.h
index de6769c..87ee419 100644
--- a/string-list.h
+++ b/string-list.h
@@ -15,8 +15,8 @@ struct string_list {
compare_strings_fn cmp; /* NULL uses strcmp() */
};
-#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0 }
-#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1 }
+#define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }
+#define STRING_LIST_INIT_DUP { NULL, 0, 0, 1, NULL }
void print_string_list(const struct string_list *p, const char *text);
void string_list_clear(struct string_list *list, int free_util);
--
To unsubscribe from this list: send the line unsubscribe 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] environment: enable core.preloadindex by default
Am 02.06.2014 18:43, schrieb Steve Hoelzer: There is consensus that the default should change because it will benefit nearly all users (some just a little, but some a lot). See [1] and replies. Git for Windows users may want to try core.fscache=true as well [1]. This eliminates the UAC penalties for repositories located on the Windows system drive [2]. [1] https://github.com/msysgit/git/pull/94 [2] https://code.google.com/p/msysgit/issues/detail?id=320 -- To unsubscribe from this list: send the line unsubscribe 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 alt-v2] Improve function dir.c:trim_trailing_spaces()
Pasha Bolokhov pasha.bolok...@gmail.com writes:
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
---
After correcting for the trailing backslash 'text\', a switch()
structure gives better readability than nested 'ifs', the way I see it.
Add a test to show that the trailing backslash 'text\' is handled
correctly
dir.c | 35 ---
t/t0008-ignores.sh | 23 +++
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index eb6f581..06483d4 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,26 @@ void clear_exclude_list(struct exclude_list *el)
static void trim_trailing_spaces(char *buf)
{
- int i, last_space = -1, nr_spaces, len = strlen(buf);
- for (i = 0; i len; i++)
- if (buf[i] == '\\')
- i++;
- else if (buf[i] == ' ') {
- if (last_space == -1) {
- last_space = i;
- nr_spaces = 1;
- } else
- nr_spaces++;
- } else
- last_space = -1;
-
- if (last_space != -1 last_space + nr_spaces == len)
- buf[last_space] = '\0';
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ switch (*p) {
+ case ' ':
+ if (!last_space)
+ last_space = p;
+ break;
+
+ case '\\':
+ p++;
+ if (!*p)
+ return;
+
Write
/* fallthru */
here.
+ default:
+ last_space = NULL;
+ }
+
+ if (last_space)
+ *last_space = '\0';
}
I think the loop structure is a lot simpler to follow (with or
without switch/case) with this change. Good.
int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..4cea858 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing
whitespace' '
test_cmp err.expect err
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+ rm -rf whitespace
+ mkdir whitespace
+ whitespace/trailing 1
+ whitespace/trailing 2
+ whitespace/trailing 3
+ whitespace/trailing 4 \\
+ whitespace/trailing 5 \\ \\
+ whitespace/trailing 6 \\a\\
Don't be cute and try to align with tabs.
+ whitespace/untracked
+ echo whitespace/trailing 1 \\ ignore
+ echo whitespace/trailing 2 ignore
+ echo whitespace/trailing 3 ignore
+ echo whitespace/trailing 4 ignore
+ echo whitespace/trailing 5 ignore
+ echo whitespace/trailing 6 a ignore
+ echo whitespace/untracked expect
+ : err.expect
Other file truncation is done with whitespace/filename without an
explicit : aka no-op command; I know this was copied from a
previous test but perhaps we want to be consistent?
+ git ls-files -o -X ignore whitespace actual 2err
+ test_cmp expect actual
+ test_cmp err.expect err
+'
+
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 v3 0/9] replace signal() with sigaction()
On Mon, Jun 02, 2014 at 12:05:29PM -0700, Junio C Hamano wrote: Johannes Sixt j.s...@viscovery.net writes: Jeremiah Mahler (9): compat/mingw.c: expand MinGW support for sigaction connect.c: replace signal() with sigaction() progress.c: replace signal() with sigaction() write_or_die.c: replace signal() with sigaction() daemon.c: replace signal() with sigaction() builtin/log.c: replace signal() with sigaction() builtin/merge-index.c: replace signal() with sigaction() builtin/verify-tag.c: replace signal() with sigaction() sigchain.c: replace signal() with sigaction() The series without patch 9/9 works on Windows so far. Without patch patch 9/9 and a more complete implementation of sigaction in compat/mingw.c the series misses its goal. But even if you complete it, it is IMHO only code churn without practical merits. Hmm, you sound a bit harsher than you usually do---although I sort of share with you the doubt on the practical merits. Alright, I'm dropping it. Too much work for no real gain other than some piece of mind. Thanks Johannes and Junio for your feedback. -- Jeremiah Mahler jmmah...@gmail.com http://github.com/jmahler -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't hardcode HEAD in dist target
Dennis Kaarsemaker den...@kaarsemaker.net writes: I'd say it would make the consistency better, because now both look at what is checked out instead of at HEAD. The version with your patch does not even look at HEAD; it looks at whatever GIT_VERSION points at, which could be a very different version that does not have anything to do with what is checked out Can't GIT_VERSION come from ./version file, for example? -- To unsubscribe from this list: send the line unsubscribe 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
Jeff King p...@peff.net writes: On Sun, Jun 01, 2014 at 01:07:21PM +0200, René Scharfe wrote: Whenever the hash table becomes too small then its size is increased, the original part (and the added space) is zerod out using memset(), and the table is rebuilt from scratch. Simplify this proceess by returning the old memory using free() and allocating the new buffer using xcalloc(), which already clears the buffer for us. That way we avoid copying the old hash table contents needlessly inside xrealloc(). While at it, use the first array member with sizeof instead of a specific type. The old code used uint32_t and int, while index is actually an array of int32_t. Their sizes are the same basically everywhere, so it's not actually a problem, but the new code is cleaner and doesn't have to be touched should the type be changed. Signed-off-by: Rene Scharfe l@web.de Looks good to me. BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked on), but actually originated in 7a979d9 (Thin pack - create packfile with missing delta base., 2006-02-19). Not that it matters, but I was just surprised since the code you are changing did not seem familiar to me. I guess there was just too much refactoring during the code movement for git-blame to pass along the blame in this case. Without -M, too much refactoring for git-blame may just be moving a function to a different place in the same file. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2.0.0 regression? request pull does not seem to find head
Looks like pull requests no longer work for me on linux. Some other trees (non-linux) work fine but I didn't yet check whether it's the local or the remote tree that's at issue. Or maybe it's a configuration change that I missed? Note: I have [push] default = matching configured in .gitconfig. [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert net/mlx4_en: Use affinity hint (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert net/mlx4_en: Use affinity hint (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ git --version git version 2.0.0.538.gb6dd70f [mst@robin linux]$ /usr/bin/git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert net/mlx4_en: Use affinity hint (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-next for you to fetch changes up to 2ae76693b8bcabf370b981cd00c36cd41d33fabc: vhost: replace rcu with mutex (2014-06-02 23:47:59 +0300) Michael S. Tsirkin (2): vhost-net: extend device allocation to vmalloc vhost: replace rcu with mutex drivers/vhost/net.c | 23 ++- drivers/vhost/vhost.c | 10 +- 2 files changed, 27 insertions(+), 6 deletions(-) [mst@robin linux]$ /usr/bin/git --version git version 1.8.3.1 -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] refs.c: change read_ref_at to use the reflog iterators
Ronnie Sahlberg sahlb...@google.com writes:
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.
Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref refname'.
This might be a minor useability regression, but I don't really think so,
since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.
Adapt the t1400 test to handle the change in log messages.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 202
++
t/t1400-update-ref.sh | 4 +-
2 files changed, 107 insertions(+), 99 deletions(-)
diff --git a/refs.c b/refs.c
index 6898263..b45bb2f 100644
--- a/refs.c
+++ b/refs.c
@@ -2936,109 +2936,117 @@ static char *ref_msg(const char *line, const char
*endp)
return xmemdupz(line, ep - line);
}
If I am not mistaken, this function will become unused with this
rewrite. Let's drop it and justify it in the log message.
+struct read_ref_at_cb {
+ const char *refname;
+ unsigned long at_time;
+ int cnt;
+ int reccnt;
+ unsigned char *sha1;
+ int found_it;
+
+ char osha1[20];
+ char nsha1[20];
These should be unsigned char, shouldn't they?
+ for_each_reflog_ent_reverse(refname, read_ref_at_ent, cb);
+
+ if (!cb.reccnt)
+ die(Log for %s is empty., refname);
+ if (cb.found_it)
+ return 0;
+
+ for_each_reflog_ent(refname, read_ref_at_ent_oldest, cb);
Hmph.
We have already scanned the same reflog in the backward direction in
full. Do we need to make another call just to pick up the entry at
the beginning? Is this because the callback is not told that it is
fed the last entry (in other words, if there is some clue that this
is the last call from for-each-reflog-ent-reverse to the callback,
the callback could record the value it received in its cb-data)?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.0.0 regression? request pull does not seem to find head
Michael S. Tsirkin m...@redhat.com writes: Looks like pull requests no longer work for me on linux. Wasn't does not seem to find head was very much deliberate? Linus's patch wanted the users to explicitly tell the tool, without tool trying to be too helpful and risking to guess incorrectly. Some other trees (non-linux) work fine but I didn't yet check whether it's the local or the remote tree that's at issue. Or maybe it's a configuration change that I missed? Note: I have [push] default = matching configured in .gitconfig. This should not affect anything in request-pull, I think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Reset by checkout?
Atsushi Nakagawa at...@chejz.com writes: One of the more underrepresented command I use in git use on a regular basis is this reset by checkout. It's what's currently achieved by this convoluted expression: `git checkout -B current-branch-name tree-ish` This is such an useful notion that I can fathom why there isn't a better, first-tier, alternative. Hmph. checkout *is* the first-tier way to do this. Why do you even want to do it via reset? Is it because you learned reset first and then learned how checkout with various modes all do useful things? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Reset by checkout?
Junio C Hamano gits...@pobox.com writes: Atsushi Nakagawa at...@chejz.com writes: One of the more underrepresented command I use in git use on a regular basis is this reset by checkout. It's what's currently achieved by this convoluted expression: `git checkout -B current-branch-name tree-ish` This is such an useful notion that I can fathom why there isn't a better, first-tier, alternative. Hmph. checkout *is* the first-tier way to do this. Why do you even want to do it via reset? Is it because you learned reset first and then learned how checkout with various modes all do useful things? Ahh, the branch to be checked out being the current branch is indeed strange. That is what reset --keep was invented for. I use git checkout -B something-else commit all the time, and somehow I thought that was what you were talking about. 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: 2.0.0 regression? request pull does not seem to find head
On Mon, Jun 02, 2014 at 02:27:25PM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Looks like pull requests no longer work for me on linux. Wasn't does not seem to find head was very much deliberate? I'm sorry I don't understand what you are asking here. Same thing happens if I use a branch name explicitly, not just HEAD. Linus's patch wanted the users to explicitly tell the tool, without tool trying to be too helpful and risking to guess incorrectly. So this is an intentional behaviour change? Which patch do you refer to? Some other trees (non-linux) work fine but I didn't yet check whether it's the local or the remote tree that's at issue. Or maybe it's a configuration change that I missed? Note: I have [push] default = matching configured in .gitconfig. This should not affect anything in request-pull, I think. I just thought I'd mention this. push behaviour is the only big incompatible change I'm aware of between 1.8 which works for me and 2.0 which doesn't. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.0.0 regression? request pull does not seem to find head
On Mon, Jun 2, 2014 at 2:01 PM, Michael S. Tsirkin m...@redhat.com wrote: [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? git request-pull is clearly correct. There is no net-next in that public repository. It *used* to be that request-pull then magically tried to fix it up for you, which in turn resulted in the guess not being right, like pointing to the wrong branch that just happened to have the same SHA1, or pointing to a branch when it _should_ have pointed to a tag. Now, if you pushed your local net-next branch to another branch name (I can find a branch name called vhost-next at that repository, then you can *tell* git that, using the same syntax you must have used for the push. So something like git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next:vhost-next should work so that git doesn't end up having to guess (and potentially guessing wrong). But it may actually be a simpler driver error, and you wanted to use vhost-next, and that net-next was actually incorrect? Linus -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Reset by checkout?
Kevin Bracey ke...@bracey.fi writes: Maybe something like this: I like the overall direction to re-organize the description by operations, but the new description seem to introduce a bit of new confusion. All modes move the current branch pointer so that HEAD now points to the specified commit. ORIG_HEAD is set to the original HEAD location. The modes differ in what happens to the contents of ORIG_HEAD, that are no longer on the reset branch; and also what happens to your not-yet-committed changes. --soft Retains the contents of ORIG_HEAD in the index+work area, leaving the difference as changes to be committed. This (and everything that talks about ORIG_HEAD) asks the user to think of the working tree state as a combination of the state the commit you were on represents plus the changes you made relative to it. Given that everything Git records is a whole-tree snapshot, state (not changes), and that is how tutorials teach Git, I wonder if the what is done to ORIG_HEAD and changes gets the user into right mindset to understand various modes of operations. And with that ORIG_HEAD and changes mindset, a --soft reset becomes very hard to explain. ORIG_HEAD and changes (you had before you issued the 'reset --soft' command) are left in the index/work, HEAD becomes the named commit, changes from that updated HEAD becomes the original changes (you had since ORIG_HEAD) mixed with the differences between ORIG_HEAD and HEAD. If you explain this in terms of state, a --soft reset will keep the state of the index and the working tree as-is and changes the HEAD pointer to point at a different commit. git reset --soft HEAD~1 would be the first step if you want to remove the last commit, but intend to recommit most or all of its changes. git status after reset --soft shows: To be committed: Changes in ORIG_HEAD relative to HEAD (+Any initial staged changes) There would be overlapping parts of Any initial staged changes and Changes in ORIG_HEAD relative to HEAD. They may be mixed, they may be partly reverted, or they may totally cancel out, depending on the changes the user made since starting to work on ORIG_HEAD. Not staged: (Any initial unstaged changes) --mixed (default) Retains the contents of ORIG_HEAD in the work area, leaving the difference as unstaged changes. I am confused by the above the same way. If the operation retains the contents of ORIG_HEAD in the working tree, would that mean the edit I made is somehow reverted? No, because you say leaving the difference ..., but then the operation is not really retaining the contents of ORIG_HEAD. It is leaving the state I had in my working tree as-is, regardless of ORIG_HEAD and/or HEAD that is updated. Not that I can think of a better way to update these descriptions, and not that I am opposing to update these descriptions to make it easier for new people to learn, but I am not sure if these treat ORIG_HEAD and the changes since that commit as separate entities is a good approach to do so. Somewhat frustrated, not by your patch but by being unable to suggest a better way X-. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
On Mon, Jun 02, 2014 at 10:40:44PM +0200, David Kastrup wrote: BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked on), but actually originated in 7a979d9 (Thin pack - create packfile with missing delta base., 2006-02-19). Not that it matters, but I was just surprised since the code you are changing did not seem familiar to me. I guess there was just too much refactoring during the code movement for git-blame to pass along the blame in this case. Without -M, too much refactoring for git-blame may just be moving a function to a different place in the same file. I tried git blame -M -C -C -C pack-objects.c but couldn't get anything but the whole thing blamed to 2834bc2. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.0.0 regression? request pull does not seem to find head
Linus Torvalds torvalds at linux-foundation.org writes: On Mon, Jun 2, 2014 at 2:01 PM, Michael S. Tsirkin mst at redhat.com wrote: [mst at robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? git request-pull is clearly correct. There is no net-next in that public repository. I am seeing something similar: $ git request-pull master origin /dev/null warn: No match for commit 64ea29197d5e13772b1f7b6c24feaa79cc97d997 found at origin warn: Are you sure you pushed 'HEAD' there? but I pushed 64ea29197d5e13772b1f7b6c24feaa79cc97d997: $ git show-ref bug_fix/init_report 64ea29197d5e13772b1f7b6c24feaa79cc97d997 refs/heads/bug_fix/init_report 64ea29197d5e13772b1f7b6c24feaa79cc97d997 refs/remotes/origin/bug_fix/init_report The warning goes away if I give an explicit end commit. Should the default value for $remote in the call to $find_matching_ref be $head rather than HEAD (and similarly for the warning message)? --James -- To unsubscribe from this list: send the line 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 alt-v3] Improve function dir.c:trim_trailing_spaces()
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
---
Add /* fallthrough */ comment to switch(), remove TABs in test,
and remove : command in file truncation in the test
dir.c | 36 +---
t/t0008-ignores.sh | 23 +++
2 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index eb6f581..c7dc846 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,27 @@ void clear_exclude_list(struct exclude_list *el)
static void trim_trailing_spaces(char *buf)
{
- int i, last_space = -1, nr_spaces, len = strlen(buf);
- for (i = 0; i len; i++)
- if (buf[i] == '\\')
- i++;
- else if (buf[i] == ' ') {
- if (last_space == -1) {
- last_space = i;
- nr_spaces = 1;
- } else
- nr_spaces++;
- } else
- last_space = -1;
-
- if (last_space != -1 last_space + nr_spaces == len)
- buf[last_space] = '\0';
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ switch (*p) {
+ case ' ':
+ if (!last_space)
+ last_space = p;
+ break;
+
+ case '\\':
+ p++;
+ if (!*p)
+ return;
+ /* fallthrough */
+
+ default:
+ last_space = NULL;
+ }
+
+ if (last_space)
+ *last_space = '\0';
}
int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..5ef5ad3 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing
whitespace' '
test_cmp err.expect err
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+ rm -rf whitespace
+ mkdir whitespace
+ whitespace/trailing 1
+ whitespace/trailing 2
+ whitespace/trailing 3
+ whitespace/trailing 4 \\
+ whitespace/trailing 5 \\ \\
+ whitespace/trailing 6 \\a\\
+ whitespace/untracked
+ echo whitespace/trailing 1 \\ ignore
+ echo whitespace/trailing 2 ignore
+ echo whitespace/trailing 3 ignore
+ echo whitespace/trailing 4 ignore
+ echo whitespace/trailing 5 ignore
+ echo whitespace/trailing 6 a ignore
+ echo whitespace/untracked expect
+ err.expect
+ git ls-files -o -X ignore whitespace actual 2err
+ test_cmp expect actual
+ test_cmp err.expect err
+'
+
test_done
--
1.9.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 alt-v3] Improve function dir.c:trim_trailing_spaces()
Pasha Bolokhov pasha.bolok...@gmail.com writes:
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
8ba87adad6 does not reset 'last_space' when a backslash is
encountered and the above line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
---
Add /* fallthrough */ comment to switch(), remove TABs in test,
and remove : command in file truncation in the test
8ba87adad6 does not seem to do anything to do with this change,
though. Tentatively I've queued the following (but not merged to
anywhere nor pushed out).
Thanks.
-- 8 --
From: Pasha Bolokhov pasha.bolok...@gmail.com
Date: Mon, 2 Jun 2014 15:36:56 -0700
Subject: [PATCH] dir.c:trim_trailing_spaces(): fix for \ sequence
Discard the unnecessary 'nr_spaces' variable, remove 'strlen()' and
improve the 'if' structure. Switch to pointers instead of integers
to control the loop.
Slightly more rare occurrences of 'text \' with a backslash
in between spaces are handled correctly. Namely, the code in
7e2e4b37 (dir: ignore trailing spaces in exclude patterns, 2014-02-09)
does not reset 'last_space' when a backslash is encountered and the above
line stays intact as a result.
Add a test at the end of t/t0008-ignores.sh to exhibit this behavior.
Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
dir.c | 34 +++---
t/t0008-ignores.sh | 23 +++
2 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/dir.c b/dir.c
index eb6f581..797805d 100644
--- a/dir.c
+++ b/dir.c
@@ -508,21 +508,25 @@ void clear_exclude_list(struct exclude_list *el)
static void trim_trailing_spaces(char *buf)
{
- int i, last_space = -1, nr_spaces, len = strlen(buf);
- for (i = 0; i len; i++)
- if (buf[i] == '\\')
- i++;
- else if (buf[i] == ' ') {
- if (last_space == -1) {
- last_space = i;
- nr_spaces = 1;
- } else
- nr_spaces++;
- } else
- last_space = -1;
-
- if (last_space != -1 last_space + nr_spaces == len)
- buf[last_space] = '\0';
+ char *p, *last_space = NULL;
+
+ for (p = buf; *p; p++)
+ switch (*p) {
+ case ' ':
+ if (!last_space)
+ last_space = p;
+ break;
+ case '\\':
+ p++;
+ if (!*p)
+ return;
+ /* fallthrough */
+ default:
+ last_space = NULL;
+ }
+
+ if (last_space)
+ *last_space = '\0';
}
int add_excludes_from_file_to_list(const char *fname,
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..5ef5ad3 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing
whitespace' '
test_cmp err.expect err
'
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+ rm -rf whitespace
+ mkdir whitespace
+ whitespace/trailing 1
+ whitespace/trailing 2
+ whitespace/trailing 3
+ whitespace/trailing 4 \\
+ whitespace/trailing 5 \\ \\
+ whitespace/trailing 6 \\a\\
+ whitespace/untracked
+ echo whitespace/trailing 1 \\ ignore
+ echo whitespace/trailing 2 ignore
+ echo whitespace/trailing 3 ignore
+ echo whitespace/trailing 4 ignore
+ echo whitespace/trailing 5 ignore
+ echo whitespace/trailing 6 a ignore
+ echo whitespace/untracked expect
+ err.expect
+ git ls-files -o -X ignore whitespace actual 2err
+ test_cmp expect actual
+ test_cmp err.expect err
+'
+
test_done
--
2.0.0-511-g1433423
--
To unsubscribe from this list: send the line unsubscribe 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] pack-objects: use free()+xcalloc() instead of xrealloc()+memset()
Jeff King p...@peff.net writes: On Mon, Jun 02, 2014 at 10:40:44PM +0200, David Kastrup wrote: BTW, the code does git-blame to Vicent's 2834bc2 (which I also worked on), but actually originated in 7a979d9 (Thin pack - create packfile with missing delta base., 2006-02-19). Not that it matters, but I was just surprised since the code you are changing did not seem familiar to me. I guess there was just too much refactoring during the code movement for git-blame to pass along the blame in this case. Without -M, too much refactoring for git-blame may just be moving a function to a different place in the same file. I tried git blame -M -C -C -C pack-objects.c but couldn't get anything but the whole thing blamed to 2834bc2. Are you two being a bit too unreasonable, or trying to be fanciful and funny and I am not getting the humor? Here is the relevant part of what 2834bc27 (pack-objects: refactor the packing list, 2013-10-24) removes from builtin/pack-objects.c: - object_ix = xrealloc(object_ix, sizeof(int) * object_ix_hashsz); - memset(object_ix, 0, sizeof(int) * object_ix_hashsz); And here is how the same rehash is done in pack-objects.c at the toplevel in the new code: + pdata-index = xrealloc(pdata-index, sizeof(uint32_t) * pdata-index_size); + memset(pdata-index, 0, sizeof(int) * pdata-index_size); Surely, the code structure may be similar, but the similarity ends there. These lines are not equivalent even under the -w option. -- To unsubscribe from this list: send the line 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] sideband.c: Get rid of ANSI sequences for non-terminal shell
From: Michael Naumov mnaou...@gmail.com Diagnostic messages received on the sideband #2 from the server side are sent to the standard error with ANSI terminal control sequence \033[K that erases to the end of line appended at the end of each line. However, some programs (e.g. GitExtensions for Windows) read and interpret and/or show the message without understanding the terminal control sequences, resulting them to be shown to their end users. To help these programs, squelch the control sequence when the standard error stream is not being sent to a tty. NOTE: I considered to cover the case that a pager has already been started. But decided that is probably not worth worrying about here, though, as we shouldn't be using a pager for commands that do network communications (and if we do, omitting the magic line-clearing signal is probably a sane thing to do). Signed-off-by: Michael Naumov mnaou...@gmail.com Thanks-to: Erik Faye-Lund kusmab...@gmail.com Thanks-to: Jeff King p...@peff.net --- sideband.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sideband.c b/sideband.c index d1125f5..7f9dc22 100644 --- a/sideband.c +++ b/sideband.c @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out) memcpy(buf, PREFIX, pf); term = getenv(TERM); - if (term strcmp(term, dumb)) + if (isatty(2) term strcmp(term, dumb)) suffix = ANSI_SUFFIX; else suffix = DUMB_SUFFIX; -- 1.8.3.msysgit.0 P.S. I gave up trying to send this letter from gmail, it eats my tab character P.S 2. Sorry, my tab character has been eaten again! P.S 3. Wrapped the letter lines to fit on the terminal P.S 4. GitExtensions tried to use TERM=dumb but this caused process leakage for diff tools. See https://github.com/gitextensions/gitextensions/issues/1092 Regards, Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.0.0
On Wed, 28 May 2014 15:31:13 -0700 Junio C Hamano gits...@pobox.com wrote:
The latest feature release Git v2.0.0 is now available at the
usual places.
git request-pull lost a few heuristics that often led to mistakes.
I've installed git 2.0.0 and now when I
git request-pull master git://neil.brown.name/md
after tagging the current commit as md/3.15-fixes and pushing out the tag,
I get
warn: No match for commit 2ac295a544dcae9299cba13ce250419117ae7fd1 found at
git://neil.brown.name/md
warn: Are you sure you pushed 'HEAD' there?
Yet
git ls-remote git://neil.brown.name/md | grep 2ac29
shows
2ac295a544dcae9299cba13ce250419117ae7fd1refs/tags/md/3.15-fixes^{}
Which seems clear and unambiguous.
Does this mean that the 'end' arg to 'git request-pull' is no longer optional
(i.e. the man page is wrong), or that too many heuristics were removed?
Looking through the change log a bit, it seems that if the 'end' arg is
omitted, then the current 'branch' name is used and must match the same name
at the git URL. Could it also check the current 'tag' name? As we are
encouraged to used signed tags, this seems sensible.
In fact, I would suggest checking for a tag first, and only considering the
branch name if there is no tag on the current commit.
Thanks,
NeilBrown
signature.asc
Description: PGP signature
Re: [ANNOUNCE] Git v2.0.0
On Mon, Jun 2, 2014 at 7:08 PM, NeilBrown ne...@suse.de wrote: git request-pull master git://neil.brown.name/md after tagging the current commit as md/3.15-fixes and pushing out the tag You should *tell* git request-pull what you're actually requesting to pull. The old let's guess based on the commit at the top of your tree may have worked for you (because you only ever had one matching thing), but it did not work in general. So the new git request-pull does not guess. If you want to request a tag to be pulled, you name it. Because if you don't name it, it now defaults to HEAD (like all other git log commands) rather than guessing. So please write it as git request-pull master git://neil.brown.name/md md/3.15-fixes I guess the man-page should be made more explicit about this too. Linus -- To unsubscribe from this list: send the line unsubscribe 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 REPORT] Git log pretty date
On Fri, May 30, 2014 at 09:08:57AM +0100, Rodrigo Fernandes wrote: Do you have any idea how does github understand that is a bug and fixes it automatically? (I'm saying this because on Github the date is correct). I looked into this. The dates you see on GitHub's web UI are actually parsed by Rugged/libgit2. The libgit2 parser is slightly more forgiving in this instance; if it sees a broken timezone, it will leave the timestamp intact, and only omit the timezone. Whereas git says no, it's broken, and the timestamp cannot be trusted. I think both are equally valid strategies, and I do not even think it is a problem that they diverge between the two implementations. I'd be OK with a patch to make git handle errors in each independently, assuming it is not too invasive. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
