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

Server dev #1374

Open
wants to merge 115 commits into
base: horizon
Choose a base branch
from
Open

Server dev #1374

wants to merge 115 commits into from

Conversation

jpolitz
Copy link
Member

@jpolitz jpolitz commented Jul 11, 2018

This PR implements a bunch of CLI-focused changes for the Pyret server.

This needs some changes before it can merge. For example, package.json and the README were designed to work with this as an npm package, and these changes should NOT be merged into horizon.

It's useful to get the PR up so we can see the aggregate changes to the server code, which include:

  • some better options for controlling check block running
  • allowing standalones to use installed NPM modules as well as bundled ones
  • numerous improvements to handling working directories and relative paths for the client and server

@TheOrangeLego and I have made the https://github.com/brownplt/pyret-npm repository to separate the client connection logic and npm package metadata into a separate repo. This lets us avoid having this repo (pyret-lang) be responsible for being installable as an npm package with a runnable entrypoint and so on.

The websockets npm package didn't support unix file sockets, so switching to
the ws package.

In addition, don't use http over localhost/ports, and instead make socket
files in /tmp/parley-<username>/comm.sock. This has a few nice effects:

- This will work well on multi-user systems – no fighting over ports, and the
  socket file can be chmod 700
- There's a well-defined global place for the `pyret` command to look to see if
  a server is running

Also updated some options, like deps-file, to be propagated through to the
server.
This is somewhat of an intermediate state; see below about cli-module-loader.

There are also lots of pathing issues that can come up here; a next step is to
write a simple repository that npm installs pyret with -g, and tests
compilation commands from various working directories. This will ensure that
local testing doesn't have random state floating around in my directories that
make this work when it shouldn't.

- Make much more use of the `this-pyret-dir` parameter:
  - Allow for `$PYRET` in config files to mean substitute with
    `this-pyret-dir`. This allows us to consolidate all the config.json files
    into one.
  - Remove lots of defaults from `pyret.js` in favor of calculating the path to
    the appropriate JS file for e.g. handalone, etc
  - Copy more files into the `build/` directory for the appropriate phase in
    order to centralize the location of dependencies. For example, jglr files
    and json config files are now copied into build/phaseX/js
  - Use the location of the pyret.js script to find the compiler (currently
    this looks for phaseA). Since this is the main entry-point, it has to
    somehow hard-code a default compiler to look for.

- Run the compiler in async mode, and use `schedulePause` along with SIGINT and
  a shutdown message to control the server.  The client-lib still has logic for
  killing the server via a PID file, but this logic should eventually be
  removed. The server should create and manage the socket file, and there
  should be an option like --clean that causes the server to unlink any old
  sockets it finds when it starts up. The client should just try to connect to
  the well-known socket for a few seconds rather than looking for a pid file,
  and start the server if the appropriate socket file doesn't exist.

- Send SIGINT (Ctrl-C) events from the client to the server via a special
  "stop" command, which breaks the current Pyret execution but keeps the server
  running.

- Add a new compiler option for a list of directories in which to _look_ for
  pre-built builtin modules like lists and ffi. These should have half of the
  semantics of the writable cache directory that defaults to compiled/ – the
  cli-module-loader will look there for cached versions of files, but only
  write to the specified destination. Essentially all the new logic in
  cli-module-loader is handling this new option. This has introduced a stupid
  slowdown in the cache-checking logic when the compiler is used from the
  server that I need to track down. I believe many .arr files are getting
  parsed more than they need to when their compiled versions already exist. All
  of this caching logic could use a long look anyway, so I'll be poking into
  this more.

- Change the compiler to resolve the name of the output .jarr file relative to
  the provided base-dir, rather than relative to the current working directory.

- Add various intelligent choices revolving around the current working
  directory to the options that the client sends to the server.
@jpolitz
Copy link
Member Author

jpolitz commented Jul 11, 2018

Ah, I forgot, this also includes --perilous, which optionally strips all annotations and several different runtime checks.

@jpolitz jpolitz requested a review from blerner July 11, 2018 17:36
@SethPoulsen
Copy link
Contributor

Yes! This is exciting.

@SethPoulsen
Copy link
Contributor

SethPoulsen commented Aug 26, 2018

With this, are we going to be able to have a desktop version of Pyret available through apt and brew?

I'm happy to look into what's required to make that happen.

@jpolitz
Copy link
Member Author

jpolitz commented Aug 28, 2018

That is absolutely the plan. The stuff I've been prototyping targets npm first; it may well be that npm install -g pyret ends up being the preferred way to do this, and any apt/brew/yum and so on package would wrap that idea.

A lot of this works. If you check out https://www.npmjs.com/package/pyret-npm (which you can install with npm install -g pyret) you get a reasonable pyret command installed; the default thing to do is pyret <some-pyret-file>.arr. I'm not advertising it at all because it's super-alpha and breaks a lot, but it's on a pretty reasonable trajectory.

If you want the simplest, stupidest thing that is currently broken to push on, in this repo:

https://github.com/brownplt/pyret-npm

I updated some dependencies in npm and I broke all the help text rendering at the console (the "chalk" library having a major version change is to blame). I could downgrade it but there were a bunch of good and related reasons to update several npm dependencies. That repo autodeploys to the npm package when pushes happen on release, so it's relatively easy to take PRs and make improvements there.

The idea is that pyret-npm will have an implementation of a client for the server side that's implemented in this PR. So fixing the way all the cli args help renders in that repo would unbreak pyret-npm in a useful way.

I'd be happy to chat about longer-term plans for this as well, there's a few different directions to push in, both in terms of libraries to expose at the node level, how to integrate well with npm packages, and how to get better performance out of Pyret when we're outside the browser environment.

@SethPoulsen SethPoulsen mentioned this pull request Aug 30, 2018
@SethPoulsen
Copy link
Contributor

I've created a homebrew tap here to (hopefully) make this accessible to even more people. It will automatically install the node dependency and everything (use --ignore-dependencies if you already have node installed not via brew). Just do

brew install SethPoulsen/pyret/pyret

The way it works is by pulling from the npm archives at https://registry.npmjs.org/pyret-npm/-/pyret-npm-0.0.16.tgz, which is in parity with how the other brew core taps work when pulling npm packages.

Once @jpolitz is happy enough with the stability of what's there, I can create a PR to homebrew-core, and then it will be as simple as

brew install pyret

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.

5 participants