* Re: thoughts on setting core.logAllRefUpdates default true for bare repos
From: Junio C Hamano @ 2009-11-04 19:49 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Johannes Schindelin, Sitaram Chamarty, git
In-Reply-To: <vpqzl729j72.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> I can't think of any, and I just tried launching two
>
> while true; do git pull; date > foo.txt ; git add .; git commit -m "xxx"; git push; done
>
> in parallel, with two different users pushing to a --bare --shared
> repository, and it did work well. But I may very well have missed
> something.
>
> (and actually, if it causes problem, it's an argument in favor of
> defaulting to false when core.shared is true, not when core.bare).
>
> Unless I missed something, I think core.logAllRefUpdates should be
> enabled for bare repos.
Your experiment justifies "could be enabled safely" and saying "should be"
based on that is a bit too strong and also premature.
The single most common reason why a bare repository is bare is because
nobody regularly logs in to the machine that hosts it and goes there to
access its contents. As reflog is a local thing, and not exposed to
outside world, enabling it alone would not help a lot to people who do not
have such a direct access to the bare repository, which by definition is
the majority because the reason why the repository is bare to begin with.
Once we add ways to expose information kept in reflog of a bare repository
more effectively and conveniently, the argument could become "should be
enabled now it would be very useful to have one".
I mentioned a possible option to show reflog entry annotations in gitweb.
That was an example of such an addition of "a way to expose".
It also is plausible to teach git-daemon a remote "log" request; similar
to "git fetch" running "upload-pack" on the other end in the bare
repository, a "git log --remote" command may run "upload-log" on the other
end and this service may allow passing the "-g" option when it does so.
That would be another addition of "a way to expose".
^ permalink raw reply
* [PATCH v2 01/13] Fix memory leak in helper method for disconnect
From: Sverre Rabbelier @ 2009-11-04 19:48 UTC (permalink / raw)
To: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
Cc: Daniel Barkalow
In-Reply-To: <1257364098-1685-1-git-send-email-srabbelier@gmail.com>
From: Daniel Barkalow <barkalow@iabervon.org>
Since some cases may need to disconnect from the helper and reconnect,
wrap the function that just disconnects in a function that also frees
transport->data.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
Moved to the beginning of the series as it is most important.
transport-helper.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index f57e84c..e24fcbb 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -67,6 +67,13 @@ static int disconnect_helper(struct transport *transport)
return 0;
}
+static int release_helper(struct transport *transport)
+{
+ disconnect_helper(transport);
+ free(transport->data);
+ return 0;
+}
+
static int fetch_with_fetch(struct transport *transport,
int nr_heads, const struct ref **to_fetch)
{
@@ -163,6 +170,6 @@ int transport_helper_init(struct transport *transport, const char *name)
transport->data = data;
transport->get_refs_list = get_refs_list;
transport->fetch = fetch;
- transport->disconnect = disconnect_helper;
+ transport->disconnect = release_helper;
return 0;
}
--
1.6.5.2.295.g0d105
^ permalink raw reply related
* [PATCH v2 02/13] Allow programs to not depend on remotes having urls
From: Sverre Rabbelier @ 2009-11-04 19:48 UTC (permalink / raw)
To: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
Cc: Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <1257364098-1685-1-git-send-email-srabbelier@gmail.com>
From: Daniel Barkalow <barkalow@iabervon.org>
For fetch and ls-remote, which use the first url of a remote, have
transport_get() determine this by passing a remote and passing NULL
for the url. For push, which uses every url of a remote, use each url
in turn if there are any, and use NULL if there are none.
This will allow the transport code to do something different if the
location is not specified with a url.
Also, have the message for a fetch say "foreign" if there is no url.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged.
builtin-fetch.c | 7 +++-
builtin-ls-remote.c | 4 +-
builtin-push.c | 68 +++++++++++++++++++++++++++++++-------------------
transport.c | 3 ++
4 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index a35a6f8..013a6ba 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -309,7 +309,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
if (!fp)
return error("cannot open %s: %s\n", filename, strerror(errno));
- url = transport_anonymize_url(raw_url);
+ if (raw_url)
+ url = transport_anonymize_url(raw_url);
+ else
+ url = xstrdup("foreign");
for (rm = ref_map; rm; rm = rm->next) {
struct ref *ref = NULL;
@@ -704,7 +707,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
if (!remote)
die("Where do you want to fetch from today?");
- transport = transport_get(remote, remote->url[0]);
+ transport = transport_get(remote, NULL);
if (verbosity >= 2)
transport->verbose = 1;
if (verbosity < 0)
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..4c6fc58 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -87,9 +87,9 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
}
}
remote = nongit ? NULL : remote_get(dest);
- if (remote && !remote->url_nr)
+ if (!nongit && !remote)
die("remote %s has no configured URL", dest);
- transport = transport_get(remote, remote ? remote->url[0] : dest);
+ transport = transport_get(remote, remote ? NULL : dest);
if (uploadpack != NULL)
transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
diff --git a/builtin-push.c b/builtin-push.c
index 8631c06..88dd9f5 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -88,6 +88,36 @@ static void setup_default_push_refspecs(void)
}
}
+static int push_with_options(struct transport *transport, int flags)
+{
+ int err;
+ int nonfastforward;
+ if (receivepack)
+ transport_set_option(transport,
+ TRANS_OPT_RECEIVEPACK, receivepack);
+ if (thin)
+ transport_set_option(transport, TRANS_OPT_THIN, "yes");
+
+ if (flags & TRANSPORT_PUSH_VERBOSE)
+ fprintf(stderr, "Pushing to %s\n", transport->url);
+ err = transport_push(transport, refspec_nr, refspec, flags,
+ &nonfastforward);
+ err |= transport_disconnect(transport);
+
+ if (!err)
+ return 0;
+
+ error("failed to push some refs to '%s'", transport->url);
+
+ if (nonfastforward && advice_push_nonfastforward) {
+ printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
+ "Merge the remote changes before pushing again. See the 'non-fast forward'\n"
+ "section of 'git push --help' for details.\n");
+ }
+
+ return 1;
+}
+
static int do_push(const char *repo, int flags)
{
int i, errs;
@@ -136,33 +166,19 @@ static int do_push(const char *repo, int flags)
url = remote->url;
url_nr = remote->url_nr;
}
- for (i = 0; i < url_nr; i++) {
- struct transport *transport =
- transport_get(remote, url[i]);
- int err;
- int nonfastforward;
- if (receivepack)
- transport_set_option(transport,
- TRANS_OPT_RECEIVEPACK, receivepack);
- if (thin)
- transport_set_option(transport, TRANS_OPT_THIN, "yes");
-
- if (flags & TRANSPORT_PUSH_VERBOSE)
- fprintf(stderr, "Pushing to %s\n", url[i]);
- err = transport_push(transport, refspec_nr, refspec, flags,
- &nonfastforward);
- err |= transport_disconnect(transport);
-
- if (!err)
- continue;
-
- error("failed to push some refs to '%s'", url[i]);
- if (nonfastforward && advice_push_nonfastforward) {
- printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
- "Merge the remote changes before pushing again. See the 'non-fast forward'\n"
- "section of 'git push --help' for details.\n");
+ if (url_nr) {
+ for (i = 0; i < url_nr; i++) {
+ struct transport *transport =
+ transport_get(remote, url[i]);
+ if (push_with_options(transport, flags))
+ errs++;
}
- errs++;
+ } else {
+ struct transport *transport =
+ transport_get(remote, NULL);
+
+ if (push_with_options(transport, flags))
+ errs++;
}
return !!errs;
}
diff --git a/transport.c b/transport.c
index 644a30a..9daa686 100644
--- a/transport.c
+++ b/transport.c
@@ -813,6 +813,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
struct transport *ret = xcalloc(1, sizeof(*ret));
ret->remote = remote;
+
+ if (!url && remote && remote->url)
+ url = remote->url[0];
ret->url = url;
if (!prefixcmp(url, "rsync:")) {
--
1.6.5.2.295.g0d105
^ permalink raw reply related
* [PATCH v2 04/13] Allow fetch to modify refs
From: Sverre Rabbelier @ 2009-11-04 19:48 UTC (permalink / raw)
To: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
Cc: Daniel Barkalow, Sverre Rabbelier
In-Reply-To: <1257364098-1685-1-git-send-email-srabbelier@gmail.com>
From: Daniel Barkalow <barkalow@iabervon.org>
This allows the transport to use the null sha1 for a ref reported to
be present in the remote repository to indicate that a ref exists but
its actual value is presently unknown and will be set if the objects
are fetched.
Also adds documentation to the API to specify exactly what the methods
should do and how they should interpret arguments.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Now only changes constness (changes to line 526-257 removed).
builtin-clone.c | 3 ++-
transport-helper.c | 4 ++--
transport.c | 13 +++++++------
transport.h | 41 +++++++++++++++++++++++++++++++++++++++--
4 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index caf3025..5df8b0f 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -362,9 +362,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
const char *repo_name, *repo, *work_tree, *git_dir;
char *path, *dir;
int dest_exists;
- const struct ref *refs, *remote_head, *mapped_refs;
+ const struct ref *refs, *remote_head;
const struct ref *remote_head_points_at;
const struct ref *our_head_points_at;
+ struct ref *mapped_refs;
struct strbuf key = STRBUF_INIT, value = STRBUF_INIT;
struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
struct transport *transport = NULL;
diff --git a/transport-helper.c b/transport-helper.c
index e24fcbb..53d8f08 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -75,7 +75,7 @@ static int release_helper(struct transport *transport)
}
static int fetch_with_fetch(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct child_process *helper = get_helper(transport);
FILE *file = xfdopen(helper->out, "r");
@@ -99,7 +99,7 @@ static int fetch_with_fetch(struct transport *transport,
}
static int fetch(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct helper_data *data = transport->data;
int i, count;
diff --git a/transport.c b/transport.c
index 9daa686..5ae8db6 100644
--- a/transport.c
+++ b/transport.c
@@ -204,7 +204,7 @@ static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)
}
static int fetch_objs_via_rsync(struct transport *transport,
- int nr_objs, const struct ref **to_fetch)
+ int nr_objs, struct ref **to_fetch)
{
struct strbuf buf = STRBUF_INIT;
struct child_process rsync;
@@ -408,7 +408,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct bundle_transport_data *data = transport->data;
return unbundle(&data->header, data->fd);
@@ -486,7 +486,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
}
static int fetch_refs_via_pack(struct transport *transport,
- int nr_heads, const struct ref **to_fetch)
+ int nr_heads, struct ref **to_fetch)
{
struct git_transport_data *data = transport->data;
char **heads = xmalloc(nr_heads * sizeof(*heads));
@@ -926,16 +926,17 @@ const struct ref *transport_get_remote_refs(struct transport *transport)
return transport->remote_refs;
}
-int transport_fetch_refs(struct transport *transport, const struct ref *refs)
+int transport_fetch_refs(struct transport *transport, struct ref *refs)
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
- const struct ref **heads = NULL;
- const struct ref *rm;
+ struct ref **heads = NULL;
+ struct ref *rm;
for (rm = refs; rm; rm = rm->next) {
nr_refs++;
if (rm->peer_ref &&
+ !is_null_sha1(rm->old_sha1) &&
!hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))
continue;
ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
diff --git a/transport.h b/transport.h
index c14da6f..503db11 100644
--- a/transport.h
+++ b/transport.h
@@ -18,11 +18,48 @@ struct transport {
int (*set_option)(struct transport *connection, const char *name,
const char *value);
+ /**
+ * Returns a list of the remote side's refs. In order to allow
+ * the transport to try to share connections, for_push is a
+ * hint as to whether the ultimate operation is a push or a fetch.
+ *
+ * If the transport is able to determine the remote hash for
+ * the ref without a huge amount of effort, it should store it
+ * in the ref's old_sha1 field; otherwise it should be all 0.
+ **/
struct ref *(*get_refs_list)(struct transport *transport, int for_push);
- int (*fetch)(struct transport *transport, int refs_nr, const struct ref **refs);
+
+ /**
+ * Fetch the objects for the given refs. Note that this gets
+ * an array, and should ignore the list structure.
+ *
+ * If the transport did not get hashes for refs in
+ * get_refs_list(), it should set the old_sha1 fields in the
+ * provided refs now.
+ **/
+ int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
+
+ /**
+ * Push the objects and refs. Send the necessary objects, and
+ * then, for any refs where peer_ref is set and
+ * peer_ref->new_sha1 is different from old_sha1, tell the
+ * remote side to update each ref in the list from old_sha1 to
+ * peer_ref->new_sha1.
+ *
+ * Where possible, set the status for each ref appropriately.
+ *
+ * The transport must modify new_sha1 in the ref to the new
+ * value if the remote accepted the change. Note that this
+ * could be a different value from peer_ref->new_sha1 if the
+ * process involved generating new commits.
+ **/
int (*push_refs)(struct transport *transport, struct ref *refs, int flags);
int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
+ /** get_refs_list(), fetch(), and push_refs() can keep
+ * resources (such as a connection) reserved for futher
+ * use. disconnect() releases these resources.
+ **/
int (*disconnect)(struct transport *connection);
char *pack_lockfile;
signed verbose : 2;
@@ -74,7 +111,7 @@ int transport_push(struct transport *connection,
const struct ref *transport_get_remote_refs(struct transport *transport);
-int transport_fetch_refs(struct transport *transport, const struct ref *refs);
+int transport_fetch_refs(struct transport *transport, struct ref *refs);
void transport_unlock_pack(struct transport *transport);
int transport_disconnect(struct transport *transport);
char *transport_anonymize_url(const char *url);
--
1.6.5.2.295.g0d105
^ permalink raw reply related
* Re: [PATCH 0/2] Set Makefile variables from configure
From: Junio C Hamano @ 2009-11-04 19:56 UTC (permalink / raw)
To: Ben Walton; +Cc: Junio C Hamano, jrnieder, git
In-Reply-To: <1257363937-sup-5123@ntdws12.chass.utoronto.ca>
Ben Walton <bwalton@artsci.utoronto.ca> writes:
> Excerpts from Junio C Hamano's message of Wed Nov 04 14:36:32 -0500 2009:
>
>> I am a bit puzzled about the "warning" logic. Is this because you expect
>> variables we typically give YesPlease/NoThanks as their values will not be
>> handled with this new PARSE_WITH_SET_MAKE_VAR() macro?
>
> No, this is because it's perfectly acceptable, in my opinion for a
> user to say:
>
> --with-pager=no
> or
> --with-editor=yes
>
> Neither of those are smart things to do, but they're not necessarily
> wrong, either. I'm open to bailing on error for these, but I thought
> leaving these as unvalidated, but with a warning, was more
> flexible...if say a user wanted to have a pager called 'no' or
> something.
What puzzled me was not "why is it not an error but just a warning?", which
is what you just explained, but "why should we even care what value is
being given to begin with?".
I am _not_ saying "I think it is more correct not to check the value at
all". I just wanted to understand in what situation it may be benefitial
to give this warning in the first place.
> For the variables that are accepting YesPlease/NoThanks, I think it's
> more suitable to use the standard autoconf header/function/library
> detection as it stands now. This macro is more for 'oddball'
> variables like DEFAULT_PAGER, DEFAULT_EDITOR, etc that aren't
> necessarily easily detectable. In some cases, even if it were
> detectable, the detection might not be right.
I am guessing from this description of 'oddball variables' that your
answer to my question is yes. That is, configure.ac writers are strongly
discouraged from using this new macro for variables that would usually get
YesPlease/NoThanks kind of values.
And then it makes sense to warn 'yes/no', as it is unlikely that the user
wants to set name (or path) of programs we allow to be tweaked to 'yes' or
'no'.
^ permalink raw reply
* Re: git subtree issues
From: Avery Pennarun @ 2009-11-04 19:57 UTC (permalink / raw)
To: Daniel Jacobs; +Cc: git
In-Reply-To: <bdccb6a00911031019y48f1d1aax9be7b49e3463595@mail.gmail.com>
On Tue, Nov 3, 2009 at 1:19 PM, Daniel Jacobs <daniel@sibblingz.com> wrote:
> Ok.. I ran the command as you suggested
> git pull -s subtree vw_extensions master
> remote: Counting objects: 5, done.
> remote: Compressing objects: 100% (3/3), done.
> remote: Total 3 (delta 2), reused 0 (delta 0)
> Unpacking objects: 100% (3/3), done.
> From git@github.com:sibblingz/vw_extensions
> * branch master -> FETCH_HEAD
> Already uptodate!
> Merge made by subtree.
>
> git diff --stat $(git merge-base HEAD^ FETCH_HEAD) FETCH_HEAD
> README | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> So, it knows there was a change. However, when I go and vim that README
> file, the change is not there. Maybe this gives you some more information.
This is extremely fishy. What if you re-run the pull without the "-s
subtree" at all? git must be trying to merge those changes in
*somewhere*...
> As an aside, I saw your git subtree tool, and it looks great, but I did not
> try to use it because I could not figure out how to install it. You might
> get a few more users if you include instructions for installation somewhere.
> :-)
Oops, good point. Somehow I forgot that. Fixed.
Thanks,
Avery
^ permalink raw reply
* Re: [PATCH 0/2] Set Makefile variables from configure
From: Jonathan Nieder @ 2009-11-04 20:16 UTC (permalink / raw)
To: Ben Walton; +Cc: Junio C Hamano, git
In-Reply-To: <1257363937-sup-5123@ntdws12.chass.utoronto.ca>
Ben Walton wrote:
> Excerpts from Junio C Hamano's message of Wed Nov 04 14:36:32 -0500 2009:
>
>> I am a bit puzzled about the "warning" logic. Is this because you expect
>> variables we typically give YesPlease/NoThanks as their values will not be
>> handled with this new PARSE_WITH_SET_MAKE_VAR() macro?
>
> No, this is because it's perfectly acceptable, in my opinion for a
> user to say:
>
> --with-pager=no
> or
> --with-editor=yes
More to the point, that’s what autoconf gives for "--without-pager" or
plain "--with-editor". It seems strange to silently use PAGER=no or
EDITOR=yes in that case.
Maybe the options should just be --pager=foo and --editor=bar, which
would be less misleading and avoid this problem. But this is not my
itch (I find it cleaner to use the Makefile directly), so I have no
strong opinion. Unfortunately, my autoconf-fu is too weak for a demo.
Jonathan
^ permalink raw reply
* Re: [PATCH 0/2] Set Makefile variables from configure
From: Ben Walton @ 2009-11-04 20:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: jrnieder, git
In-Reply-To: <7vy6mmf4hi.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]
Excerpts from Junio C Hamano's message of Wed Nov 04 14:56:57 -0500 2009:
> What puzzled me was not "why is it not an error but just a warning?", which
> is what you just explained, but "why should we even care what value is
> being given to begin with?".
I'm sorry. I misunderstood then. I think that in most cases, setting
either 'yes' or 'no' is wrong, but I wanted to leave the door open for
it for the 1%. I thought that a warning message might be noticed and
cause someone to rethink.
> I am guessing from this description of 'oddball variables' that your
> answer to my question is yes. That is, configure.ac writers are
> strongly discouraged from using this new macro for variables that
> would usually get YesPlease/NoThanks kind of values.
Yes, that's correct. This new macro is for variables that are not a
simple toggle. It is also distinct from the mechanism used to specify
a path for perl, tcl/tk, etc, in that for those, we do want to error
out on no.
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
GPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu
Contact me to arrange for a CAcert assurance meeting.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Ramsay Jones @ 2009-11-04 19:19 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AEFD9E2.6060004@viscovery.net>
Johannes Sixt wrote:
> Ramsay Jones schrieb:
>> When the NO_MMAP build variable is set, which is the case by
>> default on cygwin, the msvc linker complains:
>>
>> error LNK2001: unresolved external symbol _getpagesize
>
> Make up your mind: use the cygwin configuration or use the MSVC
> configuration. MSVC doesn't define NO_MMAP for a reason. Where is the problem?
Heh, well as I said elsewhere in this email, the "real problem" is that the
MSVC and cygwin configuration sections are not mutually exclusive and that
is fixed in patch #3. So, if you apply patch #3, this "problem" disappears.
However, ...
> I understand that you run into this error if you define NO_MMAP in your
> config.mak. I don't know whether we support NO_MMAP as a knob for to tweak
> the builds on all platforms. If this is the case (Junio?), then your
> justification must be updated.
AFAICT, the only build to not support NO_MMAP is MSVC (on cygwin *or* msysGit).
The solution was obvious and low impact, so why not remove this anomaly?
(It may even prove to be a useful capability ;-)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 0/4] Cygwin MSVC patches
From: Ramsay Jones @ 2009-11-04 19:03 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, mstormo
In-Reply-To: <4AEFD6F9.5000305@viscovery.net>
Johannes Sixt wrote:
> Just to check I understand correctly: you are running "make MSVC=1" on
> cygwin, and then you are using the resulting binaries with the POSIX tools
> from cygwin.
Yes.
$ uname -a
CYGWIN_NT-5.1 toshiba 1.5.22(0.156/4/2) 2006-11-13 17:01 i686 Cygwin
$ pwd
/home/ramsay/git
$ make clean
[...output snipped...]
$ make MSVC=1
[...975 lines of output snipped; built OK...]
$ ./git --version
git version 1.6.5.4.g57e8c.MSVC
$
See original [PATCH 0/4] email for more...
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 3/4] Makefile: keep MSVC and Cygwin configuration separate
From: Ramsay Jones @ 2009-11-04 19:29 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AEFDB6B.8010209@viscovery.net>
Johannes Sixt wrote:
>
> I like the direction of this change, but I think that you must use ':='
Yeah, I've always been a bit sloppy with recursive assignment versus simple
assignment when the value does not contain a variable reference ;-)
So yes, you are right...
> assignment, and I would put this right after the uname_* assignments at
> the beginning of the Makefile:
>
> uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
> +ifdef MSVC
> + uname_S := Windows
> + uname_O := Windows # avoid cygwin configuration
> +endif
OK with me. (but you are avoiding the MinGW configuration as well...)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
From: Ramsay Jones @ 2009-11-04 19:40 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <4AEFDE9D.6060200@viscovery.net>
Johannes Sixt wrote:
> It may be that I understand something incorrectly; but then I blame the
> justification that you gave. In this case, it would be helpful to reword
> the commit message, and perhaps add some results from your experiments.
>
The discussion which lead to this patch, including the experiments, can be
found in the email thread starting here:
http://thread.gmane.org/gmane.comp.version-control.git/129403
(along with some other unrelated stuff; but it's not a long read :)
In the above thread, Marius suggested API_WIN32, but I switched it around, since
I thought it sounded better! I also thought about GIT_WIN32. Suggestions?
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Ramsay Jones @ 2009-11-04 20:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, GIT Mailing-list, Marius Storm-Olsen
In-Reply-To: <7veiogt4g8.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> How does Cygwin-ness of the build environment affect the end result when
> you build with MSVC?
Not at all. This is an MSVC/NO_MMAP combo problem. (The "problem" also exists
with the msvc build on msysGit[1])
> I am not a Windows person, so I am only guessing,
> but I suspect that the result does not pull any library nor crt0 from what
> people usually consider "Cygwin environment". It feels that the "default
> configuration of Cygwin" that insists on NO_MMAP is the guilty party here.
>
See patch #3.
> Shouldn't this be solved by teaching the Makefile about this new "Cygwin
> but using MSVC as compiler toolchain" combination?
Yes. Err... see patch #3 :-P
> [Footnote]
>
> *1* Notice "if" at the beginning of this sentence---I am not qualified to
> make a judgement without help from area experts here. Is it a sane thing
> to run Cygwin and still use MSVC as the compiler toolchain?
About as sane as running msysGit and still use MSVC as the compiler! :D
> Is it
> commonly done? I have no idea.
>
Nor me. I just tried it, and it works (after applying these patches!); for
exactly the same reason, and to the same extent, that it works on msysGit.
[Footnote]
*1* I admit to sometimes being a bit sloppy with naming (and maybe confused
also!). Now, IIUC, MinGW is basically gcc + binutils, MSYS is bash + some
unix tools and msysGit is MinGW + MSYS + some additional packages needed to
build and run git (eg perl). So, the "MinGW build" should really be called the
msysGit build ;-) (but the config section specifically picks out the MINGW string
from uname_S)
Also, the "msvc build on MinGW" should really be the "msvc build on msysGit".
Or something like that!
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Daniel Barkalow @ 2009-11-04 20:43 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: Johannes Sixt, git
In-Reply-To: <16cee31f0911040650s3eba1067mb66a48bb50c97c28@mail.gmail.com>
On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
> 2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> >
> > You are right. But #ifdef THREADED_DELTA_SEARCH is about a "generic"
> > property of the code and is already used elsewhere in the file, whereas
> > #ifdef WIN32 would be new and is is about platform differences.
> >
> > Anyway, we would have to see what Junio says about the new function calls,
> > because he's usually quite anal when it comes to added code vs. static
> > initialization. ;)
>
> I could do it with wrappers for pthread_mutex_lock and _unlock and
> lazy init there plus lazy init cond var in cond_wait and _signal, that
> way it could be done without any additional code in the first #ifdef.
> But I don't see any simple solution for working around
> deinitialization, that's why I'd leave non-static initialization. Let
> me put some touchups and resubmit for another round.
Is it actually necessary to deinitialize? Since the variables are static
and therefore can't leak, and would presumably not need to be
reinitialized differently if they were used again, I think they should be
able to just stay. If Windows is unhappy about processes still having
locks initialized at exit, I suppose we could go through and destroy all
our mutexes and conds at cleanup time. Pthreads does have the appropriate
functions, and it would be correct to use them, although unnecessary.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: thoughts on setting core.logAllRefUpdates default true for bare repos
From: Matthieu Moy @ 2009-11-04 20:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sitaram Chamarty, git
In-Reply-To: <7v3a4ugjea.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> I can't think of any, and I just tried launching two
>>
>> while true; do git pull; date > foo.txt ; git add .; git commit -m "xxx"; git push; done
>>
>> in parallel, with two different users pushing to a --bare --shared
>> repository, and it did work well. But I may very well have missed
>> something.
>>
>> (and actually, if it causes problem, it's an argument in favor of
>> defaulting to false when core.shared is true, not when core.bare).
>>
>> Unless I missed something, I think core.logAllRefUpdates should be
>> enabled for bare repos.
>
> Your experiment justifies "could be enabled safely" and saying "should be"
> based on that is a bit too strong and also premature.
Right, there were no cause/effect relationship between the first part
and the "should".
> As reflog is a local thing, and not exposed to outside world,
> enabling it alone would not help a lot to people who do not have
> such a direct access to the bare repository, which by definition is
> the majority because the reason why the repository is bare to begin
> with.
For sure, it's the majority. But it's not 100% cases either.
And in most cases, even if the user doesn't have access to the repo,
there exists a sysadmin somewhere who has. And the day a user will
send a mail "hey, I've messed up everything, I did a push -f and what
happened?", this sysadmin will appreciate to have a log somewhere.
And another use-case for the reflog is to find know reliable where a
piece of code is comming from. "git blame" tells you who the commiter
said he was, while the reflog says reliably who the push-er was.
So, clearly, the reflog on a bare repo is not usefull for daily use
like it is for non-bare repos (where, really, it's a killer
feature ;-) ). But it doesn't seem useless either, and it doesn't cost
much, so ...
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Andrzej K. Haczewski @ 2009-11-04 21:16 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Johannes Sixt
In-Reply-To: <alpine.LFD.2.00.0911041247250.10340@xanadu.home>
2009/11/4 Nicolas Pitre <nico@fluxnic.net>:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
>> + NO_STATIC_PTHREADS_INIT = YesPlease
>
> Let's not go that route please. If Windows can't get away without
> runtime initializations then let's use them on all platforms. There is
> no gain in exploding the code path combinations here wrt testing
> coverage.
>
I don't like that approach either, but I was frighten of Junio being
anal about static inits ;).
Let's make it clear: has anyone have any objections that I add
explicit initialization of mutexes and condition variables for POSIX
also?
>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>
> Why can't you just cast the function pointer in your pthread_create
> wrapper instead? No one cares about the returned value anyway.
Because of calling convention - I'd have to cast cdecl function as
stdcall function, which would change the function call clean up (in
cdecl caller is responsible for unwinding stack, stdcall callee; the
effect - double stack unwinding).
>> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>> #ifdef THREADED_DELTA_SEARCH
>> if (!delta_search_threads) /* --threads=0 means autodetect */
>> delta_search_threads = online_cpus();
>> +
>> + init_threaded_delta_search();
>
> What about doing this at the beginning of ll_find_deltas() instead?
> And similarly for cleanup_threaded_delta_search(): call it right before
> leaving ll_find_deltas(). This way thread issues would remain more
> localized. In fact I'd move the whole thing above in ll_find_deltas()
> as well (separately from this patch though).
Sounds sensible, but I'd wait for the NO_STATIC_PTHREADS_INIT verdict.
--
Andrzej
^ permalink raw reply
* Re: [PATCH] MSVC: port pthread code to native Windows threads
From: Nicolas Pitre @ 2009-11-04 21:17 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Andrzej K. Haczewski, Johannes Sixt, git
In-Reply-To: <alpine.LNX.2.00.0911041406101.14365@iabervon.org>
On Wed, 4 Nov 2009, Daniel Barkalow wrote:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
> > 2009/11/4 Johannes Sixt <j.sixt@viscovery.net>:
> > >
> > > You are right. But #ifdef THREADED_DELTA_SEARCH is about a "generic"
> > > property of the code and is already used elsewhere in the file, whereas
> > > #ifdef WIN32 would be new and is is about platform differences.
> > >
> > > Anyway, we would have to see what Junio says about the new function calls,
> > > because he's usually quite anal when it comes to added code vs. static
> > > initialization. ;)
> >
> > I could do it with wrappers for pthread_mutex_lock and _unlock and
> > lazy init there plus lazy init cond var in cond_wait and _signal, that
> > way it could be done without any additional code in the first #ifdef.
> > But I don't see any simple solution for working around
> > deinitialization, that's why I'd leave non-static initialization. Let
> > me put some touchups and resubmit for another round.
>
> Is it actually necessary to deinitialize? Since the variables are static
> and therefore can't leak, and would presumably not need to be
> reinitialized differently if they were used again, I think they should be
> able to just stay. If Windows is unhappy about processes still having
> locks initialized at exit, I suppose we could go through and destroy all
> our mutexes and conds at cleanup time. Pthreads does have the appropriate
> functions, and it would be correct to use them, although unnecessary.
Lazy initialization would probably turn up to be more expensive
(checking a flag on each usage) than unconditionally initializing them
once. Remember that those are used at least once per object meaning a
lot.
And I much prefer having runtime initialization for both Unix and
Windows than having separate paths on each platform potentially hiding
different bugs. And given that on Unix you can statically initialize
those, then a runtime initialization is certainly not going to be _that_
costly.
And while at it, we might just deinitialize them as soon as we're done
with them saving resources (on Unix that might turn up to be a no op
anyway) which is why I suggested doing this within ll_find_deltas()
directly.
Nicolas
^ permalink raw reply
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-04 21:20 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1257364098-1685-10-git-send-email-srabbelier@gmail.com>
On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> This way the helper can write to 'refs/remotes/origin/*' instead of
> writing to 'refs/heads/*'.
>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
> ---
>
> Daniel, does this look good to you?
>
> Let's assume the following:
> * list: only HEAD is a symref (to refs/heads/<branch>), all other
> refs are '? refs/heads/<branch>'.
> * import: the helper creates branches under refs/remotes/<alias>/*
> (since the refspec code is not yet in that would allow
> it to create them under refs/<vcs>/<alias>/*)
>
> Now when updating the refs we do the following:
> transport-helper.c:145 "read_ref(posn->name, posn->old_sha1);"
>
> The value of posn->name looks like "refs/heads/<branch>". So what
> does this lookup do? It tries to lookup the new value of the ref
> _where it will be created_, this fails of course, since the ref
> currently only exists as "refs/remotes/origin/<branch>". So, we
> should instead be doing a lookup using posn->peer_ref->name, and
> not do a lookup at all if it posn->peer_ref is not set (which
> should not occur as we are passed only wanted peer refs).
That's not true for "git pull <url> <branch>"; we do want the remote ref,
but it doesn't have a local peer. I think going straight to the refspec
command is the right answer.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Sverre Rabbelier @ 2009-11-04 21:21 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0911041601170.14365@iabervon.org>
Heya,
On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> That's not true for "git pull <url> <branch>"; we do want the remote ref,
> but it doesn't have a local peer. I think going straight to the refspec
> command is the right answer.
Can you clarity what you mean with "the refspec command"?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC/PATCH 0/3] use '--bisect-refs' as bisect rev machinery option
From: Christian Couder @ 2009-11-04 21:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7veioegko3.fsf@alter.siamese.dyndns.org>
On Wednesday 04 November 2009, Junio C Hamano wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> > On Wed, 4 Nov 2009, Linus Torvalds wrote:
> >> Yes, it is a behavioral change, but is it a bad one?
> >
> > .. and perhaps we could introduce --bisect-refs as the "old behavior"
> > of '--bisect' to git rev-list?
> >
> > I kind of suspect that it is unlikely that people are using 'git
> > rev-list --bisect' while _inside_ a bisection, but then wanting to
> > bisect someting that is outside the set of commits we're currently
> > actively bisecting.
> >
> > But maybe I'm wrong.
>
> Maybe I'm wrong too, but I do not think that is plausible that people are
> doing nested bisection that way. It is probably a useful thing to do,
> but if somebody has thought of doing so we would have at least seen a
> request to add a way to tell "git-bisect" what names to use to record the
> good/bad set of commits under to make their implementation easier. I
> haven't, and I take it an indication that it is very implausible that
> such scripts by people exist to be broken by this change.
>
> I was more worried about people who reinvented the wheel and are using
> their own git-bisect.sh derivative. It probably was forked from the
> version that still used 'git rev-list --bisect", manually feeding good
> and bad set of commits to it from the command line. But then what they
> are feeding would be the same as the new --bisect option implicitly gives
> them anyway, so there won't be a regression either.
I don't remember exactly when, but at one time some people talked about
parallelizing bisection. The idea was that if it takes a long time to test
one commit but you can test many commits at the same time (for example on
different machines), then you can bisect faster by testing at the same time
the current commit that git bisect checked out for you and for example the
commit that git bisect would give you if the current commit is bad and the
commit it would give you if the current commit is good.
So to do that you would use "git bisect start ..." and then you could use:
$ git rev-list --bisect HEAD --not $GOOD_COMMITS
to get the commit that you would have to test if the current commit is bad
and:
$ git rev-list --bisect $BAD --not $GOOD_COMMITS HEAD
to get the commit that you would have to test if the current commit is good.
Ok, perhaps nobody is doing that.
And yes I agree that it would be probably better to have --bisect be the
name of the revision machinery option and --bisect-refs or perhaps another
name like --bisect-compute be the name of the special option to "git
rev-list".
But perhaps we can introduce --bisect-compute to do the same thing
that --bisect currently does and deprecate --bisect with a warning and then
a few versions later remove it and after a few more versions
introduce --bisect to do the same as --bisect-refs.
Best regards,
Christian.
^ permalink raw reply
* Re: [PATCH v2 09/13] Honour the refspec when updating refs after import
From: Daniel Barkalow @ 2009-11-04 21:30 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <fabb9a1e0911041321i1ccec898r53ddafb9405c6331@mail.gmail.com>
On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Wed, Nov 4, 2009 at 22:20, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > That's not true for "git pull <url> <branch>"; we do want the remote ref,
> > but it doesn't have a local peer. I think going straight to the refspec
> > command is the right answer.
>
> Can you clarity what you mean with "the refspec command"?
Whatever it is that lets the helper tell the transport code where in the
helper's private namespace to look for refs. I'd been thinking the helper
would advertize the "refspec" capability, and the transport code would
call the "refspec" command in order to get the helper to report that; but
then I actually only said that the helper reports refspec, and not
proposed a name for the command.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH] pack-objects: move thread autodetection closer to relevant code
From: Nicolas Pitre @ 2009-11-04 21:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrzej K. Haczewski, git, Johannes Sixt
In-Reply-To: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1920 bytes --]
Let's keep thread stuff close together if possible. And in this case,
this even reduces the #ifdef noise, and allows for skipping the
autodetection altogether if delta search is not needed (like with a pure
clone).
Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---
> 2009/11/4 Nicolas Pitre <nico@fluxnic.net>:
> >> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
> >> #ifdef THREADED_DELTA_SEARCH
> >> if (!delta_search_threads) /* --threads=0 means autodetect */
> >> delta_search_threads = online_cpus();
> >> +
> >> + init_threaded_delta_search();
> >
> > What about doing this at the beginning of ll_find_deltas() instead?
> > And similarly for cleanup_threaded_delta_search(): call it right before
> > leaving ll_find_deltas(). This way thread issues would remain more
> > localized. In fact I'd move the whole thing above in ll_find_deltas()
> > as well (separately from this patch though).
So here it is.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 02f9246..4c91e94 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1629,6 +1629,8 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
struct thread_params *p;
int i, ret, active_threads = 0;
+ if (!delta_search_threads) /* --threads=0 means autodetect */
+ delta_search_threads = online_cpus();
if (delta_search_threads <= 1) {
find_deltas(list, &list_size, window, depth, processed);
return;
@@ -2324,11 +2326,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
if (keep_unreachable && unpack_unreachable)
die("--keep-unreachable and --unpack-unreachable are incompatible.");
-#ifdef THREADED_DELTA_SEARCH
- if (!delta_search_threads) /* --threads=0 means autodetect */
- delta_search_threads = online_cpus();
-#endif
-
prepare_packed_git();
if (progress)
^ permalink raw reply related
* Re: [PATCH v2 11/13] Allow helpers to request the path to the .git directory
From: Daniel Barkalow @ 2009-11-04 21:35 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Git List, Johannes Schindelin, Johan Herland
In-Reply-To: <1257364098-1685-12-git-send-email-srabbelier@gmail.com>
On Wed, 4 Nov 2009, Sverre Rabbelier wrote:
> The 'gitdir' capability is reported by the remote helper if it
> requires the location of the .git directory. The location of the .git
> directory can then be used by the helper to store status files even
> when the current directory is not a git repository (such as is the
> case when cloning).
>
> The location of the .git dir is specified as an absolute path.
I thought we cd into the repository in the middle of clone somewhere,
before running stuff. In any case, I think it would be good to have
something like that, but I think maybe we should tell it where it can put
its status files, rather than telling it where the .git dir is. It would
also be nice if this is the absolute path that gfi will base a relative
path for the "marks" options on when importing.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API
From: Erik Faye-Lund @ 2009-11-04 21:41 UTC (permalink / raw)
To: Andrzej K. Haczewski; +Cc: Nicolas Pitre, git, Johannes Sixt
In-Reply-To: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>
On Wed, Nov 4, 2009 at 10:16 PM, Andrzej K. Haczewski
<ahaczewski@gmail.com> wrote:
>>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>>
>> Why can't you just cast the function pointer in your pthread_create
>> wrapper instead? No one cares about the returned value anyway.
>
> Because of calling convention - I'd have to cast cdecl function as
> stdcall function, which would change the function call clean up (in
> cdecl caller is responsible for unwinding stack, stdcall callee; the
> effect - double stack unwinding).
>
Couldn't the windows version of pthread_create have a wrapper
function, that corrected the calling convention, much like the
function run_thread that start_async in run-command.c has?
--
Erik "kusma" Faye-Lund
^ permalink raw reply
* Re: BUG: git rebase -i -p silently looses commits
From: Greg Price @ 2009-11-04 21:46 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Constantine Plotnikov, git
In-Reply-To: <alpine.DEB.1.00.0911021832530.2479@felix-maschine>
On Mon, 2 Nov 2009, Johannes Schindelin wrote:
> Having said that, I worked for some time on fixing this issue, and I
> actually run a version of rebase -i -p here that allows reordering
> commits, but it is far from stable (and due to GSoC and day-job
> obligations, I had no time to work on it in months).
I'm interested in this topic too. Some weeks ago I took your
rebase-i-p branch from January and rebased it onto the latest release;
it's at
git://repo.or.cz/git/price.git rebase-i-p
and now based on v1.6.5.2. I fixed a few bugs and added a feature,
and it's the version I run day to day.
Constantine and others interested in reordering commits with -p,
you're welcome to pull and build this version and try it out. It
mostly solves my problems, and maybe it will solve yours. Be warned
it does have bugs, and also be warned that I may rewind and rebase
that branch. I'd be glad to hear about bugs you see, though.
Dscho, do you have a TODO written somewhere of what work you're aware
the topic still needs? I plan to continue spending a little time
working on it, and I have my own list but it'd be good to compare
it with yours.
Cheers,
Greg
PS - I'm open to suggestions on the workflow for how to develop a
topic branch like this. Some rewinding seems necessary as half-baked
patches get finished, etc, but if Dscho or someone else finds time to
work on it too, then the rewinding gets in the way of pull/push
collaboration.
PPS - For those just scanning along, the shortlog so far:
Greg Price (6):
rebase -i -p: honor -s
rebase -i -p: get full message from original merge commit
rebase -i -p: always merge --no-ff
rebase -i: Add the "ref" command
rebase -i -p: Preserve author information on merges.
[broken] rebase -i: implement pause
Johannes Schindelin (19):
Some of Dscho's tools
debug settings in Makefile
Make CFLAGS more strict
rebase -i --root: simplify code
rebase -i: make pick_one() safer
rebase -i -p: add helper parse_commit() to find rewritten commits
rebase -i: add the "goto" command
rebase -i -p: add a helper to add mappings for rewritten commits
rebase -i -p: Add the "merge" command
rebase -i: make sure that the commands record the rewritten commits
rebase -i: move the code to write the rebase script into generate_script()
rebase -i: let generate_script output to stdout
rebase -i -p: refactor the preparation for -p into its own function
rebase -i -p: use patch-id directly to determine the dropped commits
rebase tests' fake-editor.sh: allow debugging with DEBUG_EDIT
rebase's fake-editor: prepare for "goto" and "merge" commands
rebase -i -p: generate a script using "goto" and "merge"
TODO
Make some tests in t3412 a little bit stricter
^ 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