search

mirage / ocaml-uri

RFC3986 URI parsing library for OCaml
Other
97 stars 57 forks source link

Setup uri-bench to build benchmarking code. #166

Closed tmcgilchrist closed 1 year ago

tmcgilchrist commented 1 year ago

Fix for https://github.com/mirage/ocaml-uri/issues/165

torinnd commented 1 year ago

@tmcgilchrist was this closed intentionally? I think the CI issue is still present in master. If this was intentional, can you share more context on why this PR wasn't the right way forward?

tmcgilchrist commented 1 year ago

This was only closed because I didn’t have time to follow through with it. I’ll try to find some time today to reopen this.

The approach is still the right thing to do. It comes with the small downside that future dune-release based releases need to avoid adding uri-bench to opam repository. Dune-release could be improved to handle excluding certain pacakages?

The larger issue with opam / dune is they don’t have a concept of a benchmarking package.

On Tue, 7 Mar 2023 at 1:14 am, torinnd @.***> wrote:

@tmcgilchrist https://github.com/tmcgilchrist was this closed intentionally? I think the CI issue is still present in master. If this was intentional, can you share more context on why this PR wasn't the right way forward?

— Reply to this email directly, view it on GitHub https://github.com/mirage/ocaml-uri/pull/166#issuecomment-1456213193, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABJXOMWER5KESORXFP55PLW2XWLXANCNFSM6AAAAAAUG4XPTA . You are receiving this because you were mentioned.Message ID: @.***>

avsm commented 1 year ago

Why's a benchmarking package special (it's just an executable with extra dependencies), and why not publish it also to opam-repo? It would be useful to have benchmarking tools available quickly via opam as well, and as a separate package, it wouldn't really cause any additional friction to normal uri users.

tmcgilchrist commented 1 year ago

Why's a benchmarking package special .... ?

It's an artefact that is only interesting to developers of this package, so I don't see value in including it in opam-repository. My ideal setup is to have support for benchmark packages in opam similar to with-test and have that recognised in dune with a top level command. Like what cargo bench or cabal bench do for Rust and Haskell respectively. :-)

avsm commented 1 year ago

It's useful to have benchmarking executables also available via the package manager. They don't get in the way, and sometimes I just want to run quick tests across a couple of different compilers. Being able to opam search bench; opam install ... is fine for that sort of thing.

Or to put it another way -- I'd rather fix the CI in uri with a published uri-bench package, rather than block on some future changes to the semantics of dune-publish and friends :-)

tmcgilchrist commented 1 year ago

@avsm updated the top comment and everything is building now. Please have a look