Git development
 help / color / mirror / Atom feed
* Re: [PATCH JGIT] Circular references shouldn't be created
From: Avery Pennarun @ 2009-09-17 21:40 UTC (permalink / raw)
  To: Sohn, Matthias; +Cc: Shawn O. Pearce, Robin Rosenberg, git@vger.kernel.org
In-Reply-To: <C89280B882467443A695734861B942B28759DB95@DEWDFECCR09.wdf.sap.corp>

On Thu, Sep 17, 2009 at 3:23 PM, Sohn, Matthias <matthias.sohn@sap.com> wrote:
>        void link(final String name, final String target) throws IOException {
> +               if (name.equals(target))
> +                       throw new IllegalArgumentException(
> +                                       "illegal circular reference : symref " + name
> +                                                       + " cannot refer to " + target);

This isn't a very thorough fix.  It doesn't catch longer loops, like

    HEAD -> chicken -> HEAD

or

   a -> b -> c -> d -> a

Experimenting with original git.git's implementation, I see that this
is allowed:

   git symbolic-ref refs/heads/boink refs/heads/boink

It succeeds and creates a file that looks like this:

   ref: refs/heads/boink

And "git show-ref refs/heads/boink" says: nothing (but returns an error code).

And "git log refs/heads/boink" says:

   warning: ignoring dangling symref refs/heads/boink.
   fatal: ambiguous argument 'refs/heads/boink': unknown revision or
path not in the working tree.
   Use '--' to separate paths from revisions

Clearly, in git.git, symref loops are caught at ref read time, not
write time.  This makes sense, since someone might foolishly twiddle
the repository by hand and you don't want to get into an infinite loop
in that case.  Also, it's potentially useful to allow people to set
invalid symrefs *temporarily*, as part of a multi step process.

Have fun,

Avery

^ permalink raw reply

* [PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.
From: Thiago Farina @ 2009-09-17 21:51 UTC (permalink / raw)
  To: git; +Cc: Thiago Farina

Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
 builtin-pack-objects.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 7a390e1..4494a68 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -22,15 +22,15 @@
 #include <pthread.h>
 #endif
 
-static const char pack_usage[] = "\
-git pack-objects [{ -q | --progress | --all-progress }] \n\
-	[--max-pack-size=N] [--local] [--incremental] \n\
-	[--window=N] [--window-memory=N] [--depth=N] \n\
-	[--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n\
-	[--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n\
-	[--stdout | base-name] [--include-tag] \n\
-	[--keep-unreachable | --unpack-unreachable] \n\
-	[<ref-list | <object-list]";
+static const char pack_usage[] =
+  "git pack-objects [{ -q | --progress | --all-progress }] \n"
+  "        [--max-pack-size=N] [--local] [--incremental] \n"
+  "        [--window=N] [--window-memory=N] [--depth=N] \n"
+  "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n"
+  "        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n"
+  "        [--stdout | base-name] [--include-tag] \n"
+  "        [--keep-unreachable | --unpack-unreachable] \n"
+  "        [<ref-list | <object-list]";
 
 struct object_entry {
 	struct pack_idx_entry idx;
-- 
1.6.5.rc0.dirty

^ permalink raw reply related

* Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage string.
From: Junio C Hamano @ 2009-09-17 22:00 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <1253224300-18017-1-git-send-email-tfransosi@gmail.com>

Thiago Farina <tfransosi@gmail.com> writes:

> +static const char pack_usage[] =
> +  "git pack-objects [{ -q | --progress | --all-progress }] \n"
> +  "        [--max-pack-size=N] [--local] [--incremental] \n"
> +  "        [--window=N] [--window-memory=N] [--depth=N] \n"
> +  "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n"
> +  "        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n"
> +  "        [--stdout | base-name] [--include-tag] \n"
> +  "        [--keep-unreachable | --unpack-unreachable] \n"
> +  "        [<ref-list | <object-list]";

Do you still want to keep the trailing whitespace on these lines?

^ permalink raw reply

* Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage  string.
From: Thiago Farina @ 2009-09-17 22:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvdjhgrjv.fsf@alter.siamese.dyndns.org>

On Thu, Sep 17, 2009 at 7:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Thiago Farina <tfransosi@gmail.com> writes:
>
>> +static const char pack_usage[] =
>> +  "git pack-objects [{ -q | --progress | --all-progress }] \n"
>> +  "        [--max-pack-size=N] [--local] [--incremental] \n"
>> +  "        [--window=N] [--window-memory=N] [--depth=N] \n"
>> +  "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n"
>> +  "        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n"
>> +  "        [--stdout | base-name] [--include-tag] \n"
>> +  "        [--keep-unreachable | --unpack-unreachable] \n"
>> +  "        [<ref-list | <object-list]";
>
> Do you still want to keep the trailing whitespace on these lines?
I did this to maintain the same output of the old string, but if you
want I can change, what you suggest?

^ permalink raw reply

* [PATCH] git-log --format: Add %B tag with %B(x) option
From: Johannes Gilger @ 2009-09-17 22:47 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Johannes Gilger

Since one can simply use spaces to indent any other --pretty field we
should have an option to do that with the body too.

Also the %B flag strips the trailing newlines, to enable more compact
display.

Signed-off-by: Johannes Gilger <heipei@hackvalue.de>
---
Hey list,

in my never-ending quest to beautify my personal log output I just whipped this 
up. Maybe you like it too or at least can tell me what's wrong with it ;)

Please CC me as I'm not on the list anymore (but keep up through Gmane though).

 Documentation/pretty-formats.txt |    2 ++
 pretty.c                         |   13 +++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 2a845b1..c04f118 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -123,6 +123,8 @@ The placeholders are:
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
 - '%b': body
+- '%B': body without trailing newline
+- '%B(x)': body indented with x spaces
 - '%Cred': switch color to red
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
diff --git a/pretty.c b/pretty.c
index f5983f8..6d530e1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -735,6 +735,19 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 'b':	/* body */
 		strbuf_addstr(sb, msg + c->body_off);
 		return 1;
+	case 'B':
+		if (placeholder[1] == '(') {
+			const char *body = msg + c->body_off;
+			const char *end = strchr(placeholder + 2, ')');
+			if(!end)
+				return 0;
+			pp_remainder(CMIT_FMT_MEDIUM, &body, sb, atoi(placeholder + 2));
+			strbuf_rtrim(sb);
+			return end - placeholder + 1;
+		}
+		strbuf_addstr(sb, msg + c->body_off);
+		strbuf_rtrim(sb);
+		return 1;
 	}
 	return 0;	/* unknown placeholder */
 }
-- 
1.6.5.rc1.20.geb7d9

^ permalink raw reply related

* Re: [PATCH JGIT] Circular references shouldn't be created
From: Robin Rosenberg @ 2009-09-17 22:51 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Sohn, Matthias, Shawn O. Pearce, git@vger.kernel.org
In-Reply-To: <32541b130909171440w1a6d2394t4acc6a2f791c143@mail.gmail.com>

torsdag 17 september 2009 23:40:12 skrev Avery Pennarun <apenwarr@gmail.com>:
> On Thu, Sep 17, 2009 at 3:23 PM, Sohn, Matthias <matthias.sohn@sap.com> wrote:
> >        void link(final String name, final String target) throws IOException {
> > +               if (name.equals(target))
> > +                       throw new IllegalArgumentException(
> > +                                       "illegal circular reference : symref " + name
> > +                                                       + " cannot refer to " + target);
> 
> This isn't a very thorough fix.  It doesn't catch longer loops, like
> 
>     HEAD -> chicken -> HEAD
> 
> or
> 
>    a -> b -> c -> d -> a
> 
> Experimenting with original git.git's implementation, I see that this
> is allowed:
> 
>    git symbolic-ref refs/heads/boink refs/heads/boink
> 
> It succeeds and creates a file that looks like this:
> 
>    ref: refs/heads/boink
> 
> And "git show-ref refs/heads/boink" says: nothing (but returns an error code).
> 
> And "git log refs/heads/boink" says:
> 
>    warning: ignoring dangling symref refs/heads/boink.
>    fatal: ambiguous argument 'refs/heads/boink': unknown revision or
> path not in the working tree.
>    Use '--' to separate paths from revisions
> 
> Clearly, in git.git, symref loops are caught at ref read time, not
> write time.  This makes sense, since someone might foolishly twiddle
> the repository by hand and you don't want to get into an infinite loop
> in that case.  Also, it's potentially useful to allow people to set
> invalid symrefs *temporarily*, as part of a multi step process.

I had already written a patch much like this when I decided we need to do much better.

I think we should do this in the UI by not allowing the user to make a
choice that would result in a loop and fixing the way the UI resolves
choices. When creating a new branch we should analyze the selected 
ref and dereference it if it is a symbolic name like HEAD or if it is a tag, 
and perhaps show it like "HEAD (refs/heads/master)" in the the dialog.

Using unresolvable refs as the base for a new branch should be disallowed.

-- robin

^ permalink raw reply

* Re: [PATCH] git-log --format: Add %B tag with %B(x) option
From: Junio C Hamano @ 2009-09-17 23:27 UTC (permalink / raw)
  To: Johannes Gilger; +Cc: Git Mailing List
In-Reply-To: <1253227671-20493-1-git-send-email-heipei@hackvalue.de>

Johannes Gilger <heipei@hackvalue.de> writes:

> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index 2a845b1..c04f118 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -123,6 +123,8 @@ The placeholders are:
>  - '%s': subject
>  - '%f': sanitized subject line, suitable for a filename
>  - '%b': body
> +- '%B': body without trailing newline
> +- '%B(x)': body indented with x spaces

First the design issues.

Because this will set a precedent for possible future formatting features
that take optional parameters, we need to pick the syntax carefully not
only for this feature but for the later ones that we haven't invented yet.

Let's say that pair of parentheses is a good choice and if later features
want to take more than one, it would be reasonable for them to use a
comma-separated list, e.g. %Q(1,2,3).

I wonder if it is reasonable to invoke print_wrapped_text(), not just
limit this feature to indenting.

Now, let's look at the implementation.

> diff --git a/pretty.c b/pretty.c
> index f5983f8..6d530e1 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -735,6 +735,19 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
>  	case 'b':	/* body */
>  		strbuf_addstr(sb, msg + c->body_off);
>  		return 1;
> +	case 'B':
> +		if (placeholder[1] == '(') {
> +			const char *body = msg + c->body_off;
> +			const char *end = strchr(placeholder + 2, ')');
> +			if(!end)
> +				return 0;

Style: "if (!end)"

I'd sleep better if the syntax checking is done as a separate phase way
before this in the codepath.

> +			pp_remainder(CMIT_FMT_MEDIUM, &body, sb, atoi(placeholder + 2));

What happens when atoi() fails, or %B(12Q) was given?

We tend to use strto[u]l when parsing integers and check for errors.

^ permalink raw reply

* Re: [PATCH] Avoid the use of backslash-at-eol in pack-objects usage  string.
From: Junio C Hamano @ 2009-09-18  0:54 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <a4c8a6d00909171506l6c4b6a49i22d7b337a0c6cfa2@mail.gmail.com>

Thiago Farina <tfransosi@gmail.com> writes:

> On Thu, Sep 17, 2009 at 7:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Thiago Farina <tfransosi@gmail.com> writes:
>>
>>> +static const char pack_usage[] =
>>> +  "git pack-objects [{ -q | --progress | --all-progress }] \n"
>>> +  "        [--max-pack-size=N] [--local] [--incremental] \n"
>>> +  "        [--window=N] [--window-memory=N] [--depth=N] \n"
>>> +  "        [--no-reuse-delta] [--no-reuse-object] [--delta-base-offset] \n"
>>> +  "        [--threads=N] [--non-empty] [--revs [--unpacked | --all]*] [--reflog] \n"
>>> +  "        [--stdout | base-name] [--include-tag] \n"
>>> +  "        [--keep-unreachable | --unpack-unreachable] \n"
>>> +  "        [<ref-list | <object-list]";
>>
>> Do you still want to keep the trailing whitespace on these lines?
> I did this to maintain the same output of the old string, but if you
> want I can change, what you suggest?

If you need to add or remove an option to actually _change_ the string, a
patch like this, as a preparatory step before the real improvement, would
be a very welcome clean-up.  I however would suggest doing nothing, if
this is the only patch you are going to send against this program in the
near future, to be honest.

Even though we do not have any other patch in flight that changes this
program at this moment (as expected, because we are in -rc freeze), which
means there is not much risk for this patch to cause needless conflicts
with others, we generally avoid code churn like this one, as a principle
for a maturing project.

The _very best_ thing you can do for the project on this particular issue
is to keep an eye on the list and the next time somebody wants to patch
this program in a way that affects the usage string, remind that person to
first clean-up the string without changing anything else as a preparation
patch; I however admit that I am asking a lot more work out of you.

A real improvement patch from that somebody _could_ be to remove the
trailing whitespaces from the output string, and in that case I would not
mind if two patches (one preparatory patch which is this one, and the
other being the removal of trailing whitespaces) were squashed together.
In fact, in such a trivial case, it probably be better to squash them into
one.

And that somebody _could_ be you.

^ permalink raw reply

* Add compiled date to git --version output?
From: Nazri Ramliy @ 2009-09-18  4:17 UTC (permalink / raw)
  To: git

Sometimes I wanted to know how outdated git is on my system.

Coming up with a script to parse "git --version" output to get the SHA1,
and compare that to master's SHA1 seemed a little overkill compared to
this:

diff --git a/help.c b/help.c
index 294337e..bc83491 100644
--- a/help.c
+++ b/help.c
@@ -361,6 +361,9 @@ const char *help_unknown_cmd(const char *cmd)

 int cmd_version(int argc, const char **argv, const char *prefix)
 {
-       printf("git version %s\n", git_version_string);
+       printf("git version %s compiled %s %s\n",
+                       git_version_string,
+                       __DATE__,
+                       __TIME__);
        return 0;
 }

With this, git --version gives:

        git version 1.6.5.rc1.19.g8426.dirty compiled Sep 18 2009 12:03:29

Thoughts?

Nazri

^ permalink raw reply related

* Re: Add compiled date to git --version output?
From: Brian Gernhardt @ 2009-09-18  4:43 UTC (permalink / raw)
  To: Nazri Ramliy; +Cc: git
In-Reply-To: <544dda350909172117r44761577m11e7d30a1a5d0c91@mail.gmail.com>


On Sep 18, 2009, at 12:17 AM, Nazri Ramliy wrote:

> +       printf("git version %s compiled %s %s\n",
> +                       git_version_string,
> +                       __DATE__,
> +                       __TIME__);

> Thoughts?

Any idea how compatible that is?  Sure, it'll work with GCC's cpp, but  
we try to work with a huge variety of compilers.

At the very least you might want to wrap it all in #ifdefs or something:

#if defined(__DATE__) && defined(__TIME__)
/* your version */
#else
/* old version */
#endif

(Or play fun tricks with #ifndef ... #define, but then you also have  
to play with the format string or something.)

~~Brian

^ permalink raw reply

* Re: Add compiled date to git --version output?
From: David Aguilar @ 2009-09-18  5:03 UTC (permalink / raw)
  To: Nazri Ramliy; +Cc: git
In-Reply-To: <544dda350909172117r44761577m11e7d30a1a5d0c91@mail.gmail.com>

On Fri, Sep 18, 2009 at 12:17:48PM +0800, Nazri Ramliy wrote:
> Sometimes I wanted to know how outdated git is on my system.

	$ ls -la $(which git)


> Coming up with a script to parse "git --version" output to get the SHA1,
> and compare that to master's SHA1 seemed a little overkill compared to
> this:
> 
> diff --git a/help.c b/help.c
> index 294337e..bc83491 100644
> --- a/help.c
> +++ b/help.c
> @@ -361,6 +361,9 @@ const char *help_unknown_cmd(const char *cmd)
> 
>  int cmd_version(int argc, const char **argv, const char *prefix)
>  {
> -       printf("git version %s\n", git_version_string);
> +       printf("git version %s compiled %s %s\n",
> +                       git_version_string,
> +                       __DATE__,
> +                       __TIME__);
>         return 0;
>  }
> 
> With this, git --version gives:
> 
>         git version 1.6.5.rc1.19.g8426.dirty compiled Sep 18 2009 12:03:29
> 
> Thoughts?

For whatever it's worth, I would feel more comfortable if this
were guarded behind an option e.g. 'git version --date'.

I suspect that there are a fair number of scripts out there
parsing the output of 'git version'.  'git version' is not
plumbing but we still might want to avoid breaking them.

Is it better to say "compiled on $date" or "compiled $date"?

It's meant to be informational (aka not an actual English
sentence) so I guess it could go either way; "compiled on"
is a little more proper, though.

What about "born on $date" since it gives users a subliminal
suggestion that they should consider upgrading to a fresh git?
;)


-- 

	David

^ permalink raw reply

* Re: Add compiled date to git --version output?
From: Junio C Hamano @ 2009-09-18  5:07 UTC (permalink / raw)
  To: Nazri Ramliy; +Cc: git
In-Reply-To: <544dda350909172117r44761577m11e7d30a1a5d0c91@mail.gmail.com>

Nazri Ramliy <ayiehere@gmail.com> writes:

> Sometimes I wanted to know how outdated git is on my system.
>
> Coming up with a script to parse "git --version" output to get the SHA1,
> and compare that to master's SHA1 seemed a little overkill compared to
> this:
>
> diff --git a/help.c b/help.c
> index 294337e..bc83491 100644
> --- a/help.c
> +++ b/help.c
> @@ -361,6 +361,9 @@ const char *help_unknown_cmd(const char *cmd)
>
>  int cmd_version(int argc, const char **argv, const char *prefix)
>  {
> -       printf("git version %s\n", git_version_string);
> +       printf("git version %s compiled %s %s\n",
> +                       git_version_string,
> +                       __DATE__,
> +                       __TIME__);
>         return 0;
>  }
>
> With this, git --version gives:
>
>         git version 1.6.5.rc1.19.g8426.dirty compiled Sep 18 2009 12:03:29
>
> Thoughts?

It's open source, so you are welcome to do that to your binary.

Personally, I do not want it.  My build scripts depend on the version
string at the end if the output to omit re-building what is already
installed.

Seriously, the version number is useful to track down the bug, and perhaps
your compiler and library versions might be useful to help diagnose build
related errors, but when would that __DATE__/__TIME__ be useful more than
what "ls -l /usr/bin/git" would give you?

^ permalink raw reply

* Re: Add compiled date to git --version output?
From: Nazri Ramliy @ 2009-09-18  6:04 UTC (permalink / raw)
  To: git
In-Reply-To: <7v8wgchmcc.fsf@alter.siamese.dyndns.org>

Thanks for the input guys, I've come to the conclusion that the extra
effort needed to properly implement this is not worth the benefit of
the outcome.

As some of you mentioned, ls -l `which git` is better suited for the
purpose, and that can be done universally for all files where the
filesystem stores the create timestamp of each.

nazri.

^ permalink raw reply

* RE: [PATCH JGIT] Circular references shouldn't be created
From: Sohn, Matthias @ 2009-09-18  6:37 UTC (permalink / raw)
  To: Robin Rosenberg, Avery Pennarun; +Cc: Shawn O. Pearce, git@vger.kernel.org
In-Reply-To: <200909180051.47794.robin.rosenberg@dewire.com>

Robin Rosenberg <robin.rosenberg@dewire.com> wrote on Freitag, 18. September 2009 00:52
>torsdag 17 september 2009 23:40:12 skrev Avery Pennarun
> <apenwarr@gmail.com>:
> > On Thu, Sep 17, 2009 at 3:23 PM, Sohn, Matthias
> <matthias.sohn@sap.com> wrote:
> > >        void link(final String name, final String target) throws
> IOException {
> > > +               if (name.equals(target))
> > > +                       throw new IllegalArgumentException(
> > > +                                       "illegal circular reference
> : symref " + name
> > > +                                                       + " cannot
> refer to " + target);
> >
> > This isn't a very thorough fix.  It doesn't catch longer loops, like
> >
> >     HEAD -> chicken -> HEAD
> >
> > or
> >
> >    a -> b -> c -> d -> a
> >
> > Experimenting with original git.git's implementation, I see that this
> > is allowed:
> >
> >    git symbolic-ref refs/heads/boink refs/heads/boink
> >
> > It succeeds and creates a file that looks like this:
> >
> >    ref: refs/heads/boink
> >
> > And "git show-ref refs/heads/boink" says: nothing (but returns an
> error code).
> >
> > And "git log refs/heads/boink" says:
> >
> >    warning: ignoring dangling symref refs/heads/boink.
> >    fatal: ambiguous argument 'refs/heads/boink': unknown revision or
> > path not in the working tree.
> >    Use '--' to separate paths from revisions
> >
> > Clearly, in git.git, symref loops are caught at ref read time, not
> > write time.  This makes sense, since someone might foolishly twiddle
> > the repository by hand and you don't want to get into an infinite loop
> > in that case.  Also, it's potentially useful to allow people to set
> > invalid symrefs *temporarily*, as part of a multi step process.

Looks like I was a bit short-sighted yesterday, I will try to cook a better
solution.

> 
> I had already written a patch much like this when I decided we need to
> do much better.
> 
> I think we should do this in the UI by not allowing the user to make a
> choice that would result in a loop and fixing the way the UI resolves
> choices. When creating a new branch we should analyze the selected
> ref and dereference it if it is a symbolic name like HEAD or if it is a
> tag,
> and perhaps show it like "HEAD (refs/heads/master)" in the the dialog.
> 
> Using unresolvable refs as the base for a new branch should be
> disallowed.
> 

If we would do it in the EGit UI how about catching such cases 
in other applications using JGit ?

--
Matthias

^ permalink raw reply

* Re: [RFC 15/15] Tag GIT_VERSION when Git is built with MSVC
From: Marius Storm-Olsen @ 2009-09-18  6:44 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: git, Johannes.Schindelin, msysgit, gitster, lznuaa, raa.lkml,
	snaury
In-Reply-To: <200909172218.15678.j6t@kdbg.org>

Johannes Sixt said the following on 17.09.2009 22:18:
> On Mittwoch, 16. September 2009, Marius Storm-Olsen wrote:
>> This may help us debug issues on Windows, as we now can build Git
>> natively on Windows with both MinGW and MSVC.
>>
>> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
>> ---
>>  I'm just throwing this one out there. If people think manipulating
>>  the version here, to ease debugging, I don't mind if this patch is
>>  squashed into patch 12.
>>  If people don't like it, just skip this path.
>>
>>  Makefile |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index aa918eb..2c20922 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -878,6 +878,7 @@ ifneq (,$(findstring CYGWIN,$(uname_S)))
>>  	UNRELIABLE_FSTAT = UnfortunatelyYes
>>  endif
>>  ifdef MSVC
>> +	GIT_VERSION := $(GIT_VERSION).MSVC
>>  	pathsep = ;
>>  	NO_PREAD = YesPlease
>>  	NO_OPENSSL = YesPlease
> 
> I like it, but I would not squash it into patch 12.

Ok good, we'll just keep it as is then.
Thanks.

--
.marius

^ permalink raw reply

* Re: [PATCH 14/15] Add scripts to generate projects for other buildsystems (MSVC vcproj, QMake)
From: Marius Storm-Olsen @ 2009-09-18  6:59 UTC (permalink / raw)
  To: Johannes Sixt, gitster
  Cc: git, Johannes.Schindelin, msysgit, lznuaa, raa.lkml, snaury
In-Reply-To: <200909172228.28174.j6t@kdbg.org>

Johannes Sixt said the following on 17.09.2009 22:28:
> On Mittwoch, 16. September 2009, Marius Storm-Olsen wrote:
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -179,3 +179,14 @@ configure
>>  tags
>>  TAGS
>>  cscope*
>> +*.obj
>> +*.lib
>> +*.sln
>> +*.suo
>> +*.ncb
>> +*.vcproj
>> +*.user
>> +*.idb
>> +*.pdb
>> +Debug/
>> +Release/
> 
> If I understand correctly, then 'make MSVC=1' still produce *.o files, not 
> *.obj. But if the VC++ project is used, I expect that the *.obj, *.idb, and 
> *.pdb end up in Debug/ or Release/ directories. Then why do you need entries 
> for *.obj, *.idb, and *.pdb?

When using only the vcproj generator, you are correct. However, if you 
use the qmake generator, and create vcprojs from those, the *.idb and 
*.pdb files are located in the project directory itself, and not under 
Debug/. I'm not too worried about this case though, so for me, the 
three entries *.obj, *.idb and *.pdb can go.

Junio, you want me to push a new patch?

--
.marius

^ permalink raw reply

* Re: git workflow for fully distributed mini-teams
From: Rustom Mody @ 2009-09-18  7:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <4AB24C06.5030207@viscovery.net>

On Thu, Sep 17, 2009 at 8:17 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> [On this list, we reply to all, so that the Cc list remains]
>
> Rustom Mody schrieb:
>> I started looking at git bundle and find things like master\~10.
>> Whats the backslash doing?
>
> It's intended as markup for the pipeline that generates the documentation
> from git-bundle.txt. Either the markup is incorrect, or there is a bug in
> the pipeline, because I only see it in the generated HTML. Ignore it.

Seems to be there in my git-bundle.txt as well as git-bundle.html.
So its probably the markup not the pipeline.

^ permalink raw reply

* [PATCH] git-push: add option --repo-all
From: Kirill A. Korinskiy @ 2009-09-18  7:17 UTC (permalink / raw)
  To: gitster; +Cc: git, Kirill A. Korinskiy

Example of usage: I write some software on my laptop and some time
pushing to my home/private server for backup. Some time ago my
software is done and I openin it on github, but I'm don't like kill my
private repos. Now update a two remotes repo is'n sexy, because I'm
need using a some shell wrapper:

    git remote show | while read repo; do git push $repo; done
---
 Documentation/git-push.txt |    4 ++-
 builtin-push.c             |   34 +++++++++++++++++++++-----------
 t/t5523-push-repo-all.sh   |   46 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 13 deletions(-)
 create mode 100755 t/t5523-push-repo-all.sh

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index ba6a8a2..734e745 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,6 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all | --mirror | --tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
-	   [--repo=<repository>] [-f | --force] [-v | --verbose]
 	   [<repository> <refspec>...]
 
 DESCRIPTION
@@ -134,6 +133,9 @@ useful if you write an alias or script around 'git-push'.
 	transfer spends extra cycles to minimize the number of
 	objects to be sent and meant to be used on slower connection.
 
+--repo-all::
+	Send changes to all remote repos.
+
 -v::
 --verbose::
 	Run verbosely.
diff --git a/builtin-push.c b/builtin-push.c
index 3cb1ee4..2b25293 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [-n | --dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror] [-n | --dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository> | --repo-all] [-f | --force] [-v] [<repository> <refspec>...]",
 	NULL,
 };
 
@@ -88,19 +88,13 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(struct remote *remote, void *priv)
 {
+	int flags = *((int *)priv);
 	int i, errs;
-	struct remote *remote = remote_get(repo);
 	const char **url;
 	int url_nr;
 
-	if (!remote) {
-		if (repo)
-			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
-	}
-
 	if (remote->mirror)
 		flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
 
@@ -171,13 +165,16 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
 	int tags = 0;
+	int repo_all = 0;
 	int rc;
+	struct remote *remote;
 	const char *repo = NULL;	/* default repository */
 
 	struct option options[] = {
 		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
+		OPT_BOOLEAN( 0 , "repo-all", &repo_all, "push to all remote repos"),
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
@@ -197,11 +194,24 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		add_refspec("refs/tags/*");
 
 	if (argc > 0) {
-		repo = argv[0];
-		set_refspecs(argv + 1, argc - 1);
+		if (repo_all) {
+			set_refspecs(argv, argc);
+		} else {
+			repo = argv[0];
+			set_refspecs(argv + 1, argc - 1);
+		}
 	}
 
-	rc = do_push(repo, flags);
+	remote = remote_get(repo);
+	if (!remote && !repo_all) {
+		if (repo)
+			die("bad repository '%s'", repo);
+		die("No destination configured to push to.");
+	}
+
+	rc = repo_all ?
+		for_each_remote(do_push, &flags) : do_push(remote, &flags);
+
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/t/t5523-push-repo-all.sh b/t/t5523-push-repo-all.sh
new file mode 100755
index 0000000..865b8a1
--- /dev/null
+++ b/t/t5523-push-repo-all.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='pushing to all remote repos repository'
+
+. ./test-lib.sh
+
+mk_repos () {
+	rm -rf maste mirror-1 mirror-2 &&
+	mkdir mirror-1 &&
+	(
+		cd mirror-1 &&
+		git init
+	) &&
+	mkdir mirror-2 &&
+	(
+		cd mirror-2 &&
+		git init
+	) &&
+	mkdir master &&
+	(
+		cd master &&
+		git init &&
+		git remote add mirror-1 ../mirror-1
+		git remote add mirror-2 ../mirror-2
+	)
+}
+
+
+test_expect_success 'push to mirrors' '
+
+	mk_repos &&
+	(
+		cd master &&
+		echo one >foo && git add foo && git commit -m one &&
+		git remote show &&
+		git push --all --repo-all -f
+	) &&
+	master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+	mirror_1_master=$(cd mirror-1 && git show-ref -s --verify refs/heads/master) &&
+	mirror_2_master=$(cd mirror-2 && git show-ref -s --verify refs/heads/master) &&
+	test "$master_master" = "$mirror_1_master" &&
+	test "$master_master" = "$mirror_2_master"
+
+'
+
+test_done
-- 
1.6.2

^ permalink raw reply related

* Re: [PATCH 14/15] Add scripts to generate projects for other buildsystems (MSVC vcproj, QMake)
From: Johannes Sixt @ 2009-09-18  8:21 UTC (permalink / raw)
  To: Marius Storm-Olsen, gitster
  Cc: Johannes Sixt, git, Johannes.Schindelin, msysgit, lznuaa,
	raa.lkml, snaury
In-Reply-To: <4AB32FE2.1060604@gmail.com>

Marius Storm-Olsen schrieb:
> Johannes Sixt said the following on 17.09.2009 22:28:
>> why do
>> you need entries for *.obj, *.idb, and *.pdb?
> 
> When using only the vcproj generator, you are correct. However, if you
> use the qmake generator, and create vcprojs from those, the *.idb and
> *.pdb files are located in the project directory itself, and not under
> Debug/. I'm not too worried about this case though, so for me, the three
> entries *.obj, *.idb and *.pdb can go.

Fair enough.

> Junio, you want me to push a new patch?

That's not necessary. I just wanted to make sure that you added the
entries deliberately.

With these questions answered, and the most recent change to 04/15 that
adjust test-genrandom.c, the MinGW aspect of this series is

Acked-by: Johannes Sixt <j6t@kdbg.org>

Thank you very much for your work and persistency!

-- Hannes

^ permalink raw reply

* Re: [PATCH] git-push: add option --repo-all
From: Jakub Narebski @ 2009-09-18  8:52 UTC (permalink / raw)
  To: Kirill A. Korinskiy; +Cc: gitster, git
In-Reply-To: <1253258222-11475-1-git-send-email-catap@catap.ru>

"Kirill A. Korinskiy" <catap@catap.ru> writes:

> Example of usage: I write some software on my laptop and some time
> pushing to my home/private server for backup. Some time ago my
> software is done and I openin it on github, but I'm don't like kill my
> private repos. Now update a two remotes repo is'n sexy, because I'm
> need using a some shell wrapper:
> 
>     git remote show | while read repo; do git push $repo; done

Signoff?

> ---
>  Documentation/git-push.txt |    4 ++-
>  builtin-push.c             |   34 +++++++++++++++++++++-----------
>  t/t5523-push-repo-all.sh   |   46 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 71 insertions(+), 13 deletions(-)
>  create mode 100755 t/t5523-push-repo-all.sh

I have created 'pushall' *alias* for that purpose, but I think that
such functionality would be better added to "git remote" rather than
to "git push".

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] git-push: add option --repo-all
From: Kirill A. Korinskiy @ 2009-09-18 10:44 UTC (permalink / raw)
  To: gitster; +Cc: git, Kirill A. Korinskiy
In-Reply-To: <m3r5u43a8h.fsf@localhost.localdomain>

Example of usage: I write some software on my laptop and some time
pushing to my home/private server for backup. Some time ago my
software is done and I openin it on github, but I'm don't like kill my
private repos. Now update a two remotes repo is'n sexy, because I'm
need using a some shell wrapper:

    git remote show | while read repo; do git push $repo; done

Signed-off-by: Kirill A. Korinskiy <catap@catap.ru>
---
 Documentation/git-push.txt |    3 ++
 builtin-push.c             |   34 +++++++++++++++++++++-----------
 t/t5523-push-repo-all.sh   |   46 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100755 t/t5523-push-repo-all.sh

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index ba6a8a2..92e45c2 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -134,6 +134,9 @@ useful if you write an alias or script around 'git-push'.
 	transfer spends extra cycles to minimize the number of
 	objects to be sent and meant to be used on slower connection.
 
+--repo-all::
+	Send changes to all remote repos.
+
 -v::
 --verbose::
 	Run verbosely.
diff --git a/builtin-push.c b/builtin-push.c
index 3cb1ee4..2b25293 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git push [--all | --mirror] [-n | --dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository>] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git push [--all | --mirror] [-n | --dry-run] [--porcelain] [--tags] [--receive-pack=<git-receive-pack>] [--repo=<repository> | --repo-all] [-f | --force] [-v] [<repository> <refspec>...]",
 	NULL,
 };
 
@@ -88,19 +88,13 @@ static void setup_default_push_refspecs(void)
 	}
 }
 
-static int do_push(const char *repo, int flags)
+static int do_push(struct remote *remote, void *priv)
 {
+	int flags = *((int *)priv);
 	int i, errs;
-	struct remote *remote = remote_get(repo);
 	const char **url;
 	int url_nr;
 
-	if (!remote) {
-		if (repo)
-			die("bad repository '%s'", repo);
-		die("No destination configured to push to.");
-	}
-
 	if (remote->mirror)
 		flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
 
@@ -171,13 +165,16 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
 	int tags = 0;
+	int repo_all = 0;
 	int rc;
+	struct remote *remote;
 	const char *repo = NULL;	/* default repository */
 
 	struct option options[] = {
 		OPT_BIT('q', "quiet", &flags, "be quiet", TRANSPORT_PUSH_QUIET),
 		OPT_BIT('v', "verbose", &flags, "be verbose", TRANSPORT_PUSH_VERBOSE),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
+		OPT_BOOLEAN( 0 , "repo-all", &repo_all, "push to all remote repos"),
 		OPT_BIT( 0 , "all", &flags, "push all refs", TRANSPORT_PUSH_ALL),
 		OPT_BIT( 0 , "mirror", &flags, "mirror all refs",
 			    (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE)),
@@ -197,11 +194,24 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		add_refspec("refs/tags/*");
 
 	if (argc > 0) {
-		repo = argv[0];
-		set_refspecs(argv + 1, argc - 1);
+		if (repo_all) {
+			set_refspecs(argv, argc);
+		} else {
+			repo = argv[0];
+			set_refspecs(argv + 1, argc - 1);
+		}
 	}
 
-	rc = do_push(repo, flags);
+	remote = remote_get(repo);
+	if (!remote && !repo_all) {
+		if (repo)
+			die("bad repository '%s'", repo);
+		die("No destination configured to push to.");
+	}
+
+	rc = repo_all ?
+		for_each_remote(do_push, &flags) : do_push(remote, &flags);
+
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
diff --git a/t/t5523-push-repo-all.sh b/t/t5523-push-repo-all.sh
new file mode 100755
index 0000000..865b8a1
--- /dev/null
+++ b/t/t5523-push-repo-all.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='pushing to all remote repos repository'
+
+. ./test-lib.sh
+
+mk_repos () {
+	rm -rf maste mirror-1 mirror-2 &&
+	mkdir mirror-1 &&
+	(
+		cd mirror-1 &&
+		git init
+	) &&
+	mkdir mirror-2 &&
+	(
+		cd mirror-2 &&
+		git init
+	) &&
+	mkdir master &&
+	(
+		cd master &&
+		git init &&
+		git remote add mirror-1 ../mirror-1
+		git remote add mirror-2 ../mirror-2
+	)
+}
+
+
+test_expect_success 'push to mirrors' '
+
+	mk_repos &&
+	(
+		cd master &&
+		echo one >foo && git add foo && git commit -m one &&
+		git remote show &&
+		git push --all --repo-all -f
+	) &&
+	master_master=$(cd master && git show-ref -s --verify refs/heads/master) &&
+	mirror_1_master=$(cd mirror-1 && git show-ref -s --verify refs/heads/master) &&
+	mirror_2_master=$(cd mirror-2 && git show-ref -s --verify refs/heads/master) &&
+	test "$master_master" = "$mirror_1_master" &&
+	test "$master_master" = "$mirror_2_master"
+
+'
+
+test_done
-- 
1.6.2

^ permalink raw reply related

* Re: [PATCH] git-push: add option --repo-all
From: Jakub Narebski @ 2009-09-18 11:02 UTC (permalink / raw)
  To: Kirill A. Korinskiy; +Cc: git
In-Reply-To: <877hvwzkw7.wl%catap@catap.ru>

Kirill A. Korinskiy wrote:
> At Fri, 18 Sep 2009 01:52:49 -0700 (PDT),
> Jakub Narebski <jnareb@gmail.com> wrote:
> > "Kirill A. Korinskiy" <catap@catap.ru> writes:

> > > ---
> > >  Documentation/git-push.txt |    4 ++-
> > >  builtin-push.c             |   34 +++++++++++++++++++++-----------
> > >  t/t5523-push-repo-all.sh   |   46 ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 71 insertions(+), 13 deletions(-)
> > >  create mode 100755 t/t5523-push-repo-all.sh
> > 
> > I have created 'pushall' *alias* for that purpose, but I think that
> > such functionality would be better added to "git remote" rather than
> > to "git push".
> 
> not sure, because git remote make interface for managment remotes
> repos and push make interface for pushing to remote repo. I just add a
> pushing to all repos.
> 
> I thought about pushing to some remotes repos, yes, but could not come
> up with a good symantics.

Well, git-remote has "git remote update" subcommand for fetching from
a group of remote repositories, so it is not only about managing remotes.
I think "git remote push" (or something like that) would fit in 
git-remote area of competence.

Besides git-remote understands groups of remote repositories for fetch
(update), which would be (I think) a good idea also for push.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH] git-push: add option --repo-all
From: Paolo Bonzini @ 2009-09-18 11:15 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Kirill A. Korinskiy, git
In-Reply-To: <200909181302.49335.jnareb@gmail.com>


> Well, git-remote has "git remote update" subcommand for fetching from
> a group of remote repositories, so it is not only about managing remotes.
> I think "git remote push" (or something like that) would fit in
> git-remote area of competence.
>
> Besides git-remote understands groups of remote repositories for fetch
> (update), which would be (I think) a good idea also for push.

Agreed.

Paolo

^ permalink raw reply

* Re: [PATCH 2/2] gitk: Fix the geometry when restoring from zoomed state
From: Pat Thoyts @ 2009-09-18 13:24 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Alexy Borzenkov
In-Reply-To: <19122.10359.725107.949551@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

>Pat Thoyts writes:
>
>> The patch to handle the geometry of a restored gitk by Alexy Borzenkov
>> causes the position of the columns to creep each time the application
>> is restarted.  This patch addresses this by remembering the application
>> geometry for the normal state and saving that regardless of the actual
>> state when the application is closed.
>
>So this patch replaces Alexey's patch, then?  The context in your patch
>doesn't match the changes made in Alexey's patch AFAICS.

Correct. I posted a response to Alexy's patch saying I'd post an
alternative as I had trouble with the columns resizing and creeping
due to the assertion of 'wm state normal' during the settings save
each time.
See
 http://thread.gmane.org/gmane.comp.version-control.git/128026/focus=128652
The context of mine should be the gitk repository master (was
c21398be).

-- 
Pat Thoyts                            http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD

^ permalink raw reply

* Re: Git crashes on pull
From: Tay Ray Chuan @ 2009-09-18 13:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Guido Ostkamp, git, Michael Wookey
In-Reply-To: <7vzl8v4y5g.fsf@alter.siamese.dyndns.org>

Hi,

On Wed, Sep 16, 2009 at 6:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks.
>
> The sad part of the story was that this regression was introduced by a
> change to work around recent breakage observed when fetching from the http
> server github runs, and it was the primary purpose of pushing 1.6.4.3 out.
>
> Now we need to cut a 1.6.4.4 with this fix-on-fix soon, like tomorrow.

sorry for all the trouble caused.

Junio, do you think moving out the free() would be a better option? Setting it to NULL just so we can free() is rather contrived, I feel.

-- >8 --

Subject: [PATCH] http.c: move free() out of cleanup block

Instead of initializing a variable (url) just so we can do a free() on
it, as in b202514 (http.c: avoid freeing an uninitialized pointer), we
move the free() out of cleanup block.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 http.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/http.c b/http.c
index 23b2a19..a67f62e 100644
--- a/http.c
+++ b/http.c
@@ -866,7 +866,7 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)
 	int ret = 0;
 	char *hex = xstrdup(sha1_to_hex(sha1));
 	char *filename;
-	char *url = NULL;
+	char *url;
 	struct strbuf buf = STRBUF_INIT;

 	if (has_pack_index(sha1)) {
@@ -885,9 +885,9 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)
 	if (http_get_file(url, filename, 0) != HTTP_OK)
 		ret = error("Unable to get pack index %s\n", url);

+	free(url);
 cleanup:
 	free(hex);
-	free(url);
 	return ret;
 }

--
1.6.4.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox