Re: Patch: add basic package management for 'install'

[Joao Luis Meloni Assirati]

Em 28-07-2016 21:34, address@hidden escreveu:
> Hello,
>
> Attached is a patch that extends the functionality of Coreutils 
> install creating a basic package management capability. If it is to be 
> considered useful or viable, I would like very much to receive 
> reveiews, suggestions for improvements or bug reports. My purpose is 
> to get the code integrated to Coreutils 'install'.
>
> Thank you,
> Joao Luis.
>
> [...]

Hello,

After some testing and thinking, I updated the patch in

http://lists.gnu.org/archive/html/coreutils/2016-07/msg00044.html

The only significant difference is that now if both -l (or --list) is 
given in command line and INSTALL_LIST is set in the environment 
variable, then the value of INSTALL_LIST is used and a warning is 
printed. This is so that the file name of the file list defined in 
makefiles can be overridden.

As before, if there is interest in this patch, I would be glad to 
receive suggestions and bug reports.

Thank you,

João Luis.

--- src/install.c.ORIG  2016-08-08 00:20:09.368012787 -0300
+++ src/install.c       2016-08-08 00:55:14.239956301 -0300
@@ -18,6 +18,7 @@

 #include <config.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <getopt.h>
 #include <sys/types.h>
 #include <signal.h>
@@ -28,6 +29,7 @@

 #include "system.h"
 #include "backupfile.h"
+#include "canonicalize.h"
 #include "error.h"
 #include "cp-hash.h"
 #include "copy.h"
@@ -109,6 +111,9 @@
 /* Program used to strip binaries, "strip" is default */
 static char const *strip_program = "strip";

+/* Stream to record installed files and directories */
+static FILE *install_list = NULL;
+
 /* For long options that have no equivalent short option, use a
    non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
 enum
@@ -124,6 +129,7 @@
   {GETOPT_SELINUX_CONTEXT_OPTION_DECL},
   {"directory", no_argument, NULL, 'd'},
   {"group", required_argument, NULL, 'g'},
+  {"list", required_argument, NULL, 'l'},
   {"mode", required_argument, NULL, 'm'},
   {"no-target-directory", no_argument, NULL, 'T'},
   {"owner", required_argument, NULL, 'o'},
@@ -409,6 +415,20 @@
   return is_a_dir;
 }

+/* Append fname to the install list */
+
+static void
+install_list_append (const char *fname)
+{
+  char *fname_canon;
+  if (install_list == NULL)
+    return;
+  fname_canon = canonicalize_file_name (fname);
+  if (fprintf (install_list, "%s\n", fname_canon) < 0)
+    error (EXIT_FAILURE, errno, _("cannot write to install list"));
+  free (fname_canon);
+}
+
 /* Report that directory DIR was made, if OPTIONS requests this. */
 static void
 announce_mkdir (char const *dir, void *options)
@@ -432,7 +452,10 @@

   int r = mkdir (component, DEFAULT_MODE);
   if (r == 0)
-    announce_mkdir (dir, options);
+    {
+      install_list_append (component);
+      announce_mkdir (dir, options);
+    }
   return r;
 }

@@ -448,17 +471,21 @@
           ? EXIT_SUCCESS
           : EXIT_FAILURE);

-  /* FIXME: Due to the current structure of make_dir_parents()
-     we don't have the facility to call defaultcon() before the
-     final component of DIR is created.  So for now, create the
-     final component with the context from previous component
-     and here we set the context for the final component. */
-  if (ret == EXIT_SUCCESS && x->set_security_context)
-    {
-      if (! restorecon (last_component (dir), false, false)
-          && ! ignorable_ctx_err (errno))
-        error (0, errno, _("failed to restore context for %s"),
-               quoteaf (dir));
+  if (ret == EXIT_SUCCESS)
+    {
+      install_list_append (dir);
+      /* FIXME: Due to the current structure of make_dir_parents()
+        we don't have the facility to call defaultcon() before the
+        final component of DIR is created.  So for now, create the
+        final component with the context from previous component
+        and here we set the context for the final component. */
+      if (x->set_security_context)
+       {
+         if (! restorecon (last_component (dir), false, false)
+             && ! ignorable_ctx_err (errno))
+           error (0, errno, _("failed to restore context for %s"),
+                  quoteaf (dir));
+       }
     }

   return ret;
@@ -658,6 +685,9 @@
                         or all components of --target-directory,\n\
                         then copy SOURCE to DEST\n\
   -g, --group=GROUP   set group ownership, instead of process' current 
group\n\
+  -l, --list=FILE     append to FILE the installed file or directory 
names;\n\
+                        FILE may be passed also through the environment\n\
+                        variable INSTALL_LIST\n\
   -m, --mode=MODE     set permission mode (as in chmod), instead of 
rwxr-xr-x\n\
   -o, --owner=OWNER   set ownership (super-user only)\n\
 "), stdout);
@@ -715,6 +745,7 @@
     }
   if (! copy_file (from, to, x))
     return false;
+  install_list_append (to);
   if (strip_files)
     if (! strip (to))
       {
@@ -800,6 +831,12 @@
   return ret;
 }

+static void close_install_list (void)
+{
+  if (install_list && fclose (install_list) == EOF)
+    error (0, errno, _("cannot write to install list"));
+}
+
 int
 main (int argc, char **argv)
 {
@@ -817,6 +854,7 @@
   char **file;
   bool strip_program_specified = false;
   char const *scontext = NULL;
+  char *install_list_fname = NULL;
   /* set iff kernel has extra selinux system calls */
   selinux_enabled = (0 < is_selinux_enabled ());

@@ -827,6 +865,7 @@
   textdomain (PACKAGE);

   atexit (close_stdin);
+  atexit (close_install_list);

   cp_option_init (&x);

@@ -840,8 +879,8 @@
      we'll actually use backup_suffix_string.  */
   backup_suffix_string = getenv ("SIMPLE_BACKUP_SUFFIX");

-  while ((optc = getopt_long (argc, argv, "bcCsDdg:m:o:pt:TvS:Z", 
long_options,
-                              NULL)) != -1)
+  while ((optc = getopt_long (argc, argv, "bcCsDdg:l:m:o:pt:TvS:Z",
+                             long_options, NULL)) != -1)
     {
       switch (optc)
         {
@@ -878,6 +917,9 @@
         case 'g':
           group_name = optarg;
           break;
+       case 'l':
+         install_list_fname = optarg;
+         break;
         case 'm':
           specified_mode = optarg;
           break;
@@ -1012,6 +1054,24 @@
                quoteaf (file[n_files - 1]));
     }

+  {
+    char *fname_env = getenv ("INSTALL_LIST");
+    if (fname_env)
+      {
+       if (install_list_fname)
+         error (0, 0, _("Warning: file list already defined in"
+                        "environment variable INSTALL_LIST"));
+       install_list_fname = fname_env;
+      }
+    if (install_list_fname)
+      {
+       install_list = fopen (install_list_fname, "a");
+       if (install_list == NULL)
+         error (EXIT_FAILURE, errno,
+                _("failed to open %s"), install_list_fname);
+      }
+  }
+
   if (specified_mode)
     {
       struct mode_change *change = mode_compile (specified_mode);

install-list-1.diff
Description: Text Data