Git development
 help / color / mirror / Atom feed
* Re: git-svnimport failed and now git-repack hates me
From: Shawn O. Pearce @ 2007-01-05 19:39 UTC (permalink / raw)
  To: Chris Lee; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <204011cb0701051133r1ede14a6gd5093a3e7fa88cb5@mail.gmail.com>

Chris Lee <chris133@gmail.com> wrote:
> Also, 'git-prune' took about 30 minutes to run to completion. Oddly,
> git-prune didn't remove the older packs - does git-prune ignore packs?
> 'git-repack -a -d' did remove them.

git-prune is expensive.  Very expensive on very large projects,
as it must iterate every object to decide what is needed, before
it can start to remove objects that aren't needed.

Yes, it doesn't deal with removing pack files.  That's what the -d
to git-repack is for.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC] git-remote
From: Santi Béjar @ 2007-01-05 19:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.63.0701051448560.22628@wbgn013.biozentrum.uni-wuerzburg.de>

On 1/5/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 5 Jan 2007, Santi Béjar wrote:
>
> > * In "git remote add <name> <remote>": git could use the remote url to
> > deduce a <name>, like what git-clone does.
>
> That does not make any sense. For example, I track
> "git://git.kernel.org/.../git.git" and "192.168.0.128:gits/git.git".
> Something very similar applies to the host name: if you track multiple
> Linux repos, chances are that most of them are on git.kernel.org.
>
> I guess _if_ you have more than one upstream you are tracking (which is
> not the most common case, but hey, git-remote is for exactly that case) it
> is not uncommon to have similar urls.
>
> IMHO Junio's proposal is as good as it gets.
>

I was talking about the default name, so you could do:

$ git clone\
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
$ cd linux-2.6
$ git remote add \
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-2.6.git
$ git remote show libata-2.6

Santi

^ permalink raw reply

* Re: fatal: unable to create '.git/index': File exists
From: Johannes Schindelin @ 2007-01-05 20:16 UTC (permalink / raw)
  To: Len Brown; +Cc: git
In-Reply-To: <200701051150.09968.lenb@kernel.org>

Hi,

On Fri, 5 Jan 2007, Len Brown wrote:

> I kicked off a pull.
> Realized I was on the wrong branch
> and immediately did a ^C
> 
> Now I can't change branches:
> 
> # git checkout release
> fatal: unable to create '.git/index': File exists

This usually means that .git/index.lock exists. Try removing that file.

However, it _should_ be removed by the atexit() handler, methinks. I have 
no idea why it does not.

Ciao,
Dscho

^ permalink raw reply

* Re: Default "tar" umask..
From: René Scharfe @ 2007-01-05 20:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vfyaxjiaj.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano schrieb:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
>> We just had a posting on the kernel security list where a person
>> was upset that the 2.6.19.1 and .2 tar-files were apparently group
>> and world-writable.
> 
> I had an impression that this is only an issue when you untar as 
> root, and running 'tar xf' as root _is_ a more serious security issue
> than whatever permission the tar archive itself records.

While I agree, I also detect a theory-practice-mismatch.  Users,
operating as root, can (and apparently often do) download a tar file
released by a free software project and untar it with the implied -p
option *without* getting hurt.  Most of the time they simply trust
the file contents (who checks all source files after download?), so
why shouldn't they trust the file permissions, too?

So while it's not the smartest thing to do, it clearly is happening.
And perhaps we should not be the ones trying to educate the users of
software made by _our_ users on the safety of tar by defaulting to loose
permissions.

Let's be "safe by default", even if that particular definition of "safe"
means "a bit safer, but only if the untarring is done in a very unsafe
way". ;-)

> Having said that, I do not see much reason for anybody to want to
> extract any material that is worth to be placed under version control
> in a way that is world-writable, so I do not mind having 002 as the
> default, but I feel that group-writability should be kept under
> control of the umask of end users who know what they are doing.

Yes, using 002 is tempting.  But it's got the same "looseness" problems
as 000, only on a smaller scaler: there are certainly situations where a
user doesn't want to share write permissions with all the members of her
current group.  If we change the default, let's go all the way to 022.

> Historically we used to have 022 as the default, and IIRC we loosened
> it exactly because some people hated that we created files and
> directories closed to group members.

The situation has changed a bit in that we don't need to find one mask
that fits all users -- we only need to find a default value for the
config option tar.umask now.  If a project is unhappy with that value,
it can easily change it back to zero or to a different value.

Admittedly, that doesn't help users who download from a "022" project,
but really want something looser.  I think it's more important to
avoid violating the expectations of all those who freak out at 0666
permission modes attached to their freshly downloaded files, though.

Trivial patch follows.

René


diff --git a/archive-tar.c b/archive-tar.c
index af47fdc..ae84bcb 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -15,7 +15,7 @@ static char block[BLOCKSIZE];
 static unsigned long offset;
 
 static time_t archive_time;
-static int tar_umask;
+static int tar_umask = 0022;
 static int verbose;
 
 /* writes out the whole block, but only if it is full */

^ permalink raw reply related

* Re: git-svnimport failed and now git-repack hates me
From: Chris Lee @ 2007-01-05 20:48 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <20070105193958.GE8753@spearce.org>

On 1/5/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> git-prune is expensive.  Very expensive on very large projects,
> as it must iterate every object to decide what is needed, before
> it can start to remove objects that aren't needed.
>
> Yes, it doesn't deal with removing pack files.  That's what the -d
> to git-repack is for.

Not nearly as expensive as git-repack, that's for sure. :)

And - I originally thought that adding '-d' to git-repack just told it
to call 'git-prune' afterwards. It does more than that, which is cool.
Happily importing away - up to r320k now.

^ permalink raw reply

* Re: Adding spell checker to GIT
From: Johannes Schindelin @ 2007-01-05 20:49 UTC (permalink / raw)
  To: Deepak Barua; +Cc: git
In-Reply-To: <b5a19cd20701041246we052685hd700580df2cc120d@mail.gmail.com>

Hi,

On Fri, 5 Jan 2007, Deepak Barua wrote:

>         I and sasikumar have designed and built a spell checker into
> the pre-commit hook, could someone please test it and give us your
> comments.

It's a rather big chunk of code, and it is in perl, so I'd rather put it 
in another file, but oh well.

There are a few remaining problems, and I don't even know if the second 
can be solved:

- you miss "//" comments,
- if the diff is only about a small part of a comment, you do not spell 
  check that
- you do not spell check literal messages (i.e. printf("No such fiel!"))

Ciao,
Dscho

^ permalink raw reply

* Re: Default "tar" umask..
From: Junio C Hamano @ 2007-01-05 21:03 UTC (permalink / raw)
  Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <459EB78B.60000@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Junio C Hamano schrieb:
>> ...
>> Having said that, I do not see much reason for anybody to want to
>> extract any material that is worth to be placed under version control
>> in a way that is world-writable, so I do not mind having 002 as the
>> default, but I feel that group-writability should be kept under
>> control of the umask of end users who know what they are doing.
>
> Yes, using 002 is tempting.  But it's got the same "looseness" problems
> as 000, only on a smaller scaler: there are certainly situations where a
> user doesn't want to share write permissions with all the members of her
> current group.  If we change the default, let's go all the way to 022.

I don't think the above argument makes much sense -- it does not
explain why you do not go "all the way" to 077.

On the other hand, I can explain 002 fairly easily and
consistently.  This matters only for users who can become root
and does not know or care about implied -p, and the group root
belongs to had better not contain any suspicious user, so
leaving group open does not hurt.  022 actively hurts sane usage
(i.e. work always with a sane umask and extract as non root
users) while 002 does not.

> Admittedly, that doesn't help users who download from a "022" project,
> but really want something looser.  I think it's more important to
> avoid violating the expectations of all those who freak out at 0666
> permission modes attached to their freshly downloaded files, though.

Exactly, and that is why I think 002 is much saner.

^ permalink raw reply

* Re: [PATCH] gitweb: Fix shortlog only showing HEAD revision.
From: Michael Krufky @ 2007-01-05 21:21 UTC (permalink / raw)
  To: Robert Fitzsimons; +Cc: J.H., git, linux-kernel, Michael Krufky
In-Reply-To: <20070103202555.GA25768@localhost>

Robert Fitzsimons wrote:
> My change in 190d7fdcf325bb444fa806f09ebbb403a4ae4ee6 had a small bug
> found by Michael Krufky which caused the passed in hash value to be
> ignored, so shortlog would only show the HEAD revision.
> 
> Signed-off-by: Robert Fitzsimons <robfitz@273k.net>
> ---
> 
> Thanks for finding this Michael.  It' just a small bug introducted by a
> recent change I made.  Including John 'Warthog9' so hopefully he can add
> this to the version of gitweb which is hosted on kernel.org.
> 
> Robert

Robert,

Thank you for fixing this bug so quickly.  I've noticed that the gitweb
templates on kernel.org have changed at least once since you wrote this email to
me... (I can tell, based on the fact that the git:// link has moved from the
project column to a link labeled, "git" all the way to the right.)

Unfortunately, however, the bug that I had originally reported has not yet been
fixed on the kernel.org www server.  Either the patch in question hasn't yet
been applied to that installation, or it HAS in fact been applied, but doesn't
fix the problem as intended.

Do you know which of the above is true?

Thanks again,

Mike Krufky

>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index d845e91..2e94c2c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4423,7 +4423,7 @@ sub git_shortlog {
>  	}
>  	my $refs = git_get_references();
>  
> -	my @commitlist = parse_commits($head, 101, (100 * $page));
> +	my @commitlist = parse_commits($hash, 101, (100 * $page));
>  
>  	my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, (100 * ($page+1)));
>  	my $next_link = '';

^ permalink raw reply

* Re: [PATCH] gitweb: Fix shortlog only showing HEAD revision.
From: J.H. @ 2007-01-05 21:25 UTC (permalink / raw)
  To: Michael Krufky; +Cc: Robert Fitzsimons, git, linux-kernel, Michael Krufky
In-Reply-To: <459EC16F.1070809@linuxtv.org>

On Fri, 2007-01-05 at 16:21 -0500, Michael Krufky wrote:
> Robert Fitzsimons wrote:
> > My change in 190d7fdcf325bb444fa806f09ebbb403a4ae4ee6 had a small bug
> > found by Michael Krufky which caused the passed in hash value to be
> > ignored, so shortlog would only show the HEAD revision.
> > 
> > Signed-off-by: Robert Fitzsimons <robfitz@273k.net>
> > ---
> > 
> > Thanks for finding this Michael.  It' just a small bug introducted by a
> > recent change I made.  Including John 'Warthog9' so hopefully he can add
> > this to the version of gitweb which is hosted on kernel.org.
> > 
> > Robert
> 
> Robert,
> 
> Thank you for fixing this bug so quickly.  I've noticed that the gitweb
> templates on kernel.org have changed at least once since you wrote this email to
> me... (I can tell, based on the fact that the git:// link has moved from the
> project column to a link labeled, "git" all the way to the right.)
> 
> Unfortunately, however, the bug that I had originally reported has not yet been
> fixed on the kernel.org www server.  Either the patch in question hasn't yet
> been applied to that installation, or it HAS in fact been applied, but doesn't
> fix the problem as intended.

Simple answer - it's sitting in my tree waiting for me to have enough
time to get back to gitweb.  There are several things in flight and I'm
not prepared to push them out in their current state.

So yes the problem is fixed, but it will probably be sometime this
weekend before it gets pushed out to the kernel.org servers.

> 
> Do you know which of the above is true?
> 
> Thanks again,
> 
> Mike Krufky
> 
> >  gitweb/gitweb.perl |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index d845e91..2e94c2c 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -4423,7 +4423,7 @@ sub git_shortlog {
> >  	}
> >  	my $refs = git_get_references();
> >  
> > -	my @commitlist = parse_commits($head, 101, (100 * $page));
> > +	my @commitlist = parse_commits($hash, 101, (100 * $page));
> >  
> >  	my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, (100 * ($page+1)));
> >  	my $next_link = '';
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] wt-status: remove use_add_msg in wt_status_print
From: Michael Loeffler @ 2007-01-05 21:30 UTC (permalink / raw)
  To: git

If there is nothing to commit there is no need to say something about
adding files.

If there is something to commit or untracked or modified files, git
status will give this hint. In this case it is also redundant to say it
again.

Signed-off-by: Michael Loeffler <zvpunry@zvpunry.de>
---
 wt-status.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index db42738..019ec46 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -292,9 +292,7 @@ void wt_status_print(struct wt_status *s)
 	if (s->verbose && !s->is_initial)
 		wt_status_print_verbose(s);
 	if (!s->commitable)
-		printf("%s (%s)\n",
-			s->amend ? "# No changes" : "nothing to commit",
-			use_add_msg);
+		printf("%s\n", s->amend ? "# No changes" : "nothing to commit");
 }
 
 int git_status_config(const char *k, const char *v)
-- 
1.5.0.rc0.g1fa2

^ permalink raw reply related

* Re: [PATCH 1/2] Suggest use of "git add file1 file2" when there is nothing to commit.
From: Michael Loeffler @ 2007-01-05 20:44 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061216025309.GA19955@spearce.org>

Hi,

Am Freitag, den 15.12.2006, 21:53 -0500 schrieb Shawn O. Pearce:
...
> @@ -292,7 +291,9 @@ void wt_status_print(struct wt_status *s)
>  	if (s->verbose && !s->is_initial)
>  		wt_status_print_verbose(s);
>  	if (!s->commitable)
> -		printf("%s\n", s->amend ? "# No changes" : "nothing to commit");
> +		printf("%s (%s)\n",
> +			s->amend ? "# No changes" : "nothing to commit",
> +			use_add_msg);
>  }
I don't like the new 'nothing to commit (use "git add ... message")'
message. I use git status very often to see if there is something to
commit, but now there is always this annoying "use git add ..." message.
I just want to see on what the branch is and if there is something to
commit.

If there is something to commit I get the list of untracked or modified
files with the use_add_msg and if I try to commit an empty tree as
initial commit I get the message from git-commit.sh.


bye

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Junio C Hamano @ 2007-01-05 21:37 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Chris Lee, Linus Torvalds
In-Reply-To: <20070105193958.GE8753@spearce.org>

Subject: [PATCH] builtin-prune: memory diet.

Somehow we forgot to turn save_commit_buffer off while walking
the reachable objects.  Releasing the memory for commit object
data that we do not use matters for large projects (for example,
about 90MB is saved while traversing linux-2.6 history).

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * The linux-2.6 history number for me is inflated because I
   have grafts that connects historical archive behind the
   current v2.6.12-rc2 based history...

 builtin-prune.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-prune.c b/builtin-prune.c
index 00a53b3..b469c43 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -253,6 +253,8 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
 		usage(prune_usage);
 	}
 
+	save_commit_buffer = 0;
+
 	/*
 	 * Set up revision parsing, and mark us as being interested
 	 * in all object types, not just commits.

^ permalink raw reply related

* Re: Default "tar" umask..
From: Linus Torvalds @ 2007-01-05 21:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vzm8xdw3t.fsf@assigned-by-dhcp.cox.net>



On Fri, 5 Jan 2007, Junio C Hamano wrote:
> >
> > Yes, using 002 is tempting.  But it's got the same "looseness" problems
> > as 000, only on a smaller scaler: there are certainly situations where a
> > user doesn't want to share write permissions with all the members of her
> > current group.  If we change the default, let's go all the way to 022.
> 
> I don't think the above argument makes much sense -- it does not
> explain why you do not go "all the way" to 077.

I really think that 022 is the right choice, for a very simple reason: 
peoples expectations. It's just _common_.

> On the other hand, I can explain 002 fairly easily and
> consistently.

No you can't. 002 makes no sense at all in a very common old-fashioned 
setup with a "user" group. 

Maybe I'm old, and these days most setups seem to give people their own 
group (so I'm "torvalds:torvalds" on all the machines I have access to), 
but it used to be _very_ common to have just a "user" group that all 
normal users were part of (or have the default gid depend on something 
like which department you are in).

In that situation, 002 is really effectively no different at all from 000.

Which is why 022 is the historical default for umask. 

022 really is very easy to explain: "readability (and executability) is a 
lot less dangerous than writability, and by default we only give 
writability to the user". That's why we _don't_ commonly have 066 or 077 
as the umask, and also why 002 is the default umask ONLY on systems where 
users have their own individual groups by default.

		Linus

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Linus Torvalds @ 2007-01-05 21:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Chris Lee
In-Reply-To: <7vtzz5duk1.fsf@assigned-by-dhcp.cox.net>



On Fri, 5 Jan 2007, Junio C Hamano wrote:
> 
> Somehow we forgot to turn save_commit_buffer off while walking
> the reachable objects.  Releasing the memory for commit object
> data that we do not use matters for large projects (for example,
> about 90MB is saved while traversing linux-2.6 history).

Heh. Maybe we should just make the default the other way? It's probably 
pretty easy to find any users that suddenly start segfaulting ;)

(and just setting it in "cmd_log_init" would likely catch quite a number 
of them already).

		Linus

^ permalink raw reply

* Re: Default "tar" umask..
From: Junio C Hamano @ 2007-01-05 22:15 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0701051336000.3661@woody.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 5 Jan 2007, Junio C Hamano wrote:
>> ...
>> On the other hand, I can explain 002 fairly easily and
>> consistently.
>
> No you can't. 002 makes no sense at all in a very common old-fashioned 
> setup with a "user" group. 

I do not think so (see below).

> Maybe I'm old, and these days most setups seem to give people their own 
> group (so I'm "torvalds:torvalds" on all the machines I have access to), 
> but it used to be _very_ common to have just a "user" group that all 
> normal users were part of (or have the default gid depend on something 
> like which department you are in).
>
> In that situation, 002 is really effectively no different at all from 000.

I remember those days.  People had 022 umask for that exact
reason, as you said, in such a setup.  It was quite common.  On
the other hand, modern setups often use "own" group and people
often use 002 umask.

If you extract as a normal user (i.e. without -p) then 002 is
really effectively no different at all from 000 because umask
kicks in and give the results the user would expect in either
setups, which is good.  In "user" group setup, umask 022 makes
files to 0644, in "own" group setup, umask 002 makes files to
0664.  All is good.  If the archive is made with 022, that would
break expectation of users whose umask is 002 (a sane value in
modern "own" group setups).

The current 000 was bad for users who work as root and do not
know about implied -p (which is not their fault).  When
extracting as root, the files and directories are owned by
'root' and its group.

Even in the old "user" group setups, I _thought_ the root was in
his own group or wheel in BSD, and the group was not shared with
Joe Random users, so if that is the case, group writability is
not an problem.  In the modern "own" group setups, the root user
is in its own his group 'root', so group writability is not an
issue either.

> 022 really is very easy to explain: "readability (and executability) is a 
> lot less dangerous than writability, and by default we only give 
> writability to the user". That's why we _don't_ commonly have 066 or 077 
> as the umask, and also why 002 is the default umask ONLY on systems where 
> users have their own individual groups by default.

077 was a tongue-in-cheek comment.

I think we are basing our reasoning with the same shared
understanding of historical practice of "user" group.  I wonder
why the differenece in conclusions.

Maybe my recollection of historical practice was faulty and the
root shared its group with Joe Random users?  If so, I would
agree that 002 makes no sense at all, as you said.

^ permalink raw reply

* Re: [BUG] stgit: unexpected "unknown user details" on refresh
From: Yann Dirson @ 2007-01-05 22:29 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: GIT list
In-Reply-To: <b0943d9e0701050107s21ec8527ud89425df4772998@mail.gmail.com>

On Fri, Jan 05, 2007 at 09:07:51AM +0000, Catalin Marinas wrote:
> It probably tries to set the author information for the patch and
> GIT_AUTH* or GIT_COMMIT* aren't set but it would be probably useful to
> get the full backtrace of this error (at the end of the stgit/main.py
> file, just add a "raise" before sys.exit(2)).

Email vars are set, name is taken (by git) from GECOS.  Apparently
StGIT does not use the latter.  It would surely be more user-friendly
to use it, as much for providing a (hopefully) sane default as to get
a behaviour consistent with GIT.

$ env|grep GIT
GIT_COMMITTER_EMAIL=ydirson@altern.org
GIT_AUTHOR_EMAIL=ydirson@altern.org

As you expected:

Traceback (most recent call last):
  File "./stg", line 43, in ?
    main()
  File "/export/work/yann/git/stgit/stgit/main.py", line 262, in main
    command.func(parser, options, args)
  File "/export/work/yann/git/stgit/stgit/commands/refresh.py", line 145, in func
    backup = True, sign_str = sign_str)
  File "/export/work/yann/git/stgit/stgit/stack.py", line 674, in refresh_patch
    patch.set_authdate(author_date)
  File "/export/work/yann/git/stgit/stgit/stack.py", line 265, in set_authdate
    self._set_field('authdate', date or git.author().date)
  File "/export/work/yann/git/stgit/stgit/git.py", line 475, in author
    __author = user()
  File "/export/work/yann/git/stgit/stgit/git.py", line 457, in user
    raise GitException, 'unknown user details'
stgit.git.GitException: unknown user details

Best regards,
-- 
Yann.

^ permalink raw reply

* Re: Default "tar" umask..
From: René Scharfe @ 2007-01-05 22:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <7vzm8xdw3t.fsf@assigned-by-dhcp.cox.net>

[Junio: Sorry for resending, I somehow dropped the CC:'s the first time.]

Junio C Hamano schrieb:
>>> >>> Having said that, I do not see much reason for anybody to want to
>>> >>>  extract any material that is worth to be placed under version
>>> >>> control in a way that is world-writable, so I do not mind having
>>> >>> 002 as the default, but I feel that group-writability should be
>>> >>> kept under control of the umask of end users who know what they
>>> >>> are doing.
>> >> Yes, using 002 is tempting.  But it's got the same "looseness"
>> >> problems as 000, only on a smaller scaler: there are certainly
>> >> situations where a user doesn't want to share write permissions
>> >> with all the members of her current group.  If we change the
>> >> default, let's go all the way to 022.
> > 
> > I don't think the above argument makes much sense -- it does not 
> > explain why you do not go "all the way" to 077.

Well, what I had in mind were free software projects and simple users,
i.e. publicly hosted tar files and users that only download and extract
them, and then don't add confidential changes afterwards.  You're right,
of course: why stop there?  077 would be safest. :->

> > On the other hand, I can explain 002 fairly easily and consistently.
> > This matters only for users who can become root and does not know or
> > care about implied -p, and the group root belongs to had better not
> > contain any suspicious user, so leaving group open does not hurt.
> > 022 actively hurts sane usage (i.e. work always with a sane umask and
> > extract as non root users) while 002 does not.

Hm, right, I was not thinking straight -- I didn't see that the gid of
the extracted files will be set to 0 by tar (as specified in our tar
files).  Err, unless the target system has a group named git, which
would then be used instead.  Come to think of it, having this "git"
group name in there is a bit strange and unnecessary.  How about the
following patch?


In order to make the generated tar files more friendly to users who
extract them as root using GNU tar and its implied -p option, change
the default umask to 002 and change the owner name and group name to
root.  This ensures that a) the extracted files and directories are
not world-writable and b) that they belong to user and group root.

Before they would have been assigned to a user and/or group named
git if it existed.  This also answers the question in the removed
comment: uid=0, gid=0, uname=root, gname=root is exactly what we
want.

Normal users who let tar apply their umask while extracting are
only affected if their umask allowed the world to change their
files (e.g. a umask of zero).  This case is so unlikely and strange
that we don't need to support it.

Credit goes to Junio for finding the ideal default umask of 002
through sheer logic.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

---
 archive-tar.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index af47fdc..7d52a06 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -15,7 +15,7 @@ static char block[BLOCKSIZE];
 static unsigned long offset;
 
 static time_t archive_time;
-static int tar_umask;
+static int tar_umask = 002;
 static int verbose;
 
 /* writes out the whole block, but only if it is full */
@@ -210,11 +210,10 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 	sprintf(header.size, "%011lo", S_ISREG(mode) ? size : 0);
 	sprintf(header.mtime, "%011lo", archive_time);
 
-	/* XXX: should we provide more meaningful info here? */
 	sprintf(header.uid, "%07o", 0);
 	sprintf(header.gid, "%07o", 0);
-	strlcpy(header.uname, "git", sizeof(header.uname));
-	strlcpy(header.gname, "git", sizeof(header.gname));
+	strlcpy(header.uname, "root", sizeof(header.uname));
+	strlcpy(header.gname, "root", sizeof(header.gname));
 	sprintf(header.devmajor, "%07o", 0);
 	sprintf(header.devminor, "%07o", 0);
 

^ permalink raw reply related

* Re: [PATCH 1/2] Suggest use of "git add file1 file2" when there is nothing to commit.
From: Junio C Hamano @ 2007-01-05 22:33 UTC (permalink / raw)
  To: Michael Loeffler; +Cc: git, Shawn O. Pearce
In-Reply-To: <1168029891.11130.18.camel@ibook.zvpunry.de>

Michael Loeffler <zvpunry@zvpunry.de> writes:

> Am Freitag, den 15.12.2006, 21:53 -0500 schrieb Shawn O. Pearce:
> ...
>> +		printf("%s (%s)\n",
>> +			s->amend ? "# No changes" : "nothing to commit",
>> +			use_add_msg);
>>  }
> I don't like the new 'nothing to commit (use "git add ... message")'
> message. I use git status very often to see if there is something to
> commit, but now there is always this annoying "use git add ..." message.

I tend to not like _ANY_ change at all, but I've learned to wait
and see if I get used to it when I see something that annoys me
initially, to see if the annoyance is because what it does is
truly wrong or it is because what it does is merely different
from what I am used to.

So I've been trying it out myself as one of the guinea pigs on
this one as well.

So far, my judgement is that this is of the better kind; it is
easy to get used to, and once you get used to it, it is easily
ignorable.

Other people's tastes may differ, of course.

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Linus Torvalds @ 2007-01-05 22:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Chris Lee
In-Reply-To: <Pine.LNX.4.64.0701051354590.3661@woody.osdl.org>



On Fri, 5 Jan 2007, Linus Torvalds wrote:
> 
> Heh. Maybe we should just make the default the other way? It's probably 
> pretty easy to find any users that suddenly start segfaulting ;)

This seems to pass all the tests, at least.

(But I didn't test the SVN stuff, since I don't have perl::SVN installed)

		Linus
---
diff --git a/builtin-branch.c b/builtin-branch.c
index d3df5a5..0b662a8 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -441,6 +441,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	    (rename && force_create))
 		usage(builtin_branch_usage);
 
+	if (verbose)
+		save_commit_buffer = 1;
+
 	head = xstrdup(resolve_ref("HEAD", head_sha1, 0, NULL));
 	if (!head)
 		die("Failed to resolve HEAD as a valid ref.");
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 24cb2d7..212ad59 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -67,6 +67,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	static struct rev_info *opt = &log_tree_opt;
 	int read_stdin = 0;
 
+	save_commit_buffer = 1;
 	init_revisions(opt, prefix);
 	git_config(git_default_config); /* no "diff" UI options */
 	nr_sha1 = 0;
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 87d3d63..4053651 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -251,6 +251,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	unsigned char head_sha1[20];
 	const char *current_branch;
 
+	save_commit_buffer = 1;
 	git_config(fmt_merge_msg_config);
 
 	while (argc > 1) {
diff --git a/builtin-log.c b/builtin-log.c
index a59b4ac..ac95921 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -22,6 +22,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 {
 	int i;
 
+	save_commit_buffer = 1;
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
@@ -372,6 +373,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.ignore_merges = 1;
 	rev.diffopt.msg_sep = "";
 	rev.diffopt.recursive = 1;
+	save_commit_buffer = 1;
 
 	rev.extra_headers = extra_headers;
 
@@ -569,6 +571,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	const char *limit = NULL;
 	int verbose = 0;
 
+	save_commit_buffer = 1;
 	if (argc > 1 && !strcmp(argv[1], "-v")) {
 		verbose = 1;
 		argc--;
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index c67f2fa..53d1b29 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -586,6 +586,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	int dense = 1;
 	int reflog = 0;
 
+	save_commit_buffer = 1;
 	git_config(git_show_branch_config);
 
 	/* If nothing is specified, try the default first */
diff --git a/commit.c b/commit.c
index 2a58175..660d365 100644
--- a/commit.c
+++ b/commit.c
@@ -4,7 +4,7 @@
 #include "pkt-line.h"
 #include "utf8.h"
 
-int save_commit_buffer = 1;
+int save_commit_buffer = 0;
 
 struct sort_node
 {
diff --git a/merge-recursive.c b/merge-recursive.c
index bac16f5..b98ed1a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1286,6 +1286,7 @@ int main(int argc, char *argv[])
 	const char *branch1, *branch2;
 	struct commit *result, *h1, *h2;
 
+	save_commit_buffer = 1;
 	git_config(git_default_config); /* core.filemode */
 	original_index_file = getenv(INDEX_ENVIRONMENT);
 
diff --git a/revision.c b/revision.c
index 6e4ec46..aa10088 100644
--- a/revision.c
+++ b/revision.c
@@ -737,6 +737,7 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token
 		opt->pattern_tail = &(opt->pattern_list);
 		opt->regflags = REG_NEWLINE;
 		revs->grep_filter = opt;
+		save_commit_buffer = 1;
 	}
 	append_grep_pattern(revs->grep_filter, ptn,
 			    "command line", 0, what);

^ permalink raw reply related

* Re: Default "tar" umask..
From: Junio C Hamano @ 2007-01-05 22:40 UTC (permalink / raw)
  To: René Scharfe; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <459ED17E.2080101@lsrfire.ath.cx>

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> [Junio: Sorry for resending, I somehow dropped the CC:'s the first time.]

I think that was my fault.  I somehow dropped the CC:'s and
resent without you to the list, and you responded to the first
one.

> ...  Err, unless the target system has a group named git, which
> would then be used instead.  Come to think of it, having this "git"
> group name in there is a bit strange and unnecessary.  How about the
> following patch?

It is a very good point.  I think this patch is sane, regardless
of 002 vs 022 issue.

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: alan @ 2007-01-05 22:18 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Shawn O. Pearce, git, Chris Lee
In-Reply-To: <Pine.LNX.4.64.0701051354590.3661@woody.osdl.org>

On Fri, 5 Jan 2007, Linus Torvalds wrote:

>
>
> On Fri, 5 Jan 2007, Junio C Hamano wrote:
>>
>> Somehow we forgot to turn save_commit_buffer off while walking
>> the reachable objects.  Releasing the memory for commit object
>> data that we do not use matters for large projects (for example,
>> about 90MB is saved while traversing linux-2.6 history).
>
> Heh. Maybe we should just make the default the other way? It's probably
> pretty easy to find any users that suddenly start segfaulting ;)

I am trying to import a subversion repository and have yet to be able to 
suck down the whole thing without segfaulting.  It is a large repository. 
Works fine until about the last 10% and then runs out of memory.

open3: fork failed: Cannot allocate memory at /usr/bin/git-svn line 2711
512 at /usr/bin/git-svn line 446
         main::fetch_lib() called at /usr/bin/git-svn line 314
         main::fetch() called at /usr/bin/git-svn line 173

I need to try the "partial download" script and see if that helps.

-- 
"Invoking the supernatural can explain anything, and hence explains nothing."
                   - University of Utah bioengineering professor Gregory Clark

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Junio C Hamano @ 2007-01-05 22:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Shawn O. Pearce, git, Chris Lee
In-Reply-To: <Pine.LNX.4.64.0701051439060.3661@woody.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Fri, 5 Jan 2007, Linus Torvalds wrote:
>> 
>> Heh. Maybe we should just make the default the other way? It's probably 
>> pretty easy to find any users that suddenly start segfaulting ;)
>
> This seems to pass all the tests, at least.
>
> (But I didn't test the SVN stuff, since I don't have perl::SVN installed)

I do not think we have too many branch refs (builtin-branch and
builtin-show-branch) for this patch to make any practical
difference, but I wonder why this is needed...

> diff --git a/merge-recursive.c b/merge-recursive.c
> index bac16f5..b98ed1a 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1286,6 +1286,7 @@ int main(int argc, char *argv[])
>  	const char *branch1, *branch2;
>  	struct commit *result, *h1, *h2;
>  
> +	save_commit_buffer = 1;
>  	git_config(git_default_config); /* core.filemode */
>  	original_index_file = getenv(INDEX_ENVIRONMENT);

Ah, there are those annoying "using this as the merge base whose
commit log is..." business.  I wonder if anybody is actually
reading them (I once considered squelching that output).

^ permalink raw reply

* Re: [PATCH] [gitweb] Blame "linenr" link jumps to previous state at "orig_lineno"
From: Luben Tuikov @ 2007-01-05 22:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk602jfo7.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > A quick show-and-tell of this patch is after applying it, open
> > "blame" on a file, click on a linenr link of a block which
> > has only a single line changed -- the diff between what
> > you're seeing now and before is at least that line.  This you can
> > data-mine with gitweb back to where the line was introduced.  Similar
> > argument applies to blocks of more than one line.
> 
> One non-question and one question:
> 
>  - This favors the first parent, which is obviously the right
>    thing for most of the time.  I wonder what happens to a
>    merge, though, but I realize that a line attributed to a
>    merge is even rarer, and such a line is introduced by the
>    "evil merge".
> 
>  - I wonder if the line number is correct for the parent
>    commit.  How well does this work when you clicked a line that
>    was added at the end of the file, where the $orig_lineno goes
>    beyond the whole file in the parent?

It displayed the whole file, since the previous state of the
file I was looking at was short.  In effect for the case in
point it displays the bottom of the file.

If you remember, this had been my "white whale" for about a
year and a half now.  I accidentally "got it" while using
git-blame from shell prompt.

I think it's a worthwhile thing to have in gitweb.

   Luben

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Linus Torvalds @ 2007-01-05 23:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Chris Lee
In-Reply-To: <7vac0xdr97.fsf@assigned-by-dhcp.cox.net>



On Fri, 5 Jan 2007, Junio C Hamano wrote:
> 
> I do not think we have too many branch refs (builtin-branch and
> builtin-show-branch) for this patch to make any practical
> difference

Yeah, it's mainly a "safety thing" - have the default be the "don't waste 
memory".

> but I wonder why this is needed...
> 
> > diff --git a/merge-recursive.c b/merge-recursive.c
> > index bac16f5..b98ed1a 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -1286,6 +1286,7 @@ int main(int argc, char *argv[])
> >  	const char *branch1, *branch2;
> >  	struct commit *result, *h1, *h2;
> >  
> > +	save_commit_buffer = 1;
> >  	git_config(git_default_config); /* core.filemode */
> >  	original_index_file = getenv(INDEX_ENVIRONMENT);
> 
> Ah, there are those annoying "using this as the merge base whose
> commit log is..." business.  I wonder if anybody is actually
> reading them (I once considered squelching that output).

"output_commit_title()" used it. Not just for the merge base, but for the 
regular "merging X and Y" messages, I think.

		Linus

^ permalink raw reply

* Re: git-svnimport failed and now git-repack hates me
From: Linus Torvalds @ 2007-01-05 23:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git, Chris Lee
In-Reply-To: <Pine.LNX.4.64.0701051457020.3661@woody.osdl.org>



On Fri, 5 Jan 2007, Linus Torvalds wrote:
> 
> Yeah, it's mainly a "safety thing" - have the default be the "don't waste 
> memory".

Btw, I'm not at all certain whether it's necessary or a good thing. I just 
decided to see how many people really seem to use the commit messages at 
all. So feel free to throw the patch away if you don't think this is 
worthwhile, I won't push it.

		Linus

^ 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