context: vars dirs prefixing is problematic

[lucab]

(This issue originated at coreos/rpm-ostree#3241, which contains more details about the context)

rpm-ostree internally uses libdnf for handling packages and repositories details. This includes sourcing repositories and variables content from the filesystem (e.g. runtime config such as /etc/yum.repos.d and /etc/dnf/vars).

As library consumers, we are currently having some troubles because all the vars directories are internally prefixed with install_root:

libdnf/libdnf/dnf-context.cpp

Lines 3782 to 3791 in 0ee4144

	void
	dnf_context_load_vars(DnfContext * context)
	{
	auto priv = GET_PRIVATE(context);
	priv->vars->clear();
	for (auto dir = dnf_context_get_vars_dir(context); *dir; ++dir)
	ConfigMain::addVarsFromDir(*priv->vars, std::string(priv->install_root) + *dir);
	ConfigMain::addVarsFromEnv(*priv->vars);
	priv->varsCached = true;
	}

Thus all the vars lookups are happening at <install_root>/etc/dnf/vars instead of /etc/dnf/vars.

I'm not sure if this behavior was intended, but the hardcoded prefix makes it harder to do out-of-band installs.

This behavior is also puzzling because repos lookups do not go through the same install_root prefixing, even though the external API for the two entities is basically the same:

libdnf/libdnf/dnf-context.cpp

Lines 1177 to 1187 in 0ee4144

	/**
	* dnf_context_set_repos_dir:
	* @context: a #DnfContext instance.
	* @repos_dir: the NULL terminated array of paths, e.g. ["/etc/yum.repos.d", NULL]
	*
	* Sets the repositories directories.
	*
	* Since: 0.42.0
	**/
	void
	dnf_context_set_repos_dir(DnfContext *context, const gchar * const *repos_dir)

libdnf/libdnf/dnf-context.cpp

Lines 1227 to 1237 in 0ee4144

	/**
	* dnf_context_set_vars_dir:
	* @context: a #DnfContext instance.
	* @vars_dir: the vars directories, e.g. ["/etc/dnf/vars"]
	*
	* Sets the repo variables directory.
	*
	* Since: 0.28.1
	**/
	void
	dnf_context_set_vars_dir(DnfContext *context, const gchar * const *vars_dir)

For our specific usecase we would be happier without the internal prefixing, so that vars_dir behaves the same way as the existing repo_dirs logic.

If you believe that libdnf should instead keep doing its own internal prefixing, we'd need some kind of API to set the prefix back to / separately from install_root (which in our case is already set to a different path).

[lucab]

While looking into the rpm-ostree issue, we found out that PackageKit also carries some vars_dir logic which does not expect libdnf to internally do any prefixing:

  dnf_context_set_install_root (context, destdir); // <-- install_root set to destdir
[...]
  /* Add prefix to var directories */
  var_dirs = dnf_context_get_vars_dir (context);
  if (var_dirs != NULL && var_dirs[0] != NULL) {
    g_auto(GStrv) full_var_dirs = NULL;
    guint len = g_strv_length ((gchar **)var_dirs);
    full_var_dirs = g_new0 (gchar*, len + 1);
    for (guint i = 0; i < len; i++)
      full_var_dirs[i] = g_build_filename (destdir, var_dirs[i], NULL); // <- manual destdir prefixing
    dnf_context_set_vars_dir (context, (const gchar * const*)full_var_dirs);
  }

There is an ongoing thread about this at https://github.com/PackageKit/PackageKit/pull/369/files#r863937752.

[lucab]

Looking at the two cases above from distinct projects/developers, they seem to share some common expectations:

• dnf_context_set_repos_dir() and dnf_context_set_vars_dir() are mostly symmetrical APIs, so they are naturally expected to behave in the same way
• consumers expect to performed destdir prefixing (if any) on their own (e.g. PackageKit code above), not expecting the library to perform this kind of prefixing internally

Based on the those observations, I've submitted a patch at #1506 which covers all of the above by removing the internal prefixing step.

[lucab]

I think I found one more case in microdnf logic, where the code does handle repos-dir and vars-dir in the same way:

      else if (strcmp (setopt[0], "reposdir") == 0)
        {
          reposdir_used = TRUE;
          g_auto(GStrv) reposdir = g_strsplit (setopt[1], ",", -1);
          dnf_context_set_repos_dir (ctx, (const gchar * const *)reposdir);
        }
      else if (strcmp (setopt[0], "varsdir") == 0)
        {
          varsdir_used = TRUE;
          g_auto(GStrv) varsdir = g_strsplit (setopt[1], ",", -1);
          dnf_context_set_vars_dir (ctx, (const gchar * const *)varsdir);
        }

and then the CI exercises it with explicit prefixing:

  When I execute microdnf with args "install setup --setopt=varsdir={context.dnf.installroot}/tmp/vars"

[lucab]

Additionally, @jrohel noted in PackageKit/PackageKit#369 (comment):

If new empty installation root is processed there is nothing inside. So, it is about use case. Sometime we want to use configuration from the host and sometime from the destination directory. That is why libdnf does not adding prefixes. But application can do it by libdnf API.

Which I read as an hint that this libdnf behavior was maybe not really expected/designed as currently implemented.