[buildd-tools-devel] [PATCH 10/17] [chroot_mountable] Don't derive from chroot
Jan-Marek Glogowski
glogow at fbihome.de
Tue Jun 30 18:05:48 UTC 2009
chroot_mountable does not need any access to chroot methods or
data, so don't derive from chroot.
chroot::get_path is now pure virtual in order to permit this.
---
sbuild/sbuild-chroot-block-device.cc | 2 +-
sbuild/sbuild-chroot-file.cc | 6 ++++++
sbuild/sbuild-chroot-file.h | 3 +++
sbuild/sbuild-chroot-fs-union.cc | 24 ++++++++++--------------
sbuild/sbuild-chroot-loopback.cc | 2 +-
sbuild/sbuild-chroot-mountable.cc | 33 ++++++++++++++-------------------
sbuild/sbuild-chroot-mountable.h | 9 +++------
sbuild/sbuild-chroot.cc | 6 ------
sbuild/sbuild-chroot.h | 2 +-
test/sbuild-chroot.cc | 4 ++++
10 files changed, 43 insertions(+), 48 deletions(-)
diff --git a/sbuild/sbuild-chroot-block-device.cc b/sbuild/sbuild-chroot-block-device.cc
index be0bc5f..86a1f4b 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -80,7 +80,7 @@ chroot_block_device::set_device (std::string const& device)
std::string
chroot_block_device::get_path () const
{
- return chroot_mountable::get_path();
+ return get_mount_location() + get_location();
}
void
diff --git a/sbuild/sbuild-chroot-file.cc b/sbuild/sbuild-chroot-file.cc
index 9cd28af..d54584b 100644
--- a/sbuild/sbuild-chroot-file.cc
+++ b/sbuild/sbuild-chroot-file.cc
@@ -87,6 +87,12 @@ chroot_file::set_file_repack (bool repack)
this->repack = repack;
}
+std::string
+chroot_file::get_path () const
+{
+ return get_mount_location();
+}
+
std::string const&
chroot_file::get_chroot_type () const
{
diff --git a/sbuild/sbuild-chroot-file.h b/sbuild/sbuild-chroot-file.h
index 0df1b85..e9d62b9 100644
--- a/sbuild/sbuild-chroot-file.h
+++ b/sbuild/sbuild-chroot-file.h
@@ -89,6 +89,9 @@ namespace sbuild
virtual void
setup_env (environment& env);
+ std::string
+ get_path () const;
+
virtual session_flags
get_session_flags () const;
diff --git a/sbuild/sbuild-chroot-fs-union.cc b/sbuild/sbuild-chroot-fs-union.cc
index 51d3931..e331546 100644
--- a/sbuild/sbuild-chroot-fs-union.cc
+++ b/sbuild/sbuild-chroot-fs-union.cc
@@ -40,7 +40,7 @@ namespace
emap init_errors[] =
{
// TRANSLATORS: %1% = chroot fs type
- emap(chroot_fs_union::FS_TYPE_UNKNOWN, N_("Unknown filesystem type '%1%'"))
+ emap(chroot_fs_union::FS_TYPE_UNKNOWN, N_("Unknown filesystem type '%1%'"))
};
}
@@ -75,7 +75,7 @@ chroot_fs_union::get_overlay_session_directory () const
}
void
-chroot_fs_union::set_overlay_session_directory
+chroot_fs_union::set_overlay_session_directory
(std::string const& overlay_session_directory)
{
if (!is_absname(overlay_session_directory))
@@ -114,7 +114,7 @@ chroot_fs_union::get_fs_union_branch_config () const
}
void
-chroot_fs_union::set_fs_union_branch_config
+chroot_fs_union::set_fs_union_branch_config
(std::string const& fs_union_branch_config)
{
this->fs_union_branch_config = fs_union_branch_config;
@@ -129,9 +129,9 @@ chroot_fs_union::setup_env (environment& env)
env.add("CHROOT_FS_UNION_TYPE", get_fs_union_type());
if (get_fs_union_configured())
{
- env.add("CHROOT_FS_UNION_OVERLAY_DIRECTORY",
+ env.add("CHROOT_FS_UNION_OVERLAY_DIRECTORY",
get_overlay_session_directory());
- env.add("CHROOT_FS_UNION_BRANCH_CONFIG",
+ env.add("CHROOT_FS_UNION_BRANCH_CONFIG",
get_fs_union_branch_config());
}
}
@@ -139,10 +139,7 @@ chroot_fs_union::setup_env (environment& env)
std::string
chroot_fs_union::get_path() const
{
- if (get_fs_union_configured())
- return get_mount_location();
- else
- return chroot::get_path();
+ return get_mount_location();
}
sbuild::chroot::session_flags
@@ -150,7 +147,7 @@ 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())
+ if (get_fs_union_configured())
return SESSION_CREATE | chroot_source::get_session_flags();
else
return SESSION_CREATE;
@@ -169,7 +166,7 @@ chroot_fs_union::get_details (format_detail& detail) const
detail.add(_("Filesystem union overlay directory"),
get_overlay_session_directory());
if (!this->fs_union_branch_config.empty())
- detail.add(_("Filesystem union branch config"),
+ detail.add(_("Filesystem union branch config"),
get_fs_union_branch_config());
}
@@ -182,13 +179,13 @@ chroot_fs_union::get_keyfile (keyfile& keyfile) const
keyfile::set_object_value(*this, &chroot_fs_union::get_fs_union_type,
keyfile, get_keyfile_name(), "fs-union-type");
- keyfile::set_object_value(*this,
+ keyfile::set_object_value(*this,
&chroot_fs_union::get_fs_union_branch_config,
keyfile, get_keyfile_name(),
"fs-union-branch-config");
if (get_active())
- keyfile::set_object_value(*this,
+ keyfile::set_object_value(*this,
&chroot_fs_union::get_overlay_session_directory,
keyfile, get_keyfile_name(),
"fs-union-overlay-session-directory");
@@ -221,7 +218,6 @@ chroot_fs_union::set_keyfile (keyfile const& keyfile,
(get_fs_union_configured() ?
keyfile::PRIORITY_REQUIRED :
keyfile::PRIORITY_OPTIONAL));
-
used_keys.push_back("fs-union-overlay-session-directory");
}
diff --git a/sbuild/sbuild-chroot-loopback.cc b/sbuild/sbuild-chroot-loopback.cc
index f1c1d68..fe4455c 100644
--- a/sbuild/sbuild-chroot-loopback.cc
+++ b/sbuild/sbuild-chroot-loopback.cc
@@ -79,7 +79,7 @@ chroot_loopback::set_file (std::string const& file)
std::string
chroot_loopback::get_path () const
{
- return chroot_mountable::get_path();
+ return get_mount_location() + get_location();
}
void
diff --git a/sbuild/sbuild-chroot-mountable.cc b/sbuild/sbuild-chroot-mountable.cc
index cd205be..43345b8 100644
--- a/sbuild/sbuild-chroot-mountable.cc
+++ b/sbuild/sbuild-chroot-mountable.cc
@@ -23,6 +23,7 @@
#include "sbuild-lock.h"
#include "sbuild-util.h"
+#include <cassert>
#include <cerrno>
#include <cstring>
@@ -32,7 +33,6 @@ using boost::format;
using namespace sbuild;
chroot_mountable::chroot_mountable ():
- chroot(),
mount_device(),
mount_options(),
location(),
@@ -80,7 +80,7 @@ void
chroot_mountable::set_location (std::string const& location)
{
if (!location.empty() && !is_absname(location))
- throw error(location, LOCATION_ABS);
+ throw chroot::error(location, chroot::LOCATION_ABS);
this->location = location;
}
@@ -114,17 +114,9 @@ chroot_mountable::get_mount_uuid (bool abort_on_drop_privileges)
return mount_uuid;
}
-std::string
-chroot_mountable::get_path () const
-{
- return get_mount_location() + get_location();
-}
-
void
chroot_mountable::setup_env (environment& env)
{
- chroot::setup_env(env);
-
env.add("CHROOT_MOUNT_DEVICE", get_mount_device());
env.add("CHROOT_MOUNT_OPTIONS", get_mount_options());
env.add("CHROOT_LOCATION", get_location());
@@ -134,14 +126,12 @@ chroot_mountable::setup_env (environment& env)
sbuild::chroot::session_flags
chroot_mountable::get_session_flags () const
{
- return SESSION_NOFLAGS;
+ return chroot::SESSION_NOFLAGS;
}
void
chroot_mountable::get_details (format_detail& detail) const
{
- chroot::get_details(detail);
-
if (!get_mount_options().empty())
detail.add(_("Mount Options"), get_mount_options());
if (!get_location().empty())
@@ -151,28 +141,33 @@ chroot_mountable::get_details (format_detail& detail) const
void
chroot_mountable::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);
keyfile::set_object_value(*this, &chroot_mountable::get_mount_options,
- keyfile, get_keyfile_name(), "mount-options");
+ keyfile, base->get_keyfile_name(),
+ "mount-options");
keyfile::set_object_value(*this, &chroot_mountable::get_location,
- keyfile, get_keyfile_name(), "location");
+ keyfile, base->get_keyfile_name(), "location");
}
void
chroot_mountable::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);
keyfile::get_object_value(*this, &chroot_mountable::set_mount_options,
- keyfile, get_keyfile_name(), "mount-options",
+ keyfile, base->get_keyfile_name(), "mount-options",
keyfile::PRIORITY_OPTIONAL);
used_keys.push_back("mount-options");
keyfile::get_object_value(*this, &chroot_mountable::set_location,
- keyfile, get_keyfile_name(), "location",
+ keyfile, base->get_keyfile_name(), "location",
keyfile::PRIORITY_OPTIONAL);
used_keys.push_back("location");
}
diff --git a/sbuild/sbuild-chroot-mountable.h b/sbuild/sbuild-chroot-mountable.h
index cfa6144..c19b3e4 100644
--- a/sbuild/sbuild-chroot-mountable.h
+++ b/sbuild/sbuild-chroot-mountable.h
@@ -29,7 +29,7 @@ namespace sbuild
*
* The device will be mounted on demand.
*/
- class chroot_mountable : virtual public chroot
+ class chroot_mountable
{
protected:
/// The constructor.
@@ -91,7 +91,7 @@ namespace sbuild
*
* @param location the location.
*/
- void
+ virtual void
set_location (std::string const& location);
/**
@@ -120,13 +120,10 @@ namespace sbuild
virtual std::string const&
get_mount_uuid (bool abort_on_error = false);
- virtual std::string
- get_path () const;
-
virtual void
setup_env (environment& env);
- virtual session_flags
+ virtual chroot::session_flags
get_session_flags () const;
protected:
diff --git a/sbuild/sbuild-chroot.cc b/sbuild/sbuild-chroot.cc
index ea36972..d1bd03a 100644
--- a/sbuild/sbuild-chroot.cc
+++ b/sbuild/sbuild-chroot.cc
@@ -215,12 +215,6 @@ sbuild::chroot::set_mount_location (std::string const& location)
this->mount_location = location;
}
-std::string
-sbuild::chroot::get_path () const
-{
- return get_mount_location();
-}
-
unsigned int
sbuild::chroot::get_priority () const
{
diff --git a/sbuild/sbuild-chroot.h b/sbuild/sbuild-chroot.h
index 848f320..c9e7afc 100644
--- a/sbuild/sbuild-chroot.h
+++ b/sbuild/sbuild-chroot.h
@@ -200,7 +200,7 @@ namespace sbuild
* @returns the path.
*/
virtual std::string
- get_path () const;
+ get_path () const = 0;
/**
* Get the priority of the chroot. This is a number indicating
diff --git a/test/sbuild-chroot.cc b/test/sbuild-chroot.cc
index d7e2ad0..111d9d0 100644
--- a/test/sbuild-chroot.cc
+++ b/test/sbuild-chroot.cc
@@ -48,6 +48,10 @@ public:
get_chroot_type () const
{ static const std::string type("test"); return type; }
+ virtual std::string
+ get_path () const
+ { return get_mount_location(); }
+
virtual void
setup_env (sbuild::environment& env)
{ this->sbuild::chroot::setup_env(env); }
--
1.6.3.2
More information about the Buildd-tools-devel
mailing list