* heavily redesigned and improved the checking of attributes: instead of dying

  on the first error, we now collect all errors and return them. The caller can
  now decide what to do - die or just print a warning or whatever.
* slxconfig now dies with the list of all attribute problems if there were any
* slxconfig-demuxer prints warnings for all attribute problems that were found
  (checking for each system & client in turn)
* adjusted desktop plugin to API changes concerning the attribute checks

git-svn-id: http://svn.openslx.org/svn/openslx/openslx/trunk@1796 95ad53e4-c205-0410-b2fa-d234c58c8868

3 files changed, 56 insertions, 22 deletions

diff --git a/os-plugins/OpenSLX/OSPlugin/Base.pm b/os-plugins/OpenSLX/OSPlugin/Base.pm
index 8c314ff5..a07ec125 100644
--- a/os-plugins/OpenSLX/OSPlugin/Base.pm
+++ b/os-plugins/OpenSLX/OSPlugin/Base.pm
@@ -187,7 +187,8 @@ sub getDefaultAttrsForVendorOS
 Checks if the stage3 values given in B<$stage3Attrs> are allowed and makes 
 sense.
 
-If all values are ok, this method returns 1 - if not, it dies with an appropriate message.
+This method returns an array-ref of problems found. If there were no problems, 
+this methods returns undef.
 
 Plugins may override this implementation to do checks that for instance look
 at the stage1 vendor-OS-attributes given in B<$vendorOSAttrs>.
@@ -206,7 +207,7 @@ sub checkStage3AttrValues
 
     # this default implementation does no further checks (thus relying on the
     # attributte regex check that is done in the AttributeRoster)
-    return 1;
+    return;
 }
 
 =back
diff --git a/os-plugins/OpenSLX/OSPlugin/Engine.pm b/os-plugins/OpenSLX/OSPlugin/Engine.pm
index ada0af79..75ba3154 100644
--- a/os-plugins/OpenSLX/OSPlugin/Engine.pm
+++ b/os-plugins/OpenSLX/OSPlugin/Engine.pm
@@ -157,8 +157,8 @@ sub installPlugin
         # as the attrs may be changed by the plugin during installation, we
         # have to find a way to pass them back to this process (remember;
         # installation takes place in a forked process in order to do a chroot).
-        # We simply serialize the attributes into a temp and deserialize it
-        # in the calling process.
+        # We simply serialize the attributes into a temp file and deserialize
+        # it in the calling process.
         my $serializedAttrsFile 
             = "$self->{'plugin-temp-path'}/serialized-attrs";
         my $chrootedSerializedAttrsFile 
@@ -458,8 +458,8 @@ install/remove a plugin into/from a vendor-OS:
 
 Checks if the stage3 values given in B<$stage3Attrs> are allowed and make sense.
 
-If all values are ok, this method returns 1 - if not, it dies with an 
-appropriate message.
+If all values are ok, this method returns 1 - if not, it extends the given
+problems array-ref with the problems that were found (and returns undef).
 
 This method chroots into the vendor-OS and then asks the plugin itself to check
 the attributes.
@@ -470,16 +470,45 @@ sub checkStage3AttrValues
 {
     my $self        = shift;
     my $stage3Attrs = shift;
-
+    my $problemsOut = shift;
+
+    # as the attrs may be changed by the plugin during installation, we
+    # have to find a way to pass them back to this process (remember;
+    # installation takes place in a forked process in order to do a chroot).
+    # We simply serialize the attributes into a temp file and deserialize
+    # it in the calling process.
+    my $serializedProblemsFile 
+        = "$self->{'plugin-temp-path'}/serialized-problems";
+    my $chrootedSerializedProblemsFile 
+        = "$self->{'chrooted-plugin-temp-path'}/serialized-problems";
+
+    mkpath([ $self->{'plugin-repo-path'}, $self->{'plugin-temp-path'} ]);
+
+    # HACK: do a dummy serialization here in order to get Storable 
+    # completely loaded (otherwise it will complain in the chroot about 
+    # missing modules).
+    store [], $serializedProblemsFile;
+    
     $self->_callChrootedFunctionForPlugin(
         sub {
             # let plugin check by itself
-            $self->{plugin}->checkStage3AttrValues(
+            my $problems = $self->{plugin}->checkStage3AttrValues(
                 $stage3Attrs, $self->{'vendorOS-attrs'}
             );
+
+            # serialize list of problems (executed inside chroot)
+            store($problems, $chrootedSerializedProblemsFile) if $problems;
         }
     );
 
+    # now retrieve (deserialize) the found problems and pass them on
+    my $problems = retrieve $serializedProblemsFile;
+    rmtree([ $self->{'plugin-temp-path'} ]);
+    if ($problems && ref($problems) eq 'ARRAY' && @$problems) {
+        push @$problemsOut, @$problems;
+        return;
+    }
+
     return 1;
 }
 
diff --git a/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm b/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm
index 61787309..bfaee875 100644
--- a/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm
+++ b/os-plugins/plugins/desktop/OpenSLX/OSPlugin/desktop.pm
@@ -206,17 +206,19 @@ sub checkStage3AttrValues
     my $stage3Attrs   = shift;
     my $vendorOSAttrs = shift;
     
+    my @problems;
+
     my $manager = $stage3Attrs->{'desktop::manager'} || '';
     if ($manager eq 'kdm') {
         if (!defined $vendorOSAttrs->{'desktop::kdm'}) {
             if (!$self->{distro}->isKDMInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "KDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::kdm'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::kdm is 0, so using KDM as desktop manager is not allowed for this vendor-OS!"
             );
         }
@@ -224,13 +226,13 @@ sub checkStage3AttrValues
     elsif ($manager eq 'gdm') {
         if (!defined $vendorOSAttrs->{'desktop::gdm'}) {
             if (!$self->{distro}->isGDMInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "GDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::gdm'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::gdm is 0, so using GDM as desktop manager is not allowed for this vendor-OS!"
             );
         }
@@ -238,13 +240,13 @@ sub checkStage3AttrValues
     elsif ($manager eq 'xdm') {
         if (!defined $vendorOSAttrs->{'desktop::xdm'}) {
             if (!$self->{distro}->isXDMInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "XDM is not installed in vendor-OS, so using it as desktop manager wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::xdm'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::xdm is 0, so using XDM as desktop manager is not allowed for this vendor-OS!"
             );
         }
@@ -254,13 +256,13 @@ sub checkStage3AttrValues
     if ($kind eq 'kde') {
         if (!defined $vendorOSAttrs->{'desktop::kde'}) {
             if (!$self->{distro}->isKDEInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "KDE is not installed in vendor-OS, so using it as desktop kind wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::kde'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::kde is 0, so using KDE as desktop kind is not allowed for this vendor-OS!"
             );
         }
@@ -268,13 +270,13 @@ sub checkStage3AttrValues
     elsif ($kind eq 'gnome') {
         if (!defined $vendorOSAttrs->{'desktop::gnome'}) {
             if (!$self->{distro}->isGNOMEInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "GNOME is not installed in vendor-OS, so using it as desktop kind wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::gnome'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::gnome is 0, so using GNOME as desktop kind is not allowed for this vendor-OS!"
             );
         }
@@ -282,13 +284,13 @@ sub checkStage3AttrValues
     elsif ($kind eq 'xfce') {
         if (!defined $vendorOSAttrs->{'desktop::xfce'}) {
             if (!$self->{distro}->isXFCEInstalled()) {
-                die _tr(
+                push @problems, _tr(
                     "XFCE is not installed in vendor-OS, so using it as desktop kind wouldn't work!"
                 );
             }
         }
         elsif ($vendorOSAttrs->{'desktop::xfce'} == 0) {
-            die _tr(
+            push @problems, _tr(
                 "desktop::xfce is 0, so using XFCE as desktop kind is not allowed for this vendor-OS!"
             );
         }
@@ -298,13 +300,15 @@ sub checkStage3AttrValues
         = split ',', $vendorOSAttrs->{'desktop::supported_themes'} || '';
     my $theme = $stage3Attrs->{'desktop::theme'};
     if (defined $theme && !grep { $_ eq $theme } @supportedThemes) {
-        die _tr(
+        push @problems, _tr(
             "desktop::theme '%s' does not refer to a supported theme!\nSupported themes are: %s",
             $theme, $vendorOSAttrs->{'desktop::supported_themes'} || ''
         );
     }
 
-    return 1;
+    return if !@problems;
+
+    return \@problems;
 }
 
 sub installationPhase