* Re: [PATCH (GITK)] gitk: Fix transient windows on Win32 and MacOS.
From: Johannes Sixt @ 2008-11-12 8:28 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Alexander Gavrilov, git
In-Reply-To: <18714.36964.666759.654975@cargo.ozlabs.ibm.com>
Paul Mackerras schrieb:
> Johannes Sixt writes:
>> Alexander Gavrilov schrieb:
>>> Transient windows cause problems on these platforms:
>> ...
>>> diff --git a/gitk b/gitk
>>> index 9b2a6e5..e6aafe8 100755
>> I'd appreciate if you could make it a habit to base your patches on
>> versions of gitk etc. that are available from a public repository
>
> You mean, like, git://git.kernel.org/pub/scm/gitk/gitk.git, for
> instance? :) That is the primary repository for gitk and it seems to
> be what Alexander bases his patches on.
Yes, I mean exactly this repository. It doesn't have 9b2a6e5:
$ git fetch gitk
$ git show 9b2a6e5 --
fatal: bad revision '9b2a6e5'
I assume that Alexander has another patch applied in addition to the one
that he submitted, which, therefore, is no longer "based on a publically
available version".
-- Hannes
^ permalink raw reply
* [PATCH 5/5] commit: only strip diff from message in verbose mode
From: Jeff King @ 2008-11-12 8:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20081112081609.GA3720@coredump.intra.peff.net>
When the "-v" option is given, we put diff of what is to be
committed into the commit template, and then strip it back
out again after the user has edited it.
We guess at the location of the diff by searching for the
"diff --git" header. This means that if the user puts their
own diff in the message (e.g., as a sample output), then we
will accidentally trigger the pattern, removing part of
their output.
We can avoid doing this stripping altogether if the user
didn't use "-v" in the first place, so we know that any
match we find will be a false positive.
Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this doesn't solve the problem entirely, but it at least makes
a reasonable rule: if we accidentally eat your sample diff output with
"-v", then try without it. :)
But more importantly, I suspect "-v" is used infrequently, so in
practice this should fix most cases.
builtin-commit.c | 8 +++++---
t/t7507-commit-verbose.sh | 21 +++++++++++++++++++++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index a721990..591d16b 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1015,9 +1015,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
/* Truncate the message just before the diff, if any. */
- p = strstr(sb.buf, "\ndiff --git ");
- if (p != NULL)
- strbuf_setlen(&sb, p - sb.buf + 1);
+ if (verbose) {
+ p = strstr(sb.buf, "\ndiff --git ");
+ if (p != NULL)
+ strbuf_setlen(&sb, p - sb.buf + 1);
+ }
if (cleanup_mode != CLEANUP_NONE)
stripspace(&sb, cleanup_mode == CLEANUP_ALL);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index be70166..2a3ea7a 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -42,4 +42,25 @@ test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
check_message message
'
+cat >diff <<'EOF'
+This is an example commit message that contains a diff.
+
+diff --git c/file i/file
+new file mode 100644
+index 0000000..f95c11d
+--- /dev/null
++++ i/file
+@@ -0,0 +1 @@
++this is some content
+EOF
+
+test_expect_success 'diff in message is retained without -v' '
+ git commit --amend -F diff &&
+ check_message diff
+'
+
+test_expect_failure 'diff in message is retained with -v' '
+ git commit --amend -F diff -v &&
+ check_message diff
+'
test_done
--
1.6.0.4.883.g4593ee.dirty
^ permalink raw reply related
* [PATCH 4/5] commit: loosen pattern for matching "-v" diff
From: Jeff King @ 2008-11-12 8:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20081112081609.GA3720@coredump.intra.peff.net>
When a user asks to see the diff to be applied via "git
commit -v", the diff ends up in the commit template, and we
must later remove it.
To detect the start of the included diff, we used to search
for a line beginning with "diff --git a/". However, in the
face of diff.mnemonicprefix, that will actually be "diff
--git i/".
So let's just loosen the pattern a bit to handle either
case.
Signed-off-by: Jeff King <peff@peff.net>
---
And this is the fix from before, with a test case.
builtin-commit.c | 2 +-
t/t7507-commit-verbose.sh | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 93ca496..a721990 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1015,7 +1015,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
/* Truncate the message just before the diff, if any. */
- p = strstr(sb.buf, "\ndiff --git a/");
+ p = strstr(sb.buf, "\ndiff --git ");
if (p != NULL)
strbuf_setlen(&sb, p - sb.buf + 1);
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 94b12e9..be70166 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -26,4 +26,20 @@ test_expect_success 'initial commit shows verbose diff' '
git commit --amend -v
'
+check_message() {
+ git log -1 --pretty=format:%s%n%n%b >actual &&
+ test_cmp "$1" actual
+}
+
+test_expect_success 'verbose diff is stripped out' '
+ git commit --amend -v &&
+ check_message message
+'
+
+test_expect_success 'verbose diff is stripped out (mnemonicprefix)' '
+ git config diff.mnemonicprefix true &&
+ git commit --amend -v &&
+ check_message message
+'
+
test_done
--
1.6.0.4.883.g4593ee.dirty
^ permalink raw reply related
* [PATCH 3/5] status: show "-v" diff even for initial commit
From: Jeff King @ 2008-11-12 8:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20081112081609.GA3720@coredump.intra.peff.net>
Since we can use the same "diff against empty tree" trick as
we do for the non-initial case, it is trivial to make this
work.
Signed-off-by: Jeff King <peff@peff.net>
---
I ran across this while writing my test cases for the later patches. I
don't see any reason why it shouldn't behave this way, and I suspect the
reason was just that the original author saw how we did
"wt_status_print_initial" and didn't want to have to do the equivalent
for generating diff output.
t/t7507-commit-verbose.sh | 29 +++++++++++++++++++++++++++++
wt-status.c | 5 +++--
2 files changed, 32 insertions(+), 2 deletions(-)
create mode 100755 t/t7507-commit-verbose.sh
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
new file mode 100755
index 0000000..94b12e9
--- /dev/null
+++ b/t/t7507-commit-verbose.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='verbose commit template'
+. ./test-lib.sh
+
+cat >check-for-diff <<EOF
+#!$SHELL_PATH
+exec grep '^diff --git' "\$1"
+EOF
+chmod +x check-for-diff
+test_set_editor "$PWD/check-for-diff"
+
+cat >message <<'EOF'
+subject
+
+body
+EOF
+
+test_expect_success 'setup' '
+ echo content >file &&
+ git add file &&
+ git commit -F message
+'
+
+test_expect_success 'initial commit shows verbose diff' '
+ git commit --amend -v
+'
+
+test_done
diff --git a/wt-status.c b/wt-status.c
index c78588e..3edae43 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -279,7 +279,8 @@ static void wt_status_print_verbose(struct wt_status *s)
struct rev_info rev;
init_revisions(&rev, NULL);
- setup_revisions(0, NULL, &rev, s->reference);
+ setup_revisions(0, NULL, &rev,
+ s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
rev.diffopt.detect_rename = 1;
DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
@@ -352,7 +353,7 @@ void wt_status_print(struct wt_status *s)
else if (s->commitable)
fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
- if (s->verbose && !s->is_initial)
+ if (s->verbose)
wt_status_print_verbose(s);
if (!s->commitable) {
if (s->amend)
--
1.6.0.4.883.g4593ee.dirty
^ permalink raw reply related
* [PATCH 2/5] wt-status: refactor initial commit printing
From: Jeff King @ 2008-11-12 8:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20081112081609.GA3720@coredump.intra.peff.net>
When we showed the initial commit, we had no reference to
diff against, so we went through the cache manually.
Nowadays, however, we have a virtual empty tree commit, so
we can simply diff against that to get the same results.
Signed-off-by: Jeff King <peff@peff.net>
---
I ran across this when I realized I could do the same trick for the
verbose diff (which I will do in the next patch). I think it is a
worthwhile cleanup. Not only does it remove a lot of lines, but it gives
a single codepath for printing, which will be helpful if that codepath
ever changes (e.g., with the alternate status formats we talked about a
few weeks ago).
This could be made even simpler by setting s->reference to the empty
tree when we are on the initial commit, but I think that is a little
messy. The caller may have set s->reference to an arbitrary pointer,
including one which needs free()d, and we would be overwriting that.
As it happens, with the current callers there is no problem, but it
seems like a bad interface.
wt-status.c | 28 +++-------------------------
1 files changed, 3 insertions(+), 25 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 6a7645e..c78588e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -185,31 +185,12 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,
wt_status_print_trailer(s);
}
-static void wt_status_print_initial(struct wt_status *s)
-{
- int i;
- struct strbuf buf = STRBUF_INIT;
-
- if (active_nr) {
- s->commitable = 1;
- wt_status_print_cached_header(s);
- }
- for (i = 0; i < active_nr; i++) {
- color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
- color_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), "new file: %s",
- quote_path(active_cache[i]->name, -1,
- &buf, s->prefix));
- }
- if (active_nr)
- wt_status_print_trailer(s);
- strbuf_release(&buf);
-}
-
static void wt_status_print_updated(struct wt_status *s)
{
struct rev_info rev;
init_revisions(&rev, NULL);
- setup_revisions(0, NULL, &rev, s->reference);
+ setup_revisions(0, NULL, &rev,
+ s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_print_updated_cb;
rev.diffopt.format_callback_data = s;
@@ -360,12 +341,9 @@ void wt_status_print(struct wt_status *s)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "# Initial commit");
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
- wt_status_print_initial(s);
- }
- else {
- wt_status_print_updated(s);
}
+ wt_status_print_updated(s);
wt_status_print_changed(s);
if (wt_status_submodule_summary)
wt_status_print_submodule_summary(s);
--
1.6.0.4.883.g4593ee.dirty
^ permalink raw reply related
* [PATCH 1/5] define empty tree sha1 as a macro
From: Jeff King @ 2008-11-12 8:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, git
In-Reply-To: <20081112081609.GA3720@coredump.intra.peff.net>
This can potentially be used in a few places, so let's make
it available to all parts of the code.
Signed-off-by: Jeff King <peff@peff.net>
---
I use the _HEX version in further patches, but since it is such a magic
number, I thought it made sense to keep all definitions in the same
place.
cache.h | 6 ++++++
sha1_file.c | 4 +---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index bcc57ba..46eb2af 100644
--- a/cache.h
+++ b/cache.h
@@ -528,6 +528,12 @@ static inline void hashclr(unsigned char *hash)
}
extern int is_empty_blob_sha1(const unsigned char *sha1);
+#define EMPTY_TREE_SHA1_HEX \
+ "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
+#define EMPTY_TREE_SHA1_BIN \
+ "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
+ "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
+
int git_mkstemp(char *path, size_t n, const char *template);
/*
diff --git a/sha1_file.c b/sha1_file.c
index 654d039..037e439 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2056,9 +2056,7 @@ static struct cached_object {
static int cached_object_nr, cached_object_alloc;
static struct cached_object empty_tree = {
- /* empty tree sha1: 4b825dc642cb6eb9a060e54bf8d69288fbee4904 */
- "\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60"
- "\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04",
+ EMPTY_TREE_SHA1_BIN,
OBJ_TREE,
"",
0
--
1.6.0.4.883.g4593ee.dirty
^ permalink raw reply related
* Re: git commit -v does not removes the patch
From: Jeff King @ 2008-11-12 8:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Santi Béjar, Git Mailing List
In-Reply-To: <7v4p2e2nkg.fsf@gitster.siamese.dyndns.org>
On Tue, Nov 11, 2008 at 09:13:18AM -0800, Junio C Hamano wrote:
> We may want to change this. We can say "# Everything under this line is
> deleted." at the beginning of the "#" block we produce in the commit log
> message editor, replacing the "Lines starting with '#' will be ignored, "
> we currently have. When reading back the editor result, make "git commit
> -v" scan for the "# Everything ..." line. We remove it and everything
> that follows, but we do not touch anything above that line (including the
> ones that begin with "diff" or "#") except the usual trailing whitespace
> removal. That way, people can leave a sample shell session with root
> prompt, and sample diff, in their message.
>
> If we do not see "# Everything ..." when we read it back, we can do what
> we currently do as a fallback.
I am little hesitant to do this, because I think people have scripted
minorly around the template format (at the very least, for syntax
highlighting). I think some people may have pre-written templates, as
well, though I guess your "If we do not see..." paragraph is meant to
address that. Though that brings some confusion itself, because now the
parsing rules for what is kept change if I delete that line.
Here's a patch series that at least improves the situation by turning
off the diff-stripping if we never put in a diff in the first place.
That way only people who actually _use_ "-v" will have to pay for it.
It has the fix I sent to Santi earlier, as well as some related
cleanups.
1/5: define empty tree sha1 as a macro
2/5: wt-status: refactor initial commit printing
3/5: status: show "-v" diff even for initial commit
4/5: commit: loosen pattern for matching "-v" diff
5/5: commit: only strip diff from message in verbose mode
-Peff
^ permalink raw reply
* Re: [PATCH (GITK)] gitk: Fix transient windows on Win32 and MacOS.
From: Paul Mackerras @ 2008-11-12 8:14 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Gavrilov, git
In-Reply-To: <491A827C.3010000@viscovery.net>
Johannes Sixt writes:
> Alexander Gavrilov schrieb:
> > Transient windows cause problems on these platforms:
> ...
> > diff --git a/gitk b/gitk
> > index 9b2a6e5..e6aafe8 100755
>
> I'd appreciate if you could make it a habit to base your patches on
> versions of gitk etc. that are available from a public repository
You mean, like, git://git.kernel.org/pub/scm/gitk/gitk.git, for
instance? :) That is the primary repository for gitk and it seems to
be what Alexander bases his patches on.
Paul.
^ permalink raw reply
* Re: Install issues
From: Andreas Ericsson @ 2008-11-12 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, H.Merijn Brand, git
In-Reply-To: <7vhc6e17fv.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
>> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>>> --- Makefile.org 2008-11-10 17:29:53.000000000 +0100
>>> +++ Makefile 2008-11-10 17:29:39.000000000 +0100
>>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>> ./test-sha1.sh
>>>
>>> check: common-cmds.h
>>> + @`sparse </dev/null 2>/dev/null` || (\
>>> + echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>>> + echo "Did you mean 'make test' instead?" ;\
>>> + exit 1 )
>>> for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>> Please read Documentation/SubmittingPatches, your patch lacks a signoff
>> and a commit message.
>
> Heh, for something small and obvious like this, that's asking a tad too
> much, although a properly formatted message does reduce my workload and is
> appreciated.
>
> I said "obvious" not in the sense that it is "obviously good". It is
> obvious what issue the patch wants to address.
>
> Having said that, it is far from clear if special casing "make check" like
> this is a good thing, though. The crufts resulting from "Four extra lines
> won't hurt" kind of reasoning can accumulate and snowball. Is reading the
> Makefile when your build fails in order to see if the target was what you
> really wanted to invoke (ideally, it should rater be "_before_ running
> make, reading the Makefile to find out what you want to run") a lost art
> these days?
>
Why not "make help" with as friendly a message as we can muster, like the
linux kernel does it?
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH v2 1/3] t7700: demonstrate mishandling of objects in packs with a .keep file
From: Jeff King @ 2008-11-12 8:09 UTC (permalink / raw)
To: Brandon Casey
Cc: Junio C Hamano, Shawn O. Pearce, Git Mailing List, Nicolas Pitre
In-Reply-To: <muOuA1nLBoljLnZoguxeFeKt-8Q-I9Y3ljvxnLWLt9KyA8HwVtMa4Q@cipher.nrlssc.navy.mil>
On Mon, Nov 03, 2008 at 02:37:05PM -0600, Brandon Casey wrote:
> This version replaces the use of 'head -n -1' with a grep, and should work on
> all platforms.
Hmm. I'm not sure what happened, but the version in 'next' has "head -n
-1" in it.
-Peff
^ permalink raw reply
* Re: Stgit and refresh-temp
From: Karl Hasselström @ 2008-11-12 8:01 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Jon Smirl, Git Mailing List
In-Reply-To: <b0943d9e0811110959t4eb236bvd648fbca5e482911@mail.gmail.com>
On 2008-11-11 17:59:02 +0000, Catalin Marinas wrote:
> 2008/11/7 Karl Hasselström <kha@treskal.com>:
>
> > On 2008-11-04 08:37:24 -0500, Jon Smirl wrote:
> >
> > > I hit a case when refreshing a buried patch that needed a merge
> > > conflict sorted out. I'm unable to recover out of the state.
> >
> > Hmm, so what you're saying is basically that you did something
> > with "stg refresh -p" that caused a merge conflict, and that
> > messed things up so that you needed to run "stg repair". Is that
> > right?
>
> Could be related to this - if I run 'stg goto some-patch' and it
> fails with a conflict, the HEAD points to the previous patch though
> the stack has the conflicting patch as empty (which is normal) and
> the conflicts in the index. Anything after that says HEAD and top
> not equal and 'stg repair' is needed.
Ah, yes, that could definitely be the same problem, since those two
things end up calling the same functions to handle the conflict.
I'll build a test case for that as well.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH (GITK)] gitk: Fix transient windows on Win32 and MacOS.
From: Johannes Sixt @ 2008-11-12 7:15 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git, Paul Mackerras
In-Reply-To: <200811112355.43352.angavrilov@gmail.com>
Alexander Gavrilov schrieb:
> Transient windows cause problems on these platforms:
...
> diff --git a/gitk b/gitk
> index 9b2a6e5..e6aafe8 100755
I'd appreciate if you could make it a habit to base your patches on
versions of gitk etc. that are available from a public repository so that
the index information is useful:
Applying: gitk: Fix transient windows on Win32 and MacOS.
warning: gitk-git/gitk has type 100644, expected 100755
error: patch failed: gitk-git/gitk:1754
error: gitk-git/gitk: patch does not apply
fatal: sha1 information is lacking or useless (gitk-git/gitk).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001.
I'm stopping here... :-(
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/4] git send-email: interpret unknown files as revision lists
From: Junio C Hamano @ 2008-11-12 5:48 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <1226361242-2516-3-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> +test_expect_success 'detects ambiguous reference/file conflict' '
> + echo master > master &&
> + git add master &&
> + git commit -m"add master" &&
> + test_must_fail git send-email --dry-run master > errors &&
> + grep disambiguate errors
> +'
I've queued the series in 'pu', but with a fix to this test, without which
it did not pass (and I had to rewind and rebuild 'pu').
^ permalink raw reply
* Re: [PATCH 3/4] git-remote rename: support branches->config migration
From: Junio C Hamano @ 2008-11-12 4:22 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <20081112020158.GK24201@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Nov 11, 2008 at 04:49:14PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>> There is something fishy going on between 2/4 and 3/4. 2/4 was advertised
>> to migrate remotes to config and had a call to migrate_file() for that
>> purpose. Here this one now allows to convert branches but there is no
>> change to the callsite of migrate_file().
>>
>> Which would mean that 2/4 would convert branches/foo too. And this one is
>> only to remove the leftover branches/foo file.
>>
>> Or am I utterly confused?
>
> The trick is that 2/4 already added support for remotes/foo as it uses
> remote_get() and that detects remotes/foo as well, but that is
> completely unintentional.
That is not a trick; it merely is a broken code.
The function migrate_file() introduced by [2/4] is called for any remote
definition that did not come from config (by definition, it either came
from remotes/foo or branches/foo). The function adds the entries for the
given remote definition to the config file, and then removes remotes/foo
file if the remote definition came from it. So it is a logically
consistent change if you only called this function only for remote
definitions that came from remotes/foo.
But the function is called for a remote definition that originally came
from branches/foo as well. It happily adds the definition to the config,
even though it *fails to remove* branches/foo file.
Do you still think 2/4 is a logically contained good change?
If you apply this to 5505 (taken from 3/4, but removing the check for
branches/origin file), and look at resulting t/trash*/six/.git/config
file, you will see you have already migrated the remote definition to the
config.
diff --git i/t/t5505-remote.sh w/t/t5505-remote.sh
index 1567631..60bb9e5 100755
--- i/t/t5505-remote.sh
+++ w/t/t5505-remote.sh
@@ -364,4 +364,15 @@ test_expect_success 'migrate a remote from named file in $GIT_DIR/remotes' '
test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
'
-test_done
+test_expect_success 'migrate a remote from named file in $GIT_DIR/branches' '
+ git clone one six &&
+ origin_url=$(pwd)/one &&
+ (cd six &&
+ git remote rm origin &&
+ echo "$origin_url" > .git/branches/origin &&
+ git remote rename origin origin &&
+ test "$(git config remote.origin.url)" = "$origin_url" &&
+ test "$(git config remote.origin.fetch)" = "refs/heads/master:refs/heads/origin")
+'
+
+: test_done
^ permalink raw reply related
* Re: Newbie questions regarding jgit
From: Imran M Yousuf @ 2008-11-12 2:24 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jonas Fonseca, Farrukh Najmi, git
In-Reply-To: <20081111231106.GT2932@spearce.org>
On Wed, Nov 12, 2008 at 5:11 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Jonas Fonseca <jonas.fonseca@gmail.com> wrote:
>>
>> I don't think admin rights are necessary as long as I have
>> "commit"/webdav access. And no svn or git-svn interaction should be
>> needed to upload to the maven repository.
>>
>> Take a look at the distributionManagement section of the
>> google-maven-repository:
>>
>> - http://google-maven-repository.googlecode.com/svn/repository/com/google/google/1/google-1.pom
>>
>> Looks pretty easy to set up. About maintaining it, I don't mind doing
>> "mvn deploy" once in a while, but some kind of update policy should
>> probably be worked out in any case.
>
> Then have at it. It sounds like it would be worthwhile setting up.
>
This idea is cool!
> --
> Shawn.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Blog: http://imyousuf-tech.blogs.smartitengineering.com/
Mobile: +880-1711402557
^ permalink raw reply
* Re: [PATCH 3/4] git-remote rename: support branches->config migration
From: Miklos Vajna @ 2008-11-12 2:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <7v63mtvkdx.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 826 bytes --]
On Tue, Nov 11, 2008 at 04:49:14PM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> There is something fishy going on between 2/4 and 3/4. 2/4 was advertised
> to migrate remotes to config and had a call to migrate_file() for that
> purpose. Here this one now allows to convert branches but there is no
> change to the callsite of migrate_file().
>
> Which would mean that 2/4 would convert branches/foo too. And this one is
> only to remove the leftover branches/foo file.
>
> Or am I utterly confused?
The trick is that 2/4 already added support for remotes/foo as it uses
remote_get() and that detects remotes/foo as well, but that is
completely unintentional. If you think it makes sense, just squash 3/4
to 2/4, I made it two separate patches because I think these are
logically separate changes.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Add maven distribution management info for the new snapshot repository
From: Shawn O. Pearce @ 2008-11-12 1:01 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Jonas Fonseca, Farrukh Najmi, git
In-Reply-To: <20081112005838.GU2932@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Jonas Fonseca <fonseca@diku.dk> wrote:
> > + <distributionManagement>
> > + <snapshotRepository>
> > + <id>jgit-maven-snapshot-repository</id>
> > + <name>JGit Maven Snapshot Repository</name>
> > + <url>dav:https://egit.googlecode.com/svn/maven/snapshot-repository/</url>
>
> Shouldn't that be http:// and not https:// ?
Oh, wait, never mind me.
I guess this makes sense; https:// for the distribution upload,
but the wiki page you added uses plain http:// for download.
Patch applied.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Add maven distribution management info for the new snapshot repository
From: Shawn O. Pearce @ 2008-11-12 0:58 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: Jonas Fonseca, Farrukh Najmi, git
In-Reply-To: <20081111234858.GA17662@diku.dk>
Jonas Fonseca <fonseca@diku.dk> wrote:
> Shawn O. Pearce <spearce@spearce.org> wrote Tue, Nov 11, 2008:
> > Jonas Fonseca <jonas.fonseca@gmail.com> wrote:
> > > I don't think admin rights are necessary as long as I have
> > > "commit"/webdav access. And no svn or git-svn interaction should be
> > > needed to upload to the maven repository.
> >
> > Then have at it. It sounds like it would be worthwhile setting up.
>
> Instructions at http://code.google.com/p/egit/wiki/ConfiguringMaven
>
> diff --git a/jgit-maven/jgit/pom.xml b/jgit-maven/jgit/pom.xml
> index a123470..c370783 100644
> --- a/jgit-maven/jgit/pom.xml
> +++ b/jgit-maven/jgit/pom.xml
> @@ -185,4 +185,12 @@ ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> <scope>compile</scope>
> </dependency>
> </dependencies>
> + <distributionManagement>
> + <snapshotRepository>
> + <id>jgit-maven-snapshot-repository</id>
> + <name>JGit Maven Snapshot Repository</name>
> + <url>dav:https://egit.googlecode.com/svn/maven/snapshot-repository/</url>
Shouldn't that be http:// and not https:// ?
The https interface asked me for a username/password, the
http:// one doesn't. We want this to be public, don't we?
--
Shawn.
^ permalink raw reply
* Re: [PATCH] git-diff: Add --staged as a synonym for --cached.
From: Junio C Hamano @ 2008-11-12 0:57 UTC (permalink / raw)
To: Jeff King
Cc: David Symonds, Björn Steinbrink, Johannes Schindelin, git,
Stephan Beyer
In-Reply-To: <20081111012210.GA26920@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> To me, what is really being asked with "git diff --staged" (or "git
> diff --cached" for that matter), is "what is staged?"
Ok, "what is staged (in index)?", relative to the named commit which
defaults to HEAD, is a good argument. Let's apply it.
^ permalink raw reply
* Re: [PATCH v2] git push: Interpret $GIT_DIR/branches in a Cogito compatible way
From: Junio C Hamano @ 2008-11-12 0:52 UTC (permalink / raw)
To: Martin Koegler; +Cc: git
In-Reply-To: <1226440260-26943-1-git-send-email-mkoegler@auto.tuwien.ac.at>
Thanks, will start cooking in 'next'.
^ permalink raw reply
* Re: [PATCH] Fix non-literal format in printf-style calls
From: Junio C Hamano @ 2008-11-12 0:51 UTC (permalink / raw)
To: Daniel Lowe; +Cc: git
In-Reply-To: <1226351272-22253-1-git-send-email-dlowe@bitmuse.com>
Thanks, will apply to 'maint' and 'master' after splitting.
^ permalink raw reply
* Re: [PATCH] git-submodule: Avoid printing a spurious message.
From: Junio C Hamano @ 2008-11-12 0:51 UTC (permalink / raw)
To: Alexandre Julliard; +Cc: git
In-Reply-To: <87myg67ywz.fsf@wine.dyndns.org>
Good eyes, thanks. Will apply to 'maint'.
^ permalink raw reply
* Re: [PATCH] git ls-remote: make usage string match manpage
From: Junio C Hamano @ 2008-11-12 0:50 UTC (permalink / raw)
To: Stefan Naewe; +Cc: git
In-Reply-To: <1226418751-18747-1-git-send-email-stefan.naewe@atlas-elektronik.com>
Thanks, will apply.
^ permalink raw reply
* Re: [PATCH 2/2] Cached the git configuration, which is now noticibly faster on windows.
From: Junio C Hamano @ 2008-11-12 0:50 UTC (permalink / raw)
To: Simon Hausmann; +Cc: Han-Wen Nienhuys, John Chapman, git
In-Reply-To: <200811101046.01543.simon@lst.de>
Simon Hausmann <simon@lst.de> writes:
> On Sunday 09 November 2008 Junio C Hamano, wrote:
>> These are patches to fast-import/git-p4, which you two seem to in charge
>> of.
>>
>> From: John Chapman <thestar@fussycoder.id.au>
>> Subject: [PATCH 1/2] Added support for purged files and also optimised
>> memory usage. Date: Sat, 8 Nov 2008 14:22:48 +1100
>> Message-Id: <1226114569-8506-1-git-send-email-thestar@fussycoder.id.au>
>>
>> From: John Chapman <thestar@fussycoder.id.au>
>> Subject: [PATCH 2/2] Cached the git configuration, which is now
>> noticibly faster on windows. Date: Sat, 8 Nov 2008 14:22:49 +1100
>> Message-Id: <1226114569-8506-2-git-send-email-thestar@fussycoder.id.au>
>>
>> It was unfortunately not immediately obvious from the Subject: line what
>> these patches are about, and I am guessing you missed them because of that.
>
> Ack on both patches. The second one could be done better, as suggested
> in the follow-ups, but both are clearly an improvement :)
Thanks, both of you. Will apply.
^ permalink raw reply
* Re: [PATCH 3/4] git-remote rename: support branches->config migration
From: Junio C Hamano @ 2008-11-12 0:49 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <b32cf68df41e417079a49dc02e46ffc0c571029b.1226349595.git.vmiklos@frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> This is similar to the remotes->config one, but it makes the
> branches->config conversion possible.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
> builtin-remote.c | 2 ++
> t/t5505-remote.sh | 12 ++++++++++++
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-remote.c b/builtin-remote.c
> index d9d0ba3..3af1876 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -384,6 +384,8 @@ static int migrate_file(struct remote *remote)
> remote->fetch_refspec[i], buf.buf);
> if (remote->origin == REMOTE_REMOTES)
> path = git_path("remotes/%s", remote->name);
> + else if (remote->origin == REMOTE_BRANCHES)
> + path = git_path("branches/%s", remote->name);
There is something fishy going on between 2/4 and 3/4. 2/4 was advertised
to migrate remotes to config and had a call to migrate_file() for that
purpose. Here this one now allows to convert branches but there is no
change to the callsite of migrate_file().
Which would mean that 2/4 would convert branches/foo too. And this one is
only to remove the leftover branches/foo file.
Or am I utterly confused?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox