-
Notifications
You must be signed in to change notification settings - Fork 167
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
CLI bundle: add debug and kernel options. fixes #1447 #1597
Conversation
94da497
to
69ee5e7
Compare
69ee5e7
to
2c0b5a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for taking this one on! Left a few suggestions
Also, to get the |
b859898
to
9a7ba57
Compare
The command also includes the stdlib now (for both kernel and normal libraries). This was necessary to bundle
Not sure if the test could be added to |
9a7ba57
to
f994b76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick pass
tested with: cargo run --features executable -- bundle ~/miden/miden-base/miden-lib/asm/kernels/transaction/lib --kernel ~/miden/miden-base/miden-lib/asm/kernels/transaction/api.masm
3764cac
to
9e3c9ff
Compare
8bfdba5
to
bff4676
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Closes #1447
Improve the
bundle
command:--debug
option, which is otherwise false by default--kernel
option to pass the main kernel file and treat the directory as thekernel
namespace library.Also enforces that any library, kernel or not, has at least one export.
Note: we can still build a library w/o exports using
from_dir
which does not useLibrary::new
.