status-im / nim-blscurve

Nim implementation of BLS signature scheme (Boneh-Lynn-Shacham) over Barreto-Lynn-Scott (BLS) curve BLS12-381
Apache License 2.0
26 stars 11 forks source link

nim CI broken: eth2_vectors.nim(17, 3) Error: cannot open file: yaml #39

Closed timotheecour closed 3 years ago

timotheecour commented 4 years ago

/cc @mratsim

https://dev.azure.com/nim-lang/255dfe86-e590-40bb-a8a2-3c0295ebdeb1/_apis/build/builds/2970/logs/68

2020-03-01T22:13:52.0997954Z PASS: https://github.com/bluenote10/nim-heap C                     ( 4.59764791 secs)
2020-03-01T22:13:52.0998413Z FAIL: https://github.com/status-im/nim-blscurve C
2020-03-01T22:13:52.0998905Z Test "https://github.com/status-im/nim-blscurve" in category "nimble-packages"
2020-03-01T22:13:52.0999336Z Failure: reBuildFailed
2020-03-01T22:13:52.0999628Z package test failed
2020-03-01T22:13:52.0999928Z $ nimble test
2020-03-01T22:13:52.1000511Z   Executing task test in D:\a\1\s\pkgstemp\blscurve\blscurve.nimble
2020-03-01T22:13:52.1000900Z 
2020-03-01T22:13:52.1001244Z [Suite] [Before IETF standard] BLS381-12 test suite (public interface)
2020-03-01T22:13:52.1001553Z 
2020-03-01T22:13:52.1002130Z [Suite] [v0.9.x] Ethereum2 specification BLS381-12 test vectors suite
2020-03-01T22:13:52.1002653Z D:\a\1\s\pkgstemp\blscurve\tests\eth2_vectors.nim(17, 3) Error: cannot open file: yaml
2020-03-01T22:13:52.1003159Z stack trace: (most recent call last)
2020-03-01T22:13:52.1003607Z C:\Users\VSSADM~1\AppData\Local\Temp\nimblecache\nimscriptapi.nim(165, 16)
2020-03-01T22:13:52.1004437Z D:\a\1\s\pkgstemp\blscurve\blscurve_6708.nims(40, 8) testTask
2020-03-01T22:13:52.1005523Z D:\a\1\s\pkgstemp\blscurve\blscurve_6708.nims(21, 8) test
2020-03-01T22:13:52.1006040Z D:\a\1\s\lib\system\nimscript.nim(252, 7) exec
2020-03-01T22:13:52.1006683Z D:\a\1\s\lib\system\nimscript.nim(252, 7) Error: unhandled exception: FAILED: nim c -d:BLS_USE_IETF_API=true --outdir:build -r --hints:off --warnings:off tests/eth2_vectors.nim [OSError]
2020-03-01T22:13:52.1007324Z        Tip: 1 messages have been suppressed, use --verbose to show them.
2020-03-01T22:13:52.1007766Z      Error: Exception raised during nimble script execution
2020-03-01T22:13:52.1008035Z 
2020-03-01T22:35:18.4345562Z PASS: https://github.com/status-im/nim-bncurve C                   (127.68649817 secs)
2020-03-01T22:35:18.4347310Z PASS: https://github.com/nim-lang/c2nim C                          ( 3.22719979 secs)

which breaks my recent PR's (eg https://github.com/nim-lang/Nim/pull/13546 )

mratsim commented 4 years ago
  1. For testing, blscurve now requires NimYAML, it's not a package dependency so I did not list it in the .nimble file
  2. In the CI setup it has been added as via a nimble install https://github.com/status-im/NimYAML@#head preceding nimble test
  3. To make matter worse, the original yaml package was broken by YAML upstream deleting a commit that prevents git submodule update to work in nimYAML https://github.com/flyx/NimYAML/issues/77. This means that requiring the yaml package will also trigger failures
  4. AFAIK we can't do requires https://github.com/status-im/NimYAML@#head in a nimble file as a work-around
  5. Nimble doesn't support task-level dependencies https://github.com/nim-lang/nimble/issues/482

Plans forward:

So I recommend you disable blscurve and probably all packages that requires NimYAML in the Nim CI for the time being.

In the future (a week or so), once official test vectors are available (in json format) we will remove the dependency on the current YAML test vectors (https://github.com/cfrg/draft-irtf-cfrg-hash-to-curve/pull/214)

timotheecour commented 4 years ago

AFAIK we can't do requires https://github.com/status-im/NimYAML@#head in a nimble file as a work-around

wouldn't that increase chances of mutually exclusive requirements, eg:

pkg1: 
requires https://github.com/status-im/NimYAML@#head
pkg2: 
requires https://github.com/status-im/NimYAML@#1234 # 
pkg3:
requires pkg1, pkg2

the alternative is:

in any case, that discussion should happen in https://github.com/nim-lang/nimble/issues otherwise will never be fixed; maybe open an issue there and link to here?

task-level dependencies can then be achieved by (untested): nimble install -d:with_foo mypkg

# mypkg.nimble:
when defined(with_foo):
requires foo
mratsim commented 4 years ago
  1. You probably missed https://github.com/flyx/NimYAML/issues/77 in my post.
  2. For the mutually exclusive requirements, it should be discussed in the lock files issue: https://github.com/nim-lang/nimble/issues/127. Being unable to install pkg3 due to conflicting requirements is not a bug.
    • We can also requires that a minimal commit or a maximum commit in the package for more flexibitlity but targeting a specific commit seems like a nice start.
    • Alternatively, Rust can somehow handle multiple conflicting requirements in the same codebase, including versions of the Rust compiler. But AFAIK that would require a lot of plumbing on Nim side to be able to do the same.
  3. Using nimble install -d:with_foo mypkg is not really an improvement over nimble install yaml mypkg for testing. Both require to be aware of specific testing dependencies.