jordwalke / esy-issues Goto Github PK

We can take advantage of the esy conventions, by building editor plugins
that understand how to source the project environment when you edit files
inside of an esy project. That means people would no longer have to install
global tools in order for their editors to work correctly. Each project can
have their own version of the IDE tools as dev dependencies.
This convention could be used for everything from JS/Flow development, as
well as ReasonML and C++.

Repro: change "bs-platform": "esy-ocaml/bucklescript#esy" to "bs-platform": "/path/to/local/bucklescript", then do esy install while having an existing node_modules.

• Make a package's esy.exportedEnv only activate for packages which depend on that package.
• Introduce esy.buildEnv which activates only for the current package's build.

See https://github.com/esy-ocaml/ocamlfind build script which needs buildEnv.

It looks like we are hitting the ceil of make. Looks like make calls shell with -c and that triggers the error.

An alternative approach would be to eject into into a directory, then we can place multiple files there — Makefile itself, separate shell scripts with env for corresponding packages. I think that also have the advantage of being more observable (it is easier to look into several files of interest rather than go through the 25 KSLOC Makefile).

Currently than we build sandbox repr in memory — we don't memoize node module resolution results. Try adding core (a package with a great number of interdeps) and see perf suffer a lot. I tried a basic memoization and runtime dropped from 30s to <1s.

Before doing the full switch to esy, we have to convert packages.

This issue tracks migrating all the automated packages, but we also need to migrate some of the individually crafted packages that used dependency-env.

Among them are:

• https://github.com/reasonml/BetterErrors/
• https://github.com/reasonml/rebel/blob/master/package.json
• github.com/facebook/reason

Add any you think of here.

We can try to build support in esy for supporting dependency-env simultaneously, or we could all block off a few hours at the same time, and just aggressively port everything forward.

I'm making global packages for things like refmt, and they expose wrapper scripts that simply defer to esy refmt. That's great, but that seems to be incurring an additional 50ms each invocation at least. For local project development that is wonderfully fast! Part of that time goes into checking if any package.jsons were modified and it's worth it. However, for global packages, nothing ever changes, so I think wrapper scripts could benefit from an esy-cached command version.

cc @SanderSpies

I believe opam has a separate install step. I think this could be good, but I'm not sure how valuable it would be.

We should test that when bumping a version, or adding a package, that only packages that depend on the old/new package version need to be rebuilt.

Pulled out from #53 (comment)

It'd be great to selectively purge packages. Helps coping with #53 and #59

Is there a way to say: "Rebuild this current project quickly without rebuilding dependencies - unless I need to build those dependencies"?

I imagine right now, some of the time spend is checking if dependencies are rebuilt, and I'm curious if there's a way to quickly rebuild the top level project, when it's expected that it's probably (but not definitely) the case that dependencies don't need to be rebuilt. In other words could there be something like esy buildthis that optimizes for the case when no dependencies have changed (but still recompiles them if they have changed (or haven't been built)).

We need to model optional dependencies in Esy.

Motivation

cohttp has backends both for async and lwt concurrency libraries but most apps are built around either one or another.

Currently cohttp has lwt and async listed in its depopts in the opam file. That means that with opam you don't get both lwt and async pulled and built when you use cohttp — you need to specify either lwt or async in your own dependencies to have one of them ready to use.

Proposal

I propose to add conditionalDependencies (optionalDependencies name is already taken by npm and doesn't have the needed mechanics behind it).

The suggestion is to use the same semantics as for peerDependencies but not fail if those packages mentioned in conditionalDependencies are absent from installation.

If package optdep is listed in conditionalDependencies for the package pkg:

• optdep is not being installed automatically (same as with peerDependencies) for the pkg
• Unlike with peerDependencies it is not an error to have no optdep installed at all.
• If optdep is present in the installation (required by some of packages above) then it's being pulled into pkg's env.

It would be great to render to ninja instead of makefiles, so that we could run builds on windows as well.

(Powershell is another option)

It might be nice to have an "explain" feature, that describes why it is/isn't rebuilding a package as part of esy build.

It might be nice to also have a json form of the environment variables, build commands, etc.

(This is lower priority, but might be trivial to implement0.

Right now it gives:

(node:98869) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: fetch failed with status code 404
Error: fetch failed with status code 404
    at Request.<anonymous> (/usr/local/lib/node_modules/esy/node_modules/npm-registry-client/lib/fetch.js:58:14)
    at emitOne (events.js:96:13)
    at Request.emit (events.js:188:7)
    at Request.onRequestResponse (/usr/local/lib/node_modules/esy/node_modules/request/request.js:986:10)
    at emitOne (events.js:96:13)
    at ClientRequest.emit (events.js:188:7)
    at HTTPParser.parserOnIncomingClient (_http_client.js:472:21)
    at HTTPParser.parserOnHeadersComplete (_http_common.js:99:23)
    at TLSSocket.socketOnData (_http_client.js:361:20)
    at emitOne (events.js:96:13)
    at TLSSocket.emit (events.js:188:7)
    at readableAddChunk (_stream_readable.js:176:18)
    at TLSSocket.Readable.push (_stream_readable.js:134:10)
    at TLSWrap.onread (net.js:543:20)

Add new key in package.json called esy.sandboxEnv.

Motivation:

• Set OCAMLRUNPARAM=b so we can have stack traces.
• Override C compiler (e.g. choose between clang or gcc)

It should have the same format as esy.exportedEnv but the mechanics should be the following:

• Only top level package's esy.sandboxEnv is considered.
• Env vars in esy.sandboxEnv should be propagated to all package's builds.
• Make sure those env vars participate in build cache key.

See https://github.com/jordwalke/esy/blob/9eed98f8024c3a64502387f021db7fe10ed0680a/lib/Sandbox.js#L189-L205 for the existing sandox env config. The proposed esy.sandboxEnv key is a mechanism to override this environment.

Right now, if you have a local package dependency such as symlink, esy will just rebuild everything any time you do esy build. esy should hash the package contents to detect if a rebuild is actually necessary.

I believe for local file system dependencies file://blah there are other issues with not detecting changes correctly perhaps.

We can make npm-opam support both forms until esy becomes the standard.

https://github.com/yunxing/opam-npm

We should also consider having better documentation for how people can convert their own packages.

We should probably handle --help, and --help, and --version, -v. I'm not sure if this should be handled at the bash wrapper, or the JS implementation, since usually by the time it gets to the JS implementation, its primary job is to just run the command that follows esy commandHere within the project's environment.

@kennetpostigo

This issue is not related directly to Esy but I figured out it would be useful to keep track of it somewhere.

Currently we install ocamlfind with -no-topfind because ocamlfind installs it into the OCaml compiler stdlib location which isn't allowed as we may have multiple ocamlfind packages installed for a single ocaml compiler. Moreover when #17 is merged — such behaviour will be caught and build will be failed in such cases.

Without topfind installed the #use "topfind"; will fail in OCaml toplevel. For example opam packages which depend on topkg build system (astring, alcotest, ...) will fail its build.

What we can do:

1. Patch ocamlfind to install topfind into its own lib/ directory.
2. Make ocamlfind export OCAML_TOPLEVEL_PATH env var pointing to its own lib directory. this env var is respected by some tools (for example solvuu-build generates .ocamlinit which respects it, unfortunately not topkg).
3. Make builds script which use topkg execute build with ocaml -I $OCAML_TOPLEVEL_PATH ...

Nix uses sandbox-exec on macOS and something on Linux (cgroups?) to catch sandbox violations. We should do the same.

What we need is to allow writes only to $cur__target_dir and $cur__install and reads from some whitelisted locations + dependencies' installation locations.

If I've already built esy build, then I notice that esy build-shell sometimes rebuilds packages that should have already been built. Do you know why @andreypopp

macOS doesn't have neither realpath nor readlink -f.

Right now we just make them build within the source directory but with #6 they will fail.

Instead:

• Assume by default packages support out of source builds and respect $cur__target_dir. Again #6 will catch the violations here.
• Make an opt-out mechanism for packages which doesn't support out of source builds.
• For packages which don't support out of source build we need to copy its sources over to $cur__target_dir on each build.

We should build in temp dir and mv to the esy's store when build is finished.

Repro:

• install https://github.com/andreypopp/esy-ocaml-project with yarn
• Run npm run build twice.
• See the failures.

Instead we shoulnd't fail on such builds.

What if esy first checked for matching hashes in yourPackage/.esy/store/ and reused the local artifacts if they were present. It could print out why it did/did not use the package's local artifacts in a helpful message. That way you could publish a package with prebuilt artifacts for things like ocaml/rebel.

This is just a stop gap because I think a better system wouldn't even require checking in artifacts into the package (it would be another tool ideally integrated into the package manager) but this should help us get some heavy things off the ground.

This is lower priority imho.

For complied languages, we really want a package resolver that has two features:

• By default, resolver should resolve constraints to a single version. This is needed so that the linker is not confused about which object to link with.
  For example, consider the following case:

  a project A depends on B and C
  B depends on D>= 0.0.1
  C depends on D = 0.0.1
  D has two versions, 0.0.1 and 0.0.2

The resolver should think hard and pick [email protected] for both B and C.

• On the other hand, we need the notion of "build time" dependency, which allows multiple packages to coexist.
  Consider the following case:

  a project A depends on B and C
  B depends on D = 0.0.2
  C depends on D = 0.0.1
  D has two versions, 0.0.1 and 0.0.2, and is marked as "build Time dependency" 

The resolver should pick both versions of D, and create a disk structure like:

A/node_modules/B/node_modules/[email protected]
A/node_modules/C/node_modules/[email protected]

Unfortunately, at this moment both npm and yarn don't support a smart resolver like this: yarnpkg/yarn#579.

What we can do in esy is read the constraints listed in 'package.json' file, and use our customized resolver to generate a yarn.lock or shrinkwrap file. After a file is generated, we can then delegate to yarn or npm to fetch the dependencies for us, and then call esy build to build the dependencies.

With esy resolve and esy build, first time users should be able to just work on the layer of esy, without knowing much details about yarn / npm.

Installing the same local path makes esy thing it should retrieve content from cache, even when the content has changed.

Related: #53 (comment)

This breaks and is a very common pattern in js land. We should warn/prevent this here to avoid reinstall.

We should document the buildsInSource field and add it to the PackageJsonForCompilers spec.

Right now esy shell is a almost-"pure" shell (almost same env which is used for sandboxed builds). That purity isn't what a user expects when running own commands but something which is valuable when debugging a broken build.

Instead what we should do:

• esy shell is an alias to esy $SHELL and see #7 for improvements there.
• esy pure-shell command is a "pure" shell — it should be exactly the same env which is used for building a package.

Currently esy <command> overrides $PATH env var so some of a user's commands stop working. We should make it not override $PATH but augment it instead.

Test and document that using symbolic links work. I think this requires that we have implemented proper out of source builds. (Either by packages natively supporting them, or by copying over ones that don't).

• Fix Env Dirty Computation #76
• Decide and implement general approach.

Here's an example error I got when running an esy build command: (don't ask me why lodash was a dependency)

/Users/me/github/esy-ocaml-project/node_modules/.cache/esy-env: line 2175: export: `lodash.clonedeep__name=lodash.clonedeep': not a valid identifier

@jordwalke suggests:

Try making every findlib.conf have:

ocamlc     = "ocamlc.opt"
ocamlopt   = "ocamlopt.opt"
ocamlcp    = "ocamlcp.opt"
ocamlmktop = "ocamlmktop.opt"

I think that should speed up every compilation aside from the ocaml compiler itself.
I bet it takes my cold cache esy build down from 4m to 3m.

See this line here:
https://github.com/jordwalke/esy/blob/master/.bin/esy#L5

Can our own provided realpath implementation work correctly instead of this snippet?

@yunxing @andreypopp

1. We need to update the docs to have people do git clone --recursive if cloning esy (I think).
2. Update the docs to indicate that people need to cd esy/opam-packages-conversion/.
3. I'm seeing a bunch of errors when running make

Here's some examples:

Traceback (most recent call last):
  File "./bin/build-package", line 79, in <module>
    build_package(name, *versions)
  File "./bin/build-package", line 62, in build_package
    os.path.join(package_dir, name + '.' + version)
  File "/Users/me/github/esy/opam-packages-conversion/bin/lib.py", line 20, in generate_package_json
    package_url = re.search(r"(archive:\s*|http:\s*|src:\s*)\"(.*)\"", content).group(2)
AttributeError: 'NoneType' object has no attribute 'group'
*** building ppx_type_conv

And:

Traceback (most recent call last):
  File "./bin/build-package", line 79, in <module>
    build_package(name, *versions)
  File "./bin/build-package", line 62, in build_package
    os.path.join(package_dir, name + '.' + version)
  File "/Users/me/github/esy/opam-packages-conversion/bin/lib.py", line 243, in generate_package_json
    buildFlatList(d["substs"]), breakList(d["build"]) + breakList(d["install"]))
  File "/Users/me/github/esy/opam-packages-conversion/bin/lib.py", line 191, in createPostInstallCommand
    return [unescapeBuiltinVariables(cmd) for cmd in build]
  File "/Users/me/github/esy/opam-packages-conversion/bin/lib.py", line 158, in unescapeBuiltinVariables
    return re.sub(r"%\{(.*?)\}%", escape, s)
  File "/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 155, in sub
    return _compile(pattern, flags).sub(repl, string, count)
  File "/Users/me/github/esy/opam-packages-conversion/bin/lib.py", line 157, in escape
    raise Exception("Cannot expand variable %s" % var)
Exception: Cannot expand variable share

#26 introduces dependency on python, while python is available out of the box on most systems we still want to get rid of it because of perf reasons

We need to implement locking so that concurrent builds for the same package won't race.

See flock or... suggestions?

Right now, to compute environment variables for one package, we must traverse its transitive dependency graph. (to compute "globally scoped" vars) That wouldn't be so bad except for the fact that we do that for every package, redoing the same traversal. There's an easy memoization we should apply here.

• The standalone command esy could do something better than merely print environment variables.
  • It could teach you how to use esy.
  • It could quickly report the health of the project (what's installed/missing/built/building/cached)
  • It could help you debug the most common issues (why your editor tooling / environment isn't working) etc.
  • Which packages are symlinked. When was the last time a file in that symlinked package was modified? When was the last tie it was built?

Another possibility is that esy build and esy install by default are sent to the background, and the esy command could show the progress for the current project.

When I run ./node_modules/esy/.bin/esy.js build-eject in windows (msys), I got the following error:

ejecting: _esy\sandbox.sb.in
ejecting: _esy\bin\render-env
fs.js:1154
  return binding.chmod(pathModule._makeLong(path), modeNum(mode));
                 ^

TypeError: mode must be an integer
    at TypeError (native)
    at Object.fs.chmodSync (fs.js:1154:18)
    at emitFile (C:\MinGW\msys\1.0\home\giraud\esy-ocaml-project\node_modules\esy\lib\esyBuildEjectCommand.js:50:10)
    at buildEjectCommand (C:\MinGW\msys\1.0\home\giraud\esy-ocaml-project\node_modules\esy\lib\esyBuildEjectCommand.js:324:3)
    at build-eject (C:\MinGW\msys\1.0\home\giraud\esy-ocaml-project\node_modules\esy\.bin\esy.js:145:5)
    at Object.<anonymous> (C:\MinGW\msys\1.0\home\giraud\esy-ocaml-project\node_modules\esy\.bin\esy.js:166:5)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)

the fs.constants.S_IRWXU is undefined on my host.

What do people think about the following feature:

esy scriptName

Could call the script named scriptName, stored in the "scripts" section of package.json. This would be similar to npm run scriptName, but it would construct an environment from esy (as if you were building that package using script name scriptName).

Yarn also has a similar feature - the only difference being that we would want to construct the environment according to esy's approach.

The way I think about "scripts", in this case, is that they are like named binaries placed in the path of esy commands executed in that sandbox. I imagine if for every script name, there was an executable by that same name created implicitly in the directory that you execute esy commands.

Document the currently supported commands.

An alternative to npm install --global.

Two options I see now.

1. Suggest users to make some default esy sandbox and do install there, also source its environment in .profile.

2. Implement esy linkapp command which produces a wrapper script in /usr/local/bin with embedded env and execs the needed exectable.

That will help handle esy upgrades.

I think I've encountered some problems around this. I've force pushed to a few repos. The install hung forever. Sorry for being vague, can't get a good repro right now. Feel free to close if there's no such problem!

In the following check (and similar ones) we check [$TARGET -ot dependency/package.json] (if the target is older than package.json changes), but I think we also want to check dependency/ itself (not just the package.json) just in case dependency/ is a symlink. I briefly tried adding the check for [ $TARGET -ot dependency] however I think that -ot doesn't work on symlinks (or works differently - perhaps passing through the the linked directory?) We want to check the symlink change time itself in addition to the package.json change time.

needRebuildTarget () {
  TARGET="$1"
  NEED_REBUILD="false"

  # check if target exist
  if [ ! -f "$TARGET" ]; then
    NEED_REBUILD="true"
  else
    # check sandbox package.json
    if [ "$TARGET" -ot "$ESY__SANDBOX/package.json" ]; then
      NEED_REBUILD="true"
    else
      # check each dependencies' package.json
      for dep in $DEPENDENCIES_PACKAGE_JSON; do
        if [ "$TARGET" -ot "$dep" ]; then
          NEED_REBUILD="true"
          break
        fi
      done
    fi
  fi

  echo "$NEED_REBUILD"
}