[Buildd-tools-devel] [PATCH 14/22] Remove ambiguity of location key
Jan-Marek Glogowski
glogow at fbihome.de
Thu Mar 26 21:13:52 UTC 2009
For directory and plain chroots the container is stored in the
location key. Since the introduction of the container chroot
member, this is not just ambiguous but wrong. This patch
deprecates the location key for directory and plain chroots,
if no directory key is found in the config file. On the other
hand the config parsing fails, if directory and location are set.
---
bin/schroot/schroot.conf.5.in | 6 +-
bin/schroot/setup/00check | 14 ++++--
bin/schroot/setup/10mount | 4 +-
sbuild/sbuild-chroot-directory.cc | 75 +++++++++++++++++++++++++++++++++-
sbuild/sbuild-chroot-directory.h | 13 ++++++
sbuild/sbuild-chroot-plain.cc | 4 +-
test/Makefile.am | 36 +++++++++-------
test/config-directory-deprecated.ex | 9 ++++
test/config-directory-fail.ex | 10 +++++
test/config-directory-valid.ex | 9 ++++
test/config.ex1 | 6 +-
test/config.ex2/experimental | 2 +-
test/config.ex2/sarge | 2 +-
test/config.ex2/sid | 2 +-
test/config.ex2/woody | 2 +-
test/sbuild-chroot-config.cc | 5 ++
test/sbuild-chroot-directory.cc | 50 +++++++++++++++--------
test/sbuild-chroot-plain.cc | 11 +++--
18 files changed, 200 insertions(+), 60 deletions(-)
create mode 100644 test/config-directory-deprecated.ex
create mode 100644 test/config-directory-fail.ex
create mode 100644 test/config-directory-valid.ex
diff --git a/bin/schroot/schroot.conf.5.in b/bin/schroot/schroot.conf.5.in
index 7e40583..7f3ce8c 100644
--- a/bin/schroot/schroot.conf.5.in
+++ b/bin/schroot/schroot.conf.5.in
@@ -153,7 +153,7 @@ that if \f[CI]run\-setup\-scripts\fP is set to \[oq]true\[cq] for
.PP
These chroot types have an additional (mandatory) configuration option:
.TP
-\f[CBI]location=\fP\f[CI]directory\fP
+\f[CBI]directory=\fP\f[CI]directory\fP
The directory containing the chroot environment. This is where the root will
be changed to when executing a login shell or a command. The directory must
exist and have read and execute permissions to allow users access to it. Note
@@ -307,7 +307,7 @@ this boils down to \fItrust\fP.
.br
\f[CR]description[fr_FR]=Debian instable\fP
.br
-\f[CR]location=/srv/chroot/sid\fP
+\f[CR]directory=/srv/chroot/sid\fP
.br
\f[CR]priority=3\fP
.br
@@ -396,7 +396,7 @@ this boils down to \fItrust\fP.
The system-wide chroot definition file. This file must be owned by the root
user, and not be writable by other.
.TP
-\f[BI]@SCHROOT_CONF_CHROOT_D@\fP
+\f[I]@SCHROOT_CONF_CHROOT_D@\fP
Additional chroot definitions may be placed in files under this directory.
They are treated in exactly that same manner as \fI at SCHROOT_CONF@\fP. Each
file may contain one or more chroot definitions.
diff --git a/bin/schroot/setup/00check b/bin/schroot/setup/00check
index 136d38a..f3fae64 100755
--- a/bin/schroot/setup/00check
+++ b/bin/schroot/setup/00check
@@ -53,7 +53,7 @@ if [ $1 = "setup-start" ] || [ $1 = "setup-recover" ]; then
echo "CHROOT_PATH=$CHROOT_PATH"
echo "CHROOT_CONTAINER=$CHROOT_CONTAINER"
if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ]; then
- :
+ echo "CHROOT_DIRECTORY=$CHROOT_DIRECTORY"
elif [ "$CHROOT_TYPE" = "file" ]; then
echo "CHROOT_FILE=$CHROOT_FILE"
echo "CHROOT_FILE_REPACK=$CHROOT_FILE_REPACK"
@@ -75,8 +75,8 @@ if [ $1 = "setup-start" ] || [ $1 = "setup-recover" ]; then
case "$CHROOT_TYPE" in
plain | directory)
- if [ ! -d "$CHROOT_LOCATION" ]; then
- echo "Directory '$CHROOT_LOCATION' does not exist"
+ if [ ! -d "$CHROOT_DIRECTORY" ]; then
+ echo "Directory '$CHROOT_DIRECTORY' does not exist"
exit 1
fi
;;
@@ -100,8 +100,12 @@ if [ $1 = "setup-start" ] || [ $1 = "setup-recover" ]; then
# A basic safety check, so that the root filesystem doesn't get
# toasted by accident.
- if [ -z "$CHROOT_PATH" ] || [ "$CHROOT_PATH" = "/" ] || [ "$CHROOT_LOCATION" = "/" ]; then
- echo "Invalid chroot mount location: '$CHROOT_LOCATION'"
+ if [ -z "$CHROOT_PATH" ] \
+ || [ "$CHROOT_PATH" = "/" ] \
+ || [ "$CHROOT_LOCATION" = "/" ] \
+ || [ "$CHROOT_DIRECTORY" = "/" ]
+ then
+ echo "Invalid chroot mount path, location or directory."
exit 1
fi
diff --git a/bin/schroot/setup/10mount b/bin/schroot/setup/10mount
index e667971..b8a23b7 100755
--- a/bin/schroot/setup/10mount
+++ b/bin/schroot/setup/10mount
@@ -74,10 +74,10 @@ if [ "$CHROOT_TYPE" = "plain" ] || [ "$CHROOT_TYPE" = "directory" ] || [ "$CHROO
if [ "$CHROOT_TYPE" = "plain" ]; then
CHROOT_MOUNT_OPTIONS="--rbind"
- CHROOT_MOUNT_DEVICE="$CHROOT_LOCATION"
+ CHROOT_MOUNT_DEVICE="$CHROOT_DIRECTORY"
elif [ "$CHROOT_TYPE" = "directory" ]; then
CHROOT_MOUNT_OPTIONS="--bind"
- CHROOT_MOUNT_DEVICE="$CHROOT_LOCATION"
+ CHROOT_MOUNT_DEVICE="$CHROOT_DIRECTORY"
elif [ "$CHROOT_TYPE" = "file" ]; then
UNPACK_LOCATION="${UNPACK_DIR}/${SESSION_ID}"
CHROOT_MOUNT_OPTIONS="--bind"
diff --git a/sbuild/sbuild-chroot-directory.cc b/sbuild/sbuild-chroot-directory.cc
index 237b616..2ffd5b0 100644
--- a/sbuild/sbuild-chroot-directory.cc
+++ b/sbuild/sbuild-chroot-directory.cc
@@ -91,7 +91,76 @@ chroot_directory::set_container (std::string const& directory)
throw error(directory, LOCATION_ABS);
chroot::set_container(directory);
- chroot::set_location(directory);
+}
+
+void
+chroot_directory::setup_env (environment& env)
+{
+ chroot::setup_env(env);
+ env.add("CHROOT_DIRECTORY", get_container());
+}
+
+void
+chroot_directory::get_details (format_detail& detail) const
+{
+ chroot::get_details(detail);
+ detail.add(_("Directory"), get_container());
+}
+
+void
+chroot_directory::get_keyfile (keyfile& keyfile) const
+{
+ chroot::get_keyfile(keyfile);
+
+ keyfile::set_object_value(*(static_cast<const sbuild::chroot*>(this)),
+ &chroot::get_container,
+ keyfile, get_name(), "directory");
+}
+
+void
+chroot_directory::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
+{
+ chroot::set_keyfile(keyfile, used_keys);
+
+ // Transition from previous location key to directory key
+ keyfile::priority prio_directory, prio_location;
+
+ if( ! get_active() )
+ {
+ prio_directory = keyfile::PRIORITY_DISALLOWED;
+ prio_location = keyfile::PRIORITY_DEPRECATED;
+
+ if (keyfile.has_key(get_name(), "directory"))
+ {
+ prio_directory = keyfile::PRIORITY_REQUIRED;
+ prio_location = keyfile::PRIORITY_DISALLOWED;
+ }
+ }
+ else
+ {
+ prio_directory = keyfile::PRIORITY_DISALLOWED;
+ prio_location = keyfile::PRIORITY_REQUIRED;
+
+ if (keyfile.has_key(get_name(), "directory"))
+ {
+ prio_directory = keyfile::PRIORITY_REQUIRED;
+ prio_location = keyfile::PRIORITY_OPTIONAL;
+ }
+ }
+
+ keyfile::get_object_value(*this, &chroot_directory::set_container,
+ keyfile, get_name(), "location",
+ prio_location);
+ used_keys.push_back("location");
+
+ keyfile::get_object_value(*this, &chroot_directory::set_container,
+ keyfile, get_name(), "directory",
+ prio_directory);
+ used_keys.push_back("directory");
+
+ // This key was always unused, so this just keeps schroot calm.
+ used_keys.push_back("mount-device");
}
void
@@ -102,8 +171,8 @@ chroot_directory::get_chroot_strings (std::string *type,
if (type != NULL)
*type = "directory";
if (key_name != NULL)
- *key_name = "location";
+ key_name->clear();
if (detail_name != NULL)
- *detail_name = "Location";
+ *detail_name = "Directory";
}
diff --git a/sbuild/sbuild-chroot-directory.h b/sbuild/sbuild-chroot-directory.h
index 3dedc55..117e951 100644
--- a/sbuild/sbuild-chroot-directory.h
+++ b/sbuild/sbuild-chroot-directory.h
@@ -61,6 +61,19 @@ namespace sbuild
virtual session_flags
get_session_flags () const;
+ virtual void
+ setup_env (environment& env);
+
+ virtual void
+ get_details (format_detail& detail) const;
+
+ virtual void
+ get_keyfile (keyfile& keyfile) const;
+
+ virtual void
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
+
protected:
virtual void
setup_lock (chroot::setup_type type,
diff --git a/sbuild/sbuild-chroot-plain.cc b/sbuild/sbuild-chroot-plain.cc
index 437c67e..3852324 100644
--- a/sbuild/sbuild-chroot-plain.cc
+++ b/sbuild/sbuild-chroot-plain.cc
@@ -54,8 +54,8 @@ chroot_plain::get_chroot_strings (std::string *type,
if (type != NULL)
*type = "plain";
if (key_name != NULL)
- *key_name = "location";
+ key_name->clear();
if (detail_name != NULL)
- *detail_name = "Location";
+ detail_name->clear();
}
diff --git a/test/Makefile.am b/test/Makefile.am
index 4dcaf0d..ae7dcc1 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -21,7 +21,8 @@
include $(top_srcdir)/scripts/global.mk
-LOCAL_CXXFLAGS = $(SCHROOT_CFLAGS) $(CPPUNIT_CFLAGS) -DTESTDATADIR='"./testdata"' -I$(top_srcdir)/bin
+LOCAL_CXXFLAGS = $(SCHROOT_CFLAGS) $(CPPUNIT_CFLAGS) -I$(top_srcdir)/bin \
+ -DTESTDATADIR='"./testdata"'
if USE_UNIT_TESTS
noinst_LTLIBRARIES = libtest.la
@@ -115,21 +116,24 @@ sbuild_util_LDADD = libtest.la
schroot_base_option_action_SOURCES = schroot-base-option-action.cc
schroot_base_option_action_LDADD = $(top_builddir)/bin/schroot-base/libschroot-base.la libtest.la
-EXTRA_DIST = \
- keyfile.ex1 \
- config.ex1 \
- config.ex2/file \
- config.ex2/empty \
- config.ex2/experimental \
- config.ex2/sarge \
- config.ex2/sid \
- config.ex2/woody \
- run-parts.ex1/10test1 \
- run-parts.ex1/20test2 \
- run-parts.ex1/30test3 \
- run-parts.ex2 \
- run-parts.ex3/50invalid \
- setup-test-data \
+EXTRA_DIST = \
+ keyfile.ex1 \
+ config.ex1 \
+ config.ex2/file \
+ config.ex2/empty \
+ config.ex2/experimental \
+ config.ex2/sarge \
+ config.ex2/sid \
+ config.ex2/woody \
+ config-directory-deprecated.ex \
+ config-directory-fail.ex \
+ config-directory-valid.ex \
+ run-parts.ex1/10test1 \
+ run-parts.ex1/20test2 \
+ run-parts.ex1/30test3 \
+ run-parts.ex2 \
+ run-parts.ex3/50invalid \
+ setup-test-data \
cleanup-test-data
clean-local:
diff --git a/test/config-directory-deprecated.ex b/test/config-directory-deprecated.ex
new file mode 100644
index 0000000..fe0fa9b
--- /dev/null
+++ b/test/config-directory-deprecated.ex
@@ -0,0 +1,9 @@
+[deprecated]
+description=Debian experimental
+location=/srv/chroot/experimental
+priority=3
+groups=sbuild,root
+root-groups=root,sbuild
+run-setup-scripts=true
+run-exec-scripts=true
+
diff --git a/test/config-directory-fail.ex b/test/config-directory-fail.ex
new file mode 100644
index 0000000..b27f1f1
--- /dev/null
+++ b/test/config-directory-fail.ex
@@ -0,0 +1,10 @@
+[fail]
+description=Debian experimental
+directory=/srv/chroot/experimental
+location=/srv/chroot/experimental
+priority=3
+groups=sbuild,root
+root-groups=root,sbuild
+run-setup-scripts=true
+run-exec-scripts=true
+
diff --git a/test/config-directory-valid.ex b/test/config-directory-valid.ex
new file mode 100644
index 0000000..a05988f
--- /dev/null
+++ b/test/config-directory-valid.ex
@@ -0,0 +1,9 @@
+[valid]
+description=Debian experimental
+directory=/srv/chroot/experimental
+priority=3
+groups=sbuild,root
+root-groups=root,sbuild
+run-setup-scripts=true
+run-exec-scripts=true
+
diff --git a/test/config.ex1 b/test/config.ex1
index 0349c18..d131acd 100644
--- a/test/config.ex1
+++ b/test/config.ex1
@@ -35,7 +35,7 @@ description=Debian sid (unstable)
priority=3
groups=sbuild,root
root-groups=root,sbuild
-location=/srv/chroot/sid
+directory=/srv/chroot/sid
[sid-snap]
type=lvm-snapshot
@@ -58,7 +58,7 @@ run-exec-scripts=true
[sarge]
description=Debian sarge (stable)
-location=/srv/chroot/sarge
+directory=/srv/chroot/sarge
priority=2
groups=sbuild
root-groups=root,sbuild
@@ -69,7 +69,7 @@ run-exec-scripts=true
[experimental]
description=Debian experimental
-location=/srv/chroot/experimental
+directory=/srv/chroot/experimental
priority=3
groups=sbuild,root
root-groups=root,sbuild
diff --git a/test/config.ex2/experimental b/test/config.ex2/experimental
index f4de9ad..da30416 100644
--- a/test/config.ex2/experimental
+++ b/test/config.ex2/experimental
@@ -1,6 +1,6 @@
[experimental]
description=Debian experimental
-location=/srv/chroot/experimental
+directory=/srv/chroot/experimental
priority=3
groups=sbuild,root
root-groups=root,sbuild
diff --git a/test/config.ex2/sarge b/test/config.ex2/sarge
index 59ee7f6..afb376b 100644
--- a/test/config.ex2/sarge
+++ b/test/config.ex2/sarge
@@ -1,6 +1,6 @@
[sarge]
description=Debian sarge (stable)
-location=/srv/chroot/sarge
+directory=/srv/chroot/sarge
priority=2
groups=sbuild
root-groups=root,sbuild
diff --git a/test/config.ex2/sid b/test/config.ex2/sid
index 253f489..01ec816 100644
--- a/test/config.ex2/sid
+++ b/test/config.ex2/sid
@@ -17,7 +17,7 @@ description=Debian sid (unstable)
priority=3
groups=sbuild,root
root-groups=root,sbuild
-location=/srv/chroot/sid
+directory=/srv/chroot/sid
[sid-snap]
type=lvm-snapshot
diff --git a/test/config.ex2/woody b/test/config.ex2/woody
index 4df8b2f..b4b5cab 100644
--- a/test/config.ex2/woody
+++ b/test/config.ex2/woody
@@ -1,6 +1,6 @@
[woody]
description=Debian woody (oldstable)
-location=/home/woody-chroot
+directory=/home/woody-chroot
groups=sbuild
aliases=oldstable
diff --git a/test/sbuild-chroot-config.cc b/test/sbuild-chroot-config.cc
index ecde8c4..6581c5a 100644
--- a/test/sbuild-chroot-config.cc
+++ b/test/sbuild-chroot-config.cc
@@ -82,6 +82,11 @@ public:
sbuild::chroot_config c(TESTDATADIR "/config.nonexistent", false);
}
+ void test_construction_fail_wrong()
+ {
+ sbuild::chroot_config c(TESTDATADIR "/config.ex3", false);
+ }
+
void test_add_file()
{
sbuild::chroot_config c;
diff --git a/test/sbuild-chroot-directory.cc b/test/sbuild-chroot-directory.cc
index 2e9f088..876f5d3 100644
--- a/test/sbuild-chroot-directory.cc
+++ b/test/sbuild-chroot-directory.cc
@@ -52,6 +52,9 @@ class test_chroot_directory : public test_chroot_base<chroot_directory>
CPPUNIT_TEST(test_session_flags);
CPPUNIT_TEST(test_print_details);
CPPUNIT_TEST(test_print_config);
+ CPPUNIT_TEST_EXCEPTION(test_config_fail, sbuild::error_base);
+ CPPUNIT_TEST(test_config_deprecated);
+ CPPUNIT_TEST(test_config_valid);
CPPUNIT_TEST_SUITE_END();
public:
@@ -76,7 +79,7 @@ public:
chroot->set_run_setup_scripts(false);
c->set_container("/mnt/mount-location/example");
chroot->set_mount_location("");
- CPPUNIT_ASSERT(c->get_location() == "/mnt/mount-location/example");
+ CPPUNIT_ASSERT(c->get_location() == "");
CPPUNIT_ASSERT(c->get_container() == "/mnt/mount-location/example");
CPPUNIT_ASSERT(chroot->get_path() == "/mnt/mount-location/example");
CPPUNIT_ASSERT(chroot->get_mount_location() == "");
@@ -88,24 +91,30 @@ public:
chroot->get_chroot_strings(&type, &key_name, &detail_name);
CPPUNIT_ASSERT(type == "directory");
- CPPUNIT_ASSERT(key_name == "location");
- CPPUNIT_ASSERT(detail_name == "Location");
+ CPPUNIT_ASSERT(key_name.empty());
+ CPPUNIT_ASSERT(detail_name == "Directory");
}
- void test_setup_env()
+ void test_setup_env_common(sbuild::environment& expected)
{
- sbuild::environment expected;
expected.add("CHROOT_TYPE", "directory");
expected.add("CHROOT_NAME", "test-name");
expected.add("CHROOT_DESCRIPTION", "test-description");
expected.add("CHROOT_MOUNT_LOCATION", "/mnt/mount-location");
expected.add("CHROOT_CONTAINER", "/srv/chroot/example-chroot");
- expected.add("CHROOT_LOCATION", "/srv/chroot/example-chroot");
- expected.add("CHROOT_PATH", "/srv/chroot/example-chroot");
+ expected.add("CHROOT_LOCATION", "");
+ expected.add("CHROOT_DIRECTORY", "/srv/chroot/example-chroot");
expected.add("CHROOT_SCRIPT_CONFIG", sbuild::normalname(std::string(PACKAGE_SYSCONF_DIR) + "/script-defaults"));
expected.add("CHROOT_SESSION_CLONE", "false");
- expected.add("CHROOT_SESSION_CREATE", "false");
expected.add("CHROOT_SESSION_PURGE", "false");
+ }
+
+ void test_setup_env()
+ {
+ sbuild::environment expected;
+ test_setup_env_common(expected);
+ expected.add("CHROOT_PATH", "/srv/chroot/example-chroot");
+ expected.add("CHROOT_SESSION_CREATE", "false");
test_chroot_base<chroot_directory>::test_setup_env(expected);
}
@@ -115,17 +124,9 @@ public:
chroot->set_run_setup_scripts(true);
sbuild::environment expected;
- expected.add("CHROOT_TYPE", "directory");
- expected.add("CHROOT_NAME", "test-name");
- expected.add("CHROOT_DESCRIPTION", "test-description");
- expected.add("CHROOT_MOUNT_LOCATION", "/mnt/mount-location");
- expected.add("CHROOT_LOCATION", "/srv/chroot/example-chroot");
- expected.add("CHROOT_CONTAINER", "/srv/chroot/example-chroot");
+ test_setup_env_common(expected);
expected.add("CHROOT_PATH", "/mnt/mount-location");
- expected.add("CHROOT_SCRIPT_CONFIG", sbuild::normalname(std::string(PACKAGE_SYSCONF_DIR) + "/script-defaults"));
- expected.add("CHROOT_SESSION_CLONE", "false");
expected.add("CHROOT_SESSION_CREATE", "true");
- expected.add("CHROOT_SESSION_PURGE", "false");
test_chroot_base<chroot_directory>::test_setup_env(expected);
}
@@ -157,6 +158,21 @@ public:
// TODO: Compare output.
CPPUNIT_ASSERT(!os.str().empty());
}
+
+ void test_config_fail()
+ {
+ sbuild::chroot_config c(TESTDATADIR "/config-directory-fail.ex", false);
+ }
+
+ void test_config_deprecated()
+ {
+ sbuild::chroot_config c(TESTDATADIR "/config-directory-deprecated.ex", false);
+ }
+
+ void test_config_valid()
+ {
+ sbuild::chroot_config c(TESTDATADIR "/config-directory-valid.ex", false);
+ }
};
CPPUNIT_TEST_SUITE_REGISTRATION(test_chroot_directory);
diff --git a/test/sbuild-chroot-plain.cc b/test/sbuild-chroot-plain.cc
index 8b2572d..a56e6ac 100644
--- a/test/sbuild-chroot-plain.cc
+++ b/test/sbuild-chroot-plain.cc
@@ -44,7 +44,7 @@ public:
class test_chroot_plain : public test_chroot_base<chroot_plain>
{
CPPUNIT_TEST_SUITE(test_chroot_plain);
- CPPUNIT_TEST(test_location);
+ CPPUNIT_TEST(test_directory);
CPPUNIT_TEST(test_chroot_strings);
CPPUNIT_TEST(test_setup_env);
CPPUNIT_TEST(test_print_details);
@@ -65,7 +65,7 @@ public:
}
void
- test_location()
+ test_directory()
{
sbuild::chroot_plain *c = dynamic_cast<sbuild::chroot_plain *>(chroot.get());
CPPUNIT_ASSERT(c);
@@ -81,8 +81,8 @@ public:
chroot->get_chroot_strings(&type, &key_name, &detail_name);
CPPUNIT_ASSERT(type == "plain");
- CPPUNIT_ASSERT(key_name == "location");
- CPPUNIT_ASSERT(detail_name == "Location");
+ CPPUNIT_ASSERT(key_name.empty());
+ CPPUNIT_ASSERT(detail_name.empty());
}
void test_setup_env()
@@ -91,8 +91,9 @@ public:
expected.add("CHROOT_TYPE", "plain");
expected.add("CHROOT_NAME", "test-name");
expected.add("CHROOT_DESCRIPTION", "test-description");
- expected.add("CHROOT_LOCATION", "/srv/chroot/example-chroot");
+ expected.add("CHROOT_LOCATION", "");
expected.add("CHROOT_CONTAINER", "/srv/chroot/example-chroot");
+ expected.add("CHROOT_DIRECTORY", "/srv/chroot/example-chroot");
expected.add("CHROOT_PATH", "/srv/chroot/example-chroot");
expected.add("CHROOT_SCRIPT_CONFIG", sbuild::normalname(std::string(PACKAGE_SYSCONF_DIR) + "/script-defaults"));
expected.add("CHROOT_SESSION_CLONE", "false");
--
1.6.2.1
More information about the Buildd-tools-devel
mailing list