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

Specifying --bench-on dirty is broken #59

Open
LilithHafner opened this issue Jan 10, 2025 · 2 comments
Open

Specifying --bench-on dirty is broken #59

LilithHafner opened this issue Jan 10, 2025 · 2 comments

Comments

@LilithHafner
Copy link
Contributor

This is a great error:

shell> ../../bin/benchpkg --bench-on this-ref-does-not-exist --rev dirty,main
[ Info: Downloading package's latest benchmark script, assuming it is in benchmark/benchmarks.jl
[ Info: Downloading from this-ref-does-not-exist.
ERROR: LoadError: Did not find rev this-ref-does-not-exist in repository
...

This is probably a bug:

shell> ../../bin/benchpkg --bench-on dirty --rev dirty,main
ERROR: LoadError: MethodError: no method matching length(::Nothing)
The function `length` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  length(::ExproniconLite.JLIfElse)
   @ ExproniconLite ~/.julia/packages/ExproniconLite/nABwg/src/types.jl:118
  length(::Core.SimpleVector)
   @ Base essentials.jl:933
  length(::JSON.Parser.MemoryParserState)
   @ JSON ~/.julia/packages/JSON/93Ea8/src/Parser.jl:28
  ...

Stacktrace:
 [1] benchpkg(package_name::String; rev::String, output_dir::String, script::String, exeflags::String, add::String, tune::Bool, url::String, path::String, bench_on::String, filter::String, nsamples_load_time::Int64, dont_print::Bool)
   @ AirspeedVelocity.BenchPkg ~/.julia/packages/AirspeedVelocity/8uSQr/src/BenchPkg.jl:94
 [2] benchpkg
   @ ~/.julia/packages/AirspeedVelocity/8uSQr/src/BenchPkg.jl:53 [inlined]
 [3] command_main(ARGS::Vector{String})
   @ AirspeedVelocity.BenchPkg ~/.julia/packages/Comonicon/F3QqZ/src/codegen/julia.jl:343
 [4] command_main()
   @ AirspeedVelocity.BenchPkg ~/.julia/packages/Comonicon/F3QqZ/src/codegen/julia.jl:90
 [5] top-level scope
   @ ~/.julia/bin/benchpkg:14
in expression starting at /home/x/.julia/bin/benchpkg:14
@LilithHafner
Copy link
Contributor Author

This, combined with #58 makes it hard to test benchmarks without committing them first.

@MilesCranmer
Copy link
Owner

The current way to do this to just use the -s,--script argument to specify the script explicitly, like benchpkg -s benchmark/benchmarks.jl. But yeah it would be nice if --bench-on dirty did the obvious thing!

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

No branches or pull requests

2 participants