`esy release` improvements

andreypopp commented 5 years ago

This is an umbrella issue for improvements to esy release command.

Also related (some of these are addressed here as well): #705, #436, ...

Configuration Changes Proposals

Add `"esy.release.includePackages"` [DONE]

Add "esy.release.include" configuration which will define a whitelist set of packages which should be included in a release.

The format is:

{
  "esy": {
    "release": {
      "includePackges": [<PKGPATTERN>, ...]
    }
  }
}

Where <PKGPATTERN> is one of:

Package name: "ocaml", "@opam/merlin", ...
Package name and version spec separated by @: "ocaml@4.6.x", "@opam/merlin@*", ...

esy release command should warn if pattern specified in "esy.release.includepackages" wasn't matched against any of the packages found in a project..

Example:

{
  "esy": {
    "release": {
      "include": [
        "@opam/camomile",
        "@opam/refmt@3.3.7",
      ]
    }
  }
}

Relation to the current "esy.release.deleteFromBinaryRelease": esy release should fail if both fields are present and suggest specify only "esy.release.include".

Make prefix rewriting opt-in and configurable via `"esy.release.rewritePrefix"` [DONE, but w/o hints]

Now esy tries to rewrite prefix by default but this isn't needed in most cases (unless you ship compiled code which references absolute paths to the current esy store).

I propose to make prefix rewriting opt-in and provide some heuristics to help user to diagnose possible problems.

I propose adding "esy.release.rewritePrefix" which could be:

Absent, in this case esy assumes it is set to false
false, in this case esy don't do any rewriting
true, in this case esy will do prefix rewriting (remove current prefix and include postinstall scripts which will rewrite prefix back on installation machine).

What could be done from esy side to help with that setting:

esy could check if any of the packages included in a release refer to absolute store path and issue a warning in case "esy.release.rewritePrefix": false.

Add more flexible config for released binaries [DONE]

Current "esy.release.releasedBinaries" allows to specify a set of binaries from project env to be released (exposed to outside the released sandbox) and put on $PATH.

I propose another setting "esy.release.bin" which has the familiar to npm users format and allows to set the name of the exposed binary.

There are multiple formats we could support (like original npm's "bin" field does).

Expose only a single binary named "refmt":

{
  "esy": {
    "release": {
      "bin": "refmt"
    }
  }
}

Expose multiple binaries and optionally expose them under different names:

{
  "esy": {
    "release": {
      "bin": {
        "refmt": "refmt",
        "refmt-test": "TestRefmt.exe"
      }
    }
  }
}

Relation to the current "esy.release.releasedBinaries": esy release should fail if both fields are present and suggest specify only "esy.release.bin".

Implementation Changes Proposals

The implementation of esy release command is consist of:

esy/NpmRelease.ml module
bin/esyInstallRelease.js

Get rid of `bin/esyInstallRelease.js`

I think we'd need to get rid is bin/esyInstallRelease.js - would be nice to rewrite it in Reason/OCaml and reuse esy-lib not to duplicate constants like "esy store prefix length" and logic of rewriting binaries.

Note that we can ship native code as postinstall scripts as we are shipping prebuilt binaries in release anyway. Just need to make sure we don't need to rewrite such native code as well before we can use it.

Externalize `esy release` command

Given that release commands is implemented in a single module and is very specific to npm we can consider externalizing its implementation out of esy codebase into esy-npm-release.

We'll need to see what are missing bits of esy low level commands UI we need for that.

jordwalke commented 5 years ago

All sounds great!

I propose adding "esy.release.rewritePrefix" which could be: Absent, in this case esy assumes it is set to false false, in this case esy don't do any rewriting true, in this case esy will do prefix rewriting (remove current prefix and include postinstall scripts which will rewrite prefix back on installation machine). What could be done from esy side to help with that setting: esy could check if any of the packages included in a release refer to absolute store path and issue a warning in case "esy.release.rewritePrefix": false.

What about a mode where the only paths that are rewritten are paths that refer to packages included in the release. For example, if you have a path to the ocaml compiler in your App.exe, it won't be rewritten. (That's the majority of them). If this is possible, why shouldn't this one be the default. If you have a hard coded path in your app to a package you include in the release, then isn't that a very good sign that the path needs to be correctly rewritten? (Especially if people use white lists - they only include things they actually need)

andreypopp commented 5 years ago

What about a mode where the only paths that are rewritten are paths that refer to packages included in the release. For example, if you have a path to the ocaml compiler in your App.exe, it won't be rewritten. (That's the majority of them). If this is possible, why shouldn't this one be the default. If you have a hard coded path in your app to a package you include in the release, then isn't that a very good sign that the path needs to be correctly rewritten? (Especially if people use white lists - they only include things they actually need)

I'm not sure what it buys us to rewrite only paths which refer to included packages and not others, can you elaborate why this is more useful than rewrite all paths which contain store prefixes?

esy / esy