ref: fix remote origin management (#93)

* ref: fix remote origin management

When a flatpakref file does not provide a SuggestRemoteName key,
create an origin from the Name field.

This change implements the algorithm found in flatpak's
create_origin_remote_config() function.

Remove the option of setting an origin label in packageOption.
This option was poorly documented, and its default behavior
- labeling as "flathub" if no origin was set -
would break system activation.

Test if the remote exists before deleting it. This guards against two potential issues:
1. A Flatpakref might install non-enumerable remotes that are automatically deleted
   when the application is uninstalled, so attempting to delete them without checking
   could cause errors.
2. Users might manually delete apps/remotes, which could impact nix-flatpak state;
   checking prevents errors if the remote has already been removed.

README.md

@@ -119,9 +119,8 @@ A `sha256` hash is required for  the flatpakref file. This can be generated with
 Omitting the `sha256` attribute will require an `impure` evaluation of the flake.
 
 When installing an application from a `flatpakref`, the application remote will be determined as follows:
-1. If the packageOptions contains an origin, use that as the label for the remote URL.
-2. If the package does not specify an origin, use the remote name suggested by the flatpakref (SuggestRemoteName).
-3. If neither the package sets an origin nor the flatpakref suggests a remote name, sanitize the application Name.
+1. If the package does not specify an origin, use the remote name suggested by the flatpakref in `SuggestRemoteName`.
+2. If the flatpakref does not suggest a remote name, sanitize the flatpakref `Name` key with the same algo flatpak implements in [create_origin_remote_config()](https://github.com/flatpak/flatpak/blob/b730771bd793b34fb63fcbf292beed35476e5b92/common/flatpak-dir.c#L14423).
 
 ##### Unmanaged packages and remotes
 

modules/installer.nix

@@ -61,12 +61,15 @@ let
     # Iterate over remotes and handle remotes installed from flatpakref URLs
     remotes =
       # Existing remotes (not from flatpakref)
-      (map (builtins.getAttr "name") cfg.remotes) ++
-      # Add remotes extracted from flatpakref URLs in packages
-      map
-        (package:
-          utils.getRemoteNameFromFlatpakref package.origin flatpakrefCache.${(utils.sanitizeUrl package.flatpakref)})
-        (builtins.filter (package: utils.isFlatpakref package) cfg.packages);
+      (map (builtins.getAttr "name") cfg.remotes)
+      ++
+      # Add remotes extracted from flatpakref URLs in packages.
+      # flatpakref remote names will override any origin set in the package.
+      (builtins.filter (remote: !builtins.isNull remote)
+        (map
+          (package:
+            utils.getRemoteNameFromFlatpakref null flatpakrefCache.${(utils.sanitizeUrl package.flatpakref)})
+          (builtins.filter (package: utils.isFlatpakref package) cfg.packages)));
   });
 
   statePath = "${gcroots}/${stateFile.name}";

modules/ref.nix

@@ -11,10 +11,27 @@ let
   # sanitize a URL to be used as a key in an attrset.
   sanitizeUrl = url: builtins.replaceStrings [ "https://" "/" "." ":" ] [ "https_" "_" "_" "_" ] url;
 
+  # Create an origin name using the same algorithm as create_origin_remote_config()
+  # from commons/flatpak-dir.c:
+  #    - Extract the Name key from the ref file.
+  #    - Split on '.' and get the last occurrence (prefix).
+  #    - Lowercase the prefix.
+  #    - Append the "-origin" suffix to the prefix.
+  # If we fail to parse Name, then the key is invalid, and we fail hard.
+  # See https://github.com/flatpak/flatpak/blob/b730771bd793b34fb63fcbf292beed35476e5b92/common/flatpak-dir.c#L14423
+  createOriginRemoteConfig = name:
+    let
+      parts = builtins.filter (part: part != "") (lib.strings.splitString "." name);
+      prefix = lib.strings.toLower (builtins.elemAt parts (builtins.length parts - 1));
+      origin = ''${prefix}-origin'';
+    in
+    origin;
+
   # Extract the remote name from a package that declares a flatpakref:
   # 1. if the package sets an origin, use that as label for the remote url.
   # 2. if the package does not set an origin, use the remote name suggested by the flatpakref.
-  # 3. if the package does not set an origin and the flatpakref does not suggest a remote name, sanitize application Name.
+  # 3. if the package does not set an origin and the flatpakref does not suggest a remote name,
+  #    sanitize Name.
   getRemoteNameFromFlatpakref = origin: cache:
     let
       remoteName = origin;
@@ -25,7 +42,7 @@ let
         flatpakrefdName =
           if builtins.hasAttr "SuggestRemoteName" cache
           then cache.SuggestRemoteName
-          else "${lib.toLower cache.Name}-origin";
+          else createOriginRemoteConfig cache.Name; # Name is a mandatory field. If missing, fail hard.
       in
       flatpakrefdName
     else

modules/remotes.nix

@@ -16,13 +16,23 @@ let
     # Delete all remotes that are present in the old state but not the new one
     # $OLD_STATE and $NEW_STATE are globals, declared in the output of pkgs.writeShellScript.
     # If uninstallUnmanagedState is true, then the remotes will be deleted forcefully.
+    #
+    # Test if the remote exists before deleting it. This guards against two potential issues:
+    # 1. A Flatpakref might install non-enumerable remotes that are automatically deleted
+    #    when the application is uninstalled, so attempting to delete them without checking
+    #    could cause errors.
+    # 2. Users might manually delete apps/remotes, which could impact nix-flatpak state;
+    #    checking prevents errors if the remote has already been removed.
     ${pkgs.jq}/bin/jq -r -n \
       --argjson old "$OLD_STATE" \
       --argjson new "$NEW_STATE" \
        '(($old.remotes // []) - ($new.remotes // []))[]' \
       | while read -r REMOTE_NAME; do
-          ${pkgs.flatpak}/bin/flatpak remote-delete ${if uninstallUnmanaged then " --force " else " " } --${installation} $REMOTE_NAME
-
+          if ${pkgs.flatpak}/bin/flatpak --${installation} remotes --columns=name | grep -q "^$REMOTE_NAME$"; then
+            ${pkgs.flatpak}/bin/flatpak remote-delete ${if uninstallUnmanaged then " --force " else " " } --${installation} $REMOTE_NAME
+          else
+            echo "Remote '$REMOTE_NAME' not found in flatpak remotes"
+          fi
       done
   '';
 

tests/ref-test.nix

@@ -46,16 +46,26 @@ runTests {
     expected = "example";
   };
 
-  testGetRemoteNameWithSuggestedName = {
+  testGetRemoteNameWithSuggestRemoteName = {
     expr = ref.getRemoteNameFromFlatpakref null { SuggestRemoteName = "local"; };
     expected = "local";
   };
 
-  testGetRemoteNameWithPackageName = {
+  testGetRemoteNameWithoutSuggestRemoteName = {
     expr = ref.getRemoteNameFromFlatpakref null { Name = "Example"; };
     expected = "example-origin";
   };
 
+  testGetRemoteNameWithoutSuggestRemoteNameAndNameEndingWithDot = {
+    expr = ref.getRemoteNameFromFlatpakref null { Name = "Example."; };
+    expected = "example-origin";
+  };
+
+  testGetRemoteNameWithoutSuggestRemoteNameAndNameStartingWithDot = {
+    expr = ref.getRemoteNameFromFlatpakref null { Name = ".Example"; };
+    expected = "example-origin";
+  };
+
   testFlatpakrefToAttrSet = {
     expr = ref.flatpakrefToAttrSet { flatpakref = fixturePath; sha256 = null; } { };
     expected = expectedFixtureAttrSet;