Patchwork [c-hglib] Initial commit

login
register
mail settings
Submitter Iulian Stana
Date July 1, 2013, 9:31 p.m.
Message ID <815459ad3e8f3b979084.1372714306@doppler>
Download mbox | patch
Permalink /patch/1781/
State Superseded, archived
Headers show

Comments

Iulian Stana - July 1, 2013, 9:31 p.m.
# HG changeset patch
# User Iulian Stana <julian.stana@gmail.com>
# Date 1372688980 -10800
#      Mon Jul 01 17:29:40 2013 +0300
# Node ID 815459ad3e8f3b979084d621729fa189acd9c348
# Parent  0000000000000000000000000000000000000000
Initial commit

Level 0 : raw level

"pass a raw command string, get unparsed results"

Establish a connection between hgclient and mercurial command server.
After the connection has been established, the messages were exchanged with the
purpose of verification. In the main file will be found a short example.

I've followed some of Matt's remarks:

s/chg/hg_/, make sure everything is prefixed

Also, the prototypes all need to be in the header.

Make sure your char args are char *.

You have exactly open/close in your .h file. The example in my email has
open/close/command/read. You're not going to be able to do useful work with less
than that. Note that I specifically chose hg log as my example because it's
likely to generate megabytes of output: you can't simply return it all in one
call.

I've also change:
 - the data structure name from "client" to "handle"
 - the data structure name from "channel" to "header"
 - the name of readchannel function to readheader
 - some of the return statements

I've also erase the mercurial commands for this phase. I'll come back later
with a better aproach.
Iulian Stana - July 1, 2013, 9:32 p.m.
This is a draft of my work for c-hglib up to now, I am sending this to the
list
for public review (even if this is not a patch for the core mercurial). I am
sure that the main developers following my work (Matt, Giovanni,
Pierre-Yves,
Idan and others) will have important remarks on the architecture and on the
style, and I prefer to solicit those feedback sooner than later.

I will try to not abuse of this list and to use it just for important
commits.


2013/7/2 Iulian Stana <julian.stana@gmail.com>

> # HG changeset patch
> # User Iulian Stana <julian.stana@gmail.com>
> # Date 1372688980 -10800
> #      Mon Jul 01 17:29:40 2013 +0300
> # Node ID 815459ad3e8f3b979084d621729fa189acd9c348
> # Parent  0000000000000000000000000000000000000000
> Initial commit
>
> Level 0 : raw level
>
> "pass a raw command string, get unparsed results"
>
> Establish a connection between hgclient and mercurial command server.
> After the connection has been established, the messages were exchanged
> with the
> purpose of verification. In the main file will be found a short example.
>
> I've followed some of Matt's remarks:
>
> s/chg/hg_/, make sure everything is prefixed
>
> Also, the prototypes all need to be in the header.
>
> Make sure your char args are char *.
>
> You have exactly open/close in your .h file. The example in my email has
> open/close/command/read. You're not going to be able to do useful work
> with less
> than that. Note that I specifically chose hg log as my example because it's
> likely to generate megabytes of output: you can't simply return it all in
> one
> call.
>
> I've also change:
>  - the data structure name from "client" to "handle"
>  - the data structure name from "channel" to "header"
>  - the name of readchannel function to readheader
>  - some of the return statements
>
> I've also erase the mercurial commands for this phase. I'll come back later
> with a better aproach.
>
> diff --git a/hglib/Makefile b/hglib/Makefile
> new file mode 100644
> --- /dev/null
> +++ b/hglib/Makefile
> @@ -0,0 +1,19 @@
> +CC = gcc
> +CFLAGS = -Wall -g
> +TARGET = main
> +
> +build: main
> +
> +main: main.c utils.o client.o
> +       $(CC) $(CFLAGS) utils.o client.o -o $(TARGET) main.c
> +
> +client.o: client.c
> +       $(CC) $(CFLAGS) -c client.c
> +
> +utils.o: utils.c
> +       $(CC) $(CFLAGS) -c utils.c
> +
> +.PHONY: clean
> +
> +clean:
> +       rm -f *.o *~ $(TARGET)
> diff --git a/hglib/client.c b/hglib/client.c
> new file mode 100644
> --- /dev/null
> +++ b/hglib/client.c
> @@ -0,0 +1,144 @@
> +#include <sys/types.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +
> +#include "client.h"
> +#include "utils.h"
> +
> +#define HGPATH "hg"
> +
> +struct hg_handle{
> +    int wpipe[2];
> +    int rpipe[2];
> +    pid_t childpid;
> +
> +    #define p_read  rpipe[0]
> +    #define c_write rpipe[1]
> +    #define c_read  wpipe[0]
> +    #define p_write wpipe[1]
> +};
> +
> +/*
> + * Read the channel and the datasize into the header structure.
> + * */
> +hg_header* readheader(hg_handle *handle)
> +{
> +    hg_header *ch = malloc(sizeof(hg_header));
> +    uint32_t length;
> +    char ch_char;
> +
> +    read(handle->p_read, &ch_char, 1);
> +    read(handle->p_read, &length, sizeof(uint32_t));
> +    ch->channel = ch_char;
> +    ch->length = swap_uint32(length);
> +
> +    return ch;
> +}
> +
> +/*
> + * Read the hello message, the server sends when started
> + * Watch for a bad server starting.
> + * */
> +int readhello(hg_handle *handle)
> +{
> +    hg_header *ch = readheader(handle);
> +    char *buffer = malloc(ch->length);
> +    read(handle->p_read, buffer, ch->length);
> +
> +    //TODO: Make a verification !!!
> +
> +    printf("%c%u", ch->channel, ch->length);
> +    printf("%s\n", buffer);
> +    free(ch);
> +    free(buffer);
> +
> +    return 0;
> +}
> +
> +/*
> + * Send the "command" to the cmdserver through handle.
> + * */
> +int hg_rawcommand(hg_handle *handle, const char *command)
> +{
> +    char runcommand[] = "runcommand\n";
> +    uint32_t cmdsize = swap_uint32(strlen(command));
> +    write(handle->p_write, runcommand, strlen(runcommand));
> +    write(handle->p_write, &cmdsize, sizeof(uint32_t));
> +    write(handle->p_write, command, strlen(command));
> +
> +    return 0;
> +}
> +
> +/*
> + * Receive unparse data from the server through handle.
> + * Put the data into the buffer.
> + * */
> +int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff)
> +{
> +    hg_header *ch = readheader(handle);
> +    int length = ch->length;
> +
> +    if(ch->channel == 'r'){
> +        free(ch);
> +        return 0;
> +    }
> +    else{
> +        read(handle->p_read, buffer, ch->length);
> +        buffer[ch->length] = '\0';
> +        free(ch);
> +        return length;
> +    }
> +}
> +
> +/*
> + * Create the connection with the mercurial cmdserver
> + * return the handle for this connection
> + * */
> +hg_handle* hg_open(const char *path, char *encoding, char *configs)
> +{
> +    hg_handle *handle = malloc(sizeof(hg_handle));
> +    char command[100];
> +
> +    sprintf(command,"%s serve --cmdserver pipe --config
> ui.interactive=True",
> +                             HGPATH);
> +
> +    if(path)
> +        sprintf(command, "%s -R %s", command, path);
> +
> +    if(pipe(handle->wpipe) < 0 || pipe(handle->rpipe) < 0){
> +        printf("The connection failed\n");
> +    }
> +
> +    if((handle->childpid = fork()) < 0 ){
> +        printf("Fork failed\n");
> +    }else if( handle->childpid == 0){ //child
> +        close(handle->p_write);
> +        close(handle->p_read);
> +        dup2(handle->c_read, STDIN_FILENO); close(handle->c_read);
> +        dup2(handle->c_write, STDOUT_FILENO); close(handle->c_write);
> +        execl("/bin/sh", "sh", "-c", command, NULL);
> +
> +    }else{ //parent
> +        close(handle->c_read);
> +        close(handle->c_write);
> +    }
> +
> +    readhello(handle);
> +
> +    return handle;
> +}
> +
> +/*
> + * Close the connection for a specific handle.
> + * */
> +int hg_close(hg_handle *handle)
> +{
> +    close(handle->p_read);
> +    close(handle->p_write);
> +    free(handle);
> +
> +    return 0;
> +}
> diff --git a/hglib/client.h b/hglib/client.h
> new file mode 100644
> --- /dev/null
> +++ b/hglib/client.h
> @@ -0,0 +1,37 @@
> +#ifndef _CLIENT_H_
> +#define _CLIENT_H_
> +#include <stdint.h>
> +
> +
> +struct hg_handle;
> +typedef struct hg_handle hg_handle;
> +
> +typedef struct hg_header{
> +    char channel;
> +    uint32_t length;
> +} hg_header;
> +
> +/*
> + * Create the connection with the mercurial cmdserver
> + * return the handle for this connection
> + * */
> +hg_handle* hg_open(const char *path, char *encoding, char *configs);
> +
> +/*
> + * Close the connection for a specific handle.
> + * */
> +int hg_close(hg_handle *handle);
> +
> +/*
> + * Send the "command" to the cmdserver through handle.
> + * */
> +int hg_rawcommand(hg_handle *handle, const char *command);
> +
> +/*
> + * Receive unparse data from the server through handle.
> + * Put the data into the buffer.
> + * */
> +int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff);
> +
> +
> +#endif
> diff --git a/hglib/main.c b/hglib/main.c
> new file mode 100644
> --- /dev/null
> +++ b/hglib/main.c
> @@ -0,0 +1,21 @@
> +#include <stdio.h>
> +
> +#include "client.h"
> +
> +
> +int main(int argc, char ** argv)
> +{
> +    char buffer[4096];
> +    hg_handle* handle = hg_open(NULL, "", "");
> +
> +    printf("\n\n");
> +    hg_rawcommand(handle, "log");
> +
> +    while(hg_rawread(handle, buffer, 4096)){
> +        printf("%s", buffer);
> +    }
> +
> +    hg_close(handle);
> +
> +    return 0;
> +}
> diff --git a/hglib/utils.c b/hglib/utils.c
> new file mode 100644
> --- /dev/null
> +++ b/hglib/utils.c
> @@ -0,0 +1,13 @@
> +#include<stdint.h>
> +#include<stdlib.h>
> +
> +#include "utils.h"
> +
> +void cmdbuilder(char **arg, size_t argsize);
> +
> +//! Byte swap unsigned int
> +uint32_t swap_uint32( uint32_t val )
> +{
> +    val = ((val << 8) & 0xFF00FF00 ) | ((val >> 8) & 0xFF00FF );
> +    return (val << 16) | (val >> 16);
> +}
> diff --git a/hglib/utils.h b/hglib/utils.h
> new file mode 100644
> --- /dev/null
> +++ b/hglib/utils.h
> @@ -0,0 +1,26 @@
> +#ifndef _UTILS_CHG_H_
> +#define _UTILS_CHG_H_
> +
> +#include<stdint.h>
> +#include<stdlib.h>
> +/*
> + * Build the command arguments.
> + * */
> +void cmdbuilder(char **arg, size_t argsize);
> +
> +uint32_t swap_uint32( uint32_t val );
> +
> +/* useful macro for handling error codes */
> +#define DIE(assertion, call_description)    \
> +    do {                                    \
> +        if (assertion) {                    \
> +            fprintf(stderr, "(%s, %d): ",   \
> +                    __FILE__, __LINE__);    \
> +            perror(call_description);       \
> +            exit(EXIT_FAILURE);             \
> +        }                                   \
> +    } while(0)
> +
> +
> +
> +#endif
>
Matt Mackall - July 2, 2013, 5:30 p.m.
On Tue, 2013-07-02 at 00:31 +0300, Iulian Stana wrote:
> +    #define p_read  rpipe[0]
> +    #define c_write rpipe[1]
> +    #define c_read  wpipe[0]
> +    #define p_write wpipe[1]

Yuck.

> +hg_header* readheader(hg_handle *handle)

Please always put the * on the right side of the space. Why? Compare:

char* a, b;
char *a, b;

In the first, you mislead the reader into thinking that a and b are the
same type, and that C recognizes "char*" as a "thing". It does not, and
it binds the "*" to the right side regardless of where you put the
space.

(In fact, I recently fixed a bug where someone had mislead themselves
with exactly this nonsense.)

> +    //TODO: Make a verification !!!

Let's avoid C++ comments, please. Slightly shorter comments isn't
sufficient reason to abandon compatibility with C89 compilers.

Ignoring the rest of the C code for now because I only wanted to see the
header... before you started actually writing the code.

> @@ -0,0 +1,37 @@
> +#ifndef _CLIENT_H_
> +#define _CLIENT_H_
> +#include <stdint.h>
> +
> +
> +struct hg_handle;
> +typedef struct hg_handle hg_handle;

Redundant.

> +
> +typedef struct hg_header{
> +    char channel;
> +    uint32_t length;
> +} hg_header;
> +
> +/*
> + * Create the connection with the mercurial cmdserver
> + * return the handle for this connection
> + * */
> +hg_handle* hg_open(const char *path, char *encoding, char *configs);

More const. Misplaced *. What is configs?

What if we want to run a command without a repository, like init or
clone?

> +
> +/*
> + * Close the connection for a specific handle.
> + * */
> +int hg_close(hg_handle *handle);

What does this return?

> +/*
> + * Send the "command" to the cmdserver through handle.
> + * */
> +int hg_rawcommand(hg_handle *handle, const char *command);

What does this return? How do we pass args with spaces?

> +/*
> + * Receive unparse data from the server through handle. 
> + * Put the data into the buffer.
> + * */
> +int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff);

What does this return?

How do we get exit codes?

How do we send input to a command like 'hg import -'?

How do we handle interactive commands like merge?

How do we get error output and warnings?

DO NOT WRITE ANY CODE to answer these questions because I will have many
more questions. Just show me a proposed API.
Iulian Stana - July 2, 2013, 6:55 p.m.
2013/7/2 Matt Mackall <mpm@selenic.com>

> On Tue, 2013-07-02 at 00:31 +0300, Iulian Stana wrote:
> > +    #define p_read  rpipe[0]
> > +    #define c_write rpipe[1]
> > +    #define c_read  wpipe[0]
> > +    #define p_write wpipe[1]
>
> Yuck.
>

It's something new for me, but it seems to be very helpful.


> > +hg_header* readheader(hg_handle *handle)
>
> Please always put the * on the right side of the space. Why? Compare:
>
> char* a, b;
> char *a, b;
>
> In the first, you mislead the reader into thinking that a and b are the
> same type, and that C recognizes "char*" as a "thing". It does not, and
> it binds the "*" to the right side regardless of where you put the
> space.
>
> (In fact, I recently fixed a bug where someone had mislead themselves
> with exactly this nonsense.)
>

I know the difference... I will try to avoid this kind of mistakes from now
on.



> What if we want to run a command without a repository, like init or
> clone?
>
>
From what Idan said last Wednesday, the init and the clone commands are
a little special. Until now I had focused on the raw level, like you said
in in
an older mail.
Level 0, raw level , "pass a raw command string, get unparsed results"


> How do we get exit codes?



> What does this return? (hg_close)

hg_rawcommand
>
> What does this return? How do we pass args with spaces?
>
> The hg_rawcommand and the hg_close will return the exit code, probably
0 for success and -1 for fail.

For the first commit I had tried to make the minimal to show you that the
level 0 it's working. I change a bit and now it's working to send commands
with multiple options arguments, pass with spaces.



> hg_rawread

What does this return?
>

Will return the number of bytes read. I had tried to make it looks like the
linux read command. http://linux.die.net/man/2/read


How do we send input to a command like 'hg import -'?
>
> How do we handle interactive commands like merge?
>
> How do we get error output and warnings?
>
>
I don't really know how to answer to those questions right now...
I will move this discussion on IRC.

Have a nice day,
Iulian

Patch

diff --git a/hglib/Makefile b/hglib/Makefile
new file mode 100644
--- /dev/null
+++ b/hglib/Makefile
@@ -0,0 +1,19 @@ 
+CC = gcc
+CFLAGS = -Wall -g
+TARGET = main
+
+build: main
+
+main: main.c utils.o client.o
+	$(CC) $(CFLAGS) utils.o client.o -o $(TARGET) main.c
+
+client.o: client.c 
+	$(CC) $(CFLAGS) -c client.c
+
+utils.o: utils.c
+	$(CC) $(CFLAGS) -c utils.c
+
+.PHONY: clean
+
+clean:
+	rm -f *.o *~ $(TARGET)
diff --git a/hglib/client.c b/hglib/client.c
new file mode 100644
--- /dev/null
+++ b/hglib/client.c
@@ -0,0 +1,144 @@ 
+#include <sys/types.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "client.h"
+#include "utils.h"
+
+#define HGPATH "hg"
+
+struct hg_handle{
+    int wpipe[2];
+    int rpipe[2];
+    pid_t childpid;
+
+    #define p_read  rpipe[0]
+    #define c_write rpipe[1]
+    #define c_read  wpipe[0]
+    #define p_write wpipe[1]
+};
+
+/*
+ * Read the channel and the datasize into the header structure.
+ * */
+hg_header* readheader(hg_handle *handle)
+{
+    hg_header *ch = malloc(sizeof(hg_header));
+    uint32_t length;
+    char ch_char;
+
+    read(handle->p_read, &ch_char, 1);
+    read(handle->p_read, &length, sizeof(uint32_t));
+    ch->channel = ch_char;
+    ch->length = swap_uint32(length);
+
+    return ch;
+}
+
+/*
+ * Read the hello message, the server sends when started
+ * Watch for a bad server starting.
+ * */
+int readhello(hg_handle *handle)
+{
+    hg_header *ch = readheader(handle);
+    char *buffer = malloc(ch->length);
+    read(handle->p_read, buffer, ch->length);
+
+    //TODO: Make a verification !!!
+
+    printf("%c%u", ch->channel, ch->length); 
+    printf("%s\n", buffer);
+    free(ch);
+    free(buffer);
+
+    return 0;
+}
+
+/*
+ * Send the "command" to the cmdserver through handle.
+ * */
+int hg_rawcommand(hg_handle *handle, const char *command)
+{
+    char runcommand[] = "runcommand\n";
+    uint32_t cmdsize = swap_uint32(strlen(command));
+    write(handle->p_write, runcommand, strlen(runcommand));
+    write(handle->p_write, &cmdsize, sizeof(uint32_t));
+    write(handle->p_write, command, strlen(command));
+
+    return 0;
+}
+
+/*
+ * Receive unparse data from the server through handle. 
+ * Put the data into the buffer.
+ * */
+int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff)
+{
+    hg_header *ch = readheader(handle);
+    int length = ch->length;
+
+    if(ch->channel == 'r'){
+        free(ch);
+        return 0;
+    }
+    else{
+        read(handle->p_read, buffer, ch->length);
+        buffer[ch->length] = '\0';
+        free(ch);
+        return length;
+    }
+}
+
+/*
+ * Create the connection with the mercurial cmdserver
+ * return the handle for this connection
+ * */
+hg_handle* hg_open(const char *path, char *encoding, char *configs)
+{
+    hg_handle *handle = malloc(sizeof(hg_handle));
+    char command[100];
+
+    sprintf(command,"%s serve --cmdserver pipe --config ui.interactive=True",
+                             HGPATH);
+
+    if(path)
+        sprintf(command, "%s -R %s", command, path);
+
+    if(pipe(handle->wpipe) < 0 || pipe(handle->rpipe) < 0){
+        printf("The connection failed\n");
+    }
+
+    if((handle->childpid = fork()) < 0 ){
+        printf("Fork failed\n");
+    }else if( handle->childpid == 0){ //child
+        close(handle->p_write);
+        close(handle->p_read);
+        dup2(handle->c_read, STDIN_FILENO); close(handle->c_read);
+        dup2(handle->c_write, STDOUT_FILENO); close(handle->c_write);
+        execl("/bin/sh", "sh", "-c", command, NULL);
+
+    }else{ //parent 
+        close(handle->c_read);
+        close(handle->c_write);
+    }
+
+    readhello(handle);
+
+    return handle;
+}
+
+/*
+ * Close the connection for a specific handle.
+ * */
+int hg_close(hg_handle *handle)
+{
+    close(handle->p_read);
+    close(handle->p_write);
+    free(handle);
+
+    return 0;
+}
diff --git a/hglib/client.h b/hglib/client.h
new file mode 100644
--- /dev/null
+++ b/hglib/client.h
@@ -0,0 +1,37 @@ 
+#ifndef _CLIENT_H_
+#define _CLIENT_H_
+#include <stdint.h>
+
+
+struct hg_handle;
+typedef struct hg_handle hg_handle;
+
+typedef struct hg_header{
+    char channel;
+    uint32_t length;
+} hg_header;
+
+/*
+ * Create the connection with the mercurial cmdserver
+ * return the handle for this connection
+ * */
+hg_handle* hg_open(const char *path, char *encoding, char *configs);
+
+/*
+ * Close the connection for a specific handle.
+ * */
+int hg_close(hg_handle *handle);
+
+/*
+ * Send the "command" to the cmdserver through handle.
+ * */
+int hg_rawcommand(hg_handle *handle, const char *command);
+
+/*
+ * Receive unparse data from the server through handle. 
+ * Put the data into the buffer.
+ * */
+int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff);
+
+
+#endif
diff --git a/hglib/main.c b/hglib/main.c
new file mode 100644
--- /dev/null
+++ b/hglib/main.c
@@ -0,0 +1,21 @@ 
+#include <stdio.h>
+
+#include "client.h"
+
+
+int main(int argc, char ** argv)
+{
+    char buffer[4096];
+    hg_handle* handle = hg_open(NULL, "", "");
+
+    printf("\n\n");
+    hg_rawcommand(handle, "log");
+
+    while(hg_rawread(handle, buffer, 4096)){
+        printf("%s", buffer);
+    }
+
+    hg_close(handle);
+
+    return 0;
+}
diff --git a/hglib/utils.c b/hglib/utils.c
new file mode 100644
--- /dev/null
+++ b/hglib/utils.c
@@ -0,0 +1,13 @@ 
+#include<stdint.h>
+#include<stdlib.h>
+
+#include "utils.h"
+
+void cmdbuilder(char **arg, size_t argsize);
+
+//! Byte swap unsigned int
+uint32_t swap_uint32( uint32_t val )
+{
+    val = ((val << 8) & 0xFF00FF00 ) | ((val >> 8) & 0xFF00FF ); 
+    return (val << 16) | (val >> 16);
+}
diff --git a/hglib/utils.h b/hglib/utils.h
new file mode 100644
--- /dev/null
+++ b/hglib/utils.h
@@ -0,0 +1,26 @@ 
+#ifndef _UTILS_CHG_H_
+#define _UTILS_CHG_H_
+
+#include<stdint.h>
+#include<stdlib.h>
+/*
+ * Build the command arguments.
+ * */
+void cmdbuilder(char **arg, size_t argsize);
+
+uint32_t swap_uint32( uint32_t val );
+
+/* useful macro for handling error codes */
+#define DIE(assertion, call_description)    \
+    do {                                    \
+        if (assertion) {                    \
+            fprintf(stderr, "(%s, %d): ",   \
+                    __FILE__, __LINE__);    \
+            perror(call_description);       \
+            exit(EXIT_FAILURE);             \
+        }                                   \
+    } while(0)
+
+
+
+#endif