Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] remote-curl.c: fix rpc_out()
From: Johannes Schindelin @ 2009-11-23 10:39 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Sverre Rabbelier, git, Shawn O. Pearce, Junio C Hamano
In-Reply-To: <be6fef0d0911230038y13c23e3ek5591ee0dc3eaa47a@mail.gmail.com>

Hi,

On Mon, 23 Nov 2009, Tay Ray Chuan wrote:

> On Mon, Nov 23, 2009 at 1:53 PM, Sverre Rabbelier <srabbelier@gmail.com> 
> wrote:
> > Seems like this type of patch would do very well with a test case or 
> > two?
> 
> ah, but to trigger the code involved, a sufficiently large test 
> repository is needed. The git repository would be enough. :)

I guess you meant "not be enough", as an int can hold a pretty large 
number until it turns negative.

So I think in this case it is more harm- than helpful to have a test case.

For future reference: if you need a repository with special featurs 
for testing, it is best to generate it in a test script (see the many test 
cases labeled 'setup' in our test suite for examples).

Ciao,
Dscho

^ permalink raw reply

* Re: 'error: unable to set permission to './objects/...'
From: Junio C Hamano @ 2009-11-23 10:54 UTC (permalink / raw)
  To: Rafal Rusin; +Cc: git
In-Reply-To: <9bbf67fa0911230135j7cfe5bcem991e750b6754f344@mail.gmail.com>

Rafal Rusin <rafal.rusin@gmail.com> writes:

> As for detecting this particular case, I think it's not possible.

Why not?

> I think the best solution is to add repository config switch like
> 'usefilepermissions' true by default. If set to false, git would skip
> chmod during push.
> Does that make sense to you?

Not at all.  That is like creating an "allow broken behaviour" option and
hoping for the best.

You shouldn't have to invent such a configuration variable to begin with,
as "let umask set whatever permission and leave it be" should already be
the case for !core.sharedrepository.  There is something wrong in your
set up and you need to get to the bottom of it, instead of coming up with
an even worse breakage as a unworkable workaround.

There are some things I noticed while looking at the codepaths that are
involved.

move_temp_to_file() is designed to move a temporary file that was created
by odb_mkstemp().  As the latter eventually uses mkstemp(), some
implementations of which ignore umask and create a file that is readable
only by the user, move_temp_to_file() must loosen the permission, even in
a private repository that honors umask (i.e. not in a shared repository)..

    Side note.  The creation of the temporary files in http.c that are
    given to move_temp_to_file() is not quite correct; they are created by
    fopen() without proper locking with mkstemp().  But that is a separate
    issue.

But the codepath tries to loosen it a bit too much.  Even if user's umask
is 077, files created in this codepath end up with world-readable because
we pretend that lstat() on the file returned 0444 (that is what a non-zero
value given as the second parameter to set_shared_perm() means).  We
should tighten it perhaps like the attached patch does.

In case it is not obvious, this patch is _not_ meant to help you work
around the chmod() failure you are seeing on your filesystem.  You need to
first see _why_ it fails for you, in order to come closer to the real
solution.

-- >8 --
Subject: [PATCH] move_temp_to_file(): don't loosen permission too much

We always feed 0444 as the second parameter to set_shared_perm() when
finalizing a temporary file we created using mkstemp(), as some versions
of glibc creates a temporary file with too tight a permission, ignoring
the user's umask.  As the second parameter tells set_shared_perm() to
pretend that it is the permission bits the file currently has (i.e. what
should have been set by the user's umask), we should make it just as
restrictive as user's umask suggests.

---
 sha1_file.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 63981fb..f0b8969 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2233,6 +2233,21 @@ static void write_sha1_file_prepare(const void *buf, unsigned long len,
 	git_SHA1_Final(sha1, &c);
 }
 
+static int fix_tempfile_mode(const char *filename)
+{
+	static mode_t user_mode;
+
+	if (!user_mode) {
+		user_mode = umask(0);
+		umask(user_mode);
+		user_mode = S_IFREG | ((user_mode ^ 0777) & 0666);
+	}
+
+	if (!set_shared_perm(filename, user_mode))
+		return 0;
+	return error("unable to set permission to '%s'", filename);
+}
+
 /*
  * Move the just written object into its final resting place.
  * NEEDSWORK: this should be renamed to finalize_temp_file() as
@@ -2274,9 +2289,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
 	}
 
 out:
-	if (set_shared_perm(filename, (S_IFREG|0444)))
-		return error("unable to set permission to '%s'", filename);
-	return 0;
+	return fix_tempfile_mode(filename);
 }
 
 static int write_buffer(int fd, const void *buf, size_t len)

^ permalink raw reply related

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Carlo Marcelo Arenas Belon @ 2009-11-23 11:22 UTC (permalink / raw)
  To: Ren? Scharfe; +Cc: Junio C Hamano, Bert Wesarg, git
In-Reply-To: <4B095F91.8030305@lsrfire.ath.cx>

On Sun, Nov 22, 2009 at 04:58:09PM +0100, Ren? Scharfe wrote:
> Junio C Hamano schrieb:
> > Do we kill that environment variable when we call out to external grep in
> > grep.c?  If not, we should.  An alternative is to teach our internal one
> > to also honor it, but I personally do not find it too attractive to mimic
> > the design mistake of GREP_OPTIONS myself.
> 
> We don't.  Here's a patch with a simple test case that makes git grep
> unset GREP_OPTIONS before it calls the external grep.
> 
> While we're at it, also unset GREP_COLOR and GREP_COLORS in case
> colouring is not enabled, to be on the safe side.  The presence of
> these variables alone is not sufficient to trigger coloured output with
> GNU grep, but other implementations may behave differently.

why not better to apply the proposed patch from Junio in :

  http://article.gmane.org/gmane.comp.version-control.git/127980/

it would IMHO correct all reported issues and serve as well as a catch
all from other tools that could be introduced in the future and that
will be similarly affected by this misfeature.

Carlo

^ permalink raw reply

* git describe oddity: ignoring recent tags...
From: Martin Langhoff @ 2009-11-23 12:01 UTC (permalink / raw)
  To: Git Mailing List

Clone this repo: git://dev.laptop.org/projects/olpc-update -- or just
check visually the recent history here
http://dev.laptop.org/git/projects/olpc-update/log/

git describe origin/master will respond olpc-update-2.16-20-g2d4e4b8
when it is fairly clear to me that it should be
olpc-update-2.19-1g<hash>.

For some reason, recent tags are being ignored -- and cgit even
displays them differently in
http://dev.laptop.org/git/projects/olpc-update/log/ though it is
unclear to me why.

These tags are not 'lightweight', and no tags are signed either.
Passing --tags and --debug has not helped much.

Tested with 1.6.0.6 (Fedora 9 rpm) and 1.6.3.1.26.gf5b223 (on Karmic).



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

^ permalink raw reply

* Re: git describe oddity: ignoring recent tags...
From: Björn Steinbrink @ 2009-11-23 12:30 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Git Mailing List
In-Reply-To: <46a038f90911230401sb2b1dd7u21b5d7edefe510eb@mail.gmail.com>

On 2009.11.23 13:01:51 +0100, Martin Langhoff wrote:
> Clone this repo: git://dev.laptop.org/projects/olpc-update -- or just
> check visually the recent history here
> http://dev.laptop.org/git/projects/olpc-update/log/
> 
> git describe origin/master will respond olpc-update-2.16-20-g2d4e4b8
> when it is fairly clear to me that it should be
> olpc-update-2.19-1g<hash>.
> 
> For some reason, recent tags are being ignored -- and cgit even
> displays them differently in
> http://dev.laptop.org/git/projects/olpc-update/log/ though it is
> unclear to me why.
> 
> These tags are not 'lightweight', and no tags are signed either.
> Passing --tags and --debug has not helped much.

They are lightweight:
$ git cat-file -t olpc-update-2.19
commit

And using --tags "helps" here:
$ git describe 
olpc-update-2.16-20-g2d4e4b8

$ git describe --tags
olpc-update-2.19-3-g2d4e4b8

Bjoern

^ permalink raw reply

* Re: git describe oddity: ignoring recent tags...
From: Martin Langhoff @ 2009-11-23 12:38 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Git Mailing List
In-Reply-To: <20091123123048.GA10172@atjola.homenet>

2009/11/23 Björn Steinbrink <B.Steinbrink@gmx.de>:
> They are lightweight:
> $ git cat-file -t olpc-update-2.19
> commit

Hmmm. I thought a tag with a _message_ was not lightweight.

$ git tag -ln
...
olpc-update-2.16 Release 2.16
olpc-update-2.17 Release 2.17.
olpc-update-2.18 Rlease 2.18.
olpc-update-2.19 Release 2.19.

Argh... I see now - git tag is being helpful and showing the first
line of the commit msg instead of the tag msg, so I thought they were
full tags.

DWIM bites again! ;-(

> And using --tags "helps" here:
> $ git describe
> olpc-update-2.16-20-g2d4e4b8
>
> $ git describe --tags
> olpc-update-2.19-3-g2d4e4b8

Right. Doesn't make a difference with 1.6.0.6 but it does with
1.6.3.1.26.gf5b223



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

^ permalink raw reply

* Re: git describe oddity: ignoring recent tags...
From: Thomas Rast @ 2009-11-23 12:39 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: Martin Langhoff, Git Mailing List
In-Reply-To: <20091123123048.GA10172@atjola.homenet>

Björn Steinbrink wrote:
> On 2009.11.23 13:01:51 +0100, Martin Langhoff wrote:
> > git describe origin/master will respond olpc-update-2.16-20-g2d4e4b8
> > when it is fairly clear to me that it should be
> > olpc-update-2.19-1g<hash>.
[...]
> They are lightweight:
> $ git cat-file -t olpc-update-2.19
> commit
> 
> And using --tags "helps" here:
> $ git describe 
> olpc-update-2.16-20-g2d4e4b8
> 
> $ git describe --tags
> olpc-update-2.19-3-g2d4e4b8

Note that Gits that do not have 7e425c4 (describe: Make --tags and
--all match lightweight tags more often, 2008-10-13; first released in
1.6.1) will prefer the annotated tag over the unannotated ones even
with --tags, which explains your observation

> > For some reason, recent tags are being ignored -- and cgit even
> > displays them differently in
> > http://dev.laptop.org/git/projects/olpc-update/log/ though it is
> > unclear to me why.

at least with the first Git version mentioned:

> > Tested with 1.6.0.6 (Fedora 9 rpm) and 1.6.3.1.26.gf5b223 (on Karmic).

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: Android needs repo, Chrome needs gclient. Neither work. What does that say about git?
From: tytso @ 2009-11-23 13:58 UTC (permalink / raw)
  To: Adrian May; +Cc: git, chromium-discuss
In-Reply-To: <65e170e70911222011l776a6aean7bd75f072a806616@mail.gmail.com>

On Mon, Nov 23, 2009 at 12:11:29PM +0800, Adrian May wrote:
> As for gclient and repo, without pretending to be an expert on what
> they actually do, I'm getting a strong gut feeling that if what I'm
> trying to do is pull or push code, then that's about as close as you
> can get to a definition of source control's central purpose. In the
> days of cvs or svn, I'd expect to use the source control for that. How
> come git needs help?

> > these "bolt-on scripts" give the savvy user freedom
> 
> Actually, I think their purpose is precisely the opposite: to regiment
> the ordinary developer into following their process. So having that
> code under the developer's control is a weakness.

If you don't have bolt-on scripts, and you move that into the the core
SCM, then you force *all* projects to use whatever workflow was
decided as being the One True Way of doing things as seen by the SCM
designer.  So the question is whether the SCM *should* regiment all
projects into following the SCM's designers idea of the One True
Workflow.

Git's approach is to say that it will be fairly flexible about
dictating workflow --- who pushs to whom; whether there is a single
"star" repository topology, or something that is more flexible, etc.

You seem to hate this flexibility, because it makes life harder until
you set up these bolt-on scripts.  But that's what many of us like
about git; that it doesn't force us (the project lead) into a single
way of doing things.

As far as my wanting to impose a particular regimen on my project's
developers, I've never been a big fan of the Bondage and Discpline
school of software engineering.  They can use whatever workflow they
like; they just have to deliver patches that are clean.  If they are,
I'll pull from their repository.  If they aren't, I won't.

     	       	     		     	  	    - Ted

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Jeff King @ 2009-11-23 15:00 UTC (permalink / raw)
  To: bill lam; +Cc: Nicolas Pitre, git
In-Reply-To: <20091122145352.GA3941@debian.b2j>

On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote:

> I set crontab to push to another computer for backup. It sent
> confirmation email after finished.  It looked like
> 
> Counting objects: 1
> Counting objects: 9, done.
> Delta compression using up to 2 threads.
> Compressing objects:  20% (1/5)
> Compressing objects:  40% (2/5)
> Compressing objects:  60% (3/5)
> Compressing objects:  80% (4/5)
> Compressing objects: 100% (5/5)
> Compressing objects: 100% (5/5), done.
> Writing objects:  20% (1/5)
> Writing objects:  40% (2/5)
> Writing objects:  60% (3/5)
> Writing objects:  80% (4/5)
> Writing objects: 100% (5/5)
> Writing objects: 100% (5/5), 549 bytes, done.
> Total 5 (delta 3), reused 0 (delta 0)
> 
> Often the list of progress % can be a page long.  I want output but
> not those percentage progress status.  Will that be possible?

Hmm. There seems to be a bug. pack-objects is supposed to see that
stderr is not a tty and suppress the progress messages. But it doesn't,
because send-pack gives it the --all-progress flag, which
unconditionally tells it to display progress, when the desired impact is
actually to just make the progress more verbose.

We need to do one of:

  1. make --all-progress imply "if we are using progress, then make it
     more verbose. Otherwise, ignore."

  2. fix all callers to check isatty(2) before unconditionally passing
     the option

The patch for (1) would look something like what's below.  It's simpler,
but it does change the semantics; anyone who was relying on
--all-progress to turn on progress unconditionally would need to now
also use --progress. However, turning on progress unconditionally is
usually an error (the except is if you are piping output in real-time to
the user and need to overcome the isatty check).

Nicolas, this is your code. Which do you prefer?

-Peff

---
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 4c91e94..50dd429 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -75,6 +75,7 @@ static int ignore_packed_keep;
 static int allow_ofs_delta;
 static const char *base_name;
 static int progress = 1;
+static int all_progress;
 static int window = 10;
 static uint32_t pack_size_limit, pack_size_limit_cfg;
 static int depth = 50;
@@ -2220,7 +2221,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp("--all-progress", arg)) {
-			progress = 2;
+			all_progress = 1;
 			continue;
 		}
 		if (!strcmp("-q", arg)) {
@@ -2295,6 +2296,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 		usage(pack_usage);
 	}
 
+	if (progress && all_progress)
+		progress = 2;
+
 	/* Traditionally "pack-objects [options] base extra" failed;
 	 * we would however want to take refs parameter that would
 	 * have been given to upstream rev-list ourselves, which means

^ permalink raw reply related

* Re: how to suppress progress percentage in git-push
From: Petr Baudis @ 2009-11-23 15:50 UTC (permalink / raw)
  To: Jeff King; +Cc: bill lam, Nicolas Pitre, git
In-Reply-To: <20091123145959.GA13138@sigill.intra.peff.net>

On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:
> The patch for (1) would look something like what's below.  It's simpler,
> but it does change the semantics; anyone who was relying on
> --all-progress to turn on progress unconditionally would need to now
> also use --progress. However, turning on progress unconditionally is
> usually an error (the except is if you are piping output in real-time to
> the user and need to overcome the isatty check).

I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I
would be unhappy if I would have to go through creating ptys or whatever
now. Maybe conditioning this by an environment variable?

				Petr "Pasky" Baudis

^ permalink raw reply

* Re: [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: Sitaram Chamarty @ 2009-11-23 16:28 UTC (permalink / raw)
  To: Intland Software; +Cc: Petr Baudis, git
In-Reply-To: <4B0A5720.6020804@intland.com>

On Mon, Nov 23, 2009 at 3:04 PM, Intland Software <marketing@intland.com> wrote:
> Sitaram Chamarty wrote:
>>
>> Intland: do you have a page that describes your role based ACL stuff a
>> little more?
>
>   This is a good starting point in our Knowledge Base, to read more about
> the permission model used in MR:
>
>   https://codebeamer.com/cb/wiki/11121

Thanks; I've read through that and the link to "Working Sets".

Modulo a bunch of *big* differences [*] in scale and scope between
gitolite and MR, I'm fairly certain that working sets (groups of
projects), user roles, user groups, etc., can all be done using
gitolite's group mechanism and its delegation feature.

It won't be as refined and nuanced as in MR, but in the main,
maintainable access control, which is the main point of all those
features, can be done.

-- 
Sitaram


[*] these big differences are:

  - gitolite config is all in one text file versus whatever MR uses
    internally
  - gitolite only deals with the git repo for each project; MR does with
    all sorts of stuff outside the actual code repo
  - gitolite has no UI for the end user, MR clearly does, and the user
    can do a lot of stuff by themselves (like create new projects)
  - possibly many more I missed

^ permalink raw reply

* OS X and umlauts in file names
From: Thomas Singer @ 2009-11-23 16:37 UTC (permalink / raw)
  To: git

I'm on an English OS X 10.6.2 and I created a sample file with umlauts in
its name (Überlänge.txt). When I try to stage the file in the terminal, I
can't complete the file name by typing the Ü and hitting the tab key, but I
can complete it by typing an U and hitting the tab key. Unfortunately, after
executing

 git stage Überlänge.txt

I invoked

 git status

and it still shows the file as new file. Should I set some environment
variable to be able to work with files containing umlauts in the name?

Thanks in advance,
Tom

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Jeff King @ 2009-11-23 16:43 UTC (permalink / raw)
  To: Petr Baudis; +Cc: bill lam, Nicolas Pitre, git
In-Reply-To: <20091123155043.GA28963@machine.or.cz>

On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote:

> On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:
> > The patch for (1) would look something like what's below.  It's simpler,
> > but it does change the semantics; anyone who was relying on
> > --all-progress to turn on progress unconditionally would need to now
> > also use --progress. However, turning on progress unconditionally is
> > usually an error (the except is if you are piping output in real-time to
> > the user and need to overcome the isatty check).
> 
> I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I
> would be unhappy if I would have to go through creating ptys or whatever
> now. Maybe conditioning this by an environment variable?

You wouldn't need to do anything that drastic. You would just need to
pass "--progress --all-progress" instead of only --all-progress. But you
have provided the data point that such a change would break at least one
user.

We could also leave --all-progress as-is and add new option to mean "if
you are already doing progress, do all progress".

-Peff

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Petr Baudis @ 2009-11-23 17:05 UTC (permalink / raw)
  To: Jeff King; +Cc: bill lam, Nicolas Pitre, git
In-Reply-To: <20091123164319.GA23011@sigill.intra.peff.net>

On Mon, Nov 23, 2009 at 11:43:19AM -0500, Jeff King wrote:
> On Mon, Nov 23, 2009 at 04:50:43PM +0100, Petr Baudis wrote:
> 
> > On Mon, Nov 23, 2009 at 10:00:00AM -0500, Jeff King wrote:
> > > The patch for (1) would look something like what's below.  It's simpler,
> > > but it does change the semantics; anyone who was relying on
> > > --all-progress to turn on progress unconditionally would need to now
> > > also use --progress. However, turning on progress unconditionally is
> > > usually an error (the except is if you are piping output in real-time to
> > > the user and need to overcome the isatty check).
> > 
> > I'm actually doing exactly that in the mirrorproj.cgi of Girocco, so I
> > would be unhappy if I would have to go through creating ptys or whatever
> > now. Maybe conditioning this by an environment variable?
> 
> You wouldn't need to do anything that drastic. You would just need to
> pass "--progress --all-progress" instead of only --all-progress. But you
> have provided the data point that such a change would break at least one
> user.
> 
> We could also leave --all-progress as-is and add new option to mean "if
> you are already doing progress, do all progress".

Hmm, maybe I'm confused - I just call

	git remote update

and don't pass any progress switches - would your change still affect
me? Can I pass --progress to `git remote update`?

-- 
				Petr "Pasky" Baudis
A lot of people have my books on their bookshelves.
That's the problem, they need to read them. -- Don Knuth

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Nicolas Pitre @ 2009-11-23 16:56 UTC (permalink / raw)
  To: Jeff King; +Cc: bill lam, git
In-Reply-To: <20091123145959.GA13138@sigill.intra.peff.net>

On Mon, 23 Nov 2009, Jeff King wrote:

> On Sun, Nov 22, 2009 at 10:53:53PM +0800, bill lam wrote:
> 
> > I set crontab to push to another computer for backup. It sent
> > confirmation email after finished.  It looked like
> > 
> > Counting objects: 1
> > Counting objects: 9, done.
> > Delta compression using up to 2 threads.
> > Compressing objects:  20% (1/5)
> > Compressing objects:  40% (2/5)
> > Compressing objects:  60% (3/5)
> > Compressing objects:  80% (4/5)
> > Compressing objects: 100% (5/5)
> > Compressing objects: 100% (5/5), done.
> > Writing objects:  20% (1/5)
> > Writing objects:  40% (2/5)
> > Writing objects:  60% (3/5)
> > Writing objects:  80% (4/5)
> > Writing objects: 100% (5/5)
> > Writing objects: 100% (5/5), 549 bytes, done.
> > Total 5 (delta 3), reused 0 (delta 0)
> > 
> > Often the list of progress % can be a page long.  I want output but
> > not those percentage progress status.  Will that be possible?
> 
> Hmm. There seems to be a bug. pack-objects is supposed to see that
> stderr is not a tty and suppress the progress messages. But it doesn't,
> because send-pack gives it the --all-progress flag, which
> unconditionally tells it to display progress, when the desired impact is
> actually to just make the progress more verbose.

Not exactly.

First, the progress variable is initialized with the result of isatty(2) 
by default.  The --progress argument is there to override that since 
pack-objects is often executed on the sending end of a fetch operation 
where stderr is not a terminal.

Then, during the pack-objects process, there are 3 phases: counting 
objects, compressing objects, and writing objects.  However in the fetch 
case we prefer to let the receiving end (index-pack or unpack-objects) 
take care of the progress display during the third phase.  This is why 
by default pack-objects doesn't display any "writing objects" progress 
when the generated pack is sent to stdout.  The --all-progress argument 
is there to override that, namely for a push.  The fact that 
--all-progress implies --progress is a bad side effect which wouldn't 
need to exist, but I don't think this is the cause of the issue here.

> We need to do one of:
> 
>   1. make --all-progress imply "if we are using progress, then make it
>      more verbose. Otherwise, ignore."
> 
>   2. fix all callers to check isatty(2) before unconditionally passing
>      the option

None of the above would fix the issue as this only affects progress 
display for phase #3.  You'd still get progress display for the counting 
phase and the compressing phase.

That doesn't mean it is OK for send-pack to unconditionally use 
--all-progress though, although it does provide the -q argument to 
pack-objects when push -q is used which inhibits any progress display 
already.


Nicolas

^ permalink raw reply

* Re: [PATCH 1/2] http-backend: Fix access beyond end of string.
From: Brian Gernhardt @ 2009-11-23 17:20 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Shawn O. Pearce, Tarmigan Casebolt, Git List,
	Tay Ray Chuan
In-Reply-To: <7viqdb0zhs.fsf@alter.siamese.dyndns.org>


On Nov 16, 2009, at 1:12 AM, Junio C Hamano wrote:

> 	n = out[0].rm_eo - out[0].rm_so; /* allocation */
>        ... validate and fail invalid method ...
>        cmd_arg = xmalloc(n);
>        memcpy(cmd_arg, dir + out[0].rm_so + 1, n-1);
>        cmd_arg[n-1] = '\0';

I just thought I'd point out that this change (committed as 48aec1b) fixed the problem I was having with t5541-http-push (and a couple others) hanging.  Looks like that one extra byte was overwriting something that malloc/free wanted to keep intact on OS X.

~~ Brian

^ permalink raw reply

* [PATCH] t/gitweb-lib: Split HTTP response with non-GNU sed
From: Brian Gernhardt @ 2009-11-23 17:33 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

Recognizing \r in a regex is something GNU sed will do, but other sed
implementation's won't.  (Found with BSD sed on OS X.) So use a
literal carriage return instead.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
 t/gitweb-lib.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 32b841d..35dda58 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -52,8 +52,8 @@ gitweb_run () {
 	rm -f gitweb.log &&
 	perl -- "$SCRIPT_NAME" \
 		>gitweb.output 2>gitweb.log &&
-	sed -e   '/^\r$/q' <gitweb.output >gitweb.headers &&
-	sed -e '1,/^\r$/d' <gitweb.output >gitweb.body    &&
+	sed -e   '/^
$/q' <gitweb.output >gitweb.headers &&
+	sed -e '1,/^
$/d' <gitweb.output >gitweb.body    &&
 	if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
 
 	# gitweb.log is left for debugging
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-11-23 17:39 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <20090813172508.GO1033@spearce.org>

Heya,

[picking up a topic on what to do if there's a import-marks from the
stream and the commandline]

On Thu, Aug 13, 2009 at 18:25, Shawn O. Pearce <spearce@spearce.org> wrote:
> Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> On Thu, Aug 13, 2009 at 10:07, Johannes
>> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>> > ... and will import the marks twice?
>>
>> Ah, you're right :(. What's the best way to do this? Should we dump
>> any previous marks when importing new ones?
>
> Uh, well, yes.  We shouldn't define :5 if it was in the file that
> appeared in the stream, but isn't in the file on the command line.
>
> Worse, what happens if we do this:
>
>  echo "option import-marks=/not/found" \
>  | git fast-import --import-marks=my.marks
>
> I want this to work, even though /not/found does not exist, but
> my.marks does.  So that does complicate things...

According to the docs it is allowed to specify import-marks multiple
times (the latest series does not honor that, and there's apparently
no test to verify it). What should the behavior be wrt
stream/commandline arguments? Load from stream if present and valid,
and also load from commandline? In that case, how do you override
what's in the stream?

I think the simplest is to allow the stream to specify a marks file
exactly once, and commandline arguments override what's in the stream.
Listing import-marks on the commandline is still valid and keeps the
old behavior.

Sensible?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] pack-objects: split implications of --all-progress from progress activation
From: Nicolas Pitre @ 2009-11-23 17:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Petr Baudis, bill lam, git
In-Reply-To: <20091123164319.GA23011@sigill.intra.peff.net>

Currently the --all-progress flag is used to use force progress display 
during the writing object phase even if output goes to stdout which is 
primarily the case during a push operation.  This has the unfortunate 
side effect of forcing progress display even if stderr is not a 
terminal.

Let's introduce the --all-progress-implied argument which has the same 
intent except for actually forcing the activation of any progress 
display.  With this, progress display will be automatically inhibited 
whenever stderr is not a terminal, or full progress display will be 
included otherwise.  This should let people use 'git push' within a cron 
job without filling their logs with useless percentage displays.

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

On Mon, 23 Nov 2009, Jeff King wrote:

> We could also leave --all-progress as-is and add new option to mean "if
> you are already doing progress, do all progress".

Actually, that's what I was working on when I saw the above.

diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index 2e49929..f54d433 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -9,8 +9,9 @@ git-pack-objects - Create a packed archive of objects
 SYNOPSIS
 --------
 [verse]
-'git pack-objects' [-q] [--no-reuse-delta] [--delta-base-offset] [--non-empty]
-	[--local] [--incremental] [--window=N] [--depth=N] [--all-progress]
+'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied]
+	[--no-reuse-delta] [--delta-base-offset] [--non-empty]
+	[--local] [--incremental] [--window=N] [--depth=N]
 	[--revs [--unpacked | --all]*] [--stdout | base-name]
 	[--keep-true-parents] < object-list
 
@@ -137,7 +138,7 @@ base-name::
 
 --all-progress::
 	When --stdout is specified then progress report is
-	displayed during the object count and deltification phases
+	displayed during the object count and compression phases
 	but inhibited during the write-out phase. The reason is
 	that in some cases the output stream is directly linked
 	to another command which may wish to display progress
@@ -146,6 +147,11 @@ base-name::
 	report for the write-out phase as well even if --stdout is
 	used.
 
+--all-progress-implied::
+	This is used to imply --all-progress whenever progress display
+	is activated.  Unlike --all-progress this flag doesn't actually
+	force any progress display by itself.
+
 -q::
 	This flag makes the command not to report its progress
 	on the standard error stream.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 4c91e94..4429d53 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -24,6 +24,7 @@
 
 static const char pack_usage[] =
   "git pack-objects [{ -q | --progress | --all-progress }]\n"
+  "        [--all-progress-implied]\n"
   "        [--max-pack-size=N] [--local] [--incremental]\n"
   "        [--window=N] [--window-memory=N] [--depth=N]\n"
   "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset]\n"
@@ -2124,6 +2125,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
 	int use_internal_rev_list = 0;
 	int thin = 0;
+	int all_progress_implied = 0;
 	uint32_t i;
 	const char **rp_av;
 	int rp_ac_alloc = 64;
@@ -2223,6 +2225,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			progress = 2;
 			continue;
 		}
+		if (!strcmp("--all-progress-implied", arg)) {
+			all_progress_implied = 1;
+			continue;
+		}
 		if (!strcmp("-q", arg)) {
 			progress = 0;
 			continue;
@@ -2326,6 +2332,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (keep_unreachable && unpack_unreachable)
 		die("--keep-unreachable and --unpack-unreachable are incompatible.");
 
+	if (progress && all_progress_implied)
+		progress = 2;
+
 	prepare_packed_git();
 
 	if (progress)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d26997b..8fffdbf 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -40,7 +40,7 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext
 	 */
 	const char *argv[] = {
 		"pack-objects",
-		"--all-progress",
+		"--all-progress-implied",
 		"--revs",
 		"--stdout",
 		NULL,
diff --git a/bundle.c b/bundle.c
index e04ab49..ff97adc 100644
--- a/bundle.c
+++ b/bundle.c
@@ -343,7 +343,7 @@ int create_bundle(struct bundle_header *header, const char *path,
 
 	/* write pack */
 	argv_pack[0] = "pack-objects";
-	argv_pack[1] = "--all-progress";
+	argv_pack[1] = "--all-progress-implied";
 	argv_pack[2] = "--stdout";
 	argv_pack[3] = "--thin";
 	argv_pack[4] = NULL;

^ permalink raw reply related

* Re: OS X and umlauts in file names
From: Thomas Rast @ 2009-11-23 17:45 UTC (permalink / raw)
  To: Thomas Singer; +Cc: git
In-Reply-To: <4B0ABA42.1060103@syntevo.com>

Thomas Singer wrote:
> I'm on an English OS X 10.6.2 and I created a sample file with umlauts in
> its name (Überlänge.txt). When I try to stage the file in the terminal, I
> can't complete the file name by typing the Ü and hitting the tab key, but I
> can complete it by typing an U and hitting the tab key. Unfortunately, after
> executing
> 
>  git stage Überlänge.txt

This is because of OS X's unicode normalisation.  Try any of the
many threads on the topic, e.g.,

  http://thread.gmane.org/gmane.comp.version-control.git/70688

The short version is that this Ü is in fact decomposed into an
U-umlaut duo.

Considering that this leads to endless fun[*] not just with git, and
that we German speakers have an easy way out (Ueberlaenge), I can only
suggest that you avoid umlauts wherever possible to preserve
the sanity of your users.


[*] I once had an SVN repo with two different directories both called
Übungen.  Took me a while to figure out what was going on.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH 2/2] remote-curl.c: fix rpc_out()
From: Tay Ray Chuan @ 2009-11-23 17:54 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Sverre Rabbelier, git, Shawn O. Pearce, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0911231137170.4985@pacific.mpi-cbg.de>

Hi,

On Mon, Nov 23, 2009 at 6:39 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> I guess you meant "not be enough", as an int can hold a pretty large
> number until it turns negative.

I really did mean 'enough' - enough to trigger the use of chunked
encoding. I think the most important fix here is forcing rpc_out to
(according to curl) memcpy at most size_t max bytes (the removal of the
extraneous ';' addresses this). Pushing with chunked transfer would
fail with this extra semicolon.

Removing the possibility of a negative size_t was a preventive measure,
and, like you mentioned, requires a larger repository, so it's harder
to test for.

I probably should separate these issues into separate patches.

> So I think in this case it is more harm- than helpful to have a test case.
>
> For future reference: if you need a repository with special featurs
> for testing, it is best to generate it in a test script (see the many test
> cases labeled 'setup' in our test suite for examples).

Here's what I came up with: use the git repository which fetched the
test suite, and use the environment variable GIT_REMOTE_REFSPEC to specify
the remote refspec which the tester fetches git from.


  if test -z "$GIT_REMOTE_REFSPEC"; then
  	say 'skipping test, the remote for git is not specified'
  else
  	test_expect_success 'push with chunked encoding' '
  		OWD=$(pwd) &&
  		cd $TEST_DIRECTORY/../.git/ &&
  		REPO=$(pwd) &&
  		cd "$OWD" &&
  		echo "$REPO"/objects > .git/objects/info/alternates &&
  		git fetch "$REPO" "$GIT_REMOTE_REFSPEC"/*:refs/remotes/git/* &&
  		git push -v -v origin "refs/remotes/git/*:refs/remotes/git/*" \
  			>out 2>&1 &&
  		grep "POST git-receive-pack (chunked)" out
  	'
  fi

Thoughts?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: OS X and umlauts in file names
From: Thomas Singer @ 2009-11-23 18:10 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git
In-Reply-To: <200911231845.04325.trast@student.ethz.ch>

Hi Thomas,

Thanks for the feed-back. I know the problem from SVN, too, but I had the
hope, that Git was smarter than SVN for this topic. IIRC, one could get SVN
working "somehow" with umlauts on OS X by setting some environment variable.
Unfortunately, I don't remember the details any more.

Basically, getting it "somehow" to work on OS X is just one minor step. IMHO
Git should standardize on file names in the repository and do the
platform-specific conversion independent of any locale setting, if needed.
Then and only then it would be possible to get the same characters out of
the repository, no matter whether the file was added or checked out on OS X,
Linux or Windows.

At the moment we've got a problem report regarding our SmartGit GUI client:
the user says, on command line it[1] works (German OS X) but not with
SmartGit, for me it doesn't even work on the command line (English OS X). As
you may know, Java uses characters for file names, the Java runtime
internally converts from the platform-specific byte-representation on disk
to characters. I can't simply tunnel the file name as byte array to the
invoked Git command - I simply don't know how to transform the characters of
the file name to a representation the Git command line client will
understand[2].

Tom

[1] e.g. to stage or commit files with umlauts in the file name
[2] executing an external command in Java also "only" works with strings
(aka characters), not with byte sequences


Thomas Rast wrote:
> Thomas Singer wrote:
>> I'm on an English OS X 10.6.2 and I created a sample file with umlauts in
>> its name (Überlänge.txt). When I try to stage the file in the terminal, I
>> can't complete the file name by typing the Ü and hitting the tab key, but I
>> can complete it by typing an U and hitting the tab key. Unfortunately, after
>> executing
>>
>>  git stage Überlänge.txt
> 
> This is because of OS X's unicode normalisation.  Try any of the
> many threads on the topic, e.g.,
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/70688
> 
> The short version is that this Ü is in fact decomposed into an
> U-umlaut duo.
> 
> Considering that this leads to endless fun[*] not just with git, and
> that we German speakers have an easy way out (Ueberlaenge), I can only
> suggest that you avoid umlauts wherever possible to preserve
> the sanity of your users.
> 
> 
> [*] I once had an SVN repo with two different directories both called
> Übungen.  Took me a while to figure out what was going on.
> 

^ permalink raw reply

* Re: [PATCH] pack-objects: split implications of --all-progress from  progress activation
From: Petr Baudis @ 2009-11-23 18:12 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, Jeff King, bill lam, git
In-Reply-To: <alpine.LFD.2.00.0911231221320.2059@xanadu.home>

On Mon, Nov 23, 2009 at 12:43:50PM -0500, Nicolas Pitre wrote:
> Currently the --all-progress flag is used to use force progress display 
> during the writing object phase even if output goes to stdout which is 
> primarily the case during a push operation.  This has the unfortunate 
> side effect of forcing progress display even if stderr is not a 
> terminal.
> 
> Let's introduce the --all-progress-implied argument which has the same 
> intent except for actually forcing the activation of any progress 
> display.  With this, progress display will be automatically inhibited 
> whenever stderr is not a terminal, or full progress display will be 
> included otherwise.  This should let people use 'git push' within a cron 
> job without filling their logs with useless percentage displays.
> 
> Signed-off-by: Nicolas Pitre <nico@fluxnic.net>

Ok, but what is currently the way to force the old behaviour? I believe
that should be also part of the commit message.

Naive deduction fails:

	$ git remote update --progress
	error: unknown option `progress'

Thanks,

-- 
				Petr "Pasky" Baudis
A lot of people have my books on their bookshelves.
That's the problem, they need to read them. -- Don Knuth

^ permalink raw reply

* Re: OS X and umlauts in file names
From: Johannes Schindelin @ 2009-11-23 18:23 UTC (permalink / raw)
  To: Thomas Singer; +Cc: Thomas Rast, git
In-Reply-To: <4B0AD02E.1040408@syntevo.com>

Hi,

On Mon, 23 Nov 2009, Thomas Singer wrote:

> Basically, getting it "somehow" to work on OS X is just one minor step. 
> IMHO Git should standardize on file names in the repository and do the 
> platform-specific conversion independent of any locale setting, if 
> needed.

That is contrary to the design of Git which honors content (byte-wise!) as 
much as possible, and treats file names very much as content.

There were beginnings of supporting OSX' brain-damaged filename mangling, 
but an obnoxious OSX fan worked very hard on trying to defend the OSX 
design and to decry Git's respect for the raw bytes on this list, so hard 
that even the nicest developers had no fun working on this issue anymore.

This little background may help you understand why there is no solution 
implemented in Git yet.  And maybe quite a few developers are reluctant to 
discuss the issue and possible solutions due to said sad story, too.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Junio C Hamano @ 2009-11-23 18:27 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belon; +Cc: Ren? Scharfe, Junio C Hamano, Bert Wesarg, git
In-Reply-To: <20091123112221.GA7175@sajinet.com.pe>

Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:

> why not better to apply the proposed patch from Junio in :
>
>   http://article.gmane.org/gmane.comp.version-control.git/127980/
>
> it would IMHO correct all reported issues and serve as well as a catch
> all from other tools that could be introduced in the future and that
> will be similarly affected by this misfeature.

I think René's patch is more sensible than $gmane/127980 because we have
no business mucking with these environment variables when we are running
things other than external grep.  You could be using system's "grep" in
your pre-commit hook to find some stuff, and your hook either may rely
on your having a particular set of GREP_OPTIONS in your environment, or
may be designed to work well with GREP_OPTIONS.

^ 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