RE: [PATCH v4 4/4] make poll() work on platforms that can't recv() on a non-socket
From: Erik Faye-Lund [mailto:kusmab...@gmail.com] Sent: Saturday, September 08, 2012 1:32 PM To: Joachim Schmitz Cc: Junio C Hamano; git@vger.kernel.org Subject: Re: [PATCH v4 4/4] make poll() work on platforms that can't recv() on a non-socket On Fri, Sep 7, 2012 at 5:43 PM, Joachim Schmitz j...@schmitz-digital.de wrote: This way it gets added to gnulib too. Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- compat/poll/poll.c | 5 + 1 file changed, 4 insertions(+) diff --git a/compat/poll/poll.c b/compat/poll/poll.c index e4b8319..10a204e 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -306,6 +306,10 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds) || socket_errno == ECONNABORTED || socket_errno == ENETRESET) happened |= POLLHUP; + /* some systems can't use recv() on non-socket, including HP NonStop */ + else if (/* (r == -1) */ socket_errno == ENOTSOCK) Why add commented-out code ((r == -1) )? I'm not really sure whether it may be needed, esp. for Mac OS X, where en ENOTSOCK is expected from a recv() but then dealt with by an ioctl(), which does not resert socket_errno, but does set r to something not -1. And wouldn't need this code path? There's some similar code a few lines up, which too has that part commented out: /* If the event happened on an unconnected server socket, that's fine. */ else if (r 0 || ( /* (r == -1) */ socket_errno == ENOTCONN)) happened |= (POLLIN | POLLRDNORM) sought; Bye, Jojo -- To unsubscribe from this list: send the line 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] cherry-pick: Append -x line on separate paragraph
Junio C Hamano writes: Robin Stocker ro...@nibor.org writes: Junio C Hamano writes: Robin Stocker ro...@nibor.org writes: if (opts-record_origin) { + /* Some implementations don't terminate message with final \n, so add it */ + if (msg.message[strlen(msg.message)-1] != '\n') + strbuf_addch(msgbuf, '\n'); I can agree that this is a good change. + strbuf_addch(msgbuf, '\n'); But this is somewhat dubious. Even if what we are adding is merely an extra LF, that changes the mechanically generated output format and can break existing hooks that read from these generated commit log template. Hm, for a script to break because of an extra LF it would have to be very badly written. If it looks for \n(cherry picked ..., it would still work. But I see the point. If you approach this change from the information left by -x is somehow useful point of view, it wouldn't make any difference. Scripts can match (cherry picked from ... and glean useful information out of it, with or without an empty line around it. But if you look at it from the other perspective [*1*], it makes a big difference. A script that wants to excise these lines used to be able to just delete such a line. With the proposed change, it also has to be aware of an empty line next to it. Ok, didn't think that a script would want to remove such a line. It makes sense when considering that it used to always add the line. Thanks for explaining. Is there a reason better than having an empty line there look better to _me_ to justify this change? Yes: Then please have them in the proposed commit log message to justify the change. I think the following analysis I quoted from your message summarizes the motivation well. * If the original commit message consisted just of a summary line, the commit message after -x would then not have a blank second line, which is bad style, e.g.: The title of the original commit (cherry picked ...) This is very true. So we at least want an empty line added when the original is one-liner. * If the original message did not have any trailers, the appended text would stick to the last paragraph, even though it is a separate thing. The other side of this argument is if there are trailers, we would not want an extra blank line. We need to look at the last paragraph of the log message and if it does not end with a trailer, we want an additional empty line. Maybe the solution is to detect if the original commit message ends with a trailer and in that case keep the existing behavior of not inserting a blank line? Yeah, that sounds like a good change from this makes the result look better point of view. How do you think we could best detect a tailer? Check if all lines of the last paragraph start with /[\w-]+: /? Oh, I like that proposal. I'd lean towards a new --trailer option I think. It would have the same problem of having to append it on a separate paragraph if the original commit message does not already have a trailer though. Yes. The logic would be the same. First terminate the incomplete last line, if any, and then look at the last paragraph of the commit log message (one liner is a natural degenerate case of this; its single-line title is the last paragraph) and if and only if it does not end with a trailer, add a blank line before adding the marker. The only difference between the two would be how the cherry-picked from line is formatted. Right. I'm going to work on this and submit a new version of the patch. The Cherry-picked-from change could then be made later on top of that. [Footnote] *1* Originally, we added (cherry picked from ... by default, and had a switch to disable it; later we made it off by default and made it optional (and discouraged) with -x and this was for a reason. Unless the original commit object is also available to the reader of the history, the line is a useless noise, and many people are found to cherry-pick from their private branches; by definition, the line is useless in the resulting commit of such a cherry-pick. -- To unsubscribe from this list: send the line 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] Support for setitimer() on platforms lacking it
From: Joachim Schmitz [mailto:j...@schmitz-digital.de] Sent: Friday, September 07, 2012 11:55 AM To: 'Junio C Hamano' Cc: 'git@vger.kernel.org' Subject: RE: [PATCH v3] Support for setitimer() on platforms lacking it HP NonStop (currently) doesn't have setitimer(). The previous attempt of an emulation (reverted by this commit) was not a real substitute for a recurring itimer (as here we also don't have SA_RESTART, so can't re-arm the timer). As setitimer() is only used in cases of perceived latency and it doesn't affect correctness, it now gets disabled entirely, if NO_SETITIMER is set. HP NonStop does provide struct itimerval, but other platforms may not, so this is taken care of in this commit too, by setting NO_STRUCT_ITIMERVAL. Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- Makefile | 5 + compat/itimer.c | 50 -- git-compat-util.h | 11 +-- 3 files changed, 14 insertions(+), 52 deletions(-) delete mode 100644 compat/itimer.c diff --git a/Makefile b/Makefile index ac49320..7be555b 100644 --- a/Makefile +++ b/Makefile @@ -157,6 +157,11 @@ all:: # Define NO_PREAD if you have a problem with pread() system call (e.g. # cygwin1.dll before v1.5.22). # +# Define NO_SETITIMER if you don't have setitimer() +# +# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval +# This also implies NO_SETITIMER +# # Define NO_THREAD_SAFE_PREAD if your pread() implementation is not # thread-safe. (e.g. compat/pread.c or cygwin) # Here too (just like in my MKDIR_WO_TRAILING_SLASH patch) it is missing the part that adds ifdef NO_STRUCT_ITIMERVAL COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL NO_SETITIMER=YesPlease endif ifdef NO_SETITIMER COMPAT_CFLAGS += -DNO_SETITIMER endif diff --git a/compat/itimer.c b/compat/itimer.c deleted file mode 100644 index 713f1ff..000 --- a/compat/itimer.c +++ /dev/null @@ -1,50 +0,0 @@ -#include ../git-compat-util.h - -static int git_getitimer(int which, struct itimerval *value) -{ - int ret = 0; - - switch (which) { - case ITIMER_REAL: - value-it_value.tv_usec = 0; - value-it_value.tv_sec = alarm(0); - ret = 0; /* if alarm() fails, we get a SIGLIMIT */ - break; - case ITIMER_VIRTUAL: /* FALLTHRU */ - case ITIMER_PROF: errno = ENOTSUP; ret = -1; break; - default: errno = EINVAL; ret = -1; - } - return ret; -} - -int git_setitimer(int which, const struct itimerval *value, - struct itimerval *ovalue) -{ - int ret = 0; - - if (!value - || value-it_value.tv_usec 0 - || value-it_value.tv_usec 100 - || value-it_value.tv_sec 0) { - errno = EINVAL; - return -1; - } - - else if (ovalue) - if (!git_getitimer(which, ovalue)) - return -1; /* errno set in git_getitimer() */ - - else - switch (which) { - case ITIMER_REAL: - alarm(value-it_value.tv_sec + - (value-it_value.tv_usec 0) ? 1 : 0); - ret = 0; /* if alarm() fails, we get a SIGLIMIT */ - break; - case ITIMER_VIRTUAL: /* FALLTHRU */ - case ITIMER_PROF: errno = ENOTSUP; ret = -1; break; - default: errno = EINVAL; ret = -1; - } - - return ret; -} diff --git a/git-compat-util.h b/git-compat-util.h index 18089f0..4628d7a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -162,9 +162,16 @@ #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NO_STRUCT_ITIMERVAL +struct itimerval { + struct timeval it_interval; + struct timeval it_value; +} +#define NO_SETITIMER The above line gets obsolete with further up mentioned change in Makefile +#endif + #ifdef NO_SETITIMER -#define setitimer(a,b,c) git_setitimer((a),(b),(c)) -extern int git_setitimer(int, const struct itimerval *, struct itimerval *); +#define setitimer(which,value,ovalue) #endif #ifdef MKDIR_WO_TRAILING_SLASH -- 1.7.12 -- To unsubscribe from this list: send the line 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] doc: move rev-list option -n from git-log.txt to rev-list-options.txt
On Sat, Sep 8, 2012 at 12:14 AM, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: Documentation/git-log.txt | 6 ++ Documentation/rev-list-options.txt | 3 ++- 2 tập tin đã bị thay đổi, 4 được thêm vào(+), 5 bị xóa(-) That is one reason why core.local=C (repo specific) and git -c core.locale=C (can be used in an alias) would be useful ;) Or LC_ALL=C LANG=C git format-patch The only problem is I forget to do that from time to time (and doing that bothers me too) It does not bother me (even though I do not read Vietnamese), but this has been brought up a few times, and we may want to revert the i18n of the diffstat summary. It does not seem to add much value to the system but annoys people. That's one step towards a better interface for non English speaking users. git log interface for example still shows Author, Commit, Date in English and these strings are shared with format-patch. Reverting back to English to me is a step back. This brings back to a series I posted about two weeks ago and got no comments http://article.gmane.org/gmane.comp.version-control.git/204285 I think it's a reasonable approach. Use English for machine interface, otherwise a native language if available. After all, the upstream diffstat does not localizes this string (I just checked diffstat-1.55 with Jan 2012 timestamp). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] Support for setitimer() on platforms lacking it
HP NonStop (currently) doesn't have setitimer(). As setitimer() is only used in cases of perceived latency and it doesn't affect correctness, it gets disabled entirely if NO_SETITIMER is set. HP NonStop does provide struct itimerval, but other platforms may not, so this is taken care of in this commit too, by setting NO_STRUCT_ITIMERVAL. Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- Makefile | 12 git-compat-util.h | 11 +++ 2 files changed, 23 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index ac49320..7be555b 100644 --- a/Makefile +++ b/Makefile @@ -157,6 +157,11 @@ all:: # Define NO_PREAD if you have a problem with pread() system call (e.g. # cygwin1.dll before v1.5.22). # +# Define NO_SETITIMER if you don't have setitimer() +# +# Define NO_STRUCT_ITIMERVAL if you don't have struct itimerval +# This also implies NO_SETITIMER +# # Define NO_THREAD_SAFE_PREAD if your pread() implementation is not # thread-safe. (e.g. compat/pread.c or cygwin) # @@ -1647,6 +1652,13 @@ ifdef OBJECT_CREATION_USES_RENAMES COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1 endif +ifdef NO_STRUCT_ITIMERVAL + COMPAT_CFLAGS += -DNO_STRUCT_ITIMERVAL + NO_SETITIMER=YesPlease +endif +ifdef NO_SETITIMER + COMPAT_CFLAGS += -DNO_SETITIMER +endif ifdef NO_PREAD COMPAT_CFLAGS += -DNO_PREAD COMPAT_OBJS += compat/pread.o diff --git a/git-compat-util.h b/git-compat-util.h index 18089f0..4628d7a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -162,6 +162,17 @@ #define probe_utf8_pathname_composition(a,b) #endif +#ifdef NO_STRUCT_ITIMERVAL +struct itimerval { + struct timeval it_interval; + struct timeval it_value; +} +#endif + +#ifdef NO_SETITIMER +#define setitimer(which,value,ovalue) +#endif + #ifdef MKDIR_WO_TRAILING_SLASH #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b)) extern int compat_mkdir_wo_trailing_slash(const char*, mode_t); -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Document MKDIR_WO_TRAILING_SLASH in Makefile
Signed-off-by: Joachim Schmitz j...@schmitz-digital.de --- Makefile | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Makefile b/Makefile index 66e8216..21b4816 100644 --- a/Makefile +++ b/Makefile @@ -90,6 +90,8 @@ all:: # # Define NO_MKDTEMP if you don't have mkdtemp in the C library. # +# Define MKDIR_WO_TRAILING_SLASH if your mkdir() can't deal with trailing slash. +# # Define NO_MKSTEMPS if you don't have mkstemps in the C library. # # Define NO_STRTOK_R if you don't have strtok_r in the C library. @@ -1639,6 +1641,10 @@ ifdef NO_MKDTEMP COMPAT_CFLAGS += -DNO_MKDTEMP COMPAT_OBJS += compat/mkdtemp.o endif +ifdef MKDIR_WO_TRAILING_SLASH + COMPAT_CFLAGS += -DMKDIR_WO_TRAILING_SLASH + COMPAT_OBJS += compat/mkdir.o +endif ifdef NO_MKSTEMPS COMPAT_CFLAGS += -DNO_MKSTEMPS endif -- 1.7.12 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Restore hostname logging in inetd mode
The following changes since commit 0ce986446163b37c7f663ce7a408e7f94c31ba63: The fourth batch for 1.8.0 (2012-09-07 11:25:22 -0700) are available in the git repository at: git://git.inai.de/git master for you to fetch changes up to 864633738f6432574402afc43b6bd83c83fc8916: daemon: restore getpeername(0,...) use (2012-09-08 19:00:35 +0200) Jan Engelhardt (1): daemon: restore getpeername(0,...) use daemon.c | 55 +++ 1 file changed, 51 insertions(+), 4 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] daemon: restore getpeername(0,...) use
This reverts f9c87be6b42dd0f8b31a4bb8c6a44326879fdd1a, in a sense, because that commit broke logging of Connection from ... when git-daemon is run under xinetd. This patch here computes the text representation of the peer and then copies that to environment variables such that the code in execute() and subfunctions can stay as-is. Signed-off-by: Jan Engelhardt jeng...@inai.de --- daemon.c | 55 +++ 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/daemon.c b/daemon.c index 4602b46..eaf08c2 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,4 @@ +#include stdbool.h #include cache.h #include pkt-line.h #include exec_cmd.h @@ -1164,6 +1165,54 @@ static int serve(struct string_list *listen_addr, int listen_port, return service_loop(socklist); } +static void inetd_mode_prepare(void) +{ + struct sockaddr_storage ss; + struct sockaddr *addr = (void *)ss; + socklen_t slen = sizeof(ss); + char addrbuf[256], portbuf[6] = ; + + if (!freopen(/dev/null, w, stderr)) + die_errno(failed to redirect stderr to /dev/null); + + /* +* Windows is said to not be able to handle this, so we will simply +* ignore failure here. (It only affects a log message anyway.) +*/ + if (getpeername(0, addr, slen) 0) + return; + + if (addr-sa_family == AF_INET) { + const struct sockaddr_in *sin_addr = (void *)addr; + + if (inet_ntop(addr-sa_family, sin_addr-sin_addr, + addrbuf, sizeof(addrbuf)) == NULL) + return; + snprintf(portbuf, sizeof(portbuf), %hu, +ntohs(sin_addr-sin_port)); +#ifndef NO_IPV6 + } else if (addr-sa_family == AF_INET6) { + const struct sockaddr_in6 *sin6_addr = (void *)addr; + + addrbuf[0] = '['; + addrbuf[1] = '\0'; + if (inet_ntop(AF_INET6, sin6_addr-sin6_addr, addrbuf + 1, + sizeof(addrbuf) - 2) == NULL) + return; + strcat(addrbuf, ]); + + snprintf(portbuf, sizeof(portbuf), %hu, +ntohs(sin6_addr-sin6_port)); +#endif + } else { + snprintf(addrbuf, sizeof(addrbuf), AF %d, +addr-sa_family); + } + if (setenv(REMOTE_ADDR, addrbuf, true) 0) + return; + setenv(REMOTE_PORT, portbuf, true); +} + int main(int argc, char **argv) { int listen_port = 0; @@ -1341,10 +1390,8 @@ int main(int argc, char **argv) die(base-path '%s' does not exist or is not a directory, base_path); - if (inetd_mode) { - if (!freopen(/dev/null, w, stderr)) - die_errno(failed to redirect stderr to /dev/null); - } + if (inetd_mode) + inetd_mode_prepare(); if (inetd_mode || serve_mode) return execute(); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] daemon: restore getpeername(0,...) use
Jan Engelhardt wrote: This reverts f9c87be6b42dd0f8b31a4bb8c6a44326879fdd1a, in a sense, because that commit broke logging of Connection from ... when git-daemon is run under xinetd. This patch here computes the text representation of the peer and then copies that to environment variables such that the code in execute() and subfunctions can stay as-is. Signed-off-by: Jan Engelhardt jeng...@inai.de --- daemon.c | 55 +++ 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/daemon.c b/daemon.c index 4602b46..eaf08c2 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,4 @@ +#include stdbool.h #include cache.h #include pkt-line.h #include exec_cmd.h @@ -1164,6 +1165,54 @@ static int serve(struct string_list *listen_addr, int listen_port, return service_loop(socklist); } +static void inetd_mode_prepare(void) +{ + struct sockaddr_storage ss; + struct sockaddr *addr = (void *)ss; + socklen_t slen = sizeof(ss); + char addrbuf[256], portbuf[6] = ; + + if (!freopen(/dev/null, w, stderr)) + die_errno(failed to redirect stderr to /dev/null); + + /* + * Windows is said to not be able to handle this, so we will simply + * ignore failure here. (It only affects a log message anyway.) + */ + if (getpeername(0, addr, slen) 0) + return; + + if (addr-sa_family == AF_INET) { + const struct sockaddr_in *sin_addr = (void *)addr; + + if (inet_ntop(addr-sa_family, sin_addr-sin_addr, + addrbuf, sizeof(addrbuf)) == NULL) + return; + snprintf(portbuf, sizeof(portbuf), %hu, + ntohs(sin_addr-sin_port)); +#ifndef NO_IPV6 + } else if (addr-sa_family == AF_INET6) { + const struct sockaddr_in6 *sin6_addr = (void *)addr; + + addrbuf[0] = '['; + addrbuf[1] = '\0'; + if (inet_ntop(AF_INET6, sin6_addr-sin6_addr, addrbuf + 1, + sizeof(addrbuf) - 2) == NULL) + return; + strcat(addrbuf, ]); + + snprintf(portbuf, sizeof(portbuf), %hu, + ntohs(sin6_addr-sin6_port)); +#endif + } else { + snprintf(addrbuf, sizeof(addrbuf), AF %d, + addr-sa_family); + } + if (setenv(REMOTE_ADDR, addrbuf, true) 0) + return; + setenv(REMOTE_PORT, portbuf, true); setenv() is not a function available on all plattfomrs. Bye, Jojo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG] 'git show' gives duplicate errors for ambiguous args
With the git.git repository: $ git show abcd error: short SHA1 abcd is ambiguous. error: short SHA1 abcd is ambiguous. fatal: ambiguous argument 'abcd': unknown revision or path not in the working tree. Use '--' to separate paths from revisions The is ambiguous message shouldn't be shown twice. The first error is printed by revision.c:1796 (a call to handle_revision_arg), and the second by revision.c:1808 (a call to verify_filename). I don't see an easy way to suppress it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: checkout extra files
Angelo Borsotti angelo.borso...@gmail.com writes: It makes quite clear that the command accepts wildcards (not expanded by the shell), which was is not clear in the current man page (although one could imagine that path could also be a wildcard). P.S. In the man page there is also a pathspec *git checkout* [-p|--patch] [tree-ish] [--] pathspec... that should perhaps be a path That's backwards. Saying path as if it means a plain vanilla pathname is a cause of confusion. The command takes pathspec, which is a pattern (see git help glossary). The places in the text that say path may need to be fixed. It just happens that you do not realize that you are using pathspec when you say git checkout hello.c, as the pattern hello.c only matches the one pathname hello.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: Restore hostname logging in inetd mode
Please don't throw a pull request for a patch whose worth hasn't been justified in a discussion on the list. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitk: Teach Reread references to reload tags
Tag contents, once read, are forever cached in memory. This makes gitk unable to notice when tag contents change. Allow users to cause a reload of the tag contents by using the File-Reread references action. Reported-by: Tim McCormack cor...@brainonfire.net Suggested-by: Junio C Hamano gits...@pobox.com Signed-off-by: David Aguilar dav...@gmail.com --- gitk |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/gitk b/gitk index 9bba9aa..a124822 100755 --- a/gitk +++ b/gitk @@ -10599,7 +10599,7 @@ proc movedhead {hid head} { } proc changedrefs {} { -global cached_dheads cached_dtags cached_atags +global cached_dheads cached_dtags cached_atags tagcontents global arctags archeads arcnos arcout idheads idtags foreach id [concat [array names idheads] [array names idtags]] { @@ -10611,6 +10611,7 @@ proc changedrefs {} { } } } +catch {unset tagcontents} catch {unset cached_dtags} catch {unset cached_atags} catch {unset cached_dheads} -- 1.7.7.2.448.gee6df -- To unsubscribe from this list: send the line 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] daemon: restore getpeername(0,...) use
Joachim Schmitz j...@schmitz-digital.de writes: + setenv(REMOTE_PORT, portbuf, true); setenv() is not a function available on all plattfomrs. Please do some homework before adding irrelevant noise. At the minimum, run git grep to see if we already use it in other places, and investigate why we can use it safely across platforms we already support. -- To unsubscribe from this list: send the line 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: Restore hostname logging in inetd mode
On Saturday 2012-09-08 20:57, Junio C Hamano wrote: Please don't throw a pull request for a patch whose worth hasn't been justified in a discussion on the list. Thanks. Let me postulate that people like to get cover letters with the git:// URL so they can fetch+look at it, a diffstat and shortlog. And 'lo, that is exactly what git-request-pull thankfully generates. In my defense: Just because the command is called request-pull, does not mean you absolutely have to merge/pull it. In fact, it does not even mention merge/pull at all. The following changes since commit [SHA]: [Commit Message] are available in the git repository at: git://[...] for you to fetch changes up to [SHA]: [Commit Message] [diffstat,shortlog] In contrast to many a LKML postings which explicitly state the pull intent: Hi [Maintainer], Please pull from the git repository at [URL] to receive [...] [SHA] [Commit Message] on top of commit [SHA] [Commit Message] [diffstat,shortlog] -- To unsubscribe from this list: send the line 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] daemon: restore getpeername(0,...) use
From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Saturday, September 08, 2012 9:04 PM To: Joachim Schmitz Cc: git@vger.kernel.org Subject: Re: [PATCH] daemon: restore getpeername(0,...) use Joachim Schmitz j...@schmitz-digital.de writes: + setenv(REMOTE_PORT, portbuf, true); setenv() is not a function available on all plattfomrs. Please do some homework before adding irrelevant noise. At the minimum, run git grep to see if we already use it in other places, and investigate why we can use it safely across platforms we already support. Hmm, guess I missed the indirect inclusion of git-compat-util.h And didn't know about 'git grep', so thanks for the hint Will look closer next time ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] daemon: restore getpeername(0,...) use
On Saturday 2012-09-08 20:59, Junio C Hamano wrote: diff --git a/daemon.c b/daemon.c index 4602b46..eaf08c2 100644 --- a/daemon.c +++ b/daemon.c @@ -1,3 +1,4 @@ +#include stdbool.h #include cache.h #include pkt-line.h #include exec_cmd.h Platform agnostic parts of the code that use git-compat-util.h (users of cache.h are indirectly users of it) are not allowed to do platform specific include like this at their beginning. This is the first use of stdbool.h; what do you need it for? For the use in setenv(,,true). It was not entirely obvious in which .h to add it; the most reasonable place was daemon.c itself, since the other .c files do not seem to need it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitk: Rename 'tagcontents' to 'cached_tagcontent'
Name the 'tagcontents' variable similarly to the rest of the variables cleared in the changedrefs() function. This makes the naming consistent and provides a hint that it should be cleared when reloading gitk's cache. Suggested-by: Junio C Hamano gits...@pobox.com Signed-off-by: David Aguilar dav...@gmail.com --- Follow-up to 'gitk: Teach Reread references to reload tags' gitk | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gitk b/gitk index a124822..6f24f53 100755 --- a/gitk +++ b/gitk @@ -10599,7 +10599,7 @@ proc movedhead {hid head} { } proc changedrefs {} { -global cached_dheads cached_dtags cached_atags tagcontents +global cached_dheads cached_dtags cached_atags cached_tagcontent global arctags archeads arcnos arcout idheads idtags foreach id [concat [array names idheads] [array names idtags]] { @@ -10611,7 +10611,7 @@ proc changedrefs {} { } } } -catch {unset tagcontents} +catch {unset cached_tagcontent} catch {unset cached_dtags} catch {unset cached_atags} catch {unset cached_dheads} @@ -10664,7 +10664,7 @@ proc listrefs {id} { } proc showtag {tag isnew} { -global ctext tagcontents tagids linknum tagobjid +global ctext cached_tagcontent tagids linknum tagobjid if {$isnew} { addtohistory [list showtag $tag 0] savectextpos @@ -10673,13 +10673,13 @@ proc showtag {tag isnew} { clear_ctext settabs 0 set linknum 0 -if {![info exists tagcontents($tag)]} { +if {![info exists cached_tagcontent($tag)]} { catch { - set tagcontents($tag) [exec git cat-file tag $tag] + set cached_tagcontent($tag) [exec git cat-file tag $tag] } } -if {[info exists tagcontents($tag)]} { - set text $tagcontents($tag) +if {[info exists cached_tagcontent($tag)]} { + set text $cached_tagcontent($tag) } else { set text [mc Tag]: $tag\n[mc Id]: $tagids($tag) } -- 1.7.7.2.448.gee6df -- To unsubscribe from this list: send the line 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: checkout extra files
From: Junio C Hamano gits...@pobox.com Sent: Friday, September 07, 2012 9:49 PM Junio C Hamano gits...@pobox.com writes: But that is not what is happening at all. What goes on is far simpler than that. - the shell sees '*', matches it against working tree files, to obtain f1 and f2; - the shell tells git to checkout e6f935e -- f1 f2; - git looks into the tree of e6f935e to find paths that match f1 and f2. When git is run by the shell in the last step, it has _no_ clue that the end user typed * from the shell. It only sees f1 and f2 on the command line. There is no set T to be intersected with set W, so stop thinking in those terms, and you will be fine. Now the question is, _you_ will be fine, but can the documentation be updated in such a way so that it will help _others_ to also stop thinking about intersection between set W and set T? I do not have a good answer to that. Let's do this. I do not want a shell tutorial in git checkout documentation, but this would fit better in the documentation for the CLI convention. The difficulty with putting it in gitcli is that it is referenced from almost nowhere, so won't provide help to the user. Having said that, it would therefore be better to point folk at gitcli in a few more places, not just the 'see also' line at the very end of the general 'git' page, and buried within rev-parse. -- 8 -- gitcli: contrast wildcard given to shell and to git People who are not used to working with shell may intellectually understand how the command line argument is massaged by the shell but still have a hard time visualizing the difference between letting the shell expand fileglobs and having Git see the fileglob to use as a pathspec. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/gitcli.txt | 16 1 file changed, 16 insertions(+) diff --git c/Documentation/gitcli.txt w/Documentation/gitcli.txt index ea17f7a..220621b 100644 --- c/Documentation/gitcli.txt +++ w/Documentation/gitcli.txt @@ -38,6 +38,22 @@ arguments. Here are the rules: you have to say either `git diff HEAD --` or `git diff -- HEAD` to disambiguate. + * Many commands allow wildcards in paths, but you need to protect +them from getting globbed by the shell. These two mean different things: ++ + +$ git checkout -- *.c +$ git checkout -- \*.c + ++ +The former lets your shell expand the fileglob, and you are asking +the dot-C files in your working tree to be overwritten with the version +in the index. The latter passes the `*.c` to Git, and you are asking +the paths in the index that match the pattern to be checked out to your +working tree. After running `git add hello.c; rm hello.c`, you will _not_ +see `hello.c` in your working tree with the former, but with the latter +you will. + When writing a script that is expected to handle random user-input, it is a good practice to make it explicit which arguments are which by placing disambiguating `--` at appropriate places. -- To unsubscribe from this list: send the line 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: checkout extra files
Philip Oakley philipoak...@iee.org writes: Having said that, it would therefore be better to point folk at gitcli in a few more places, not just the 'see also' line at the very end of the general 'git' page, and buried within rev-parse. Didn't we update the very early part of git(1) for exactly for that reason recently? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] t0000: verify that real_path() works correctly with absolute paths
On 09/07/2012 01:08 AM, Junio C Hamano wrote: mhag...@alum.mit.edu writes: From: Michael Haggerty mhag...@alum.mit.edu There is currently a bug: if passed an absolute top-level path that doesn't exist (e.g., /foo) it incorrectly interprets the path as a relative path (e.g., returns $(pwd)/foo). So mark the test as failing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- t/t-basic.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/t/t-basic.sh b/t/t-basic.sh index 1a51634..ad002ee 100755 --- a/t/t-basic.sh +++ b/t/t-basic.sh @@ -458,7 +458,17 @@ test_expect_success 'real path rejects the empty string' ' test_must_fail test-path-utils real_path ' -test_expect_success SYMLINKS 'real path works as expected' ' +test_expect_failure 'real path works on absolute paths' ' +nopath=hopefully-absent-path +test / = $(test-path-utils real_path /) +test /$nopath = $(test-path-utils real_path /$nopath) You could perhaps do sfx=0 while test -e /$nopath$sfx do sfx=$(( $sfx + 1 )) done nopath=$nopath$sfx test /$nopath = $(test-path-utils real_path /$nopath) if you really cared. The possibility is obvious. Are you advocating it? I considered that approach, but came to the opinion that it would be overkill that would only complicate the code for no real advantage, given that (1) I picked a name that is pretty implausible for an existing file, (2) the test suite only reads the file, never writes it (so there is no risk that a copy from a previous run gets left behind), (3) it's only test suite code, and any failures would have minor consequences. Please let me know if you assess the situation differently. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Teaching -Xours/-Xtheirs to binary ll-merge driver
The part that grants Stephen's wish is unchanged from the earlier perhaps like this patch, but this time with a bit of documentation and test. A more important change between the two actually is [PATCH 2/2]. When a binary synthetic attribute is given to a path, we used to (1) disable textual diff and (2) disable CR/LF conversion, but it is also sane to disable the textual merge for a path marked as binary, and setting the -merge attribute to summon the binary ll-merge driver is the way to do so. Junio C Hamano (2): merge: teach -Xours/-Xtheirs to binary ll-merge driver attr: binary attribute should choose built-in binary merge driver Documentation/gitattributes.txt| 2 +- Documentation/merge-strategies.txt | 3 ++- attr.c | 2 +- ll-merge.c | 25 - t/t6037-merge-ours-theirs.sh | 14 +- 5 files changed, 37 insertions(+), 9 deletions(-) -- 1.7.12.322.g2c7d289 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] merge: teach -Xours/-Xtheirs to binary ll-merge driver
The (discouraged) -Xours/-Xtheirs modes of merge are supposed to give a quick and dirty way to come up with a random mixture of cleanly merged parts and punted conflict resolution to take contents from one side in conflicting parts. These options however were only passed down to the low level merge driver for text. Teach the built-in binary merge driver to notice them as well. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/merge-strategies.txt | 3 ++- ll-merge.c | 25 - t/t6037-merge-ours-theirs.sh | 14 +- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 595a3cf..66db802 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -32,13 +32,14 @@ ours;; This option forces conflicting hunks to be auto-resolved cleanly by favoring 'our' version. Changes from the other tree that do not conflict with our side are reflected to the merge result. + For a binary file, the entire contents are taken from our side. + This should not be confused with the 'ours' merge strategy, which does not even look at what the other tree contains at all. It discards everything the other tree did, declaring 'our' history contains all that happened in it. theirs;; - This is opposite of 'ours'. + This is the opposite of 'ours'. patience;; With this option, 'merge-recursive' spends a little extra time diff --git a/ll-merge.c b/ll-merge.c index da59738..8535e2d 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -46,16 +46,31 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused, assert(opts); /* -* The tentative merge result is ours for the final round, -* or common ancestor for an internal merge. Still return -* conflicted merge status. +* The tentative merge result is the or common ancestor for an internal merge. */ - stolen = opts-virtual_ancestor ? orig : src1; + if (opts-virtual_ancestor) { + stolen = orig; + } else { + switch (opts-variant) { + default: + case XDL_MERGE_FAVOR_OURS: + stolen = src1; + break; + case XDL_MERGE_FAVOR_THEIRS: + stolen = src2; + break; + } + } result-ptr = stolen-ptr; result-size = stolen-size; stolen-ptr = NULL; - return 1; + + /* +* With -Xtheirs or -Xours, we have cleanly merged; +* otherwise we got a conflict. +*/ + return (opts-variant ? 0 : 1); } static int ll_xdl_merge(const struct ll_merge_driver *drv_unused, diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh index 2cf42c7..8d05671 100755 --- a/t/t6037-merge-ours-theirs.sh +++ b/t/t6037-merge-ours-theirs.sh @@ -53,7 +53,19 @@ test_expect_success 'recursive favouring ours' ' ! grep 1 file ' -test_expect_success 'pull with -X' ' +test_expect_success 'binary file with -Xours/-Xtheirs' ' + echo file -merge .gitattributes + + git reset --hard master + git merge -s recursive -X theirs side + git diff --exit-code side HEAD -- file + + git reset --hard master + git merge -s recursive -X ours side + git diff --exit-code master HEAD -- file +' + +test_expect_success 'pull passes -X to underlying merge' ' git reset --hard master git pull -s recursive -Xours . side git reset --hard master git pull -s recursive -X ours . side git reset --hard master git pull -s recursive -Xtheirs . side -- 1.7.12.322.g2c7d289 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] attr: binary attribute should choose built-in binary merge driver
The built-in binary attribute macro expands to -diff -text, so that textual diff is not produced, and the contents will not go through any CR/LF conversion ever. During a merge, it should also choose the binary low-level merge driver, but it didn't. Make it expand to -diff -merge -text. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/gitattributes.txt | 2 +- attr.c | 2 +- t/t6037-merge-ours-theirs.sh| 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index a85b187..ead7254 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -904,7 +904,7 @@ file at the toplevel (i.e. not in any subdirectory). The built-in macro attribute binary is equivalent to: -[attr]binary -diff -text +[attr]binary -diff -merge -text diff --git a/attr.c b/attr.c index 303751f..3f581b3 100644 --- a/attr.c +++ b/attr.c @@ -306,7 +306,7 @@ static void free_attr_elem(struct attr_stack *e) } static const char *builtin_attr[] = { - [attr]binary -diff -text, + [attr]binary -diff -merge -text, NULL, }; diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh index 8d05671..3889eca 100755 --- a/t/t6037-merge-ours-theirs.sh +++ b/t/t6037-merge-ours-theirs.sh @@ -54,7 +54,7 @@ test_expect_success 'recursive favouring ours' ' ' test_expect_success 'binary file with -Xours/-Xtheirs' ' - echo file -merge .gitattributes + echo file binary .gitattributes git reset --hard master git merge -s recursive -X theirs side -- 1.7.12.322.g2c7d289 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Support for setitimer() on platforms lacking it
Will queue (together with the latest MKDIR one). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/8] absolute_path(): reject the empty string
On 09/07/2012 01:09 AM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- I think I asked why this matters (iow, why it is the right thing to do to reject an empty string, instead of treating it as the current directory) in the previous round. I would have expected to find the answer be above the S-o-b line here. The reasons that the change is desirable: 1. The empty string is not a legitimate path according to POSIX; e.g., see Linux's path_resolution(7): Empty pathname In the original UNIX, the empty pathname referred to the current directory. Nowadays POSIX decrees that an empty pathname must not be resolved successfully. Linux returns ENOENT in this case. Accordingly, comparable standard functions like realpath(3) reject the empty string. 2. The functions did not handle the empty path consistently with the way they handled other paths (namely, the return value contained a trailing slash). 3. This unusual behavior was undocumented. The above points let me to the conclusion that the anomalous handling of the empty string was a bug in the implementation rather than an intended behavior. Moreover, a quick check of callers didn't turn up any that seemed to rely on the strange behavior. Do you want a re-roll with this verbiage added to the commit messages of the two relevant commits? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] t0000: verify that real_path() works correctly with absolute paths
Michael Haggerty mhag...@alum.mit.edu writes: The possibility is obvious. Are you advocating it? I considered that approach, but came to the opinion that it would be overkill that would only complicate the code for no real advantage, given that (1) I picked a name that is pretty implausible for an existing file, (2) the test suite only reads the file, never writes it (so there is no risk that a copy from a previous run gets left behind), (3) it's only test suite code, and any failures would have minor consequences. (4) if it only runs once at the very beginning of the test and sets a variable that is named prominently clear what it means and lives throughout the test, then we do not even have to say hopefully and appear lazy and loose to the readers of the test who wonders what happens when the path does exist; doing so will help reducing the noise on the mailing list in the future. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/8] absolute_path(): reject the empty string
Michael Haggerty mhag...@alum.mit.edu writes: On 09/07/2012 01:09 AM, Junio C Hamano wrote: Michael Haggerty mhag...@alum.mit.edu writes: Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- I think I asked why this matters (iow, why it is the right thing to do to reject an empty string, instead of treating it as the current directory) in the previous round. I would have expected to find the answer be above the S-o-b line here. The reasons that the change is desirable: 1. The empty string is not a legitimate path according to POSIX; e.g., see Linux's path_resolution(7): Empty pathname In the original UNIX, the empty pathname referred to the current directory. Nowadays POSIX decrees that an empty pathname must not be resolved successfully. Linux returns ENOENT in this case. Accordingly, comparable standard functions like realpath(3) reject the empty string. 2. The functions did not handle the empty path consistently with the way they handled other paths (namely, the return value contained a trailing slash). 3. This unusual behavior was undocumented. The above points let me to the conclusion that the anomalous handling of the empty string was a bug in the implementation rather than an intended behavior. Moreover, a quick check of callers didn't turn up any that seemed to rely on the strange behavior. Do you want a re-roll with this verbiage added to the commit messages of the two relevant commits? What the function used to do does not really matter when we are trying to see if the behaviour of the new implementation makes sense, even though it is a good supporting argument to help us judge that this change will not cause regressions to existing callers. A two sentence paragraph, with realpath(3) from POSIX.1 rejects an empty string as input as the primary justification, with no existing callers passes an empty string and expects to get the current directory as a supporting argument, should suffice, I would think. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitk: Rename 'tagcontents' to 'cached_tagcontent'
I've applied these two, on top of Paul's master branch at git://ozlabs.org/~paulus/gitk.git and tentatively queued in 'pu', but I would prefer to see it eyeballed by and queued in his tree first. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] t0000: verify that real_path() works correctly with absolute paths
Junio C Hamano gits...@pobox.com writes: (4) if it only runs once at the very beginning of the test and sets a variable that is named prominently clear what it means and lives throughout the test, then we do not even have to say hopefully and appear lazy and loose to the readers of the test who wonders what happens when the path does exist; doing so will help reducing the noise on the mailing list in the future. Having said that, I really do not deeply care either way. If you are rerollilng the series for other changes, I wouldn't shed tears even if I do not see any change to this part. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] Add some string_list-related functions
This patch series adds a few functions to the string_list API. They will be used in two upcoming patch series. Unfortunately, both of the series (which are otherwise logically independent) need the same function; therefore, I am submitting these string-list enhancements as a separate series on which the other two can depend. This patch series applies to current master. Michael Haggerty (4): Add a new function, string_list_split_in_place() Add a new function, filter_string_list() Add a new function, string_list_remove_duplicates() Add a function string_list_longest_prefix() .gitignore | 1 + Documentation/technical/api-string-list.txt | 32 ++ Makefile| 1 + string-list.c | 77 string-list.h | 41 + t/t0063-string-list.sh | 93 + test-string-list.c | 47 +++ 7 files changed, 292 insertions(+) create mode 100755 t/t0063-string-list.sh create mode 100644 test-string-list.c -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] Add a new function, string_list_split_in_place()
Split a string into a string_list on a separator character. This is similar to the strbuf_split_*() functions except that it works with the more powerful string_list interface. If strdup_strings is false, it reuses the memory from the input string (thereby needing no string memory allocations, though of course allocations are still needed for the string_list_items array). Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- In the tests, I use here documents to specify the expected output. Is this OK? (It is certainly convenient.) .gitignore | 1 + Documentation/technical/api-string-list.txt | 12 ++ Makefile| 1 + string-list.c | 23 +++ string-list.h | 19 + t/t0063-string-list.sh | 63 + test-string-list.c | 25 7 files changed, 144 insertions(+) create mode 100755 t/t0063-string-list.sh create mode 100644 test-string-list.c diff --git a/.gitignore b/.gitignore index bb5c91e..0ca7df8 100644 --- a/.gitignore +++ b/.gitignore @@ -193,6 +193,7 @@ /test-run-command /test-sha1 /test-sigchain +/test-string-list /test-subprocess /test-svn-fe /common-cmds.h diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt index 5a0c14f..3b959a2 100644 --- a/Documentation/technical/api-string-list.txt +++ b/Documentation/technical/api-string-list.txt @@ -124,6 +124,18 @@ counterpart for sorted lists, which performs a binary search. is set. The third parameter controls if the `util` pointer of the items should be freed or not. +`string_list_split_in_place`:: + + Split string into substrings on character delim and append the + substrings to a string_list. The delimiter characters in + string are overwritten with NULs in the process. If maxsplit + is a positive integer, then split at most maxsplit times. If + list.strdup_strings is not set, then the new string_list_items + point into string, which therefore must not be modified or + freed while the string_list is in use. Return the number of + substrings appended to the list. + + Data structures --- diff --git a/Makefile b/Makefile index 66e8216..ebbb381 100644 --- a/Makefile +++ b/Makefile @@ -501,6 +501,7 @@ TEST_PROGRAMS_NEED_X += test-run-command TEST_PROGRAMS_NEED_X += test-scrap-cache-tree TEST_PROGRAMS_NEED_X += test-sha1 TEST_PROGRAMS_NEED_X += test-sigchain +TEST_PROGRAMS_NEED_X += test-string-list TEST_PROGRAMS_NEED_X += test-subprocess TEST_PROGRAMS_NEED_X += test-svn-fe diff --git a/string-list.c b/string-list.c index d9810ab..110449c 100644 --- a/string-list.c +++ b/string-list.c @@ -194,3 +194,26 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_ list-items[i] = list-items[list-nr-1]; list-nr--; } + +int string_list_split_in_place(struct string_list *list, char *string, + int delim, int maxsplit) +{ + int count = 0; + char *p = string, *end; + for (;;) { + count++; + if (maxsplit 0 count maxsplit) { + string_list_append(list, p); + return count; + } + end = strchr(p, delim); + if (end) { + *end = '\0'; + string_list_append(list, p); + p = end + 1; + } else { + string_list_append(list, p); + return count; + } + } +} diff --git a/string-list.h b/string-list.h index 0684cb7..7e51d03 100644 --- a/string-list.h +++ b/string-list.h @@ -45,4 +45,23 @@ int unsorted_string_list_has_string(struct string_list *list, const char *string struct string_list_item *unsorted_string_list_lookup(struct string_list *list, const char *string); void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util); + +/* + * Split string into substrings on character delim and append the + * substrings to list. The delimiter characters in string are + * overwritten with NULs in the process. If maxsplit is a positive + * integer, then split at most maxsplit times. If list.strdup_strings + * is not set, then the new string_list_items point into string, which + * therefore must not be modified or freed while the string_list + * is in use. Return the number of substrings appended to list. + * + * Examples: + * string_list_split_in_place(l, foo:bar:baz, ':', -1) - [foo, bar, baz] + * string_list_split_in_place(l, foo:bar:baz, ':', 1) - [foo, bar:baz] + * string_list_split_in_place(l, foo:bar:, ':', -1) - [foo, bar, ] + * string_list_split_in_place(l, , ':',
[PATCH 3/4] Add a new function, string_list_remove_duplicates()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-string-list.txt | 4 string-list.c | 17 + string-list.h | 5 + 3 files changed, 26 insertions(+) diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt index 15b8072..9206f8f 100644 --- a/Documentation/technical/api-string-list.txt +++ b/Documentation/technical/api-string-list.txt @@ -104,6 +104,10 @@ write `string_list_insert(...)-util = ...;`. Look up a given string in the string_list, returning the containing string_list_item. If the string is not found, NULL is returned. +`string_list_remove_duplicates`:: + + Remove all but the first entry that has a given string value. + * Functions for unsorted lists only `string_list_append`:: diff --git a/string-list.c b/string-list.c index 72610ce..bfef6cf 100644 --- a/string-list.c +++ b/string-list.c @@ -92,6 +92,23 @@ struct string_list_item *string_list_lookup(struct string_list *list, const char return list-items + i; } +void string_list_remove_duplicates(struct string_list *list, int free_util) +{ + if (list-nr 1) { + int src, dst; + for (src = dst = 1; src list-nr; src++) { + if (!strcmp(list-items[dst - 1].string, list-items[src].string)) { + if (list-strdup_strings) + free(list-items[src].string); + if (free_util) + free(list-items[src].util); + } else + list-items[dst++] = list-items[src]; + } + list-nr = dst; + } +} + int for_each_string_list(struct string_list *list, string_list_each_func_t fn, void *cb_data) { diff --git a/string-list.h b/string-list.h index 84996aa..c4dc659 100644 --- a/string-list.h +++ b/string-list.h @@ -38,6 +38,7 @@ int for_each_string_list(struct string_list *list, void filter_string_list(struct string_list *list, int free_util, string_list_each_func_t fn, void *cb_data); + /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); int string_list_find_insert_index(const struct string_list *list, const char *string, @@ -47,6 +48,10 @@ struct string_list_item *string_list_insert_at_index(struct string_list *list, int insert_at, const char *string); struct string_list_item *string_list_lookup(struct string_list *list, const char *string); +/* Remove all but the first entry that has a given string value. */ +void string_list_remove_duplicates(struct string_list *list, int free_util); + + /* Use these functions only on unsorted lists: */ struct string_list_item *string_list_append(struct string_list *list, const char *string); void sort_string_list(struct string_list *list); -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] Add a new function, filter_string_list()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-string-list.txt | 8 string-list.c | 17 + string-list.h | 9 + 3 files changed, 34 insertions(+) diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt index 3b959a2..15b8072 100644 --- a/Documentation/technical/api-string-list.txt +++ b/Documentation/technical/api-string-list.txt @@ -60,6 +60,14 @@ Functions * General ones (works with sorted and unsorted lists as well) +`filter_string_list`:: + + Apply a function to each item in a list, retaining only the + items for which the function returns true. If free_util is + true, call free() on the util members of any items that have + to be deleted. Preserve the order of the items that are + retained. + `print_string_list`:: Dump a string_list to stdout, useful mainly for debugging purposes. It diff --git a/string-list.c b/string-list.c index 110449c..72610ce 100644 --- a/string-list.c +++ b/string-list.c @@ -102,6 +102,23 @@ int for_each_string_list(struct string_list *list, return ret; } +void filter_string_list(struct string_list *list, int free_util, + string_list_each_func_t fn, void *cb_data) +{ + int src, dst = 0; + for (src = 0; src list-nr; src++) { + if (fn(list-items[src], cb_data)) { + list-items[dst++] = list-items[src]; + } else { + if (list-strdup_strings) + free(list-items[src].string); + if (free_util) + free(list-items[src].util); + } + } + list-nr = dst; +} + void string_list_clear(struct string_list *list, int free_util) { if (list-items) { diff --git a/string-list.h b/string-list.h index 7e51d03..84996aa 100644 --- a/string-list.h +++ b/string-list.h @@ -29,6 +29,15 @@ int for_each_string_list(struct string_list *list, #define for_each_string_list_item(item,list) \ for (item = (list)-items; item (list)-items + (list)-nr; ++item) +/* + * Apply fn to each item in list, retaining only the ones for which + * the function returns true. If free_util is true, call free() on + * the util members of any items that have to be deleted. Preserve + * the order of the items that are retained. + */ +void filter_string_list(struct string_list *list, int free_util, + string_list_each_func_t fn, void *cb_data); + /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); int string_list_find_insert_index(const struct string_list *list, const char *string, -- 1.7.11.3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] Add a function string_list_longest_prefix()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-string-list.txt | 8 string-list.c | 20 +++ string-list.h | 8 t/t0063-string-list.sh | 30 + test-string-list.c | 22 + 5 files changed, 88 insertions(+) diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt index 9206f8f..291ac4c 100644 --- a/Documentation/technical/api-string-list.txt +++ b/Documentation/technical/api-string-list.txt @@ -68,6 +68,14 @@ Functions to be deleted. Preserve the order of the items that are retained. +`string_list_longest_prefix`:: + + Return the longest string within a string_list that is a + prefix (in the sense of prefixcmp()) of the specified string, + or NULL if no such prefix exists. This function does not + require the string_list to be sorted (it does a linear + search). + `print_string_list`:: Dump a string_list to stdout, useful mainly for debugging purposes. It diff --git a/string-list.c b/string-list.c index bfef6cf..043f6c4 100644 --- a/string-list.c +++ b/string-list.c @@ -136,6 +136,26 @@ void filter_string_list(struct string_list *list, int free_util, list-nr = dst; } +char *string_list_longest_prefix(const struct string_list *prefixes, +const char *string) +{ + int i, max_len = -1; + char *retval = NULL; + + for (i = 0; i prefixes-nr; i++) { + char *prefix = prefixes-items[i].string; + if (!prefixcmp(string, prefix)) { + int len = strlen(prefix); + if (len max_len) { + retval = prefix; + max_len = len; + } + } + } + + return retval; +} + void string_list_clear(struct string_list *list, int free_util) { if (list-items) { diff --git a/string-list.h b/string-list.h index c4dc659..680916c 100644 --- a/string-list.h +++ b/string-list.h @@ -38,6 +38,14 @@ int for_each_string_list(struct string_list *list, void filter_string_list(struct string_list *list, int free_util, string_list_each_func_t fn, void *cb_data); +/* + * Return the longest string in prefixes that is a prefix (in the + * sense of prefixcmp()) of string, or NULL if no such prefix exists. + * This function does not require the string_list to be sorted (it + * does a linear search). + */ +char *string_list_longest_prefix(const struct string_list *prefixes, const char *string); + /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh index 0eede83..fa96eba 100755 --- a/t/t0063-string-list.sh +++ b/t/t0063-string-list.sh @@ -15,6 +15,14 @@ string_list_split_in_place() { } +longest_prefix() { + test $(test-string-list longest_prefix $1 $2) = $3 +} + +no_longest_prefix() { + test_must_fail test-string-list longest_prefix $1 $2 +} + string_list_split_in_place foo:bar:baz : -1 EOF 3 [0]: foo @@ -60,4 +68,26 @@ string_list_split_in_place : : -1 EOF [1]: EOF +test_expect_success test longest_prefix ' + no_longest_prefix - '' + no_longest_prefix - x + longest_prefix x + longest_prefix x x x + longest_prefix foo + longest_prefix : foo + longest_prefix f foo f + longest_prefix foo foobar foo + longest_prefix foo foo foo + no_longest_prefix bar foo + no_longest_prefix bar:bar foo + no_longest_prefix foobar foo + longest_prefix foo:bar foo foo + longest_prefix foo:bar bar bar + longest_prefix foo::bar foo foo + longest_prefix foo:foobar foo foo + longest_prefix foobar:foo foo foo + longest_prefix foo: bar + longest_prefix :foo bar +' + test_done diff --git a/test-string-list.c b/test-string-list.c index f08d3cc..c7e71f2 100644 --- a/test-string-list.c +++ b/test-string-list.c @@ -19,6 +19,28 @@ int main(int argc, char **argv) return 0; } + if (argc == 4 !strcmp(argv[1], longest_prefix)) { + /* arguments: colon-separated-prefixes|- string */ + struct string_list prefixes = STRING_LIST_INIT_NODUP; + int retval; + char *prefix_string = xstrdup(argv[2]); + char *string = argv[3]; + char *match; + + if (strcmp(prefix_string, -)) + string_list_split_in_place(prefixes, prefix_string, ':', -1); + match = string_list_longest_prefix(prefixes, string); + if (match) { +