Fossil

View Ticket
Login
Ticket UUID: 77de516a1f2bfc1bd996a8520d0ce59b1b324e77
Title: Post-push patch
Status: Open Type: Feature_Request
Severity: Important Priority:
Subsystem: Resolution: Open
Last Modified: 2010-10-19 08:03:24
Version Found In: n/a
Description:
Please see the mailing list archives, subject thread: "autobuild".

To sum it up, someone posted that he'd like to see a post-push hook in Fossil's server side. A lot of questions were asked which no one cared to answer.

Then I came along, answered the questions and came up with a small patch to implement a minimal post-push hook. Posted the patch back to the mailing list with a request for suggestions and comments, which everyone ignored. :D

So, here is my diff (to run against src/xfer.c). Be aware it's not complete, as noted in the code. Discussion, anyone?

patch 1


wolfgang added on 2010-10-15 21:08:35:
I've used your patch and after adding

    http_exchange(&send, &recv, cloneFlag==0 || nCycle>0);
    blob_reset(&send);
to the client code and making the cmd array in the hook function larger, it works.

But some remarks:

  • The server process waits until the end of the hook. I don't think, that is is really a good thing
  • simply appending outputs to a file may become tricky, if multiple client request come in at the same time. The parallel running hook jobs will append their data at the same time and will mess up the logs

The idea of sending a special 'do-it'-line from the client is nice. The whole thing might be extended - i know, your patch is not finished :-) - with:

  • the hook command should be configured by a setting (gui and/or command line setting)
  • redirects of stdin/stderr should be part of the command configuration
  • the client could use a special settings string, which would be appended to the 'do-it'-line. This setting could be read by the server and added to the command line of the server. This way, the users of such hooks can configure the message passing during the sync. Given your patch, the client might send #FAT build and the server would call targetcmd build
  • if both sides of the line use a setting for the 'do-it' string (FAT in your patch), it would be fully configurable, if the clients wants to send a 'do-it' line and/or the server recognizes these requests.

best regards
Wolfgang

The results of my tests:

Index: src/xfer.c
===================================================================
--- src/xfer.c
+++ src/xfer.c
@@ -40,10 +40,26 @@
   int nDeltaRcvd;     /* Number of deltas received */
   int nDanglingFile;  /* Number of dangling deltas received */
   int mxSend;         /* Stop sending "file" with pOut reaches this size */
 };

+/*
+** Let a server-side external agent know that a push has completed. /fatman
+*/
+void post_push_hook() {
+  /*
+  ** TO DO: get the string cmd from a config file? Or the database local
+  ** settings, as someone suggested? Ditto output and error logs. /fatman
+  */
+  char cmd[1024] = "targetcmd";
+  strcat(cmd, " >target.out 2>target.err");
+  const char *ptr = cmd;
+  int rc = system(ptr);
+  if (rc != 0) {
+    fossil_print("The post-push-hook command \"%s\" failed.", ptr);
+  }
+}

 /*
 ** The input blob contains a UUID.  Convert it into a record ID.
 ** Create a phantom record if no prior record exists and
 ** phantomize is true.
@@ -617,11 +633,17 @@
   );
   zNow = db_text(0, "SELECT strftime('%%Y-%%m-%%dT%%H:%%M:%%S', 'now')");
   @ # timestamp %s(zNow)
   manifest_crosslink_begin();
   while( blob_line(xfer.pIn, &xfer.line) ){
-    if( blob_buffer(&xfer.line)[0]=='#' ) continue;
+    if( blob_buffer(&xfer.line)[0]=='#' ){
+      if ( 0 == memcmp(blob_buffer(&xfer.line), "#FAT", 4) ){
+        fprintf(stderr,"HOOK\n");
+       post_push_hook();
+      }
+      continue;
+    }
     xfer.nToken = blob_tokenize(&xfer.line, xfer.aToken, count(xfer.aToken));

     /*   file UUID SIZE \n CONTENT
     **   file UUID DELTASRC SIZE \n CONTENT
     **
@@ -1330,14 +1352,20 @@
     }

     /* If this is a clone, the go at least two rounds */
     if( cloneFlag && nCycle==1 ) go = 1;
   };
+  if (pushFlag && nFileSend > 0) {
+    blob_appendf(&send, "#FAT\n");
+    http_exchange(&send, &recv, cloneFlag==0 || nCycle>0);
+    blob_reset(&send);
+    nCardSent++;
+  }
   transport_stats(&nSent, &nRcvd, 1);
   fossil_print("Total network traffic: %d bytes sent, %d bytes received\n",
                nSent, nRcvd);
   transport_close();
   transport_global_shutdown();
   db_multi_exec("DROP TABLE onremote");
   manifest_crosslink_end();
   db_end_transaction(0);
 }

wolfgang added on 2010-10-16 15:12:59:
I've attached a patch file, which adds a working push-hook to fossil. It's configured using the settings command/gui-page.

The client and server can be configured:

  • client pattern: the pattern, which will be sent to the server can be configured on client side
  • server pattern: the prefix of the client pattern, which will be accepted to start the hook can be configured on server side
  • the executed command can be configured on server side. The pattern, sent by the client will be added to the command line

anonymous claiming to be arichardson added on 2010-10-18 13:04:03:
> The server process waits until the end of the hook. I don't think, that is is really a good thing

Agreed, this would be a bad thing. Could we simply append " &" to the hook command? That detaches the process from the command line, so it can complete in its own time. It works in *nix and I think it would work in Windows, though that point needs checking. One technique I know works in Windows is to prepend "start " to the command, but that doesn't work in *nix.

> simply appending outputs to a file may become tricky, if multiple client request come in at the same time

How about appending to a file named for the remote Fossil? That data could be included in the comment card, eg. #FAT[12345678] saves output to 12345678.err.log.

> The whole thing might be extended - i know, your patch is not finished :-)

Great - exactly what I was after. I don't want the boss catching me at this as it would be somewhat hard to explain. ;)

> the hook command should be configured by a setting

Agreed.

> redirects of stdin/stderr should be part of the command configuration

I like it.

> the client could use a special settings string, which would be appended to the 'do-it'-line

Interesting. Does this not raise the spectre of potential abuse, though? Perhaps the remote user could be granted a special "allow custom hook" privilege.

> if both sides of the line use a setting for the 'do-it' string

I can see possibilities there.


wolfgang added on 2010-10-18 14:07:39:
Take a look at [9cf288de27], it's a complete implementation of a configurable post push hook.

Agreed, this would be a bad thing. Could we simply append " &" to the hook command? That detaches the process from the command line, so it can complete in its own time. It works in *nix and I think it would work in Windows, though that point needs checking. One technique I know works in Windows is to prepend "start " to the command, but that doesn't work in *nix.
OK, i see the point. We shouldn't add to much (OS-specific!) code to fossil. Using synchronous/asynchronous hooks should be part of the started scripts/programms.
How about appending to a file named for the remote Fossil? That data could be included in the comment card, eg. #FAT12345678 saves output to 12345678.err.log.
The hook can be triggered simultaneously from different sides. I think, i'll add a stdin/stderr redirect to a file with a timestamp/random number part. This fallback redirection is in addition to the redirect, defined in the program/script.
Interesting. Does this not raise the spectre of potential abuse, though? Perhaps the remote user could be granted a special "allow custom hook" privilege.
The server has its own setting which controls, which arguments are accepted. So i think, we don't need a new user permission. Checking user permissions would only check the permission of the sync user, not the permissions of the commit users. If needed, the called hook could check the timeline(rss?) to do a more qualified permission check.

In addition to my first remarks, i added a server side command lineoption, to activate the hook command without the need of a push.

Some ideas - implemented in [a94ef5c00d]:

  • I'll add a new client and server side option, to trigger the the hook on all sync, not only, if files are pushed. The client will send a mark, if files where pushed on the push hook pattern message.
  • The name of the fallback log file will be added as first argument to the command line. The push hook pattern will be the second argument.

Attachments: