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

opam: Remove unnecessary constraints #143

Closed
wants to merge 2 commits into from

Conversation

kit-ty-kate
Copy link

Tested locally, they all work fine.

Tested locally, they all work fine.
@phated
Copy link
Member

phated commented Mar 10, 2022

If you also update the package.json and esy.lock, you'll notice the failures on windows. We're still waiting for a fix.

@phated
Copy link
Member

phated commented Mar 10, 2022

@kit-ty-kate this actually also broke for me on MacOS. I just pushed the esy changes to show you that it's broken and we shouldn't remove these constraints until the fixes can be made upstream.

@kit-ty-kate
Copy link
Author

@kit-ty-kate this actually also broke for me on MacOS

Really? My local testing machine is macOS and was working perfectly fine

@phated
Copy link
Member

phated commented Mar 10, 2022

The error seems to be happening on dune install inside of one (all?) of the JSOO packages. It is trying to call opam, which isn't available in an esy sandbox.

@phated
Copy link
Member

phated commented Mar 10, 2022

This is the error (I'm assuming we'll see this same thing in CI soon):

$ esy
info esy 0.6.13-dev (using package.json)
info fetching: done                                                                                     
info installing: done                                                                                              
info building @opam/js_of_ocaml-ppx@opam:4.0.0@bca7f5ce
error: build failed with exit code: 1
  build log:
    # esy-build-package: building: @opam/js_of_ocaml-ppx@opam:4.0.0
    # esy-build-package: pwd: /Users/phated/.esy/3/b/opam__s__js__of__ocaml_ppx-opam__c__4.0.0-eb663ec4
    # esy-build-package: running: 'dune' 'build' '-p' 'js_of_ocaml-ppx' '-j' '4' '--promote-install-files=false' '@install'
    (cd tools/version && /Users/phated/.esy/3_________________________________________________________________/i/ocaml-4.13.1000-8a7a040f/bin/ocaml -I +compiler-libs /Users/phated/.esy/3/b/opam__s__js__of__ocaml_ppx-opam__c__4.0.0-eb663ec4/_build/.dune/default/tools/version/dune.ml)
    fatal: not a git repository (or any of the parent directories): .git
    # esy-build-package: running: 'dune' 'install' '-p' 'js_of_ocaml-ppx' '--create-install-files' 'js_of_ocaml-ppx'
    Error: Program opam not found in the tree or in PATH
    error: command failed: 'dune' 'install' '-p' 'js_of_ocaml-ppx' '--create-install-files' 'js_of_ocaml-ppx' (exited with 1)
    esy-build-package: exiting with errors above...
    
  building @opam/js_of_ocaml-ppx@opam:4.0.0
esy: exiting due to errors above

@kit-ty-kate
Copy link
Author

kit-ty-kate commented Mar 10, 2022

That sounds like ocaml/dune#5455. It looks like esy maintainers should remove dune 3 from their repository.

Then my first commit was fine on its own given only the opam file was modified.

@phated
Copy link
Member

phated commented Mar 10, 2022

It looks like esy maintainers should remove dune 3 from their repository.

There isn't an esy repository.

Then my first commit was fine on its own given only the opam file was modified.

You can install binaryen.ml with @opam/binaryen or @grain/binaryen.ml and I don't want to vary the dependencies people will get because then you can't use esy to get the @opam/binaryen version.

Please 🙏 leave these constraints in place until we can actually fix the ecosystem.

@kit-ty-kate
Copy link
Author

There isn't an esy repository.

I meant this: https://github.com/esy-ocaml/esy-opam-override

@phated
Copy link
Member

phated commented Mar 11, 2022

I meant this: https://github.com/esy-ocaml/esy-opam-override

The esy overrides project is not meant to remove a package from opam, it is mainly just to apply a patch to an opam file on a few opam packages that likely can't/won't be changed upstream. The problems with dune 3 seem to be much deeper.

@phated
Copy link
Member

phated commented May 4, 2022

Closing this, as I'm tracking the dune 3 bugs that cause it to break if opam is not available on the path. We'll open the constraints as soon as that is fixed and released!

@phated phated closed this May 4, 2022
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.

2 participants