Git development
 help / color / mirror / Atom feed
* Re: [PATCH] config.txt: Describe special 'none' handling in core.gitProxy.
From: Junio C Hamano @ 2009-03-18  5:41 UTC (permalink / raw)
  To: Emil Sit; +Cc: git
In-Reply-To: <1237311102.24607.1305895377@webmail.messagingengine.com>

Thanks.

^ permalink raw reply

* Re: --exec-path not always honored
From: Junio C Hamano @ 2009-03-18  5:41 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <49BF692B.9020002@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> git-gc is a builtin. Should git setenv("GIT_EXEC_PATH") before it runs
> other git commands?

I think we just never have bothered about such a use case, but you are
right.  It probably is a good solution, although setenv makes me feel a
bit nervous for no rational reason.

^ permalink raw reply

* What's in git.git (Mar 2009, #05; Tue, 17)
From: Junio C Hamano @ 2009-03-18  5:41 UTC (permalink / raw)
  To: git

Notable topics graduated are Jay's "git remote" improvements, Kjetil's
"git checkout" optimization, and René's "git grep --color".

Post 1.6.2 cycle seems to be nicely progressing, and with another mass
graduation like this hopefully we can go into pre-release freeze for the
next release fairly soon.

* The 'master' branch has these since the last announcement
  in addition to what are already in v1.6.2.1.

Alex Riesen (1):
  disable post-checkout test on Cygwin

Benjamin Kramer (1):
  Fix various dead stores found by the clang static analyzer

Brian Gernhardt (2):
  Create USE_ST_TIMESPEC and turn it on for Darwin
  Makefile: Set compiler switch for USE_NSEC

Chris Johnsen (2):
  git-push.txt: describe how to default to pushing only current branch
  Documentation: remove extra quoting/emphasis around literal texts

Daniel Barkalow (1):
  Give error when no remote is configured

Emil Sit (1):
  config.txt: Describe special 'none' handling in core.gitProxy.

Jay Soffian (18):
  move duplicated get_local_heads() to remote.c
  move duplicated ref_newer() to remote.c
  move locate_head() to remote.c
  remote: simplify guess_remote_head()
  remote: make copy_ref() perform a deep copy
  remote: let guess_remote_head() optionally return all matches
  remote: make match_refs() copy src ref before assigning to peer_ref
  remote: make match_refs() not short-circuit
  string-list: new for_each_string_list() function
  builtin-remote: refactor duplicated cleanup code
  builtin-remote: remove unused code in get_ref_states
  builtin-remote: rename variables and eliminate redundant function call
  builtin-remote: make get_remote_ref_states() always populate
    states.tracked
  builtin-remote: fix two inconsistencies in the output of "show <remote>"
  builtin-remote: teach show to display remote HEAD
  builtin-remote: add set-head subcommand
  builtin-remote: new show output style
  builtin-remote: new show output style for push refspecs

Jeff King (5):
  test scripts: refactor start_httpd helper
  add basic http clone/fetch tests
  refactor find_ref_by_name() to accept const list
  remote: make guess_remote_head() use exact HEAD lookup if it is available
  ls-files: require worktree when --deleted is given

Johannes Schindelin (2):
  rsync transport: allow local paths, and fix tests
  winansi: support ESC [ K (erase in line)

Johannes Sixt (1):
  recv_sideband: Bands #2 and #3 always go to stderr

Junio C Hamano (6):
  builtin-remote.c: no "commented out" code, please
  Not all systems use st_[cm]tim field for ns resolution file timestamp
  grep: cast printf %.*s "precision" argument explicitly to int
  read-tree A B C: do not create a bogus index and do not segfault
  Remove total confusion from git-fetch and git-push
  Update draft release notes to 1.6.3

Kjetil Barvik (17):
  lstat_cache(): small cleanup and optimisation
  lstat_cache(): generalise longest_match_lstat_cache()
  lstat_cache(): swap func(length, string) into func(string, length)
  unlink_entry(): introduce schedule_dir_for_removal()
  create_directories(): remove some memcpy() and strchr() calls
  write_entry(): cleanup of some duplicated code
  write_entry(): use fstat() instead of lstat() when file is open
  show_patch_diff(): remove a call to fstat()
  lstat_cache(): print a warning if doing ping-pong between cache types
  check_updates(): effective removal of cache entries marked CE_REMOVE
  fix compile error when USE_NSEC is defined
  make USE_NSEC work as expected
  verify_uptodate(): add ce_uptodate(ce) test
  write_index(): update index_state->timestamp after flushing to disk
  Record ns-timestamps if possible, but do not use it without USE_NSEC
  checkout bugfix: use stat.mtime instead of stat.ctime in two places
  Revert "lstat_cache(): print a warning if doing ping-pong between cache
    types"

Michael J Gruber (1):
  git-branch.txt: document -f correctly

Miklos Vajna (1):
  Tests: use test_cmp instead of diff where possible

Nguyễn Thái Ngọc Duy (1):
  grep: prefer builtin over external one when coloring results

Petr Kodl (2):
  MinGW: a helper function that translates Win32 API error codes
  MinGW: a hardlink implementation

René Scharfe (6):
  grep: micro-optimize hit collection for AND nodes
  grep: remove grep_opt argument from match_expr_eval()
  grep: add pmatch and eflags arguments to match_one_pattern()
  grep: color patterns in output
  grep: add support for coloring with external greps
  pickaxe: count regex matches only once

Stephen Boyd (1):
  git-send-email.txt: describe --compose better

Thomas Rast (2):
  send-email: respect in-reply-to regardless of threading
  send-email: test --no-thread --in-reply-to combination

^ permalink raw reply

* Re: Ability to edit message from git rebase --interactive.
From: Sverre Rabbelier @ 2009-03-18  5:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Johannes Schindelin, Olivier Goffart, git
In-Reply-To: <7vsklbod0l.fsf@gitster.siamese.dyndns.org>

Heya,

On Wed, Mar 18, 2009 at 02:06, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
> I am not quite sure what rephrase is buying us.  Do we also want to
> introduce retree that allows you to muck with the tree object recorded
> without giving you a chance to clobber the commit log message?

Is that a common operation? Rephrase is, at least to me...

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* GitTogether '09
From: Christian Couder @ 2009-03-18  5:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20090310001613.GL11989@spearce.org>

Le mardi 10 mars 2009, Shawn O. Pearce a écrit :
> Folks, its that time of year again.  We need to apply to be a
> mentoring organization for Google Summer of Code 2009.

Maybe we could also start discussing GitTogether '09 location and date.

I have added a GitTogether '09 heading to the wiki page and a few location 
and date ideas:

http://git.or.cz/gitwiki/GitTogether

The location and dates are:

 * San Francisco in October, just after the Google Summer of Code Mentor

 * Portland Oregon in September, during or after LinuxCon 
(http://events.linuxfoundation.org/events/linuxcon) and/or Linux Plumbers 
Conference (http://linuxplumbersconf.org/)

Best regards,
Christian.

^ permalink raw reply

* Re: What's cooking in git.git (Mar 2009, #05; Tue, 17)
From: Jeff King @ 2009-03-18  5:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Miklos Vajna
In-Reply-To: <7vbprzo0si.fsf@gitster.siamese.dyndns.org>

On Tue, Mar 17, 2009 at 10:30:37PM -0700, Junio C Hamano wrote:

> * mv/parseopt-ls-files (Sat Mar 7 20:27:22 2009 -0500) 4 commits
>  + ls-files: fix broken --no-empty-directory
>  + t3000: use test_cmp instead of diff
>  + parse-opt: migrate builtin-ls-files.
>  + Turn the flags in struct dir_struct into a single variable
> 
> The tip one was a subject for further discussion, but nothing is queued
> yet.

I am inclined to leave it as-is. The other sane option would be
converting it to use NONEG, as Miklos suggested.

Doing it right would probably mean adding a "this option is the opposite
of what we would usually do" flag to parse-options which would display
the option as "no-<option>" in the usage, and would reverse clearing and
setting the bit (i.e., --empty-directory would clear the HIDE_DIRECTORY
bit and --no-empty-directory would set it). But I don't think it is
worth the work to add a negatable version of an option that has never
existed before and which nobody has requested to use.

-Peff

^ permalink raw reply

* Re: [PATCH1/2] Libify blame
From: pi song @ 2009-03-18  5:59 UTC (permalink / raw)
  To: Junio C Hamano, git; +Cc: rene.scharfe
In-Reply-To: <7vocvzmlqf.fsf@gitster.siamese.dyndns.org>

Don't you think we should rather split up into smaller files before
start reorganizing things?

Pi Song

On Wed, Mar 18, 2009 at 4:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> pi song <pi.songs@gmail.com> writes:
>
>> diff --git a/blame.h b/blame.h
>> new file mode 100644
>> index 0000000..72d1e2a
>> --- /dev/null
>> +++ b/blame.h
>> @@ -0,0 +1,166 @@
>> +/*
>> + * for storing stats. it can be used
>> + * across multiple blame operations
>> + */
>> +struct blame_stat {
>> +     int num_read_blob;
>> +     int num_get_patch;
>> +     int num_commits;
>> +};
>
> As I said in my previous message, I do not understand why this is not part
> of the super-scoreboard (now blame_info).
>
>> +#define PICKAXE_BLAME_MOVE           01
>> +#define PICKAXE_BLAME_COPY           02
>> +#define PICKAXE_BLAME_COPY_HARDER    04
>> +#define PICKAXE_BLAME_COPY_HARDEST   010
>> +
>> +#define BLAME_DEFAULT_MOVE_SCORE     20
>> +#define BLAME_DEFAULT_COPY_SCORE     40
>> +
>> +/* bits #0..7 in revision.h, #8..11 used for merge_bases() in commit.c */
>> +#define METAINFO_SHOWN               (1u<<12)
>> +#define MORE_THAN_ONE_PATH   (1u<<13)
>
> Do we need to expose all of these constants outside blame.c?  I think the
> library caller needs access to the first four above, but I tend to think
> the latter four are purely internal implementation detail that should be
> kept in blame.c.
>
>> +/* output formatting constants */
>> +#define OUTPUT_ANNOTATE_COMPAT  001
>> +#define OUTPUT_LONG_OBJECT_NAME 002
>> +#define OUTPUT_RAW_TIMESTAMP    004
>> +#define OUTPUT_PORCELAIN        010
>> +#define OUTPUT_SHOW_NAME        020
>> +#define OUTPUT_SHOW_NUMBER      040
>> +#define OUTPUT_SHOW_SCORE      0100
>> +#define OUTPUT_NO_AUTHOR       0200
>
> I think these can be public.
>
>> +/*
>> + * One blob in a commit that is being suspected
>> + */
>> +struct origin {
>> +     int refcnt;
>> +     struct origin *previous;
>> +     struct commit *commit;
>> +     mmfile_t file;
>> +     unsigned char blob_sha1[20];
>> +     char path[FLEX_ARRAY];
>> +};
>
> I somehow doubt we would want to expose this level of implementation
> detail to the callers of the library.  If we need to, the structure needs
> to be renamed---"origin" is way too generic a name.
>
>> +extern void assign_blame(struct blame_scoreboard *sb, int opt) ;
>
> Lose the extra SP before ";".  I had to fix them in your previous patch
> and there were many.
>

^ permalink raw reply

* Re: [PATCH1/2] Libify blame
From: Junio C Hamano @ 2009-03-18  6:20 UTC (permalink / raw)
  To: pi.songs; +Cc: git, rene.scharfe
In-Reply-To: <1b29507a0903172259t348cb4d5n70f5b3003b1eeb00@mail.gmail.com>

pi song <pi.songs@gmail.com> writes:

> Don't you think we should rather split up into smaller files before
> start reorganizing things?

Yes, but splitting it wrong is, eh, wrong ;-)

^ permalink raw reply

* Re: [EGIT PATCH 01/25] Use generics for collections in commit dialog and import page
From: Robin Rosenberg @ 2009-03-18  6:32 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20090318014537.GC23521@spearce.org>

onsdag 18 mars 2009 02:45:37 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > Subject: Re: [EGIT PATCH 01/25] Use generics for collections in commit
> ..............................^^
> 
> Are there 24 remaining patches in this series?  I only got this one.

Ah, sorry. There's only one that I want to submit.

-- robin

^ permalink raw reply

* Re: [JGIT PATCH 00/12] Improve test coverage in revwalk
From: Robin Rosenberg @ 2009-03-18  6:34 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1237340451-31562-1-git-send-email-spearce@spearce.org>

onsdag 18 mars 2009 02:40:39 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Most of these patches are to improve the test coverage within the
> revwalk package.
> 
> 
> The last commit points out what I feared, which is that a RevWalk
> with a PathFilter applied doesn't produce the same results that
> git-core would produce in the same situation.  We're either missing
> some functions necessary to implement it, or we flat out produce
> a wrong graph in some cases.
> 
> The tests are commented out because JUnit doesn't have a notion of
> "known broken".  But I did leave in TODO comments.  I'd like to
> apply the test, and then work later to improve it, but I'm open
> to suggestions.

If we'd switch to JUnit4/TestNG we could abuse the expected exception
annotation.

-- robin

^ permalink raw reply

* Re: GitTogether '09
From: Sverre Rabbelier @ 2009-03-18  6:46 UTC (permalink / raw)
  To: Christian Couder; +Cc: Shawn O. Pearce, git
In-Reply-To: <200903180651.33381.chriscool@tuxfamily.org>

Heya,

On Wed, Mar 18, 2009 at 06:51, Christian Couder <chriscool@tuxfamily.org> wrote:
>  * San Francisco in October, just after the Google Summer of Code Mentor

Considering that (assuming git is accepted as mentoring organization)
we can ask Google for 2 airplane tickets, I'm voting for this option
;).

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH1/2] Libify blame
From: pi song @ 2009-03-18  6:52 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <7v3adbmjwy.fsf@gitster.siamese.dyndns.org>

Wait. If you look at the builtin-blame.c, out of question it is very
messy. Things like print_usage() or -L parameter parsing for example
is not done upfront but hiding somewhere. Some functions are not very
clear if they are frontend or backend. I would say nobody would be
able to split it right in the first place. What you could do is to
split it to something "roughly right" and then work from that.

My latest two patches really do nothing but just splitting files. I
haven't changed any logics or renamed any thing only to make this big
beast more *manageable* rather than tackling the problem directly.
Yes, some bits are  still wrong but I believe 70% of the functions
should already stay in the right place. The following patches will
make the structure more right *gradually*.

Pi Song


On Wed, Mar 18, 2009 at 5:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> pi song <pi.songs@gmail.com> writes:
>
>> Don't you think we should rather split up into smaller files before
>> start reorganizing things?
>
> Yes, but splitting it wrong is, eh, wrong ;-)
>

^ permalink raw reply

* Re: [PATCH1/2] Libify blame
From: pi song @ 2009-03-18  7:01 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <1b29507a0903172352x7864911fm1104e22eddde54f1@mail.gmail.com>

BTW, following patches are not available yet : P

On Wed, Mar 18, 2009 at 5:52 PM, pi song <pi.songs@gmail.com> wrote:
> Wait. If you look at the builtin-blame.c, out of question it is very
> messy. Things like print_usage() or -L parameter parsing for example
> is not done upfront but hiding somewhere. Some functions are not very
> clear if they are frontend or backend. I would say nobody would be
> able to split it right in the first place. What you could do is to
> split it to something "roughly right" and then work from that.
>
> My latest two patches really do nothing but just splitting files. I
> haven't changed any logics or renamed any thing only to make this big
> beast more *manageable* rather than tackling the problem directly.
> Yes, some bits are  still wrong but I believe 70% of the functions
> should already stay in the right place. The following patches will
> make the structure more right *gradually*.
>
> Pi Song
>
>
> On Wed, Mar 18, 2009 at 5:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> pi song <pi.songs@gmail.com> writes:
>>
>>> Don't you think we should rather split up into smaller files before
>>> start reorganizing things?
>>
>> Yes, but splitting it wrong is, eh, wrong ;-)
>>
>

^ permalink raw reply

* Re: GitTogether '09
From: Christian Couder @ 2009-03-18  7:05 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Shawn O. Pearce, git, gittogether
In-Reply-To: <fabb9a1e0903172346j74b9992r1b0a8d6eb523103c@mail.gmail.com>

Le mercredi 18 mars 2009, Sverre Rabbelier a écrit :
> Heya,
>
> On Wed, Mar 18, 2009 at 06:51, Christian Couder <chriscool@tuxfamily.org> 
wrote:
> >  * San Francisco in October, just after the Google Summer of Code
> > Mentor
>
> Considering that (assuming git is accepted as mentoring organization)
> we can ask Google for 2 airplane tickets, I'm voting for this option
> ;).

I thought that it was only one international airplane ticket last year, 
except perhaps when there are no US based mentor. And we are not sure that 
Google or another company in the Bay Area will be able to host us this 
year. So the cost of hosting the GitTogether might be higher than the cost 
of a few airplane tickets.

Regards,
Christian.

^ permalink raw reply

* Re: [PATCH] Define a version of lstat(2) specially for copy operation
From: Alex Riesen @ 2009-03-18  7:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Sixt, Jeff King, layer, git
In-Reply-To: <20090317213820.GC13458@blimp.localdomain>

2009/3/17 Alex Riesen <raa.lkml@gmail.com>:
>> hooks copied from templates?  I think we could pass mode 0 to copy_files()
>> and have the function special case it (and allow a platform specific
>> copy_files() implemented by Cygwin).  lstat() in the copy_templates_1()
>> codepath is primarily done to see if we need to descend into a directory
>> or symlink() and our use of st.st_mode to pass to copy_files() is a no
>> cost side effect on platforms with x-bit support.
>
> And I don't think that the platform broken in so many ways deserves
> that kind of treatment. Maybe this patch is enough. Will test it
> tomorrow, when I get to mine so much hated Windows system.
>

Ok, I just tested it and it works. Junio, this patch can replace the previous
change which disabled the test.

^ permalink raw reply

* [PATCH] MinGW: implement mmap
From: Johannes Sixt @ 2009-03-18  7:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Janos Laube, Johannes Schindelin

From: Janos Laube <janos.dev@gmail.com>
Date: Fri, 13 Mar 2009 16:50:45 +0100

Add USE_WIN32_MMAP which triggers the use of windows' native
file memory mapping functionality in git_mmap()/git_munmap() functions.

As git functions currently use mmap with MAP_PRIVATE set only, this
implementation supports only that mode for now.

On Windows, offsets for memory mapped files need to match the allocation
granularity. Take this into account when calculating the packed git-
windowsize and file offsets. At the moment, the only function which makes
use of offsets in conjunction with mmap is use_pack() in sha1-file.c.

Git fast-import's code path tries to map a portion of the temporary
packfile that exceeds the current filesize, i.e. offset+length is
greater than the filesize. The NO_MMAP code worked with that since pread()
just reads the file content until EOF and returns gracefully, while
MapViewOfFile() aborts the mapping and returns 'Access Denied'.
Working around that by determining the filesize and adjusting the length
parameter.

Signed-off-by: Janos Laube <janos.dev@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
  Makefile           |    7 +++++-
  compat/mingw.h     |    5 ++++
  compat/win32mmap.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
  git-compat-util.h  |   12 ++++++++--
  4 files changed, 73 insertions(+), 4 deletions(-)
  create mode 100644 compat/win32mmap.c

diff --git a/Makefile b/Makefile
index 1087884..38e75e5 100644
--- a/Makefile
+++ b/Makefile
@@ -776,7 +776,6 @@ ifneq (,$(findstring CYGWIN,$(uname_S)))
  	COMPAT_OBJS += compat/cygwin.o
  endif
  ifneq (,$(findstring MINGW,$(uname_S)))
-	NO_MMAP = YesPlease
  	NO_PREAD = YesPlease
  	NO_OPENSSL = YesPlease
  	NO_CURL = YesPlease
@@ -799,6 +798,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
  	RUNTIME_PREFIX = YesPlease
  	NO_POSIX_ONLY_PROGRAMS = YesPlease
  	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+	USE_WIN32_MMAP = YesPlease
  	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
  	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
  	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
@@ -967,6 +967,11 @@ endif
  ifdef NO_MMAP
  	COMPAT_CFLAGS += -DNO_MMAP
  	COMPAT_OBJS += compat/mmap.o
+else
+	ifdef USE_WIN32_MMAP
+		COMPAT_CFLAGS += -DUSE_WIN32_MMAP
+		COMPAT_OBJS += compat/win32mmap.o
+	endif
  endif
  ifdef NO_PREAD
  	COMPAT_CFLAGS += -DNO_PREAD
diff --git a/compat/mingw.h b/compat/mingw.h
index 6e24686..f5da647 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -160,6 +160,11 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
  int mingw_rename(const char*, const char*);
  #define rename mingw_rename

+#ifdef USE_WIN32_MMAP
+int mingw_getpagesize(void);
+#define getpagesize mingw_getpagesize
+#endif
+
  /* Use mingw_lstat() instead of lstat()/stat() and
   * mingw_fstat() instead of fstat() on Windows.
   */
diff --git a/compat/win32mmap.c b/compat/win32mmap.c
new file mode 100644
index 0000000..66314b8
--- /dev/null
+++ b/compat/win32mmap.c
@@ -0,0 +1,53 @@
+#include "../git-compat-util.h"
+
+/* Note that this doesn't return the actual pagesize, but
+ * the allocation granularity. If future Windows specific git code
+ * needs the real getpagesize function, we need to find another solution.
+ */
+int mingw_getpagesize(void)
+{
+	SYSTEM_INFO si;
+	GetSystemInfo(&si);
+	return si.dwAllocationGranularity;
+}
+
+void *git_mmap
+(void *start, size_t length, int prot, int flags, int fd, off_t offset)
+{
+	HANDLE hmap;
+	void *temp;
+	size_t len;
+	struct stat st;
+	uint64_t o = offset;
+	uint32_t l = o & 0xFFFFFFFF;
+	uint32_t h = (o >> 32) & 0xFFFFFFFF;
+
+	if (!fstat(fd, &st))
+		len = xsize_t(st.st_size);
+	else
+		die("mmap: could not determine filesize");
+
+	if ((length + offset) > len)
+		length = len - offset;
+
+	if (!(flags & MAP_PRIVATE))
+		die("Invalid usage of mmap when built with USE_WIN32_MMAP");
+
+	hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), 0, PAGE_WRITECOPY,
+		0, 0, 0);
+
+	if (!hmap)
+		return MAP_FAILED;
+
+	temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
+
+	if (!CloseHandle(hmap))
+		warning("unable to close file mapping handle\n");
+
+	return temp ? temp : MAP_FAILED;
+}
+
+int git_munmap(void *start, size_t length)
+{
+	return !UnmapViewOfFile(start);
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index 878d83d..1eef4eb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -166,7 +166,7 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
  	return strncmp(str, prefix, len) ? NULL : str + len;
  }

-#ifdef NO_MMAP
+#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)

  #ifndef PROT_READ
  #define PROT_READ 1
@@ -180,13 +180,19 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
  extern void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
  extern int git_munmap(void *start, size_t length);

+#else /* NO_MMAP || USE_WIN32_MMAP */
+
+#include <sys/mman.h>
+
+#endif /* NO_MMAP || USE_WIN32_MMAP */
+
+#ifdef NO_MMAP
+
  /* This value must be multiple of (pagesize * 2) */
  #define DEFAULT_PACKED_GIT_WINDOW_SIZE (1 * 1024 * 1024)

  #else /* NO_MMAP */

-#include <sys/mman.h>
-
  /* This value must be multiple of (pagesize * 2) */
  #define DEFAULT_PACKED_GIT_WINDOW_SIZE \
  	(sizeof(void*) >= 8 \
-- 
1.6.2.rc2.971.g14d5

^ permalink raw reply related

* Re: [PATCH] Define a version of lstat(2) specially for copy operation
From: Junio C Hamano @ 2009-03-18  7:41 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Sixt, Jeff King, layer, git
In-Reply-To: <20090317213820.GC13458@blimp.localdomain>

Alex Riesen <raa.lkml@gmail.com> writes:

> So that Cygwin port can continue work around its supporting
> library and get access to its faked file attributes.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> ...
> diff --git a/builtin-init-db.c b/builtin-init-db.c
> index ee3911f..f3f781b 100644
> --- a/builtin-init-db.c
> +++ b/builtin-init-db.c
> @@ -66,7 +66,7 @@ static void copy_templates_1(char *path, int baselen,
>  		else
>  			exists = 1;
>  
> -		if (lstat(template, &st_template))
> +		if (lstat_for_copy(template, &st_template))
>  			die("cannot stat template %s", template);
>  
>  		if (S_ISDIR(st_template.st_mode)) {

Yuck; that's a bit too ugly for generic code.  Will there be other places
that this needs to be used?  If so, we'd probably need to encourage its
use where appropriate, which is even uglier but we cannot avoid it...

Also when the underlying system does not know the executable bit, how
would this help?  I thought that earlier you said the part that checks if
it wants to execute hooks with access(X_OK) will fail, so...

^ permalink raw reply

* Re: --exec-path not always honored
From: Johannes Sixt @ 2009-03-18  7:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v7i2nmlpt.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> Johannes Sixt <j.sixt@viscovery.net> writes:
> 
>> git-gc is a builtin. Should git setenv("GIT_EXEC_PATH") before it runs
>> other git commands?
> 
> I think we just never have bothered about such a use case, but you are
> right.  It probably is a good solution, although setenv makes me feel a
> bit nervous for no rational reason.

This patch fixes the use case and is IMO the most logical solution.

diff --git a/exec_cmd.c b/exec_cmd.c
index 217c125..408e4e5 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -61,6 +61,10 @@ const char *git_extract_argv0_path(const char *argv0)
 void git_set_argv_exec_path(const char *exec_path)
 {
 	argv_exec_path = exec_path;
+	/*
+	 * Propagate this setting to external programs.
+	 */
+	setenv(EXEC_PATH_ENVIRONMENT, exec_path, 1);
 }

^ permalink raw reply related

* Re: What's cooking in git.git (Mar 2009, #05; Tue, 17)
From: Miklos Vajna @ 2009-03-18  7:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090318055222.GA17128@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 847 bytes --]

On Wed, Mar 18, 2009 at 01:52:22AM -0400, Jeff King <peff@peff.net> wrote:
> I am inclined to leave it as-is. The other sane option would be
> converting it to use NONEG, as Miklos suggested.
> 
> Doing it right would probably mean adding a "this option is the opposite
> of what we would usually do" flag to parse-options which would display
> the option as "no-<option>" in the usage, and would reverse clearing and
> setting the bit (i.e., --empty-directory would clear the HIDE_DIRECTORY
> bit and --no-empty-directory would set it). But I don't think it is
> worth the work to add a negatable version of an option that has never
> existed before and which nobody has requested to use.

ACK, I'm fine with the current shape of the topic branch.

I did not think about the user confusion regarding usage when I
suggested NONEG.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [PATCH] blame: read custom grafts given by -S before calling setup_revisions()
From: Junio C Hamano @ 2009-03-18  7:50 UTC (permalink / raw)
  To: git

setup_revisions() while getting the command line arguments parses the
given commits from the command line, which means their direct parents will
not be rewritten by the custom graft file.

Call read_ancestry() early to work around this issue.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-blame.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index 4ea3431..0c241a9 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2346,6 +2346,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	argc = parse_options_end(&ctx);
 
+	if (revs_file && read_ancestry(revs_file))
+		die("reading graft file %s failed: %s",
+		    revs_file, strerror(errno));
+
 	if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);
@@ -2484,10 +2488,6 @@ parse_done:
 	sb.ent = ent;
 	sb.path = path;
 
-	if (revs_file && read_ancestry(revs_file))
-		die("reading graft file %s failed: %s",
-		    revs_file, strerror(errno));
-
 	read_mailmap(&mailmap, ".mailmap", NULL);
 
 	if (!incremental)
-- 
1.6.2.1.278.g7b6274e

^ permalink raw reply related

* Re: [PATCH] Define a version of lstat(2) specially for copy operation
From: Johannes Sixt @ 2009-03-18  7:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, Jeff King, layer, git
In-Reply-To: <7vmybjl1l6.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
>> So that Cygwin port can continue work around its supporting
>> library and get access to its faked file attributes.
>>
>> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
>> ...
>> diff --git a/builtin-init-db.c b/builtin-init-db.c
>> index ee3911f..f3f781b 100644
>> --- a/builtin-init-db.c
>> +++ b/builtin-init-db.c
>> @@ -66,7 +66,7 @@ static void copy_templates_1(char *path, int baselen,
>>  		else
>>  			exists = 1;
>>  
>> -		if (lstat(template, &st_template))
>> +		if (lstat_for_copy(template, &st_template))
>>  			die("cannot stat template %s", template);
>>  
>>  		if (S_ISDIR(st_template.st_mode)) {
> 
> Yuck; that's a bit too ugly for generic code.  Will there be other places
> that this needs to be used?  If so, we'd probably need to encourage its
> use where appropriate, which is even uglier but we cannot avoid it...
> 
> Also when the underlying system does not know the executable bit, how
> would this help?  I thought that earlier you said the part that checks if
> it wants to execute hooks with access(X_OK) will fail, so...

The "underlying system" in this case is Cygwin, and it *does* have an
executable bit.

But the FS gymnastics that implement it are slow and affect all lstat()
calls, so we have replaced lstat() with a simpler and faster
implementation. Only that the replacement doesn't know about the X bit
anymore; it always returns mode 0666.

Therefore, if a file is created whose mode is influenced by the fast
lstat(), then it will always be non-X. The access(, X_OK) call on the hook
script would do the right thing if only the script were created with the
correct mode. access(, X_OK) fails because the file was created with non-X
permissions.

-- Hannes

^ permalink raw reply

* Re: fetch and pull
From: Björn Steinbrink @ 2009-03-18  8:58 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: John Dlugosz, git, Junio C Hamano
In-Reply-To: <20090318063103.6117@nanako3.lavabit.com>

On 2009.03.18 06:31:03 +0900, Nanako Shiraishi wrote:
> Quoting John Dlugosz <JDlugosz@TradeStation.com>:
> > I think the documentation for git-pull might also be garbled from
> > text being of different eras.  "Normally the branch merged is the
> > HEAD of the remote"?  That will be basically random since the last
> > thing the upstream repo user did will control what his HEAD is.
> 
> That's how it's supposed to work, and the documentation isn't from a
> different era, either. Majority of users clone from a central
> repository and keep pulling to update their clones, and in that kind
> of setting, HEAD will never change. A HEAD in a bare repository tells
> people which branch is the primary branch of the project.

But _if_ HEAD changes in the remote repo, that has normally no effect on
the behaviour of pull. Especially since the examples in the man page are
"git pull" and "git pull origin".

AFAICT the remote's HEAD affects the pull behaviour when:

1) You do "git pull git://host/repo.git" or similar, i.e. you don't use
a remote. Because then, no refspec is given to fetch, and it defaults to
fetching HEAD.

2) You do "git pull origin" and there's no remote.origin.fetch set, same
as above then.

3) You do "git pull" and either is branch.<name>.remote set to an url
(same as 1) then) or to a remote that has no default fetch refspec set
(same as 2) then).


1) probably isn't that common for most users, and even if, it's not
one of the commands given as examples to which the paragraph applies.

2) I don't think that's a common setup, and it's not a default setup, so
that hardly qualifies for "normally".

3) I've actually seen cases of the first form of this setup (remote set
to an url), but usually that was paired with a confused user.


So while it's true that the remote's HEAD might be what you merge, it's
not quite what happens "normally". Of course it's true that the most
common setup is probably that HEAD references master and that
branch.<name>.merge is also set to refs/heads/master, but while the
outcome is basically the same, I'd rather say that that is a coincidence
rather than what the text means to the reader.


I'm unsure about how to improve that section though. Basically, the last
paragraph ("When no refspec was given ...") above the EXAMPLES section
needs to be repeated, but that feels wrong. Anyone got a better idea?

Well, at least I finally realised that pull _might_ default to the
remote's HEAD, and for that part, I'll send a patch.

Björn

^ permalink raw reply

* Re: [JGIT PATCH 00/12] Improve test coverage in revwalk
From: Robin Rosenberg @ 2009-03-18  9:21 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <200903180734.59435.robin.rosenberg.lists@dewire.com>

onsdag 18 mars 2009 07:34:59 skrev Robin Rosenberg <robin.rosenberg.lists@dewire.com>:
> onsdag 18 mars 2009 02:40:39 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > The tests are commented out because JUnit doesn't have a notion of
> > "known broken".  But I did leave in TODO comments.  I'd like to
> > apply the test, and then work later to improve it, but I'm open
> > to suggestions.
> 
> If we'd switch to JUnit4/TestNG we could abuse the expected exception
> annotation.

..or the  @Ignore annotation

-- robin

^ permalink raw reply

* Re: [PATCH] Define a version of lstat(2) specially for copy operation
From: Junio C Hamano @ 2009-03-18  9:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Alex Riesen, Jeff King, layer, git
In-Reply-To: <49C0A919.7070606@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> The "underlying system" in this case is Cygwin, and it *does* have an
> executable bit.
>
> But the FS gymnastics that implement it are slow and affect all lstat()
> calls, so we have replaced lstat() with a simpler and faster
> implementation. Only that the replacement doesn't know about the X bit
> anymore; it always returns mode 0666.
>
> Therefore, if a file is created whose mode is influenced by the fast
> lstat(), then it will always be non-X. The access(, X_OK) call on the hook
> script would do the right thing if only the script were created with the
> correct mode. access(, X_OK) fails because the file was created with non-X
> permissions.

OK.  To make sure that I understand the situation better, allow me to
paraphrase what you said.

I've always thought core.filemode was about "On FAT the filesystem does
not have x-bit and Cygwin fakes it by looking at .exe extension and using
other heuristics", but this lstat() "trick" is not about that.  It is "On
Windows you need to issue multiple FS API calls in order to get full
information to fill struct stat, which is expensive.  We omit the one to
learn about the x-bit; it won't make a difference because most people run
with core.filemode."

If that is what is going on, I have a few quick questions:

 (1) How much of "struct stat" information can you get with the easiest
     and quickest FS API call on Windows?  Does it give you big enough
     subset of what we store in the cache entry to detect modification
     reliably?

 (2) When you do an equivalent of "chmod +x path" on Windows, does it
     change one of the fields in your answer to (1)?  In other words, can
     you detect such a change with the fastest and reliable-enough FS API
     call?

If answers to both questions are "yes", then I suspect we might be able to
improve the situation without sacrificing performance too much.

In the generic part, we use lstat(2) for various purposes:

 * To learn existence and type of a FS entity, in order to decide if we
   need to descend into a directory or treat it as a blob;

 * To learn the current "status" that can be compared with what is stored
   in the cache entry, in order to decide if the FS entity hasn't been
   modified;

 * To learn the last-modified timestamp, in order to implement the
   racy-git avoidance logic;

 * To learn x-bit, so that we can update it in the cache entry, and give
   the appropriate x-bit to a file that we create (Alex's
   lstat_for_copy() is about this usage).

If the first three can be learned with a fast-and-reliable-enough FS API
call, and the x-bit and perhaps something else that do not fall into the
first three need a lot more work to figure out, we could split lstat()
into two.  The result from a "fast" variant of lstat() is used for the
first three and allow platform implementation of it to be incomplete
(i.e. the Cygwin "trick" to omit x-bit is OK for that purpose), and add
another "slow" variant to complement it:

	int lstat_fast(const char *path, struct stat *st);
        void lstat_remainder(const char *path, struct stat *st);

On POSIX systems, we implement it as

        #define lstat_fast(path,st) lstat((path),(st))
        #define lstat_remainder(path,st) do { ; /* nothing */ } while (0)
 
but on Windows and Cygwin, we might implement the format to get
good-enough information for comparison purposes and implement latter to
fill the x-bit and some other parts that are expensive to learn.

Most of the callers that are only interested in seeing if a path has
changed can use the lstat_fast(), while the codepath that actually does
use the information for modified path can augment the information a
previous call to "fast" variant obtained with an additional call to the
"slow" lstat_remainder().

The attached patch to convert a part of read-cache.c is only for
illustration purposes.

The refresh_cache_ent() function first calls lstat_fast() to get cheap
information that is good enough, gives it to ie_match_stat(), to see if
anything has changed, and let fill_stat_cache_info() to update the cached
information for later comparison.

We do not have to touch ce_match_stat_basic() that does care about x-bit
but assuming that the answer to question (2) is "yes", we would not miss
MODE_CHANGED report from it; other information such as timestamps would
have changed in such a case as well, and ie_match_stat() that called
ce_match_stat_basic() will still say "changed" for such an entry.

The add_file_to_index() function checks if the path exists with the "fast"
one, gives the result to add_to_index() that wants the full mode bits
hence invokes the "slow" one to fill in the remainder.

If you have to pass information between "fast" and "slow" variant other
than what you can put in "struct stat" in order for "slow" one to take
advantage of what "fast" one already has done, we would need to introduce
something like

	struct gitstat {
        	struct stat st;
		some other info for platform;
	};

and pass that around instead, which would become a larger change, though.
I am hoping it won't be the case.

And if we do not need such an extra "struct gitstat", then there is no
reason for us to introduce lstat_fast().  We can just use lstat(), keep
the "fast and incomplete" Cygwin one as lstat(), but insert calls to
lstat_remainder() at strategic places.  Immediately before copy_file() is
called in builtin-init-db.c::copy_templates_1() would be one of those
strategic places.



diff --git a/read-cache.c b/read-cache.c
index 3f58711..d12d11b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -562,7 +562,7 @@ static void record_intent_to_add(struct cache_entry *ce)
 int add_to_index(struct index_state *istate, const char *path, struct stat *st, int flags)
 {
 	int size, namelen, was_same;
-	mode_t st_mode = st->st_mode;
+	mode_t st_mode;
 	struct cache_entry *ce, *alias;
 	unsigned ce_option = CE_MATCH_IGNORE_VALID|CE_MATCH_RACY_IS_DIRTY;
 	int verbose = flags & (ADD_CACHE_VERBOSE | ADD_CACHE_PRETEND);
@@ -571,6 +571,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE|
 			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
 
+	lstat_remainder(path, st);
+	st_mode = st->st_mode;
 	if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
 		return error("%s: can only add regular files, symbolic links or git-directories", path);
 
@@ -637,7 +639,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 int add_file_to_index(struct index_state *istate, const char *path, int flags)
 {
 	struct stat st;
-	if (lstat(path, &st))
+	if (lstat_fast(path, &st))
 		die("%s: unable to stat (%s)", path, strerror(errno));
 	return add_to_index(istate, path, &st, flags);
 }
@@ -1013,7 +1015,7 @@ static struct cache_entry *refresh_cache_ent(struct index_state *istate,
 		return ce;
 	}
 
-	if (lstat(ce->name, &st) < 0) {
+	if (lstat_fast(ce->name, &st) < 0) {
 		if (err)
 			*err = errno;
 		return NULL;

^ permalink raw reply related

* Re: fetch and pull
From: Junio C Hamano @ 2009-03-18  9:37 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Nanako Shiraishi, John Dlugosz, git
In-Reply-To: <20090318085849.GA8118@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> 1) You do "git pull git://host/repo.git" or similar, i.e. you don't use
> a remote. Because then, no refspec is given to fetch, and it defaults to
> fetching HEAD.
> ...
> 1) probably isn't that common for most users, and even if, it's not
> one of the commands given as examples to which the paragraph applies.

I think "a maintainer responds to a pull request" is the only case that
happens often in practice for a fetch+merge of HEAD from a remote
repository.

Even in that case, we strongly encourage people to say not just "which
repository location" but also "which branch", i.e.

	Linus, please pull from:

		git://git.or.cz/alt-git.git for-linus

	to obtain the following commits.

When Linus cuts and pastes that to his shell, the command will become:

	$ git pull git://git.or.cz/alt-git.git for-linus

and HEAD is not involved in such a use case.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox