Git development
 help / color / mirror / Atom feed
* Re: [PATCH] make shallow repository deepening more network efficient
From: Johannes Schindelin @ 2009-09-26 13:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vmy58r72b.fsf@alter.siamese.dyndns.org>

Hi,

On Sat, 5 Sep 2009, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > First of all, I can't find any reason why thin pack generation is 
> > explicitly disabled when dealing with a shallow repository.  The 
> > possible delta base objects are collected from the edge commits which 
> > are always obtained through history walking with the same shallow refs 
> > as the client, Therefore the client is always going to have those base 
> > objects available. So let's remove that restriction.
> >
> > Then we can make shallow repository deepening much more efficient by 
> > using the remote's unshallowed commits as edge commits to get preferred 
> > base objects for thin pack generation.  On git.git, this makes the data 
> > transfer for the deepening of a shallow repository from depth 1 to depth 2
> > around 134 KB instead of 3.68 MB.
> >
> > Signed-off-by: Nicolas Pitre <nico@cam.org>
> 
> Dscho, this is your code from around ed09aef (support fetching into a
> shallow repository, 2006-10-30) and f53514b (allow deepening of a shallow
> repository, 2006-10-30).  The latter disables thin pack transfer but the
> log does not attempt to justify the change.
> 
> Have any comments?

Unfortunately, I do not have any.  I tried to remember, then I tried to 
find some documentation, but stopped when I found out that I developed 
the code on an iBook which died early Nov 2006.

So no, I do not remember why the change.

Sorry,
Dscho

^ permalink raw reply

* Re: Distribution size
From: Tay Ray Chuan @ 2009-09-26 11:53 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Hi,

On Sat, Sep 26, 2009 at 7:11 PM, Marc Weber <marco-oweber@gmx.de> wrote:
> git-1.6.4.4 i%du -hs .
> 115M    .

perhaps it's due to usage of hard links?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: Distribution size
From: Jakub Narebski @ 2009-09-26 11:27 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Marc Weber <marco-oweber@gmx.de> writes:

> Has anyone thought about reducing distribution size ?
> 
> comparison:
> 
> git-1.6.4.4 i%du -hs .
> 115M    .

Errr... what?

  $ du -hs /usr/libexec/git-core/
  8.0M

  $ du -shc `rpm -ql git`
  ...
  27M
 
But

  $ du -sh git/.git
  49M

(there is some of my commits there, some old branches pinned, and some
commits with other git(web) forks).

And

  $ git archive HEAD --output=tmp.tar
  $ du -sh tmp.tar 
  11M     tmp.tar

> mercurial-1.0.2 i%du -hs .
> 5.4M    .
> 
> bazaar-1.10rc1 i%du -hs
> 21M     .
> 
> darcs-2.2.1 i%du -hs    /nix/store/8xwfavyv22pvm3s60wvzcbq561fjk5di-darcs-2.2.1 nixos   
> 32M	.
> 
> There was an attempt to reduce size by using a shared library in 2007:
> http://markmail.org/message/vzukaie4qvbghkq5
> 
> So is there a reason why git takes up so much more disk space other than
> that its using many small executables?

Are you sure you are comparing the same thing in all cases?
And not in some cases after "make", with all object files?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Distribution size
From: Johannes Schindelin @ 2009-09-26 11:22 UTC (permalink / raw)
  To: Marc Weber; +Cc: git
In-Reply-To: <1253962653-sup-1882@nixos>

Hi,

On Sat, 26 Sep 2009, Marc Weber wrote:

> Has anyone thought about reducing distribution size ?
> 
> comparison:
> 
> git-1.6.4.4 i%du -hs .
> 115M    .

Funny.  Git for Windows is less than 12MB [*1*].  And it includes a bash 
and Perl.  (Did you count Python on Windows?)

So maybe your analysis is severely borked.

Ciao,
Dscho

[*1*]: See http://msysgit.googlecode.com/

^ permalink raw reply

* Distribution size
From: Marc Weber @ 2009-09-26 11:11 UTC (permalink / raw)
  To: git

Hi,

Has anyone thought about reducing distribution size ?

comparison:

git-1.6.4.4 i%du -hs .
115M    .

mercurial-1.0.2 i%du -hs .
5.4M    .

bazaar-1.10rc1 i%du -hs
21M     .

darcs-2.2.1 i%du -hs    /nix/store/8xwfavyv22pvm3s60wvzcbq561fjk5di-darcs-2.2.1 nixos   
32M	.

There was an attempt to reduce size by using a shared library in 2007:
http://markmail.org/message/vzukaie4qvbghkq5

So is there a reason why git takes up so much more disk space other than
that its using many small executables?

Marc Weber

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows  Explorer
From: Sebastian Schuberth @ 2009-09-26  9:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, mstormo
In-Reply-To: <20090926000500.GE14660@spearce.org>

On Sat, Sep 26, 2009 at 02:05, Shawn O. Pearce <spearce@spearce.org> wrote:

> Odd.  If I copy and paste from Thunderbird, its fine.  But if I
> save the body out as an attachment from mutt, it fails.
>
> I wonder if it has to do with the From header appearing in the top
> of the body; this header has to be escaped with a leading space in
> mbox format.  It looks like Thunderbird might be doing some magic to
> remove that leading space from the context lines, while mutt isn't.
>
> Next time, don't include the first From line?

Will try. So what about these two patches? Should I re-send them with
the first "From" stripped?

Or will *.patch files that are attached to emails, instead of sending
the patch inline, be accepted?

-- 
Sebastian Schuberth

^ permalink raw reply

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-25 23:53 UTC (permalink / raw)
  To: git

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 24865cf..221d49c 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_index_die(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related

* Re: [PATCH] make 'git clone' ask the remote only for objects it cares about
From: Andreas Schwab @ 2009-09-26  7:21 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Junio C Hamano, Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909252314260.4997@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> The fact that the git repository on gcc.gnu.org has lots of stuff in 
> "remote" branches that don't get cloned by default is a separate 
> configuration/policy issue on that server which might need (or not) to 
> be looked into.  For instance at least, as a bare repository, it should 
> have all the git files in gcc.git/ directly instead of gcc.git/.git/.

The remote is just a git-svn tree.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Jason Merrill @ 2009-09-26  4:44 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD4F7B.4030701@redhat.com>

Incidentally, somewhat related to this issue, I've noticed that if I 
fetch a branch which I don't currently have in my repository, and I have 
most of the commits on that branch in my object store (or in an 
alternate repository) but not the most recent commit, git fetch isn't 
smart enough to only grab the commits I'm actually missing, it wants to 
fetch much more.

I would expect that since the clone pulled down everything in the 
gcc.git repository, I could then do

git config remote.origin.fetch 'refs/remotes/*:refs/remotes/origin/*'
git fetch

and have all the branches, not just the ones in refs/heads.  But when I 
do this git fetch wants to fetch some 500k redundant objects.

Jason

^ permalink raw reply

* [PATCH] make 'git clone' ask the remote only for objects it cares about
From: Nicolas Pitre @ 2009-09-26  3:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909252045290.4997@xanadu.home>

Current behavior of 'git clone' when not using --mirror is to fetch 
everything from the peer, and then filter out unwanted refs just before 
writing them out to the cloned repository.  This may become highly 
inefficient if the peer has an unusual ref namespace, or if it simply 
has "remotes" refs of its own, and those locally unwanted refs are 
connecting to a large set of objects which becomes unreferenced as soon 
as they are fetched.

Let's filter out those unwanted refs from the peer _before_ asking it 
what refs we want to fetch instead, which is the most logical thing to 
do anyway.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---

On Fri, 25 Sep 2009, Nicolas Pitre wrote:

> On Fri, 25 Sep 2009, Jason Merrill wrote:
> 
> > On 09/25/2009 04:47 PM, Nicolas Pitre wrote:
> > > Do you have access to the remote machine?  Is it possible to have a
> > > tarball of the gcc.git directory from there?
> > 
> > http://gcc.gnu.org/gcc-git.tar.gz
> > 
> > I'll leave it there for a few days.
> 
> Thanks, I got it now.  And I was able to reproduce the issue locally.
> 
> Cloning the original repository does transfer objects which become 
> unreferenced in the clone.  But cloning that cloned repository (before 
> pruning the unreferenced objects) does not transfer those objects again.  
> 
> Just need to find out why.

And the "why" is described above.  The problem was actually on the 
client side and was affecting clones of any repository containing 
anything outside refs/heads and refs/tags.

The fact that the git repository on gcc.gnu.org has lots of stuff in 
"remote" branches that don't get cloned by default is a separate 
configuration/policy issue on that server which might need (or not) to 
be looked into.  For instance at least, as a bare repository, it should 
have all the git files in gcc.git/ directly instead of gcc.git/.git/.

diff --git a/builtin-clone.c b/builtin-clone.c
index bab2d84..edf7c7f 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -329,24 +329,28 @@ static void remove_junk_on_signal(int signo)
 	raise(signo);
 }
 
-static struct ref *write_remote_refs(const struct ref *refs,
-		struct refspec *refspec, const char *reflog)
+static struct ref *wanted_peer_refs(const struct ref *refs,
+		struct refspec *refspec)
 {
 	struct ref *local_refs = NULL;
 	struct ref **tail = &local_refs;
-	struct ref *r;
 
 	get_fetch_map(refs, refspec, &tail, 0);
 	if (!option_mirror)
 		get_fetch_map(refs, tag_refspec, &tail, 0);
 
+	return local_refs;
+}
+
+static void write_remote_refs(const struct ref *local_refs, const char *reflog)
+{
+	const struct ref *r;
+
 	for (r = local_refs; r; r = r->next)
 		add_extra_ref(r->peer_ref->name, r->old_sha1, 0);
 
 	pack_refs(PACK_REFS_ALL);
 	clear_extra_refs();
-
-	return local_refs;
 }
 
 int cmd_clone(int argc, const char **argv, const char *prefix)
@@ -495,9 +499,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	strbuf_reset(&value);
 
-	if (path && !is_bundle)
+	if (path && !is_bundle) {
 		refs = clone_local(path, git_dir);
-	else {
+		mapped_refs = wanted_peer_refs(refs, refspec);
+	} else {
 		struct remote *remote = remote_get(argv[0]);
 		transport = transport_get(remote, remote->url[0]);
 
@@ -520,14 +525,16 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 					     option_upload_pack);
 
 		refs = transport_get_remote_refs(transport);
-		if (refs)
-			transport_fetch_refs(transport, refs);
+		if (refs) {
+			mapped_refs = wanted_peer_refs(refs, refspec);
+			transport_fetch_refs(transport, mapped_refs);
+		}
 	}
 
 	if (refs) {
 		clear_extra_refs();
 
-		mapped_refs = write_remote_refs(refs, refspec, reflog_msg.buf);
+		write_remote_refs(mapped_refs, reflog_msg.buf);
 
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =

^ permalink raw reply related

* Re: [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Jeff King @ 2009-09-26  3:31 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1253923602-17818-1-git-send-email-vmiklos@frugalware.org>

On Sat, Sep 26, 2009 at 02:06:42AM +0200, Miklos Vajna wrote:

> diff --git a/refs.c b/refs.c
> index 24865cf..221d49c 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
>  	if (!found)
>  		return 0;
>  	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		unable_to_lock_index_die(git_path("packed-refs"), errno);
>  		return error("cannot delete '%s' from packed refs", refname);
> +	}
>  
>  	for (list = packed_ref_list; list; list = list->next) {
>  		char line[PATH_MAX + 100];

I have several concerns about this patch:

  1. We used to return error(), but now we die. Are there any callers
     which care about the difference?

  2. If we did want to die, then the "return error()" just below is
     unreachable, and should be removed.

  3. If we did want to die, should we not just pass LOCK_DIE_ON_ERROR to
     hold_lock_file_for_update?

I suspect (2) and (3) are irrelevant because the answer to (1) is that
yes, some callers do care (e.g., it looks like builtin-remote calls
delete_ref, and notes an error but continues to do useful work
afterwards). So probably you would need to first refactor
unable_to_lock_index_die() to handle just printing the error without
dying.

-Peff

^ permalink raw reply

* Re: how optparse can go horribly wrong
From: Nicolas Sebrecht @ 2009-09-26  1:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Nicolas Sebrecht
In-Reply-To: <20090925233226.GC14660@spearce.org>

The 25/09/09, Shawn O. Pearce wrote:
> *sigh*.  Someone just ran into this today:
> 
>   $ git commit -a -ammend
>   [work ce38944] mend
>    1 files changed, 2 insertions(+), 0 deletions(-)
> 
> Omit one - and include an extra 'm', and instead of --amend you
> have -a -m mend.  Which isn't exactly what you wanted.
> 
> We do catch -amend with an error though:
> 
>   $ git commit -amend
>   error: did you mean `--amend` (with two dashes ?)

OTOH, this is a bit odd because a commit with the message "end" makes
perfect sense for a "fast and crappy commit local workflow".

And we allow -ammend (with two 'm')

  $ git commit -ammend
  [next 101f014] mend
   1 files changed, 1 insertions(+), 0 deletions(-)
  $

> I wonder, should the -m flag on commit not allow cuddling its
> value against the switch when its combined in short form with
> other switches?

Doing this only to -m flag would break consistency. That said, I don't
have any opinion in disallowing the sticked form for _all_ short
options.

-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Nicolas Pitre @ 2009-09-26  0:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD4F7B.4030701@redhat.com>

On Fri, 25 Sep 2009, Jason Merrill wrote:

> On 09/25/2009 04:47 PM, Nicolas Pitre wrote:
> > Do you have access to the remote machine?  Is it possible to have a
> > tarball of the gcc.git directory from there?
> 
> http://gcc.gnu.org/gcc-git.tar.gz
> 
> I'll leave it there for a few days.

Thanks, I got it now.  And I was able to reproduce the issue locally.

Cloning the original repository does transfer objects which become 
unreferenced in the clone.  But cloning that cloned repository (before 
pruning the unreferenced objects) does not transfer those objects again.  

Just need to find out why.


Nicolas

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Hin-Tak Leung @ 2009-09-26  0:43 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Nicolas Pitre, Matthieu Moy, git
In-Reply-To: <4ABD25FE.2040902@redhat.com>

On Fri, Sep 25, 2009 at 9:20 PM, Jason Merrill <jason@redhat.com> wrote:
> On 09/25/2009 03:53 PM, Nicolas Pitre wrote:
>>
>> I did reproduce the issue with git:// back when this discussion started.
>> I also asked for more information about the remote which didn't come
>> forth.
>
> Looking back, I only see you asking about the git version on the server,
> which is 1.6.4.

Hmm, I was under the impression from the previous thread that the
server is a bit older and/or have more backward compatible settings to
cater for older git clients?

>
> So again:
>
> git clone git://gcc.gnu.org/git/gcc.git
>  (1399509 objects, ~600MB .git dir)
> git gc --prune=now (988906 objects, ~450MB .git dir)
>
> ...then
>
> git clone git://gcc.gnu.org/git/gcc.git --reference $firstclone
>  (573401 objects, ~550MB .git dir)
> git fsck (clean)
> git gc --prune=now (5 objects, ~7MB .git dir)
>
> What's going on here?

FWIW, I still have my clone (git://) and do my periodic 'git fetch'
and 'git gc prune=now' (learned my lessons!) and it is currently .git
dir is about 350MB. (from previous discussion the optimal at the time
was about 300MB, so it has grown a bit in the last couple of months).

And thanks everybody for all the discussion and advice. git is a great
tool. (and I have essentially stopped using svn, prefering git-svn!).

^ permalink raw reply

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-26  0:06 UTC (permalink / raw)
  To: git

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 refs.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 24865cf..221d49c 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_index_die(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows Explorer
From: Shawn O. Pearce @ 2009-09-26  0:05 UTC (permalink / raw)
  To: Sebastian Schuberth; +Cc: git, mstormo
In-Reply-To: <bdca99240909251658q395a62b6r8d5998382ac3fc7b@mail.gmail.com>

Sebastian Schuberth <sschuberth@gmail.com> wrote:
> On Sat, Sep 26, 2009 at 00:59, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Sebastian Schuberth <sschuberth@gmail.com> wrote:
> >> On Sat, Sep 26, 2009 at 00:05, Shawn O. Pearce <spearce@spearce.org> wrote:
> >> > Something is wrong with both patches; neither applies.
> >>
> >> Hmm, the patches apply fine onto master for me.
> >
> > After going through your client and the GMane email gateway?
> > Or before you pasted it into the message?
> 
> Both. As a test for the first, I've copied & pasted my mail as it
> appears for me in gmane.comp.version-control.git to a new file and
> successfully applied that file via "git apply". I had to do it this
> way, as on Windows I cannot really use "git am" with Thunderbird.

Odd.  If I copy and paste from Thunderbird, its fine.  But if I
save the body out as an attachment from mutt, it fails.

I wonder if it has to do with the From header appearing in the top
of the body; this header has to be escaped with a leading space in
mbox format.  It looks like Thunderbird might be doing some magic to
remove that leading space from the context lines, while mutt isn't.

Next time, don't include the first From line?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows  Explorer
From: Sebastian Schuberth @ 2009-09-25 23:58 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, mstormo
In-Reply-To: <20090925225940.GB14660@spearce.org>

On Sat, Sep 26, 2009 at 00:59, Shawn O. Pearce <spearce@spearce.org> wrote:

> Sebastian Schuberth <sschuberth@gmail.com> wrote:
>> On Sat, Sep 26, 2009 at 00:05, Shawn O. Pearce <spearce@spearce.org> wrote:
>> > Something is wrong with both patches; neither applies. ??It looks
>> > to be an issue with whitespace, like context lines are gaining an
>> > extra space at the start of the line.
>>
>> Hmm, the patches apply fine onto master for me.
>
> After going through your client and the GMane email gateway?
> Or before you pasted it into the message?

Both. As a test for the first, I've copied & pasted my mail as it
appears for me in gmane.comp.version-control.git to a new file and
successfully applied that file via "git apply". I had to do it this
way, as on Windows I cannot really use "git am" with Thunderbird.

-- 
Sebastian Schuberth

^ permalink raw reply

* how optparse can go horribly wrong
From: Shawn O. Pearce @ 2009-09-25 23:32 UTC (permalink / raw)
  To: git

*sigh*.  Someone just ran into this today:

  $ git commit -a -ammend
  [work ce38944] mend
   1 files changed, 2 insertions(+), 0 deletions(-)

Omit one - and include an extra 'm', and instead of --amend you
have -a -m mend.  Which isn't exactly what you wanted.

We do catch -amend with an error though:

  $ git commit -amend
  error: did you mean `--amend` (with two dashes ?)

I wonder, should the -m flag on commit not allow cuddling its
value against the switch when its combined in short form with
other switches?

-- 
Shawn.

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Jason Merrill @ 2009-09-25 23:17 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909251629330.4997@xanadu.home>

On 09/25/2009 04:47 PM, Nicolas Pitre wrote:
> Do you have access to the remote machine?  Is it possible to have a
> tarball of the gcc.git directory from there?

http://gcc.gnu.org/gcc-git.tar.gz

I'll leave it there for a few days.

Jason

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows Explorer
From: Shawn O. Pearce @ 2009-09-25 22:59 UTC (permalink / raw)
  To: Sebastian Schuberth; +Cc: git, mstormo
In-Reply-To: <bdca99240909251541h2e9932a3r67c1d8604e56a8df@mail.gmail.com>

Sebastian Schuberth <sschuberth@gmail.com> wrote:
> On Sat, Sep 26, 2009 at 00:05, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Something is wrong with both patches; neither applies. ??It looks
> > to be an issue with whitespace, like context lines are gaining an
> > extra space at the start of the line.
> 
> Hmm, the patches apply fine onto master for me.

After going through your client and the GMane email gateway?
Or before you pasted it into the message?
 
> As a side note, I've never liked sending patches via mail. It just
> feels like writing a text on a computer just to print it out, scan in
> the sheet again, and then sending the scanned image as an attachment
> via email. Isn't there a better way to contribute patches to Git, like
> pushing to a mob branch as for the msysGit project?

It works fine, so long as the tools in the chain leave the body
of the message alone and don't try to outsmart the human who put
it there.

The git project in particular as rules about only applying patches
off the mailing list, because then all contributions are in a
permanent, public archive.  Its also the form of communication most
contributors prefer for code reviews.

-- 
Shawn.

^ permalink raw reply

* Re: [JGIT PATCH 1/2] add support for core.logAllRefUpdates configuration parameter
From: Robin Rosenberg @ 2009-09-25 22:52 UTC (permalink / raw)
  To: Halstrick, Christian; +Cc: Shawn O. Pearce, git@vger.kernel.org
In-Reply-To: <200909260045.21171.robin.rosenberg@dewire.com>

lördag 26 september 2009 00:45:20 skrev Robin Rosenberg <robin.rosenberg@dewire.com>:
> onsdag 23 september 2009 18:42:29 skrev "Halstrick, Christian" <christian.halstrick@sap.com>:
> > From: Christian Halstrick <christian.halstrick@sap.com>
> > 
> > JGit should understand configuration parameter logAllRefUpdates and should
> > only log updates of refs when
> >   either the log file for this ref is already present
> >   or this configuration parameter is set to true
> > Before this commit JGit was always writing logs, regardless of this
> > parameter or existence of logfiles.
> 
> A few minor things:
Forgot: Please:
> Format the comment nicely and space betwee paragraphs.
[...]

-- robin

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows  Explorer
From: Sebastian Schuberth @ 2009-09-25 22:45 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Shawn O. Pearce, git, mstormo
In-Reply-To: <81b0412b0909251511m24e5c6bcg3f8c4e3576ad3bd3@mail.gmail.com>

On Sat, Sep 26, 2009 at 00:11, Alex Riesen <raa.lkml@gmail.com> wrote:

> On Sat, Sep 26, 2009 at 00:05, Shawn O. Pearce <spearce@spearce.org> wrote:
>> Sebastian Schuberth <sschuberth@gmail.com> wrote:
>>> In order to be able to open the generated solution file by double-clicking it
>>> in Windows Explorer, all project files need to use DOS line-endings and a
>>> comment about the Visual Studio version needs to be added to the header of the
>>> solution file. This also fixes the icon that is displayed for the solution file
>>> in Windows Explorer.
>>> Note that opening the solution file from a running instance of Visual Studio
>>> already worked before.
>>
>> Something is wrong with both patches; neither applies.  It looks
>> to be an issue with whitespace, like context lines are gaining an
>> extra space at the start of the line.
>
> Besides, there is PERLIO environment variable. Try setting it to
> PERLIO=':perlio'

But I don't feel like using it here would be the right thing to do.
Visual Studio project files should never ever be written out with Unix
line-endings, not even if someone created them under Unix. So instead
of relying on the environment variable to be set correctly, I believe
it's better to always enforce the correct line-endings here.

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [JGIT PATCH 1/2] add support for core.logAllRefUpdates configuration parameter
From: Robin Rosenberg @ 2009-09-25 22:45 UTC (permalink / raw)
  To: Halstrick, Christian; +Cc: Shawn O. Pearce, git@vger.kernel.org
In-Reply-To: <D35B4A582834DC418CCF9AF41AB69B70016953F62F@DEWDFECCR04.wdf.sap.corp>

onsdag 23 september 2009 18:42:29 skrev "Halstrick, Christian" <christian.halstrick@sap.com>:
> From: Christian Halstrick <christian.halstrick@sap.com>
> 
> JGit should understand configuration parameter logAllRefUpdates and should
> only log updates of refs when
>   either the log file for this ref is already present
>   or this configuration parameter is set to true
> Before this commit JGit was always writing logs, regardless of this
> parameter or existence of logfiles.

A few minor things:
Format the comment nicely and space betwee paragraphs.
Start the comment with a capital letter
A cover letter for multiple patches is nice, though in this case you could
actually just squash there patches into one patch.

and one major.
RefUpdateTest broke.

Probably because of this.
> +		logAllRefUpdates = rc.getBoolean("core", "logAllRefUpdates", false);

According to the git-config man page logAllRefUpdates is true by default for non-bare
repos and false for bare repos.

I had some trouble applying these patches as git-am did not like them. Apparently
git-am doesn't like base64. Could you set your mailer to use a more plain-ish format
or use git-send-email. 

-- robin

^ permalink raw reply

* Re: [PATCH 1/2] Make generated MSVC solution file open from Windows  Explorer
From: Sebastian Schuberth @ 2009-09-25 22:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, mstormo
In-Reply-To: <20090925220510.GY14660@spearce.org>

On Sat, Sep 26, 2009 at 00:05, Shawn O. Pearce <spearce@spearce.org> wrote:

> Sebastian Schuberth <sschuberth@gmail.com> wrote:
>> From c6d29a2d243647bb2877eb2114938ae20c8e56e5 Mon Sep 17 00:00:00 2001
>> From: Sebastian Schuberth <sschuberth@gmail.com>
>> Date: Thu, 24 Sep 2009 15:52:25 +0200
>> Subject: [PATCH 1/2] Make generated MSVC solution file open from Windows Explorer
>>
>> In order to be able to open the generated solution file by double-clicking it
>> in Windows Explorer, all project files need to use DOS line-endings and a
>> comment about the Visual Studio version needs to be added to the header of the
>> solution file. This also fixes the icon that is displayed for the solution file
>> in Windows Explorer.
>> Note that opening the solution file from a running instance of Visual Studio
>> already worked before.
>
> Something is wrong with both patches; neither applies.  It looks
> to be an issue with whitespace, like context lines are gaining an
> extra space at the start of the line.

Hmm, the patches apply fine onto master for me.

As a side note, I've never liked sending patches via mail. It just
feels like writing a text on a computer just to print it out, scan in
the sheet again, and then sending the scanned image as an attachment
via email. Isn't there a better way to contribute patches to Git, like
pushing to a mob branch as for the msysGit project?

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: git stash list shows timestamp in stead of "stash number", when setting date = local for log in config
From: Shawn O. Pearce @ 2009-09-25 22:29 UTC (permalink / raw)
  To: Jeff King; +Cc: Alf Kristian St??yle, git
In-Reply-To: <20090924070121.GD24486@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Tue, Sep 15, 2009 at 04:56:41PM +0200, Alf Kristian St??yle wrote:
> > When doing a "git stash list" I get this strange stash record:
> > stash@{Tue Sep 15 16:28:12 2009}: WIP on master: 2262276 ...
> > 
> > I have a global config setting on log:
> > 
> > [log]
> > date = local
> > 
> > If setting the date config to default or removing the setting, the
> > stash record looks correct:
> > stash@{0}: WIP on master: 2262276 ...
> 
> The patch below implements the former. The only downside I can think of
> is if somebody is relying on "log.date" to set the output format for
> reflogs, because they really like the date version better. In that case,
> I think we should wait for them to complain (which I doubt will happen),
> and then add a log.reflogDates config option to appease them.
> 
> Shawn, reflogs are your thing. Any comments?

I agree.  I doubt anyone is relying on log.date to reformat the
output of `git reflog show` or `git stash list`, so this is probably
a reasonable change to make.  Even if they were trying to use that,
its a bug.

Care to wrap this up in a patch?
 
-- 
Shawn.

^ 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