search

mirage / checkseum

MIT License
15 stars 13 forks source link

Use which instead of command (esy) #56

Closed dinosaure closed 3 years ago

dinosaure commented 3 years ago

A possible fix for esy (/cc @sternenseemann, @mseri).

sternenseemann commented 3 years ago

Note that one downside of this is that which is not part of coreutils, so we'll need to add the extra dependency in nixpkgs.

command on the other hand is specified by POSIX. I guess depends on what you are trying to fix, exactly?

dinosaure commented 3 years ago

command on the other hand is specified by POSIX. I guess depends on what you are trying to fix, exactly?

It seems that command is not available into an esy environment and fails to install checkseum. I try to deploy a CI for esy to see what is the best solution.

mseri commented 3 years ago

What if you use it as failover, like command -v opam || which opam ?

dinosaure commented 3 years ago

So I added a CI to see if we can install checkseum via esy and apply your suggestion @mseri. Can you confirm that this PR fix your problem?

mseri commented 3 years ago

I am testing it right now

mseri commented 3 years ago

I cannot understand why it fails...

error: build failed with exit code: 1
  build log:
    # esy-build-package: building: @_linux.musl.x86_64/_opam_checkseum@path:/home/runner/.esy/mocks/f2dd81fb/esy.json
    # esy-build-package: pwd: /home/runner/.esy/3/b/__linux.musl.x86__64__s____opam__checkseum-e5c1ed72
    # esy-build-package: running: 'cp' '-r' '-p' '/home/runner/.esy/source/i/opam__s__checkseum__6349c998/.' '/home/runner/.esy/3/b/__linux.musl.x86__64__s____opam__checkseum-e5c1ed72'
    # esy-build-package: running: 'mv' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-e5c1ed72' '_____install_____'
    # esy-build-package: running: 'mkdir' '-p' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-e5c1ed72/_esy'
    # esy-build-package: running: 'mv' '_____install_____' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-e5c1ed72/linux.musl.x86_64-sysroot'
    # esy-build-package: running: 'dune' 'build' '-p' 'checkseum' '-j' '4' '-x' 'linux.musl.x86_64' '@install'
            make freestanding/libcheckseum_freestanding_stubs.a [default.linux.musl.x86_64]
    touch libcheckseum_freestanding_stubs.a
    # esy-build-package: running: './install/install.ml'
    Exception:
    Unix.Unix_error(Unix.ENOENT, "chmod", "_build/default/META.checkseum")
    error: command failed: './install/install.ml' (exited with 2)
    esy-build-package: exiting with errors above...

Maybe esy is not setting DUNE_BUILD_DIR when cross compiling? I need to ask somebody that knows it better, my knowledge is limited to using reason-mobile in the CI for cross-compilation

mseri commented 3 years ago

But at least the old part works fine, this new error needs to be fixed by esy or reason-mobile imo

dinosaure commented 3 years ago

Sorry to ask you again but can you try with 6afb373? I think it should solve the last issue.

dinosaure commented 3 years ago

Ok, so I deleted 6afb373 and replace it by a trick with the esy environement. @mseri, if you put ESY_CHECKSEUM in your global environment with this PR, you should be able to build checkseum with esy (when, in this path, we don't manipulate the META file). Again, it's possible to try my last update and add ESY_CHECKSEUM into your workflow?

It's not a good solution but it seems the best for our worlds (opam, dune, mirage and esy).

mseri commented 3 years ago

I agree that it would be the easiest option, but for some reason it still fails, as if the variable is not passed through

build log:
    # esy-build-package: building: @_linux.musl.x86_64/_opam_checkseum@path:/home/runner/.esy/mocks/d4cc0711/esy.json
    # esy-build-package: pwd: /home/runner/.esy/3/b/__linux.musl.x86__64__s____opam__checkseum-471fa34f
    # esy-build-package: running: 'cp' '-r' '-p' '/home/runner/.esy/source/i/opam__s__checkseum__e3041101/.' '/home/runner/.esy/3/b/__linux.musl.x86__64__s____opam__checkseum-471fa34f'
    # esy-build-package: running: 'mv' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-471fa34f' '_____install_____'
    # esy-build-package: running: 'mkdir' '-p' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-471fa34f/_esy'
    # esy-build-package: running: 'mv' '_____install_____' '/home/runner/.esy/3__________________________________________________________________/s/__linux.musl.x86__64__s____opam__checkseum-471fa34f/linux.musl.x86_64-sysroot'
    # esy-build-package: running: 'dune' 'build' '-p' 'checkseum' '-j' '4' '-x' 'linux.musl.x86_64' '@install'
            make freestanding/libcheckseum_freestanding_stubs.a [default.linux.musl.x86_64]
    touch libcheckseum_freestanding_stubs.a
    # esy-build-package: running: './install/install.ml'
    Exception:
    Unix.Unix_error(Unix.ENOENT, "chmod", "_build/default/META.checkseum")
    error: command failed: './install/install.ml' (exited with 2)
    esy-build-package: exiting with errors above...

  building @_linux.musl.x86_64/_opam_checkseum@path:/home/runner/.esy/mocks/d4cc0711/esy.json
esy: exiting due to errors above
mseri commented 3 years ago

(you can check how I am testing it here: https://github.com/mseri/doi2bib/pull/12)

dinosaure commented 3 years ago

Ok so I tried to play a bit with esy and my final solution is soooo bad but it's working, see: https://github.com/dinosaure/doi2bib/runs/3203366328?check_suite_focus=true#step:12:170

So I will do the release and should apply this patch for digestif too, thanks for your time @sternenseemann and @mseri.

mseri commented 3 years ago

Thanks for your time and for fixing the issue!