Re: [PATCH v2] Add a Windows-specific fallback to getenv(HOME);
Hi, On Thu, Jun 05, 2014 at 11:44:22PM +0200, Karsten Blees wrote: I think the most time-preserving option is to send it upstream as unchanged as possible (probably with the bugfix-patches squashed). I plan to submit one by one or in a small series. Agreed about the squashes, I have several of them prepared, thanks to your hints. Stepan -- To unsubscribe from this list: send the line 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: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function
Hello, On Fri, Jun 06, 2014 at 12:00:51AM +0200, Karsten Blees wrote: Am 05.06.2014 18:56, schrieb Johannes Sixt: Within mingw.c, if some other function inside mingw.c wants to use mingw_unlink, then it should be written as 'mingw_unlink(foo)', not 'unlink(foo)'. I very much like this approach. In fact, we already do this for e.g. mingw_raise. Hannes, this is consistent with your commit 06bc4b7. Settled. Other callers would typically want the wrapped version (i.e. mingw_*). If this assumption were true, then we have to keep the wrapper macros defined, both above and below the wrapper function definition. You are in fact advocating my patch. Updated version follows. Stepan -- To unsubscribe from this list: send the line 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] mingw: redefine the wrapper macro after the corresponding function
From 624d15bd5d2d06035abc17415127a7cf37b5f981 Mon Sep 17 00:00:00 2001
From: Stepan Kasal ka...@ucw.cz
Date: Thu, 5 Jun 2014 09:17:17 +0200
mingw.c defines several wrapper functions, e.g., mingw_unlink().
These wrappers are deployed by macros like this:
#define unlink mingw_unlink
The mingw_foo() wrapper often calls the original function, so it has
to be #undef'ed at that place.
But for the rest of mingw.c, if the function is used, the user probably
meant the fixed functionality. So it is safer to redefine the macro
back.
Nonetheless, it is preferable to call mingw_foo() explicitly throughout
mingw.c itself.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..ee83211 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -224,6 +224,7 @@ int mingw_unlink(const char *pathname)
ret = unlink(pathname);
return ret;
}
+#define unlink mingw_unlink
static int is_dir_empty(const char *path)
{
@@ -279,6 +280,7 @@ int mingw_rmdir(const char *pathname)
ret = rmdir(pathname);
return ret;
}
+#define rmdir mingw_rmdir
#undef open
int mingw_open (const char *filename, int oflags, ...)
@@ -303,6 +305,7 @@ int mingw_open (const char *filename, int oflags, ...)
}
return fd;
}
+#define open mingw_open
static BOOL WINAPI ctrl_ignore(DWORD type)
{
@@ -328,6 +331,7 @@ int mingw_fgetc(FILE *stream)
SetConsoleCtrlHandler(ctrl_ignore, FALSE);
return ch;
}
+#define fgetc mingw_fgetc
#undef fopen
FILE *mingw_fopen (const char *filename, const char *otype)
@@ -336,6 +340,7 @@ FILE *mingw_fopen (const char *filename, const char *otype)
filename = nul;
return fopen(filename, otype);
}
+#define fopen mingw_fopen
#undef freopen
FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream)
@@ -344,6 +349,7 @@ FILE *mingw_freopen (const char *filename, const char
*otype, FILE *stream)
filename = nul;
return freopen(filename, otype, stream);
}
+#define freopen mingw_freopen
#undef fflush
int mingw_fflush(FILE *stream)
@@ -366,6 +372,7 @@ int mingw_fflush(FILE *stream)
return ret;
}
+#define fflush mingw_fflush
/*
* The unit of FILETIME is 100-nanoseconds since January 1, 1601, UTC.
@@ -525,7 +532,7 @@ int mingw_utime (const char *file_name, const struct
utimbuf *times)
SetFileAttributes(file_name, attrs ~FILE_ATTRIBUTE_READONLY);
}
- if ((fh = open(file_name, O_RDWR | O_BINARY)) 0) {
+ if ((fh = mingw_open(file_name, O_RDWR | O_BINARY)) 0) {
rc = -1;
goto revert_attrs;
}
@@ -564,7 +571,7 @@ int mkstemp(char *template)
char *filename = mktemp(template);
if (filename == NULL)
return -1;
- return open(filename, O_RDWR | O_CREAT, 0600);
+ return mingw_open(filename, O_RDWR | O_CREAT, 0600);
}
int gettimeofday(struct timeval *tv, void *tz)
@@ -629,6 +636,7 @@ char *mingw_getcwd(char *pointer, int len)
pointer[i] = '/';
return ret;
}
+#define getcwd mingw_getcwd
/*
* See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
@@ -700,7 +708,7 @@ static const char *parse_interpreter(const char *cmd)
if (n = 4 !strcasecmp(cmd+n-4, .exe))
return NULL;
- fd = open(cmd, O_RDONLY);
+ fd = mingw_open(cmd, O_RDONLY);
if (fd 0)
return NULL;
n = read(fd, buf, sizeof(buf)-1);
@@ -1183,6 +1191,7 @@ char *mingw_getenv(const char *name)
}
return result;
}
+#define getenv mingw_getenv
/*
* Note, this isn't a complete replacement for getaddrinfo. It assumes
@@ -1199,7 +1208,7 @@ static int WSAAPI getaddrinfo_stub(const char *node,
const char *service,
struct sockaddr_in *sin;
if (node) {
- h = gethostbyname(node);
+ h = mingw_gethostbyname(node);
if (!h)
return WSAGetLastError();
}
@@ -1366,6 +1375,7 @@ int mingw_gethostname(char *name, int namelen)
ensure_socket_initialization();
return gethostname(name, namelen);
}
+#define gethostname mingw_gethostname
#undef gethostbyname
struct hostent *mingw_gethostbyname(const char *host)
@@ -1373,6 +1383,7 @@ struct hostent *mingw_gethostbyname(const char *host)
ensure_socket_initialization();
return gethostbyname(host);
}
+#define gethostbyname mingw_gethostbyname
void mingw_freeaddrinfo(struct addrinfo *res)
{
@@ -1429,6 +1440,7 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t
sz)
SOCKET s = (SOCKET)_get_osfhandle(sockfd);
return connect(s, sa, sz);
}
+#define connect mingw_connect
#undef bind
int mingw_bind(int sockfd, struct sockaddr *sa, size_t
git-svn, different merge-parent selected in independent clones following SVN merge
Hi, git version 1.8.3.2 I've used git-svn on a few repositories for a long time. In what is a testament to the consistency and stability of git-svn, despite often maintaining separate git-svn clones on different machines, I've never once seen a different commit-sha in independent clones for the same SVN revision. That is until just now. Two of three clones have the same commit-sha, the third has a different sha for the same SVN revision (and as expected for all subsequent commits). All of the tree-shas are identical. I never mix/push/pull local commits between these independent clones. The commit where the commit-sha diverged on the one clone is an SVN commit that involved a branch merge. The difference in this clone is that git-svn selected a different parent commit for one of the two parents of the merge (the other parent is the same). For that commit, the two clones that agreed had: commit de2dbe2e045f626cff8e282da1660c239b926765 tree f57a4d8b88262e1f1cd79f98c7d2f5ae82751636 parent f058fd2e05a2ef54c6bf056a3d2a46d17735253d parent e2722a1a24b490dbc8d7375b64050f1a0c010018 and the one that did not had: commit a3cfdff262b6afe8b22acd92f01554294f98c851 tree f57a4d8b88262e1f1cd79f98c7d2f5ae82751636 parent f058fd2e05a2ef54c6bf056a3d2a46d17735253d parent be09b04a3fa582ba12420e0a9b9c3233b41b600d (tree and one parent same, other parent and therefore new commit sha differ) On investigation, I found that commit be09b0 is actually an ancestor of e2722a, and the parent commit of e2722a is another (SVN) merge commit, with be09b0 as one of the parents. My best guess is that this can happen when git-svn rebase-ing trunk/master, when the associated branch isn't fully fetched. I often run git svn fetch on these clones, which fetches all branches, before git svn rebase but there's a chance that I've occasionally run git svn rebase on master (trunk) on its own, which only fetches trunk. So my questions are: 1) Does that sound like the most likely way this would happen - seeing an SVN merge hit trunk during an git svn rebase on master, when the merged branch was not completely git svn fetch-ed? And on the other agreeing clones, the better parent commit had already been picked-up as part of a git svn fetch? 2) If so, is there any reasonable way to prevent this ... I guess making sure (perhaps via an alias) that svn rebases occur only via git svn fetch followed by git svn rebase -l . Thanks Brett -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git for Windows SDK
- Original Message - From: Johannes Schindelin johannes.schinde...@gmx.de was Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME); snip Seriously again, I am in favor of calling it the Git for Windows SDK. But really, it is bikeshedding at this point. There is real work to do, still, before we can switch. Lots of unaddressed questions. Too little time. Speaking of which... budget's depleted for today ;-) Ciao, Dscho -- I like the Git for Windows SDK name. For me it communicates the intent very well and reflects in my mind the wider world naming conventuion for such things. It probably isn't an IDE in the wider sense, though I'm sure it could be, depending on viewpoint. so +1 on the G4W SDK. /bikeshedding Philip -- To unsubscribe from this list: send the line 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: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function
Am 06.06.2014 10:32, schrieb Stepan Kasal:
Hello,
On Fri, Jun 06, 2014 at 12:00:51AM +0200, Karsten Blees wrote:
Am 05.06.2014 18:56, schrieb Johannes Sixt:
Within mingw.c, if some other function inside mingw.c wants to use
mingw_unlink, then it should be written as 'mingw_unlink(foo)', not
'unlink(foo)'.
I very much like this approach. In fact, we already do this for e.g.
mingw_raise.
Hannes, this is consistent with your commit 06bc4b7. Settled.
Other callers would typically want the wrapped version (i.e.
mingw_*).
If this assumption were true, then we have to keep the wrapper macros
defined, both above and below the wrapper function definition.
That's not what I meant. Assume all other callers are written 'mingw_foo', as
suggested by Hannes, and no one except 'mingw_foo' has the need to call
MSVCRT's 'foo' directly. Then its irrelevant whether the #undef is at the top
or immediately before 'mingw_foo'. Having the #undef in close vicinity of the
function definition helps removing it when its no longer needed.
Thinking about this some more, the best solution is probably to eliminate the
problem altogether by adding inline-wrappers for required CRT-functions, e.g.:
mingw.h:
static inline int crt_gethostname(char *host, int namelen)
{
return gethostname(host, namelen);
}
int mingw_gethostname(char *host, int namelen);
#define gethostname mingw_gethostname
mingw.c:
int mingw_gethostname(char *name, int namelen)
{
ensure_socket_initialization();
return crt_gethostname(name, namelen);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git autocorrect bug
On Thu, Jun 05, 2014 at 04:28:23AM -0400, David Turner wrote:
On Thu, 2014-06-05 at 13:29 +0700, Duy Nguyen wrote:
On Thu, Jun 5, 2014 at 10:49 AM, David Turner dtur...@twopensource.com
wrote:
fatal: internal error: work tree has already been set
Current worktree: /home/dturner/git
New worktree: /home/dturner/git/foo
This is the part you complain about, right?
Yes.
I think I might know
what's going on here. But do you expect git git foo to turn to git
init foo in the first place?
Yes.
I was hoping you would say no so I could get away without doing
anything :) The problem is setup pollution. When somebody looks up
an alias (autocorrect does), $GIT_DIR must be searched because
$GIT_DIR/config may have repo-local aliases. But 'git init' (and
clone) expects a clean no-setup state.
This is a known issue. You can reproduce by aliasing init to
something, then init a new repo using that alias. In fact Jonathan
wrote a few test to catch this. The solution is we start out fresh in
a new process. The fork/exec overhead should not matter because this
is interactive session.
I'm just wondering if I should remove the only applicable to init and
clone check in the patch because there's another companion problem:
if we find $GIT_DIR automatically, then $GIT_DIR/config points out
that work-tree must be moved, it'll get nasty because we already set
everything up for the auto-found worktree. But maybe I already solved
that, not sure..
-- 8 --
diff --git a/compat/mingw.c b/compat/mingw.c
index e9892f8..34722fe 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1080,19 +1080,6 @@ int mingw_kill(pid_t pid, int sig)
return -1;
}
-static char **copy_environ(void)
-{
- char **env;
- int i = 0;
- while (environ[i])
- i++;
- env = xmalloc((i+1)*sizeof(*env));
- for (i = 0; environ[i]; i++)
- env[i] = xstrdup(environ[i]);
- env[i] = NULL;
- return env;
-}
-
void free_environ(char **env)
{
int i;
diff --git a/git-compat-util.h b/git-compat-util.h
index 76910e6..1db4dec 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -732,4 +732,6 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define gmtime_r git_gmtime_r
#endif
+char **copy_environ(void);
+
#endif
diff --git a/git.c b/git.c
index 7780572..77d9204 100644
--- a/git.c
+++ b/git.c
@@ -20,6 +20,22 @@ const char git_more_info_string[] =
static struct startup_info git_startup_info;
static int use_pager = -1;
+static char orig_cwd[PATH_MAX];
+static char **orig_environ;
+
+static void save_env(void)
+{
+ getcwd(orig_cwd, sizeof(orig_cwd));
+ orig_environ = copy_environ();
+}
+
+static void restore_env(void)
+{
+ if (*orig_cwd chdir(orig_cwd))
+ die_errno(could not move to %s, orig_cwd);
+ if (orig_environ)
+ environ = orig_environ;
+}
static void commit_pager_choice(void) {
switch (use_pager) {
@@ -459,7 +475,7 @@ int is_builtin(const char *s)
return 0;
}
-static void handle_builtin(int argc, const char **argv)
+static void handle_builtin(int argc, const char **argv, int preprocessed)
{
const char *cmd = argv[0];
int i;
@@ -484,6 +500,11 @@ static void handle_builtin(int argc, const char **argv)
struct cmd_struct *p = commands+i;
if (strcmp(p-cmd, cmd))
continue;
+ if (preprocessed
+ (p-fn == cmd_init_db || p-fn == cmd_clone)) {
+ restore_env();
+ break;
+ }
exit(run_builtin(p, argc, argv));
}
}
@@ -524,13 +545,13 @@ static void execv_dashed_external(const char **argv)
strbuf_release(cmd);
}
-static int run_argv(int *argcp, const char ***argv)
+static int run_argv(int *argcp, const char ***argv, int done_help)
{
int done_alias = 0;
while (1) {
/* See if it's a builtin */
- handle_builtin(*argcp, *argv);
+ handle_builtin(*argcp, *argv, done_help || done_alias);
/* .. then try the external ones */
execv_dashed_external(*argv);
@@ -539,7 +560,10 @@ static int run_argv(int *argcp, const char ***argv)
* of overriding git log with git show by having
* alias.log = show
*/
- if (done_alias || !handle_alias(argcp, argv))
+ if (done_alias)
+ break;
+ save_env();
+ if (!handle_alias(argcp, argv))
break;
done_alias = 1;
}
@@ -581,7 +605,7 @@ int main(int argc, char **av)
if (starts_with(cmd, git-)) {
cmd += 4;
argv[0] = cmd;
- handle_builtin(argc, argv);
+ handle_builtin(argc, argv, 0);
die(cannot handle %s as a builtin,
Re: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function
Hi Karsten,
On Fri, Jun 06, 2014 at 11:43:03AM +0200, Karsten Blees wrote:
[...] Assume all other callers are written
'mingw_foo', as suggested by Hannes, and no one except 'mingw_foo'
has the need to call MSVCRT's 'foo' directly. Then its irrelevant
whether the #undef is at the top or immediately before 'mingw_foo'.
Yet there is still danger that someone calls foo() by mistake.
It is still best to have a protection:
#define foo choke_here_do_not_use_this
Thinking about this some more, the best solution is probably to
eliminate the problem altogether by adding inline-wrappers for
required CRT-functions, e.g.:
Yes, this is acceptable. But I wouldn't pollute mingw.h. You can do
it on top of mingw.c like this:
#undef gethostname
static inline int crt_gethostname(char *host, int namelen)
{
return gethostname(host, namelen);
}
#define gethostname please_call_the_mingw_or_crt_version
This would also be an acceptable solution, though I still prefer my
solution, because, as you put it:
Having the #undef in close vicinity of the function definition
helps removing it when it's no longer needed.
Stepan
PS: Anyway, this is another patch which I can mark as too much
discussion, try later. Then I can proceed and submit your unicode
branch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Kitchên Design Lancashire Reviews
Kitchên Design Lancashire Reviews. what a great kitchên and price Kitchên Design Lancashire Reviews can provide. -- View this message in context: http://git.661346.n2.nabble.com/Kitchen-Design-Lancashire-Reviews-tp7612483.html Sent from the git mailing list archive at Nabble.com. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] userdiff: support C# async methods and correct C# keywords
On Thu, Jun 5, 2014 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Sup Yut Sum ch3co...@gmail.com writes: async is in C# 5.0 foreach is in C# 1.0 instanceof is in Java. The similar keywords are typeof, is, as in C# 1.0 This one made me read it twice, until I realized you meant instanceof() is listed as keywords, but there is no such thing (it is in Java, though); in C# we use typeof() for similar purposes The original email was a bit hard to parse. Junio's clarification left out the C# keywords 'is' and 'as'. I suggest phrasing it like this: instanceof() is listed as keywords, but there is no such thing (it is in Java, though); in C# we use typeof(), 'is', and 'as for similar purposes -- To unsubscribe from this list: send the line 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/5] Detect console streams more reliably on Windows
From: Karsten Blees bl...@dcon.de
GetStdHandle(STD_OUTPUT_HANDLE) doesn't work for stderr if stdout is
redirected. Use _get_osfhandle of the FILE* instead.
_isatty() is true for all character devices (including parallel and serial
ports). Check return value of GetConsoleScreenBufferInfo instead to
reliably detect console handles (also don't initialize internal state from
an uninitialized CONSOLE_SCREEN_BUFFER_INFO structure if the function
fails).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/winansi.c | 50 ++
1 file changed, 26 insertions(+), 24 deletions(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index abe0fea..c4be401 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -25,27 +25,39 @@ static HANDLE console;
static WORD plain_attr;
static WORD attr;
static int negative;
+static FILE *last_stream = NULL;
-static void init(void)
+static int is_console(FILE *stream)
{
CONSOLE_SCREEN_BUFFER_INFO sbi;
+ HANDLE hcon;
static int initialized = 0;
- if (initialized)
- return;
- console = GetStdHandle(STD_OUTPUT_HANDLE);
- if (console == INVALID_HANDLE_VALUE)
- console = NULL;
+ /* use cached value if stream hasn't changed */
+ if (stream == last_stream)
+ return console != NULL;
- if (!console)
- return;
+ last_stream = stream;
+ console = NULL;
- GetConsoleScreenBufferInfo(console, sbi);
- attr = plain_attr = sbi.wAttributes;
- negative = 0;
+ /* get OS handle of the stream */
+ hcon = (HANDLE) _get_osfhandle(_fileno(stream));
+ if (hcon == INVALID_HANDLE_VALUE)
+ return 0;
+
+ /* check if its a handle to a console output screen buffer */
+ if (!GetConsoleScreenBufferInfo(hcon, sbi))
+ return 0;
+
+ if (!initialized) {
+ attr = plain_attr = sbi.wAttributes;
+ negative = 0;
+ initialized = 1;
+ }
- initialized = 1;
+ console = hcon;
+ return 1;
}
static int write_console(const char *str, size_t len)
@@ -292,12 +304,7 @@ int winansi_fputs(const char *str, FILE *stream)
{
int rv;
- if (!isatty(fileno(stream)))
- return fputs(str, stream);
-
- init();
-
- if (!console)
+ if (!is_console(stream))
return fputs(str, stream);
rv = ansi_emulate(str, stream);
@@ -315,12 +322,7 @@ int winansi_vfprintf(FILE *stream, const char *format,
va_list list)
char *buf = small_buf;
va_list cp;
- if (!isatty(fileno(stream)))
- goto abort;
-
- init();
-
- if (!console)
+ if (!is_console(stream))
goto abort;
va_copy(cp, list);
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Windows dirent patches
Hello, This is a series of dirent modifications, 4 tiny ones and one bigger. As the date indicates, these are battle tested in mysgit for several years. Regards, Stepan Karsten Blees (5): Win32 dirent: remove unused dirent.d_ino member Win32 dirent: remove unused dirent.d_reclen member Win32 dirent: change FILENAME_MAX to MAX_PATH Win32 dirent: clarify #include directives Win32 dirent: improve dirent implementation compat/win32/dirent.c | 116 -- compat/win32/dirent.h | 8 +--- config.mak.uname | 2 + 3 files changed, 59 insertions(+), 67 deletions(-) -- 2.0.0.9635.g0be03cb -- To unsubscribe from this list: send the line 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 5/5] Win32: Thread-safe windows console output
From: Karsten Blees bl...@dcon.de Winansi.c has many static variables that are accessed and modified from the [v][f]printf / fputs functions overridden in the file. This may cause multi threaded git commands that print to the console to produce corrupted output or even crash. Additionally, winansi.c doesn't override all functions that can be used to print to the console (e.g. fwrite, write, fputc are missing), so that ANSI escapes don't work properly for some git commands (e.g. git-grep). Instead of doing ANSI emulation in just a few wrapped functions on top of the IO API, let's plug into the IO system and take advantage of the thread safety inherent to the IO system. Redirect stdout and stderr to a pipe if they point to the console. A background thread reads from the pipe, handles ANSI escape sequences and UTF-8 to UTF-16 conversion, then writes to the console. The pipe-based stdout and stderr replacements must be set to unbuffered, as MSVCRT doesn't support line buffering and fully buffered streams are inappropriate for console output. Due to the byte-oriented pipe, ANSI escape sequences and multi-byte UTF-8 sequences can no longer be expected to arrive in one piece. Replace the string-based ansi_emulate() with a simple stateful parser (this also fixes colored diff hunk headers, which were broken as of commit 2efcc977). Override isatty to return true for the pipes redirecting to the console. Exec/spawn obtain the original console handle to pass to the next process via winansi_get_osfhandle(). All other overrides are gone, the default stdio implementations work as expected with the piped stdout/stderr descriptors. Global variables are either initialized on startup (single threaded) or exclusively modified by the background thread. Threads communicate through the pipe, no further synchronization is necessary. The background thread is terminated by disonnecting the pipe after flushing the stdio and pipe buffers. This doesn't work for anonymous pipes (created via CreatePipe), as DisconnectNamedPipe only works on the read end, which discards remaining data. Thus we have to setup the pipe manually, with the write end beeing the server (opened with CreateNamedPipe) and the read end the client (opened with CreateFile). Limitations: doesn't track reopened or duped file descriptors, i.e.: - fdopen(1/2) returns fully buffered streams - dup(1/2), dup2(1/2) returns normal pipe descriptors (i.e. isatty() = false, winansi_get_osfhandle won't return the original console handle) Currently, only the git-format-patch command uses xfdopen(xdup(1)) (see realstdout in builtin/log.c), but works well with these limitations. Many thanks to Atsushi Nakagawa at...@chejz.com for suggesting and reviewing the thread-exit-mechanism. Signed-off-by: Karsten Blees bl...@dcon.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- compat/mingw.c | 9 +- compat/mingw.h | 12 +- compat/winansi.c | 402 --- 3 files changed, 274 insertions(+), 149 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index c03bafa..831043e 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -865,9 +865,9 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env, memset(si, 0, sizeof(si)); si.cb = sizeof(si); si.dwFlags = STARTF_USESTDHANDLES; - si.hStdInput = (HANDLE) _get_osfhandle(fhin); - si.hStdOutput = (HANDLE) _get_osfhandle(fhout); - si.hStdError = (HANDLE) _get_osfhandle(fherr); + si.hStdInput = winansi_get_osfhandle(fhin); + si.hStdOutput = winansi_get_osfhandle(fhout); + si.hStdError = winansi_get_osfhandle(fherr); /* concatenate argv, quoting args as we go */ strbuf_init(args, 0); @@ -1861,4 +1861,7 @@ void mingw_startup() _setmode(_fileno(stdin), _O_BINARY); _setmode(_fileno(stdout), _O_BINARY); _setmode(_fileno(stderr), _O_BINARY); + + /* initialize Unicode console */ + winansi_init(); } diff --git a/compat/mingw.h b/compat/mingw.h index 96d15ca..82e75d3 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -317,14 +317,10 @@ int mingw_raise(int sig); * ANSI emulation wrappers */ -int winansi_fputs(const char *str, FILE *stream); -int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2))); -int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3))); -int winansi_vfprintf(FILE *stream, const char *format, va_list list); -#define fputs winansi_fputs -#define printf(...) winansi_printf(__VA_ARGS__) -#define fprintf(...) winansi_fprintf(__VA_ARGS__) -#define vfprintf winansi_vfprintf +void winansi_init(void); +int winansi_isatty(int fd); +HANDLE winansi_get_osfhandle(int fd); +#define isatty winansi_isatty /* * git specific compatibility diff --git a/compat/winansi.c b/compat/winansi.c index bec6713..598fa1a 100644 --- a/compat/winansi.c +++ b/compat/winansi.c @@ -4,18
[PATCH 0/5] First part of Unicode console support for msysgit
Hello, this is first part of the unicode support pathes from msysgit. The first three patches originate in Jun 2010, though some fixups from 2012 have been squashed in. The fourth one is just a trivial prerequisite for the last one, that was written in Jan 2012, with a fixup from Mar 2012. Regards, Stepan Karsten Blees (5): Support Unicode console output on Windows Detect console streams more reliably on Windows Warn if the Windows console font doesn't support Unicode Win32: move main macro to a function Win32: Thread-safe windows console output compat/mingw.c | 24 ++- compat/mingw.h | 24 +-- compat/winansi.c | 446 --- 3 files changed, 356 insertions(+), 138 deletions(-) -- 2.0.0.9635.g0be03cb -- To unsubscribe from this list: send the line 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 3/5] Warn if the Windows console font doesn't support Unicode
From: Karsten Blees bl...@dcon.de
Unicode console output won't display correctly with default settings
because the default console font (Terminal) only supports the system's
OEM charset. Unfortunately, this is a user specific setting, so it cannot
be easily fixed by e.g. some registry tricks in the setup program.
This change prints a warning on exit if console output contained non-ascii
characters and the console font is supposedly not a TrueType font (which
usually have decent Unicode support).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/winansi.c | 66
1 file changed, 66 insertions(+)
diff --git a/compat/winansi.c b/compat/winansi.c
index c4be401..bec6713 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -2,8 +2,11 @@
* Copyright 2008 Peter Harris g...@peter.is-a-geek.org
*/
+#undef NOGDI
#include ../git-compat-util.h
#include malloc.h
+#include wingdi.h
+#include winreg.h
/*
Functions to be wrapped:
@@ -27,6 +30,62 @@ static WORD attr;
static int negative;
static FILE *last_stream = NULL;
+#ifdef __MINGW32__
+typedef struct _CONSOLE_FONT_INFOEX {
+ ULONG cbSize;
+ DWORD nFont;
+ COORD dwFontSize;
+ UINT FontFamily;
+ UINT FontWeight;
+ WCHAR FaceName[LF_FACESIZE];
+} CONSOLE_FONT_INFOEX, *PCONSOLE_FONT_INFOEX;
+#endif
+
+typedef BOOL (WINAPI *PGETCURRENTCONSOLEFONTEX)(HANDLE, BOOL,
+ PCONSOLE_FONT_INFOEX);
+
+static void print_font_warning(void)
+{
+ warning(Your console font probably doesn\'t support Unicode. If
+ you experience strange characters in the output, consider
+ switching to a TrueType font such as Lucida Console!);
+}
+
+static void check_truetype_font(void)
+{
+ static int truetype_font_checked;
+ DWORD fontFamily = 0;
+ PGETCURRENTCONSOLEFONTEX pGetCurrentConsoleFontEx;
+
+ /* don't do this twice */
+ if (truetype_font_checked)
+ return;
+ truetype_font_checked = 1;
+
+ /* GetCurrentConsoleFontEx is available since Vista */
+ pGetCurrentConsoleFontEx = (PGETCURRENTCONSOLEFONTEX) GetProcAddress(
+ GetModuleHandle(kernel32.dll),
GetCurrentConsoleFontEx);
+ if (pGetCurrentConsoleFontEx) {
+ CONSOLE_FONT_INFOEX cfi;
+ cfi.cbSize = sizeof(cfi);
+ if (pGetCurrentConsoleFontEx(console, 0, cfi))
+ fontFamily = cfi.FontFamily;
+ } else {
+ /* pre-Vista: check default console font in registry */
+ HKEY hkey;
+ if (ERROR_SUCCESS == RegOpenKeyExA(HKEY_CURRENT_USER,
Console, 0,
+ KEY_READ, hkey)) {
+ DWORD size = sizeof(fontFamily);
+ RegQueryValueExA(hkey, FontFamily, NULL, NULL,
+ (LPVOID) fontFamily, size);
+ RegCloseKey(hkey);
+ }
+ }
+
+ if (!(fontFamily TMPF_TRUETYPE))
+ atexit(print_font_warning);
+}
+
static int is_console(FILE *stream)
{
CONSOLE_SCREEN_BUFFER_INFO sbi;
@@ -69,6 +128,13 @@ static int write_console(const char *str, size_t len)
WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+ /*
+* if non-ascii characters are printed, check that the current console
+* font supports this
+*/
+ if (wlen != len)
+ check_truetype_font();
+
/* return original (utf-8 encoded) length */
return len;
}
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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 5/5] Win32 dirent: improve dirent implementation
From: Karsten Blees bl...@dcon.de
Improve the dirent implementation by removing the relics that were once
necessary to plug into the now unused MinGW runtime, in preparation for
Unicode file name support.
Move FindFirstFile to opendir, and FindClose to closedir, with the
following implications:
- DIR.dd_name is no longer needed
- chdir(one); opendir(relative); chdir(two); readdir() works as expected
(i.e. lists one/relative instead of two/relative)
- DIR.dd_handle is a valid handle for the entire lifetime of the DIR struct
- thus, all checks for dd_handle == INVALID_HANDLE_VALUE and dd_handle == 0
have been removed
- the special case that the directory has been fully read (which was
previously explicitly tracked with dd_handle == INVALID_HANDLE_VALUE
dd_stat != 0) is now handled implicitly by the FindNextFile error
handling code (if a client continues to call readdir after receiving
NULL, FindNextFile will continue to fail with ERROR_NO_MORE_FILES, to
the same effect)
- extracting dirent data from WIN32_FIND_DATA is needed in two places, so
moved to its own method
- GetFileAttributes is no longer needed. The same information can be
obtained from the FindFirstFile error code, which is ERROR_DIRECTORY if
the name is NOT a directory (- ENOTDIR), otherwise we can use
err_win_to_posix (e.g. ERROR_PATH_NOT_FOUND - ENOENT). The
ERROR_DIRECTORY case could be fixed in err_win_to_posix, but this
probably breaks other functionality.
Removes the ERROR_NO_MORE_FILES check after FindFirstFile (this was
fortunately a NOOP (searching for '*' always finds '.' and '..'),
otherwise the subsequent code would have copied data from an uninitialized
buffer).
Changes malloc to git support function xmalloc, so opendir will die() if
out of memory, rather than failing with ENOMEM and letting git work on
incomplete directory listings (error handling in dir.c is quite sparse).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.c | 113 --
1 file changed, 54 insertions(+), 59 deletions(-)
diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
index fac7f25..82a515c 100644
--- a/compat/win32/dirent.c
+++ b/compat/win32/dirent.c
@@ -4,92 +4,88 @@ struct DIR {
struct dirent dd_dir; /* includes d_type */
HANDLE dd_handle; /* FindFirstFile handle */
int dd_stat; /* 0-based index */
- char dd_name[1]; /* extend struct */
};
+static inline void finddata2dirent(struct dirent *ent, WIN32_FIND_DATAA *fdata)
+{
+ /* copy file name from WIN32_FIND_DATA to dirent */
+ memcpy(ent-d_name, fdata-cFileName, sizeof(ent-d_name));
+
+ /* Set file type, based on WIN32_FIND_DATA */
+ if (fdata-dwFileAttributes FILE_ATTRIBUTE_DIRECTORY)
+ ent-d_type = DT_DIR;
+ else
+ ent-d_type = DT_REG;
+}
+
DIR *opendir(const char *name)
{
- DWORD attrs = GetFileAttributesA(name);
+ char pattern[MAX_PATH];
+ WIN32_FIND_DATAA fdata;
+ HANDLE h;
int len;
- DIR *p;
+ DIR *dir;
- /* check for valid path */
- if (attrs == INVALID_FILE_ATTRIBUTES) {
- errno = ENOENT;
+ /* check that name is not NULL */
+ if (!name) {
+ errno = EINVAL;
return NULL;
}
-
- /* check if it's a directory */
- if (!(attrs FILE_ATTRIBUTE_DIRECTORY)) {
- errno = ENOTDIR;
- return NULL;
- }
-
/* check that the pattern won't be too long for FindFirstFileA */
len = strlen(name);
- if (is_dir_sep(name[len - 1]))
- len--;
if (len + 2 = MAX_PATH) {
errno = ENAMETOOLONG;
return NULL;
}
-
- p = malloc(sizeof(DIR) + len + 2);
- if (!p)
+ /* copy name to temp buffer */
+ memcpy(pattern, name, len + 1);
+
+ /* append optional '/' and wildcard '*' */
+ if (len !is_dir_sep(pattern[len - 1]))
+ pattern[len++] = '/';
+ pattern[len++] = '*';
+ pattern[len] = 0;
+
+ /* open find handle */
+ h = FindFirstFileA(pattern, fdata);
+ if (h == INVALID_HANDLE_VALUE) {
+ DWORD err = GetLastError();
+ errno = (err == ERROR_DIRECTORY) ? ENOTDIR :
err_win_to_posix(err);
return NULL;
+ }
- memset(p, 0, sizeof(DIR) + len + 2);
- strcpy(p-dd_name, name);
- p-dd_name[len] = '/';
- p-dd_name[len+1] = '*';
-
- p-dd_handle = INVALID_HANDLE_VALUE;
- return p;
+ /* initialize DIR structure and copy first dir entry */
+ dir = xmalloc(sizeof(DIR));
+ dir-dd_handle = h;
+ dir-dd_stat = 0;
+ finddata2dirent(dir-dd_dir, fdata);
+ return dir;
}
struct dirent *readdir(DIR *dir)
{
-
[PATCH 1/5] Support Unicode console output on Windows
From: Karsten Blees bl...@dcon.de
WriteConsoleW seems to be the only way to reliably print unicode to the
console (without weird code page conversions).
Also redirects vfprintf to the winansi.c version.
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.h | 2 ++
compat/winansi.c | 26 --
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 3eaf822..a465d1e 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -320,9 +320,11 @@ int mingw_raise(int sig);
int winansi_fputs(const char *str, FILE *stream);
int winansi_printf(const char *format, ...) __attribute__((format (printf, 1,
2)));
int winansi_fprintf(FILE *stream, const char *format, ...)
__attribute__((format (printf, 2, 3)));
+int winansi_vfprintf(FILE *stream, const char *format, va_list list);
#define fputs winansi_fputs
#define printf(...) winansi_printf(__VA_ARGS__)
#define fprintf(...) winansi_fprintf(__VA_ARGS__)
+#define vfprintf winansi_vfprintf
/*
* git specific compatibility
diff --git a/compat/winansi.c b/compat/winansi.c
index dedce21..abe0fea 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -3,6 +3,7 @@
*/
#include ../git-compat-util.h
+#include malloc.h
/*
Functions to be wrapped:
@@ -10,6 +11,7 @@
#undef printf
#undef fprintf
#undef fputs
+#undef vfprintf
/* TODO: write */
/*
@@ -46,6 +48,18 @@ static void init(void)
initialized = 1;
}
+static int write_console(const char *str, size_t len)
+{
+ /* convert utf-8 to utf-16, write directly to console */
+ int wlen = MultiByteToWideChar(CP_UTF8, 0, str, len, NULL, 0);
+ wchar_t *wbuf = (wchar_t *) alloca(wlen * sizeof(wchar_t));
+ MultiByteToWideChar(CP_UTF8, 0, str, len, wbuf, wlen);
+
+ WriteConsoleW(console, wbuf, wlen, NULL, NULL);
+
+ /* return original (utf-8 encoded) length */
+ return len;
+}
#define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)
#define BACKGROUND_ALL (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE)
@@ -245,13 +259,15 @@ static int ansi_emulate(const char *str, FILE *stream)
int rv = 0;
const char *pos = str;
+ fflush(stream);
+
while (*pos) {
pos = strstr(str, \033[);
if (pos) {
size_t len = pos - str;
if (len) {
- size_t out_len = fwrite(str, 1, len, stream);
+ size_t out_len = write_console(str, len);
rv += out_len;
if (out_len len)
return rv;
@@ -260,14 +276,12 @@ static int ansi_emulate(const char *str, FILE *stream)
str = pos + 2;
rv += 2;
- fflush(stream);
-
pos = set_attr(str);
rv += pos - str;
str = pos;
} else {
- rv += strlen(str);
- fputs(str, stream);
+ size_t len = strlen(str);
+ rv += write_console(str, len);
return rv;
}
}
@@ -294,7 +308,7 @@ int winansi_fputs(const char *str, FILE *stream)
return EOF;
}
-static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
+int winansi_vfprintf(FILE *stream, const char *format, va_list list)
{
int len, rv;
char small_buf[256];
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32 dirent: clarify #include directives
From: Karsten Blees bl...@dcon.de
Git-compat-util.h is two dirs up, and already includes dirent.h (which
is the same as dirent.h due to -Icompat/win32 in the Makefile).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
index 7a0debe..fac7f25 100644
--- a/compat/win32/dirent.c
+++ b/compat/win32/dirent.c
@@ -1,5 +1,4 @@
-#include ../git-compat-util.h
-#include dirent.h
+#include ../../git-compat-util.h
struct DIR {
struct dirent dd_dir; /* includes d_type */
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32 dirent: remove unused dirent.d_ino member
From: Karsten Blees bl...@dcon.de
There are no proper inodes on Windows, so remove dirent.d_ino and #define
NO_D_INO_IN_DIRENT in the Makefile (this skips e.g. an ineffective qsort in
fsck.c).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 1 -
config.mak.uname | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index 927a25c..b38973b 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -9,7 +9,6 @@ typedef struct DIR DIR;
#define DT_LNK 3
struct dirent {
- long d_ino; /* Always zero. */
char d_name[FILENAME_MAX]; /* File name. */
union {
unsigned short d_reclen; /* Always zero. */
diff --git a/config.mak.uname b/config.mak.uname
index 1ae675b..8131c81 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -354,6 +354,7 @@ ifeq ($(uname_S),Windows)
NO_POSIX_GOODIES = UnfortunatelyYes
NATIVE_CRLF = YesPlease
DEFAULT_HELP_FORMAT = html
+ NO_D_INO_IN_DIRENT = YesPlease
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -503,6 +504,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_INET_NTOP = YesPlease
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
+ NO_D_INO_IN_DIRENT = YesPlease
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI
-Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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 3/5] Win32 dirent: change FILENAME_MAX to MAX_PATH
From: Karsten Blees bl...@dcon.de
FILENAME_MAX and MAX_PATH are both 260 on Windows, however, MAX_PATH is
used throughout the other Win32 code in Git, and also defines the length
of file name buffers in the Win32 API (e.g. WIN32_FIND_DATA.cFileName,
from which we're copying the dirent data).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index 7f4e6c7..8838cd6 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -9,8 +9,8 @@ typedef struct DIR DIR;
#define DT_LNK 3
struct dirent {
- char d_name[FILENAME_MAX]; /* File name. */
unsigned char d_type; /* file type to prevent lstat after readdir
*/
+ char d_name[MAX_PATH]; /* file name */
};
DIR *opendir(const char *dirname);
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32: move main macro to a function
From: Karsten Blees bl...@dcon.de
The code in the MinGW main macro is getting more and more complex, move to
a separate initialization function for readabiliy and extensibility.
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.c | 15 +++
compat/mingw.h | 14 --
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index a0e13bc..c03bafa 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1847,3 +1847,18 @@ int mingw_offset_1st_component(const char *path)
return offset + is_dir_sep(path[offset]);
}
+
+void mingw_startup()
+{
+ /* copy executable name to argv[0] */
+ __argv[0] = xstrdup(_pgmptr);
+
+ /* initialize critical section for waitpid pinfo_t list */
+ InitializeCriticalSection(pinfo_cs);
+
+ /* set up default file mode and file modes for stdin/out/err */
+ _fmode = _O_BINARY;
+ _setmode(_fileno(stdin), _O_BINARY);
+ _setmode(_fileno(stdout), _O_BINARY);
+ _setmode(_fileno(stderr), _O_BINARY);
+}
diff --git a/compat/mingw.h b/compat/mingw.h
index a465d1e..96d15ca 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -365,22 +365,16 @@ void free_environ(char **env);
extern CRITICAL_SECTION pinfo_cs;
/*
- * A replacement of main() that ensures that argv[0] has a path
- * and that default fmode and std(in|out|err) are in binary mode
+ * A replacement of main() that adds win32 specific initialization.
*/
+void mingw_startup();
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(c,v); \
int main(int argc, char **argv) \
{ \
- extern CRITICAL_SECTION pinfo_cs; \
- _fmode = _O_BINARY; \
- _setmode(_fileno(stdin), _O_BINARY); \
- _setmode(_fileno(stdout), _O_BINARY); \
- _setmode(_fileno(stderr), _O_BINARY); \
- argv[0] = xstrdup(_pgmptr); \
- InitializeCriticalSection(pinfo_cs); \
- return mingw_main(argc, argv); \
+ mingw_startup(); \
+ return mingw_main(__argc, __argv); \
} \
static int mingw_main(c,v)
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32 dirent: remove unused dirent.d_reclen member
From: Karsten Blees bl...@dcon.de
Remove the union around dirent.d_type and the unused dirent.d_reclen member
(which was necessary for compatibility with the MinGW dirent runtime, which
is no longer used).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index b38973b..7f4e6c7 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -10,10 +10,7 @@ typedef struct DIR DIR;
struct dirent {
char d_name[FILENAME_MAX]; /* File name. */
- union {
- unsigned short d_reclen; /* Always zero. */
- unsigned char d_type; /* Reimplementation adds this */
- };
+ unsigned char d_type; /* file type to prevent lstat after readdir
*/
};
DIR *opendir(const char *dirname);
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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: Proposal for pruning tags
On Thu, Jun 5, 2014 at 3:50 PM, Junio C Hamano gits...@pobox.com wrote: I think you need to explain what you mean by prune a lot better than what you are doing in your message to be understood by others. After seeing the above two commands, my *guess* of what you want to do is to remove any of your local tag that is *not* present in the repository you usually fetch from (aka origin), but that directly contradicts with what you said you wish, i.e. This is not only wasteful, but dangerous. I might accidentally delete a local tag I haven't pushed yet... which only shows that your definition of prune is different from remove what I do not have at 'origin'. But it does not say *how* that is different. How should prune behave differently from the two commands above? How does your prune decide a tag needs to be removed locally when it is not at your origin [*1*]? There is *nothing* in git that lets you look at a local tag that is missing from the other side and determine if that is something you did not want to push (hence it is missing there) of if that is something you forgot to push (hence it is missing there but you would rather have pushed if you did not forget). So you must have some new mechanism to record and/or infer that distinction in mind, but it is not clear what it is from your message. So until that is clarified, there is not much more to say if your feature has any merit---as there is no way to tell what that feature exactly is, at least not yet ;-) snip You're right I didn't clarify, although I feel you're not providing the most welcome response to someone who isn't as familiar with the internals of Git as you are. It was an oversight on my part. What I was expecting is that it would behave exactly like branch pruning does, but that would require remote tracking tags, which we don't have. So, apparently my idea doesn't hold much water. The general problem I see in the day to day workflow with my team is that if tags exist locally and they push, those tags continuously get recreated on the remote repo even after I delete them remotely. So I can never truly delete tags until I go to each person and make sure the tool they're using isn't accidentally pushing tags. For example, SourceTree pushes all tags by default. Everyone on my team is new to Git, so they don't know to turn that off. Having git clean up tags automatically would really help with this, even though you may not feel it's the responsibility of Git. It's more of a usability issue, it's just prone to error. I can setup my config to prune tracking branches after I pull. Having something like this for tags would be wonderful. However, this requires a bigger overhaul than what I initially was proposing. -- To unsubscribe from this list: send the line 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/3] pretty: free the gpg status buf
4a868fd (pretty: parse the gpg status lines rather than the output, 2013-02-14) made the gpg status lines available to callers and made sure they freed the used space, but missed one spot. Free the status line buffer also in the remaining spot. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- pretty.c | 1 + 1 file changed, 1 insertion(+) diff --git a/pretty.c b/pretty.c index 4f51287..f1e8a70 100644 --- a/pretty.c +++ b/pretty.c @@ -1538,6 +1538,7 @@ void format_commit_message(const struct commit *commit, free(context.commit_encoding); logmsg_free(context.message, commit); free(context.signature_check.gpg_output); + free(context.signature_check.gpg_status); free(context.signature_check.signer); } -- 2.0.0.533.gae2e602 -- To unsubscribe from this list: send the line 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/3] verify-commit: verify commit signatures
Hi there, Some of you may remember me from my more active times... Anyways, a recent blog post about signed commits in git triggered me to look at our tools for that again. It seems that we only have the log/pretty family on the user facing side, but everything we need under the hood. So here's a suggestion to implement verify-commit in a way which is completely analogous to verify-tag. In fact, it could be coded more elegantly, but I kept it this way so that we could merge the two more easily in case we wish to do so. I will follow up with tests if the design principle is something we agree upon. Michael J Gruber (3): pretty: free the gpg status buf gpg-interface: provide access to the payload verify-commit: scriptable commit signature verification Documentation/git-verify-commit.txt | 28 +++ Makefile| 1 + builtin.h | 1 + builtin/merge.c | 1 + builtin/verify-commit.c | 98 + command-list.txt| 1 + commit.c| 1 + git.c | 1 + gpg-interface.h | 1 + pretty.c| 2 + 10 files changed, 135 insertions(+) create mode 100644 Documentation/git-verify-commit.txt create mode 100644 builtin/verify-commit.c -- 2.0.0.533.gae2e602 -- To unsubscribe from this list: send the line 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 3/3] verify-commit: scriptable commit signature verification
Commit signatures can be verified using git show -s --show-signature
or the %G? pretty format and parsing the output, which is well suited
for user inspection, but not for scripting.
Provide a command verify-commit which is analogous to verify-tag: It
returns 0 for good signatures and non-zero otherwise, has the gpg output
on stderr and (optionally) the commit object on stdout, sans the
signature, just like verify-tag does.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Documentation/git-verify-commit.txt | 28 +++
Makefile| 1 +
builtin.h | 1 +
builtin/verify-commit.c | 98 +
command-list.txt| 1 +
git.c | 1 +
6 files changed, 130 insertions(+)
create mode 100644 Documentation/git-verify-commit.txt
create mode 100644 builtin/verify-commit.c
diff --git a/Documentation/git-verify-commit.txt
b/Documentation/git-verify-commit.txt
new file mode 100644
index 000..dcd7803
--- /dev/null
+++ b/Documentation/git-verify-commit.txt
@@ -0,0 +1,28 @@
+git-verify-commit(1)
+=
+
+NAME
+
+git-verify-commit - Check the GPG signature of commits
+
+SYNOPSIS
+
+[verse]
+'git verify-commit' commit...
+
+DESCRIPTION
+---
+Validates the gpg signature created by 'git commit -S'.
+
+OPTIONS
+---
+-v::
+--verbose::
+ Print the contents of the commit object before validating it.
+
+commit...::
+ SHA-1 identifiers of Git commit objects.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 07ea105..b92418d 100644
--- a/Makefile
+++ b/Makefile
@@ -999,6 +999,7 @@ BUILTIN_OBJS += builtin/update-ref.o
BUILTIN_OBJS += builtin/update-server-info.o
BUILTIN_OBJS += builtin/upload-archive.o
BUILTIN_OBJS += builtin/var.o
+BUILTIN_OBJS += builtin/verify-commit.o
BUILTIN_OBJS += builtin/verify-pack.o
BUILTIN_OBJS += builtin/verify-tag.o
BUILTIN_OBJS += builtin/write-tree.o
diff --git a/builtin.h b/builtin.h
index c47c110..5d91f31 100644
--- a/builtin.h
+++ b/builtin.h
@@ -128,6 +128,7 @@ extern int cmd_update_server_info(int argc, const char
**argv, const char *prefi
extern int cmd_upload_archive(int argc, const char **argv, const char *prefix);
extern int cmd_upload_archive_writer(int argc, const char **argv, const char
*prefix);
extern int cmd_var(int argc, const char **argv, const char *prefix);
+extern int cmd_verify_commit(int argc, const char **argv, const char *prefix);
extern int cmd_verify_tag(int argc, const char **argv, const char *prefix);
extern int cmd_version(int argc, const char **argv, const char *prefix);
extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
new file mode 100644
index 000..69b7c6d
--- /dev/null
+++ b/builtin/verify-commit.c
@@ -0,0 +1,98 @@
+/*
+ * Builtin git commit-commit
+ *
+ * Copyright (c) 2014 Michael J Gruber g...@drmicha.warpmail.net
+ *
+ * Based on git-verify-tag
+ */
+#include cache.h
+#include builtin.h
+#include commit.h
+#include run-command.h
+#include signal.h
+#include parse-options.h
+#include gpg-interface.h
+
+static const char * const verify_commit_usage[] = {
+ N_(git verify-commit [-v|--verbose] commit...),
+ NULL
+};
+
+static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned
long size, int verbose)
+{
+ struct signature_check signature_check;
+
+ memset(signature_check, 0, sizeof(signature_check));
+
+ check_commit_signature(lookup_commit(sha1), signature_check);
+
+ if (verbose signature_check.payload)
+ fputs(signature_check.payload, stdout);
+
+ if (signature_check.gpg_output)
+ fputs(signature_check.gpg_output, stderr);
+
+ free(signature_check.gpg_output);
+ free(signature_check.gpg_status);
+ free(signature_check.signer);
+ free(signature_check.key);
+ return signature_check.result != 'G';
+}
+
+static int verify_commit(const char *name, int verbose)
+{
+ enum object_type type;
+ unsigned char sha1[20];
+ char *buf;
+ unsigned long size;
+ int ret;
+
+ if (get_sha1(name, sha1))
+ return error(commit '%s' not found., name);
+
+ type = sha1_object_info(sha1, NULL);
+ if (type != OBJ_COMMIT)
+ return error(%s: cannot verify a non-commit object of type
%s.,
+ name, typename(type));
+
+ buf = read_sha1_file(sha1, type, size);
+ if (!buf)
+ return error(%s: unable to read file., name);
+
+ ret = run_gpg_verify(sha1, buf, size, verbose);
+
+ free(buf);
+ return ret;
+}
+
+static int git_verify_commit_config(const char *var, const char *value, void
*cb)
+{
+ int status = git_gpg_config(var, value, cb);
+ if (status)
[PATCH 2/3] gpg-interface: provide access to the payload
In contrast to tag signatures, commit signatures are put into the
header, that is between the other header parts and commit messages.
Provide access to the commit content sans the signature, which is the
payload that is actually signed. Commit signature verification does the
parsing anyways, and callers may wish to act on or display the commit
object sans the signature.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
builtin/merge.c | 1 +
commit.c| 1 +
gpg-interface.h | 1 +
pretty.c| 1 +
4 files changed, 4 insertions(+)
diff --git a/builtin/merge.c b/builtin/merge.c
index 428ca24..6a9812a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1282,6 +1282,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
printf(_(Commit %s has a good GPG signature by
%s\n),
hex, signature_check.signer);
+ free(signature_check.payload);
free(signature_check.gpg_output);
free(signature_check.gpg_status);
free(signature_check.signer);
diff --git a/commit.c b/commit.c
index f479331..e9686b2 100644
--- a/commit.c
+++ b/commit.c
@@ -1219,6 +1219,7 @@ void check_commit_signature(const struct commit* commit,
struct signature_check
gpg_output, gpg_status);
if (status !gpg_output.len)
goto out;
+ sigc-payload = strbuf_detach(payload, NULL);
sigc-gpg_output = strbuf_detach(gpg_output, NULL);
sigc-gpg_status = strbuf_detach(gpg_status, NULL);
parse_gpg_output(sigc);
diff --git a/gpg-interface.h b/gpg-interface.h
index a85cb5b..d727c25 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -2,6 +2,7 @@
#define GPG_INTERFACE_H
struct signature_check {
+ char *payload;
char *gpg_output;
char *gpg_status;
char result; /* 0 (not checked),
diff --git a/pretty.c b/pretty.c
index f1e8a70..24fb877 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1537,6 +1537,7 @@ void format_commit_message(const struct commit *commit,
free(context.commit_encoding);
logmsg_free(context.message, commit);
+ free(context.signature_check.payload);
free(context.signature_check.gpg_output);
free(context.signature_check.gpg_status);
free(context.signature_check.signer);
--
2.0.0.533.gae2e602
--
To unsubscribe from this list: send the line 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 04/20] contrib/examples/git-merge.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
contrib/examples/git-merge.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh
index 7e40f40..52f2aaf 100755
--- a/contrib/examples/git-merge.sh
+++ b/contrib/examples/git-merge.sh
@@ -161,7 +161,7 @@ merge_name () {
return
fi
fi
- if test $remote = FETCH_HEAD -a -r $GIT_DIR/FETCH_HEAD
+ if test $remote = FETCH_HEAD test -r $GIT_DIR/FETCH_HEAD
then
sed -e 's/ not-for-merge / /' -e 1q \
$GIT_DIR/FETCH_HEAD
@@ -527,7 +527,7 @@ do
git diff-files --name-only
git ls-files --unmerged
} | wc -l`
- if test $best_cnt -le 0 -o $cnt -le $best_cnt
+ if test $best_cnt -le 0 || test $cnt -le $best_cnt
then
best_strategy=$strategy
best_cnt=$cnt
--
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
[PATCH 11/20] t/lib-httpd.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
t/lib-httpd.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 252cbf1..38a47fe 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -132,7 +132,7 @@ prepare_httpd() {
HTTPD_URL_USER=$HTTPD_PROTO://user%40host@$HTTPD_DEST
HTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:pass%40host@$HTTPD_DEST
- if test -n $LIB_HTTPD_DAV -o -n $LIB_HTTPD_SVN
+ if test -n $LIB_HTTPD_DAV || test -n $LIB_HTTPD_SVN
then
HTTPD_PARA=$HTTPD_PARA -DDAV
--
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
[PATCH 16/20] t/t5403-post-checkout-hook.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
t/t5403-post-checkout-hook.sh |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1753ef2..fc898c9 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -39,7 +39,7 @@ test_expect_success 'post-checkout receives the right
arguments with HEAD unchan
old=$(awk {print \$1} clone1/.git/post-checkout.args)
new=$(awk {print \$2} clone1/.git/post-checkout.args)
flag=$(awk {print \$3} clone1/.git/post-checkout.args)
- test $old = $new -a $flag = 1
+ test $old = $new test $flag = 1
'
test_expect_success 'post-checkout runs as expected ' '
@@ -52,7 +52,7 @@ test_expect_success 'post-checkout args are correct with git
checkout -b ' '
old=$(awk {print \$1} clone1/.git/post-checkout.args)
new=$(awk {print \$2} clone1/.git/post-checkout.args)
flag=$(awk {print \$3} clone1/.git/post-checkout.args)
- test $old = $new -a $flag = 1
+ test $old = $new test $flag = 1
'
test_expect_success 'post-checkout receives the right args with HEAD changed '
'
@@ -60,7 +60,7 @@ test_expect_success 'post-checkout receives the right args
with HEAD changed ' '
old=$(awk {print \$1} clone2/.git/post-checkout.args)
new=$(awk {print \$2} clone2/.git/post-checkout.args)
flag=$(awk {print \$3} clone2/.git/post-checkout.args)
- test $old != $new -a $flag = 1
+ test $old != $new test $flag = 1
'
test_expect_success 'post-checkout receives the right args when not switching
branches ' '
@@ -68,7 +68,7 @@ test_expect_success 'post-checkout receives the right args
when not switching br
old=$(awk {print \$1} clone2/.git/post-checkout.args)
new=$(awk {print \$2} clone2/.git/post-checkout.args)
flag=$(awk {print \$3} clone2/.git/post-checkout.args)
- test $old = $new -a $flag = 0
+ test $old = $new test $flag = 0
'
if test $(git config --bool core.filemode) = true; then
--
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
[PATCH 12/20] t/t0025-crlf-auto.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- t/t0025-crlf-auto.sh |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh index b0e5694..28102de 100755 --- a/t/t0025-crlf-auto.sh +++ b/t/t0025-crlf-auto.sh @@ -36,7 +36,7 @@ test_expect_success 'default settings cause no changes' ' onediff=$(git diff one) twodiff=$(git diff two) threediff=$(git diff three) - test -z $onediff -a -z $twodiff -a -z $threediff + test -z $onediff test -z $twodiff test -z $threediff ' test_expect_success 'crlf=true causes a CRLF file to be normalized' ' @@ -111,7 +111,7 @@ test_expect_success 'autocrlf=true does not normalize CRLF files' ' onediff=$(git diff one) twodiff=$(git diff two) threediff=$(git diff three) - test -z $onediff -a -z $twodiff -a -z $threediff + test -z $onediff test -z $twodiff test -z $threediff ' test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' ' @@ -126,7 +126,7 @@ test_expect_success 'text=auto, autocrlf=true _does_ normalize CRLF files' ' onediff=$(git diff one) twodiff=$(git diff two) threediff=$(git diff three) - test -z $onediff -a -n $twodiff -a -z $threediff + test -z $onediff test -n $twodiff test -z $threediff ' test_expect_success 'text=auto, autocrlf=true does not normalize binary files' ' -- 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
[PATCH 07/20] git-bisect.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
git-bisect.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index af4d04c..1e0d602 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -408,7 +408,7 @@ bisect_replay () {
bisect_reset
while read git bisect command rev
do
- test $git $bisect = git bisect -o $git = git-bisect ||
continue
+ test $git $bisect = git bisect || test $git =
git-bisect || continue
if test $git = git-bisect
then
rev=$command
--
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
[PATCH 06/20] contrib/examples/git-resolve.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- contrib/examples/git-resolve.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/examples/git-resolve.sh b/contrib/examples/git-resolve.sh index 48d0fc9..70fdc27 100755 --- a/contrib/examples/git-resolve.sh +++ b/contrib/examples/git-resolve.sh @@ -76,7 +76,7 @@ case $common in 2/dev/null || continue # Count the paths that are unmerged. cnt=$(GIT_INDEX_FILE=$G git ls-files --unmerged | wc -l) - if test $best_cnt -le 0 -o $cnt -le $best_cnt + if test $best_cnt -le 0 || test $cnt -le $best_cnt then best=$c best_cnt=$cnt -- 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
[PATCH 13/20] t/t0026-eol-config.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- t/t0026-eol-config.sh |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t0026-eol-config.sh b/t/t0026-eol-config.sh index e1126aa..4807b0f 100755 --- a/t/t0026-eol-config.sh +++ b/t/t0026-eol-config.sh @@ -36,7 +36,7 @@ test_expect_success 'eol=lf puts LFs in normalized file' ' ! has_cr two onediff=$(git diff one) twodiff=$(git diff two) - test -z $onediff -a -z $twodiff + test -z $onediff test -z $twodiff ' test_expect_success 'eol=crlf puts CRLFs in normalized file' ' @@ -49,7 +49,7 @@ test_expect_success 'eol=crlf puts CRLFs in normalized file' ' ! has_cr two onediff=$(git diff one) twodiff=$(git diff two) - test -z $onediff -a -z $twodiff + test -z $onediff test -z $twodiff ' test_expect_success 'autocrlf=true overrides eol=lf' ' @@ -63,7 +63,7 @@ test_expect_success 'autocrlf=true overrides eol=lf' ' has_cr two onediff=$(git diff one) twodiff=$(git diff two) - test -z $onediff -a -z $twodiff + test -z $onediff test -z $twodiff ' test_expect_success 'autocrlf=true overrides unset eol' ' @@ -77,7 +77,7 @@ test_expect_success 'autocrlf=true overrides unset eol' ' has_cr two onediff=$(git diff one) twodiff=$(git diff two) - test -z $onediff -a -z $twodiff + test -z $onediff test -z $twodiff ' test_done -- 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
[PATCH 15/20] t/t5000-tar-tree.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
t/t5000-tar-tree.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 74fc5a8..ad6fa0d 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -72,7 +72,7 @@ check_tar() {
for header in *.paxheader
do
data=${header%.paxheader}.data
- if test -h $data -o -e $data
+ if test -h $data || test -e $data
then
path=$(get_pax_header $header path)
if test -n $path
--
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
[PATCH 08/20] git-mergetool.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
git-mergetool.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index d08dc92..9a046b7 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -205,7 +205,7 @@ checkout_staged_file () {
$(git checkout-index --temp --stage=$1 $2 2/dev/null) \
: '\([^ ]*\)')
- if test $? -eq 0 -a -n $tmpfile
+ if test $? -eq 0 test -n $tmpfile
then
mv -- $(git rev-parse --show-cdup)$tmpfile $3
else
@@ -256,7 +256,7 @@ merge_file () {
checkout_staged_file 2 $MERGED $LOCAL
checkout_staged_file 3 $MERGED $REMOTE
- if test -z $local_mode -o -z $remote_mode
+ if test -z $local_mode || test -z $remote_mode
then
echo Deleted merge conflict for '$MERGED':
describe_file $local_mode local $LOCAL
--
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
[PATCH 17/20] t/t5538-push-shallow.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- t/t5538-push-shallow.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index 8e54ac5..63d9ca9 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -121,7 +121,7 @@ EOF ) ' -if test -n $NO_CURL -o -z $GIT_TEST_HTTPD; then +if test -n $NO_CURL || test -z $GIT_TEST_HTTPD; then say 'skipping remaining tests, git built without http support' test_done fi -- 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
[PATCH 05/20] contrib/examples/git-repack.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- contrib/examples/git-repack.sh |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/examples/git-repack.sh b/contrib/examples/git-repack.sh index f312405..96e3fed 100755 --- a/contrib/examples/git-repack.sh +++ b/contrib/examples/git-repack.sh @@ -76,8 +76,8 @@ case ,$all_into_one, in existing=$existing $e fi done - if test -n $existing -a -n $unpack_unreachable -a \ - -n $remove_redundant + if test -n $existing test -n $unpack_unreachable \ + test -n $remove_redundant then # This may have arbitrary user arguments, so we # have to protect it against whitespace splitting -- 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
[PATCH 20/20] CodingGuidelines: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- Documentation/CodingGuidelines | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index f424dbd..2d426bc 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -106,6 +106,18 @@ For shell scripts specifically (not exhaustive): interface translatable. See Marking strings for translation in po/README. + - We do not write our test command with -a and -o and use + or || to concatenate multiple test commands instead, because + the use of -a/-o is often error-prone. E.g. + + test -n $x -a $a = $b + + is buggy and breaks when $x is =, but + + test -n $x test $a = $b + + does not have such a problem. + For C programs: - We use tabs to indent, and interpret tabs as taking up to -- 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
[PATCH 14/20] t/t4102-apply-rename.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- t/t4102-apply-rename.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4102-apply-rename.sh b/t/t4102-apply-rename.sh index 49e2d6c..fae3059 100755 --- a/t/t4102-apply-rename.sh +++ b/t/t4102-apply-rename.sh @@ -52,6 +52,6 @@ EOF test_expect_success 'apply copy' \ 'git apply --index --stat --summary --apply test-patch - test $(cat bar) = This is bar -a $(cat foo) = This is foo' + test $(cat bar) = This is bar test $(cat foo) = This is foo' test_done -- 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
[PATCH 10/20] git-submodule.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
git-submodule.sh | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index b55d83a..1e3a5a6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -396,7 +396,7 @@ cmd_add()
sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
fi
- if test -z $repo -o -z $sm_path; then
+ if test -z $repo || test -z $sm_path; then
usage
fi
@@ -453,7 +453,7 @@ Use -f if you really want to add it. 2
# perhaps the path exists and is already a git repo, else clone it
if test -e $sm_path
then
- if test -d $sm_path/.git -o -f $sm_path/.git
+ if test -d $sm_path/.git || test -f $sm_path/.git
then
eval_gettextln Adding existing repo at '\$sm_path' to
the index
else
@@ -835,7 +835,7 @@ Maybe you want to use 'update --init'?)
continue
fi
- if ! test -d $sm_path/.git -o -f $sm_path/.git
+ if ! test -d $sm_path/.git || test -f $sm_path/.git
then
module_clone $sm_path $name $url $reference
$depth || exit
cloned_modules=$cloned_modules;$name
@@ -860,11 +860,11 @@ Maybe you want to use 'update --init'?)
die $(eval_gettext Unable to find current
${remote_name}/${branch} revision in submodule path '\$sm_path')
fi
- if test $subsha1 != $sha1 -o -n $force
+ if test $subsha1 != $sha1 || test -n $force
then
subforce=$force
# If we don't already have a -f flag and the submodule
has never been checked out
- if test -z $subsha1 -a -z $force
+ if test -z $subsha1 test -z $force
then
subforce=-f
fi
@@ -1034,7 +1034,7 @@ cmd_summary() {
then
head=$rev
test $# = 0 || shift
- elif test -z $1 -o $1 = HEAD
+ elif test -z $1 || test $1 = HEAD
then
# before the first commit: compare with an empty tree
head=$(git hash-object -w -t tree --stdin /dev/null)
@@ -1059,13 +1059,17 @@ cmd_summary() {
while read mod_src mod_dst sha1_src sha1_dst status sm_path
do
# Always show modules deleted or type-changed
(blob-module)
- test $status = D -o $status = T echo $sm_path
continue
+ case $status in
+ [DT])
+ printf '%s\n' $sm_path
+ continue
+ esac
# Respect the ignore setting for --for-status.
if test -n $for_status
then
name=$(module_name $sm_path)
ignore_config=$(get_submodule_config $name
ignore none)
- test $status != A -a $ignore_config = all
continue
+ test $status != A test $ignore_config = all
continue
fi
# Also show added or modified modules which are checked
out
GIT_DIR=$sm_path/.git git-rev-parse --git-dir
/dev/null 21
@@ -1125,7 +1129,7 @@ cmd_summary() {
*)
errmsg=
total_commits=$(
- if test $mod_src = 16 -a $mod_dst = 16
+ if test $mod_src = 16 test $mod_dst = 16
then
range=$sha1_src...$sha1_dst
elif test $mod_src = 16
@@ -1162,7 +1166,7 @@ cmd_summary() {
# i.e. deleted or changed to blob
test $mod_dst = 16 echo $errmsg
else
- if test $mod_src = 16 -a $mod_dst = 16
+ if test $mod_src = 16 test $mod_dst = 16
then
limit=
test $summary_limit -gt 0
limit=-$summary_limit
@@ -1233,7 +1237,11 @@ cmd_status()
say U$sha1 $displaypath
continue
fi
- if test -z $url || ! test -d $sm_path/.git -o -f
$sm_path/.git
+ if test -z $url ||
+ {
+ ! test -d
[PATCH 19/20] t/test-lib-functions.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern
shells, the reason to avoid test cond test cond spawning
one extra process by using a single test cond -a cond no
longer exists.
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
t/test-lib-functions.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 158e10a..0681003 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -542,7 +542,7 @@ test_must_fail () {
if test $exit_code = 0; then
echo 2 test_must_fail: command succeeded: $*
return 1
- elif test $exit_code -gt 129 -a $exit_code -le 192; then
+ elif test $exit_code -gt 129 test $exit_code -le 192; then
echo 2 test_must_fail: died by signal: $*
return 1
elif test $exit_code = 127; then
@@ -569,7 +569,7 @@ test_must_fail () {
test_might_fail () {
$@
exit_code=$?
- if test $exit_code -gt 129 -a $exit_code -le 192; then
+ if test $exit_code -gt 129 test $exit_code -le 192; then
echo 2 test_might_fail: died by signal: $*
return 1
elif test $exit_code = 127; then
--
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
[PATCH 09/20] git-rebase--interactive.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- git-rebase--interactive.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 6ec9d3c..797571f 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -1013,7 +1013,7 @@ then git rev-list $revisions | while read rev do - if test -f $rewritten/$rev -a $(sane_grep $rev $state_dir/not-cherry-picks) = + if test -f $rewritten/$rev test $(sane_grep $rev $state_dir/not-cherry-picks) = then # Use -f2 because if rev-list is telling us this commit is # not worthwhile, we don't want to track its multiple heads, -- 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
[PATCH 00/20] avoid test cond -a/-o cond
These patch series convert test -a/-o to and ||. This is the the third version. Changes: - Change the commit message for consistency with the comment to the patch to the CodingGuideline (Junio proposed patch http://www.spinics.net/lists/git/msg232199.html) expressing even better the meaning of the patches ( in particular, the phrase Better known vs. error prone). The previous comment was convert test -a/-o to and || - Therefore also includes the Julio patch to the CodingGuideline I have done the rebase with master and the patches also apply to next. I hope I have done the right thing in resending the patch series. I am Sorry if not. Elia Pinto (20): check_bindir: avoid test cond -a/-o cond contrib/examples/git-clone.sh: avoid test cond -a/-o cond contrib/examples/git-commit.sh: avoid test cond -a/-o cond contrib/examples/git-merge.sh: avoid test cond -a/-o cond contrib/examples/git-repack.sh: avoid test cond -a/-o cond contrib/examples/git-resolve.sh: avoid test cond -a/-o cond git-bisect.sh: avoid test cond -a/-o cond git-mergetool.sh: avoid test cond -a/-o cond git-rebase--interactive.sh: avoid test cond -a/-o cond git-submodule.sh: avoid test cond -a/-o cond t/lib-httpd.sh: avoid test cond -a/-o cond t/t0025-crlf-auto.sh: avoid test cond -a/-o cond t/t0026-eol-config.sh: avoid test cond -a/-o cond t/t4102-apply-rename.sh: avoid test cond -a/-o cond t/t5000-tar-tree.sh: avoid test cond -a/-o cond t/t5403-post-checkout-hook.sh: avoid test cond -a/-o cond t/t5538-push-shallow.sh: avoid test cond -a/-o cond t/t9814-git-p4-rename.sh: avoid test cond -a/-o cond t/test-lib-functions.sh: avoid test cond -a/-o cond CodingGuidelines: avoid test cond -a/-o cond Documentation/CodingGuidelines | 12 check_bindir|2 +- contrib/examples/git-clone.sh |2 +- contrib/examples/git-commit.sh |4 ++-- contrib/examples/git-merge.sh |4 ++-- contrib/examples/git-repack.sh |4 ++-- contrib/examples/git-resolve.sh |2 +- git-bisect.sh |2 +- git-mergetool.sh|4 ++-- git-rebase--interactive.sh |2 +- git-submodule.sh| 32 t/lib-httpd.sh |2 +- t/t0025-crlf-auto.sh|6 +++--- t/t0026-eol-config.sh |8 t/t4102-apply-rename.sh |2 +- t/t5000-tar-tree.sh |2 +- t/t5403-post-checkout-hook.sh |8 t/t5538-push-shallow.sh |2 +- t/t9814-git-p4-rename.sh|4 ++-- t/test-lib-functions.sh |4 ++-- 20 files changed, 64 insertions(+), 44 deletions(-) -- 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
[PATCH 01/20] check_bindir: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- check_bindir |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_bindir b/check_bindir index a1c4c3e..623eadc 100755 --- a/check_bindir +++ b/check_bindir @@ -2,7 +2,7 @@ bindir=$1 gitexecdir=$2 gitcmd=$3 -if test $bindir != $gitexecdir -a -x $gitcmd +if test $bindir != $gitexecdir test -x $gitcmd then echo echo !! You have installed git-* commands to new gitexecdir. -- 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
[PATCH 02/20] contrib/examples/git-clone.sh: avoid test cond -a/-o cond
The construct is error-prone; test being built-in in most modern shells, the reason to avoid test cond test cond spawning one extra process by using a single test cond -a cond no longer exists. Signed-off-by: Elia Pinto gitter.spi...@gmail.com --- contrib/examples/git-clone.sh |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/examples/git-clone.sh b/contrib/examples/git-clone.sh index b4c9376..08cf246 100755 --- a/contrib/examples/git-clone.sh +++ b/contrib/examples/git-clone.sh @@ -516,7 +516,7 @@ then case $no_checkout in '') - test z$quiet = z -a z$no_progress = z v=-v || v= + test z$quiet = z test z$no_progress = z v=-v || v= git read-tree -m -u $v HEAD HEAD esac fi -- 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 2/2] mingw: avoid const warning
Am 29.05.2014 12:47, schrieb Stepan Kasal:
Fix const warnings in http-fetch.c and remote-curl.c main() where is
argv declared as const.
The fix should work for all future declarations of main, no matter
whether the second parameter's type is char**, const char**, or
char *[].
I'm 100% in favor of a solution that doesn't restrict main to non-const char**!
Thanks.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 15f0c9d..8745d19 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -369,10 +369,11 @@ extern CRITICAL_SECTION pinfo_cs;
void mingw_startup();
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(c,v); \
-int main(int argc, char **argv) \
+int main(c, char **main_argv_not_used) \
{ \
+ typedef v, **argv_type; \
mingw_startup(); \
- return mingw_main(__argc, __argv); \
+ return mingw_main(__argc, (argv_type)__argv); \
} \
static int mingw_main(c,v)
I have to admit I had trouble understanding what 'typedef v, **arv_type;' does
(looks invalid at first glance), and why you would need main_argv_not_used
instead of just main(c,v).
So, I'd like to award +10 points for cleverness, but -10 for obscurity ;-)
Probably deserves a comment or an explanation in the commit message.
A simpler solution that works with all definitions of main() is to cast to
void* (tell the compiler all responsibility is on us). I.e.:
void mingw_startup();
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(c,v); \
int main(c,v) \
{ \
mingw_startup(); \
return mingw_main(__argc, (void *) __argv); \
} \
static int mingw_main(c,v)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] replace: add --graft option
On Thu, Jun 5, 2014 at 11:49 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
Not a problem with this patch, but the above sequence somehow makes
me wonder if lookup-commit-or-die is a good API for this sort of
thing. Wouldn't it be more natural if it took not unsigned char
old[20] but anything that would be understood by get_sha1()?
It could be that this particular caller is peculiar and all the
existing callers are happy, though. I didn't git grep to spot
patterns in existing callers.
lookup_commit_or_die() looked like a good API to me because I saw that
it checked a lot of things and die in case of problems, which could
make the patch shorter.
+ if (read_sha1_commit(old, buf))
+ die(_(Invalid commit: '%s'), old_ref);
+ /* make sure the commit is not corrupt */
+ if (parse_commit_buffer(commit, buf.buf, buf.len))
+ die(_(Could not parse commit: '%s'), old_ref);
It is unclear to me what you are trying to achieve with these two.
If the earlier lookup-commit has returned a commit object that has
already been parsed, parse_commit_buffer() would not check anything,
would it?
Yeah, you are right. I missed the fact that lookup_commit_or_die()
calls parse_object() which itself calls read_sha1_file() and then
parse_object_buffer() which calls parse_commit_buffer().
Here is a backtrace that shows this:
#0 parse_commit_buffer (item=0x8597b0, buffer=0x851730, size=228) at
commit.c:251
#1 0x004fa215 in parse_object_buffer (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234,
type=OBJ_COMMIT, size=228,
buffer=0x851730, eaten_p=0x7fffdacc) at object.c:198
#2 0x004fa50a in parse_object (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234) at
object.c:264
#3 0x004a89ef in lookup_commit_reference_gently
(sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234, quiet=0) at
commit.c:38
#4 0x004a8a48 in lookup_commit_reference (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234) at
commit.c:47
#5 0x004a8a67 in lookup_commit_or_die (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234,
ref_name=0x7fffe465
093e41a79d4a8bfe3c758a96c95e5bf9e35ed89c) at commit.c:52
#6 0x0047f89a in create_graft (argc=1, argv=0x7fffe130,
refdir=0x0, force=0) at builtin/replace.c:353
#7 0x0047ff71 in cmd_replace (argc=1, argv=0x7fffe130,
prefix=0x0) at builtin/replace.c:461
#8 0x00405441 in run_builtin (p=0x7eee90, argc=3,
argv=0x7fffe130) at git.c:314
#9 0x0040563a in handle_builtin (argc=3, argv=0x7fffe130)
at git.c:487
#10 0x00405754 in run_argv (argcp=0x7fffe01c,
argv=0x7fffe020) at git.c:533
#11 0x004058f9 in main (argc=3, av=0x7fffe128) at git.c:616
A typical sequence would look more like this:
commit = lookup_commit(...);
if (parse_commit(commit))
oops there is an error;
/* otherwise */
use(commit-buffer);
without reading a commit object using low-level read-sha1-file
interface yourself, no?
Yeah, or I could just rely on the fact that lookup_commit_or_die()
already parses the commit, with something like this:
if (get_sha1(old_ref, old) 0)
die(_(Not a valid object name: '%s'), old_ref);
/* parse the commit buffer to make sure the commit is not corrupt */
commit = lookup_commit_or_die(old, old_ref);
/* find existing parents */
parent_start = buf.buf;
parent_start += 46; /* tree + hex sha1 + \n */
parent_end = parent_start;
...
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
Feature Request: Fix/Make git-rebase work with git-subtree (was: Re: subtree merges lose prefix after rebase)
Hi All ~ This is sort of re-open of a 2012 thread: http://git.661346.n2.nabble.com/subtree-merges-lose-prefix-after-rebase-td7332850.html A related stackoverflow post that describes the problem a work-arounds/what was attempted to solve the problem: http://stackoverflow.com/questions/12858199/how-to-rebase-after-git-subtree-add/23094756 I understand that git-subtree/git-rebase are currently working as intended, and sub-modules were, in theory, were intended to be a replacement of subtrees. However, I would like to request re-looking at the viability of making the rebase example work as someone using a git-subtree should expect it to. I don't know enough of the under-the-hood detail of what it would take to implement - but so far it looks like there's over 1k people on the stackoverflow that have had the same issue I ran into were looking for a work-around. Thanks, Eric On Thu, Mar 1, 2012 at 6:30 PM, Junio C Hamano gits...@pobox.com wrote: Zakaria ElQotbi zaka...@elqotbi.com writes: why subtree merges lose prefix after an interactive rebase, is it a known issue ? or I'am missing somethings: ... $ git rebase -i -p a6d4e8e # this the hash of merge b commit $ git commit --amend -m merge b edit $ git rebase --continue $ tree . |-- C |-- projects | |-- a | | `-- A | `-- b | `-- B `-- README Rebase essentially is a stepwise cherry-pick, and cherry-pick does not see anything but the trees recorded in the commit being rebased and in its parent. Your original history is to merge in projects a, b and c in order, renaming them using subtree merge to their own subdirectory. You rebase the commits after the one that merges b, i.e. the merge of project c, in that history. As far as that rebased commit is concerned, the change it makes relative to its parent commit is to add file C at the root level. So you are starting from the state you merged a and b into the whole project, and replaing that commit that adds C at the root level. That matches the above picture. In short, this is expected, because rebase does not know anything about evil merges made by 'subtree' (or 'ours' for that matter). And I do not think there is any plan to make rebase aware of subtree merges. After all, subtree merge was invented merely as a short-term hack to serve as a stop gap measure until submodule support becomes mature. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] replace: add --graft option
On Fri, Jun 6, 2014 at 5:29 PM, Christian Couder christian.cou...@gmail.com wrote: Yeah, or I could just rely on the fact that lookup_commit_or_die() already parses the commit, with something like this: if (get_sha1(old_ref, old) 0) die(_(Not a valid object name: '%s'), old_ref); /* parse the commit buffer to make sure the commit is not corrupt */ commit = lookup_commit_or_die(old, old_ref); /* find existing parents */ parent_start = buf.buf; parent_start += 46; /* tree + hex sha1 + \n */ parent_end = parent_start; This last part should be: /* find existing parents */ strbuf_addstr(buf, commit-buffer); parent_start = buf.buf; parent_start += 46; /* tree + hex sha1 + \n */ parent_end = parent_start; ... I will send an updated patch series soon. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] contrib: add convert-grafts-to-replace-refs.sh
On Thu, Jun 5, 2014 at 11:55 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..8472879
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+die () {
+ echo 2 $@
+ exit 1
+}
Don't we install git-sh-setup in GIT_EXEC_PATH, in order to allow
these third-party scripts to begin with:
. $(git --exec-path)/git-sh-setup
just like our own scripted Porcelains?
Ok, I will use that.
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
Format the above like so:
grep '^[^# ]' $GRAFTS_FILE |
while read definition
do
which is easier to see what that do is doing.
Ok.
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ }
Hmph, why not if/then/fi?
Ok, I will use if/then/fi.
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
GOOD DAY,
My name is Mr Yao Yuta from Hong Kong, I want you to be my partner in a business project. If Interested Contact me back via my email address. Thank you, Mr. Yao Yuta. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Windows dirent patches
Am 06.06.2014 15:43, schrieb Stepan Kasal: Hello, This is a series of dirent modifications, 4 tiny ones and one bigger. As the date indicates, these are battle tested in mysgit for several years. The dates are actually missing from the patches, otherwise full ack from me. 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: [RFC PATCH] clone: add clone.recursesubmodules config option
Heiko Voigt hvo...@hvoigt.net writes: On Thu, Jun 05, 2014 at 07:48:33PM +1200, Chris Packham wrote: ... I'm a little confused at how autoclone and autoinit differ. Aren't they the same? i.e. when this module appears grab it by default. I see autoupdate as a little different meaning update it if it's been initialised. Also does autoinit imply autoupdate? autoclone is about cloning the history of submodules. So e.g. when a submodule first appears in the superprojects history whether it should automatically be cloned to .git/modules. autoinit is all about the checkout phase. When a commit with a new submodule is checked out: Should that new submodule be automatically initialised? As far as autoupdate is concerned: Maybe autoinit can imply that it is enabled, yes. But I guess we still need autoupdate for the case of big submodules that cause to much performance trouble if updated by every checkout. So its actually three values: autoclone, autoinit, autoupdate. Damn, these configurations become more complicated everytime. I suspect that as an end-user you do not need to set all three in most cases. Just like an unspecified autoupdate can default to whatever autoinit setting for the submodule is, because it is less likely that a user wants to have a submodule checked out *and* leave it stale, an unspecified autoinit can default to the autoclone setting, because it is less likely that a user who does not want to have a checkout would want to spend network bandwidth to clone 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: [PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd
Richard Hansen rhan...@bbn.com writes: On 2014-06-05 17:11, Junio C Hamano wrote: ... In any case, the above explanation confuses me somewhat. test_eval_ is fed a scriptlet defined for various test_expect_success tests, and they are written in POSIX shells, not zsh, so wouldn't it be wrong to run them as if they are zsh native scripts, following non-POSIX shell syntax rules? The scriptlets in lib-prompt-tests.sh are not actually written for POSIX sh -- they are written in a common subset of the zsh and bash languages (I should document this in lib-prompt-tests.sh). We want to test how the __git_ps1 code behaves when interpreted in native zsh mode (default options), because that's how it will be used in the wild, so the scriptlets must be valid zsh code. We also want to test how __git_ps1 behaves in native bash mode, so the scriptlets must also be valid bash code. (Fortunately the similarities between the shells make this easy to do.) OK. The above all makes sense, but I think we would prefer a solution with changes limited to lib-prompt-tests and lib-zsh without touching lib-test-functions at all if that is the case. An alternative to this commit -- and I kinda like this idea so I'm tempted to rewrite the series -- would be to do change the form of the tests in lib-prompt-tests.sh to something like this: test_expect_success 'name of test here' ' run_in_native_shell_mode '\'' scriptlet code here '\'' ' Yeah, or even: prompt_test_expect success 'name of test' ' scriptlet code here ' with a helper prompt_test_expect that wraps whatever logic you will have in run-in-native-shell-mode. ... This approach makes it clear to others that the scriptlet code is not actually POSIX shell code, but a common subset of multiple shell languages. What do you think? ;-) Ignoring t9902 for a moment, we could even stop doing that messy 'exec $SHELL $0 $@' stuff in lib-*sh.sh and let t9903 and t9904 run under /bin/sh. Then run_in_native_shell_mode() would be defined as follows: No, let's not go there (and you stated the reason why we do not want to yourself already ;-). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/4] replace: add --graft option
Christian Couder christian.cou...@gmail.com writes:
On Thu, Jun 5, 2014 at 11:49 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
Not a problem with this patch, but the above sequence somehow makes
me wonder if lookup-commit-or-die is a good API for this sort of
thing. Wouldn't it be more natural if it took not unsigned char
old[20] but anything that would be understood by get_sha1()?
It could be that this particular caller is peculiar and all the
existing callers are happy, though. I didn't git grep to spot
patterns in existing callers.
lookup_commit_or_die() looked like a good API to me because I saw that
it checked a lot of things and die in case of problems, which could
make the patch shorter.
Perhaps I was vague. find the commit object and die if that object
is not properly formed is a good thing. I was referring to the
fact that you
- first had to do get-sha1 yourself to turn the extended sha1
you got from the user into a binary object name, and die with
your own error message if the user input was malformed.
- and then had to call lookup-commit-or-die to do the checking
and let it die.
It would have been simpler for *this* particular codepath if we had
another helper function you can use like so:
commit = lookup_commit_with_extended_sha1_or_die(old_ref);
which did the two-call sequence you handrolled above, and I was
wondering if other existing callers to lookup-commit-or-die wished
the same thing.
--
To unsubscribe from this list: send the line 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: Proposal for pruning tags
Robert Dailey rcdailey.li...@gmail.com writes: ... Having git clean up tags automatically would really help with this, even though you may not feel it's the responsibility of Git. It's more of a usability issue, I agree with Having ... help with this. I did not say at all that it is not something Git should and can try to help. I also agree with it is a usability issue. The thing is, the word automatically in your clean up tags automatically is still too loose a definition of what we want, and we cannot come up with a way to help users without tightening that looseness. As you said, you are looking for something that can tell between two kinds of tags that locally exist without having a copy at the 'origin': - ones that you do not want to keep - others that you haven't pushed to (or forgot to push to) 'origin' without giving the users a way to help Git to tell these two kinds apart and only remove the former. So... -- To unsubscribe from this list: send the line 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] Use starts_with() for C strings instead of memcmp()
Convert three cases of checking for a constant prefix using memcmp() to
starts_with(). This way there is no need for magic string length
constants and we avoid running over the end of the string should it be
shorter than the prefix.
Signed-off-by: Rene Scharfe l@web.de
---
These are the easy cases I found; there are several more comparisons
of strings to constants using memcmp(). Some could benefit from
skip_prefix(), others may need a bit more thought, and perhaps I
missed a few.
builtin/for-each-ref.c | 2 +-
fetch-pack.c | 2 +-
remote.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 3e1d5c3..4135980 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -193,7 +193,7 @@ static int verify_format(const char *format)
at = parse_atom(sp + 2, ep);
cp = ep + 1;
- if (!memcmp(used_atom[at], color:, 6))
+ if (starts_with(used_atom[at], color:))
need_color_reset_at_eol = !!strcmp(used_atom[at],
color_reset);
}
return 0;
diff --git a/fetch-pack.c b/fetch-pack.c
index 2bb..b12bd4c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -507,7 +507,7 @@ static void filter_refs(struct fetch_pack_args *args,
int keep = 0;
next = ref-next;
- if (!memcmp(ref-name, refs/, 5)
+ if (starts_with(ref-name, refs/)
check_refname_format(ref-name, 0))
; /* trash */
else {
diff --git a/remote.c b/remote.c
index eea2c8d..0f6ef36 100644
--- a/remote.c
+++ b/remote.c
@@ -1194,7 +1194,7 @@ static int match_explicit(struct ref *src, struct ref
*dst,
case 1:
break;
case 0:
- if (!memcmp(dst_value, refs/, 5))
+ if (starts_with(dst_value, refs/))
matched_dst = make_linked_ref(dst_value, dst_tail);
else if (is_null_sha1(matched_src-new_sha1))
error(unable to delete '%s': remote ref does not
exist,
--
2.0.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] userdiff: support C# async methods and correct C# keywords
Steve Hoelzer shoel...@gmail.com writes: On Thu, Jun 5, 2014 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Sup Yut Sum ch3co...@gmail.com writes: async is in C# 5.0 foreach is in C# 1.0 instanceof is in Java. The similar keywords are typeof, is, as in C# 1.0 This one made me read it twice, until I realized you meant instanceof() is listed as keywords, but there is no such thing (it is in Java, though); in C# we use typeof() for similar purposes The original email was a bit hard to parse. Junio's clarification left out the C# keywords 'is' and 'as'. I suggest phrasing it like this: instanceof() is listed as keywords, but there is no such thing (it is in Java, though); in C# we use typeof(), 'is', and 'as for similar purposes You would need to balance the quotes around as ;-) But reading the patch again after noticing that you have () after typeof but not after is/as, I am not sure if the change proposed here is even correct for the latter two. I do not speal c-sharp, so I asked http://msdn.microsoft.com/en-us/library/cscsdfbt.aspx for some examples and here are what I found: Type t = typeof(ExampleClass) Base b = derived as Base; if (obj is MyObject) ... Unlike the control-flow keywords (e.g. do/while/for/...), do they typically appear at the beginning of lines? Isn't the purpose of these !^[ \t]* patterns to reject lines that begin with the language keywords that do not start functions, so listing a keyword that does not usually appear at the beginning of line looks like a churn that is not useful. diff --git a/userdiff.c b/userdiff.c index fad52d6..96eda6c 100644 --- a/userdiff.c +++ b/userdiff.c @@ -134,9 +134,9 @@ PATTERNS(cpp, |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*), PATTERNS(csharp, /* Keywords */ - !^[ \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n + !^[ \t]*(do|while|for|foreach|if|else|typeof|is|as|new|return|switch|case|default|throw|try|catch|using)\n /* Methods and constructors */ - ^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n + ^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe|async)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n /* Properties */ ^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n /* Type definitions */ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] First part of Unicode console support for msysgit
Am 06.06.2014 15:42, schrieb Stepan Kasal: Hello, this is first part of the unicode support pathes from msysgit. Nicely done, thanks! I think its important to reiterate that these patches were written several years apart, so there are some inconsistencies and back-and-forth changes (e.g. [5/5] fixes line break errors introduced in [3/5]). I'm OK with merging this as is, if there are no objections from the list, simply because it represents the battle tested history we have in Git for Windows. The only real complaint I have is that I'm missing [6/5] Win32: fix broken pipe detection [1], which leaves this series in a slightly broken state (terminating the pager will not terminate the calling git process). Nitpicking follows... The first three patches originate in Jun 2010, though some fixups from 2012 have been squashed in. The fourth one is just a trivial prerequisite for the last one, that was written in Jan 2012, with a fixup from Mar 2012. The dates are missing from the patches. It would also have been nice to name (or link to) the patches you sqashed. Regards, Stepan Karsten Blees (5): Support Unicode console output on Windows This introduces WriteConsoleW, so you could have squashed half of Win32: fix segfault in WriteConsoleW when debugging in gdb [2] (second half in [5/5]). Detect console streams more reliably on Windows Warn if the Windows console font doesn't support Unicode I think this one includes MSVC: fix winansi.c compile errors [3] and Unicode console: fix font warning on Vista and Win7 [4] (which is partly reverted by [5/5], which also fixes the formatting). Win32: move main macro to a function Note: this one was submitted seperately on May 29 and May 1 (can't find it in the gmane archive, though). Win32: Thread-safe windows console output compat/mingw.c | 24 ++- compat/mingw.h | 24 +-- compat/winansi.c | 446 --- 3 files changed, 356 insertions(+), 138 deletions(-) [1] https://github.com/msysgit/git/commit/67934f93 [2] https://github.com/msysgit/git/commit/cd0792af [3] https://github.com/msysgit/git/commit/3abcb04d [4] https://github.com/msysgit/git/commit/981aa538 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] hashmap: add enum for hashmap free_entries option
Am 05.06.2014 08:06, schrieb Heiko Voigt:
This allows a reader to immediately know which options can be used and
what this parameter is about.
[...]
-void hashmap_free(struct hashmap *map, int free_entries)
+void hashmap_free(struct hashmap *map, enum hashmap_free_options
free_entries)
[...]
+enum hashmap_free_options {
+ HASHMAP_NO_FREE_ENTRIES = 0,
+ HASHMAP_FREE_ENTRIES = 1,
+};
This was meant as a boolean parameter. Would it make sense to have
enum boolean {
false,
true
};
or similar in some central place?
Note that an earlier version took a function pointer, and you could pass
stdlib's free() in the common case, or a special free routine for nested entry
structures, or NULL to do the cleanup yourself.
--
To unsubscribe from this list: send the line 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: [msysGit] [PATCH] mingw: redefine the wrapper macro after the corresponding function
Am 06.06.2014 13:10, schrieb Stepan Kasal:
Hi Karsten,
On Fri, Jun 06, 2014 at 11:43:03AM +0200, Karsten Blees wrote:
Thinking about this some more, the best solution is probably to
eliminate the problem altogether by adding inline-wrappers for
required CRT-functions, e.g.:
Yes, this is acceptable. But I wouldn't pollute mingw.h. You can do
it on top of mingw.c like this:
But having it in the .h file may come in handy if we want to split the overlong
mingw.c into several compilation units...
#undef gethostname
static inline int crt_gethostname(char *host, int namelen)
{
return gethostname(host, namelen);
}
#define gethostname please_call_the_mingw_or_crt_version
Now you're mixing all three variants...note that with my suggestion to #define
crt_foo in mingw.h, you don't need '#undef foo', nor redefine foo (your
variant), nor rename other callers in mingw.c to 'mingw_foo' (Hannes' variant).
Callers of foo() would simply write foo(), no matter whether in mingw.c or
anywhere else. In the special case that you really want the CRT version, you'd
write crt_foo(). This works everywhere, even in core-git code wrapped in #ifdef
GIT_WINDOWS_NATIVE.
--
To unsubscribe from this list: send the line 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: [msysGit] Re: [PATCH 0/5] First part of Unicode console support for msysgit
Hello Karsten, On Fri, Jun 06, 2014 at 07:44:33PM +0200, Karsten Blees wrote: Nicely done, thanks! thank you for your kind words. Please hold back, I will re-submit in a few days. Note: this one was submitted seperately on May 29 and May 1 (can't find it in the gmane archive, though). It was cc'ed to msysgit as usual and gmane selected it as the primary place to store it: http://thread.gmane.org/gmane.comp.version-control.msysgit/20324 regards, Stepan -- To unsubscribe from this list: send the line 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 20/20] CodingGuidelines: avoid test cond -a/-o cond
On 2014-06-06 16.56, Elia Pinto wrote: + - We do not write our test command with -a and -o and use That and and and could be somewhat confusing. How about: We do not write our test command with -a and/or -o. Instead we use or || to concatenate multiple test commands instead, because the use of -a/-o is often error-prone. E.g. (The rest looks OK) -- To unsubscribe from this list: send the line 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/2] mingw: avoid const warning
Karsten Blees karsten.bl...@gmail.com writes:
Am 29.05.2014 12:47, schrieb Stepan Kasal:
Fix const warnings in http-fetch.c and remote-curl.c main() where is
argv declared as const.
The fix should work for all future declarations of main, no matter
whether the second parameter's type is char**, const char**, or
char *[].
I'm 100% in favor of a solution that doesn't restrict main to non-const
char**! Thanks.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 15f0c9d..8745d19 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -369,10 +369,11 @@ extern CRITICAL_SECTION pinfo_cs;
void mingw_startup();
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(c,v); \
-int main(int argc, char **argv) \
+int main(c, char **main_argv_not_used) \
{ \
+typedef v, **argv_type; \
mingw_startup(); \
-return mingw_main(__argc, __argv); \
+return mingw_main(__argc, (argv_type)__argv); \
} \
static int mingw_main(c,v)
I have to admit I had trouble understanding what 'typedef v,
**arv_type;' does (looks invalid at first glance), and why you
would need main_argv_not_used instead of just main(c,v).
So, I'd like to award +10 points for cleverness, but -10 for
obscurity ;-) Probably deserves a comment or an explanation in the
commit message.
Agreed. The typedef one is a cute hack.
I am wondering why the solution is not a more obvious drop const
that is not ANSI C, though. I only have a ready-access to N1570
draft but in it I find:
5.1.2.2.1 Program startup
The function called at program startup is named main. The
implementation declares no prototype for this function. It shall
be defined with a return type of int and with no parameters:
int main(void) { /* ... */ }
or with two parameters (referred to here as argc and argv,
though any names may be used, as they are local to the function
in which they are declared):
int main(int argc, char *argv[]) { /* ... */ }
or equivalent;10) or in some other implementation-defined manner.
---
10) Thus, int can be replaced by a typedef name defined as int,
or the type of argv can be written as char ** argv, and so on.
A simpler solution that works with all definitions of main() is to
cast to void* (tell the compiler all responsibility is on
us).
Can you cast away the constness that way, though?
--
To unsubscribe from this list: send the line 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] Add a Windows-specific fallback to getenv(HOME);
On 04.06.2014 18:16, Stepan Kasal wrote: plan is to switch to mingwGitDevEnv for said release. No more msysGit. Like, bu-bye. Thanks for all the fish. Interesting. With msysgit, there is the net installer - first time I installed msys/mingw sucessfully, it was as easy as Cygwin, perhaps even easier. And with mingwGitDevEnv, there's the equivalent installer at [1]. When I go to mingwGitDevEnv home page, I read about chickens, eggs, and upgrading Perl (which msysGit simply gives up, hinting that it is almost impossible). I have absolutely no idea what chickens and eggs that would be. If you care to elaborate, please consider using the mingwGitDevEnv mailing list [2]. PPS: from marketing point of view, mingwGitDevEnv is far from usable name. Dscho, if you support the idea, would you mind franchising msysGit 2.0 for a decent amount? Doh. Marketing. As if we would sell something. I still believe developers are more interested in getting things done no matter what the tools are called. At east they should be. [1] http://mingwgitdevenv.cloudapp.net/job/mingwGitDevEnv-build-installer/lastSuccessfulBuild/artifact/download.html [2] http://groups.google.com/group/mingwGitDevEnv -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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/5] Windows dirent patches
On Fri, Jun 06, 2014 at 06:33:27PM +0200, Karsten Blees wrote: The dates are actually missing from the patches, [...] oops, this was first time I tried to use git-send-email. I hope this time it'll work better. Stepan Hello, This is a series of dirent modifications, 4 tiny ones and one bigger. As the date indicates, these are battle tested in mysgit for several years. Regards, Stepan Karsten Blees (5): Win32 dirent: remove unused dirent.d_ino member Win32 dirent: remove unused dirent.d_reclen member Win32 dirent: change FILENAME_MAX to MAX_PATH Win32 dirent: clarify #include directives Win32 dirent: improve dirent implementation compat/win32/dirent.c | 116 -- compat/win32/dirent.h | 8 +--- config.mak.uname | 2 + 3 files changed, 59 insertions(+), 67 deletions(-) -- 2.0.0.9635.g0be03cb -- To unsubscribe from this list: send the line 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 3/5] Win32 dirent: change FILENAME_MAX to MAX_PATH
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 17:43:14 +0100
FILENAME_MAX and MAX_PATH are both 260 on Windows, however, MAX_PATH is
used throughout the other Win32 code in Git, and also defines the length
of file name buffers in the Win32 API (e.g. WIN32_FIND_DATA.cFileName,
from which we're copying the dirent data).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index 7f4e6c7..8838cd6 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -9,8 +9,8 @@ typedef struct DIR DIR;
#define DT_LNK 3
struct dirent {
- char d_name[FILENAME_MAX]; /* File name. */
unsigned char d_type; /* file type to prevent lstat after readdir
*/
+ char d_name[MAX_PATH]; /* file name */
};
DIR *opendir(const char *dirname);
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32 dirent: clarify #include directives
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 17:47:41 +0100
Git-compat-util.h is two dirs up, and already includes dirent.h (which
is the same as dirent.h due to -Icompat/win32 in the Makefile).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
index 7a0debe..fac7f25 100644
--- a/compat/win32/dirent.c
+++ b/compat/win32/dirent.c
@@ -1,5 +1,4 @@
-#include ../git-compat-util.h
-#include dirent.h
+#include ../../git-compat-util.h
struct DIR {
struct dirent dd_dir; /* includes d_type */
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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/5] Win32 dirent: remove unused dirent.d_reclen member
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 17:38:25 +0100
Remove the union around dirent.d_type and the unused dirent.d_reclen member
(which was necessary for compatibility with the MinGW dirent runtime, which
is no longer used).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index b38973b..7f4e6c7 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -10,10 +10,7 @@ typedef struct DIR DIR;
struct dirent {
char d_name[FILENAME_MAX]; /* File name. */
- union {
- unsigned short d_reclen; /* Always zero. */
- unsigned char d_type; /* Reimplementation adds this */
- };
+ unsigned char d_type; /* file type to prevent lstat after readdir
*/
};
DIR *opendir(const char *dirname);
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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 5/5] Win32 dirent: improve dirent implementation
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 17:57:02 +0100
Improve the dirent implementation by removing the relics that were once
necessary to plug into the now unused MinGW runtime, in preparation for
Unicode file name support.
Move FindFirstFile to opendir, and FindClose to closedir, with the
following implications:
- DIR.dd_name is no longer needed
- chdir(one); opendir(relative); chdir(two); readdir() works as expected
(i.e. lists one/relative instead of two/relative)
- DIR.dd_handle is a valid handle for the entire lifetime of the DIR struct
- thus, all checks for dd_handle == INVALID_HANDLE_VALUE and dd_handle == 0
have been removed
- the special case that the directory has been fully read (which was
previously explicitly tracked with dd_handle == INVALID_HANDLE_VALUE
dd_stat != 0) is now handled implicitly by the FindNextFile error
handling code (if a client continues to call readdir after receiving
NULL, FindNextFile will continue to fail with ERROR_NO_MORE_FILES, to
the same effect)
- extracting dirent data from WIN32_FIND_DATA is needed in two places, so
moved to its own method
- GetFileAttributes is no longer needed. The same information can be
obtained from the FindFirstFile error code, which is ERROR_DIRECTORY if
the name is NOT a directory (- ENOTDIR), otherwise we can use
err_win_to_posix (e.g. ERROR_PATH_NOT_FOUND - ENOENT). The
ERROR_DIRECTORY case could be fixed in err_win_to_posix, but this
probably breaks other functionality.
Removes the ERROR_NO_MORE_FILES check after FindFirstFile (this was
fortunately a NOOP (searching for '*' always finds '.' and '..'),
otherwise the subsequent code would have copied data from an uninitialized
buffer).
Changes malloc to git support function xmalloc, so opendir will die() if
out of memory, rather than failing with ENOMEM and letting git work on
incomplete directory listings (error handling in dir.c is quite sparse).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.c | 113 --
1 file changed, 54 insertions(+), 59 deletions(-)
diff --git a/compat/win32/dirent.c b/compat/win32/dirent.c
index fac7f25..82a515c 100644
--- a/compat/win32/dirent.c
+++ b/compat/win32/dirent.c
@@ -4,92 +4,88 @@ struct DIR {
struct dirent dd_dir; /* includes d_type */
HANDLE dd_handle; /* FindFirstFile handle */
int dd_stat; /* 0-based index */
- char dd_name[1]; /* extend struct */
};
+static inline void finddata2dirent(struct dirent *ent, WIN32_FIND_DATAA *fdata)
+{
+ /* copy file name from WIN32_FIND_DATA to dirent */
+ memcpy(ent-d_name, fdata-cFileName, sizeof(ent-d_name));
+
+ /* Set file type, based on WIN32_FIND_DATA */
+ if (fdata-dwFileAttributes FILE_ATTRIBUTE_DIRECTORY)
+ ent-d_type = DT_DIR;
+ else
+ ent-d_type = DT_REG;
+}
+
DIR *opendir(const char *name)
{
- DWORD attrs = GetFileAttributesA(name);
+ char pattern[MAX_PATH];
+ WIN32_FIND_DATAA fdata;
+ HANDLE h;
int len;
- DIR *p;
+ DIR *dir;
- /* check for valid path */
- if (attrs == INVALID_FILE_ATTRIBUTES) {
- errno = ENOENT;
+ /* check that name is not NULL */
+ if (!name) {
+ errno = EINVAL;
return NULL;
}
-
- /* check if it's a directory */
- if (!(attrs FILE_ATTRIBUTE_DIRECTORY)) {
- errno = ENOTDIR;
- return NULL;
- }
-
/* check that the pattern won't be too long for FindFirstFileA */
len = strlen(name);
- if (is_dir_sep(name[len - 1]))
- len--;
if (len + 2 = MAX_PATH) {
errno = ENAMETOOLONG;
return NULL;
}
-
- p = malloc(sizeof(DIR) + len + 2);
- if (!p)
+ /* copy name to temp buffer */
+ memcpy(pattern, name, len + 1);
+
+ /* append optional '/' and wildcard '*' */
+ if (len !is_dir_sep(pattern[len - 1]))
+ pattern[len++] = '/';
+ pattern[len++] = '*';
+ pattern[len] = 0;
+
+ /* open find handle */
+ h = FindFirstFileA(pattern, fdata);
+ if (h == INVALID_HANDLE_VALUE) {
+ DWORD err = GetLastError();
+ errno = (err == ERROR_DIRECTORY) ? ENOTDIR :
err_win_to_posix(err);
return NULL;
+ }
- memset(p, 0, sizeof(DIR) + len + 2);
- strcpy(p-dd_name, name);
- p-dd_name[len] = '/';
- p-dd_name[len+1] = '*';
-
- p-dd_handle = INVALID_HANDLE_VALUE;
- return p;
+ /* initialize DIR structure and copy first dir entry */
+ dir = xmalloc(sizeof(DIR));
+ dir-dd_handle = h;
+ dir-dd_stat = 0;
+ finddata2dirent(dir-dd_dir, fdata);
+ return dir;
}
struct
[PATCH 1/5] Win32 dirent: remove unused dirent.d_ino member
From: Karsten Blees bl...@dcon.de
Date: Fri, 7 Jan 2011 17:34:33 +0100
There are no proper inodes on Windows, so remove dirent.d_ino and #define
NO_D_INO_IN_DIRENT in the Makefile (this skips e.g. an ineffective qsort in
fsck.c).
Signed-off-by: Karsten Blees bl...@dcon.de
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/win32/dirent.h | 1 -
config.mak.uname | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/compat/win32/dirent.h b/compat/win32/dirent.h
index 927a25c..b38973b 100644
--- a/compat/win32/dirent.h
+++ b/compat/win32/dirent.h
@@ -9,7 +9,6 @@ typedef struct DIR DIR;
#define DT_LNK 3
struct dirent {
- long d_ino; /* Always zero. */
char d_name[FILENAME_MAX]; /* File name. */
union {
unsigned short d_reclen; /* Always zero. */
diff --git a/config.mak.uname b/config.mak.uname
index 1ae675b..8131c81 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -354,6 +354,7 @@ ifeq ($(uname_S),Windows)
NO_POSIX_GOODIES = UnfortunatelyYes
NATIVE_CRLF = YesPlease
DEFAULT_HELP_FORMAT = html
+ NO_D_INO_IN_DIRENT = YesPlease
CC = compat/vcbuild/scripts/clink.pl
AR = compat/vcbuild/scripts/lib.pl
@@ -503,6 +504,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_INET_NTOP = YesPlease
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
+ NO_D_INO_IN_DIRENT = YesPlease
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI
-Icompat -Icompat/win32
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
--
2.0.0.9635.g0be03cb
--
To unsubscribe from this list: send the line 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: [msysGit] Re: [PATCH 0/5] First part of Unicode console support for msysgit
Hello, On Fri, Jun 06, 2014 at 07:44:33PM +0200, Karsten Blees wrote: Karsten Blees (5): Support Unicode console output on Windows [..] you could have squashed half of Win32: fix segfault in WriteConsoleW when debugging in gdb [2] (second half in [5/5]). Detect console streams more reliably on Windows Warn if the Windows console font doesn't support Unicode I think this one includes MSVC: fix winansi.c compile errors [3] and Unicode console: fix font warning on Vista and Win7 [4] [...] Win32: move main macro to a function Win32: Thread-safe windows console output [2] https://github.com/msysgit/git/commit/cd0792af [3] https://github.com/msysgit/git/commit/3abcb04d [4] https://github.com/msysgit/git/commit/981aa538 Indeed, you identified them correctly. And [2] is actually squashed in [5/5]; I think I can keep it that way. I'll add the missing fix, take care about original dates, improve the cover letter (the above links), and resubmit. Thanks for review this batch of your patches. Regards, Stepan -- To unsubscribe from this list: send the line 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: show staged submodules regardless of ignore config
From: Jens Lehmann jens.lehm...@web.de
Date: Tue, 8 Apr 2014 21:36:08 +0200
Currently setting submodule.name.ignore and/or diff.ignoreSubmodules to
all suppresses all output of submodule changes for gitk. This is really
confusing, as even when the user chooses to record a new commit for an
ignored submodule by adding it manually this change won't show up under
Local changes checked in to index but not committed.
Fix that by using the '--ignore-submodules=dirty' option for both callers
of git diff-index --cached when the underlying git version supports that
option.
Signed-off-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Paul, I've been carrying this in my 'pu' but I would prefer
changes to gitk fed to me through you. Could you apply this so
that I can drop my tentative copy?
Thanks.
gitk | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/gitk b/gitk
index 90764e8..f6efaa6 100755
--- a/gitk
+++ b/gitk
@@ -5205,11 +5205,15 @@ proc dohidelocalchanges {} {
# spawn off a process to do git diff-index --cached HEAD
proc dodiffindex {} {
global lserial showlocalchanges vfilelimit curview
-global hasworktree
+global hasworktree git_version
if {!$showlocalchanges || !$hasworktree} return
incr lserial
-set cmd |git diff-index --cached HEAD
+if {[package vcompare $git_version 1.7.2] = 0} {
+ set cmd |git diff-index --cached --ignore-submodules=dirty HEAD
+} else {
+ set cmd |git diff-index --cached HEAD
+}
if {$vfilelimit($curview) ne {}} {
set cmd [concat $cmd -- $vfilelimit($curview)]
}
@@ -7705,7 +7709,7 @@ proc addtocflist {ids} {
}
proc diffcmd {ids flags} {
-global log_showroot nullid nullid2
+global log_showroot nullid nullid2 git_version
set i [lsearch -exact $ids $nullid]
set j [lsearch -exact $ids $nullid2]
@@ -7726,6 +7730,9 @@ proc diffcmd {ids flags} {
}
}
} elseif {$j = 0} {
+ if {[package vcompare $git_version 1.7.2] = 0} {
+ set flags $flags --ignore-submodules=dirty
+ }
set cmd [concat | git diff-index --cached $flags]
if {[llength $ids] 1} {
# comparing index with specific revision
--
2.0.0-531-gbd04298
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-gui: show staged submodules regardless of ignore config
From: Jens Lehmann jens.lehm...@web.de
Date: Tue, 8 Apr 2014 21:30:51 +0200
Currently setting submodule.name.ignore and/or diff.ignoreSubmodules to
all suppresses all output of submodule changes for git-gui. This is
really confusing, as even when the user chooses to record a new commit for
an ignored submodule by adding it manually this change won't show up under
Staged Changes (Will Commit).
Fix that by using the '--ignore-submodules=dirty' option for both callers
of git diff-index --cached when the underlying git version supports that
option.
Signed-off-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Pat, I've been carrying this in my 'pu' but I would prefer
changes to git-gui fed to me through you. Could you apply this
so that I can drop my tentative copy?
git-gui.sh | 6 +-
lib/diff.tcl | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/git-gui.sh b/git-gui.sh
index cf2209b..c69bfb3 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1558,7 +1558,11 @@ proc rescan_stage2 {fd after} {
set rescan_active 2
ui_status [mc Scanning for modified files ...]
- set fd_di [git_read diff-index --cached -z [PARENT]]
+ if {[git-version = 1.7.2]} {
+ set fd_di [git_read diff-index --cached
--ignore-submodules=dirty -z [PARENT]]
+ } else {
+ set fd_di [git_read diff-index --cached -z [PARENT]]
+ }
set fd_df [git_read diff-files -z]
fconfigure $fd_di -blocking 0 -translation binary -encoding binary
diff --git a/lib/diff.tcl b/lib/diff.tcl
index 30d9a79..b0a5180 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -287,6 +287,9 @@ proc start_show_diff {cont_info {add_opts {}}} {
if {$w eq $ui_index} {
lappend cmd diff-index
lappend cmd --cached
+ if {[git-version = 1.7.2]} {
+ lappend cmd --ignore-submodules=dirty
+ }
} elseif {$w eq $ui_workdir} {
if {[string first {U} $m] = 0} {
lappend cmd diff
--
2.0.0-531-gbd04298
--
To unsubscribe from this list: send the line 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/2] mingw: avoid const warning
Am 06.06.2014 21:13, schrieb Junio C Hamano:
Karsten Blees karsten.bl...@gmail.com writes:
Am 29.05.2014 12:47, schrieb Stepan Kasal:
Fix const warnings in http-fetch.c and remote-curl.c main() where is
argv declared as const.
The fix should work for all future declarations of main, no matter
whether the second parameter's type is char**, const char**, or
char *[].
I'm 100% in favor of a solution that doesn't restrict main to non-const
char**! Thanks.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/mingw.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 15f0c9d..8745d19 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -369,10 +369,11 @@ extern CRITICAL_SECTION pinfo_cs;
void mingw_startup();
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(c,v); \
-int main(int argc, char **argv) \
+int main(c, char **main_argv_not_used) \
{ \
+ typedef v, **argv_type; \
mingw_startup(); \
- return mingw_main(__argc, __argv); \
+ return mingw_main(__argc, (argv_type)__argv); \
} \
static int mingw_main(c,v)
I have to admit I had trouble understanding what 'typedef v,
**arv_type;' does (looks invalid at first glance), and why you
would need main_argv_not_used instead of just main(c,v).
So, I'd like to award +10 points for cleverness, but -10 for
obscurity ;-) Probably deserves a comment or an explanation in the
commit message.
Agreed. The typedef one is a cute hack.
I am wondering why the solution is not a more obvious drop const
that is not ANSI C, though. I only have a ready-access to N1570
draft but in it I find:
Actually, that was the original solution ($gmane/247535). I just complained
because it was slightly different from what we had in msysgit for quite some
time [1], causing merge conflicts.
I guess compilers probably won't complain if you declare argv const, even if
the standard is more strict. After all, you aren't supposed to modify argv.
A simpler solution that works with all definitions of main() is to
cast to void* (tell the compiler all responsibility is on
us).
Can you cast away the constness that way, though?
Not 'away'. This passes a non-const value to a const parameter, which is
typically not a problem. Its just 'char**' to 'const char**' that produces the
warning, because 'const char' hides behind a non-const pointer, see [2].
'void*' to 'const char**' works, though.
[1] https://github.com/msysgit/git/commit/6949537a
[2] http://c-faq.com/ansi/constmismatch.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] Warn if the Windows console font doesn't support Unicode
Stepan Kasal: + warning(Your console font probably doesn\'t support Unicode. If + you experience strange characters in the output, consider + switching to a TrueType font such as Lucida Console!); As you mention this is an old patch series, but I would recommend modernizing the suggestion here to recomment Consolas. It is available in all current versions of Windows (Vista and later), and seem to have better Unicode support according to http://www.fileformat.info/info/unicode/font/consolas/list.htm vs http://www.fileformat.info/info/unicode/font/lucida_console/list.htm -- \\// Peter - http://www.softwolves.pp.se/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Win32: Thread-safe windows console output
Stepan Kasal: + /* only called from console_thread, so a static buffer will do */ + static wchar_t wbuf[2 * BUFFER_SIZE + 1]; Wouldn't BUFFER_SIZE + 1 (or even BUFFER_SIZE) do here? If you convert from up to BUFFER_SIZE octets of UTF-8 input, you should never get back more than BUFFER_SIZE code units of UTF-16 output. Worst case would be ASCII, which is one UTF-16 code unit per UTF-8 octet, everything else is less (non-BMP is four UTF-8 octets mapping to two UTF-16 code units). -- \\// Peter - http://www.softwolves.pp.se/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Win32: Thread-safe windows console output
Am 06.06.2014 23:29, schrieb Peter Krefting: Stepan Kasal: +/* only called from console_thread, so a static buffer will do */ +static wchar_t wbuf[2 * BUFFER_SIZE + 1]; Wouldn't BUFFER_SIZE + 1 (or even BUFFER_SIZE) do here? If you convert from up to BUFFER_SIZE octets of UTF-8 input, you should never get back more than BUFFER_SIZE code units of UTF-16 output. Worst case would be ASCII, which is one UTF-16 code unit per UTF-8 octet, everything else is less (non-BMP is four UTF-8 octets mapping to two UTF-16 code units). You're right for MultiByteToWideChar. However, the next patch series will introduce another conversion function that converts invalid UTF-8 to hex code, i.e. two wide chars per invalid UTF-8 char, +1 for L'\0' (see [1] mingw.h:365ff for space requirement rationale). And yet another patch will replace this patch's MultiByteToWideChar for consistentcy [2]. [1] https://github.com/msysgit/git/commit/018c94a8 [2] https://github.com/msysgit/git/commit/45e28a4d -- To unsubscribe from this list: send the line 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 v14 03/40] refs.c: constify the sha arguments for ref_transaction_create|delete|update
ref_transaction_create|delete|update has no need to modify the sha1
arguments passed to it so it should use const unsigned char* instead
of unsigned char*.
Some functions, such as fast_forward_to(), already have its old/new
sha1 arguments as consts. This function will at some point need to
use ref_transaction_update() in which case this change is required.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 7 ---
refs.h | 7 ---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 33541f4..a767ef6 100644
--- a/refs.c
+++ b/refs.c
@@ -,7 +,8 @@ static struct ref_update *add_update(struct
ref_transaction *transaction,
void ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1, unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old)
{
struct ref_update *update = add_update(transaction, refname);
@@ -3347,7 +3348,7 @@ void ref_transaction_update(struct ref_transaction
*transaction,
void ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1,
+ const unsigned char *new_sha1,
int flags)
{
struct ref_update *update = add_update(transaction, refname);
@@ -3361,7 +3362,7 @@ void ref_transaction_create(struct ref_transaction
*transaction,
void ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
- unsigned char *old_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old)
{
struct ref_update *update = add_update(transaction, refname);
diff --git a/refs.h b/refs.h
index 306d833..b893838 100644
--- a/refs.h
+++ b/refs.h
@@ -237,7 +237,8 @@ struct ref_transaction *ref_transaction_begin(void);
*/
void ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1, unsigned char *old_sha1,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old);
/*
@@ -248,7 +249,7 @@ void ref_transaction_update(struct ref_transaction
*transaction,
*/
void ref_transaction_create(struct ref_transaction *transaction,
const char *refname,
- unsigned char *new_sha1,
+ const unsigned char *new_sha1,
int flags);
/*
@@ -258,7 +259,7 @@ void ref_transaction_create(struct ref_transaction
*transaction,
*/
void ref_transaction_delete(struct ref_transaction *transaction,
const char *refname,
- unsigned char *old_sha1,
+ const unsigned char *old_sha1,
int flags, int have_old);
/*
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 02/40] refs.c: ref_transaction_commit should not free the transaction
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 1 +
refs.c | 1 -
refs.h | 5 ++---
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 405267f..1fd7a89 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -369,6 +369,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
update_refs_stdin();
ret = ref_transaction_commit(transaction, msg,
UPDATE_REFS_DIE_ON_ERR);
+ ref_transaction_free(transaction);
return ret;
}
diff --git a/refs.c b/refs.c
index 48573e3..33541f4 100644
--- a/refs.c
+++ b/refs.c
@@ -3483,7 +3483,6 @@ cleanup:
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
free(delnames);
- ref_transaction_free(transaction);
return ret;
}
diff --git a/refs.h b/refs.h
index a07a5d0..306d833 100644
--- a/refs.h
+++ b/refs.h
@@ -216,8 +216,7 @@ enum action_on_err {
/*
* Begin a reference transaction. The reference transaction must
- * eventually be commited using ref_transaction_commit() or freed by
- * calling ref_transaction_free().
+ * be freed by calling ref_transaction_free().
*/
struct ref_transaction *ref_transaction_begin(void);
@@ -265,7 +264,7 @@ void ref_transaction_delete(struct ref_transaction
*transaction,
/*
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
- * problem. The ref_transaction is freed by this function.
+ * problem.
*/
int ref_transaction_commit(struct ref_transaction *transaction,
const char *msg, enum action_on_err onerr);
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 37/40] refs.c: pass a skip list to name_conflict_fn
Allow passing a list of refs to skip checking to name_conflict_fn.
There are some conditions where we want to allow a temporary conflict and skip
checking those refs. For example if we have a transaction that
1, guarantees that m is a packed refs and there is no loose ref for m
2, the transaction will delete m from the packed ref
3, the transaction will create conflicting m/m
For this case we want to be able to lock and create m/m since we know that the
conflict is only transient. I.e. the conflict will be automatically resolved
by the transaction when it deletes m.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/refs.c b/refs.c
index 66427f0..a5fd943 100644
--- a/refs.c
+++ b/refs.c
@@ -791,15 +791,18 @@ static int names_conflict(const char *refname1, const
char *refname2)
struct name_conflict_cb {
const char *refname;
- const char *oldrefname;
const char *conflicting_refname;
+ const char **skip;
+ int skipnum;
};
static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
{
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
- if (data-oldrefname !strcmp(data-oldrefname, entry-name))
- return 0;
+ int i;
+ for (i = 0; i data-skipnum; i++)
+ if (!strcmp(entry-name, data-skip[i]))
+ return 0;
if (names_conflict(data-refname, entry-name)) {
data-conflicting_refname = entry-name;
return 1;
@@ -812,15 +815,18 @@ static int name_conflict_fn(struct ref_entry *entry, void
*cb_data)
* conflicting with the name of an existing reference in dir. If
* oldrefname is non-NULL, ignore potential conflicts with oldrefname
* (e.g., because oldrefname is scheduled for deletion in the same
- * operation).
+ * operation). skip contains a list of refs we want to skip checking for
+ * conflicts with.
*/
-static int is_refname_available(const char *refname, const char *oldrefname,
- struct ref_dir *dir)
+static int is_refname_available(const char *refname,
+ struct ref_dir *dir,
+ const char **skip, int skipnum)
{
struct name_conflict_cb data;
data.refname = refname;
- data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
+ data.skip = skip;
+ data.skipnum = skipnum;
sort_ref_dir(dir);
if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, data)) {
@@ -2047,7 +2053,8 @@ int dwim_log(const char *str, int len, unsigned char
*sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
- int flags, int *type_p)
+ int flags, int *type_p,
+ const char **skip, int skipnum)
{
char *ref_file;
const char *orig_refname = refname;
@@ -2096,7 +2103,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* name is a proper prefix of our refname.
*/
if (missing
-!is_refname_available(refname, NULL, get_packed_refs(ref_cache)))
{
+!is_refname_available(refname, get_packed_refs(ref_cache),
+ skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2154,7 +2162,7 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
+ return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);
}
/*
@@ -2645,10 +2653,12 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (!symref)
return error(refname %s not found, oldrefname);
- if (!is_refname_available(newrefname, oldrefname,
get_packed_refs(ref_cache)))
+ if (!is_refname_available(newrefname, get_packed_refs(ref_cache),
+ oldrefname, 1))
return 1;
- if (!is_refname_available(newrefname, oldrefname,
get_loose_refs(ref_cache)))
+ if (!is_refname_available(newrefname, get_loose_refs(ref_cache),
+ oldrefname, 1))
return 1;
if (log rename(git_path(logs/%s, oldrefname),
git_path(TMP_RENAMED_LOG)))
@@ -2678,7 +2688,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
logmoved = log;
- lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL);
+ lock =
[PATCH v14 20/40] sequencer.c: use ref transactions for all ref updates
Change to use ref transactions for all updates to refs.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
sequencer.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0a80c58..fd8acaf 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -272,23 +272,31 @@ static int error_dirty_index(struct replay_opts *opts)
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
int unborn, struct replay_opts *opts)
{
- struct ref_lock *ref_lock;
+ struct ref_transaction *transaction;
struct strbuf sb = STRBUF_INIT;
- int ret;
+ struct strbuf err = STRBUF_INIT;
read_cache();
if (checkout_fast_forward(from, to, 1))
exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from,
- 0, NULL);
- if (!ref_lock)
- return error(_(Failed to lock HEAD during fast_forward_to));
strbuf_addf(sb, %s: fast-forward, action_name(opts));
- ret = write_ref_sha1(ref_lock, to, sb.buf);
+
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, to, from,
+ 0, !unborn, err) ||
+ ref_transaction_commit(transaction, sb.buf, err)) {
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(sb);
+ strbuf_release(err);
+ return -1;
+ }
strbuf_release(sb);
- return ret;
+ ref_transaction_free(transaction);
+ return 0;
}
static int do_recursive_merge(struct commit *base, struct commit *next,
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 17/40] tag.c: use ref transactions when doing updates
Change tag.c to use ref transactions for all ref updates.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/tag.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..c9bfc9a 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
- struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
@@ -701,11 +702,13 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
- if (!lock)
- die(_(%s: cannot lock the ref), ref.buf);
- if (write_ref_sha1(lock, object, NULL) 0)
- die(_(%s: cannot update the ref), ref.buf);
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, object, prev,
+ 0, !is_null_sha1(prev), err) ||
+ ref_transaction_commit(transaction, NULL, err))
+ die(%s, err.buf);
+ ref_transaction_free(transaction);
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 09/40] refs.c: make update_ref_write update a strbuf on failure
Change update_ref_write to also update an error strbuf on failure.
This makes the error available to ref_transaction_commit callers if the
transaction failed due to update_ref_sha1/write_ref_sha1 failures.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index dd498cf..6696082 100644
--- a/refs.c
+++ b/refs.c
@@ -3343,10 +3343,13 @@ static struct ref_lock *update_ref_lock(const char
*refname,
static int update_ref_write(const char *action, const char *refname,
const unsigned char *sha1, struct ref_lock *lock,
- enum action_on_err onerr)
+ struct strbuf *err, enum action_on_err onerr)
{
if (write_ref_sha1(lock, sha1, action) 0) {
const char *str = Cannot update the ref '%s'.;
+ if (err)
+ strbuf_addf(err, str, refname);
+
switch (onerr) {
case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
@@ -3467,7 +3470,7 @@ int update_ref(const char *action, const char *refname,
lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
if (!lock)
return 1;
- return update_ref_write(action, refname, sha1, lock, onerr);
+ return update_ref_write(action, refname, sha1, lock, NULL, onerr);
}
static int ref_update_compare(const void *r1, const void *r2)
@@ -3549,7 +3552,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
ret = update_ref_write(msg,
update-refname,
update-new_sha1,
- update-lock, onerr);
+ update-lock, err, onerr);
update-lock = NULL; /* freed by update_ref_write */
if (ret)
goto cleanup;
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 15/40] refs.c: make ref_transaction_begin take an err argument
Add an err argument to _begin so that on non-fatal failures in future ref
backends we can report a nice error back to the caller.
While _begin can currently never fail for other reasons than OOM, in which
case we die() anyway, we may add other types of backends in the future.
For example, a hypothetical MySQL backend could fail in _being with
Can not connect to MySQL server. No route to host.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 2 +-
refs.c | 2 +-
refs.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 7c9c248..c6ad0be 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -365,7 +365,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
die(Refusing to perform update with empty message.);
if (read_stdin) {
- transaction = ref_transaction_begin();
+ transaction = ref_transaction_begin(err);
if (delete || no_deref || argc 0)
usage_with_options(git_update_ref_usage, options);
if (end_null)
diff --git a/refs.c b/refs.c
index d9b99c7..ee87eda 100644
--- a/refs.c
+++ b/refs.c
@@ -3387,7 +3387,7 @@ struct ref_transaction {
size_t nr;
};
-struct ref_transaction *ref_transaction_begin(void)
+struct ref_transaction *ref_transaction_begin(struct strbuf *err)
{
return xcalloc(1, sizeof(struct ref_transaction));
}
diff --git a/refs.h b/refs.h
index 58ba365..743358f 100644
--- a/refs.h
+++ b/refs.h
@@ -257,7 +257,7 @@ enum action_on_err {
* Begin a reference transaction. The reference transaction must
* be freed by calling ref_transaction_free().
*/
-struct ref_transaction *ref_transaction_begin(void);
+struct ref_transaction *ref_transaction_begin(struct strbuf *err);
/*
* The following functions add a reference check or update to a
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 32/40] refs.c: make delete_ref use a transaction
Change delete_ref to use a ref transaction for the deletion. At the same time
since we no longer have any callers of repack_without_ref we can now delete
this function.
Change delete_ref to return 0 on success and 1 on failure instead of the
previous 0 on success either 1 or -1 on failure.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 34 +-
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index e19041a..4cdbc26 100644
--- a/refs.c
+++ b/refs.c
@@ -2513,11 +2513,6 @@ static int repack_without_refs(const char **refnames,
int n, struct strbuf *err)
return ret;
}
-static int repack_without_ref(const char *refname)
-{
- return repack_without_refs(refname, 1, NULL);
-}
-
static int add_err_if_unremovable(const char *op, const char *file,
struct strbuf *e, int rc)
{
@@ -2557,24 +2552,21 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
- struct ref_lock *lock;
- int ret = 0, flag = 0;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
- lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
- if (!lock)
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_delete(transaction, refname, sha1, delopt,
+ sha1 !is_null_sha1(sha1), err) ||
+ ref_transaction_commit(transaction, NULL, err)) {
+ error(%s, err.buf);
+ ref_transaction_free(transaction);
+ strbuf_release(err);
return 1;
- ret |= delete_ref_loose(lock, flag, NULL);
-
- /* removing the loose one could have resurrected an earlier
-* packed one. Also, if it was not loose we need to repack
-* without it.
-*/
- ret |= repack_without_ref(lock-ref_name);
-
- unlink_or_warn(git_path(logs/%s, lock-ref_name));
- clear_loose_ref_cache(ref_cache);
- unlock_ref(lock);
- return ret;
+ }
+ ref_transaction_free(transaction);
+ return 0;
}
/*
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 36/40] refs.c: call lock_ref_sha1_basic directly from commit
Skip using the lock_any_ref_for_update wrapper and call lock_ref_sha1_basic
directly from the commit function.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 7da5357..66427f0 100644
--- a/refs.c
+++ b/refs.c
@@ -3564,12 +3564,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
- update-lock = lock_any_ref_for_update(update-refname,
- (update-have_old ?
- update-old_sha1 :
- NULL),
- update-flags,
- update-type);
+ update-lock = lock_ref_sha1_basic(update-refname,
+ (update-have_old ?
+ update-old_sha1 :
+ NULL),
+ update-flags,
+ update-type);
if (!update-lock) {
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 25/40] fast-import.c: use a ref transaction when dumping tags
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
fast-import.c | 29 +++--
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 4a7b196..587ef4a 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1735,15 +1735,32 @@ static void dump_tags(void)
{
static const char *msg = fast-import;
struct tag *t;
- struct ref_lock *lock;
- char ref_name[PATH_MAX];
+ struct strbuf ref_name = STRBUF_INIT;
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction;
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ failure |= error(%s, err.buf);
+ goto cleanup;
+ }
for (t = first_tag; t; t = t-next_tag) {
- sprintf(ref_name, tags/%s, t-name);
- lock = lock_ref_sha1(ref_name, NULL);
- if (!lock || write_ref_sha1(lock, t-sha1, msg) 0)
- failure |= error(Unable to update %s, ref_name);
+ strbuf_reset(ref_name);
+ strbuf_addf(ref_name, refs/tags/%s, t-name);
+
+ if (ref_transaction_update(transaction, ref_name.buf, t-sha1,
+ NULL, 0, 0, err)) {
+ failure |= error(%s, err.buf);
+ goto cleanup;
+ }
}
+ if (ref_transaction_commit(transaction, msg, err))
+ failure |= error(%s, err.buf);
+
+ cleanup:
+ ref_transaction_free(transaction);
+ strbuf_release(ref_name);
+ strbuf_release(err);
}
static void dump_marks_helper(FILE *f,
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 14/40] refs.c: update ref_transaction_delete to check for error and return status
Change ref_transaction_delete() to do basic error checking and return
non-zero of error. Update all callers to check the return for
ref_transaction_delete(). There are currently no conditions in _delete that
will return error but there will be in the future. Add an err argument that
will be updated on failure.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 5 +++--
refs.c | 16 +++-
refs.h | 12
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 41121fa..7c9c248 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -258,8 +258,9 @@ static const char *parse_cmd_delete(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(delete %s: extra input: %s, refname, next);
- ref_transaction_delete(transaction, refname, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_delete(transaction, refname, old_sha1,
+ update_flags, have_old, err))
+ die(%s, err.buf);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index 3cb99f6..d9b99c7 100644
--- a/refs.c
+++ b/refs.c
@@ -3459,19 +3459,25 @@ int ref_transaction_create(struct ref_transaction
*transaction,
return 0;
}
-void ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old)
+int ref_transaction_delete(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old,
+ struct strbuf *err)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+ if (have_old !old_sha1)
+ die(BUG: have_old is true but old_sha1 is NULL);
+
+ update = add_update(transaction, refname);
update-flags = flags;
update-have_old = have_old;
if (have_old) {
assert(!is_null_sha1(old_sha1));
hashcpy(update-old_sha1, old_sha1);
}
+ return 0;
}
int update_ref(const char *action, const char *refname,
diff --git a/refs.h b/refs.h
index bda790e6..58ba365 100644
--- a/refs.h
+++ b/refs.h
@@ -303,11 +303,15 @@ int ref_transaction_create(struct ref_transaction
*transaction,
* Add a reference deletion to transaction. If have_old is true, then
* old_sha1 holds the value that the reference should have had before
* the update (which must not be the null SHA-1).
+ * Function returns 0 on success and non-zero on failure. A failure to delete
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back.
*/
-void ref_transaction_delete(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *old_sha1,
- int flags, int have_old);
+int ref_transaction_delete(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *old_sha1,
+ int flags, int have_old,
+ struct strbuf *err);
/*
* Commit all of the changes that have been queued in transaction, as
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 33/40] refs.c: pass the ref log message to _create/delete/update instead of _commit
Change the reference transactions so that we pass the reflog message
through to the create/delete/update function instead of the commit message.
This allows for individual messages for each change in a multi ref
transaction.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 4 ++--
builtin/commit.c | 4 ++--
builtin/fetch.c| 3 +--
builtin/receive-pack.c | 7 ---
builtin/replace.c | 4 ++--
builtin/tag.c | 4 ++--
builtin/update-ref.c | 13 +++--
fast-import.c | 8
refs.c | 34 +-
refs.h | 8
sequencer.c| 4 ++--
walker.c | 5 ++---
12 files changed, 53 insertions(+), 45 deletions(-)
diff --git a/branch.c b/branch.c
index c1eae00..e0439af 100644
--- a/branch.c
+++ b/branch.c
@@ -301,8 +301,8 @@ void create_branch(const char *head,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, sha1,
- null_sha1, 0, !forcing, err) ||
- ref_transaction_commit(transaction, msg, err))
+ null_sha1, 0, !forcing, msg, err) ||
+ ref_transaction_commit(transaction, err))
die(%s, err.buf);
ref_transaction_free(transaction);
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 14cd9f4..e01b333 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1753,8 +1753,8 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
ref_transaction_update(transaction, HEAD, sha1,
current_head ?
current_head-object.sha1 : NULL,
- 0, !!current_head, err) ||
- ref_transaction_commit(transaction, sb.buf, err)) {
+ 0, !!current_head, sb.buf, err) ||
+ ref_transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 55f457c..faa1233 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -673,10 +673,9 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
}
}
}
-
if (rc STORE_REF_ERROR_DF_CONFLICT)
error(_(some local refs could not be updated; try running\n
- 'git remote prune %s' to remove any old, conflicting
+ 'git remote prune %s' to remove any old, conflicting
branches), remote_name);
abort:
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 13f4a63..5653fa2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -586,10 +586,11 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, namespaced_name,
- new_sha1, old_sha1, 0, 1, err) ||
- ref_transaction_commit(transaction, push, err)) {
-
+ new_sha1, old_sha1, 0, 1, push,
+ err) ||
+ ref_transaction_commit(transaction, err)) {
const char *str;
+
string_list_append(error_strings, err.buf);
str = error_strings.items[error_strings.nr - 1].string;
strbuf_release(err);
diff --git a/builtin/replace.c b/builtin/replace.c
index cf92e5d..c42b26e 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -171,8 +171,8 @@ static int replace_object_sha1(const char *object_ref,
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref, repl, prev,
- 0, !is_null_sha1(prev), err) ||
- ref_transaction_commit(transaction, NULL, err))
+ 0, !is_null_sha1(prev), NULL, err) ||
+ ref_transaction_commit(transaction, err))
die(%s, err.buf);
ref_transaction_free(transaction);
diff --git a/builtin/tag.c b/builtin/tag.c
index c9bfc9a..74af63e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -705,8 +705,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
- 0, !is_null_sha1(prev), err) ||
-
[PATCH v14 40/40] refs.c: make write_ref_sha1 static
No external users call write_ref_sha1 any more so lets declare it static.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 6 +-
refs.h | 3 ---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 9a3a501..c9d2a89 100644
--- a/refs.c
+++ b/refs.c
@@ -2634,6 +2634,9 @@ static int rename_tmp_log(const char *newrefname)
return 0;
}
+static int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
+ const char *logmsg);
+
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
unsigned char sha1[20], orig_sha1[20];
@@ -2882,7 +2885,8 @@ static int is_branch(const char *refname)
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
}
-int write_ref_sha1(struct ref_lock *lock,
+/** Writes sha1 into the ref specified by the lock. **/
+static int write_ref_sha1(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
{
static char term = '\n';
diff --git a/refs.h b/refs.h
index 1583097..8177052 100644
--- a/refs.h
+++ b/refs.h
@@ -195,9 +195,6 @@ extern int commit_ref(struct ref_lock *lock);
/** Release any lock taken but not written. **/
extern void unlock_ref(struct ref_lock *lock);
-/** Writes sha1 into the ref specified by the lock. **/
-extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1,
const char *msg);
-
/** Setup reflog before using. **/
int log_ref_setup(const char *refname, char *logfile, int bufsize);
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 18/40] replace.c: use the ref transaction functions for updates
Update replace.c to use ref transactions for updates. Reviewed-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/replace.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 4b3705d..cf92e5d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -154,7 +154,8 @@ static int replace_object_sha1(const char *object_ref, unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; - struct ref_lock *lock; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; obj_type = sha1_object_info(object, NULL); repl_type = sha1_object_info(repl, NULL); @@ -167,12 +168,14 @@ static int replace_object_sha1(const char *object_ref, check_ref_valid(object, prev, ref, sizeof(ref), force); - lock = lock_any_ref_for_update(ref, prev, 0, NULL); - if (!lock) - die(%s: cannot lock the ref, ref); - if (write_ref_sha1(lock, repl, NULL) 0) - die(%s: cannot update the ref, ref); + transaction = ref_transaction_begin(err); + if (!transaction || + ref_transaction_update(transaction, ref, repl, prev, + 0, !is_null_sha1(prev), err) || + ref_transaction_commit(transaction, NULL, err)) + die(%s, err.buf); + ref_transaction_free(transaction); return 0; } -- 2.0.0.582.ge25c160 -- To unsubscribe from this list: send the line 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 v14 34/40] refs.c: pass NULL as *flags to read_ref_full
We call read_ref_full with a pointer to flags from rename_ref but since
we never actually use the returned flags we can just pass NULL here instead.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 88d9351..8848dbf 100644
--- a/refs.c
+++ b/refs.c
@@ -2657,7 +2657,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
goto rollback;
}
- if (!read_ref_full(newrefname, sha1, 1, flag)
+ if (!read_ref_full(newrefname, sha1, 1, NULL)
delete_ref(newrefname, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
if (remove_empty_directories(git_path(%s,
newrefname))) {
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 12/40] refs.c: change ref_transaction_update() to do error checking and return status
Update ref_transaction_update() do some basic error checking and return
non-zero on error. Update all callers to check ref_transaction_update() for
error. There are currently no conditions in _update that will return error but
there will be in the future. Add an err argument that will be updated on
failure. In future patches we will start doing both locking and checking
for name conflicts in _update instead of _commit at which time this function
will start returning errors for these conditions.
Also check for BUGs during update and die(BUG:...) if we are calling
_update with have_old but the old_sha1 pointer is NULL.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 12 +++-
refs.c | 18 --
refs.h | 14 +-
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 88ab785..3067b11 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -16,6 +16,7 @@ static struct ref_transaction *transaction;
static char line_termination = '\n';
static int update_flags;
+static struct strbuf err = STRBUF_INIT;
/*
* Parse one whitespace- or NUL-terminated, possibly C-quoted argument
@@ -197,8 +198,9 @@ static const char *parse_cmd_update(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(update %s: extra input: %s, refname, next);
- ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old, err))
+ die(%s, err.buf);
update_flags = 0;
free(refname);
@@ -286,8 +288,9 @@ static const char *parse_cmd_verify(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(verify %s: extra input: %s, refname, next);
- ref_transaction_update(transaction, refname, new_sha1, old_sha1,
- update_flags, have_old);
+ if (ref_transaction_update(transaction, refname, new_sha1, old_sha1,
+ update_flags, have_old, err))
+ die(%s, err.buf);
update_flags = 0;
free(refname);
@@ -342,7 +345,6 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
const char *refname, *oldval, *msg = NULL;
unsigned char sha1[20], oldsha1[20];
int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0, flags = 0;
- struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_STRING( 'm', NULL, msg, N_(reason), N_(reason of the
update)),
OPT_BOOL('d', NULL, delete, N_(delete the reference)),
diff --git a/refs.c b/refs.c
index fb44978..f5e7a12 100644
--- a/refs.c
+++ b/refs.c
@@ -3418,19 +3418,25 @@ static struct ref_update *add_update(struct
ref_transaction *transaction,
return update;
}
-void ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- int flags, int have_old)
+int ref_transaction_update(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ int flags, int have_old,
+ struct strbuf *err)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+
+ if (have_old !old_sha1)
+ die(BUG: have_old is true but old_sha1 is NULL);
+ update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
update-flags = flags;
update-have_old = have_old;
if (have_old)
hashcpy(update-old_sha1, old_sha1);
+ return 0;
}
void ref_transaction_create(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index 6ee9c9e..32edf3f 100644
--- a/refs.h
+++ b/refs.h
@@ -234,12 +234,16 @@ struct ref_transaction *ref_transaction_begin(void);
* be deleted. If have_old is true, then old_sha1 holds the value
* that the reference should have had before the update, or zeros if
* it must not have existed beforehand.
+ * Function returns 0 on success and non-zero on failure. A failure to update
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back. On failure the err buffer will be updated.
*/
-void ref_transaction_update(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char
[PATCH v14 39/40] fetch.c: change s_update_ref to use a ref transaction
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 33 +++--
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index faa1233..52f1ebc 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,23 +375,36 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+ int ret, df_conflict = 0;
if (dry_run)
return 0;
if (!rla)
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
- lock = lock_any_ref_for_update(ref-name,
- check_old ? ref-old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
+
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old, msg, err))
+ goto fail;
+
+ ret = ref_transaction_commit(transaction, err);
+ if (ret == UPDATE_REFS_NAME_CONFLICT)
+ df_conflict = 1;
+ if (ret)
+ goto fail;
+
+ ref_transaction_free(transaction);
return 0;
+fail:
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(err);
+ return df_conflict ? STORE_REF_ERROR_DF_CONFLICT
+ : STORE_REF_ERROR_OTHER;
}
#define REFCOL_WIDTH 10
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 38/40] refs.c: propagate any errno==ENOTDIR from _commit back to the callers
In _commit, ENOTDIR can happen in the call to lock_ref_sha1_basic, either when
we lstat the new refname and it returns ENOTDIR or if the name checking
function reports that the same type of conflict happened. In both cases it
means that we can not create the new ref due to a name conflict.
For these cases, save the errno value and abort and make sure that the caller
can see errno==ENOTDIR.
Also start defining specific return codes for _commit, assign -1 as a generic
error and -2 as the error that refers to a name conflict. Callers can (and
should) use that return value inspecting errno directly.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 22 +++---
refs.h | 6 ++
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index a5fd943..9a3a501 100644
--- a/refs.c
+++ b/refs.c
@@ -3548,7 +3548,7 @@ static int ref_update_reject_duplicates(struct ref_update
**updates, int n,
int ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err)
{
- int ret = 0, delnum = 0, i;
+ int ret = 0, delnum = 0, i, df_conflict = 0;
const char **delnames;
int n = transaction-nr;
struct ref_update **updates = transaction-updates;
@@ -3566,9 +3566,10 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
/* Copy, sort, and reject duplicate refs */
qsort(updates, n, sizeof(*updates), ref_update_compare);
- ret = ref_update_reject_duplicates(updates, n, err);
- if (ret)
+ if (ref_update_reject_duplicates(updates, n, err)) {
+ ret = -1;
goto cleanup;
+ }
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
@@ -3582,10 +3583,12 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
update-type,
delnames, delnum);
if (!update-lock) {
+ if (errno == ENOTDIR)
+ df_conflict = 1;
if (err)
strbuf_addf(err, Cannot lock the ref '%s'.,
update-refname);
- ret = 1;
+ ret = -1;
goto cleanup;
}
}
@@ -3603,6 +3606,7 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (err)
strbuf_addf(err, str, update-refname);
+ ret = -1;
goto cleanup;
}
}
@@ -3613,14 +3617,16 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (update-lock) {
- ret |= delete_ref_loose(update-lock, update-type,
- err);
+ if (delete_ref_loose(update-lock, update-type, err))
+ ret = -1;
+
if (!(update-flags REF_ISPRUNING))
delnames[delnum++] = update-lock-ref_name;
}
}
- ret |= repack_without_refs(delnames, delnum, err);
+ if (repack_without_refs(delnames, delnum, err))
+ ret = -1;
for (i = 0; i delnum; i++)
unlink_or_warn(git_path(logs/%s, delnames[i]));
clear_loose_ref_cache(ref_cache);
@@ -3633,6 +3639,8 @@ cleanup:
if (updates[i]-lock)
unlock_ref(updates[i]-lock);
free(delnames);
+ if (df_conflict)
+ ret = -2;
return ret;
}
diff --git a/refs.h b/refs.h
index 5c6c20f..1583097 100644
--- a/refs.h
+++ b/refs.h
@@ -323,7 +323,13 @@ int ref_transaction_delete(struct ref_transaction
*transaction,
* Commit all of the changes that have been queued in transaction, as
* atomically as possible. Return a nonzero value if there is a
* problem.
+ * If the transaction is already in failed state this function will return
+ * an error.
+ * Function returns 0 on success, -1 for generic failures and
+ * UPDATE_REFS_NAME_CONFLICT (-2) if the failure was due to a name
+ * collision (ENOTDIR).
*/
+#define UPDATE_REFS_NAME_CONFLICT -2
int ref_transaction_commit(struct ref_transaction *transaction,
struct strbuf *err);
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 35/40] refs.c: move the check for valid refname to lock_ref_sha1_basic
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.
If lock_ref_sha1_basic fails the check_refname_format test, set errno to
EINVAL before returning NULL. This to guarantee that we will not return an
error without updating errno.
This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.
This changes semantics for lock_ref_sha1_basic slightly. With this change
it is no longer possible to open a ref that has a badly name which breaks
any codepaths that tries to open and repair badly named refs. The normal refs
API should not allow neither creating nor accessing refs with invalid names.
If we need such recovery code we could add it as an option to git fsck and have
git fsck be the only sanctioned way of bypassing the normal API and checks.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 8848dbf..7da5357 100644
--- a/refs.c
+++ b/refs.c
@@ -2058,6 +2058,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int missing = 0;
int attempts_remaining = 3;
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
+ errno = EINVAL;
+ return NULL;
+ }
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
@@ -2149,8 +2154,6 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
- return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
}
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line 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 v14 13/40] refs.c: change ref_transaction_create to do error checking and return status
Do basic error checking in ref_transaction_create() and make it return
non-zero on error. Update all callers to check the result of
ref_transaction_create(). There are currently no conditions in _create that
will return error but there will be in the future. Add an err argument that
will be updated on failure.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/update-ref.c | 4 +++-
refs.c | 18 +++--
refs.h | 55 +---
3 files changed, 63 insertions(+), 14 deletions(-)
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 3067b11..41121fa 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -226,7 +226,9 @@ static const char *parse_cmd_create(struct strbuf *input,
const char *next)
if (*next != line_termination)
die(create %s: extra input: %s, refname, next);
- ref_transaction_create(transaction, refname, new_sha1, update_flags);
+ if (ref_transaction_create(transaction, refname, new_sha1,
+ update_flags, err))
+ die(%s, err.buf);
update_flags = 0;
free(refname);
diff --git a/refs.c b/refs.c
index f5e7a12..3cb99f6 100644
--- a/refs.c
+++ b/refs.c
@@ -3439,18 +3439,24 @@ int ref_transaction_update(struct ref_transaction
*transaction,
return 0;
}
-void ref_transaction_create(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- int flags)
+int ref_transaction_create(struct ref_transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ int flags,
+ struct strbuf *err)
{
- struct ref_update *update = add_update(transaction, refname);
+ struct ref_update *update;
+
+ if (!new_sha1 || is_null_sha1(new_sha1))
+ die(BUG: create ref with null new_sha1);
+
+ update = add_update(transaction, refname);
- assert(!is_null_sha1(new_sha1));
hashcpy(update-new_sha1, new_sha1);
hashclr(update-old_sha1);
update-flags = flags;
update-have_old = 1;
+ return 0;
}
void ref_transaction_delete(struct ref_transaction *transaction,
diff --git a/refs.h b/refs.h
index 32edf3f..bda790e6 100644
--- a/refs.h
+++ b/refs.h
@@ -10,6 +10,45 @@ struct ref_lock {
int force_write;
};
+/*
+ * A ref_transaction represents a collection of ref updates
+ * that should succeed or fail together.
+ *
+ * Calling sequence
+ *
+ * - Allocate and initialize a `struct ref_transaction` by calling
+ * `ref_transaction_begin()`.
+ *
+ * - List intended ref updates by calling functions like
+ * `ref_transaction_update()` and `ref_transaction_create()`.
+ *
+ * - Call `ref_transaction_commit()` to execute the transaction.
+ * If this succeeds, the ref updates will have taken place and
+ * the transaction cannot be rolled back.
+ *
+ * - At any time call `ref_transaction_free()` to discard the
+ * transaction and free associated resources. In particular,
+ * this rolls back the transaction if it has not been
+ * successfully committed.
+ *
+ * Error handling
+ * --
+ *
+ * On error, transaction functions append a message about what
+ * went wrong to the 'err' argument. The message mentions what
+ * ref was being updated (if any) when the error occurred so it
+ * can be passed to 'die' or 'error' as-is.
+ *
+ * The message is appended to err without first clearing err.
+ * This allows the caller to prepare preamble text to the generated
+ * error message:
+ *
+ * strbuf_addf(err, Error while doing foo-bar: );
+ * if (ref_transaction_update(..., err)) {
+ * ret = error(%s, err.buf);
+ * goto cleanup;
+ * }
+ */
struct ref_transaction;
/*
@@ -236,7 +275,7 @@ struct ref_transaction *ref_transaction_begin(void);
* it must not have existed beforehand.
* Function returns 0 on success and non-zero on failure. A failure to update
* means that the transaction as a whole has failed and will need to be
- * rolled back. On failure the err buffer will be updated.
+ * rolled back.
*/
int ref_transaction_update(struct ref_transaction *transaction,
const char *refname,
@@ -250,11 +289,15 @@ int ref_transaction_update(struct ref_transaction
*transaction,
* that the reference should have after the update; it must not be the
* null SHA-1. It is verified that the reference does not exist
* already.
+ * Function returns 0 on success and non-zero on failure. A failure to create
+ * means that the transaction as a whole has failed and will need to be
+ * rolled back.
*/
-void ref_transaction_create(struct ref_transaction *transaction,
-
[PATCH v14 24/40] receive-pack.c: use a reference transaction for updating the refs
Wrap all the ref updates inside a transaction.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/receive-pack.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c323081..13f4a63 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,7 @@ static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
static const char *alt_shallow_file;
+static struct string_list error_strings = STRING_LIST_INIT_DUP;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -475,7 +476,6 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
const char *namespaced_name;
unsigned char *old_sha1 = cmd-old_sha1;
unsigned char *new_sha1 = cmd-new_sha1;
- struct ref_lock *lock;
/* only refs/... are allowed */
if (!starts_with(name, refs/) || check_refname_format(name + 5, 0)) {
@@ -576,19 +576,31 @@ static const char *update(struct command *cmd, struct
shallow_info *si)
return NULL; /* good */
}
else {
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction;
+
if (shallow_update si-shallow_ref[cmd-index]
update_shallow_ref(cmd, si))
return shallow error;
- lock = lock_any_ref_for_update(namespaced_name, old_sha1,
- 0, NULL);
- if (!lock) {
- rp_error(failed to lock %s, name);
- return failed to lock;
- }
- if (write_ref_sha1(lock, new_sha1, push)) {
- return failed to write; /* error() already called */
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, namespaced_name,
+ new_sha1, old_sha1, 0, 1, err) ||
+ ref_transaction_commit(transaction, push, err)) {
+
+ const char *str;
+ string_list_append(error_strings, err.buf);
+ str = error_strings.items[error_strings.nr - 1].string;
+ strbuf_release(err);
+
+ ref_transaction_free(transaction);
+ rp_error(%s, str);
+ return str;
}
+
+ ref_transaction_free(transaction);
+ strbuf_release(err);
return NULL; /* good */
}
}
@@ -1215,5 +1227,6 @@ int cmd_receive_pack(int argc, const char **argv, const
char *prefix)
packet_flush(1);
sha1_array_clear(shallow);
sha1_array_clear(ref);
+ string_list_clear(error_strings, 0);
return 0;
}
--
2.0.0.582.ge25c160
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
