* Re: [PATCH/RFC] bash: add --word-diff option to diff auto-completion
From: SZEDER Gábor @ 2011-09-13 23:37 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Rodrigo Rosenfeld Rosas, Thomas Rast, git
In-Reply-To: <20110913232941.GC2078@goldbirke>
On Wed, Sep 14, 2011 at 01:29:41AM +0200, SZEDER Gábor wrote:
> Or is it just too late here and I'm missing something
> obvious?
>
> Completing the mode for --word-diff=<TAB> is a good idea, but c'mon,
> there are plenty of examples ;) Have a look at _git_am(),
> _git_format_patch(), or _git_init() for something easy, and
> _git_commit(), _git_log(), or _git_notes() for something fancy.
>
> Note that --word-diff= is also valid for log and shortlog, so the same
> can be done there, too.
Not shortlog, show. It's definitely too late... ;)
^ permalink raw reply
* Re: [PATCH/RFC] bash: add --word-diff option to diff auto-completion
From: SZEDER Gábor @ 2011-09-13 23:29 UTC (permalink / raw)
To: Jonathan Nieder
Cc: SZEDER Gábor, Rodrigo Rosenfeld Rosas, Thomas Rast, git
In-Reply-To: <20110913191448.GC14917@elie>
Hi,
On Tue, Sep 13, 2011 at 02:14:48PM -0500, Jonathan Nieder wrote:
> From: Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com>
> Date: Tue, 13 Sep 2011 15:24:38 -0300
>
> Add "--word-diff" to diff completion, since this is a common
> desired option when looking at diffs.
>
> Signed-off-by: Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com>
> ---
> Hi Gábor,
>
> Here's a patch. What do you think?
Looks obviously good to me, ...
> I was thinking it would be nice to complete --word-diff-regex, too,
> and to be able to do
>
> git diff --color-words=<TAB>
> git diff --word-diff=<TAB>
... but yeah, there is room for while-at-its ;)
The completion script currently only offers --color-words but not
--color-words=. This is sort of OK, because --color-words' parameters
are optional. However, in several cases the completion script offers
both --option and --option= to indicate that it takes an optional
parameter, see e.g.
diff --dirstat --dirstat-by-file
commit --untracked-files
format-patch --thread
init --shared
log --decorate
(But we don't do this in all such cases, see e.g. diff --stat --color
or log --branches --tags --remotes.)
So I think it's fine to offer both --color-words and --color-words=,
and both --word-diff and --word-diff=.
> but I couldn't find any examples of the latter to crib from
I'm not sure what you mean by git diff --color-words=<TAB>, because it
takes a regexp. Or is it just too late here and I'm missing something
obvious?
Completing the mode for --word-diff=<TAB> is a good idea, but c'mon,
there are plenty of examples ;) Have a look at _git_am(),
_git_format_patch(), or _git_init() for something easy, and
_git_commit(), _git_log(), or _git_notes() for something fancy.
Note that --word-diff= is also valid for log and shortlog, so the same
can be done there, too.
Best,
Gábor
^ permalink raw reply
* Re: [Survey] Signed push
From: Guenter Roeck @ 2011-09-13 23:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaaa8xufi.fsf@alter.siamese.dyndns.org>
On Tue, Sep 13, 2011 at 12:45:37PM -0400, Junio C Hamano wrote:
[ ... ]
> 1. Improved pull requests.
>
noise for me
[ ... ]
> 2. Signed pushes.
>
Excellent idea, long since overdue.
Guenter
^ permalink raw reply
* Re: [PATCH 2/2] obstack.c: Fix some sparse warnings
From: Ramsay Jones @ 2011-09-13 23:18 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <CAGdFq_ifU2WWbCpRY_EFY=_hwwtFs0eqMhJ7sRoUhrivABFoFw@mail.gmail.com>
Sverre Rabbelier wrote:
> Heya,
>
> On Sun, Sep 11, 2011 at 21:26, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
>> compat/obstack.c:399:1: error: symbol 'print_and_abort' redeclared with \
>> different type (originally declared at compat/obstack.c:95) \
>> - different modifiers
>
>> @@ -395,7 +395,6 @@ _obstack_memory_used (struct obstack *h)
>> # endif
>>
>> static void
>> -__attribute__ ((noreturn))
>> print_and_abort (void)
>> {
>> /* Don't change any of these strings. Yes, it would be possible to add
>
> Wouldn't the better solution be to add noreturn to the declaration at
> compat/obstack.c:95?
Hmm, well ... maybe; it is at least debatable. But I decided no! :-D
First, although I would not dismiss the possibility of some optimization
of the code of print_and_abort() (the *callee*), the main benefit of the
noreturn attribute should in fact be at the call sites (ie the *caller*).
So, yes, in general, the declaration of the function should have the
noreturn attribute applied, in addition to the definition, in order to
allow the compiler to apply some optimizations to the call sites.
[Note, also, that we should use the NORETURN and NORETURN_PTR macros.]
In this case, however, there are no (direct) call sites. This function
would only be called indirectly via the 'obstack_alloc_failed_handler'
function pointer. So, this would require the use of NORETURN_PTR on
that function pointer. In order to keep both the compiler(s) and sparse
happy, the required change would look like the diff given at the end
of this mail.
This would work fine, and I would happily change the patch to include
this if it is deemed the better approach. However, I looked at the
call sites in _obstack_begin[_1](), and _obstack_newchunck() and could
not see any great opportunity for optimizing the code ... so I decided
to go for the simpler patch ...
ATB,
Ramsay Jones
-- >8 --
diff --git a/compat/obstack.c b/compat/obstack.c
index a89ab5b..2029b8f 100644
--- a/compat/obstack.c
+++ b/compat/obstack.c
@@ -92,8 +92,8 @@ enum
abort gracefully or use longjump - but shouldn't return. This
variable by default points to the internal function
`print_and_abort'. */
-static void print_and_abort (void);
-void (*obstack_alloc_failed_handler) (void) = print_and_abort;
+static void NORETURN print_and_abort (void);
+NORETURN_PTR void (*obstack_alloc_failed_handler) (void) = print_and_abort;
# ifdef _LIBC
# if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3_4)
@@ -395,7 +395,7 @@ _obstack_memory_used (struct obstack *h)
# endif
static void
-__attribute__ ((noreturn))
+NORETURN
print_and_abort (void)
{
/* Don't change any of these strings. Yes, it would be possible to add
diff --git a/compat/obstack.h b/compat/obstack.h
index d178bd6..122f93f 100644
--- a/compat/obstack.h
+++ b/compat/obstack.h
@@ -194,7 +194,7 @@ void obstack_free (struct obstack *, void *);
more memory. This can be set to a user defined function which
should either abort gracefully or use longjump - but shouldn't
return. The default action is to print a message and abort. */
-extern void (*obstack_alloc_failed_handler) (void);
+extern NORETURN_PTR void (*obstack_alloc_failed_handler) (void);
\f
/* Pointer to beginning of object being allocated or to be allocated next.
Note that this might not be the final address of the object
^ permalink raw reply related
* Re: [PATCH 1/3] make-static: master
From: Ramsay Jones @ 2011-09-13 22:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In-Reply-To: <7vhb4in4j7.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Many symbols that are exported to the global scope do not have to be.
>
> Signed-off-by: Junio C Hamano <junio@pobox.com>
> ---
> * To be applied on top of 3793ac5 (RelNotes/1.7.7: minor fixes, 2011-09-07)
>
[snipped patch]
commit f34196da7b55cbf9f2651e095b6559430aff0baf (make-static: master, 11-09-2011)
in the next branch (at repo.or.cz), but *not* this patch, breaks the build on
cygwin.
The failure is caused by this part of the commit:
diff --git a/environment.c b/environment.c
index e96edcf..478f2afa 100644
--- a/environment.c
+++ b/environment.c
@@ -147,11 +147,6 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !get_git_work_tree();
}
-int have_git_dir(void)
-{
- return !!git_dir;
-}
-
const char *get_git_dir(void)
{
if (!git_dir)
since have_git_dir() is used in compat/cygwin.c (line 117).
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 2/3] Fix some "variable might be used uninitialized" warnings
From: Ramsay Jones @ 2011-09-13 22:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
In-Reply-To: <7vpqj6olfa.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>
>> In particular, gcc complains as follows:
>>
>> CC tree-walk.o
>> tree-walk.c: In function `traverse_trees':
>> tree-walk.c:347: warning: 'e' might be used uninitialized in this \
>> function
>>
>> CC builtin/revert.o
>> builtin/revert.c: In function `verify_opt_mutually_compatible':
>> builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
>> this function
>
> Could you also add something to this effect to the commit log message:
>
> but I have verified that these are gcc being not careful
> enough and they are never used uninitialized.
see below for the v2 patch.
> If that is what you indeed have done, that is.
Indeed. The builtin/revert.c warning is straight-forward, but the tree-walk.c
warning is somewhat less so! ;-)
Imagine traverse_trees() (tree-walk.c:324) was called with n == 0 (let's ignore
the effective calls to xmalloc(0) and xcalloc(0,..) at the start of that function).
At first blush it looked like 'e' would remain uninitialized in the call to
prune_traversal() at line 403. Indeed it *would* be if you ever got to that line.
However, since the 'mask' variable (set at line 391) remains set to zero at line 401,
the flow of control leaves the loop before 'e' is used.
[I don't think traverse_trees() would ever be called with n == 0 anyway; the call
site in builtin/merge-tree.c is called with the constant 3, and the call-chains(s)
which start from unpack_trees() are protected by "if (len)", where 'len' is unsigned.]
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH v2 2/3] Fix some "variable might be used uninitialized" warnings
In particular, gcc complains as follows:
CC tree-walk.o
tree-walk.c: In function `traverse_trees':
tree-walk.c:347: warning: 'e' might be used uninitialized in this \
function
CC builtin/revert.o
builtin/revert.c: In function `verify_opt_mutually_compatible':
builtin/revert.c:113: warning: 'opt2' might be used uninitialized in \
this function
However, I have verified that the analysis performed by gcc was too
conservative and that these variables are not, in fact, used while
uninitialized.
In order to suppress the warnings, we add an NULL pointer initializer
to the declarations of the 'e' and 'opt2' variables.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
builtin/revert.c | 2 +-
tree-walk.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index ba27cf1..200149e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -110,7 +110,7 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
static void verify_opt_mutually_compatible(const char *me, ...)
{
- const char *opt1, *opt2;
+ const char *opt1, *opt2 = NULL;
va_list ap;
va_start(ap, me);
diff --git a/tree-walk.c b/tree-walk.c
index 808bb55..a8d8a66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -344,7 +344,7 @@ int traverse_trees(int n, struct tree_desc *t, struct traverse_info *info)
unsigned long mask, dirmask;
const char *first = NULL;
int first_len = 0;
- struct name_entry *e;
+ struct name_entry *e = NULL;
int len;
for (i = 0; i < n; i++) {
--
1.7.6
^ permalink raw reply related
* Re: [PATCH v4] gitk: Allow commit editing
From: Michal Sojka @ 2011-09-13 23:11 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Jeff King, git
In-Reply-To: <20110908205945.GB8091@bloggs.ozlabs.ibm.com>
On Thu, 08 Sep 2011, Paul Mackerras wrote:
> On Sat, Aug 27, 2011 at 02:31:02PM +0200, Michal Sojka wrote:
> > Here is a new version with the micro-optimization.
> >
> > Another minor change is that this patch now applies to gitk repo
> > (http://git.kernel.org/pub/scm/gitk/gitk.git) instead of the main git
> > repo.
> >
> > -Michal
> >
> > --8<---------------cut here---------------start------------->8---
> > I often use gitk to review patches before pushing them out and I would
> > like to have an easy way to edit commits. The current approach with
> > copying the commitid, switching to terminal, invoking git rebase -i,
> > editing the commit and switching back to gitk is a way too complicated
> > just for changing a single letter in the commit message or removing a
> > debug printf().
> >
> > This patch adds "Edit this commit" item to gitk's context menu which
> > invokes interactive rebase in a non-interactive way :-). git gui is
> > used to actually edit the commit.
> >
> > Besides editing the commit message, splitting of commits, as described
> > in git-rebase(1), is also supported.
> >
> > The user is warned if the commit to be edited is contained in another
> > ref besides the current branch and the stash (e.g. in a remote
> > branch). Additionally, error box is displayed if the user attempts to
> > edit a commit not contained in the current branch.
>
> I have to say that this patch makes me pretty nervous. I can see the
> attractiveness of the feature, but I don't like making gitk
> unresponsive for a potentially long time, i.e. until git gui exits.
> It may not be clear to users that the reason gitk isn't responding is
> because some other git gui window is still running.
I understand this. See below for a possible solution.
> Also, if some subsequent commit no longer applies because of the
> changes you make to a commit, it's going to abort the rebase
> completely and thus lose the changes you made. That could be
> annoying.
Agreed. A solution could be to create a ref called for example
refs/gitk/failed-rebase and then abort the rebase together with
displaying the error message explaining what happened. The edited commit
would remain visible and a user can manually cherry pick remaining
commits and then reset the original branch to this failed-rebase ref.
> I usually do this by starting a new branch just before the commit I
> want to change and then use a combination of the cherry-pick menu item
> and git commit --amend. Maybe something to make that simpler for
> users would be good, i.e. automate it a bit but still have it be a
> step-by-step process if necessary. Part of the problem of course is
> that neither gitk nor git gui are really designed to be an editing
> environment.
So what about the following:
1) When user selects "Edit commit", git rebase -i is called, git citool
is started on background and things will be set up (I do not know yet
how exactly) so that same callback is called when git citool exits.
2) gitk updates the list of commits and marks the current (detached)
head by a tag (colored box) saying that there is rebase in progress.
The same tag would appear if gitk is executed manually during
interactive rebase.
3) When git citool exits (or when you explicitly select something from
rebase tag's context menu), rebase would continue.
> In fact you really want an edit/compile/test environment so you don't
> introduce new bugs.
Another possibility would be to allow only editing of the commit
message. Then you don't need compile/test steps and the rebase should
not fail due to conflicts.
I do not think I would like this limitation, but it would certainly be
better then nothing.
-Michal
^ permalink raw reply
* Re: git checkout --orphan skips reflogging
From: Junio C Hamano @ 2011-09-13 23:04 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: Git List
In-Reply-To: <CA+gfSn-tVgj=FYiVGK7kmH4gpnXF3HUbs+f=DfRey6GrpadVYg@mail.gmail.com>
Dmitry Ivankov <divanorama@gmail.com> writes:
> In short, git checkout --orphan doesn't write
> HEAD_sha1 -> 00000
> entry to logs/HEAD, while git-comit will write
> 00000 -> new_orphan_HEAD_sha1
> entry. And then reflog backward walk will stop on 000 -> entry and
> won't see earlier history.
Funny. From the point of view of the _current_ branch, it sort of makes
sense to stop the traversal at that point, but I agree for HEAD reflog
that records branch switching, the traversal should not stop.
I am not sure if recording 0{40} after --orphan is the right thing to do
either (for that matter, I do not necessarily think running --orphan is a
sane thing to do, but that is a separate issue).
> Isn't it also a bug in reflog walking that we rely on each old_sha1
> being new_sha1 of a previous entry?
I am not all that familiar with the reflog walking (which is admittedly a
bolted-on hack that injects commits with fake ancestry) code, but I think
it assumes the old sha1 field on the current entry matches the new sha1
field on the previous entry, and we could change it to be a bit more
robust.
The attached patch _may_ (I didn't even compile test it) remove the
dependency on osha1[] and make the code consistently use nsha1[], but I
think stopping at the 0{40} is pretty much fundamental in the revision
walking machinery the reflog walking code is piggy-backing on, and I do
not think this patch would change that.
reflog-walk.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/reflog-walk.c b/reflog-walk.c
index 5d81d39..261d300 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -211,6 +211,13 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
return 0;
}
+static struct reflog_info *peek_reflog_ent(struct commit_reflog *clog)
+{
+ if (clog->recno < 0)
+ return NULL;
+ return &clog->reflogs->items[clog->recno];
+}
+
void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
{
struct commit_info *commit_info =
@@ -223,20 +230,20 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
return;
commit_reflog = commit_info->util;
- if (commit_reflog->recno < 0) {
+ reflog = peek_reflog_ent(commit_reflog);
+ if (!reflog) {
commit->parents = NULL;
return;
}
-
- reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
info->last_commit_reflog = commit_reflog;
commit_reflog->recno--;
- commit_info->commit = (struct commit *)parse_object(reflog->osha1);
- if (!commit_info->commit) {
+ reflog = peek_reflog_ent(commit_reflog);
+ if (!reflog) {
commit->parents = NULL;
return;
}
+ commit_info->commit = (struct commit *)parse_object(reflog->nsha1);
commit->parents = xcalloc(sizeof(struct commit_list), 1);
commit->parents->item = commit_info->commit;
}
^ permalink raw reply related
* [PATCH v2 2/2] request-pull: state exact commit object name
From: Junio C Hamano @ 2011-09-13 22:28 UTC (permalink / raw)
To: git
In-Reply-To: <1315952896-17258-1-git-send-email-gitster@pobox.com>
A typical pull-request begins like this:
The following changes since commit f696543dad6c7ba27b0c4fab167a5687263a9ba0:
Flobar 2.4.3 (2011-09-13 12:34:56 +0900)
are available in the git repository at:
git://git.kernel.org/pub/flobar.git/ master
which is followed by the shortlog and expected diffstat. This tells you
where the requester based his work on in excruciating detail, but does not
tell you what you should expect to fetch, any more than "whatever happened
to be at the named branch when you happened to notice the request."
Update the message slightly to say:
git://git.kernel.org/pub/flobar.git/ 5738c9c21e53356ab5020912116e7f82fd2d428f ;# master
so that the line still can be cut&pasted after "git fetch" (or "git
pull"), to form a command line that looks like:
$ git <repository> <full commit object name> ;# branch
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-request-pull.sh | 2 +-
t/t5150-request-pull.sh | 11 +++++++----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/git-request-pull.sh b/git-request-pull.sh
index fc080cc..b5a2d0f 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -70,7 +70,7 @@ git show -s --format='The following changes since commit %H:
%s (%ci)
are available in the git repository at:' $baserev &&
-echo " $url $branch" &&
+echo " $url $headrev ;# $branch" &&
echo &&
git shortlog ^$baserev $headrev &&
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 9cc0a42..e9d657e 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -70,9 +70,10 @@ test_expect_success 'setup: two scripts for reading pull requests' '
/ in the git repository at:$/!d
n
/^$/ n
- s/^[ ]*\(.*\) \([^ ]*\)/please pull\
+ s/^[ ]*\(.*\) \([^ ]*\) ;# \([^ ]*\)/please pull\
\1\
- \2/p
+ \2\
+ \3/p
q
EOT
@@ -145,6 +146,7 @@ test_expect_success 'pull request after push' '
{
read task &&
read repository &&
+ read head &&
read branch
} <digest &&
(
@@ -153,6 +155,7 @@ test_expect_success 'pull request after push' '
git pull --ff-only "$repository" "$branch"
) &&
test "$branch" = for-upstream &&
+ test "$head" = "$(GIT_DIR=downstream.git git rev-parse for-upstream)" &&
test_cmp local/mnemonic.txt upstream-private/mnemonic.txt
'
@@ -170,10 +173,10 @@ test_expect_success 'request names an appropriate branch' '
git request-pull initial "$downstream_url" >../request
) &&
sed -nf read-request.sed <request >digest &&
- cat digest &&
{
read task &&
read repository &&
+ read head &&
read branch
} <digest &&
{
@@ -193,7 +196,7 @@ test_expect_success 'pull request format' '
SUBJECT (DATE)
are available in the git repository at:
- URL BRANCH
+ URL OBJECT_NAME ;# BRANCH
SHORTLOG
--
1.7.7.rc1.1.g1e5814
^ permalink raw reply related
* [PATCH v2 0/2] State commit name explicitly in request-pull messages
From: Junio C Hamano @ 2011-09-13 22:28 UTC (permalink / raw)
To: git
In-Reply-To: <7vaaa8xufi.fsf@alter.siamese.dyndns.org>
Here is an alternative approach to the earlier "request-pull" patch.
Junio C Hamano (2):
fetch: allow asking for an explicit commit object by name
request-pull: state exact commit object name
git-request-pull.sh | 2 +-
remote.c | 25 +++++++++++++++++++++++--
t/t5150-request-pull.sh | 11 +++++++----
3 files changed, 31 insertions(+), 7 deletions(-)
--
1.7.7.rc1.1.g1e5814
^ permalink raw reply
* [PATCH v2 1/2] fetch: allow asking for an explicit commit object by name
From: Junio C Hamano @ 2011-09-13 22:28 UTC (permalink / raw)
To: git
In-Reply-To: <1315952896-17258-1-git-send-email-gitster@pobox.com>
This teaches "git fetch" (hence "git pull") to accept an explicit commit
object name in the LHS of the refspec, as long as the named commit is at
the tip of an advertised ref. E.g.
$ git pull origin 5738c9c21e53356ab5020912116e7f82fd2d428f
$ git fetch origin 5738c9c21e53356ab5020912116e7f82fd2d428f:refs/remotes/origin
would behave exactly as if you asked
$ git pull origin refs/heads/master
$ git fetch origin refs/heads/master:refs/remotes/origin
when the output from "git ls-remote origin" said the remote side has the
commit object whose name is 5738c9c21e53356ab5020912116e7f82fd2d428f at
the tip of refs/heads/master branch ref.
This does not allow asking for a random object that may or may not exist
in the repository (this has been a longstanding security feature).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/remote.c b/remote.c
index ca42a12..76c2943 100644
--- a/remote.c
+++ b/remote.c
@@ -1387,6 +1387,25 @@ struct ref *get_remote_ref(const struct ref *remote_refs, const char *name)
return copy_ref(ref);
}
+/*
+ * Allow fetching an explicitly-named commit from the command line,
+ * but only if it exactly matches the commit at the tip of one of the
+ * advertised refs.
+ */
+static struct ref *get_remote_commit(const struct ref *remote_refs, const char *hex)
+{
+ const struct ref *ref;
+ unsigned char sha1[20];
+
+ if (get_sha1_hex(hex, sha1) || hex[40])
+ return NULL;
+
+ for (ref = remote_refs; ref; ref = ref->next)
+ if (!strchr(ref->name, '^') && !hashcmp(sha1, ref->old_sha1))
+ return copy_ref(ref);
+ return NULL;
+}
+
static struct ref *get_local_ref(const char *name)
{
if (!name || name[0] == '\0')
@@ -1416,8 +1435,10 @@ int get_fetch_map(const struct ref *remote_refs,
const char *name = refspec->src[0] ? refspec->src : "HEAD";
ref_map = get_remote_ref(remote_refs, name);
- if (!missing_ok && !ref_map)
- die("Couldn't find remote ref %s", name);
+ if (!ref_map)
+ ref_map = get_remote_commit(remote_refs, name);
+ if (!ref_map && !missing_ok)
+ die("Couldn't find remote ref that matches %s", name);
if (ref_map) {
ref_map->peer_ref = get_local_ref(refspec->dst);
if (ref_map->peer_ref && refspec->force)
--
1.7.7.rc1.1.g1e5814
^ permalink raw reply related
* Re: [PATCH] fetch: avoid quadratic loop checking for updated submodules
From: Jeff King @ 2011-09-13 22:17 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Junio C Hamano, git
In-Reply-To: <4E6FAB46.30508@web.de>
On Tue, Sep 13, 2011 at 09:13:10PM +0200, Jens Lehmann wrote:
> The real world use case I have for that is that when a bug introduced by
> a new submodule commit is detected later on, the superproject's fix
> records an older submodule commit to remove the problematic change from
> the superproject. But the submodule's branch isn't rewound (as that is
> most probably master) but a fix is applied on top of it. Then that one
> gets tested and - if it was found ok - recorded in the superproject.
OK, that makes sense. It is a "rewind" from the perspective of the
superproject, but there is never a fork; because the submodule didn't
rewind, when we do get a new submodule state in the superproject, it
will be a fast forward from the old state.
That is a reasonable use case.
It's still probably a minority case, and we have to pay for the full
traversal each time, which is annoying. But now that it's turned off by
default if you don't have any submodules, I'm less concerned about the
performance impact. And superproject repositories are probably not going
to have the same number of commits as submodule repositories, so it may
be less of an issue.
One thing that could make it slightly less expensive (but I wouldn't
worry about implementing until it becomes an issue): you do a full diff
and collect any changed GITLINK entries, and then compare the paths we
get with the submodule config. Couldn't you instead do something like
this:
- let S = set of submodule paths that we care about, from the config
- start the "git rev-list $new --not $old" traversal, as we do now
- for each commit
- diff using a pathspec of S
- for each changed entry
- add it to the set of changed submodules
- remove it from S
- if S is empty, break
That has two advantages:
1. We limit the diff by pathspec, which means we can avoid looking at
irrelevant parts of the tree (we don't do this yet, but hopefully
we will in the future).
2. You can break out of the traversal early if you already know you
have changes in each submodule of interest.
> Changes like this could be overlooked if you only compare "before" and
> "after" instead of traversing, leading to not fetching a submodule which
> does have new commits that are used in the newly fetched superproject's
> commits. I'd like to have on-demand fetch keep the correct solution of
> traversing unless we have good reasons against it, especially as teaching
> checkout & friends to recursively update submodules too depends on all
> needed commits being present.
Out of curiosity, what happens if we don't have such a commit? I know
you said that your policy is never to rewind a submodule's commit that
has been published in a superproject, and I think that's the only sane
thing to do. But it's not enforced by git itself, and I wonder how badly
we break if it does happen (i.e., I'm hoping the answer is "you can't
diff or checkout superproject revisions that reference the missing bit"
and not "git log crashes when it gets to that point in history").
-Peff
^ permalink raw reply
* [nit] diff func headers ignore context
From: Jeff King @ 2011-09-13 22:04 UTC (permalink / raw)
To: git
In-Reply-To: <20110913215824.GG24490@sigill.intra.peff.net>
On Tue, Sep 13, 2011 at 05:58:25PM -0400, Jeff King wrote:
> @@ -609,26 +610,23 @@ int finish_async(struct async *async)
> int run_hook(const char *index_file, const char *name, ...)
> {
> struct child_process hook;
> - const char **argv = NULL, *env[2];
> + struct argv_array argv = ARGV_ARRAY_INIT;
I find this diff function header pretty confusing. Of course we're not
in finish_async, as you can see by the fact that the context contains
the start of run_hook.
I don't think this is something that can be solved with xfuncname
config; we would have to teach xdiff to look at context lines when
picking a header line.
Am I the only one who finds this confusing? Can anyone think of a reason
to keep showing finish_async in this example?
-Peff
^ permalink raw reply
* [PATCH 6/7] checkout: use argv_array API
From: Jeff King @ 2011-09-13 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
We were using a similar ad-hoc rev_list_args structure, but
this saves some code.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/checkout.c | 27 ++++++++-------------------
1 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 28cdc51..bb056e4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -19,6 +19,7 @@
#include "ll-merge.h"
#include "resolve-undo.h"
#include "submodule.h"
+#include "argv-array.h"
static const char * const checkout_usage[] = {
"git checkout [options] <branch>",
@@ -588,24 +589,12 @@ static void update_refs_for_switch(struct checkout_opts *opts,
report_tracking(new);
}
-struct rev_list_args {
- int argc;
- int alloc;
- const char **argv;
-};
-
-static void add_one_rev_list_arg(struct rev_list_args *args, const char *s)
-{
- ALLOC_GROW(args->argv, args->argc + 1, args->alloc);
- args->argv[args->argc++] = s;
-}
-
static int add_one_ref_to_rev_list_arg(const char *refname,
const unsigned char *sha1,
int flags,
void *cb_data)
{
- add_one_rev_list_arg(cb_data, refname);
+ argv_array_push(cb_data, refname);
return 0;
}
@@ -684,15 +673,14 @@ static void suggest_reattach(struct commit *commit, struct rev_info *revs)
*/
static void orphaned_commit_warning(struct commit *commit)
{
- struct rev_list_args args = { 0, 0, NULL };
+ struct argv_array args = ARGV_ARRAY_INIT;
struct rev_info revs;
- add_one_rev_list_arg(&args, "(internal)");
- add_one_rev_list_arg(&args, sha1_to_hex(commit->object.sha1));
- add_one_rev_list_arg(&args, "--not");
+ argv_array_push(&args, "(internal)");
+ argv_array_push(&args, sha1_to_hex(commit->object.sha1));
+ argv_array_push(&args, "--not");
for_each_ref(add_one_ref_to_rev_list_arg, &args);
- add_one_rev_list_arg(&args, "--");
- add_one_rev_list_arg(&args, NULL);
+ argv_array_push(&args, "--");
init_revisions(&revs, NULL);
if (setup_revisions(args.argc - 1, args.argv, &revs, NULL) != 1)
@@ -704,6 +692,7 @@ static void orphaned_commit_warning(struct commit *commit)
else
describe_detached_head(_("Previous HEAD position was"), commit);
+ argv_array_clear(&args);
clear_commit_marks(commit, -1);
for_each_ref(clear_commit_marks_from_one_ref, NULL);
}
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 7/7] run_hook: use argv_array API
From: Jeff King @ 2011-09-13 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
This was a pretty straightforward use, so it really doesn't
save that many lines. Still, perhaps it's a little bit more
readable.
Signed-off-by: Jeff King <peff@peff.net>
---
run-command.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/run-command.c b/run-command.c
index 70e8a24..73e013e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "run-command.h"
#include "exec_cmd.h"
+#include "argv-array.h"
static inline void close_pair(int fd[2])
{
@@ -609,26 +610,23 @@ int finish_async(struct async *async)
int run_hook(const char *index_file, const char *name, ...)
{
struct child_process hook;
- const char **argv = NULL, *env[2];
+ struct argv_array argv = ARGV_ARRAY_INIT;
+ const char *p, *env[2];
char index[PATH_MAX];
va_list args;
int ret;
- size_t i = 0, alloc = 0;
if (access(git_path("hooks/%s", name), X_OK) < 0)
return 0;
va_start(args, name);
- ALLOC_GROW(argv, i + 1, alloc);
- argv[i++] = git_path("hooks/%s", name);
- while (argv[i-1]) {
- ALLOC_GROW(argv, i + 1, alloc);
- argv[i++] = va_arg(args, const char *);
- }
+ argv_array_push(&argv, git_path("hooks/%s", name));
+ while ((p = va_arg(args, const char *)))
+ argv_array_push(&argv, p);
va_end(args);
memset(&hook, 0, sizeof(hook));
- hook.argv = argv;
+ hook.argv = argv.argv;
hook.no_stdin = 1;
hook.stdout_to_stderr = 1;
if (index_file) {
@@ -639,6 +637,6 @@ int run_hook(const char *index_file, const char *name, ...)
}
ret = run_command(&hook);
- free(argv);
+ argv_array_clear(&argv);
return ret;
}
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 5/7] bisect: use argv_array API
From: Jeff King @ 2011-09-13 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
Now that the argv_array API exists, we can save some lines
of code.
Signed-off-by: Jeff King <peff@peff.net>
---
bisect.c | 48 +++++++++++-------------------------------------
1 files changed, 11 insertions(+), 37 deletions(-)
diff --git a/bisect.c b/bisect.c
index dd7e8ed..ef92871 100644
--- a/bisect.c
+++ b/bisect.c
@@ -10,18 +10,13 @@
#include "log-tree.h"
#include "bisect.h"
#include "sha1-array.h"
+#include "argv-array.h"
static struct sha1_array good_revs;
static struct sha1_array skipped_revs;
static const unsigned char *current_bad_sha1;
-struct argv_array {
- const char **argv;
- int argv_nr;
- int argv_alloc;
-};
-
static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL};
static const char *argv_show_branch[] = {"show-branch", NULL, NULL};
@@ -404,21 +399,6 @@ struct commit_list *find_bisection(struct commit_list *list,
return best;
}
-static void argv_array_push(struct argv_array *array, const char *string)
-{
- ALLOC_GROW(array->argv, array->argv_nr + 1, array->argv_alloc);
- array->argv[array->argv_nr++] = string;
-}
-
-static void argv_array_push_sha1(struct argv_array *array,
- const unsigned char *sha1,
- const char *format)
-{
- struct strbuf buf = STRBUF_INIT;
- strbuf_addf(&buf, format, sha1_to_hex(sha1));
- argv_array_push(array, strbuf_detach(&buf, NULL));
-}
-
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
@@ -448,16 +428,10 @@ static void read_bisect_paths(struct argv_array *array)
die_errno("Could not open file '%s'", filename);
while (strbuf_getline(&str, fp, '\n') != EOF) {
- char *quoted;
- int res;
-
strbuf_trim(&str);
- quoted = strbuf_detach(&str, NULL);
- res = sq_dequote_to_argv(quoted, &array->argv,
- &array->argv_nr, &array->argv_alloc);
- if (res)
+ if (sq_dequote_to_argv_array(str.buf, array))
die("Badly quoted content in file '%s': %s",
- filename, quoted);
+ filename, str.buf);
}
strbuf_release(&str);
@@ -622,7 +596,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,
const char *bad_format, const char *good_format,
int read_paths)
{
- struct argv_array rev_argv = { NULL, 0, 0 };
+ struct argv_array rev_argv = ARGV_ARRAY_INIT;
int i;
init_revisions(revs, prefix);
@@ -630,17 +604,17 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,
revs->commit_format = CMIT_FMT_UNSPECIFIED;
/* rev_argv.argv[0] will be ignored by setup_revisions */
- argv_array_push(&rev_argv, xstrdup("bisect_rev_setup"));
- argv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format);
+ argv_array_push(&rev_argv, "bisect_rev_setup");
+ argv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1));
for (i = 0; i < good_revs.nr; i++)
- argv_array_push_sha1(&rev_argv, good_revs.sha1[i],
- good_format);
- argv_array_push(&rev_argv, xstrdup("--"));
+ argv_array_pushf(&rev_argv, good_format,
+ sha1_to_hex(good_revs.sha1[i]));
+ argv_array_push(&rev_argv, "--");
if (read_paths)
read_bisect_paths(&rev_argv);
- argv_array_push(&rev_argv, NULL);
- setup_revisions(rev_argv.argv_nr, rev_argv.argv, revs, NULL);
+ setup_revisions(rev_argv.argc, rev_argv.argv, revs, NULL);
+ /* XXX leak rev_argv, as "revs" may still be pointing to it */
}
static void bisect_common(struct rev_info *revs)
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 4/7] quote: provide sq_dequote_to_argv_array
From: Jeff King @ 2011-09-13 21:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
This is similar to sq_dequote_to_argv, but more convenient
if you have an argv_array. It's tempting to just feed the
components of the argv_array to sq_dequote_to_argv instead,
but:
1. It wouldn't maintain the NULL-termination invariant
of argv_array.
2. It doesn't match the memory ownership policy of
argv_array (in which each component is free-able, not a
pointer into a separate buffer).
Signed-off-by: Jeff King <peff@peff.net>
---
quote.c | 23 ++++++++++++++++++++---
quote.h | 8 ++++++++
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/quote.c b/quote.c
index 63d3b01..87bc65e 100644
--- a/quote.c
+++ b/quote.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "quote.h"
+#include "argv-array.h"
int quote_path_fully = 1;
@@ -120,7 +121,9 @@ void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
return sq_dequote_step(arg, NULL);
}
-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)
+static int sq_dequote_to_argv_internal(char *arg,
+ const char ***argv, int *nr, int *alloc,
+ struct argv_array *array)
{
char *next = arg;
@@ -130,13 +133,27 @@ int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)
char *dequoted = sq_dequote_step(next, &next);
if (!dequoted)
return -1;
- ALLOC_GROW(*argv, *nr + 1, *alloc);
- (*argv)[(*nr)++] = dequoted;
+ if (argv) {
+ ALLOC_GROW(*argv, *nr + 1, *alloc);
+ (*argv)[(*nr)++] = dequoted;
+ }
+ if (array)
+ argv_array_push(array, dequoted);
} while (next);
return 0;
}
+int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)
+{
+ return sq_dequote_to_argv_internal(arg, argv, nr, alloc, NULL);
+}
+
+int sq_dequote_to_argv_array(char *arg, struct argv_array *array)
+{
+ return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);
+}
+
/* 1 means: quote as octal
* 0 means: quote as octal if (quote_path_fully)
* -1 means: never quote
diff --git a/quote.h b/quote.h
index 252b0df..133155a 100644
--- a/quote.h
+++ b/quote.h
@@ -45,6 +45,14 @@
*/
extern int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
+/*
+ * Same as above, but store the unquoted strings in an argv_array. We will
+ * still modify arg in place, but unlike sq_dequote_to_argv, the argv_array
+ * will duplicate and take ownership of the strings.
+ */
+struct argv_array;
+extern int sq_dequote_to_argv_array(char *arg, struct argv_array *);
+
extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);
extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);
extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 3/7] refactor argv_array into generic code
From: Jeff King @ 2011-09-13 21:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
The submodule code recently grew generic code to build a
dynamic argv array. Many other parts of the code can reuse
this, too, so let's make it generically available.
There are two enhancements not found in the original code:
1. We now handle the NULL-termination invariant properly,
even when no strings have been pushed (before, you
could have an empty, NULL argv). This was not a problem
for the submodule code, which always pushed at least
one argument, but was not sufficiently safe for
generic code.
2. There is a formatted variant of the "push" function.
This is a convenience function which was not needed by
the submodule code, but will make it easier to port
other users to the new code.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-argv-array.txt | 46 ++++++++++++++++++++++++
Makefile | 2 +
argv-array.c | 52 ++++++++++++++++++++++++++++
argv-array.h | 20 +++++++++++
submodule.c | 41 +++-------------------
5 files changed, 126 insertions(+), 35 deletions(-)
create mode 100644 Documentation/technical/api-argv-array.txt
create mode 100644 argv-array.c
create mode 100644 argv-array.h
diff --git a/Documentation/technical/api-argv-array.txt b/Documentation/technical/api-argv-array.txt
new file mode 100644
index 0000000..49b3d52
--- /dev/null
+++ b/Documentation/technical/api-argv-array.txt
@@ -0,0 +1,46 @@
+argv-array API
+==============
+
+The argv-array API allows one to dynamically build and store
+NULL-terminated lists. An argv-array maintains the invariant that the
+`argv` member always points to a non-NULL array, and that the array is
+always NULL-terminated at the element pointed to by `argv[argc]`. This
+makes the result suitable for passing to functions expecting to receive
+argv from main(), or the link:api-run-command.html[run-command API].
+
+The link:api-string-list.html[string-list API] is similar, but cannot be
+used for these purposes; instead of storing a straight string pointer,
+it contains an item structure with a `util` field that is not compatible
+with the traditional argv interface.
+
+Each `argv_array` manages its own memory. Any strings pushed into the
+array are duplicated, and all memory is freed by argv_array_clear().
+
+Data Structures
+---------------
+
+`struct argv_array`::
+
+ A single array. This should be initialized by assignment from
+ `ARGV_ARRAY_INIT`, or by calling `argv_array_init`. The `argv`
+ member contains the actual array; the `argc` member contains the
+ number of elements in the array, not including the terminating
+ NULL.
+
+Functions
+---------
+
+`argv_array_init`::
+ Initialize an array. This is no different than assigning from
+ `ARGV_ARRAY_INIT`.
+
+`argv_array_push`::
+ Push a copy of a string onto the end of the array.
+
+`argv_array_pushf`::
+ Format a string and push it onto the end of the array. This is a
+ convenience wrapper combining `strbuf_addf` and `argv_array_push`.
+
+`argv_array_clear`::
+ Free all memory associated with the array and return it to the
+ initial, empty state.
diff --git a/Makefile b/Makefile
index e40ac0c..ec7d06c 100644
--- a/Makefile
+++ b/Makefile
@@ -497,6 +497,7 @@ VCSSVN_LIB=vcs-svn/lib.a
LIB_H += advice.h
LIB_H += archive.h
+LIB_H += argv-array.h
LIB_H += attr.h
LIB_H += blob.h
LIB_H += builtin.h
@@ -575,6 +576,7 @@ LIB_OBJS += alloc.o
LIB_OBJS += archive.o
LIB_OBJS += archive-tar.o
LIB_OBJS += archive-zip.o
+LIB_OBJS += argv-array.o
LIB_OBJS += attr.o
LIB_OBJS += base85.o
LIB_OBJS += bisect.o
diff --git a/argv-array.c b/argv-array.c
new file mode 100644
index 0000000..a50507a
--- /dev/null
+++ b/argv-array.c
@@ -0,0 +1,52 @@
+#include "cache.h"
+#include "argv-array.h"
+#include "strbuf.h"
+
+static const char *empty_argv_storage = NULL;
+const char **empty_argv = &empty_argv_storage;
+
+void argv_array_init(struct argv_array *array)
+{
+ array->argv = empty_argv;
+ array->argc = 0;
+ array->alloc = 0;
+}
+
+static void argv_array_push_nodup(struct argv_array *array, const char *value)
+{
+ if (array->argv == empty_argv)
+ array->argv = NULL;
+
+ ALLOC_GROW(array->argv, array->argc + 2, array->alloc);
+ array->argv[array->argc++] = value;
+ array->argv[array->argc] = NULL;
+}
+
+void argv_array_push(struct argv_array *array, const char *value)
+{
+ argv_array_push_nodup(array, xstrdup(value));
+}
+
+void argv_array_pushf(struct argv_array *array, const char *fmt, ...)
+{
+ va_list ap;
+ struct strbuf v = STRBUF_INIT;
+
+ va_start(ap, fmt);
+ strbuf_vaddf(&v, fmt, ap);
+ va_end(ap);
+
+ argv_array_push_nodup(array, strbuf_detach(&v, NULL));
+}
+
+void argv_array_clear(struct argv_array *array)
+{
+ if (array->argv != empty_argv) {
+ int i;
+ for (i = 0; i < array->argc; i++)
+ free((char **)array->argv[i]);
+ free(array->argv);
+ }
+ argv_array_init(array);
+}
+
diff --git a/argv-array.h b/argv-array.h
new file mode 100644
index 0000000..2b6273b
--- /dev/null
+++ b/argv-array.h
@@ -0,0 +1,20 @@
+#ifndef ARGV_ARRAY_H
+#define ARGV_ARRAY_H
+
+extern const char **empty_argv;
+
+struct argv_array {
+ const char **argv;
+ int argc;
+ int alloc;
+};
+
+#define ARGV_ARRAY_INIT { empty_argv, 0, 0 };
+
+void argv_array_init(struct argv_array *);
+void argv_array_push(struct argv_array *, const char *);
+__attribute__((format (printf,2,3)))
+void argv_array_pushf(struct argv_array *, const char *fmt, ...);
+void argv_array_clear(struct argv_array *);
+
+#endif /* ARGV_ARRAY_H */
diff --git a/submodule.c b/submodule.c
index 9431c42..6306737 100644
--- a/submodule.c
+++ b/submodule.c
@@ -9,6 +9,7 @@
#include "refs.h"
#include "string-list.h"
#include "sha1-array.h"
+#include "argv-array.h"
static struct string_list config_name_for_path;
static struct string_list config_fetch_recurse_submodules_for_name;
@@ -388,52 +389,22 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])
sha1_array_append(&ref_tips_after_fetch, new_sha1);
}
-struct argv_array {
- const char **argv;
- unsigned int argc;
- unsigned int alloc;
-};
-
-static void init_argv(struct argv_array *array)
-{
- array->argv = NULL;
- array->argc = 0;
- array->alloc = 0;
-}
-
-static void push_argv(struct argv_array *array, const char *value)
-{
- ALLOC_GROW(array->argv, array->argc + 2, array->alloc);
- array->argv[array->argc++] = xstrdup(value);
- array->argv[array->argc] = NULL;
-}
-
-static void clear_argv(struct argv_array *array)
-{
- int i;
- for (i = 0; i < array->argc; i++)
- free((char **)array->argv[i]);
- free(array->argv);
- init_argv(array);
-}
-
static void add_sha1_to_argv(const unsigned char sha1[20], void *data)
{
- push_argv(data, sha1_to_hex(sha1));
+ argv_array_push(data, sha1_to_hex(sha1));
}
static void calculate_changed_submodule_paths(void)
{
struct rev_info rev;
struct commit *commit;
- struct argv_array argv;
+ struct argv_array argv = ARGV_ARRAY_INIT;
init_revisions(&rev, NULL);
- init_argv(&argv);
- push_argv(&argv, "--"); /* argv[0] program name */
+ argv_array_push(&argv, "--"); /* argv[0] program name */
sha1_array_for_each_unique(&ref_tips_after_fetch,
add_sha1_to_argv, &argv);
- push_argv(&argv, "--not");
+ argv_array_push(&argv, "--not");
sha1_array_for_each_unique(&ref_tips_before_fetch,
add_sha1_to_argv, &argv);
setup_revisions(argv.argc, argv.argv, &rev, NULL);
@@ -460,7 +431,7 @@ static void calculate_changed_submodule_paths(void)
}
}
- clear_argv(&argv);
+ argv_array_clear(&argv);
sha1_array_clear(&ref_tips_before_fetch);
sha1_array_clear(&ref_tips_after_fetch);
initialized_fetch_ref_tips = 0;
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 2/7] quote.h: fix bogus comment
From: Jeff King @ 2011-09-13 21:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
Commit 758e915 made sq_quote_next static, removing it from
quote.h. However, it forgot to update the related comment,
making it appear as a confusing description of sq_quote_to_argv.
Let's remove the crufty bits, and elaborate more on sq_quote_to_argv.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.h | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/quote.h b/quote.h
index 024e21d..252b0df 100644
--- a/quote.h
+++ b/quote.h
@@ -40,9 +40,8 @@
/*
* Same as the above, but can be used to unwrap many arguments in the
- * same string separated by space. "next" is changed to point to the
- * next argument that should be passed as first parameter. When there
- * is no more argument to be dequoted, "next" is updated to point to NULL.
+ * same string separated by space. Like sq_quote, it works in place,
+ * modifying arg and appending pointers into it to argv.
*/
extern int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 1/7] add sha1_array API docs
From: Jeff King @ 2011-09-13 21:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
In-Reply-To: <20110913215026.GA26743@sigill.intra.peff.net>
This API was introduced in 902bb36, but never documented.
Let's be nice to future users of the code.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-sha1-array.txt | 79 ++++++++++++++++++++++++++++
1 files changed, 79 insertions(+), 0 deletions(-)
create mode 100644 Documentation/technical/api-sha1-array.txt
diff --git a/Documentation/technical/api-sha1-array.txt b/Documentation/technical/api-sha1-array.txt
new file mode 100644
index 0000000..4a4bae8
--- /dev/null
+++ b/Documentation/technical/api-sha1-array.txt
@@ -0,0 +1,79 @@
+sha1-array API
+==============
+
+The sha1-array API provides storage and manipulation of sets of SHA1
+identifiers. The emphasis is on storage and processing efficiency,
+making them suitable for large lists. Note that the ordering of items is
+not preserved over some operations.
+
+Data Structures
+---------------
+
+`struct sha1_array`::
+
+ A single array of SHA1 hashes. This should be initialized by
+ assignment from `SHA1_ARRAY_INIT`. The `sha1` member contains
+ the actual data. The `nr` member contains the number of items in
+ the set. The `alloc` and `sorted` members are used internally,
+ and should not be needed by API callers.
+
+Functions
+---------
+
+`sha1_array_append`::
+ Add an item to the set. The sha1 will be placed at the end of
+ the array (but note that some operations below may lose this
+ ordering).
+
+`sha1_array_sort`::
+ Sort the elements in the array.
+
+`sha1_array_lookup`::
+ Perform a binary search of the array for a specific sha1.
+ If found, returns the offset (in number of elements) of the
+ sha1. If not found, returns a negative integer. If the array is
+ not sorted, this function has the side effect of sorting it.
+
+`sha1_array_clear`::
+ Free all memory associated with the array and return it to the
+ initial, empty state.
+
+`sha1_array_for_each_unique`::
+ Efficiently iterate over each unique element of the list,
+ executing the callback function for each one. If the array is
+ not sorted, this function has the side effect of sorting it.
+
+Examples
+--------
+
+-----------------------------------------
+void print_callback(const unsigned char sha1[20],
+ void *data)
+{
+ printf("%s\n", sha1_to_hex(sha1));
+}
+
+void some_func(void)
+{
+ struct sha1_array hashes = SHA1_ARRAY_INIT;
+ unsigned char sha1[20];
+
+ /* Read objects into our set */
+ while (read_object_from_stdin(sha1))
+ sha1_array_append(&hashes, sha1);
+
+ /* Check if some objects are in our set */
+ while (read_object_from_stdin(sha1)) {
+ if (sha1_array_lookup(&hashes, sha1) >= 0)
+ printf("it's in there!\n");
+
+ /*
+ * Print the unique set of objects. We could also have
+ * avoided adding duplicate objects in the first place,
+ * but we would end up re-sorting the array repeatedly.
+ * Instead, this will sort once and then skip duplicates
+ * in linear time.
+ */
+ sha1_array_for_each_unique(&hashes, print_callback, NULL);
+}
+-----------------------------------------
--
1.7.7.rc1.2.gb2409
^ permalink raw reply related
* [PATCH 0/7] create argv_array API
From: Jeff King @ 2011-09-13 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christian Couder, Jens Lehmann, git
On Tue, Sep 13, 2011 at 10:34:58AM -0700, Junio C Hamano wrote:
> > Will do. Junio, do you want me to re-roll the quadratic fix, or just
> > build the refactoring on top?
>
> The latter would be fine and it probably is not even urgent.
I went ahead and did it now, while it is still in my head. The first two
patches are unrelated fixups I noticed while working on it. The third is
the refactoring, and then the rest use it in various places. They're
certainly not urgent, and the final one borders on code churn, so they
may not all be worth applying. But I don't think they conflict with
anything in 'next', so now might be a good time.
These are on top of what you have in jk/maint-fetch-submodule-check-fix.
[1/7]: add sha1_array API docs
[2/7]: quote.h: fix bogus comment
[3/7]: refactor argv_array into generic code
[4/7]: quote: provide sq_dequote_to_argv_array
[5/7]: bisect: use argv_array API
[6/7]: checkout: use argv_array API
[7/7]: run_hook: use argv_array API
-Peff
^ permalink raw reply
* [PATCH] git-p4: import utf16 file properly
From: Chris Li @ 2011-09-13 21:33 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Junio C Hamano
The current git-p4 does not handle utf16 files properly.
The "p4 print" command, when output to stdout, converts the
utf16 file into utf8. That effectively imported the utf16 file
as utf8 for git. In other words, git-p4 import a different
file compare to file check out by perforce. This breakes my
windows build in the company project.
The fix is simple, just ask perforce to print the depot
file into a real file. This way perforce will not performe
the utf16 to utf8 conversion. Git can import the exact same
file as perforce checkout.
---
contrib/fast-import/git-p4 | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 6b9de9e..5fb1ac7 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -1239,6 +1239,11 @@ class P4Sync(Command, P4UserMap):
contents = map(lambda text:
re.sub(r'(?i)\$(Id|Header):[^$]*\$',r'$\1$', text), contents)
elif file['type'] in ('text+k', 'ktext', 'kxtext',
'unicode+k', 'binary+k'):
contents = map(lambda text:
re.sub(r'\$(Id|Header|Author|Date|DateTime|Change|File|Revision):[^$\n]*\$',r'$\1$',
text), contents)
+ elif file['type'] == 'utf16':
+ tmpFile = tempfile.NamedTemporaryFile()
+ p4CmdList("print -o %s %s"%(tmpFile.name, file['depotFile']))
+ contents = [ open(tmpFile.name).read() ]
+ tmpFile.close()
self.gitStream.write("M %s inline %s\n" % (mode, relPath))
--
1.7.6
^ permalink raw reply related
* Re: [PATCH] fetch: avoid quadratic loop checking for updated submodules
From: Jens Lehmann @ 2011-09-13 19:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, git-dev
In-Reply-To: <7vwrdd5x61.fsf@alter.siamese.dyndns.org>
Am 12.09.2011 22:21, schrieb Junio C Hamano:
> Jeff King <peff@peff.net> writes:
>
>> Instead, this patch structures the code like this:
>
> Yup, I agree that's the right way to do the other half of the issue.
Ack from me too! I tested it on the repo with 3k refs and the time went
down from 142s to 1s (when applied to 3793ac56b4, as later versions of
master contain my other half which would skip Peff's code).
On current master including my other half this takes 0.90s, while running
with Peff's code on top of 3793ac56b4 it takes .96s. That is 6 hundreds
of a second (7%) extra for not having to worry if one must run "git fetch
--recurse-submodules" or not.
^ permalink raw reply
* [PATCH/RFC] bash: add --word-diff option to diff auto-completion
From: Jonathan Nieder @ 2011-09-13 19:14 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Rodrigo Rosenfeld Rosas, Thomas Rast, git
In-Reply-To: <4E6FA541.7000100@yahoo.com.br>
From: Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com>
Date: Tue, 13 Sep 2011 15:24:38 -0300
Add "--word-diff" to diff completion, since this is a common
desired option when looking at diffs.
Signed-off-by: Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com>
---
Hi Gábor,
Here's a patch. What do you think?
I was thinking it would be nice to complete --word-diff-regex, too,
and to be able to do
git diff --color-words=<TAB>
git diff --word-diff=<TAB>
but I couldn't find any examples of the latter to crib from, so I've
left the patch unmangled except for stealing a patch description from
a separate email.
Rodrigo Rosenfeld Rosas wrote:
> I use Thunderbird, but couldn't find all options as instructed in 'git help
> format-patch'.
Indeed, sending patches unmangled seems to be a common difficult step
when starting to contribute to projects like linux and git. I wonder
if it would make sense to include some sort of
patch-sending-reviewing-tweaking-and-receiving tutorial to point to in
the documentation.
Thanks for keeping the completion code in good shape.
Jonathan
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..f4aaffe 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1300,7 +1300,7 @@ _git_describe ()
__git_diff_common_options="--stat --numstat --shortstat --summary
--patch-with-stat --name-only --name-status --color
- --no-color --color-words --no-renames --check
+ --no-color --color-words --word-diff --no-renames --check
--full-index --binary --abbrev --diff-filter=
--find-copies-harder
--text --ignore-space-at-eol --ignore-space-change
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH] fetch: avoid quadratic loop checking for updated submodules
From: Jens Lehmann @ 2011-09-13 19:13 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, git-dev
In-Reply-To: <20110912224934.GA28994@sigill.intra.peff.net>
Am 13.09.2011 00:49, schrieb Jeff King:
> So with a set of changes like:
>
> [assume submodule at commit A, superproject at commit B]
>
> 1. Make commit C in submodule repo.
>
> 2. Make commit D in superproject repo.
>
> 3. Make commit E in submodule repo.
>
> 4. Make commit F in superproject repo.
>
> what does it buy us to find out that the submodule changed from "A" to
> "C"? We can't actually fetch it. We can only fetch the tips of the
> submodule and hope that they include everything we wanted (i.e., both C
> and E; which might not be the case of E rewound and is not a descendant
> of C).
Yes. But working with submodules in my experience only then works well
when you never drop a submodule commit recorded in any superproject. At
my dayjob we have the convention: You may only record commits that are
on the submodule's master - or another never to be rewound integration
branch - in the superproject. That gives us all needed commits in a
simple fetch.
> So since we must accept that we can't necessarily get every intermediate
> step, I wonder if we are simply better off diffing the "before" and
> "after" state of a particular ref, rather than traversing. It's way
> cheaper, and is just as likely to give us the same information (i.e.,
> which submodule paths had changed commits).
The real world use case I have for that is that when a bug introduced by
a new submodule commit is detected later on, the superproject's fix
records an older submodule commit to remove the problematic change from
the superproject. But the submodule's branch isn't rewound (as that is
most probably master) but a fix is applied on top of it. Then that one
gets tested and - if it was found ok - recorded in the superproject.
Changes like this could be overlooked if you only compare "before" and
"after" instead of traversing, leading to not fetching a submodule which
does have new commits that are used in the newly fetched superproject's
commits. I'd like to have on-demand fetch keep the correct solution of
traversing unless we have good reasons against it, especially as teaching
checkout & friends to recursively update submodules too depends on all
needed commits being present.
^ 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