Re: [msysGit] Re: [PATCH bc/connect-plink] t5601-clone: remove broken and pointless check for plink.exe
On Thu, Aug 13, 2015 at 10:37 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi kusma, On 2015-08-12 13:58, Erik Faye-Lund wrote: On Wed, Aug 12, 2015 at 1:07 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On 2015-08-11 22:51, Johannes Sixt wrote: Invoking plink requires special treatment, and we have support and even test cases for the commands 'plink' and 'tortoiseplink'. We also support .exe variants for these two and there is a test for 'plink.exe'. On Windows, however, where support for plink.exe would be relevant, the test case fails because it is not possible to execute a file with a .exe extension that is actually not a binary executable---it is a shell script in our test. We have to disable the test case on Windows. Oh how would I wish you were working on Git for Windows even *just* a bit *with* me. At least I would wish for a more specific description of the development environment, because it sure as hell is not anything anybody can download and install as easily as Git for Windows' SDK. FWIW Git for Windows has this patch (that I wanted to contribute in due time, what with being busy with all those tickets) to solve the problem mentioned in your patch in a different way: https://github.com/git-for-windows/git/commit/2fff4b54a0d4e5c5e2e4638c9b0739d3c1ff1e45 Yuck. On Windows, it's the extension of a file that dictates what kind of file it is (and if it's executable or not), not the contents. Careful. If you continue along those lines, interactive rebase, `git add -p` and all those wonderful scripts Git has will have to stop working. Because those scripts completely disagree with what you just said about Windows if you think about it: *none* of them has an extension. I know that you do not mean this, of course, but that is the argument you were making... ;-) You should know better than to straw-man like that. I was not arguing to break any current functionality, but to not move further away from Windows' semantics. But if I would have, there's nothing that would stop us from renaming those scrips to *.sh, and let the filename dictate how to execute them. Or provide batch-files to wrap them. If we get a shell script written with the .exe-prefix, it's considered as an invalid executable by the system. And if we get a shell script without any `.exe` suffix, it is still considered as an invalid executable by the system. Nope, it's considered an unknown file, not an executable at all. And even if we tack on an `.sh` suffix (which is *not* in line with the way Git works), it is *still* considered as an invalid executable by the system. That's not necessarily true; the Git for Windows installer (optionally, but on by default) registers /bin/sh as a file-handler for .sh files. Windows knows just fine how to execute them, unless the user opts out. But again, I was not arguing to patch git to not parse the shebang. -- To unsubscribe from this list: send the line 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 bc/connect-plink] t5601-clone: remove broken and pointless check for plink.exe
On Wed, Aug 12, 2015 at 1:07 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Johannes, On 2015-08-11 22:51, Johannes Sixt wrote: Invoking plink requires special treatment, and we have support and even test cases for the commands 'plink' and 'tortoiseplink'. We also support .exe variants for these two and there is a test for 'plink.exe'. On Windows, however, where support for plink.exe would be relevant, the test case fails because it is not possible to execute a file with a .exe extension that is actually not a binary executable---it is a shell script in our test. We have to disable the test case on Windows. Oh how would I wish you were working on Git for Windows even *just* a bit *with* me. At least I would wish for a more specific description of the development environment, because it sure as hell is not anything anybody can download and install as easily as Git for Windows' SDK. FWIW Git for Windows has this patch (that I wanted to contribute in due time, what with being busy with all those tickets) to solve the problem mentioned in your patch in a different way: https://github.com/git-for-windows/git/commit/2fff4b54a0d4e5c5e2e4638c9b0739d3c1ff1e45 Yuck. On Windows, it's the extension of a file that dictates what kind of file it is (and if it's executable or not), not the contents. If we get a shell script written with the .exe-prefix, it's considered as an invalid executable by the system. We should consider it the same way, otherwise we're on the path to user-experience schizophrenia. I'm not sure I consider this commit a step in the right direction. -- To unsubscribe from this list: send the line 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] compat/mingw: stubs for getpgid() and tcgetpgrp()
On Wed, Apr 15, 2015 at 8:29 PM, Johannes Sixt j...@kdbg.org wrote: Windows does not have process groups. It is, therefore, the simplest to pretend that each process is in its own process group. Windows does have some concept of process groups, but probably not quite what you want: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682083%28v=vs.85%29.aspx -- To unsubscribe from this list: send the line 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] systemd socket activation support
On Thu, Apr 2, 2015 at 8:09 AM, Shawn Landden sh...@churchofgit.com wrote: From: Shawn Landden shawnland...@gmail.com v1.1: actually test... Signed-off-by: Shawn Landden sh...@churchofgit.com --- daemon.c | 35 +++--- git-daemon.service | 7 +++ git-daemon.socket | 9 sd-daemon.c| 132 + sd-daemon.h| 91 5 files changed, 268 insertions(+), 6 deletions(-) create mode 100644 git-daemon.service create mode 100644 git-daemon.socket create mode 100644 sd-daemon.c create mode 100644 sd-daemon.h diff --git a/daemon.c b/daemon.c index 9ee2187..4677058 100644 --- a/daemon.c +++ b/daemon.c @@ -5,6 +5,8 @@ #include strbuf.h #include string-list.h +#include sd-daemon.c + You meant sd-daemon.h, no? -- To unsubscribe from this list: send the line 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: t5539 broken under Mac OS X
On Tue, Jan 27, 2015 at 3:51 AM, Junio C Hamano gits...@pobox.com wrote: Erik Faye-Lund kusmab...@gmail.com writes: On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: Exactly. I am happy to submit a patch, but I cannot think of any mechanisms besides: 1. Calling `id`, which I suspect is very not portable. 2. Writing a C program to check getuid(). That's portable for most Unixes. It looks like we already have a hacky wrapper on mingw that will always return 1. Is (2) too gross? Not overly gross compared to some existing test-*.c files, I would say. I wondered what 'perl -e 'print $' would say in mingw, and if that is portable enough, though. $ perl -e 'print $' 500 Thanks for a follow-up. Is id -u not useful over there? I ask because that is what is used in the version tentatively queued on 'pu' for NOT_ROOT prerequisite (the jk/sanity topic). It's pretty much the same thing: $ id -u 500 The SANITY prerequisite in that topic needs to be replaced with the one from Torsten that attempts to check what we want to know in a more direct way; i.e. after making a directory or a file read-only, does the filesystem really honours that, or lets us clobber? is what we need to know to skip some tests, and we should check that, instead of is / writable by us? or are we root?. $ test -w / echo yes yes $ mkdir foo chmod a= foo $ test -w w echo yes $ rm -r foo rm: directory `foo' is write protected; descend into it anyway? n $ rm -r foo /dev/null $ ls -la foo ls: foo: No such file or directory $ So, Windows does only kind-of respect read-only flags. Dunno if this tells you something useful, 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: t5539 broken under Mac OS X
On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: Exactly. I am happy to submit a patch, but I cannot think of any mechanisms besides: 1. Calling `id`, which I suspect is very not portable. 2. Writing a C program to check getuid(). That's portable for most Unixes. It looks like we already have a hacky wrapper on mingw that will always return 1. Is (2) too gross? Not overly gross compared to some existing test-*.c files, I would say. I wondered what 'perl -e 'print $' would say in mingw, and if that is portable enough, though. $ perl -e 'print $' 500 -- To unsubscribe from this list: send the line 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] wincred: fix get credential if username has @
Sorry for the extremely delayed reply, I had a bug in my mail-filters.
Hopefully fixed now.
On Wed, Nov 19, 2014 at 11:41 PM, Junio C Hamano gits...@pobox.com wrote:
Aleksey Vasenev margtu-f...@ya.ru writes:
To: git@vger.kernel.org
Cc: Junio C Hamano gits...@pobox.com, Aleksey Vasenev margtu-f...@ya.ru
Sorry, but I am hardly qualified to review this one, especially
without any log message that explains what breaks and how it breaks
with the current code, which may lead the reader to understand how
the updated code fixes the issue. Cc'ing me does not help us very
much.
$ git shortlog --no-merges -n contrib/credential/wincred/
gives me a few names who may be able to give us some inputs, so I'll
Cc them.
Thanks.
I noticed the breakage myself around the same time, and posted about it here:
https://groups.google.com/d/msg/msysgit/YVuCqmwwRyY/HULHj5OoE88J
Unfortunately, it stopped there.
Signed-off-by: Aleksey Vasenev margtu-f...@ya.ru
---
.../credential/wincred/git-credential-wincred.c| 25
+++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index a1d38f0..0061340 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -111,14 +111,23 @@ static void write_item(const char *what, LPCWSTR wbuf,
int wlen)
* Match an (optional) expected string and a delimiter in the target string,
* consuming the matched text by updating the target pointer.
*/
-static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
+
+static LPCWSTR wcsstr_last(LPCWSTR str, LPCWSTR find)
+{
+ LPCWSTR res = NULL, pos;
+ for (pos = wcsstr(str, find); pos; pos = wcsstr(pos + 1, find))
+ res = pos;
+ return res;
+}
+
Ugh, there's no wcsrstr? I guess this is a reasonable way to emulate it...
+static int match_part_with_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR
delim, int last)
{
LPCWSTR delim_pos, start = *ptarget;
int len;
/* find start of delimiter (or end-of-string if delim is empty) */
if (*delim)
- delim_pos = wcsstr(start, delim);
+ delim_pos = last ? wcsstr_last(start, delim) : wcsstr(start,
delim);
else
delim_pos = start + wcslen(start);
@@ -138,6 +147,16 @@ static int match_part(LPCWSTR *ptarget, LPCWSTR want,
LPCWSTR delim)
return !want || (!wcsncmp(want, start, len) !want[len]);
}
+static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
+{
+ return match_part_with_last(ptarget, want, delim, 0);
+}
+
+static int match_part_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
+{
+ return match_part_with_last(ptarget, want, delim, 1);
+}
+
static int match_cred(const CREDENTIALW *cred)
{
LPCWSTR target = cred-TargetName;
@@ -146,7 +165,7 @@ static int match_cred(const CREDENTIALW *cred)
return match_part(target, Lgit, L:)
match_part(target, protocol, L://)
- match_part(target, wusername, L@)
+ match_part_last(target, wusername, L@)
match_part(target, host, L/)
match_part(target, path, L);
}
Looks reasonable enough to me.
Acked-by: Erik Faye-Lund kusmab...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH 01/14] MINGW: compat/mingw.h: do not attempt to redefine lseek on mingw-w64
On Wed, Oct 8, 2014 at 8:00 PM, Marat Radchenko ma...@slonopotamus.org wrote: Unlike MinGW, MinGW-W64 has lseek already properly defined in io.h. Signed-off-by: Marat Radchenko ma...@slonopotamus.org Acked-by: Eric Faye-Lund kusmab...@gmail.com I spell my name with a K, Erik Faye-Lund. -- To unsubscribe from this list: send the line 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 2/2] wincred: improve compatibility with windows versions
On Thu, Jan 10, 2013 at 1:10 PM, Karsten Blees karsten.bl...@gmail.com wrote:
static int match_cred(const CREDENTIALW *cred)
{
- return (!wusername || !wcscmp(wusername, cred-UserName))
- match_attr(cred, Lgit_protocol, protocol)
- match_attr(cred, Lgit_host, host)
- match_attr(cred, Lgit_path, path);
+ LPCWSTR target = cred-TargetName;
+ if (wusername wcscmp(wusername, cred-UserName))
+ return 0;
+
+ return match_part(target, Lgit, L:)
+ match_part(target, protocol, L://)
+ match_part(target, wusername, L@)
This seems to have broken credentials with '@' in the username, which
isn't all that unusual these days... :(
--
To unsubscribe from this list: send the line 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: Location of git config on Windows
On Mon, Aug 18, 2014 at 5:14 PM, Daniel Corbe co...@corbe.net wrote: Karsten Blees karsten.bl...@gmail.com writes: Am 18.08.2014 00:01, schrieb Erik Faye-Lund: On Sun, Aug 17, 2014 at 10:18 PM, Daniel Corbe co...@corbe.net wrote: I installed git on my Windows machine while it was connected to my corporate network. It picked up on that fact and used a mapped drive to store its configuration file. As a result, I cannot currently use git when disconnected from my network. It throws the following error message: fatal: unable to access 'Z:\/.config/git/config': Invalid argument Obviously this value is stored in the registry somewhere because I made an attempt to uninstall and reinstall git with the same results. Can someone give me some guidance here? Git looks for the per-user configuration in $HOME/.gitconfig, and if $HOME is not set, it falls back to $HOMEDIR/$HOMEPATH/.gitconfig. My guess would be some of these environment variables are incorrectly set on your system. To be precise, git checks if %HOME% is set _and_ the directory exists before falling back to %HOMEDRIVE%%HOMEPATH%. If %HOMEDRIVE%%HOMEPATH% isn't set or the directory doesn't exist either, it falls back to %USERPROFILE%, which is always local (C:/Users/yourname), even if disconnected from the network (at least that's how its supposed to be). Awesome! Thanks for the advice. %HOMEDRIVE% and %HOMEPATH% are indeed set by my system and point to an (often disconnected) network drive. I manually forced %HOME% to %USERPROFILE% and it works like a charm now. I would argue that on Windows %USERPROFILE% should be checked first (or at least after %HOME%). Why? Then people won't be able to have their config files on network-shares, no? I think a somewhat better approach would be to resolve the home directory lazily, unless %HOME% is set. That way we can check that %HOMEDRIVE%%HOMEPATH% actually exists as it's being accessed. Or you could just restart your shell when you disconnect... -- To unsubscribe from this list: send the line 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: Location of git config on Windows
On Mon, Aug 18, 2014 at 5:40 PM, Daniel Corbe co...@corbe.net wrote: Erik Faye-Lund kusmab...@gmail.com writes: Or you could just restart your shell when you disconnect... Well I'm not that daft. I tried that and if it had resolved my problem I wouldn't have posted. Hm, but isn't that what Karsten explains in his last paragraph? What shell are you running msys or cmd? -- To unsubscribe from this list: send the line 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: Location of git config on Windows
On Mon, Aug 18, 2014 at 5:47 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Mon, Aug 18, 2014 at 5:40 PM, Daniel Corbe co...@corbe.net wrote: Erik Faye-Lund kusmab...@gmail.com writes: Or you could just restart your shell when you disconnect... Well I'm not that daft. I tried that and if it had resolved my problem I wouldn't have posted. Hm, but isn't that what Karsten explains in his last paragraph? What shell are you running msys or cmd? Our /etc/profile does this: https://github.com/msysgit/msysgit/blob/master/etc/profile#L38 ...however, our git-wrapper only does this: https://github.com/msysgit/msysgit/blob/master/src/git-wrapper/git-wrapper.c#L71 So yeah, we don't seem to actually check if %HOMEDRIVE%%HOMEPATH% exists. Perhaps fixing this is the right thing to do then? Since the git-wrapper is run for *every* invokation of git, you wouldn't even have to restart the shell in this case. -- To unsubscribe from this list: send the line 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: Location of git config on Windows
On Mon, Aug 18, 2014 at 7:05 PM, Daniel Corbe co...@corbe.net wrote: Erik Faye-Lund kusmab...@gmail.com writes: On Mon, Aug 18, 2014 at 5:47 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Mon, Aug 18, 2014 at 5:40 PM, Daniel Corbe co...@corbe.net wrote: Erik Faye-Lund kusmab...@gmail.com writes: Or you could just restart your shell when you disconnect... Well I'm not that daft. I tried that and if it had resolved my problem I wouldn't have posted. Hm, but isn't that what Karsten explains in his last paragraph? What shell are you running msys or cmd? Our /etc/profile does this: https://github.com/msysgit/msysgit/blob/master/etc/profile#L38 ...however, our git-wrapper only does this: https://github.com/msysgit/msysgit/blob/master/src/git-wrapper/git-wrapper.c#L71 So yeah, we don't seem to actually check if %HOMEDRIVE%%HOMEPATH% exists. Perhaps fixing this is the right thing to do then? Since the git-wrapper is run for *every* invokation of git, you wouldn't even have to restart the shell in this case. But again, restarting the shell doesn't fix the problem. Not for cmd, no. But for Git Bash, it should. -- To unsubscribe from this list: send the line 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: Location of git config on Windows
On Sun, Aug 17, 2014 at 10:18 PM, Daniel Corbe co...@corbe.net wrote: I installed git on my Windows machine while it was connected to my corporate network. It picked up on that fact and used a mapped drive to store its configuration file. As a result, I cannot currently use git when disconnected from my network. It throws the following error message: fatal: unable to access 'Z:\/.config/git/config': Invalid argument Obviously this value is stored in the registry somewhere because I made an attempt to uninstall and reinstall git with the same results. Can someone give me some guidance here? Git looks for the per-user configuration in $HOME/.gitconfig, and if $HOME is not set, it falls back to $HOMEDIR/$HOMEPATH/.gitconfig. My guess would be some of these environment variables are incorrectly set on your system. -- To unsubscribe from this list: send the line 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: Undefined reference to __builtin_ctzll
On Wed, Aug 13, 2014 at 10:36 AM, Радослав Йовчев radoslav...@gmail.com wrote:
Dear GIT community,
I found myself in situation where I had to install GIT on Debian 3.1
sarge. It comes with GCC 3.3.5. I tried to built from source but the
libgcc was not providing the ctzll function, thus I decided to put an
implementation.
I do not know how to post and do a nice patch (and whether somebody
will care), but I guess, for reference I can post my solution. Just
appended in compat/strlcpy.c the following:
int __builtin_ctzll (long long x)
{
int i;
for (i = 0; i 8 * sizeof (long long); ++i)
if (x ((long long) 1 i))
break;
return i;
}
I guess that some ifdef macro can be used to detect compiler version
or missing __builtin_ctzll.
It seems __builtin_ctzll is only available in GCC 3.4.0 and beyond, so
I think a better fix is something like this:
diff --git a/ewah/ewok.h b/ewah/ewok.h
index 43adeb5..2700fa3 100644
--- a/ewah/ewok.h
+++ b/ewah/ewok.h
@@ -47,7 +47,7 @@ static inline uint32_t ewah_bit_popcount64(uint64_t x)
return (x * 0x0101010101010101ULL) 56;
}
-#ifdef __GNUC__
+#if (__GNUC__ == 3 __GNUC_MINOR__ = 4) || __GNUC__ 3
#define ewah_bit_ctz64(x) __builtin_ctzll(x)
#else
static inline int ewah_bit_ctz64(uint64_t x)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH 4/6] t4210: skip command-line encoding tests on mingw
On Thu, Jul 17, 2014 at 5:37 PM, Stepan Kasal ka...@ucw.cz wrote: From: Pat Thoyts pattho...@users.sourceforge.net On Windows the application command line is provided as unicode and in mingw-git we convert that to utf-8. So these tests that require a iso-8859-1 input are being subverted by the encoding transformations we perform and should be skipped. Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net Signed-off-by: Stepan Kasal ka...@ucw.cz --- t/t4210-log-i18n.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4210-log-i18n.sh b/t/t4210-log-i18n.sh index 52a7472..9110404 100755 --- a/t/t4210-log-i18n.sh +++ b/t/t4210-log-i18n.sh @@ -34,7 +34,7 @@ test_expect_success 'log --grep searches in log output encoding (utf8)' ' test_cmp expect actual ' -test_expect_success 'log --grep searches in log output encoding (latin1)' ' +test_expect_success NOT_MINGW 'log --grep searches in log output encoding (latin1)' ' cat expect -\EOF latin1 utf8 @@ -43,7 +43,7 @@ test_expect_success 'log --grep searches in log output encoding (latin1)' ' test_cmp expect actual ' -test_expect_success 'log --grep does not find non-reencoded values (utf8)' ' +test_expect_success NOT_MINGW 'log --grep does not find non-reencoded values (utf8)' ' Perhaps these checks would be more readable a few years in the future, if we make a separate capability along the lines of NON_UNICODE_LOCALE? -- To unsubscribe from this list: send the line 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: No fchmod() under msygit - Was: Re: [PATCH 00/14] Add submodule test harness
On Wed, Jul 9, 2014 at 10:00 PM, Eric Wong normalper...@yhbt.net wrote: Torsten Bögershausen tbo...@web.de wrote: (And why is it 0 and not 0777) This is to preserve the uncommon sticky/sgid/suid bits. Probably not needed, but better to keep as much intact as possible. Can we avoid the fchmod() all together ? For single-user systems, sure. For multi-user systems with git-imap-send users and passwords in $GIT_CONFIG, I suggest not. You're saying this as if Windows is a single-user system. It's not, but it uses ACLs rather than POSIX permissions to manage file-system permissions. So far we've opted to ignore ACLs in Git for Windows, 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 7/7] determine_author_info: stop leaking name/email
On Mon, Jun 23, 2014 at 11:28 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Wed, Jun 18, 2014 at 4:36 PM, Jeff King p...@peff.net wrote:
When we get the author name and email either from an
existing commit or from the --author option, we create a
copy of the strings. We cannot just free() these copies,
since the same pointers may also be pointing to getenv()
storage which we do not own.
Instead, let's treat these the same way as we do the date
buffer: keep a strbuf to be released, and point the bare
pointers at the strbuf.
Signed-off-by: Jeff King p...@peff.net
---
builtin/commit.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 62abee0..72beb7f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -546,16 +546,20 @@ static void strbuf_add_pair(struct strbuf *buf, const
struct pointer_pair *p)
strbuf_add(buf, p-begin, p-end - p-begin);
}
-static char *xmemdupz_pair(const struct pointer_pair *p)
+static char *set_pair(struct strbuf *buf, struct pointer_pair *p)
{
- return xmemdupz(p-begin, p-end - p-begin);
+ strbuf_reset(buf);
+ strbuf_add_pair(buf, p);
+ return buf-buf;
}
static void determine_author_info(struct strbuf *author_ident)
{
char *name, *email, *date;
struct ident_split author;
- struct strbuf date_buf = STRBUF_INIT;
+ struct strbuf name_buf = STRBUF_INIT,
+ mail_buf = STRBUF_INIT,
nit: The associated 'char *' variable is named email, so perhaps
s/mail_buf/email_buf/g
+ date_buf = STRBUF_INIT;
name = getenv(GIT_AUTHOR_NAME);
email = getenv(GIT_AUTHOR_EMAIL);
@@ -572,8 +576,8 @@ static void determine_author_info(struct strbuf
*author_ident)
if (split_ident_line(ident, a, len) 0)
die(_(commit '%s' has malformed author line),
author_message);
- name = xmemdupz_pair(ident.name);
- email = xmemdupz_pair(ident.mail);
+ name = set_pair(name_buf, ident.name);
+ email = set_pair(mail_buf, ident.mail);
if (ident.date.begin) {
strbuf_reset(date_buf);
strbuf_addch(date_buf, '@');
@@ -589,8 +593,8 @@ static void determine_author_info(struct strbuf
*author_ident)
if (split_ident_line(ident, force_author,
strlen(force_author)) 0)
die(_(malformed --author parameter));
- name = xmemdupz_pair(ident.name);
- email = xmemdupz_pair(ident.mail);
+ name = set_pair(name_buf, ident.name);
+ email = set_pair(mail_buf, ident.mail);
Does the code become too convoluted with these changes? You're now
maintaining three 'char *' variables in parallel with three strbuf
variables. Is it possible to drop the 'char *' variables and just pass
the .buf member of the strbufs to fmt_ident()?
Alternately, you also could solve the leaks by having an envdup() helper:
static char *envdup(const char *s)
{
const char *v = getenv(s);
return v ? xstrdup(v) : NULL;
}
...
name = envdup(GIT_AUTHOR_NAME);
email = envdup(GIT_AUTHOR_EMAIL);
...
And then just free() 'name' and 'email' normally.
This approach has the added benefit of fixing the case where getenv
uses a static buffer, like POSIX allows.
--
To unsubscribe from this list: send the line 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 1/3] add strnncmp() function
On Tue, Jun 17, 2014 at 9:34 AM, Jeremiah Mahler jmmah...@gmail.com wrote:
Add a strnncmp() function which behaves like strncmp() except it takes
the length of both strings instead of just one. It behaves the same as
strncmp() up to the minimum common length between the strings. When the
strings are identical up to this minimum common length, the length
difference is returned.
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
strbuf.c | 9 +
strbuf.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index ac62982..4eb7954 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -600,3 +600,12 @@ char *xstrdup_tolower(const char *string)
result[i] = '\0';
return result;
}
+
+int strnncmp(const char *a, int len_a, const char *b, int len_b)
+{
+ int min_len = (len_a len_b) ? len_a : len_b;
+ int cmp = strncmp(a, b, min_len);
+ if (cmp)
+ return cmp;
+ return (len_a - len_b);
+}
Using a name that sounds like it's from the stdlib makes me cringe a
little bit. Names that start with str reserved for stdlib[1][2], but
we already ignore this for strbuf (and perhaps some other functions).
However, in this case it doesn't seem *that* unlikely that we might
collide with some stdlib-extensions.
[1]:
http://pubs.opengroup.org/onlinepubs/007904975/functions/xsh_chap02_02.html#tag_02_02_02
[2]: http://www.gnu.org/software/libc/manual/html_node/Reserved-Names.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: [msysGit] Multi-line commit message changed to a single one
On Fri, Jun 13, 2014 at 1:44 PM, jese@gmail.com wrote: Hi, Here is a simple script to show my problem. I don't know if it's a wrong operation (from me), a wrong configuration/parameter, or either a bug, but I don't get what I expected. From a git repo (#1), I created a commit message containing two lines. Then I created a patch (git format-patch) and copy/paste it to another git repo (#2). When I import this patch (git am), the commit message is modified and both lines are merged to a single one. Screenshot providen (repo1 : two separated lines, repo2 : a single line) with .sh script to reproduce the whole test-case. If someone could tell me what is wrong... Thanks in advance This isn't unique to Git for Windows, I just tested on Linux and the same occurs. What seems to happen is that git format-patch puts both lines in the subject of the e-mail, as you can find described here: https://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html By default, the subject of a single patch is [PATCH] followed by the concatenation of lines from the commit message up to the first blank line (see the DISCUSSION section of git-commit(1)). So, this is entirely intentional, it seems. -- To unsubscribe from this list: send the line 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] Add a Windows-specific fallback to getenv(HOME);
On Thu, Jun 5, 2014 at 11:40 AM, Karsten Blees karsten.bl...@gmail.com wrote: Am 05.06.2014 10:03, schrieb Stepan Kasal: From: Johannes Schindelin johannes.schinde...@gmx.de Date: Wed, 2 Jun 2010 00:41:33 +0200 If HOME is not set, use $HOMEDRIVE$HOMEPATH Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hello Karsten, thanks for your explanation. There are more things to be done, but I hope you can ack this patch as a step forward. No, not really. Its sure better than introducing a special get_home_directory(), but it still increases the diff between upstream and msysgit rather than reducing it. The main critique points still remain: * $HOME is usually set up correctly before calling git, so this is essentially dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do this correctly) What about when tools like TortoiseGit and Git Extensions call git? We're not guaranteed that they did the $HOME-dance, are we? * even if $HOME was empty, git should setenv(HOME) so that child processes can benefit from it (similar to TMPDIR and TERM in current msysgit's mingw_startup()). Not setting $HOME because it may hypothetically break child processes is a very weak argument, as we always did set $HOME in etc/profile (since the initial version back in 2007). * no fallback to $USERPROFILE doesn't work with diconnected home share If you really have time to spare, I suggest you focus on getting the Unicode patches upstream so that we can progress from there (e.g. move $HOME setup to mingw_startup() so that we can get rid of redundant logic in etc/profile, git-wrapper, git-bash.bat, git-cmd.bat etc.). Perhaps we can patch up the upstream to better match Git for Windows without upstreaming the Unicode patches? Don't get me wrong; I think upstreaming them is a good idea, but in case time is lacking... -- To unsubscribe from this list: send the line 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] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote:
On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote:
@@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
void home_config_paths(char **global, char **xdg, char *file)
{
char *xdg_home = getenv(XDG_CONFIG_HOME);
- char *home = getenv(HOME);
+ const char *home = get_home_directory();
char *to_free = NULL;
if (!home) {
Just checking. Instead of replace the call sites, can we check and
setenv(HOME) if it's missing instead? MinGW port already replaces
main(). Extra initialization should not be a problem. I feel
getenv(HOME) a tiny bit more familiar than get_home_directory(),
but that's really weak argument as the number of call sites has not
increased in 4 years.
Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
/etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
need one more place?
It seems some of these could be dropped...
--
To unsubscribe from this list: send the line 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] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
Hi Erik,
On Wed, 4 Jun 2014, Erik Faye-Lund wrote:
On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote:
On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote:
@@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...)
void home_config_paths(char **global, char **xdg, char *file)
{
char *xdg_home = getenv(XDG_CONFIG_HOME);
- char *home = getenv(HOME);
+ const char *home = get_home_directory();
char *to_free = NULL;
if (!home) {
Just checking. Instead of replace the call sites, can we check and
setenv(HOME) if it's missing instead? MinGW port already replaces
main(). Extra initialization should not be a problem. I feel
getenv(HOME) a tiny bit more familiar than get_home_directory(),
but that's really weak argument as the number of call sites has not
increased in 4 years.
Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in
/etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really
need one more place?
It seems some of these could be dropped...
No. Git is not always called through Bash or the git-wrapper,
unfortunately.
I'm aware of that. But you said in a previous e-mail that e.g putty
got confused when we set HOME. How is this a problem for git.exe, but
not when we set it in the shell?
--
To unsubscribe from this list: send the line 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] replace signal() with sigaction()
On Wed, May 28, 2014 at 10:11 AM, Chris Packham judge.pack...@gmail.com wrote:
On 28/05/14 19:40, Johannes Sixt wrote:
Am 5/28/2014 8:14, schrieb Jeremiah Mahler:
From signal(2)
The behavior of signal() varies across UNIX versions, and has also var‐
ied historically across different versions of Linux. Avoid its use:
use sigaction(2) instead. See Portability below.
This patch set replaces calls to signal() with sigaction() in all files
except sigchain.c. sigchain.c is a bit more complicated than the others
and will be done in a separate patch.
In compat/mingw.c we have:
int sigaction(int sig, struct sigaction *in, struct sigaction *out)
{
if (sig != SIGALRM)
return errno = EINVAL,
error(sigaction only implemented for SIGALRM);
if (out != NULL)
return errno = EINVAL,
error(sigaction: param 3 != NULL not implemented);
timer_fn = in-sa_handler;
return 0;
}
Notice only implemented for SIGALRM. Are adding the missing signals
somewhere (here or in a later patch)?
* note: not a windows/mingw programmer *
Will the ones setting SIG_IGN be OK? Presumably we won't get these
signals on windows anyway so we're already getting what we want.
We'll still emit an useless error unless someone cooks up a fix.
--
To unsubscribe from this list: send the line 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] progress.c: replace signal() with sigaction()
On Wed, May 28, 2014 at 10:19 AM, David Kastrup d...@gnu.org wrote:
Chris Packham judge.pack...@gmail.com writes:
On 28/05/14 18:14, Jeremiah Mahler wrote:
From signal(2)
The behavior of signal() varies across UNIX versions, and has also var‐
ied historically across different versions of Linux. Avoid its use:
use sigaction(2) instead. See Portability below.
Minor nit. The last sentence applies to the man page you're quoting and
doesn't really make sense when viewed in the context of this commit
message. Same applies to other patches in this series.
Replaced signal() with sigaction() in progress.c
Signed-off-by: Jeremiah Mahler jmmah...@gmail.com
---
progress.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/progress.c b/progress.c
index 261314e..24df263 100644
--- a/progress.c
+++ b/progress.c
@@ -66,8 +66,12 @@ static void set_progress_signal(void)
static void clear_progress_signal(void)
{
struct itimerval v = {{0,},};
+struct sigaction sa;
+
+memset(sa, 0, sizeof(sa));
+sa.sa_handler = SIG_IGN;
A C99 initialiser here would save the call to memset. Unfortunately
Documentation/CodingGuidelines is fairly clear on not using C99
initialisers, given the fact we're now at git 2.0 maybe it's time to
revisit this policy?
If I look at the initialization of v in the context immediately above
the new code, it would appear that somebody already revisited this
policy.
Huh, the initialization of v doesn't use C99-features...?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] clean: add a flag to back up cleaned files
The combination of git clean and fat fingers can some times cause
data-loss, which can be frustrating.
So let's add a flag that imports the files to be deleted into the
object-database, in a way similar to what git-stash does. Maintain
a reflog of the previously backed up clean-runs.
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
I've had a similar patch laying around for quite a while, but since
f538a91 (git-clean: Display more accurate delete messages), this
patch is a lot less nasty than before. So here you go, perhaps
someone else has fat fingers and hate to lose work?
Documentation/config.txt| 4 ++
Documentation/git-clean.txt | 4 ++
builtin/clean.c | 111 +++-
t/t7300-clean.sh| 12 +
4 files changed, 130 insertions(+), 1 deletion(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..d58fe31 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -797,6 +797,10 @@ clean.requireForce::
A boolean to make git-clean do nothing unless given -f,
-i or -n. Defaults to true.
+clean.backup::
+ A boolean to make git-clean back up files before they are
+ deleted. Defaults to false.
+
color.branch::
A boolean to enable/disable color in the output of
linkgit:git-branch[1]. May be set to `always`,
diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 8997922..bc9d703 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -41,6 +41,10 @@ OPTIONS
--interactive::
Show what would be done and clean files interactively. See
``Interactive mode'' for details.
+-b::
+--backup::
+ Back up files to a reflog before deleting them. The tree of
+ backed up files are stored in the reflog for refs/clean-backup.
-n::
--dry-run::
diff --git a/builtin/clean.c b/builtin/clean.c
index 9a91515..96fb4b2 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -16,9 +16,12 @@
#include column.h
#include color.h
#include pathspec.h
+#include tree-walk.h
+#include unpack-trees.h
+#include cache-tree.h
static int force = -1; /* unset */
-static int interactive;
+static int interactive, backup;
static struct string_list del_list = STRING_LIST_INIT_DUP;
static unsigned int colopts;
@@ -120,6 +123,11 @@ static int git_clean_config(const char *var, const char
*value, void *cb)
return 0;
}
+ if (!strcmp(var, clean.backup)) {
+ backup = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, clean.requireforce)) {
force = !git_config_bool(var, value);
return 0;
@@ -148,6 +156,93 @@ static int exclude_cb(const struct option *opt, const char
*arg, int unset)
return 0;
}
+static int backed_up_anything;
+
+static void backup_file(const char *path, struct stat *st)
+{
+ if (add_to_cache(path, st, 0))
+ die(_(backing up '%s' failed), path);
+ backed_up_anything = 1;
+}
+
+static struct commit_list *parents;
+
+static void prepare_backup(void)
+{
+ struct unpack_trees_options opts;
+ unsigned char sha1[20];
+ struct tree *tree;
+ struct commit *parent;
+ struct tree_desc t;
+
+ if (get_sha1(HEAD, sha1))
+ die(_(You do not have the initial commit yet));
+
+ /* prepare parent-list */
+ parent = lookup_commit_or_die(sha1, HEAD);
+ commit_list_insert(parent, parents);
+
+ /* load HEAD into the index */
+
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_(Failed to unpack tree object %s), sha1);
+
+ parse_tree(tree);
+ init_tree_desc(t, tree-buffer, tree-size);
+
+ memset(opts, 0, sizeof(opts));
+ opts.head_idx = -1;
+ opts.src_index = the_index;
+ opts.dst_index = the_index;
+ opts.index_only = 1;
+
+ if (unpack_trees(1, t, opts)) {
+ /* We've already reported the error, finish dying */
+ exit(128);
+ }
+}
+
+static void finish_backup(void)
+{
+ const char *ref = refs/clean-backup;
+ unsigned char commit_sha1[20];
+ struct strbuf msg = STRBUF_INIT;
+ char logfile[PATH_MAX];
+ struct stat st;
+
+ if (!backed_up_anything)
+ return;
+
+ if (!active_cache_tree)
+ active_cache_tree = cache_tree();
+
+ if (!cache_tree_fully_valid(active_cache_tree)) {
+ if (cache_tree_update(active_cache_tree,
+ (const struct cache_entry * const *)active_cache,
+ active_nr, 0) 0)
+ die(failed to update cache);
+ }
+
+ strbuf_addstr(msg, Automatically committed by git-clean);
+
+ /* create a reflog, if there isn't one */
+ git_snpath(logfile, sizeof(logfile), logs/%s, ref);
+ if (stat(logfile, st
Re: [PATCH/RFC] clean: add a flag to back up cleaned files
On Tue, May 27, 2014 at 6:37 PM, Jeff King p...@peff.net wrote:
On Tue, May 27, 2014 at 04:17:34PM +0200, Erik Faye-Lund wrote:
The combination of git clean and fat fingers can some times cause
data-loss, which can be frustrating.
So let's add a flag that imports the files to be deleted into the
object-database, in a way similar to what git-stash does. Maintain
a reflog of the previously backed up clean-runs.
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
I've had a similar patch laying around for quite a while, but since
f538a91 (git-clean: Display more accurate delete messages), this
patch is a lot less nasty than before. So here you go, perhaps
someone else has fat fingers and hate to lose work?
I've definitely considered doing something like this before (and for
git reset --hard). My biggest concern would be poor performance in
some cases. But since it's optional, and one can presumably override it
with --no-backup for a specific large cleanup, it would not hurt anybody
who does not want to play with it.
I also made it opt-in rather than opt-out by default. Perhaps it
shouldn't be, though - dunno.
+ /* load HEAD into the index */
+
+ tree = parse_tree_indirect(sha1);
+ if (!tree)
+ die(_(Failed to unpack tree object %s), sha1);
+
+ parse_tree(tree);
+ init_tree_desc(t, tree-buffer, tree-size);
+
+ memset(opts, 0, sizeof(opts));
+ opts.head_idx = -1;
+ opts.src_index = the_index;
+ opts.dst_index = the_index;
+ opts.index_only = 1;
+
+ if (unpack_trees(1, t, opts)) {
+ /* We've already reported the error, finish dying */
+ exit(128);
+ }
This bit of the code surprised me. I guess you are trying to re-create
the index state of the HEAD so that the commit you build on top of it
contains _only_ the untracked files as changes, and not whatever
intermediate index state you had. That makes some sense to me, as clean
is never touching the index state.
TBH, I didn't really think this stuff through, I basically just hacked
on this until I got it to save away superfluous files when the index
matched HEAD. So this part is more accidental than designed. I'm not
very familiar with the index-maniuplation code in Git either.
I *think* the right thing to do would be to save the tree of HEAD
*plus* those deleted files in this case. That way it only records the
destruction itself. This does *not* seem to be what currently happens.
If I have a change staged, that staged change also gets included in
the commit. That's not ideal, I think.
Though taking a step back for a moment, I'd like to think about doing
something similar for reset --hard, which is the other destructive
operation in git[1]. In that case, I think saving the index state is also
useful, because it is being reset, too (and while those blobs are
recoverable, the exact state is sometimes useful).
I agree. I guess that should essentially do a full git-stash.
If we were to do that, should it be a separate ref? Or should there be a
single backup ref for such oops, undo that operations? If the latter,
what should that ref look like? I think it would look something like
refs/stash, with the index and the working tree state stored as separate
commits (even though in the clean case, the index state is not likely
to be that interesting, it is cheap to store and makes the recovery
tools uniform to use).
Hmm, perhaps. I do like the concept of a git undo of sorts, but
perhaps that'll raise the expectations even further? Or maybe raising
them a good thing, so people add missing features? :)
And if we are going to store it like that, should we just be using git
stash save --keep-index --include-untracked? I think we would just need
to teach it a --no-reset option to leave the tracked files as-is.
Hm, interesting. But it does leave me with a bit of a bad feeling; git
stash is currently a shell-script, and I want us to move *away* from
depending on those rather than towards... Or perhaps I just convinced
myself to port git-stash to C? I guess the full script won't be
needed, only the heavy lifting...
I realize that I went a few steps down the if... chain there to get to
should we just be using stash?. But it would also make the code here
dirt-simple.
Simplicity is good, and it's always good to hear some different ideas
on how to reach a goal.
[1] Actually, reset --hard may be more of an education issue, as
simply running git stash has the same effect, but keeps a backup.
It's just that reset --hard is advertised as the solution to many
problems.
+ if (!active_cache_tree)
+ active_cache_tree = cache_tree();
+
+ if (!cache_tree_fully_valid(active_cache_tree)) {
+ if (cache_tree_update(active_cache_tree,
+ (const struct cache_entry * const *)active_cache,
+ active_nr, 0) 0)
+ die(failed to update cache
Re: [msysGit] Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k
On Tue, May 20, 2014 at 10:46 AM, Thomas Braun
thomas.br...@byte-physics.de wrote:
Am 19.05.2014 22:29, schrieb Erik Faye-Lund:
[...]
Would we need to wrap both ends, shouldn't wrapping only reading be
good enough to prevent deadlocking?
compat/poll/poll.c already contains a function called IsSocketHandle
that is able to tell if a HANDLE points to a socket or not.
This very quick attempt did not work out :(
diff --git a/compat/mingw.c b/compat/mingw.c
index 0335958..ec1d81f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)
return fd;
}
+#define is_console_handle(h) (((long) (h) 3) == 3)
+
+static int is_socket_handle(HANDLE h)
+{
+WSANETWORKEVENTS ev;
+
+if (is_console_handle(h))
+return 0;
+
+/*
+ * Under Wine, it seems that getsockopt returns 0 for pipes too.
+ * WSAEnumNetworkEvents instead distinguishes the two correctly.
+ */
+ev.lNetworkEvents = 0xDEADBEEF;
+WSAEnumNetworkEvents((SOCKET) h, NULL, ev);
+return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
+#undef read
+ssize_t mingw_read(int fd, void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return read(fd, buf, count);
+
+ret = recv((SOCKET)fh, buf, count, 0);
+if (ret 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return write(fd, buf, count);
+
+return send((SOCKET)fh, buf, count, 0);
+if (ret 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+
static BOOL WINAPI ctrl_ignore(DWORD type)
{
return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 08b83fe..1690098 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
+ssize_t mingw_read(int fd, void *buf, size_t count);
+#define read mingw_read
+
+ssize_t mingw_write(int fd, const void *buf, size_t count);
+#define write mingw_write
+
int mingw_fgetc(FILE *stream);
#define fgetc mingw_fgetc
According to [1] you also have to pass WSA_FLAG_OVERLAPPED to avoid the
deadlock.
With that change I don't get a hang anymore but a read error with
errno 10054 aka WSAECONNRESET.
[1]: https://groups.google.com/forum/#!msg/msysgit/at8D7J-h7mw/PM9w-d41cDYJ
Yeah, sorry, I noticed this right after sending and tested with that
as well. My results were the same :/
--
To unsubscribe from this list: send the line 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 v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 5/6/2014 2:17, schrieb Eric Wong:
Users may already store sensitive data such as imap.pass in
..git/config; making the file world-readable when git config
is called to edit means their password would be compromised
on a shared system.
[v2: updated for section renames, as noted by Junio]
Signed-off-by: Eric Wong normalper...@yhbt.net
---
config.c | 16
t/t1300-repo-config.sh | 10 ++
2 files changed, 26 insertions(+)
diff --git a/config.c b/config.c
index a30cb5c..c227aa8 100644
--- a/config.c
+++ b/config.c
@@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
+ if (fchmod(fd, st.st_mode 0) 0) {
+ error(fchmod on %s failed: %s,
+ lock-filename, strerror(errno));
+ ret = CONFIG_NO_WRITE;
+ goto out_free;
+ }
This introduces a severe failure in the Windows port because we do not
implement fchmod. Existing fchmod invocations do not check the return
value, and they are only interested in removing the write bits, and we
generally don't care a lot if files remain writable.
I'm not proficient enough to add any ACL fiddling to fchmod that would be
required by the above change, whose purpose is to be strict about
permissions. Nor am I interested (who the heck is sharing a Windows box
anyway? ;-)
Therefore, here's just a work-around patch to keep things going on
Windows. Any opinions from the Windows corner?
Since we use MSVCRT's chmod implementation directly which only checks
for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
using Get/SetFileInformationByHandle instead? That takes us in a
better direction, IMO. Adding full ACL support seems like a bigger
project.
If there's no objection, I'll sketch up a patch for that...
--
To unsubscribe from this list: send the line 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 v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:44 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt j.s...@viscovery.net wrote:
Am 5/6/2014 2:17, schrieb Eric Wong:
Users may already store sensitive data such as imap.pass in
..git/config; making the file world-readable when git config
is called to edit means their password would be compromised
on a shared system.
[v2: updated for section renames, as noted by Junio]
Signed-off-by: Eric Wong normalper...@yhbt.net
---
config.c | 16
t/t1300-repo-config.sh | 10 ++
2 files changed, 26 insertions(+)
diff --git a/config.c b/config.c
index a30cb5c..c227aa8 100644
--- a/config.c
+++ b/config.c
@@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
+ if (fchmod(fd, st.st_mode 0) 0) {
+ error(fchmod on %s failed: %s,
+ lock-filename, strerror(errno));
+ ret = CONFIG_NO_WRITE;
+ goto out_free;
+ }
This introduces a severe failure in the Windows port because we do not
implement fchmod. Existing fchmod invocations do not check the return
value, and they are only interested in removing the write bits, and we
generally don't care a lot if files remain writable.
I'm not proficient enough to add any ACL fiddling to fchmod that would be
required by the above change, whose purpose is to be strict about
permissions. Nor am I interested (who the heck is sharing a Windows box
anyway? ;-)
Therefore, here's just a work-around patch to keep things going on
Windows. Any opinions from the Windows corner?
Since we use MSVCRT's chmod implementation directly which only checks
for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
using Get/SetFileInformationByHandle instead? That takes us in a
better direction, IMO. Adding full ACL support seems like a bigger
project.
If there's no objection, I'll sketch up a patch for that...
OK, this turned out a bit more yucky than I assumed, because
SetFileInformationByHandle is only available on Windows Vista and up.
There's a static library (FileExtd.lib) that ships with MSVC that
emulates it for legacy support, I guess I should have a look at what
that does.
--
To unsubscribe from this list: send the line 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 v2] config: preserve config file permissions on edits
On Mon, May 19, 2014 at 9:17 PM, Thomas Braun
thomas.br...@virtuell-zuhause.de wrote:
Am Montag, den 19.05.2014, 11:59 +0200 schrieb Erik Faye-Lund:
On Mon, May 19, 2014 at 9:44 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
On Mon, May 19, 2014 at 9:13 AM, Johannes Sixt j.s...@viscovery.net
wrote:
Am 5/6/2014 2:17, schrieb Eric Wong:
Users may already store sensitive data such as imap.pass in
..git/config; making the file world-readable when git config
is called to edit means their password would be compromised
on a shared system.
[v2: updated for section renames, as noted by Junio]
Signed-off-by: Eric Wong normalper...@yhbt.net
---
config.c | 16
t/t1300-repo-config.sh | 10 ++
2 files changed, 26 insertions(+)
diff --git a/config.c b/config.c
index a30cb5c..c227aa8 100644
--- a/config.c
+++ b/config.c
@@ -1636,6 +1636,13 @@ int git_config_set_multivar_in_file(const char
*config_filename,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
+ if (fchmod(fd, st.st_mode 0) 0) {
+ error(fchmod on %s failed: %s,
+ lock-filename, strerror(errno));
+ ret = CONFIG_NO_WRITE;
+ goto out_free;
+ }
This introduces a severe failure in the Windows port because we do not
implement fchmod. Existing fchmod invocations do not check the return
value, and they are only interested in removing the write bits, and we
generally don't care a lot if files remain writable.
I'm not proficient enough to add any ACL fiddling to fchmod that would be
required by the above change, whose purpose is to be strict about
permissions. Nor am I interested (who the heck is sharing a Windows box
anyway? ;-)
Therefore, here's just a work-around patch to keep things going on
Windows. Any opinions from the Windows corner?
Since we use MSVCRT's chmod implementation directly which only checks
for S_IWRITE,and Get/SetFileAttributes to simply set or clear the
FILE_ATTRIBUTE_READONLY-flag, perhaps we could do the same except
using Get/SetFileInformationByHandle instead? That takes us in a
better direction, IMO. Adding full ACL support seems like a bigger
project.
If there's no objection, I'll sketch up a patch for that...
OK, this turned out a bit more yucky than I assumed, because
SetFileInformationByHandle is only available on Windows Vista and up.
There's a static library (FileExtd.lib) that ships with MSVC that
emulates it for legacy support, I guess I should have a look at what
that does.
Do we still want to fully support Windows XP?
Adding a work around here for a EOL operating system sounds like a lot
of effort.
I personally don't care about Windows XP, but some other influential
people (J6T IIRC) disagreed the last time this was discussed. I don't
want to step on anyone's toes.
--
To unsubscribe from this list: send the line 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/RFC] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Thomas Braun wrote: pushing over the dump git protocol with a windows git client. I've never heard of the dump git protocol. Do you mean the git protocol that's used with git:// URLs? [...] Alternative approaches considered but deemed too invasive: - Rewrite read/write wrappers in mingw.c in order to distinguish between a file descriptor which has a socket behind and a file descriptor which has a file behind. I assume here too invasive means too much engineering effort? It sounds like a clean fix, not too invasive at all. But I can understand wanting a stopgap in the meantime. Yeah, now that the problem seems to be understood, I don't think that would be too bad. I recently killed off our previous write()-wrapper in c9df6f4, but I see no reason why we can't add a new one. Would we need to wrap both ends, shouldn't wrapping only reading be good enough to prevent deadlocking? compat/poll/poll.c already contains a function called IsSocketHandle that is able to tell if a HANDLE points to a socket or not. -- To unsubscribe from this list: send the line 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/RFC] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 10:00 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
On Mon, May 19, 2014 at 9:33 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Hi,
Thomas Braun wrote:
pushing over the dump git protocol with a windows git client.
I've never heard of the dump git protocol. Do you mean the git
protocol that's used with git:// URLs?
[...]
Alternative approaches considered but deemed too invasive:
- Rewrite read/write wrappers in mingw.c in order to distinguish between
a file descriptor which has a socket behind and a file descriptor
which has a file behind.
I assume here too invasive means too much engineering effort?
It sounds like a clean fix, not too invasive at all. But I can
understand wanting a stopgap in the meantime.
Yeah, now that the problem seems to be understood, I don't think that
would be too bad. I recently killed off our previous write()-wrapper
in c9df6f4, but I see no reason why we can't add a new one.
Would we need to wrap both ends, shouldn't wrapping only reading be
good enough to prevent deadlocking?
compat/poll/poll.c already contains a function called IsSocketHandle
that is able to tell if a HANDLE points to a socket or not.
This very quick attempt did not work out :(
diff --git a/compat/mingw.c b/compat/mingw.c
index 0335958..ec1d81f 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -370,6 +370,65 @@ int mingw_open (const char *filename, int oflags, ...)
return fd;
}
+#define is_console_handle(h) (((long) (h) 3) == 3)
+
+static int is_socket_handle(HANDLE h)
+{
+WSANETWORKEVENTS ev;
+
+if (is_console_handle(h))
+return 0;
+
+/*
+ * Under Wine, it seems that getsockopt returns 0 for pipes too.
+ * WSAEnumNetworkEvents instead distinguishes the two correctly.
+ */
+ev.lNetworkEvents = 0xDEADBEEF;
+WSAEnumNetworkEvents((SOCKET) h, NULL, ev);
+return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
+#undef read
+ssize_t mingw_read(int fd, void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return read(fd, buf, count);
+
+ret = recv((SOCKET)fh, buf, count, 0);
+if (ret 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+#undef write
+ssize_t mingw_write(int fd, const void *buf, size_t count)
+{
+int ret;
+HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+if (fh == INVALID_HANDLE_VALUE) {
+errno = EBADF;
+return -1;
+}
+
+if (!is_socket_handle(fh))
+return write(fd, buf, count);
+
+return send((SOCKET)fh, buf, count, 0);
+if (ret 0)
+errno = WSAGetLastError();
+return ret;
+}
+
+
static BOOL WINAPI ctrl_ignore(DWORD type)
{
return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 08b83fe..1690098 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -177,6 +177,12 @@ int mingw_rmdir(const char *path);
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
+ssize_t mingw_read(int fd, void *buf, size_t count);
+#define read mingw_read
+
+ssize_t mingw_write(int fd, const void *buf, size_t count);
+#define write mingw_write
+
int mingw_fgetc(FILE *stream);
#define fgetc mingw_fgetc
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] send-pack.c: Allow to disable side-band-64k
On Mon, May 19, 2014 at 11:15 PM, Thomas Braun thomas.br...@byte-physics.de wrote: Am 19.05.2014 21:33, schrieb Jonathan Nieder: Hi, Thomas Braun wrote: pushing over the dump git protocol with a windows git client. I've never heard of the dump git protocol. Do you mean the git protocol that's used with git:// URLs? You are right I mean the protocol involving git:// URLs. But unfortunately I got it wrong as according to [1] the git:// is one of the so-called smart protocols. That was also the source where I read that there are smart and dump protocols. [1]: http://git-scm.com/book/en/Git-Internals-Transfer-Protocols [...] Alternative approaches considered but deemed too invasive: - Rewrite read/write wrappers in mingw.c in order to distinguish between a file descriptor which has a socket behind and a file descriptor which has a file behind. I assume here too invasive means too much engineering effort? It sounds like a clean fix, not too invasive at all. But I can understand wanting a stopgap in the meantime. No actually I meant too invasive in the sense of requiring large rewrites which only benefit git on windows and hurt all others. The two fixes I can think of either involve: - In a read *and* write wrapper the need to check if the fd is a socket, if yes use send/recv if no use read/write. According to Erik's comments this should be possible. But I would deem the expected performance penalty quite large as that will be done in every call. You clearly haven't stepped through MSVCRT's read and write implementations :P I wouldn't worry too much about this, at least not until the numbers are in. -- To unsubscribe from this list: send the line 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] wincred: avoid overwriting configured variables
On Wed, May 14, 2014 at 10:45 AM, Stepan Kasal ka...@ucw.cz wrote: Hello kusma, On Tue, May 13, 2014 at 08:56:54AM +0200, Erik Faye-Lund wrote: --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,12 +1,12 @@ all: git-credential-wincred.exe -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall - -include ../../../config.mak.autogen -include ../../../config.mak +CC ?= gcc +RM ?= rm -f +CFLAGS ?= -O2 -Wall + prefix ?= /usr/local libexecdir ?= $(prefix)/libexec/git-core Yeah, looks good to me. thanks, but it looks you replied only to my personal mail. Was it intentional? No, sorry about that. Consider the patches Acked-by: Erik Faye-Lund kusmab...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] wincred: avoid overwriting configured variables
On Tue, May 13, 2014 at 8:01 AM, Stepan Kasal ka...@ucw.cz wrote: From: Pat Thoyts pattho...@users.sourceforge.net Date: Wed, 24 Oct 2012 00:15:29 +0100 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net Signed-off-by: Stepan Kasal ka...@ucw.cz --- contrib/credential/wincred/Makefile | 4 1 file changed, 4 deletions(-) diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index 39fa5e0..e64cd9a 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,9 +1,5 @@ all: git-credential-wincred.exe -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall - Would it be better to set these if not already set, i.e: -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall +CC ?= gcc +RM ?= rm -f +CFLAGS ?= -O2 -Wall instead? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug - Git reset --quiet not quiet
On Mon, May 12, 2014 at 9:16 PM, Thomas-Louis Laforest tllafor...@arbault.ca wrote: Good afternoon, When running this command on Git for Windows (version 1.9.2-preview20140411) git reset --quiet --hard with one file having read/write lock git ask this question : Unlink of file '' failed. Should I try again? (y/n) I will have expected the command --quiet to remove the question and auto-answer no. This broke an automated script we use. Thanks for reporting this. The problem here is really a nasty case of layering: the question comes from a place deep inside the OS compatibility layer, which doesn't know about the --quiet flag. However, do note that if fixed, the command would still fail under these conditions. But it won't hang forever, as it does now. Mainline Git-folks: The problem here is essentially unlink returning EBUSY (although that's not *exactly* what happes - but it's close enough, implementation details in mingw_unlink), which most of the git codebase assume won't happen. On Windows, this happens *all* the time, usually due to antivirus-software scanning a newly written file. We currently retry a few times with some waiting in mingw_unlink, and then finally prompts the user. But this gives the problem described above, as mingw_unlink has no clue about --quiet. I guess this could be solved in a few ways. 1) Let mingw_unlink() know about the quiet-flag. This probably involves moving the quiet-flag from each tool into libgit.a. 2) Make the quiet-flags close stdout instead of suppressing every output. 3) Make the higher level call-sites of Git EBUSY-aware. This probably involves making an interactive convenience-wrapper of unlink, that accepts a quiet flag - similar to what mingw_unlink does. Option 1) seems quite error-prone to me - it's difficult to make sure all code-paths actually set this flag, so there's a good chance of regressions. Option 2) also sounds a bit risky, as we lose stdout forever, with no escape-hatch. So to me option 3) seems preferable although it might translate into a bit of churn. Thoughts? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug - Git reset --quiet not quiet
On Tue, May 13, 2014 at 11:38 AM, Johannes Sixt j.s...@viscovery.net wrote: Am 5/13/2014 11:09, schrieb Erik Faye-Lund: On Mon, May 12, 2014 at 9:16 PM, Thomas-Louis Laforest tllafor...@arbault.ca wrote: When running this command on Git for Windows (version 1.9.2-preview20140411) git reset --quiet --hard with one file having read/write lock git ask this question : Unlink of file '' failed. Should I try again? (y/n) I will have expected the command --quiet to remove the question and auto-answer no. This broke an automated script we use. ... I guess this could be solved in a few ways. 1) Let mingw_unlink() know about the quiet-flag. This probably involves moving the quiet-flag from each tool into libgit.a. 2) Make the quiet-flags close stdout instead of suppressing every output. 3) Make the higher level call-sites of Git EBUSY-aware. This probably involves making an interactive convenience-wrapper of unlink, that accepts a quiet flag - similar to what mingw_unlink does. Is any of this really needed? We have this in ask_yes_no_if_possible(): if (!isatty(_fileno(stdin)) || !isatty(_fileno(stderr))) return 0; i.e., we answer no automatically without asking if at least one of stdin and stderr are not a terminal. Perhaps the OP's problem is that they do not redirect these channels to files or something in their automated scripts? In particular, it should be sufficient to redirect stdin from /dev/null (a.k.a. nul on Windows). Well, sure. But if sounds like surprising behavior to me. And I also suspect doing this unconditionally on all platforms will fix strange corner-cases on other systems, when using Windows file-systems. AFAIK, the whole cannot delete an open file-thing is an NTFS-detail (and possibly also FAT, which is quite commonly used on non-Windows). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Add read-cache--daemon
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
diff --git a/Makefile b/Makefile
index 028749b..98d22de 100644
--- a/Makefile
+++ b/Makefile
@@ -1502,6 +1502,12 @@ ifdef HAVE_DEV_TTY
BASIC_CFLAGS += -DHAVE_DEV_TTY
endif
+ifdef HAVE_SHM
+ BASIC_CFLAGS += -DHAVE_SHM
+ EXTLIBS += -lrt
+ PROGRAM_OBJS += read-cache--daemon.o
+endif
+
I think read-cache--daemon will fail in case of NO_UNIX_SOCKETS.
But, read-cache--daemon.c only gets compiled if we have shm_open...
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..b2116ab 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,4 +723,12 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
#define gmtime_r git_gmtime_r
#endif
+#ifndef HAVE_SHM
+static inline int shm_open(const char *path, int flags, int mode)
+{
+ errno = ENOSYS;
+ return -1;
+}
+#endif
...yet, you introduce a compatibility-shim...
diff --git a/read-cache--daemon.c b/read-cache--daemon.c
new file mode 100644
index 000..52b4067
--- /dev/null
+++ b/read-cache--daemon.c
@@ -0,0 +1,167 @@
+#include cache.h
+#include sigchain.h
+#include unix-socket.h
+#include split-index.h
+#include pkt-line.h
+
+static char *socket_path;
+static struct strbuf shm_index = STRBUF_INIT;
+static struct strbuf shm_sharedindex = STRBUF_INIT;
+
+static void cleanup_socket(void)
+{
+ if (socket_path)
+ unlink(socket_path);
+ if (shm_index.len)
+ shm_unlink(shm_index.buf);
+ if (shm_sharedindex.len)
+ shm_unlink(shm_sharedindex.buf);
+}
+
+static void cleanup_socket_on_signal(int sig)
+{
+ cleanup_socket();
+ sigchain_pop(sig);
+ raise(sig);
+}
+
+static void share_index(struct index_state *istate, struct strbuf *shm_path)
+{
+ struct strbuf sb = STRBUF_INIT;
+ void *map;
+ int fd;
+
+ strbuf_addf(sb, /git-index-%s, sha1_to_hex(istate-sha1));
+ if (shm_path-len strcmp(sb.buf, shm_path-buf)) {
+ shm_unlink(shm_path-buf);
+ strbuf_reset(shm_path);
+ }
+ fd = shm_open(sb.buf, O_RDWR | O_CREAT | O_TRUNC, 0700);
+ if (fd 0)
+ return;
...that only gets called from read-cache--daemon.c, which already only
gets compiled if we have open?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] Add read-cache--daemon for caching index and related stuff
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: diff --git a/Documentation/git-read-cache--daemon.txt b/Documentation/git-read-cache--daemon.txt new file mode 100644 index 000..1b05be4 --- /dev/null +++ b/Documentation/git-read-cache--daemon.txt @@ -0,0 +1,27 @@ +git-read-cache--daemon(1) += + +NAME + +git-daemon - A simple cache server for speeding up index file access + +SYNOPSIS + +[verse] +'git daemon' [--detach] + Huh, git daemon can't be right... diff --git a/Makefile b/Makefile index d0a2b4b..a44ab0b 100644 --- a/Makefile +++ b/Makefile @@ -1504,6 +1504,12 @@ ifdef HAVE_DEV_TTY BASIC_CFLAGS += -DHAVE_DEV_TTY endif +ifdef HAVE_SHM + BASIC_CFLAGS += -DHAVE_SHM + EXTLIBS += -lrt + PROGRAM_OBJS += read-cache--daemon.o +endif I think this also needs to be protected against NO_UNIX_SOCKETS -- To unsubscribe from this list: send the line 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/8] unix-socket: stub impl. for platforms with no unix socket support
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
With this we can make unix_stream_* calls without #ifdef.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Makefile | 2 ++
unix-socket.h | 18 ++
2 files changed, 20 insertions(+)
diff --git a/Makefile b/Makefile
index 028749b..d0a2b4b 100644
--- a/Makefile
+++ b/Makefile
@@ -1417,6 +1417,8 @@ ifndef NO_UNIX_SOCKETS
LIB_H += unix-socket.h
PROGRAM_OBJS += credential-cache.o
PROGRAM_OBJS += credential-cache--daemon.o
+else
+ BASIC_CFLAGS += -DNO_UNIX_SOCKETS
endif
ifdef NO_ICONV
diff --git a/unix-socket.h b/unix-socket.h
index e271aee..f1cba70 100644
--- a/unix-socket.h
+++ b/unix-socket.h
@@ -1,7 +1,25 @@
#ifndef UNIX_SOCKET_H
#define UNIX_SOCKET_H
+#ifndef NO_UNIX_SOCKETS
+
int unix_stream_connect(const char *path);
int unix_stream_listen(const char *path);
+#else
+
+static inline int unix_stream_connect(const char *path)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+static inline int unix_stream_listen(const char *path)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+#endif
+
#endif /* UNIX_SOCKET_H */
OK, so I missed this before my other two comments. But still... in
what way does errno=ENOSYS make this *work*? Won't we end up compiling
lots of non-functional tools on Windows in this case?
--
To unsubscribe from this list: send the line 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/8] unix-socket: stub impl. for platforms with no unix socket support
On Tue, May 13, 2014 at 1:59 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com
wrote:
With this we can make unix_stream_* calls without #ifdef.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Makefile | 2 ++
unix-socket.h | 18 ++
2 files changed, 20 insertions(+)
diff --git a/Makefile b/Makefile
index 028749b..d0a2b4b 100644
--- a/Makefile
+++ b/Makefile
@@ -1417,6 +1417,8 @@ ifndef NO_UNIX_SOCKETS
LIB_H += unix-socket.h
PROGRAM_OBJS += credential-cache.o
PROGRAM_OBJS += credential-cache--daemon.o
+else
+ BASIC_CFLAGS += -DNO_UNIX_SOCKETS
endif
ifdef NO_ICONV
diff --git a/unix-socket.h b/unix-socket.h
index e271aee..f1cba70 100644
--- a/unix-socket.h
+++ b/unix-socket.h
@@ -1,7 +1,25 @@
#ifndef UNIX_SOCKET_H
#define UNIX_SOCKET_H
+#ifndef NO_UNIX_SOCKETS
+
int unix_stream_connect(const char *path);
int unix_stream_listen(const char *path);
+#else
+
+static inline int unix_stream_connect(const char *path)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+static inline int unix_stream_listen(const char *path)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+#endif
+
#endif /* UNIX_SOCKET_H */
OK, so I missed this before my other two comments. But still... in
what way does errno=ENOSYS make this *work*? Won't we end up compiling
lots of non-functional tools on Windows in this case?
ENOSYS makes git-credential-cache.c just die with the message unable
to start cache daemon, and git-credential--daemon.c die with unable
to bind to socket_path.
--
To unsubscribe from this list: send the line 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/8] read-cache: try index data from shared memory
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/config.txt | 4
cache.h | 1 +
config.c | 12
environment.c| 1 +
read-cache.c | 43 +++
submodule.c | 1 +
6 files changed, 62 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d8b6cc9..ccbe00b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -617,6 +617,10 @@ relatively high IO latencies. With this set to 'true',
Git will do the
index comparison to the filesystem data in parallel, allowing
overlapping IO's.
+core.useReadCacheDaemon::
+ Use `git read-cache--daemon` to speed up index reading. See
+ linkgit:git-read-cache--daemon for more information.
+
core.createObject::
You can set this to 'link', in which case a hardlink followed by
a delete of the source are used to make sure that object creation
diff --git a/cache.h b/cache.h
index d0ff11c..fb29c7e 100644
--- a/cache.h
+++ b/cache.h
@@ -603,6 +603,7 @@ extern size_t packed_git_limit;
extern size_t delta_base_cache_limit;
extern unsigned long big_file_threshold;
extern unsigned long pack_size_limit_cfg;
+extern int use_read_cache_daemon;
/*
* Do replace refs need to be checked this run? This variable is
diff --git a/config.c b/config.c
index a30cb5c..5c832ad 100644
--- a/config.c
+++ b/config.c
@@ -874,6 +874,18 @@ static int git_default_core_config(const char *var,
const char *value)
return 0;
}
+#ifdef HAVE_SHM
+ /*
+* Currently git-read-cache--daemon is only built when
+* HAVE_SHM is set. Ignore user settings if HAVE_SHM is not
+* defined.
+*/
+ if (!strcmp(var, core.usereadcachedaemon)) {
+ use_read_cache_daemon = git_config_bool(var, value);
+ return 0;
+ }
+#endif
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/environment.c b/environment.c
index 5c4815d..b76a414 100644
--- a/environment.c
+++ b/environment.c
@@ -63,6 +63,7 @@ int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
+int use_read_cache_daemon;
/*
* The character that begins a commented line in user-editable file
diff --git a/read-cache.c b/read-cache.c
index a5031f3..0e46523 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1462,6 +1462,48 @@ static struct cache_entry *create_from_disk(struct
ondisk_cache_entry *ondisk,
return ce;
}
+static void *try_shm(void *mmap, size_t *mmap_size)
+{
+ struct strbuf sb = STRBUF_INIT;
+ size_t old_size = *mmap_size;
+ void *new_mmap;
+ struct stat st;
+ int fd;
+
+ if (old_size = 20 || !use_read_cache_daemon)
+ return mmap;
+
+ strbuf_addf(sb, /git-index-%s.lock,
+ sha1_to_hex((unsigned char *)mmap + old_size - 20));
+ fd = shm_open(sb.buf, O_RDONLY, 0777);
OK, so *here* you do an unguarded use of shm_open, but the code never
gets executed because of the HAVE_SHM guard in
git_default_core_config. Perhaps not introduce the compatibility shim
until this patch, then?
--
To unsubscribe from this list: send the line 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 8/8] read-cache: inform the daemon that the index has been updated
On Tue, May 13, 2014 at 1:15 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
The daemon would immediately load the new index in memory in
background. Next time Git needs to read the index again, everything is
ready.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
cache.h | 1 +
read-cache.c | 53 -
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index fb29c7e..3115b86 100644
--- a/cache.h
+++ b/cache.h
@@ -483,6 +483,7 @@ extern int is_index_unborn(struct index_state *);
extern int read_index_unmerged(struct index_state *);
#define COMMIT_LOCK(1 0)
#define CLOSE_LOCK (1 1)
+#define REFRESH_DAEMON (1 2)
extern int write_locked_index(struct index_state *, struct lock_file *lock,
unsigned flags);
extern int discard_index(struct index_state *);
extern int unmerged_index(const struct index_state *);
diff --git a/read-cache.c b/read-cache.c
index e98521f..d5c9247 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -16,6 +16,9 @@
#include varint.h
#include split-index.h
#include sigchain.h
+#include unix-socket.h
+#include pkt-line.h
+#include run-command.h
static struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
unsigned int options);
@@ -2030,6 +2033,32 @@ void set_alternate_index_output(const char *name)
alternate_index_output = name;
}
+static void refresh_daemon(struct index_state *istate)
+{
+ int fd;
+ fd = unix_stream_connect(git_path(daemon/index));
+ if (fd 0) {
+ struct child_process cp;
+ const char *av[] = {read-cache--daemon, --detach, NULL };
+ memset(cp, 0, sizeof(cp));
+ cp.argv = av;
+ cp.git_cmd = 1;
+ cp.no_stdin = 1;
+ if (run_command(cp))
+ warning(_(failed to start read-cache--daemon: %s),
+ strerror(errno));
+ return;
+ }
+ /*
+* packet_write() could die() but unless this is from
+* update_index_if_able(), we're about to exit anyway,
+* probably ok to die (for now). Blocking mode is another
+* problem to deal with later.
+*/
+ packet_write(fd, refresh);
+ close(fd);
+}
+
Seems the argument 'istate' isn't used.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Add read-cache--daemon
On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote: What do you think is a good replacement for unix socket on Windows? It's only used to refresh the cache in the daemon, no sensitive data sent over, so security is not a problem. I'm thinking maybe just TCP/IP server, but that's going to be a system-wide daemon.. Perhaps the windows daemon could just monitor $GIT_DIR/index and refresh it? Windows has support for Named Pipes, which seems like the right kind of communication channel. However, the programming model differs quite a bit from unix-sockets: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Add read-cache--daemon
On Tue, May 13, 2014 at 3:49 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote: What do you think is a good replacement for unix socket on Windows? It's only used to refresh the cache in the daemon, no sensitive data sent over, so security is not a problem. I'm thinking maybe just TCP/IP server, but that's going to be a system-wide daemon.. Perhaps the windows daemon could just monitor $GIT_DIR/index and refresh it? Windows has support for Named Pipes, which seems like the right kind of communication channel. However, the programming model differs quite a bit from unix-sockets: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx Yeah that was my first option, but if code cannot be shared to differences then we probably should go another way. The old FindWindow/PostMessage still works with modern Windows, right? Maybe we could create a window with a name derived from the daemon's pid and save the name in the index, then PostMessage can signal the daemon. On the UNIX front, we store pid and send SIGUSR1 instead..The good thing here is the Git side will be very simple (PostMessage vs kill). Hmmm I'm a bit worried about having to load in USER32.DLL just to read the cache that way. But it seems we already do that, thanks to compat/poll/poll.c (it depends on DispatchMessage, MsgWaitForMultipleObjects, PeekMessage and TranslateMessage, all from that DLL). Preferably, we should delay-load USER32.DLL in compat/poll/poll.c, but if we start needing it for the reading the index, it'll be loaded by the vast majority of processes anyway. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Add read-cache--daemon
On Tue, May 13, 2014 at 4:10 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, May 13, 2014 at 9:06 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Tue, May 13, 2014 at 3:49 PM, Duy Nguyen pclo...@gmail.com wrote: On Tue, May 13, 2014 at 8:37 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Tue, May 13, 2014 at 3:01 PM, Duy Nguyen pclo...@gmail.com wrote: What do you think is a good replacement for unix socket on Windows? It's only used to refresh the cache in the daemon, no sensitive data sent over, so security is not a problem. I'm thinking maybe just TCP/IP server, but that's going to be a system-wide daemon.. Perhaps the windows daemon could just monitor $GIT_DIR/index and refresh it? Windows has support for Named Pipes, which seems like the right kind of communication channel. However, the programming model differs quite a bit from unix-sockets: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365594%28v=vs.85%29.aspx Yeah that was my first option, but if code cannot be shared to differences then we probably should go another way. The old FindWindow/PostMessage still works with modern Windows, right? Maybe we could create a window with a name derived from the daemon's pid and save the name in the index, then PostMessage can signal the daemon. On the UNIX front, we store pid and send SIGUSR1 instead..The good thing here is the Git side will be very simple (PostMessage vs kill). Hmmm I'm a bit worried about having to load in USER32.DLL just to read the cache that way. But it seems we already do that, thanks to compat/poll/poll.c (it depends on DispatchMessage, MsgWaitForMultipleObjects, PeekMessage and TranslateMessage, all from that DLL). Preferably, we should delay-load USER32.DLL in compat/poll/poll.c, but if we start needing it for the reading the index, it'll be loaded by the vast majority of processes anyway. Thanks for the info. I'll see if we can still stick to named pipes. If we have to load user32.dll, hopefully the gain will outweigh load time for that dell. I just timed it here on my system, and omitting USER32.DLL didn't gain anything for git --version, so I suspect I was worrying too 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 v1 04/25] contrib: remove 'buildsystems'
On Fri, May 9, 2014 at 2:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: No activity since 2010, no documentation, no tests. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/buildsystems/Generators.pm| 42 -- contrib/buildsystems/Generators/QMake.pm | 189 - contrib/buildsystems/Generators/Vcproj.pm | 626 -- contrib/buildsystems/engine.pl| 359 - contrib/buildsystems/generate | 29 -- contrib/buildsystems/parse.pl | 228 --- 6 files changed, 1473 deletions(-) delete mode 100644 contrib/buildsystems/Generators.pm delete mode 100644 contrib/buildsystems/Generators/QMake.pm delete mode 100644 contrib/buildsystems/Generators/Vcproj.pm delete mode 100755 contrib/buildsystems/engine.pl delete mode 100755 contrib/buildsystems/generate delete mode 100755 contrib/buildsystems/parse.pl Please don't. This script is useful to build with the MSVC IDE, which enables us to use their excellent debugger. -- To unsubscribe from this list: send the line 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 v1 04/25] contrib: remove 'buildsystems'
On Fri, May 9, 2014 at 10:14 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 2:58 AM, Felipe Contreras felipe.contre...@gmail.com wrote: No activity since 2010, no documentation, no tests. Signed-off-by: Felipe Contreras felipe.contre...@gmail.com --- contrib/buildsystems/Generators.pm| 42 -- contrib/buildsystems/Generators/QMake.pm | 189 - contrib/buildsystems/Generators/Vcproj.pm | 626 -- contrib/buildsystems/engine.pl| 359 - contrib/buildsystems/generate | 29 -- contrib/buildsystems/parse.pl | 228 --- 6 files changed, 1473 deletions(-) delete mode 100644 contrib/buildsystems/Generators.pm delete mode 100644 contrib/buildsystems/Generators/QMake.pm delete mode 100644 contrib/buildsystems/Generators/Vcproj.pm delete mode 100755 contrib/buildsystems/engine.pl delete mode 100755 contrib/buildsystems/generate delete mode 100755 contrib/buildsystems/parse.pl Please don't. This script is useful to build with the MSVC IDE, which enables us to use their excellent debugger. If you want this script to remain in contrib, please: a) Write at least a few tests b) Write some documentation c) Explain why it cannot live outside the git.git repository like other tools. [1][2][3] (Adding Marius, the original author to the CC-list) Uh, why is such a burden required all of a sudden? contrib/README mentions no such requirements, and the scripts have been accepted (and maintained) since. Besides, you say No activity since 2010 - this is not the case, bc380fc is from November 2013. And there's already *some* documentation in the scripts themselves. Please stop your pointless crusade that'll only break other people's work-flows. -- To unsubscribe from this list: send the line 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 v1 04/25] contrib: remove 'buildsystems'
On Fri, May 9, 2014 at 10:48 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 10:14 AM, Felipe Contreras felipe.contre...@gmail.com wrote: If you want this script to remain in contrib, please: a) Write at least a few tests b) Write some documentation c) Explain why it cannot live outside the git.git repository like other tools. [1][2][3] (Adding Marius, the original author to the CC-list) Uh, why is such a burden required all of a sudden? contrib/README mentions no such requirements, and the scripts have been accepted (and maintained) since. contrib/README mentions clearly the expectation that these scripts eventually move to the core once they mature. This is never going to happen for these. Yes, *expectation*. Not requirement. It also mentions that inactive ones would be proposed for removal, and this one is clearly inactive. It has 9 commits (if you count the one that changes the execution bit). It mentions that Junio *might* suggest things to be removed, not that things *should* be removed if left unmaintained. And this script is not unmaintained, it's simply just still working. Besides, you say No activity since 2010 - this is not the case, bc380fc is from November 2013. You think changing the execution bit of a file is considered activity? Well, now we're getting into semantics, which I don't care so much about. It shows some sort of interest in the scripts, at least. And there's already *some* documentation in the scripts themselves. That's nice. So you can just copy that into a README. Feel free to scratch that itch yourself, you're the one inventing new requirements here. Please stop your pointless crusade that'll only break other people's work-flows. If you care about these scripts, it should be trivial for you to add at least a few tests, souldn't it? Again, testing this is not my itch. Feel free to scratch that one yourself, but please don't remove the script. Please tell me how exactly will your work-flow be broken. More specifically, tell me why your scripts cannot be moved outside of git, like git-extras[1], git-deploy[2], git-ftp[3], and countless other tools. Moving the script out of the repo makes it less convenient to bisect issues with MSVC, as it depends heavily on the top-level Makefile. Moving it out would require figuring out what version of the script matches a given git revision, which is a hassle. Again, please stop this pointless crusade. -- To unsubscribe from this list: send the line 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 v1 04/25] contrib: remove 'buildsystems'
On Fri, May 9, 2014 at 11:32 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 10:48 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 10:14 AM, Felipe Contreras felipe.contre...@gmail.com wrote: If you want this script to remain in contrib, please: a) Write at least a few tests b) Write some documentation c) Explain why it cannot live outside the git.git repository like other tools. [1][2][3] (Adding Marius, the original author to the CC-list) Uh, why is such a burden required all of a sudden? contrib/README mentions no such requirements, and the scripts have been accepted (and maintained) since. contrib/README mentions clearly the expectation that these scripts eventually move to the core once they mature. This is never going to happen for these. Yes, *expectation*. Not requirement. That's right, but these tools fail all expectations. It also mentions that inactive ones would be proposed for removal, and this one is clearly inactive. It has 9 commits (if you count the one that changes the execution bit). It mentions that Junio *might* suggest things to be removed, not that things *should* be removed if left unmaintained. That's right. And this script is not unmaintained, it's simply just still working. Prove it. Either way, if there was people actively caring about these scripts, there should be cleanups, tests, documentation. But there's nothing. Besides, you say No activity since 2010 - this is not the case, bc380fc is from November 2013. You think changing the execution bit of a file is considered activity? Well, now we're getting into semantics, which I don't care so much about. Convenient. Yeah, the part above here goes in my don't argue with idiots, they'll drag you down to their level and beat you with experience-filter. Good luck trying to convince *anyone* with this line of argumentation. It shows some sort of interest in the scripts, at least. Not it doesn't. Jonathan Nieder updated the execution bit on a bunch of scripts in contrib, these being just in the way. It doesn't show interest at all. All of those changes relate to the MSVC-build. So it's not just some batch-fixup as you're trying to suggest. And there's already *some* documentation in the scripts themselves. That's nice. So you can just copy that into a README. Feel free to scratch that itch yourself, you're the one inventing new requirements here. If you care about these scripts, you have an interesting way of showing it. Please stop your pointless crusade that'll only break other people's work-flows. If you care about these scripts, it should be trivial for you to add at least a few tests, souldn't it? Again, testing this is not my itch. Feel free to scratch that one yourself, but please don't remove the script. If you don't care that these scripts keep working properly, I don't see why anybody else would. You're the one making up requirements for tests here, so this is your itch. This script gets fixed by it's stake-holders when it breaks, and that has worked out fine so far. Please tell me how exactly will your work-flow be broken. More specifically, tell me why your scripts cannot be moved outside of git, like git-extras[1], git-deploy[2], git-ftp[3], and countless other tools. Moving the script out of the repo makes it less convenient to bisect issues with MSVC, as it depends heavily on the top-level Makefile. Moving it out would require figuring out what version of the script matches a given git revision, which is a hassle. The script doesn't depend on the version of the Makefile, and proof of that is that is has *never* been changed even though the Makefile has. Except it has, in 74cf9bd. -- To unsubscribe from this list: send the line 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 v1 04/25] contrib: remove 'buildsystems'
On Fri, May 9, 2014 at 12:57 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 11:32 AM, Felipe Contreras felipe.contre...@gmail.com wrote: Erik Faye-Lund wrote: On Fri, May 9, 2014 at 10:48 AM, Felipe Contreras You think changing the execution bit of a file is considered activity? Well, now we're getting into semantics, which I don't care so much about. Convenient. Yeah, the part above here goes in my don't argue with idiots, they'll drag you down to their level and beat you with experience-filter. Good luck trying to convince *anyone* with this line of argumentation. It has been demonstrated that there is inactivity. The fact that your semantics about inactivity differ from the rest of the world is irrelevant. The script doesn't depend on the version of the Makefile, and proof of that is that is has *never* been changed even though the Makefile has. Except it has, in 74cf9bd. Once change in *four* years. My god! How are people ever going to keep up with such amount of changes if it moves out-of-tree! It's rather amusing to see you react to my definition of activity, when you seem to have a rather unusual definition of never... -- To unsubscribe from this list: send the line 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] t3910: show failure of core.precomposeunicode with decomposed filenames
On Mon, May 5, 2014 at 11:46 PM, Jeff King p...@peff.net wrote: On Sun, May 04, 2014 at 08:13:15AM +0200, Torsten Bögershausen wrote: 1. Tell everyone that NFD in the git repo is wrong, and they should make a new commit to normalize all their in-repo files to be precomposed. This is probably not the right thing to do, because it still doesn't fix checkouts of old history. And it spreads the problem to people on byte-preserving filesystems (like ext4), because now they have to start precomposing their filenames as they are adde to git. (typo: ^added) I'm not sure if I follow. People running ext4 (or Linux in general, or Windows, or Unix) do not suffer from file system feature of Mac OS, which accepts precomposed/decomposed Unicode but returns decompomsed. What I mean by spreads the problem is that git on Linux does not need to care about utf8 at all. It treats filenames as a byte sequence. But if we were to start enforcing filenames should be precomposed utf8, then people adding files on Linux would want to enforce that, too. People on Linux could ignore the issue as they do now, but they would then create problems for OS X users if they add decomposed filenames. IOW, if the OS X code assumes all repo filenames are precomposed, then other systems become a possible vector for violating that assumption. FWIW, Git for Windows also doesn't deal with that filenames are just a byte-sequence-notion. We have patches in place that require filenames to *either* be valid UTF-8 or Windows-1252. Windows itself treats filenames as Unicode characters, not arbitrary byte sequences. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 9:46 PM, Sebastian Schuberth sschube...@gmail.com wrote: On Wed, Apr 30, 2014 at 6:52 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: We can keep this patch in the msysGit repo for the 2.0 release. FWIW the plan is to switch to mingwGitDevEnv for the 2.0 release. It is not quite clear as of yet how patches will be managed with said environment. The environment is just that: The environment to build Git for Windows. This means that patches on top of Git for Windows could still be maintained in msysgit/git (or a fork thereof) on GitHub. Thanks for the heads up. Even so, are you guys OK with me pushing this patch to our downstream repo? -- To unsubscribe from this list: send the line 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: #178 parsing of pretty=format:%an %ad causes fatal: bad revision '%ad'
On Fri, May 2, 2014 at 1:50 PM, Dave Bradley dbradl...@bell.net wrote: Hi, I’m very new to ‘git’ github. I reported the #178 issue in github and the issue has been closed, I believe this means no further discussion. When you say the #178 issue in github, you really mean issue #178 for Git for Windows on GitHub, available at https://github.com/msysgit/git/issues/178 for those interested. That issue tracker is for the Windows port of Git for Windows. It's intended to track breakages in Git for Windows compared to Git on, say Linux. It's not a general issue tracker for problems with Git. Still, it seems a lot of people think I downloaded Git for Windows, and here's something that didn't work the way I expected it to, I'll file a bug. Those kinds of bug-reports usually gets closed quickly, as it's outside the scope of Git for Windows to decide how Git should behave - we try to make it behave consistently between Windows and Unixy-platforms. This is indeed the right forum to address your issue. So thank you for moving the discussion here. -- To unsubscribe from this list: send the line 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: #178 parsing of pretty=format:%an %ad causes fatal: bad revision '%ad'
On Fri, May 2, 2014 at 7:23 PM, Jonathan Nieder jrnie...@gmail.com wrote: (resending with the correct address for the Git for Windows developers. Sorry for the noise.) Hi Dave, Dave Bradley wrote: G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all --pretty=format:%an %ad -- pom.xml Mon Nov 23 03:09:17 2009 + Mon Nov 23 02:42:24 2009 + G:\ws_test_env\GIT_TESTBED_TMP\fest-swing-1.xgit log --all --pretty=format:%an %ad -- pom.xml fatal: bad revision '%ad' On Linux, this example gets passed to git as six arguments: log --all --pretty=format:%an %ad -- pom.xml As does it on Windows. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/12] MINGW: fix main() signature in http-fetch.c and remote-curl.c
On Wed, Apr 30, 2014 at 10:35 AM, Karsten Blees karsten.bl...@gmail.com wrote: Am 29.04.2014 11:12, schrieb Marat Radchenko: On MinGW, compat/mingw.h defines a 'mingw_main' wrapper function. Fix `warning: passing argument 2 of 'mingw_main' from incompatible pointer type` in http-fetch.c and remote-curl.c by dropping 'const'. Would you mind cross checking your changes with the msysgit fork? Fixing the same thing in a slightly different manner will just cause unnecessary conflicts (i.e. unnecessary work for the Git for Windows maintainers, as well as for you to come up with an alternate solution for something that's long been fixed). See https://github.com/msysgit/git/commit/9206e7fd (squashed from https://github.com/msysgit/git/commit/0115ef83 and https://github.com/msysgit/git/commit/6949537a). While it's certainly a good point, I think this is *our* fault for not upstreaming our changes, and the responsibility of cleaning up merges should lie on our shoulders. We've diverged a lot. Sure, Dscho does a great job making the divergence less painful, but IMO we should try to reduce the delta as well. For instance, I think the Unicode-support patches have proven themselves by now and should go upstream. And there's probably a ton of smaller free-standing clean-ups that could get the same treatment. -- To unsubscribe from this list: send the line 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] wincred: add install target and avoid overwriting configured variables
On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal ka...@ucw.cz wrote: From: Pat Thoyts pattho...@users.sourceforge.net Date: Wed, 24 Oct 2012 00:15:29 +0100 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net Signed-off-by: Stepan Kasal ka...@ucw.cz --- Another one from msysGit project. Original subject by Pat; I would suggest: wincred: improve Makefile I'm a little bit unsure about this, because the makefile was basically just copied from contrib/credential/osxkeychain/Makefile (which was the first credential helper) and tweaked slightly. So, what makes wincred special compared to gnome-keyring, netrc and osxkeychain wrt installation? Shouldn't all helpers get the same treatment? contrib/credential/wincred/Makefile | 22 ++ 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index bad45ca..3ce6aba 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,14 +1,20 @@ -all: git-credential-wincred.exe - -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall - -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-wincred.exe : git-credential-wincred.c +prefix ?= /usr/local +libexecdir ?= $(prefix)/libexec/git-core + +INSTALL ?= install + +GIT_CREDENTIAL_WINCRED := git-credential-wincred.exe Why this variable? IMO, it's just as GIT_CREDENTIAL_WINCRED easy to miss-spell as git-credential-wincred.exe, and it doesn't seem to be possible to overload. + +all: $(GIT_CREDENTIAL_WINCRED) + Also, why move the all-target down from the top? Is it simply because of the definition above? -- To unsubscribe from this list: send the line 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] wincred: add install target and avoid overwriting configured variables
On Wed, Apr 30, 2014 at 1:27 PM, Stepan Kasal ka...@ucw.cz wrote: Hello, On Wed, Apr 30, 2014 at 8:46 AM, Stepan Kasal ka...@ucw.cz wrote: Date: Wed, 24 Oct 2012 00:15:29 +0100 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net Signed-off-by: Stepan Kasal ka...@ucw.cz --- Another one from msysGit project. Original subject by Pat; I would suggest: wincred: improve Makefile On Wed, Apr 30, 2014 at 11:21:17AM +0200, Erik Faye-Lund wrote: I'm a little bit unsure about this, because the makefile was basically just copied from contrib/credential/osxkeychain/Makefile (which was the first credential helper) and tweaked slightly. So, what makes wincred special compared to gnome-keyring, netrc and osxkeychain wrt installation? Shouldn't all helpers get the same treatment? I can only guess that the hardwired CC and CFLAGS values can cause problems. I doubt that a patch that doesn't describe exactly what kind of issues will get merged. And it certainly won't get my ack unless I understand why. It is probably much sane on Windows to use the compiler that the user set up for the build. But you can already do that, the same way as for the rest of git, by overloading these in config.mak in the root of the git repo. I'm not sure if users of, say, OS X, have the same problems, so I would hesitate to apply these changes to all helpers. Even if I bought that it's needed (which I'm currently skeptical to), I think the dunno about OSX is a bit of a cop-out. From: Pat Thoyts pattho...@users.sourceforge.net contrib/credential/wincred/Makefile | 22 ++ 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/contrib/credential/wincred/Makefile b/contrib/credential/wincred/Makefile index bad45ca..3ce6aba 100644 --- a/contrib/credential/wincred/Makefile +++ b/contrib/credential/wincred/Makefile @@ -1,14 +1,20 @@ -all: git-credential-wincred.exe - -CC = gcc -RM = rm -f -CFLAGS = -O2 -Wall - -include ../../../config.mak.autogen -include ../../../config.mak -git-credential-wincred.exe : git-credential-wincred.c +prefix ?= /usr/local +libexecdir ?= $(prefix)/libexec/git-core + +INSTALL ?= install + +GIT_CREDENTIAL_WINCRED := git-credential-wincred.exe Why this variable? IMO, it's just as GIT_CREDENTIAL_WINCRED easy to miss-spell as git-credential-wincred.exe, and it doesn't seem to be possible to overload. If you mis-spell a variable name, nothing is build. If you misspell a binary name, that binary may get compiled using a default rule; that is why I would generally prefer variables. Following that logic, you should be submitting similar patches to the main git Makefile as well. Somehow I doubt that'll happen. Moreover, if the cardinality of the set ever increases, the indirection may get helpful. I don't think there's any reason to expect the number of binaries to increase, so that's moot. And if I'm wrong, let's deal with that when the time comes. It's not like this version is prepared for the variable being a list either - neither should there IMO. + +all: $(GIT_CREDENTIAL_WINCRED) + Also, why move the all-target down from the top? Is it simply because of the definition above? Again, I agree with Pat that it is nicer this way, but no big deal. We don't usually do this is subjectively better-patches in Git. Instead we try to follow the current style. -- To unsubscribe from this list: send the line 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] Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR
On Mon, Apr 28, 2014 at 11:01 PM, Dave Borowitz dborow...@google.com wrote: The original implementation of CURL_CONFIG support did not match the original behavior of using -lcurl when CURLDIR was not set. This broke implementations that were lacking curl-config but did have libcurl installed along system libraries, such as MSysGit. In other words, the assumption that curl-config is always installed was incorrect. Instead, if CURL_CONFIG is empty or returns an empty result (e.g. due to curl-config being missing), use the old behavior of falling back to -lcurl. Signed-off-by: Dave Borowitz dborow...@google.com --- Makefile | 41 - 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 74a929b..81e8214 100644 --- a/Makefile +++ b/Makefile @@ -35,14 +35,17 @@ all:: # transports (neither smart nor dumb). # # Define CURL_CONFIG to the path to a curl-config binary other than the -# default 'curl-config'. +# default 'curl-config'. If CURL_CONFIG is unset or points to a binary that +# is not found, defaults to the CURLDIR behavior. # # Define CURL_STATIC to statically link libcurl. Only applies if # CURL_CONFIG is used. # # Define CURLDIR=/foo/bar if your curl header and library files are in -# /foo/bar/include and /foo/bar/lib directories. This overrides CURL_CONFIG, -# but is less robust. +# /foo/bar/include and /foo/bar/lib directories. This overrides +# CURL_CONFIG, but is less robust. If not set, and CURL_CONFIG is not set, +# uses -lcurl with no additional library detection (other than +# NEEDS_*_WITH_CURL). This is wrong, no? With CURL_CONFIG not set, it currently *does* run curl-config, see below. # # Define NO_EXPAT if you do not have expat installed. git-http-push is # not built, and you cannot push using http:// and https:// transports (dumb). @@ -1127,9 +1130,27 @@ ifdef NO_CURL REMOTE_CURL_NAMES = else ifdef CURLDIR - # Try -Wl,-rpath=$(CURLDIR)/$(lib) in such a case. - BASIC_CFLAGS += -I$(CURLDIR)/include - CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl + CURL_LIBCURL = + else + CURL_CONFIG = curl-config + ifeq $(CURL_CONFIG) + CURL_LIBCURL = + else + CURL_LIBCURL := $(shell $(CURL_CONFIG) --libs) + endif Doesn't that definition just define CURL_CONFIG unconditionally? How are the first condition ever supposed to get triggered? $ make make: curl-config: Command not found GIT_VERSION = 1.9.2.462.gf3f11fa make: curl-config: Command not found * new build flags * new link flags * new prefix flags GEN common-cmds.h ... Yuck. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Mon, Apr 28, 2014 at 6:29 PM, Erik Faye-Lund kusmab...@gmail.com wrote: MinGW builds of cURL does not ship with curl-config unless built with the autoconf based build system, which is not the practice recommended by the documentation. MsysGit has had issues with binaries of that sort, so it has switched away from autoconf-based cURL-builds. Unfortunately, broke pushing over WebDAV on Windows, because http-push.c depends on cURL's multi-threaded API, which we could not determine the presence of any more. Since troublesome curl-versions are ancient, and not even present in RedHat 5, let's just assume cURL is capable instead of doing a non-robust check. Instead, add a check for curl_multi_init to our configure-script, for those on ancient system. They probably already need to do the configure-dance anyway. Signed-off-by: Erik Faye-Lund kusmab...@gmail.com Junio, this patch still applies, and is required on Git for Windows for http-push to work, even after f3f11fa (Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: do not depend on curl-config
On Wed, Apr 30, 2014 at 5:27 PM, Junio C Hamano gits...@pobox.com wrote: Erik Faye-Lund kusmab...@gmail.com writes: MinGW builds of cURL does not ship with curl-config unless built with the autoconf based build system, which is not the practice recommended by the documentation. MsysGit has had issues with binaries of that sort, so it has switched away from autoconf-based cURL-builds. Unfortunately, broke pushing over WebDAV on Windows, because http-push.c depends on cURL's multi-threaded API, which we could not determine the presence of any more. Since troublesome curl-versions are ancient, and not even present in RedHat 5, let's just assume cURL is capable instead of doing a non-robust check. Instead, add a check for curl_multi_init to our configure-script, for those on ancient system. They probably already need to do the configure-dance anyway. Signed-off-by: Erik Faye-Lund kusmab...@gmail.com --- OK, here's a proper patch. I've even tested it! ;) Makefile | 8 +++- configure.ac | 11 +++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index e90f57e..f6b5847 100644 --- a/Makefile +++ b/Makefile @@ -1133,13 +1133,11 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) - curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) - ifeq $(curl_check) 070908 - ifndef NO_EXPAT + ifndef NO_EXPAT + ifndef NO_CURL_MULTI PROGRAM_OBJS += http-push.o endif Dave's ask curl-config about proper compilation options if available and this change does *not* semantically conflict, as the former should stress if available part (but note that the key word is should). How old/battle tested is this change? Not very. I've successfully tested it on two systems, msysGit and RedHat 5. My inclination at this point is to revert the merge of Dave's series from 2.0 (yes, I know we have been looking at fixing it and I _think_ the issue of unpleasant error message you reported can be fixed, but at this rate we would not know if we eradicated all the issues for platforms and distros with confidence by the final), kick it back to 'next'/'pu', and queue this change also in 'next'/'pu' and cook them so that we can merge them early in the 2.1 cycle. Sounds like a sensible plan to me. We can keep this patch in the msysGit repo for the 2.0 release. This new makefile variable needs a comment at the top, no? Makefile | 4 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index f7d33da..3164b62 100644 --- a/Makefile +++ b/Makefile @@ -34,6 +34,10 @@ all:: # git-http-push are not built, and you cannot use http:// and https:// # transports (neither smart nor dumb). # +# Define NO_CURL_MULTI if your libcurl is sufficiently old and lacks +# curl_multi_init (git http-push to run the deprecated dumb push over +# http/webdab will not be built). +# # Define CURL_CONFIG to the path to a curl-config binary other than the # default 'curl-config'. If CURL_CONFIG is unset or points to a binary that # is not found, defaults to the CURLDIR behavior. Ah yes. Sorry for missing that. Perhaps the text should even mention the old break-off version, that is curl v7.9.8 as far as I can tell...? -- To unsubscribe from this list: send the line 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 version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 9:36 AM, Marat Radchenko ma...@slonopotamus.org wrote: Silvola Tuomas wrote Hello, I installed git for windows 1.9.0 but any push operation I tried with it produced an error message saying git: 'http-push' is not a git command. Other commands like pull, add, and commit worked just fine. At the end of this day I noticed that C:\Program Files (x86)\Git\libexec\git-core just didn't have the file git-http-push. There were git-http-backend, git-http-fetch and git-imap-send and such but no git-http-push. I resolved my issue by uninstalling 1.9.0, installing an older version instead (1.8.1.2; this is when push started working) and 1.9.0 right on top of the older version. Now git push command works as expected. Br, Tuomas Silvola From Makefile: curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) ifeq $(curl_check) 070908 ifndef NO_EXPAT PROGRAM_OBJS += http-push.o endif endif if there's no curl-config, http-push.c is silently skipped. This check also doesn't play with cross-compiling when you cannot call curl-config because it is for other arch. There's also a mystic git-http-push$X that is not referenced from anywhere. We're using Curl 7.30.0 in msysGit (and thus also Git for Windows), so we should be able to include it. However, we do not have curl-config installed. Looking at 08900987 (Decide whether to build http-push in the Makefile), that commit is from 2005, so it seems we've broken something. Further, looking a bit at our curl build-script, we don't seem to to install curl-config. HOWEVER, 37e42ab (curl: update to 7.28.1 and enable ipv6), dated 1. Feb 2013 adds a function to remove curl-config. Pat, why is this? My knee-jerk suspicion would be that it's because it's a stale curl-config from a previous install (that *did* install it). However, it doesn't seem like the mingw32 Makefile (the one you get without running configure, it seems) even tries to build curl-config. In fact, it seems this is simply built by configure itself. Which we don't run, again since 37e42ab (curl: update to 7.28.1 and enable ipv6). So it seems that 08900987 (Decide whether to build http-push in the Makefile) makes a bad assumption about the availability of curl-config on new libcurl installations; it's not present on stock Windows builds. -- To unsubscribe from this list: send the line 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 version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 10:48 AM, Erik Faye-Lund kusmab...@gmail.com wrote: So it seems that 08900987 (Decide whether to build http-push in the Makefile) makes a bad assumption about the availability of curl-config on new libcurl installations; it's not present on stock Windows builds. I wonder, though. That check is over 8 years old. Are that old systems (that haven't been upgraded) still able to build Git? Even my old RedHat 5 setup has curl 7.15.5... Perhaps the following is the right thing to do? If not, perhaps we could move this complication to configure.ac, which could get the version number from the header-file instead? That way, quirks only affect quirky systems... (white-space damaged, I'll post a proper patch if there's some agreement) --- diff --git a/Makefile b/Makefile index 29a555d..6da72e7 100644 --- a/Makefile +++ b/Makefile @@ -1133,13 +1133,8 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) -curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) -ifeq $(curl_check) 070908 -ifndef NO_EXPAT -PROGRAM_OBJS += http-push.o -endif -endif ifndef NO_EXPAT +PROGRAM_OBJS += http-push.o ifdef EXPATDIR BASIC_CFLAGS += -I$(EXPATDIR)/include EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat -- To unsubscribe from this list: send the line 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] Sleep 1 millisecond in poll() to avoid busy wait
On Mon, Apr 28, 2014 at 10:39 AM, Stepan Kasal ka...@ucw.cz wrote: From: theoleblond theodore.lebl...@gmail.com Date: Wed, 16 May 2012 06:52:49 -0700 I played around with this quite a bit. After trying some more complex schemes, I found that what worked best is to just sleep 1 millisecond between iterations. Though it's a very short time, it still completely eliminates the busy wait condition, without hurting perf. There code uses SleepEx(1, TRUE) to sleep. See this page for a good discussion of why that is better than calling SwitchToThread, which is what was used previously: http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1 Note that calling SleepEx(0, TRUE) does *not* solve the busy wait. The most striking case was when testing on a UNC share with a large repo, on a single CPU machine. Without the fix, it took 4 minutes 15 seconds, and with the fix it took just 1:08! I think it's because git-upload-pack's busy wait was eating the CPU away from the git process that's doing the real work. With multi-proc, the timing is not much different, but tons of CPU time is still wasted, which can be a killer on a server that needs to do bunch of other things. I also tested the very fast local case, and didn't see any measurable difference. On a big repo with 4500 files, the upload-pack took about 2 seconds with and without the fix. --- This is one of the patches that lives in msysGit, could it be accepted upstream? It modifies the Windows compat function only. compat/poll/poll.c comes from Gnulib, so it would be better to submit the patch there and update. -- To unsubscribe from this list: send the line 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 version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 11:01 AM, Erik Faye-Lund kusmab...@gmail.com wrote: On Mon, Apr 28, 2014 at 10:48 AM, Erik Faye-Lund kusmab...@gmail.com wrote: So it seems that 08900987 (Decide whether to build http-push in the Makefile) makes a bad assumption about the availability of curl-config on new libcurl installations; it's not present on stock Windows builds. I wonder, though. That check is over 8 years old. Are that old systems (that haven't been upgraded) still able to build Git? Even my old RedHat 5 setup has curl 7.15.5... Perhaps the following is the right thing to do? If not, perhaps we could move this complication to configure.ac, which could get the version number from the header-file instead? That way, quirks only affect quirky systems... And here's a stab at that. Not really tested, as I don't have an affected system, so it's probably broken somehow ;) But if someone want's to pick it up, at least there's a starting-point. --- diff --git a/Makefile b/Makefile index 29a555d..b94f830 100644 --- a/Makefile +++ b/Makefile @@ -1133,11 +1133,8 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) - curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) - ifeq $(curl_check) 070908 - ifndef NO_EXPAT - PROGRAM_OBJS += http-push.o - endif + ifndef NO_CAPABLE_CURL + PROGRAM_OBJS += http-push.o endif ifndef NO_EXPAT ifdef EXPATDIR diff --git a/configure.ac b/configure.ac index 2f43393..47991c0 100644 --- a/configure.ac +++ b/configure.ac @@ -513,6 +513,16 @@ AC_CHECK_LIB([curl], [curl_global_init], [NO_CURL=], [NO_CURL=YesPlease]) +AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([#include curlver.h], + [#if LIBCURL_VERSION_NUM 0x070908 +#error version too old +#endif + ])], + [NO_CAPABLE_CURL=YesPlease], + [NO_CAPABLE_CURL=]) +GIT_CONF_SUBST([NO_CAPABLE_CURL]) + GIT_UNSTASH_FLAGS($CURLDIR) GIT_CONF_SUBST([NO_CURL]) -- To unsubscribe from this list: send the line 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] poll/select: prevent busy-waiting
On Mon, Apr 28, 2014 at 1:42 PM, Stepan Kasal ka...@ucw.cz wrote:
From: Paolo Bonzini bonz...@gnu.org
Date: Mon, 21 May 2012 09:52:42 +0200
Backported from Gnulib.
2012-05-21 Paolo Bonzini bonz...@gnu.org
poll/select: prevent busy-waiting. SwitchToThread() only gives away
the rest of the current time slice to another thread in the current
process. So if the thread that feeds the file decscriptor we're
polling is not in the current process, we get busy-waiting.
* lib/poll.c: Use SleepEx(1, TRUE) instead of SwitchToThread().
Patch from Theodore Leblond.
* lib/select.c: Split polling out of the loop that sets the output
fd_sets. Check for zero result and loop if the wait timeout is
infinite.
Signed-off-by: Stepan Kasal ka...@ucw.cz
---
compat/poll/poll.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index 31163f2..a9b41d8 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -605,7 +605,7 @@ restart:
if (!rc timeout == INFTIM)
{
- SwitchToThread();
+ SleepEx (1, TRUE);
goto restart;
}
--
1.9.2.msysgit.0.158.g6070cee
Thanks for taking the effort!
Acked-by: Erik Faye-Lund kusmab...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 11:01 AM, Erik Faye-Lund kusmab...@gmail.com wrote: On Mon, Apr 28, 2014 at 10:48 AM, Erik Faye-Lund kusmab...@gmail.com wrote: So it seems that 08900987 (Decide whether to build http-push in the Makefile) makes a bad assumption about the availability of curl-config on new libcurl installations; it's not present on stock Windows builds. I wonder, though. That check is over 8 years old. Are that old systems (that haven't been upgraded) still able to build Git? Even my old RedHat 5 setup has curl 7.15.5... Perhaps the following is the right thing to do? If not, perhaps we could move this complication to configure.ac, which could get the version number from the header-file instead? That way, quirks only affect quirky systems... (white-space damaged, I'll post a proper patch if there's some agreement) --- diff --git a/Makefile b/Makefile index 29a555d..6da72e7 100644 --- a/Makefile +++ b/Makefile @@ -1133,13 +1133,8 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) -curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) -ifeq $(curl_check) 070908 -ifndef NO_EXPAT -PROGRAM_OBJS += http-push.o -endif -endif ifndef NO_EXPAT +PROGRAM_OBJS += http-push.o ifdef EXPATDIR BASIC_CFLAGS += -I$(EXPATDIR)/include EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat I've built an installer with this patch applied, and it seems to do the trick: https://dl.dropboxusercontent.com/u/1737924/Git-1.9.2-http-push.exe -- To unsubscribe from this list: send the line 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: git version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 3:20 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
Hi kusma,
On Mon, 28 Apr 2014, Erik Faye-Lund wrote:
On Mon, Apr 28, 2014 at 10:48 AM, Erik Faye-Lund kusmab...@gmail.com wrote:
So it seems that 08900987 (Decide whether to build http-push in the
Makefile) makes a bad assumption about the availability of
curl-config on new libcurl installations; it's not present on stock
Windows builds.
I wonder, though. That check is over 8 years old. Are that old systems
(that haven't been upgraded) still able to build Git? Even my old
RedHat 5 setup has curl 7.15.5...
The easiest way in my humble opinion would be to install a script like
this into /mingw/bin/:
-- snip --
#!/bin/sh
case $1 in
--vernum)
version=$(curl -V) || exit
version=$(echo ${version%% (*)*} | tr . \ )
eval printf %02d%02d%02d ${version#curl }
;;
--cflags)
;;
--libs)
echo -lcurl
;;
esac
-- snap --
That way, upstream Git does not have anything to change (and we avoid
discussing five versions of essentially the same patch :-P).
Huh? I think I only really proposed one patch...?
--
To unsubscribe from this list: send the line 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: git version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 3:47 PM, Marat Radchenko ma...@slonopotamus.org wrote: On Mon, Apr 28, 2014 at 03:20:46PM +0200, Johannes Schindelin wrote: That way, upstream Git does not have anything to change (and we avoid discussing five versions of essentially the same patch :-P). This bug isn't specific to msysGit but also affects all environments where curl-config is not available or cannot be run for some reason, for example during cross-compilation. True. I think the assumption simply was a bad one, and it has probably gone unnoticed for a while because pushing over WebDAV is not all that common. If anyone turns up a system with an old enough libcurl, they should probably consult curlver.h instead. And if so, they are probably on a system so crippled they need to run automake anyway. -- To unsubscribe from this list: send the line 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 02/12] MINGW: compat/bswap.h: include stdint.h
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: bswap.h uses uint32_t type which might not be defined. This patch adds direct include so bswap.h can be safely included. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- compat/bswap.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compat/bswap.h b/compat/bswap.h index 120c6c1..d170447 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -5,6 +5,8 @@ * operation. */ +#include stdint.h + /* * Default version that the compiler ought to optimize properly with * constant values. Hmm, what's the symptom this fixes? From what I can tell, bswap.h is included after stdint.h from git-compat-util.h anyway... -- To unsubscribe from this list: send the line 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 05/12] MINGW: git-compat-util.h: use inttypes.h for printf macros.
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote:
Also, pass -D__USE_MINGW_ANSI_STDIO=0 to select MSVCRT-compatible
macro definitions.
Signed-off-by: Marat Radchenko ma...@slonopotamus.org
---
compat/mingw.h| 2 --
compat/msvc.h | 3 +++
config.mak.uname | 3 ++-
git-compat-util.h | 11 ++-
4 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/compat/mingw.h b/compat/mingw.h
index 262b300..c502a22 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -342,8 +342,6 @@ static inline char *mingw_find_last_dir_sep(const char
*path)
}
#define find_last_dir_sep mingw_find_last_dir_sep
#define PATH_SEP ';'
-#define PRIuMAX I64u
-#define PRId64 I64d
void mingw_open_html(const char *path);
#define open_html mingw_open_html
diff --git a/compat/msvc.h b/compat/msvc.h
index 580bb55..cb41ce3 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -15,6 +15,9 @@
#define strtoull _strtoui64
#define strtoll _strtoi64
+#define PRIuMAX I64u
+#define PRId64 I64d
+
static __inline int strcasecmp (const char *s1, const char *s2)
{
int size1 = strlen(s1);
diff --git a/config.mak.uname b/config.mak.uname
index 6c2e6df..e5edae6 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -321,6 +321,7 @@ ifeq ($(uname_S),Windows)
NO_PREAD = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
NO_LIBGEN_H = YesPlease
+ NO_INTTYPES_H = UnfortunatelyYes
NO_POLL = YesPlease
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
@@ -502,7 +503,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_INET_NTOP = YesPlease
NO_POSIX_GOODIES = UnfortunatelyYes
DEFAULT_HELP_FORMAT = html
- COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI
-Icompat -Icompat/win32
+ COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -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 \
compat/win32/pthread.o compat/win32/syslog.o \
diff --git a/git-compat-util.h b/git-compat-util.h
index f6d3a46..aa57a04 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,6 +85,12 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
+#ifndef NO_INTTYPES_H
+#include inttypes.h
+#else
+#include stdint.h
+#endif
+
Just checking that I understand: Does this mean that we now require an
MSVC-version that has stdint.h? If so, I'm not against such a case.
IMO, the biggest benefit of using MSVC is not building on legacy
systems, but being able to use it's debugger. And for that purpose
it's probably OK to increase the required version.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/12] MINGW: compat/bswap.h: include stdint.h
On Mon, Apr 28, 2014 at 4:52 PM, Marat Radchenko ma...@slonopotamus.org wrote: On Mon, Apr 28, 2014 at 04:45:43PM +0200, Erik Faye-Lund wrote: bswap.h is included after stdint.h from git-compat-util.h anyway... That only becomes true after PATCH 05 when talking about MinGW. Will drop this one. Right, thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/12] MINGW: config.mak.uname: drop -DNOGDI
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: On MinGW-W64, MsgWaitForMultipleObjects is guarded with #ifndef NOGDI. Unfortunately, including wingdi.h brings in #define ERROR 0 which conflicts with several Git internal enums. So, #undef ERROR. Signed-off-by: Marat Radchenko ma...@slonopotamus.org --- config.mak.uname | 4 ++-- git-compat-util.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/config.mak.uname b/config.mak.uname index a376b32..4883fd5 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -363,7 +363,7 @@ ifeq ($(uname_S),Windows) COMPAT_OBJS = compat/msvc.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ compat/win32/dirent.o - COMPAT_CFLAGS = -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ + COMPAT_CFLAGS = -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\.exe\ BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib invalidcontinue.obj PTHREAD_LIBS = @@ -503,7 +503,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_INET_NTOP = YesPlease NO_POSIX_GOODIES = UnfortunatelyYes DEFAULT_HELP_FORMAT = html - COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -DNOGDI -Icompat -Icompat/win32 + COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -D__USE_MINGW_ACCESS -D_USE_32BIT_TIME_T -Icompat -Icompat/win32 COMPAT_CFLAGS += -DSTRIP_EXTENSION=\.exe\ COMPAT_OBJS += compat/mingw.o compat/winansi.o \ compat/win32/pthread.o compat/win32/syslog.o \ diff --git a/git-compat-util.h b/git-compat-util.h index aa57a04..48bf0f7 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -98,6 +98,8 @@ #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */ #include winsock2.h #include windows.h +/* wingdi.h defines ERROR=0, undef it to avoid conflicts */ +#undef ERROR #define GIT_WINDOWS_NATIVE #endif Perhaps it would be cleaner to just undef NOGDI in compat/poll/poll.c? That way we don't end up pulling in all kinds of unrelated GUI-stuff into places where they're not needed...? -- To unsubscribe from this list: send the line 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 07/12] MINGW: config.mak.uname: reorganize MINGW settings
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: HAVE_LIBCHARSET_H and NO_R_TO_GCC_LINKER are not specific to msysGit, they're general MinGW settings. Actually, HAVE_LIBCHARSET_H is. It's only present because we have libiconv installed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] MINGW: compat/mingw.h: do not attempt to redefine lseek on mingw-w64
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: mingw-w64 has lseek defined in io.h. msysGit has a declaration of it in io.h as well. But it's not a preprocessor-definition... Are you saying that it's a preprocessor-define in mingw-w64, that points to a 64-bit version? If so, looks good. -- To unsubscribe from this list: send the line 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 05/12] MINGW: git-compat-util.h: use inttypes.h for printf macros.
On Mon, Apr 28, 2014 at 5:00 PM, Marat Radchenko ma...@slonopotamus.org wrote: On Mon, Apr 28, 2014 at 04:53:52PM +0200, Erik Faye-Lund wrote: Just checking that I understand: Does this mean that we now require an MSVC-version that has stdint.h? If so, I'm not against such a case. IMO, the biggest benefit of using MSVC is not building on legacy systems, but being able to use it's debugger. And for that purpose it's probably OK to increase the required version. Ouch, that was not intentional. What minimal MSVC version is currently supported and who decides if it is OK to increase required one? I don't know what the oldest one anyone have ever used, but I don't think that matters. IMO, just noting it in the commit-message is good enough. Others might feel differently, though. stdint.h is available in VS10 and onwards. contrib/buildsystems/Generators/Vcproj.pm generates project files for VS9. -- To unsubscribe from this list: send the line 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 07/12] MINGW: config.mak.uname: reorganize MINGW settings
On Mon, Apr 28, 2014 at 5:04 PM, Marat Radchenko ma...@slonopotamus.org wrote: On Mon, Apr 28, 2014 at 04:58:11PM +0200, Erik Faye-Lund wrote: On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: HAVE_LIBCHARSET_H and NO_R_TO_GCC_LINKER are not specific to msysGit, they're general MinGW settings. Actually, HAVE_LIBCHARSET_H is. It's only present because we have libiconv installed. 1. What are other ways to provide iconv on MinGW? I'm not sure I understand. To set HAVE_LIBCHARSET_H, we need to have libcharset.h. MinGW doesn't supply by default to my knowledge, so we get it from iconv. The THIS_IS_MSYSGIT file is there for us to be able to pick the right defaults for msysGit, and us having libcharset is indeed a msysGit-detail. Not all iconv-flavors supply libcharset.h, so this tells a particularity about the one we have in msysGit. 2. One can still completely disable iconv with NO_ICONV=1 Sure. And it does seem like the current setup assumes that anyone building for MinGW has iconv. But perhaps that's a mistake? All in all, I think maybe the line between MinGW and msysGit is a bit blurry at the moment. On the other hand, I don't know of anyone else than Sebastian that builds outside of msysGit. To be honest, I think the whole THIS_IS_MSYSGIT-block should have stayed downstream. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/12] MINGW: config.mak.uname allow using CURL for non-msysGit builds
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: Also, fix `warning: passing argument 2 of 'mingw_main' from incompatible pointer type` in http-fetch.c and remote-curl.c. These seems completely unrelated, perhaps it should be split in two? -- To unsubscribe from this list: send the line 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 v1] Towards MinGW(-W64) cross-compilation
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: This patch series fixes building on modern MinGW and (32bit only yet) MinGW-W64. *Compilation* tested on: - MSVC (via WinGit environment) - msysGit environment - Linux cross-toolchain i686-pc-mingw32 (4.8.2) with mingw-runtime-3.20.2 - Linux cross-toolchain i686-w64-mingw32 (4.8.2) with mingw64-runtime-3.1.0 Stuff still required to make Git build with x86_64 MinGW-W64 toolchain: 1. Drop -D_USE_32BIT_TIME_T that was added in fa93bb to config.mak.uname because time_t cannot be 32bit on x86_64. I haven't yet figured out what should break if this define is removed (pointers are welcome) and why it was added in the first place. 2. Stop passing --large-address-aware to linker. I wonder if it does anything for 32bit MinGW builds. 3. Fix several places with mismatched pointer size casts. Building it from Gentoo Linux: MinGW: crossdev -t i686-pc-mingw32 ARCH=x86 emerge-i686-pc-mingw32 -u dev-libs/libiconv sys-libs/zlib net-misc/curl sys-devel/gettext expat cd git make CROSS_COMPILE=i686-pc-mingw32- CC=i686-pc-mingw32-gcc NO_OPENSSL=1 MINGW=1 CURLDIR=/usr/i686-pc-mingw32/usr MinGW-W64 (32 bit): crossdev -t i686-w64-mingw32 ARCH=x86 emerge-i686-w64-mingw32 -u dev-libs/libiconv sys-libs/zlib net-misc/curl sys-devel/gettext expat cd git make CROSS_COMPILE=i686-w64-mingw32- CC=i686-w64-mingw32-gcc NO_OPENSSL=1 MINGW=1 CURLDIR=/usr/i686-w64-mingw32/usr Debian/Ubuntu build instructions are WIP (xdeb is non-trivial at all). Apart from some minor nits, this looks good to me. Thanks a lot for spending the time, and I look forward to a second iteration! -- To unsubscribe from this list: send the line 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 06/12] MSVC: config.mak.uname: drop -D__USE_MINGW_ACCESS from compile definitions
On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: -D__USE_MINGW_ACCESS only affects MinGW and does nothing when MSVC is used. Seems reasonable in itself. But, doesn't this mean that our access are currently broken on MSVC? The comment about __USE_MINGW_ACCESS is: /* Old versions of MSVCRT access() just ignored X_OK, while the version shipped with Vista, returns an error code. This will restore the old behaviour */ This sounds like we should lift the access-fix up one abstraction, into Git. But wait a minute. In Git for Windows, we already wrapped up access for unicode-support (03a102ff - Win32: Unicode file name support (except dirent)), doing the exact same thing already. This patch isn't upstreamed yet, though. Sounds like there's some cleaning up left to do on our behalf :) This clean-up makes sense regardless, 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 v2] Sleep 1 millisecond in poll() to avoid busy wait
On Mon, Apr 28, 2014 at 5:35 PM, Stepan Kasal ka...@ucw.cz wrote: From: theoleblond theodore.lebl...@gmail.com Date: Wed, 16 May 2012 06:52:49 -0700 SwitchToThread() only gives away the rest of the current time slice to another thread in the current process. So if the thread that feeds the file decscriptor we're polling is not in the current process, we get busy-waiting. I played around with this quite a bit. After trying some more complex schemes, I found that what worked best is to just sleep 1 millisecond between iterations. Though it's a very short time, it still completely eliminates the busy wait condition, without hurting perf. There code uses SleepEx(1, TRUE) to sleep. See this page for a good discussion of why that is better than calling SwitchToThread, which is what was used previously: http://stackoverflow.com/questions/1383943/switchtothread-vs-sleep1 Note that calling SleepEx(0, TRUE) does *not* solve the busy wait. The most striking case was when testing on a UNC share with a large repo, on a single CPU machine. Without the fix, it took 4 minutes 15 seconds, and with the fix it took just 1:08! I think it's because git-upload-pack's busy wait was eating the CPU away from the git process that's doing the real work. With multi-proc, the timing is not much different, but tons of CPU time is still wasted, which can be a killer on a server that needs to do bunch of other things. I also tested the very fast local case, and didn't see any measurable difference. On a big repo with 4500 files, the upload-pack took about 2 seconds with and without the fix. --- On Mon, Apr 28, 2014 at 05:05:47PM +0200, Johannes Sixt wrote: [...] but I very much prefer the commit message of the earlier post. ... but Paolo had a nice short description of the issue there; I inserted that to the top of the earlier commit message. The latter diff (without the comment), gets us closer to gnulib's poll.c. Good stuff! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/12] MINGW: config.mak.uname allow using CURL for non-msysGit builds
On Mon, Apr 28, 2014 at 6:23 PM, Marat Radchenko ma...@slonopotamus.org wrote: On Mon, Apr 28, 2014 at 05:26:38PM +0200, Erik Faye-Lund wrote: On Mon, Apr 28, 2014 at 3:51 PM, Marat Radchenko ma...@slonopotamus.org wrote: Also, fix `warning: passing argument 2 of 'mingw_main' from incompatible pointer type` in http-fetch.c and remote-curl.c. These seems completely unrelated, perhaps it should be split in two? Okay, will split. Though there is a connection - until this patch, http-fetch.c and remote-curl.c never built for MinGW. Ahh, thanks for pointing that out. But yeah, I think splitting is still the right option. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] Makefile: do not depend on curl-config
MinGW builds of cURL does not ship with curl-config unless built with the autoconf based build system, which is not the practice recommended by the documentation. MsysGit has had issues with binaries of that sort, so it has switched away from autoconf-based cURL-builds. Unfortunately, broke pushing over WebDAV on Windows, because http-push.c depends on cURL's multi-threaded API, which we could not determine the presence of any more. Since troublesome curl-versions are ancient, and not even present in RedHat 5, let's just assume cURL is capable instead of doing a non-robust check. Instead, add a check for curl_multi_init to our configure-script, for those on ancient system. They probably already need to do the configure-dance anyway. Signed-off-by: Erik Faye-Lund kusmab...@gmail.com --- OK, here's a proper patch. I've even tested it! ;) Makefile | 8 +++- configure.ac | 11 +++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index e90f57e..f6b5847 100644 --- a/Makefile +++ b/Makefile @@ -1133,13 +1133,11 @@ else REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) PROGRAM_OBJS += http-fetch.o PROGRAMS += $(REMOTE_CURL_NAMES) - curl_check := $(shell (echo 070908; curl-config --vernum) 2/dev/null | sort -r | sed -ne 2p) - ifeq $(curl_check) 070908 - ifndef NO_EXPAT + ifndef NO_EXPAT + ifndef NO_CURL_MULTI PROGRAM_OBJS += http-push.o endif - endif - ifndef NO_EXPAT + ifdef EXPATDIR BASIC_CFLAGS += -I$(EXPATDIR)/include EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat diff --git a/configure.ac b/configure.ac index 2f43393..e7ef9f7 100644 --- a/configure.ac +++ b/configure.ac @@ -513,6 +513,17 @@ AC_CHECK_LIB([curl], [curl_global_init], [NO_CURL=], [NO_CURL=YesPlease]) +if test -z $NO_CURL; then + +AC_CHECK_DECLS([curl_multi_init], +[NO_CURL_MULTI=], +[NO_CURL_MULTI=UnfortunatelyYes], +[[#include curl/curl.h]]) + +GIT_CONF_SUBST([NO_CURL_MULTI]) + +fi + GIT_UNSTASH_FLAGS($CURLDIR) GIT_CONF_SUBST([NO_CURL]) -- 1.9.2.msysgit.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git version 1.9.0 missing git-http-push?
On Mon, Apr 28, 2014 at 8:39 PM, Dave Borowitz dborow...@google.com wrote: On Mon, Apr 28, 2014 at 11:31 AM, Junio C Hamano gits...@pobox.com wrote: Erik Faye-Lund kusmab...@gmail.com writes: We're using Curl 7.30.0 in msysGit (and thus also Git for Windows), so we should be able to include it. However, we do not have curl-config installed. Hmmm, between 2.0-rc0 and 2.0-rc1 there is 61a64fff (Makefile: use curl-config to determine curl flags, 2014-04-15) merged already, which makes this assumption and a claim based on that assumption: curl-config should always be installed alongside a curl distribution, and its purpose is to provide flags for building against libcurl, so use it instead of guessing flags and dependent libraries. which may make things worse for you guys, unless you are explicitly setting CURLDIR and other Makefile variables. Yes, I can see that assumption may not always hold. But I'm slightly surprised this worked in the first place; are you saying this is a system where: -curl-config is not installed -but -lcurl still works -without setting CURLDIR ? Yes, our cURL is installed globally together with the system libraries. I think I should probably re-roll the patch to default to the old behavior (blind -lcurl) if curl-config returns the empty string, which I believe is also the case when the binary is not found. That sounds like it would work for MsysGit, yeah. -- To unsubscribe from this list: send the line 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] send-email: recognize absolute path on Windows
On Tue, Apr 22, 2014 at 6:50 PM, Junio C Hamano gits...@pobox.com wrote:
Erik Faye-Lund kusmab...@gmail.com writes:
Shouldn't the latter also be anchored at the beginning of the string
with a leading ^?
+}
+
+require File::Spec::Functions;
+return File::Spec::Functions::file_name_is_absolute($path);
We already use File::Spec qw(something else) at the beginning, no?
Why not throw file_name_is_absolute into that qw() instead?
Ahh, OK, if you did so, you won't have any place to hook the only
on msys do this trick into.
It somehow feels somewhat confusing that we define a sub with the
same name as the system one, while not overriding it entirely but
delegate back to the system one. I am debating myself if it is more
obvious if it is done this way:
use File::Spec::Functions qw(file_name_is_absolute);
if ($^O eq 'msys') {
sub file_name_is_absolute {
return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
}
}
In this case, we end up requiring that module even when we end up
using it, no?
Also somebody earlier mentioned that we would be redefining, which
has a different kind of ugliness, so I'd agree with the code structure
of what you sent out (which has been queued on 'pu').
My earlier question don't we want to make sure 'C:' is at the
betginning of the string? still stands, though. I do not think I
futzed with your regexp in the version I queued on 'pu'.
Ah, yes of course. Thanks for spotting that. I also like the other
clean-ups you did to the regex (above).
--
To unsubscribe from this list: send the line 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] send-email: recognize absolute path on Windows
On Wed, Apr 16, 2014 at 7:19 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Erik Faye-Lund kusmab...@gmail.com writes:
So let's manually check for these in that case, and fall back to
the File::Spec-helper on other platforms (e.g Win32 with native
Perl)
...
+sub file_name_is_absolute {
+my ($path) = @_;
+
+# msys does not grok DOS drive-prefixes
+if ($^O eq 'msys') {
+return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
Shouldn't the latter also be anchored at the beginning of the string
with a leading ^?
+}
+
+require File::Spec::Functions;
+return File::Spec::Functions::file_name_is_absolute($path);
We already use File::Spec qw(something else) at the beginning, no?
Why not throw file_name_is_absolute into that qw() instead?
Ahh, OK, if you did so, you won't have any place to hook the only
on msys do this trick into.
It somehow feels somewhat confusing that we define a sub with the
same name as the system one, while not overriding it entirely but
delegate back to the system one. I am debating myself if it is more
obvious if it is done this way:
use File::Spec::Functions qw(file_name_is_absolute);
if ($^O eq 'msys') {
sub file_name_is_absolute {
return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
}
}
In this case, we end up requiring that module even when we end up
using it, no? Not that I have very strong objections for doing just
that, after all, it appears to be built-in. (As you might understand
from this message, my perl-fu is really lacking :-P)
--
To unsubscribe from this list: send the line 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 v3] send-email: recognize absolute path on Windows
From: Erik Faye-Lund kusmab...@googlemail.com
On Windows, absolute paths might start with a DOS drive prefix,
which these two checks failed to recognize.
Unfortunately, we cannot simply use the file_name_is_absolute
helper in File::Spec::Functions, because Git for Windows has an
MSYS-based Perl, where this helper doesn't grok DOS
drive-prefixes.
So let's manually check for these in that case, and fall back to
the File::Spec-helper on other platforms (e.g Win32 with native
Perl)
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
So here's a version that does the old and long-time tested
approach without requiring breaking changes to msysGit's perl.
git-send-email.perl | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fdb0029..8f5f986 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1113,6 +1113,18 @@ sub ssl_verify_params {
}
}
+sub file_name_is_absolute {
+ my ($path) = @_;
+
+ # msys does not grok DOS drive-prefixes
+ if ($^O eq 'msys') {
+ return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
+ }
+
+ require File::Spec::Functions;
+ return File::Spec::Functions::file_name_is_absolute($path);
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
if ($dry_run) {
# We don't want to send the email.
- } elsif ($smtp_server =~ m#^/#) {
+ } elsif (file_name_is_absolute($smtp_server)) {
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
@@ -1271,7 +1283,7 @@ X-Mailer: git-send-email $gitversion
printf (($dry_run ? Dry- : ).Sent %s\n, $subject);
} else {
print (($dry_run ? Dry- : ).OK. Log says:\n);
- if ($smtp_server !~ m#^/#) {
+ if (!file_name_is_absolute($smtp_server)) {
print Server: $smtp_server\n;
print MAIL FROM:$raw_from\n;
foreach my $entry (@recipients) {
--
1.9.0.msysgit.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] contrib/credential/wincred/git-credential-wincred.c: reduce scope of variables
On Wed, Apr 16, 2014 at 11:33 AM, Elia Pinto gitter.spi...@gmail.com wrote:
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
contrib/credential/wincred/git-credential-wincred.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index a1d38f0..edff71c 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -258,11 +258,13 @@ static void read_credential(void)
int main(int argc, char *argv[])
{
- const char *usage =
- usage: git credential-wincred get|store|erase\n;
if (!argv[1])
+ {
+ const char *usage =
+ usage: git credential-wincred get|store|erase\n;
die(usage);
+ }
This is not the indentation/bracket-style we use in this source-file.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] xdiff/xprepare.c: reduce scope of variables
On Wed, Apr 16, 2014 at 11:33 AM, Elia Pinto gitter.spi...@gmail.com wrote:
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
xdiff/xprepare.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 63a22c6..e0b6987 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -161,8 +161,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t
*mf, long narec, xpparam_
xdlclassifier_t *cf, xdfile_t *xdf) {
unsigned int hbits;
long nrec, hsize, bsize;
- unsigned long hav;
- char const *blk, *cur, *top, *prev;
+ char const *blk, *cur, *prev;
xrecord_t *crec;
xrecord_t **recs, **rrecs;
xrecord_t **rhash;
@@ -193,7 +192,9 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t
*mf, long narec, xpparam_
nrec = 0;
if ((cur = blk = xdl_mmfile_first(mf, bsize)) != NULL) {
+char const *top;
for (top = blk + bsize; cur top; ) {
+unsigned long hav;
prev = cur;
We do not indent with spaces.
--
To unsubscribe from this list: send the line 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] send-email: recognize absolute path on Windows
From: Erik Faye-Lund kusmab...@googlemail.com
On Windows, absolute paths might start with a DOS drive prefix,
which this check fails to recognize.
Unfortunately, we cannot simply use the file_name_is_absolute
helper in File::Spec::Functions, because Git for Windows has an
MSYS-based Perl, where this helper doesn't grok DOS
drive-prefixes.
So let's manually check for these in that case, and fall back to
the File::Spec-helper on other platforms (e.g Win32 with native
Perl)
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
Here's a patch that we've been running with a variation of in
Git for Windows for a while. That version wasn't quite palatable,
as it recognized DOS drive-prefixes on all platforms.
git-send-email.perl | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fdb0029..c4d85a7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1113,6 +1113,18 @@ sub ssl_verify_params {
}
}
+sub file_name_is_absolute {
+ my ($path) = @_;
+
+ # msys does not grok DOS drive-prefixes
+ if ($^O eq 'msys') {
+ return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
+ }
+
+ require File::Spec::Functions;
+ return File::Spec::Functions::file_name_is_absolute($path);
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
if ($dry_run) {
# We don't want to send the email.
- } elsif ($smtp_server =~ m#^/#) {
+ } elsif (file_name_is_absolute($smtp_server)) {
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
--
1.9.0.msysgit.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: recognize absolute path on Windows
On Tue, Apr 15, 2014 at 12:32 PM, Johannes Sixt j.s...@viscovery.net wrote:
Am 4/15/2014 10:44, schrieb Erik Faye-Lund:
From: Erik Faye-Lund kusmab...@googlemail.com
On Windows, absolute paths might start with a DOS drive prefix,
which this check fails to recognize.
Unfortunately, we cannot simply use the file_name_is_absolute
helper in File::Spec::Functions, because Git for Windows has an
MSYS-based Perl, where this helper doesn't grok DOS
drive-prefixes.
So let's manually check for these in that case, and fall back to
the File::Spec-helper on other platforms (e.g Win32 with native
Perl)
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
Here's a patch that we've been running with a variation of in
Git for Windows for a while. That version wasn't quite palatable,
as it recognized DOS drive-prefixes on all platforms.
Did you consider patching msysgit's lib/perl5/5.8.8/File/Spec.pm by
inserting a line msys = 'Win32', near the top of the file; it is the
hash table that decides which path style is selected depending on $^O.
Then File::Spec-file_name_is_absolute($path) could be used without a wrapper.
I did not, but that works, and is IMO much nicer. Thanks for the idea!
git-send-email.perl | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fdb0029..c4d85a7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1113,6 +1113,18 @@ sub ssl_verify_params {
}
}
+sub file_name_is_absolute {
+ my ($path) = @_;
+
+ # msys does not grok DOS drive-prefixes
+ if ($^O eq 'msys') {
+ return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
+ }
+
+ require File::Spec::Functions;
+ return File::Spec::Functions::file_name_is_absolute($path);
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
if ($dry_run) {
# We don't want to send the email.
- } elsif ($smtp_server =~ m#^/#) {
+ } elsif (file_name_is_absolute($smtp_server)) {
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
There's another instance in the non-$quiet code path around line 1275 that
needs the same treatment.
Good catch, thanks!
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-email: recognize absolute path on Windows
On Tue, Apr 15, 2014 at 12:42 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Tue, Apr 15, 2014 at 12:32 PM, Johannes Sixt j.s...@viscovery.net wrote: Am 4/15/2014 10:44, schrieb Erik Faye-Lund: From: Erik Faye-Lund kusmab...@googlemail.com On Windows, absolute paths might start with a DOS drive prefix, which this check fails to recognize. Unfortunately, we cannot simply use the file_name_is_absolute helper in File::Spec::Functions, because Git for Windows has an MSYS-based Perl, where this helper doesn't grok DOS drive-prefixes. So let's manually check for these in that case, and fall back to the File::Spec-helper on other platforms (e.g Win32 with native Perl) Signed-off-by: Erik Faye-Lund kusmab...@gmail.com --- Here's a patch that we've been running with a variation of in Git for Windows for a while. That version wasn't quite palatable, as it recognized DOS drive-prefixes on all platforms. Did you consider patching msysgit's lib/perl5/5.8.8/File/Spec.pm by inserting a line msys = 'Win32', near the top of the file; it is the hash table that decides which path style is selected depending on $^O. Then File::Spec-file_name_is_absolute($path) could be used without a wrapper. I did not, but that works, and is IMO much nicer. Thanks for the idea! Actually, after having tried that, other stuff starts to break... So back to the drawing-board. -- To unsubscribe from this list: send the line 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: Re* [PATCH] send-email: recognize absolute path on Windows
On Tue, Apr 15, 2014 at 10:37 PM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Thanks, both. I'd expect another round then?
-- 8 --
From: Erik Faye-Lund kusmab...@googlemail.com
On Windows, absolute paths might start with a DOS drive prefix,
which these checks fail to recognize.
Use file_name_is_absolute from File::Spec::Functions for
portability. The Perl module msysgit has been shipping needs to be
updated for this patch to work, though.
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Helepd-by: Johannes Sixt j...@kdbg.org
---
git-send-email.perl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fdb0029..eda3917 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -25,7 +25,7 @@
use Data::Dumper;
use Term::ANSIColor;
use File::Temp qw/ tempdir tempfile /;
-use File::Spec::Functions qw(catfile);
+use File::Spec::Functions qw(catfile file_name_is_absolute);
use Error qw(:try);
use Git;
@@ -1197,7 +1197,7 @@ sub send_message {
if ($dry_run) {
# We don't want to send the email.
- } elsif ($smtp_server =~ m#^/#) {
+ } elsif (file_name_is_absolute($smtp_server)) {
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
@@ -1271,7 +1271,7 @@ sub send_message {
printf (($dry_run ? Dry- : ).Sent %s\n, $subject);
} else {
print (($dry_run ? Dry- : ).OK. Log says:\n);
- if ($smtp_server !~ m#^/#) {
+ if (file_name_is_absolute($smtp_server)) {
Obviously this has to be !file_name_is_absolute($smtp_server) ;-)
Heh, yeah. Apart from that, your patch is identical to mine. But, ugh.
Modifying File::Spec into thinking msys is Win32 doesn't seems to
work, as I get other random path-errors in that case:
Error in tempdir() using \tmp\XX: Parent directory (\tmp) is
not a directory at /libexec/git-core/git-send-email line 554
--
To unsubscribe from this list: send the line 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: Our official home page and logo for the Git project
On Fri, Apr 11, 2014 at 9:25 PM, Junio C Hamano gits...@pobox.com wrote: The motion is about this: Outside people, like the party who approached us about putting our logo on their trinket, seem to associate that logo we see on git-scm.com today with our project, but we never officially said it was our logo (we did not endorse that git-scm.com is our official home page, either, for that matter). It is silly for us to have to say Ehh, that is a logo that was randomly done and slapped on git-scm.com which is not even our official home page, and the logo is licensed CC-BY by somebody else. Go talk to them., every time such a request comes. Please help us by letting us answer Yup, that is a logo (among others) that represents our project, and we are OK with you using it to help promote our project instead. That is what I meant by our official logo in the first message. So,... seconds? Seconded. -- To unsubscribe from this list: send the line 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] GitMinutes about Git for Windows
On Mon, Apr 14, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Dear friends of Git for Windows, it was very delightful to be on the show, hosted by Thomas Ferris Nicolaisen: http://episodes.gitminutes.com/2014/04/gitminutes-28-johannes-schindelin-on.html Really enjoyable, thanks! -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/19] Portable alloca for Git
On Wed, Apr 9, 2014 at 2:48 PM, Kirill Smelkov k...@mns.spb.ru wrote: On Thu, Mar 27, 2014 at 06:22:50PM +0400, Kirill Smelkov wrote: On Mon, Mar 24, 2014 at 02:47:24PM -0700, Junio C Hamano wrote: Kirill Smelkov k...@mns.spb.ru writes: On Fri, Feb 28, 2014 at 06:19:58PM +0100, Erik Faye-Lund wrote: On Fri, Feb 28, 2014 at 6:00 PM, Kirill Smelkov k...@mns.spb.ru wrote: ... In fact that would be maybe preferred, for maintainers to enable alloca with knowledge and testing, as one person can't have them all at hand. Yeah, you're probably right. Erik, the patch has been merged into pu today. Would you please follow-up with tested MINGW change? Sooo I lost track but this discussion seems to have petered out around here. I think the copy we have had for a while on 'pu' is basically sound, and can easily built on by platform folks by adding or removing the -DHAVE_ALLOCA_H from the Makefile. Yes, that is all correct - that version works and we can improve it in the future with platform-specific follow-up patches, if needed. Junio, thanks for merging this and other diff-tree patches to next. It so happened that I'm wrestling with MSysGit today, so please also find alloca-for-mingw patch attached below. Thanks, Kirill 8 Subject: [PATCH] mingw: activate alloca Both MSVC and MINGW have alloca(3) definitions in malloc.h, so by moving win32-compat alloca.h from compat/vcbuild/include/ to compat/win32/ , which is included by both MSVC and MINGW CFLAGS, we can make alloca() work on both those Windows environments. In MINGW, malloc.h has explicit check for GNUC and if it is so, defines alloca to __builtin_alloca, so it looks like we don't need to add any code to here-shipped alloca.h to get optimum performance. Compile-tested on Windows in MSysGit. Signed-off-by: Kirill Smelkov k...@mns.spb.ru Looks good to me! -- To unsubscribe from this list: send the line 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 color scheme
On Fri, Feb 28, 2014 at 11:38 PM, Robert Dailey rcdailey.li...@gmail.com wrote: Is there a way to change color scheme in msysgit without going through the Properties Colors settings? Reason I ask is because I share the same HOME directory and .bashrc file between msysgit and cygwin, and it'd be nice to use the same color scheme defined in the bashrc between both. Not really. The reason is that in cygwin, it's the terminal emulatorl that does the coloring, but this doesn't work for non-cygwin programs. So we're detecting if stdout points to a console, and setting win32-console attributes directly. If you're interested in the details, see compat/winansi.c. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/19] Portable alloca for Git
On Mon, Feb 24, 2014 at 5:21 PM, Kirill Smelkov k...@mns.spb.ru wrote: diff --git a/Makefile b/Makefile index dddaf4f..0334806 100644 --- a/Makefile +++ b/Makefile @@ -316,6 +321,7 @@ endif ifeq ($(uname_S),Windows) GIT_VERSION := $(GIT_VERSION).MSVC pathsep = ; + HAVE_ALLOCA_H = YesPlease NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease In MSVC, alloca is defined in in malloc.h, not alloca.h: http://msdn.microsoft.com/en-us/library/wb1s57t5.aspx In fact, it has no alloca.h at all. But we don't have malloca.h in mingw either, so creating a compat/win32/alloca.h that includes malloc.h is probably sufficient. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/19] Portable alloca for Git
On Fri, Feb 28, 2014 at 2:44 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Mon, Feb 24, 2014 at 5:21 PM, Kirill Smelkov k...@mns.spb.ru wrote: diff --git a/Makefile b/Makefile index dddaf4f..0334806 100644 --- a/Makefile +++ b/Makefile @@ -316,6 +321,7 @@ endif ifeq ($(uname_S),Windows) GIT_VERSION := $(GIT_VERSION).MSVC pathsep = ; + HAVE_ALLOCA_H = YesPlease NO_PREAD = YesPlease NEEDS_CRYPTO_WITH_SSL = YesPlease NO_LIBGEN_H = YesPlease In MSVC, alloca is defined in in malloc.h, not alloca.h: http://msdn.microsoft.com/en-us/library/wb1s57t5.aspx In fact, it has no alloca.h at all. But we don't have malloca.h in mingw either, so creating a compat/win32/alloca.h that includes malloc.h is probably sufficient. But we don't have alloca.h in mingw either, sorry. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
