This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: ecosconfig patch


>>>>> "Patrick" == Patrick Doyle <wpd@delcomsys.com> writes:

    Patrick> I have made a trivial modification to ecosconfig to add
    Patrick> support for a new command called "test". Basically
    Patrick> "ecosconfig test" does the same thing that "ecosconfig
    Patrick> check" does, except that it does not rewrite the ecos.ecc
    Patrick> config file. I did this because I frequently try things
    Patrick> out by just editing the file in emacs, running
    Patrick> "ecosconfig check" to see if I created any conflicts, and
    Patrick> then either continue with whatever I was trying or revert
    Patrick> to the original ecos.ecc. The problem is, ecosconfig
    Patrick> overwrites the file, so I have to reload it. Sometimes
    Patrick> Emacs is able to reconstruct where I was in the file, and
    Patrick> sometimes it cannot.

    Patrick> Anyway, I have made this simple modification and now I
    Patrick> have two questions: 1) Are you (the eCos maintainers)
    Patrick> interested in it? (I anticipate the answer to this would
    Patrick> probably be "Sure, why not? If it doesn't break anything,
    Patrick> it seems like it might be useful in some circumstances.")

    Patrick> 2) Would it have been better to have added an option to
    Patrick> "ecosconfig check", perhaps "-n" for "Don't rewrite the
    Patrick> config file" instead of adding a new command? I could
    Patrick> easily switch to a "-n" option of the "check" command if
    Patrick> that makes more sense.

I would go further than that and make "-n" a valid option for all the
ecosconfig subcommands, i.e. a --no-update flag that suppresses any
modifications to the file system. That would be somewhat similar to
cvs -n. A possible patch to implement this is at the bottom.

    Patrick> Also, as long as I have your attention :-), I started
    Patrick> looking at modifying the export/import behavior to
    Patrick> support listing removed packages as was discussed on this
    Patrick> list a few weeks ago. I have gotten far enough into this
    Patrick> to realize that it is significantly more complicated than
    Patrick> my "ecosconfig test" patch and also to wonder about what
    Patrick> the "best" way to handle this would be. For example,
    Patrick> should I try (somehow) to maintain some level of
    Patrick> backwards compatibility with previous version of
    Patrick> ecosconfig? If so, then I think I would flag the removed
    Patrick> packages with a "-removed" option in the export file.
    Patrick> That way, one would only get a warning ("I don't know
    Patrick> what the '-removed' option is") from older versions of
    Patrick> ecosconfig. If not, then I guess I would add a
    Patrick> "removed_package" command to the interpreter which will
    Patrick> choke older versions of ecosconfig. Alternatively, I
    Patrick> could list the removed packages in a comment line, but
    Patrick> then I would need to modify the parser to look for those
    Patrick> special comment lines.

    Patrick> As I said, it got much more complicated much more
    Patrick> quickly. Any suggestions? Should I drop this because one
    Patrick> of you have already dashed out a solution? (Did a
    Patrick> solution sneak into the CVS archives when I wasn't
    Patrick> looking?)

Hmmm, on reflection I should probably have got involved in that
discussion early on. No solution has sneaked into CVS when you were
not looking for the simple reason that I do not believe there is any
simple way of implementing the functionality, at least not as
described in those messages. The current implementation of
export/import and templates is based on the savefile code, not
necessarily because that is the optimal implementation but rather
because it was easy to do and at the time there was no easy way of
implementing something better.

For an eCos savefile (minimal or otherwise) to provide information
about removed packages libcdl would have to know which packages have
been removed. Right now no attempt is made to keep track of historical
information like that. Packages can be removed for a variety of
reasons - explicit removes, changing the target, changing the
template, changing a package version - so adding code to correctly
keep track of removed packages would be a fair bit of work, i.e. quite
a few days even for people familiar with the libcdl internals.

An alternative approach would involve figuring out at savefile
generation time what has happened. That would require building a new
temporary configuration with the current target and template and
detecting discrepancies between the current and temporary
configuration. The run-time overheads would be considerable, and
probably unacceptable.

If we were only interested in hand-edited templates or import files,
it would be possible to add a remove_package command to the savefile
syntax. libcdl would never output a remove_package command, but if
such a command is manually inserted into a savefile then libcdl could
take appropriate action on reading it back.

In theory there should not be a problem using the newer savefile
syntax with older versions of libcdl: remove_package would have to be
listed as one of the valid savefile commands at the start of the file,
and when an older version of libcdl tried to load in the newer
savefile it should end up executing
CdlToplevelBody::savefile_handle_unsupported() which mostly ignores
the data but tries to make sure that it will be preserved when the
configuration is saved again. Unfortunately that code has received
very little testing so it may not quite work as well as intended.

However, I am not convinced that changing the way savefiles work is
the correct way to address the underlying problem. Instead what I
think people really want is an easier way to manipulate configuration
data, e.g. via a Tcl script:

    #! ecosconfig cdlsh
    cdl_config new <target> <template>
    cdl_config unload_package <whatever>
    cdl_set <option> <value>
    ...
    cdl_config save
    cdl_config tree

Scriptable access to the configuration data has been a fundamental
part of the design from the beginning, but so far has not been
adequately resourced. Implementing it would be non-trivial, there are
lots of complications such as correctly interfacing to the libcdl
transaction model, inference engine, ...

Because an eCos template or savefile is just an executable Tcl script,
it would actually be possible to replace existing .ecm files etc. with
hand-written scripts like the above that act on the current
configuration. This would be transparent to end users - they would
still be importing .ecm files, with no need to worry about the
internal implementation of those files. Obviously there would be
plenty of other uses for such scripting facilities.

Bart


Index: ChangeLog
===================================================================
RCS file:
/home/cvs/ecc/host/tools/configtool/standalone/common/ChangeLog,v
retrieving revision 1.14
diff -u -u -r1.14 ChangeLog
--- ChangeLog	  2001/12/06 14:10:48	1.14
+++ ChangeLog	  2001/12/29 14:59:06
@@ -1,3 +1,10 @@
+2001-12-29  Bart Veer  <bartv@redhat.com>
+
+	* ecosconfig.cxx, cdl_exec.cxx, cdl_exec.hxx
+	Add new option -n to suppress modifications to the file
system,
+	especially ecos.ecc. Suggested by Patrick Doyle
+	(wpd@delcomsys.com)
+
 2001-12-06  Bart Veer  <bartv@redhat.com>

	     * cdl_exec.cxx (report_conflicts):
Index: cdl_exec.hxx
===================================================================
RCS file:
/home/cvs/ecc/host/tools/configtool/standalone/common/cdl_exec.hxx,v
retrieving revision 1.18
diff -u -u -r1.18 cdl_exec.hxx
--- cdl_exec.hxx  2000/06/28 15:26:44	1.18
+++ cdl_exec.hxx  2001/12/29 15:00:26
@@ -1,7 +1,7 @@
 //####COPYRIGHTBEGIN####
 //
 //
----------------------------------------------------------------------------
-// Copyright (C) 1998, 1999, 2000 Red Hat, Inc.
+// Copyright (C) 1998, 1999, 2000, 2001 Red Hat, Inc.
 //
 // This program is part of the eCos host tools.
 //
@@ -43,11 +43,13 @@
     static void set_quiet_mode(bool);
     static void set_verbose_mode(bool);
     static void set_ignore_errors_mode(bool);
+    static void set_no_updates_mode(bool);

 protected:
     static bool quiet;
     static bool verbose;
     static bool ignore_errors;
+    static bool no_updates;
     std::string repository;
     std::string savefile;
     std::string install_prefix;


Index: cdl_exec.cxx
===================================================================
RCS file:
/home/cvs/ecc/host/tools/configtool/standalone/common/cdl_exec.cxx,v
retrieving revision 1.34
diff -u -u -r1.34 cdl_exec.cxx
--- cdl_exec.cxx  2001/12/06 14:10:49	1.34
+++ cdl_exec.cxx  2001/12/29 14:59:53
@@ -54,6 +54,7 @@
 bool cdl_exec::quiet            = false;
 bool cdl_exec::verbose          = false;
 bool cdl_exec::ignore_errors    = false;
+bool cdl_exec::no_updates       = false;

 cdl_exec::cdl_exec (const std::string repository_arg, const
 std::string savefile_arg,
                     const std::string install_arg, bool
 no_resolve_arg)
@@ -98,6 +99,12 @@
     ignore_errors = new_val;
 }

+void
+cdl_exec::set_no_updates_mode(bool new_val)
+{
+    no_updates = new_val;
+}
+
 //
----------------------------------------------------------------------------
 void
 cdl_exec::init(bool load_config)
@@ -156,11 +163,13 @@

         // Now report any conflicts which the inference engine could
         not report.
         report_conflicts();
-
+
         // A savefile should be generated/updated even if there are
         conflicts.
         // Otherwise the user does not have a chance to edit the
         savefile
         // and fix things.
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -187,7 +196,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -214,7 +225,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -244,7 +257,9 @@
         // configuration is conflict-free. This is different from
         // updating the savefile.
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
-            config->save (cdl_savefile, /* minimal = */ true);
+            if (!no_updates) {
+                config->save (cdl_savefile, /* minimal = */ true);
+            }
             status = true;
         }
     } catch (CdlStringException exception) {
@@ -270,7 +285,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -299,7 +316,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -334,7 +353,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -364,7 +385,9 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
@@ -390,9 +413,15 @@
             config->resolve_all_conflicts();
         }
         report_conflicts();
-        config->save (savefile);
-        // A build tree should only be generated if there are no
         conflicts.
-        if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
+        if (!no_updates) {
+            config->save (savefile);
+        }
+        // A build tree should only be generated if there are no
         conflicts,
+        // and suppressed if -n is given.
+        if (no_updates) {
+            // Do nothing
+        }
+        else if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
 #ifdef _MSC_VER
             char cwd [_MAX_PATH + 1];
 #else
@@ -527,7 +556,9 @@
         // change.
         // However, updating the savefile is worthwhile because it
         // will now contain more accurate information about the
         state.
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }

         // report current target and template
         printf ("Target: %s\n", config->get_hardware ().c_str ());
@@ -615,7 +646,9 @@
         CdlTransactionBody::set_callback_fn(&transaction_callback);
         config->resolve_all_conflicts ();
         report_conflicts();
-        config->save (savefile);
+        if (!no_updates) {
+            config->save (savefile);
+        }
         if (ignore_errors || (0 ==
         config->get_all_conflicts().size())) {
             status = true;
         }
Index: ecosconfig.cxx
===================================================================
RCS file:
         /home/cvs/ecc/host/tools/configtool/standalone/common/ecosconfig.cxx,v
retrieving revision 1.25
diff -u -u -r1.25 ecosconfig.cxx
--- ecosconfig.cxx	2001/05/16 19:28:34	1.25
+++ ecosconfig.cxx	2001/12/29 15:01:37
@@ -89,6 +89,7 @@
     bool quiet = false;         // -q, --quiet
     bool verbose = false;       // -v, --verbose
     bool ignore_errors = false; // -i, --ignore-errors
+    bool no_updates = false;    // -n, --no-updates,
     bool help = false;          // --help

     // getopt() cannot easily be used here since this code has to
@@ -110,6 +111,8 @@
         } else if ((0 == strcmp(arg, "-i")) || (0 == strcmp(arg,
"--ignore-errors"))) {
             // Duplicate use of -i and the other flags is harmless.
             ignore_errors = true;
+        } else if ((0 == strcmp(arg, "-n")) || (0 == strcmp(arg,
"--no-updates"))) {
+            no_updates = true;
         } else if (0 == strcmp(arg, "--version")) {
             version = true;
         } else if (0 == strcmp(arg, "--no-resolve")) {
@@ -206,6 +209,7 @@
     printf("no_resolve is %d\n", no_resolve);
     printf("quiet is %d\n", quiet);
     printf("verbose is %d\n", verbose);
+    printf("no-updates is %d\n", no_updates);
     printf("ignore_errors is %d\n", ignore_errors);
     printf("repository is %s\n", repository.c_str());
     printf("savefile is %s\n", savefile.c_str());
@@ -275,6 +279,7 @@
     cdl_exec::set_quiet_mode(quiet);
     cdl_exec::set_verbose_mode(verbose);
     cdl_exec::set_ignore_errors_mode(ignore_errors);
+    cdl_exec::set_no_updates_mode(no_updates);

     // Now identify and process the sub-command.
     const std::string command = argv [command_index];
@@ -442,5 +447,6 @@
     printf ("    -q, --quiet                                : reduce
     verbosity\n");
     printf ("    -v, --verbose                              :
     increase verbosity\n");
     printf ("    -i, --ignore-errors                        : ignore
     unresolved conflicts\n");
+    printf ("    -n, --no-updates                           :
     read-only mode, do not modify the file system\n");
     printf ("    --help                                     : display
     this message\n");
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]