Re: [PATCH v4 23/23] compat/mingw.h: Fix the MinGW and msvc builds
On Sat, Dec 21, 2013 at 3:00 PM, Jeff King p...@peff.net wrote: From: Ramsay Jones ram...@ramsay1.demon.co.uk Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk Signed-off-by: Junio C Hamano gits...@pobox.com Signed-off-by: Jeff King p...@peff.net --- compat/mingw.h | 1 + 1 file changed, 1 insertion(+) diff --git a/compat/mingw.h b/compat/mingw.h index 92cd728..8828ede 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -345,6 +345,7 @@ static inline char *mingw_find_last_dir_sep(const char *path) #define PATH_SEP ';' #define PRIuMAX I64u #define PRId64 I64d +#define PRIx64 I64x Please, move this before patch #8, and adjust the commit message. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Crash on context menu
On Thu, Dec 26, 2013 at 2:16 PM, George Bateman georgebatema...@gmail.com wrote: Win7 SP1; Git identifies itself as version 1.8.4-preview20130916. I copied the Git Bash shortcut from the start menu to the root of a Git repository (no remote part). I use the advanced context menu. For other files in the directory, it works fine, but hovering over the Git GUI option of the Bash shortcut's menu causes a hang and this error: Problem signature: Problem Event Name:APPCRASH Application Name:explorer.exe Application Version:6.1.7601.17567 Application Timestamp:4d672ee4 Fault Module Name:git_shell_ext64.dll Fault Module Version:0.1.0.0 Fault Module Timestamp:5236e807 Exception Code:c005 Exception Offset:5952 OS Version:6.1.7601.2.1.0.768.3 Locale ID:2057 Additional Information 1:e0e1 Additional Information 2:e0e1a815a0aa94764feb60e78ef36122 Additional Information 3:adad Additional Information 4:adad544d8612f37837c12844e48b8ca2 It seems odd that it occurs on hover, but nothing else - time or other menu items - trigger it. This is a problem with git-cheetah, and not git itself, so the question is probably better suited at the msysGit mailing list, as git-cheetah is mostly developed there. -- To unsubscribe from this list: send the line unsubscribe 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] Fwd: static initializers
On Mon, Jan 6, 2014 at 11:05 PM, Stefan Zager sza...@google.com wrote: Forwarding to msysgit for any guidance about win equivalents for PTHREAD_MUTEX_INITIALIZER or __attribute__((constructor)). As you've probably already guessed, PTHREAD_MUTEX_INITIALIZER isn't supported in our pthreads-emulator. I did send out a patch adding support for it a while ago, but it hasn't been heavily tested. __attribute__((constructor)) doesn't work on MSVC, which we also build with. -- To unsubscribe from this list: send the line unsubscribe 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] Fix safe_create_leading_directories() for Windows
On Thu, Jan 2, 2014 at 9:46 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Sebastian, On Thu, 2 Jan 2014, Sebastian Schuberth wrote: On 02.01.2014 18:33, Johannes Schindelin wrote: -- snip -- On Linux, we can get away with assuming that the directory separator is a forward slash, but that is wrong in general. For that purpose, the is_dir_sep() function was introduced a long time ago. By using it in safe_create_leading_directories(), we proof said function for use on platforms where the directory separator is different from Linux'. -- snap -- While I'd be fine with this, I do not think we really need it. I also would have been fine with your commit message. But I knew Junio wouldn't be. As you say, is_dir_sep() has been introduced a long time ago, so people should be aware of it, and it should also be immediately clear from the diff why using it is better than hard-coding '/'. That said, I see any further explanations on top of the commit message title is an added bonus, and as just a bonus a link to a pull request should be fine. You don't need to understand or appreciate the concept of pull requests in order to follow the link and read the text in there. Well, you and I both know how easy GitHub's pull request made things for us as well as for contributors. I really cannot thank Erik enough for bullying me into using and accepting them. Huh? I don't think you refer to me, because I really dislike them (and I always have IIRC). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Synchronize git-credential-wincred from msysgit
On Sun, Jan 12, 2014 at 12:59 PM, 乙酸鋰 ch3co...@gmail.com wrote: Hi, Please cherry pick from msysgit/git commit 3c8cbb4edc8f577940c52115c992d17575587f99 to synchronize git-credential-wincred This was the change they made half year ago. It's actually a two-patch series. Cover-letter: http://permalink.gmane.org/gmane.comp.version-control.msysgit/18626 Patches: http://article.gmane.org/gmane.comp.version-control.msysgit/18627 http://article.gmane.org/gmane.comp.version-control.msysgit/18628 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] prefer xwrite instead of write
Our xwrite wrapper already deals with a few potential hazards, and
are as such more robust. Prefer it instead of write to get the
robustness benefits everywhere.
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
With this patch, we don't call write directly any more; all calls
goes via the xwrite-wrapper.
builtin/merge.c| 2 +-
streaming.c| 2 +-
transport-helper.c | 5 ++---
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 4941a6c..9383c31 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct
commit_list *remotehead
sha1_to_hex(commit-object.sha1));
pretty_print_commit(ctx, commit, out);
}
- if (write(fd, out.buf, out.len) 0)
+ if (xwrite(fd, out.buf, out.len) 0)
die_errno(_(Writing SQUASH_MSG));
if (close(fd))
die_errno(_(Finishing SQUASH_MSG));
diff --git a/streaming.c b/streaming.c
index 9659f18..d7c9f32 100644
--- a/streaming.c
+++ b/streaming.c
@@ -538,7 +538,7 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1,
struct stream_filter *f
goto close_and_exit;
}
if (kept (lseek(fd, kept - 1, SEEK_CUR) == (off_t) -1 ||
-write(fd, , 1) != 1))
+xwrite(fd, , 1) != 1))
goto close_and_exit;
result = 0;
diff --git a/transport-helper.c b/transport-helper.c
index 2010674..bf64ad7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1129,9 +1129,8 @@ static int udt_do_write(struct unidirectional_transfer *t)
return 0; /* Nothing to write. */
transfer_debug(%s is writable, t-dest_name);
- bytes = write(t-dest, t-buf, t-bufuse);
- if (bytes 0 errno != EWOULDBLOCK errno != EAGAIN
- errno != EINTR) {
+ bytes = xwrite(t-dest, t-buf, t-bufuse);
+ if (bytes 0 errno != EWOULDBLOCK) {
error(write(%s) failed: %s, t-dest_name, strerror(errno));
return -1;
} else if (bytes 0) {
--
1.8.4.msysgit.0
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mingw: remove mingw_write
Since 0b6806b9 (xread, xwrite: limit size of IO to 8MB), this
wrapper is no longer needed, as read and write are already split
into small chunks.
Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
---
compat/mingw.c | 17 -
compat/mingw.h | 3 ---
2 files changed, 20 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index fecb98b..e9892f8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -304,23 +304,6 @@ int mingw_open (const char *filename, int oflags, ...)
return fd;
}
-#undef write
-ssize_t mingw_write(int fd, const void *buf, size_t count)
-{
- /*
-* While write() calls to a file on a local disk are translated
-* into WriteFile() calls with a maximum size of 64KB on Windows
-* XP and 256KB on Vista, no such cap is placed on writes to
-* files over the network on Windows XP. Unfortunately, there
-* seems to be a limit of 32MB-28KB on X64 and 64MB-32KB on x86;
-* bigger writes fail on Windows XP.
-* So we cap to a nice 31MB here to avoid write failures over
-* the net without changing the number of WriteFile() calls in
-* the local case.
-*/
- return write(fd, buf, min(count, 31 * 1024 * 1024));
-}
-
static BOOL WINAPI ctrl_ignore(DWORD type)
{
return TRUE;
diff --git a/compat/mingw.h b/compat/mingw.h
index 92cd728..e033e72 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -180,9 +180,6 @@ int mingw_rmdir(const char *path);
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
-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
--
1.8.4.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 1/2] prefer xwrite instead of write
On Fri, Jan 17, 2014 at 8:07 PM, Junio C Hamano gits...@pobox.com wrote: Jonathan Nieder jrnie...@gmail.com writes: Hi, Erik Faye-Lund wrote: --- a/builtin/merge.c +++ b/builtin/merge.c @@ -367,7 +367,7 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead sha1_to_hex(commit-object.sha1)); pretty_print_commit(ctx, commit, out); } -if (write(fd, out.buf, out.len) 0) +if (xwrite(fd, out.buf, out.len) 0) die_errno(_(Writing SQUASH_MSG)); Shouldn't this use write_in_full() to avoid a silently truncated result? (*) Meaning this? If so, I think it makes sense. Yeah, I think that's better. Thanks, both! -- To unsubscribe from this list: send the line unsubscribe 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] Consecutive git gc fails on Windows network share
On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On Mon, 20 Jan 2014, Torsten Bögershausen wrote: b) add +++ at the places where you added the stat() and chmod(), c) and to send the question is this a good implementation ? to upstream git. I think your implementation makes sense. As I said in my other reply, I think that the problem would be addressed more generally in compat/mingw.c. It is to be doubted highly that upstream wants to handle cases such as rename() cannot overwrite read-only files on Windows everywhere they call rename() because the platforms upstream cares about do not have that problem. I'm not so sure. A quick test shows me that this is not the case for NTFS. Since this is over a network-share, the problem is probably server-side, and can affect other systems as well. So a work-around might be appropriate for all systems, 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: [msysGit] Consecutive git gc fails on Windows network share
On Tue, Jan 21, 2014 at 5:57 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
Hi kusma,
On Tue, 21 Jan 2014, Erik Faye-Lund wrote:
On Tue, Jan 21, 2014 at 12:25 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
On Mon, 20 Jan 2014, Torsten Bögershausen wrote:
b) add +++ at the places where you added the stat() and chmod(),
c) and to send the question is this a good implementation ? to upstream
git.
I think your implementation makes sense.
As I said in my other reply, I think that the problem would be
addressed more generally in compat/mingw.c. It is to be doubted highly
that upstream wants to handle cases such as rename() cannot overwrite
read-only files on Windows everywhere they call rename() because the
platforms upstream cares about do not have that problem.
I'm not so sure. A quick test shows me that this is not the case for
NTFS. Since this is over a network-share, the problem is probably
server-side, and can affect other systems as well.
So a work-around might be appropriate for all systems, no?
I do not think that the problem occurs if you run the same commands on
Linux, on a mounted Samba share. So I guess that upstream Git can enjoy
their luxury of not having to care.
In any case, if we would need this also for Linux, doing it for only one
user of rename() would probably not be good enough, either... so something
similar to mingw_rename() would be needed (interfering with mingw_rename
itself, of course).
Indeed. I was thinking of something along the lines of adding a
xrename in wrapper.c.
But you're probably right that this doesn't happen under Samba; surely
Samba would have added a work-around for such a filesystem by now. So
yeah, you've convinced me. mingw_rename is probably the place to do
that.
The work-around would probably look something like this:
diff --git a/compat/mingw.c b/compat/mingw.c
index a37bbf3..580b820 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1697,6 +1697,14 @@ int mingw_rename(const char *pold, const char *pnew)
*/
if (!_wrename(wpold, wpnew))
return 0;
+
+if (errno == EPERM) {
+/* read-only files cannot be moved over on network shares */
+_wchmod(wpnew, 0666);
+if (!_wrename(wpold, wpnew))
+return 0;
+}
+
if (errno != EEXIST)
return -1;
repeat:
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Feature Request Google Authenticator Support
On Thu, Jan 30, 2014 at 5:07 AM, Max Rahm ac90b...@gmail.com wrote: Github supports google authenticator 2-step authentication. I enabled it and how can't figure out how to connect to my github account through git. I've looked pretty hard in the man pages and on google and can't seem to find anything on how to set up git to work with a repository with 2-step verification. Here's a link to my stackoverflow question with my exact problem if there's something I'm missing. http://stackoverflow.com/questions/21447137/git-github-not-working-with-google-authenticator-osx As far as I can tell the feature is not supported. I'd like to be able to use the 2-step authentication but obviously I'd like to be able to push my code :D This sounds like a question for the GitHub support rather than the Git community. -- To unsubscribe from this list: send the line unsubscribe 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] gc: config option for running --auto in background
On Sat, Feb 8, 2014 at 8:08 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
`gc --auto` takes time and can block the user temporarily (but not any
less annoyingly). Make it run in background on systems that support
it. The only thing lost with running in background is printouts. But
gc output is not really interesting. You can keep it in foreground by
changing gc.autodetach.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/config.txt | 4
builtin/gc.c | 23 ++-
t/t5400-send-pack.sh | 1 +
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..4781773 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1167,6 +1167,10 @@ gc.autopacklimit::
--auto` consolidates them into one larger pack. The
default value is 50. Setting this to 0 disables it.
+gc.autodetach::
+ Make `git gc --auto` return immediately andrun in background
+ if the system supports it. Default is true.
+
gc.packrefs::
Running `git pack-refs` in a repository renders it
unclonable by Git versions prior to 1.5.1.2 over dumb
diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d..ed5cc3c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -29,6 +29,7 @@ static int pack_refs = 1;
static int aggressive_window = 250;
static int gc_auto_threshold = 6700;
static int gc_auto_pack_limit = 50;
+static int detach_auto = 1;
static const char *prune_expire = 2.weeks.ago;
static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
@@ -73,6 +74,10 @@ static int gc_config(const char *var, const char *value,
void *cb)
gc_auto_pack_limit = git_config_int(var, value);
return 0;
}
+ if (!strcmp(var, gc.autodetach)) {
+ detach_auto = git_config_bool(var, value);
+ return 0;
+ }
if (!strcmp(var, gc.pruneexpire)) {
if (value strcmp(value, now)) {
unsigned long now = approxidate(now);
@@ -301,11 +306,19 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
*/
if (!need_to_gc())
return 0;
- if (!quiet)
- fprintf(stderr,
- _(Auto packing the repository for
optimum performance. You may also\n
- run \git gc\ manually. See
- \git help gc\ for more
information.\n));
+ if (!quiet) {
+ if (detach_auto)
+ fprintf(stderr, _(Auto packing the
repository in background for optimum performance.\n));
+ else
+ fprintf(stderr, _(Auto packing the
repository for optimum performance.\n));
+ fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
+ }
+ if (detach_auto)
+ /*
+* failure to daemonize is ok, we'll continue
+* in foreground
+*/
+ daemonize();
While I agree that it should be OK, shouldn't we warn the user?
--
To unsubscribe from this list: send the line unsubscribe 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/2] daemon: move daemonize() to libgit.a
On Sat, Feb 8, 2014 at 8:08 AM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
diff --git a/setup.c b/setup.c
index 6c3f85f..b09a412 100644
--- a/setup.c
+++ b/setup.c
@@ -787,3 +787,27 @@ void sanitize_stdfds(void)
if (fd 2)
close(fd);
}
+
+int daemonize(void)
+{
+#ifdef NO_POSIX_GOODIES
+ errno = -ENOSYS;
+ return -1;
+#else
+ switch (fork()) {
+ case 0:
+ break;
+ case -1:
+ die_errno(fork failed);
+ default:
+ exit(0);
+ }
+ if (setsid() == -1)
+ die_errno(setsid failed);
+ close(0);
+ close(1);
+ close(2);
+ sanitize_stdfds();
+ return 0;
+#endif
+}
Nice change.
Just a nit: When I added the NO_POSIX_GOODIES-flag, Junio wanted the
implementations to be separate.
--
To unsubscribe from this list: send the line unsubscribe 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] gc: config option for running --auto in background
On Mon, Feb 10, 2014 at 2:17 PM, Duy Nguyen pclo...@gmail.com wrote:
On Mon, Feb 10, 2014 at 6:03 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
`gc --auto` takes time and can block the user temporarily (but not any
- if (!quiet)
- fprintf(stderr,
- _(Auto packing the repository for
optimum performance. You may also\n
- run \git gc\ manually. See
- \git help gc\ for more
information.\n));
+ if (!quiet) {
+ if (detach_auto)
+ fprintf(stderr, _(Auto packing the
repository in background for optimum performance.\n));
+ else
+ fprintf(stderr, _(Auto packing the
repository for optimum performance.\n));
+ fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
+ }
+ if (detach_auto)
+ /*
+* failure to daemonize is ok, we'll continue
+* in foreground
+*/
+ daemonize();
While I agree that it should be OK, shouldn't we warn the user?
If --quiet is set, we should not be printing anyway. If not, I thinkg
we could only print auto packing in background.. when we actually
can do that, else just print the old message. It means an #ifdef
NO_POSIX_GOODIES here again though..
Yuck, it's probably better to simply silently drop the detaching, I guess.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pack bitmap woes on Windows
On Wed, Feb 12, 2014 at 8:27 AM, Johannes Sixt j.s...@viscovery.net wrote:
Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with
the following symptoms. I haven't followed the topic. Have there been
patches floating that addressed the problem in one way or another?
(gdb) run
Starting program: D:\Src\mingw-git\t\trash
directory.t5310-pack-bitmaps/..\..\git.exe rev-list --test-bitmap HEAD
[New thread 3528.0x8d4]
Bitmap v1 test (20 entries loaded)
Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36
checksum
Breakpoint 1, die (err=0x5939e9 Out of memory, realloc failed) at usage.c:97
97 if (die_is_recursing()) {
(gdb) bt
#0 die (err=0x5939e9 Out of memory, realloc failed) at usage.c:97
#1 0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109
~800 megs is a pretty large allocation for 32-bit systems. What gives?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
On Wed, Feb 12, 2014 at 2:54 AM, Stefan Zager sza...@chromium.org wrote: We in the chromium project have a keen interest in adding threading to git in the pursuit of performance for lengthy operations (checkout, status, blame, ...). Our motivation comes from hitting some performance walls when working with repositories the size of chromium and blink: https://chromium.googlesource.com/chromium/src https://chromium.googlesource.com/chromium/blink We are particularly concerned with the performance of msysgit, and we have already chalked up a significant performance gain by turning on the threading code in pack-objects (which was already enabled for posix platforms, but not on msysgit, owing to the lack of a correct pread implementation). How did you manage to do this? I'm not aware of any way to implement pread on Windows (without going down the insanity-path of wrapping and potentially locking inside every IO operation)... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pack bitmap woes on Windows
On Wed, Feb 12, 2014 at 3:09 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup d...@gnu.org wrote: Johannes Sixt j.s...@viscovery.net writes: Am 2/12/2014 13:55, schrieb David Kastrup: Johannes Sixt j.s...@viscovery.net writes: Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with the following symptoms. I haven't followed the topic. Have there been patches floating that addressed the problem in one way or another? (gdb) run Starting program: D:\Src\mingw-git\t\trash directory.t5310-pack-bitmaps/..\..\git.exe rev-list --test-bitmap HEAD [New thread 3528.0x8d4] Bitmap v1 test (20 entries loaded) Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help? YES! t5310 passes after reverting this commit. Oh. I just looked through the backtrace until finding a routine reasonably related with the regtest and checked for the last commit changing it, then posted my question. Then I looked through the diff of the patch and considered it unconspicuous. So I commenced reading through earlier commits. I actually don't have a good idea what might be wrong here. The code is somewhat distasteful as it basically uses eword_t and uint64_t interchangeably, but then this does match its current definition. Perhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped? That is indeed the 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: pack bitmap woes on Windows
On Wed, Feb 12, 2014 at 3:22 PM, Erik Faye-Lund kusmab...@gmail.com wrote: On Wed, Feb 12, 2014 at 3:09 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup d...@gnu.org wrote: Johannes Sixt j.s...@viscovery.net writes: Am 2/12/2014 13:55, schrieb David Kastrup: Johannes Sixt j.s...@viscovery.net writes: Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with the following symptoms. I haven't followed the topic. Have there been patches floating that addressed the problem in one way or another? (gdb) run Starting program: D:\Src\mingw-git\t\trash directory.t5310-pack-bitmaps/..\..\git.exe rev-list --test-bitmap HEAD [New thread 3528.0x8d4] Bitmap v1 test (20 entries loaded) Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 checksum Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help? YES! t5310 passes after reverting this commit. Oh. I just looked through the backtrace until finding a routine reasonably related with the regtest and checked for the last commit changing it, then posted my question. Then I looked through the diff of the patch and considered it unconspicuous. So I commenced reading through earlier commits. I actually don't have a good idea what might be wrong here. The code is somewhat distasteful as it basically uses eword_t and uint64_t interchangeably, but then this does match its current definition. Perhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped? That is indeed the case. Looking a bit at it, the whole byte-order detection mess seems insane to me. MinGW falls inside the defined(__GNUC__) (defined(__i386__) || defined(__x86_64__))-block, but does not define __BYTE_ORDER. It seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2 assumes __BYTE_ORDER was guaranteed to always be defined, probably fooled by the following check: #if !defined(__BYTE_ORDER) # error Cannot determine endianness #endif However, that check is only performed if we're NOT building with GCC on x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either) The following would make __BYTE_ORDER a guarantee, and show that MinGW does not define it: ---8--- diff --git a/compat/bswap.h b/compat/bswap.h index 120c6c1..c73bf0e 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -109,10 +109,6 @@ static inline uint64_t git_bswap64(uint64_t x) # endif #endif -#if !defined(__BYTE_ORDER) -# error Cannot determine endianness -#endif - #if __BYTE_ORDER == __BIG_ENDIAN # define ntohll(n) (n) # define htonll(n) (n) @@ -123,6 +119,10 @@ static inline uint64_t git_bswap64(uint64_t x) #endif +#if !defined(__BYTE_ORDER) +# error Cannot determine endianness +#endif + /* * Performance might be improved if the CPU architecture is OK with * unaligned 32-bit loads and a fast ntohl() is available. ---8--- But another level of insanity (IMO) is defining double-underscore macros. These symbols are reserved for the implementation. Sure, knowing we're on a given implementation and defining something *else* dependent on them is fine. But defining them is just yuckiness, and not very standard-kosher. IMO, we should rather do the definition the other way around: #if !defined(BYTE_ORDER) # if defined(__BYTE_ORDER) defined(__LITTLE_ENDIAN) defined(__BIG_ENDIAN) # define BYTE_ORDER __BYTE_ORDER # define LITTLE_ENDIAN __LITTLE_ENDIAN # define BIG_ENDIAN __BIG_ENDIAN # endif #endif ...and only referred to BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN. That way we'd follow closer to where opengroup are heading: http://www.opengroup.org/austin/docs/austin_514.txt -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager sza...@google.com wrote: On Wed, Feb 12, 2014 at 3:59 AM, Erik Faye-Lund kusmab...@gmail.com wrote: On Wed, Feb 12, 2014 at 2:54 AM, Stefan Zager sza...@chromium.org wrote: We are particularly concerned with the performance of msysgit, and we have already chalked up a significant performance gain by turning on the threading code in pack-objects (which was already enabled for posix platforms, but not on msysgit, owing to the lack of a correct pread implementation). How did you manage to do this? I'm not aware of any way to implement pread on Windows (without going down the insanity-path of wrapping and potentially locking inside every IO operation)... I don't want to steal the thunder of my coworker, who wrote the implementation. He plans to submit it upstream soon-ish. It relies on using the lpOverlapped argument to ReadFile(), with some additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified. Is the code available somewhere? I'm especially interested in the additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified-part, as this was what I ended up butting my head into when trying to do this myself. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make the git codebase thread-safe
On Wed, Feb 12, 2014 at 7:34 PM, Stefan Zager sza...@google.com wrote: On Wed, Feb 12, 2014 at 10:27 AM, Erik Faye-Lund kusmab...@gmail.com wrote: On Wed, Feb 12, 2014 at 7:20 PM, Stefan Zager sza...@google.com wrote: I don't want to steal the thunder of my coworker, who wrote the implementation. He plans to submit it upstream soon-ish. It relies on using the lpOverlapped argument to ReadFile(), with some additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified. Is the code available somewhere? I'm especially interested in the additional tomfoolery to make sure that the implicit position pointer for the file descriptor doesn't get modified-part, as this was what I ended up butting my head into when trying to do this myself. https://chromium-review.googlesource.com/#/c/186104/ ReOpenFile, that's fantastic. Thanks a lot! -- To unsubscribe from this list: send the line unsubscribe 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] Git for Windows 1.9.0 (fwd)
On Tue, Feb 18, 2014 at 3:33 AM, Mike Hommey m...@glandium.org wrote: On Tue, Feb 18, 2014 at 12:52:28AM +0100, Johannes Schindelin wrote: Hopefully the Postfix Greylisting Policy Server will not try again to greylist me, as it might however do without violating the RFC. -- Forwarded message -- Date: Tue, 18 Feb 2014 00:38:54 +0100 (CET) From: Johannes Schindelin johannes.schinde...@gmx.de To: msys...@googlegroups.com Cc: git@vger.kernel.org Subject: [msysGit] Git for Windows 1.9.0 Dear Git fanbois, this announcement informs you that the small team of volunteers who keep the Git ship afloat for the most prevalent desktop operating system managed to release yet another version of Git for Windows: Git Release Notes (Git-1.9.0-preview20140217) Last update: 17 February 2013 Changes since Git-1.8.5.2-preview20131230 New Features - Comes with Git 1.9.0 plus Windows-specific patches. - Better work-arounds for Windows-specific path length limitations (pull request #122) - Uses optimized TortoiseGitPLink when detected (msysGit pull request #154) - Allow Windows users to use Linux Git on their files, using Vagrant http://www.vagrantup.com/ (msysGit pull request #159) I was curious about this, so i went to github and... couldn't find any pull request above #126. It's right here: https://github.com/msysgit/msysgit/pull/159 You probably looked in our git repo rather than our msysGit 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: [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
Re: [PATCH 17/19] Portable alloca for Git
On Fri, Feb 28, 2014 at 6:00 PM, Kirill Smelkov k...@mns.spb.ru wrote: On Fri, Feb 28, 2014 at 02:50:04PM +0100, Erik Faye-Lund wrote: 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. Don't we have that for MSVC already in compat/vcbuild/include/alloca.h and ifeq ($(uname_S),Windows) ... BASIC_CFLAGS = ... -Icompat/vcbuild/include ... in config.mak.uname ? Ah, of course. Thanks for setting me straight! And as I've not touched MINGW part in config.mak.uname the patch stays valid as it is :) and we can incrementally update what platforms have working alloca with follow-up patches. 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. -- To unsubscribe from this list: send the line unsubscribe 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 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: 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
[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
[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
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
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: 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 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: #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/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: [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 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 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 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: [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: [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: 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: 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: 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
