* Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
From: Johannes Sixt @ 2008-09-23 20:41 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git, Junio C Hamano, Steffen Prohaska
In-Reply-To: <20080923194802.GQ21650@dpotapov.dyndns.org>
On Dienstag, 23. September 2008, Dmitry Potapov wrote:
> On Tue, Sep 23, 2008 at 09:03:08PM +0200, Johannes Sixt wrote:
> > On Dienstag, 23. September 2008, Dmitry Potapov wrote:
> > > +static int do_stat(const char *file_name, struct stat *buf, stat_fn_t
> > > cygstat) +{
> > > + WIN32_FILE_ATTRIBUTE_DATA fdata;
> > > +
> > > + if (file_name[0] == '/')
> > > + return cygstat (file_name, buf);
> >
> > You should do this in the caller; it would make this function's
> > semantics much clearer.
>
> IMHO, the semantic of this function is clear: do_stat performs stat/lstat
> using Windows API with falling back on Cygwin implementation in those
> rare cases that it cannot handle correctly. Absolute path is just one of
> those cases. So, I am not sure what you win by moving this two lines out.
You copied the function from compat/mingw.c. There it has the meaning "Fill in
struct stat using Win32 API" and nothing else. Here it has the meaning "Fill
in struct stat using Win32 API if you can, and using cygstat() in certain
exceptional cases". If you stayed with the original meaning, it would be
slightly easier to factor out common code.
> > > + /* st_dev, st_rdev are not used by Git */
> > > + buf->st_dev = buf->st_rdev = 0;
>
> I set this to 0, while MinGW Git uses _getdrive(). I have no idea why
> it does so.
Indeed. Calling _getdrive() is absolutely useless.
> > You do duplicate a lot of code here. Any chances to factor out the
> > common parts?
>
> I don't see much common code here. Initialization of 5 variables where
> four of them are just constants? Perhaps, the biggest common part here
> is conversion of dwFileAttributes to st_mode, but it is still 5 lines of
> trivial code.
Sigh. I gave a pointer how to unify the two functions (although I missed the
fact that the member variables are named differently). I'd appreciate if you
did not make it more difficult than necessary to factor out common code.
-- Hannes
^ permalink raw reply
* Re: Locking binary files
From: Dmitry Potapov @ 2008-09-23 20:46 UTC (permalink / raw)
To: Mario Pareja; +Cc: Andreas Ericsson, Git Mailing List
In-Reply-To: <94c1db200809230656q4a9a765dw2354c0058b1d940c@mail.gmail.com>
On Tue, Sep 23, 2008 at 09:56:57AM -0400, Mario Pareja wrote:
>
> The SVN client will make locked files read-only until a lock is
> obtained for them. This helps "remind" you that a lock should be
> obtained before editing such a file. Requiring the developer to obtain
> a lock ensures that nobody else is editing the file and prevents
> wasted work. Upon commit, the file is marked as unlocked and the
> local file is once again read-only.
The approach that SVN takes is not only impossible for distributed
environment, it does not work even in a _single_ repository where you
have branching and merging. If you have a topic branch then your lock
will have a zero effect on other developers or lock of other developers
on you. Obviously, you are going to have the binary merge conflict at
the end. But it is even worse than that. Somebody locked a file on the
master branch and you clone from it. Now, this somebody unlocked this
file, but this file on your branch remains locked but this person, and
this person may even not aware that about your branch. That is insane!
Dmitry
^ permalink raw reply
* [PATCH 1/3] Prepare for non-interactive merge-preserving rebase
From: Andreas Ericsson @ 2008-09-23 20:57 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano, Stephen Haberman
This patch adds two tests (really three, but one of
them just handles setup) which we currently expect
to fail.
One of them tests "git rebase -p", without the -i flag,
to make sure it works without phony editors and suchlike.
The other tests "git pull --rebase --preserve-merges"
to make sure that the same functionality exists there.
The test was originally written by Stephen Habermann
<stephen@exigencecorp.com> but has been significantly
modified since its creation.
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
Stephen, I had to modify the tests a bit to get them to work with
how I implemented the merge-preserving rebase, and also to remove
a lot of the cruft that was previously in there. Hope you're ok
with the attribution in the commit message.
t/t3409-rebase-preserve-merges.sh | 68 +++++++++++++++++++++++++++++++++++++
1 files changed, 68 insertions(+), 0 deletions(-)
create mode 100644 t/t3409-rebase-preserve-merges.sh
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
new file mode 100644
index 0000000..532b220
--- /dev/null
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+#
+# Copyright(C) 2008 Stephen Habermann & Andreas Ericsson
+#
+test_description='git rebase -p should preserve merges
+
+This test runs various incantations of "git rebase -p" and checks
+that merges are properly carried along
+'
+. ./test-lib.sh
+
+GIT_AUTHOR_EMAIL=bogus_email_address
+export GIT_AUTHOR_EMAIL
+
+#echo 'Setting up:
+#
+#A1--A2 <-- origin/master
+# \ \
+# B1--M <-- topic
+# \
+# B2 <-- origin/topic
+#
+#'
+
+test_expect_success 'setup for merge-preserving rebase' \
+ 'echo First > A &&
+ git add A &&
+ git-commit -m "Add A1" &&
+ git checkout -b topic &&
+ echo Second > B &&
+ git add B &&
+ git-commit -m "Add B1" &&
+ git checkout -f master &&
+ echo Third >> A &&
+ git-commit -a -m "Modify A2" &&
+
+ git clone ./. clone1 &&
+ cd clone1 &&
+ git checkout -b topic origin/topic &&
+ git merge origin/master &&
+ cd ..
+
+ git clone ./. clone2
+ cd clone2 &&
+ git checkout -b topic origin/topic &&
+ git merge origin/master &&
+ cd .. &&
+
+ git checkout topic &&
+ echo Fourth >> B &&
+ git commit -a -m "Modify B2"
+'
+
+test_expect_failure 'git pull --rebase -p on moved topic' '
+ cd clone1 &&
+ git pull --rebase --preserve-merges &&
+ test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
+'
+
+test_expect_failure 'rebase -p merge on moved topic' '
+ cd ../clone2 &&
+ git fetch &&
+ git rebase -p origin/topic &&
+ test 1 = $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) &&
+ test 1 = $(git rev-list --all --pretty=oneline | grep "Merge commit" | wc -l)
+'
+
+test_done
--
1.6.0.2.307.gc4275.dirty
^ permalink raw reply related
* [PATCH 2/3] git rebase: Support non-interactive merge-preserving rebase
From: Andreas Ericsson @ 2008-09-23 20:58 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano, Stephen Haberman
Previously, 'git rebase --preserve-merges' would only work
in interactive mode. For some workflows, this was quite a
limitation.
This patch adds a workaround, invoking the
git-rebase--interactive helper with GIT_EDITOR set to :
in case the user passes "-p" but not "-i" to the rebase
command.
The effect is that the interactive rebase helper is used,
but the user won't see an editor.
Since this patch fixes the latter of the two expected
testfailures in t3409-rebase-preserve-merges, that test
is now set to expect success.
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
Documentation/git-rebase.txt | 3 +--
git-rebase.sh | 22 +++++++++++++++++++---
t/t3409-rebase-preserve-merges.sh | 2 +-
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 59c1b02..ddec6a6 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -250,8 +250,7 @@ OPTIONS
-p::
--preserve-merges::
- Instead of ignoring merges, try to recreate them. This option
- only works in interactive mode.
+ Instead of ignoring merges, try to recreate them.
include::merge-strategies.txt[]
diff --git a/git-rebase.sh b/git-rebase.sh
index 528b604..03e5f95 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -138,10 +138,26 @@ finish_rb_merge () {
}
is_interactive () {
- test -f "$dotest"/interactive ||
- while :; do case $#,"$1" in 0,|*,-i|*,--interactive) break ;; esac
+ while test $# != 0
+ do
+ case "$1" in
+ -i|--interactive)
+ interactive_rebase=explicit
+ break
+ ;;
+ -p|--preserve-merges)
+ interactive_rebase=implied
+ ;;
+ esac
shift
- done && test -n "$1"
+ done
+
+ if [ "$interactive_rebase" = implied ]; then
+ GIT_EDITOR=:
+ export GIT_EDITOR
+ fi
+
+ test -n "$interactive_rebase" || test -f "$dotest"/interactive
}
test -f "$GIT_DIR"/rebase-apply/applying &&
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 532b220..21b8c79 100644
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -57,7 +57,7 @@ test_expect_failure 'git pull --rebase -p on moved topic' '
test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
'
-test_expect_failure 'rebase -p merge on moved topic' '
+test_expect_success 'rebase -p merge on moved topic' '
cd ../clone2 &&
git fetch &&
git rebase -p origin/topic &&
--
1.6.0.2.307.gc4275.dirty
^ permalink raw reply related
* [PATCH 3/3] git pull: Support --preserve-merges as a flag to rebase
From: Andreas Ericsson @ 2008-09-23 20:58 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano, Stephen Haberman
Now that "git rebase" supports non-interactive rebases
preserving merges, this patch is the next logical step
for those who wish to use such a workflow.
Since this patch makes the last test marked as expecting
failure in t3409-rebase-preserve-merges, we now alter it
to expect success.
Signed-off-by: Andreas Ericsson <ae@op5.se>
---
Documentation/git-pull.txt | 4 ++++
git-pull.sh | 6 +++++-
t/t3409-rebase-preserve-merges.sh | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 7578623..333fc55 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -47,6 +47,10 @@ unless you have read linkgit:git-rebase[1] carefully.
--no-rebase::
Override earlier --rebase.
+--preserve-merges::
+ Preserves merge commits when rebasing. Implies --rebase,
+ so the same warnings naturally apply.
+
include::fetch-options.txt[]
include::pull-fetch-param.txt[]
diff --git a/git-pull.sh b/git-pull.sh
index 75c3610..270a50d 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -58,6 +58,10 @@ do
-r|--r|--re|--reb|--reba|--rebas|--rebase)
rebase=true
;;
+ --preserve-merges) # no short option for this
+ preserve_merges="--preserve-merges"
+ rebase=true
+ ;;
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
rebase=false
;;
@@ -179,7 +183,7 @@ fi
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
test true = "$rebase" &&
- exec git-rebase $strategy_args --onto $merge_head \
+ exec git-rebase $preserve_merges $strategy_args --onto $merge_head \
${oldremoteref:-$merge_head}
exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
"$merge_name" HEAD $merge_head
diff --git a/t/t3409-rebase-preserve-merges.sh b/t/t3409-rebase-preserve-merges.sh
index 21b8c79..9a376ef 100644
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -51,7 +51,7 @@ test_expect_success 'setup for merge-preserving rebase' \
git commit -a -m "Modify B2"
'
-test_expect_failure 'git pull --rebase -p on moved topic' '
+test_expect_success 'git pull --rebase -p on moved topic' '
cd clone1 &&
git pull --rebase --preserve-merges &&
test $(git rev-list --all --pretty=oneline | grep "Modify A" | wc -l) = 1
--
1.6.0.2.307.gc4275.dirty
^ permalink raw reply related
* Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
From: Dmitry Potapov @ 2008-09-23 21:11 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano, Steffen Prohaska
In-Reply-To: <200809232241.42649.johannes.sixt@telecom.at>
On Tue, Sep 23, 2008 at 10:41:42PM +0200, Johannes Sixt wrote:
>
> You copied the function from compat/mingw.c. There it has the meaning "Fill in
> struct stat using Win32 API" and nothing else. Here it has the meaning "Fill
> in struct stat using Win32 API if you can, and using cygstat() in certain
> exceptional cases". If you stayed with the original meaning, it would be
> slightly easier to factor out common code.
do_stat() always fills in the structure, but it can do that fast using
Win32 API or fallback on cygstat() in exceptional cases. So, I don't
think I change its meaning much, its implementation certainly differs.
> > > You do duplicate a lot of code here. Any chances to factor out the
> > > common parts?
> >
> > I don't see much common code here. Initialization of 5 variables where
> > four of them are just constants? Perhaps, the biggest common part here
> > is conversion of dwFileAttributes to st_mode, but it is still 5 lines of
> > trivial code.
>
> Sigh. I gave a pointer how to unify the two functions (although I missed the
> fact that the member variables are named differently). I'd appreciate if you
> did not make it more difficult than necessary to factor out common code.
Because the stat structure is different and handling exceptional
situation is different, I don't think we can have a single do_stat
function for Cygwin and MinGW. Yet, perhaps, it is possible to
move some code in common functions even if it is just a few lines.
The first candidate is win_attr_to_st_mode(), which converts
dwFileAttributes returned by GetFileAttributesExA to st_mode.
Another possible function is that obtains and converts Win32 error
code to errno value. These function can be placed into some common
header (for example, win32.h), which will included by both
implementations. Does it make sense?
Dmitry
^ permalink raw reply
* [PATCH 1/2] git remote: move part of prune() into prune_one()
From: Lars Hjemli @ 2008-09-23 21:13 UTC (permalink / raw)
To: git
In-Reply-To: <1222204405-30454-1-git-send-email-hjemli@gmail.com>
The new function will be reused by prune_all() in the next patch.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
builtin-remote.c | 67 +++++++++++++++++++++++++++++------------------------
1 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 4cb763f..626256e 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -537,6 +537,41 @@ static int show(int argc, const char **argv)
return result;
}
+static int prune_one(const char *name, int dry_run)
+{
+ struct ref_states states;
+ int result = 0;
+ int i;
+
+ memset(&states, 0, sizeof(states));
+ get_remote_ref_states(name, &states, 1);
+
+ if (states.stale.nr) {
+ printf("Pruning %s\n", name);
+ printf("URL: %s\n",
+ states.remote->url_nr
+ ? states.remote->url[0]
+ : "(no URL)");
+ }
+
+ for (i = 0; i < states.stale.nr; i++) {
+ const char *refname = states.stale.items[i].util;
+
+ if (!dry_run)
+ result |= delete_ref(refname, NULL);
+
+ printf(" * [%s] %s\n", dry_run ? "would prune" : "pruned",
+ abbrev_ref(refname, "refs/remotes/"));
+ }
+
+ /* NEEDSWORK: free remote */
+ string_list_clear(&states.new, 0);
+ string_list_clear(&states.stale, 0);
+ string_list_clear(&states.tracked, 0);
+
+ return result;
+}
+
static int prune(int argc, const char **argv)
{
int dry_run = 0, result = 0;
@@ -545,42 +580,14 @@ static int prune(int argc, const char **argv)
OPT__DRY_RUN(&dry_run),
OPT_END()
};
- struct ref_states states;
argc = parse_options(argc, argv, options, builtin_remote_usage, 0);
if (argc < 1)
usage_with_options(builtin_remote_usage, options);
- memset(&states, 0, sizeof(states));
- for (; argc; argc--, argv++) {
- int i;
-
- get_remote_ref_states(*argv, &states, 1);
-
- if (states.stale.nr) {
- printf("Pruning %s\n", *argv);
- printf("URL: %s\n",
- states.remote->url_nr
- ? states.remote->url[0]
- : "(no URL)");
- }
-
- for (i = 0; i < states.stale.nr; i++) {
- const char *refname = states.stale.items[i].util;
-
- if (!dry_run)
- result |= delete_ref(refname, NULL);
-
- printf(" * [%s] %s\n", dry_run ? "would prune" : "pruned",
- abbrev_ref(refname, "refs/remotes/"));
- }
-
- /* NEEDSWORK: free remote */
- string_list_clear(&states.new, 0);
- string_list_clear(&states.stale, 0);
- string_list_clear(&states.tracked, 0);
- }
+ for (; argc; argc--, argv++)
+ result |= prune_one(*argv, dry_run);
return result;
}
--
1.6.0.2.309.gc1f46
^ permalink raw reply related
* [PATCH 0/2] Teach `git remote` howto prune all remotes
From: Lars Hjemli @ 2008-09-23 21:13 UTC (permalink / raw)
To: git
Lars Hjemli (2):
git remote: split part of prune() into prune_one()
git remote: prune all remotes with `prune -a`
Documentation/git-remote.txt | 4 +-
builtin-remote.c | 97 ++++++++++++++++++++++++++++-------------
2 files changed, 69 insertions(+), 32 deletions(-)
^ permalink raw reply
* Re: Locking binary files
From: Daniel Barkalow @ 2008-09-23 21:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mario Pareja, Andreas Ericsson, Git Mailing List
In-Reply-To: <7v7i92tzgb.fsf@gitster.siamese.dyndns.org>
On Tue, 23 Sep 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > I think the right tool on the git side is actually a "smudge/clean"
> > script. When you check something out, git converts it from the
> > repository-stored form to a working tree form using a script (if there is
> > one configured); this could check whether you've got the appropriate lock,
> > and make the file unwritable if you don't.
>
> An obvious question is "how would such a script check the lock when you
> are 30,000 ft above ground"; in other words, this "locking mechanism"
> contradicts the very nature of distributed development theme. The best
> mechanism should always be on the human side. An SCM auguments
> inter-developer communication, but it is not a _substitute_ for
> communication.
If you're offline, you can't get new locks, nor release them. But it can
make reasonable decisions if it remembers what locks you got before.
On the other hand, you can just make the file writable yourself while
disconnected, and nothing bad happens to anybody else; if someone else
locks the file and starts working, they'll block your eventual push until
they push and you merge. And nothing too bad happens to you; you get stuck
redoing the change later (as a merge), but (a) you would have had to do
the work then anyway; (b) you knew you weren't protecting yourself; and
(c) at least you got to practice on the plane.
The point of the locking is just that, if you get the lock for a
particular file in a particular branch on a particular shared repository,
you can be sure you won't have to merge that file in order to push there,
and you can get this worked out in advance of having the push ready. A
secondary concern is that you might want to stop yourself from working on
certain things without this kind of reservation, but that's a local
decision.
> But if you limit the use case to an always tightly connected environment
> (aka "not distributed at all"), I agree the above would be a very
> reasonable approach.
>
> Such a setup would need a separate locking infrastructure and an end user
> command that grabs the lock and when successful makes the file in the work
> tree read/write. The user butchers the contents after taking the lock,
> saves, and then when running "git commit", probably the post-commit hook
> would release any relevant locks.
The lock needs to last until you push to the repository the lock is for;
otherwise you have the exclusive ability to make changes, but someone who
grabs the lock right after you release it will still be working on the
version without your change, which is what the lock is supposed to
prevent.
> All these can be left outside the scope of git, as they can be hooked into
> git with the existing infrastructure. Once a BCP materializes it could be
> added to contrib/ just like the "paranoid" update hook.
It would be handy to link against some of git, since it will want to use
git config files and remotes and refspecs to figure out what lock to ask
for on the client side, and how to communicate with the target remote
repository, and the process of getting a lock requires checking that
you're up-to-date, and git's also got a bunch of useful code for atomic
file updates and repository-scoped filename management. But adding this
doesn't have to modify any existing behavior.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH 2/2] git remote: prune all remotes with `prune -a`
From: Lars Hjemli @ 2008-09-23 21:13 UTC (permalink / raw)
To: git
In-Reply-To: <1222204405-30454-2-git-send-email-hjemli@gmail.com>
This is handy when working with lots of remotes.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/git-remote.txt | 4 +++-
builtin-remote.c | 32 ++++++++++++++++++++++++++++++--
2 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index bb99810..55c108e 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -13,7 +13,7 @@ SYNOPSIS
'git remote add' [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>
'git remote rm' <name>
'git remote show' [-n] <name>
-'git remote prune' [-n | --dry-run] <name>
+'git remote prune' [-n | --dry-run] [-a | <name>]
'git remote update' [group]
DESCRIPTION
@@ -82,6 +82,8 @@ referenced by <name>, but are still locally available in
+
With `--dry-run` option, report what branches will be pruned, but do no
actually prune them.
++
+With `-a` option, prune all remotes.
'update'::
diff --git a/builtin-remote.c b/builtin-remote.c
index 626256e..973637e 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -572,20 +572,48 @@ static int prune_one(const char *name, int dry_run)
return result;
}
+struct prune_cbdata {
+ int result;
+ int dry_run;
+};
+
+static int prune_all_cb(struct remote *remote, void *cbdata)
+{
+ struct prune_cbdata *cb = cbdata;
+
+ cb->result |= prune_one(remote->name, cb->dry_run);
+ return 0;
+}
+
+static int prune_all(int dry_run)
+{
+ int result;
+ struct prune_cbdata cb;
+
+ cb.result = 0;
+ cb.dry_run = dry_run;
+ result = for_each_remote(prune_all_cb, &cb);
+ return (result | cb.result);
+}
+
static int prune(int argc, const char **argv)
{
- int dry_run = 0, result = 0;
+ int dry_run = 0, all = 0, result = 0;
struct option options[] = {
OPT_GROUP("prune specific options"),
+ OPT_BOOLEAN('a', NULL, &all, "prune all remotes"),
OPT__DRY_RUN(&dry_run),
OPT_END()
};
argc = parse_options(argc, argv, options, builtin_remote_usage, 0);
- if (argc < 1)
+ if (!argc == !all)
usage_with_options(builtin_remote_usage, options);
+ if (all)
+ return prune_all(dry_run);
+
for (; argc; argc--, argv++)
result |= prune_one(*argv, dry_run);
--
1.6.0.2.309.gc1f46
^ permalink raw reply related
* Re: clone fails: Could not get the current working directory
From: John Freeman @ 2008-09-23 21:16 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0809230801l2e6b1a71v1210317fe636aeba@mail.gmail.com>
Alex Riesen wrote:
> I actually expected "ls -R" giving error about unable to read the
> directory (permissions).
This worked.
> Again: try a simple program which just does getpwd for this pathname
From what I gather, getpwd() is in libiberty, which I didn't feel like
messing with for this example. I ran a small test program that called
getcwd() in the repo directory, and it failed. errno was set to EACCES,
indicating insufficient permissions.
http://www.opengroup.org/onlinepubs/009695399/functions/getcwd.html
I may get around later to patching this, but for now we're moving ahead
with a workaround.
- John
^ permalink raw reply
* Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase
From: Stephen Haberman @ 2008-09-23 21:22 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <48D95836.6040200@op5.se>
> Stephen, I had to modify the tests a bit to get them to work with how
> I implemented the merge-preserving rebase, and also to remove a lot of
> the cruft that was previously in there. Hope you're ok with the
> attribution in the commit message.
No problem, it looks great.
This is awesome. Thanks for the insanely short turnaround. The
GIT_EDITOR=: hack is neat. I did not think it would be that simple.
- Stephen
^ permalink raw reply
* Stash missing, but not. Can apply, but not drop or list the stash.
From: mattjackets @ 2008-09-23 21:23 UTC (permalink / raw)
To: git
I have a strange stash problem. There is a single stash in the repo.
git stash apply 0 -- works, but results in a conflict. Lets just go
ahead and drop the stash...
git stash list -- shows nothing. huh? Lets go ahead with the drop
anyway and hope it works...
$ git stash drop stash@{0}
fatal: Log .git/logs/refs/stash is empty.
stash@{0}: not a valid stashed state
sure enough, .git/logs/refs/stash is empty
git stash clear -- does nothing
I'm at a loss. I can apply the stash cleanly to older revisions, and
gitk still shows the stash branch. How can I fix this? is it safe to
simply delete the stash branch as if it was any other branch?
Thanks,
Matt
--
matt
mattlist@fastmail.fm
--
http://www.fastmail.fm - Access all of your messages and folders
wherever you are
^ permalink raw reply
* Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
From: Dmitry Potapov @ 2008-09-23 21:28 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Johannes Sixt, Junio C Hamano, Steffen Prohaska
In-Reply-To: <20080923201739.GK3669@spearce.org>
On Tue, Sep 23, 2008 at 01:17:39PM -0700, Shawn O. Pearce wrote:
> Dmitry Potapov <dpotapov@gmail.com> wrote:
> >
> > static void init_stat(void)
> > {
> > git_config(git_cygwin_config, NULL);
> > cygwin_stat_fn = native_stat ? cygwin_stat : stat;
> > cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
>
> if (native_stat < 0 && have_git_dir()) {
> native_stat = 0;
> git_config(git_cygwin_config, NULL);
> cygwin_stat_fn = native_stat ? cygwin_stat : stat;
> cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
> }
I am not sure that I understand what you are trying to do here.
First, in my implementation, init_stat was supposed to always set
cygwin_stat_fn() and cygwin_lstat_fn(), otherwise the code is going
to hit the NULL pointer call.
Second, the check of native_stat < 0 is absolutely useless, because once
we set cygwin_stat_fn and cygwin_lstat_fn, we are never going to call
init_stat() again.
Did you mean this:
if (have_git_dir())
git_config(git_cygwin_config, NULL);
else
native_stat = 0
cygwin_stat_fn = native_stat ? cygwin_stat : stat;
cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
Or:
if (have_git_dir()) {
git_config(git_cygwin_config, NULL);
cygwin_stat_fn = native_stat ? cygwin_stat : stat;
cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
}
and
> > static int cygwin_stat_choice(const char *file_name, struct stat *buf)
> > {
> > init_stat();
> > return (*cygwin_stat_fn)(file_name, buf);
change the above line to:
return (cygwin_stat_fn ? cygwin_stat_fn : stat) (file_name, buf);
so init_stat may be called a few times outside of git directory and then
use the default cygwin function, and once we enter to it then load the
configuration option and act accordingly.
Dmitry
^ permalink raw reply
* Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase
From: Andreas Ericsson @ 2008-09-23 21:30 UTC (permalink / raw)
To: Stephen Haberman; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20080923162211.d4b15373.stephen@exigencecorp.com>
Stephen Haberman wrote:
>> Stephen, I had to modify the tests a bit to get them to work with how
>> I implemented the merge-preserving rebase, and also to remove a lot of
>> the cruft that was previously in there. Hope you're ok with the
>> attribution in the commit message.
>
> No problem, it looks great.
>
> This is awesome. Thanks for the insanely short turnaround.
It requires a bit of testing though. All the t/t34* tests pass with
all the patches applied, and some manual tries worked just fine too,
but if you wanna give it a twirl where you work, that'd be great.
> The
> GIT_EDITOR=: hack is neat. I did not think it would be that simple.
>
Actually, you should be able to use vanilla "git-rebase -i -p" without
getting an editor by doing something like this:
GIT_EDITOR=: git rebase -i -p
but recommending a hack like that to work around a UI deficiency didn't
really appeal to me. If Junio doesn't like the patches though, you could
try using that.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: Locking binary files
From: Dmitry Potapov @ 2008-09-23 21:54 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Mario Pareja, Andreas Ericsson, Git Mailing List
In-Reply-To: <alpine.LNX.1.00.0809231551320.19665@iabervon.org>
On Tue, Sep 23, 2008 at 05:13:29PM -0400, Daniel Barkalow wrote:
>
> The lock needs to last until you push to the repository the lock is for;
> otherwise you have the exclusive ability to make changes, but someone who
> grabs the lock right after you release it will still be working on the
> version without your change, which is what the lock is supposed to
> prevent.
It still will happen if developers work on topic branches, and it is not
a rate situation with Git. Thus locking some particular path is stupid.
What you may want instead is too mark SHA-1 of this file as being edited
and later maybe as being replaced with another one. In this case, anyone
who has the access to the central information storage will get warning
about attempt to edit a file that is edited or already replaced with a
new version.
Dmitry
^ permalink raw reply
* Re: [PATCH] add GIT_FAST_STAT mode for Cygwin
From: Shawn O. Pearce @ 2008-09-23 21:58 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: git, Johannes Sixt, Junio C Hamano, Steffen Prohaska
In-Reply-To: <20080923212858.GU21650@dpotapov.dyndns.org>
Dmitry Potapov <dpotapov@gmail.com> wrote:
> I am not sure that I understand what you are trying to do here.
...
> Did you mean this:
...
> if (have_git_dir()) {
> git_config(git_cygwin_config, NULL);
> cygwin_stat_fn = native_stat ? cygwin_stat : stat;
> cygwin_lstat_fn = native_stat ? cygwin_lstat : lstat;
> }
Err, yes, something more like that.
> > > static int cygwin_stat_choice(const char *file_name, struct stat *buf)
> > > {
> > > init_stat();
> > > return (*cygwin_stat_fn)(file_name, buf);
>
> change the above line to:
> return (cygwin_stat_fn ? cygwin_stat_fn : stat) (file_name, buf);
Right.
> so init_stat may be called a few times outside of git directory and then
> use the default cygwin function, and once we enter to it then load the
> configuration option and act accordingly.
Yup, exactly. Sorry I wasn't being very clear earlier.
--
Shawn.
^ permalink raw reply
* Re: Locking binary files
From: Daniel Barkalow @ 2008-09-23 22:29 UTC (permalink / raw)
To: Dmitry Potapov
Cc: Junio C Hamano, Mario Pareja, Andreas Ericsson, Git Mailing List
In-Reply-To: <20080923215422.GV21650@dpotapov.dyndns.org>
On Wed, 24 Sep 2008, Dmitry Potapov wrote:
> On Tue, Sep 23, 2008 at 05:13:29PM -0400, Daniel Barkalow wrote:
> >
> > The lock needs to last until you push to the repository the lock is for;
> > otherwise you have the exclusive ability to make changes, but someone who
> > grabs the lock right after you release it will still be working on the
> > version without your change, which is what the lock is supposed to
> > prevent.
>
> It still will happen if developers work on topic branches, and it is not
> a rate situation with Git. Thus locking some particular path is stupid.
> What you may want instead is too mark SHA-1 of this file as being edited
> and later maybe as being replaced with another one. In this case, anyone
> who has the access to the central information storage will get warning
> about attempt to edit a file that is edited or already replaced with a
> new version.
No, your goal is to avoid having to do a merge in order to do a particular
push. That push is the push to the shared location. It doesn't matter if
you use topic branches, because your eventual goal is still to push to the
shared location (or, possibly, to have the project maintainer push to the
shared location with some sort of interesting delegation), so you lock the
shared location, not your topic branch.
On the other hand, it's easily possible that other people (or you) want to
fork the image, such that only some locations (either different paths in
the project or the same path in different branches) get your change and
other branches get different changes made at the same time. Of course, if
you want to change multiple things, you need to get multiple locks.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH] Add OS X support to the pre-auto-gc example hook
From: Jonathan del Strother @ 2008-09-23 22:43 UTC (permalink / raw)
To: git; +Cc: Miklos Vajna, Jonathan del Strother
Signed-off-by: Jonathan del Strother <jon.delStrother@bestbefore.tv>
---
Darwin / OS X has a pmset tool for getting power management information. How about adding OS X support to the auto-gc hook?
Shell scripting isn't my forté, suggestions for improvements would be welcome.
contrib/hooks/pre-auto-gc-battery | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery
index 0096f57..0b7bcde 100644
--- a/contrib/hooks/pre-auto-gc-battery
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -1,9 +1,9 @@
#!/bin/sh
#
# An example hook script to verify if you are on battery, in case you
-# are running Linux. Called by git-gc --auto with no arguments. The hook
-# should exit with non-zero status after issuing an appropriate message
-# if it wants to stop the auto repacking.
+# are running Linux or OS X. Called by git-gc --auto with no arguments.
+# The hook should exit with non-zero status after issuing an appropriate
+# message if it wants to stop the auto repacking.
#
# This hook is stored in the contrib/hooks directory. Your distribution
# may have put this somewhere else. If you want to use this hook, you
@@ -30,6 +30,9 @@ then
elif grep -q '0x01$' /proc/apm 2>/dev/null
then
exit 0
+elif test -x /usr/bin/pmset && (! /usr/bin/pmset -g batt | grep -q 'Battery Power' )
+then
+ exit 0
fi
echo "Auto packing deferred; not on AC"
--
1.6.0.2.308.gd442a.dirty
^ permalink raw reply related
* Re: Stash missing, but not. Can apply, but not drop or list the stash.
From: Brandon Casey @ 2008-09-23 23:08 UTC (permalink / raw)
To: mattjackets; +Cc: git
In-Reply-To: <1222204981.28575.1275592473@webmail.messagingengine.com>
mattjackets wrote:
> I have a strange stash problem. There is a single stash in the repo.
>
> git stash apply 0 -- works, but results in a conflict.
The correct form is 'git stash apply stash@{0}'
'git stash apply 0' is doing the right thing for the wrong reason.
For example, 'git stash apply 1' would not do the right thing.
This is unrelated to your issue, but it seems you have uncovered
a flaw which should be fixed.
In git stash, when your command line above is used, eventually
the following command is executed:
git rev-parse --revs-only --no-flags --default refs/stash 0
'git rev-parse' fails to resolve revision "0" and prints out nothing
and then falls back to printing the revision of "refs/stash". This
is not what is desired by stash.
Either rev-parse needs to error out in this case, or 'git stash' needs
to be changed so that '--default refs/stash' is not used here. Possibly,
something like what is done inside drop_stash().
> Lets just go
> ahead and drop the stash...
>
> git stash list -- shows nothing. huh? Lets go ahead with the drop
> anyway and hope it works...
>
> $ git stash drop stash@{0}
> fatal: Log .git/logs/refs/stash is empty.
> stash@{0}: not a valid stashed state
>
> sure enough, .git/logs/refs/stash is empty
Right, it must exist since you actually got the error message
'fatal: Log .git/logs/refs/stash is empty', but it contains
nothing. Not sure how that happened.
> git stash clear -- does nothing
It correctly removes .git/refs/stash for me.
> I'm at a loss. I can apply the stash cleanly to older revisions, and
> gitk still shows the stash branch. How can I fix this? is it safe to
> simply delete the stash branch as if it was any other branch?
In this case yes it would be safe to just do 'rm .git/refs/stash', but
like I said, 'git stash clear' worked for me.
-brandon
^ permalink raw reply
* Re: Locking binary files
From: Dmitry Potapov @ 2008-09-23 23:21 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Mario Pareja, Andreas Ericsson, Git Mailing List
In-Reply-To: <alpine.LNX.1.00.0809231811560.19665@iabervon.org>
On Tue, Sep 23, 2008 at 06:29:53PM -0400, Daniel Barkalow wrote:
> On Wed, 24 Sep 2008, Dmitry Potapov wrote:
>
> > It still will happen if developers work on topic branches, and it is not
> > a rate situation with Git. Thus locking some particular path is stupid.
> > What you may want instead is too mark SHA-1 of this file as being edited
> > and later maybe as being replaced with another one. In this case, anyone
> > who has the access to the central information storage will get warning
> > about attempt to edit a file that is edited or already replaced with a
> > new version.
>
> No, your goal is to avoid having to do a merge in order to do a particular
> push. That push is the push to the shared location. It doesn't matter if
> you use topic branches, because your eventual goal is still to push to the
> shared location (or, possibly, to have the project maintainer push to the
> shared location with some sort of interesting delegation), so you lock the
> shared location, not your topic branch.
What are you saying is that when I am locking some file on the current
branch, Git (or whatever script that performs this locking) should figure
out what is the original shared branch for it and lock the file there.
When you have finished to edit and push changes then the lock should be
removed if changes are pushed to this shared branch, otherwise it should
be some token of delegation to the project maintainer who is going to
push (or probably first merge, because other files may need that) to
this branch.
Maybe, it can work, but it sounds too complex to me. I believe that my
idea using SHA-1 is better. After all, what is file? It is its content.
At least, in Git, we always identify files by their content. Thus if you
lock some file, you put a lock on certain SHA-1. Now, regardless of
branches and paths, this lock can work provided that you have access to
some shared location. Of course, this lock is purely advisory, but it is
good, because you may want to ignore it in some case. For instance, you
want to created a new branch based on the current shared location and
have no plan to ever merge it back. In this case, the lock on the shared
branch should not matter to you. This is true regardless how you
implement locking, and in your scheme it will another special case.
Dmitry
^ permalink raw reply
* [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied
From: Brandon Casey @ 2008-09-23 23:57 UTC (permalink / raw)
To: mattjackets; +Cc: Git Mailing List
In-Reply-To: <klevRMI-z5Id8iuqn2rqrKQZ8LdPNE4lABeC502X9y1Es5wwQ-s8GA@cipher.nrlssc.navy.mil>
apply_stash() and show_stash() each call rev-parse with
'--default refs/stash' as an argument. This option causes rev-parse to
operate on refs/stash if it is not able to successfully operate on any
element of the command line. This includes failure to supply a "valid"
revision. This has the effect of causing 'stash apply' and 'stash show'
to operate as if stash@{0} had been supplied when an invalid revision is
supplied.
e.g. 'git stash apply stahs@{1}' would fall back to
'git stash apply stash@{0}'
This patch modifies these two functions so that they avoid using the
--default option of rev-parse.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
This should fix the case I mention above, but it does not fix the
case where a non-existent reflog entry is specified. In this case
the last entry will be selected.
$ git stash list
stash@{0}: WIP on master: c050772... small java change
stash@{1}: WIP on master: c050772... small java change
stash@{2}: WIP on master: c050772... small java change
stash@{3}: WIP on master: c050772... small java change
$ git stash apply stash@{10}
warning: Log for 'stash' only has 4 entries.
# On branch master
# Changed but not updated:
... etc.
stash@{3} was applied.
Luckily, the dangerous case has no effect.
$ git stash drop stash@{10}
Dropped stash@{10} (b7a2467e58109c92d799d059f508f35853d0bff7)
$ git stash list
stash@{0}: WIP on master: c050772... small java change
stash@{1}: WIP on master: c050772... small java change
stash@{2}: WIP on master: c050772... small java change
stash@{3}: WIP on master: c050772... small java change
-brandon
git-stash.sh | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index d799c76..6bd2572 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -144,7 +144,14 @@ show_stash () {
then
flags=--stat
fi
- s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
+
+ if test $# = 0
+ then
+ set x "$ref_stash@{0}"
+ shift
+ fi
+
+ s=$(git rev-parse --revs-only --no-flags "$@")
w_commit=$(git rev-parse --verify "$s") &&
b_commit=$(git rev-parse --verify "$s^") &&
@@ -163,13 +170,19 @@ apply_stash () {
shift
esac
+ if test $# = 0
+ then
+ set x "$ref_stash@{0}"
+ shift
+ fi
+
# current index state
c_tree=$(git write-tree) ||
die 'Cannot apply a stash in the middle of a merge'
# stash records the work tree, and is a merge between the
# base commit (first parent) and the index tree (second parent).
- s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+ s=$(git rev-parse --revs-only --no-flags "$@") &&
w_tree=$(git rev-parse --verify "$s:") &&
b_tree=$(git rev-parse --verify "$s^1:") &&
i_tree=$(git rev-parse --verify "$s^2:") ||
--
1.6.0.1.244.gdc19
^ permalink raw reply related
* Re: [PATCH v2 4/4] diff.c: convert builtin funcname patterns to non-GNU extended regex syntax
From: Brandon Casey @ 2008-09-24 0:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <i-l9YX2TO45e2OB9LuoxrAN6a2iFYaH_eEGlVmRsP0oa97XuwX4eGQ@cipher.nrlssc.navy.mil>
Brandon Casey wrote:
> I'll do some testing on non-GNU platforms today.
Well, all I've done is compile, and it compiles and runs the
tests correctly. Thought I'd let you know I did at least that.
-brandon
^ permalink raw reply
* Re: [PATCH 1/3] Prepare for non-interactive merge-preserving rebase
From: SZEDER Gábor @ 2008-09-24 0:10 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Stephen Haberman, Git Mailing List, Junio C Hamano
In-Reply-To: <48D95FE1.30200@op5.se>
Hi Andreas,
First of all, thanks for the work!
On Tue, Sep 23, 2008 at 11:30:09PM +0200, Andreas Ericsson wrote:
> It requires a bit of testing though. All the t/t34* tests pass with
> all the patches applied, and some manual tries worked just fine too,
> but if you wanna give it a twirl where you work, that'd be great.
Unfortunately in my example workflow[1] posted earlier today your
patch series does not work in the way I would like it to behave.
The following DAG is created by the commands below:
-A---B master
\
C---M topic
\ /
D
git init
echo 1 >foo
git add foo
git commit -m 'first on master' # A
echo 2 >>foo
git commit -m 'second on master' foo # B
git checkout -b topic HEAD^
echo 1 >bar
git add bar
git commit -m 'first on topic' # C
git checkout -b subtopic
echo 1 >baz
git add baz
git commit -m 'first on subtopic' # D
git checkout topic
git merge --no-ff subtopic # M
If I now execute 'git rebase -p master topic', I get the following:
-A---B master
\ \
\ C'---M' topic
\ /
C----D
But I would rather like to have the following:
-A---B master
\
C'---M' topic
\ /
D'
Would such a behaviour possible at all?
Thanks,
Gábor
[1] http://article.gmane.org/gmane.comp.version-control.git/96548
^ permalink raw reply
* [RFC PATCH v2] fetch-pack: log(n)-transmission find_common()
From: Thomas Rast @ 2008-09-24 0:48 UTC (permalink / raw)
To: git; +Cc: Clemens Buchacher
In-Reply-To: <1221692484-21977-1-git-send-email-trast@student.ethz.ch>
Replaces the existing simple history search with a more sophisticated
algorithm:
1) Walk history with exponentially increasing stride lengths; i.e.,
send the 1st commit, then the 2nd after that, then the 4th after
that, and so on.
2) Bisect the resulting intervals.
Combined with tracking the "outstanding haves" so that we can detect
which sha1s were never ACKed by upload-pack (and hence not common),
this gives O(log(n)) required "have" lines.
Unfortunately this cannot work if the server sends fake ACKs, so we
introduce a capability 'exp-stride' which instructs upload-pack to
disable ok_to_give_up(). (Which incidentally saves the server a lot
of work.)
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
So, after the resounding success that v1 of this patch was, I went
through and actually made it work, or at least I hope so.
I eventually gave up on trying to understand the exact interaction of
mark_common(), which sometimes pushes, and rev_list_push(), which
sometimes marks, and just rewrote them to each only do one thing. I
also rewrote the old strategy in terms of the new get_rev_stride
(always keeping stride length at 1) because that was far less work
than integrating the existing code with the new one.
For the timings below, I ran the (new) daemon locally, with two repos:
- a 'clone --mirror' of my own git.git, with a branch 'next' 525
commits from the above cc185a6
- a 'clone --mirror' of the completely unrelated adps.git, which
shares no objects with git.git
Some not really statistically sound timings, usually best of 3-5:
## A stripped down git.git, with only one branch and no tags
$ git branch -a
* master
$ git tag
$ git log -1 --pretty=oneline master
cc185a6a8ac24737a26ec4b40cc401c2db8b2e97 Start draft release notes for 1.6.0.3
## I still have the old (dashed) forms in /usr/bin from RPMs
$ /usr/bin/git --version
git version 1.5.6
## (1a) "Updating" from the git.git that is further ahead (OLD)
$ time /usr/bin/git-fetch-pack -k git://localhost/git next
[...]
real 0m1.004s
user 0m0.228s
sys 0m0.028s
## (1b) "Updating" from the git.git that is further ahead (NEW)
$ time git fetch-pack -k git://localhost/git next
[...]
real 0m0.977s
user 0m0.208s
sys 0m0.068s
## (2a) Fetching the unrelated repo from scratch (OLD)
$ time /usr/bin/git-fetch-pack -k git://localhost/adps master
[...]
real 0m9.560s
user 0m0.720s
sys 0m0.128s
## (2b) Fetching the unrelated repo from scratch (NEW)
$ time git fetch-pack -k git://localhost/adps master
[...]
real 0m0.596s
user 0m0.372s
sys 0m0.008s
## (3a) Fetching over the network from real repo (OLD)
$ time /usr/bin/git-fetch-pack git://git.kernel.org/pub/scm/git/git.git next
[...]
user 0m0.528s
sys 0m0.136s
## (3b) Fetching over the network from real repo (NEW)
$ time git fetch-pack git://git.kernel.org/pub/scm/git/git.git next
[...]
user 0m0.540s
sys 0m0.180s
## Add more refs to make it more interesting
$ git remote add origin ~/dev/git
$ git fetch --tags origin
## (4a) like 1a, but with lots of tags
$ time /usr/bin/git-fetch-pack -k git://localhost/git next
[...]
real 0m1.075s
user 0m0.452s
sys 0m0.048s
## (4b) like 1b, but with lots of tags
$ time git fetch-pack -k git://localhost/git next
[...]
real 0m0.837s
user 0m0.236s
sys 0m0.040s
Clearly, this approach does solve the issue I mentioned in the
motivation earlier in the thread, where the initial fetch of
completely disjoint repositories takes _ages_. Somewhat to my own
surprise, it seems to do quite well in _all_ cases even though it
aggressively digs through history.
Note, however, that the timings aren't really solid; the final write
of the pack, which I assume is fsync()ed, seems to have a big impact
on the execution time. Test (3) obviously depends more on network
performance than anything else; and it uses the old (linear)
algorithm, but in the new implementation.
I'd appreciate testers and eyeballs on the patch. I'll think about
the ref heads "reduced DAG" again, to possibly shave off some more
(client side cpu) time when I get the time, but that likely won't be
in the next few days.
- Thomas
PS: Thanks drizzd for the discussion of the topic on IRC.
PPS: Junio, enjoy your holidays :-)
builtin-fetch-pack.c | 477 +++++++++++++++++++++++++++++++++++++++++---------
upload-pack.c | 7 +-
2 files changed, 400 insertions(+), 84 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 4dfef29..721fe08 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -25,6 +25,12 @@ static const char fetch_pack_usage[] =
#define COMMON_REF (1U << 2)
#define SEEN (1U << 3)
#define POPPED (1U << 4)
+#define BISECTED_FW (1U << 5)
+#define BISECTED_BW (1U << 6)
+#define NOTCOMMON (1U << 7)
+#define HAS_INFO (1U << 8)
+#define CLEAN_MARKS (COMMON | COMMON_REF | SEEN | POPPED | BISECTED_FW \
+ | BISECTED_BW | NOTCOMMON)
static int marked;
@@ -34,133 +40,412 @@ static int marked;
*/
#define MAX_IN_VAIN 256
-static struct commit_list *rev_list;
-static int non_common_revs, multi_ack, use_sideband;
+struct work_list
+{
+ struct work_list *next;
+ struct commit *commit;
+ unsigned int stride; /* current stride length between commits */
+ unsigned int steps; /* steps left to go before emitting again */
+ unsigned int depth; /* sideline depth */
+ unsigned int bisect;
+};
+
+/* "recursion stack" of our strategy of choosing sha1s to transmit */
+static struct work_list *work_list = NULL;
-static void rev_list_push(struct commit *commit, int mark)
+/* list of commits not ACKed yet */
+static struct commit_list *outstanding = NULL;
+static struct commit_list *outstanding_end = NULL;
+
+struct commit_info
{
- if (!(commit->object.flags & mark)) {
- commit->object.flags |= mark;
+ struct commit_list *children;
+};
- if (!(commit->object.parsed))
- if (parse_commit(commit))
- return;
- insert_by_date(commit, &rev_list);
+static int non_common_revs = 1, multi_ack, use_sideband, exp_stride_algorithm;
- if (!(commit->object.flags & COMMON))
- non_common_revs++;
+static struct commit_info *get_commit_info(struct commit *commit)
+{
+ if (commit->object.flags & HAS_INFO)
+ return commit->util;
+
+ struct commit_info *info = xmalloc(sizeof(struct commit_info));
+ info->children = NULL;
+ commit->util = info;
+ commit->object.flags |= HAS_INFO;
+ return info;
+}
+
+static void work_list_insert(struct work_list *work_item)
+{
+ struct work_list **pp = &work_list;
+ struct work_list *p;
+
+ while ((p = *pp)) {
+ if (p->bisect || work_item->bisect) {
+ if (p->bisect > work_item->bisect)
+ break;
+ } else {
+ if (p->depth > work_item->depth)
+ break;
+ else if (p->commit->date < work_item->commit->date)
+ break;
+ }
+
+ pp = &p->next;
}
+
+ work_item->next = p;
+ *pp = work_item;
}
-static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static void add_child(struct commit *parent, struct commit *child)
+{
+ struct commit_info *info = get_commit_info(parent);
+ struct commit_list *item;
+ for (item = info->children; item; item = item->next)
+ if (item->item == child)
+ break;
+ if (!item)
+ commit_list_insert(child, &(info->children));
+}
+
+
+/*
+ Marks all (known) children of this commit NOTCOMMON. We call this
+ for all sha1s the server did not ACK.
+*/
+
+static void mark_not_common(struct commit *commit)
+{
+ struct commit_info *info = get_commit_info(commit);
+ struct commit_list *child;
+
+ assert(!(commit->object.flags & COMMON));
+
+ if (commit->object.flags & NOTCOMMON)
+ return;
+
+ commit->object.flags |= NOTCOMMON;
+
+ for (child = info->children; child; child = child->next)
+ mark_not_common(child->item);
+}
+
+
+static void check_parsed_and_mark(struct commit *commit)
+{
+ struct commit_list *parent;
+
+ if (!commit && commit->object.parsed)
+ return;
+
+ parse_commit(commit);
+
+ for (parent = commit->parents; parent; parent = parent->next) {
+ add_child(parent->item, commit);
+ if (parent->item->object.flags & NOTCOMMON)
+ mark_not_common(commit);
+ }
+}
+
+
+static void rev_list_push(struct commit *commit, unsigned int stride,
+ unsigned int steps, unsigned int depth)
+{
+ struct work_list *work_item;
+
+ check_parsed_and_mark(commit);
+
+ work_item = xmalloc(sizeof(struct work_list));
+ work_item->commit = commit;
+ work_item->stride = stride;
+ work_item->steps = steps;
+ work_item->depth = depth;
+ work_item->bisect = 0;
+ work_list_insert(work_item);
+}
+
+static int rev_list_insert_ref(const char *path, const unsigned char *sha1,
+ int flag, void *cb_data)
{
struct object *o = deref_tag(parse_object(sha1), path, 0);
- if (o && o->type == OBJ_COMMIT)
- rev_list_push((struct commit *)o, SEEN);
+ if (o && o->type == OBJ_COMMIT && !(o->flags & SEEN))
+ rev_list_push((struct commit *)o, 1, 1, 0);
return 0;
}
-static int clear_marks(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int clear_marks(const char *path, const unsigned char *sha1, int flag,
+ void *cb_data)
{
struct object *o = deref_tag(parse_object(sha1), path, 0);
if (o && o->type == OBJ_COMMIT)
- clear_commit_marks((struct commit *)o,
- COMMON | COMMON_REF | SEEN | POPPED);
+ clear_commit_marks((struct commit *)o, CLEAN_MARKS);
return 0;
}
+
/*
- This function marks a rev and its ancestors as common.
- In some cases, it is desirable to mark only the ancestors (for example
- when only the server does not yet know that they are common).
+ Mark commits backwards through history as COMMON.
*/
-static void mark_common(struct commit *commit,
- int ancestors_only, int dont_parse)
+static void mark_common(struct commit *commit, int ancestors_only, int dont_parse)
{
- if (commit != NULL && !(commit->object.flags & COMMON)) {
- struct object *o = (struct object *)commit;
+ struct commit_list *parents;
+ struct object *o;
- if (!ancestors_only)
- o->flags |= COMMON;
+ if (commit == NULL || commit->object.flags & (COMMON_REF|COMMON))
+ return;
- if (!(o->flags & SEEN))
- rev_list_push(commit, SEEN);
- else {
- struct commit_list *parents;
-
- if (!ancestors_only && !(o->flags & POPPED))
- non_common_revs--;
- if (!o->parsed && !dont_parse)
- if (parse_commit(commit))
- return;
-
- for (parents = commit->parents;
- parents;
- parents = parents->next)
- mark_common(parents->item, 0, dont_parse);
- }
+ if (!dont_parse && !(commit->object.parsed))
+ check_parsed_and_mark(commit);
+
+ o = (struct object *)commit;
+
+ if (!ancestors_only)
+ o->flags |= COMMON;
+
+ for (parents = commit->parents; parents; parents = parents->next) {
+ add_child(parents->item, commit);
+ mark_common(parents->item, 0, dont_parse);
}
}
+
+
/*
- Get the next rev to send, ignoring the common.
+ Queue all revs in the list for simple backward search. Does not
+ requeue already SEEN commits.
*/
-static const unsigned char* get_rev(void)
+static void queue_list(struct commit *child, struct commit_list *list,
+ unsigned int stride, unsigned int steps,
+ unsigned int depth)
{
- struct commit *commit = NULL;
+ while (list) {
+ add_child(list->item, child);
+ if (!(list->item->object.flags & SEEN))
+ rev_list_push(list->item, stride, steps, depth);
+ list = list->next;
+ }
+}
+
+
+/*
+ Queues a commit for bisection
+*/
+
+static void setup_bisect(struct commit *commit, int depth)
+{
+ struct work_list *work_item = xmalloc(sizeof(struct work_list));
+
+ work_item->commit = commit;
+ work_item->bisect = 1+depth;
+ work_item->depth = 0;
+ work_list_insert(work_item);
+}
+
+
+/*
+ Queues a list of commits for bisection
+*/
+
+static void setup_bisect_all (struct commit_list* list, int depth)
+{
+ while (list) {
+ setup_bisect(list->item, depth);
+ list = list->next;
+ }
+}
+
+
+/*
+ Backwards search. If we use exp_stride_algorithm, it takes
+ exponential strides between commits chosen.
+*/
+
+static struct commit* get_rev_stride(struct work_list *work_item)
+{
+ struct commit *commit = work_item->commit;
+ unsigned int steps = work_item->steps;
+
+ while (steps++ < work_item->stride-1 && commit->parents
+ && !(commit->object.flags & (SEEN|COMMON|COMMON_REF))) {
+ /* all but the first parent line are queued for later */
+ if (commit->parents)
+ queue_list(commit, commit->parents->next,
+ work_item->stride, steps,
+ work_item->depth+1);
+
+ /* follow the first parent line directly */
+ commit->object.flags |= SEEN;
+ add_child(commit->parents->item, commit);
+ commit = commit->parents->item;
+ check_parsed_and_mark(commit);
+ }
+
+ if (commit->object.flags & (SEEN|COMMON)) {
+ /* already been here! */
+ commit = NULL;
+ } else if (commit->object.flags & COMMON_REF) {
+ /* this came from a ref; we stop here, but need to
+ * emit it so the server knows too */
+ commit->object.flags |= POPPED|SEEN|BISECTED_BW;
+ if (exp_stride_algorithm)
+ setup_bisect(commit, 0);
+ } else {
+ /* usual case: this is news to us, so we scan further
+ * back. note that the first-depth bisection only
+ * needs to go forward */
+ commit->object.flags |= POPPED|SEEN|BISECTED_BW;
+ queue_list(commit, commit->parents,
+ exp_stride_algorithm ? work_item->stride*2 : 1,
+ 0, work_item->depth);
+ if (exp_stride_algorithm)
+ setup_bisect(commit, 0);
+ }
+
+ free(work_item);
+
+ return commit;
+}
- while (commit == NULL) {
- unsigned int mark;
- struct commit_list *parents;
- if (rev_list == NULL || non_common_revs == 0)
+/*
+ Bisection state of the exponential stride algorithm: scans forward
+ (backward) for a non-common/already-sent (common/already-sent,
+ resp.) commit, then emits the middle of that range, and re-queues.
+
+ This means we bisect the discovered range in log(n) transmissions,
+ although we do make O(n) steps through history.
+*/
+
+static struct commit* get_rev_bisect(struct work_list *work_item)
+{
+ struct commit *full;
+ struct commit *half;
+ int half_step;
+ struct commit_info *info;
+ unsigned int flags = work_item->commit->object.flags;
+
+ if (!(flags & (COMMON|BISECTED_BW))) {
+ /* We don't know anything about the history _backward_
+ * from this, so search it */
+
+ full = work_item->commit;
+ full->object.flags |= BISECTED_BW;
+ half = work_item->commit;
+ half_step = 0;
+
+ while (full && full->parents) {
+ full = full->parents->item;
+ half_step ^= 1;
+ if (half_step)
+ half = half->parents->item;
+ if (full->object.flags & (POPPED|COMMON))
+ break;
+ }
+
+ /* also insert the same bisection again so we can try
+ * forward too */
+ work_list_insert(work_item);
+
+ if (full->object.flags & POPPED
+ && !(full->object.flags & NOTCOMMON)
+ && !(half->object.flags & (COMMON|POPPED))) {
+ setup_bisect(half, work_item->bisect);
+ half->object.flags |= POPPED;
+ return half;
+ } else {
return NULL;
+ }
+ }
- commit = rev_list->item;
- if (!commit->object.parsed)
- parse_commit(commit);
- parents = commit->parents;
-
- commit->object.flags |= POPPED;
- if (!(commit->object.flags & COMMON))
- non_common_revs--;
-
- if (commit->object.flags & COMMON) {
- /* do not send "have", and ignore ancestors */
- commit = NULL;
- mark = COMMON | SEEN;
- } else if (commit->object.flags & COMMON_REF)
- /* send "have", and ignore ancestors */
- mark = COMMON | SEEN;
- else
- /* send "have", also for its ancestors */
- mark = SEEN;
-
- while (parents) {
- if (!(parents->item->object.flags & SEEN))
- rev_list_push(parents->item, mark);
- if (mark & COMMON)
- mark_common(parents->item, 1, 0);
- parents = parents->next;
+ if (!(flags & (NOTCOMMON|BISECTED_FW))) {
+ /* We don't know anything about the history _forward_
+ * from this, so search it */
+
+ full = work_item->commit;
+ full->object.flags |= BISECTED_FW;
+ half = work_item->commit;
+ half_step = 0;
+
+ while (full && (info=get_commit_info(full))->children) {
+ setup_bisect_all(info->children->next, work_item->bisect);
+ full = info->children->item;
+ half_step ^= 1;
+ if (half_step) {
+ info = get_commit_info(half);
+ half = info->children->item;
+ }
+ if (full->object.flags & (POPPED|NOTCOMMON))
+ break;
}
- rev_list = rev_list->next;
+ if (full->object.flags & POPPED
+ && !(full->object.flags & COMMON)
+ && !(half->object.flags & (NOTCOMMON|POPPED))) {
+ setup_bisect(half, work_item->bisect);
+ half->object.flags |= POPPED;
+ free(work_item);
+ return half;
+ }
}
- return commit->object.sha1;
+ free(work_item);
+ return NULL;
}
+
+/*
+ Get the next revision to send.
+*/
+
+static struct commit *get_rev(void)
+{
+ struct commit *commit = NULL;
+
+ while (commit == NULL && work_list) {
+ struct work_list *work_item = NULL;
+
+ work_item = work_list;
+ /* we update the pointer early so the get_rev
+ * functions can free() or reuse the work_item as
+ * required */
+ work_list = work_item->next;
+
+ commit = work_item->commit;
+
+ check_parsed_and_mark(commit);
+
+ if (work_item->bisect) {
+ commit = get_rev_bisect(work_item);
+ } else {
+ commit = get_rev_stride(work_item);
+ }
+ }
+
+ return commit;
+}
+
+static void pop_outstanding()
+{
+ struct commit_list *item = outstanding;
+ outstanding = item->next;
+ free(item);
+}
+
+
static int find_common(int fd[2], unsigned char *result_sha1,
struct ref *refs)
{
int fetching;
int count = 0, flushes = 0, retval;
+ struct commit *commit;
const unsigned char *sha1;
unsigned in_vain = 0;
int got_continue = 0;
@@ -192,7 +477,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
}
if (!fetching)
- packet_write(fd[1], "want %s%s%s%s%s%s%s%s\n",
+ packet_write(fd[1], "want %s%s%s%s%s%s%s%s%s\n",
sha1_to_hex(remote),
(multi_ack ? " multi_ack" : ""),
(use_sideband == 2 ? " side-band-64k" : ""),
@@ -200,6 +485,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
(args.use_thin_pack ? " thin-pack" : ""),
(args.no_progress ? " no-progress" : ""),
(args.include_tag ? " include-tag" : ""),
+ (exp_stride_algorithm ? " exp-stride" : ""),
" ofs-delta");
else
packet_write(fd[1], "want %s\n", sha1_to_hex(remote));
@@ -243,13 +529,25 @@ static int find_common(int fd[2], unsigned char *result_sha1,
flushes = 0;
retval = -1;
- while ((sha1 = get_rev())) {
+ while ((commit = get_rev())) {
+ sha1 = commit->object.sha1;
packet_write(fd[1], "have %s\n", sha1_to_hex(sha1));
if (args.verbose)
fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
in_vain++;
+
+ if (outstanding) {
+ commit_list_insert(commit, &(outstanding_end->next));
+ outstanding_end = outstanding_end->next;
+ } else {
+ commit_list_insert(commit, &outstanding);
+ outstanding_end = outstanding;
+ }
+
if (!(31 & ++count)) {
int ack;
+ int unwound = 0;
+ struct commit_list *item;
packet_flush(fd[1]);
flushes++;
@@ -274,12 +572,23 @@ static int find_common(int fd[2], unsigned char *result_sha1,
} else if (ack == 2) {
struct commit *commit =
lookup_commit(result_sha1);
+ while (commit != outstanding->item) {
+ mark_not_common(outstanding->item);
+ pop_outstanding();
+ unwound++;
+ }
+ pop_outstanding();
+ unwound++;
mark_common(commit, 0, 1);
retval = 0;
in_vain = 0;
got_continue = 1;
}
} while (ack);
+ while (unwound++ < 32) {
+ mark_not_common(outstanding->item);
+ pop_outstanding();
+ }
flushes--;
if (got_continue && MAX_IN_VAIN < in_vain) {
if (args.verbose)
@@ -445,9 +754,8 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
continue;
if (!(o->flags & SEEN)) {
- rev_list_push((struct commit *)o, COMMON_REF | SEEN);
-
- mark_common((struct commit *)o, 1, 1);
+ rev_list_push((struct commit *)o, 1, 0, 0);
+ o->flags |= COMMON_REF;
}
}
@@ -597,6 +905,11 @@ static struct ref *do_fetch_pack(int fd[2],
fprintf(stderr, "Server supports side-band\n");
use_sideband = 1;
}
+ if (server_supports("exp-stride")) {
+ if (args.verbose)
+ fprintf(stderr, "Server supports exp-stride\n");
+ exp_stride_algorithm = 1;
+ }
if (everything_local(&ref, nr_match, match)) {
packet_flush(fd[1]);
goto all_done;
diff --git a/upload-pack.c b/upload-pack.c
index e5adbc0..14012ee 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -37,6 +37,7 @@ static unsigned int timeout;
*/
static int use_sideband;
static int debug_fd;
+static int exp_stride_algorithm;
static void reset_timeout(void)
{
@@ -414,7 +415,7 @@ static int get_common_commits(void)
if (!prefixcmp(line, "have ")) {
switch (got_sha1(line+5, sha1)) {
case -1: /* they have what we do not */
- if (multi_ack && ok_to_give_up())
+ if (multi_ack && !exp_stride_algorithm && ok_to_give_up())
packet_write(1, "ACK %s continue\n",
sha1_to_hex(sha1));
break;
@@ -501,6 +502,8 @@ static void receive_needs(void)
no_progress = 1;
if (strstr(line+45, "include-tag"))
use_include_tag = 1;
+ if (strstr(line+45, "exp-stride"))
+ exp_stride_algorithm = 1;
/* We have sent all our refs already, and the other end
* should have chosen out of them; otherwise they are
@@ -573,7 +576,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
{
static const char *capabilities = "multi_ack thin-pack side-band"
" side-band-64k ofs-delta shallow no-progress"
- " include-tag";
+ " include-tag exp-stride";
struct object *o = parse_object(sha1);
if (!o)
--
tg: (1293c95..) t/fetch-pack-speedup (depends on: origin/master)
^ permalink raw reply related
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