[Buildd-tools-devel] Bug#459658: Bug#459658: marked as done (schroot: Unable to read properly run-exec-scripts option.)
Roger Leigh
rleigh at whinlatter.ukfsn.org
Tue Jan 15 22:58:00 UTC 2008
tags 459658 + patch fixed-upstream pending
thanks
On Tue, Jan 08, 2008 at 10:59:21PM +0000, Roger Leigh wrote:
> reopen 459658
> retitle 459658 sbuild::chroot::set_keyfile should warn if unused keys
> are present
> thanks
>
> >> > I'm also attaching the schroot -i -c sid32 --debug=notice output.
> >>
> >> Thanks. The reason for this is due to a typo in the configuration
> >
> > Stupid me, my fault. I'm closing this accordingly.
>
> I'm reopening this to act as a reminder for me to fix this.
>
> Reminder to self:
>
> Wrap sbuild::keyfile::get_object_value and
> sbuild::keyfile::get_object_list_value to either delete or note all
> keys used. When parsing is done, list any remaining keys (if
> deleting) or subtract the used keys from the whole set, and display
> the result.
>
> If deleting, duplicate the keyfile (for the chroot group only), so
> the original remains intact.
>
The following patch was committed into GIT to fix this. It will
be included in the next release of schroot.
Regards,
Roger
diff --git a/ChangeLog b/ChangeLog
index 0239b01..6d0f68d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,28 @@
+2008-01-15 Roger Leigh <rleigh at debian.org>
+
+ * All classes derived from sbuild::chroot updated to use new
+ set_keyfile argument.
+
+ * sbuild/sbuild-chroot.cc
+ (set_keyfile): sets used_keys for each key used.
+
+ * sbuild/sbuild-chroot.h
+ (operator >>): Get list of used keys, and pass to
+ keyfile::check_keys to find unused keys.
+ (set_keyfile): Add used_keys parameter.
+
+ * sbuild/sbuild-basic-keyfile.tcc
+ (check_keys): Use set_difference to compare used and available
+ keys, and print warnings about the differences.
+
+ * sbuild/sbuild-basic-keyfile.h: Make user-visible types public.
+ Add check_keys method to warn about unused keys.
+
+ * sbuild/sbuild-keyfile-base.(cc|h): Add UNKNOWN_KEY error code
+ and string.
+
+ * sbuild/sbuild-types.h: Add string_set typedef.
+
2008-01-13 Roger Leigh <rleigh at debian.org>
* bin/schroot/exec/*: Use set -e to ensure abort on failure.
diff --git a/sbuild/sbuild-basic-keyfile.h b/sbuild/sbuild-basic-keyfile.h
index d7b8309..84b7a51 100644
--- a/sbuild/sbuild-basic-keyfile.h
+++ b/sbuild/sbuild-basic-keyfile.h
@@ -129,7 +129,7 @@ namespace sbuild
template <typename K, typename P = basic_keyfile_parser<K> >
class basic_keyfile : public keyfile_base
{
- private:
+ public:
/// Group name.
typedef typename K::group_name_type group_name_type;
@@ -145,6 +145,7 @@ namespace sbuild
/// Line number.
typedef typename K::size_type size_type;
+ private:
/// Parse type.
typedef P parse_type;
@@ -202,6 +203,18 @@ namespace sbuild
get_keys (group_name_type const& group) const;
/**
+ * Check for unused keys in a group. If keys other than the
+ * specified keys exist in the specified group, print a warning
+ * about unknown keys having been used.
+ *
+ * @param group the group to use.
+ * @param keys the keys which have been used.
+ */
+ void
+ check_keys (group_name_type const& group,
+ string_list const& keys) const;
+
+ /**
* Check if a group exists.
*
* @param group the group to check for.
diff --git a/sbuild/sbuild-basic-keyfile.tcc b/sbuild/sbuild-basic-keyfile.tcc
index ac6214b..5f9499e 100644
--- a/sbuild/sbuild-basic-keyfile.tcc
+++ b/sbuild/sbuild-basic-keyfile.tcc
@@ -16,7 +16,9 @@
*
*********************************************************************/
+#include <algorithm>
#include <fstream>
+#include <set>
template <typename K, typename P>
sbuild::basic_keyfile<K, P>::basic_keyfile ():
@@ -89,6 +91,33 @@ sbuild::basic_keyfile<K, P>::get_keys (group_name_type const& group) const
}
template <typename K, typename P>
+void
+sbuild::basic_keyfile<K, P>::check_keys (group_name_type const& group,
+ string_list const& keys) const
+{
+ const string_list total(get_keys(group));
+
+ const string_set a(total.begin(), total.end());
+ const string_set b(keys.begin(), keys.end());
+
+ string_set unused;
+
+ set_difference(a.begin(), a.end(),
+ b.begin(), b.end(),
+ inserter(unused, unused.begin()));
+
+ for (string_set::const_iterator pos = unused.begin();
+ pos != unused.end();
+ ++pos)
+ {
+ size_type line = get_line(group, *pos);
+ error e(line, group, UNKNOWN_KEY, *pos);
+ e.set_reason(_("This option may be present in a newer version"));
+ log_exception_warning(e);
+ }
+}
+
+template <typename K, typename P>
bool
sbuild::basic_keyfile<K, P>::has_group (group_name_type const& group) const
{
diff --git a/sbuild/sbuild-chroot-block-device.cc b/sbuild/sbuild-chroot-block-device.cc
index 76db5d6..97f5643 100644
--- a/sbuild/sbuild-chroot-block-device.cc
+++ b/sbuild/sbuild-chroot-block-device.cc
@@ -192,19 +192,23 @@ chroot_block_device::get_keyfile (keyfile& keyfile) const
}
void
-chroot_block_device::set_keyfile (keyfile const& keyfile)
+chroot_block_device::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
- chroot::set_keyfile(keyfile);
+ chroot::set_keyfile(keyfile, used_keys);
keyfile::get_object_value(*this, &chroot_block_device::set_device,
keyfile, get_name(), "device",
keyfile::PRIORITY_REQUIRED);
+ used_keys.push_back("device");
keyfile::get_object_value(*this, &chroot_block_device::set_mount_options,
keyfile, get_name(), "mount-options",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("mount-options");
keyfile::get_object_value(*this, &chroot_block_device::set_location,
keyfile, get_name(), "location",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("location");
}
diff --git a/sbuild/sbuild-chroot-block-device.h b/sbuild/sbuild-chroot-block-device.h
index d7a891b..d542a09 100644
--- a/sbuild/sbuild-chroot-block-device.h
+++ b/sbuild/sbuild-chroot-block-device.h
@@ -122,7 +122,8 @@ namespace sbuild
get_keyfile (keyfile& keyfile) const;
virtual void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
private:
/// The block device to use.
diff --git a/sbuild/sbuild-chroot-directory.cc b/sbuild/sbuild-chroot-directory.cc
index a576a62..bd2dad8 100644
--- a/sbuild/sbuild-chroot-directory.cc
+++ b/sbuild/sbuild-chroot-directory.cc
@@ -123,11 +123,13 @@ chroot_directory::get_keyfile (keyfile& keyfile) const
}
void
-chroot_directory::set_keyfile (keyfile const& keyfile)
+chroot_directory::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
- chroot::set_keyfile(keyfile);
+ chroot::set_keyfile(keyfile, used_keys);
keyfile::get_object_value(*this, &chroot_directory::set_location,
keyfile, get_name(), "location",
keyfile::PRIORITY_REQUIRED);
+ used_keys.push_back("location");
}
diff --git a/sbuild/sbuild-chroot-directory.h b/sbuild/sbuild-chroot-directory.h
index 607563a..e3a6923 100644
--- a/sbuild/sbuild-chroot-directory.h
+++ b/sbuild/sbuild-chroot-directory.h
@@ -80,7 +80,9 @@ namespace sbuild
get_keyfile (keyfile& keyfile) const;
virtual void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
+
};
}
diff --git a/sbuild/sbuild-chroot-file.cc b/sbuild/sbuild-chroot-file.cc
index a511e08..9072685 100644
--- a/sbuild/sbuild-chroot-file.cc
+++ b/sbuild/sbuild-chroot-file.cc
@@ -170,18 +170,21 @@ chroot_file::get_keyfile (keyfile& keyfile) const
}
void
-chroot_file::set_keyfile (keyfile const& keyfile)
+chroot_file::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
- chroot::set_keyfile(keyfile);
- chroot_source::set_keyfile(keyfile);
+ chroot::set_keyfile(keyfile, used_keys);
+ chroot_source::set_keyfile(keyfile, used_keys);
keyfile::get_object_value(*this, &chroot_file::set_file,
keyfile, get_name(), "file",
keyfile::PRIORITY_REQUIRED);
+ used_keys.push_back("file");
keyfile::get_object_value(*this, &chroot_file::set_file_repack,
keyfile, get_name(), "file-repack",
get_active() ?
keyfile::PRIORITY_REQUIRED :
keyfile::PRIORITY_DISALLOWED);
+ used_keys.push_back("file-repack");
}
diff --git a/sbuild/sbuild-chroot-file.h b/sbuild/sbuild-chroot-file.h
index e046160..edcfb67 100644
--- a/sbuild/sbuild-chroot-file.h
+++ b/sbuild/sbuild-chroot-file.h
@@ -105,7 +105,8 @@ namespace sbuild
get_keyfile (keyfile& keyfile) const;
virtual void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
private:
/// The file to use.
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.cc b/sbuild/sbuild-chroot-lvm-snapshot.cc
index d31b2a2..02a1e18 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.cc
+++ b/sbuild/sbuild-chroot-lvm-snapshot.cc
@@ -212,18 +212,21 @@ chroot_lvm_snapshot::get_keyfile (keyfile& keyfile) const
}
void
-chroot_lvm_snapshot::set_keyfile (keyfile const& keyfile)
+chroot_lvm_snapshot::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
- chroot_block_device::set_keyfile(keyfile);
- chroot_source::set_keyfile(keyfile);
+ chroot_block_device::set_keyfile(keyfile, used_keys);
+ chroot_source::set_keyfile(keyfile, used_keys);
keyfile::get_object_value(*this, &chroot_lvm_snapshot::set_snapshot_device,
keyfile, get_name(), "lvm-snapshot-device",
get_active() ?
keyfile::PRIORITY_REQUIRED :
keyfile::PRIORITY_DISALLOWED);
+ used_keys.push_back("lvm-snapshot-device");
keyfile::get_object_value(*this, &chroot_lvm_snapshot::set_snapshot_options,
keyfile, get_name(), "lvm-snapshot-options",
keyfile::PRIORITY_REQUIRED);
+ used_keys.push_back("lvm-snapshot-options");
}
diff --git a/sbuild/sbuild-chroot-lvm-snapshot.h b/sbuild/sbuild-chroot-lvm-snapshot.h
index 8c976a5..6d9a4aa 100644
--- a/sbuild/sbuild-chroot-lvm-snapshot.h
+++ b/sbuild/sbuild-chroot-lvm-snapshot.h
@@ -110,7 +110,8 @@ namespace sbuild
get_keyfile (keyfile& keyfile) const;
virtual void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
private:
/// LVM snapshot device name for lvcreate.
diff --git a/sbuild/sbuild-chroot-source.cc b/sbuild/sbuild-chroot-source.cc
index d93c393..3a31a1e 100644
--- a/sbuild/sbuild-chroot-source.cc
+++ b/sbuild/sbuild-chroot-source.cc
@@ -144,21 +144,26 @@ chroot_source::get_keyfile (keyfile& keyfile) const
}
void
-chroot_source::set_keyfile (keyfile const& keyfile)
+chroot_source::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
keyfile::get_object_list_value(*this, &chroot_source::set_source_users,
keyfile, get_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_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_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_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 6b1129f..670526a 100644
--- a/sbuild/sbuild-chroot-source.h
+++ b/sbuild/sbuild-chroot-source.h
@@ -155,7 +155,8 @@ namespace sbuild
get_keyfile (keyfile& keyfile) const;
void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
private:
/// Users allowed to access the source chroot.
diff --git a/sbuild/sbuild-chroot.cc b/sbuild/sbuild-chroot.cc
index 2a8b957..8ae6045 100644
--- a/sbuild/sbuild-chroot.cc
+++ b/sbuild/sbuild-chroot.cc
@@ -26,10 +26,8 @@
#include "sbuild-chroot-lvm-snapshot.h"
#include "sbuild-lock.h"
-#include <algorithm>
#include <cerrno>
#include <map>
-#include <set>
#include <utility>
#include <ext/stdio_filebuf.h>
@@ -567,78 +565,109 @@ sbuild::chroot::get_keyfile (keyfile& keyfile) const
}
void
-sbuild::chroot::set_keyfile (keyfile const& keyfile)
+sbuild::chroot::set_keyfile (keyfile const& keyfile,
+ string_list& used_keys)
{
+ // Keys which are used elsewhere, but should be counted as "used".
+ used_keys.push_back("type");
+
+ string_list keys = keyfile.get_keys(get_name());
+ for (string_list::const_iterator pos = keys.begin();
+ pos != keys.end();
+ ++pos)
+ {
+ static regex description_keys("^description\\[.*\\]$");
+ if (regex_search(*pos, description_keys))
+ used_keys.push_back(*pos);
+ }
+
// This is set not in the configuration file, but set in the keyfile
// manually. The user must not have the ability to set this option.
keyfile::get_object_value(*this, &chroot::set_active,
keyfile, get_name(), "active",
keyfile::PRIORITY_REQUIRED);
+ used_keys.push_back("active");
keyfile::get_object_value(*this, &chroot::set_run_setup_scripts,
keyfile, get_name(), "run-setup-scripts",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("run-setup-scripts");
keyfile::get_object_value(*this, &chroot::set_run_exec_scripts,
keyfile, get_name(), "run-session-scripts",
keyfile::PRIORITY_DEPRECATED);
+ used_keys.push_back("run-session-scripts");
keyfile::get_object_value(*this, &chroot::set_run_exec_scripts,
keyfile, get_name(), "run-exec-scripts",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("run-exec-scripts");
keyfile::get_object_value(*this, &chroot::set_script_config,
keyfile, get_name(), "script-config",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("script-config");
keyfile::get_object_value(*this, &chroot::set_priority,
keyfile, get_name(), "priority",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("priority");
keyfile::get_object_list_value(*this, &chroot::set_aliases,
keyfile, get_name(), "aliases",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("aliases");
keyfile::get_object_value(*this, &chroot::set_environment_filter,
keyfile, get_name(), "environment-filter",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("environment-filter");
keyfile::get_object_value(*this, &chroot::set_description,
keyfile, get_name(), "description",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("description");
keyfile::get_object_list_value(*this, &chroot::set_users,
keyfile, get_name(), "users",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("users");
keyfile::get_object_list_value(*this, &chroot::set_groups,
keyfile, get_name(), "groups",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("groups");
keyfile::get_object_list_value(*this, &chroot::set_root_users,
keyfile, get_name(), "root-users",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("root-users");
keyfile::get_object_list_value(*this, &chroot::set_root_groups,
keyfile, get_name(), "root-groups",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("root-groups");
keyfile::get_object_value(*this, &chroot::set_mount_location,
keyfile, get_name(), "mount-location",
get_active() ?
keyfile::PRIORITY_REQUIRED :
keyfile::PRIORITY_DISALLOWED);
+ used_keys.push_back("mount-location");
keyfile::get_object_value(*this, &chroot::set_mount_device,
keyfile, get_name(), "mount-device",
get_active() ?
keyfile::PRIORITY_OPTIONAL :
keyfile::PRIORITY_DISALLOWED);
+ used_keys.push_back("mount-device");
keyfile::get_object_list_value(*this, &chroot::set_command_prefix,
keyfile, get_name(), "command-prefix",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("command-prefix");
keyfile::get_object_value(*this, &chroot::set_persona,
keyfile, get_name(), "personality",
keyfile::PRIORITY_OPTIONAL);
+ used_keys.push_back("personality");
}
diff --git a/sbuild/sbuild-chroot.h b/sbuild/sbuild-chroot.h
index f850b42..2910f74 100644
--- a/sbuild/sbuild-chroot.h
+++ b/sbuild/sbuild-chroot.h
@@ -571,7 +571,9 @@ namespace sbuild
operator >> (keyfile const& keyfile,
ptr& rhs)
{
- rhs->set_keyfile(keyfile);
+ string_list used;
+ rhs->set_keyfile(keyfile, used);
+ keyfile.check_keys(rhs->get_name(), used);
return keyfile;
}
@@ -623,9 +625,11 @@ namespace sbuild
* be determined.
*
* @param keyfile the keyfile to get the properties from.
+ * @param used_keys a list of the keys used will be set.
*/
virtual void
- set_keyfile (keyfile const& keyfile);
+ set_keyfile (keyfile const& keyfile,
+ string_list& used_keys);
private:
/// Chroot name.
diff --git a/sbuild/sbuild-keyfile-base.cc b/sbuild/sbuild-keyfile-base.cc
index 001e4e6..16d2d28 100644
--- a/sbuild/sbuild-keyfile-base.cc
+++ b/sbuild/sbuild-keyfile-base.cc
@@ -119,7 +119,12 @@ namespace
// TRANSLATORS: %3% = key name ("keyname=value" in configuration file)
// TRANSLATORS: %4% = additional details
emap(keyfile_base::PASSTHROUGH_LGK,
- N_("line %1% [%2%] %3%: %4%"))
+ N_("line %1% [%2%] %3%: %4%")),
+ // TRANSLATORS: %1% = line number in configuration file
+ // TRANSLATORS: %2% = group name ("[groupname]" in configuration file)
+ // TRANSLATORS: %4% = key name ("keyname=value" in configuration file)
+ emap(keyfile_base::UNKNOWN_KEY,
+ N_("line %1% [%2%]: Unknown key '%4%' used")),
};
}
diff --git a/sbuild/sbuild-keyfile-base.h b/sbuild/sbuild-keyfile-base.h
index 5fabe3c..43cc7df 100644
--- a/sbuild/sbuild-keyfile-base.h
+++ b/sbuild/sbuild-keyfile-base.h
@@ -74,7 +74,8 @@ namespace sbuild
PASSTHROUGH_G, ///< Pass through exception with group.
PASSTHROUGH_GK, ///< Pass through exception with group and key.
PASSTHROUGH_LG, ///< Pass through exception with line and group.
- PASSTHROUGH_LGK ///< Pass through exception with line, group and key.
+ PASSTHROUGH_LGK, ///< Pass through exception with line, group and key.
+ UNKNOWN_KEY ///< The key is unknown.
};
/// Exception type.
diff --git a/sbuild/sbuild-types.h b/sbuild/sbuild-types.h
index 5804b0f..dedf1e5 100644
--- a/sbuild/sbuild-types.h
+++ b/sbuild/sbuild-types.h
@@ -23,6 +23,7 @@
#include <ctime>
#include <ios>
#include <locale>
+#include <set>
#include <string>
#include <vector>
@@ -32,6 +33,9 @@ namespace sbuild
/// A string vector.
typedef std::vector<std::string> string_list;
+ /// A string set.
+ typedef std::set<std::string> string_set;
+
/**
* A date representation.
*/
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20080115/71b296b3/attachment.pgp
More information about the Buildd-tools-devel
mailing list