Git development
 help / color / mirror / Atom feed
* Re: [RFC PATCH 1/2] Replace memset(0) with static initialization where possible
From: Shawn O. Pearce @ 2008-10-09 19:17 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <NveF6_7LIvvEmRZEvLeTO5lw7EzzmOQkz1WGEMYGSFKDWqSwAeLwBw@cipher.nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Is there interest in a patch like this?

I think this is not a worthwhile change.
 
> "Possible" benefits:
> 
>   1) more concise, so it improves readability in most cases

I'm not sure.

Maybe I'm just too used to reading memset(&foo, 0, sizeof(foo)),
but {{0},} seems very difficult to read.

>   2) gives compiler more flexibility when optimizing

Shouldn't a good C compiler notice something like a memset and inline
it when possible?  They already can inline strlen on a constant.

> Drawbacks:
> 
>   1) many lines touched for no functional change

That's a pretty big drawback.

What happens when a struct gets a struct as its first member?
Do all the {0,} inits for it have to change to {{0,},} ?

-- 
Shawn.

^ permalink raw reply

* Re: [RFC PATCH 1/2] Replace memset(0) with static initialization where possible
From: Nicolas Pitre @ 2008-10-09 19:47 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <NveF6_7LIvvEmRZEvLeTO5lw7EzzmOQkz1WGEMYGSFKDWqSwAeLwBw@cipher.nrlssc.navy.mil>

On Thu, 9 Oct 2008, Brandon Casey wrote:

> "Possible" benefits:
> 
>   1) more concise, so it improves readability in most cases
>   2) gives compiler more flexibility when optimizing

Beware beware.  At some point this wasn't a gain at all with some gcc 
versions as it was stupid enough to construct a temporary object on the 
stack corresponding to "{ 0, }" and afterward do the assignment by 
*copying* this object to a different stack slot corresponding to the 
actual variable instead of constructing the initial value in place.

Also note that, on the other hand, gcc is smart enough to optimize a 
memset() using a constant size and value already by doing appropriate 
code replacement inline.  I've also seen cases where gcc did the 
opposite and replaced an explicit assignment like your patch does with 
an actual call to memset() when optimizing for size.

So when claiming flexibility for the compiler to better optimize things, 
please make sure this is really what is happening through assembly dump 
inspection.


Nicolas

^ permalink raw reply

* fetch fails with a short read of received pack
From: Alex Riesen @ 2008-10-09 19:55 UTC (permalink / raw)
  To: git; +Cc: Nicolas Pitre, Shawn O. Pearce, Junio C Hamano

Somehow I got my gnulib mirror to a strange state where I can't fetch
anything from it:

    $ cd gnulib
    $ git fetch -f ../gnulib.git 'refs/heads/*:refs/remotes/origin/*'
    remote: Counting objects: 2202, done.
    remote: Compressing objects: 100% (633/633), done.
    remote: Total 1769 (delta 1448), reused 1455 (delta 1134)
    Receiving objects: 100% (1769/1769), 404.11 KiB, done.
    fatal: premature end of pack file, 1745 bytes missing
    fatal: index-pack failed

This is with current Shawn's master (Junio's master erroneously gives
an error). Bisect points at ac0463ed44e859c84e354cd0ae47d9b5b124e112

    pack-objects: use fixup_pack_header_footer()'s validation mode

    When limiting the pack size, a new header has to be written to the
    pack and a new SHA1 computed.  Make sure that the SHA1 of what is being
    read back matches the SHA1 of what was written.

This and the diff make no sense to me (as to what could be broken).
I made both repos available at:

    The source, it unpacks in gitlib.git (as in example above)
	http://vin-soft.org/~ftp-tmp/gnulib-broken-src.tar.bz2

    The target, where the fetch in the example is run
	http://vin-soft.org/~ftp-tmp/gnulib-broken-tgt.tar.bz2

Could someone who actually understands the code please have a look?

^ permalink raw reply

* Git newbie question: permissions
From: Ed Schofield @ 2008-10-09 20:20 UTC (permalink / raw)
  To: git

Hi everyone,

I have a bare git repository that users in a particular group
("webdev") are pulling from and pushing to using the ssh transport.
One of the users has just reported this error during a push:

Counting objects: 103, done.
Compressing objects: 100% (68/68), done.
error: unable to write sha1 filename
./objects/4f/
973ce5c66f082af5087948cec57001f0c4da50: Permission denied

fatal: failed to write object
error: pack-objects died with strange error
error: failed to push some refs to '/var/git/myrepo.git'

I'd appreciate some help on getting my repository back to a sane
state, allowing this user to finish his push, and making sure
permissions are right in the future.

I don't think I specified "--shared=group" when initializing the
repository. Afterwards I manually set all files to have 660
permissions, dirs as 770, and set the group ownership to "webdev", but
I probably made a mistake by not setting the setgid bit on
directories. Now there are some objects directories with 755
permissions and different group ownership (the default groups of the
other users).

I have now run "git --bare init --shared=group" to reinitialize the
repository. This seems to have changed the directories to be g+sx. (Is
this all it did?). There are still some objects directories with 755
permissions rather than 770, which I presume I want, and the group
ownership of these is wrong. Shall I change these by hand? The sha1
files all have 444 permissions; is this right?

The last question I have is how to ensure that git creates object
files etc. with the right permissions when users push in future.

I'd appreciate any help!

-- Ed

^ permalink raw reply

* Re: fetch fails with a short read of received pack
From: Samuel Tardieu @ 2008-10-09 20:24 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Nicolas Pitre, Shawn O. Pearce, Junio C Hamano
In-Reply-To: <20081009195518.GA1497@blimp.localhost>

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

Alex> Somehow I got my gnulib mirror to a strange state where I can't
Alex> fetch anything from it:

You're the third person to report this problem in three days (I was
the second one). I thought my repository or the upstream one (GCC) was
corrupted in some ways, but it looks like a general problem.

Alex> fatal: premature end of pack file, 1745 bytes missing
Alex> fatal: index-pack failed

Alex> This is with current Shawn's master (Junio's master erroneously
Alex> gives an error).

The error message has been fixed three days ago, it hasn't reached
Junio's repository yet.

Alex> Bisect points at ac0463ed44e859c84e354cd0ae47d9b5b124e112

Incidentally, my first "git fetch" problem was around September 8 if I
remember correctly, so this looks plausible to have had the bug around
the date of the commit you mention (Aug 29).

 Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

^ permalink raw reply

* Re: [RFC PATCH 1/2] Replace memset(0) with static initialization where possible
From: Brandon Casey @ 2008-10-09 20:30 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <20081009191727.GY8203@spearce.org>

Shawn O. Pearce wrote:
> Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> Is there interest in a patch like this?
> 
> I think this is not a worthwhile change.
>  
>> "Possible" benefits:
>>
>>   1) more concise, so it improves readability in most cases
> 
> I'm not sure.
> 
> Maybe I'm just too used to reading memset(&foo, 0, sizeof(foo)),

Well, I don't have to get the sizeof(foo) part right if I use {0,}.

> but {{0},} seems very difficult to read.

It wouldn't be the first usage in the git source.

>>   2) gives compiler more flexibility when optimizing
> 
> Shouldn't a good C compiler notice something like a memset and inline
> it when possible?  They already can inline strlen on a constant.

I'm sure most of them do.

>> Drawbacks:
>>
>>   1) many lines touched for no functional change
> 
> That's a pretty big drawback.
> 
> What happens when a struct gets a struct as its first member?
> Do all the {0,} inits for it have to change to {{0,},} ?

yes, if you don't want compiler warnings.

-brandon

^ permalink raw reply

* Re: fetch fails with a short read of received pack
From: Nicolas Pitre @ 2008-10-09 20:31 UTC (permalink / raw)
  To: Samuel Tardieu; +Cc: Alex Riesen, git, Shawn O. Pearce, Junio C Hamano
In-Reply-To: <2008-10-09-22-24-15+trackit+sam@rfc1149.net>

On Thu, 9 Oct 2008, Samuel Tardieu wrote:

> >>>>> "Alex" == Alex Riesen <raa.lkml@gmail.com> writes:
> 
> Alex> Somehow I got my gnulib mirror to a strange state where I can't
> Alex> fetch anything from it:
> 
> You're the third person to report this problem in three days (I was
> the second one). I thought my repository or the upstream one (GCC) was
> corrupted in some ways, but it looks like a general problem.

Alex has a good test case and his bissection points at me.
So I'm on it.


Nicolas

^ permalink raw reply

* Re: [RFC PATCH 1/2] Replace memset(0) with static initialization where possible
From: Brandon Casey @ 2008-10-09 20:56 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0810091534430.26244@xanadu.home>

Nicolas Pitre wrote:
> On Thu, 9 Oct 2008, Brandon Casey wrote:
> 
>> "Possible" benefits:
>>
>>   1) more concise, so it improves readability in most cases
>>   2) gives compiler more flexibility when optimizing
> 
> Beware beware.  At some point this wasn't a gain at all with some gcc 
> versions as it was stupid enough to construct a temporary object on the 
> stack corresponding to "{ 0, }" and afterward do the assignment by 
> *copying* this object to a different stack slot corresponding to the 
> actual variable instead of constructing the initial value in place.
> 
> Also note that, on the other hand, gcc is smart enough to optimize a 
> memset() using a constant size and value already by doing appropriate 
> code replacement inline.  I've also seen cases where gcc did the 
> opposite and replaced an explicit assignment like your patch does with 
> an actual call to memset() when optimizing for size.

I'd call that flexibility.

> So when claiming flexibility for the compiler to better optimize things, 
> please make sure this is really what is happening through assembly dump 
> inspection.

I didn't claim the compiler _would_ do a better job at optimizing if
memset wasn't used (though my expectation is that it would do no worse,
and I don't have assembly dumps to back that up). I suggested it could
give the compiler more flexibility. In some strange way you seem to
agree with me and have given 3 examples of ways that compilers may
optimize static initialization. :)

Anyway, this is a nothing patch. There is no functional change.
If readability is not improved, it is not worth applying. Of course, I
don't plan on scanning through and converting all of the existing assignments
which use { 0, } to use memset. I find a single step declaration/initialization
simpler. Not sure why that seems to be the case in the git source for simple
variables but not structures.

-brandon

^ permalink raw reply

* Re: [RFC PATCH 1/2] Replace memset(0) with static initialization where possible
From: Nicolas Pitre @ 2008-10-09 21:12 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <CqZt8k7g-Uov2ItkgRw6K65RC0ee37ZAckIrbwPLNwDiaHtigtevnA@cipher.nrlssc.navy.mil>

On Thu, 9 Oct 2008, Brandon Casey wrote:

> Nicolas Pitre wrote:
> > So when claiming flexibility for the compiler to better optimize things, 
> > please make sure this is really what is happening through assembly dump 
> > inspection.
> 
> I didn't claim the compiler _would_ do a better job at optimizing if
> memset wasn't used (though my expectation is that it would do no worse,
> and I don't have assembly dumps to back that up).

My point is that such expectation might not always be true.

> I suggested it could
> give the compiler more flexibility. In some strange way you seem to
> agree with me and have given 3 examples of ways that compilers may
> optimize static initialization. :)

Actually the first example was a case of making things worse.

> Anyway, this is a nothing patch. There is no functional change.
> If readability is not improved, it is not worth applying. Of course, I
> don't plan on scanning through and converting all of the existing assignments
> which use { 0, } to use memset. I find a single step declaration/initialization
> simpler. Not sure why that seems to be the case in the git source for simple
> variables but not structures.

... because of that possible gcc making things worse I mentioned.


Nicolas

^ permalink raw reply

* Re: [PATCH] Implement git clone -v
From: Miklos Vajna @ 2008-10-09 21:26 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Alex Riesen, Constantine Plotnikov, git
In-Reply-To: <20081009152028.GA29829@spearce.org>

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

On Thu, Oct 09, 2008 at 08:20:28AM -0700, "Shawn O. Pearce" <spearce@spearce.org> wrote:
> I'm amending the message as the following:
> 
> --8<--
> Implement git clone -v
> 
> The new -v option forces the progressbar, even in case the output
> is not a terminal.  This can be useful if the caller is an IDE or
> wrapper which wants to scrape the progressbar from stderr and show
> its information in a different format.

Thanks!

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

^ permalink raw reply

* Re: [RFC PATCH 2/2] Replace calls to strbuf_init(&foo, 0) with static STRBUF_INIT initializer
From: Alex Riesen @ 2008-10-09 21:28 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <UUi8gSMV6CruoYIkVNOQZ4FNzsbqZcSNu6jdYH8GqIo@cipher.nrlssc.navy.mil>

2008/10/9 Brandon Casey <casey@nrlssc.navy.mil>:
> Many call sites use strbuf_init(&foo, 0) to initialize local strbuf variable
> "foo" which has not been accessed since its declaration. These can be
> replaced with a static initialization using the STRBUF_INIT macro which is
> just as readable, saves a function call, and takes up fewer lines.
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> This does the same thing for strbuf_init() that the last one did for memset().
>

But it does this using a predefined interface (using the STRBUG_INIT macro),
so next time strbuf changed, the code using it may be left as is.

I didn't like the memset->zero-init, but I do like this one.
It is a bit unclear, by the way, why strbuf_init wasn't inlined.

^ permalink raw reply

* Re: Git newbie question: permissions
From: Marc Weber @ 2008-10-09 21:29 UTC (permalink / raw)
  To: git
In-Reply-To: <1b5a37350810091320l72ae0a86m39db4258c9f4827e@mail.gmail.com>

> The last question I have is how to ensure that git creates object
> files etc. with the right permissions when users push in future.
Have a look at the config file. It should contain

[core]
        sharedrepository = 1
now.

I've never used that option before but I think this option should be
enough to ensure that it works in the future if it did for other repos
in the past..

Marc Weber

^ permalink raw reply

* Re: Git newbie question: permissions
From: Samuel Lucas Vaz de Mello @ 2008-10-09 21:05 UTC (permalink / raw)
  To: git
In-Reply-To: <1b5a37350810091320l72ae0a86m39db4258c9f4827e@mail.gmail.com>

Ed Schofield wrote:
> I don't think I specified "--shared=group" when initializing the
> repository. Afterwards I manually set all files to have 660
> permissions, dirs as 770, and set the group ownership to "webdev", but
> I probably made a mistake by not setting the setgid bit on
> directories. Now there are some objects directories with 755
> permissions and different group ownership (the default groups of the
> other users).
>   
Hi Ed!

I'm also a newbie here and I have a very similar setup to yours.

The only difference is that my repository was created using 
git-cvsimport and afterwards I used git-config to set 
core.sharedrepository=1 and manually set up the permissions.

I also got objects created with the users' default group, but for now I 
just changed the deafault group for those users until I find a better 
solution.

Another issue with this setup: if I run git-gc in the shared repo, it 
recreate the files in logs/refs/heads with 644 permissions, which 
prevents users to push until I manually fix the permissions.

Someone else have faced these kind of problems?

Regards,

 - Samuel

^ permalink raw reply

* ls-files [Was: Re: Fwd: git status options feature suggestion]
From: James Cloos @ 2008-10-09 21:23 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Caleb Cushing
In-Reply-To: <alpine.DEB.1.00.0810091101230.22125@pacific.mpi-cbg.de.mpi-cbg.de>

>>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > How about "git ls-files -o"?
>> 
>> doh... hadn't even heard of that command.

Johannes> Which is good!  As ls-files is listed as plumbing.
Johannes> Users should not need to call ls-files,

That is a bug, then.  ls-files is one of the more important user-level
commands in git.

It is vastly more efficient than find(1) or a --recursive call to
grep(1).

Searching through a repository to find which file(s) define or use some
function, struct, class or similar is a common occurance.  Or to find
which dir(s) contain(s) file(s) matching a given regexp.  Or a number
of other uses.  (Tags might be useful if one does a lot of searching
in a given repo, but grep is quicker for infrequent searches and the
tags utils do not support all file types.)

ls-files is definitely dual-use.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

^ permalink raw reply

* Re: ls-files [Was: Re: Fwd: git status options feature suggestion]
From: Shawn O. Pearce @ 2008-10-09 21:41 UTC (permalink / raw)
  To: James Cloos; +Cc: git, Johannes Schindelin, Caleb Cushing
In-Reply-To: <m3ej2p4g3r.fsf_-_@lugabout.jhcloos.org>

James Cloos <cloos@jhcloos.com> wrote:
> >>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> > How about "git ls-files -o"?
> >> 
> >> doh... hadn't even heard of that command.
> 
> Johannes> Which is good!  As ls-files is listed as plumbing.
> Johannes> Users should not need to call ls-files,
> 
> That is a bug, then.  ls-files is one of the more important user-level
> commands in git.
> 
> It is vastly more efficient than find(1) or a --recursive call to
> grep(1).

How about using "git grep" then?  No need for ls-files...

-- 
Shawn.

^ permalink raw reply

* Re: Git newbie question: permissions
From: Samuel Tardieu @ 2008-10-09 21:41 UTC (permalink / raw)
  To: Ed Schofield; +Cc: git
In-Reply-To: <1b5a37350810091320l72ae0a86m39db4258c9f4827e@mail.gmail.com>

>>>>> "Ed" == Ed Schofield <edschofield@gmail.com> writes:

Ed> I have now run "git --bare init --shared=group" to reinitialize
Ed> the repository. This seems to have changed the directories to be
Ed> g+sx. (Is this all it did?). There are still some objects
Ed> directories with 755 permissions rather than 770, which I presume
Ed> I want, and the group ownership of these is wrong. Shall I change
Ed> these by hand? The sha1 files all have 444 permissions; is this
Ed> right?

Ed> The last question I have is how to ensure that git creates object
Ed> files etc. with the right permissions when users push in future.

As Marc said, you should first make sure that "config" contains
"sharedrepository = 1" in the "[core]" section.

Then you can do the following:

  - remove all permissions for "others":  chmod -R o-rwx .
  - mirror "user" permissions to "group": chmod -R g=u .
  - add +s flag to directories:           find . -type d | xargs chmod g+s

This should fix your current situation. The "sharedrepository = 1"
will tell git to maintain a proper shared state in the future
on objects it creates (i.e. mirror "user" permission to "group" ones).

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/

^ permalink raw reply

* Re: [JGIT PATCH 2/5] Fix UnpackedObjectLoader.getBytes to return a copy
From: Jonas Fonseca @ 2008-10-09 21:46 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Robin Rosenberg, git
In-Reply-To: <1222824690-7632-3-git-send-email-spearce@spearce.org>

On Wed, Oct 1, 2008 at 03:31, Shawn O. Pearce <spearce@spearce.org> wrote:
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> index 5282491..87e861f 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> @@ -105,7 +105,12 @@ protected void setId(final ObjectId id) {
>         * @throws IOException
>         *             the object cannot be read.
>         */
> -       public abstract byte[] getBytes() throws IOException;
> +       public final byte[] getBytes() throws IOException {
> +               final byte[] data = getCachedBytes();
> +               final byte[] copy = new byte[data.length];
> +               System.arraycopy(data, 0, copy, 0, data.length);
> +               return data;
> +       }

If I understand correctly, shouldn't this return the copy variable?

-- 
Jonas Fonseca

^ permalink raw reply

* Re: [JGIT PATCH 2/5] Fix UnpackedObjectLoader.getBytes to return a copy
From: Shawn O. Pearce @ 2008-10-09 21:49 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Robin Rosenberg, git
In-Reply-To: <2c6b72b30810091446y22cb2e00te7a25676ee21ddac@mail.gmail.com>

Jonas Fonseca <jonas.fonseca@gmail.com> wrote:
> On Wed, Oct 1, 2008 at 03:31, Shawn O. Pearce <spearce@spearce.org> wrote:
> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> > index 5282491..87e861f 100644
> > --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> > +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
> > @@ -105,7 +105,12 @@ protected void setId(final ObjectId id) {
> >         * @throws IOException
> >         *             the object cannot be read.
> >         */
> > -       public abstract byte[] getBytes() throws IOException;
> > +       public final byte[] getBytes() throws IOException {
> > +               final byte[] data = getCachedBytes();
> > +               final byte[] copy = new byte[data.length];
> > +               System.arraycopy(data, 0, copy, 0, data.length);
> > +               return data;
> > +       }
> 
> If I understand correctly, shouldn't this return the copy variable?

F&@!#*@#&@!#*@!@!&#@#@*@!

Yes.  :-)

I think its already committed to master too.  Can you send a patch
along to fix, and point out how stupid I am?

-- 
Shawn.

^ permalink raw reply

* [PATCH] git-daemon: Worked around uclibc buffer problem
From: Lars Stoltenow @ 2008-10-09 21:34 UTC (permalink / raw)
  To: git

uclibc immediately write()s every string argument to fprintf, which causes
superfluous 'remote: ' strings to appear when cloning repos. This patch
makes it write the message in one shot.

Signed-off-by: Lars Stoltenow <penma@penma.de>
---
 builtin-pack-objects.c |    7 +++++--
 progress.c             |    8 ++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 1158e42..94f7404 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2232,9 +2232,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (nr_result)
 		prepare_pack(window, depth);
 	write_pack_file();
-	if (progress)
-		fprintf(stderr, "Total %"PRIu32" (delta %"PRIu32"),"
+	if (progress) {
+		char message[128];
+		snprintf(message, sizeof(message), "Total %"PRIu32" (delta %"PRIu32"),"
 			" reused %"PRIu32" (delta %"PRIu32")\n",
 			written, written_delta, reused, reused_delta);
+		fputs(message, stderr);
+	}
 	return 0;
 }
diff --git a/progress.c b/progress.c
index 55a8687..56c9134 100644
--- a/progress.c
+++ b/progress.c
@@ -94,16 +94,20 @@ static int display(struct progress *progress, unsigned n, const char *done)
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
+			char message[128];
 			progress->last_percent = percent;
-			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
+			snprintf(message, sizeof(message), "%s: %3u%% (%u/%u)%s%s",
 				progress->title, percent, n,
 				progress->total, tp, eol);
+			fputs(message, stderr);
 			fflush(stderr);
 			progress_update = 0;
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
+		char message[128];
+		snprintf(message, sizeof(message), "%s: %u%s%s", progress->title, n, tp, eol);
+		fputs(message, stderr);
 		fflush(stderr);
 		progress_update = 0;
 		return 1;
-- 
1.6.0.2.GIT

^ permalink raw reply related

* [PATCH] test-lib: fix color reset in say_color()
From: Miklos Vajna @ 2008-10-09 22:07 UTC (permalink / raw)
  To: git

When executing a single test with colors enabled, the cursor was not set
back to the previous one, and you had to hit an extra enter to get it
back.

Work around this problem by calling 'tput sgr0' before printing the
final newline.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

Actually I'm not 100% sure about how many users are affected. I have a
black background in konsole with white letters, and after the test I get
a green cursor, and once I hit enter, I get the white one back.

 t/test-lib.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index e2b106c..fb89741 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -112,8 +112,9 @@ if test -n "$color"; then
 			*) test -n "$quiet" && return;;
 		esac
 		shift
-		echo "* $*"
+		printf "* $*"
 		tput sgr0
+		echo
 		)
 	}
 else
-- 
1.6.0.2

^ permalink raw reply related

* [JGIT PATCH] Fix ObjectLoader.getBytes to really return the created copy
From: Jonas Fonseca @ 2008-10-09 22:09 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jonas Fonseca, Robin Rosenberg, git
In-Reply-To: <20081009214926.GA8203@spearce.org>

The bug was carried over from the original code in PackedObjectLoader by
commit 4c49ab5a4ec8555681ceabf17142a15bf90c2c24.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 .../src/org/spearce/jgit/lib/ObjectLoader.java     |    2 +-

 Shawn O. Pearce <spearce@spearce.org> wrote Thu, Oct 09, 2008:
 > I think its already committed to master too.  Can you send a patch
 > along to fix, and point out how stupid I am?

 Yes, I noticed it has already been applied. I am lagging behind a bit.
 Anyway, here is a trivial patch.

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
index 87e861f..8d745dd 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectLoader.java
@@ -109,7 +109,7 @@ protected void setId(final ObjectId id) {
 		final byte[] data = getCachedBytes();
 		final byte[] copy = new byte[data.length];
 		System.arraycopy(data, 0, copy, 0, data.length);
-		return data;
+		return copy;
 	}
 
 	/**
-- 
1.6.0.2.665.g48ddf


-- 
Jonas Fonseca

^ permalink raw reply related

* Re: ls-files [Was: Re: Fwd: git status options feature suggestion]
From: Jeremy Ramer @ 2008-10-09 22:13 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: James Cloos, git, Johannes Schindelin, Caleb Cushing
In-Reply-To: <20081009214118.GZ8203@spearce.org>

On Thu, Oct 9, 2008 at 3:41 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> James Cloos <cloos@jhcloos.com> wrote:
>> >>>>> "Johannes" == Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> >> > How about "git ls-files -o"?
>> >>
>> >> doh... hadn't even heard of that command.
>>
>> Johannes> Which is good!  As ls-files is listed as plumbing.
>> Johannes> Users should not need to call ls-files,
>>
>> That is a bug, then.  ls-files is one of the more important user-level
>> commands in git.
>>
>> It is vastly more efficient than find(1) or a --recursive call to
>> grep(1).
>
> How about using "git grep" then?  No need for ls-files...

I use git-grep for searching the contents of files in the repo, but it
seems to me that git ls-files is necessary for quickly parsing the
file names themselves.

>
> --
> Shawn.
> --
> 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] git-daemon: Worked around uclibc buffer problem
From: Lars Stoltenow @ 2008-10-09 22:51 UTC (permalink / raw)
  To: git

uclibc immediately write()s every string argument to fprintf, which causes
superfluous 'remote: ' strings to appear when cloning repos. This patch
makes it write the message in one shot.

Signed-off-by: Lars Stoltenow <penma@penma.de>
---
 builtin-pack-objects.c |    7 +++++--
 progress.c             |    8 ++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 1158e42..94f7404 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2232,9 +2232,12 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (nr_result)
 		prepare_pack(window, depth);
 	write_pack_file();
-	if (progress)
-		fprintf(stderr, "Total %"PRIu32" (delta %"PRIu32"),"
+	if (progress) {
+		char message[128];
+		snprintf(message, sizeof(message), "Total %"PRIu32" (delta %"PRIu32"),"
 			" reused %"PRIu32" (delta %"PRIu32")\n",
 			written, written_delta, reused, reused_delta);
+		fputs(message, stderr);
+	}
 	return 0;
 }
diff --git a/progress.c b/progress.c
index 55a8687..56c9134 100644
--- a/progress.c
+++ b/progress.c
@@ -94,16 +94,20 @@ static int display(struct progress *progress, unsigned n, const char *done)
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
+			char message[128];
 			progress->last_percent = percent;
-			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
+			snprintf(message, sizeof(message), "%s: %3u%% (%u/%u)%s%s",
 				progress->title, percent, n,
 				progress->total, tp, eol);
+			fputs(message, stderr);
 			fflush(stderr);
 			progress_update = 0;
 			return 1;
 		}
 	} else if (progress_update) {
-		fprintf(stderr, "%s: %u%s%s", progress->title, n, tp, eol);
+		char message[128];
+		snprintf(message, sizeof(message), "%s: %u%s%s", progress->title, n, tp, eol);
+		fputs(message, stderr);
 		fflush(stderr);
 		progress_update = 0;
 		return 1;
-- 
1.6.0.2.GIT

^ permalink raw reply related

* Re: ls-files
From: James Cloos @ 2008-10-09 22:52 UTC (permalink / raw)
  To: git; +Cc: Shawn O. Pearce, Johannes Schindelin, Caleb Cushing
In-Reply-To: <20081009214118.GZ8203@spearce.org>

>>>>> "Shawn" == Shawn O Pearce <spearce@spearce.org> writes:

>> [ls-files] is vastly more efficient than find(1) or a --recursive
>> call to grep(1).

Shawn> How about using "git grep" then?  No need for ls-files...

I often filter the list of files before passing them to xargs, so git
grep helps for some use cases, but not all.

Also, (at least as of 1.6.0.2.307.gc427), git grep does not support -P
(Perl compatible regexps) or --color.

-JimC
-- 
James Cloos <cloos@jhcloos.com>         OpenPGP: 1024D/ED7DAEA6

^ permalink raw reply

* Re: Git newbie question: permissions
From: Ed Schofield @ 2008-10-09 22:59 UTC (permalink / raw)
  To: git
In-Reply-To: <2008-10-09-23-41-14+trackit+sam@rfc1149.net>

On Thu, Oct 9, 2008 at 10:41 PM, Samuel Tardieu <sam@rfc1149.net> wrote:
>>>>>> "Ed" == Ed Schofield <edschofield@gmail.com> writes:
>
> Ed> I have now run "git --bare init --shared=group" to reinitialize
> Ed> the repository. This seems to have changed the directories to be
> Ed> g+sx. (Is this all it did?). There are still some objects
> Ed> directories with 755 permissions rather than 770, which I presume
> Ed> I want, and the group ownership of these is wrong. Shall I change
> Ed> these by hand? The sha1 files all have 444 permissions; is this
> Ed> right?
>
> Ed> The last question I have is how to ensure that git creates object
> Ed> files etc. with the right permissions when users push in future.
>
> As Marc said, you should first make sure that "config" contains
> "sharedrepository = 1" in the "[core]" section.
>
> Then you can do the following:
>
>  - remove all permissions for "others":  chmod -R o-rwx .
>  - mirror "user" permissions to "group": chmod -R g=u .
>  - add +s flag to directories:           find . -type d | xargs chmod g+s
>
> This should fix your current situation. The "sharedrepository = 1"
> will tell git to maintain a proper shared state in the future
> on objects it creates (i.e. mirror "user" permission to "group" ones).

This worked beautifully. Thanks Sam, thanks Marc!

-- Ed

^ 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