* [PATCH v2] Submodule merge support
@ 2007-05-20 15:42 Martin Waitz
2007-05-21 6:20 ` Shawn O. Pearce
0 siblings, 1 reply; 10+ messages in thread
From: Martin Waitz @ 2007-05-20 15:42 UTC (permalink / raw)
To: git
When merge-recursive gets to a dirlink, it starts an automatic submodule
merge and then uses the resulting merge commit for the top-level tree.
The submodule merge is done in another process to decouple object databases.
Submodule merges are done solely in the submodules' history, without taking
the supermodule (and it's merge base) into account. If the submodule merge
is successful then the new submodule version will be used in the merged
supermodule.
If one side of the merge removed any submodule commits (e.g. by switching to
a different branch) then the automatic merge is stopped so that the user can
take a closer look on what happened.
Signed-off-by: Martin Waitz <tali@admingilde.org>
---
This patch is based on my previous submodule checkout patch and the
start-commands-in-submodule patch.
This version takes index_only into account and does not need a new
helper script as all code is done in C now.
The entire ll_merge code in merge-recursive still should be moved to
some generic place, but that is for another patch.
merge-recursive.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 122 insertions(+), 0 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 8f72b2c..72562a8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -11,6 +11,7 @@
#include "diff.h"
#include "diffcore.h"
#include "run-command.h"
+#include "refs.h"
#include "tag.h"
#include "unpack-trees.h"
#include "path-list.h"
@@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
void *buf;
unsigned long size;
+ if (S_ISDIRLNK(mode)) {
+ /* defer dirlinks to another process, don't try to */
+ /* read the object "sha" here */
+ const char *dirlink_checkout[] = {
+ "dirlink-checkout", path, sha1_to_hex(sha), NULL
+ };
+ struct child_process cmd = {
+ .argv = dirlink_checkout,
+ .git_cmd = 1,
+ };
+
+ run_command(&cmd);
+ goto update_index;
+ }
+
buf = read_sha1_file(sha, &type, &size);
if (!buf)
die("cannot read object %s '%s'", sha1_to_hex(sha), path);
@@ -1025,6 +1041,105 @@ static int ll_merge(mmbuffer_t *result_buf,
return merge_status;
}
+
+static int ll_dirlink_merge_base(const char *path,
+ const unsigned char *a,
+ const unsigned char *b,
+ unsigned char *result)
+{
+ const char *merge_base[] = {
+ "merge-base",
+ sha1_to_hex(a),
+ sha1_to_hex(b),
+ NULL
+ };
+ struct child_process cmd = {
+ .argv = merge_base,
+ .submodule = path,
+ .git_cmd = 1,
+ .out = -1,
+ };
+ char hex[40];
+ int status;
+
+ status = start_command(&cmd);
+ if (status) return status;
+
+ status = read(cmd.out, hex, sizeof(hex));
+ if (status != 40) return status;
+
+ status = finish_command(&cmd);
+ if (status) return status;
+
+ status = get_sha1_hex(hex, result);
+
+ return status;
+}
+
+static int ll_dirlink_merge(const char *path,
+ const unsigned char *o,
+ const unsigned char *a,
+ const unsigned char *b,
+ unsigned char *result)
+{
+ char b_hex[40+1];
+ const char *merge[] = {
+ "merge", b_hex, NULL
+ };
+ struct child_process cmd = {
+ .argv = merge,
+ .submodule = path,
+ .git_cmd = 1,
+ };
+ int status;
+ unsigned char base[20];
+ unsigned char test[20];
+
+ if (index_only) {
+ /* as submodules have their own history we don't have to */
+ /* try to do the index_only intermediate merges. */
+ /* however we still want to get a submodule version */
+ /* which is suitable as merge-base, just to make sure that */
+ /* all merge parents contain this base. */
+ /* The real merge (below) aborts if this check fails */
+ return ll_dirlink_merge_base(path, a, b, result);
+ }
+
+ strcpy(b_hex, sha1_to_hex(b));
+ output(3, "merging submodule %s:", path);
+ output(3, " o=%s", sha1_to_hex(o));
+ output(3, " a=%s", sha1_to_hex(a));
+ output(3, " b=%s", sha1_to_hex(b));
+
+ /* first check that the submodule is in the current state */
+ /* so that it can be merged. */
+ status = resolve_gitlink_ref(path, "HEAD", test);
+ if (hashcmp(test, a)) {
+ return error("can't merge submodule %s: not up to date.", path);
+ }
+
+ /* check that both sides of the superproject only did a */
+ /* fast forward of the subproject so that it can be merged */
+ /* automatically. */
+ status = ll_dirlink_merge_base(path, a, b, base);
+ if (status) return status;
+ status = ll_dirlink_merge_base(path, o, base, test);
+ if (status) return status;
+ if (hashcmp(test, o)) {
+ return error("can't merge submodule %s: conflicting history",
+ path);
+ }
+
+ /* now start another merge process for the submodule */
+ status = run_command(&cmd);
+ if (status) return status;
+
+ /* get the new merged version */
+ status = resolve_gitlink_ref(path, "HEAD", result);
+
+ return status;
+}
+
static struct merge_file_info merge_file(struct diff_filespec *o,
struct diff_filespec *a, struct diff_filespec *b,
const char *branch1, const char *branch2)
@@ -1069,6 +1184,13 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
free(result_buf.ptr);
result.clean = (merge_status == 0);
+ } else if (S_ISDIRLNK(a->mode)) {
+ int merge_status;
+
+ merge_status = ll_dirlink_merge(a->path,
+ o->sha1, a->sha1, b->sha1, result.sha);
+
+ result.clean = (merge_status == 0);
} else {
if (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))
die("cannot merge modes?");
--
1.5.2.2.g081e
--
Martin Waitz
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-20 15:42 [PATCH v2] Submodule merge support Martin Waitz
@ 2007-05-21 6:20 ` Shawn O. Pearce
2007-05-21 7:32 ` Martin Waitz
0 siblings, 1 reply; 10+ messages in thread
From: Shawn O. Pearce @ 2007-05-21 6:20 UTC (permalink / raw)
To: Martin Waitz; +Cc: git
> @@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
> void *buf;
> unsigned long size;
>
> + if (S_ISDIRLNK(mode)) {
> + /* defer dirlinks to another process, don't try to */
> + /* read the object "sha" here */
> + const char *dirlink_checkout[] = {
> + "dirlink-checkout", path, sha1_to_hex(sha), NULL
> + };
> + struct child_process cmd = {
> + .argv = dirlink_checkout,
> + .git_cmd = 1,
> + };
My Solaris 9 system cannot compile this syntax, even though it is
a clean way to initalize the child_process. That's why I've always
used something more like:
struct child_process cmd;
memset(&cmd, 0, sizeof(cmd));
cmd.argv = dirlink_checkout;
cmd.git_cmd = 1;
and actually that raises another point, does the compiler 0 fill
the stack-allocated struct that is initalized like you write, or
does it avoid filling the other fields that aren't mentioned in
the initialization?
> + status = read(cmd.out, hex, sizeof(hex));
> + if (status != 40) return status;
OK, this is probably just never trusting the OS, but shouldn't that
read be wrapped up in a loop, like our read_in_full? We want 40
bytes here, and expect it, and the read call is allowed to return
as few as 1 byte....
--
Shawn.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-21 6:20 ` Shawn O. Pearce
@ 2007-05-21 7:32 ` Martin Waitz
2007-05-21 7:37 ` Shawn O. Pearce
2007-05-21 7:54 ` Junio C Hamano
0 siblings, 2 replies; 10+ messages in thread
From: Martin Waitz @ 2007-05-21 7:32 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 1176 bytes --]
hoi :)
On Mon, May 21, 2007 at 02:20:05AM -0400, Shawn O. Pearce wrote:
> > @@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
> > void *buf;
> > unsigned long size;
> >
> > + if (S_ISDIRLNK(mode)) {
> > + /* defer dirlinks to another process, don't try to */
> > + /* read the object "sha" here */
> > + const char *dirlink_checkout[] = {
> > + "dirlink-checkout", path, sha1_to_hex(sha), NULL
> > + };
> > + struct child_process cmd = {
> > + .argv = dirlink_checkout,
> > + .git_cmd = 1,
> > + };
>
> My Solaris 9 system cannot compile this syntax, even though it is
> a clean way to initalize the child_process.
any special thing it does not like in the above code or does it just
not support structs that are initialized that way?
> > + status = read(cmd.out, hex, sizeof(hex));
> > + if (status != 40) return status;
>
> OK, this is probably just never trusting the OS, but shouldn't that
> read be wrapped up in a loop, like our read_in_full? We want 40
> bytes here, and expect it, and the read call is allowed to return
> as few as 1 byte....
right.
--
Martin Waitz
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-21 7:32 ` Martin Waitz
@ 2007-05-21 7:37 ` Shawn O. Pearce
2007-05-21 7:55 ` Junio C Hamano
2007-05-21 15:42 ` Alex Riesen
2007-05-21 7:54 ` Junio C Hamano
1 sibling, 2 replies; 10+ messages in thread
From: Shawn O. Pearce @ 2007-05-21 7:37 UTC (permalink / raw)
To: Martin Waitz; +Cc: git
Martin Waitz <tali@admingilde.org> wrote:
> On Mon, May 21, 2007 at 02:20:05AM -0400, Shawn O. Pearce wrote:
> > > @@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
> > > void *buf;
> > > unsigned long size;
> > >
> > > + if (S_ISDIRLNK(mode)) {
> > > + /* defer dirlinks to another process, don't try to */
> > > + /* read the object "sha" here */
> > > + const char *dirlink_checkout[] = {
> > > + "dirlink-checkout", path, sha1_to_hex(sha), NULL
> > > + };
> > > + struct child_process cmd = {
> > > + .argv = dirlink_checkout,
> > > + .git_cmd = 1,
> > > + };
> >
> > My Solaris 9 system cannot compile this syntax, even though it is
> > a clean way to initalize the child_process.
>
> any special thing it does not like in the above code or does it just
> not support structs that are initialized that way?
Its a very old Sun C compiler, and it doesn't like structs to be
initialized that way. Yes, newer compilers are better, and gcc is
also better, but I'm unable to get our UNIX admins to actually do
their job and keep systems usable by the users.
/me starts to wonder why he continues with this day-job thing...
--
Shawn.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-21 7:37 ` Shawn O. Pearce
@ 2007-05-21 7:55 ` Junio C Hamano
2007-05-21 15:42 ` Alex Riesen
1 sibling, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2007-05-21 7:55 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Martin Waitz, git
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Its a very old Sun C compiler, and it doesn't like structs to be
> initialized that way. Yes, newer compilers are better, and gcc is
> also better, but I'm unable to get our UNIX admins to actually do
> their job and keep systems usable by the users.
>
> /me starts to wonder why he continues with this day-job thing...
Time for a "git company" ;-)?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-21 7:37 ` Shawn O. Pearce
2007-05-21 7:55 ` Junio C Hamano
@ 2007-05-21 15:42 ` Alex Riesen
1 sibling, 0 replies; 10+ messages in thread
From: Alex Riesen @ 2007-05-21 15:42 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Martin Waitz, git
On 5/21/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Its a very old Sun C compiler, and it doesn't like structs to be
> initialized that way. Yes, newer compilers are better, and gcc is
> also better, but I'm unable to get our UNIX admins to actually do
> their job and keep systems usable by the users.
>
> /me starts to wonder why he continues with this day-job thing...
Because the new job may involve windows
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] Submodule merge support
2007-05-21 7:32 ` Martin Waitz
2007-05-21 7:37 ` Shawn O. Pearce
@ 2007-05-21 7:54 ` Junio C Hamano
2007-05-21 12:48 ` [PATCH] SubmittingPatches: mention older C compiler compatibility Johannes Schindelin
1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2007-05-21 7:54 UTC (permalink / raw)
To: Martin Waitz; +Cc: Shawn O. Pearce, git
Martin Waitz <tali@admingilde.org> writes:
>> > + if (S_ISDIRLNK(mode)) {
>> > + /* defer dirlinks to another process, don't try to */
>> > + /* read the object "sha" here */
>> > + const char *dirlink_checkout[] = {
>> > + "dirlink-checkout", path, sha1_to_hex(sha), NULL
>> > + };
>> > + struct child_process cmd = {
>> > + .argv = dirlink_checkout,
>> > + .git_cmd = 1,
>> > + };
>>
>> My Solaris 9 system cannot compile this syntax, even though it is
>> a clean way to initalize the child_process.
>
> any special thing it does not like in the above code or does it just
> not support structs that are initialized that way?
Portability rules:
- We do not do C99 initializers;
- We do not do decl-after-statement;
Readability rules:
- We always write NULL, not 0, for a NULL pointer.
There may be a handful more unwritten rules we use.
>> > + status = read(cmd.out, hex, sizeof(hex));
>> > + if (status != 40) return status;
>>
>> OK, this is probably just never trusting the OS, but shouldn't that
>> read be wrapped up in a loop, like our read_in_full? We want 40
>> bytes here, and expect it, and the read call is allowed to return
>> as few as 1 byte....
>
> right.
I think we have read-in-full or something like that for this
exact purpose.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] SubmittingPatches: mention older C compiler compatibility
2007-05-21 7:54 ` Junio C Hamano
@ 2007-05-21 12:48 ` Johannes Schindelin
2007-05-27 14:39 ` [PATCH] Add -Wdeclaration-after-statement to CFLAGS to help enforce the instructions in SubmittingPatches Johan Herland
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Schindelin @ 2007-05-21 12:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Martin Waitz, Shawn O. Pearce, git
We do not appreciate C99 initializers, declarations after statements,
or "0" instead of "NULL".
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
On Mon, 21 May 2007, Junio C Hamano wrote:
> Portability rules:
>
> - We do not do C99 initializers;
> - We do not do decl-after-statement;
>
> Readability rules:
>
> - We always write NULL, not 0, for a NULL pointer.
>
> There may be a handful more unwritten rules we use.
... so let's start with these 3.
Documentation/SubmittingPatches | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 6a4da2d..cc74b4b 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -65,6 +65,19 @@ in templates/hooks--pre-commit. To help ensure this does not happen,
run git diff --check on your changes before you commit.
+(1a) Try to be nice to older C compilers
+
+We pride ourselves with the wide range of C compilers you can compile
+git with. That means that you should not use C99 initializers, even
+if a lot of compilers grok it.
+
+Also, variables have to be declared at the beginning of the block
+(you can check this with gcc, using the -Wdeclaration-after-statement
+option).
+
+Another thing: NULL pointers shall be written as NULL, not as 0.
+
+
(2) Generate your patch using git tools out of your commits.
git based diff tools (git, Cogito, and StGIT included) generate
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] Add -Wdeclaration-after-statement to CFLAGS to help enforce the instructions in SubmittingPatches
2007-05-21 12:48 ` [PATCH] SubmittingPatches: mention older C compiler compatibility Johannes Schindelin
@ 2007-05-27 14:39 ` Johan Herland
2007-05-27 14:58 ` Morten Welinder
0 siblings, 1 reply; 10+ messages in thread
From: Johan Herland @ 2007-05-27 14:39 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Junio C Hamano, Martin Waitz,
Shawn O. Pearce
Signed-off-by: Johan Herland <johan@herland.net>
---
On Monday 21 May 2007, Johannes Schindelin wrote:
>
> We do not appreciate C99 initializers, declarations after statements,
> or "0" instead of "NULL".
>
> [...]
>
> +Also, variables have to be declared at the beginning of the block
> +(you can check this with gcc, using the -Wdeclaration-after-statement
> +option).
Why not automatically enforce it by putting -Wdeclaration-after-statement
in the Makefile?
It should probably be protected by some GCC if-ery, but then again, so should this:
CC = gcc
Have fun!
...Johan
Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 29243c6..4e91516 100644
--- a/Makefile
+++ b/Makefile
@@ -135,7 +135,7 @@ uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
# CFLAGS and LDFLAGS are for the users to override from the command line.
-CFLAGS = -g -O2 -Wall
+CFLAGS = -g -O2 -Wall -Wdeclaration-after-statement
LDFLAGS =
ALL_CFLAGS = $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
--
1.5.2.101.gee49f
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-05-27 14:58 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-20 15:42 [PATCH v2] Submodule merge support Martin Waitz
2007-05-21 6:20 ` Shawn O. Pearce
2007-05-21 7:32 ` Martin Waitz
2007-05-21 7:37 ` Shawn O. Pearce
2007-05-21 7:55 ` Junio C Hamano
2007-05-21 15:42 ` Alex Riesen
2007-05-21 7:54 ` Junio C Hamano
2007-05-21 12:48 ` [PATCH] SubmittingPatches: mention older C compiler compatibility Johannes Schindelin
2007-05-27 14:39 ` [PATCH] Add -Wdeclaration-after-statement to CFLAGS to help enforce the instructions in SubmittingPatches Johan Herland
2007-05-27 14:58 ` Morten Welinder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).