Git development
 help / color / mirror / Atom feed
* aliases causing “Permission denied” error in git v1.7
From: Алексей Данченков @ 2011-11-04  8:09 UTC (permalink / raw)
  To: git

Hi!

$ git co -b newbranch
$ git co oldbranch

results in "fatal: cannot exec 'git-co': Permission denied" error.

In the same time, things like

$ git checkout -b newbranch
$ git checkout oldbranch

and

$ sudo git co -b newbranch
$ sudo git co oldbranch

work as expected. Ownership rights for the .git die are set for the user
and 0755/0644 are the mode for .git dir/subdir/files. There are no git-co
script anywhere in the system (as it is an alias to git-checkout, which
resides in /usr/libexec/git-core).

Aliases are defined in .gitconfig of the home dir:

[alias]
co = checkout

There is no difference in git config for root or normal user.

Things work well as expected when I install git v1.6. Going back to 1.7
gives me the same error.

What am I missing?

---

Gentoo / kernel v3.0.6 / git v1.7.3.4

^ permalink raw reply

* [PATCH] http-push: don't always prompt for password
From: Stefan Naewe @ 2011-11-04  7:03 UTC (permalink / raw)
  To: gitster, git, peff; +Cc: Stefan Naewe
In-Reply-To: <7vfwi6jucg.fsf@alter.siamese.dyndns.org>

http-push prompts for a password when the URL is set as
'https://user@host/repo' even though there is one set
in ~/.netrc. Pressing ENTER at the password prompt succeeds
then, but is a annoying and makes it almost useless
in a shell script, e.g.

Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
---
 http.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index a4bc770..008ad72 100644
--- a/http.c
+++ b/http.c
@@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
 	curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 #endif
 
-	init_curl_http_auth(result);
-
 	if (ssl_cert != NULL)
 		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
 	if (has_cert_password())
@@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
 		else if (missing_target(&results))
 			ret = HTTP_MISSING_TARGET;
 		else if (results.http_code == 401) {
-			if (user_name) {
+			if (user_name && user_pass) {
 				ret = HTTP_NOAUTH;
 			} else {
 				/*
@@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
 				 * but that is non-portable.  Using git_getpass() can at least be stubbed
 				 * on other platforms with a different implementation if/when necessary.
 				 */
-				user_name = xstrdup(git_getpass_with_description("Username", description));
+				if (!user_name)
+					user_name = xstrdup(git_getpass_with_description("Username", description));
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
-- 
1.7.8.rc0.1.gb345ae

^ permalink raw reply related

* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Jonathan Nieder @ 2011-11-04  6:38 UTC (permalink / raw)
  To: Ben Walton; +Cc: Eric Wong, git
In-Reply-To: <1320372215-sup-8341@pinkfloyd.chass.utoronto.ca>

Ben Walton wrote:

> Fixing this locally to the use of the minimized url let me move on
> farther but I then got another core dump.

If it continues like this, it might be possible to get help from svn
developers.

E.g., I would love to see a nice summary of the relevant API changes,
like so (except with more truth):

  Previously the svn_frob() function would accept a filename with
  spaces in it in its "path" argument.  Only the svn_plugh() function
  and its relatives required escaped paths.  And all functions
  returning paths would unescape them.

  With Subversion 1.7, passing a filename with a space in it to
  svn_frob() trips an assertion, so we have to escape the "path"
  argument.  This requires ... changes in application code.

  Unfortunately, back in Subversion 1.6, svn_frob() escapes its
  argument, so an application modified as above will find its "path"
  argument to be double-escaped.  There does not seem to be any
  way for applications to target both Subversion 1.6 and 1.7 without
  doing ...

  Subversion 1.8 should follow the following simple, consistent
  semantics when requested with a flag, which would allow me to easily
  target my code against both SVN >= 1.8 and <= 1.6 with a few "if"
  statements (forgetting 1.7 as if it were a bad dream).
 
Or to put it another way: if getting git-svn to cooperate starts to
prove difficult, maybe abstracting away from git and understanding
what changed for libsvn callers in general could help.  On one hand,
it could help in making sure the guarantees that libsvn offers are
well documented, stable in the future, and clear.  On the other hand,
it might make the appropriate changes to git more obvious.

Sorry for the ramble.  I wish I could offer a fix instead.
Jonathan

^ permalink raw reply

* Re: Folder level Acces in git
From: Joshua Jensen @ 2011-11-04  4:21 UTC (permalink / raw)
  To: Eugene Sajine; +Cc: redhat1981, git
In-Reply-To: <CAPZPVFY15AqCpWcRbv0tjXBz4G2kQTm+nMGpsYzCKe0niHV_dA@mail.gmail.com>

----- Original Message -----
From: Eugene Sajine
Date: 11/3/2011 1:28 PM
> Are you sure that the way your have organized the repository is
> actually correct? If you need to manage the access on folder level why
> don't you simply split up the project into several
> repositories/projects which each team is going to work with
> independently?
>
> This seems to me to be much simpler and cleaner solution then any
> other alternative.
>
Submodules are _not_ simple at all.  Our organization of nearly 100 
developers using Git pretty much let out a collective cheer when we 
finally removed the submodules.  Submodules are an absolute pain to 
develop within; there are a number of Git mailing list exchanges about 
that, but I'd be happy to go into great detail if anybody cares.  Even 
submodules that are read-only are a pain as it takes two steps (git pull 
+ git submodule update) to actually get them up to date.

Ick.

In short, if it is an absolute requirement to manage access to a 
repository on a folder level, get Subversion or Perforce.  DVCS is not 
for you...

Josh

^ permalink raw reply

* Re: How to make "git push/pull" work in non-clone repo?
From: Hong-Ming Su @ 2011-11-04  3:33 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Kirill Likhodedov, git
In-Reply-To: <m2fwi5nnt8.fsf@igel.home>

git push -u can set upstream too.
Thanks all!

On Fri, Nov 4, 2011 at 1:00 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Hong-Ming Su <halleyinvent@gmail.com> writes:
>
>> /d/workspace/git/work1 (master)
>> $ git remote add origin ../depot
>
> $ git branch --set-upstream master origin/master
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
>

^ permalink raw reply

* [PATCH] fsck: print progress
From: Nguyễn Thái Ngọc Duy @ 2011-11-04  3:10 UTC (permalink / raw)
  To: git, Junio C Hamano, Jeff King; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <20111103211819.GA17341@sigill.intra.peff.net>

fsck is usually a long process and it would be nice if it prints
progress from time to time.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 OK let's go with total object count for more accurate progress. One
 thing I did not realize that the title is not copied to struct
 progress and we can update title (e.g. current pack) while displaying
 progress.

 Updating once every 1024 objects may feel sluggish on large blobs,
 but we have more important things to worry about when it comes to
 large blobs than this progress bar.

 multithread fsck sounds interesting, I'll look into it.

 Documentation/git-fsck.txt |   12 +++++++++-
 builtin/fsck.c             |   49 ++++++++++++++++++++++++++++++++++++++++---
 pack-check.c               |   11 +++++++--
 pack.h                     |    4 ++-
 4 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index a2a508d..5245101 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git fsck' [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]
-	 [--[no-]full] [--strict] [--verbose] [--lost-found] [<object>*]
+	 [--[no-]full] [--strict] [--verbose] [--lost-found]
+	 [--[no-]progress] [<object>*]
 
 DESCRIPTION
 -----------
@@ -72,6 +73,15 @@ index file, all SHA1 references in .git/refs/*, and all reflogs (unless
 	a blob, the contents are written into the file, rather than
 	its object name.
 
+--progress::
+--no-progress::
+	When fsck is run in a terminal, it will show the progress.
+	These options can force progress to be shown or not
+	regardless terminal check.
++
+Progress is not shown when --verbose is used. --progress is ignored
+in this case.
+
 It tests SHA1 and general object sanity, and it does full tracking of
 the resulting reachability and everything else. It prints out any
 corruption it finds (missing or bad objects), and if you use the
diff --git a/builtin/fsck.c b/builtin/fsck.c
index df1a88b..e0cc4de 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -11,6 +11,7 @@
 #include "fsck.h"
 #include "parse-options.h"
 #include "dir.h"
+#include "progress.h"
 
 #define REACHABLE 0x0001
 #define SEEN      0x0002
@@ -27,6 +28,8 @@ static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
 static int verbose;
+static int show_progress = -1;
+
 #define ERROR_OBJECT 01
 #define ERROR_REACHABLE 02
 
@@ -512,15 +515,20 @@ static void get_default_heads(void)
 static void fsck_object_dir(const char *path)
 {
 	int i;
+	struct progress *progress = NULL;
 
 	if (verbose)
 		fprintf(stderr, "Checking object directory\n");
 
+	if (show_progress)
+		progress = start_progress("Checking object directories", 256);
 	for (i = 0; i < 256; i++) {
 		static char dir[4096];
 		sprintf(dir, "%s/%02x", path, i);
 		fsck_dir(i, dir);
+		display_progress(progress, i+1);
 	}
+	stop_progress(&progress);
 	fsck_sha1_list();
 }
 
@@ -591,6 +599,7 @@ static struct option fsck_opts[] = {
 	OPT_BOOLEAN(0, "strict", &check_strict, "enable more strict checking"),
 	OPT_BOOLEAN(0, "lost-found", &write_lost_and_found,
 				"write dangling objects in .git/lost-found"),
+	OPT_BOOL   (0, "progress", &show_progress, "show progress"),
 	OPT_END(),
 };
 
@@ -603,6 +612,12 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	argc = parse_options(argc, argv, prefix, fsck_opts, fsck_usage, 0);
+
+	if (show_progress == -1)
+		show_progress = isatty(2);
+	if (verbose)
+		show_progress = 0;
+
 	if (write_lost_and_found) {
 		check_full = 1;
 		include_reflogs = 0;
@@ -622,20 +637,46 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
 	if (check_full) {
 		struct packed_git *p;
+		int i, nr_objects = 0, object_count;
+		struct progress *progress = NULL;
 
 		prepare_packed_git();
-		for (p = packed_git; p; p = p->next)
+
+		if (show_progress) {
+			for (p = packed_git; p; p = p->next) {
+				if (open_pack_index(p))
+					continue;
+				nr_objects += p->num_objects;
+			}
+
+			object_count = 0;
+			progress = start_progress("Verifying packs", nr_objects);
+		}
+
+		for (i = 1, p = packed_git; p; p = p->next, i++) {
 			/* verify gives error messages itself */
-			verify_pack(p);
+			verify_pack(p, progress, object_count);
+			object_count += p->num_objects;
+		}
+		stop_progress(&progress);
 
-		for (p = packed_git; p; p = p->next) {
+		if (show_progress) {
+			object_count = 0;
+			progress = start_progress("Checking objects", nr_objects);
+		}
+
+		for (i = 1, p = packed_git; p; p = p->next, i++) {
 			uint32_t j, num;
 			if (open_pack_index(p))
 				continue;
 			num = p->num_objects;
-			for (j = 0; j < num; j++)
+			for (j = 0; j < num; j++) {
 				fsck_sha1(nth_packed_object_sha1(p, j));
+				display_progress(progress, object_count + j+1);
+			}
+			object_count += p->num_objects;
 		}
+		stop_progress(&progress);
 	}
 
 	heads = 0;
diff --git a/pack-check.c b/pack-check.c
index 0c19b6e..80de302 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "pack.h"
 #include "pack-revindex.h"
+#include "progress.h"
 
 struct idx_entry {
 	off_t                offset;
@@ -42,7 +43,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
 }
 
 static int verify_packfile(struct packed_git *p,
-		struct pack_window **w_curs)
+			   struct pack_window **w_curs,
+			   struct progress *progress, uint32_t base_count)
 {
 	off_t index_size = p->index_size;
 	const unsigned char *index_base = p->index_data;
@@ -126,7 +128,10 @@ static int verify_packfile(struct packed_git *p,
 			break;
 		}
 		free(data);
+		if (((base_count + i) & 1023) == 0)
+			display_progress(progress, base_count + i);
 	}
+	display_progress(progress, base_count + i);
 	free(entries);
 
 	return err;
@@ -155,7 +160,7 @@ int verify_pack_index(struct packed_git *p)
 	return err;
 }
 
-int verify_pack(struct packed_git *p)
+int verify_pack(struct packed_git *p, struct progress *progress, uint32_t base_count)
 {
 	int err = 0;
 	struct pack_window *w_curs = NULL;
@@ -164,7 +169,7 @@ int verify_pack(struct packed_git *p)
 	if (!p->index_data)
 		return -1;
 
-	err |= verify_packfile(p, &w_curs);
+	err |= verify_packfile(p, &w_curs, progress, base_count);
 	unuse_pack(&w_curs);
 
 	return err;
diff --git a/pack.h b/pack.h
index 722a54e..3a63bf6 100644
--- a/pack.h
+++ b/pack.h
@@ -70,10 +70,12 @@ struct pack_idx_entry {
 	off_t offset;
 };
 
+struct progress;
+
 extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, unsigned char *sha1);
 extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);
 extern int verify_pack_index(struct packed_git *);
-extern int verify_pack(struct packed_git *);
+extern int verify_pack(struct packed_git *, struct progress *, uint32_t);
 extern void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);
 extern char *index_pack_lockfile(int fd);
 extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned char *);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Ben Walton @ 2011-11-04  2:11 UTC (permalink / raw)
  To: Eric Wong; +Cc: Jonathan Nieder, git
In-Reply-To: <20111102220941.GA3925@dcvr.yhbt.net>

Excerpts from Eric Wong's message of Wed Nov 02 18:09:41 -0400 2011:

Hi Eric,

> I don't have much time to help you fix it, but I got numerous errors
> on SVN 1.6.x (svn 1.6.12).  Can you make sure things continue to
> work on 1.6 and earlier, also?

Yes, it's a bit of a mess, I think.  It looks as though the
modification required within Git::SVN::Ra is going to negatively
impact other code paths that interact with that package from the
outside.

For example, when doing git svn init --minimize-url ..., the minimized
url is not escaped while the url is.  The minimized url is used to
strip off the head from the full url using a regex.  This now breaks
because of the escaping.

Fixing this locally to the use of the minimized url let me move on
farther but I then got another core dump.

> Maybe just enable the escaping for file:// on >= SVN 1.7

I think that it would be best if this change was only effective for
1.7.

I wonder if all URL-ish objects should be (conditionally iff svn >=
1.7) subjected to escaping?

This would require some restructuring and will take me a bit of time
to work out as I need to familiarize myself with the code to a deeper
level.

Pointers welcomed. :)

Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302

^ permalink raw reply

* [PATCHv2] Add options to specify snapshot file name, prefix
From: Thomas Guyot-Sionnest @ 2011-11-04  0:53 UTC (permalink / raw)
  To: git; +Cc: Thomas Guyot-Sionnest
In-Reply-To: <1320302318-28315-1-git-send-email-dermoth@aei.ca>

commit b629275 implemented "smart" snapshot names and prefixes. I have
scripts that used to rely on the old behaviour which allowed in some
cases to have fixed prefix, and would require modifications to work with
newer Gitweb.

This patch adds two parameters for overriding the snapshot name and
prefix, sn and sp respectively. For example, to get a snapshot
named "myproject.[suffix]" with no prefix one can add this query string:
  ?sn=myproject;sp=

Signed-off-by: Thomas Guyot-Sionnest <dermoth@aei.ca>
---
 gitweb/gitweb.perl |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4f0c3bd..9c91f01 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -756,6 +756,8 @@ our @cgi_param_mapping = (
 	searchtext => "s",
 	searchtype => "st",
 	snapshot_format => "sf",
+	snapshot_name => "sn",
+	snapshot_prefix => "sp",
 	extra_options => "opt",
 	search_use_regexp => "sr",
 	ctag => "by_tag",
@@ -6684,11 +6686,19 @@ sub git_snapshot {
 	}
 
 	my ($name, $prefix) = snapshot_name($project, $hash);
+	if (defined($input_params{'snapshot_name'})) {
+		$name = $input_params{'snapshot_name'};
+	}
+	if (defined($input_params{'snapshot_prefix'})) {
+		$prefix = $input_params{'snapshot_prefix'};
+	} else {
+		$prefix .= '/';
+	}
 	my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
 	my $cmd = quote_command(
 		git_cmd(), 'archive',
 		"--format=$known_snapshot_formats{$format}{'format'}",
-		"--prefix=$prefix/", $hash);
+		"--prefix=$prefix", $hash);
 	if (exists $known_snapshot_formats{$format}{'compressor'}) {
 		$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
 	}
-- 
1.7.7.1

^ permalink raw reply related

* Re: git rev-parse --since=1970-01-01 does not work reliably
From: Junio C Hamano @ 2011-11-04  0:36 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Dmitry V. Levin
In-Reply-To: <CACsJy8AewxbocqQ3gvgcrbSuNyKa0BCqDn6OV31m_6-P1AxJCA@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> 2011/11/1 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>> Subject: [PATCH] Do not accept negative time_t
>>
>> We use unsigned long internally to present time, negative value just
>> breaks thing.
>
> Junio, what do you think about this patch?

At this late point in the release cycle, the topic itself is a Meh for me,
especially when other discussions for next cycle occupies my attention.

^ permalink raw reply

* Re: git rev-parse --since=1970-01-01 does not work reliably
From: Nguyen Thai Ngoc Duy @ 2011-11-03 23:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Dmitry V. Levin
In-Reply-To: <20111031231320.GA3857@do>

2011/11/1 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> Subject: [PATCH] Do not accept negative time_t
>
> We use unsigned long internally to present time, negative value just
> breaks thing.

Junio, what do you think about this patch?

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  date.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/date.c b/date.c
> index 353e0a5..9cbd521 100644
> --- a/date.c
> +++ b/date.c
> @@ -653,8 +653,12 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
>        if (*timestamp == -1)
>                return -1;
>
> -       if (!tm_gmt)
> +       if (!tm_gmt) {
> +               if ((time_t)*timestamp < (time_t)*offset * 60)
> +                       die("unsupported time before Epoch");
>                *timestamp -= *offset * 60;
> +       }
> +
>        return 0; /* success */
>  }
>
> @@ -722,6 +726,8 @@ static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)
>
>        n = mktime(tm) - sec;
>        localtime_r(&n, tm);
> +       if (n < 0)
> +               die("unsupported time before Epoch");
>        return n;
>  }
>
> --
> 1.7.4.74.g639db
> -- 8< --
>



-- 
Duy

^ permalink raw reply

* Re: [PATCHv2] Improve use of select in http backend
From: Junio C Hamano @ 2011-11-03 23:14 UTC (permalink / raw)
  To: Mika Fischer; +Cc: git, Jeff King, daniel
In-Reply-To: <1320265288-12647-1-git-send-email-mika.fischer@zoopnet.de>

I re-read this thread once again, and my understanding of the current
situation on these two patch series is the following. Please correct me if
I am wrong.

 * This is not a regression fix, this is not a correctness fix, but it is
   a performance improvement;

 * Jeff gave an idea for improvement around the use of (rather, not having
   to use) data_received; and

 * Mika understood Jeff's suggestion, but was hesitant due to one
   potential issue around curl_multi_fdset() and asked Daniel's opinion,
   to which Daniel responded that the worrysome situation would not
   happen.

It appears to me that the next step is for Mika to decide either (1) we go
ahead with the original patch and leave the improvement for later, or (2)
update the patch as Jeff suggested and we review it again.

I can go either way, but whichever way you choose, I would want to see the
patches properly signed-off.

Thanks.

^ permalink raw reply

* Re: [ANNOUNCE] Git 1.7.8.rc0
From: Junio C Hamano @ 2011-11-03 23:02 UTC (permalink / raw)
  To: Stefan Naewe, Jeff King; +Cc: Michael J Gruber, git
In-Reply-To: <20111102200947.GA5628@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Normally I would say to implement in favor of the no-netrc case, as it
> is probably more common (and will hopefully be more so after the auth
> helpers are finished). But the problem is that the penalties are
> different. On the one hand, we have the extra http round-trip. Which is
> annoying, but mostly invisible to the user. But on the other, we have
> git prompting the user unnecessarily, which is just awful.

Ok, so are we in agreement that Stefan's patch $gmane/184617 is the right
fix at least for the time being?

This will be a minor regression if left unfixed, so I'd like to have a
minimum fix in before I tag -rc1 over the weekend.

Could any one of you guys please care to package it up with a readable
commit log message with a sign-off?

^ permalink raw reply

* What's cooking in git.git (Nov 2011, #01; Thu, 3)
From: Junio C Hamano @ 2011-11-03 22:16 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.

--------------------------------------------------
[New Topics]

* jc/stream-to-pack (2011-11-03) 4 commits
 - Bulk check-in
 - finish_tmp_packfile(): a helper function
 - create_tmp_packfile(): a helper function
 - write_pack_header(): a helper function

Teaches "git add" to send large-ish blob data straight to a packfile.
This is a continuation to the "large file support" topic. I think this
codepath to move data from worktree to repository needs to become aware of
streaming, just like the checkout codepath that goes the other way, which
was done in the previous "large file support" topic in the 1.7.7 cycle.

* jn/gitweb-side-by-side-diff (2011-10-31) 8 commits
 - gitweb: Add navigation to select side-by-side diff
 - gitweb: Use href(-replay=>1,...) for formats links in "commitdiff"
 - t9500: Add basic sanity tests for side-by-side diff in gitweb
 - t9500: Add test for handling incomplete lines in diff by gitweb
 - gitweb: Give side-by-side diff extra CSS styling
 - gitweb: Add a feature to show side-by-side diff
 - gitweb: Extract formatting of diff chunk header
 - gitweb: Refactor diff body line classification

Replaces a series from Kato Kazuyoshi on the same topic.

* vr/msvc (2011-10-31) 3 commits
 - MSVC: Remove unneeded header stubs
 - Compile fix for MSVC: Include <io.h>
 - Compile fix for MSVC: Do not include sys/resources.h

It seems this needs to be rehashed with msysgit folks.

* mf/curl-select-fdset (2011-11-02) 2 commits
 - http.c: Use timeout suggested by curl instead of fixed 50ms timeout
 - http.c: Use curl_multi_fdset to select on curl fds instead of just sleeping

* na/strtoimax (2011-11-02) 2 commits
 - Support sizes >=2G in various config options accepting 'g' sizes.
 - Add strtoimax() compatibility function.

--------------------------------------------------
[Graduated to "master"]

* dm/pack-objects-update (2011-10-20) 4 commits
  (merged to 'next' on 2011-10-27 at fa52898)
 + pack-objects: don't traverse objects unnecessarily
 + pack-objects: rewrite add_descendants_to_write_order() iteratively
 + pack-objects: use unsigned int for counter and offset values
 + pack-objects: mark add_to_write_order() as inline

* ef/mingw-upload-archive (2011-10-30) 3 commits
  (merged to 'next' on 2011-10-30 at 5267fa3)
 + upload-archive: use start_command instead of fork
 + compat/win32/poll.c: upgrade from upstream
 + mingw: move poll out of sys-folder

* jk/git-tricks (2011-10-21) 3 commits
  (merged to 'next' on 2011-10-23 at 7c9bf71)
 + completion: match ctags symbol names in grep patterns
 + contrib: add git-jump script
 + contrib: add diff highlight script

* nd/pretty-commit-log-message (2011-10-23) 2 commits
  (merged to 'next' on 2011-10-27 at 4b61df7)
 + pretty.c: use original commit message if reencoding fails
 + pretty.c: free get_header() return value

* ss/blame-textconv-fake-working-tree (2011-10-28) 2 commits
  (merged to 'next' on 2011-10-30 at 9588bc1)
 + (squash) test for previous
 + blame.c: Properly initialize strbuf after calling, textconv_object()

--------------------------------------------------
[Stalled]

* hv/submodule-merge-search (2011-10-13) 4 commits
 - submodule.c: make two functions static
 - allow multiple calls to submodule merge search for the same path
 - push: Don't push a repository with unpushed submodules
 - push: teach --recurse-submodules the on-demand option

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.

* jc/lookup-object-hash (2011-08-11) 6 commits
 - object hash: replace linear probing with 4-way cuckoo hashing
 - object hash: we know the table size is a power of two
 - object hash: next_size() helper for readability
 - pack-objects --count-only
 - object.c: remove duplicated code for object hashing
 - object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload.

* jc/verbose-checkout (2011-10-16) 2 commits
 - checkout -v: give full status output after switching branches
 - checkout: move the local changes report to the end

This is just to leave a record that the reason why we do not do this not
because we are incapable of coding this, but because it is not a good idea
to do this. I suspect people who are new to git that might think they need
it would soon realize the don't.

Will keep in 'pu' as a showcase for a while and then will drop.

--------------------------------------------------
[Cooking]

* nd/misc-cleanups (2011-10-27) 6 commits
  (merged to 'next' on 2011-10-28 at 2527a49)
 + unpack_object_header_buffer(): clear the size field upon error
 + tree_entry_interesting: make use of local pointer "item"
 + tree_entry_interesting(): give meaningful names to return values
 + read_directory_recursive: reduce one indentation level
 + get_tree_entry(): do not call find_tree_entry() on an empty tree
 + tree-walk.c: do not leak internal structure in tree_entry_len()

These are unquestionably good parts taken out of a larger series, so that
we can focus more on the other changes in later rounds of review.

Will keep in 'next' during this cycle.

* rs/allocate-cache-entry-individually (2011-10-26) 2 commits
  (merged to 'next' on 2011-10-27 at 2e4acd6)
 + cache.h: put single NUL at end of struct cache_entry
 + read-cache.c: allocate index entries individually

Will keep in 'next' during this cycle.

* mh/ref-api-3 (2011-10-19) 11 commits
  (merged to 'next' on 2011-10-23 at 92e2d35)
 + is_refname_available(): reimplement using do_for_each_ref_in_array()
 + names_conflict(): simplify implementation
 + names_conflict(): new function, extracted from is_refname_available()
 + repack_without_ref(): reimplement using do_for_each_ref_in_array()
 + do_for_each_ref_in_array(): new function
 + do_for_each_ref(): correctly terminate while processesing extra_refs
 + add_ref(): take a (struct ref_entry *) parameter
 + create_ref_entry(): extract function from add_ref()
 + parse_ref_line(): add a check that the refname is properly formatted
 + repack_without_ref(): remove temporary
 + Rename another local variable name -> refname
 (this branch uses mh/ref-api-2.)

Will keep in 'next' during this cycle.

* rr/revert-cherry-pick (2011-10-23) 5 commits
  (merged to 'next' on 2011-10-26 at 27b7496)
 + revert: simplify communicating command-line arguments
 + revert: allow mixed pick and revert instructions
 + revert: make commit subjects in insn sheet optional
 + revert: simplify getting commit subject in format_todo()
 + revert: free msg in format_todo()

The internals of "git revert/cherry-pick" has been further refactored to
serve as the basis for the sequencer.

Will keep in 'next' during this cycle.

* jc/check-ref-format-fixup (2011-10-19) 2 commits
  (merged to 'next' on 2011-10-19 at 98981be)
 + Revert "Restrict ref-like names immediately below $GIT_DIR"
  (merged to 'next' on 2011-10-15 at 8e89bc5)
 + Restrict ref-like names immediately below $GIT_DIR

This became a no-op except for the bottom one which is part of the other
topic now.
Will discard once the other topic graduates to 'master'.

* cb/daemon-permission-errors (2011-10-17) 2 commits
 - daemon: report permission denied error to clients
 - daemon: add tests

The tip commit might be loosening things a bit too much.
Will keep in 'pu' until hearing a convincing argument for the patch.

* mh/ref-api-2 (2011-10-17) 14 commits
  (merged to 'next' on 2011-10-19 at cc89f0e)
 + resolve_gitlink_ref_recursive(): change to work with struct ref_cache
 + Pass a (ref_cache *) to the resolve_gitlink_*() helper functions
 + resolve_gitlink_ref(): improve docstring
 + get_ref_dir(): change signature
 + refs: change signatures of get_packed_refs() and get_loose_refs()
 + is_dup_ref(): extract function from sort_ref_array()
 + add_ref(): add docstring
 + parse_ref_line(): add docstring
 + is_refname_available(): remove the "quiet" argument
 + clear_ref_array(): rename from free_ref_array()
 + refs: rename parameters result -> sha1
 + refs: rename "refname" variables
 + struct ref_entry: document name member
 + cache.h: add comments for git_path() and git_path_submodule()
 (this branch is used by mh/ref-api-3.)

Will keep in 'next' during this cycle.

* jc/signed-commit (2011-10-21) 7 commits
  (merged to 'next' on 2011-10-23 at 03eec25)
 + pretty: %G[?GS] placeholders
 + parse_signed_commit: really use the entire commit log message
 + test "commit -S" and "log --show-signature"
 + t7004: extract generic "GPG testing" bits
 + log: --show-signature
 + commit: teach --gpg-sign option
 + Split GPG interface into its own helper library

This is to replace the earlier "signed push" experiments.
Will keep in 'next' during this cycle.

* sg/complete-refs (2011-10-21) 9 commits
  (merged to 'next' on 2011-10-26 at d65e2b4)
 + completion: remove broken dead code from __git_heads() and __git_tags()
 + completion: fast initial completion for config 'remote.*.fetch' value
 + completion: improve ls-remote output filtering in __git_refs_remotes()
 + completion: query only refs/heads/ in __git_refs_remotes()
 + completion: support full refs from remote repositories
 + completion: improve ls-remote output filtering in __git_refs()
 + completion: make refs completion consistent for local and remote repos
 + completion: optimize refs completion
 + completion: document __gitcomp()

Will keep in 'next' until an Ack or two from completion folks.

* jc/request-pull-show-head-4 (2011-10-15) 11 commits
  (merged to 'next' on 2011-10-15 at 7e340ff)
 + fmt-merge-msg.c: Fix an "dubious one-bit signed bitfield" sparse error
  (merged to 'next' on 2011-10-10 at 092175e)
 + environment.c: Fix an sparse "symbol not declared" warning
 + builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
  (merged to 'next' on 2011-10-07 at fcaeca0)
 + fmt-merge-msg: use branch.$name.description
  (merged to 'next' on 2011-10-06 at fa5e0fe)
 + request-pull: use the branch description
 + request-pull: state what commit to expect
 + request-pull: modernize style
 + branch: teach --edit-description option
 + format-patch: use branch description in cover letter
 + branch: add read_branch_desc() helper function
 + Merge branch 'bk/ancestry-path' into jc/branch-desc

Allow setting "description" for branches and use it to help communications
between humans in various workflow elements.

Will keep in 'next' during this cycle.

--------------------------------------------------
[Discarded]

* kk/gitweb-side-by-side-diff (2011-10-17) 2 commits
 . gitweb: add a feature to show side-by-side diff
 . gitweb: change format_diff_line() to remove leading SP from $diff_class

jn/gitweb-side-by-side-diff replaces this.

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Jeff King @ 2011-11-03 21:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vwrbhdixe.fsf@alter.siamese.dyndns.org>

On Thu, Nov 03, 2011 at 01:56:13PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So you would agree that we are better summing the objects for all packs
> > and showing one big progress bar?
> 
> If it can be done without sacrificing the clarity of the code, compared to
> the "we will do new and smaller ones first so in practice it does not
> matter" approach taken by the patch in question, I would not mind it, but
> to be honest, I do not deeply care either way.

I looked briefly at doing this. It's a little annoying with the
verify_packs code, because you have to pass around the "how far are we
into the progress" counter separately. But I confess I don't care that
much either way, either. With the two minor fixups I sent in my original
review, I think Duy's patch would be OK by me.

-Peff

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Junio C Hamano @ 2011-11-03 20:56 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20111103202954.GC19483@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So you would agree that we are better summing the objects for all packs
> and showing one big progress bar?

If it can be done without sacrificing the clarity of the code, compared to
the "we will do new and smaller ones first so in practice it does not
matter" approach taken by the patch in question, I would not mind it, but
to be honest, I do not deeply care either way.

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Jeff King @ 2011-11-03 20:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v8vnxeyrp.fsf@alter.siamese.dyndns.org>

On Thu, Nov 03, 2011 at 01:28:42PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> Hmm, once you do this kind of thing I would expect two (or more) progress
> >> bars to be shown, something like:
> >> 
> >> 	$ git fsck --progress
> >> 	checking packs: 3 of 7 (42% done)
> >>      checking objects in pack: 12405 of 332340 (4% done)
> >
> > I don't think we can do multiple lines portably, though, because the
> > progress code just uses "\r" to return to the beginning of the line.
> 
> It was meant to be a tongue-in-cheek comment. I personally hate those
> "Installing n of N packages / Installation of package n p% done" progress
> meters when we know the weight of each of these N packages varies.

OK, I missed your sarcasm. You're too understated. ;)

So you would agree that we are better summing the objects for all packs
and showing one big progress bar?

-Peff

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Junio C Hamano @ 2011-11-03 20:28 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20111103195147.GA21318@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

>> Hmm, once you do this kind of thing I would expect two (or more) progress
>> bars to be shown, something like:
>> 
>> 	$ git fsck --progress
>> 	checking packs: 3 of 7 (42% done)
>>      checking objects in pack: 12405 of 332340 (4% done)
>
> I don't think we can do multiple lines portably, though, because the
> progress code just uses "\r" to return to the beginning of the line.

It was meant to be a tongue-in-cheek comment. I personally hate those
"Installing n of N packages / Installation of package n p% done" progress
meters when we know the weight of each of these N packages varies.

I also agree with your "how would you know which one is throughput and
which one is counts" comment, which would mean the particular abstraction
you mentioned is too premature even though it looks nice on the surface.

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Jeff King @ 2011-11-03 19:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vd3d9f0u8.fsf@alter.siamese.dyndns.org>

On Thu, Nov 03, 2011 at 12:43:59PM -0700, Junio C Hamano wrote:

> > But I almost wonder if it is worth factoring out the concept of a
> > "progress group", where you would call it like:
> >
> >   progress = progress_group_start("Checking objects in pack", nr_packs);
> >   for (p = packed_git; p; p = p->next) {
> >           progress_group_next(progress, p->num_objects);
> >           for (j = 0; j < num; j++) {
> >                   fsck_sha1(p, j);
> >                   display_progress(progress, j+1);
> >           }
> >   }
> >   stop_progress(&progress);
> 
> Hmm, once you do this kind of thing I would expect two (or more) progress
> bars to be shown, something like:
> 
> 	$ git fsck --progress
> 	checking packs: 3 of 7 (42% done)
>         checking objects in pack: 12405 of 332340 (4% done)

I don't think we can do multiple lines portably, though, because the
progress code just uses "\r" to return to the beginning of the line.

However, I do think it's a nice design even if the output is the same
right now, because the calling code is specifying more clearly to the
progress code what it actually means. Which means it is easier to
tweak the progress code later to make a prettier representation of that
meaning.

One downside (as with all abstractions :) ), is that it's hard to
deviate from the defaults. With the above, how would you specify if it
was a group of throughput measurements, not counts? Or that you wanted
to use start_progress_delay instead of start_progress on each one?

Or hey, if you really want to get crazy, why can't we check each pack in
parallel on a different CPU, and have all of the progress meters
displayed and moving simultaneously? :)

The last one is obviously a bit tongue in cheek. But it does raise an
interesting point: is seeing the per-pack meter actually useful (whether
parallel or not)? The user just wants to know that progress is being
made, and when it is done. And maybe that argues for just summing the
size of each pack and showing a single progress meter per task.

-Peff

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Junio C Hamano @ 2011-11-03 19:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <20111103193826.GB19483@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> a little more readable, or even:
>
>   stop_progress_msg(&progress, p->next ? NULL : "done\n");

Yeah, this one looks neat.

> But I almost wonder if it is worth factoring out the concept of a
> "progress group", where you would call it like:
>
>   progress = progress_group_start("Checking objects in pack", nr_packs);
>   for (p = packed_git; p; p = p->next) {
>           progress_group_next(progress, p->num_objects);
>           for (j = 0; j < num; j++) {
>                   fsck_sha1(p, j);
>                   display_progress(progress, j+1);
>           }
>   }
>   stop_progress(&progress);

Hmm, once you do this kind of thing I would expect two (or more) progress
bars to be shown, something like:

	$ git fsck --progress
	checking packs: 3 of 7 (42% done)
        checking objects in pack: 12405 of 332340 (4% done)

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Jeff King @ 2011-11-03 19:38 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320310234-11243-1-git-send-email-pclouds@gmail.com>

On Thu, Nov 03, 2011 at 03:50:34PM +0700, Nguyen Thai Ngoc Duy wrote:

>  2011/11/3 Jeff King <peff@peff.net>:
>  > We're actually leaking some memory here, since stop_progress will also
>  > free() the progress object and any associated resources. It's not a lot,
>  > but it's kind of ugly.
> 
>  Fixed by always calling stop_progress_msg() but make sure no newline
>  is printed (actually "done\n" is not)

Thanks, that sounds reasonable, and the output looks good.

>  > Or perhaps we could even come up with a total object count
>  > before starting.  I guess it would involve mapping each pack index
>  > simultaneously, though by my reading of the code, I think we do that
>  > anyway (the opened index is cached in the pack object).
> 
>  I think this way is better because we can count two numbers at a
>  time, nr. packs and nr. objects of current pack. Total object
>  (I assume you mean of all packs) may be less informative.

Yeah, I meant all of the packs. It's a little more accurate as a
progress meter, because you don't otherwise know what's in each of the
packs. For example, if I see that we are halfway through "pack 1/2",
does that mean we are a quarter of the way done? Not really. Pack 2/2
may be much smaller or much bigger. Finishing pack 1 may make us 99%
done, or it may make us 10% done.

Showing all of the objects in a flat list gives a more accurate
representation (though it's still not entirely accurate; even one
gigantic blob may dwarf the earlier objects. But it's the best we can
do).

In practice, though, I think people tend to have one really big pack and
some small ones. And the packs are sorted in recency order, so we'll
quickly go through the early little ones, and then spend all of our time
on the big old one. Unless they have .keep files.

So I don't really care that much. But it would also make the newline
stuff go away.

> +		for (i = 1, p = packed_git; p; p = p->next, i++) {
> +			if (show_progress) {
> +				char buf[32];
> +				snprintf(buf, sizeof(buf), "Verifying pack %d/%d",
> +					 i, nr_packs);
> +				if (open_pack_index(p))
> +					continue;
> +				progress = start_progress(buf, p->num_objects);
> +			}
>  			/* verify gives error messages itself */
> +			verify_pack(p, progress);
>  
> +			if (p->next)
> +				stop_progress_msg(&progress, NULL);
> +		}
> +		stop_progress(&progress);

You start the progress in the loop, but stop the final one outside the
loop. Which means that if there are no packs, we'll call stop_progress
even though we didn't start one. I think the progress code will handle
the NULL fine, but it took me a minute to convince myself it's right.

I would find this:

  if (p->next)
          stop_progress_msg(&progress, NULL);
  else
          stop_progress(&progress);

a little more readable, or even:

  stop_progress_msg(&progress, p->next ? NULL : "done\n");

But I almost wonder if it is worth factoring out the concept of a
"progress group", where you would call it like:

  progress = progress_group_start("Checking objects in pack", nr_packs);
  for (p = packed_git; p; p = p->next) {
          progress_group_next(progress, p->num_objects);
          for (j = 0; j < num; j++) {
                  fsck_sha1(p, j);
                  display_progress(progress, j+1);
          }
  }
  stop_progress(&progress);

and progress_set_next would take care of formatting the %d/%d counter,
and would not output a newline before writing the new description line.

> +           snprintf(buf, sizeof(buf), "Checking objects on pack %d/%d",
> +                    i, nr_packs);

s/on/in/

-Peff

^ permalink raw reply

* Re: Folder level Acces in git
From: Eugene Sajine @ 2011-11-03 19:28 UTC (permalink / raw)
  To: redhat1981; +Cc: git
In-Reply-To: <1320300655224-6958047.post@n2.nabble.com>

On Thu, Nov 3, 2011 at 2:10 AM, redhat1981 <redhat1981@gmail.com> wrote:
> Hi,
>
> I am using Gitosis, Adding the gitosis conf file
>
> [group testabc]
> writable = testabc
> members =   shrii Abhijeet premkumar
> add cgit = yes
> gitweb = yes
>
>
> [group testabc-readonly]
> readonly = testabc
> members =  Ganesh Shweta
> add cgit = yes
> gitweb = yes
>
> Inside the repository, testabc let us say there are folders folder1, folder
> 2 etc, I want some users to have read/write, read or no access to the
> folder1 or folder2, Is this possible in Git, I have done it in SVN, Please
> help!!
>
> redhat1981@gmail.com
>


Are you sure that the way your have organized the repository is
actually correct? If you need to manage the access on folder level why
don't you simply split up the project into several
repositories/projects which each team is going to work with
independently?

This seems to me to be much simpler and cleaner solution then any
other alternative.

Thanks,
Eugene

^ permalink raw reply

* Re: error from 'git push' on v1.7.8-rc0
From: Junio C Hamano @ 2011-11-03 19:15 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: gitlist
In-Reply-To: <7vmxcdf2x7.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Stefan Näwe <stefan.naewe@gmail.com> writes:
>
>> I get errors from git push when trying to delete a (remote) branch:
>>
>> $ ./bin-wrappers/git versiongit version 1.7.8.rc0
>> $ ./bin-wrappers/git push -q . :refs/heads/nogofatal: bad object
>> 0000000000000000000000000000000000000000fatal: bad object
>> 0000000000000000000000000000000000000000remote: warning: Allowing
>> deletion of corrupt ref.
>
> Thanks. I think the operation does _not_ error out and fail to delete, but
> I agree that the "fatal:" message should be squelched.

-- >8 --
Subject: receive-pack: do not expect object 0{40} to exist

When pushing to delete a ref, it uses 0{40} as an object name to signal
that the request is a deletion. We shouldn't trigger "deletion of a
corrupt ref" warning in such a case, which was designed to notice that a
ref points at an object that is truly missing from the repository.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/receive-pack.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 261b610..7ec68a1 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -634,7 +634,7 @@ static int command_singleton_iterator(void *cb_data, unsigned char sha1[20])
 	struct command **cmd_list = cb_data;
 	struct command *cmd = *cmd_list;
 
-	if (!cmd)
+	if (!cmd || is_null_sha1(cmd->new_sha1))
 		return -1; /* end of list */
 	*cmd_list = NULL; /* this returns only one */
 	hashcpy(sha1, cmd->new_sha1);
@@ -659,11 +659,16 @@ static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])
 	struct command **cmd_list = cb_data;
 	struct command *cmd = *cmd_list;
 
-	if (!cmd)
-		return -1; /* end of list */
-	*cmd_list = cmd->next;
-	hashcpy(sha1, cmd->new_sha1);
-	return 0;
+	while (cmd) {
+		if (!is_null_sha1(cmd->new_sha1)) {
+			hashcpy(sha1, cmd->new_sha1);
+			*cmd_list = cmd->next;
+			return 0;
+		}
+		cmd = cmd->next;
+	}
+	*cmd_list = NULL;
+	return -1; /* end of list */
 }
 
 static void execute_commands(struct command *commands, const char *unpacker_error)

^ permalink raw reply related

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-03 19:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7vvcr1f38j.fsf@alter.siamese.dyndns.org>

On Thu, Nov 3, 2011 at 11:52 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Ahh, sorry for the noise. I realize that we already have a winner, namely,
> the proposal outlined in your message I was responding to.

No, no, don't consider my "put in the merge message" a winner at all.

I personally dislike it, and don't really think it's a wonderful thing
at all. I really does have real downsides:

 - internal signatures really *are* a disaster for maintenance. You
can never fix them if they need fixing (and "need fixing" may well be
"you want to re-sign things after a repository format change")

 - they are ugly as heck, and you really don't want to see them in
99.999% of all cases.

So putting those things iin the merge commit message may have some
upsides, but it has tons of downsides too.

I think your refs/audit/ idea should be given real thought, because
maybe that's the right idea.

                           Linus

^ permalink raw reply

* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Linus Torvalds @ 2011-11-03 19:06 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Ted Ts'o, Shawn Pearce, git, James Bottomley, Jeff Garzik,
	Andrew Morton, linux-ide, LKML
In-Reply-To: <7v62j1gitn.fsf@alter.siamese.dyndns.org>

On Thu, Nov 3, 2011 at 11:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> It is not "Whatever".
>
>  $ git fetch git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v3.0
>  fatal: Couldn't find remote ref v3.0
>
> I do not think we ever DWIMmed fetch refspecs to prefix refs/tags/, so it
> is not the naming but fetching tags without saying "git fetch tag v3.0"
> (which IIRC was your invention long time ago).

Ahh. Yeah, and not DWIM'ing tags is probably ok. I'd completely
forgotten about the special "tag" shortcut.

Which probably means it was a bad ui decision to begin with. But once
more, the UI is clearly designed for fetching the tags into your own
tag-space (ie it does "refs/tags/<tag>:refs/tags/<tag>") rather than
fetching the tag just for verification.

> If we changed this "git fetch $there v3.0" to fetch tag, it would help the
> final step in your illustration, and I do not think it would be a huge
> regression---the only case it becomes fuzzy is when they have v3.0 branch
> at the same time, but the owner of such a repository is already playing
> with fire.

Yeah, extending DWIM for remote repos to do the same thing it does for
local repositories is probably the right thing regardless of any other
issues.

We already have the "tag and branch with the same name" issue for
local repositories, and we have perfectly good disambiguation rules
for when disambiguation is necessary. Making the DWIM rules be the
same for a remote case sounds sane.

That said, I don't think it's a big deal either. I was just confused
by the expansion being different, but having to have the refs/tags/
there isn't a dealbreaker by any means.

>> Quite frankly, I think that's a git bug, but it's a git bug because
>> "git fetch" was designed to get the commit to merge. Fair enough.
>
> And because FETCH_HEAD started as (and probably still is) an internal
> implementation detail of communication between fetch and merge inside
> pull.

Well, I certainly don't consider it to be just "an implementation
detail" personally. I use FETCH_HEAD all the time (the same way I use
ORIG_HEAD and just plain HEAD). It's very useful for "fetch and check
what they have", when you want to look at something but you don't want
all the remote tags and crud. So I consider it a honest-to-goodness
real user feature.

>So I do not have any issue in changing it to store tags unpeeled there.

In fact, storing the peeled was really surprising to me, especially
since it actually *says* "tag" in the .git/FETCH_HEAD file. So the
.git/FETCH_HEAD file really currently ends up being actively wrogn and
misleading for tags we fetch: it looks something like

  <sha-of-commit>  tag '<tagname>'  of <reponame>

and says it is a tag, but the SHA1 is of the peeled commit. That's
just crazy, and actually made me think the other end (Rusty, in this
case) had done something wrong initially (ie I quite reasonably - I
thought - blamed it on Rusty using a non-signed tag).

>> WTF?
>
> This is not WTF but "fetching a history to store the tip of it in your
> refs/ namespace causes tags pointing into the history line followed
> automatically", and it exactly is what you want to happen if rusty asked
> you to fetch his for-linus branch (which the tag may point at) instead.

Well, yes and no. But mostly no.

If I just fetch his for-linus branch, I don't get (and I don't want)
his tags. It's only because I fetched it into my ref-space.

And I only fetched it into my ref-space, because otherwise the crazy
git peeling happened if I don't do that.

So I didn't want those other tags, and I really normally wouldn't have
gotten them. Only because I had to do that odd work-around to avoid
the peeling did I get it, because then the totally unrelated logic of
"ok, get the tags too" triggered.

So it's a WTF, because this work-around ends up having the special
side effects - and they make sense when you *really* fetch his branch
and make it part of your name-space, but not when you only did the
"part of my namespace" as a workaround for another git issue.

Obviously, you can use "-n" (--no-tags) to fetch the tag, and that
actually fixes the issue, but that is it's own kind of WTF too: in
order to fetch just *one* tag, you have to specify that you don't want
tags? Not exactly a greatly intuitive use case ;)

Anyway, the one-line rpatch I sent basically avoids all these WTF
moments, by just making "git fetch <repo> <tagname>" work (apart from
the DWIMmery on the tag-name, but that's a totally independent small
detail that doesn't really matter)

>> We got three other
>> tags too that we didn't even ask for!
>
> We could change the rule to read "fetching a history to store the tip of it
> in your refs/heads namespace causes autofollow". I am not sure if that is
> what we really want, though.

No, I think the current "follow tags" rule is fine. It's just that it
didn't really mesh well with "damn, I have to work around this other
git issue".

> We could update three things:
>
>  - DWIM $name in "git fetch $there $name" to refs/tags/$name when it makes
>   sense;
>  - FETCH_HEAD stores unpeeled object names; and
>  - "git pull" learns --verify option.

Yes. I think that would indeed solve everything.

> Then
>
>  $ git pull --verify rusty rusty@rustcorp.com.au-v3.1-8068-g5087a50
>
> could integrate the history leading to that tag to your current branch
> while running verify-tag on it.

Agreed. The only remaining issue then would be how that "yes, I
verified the tag" part would be actually saved for posterity. My
suggestion would be to to just punt that question, and let the user
decide, by simply:

 - start the editor by default with "--verify"

 - output the "gpg --verify" result into the end of the commit file,
along with the tag content (which has the original pgp signature, of
course).

 - let the user decide what part of it he wants to use.

In particular, the "gpg --verify" result may well be something that
the user wants to *act* on - maybe the key didn't exist in the key
ring, or maybe it does exist but doesn't have quite enough trust and
gpg complains about that etc etc. But that's all something that "start
the editor and show the user what is up" would let the user decide on.

> For this, disabling the tag-auto-following is not necessary, as you are
> not storing the retrieved tag anywhere.

Exactly,

>> That said, I do think that the "signature in the pull request" should
>> also "just work", and I'm not entirely sure which one is better.
>
> I do not think it is necessarily either/or choice.

No, I think we can do both, and it actually ends up being just a
matter of convenience which one a particular project ends up using (or
even use both depending on preferences of particular sub-lieutenants
within the project).

> I wonder if an approach like the following, in addition to the three
> things I listed above, may give us a workable solution:
>
>  * "git fetch linus v3.0" called by "git pull --verify linus v3.0" fetches
>   the v3.0 unpeeled into FETCH_HEAD, GPG verifies it, creates
>   refs/audit/$u, before running "git merge". $u is derived from v3.0
>   (given tag), the identity of the GPG signer, and perhaps timestamp to
>   make it both identifiable and unique under refs/audit/ hierarchy.

So far so good, but see above: it may turn out that the user will
*re-verify* the key after having done some gpg action. So..

>  * You "git push origin". This causes refs/audit/* refs that point at
>   commits in the transferred history to auto-follow, just like the
>   current "git fetch $there $src:$dst" causes refs/tags/* auto-follow.
>   The refs/audit/* hierarchy in your public repository will be populated
>   by lieutenant signatures.

So I don't think auto-follow is good here.

I could *easily* see various companies using this for their own
internal audit, without really wanting to expose things outside of the
company. So auto-following sounds like the wrong approach. Make it an
explicit "expose audit checks" thing.

>  * (Optional) You may have signed "git tag -s 'Linux v3.2' v3.2 master"
>   before you push origin out, or you may have not. Currently, you do have
>   to "git push origin v3.2" separately if you did. The above auto-follow
>   could be extended to push refs/tags/* hierarchy to eliminate this step
>   as well.

So far I haven't really had any issues with having to do a "git push
--tags" to push things out.

That said, maybe the auto-push could just be a per-repo option, and
then you can have it both ways.

> Note that because of the way "upload-pack" protocol is structured, the
> first response from "upload-pack" after it gets connection is the
> advertisement of refs, and there is no way for "fetch-pack" to ask for
> customized refs advertisement to it. So for this to work without incurring
> undue overhead for normal users, we would need to exclude refs/audit/*
> from the normal ref advertisement (i.e. "ls-remote" does not see it) so
> that "git fetch" by casual users will not have to wait for megabytes of
> ref advertisements before issuing its first "want" request.

I think that would be a good thing, and make it much more palatable.
After all, th elikelihood is that *nobody* will ever care about the
audit cases at all. They are very much a "..but what if xyz happens"
kind of safety net for the extreme badness, not anything you'd expect
to use.

                         Linus

^ permalink raw reply

* Re: error from 'git push' on v1.7.8-rc0
From: Junio C Hamano @ 2011-11-03 18:59 UTC (permalink / raw)
  To: Stefan Näwe; +Cc: gitlist, Junio C Hamano
In-Reply-To: <CAJzBP5Q1_zX+H0jeBZNB81KLYAbtJWhUuHA3rf8CuW-_OSFXbg@mail.gmail.com>

Stefan Näwe <stefan.naewe@gmail.com> writes:

> I get errors from git push when trying to delete a (remote) branch:
>
> $ ./bin-wrappers/git versiongit version 1.7.8.rc0
> $ ./bin-wrappers/git push -q . :refs/heads/nogofatal: bad object
> 0000000000000000000000000000000000000000fatal: bad object
> 0000000000000000000000000000000000000000remote: warning: Allowing
> deletion of corrupt ref.

Thanks. I think the operation does _not_ error out and fail to delete, but
I agree that the "fatal:" message should be squelched.

^ 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