[PATCH 0/2] Change the way of handling CONFFILES

[PATCH 0/2] Change the way of handling CONFFILES

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

It's a very common situation in OE/Yocto that the recipe authors/maintainers
either forget to set the CONFFILES variable or set it wrong.

For example, we don't have CONFFILES set in the shadow recipe. As a result,
/etc/login.defs from the shadow package is not treated as a config file.
Another example is the base-files recipe. We set the CONFFILES variable, but
it's not a complete list. Basically, all files under /etc should be treated as
config files for this recipe.

Such mistakes are not easy to find, because when we add or upgrade a recipe,
we usually only test whether it functions well, we don't take into consideration
the on-target upgrade process.

So we need to improve the situation here.

This patchset consists of two patches. The first one is the main patch which changes
the way CONFFILES is handled in our project. The second one serves as an example how
to fix individual recipes.

As almost all files under /etc should be considerred as config files, we don't need to
modify a lot of recipes after the first patch. Take shadow as an example. We don't need
to modify that recipe after this change. The ones that need to be paid attention to are
those that set CONFFILES in their recipes. The second patch serves as an exmple how to
fix this.

//Chen Qi

The following changes since commit ba8506ee342a1a579ceb309f3f50285eaad84f25:

  local.conf.extended: Bring into sync with OE-Core (2013-12-12 23:18:39 +0000)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib ChenQi/CONFFILES
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/CONFFILES

Chen Qi (2):
  packaging: change the process of CONFFILES handling
  base-files: Fix CONFFILES

 meta/classes/insane.bbclass                       |    2 +-
 meta/classes/package.bbclass                      |   35 +++++++++++++++++++++
 meta/classes/package_deb.bbclass                  |    2 +-
 meta/classes/package_ipk.bbclass                  |    2 +-
 meta/classes/package_rpm.bbclass                  |    2 +-
 meta/conf/bitbake.conf                            |    2 ++
 meta/recipes-core/base-files/base-files_3.0.14.bb |    3 --
 7 files changed, 41 insertions(+), 7 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] packaging: change the process of CONFFILES handling

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Previously, the CONFFILES variable is a list of config files to be
packaged. However, as these files may come directly from the source
instead of our WORKDIR, it's very common mistake that we forget to set
this variable in our recipes. This will lead to unexpected behavior
when doing an on-target upgrade. For example, we modify the /etc/login.defs
locally and then do an upgrade. The file will be replaced, which is
obviously not correct.

This patch changes the handling and the semantics of CONFFILES. After this
change, the CONFFILES can take the same form as FILES. That means, we don't
have to list a bunch of files for CONFFILES. It will just be expanded
like the FILES variable.

As almost all files under /etc are basically configuration files, we
provide a default value for CONFFILES.

    CONNFFILES_DEFAULT = "${sysconfdir}"

In this way, we don't need to modify every recipe to set the CONFFILES
variable. Of course, setting CONFFILES in recipes take precedence over
the CONFFILES_DEFAULT. For example, if the recipe author decides that
package A should only treat files under ${sysconfdir}/default/ as config
files, he/she can write like this.

    CONFFILES_A = "${sysconfdir}/default"

The above situation should not be common. As according to FHS, the /etc
directory is a place for all system related configuration files.

[YOCTO #5200]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/insane.bbclass      |    2 +-
 meta/classes/package.bbclass     |   35 +++++++++++++++++++++++++++++++++++
 meta/classes/package_deb.bbclass |    2 +-
 meta/classes/package_ipk.bbclass |    2 +-
 meta/classes/package_rpm.bbclass |    2 +-
 meta/conf/bitbake.conf           |    2 ++
 6 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index a51f504..7376846 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -981,7 +981,7 @@ python () {
 
     issues = []
     if (d.getVar('PACKAGES', True) or "").split():
-        for var in 'RDEPENDS', 'RRECOMMENDS', 'RSUGGESTS', 'RCONFLICTS', 'RPROVIDES', 'RREPLACES', 'FILES', 'pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm', 'ALLOW_EMPTY':
+        for var in 'RDEPENDS', 'RRECOMMENDS', 'RSUGGESTS', 'RCONFLICTS', 'RPROVIDES', 'RREPLACES', 'FILES', 'CONFFILES', 'pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm', 'ALLOW_EMPTY':
             if d.getVar(var):
                 issues.append(var)
     for i in issues:
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 44a852f..16ba4bd 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -234,6 +234,41 @@ python () {
         d.setVar("PACKAGERDEPTASK", "")
 }
 
+# Called in package_<rpm,ipk,deb>.bbclass to get the correct list of configuration files
+def get_conffiles(pkg, d):
+    import glob
+    pkgdest = d.getVar('PKGDEST', True)
+    conf_list = []
+    root = os.path.join(pkgdest, pkg)
+    cwd = os.getcwd()
+    os.chdir(root)
+    conffiles = (d.getVar('CONFFILES_%s' % pkg, True) or d.getVar('CONFFILES_DEFAULT', True) or "").split()
+    for conffile in conffiles:
+        if os.path.isabs(conffile):
+            conffile = '.' + conffile
+        if not conffile.startswith('./'):
+            conffile = './' + conffile
+        if not os.path.islink(conffile) and os.path.isdir(conffile):
+            newconffiles = [ os.path.join(conffile, x) for x in os.listdir(conffile) ]
+            if newconffiles:
+                conffiles += newconffiles
+                continue
+        globbed = glob.glob(conffile)
+        if globbed:
+            if [ conffile ] != globbed:
+                conffile += globbed
+                continue
+        if (not os.path.islink(conffile)) and (not os.path.exists(conffile)):
+            continue
+        if conffile in conf_list:
+            continue
+        conf_list.append(conffile)
+    # Remove the leading './'
+    for i in range(0, len(conf_list)):
+        conf_list[i] = conf_list[i][1:]
+    os.chdir(cwd)
+    return conf_list
+
 def splitdebuginfo(file, debugfile, debugsrcdir, sourcefile, d):
     # Function to split a single file into two components, one is the stripped
     # target system binary, the other contains any debugging information. The
diff --git a/meta/classes/package_deb.bbclass b/meta/classes/package_deb.bbclass
index d18c250..2a983b7 100644
--- a/meta/classes/package_deb.bbclass
+++ b/meta/classes/package_deb.bbclass
@@ -389,7 +389,7 @@ python do_package_deb () {
             scriptfile.close()
             os.chmod(os.path.join(controldir, script), 0755)
 
-        conffiles_str = localdata.getVar("CONFFILES", True)
+        conffiles_str = ' '.join(get_conffiles(pkg, d))
         if conffiles_str:
             try:
                 conffiles = open(os.path.join(controldir, 'conffiles'), 'w')
diff --git a/meta/classes/package_ipk.bbclass b/meta/classes/package_ipk.bbclass
index d0db945..a8d152b 100644
--- a/meta/classes/package_ipk.bbclass
+++ b/meta/classes/package_ipk.bbclass
@@ -390,7 +390,7 @@ python do_package_ipk () {
             scriptfile.close()
             os.chmod(os.path.join(controldir, script), 0755)
 
-        conffiles_str = localdata.getVar("CONFFILES", True)
+        conffiles_str = ' '.join(get_conffiles(pkg, d))
         if conffiles_str:
             try:
                 conffiles = open(os.path.join(controldir, 'conffiles'), 'w')
diff --git a/meta/classes/package_rpm.bbclass b/meta/classes/package_rpm.bbclass
index 346b7ab..6d7886a 100644
--- a/meta/classes/package_rpm.bbclass
+++ b/meta/classes/package_rpm.bbclass
@@ -774,7 +774,7 @@ python write_specfile () {
 
         bb.data.update_data(localdata)
 
-        conffiles = (localdata.getVar('CONFFILES', True) or "").split()
+        conffiles = get_conffiles(pkg, d)
 
         splitname    = strip_multilib(pkgname, d)
 
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 386c935..a02a6fa 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -272,6 +272,8 @@ PACKAGES = "${PN}-dbg ${PN}-staticdev ${PN}-dev ${PN}-doc ${PN}-locale ${PACKAGE
 PACKAGES_DYNAMIC = "^${PN}-locale-.*"
 FILES = ""
 
+CONFFILES_DEFAULT = "${sysconfdir}"
+
 FILES_${PN} = "${bindir}/* ${sbindir}/* ${libexecdir}/* ${libdir}/lib*${SOLIBS} \
             ${sysconfdir} ${sharedstatedir} ${localstatedir} \
             ${base_bindir}/* ${base_sbindir}/* \
-- 
1.7.9.5

[PATCH 2/2] base-files: Fix CONFFILES

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

All files under ${sysconfdir} are configuration files for base-files
package. As this is the default behavior for CONFFILES handling, we
should not override it in this recipe.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-core/base-files/base-files_3.0.14.bb |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files_3.0.14.bb b/meta/recipes-core/base-files/base-files_3.0.14.bb
index be3921a..a978da0 100644
--- a/meta/recipes-core/base-files/base-files_3.0.14.bb
+++ b/meta/recipes-core/base-files/base-files_3.0.14.bb
@@ -142,6 +142,3 @@ FILES_${PN} = "/"
 FILES_${PN}-doc = "${docdir} ${datadir}/common-licenses"
 
 PACKAGE_ARCH = "${MACHINE_ARCH}"
-
-CONFFILES_${PN} = "${sysconfdir}/fstab ${@['', '${sysconfdir}/hostname'][(d.getVar('hostname', True) != '')]} ${sysconfdir}/shells"
-
-- 
1.7.9.5

Re: [PATCH 0/2] Change the way of handling CONFFILES

[Paul Eggleton]

Hi Qi,

On Friday 13 December 2013 11:09:01 Qi.Chen@windriver.com wrote:
> It's a very common situation in OE/Yocto that the recipe authors/maintainers
> either forget to set the CONFFILES variable or set it wrong.
> 
> For example, we don't have CONFFILES set in the shadow recipe. As a result,
> /etc/login.defs from the shadow package is not treated as a config file.
> Another example is the base-files recipe. We set the CONFFILES variable, but
> it's not a complete list. Basically, all files under /etc should be treated
> as config files for this recipe.
> 
> Such mistakes are not easy to find, because when we add or upgrade a recipe,
> we usually only test whether it functions well, we don't take into
> consideration the on-target upgrade process.
> 
> So we need to improve the situation here.
> 
> This patchset consists of two patches. The first one is the main patch which
> changes the way CONFFILES is handled in our project. The second one serves
> as an example how to fix individual recipes.
> 
> As almost all files under /etc should be considerred as config files, we
> don't need to modify a lot of recipes after the first patch. Take shadow as
> an example. We don't need to modify that recipe after this change. The ones
> that need to be paid attention to are those that set CONFFILES in their
> recipes. The second patch serves as an exmple how to fix this.

This definitely sounds like a good idea, but do we need to give special 
consideration to /etc/init.d/ since files under there aren't really 
configuration files?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 0/2] Change the way of handling CONFFILES

[Enrico Scholz]

<Qi.Chen-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org> writes:

> As almost all files under /etc should be considerred as config files,

A lot of projects (especially from the freedesktop edge) are (ab)using
/etc to store non configuration files. E.g. all the dbus confguration or
gconf schemes are not intended to be modified by users.



Enrico

Re: [PATCH 0/2] Change the way of handling CONFFILES

[ChenQi]

On 12/14/2013 12:51 AM, Paul Eggleton wrote:
> Hi Qi,
>
> On Friday 13 December 2013 11:09:01 Qi.Chen@windriver.com wrote:
>> It's a very common situation in OE/Yocto that the recipe authors/maintainers
>> either forget to set the CONFFILES variable or set it wrong.
>>
>> For example, we don't have CONFFILES set in the shadow recipe. As a result,
>> /etc/login.defs from the shadow package is not treated as a config file.
>> Another example is the base-files recipe. We set the CONFFILES variable, but
>> it's not a complete list. Basically, all files under /etc should be treated
>> as config files for this recipe.
>>
>> Such mistakes are not easy to find, because when we add or upgrade a recipe,
>> we usually only test whether it functions well, we don't take into
>> consideration the on-target upgrade process.
>>
>> So we need to improve the situation here.
>>
>> This patchset consists of two patches. The first one is the main patch which
>> changes the way CONFFILES is handled in our project. The second one serves
>> as an example how to fix individual recipes.
>>
>> As almost all files under /etc should be considerred as config files, we
>> don't need to modify a lot of recipes after the first patch. Take shadow as
>> an example. We don't need to modify that recipe after this change. The ones
>> that need to be paid attention to are those that set CONFFILES in their
>> recipes. The second patch serves as an exmple how to fix this.
> This definitely sounds like a good idea, but do we need to give special
> consideration to /etc/init.d/ since files under there aren't really
> configuration files?
>
> Cheers,
> Paul
>

I thought about this issue. I then referenced ubuntu to see how it 
treated files under /etc/init.d/.
On ubuntu, files under /etc/init.d/ are also treated as configuration 
files. I think the rational behind
this decision might be that if the user modifies some init script, he 
must have modified it for some
reason which can not be silently ignored.

That's why I didn't deal with /etc/init.d/ specially.

Best Regards,
Chen Qi

Re: [PATCH 0/2] Change the way of handling CONFFILES

[ChenQi]

On 12/14/2013 01:18 AM, Enrico Scholz wrote:
> <Qi.Chen-CWA4WttNNZF54TAoqtyWWQ@public.gmane.org> writes:
>
>> As almost all files under /etc should be considerred as config files,
> A lot of projects (especially from the freedesktop edge) are (ab)using
> /etc to store non configuration files. E.g. all the dbus confguration or
> gconf schemes are not intended to be modified by users.
>

Thanks for your information.

To be frank, I'm not familiar with dbus.
But I think in such situations where some files under /etc are not 
configuration files, we can override the CONFFILES variable in those 
related recipes. And this work could be well handled by someone who's 
familiar with those recipes.

Note that this patch will not affect recipes that have already set the 
CONFFILES variable
Take dbus as an example. In dbus.inc, we have:
CONFFILES_${PN} = "${sysconfdir}/dbus-1/system.conf 
${sysconfdir}/dbus-1/session.conf"

So the CONFFILES for dbus are not all files under /etc/, but only the 
two files listed above.

Best Regards,
Chen Qi

>
> Enrico
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
>

Re: [PATCH 0/2] Change the way of handling CONFFILES

[Burton, Ross]

On 13 December 2013 17:18, Enrico Scholz
<enrico.scholz@sigma-chemnitz.de> wrote:
> A lot of projects (especially from the freedesktop edge) are (ab)using
> /etc to store non configuration files. E.g. all the dbus confguration or
> gconf schemes are not intended to be modified by users.

DBus configuration is available to be modified by admins, which is why
it's in /etc.

Ross