repo: Fix non-system remotes-config-dir usage

Before commit e0346c1, a non-system repo could specify
remotes-config-dir and have remotes read from there. However, adding
remotes would only be done in the config dir for a system repo. Restore
that by respecting remotes-config-dir when no sysroot is found and
adding back the ostree_repo_is_system() check when adding remotes.

Closes: #1133

Closes: #1151
Approved by: cgwalters

Makefile-tests.am

@@ -186,6 +186,7 @@ endif
 
 js_installed_tests = \
 	tests/test-core.js \
+	tests/test-remotes-config-dir.js \
 	tests/test-sizes.js \
 	tests/test-sysroot.js \
 	$(NULL)

src/libostree/ostree-repo.c

@@ -1010,8 +1010,11 @@ impl_repo_remote_add (OstreeRepo     *self,
 
   remote = ostree_remote_new (name);
 
+  /* Only add repos in remotes.d for system repos since that was the
+   * legacy behavior and non-system repos would not expect it.
+   */
   g_autoptr(GFile) etc_ostree_remotes_d = get_remotes_d_dir (self, sysroot);
-  if (etc_ostree_remotes_d)
+  if (etc_ostree_remotes_d && ostree_repo_is_system (self))
     {
       g_autoptr(GError) local_error = NULL;
 
@@ -2040,10 +2043,6 @@ static GFile *
 get_remotes_d_dir (OstreeRepo          *self,
                    GFile               *sysroot)
 {
-  /* Support explicit override */
-  if (self->sysroot_dir != NULL && self->remotes_config_dir != NULL)
-    return g_file_resolve_relative_path (self->sysroot_dir, self->remotes_config_dir);
-
   g_autoptr(GFile) sysroot_owned = NULL;
   /* Very complicated sysroot logic; this bit breaks the otherwise mostly clean
    * layering between OstreeRepo and OstreeSysroot. First, If a sysroot was
@@ -2079,10 +2078,18 @@ get_remotes_d_dir (OstreeRepo          *self,
   if (sysroot == NULL && sysroot_ref == NULL)
     sysroot = self->sysroot_dir;
 
-  /* Did we find a sysroot? If not, NULL means use the repo config, otherwise
+  /* Was the config directory specified? If so, use that with the
-   * return the path in /etc.
+   * optional sysroot prepended. If not, return the path in /etc if the
+   * sysroot was found and NULL otherwise to use the repo config.
    */
-  if (sysroot == NULL)
+  if (self->remotes_config_dir != NULL)
+    {
+      if (sysroot == NULL)
+        return g_file_new_for_path (self->remotes_config_dir);
+      else
+        return g_file_resolve_relative_path (sysroot, self->remotes_config_dir);
+    }
+  else if (sysroot == NULL)
     return NULL;
   else
     return g_file_resolve_relative_path (sysroot, SYSCONF_REMOTES);

tests/test-remotes-config-dir.js

@@ -0,0 +1,73 @@
+#!/usr/bin/env gjs
+//
+// Copyright (C) 2013 Colin Walters <walters@verbum.org>
+// Copyright (C) 2017 Dan Nicholson <nicholson@endlessm.com>
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the
+// Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+// Boston, MA 02111-1307, USA.
+
+const GLib = imports.gi.GLib;
+const Gio = imports.gi.Gio;
+const OSTree = imports.gi.OSTree;
+
+function assertEquals(a, b) {
+    if (a != b)
+	throw new Error("assertion failed " + JSON.stringify(a) + " == " + JSON.stringify(b));
+}
+
+function assertNotEquals(a, b) {
+    if (a == b)
+	throw new Error("assertion failed " + JSON.stringify(a) + " != " + JSON.stringify(b));
+}
+
+print('1..3')
+
+let remotesDir = Gio.File.new_for_path('remotes.d');
+remotesDir.make_directory(null);
+
+let remoteConfig = GLib.KeyFile.new()
+remoteConfig.set_value('remote "foo"', 'url', 'http://foo')
+
+let remoteConfigFile = remotesDir.get_child('foo.conf')
+remoteConfig.save_to_file(remoteConfigFile.get_path())
+
+// Use the full Repo constructor to set remotes-config-dir
+let repoFile = Gio.File.new_for_path('repo');
+let repo = new OSTree.Repo({path: repoFile,
+                            remotes_config_dir: remotesDir.get_path()});
+repo.create(OSTree.RepoMode.ARCHIVE_Z2, null);
+repo.open(null);
+
+// See if the remotes.d remote exists
+let remotes = repo.remote_list()
+assertNotEquals(remotes.indexOf('foo'), -1);
+
+print("ok read-remotes-config-dir");
+
+// Adding a remote should not go in the remotes.d dir
+repo.remote_add('bar', 'http://bar', null, null);
+remotes = repo.remote_list()
+assertNotEquals(remotes.indexOf('bar'), -1);
+assertEquals(remotesDir.get_child('bar.conf').query_exists(null), false);
+
+print("ok add-not-in-remotes-config-dir");
+
+// Removing the remotes.d remote should delete the conf file
+repo.remote_delete('foo', null);
+remotes = repo.remote_list()
+assertEquals(remotes.indexOf('foo'), -1);
+assertEquals(remotesDir.get_child('foo.conf').query_exists(null), false);
+
+print("ok delete-in-remotes-config-dir");