avoid x2 setting of set_fact when 'cacheable' (#50564)

* avoid x2 setting of set_fact when 'cacheable'

  fixes #50556

* ammend docs to new behaviour

* added period

changelogs/fragments/fix_set_fact_cacheable.yml

@@ -0,0 +1,2 @@
+bugfixes:
+    - fix issue in which 'cacheable' was not being properly applied and the fact was x2 defined

docs/docsite/rst/user_guide/playbooks_variables.rst

@@ -1036,8 +1036,7 @@ Basically, anything that goes into "role defaults" (the defaults folder inside t
 .. [1] Tasks in each role will see their own role's defaults. Tasks defined outside of a role will see the last role's defaults.
 .. [2] Variables defined in inventory file or provided by dynamic inventory.
 .. [3] Includes vars added by 'vars plugins' as well as host_vars and group_vars which are added by the default vars plugin shipped with Ansible.
-.. [4] When created with set_facts's cacheable option, variables will have the high precedence in the play,
+.. [4] When created with set_facts's cacheable option.
-       but will be the same as a host facts precedence when they come from the cache.
 
 .. note:: Within any section, redefining a var will overwrite the previous instance.
           If multiple groups have the same variable, the last one loaded wins.

lib/ansible/executor/task_executor.py

@@ -684,7 +684,7 @@ class TaskExecutor:
                 return failed_when_result
 
             if 'ansible_facts' in result:
-                if self._task.action in ('set_fact', 'include_vars'):
+                if self._task.action in ('include_vars', 'set_fact'):
                     vars_copy.update(result['ansible_facts'])
                 else:
                     # TODO: cleaning of facts should eventually become part of taskresults instead of vars

lib/ansible/plugins/strategy/__init__.py

@@ -570,10 +570,10 @@ class StrategyBase:
                         else:
                             cacheable = result_item.pop('_ansible_facts_cacheable', False)
                             for target_host in host_list:
-                                if not original_task.action == 'set_fact' or cacheable:
+                                if original_task.action == 'set_fact' and not cacheable:
-                                    self._variable_manager.set_host_facts(target_host, result_item['ansible_facts'].copy())
-                                if original_task.action == 'set_fact':
                                     self._variable_manager.set_nonpersistent_facts(target_host, result_item['ansible_facts'].copy())
+                                else:
+                                    self._variable_manager.set_host_facts(target_host, result_item['ansible_facts'].copy())
 
                     if 'ansible_stats' in result_item and 'data' in result_item['ansible_stats'] and result_item['ansible_stats']['data']: