Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support old-style shell for rebar3 shell #995

Merged
merged 2 commits into from
Jan 10, 2016

Conversation

ferd
Copy link
Collaborator

@ferd ferd commented Jan 6, 2016

This is quite the hack.

This requires to detect the current shell running; if it's the new
shell, business as usual.

However, if it's the old shell, we have to find a way to take over it
and drive IO. This requires a few steps because:

  • the old shell does not let you be supervised intelligently (it uses
    supervisor_bridge, so killing the child is not a supported operation
    from the supervisor)
  • the old shell ignores all trappable exit signals except those coming
    from the Port in charge of stdio ({fd, 0, 1})
  • the old shell shuts down on all exit signals from the stdio Port
    except for badsig, and replicates the shutdown reason otherwise
  • An escript does not tolerate the user process dying (old shell) for
    any non-normal reason without also taking the whole escript down
  • Booting in an escript has an implicit 'noshell' argument interpreted
    by the old shell as a way to boot the stdio Port with only stdout
    taken care of

Because of all these points, we have to kill the old user process by
sending it a message pretending to be the Stdio port dying of reason
normal, which lets it die without triggering the ire of its
supervision tree and keeping the escript alive. This, in turn, kills the
old stdio port since its parent (user.erl) has died.

Then we have to boot our copy of user.erl (rebar_user.erl) which
conveniently ignores the possibility of running the stdio port on stdout
only -- always using stdin and stdout, giving us a bona fide old-style
shell.

A known issue introduced is that running r3:do(ct) seems to then kill
the shell, and r3:do(dialyzer) appears to have an odd failure, but
otherwise most other commands appear to work fine.

This should fix #946 and #563

This is quite the hack.

This requires to detect the current shell running; if it's the new
shell, business as usual.

However, if it's the old shell, we have to find a way to take over it
and drive IO. This requires a few steps because:

- the old shell does not let you be supervised intelligently (it uses
  supervisor_bridge, so killing the child is not a supported operation
  from the supervisor)
- the old shell ignores all trappable exit signals except those coming
  from the Port in charge of stdio ({fd, 0, 1})
- the old shell shuts down on all exit signals from the stdio Port
  except for badsig, and replicates the shutdown reason otherwise
- An escript does not tolerate the `user` process dying (old shell) for
  any non-normal reason without also taking the whole escript down
- Booting in an escript has an implicit 'noshell' argument interpreted
  by the old shell as a way to boot the stdio Port with only stdout
  taken care of

Because of all these points, we have to kill the old `user` process by
sending it a message pretending to be the Stdio port dying of reason
`normal`, which lets it die without triggering the ire of its
supervision tree and keeping the escript alive. This, in turn, kills the
old stdio port since its parent (user.erl) has died.

Then we have to boot our copy of user.erl (rebar_user.erl) which
conveniently ignores the possibility of running the stdio port on stdout
only -- always using stdin *and* stdout, giving us a bona fide old-style
shell.

A known issue introduced is that running r3:do(ct) seems to then kill
the shell, and r3:do(dialyzer) appears to have an odd failure, but
otherwise most other commands appear to work fine.
@ferd
Copy link
Collaborator Author

ferd commented Jan 6, 2016

This feels very dirty.

@StoneCypher
Copy link

(this is gruesome 😀)

@ferd
Copy link
Collaborator Author

ferd commented Jan 6, 2016

Do not merge yet. I just found out that the old style shell is always booted even in an escript, so the mechanism chosen to replace shells is wrong. Will have to fix this.

This reuses the trick used within OTP to pick within old and new shell.

The 'user' structure is the same for all cases (escript, escript + dumb
TERM, unstable install, unstable install + dumb TERM), so we take it
down first.

Then we boot the TTY driver, which fails if TERM=dumb, in which case we
boot the retro-style usr. If it worked, we shut down the driver again,
and boot a modern shell structure.

This avoids all warnings and seems to work in all cases.
@ferd
Copy link
Collaborator Author

ferd commented Jan 6, 2016

fixed.

@ferd
Copy link
Collaborator Author

ferd commented Jan 7, 2016

So things appear to work, but I still get weird failures on r3:do(ct) calls (which kill the shell).

@ferd
Copy link
Collaborator Author

ferd commented Jan 7, 2016

OK, I can confirm the problem with r3:do(ct) happens only in rebar3 and is related to reloading and purging rebar_agent during a test runs, which forces it to terminate. This happens in master anyway and isn't a regression. It happens there: https://github.com/rebar/rebar3/blob/72855b223bce28506282062670080a1919e814d6/src/rebar_agent.erl#L108

There's possible fixes there (move to a soft purge, subtract rebar_agent from the modules list, etc.), but they should go in a different PR and only if we care a lot about r3:do(ct) within the rebar3 project.

I think this is good to merge if it passes review -- I know this is one hell of a janky patch pulling big chunks of code from OTP itself, but yeaaah.

@tsloughter
Copy link
Collaborator

If it helps some users I'm fine with it.

@ferd
Copy link
Collaborator Author

ferd commented Jan 10, 2016

At the very least it brings the feature to all of windows users. Otherwise I don't know about what has a dumb term. Maybe output to files or some remote consoles?

@tsloughter
Copy link
Collaborator

Ok, cool, +1

tsloughter added a commit that referenced this pull request Jan 10, 2016
Support old-style shell for rebar3 shell
@tsloughter tsloughter merged commit 48ac697 into erlang:master Jan 10, 2016
@StoneCypher
Copy link

speaking as erlang's only windows user, i support this patch

even so, it's reaper level grim 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unknown TERM prevents shell prompt
3 participants