* [PATCH 08/18] read-cache.c: mark file-local functions static
From: Junio C Hamano @ 2010-01-12 7:52 UTC (permalink / raw)
To: git
In-Reply-To: <1263282781-25596-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* has conflicts when applied to 'pu' but nothing "am -3" cannot fix.
* remove_index_entry_at() is left exported even though it can become
static in 'master'; it is used in 'pu'.
cache.h | 2 --
read-cache.c | 6 ++++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 30b9048..e7bb6b7 100644
--- a/cache.h
+++ b/cache.h
@@ -445,7 +445,6 @@ extern int index_name_pos(const struct index_state *, const char *name, int name
#define ADD_CACHE_JUST_APPEND 8 /* Append only; tree.c::read_tree() */
#define ADD_CACHE_NEW_ONLY 16 /* Do not replace existing ones */
extern int add_index_entry(struct index_state *, struct cache_entry *ce, int option);
-extern struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
extern void rename_index_entry_at(struct index_state *, int pos, const char *new_name);
extern int remove_index_entry_at(struct index_state *, int pos);
extern void remove_marked_cache_entries(struct index_state *istate);
@@ -615,7 +614,6 @@ static inline void hashclr(unsigned char *hash)
{
memset(hash, 0, 20);
}
-extern int is_empty_blob_sha1(const unsigned char *sha1);
#define EMPTY_TREE_SHA1_HEX \
"4b825dc642cb6eb9a060e54bf8d69288fbee4904"
diff --git a/read-cache.c b/read-cache.c
index 9033dd3..9f4f44c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -15,6 +15,8 @@
#include "revision.h"
#include "blob.h"
+static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really);
+
/* Index extensions.
*
* The first letter should be 'A'..'Z' for extensions that are not
@@ -156,7 +158,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
return 0;
}
-int is_empty_blob_sha1(const unsigned char *sha1)
+static int is_empty_blob_sha1(const unsigned char *sha1)
{
static const unsigned char empty_blob_sha1[20] = {
0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
@@ -1141,7 +1143,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
return has_errors;
}
-struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)
+static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int really)
{
return refresh_cache_ent(&the_index, ce, really, NULL);
}
--
1.6.6.280.ge295b7.dirty
^ permalink raw reply related
* [PATCH 04/18] date.c: mark file-local function static
From: Junio C Hamano @ 2010-01-12 7:52 UTC (permalink / raw)
To: git
In-Reply-To: <1263282781-25596-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Has a trivial conflict in git-compat-util.h when applyed to 'pu'
date.c | 2 +-
git-compat-util.h | 1 -
2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/date.c b/date.c
index 5d05ef6..45f3684 100644
--- a/date.c
+++ b/date.c
@@ -9,7 +9,7 @@
/*
* This is like mktime, but without normalization of tm_wday and tm_yday.
*/
-time_t tm_to_time_t(const struct tm *tm)
+static time_t tm_to_time_t(const struct tm *tm)
{
static const int mdays[] = {
0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
diff --git a/git-compat-util.h b/git-compat-util.h
index 5c59687..85dea12 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -198,7 +198,6 @@ extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)))
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
extern int prefixcmp(const char *str, const char *prefix);
-extern time_t tm_to_time_t(const struct tm *tm);
static inline const char *skip_prefix(const char *str, const char *prefix)
{
--
1.6.6.280.ge295b7.dirty
^ permalink raw reply related
* [PATCH 03/18] pretty.c: mark file-local function static
From: Junio C Hamano @ 2010-01-12 7:52 UTC (permalink / raw)
To: git
In-Reply-To: <1263282781-25596-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
commit.h | 1 -
pretty.c | 2 +-
2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/commit.h b/commit.h
index e5332ef..24128d7 100644
--- a/commit.h
+++ b/commit.h
@@ -73,7 +73,6 @@ struct pretty_print_context
struct reflog_walk_info *reflog_info;
};
-extern int non_ascii(int);
extern int has_non_ascii(const char *text);
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
extern char *reencode_commit_message(const struct commit *commit,
diff --git a/pretty.c b/pretty.c
index 8f5bd1a..9001379 100644
--- a/pretty.c
+++ b/pretty.c
@@ -83,7 +83,7 @@ static int get_one_line(const char *msg)
}
/* High bit set, or ISO-2022-INT */
-int non_ascii(int ch)
+static int non_ascii(int ch)
{
return !isascii(ch) || ch == '\033';
}
--
1.6.6.280.ge295b7.dirty
^ permalink raw reply related
* [PATCH 02/18] builtin-rev-list.c: mark file-local function static
From: Junio C Hamano @ 2010-01-12 7:52 UTC (permalink / raw)
To: git
In-Reply-To: <1263282781-25596-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
bisect.h | 2 --
builtin-rev-list.c | 2 +-
2 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/bisect.h b/bisect.h
index 82f8fc1..0862ce5 100644
--- a/bisect.h
+++ b/bisect.h
@@ -27,8 +27,6 @@ struct rev_list_info {
const char *header_prefix;
};
-extern int show_bisect_vars(struct rev_list_info *info, int reaches, int all);
-
extern int bisect_next_all(const char *prefix);
extern int estimate_bisect_steps(int all);
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index cd97ded..c924b3a 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -253,7 +253,7 @@ static void print_var_int(const char *var, int val)
printf("%s=%d\n", var, val);
}
-int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
+static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
{
int cnt, flags = info->bisect_show_flags;
char hex[41] = "";
--
1.6.6.280.ge295b7.dirty
^ permalink raw reply related
* Re: [PATCH 1/4] git-svn: fix the trivial case of 'src and dst not in the same repo' during branch/tag
From: Eric Wong @ 2010-01-12 4:27 UTC (permalink / raw)
To: Igor Mironov; +Cc: git
In-Reply-To: <4B4B4FDB.4000602@gmail.com>
Igor Mironov <igor.a.mironov@gmail.com> wrote:
> This fixes the following issue:
> $ git svn branch -t --username=svnuser --commit-url=https://myproj.domain.com/svn mytag
Thanks Igor!
I've shorted the subject lines and line-wrapped the commit messages to
fit in standard terminals, and pushed them out to:
git://git.bogomips.org/git-svn
Igor Mironov (4):
git-svn: fix mismatched src/dst errors for branch/tag
git-svn: respect commiturl option for branch/tag
git-svn: add --username/commit-url options for branch/tag
git-svn: document --username/commit-url for branch/tag
I'll be working on dcommit error handling in a bit
before asking Junio to pull.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH] Display author and committer after "git commit"
From: Adam Megacz @ 2010-01-12 1:51 UTC (permalink / raw)
To: git
In-Reply-To: <7vzl4lw160.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Why isn't the "# Author:" and "# Committer:" information you see along
> with "git status" output in the editor "git commit" gives you sufficient
> if it is to avoid unconfigured/misconfigured names and e-mail
> addresses?
It is sufficient! But, as others have mentioned, it is not displayed
when "git commit -m" is used. The patch in this thread rectifies that
omission.
> Too much clutter for too little gain, except for a very first few commits
> in the repository.
Funny, I think of those "+++" "-----" histogram things as clutter. I
guess it's subjective.
- a
^ permalink raw reply
* Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add "git" links in project list page
From: J.H. @ 2010-01-12 0:39 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, John 'Warthog9' Hawley
In-Reply-To: <201001091220.20446.jnareb@gmail.com>
On 01/09/2010 03:20 AM, Jakub Narebski wrote:
> On Sat, 9 Jan 2010, J.H. wrote:
>> On 01/03/2010 08:07 AM, Jakub Narebski wrote:
>>> From: John 'Warthog9' Hawley <warthog9@kernel.org>
>>>
>>> This adds a "git" link for each project in the project list page,
>>> should a common $gitlinkurl_base be defined and not empty. The full
>>> URL of each link is composed of $gitlinkurl_base and project name.
>>> It is intended for git:// links, and in fact GITWEB_BASE_URL build
>>> variable is used as its default value only if it starts with git://
>>>
>>> This does make the assumption that the git repositories share a common
>>> path. Nothing to date is known to actually make use of introduced
>>> link.
>>>
>>> Created "git" link follows rel=vcs-* microformat specification:
>>> http://kitenet.net/~joey/rfc/rel-vcs/
>>>
>>> Signed-off-by: John 'Warthog9' Hawley <warthog9@kernel.org>
>>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>>> ---
>>> I think it might be good idea... but for the fact "Nothing to date is
>>> known to actually make use of introduced link". What's its intended
>>> use?
>>>
>>> Differences to original version by John 'Warthog9' Hawley (J.H.):
>>> * It doesn't cause syntax error ;-)
>>> * Escaping of attribute value is left to CGI.pm (avoid double escaping)
>>> * $gitlinkurl got renamed to $gitlinkurl_base, now includes git://
>>> prefix, and defaults to GITWEB_BASE_URL if it begins with git://
>>> * Added description of $gitlinkurl_base to gitweb/README
>>> * Uses rel=vcs-* microformat by Joey Hess
>
>>> gitweb/README | 4 ++++
>>> gitweb/gitweb.perl | 8 ++++++++
>>> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> A reminder - this patch series consists of the following patches:
> [PATCHv2 1/4 (resent)] gitweb: Load checking
> [RFC/PATCHv2 2/4 (resent)] gitweb: Add option to force version match
> [PATCHv3 3/4 (resent)] gitweb: Optionally add "git" links in project list page
> [PATCHv2/RFC 4/4 (resent)] gitweb: Makefile improvements
>
>> Ok I've been debating this as I've been going through the patches, I've
>> got small modifications on top of your patches Jakub for 1 and 2,
>> haven't pushed them yet but they are relatively trivial. The changes to
>> the first patch sets things up for additional load checkers to be added
>> later on.
>
> Good idea, although I think that such addition can be left for a separate
> patch.
It only set things up for additional checkers, adding a framework for
them to be added. I would agree that other load checkers should be in
additional patches, just trying to make it easy for other later on.
> By the way, are you doing if-elsif fallback chain, trying different
> mechanisms (like '/proc/loadavg', BSD::getloadavg, etc.), or did
> you made get_loadavg() into code reference, i.e. run it with
> $get_loadavg->(), which has the advantage that the gitweb admin can
> override it in gitweb config file (including such thing like simply
> using load average over last 5 minutes, and not over last minute)?
I had set it up using the if-elsif fallback chain. Not sure that an
admin would be overriding it for the most part, but it probably wouldn't
be hard to do both. Decent defaults for the general case, and if anyone
wants they can override it afterwards for special cases.
>
>> The second changes the error message to use/abuse die_error()
>> vs. doing it's own thing (though I still think this should be on by
>> default).
>
> True, the error message could use improvement (and not only using
> its own class instead of abusing 'readme' class, or renaming 'readme'
> class to something more generic). The problem with error message for
> this is who is the target of this message: is it gitweb administrator
> (who can change gitweb configuration), or is it gitweb user (who need
> to contact web admin).
Admin for sure, this is intended more to be a speed bump that forces the
admin to stop and consider what's happening. I originally added it
because I ended up in a situation where gitweb didn't match the
installed git and gitweb was dying silently. I wanted to just give the
admin a better guess at what was going on mainly.
> The problem with this patch is that for it to be useful for protecting
> against silent errors it should be on by default, but OTOH having it on
> by default is quite inconvenient.
Upside is once it's been disabled it should stay disabled. I'll admit
it's not ideal but it's at least a stop gap in the interim till the
other silent errors get filled in.
> Best solution would be to treat core of this issue, namely eliminate
> silent errors and always provide some message in case of error.
Agreed, this is really just a stop-gap measure. I would imagine *most*
instalations of gitweb are installed from the distro's package
management so that would likely be upgraded at the same time as git itself.
>> Patch 4 I don't have anything to add or change at this point.
>>
>> This patch has me pondering and I'm unsure of what I'd suggest, mainly
>> because of the addition of the smart http support meaning that git://
>> and http:// are legitimate and useful links for supporting full git
>> transactions.
>>
>> I may withdraw the patch entirely since the link on kernel.org has been
>> around for years, and I'm unsure if anything actually uses it (though I
>> can see it being useful still). If it stays I think there's got to be a
>> way to specifically mark a url as being the one to link to vs.
>> defaulting to git:// (or allow for a marking to override the git://) and
>> I need to ponder that.
>
> Also, it has to be _fast_, I think, i.e. no reading cloneurl and repo
> config (for gitweb.url) for each repository.
>
> You can always remove the check for "git://" prefix, and/or take first
> base in @git_base_url_list.
Taking the first one by default may make more sense - I'll try that.
>> I have given some initial thought to converting the $output options I'm
>> currently using to a print <FH> that Jakub is suggesting & exploring.
>
> It's 'print {$fh}', i.e. use indirect filehandle, not global filehandle.
>
>> I think all told it's going to be a similarly sized patch, since all
>> output still has to get adjusted (including the things that directly
>> output but don't print).
>
> print -> print {$fh} can be separate patch, and it can be checked that
> it produces the same results. Well print -> $output .= could also be
> separate patch...
>
>> I'm unsure if there's a real advantage to
>> either way, other than design preference. My patch (forcing the output
>> to get passed around) moves towards more of a modal style design
>> separating data & layout vs. it's combined nature now, well it's a step
>> in that direction anyway.
>
> Errr... what? Forcing buffering (you need to read whole output into
> memory, including for snapshots (uncompressed in case of .tar.gz))
> is IMVHO orthogonal to the issue of separating data & layout.
> BTW. Modern web server interfaces like Rack, PSGI/Plack etc. explicitly
> include streaming support.
The inbuilt streaming support does change things, and I don't think it
ultimately changes my caching engine really anyway - I should have that
change done shortly.
>
> The advantage of doing 'print {$fh}' is that $fh can be \*STDOUT, can
> be \$buffer, but can be filehandle to (temporary) file on disk, and
> you can even "tee" it, i.e. both write to memory/file, and to STDOUT.
> The number of possible choices / possible improvements is much larger.
>
> And what is also important it means that people who do not use caching
> do not suffer latency penalty and memory pressure from caching
> infrastructure they do not use.
>
>
> P.S.
> Subject: [Virus] Exploit.PDF-9669
> X-Virus-Scanned: ClamAV 0.88.7/10275/Fri Jan 8 17:06:46 2010 on shards.monkeyblade.net
> X-Virus-Status: Infected with Exploit.PDF-9669
> X-Original-Subject: Re: [PATCHv3 3/4 (resent)] gitweb: Optionally add "git"
> links in project list page
>
> A message sent from <warthog9@eaglescrag.net> to
> <jnareb@gmail.com>
> <git@vger.kernel.org>
> <warthog9@kernel.org>
> contained Exploit.PDF-9669 and has not been delivered.
Clamav on my personal server wigging out and deciding that all kinds of
stuff was PDF viruses, *NO* idea why. I've had a different problem but
still involving Clamav on kernel.org. Both had been working fine for
quite some time and the only thing I can think of is that either an
update from clamav went awry, or a date dependent thing triggered and is
causing havoc.
Should have a patch series out in an hour or so.
- John 'Warthog9' Hawley
^ permalink raw reply
* Re: Unable to get "pretty" URL aliases working
From: Adam Nielsen @ 2010-01-11 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hrpvzqa.fsf@alter.siamese.dyndns.org>
Hi all,
Thanks for the replies!
> In any case, "attempting to learn Git" doesn't mix well with use of
> "insteadOf" to me. If you know /path/to/repos/project.git is what you
> want to access, any "attempting to learn Git" person would do more
> straight-forward "git clone ssh://myserver/path/to/repos/project.git", or
> "git clone myserver:/path/to/repos/project.git" which is even better (it
> is shorter to type and is a more natural form to spell ssh transport).
Jumping in the deep end is a valid method of learning :-) Does that
mean anyone using my repositories really needs to know exactly where on
the server I choose to keep them? Something like "git clone
myserver:/mnt/raid/nightly-backedup/public/repositories/git/project.git"
isn't particularly friendly. (It's something that bugs me with NFS as
well.) What happens if I want to move the repositories to another area
on the server? Do all users have to update their local copies with a
new origin address?
Is there any hacky way that this could be made to work? I guess I could
symlink my repository directory to /git, then myserver:/git/project.git
might work.
What actually happens when you use the ssh:// style connection?
git-daemon has a --base-path option which solves this issue for git://
URLs, so do ssh:// URLs not use the git protocol at all? What is
git+ssh://? Does that SSH to the machine and then connect to git-daemon
via localhost? Because that would presumably make use of --base-path.
Thanks again,
Adam.
^ permalink raw reply
* Re: [PATCH 6/6] Documentation: tweak How Merge Works
From: Thomas Rast @ 2010-01-11 23:11 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <20100111084355.GF23806@progeny.tock>
The below and earlier comments aside, I really like this series. It
seems to make the manpage much more accessible.
Jonathan Nieder wrote:
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index ec9c6d3..7ae0f65 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -96,62 +96,56 @@ merge commit.
>
> This behavior can be suppressed with the `--no-ff` option.
>
> -include::merge-strategies.txt[]
> -
> -
I'm not sure whether you deliberately did this, or deliberately
deferred it to this patch, but this "sneak moves" the merge-strategies
section beyond "TRUE MERGE" (was "HOW MERGE WORKS").
So the section layout changes as follows when comparing current master
with your series:
NAME
SYNOPSIS
DESCRIPTION
OPTIONS
+PRE-MERGE CHECKS
+FAST-FORWARD MERGE
+TRUE MERGE
MERGE STRATEGIES
-CONFIGURATION
-HOW MERGE WORKS
HOW CONFLICTS ARE PRESENTED
HOW TO RESOLVE CONFLICTS
EXAMPLES
+CONFIGURATION
SEE ALSO
AUTHOR
DOCUMENTATION
GIT
NOTES
While I agree with the general intent of deferring the strategies
further back, wouldn't it be better go all the way and instead put
them before (or even after, but one of them uses -s ours) "EXAMPLES"?
The average user will care more about conflicts than about strategies
other than 'recursive'.
> +1. A version reconciling the changes from all branches to be
> + merged is written to the index file and your working tree;
> +2. The index file is written out as a tree;
> 3. The tree gets committed; and
> 4. The `HEAD` pointer gets advanced.
Could we do away with the detail here? The user most likely does not
care about the exact order because he cannot "see" it happening
anyway. So how about
A merged version reconciling the changes from all branches to be
merged is committed, and your HEAD, index, and working tree are
updated to it. (It is possible to have modifications in the working
tree as long as they do not overlap; the update will preserve them.)
and then snip everything up to
> +When it is not obvious how to reconcile the changes, the following
> +happens:
because that is far more important to the user: he is left in the
middle of the described state.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH 4/6] Documentation: emphasize when git merge terminates early
From: Thomas Rast @ 2010-01-11 23:11 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Petr Baudis, git
In-Reply-To: <20100111083754.GD23806@progeny.tock>
Jonathan Nieder wrote:
> +In some other version control systems, you merge/update and then
> +commit, with the risk of clobbering your changes with humongous
> +conflicts. You shouldn't be pulling or merging if you haven't got
> +your own work into good shape and committed it locally (see also
> +linkgit:git-stash[1]).
Isn't it wrong to talk about other VCS, especially when writing down
what Git does *not*? The user wants to know what git *does*,
otherwise he wouldn't be looking in the manual.
> +If all named commits are already ancestors of `HEAD`, 'git merge'
> +will exit early with the message "Already up-to-date."
<aside>
I suspect this is little-known but the equivalent message for an
octopus is actually: "Already up-to-date. Yeeah!"
</aside>
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Junio C Hamano @ 2010-01-11 22:51 UTC (permalink / raw)
To: Thiago Farina; +Cc: Nicolas Pitre, Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <a4c8a6d01001111434j18b77b88j525b9acb47c7e100@mail.gmail.com>
Thiago Farina <tfransosi@gmail.com> writes:
> I can add the comment if Junio is ok with it. Something like this:
> "This functions is for debugging purpose only. Please, do NOT remove
> it."
Something like this should suffice, as I think "Please blah" is just
superfluous.
I don't know if I like "#ifdef DEBUG" better, though. It will still let
you say "call print_string_list(...)" in your debugger session, and people
wouldn't muck with it even if there is no caller.
diff --git a/string-list.c b/string-list.c
index 1ac536e..793e619 100644
--- a/string-list.c
+++ b/string-list.c
@@ -139,6 +139,7 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c
}
+#ifndef NDEBUG
void print_string_list(const char *text, const struct string_list *p)
{
int i;
@@ -147,6 +148,7 @@ void print_string_list(const char *text, const struct string_list *p)
for (i = 0; i < p->nr; i++)
printf("%s:%p\n", p->items[i].string, p->items[i].util);
}
+#endif
struct string_list_item *string_list_append(const char *string, struct string_list *list)
{
diff --git a/string-list.h b/string-list.h
index 6569cf6..8598257 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,7 +12,6 @@ struct string_list
unsigned int strdup_strings:1;
};
-void print_string_list(const char *text, const struct string_list *p);
void string_list_clear(struct string_list *list, int free_util);
/* Use this function to call a custom clear function on each util pointer */
^ permalink raw reply related
* Re: [RFC PATCH (WIP)] Show a dirty working tree and a detached HEAD in status for submodule
From: Junio C Hamano @ 2010-01-11 22:45 UTC (permalink / raw)
To: Jens Lehmann
Cc: Git Mailing List, Junio C Hamano, Johannes Schindelin,
Shawn O. Pearce, Heiko Voigt, Lars Hjemli
In-Reply-To: <4B4BA096.5000909@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> Until now a submodule only showed up as changed in the supermodule when
> the last commit in the submodule differed from the one in the index or
> the last commit of the superproject. A dirty working tree or a detached
> HEAD in a submodule were just ignored when looking at it from the
> superproject.
>
> This patch shows these changes when using git status or one of the diff
> commands which compare against the working tree in the superproject.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
>
> This is the first version of a patch letting git status and the git
> diff family show dirty working directories and a detached HEAD in
> directories. It is not intended to be merged in its present form but
> to be used as a starting point for discussion if this is going in
> the right direction.
>
>
> What the patch does:
>
> * It makes git show submodules as modified in the superproject when
> one or more of these conditions are met:
>
> a) The submodule contains untracked files
> b) The submodule contains modified files
> c) The submodules HEAD is not on a local or remote branch
>
> That can be seen when using either "git status", "git diff[-files]"
> & "git diff[-index] HEAD" (and with "git gui" & gitk).
If the submodule is checked out, _and_ if the HEAD there, either detached
or not, does not agree with what the "other" one records (i.e. the commit
recorded in an entry in the index, or in the tree, that you are comparing
your work tree against), then it also should be considered modified. I
don't think your (a)-(c) cover this case.
Also I don't understand why you want to treat (c) any specially at all.
Even if (c) is something we _should_ report, please do not call that as
"detached" in its implementation. "detached HEAD" has a very precise
technical meaning, and can point at the same commit as a local or a remote
tracking branch, which is very different from the definition your
implementation seems to use.
> * This behavior is not configurable but activated by default. A config
> option is needed here.
I doubt it.
My gut feeling is that this should be _always_ on for a submodule
directory that has been "submodule init/update". The user is interested
in that particular submodule, and any change to it should be reported for
both classes of users. Theose who meant to use the submodule read-only
need to be able to notice that they accidentally made the submodule dirty
before making a commit in the superproject. Those who wanted to work in
submodule needs to know if the state is in sync with what they expect
before making a commit in the superproject.
That of course is provided if the unconditional check does not trigger for
submodules that the user hasn't "submodue init"ed; I think you did that
correctly at the beginning of your is_submodule_modified() implementation.
> +static int is_submodule_head_detached(const char *path)
> +{
I don't understand why you should care which branch the submodule happens
to be on, as long as the next commit you make in the superproject records
the commit that is checked out in the submodule.
Of course you may want to be careful when "pushing" the superproject
results out (i.e. you would want to push out the history leading to that
commit at the submodule HEAD in the submodule history), so that the people
who are pulling from the repository you are pushing into will have
everything available.
But the thing is, in a distributed environment, the submodule HEAD being
at the tip of _some_ branch (either local or remote) you have doesn't mean
anything to help them. IOW, for protect others, you would need a check
when you _push out_ (either in 'push' or on the receiving end).
So I'd suggest dropping this condition in "status/diff" that is about
preparing to make the next commit in your _local_ history.
If "must be reachable from somewhere" is a condition worth caring about in
some context other than "status/diff", you can do an equivalent of:
$ git rev-parse HEAD --not --all | git rev-list --stdin
and see if anything comes out (in which case you have commits that are not
reachable from any of your refs other than the detached HEAD).
But that is not "is HEAD detached?"; it is something else. "Dangling",
perhaps, as that is how "git fsck" call commits that are not reachable
from any of your refs ("fsck" considers HEAD a part of refs, so it is not
strictly correct but it is much closer).
^ permalink raw reply
* [PATCH 2/2] Use $(git rev-parse --show-toplevel) in cd_to_toplevel().
From: Steven Drake @ 2010-01-11 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <alpine.LNX.2.00.1001102011420.2560@vqena.qenxr.bet.am>
rev-parse --show-toplevel gives the absolute (aka "physical") path of the
toplevel directory and is more portable as 'cd -P' is not supported by all
shell implementations.
This is also closer to what setup_work_tree() does.
Signed-off-by: Steven Drake <sdrake@xnet.co.nz>
---
git-sh-setup.sh | 19 +++++--------------
1 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index dfcb807..d56426d 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -120,20 +120,11 @@ is_bare_repository () {
}
cd_to_toplevel () {
- cdup=$(git rev-parse --show-cdup)
- if test ! -z "$cdup"
- then
- # The "-P" option says to follow "physical" directory
- # structure instead of following symbolic links. When cdup is
- # "../", this means following the ".." entry in the current
- # directory instead textually removing a symlink path element
- # from the PWD shell variable. The "-P" behavior is more
- # consistent with the C-style chdir used by most of Git.
- cd -P "$cdup" || {
- echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
- exit 1
- }
- fi
+ cdup=$(git rev-parse --show-toplevel) &&
+ cd "$cdup" || {
+ echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
+ exit 1
+ }
}
require_work_tree () {
--
1.6.4
^ permalink raw reply related
* [PATCH 1/2] Add 'git rev-parse --show-toplevel' option.
From: Steven Drake @ 2010-01-11 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <alpine.LNX.2.00.1001102011420.2560@vqena.qenxr.bet.am>
Shows the absolute path of the top-level working directory.
Signed-off-by: Steven Drake <sdrake@xnet.co.nz>
---
Documentation/git-rev-parse.txt | 3 +++
builtin-rev-parse.c | 6 ++++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 82045a2..dc829b3 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -112,6 +112,9 @@ OPTIONS
--remotes::
Show tag refs found in `$GIT_DIR/refs/remotes`.
+--show-toplevel::
+ Show the absolute path of the top-level directory.
+
--show-prefix::
When the command is invoked from a subdirectory, show the
path of the current directory relative to the top-level
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 37d0233..96ab8bb 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -581,6 +581,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
for_each_remote_ref(show_reference, NULL);
continue;
}
+ if (!strcmp(arg, "--show-toplevel")) {
+ const char *work_tree = get_git_work_tree();
+ if (work_tree)
+ printf("%s\n", work_tree);
+ continue;
+ }
if (!strcmp(arg, "--show-prefix")) {
if (prefix)
puts(prefix);
--
1.6.4
^ permalink raw reply related
* Re: [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Thiago Farina @ 2010-01-11 22:34 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <alpine.LFD.2.00.1001111722150.10143@xanadu.home>
On Mon, Jan 11, 2010 at 8:23 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 11 Jan 2010, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > On Mon, 11 Jan 2010, Thiago Farina wrote:
>> >
>> >> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>> >> ---
>> >
>> > It was never used, except for debugging. Does it hurt you really all that
>> > much?
>>
>> Exactly my feeling.
>>
>> I think I discarded at least two other patches sent to me to remove this
>> "unused" (but obviously meant for debugging) function in the past. I
>> guess we'll keep the function this time, too.
>
> If so many people are so inclined to discard it, then maybe a little
> comment could be added to explain its existance?
>
I can add the comment if Junio is ok with it. Something like this:
"This functions is for debugging purpose only. Please, do NOT remove
it."
^ permalink raw reply
* Re: [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Nicolas Pitre @ 2010-01-11 22:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Thiago Farina, git
In-Reply-To: <7v3a2ccmrg.fsf@alter.siamese.dyndns.org>
On Mon, 11 Jan 2010, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Mon, 11 Jan 2010, Thiago Farina wrote:
> >
> >> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
> >> ---
> >
> > It was never used, except for debugging. Does it hurt you really all that
> > much?
>
> Exactly my feeling.
>
> I think I discarded at least two other patches sent to me to remove this
> "unused" (but obviously meant for debugging) function in the past. I
> guess we'll keep the function this time, too.
If so many people are so inclined to discard it, then maybe a little
comment could be added to explain its existance?
Nicolas
^ permalink raw reply
* Re: [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Junio C Hamano @ 2010-01-11 22:14 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Thiago Farina, git
In-Reply-To: <alpine.DEB.1.00.1001112252040.4985@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 11 Jan 2010, Thiago Farina wrote:
>
>> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>> ---
>
> It was never used, except for debugging. Does it hurt you really all that
> much?
Exactly my feeling.
I think I discarded at least two other patches sent to me to remove this
"unused" (but obviously meant for debugging) function in the past. I
guess we'll keep the function this time, too.
^ permalink raw reply
* [RFC PATCH (WIP)] Show a dirty working tree and a detached HEAD in status for submodule
From: Jens Lehmann @ 2010-01-11 22:05 UTC (permalink / raw)
To: Git Mailing List
Cc: Junio C Hamano, Johannes Schindelin, Shawn O. Pearce, Heiko Voigt,
Lars Hjemli
Until now a submodule only showed up as changed in the supermodule when
the last commit in the submodule differed from the one in the index or
the last commit of the superproject. A dirty working tree or a detached
HEAD in a submodule were just ignored when looking at it from the
superproject.
This patch shows these changes when using git status or one of the diff
commands which compare against the working tree in the superproject.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
This is the first version of a patch letting git status and the git
diff family show dirty working directories and a detached HEAD in
directories. It is not intended to be merged in its present form but
to be used as a starting point for discussion if this is going in
the right direction.
What the patch does:
* It makes git show submodules as modified in the superproject when
one or more of these conditions are met:
a) The submodule contains untracked files
b) The submodule contains modified files
c) The submodules HEAD is not on a local or remote branch
That can be seen when using either "git status", "git diff[-files]"
& "git diff[-index] HEAD" (and with "git gui" & gitk).
What the patch doesn't do (yet):
* It still breaks tests t7400-submodule-basic.sh &
t7407-submodule-foreach.sh.
* It doesn't give detailed output when doing a "git diff* -p" with or
without the --submodule option. It should show something like
diff --git a/sub b/sub
index 5431f52..3f35670 160000
--- a/sub
+++ b/sub
@@ -1 +1 @@
-Subproject commit 5431f529197f3831cdfbba1354a819a79f948f6f
+Subproject commit 3f356705649b5d566d97ff843cf193359229a453-dirty
for "git diff* -p" (notice the "-dirty" in the last line) or
Submodule sub contains untracked files
Submodule sub contains modified files
Submodule sub contains a HEAD not on any branch
Submodule sub 5431f52..3f35670:
> commit message 1
when using the --submodule option of the diff family.
* This behavior is not configurable but activated by default. A config
option is needed here.
* It doesn't give optimal performance:
- Apart from the fact that checking submodules this way will always
be slower than ignoring their changes as git does until now, doing
two run_command() calls for each submodule is not going to help at
all (especially when running on Windows).
- AFAICS the check for a detached HEAD would be faster for the most
probable case if it would check against remotes/origin/master first.
And it could stop when the first branch was found instead of
continuing to look for others too as "git branch --contains" does.
- Similar for the test for a dirty working directory, no need to have
the full list of new and modified files, it could stop at the first
one it finds.
- If no detailed output is wanted the examination of HEAD and the
working directory is not necessary when the HEAD and the commit in
the index of the superproject already don't match.
What do you think?
diff-lib.c | 7 +++-
submodule.c | 103 +++++++++++++++++++++++++++++++++++++++++++
submodule.h | 1 +
t/t7506-status-submodule.sh | 45 ++++++++++++++++++-
4 files changed, 154 insertions(+), 2 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 1c7e652..323305a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -159,7 +159,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
continue;
}
- if (ce_uptodate(ce) || ce_skip_worktree(ce))
+ if ((ce_uptodate(ce) && !S_ISGITLINK(ce->ce_mode)) || ce_skip_worktree(ce))
continue;
/* If CE_VALID is set, don't look at workdir for file removal */
@@ -176,6 +176,11 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
continue;
}
changed = ce_match_stat(ce, &st, ce_option);
+ if (S_ISGITLINK(ce->ce_mode)) {
+ /* TODO: This should not be executed when the submodule is changed
+ * and only short ouptut is wanted for performance reasons. */
+ changed |= is_submodule_modified(ce->name);
+ }
if (!changed) {
ce_mark_uptodate(ce);
if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
diff --git a/submodule.c b/submodule.c
index 86aad65..b35f1b3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -4,6 +4,7 @@
#include "diff.h"
#include "commit.h"
#include "revision.h"
+#include "run-command.h"
int add_submodule_odb(const char *path)
{
@@ -112,3 +113,105 @@ void show_submodule_summary(FILE *f, const char *path,
}
strbuf_release(&sb);
}
+
+static int is_submodule_head_detached(const char *path)
+{
+ int retval, len;
+ struct child_process branch;
+ const char *argv[] = {
+ "branch",
+ "-a",
+ "--contains",
+ "HEAD",
+ NULL,
+ };
+ char *env[3];
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "GIT_WORK_TREE=%s", path);
+ env[0] = strbuf_detach(&buf, NULL);
+ strbuf_addf(&buf, "GIT_DIR=%s/.git", path);
+ env[1] = strbuf_detach(&buf, NULL);
+ env[2] = NULL;
+
+ memset(&branch, 0, sizeof(branch));
+ branch.argv = argv;
+ branch.env = (const char *const *)env;
+ branch.git_cmd = 1;
+ branch.no_stdin = 1;
+ branch.out = -1;
+ if (start_command(&branch))
+ die("Could not run git branch -a --contains HEAD");
+
+ len = strbuf_read(&buf, branch.out, 1024);
+ close(branch.out);
+
+ if (finish_command(&branch))
+ die("git branch -a --contains HEAD failed");
+
+ retval = (strncmp(buf.buf, "* (no branch)", 13) == 0);
+
+ free(env[0]);
+ free(env[1]);
+ strbuf_release(&buf);
+ return retval;
+}
+
+static int is_submodule_working_directory_dirty(const char *path)
+{
+ int len;
+ struct child_process branch;
+ const char *argv[] = {
+ "status",
+ "--porcelain",
+ NULL,
+ };
+ char *env[3];
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(&buf, "GIT_WORK_TREE=%s", path);
+ env[0] = strbuf_detach(&buf, NULL);
+ strbuf_addf(&buf, "GIT_DIR=%s/.git", path);
+ env[1] = strbuf_detach(&buf, NULL);
+ env[2] = NULL;
+
+ memset(&branch, 0, sizeof(branch));
+ branch.argv = argv;
+ branch.env = (const char *const *)env;
+ branch.git_cmd = 1;
+ branch.no_stdin = 1;
+ branch.out = -1;
+ if (start_command(&branch))
+ die("Could not run git status --porcelain");
+
+ len = strbuf_read(&buf, branch.out, 1024);
+ close(branch.out);
+
+ if (finish_command(&branch))
+ die("git status --porcelain failed");
+
+ free(env[0]);
+ free(env[1]);
+ strbuf_release(&buf);
+ return len != 0;
+}
+
+int is_submodule_modified(const char *path)
+{
+ struct strbuf buffer = STRBUF_INIT;
+
+ strbuf_addf(&buffer, "%s/.git/", path);
+ if (!is_directory(buffer.buf)) {
+ strbuf_release(&buffer);
+ return 0;
+ }
+ strbuf_release(&buffer);
+
+ if (is_submodule_head_detached(path))
+ return 1;
+
+ if (is_submodule_working_directory_dirty(path))
+ return 1;
+
+ return 0;
+}
diff --git a/submodule.h b/submodule.h
index 4c0269d..0773121 100644
--- a/submodule.h
+++ b/submodule.h
@@ -4,5 +4,6 @@
void show_submodule_summary(FILE *f, const char *path,
unsigned char one[20], unsigned char two[20],
const char *del, const char *add, const char *reset);
+int is_submodule_modified(const char *path);
#endif
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 3ca17ab..509754a 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -10,8 +10,12 @@ test_expect_success 'setup' '
: >bar &&
git add bar &&
git commit -m " Add bar" &&
+ : >foo &&
+ git add foo &&
+ git commit -m " Add foo" &&
cd .. &&
- git add sub &&
+ echo output > .gitignore
+ git add sub .gitignore &&
git commit -m "Add submodule sub"
'
@@ -23,6 +27,45 @@ test_expect_success 'commit --dry-run -a clean' '
git commit --dry-run -a |
grep "nothing to commit"
'
+
+echo "changed" > sub/foo
+test_expect_success 'status with modified file in submodule' '
+ git status | grep "modified: sub"
+'
+test_expect_success 'status with modified file in submodule (porcelain)' '
+ git status --porcelain >output &&
+ diff output - <<-EOF
+ M sub
+EOF
+'
+(cd sub && git checkout foo)
+
+echo "content" > sub/new-file
+test_expect_success 'status with untracked file in submodule' '
+ git status | grep "modified: sub"
+'
+test_expect_success 'status with untracked file in submodule (porcelain)' '
+ git status --porcelain >output &&
+ diff output - <<-EOF
+ M sub
+EOF
+'
+rm sub/new-file
+
+(cd sub && 2>/dev/null
+old_head=$(cat .git/refs/heads/master) &&
+git reset --hard HEAD^ &&
+git checkout $old_head 2>/dev/null)
+test_expect_success 'status with detatched HEAD in submodule' '
+ git status | grep "modified: sub"
+'
+test_expect_success 'status with detatched HEAD in submodule (porcelain)' '
+ git status --porcelain >output &&
+ diff output - <<-EOF
+ M sub
+EOF
+'
+
test_expect_success 'rm submodule contents' '
rm -rf sub/* sub/.git
'
--
1.6.6.203.g6b27d.dirty
^ permalink raw reply related
* Re: [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Johannes Schindelin @ 2010-01-11 21:52 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
In-Reply-To: <1263245389-1558-1-git-send-email-tfransosi@gmail.com>
Hi.
On Mon, 11 Jan 2010, Thiago Farina wrote:
> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
> ---
It was never used, except for debugging. Does it hurt you really all that
much?
Ciao,
Dscho
^ permalink raw reply
* [PATCH] string-list: remove print_string_list, since it is not used anymore.
From: Thiago Farina @ 2010-01-11 21:29 UTC (permalink / raw)
To: git
Signed-off-by: Thiago Farina <tfransosi@gmail.com>
---
string-list.c | 10 ----------
string-list.h | 1 -
2 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/string-list.c b/string-list.c
index 1ac536e..e929745 100644
--- a/string-list.c
+++ b/string-list.c
@@ -138,16 +138,6 @@ void string_list_clear_func(struct string_list *list, string_list_clear_func_t c
list->nr = list->alloc = 0;
}
-
-void print_string_list(const char *text, const struct string_list *p)
-{
- int i;
- if ( text )
- printf("%s\n", text);
- for (i = 0; i < p->nr; i++)
- printf("%s:%p\n", p->items[i].string, p->items[i].util);
-}
-
struct string_list_item *string_list_append(const char *string, struct string_list *list)
{
ALLOC_GROW(list->items, list->nr + 1, list->alloc);
diff --git a/string-list.h b/string-list.h
index 6569cf6..8598257 100644
--- a/string-list.h
+++ b/string-list.h
@@ -12,7 +12,6 @@ struct string_list
unsigned int strdup_strings:1;
};
-void print_string_list(const char *text, const struct string_list *p);
void string_list_clear(struct string_list *list, int free_util);
/* Use this function to call a custom clear function on each util pointer */
--
1.6.6.103.g699d2
^ permalink raw reply related
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Linus Torvalds @ 2010-01-11 21:24 UTC (permalink / raw)
To: Fredrik Kuivinen
Cc: Junio C Hamano, Miles Bader, Jeff King, Nguyen Thai Ngoc Duy, git
In-Reply-To: <4c8ef71001111307q6679039ajbef22f2e1748df56@mail.gmail.com>
On Mon, 11 Jan 2010, Fredrik Kuivinen wrote:
>
> Another option is to use memmem instead. As we know the length of the
> buffer already it should be a slight improvement over strstr for
> everyone. memmem may cause some portability problems though as it is a
> GNU extension.
I'd almost prefer to just drop the strstr entirely.
It's not actually all *that* big a win, even on my machine. I get
- strstr:
real 0m0.309s
user 0m0.168s
sys 0m0.136s
- regexec:
real 0m0.410s
user 0m0.220s
sys 0m0.116s
so yeah, it's slower, but not by a huge degree. With strstr, "git grep"
actually beats the external grep for me, but I don't really care. It's
already way better than it used to be - and clearly strstr has a lot of
potential problems.
Sure memmem() might be better for you than strstr, but on the other hand,
it might easily be worse than strstr for others - and not just from a
portability standpoint. Is memmem() optimized to take advantage of SSE4.2?
I suspect it is not, exactly _because_ it's a GNU extension, so Intel
hasn't published optimized sample code for people to use.
So I would argue against even bothering to try memmem. Especially since in
your case, regexec() is apparently faster than memmem _anyway_. I expect
that it is for me too, but I'm too lazy to check.
Linus
^ permalink raw reply
* [PATCH] push: spell 'Note about fast-forwards' section name correctly in error message.
From: Matthieu Moy @ 2010-01-11 21:09 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
The error message in case of non-fast forward points to 'git push
--help', but used to talk about a section 'non-fast-forward', while the
actual section name is 'Note about fast-forwards'.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
My bad, I'm the original author of the patch introducing this ;-).
builtin-push.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin-push.c b/builtin-push.c
index f7661d2..28a26e7 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -125,8 +125,8 @@ static int push_with_options(struct transport *transport, int flags)
if (nonfastforward && advice_push_nonfastforward) {
printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
- "Merge the remote changes before pushing again. See the 'non-fast-forward'\n"
- "section of 'git push --help' for details.\n");
+ "Merge the remote changes before pushing again. See the 'Note about\n"
+ "fast-forwards' section of 'git push --help' for details.\n");
}
return 1;
--
1.6.6.198.gbaea2
^ permalink raw reply related
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Fredrik Kuivinen @ 2010-01-11 21:07 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Miles Bader, Jeff King, Nguyen Thai Ngoc Duy, git
In-Reply-To: <alpine.LFD.2.00.1001111159270.17145@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
On Mon, Jan 11, 2010 at 21:07, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Mon, 11 Jan 2010, Fredrik Kuivinen wrote:
>> >
>> > Try a complex pattern ("qwerty.*as" finds the same line), and see if that
>> > too is slower than before. If that is faster than it used to be (with
>> > --no-ext-grep, of course), then it's strstr() that is badly implemented.
>>
>> Ah, yes, that's it. With the pattern "qwerty.*as" I get 2.5s with the
>> patch and 6s without.
>
> Ok, so on your machine, regcomp() is basically twice as fast as strstr().
Yes.
> Which is not entirely unexpected: I was actually surprised by strstr()
> being apparently so good on my machine. I do not generally expect things
> like that to be at all optimized for bigger working sets. Most common uses
> of strstr() are in short strings - not "strings" that are many kilobytes
> in size (the whole file).
>
> In fact, I suspect it works so well for me because in my version of glibc
> it's not just SSE-optimized: judging by the naming it's SSE4.2 optimized -
> so the case I see on my machine will _only_ happen on Nehalem-based cores
> (ie the new "Core i[357]" cpu's).
>
> It is entirely possible that strstr in general is a disaster.
Another option is to use memmem instead. As we know the length of the
buffer already it should be a slight improvement over strstr for
everyone. memmem may cause some portability problems though as it is a
GNU extension.
I get these results: (git-grep --no-ext-grep qwerty, best of five)
Junio's patch: 0:04.84
memmem (attached patch on top of Junio's): 0:02.91
regcomp/regexec (I changed is_fixed to always return 0, also on top of
Junio's): 0:02.02
- Fredrik
[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 1236 bytes --]
diff --git a/grep.c b/grep.c
index 940e200..d34247f 100644
--- a/grep.c
+++ b/grep.c
@@ -264,13 +264,14 @@ static void show_name(struct grep_opt *opt, const char *name)
}
-static int fixmatch(const char *pattern, char *line, int ignore_case, regmatch_t *match)
+static int fixmatch(const char *pattern, char *line, char *eol,
+ int ignore_case, regmatch_t *match)
{
char *hit;
if (ignore_case)
hit = strcasestr(line, pattern);
else
- hit = strstr(line, pattern);
+ hit = memmem(line, eol - line, pattern, strlen(pattern));
if (!hit) {
match->rm_so = match->rm_eo = -1;
@@ -333,7 +334,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
again:
if (p->fixed)
- hit = !fixmatch(p->pattern, bol, p->ignore_case, pmatch);
+ hit = !fixmatch(p->pattern, bol, eol, p->ignore_case, pmatch);
else
hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);
@@ -646,7 +647,7 @@ static int look_ahead(struct grep_opt *opt,
regmatch_t m;
if (p->fixed)
- hit = !fixmatch(p->pattern, bol, p->ignore_case, &m);
+ hit = !fixmatch(p->pattern, bol, bol + *left_p, p->ignore_case, &m);
else
hit = !regexec(&p->regexp, bol, 1, &m, 0);
if (!hit || m.rm_so < 0 || m.rm_eo < 0)
^ permalink raw reply related
* Re: [PATCH 9/9] rerere forget path: forget recorded resolution
From: Johannes Sixt @ 2010-01-11 21:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3a2cif04.fsf@alter.siamese.dyndns.org>
On Montag, 11. Januar 2010, Junio C Hamano wrote:
> Actually it is _very_ easy to fool rerere to do something totally
> unexpected, and I have been thinking about using the similarity comparison
> algorithm on the region outside the conflicted area between preimage and
> thisimage and reject use of rerere.
>
> Try this in an empty directory.
>[snip]
> Now, immediately after this sequence, rerere will give you an disaster.
Indeed. The problem here is that two entirely different resolutions are
recorded for the same conflict hash *in one run of rerere*. The damage can be
avoided if conflict hashes are not reused in do_plain_rerere (in the first
loop). (Though, I'm currently not in the mood to look into this in more
depth.)
Of course, this does not mean that *both* conflicts can be resolved
automatically when the merge is repeated. In my use-case this would have been
desirable (and even your example would suggest it is, but that is not
generally true).
-- Hannes
^ permalink raw reply
* Re: How to check new commit availability without full fetch?
From: Nicolas Pitre @ 2010-01-11 20:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin Rosenberg, SLONIK.AZ, Git Mailing List
In-Reply-To: <7vmy0kjvms.fsf@alter.siamese.dyndns.org>
On Mon, 11 Jan 2010, Junio C Hamano wrote:
> Ahh, you are right. It walks from objects the remote side told us are at
> the tip, and stops at what we know are complete (i.e. reachable from our
> tip of objects); immediately after --dry-run slurped objects, the next
> fetch will prove everything is locally available and complete before going
> over the network.
>
> But either I am very confused or the use of fields from "struct ref" is
> unintuitive in this codepath.
>
> Why does it feed ref->old_sha1? We are feeding _their_ tip commits to:
>
> rev-list --objects --stdin --not --all
>
> and expecting it to report failure when some of their tip commits lead to
> what we don't have yet. The reason why we have old_sha1[] vs new_sha1[]
> is because we want to report what changed from what, and also to protect
> us from simultaneous updates by doing compare-and-swap using the value we
> read from our refs when we started in old_sha1[], so I would have expected
> that ref_map elements would have _their_ commits on the new_sha1[] side,
> but apparently that is not what is happening, and it has been this way for
> a long time. The use of old_sha1[] came from 4191c35 (git-fetch: avoid
> local fetching from alternate (again), 2007-11-11), so it is a lot more
> likely that I am confused than the code is wrong and nobody noticed so
> far.
Very confusing indeed. I first discovered about quickfetch() myself
when I fixed shallow clone leading to commit 86386829.
If old_sha1[] was our refs then quickfetch() would always succeed and
we'd never fetch anything.
> What am I missing?
Digging a bit, it looks like get_remote_heads() is storing the remote's
heads into old_sha1. And so is performed in get_refs_from_bundle(), and
in insert_packed_refs() from get_refs_via_rsync(), etc.
Looking at the struct ref definition, we can see:
struct ref {
struct ref *next;
unsigned char old_sha1[20];
unsigned char new_sha1[20];
[...]
struct ref *peer_ref; /* when renaming */
[...]
};
And apparently store_updated_refs() ends up using that peer_ref like
this:
for (rm = ref_map; rm; rm = rm->next) {
struct ref *ref = NULL;
if (rm->peer_ref) {
ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
strcpy(ref->name, rm->peer_ref->name);
hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
hashcpy(ref->new_sha1, rm->old_sha1);
ref->force = rm->peer_ref->force;
}
So.... Doesn't this all look like a total mess of needless (and even
leaked in this case) allocations and duplications, besides being
completely unintuitive? Both hashcpy() above certainly throw my sense
of logic aside...
Nicolas
^ 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