Git development
 help / color / mirror / Atom feed
* Re: Deciding between Git/Mercurial
From: Robin Rosenberg @ 2009-09-27 18:01 UTC (permalink / raw)
  To: newsgroups; +Cc: git
In-Reply-To: <h9nlhj$heq$1@ger.gmane.org>

söndag 27 september 2009 14:24:32 skrev Anteru <newsgroups@catchall.shelter13.net>:
> Hi,
> 
> I'm currently evaluating DVCS for a project, and we're at a point where
> it comes down to either Mercurial or Git. Right now, I'm advocating for
> Git, while my co-workers like Mercurial, so I'd like to provide some
> good arguments in favor of git. Unfortunately, I'm not a git expert, so
> I hope I can get some help here ...

You have to read carefully. This (or the mercurial list) may not be the
most objective sources of information.

> First of all, what's the matter with git and Windows, is there some
> long-term commitment to make git work on Windows as well as on Linux?

Besides msysgit there is JGit and a port of it to C# (and  thus any dotnet-ish 
language). The msysgit teams seems very committed and passionate about
the project, but they need more assistance from genuine Windows users. Note
that the current model of file locking can never work as well on Windows
as it does on Unix. Something better is needed for flawless operation.

> I'm using msysgit on Windows, and personally I'm happy with it, but my
> co-workers constantly nag that Mercurial has superior portability ...

Might be somewhat true, but msysgit works very well. Not sure how
mercurial handles unicode issues. CRLF issues seems to be ignored (not handled).

> Mercurial's revision number system: With git, I get an SHA1 hash for
> every commit, but it's not possible to see whether Hash1 is newer than
> Hash2, while Mecurial also adds a running number to each commit. What's

But those numbers cannot be communicated since they are local to your
clone.

> the rationale behind this decision for git, and is it possible to
> emulate Mercurial's behavior somehow?

git-cvsserver has to do something along those line  The numbering is
per file.

Maintainers tend to tag versions using the common numbered schem
and that is typically enough.

-- robin

^ permalink raw reply

* gitweb atom feeds broken (on repo.or.cz only?)
From: Sebastian Pipping @ 2009-09-27 17:35 UTC (permalink / raw)
  To: git, Petr Baudis; +Cc: Robert Buchholz

hello git list, hello petr!


i noticed that the atom feeds generated by repo.or.cz's gitweb (e.g.
[1]) show no content in firefox 3.5.2.  this seems to be due to invalid
xml in it as shown by running [1] through feedvalidator, results at [2].

their rss neighbors seemed not to be affected where i looked.
a fix would be cool still, as the atom feeds contain more detailed content.

sorry for the noise in case repo.or.cz runs customized atom generation.



sebastian


[1] http://repo.or.cz/w/dottout.git?a=atom
[2]
http://feedvalidator.org/check.cgi?url=http%3A%2F%2Frepo.or.cz%2Fw%2Fdottout.git%3Fa%3Datom

^ permalink raw reply

* New script: build-git
From: Øyvind A. Holm @ 2009-09-27 13:33 UTC (permalink / raw)
  To: git

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

It’s important to have the bleeding-edge Git installed on all computers 
I use, so I created a bash script to automatically download, install and 
test new versions from the master branch. It’s been maturing now for 
some weeks, and works fine on all computers I use, which is Ubuntu and 
Debian based. It’s built with paranoia in mind to avoid accidents, and 
places each version under its separate directory structure under 
/usr/local by default, but that’s easily configurable.

In case someone else has use for such a script, it’s available from

  git://github.com/sunny256/build-git.git
  http://github.com/sunny256/build-git.git

or

  git://gitorious.org/build-git/build-git.git
  http://git.gitorious.org/build-git/build-git.git

Comments and patches/enhancements welcome.

Regards,
Øyvind

+-| Øyvind A. Holm <sunny@sunbase.org> - N 60.39548° E 5.31735° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
+------------| 47ab564a-ab68-11de-8e10-00248cd5cf1e |-------------+

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Deciding between Git/Mercurial
From: Anteru @ 2009-09-27 12:24 UTC (permalink / raw)
  To: git

Hi,

I'm currently evaluating DVCS for a project, and we're at a point where
it comes down to either Mercurial or Git. Right now, I'm advocating for
Git, while my co-workers like Mercurial, so I'd like to provide some
good arguments in favor of git. Unfortunately, I'm not a git expert, so
I hope I can get some help here ...

First of all, what's the matter with git and Windows, is there some
long-term commitment to make git work on Windows as well as on Linux?
I'm using msysgit on Windows, and personally I'm happy with it, but my
co-workers constantly nag that Mercurial has superior portability ...

Mercurial's revision number system: With git, I get an SHA1 hash for
every commit, but it's not possible to see whether Hash1 is newer than
Hash2, while Mecurial also adds a running number to each commit. What's
the rationale behind this decision for git, and is it possible to
emulate Mercurial's behavior somehow?

Integration into tools: We're using Trac currently, which also has a
nice binding to Mercurial (well, obviously easy to do as Mercurial is
written in Python, just as Trac itself), while the git support is in
development and looks quite alpha'ish. Do you plan to make it easier to
integrate git with other tools by providing bindings to other languages,
or is this a low-priority issue?

So far, my key arguments are that git is more robust (more projects
using it, larger developer base), of course git's excellent performance
and the much better support for SVN, which is important for us as we can
slowly migrate from SVN->Git, while hgmercurial is still in the making
(and Python's SVN->Hg switch is for instance waiting for it).

Cheers,
  Anteru

^ permalink raw reply

* Re: git log --pretty=format:%h prints (unrequired) abbreviated sha
From: alexandrul @ 2009-09-27 10:40 UTC (permalink / raw)
  To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550909261455g4eef8432jbb90048417deecba@mail.gmail.com>

Marco Costalba wrote:
> Hi Alexandrul,
> 
> Now it works !!!! :-)  :-)
> 
> It was a virus !  I have run Malwarebytes Anti-malware 1.41 and it
> found some virus (I send you the logs in attachment), after removing
> them the problem disappeared.
> 
> I have Kaspersky as antivirus, but, although a very good antivirus it
> failed to identify them.
> 
> I didn't know this Malwarebytes, but it seems very powerful.
> 
> Thanks anyhow for your exceptional help, I have really appreciated that !!!!
> 
> 
> Thanks again
> Marco
> 

You are welcome.

Unfortunately, Kaspersky and Malwarebytes target different threats, but 
both of them are very good at what they do.

Have a nice day,
   A.

^ permalink raw reply

* Re: Interim maintainer tree
From: Jeff King @ 2009-09-27  8:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20090925160504.GW14660@spearce.org>

On Fri, Sep 25, 2009 at 09:05:04AM -0700, Shawn O. Pearce wrote:

> Junio is on vaction for the next week.  In his absence Peff and I
> are trying to keep up with current patches in my fork:
> 
>   git://repo.or.cz/git/spearce.git
>   http://repo.or.cz/r/git/spearce.git

I've collected a few more topics in my repo:

  git://github.com/peff/git.git

Rather than cook my own, slightly different 'next', I'm just publishing
the tips of the topics themselves. Which means they are subject to
rewinding until you merge them into 'next', at which point I will leave
them stable.

What's in there now is:

  as/parseopt-ambiguous
    Fix for "git branch --no-merge" with my test added. Should be ready
    for next.

  jk/reflog-date
    My reflog fix. Should be ready for next.

  mr/gitweb-snapshot
    v5, replacing what Junio had in pu. Should probably stay in pu until
    we hear from Jakub.

  mr/instaweb
    Looked OK to me, but I have no capacity to actually test it.
    Unlikely to break anything else, though, so probably ready to cook
    in next.

  mv/lock-error
    My tweaked version, which got the OK from Miklos. Ready for next, I
    think.

  np/clone-smaller
    Nico's fix to stop copying unnecessary objects during clone. Patch
    looks sane, I confirmed that it fixes the issue, and it doesn't seem
    to break normal clones. Ready to cook in next, I think.

-Peff

^ permalink raw reply

* Re: [PATCH] gitweb: fix spelling errors in comments
From: Miklos Vajna @ 2009-09-27  8:54 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090927083507.GA25891@coredump.intra.peff.net>

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

On Sun, Sep 27, 2009 at 04:35:07AM -0400, Jeff King <peff@peff.net> wrote:
> > The plural form of "id" is "ids", not "id's", right?
> 
> It is a matter of some debate, actually. As an abbreviation, it should
> rightly be "ID", and therefore "IDs". Most style manuals indicate that
> no apostrophe should be used these days, unless it is an abbreviation
> separated by dots (e.g., "I.D.'s").
> 
> Some disagree, and some indicate that you should use an apostrophe where
> it may be visually more clear (for example, in single-letter
> abbreviations like "A's").
> 
> There is a nice summary of some style guides here:
> 
>   http://answers.google.com/answers/threadview?id=499296
> 
> Honestly, for such an informal bit of text as a code comment, I'm not
> sure it is worth nit-picking the grammar (e.g., we should be
> writing SHA-1 everywhere, and we obviously don't). I'll let Shawn decide
> whether he wants to apply or not.

As a non-native I did not know to resolve "id's" to "id is" or "id has",
and once I figured it out, I thought about sending a patch.

Though, if that helps, I can resend it with "Blobs defined by
non-textual hash IDs can be cached" to be more readable.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-27  8:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthieu.Moy, spearce, git
In-Reply-To: <20090927082123.GD15393@coredump.intra.peff.net>

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

On Sun, Sep 27, 2009 at 04:21:23AM -0400, Jeff King <peff@peff.net> wrote:
> Maybe it is just me, but that extra die() that should never be reached
> is terribly ugly. I would do it with two functions, one that dies and
> one that doesn't, with a helper to format the message. IOW, this:

Okay, that's fine with me - thanks for providing a patch as well.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] gitweb: fix spelling errors in comments
From: Jeff King @ 2009-09-27  8:35 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <1254007953-1961-1-git-send-email-vmiklos@frugalware.org>

On Sun, Sep 27, 2009 at 01:32:33AM +0200, Miklos Vajna wrote:

> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>  gitweb/gitweb.perl |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> The plural form of "id" is "ids", not "id's", right?

It is a matter of some debate, actually. As an abbreviation, it should
rightly be "ID", and therefore "IDs". Most style manuals indicate that
no apostrophe should be used these days, unless it is an abbreviation
separated by dots (e.g., "I.D.'s").

Some disagree, and some indicate that you should use an apostrophe where
it may be visually more clear (for example, in single-letter
abbreviations like "A's").

There is a nice summary of some style guides here:

  http://answers.google.com/answers/threadview?id=499296

Honestly, for such an informal bit of text as a code comment, I'm not
sure it is worth nit-picking the grammar (e.g., we should be
writing SHA-1 everywhere, and we obviously don't). I'll let Shawn decide
whether he wants to apply or not.

-Peff

^ permalink raw reply

* Re: git stash list shows timestamp in stead of "stash number", when  setting date = local for log in config
From: Alf Kristian Støyle @ 2009-09-27  8:25 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jeff King, git
In-Reply-To: <20090925222920.GZ14660@spearce.org>

Hi guys, thanks for the explanation, and thanks for fixing this. I
wasn't aware that stash just is a reflog.

- Alf


On Sat, Sep 26, 2009 at 00:29, Shawn O. Pearce <spearce@spearce.org> wrote:
> Jeff King <peff@peff.net> wrote:
>> On Tue, Sep 15, 2009 at 04:56:41PM +0200, Alf Kristian St??yle wrote:
>> > When doing a "git stash list" I get this strange stash record:
>> > stash@{Tue Sep 15 16:28:12 2009}: WIP on master: 2262276 ...
>> >
>> > I have a global config setting on log:
>> >
>> > [log]
>> > date = local
>> >
>> > If setting the date config to default or removing the setting, the
>> > stash record looks correct:
>> > stash@{0}: WIP on master: 2262276 ...
>>
>> The patch below implements the former. The only downside I can think of
>> is if somebody is relying on "log.date" to set the output format for
>> reflogs, because they really like the date version better. In that case,
>> I think we should wait for them to complain (which I doubt will happen),
>> and then add a log.reflogDates config option to appease them.
>>
>> Shawn, reflogs are your thing. Any comments?
>
> I agree.  I doubt anyone is relying on log.date to reformat the
> output of `git reflog show` or `git stash list`, so this is probably
> a reasonable change to make.  Even if they were trying to use that,
> its a bug.
>
> Care to wrap this up in a patch?
>
> --
> Shawn.
>

^ permalink raw reply

* Re: [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer
From: Giuseppe Scrivano @ 2009-09-27  8:21 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jeff King, git, Johannes Schindelin, Sverre Rabbelier
In-Reply-To: <alpine.LFD.2.00.0909262038470.4997@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> And the compiler (at least gcc) is indeed smart enough to realize that 
> nothing uses the result from the last statement, and does optimize away 
> the code associated to it already.  So this patch is unlikely to change 
> anything to the compiled result.

Right, and gcc can do many other amazing things.  But still it is used a
variable that is never accessed, removing it can make the code slightly
more readable.

Cheers,
Giuseppe

^ permalink raw reply

* Re: [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Jeff King @ 2009-09-27  8:21 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: Matthieu.Moy, spearce, git
In-Reply-To: <1254006909-1862-1-git-send-email-vmiklos@frugalware.org>

On Sun, Sep 27, 2009 at 01:15:09AM +0200, Miklos Vajna wrote:

> Changes since the previous one:
> 
> * unable_to_lock_index() renamed to unable_to_lock()
> * NORETURN is back for unable_to_lock_index_die()

Much better, but:

> +	if (noreturn)
> +		die(buf.buf);
> +	ret = error(buf.buf);

These need to be:

  die("%s", buf.buf);

as the resulting message (which contains the filename) may have '%' in
it.

> +NORETURN void unable_to_lock_index_die(const char *path, int err)
> +{
> +	unable_to_lock(path, err, 1);
> +	die("unable_to_lock() should have died already");
>  }

Maybe it is just me, but that extra die() that should never be reached
is terribly ugly. I would do it with two functions, one that dies and
one that doesn't, with a helper to format the message. IOW, this:

-- >8 --
From: Miklos Vajna <vmiklos@frugalware.org>
Subject: [PATCH] git branch -D: give a better error message when lockfile creation fails

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 cache.h    |    1 +
 lockfile.c |   26 ++++++++++++++++++++------
 refs.c     |    4 +++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index 1a6412d..a5eeead 100644
--- a/cache.h
+++ b/cache.h
@@ -489,6 +489,7 @@ struct lock_file {
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
+extern int unable_to_lock_error(const char *path, int err);
 extern NORETURN void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
diff --git a/lockfile.c b/lockfile.c
index eb931ed..6851fa5 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -155,18 +155,32 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 	return lk->fd;
 }
 
-
-NORETURN void unable_to_lock_index_die(const char *path, int err)
+static char *unable_to_lock_message(const char *path, int err)
 {
+	struct strbuf buf = STRBUF_INIT;
+
 	if (err == EEXIST) {
-		die("Unable to create '%s.lock': %s.\n\n"
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s.\n\n"
 		    "If no other git process is currently running, this probably means a\n"
 		    "git process crashed in this repository earlier. Make sure no other git\n"
 		    "process is running and remove the file manually to continue.",
 		    path, strerror(err));
-	} else {
-		die("Unable to create '%s.lock': %s", path, strerror(err));
-	}
+	} else
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s", path, strerror(err));
+	return strbuf_detach(&buf, NULL);
+}
+
+int unable_to_lock_error(const char *path, int err)
+{
+	char *msg = unable_to_lock_message(path, err);
+	error("%s", msg);
+	free(msg);
+	return -1;
+}
+
+NORETURN void unable_to_lock_index_die(const char *path, int err)
+{
+	die("%s", unable_to_lock_message(path, err));
 }
 
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
diff --git a/refs.c b/refs.c
index 24865cf..808f56b 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock_error(git_path("packed-refs"), errno);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc2.197.g25cf3

^ permalink raw reply related

* Re: 'git branch --no-merge' is ambiguous
From: Andreas Schwab @ 2009-09-27  8:01 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Sixt, Pierre Habouzit, Git Mailing List
In-Reply-To: <20090927073305.GA15393@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> -- >8 --
> From: Andreas Schwab <schwab@linux-m68k.org>
> Subject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks
>
> parse_long_opt always matches both --opt and --no-opt for any option
> "opt", and only get_value checks whether --no-opt is actually valid.
> Since the options for git branch contains both "no-merged" and "merged"
> there are two matches for --no-merge, but no exact match.  With this
> patch the negation of a NONEG option is rejected earlier, but it changes
> the error message from "option `no-opt' isn't available" to "unknown
> option `no-opt'".
>
> [jk: added test]
>
> Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* [PATCH] improve reflog date/number heuristic
From: Jeff King @ 2009-09-27  7:50 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Alf Kristian St??yle, git
In-Reply-To: <20090925222920.GZ14660@spearce.org>

When we show a reflog, we have two ways of naming the entry:
by sequence number (e.g., HEAD@{0}) or by date (e.g.,
HEAD@{10 minutes ago}). There is no explicit option to set
one or the other, but we guess based on whether or not the
user has provided us with a date format, showing them the
date version if they have done so, and the sequence number
otherwise.

This usually made sense if the use did something like "git
log -g --date=relative". However, it didn't make much sense
if the user set the date format using the log.date config
variable; in that case, all of their reflogs would end up as
dates.

This patch records the source of the date format and only
triggers the date-based view if --date= was given on the
command line.

Signed-off-by: Jeff King <peff@peff.net>
---
On Fri, Sep 25, 2009 at 03:29:20PM -0700, Shawn O. Pearce wrote:

> I agree.  I doubt anyone is relying on log.date to reformat the
> output of `git reflog show` or `git stash list`, so this is probably
> a reasonable change to make.  Even if they were trying to use that,
> its a bug.
> 
> Care to wrap this up in a patch?

Here it is.

 log-tree.c |    4 +++-
 revision.c |    2 ++
 revision.h |    3 ++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 1c9eefe..1618f3c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -390,7 +390,9 @@ void show_log(struct rev_info *opt)
 			 */
 			show_reflog_message(opt->reflog_info,
 				    opt->commit_format == CMIT_FMT_ONELINE,
-				    opt->date_mode);
+				    opt->date_mode_explicit ?
+					opt->date_mode :
+					DATE_NORMAL);
 			if (opt->commit_format == CMIT_FMT_ONELINE)
 				return;
 		}
diff --git a/revision.c b/revision.c
index 35eca4a..9fc4e8d 100644
--- a/revision.c
+++ b/revision.c
@@ -1159,8 +1159,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->simplify_history = 0;
 	} else if (!strcmp(arg, "--relative-date")) {
 		revs->date_mode = DATE_RELATIVE;
+		revs->date_mode_explicit = 1;
 	} else if (!strncmp(arg, "--date=", 7)) {
 		revs->date_mode = parse_date_format(arg + 7);
+		revs->date_mode_explicit = 1;
 	} else if (!strcmp(arg, "--log-size")) {
 		revs->show_log_size = 1;
 	}
diff --git a/revision.h b/revision.h
index 9d0dddb..b6421a6 100644
--- a/revision.h
+++ b/revision.h
@@ -81,7 +81,8 @@ struct rev_info {
 			show_merge:1,
 			abbrev_commit:1,
 			use_terminator:1,
-			missing_newline:1;
+			missing_newline:1,
+			date_mode_explicit:1;
 	enum date_mode date_mode;
 
 	unsigned int	abbrev;
-- 
1.6.5.rc2.197.g25cf3

^ permalink raw reply related

* Re: [PATCH] git-am: force egrep to use correct characters set
From: Jeff King @ 2009-09-27  7:40 UTC (permalink / raw)
  To: Christian Himpel; +Cc: git
In-Reply-To: <f0bd48168975c3b2328cf26f9a37a0f54b898473.1253896646.git.chressie@gmail.com>

On Fri, Sep 25, 2009 at 06:43:20PM +0200, Christian Himpel wrote:

> According to egrep(1) the US-ASCII table is used when LC_ALL=C is set.
> We do not rely here on the LC_ALL value we get from the environment.

Hmm. Probably makes sense here, as it is a wide enough range that it may
pick up other stray non-ascii characters in other charsets (though as
the manpage notes, the likely thing is to pick up A-Z along with a-z,
which is OK here as we encompass both in our range).

There are two other calls to egrep with brackets (both in
git-submodule.sh), but they are just [0-7], which is presumably OK in
just about any charset.

Do you happen to know a charset in which this is a problem, just for
reference?

-Peff

^ permalink raw reply

* Re: 'git branch --no-merge' is ambiguous
From: Jeff King @ 2009-09-27  7:33 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Johannes Sixt, Pierre Habouzit, Git Mailing List
In-Reply-To: <m2ljk250f7.fsf@igel.home>

On Fri, Sep 25, 2009 at 08:44:44PM +0200, Andreas Schwab wrote:

> parse_long_opt always matches both --opt and --no-opt for any option
> "opt", and only get_value checks whether --no-opt is actually valid.
> Since the options for git branch contains both "no-merged" and "merged"
> there are two matches for --no-merge, but no exact match.  With this
> patch the negation of a NONEG option is rejected earlier, but it changes
> the error message from "option `no-opt' isn't available" to "unknown
> option `no-opt'".

Thanks. Reading through the code, I came to the same conclusion: we
shouldn't be looking at --no-* at all if we are NONEG. I think the
change in error message is acceptable.

It is a little bit annoying that builtin-branch needs to have this as
two separate options in the first place. But it wants to be able to do
--no-merged with an argument, which is not currently possible with just
a negation of --merged. I don't know if it is worth adding an
OPT_NEGARG.

Below is what I'm going to commit.  Can I get your Signed-off-by?

-- >8 --
From: Andreas Schwab <schwab@linux-m68k.org>
Subject: [PATCH] parse-opt: ignore negation of OPT_NONEG for ambiguity checks

parse_long_opt always matches both --opt and --no-opt for any option
"opt", and only get_value checks whether --no-opt is actually valid.
Since the options for git branch contains both "no-merged" and "merged"
there are two matches for --no-merge, but no exact match.  With this
patch the negation of a NONEG option is rejected earlier, but it changes
the error message from "option `no-opt' isn't available" to "unknown
option `no-opt'".

[jk: added test]

Signed-off-by: Jeff King <peff@peff.net>
---
 parse-options.c          |    3 +++
 t/t0040-parse-options.sh |   20 ++++++++++++++++++++
 test-parse-options.c     |    5 +++++
 3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index a64a4d6..f559411 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -230,6 +230,9 @@ is_abbreviated:
 				abbrev_flags = flags;
 				continue;
 			}
+			/* negation allowed? */
+			if (options->flags & PARSE_OPT_NONEG)
+				continue;
 			/* negated and abbreviated very much? */
 			if (!prefixcmp("no-", arg)) {
 				flags |= OPT_UNSET;
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index bbc821e..3d450ed 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -33,6 +33,8 @@ Magic arguments
     --quux                means --quux
     -NUM                  set integer to NUM
     +                     same as -b
+    --ambiguous           positive ambiguity
+    --no-ambiguous        negative ambiguity
 
 Standard options
     --abbrev[=<n>]        use <n> digits to display SHA-1s
@@ -315,4 +317,22 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' '
 	test_cmp expect output
 '
 
+cat >expect <<EOF
+boolean: 0
+integer: 0
+timestamp: 0
+string: (not set)
+abbrev: 7
+verbose: 0
+quiet: no
+dry run: no
+file: (not set)
+EOF
+
+test_expect_success 'negation of OPT_NONEG flags is not ambiguous' '
+	test-parse-options --no-ambig >output 2>output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
+
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index efa734b..acd1a2b 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -8,6 +8,7 @@ static int abbrev = 7;
 static int verbose = 0, dry_run = 0, quiet = 0;
 static char *string = NULL;
 static char *file = NULL;
+static int ambiguous;
 
 static int length_callback(const struct option *opt, const char *arg, int unset)
 {
@@ -59,6 +60,10 @@ int main(int argc, const char **argv)
 			number_callback),
 		{ OPTION_BOOLEAN, '+', NULL, &boolean, NULL, "same as -b",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
+		{ OPTION_BOOLEAN, 0, "ambiguous", &ambiguous, NULL,
+		  "positive ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
+		{ OPTION_BOOLEAN, 0, "no-ambiguous", &ambiguous, NULL,
+		  "negative ambiguity", PARSE_OPT_NOARG | PARSE_OPT_NONEG },
 		OPT_GROUP("Standard options"),
 		OPT__ABBREV(&abbrev),
 		OPT__VERBOSE(&verbose),
-- 
1.6.5.rc2.197.g25cf3

^ permalink raw reply related

* Re: git clone sending unneeded objects
From: Jason Merrill @ 2009-09-27  4:35 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Nicolas Pitre, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <20090927020409.GK14660@spearce.org>

On 09/26/2009 10:04 PM, Shawn O. Pearce wrote:
> Actually, if those refs have not changed, quickfetch should kick in
> and realize that all 410610 objects are reachable locally without
> errors, permitting the client to avoid the object transfer.
>
> However, if *ANY* of those refs were to change to something you
> don't actually have, quickfetch would fail, and we would need to
> fetch all 410610 objects.

Right.  That seems unfortunate to me; couldn't fetch do a bit more 
checking before it decides to download the whole world again?

Jason

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Nicolas Pitre @ 2009-09-27  2:31 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <20090927020409.GK14660@spearce.org>

On Sat, 26 Sep 2009, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@fluxnic.net> wrote:
> > And even if the broken clone (before my patch) did pull everything from 
> > gcc.git, in the cloned repository those 410610 extra objects are 
> > considered as garbage because nothing actually reference them.  So even 
> > if you decide to fetch the extra branches that the initial clone didn't 
> > pick up, or if you do reference that repository with "garbage" objects 
> > for another clone to which you want to add those extra branches, git has 
> > no way to know that it already had access to those objects locally and 
> > "ungarbage" them as they aren't referenced.  Result is a useless fetch 
> > of 410610 objects that you already have, but that you weren't supposed 
> > to have in the first place.
> 
> Just to clarify a minor nit:
> 
> Actually, if those refs have not changed, quickfetch should kick in
> and realize that all 410610 objects are reachable locally without
> errors, permitting the client to avoid the object transfer.
> 
> However, if *ANY* of those refs were to change to something you
> don't actually have, quickfetch would fail, and we would need to
> fetch all 410610 objects.

Right.  But since we're talking about a git mirror for the gcc svn repo 
and gcc is a rather active project, the likelyhood of any ref to change 
at any time is rather high.


Nicolas

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Nicolas Pitre @ 2009-09-27  2:26 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABE1818.6010007@redhat.com>

On Sat, 26 Sep 2009, Jason Merrill wrote:

> On 09/26/2009 12:44 AM, Jason Merrill wrote:
> > git config remote.origin.fetch 'refs/remotes/*:refs/remotes/origin/*'
> > git fetch
> 
> git count-objects -v before:
> 
> count: 44
> size: 1768
> in-pack: 1399509
> packs: 1
> size-pack: 600456
> prune-packable: 0
> garbage: 0

I'm sure if you had done 'git rev-list --all --objects | wc -l' at that 
point, the result would have been something around 900000.  That's the 
actual number of objects git had a reference to, compared to the total 
objects contained in the object store.

> and after (transferred 278MB):
> 
> count: 44
> size: 1768
> in-pack: 1947339
> packs: 2
> size-pack: 1178408
> prune-packable: 8
> garbage: 0

And those 500000 extra objects or so (minus a couple dozens which were 
probably used to "complete" the fetched thin pack and are duplicates of 
local objects -- the fetch progress message gave the exact number) were 
obtained from the remote repository because git has no way to tell the 
remote it already had them.  That's what I was explaining in my previous 
email.

> and then after git gc --prune=now:
> 
> count: 0
> size: 0
> in-pack: 1399613
> packs: 1
> size-pack: 839900
> prune-packable: 0
> garbage: 0
> 
> So I only actually needed 104 more objects, but fetch wasn't clever enough to
> see that, and my new pack is much less efficient.

Like I said, it's not that the fetch wasn't clever enough.  Rather that 
your initial clone asked for way too many objects in the first place.  
That's what my patch fixed.

Now the pack efficiency can be explained as well.  A single pack is 
always going to be more efficient than 2 packs.  Problem is when you do 
a gc, by default git does the least costly operation which consists of 
copying as much data from existing packs without extra processing.  
That means that many objects were copied from the second (newly 
received) pack although a better delta representation was most probably 
available in the other larger pack (remember that most objects from that 
second pack already existed in the first pack).  Git do select the 
second pack in preference to the other pack because it is more recent, 
and normally more recent packs contains more recent objects which is a 
good heuristic to optimizes the object enumeration.  In this case this 
didn't produce a good result, but again we're talking about a scenario 
which is bogus from the start and shouldn't be.

So if you do a 'git gc --aggressive' and let it run for a while, you 
should get back a smaller pack, possibly even much smaller than the 
original 
one.


Nicolas

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Shawn O. Pearce @ 2009-09-27  2:04 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <alpine.LFD.2.00.0909262059520.4997@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> wrote:
> And even if the broken clone (before my patch) did pull everything from 
> gcc.git, in the cloned repository those 410610 extra objects are 
> considered as garbage because nothing actually reference them.  So even 
> if you decide to fetch the extra branches that the initial clone didn't 
> pick up, or if you do reference that repository with "garbage" objects 
> for another clone to which you want to add those extra branches, git has 
> no way to know that it already had access to those objects locally and 
> "ungarbage" them as they aren't referenced.  Result is a useless fetch 
> of 410610 objects that you already have, but that you weren't supposed 
> to have in the first place.

Just to clarify a minor nit:

Actually, if those refs have not changed, quickfetch should kick in
and realize that all 410610 objects are reachable locally without
errors, permitting the client to avoid the object transfer.

However, if *ANY* of those refs were to change to something you
don't actually have, quickfetch would fail, and we would need to
fetch all 410610 objects.

-- 
Shawn.

^ permalink raw reply

* Re: git clone sending unneeded objects
From: Nicolas Pitre @ 2009-09-27  1:27 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <4ABD9C2C.60800@redhat.com>

On Sat, 26 Sep 2009, Jason Merrill wrote:

> Incidentally, somewhat related to this issue, I've noticed that if I fetch a
> branch which I don't currently have in my repository, and I have most of the
> commits on that branch in my object store (or in an alternate repository) but
> not the most recent commit, git fetch isn't smart enough to only grab the
> commits I'm actually missing, it wants to fetch much more.
> 
> I would expect that since the clone pulled down everything in the gcc.git
> repository, I could then do
> 
> git config remote.origin.fetch 'refs/remotes/*:refs/remotes/origin/*'
> git fetch
> 
> and have all the branches, not just the ones in refs/heads.  But when I do
> this git fetch wants to fetch some 500k redundant objects.

Well...  Assuming a fixed git using the patch I posted yesterday, my 
clone of gcc.git has 988941 objects.  The source repository used for the 
clone has 1399551 objects.  Of course the source repo has more objects 
because it has extra branches in the refs/remotes/ namespace that the 
clone didn't fetch.  If you wish to also fetch those branches as you 
illustrated above then you'll get the difference i.e. 410610 additional 
objects.

And even if the broken clone (before my patch) did pull everything from 
gcc.git, in the cloned repository those 410610 extra objects are 
considered as garbage because nothing actually reference them.  So even 
if you decide to fetch the extra branches that the initial clone didn't 
pick up, or if you do reference that repository with "garbage" objects 
for another clone to which you want to add those extra branches, git has 
no way to know that it already had access to those objects locally and 
"ungarbage" them as they aren't referenced.  Result is a useless fetch 
of 410610 objects that you already have, but that you weren't supposed 
to have in the first place.


Nicolas

^ permalink raw reply

* Re: [PATCH] Remove various dead assignments and dead increments found by the clang static analyzer
From: Nicolas Pitre @ 2009-09-27  0:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Giuseppe Scrivano, git, Johannes Schindelin, Sverre Rabbelier
In-Reply-To: <20090926204604.GA2960@coredump.intra.peff.net>

On Sat, 26 Sep 2009, Jeff King wrote:

> On Sat, Sep 26, 2009 at 09:15:58PM +0200, Giuseppe Scrivano wrote:
> 
> > Here is a cleaned patch.  I think these assignments can be removed
> > without any problem.
> 
> I don't agree. For example:
> 
> > --- a/builtin-fetch--tool.c
> > +++ b/builtin-fetch--tool.c
> > @@ -169,7 +169,7 @@ static int append_fetch_head(FILE *fp,
> >  			note_len += sprintf(note + note_len, "%s ", kind);
> >  		note_len += sprintf(note + note_len, "'%s' of ", what);
> >  	}
> > -	note_len += sprintf(note + note_len, "%.*s", remote_len, remote);
> > +	sprintf(note + note_len, "%.*s", remote_len, remote);
> 
> This is a very particular C idiom: you are building a string over
> several statements using a function that adds to the string and tells
> you how much it added. The implicit invariant of the note_len variable
> is that it _always_ contains the current length, so each statement uses
> it as input and pushes it forward on output.
> 
> Any experienced C programmer should look at that and be able to see
> exactly what's going on. And people adding more lines don't need to
> munge the existing lines; the invariant property of note_len means they
> just need to add more, similar lines.
> 
> But your patch destroys that invariant. It makes it harder to see what's
> going on, because it breaks the idiom. And it makes it more likely for
> somebody adding a line further on to make a mistake (and certainly it
> makes their patch harder to read and review, as they have to munge
> unrelated lines).
> 
> So no, while there is no code _now_ that is relying on the invariant
> being kept after the last statement (which is what the static analyzer
> is finding out), the point is not for the compiler to realize that, but
> for human programmers to see it.

And the compiler (at least gcc) is indeed smart enough to realize that 
nothing uses the result from the last statement, and does optimize away 
the code associated to it already.  So this patch is unlikely to change 
anything to the compiled result.


Nicolas

^ permalink raw reply

* Re: [PATCH] make 'git clone' ask the remote only for objects it cares about
From: Nicolas Pitre @ 2009-09-27  0:26 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Jason Merrill, Matthieu Moy, git, Hin-Tak Leung
In-Reply-To: <20090926195039.GG14660@spearce.org>

On Sat, 26 Sep 2009, Shawn O. Pearce wrote:

> Nicolas Pitre <nico@fluxnic.net> wrote:
> > Current behavior of 'git clone' when not using --mirror is to fetch 
> > everything from the peer, and then filter out unwanted refs just before 
> > writing them out to the cloned repository.  This may become highly 
> > inefficient if the peer has an unusual ref namespace, or if it simply 
> > has "remotes" refs of its own, and those locally unwanted refs are 
> > connecting to a large set of objects which becomes unreferenced as soon 
> > as they are fetched.
> ...
> > +static void write_remote_refs(const struct ref *local_refs, const char *reflog)
> 
> Here reflog is now unused.  I'm going to squash this in.

Yeah, I noticed.  Since I didn't know what was the original intent for 
it, I just left it there.


Nicolas

^ permalink raw reply

* [PATCH] gitweb: fix spelling errors in comments
From: Miklos Vajna @ 2009-09-26 23:32 UTC (permalink / raw)
  To: git

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 gitweb/gitweb.perl |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

The plural form of "id" is "ids", not "id's", right?

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b2193..b66c4d0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4949,7 +4949,7 @@ sub git_blob_plain {
 			die_error(400, "No file name defined");
 		}
 	} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
-		# blobs defined by non-textual hash id's can be cached
+		# blobs defined by non-textual hash ids can be cached
 		$expires = "+1d";
 	}
 
@@ -5001,7 +5001,7 @@ sub git_blob {
 			die_error(400, "No file name defined");
 		}
 	} elsif ($hash =~ m/^[0-9a-fA-F]{40}$/) {
-		# blobs defined by non-textual hash id's can be cached
+		# blobs defined by non-textual hash ids can be cached
 		$expires = "+1d";
 	}
 
@@ -5346,7 +5346,7 @@ sub git_commit {
 	@difftree = map { chomp; $_ } <$fd>;
 	close $fd or die_error(404, "Reading git-diff-tree failed");
 
-	# non-textual hash id's can be cached
+	# non-textual hash ids can be cached
 	my $expires;
 	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
 		$expires = "+1d";
@@ -5513,7 +5513,7 @@ sub git_blobdiff {
 		$hash_parent ||= $diffinfo{'from_id'};
 		$hash        ||= $diffinfo{'to_id'};
 
-		# non-textual hash id's can be cached
+		# non-textual hash ids can be cached
 		if ($hash_base =~ m/^[0-9a-fA-F]{40}$/ &&
 		    $hash_parent_base =~ m/^[0-9a-fA-F]{40}$/) {
 			$expires = '+1d';
@@ -5733,7 +5733,7 @@ sub git_commitdiff {
 		die_error(400, "Unknown commitdiff format");
 	}
 
-	# non-textual hash id's can be cached
+	# non-textual hash ids can be cached
 	my $expires;
 	if ($hash =~ m/^[0-9a-fA-F]{40}$/) {
 		$expires = "+1d";
-- 
1.6.5.rc1.44.ga1675.dirty

^ permalink raw reply related

* [PATCH] git branch -D: give a better error message when lockfile creation fails
From: Miklos Vajna @ 2009-09-26 23:15 UTC (permalink / raw)
  To: Matthieu.Moy; +Cc: spearce, peff, git
In-Reply-To: <vpqy6o15v6m.fsf@bauges.imag.fr>

Previously the old error message just told the user that it was not
possible to delete the ref from the packed-refs file. Give instructions
on how to resolve the problem.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

Changes since the previous one:

* unable_to_lock_index() renamed to unable_to_lock()
* NORETURN is back for unable_to_lock_index_die()

 cache.h    |    1 +
 lockfile.c |   23 ++++++++++++++++++-----
 refs.c     |    4 +++-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/cache.h b/cache.h
index 1a6412d..797cc4c 100644
--- a/cache.h
+++ b/cache.h
@@ -489,6 +489,7 @@ struct lock_file {
 };
 #define LOCK_DIE_ON_ERROR 1
 #define LOCK_NODEREF 2
+extern int unable_to_lock(const char *path, int err, int noreturn);
 extern NORETURN void unable_to_lock_index_die(const char *path, int err);
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
 extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
diff --git a/lockfile.c b/lockfile.c
index eb931ed..3bacda4 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -156,17 +156,30 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
 }
 
 
-NORETURN void unable_to_lock_index_die(const char *path, int err)
+int unable_to_lock(const char *path, int err, int noreturn)
 {
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
 	if (err == EEXIST) {
-		die("Unable to create '%s.lock': %s.\n\n"
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s.\n\n"
 		    "If no other git process is currently running, this probably means a\n"
 		    "git process crashed in this repository earlier. Make sure no other git\n"
 		    "process is running and remove the file manually to continue.",
 		    path, strerror(err));
-	} else {
-		die("Unable to create '%s.lock': %s", path, strerror(err));
-	}
+	} else
+		strbuf_addf(&buf, "Unable to create '%s.lock': %s", path, strerror(err));
+	if (noreturn)
+		die(buf.buf);
+	ret = error(buf.buf);
+	strbuf_release(&buf);
+	return ret;
+}
+
+NORETURN void unable_to_lock_index_die(const char *path, int err)
+{
+	unable_to_lock(path, err, 1);
+	die("unable_to_lock() should have died already");
 }
 
 int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
diff --git a/refs.c b/refs.c
index 24865cf..3d635ae 100644
--- a/refs.c
+++ b/refs.c
@@ -972,8 +972,10 @@ static int repack_without_ref(const char *refname)
 	if (!found)
 		return 0;
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
-	if (fd < 0)
+	if (fd < 0) {
+		unable_to_lock(git_path("packed-refs"), errno, 0);
 		return error("cannot delete '%s' from packed refs", refname);
+	}
 
 	for (list = packed_ref_list; list; list = list->next) {
 		char line[PATH_MAX + 100];
-- 
1.6.5.rc1.44.ga1675.dirty

^ 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