(Android) Play Store builds: Do not resolve symlinks when handling core paths

core_updater_list.c

@@ -273,6 +273,7 @@ bool core_updater_list_get_core(
       const char *local_core_path,
       const core_updater_list_entry_t **entry)
 {
+   bool resolve_symlinks;
    size_t num_entries;
    size_t i;
    char real_core_path[PATH_MAX_LENGTH];
@@ -289,7 +290,14 @@ bool core_updater_list_get_core(
 
    /* Resolve absolute pathname of local_core_path */
    strlcpy(real_core_path, local_core_path, sizeof(real_core_path));
-   path_resolve_realpath(real_core_path, sizeof(real_core_path), true);
+   /* Can't resolve symlinks when dealing with cores
+    * installed via play feature delivery, because the
+    * source files have non-standard file names (which
+    * will not be recognised by regular core handling
+    * routines) */
+   resolve_symlinks = (core_list->type != CORE_UPDATER_LIST_TYPE_PFD);
+   path_resolve_realpath(real_core_path, sizeof(real_core_path),
+         resolve_symlinks);
 
    if (string_is_empty(real_core_path))
       return false;
@@ -391,10 +399,16 @@ static bool core_updater_list_set_paths(
 {
    char *last_underscore                  = NULL;
    char *tmp_url                          = NULL;
+   bool is_archive                        = true;
+   /* Can't resolve symlinks when dealing with cores
+    * installed via play feature delivery, because the
+    * source files have non-standard file names (which
+    * will not be recognised by regular core handling
+    * routines) */
+   bool resolve_symlinks                  = (list_type != CORE_UPDATER_LIST_TYPE_PFD);
    char remote_core_path[PATH_MAX_LENGTH];
    char local_core_path[PATH_MAX_LENGTH];
    char local_info_path[PATH_MAX_LENGTH];
-   bool is_archive;
 
    remote_core_path[0] = '\0';
    local_core_path[0]  = '\0';
@@ -460,7 +474,8 @@ static bool core_updater_list_set_paths(
    if (is_archive)
       path_remove_extension(local_core_path);
 
-   path_resolve_realpath(local_core_path, sizeof(local_core_path), true);
+   path_resolve_realpath(local_core_path, sizeof(local_core_path),
+         resolve_symlinks);
 
    if (entry->local_core_path)
    {

menu/cbs/menu_cbs_ok.c

@@ -2168,7 +2168,7 @@ static int action_ok_playlist_entry_collection(const char *path,
    if (!string_is_empty(entry->path))
    {
       strlcpy(content_path, entry->path, sizeof(content_path));
-      playlist_resolve_path(PLAYLIST_LOAD, content_path, sizeof(content_path));
+      playlist_resolve_path(PLAYLIST_LOAD, false, content_path, sizeof(content_path));
    }
 
    /* Cache entry label */
@@ -2239,7 +2239,7 @@ static int action_ok_playlist_entry_collection(const char *path,
             /* Core path is invalid - just copy what we have
              * and hope for the best... */
             strlcpy(core_path, entry->core_path, sizeof(core_path));
-            playlist_resolve_path(PLAYLIST_LOAD, core_path, sizeof(core_path));
+            playlist_resolve_path(PLAYLIST_LOAD, true, core_path, sizeof(core_path));
          }
       }
    }
@@ -3284,7 +3284,7 @@ static int action_ok_core_deferred_set(const char *new_core_path,
          true);
 
    strlcpy(resolved_core_path, new_core_path, sizeof(resolved_core_path));
-   playlist_resolve_path(PLAYLIST_SAVE, resolved_core_path, sizeof(resolved_core_path));
+   playlist_resolve_path(PLAYLIST_SAVE, true, resolved_core_path, sizeof(resolved_core_path));
 
    /* the update function reads our entry
     * as const, so these casts are safe */

play_feature_delivery/play_feature_delivery.c

@@ -44,22 +44,28 @@
 typedef struct
 {
 #ifdef HAVE_THREADS
-   slock_t *lock;
+   slock_t *enabled_lock;
+   slock_t *status_lock;
 #endif
    unsigned download_progress;
    enum play_feature_delivery_install_status last_status;
    char last_core_name[256];
+   bool enabled;
+   bool enabled_set;
    bool active;
 } play_feature_delivery_state_t;
 
 static play_feature_delivery_state_t play_feature_delivery_state = {
 
 #ifdef HAVE_THREADS
-   NULL,                       /* lock */
+   NULL,                       /* enabled_lock */
+   NULL,                       /* status_lock */
 #endif
    0,                          /* download_progress */
    PLAY_FEATURE_DELIVERY_IDLE, /* last_status */
    {'\0'},                     /* last_core_name */
+   false,                      /* enabled */
+   false,                      /* enabled_set */
    false,                      /* active */
 };
 
@@ -85,7 +91,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom
 
    /* Lock mutex */
 #ifdef HAVE_THREADS
-   slock_lock(state->lock);
+   slock_lock(state->status_lock);
 #endif
 
    /* Only update status if an install is active */
@@ -114,7 +120,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom
 
    /* Unlock mutex */
 #ifdef HAVE_THREADS
-   slock_unlock(state->lock);
+   slock_unlock(state->status_lock);
 #endif
 }
 
@@ -130,7 +136,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom
 
    /* Lock mutex */
 #ifdef HAVE_THREADS
-   slock_lock(state->lock);
+   slock_lock(state->status_lock);
 #endif
 
    /* Only update status if an install is active */
@@ -184,7 +190,7 @@ JNIEXPORT void JNICALL Java_com_retroarch_browser_retroactivity_RetroActivityCom
 
    /* Unlock mutex */
 #ifdef HAVE_THREADS
-   slock_unlock(state->lock);
+   slock_unlock(state->status_lock);
 #endif
 }
 
@@ -198,12 +204,23 @@ void play_feature_delivery_init(void)
    play_feature_delivery_state_t* state = play_feature_delivery_get_state();
 
    play_feature_delivery_deinit();
-#ifdef HAVE_THREADS
-   if (!state->lock)
-      state->lock = slock_new();
 
-   retro_assert(state->lock);
+#ifdef HAVE_THREADS
+   if (!state->enabled_lock)
+      state->enabled_lock = slock_new();
+
+   retro_assert(state->enabled_lock);
+
+   if (!state->status_lock)
+      state->status_lock = slock_new();
+
+   retro_assert(state->status_lock);
 #endif
+
+   /* Note: Would like to cache whether this
+    * is a Play Store build here, but
+    * play_feature_delivery_init() is called
+    * too early in the startup sequence... */
 }
 
 /* Must be called upon program termination */
@@ -212,10 +229,16 @@ void play_feature_delivery_deinit(void)
    play_feature_delivery_state_t* state = play_feature_delivery_get_state();
 
 #ifdef HAVE_THREADS
-   if (state->lock)
+   if (state->enabled_lock)
    {
-      slock_free(state->lock);
+      slock_free(state->enabled_lock);
-      state->lock = NULL;
+      state->enabled_lock = NULL;
+   }
+
+   if (state->status_lock)
+   {
+      slock_free(state->status_lock);
+      state->status_lock = NULL;
    }
 #endif
 }
@@ -251,8 +274,10 @@ static bool play_feature_delivery_get_core_name(
 }
 
 /* Returns true if current build utilises
- * play feature delivery */
+ * play feature delivery
-bool play_feature_delivery_enabled(void)
+ * > Relies on a Java function call, and is
+ *   therefore slow */
+static bool play_feature_delivery_enabled_internal(void)
 {
    JNIEnv *env             = jni_thread_getenv();
    struct android_app *app = (struct android_app*)g_android;
@@ -269,6 +294,39 @@ bool play_feature_delivery_enabled(void)
    return enabled;
 }
 
+/* Returns true if current build utilises
+ * play feature delivery */
+bool play_feature_delivery_enabled(void)
+{
+   play_feature_delivery_state_t* state = play_feature_delivery_get_state();
+   bool enabled;
+
+   /* Lock mutex */
+#ifdef HAVE_THREADS
+   slock_lock(state->enabled_lock);
+#endif
+
+   /* Calling Java functions is slow. We need to
+    * check Play Store build status frequently,
+    * often in loops, so rely on a cached global
+    * status flag instead dealing with Java
+    * interfaces */
+   if (!state->enabled_set)
+   {
+      state->enabled     = play_feature_delivery_enabled_internal();
+      state->enabled_set = true;
+   }
+
+   enabled = state->enabled;
+
+   /* Unlock mutex */
+#ifdef HAVE_THREADS
+   slock_unlock(state->enabled_lock);
+#endif
+
+   return enabled;
+}
+
 /* Returns a list of cores currently available
  * via play feature delivery.
  * Returns a new string_list on success, or
@@ -408,7 +466,7 @@ bool play_feature_delivery_download_status(
 
    /* Lock mutex */
 #ifdef HAVE_THREADS
-   slock_lock(state->lock);
+   slock_lock(state->status_lock);
 #endif
 
    /* Copy status parameters */
@@ -422,7 +480,7 @@ bool play_feature_delivery_download_status(
 
    /* Unlock mutex */
 #ifdef HAVE_THREADS
-   slock_unlock(state->lock);
+   slock_unlock(state->status_lock);
 #endif
 
    return active;
@@ -459,7 +517,7 @@ bool play_feature_delivery_download(const char *core_file)
 
    /* Lock mutex */
 #ifdef HAVE_THREADS
-   slock_lock(state->lock);
+   slock_lock(state->status_lock);
 #endif
 
    /* We only support one download at a time */
@@ -487,7 +545,7 @@ bool play_feature_delivery_download(const char *core_file)
 
    /* Unlock mutex */
 #ifdef HAVE_THREADS
-   slock_unlock(state->lock);
+   slock_unlock(state->status_lock);
 #endif
 
    return success;

playlist.c

@@ -334,7 +334,8 @@ static bool playlist_core_path_equal(const char *real_core_path, const char *ent
    strlcpy(entry_real_core_path, entry_core_path, sizeof(entry_real_core_path));
    if (!string_is_equal(entry_real_core_path, FILE_PATH_DETECT) &&
        !string_is_equal(entry_real_core_path, FILE_PATH_BUILTIN))
-      path_resolve_realpath(entry_real_core_path, sizeof(entry_real_core_path), true);
+      playlist_resolve_path(PLAYLIST_SAVE, true, entry_real_core_path,
+            sizeof(entry_real_core_path));
 
    if (string_is_empty(entry_real_core_path))
       return false;
@@ -764,14 +765,15 @@ bool playlist_push_runtime(playlist_t *playlist,
    if (!string_is_empty(entry->path))
    {
       strlcpy(real_path, entry->path, sizeof(real_path));
-      path_resolve_realpath(real_path, sizeof(real_path), true);
+      playlist_resolve_path(PLAYLIST_SAVE, false, real_path, sizeof(real_path));
    }
 
    /* Get 'real' core path */
    strlcpy(real_core_path, entry->core_path, sizeof(real_core_path));
    if (!string_is_equal(real_core_path, FILE_PATH_DETECT) &&
        !string_is_equal(real_core_path, FILE_PATH_BUILTIN))
-      path_resolve_realpath(real_core_path, sizeof(real_core_path), true);
+      playlist_resolve_path(PLAYLIST_SAVE, true, real_core_path,
+             sizeof(real_core_path));
 
    if (string_is_empty(real_core_path))
    {
@@ -869,6 +871,7 @@ success:
 /**
  * playlist_resolve_path:
  * @mode      : PLAYLIST_LOAD or PLAYLIST_SAVE
+ * @is_core   : Set true if path to be resolved is a core file
  * @path      : The path to be modified
  *
  * Resolves the path of an item, such as the content path or path to the core, to a format
@@ -879,7 +882,7 @@ success:
  * install (iOS)
 **/
 void playlist_resolve_path(enum playlist_file_mode mode,
-      char *path, size_t len)
+      bool is_core, char *path, size_t len)
 {
 #ifdef HAVE_COCOATOUCH
    char tmp[PATH_MAX_LENGTH];
@@ -903,10 +906,22 @@ void playlist_resolve_path(enum playlist_file_mode mode,
       fill_pathname_abbreviate_special(path, tmp2, len);
    }
 #else
+   bool resolve_symlinks = true;
+
    if (mode == PLAYLIST_LOAD)
       return;
 
-   path_resolve_realpath(path, len, true);
+#if defined(ANDROID)
+   /* Can't resolve symlinks when dealing with cores
+    * installed via play feature delivery, because the
+    * source files have non-standard file names (which
+    * will not be recognised by regular core handling
+    * routines) */
+   if (is_core)
+      resolve_symlinks = !play_feature_delivery_enabled();
+#endif
+
+   path_resolve_realpath(path, len, resolve_symlinks);
 #endif
 }
 
@@ -941,14 +956,14 @@ bool playlist_push(playlist_t *playlist,
    if (!string_is_empty(entry->path))
    {
       strlcpy(real_path, entry->path, sizeof(real_path));
-      playlist_resolve_path(PLAYLIST_SAVE, real_path, sizeof(real_path));
+      playlist_resolve_path(PLAYLIST_SAVE, false, real_path, sizeof(real_path));
    }
 
    /* Get 'real' core path */
    strlcpy(real_core_path, entry->core_path, sizeof(real_core_path));
    if (!string_is_equal(real_core_path, FILE_PATH_DETECT) &&
        !string_is_equal(real_core_path, FILE_PATH_BUILTIN))
-       playlist_resolve_path(PLAYLIST_SAVE, real_core_path,
+      playlist_resolve_path(PLAYLIST_SAVE, true, real_core_path,
              sizeof(real_core_path));
 
    if (string_is_empty(real_core_path))
@@ -2976,7 +2991,8 @@ bool playlist_entries_are_equal(
       strlcpy(real_core_path_a, entry_a->core_path, sizeof(real_core_path_a));
       if (!string_is_equal(real_core_path_a, FILE_PATH_DETECT) &&
           !string_is_equal(real_core_path_a, FILE_PATH_BUILTIN))
-         path_resolve_realpath(real_core_path_a, sizeof(real_core_path_a), true);
+         playlist_resolve_path(PLAYLIST_SAVE, true,
+               real_core_path_a, sizeof(real_core_path_a));
    }
 
    return playlist_core_path_equal(real_core_path_a, entry_b->core_path, config);
@@ -3076,7 +3092,7 @@ void playlist_set_default_core_path(playlist_t *playlist, const char *core_path)
    strlcpy(real_core_path, core_path, sizeof(real_core_path));
    if (!string_is_equal(real_core_path, FILE_PATH_DETECT) &&
        !string_is_equal(real_core_path, FILE_PATH_BUILTIN))
-       playlist_resolve_path(PLAYLIST_SAVE,
+       playlist_resolve_path(PLAYLIST_SAVE, true,
              real_core_path, sizeof(real_core_path));
 
    if (string_is_empty(real_core_path))

playlist.h

@@ -223,7 +223,8 @@ void playlist_delete_by_path(playlist_t *playlist,
 /**
  * playlist_resolve_path:
  * @mode      : PLAYLIST_LOAD or PLAYLIST_SAVE
- * @path        : The path to be modified
+ * @is_core   : Set true if path to be resolved is a core file
+ * @path      : The path to be modified
  *
  * Resolves the path of an item, such as the content path or path to the core, to a format
  * appropriate for saving or loading depending on the @mode parameter
@@ -233,7 +234,7 @@ void playlist_delete_by_path(playlist_t *playlist,
  * install (iOS)
  **/
 void playlist_resolve_path(enum playlist_file_mode mode,
-      char *path, size_t len);
+      bool is_core, char *path, size_t len);
 
 /**
  * playlist_push:

retroarch.c

@@ -20305,6 +20305,17 @@ static bool load_dynamic_core(
       struct rarch_state *p_rarch,
       const char *path, char *buf, size_t size)
 {
+#if defined(ANDROID)
+   /* Can't resolve symlinks when dealing with cores
+    * installed via play feature delivery, because the
+    * source files have non-standard file names (which
+    * will not be recognised by regular core handling
+    * routines) */
+   bool resolve_symlinks = !play_feature_delivery_enabled();
+#else
+   bool resolve_symlinks = true;
+#endif
+
    /* Can't lookup symbols in itself on UWP */
 #if !(defined(__WINRT__) || defined(WINAPI_FAMILY) && WINAPI_FAMILY == WINAPI_FAMILY_PHONE_APP)
    if (dylib_proc(NULL, "retro_init"))
@@ -20323,7 +20334,7 @@ static bool load_dynamic_core(
    /* Need to use absolute path for this setting. It can be
     * saved to content history, and a relative path would
     * break in that scenario. */
-   path_resolve_realpath(buf, size, true);
+   path_resolve_realpath(buf, size, resolve_symlinks);
    if ((p_rarch->lib_handle = dylib_load(path)))
       return true;
    return false;