Git development
 help / color / mirror / Atom feed
* Re: [PATCHv3] Make git-clone respect branch.autosetuprebase
From: Junio C Hamano @ 2009-03-04  6:29 UTC (permalink / raw)
  To: Pat Notz; +Cc: git
In-Reply-To: <1236111825-32159-1-git-send-email-pknotz@sandia.gov>

"Pat Notz" <pknotz@sandia.gov> writes:

> When git-clone creates an initial branch it was not
> checking the branch.autosetuprebase configuration
> option (which may exist in ~/.gitconfig).
>
> Added function to branch.c to handle autorebase
> configuration setup.
>
> Signed-off-by: Pat Notz <pknotz@sandia.gov>

Thanks.

Your patch still has two codepaths that set the branch.*.merge and other
information and they can diverge with later changes.  You can refactor
them even more to avoid that risk, like the attached patch.

The test is taken from your patch but runs inside a subshell, so that
later tests others may add to the script will be safer from the
environment variable settings and chdir this test does.

-- >8 --

When git-clone creates an initial branch it was not checking the
branch.autosetuprebase configuration option (which may exist in
~/.gitconfig).  Refactor the code used by "git branch" to create
a new branch, and use it instead of the insufficiently duplicated code
in builtin-clone.

---
 branch.c         |   49 +++++++++++++++++++++++++++++++++----------------
 branch.h         |    7 +++++++
 builtin-clone.c  |   18 +++---------------
 t/t5601-clone.sh |   15 +++++++++++++++
 4 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/branch.c b/branch.c
index 1f00e44..d20fb04 100644
--- a/branch.c
+++ b/branch.c
@@ -32,21 +32,48 @@ static int find_tracked_branch(struct remote *remote, void *priv)
 	return 0;
 }
 
-static int should_setup_rebase(const struct tracking *tracking)
+static int should_setup_rebase(const char *origin)
 {
 	switch (autorebase) {
 	case AUTOREBASE_NEVER:
 		return 0;
 	case AUTOREBASE_LOCAL:
-		return tracking->remote == NULL;
+		return origin == NULL;
 	case AUTOREBASE_REMOTE:
-		return tracking->remote != NULL;
+		return origin != NULL;
 	case AUTOREBASE_ALWAYS:
 		return 1;
 	}
 	return 0;
 }
 
+void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
+{
+	struct strbuf key = STRBUF_INIT;
+	int rebasing = should_setup_rebase(origin);
+
+	strbuf_addf(&key, "branch.%s.remote", local);
+	git_config_set(key.buf, origin ? origin : ".");
+
+	strbuf_reset(&key);
+	strbuf_addf(&key, "branch.%s.merge", local);
+	git_config_set(key.buf, remote);
+
+	if (rebasing) {
+		strbuf_reset(&key);
+		strbuf_addf(&key, "branch.%s.rebase", local);
+		git_config_set(key.buf, "true");
+	}
+
+	if (flag & BRANCH_CONFIG_VERBOSE)
+		printf("Branch %s set up to track %s branch %s %s.\n",
+		       local,
+		       origin ? "remote" : "local",
+		       remote,
+		       rebasing ? "by rebasing" : "by merging");
+	strbuf_release(&key);
+}
+
 /*
  * This is called when new_ref is branched off of orig_ref, and tries
  * to infer the settings for branch.<new_ref>.{remote,merge} from the
@@ -55,7 +82,6 @@ static int should_setup_rebase(const struct tracking *tracking)
 static int setup_tracking(const char *new_ref, const char *orig_ref,
                           enum branch_track track)
 {
-	char key[1024];
 	struct tracking tracking;
 
 	if (strlen(new_ref) > 1024 - 7 - 7 - 1)
@@ -80,19 +106,10 @@ static int setup_tracking(const char *new_ref, const char *orig_ref,
 		return error("Not tracking: ambiguous information for ref %s",
 				orig_ref);
 
-	sprintf(key, "branch.%s.remote", new_ref);
-	git_config_set(key, tracking.remote ?  tracking.remote : ".");
-	sprintf(key, "branch.%s.merge", new_ref);
-	git_config_set(key, tracking.src ? tracking.src : orig_ref);
-	printf("Branch %s set up to track %s branch %s.\n", new_ref,
-		tracking.remote ? "remote" : "local", orig_ref);
-	if (should_setup_rebase(&tracking)) {
-		sprintf(key, "branch.%s.rebase", new_ref);
-		git_config_set(key, "true");
-		printf("This branch will rebase on pull.\n");
-	}
-	free(tracking.src);
+	install_branch_config(BRANCH_CONFIG_VERBOSE, new_ref, tracking.remote,
+			      tracking.src ? tracking.src : orig_ref);
 
+	free(tracking.src);
 	return 0;
 }
 
diff --git a/branch.h b/branch.h
index 9f0c2a2..eed817a 100644
--- a/branch.h
+++ b/branch.h
@@ -21,4 +21,11 @@ void create_branch(const char *head, const char *name, const char *start_name,
  */
 void remove_branch_state(void);
 
+/*
+ * Configure local branch "local" to merge remote branch "remote"
+ * taken from origin "origin".
+ */
+#define BRANCH_CONFIG_VERBOSE 01
+extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote);
+
 #endif
diff --git a/builtin-clone.c b/builtin-clone.c
index c338910..a5f000a 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -20,6 +20,7 @@
 #include "dir.h"
 #include "pack-refs.h"
 #include "sigchain.h"
+#include "branch.h"
 
 /*
  * Overall FIXMEs:
@@ -350,19 +351,6 @@ static struct ref *write_remote_refs(const struct ref *refs,
 	return local_refs;
 }
 
-static void install_branch_config(const char *local,
-				  const char *origin,
-				  const char *remote)
-{
-	struct strbuf key = STRBUF_INIT;
-	strbuf_addf(&key, "branch.%s.remote", local);
-	git_config_set(key.buf, origin);
-	strbuf_reset(&key);
-	strbuf_addf(&key, "branch.%s.merge", local);
-	git_config_set(key.buf, remote);
-	strbuf_release(&key);
-}
-
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int use_local_hardlinks = 1;
@@ -553,7 +541,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		remote_head = NULL;
 		option_no_checkout = 1;
 		if (!option_bare)
-			install_branch_config("master", option_origin,
+			install_branch_config(0, "master", option_origin,
 					      "refs/heads/master");
 	}
 
@@ -583,7 +571,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				      head_points_at->peer_ref->name,
 				      reflog_msg.buf);
 
-			install_branch_config(head, option_origin,
+			install_branch_config(0, head, option_origin,
 					      head_points_at->name);
 		}
 	} else if (remote_head) {
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 44793f2..2335d8b 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -159,4 +159,19 @@ test_expect_success 'clone a void' '
 	test_cmp target-6/.git/config target-7/.git/config
 '
 
+test_expect_success 'clone respects global branch.autosetuprebase' '
+	(
+		HOME=$(pwd) &&
+		export HOME &&
+		test_config="$HOME/.gitconfig" &&
+		unset GIT_CONFIG_NOGLOBAL &&
+		git config -f "$test_config" branch.autosetuprebase remote &&
+		rm -fr dst &&
+		git clone src dst &&
+		cd dst &&
+		actual="z$(git config branch.master.rebase)" &&
+		test ztrue = $actual
+	)
+'
+
 test_done

^ permalink raw reply related

* Re: Subject: [PATCH] Push to create
From: Junio C Hamano @ 2009-03-04  6:35 UTC (permalink / raw)
  To: Jeff King; +Cc: Jay Soffian, Shawn O. Pearce, git
In-Reply-To: <20090304054211.GA3753@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Now we just need somebody who cares enough about this feature to work on
> it. ;)

We actually do not need anything.  But somebody who cares enough might
;-).

^ permalink raw reply

* Re: [PATCH v2] git-clone: Add option --branch to override initial branch
From: Junio C Hamano @ 2009-03-04  6:55 UTC (permalink / raw)
  To: Tor Arne Vestbø; +Cc: git, Johannes.Schindelin
In-Reply-To: <1236040414-19089-1-git-send-email-torarnv@gmail.com>

Tor Arne Vestbø <torarnv@gmail.com> writes:

> The options --branch and -b allow the user to override the initial
> branch created and checked out by git-clone (normally this is the
> active branch of the remote repository).
>
> If the selected branch is not found the operation aborts.
>
> Signed-off-by: Tor Arne Vestbø <torarnv@gmail.com>

The semantics and desirability of the new feature have been already
discussed, and I am not convinced that it is necessary, in the sense that
I do not think I likely ever use this myself, but I am just one of git
users so that is not a strong basis for rejection.

I'll let others discuss more about the design issues, and will only talk
about code in this message.

> diff --git a/builtin-clone.c b/builtin-clone.c
> index c338910..5fc01ce 100644
> --- a/builtin-clone.c
> +++ b/builtin-clone.c
> @@ -38,6 +38,7 @@ static int option_quiet, option_no_checkout, option_bare, option_mirror;
>  static int option_local, option_no_hardlinks, option_shared;
>  static char *option_template, *option_reference, *option_depth;
>  static char *option_origin = NULL;
> +static char *option_branch = NULL;

I see this was copied from the line immediately above, but please do not
initialize static variables to 0 or NULL.  BSS will take care of it.

> @@ -372,7 +375,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	const char *repo_name, *repo, *work_tree, *git_dir;
>  	char *path, *dir;
>  	int dest_exists;
> -	const struct ref *refs, *head_points_at, *remote_head, *mapped_refs;
> +	const struct ref *refs, *mapped_refs;
> +	const struct ref *remote_head = NULL;
> +	const struct ref *head_points_at = NULL;
>  	struct strbuf key = STRBUF_INIT, value = STRBUF_INIT;
>  	struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
>  	struct transport *transport = NULL;
> @@ -545,12 +550,31 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
>  
> -		head_points_at = locate_head(refs, mapped_refs, &remote_head);
> +		if (option_branch) {
> +		    const int offset = 11;
> +		    const char *branch = option_branch;

One indent level in git code equals a HT, i.e. 8 places.

> +		    if (!prefixcmp(branch, "refs/heads/"))
> +			branch += offset;

I suspect that you are trying to protect your code against somebody
miscounting the length of "refs/heads/" (perhaps when updating this
codepath in git version 47 that keeps local branches somewhere else, such
as "refs/local-heads/"), but this "const int offset" does not buy you
anything.  He will likely to leave "offset" to 11 just the same.

It is a different story if it were done like this:

		static const char heads_prefix[] = "refs/heads/";
                if (!prefixcmp(branch, heads_prefix))
                	branch += strlen(heads_prefix);

to let the compiler notice heads_prefix is a constant and optimize the
strlen() out, but I personally think it is overkill.

> +		    const struct ref *r;

We do not tolerate decl-after-statement.

> +		    for (r = mapped_refs; r; r = r->next) {
> +			if (!strcmp(r->name + offset, branch)) {
> +			    /* Override initial branch */
> +			    head_points_at = r;
> +			    remote_head = r;
> +			    break;
> +			}
> +		    }

This duplicates major part of what locate_head() does but with a different
target other than "master", doesn't it?

You would want to refactor this, but I think 'next/pu' already has some
refactoring of the locate_head() logic, so you may want to look at it and
either build your changes on top of it, or wait until that other topic to
stabilize.

> +		    if (!head_points_at)
> +			die("remote has no branch named '%s'.", option_branch);
> +
> +		} else {
> +		    head_points_at = locate_head(refs, mapped_refs, &remote_head);
> +		}

This falls into more personal taste than coding guideline, but it often is
easier to read to arrange your code:

	if (... condition ...) {
        	shorter codepath
	} else {
        	much
                longer
                code
                path
	}

For one thing, it is much easier to miss a short "else" clause hanging at
the end of loooong "if" part.

^ permalink raw reply

* Re: [PATCH 1/6] Modify description file to say what this file is
From: John Tapsell @ 2009-03-04  7:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vy6w2n3cz.fsf@gitster.siamese.dyndns.org>

Hi Junio,
  Did you commit my patches?  I don't see them yet in git.git ?

John Tapsell

2009/2/19 Junio C Hamano <gitster@pobox.com>:
> [PATCH 1/6] Modify description file to say what this file is
>
> Looks good.
>
> [PATCH 2/6] Google has renamed the imap folder
>
> Jeff already pointed out an obvious thinko; I could fix-up locally (just
> ask).
>
> [PATCH 3/6] Improve error message for branching an existing branch
>
> The extra sentence is useless noise to annoy users and make them shout
> "none of your business!" back to git.
>
> I would probably get this error message "already exists." more from
> forgetting to say "-f" in this sequence:
>
>    $ git branch -f pu next
>    $ git checkout pu
>    $ sh rebuild-pu-script
>
> to rebuild pu on top of updated next, and "did you mean to checkout?"
> misses the mark by a kilometer.
>
> [PATCH 4/6] Improve error message for git-filter-branch
>
> Looks good, with Sverre's rewording would be better, which I could locally
> squash in.  Needs signoff, which I could locally forge (just ask to fix-up
> and forge).
>
> [PATCH 5/6] Change output "error: " to "Error: " etc
>
> Jeff is right, and the patch is wrong.
>
> [PATCH 6/6] Mention to the user that they can reorder commits
>
> The placement of the new message does not feel right, as adding anything
> near "If you remove ... WILL BE LOST" will cloud out that message which is
> more important.
>
> I think it should come near or perhaps even before Commands, if we were to
> add anything here.
>
> But I am afraid that the proposed new message will hurt the clueless users
> more than it would help them.
>
> The cheat-sheet at the top is not for learning what the command can do for
> the first time.  It is there to remind people (who already have general
> idea on what can be done) how exactly the commands are spelled.  If
> somebody does not even know that the purpose of rebase-i is to amend and
> resequence, he will more likely destroy his history by blindly using the
> command without knowing what is going on, than making a lucky guess.
>
> For that reason, a more appropriate line to add, if we were to add
> anything, might be:
>
>  #  s, squash = use commit, but meld into previous commit
>  #
> +# If you do not know what is going on, remove everything and exit the editor!
> +#
>  # If you remove a line here THAT COMMIT WILL BE LOST.
>  # However, if you remove everything, the rebase will be aborted.
>
>

^ permalink raw reply

* Re: [TopGit] Multiple concurrent sets of patches
From: martin f krafft @ 2009-03-04  7:16 UTC (permalink / raw)
  To: Jonas Smedegaard, git, pasky, u.kleine-koenig
In-Reply-To: <20090303192221.GV12820@jones.dk>

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

also sprach Jonas Smedegaard <dr@jones.dk> [2009.03.03.2022 +0100]:
> I know that I can create all those TopGit branches one by one, but
> I would then need to explicitly declare a list of TopGit branches
> to apply each time I want to (re)generate a quilt patchlist.

There are two ways to achieve what you want with TopGit. Uwe
outlined one way:

- create a new branch depending on all the patch branches you want
  to use. This is what I advocated for packaging in Debian's topgit
  0.3-1 package:
  http://git.debian.org/?p=collab-maint/topgit.git;a=blob;f=debian/README.source;hb=debian/topgit-0.3-1

- declare the list of patches to use, as you suggest. This is what
  the current tg2quilt.mk approach of Debian's topgit 0.5-1 package
  does.

In the context where you have a single debian/rules file to prepare
a quilt series as part of building your package, I think the latter
makes more sense, as it keeps information in debian/rules and
alleviates the user of repetetive steps.

However, in the special context of a security fix, the suggestion
illustrated by Uwe probably makes a lot more sense. One reason for
this is because it is not yet possible to use TopGit patch branches
of the past, meaning that you can only ever use the tip of each, and
that tg-update basically destroys the infrastructure needed to go
back in time. By creating a special depending branch for the
security fix, however, you are preserving the graph at the time,
which is the tag you were alluding to.

This is what I can add to this discussion. I don't think I have
actually fully understood the scope of the problem yet.

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"no work of art ever puts forward views.
 views belong to people
 who are not artists."
                                                        -- oscar wilde
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [RFC/PATCH 1/2] improve missing repository error message
From: Jeff King @ 2009-03-04  8:32 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Theodore Tso, Junio C Hamano, git
In-Reply-To: <20090303184106.GH14365@spearce.org>

Certain remote commands, when asked to do something in a
particular directory that was not actually a git repository,
would say "unable to chdir or not a git archive". The
"chdir" bit is an unnecessary detail, and the term "git
archive" is much less common these days than "git repository".

So let's switch them all to:

  fatal: '%s' does not appear to be a git repository

Signed-off-by: Jeff King <peff@peff.net>
---
On Tue, Mar 03, 2009 at 10:41:06AM -0800, Shawn O. Pearce wrote:

> IMHO, maybe we also should change the error message that receive-pack
> produces when the path its given isn't a Git repository.  Its really
> not very human friendly to say "unable to chdir or not a git archive".
> Hell, we don't even call them archives, we call them repositories.

I agree completely.

Perhaps this should match the local "Not a git repository: %s". I prefer
this text, but maybe there is value in consistency.

 builtin-receive-pack.c   |    2 +-
 builtin-upload-archive.c |    2 +-
 daemon.c                 |    2 +-
 upload-pack.c            |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 849f1fe..a970b39 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -675,7 +675,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	setup_path();
 
 	if (!enter_repo(dir, 0))
-		die("'%s': unable to chdir or not a git archive", dir);
+		die("'%s' does not appear to be a git repository", dir);
 
 	if (is_repository_shallow())
 		die("attempt to push into a shallow repository");
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index a9b02fa..0206b41 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -35,7 +35,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
 	strcpy(buf, argv[1]); /* enter-repo smudges its argument */
 
 	if (!enter_repo(buf, 0))
-		die("not a git archive");
+		die("'%s' does not appear to be a git repository", buf);
 
 	/* put received options in sent_argv[] */
 	sent_argc = 1;
diff --git a/daemon.c b/daemon.c
index d93cf96..13401f1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -229,7 +229,7 @@ static char *path_ok(char *directory)
 	}
 
 	if (!path) {
-		logerror("'%s': unable to chdir or not a git archive", dir);
+		logerror("'%s' does not appear to be a git repository", dir);
 		return NULL;
 	}
 
diff --git a/upload-pack.c b/upload-pack.c
index 19c24db..e15ebdc 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -645,7 +645,7 @@ int main(int argc, char **argv)
 	dir = argv[i];
 
 	if (!enter_repo(dir, strict))
-		die("'%s': unable to chdir or not a git archive", dir);
+		die("'%s' does not appear to be a git repository", dir);
 	if (is_repository_shallow())
 		die("attempt to fetch/clone from a shallow repository");
 	if (getenv("GIT_DEBUG_SEND_PACK"))
-- 
1.6.2.rc2.24.g55bc2

^ permalink raw reply related

* [RFC/PATCH 2/2] make remote hangup warnings more friendly
From: Jeff King @ 2009-03-04  8:42 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Theodore Tso, Junio C Hamano, git
In-Reply-To: <20090303184106.GH14365@spearce.org>

When a user asks to do a remote operation but the remote
side thinks the operation is invalid (e.g., because the user
pointed to a directory which is not actually a repository),
then we generally end up showing the user two fatal
messages: one from the remote explaining what went wrong, and
one from the local side complaining of an unexpected hangup.

For example:

  $ git push /nonexistent master
  fatal: '/nonexistent' does not appear to be a git repository
  fatal: The remote end hung up unexpectedly

In this case, the second message is useless noise, and the
user is better off seeing just the first.

This patch tries to suppress the "hung up" message when it
is redundant (but still exits with an error code, of
course).

One heuristic would be to suppress it whenever the remote
has said something on stderr. Unfortunately, in many
transports we don't actually handle stderr ourselves; it
is simply a clone of the parent program's stderr and goes
straight to the terminal.

Instead, we note that the remote end will typically perform
such an "expected" hangup at the beginning of a packet
instead of in the middle. Therefore if we are expecting a
packet and get an end-of-file from the remote, we assume
they have printed something useful and exit without further
messages. Any other short read or eof is reported as before.

Signed-off-by: Jeff King <peff@peff.net>
---
There are two possible problems with this patch:

  1. The "beginning of packet" heuristic may not be the best. I
     tried a few obvious test cases like pushing and fetching with
     invalid repos and bogus --receive-pack= settings. All of them have
     useful output from the remote. You would of course get no message
     if the remote was cut off randomly at just the right spot.

    The "remote said something on stderr" heuristic does seem better.
    But we would have to start processing stderr for local and ssh
    connections, which we don't do currently. On the other hand, that
    might be nicer in the long run, because you can mark the remote
    errors as remote, which makes it more obvious what is going on.
    E.g.,:

      $ git push host:bogus master
      remote: fatal: 'bogus' does not appear to be a git repository

  2. Even "remote said something on stderr" may not be a desirable
     heuristic. In the case of a bogus --receive-pack, you get:

       $ git push --receive-pack=bogus host:repo master
       sh: bogus: command not found

     So you are losing the part where git says "fatal: ". Maybe it
     is obvious that such an error is fatal. It is to me, but I am not
     the intended audience.

     I think this would be improved somewhat with stderr processing to:

       remote: sh: bogus: command not found

     Or you could even trigger the suppression only if stderr had a line
     matching "^fatal:".

     Or it may even be that adding "remote:" is enough to make things
     less confusing:

       remote: fatal: 'bogus' does not appear to be a git repository
       fatal: The remote end hung up unexpectedly

     I think I still favor suppression in that case, but it is at least
     more clear why there are two fatals.

So you can see, the possibilities are endless. The perfect bikeshed. ;)

 pkt-line.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/pkt-line.c b/pkt-line.c
index f5d0086..f2b387c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -63,13 +63,16 @@ void packet_write(int fd, const char *fmt, ...)
 	safe_write(fd, buffer, n);
 }
 
-static void safe_read(int fd, void *buffer, unsigned size)
+static void safe_read(int fd, void *buffer, unsigned size, int eof_warn)
 {
 	ssize_t ret = read_in_full(fd, buffer, size);
 	if (ret < 0)
 		die("read error (%s)", strerror(errno));
-	else if (ret < size)
+	else if (ret < size) {
+		if (ret == 0 && !eof_warn)
+			exit(128);
 		die("The remote end hung up unexpectedly");
+	}
 }
 
 int packet_read_line(int fd, char *buffer, unsigned size)
@@ -78,7 +81,7 @@ int packet_read_line(int fd, char *buffer, unsigned size)
 	unsigned len;
 	char linelen[4];
 
-	safe_read(fd, linelen, 4);
+	safe_read(fd, linelen, 4, 0);
 
 	len = 0;
 	for (n = 0; n < 4; n++) {
@@ -103,7 +106,7 @@ int packet_read_line(int fd, char *buffer, unsigned size)
 	len -= 4;
 	if (len >= size)
 		die("protocol error: bad line length %d", len);
-	safe_read(fd, buffer, len);
+	safe_read(fd, buffer, len, 1);
 	buffer[len] = 0;
 	return len;
 }
-- 
1.6.2.rc2.24.g55bc2

^ permalink raw reply related

* Re: [PATCH v2] git-clone: Add option --branch to override initial branch
From: Johannes Schindelin @ 2009-03-04  8:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Tor Arne Vestbø, git
In-Reply-To: <7vbpsh93q5.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1036 bytes --]

Hi,

On Tue, 3 Mar 2009, Junio C Hamano wrote:

> Tor Arne Vestbø <torarnv@gmail.com> writes:
> 
> > +		    if (!prefixcmp(branch, "refs/heads/"))
> > +			branch += offset;
> 
> I suspect that you are trying to protect your code against somebody
> miscounting the length of "refs/heads/" (perhaps when updating this
> codepath in git version 47 that keeps local branches somewhere else, such
> as "refs/local-heads/"), but this "const int offset" does not buy you
> anything.  He will likely to leave "offset" to 11 just the same.
> 
> It is a different story if it were done like this:
> 
> 		static const char heads_prefix[] = "refs/heads/";
>                 if (!prefixcmp(branch, heads_prefix))
>                 	branch += strlen(heads_prefix);
> 
> to let the compiler notice heads_prefix is a constant and optimize the
> strlen() out, but I personally think it is overkill.

Of course you could also do this instead (which I personally think would 
not be overkill):

		branch = skip_prefix(branch, "refs/heads/");

Ciao,
Dscho

^ permalink raw reply

* Re: [RFC/PATCH 1/2] improve missing repository error message
From: Matthieu Moy @ 2009-03-04  9:19 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, Theodore Tso, Junio C Hamano, git
In-Reply-To: <20090304083229.GA31798@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> +		die("'%s' does not appear to be a git repository", dir);

It may be sensible to distinguish the case where dir exists as a
directory but isn't a repository, and the case where it does not exist
at all, like "directory exists but does not appear to be a git
repository" Vs "no such directory".

Just in case someone does a "mkdir" on the server and doesn't know he
has to "git init" there too.

My 2 cents,

-- 
Matthieu

^ permalink raw reply

* [ANNOUNCE] GIT 1.6.2
From: Junio C Hamano @ 2009-03-04  9:23 UTC (permalink / raw)
  To: git

The latest feature release GIT 1.6.2 is available at the usual
places:

  http://www.kernel.org/pub/software/scm/git/

  git-1.6.2.tar.{gz,bz2}			(source tarball)
  git-htmldocs-1.6.2.tar.{gz,bz2}		(preformatted docs)
  git-manpages-1.6.2.tar.{gz,bz2}		(preformatted docs)

The RPM binary packages for a few architectures are found in:

  RPMS//git-*-1.6.2-1.fc9.$arch.rpm	(RPM)

----------------------------------------------------------------
GIT v1.6.2 Release Notes
========================

With the next major release, "git push" into a branch that is
currently checked out will be refused by default.  You can choose
what should happen upon such a push by setting the configuration
variable receive.denyCurrentBranch in the receiving repository.

To ease the transition plan, the receiving repository of such a
push running this release will issue a big warning when the
configuration variable is missing.  Please refer to:

  http://git.or.cz/gitwiki/GitFaq#non-bare
  http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

for more details on the reason why this change is needed and the
transition plan.

For a similar reason, "git push $there :$killed" to delete the branch
$killed in a remote repository $there, if $killed branch is the current
branch pointed at by its HEAD, gets a large warning.  You can choose what
should happen upon such a push by setting the configuration variable
receive.denyDeleteCurrent in the receiving repository.


Updates since v1.6.1
--------------------

(subsystems)

* git-svn updates.

* gitweb updates, including a new patch view and RSS/Atom feed
  improvements.

* (contrib/emacs) git.el now has commands for checking out a branch,
  creating a branch, cherry-picking and reverting commits; vc-git.el
  is not shipped with git anymore (it is part of official Emacs).

(performance)

* pack-objects autodetects the number of CPUs available and uses threaded
  version.

(usability, bells and whistles)

* automatic typo correction works on aliases as well

* @{-1} is a way to refer to the last branch you were on.  This is
  accepted not only where an object name is expected, but anywhere
  a branch name is expected and acts as if you typed the branch name.
  E.g. "git branch --track mybranch @{-1}", "git merge @{-1}", and
  "git rev-parse --symbolic-full-name @{-1}" would work as expected.

* When refs/remotes/origin/HEAD points at a remote tracking branch that
  has been pruned away, many git operations issued warning when they
  internally enumerated the refs.  We now warn only when you say "origin"
  to refer to that pruned branch.

* The location of .mailmap file can be configured, and its file format was
  enhanced to allow mapping an incorrect e-mail field as well.

* "git add -p" learned 'g'oto action to jump directly to a hunk.

* "git add -p" learned to find a hunk with given text with '/'.

* "git add -p" optionally can be told to work with just the command letter
  without Enter.

* when "git am" stops upon a patch that does not apply, it shows the
  title of the offending patch.

* "git am --directory=<dir>" and "git am --reject" passes these options
  to underlying "git apply".

* "git am" learned --ignore-date option.

* "git blame" aligns author names better when they are spelled in
  non US-ASCII encoding.

* "git clone" now makes its best effort when cloning from an empty
  repository to set up configuration variables to refer to the remote
  repository.

* "git checkout -" is a shorthand for "git checkout @{-1}".

* "git cherry" defaults to whatever the current branch is tracking (if
  exists) when the <upstream> argument is not given.

* "git cvsserver" can be told not to add extra "via git-CVS emulator" to
  the commit log message it serves via gitcvs.commitmsgannotation
  configuration.

* "git cvsserver" learned to handle 'noop' command some CVS clients seem
  to expect to work.

* "git diff" learned a new option --inter-hunk-context to coalesce close
  hunks together and show context between them.

* The definition of what constitutes a word for "git diff --color-words"
  can be customized via gitattributes, command line or a configuration.

* "git diff" learned --patience to run "patience diff" algorithm.

* "git filter-branch" learned --prune-empty option that discards commits
  that do not change the contents.

* "git fsck" now checks loose objects in alternate object stores, instead
  of misreporting them as missing.

* "git gc --prune" was resurrected to allow "git gc --no-prune" and
  giving non-default expiration period e.g. "git gc --prune=now".

* "git grep -w" and "git grep" for fixed strings have been optimized.

* "git mergetool" learned -y(--no-prompt) option to disable prompting.

* "git rebase -i" can transplant a history down to root to elsewhere
  with --root option.

* "git reset --merge" is a new mode that works similar to the way
  "git checkout" switches branches, taking the local changes while
  switching to another commit.

* "git submodule update" learned --no-fetch option.

* "git tag" learned --contains that works the same way as the same option
  from "git branch".


Fixes since v1.6.1
------------------

All of the fixes in v1.6.1.X maintenance series are included in this
release, unless otherwise noted.

Here are fixes that this release has, but have not been backported to
v1.6.1.X series.

* "git-add sub/file" when sub is a submodule incorrectly added the path to
  the superproject.

* "git bundle" did not exclude annotated tags even when a range given
  from the command line wanted to.

* "git filter-branch" unnecessarily refused to work when you had
  checked out a different commit from what is recorded in the superproject
  index in a submodule.

* "git filter-branch" incorrectly tried to update a nonexistent work tree
  at the end when it is run in a bare repository.

* "git gc" did not work if your repository was created with an ancient git
  and never had any pack files in it before.

* "git mergetool" used to ignore autocrlf and other attributes
  based content rewriting.

* branch switching and merges had a silly bug that did not validate
  the correct directory when making sure an existing subdirectory is
  clean.

* "git -p cmd" when cmd is not a built-in one left the display in funny state
  when killed in the middle.

^ permalink raw reply

* Re: How does Git know which files no longer needed during upgrade?
From: Jeff King @ 2009-03-04  9:49 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: dealmaker, git
In-Reply-To: <20090303233058.GE4371@genesis.frugalware.org>

On Wed, Mar 04, 2009 at 12:30:58AM +0100, Miklos Vajna wrote:

> First, I think storing upstream code (that you will never touch) in
> version control is a horrible idea, but if you really do it, I would do
> something like:
> 
> cd /path/to/copy
> rm -rf *
> cp -a /path/to/new/version/* .
> git add -A
> git commit -m 'update foo to 2.0'

Nit: "rm -rf *" will miss files starting with '.'. So it is probably
simpler to say what you mean: delete all files managed by git:

  git ls-files -z | xargs -0 rm -f

-Peff

^ permalink raw reply

* Re: [PATCH 1/6] Modify description file to say what this file is
From: Jeff King @ 2009-03-04 10:00 UTC (permalink / raw)
  To: John Tapsell; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <43d8ce650903032307i512268f4sa3240c517b51d0b4@mail.gmail.com>

On Wed, Mar 04, 2009 at 07:07:14AM +0000, John Tapsell wrote:

> Hi Junio,
>   Did you commit my patches?  I don't see them yet in git.git ?

Try looking in 'next', which is where new patches cook before going to
master (especially now, as we are right in the middle of release freeze
on master).

  git log --author=John.Tapsell origin/next

-Peff

^ permalink raw reply

* Re: [PATCH 1/6] Modify description file to say what this file is
From: Junio C Hamano @ 2009-03-04 10:01 UTC (permalink / raw)
  To: John Tapsell; +Cc: Git Mailing List
In-Reply-To: <43d8ce650903032307i512268f4sa3240c517b51d0b4@mail.gmail.com>

John Tapsell <johnflux@gmail.com> writes:

> Hi Junio,
>   Did you commit my patches?  I don't see them yet in git.git ?

Please look for them in the 'next' branch.

Patches that came after -rc1 are at best queued in 'next' for post 1.6.2.
The only exceptions are critical and obvious fixes, and fixes to
regressions introduced since 1.6.1.

^ permalink raw reply

* Re: Why isn't hook file cloned to bared repository ?
From: Johannes Schindelin @ 2009-03-04 10:04 UTC (permalink / raw)
  To: Bryan Donlan; +Cc: Emily Ren, git
In-Reply-To: <3e8340490903032213u56734301o39c9d00633410fd5@mail.gmail.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1123 bytes --]

Hi,

On Wed, 4 Mar 2009, Bryan Donlan wrote:

> On Wed, Mar 4, 2009 at 12:40 AM, Emily Ren <lingyan.ren@gmail.com> wrote:
>
> > I added file "update" in my git repository my_repo/.git/hooks/,  then
> > I run command "git clone --bare my_repo" to generate a bared
> > repository my_repo.git. But there's no update in my_repo.git/hooks.
> >
> > Do you know why ?
> 
> Because allowing code from an untrusted third-party repository to be
> executed automatically without giving a chance to examine it is not a
> very good idea from a security standpoint. In addition, hooks are
> often not of interest to the person cloning the repository. Because of
> these reasons, git clone will not copy hooks from the source
> repository (for consistency, this is the case even when the source is
> local).

I might add that hooks are not part of the repository.  They are not 
versioned, for example.

Having said that, nothing prevents you from committing a set of example 
hooks and a script to install them, and tell your users that they may 
install default hooks using that script.  I do that for one of my 
projects.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] git-clone.txt: document that pushing from a shallow clone may work
From: Adeodato Simó @ 2009-03-04 10:19 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Johannes Sixt, git, gitster, Mikael Magnusson, Joey Hess
In-Reply-To: <76718490903031127v7fcec124je7292c4c465208b8@mail.gmail.com>

* Jay Soffian [Tue, 03 Mar 2009 14:27:29 -0500]:

> On Tue, Mar 3, 2009 at 7:08 AM, Adeodato Simó <dato@net.com.org.es> wrote:
> > Well, I don't know if the set of cases where it'll work can be defined
> > in detail to a point where it is useful. If it is, then sure, let's do
> > it.

> > My point is that if it will work in some cases, then the documentation
> > should *acknowledge that fact*, because else people will assume the
> > documentation is wrong, and believe it is intended to work, which is not
> > the case.

> Wy not just say "pushing into a shallow repository is not supported"
> instead of "pushing into a shallow repository won't work."

I don't think such a wording is enough (adjusted, of course, to be about
pushing from, not to, which is the case at hand).

But I'll try to stay silent, and see if Junio has an opinion on the
matter.

Thanks,

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
- Oh my God, you're pimping me out for a new roof?
- And windows!
                -- Andrew and Bree Van De Kamp

^ permalink raw reply

* Re: [PATCH v2] git-clone: Add option --branch to override initial branch
From: Tor Arne Vestbø @ 2009-03-04 10:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes.Schindelin
In-Reply-To: <7vbpsh93q5.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> I'll let others discuss more about the design issues, and will only talk
> about code in this message.

[...snip...]

Great feedback, much appreciated! :) I'll work up a new patch as soon as 
I have some free cycles. Thanks!

Tor Arne

^ permalink raw reply

* Re: remote branches, and branch names in general
From: Jeff King @ 2009-03-04 10:29 UTC (permalink / raw)
  To: Jay Soffian; +Cc: John Dlugosz, Jakub Narebski, git
In-Reply-To: <76718490903031132v592b8368p6355ea2bd0cda4ae@mail.gmail.com>

On Tue, Mar 03, 2009 at 02:32:00PM -0500, Jay Soffian wrote:

> On Tue, Mar 3, 2009 at 11:11 AM, Jeff King <peff@peff.net> wrote:
> > Yes, the "branch" command deals only with creating things in refs/heads,
> 
> Unless given -r, in which case it looks in refs/remotes, or -a, in
> which case it looks in refs/heads and refs/remotes. :-)

OK, fair enough. ;P

But I think the lesson to be learned is that there is a difference
between arbitrary ref lookup (which you might use with log, show, diff,
or even as the branching-off point for branch) and specialized commands
which assume you are working in some part of the ref hierarchy (heads,
tags, or even remotes).

-Peff

^ permalink raw reply

* Re: [RFC/PATCH 1/2] improve missing repository error message
From: Jeff King @ 2009-03-04 10:35 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: Shawn O. Pearce, Theodore Tso, Junio C Hamano, git
In-Reply-To: <vpqtz69lk6a.fsf@bauges.imag.fr>

On Wed, Mar 04, 2009 at 10:19:25AM +0100, Matthieu Moy wrote:

> > +		die("'%s' does not appear to be a git repository", dir);
> 
> It may be sensible to distinguish the case where dir exists as a
> directory but isn't a repository, and the case where it does not exist
> at all, like "directory exists but does not appear to be a git
> repository" Vs "no such directory".
> 
> Just in case someone does a "mkdir" on the server and doesn't know he
> has to "git init" there too.

I agree it would be nice to be more descriptive; however, it's not quite
as simple as checking if the chdir failed. For "non-strict" repo lookup,
we check a number of variants. For $foo, we check:

  $foo.git/.git
  $foo/.git
  $foo.git
  $foo

If $foo exists but isn't a git directory, but $foo/.git does not exist,
then what is the correct response?  I guess we can say "no such
directory" only if $foo and $foo.git don't exist, and "not a git
repository" for the others.

I also don't know if we care about information leakage; with such a
patch can I now start probing the existence of arbitrary directories via
git-daemon (I haven't checked to see if there are other path
restrictions in place)?

-Peff

^ permalink raw reply

* Re: [PATCH] git-clone.txt: document that pushing from a shallow clone may work
From: Junio C Hamano @ 2009-03-04 10:45 UTC (permalink / raw)
  To: Adeodato Simó
  Cc: Jay Soffian, Johannes Sixt, git, Mikael Magnusson, Joey Hess
In-Reply-To: <20090304101939.GA7142@chistera.yi.org>

Adeodato Simó <dato@net.com.org.es> writes:

>> Wy not just say "pushing into a shallow repository is not supported"
>> instead of "pushing into a shallow repository won't work."
>
> I don't think such a wording is enough (adjusted, of course, to be about
> pushing from, not to, which is the case at hand).
>
> But I'll try to stay silent, and see if Junio has an opinion on the
> matter.

I would be a terrible judge for things like this; I lost my git virginity
long time ago.

If I have to say something on this...

 * I think "is not supported" is a succinct way to give good enough
   information, but it would only work for intelligent people.

 * Not everybody is intelligent; some try it out themselves, see that the
   operation _seems to_ work for their limited number of trials, and would
   conclude it would work most of the time.  And they congratulate their
   own intelligence for saying "most of the time", not "always".  And they
   get upset when they see it does not work, even though they have been
   warned.

 * Hence, I do not think "is not supported" is a statement that is a bit
   too weak.  At least you need to say "it may seem to work, but no
   guarantees", _if_ your objective is to cover the backside of "shallow".

But I do not think that is what we should be aiming for to begin with.

It is not like nobody can precisely answer when "pushing from shallow"
works and when it doesn't.  It would be true for a hack that was not well
designed but merely was meant to be "good enough for most of the time",
but I do not think "shallow" is that horrible a hack.

Isn't the rule more or less like:

    If your shallow repository's history does not extend long enough and
    the other repository forked before your truncated history, wyou cannot
    compute the common ancestor and you cannot push out.

^ permalink raw reply

* Re: [ANNOUNCE] GIT 1.6.2
From: Junio C Hamano @ 2009-03-04 10:50 UTC (permalink / raw)
  To: git
In-Reply-To: <7veixd7iac.fsf@gitster.siamese.dyndns.org>

I've pushed out only 'master' which is at v1.6.2 tonight; I also have
prepared the other integration branches, rebased js/notes as promised, and
have already rewound 'next', but it appears that I may not have my git
Wednesday tomorrow, and I'd like to do a bit more sanity checks on the
result, so the public repositories will likely to stay the same for a day.

^ permalink raw reply

* Re: [RFC PATCH] Windows: Assume all file names to be UTF-8 encoded.
From: Peter Krefting @ 2009-03-04 10:51 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git
In-Reply-To: <37fcd2780903030913q4ee0e5d0t45dc3b124285f748@mail.gmail.com>

Dmitry Potapov:

> No, it does not, if you have wchar_t that is only 16-bit wide, because 
> characters outside of the BMP have integer values in Unicode greater than 
> 65535...

UTF-16 allows you to reference all of Unicode (i.e up to U+10FFFF) using 
surrogate pairs. That means that not all characters can be represented as a 
single wchar_t, that is true. The problem with changing wchar_t is that it 
was defined to use 16-bit values at a time where Unicode was defined to use 
16-bit code points (but they soon figured out that was not enough).

Anyway, this is getting off-topic. Please feel free reply in private.

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [RFC PATCH] Windows: Assume all file names to be UTF-8 encoded.
From: Peter Krefting @ 2009-03-04 10:53 UTC (permalink / raw)
  To: John Dlugosz; +Cc: git
In-Reply-To: <450196A1AAAE4B42A00A8B27A59278E709F076D2@EXCHANGE.trad.tradestation.com>

John Dlugosz:

> Actually, UTF-8 is a valid code page on Windows.

Yes, but I am unsure whether it can be set as a thread locale for the sake 
of file APIs.

> Also, there is a national code page that =will= represent all file names 
> on the systems and is supported: That is the Chinese GB18030, code page 
> 54936.

Yeah, but unfortunately it is explicitly documented that it is only 
supported in MultiByteToWideChar, WideCharToMultiByte and some text painting 
APIs in Windows, i.e the stdio functions and others may break horribly.

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [PATCH] git-clone.txt: document that pushing from a shallow clone may work
From: Johannes Schindelin @ 2009-03-04 11:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Adeodato Simó, Jay Soffian, Johannes Sixt, git,
	Mikael Magnusson, Joey Hess
In-Reply-To: <7vvdqp5zx9.fsf@gitster.siamese.dyndns.org>

Hi,

On Wed, 4 Mar 2009, Junio C Hamano wrote:

> Isn't the rule more or less like:
> 
>     If your shallow repository's history does not extend long enough and
>     the other repository forked before your truncated history, wyou cannot
>     compute the common ancestor and you cannot push out.

Exactly.

Back when I wrote the shallow code, I meant to investigate how to detect 
that condition, and fail gracefully (i.e. with a meaningful error 
message).

IMHO that is more needed than documentation changes, as it affects more 
users ;-)

But in the meantime, I had no use for shallow clones, and kind of hoped 
that it would become somebody else's itch.

Ciao,
Dscho

^ permalink raw reply

* can not clone via git:// anymore
From: Hinko Kocevar @ 2009-03-04 11:24 UTC (permalink / raw)
  To: git

Hi,

I've recently discovered that my GIT repository is not letting me clone it via git clone git://.
It works using git clone git@.. (SSH) but with GIT protocol..

Here is the case:
hinkok@alala /tmp $ git --version
git version 1.6.0.6
hinkok@alala /tmp $ git clone git://zidar/sdk.git
Initialized empty Git repository in /tmp/sdk/.git/
fatal: The remote end hung up unexpectedly

My earlier clone (few months old) has this in .git/config:
hinkok@alala /work/git/sdk.git $ cat .git/config 
[core]
	repositoryformatversion = 0
	filemode = true
	bare = false
	logallrefupdates = true
[remote "origin"]
	url = git@zidar:repositories/sdk.git
	fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
	remote = origin
	merge = refs/heads/master


But I remember cloning via SSH that time, because I needed git push to work,
but now other developer do not need the SSH access, but only GIT read-only -
they can only clone, not push.

I do not administer the server git repos is located on, so I must have some
information before I attack the admin..

Thank you,
Hinko

-- 
Hinko Kočevar, OSS developer
ČETRTA POT, d.o.o.
Planina 3, 4000 Kranj, SI EU
tel     ++386 (0) 4 280 66 03
e-mail  hinko.kocevar@cetrtapot.si
http    www.cetrtapot.si

^ permalink raw reply

* Re: How does Git know which files no longer needed during upgrade?
From: Stefan Näwe @ 2009-03-04 12:28 UTC (permalink / raw)
  To: git
In-Reply-To: <20090304094951.GA32433@coredump.intra.peff.net>

Jeff King <peff <at> peff.net> writes:

> 
> > cd /path/to/copy
> > rm -rf *
> > cp -a /path/to/new/version/* .
> > git add -A
> > git commit -m 'update foo to 2.0'
> 
> Nit: "rm -rf *" will miss files starting with '.'. So it is probably
> simpler to say what you mean: delete all files managed by git:
> 
>   git ls-files -z | xargs -0 rm -f
> 
> -Peff

But maybe one wants to keep a .gitignore file. 

Regrads,
Stefan

^ 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