Git development
 help / color / mirror / Atom feed
* Re: [PATCH] push: Learn to set up branch tracking with '--track'
From: Jeff King @ 2009-01-30 16:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901301656290.3586@pacific.mpi-cbg.de>

On Fri, Jan 30, 2009 at 04:58:25PM +0100, Johannes Schindelin wrote:

> > So either we don't care about http-push being consistent with send-pack,
> > and it is OK to have this feature in one but not the other. Or we do,
> > and we really need to clean up the current divergence.
> 
> I do not see how your patch to send-pack makes that divergence any 
> better, or for that matter, keeps it as bad as it is.

No, it makes it worse. My point was that I am not sure people actually
_care_ that much about the divergence.

> In other words, if you want to give the other protocols at least a 
> _chance_ to catch up, you definitely need the support for push --track in 
> builtin-push.c or at least in transport.c.

But neither of those places has the information to do it _right_. I
think the right thing to do is:

  1. factor out "generic" routines from send-pack, including status
     output formatting and tracking ref updating

  2. refactor http-push to use those routines, bringing it in line with
     send-pack

  3. add --track support in the same generic way, and hook it from both
     transports

I can try to work on this, but I'm not excited about major surgery to
http-push, which I don't have a working test setup for.  I can't bring
myself to care about refactoring rsync, given the recent deprecation
discussion.

If it is going to be added to push or transport, then the transport API
needs refactoring to actually pass out information on what happened
(specifically, how we expanded the refspecs into matching ref pairs).
And maybe that is a more sensible long-term solution, but it is going
involve a lot of changes, too.

-Peff

^ permalink raw reply

* Re: [PATCH] Quick and dirty mergetool fix - take 2
From: Jonas Flodén @ 2009-01-30 16:22 UTC (permalink / raw)
  To: Charles Bailey; +Cc: git
In-Reply-To: <1233332166-14300-1-git-send-email-charles@hashpling.org>

Charles Bailey wrote:
> ---
>  git-mergetool.sh |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index aefdca7..d495a6b 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -131,7 +131,7 @@ checkout_staged_file () {
>      tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
>  
>      if test $? -eq 0 -a -n "$tmpfile" ; then
> -	mv -- "$tmpfile" "$3"
> +	mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
>      fi
>  }
>  
> @@ -161,9 +161,9 @@ merge_file () {
>      local_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}'`
>      remote_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}'`
>  
> -    base_present   && checkout_staged_file 1 "$prefix$MERGED" "$BASE"
> -    local_present  && checkout_staged_file 2 "$prefix$MERGED" "$LOCAL"
> -    remote_present && checkout_staged_file 3 "$prefix$MERGED" "$REMOTE"
> +    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
> +    local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
> +    remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
>  
>      if test -z "$local_mode" -o -z "$remote_mode"; then
>  	echo "Deleted merge conflict for '$MERGED':"

Yes, I was just about to propose a similar patch.

This solution works for me!

You might want to remove $prefix from the top of the
file since it's not used any more.

Thanks!

// Jonas

^ permalink raw reply

* Re: [PATCH] Quick and dirty mergetool fix - take 2
From: Charles Bailey @ 2009-01-30 16:18 UTC (permalink / raw)
  To: Jonas Flodén; +Cc: git
In-Reply-To: <1233332166-14300-1-git-send-email-charles@hashpling.org>

On Fri, Jan 30, 2009 at 04:16:06PM +0000, Charles Bailey wrote:

Sorry, I meant to add this comment to the last patch.

I have a test so I'm more confident about this version. It's not
signed off yet as I want to clean up the new test and make sure I've
considered everything - well more things - but it should at least work
now.

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Jay Soffian @ 2009-01-30 16:17 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de>

On Thu, Jan 29, 2009 at 7:34 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Many, many users set up non-bare repositories on their server, and are
> confused that the working directory is not updated.

This comes up on the list from time-to-time and is even in the FAQ. It
has even been suggested that HEAD be detached when pushing into a
non-bare repository, but I am not suggesting that again.

I wonder if it might be helpful to teach clone to setup a push line in
the cloned repo. i.e.:

[remote "origin"]
	url = ...
	fetch = +refs/heads/*:refs/remotes/origin/*
	push = refs/heads/*:refs/remotes/origin/*

This could be a configurable default behavior when cloning from a
non-bare repo (can that be determined?) and/or as a switch
(--satellite perhaps?).

j.

^ permalink raw reply

* [PATCH] Quick and dirty mergetool fix - take 2
From: Charles Bailey @ 2009-01-30 16:16 UTC (permalink / raw)
  To: Jonas Flodén; +Cc: git, Charles Bailey
In-Reply-To: <4983241B.6020208@gmail.com>

---
 git-mergetool.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index aefdca7..d495a6b 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -131,7 +131,7 @@ checkout_staged_file () {
     tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
 
     if test $? -eq 0 -a -n "$tmpfile" ; then
-	mv -- "$tmpfile" "$3"
+	mv -- "$(git rev-parse --show-cdup)$tmpfile" "$3"
     fi
 }
 
@@ -161,9 +161,9 @@ merge_file () {
     local_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}'`
     remote_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}'`
 
-    base_present   && checkout_staged_file 1 "$prefix$MERGED" "$BASE"
-    local_present  && checkout_staged_file 2 "$prefix$MERGED" "$LOCAL"
-    remote_present && checkout_staged_file 3 "$prefix$MERGED" "$REMOTE"
+    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
+    local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
+    remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
 	echo "Deleted merge conflict for '$MERGED':"
-- 
1.6.1.235.gc9d403

^ permalink raw reply related

* diff-index --cached on bare repository
From: Johannes Gilger @ 2009-01-30 16:13 UTC (permalink / raw)
  To: git

Hi list,

today I stumbled over the output of "git diff-index --cached HEAD" in a 
bare repository. While diff-index HEAD just reminds me that I got to 
have a work-tree, --cached outputs all the files in HEAD as deleted. The 
question is if this is intended behaviour? Or is it irrelevant because 
diff-index is only plumbing? I found a mail of the gitk-team working 
around this, which is funny because I discovered this while working on 
GitX.

So please, enlighten me ;)

Greetings,
Jojo

-- 
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* [TOY PATCH] grep: Add the option '--show-in-pager'
From: Johannes Schindelin @ 2009-01-30 16:04 UTC (permalink / raw)
  To: git
In-Reply-To: <cover.1233330920u.git.johannes.schindelin@gmx.de>

This adds an option to open the matching files in the pager, and if the
pager happens to be "less" and there is only one grep pattern, it also
jumps to the first match right away.

So, 'git grep -p abc' is a short form for 'less +/abc $(grep -l abc)'
except that it works also with spaces in file names, and it does not
start the pager if there was no matching file.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Yes, the name '--show-in-pager' sucks, but I have no idea how to 
	make the intention of the option clearer.

 builtin-grep.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 git.c          |    2 +-
 grep.c         |    6 +++++-
 grep.h         |    1 +
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index bebf15c..9d17cea 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -11,6 +11,7 @@
 #include "tree-walk.h"
 #include "builtin.h"
 #include "grep.h"
+#include "string-list.h"
 
 #ifndef NO_EXTERNAL_GREP
 #ifdef __unix__
@@ -379,6 +380,19 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 }
 #endif
 
+static void run_pager(struct grep_opt *opt)
+{
+	char **argv = xmalloc(sizeof(const char *)
+			* (opt->collect_file_names->nr + 1));
+	int i;
+
+	for (i = 0; i < opt->collect_file_names->nr; i++)
+		argv[i] = opt->collect_file_names->items[i].string;
+	argv[opt->collect_file_names->nr] = NULL;
+
+	execvp(argv[0], argv);
+}
+
 static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 {
 	int hit = 0;
@@ -424,6 +438,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 			nr--; /* compensate for loop control */
 		}
 	}
+	if (hit && opt->collect_file_names)
+		run_pager(opt);
 	free_grep_patterns(opt);
 	return hit;
 }
@@ -525,6 +541,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct grep_opt opt;
 	struct object_array list = { 0, 0, NULL };
 	const char **paths = NULL;
+	struct string_list show_in_pager = { NULL, 0, 0, 1 };
 	int i;
 
 	memset(&opt, 0, sizeof(opt));
@@ -613,6 +630,19 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.unmatch_name_only = 1;
 			continue;
 		}
+		if (!strcmp("-p", arg) ||
+		    !strcmp("--show-in-pager", arg)) {
+			const char *pager = getenv("GIT_PAGER");
+			if (!pager)
+				pager = getenv("PAGER");
+			if (!pager)
+				pager = "less";
+			builtin_grep = 1;
+			opt.name_only = 1;
+			opt.collect_file_names = &show_in_pager;
+			string_list_append(pager, &show_in_pager);
+			continue;
+		}
 		if (!strcmp("-z", arg) ||
 		    !strcmp("--null", arg)) {
 			opt.null_following_name = 1;
@@ -802,6 +832,24 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		paths[1] = NULL;
 	}
 
+	if (show_in_pager.nr && opt.pattern_list && !opt.pattern_list->next) {
+		const char *pager = show_in_pager.items[0].string;
+		int len = strlen(pager);
+
+		if (len > 4 && is_dir_sep(pager[len - 5]))
+			pager += len - 4;
+
+		if (!strcmp("less", pager)) {
+			struct strbuf buf = STRBUF_INIT;
+			strbuf_addf(&buf, "+/%s", opt.pattern_list->pattern);
+			string_list_append(buf.buf, &show_in_pager);
+			strbuf_release(&buf);
+		}
+	}
+
+	if (!show_in_pager.nr)
+		setup_pager();
+
 	if (!list.nr) {
 		if (!cached)
 			setup_work_tree();
@@ -817,6 +865,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		if (grep_object(&opt, paths, real_obj, list.objects[i].name))
 			hit = 1;
 	}
+	if (hit && show_in_pager.nr)
+		run_pager(&opt);
 	free_grep_patterns(&opt);
 	return !hit;
 }
diff --git a/git.c b/git.c
index 3c07567..dbd86ba 100644
--- a/git.c
+++ b/git.c
@@ -305,7 +305,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "fsck-objects", cmd_fsck, RUN_SETUP },
 		{ "gc", cmd_gc, RUN_SETUP },
 		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+		{ "grep", cmd_grep, RUN_SETUP },
 		{ "help", cmd_help },
 #ifndef NO_CURL
 		{ "http-fetch", cmd_http_fetch, RUN_SETUP },
diff --git a/grep.c b/grep.c
index 062b2b6..051f78d 100644
--- a/grep.c
+++ b/grep.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "grep.h"
 #include "xdiff-interface.h"
+#include "string-list.h"
 
 void append_header_grep_pattern(struct grep_opt *opt, enum grep_header_field field, const char *pat)
 {
@@ -265,7 +266,10 @@ static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
 
 static void show_name(struct grep_opt *opt, const char *name)
 {
-	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
+	if (opt->collect_file_names)
+		string_list_append(name, opt->collect_file_names);
+	else
+		printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
 }
 
 static int fixmatch(const char *pattern, char *line, regmatch_t *match)
diff --git a/grep.h b/grep.h
index 5102ce3..2b5aa06 100644
--- a/grep.h
+++ b/grep.h
@@ -79,6 +79,7 @@ struct grep_opt {
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
+	struct string_list *collect_file_names;
 };
 
 extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
-- 
1.6.1.2.531.g6f52

^ permalink raw reply related

* Re: [PATCH] Quick and dirty mergetool fix
From: Jonas Flodén @ 2009-01-30 16:00 UTC (permalink / raw)
  To: Charles Bailey; +Cc: git
In-Reply-To: <1233325351-31496-1-git-send-email-charles@hashpling.org>

Hmm, with that patch I get get this instead:

mv: cannot stat `.merge_file_Yg2IIs': No such file or directory
mv: cannot stat `.merge_file_4xGT8t': No such file or directory
mv: cannot stat `.merge_file_4XAVAt': No such file or directory

Which seems to be a problem in checkout_staged_file()

The .merge_file_* files exists in the repository root (the dir with
.git) but I don't know how to fix that..

Thanks!

// Jonas

^ permalink raw reply

* Re: Finding the name of the parent branch?
From: Pascal Obry @ 2009-01-30 15:58 UTC (permalink / raw)
  To: thomas.koch; +Cc: Git Mailing List
In-Reply-To: <200901301526.24566.thomas.koch@ymc.ch>

Thomas Koch a écrit :
> Isn't your problem solved by topgit?

Don't know! I have never used topgit. Why do you think so?

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* Re: [PATCH] push: Learn to set up branch tracking with '--track'
From: Johannes Schindelin @ 2009-01-30 15:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git, gitster
In-Reply-To: <20090130050925.GA18809@coredump.intra.peff.net>

Hi,

On Fri, 30 Jan 2009, Jeff King wrote:

> On Fri, Jan 30, 2009 at 01:28:38AM +0100, Johannes Schindelin wrote:
> 
> > > Something like the patch below (which is obviously missing all of the 
> > > infrastructure for doing this optionally, but is meant to illustrate 
> > > what I'm talking about).
> > 
> > Except that you miss http:// and rsync:// protocols.  Those were the 
> > reasons I did not touch send-pack.
> 
> You didn't comment on the part of my email where I said exactly that,
> but that I think this is still the right path forward.
> 
> Pushing through those protocols is sorely in need of update (actually,
> I thought rsync was all but dead at this point). But http push is
> missing the update of tracking refs, the usual status output (it still
> has the "Maybe you are not up-to-date and need to pull first?" message
> that was removed from send-pack a year and a half ago), and who knows
> what other tweaks made to do_send_pack (which it appears to have been
> copy-and-pasted from in 2006) in the last few years.
> 
> So either we don't care about http-push being consistent with send-pack,
> and it is OK to have this feature in one but not the other. Or we do,
> and we really need to clean up the current divergence.

I do not see how your patch to send-pack makes that divergence any 
better, or for that matter, keeps it as bad as it is.

In other words, if you want to give the other protocols at least a 
_chance_ to catch up, you definitely need the support for push --track in 
builtin-push.c or at least in transport.c.

Ciao,
Dscho

^ permalink raw reply

* Re: [StGit PATCH] Check for local changes with "goto"
From: Karl Hasselström @ 2009-01-30 15:26 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0901300601j27ab6ebdq4b38a9f7c0cbe261@mail.gmail.com>

On 2009-01-30 14:01:20 +0000, Catalin Marinas wrote:

> @@ -706,9 +706,15 @@ class Index(RunWithEnv):
>                      ).output_one_line())
>          except run.RunException:
>              raise MergeException('Conflicting merge')
> -    def is_clean(self):
> +    def is_clean(self, tree = None):
> +        """Check whether the index is clean relative to the given tree."""
> +        if tree:
> +            sha1 = tree.sha1
> +        else:
> +            sha1 = 'HEAD'
>          try:
> -            self.run(['git', 'update-index', '--refresh']).discard_output()
> +            self.run(['git', 'diff-index', '--quiet', '--cached', sha1]
> +                    ).discard_output()
>          except run.RunException:
>              return False
>          else:

OK (though I personally would have allowed only Tree objects, with no
defaulting to the current HEAD).

The docstring should say s/tree/treeish/.

> @@ -858,6 +864,15 @@ class IndexAndWorktree(RunWithEnvCwd):
>          cmd = ['git', 'update-index', '--remove']
>          self.run(cmd + ['-z', '--stdin']
>                   ).input_nulterm(paths).discard_output()
> +    def worktree_clean(self):
> +        """Check whether the worktree is clean relative and no updates or
> +        merges are needed."""
> +        try:
> +            self.run(['git', 'update-index', '--refresh']).discard_output()
> +        except run.RunException:
> +            return False
> +        else:
> +            return True

Clean relative to the index.

And what do merges have to do with it?

> +        # Check for not clean index and worktree
> +        if check_clean and iw:
> +            if not iw.worktree_clean():
> +                self.__halt('Repository not clean. Use "refresh" or '
> +                            '"status --reset"')
> +            elif not iw.index.is_clean()):
> +                self.__halt('Index and HEAD different. Use "repair" to '
> +                            'recover additional commits')

The first message is good, but the second one is misleading -- you'd
be much better off just reusing the same message as for the first
case. (You could recommend refresh --index if you want to get fancy.)

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* [PATCH --no-flowed] http-push: refactor request url creation
From: Tay Ray Chuan @ 2009-01-30 15:05 UTC (permalink / raw)
  To: git, Junio C Hamano

Hi,

Currently, functions that deal with objects on the remote repository
have to allocate and do strcpys to generate the URL.

This patch saves them this trouble, by providing two functions,
"append_remote_object_url" and "get_remote_object_url".

Both generate a URL, with either the object's 2-digit hex directory
(eg. /objects/a1/), or the complete object location (eg.
/objects/a1/b2).

However, they differ in that "append_remote_object_url" appends this
URL to a strbuf, while "get_remote_object_url" wraps around the former
and returns the URL directly in char*. Users usually would use
"get_remote_object_url", but may find "append_remote_object_url"
useful if they require further string operations on the URL.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Acked-by: Junio C Hamano <gitster@pobox.com>
---

* renamed only_two_digit_postfix in original patch to only_two_digit_prefix
* updated with Junio's changes (if (...) and fix memory leak)
* updated with Junio's double interface
* added back use of temporary string "url" in "start_fetch_loose"
* rebased and generated on master (a34a9db)

Crossing my fingers about format=flowed and linewraps.

 http-push.c |   62 +++++++++++++++++++++++-----------------------------------
 1 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/http-push.c b/http-push.c
index 59037df..816e658 100644
--- a/http-push.c
+++ b/http-push.c
@@ -209,6 +209,20 @@ static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum d
 	return dav_headers;
 }

+static void append_remote_object_url(struct strbuf *buf, const char *url, const char *hex, int only_two_digit_prefix)
+{
+	strbuf_addf(buf, "%sobjects/%.*s/", url, 2, hex);
+	if (!only_two_digit_prefix)
+		strbuf_addf(buf, "%s", hex+2);
+}
+
+static char *get_remote_object_url(const char *url, const char *hex, int only_two_digit_prefix)
+{
+	struct strbuf buf = STRBUF_INIT;
+	append_remote_object_url(&buf, url, hex, only_two_digit_prefix);
+	return strbuf_detach(&buf, NULL);
+}
+
 static void finish_request(struct transfer_request *request);
 static void release_request(struct transfer_request *request);

@@ -255,7 +269,6 @@ static void start_fetch_loose(struct transfer_request *request)
 	char *filename;
 	char prevfile[PATH_MAX];
 	char *url;
-	char *posn;
 	int prevlocal;
 	unsigned char prev_buf[PREV_BUF_SIZE];
 	ssize_t prev_read = 0;
@@ -305,17 +318,8 @@ static void start_fetch_loose(struct transfer_request *request)

 	git_SHA1_Init(&request->c);

-	url = xmalloc(strlen(remote->url) + 50);
-	request->url = xmalloc(strlen(remote->url) + 50);
-	strcpy(url, remote->url);
-	posn = url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	strcpy(request->url, url);
+	url = get_remote_object_url(remote->url, hex, 0);
+	request->url = xstrdup(url);

 	/* If a previous temp file is present, process what was already
 	   fetched. */
@@ -388,16 +392,8 @@ static void start_mkcol(struct transfer_request *request)
 {
 	char *hex = sha1_to_hex(request->obj->sha1);
 	struct active_request_slot *slot;
-	char *posn;

-	request->url = xmalloc(strlen(remote->url) + 13);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	strcpy(posn, "/");
+	request->url = get_remote_object_url(remote->url, hex, 1);

 	slot = get_active_slot();
 	slot->callback_func = process_response;
@@ -512,7 +508,7 @@ static void start_put(struct transfer_request *request)
 {
 	char *hex = sha1_to_hex(request->obj->sha1);
 	struct active_request_slot *slot;
-	char *posn;
+	struct strbuf buf = STRBUF_INIT;
 	enum object_type type;
 	char hdr[50];
 	void *unpacked;
@@ -551,21 +547,13 @@ static void start_put(struct transfer_request *request)

 	request->buffer.buf.len = stream.total_out;

-	request->url = xmalloc(strlen(remote->url) +
-			       strlen(request->lock->token) + 51);
-	strcpy(request->url, remote->url);
-	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
-	memcpy(posn, hex, 2);
-	posn += 2;
-	*(posn++) = '/';
-	strcpy(posn, hex + 2);
-	request->dest = xmalloc(strlen(request->url) + 14);
-	sprintf(request->dest, "Destination: %s", request->url);
-	posn += 38;
-	*(posn++) = '_';
-	strcpy(posn, request->lock->token);
+	strbuf_addstr(&buf, "Destination: ");
+	append_remote_object_url(&buf, remote->url, hex, 0);
+	request->dest = strbuf_detach(&buf, NULL);
+
+	append_remote_object_url(&buf, remote->url, hex, 0);
+	strbuf_addstr(&buf, request->lock->token);
+	request->url = strbuf_detach(&buf, NULL);

 	slot = get_active_slot();
 	slot->callback_func = process_response;
-- 
1.6.1.1.241.gc53a6.dirty

^ permalink raw reply related

* Re: [PATCH] Don't try to reclose in command_close_bidi_pipe
From: Yuval Kogman @ 2009-01-30 15:06 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0901300635v4e6009f1n7da405ebc4c8dd68@mail.gmail.com>

Resending, forgot to CC

2009/1/30 Tay Ray Chuan <rctay89@gmail.com>
> From this message, you seem to intend this as a fix. Can you tell us
> how one might go about to reproduce this issue?

Here is an example:
http://github.com/yanick/git-cpan-patch/blob/ed67d3f86f371764935fd0da3e7f08536c95b606/git-cpan-import#L190

Since git-commit-tree requires the message to be written before it can
write the sha1 of the new commit object, the handle has to be closed
already, which makes command_close_bidi_pipe die with an error.
The workaround in the linked code reopens a fake handle so that
close() will not error.
For what it's worth, I've been poking at t/t9700/test.pl but since
it's pretty sparse I figured I should contribute this fix
independently.

> Is "unless defined(fileno($fh))" a reliable way to check if the handle
> is closed?

Yes, since the handle is expected to be the result of a pipe open. The
documentation refers to this case:

    open my $fh, "<", \$buffer;

in which case the filehandle $fh has no associated file descriptor.

fileno() is not a universal check, but it is applicable in this situation.

^ permalink raw reply

* Re: Finding the name of the parent branch?
From: Thomas Koch @ 2009-01-30 14:26 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Git Mailing List
In-Reply-To: <a2633edd0901300256k2ad7530elcaca5cdc30c17534@mail.gmail.com>

Am Friday 30 January 2009 11:56:12 schrieb Pascal Obry:
> I want to find the name of the parent branch in a script. What is the best
> way?
>
>                          o---o---o---C
>                          /
>                     o---o---o---B
>                    /
>            ---o---o---o---o---o---A
>
> For B I want to get A and for C I want to get B.
>
> All this can be found in gitk, but from a script I did not found a
> good solution.
>
> Any idea?
>
> Thanks.
> Pascal.

Isn't your problem solved by topgit?

-- 
Thomas Koch, YMC AG, http://www.ymc.ch
Phone: +41 (0)71 / 508 24 86

^ permalink raw reply

* Re: Finding the name of the parent branch?
From: Santi Béjar @ 2009-01-30 14:46 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Git Mailing List
In-Reply-To: <a2633edd0901300606ub4c507bocccb6747b436f01f@mail.gmail.com>

2009/1/30 Pascal Obry <pascal@obry.net>:
>> At least part of. You have to understand the branch model:
>>
>> git model:
>>  * a branch is just a pointer to a commit
>>  * you cannot say "this commit was done in that branch"
>>  * what you can say is "this commit is contained in that branch"
>
> The second point wasn't clear to me.

A practical example:

$ git clone path/to/project.git
$ cd project
$ git checkout -b bugfix-1234 origin/master
$ # hack, hack, hack
$ git commit -a -m "Fix for the Bug #1234"
$ git push origin HEAD:master

So, you've done a commit in a local branch named bugfix-1234 and once
you push it to the master branch in origin there is nothing to tell
you so. A commit is defined with the current state, the old commit(s)
and some metadata (author and committer) but nothing about which
branch it was made, and as a branch is a pointer to a commit there is
nothing more.

Santi

^ permalink raw reply

* Re: [PATCH] Don't try to reclose in command_close_bidi_pipe
From: Tay Ray Chuan @ 2009-01-30 14:35 UTC (permalink / raw)
  To: Yuval Kogman; +Cc: git
In-Reply-To: <1233291570-23295-1-git-send-email-nothingmuch@woobling.org>

Hi,

On Fri, Jan 30, 2009 at 12:59 PM, Yuval Kogman <nothingmuch@woobling.org> wrote:
> Some commands require their standard input to be closed (like
> git-commit-tree). This patch changes command_close_bidi_pipe so no
> longer tries to close already closed handles, resulting in an error.

>From this message, you seem to intend this as a fix. Can you tell us
how one might go about to reproduce this issue?

> ---
>  perl/Git.pm |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/perl/Git.pm b/perl/Git.pm
> index 7d7f2b1..283bba8 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -422,6 +422,7 @@ sub command_close_bidi_pipe {
>        local $?;
>        my ($pid, $in, $out, $ctx) = @_;
>        foreach my $fh ($in, $out) {
> +               next unless defined(fileno($fh));

Quoting the perldoc for fileno at http://perldoc.perl.org/functions/fileno.html

"(Filehandles connected to memory objects via new features of open may
return undefined even though they are open.)"

Is "unless defined(fileno($fh))" a reliable way to check if the handle
is closed?

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Jeff King @ 2009-01-30 14:35 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, git, gitster
In-Reply-To: <alpine.DEB.1.00.0901301421170.3586@pacific.mpi-cbg.de>

On Fri, Jan 30, 2009 at 02:23:02PM +0100, Johannes Schindelin wrote:

> > On the other hand, you also missed the boat on receive.denyDeletes and
> > receive.denyNonFastForwards.
> 
> The idea with these is that they are _booleans_, and therefore
> 
> 	[receive]
> 		denyDeletes
> 
> is something natural to write, because "denyDeletes" is _not_ the default.
> 
> However, with denyCurrentBranch we wanted to change the default in the 
> long run, so I agree it was a not-so-brilliant choice.

Good point. I do agree that allowCurrentBranch would have been a better
name, but I don't know that it is worth the pain now of adding new
config plus supporting the old name forever.

-Peff

^ permalink raw reply

* [PATCH] Quick and dirty mergetool fix
From: Charles Bailey @ 2009-01-30 14:22 UTC (permalink / raw)
  To: Jonas Flodén; +Cc: git, Charles Bailey
In-Reply-To: <20090130140615.GA26321@hashpling.org>

---
 git-mergetool.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-mergetool.sh b/git-mergetool.sh
index aefdca7..db9ef43 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -161,9 +161,9 @@ merge_file () {
     local_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==2) print $1;}'`
     remote_mode=`git ls-files -u -- "$MERGED" | awk '{if ($3==3) print $1;}'`
 
-    base_present   && checkout_staged_file 1 "$prefix$MERGED" "$BASE"
-    local_present  && checkout_staged_file 2 "$prefix$MERGED" "$LOCAL"
-    remote_present && checkout_staged_file 3 "$prefix$MERGED" "$REMOTE"
+    base_present   && checkout_staged_file 1 "$MERGED" "$BASE"
+    local_present  && checkout_staged_file 2 "$MERGED" "$LOCAL"
+    remote_present && checkout_staged_file 3 "$MERGED" "$REMOTE"
 
     if test -z "$local_mode" -o -z "$remote_mode"; then
 	echo "Deleted merge conflict for '$MERGED':"
-- 
1.6.1.235.gc9d403

^ permalink raw reply related

* Re: Finding the name of the parent branch?
From: Pascal Obry @ 2009-01-30 14:06 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Git Mailing List
In-Reply-To: <adf1fd3d0901300557t19ca4bccn7fe24013a05d2d57@mail.gmail.com>

Santi,

Thanks for your patience!

> gitk topicbranch ^trackingbranch
>
> But I agree that a way to refer to the tracking branch would be great, as:
>
> branch^{origin}
>
> so you can say, for example:
>
> gitk topicbranch ^topicbranch^{origin}

That's exactly what I'm looking for!

> At least part of. You have to understand the branch model:
>
> git model:
>  * a branch is just a pointer to a commit
>  * you cannot say "this commit was done in that branch"
>  * what you can say is "this commit is contained in that branch"

The second point wasn't clear to me.

I had 2 out of 3 OK :)

Thanks.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Johannes Schindelin @ 2009-01-30 14:11 UTC (permalink / raw)
  To: Jeff King; +Cc: git, gitster
In-Reply-To: <20090130025546.GA18257@coredump.intra.peff.net>

Hi,

On Thu, 29 Jan 2009, Jeff King wrote:

> On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin wrote:
> 
> > 	Let's be honest here, I have not much respect for users who fail 
> > 	to read up enough to understand what they are doing.
> > 
> > 	But hearing from those users constantly is really unnerving.  And 
> > 	it would be a one-time cost to old-timers.
> 
> I am not personally opposed to changing this default. I seem to
> recall some opposition when this was brought up initially, but I don't
> recall any specific reason besides "change is bad". Maybe those who
> oppose want to summarize their arguments here.

We like to play it safe when changing behavior that does not meet 
expectations of old-timers.

For example, all those early adopters who have forks of the linux-2.6 
repository (and probably that repository itself, too) do not have 
core.bare set.

So whenever an old-timer would upgrade to a new Git _with_ my patch, they 
would need to change their setup.

A one-time cost.

And far easier to accomodate than the push for non-dashed commands (which 
people still seem to grumble about, even if they should have realized by 
now that calling Git through the wrapper exclusively brings so many 
advantages).

> I was hoping that introducing the warning would cause new users to "get 
> it". But since this warning was put in place, I think we have still 
> gotten a few questions on the list about this. I don't know if it simply 
> because they are on older versions, or if the warning is insufficient. 
> If the former, then perhaps that argues for leaving it a little longer.

I would argue it is because users cannot read :-)

> >  	case DENY_REFUSE:
> > -		if (!is_ref_checked_out(name))
> > +		if (is_bare_repository() || !is_ref_checked_out(name))
> 
> Now what is this change about?

I missed the fact that is_ref_checked_out() already checked for that.

> > --- a/t/t5701-clone-local.sh
> > +++ b/t/t5701-clone-local.sh
> > @@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '
> >  test_expect_success 'clone empty repository' '
> >  	cd "$D" &&
> >  	mkdir empty &&
> > -	(cd empty && git init) &&
> > +	(cd empty && git init && git config receive.denyCurrentBranch false) &&
> >  	git clone empty empty-clone &&
> >  	test_tick &&
> >  	(cd empty-clone
> 
> Perhaps some of these tests would do better to actually just use a bare
> repository.

Right.  I just ran out of time, but did not want to hide the patch from 
the community.

> That would better match the expected workflow for cloning empty, anyway.

Well, I did not want to mix up the two of them.  Besides, I have this 
patch in my personal tree for quite some time now, always wanting to clean 
it up enough to send it...)

Ciao,
Dscho

^ permalink raw reply

* Re: [StGit PATCH] Check for local changes with "goto"
From: Catalin Marinas @ 2009-01-30 14:01 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <20090129034512.GD24344@diana.vm.bytemark.co.uk>

2009/1/29 Karl Hasselström <kha@treskal.com>:
> On 2009-01-28 23:13:05 +0000, Catalin Marinas wrote:
>> +        # Check for not clean index
>> +        if check_clean and iw and not iw.index.is_clean():
>> +            self.__halt('Repository not clean. Use "refresh" or '
>> +                        '"status --reset"')
>
> ... this, which doesn't do what I think you think it does.
>
> Index.is_clean() calls "git update-index --refresh", which checks for
> changes in the worktree relative to the index. It's bad design to have
> it in Index rather than IndexAndWorktree, but that's my fault, not
> yours. ;-) But the point that breaks your patch is that it doesn't
> check for changes between index and HEAD -- try it and see.
>
> The fix I'd suggest is to move the existing is_clean() method to
> IndexAndWorktree, and call it maybe worktree_clean(). And create a
> method in Index() called is_clean(tree) that checks whether the index
> is clean with respect to the given Tree (I think this method should
> just call "git diff-index --quiet --cached <tree>".). Then call both
> of these methods.

What about this (only pasting the relevant hunks, though they may be
wrapped by the web interface):

diff --git a/stgit/lib/git.py b/stgit/lib/git.py
index e2b4266..7e1b9dd 100644
--- a/stgit/lib/git.py
+++ b/stgit/lib/git.py
@@ -706,9 +706,15 @@ class Index(RunWithEnv):
                     ).output_one_line())
         except run.RunException:
             raise MergeException('Conflicting merge')
-    def is_clean(self):
+    def is_clean(self, tree = None):
+        """Check whether the index is clean relative to the given tree."""
+        if tree:
+            sha1 = tree.sha1
+        else:
+            sha1 = 'HEAD'
         try:
-            self.run(['git', 'update-index', '--refresh']).discard_output()
+            self.run(['git', 'diff-index', '--quiet', '--cached', sha1]
+                    ).discard_output()
         except run.RunException:
             return False
         else:
@@ -858,6 +864,15 @@ class IndexAndWorktree(RunWithEnvCwd):
         cmd = ['git', 'update-index', '--remove']
         self.run(cmd + ['-z', '--stdin']
                  ).input_nulterm(paths).discard_output()
+    def worktree_clean(self):
+        """Check whether the worktree is clean relative and no updates or
+        merges are needed."""
+        try:
+            self.run(['git', 'update-index', '--refresh']).discard_output()
+        except run.RunException:
+            return False
+        else:
+            return True

 class Branch(object):
     """Represents a Git branch."""
diff --git a/stgit/lib/transaction.py b/stgit/lib/transaction.py
index 54de127..8abf296 100644
--- a/stgit/lib/transaction.py
+++ b/stgit/lib/transaction.py
@@ -183,13 +183,22 @@ class StackTransaction(object):
             self.__checkout(self.__stack.head.data.tree, iw,
                             allow_bad_head = True)
     def run(self, iw = None, set_head = True, allow_bad_head = False,
-            print_current_patch = True):
+            print_current_patch = True, check_clean = False):
         """Execute the transaction. Will either succeed, or fail (with an
         exception) and do nothing."""
         self.__check_consistency()
         log.log_external_mods(self.__stack)
         new_head = self.head

+        # Check for not clean index and worktree
+        if check_clean and iw:
+            if not iw.worktree_clean():
+                self.__halt('Repository not clean. Use "refresh" or '
+                            '"status --reset"')
+            elif not iw.index.is_clean()):
+                self.__halt('Index and HEAD different. Use "repair" to '
+                            'recover additional commits')
+
         # Set branch head.
         if set_head:
             if iw:


Thanks.

-- 
Catalin

^ permalink raw reply related

* Re: git mergetool from next not working in subdirectory
From: Charles Bailey @ 2009-01-30 14:06 UTC (permalink / raw)
  To: Jonas Flodén; +Cc: git
In-Reply-To: <glutv9$4b1$1@ger.gmane.org>

On Fri, Jan 30, 2009 at 02:05:45PM +0100, Jonas Flodén wrote:
> ~/src/rep/subdir $ git mergetool
> Merging the files: a/b/file.c
> 
> git checkout-index: subdir/subdir/a/b/file.c is not in the cache
> git checkout-index: subdir/subdir/a/b/file.c is not in the cache
> git checkout-index: subdir/subdir/a/b/file.c is not in the cache
> Normal merge conflict for 'a/b/file.c':
>   {local}: modified
> 
>   {remote}: modified


Oh bother. I know exactly what this is. ls-files -u shows paths
relative to the work tree root, whereas checkout-index responds to
relative paths. Essentially the "$prefix" needs to come out of the
calls to the new checkout_staged_file shell function.

This is the bad commit 0ec7b6c26dad51f690cd985d4a5357634123c4b7

I can patch this (and more importantly add a test for this) tonight if
nobody beats me to it.

-- 
Charles Bailey
http://ccgi.hashpling.plus.com/blog/

^ permalink raw reply

* Re: Finding the name of the parent branch?
From: Santi Béjar @ 2009-01-30 13:57 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Git Mailing List
In-Reply-To: <a2633edd0901300535mf2d9ceemd9e9009e0aa18b40@mail.gmail.com>

2009/1/30 Pascal Obry <pascal@obry.net>:
> Santi,
>
>> Maybe if you explain why you want it (a use case) instead of just this
>> specific problem...
>
> To know the proper merge base to display all commits done on a specific
> topic branch.
>

gitk topicbranch ^trackingbranch

But I agree that a way to refer to the tracking branch would be great, as:

branch^{origin}

so you can say, for example:

gitk topicbranch ^topicbranch^{origin}

>>>> Just a counterexample, just rearranging you graph:
>>>>
>>>>                        o---B
>>>>                        /
>>>>                   o---2---o---o---o---C
>>>>                  /
>>>>          ---o---1---o---o---o---A
>>>>
>>>> From you description: For B I would get C and for C I would get A.
>>
>> Please, if you quote text do not edit it (the 1 and the 2 in this case).
>
> Well I've just added 1 and 2, nothing changed in the semantic!
>
>> Yes. Compare your sentence and mine:
>>
>> For B I want to get A and for C I want to get B.
>> For B I would get C and for C I would get A.
>>
>> So for B you get A while I get C, and the equivalent for C.
>
> Ok, that's expected since you have renamed B to C and C to B.

I did not rename B to C and C to B, I just draw them differently.

Let's put name to the commits:

Your tree:


                        i---j---k---C
                        /
                   f---g---h---B
                  /
          ---a---b---c---d---e---A

My tree:

                       h---B
                       /
                  f---g---i---j---k---C
                 /
         ---a---b---c---d---e---A

So the commits in all the branches are equal, the only change is that
I painted B above C.

> So when I said:
>
>    For B I want to get A and for C I want to get B.
>
> It is equivalent to your (just rename B and C).:
>
>   For B I would get C and for C I would get A.
>
> Frankly I do not see your point... That's maybe the cause of the
> problem I'm having....

At least part of. You have to understand the branch model:

git model:
  * a branch is just a pointer to a commit
  * you cannot say "this commit was done in that branch"
  * what you can say is "this commit is contained in that branch"

in contrast to other models:
  * where a commit really belongs to a branch (it is specified at
commit time somehow)
  * you can say "this commit was done in that branch"

HTH,
Santi

^ permalink raw reply

* Re: git mergetool from next not working in subdirectory
From: Johannes Gilger @ 2009-01-30 13:50 UTC (permalink / raw)
  To: git
In-Reply-To: <glutv9$4b1$1@ger.gmane.org>

On 2009-01-30, Jonas Flodén <jonas.floden@gmail.com> wrote:
> I just upgraded to the current 'next' version of git
> ...
> This works perfectly fine with the 'master' version (1.6.1.2-253-ga34a).

Hi,

as you're running next you seem to be interested in the git development. 
That's why I suggest you try to bisect the commit that causes this. 
Fixing it is another problem entirely, but this way you'd at least take 
the tedious of work of a. duplicating your setup b. finding the bug off 
of the git developers.

Greetings,
Jojo

-- 
Johannes Gilger <heipei@hackvalue.de>
http://hackvalue.de/heipei/
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

^ permalink raw reply

* Re: Finding the name of the parent branch?
From: Pascal Obry @ 2009-01-30 13:35 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Git Mailing List
In-Reply-To: <adf1fd3d0901300516y3d1bf58dmda9c5172586d828@mail.gmail.com>

Santi,

> Maybe if you explain why you want it (a use case) instead of just this
> specific problem...

To know the proper merge base to display all commits done on a specific
topic branch.

>>> Just a counterexample, just rearranging you graph:
>>>
>>>                        o---B
>>>                        /
>>>                   o---2---o---o---o---C
>>>                  /
>>>          ---o---1---o---o---o---A
>>>
>>> From you description: For B I would get C and for C I would get A.
>
> Please, if you quote text do not edit it (the 1 and the 2 in this case).

Well I've just added 1 and 2, nothing changed in the semantic!

> Yes. Compare your sentence and mine:
>
> For B I want to get A and for C I want to get B.
> For B I would get C and for C I would get A.
>
> So for B you get A while I get C, and the equivalent for C.

Ok, that's expected since you have renamed B to C and C to B.

My tree was:


                        o---o---o---C
                        /
                   o---o---o---B
                  /
          ---o---o---o---o---o---A

Your's was:

                       o---B
                       /
                  o---o---o---o---o---C
                 /
         ---o---o---o---o---o---A

So when I said:

    For B I want to get A and for C I want to get B.

It is equivalent to your (just rename B and C).:

   For B I would get C and for C I would get A.

Frankly I do not see your point... That's maybe the cause of the
problem I'm having....

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ 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