Repository: xonsh/xonsh

Issue:
Infinite alias call
I'm trying to override 'ls' command to display dotfiles in my $DOTFILES directory.
This code goes into an endless loop because _ls function calls ls command and it calls _ls function.

```python
def _ls():
    if $(pwd).rstrip(os.linesep) == $DOTFILES:
        ls -Ga
    else:
        ls -G
aliases['ls'] = _ls
```

## xonfig

<details>

```
+------------------+---------------------+
| xonsh            | 0.8.10              |
| Git SHA          | 2cb42bdb            |
| Commit Date      | Feb 6 16:49:16 2019 |
| Python           | 3.6.4               |
| PLY              | 3.11                |
| have readline    | True                |
| prompt toolkit   | 2.0.7               |
| shell type       | prompt_toolkit2     |
| pygments         | 2.3.1               |
| on posix         | True                |
| on linux         | False               |
| on darwin        | True                |
| on windows       | False               |
| on cygwin        | False               |
| on msys2         | False               |
| is superuser     | False               |
| default encoding | utf-8               |
| xonsh encoding   | utf-8               |
| encoding errors  | surrogateescape     |
+------------------+---------------------+
```

</details>

## Current Behavior
<!--- Tell us what happens instead of the expected behavior -->
<!--- If part of your bug report is a traceback, please first enter debug mode before triggering the error
To enter debug mode, set the environment variableNSH_DEBUG=1` _before_ starting `xonsh`.  `XO
On Linux and OSX, an easy way to to do this is to run `env XONSH_DEBUG=1 xonsh` -->

When I input "ls" command, the terminal freezes.


Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=h1) Report
> Merging [#4218](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=desc) (6e5af2e) into [main](https://codecov.io/gh/xonsh/xonsh/commit/5a792a6eec4889cbbc50619da6b26f7471d7ce5d?el=desc) (5a792a6) will **increase** coverage by `0.00%`.
> The diff coverage is `60.00%`.

[![Impacted file tree graph](https://codecov.io/gh/xonsh/xonsh/pull/4218/graphs/tree.svg?width=650&height=150&src=pr&token=GXLt8EOk8Z)](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##             main    #4218   +/-   ##
=======================================
  Coverage   52.88%   52.89%           
=======================================
  Files         129      129           
  Lines       20006    20020   +14     
  Branches     3604     3608    +4     
=======================================
+ Hits        10581    10589    +8     
- Misses       8548     8551    +3     
- Partials      877      880    +3     
```


| [Impacted Files](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [xonsh/procs/specs.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree#diff-eG9uc2gvcHJvY3Mvc3BlY3MucHk=) | `58.48% <58.33%> (-0.08%)` | :arrow_down: |
| [xonsh/procs/proxies.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree#diff-eG9uc2gvcHJvY3MvcHJveGllcy5weQ==) | `45.31% <66.66%> (+0.14%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=footer). Last update [5a792a6...6e5af2e](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

gforsyth: Thanks for tackling this, @anki-code !  

I think the general solution you have here is correct.  The usage of a "private" environment variable to handle it seems a little messy to me, though.  Do you think you can implement this with an attribute in the `spec`?
anki-code: Hi @gforsyth ! Thank you for the review!

I'm using environment variable because we have different types of aliases i.e. `ExecAlias` that can execute subcommands in the distinct process that has less control from parent.
gforsyth: Hey @anki-code -- sorry for the delay.  Anytime we mess with the proxies I need to take a closer look and I'm a bit slammed for time.  I'm going to try to get to this later this week
anki-code: Hi @gforsyth! It's kind reminder :)
gforsyth: Hey @scopatz -- dunno if you're around but if you can take a quick peek at this, I'd appreciate it.  
anki-code: I've added master and broken tests looks unrelated to this issue. @jnoortheen it's new tests from 4106 please take look.
codecov-commenter: # [Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh) Report
> Merging [#4218](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh) (545e233) into [main](https://codecov.io/gh/xonsh/xonsh/commit/9d0f371e3df63a672085d416edd92f03d7d8bcc5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh) (9d0f371) will **decrease** coverage by `0.06%`.
> The diff coverage is `60.00%`.

> :exclamation: Current head 545e233 differs from pull request most recent head 41ac416. Consider uploading reports for the commit 41ac416 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/xonsh/xonsh/pull/4218/graphs/tree.svg?width=650&height=150&src=pr&token=GXLt8EOk8Z&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh)](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh)

```diff
@@            Coverage Diff             @@
##             main    #4218      +/-   ##
==========================================
- Coverage   53.03%   52.96%   -0.07%     
==========================================
  Files         129      129              
  Lines       20056    20070      +14     
  Branches     3615     3619       +4     
==========================================
- Hits        10636    10631       -5     
- Misses       8543     8561      +18     
- Partials      877      878       +1     
```


| [Impacted Files](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh) | Coverage Δ | |
|---|---|---|
| [xonsh/procs/specs.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh#diff-eG9uc2gvcHJvY3Mvc3BlY3MucHk=) | `58.48% <58.33%> (-0.08%)` | :arrow_down: |
| [xonsh/procs/proxies.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh#diff-eG9uc2gvcHJvY3MvcHJveGllcy5weQ==) | `45.31% <66.66%> (+0.14%)` | :arrow_up: |
| [xonsh/procs/posix.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh#diff-eG9uc2gvcHJvY3MvcG9zaXgucHk=) | `60.37% <0.00%> (-2.84%)` | :arrow_down: |
| [xonsh/procs/pipelines.py](https://codecov.io/gh/xonsh/xonsh/pull/4218/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh#diff-eG9uc2gvcHJvY3MvcGlwZWxpbmVzLnB5) | `74.46% <0.00%> (-0.78%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh). Last update [9d0f371...41ac416](https://codecov.io/gh/xonsh/xonsh/pull/4218?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xonsh).

anki-code: @gforsyth I think @scopatz is far far in a galaxy :)
We can wait but it will be good to have this nice fix in the next release.

anki-code: @gforsyth does this pr have a chance to be released? :)
anki-code: @daniel-shimon, all comments are done.
daniel-shimon: @anki-code FYI you can wrap parameterized values with `pytest.param`:

```
@pytest.mark.parametrize("case", 
    "will always run", 
    pytest.param("will run on unix", marks=skip_if_on_windows), 
) 
```
anki-code: @gforsyth we're ready to merge :)

Patch:
diff --git a/news/fix_aliases_infinite_loop.rst b/news/fix_aliases_infinite_loop.rst
new file mode 100644
index 00000000..60a5ab32
--- /dev/null
+++ b/news/fix_aliases_infinite_loop.rst
@@ -0,0 +1,23 @@
+**Added:**
+
+* Ability to call the tool by the name from callable alias with the same name without the infinite loop error.
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>
diff --git a/xonsh/procs/proxies.py b/xonsh/procs/proxies.py
index 4e9b100b..d9a5ff74 100644
--- a/xonsh/procs/proxies.py
+++ b/xonsh/procs/proxies.py
@@ -500,10 +500,16 @@ class ProcProxyThread(threading.Thread):
             sp_stderr = sys.stderr
         # run the function itself
         try:
+            alias_stack = builtins.__xonsh__.env.get("__ALIAS_STACK", "")
+            if self.env.get("__ALIAS_NAME"):
+                alias_stack += ":" + self.env["__ALIAS_NAME"]
+
             with STDOUT_DISPATCHER.register(sp_stdout), STDERR_DISPATCHER.register(
                 sp_stderr
             ), xt.redirect_stdout(STDOUT_DISPATCHER), xt.redirect_stderr(
                 STDERR_DISPATCHER
+            ), builtins.__xonsh__.env.swap(
+                __ALIAS_STACK=alias_stack
             ):
                 r = self.f(self.args, sp_stdin, sp_stdout, sp_stderr, spec, spec.stack)
         except SystemExit as e:
diff --git a/xonsh/procs/specs.py b/xonsh/procs/specs.py
index 845c4e52..384edf71 100644
--- a/xonsh/procs/specs.py
+++ b/xonsh/procs/specs.py
@@ -355,6 +355,8 @@ class SubprocSpec:
         # pure attrs
         self.args = list(cmd)
         self.alias = None
+        self.alias_name = None
+        self.alias_stack = builtins.__xonsh__.env.get("__ALIAS_STACK", "").split(":")
         self.binary_loc = None
         self.is_proxy = False
         self.background = False
@@ -442,6 +444,7 @@ class SubprocSpec:
         kwargs = {n: getattr(self, n) for n in self.kwnames}
         self.prep_env(kwargs)
         if callable(self.alias):
+            kwargs["env"]["__ALIAS_NAME"] = self.alias_name
             p = self.cls(self.alias, self.cmd, **kwargs)
         else:
             self.prep_preexec_fn(kwargs, pipeline_group=pipeline_group)
@@ -589,17 +592,29 @@ class SubprocSpec:
     def resolve_alias(self):
         """Sets alias in command, if applicable."""
         cmd0 = self.cmd[0]
+
+        if cmd0 in self.alias_stack:
+            # Disabling the alias resolving to prevent infinite loop in call stack
+            # and futher using binary_loc to resolve the alias name.
+            self.alias = None
+            return
+
         if callable(cmd0):
             alias = cmd0
         else:
             alias = builtins.aliases.get(cmd0, None)
+            if alias is not None:
+                self.alias_name = cmd0
         self.alias = alias
 
     def resolve_binary_loc(self):
         """Sets the binary location"""
         alias = self.alias
         if alias is None:
-            binary_loc = xenv.locate_binary(self.cmd[0])
+            cmd0 = self.cmd[0]
+            binary_loc = xenv.locate_binary(cmd0)
+            if binary_loc == cmd0 and cmd0 in self.alias_stack:
+                raise Exception(f'Recursive calls to "{cmd0}" alias.')
         elif callable(alias):
             binary_loc = None
         else:


Test patch:
diff --git a/tests/test_integrations.py b/tests/test_integrations.py
index 9ade1fa8..fb125d28 100644
--- a/tests/test_integrations.py
+++ b/tests/test_integrations.py
@@ -472,15 +472,57 @@ a
     ),
 ]
 
+UNIX_TESTS = [
+    # testing alias stack: lambda function
+    (
+        """
+def _echo():
+    echo hello
+
+aliases['echo'] = _echo
+echo
+""",
+        "hello\n",
+        0,
+    ),
+    # testing alias stack: ExecAlias
+    (
+        """
+aliases['echo'] = "echo @('hello')"
+echo
+""",
+        "hello\n",
+        0,
+    ),
+    # testing alias stack: callable alias (ExecAlias) + no binary location + infinite loop
+    (
+        """
+aliases['first'] = "second @(1)"
+aliases['second'] = "first @(1)"
+first
+""",
+        lambda out: 'Recursive calls to "first" alias.' in out,
+        0,
+    ),
+]
 
 @skip_if_no_xonsh
 @pytest.mark.parametrize("case", ALL_PLATFORMS)
 def test_script(case):
     script, exp_out, exp_rtn = case
     out, err, rtn = run_xonsh(script)
-    assert exp_out == out
+    if callable(exp_out):
+        assert exp_out(out)
+    else:
+        assert exp_out == out
     assert exp_rtn == rtn
 
+@skip_if_no_xonsh
+@skip_if_on_windows
+@pytest.mark.parametrize("case", UNIX_TESTS)
+def test_unix_tests(case):
+    test_script(case)
+
 
 ALL_PLATFORMS_STDERR = [
     # test redirecting a function alias