Git development
 help / color / mirror / Atom feed
* 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

* Re: [PATCH] t/gitweb-lib: Split HTTP response with non-GNU sed
From: Junio C Hamano @ 2009-11-23 18:27 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List, Jakub Narebski
In-Reply-To: <1258997622-62403-1-git-send-email-brian@gernhardtsoftware.com>

Brian Gernhardt <brian@gernhardtsoftware.com> writes:

> 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.

I'd actually prefer not having to deal with this issue.  How about doing
something like this instead?

 t/gitweb-lib.sh |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 32b841d..3121950 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -52,8 +52,18 @@ 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    &&
+	perl -w -e '
+		open O, ">gitweb.headers";
+		while (<>) {
+			print O;
+			last if (/^\r$/ || /^$/);
+		}
+		open O, ">gitweb.body";
+		while (<>) {
+			print O;
+		}
+		close O;
+	' gitweb.output &&
 	if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
 
 	# gitweb.log is left for debugging

^ permalink raw reply related

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

On Mon, Nov 23, 2009 at 7:10 PM, Thomas Singer
<thomas.singer@syntevo.com> wrote:
> 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].

Ouch - so git is respecting whatever name the user and/or OS have
picked, but Java wants to canonicalize it,  and whatever scheme it
uses does not match OSX? That must hurt Java usage on OSX a lot. Sure
they have a workaround...?

Suggestions:

1 - Configure Java to canonicalize in the same style as OSX. Actually,
OSX's canonicalization is somewhat arbitrary so I think it exposes a
call to canonicalize a string "the right way".

2 - Many git calls accept filenames via STDIN - Java will surely write
binary there...

3 - xargs with its -z parameter can complement #2

hth,


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: [PATCH] pack-objects: split implications of --all-progress from progress activation
From: Nicolas Pitre @ 2009-11-23 18:27 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, Jeff King, bill lam, git
In-Reply-To: <20091123181206.GD26996@machine.or.cz>

On Mon, 23 Nov 2009, Petr Baudis wrote:

> 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?

If any existing out-of-tree users of pack-objects were using 
--all-progress then nothing has changed for them.

> I believe that should be also part of the commit message.
> 
> Naive deduction fails:
> 
> 	$ git remote update --progress
> 	error: unknown option `progress'

Usage of 'git remote" is about fetching and not pushing, right? My patch 
only affects pushes.  So I don't know what old behavior you're after.


Nicolas

^ permalink raw reply

* Re: [PATCH] t/gitweb-lib: Split HTTP response with non-GNU sed
From: Brian Gernhardt @ 2009-11-23 18:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Jakub Narebski
In-Reply-To: <7vocmtyu3v.fsf@alter.siamese.dyndns.org>


On Nov 23, 2009, at 1:27 PM, Junio C Hamano wrote:

> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
> 
>> 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.
> 
> I'd actually prefer not having to deal with this issue.  How about doing
> something like this instead?

Works for me.  I just went for the obvious minimal change to make it work.  But since we're testing a perl script, we might as well just use perl to deal with it's output.

~~ Brian

^ permalink raw reply

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

On Mon, Nov 23, 2009 at 01:27:14PM -0500, Nicolas Pitre wrote:
> On Mon, 23 Nov 2009, Petr Baudis wrote:
> > Naive deduction fails:
> > 
> > 	$ git remote update --progress
> > 	error: unknown option `progress'
> 
> Usage of 'git remote" is about fetching and not pushing, right? My patch 
> only affects pushes.  So I don't know what old behavior you're after.

Oh, I'm sorry, in that case I was confused from the very beginning;
somehow I saw pull instead of push in the initial mail.

				Petr "Pasky" Baudis

^ permalink raw reply

* Re: how to suppress progress percentage in git-push
From: Jeff King @ 2009-11-23 19:25 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: bill lam, git
In-Reply-To: <alpine.LFD.2.00.0911231043310.2059@xanadu.home>

On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote:

> > 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.

I think it does fix the issue, as it is exactly the side effect that we
are concerned about (and I tested my patch for 1, which squelched it).
But your --all-progress-implied is a better fix, so I think we should go
with that.

> 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.

It does, and I almost suggested that (although -q is new as of 1.6.5, so
it may not help the original poster). But "-q" actually does more than
that; it also silences any output for a successful push, which may not
be desired.

-Peff

^ permalink raw reply

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

On Mon, Nov 23, 2009 at 06:05:47PM +0100, Petr Baudis wrote:

> > 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`?

Oh, I misunderstood; I thought you were calling pack-objects directly.
So you are actually relying on the "even though isatty(2) is not true,
we always print progress messages" behavior? I think that behavior is
buggy. It hurts everybody pushing via cron, and it violates the usual
rule for when we show progress messages.

I don't think you can get a --progress pushed all the way down to the
pack-objects in this case; we would need to add code to override the
isatty check.

That being said, your example of "remote update" means you are dealing
with fetch, and we are not touching the fetch code path at all.

-Peff

^ permalink raw reply

* Re: [PATCH] pack-objects: split implications of --all-progress from progress activation
From: Jeff King @ 2009-11-23 19:32 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, Petr Baudis, 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>

Thanks,

Tested-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply

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

On Mon, 23 Nov 2009, Jeff King wrote:

> On Mon, Nov 23, 2009 at 11:56:43AM -0500, Nicolas Pitre wrote:
> 
> > > 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.
> 
> I think it does fix the issue, as it is exactly the side effect that we
> are concerned about (and I tested my patch for 1, which squelched it).

Sorry, you were right.  I somehow understood something else initially.

> But your --all-progress-implied is a better fix, so I think we should go
> with that.

OK.


Nicolas

^ 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