[buildd-tools-devel] [PATCH 12/17] [chroot_source] Don't derive from chroot

Jan-Marek Glogowski glogow at fbihome.de
Tue Jun 30 18:05:50 UTC 2009


chroot_source does not need any access to chroot methods or
data, so don't derive from chroot.
---
 sbuild/sbuild-chroot-block-device.cc |    5 +++
 sbuild/sbuild-chroot-block-device.h  |    5 ++-
 sbuild/sbuild-chroot-fs-union.cc     |   51 ++++++++++++++++++----------------
 sbuild/sbuild-chroot-fs-union.h      |    4 +--
 sbuild/sbuild-chroot-loopback.cc     |    5 +++
 sbuild/sbuild-chroot-loopback.h      |    5 ++-
 sbuild/sbuild-chroot-lvm-snapshot.h  |    2 +-
 sbuild/sbuild-chroot-source.cc       |   46 ++++++++++++++++++++++--------
 sbuild/sbuild-chroot-source.h        |   10 +-----
 9 files changed, 80 insertions(+), 53 deletions(-)

diff --git a/sbuild/sbuild-chroot-block-device.cc b/sbuild/sbuild-chroot-block-device.cc
index 86a1f4b..fe279b7 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -32,6 +32,7 @@ using boost::format;
 using namespace sbuild;
 
 chroot_block_device::chroot_block_device ():
+  chroot(),
   chroot_fs_union(),
   chroot_mountable(),
   device()
@@ -106,6 +107,7 @@ chroot_block_device::get_chroot_type () const
 void
 chroot_block_device::setup_env (environment& env)
 {
+  chroot::setup_env(env);
   chroot_fs_union::setup_env(env);
   chroot_mountable::setup_env(env);
 
@@ -181,6 +183,7 @@ chroot_block_device::get_session_flags () const
 void
 chroot_block_device::get_details (format_detail& detail) const
 {
+  chroot::get_details(detail);
   chroot_fs_union::get_details(detail);
   chroot_mountable::get_details(detail);
 
@@ -191,6 +194,7 @@ chroot_block_device::get_details (format_detail& detail) const
 void
 chroot_block_device::get_keyfile (keyfile& keyfile) const
 {
+  chroot::get_keyfile(keyfile);
   chroot_fs_union::get_keyfile(keyfile);
   chroot_mountable::get_keyfile(keyfile);
 
@@ -202,6 +206,7 @@ void
 chroot_block_device::set_keyfile (keyfile const& keyfile,
 				  string_list&   used_keys)
 {
+  chroot::set_keyfile(keyfile, used_keys);
   chroot_fs_union::set_keyfile(keyfile, used_keys);
   chroot_mountable::set_keyfile(keyfile, used_keys);
 
diff --git a/sbuild/sbuild-chroot-block-device.h b/sbuild/sbuild-chroot-block-device.h
index 7cef8f6..38e87d9 100644
--- a/sbuild/sbuild-chroot-block-device.h
+++ b/sbuild/sbuild-chroot-block-device.h
@@ -30,7 +30,8 @@ namespace sbuild
    *
    * The device will be mounted on demand.
    */
-  class chroot_block_device : public chroot_fs_union,
+  class chroot_block_device : public chroot,
+			      public chroot_fs_union,
 			      public chroot_mountable
   {
   protected:
@@ -77,7 +78,7 @@ namespace sbuild
     virtual void
     setup_env (environment& env);
 
-    virtual session_flags
+    virtual chroot::session_flags
     get_session_flags () const;
 
     // Specialisation of the chroot_mountable interface
diff --git a/sbuild/sbuild-chroot-fs-union.cc b/sbuild/sbuild-chroot-fs-union.cc
index e331546..5f2b2f6 100644
--- a/sbuild/sbuild-chroot-fs-union.cc
+++ b/sbuild/sbuild-chroot-fs-union.cc
@@ -51,7 +51,6 @@ error<chroot_fs_union::error_code>::error_strings
  init_errors + (sizeof(init_errors) / sizeof(init_errors[0])));
 
 chroot_fs_union::chroot_fs_union ():
-  chroot(),
   chroot_source(),
   fs_union_type("none")
 {
@@ -79,7 +78,7 @@ chroot_fs_union::set_overlay_session_directory
 (std::string const& overlay_session_directory)
 {
   if (!is_absname(overlay_session_directory))
-    throw chroot::error(overlay_session_directory, LOCATION_ABS);
+    throw chroot::error(overlay_session_directory, chroot::LOCATION_ABS);
 
   this->overlay_session_directory = overlay_session_directory;
 }
@@ -95,7 +94,6 @@ chroot_fs_union::set_fs_union_type (std::string const& fs_union_type)
 {
   if ((fs_union_type == "aufs") || (fs_union_type == "unionfs"))
     {
-      set_run_setup_scripts(true);
       this->fs_union_type = fs_union_type;
     }
   else
@@ -123,7 +121,6 @@ chroot_fs_union::set_fs_union_branch_config
 void
 chroot_fs_union::setup_env (environment& env)
 {
-  chroot::setup_env(env);
   chroot_source::setup_env(env);
 
   env.add("CHROOT_FS_UNION_TYPE", get_fs_union_type());
@@ -139,27 +136,26 @@ chroot_fs_union::setup_env (environment& env)
 std::string
 chroot_fs_union::get_path() const
 {
-  return get_mount_location();
+  /// @todo: Remove need for this by passing in group name
+  const chroot *base = dynamic_cast<const chroot *>(this);
+  assert(base != 0);
+
+  return base->get_mount_location();
 }
 
-sbuild::chroot::session_flags
+chroot::session_flags
 chroot_fs_union::get_session_flags () const
 {
   std::string type = get_fs_union_type();
-  if (get_run_setup_scripts() == true) {
-    if (get_fs_union_configured())
-      return SESSION_CREATE | chroot_source::get_session_flags();
-    else
-      return SESSION_CREATE;
-  }
+  if (get_fs_union_configured())
+    return chroot::SESSION_CREATE | chroot_source::get_session_flags();
   else
-    return SESSION_NOFLAGS;
+    return chroot::SESSION_CREATE;
 }
 
 void
 chroot_fs_union::get_details (format_detail& detail) const
 {
-  chroot::get_details(detail);
   chroot_source::get_details(detail);
 
   if (!this->overlay_session_directory.empty())
@@ -173,21 +169,25 @@ chroot_fs_union::get_details (format_detail& detail) const
 void
 chroot_fs_union::get_keyfile (keyfile& keyfile) const
 {
-  chroot::get_keyfile(keyfile);
+  /// @todo: Remove need for this by passing in group name
+  const chroot *base = dynamic_cast<const chroot *>(this);
+  assert(base != 0);
+
   chroot_source::get_keyfile(keyfile);
 
   keyfile::set_object_value(*this, &chroot_fs_union::get_fs_union_type,
-			    keyfile, get_keyfile_name(), "fs-union-type");
+			    keyfile, base->get_keyfile_name(),
+			    "fs-union-type");
 
   keyfile::set_object_value(*this,
 			    &chroot_fs_union::get_fs_union_branch_config,
-			    keyfile, get_keyfile_name(),
+			    keyfile, base->get_keyfile_name(),
 			    "fs-union-branch-config");
 
-  if (get_active())
+  if (base->get_active())
     keyfile::set_object_value(*this,
 			      &chroot_fs_union::get_overlay_session_directory,
-			      keyfile, get_keyfile_name(),
+			      keyfile, base->get_keyfile_name(),
 			      "fs-union-overlay-session-directory");
 }
 
@@ -195,25 +195,28 @@ void
 chroot_fs_union::set_keyfile (keyfile const& keyfile,
 			      string_list&   used_keys)
 {
-  chroot::set_keyfile(keyfile, used_keys);
+  /// @todo: Remove need for this by passing in group name
+  const chroot *base = dynamic_cast<const chroot *>(this);
+  assert(base != 0);
+
   chroot_source::set_keyfile(keyfile, used_keys);
 
   keyfile::get_object_value(*this, &chroot_fs_union::set_fs_union_type,
-			    keyfile, get_keyfile_name(), "fs-union-type",
+			    keyfile, base->get_keyfile_name(), "fs-union-type",
 			    keyfile::PRIORITY_OPTIONAL);
   used_keys.push_back("fs-union-type");
 
   keyfile::get_object_value(*this,
 			    &chroot_fs_union::set_fs_union_branch_config,
-			    keyfile, get_keyfile_name(),
+			    keyfile, base->get_keyfile_name(),
 			    "fs-union-branch-config",
 			    keyfile::PRIORITY_OPTIONAL);
   used_keys.push_back("fs-union-branch-config");
 
-  if (get_active())
+  if (base->get_active())
     keyfile::get_object_value(*this,
 			      &chroot_fs_union::set_overlay_session_directory,
-			      keyfile, get_keyfile_name(),
+			      keyfile, base->get_keyfile_name(),
 			      "fs-union-overlay-session-directory",
 			      (get_fs_union_configured() ?
 			       keyfile::PRIORITY_REQUIRED :
diff --git a/sbuild/sbuild-chroot-fs-union.h b/sbuild/sbuild-chroot-fs-union.h
index 985903f..495f307 100644
--- a/sbuild/sbuild-chroot-fs-union.h
+++ b/sbuild/sbuild-chroot-fs-union.h
@@ -40,8 +40,6 @@ namespace sbuild
     /// The constructor.
     chroot_fs_union ();
 
-    friend class chroot;
-
   public:
     /// The destructor.
     virtual ~chroot_fs_union ();
@@ -129,7 +127,7 @@ namespace sbuild
     virtual void
     setup_env (environment& env);
 
-    virtual session_flags
+    virtual chroot::session_flags
     get_session_flags () const;
 
   protected:
diff --git a/sbuild/sbuild-chroot-loopback.cc b/sbuild/sbuild-chroot-loopback.cc
index fe4455c..12e0a1e 100644
--- a/sbuild/sbuild-chroot-loopback.cc
+++ b/sbuild/sbuild-chroot-loopback.cc
@@ -32,6 +32,7 @@ using boost::format;
 using namespace sbuild;
 
 chroot_loopback::chroot_loopback ():
+  chroot(),
   chroot_fs_union(),
   chroot_mountable(),
   file()
@@ -105,6 +106,7 @@ chroot_loopback::get_chroot_type () const
 void
 chroot_loopback::setup_env (environment& env)
 {
+  chroot::setup_env(env);
   chroot_fs_union::setup_env(env);
   chroot_mountable::setup_env(env);
 
@@ -154,6 +156,7 @@ chroot_loopback::get_session_flags () const
 void
 chroot_loopback::get_details (format_detail& detail) const
 {
+  chroot::get_details(detail);
   chroot_fs_union::get_details(detail);
   chroot_mountable::get_details(detail);
 
@@ -164,6 +167,7 @@ chroot_loopback::get_details (format_detail& detail) const
 void
 chroot_loopback::get_keyfile (keyfile& keyfile) const
 {
+  chroot::get_keyfile(keyfile);
   chroot_fs_union::get_keyfile(keyfile);
   chroot_mountable::get_keyfile(keyfile);
 
@@ -175,6 +179,7 @@ void
 chroot_loopback::set_keyfile (keyfile const& keyfile,
 			      string_list&   used_keys)
 {
+  chroot::set_keyfile(keyfile, used_keys);
   chroot_fs_union::set_keyfile(keyfile, used_keys);
   chroot_mountable::set_keyfile(keyfile, used_keys);
 
diff --git a/sbuild/sbuild-chroot-loopback.h b/sbuild/sbuild-chroot-loopback.h
index 98b5295..d47fb69 100644
--- a/sbuild/sbuild-chroot-loopback.h
+++ b/sbuild/sbuild-chroot-loopback.h
@@ -30,7 +30,8 @@ namespace sbuild
    *
    * The file will be mounted on demand.
    */
-  class chroot_loopback : public chroot_fs_union,
+  class chroot_loopback : public chroot,
+			  public chroot_fs_union,
 			  public chroot_mountable
   {
   protected:
@@ -74,7 +75,7 @@ namespace sbuild
     virtual void
     setup_env (environment& env);
 
-    virtual session_flags
+    virtual chroot::session_flags
     get_session_flags () const;
 
     // Implementation of the chroot_mountable interface
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.h b/sbuild/sbuild-chroot-lvm-snapshot.h
index c4daa44..ab89ebe 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.h
+++ b/sbuild/sbuild-chroot-lvm-snapshot.h
@@ -89,7 +89,7 @@ namespace sbuild
     virtual void
     setup_env (environment& env);
 
-    virtual session_flags
+    virtual chroot::session_flags
     get_session_flags () const;
 
     // Implementation of the chroot_mountable interface
diff --git a/sbuild/sbuild-chroot-source.cc b/sbuild/sbuild-chroot-source.cc
index 1fec773..bdd4bec 100644
--- a/sbuild/sbuild-chroot-source.cc
+++ b/sbuild/sbuild-chroot-source.cc
@@ -29,7 +29,6 @@ using boost::format;
 using namespace sbuild;
 
 chroot_source::chroot_source ():
-  chroot(),
   is_source(false),
   source_users(),
   source_groups(),
@@ -122,15 +121,18 @@ chroot_source::setup_env (environment& env)
 sbuild::chroot::session_flags
 chroot_source::get_session_flags () const
 {
+  const chroot *base = dynamic_cast<const chroot *>(this);
+  assert(base != 0);
+
   if (this->is_source)
     // -source chroots are not clonable.
-    return SESSION_NOFLAGS;
-  else if (get_active())
+    return chroot::SESSION_NOFLAGS;
+  else if (base->get_active())
     //Active chroots are already cloned, but need purging.
-    return SESSION_PURGE;
+    return chroot::SESSION_PURGE;
   else // Inactive, not -source.
     // Inactive chroots are clonable.
-    return SESSION_CLONE;
+    return chroot::SESSION_CLONE;
 }
 
 void
@@ -147,19 +149,28 @@ chroot_source::get_details (format_detail& detail) const
 void
 chroot_source::get_keyfile (keyfile& keyfile) const
 {
+  const chroot *base;
+
   if (!this->is_source)
     {
+      base = dynamic_cast<const chroot *>(this);
+      assert(base != 0);
+
       keyfile::set_object_list_value(*this, &chroot_source::get_source_users,
-				     keyfile, get_keyfile_name(), "source-users");
+				     keyfile, base->get_keyfile_name(), 
+				     "source-users");
 
       keyfile::set_object_list_value(*this, &chroot_source::get_source_groups,
-				     keyfile, get_keyfile_name(), "source-groups");
+				     keyfile, base->get_keyfile_name(), 
+				     "source-groups");
 
       keyfile::set_object_list_value(*this, &chroot_source::get_source_root_users,
-				     keyfile, get_keyfile_name(), "source-root-users");
+				     keyfile, base->get_keyfile_name(), 
+				     "source-root-users");
 
       keyfile::set_object_list_value(*this, &chroot_source::get_source_root_groups,
-				     keyfile, get_keyfile_name(), "source-root-groups");
+				     keyfile, base->get_keyfile_name(), 
+				     "source-root-groups");
     }
 }
 
@@ -167,25 +178,34 @@ void
 chroot_source::set_keyfile (keyfile const& keyfile,
 			    string_list&   used_keys)
 {
+  const chroot *base;
+
   if (!this->is_source)
     {
+      base = dynamic_cast<const chroot *>(this);
+      assert(base != 0);
+
       keyfile::get_object_list_value(*this, &chroot_source::set_source_users,
-				     keyfile, get_keyfile_name(), "source-users",
+				     keyfile, base->get_keyfile_name(),
+				     "source-users",
 				     keyfile::PRIORITY_OPTIONAL);
       used_keys.push_back("source-users");
 
       keyfile::get_object_list_value(*this, &chroot_source::set_source_groups,
-				     keyfile, get_keyfile_name(), "source-groups",
+				     keyfile, base->get_keyfile_name(),
+				     "source-groups",
 				     keyfile::PRIORITY_OPTIONAL);
       used_keys.push_back("source-groups");
 
       keyfile::get_object_list_value(*this, &chroot_source::set_source_root_users,
-				     keyfile, get_keyfile_name(), "source-root-users",
+				     keyfile, base->get_keyfile_name(),
+				     "source-root-users",
 				     keyfile::PRIORITY_OPTIONAL);
       used_keys.push_back("source-root-users");
 
       keyfile::get_object_list_value(*this, &chroot_source::set_source_root_groups,
-				     keyfile, get_keyfile_name(), "source-root-groups",
+				     keyfile, base->get_keyfile_name(),
+				     "source-root-groups",
 				     keyfile::PRIORITY_OPTIONAL);
       used_keys.push_back("source-root-groups");
     }
diff --git a/sbuild/sbuild-chroot-source.h b/sbuild/sbuild-chroot-source.h
index 166da19..a597dac 100644
--- a/sbuild/sbuild-chroot-source.h
+++ b/sbuild/sbuild-chroot-source.h
@@ -30,23 +30,17 @@ namespace sbuild
    * This interface may be implemented by any chroot wishing to
    * provide such functionality.
    *
-   * While this is effectively an interface, in practice this derives
-   * from sbuild::chroot, to allow setting and getting of data from a
-   * keyfile, including storing the keyfile options.
-   *
    * Chroot types implementing chroot_source should, at a minimum,
    * implement clone_source().  This should create and return a source
    * chroot, and must call clone_source_setup() to set up the source
    * chroot.
    */
-  class chroot_source : virtual public chroot
+  class chroot_source
   {
   protected:
     /// The constructor.
     chroot_source ();
 
-    friend class chroot;
-
   public:
     /// The destructor.
     virtual ~chroot_source ();
@@ -145,7 +139,7 @@ namespace sbuild
     setup_env (environment& env);
 
   protected:
-    virtual session_flags
+    virtual chroot::session_flags
     get_session_flags () const;
 
     virtual void
-- 
1.6.3.2




More information about the Buildd-tools-devel mailing list